diff --git a/.github/workflows/README.md b/.github/workflows/README.md index 2f1cbaf8d..5ebd9291b 100644 --- a/.github/workflows/README.md +++ b/.github/workflows/README.md @@ -415,7 +415,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 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 bcab59a78..df2e3f39a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1658,6 +1658,28 @@ 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 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 + 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 @@ -1729,6 +1751,7 @@ jobs: - rds-deletion-protection-scope - cloud-sql-delete-scope - aws-tfstate-platform-key + - rollback-ecr-registry - azure-kv-access-policy - migration-down-gate if: always() diff --git a/.github/workflows/rollback.yml b/.github/workflows/rollback.yml index e754e7e8a..c86e1a61c 100644 --- a/.github/workflows/rollback.yml +++ b/.github/workflows/rollback.yml @@ -237,24 +237,52 @@ jobs: aws-region: ${{ vars.AWS_REGION || 'us-east-1' }} - name: Verify image exists + # 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_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 + 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" != "$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: $ECR_REPOSITORY:$IMAGE_TAG" + echo "Checking if image exists: $IMAGE_URI" + # --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 \ - --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" @@ -344,24 +372,52 @@ jobs: aws-region: ${{ vars.AWS_REGION || 'us-east-1' }} - name: Verify image exists + # 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_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 + 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" != "$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: $ECR_REPOSITORY:$IMAGE_TAG" + echo "Checking if image exists: $IMAGE_URI" + # --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 \ - --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 new file mode 100755 index 000000000..6a9f0b2f6 --- /dev/null +++ b/scripts/test-rollback-ecr-registry.sh @@ -0,0 +1,311 @@ +#!/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 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 +# `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="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 --------------------------------------------------------- +# +# 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 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. 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" + 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_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 +} + +# 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|empty) [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"; } || + { [[ "$want" == empty ]] && [[ ! -s "$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 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. +# 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 + 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 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}: ${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" "$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 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, 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" "$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" "$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" "$(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" + + # 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 <