Skip to content

Commit edd031d

Browse files
iliapoloiliapolo
authored
feat: disallow cross account asset publishing in some scenarios (#31623)
### Issue # (if applicable) Closes #<issue number here>. ### Reason for this change ### Description of changes ### Description of how you validated changes ### Checklist - [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 8067294 commit edd031d

9 files changed

Lines changed: 259 additions & 34 deletions

File tree

‎packages/aws-cdk/lib/api/deploy-stack.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
import { StackActivityMonitor, StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
1818
import { TemplateBodyParameter, makeBodyParameter } from './util/template-body-parameter';
1919
import { AssetManifestBuilder } from '../util/asset-manifest-builder';
20+
import { determineAllowCrossAccountAssetPublishing } from './util/checks';
2021
import { publishAssets } from '../util/asset-publishing';
2122

2223
export interface DeployStackResult {
@@ -287,8 +288,15 @@ export async function deployStack(options: DeployStackOptions): Promise<DeploySt
287288
options.envResources,
288289
options.sdk,
289290
options.overrideTemplate);
291+
let bootstrapStackName: string | undefined;
292+
try {
293+
bootstrapStackName = (await options.envResources.lookupToolkit()).stackName;
294+
} catch (e) {
295+
debug(`Could not determine the bootstrap stack name: ${e}`);
296+
}
290297
await publishAssets(legacyAssets.toManifest(stackArtifact.assembly.directory), options.sdkProvider, stackEnv, {
291298
parallel: options.assetParallelism,
299+
allowCrossAccount: await determineAllowCrossAccountAssetPublishing(options.sdk, bootstrapStackName),
292300
});
293301

294302
if (hotswapMode !== HotswapMode.FULL_DEPLOYMENT) {

‎packages/aws-cdk/lib/api/deployments.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { deployStack, DeployStackResult, destroyStack, DeploymentMethod } from '
1212
import { EnvironmentResources, EnvironmentResourcesRegistry } from './environment-resources';
1313
import { HotswapMode } from './hotswap/common';
1414
import { loadCurrentTemplateWithNestedStacks, loadCurrentTemplate, RootTemplateWithNestedStacks } from './nested-stack-helpers';
15+
import { determineAllowCrossAccountAssetPublishing } from './util/checks';
1516
import { CloudFormationStack, Template, ResourcesToImport, ResourceIdentifierSummaries, stabilizeStack, uploadStackTemplateAssets } from './util/cloudformation';
1617
import { StackActivityMonitor, StackActivityProgress } from './util/cloudformation/stack-activity-monitor';
1718
import { StackEventPoller } from './util/cloudformation/stack-event-poller';
@@ -384,6 +385,7 @@ export class Deployments {
384385
private readonly publisherCache = new Map<AssetManifest, cdk_assets.AssetPublishing>();
385386
private readonly environmentResources: EnvironmentResourcesRegistry;
386387

388+
private _allowCrossAccountAssetPublishing: boolean | undefined;
387389
constructor(private readonly props: DeploymentsProps) {
388390
this.sdkProvider = props.sdkProvider;
389391
this.environmentResources = new EnvironmentResourcesRegistry(props.toolkitStackName);
@@ -808,7 +810,10 @@ export class Deployments {
808810
*/
809811
public async publishAssets(asset: cxapi.AssetManifestArtifact, options: PublishStackAssetsOptions) {
810812
const { manifest, stackEnv } = await this.prepareAndValidateAssets(asset, options);
811-
await publishAssets(manifest, this.sdkProvider, stackEnv, options.publishOptions);
813+
await publishAssets(manifest, this.sdkProvider, stackEnv, {
814+
...options.publishOptions,
815+
allowCrossAccount: await this.allowCrossAccountAssetPublishingForEnv(stackEnv),
816+
});
812817
}
813818

814819
/**
@@ -846,12 +851,21 @@ export class Deployments {
846851

847852
// No need to validate anymore, we already did that during build
848853
const publisher = this.cachedPublisher(assetManifest, stackEnv, options.stackName);
849-
await publisher.publishEntry(asset);
854+
// eslint-disable-next-line no-console
855+
await publisher.publishEntry(asset, { allowCrossAccount: await this.allowCrossAccountAssetPublishingForEnv(stackEnv) });
850856
if (publisher.hasFailures) {
851857
throw new Error(`Failed to publish asset ${asset.id}`);
852858
}
853859
}
854860

861+
private async allowCrossAccountAssetPublishingForEnv(env: cxapi.Environment): Promise<boolean> {
862+
if (this._allowCrossAccountAssetPublishing === undefined) {
863+
const sdk = (await this.cachedSdkForEnvironment(env, Mode.ForReading)).sdk;
864+
this._allowCrossAccountAssetPublishing = await determineAllowCrossAccountAssetPublishing(sdk, this.props.toolkitStackName);
865+
}
866+
return this._allowCrossAccountAssetPublishing;
867+
}
868+
855869
/**
856870
* Return whether a single asset has been published already
857871
*/

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

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { debug } from '../../logging';
2+
import { ISDK } from '../aws-auth/sdk';
3+
4+
export async function determineAllowCrossAccountAssetPublishing(sdk: ISDK, customStackName?: string): Promise<boolean> {
5+
try {
6+
const stackName = customStackName || 'CDKToolkit';
7+
const stackInfo = await getBootstrapStackInfo(sdk, stackName);
8+
9+
if (!stackInfo.hasStagingBucket) {
10+
// indicates an intentional cross account setup
11+
return true;
12+
}
13+
14+
if (stackInfo.bootstrapVersion >= 21) {
15+
// bootstrap stack version 21 contains a fix that will prevent cross
16+
// account publishing on the IAM level
17+
// https://github.com/aws/aws-cdk/pull/30823
18+
return true;
19+
}
20+
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.
23+
return false;
24+
} catch (e) {
25+
debug(`Error determining cross account asset publishing: ${e}`);
26+
debug('Defaulting to disallowing cross account asset publishing');
27+
return false;
28+
}
29+
}
30+
31+
interface BootstrapStackInfo {
32+
hasStagingBucket: boolean;
33+
bootstrapVersion: number;
34+
}
35+
36+
export async function getBootstrapStackInfo(sdk: ISDK, stackName: string): Promise<BootstrapStackInfo> {
37+
try {
38+
const cfn = sdk.cloudFormation();
39+
const stackResponse = await cfn.describeStacks({ StackName: stackName }).promise();
40+
41+
if (!stackResponse.Stacks || stackResponse.Stacks.length === 0) {
42+
throw new Error(`Toolkit stack ${stackName} not found`);
43+
}
44+
45+
const stack = stackResponse.Stacks[0];
46+
const versionOutput = stack.Outputs?.find(output => output.OutputKey === 'BootstrapVersion');
47+
48+
if (!versionOutput?.OutputValue) {
49+
throw new Error(`Unable to find BootstrapVersion output in the toolkit stack ${stackName}`);
50+
}
51+
52+
const bootstrapVersion = parseInt(versionOutput.OutputValue);
53+
if (isNaN(bootstrapVersion)) {
54+
throw new Error(`Invalid BootstrapVersion value: ${versionOutput.OutputValue}`);
55+
}
56+
57+
// try to get bucketname from the logical resource id
58+
let bucketName: string | undefined;
59+
const resourcesResponse = await cfn.describeStackResources({ StackName: stackName }).promise();
60+
const bucketResource = resourcesResponse.StackResources?.find(resource =>
61+
resource.ResourceType === 'AWS::S3::Bucket',
62+
);
63+
bucketName = bucketResource?.PhysicalResourceId;
64+
65+
let hasStagingBucket = !!bucketName;
66+
67+
return {
68+
hasStagingBucket,
69+
bootstrapVersion,
70+
};
71+
} catch (e) {
72+
throw new Error(`Error retrieving toolkit stack info: ${e}`);
73+
}
74+
}

‎packages/aws-cdk/lib/util/asset-publishing.ts‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ export interface PublishAssetsOptions {
2525
* @default true To remain backward compatible.
2626
*/
2727
readonly parallel?: boolean;
28+
29+
/**
30+
* Whether cdk-assets is allowed to do cross account publishing.
31+
*/
32+
readonly allowCrossAccount: boolean;
2833
}
2934

3035
/**
@@ -34,7 +39,7 @@ export async function publishAssets(
3439
manifest: cdk_assets.AssetManifest,
3540
sdk: SdkProvider,
3641
targetEnv: cxapi.Environment,
37-
options: PublishAssetsOptions = {},
42+
options: PublishAssetsOptions,
3843
) {
3944
// This shouldn't really happen (it's a programming error), but we don't have
4045
// the types here to guide us. Do an runtime validation to be super super sure.
@@ -56,7 +61,7 @@ export async function publishAssets(
5661
publishAssets: true,
5762
quiet: options.quiet,
5863
});
59-
await publisher.publish();
64+
await publisher.publish({ allowCrossAccount: options.allowCrossAccount });
6065
if (publisher.hasFailures) {
6166
throw new Error('Failed to publish one or more assets. See the error messages above for more information.');
6267
}

‎packages/aws-cdk/package.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@
104104
"archiver": "^5.3.2",
105105
"aws-sdk": "^2.1691.0",
106106
"camelcase": "^6.3.0",
107-
"cdk-assets": "^2.154.0",
107+
"cdk-assets": "^2.155.0",
108108
"cdk-from-cfn": "^0.162.0",
109109
"chalk": "^4",
110110
"chokidar": "^3.6.0",

‎packages/aws-cdk/test/api/bootstrap.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,9 @@ const env = {
1111
name: 'mock',
1212
};
1313

14+
jest.mock('../../lib/api/util/checks', () => ({
15+
determineAllowCrossAccountAssetPublishing: jest.fn().mockResolvedValue(true),
16+
}));
1417
let sdk: MockSdkProvider;
1518
let executed: boolean;
1619
let protectedTermination: boolean;

‎packages/aws-cdk/test/api/deploy-stack.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@ import { MockedObject, mockResolvedEnvironment, MockSdk, MockSdkProvider, SyncHa
88
import { NoBootstrapStackEnvironmentResources } from '../../lib/api/environment-resources';
99

1010
jest.mock('../../lib/api/hotswap-deployments');
11+
jest.mock('../../lib/api/util/checks', () => ({
12+
determineAllowCrossAccountAssetPublishing: jest.fn().mockResolvedValue(true),
13+
}));
1114

1215
const FAKE_STACK = testStack({
1316
stackName: 'withouterrors',

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

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
import * as AWS from 'aws-sdk';
2+
import * as AWSMock from 'aws-sdk-mock';
3+
import { ISDK } from '../../../lib/api/aws-auth';
4+
import { determineAllowCrossAccountAssetPublishing, getBootstrapStackInfo } from '../../../lib/api/util/checks';
5+
6+
describe('determineAllowCrossAccountAssetPublishing', () => {
7+
let mockSDK: ISDK;
8+
9+
beforeEach(() => {
10+
mockSDK = {
11+
cloudFormation: () => new AWS.CloudFormation(),
12+
} as ISDK;
13+
});
14+
15+
afterEach(() => {
16+
AWSMock.restore();
17+
});
18+
19+
it('should return true when hasStagingBucket is false', async () => {
20+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
21+
callback(null, {
22+
Stacks: [{
23+
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '1' }],
24+
}],
25+
});
26+
});
27+
28+
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
29+
callback(null, { StackResources: [] });
30+
});
31+
32+
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
33+
expect(result).toBe(true);
34+
});
35+
36+
it('should return true when bootstrap version is >= 21', async () => {
37+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
38+
callback(null, {
39+
Stacks: [{
40+
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '21' }],
41+
}],
42+
});
43+
});
44+
45+
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
46+
callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] });
47+
});
48+
49+
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
50+
expect(result).toBe(true);
51+
});
52+
53+
it('should return false for other scenarios', async () => {
54+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
55+
callback(null, {
56+
Stacks: [{
57+
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '20' }],
58+
}],
59+
});
60+
});
61+
62+
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
63+
callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] });
64+
});
65+
66+
const result = await determineAllowCrossAccountAssetPublishing(mockSDK);
67+
expect(result).toBe(false);
68+
});
69+
});
70+
71+
describe('getBootstrapStackInfo', () => {
72+
let mockSDK: ISDK;
73+
74+
beforeEach(() => {
75+
mockSDK = {
76+
cloudFormation: () => new AWS.CloudFormation(),
77+
} as ISDK;
78+
});
79+
80+
afterEach(() => {
81+
AWSMock.restore();
82+
});
83+
84+
it('should return correct BootstrapStackInfo', async () => {
85+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
86+
callback(null, {
87+
Stacks: [{
88+
Outputs: [{ OutputKey: 'BootstrapVersion', OutputValue: '21' }],
89+
}],
90+
});
91+
});
92+
93+
AWSMock.mock('CloudFormation', 'describeStackResources', (_params: any, callback: Function) => {
94+
callback(null, { StackResources: [{ ResourceType: 'AWS::S3::Bucket', PhysicalResourceId: 'some-bucket' }] });
95+
});
96+
97+
const result = await getBootstrapStackInfo(mockSDK, 'CDKToolkit');
98+
expect(result).toEqual({
99+
hasStagingBucket: true,
100+
bootstrapVersion: 21,
101+
});
102+
});
103+
104+
it('should throw error when stack is not found', async () => {
105+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
106+
callback(null, { Stacks: [] });
107+
});
108+
109+
await expect(getBootstrapStackInfo(mockSDK, 'CDKToolkit')).rejects.toThrow('Toolkit stack CDKToolkit not found');
110+
});
111+
112+
it('should throw error when BootstrapVersion output is missing', async () => {
113+
AWSMock.mock('CloudFormation', 'describeStacks', (_params: any, callback: Function) => {
114+
callback(null, {
115+
Stacks: [{
116+
Outputs: [],
117+
}],
118+
});
119+
});
120+
121+
await expect(getBootstrapStackInfo(mockSDK, 'CDKToolkit')).rejects.toThrow('Unable to find BootstrapVersion output in the toolkit stack CDKToolkit');
122+
});
123+
});

0 commit comments

Comments
 (0)