Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/terraform-plan-gcs-transport.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"generate-terraform-plan": major
"apply-terraform-plan": major
---

Stop committing the Terraform binary plan file to git. A binary plan embeds a full copy of the Terraform state, including sensitive values in cleartext, and the tip-delete cleanup never removed it from git history.

generate-terraform-plan now uploads the binary plan to a GCS object keyed by a slug of the terraform path plus the plan's commit SHA, and the plan PR commits only the redacted plan text plus a small pointer file (default `tfplan.commit`) recording that SHA and the plan's sha256 digest. apply-terraform-plan reads the pointer, rebuilds the object path from its own inputs, downloads the plan, verifies its digest against the pointer (so the applied bytes are exactly the reviewed plan), applies it, and best-effort deletes the object once the cleanup PR has merged.

Breaking changes: both actions gain a `plan_bucket` input (required for the plan-PR/apply flow; not needed for PR-comment-only or auto_approve usage) and must be upgraded together. Consumers need a GCS bucket that the plan workflow's service account can write objects to and the apply workflow's service account can read (and ideally delete) objects from, plus a lifecycle rule to expire plans that are never applied (e.g. superseded plan PRs).

Consumer workflows must also update their trigger paths: apply workflows typically trigger on `push` with a `paths` filter on `tfplan.binary`, which is never committed anymore. Point that filter (and any `!tfplan.binary` excludes in plan workflows) at the pointer file (default `tfplan.commit`) instead, or apply will never run.
Comment thread
khan-actions-bot marked this conversation as resolved.

Security model for the bucket: a binary plan contains the same sensitive data as the Terraform state itself, so treat the plan bucket exactly like the state bucket. Create it in the same project, enable uniform bucket-level access and public access prevention, and grant object access only to the CI service accounts (no human-facing grants). Note that project-level IAM (owners/editors, storage admins) inherits access to any bucket in the project; that audience can already read the state bucket, so the plan bucket exposes nothing new to it.

Note: this only stops new leaks. Any `tfplan.binary` still committed at a repo's tip should be deleted, and histories that ever contained committed binary plans still need scrubbing and rotation of any secrets present in the embedded state.
103 changes: 93 additions & 10 deletions actions/apply-terraform-plan/action.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
# This workflow applies checked-in Terraform plans created by the `terraform-plan` workflow
# This workflow applies Terraform plans created by the `terraform-plan` workflow
# and can be called from any repository. It handles:
# - Checking for plan files (unless auto_approve is enabled)
# - Checking for the committed plan pointer/text files (unless auto_approve is enabled)
# - Authenticating with GCP
# - Downloading the binary plan from GCS (binary plans embed a full copy of the
# Terraform state, including sensitive values, so they are never committed)
# - Applying the Terraform plan (or running a blind apply with -auto-approve)
# - Cleaning up plan files
# - Commenting on PRs for success/failure
Expand All @@ -20,6 +22,14 @@ inputs:
description: "Path to the plan text file (relative to terraform_path)"
required: false
default: "tfplan.txt"
plan_pointer_path:
description: "Path to the committed pointer file (relative to terraform_path) recording the commit SHA whose uploaded binary plan should be applied and that plan's sha256 digest"
required: false
default: "tfplan.commit"
plan_bucket:
description: "GCS bucket, optionally with a path prefix, that the generate-terraform-plan action uploaded the binary plan to. Must match that action's plan_bucket input. Required unless auto_approve is 'true'. The service account needs permission to read (and, for cleanup, delete) objects in this bucket. A binary plan contains the same sensitive data as the Terraform state, so restrict this bucket exactly like the state bucket: CI service accounts only, no human-facing grants."
required: false
default: ""
gcloud_project:
description: "Google Cloud project ID"
required: true
Expand Down Expand Up @@ -68,8 +78,9 @@ runs:
if [ "${{ inputs.auto_approve }}" == "true" ]; then
echo "Auto-approve mode enabled, skipping plan file check"
echo "skip_apply=false" >> "$GITHUB_OUTPUT"
elif [ ! -f "${{ inputs.terraform_path }}/${{ inputs.plan_file_path }}" ] || [ ! -f "${{ inputs.terraform_path }}/${{ inputs.plan_text_path }}" ]; then
elif [ ! -f "${{ inputs.terraform_path }}/${{ inputs.plan_pointer_path }}" ] || [ ! -f "${{ inputs.terraform_path }}/${{ inputs.plan_text_path }}" ]; then
echo "Plan files not found, skipping apply steps"
echo "::warning::Plan pointer/text files not found under ${{ inputs.terraform_path }}; skipping apply. If a plan PR was just merged, check that this workflow's paths filter watches ${{ inputs.plan_pointer_path }} (not the old tfplan.binary)."
echo "skip_apply=true" >> "$GITHUB_OUTPUT"
else
echo "skip_apply=false" >> "$GITHUB_OUTPUT"
Expand All @@ -87,6 +98,55 @@ runs:
workload_identity_provider: ${{ inputs.workload_identity_provider }}
service_account: ${{ inputs.service_account }}

# generate-terraform-plan uploads the binary plan to GCS (keyed by a slug
# of the terraform path plus the commit SHA the plan was generated from)
# and commits only a pointer file, because the binary embeds a full copy
# of the Terraform state, including sensitive values in cleartext.
# Rebuilding the object path here from our own plan_bucket input, rather
# than trusting a URI committed to the repo, means a tampered pointer can
# at worst select another plan previously uploaded for this same stack.
- name: Download binary plan from GCS
if: steps.check_files.outputs.skip_apply != 'true' && inputs.auto_approve != 'true'
id: download
shell: bash
working-directory: ${{ inputs.terraform_path }}
run: |
if [ -z "${{ inputs.plan_bucket }}" ]; then
echo "The plan_bucket input is required to download the binary plan uploaded by generate-terraform-plan." >&2
exit 1
fi
plan_sha=$(sed -n '1p' "${{ inputs.plan_pointer_path }}" | tr -d '[:space:]')
plan_hash=$(sed -n '2p' "${{ inputs.plan_pointer_path }}" | tr -d '[:space:]')
if ! printf '%s' "$plan_sha" | grep -Eq '^[0-9a-f]{40}$'; then
echo "Pointer file ${{ inputs.plan_pointer_path }} line 1 does not contain a valid commit SHA: '$plan_sha'" >&2
exit 1
fi
if ! printf '%s' "$plan_hash" | grep -Eq '^[0-9a-f]{64}$'; then
echo "Pointer file ${{ inputs.plan_pointer_path }} line 2 does not contain a valid sha256 plan digest: '$plan_hash'" >&2
exit 1
fi
# Slug the terraform path the same way generate-terraform-plan does;
# utils/terraform-plan-object-key.test.ts pins the two copies to each
# other. The short hash suffix keeps two terraform paths that
# normalize to the same slug from colliding on an object key.
slug=$(printf '%s' "${{ inputs.terraform_path }}" | tr -c 'A-Za-z0-9._-' '-' | sed -E 's/-+/-/g; s/^-//; s/-$//')
slug="${slug}-$(printf '%s' "${{ inputs.terraform_path }}" | sha256sum | cut -c1-8)"
# The object's filename segment is a fixed literal on both sides so
# the key can't depend on the two actions' independently configurable
# local filename inputs.
object="gs://${{ inputs.plan_bucket }}/${slug}/${plan_sha}/tfplan.binary"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): Optional design thought, take it or leave it: the object could be content-addressed by the plan's sha256 (`(redacted) instead of slug + commit SHA. The pointer's digest is already the trust anchor apply enforces, so this keeps the tamper property, subsumes the per-stack collision concern the slug hash-suffix exists for, and would let you drop the duplicated slug pipeline in both actions plus most of the pinning test. One tradeoff to weigh: a content-addressed key no longer scopes by stack, so a tampered pointer could select any plan ever uploaded to the bucket rather than only this stack's — the current "at worst another plan for this same stack" property would widen. Non-blocking; the current scheme is correct as written.

gcloud storage cp "$object" "${{ inputs.plan_file_path }}"
Comment thread
khan-actions-bot marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note (non-blocking): Re-running this workflow on the same commit after a completed apply is not idempotent. Once apply succeeds, the cleanup PR merges, and the object is deleted, a GitHub "Re-run all jobs" re-checks out the same commit — whose committed pointer the cleanup PR never touched — so check_files sets skip_apply=false and this gcloud storage cp 404s on the deleted object. With no continue-on-error, the re-run fails here rather than skipping cleanly. This is inherent to the (correct) delete-after-apply ordering; the bucket lifecycle rule and a fresh plan are the intended recovery. Flagging only so operators know re-runs are not safe to retry blindly.

# Verify the downloaded bytes are exactly the plan that was reviewed;
# the object at this key could in principle have been overwritten
# since the plan PR was approved (e.g. by a re-run of the plan
# workflow on the same commit after state drift).
actual_hash=$(sha256sum "${{ inputs.plan_file_path }}" | cut -d' ' -f1)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): The digest-verify-before-apply path — the security control this PR is built around — has no test. Nothing in utils/*.test.ts exercises the pointer parse (sed -n '1p'/'2p'), the ^[0-9a-f]{40}$/{64}$ validators, or this actual_hash != plan_hash comparison; CI runs only vitest. If the comparison were ever inverted or the pointer's two lines swapped, an unreviewed plan would apply and no test would catch it. The test file already extracts and execSync-es action shell lines for the slug pipeline, so this logic is testable the same way: round-trip a written pointer, assert the validators reject a 63-hex digest / 41-char SHA / whitespace, and assert the mismatch branch exits 1.

if [ "$actual_hash" != "$plan_hash" ]; then
echo "Downloaded plan digest mismatch: the pointer records $plan_hash but the object has $actual_hash. Refusing to apply an unreviewed plan." >&2
exit 1
fi
echo "object=$object" >> "$GITHUB_OUTPUT"

- name: Terraform Init
if: steps.check_files.outputs.skip_apply != 'true'
shell: bash
Expand All @@ -106,9 +166,12 @@ runs:
fi
continue-on-error: true

# Once the plan is applied, we remove the checked-in plan files to clean up.
# This prevents stale plans from being applied and ensures that each
# apply job has a fresh plan generated for it.
# Once the plan is applied, we remove the committed pointer and plan text
# files to clean up. This prevents stale plans from being applied and
# ensures that each apply job has a fresh plan generated for it. The
# uploaded binary plan is deleted from GCS only after the cleanup PR
# merges (see the final cleanup step below), so a cleanup failure never
# leaves the base branch pointing at a missing object.
#
# The base branch (typically `master`/`main`) is protected and rejects
# direct pushes, so rather than committing and pushing the cleanup directly
Expand All @@ -129,7 +192,9 @@ runs:
run: |
# create-pull-request stages these deletions itself via add-paths; a
# plain rm is more robust than `git rm -f` when a file is already gone.
rm -f "${{ inputs.terraform_path }}/${{ inputs.plan_file_path }}" "${{ inputs.terraform_path }}/${{ inputs.plan_text_path }}"
# The downloaded binary is untracked but removed too, so nothing later
# in the job can pick up an already-applied plan.
rm -f "${{ inputs.terraform_path }}/${{ inputs.plan_pointer_path }}" "${{ inputs.terraform_path }}/${{ inputs.plan_text_path }}" "${{ inputs.terraform_path }}/${{ inputs.plan_file_path }}"

# Scope the cleanup branch to this terraform stack. If two apply jobs
# for different stacks run concurrently in the same repo, a shared
Expand All @@ -154,12 +219,12 @@ runs:
body: |
This PR was automatically generated by the `${{ github.workflow }}` workflow after the Terraform plan was applied.

It removes the checked-in plan files so that a stale plan can't be re-applied and the next run generates a fresh plan. It is merged automatically to complete the apply cycle.
It removes the committed plan pointer and plan text files so that a stale plan can't be re-applied and the next run generates a fresh plan. It is merged automatically to complete the apply cycle.

Plan file: `${{ inputs.terraform_path }}/${{ inputs.plan_file_path }}`
Plan pointer file: `${{ inputs.terraform_path }}/${{ inputs.plan_pointer_path }}`
Plaintext plan file: `${{ inputs.terraform_path }}/${{ inputs.plan_text_path }}`
add-paths: |
${{ inputs.terraform_path }}/${{ inputs.plan_file_path }}
${{ inputs.terraform_path }}/${{ inputs.plan_pointer_path }}
${{ inputs.terraform_path }}/${{ inputs.plan_text_path }}

- name: Merge plan-file cleanup PR
Expand All @@ -183,6 +248,24 @@ runs:
echo "Failed to merge cleanup PR #$PR_NUMBER after several attempts" >&2
exit 1

# Delete the uploaded binary plan only after the cleanup PR has merged.
# If the object were deleted first and the cleanup PR then failed to
# create or merge, the base branch would keep a pointer to a missing
# object and every re-run of this workflow would hard-fail at download
# until a new plan regenerates. (This step is skipped automatically if
# any earlier step failed.)
- name: Delete uploaded plan from GCS
if: steps.apply.outcome == 'success' && inputs.cleanup_plan_files == 'true' && inputs.auto_approve != 'true' && steps.download.outputs.object != ''
shell: bash
run: |
# Best-effort: a failure (e.g. a service account without delete
# permission) shouldn't fail the run since the apply already
# succeeded and the pointer is gone. A bucket lifecycle rule should
# expire anything this misses, including plans that are never applied.
if ! gcloud storage rm "${{ steps.download.outputs.object }}"; then
echo "Warning: failed to delete ${{ steps.download.outputs.object }}; relying on the bucket lifecycle rule." >&2
fi

- name: Find merged PR
if: steps.apply.outcome == 'success' || steps.apply.outcome == 'failure'
id: find_pr
Expand Down
65 changes: 62 additions & 3 deletions actions/generate-terraform-plan/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
# - Checking for plan files
# - Authenticating with GCP
# - Generating a Terraform plan
# - Handling PR creation/comments
# - Uploading the binary plan to GCS (binary plans embed a full copy of the
# Terraform state, including sensitive values, so they are never committed)
# - Handling PR creation/comments (the PR contains only the redacted plan
# text and a pointer file recording which uploaded plan to apply)
# - Commenting on PRs for success/failure
# All environment variables starting with TF_VAR_ will be automatically picked up by Terraform
# Example: TF_VAR_project_id, TF_VAR_region, etc.
Expand All @@ -23,6 +26,14 @@ inputs:
description: "Filename for the text plan file"
required: false
default: "tfplan.txt"
plan_file_pointer:
description: "Filename for the committed pointer file that records the commit SHA whose uploaded binary plan should be applied and that plan's sha256 digest"
required: false
default: "tfplan.commit"
plan_bucket:
description: "GCS bucket, optionally with a path prefix (e.g. 'my-bucket/terraform-plans'), where the binary plan is uploaded. Required when enable_pr_creation is 'true'. Must match the apply-terraform-plan action's plan_bucket input. The service account needs permission to create objects in this bucket. A binary plan contains the same sensitive data as the Terraform state, so restrict this bucket exactly like the state bucket: CI service accounts only, no human-facing grants."
required: false
default: ""
workload_identity_provider:
description: "Workload Identity Provider for Google Cloud authentication"
required: true
Expand Down Expand Up @@ -132,6 +143,52 @@ runs:
echo "has_changes=true" >> "$GITHUB_OUTPUT"
fi

# A binary plan file embeds a full copy of the Terraform state, including
# sensitive values in cleartext, regardless of what the plan changes.
# Committing it would put that state into git history permanently, so we
# upload it to an access-controlled GCS bucket instead and commit only a
# small pointer file recording the commit SHA the plan was generated from.
# The apply-terraform-plan action rebuilds the object path from its own
# plan_bucket input plus that SHA and downloads the exact plan that was
# reviewed. Plans that are never applied (e.g. superseded plan PRs) leave
# objects behind; configure a lifecycle rule on the bucket to expire them.
#
# The gcloud CLI is preinstalled on GitHub-hosted runners and picks up the
# credentials exported by the auth step above.
- name: Upload binary plan to GCS
if: inputs.enable_pr_creation == 'true' && github.ref_name == inputs.pr_creation_branch && steps.plan.outputs.has_changes == 'true'
id: upload
shell: bash
working-directory: ${{ inputs.terraform_path }}
run: |
if [ -z "${{ inputs.plan_bucket }}" ]; then
echo "The plan_bucket input is required when enable_pr_creation is 'true': the binary plan is uploaded to GCS instead of being committed." >&2
exit 1
fi
# Slug the terraform path the same way apply-terraform-plan does, so
# concurrent stacks in one repo can't collide on an object path;
# utils/terraform-plan-object-key.test.ts pins the two copies to each
# other. The short hash suffix keeps two terraform paths that
# normalize to the same slug from colliding.
slug=$(printf '%s' "${{ inputs.terraform_path }}" | tr -c 'A-Za-z0-9._-' '-' | sed -E 's/-+/-/g; s/^-//; s/-$//')
Comment thread
khan-actions-bot marked this conversation as resolved.
slug="${slug}-$(printf '%s' "${{ inputs.terraform_path }}" | sha256sum | cut -c1-8)"
# The object's filename segment is a fixed literal on both sides so
# the key can't depend on the two actions' independently configurable
# local filename inputs.
object="gs://${{ inputs.plan_bucket }}/${slug}/${{ github.sha }}/tfplan.binary"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (non-blocking): The object-key test pins the slug pipeline and the /tfplan.binary suffix, but not the full path skeleton. It only asserts each object= line ends in /tfplan.binary, so a reorder in one action (e.g. (redacted) here vs ${slug}/${sha}/...in apply) would keep all tests green yet make every apply reconstruct a different key and 404 — exactly the drift the suite exists to prevent. Consider asserting the twoobject= lines are structurally identical after normalizing the one intended difference (${{ github.sha }}vs${plan_sha}`).

plan_hash=$(sha256sum "${{ inputs.plan_file_binary }}" | cut -d' ' -f1)
gcloud storage cp "${{ inputs.plan_file_binary }}" "$object"
rm -f "${{ inputs.plan_file_binary }}"
# The pointer records the commit SHA the plan was generated from and
# the plan's sha256, which apply-terraform-plan verifies after
# download so the applied bytes are exactly the reviewed plan. It
# deliberately holds no gs:// URI: apply reconstructs the object path
# from its own workflow-configured inputs, so a tampered pointer
# cannot point apply at an arbitrary object.
printf '%s\n%s\n' "${{ github.sha }}" "$plan_hash" > "${{ inputs.plan_file_pointer }}"
echo "object=$object" >> "$GITHUB_OUTPUT"
echo "Uploaded binary plan to $object (sha256 $plan_hash)"

- name: Get Last Deployed Commit
if: steps.plan.outputs.has_changes == 'true' && github.ref_name == inputs.pr_creation_branch && inputs.enable_pr_creation == 'true'
id: last_deployed
Expand Down Expand Up @@ -198,8 +255,10 @@ runs:

Note, this plan reflects the current state of the `${{ inputs.pr_creation_branch }}` branch and may include changes from multiple contributors.

Plan file: `${{ inputs.terraform_path }}/${{ inputs.plan_file_binary }}`
Plaintext plan file: `${{ inputs.terraform_path }}/${{ inputs.plan_file_text }}`
Binary plan: `${{ steps.upload.outputs.object }}`

The binary plan is stored in GCS rather than committed because it embeds a full copy of the Terraform state, including sensitive values. The committed pointer file `${{ inputs.terraform_path }}/${{ inputs.plan_file_pointer }}` records which uploaded plan the apply workflow will download.

## Changes Since Last Deployment
You can view all changes that have not yet been deployed [here](https://github.com/${{ github.repository }}/compare/${{ steps.last_deployed.outputs.last_deployed }}...${{ github.sha }}).
Expand All @@ -217,7 +276,7 @@ runs:

${{ inputs.pr_extra_text }}
add-paths: |
${{ inputs.terraform_path }}/${{ inputs.plan_file_binary }}
${{ inputs.terraform_path }}/${{ inputs.plan_file_pointer }}
${{ inputs.terraform_path }}/${{ inputs.plan_file_text }}
base: ${{ inputs.pr_creation_branch }}
reviewers: ${{ steps.reviewer.outputs.reviewer }}
Expand Down
Loading
Loading