test(core): cross-path key-encoding contract; encode keys in unsigned URLs#109
Merged
alukach merged 2 commits intoJul 10, 2026
Merged
Conversation
… URLs Add a contract test asserting the three backend URL builders — the authenticated presigned signer, the anonymous UnsignedUrlSigner, and the raw-signed build_backend_url — emit byte-identical wire paths that percent-decode back to the logical key, over a corpus covering every character class that has diverged before (=, spaces, *, %, ~, #, unicode, literal %3D). If an object_store upgrade shifts its path encoding, this is the loud alarm. The test immediately caught a third instance of the #105 bug class: UnsignedUrlSigner spliced the raw key with no encoding, so anonymous URLs carried literal key bytes — a key holding %3D decodes to = on the backend, and a # truncates the path as a URL fragment. Encode with the same strict set as the signed builders. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
🚀 Latest commit deployed to https://multistore-proxy-pr-109.development-seed.workers.dev
|
alukach
added a commit
that referenced
this pull request
Jul 10, 2026
…ll backend paths (#108) * fix(core): percent-encode object keys in raw-signed backend URLs Multipart operations (CreateMultipartUpload, UploadPart, Complete, Abort) build their backend URL by splicing the decoded object key into a string. For key characters that url::Url leaves literal in paths but that are outside the RFC 3986 unreserved set (`=`, `!`, `(`, `)`, `:`, `@`, ...), the request went out — and was signed — with the literal byte, while S3/MinIO reconstruct the SigV4 canonical URI by strict-encoding the decoded path. The signatures never matched, so multipart uploads to Hive-style partition keys (`country_iso=ETH/...`) failed with 403 SignatureDoesNotMatch at CreateMultipartUpload. Encode the assembled prefix+key with the SigV4 strict set (unreserved chars, `/` kept as separator) before splicing. The URL string is both the signing input and the wire bytes, so the two stay byte-identical on any backend. Presigned CRUD ops already encode this way via object_store's STRICT_ENCODE_SET, which is why single PUT/GET on such keys worked. Also syncs Cargo.lock with the 0.6.3 release version bump. Reported downstream as source-cooperative/data.source.coop#180. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: state the encode-set rationale timelessly Comments described the pre-fix state ('already work', 'pre-existing'); reword to describe the invariant instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): build presigned object paths byte-faithfully Path::from percent-encodes characters object_store deems unsafe (*, %, ~, #, ...) into the logical path, so presigned CRUD silently renamed such objects on the backend (a*.bin stored as a%2A.bin) while the raw-signed multipart path stores the true key. Consequences: multipart- written keys were unreadable through GET, listings showed names that 404 on fetch, and 100%.txt / 100%25.txt could alias to one backend object — serving wrong content with a 200. Use Path::parse in build_object_path: byte-faithful, with object_store encoding the wire URL exactly once. Keys with empty or relative segments (a//b, a/../b), which Path::from silently collapsed to a different key, now return 400 InvalidRequest. Objects stored under mangled names by earlier versions keep their mangled backend names and must be addressed accordingly (or renamed). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test(core): cross-path key-encoding contract; encode keys in unsigned URLs (#109) Add a contract test asserting the three backend URL builders — the authenticated presigned signer, the anonymous UnsignedUrlSigner, and the raw-signed build_backend_url — emit byte-identical wire paths that percent-decode back to the logical key, over a corpus covering every character class that has diverged before (=, spaces, *, %, ~, #, unicode, literal %3D). If an object_store upgrade shifts its path encoding, this is the loud alarm. The test immediately caught a third instance of the #105 bug class: UnsignedUrlSigner spliced the raw key with no encoding, so anonymous URLs carried literal key bytes — a key holding %3D decodes to = on the backend, and a # truncates the path as a URL fragment. Encode with the same strict set as the signed builders. Co-authored-by: Claude Fable 5 <noreply@anthropic.com> * chore(core): apply review trims - drop dead `let key = key.as_str()` rebinding in UnsignedUrlSigner - decode one builder in the contract's decode test; byte-equality test pins the other builders to it - fix stale multipart-matrix comment claiming the presigned path still rewrites INVALID-set chars (this PR removes that behavior) - doc build_object_path's residual leading/trailing-slash stripping Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(core): validate object keys once for every keyed operation Reject keys with empty, `.`, or `..` path segments (including leading/trailing slashes) or ASCII control characters with 400 InvalidRequest at operation-parse time (build_s3_operation), for every keyed operation. The presigned path already rejected interior degenerate segments via Path::parse but silently stripped leading/trailing slashes (a DELETE of `dir/` deleted `dir`); the raw-signed multipart path accepted all of them — writing objects the presigned path can't address, breaking listings that cover them (object_store fails parsing listed keys with empty segments), and letting a literal `..` reach URL normalization, which on non-WHATWG-normalizing runtimes retargets the signed backend request across buckets. build_backend_url enforces the same rule as a backstop for hand-built operations. Batch-delete body keys stay exempt: they never enter a URL path, and permissiveness there is the remediation route for legacy degenerate keys. Documented in docs/reference/operations.md alongside the byte-faithfulness guarantee and the pre-0.6.4 mangled-name migration note. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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.
What
Adds
crates/core/tests/key_encoding_contract.rs: for a corpus of logical keys covering every character class that has diverged before (=from #105, spaces,*/%/~/#from #108, unicode, literal%3D), assert that all three backend URL builders — the authenticated presigned signer, the anonymousUnsignedUrlSigner, and the raw-signedbuild_backend_url— emit byte-identical wire paths that percent-decode back to exactly the logical key (the decode is what the backend uses to pick the object). If an object_store upgrade ever shifts its path encoding, these tests are the loud alarm.What the test caught immediately
A third instance of the #105 bug class:
UnsignedUrlSigner(anonymous buckets) spliced the raw key into the URL with no encoding. Consequences on the wire:%3Ddecodes to=on the backend — wrong object addressed;#gets truncated at the#by URL parsing (becomes a fragment);=,*, etc. go out literal while the other builders send%3D/%2A— the same cross-path split-brain fix(core): byte-faithful, consistently validated object keys across all backend paths #108 fixes for authenticated buckets.Fixed by encoding with the same strict set as the signed builders (
S3_PATH_ENCODE_SET, nowpub(crate)). Verified fail-before/pass-after: without the fix the contract test fails exactly on those keys.Tests
cargo test: full native suite green, including the 2 new contract tests.🤖 Generated with Claude Code