Skip to content

Commit 5860098

Browse files
committed
feat(iam): add feature flag to validate PolicyStatement SID is alphanumeric
- Add IAM_POLICY_STATEMENT_VALIDATE_SID feature flag - Validate SIDs are alphanumeric (A-Z, a-z, 0-9) when flag enabled - Fix invalid SIDs in aws-ecs cluster.ts - Add comprehensive test coverage for SID validation Closes #34819
1 parent cf11a81 commit 5860098

File tree

6 files changed

+135
-2
lines changed

6 files changed

+135
-2
lines changed

CHANGELOG.md

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@
22

33
All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
44

5+
## Unreleased
6+
7+
### Features
8+
9+
* **iam:** add feature flag to validate PolicyStatement SID is alphanumeric ([#34819](https://github.com/aws/aws-cdk/issues/34819))
10+
- New feature flag `@aws-cdk/aws-iam:policyStatementValidateSid` enforces IAM SID validation
11+
- SIDs must be alphanumeric (A-Z, a-z, 0-9) per AWS IAM requirements
12+
- Validation occurs at synthesis time when flag is enabled
13+
- Fixed invalid SIDs in aws-ecs cluster.ts to be alphanumeric
14+
515
## [1.158.0](https://github.com/aws/aws-cdk/compare/v1.157.0...v1.158.0) (2022-05-27)
616

717

packages/aws-cdk-lib/aws-ecs/lib/cluster.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -392,14 +392,14 @@ export class Cluster extends Resource implements ICluster {
392392
};
393393

394394
key.addToResourcePolicy(new PolicyStatement({
395-
sid: 'Allow generate data key access for Fargate tasks.',
395+
sid: 'AllowGenerateDataKeyAccessForFargateTasks',
396396
principals: [new ServicePrincipal('fargate.amazonaws.com')],
397397
resources: ['*'],
398398
actions: ['kms:GenerateDataKeyWithoutPlaintext'],
399399
conditions: clusterConditions,
400400
}));
401401
key.addToResourcePolicy(new PolicyStatement({
402-
sid: 'Allow grant creation permission for Fargate tasks.',
402+
sid: 'AllowGrantCreationPermissionForFargateTasks',
403403
principals: [new ServicePrincipal('fargate.amazonaws.com')],
404404
resources: ['*'],
405405
actions: ['kms:CreateGrant'],

packages/aws-cdk-lib/aws-iam/lib/policy-document.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,13 @@ export class PolicyDocument implements cdk.IResolvable {
7878
this.freezeStatements();
7979
this._maybeMergeStatements(context.scope);
8080

81+
// Validate statement SIDs if feature flag is enabled
82+
if (cdk.FeatureFlags.of(context.scope).isEnabled(cxapi.IAM_POLICY_STATEMENT_VALIDATE_SID)) {
83+
for (const statement of this.statements) {
84+
statement._validateSid();
85+
}
86+
}
87+
8188
// In the previous implementation of 'merge', sorting of actions/resources on
8289
// a statement always happened, even on singular statements. In the new
8390
// implementation of 'merge', sorting only happens when actually combining 2

packages/aws-cdk-lib/aws-iam/lib/policy-statement.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -232,6 +232,16 @@ export class PolicyStatement {
232232
}
233233
}
234234

235+
/**
236+
* Validate that the SID is alphanumeric
237+
* @internal
238+
*/
239+
public _validateSid() {
240+
if (this._sid !== undefined && !cdk.Token.isUnresolved(this._sid) && !/^[0-9A-Za-z]*$/.test(this._sid)) {
241+
throw new UnscopedValidationError(`Statement ID (sid) must be alphanumeric. Got '${this._sid}'. The Sid element supports ASCII uppercase letters (A-Z), lowercase letters (a-z), and numbers (0-9).`);
242+
}
243+
}
244+
235245
private validatePolicyActions(actions: string[]) {
236246
// In case of an unresolved list of actions return early
237247
if (cdk.Token.isUnresolved(actions)) return;

packages/aws-cdk-lib/aws-iam/test/policy-statement.test.ts

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1+
import * as cdk from '../../core';
12
import { Stack } from '../../core';
3+
import * as cxapi from '../../cx-api';
24
import { AnyPrincipal, Group, PolicyDocument, PolicyStatement, Effect } from '../lib';
35

46
describe('IAM policy statement', () => {
@@ -260,4 +262,93 @@ describe('IAM policy statement', () => {
260262
expect(mod).toThrow(/can no longer be modified/);
261263
}
262264
});
265+
266+
describe('SID validation', () => {
267+
test('does not validate when feature flag is disabled', () => {
268+
const app = new cdk.App();
269+
const stack = new Stack(app, 'Stack');
270+
const doc = new PolicyDocument();
271+
doc.addStatements(new PolicyStatement({
272+
sid: 'Invalid-SID-With-Dashes',
273+
actions: ['s3:GetObject'],
274+
resources: ['*'],
275+
}));
276+
277+
expect(() => stack.resolve(doc)).not.toThrow();
278+
});
279+
280+
test('validates alphanumeric SID when feature flag is enabled', () => {
281+
const app = new cdk.App({
282+
context: { [cxapi.IAM_POLICY_STATEMENT_VALIDATE_SID]: true },
283+
});
284+
const stack = new Stack(app, 'Stack');
285+
const doc = new PolicyDocument();
286+
doc.addStatements(new PolicyStatement({
287+
sid: 'ValidSID123',
288+
actions: ['s3:GetObject'],
289+
resources: ['*'],
290+
}));
291+
292+
expect(() => stack.resolve(doc)).not.toThrow();
293+
});
294+
295+
test('throws error for invalid SID when feature flag is enabled', () => {
296+
const app = new cdk.App({
297+
context: { [cxapi.IAM_POLICY_STATEMENT_VALIDATE_SID]: true },
298+
});
299+
const stack = new Stack(app, 'Stack');
300+
const doc = new PolicyDocument();
301+
doc.addStatements(new PolicyStatement({
302+
sid: 'Invalid-SID',
303+
actions: ['s3:GetObject'],
304+
resources: ['*'],
305+
}));
306+
307+
expect(() => stack.resolve(doc)).toThrow(/Statement ID \(sid\) must be alphanumeric/);
308+
});
309+
310+
test('allows empty SID', () => {
311+
const app = new cdk.App({
312+
context: { [cxapi.IAM_POLICY_STATEMENT_VALIDATE_SID]: true },
313+
});
314+
const stack = new Stack(app, 'Stack');
315+
const doc = new PolicyDocument();
316+
doc.addStatements(new PolicyStatement({
317+
sid: '',
318+
actions: ['s3:GetObject'],
319+
resources: ['*'],
320+
}));
321+
322+
expect(() => stack.resolve(doc)).not.toThrow();
323+
});
324+
325+
test('allows undefined SID', () => {
326+
const app = new cdk.App({
327+
context: { [cxapi.IAM_POLICY_STATEMENT_VALIDATE_SID]: true },
328+
});
329+
const stack = new Stack(app, 'Stack');
330+
const doc = new PolicyDocument();
331+
doc.addStatements(new PolicyStatement({
332+
actions: ['s3:GetObject'],
333+
resources: ['*'],
334+
}));
335+
336+
expect(() => stack.resolve(doc)).not.toThrow();
337+
});
338+
339+
test('allows tokens in SID', () => {
340+
const app = new cdk.App({
341+
context: { [cxapi.IAM_POLICY_STATEMENT_VALIDATE_SID]: true },
342+
});
343+
const stack = new Stack(app, 'Stack');
344+
const doc = new PolicyDocument();
345+
doc.addStatements(new PolicyStatement({
346+
sid: cdk.Fn.ref('SomeParameter'),
347+
actions: ['s3:GetObject'],
348+
resources: ['*'],
349+
}));
350+
351+
expect(() => stack.resolve(doc)).not.toThrow();
352+
});
353+
});
263354
});

packages/aws-cdk-lib/cx-api/lib/features.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ export const Enable_IMDS_Blocking_Deprecated_Feature = '@aws-cdk/aws-ecs:enableI
132132
export const Disable_ECS_IMDS_Blocking = '@aws-cdk/aws-ecs:disableEcsImdsBlocking';
133133
export const ALB_DUALSTACK_WITHOUT_PUBLIC_IPV4_SECURITY_GROUP_RULES_DEFAULT = '@aws-cdk/aws-elasticloadbalancingV2:albDualstackWithoutPublicIpv4SecurityGroupRulesDefault';
134134
export const IAM_OIDC_REJECT_UNAUTHORIZED_CONNECTIONS = '@aws-cdk/aws-iam:oidcRejectUnauthorizedConnections';
135+
export const IAM_POLICY_STATEMENT_VALIDATE_SID = '@aws-cdk/aws-iam:policyStatementValidateSid';
135136
export const ENABLE_ADDITIONAL_METADATA_COLLECTION = '@aws-cdk/core:enableAdditionalMetadataCollection';
136137
export const LAMBDA_CREATE_NEW_POLICIES_WITH_ADDTOROLEPOLICY = '@aws-cdk/aws-lambda:createNewPoliciesWithAddToRolePolicy';
137138
export const SET_UNIQUE_REPLICATION_ROLE_NAME = '@aws-cdk/aws-s3:setUniqueReplicationRoleName';
@@ -1469,6 +1470,20 @@ export const FLAGS: Record<string, FlagInfo> = {
14691470
compatibilityWithOldBehaviorMd: 'Disable the feature flag to allow unsecure OIDC connection.',
14701471
},
14711472

1473+
//////////////////////////////////////////////////////////////////////
1474+
[IAM_POLICY_STATEMENT_VALIDATE_SID]: {
1475+
type: FlagType.BugFix,
1476+
summary: 'Validate PolicyStatement SID is alphanumeric',
1477+
detailsMd: `
1478+
When enabled, PolicyStatement SID validation enforces alphanumeric characters only (A-Z, a-z, 0-9)
1479+
per IAM requirements. Invalid SIDs will throw an error at synthesis time.
1480+
1481+
When disabled, no SID validation occurs, maintaining backward compatibility with existing code
1482+
that may use invalid SID characters.`,
1483+
introducedIn: { v2: 'V2NEXT' },
1484+
recommendedValue: true,
1485+
},
1486+
14721487
//////////////////////////////////////////////////////////////////////
14731488
[ENABLE_ADDITIONAL_METADATA_COLLECTION]: {
14741489
type: FlagType.VisibleContext,

0 commit comments

Comments
 (0)