Skip to content

build: skip GCP secret retrieval and e2e tests for fork PRs#1398

Open
adilburaksen wants to merge 1 commit into
cdapio:developfrom
adilburaksen:fix/fork-pr-gcp-secret-exposure
Open

build: skip GCP secret retrieval and e2e tests for fork PRs#1398
adilburaksen wants to merge 1 commit into
cdapio:developfrom
adilburaksen:fix/fork-pr-gcp-secret-exposure

Conversation

@adilburaksen
Copy link
Copy Markdown

The build workflow uses a self-hosted k8s runner that has ambient GCP workload identity. When a fork PR receives the build label, the workflow checks out the fork's code, then calls get-secretmanager-secrets to retrieve CDAP_UI_E2E_GCP_SERVICE_ACCOUNT_CONTENTS and SCM_TEST_REPO_PAT from Secret Manager — using the runner's ambient credentials, not a GitHub secret. The service account key is then written to key_file.json on disk before the fork-controlled build script runs.

This means a fork contributor whose PR gets labeled build can read GCP credentials and the SCM PAT from their own build script. Run #22436281006 (head: GnsP/cdap-ui, a fork) shows the secret retrieval step completing successfully for an external contributor.

Fix: gate both the secret retrieval step and the test step on head.repo.full_name == github.repository. Fork PRs still run the build and unit test steps — only the GCP-backed e2e tests are skipped, since those require credentials that should not be accessible to unreviewed fork code.

The build workflow retrieves GCP credentials from Secret Manager using the
k8s runner's ambient workload identity, then writes the service account key
to disk and passes it to e2e tests. Because the workflow triggers on
pull_request (including fork PRs) and the runner has ambient GCP access,
a fork PR with a labeled build trigger can execute attacker-controlled code
with access to those credentials.

Gate the two affected steps on same-repository PRs and non-PR events.
Fork PRs will still run the build and unit test steps; e2e tests require
a same-repo branch where credentials can be safely accessed.
@gemini-code-assist
Copy link
Copy Markdown

Note

Gemini is unable to generate a review for this pull request due to the file types involved not being currently supported.

adilburaksen added a commit to adilburaksen/cdap-ui that referenced this pull request May 11, 2026
Demonstrates that fork-controlled build script runs after GCP
secrets are written to disk in build.yml (steps 6→10).

poc.js reads key_file.json structure and env var presence only.
No credential content is exfiltrated or transmitted.

Ref: cdapio#1398
@adilburaksen
Copy link
Copy Markdown
Author

@GnsP Hi! This PR fixes a security issue I reported to Google VRP — fork PRs from external contributors were triggering k8s runners with ambient GCP WIF credentials, allowing SA key and SCM PAT exfiltration. The fix skips secret retrieval and e2e tests for fork PRs. Could you take a look when you have a moment? Happy to provide more context if needed. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant