Skip to content

fix(secretsmanager): add fallback grants for cross-account secrets#35478

Open
puretension wants to merge 5 commits intoaws:mainfrom
puretension:fix/secretsmanager-cross-account-grants-35476
Open

fix(secretsmanager): add fallback grants for cross-account secrets#35478
puretension wants to merge 5 commits intoaws:mainfrom
puretension:fix/secretsmanager-cross-account-grants-35476

Conversation

@puretension
Copy link
Copy Markdown
Contributor

Issue # (if applicable)

Closes #35476

Reason for this change

When using Secret.fromSecretAttributes for cross-account registry credentials in ECS, the standard grantRead mechanism fails to attach policies to the task execution role, causing authentication failures when pulling container images.

Description of changes

Added fallback logic in Secret.grantRead() and Secret.grantWrite() methods:
• When autoCreatePolicy is false and no principal statement is created, use Grant.addToPrincipal() as fallback
• Ensures cross-account secrets can grant permissions to ECS task execution roles
• Maintains backward compatibility - only activates for imported secrets when standard grants fail

Describe any new or updated permissions being added

No new permissions added. The fallback uses the same IAM actions as the standard grant:
Read: secretsmanager:GetSecretValue, secretsmanager:DescribeSecret
Write: secretsmanager:PutSecretValue, secretsmanager:UpdateSecret,
secretsmanager:UpdateSecretVersionStage

Description of how you validated changes

Unit Tests:
• Added cross-account-grants.test.ts with tests for grantRead() and grantWrite()
• Tests verify managed policy creation for cross-account scenarios
• All tests pass: yarn jest --testPathPattern aws-secretsmanager/test/cross-account-grants.test.ts

Verification:
• Fallback triggers when autoCreatePolicy is false and !result.principalStatement
• Generated CloudFormation includes correct AWS::IAM::Policy resources
• Cross-account secrets now properly grant permissions to ECS task execution roles

Checklist


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
…t secrets

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
…secrets

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
@aws-cdk-automation aws-cdk-automation requested a review from a team September 12, 2025 20:12
@github-actions github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. p2 labels Sep 12, 2025
Copy link
Copy Markdown
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(This review is outdated)

…grants

Signed-off-by: puretension <rlrlfhtm5@gmail.com>
@aws-cdk-automation aws-cdk-automation dismissed their stale review September 12, 2025 20:24

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@puretension
Copy link
Copy Markdown
Contributor Author

puretension commented Sep 18, 2025

Hi team — just a gentle follow-up on this PR (#35478).
It’s up to date and passing all checks, and I’ve included motivation, tests, and the linked issue(#35476) for context.
As an active CDK user, I’m eager to get this fix in since it unblocks a real cross-account workflow.
Please let me know if there’s anything else I can provide or adjust to help move it forward.
Thanks so much!

@aws-cdk-automation aws-cdk-automation added the pr/needs-further-review PR requires additional review from our team specialists due to the scope or complexity of changes. label Sep 30, 2025
@gshpychka
Copy link
Copy Markdown
Contributor

gshpychka commented Mar 18, 2026

Hi team — just a gentle follow-up on this PR (#35478). It’s up to date and passing all checks, and I’ve included motivation, tests, and the linked issue(#35476) for context. As an active CDK user, I’m eager to get this fix in since it unblocks a real cross-account workflow. Please let me know if there’s anything else I can provide or adjust to help move it forward. Thanks so much!

From my analysis, this PR does nothing (i.e. the resulting template is the same as before). See my comment on the issue: #35476 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. p2 pr/needs-further-review PR requires additional review from our team specialists due to the scope or complexity of changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(ecs): registry credential grants are not enough

4 participants