Skip to content

fix(secretsmanager): grant KMS decrypt/encrypt directly to grantee in grantRead/grantWrite#37243

Open
saurav61091 wants to merge 4 commits intoaws:mainfrom
saurav61091:fix/secretsmanager-grant-kms-decrypt
Open

fix(secretsmanager): grant KMS decrypt/encrypt directly to grantee in grantRead/grantWrite#37243
saurav61091 wants to merge 4 commits intoaws:mainfrom
saurav61091:fix/secretsmanager-grant-kms-decrypt

Conversation

@saurav61091
Copy link
Copy Markdown

Summary

When using Secret.fromSecretAttributes with an encryptionKey, calling secret.grantRead(role) only granted kms:Decrypt via ViaServicePrincipal but not directly to the grantee's IAM policy. This meant the IAM role lacked the kms:Decrypt permission needed to actually read the secret value.

Fix

Added this.encryptionKey.grantDecrypt(grantee) in grantRead and this.encryptionKey.grantEncrypt(grantee) in grantWrite, in addition to the existing ViaServicePrincipal grants. This mirrors how DynamoDB's grantReadData handles KMS keys.

Testing

Updated existing tests to verify the KMS policy statements are added to the grantee's IAM policy.

Fixes #20087


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

…ndition

When creating a CloudTrail Trail with an SNS topic from another account,
the deployment fails with InsufficientSnsTopicPolicyException. The
previous implementation used grantPublish which doesn't add the required
aws:SourceArn condition.

Replace grantPublish with an explicit addToResourcePolicy that includes
the aws:SourceArn condition, enabling cross-account CloudTrail-to-SNS
publishing while also improving security for same-account scenarios.

Fixes aws#11387
When using Secret.fromSecretAttributes with an encryptionKey, calling
grantRead only granted kms:Decrypt via the ViaServicePrincipal but
not directly to the grantee. This meant the grantee's IAM policy
lacked the kms:Decrypt permission needed to access the secret.

Add grantDecrypt/grantEncrypt directly to the grantee in addition to
the existing ViaServicePrincipal grants.

Fixes aws#20087
@github-actions github-actions bot added beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK bug This issue is a bug. effort/small Small work item – less than a day of effort p2 labels Mar 13, 2026
@aws-cdk-automation aws-cdk-automation requested a review from a team March 13, 2026 12:32
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)

Add integration test verifying that grantRead on a secret with a KMS
encryption key correctly adds kms:Decrypt to the grantee's IAM policy.
@aws-cdk-automation aws-cdk-automation dismissed their stale review March 13, 2026 12:47

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

@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 Mar 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 14, 2026

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
Please try merge from main to avoid findings unrelated to the PR.


TestsPassed ✅SkippedFailed
Security Guardian Results48 ran48 passed
TestResult
No test annotations available

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 14, 2026

⚠️ Experimental Feature: This security report is currently in experimental phase. Results may include false positives and the rules are being actively refined.
Please try merge from main to avoid findings unrelated to the PR.


TestsPassed ☑️SkippedFailed ❌️
Security Guardian Results with resolved templates48 ran47 passed1 failed
TestResult
Security Guardian Results with resolved templates
packages/@aws-cdk-testing/framework-integ/test/aws-secretsmanager/test/integ.secret-grant-read-kms.js.snapshot/cdk-integ-secret-grant-read-kms.template.json
iam-role-root-principal-needs-conditions.guard❌ failure

…S guard in integ test

The integ test for grantRead with KMS uses AccountRootPrincipal as the
assumed-by principal for a test role, which triggered the security guard
rule IAM_ROLE_ROOT_PRINCIPAL_NEEDS_CONDITIONS. Added the guard suppression
metadata to both the integ test source and the snapshot since the root
principal is intentional for this test-only role.
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. effort/small Small work item – less than a day of effort 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.

aws-secretsmanager: grantRead should add Decrypt Policy for kms key

2 participants