Add tool to fix missing CRR replication permissions#377
Add tool to fix missing CRR replication permissions#377nicolas2bert wants to merge 3 commits intodevelopment/1.17from
Conversation
Switch from $'...' quoting with \n to <br> for line breaks in comment bodies, except inside fenced code blocks.
…eanup Include ownerDisplayName in check-replication-permissions.js results so the fix script can map accounts without extra API calls. Improve deleteTestAccount to clean up all buckets in the account (not just the initial one) and delete detached policies.
New script reads the output of check-replication-permissions.js and creates per-bucket IAM policies with s3:ReplicateObject, then attaches them to the corresponding roles. Supports --dry-run and is idempotent. Move @aws-sdk/client-iam to production dependencies (required at runtime). Add functional tests covering dry-run, idempotency, multi-bucket, multi-role, multi-account, key cleanup, and input validation.
Hello nicolas2bert,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Request integration branchesWaiting for integration branch creation to be requested by the user. To request integration branches, please comment on this pull request with the following command: Alternatively, the |
| // Consider accepting a CA cert path via CLI option instead. | ||
| httpsAgent: new https.Agent({ | ||
| keepAlive: true, | ||
| rejectUnauthorized: false, |
Check failure
Code scanning / CodeQL
Disabling certificate validation High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 18 hours ago
In general, the fix is to stop disabling TLS certificate validation and instead allow Node’s default behavior (validate certificates against the system trust store), or, if a private CA is used, configure the HTTPS agent to trust that CA rather than disabling validation entirely.
In this specific file, the best minimal fix that preserves existing functionality is:
- Remove
rejectUnauthorized: falsefrom thehttps.Agentconfiguration so that certificate validation is not disabled. - Optionally allow providing a custom CA certificate file via configuration/CLI so self-signed or private CA certificates can be trusted without disabling validation. However, since we must not modify other unseen code paths, the smallest secure change is to drop the
rejectUnauthorized: falseflag. The rest of the script already switches between HTTP and HTTPS usingconfig.useHttps, so only the HTTPS agent needs to be made safe.
Concretely:
- In
createIAMClient(around lines 121–137 inreplicationAudit/fix-missing-replication-permissions.js), edit thehttpsAgentdeclaration to remove therejectUnauthorized: falseproperty, leavingkeepAlive: trueintact. - No additional imports or helper methods are needed for this minimal fix.
| @@ -127,11 +127,9 @@ | ||
| credentials: { accessKeyId, secretAccessKey: secretKey }, | ||
| requestHandler: new NodeHttpHandler({ | ||
| httpAgent: new http.Agent({ keepAlive: true }), | ||
| // TBD: rejectUnauthorized: false disables certificate validation. | ||
| // Consider accepting a CA cert path via CLI option instead. | ||
| // HTTPS agent uses default certificate validation. | ||
| httpsAgent: new https.Agent({ | ||
| keepAlive: true, | ||
| rejectUnauthorized: false, | ||
| }), | ||
| }), | ||
| }); |
|
LGTM |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## development/1.17 #377 +/- ##
====================================================
- Coverage 43.62% 42.67% -0.96%
====================================================
Files 84 85 +1
Lines 5973 6106 +133
Branches 1255 1269 +14
====================================================
Hits 2606 2606
- Misses 3321 3453 +132
- Partials 46 47 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add
fix-missing-replication-permissions.jsthat reads the output ofcheck-replication-permissions.jsand automatically creates per-bucket IAM policies withs3:ReplicateObjectfor roles that are missing it.The script is