From fb9a915f5177cb3d90510df7d52d140d6c443654 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Mon, 19 Oct 2026 13:22:49 +0100 Subject: [PATCH 1/2] fix(ci): verify the rollback image in AWS_ACCOUNT_ID's registry The AWS rollback jobs deploy IMAGE_URI, built from the AWS_ACCOUNT_ID repository variable, but the "Verify image exists" step called `aws ecr describe-images` with no --registry-id, so it checked the registry of the account behind AWS_ROLE_TO_ASSUME instead. When those two admin-set variables pointed at different accounts, the step reported the image present in a registry Terraform does not pull from. Both rollback-aws-lambda and rollback-aws-fargate now: - refuse to run when AWS_ACCOUNT_ID is unset, when the caller identity cannot be read, or when AWS_ACCOUNT_ID differs from the assumed role's account (which also guards the Terraform apply, not just the lookup); - pin describe-images to --registry-id "$AWS_ACCOUNT_ID". scripts/test-rollback-ecr-registry.sh lifts the step out of rollback.yml and runs it against a stubbed aws CLI; ci.yml runs it as the rollback-ecr-registry job, required by ci-success. Closes #142 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/README.md | 5 +- .github/workflows/ci.yml | 22 ++ .github/workflows/rollback.yml | 54 ++++- scripts/test-rollback-ecr-registry.sh | 278 ++++++++++++++++++++++++++ 4 files changed, 356 insertions(+), 3 deletions(-) create mode 100755 scripts/test-rollback-ecr-registry.sh diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 022eb0813..9e02fc3aa 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -414,7 +414,10 @@ gh workflow run rollback.yml \ ### Safety Features -- **Image Verification** - Confirms image exists before deploying +- **Image Verification** - Confirms image exists before deploying. For AWS, + the lookup is pinned to the `AWS_ACCOUNT_ID` registry the image URI names, + and the rollback refuses to run if `AWS_ACCOUNT_ID` is not the account of + `AWS_ROLE_TO_ASSUME` - **Audit Trail** - Records all rollbacks (365 day retention) - **Reason Tracking** - Requires reason for accountability - **Manual Only** - Cannot be triggered automatically diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index b885e287e..f852e5f16 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1157,6 +1157,27 @@ jobs: - name: Run state namespace self-tests run: bash scripts/test-aws-tfstate-platform-key.sh + # Run the AWS rollback jobs' "Verify image exists" step, lifted out of + # rollback.yml, against a stubbed aws CLI (#142). IMAGE_URI names the + # AWS_ACCOUNT_ID registry while the step runs as AWS_ROLE_TO_ASSUME, so the + # step must refuse when the two accounts differ and must look the tag up in + # the registry IMAGE_URI names. A rollback run cannot show this without real + # credentials, so it is asserted here. Fast (shell only), so it always runs. + rollback-ecr-registry: + name: Rollback ECR registry check + runs-on: ubuntu-latest + permissions: + contents: read + + steps: + - name: Checkout code + uses: actions/checkout@93cb6efe18208431cddfb8368fd83d5badbf9bfd # v5.0.1 + with: + persist-credentials: false + + - name: Run rollback verify-step self-tests + run: bash scripts/test-rollback-ecr-registry.sh + # Assert that no Terraform file declares an azurerm_key_vault_access_policy # resource. This project's only Key Vault sets enable_rbac_authorization = # true, and an RBAC-enabled vault ignores access policies entirely, so such a @@ -1205,6 +1226,7 @@ jobs: - rds-deletion-protection-scope - cloud-sql-delete-scope - aws-tfstate-platform-key + - rollback-ecr-registry - azure-kv-access-policy if: always() permissions: diff --git a/.github/workflows/rollback.yml b/.github/workflows/rollback.yml index e754e7e8a..41f81b9ce 100644 --- a/.github/workflows/rollback.yml +++ b/.github/workflows/rollback.yml @@ -237,9 +237,13 @@ jobs: aws-region: ${{ vars.AWS_REGION || 'us-east-1' }} - name: Verify image exists + # AWS_ACCOUNT_ID, ECR_REPOSITORY and AWS_REGION are the same expressions + # the validate job builds IMAGE_URI from, so this step checks the + # registry Terraform is about to pull from (#142). env: IMAGE_TAG: ${{ inputs.image_tag }} ECR_REPOSITORY: ${{ vars.ECR_REPOSITORY }} + AWS_ACCOUNT_ID: ${{ vars.AWS_ACCOUNT_ID }} AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }} run: | set -euo pipefail @@ -248,10 +252,31 @@ jobs: echo "::error::ECR_REPOSITORY repository variable is not set" exit 1 fi + if [ -z "${AWS_ACCOUNT_ID:-}" ]; then + echo "::error::AWS_ACCOUNT_ID repository variable is not set" + exit 1 + fi + + # IMAGE_URI names the AWS_ACCOUNT_ID registry, but Terraform applies + # with the assumed role, and the deploy workflows push to the ECR + # repository in that role's account. The two are separate admin-set + # variables, so refuse to roll back when they disagree rather than + # deploy an image from a registry this job never checked. + if ! CALLER_ACCOUNT="$(aws sts get-caller-identity --query Account --output text)"; then + echo "::error::Could not determine the assumed role's AWS account" + exit 1 + fi + if [ "$CALLER_ACCOUNT" != "$AWS_ACCOUNT_ID" ]; then + echo "::error::AWS_ACCOUNT_ID ($AWS_ACCOUNT_ID) does not match the assumed role's account ($CALLER_ACCOUNT); refusing to roll back" + exit 1 + fi - echo "Checking if image exists: $ECR_REPOSITORY:$IMAGE_TAG" + echo "Checking if image exists: $AWS_ACCOUNT_ID/$ECR_REPOSITORY:$IMAGE_TAG" + # --registry-id pins the lookup to the registry IMAGE_URI names + # instead of the CLI's default (the caller's own account). if aws ecr describe-images \ + --registry-id "$AWS_ACCOUNT_ID" \ --repository-name "$ECR_REPOSITORY" \ --image-ids "imageTag=$IMAGE_TAG" \ --region "$AWS_REGION" > /dev/null 2>&1; then @@ -344,9 +369,13 @@ jobs: aws-region: ${{ vars.AWS_REGION || 'us-east-1' }} - name: Verify image exists + # AWS_ACCOUNT_ID, ECR_REPOSITORY and AWS_REGION are the same expressions + # the validate job builds IMAGE_URI from, so this step checks the + # registry Terraform is about to pull from (#142). env: IMAGE_TAG: ${{ inputs.image_tag }} ECR_REPOSITORY: ${{ vars.ECR_REPOSITORY }} + AWS_ACCOUNT_ID: ${{ vars.AWS_ACCOUNT_ID }} AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }} run: | set -euo pipefail @@ -355,10 +384,31 @@ jobs: echo "::error::ECR_REPOSITORY repository variable is not set" exit 1 fi + if [ -z "${AWS_ACCOUNT_ID:-}" ]; then + echo "::error::AWS_ACCOUNT_ID repository variable is not set" + exit 1 + fi + + # IMAGE_URI names the AWS_ACCOUNT_ID registry, but Terraform applies + # with the assumed role, and the deploy workflows push to the ECR + # repository in that role's account. The two are separate admin-set + # variables, so refuse to roll back when they disagree rather than + # deploy an image from a registry this job never checked. + if ! CALLER_ACCOUNT="$(aws sts get-caller-identity --query Account --output text)"; then + echo "::error::Could not determine the assumed role's AWS account" + exit 1 + fi + if [ "$CALLER_ACCOUNT" != "$AWS_ACCOUNT_ID" ]; then + echo "::error::AWS_ACCOUNT_ID ($AWS_ACCOUNT_ID) does not match the assumed role's account ($CALLER_ACCOUNT); refusing to roll back" + exit 1 + fi - echo "Checking if image exists: $ECR_REPOSITORY:$IMAGE_TAG" + echo "Checking if image exists: $AWS_ACCOUNT_ID/$ECR_REPOSITORY:$IMAGE_TAG" + # --registry-id pins the lookup to the registry IMAGE_URI names + # instead of the CLI's default (the caller's own account). if aws ecr describe-images \ + --registry-id "$AWS_ACCOUNT_ID" \ --repository-name "$ECR_REPOSITORY" \ --image-ids "imageTag=$IMAGE_TAG" \ --region "$AWS_REGION" > /dev/null 2>&1; then diff --git a/scripts/test-rollback-ecr-registry.sh b/scripts/test-rollback-ecr-registry.sh new file mode 100755 index 000000000..56a7c2895 --- /dev/null +++ b/scripts/test-rollback-ecr-registry.sh @@ -0,0 +1,278 @@ +#!/usr/bin/env bash +# test-rollback-ecr-registry.sh +# +# Offline test for the "Verify image exists" step of the two AWS rollback jobs +# in .github/workflows/rollback.yml (#142). The step's `run:` block is lifted +# out of the workflow and executed against a stubbed `aws` on PATH, so what is +# tested is the text that ships, not a copy of it. +# +# The validate job builds IMAGE_URI from the AWS_ACCOUNT_ID repository +# variable, and Terraform then deploys that URI with the role in +# AWS_ROLE_TO_ASSUME. Before #142 the verify step asked ECR with no +# --registry-id, so it checked the assumed role's registry, which equals +# AWS_ACCOUNT_ID's only by convention. When the two diverged, the step +# reported "Image exists in ECR" for a registry Terraform does not pull from. +# +# The stub models the real CLI's defaults: `describe-images` without +# --registry-id resolves against the caller's account, and +# `sts get-caller-identity` returns the caller's account. PATH is limited to +# the stub directory plus /usr/bin:/bin, so no real AWS CLI is reachable. +# +# Account IDs are assembled at runtime from adjacent literals so this file +# never holds a contiguous 12-digit token for git-secrets to flag. +# +# Set ROLLBACK_WORKFLOW to run the same cases against another copy of the +# workflow (e.g. the pre-fix one, to see the cases fail). +# +# Exits 0 when all cases pass; exits 1 on any failure. + +set -euo pipefail + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +REPO_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" +WORKFLOW="${ROLLBACK_WORKFLOW:-${REPO_ROOT}/.github/workflows/rollback.yml}" + +WORK="$(mktemp -d)" +trap 'rm -rf "$WORK"' EXIT +mkdir -p "$WORK/bin" "$WORK/home" + +pass=0 +fail=0 + +note_pass() { + echo "PASS: $1" + ((pass++)) || true +} + +note_fail() { + echo "FAIL: $1" + ((fail++)) || true +} + +ACCT_URI="111111""111111" +ACCT_ROLE="222222""222222" +REGION="us-east-1" +REPO="cudly" +TAG="sha-abc1234" +JOBS="rollback-aws-lambda rollback-aws-fargate" + +# --- Step extraction --------------------------------------------------------- +# +# extract_block MODE JOB STEP +# +# Prints the `run:` block (MODE=run) or the `env:` mapping (MODE=env) of the +# step named STEP inside the top-level job JOB, de-indented. Jobs are delimited +# by two-space-indent headers, steps by ` - ` items, and a block ends at +# the first non-blank line indented less than its content. +extract_block() { + awk -v mode="$1" -v job="$2" -v step="$3" ' + /^ [A-Za-z0-9_.-]+:[[:space:]]*$/ { injob = ($0 == " " job ":"); instep = 0; blk = ""; next } + !injob { next } + /^ - / { instep = ($0 == " - name: " step); blk = ""; next } + !instep { next } + /^ [A-Za-z_-]+:/ { + blk = "" + if ($0 ~ /^ run: [|][[:space:]]*$/) blk = "run" + else if ($0 ~ /^ env:[[:space:]]*$/) blk = "env" + next + } + blk == mode { + if ($0 ~ /^[[:space:]]*$/) { print ""; next } + if (substr($0, 1, 10) == " ") { print substr($0, 11); next } + blk = "" + } + ' "$WORKFLOW" +} + +# --- The aws stub ------------------------------------------------------------ +cat >"$WORK/bin/aws" <<'STUB' +#!/usr/bin/env bash +printf '%s\n' "$*" >>"$STUB_LOG" +case "${1:-} ${2:-}" in + "sts get-caller-identity") + if [ "${STUB_STS_FAIL:-0}" = 1 ]; then + echo "Unable to locate credentials. You can configure credentials by running \"aws configure\"." >&2 + exit 253 + fi + # Model the output shape: without `--query Account --output text` the + # real CLI prints JSON, which must never compare equal to an account ID. + if [[ " $* " == *" --query Account "* && " $* " == *" --output text "* ]]; then + printf '%s\n' "$STUB_CALLER_ACCOUNT" + else + printf '{"UserId":"x","Account":"%s","Arn":"arn"}\n' "$STUB_CALLER_ACCOUNT" + fi + exit 0 + ;; + "ecr describe-images") + shift 2 + # The real CLI defaults the registry to the caller's own account. + registry="$STUB_CALLER_ACCOUNT" + repo="" + tag="" + region="" + while [ $# -gt 0 ]; do + case "$1" in + --registry-id) registry="$2"; shift 2 ;; + --repository-name) repo="$2"; shift 2 ;; + --image-ids) tag="${2#imageTag=}"; shift 2 ;; + --region) region="$2"; shift 2 ;; + *) echo "stub aws: unexpected describe-images argument: $1" >&2; exit 2 ;; + esac + done + for img in ${STUB_IMAGES:-}; do + if [ "$img" = "${registry}/${region}/${repo}:${tag}" ]; then + echo '{"imageDetails":[{"imageTags":["'"$tag"'"]}]}' + exit 0 + fi + done + echo "An error occurred (ImageNotFoundException) when calling the DescribeImages operation" >&2 + exit 254 + ;; +esac +echo "stub aws: unexpected command: $*" >&2 +exit 2 +STUB +chmod +x "$WORK/bin/aws" + +# run_step JOB AWS_ACCOUNT_ID CALLER_ACCOUNT IMAGES STS_FAIL ECR_REPOSITORY +# +# Runs JOB's verify step the way the runner does (bash -e -o pipefail with the +# step's env), with only the stub reachable. Sets STEP_RC, STEP_OUT, STEP_LOG. +run_step() { + local job="$1" + local script="$WORK/step-${job}.sh" + STEP_LOG="$WORK/aws-${job}.log" + : >"$STEP_LOG" + extract_block run "$job" "Verify image exists" >"$script" + set +e + STEP_OUT="$(env -i \ + PATH="$WORK/bin:/usr/bin:/bin" \ + HOME="$WORK/home" \ + AWS_CONFIG_FILE=/dev/null \ + AWS_SHARED_CREDENTIALS_FILE=/dev/null \ + STUB_LOG="$STEP_LOG" \ + STUB_CALLER_ACCOUNT="$3" \ + STUB_IMAGES="$4" \ + STUB_STS_FAIL="$5" \ + IMAGE_TAG="$TAG" \ + ECR_REPOSITORY="$6" \ + AWS_ACCOUNT_ID="$2" \ + AWS_REGION="$REGION" \ + bash --noprofile --norc -eo pipefail "$script" 2>&1)" + STEP_RC=$? + set -e +} + +# expect LABEL WANT(pass|fail) TEXT +expect() { + local label="$1" want="$2" text="$3" + local ok=1 + if [[ "$want" == pass && "$STEP_RC" -ne 0 ]] || [[ "$want" == fail && "$STEP_RC" -eq 0 ]]; then + ok=0 + fi + if [[ -n "$text" && "$STEP_OUT" != *"$text"* ]]; then + ok=0 + fi + if [[ "$ok" -eq 1 ]]; then + note_pass "$label" + else + note_fail "$label" + local wanted="the step to ${want}" + if [[ -n "$text" ]]; then + wanted="${wanted} with output containing '${text}'" + fi + echo " wanted ${wanted}; exit ${STEP_RC}, output:" + printf '%s\n' "$STEP_OUT" | sed 's/^/ /' + fi +} + +# expect_log LABEL (has|lacks) TEXT +expect_log() { + local label="$1" want="$2" text="$3" + if { [[ "$want" == has ]] && grep -Fq -- "$text" "$STEP_LOG"; } || + { [[ "$want" == lacks ]] && ! grep -Fq -- "$text" "$STEP_LOG"; }; then + note_pass "$label" + else + note_fail "$label" + echo " wanted the aws call log to ${want} '${text}', log:" + sed 's/^/ /' "$STEP_LOG" + fi +} + +# --- The steps exist and read the same variables as IMAGE_URI ---------------- +# +# Asserted first: an extractor that finds nothing would make every behavioral +# case below fail for the wrong reason, or a future rename pass unnoticed. +validate_env="$(extract_block env validate "Validate inputs")" +if [[ "$validate_env" == *"AWS_ACCOUNT_ID:"* ]]; then + note_pass "validate job's Validate inputs step is found" +else + note_fail "validate job's Validate inputs step env not found in ${WORKFLOW}" +fi + +for job in $JOBS; do + run_block="$(extract_block run "$job" "Verify image exists")" + if [[ "$run_block" == *"aws ecr describe-images"* ]]; then + note_pass "${job}: Verify image exists step is found" + else + note_fail "${job}: no Verify image exists step calling aws ecr describe-images in ${WORKFLOW}" + continue + fi + + # The verified registry must come from the same expressions IMAGE_URI is + # built from, or the step can check one registry while deploying another. + verify_env="$(extract_block env "$job" "Verify image exists")" + for key in AWS_ACCOUNT_ID ECR_REPOSITORY AWS_REGION IMAGE_TAG; do + want_line="$(printf '%s\n' "$validate_env" | grep -E "^${key}:" || true)" + got_line="$(printf '%s\n' "$verify_env" | grep -E "^${key}:" || true)" + if [[ -n "$want_line" && "$got_line" == "$want_line" ]]; then + note_pass "${job}: ${key} comes from the same expression as IMAGE_URI" + else + note_fail "${job}: ${key} differs from the IMAGE_URI source" + echo " validate: ${want_line:-}" + echo " verify: ${got_line:-}" + fi + done +done + +# --- Behavior ---------------------------------------------------------------- +for job in $JOBS; do + # Positive case first: a step that always fails would pass every refusal. + run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 0 "$REPO" + expect "${job}: matching accounts with the image present pass" pass "Image exists in ECR" + expect_log "${job}: describe-images is pinned to the IMAGE_URI registry" has \ + "ecr describe-images --registry-id ${ACCT_URI} --repository-name ${REPO}" + + # #142: the image exists only in the assumed role's registry. The pre-fix + # step checked that registry and passed. + run_step "$job" "$ACCT_URI" "$ACCT_ROLE" "${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 "$REPO" + expect "${job}: AWS_ACCOUNT_ID diverging from the assumed role is refused" fail \ + "does not match the assumed role's account" + + # Image present in both registries: the verify alone would now be + # self-consistent, but Terraform still applies with the other account's + # role, so the divergence itself must fail the step. + run_step "$job" "$ACCT_URI" "$ACCT_ROLE" \ + "${ACCT_URI}/${REGION}/${REPO}:${TAG} ${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 "$REPO" + expect "${job}: divergence is refused even when both registries hold the tag" fail \ + "does not match the assumed role's account" + expect_log "${job}: no image lookup runs once the accounts disagree" lacks "ecr describe-images" + + run_step "$job" "" "$ACCT_ROLE" "${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 "$REPO" + expect "${job}: an unset AWS_ACCOUNT_ID is refused" fail "AWS_ACCOUNT_ID repository variable is not set" + + run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 1 "$REPO" + expect "${job}: an unreadable caller identity is refused" fail \ + "Could not determine the assumed role's AWS account" + expect_log "${job}: no image lookup runs without a caller identity" lacks "ecr describe-images" + + run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:sha-other" 0 "$REPO" + expect "${job}: a missing tag in the IMAGE_URI registry is refused" fail "Image not found in ECR" + + run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 0 "" + expect "${job}: an unset ECR_REPOSITORY is refused" fail "ECR_REPOSITORY repository variable is not set" +done + +echo +echo "passed: ${pass}, failed: ${fail}" +[[ "$fail" -eq 0 ]] From db422096ddc062024cd4d14f01a9b95a5cb5b295 Mon Sep 17 00:00:00 2001 From: Astrid Gealer Date: Wed, 21 Oct 2026 09:20:31 +0100 Subject: [PATCH 2/2] fix(ci): verify the rollback image against the IMAGE_URI Terraform deploys The AWS rollback jobs bind environment:, so vars.AWS_ACCOUNT_ID, AWS_REGION and ECR_REPOSITORY resolve environment-first there but repo/org-level in the validate job that builds IMAGE_URI. An environment-scoped AWS_ACCOUNT_ID could therefore make the verify step check one registry while Terraform deploys from another (#142 again). Both verify steps now read IMAGE_URI from needs.validate.outputs.image_uri, parse account, region, repository and tag out of it (failing closed on anything that is not an ECR image URI), compare that account with the STS caller account, and run describe-images against the parsed values. This also drops the "repository variable is not set" messages. scripts/test-rollback-ecr-registry.sh runs every case with environment-scoped decoys under the old variable names, adds malformed URI rows (empty, no tag, 11-digit account, non-ECR host, GCP URI, trailing text, multi-line) that must fail before any AWS call, and replaces the env-line source-equivalence check with an assertion that the verify and Terraform steps read the same job output. Refs #142 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/README.md | 6 +- .github/workflows/ci.yml | 7 +- .github/workflows/rollback.yml | 114 +++++++++++----------- scripts/test-rollback-ecr-registry.sh | 135 ++++++++++++++++---------- 4 files changed, 151 insertions(+), 111 deletions(-) diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 9e02fc3aa..a4f516e96 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -415,9 +415,9 @@ gh workflow run rollback.yml \ ### Safety Features - **Image Verification** - Confirms image exists before deploying. For AWS, - the lookup is pinned to the `AWS_ACCOUNT_ID` registry the image URI names, - and the rollback refuses to run if `AWS_ACCOUNT_ID` is not the account of - `AWS_ROLE_TO_ASSUME` + the check parses the account, region and repository out of the exact image + URI Terraform deploys, refuses to run if that account is not the account of + `AWS_ROLE_TO_ASSUME`, and pins the lookup to that registry - **Audit Trail** - Records all rollbacks (365 day retention) - **Reason Tracking** - Requires reason for accountability - **Manual Only** - Cannot be triggered automatically diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8b7882247..43abee519 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1185,9 +1185,10 @@ jobs: # Run the AWS rollback jobs' "Verify image exists" step, lifted out of # rollback.yml, against a stubbed aws CLI (#142). IMAGE_URI names the # AWS_ACCOUNT_ID registry while the step runs as AWS_ROLE_TO_ASSUME, so the - # step must refuse when the two accounts differ and must look the tag up in - # the registry IMAGE_URI names. A rollback run cannot show this without real - # credentials, so it is asserted here. Fast (shell only), so it always runs. + # step must parse the registry out of the IMAGE_URI Terraform deploys, refuse + # when its account differs from the role's, and look the tag up there. A + # rollback run cannot show this without real credentials, so it is asserted + # here. Fast (shell only), so it always runs. rollback-ecr-registry: name: Rollback ECR registry check runs-on: ubuntu-latest diff --git a/.github/workflows/rollback.yml b/.github/workflows/rollback.yml index 41f81b9ce..c86e1a61c 100644 --- a/.github/workflows/rollback.yml +++ b/.github/workflows/rollback.yml @@ -237,49 +237,52 @@ jobs: aws-region: ${{ vars.AWS_REGION || 'us-east-1' }} - name: Verify image exists - # AWS_ACCOUNT_ID, ECR_REPOSITORY and AWS_REGION are the same expressions - # the validate job builds IMAGE_URI from, so this step checks the - # registry Terraform is about to pull from (#142). + # Checks the exact IMAGE_URI the Terraform step below deploys (#142). + # The account, region and repository are parsed out of that string + # rather than re-read from vars: this job binds `environment:`, so + # vars.* resolve environment-first here but repo/org-level in the + # validate job that built IMAGE_URI, and the two can differ. env: - IMAGE_TAG: ${{ inputs.image_tag }} - ECR_REPOSITORY: ${{ vars.ECR_REPOSITORY }} - AWS_ACCOUNT_ID: ${{ vars.AWS_ACCOUNT_ID }} - AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }} + IMAGE_URI: ${{ needs.validate.outputs.image_uri }} run: | set -euo pipefail - if [ -z "${ECR_REPOSITORY:-}" ]; then - echo "::error::ECR_REPOSITORY repository variable is not set" + # .dkr.ecr..amazonaws.com/:, as the + # validate job builds it. Anything else fails closed. + IMAGE_URI="${IMAGE_URI:-}" + ECR_URI_RE='^([0-9]{12})\.dkr\.ecr\.([a-z0-9-]+)\.amazonaws\.com/([a-z0-9][a-z0-9._/-]*):([A-Za-z0-9._-]+)$' + if [[ ! "$IMAGE_URI" =~ $ECR_URI_RE ]]; then + echo "::error::Image URI from the validate job is not an ECR image URI (.dkr.ecr..amazonaws.com/:)" exit 1 fi - if [ -z "${AWS_ACCOUNT_ID:-}" ]; then - echo "::error::AWS_ACCOUNT_ID repository variable is not set" - exit 1 - fi - - # IMAGE_URI names the AWS_ACCOUNT_ID registry, but Terraform applies - # with the assumed role, and the deploy workflows push to the ECR - # repository in that role's account. The two are separate admin-set - # variables, so refuse to roll back when they disagree rather than - # deploy an image from a registry this job never checked. + REGISTRY_ACCOUNT="${BASH_REMATCH[1]}" + REGISTRY_REGION="${BASH_REMATCH[2]}" + REPOSITORY="${BASH_REMATCH[3]}" + IMAGE_TAG="${BASH_REMATCH[4]}" + + # Terraform applies with the assumed role, and the deploy workflows + # push to the ECR repository in that role's account. Refuse to roll + # back when the registry IMAGE_URI names is in a different account, + # rather than deploy an image from a registry this job never checked. if ! CALLER_ACCOUNT="$(aws sts get-caller-identity --query Account --output text)"; then echo "::error::Could not determine the assumed role's AWS account" exit 1 fi - if [ "$CALLER_ACCOUNT" != "$AWS_ACCOUNT_ID" ]; then - echo "::error::AWS_ACCOUNT_ID ($AWS_ACCOUNT_ID) does not match the assumed role's account ($CALLER_ACCOUNT); refusing to roll back" + if [ "$CALLER_ACCOUNT" != "$REGISTRY_ACCOUNT" ]; then + echo "::error::Image URI registry account ($REGISTRY_ACCOUNT) does not match the assumed role's account ($CALLER_ACCOUNT); refusing to roll back" exit 1 fi - echo "Checking if image exists: $AWS_ACCOUNT_ID/$ECR_REPOSITORY:$IMAGE_TAG" + echo "Checking if image exists: $IMAGE_URI" - # --registry-id pins the lookup to the registry IMAGE_URI names - # instead of the CLI's default (the caller's own account). + # --registry-id and --region pin the lookup to the registry IMAGE_URI + # names instead of the CLI's defaults (the caller's own account and + # the configured region). if aws ecr describe-images \ - --registry-id "$AWS_ACCOUNT_ID" \ - --repository-name "$ECR_REPOSITORY" \ + --registry-id "$REGISTRY_ACCOUNT" \ + --repository-name "$REPOSITORY" \ --image-ids "imageTag=$IMAGE_TAG" \ - --region "$AWS_REGION" > /dev/null 2>&1; then + --region "$REGISTRY_REGION" > /dev/null 2>&1; then echo "✅ Image exists in ECR" else echo "::error::Image not found in ECR" @@ -369,49 +372,52 @@ jobs: aws-region: ${{ vars.AWS_REGION || 'us-east-1' }} - name: Verify image exists - # AWS_ACCOUNT_ID, ECR_REPOSITORY and AWS_REGION are the same expressions - # the validate job builds IMAGE_URI from, so this step checks the - # registry Terraform is about to pull from (#142). + # Checks the exact IMAGE_URI the Terraform step below deploys (#142). + # The account, region and repository are parsed out of that string + # rather than re-read from vars: this job binds `environment:`, so + # vars.* resolve environment-first here but repo/org-level in the + # validate job that built IMAGE_URI, and the two can differ. env: - IMAGE_TAG: ${{ inputs.image_tag }} - ECR_REPOSITORY: ${{ vars.ECR_REPOSITORY }} - AWS_ACCOUNT_ID: ${{ vars.AWS_ACCOUNT_ID }} - AWS_REGION: ${{ vars.AWS_REGION || 'us-east-1' }} + IMAGE_URI: ${{ needs.validate.outputs.image_uri }} run: | set -euo pipefail - if [ -z "${ECR_REPOSITORY:-}" ]; then - echo "::error::ECR_REPOSITORY repository variable is not set" + # .dkr.ecr..amazonaws.com/:, as the + # validate job builds it. Anything else fails closed. + IMAGE_URI="${IMAGE_URI:-}" + ECR_URI_RE='^([0-9]{12})\.dkr\.ecr\.([a-z0-9-]+)\.amazonaws\.com/([a-z0-9][a-z0-9._/-]*):([A-Za-z0-9._-]+)$' + if [[ ! "$IMAGE_URI" =~ $ECR_URI_RE ]]; then + echo "::error::Image URI from the validate job is not an ECR image URI (.dkr.ecr..amazonaws.com/:)" exit 1 fi - if [ -z "${AWS_ACCOUNT_ID:-}" ]; then - echo "::error::AWS_ACCOUNT_ID repository variable is not set" - exit 1 - fi - - # IMAGE_URI names the AWS_ACCOUNT_ID registry, but Terraform applies - # with the assumed role, and the deploy workflows push to the ECR - # repository in that role's account. The two are separate admin-set - # variables, so refuse to roll back when they disagree rather than - # deploy an image from a registry this job never checked. + REGISTRY_ACCOUNT="${BASH_REMATCH[1]}" + REGISTRY_REGION="${BASH_REMATCH[2]}" + REPOSITORY="${BASH_REMATCH[3]}" + IMAGE_TAG="${BASH_REMATCH[4]}" + + # Terraform applies with the assumed role, and the deploy workflows + # push to the ECR repository in that role's account. Refuse to roll + # back when the registry IMAGE_URI names is in a different account, + # rather than deploy an image from a registry this job never checked. if ! CALLER_ACCOUNT="$(aws sts get-caller-identity --query Account --output text)"; then echo "::error::Could not determine the assumed role's AWS account" exit 1 fi - if [ "$CALLER_ACCOUNT" != "$AWS_ACCOUNT_ID" ]; then - echo "::error::AWS_ACCOUNT_ID ($AWS_ACCOUNT_ID) does not match the assumed role's account ($CALLER_ACCOUNT); refusing to roll back" + if [ "$CALLER_ACCOUNT" != "$REGISTRY_ACCOUNT" ]; then + echo "::error::Image URI registry account ($REGISTRY_ACCOUNT) does not match the assumed role's account ($CALLER_ACCOUNT); refusing to roll back" exit 1 fi - echo "Checking if image exists: $AWS_ACCOUNT_ID/$ECR_REPOSITORY:$IMAGE_TAG" + echo "Checking if image exists: $IMAGE_URI" - # --registry-id pins the lookup to the registry IMAGE_URI names - # instead of the CLI's default (the caller's own account). + # --registry-id and --region pin the lookup to the registry IMAGE_URI + # names instead of the CLI's defaults (the caller's own account and + # the configured region). if aws ecr describe-images \ - --registry-id "$AWS_ACCOUNT_ID" \ - --repository-name "$ECR_REPOSITORY" \ + --registry-id "$REGISTRY_ACCOUNT" \ + --repository-name "$REPOSITORY" \ --image-ids "imageTag=$IMAGE_TAG" \ - --region "$AWS_REGION" > /dev/null 2>&1; then + --region "$REGISTRY_REGION" > /dev/null 2>&1; then echo "✅ Image exists in ECR" else echo "::error::Image not found in ECR" diff --git a/scripts/test-rollback-ecr-registry.sh b/scripts/test-rollback-ecr-registry.sh index 56a7c2895..6a9f0b2f6 100755 --- a/scripts/test-rollback-ecr-registry.sh +++ b/scripts/test-rollback-ecr-registry.sh @@ -6,12 +6,20 @@ # out of the workflow and executed against a stubbed `aws` on PATH, so what is # tested is the text that ships, not a copy of it. # -# The validate job builds IMAGE_URI from the AWS_ACCOUNT_ID repository -# variable, and Terraform then deploys that URI with the role in -# AWS_ROLE_TO_ASSUME. Before #142 the verify step asked ECR with no -# --registry-id, so it checked the assumed role's registry, which equals -# AWS_ACCOUNT_ID's only by convention. When the two diverged, the step -# reported "Image exists in ECR" for a registry Terraform does not pull from. +# The validate job builds IMAGE_URI from the AWS_ACCOUNT_ID variable, and +# Terraform then deploys that URI with the role in AWS_ROLE_TO_ASSUME. Before +# #142 the verify step asked ECR with no --registry-id, so it checked the +# assumed role's registry, which equals AWS_ACCOUNT_ID's only by convention. +# When the two diverged, the step reported "Image exists in ECR" for a +# registry Terraform does not pull from. +# +# The rollback jobs bind `environment:` and the validate job does not, so +# vars.* can resolve to different values in the two. The verify step therefore +# parses account, region and repository out of the IMAGE_URI the validate job +# emitted (the string Terraform deploys) instead of re-reading vars. Every +# case runs with AWS_ACCOUNT_ID, ECR_REPOSITORY, AWS_REGION and IMAGE_TAG set +# to environment-scoped decoys that disagree with IMAGE_URI, so a step that +# reads them instead of IMAGE_URI fails. # # The stub models the real CLI's defaults: `describe-images` without # --registry-id resolves against the caller's account, and @@ -51,9 +59,20 @@ note_fail() { ACCT_URI="111111""111111" ACCT_ROLE="222222""222222" -REGION="us-east-1" +REGION="eu-central-1" REPO="cudly" TAG="sha-abc1234" +# Environment-scoped values a rollback job could see for the same variable +# names, all different from what IMAGE_URI names. +ENV_ACCOUNT="$ACCT_ROLE" +ENV_REPO="cudly-env" +ENV_REGION="us-east-1" +ENV_TAG="sha-env0000" + +# uri ACCOUNT REPOSITORY TAG prints the ECR image URI the validate job builds. +uri() { + printf '%s.dkr.ecr.%s.amazonaws.com/%s:%s' "$1" "$REGION" "$2" "$3" +} JOBS="rollback-aws-lambda rollback-aws-fargate" # --- Step extraction --------------------------------------------------------- @@ -134,10 +153,12 @@ exit 2 STUB chmod +x "$WORK/bin/aws" -# run_step JOB AWS_ACCOUNT_ID CALLER_ACCOUNT IMAGES STS_FAIL ECR_REPOSITORY +# run_step JOB IMAGE_URI CALLER_ACCOUNT IMAGES STS_FAIL # # Runs JOB's verify step the way the runner does (bash -e -o pipefail with the -# step's env), with only the stub reachable. Sets STEP_RC, STEP_OUT, STEP_LOG. +# step's env), with only the stub reachable. IMAGES entries are +# //:. The ENV_* decoys are exported under +# the variable names the pre-fix step read. Sets STEP_RC, STEP_OUT, STEP_LOG. run_step() { local job="$1" local script="$WORK/step-${job}.sh" @@ -154,10 +175,11 @@ run_step() { STUB_CALLER_ACCOUNT="$3" \ STUB_IMAGES="$4" \ STUB_STS_FAIL="$5" \ - IMAGE_TAG="$TAG" \ - ECR_REPOSITORY="$6" \ - AWS_ACCOUNT_ID="$2" \ - AWS_REGION="$REGION" \ + IMAGE_URI="$2" \ + IMAGE_TAG="$ENV_TAG" \ + ECR_REPOSITORY="$ENV_REPO" \ + AWS_ACCOUNT_ID="$ENV_ACCOUNT" \ + AWS_REGION="$ENV_REGION" \ bash --noprofile --norc -eo pipefail "$script" 2>&1)" STEP_RC=$? set -e @@ -186,11 +208,12 @@ expect() { fi } -# expect_log LABEL (has|lacks) TEXT +# expect_log LABEL (has|lacks|empty) [TEXT] expect_log() { - local label="$1" want="$2" text="$3" + local label="$1" want="$2" text="${3:-}" if { [[ "$want" == has ]] && grep -Fq -- "$text" "$STEP_LOG"; } || - { [[ "$want" == lacks ]] && ! grep -Fq -- "$text" "$STEP_LOG"; }; then + { [[ "$want" == lacks ]] && ! grep -Fq -- "$text" "$STEP_LOG"; } || + { [[ "$want" == empty ]] && [[ ! -s "$STEP_LOG" ]]; }; then note_pass "$label" else note_fail "$label" @@ -199,17 +222,12 @@ expect_log() { fi } -# --- The steps exist and read the same variables as IMAGE_URI ---------------- +# --- The steps exist and read the IMAGE_URI Terraform deploys --------------- # # Asserted first: an extractor that finds nothing would make every behavioral # case below fail for the wrong reason, or a future rename pass unnoticed. -validate_env="$(extract_block env validate "Validate inputs")" -if [[ "$validate_env" == *"AWS_ACCOUNT_ID:"* ]]; then - note_pass "validate job's Validate inputs step is found" -else - note_fail "validate job's Validate inputs step env not found in ${WORKFLOW}" -fi - +# shellcheck disable=SC2016 # a literal GitHub expression, not shell +IMAGE_URI_LINE='IMAGE_URI: ${{ needs.validate.outputs.image_uri }}' for job in $JOBS; do run_block="$(extract_block run "$job" "Verify image exists")" if [[ "$run_block" == *"aws ecr describe-images"* ]]; then @@ -219,58 +237,73 @@ for job in $JOBS; do continue fi - # The verified registry must come from the same expressions IMAGE_URI is - # built from, or the step can check one registry while deploying another. - verify_env="$(extract_block env "$job" "Verify image exists")" - for key in AWS_ACCOUNT_ID ECR_REPOSITORY AWS_REGION IMAGE_TAG; do - want_line="$(printf '%s\n' "$validate_env" | grep -E "^${key}:" || true)" - got_line="$(printf '%s\n' "$verify_env" | grep -E "^${key}:" || true)" - if [[ -n "$want_line" && "$got_line" == "$want_line" ]]; then - note_pass "${job}: ${key} comes from the same expression as IMAGE_URI" + # The verify step must read the same job output the Terraform step applies; + # the behavioral cases below then show it checks what that string names. + for step in "Verify image exists" "Rollback with Terraform"; do + if extract_block env "$job" "$step" | grep -Fqx -- "$IMAGE_URI_LINE"; then + note_pass "${job}: ${step} reads IMAGE_URI from the validate job output" else - note_fail "${job}: ${key} differs from the IMAGE_URI source" - echo " validate: ${want_line:-}" - echo " verify: ${got_line:-}" + note_fail "${job}: ${step} env lacks '${IMAGE_URI_LINE}'" fi done done # --- Behavior ---------------------------------------------------------------- +GOOD_URI="$(uri "$ACCT_URI" "$REPO" "$TAG")" for job in $JOBS; do # Positive case first: a step that always fails would pass every refusal. - run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 0 "$REPO" + run_step "$job" "$GOOD_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 0 expect "${job}: matching accounts with the image present pass" pass "Image exists in ECR" - expect_log "${job}: describe-images is pinned to the IMAGE_URI registry" has \ - "ecr describe-images --registry-id ${ACCT_URI} --repository-name ${REPO}" + expect_log "${job}: describe-images is pinned to the account, repository, tag and region IMAGE_URI names" has \ + "ecr describe-images --registry-id ${ACCT_URI} --repository-name ${REPO} --image-ids imageTag=${TAG} --region ${REGION}" + + # A namespaced repository keeps its slash. + run_step "$job" "$(uri "$ACCT_URI" "team/${REPO}" "$TAG")" "$ACCT_URI" \ + "${ACCT_URI}/${REGION}/team/${REPO}:${TAG}" 0 + expect "${job}: a namespaced repository is parsed whole" pass "Image exists in ECR" - # #142: the image exists only in the assumed role's registry. The pre-fix - # step checked that registry and passed. - run_step "$job" "$ACCT_URI" "$ACCT_ROLE" "${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 "$REPO" - expect "${job}: AWS_ACCOUNT_ID diverging from the assumed role is refused" fail \ + # #142, and the environment-scope variant: the assumed role (and the + # environment's AWS_ACCOUNT_ID decoy) is the other account, and the image + # exists only there. The pre-fix steps checked that registry and passed. + run_step "$job" "$GOOD_URI" "$ACCT_ROLE" "${ACCT_ROLE}/${ENV_REGION}/${ENV_REPO}:${ENV_TAG} ${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 + expect "${job}: an IMAGE_URI account diverging from the assumed role is refused" fail \ "does not match the assumed role's account" # Image present in both registries: the verify alone would now be # self-consistent, but Terraform still applies with the other account's # role, so the divergence itself must fail the step. - run_step "$job" "$ACCT_URI" "$ACCT_ROLE" \ - "${ACCT_URI}/${REGION}/${REPO}:${TAG} ${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 "$REPO" + run_step "$job" "$GOOD_URI" "$ACCT_ROLE" \ + "${ACCT_URI}/${REGION}/${REPO}:${TAG} ${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 expect "${job}: divergence is refused even when both registries hold the tag" fail \ "does not match the assumed role's account" expect_log "${job}: no image lookup runs once the accounts disagree" lacks "ecr describe-images" - run_step "$job" "" "$ACCT_ROLE" "${ACCT_ROLE}/${REGION}/${REPO}:${TAG}" 0 "$REPO" - expect "${job}: an unset AWS_ACCOUNT_ID is refused" fail "AWS_ACCOUNT_ID repository variable is not set" - - run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 1 "$REPO" + run_step "$job" "$GOOD_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 1 expect "${job}: an unreadable caller identity is refused" fail \ "Could not determine the assumed role's AWS account" expect_log "${job}: no image lookup runs without a caller identity" lacks "ecr describe-images" - run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:sha-other" 0 "$REPO" + run_step "$job" "$(uri "$ACCT_URI" "$REPO" sha-other)" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 0 expect "${job}: a missing tag in the IMAGE_URI registry is refused" fail "Image not found in ECR" - run_step "$job" "$ACCT_URI" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 0 "" - expect "${job}: an unset ECR_REPOSITORY is refused" fail "ECR_REPOSITORY repository variable is not set" + # Malformed or empty IMAGE_URI fails closed, before any AWS call. + while IFS='|' read -r label bad; do + run_step "$job" "$bad" "$ACCT_URI" "${ACCT_URI}/${REGION}/${REPO}:${TAG}" 0 + expect "${job}: ${label} is refused" fail "is not an ECR image URI" + expect_log "${job}: no AWS call runs for ${label}" empty + done <