fix(bootstrap): disallow AssumeRole with ExternalId by default

packages/@aws-cdk-testing/cli-integ/lib/with-cdk-app.ts

@@ -312,6 +312,11 @@ export interface CdkModernBootstrapCommandOptions extends CommonCdkBootstrapComm
    */
   readonly customPermissionsBoundary?: string;
 
+  /**
+   * @default true
+   */
+  readonly denyExternalId?: boolean;
+
   /**
    * @default undefined
    */
@@ -509,6 +514,10 @@ export class TestFixture extends ShellHelper {
     } else if (options.examplePermissionsBoundary !== undefined) {
       args.push('--example-permissions-boundary');
     }
+    if (options.denyExternalId !== undefined) {
+      args.push(options.denyExternalId ? '--deny-external-id' : '--no-deny-external-id');
+    }
+
     if (options.usePreviousParameters === false) {
       args.push('--no-previous-parameters');
     }

packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/bootstrap/cdk-bootstrap-deploy-role-cannot-be-assumed-with-external-id.integtest.ts

@@ -0,0 +1,47 @@
+import { AssumeRoleCommand } from '@aws-sdk/client-sts';
+import { integTest, withoutBootstrap } from '../../../lib';
+
+jest.setTimeout(2 * 60 * 60_000); // Includes the time to acquire locks, worst-case single-threaded runtime
+
+integTest('deploy role cannot be assumed with external id', withoutBootstrap(async (fixture) => {
+  const bootstrapStackName = fixture.bootstrapStackName;
+
+  await fixture.cdkBootstrapModern({
+    toolkitStackName: bootstrapStackName,
+    cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess',
+  });
+
+  const account = await fixture.aws.account();
+  const deployRoleArn = `arn:aws:iam::${account}:role/cdk-${fixture.qualifier}-deploy-role-${account}-${fixture.aws.region}`;
+
+  // Attempt to assume the deploy role with an external ID should fail
+  await expect(
+    fixture.aws.sts.send(new AssumeRoleCommand({
+      RoleArn: deployRoleArn,
+      RoleSessionName: 'test-external-id-failure',
+      ExternalId: 'some-external-id',
+    })),
+  ).rejects.toThrow();
+}));
+
+integTest('deploy role can be assumed with ExternalId if protection is switched off', withoutBootstrap(async (fixture) => {
+  const bootstrapStackName = fixture.bootstrapStackName;
+
+  await fixture.cdkBootstrapModern({
+    toolkitStackName: bootstrapStackName,
+    cfnExecutionPolicy: 'arn:aws:iam::aws:policy/AdministratorAccess',
+    denyExternalId: false,
+  });
+
+  const account = await fixture.aws.account();
+  const deployRoleArn = `arn:aws:iam::${account}:role/cdk-${fixture.qualifier}-deploy-role-${account}-${fixture.aws.region}`;
+
+  // Attempt to assume the deploy role with an external ID should fail
+  await expect(
+    fixture.aws.sts.send(new AssumeRoleCommand({
+      RoleArn: deployRoleArn,
+      RoleSessionName: 'test-external-id-failure',
+      ExternalId: 'some-external-id',
+    })),
+  ).resolves.toBeTruthy();
+}));

packages/@aws-cdk/toolkit-lib/lib/api/bootstrap/bootstrap-environment.ts

@@ -213,27 +213,37 @@ export class Bootstrapper {
       }
     }
 
-    return current.update(
-      bootstrapTemplate,
-      {
-        FileAssetsBucketName: params.bucketName,
-        FileAssetsBucketKmsKeyId: kmsKeyId,
-        // Empty array becomes empty string
-        TrustedAccounts: trustedAccounts.join(','),
-        TrustedAccountsForLookup: trustedAccountsForLookup.join(','),
-        CloudFormationExecutionPolicies: cloudFormationExecutionPolicies.join(','),
-        Qualifier: params.qualifier,
-        PublicAccessBlockConfiguration:
-          params.publicAccessBlockConfiguration || params.publicAccessBlockConfiguration === undefined
-            ? 'true'
-            : 'false',
-        InputPermissionsBoundary: policyName,
-      },
-      {
-        ...options,
-        terminationProtection: options.terminationProtection ?? current.terminationProtection,
-      },
-    );
+    const bootstrapTemplateParameters: Record<string, string | undefined> = {
+      FileAssetsBucketName: params.bucketName,
+      FileAssetsBucketKmsKeyId: kmsKeyId,
+      // Empty array becomes empty string
+      TrustedAccounts: trustedAccounts.join(','),
+      TrustedAccountsForLookup: trustedAccountsForLookup.join(','),
+      CloudFormationExecutionPolicies: cloudFormationExecutionPolicies.join(','),
+      Qualifier: params.qualifier,
+      PublicAccessBlockConfiguration:
+        params.publicAccessBlockConfiguration || params.publicAccessBlockConfiguration === undefined
+          ? 'true'
+          : 'false',
+      InputPermissionsBoundary: policyName,
+    };
+
+    const templateParameters = await this.templateParameters();
+
+    // Conditionally set these parameters: only set these parameters if they are accepted by the template.
+    // If we pass them unconditionally, older customized templates that don't know about these
+    // parameters yet will fail to deploy.
+    if (params.denyExternalId !== undefined) {
+      if (!templateParameters.includes('DenyExternalId')) {
+        throw new ToolkitError('The selected bootstrap template does not accept the DenyExternalId parameter');
+      }
+      bootstrapTemplateParameters.DenyExternalId = `${params.denyExternalId}`;
+    }
+
+    return current.update(bootstrapTemplate, bootstrapTemplateParameters, {
+      ...options,
+      terminationProtection: options.terminationProtection ?? current.terminationProtection,
+    });
   }
 
   private async getPolicyName(
@@ -368,14 +378,23 @@ export class Bootstrapper {
     }
   }
 
-  private async loadTemplate(params: BootstrappingParameters = {}): Promise<any> {
+  /**
+   * Return the set of parameter names accepted by the current bootstrapping template
+   */
+  private async templateParameters(legacyParams: BootstrappingParameters = {}): Promise<string[]> {
+    const template = await this.loadTemplate(legacyParams);
+
+    return Object.keys(template.Parameters ?? {});
+  }
+
+  private async loadTemplate(legacyParams: BootstrappingParameters = {}): Promise<any> {
     switch (this.source.source) {
       case 'custom':
         return loadStructuredFile(this.source.templateFile);
       case 'default':
         return loadStructuredFile(path.join(bundledPackageRootDir(__dirname), 'lib', 'api', 'bootstrap', 'bootstrap-template.yaml'));
       case 'legacy':
-        return legacyBootstrapTemplate(params);
+        return legacyBootstrapTemplate(legacyParams);
     }
   }
 }

packages/@aws-cdk/toolkit-lib/lib/api/bootstrap/bootstrap-props.ts

@@ -146,4 +146,11 @@ export interface BootstrappingParameters {
    * @default - No value, optional argument
    */
   readonly customPermissionsBoundary?: string;
+
+  /**
+   * Whether to deny AssumeRole calls with an ExternalId
+   *
+   * @default - template default (true)
+   */
+  readonly denyExternalId?: boolean;
 }

packages/@aws-cdk/toolkit-lib/test/api/bootstrap/bootstrap2.test.ts

@@ -645,6 +645,86 @@ describe('Bootstrapping v2', () => {
     );
   });
 
+  describe('ExternalId protection', () => {
+    test('denyExternalId parameter is not present by default', async () => {
+      // GIVEN
+      const mockSdk = new MockSdkProvider();
+      (ToolkitInfo as any).lookup = jest.fn().mockResolvedValue(ToolkitInfo.fromStack(mockBootstrapStack({
+        Outputs: [
+          { OutputKey: 'BootstrapVersion', OutputValue: '1' },
+        ],
+      })));
+
+      // WHEN
+      await bootstrapper.bootstrapEnvironment(env, mockSdk, {
+        parameters: {},
+      });
+
+      // THEN
+      expect(mockDeployStack).toHaveBeenCalledWith(
+        expect.objectContaining({
+          parameters: expect.not.objectContaining({
+            DenyExternalId: 'true',
+          }),
+        }),
+        expect.anything(),
+      );
+    });
+
+    test.each([false, true])('denyExternalId parameter can be set to %p', async (param) => {
+      // GIVEN
+      const mockSdk2 = new MockSdkProvider();
+      (ToolkitInfo as any).lookup = jest.fn().mockResolvedValue(ToolkitInfo.fromStack(mockBootstrapStack({
+        Outputs: [
+          { OutputKey: 'BootstrapVersion', OutputValue: '1' },
+        ],
+      })));
+
+      // WHEN
+      await bootstrapper.bootstrapEnvironment(env, mockSdk2, {
+        parameters: {
+          denyExternalId: param,
+        },
+      });
+
+      // THEN
+      expect(mockDeployStack).toHaveBeenCalledWith(
+        expect.objectContaining({
+          parameters: expect.objectContaining({
+            DenyExternalId: `${param}`,
+          }),
+        }),
+        expect.anything(),
+      );
+    });
+
+    test('bootstrap template contains ExternalId conditions', async () => {
+      // GIVEN
+      const testBootstrapper = new Bootstrapper({ source: 'default' }, ioHelper);
+
+      // WHEN
+      const template = await (testBootstrapper as any).loadTemplate();
+
+      // THEN
+      expect(template.Parameters.DenyExternalId).toBeDefined();
+      expect(template.Parameters.DenyExternalId.Default).toBe('true');
+      expect(template.Conditions.ShouldDenyExternalId).toBeDefined();
+
+      // Check that roles have the ExternalId condition
+      const filePublishingRole = template.Resources.FilePublishingRole;
+      expect(filePublishingRole.Properties.AssumeRolePolicyDocument.Statement).toEqual(
+        expect.arrayContaining([
+          expect.objectContaining({
+            Action: 'sts:AssumeRole',
+            Condition: expect.objectContaining({
+              'Fn::If': expect.arrayContaining(['ShouldDenyExternalId']),
+            }),
+          }),
+        ]),
+      );
+    });
+  });
+
   describe('contains sts:TagSession on trusted accounts', () => {
     let template: Template;
 
@@ -659,7 +739,7 @@ describe('Bootstrapping v2', () => {
         'Fn::If': Match.arrayWith([
           conditionName,
           Match.objectLike({
-            Action: Match.arrayWith(['sts:AssumeRole', 'sts:TagSession']),
+            Action: Match.arrayWith(['sts:TagSession']),
           }),
         ]),
       });
@@ -680,23 +760,14 @@ describe('Bootstrapping v2', () => {
       template = Template.fromJSON(rawTemplate);
     });
 
-    test('in the FilePublishingRole', async () => {
-      template.hasResource('AWS::IAM::Role', {
-        Properties: {
-          RoleName: iamRoleName('file-publishing-role'),
-          AssumeRolePolicyDocument: {
-            Statement: Match.arrayWith([
-              statementWithCondition('HasTrustedAccounts'),
-            ]),
-          },
-        },
-      });
-    });
-
-    test('in the ImagePublishingRole', async () => {
+    test.each([
+      ['FilePublishingRole', 'file-publishing-role'],
+      ['ImagePublishingRole', 'image-publishing-role'],
+      ['DeploymentActionRole', 'deploy-role'],
+    ])('in the %p', async (_, roleName) => {
       template.hasResource('AWS::IAM::Role', {
         Properties: {
-          RoleName: iamRoleName('image-publishing-role'),
+          RoleName: iamRoleName(roleName),
           AssumeRolePolicyDocument: {
             Statement: Match.arrayWith([
               statementWithCondition('HasTrustedAccounts'),
@@ -719,18 +790,5 @@ describe('Bootstrapping v2', () => {
         },
       });
     });
-
-    test('in the DeploymentActionRole', async () => {
-      template.hasResource('AWS::IAM::Role', {
-        Properties: {
-          RoleName: iamRoleName('deploy-role'),
-          AssumeRolePolicyDocument: {
-            Statement: Match.arrayWith([
-              statementWithCondition('HasTrustedAccounts'),
-            ]),
-          },
-        },
-      });
-    });
   });
 });

packages/@aws-cdk/toolkit-lib/test/api/bootstrap/external-id.test.ts

@@ -0,0 +1,73 @@
+import { Bootstrapper } from '../../../lib/api/bootstrap';
+import type { IIoHost } from '../../../lib/api/io';
+import { asIoHelper } from '../../../lib/api/io/private';
+
+describe('ExternalId Protection Integration Test', () => {
+  let ioHost: IIoHost;
+  let ioHelper: any;
+
+  beforeEach(() => {
+    ioHost = {
+      notify: jest.fn(),
+      requestResponse: jest.fn(),
+    };
+    ioHelper = asIoHelper(ioHost, 'bootstrap');
+  });
+
+  test('bootstrap template denies AssumeRole with ExternalId by default', async () => {
+    // GIVEN
+    const bootstrapper = new Bootstrapper({ source: 'default' }, ioHelper);
+
+    // WHEN
+    const template = await (bootstrapper as any).loadTemplate();
+
+    // THEN
+    // Verify the parameter exists
+    expect(template.Parameters.DenyExternalId).toMatchObject({
+      Type: 'String',
+      Default: 'true',
+      AllowedValues: ['true', 'false'],
+    });
+
+    // Verify the condition exists
+    expect(template.Conditions.ShouldDenyExternalId).toEqual({
+      'Fn::Equals': ['true', { Ref: 'DenyExternalId' }],
+    });
+
+    // Verify each role has the ExternalId condition
+    const rolesToCheck = [
+      'FilePublishingRole',
+      'ImagePublishingRole',
+      'LookupRole',
+      'DeploymentActionRole',
+    ];
+
+    for (const roleName of rolesToCheck) {
+      const role = template.Resources[roleName];
+      expect(role).toBeDefined();
+
+      // Find AssumeRole statements for AWS principals (not service principals)
+      const assumeRoleStatements = role.Properties.AssumeRolePolicyDocument.Statement.filter(
+        (stmt: any) => stmt.Action === 'sts:AssumeRole' && stmt.Principal?.AWS,
+      );
+
+      // Each AssumeRole statement should have the ExternalId condition
+      for (const stmt of assumeRoleStatements) {
+        expect(stmt.Condition).toEqual({
+          'Fn::If': [
+            'ShouldDenyExternalId',
+            { Null: { 'sts:ExternalId': 'true' } },
+            { Ref: 'AWS::NoValue' },
+          ],
+        });
+      }
+    }
+
+    // Verify CloudFormationExecutionRole does NOT have the condition (it's assumed by service)
+    const cfnRole = template.Resources.CloudFormationExecutionRole;
+    const cfnStatements = cfnRole.Properties.AssumeRolePolicyDocument.Statement;
+    for (const stmt of cfnStatements) {
+      expect(stmt.Condition).toBeUndefined();
+    }
+  });
+});