sec(ci): lambda deploy has same unsafe failure-path state-lock delete as #438

[cristim]

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.

[cristim]

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.

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() then aws 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 key github-<environment>/terraform.tfstate
• .github/workflows/rollback.yml:230 - writes the same state key, with no concurrency: group at all
• .github/workflows/deploy-aws-fargate.yml:160-167 - the comment that spells out why this construct is forbidden

The 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:

• a workflow_dispatch of environment=dev from a feature branch, and
• the push-to-main dev 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 apply holding the lock. Run B starts in a different concurrency group, fails on lock acquisition, and its if: 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:230 writes 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-167 states the rule verbatim:

a blanket failure-path delete is itself a state-corruption risk: a run that fails because it could not acquire the lock would delete the lock held by another run that is still actively applying.

Suggested severity

Recommend raising to severity/critical and keeping priority/p1. The blast radius is the production account holding the money-path Lambda, and the fix (delete the step, adopt the fargate model) is effort/xs as already labelled.

Fix direction

As already described here: delete the step and adopt the fargate model (operator-gated clear_stale_lock input plus runbooks/terraform-stuck-lock.md). Additionally re-key the concurrency group on the state key (environment plus cloud) rather than github.ref, and add a group to rollback.yml, otherwise the same overlap remains possible with the failure-path delete removed. The concurrency gap across rollback.yml, database-migration.yml, cleanup-staging.yml and destroy-fargate-dev.yml is filed separately from this review; destroy-fargate-dev.yml:63-66 has an unconditional variant of the same lock delete and is filed separately too.

[cristim]

Verified resolved at 3c0f8ac: the automatic failure/cancel-path aws s3 rm of 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.