From 92412a43fdb8804231d95a337f5e6f6410bab9e1 Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 23 Jul 2026 14:43:06 -0400 Subject: [PATCH] fix(migration): verify backup AVAILABLE pre-truncate; document wildcard risk acceptance (cross-review FIX/NIT) --- cdk/common.py | 8 +++++++ scripts/migrate_tables.py | 46 ++++++++++++++++++++++++++------------- 2 files changed, 39 insertions(+), 15 deletions(-) diff --git a/cdk/common.py b/cdk/common.py index 54fc9e8..f9d1022 100644 --- a/cdk/common.py +++ b/cdk/common.py @@ -61,6 +61,14 @@ def make_slack_bot_read_policy(scope, table_name): affect in-account callers. PrincipalArn is wildcarded by the bot's CFN stack prefix because role-name hash suffixes rotate on replacement; account + prefix + read-only actions bound the grant. + + Accepted risk (GPT-4.1 cross-review 2026-07-23): a mgmt principal with + iam:CreateRole could mint a prefix-matching role, but that capability + already implies near-org-admin blast radius; exact-ARN pinning was + rejected because it breaks on CFN role replacement. Assumption: the bot + Lambdas call DynamoDB DIRECTLY under their execution roles - if a + consumer ever role-chains first, aws:PrincipalArn will not match and + reads fail closed. """ stack = Stack.of(scope) return iam.PolicyDocument( diff --git a/scripts/migrate_tables.py b/scripts/migrate_tables.py index bffca17..aa29530 100644 --- a/scripts/migrate_tables.py +++ b/scripts/migrate_tables.py @@ -118,6 +118,36 @@ def normalize_cutoff(value): return value.replace("Z", "+00:00") +def _truncate_destination(name, cfg, dst, args): + """Backup-gated destination truncate for the truncate-and-load tables.""" + if args.execute: + if not args.backup_arn: + sys.exit( + f"ERROR: {name} is truncate-and-load; pass --backup-arn of an " + "AVAILABLE on-demand backup of the DESTINATION table taken " + "just now (aws dynamodb create-backup / describe-backup)." + ) + desc = dst.meta.client.describe_backup(BackupArn=args.backup_arn) + details = desc["BackupDescription"]["BackupDetails"] + src_table = desc["BackupDescription"]["SourceTableDetails"] + if details["BackupStatus"] != "AVAILABLE" or src_table["TableName"] != name: + sys.exit( + f"ERROR: backup {args.backup_arn} is " + f"{details['BackupStatus']} for table " + f"{src_table['TableName']!r}; need an AVAILABLE backup " + f"of {name!r}. Aborting before truncate." + ) + dest_keys = [{k: item[k] for k in cfg["keys"]} for item in scan_items(dst)] + print(f"[{name}] truncate: {len(dest_keys)} destination rows to delete") + if args.execute: + with dst.batch_writer() as batch: + for key in dest_keys: + batch.delete_item(Key=key) + print(f"[{name}] truncate: done") + else: + print(f"[{name}] DRY-RUN: no deletes performed") + + def copy_table(name, cfg, args): src = session_table(args.source_profile, SOURCE_ACCOUNT, name) dst = session_table(args.dest_profile, DEST_ACCOUNT, name) @@ -129,21 +159,7 @@ def copy_table(name, cfg, args): filter_kwargs = comments_cutoff_filter(normalize_cutoff(args.cutoff)) if cfg["mode"] == "truncate_load": - if args.execute and not args.backup_arn: - sys.exit( - f"ERROR: {name} is truncate-and-load; pass --backup-arn of an " - "AVAILABLE on-demand backup of the DESTINATION table taken " - "just now (aws dynamodb create-backup / describe-backup)." - ) - dest_keys = [{k: item[k] for k in cfg["keys"]} for item in scan_items(dst)] - print(f"[{name}] truncate: {len(dest_keys)} destination rows to delete") - if args.execute: - with dst.batch_writer() as batch: - for key in dest_keys: - batch.delete_item(Key=key) - print(f"[{name}] truncate: done") - else: - print(f"[{name}] DRY-RUN: no deletes performed") + _truncate_destination(name, cfg, dst, args) copied = 0 if args.execute: