fix(cli-internal): harden drift detection against false negatives from CR review

Safety-critical (false negatives in rollback gate):
1. CloudFormation::Stack changes without ChangeSetId tracked as incomplete
2. Recoverable failures with empty Changes return skipped:true
3. DELETE_COMPLETE/DELETE_FAILED nested changesets tracked as incomplete

Robustness:
4. extractStackNameFromArn in catch block wrapped in try-catch
5. validateLockRollbackDrift catch block properly typed
6. DELETE_FAILED added to terminal statuses for polling

Author: Benjamin Pace

packages/amplify-cli/src/commands/drift-detection/detect-template-drift.ts

@@ -261,8 +261,17 @@ async function analyzeChangeSet(
       return result;
     }
 
-    // EarlyValidation failures still have Changes populated — fall through to process them
+    // Recoverable failures (EarlyValidation, Template format errors) still have Changes populated.
+    // However, if Changes is empty despite the recoverable classification, treat as incomplete.
     if (isRecoverableFailure(changeSet.StatusReason)) {
+      if (!changeSet.Changes || changeSet.Changes.length === 0) {
+        print.warn(`Recoverable failure but no Changes populated: ${changeSet.StatusReason}`);
+        return {
+          changes: [],
+          skipped: true,
+          skipReason: `Changeset failed with no usable changes: ${changeSet.StatusReason}`,
+        };
+      }
       print.warn(`Nested changeset FAILED (recoverable): ${changeSet.StatusReason}`);
     } else if (isRoot) {
       // Root changeset FAILED because a nested changeset failed. The root's StatusReason
@@ -299,6 +308,16 @@ async function analyzeChangeSet(
     const changeInfo: ResourceChangeWithNested = { ...rc };
 
     // Check if this is a nested stack with its own changeset
+    if (rc.ResourceType === 'AWS::CloudFormation::Stack' && !rc.ChangeSetId) {
+      // Stack change without a nested changeset — can't inspect its contents.
+      // This happens when IncludeNestedStacks creates the root changeset but the
+      // nested changeset failed before being created. Track as incomplete.
+      const stackLabel = rc.LogicalResourceId || rc.PhysicalResourceId || 'unknown';
+      print.warn(`Nested stack ${stackLabel} has no changeset — tracking as incomplete`);
+      skippedStacks.push(stackLabel);
+      result.changes.push(changeInfo);
+      continue;
+    }
     if (rc.ResourceType === 'AWS::CloudFormation::Stack' && rc.ChangeSetId && rc.PhysicalResourceId) {
       try {
         // Extract stack name and changeset name from ARNs using parseArn utility
@@ -317,25 +336,34 @@ async function analyzeChangeSet(
             ChangeSetName: changeSetName,
           }),
         );
-        const terminalStatuses = ['CREATE_COMPLETE', 'FAILED', 'DELETE_COMPLETE'];
-        const maxPollAttempts = 30;
-        for (let attempt = 0; !terminalStatuses.includes(nestedChangeSet.Status!) && attempt < maxPollAttempts; attempt++) {
+        const terminalStatuses = ['CREATE_COMPLETE', 'FAILED', 'DELETE_COMPLETE', 'DELETE_FAILED'];
+        const NESTED_POLL_MAX_ATTEMPTS = 30;
+        const NESTED_POLL_INTERVAL_MS = 2000;
+        for (let attempt = 0; !terminalStatuses.includes(nestedChangeSet.Status ?? '') && attempt < NESTED_POLL_MAX_ATTEMPTS; attempt++) {
           print.debug(`Nested changeset ${stackName} is ${nestedChangeSet.Status}, waiting...`);
-          await new Promise((resolve) => setTimeout(resolve, 2000));
+          await new Promise((resolve) => setTimeout(resolve, NESTED_POLL_INTERVAL_MS));
           nestedChangeSet = await cfn.send(
             new DescribeChangeSetCommand({
               StackName: stackName,
               ChangeSetName: changeSetName,
             }),
           );
         }
-        if (!terminalStatuses.includes(nestedChangeSet.Status!)) {
+        if (!terminalStatuses.includes(nestedChangeSet.Status ?? '')) {
           print.warn(`Nested changeset ${stackName} did not reach terminal status (${nestedChangeSet.Status})`);
           skippedStacks.push(stackName);
           result.changes.push(changeInfo);
           continue;
         }
 
+        // Deleted or failed-to-delete changesets can't be analyzed — track as incomplete
+        if (nestedChangeSet.Status === 'DELETE_COMPLETE' || nestedChangeSet.Status === 'DELETE_FAILED') {
+          print.warn(`Nested changeset ${stackName} was deleted (${nestedChangeSet.Status}) — tracking as incomplete`);
+          skippedStacks.push(stackName);
+          result.changes.push(changeInfo);
+          continue;
+        }
+
         // Print nested changeset details
         if (nestedChangeSet.Changes && nestedChangeSet.Changes.length > 0) {
           print.debug(`Nested Stack: ${stackName}`);
@@ -371,11 +399,16 @@ async function analyzeChangeSet(
           print.debug(`Processed ${nestedResult.changes.length} nested changes`);
         }
       } catch (error: any) {
-        // Log error and track as incomplete
+        // Log error and track as incomplete. Use LogicalResourceId as fallback
+        // since extractStackNameFromArn could throw on malformed ARNs.
         print.warn(`⚠ Could not fetch nested changeset for ${rc.LogicalResourceId}: ${error.message}`);
         print.debug(`Stack ARN: ${rc.PhysicalResourceId}`);
         print.debug(`ChangeSet ID: ${rc.ChangeSetId}`);
-        skippedStacks.push(extractStackNameFromArn(rc.PhysicalResourceId));
+        try {
+          skippedStacks.push(extractStackNameFromArn(rc.PhysicalResourceId));
+        } catch {
+          skippedStacks.push(rc.LogicalResourceId || 'unknown');
+        }
       }
     }
 

packages/amplify-cli/src/commands/gen2-migration/lock.ts

@@ -69,10 +69,20 @@ const isExpectedLockDrift = (change: ResourceChangeWithNested): boolean => {
   if (change.Scope?.length === 1 && change.Scope[0] === 'DeletionPolicy') return true;
 
   // Cascading IAM Policy change caused by DeletionPolicy modification on a referenced resource.
-  // All Details must be Dynamic ResourceAttribute re-evaluations (no static/direct changes).
-  if (change.ResourceType === 'AWS::IAM::Policy' && change.Details?.length) {
+  // Must be: Properties-only scope, all Details are Dynamic ResourceAttribute re-evaluations
+  // with CausingEntity referencing a table attribute (e.g., TodoTable.Arn, TodoTable.StreamArn).
+  if (
+    change.ResourceType === 'AWS::IAM::Policy' &&
+    change.Scope?.length === 1 &&
+    change.Scope[0] === 'Properties' &&
+    change.Details?.length
+  ) {
     return change.Details.every(
-      (d) => d.ChangeSource === 'ResourceAttribute' && d.Evaluation === 'Dynamic' && d.Target?.RequiresRecreation === 'Never',
+      (d) =>
+        d.ChangeSource === 'ResourceAttribute' &&
+        d.Evaluation === 'Dynamic' &&
+        d.Target?.RequiresRecreation === 'Never' &&
+        /Table\.(Arn|StreamArn)$/.test(d.CausingEntity ?? ''),
     );
   }
 
@@ -310,8 +320,8 @@ export class AmplifyMigrationLockStep extends AmplifyMigrationStep {
       }
 
       return { valid: true };
-    } catch (e) {
-      return { valid: false, report: e.message };
+    } catch (e: any) {
+      return { valid: false, report: e?.message ?? String(e) };
     }
   }