Skip to content

Commit c4510b2

Browse files
author
Benjamin Pace
committed
fix(cli-internal): treat Add/Remove on nested stack wrappers as real drift
hasRealDrift silently skipped CloudFormation::Stack entries without nestedChanges regardless of Action. If a nested stack was added or removed outside Amplify, this produced a false negative allowing rollback when it should be blocked.
1 parent 7876c08 commit c4510b2

File tree

2 files changed

+71
-2
lines changed
  • packages/amplify-cli/src

2 files changed

+71
-2
lines changed

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

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -707,6 +707,70 @@ describe('AmplifyMigrationLockStep', () => {
707707
expect(valid).toBe(true);
708708
});
709709

710+
it('should fail validation when IAM Policy cascading change is from a non-table resource', async () => {
711+
mockDetectTemplateDrift.mockResolvedValueOnce({
712+
changes: [
713+
{
714+
Action: 'Modify',
715+
LogicalResourceId: 'SomeLambdaPolicy',
716+
ResourceType: 'AWS::IAM::Policy',
717+
Scope: ['Properties'],
718+
Details: [
719+
{
720+
ChangeSource: 'ResourceAttribute',
721+
Evaluation: 'Dynamic',
722+
Target: { Attribute: 'Properties', Name: 'PolicyDocument', RequiresRecreation: 'Never' },
723+
CausingEntity: 'MyLambdaFunction.Arn',
724+
},
725+
],
726+
},
727+
],
728+
skipped: false,
729+
});
730+
731+
const plan = await lockStep.rollback();
732+
const valid = await plan.validate();
733+
734+
expect(valid).toBe(false);
735+
});
736+
737+
it('should fail validation when a nested stack is added (Add action on CloudFormation::Stack)', async () => {
738+
mockDetectTemplateDrift.mockResolvedValueOnce({
739+
changes: [
740+
{
741+
Action: 'Add',
742+
LogicalResourceId: 'newUnexpectedStack',
743+
ResourceType: 'AWS::CloudFormation::Stack',
744+
},
745+
],
746+
skipped: false,
747+
});
748+
749+
const plan = await lockStep.rollback();
750+
const valid = await plan.validate();
751+
752+
expect(valid).toBe(false);
753+
});
754+
755+
it('should fail validation when a nested stack is removed (Remove action on CloudFormation::Stack)', async () => {
756+
mockDetectTemplateDrift.mockResolvedValueOnce({
757+
changes: [
758+
{
759+
Action: 'Remove',
760+
LogicalResourceId: 'authStack',
761+
ResourceType: 'AWS::CloudFormation::Stack',
762+
PhysicalResourceId: 'arn:aws:cloudformation:us-east-1:123:stack/auth-stack/abc',
763+
},
764+
],
765+
skipped: false,
766+
});
767+
768+
const plan = await lockStep.rollback();
769+
const valid = await plan.validate();
770+
771+
expect(valid).toBe(false);
772+
});
773+
710774
it('should pass validation when CloudFormation::Stack wrapper has no nestedChanges', async () => {
711775
mockDetectTemplateDrift.mockResolvedValueOnce({
712776
changes: [

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

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,9 @@ const ALLOW_ALL_POLICY = {
5656
* 1. Direct DeletionPolicy changes — Modify with Scope exactly `['DeletionPolicy']`
5757
* 2. Cascading IAM Policy changes — CFN flags IAM policies that reference the modified
5858
* table's attributes (e.g., `TodoTable.Arn` in PolicyDocument) as Dynamic re-evaluations.
59-
* These have `ChangeSource: ResourceAttribute`, `Evaluation: Dynamic`, and
60-
* `RequiresRecreation: Never` — they are harmless re-evaluations, not actual changes.
59+
* These have `ChangeSource: ResourceAttribute`, `Evaluation: Dynamic`,
60+
* `RequiresRecreation: Never`, and `CausingEntity` matching `*Table.Arn` or
61+
* `*Table.StreamArn` — they are harmless re-evaluations, not actual changes.
6162
*
6263
* For lock rollback to determine whether the environment is safe to revert, these expected
6364
* changes must be filtered out so only real drift blocks the rollback.
@@ -100,6 +101,10 @@ function hasRealDrift(changes: ResourceChangeWithNested[]): boolean {
100101
if (hasRealDrift(change.nestedChanges)) return true;
101102
} else if (change.ResourceType !== 'AWS::CloudFormation::Stack') {
102103
if (!isExpectedLockDrift(change)) return true;
104+
} else if (change.Action !== 'Modify') {
105+
// Add/Remove on a CloudFormation::Stack without nestedChanges is real drift —
106+
// an entire nested stack was added or deleted outside Amplify.
107+
return true;
103108
}
104109
}
105110
return false;

0 commit comments

Comments
 (0)