From 427bf630cb2e28ec98477b313eef32d5b9b91525 Mon Sep 17 00:00:00 2001 From: Rico Hermans Date: Thu, 24 Oct 2024 11:44:21 +0200 Subject: [PATCH] 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* --- packages/aws-cdk/lib/api/util/checks.ts | 14 ++++++++++---- packages/aws-cdk/test/api/util/checks.test.ts | 9 +++++++++ 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/aws-cdk/lib/api/util/checks.ts b/packages/aws-cdk/lib/api/util/checks.ts index 11c1c856eb384..a094156ba74fb 100644 --- a/packages/aws-cdk/lib/api/util/checks.ts +++ b/packages/aws-cdk/lib/api/util/checks.ts @@ -18,13 +18,19 @@ export async function determineAllowCrossAccountAssetPublishing(sdk: ISDK, custo return true; } - // other scenarios are highly irregular and potentially dangerous so we prevent it by - // instructing cdk-assets to detect foreign bucket ownership and reject. + // If there is a staging bucket AND the bootstrap version is old, then we want to protect + // against accidental cross-account publishing. return false; } catch (e) { + // You would think we would need to fail closed here, but the reality is + // that we get here if we couldn't find the bootstrap stack: that is + // completely valid, and many large organizations may have their own method + // of creating bootstrap resources. If they do, there's nothing for us to validate, + // but we can't use that as a reason to disallow cross-account publishing. We'll just + // have to trust they did their due diligence. So we fail open. debug(`Error determining cross account asset publishing: ${e}`); - debug('Defaulting to disallowing cross account asset publishing'); - return false; + debug('Defaulting to allowing cross account asset publishing'); + return true; } } diff --git a/packages/aws-cdk/test/api/util/checks.test.ts b/packages/aws-cdk/test/api/util/checks.test.ts index 2db7e3d1603ea..660577a8f6aec 100644 --- a/packages/aws-cdk/test/api/util/checks.test.ts +++ b/packages/aws-cdk/test/api/util/checks.test.ts @@ -50,6 +50,15 @@ describe('determineAllowCrossAccountAssetPublishing', () => { expect(result).toBe(true); }); + it('should return true if looking up the bootstrap stack fails', async () => { + AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { + callback(new Error('Could not read bootstrap stack')); + }); + + const result = await determineAllowCrossAccountAssetPublishing(mockSDK); + expect(result).toBe(true); + }); + it('should return false for other scenarios', async () => { AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => { callback(null, {