refactor(cli): require approval is a part of the CliIoHost (#151)

`CliIoHost` now governs when to ask for approval or not. This was
previously passed into deploy via deployOptions.

- `deployOptions.requireApproval` was deprecated before, and now it is
no longer being used (this may have to be pulled into a different change
that removes the `requireApproval` option entirely in a minor version
bump of toolkit-lib.
- `CliIoHost` has a property, `requireApproval` that can be set. When
the `requestResponse` API is called, we check to see if the message code
is a known `requireApproval` code and if so, see if the request is
necessary or if the default can be used.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license

---------

Signed-off-by: github-actions <github-actions@github.com>
Co-authored-by: github-actions <github-actions@github.com>

Author: kaizencc

packages/@aws-cdk/cloud-assembly-schema/lib/integ-tests/commands/common.ts

@@ -8,7 +8,7 @@ export enum RequireApproval {
   NEVER = 'never',
 
   /**
-   * Prompt for approval for any type  of change to the stack
+   * Prompt for approval for any type of change to the stack
    */
   ANYCHANGE = 'any-change',
 

packages/@aws-cdk/toolkit-lib/CODE_REGISTRY.md

@@ -17,7 +17,7 @@
 | CDK_TOOLKIT_W5022 | Empty existing stack, stack will be destroyed | warn | n/a |
 | CDK_TOOLKIT_I5031 | Informs about any log groups that are traced as part of the deployment | info | n/a |
 | CDK_TOOLKIT_I5050 | Confirm rollback during deployment | info | [ConfirmationRequest](https://docs.aws.amazon.com/cdk/api/toolkit-lib/interfaces/ConfirmationRequest.html) |
-| CDK_TOOLKIT_I5060 | Confirm deploy security sensitive changes | info | [ConfirmationRequest](https://docs.aws.amazon.com/cdk/api/toolkit-lib/interfaces/ConfirmationRequest.html) |
+| CDK_TOOLKIT_I5060 | Confirm deploy security sensitive changes | info | [DeployConfirmationRequest](https://docs.aws.amazon.com/cdk/api/toolkit-lib/interfaces/DeployConfirmationRequest.html) |
 | CDK_TOOLKIT_I5100 | Stack deploy progress | info | [StackDeployProgress](https://docs.aws.amazon.com/cdk/api/toolkit-lib/interfaces/StackDeployProgress.html) |
 | CDK_TOOLKIT_I5310 | The computed settings used for file watching | debug | [WatchSettings](https://docs.aws.amazon.com/cdk/api/toolkit-lib/interfaces/WatchSettings.html) |
 | CDK_TOOLKIT_I5311 | File watching started | info | [FileWatchEvent](https://docs.aws.amazon.com/cdk/api/toolkit-lib/interfaces/FileWatchEvent.html) |

packages/@aws-cdk/toolkit-lib/lib/actions/deploy/index.ts

@@ -1,6 +1,8 @@
 import type { CloudFormationStackArtifact } from '@aws-cdk/cx-api';
 import type { BaseDeployOptions } from './private/deploy-options';
 import type { Tag } from '../../api/aws-cdk';
+import type { ConfirmationRequest } from '../../toolkit/types';
+import type { PermissionChangeType } from '../diff/private/helpers';
 
 export type DeploymentMethod = DirectDeploymentMethod | ChangeSetDeploymentMethod;
 
@@ -122,8 +124,7 @@ export interface DeployOptions extends BaseDeployOptions {
    * Require a confirmation for security relevant changes before continuing with the deployment
    *
    * @default RequireApproval.NEVER
-   * @deprecated in future a message containing the full diff will be emitted and a response requested.
-   * Approval workflows should be implemented in the `IIoHost`.
+   * @deprecated requireApproval is governed by the `IIoHost`. This property is no longer used.
    */
   readonly requireApproval?: RequireApproval;
 
@@ -228,3 +229,14 @@ export interface StackDeployProgress {
    */
   readonly stack: CloudFormationStackArtifact;
 }
+
+/**
+ * Payload for a yes/no confirmation in deploy. Includes information on
+ * what kind of change is being made.
+ */
+export interface DeployConfirmationRequest extends ConfirmationRequest {
+  /**
+   * The type of change being made to the IAM permissions.
+   */
+  readonly permissionChangeType?: PermissionChangeType;
+}

packages/@aws-cdk/toolkit-lib/lib/actions/diff/private/helpers.ts

@@ -1,25 +1,43 @@
 import type { DescribeChangeSetOutput } from '@aws-cdk/cloudformation-diff';
 import { fullDiff } from '@aws-cdk/cloudformation-diff';
 import type * as cxapi from '@aws-cdk/cx-api';
-import { ToolkitError } from '../../../api/shared-public';
-import { RequireApproval } from '../../deploy';
 
 /**
- * Return whether the diff has security-impacting changes that need confirmation
+ * Different types of permissioning changes in a diff
  */
-export function diffRequiresApproval(
+export enum PermissionChangeType {
+  /**
+   * No permission changes
+   */
+  NONE = 'none',
+
+  /**
+   * Permissions are broadening
+   */
+  BROADENING = 'broadening',
+
+  /**
+   * Permissions are changed but not broadening
+   */
+  NON_BROADENING = 'non-broadening',
+}
+
+/**
+ * Return whether the diff has security-impacting changes that need confirmation.
+ */
+export function determinePermissionType(
   oldTemplate: any,
   newTemplate: cxapi.CloudFormationStackArtifact,
-  requireApproval: RequireApproval,
   changeSet?: DescribeChangeSetOutput,
-): boolean {
-  // @todo return or print the full diff.
+): PermissionChangeType {
+  // @todo return a printable version of 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}`);
+  if (diff.permissionsBroadened) {
+    return PermissionChangeType.BROADENING;
+  } else if (diff.permissionsAnyChanges) {
+    return PermissionChangeType.NON_BROADENING;
+  } else {
+    return PermissionChangeType.NONE;
   }
 }

packages/@aws-cdk/toolkit-lib/lib/api/io/private/codes.ts

@@ -1,7 +1,7 @@
 import type * as cxapi from '@aws-cdk/cx-api';
 import type { SdkTrace } from './sdk-logger';
 import type { BootstrapEnvironmentProgress } from '../../../actions/bootstrap';
-import type { StackDeployProgress } from '../../../actions/deploy';
+import type { DeployConfirmationRequest, StackDeployProgress } from '../../../actions/deploy';
 import type { StackDestroyProgress } from '../../../actions/destroy';
 import type { StackDetailsPayload } from '../../../actions/list';
 import type { StackRollbackProgress } from '../../../actions/rollback';
@@ -96,10 +96,10 @@ export const CODES = {
     description: 'Confirm rollback during deployment',
     interface: 'ConfirmationRequest',
   }),
-  CDK_TOOLKIT_I5060: make.confirm<ConfirmationRequest>({
+  CDK_TOOLKIT_I5060: make.confirm<DeployConfirmationRequest>({
     code: 'CDK_TOOLKIT_I5060',
     description: 'Confirm deploy security sensitive changes',
-    interface: 'ConfirmationRequest',
+    interface: 'DeployConfirmationRequest',
   }),
 
   CDK_TOOLKIT_I5100: make.info<StackDeployProgress>({

packages/@aws-cdk/toolkit-lib/lib/toolkit/toolkit.ts

@@ -11,7 +11,7 @@ import { BootstrapSource } from '../actions/bootstrap';
 import { AssetBuildTime, type DeployOptions, RequireApproval } from '../actions/deploy';
 import { type ExtendedDeployOptions, buildParameterMap, createHotswapPropertyOverrides, removePublishedAssets } from '../actions/deploy/private';
 import { type DestroyOptions } from '../actions/destroy';
-import { diffRequiresApproval } from '../actions/diff/private';
+import { determinePermissionType } from '../actions/diff/private';
 import { type ListOptions } from '../actions/list';
 import { type RollbackOptions } from '../actions/rollback';
 import { type SynthOptions } from '../actions/synth';
@@ -285,8 +285,6 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
 
     await migrator.tryMigrateResources(stackCollection, options);
 
-    const requireApproval = options.requireApproval ?? RequireApproval.NEVER;
-
     const parameterMap = buildParameterMap(options.parameters?.parameters);
 
     const hotswapMode = options.hotswap ?? HotswapMode.FULL_DEPLOYMENT;
@@ -353,19 +351,17 @@ export class Toolkit extends CloudAssemblySourceBuilder implements AsyncDisposab
         return;
       }
 
-      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`;
-          const confirmed = await ioHost.requestResponse(CODES.CDK_TOOLKIT_I5060.req(question, {
-            motivation,
-            concurrency,
-          }));
-          if (!confirmed) {
-            throw new ToolkitError('Aborted by user');
-          }
-        }
+      const currentTemplate = await deployments.readCurrentTemplate(stack);
+      const permissionChangeType = determinePermissionType(currentTemplate, stack);
+      const deployMotivation = '"--require-approval" is enabled and stack includes security-sensitive updates.';
+      const deployQuestion = `${deployMotivation}\nDo you wish to deploy these changes`;
+      const deployConfirmed = await ioHost.requestResponse(CODES.CDK_TOOLKIT_I5060.req(deployQuestion, {
+        motivation: deployMotivation,
+        concurrency,
+        permissionChangeType: permissionChangeType,
+      }));
+      if (!deployConfirmed) {
+        throw new ToolkitError('Aborted by user');
       }
 
       // Following are the same semantics we apply with respect to Notification ARNs (dictated by the SDK)

packages/@aws-cdk/toolkit-lib/test/_helpers/test-io-host.ts

@@ -1,14 +1,18 @@
 import type { IIoHost, IoMessage, IoMessageLevel, IoRequest } from '../../lib';
+import { RequireApproval } from '../../lib';
 import { isMessageRelevantForLevel } from '../../lib/api/io/private/level-priority';
 
 /**
  * A test implementation of IIoHost that does nothing but can by spied on.
- * Optional set a level to filter out all irrelevant messages.
+ * Optionally set a level to filter out all irrelevant messages.
+ * Optionally set a approval level.
  */
 export class TestIoHost implements IIoHost {
   public readonly notifySpy: jest.Mock<any, any, any>;
   public readonly requestSpy: jest.Mock<any, any, any>;
 
+  public requireDeployApproval = RequireApproval.NEVER;
+
   constructor(public level: IoMessageLevel = 'info') {
     this.notifySpy = jest.fn();
     this.requestSpy = jest.fn();
@@ -21,9 +25,27 @@ export class TestIoHost implements IIoHost {
   }
 
   public async requestResponse<T, U>(msg: IoRequest<T, U>): Promise<U> {
-    if (isMessageRelevantForLevel(msg, this.level)) {
+    if (isMessageRelevantForLevel(msg, this.level) && this.needsApproval(msg)) {
       this.requestSpy(msg);
     }
     return msg.defaultResponse;
   }
+
+  private needsApproval(msg: IoRequest<any, any>): boolean {
+    // Return true if the code is unrelated to approval
+    if (!['CDK_TOOLKIT_I5060'].includes(msg.code)) {
+      return true;
+    }
+
+    switch (this.requireDeployApproval) {
+      case RequireApproval.NEVER:
+        return false;
+      case RequireApproval.ANY_CHANGE:
+        return true;
+      case RequireApproval.BROADENING:
+        return msg.data?.permissionChangeType === 'broadening';
+      default:
+        return true;
+    }
+  }
 }

packages/@aws-cdk/toolkit-lib/test/actions/deploy.test.ts

@@ -33,6 +33,7 @@ jest.mock('../../lib/api/aws-cdk', () => {
 beforeEach(() => {
   ioHost.notifySpy.mockClear();
   ioHost.requestSpy.mockClear();
+  ioHost.requireDeployApproval = RequireApproval.NEVER;
   jest.clearAllMocks();
   mockFindCloudWatchLogGroups.mockReturnValue({
     env: { name: 'Z', account: 'X', region: 'Y' },
@@ -51,35 +52,45 @@ describe('deploy', () => {
     successfulDeployment();
   });
 
-  test('request response when require approval is set', async () => {
+  test('request response when changes exceed require approval threshold', async () => {
     // WHEN
+    // this is the lowest threshold; always require approval
+    ioHost.requireDeployApproval = RequireApproval.ANY_CHANGE;
+
     const cx = await builderFixture(toolkit, 'stack-with-role');
-    await toolkit.deploy(cx, {
-      requireApproval: RequireApproval.ANY_CHANGE,
-    });
+    await toolkit.deploy(cx);
 
     // THEN
     expect(ioHost.requestSpy).toHaveBeenCalledWith(expect.objectContaining({
       action: 'deploy',
       level: 'info',
       code: 'CDK_TOOLKIT_I5060',
       message: expect.stringContaining('Do you wish to deploy these changes'),
+      data: expect.objectContaining({
+        motivation: expect.stringContaining('stack includes security-sensitive updates.'),
+        permissionChangeType: 'broadening',
+      }),
     }));
   });
 
-  test('skips response by default', async () => {
+  test('skips response when changes do not meet require approval threshold', async () => {
     // WHEN
+    // never require approval, so we expect the IoHost to skip
+    ioHost.requireDeployApproval = RequireApproval.NEVER;
+
     const cx = await builderFixture(toolkit, 'stack-with-role');
-    await toolkit.deploy(cx, {
-      requireApproval: RequireApproval.NEVER,
-    });
+    await toolkit.deploy(cx);
 
     // THEN
     expect(ioHost.requestSpy).not.toHaveBeenCalledWith(expect.objectContaining({
       action: 'deploy',
       level: 'info',
       code: 'CDK_TOOLKIT_I5060',
       message: expect.stringContaining('Do you wish to deploy these changes'),
+      data: expect.objectContaining({
+        motivation: expect.stringContaining('stack includes security-sensitive updates.'),
+        permissionChangeType: 'broadening',
+      }),
     }));
   });
 

packages/aws-cdk/lib/toolkit/cli-io-host.ts

@@ -1,4 +1,5 @@
 import * as util from 'node:util';
+import { RequireApproval } from '@aws-cdk/cloud-assembly-schema';
 import * as chalk from 'chalk';
 import * as promptly from 'promptly';
 import { ToolkitError } from './error';
@@ -163,6 +164,13 @@ export interface CliIoHostProps {
   readonly isCI?: boolean;
 
   /**
+   * In what scenarios should the CliIoHost ask for approval
+   *
+   * @default RequireApproval.BROADENING
+   */
+  readonly requireDeployApproval?: RequireApproval;
+
+  /*
    * The initial Toolkit action the hosts starts with.
    *
    * @default StackActivityProgress.BAR
@@ -195,6 +203,7 @@ export class CliIoHost implements IIoHost {
   private _isTTY: boolean;
   private _logLevel: IoMessageLevel;
   private _internalIoHost?: IIoHost;
+  private _requireDeployApproval: RequireApproval;
   private _progress: StackActivityProgress = StackActivityProgress.BAR;
 
   // Stack Activity Printer
@@ -209,6 +218,7 @@ export class CliIoHost implements IIoHost {
     this._isTTY = props.isTTY ?? process.stdout.isTTY ?? false;
     this._logLevel = props.logLevel ?? 'info';
     this._isCI = props.isCI ?? isCI();
+    this._requireDeployApproval = props.requireDeployApproval ?? RequireApproval.BROADENING;
 
     this.stackProgress = props.stackProgress ?? StackActivityProgress.BAR;
   }
@@ -297,6 +307,20 @@ export class CliIoHost implements IIoHost {
     this._isTTY = value;
   }
 
+  /**
+   * Return the conditions for requiring approval in this CliIoHost.
+   */
+  public get requireDeployApproval() {
+    return this._requireDeployApproval;
+  }
+
+  /**
+   * Set the conditions for requiring approval in this CliIoHost.
+   */
+  public set requireDeployApproval(approval: RequireApproval) {
+    this._requireDeployApproval = approval;
+  }
+
   /**
    * Whether the CliIoHost is running in CI mode. In CI mode, all non-error output goes to stdout instead of stderr.
    */
@@ -394,6 +418,29 @@ export class CliIoHost implements IIoHost {
     ].includes(msg.code);
   }
 
+  /**
+   * Detect special messages encode information about whether or not
+   * they require approval
+   */
+  private skipApprovalStep(msg: IoRequest<any, any>): boolean {
+    const approvalToolkitCodes = ['CDK_TOOLKIT_I5060'];
+    if (!approvalToolkitCodes.includes(msg.code)) {
+      false;
+    }
+
+    switch (this.requireDeployApproval) {
+      // Never require approval
+      case RequireApproval.NEVER:
+        return true;
+      // Always require approval
+      case RequireApproval.ANYCHANGE:
+        return false;
+      // Require approval if changes include broadening permissions
+      case RequireApproval.BROADENING:
+        return ['none', 'non-broadening'].includes(msg.data?.permissionChangeType);
+    }
+  }
+
   /**
    * Determines the output stream, based on message level and configuration.
    */
@@ -454,6 +501,14 @@ export class CliIoHost implements IIoHost {
         throw new ToolkitError(`${motivation}, but concurrency is greater than 1 so we are unable to get a confirmation from the user`);
       }
 
+      // Special approval prompt
+      // Determine if the message needs approval. If it does, continue (it is a basic confirmation prompt)
+      // If it does not, return success (true). We only check messages with codes that we are aware
+      // are requires approval codes.
+      if (this.skipApprovalStep(msg)) {
+        return true;
+      }
+
       // Basic confirmation prompt
       // We treat all requests with a boolean response as confirmation prompts
       if (isConfirmationPrompt(msg)) {