Skip to content

Commit 0e808d8

Browse files
authored
fix(secretsmanager): arnForPolicies evaluates to the partial ARN if accessed from a cross-env stack (#26308)
Currently, `Secret.secretFullArn` returns the partial ARN if the secret is referenced between cross-env stacks. An obvious practical implication is that `grant*` methods will produce an incorrect ARN for the IAM policies, since a full ARN is required for cross-environment access. This PR partially fixes the issue - I reimplemented `arnForPolicies` to be lazily evaluated. It checks if the value is being accessed across environments and adds `-??????` to the ARN if it is. Now, this does not solve the underlying issue of `secretFullArn` returning the partial ARN. While it should return undefined, we have to check how the prop is accessed (same environment or cross-env) before we know whether to return the ARN or `undefined`. If we use a `Lazy` here, it still cannot return `undefined` (only `any` as the closest thing). So I don't think the underlying cause could be solved currently, that's why I opted for this partial fix that would address the most practical consequence of the bug. This is a partial fix for #22468. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent 1d9d808 commit 0e808d8

File tree

3 files changed

+83
-3
lines changed

3 files changed

+83
-3
lines changed

packages/aws-cdk-lib/aws-iam/test/merge-statements.test.ts

+35-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { App, Stack } from '../../core';
1+
import { App, Lazy, Stack } from '../../core';
22
import * as iam from '../lib';
33
import { PolicyStatement } from '../lib';
44

@@ -490,6 +490,40 @@ test('lazily generated statements are merged correctly', () => {
490490
]);
491491
});
492492

493+
test('merge statements if resource is a lazy', () => {
494+
const stack = new Stack();
495+
const user1 = new iam.User(stack, 'User1');
496+
const user2 = new iam.User(stack, 'User2');
497+
const resourceToken = Lazy.string({
498+
produce: () => 'a',
499+
});
500+
501+
assertMerged([
502+
new iam.PolicyStatement({
503+
resources: [resourceToken],
504+
actions: ['service:Action'],
505+
principals: [user1],
506+
}),
507+
new iam.PolicyStatement({
508+
resources: [resourceToken],
509+
actions: ['service:Action'],
510+
principals: [user2],
511+
}),
512+
], [
513+
{
514+
Effect: 'Allow',
515+
Resource: 'a',
516+
Action: 'service:Action',
517+
Principal: {
518+
AWS: [
519+
{ 'Fn::GetAtt': ['User1E278A736', 'Arn'] },
520+
{ 'Fn::GetAtt': ['User21F1486D1', 'Arn'] },
521+
],
522+
},
523+
},
524+
]);
525+
});
526+
493527
function assertNoMerge(statements: iam.PolicyStatement[]) {
494528
const app = new App();
495529
const stack = new Stack(app, 'Stack');

packages/aws-cdk-lib/aws-secretsmanager/lib/secret.ts

+15-2
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { RotationSchedule, RotationScheduleOptions } from './rotation-schedule';
44
import * as secretsmanager from './secretsmanager.generated';
55
import * as iam from '../../aws-iam';
66
import * as kms from '../../aws-kms';
7-
import { ArnFormat, FeatureFlags, Fn, IResource, Lazy, RemovalPolicy, Resource, ResourceProps, SecretValue, Stack, Token, TokenComparison } from '../../core';
7+
import { ArnFormat, FeatureFlags, Fn, IResolveContext, IResource, Lazy, RemovalPolicy, Resource, ResourceProps, SecretValue, Stack, Token, TokenComparison } from '../../core';
88
import * as cxapi from '../../cx-api';
99

1010
const SECRET_SYMBOL = Symbol.for('@aws-cdk/secretsmanager.Secret');
@@ -340,9 +340,22 @@ abstract class SecretBase extends Resource implements ISecret {
340340
protected abstract readonly autoCreatePolicy: boolean;
341341

342342
private policy?: ResourcePolicy;
343+
private _arnForPolicies: string;
343344

344345
constructor(scope: Construct, id: string, props: ResourceProps = {}) {
345346
super(scope, id, props);
347+
this._arnForPolicies = Lazy.uncachedString({
348+
produce: (context: IResolveContext) => {
349+
const consumingStack = Stack.of(context.scope);
350+
if (this.stack.account !== consumingStack.account ||
351+
(this.stack.region !== consumingStack.region &&
352+
!consumingStack._crossRegionReferences) || !this.secretFullArn) {
353+
return `${this.secretArn}-??????`;
354+
} else {
355+
return this.secretFullArn;
356+
}
357+
},
358+
});
346359

347360
this.node.addValidation({ validate: () => this.policy?.document.validateForResourcePolicy() ?? [] });
348361
}
@@ -450,7 +463,7 @@ abstract class SecretBase extends Resource implements ISecret {
450463
* then we need to add a suffix to capture the full ARN's format.
451464
*/
452465
protected get arnForPolicies() {
453-
return this.secretFullArn ? this.secretFullArn : `${this.secretArn}-??????`;
466+
return this._arnForPolicies;
454467
}
455468

456469
/**

packages/aws-cdk-lib/aws-secretsmanager/test/secret.test.ts

+33
Original file line numberDiff line numberDiff line change
@@ -1357,3 +1357,36 @@ describe('secretObjectValue', () => {
13571357
});
13581358
});
13591359
});
1360+
1361+
test('cross-environment grant with direct object reference', () => {
1362+
// GIVEN
1363+
const producerStack = new cdk.Stack(app, 'ProducerStack', { env: { region: 'foo', account: '1111111111' } });
1364+
const consumerStack = new cdk.Stack(app, 'ConsumerStack', { env: { region: 'bar', account: '1111111111' } });
1365+
const secret = new secretsmanager.Secret(producerStack, 'Secret', { secretName: 'MySecret' });
1366+
const role = new iam.Role(consumerStack, 'Role', { assumedBy: new iam.AccountRootPrincipal() });
1367+
1368+
// WHEN
1369+
secret.grantRead(role);
1370+
1371+
// THEN
1372+
Template.fromStack(consumerStack).hasResourceProperties('AWS::IAM::Policy', {
1373+
PolicyDocument: {
1374+
Version: '2012-10-17',
1375+
Statement: [{
1376+
Action: [
1377+
'secretsmanager:GetSecretValue',
1378+
'secretsmanager:DescribeSecret',
1379+
],
1380+
Effect: 'Allow',
1381+
Resource: {
1382+
'Fn::Join': ['', [
1383+
'arn:',
1384+
{ Ref: 'AWS::Partition' },
1385+
':secretsmanager:foo:1111111111:secret:MySecret-??????',
1386+
]],
1387+
},
1388+
}],
1389+
},
1390+
});
1391+
1392+
});

0 commit comments

Comments
 (0)