Skip to content

Commit 427bf63

Browse files
authored
fix(cli): cross-account asset publishing doesn't work without bootstrap stack (#31876)
If the bootstrap stack can't be found, it can't be validated. We used to fail closed, but that just means that cross-account publishing is broken. Instead, we have to fail open. This is not the only protection mechanism we have, so the local check is more of a bonus. Fixes #31866. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent d884c7c commit 427bf63

File tree

2 files changed

+19
-4
lines changed

2 files changed

+19
-4
lines changed

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,13 +18,19 @@ export async function determineAllowCrossAccountAssetPublishing(sdk: ISDK, custo
1818
return true;
1919
}
2020

21-
// other scenarios are highly irregular and potentially dangerous so we prevent it by
22-
// instructing cdk-assets to detect foreign bucket ownership and reject.
21+
// If there is a staging bucket AND the bootstrap version is old, then we want to protect
22+
// against accidental cross-account publishing.
2323
return false;
2424
} catch (e) {
25+
// You would think we would need to fail closed here, but the reality is
26+
// that we get here if we couldn't find the bootstrap stack: that is
27+
// completely valid, and many large organizations may have their own method
28+
// of creating bootstrap resources. If they do, there's nothing for us to validate,
29+
// but we can't use that as a reason to disallow cross-account publishing. We'll just
30+
// have to trust they did their due diligence. So we fail open.
2531
debug(`Error determining cross account asset publishing: ${e}`);
26-
debug('Defaulting to disallowing cross account asset publishing');
27-
return false;
32+
debug('Defaulting to allowing cross account asset publishing');
33+
return true;
2834
}
2935
}
3036

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,15 @@ describe('determineAllowCrossAccountAssetPublishing', () => {
5050
expect(result).toBe(true);
5151
});
5252

53+
it('should return true if looking up the bootstrap stack fails', async () => {
54+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
55+
callback(new Error('Could not read bootstrap stack'));
56+
});
57+
58+
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
59+
expect(result).toBe(true);
60+
});
61+
5362
it('should return false for other scenarios', async () => {
5463
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
5564
callback(null, {

0 commit comments

Comments
 (0)