From 2324b658d7a577e7fe9e13de27a1c8e39ba809f3 Mon Sep 17 00:00:00 2001 From: rplusq Date: Fri, 10 Jul 2026 12:59:40 +0100 Subject: [PATCH 1/6] feat(cd): opt-in re-run-safe freshness guard on the apply steps MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an opt-in `enforce-fresh` compare-and-set guard as a STEP inside the side-effecting jobs (deploy-app's ECS deploy, deploy-infra's terraform apply), not as an upstream gate job — so it re-evaluates against live state on ANY re-run ("re-run failed jobs" / "re-run this job"), making a deploy idempotent and unable to roll the environment back from a superseded/stale run. The step reads the live version (app: /health banner via stage-url; infra: deployed_infra_commit SSM marker via a caller-supplied app-account role) and skips the apply (green no-op) unless that commit is an ancestor-or-equal of the target — fail-open on an unreadable version, fail-closed on a live commit unresolvable in git history, `allow-stale` to override. Default false → zero behaviour change for consumers that don't opt in. cd.yml threads the inputs through. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/cd.yml | 26 +++++++++++ .github/workflows/deploy-app.yml | 61 ++++++++++++++++++++++++ .github/workflows/deploy-infra.yml | 74 ++++++++++++++++++++++++++++++ 3 files changed, 161 insertions(+) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 41355f2..b8ed64a 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -59,6 +59,24 @@ on: description: 'The run label to use for the actions' type: string default: 'ubuntu-latest' + enforce-fresh: + description: > + Opt-in re-run-safe freshness guard. When true, the infra apply and the + app deploy each re-read the live version immediately before applying and + skip (green no-op) if a newer/divergent version is already live — making + the deploy idempotent under any re-run. App is checked against + `version` (via `stage-url`/health), infra against `github.sha` (via the + deployed_infra_commit SSM marker). Default false = unchanged behaviour. + type: boolean + default: false + freshness-module-id: + description: 'Terraform module.this.id prefix, for the infra SSM marker read. Required when enforce-fresh.' + type: string + default: '' + freshness-allow-stale: + description: 'Bypass the freshness guard (deliberate rollback from an older ref).' + type: boolean + default: false concurrency: cd-${{ inputs.stage }} @@ -83,6 +101,11 @@ jobs: tf-directory: ${{ inputs.tf-directory }} tf-variables: ${{ inputs.tf-variables }} run-label: ${{ inputs.run-label }} + enforce-fresh: ${{ inputs.enforce-fresh }} + freshness-target-commit: ${{ github.sha }} + freshness-module-id: ${{ inputs.freshness-module-id }} + freshness-aws-role-arn: ${{ inputs.aws-role-arn }} + freshness-allow-stale: ${{ inputs.freshness-allow-stale }} deploy-app: name: Deploy App @@ -99,3 +122,6 @@ jobs: aws-role-arn: ${{ inputs.aws-role-arn }} aws-region: ${{ inputs.aws-region }} run-label: ${{ inputs.run-label }} + enforce-fresh: ${{ inputs.enforce-fresh }} + freshness-target-commit: ${{ inputs.version }} + freshness-allow-stale: ${{ inputs.freshness-allow-stale }} diff --git a/.github/workflows/deploy-app.yml b/.github/workflows/deploy-app.yml index ca0c90a..74756e3 100644 --- a/.github/workflows/deploy-app.yml +++ b/.github/workflows/deploy-app.yml @@ -35,6 +35,24 @@ on: description: 'The run label to use for the actions' type: string default: 'ubuntu-latest' + enforce-fresh: + description: > + Opt-in re-run-safe guard. When true, immediately before the ECS deploy + this job re-reads the live app version and SKIPS the deploy (green + no-op) if it is NOT an ancestor-or-equal of `freshness-target-commit` — + i.e. a newer/divergent version is already live, so deploying would roll + it back. Being a step inside this job, it re-evaluates on any re-run. + Default false = unchanged behaviour for callers that do not opt in. + type: boolean + default: false + freshness-target-commit: + description: 'Commit this run intends to make live (the image tag/sha). Required when enforce-fresh.' + type: string + default: '' + freshness-allow-stale: + description: 'Bypass the guard (deliberate rollback from an older ref).' + type: boolean + default: false concurrency: deploy-${{ inputs.stage }} @@ -74,8 +92,51 @@ jobs: id: login-ecr uses: aws-actions/amazon-ecr-login@v2 + # Re-run-safe freshness guard (opt-in). Compare-and-set: skip the deploy if + # the live app version is already newer/divergent vs the version this run + # would ship — so a stale re-run (or a superseded run's "re-run this job") + # cannot roll the app back. A STEP here (not an upstream gate job) so it + # re-evaluates whenever this job runs. Fail-open on an unreadable /health; + # fail-closed on a live commit unresolvable in git history. + - name: Assert fresh + id: freshness + if: ${{ inputs.enforce-fresh }} + env: + TARGET: ${{ inputs.freshness-target-commit }} + HEALTH_URL: ${{ inputs.stage-url }} + ALLOW_STALE: ${{ inputs.freshness-allow-stale }} + run: | + set -euo pipefail + git fetch --no-tags --prune --unshallow origin 2>/dev/null || git fetch --no-tags origin 2>/dev/null || true + if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$TARGET" 2>/dev/null || true; fi + if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then + echo "::error::assert-fresh(app): target ${TARGET} is not a resolvable commit"; exit 1 + fi + health="$(curl -fsS --max-time 15 --retry 3 --retry-delay 5 "$HEALTH_URL" || true)" + short="$(printf '%s' "$health" | sed -n 's/.*commit hash: \([0-9a-f]\{7,40\}\).*/\1/p;T;q')" + running=""; [ -z "$short" ] || running="$(git rev-parse "$short" 2>/dev/null || echo "$short")" + skip=false + if [ -z "$running" ] || [ "$running" = "None" ]; then + echo "app: live version unknown — not blocking" + else + if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi + if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then + echo "::warning::app: live ${running:0:7} unresolvable — stale"; skip=true + elif git merge-base --is-ancestor "$running" "$TARGET"; then + echo "app: live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" + else + echo "::warning::app: live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already live"; skip=true + fi + fi + if [ "$skip" = "true" ] && [ "$ALLOW_STALE" = "true" ]; then + echo "::notice::app: STALE but allow-stale set — proceeding"; skip=false + fi + [ "$skip" = "true" ] && echo "::notice::app: STALE — skipping ECS deploy (a newer version is already live)" || true + echo "skip=$skip" >> "$GITHUB_OUTPUT" + - name: Deploy image to ECS id: deploy + if: ${{ steps.freshness.outputs.skip != 'true' }} uses: WalletConnect/actions/aws/ecs/deploy-image/@2.5.4 with: aws-role-arn: ${{ inputs.aws-role-arn }} diff --git a/.github/workflows/deploy-infra.yml b/.github/workflows/deploy-infra.yml index 6703b75..38b8796 100644 --- a/.github/workflows/deploy-infra.yml +++ b/.github/workflows/deploy-infra.yml @@ -39,6 +39,31 @@ on: description: 'The run label to use for the actions' type: string default: 'ubuntu-latest' + enforce-fresh: + description: > + Opt-in re-run-safe guard. When true, immediately before `terraform + apply` this job re-reads the live infra commit (the deployed_infra_commit + SSM marker) and SKIPS the apply (green no-op) if it is NOT an + ancestor-or-equal of `freshness-target-commit`. A step inside this job, + so it re-evaluates on any re-run. Default false = unchanged behaviour. + type: boolean + default: false + freshness-target-commit: + description: 'Commit this run intends to apply (github.sha). Required when enforce-fresh.' + type: string + default: '' + freshness-module-id: + description: 'Terraform module.this.id prefix, for the deployed_infra_commit SSM marker read. Required when enforce-fresh.' + type: string + default: '' + freshness-aws-role-arn: + description: 'AWS role to assume to read the SSM marker (the app account, not monitoring). Required when enforce-fresh.' + type: string + default: '' + freshness-allow-stale: + description: 'Bypass the guard (deliberate rollback from an older ref).' + type: boolean + default: false secrets: TF_API_TOKEN: required: true @@ -67,6 +92,54 @@ jobs: submodules: recursive token: ${{ secrets.PRIVATE_SUBMODULE_ACCESS_TOKEN || github.token }} + # Re-run-safe freshness guard (opt-in), placed BEFORE the monitoring-creds + # config so that config restores monitoring creds for the Grafana steps + # (this guard needs app-account creds to read the SSM marker). Compare-and- + # set: skip `terraform apply` if the live infra commit is already + # newer/divergent vs what this run would apply, so a stale re-run cannot + # roll infra back. A step inside this job → re-evaluates on any re-run. + - name: Configure AWS Credentials for freshness read + if: ${{ inputs.enforce-fresh }} + uses: aws-actions/configure-aws-credentials@v4 + with: + role-to-assume: ${{ inputs.freshness-aws-role-arn }} + aws-region: ${{ inputs.aws-region }} + + - name: Assert fresh + id: freshness + if: ${{ inputs.enforce-fresh }} + env: + TARGET: ${{ inputs.freshness-target-commit }} + MODULE_ID: ${{ inputs.freshness-module-id }} + AWS_REGION: ${{ inputs.aws-region }} + ALLOW_STALE: ${{ inputs.freshness-allow-stale }} + run: | + set -euo pipefail + git fetch --no-tags --prune --unshallow origin 2>/dev/null || git fetch --no-tags origin 2>/dev/null || true + if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$TARGET" 2>/dev/null || true; fi + if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then + echo "::error::assert-fresh(infra): target ${TARGET} is not a resolvable commit"; exit 1 + fi + running="$(aws ssm get-parameter --name "/${MODULE_ID}/deployed_infra_commit" --query 'Parameter.Value' --output text --region "$AWS_REGION" 2>/dev/null || true)" + skip=false + if [ -z "$running" ] || [ "$running" = "None" ] || [ "$running" = "unknown" ]; then + echo "infra: live marker unknown — not blocking" + else + if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi + if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then + echo "::warning::infra: live ${running:0:7} unresolvable — stale"; skip=true + elif git merge-base --is-ancestor "$running" "$TARGET"; then + echo "infra: live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" + else + echo "::warning::infra: live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already applied"; skip=true + fi + fi + if [ "$skip" = "true" ] && [ "$ALLOW_STALE" = "true" ]; then + echo "::notice::infra: STALE but allow-stale set — proceeding"; skip=false + fi + [ "$skip" = "true" ] && echo "::notice::infra: STALE — skipping terraform apply (a newer commit is already applied)" || true + echo "skip=$skip" >> "$GITHUB_OUTPUT" + - name: Configure AWS Credentials for Monitoring account uses: aws-actions/configure-aws-credentials@v4 with: @@ -105,6 +178,7 @@ jobs: ${{ inputs.tf-variables }} - name: Apply on ${{ inputs.stage }} + if: ${{ steps.freshness.outputs.skip != 'true' }} working-directory: ${{ inputs.tf-directory }} run: terraform apply -auto-approve -no-color From bb066ff9c746ca4d2110e87725aed715f4a56593 Mon Sep 17 00:00:00 2001 From: rplusq Date: Fri, 10 Jul 2026 13:13:17 +0100 Subject: [PATCH 2/6] =?UTF-8?q?fix(cd):=20harden=20freshness=20guard=20per?= =?UTF-8?q?=20review=20=E2=80=94=20fail-closed=20config,=20portable,=20all?= =?UTF-8?q?ow-stale=20bypass?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Missing freshness-module-id when enforce-fresh now FAILS CLOSED (was a silent fail-open that didn't protect infra). - allow-stale fully bypasses the guard (steps gated `enforce-fresh && !allow-stale`) so a deliberate rollback can't be wedged by the guard's own machinery. - Portable extraction (grep|awk, not GNU sed `T`) + explicit `shell: bash`. - App target that isn't a git commit → warn + fail-open (skip=false), not a hard block; blind live-version reads emit `::warning` so an inert guard is visible. - continue-on-error on the infra freshness-creds step so a transient STS blip fails the guard OPEN rather than wedging the deploy. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/cd.yml | 5 +++++ .github/workflows/deploy-app.yml | 27 +++++++++++++++------------ .github/workflows/deploy-infra.yml | 28 +++++++++++++++++----------- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index b8ed64a..6153dc7 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -67,6 +67,11 @@ on: the deploy idempotent under any re-run. App is checked against `version` (via `stage-url`/health), infra against `github.sha` (via the deployed_infra_commit SSM marker). Default false = unchanged behaviour. + Opting in expects `version` to be a git commit SHA and `stage-url` to + serve the `commit hash: ` banner; a non-commit `version` or an + unreadable banner fails OPEN with a warning (never a false block). + `freshness-module-id` is REQUIRED when enforce-fresh (missing it fails + the infra apply closed). type: boolean default: false freshness-module-id: diff --git a/.github/workflows/deploy-app.yml b/.github/workflows/deploy-app.yml index 74756e3..499a27c 100644 --- a/.github/workflows/deploy-app.yml +++ b/.github/workflows/deploy-app.yml @@ -98,40 +98,43 @@ jobs: # cannot roll the app back. A STEP here (not an upstream gate job) so it # re-evaluates whenever this job runs. Fail-open on an unreadable /health; # fail-closed on a live commit unresolvable in git history. + # allow-stale bypasses the guard entirely (step skipped → skip output empty + # → deploy runs), so a deliberate rollback can't be wedged by the guard's own + # machinery (unresolvable target, etc.). - name: Assert fresh id: freshness - if: ${{ inputs.enforce-fresh }} + if: ${{ inputs.enforce-fresh && !inputs.freshness-allow-stale }} + shell: bash env: TARGET: ${{ inputs.freshness-target-commit }} HEALTH_URL: ${{ inputs.stage-url }} - ALLOW_STALE: ${{ inputs.freshness-allow-stale }} run: | set -euo pipefail git fetch --no-tags --prune --unshallow origin 2>/dev/null || git fetch --no-tags origin 2>/dev/null || true if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$TARGET" 2>/dev/null || true; fi if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then - echo "::error::assert-fresh(app): target ${TARGET} is not a resolvable commit"; exit 1 + # Target isn't a git commit (e.g. a semver/'latest' ECR tag from a + # non-pay-core consumer). Can't ancestry-check → fail OPEN, LOUDLY, + # so it never masquerades as protection. + echo "::warning::assert-fresh(app): target '${TARGET}' is not a git commit — cannot evaluate freshness; proceeding UNGUARDED" + echo "skip=false" >> "$GITHUB_OUTPUT"; exit 0 fi health="$(curl -fsS --max-time 15 --retry 3 --retry-delay 5 "$HEALTH_URL" || true)" - short="$(printf '%s' "$health" | sed -n 's/.*commit hash: \([0-9a-f]\{7,40\}\).*/\1/p;T;q')" + short="$(printf '%s' "$health" | grep -oiE 'commit hash: [0-9a-f]{7,40}' | head -n1 | awk '{print $3}')" running=""; [ -z "$short" ] || running="$(git rev-parse "$short" 2>/dev/null || echo "$short")" skip=false if [ -z "$running" ] || [ "$running" = "None" ]; then - echo "app: live version unknown — not blocking" + echo "::warning::assert-fresh(app): live version unreadable at ${HEALTH_URL} — proceeding UNGUARDED (fail-open)" else if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then - echo "::warning::app: live ${running:0:7} unresolvable — stale"; skip=true + echo "::warning::assert-fresh(app): live ${running:0:7} unresolvable in history — treating as stale"; skip=true elif git merge-base --is-ancestor "$running" "$TARGET"; then - echo "app: live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" + echo "assert-fresh(app): live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" else - echo "::warning::app: live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already live"; skip=true + echo "::warning::assert-fresh(app): live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already live; skipping ECS deploy"; skip=true fi fi - if [ "$skip" = "true" ] && [ "$ALLOW_STALE" = "true" ]; then - echo "::notice::app: STALE but allow-stale set — proceeding"; skip=false - fi - [ "$skip" = "true" ] && echo "::notice::app: STALE — skipping ECS deploy (a newer version is already live)" || true echo "skip=$skip" >> "$GITHUB_OUTPUT" - name: Deploy image to ECS diff --git a/.github/workflows/deploy-infra.yml b/.github/workflows/deploy-infra.yml index 38b8796..4e664e4 100644 --- a/.github/workflows/deploy-infra.yml +++ b/.github/workflows/deploy-infra.yml @@ -98,8 +98,13 @@ jobs: # set: skip `terraform apply` if the live infra commit is already # newer/divergent vs what this run would apply, so a stale re-run cannot # roll infra back. A step inside this job → re-evaluates on any re-run. + # allow-stale bypasses the guard entirely (steps skipped → skip output empty + # → apply runs), so a deliberate rollback isn't wedged by the guard itself. + # continue-on-error on the creds step: a transient STS/OIDC blip must not + # wedge the deploy — the SSM read then fails and the guard fails OPEN. - name: Configure AWS Credentials for freshness read - if: ${{ inputs.enforce-fresh }} + if: ${{ inputs.enforce-fresh && !inputs.freshness-allow-stale }} + continue-on-error: true uses: aws-actions/configure-aws-credentials@v4 with: role-to-assume: ${{ inputs.freshness-aws-role-arn }} @@ -107,14 +112,19 @@ jobs: - name: Assert fresh id: freshness - if: ${{ inputs.enforce-fresh }} + if: ${{ inputs.enforce-fresh && !inputs.freshness-allow-stale }} + shell: bash env: TARGET: ${{ inputs.freshness-target-commit }} MODULE_ID: ${{ inputs.freshness-module-id }} AWS_REGION: ${{ inputs.aws-region }} - ALLOW_STALE: ${{ inputs.freshness-allow-stale }} run: | set -euo pipefail + # A guard that can't locate its marker would silently NOT protect — the + # exact failure mode this exists to prevent. Fail CLOSED on missing config. + if [ -z "$MODULE_ID" ]; then + echo "::error::assert-fresh(infra): enforce-fresh is set but freshness-module-id is empty — refusing to apply unguarded"; exit 1 + fi git fetch --no-tags --prune --unshallow origin 2>/dev/null || git fetch --no-tags origin 2>/dev/null || true if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$TARGET" 2>/dev/null || true; fi if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then @@ -123,21 +133,17 @@ jobs: running="$(aws ssm get-parameter --name "/${MODULE_ID}/deployed_infra_commit" --query 'Parameter.Value' --output text --region "$AWS_REGION" 2>/dev/null || true)" skip=false if [ -z "$running" ] || [ "$running" = "None" ] || [ "$running" = "unknown" ]; then - echo "infra: live marker unknown — not blocking" + echo "::warning::assert-fresh(infra): live marker unreadable (/${MODULE_ID}/deployed_infra_commit) — proceeding UNGUARDED (fail-open)" else if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then - echo "::warning::infra: live ${running:0:7} unresolvable — stale"; skip=true + echo "::warning::assert-fresh(infra): live ${running:0:7} unresolvable in history — treating as stale"; skip=true elif git merge-base --is-ancestor "$running" "$TARGET"; then - echo "infra: live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" + echo "assert-fresh(infra): live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" else - echo "::warning::infra: live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already applied"; skip=true + echo "::warning::assert-fresh(infra): live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already applied; skipping terraform apply"; skip=true fi fi - if [ "$skip" = "true" ] && [ "$ALLOW_STALE" = "true" ]; then - echo "::notice::infra: STALE but allow-stale set — proceeding"; skip=false - fi - [ "$skip" = "true" ] && echo "::notice::infra: STALE — skipping terraform apply (a newer commit is already applied)" || true echo "skip=$skip" >> "$GITHUB_OUTPUT" - name: Configure AWS Credentials for Monitoring account From 7f42a24a51ecf691453a817e2fc1b49feae1ec5c Mon Sep 17 00:00:00 2001 From: rplusq Date: Fri, 10 Jul 2026 13:20:03 +0100 Subject: [PATCH 3/6] fix(cd): keep the app freshness guard fail-open on a banner-less /health The grep|awk parser exits 1 when /health is unreachable or lacks the `commit hash:` token (common when /health flaps during an ECS task cycle); under set -euo pipefail that aborted the step before the fail-open branch, turning warn-and-proceed into a red deploy. Add `|| true` so an unreadable banner proceeds UNGUARDED with a warning, as intended. (Both re-review passes flagged this as the sole remaining HIGH.) Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/deploy-app.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/deploy-app.yml b/.github/workflows/deploy-app.yml index 499a27c..a9af65f 100644 --- a/.github/workflows/deploy-app.yml +++ b/.github/workflows/deploy-app.yml @@ -120,7 +120,11 @@ jobs: echo "skip=false" >> "$GITHUB_OUTPUT"; exit 0 fi health="$(curl -fsS --max-time 15 --retry 3 --retry-delay 5 "$HEALTH_URL" || true)" - short="$(printf '%s' "$health" | grep -oiE 'commit hash: [0-9a-f]{7,40}' | head -n1 | awk '{print $3}')" + # `|| true`: grep exits 1 when the banner is absent (curl failed / body + # lacks the token — common when /health flaps during an ECS cycle); under + # `set -euo pipefail` that would kill the step BEFORE the fail-open branch, + # turning warn-and-proceed into a red deploy. Keep it fail-open. + short="$(printf '%s' "$health" | grep -oiE 'commit hash: [0-9a-f]{7,40}' | head -n1 | awk '{print $3}' || true)" running=""; [ -z "$short" ] || running="$(git rev-parse "$short" 2>/dev/null || echo "$short")" skip=false if [ -z "$running" ] || [ "$running" = "None" ]; then From 46cb0e8a73774459cdc57c813138b6c816f30d0a Mon Sep 17 00:00:00 2001 From: rplusq Date: Mon, 13 Jul 2026 14:23:18 +0100 Subject: [PATCH 4/6] fix(cd): fail-closed on a genuine infra-marker read error (keep /health fail-open) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per review: the guard could still permit a rollback if a live-version read blips during a stale/re-run race. Split the read-failure policy — the app /health read stays fail-OPEN (it flaps during ECS cycling; blocking would wedge real deploys), but the reliable SSM marker read now fails CLOSED on a genuine error (access denied / throttle / timeout): refuse to apply blind rather than risk overwriting a newer commit. A not-yet-created marker (ParameterNotFound, first apply) still proceeds. freshness-allow-stale overrides. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/cd.yml | 10 ++++++---- .github/workflows/deploy-infra.yml | 22 ++++++++++++++++++++-- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/.github/workflows/cd.yml b/.github/workflows/cd.yml index 6153dc7..58e62c9 100644 --- a/.github/workflows/cd.yml +++ b/.github/workflows/cd.yml @@ -68,10 +68,12 @@ on: `version` (via `stage-url`/health), infra against `github.sha` (via the deployed_infra_commit SSM marker). Default false = unchanged behaviour. Opting in expects `version` to be a git commit SHA and `stage-url` to - serve the `commit hash: ` banner; a non-commit `version` or an - unreadable banner fails OPEN with a warning (never a false block). - `freshness-module-id` is REQUIRED when enforce-fresh (missing it fails - the infra apply closed). + serve the `commit hash: ` banner. Read-failure policy: the app + `/health` read fails OPEN (it flaps during ECS cycling — a warning, never + a false block), but the reliable infra SSM-marker read fails CLOSED on a + genuine error (access denied / throttle / timeout — refuses to apply + blind), while a not-yet-created marker still proceeds. `freshness-module-id` + is REQUIRED when enforce-fresh; `freshness-allow-stale` overrides all of it. type: boolean default: false freshness-module-id: diff --git a/.github/workflows/deploy-infra.yml b/.github/workflows/deploy-infra.yml index 4e664e4..5af25e8 100644 --- a/.github/workflows/deploy-infra.yml +++ b/.github/workflows/deploy-infra.yml @@ -130,10 +130,28 @@ jobs: if ! git cat-file -e "${TARGET}^{commit}" 2>/dev/null; then echo "::error::assert-fresh(infra): target ${TARGET} is not a resolvable commit"; exit 1 fi - running="$(aws ssm get-parameter --name "/${MODULE_ID}/deployed_infra_commit" --query 'Parameter.Value' --output text --region "$AWS_REGION" 2>/dev/null || true)" + # Read the marker distinguishing "not created yet" (legit first apply → + # fail OPEN) from a genuine read ERROR — access denied, throttle, timeout + # (anomalous → fail CLOSED). SSM is a reliable read, so an error here is a + # real red flag and we will NOT apply blind and risk a rollback. (The app + # /health guard stays fail-open because it flaps during ECS cycling; this + # marker does not.) `freshness-allow-stale` overrides for a deliberate run. + set +e + running="$(aws ssm get-parameter --name "/${MODULE_ID}/deployed_infra_commit" --query 'Parameter.Value' --output text --region "$AWS_REGION" 2>/tmp/ssm_err)" + ssm_rc=$? + set -e skip=false + if [ "$ssm_rc" -ne 0 ]; then + if grep -q "ParameterNotFound" /tmp/ssm_err; then + echo "::warning::assert-fresh(infra): marker /${MODULE_ID}/deployed_infra_commit not created yet (first apply) — proceeding" + running="" + else + echo "::error::assert-fresh(infra): could not read the deployed_infra_commit marker ($(tr -d '\n' /dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then From a44372384c0fdabcd85fd07454637f70aa2c7797 Mon Sep 17 00:00:00 2001 From: rplusq Date: Mon, 13 Jul 2026 14:36:29 +0100 Subject: [PATCH 5/6] docs(cd): fix stale fail-open comment + restore blind-proceed warning Review polish on the fail-closed change (both advisors flagged the comment): - The creds step no longer claims to fail OPEN on an STS/OIDC blip (it now fails closed via the SSM read); drop continue-on-error so a creds failure surfaces loudly at its own step instead of re-surfacing as "can't read marker". - Restore ::warning on the empty/None/unknown proceed-open path and word it accurately (first apply OR a never-completed prior apply); drop the duplicate not-found log. No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- .github/workflows/deploy-infra.yml | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/deploy-infra.yml b/.github/workflows/deploy-infra.yml index 5af25e8..69119ad 100644 --- a/.github/workflows/deploy-infra.yml +++ b/.github/workflows/deploy-infra.yml @@ -100,11 +100,12 @@ jobs: # roll infra back. A step inside this job → re-evaluates on any re-run. # allow-stale bypasses the guard entirely (steps skipped → skip output empty # → apply runs), so a deliberate rollback isn't wedged by the guard itself. - # continue-on-error on the creds step: a transient STS/OIDC blip must not - # wedge the deploy — the SSM read then fails and the guard fails OPEN. + # No continue-on-error: the marker read below fails CLOSED, so a creds + # failure should surface loudly at THIS step (with the OIDC error) and block + # the apply — not be swallowed and re-surface as a confusing "can't read + # marker" at the next step. Rerun, or set freshness-allow-stale to override. - name: Configure AWS Credentials for freshness read if: ${{ inputs.enforce-fresh && !inputs.freshness-allow-stale }} - continue-on-error: true uses: aws-actions/configure-aws-credentials@v4 with: role-to-assume: ${{ inputs.freshness-aws-role-arn }} @@ -143,15 +144,14 @@ jobs: skip=false if [ "$ssm_rc" -ne 0 ]; then if grep -q "ParameterNotFound" /tmp/ssm_err; then - echo "::warning::assert-fresh(infra): marker /${MODULE_ID}/deployed_infra_commit not created yet (first apply) — proceeding" - running="" + running="" # marker not created yet (first apply) — the open check below logs it else echo "::error::assert-fresh(infra): could not read the deployed_infra_commit marker ($(tr -d '\n' /dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then From 8539e3f89bd270dd966b6398e32d07fa26094d56 Mon Sep 17 00:00:00 2001 From: rplusq Date: Wed, 15 Jul 2026 13:09:27 +0100 Subject: [PATCH 6/6] Close deploy freshness re-run gaps --- .github/workflows/deploy-app.yml | 54 ++++++++++++++++++----------- .github/workflows/deploy-infra.yml | 55 ++++++++++++++++++++++++++++-- 2 files changed, 86 insertions(+), 23 deletions(-) diff --git a/.github/workflows/deploy-app.yml b/.github/workflows/deploy-app.yml index a9af65f..818e75d 100644 --- a/.github/workflows/deploy-app.yml +++ b/.github/workflows/deploy-app.yml @@ -96,8 +96,9 @@ jobs: # the live app version is already newer/divergent vs the version this run # would ship — so a stale re-run (or a superseded run's "re-run this job") # cannot roll the app back. A STEP here (not an upstream gate job) so it - # re-evaluates whenever this job runs. Fail-open on an unreadable /health; - # fail-closed on a live commit unresolvable in git history. + # re-evaluates whenever this job runs. The ECS control plane is a reliable + # read, like the infra SSM marker, so a read error fails CLOSED rather than + # allowing a deploy to proceed blind. # allow-stale bypasses the guard entirely (step skipped → skip output empty # → deploy runs), so a deliberate rollback can't be wedged by the guard's own # machinery (unresolvable target, etc.). @@ -107,7 +108,8 @@ jobs: shell: bash env: TARGET: ${{ inputs.freshness-target-commit }} - HEALTH_URL: ${{ inputs.stage-url }} + NAME: ${{ steps.build_task_name.outputs.name }} + AWS_REGION: ${{ inputs.aws-region }} run: | set -euo pipefail git fetch --no-tags --prune --unshallow origin 2>/dev/null || git fetch --no-tags origin 2>/dev/null || true @@ -119,25 +121,37 @@ jobs: echo "::warning::assert-fresh(app): target '${TARGET}' is not a git commit — cannot evaluate freshness; proceeding UNGUARDED" echo "skip=false" >> "$GITHUB_OUTPUT"; exit 0 fi - health="$(curl -fsS --max-time 15 --retry 3 --retry-delay 5 "$HEALTH_URL" || true)" - # `|| true`: grep exits 1 when the banner is absent (curl failed / body - # lacks the token — common when /health flaps during an ECS cycle); under - # `set -euo pipefail` that would kill the step BEFORE the fail-open branch, - # turning warn-and-proceed into a red deploy. Keep it fail-open. - short="$(printf '%s' "$health" | grep -oiE 'commit hash: [0-9a-f]{7,40}' | head -n1 | awk '{print $3}' || true)" - running=""; [ -z "$short" ] || running="$(git rev-parse "$short" 2>/dev/null || echo "$short")" + set +e + td="$(aws ecs describe-services --cluster "${NAME}-cluster" --services "${NAME}-service" --query "services[0].deployments[?status=='PRIMARY'].taskDefinition | [0]" --output text --region "$AWS_REGION" 2>/tmp/ecs_err)" + ecs_rc=$? + set -e + if [ "$ecs_rc" -ne 0 ] || [ -z "$td" ] || [ "$td" = "None" ]; then + echo "::error::assert-fresh(app): could not read the PRIMARY ECS task definition ($(tr -d '\n' /tmp/ecs_err)" + image_rc=$? + set -e + if [ "$image_rc" -ne 0 ] || [ -z "$image" ] || [ "$image" = "None" ]; then + set +e + image="$(aws ecs describe-task-definition --task-definition "$td" --query "taskDefinition.containerDefinitions[0].image" --output text --region "$AWS_REGION" 2>/tmp/ecs_err)" + image_rc=$? + set -e + fi + if [ "$image_rc" -ne 0 ] || [ -z "$image" ] || [ "$image" = "None" ]; then + echo "::error::assert-fresh(app): could not read the PRIMARY ECS task definition ($(tr -d '\n' /dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi + if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then + echo "::warning::assert-fresh(app): live ${running:0:7} unresolvable in history — treating as stale"; skip=true + elif git merge-base --is-ancestor "$running" "$TARGET"; then + echo "assert-fresh(app): live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" else - if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$running" 2>/dev/null || true; fi - if ! git cat-file -e "${running}^{commit}" 2>/dev/null; then - echo "::warning::assert-fresh(app): live ${running:0:7} unresolvable in history — treating as stale"; skip=true - elif git merge-base --is-ancestor "$running" "$TARGET"; then - echo "assert-fresh(app): live ${running:0:7} is an ancestor of ${TARGET:0:7} — fresh" - else - echo "::warning::assert-fresh(app): live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already live; skipping ECS deploy"; skip=true - fi + echo "::warning::assert-fresh(app): live ${running:0:7} is NOT an ancestor of ${TARGET:0:7} — newer/divergent already live; skipping ECS deploy"; skip=true fi echo "skip=$skip" >> "$GITHUB_OUTPUT" diff --git a/.github/workflows/deploy-infra.yml b/.github/workflows/deploy-infra.yml index 69119ad..eb875fc 100644 --- a/.github/workflows/deploy-infra.yml +++ b/.github/workflows/deploy-infra.yml @@ -134,9 +134,9 @@ jobs: # Read the marker distinguishing "not created yet" (legit first apply → # fail OPEN) from a genuine read ERROR — access denied, throttle, timeout # (anomalous → fail CLOSED). SSM is a reliable read, so an error here is a - # real red flag and we will NOT apply blind and risk a rollback. (The app - # /health guard stays fail-open because it flaps during ECS cycling; this - # marker does not.) `freshness-allow-stale` overrides for a deliberate run. + # real red flag and we will NOT apply blind and risk a rollback. The app + # guard likewise uses the reliable ECS control plane and fails closed. + # `freshness-allow-stale` overrides for a deliberate run. set +e running="$(aws ssm get-parameter --name "/${MODULE_ID}/deployed_infra_commit" --query 'Parameter.Value' --output text --region "$AWS_REGION" 2>/tmp/ssm_err)" ssm_rc=$? @@ -212,3 +212,52 @@ jobs: with: workspace-id: ${{ steps.grafana-get-key.outputs.workspace-id }} key-name: ${{ steps.grafana-get-key.outputs.key-name }} + + - name: Configure AWS Credentials for marker write + if: ${{ inputs.enforce-fresh && !inputs.freshness-allow-stale && steps.freshness.outputs.skip != 'true' }} + uses: aws-actions/configure-aws-credentials@v4 + with: + role-to-assume: ${{ inputs.freshness-aws-role-arn }} + aws-region: ${{ inputs.aws-region }} + + - name: Advance infra marker + if: ${{ inputs.enforce-fresh && !inputs.freshness-allow-stale && steps.freshness.outputs.skip != 'true' }} + shell: bash + env: + SHA: ${{ inputs.freshness-target-commit }} + MODULE_ID: ${{ inputs.freshness-module-id }} + AWS_REGION: ${{ inputs.aws-region }} + run: | + set -euo pipefail + set +e + current="$(aws ssm get-parameter --name "/${MODULE_ID}/deployed_infra_commit" --query 'Parameter.Value' --output text --region "$AWS_REGION" 2>/tmp/ssm_err)" + ssm_rc=$? + set -e + if [ "$ssm_rc" -ne 0 ]; then + if grep -q "ParameterNotFound" /tmp/ssm_err; then + aws ssm put-parameter --name "/${MODULE_ID}/deployed_infra_commit" --type String --overwrite --value "$SHA" --region "$AWS_REGION" >/dev/null + exit 0 + fi + echo "::warning::advance-infra-marker: could not read the deployed_infra_commit marker ($(tr -d '\n' /dev/null + exit 0 + fi + if ! git cat-file -e "${SHA}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$SHA" 2>/dev/null || true; fi + if ! git cat-file -e "${current}^{commit}" 2>/dev/null; then git fetch --no-tags origin "$current" 2>/dev/null || true; fi + if ! git cat-file -e "${SHA}^{commit}" 2>/dev/null; then + echo "::warning::advance-infra-marker: target ${SHA:0:7} is unresolvable in history — marker not advanced" + exit 0 + fi + if ! git cat-file -e "${current}^{commit}" 2>/dev/null; then + echo "::warning::advance-infra-marker: current marker ${current:0:7} is unresolvable in history — marker not advanced" + exit 0 + fi + if git merge-base --is-ancestor "$current" "$SHA"; then + aws ssm put-parameter --name "/${MODULE_ID}/deployed_infra_commit" --type String --overwrite --value "$SHA" --region "$AWS_REGION" >/dev/null + else + echo "::warning::advance-infra-marker: marker ${current:0:7} newer/divergent than ${SHA:0:7} — refusing to regress" + exit 0 + fi