Fix S3 SSE-C copy/move (#500) and add addressing_style support (#527)#576
Merged
Conversation
Contributor
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #576 +/- ##
========================================
- Coverage 94.3% 94.3% -0.1%
========================================
Files 28 28
Lines 2259 2267 +8
========================================
+ Hits 2132 2138 +6
- Misses 127 129 +2
🚀 New features to boost your workflow:
|
Fixes #500: copy/move/touch on S3 paths now forward CopySource* extras (notably CopySourceSSECustomerKey) by filtering extra_args against S3Transfer.ALLOWED_COPY_ARGS instead of ALLOWED_UPLOAD_ARGS/DOWNLOAD_ARGS. Fixes #527: adds addressing_style kwarg to S3Client that flows through to botocore.config.Config(s3={"addressing_style": ...}) for both the resource and client, including the unsigned client used for public URLs. Also fixes a latent TypeError in the same-key touch path where Metadata/MetadataDirective in user-supplied extra_args would collide with the explicit kwargs to copy_from. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
botocore's sse_md5 handler base64-encodes SSECustomerKey and computes SSECustomerKeyMD5 itself. Passing an already-base64-encoded string double-encodes the key, so S3 rejects PutObject with "The secret key was invalid for the specified algorithm". Pass the raw 32 random bytes instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
pjbull
force-pushed
the
pjbull-fix-issues-500-527
branch
from
61ed796 to
8cc33d0
Compare
The drivendata CI bucket has an S3 bucket policy that explicitly denies PutObject when SSE-C is specified, so the SSE-C live regression test can't run end-to-end there. Detect that exact AccessDenied message and pytest.skip instead of failing, so the test still provides regression coverage on buckets that allow SSE-C without breaking CI on ones that don't. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Jun 28, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related S3 fixes that share a refactor of how
S3Clientconstructs its boto3 resource/client.#500 — SSE-C copy/move was broken.
_move_filefilteredextra_argsthroughALLOWED_UPLOAD_ARGS/ALLOWED_DOWNLOAD_ARGS, which excludes theCopySourceSSECustomer*keys. As a result,S3Path.copy,S3Path.move, andS3Path.touchon SSE-C objects failed withInvalidRequestfrom S3. The fix introduces a third filter,boto3_copy_extra_args, built fromS3Transfer.ALLOWED_COPY_ARGS, and routes bothObject.copy(ExtraArgs=...)(cross-key) andObject.copy_from(**extras)(same-key touch) through it.#527 — no way to use virtual-hosted addressing. Added an
addressing_stylekwarg onS3Clientthat gets folded into a singlebotocore.config.Config(s3={"addressing_style": ...})passed to both the resource and client. The existingno_sign_requestpath now composes with it via the sameConfig, and_get_public_url's unsigned client picks up the sames3settings so presigned URLs against virtual-host-only endpoints work too.Drive-by fix: in the same-key touch branch of
_move_file,copy_fromwas called with explicitMetadataandMetadataDirectiveand**boto3_copy_extra_args. Since both keys live inALLOWED_COPY_ARGS, a user who passed either viaextra_argswould hitTypeError: copy_from() got multiple values for keyword argument. Those keys are now dropped from the splat in the touch branch.Tests
test_copy_and_move_pass_copy_extra_args,test_touch_passes_copy_extra_args_without_collision,test_s3_addressing_style_virtual_passed_to_boto3.s3_rig.live_server):test_sse_c_copy_and_move_liveround-trips an SSE-C object through write/copy/touch/move;test_addressing_style_virtual_liveasserts the presigned URL host is{bucket}.s3...and round-trips a real object.resource_config/client_config, andMockBoto3Objectrecordslast_copy/last_copy_fromso tests can inspect what was forwarded.Boto3 docs verified (boto3 1.43.36)
S3Transfer.ALLOWED_COPY_ARGScontainsCopySourceSSECustomer*,SSECustomer*,MetadataDirective, etc.Object.copy(ExtraArgs=...)validates againstALLOWED_COPY_ARGSand raises on anything else, so the prefilter is required for that path.Object.copy_from(**kw)forwards directly to theCopyObjectAPI without validation; using the same allow-list is conservative but safe.Config(s3={"addressing_style": "virtual"})is the documented botocore knob.Closes #500
Closes #527
Contributor checklist:
CONTRIBUTING.mdCloses #issueappears in the PR summary (e.g.,Closes #123).HISTORY.mdwith the issue that is addressed and the PR you are submitting. If the top section is not## UNRELEASED, then you need to add a new section to the top of the document for your change.Note on live testing: the changes ship with two live-server-only tests (
test_sse_c_copy_and_move_live,test_addressing_style_virtual_live) that exercise both fixes end-to-end against a real S3 bucket. They have not been run from this branch yet againstLIVE_S3_BUCKET; maintainers runningUSE_LIVE_CLOUD=1 make test-live-cloudwill cover that gap.