Skip to content

Commit 8d06824

Browse files
authored
fix(cli): deploy-role is not authorized to perform DescribeStackResources (#31878)
The cross-account asset uploading detection check required that the `deploy-role` could call `DescribeStackResources` -- which it can't. Instead, rely on parsing the `Outputs` of `DescribeStacks`. This is equivalent for the built-in stack, and relies on stack customizers to have removed the Output or put a dummy value there that does not look like a stack name (like `''`, `'-'` or `'*'`). It's not *as* good, but probably good enough. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 427bf63 commit 8d06824

File tree

2 files changed

+43
-24
lines changed

2 files changed

+43
-24
lines changed

packages/aws-cdk/lib/api/util/checks.ts

+10-9
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,16 @@ export async function getBootstrapStackInfo(sdk: ISDK, stackName: string): Promi
6060
throw new Error(`Invalid BootstrapVersion value: ${versionOutput.OutputValue}`);
6161
}
6262

63-
// try to get bucketname from the logical resource id
64-
let bucketName: string | undefined;
65-
const resourcesResponse = await cfn.describeStackResources({ StackName: stackName }).promise();
66-
const bucketResource = resourcesResponse.StackResources?.find(resource =>
67-
resource.ResourceType === 'AWS::S3::Bucket',
68-
);
69-
bucketName = bucketResource?.PhysicalResourceId;
70-
71-
let hasStagingBucket = !!bucketName;
63+
// try to get bucketname from the logical resource id. If there is no
64+
// bucketname, or the value doesn't look like an S3 bucket name, we assume
65+
// the bucket doesn't exist (this is for the case where a template customizer did
66+
// not dare to remove the Output, but put a dummy value there like '' or '-' or '***').
67+
//
68+
// We would have preferred to look at the stack resources here, but
69+
// unfortunately the deploy role doesn't have permissions call DescribeStackResources.
70+
const bucketName = stack.Outputs?.find(output => output.OutputKey === 'BucketName')?.OutputValue;
71+
// Must begin and end with letter or number.
72+
const hasStagingBucket = !!(bucketName && bucketName.match(/^[a-z0-9]/) && bucketName.match(/[a-z0-9]$/));
7273

7374
return {
7475
hasStagingBucket,

packages/aws-cdk/test/api/util/checks.test.ts

+33-15
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,20 @@ describe('determineAllowCrossAccountAssetPublishing', () => {
2525
});
2626
});
2727

28-
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
29-
callback(null, { StackResources: [] });
28+
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
29+
expect(result).toBe(true);
30+
});
31+
32+
it.each(['', '-', '*', '---'])('should return true when the bucket output does not look like a real bucket', async (notABucketName) => {
33+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
34+
callback(null, {
35+
Stacks: [{
36+
Outputs: [
37+
{ OutputKey: 'BootstrapVersion', OutputValue: '1' },
38+
{ OutputKey: 'BucketName', OutputValue: notABucketName },
39+
],
40+
}],
41+
});
3042
});
3143

3244
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
@@ -37,13 +49,21 @@ describe('determineAllowCrossAccountAssetPublishing', () => {
3749
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
3850
callback(null, {
3951
Stacks: [{
40-
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '21' }],
52+
Outputs: [
53+
{ OutputKey: 'BootstrapVersion', OutputValue: '21' },
54+
{ OutputKey: 'BucketName', OutputValue: 'some-bucket' },
55+
],
4156
}],
4257
});
4358
});
4459

45-
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
46-
callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] });
60+
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
61+
expect(result).toBe(true);
62+
});
63+
64+
it('should return true if looking up the bootstrap stack fails', async () => {
65+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
66+
callback(new Error('Could not read bootstrap stack'));
4767
});
4868

4969
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
@@ -63,15 +83,14 @@ describe('determineAllowCrossAccountAssetPublishing', () => {
6383
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
6484
callback(null, {
6585
Stacks: [{
66-
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '20' }],
86+
Outputs: [
87+
{ OutputKey: 'BootstrapVersion', OutputValue: '20' },
88+
{ OutputKey: 'BucketName', OutputValue: 'some-bucket' },
89+
],
6790
}],
6891
});
6992
});
7093

71-
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
72-
callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] });
73-
});
74-
7594
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
7695
expect(result).toBe(false);
7796
});
@@ -94,15 +113,14 @@ describe('getBootstrapStackInfo', () => {
94113
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
95114
callback(null, {
96115
Stacks: [{
97-
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '21' }],
116+
Outputs: [
117+
{ OutputKey: 'BootstrapVersion', OutputValue: '21' },
118+
{ OutputKey: 'BucketName', OutputValue: 'some-bucket' },
119+
],
98120
}],
99121
});
100122
});
101123

102-
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
103-
callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] });
104-
});
105-
106124
const result = await getBootstrapStackInfo(mockSDK, 'CDKToolkit');
107125
expect(result).toEqual({
108126
hasStagingBucket: true,

0 commit comments

Comments
 (0)