Skip to content

ci(terraform): restrict the privileged terraform job to non-fork PRs#2682

Open
adilburaksen wants to merge 1 commit into
bazelbuild:masterfrom
adilburaksen:fix-terraform-fork-guard
Open

ci(terraform): restrict the privileged terraform job to non-fork PRs#2682
adilburaksen wants to merge 1 commit into
bazelbuild:masterfrom
adilburaksen:fix-terraform-fork-guard

Conversation

@adilburaksen

Copy link
Copy Markdown

Problem

.github/workflows/terraform.yml triggers on pull_request for buildkite/terraform/**, and the privileged terraform job:

  • authenticates to GCP with credentials_json: ${{ secrets.GCP_SA_KEY }}, and
  • runs terraform init / terraform plan over the pull request's own .tf files, with TF_VAR_buildkite_api_token: ${{ secrets.BUILDKITE_API_TOKEN }} in the environment.

On a pull_request event the default actions/checkout checks out the fork's code, and terraform init/plan execute attacker-controlled HCL — module source fetching and data "external" blocks run arbitrary programs during plan. So a fork PR that edits a file under buildkite/terraform/<org>/ can run arbitrary commands in a job that already holds the GCP service-account key and the Buildkite API token, which is enough to exfiltrate both.

The job's guard only checks that orgs changed:

    # Run only if we have changed orgs AND (it's a push to master OR PR from trusted users)
    if: needs.detect-changes.outputs.orgs != '[]'

The "PR from trusted users" part of that comment was never implemented, and the bazel / bazel-trusted / bazel-testing environments have no required reviewers, so nothing currently prevents a fork PR from reaching this job (the only remaining barrier is GitHub's "approve fork PR workflows" click, which is routinely granted to prior contributors).

Fix

Gate the privileged job so fork pull requests can no longer reach it, while keeping push and same-repo PR behaviour unchanged:

    if: >-
      needs.detect-changes.outputs.orgs != '[]'
      && (github.event_name == 'push'
          || github.event.pull_request.head.repo.full_name == github.repository)

This implements the originally-intended "trusted users" guard. (Alternatively / additionally, the three environments could be configured with required reviewers.)

The terraform job authenticates with GCP credentials and runs
`terraform init`/`plan` over the pull request's own .tf files. On
`pull_request` this checks out fork-controlled code, and terraform
init/plan execute that HCL (module sources, `external` data sources),
so a fork PR can run arbitrary commands while the GCP service-account
key and the Buildkite API token are in scope.

The job's `if:` only checked that orgs changed; the intended
"PR from trusted users" guard was never implemented, and the matrix
environments (bazel / bazel-trusted / bazel-testing) have no required
reviewers, so nothing stops a fork PR from reaching this job.

Gate the job so fork pull requests can no longer reach it: run on push,
or on pull requests that originate from this repository.
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