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:
-
:111 builds the URI Terraform deploys from the AWS_ACCOUNT_ID repository variable:
IMAGE_URI="${AWS_ACCOUNT_ID}.dkr.ecr.${AWS_REGION}.amazonaws.com/${ECR_REPOSITORY}:${IMAGE_TAG}"
-
:210-214 verifies 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.
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.ymlderives 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):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-imagejob 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:
(requires threading
AWS_ACCOUNT_IDinto the verify step'senv:, which currently carries onlyIMAGE_TAG,ECR_REPOSITORYandAWS_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:
(b) is the stronger option because it also catches the case where
AWS_ACCOUNT_IDis stale relative toAWS_ROLE_TO_ASSUMEfor the deploy, not just the verify. (a) only makes the verify self-consistent.Applies to both
rollback-aws-lambdaandrollback-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.