Skip to content

fix(cli): bootstrap FilePublishingRoleDefaultPolicy KMS permission when FileAssetsBucketKmsKeyId is an ARN - #1880

Open
lemon0333 wants to merge 1 commit into
aws:mainfrom
lemon0333:fix/bootstrap-kms-key-arn
Open

fix(cli): bootstrap FilePublishingRoleDefaultPolicy KMS permission when FileAssetsBucketKmsKeyId is an ARN#1880
lemon0333 wants to merge 1 commit into
aws:mainfrom
lemon0333:fix/bootstrap-kms-key-arn

Conversation

@lemon0333

Copy link
Copy Markdown
Contributor

Fixes #921

Description

FileAssetsBucketKmsKeyId's parameter description explicitly allows either a KMS key ID or a full key ARN. However, the bootstrap template unconditionally built the KMS resource ARN for the FilePublishingRoleDefaultPolicy as:

arn:${AWS::Partition}:kms:${AWS::Region}:${AWS::AccountId}:key/${FileAssetsBucketKmsKeyId}

When a (cross-account) key ARN is passed as the parameter value, this produces a malformed nested ARN — arn:...:key/arn:aws:kms:... — and therefore an invalid IAM policy statement, so the file publishing role can't use the key.

This adds a CFN HasCustomKmsKeyArn condition that detects the ARN form by checking whether the first :-delimited token of the value is arn (Fn::Equals + Fn::Select + Fn::Split). When the value is already an ARN it is used verbatim as the resource; a bare key ID keeps the existing arn:...:key/${...} construction. The CreateNewKey and AWS_MANAGED_KEY paths are unchanged.

Extended bootstrap-template.test.ts to assert the new condition and the nested Fn::If on the KMS statement resource. Bootstrap template/api tests pass (58 tests).

Checklist

  • This change contains a major version upgrade for a dependency and I confirm all breaking changes are addressed
    • Release notes for the new version:

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

…AssetsBucketKmsKeyId is an ARN

The bootstrap template always constructed the KMS resource ARN for the
FilePublishingRoleDefaultPolicy as
'arn:${Partition}:kms:${Region}:${AccountId}:key/${FileAssetsBucketKmsKeyId}'.
The FileAssetsBucketKmsKeyId parameter description explicitly permits either a
key ID or a full key ARN, so when a (cross-account) key ARN is supplied this
produced a malformed 'arn:...:key/arn:aws:kms:...' value and an invalid IAM
policy statement.

Add a HasCustomKmsKeyArn condition that detects the ARN form (first
':'-delimited token equals 'arn') and, in that case, uses the value directly
as the resource; bare key IDs keep the existing 'key/${...}' construction. The
create-new-key and AWS_MANAGED_KEY paths are unaffected.
@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.13%. Comparing base (7ff50e7) to head (627b719).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1880      +/-   ##
==========================================
+ Coverage   91.10%   91.13%   +0.03%     
==========================================
  Files          80       80              
  Lines       12205    12205              
  Branches     1742     1744       +2     
==========================================
+ Hits        11119    11123       +4     
+ Misses       1050     1046       -4     
  Partials       36       36              
Flag Coverage Δ
suite.unit 91.13% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(bootstrap) FilePublishingRoleDefaultPolicy permission incorrect when FileAssetsBucketKmsKeyId is an ARN value

2 participants