Repository navigation
sec(ci): lambda deploy has same unsafe failure-path state-lock delete as #438 #1269
Description
Problem
.github/workflows/deploy-aws-lambda.yml (around lines 190-199) still has an unconditional failure-path state-lock release:
- name: Release state lock on failure
if: failure() || cancelled()
run: |
...
aws s3 rm "s3://${BUCKET}/${LOCK_KEY}"This is the same state-corruption race that #438 closed for deploy-aws-fargate.yml: a run that fails because it could not acquire the lock will execute this cleanup and delete the lock held by another run that is still actively applying, enabling parallel state writes.
The Fargate workflow now relies solely on the operator-triggered clear_stale_lock input (PR #818). The Lambda workflow should adopt the same approach.
Fix
Remove the automatic Release state lock on failure step from deploy-aws-lambda.yml and add an operator-triggered clear_stale_lock workflow_dispatch boolean input (mirroring the Fargate workflow). Terraform already releases its own lock on a clean apply error; a surviving lock means the run died abnormally and needs operator confirmation before clearing. Reference runbooks/terraform-stuck-lock.md.
Out of scope for #818
PR #818 is scoped to the Fargate workflow only; this is the parallel fix for the Lambda deploy path.
Activity
cristim commentedon Jul 28, 2026
Reviewed commit:
be11bdcb5. Note:origin/mainmoved to3e9660d06during the review; re-verify against currentmainbefore changing code, since a finding may have been fixed or moved.Still present on
3e9660d06. The 2026-07-28 full review reached this independently and adds the piece that turns it from a theoretical race into a reachable one: the concurrency groups do not cover the resource being protected.Where
.github/workflows/deploy-aws-lambda.yml:190-201-if: failure() || cancelled()thenaws s3 rm "s3://${BUCKET}/${LOCK_KEY}"(as already described in this issue).github/workflows/deploy-aws-lambda.yml:29-31-concurrency: deploy-lambda-${{ github.ref }}.github/workflows/deploy-aws-lambda.yml:150- state keygithub-<environment>/terraform.tfstate.github/workflows/rollback.yml:230- writes the same state key, with noconcurrency:group at all.github/workflows/deploy-aws-fargate.yml:160-167- the comment that spells out why this construct is forbiddenThe concurrency-group mismatch
The concurrency group is keyed on
github.ref. The state key is ref-independent (github-<environment>/terraform.tfstate). So two runs targeting the same state can sit in different concurrency groups and run simultaneously:workflow_dispatchofenvironment=devfrom a feature branch, andmaindev deploy.Different groups, one state key. The serialisation that would otherwise make the failure-path delete "mostly harmless" does not exist for the case that matters.
Failure scenario
Run A is mid-
terraform applyholding the lock. Run B starts in a different concurrency group, fails on lock acquisition, and itsif: failure()handler deletes A's lock while A is still applying. A third run then acquires the freed lock and applies concurrently, producing corrupted or split-brain state for the account that holds the purchase-executing Lambda, its RDS instance and its secrets.rollback.yml:230writes the same key with no concurrency group at all, so a rollback overlapping a deploy widens the window further, and in that pairing it is the rollback's lock that the deploy's failure handler deletes.deploy-aws-fargate.yml:160-167states the rule verbatim:Suggested severity
Recommend raising to
severity/criticaland keepingpriority/p1. The blast radius is the production account holding the money-path Lambda, and the fix (delete the step, adopt the fargate model) iseffort/xsas already labelled.Fix direction
As already described here: delete the step and adopt the fargate model (operator-gated
clear_stale_lockinput plusrunbooks/terraform-stuck-lock.md). Additionally re-key the concurrency group on the state key (environment plus cloud) rather thangithub.ref, and add a group torollback.yml, otherwise the same overlap remains possible with the failure-path delete removed. The concurrency gap acrossrollback.yml,database-migration.yml,cleanup-staging.ymlanddestroy-fargate-dev.ymlis filed separately from this review;destroy-fargate-dev.yml:63-66has an unconditional variant of the same lock delete and is filed separately too.cristim commentedon Sep 2, 2026
Verified resolved at 3c0f8ac: the automatic failure/cancel-path
aws s3 rmof the Terraform state lock no longer exists in deploy-aws-lambda.yml; recovery is the operator-confirmed force-unlock in runbooks/terraform-stuck-lock.md, and the deploy job is serialized on a per-environment state concurrency group. Evidence: .github/workflows/deploy-aws-lambda.yml:300-312 and :221-223 (#1812). Residual axes checked: rollback.yml now shares the aws-tfstate- group (:185); destroy-fargate-dev.yml's unconditional lock delete was also removed (:117-120); deploy-aws-fargate.yml keeps the operator-gated clear_stale_lock input only. Closing as completed; reopen if the behaviour recurs.