Repository navigation
fix(ci): rollback verifies the image against the assumed role's registry, not AWS_ACCOUNT_ID's #142
Description
Found during the independent review of PR LeanerCloud/cloud-commitments-cli#1641. Filed rather than fixed there to keep that p0 to one concern — but note the fix is one line, so it is cheap to pick up.
What
.github/workflows/rollback.yml derives the image to deploy and the image it verifies from two different sources, which are only equal by convention:
-
:111builds the URI Terraform deploys from theAWS_ACCOUNT_IDrepository variable:IMAGE_URI="${AWS_ACCOUNT_ID}.dkr.ecr.${AWS_REGION}.amazonaws.com/${ECR_REPOSITORY}:${IMAGE_TAG}" -
:210-214verifies existence with no--registry-id, so the call resolves against the account of the assumed role (vars.AWS_ROLE_TO_ASSUME,:193/:281):aws ecr describe-images \ --repository-name "$ECR_REPOSITORY" \ --image-ids "imageTag=$IMAGE_TAG" \ --region "$AWS_REGION"
If those two admin-set values ever point at different accounts — a cross-account setup, a stale variable after an account migration, a copy-paste into the wrong environment — the workflow cheerfully reports "✅ Image exists in ECR" having checked a registry that is not the one Terraform then pulls from. The rollback proceeds and fails later at deploy, or worse, succeeds against an unintended image.
This is a misconfiguration hazard rather than an attack path: both values are repository variables only an admin can set. But it is a check that silently asserts the wrong thing, which is the failure mode the "no silent fallbacks" rule exists to prevent — a verification step that can pass while the thing it claims to verify is false is worse than no verification.
Shape is pre-existing (the deleted verify-image job had the same divergence); PR LeanerCloud/cloud-commitments-cli#1641 inlined it into the gated rollback jobs without changing it.
Fix direction
Either:
(a) Pin the lookup to the same account the URI was built from — one line, preferred:
aws ecr describe-images \
--registry-id "$AWS_ACCOUNT_ID" \
--repository-name "$ECR_REPOSITORY" \
...(requires threading AWS_ACCOUNT_ID into the verify step's env:, which currently carries only IMAGE_TAG, ECR_REPOSITORY and AWS_REGION.)
(b) Assert they match, and fail loud if not — derive the authenticated account and compare, so a divergence is reported rather than papered over:
ACTUAL=$(aws sts get-caller-identity --query Account --output text)
if [ "$ACTUAL" != "$AWS_ACCOUNT_ID" ]; then
echo "::error::AWS_ACCOUNT_ID ($AWS_ACCOUNT_ID) does not match the assumed role's account ($ACTUAL)"
exit 1
fi(b) is the stronger option because it also catches the case where AWS_ACCOUNT_ID is stale relative to AWS_ROLE_TO_ASSUME for the deploy, not just the verify. (a) only makes the verify self-consistent.
Applies to both rollback-aws-lambda and rollback-aws-fargate, which carry identical copies of the verify step.
Related: LeanerCloud/cloud-commitments-cli#1542 / PR LeanerCloud/cloud-commitments-cli#1641, LeanerCloud/cloud-commitments-cli#1648.
Activity