Skip to content

fix(core): percent-encode object keys in raw-signed backend URLs#105

Merged
alukach merged 3 commits into
mainfrom
fix/multipart-key-encoding
Jul 10, 2026
Merged

fix(core): percent-encode object keys in raw-signed backend URLs#105
alukach merged 3 commits into
mainfrom
fix/multipart-key-encoding

Conversation

@alukach

@alukach alukach commented Jul 10, 2026

Copy link
Copy Markdown
Member

Problem

Multipart uploads to keys containing = (e.g. Hive-style partition paths like by_country/country_iso=ETH/ETH.pmtiles) fail with 403 SignatureDoesNotMatch at CreateMultipartUpload. Single PUTs on the same keys work. Reported downstream as source-cooperative/data.source.coop#180, where large unsplittable files (PMTiles) on partition paths cannot be uploaded at all (single PUT is capped by the Workers ~100 MB body limit).

Root cause

Raw-signed operations (multipart Create/UploadPart/Complete/Abort) build their backend URL in build_backend_url by splicing the decoded key into a string. url::Url leaves = — and the rest of ! ( ) : @ + , ; $ & ' — literal in paths, so the request went out and was signed with the literal byte. S3 (and MinIO — s3utils.EncodePath) reconstruct the SigV4 canonical URI by strict-encoding the decoded path (=%3D), computing a different signature.

Presigned CRUD ops already encode with object_store's STRICT_ENCODE_SET, which is why plain PUT/GET on these keys worked — the asymmetry was only in the raw-signed path.

Fix

Encode the assembled prefix + key with the SigV4 strict set (RFC 3986 unreserved chars, / kept as the segment separator) before splicing. The URL string returned by build_backend_url is both the signing input (sign_s3_request signs Url::parse(url).path(), which does not re-encode %) and the wire bytes (send_raw passes the string to the Fetch Request constructor), so signature and wire stay byte-identical — no reliance on backend-specific canonicalization. One change covers all three call sites (execute_multipart, build_streaming_forward, execute_delete_objects; the last has no key in its URL). percent-encoding was already a direct dependency.

Keys legitimately containing a literal %3D round-trip correctly (% is encoded → %253D). Query strings (uploadId, partNumber) were already handled via form_urlencoded and are untouched, as is inbound signature verification (signing_path, #103).

Tests

  • Unit (crates/core/src/backend/multipart.rs): special chars encoded + Url::parse round-trip (canonical URI == wire path), full AWS-special set with / preserved, literal-%3D double-encoding.
  • Integration (tests/integration/test_integration.py): parametrized multipart round-trip (upload via transfer manager → GET back → delete) against MinIO for a country_iso=ETH partition key and a kitchen-sink specials key.

Verified fail-before/pass-after locally with scripts/integration-test.sh:

  • Before the fix: both parametrized cases fail with SignatureDoesNotMatch at CreateMultipartUpload — exact reproduction of the downstream issue.
  • After the fix: full integration suite green (24 passed, 4 OIDC tests self-skipped locally), plus cargo test (126 tests), cargo fmt, clippy, and wasm cargo check.

Known limitation surfaced (pre-existing, out of scope)

Characters in object_store's PathPart INVALID set (*, %, ~, #, ?, |, \, {, }, [, ], <, >, ", ^) are silently rewritten in the logical key by the presigned CRUD path (Path::from encodes *%2A, so the backend object is created/fetched under the mangled key). Presigned PUT+GET round-trips are self-consistently mangled, but a byte-faithful multipart write of such a key can't be read back through the presigned GET. * is therefore excluded from the integration test's specials key (noted in a comment). Worth a follow-up issue if byte-faithful keys for those characters matter.

Also syncs Cargo.lock with the 0.6.3 release version bump (main's lock was stale; any local cargo build regenerates the same diff).

🤖 Generated with Claude Code

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>
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>
@github-actions

github-actions Bot commented Jul 10, 2026

Copy link
Copy Markdown

🚀 Latest commit deployed to https://multistore-proxy-pr-105.development-seed.workers.dev

  • Date: 2026-07-10T19:19:01Z
  • Commit: d80b460

alukach added a commit that referenced this pull request Jul 10, 2026
A first-ever workers.dev name and freshly-put secrets both propagate
eventually. The wait step passed on a single 200 from /, which needs
neither config nor secrets, so smoke tests on brand-new PR preview
workers started against a not-yet-consistent edge (bare 404s) or a
pre-secrets worker version (federation 500s) — see the first runs on
#105 and #106, which both passed untouched on rerun.

Probe /.well-known/openid-configuration instead (mounted only once
OIDC_PROVIDER_KEY is loaded, so it proves the post-secrets version is
live) and require 3 consecutive successes; fall back to / when
set_secrets is disabled. Window extended to 3 minutes.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
@alukach alukach marked this pull request as ready for review July 10, 2026 19:26
@alukach alukach merged commit c03d7a4 into main Jul 10, 2026
12 checks passed
@alukach alukach deleted the fix/multipart-key-encoding branch July 10, 2026 19:26
alukach added a commit that referenced this pull request Jul 10, 2026
… 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>
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>
alukach added a commit to source-cooperative/data.source.coop that referenced this pull request Jul 10, 2026
multistore 0.6.4 (developmentseed/multistore#105) percent-encodes the
object key with the SigV4 strict set when building backend URLs for
raw-signed multipart operations, fixing SignatureDoesNotMatch on keys
containing `=` and other reserved characters. Turns the regression
tests from the previous commit green.

Fixes #180

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant