From 77863b7c6806a04e0e8bc4a40e80c90e8f738d0d Mon Sep 17 00:00:00 2001 From: Kaizen Conroy Date: Fri, 17 Jan 2025 13:44:25 -0500 Subject: [PATCH] require approval is live --- .../lib/actions/diff/private/helpers.ts | 24 +++++++++++++ .../toolkit/lib/actions/diff/private/index.ts | 1 + .../@aws-cdk/toolkit/lib/toolkit/toolkit.ts | 24 ++++++------- .../toolkit/test/actions/deploy.test.ts | 36 +++++++++++-------- 4 files changed, 59 insertions(+), 26 deletions(-) create mode 100644 packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts create mode 100644 packages/@aws-cdk/toolkit/lib/actions/diff/private/index.ts diff --git a/packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts b/packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts new file mode 100644 index 0000000000000..98155f3d02c0e --- /dev/null +++ b/packages/@aws-cdk/toolkit/lib/actions/diff/private/helpers.ts @@ -0,0 +1,24 @@ +import { DescribeChangeSetOutput, fullDiff } from '@aws-cdk/cloudformation-diff'; +import * as cxapi from '@aws-cdk/cx-api'; +import { ToolkitError } from '../../../api/errors'; +import { RequireApproval } from '../../deploy'; + +/** + * Return whether the diff has security-impacting changes that need confirmation + */ +export function diffRequiresApproval( + oldTemplate: any, + newTemplate: cxapi.CloudFormationStackArtifact, + requireApproval: RequireApproval, + changeSet?: DescribeChangeSetOutput, +): boolean { + // @todo return or print the full diff. + const diff = fullDiff(oldTemplate, newTemplate.template, changeSet); + + switch (requireApproval) { + case RequireApproval.NEVER: return false; + case RequireApproval.ANY_CHANGE: return diff.permissionsAnyChanges; + case RequireApproval.BROADENING: return diff.permissionsBroadened; + default: throw new ToolkitError(`Unrecognized approval level: ${requireApproval}`); + } +} diff --git a/packages/@aws-cdk/toolkit/lib/actions/diff/private/index.ts b/packages/@aws-cdk/toolkit/lib/actions/diff/private/index.ts new file mode 100644 index 0000000000000..c5f595cf9d428 --- /dev/null +++ b/packages/@aws-cdk/toolkit/lib/actions/diff/private/index.ts @@ -0,0 +1 @@ +export * from './helpers'; diff --git a/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts b/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts index e2c08656d0bd1..056e25471f25d 100644 --- a/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts +++ b/packages/@aws-cdk/toolkit/lib/toolkit/toolkit.ts @@ -8,6 +8,7 @@ import { AssetBuildTime, DeployOptions, RequireApproval } from '../actions/deplo import { buildParameterMap, removePublishedAssets } from '../actions/deploy/private'; import { DestroyOptions } from '../actions/destroy'; import { DiffOptions } from '../actions/diff'; +import { diffRequiresApproval } from '../actions/diff/private'; import { ListOptions } from '../actions/list'; import { RollbackOptions } from '../actions/rollback'; import { SynthOptions } from '../actions/synth'; @@ -207,7 +208,7 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab }); await migrator.tryMigrateResources(stackCollection, options); - // const requireApproval = options.requireApproval ?? RequireApproval.BROADENING; + const requireApproval = options.requireApproval ?? RequireApproval.NEVER; const parameterMap = buildParameterMap(options.parameters?.parameters); @@ -281,17 +282,16 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab return; } - // @TODO - // if (requireApproval !== RequireApproval.NEVER) { - // const currentTemplate = await deployments.readCurrentTemplate(stack); - // if (printSecurityDiff(currentTemplate, stack, requireApproval)) { - // await askUserConfirmation( - // concurrency, - // '"--require-approval" is enabled and stack includes security-sensitive updates', - // 'Do you wish to deploy these changes', - // ); - // } - // } + if (requireApproval !== RequireApproval.NEVER) { + const currentTemplate = await deployments.readCurrentTemplate(stack); + if (diffRequiresApproval(currentTemplate, stack, requireApproval)) { + const motivation = '"--require-approval" is enabled and stack includes security-sensitive updates.'; + const question = `${motivation}\nDo you wish to deploy these changes`; + // @todo reintroduce concurrency and corked logging in CliHost + const confirmed = await ioHost.requestResponse(confirm('CDK_TOOLKIT_I5060', question, motivation, true, concurrency)); + if (!confirmed) { throw new ToolkitError('Aborted by user'); } + } + } // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK) // diff --git a/packages/@aws-cdk/toolkit/test/actions/deploy.test.ts b/packages/@aws-cdk/toolkit/test/actions/deploy.test.ts index fcff9d1dd86fb..ffa665b2e77a1 100644 --- a/packages/@aws-cdk/toolkit/test/actions/deploy.test.ts +++ b/packages/@aws-cdk/toolkit/test/actions/deploy.test.ts @@ -1,10 +1,10 @@ -import * as s3 from 'aws-cdk-lib/aws-s3'; -import * as core from 'aws-cdk-lib/core'; -import { Toolkit } from '../../lib/toolkit'; -import { fixture, TestIoHost } from '../_helpers'; import { CloudFormationClient, DescribeChangeSetCommand, DescribeStacksCommand, StackStatus } from '@aws-sdk/client-cloudformation'; +import * as iam from 'aws-cdk-lib/aws-iam'; +import * as core from 'aws-cdk-lib/core'; import { mockClient } from 'aws-sdk-client-mock'; -import { StackSelectionStrategy } from '../../lib'; +import { RequireApproval, StackSelectionStrategy } from '../../lib'; +import { Toolkit } from '../../lib/toolkit'; +import { TestIoHost } from '../_helpers'; const ioHost = new TestIoHost(); const notifySpy = jest.spyOn(ioHost, 'notify'); @@ -23,10 +23,6 @@ const cxFromBuilder = async () => { }); }; -const cxFromApp = async (name: string) => { - return cdk.fromCdkApp(`node ${fixture(name)}`); -}; - beforeEach(() => { requestResponseSpy.mockClear(); notifySpy.mockClear(); @@ -70,19 +66,31 @@ describe('deploy', () => { })); }); - test('deploy from app', async () => { + test('request response when require approval is set', async () => { // WHEN - await cdk.deploy(await cxFromApp('two-empty-stacks')); + const cx = await cdk.fromAssemblyBuilder(async () => { + const app = new core.App(); + const stack = new core.Stack(app, 'Stack1'); + new iam.Role(stack, 'Role', { + assumedBy: new iam.ArnPrincipal('arn'), + }); + return app.synth() as any; + }); + + await cdk.deploy(cx, { + requireApproval: RequireApproval.ANY_CHANGE, + }); // THEN - expect(notifySpy).toHaveBeenCalledWith(expect.objectContaining({ + expect(requestResponseSpy).toHaveBeenCalledWith(expect.objectContaining({ action: 'deploy', level: 'info', - message: expect.stringContaining('Deployment time:'), + code: 'CDK_TOOLKIT_I5060', + message: expect.stringContaining('Do you wish to deploy these changes'), })); }); - test('deploy no resources results in warning', async () => { + test('stack information is returned when successfully deployed', async () => { // WHEN const cx = await cxFromBuilder(); await cdk.deploy(cx, {