sec(ci): destroy-fargate-dev.yml force-deletes the state lock and matches ECR repos by substring #1592

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

Where

• .github/workflows/destroy-fargate-dev.yml:63-66 - aws s3 rm ...tflock with || true, run before terraform init, on every invocation
• .github/workflows/destroy-fargate-dev.yml:70-78 - the ECR delete loop; the query at :73 uses contains(repositoryName,'cudly-dev')
• Contrast: .github/workflows/deploy-aws-fargate.yml:117-118 gates its equivalent behind the operator input clear_stale_lock (:40-44)
• Contrast: .github/workflows/cleanup-staging.yml:85-91 adds an explicit case "$REPO" in cudly-staging*) allow-list

What

Two independent defects in one workflow.

1. Unconditional state-lock delete. The aws s3 rm at :65 runs on every invocation, before terraform init, with || true so it cannot fail. It is not gated behind an operator input the way the Fargate deploy workflow gates its equivalent. There is no concurrency: group on this workflow either, so nothing prevents two invocations overlapping.

2. Substring ECR matching followed by force-delete. The query is contains(repositoryName,'cudly-dev'), a substring match, and the loop then calls delete-repository --force. cleanup-staging.yml:85-91 added an explicit case allow-list for precisely this reason, with the in-file rationale calling out:

   in particular the bare 'cudly' prod-adjacent repo

   The dev workflow never received that guard.

Failure scenario

1. The workflow is dispatched while another run (or a deploy) holds the github-fargate-dev lock. The unconditional delete frees the lock out from under the run that is still applying, both proceed, and the state is corrupted by concurrent writes.
2. Any ECR repository whose name merely contains cudly-dev - for example backup-cudly-dev or cudly-dev-prod-mirror - is force-deleted along with every image it holds, in whatever account AWS_ROLE_TO_ASSUME points at. There is no confirmation step between the query and the delete.

Fix direction

• Gate the lock delete behind an explicit clear_stale_lock-style workflow_dispatch input, mirroring deploy-aws-fargate.yml:40-44,117-118, and drop the || true so a failed delete is visible.

• Add a concurrency: group keyed on the state key (not on github.ref - the state key is ref-independent).

• Delete only the exact repository names this workflow owns. Enumerate them (the same names the dev Fargate deploy pushes to) and compare with ==.

  Not starts_with('cudly-dev'), and not the case "$REPO" in cudly-dev*) allow-list cleanup-staging.yml uses: both still match cudly-dev-prod-mirror, which is the counterexample in the failure scenario above. A prefix guard reads as a fix for a substring bug while leaving the same class open, and copying the staging workflow's guard would spread that shape rather than contain it. An exact list is also shorter than either.

Remedy simplified (2026-08-03): starts_with plus a prefix allow-list was replaced with an exact-name list. The original remedy did not close its own failure scenario, and the repo has shipped substring-for-equality six times; an exact comparison is both smaller and the only one that holds.

Related

• sec(ci): lambda deploy has same unsafe failure-path state-lock delete as #438 #1269 - the same class of unsafe failure-path lock delete in deploy-aws-lambda.yml; fixing both together is one coherent change.
• fix(ci): restrict staging ECR cleanup to cudly-staging repos #1235 / ci: harden remaining staging cleanup + rollback hardening (silent AWS error swallowing, verify-image var-interpolation) #1330 - the earlier ECR delete-loop scope tightening that covered cleanup-staging.yml but not destroy-fargate-dev.yml.
• The missing concurrency: group is also tracked as part of the broader no-concurrency issue from this review.