S3UTILS-222 use one IAM policy per bucket instead of per role

Switching from one policy per role (covering multiple buckets) to one
policy per bucket eliminates stale-policy issues on re-runs: since the
policy document is always identical for a given bucket,
EntityAlreadyExists becomes a true no-op with no version management
needed.

Author: nicolas2bert

replicationAudit/README.md

@@ -484,9 +484,10 @@ Output saved to: /tmp/missing.json
 Reads the output of `check-replication-permissions.js` and creates IAM policies
 with `s3:ReplicateObject` for roles that are missing it.
 
-The script applies **minimal changes**: one policy per role (covering all affected
-buckets), with an explicit Statement ID (`AllowReplicateObjectAuditFix`) so the
-policies are easily identifiable later.
+The script applies **minimal changes**: one policy per bucket, with an explicit
+Statement ID (`AllowReplicateObjectAuditFix`) so the policies are easily
+identifiable later. Using one policy per bucket makes re-runs truly idempotent:
+if the policy already exists, its document is guaranteed to be identical.
 
 ### Prerequisites
 
@@ -517,19 +518,19 @@ node fix-missing-replication-permissions.js <input-file> <vault-host> <admin-con
 
 ### How It Works
 
-1. **Reads** the missing permissions file and groups buckets by account and role
+1. **Reads** the missing permissions file and enriches each entry with parsed role fields
 2. **Maps** account IDs to names using `ownerDisplayName` from the input (no API call)
-3. For each account:
+3. For each bucket entry (grouped by account for credential reuse):
    - **Generates** a temporary access key via vault admin API (15-minute auto-expiry)
-   - **Creates** one IAM policy per role with `s3:ReplicateObject` for all affected buckets
+   - **Creates** one IAM policy per bucket with `s3:ReplicateObject`
    - **Attaches** the policy to the role
    - **Deletes** the temporary access key (falls back to auto-expiry on failure)
 4. **Writes** results to the output file
 
 ### Policy Created
 
-For each role, the script creates a single policy named
-`s3-replication-audit-fix-<roleName>`:
+For each bucket, the script creates a policy named
+`s3-replication-audit-fix-<bucketName>`:
 
 ```json
 {
@@ -538,10 +539,7 @@ For each role, the script creates a single policy named
     "Sid": "AllowReplicateObjectAuditFix",
     "Effect": "Allow",
     "Action": "s3:ReplicateObject",
-    "Resource": [
-      "arn:aws:s3:::bucket-old-1/*",
-      "arn:aws:s3:::bucket-old-2/*"
-    ]
+    "Resource": "arn:aws:s3:::bucket-old-1/*"
   }]
 }
 ```
@@ -556,10 +554,10 @@ For each role, the script creates a single policy named
     "inputFile": "missing.json",
     "dryRun": false,
     "counts": {
-      "totalRolesProcessed": 1,
+      "totalBucketsProcessed": 3,
       "totalBucketsFixed": 3,
-      "policiesCreated": 1,
-      "policiesAttached": 1,
+      "policiesCreated": 3,
+      "policiesAttached": 3,
       "keysCreated": 1,
       "keysDeleted": 1,
       "errors": 0
@@ -571,9 +569,19 @@ For each role, the script creates a single policy named
       "accountName": "testaccount",
       "roleName": "crr-role-outdated",
       "roleArn": "arn:aws:iam::267390090509:role/crr-role-outdated",
-      "policyName": "s3-replication-audit-fix-crr-role-outdated",
-      "policyArn": "arn:aws:iam::267390090509:policy/s3-replication-audit-fix-crr-role-outdated",
-      "buckets": ["bucket-old-1", "bucket-old-2", "bucket-old-3"],
+      "policyName": "s3-replication-audit-fix-bucket-old-1",
+      "policyArn": "arn:aws:iam::267390090509:policy/s3-replication-audit-fix-bucket-old-1",
+      "bucket": "bucket-old-1",
+      "status": "success"
+    },
+    {
+      "accountId": "267390090509",
+      "accountName": "testaccount",
+      "roleName": "crr-role-outdated",
+      "roleArn": "arn:aws:iam::267390090509:role/crr-role-outdated",
+      "policyName": "s3-replication-audit-fix-bucket-old-2",
+      "policyArn": "arn:aws:iam::267390090509:policy/s3-replication-audit-fix-bucket-old-2",
+      "bucket": "bucket-old-2",
       "status": "success"
     }
   ],
@@ -589,18 +597,24 @@ Input:  missing.json
 Output: replication-fix-results.json
 Vault/IAM: 13.50.166.21:8600
 
-Processing 1 role(s)
+Processing 3 bucket(s)
 
-[1/1] Role "crr-role-outdated" — account "testaccount" (3 bucket(s))
-  Created policy "s3-replication-audit-fix-crr-role-outdated"
+[1/3] Bucket "bucket-old-1" — role "crr-role-outdated" — account "testaccount"
+  Created policy "s3-replication-audit-fix-bucket-old-1"
+  Attached policy to role "crr-role-outdated"
+[2/3] Bucket "bucket-old-2" — role "crr-role-outdated" — account "testaccount"
+  Created policy "s3-replication-audit-fix-bucket-old-2"
+  Attached policy to role "crr-role-outdated"
+[3/3] Bucket "bucket-old-3" — role "crr-role-outdated" — account "testaccount"
+  Created policy "s3-replication-audit-fix-bucket-old-3"
   Attached policy to role "crr-role-outdated"
 Deleted temp key for account "testaccount" (267390090509)
 
 === Summary ===
-Roles processed:       1
+Buckets processed:     3
 Buckets fixed:         3
-Policies created:      1
-Policies attached:     1
+Policies created:      3
+Policies attached:     3
 Keys created:          1
 Keys deleted:          1
 Errors:                0
@@ -614,7 +628,8 @@ Done.
 
 The script is safe to run multiple times:
 
-- If the policy already exists, it is reused (not duplicated)
+- Each bucket has its own policy, so if it already exists the document is
+  guaranteed to be identical — `EntityAlreadyExists` is a true no-op
 - Attaching an already-attached policy is a no-op in IAM
 - Temporary access keys auto-expire after 15 minutes even if deletion fails
 

replicationAudit/fix-missing-replication-permissions.js

@@ -6,6 +6,15 @@
  * Reads the output of check-replication-permissions.js and creates IAM policies
  * with s3:ReplicateObject for roles that are missing it, then attaches them.
  *
+ * TBD: This script does not re-check whether the permission is still missing
+ * before applying the fix. This means:
+ *   - If someone manually added s3:ReplicateObject between check and fix,
+ *     a redundant (but harmless) policy is created.
+ *   - If the fix policy is later modified externally, re-running won't
+ *     detect or correct it (EntityAlreadyExists skips the policy).
+ * We keep it simple today: the intended workflow is check → fix → re-check.
+ * Re-run check-replication-permissions.js after fixing to verify the result.
+ *
  * Usage: node fix-missing-replication-permissions.js <input-file> <vault-host> <admin-config> [output-file] [--iam-port <port>] [--https] [--dry-run]
  *
  * Requires: vaultclient, @aws-sdk/client-iam (both in s3utils dependencies)
@@ -83,45 +92,15 @@ function parseRoleArn(arn) {
     return { accountId: match[1], roleName: match[2] };
 }
 
-/**
- * Group missing entries by role, collecting all affected buckets per role.
- *
- * Input (results from check-replication-permissions.js):
- *   [
- *     { bucket: "bucket-old-1", ownerDisplayName: "testaccount", sourceRole: "arn:aws:iam::123:role/crr-role" },
- *     { bucket: "bucket-old-2", ownerDisplayName: "testaccount", sourceRole: "arn:aws:iam::123:role/crr-role" },
- *   ]
- *
- * Output:
- *   [
- *     { accountId: "123", accountName: "testaccount", roleName: "crr-role",
- *       roleArn: "arn:aws:iam::123:role/crr-role", buckets: ["bucket-old-1", "bucket-old-2"] }
- *   ]
- */
-function groupByRole(results) {
-    const roles = Object.groupBy(results, entry => entry.sourceRole);
-
-    return Object.entries(roles).map(([roleArn, entries]) => {
-        const { accountId, roleName } = parseRoleArn(roleArn);
-        return {
-            accountId,
-            accountName: entries[0].ownerDisplayName,
-            roleName,
-            roleArn,
-            buckets: entries.map(e => e.bucket),
-        };
-    });
-}
-
-/** Build the IAM policy document */
-function buildPolicyDocument(buckets) {
+/** Build the IAM policy document for a single bucket */
+function buildPolicyDocument(bucket) {
     return {
         Version: '2012-10-17',
         Statement: [{
             Sid: STATEMENT_ID,
             Effect: 'Allow',
             Action: 's3:ReplicateObject',
-            Resource: buckets.map(b => `arn:aws:s3:::${b}/*`),
+            Resource: `arn:aws:s3:::${bucket}/*`,
         }],
     };
 }
@@ -190,13 +169,11 @@ async function main() {
         process.exit(1);
     }
 
-    // Group by role, sorted by account so roles in the same account are
-    // processed consecutively — reduces the chance of cached credentials
-    // expiring while other accounts are being processed.
-    const roles = groupByRole(entries)
-        .sort((a, b) => (a.accountId < b.accountId ? -1 : 1));
+    // Sort by sourceRole so entries in the same account are processed
+    // consecutively, maximising credential cache hits.
+    entries.sort((a, b) => (a.sourceRole < b.sourceRole ? -1 : 1));
 
-    log(`Processing ${roles.length} role(s)`);
+    log(`Processing ${entries.length} bucket(s)`);
     log('');
 
     // Read admin credentials
@@ -223,7 +200,7 @@ async function main() {
             inputFile: config.inputFile,
             dryRun: config.dryRun,
             counts: {
-                totalRolesProcessed: roles.length,
+                totalBucketsProcessed: entries.length,
                 totalBucketsFixed: 0,
                 policiesCreated: 0,
                 policiesAttached: 0,
@@ -236,33 +213,34 @@ async function main() {
         errors: [],
     };
 
-    // Cache IAM client per account to reuse connections across roles
+    // Cache IAM client per account to reuse connections across buckets
     // and for cleanup (key deletion).
     // Map<accountId, { accountName, accessKeyId, iamClient }>
     const accountCache = new Map();
 
-    for (let i = 0; i < roles.length; i++) {
-        const { accountId, accountName, roleName, roleArn, buckets } = roles[i];
-        const policyName = `${POLICY_PREFIX}-${roleName}`;
-        const policyDocument = buildPolicyDocument(buckets);
+    for (let i = 0; i < entries.length; i++) {
+        const entry = entries[i];
+        const { accountId, roleName } = parseRoleArn(entry.sourceRole);
+        const { bucket, ownerDisplayName: accountName } = entry;
+        const policyName = `${POLICY_PREFIX}-${bucket}`;
+        const policyDocument = buildPolicyDocument(bucket);
 
-        log(`[${i + 1}/${roles.length}] Role "${roleName}" — account "${accountName}" (${buckets.length} bucket(s))`);
+        log(`[${i + 1}/${entries.length}] Bucket "${bucket}" — role "${roleName}" — account "${accountName}"`);
 
         const fix = {
             accountId,
             accountName,
             roleName,
-            roleArn,
+            roleArn: entry.sourceRole,
             policyName,
-            buckets,
+            bucket,
             status: 'pending',
         };
 
         if (config.dryRun) {
             fix.status = 'dry-run';
             log(`  [DRY-RUN] Would create policy "${policyName}"`);
             log(`  [DRY-RUN] Would attach to role "${roleName}"`);
-            log(`  Buckets: ${buckets.join(', ')}`);
             outcome.fixes.push(fix);
             continue;
         }
@@ -284,9 +262,10 @@ async function main() {
 
             const { iamClient } = accountCache.get(accountId);
 
-            // Idempotent/safe to re-run: CreatePolicy reuses an existing policy
-            // with the same name, and AttachRolePolicy is a no-op if
-            // the policy is already attached to the role.
+            // Idempotent/safe to re-run: each bucket has its own policy,
+            // so EntityAlreadyExists means the exact same policy document
+            // already exists — a true no-op. AttachRolePolicy is also
+            // a no-op if the policy is already attached to the role.
             let policyArn;
             try {
                 const resp = await iamClient.send(new CreatePolicyCommand({
@@ -300,7 +279,7 @@ async function main() {
                 if (err.name === 'EntityAlreadyExistsException'
                     || err.Code === 'EntityAlreadyExists') {
                     policyArn = `arn:aws:iam::${accountId}:policy/${policyName}`;
-                    log(`  Policy "${policyName}" already exists, reusing`);
+                    log(`  Policy "${policyName}" already exists, skipping`);
                 } else {
                     throw err;
                 }
@@ -316,7 +295,7 @@ async function main() {
             log(`  Attached policy to role "${roleName}"`);
 
             fix.status = 'success';
-            outcome.metadata.counts.totalBucketsFixed += buckets.length;
+            outcome.metadata.counts.totalBucketsFixed++;
         } catch (err) {
             fix.status = 'error';
             fix.error = err.message;
@@ -325,6 +304,7 @@ async function main() {
                 accountId,
                 accountName,
                 roleName,
+                bucket,
                 policyName,
                 message: err.message,
             });
@@ -356,7 +336,7 @@ async function main() {
 
     // Print summary
     log('\n=== Summary ===');
-    log(`Roles processed:       ${outcome.metadata.counts.totalRolesProcessed}`);
+    log(`Buckets processed:     ${outcome.metadata.counts.totalBucketsProcessed}`);
     log(`Buckets fixed:         ${outcome.metadata.counts.totalBucketsFixed}`);
     log(`Policies created:      ${outcome.metadata.counts.policiesCreated}`);
     log(`Policies attached:     ${outcome.metadata.counts.policiesAttached}`);