fix(core): byte-faithful, consistently validated object keys across all backend paths

[alukach]

Problem

The proxy builds backend request paths in three places, and they disagreed on what a logical key means:

1. Presigned CRUD (build_object_path) used object_store::path::Path::from, which percent-encodes characters object_store deems unsafe (*, %, ~, #, ?, [, ], ...) into the logical path, silently renaming objects on the backend. Verified live against MinIO:
   • Presigned PUT of star*.txt creates backend object star%2A.txt — a silent rename invisible to the uploader (self-round-trip works) but visible in listings, to direct bucket consumers, and to any list-then-fetch client (404).
   • A multipart-written mp*.txt (true key, post-fix(core): percent-encode object keys in raw-signed backend URLs #105) is unreadable and undeletable through the presigned path (GET → NoSuchKey, DELETE → silent 204 no-op).
   • 100%.txt and 100%25.txt can alias to the same backend object: a GET then returns the wrong file's contents with a 200.
2. Anonymous CRUD (UnsignedUrlSigner, used for credential-less backends) spliced the raw key into a URL string with no encoding: a key holding a literal %3D decoded to = on the backend, and once keys are byte-faithful, #/? would truncate the wire path at the fragment/query boundary.
3. Raw-signed multipart / aws-chunked (build_backend_url) accepted degenerate keys the other paths can't address: a//b.txt written via multipart is unreadable and undeletable via presigned and breaks every listing page that covers it (object_store fails parsing listed keys with empty segments), and a literal .. segment survives to url::Url::parse normalization — on runtimes that don't WHATWG-normalize request paths (e.g. the axum server example; CF Workers normalizes and is safe), that retargets the signed backend request across buckets.

Fix

• build_object_path: Path::from → Path::parse. Byte-faithful; object_store's URL builder then percent-encodes the raw path exactly once at the wire boundary.
• UnsignedUrlSigner: percent-encode the key with the same strict set the signed builders use (shared S3_PATH_ENCODE_SET).
• New validate_key, called once in build_s3_operation for every keyed operation: keys with empty, ., or .. segments (including leading/trailing slashes, e.g. dir/ folder markers) or ASCII control characters return 400 InvalidRequest instead of being silently rewritten (Path::from collapsed a//b → a/b; Path::parse strips dir/ → dir, so a DELETE of a/ deleted a) or written where other paths can't address them. build_backend_url enforces the same rule as a backstop for hand-built operations. Real S3 accepts such keys; the proxy is deliberately stricter — documented in docs/reference/operations.md. Batch-delete body keys are exempt: they never enter a URL path, and permissiveness there is the remediation route for legacy degenerate keys.

All three builders now emit byte-identical wire paths that percent-decode back to exactly the logical key — and agree on the rejected set.

Tests

• Contract (crates/core/tests/key_encoding_contract.rs): for a corpus covering every character class that has diverged before (= from fix(core): percent-encode object keys in raw-signed backend URLs #105, spaces, */%/~/#, unicode, literal %3D), all three builders emit byte-identical wire paths that decode back to the logical key. Loud alarm if an object_store upgrade changes its encoding.
• Unit: byte-faithful round-trip and prefix application; degenerate keys → InvalidRequest for every keyed operation; hand-built degenerate operations can't reach a backend URL.
• Integration (MinIO): the backend object carries the exact requested key on presigned PUT (verified with a direct-to-MinIO client); 100%.txt vs 100%25.txt stay distinct objects; multipart write + presigned GET agree on a * key; a//b and dir/ → 400 on both single PUT and multipart create. (.. segments are collapsed by WHATWG URL parsing at the CF Workers edge before the proxy sees them; the unit tests cover those.)

⚠️ Migration notes

1. Previously mangled names stay mangled. Objects stored under percent-encoded names by earlier versions (star%2A.txt) keep those literal backend names: after this change they're addressable by that literal name rather than the original logical one. If any production data contains keys with the affected characters, a one-time rename sweep is needed. The affected character set is rare in real keys, and multipart uploads never produced mangled names (pre-fix(core): percent-encode object keys in raw-signed backend URLs #105 they failed outright for every affected character except ~, whose keys were stored byte-faithfully), so the exposure is bounded to presigned single PUTs.
2. New 400s. Keys with empty, ., or .. segments, leading/trailing slashes (dir/ folder markers), or control characters now return 400 InvalidRequest on every keyed operation. Real S3 accepts these keys; previously the proxy either silently rewrote them to a different key or wrote objects its other paths couldn't address. Batch delete still accepts them in its body, as the remediation path.

🤖 Generated with Claude Code

[github-actions]

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

• Date: 2026-07-10T21:59:35Z
• Commit: a9dbc87

[github-actions]

📖 Docs preview deployed to https://multistore-docs-pr-108.development-seed.workers.dev

• Date: 2026-07-10T21:59:35Z
• Commit: a9dbc87

[claude]

Claude finished @alukach's task in 3m 22s —— View job

---

✅ No blocking issues — safe to merge.

Reviewed the full diff (gh pr diff 108) across all 7 changed files: crates/core/src/api/request.rs, crates/core/src/backend/multipart.rs, crates/core/src/backend/url_signer.rs, crates/core/src/proxy.rs, crates/core/tests/key_encoding_contract.rs, docs/reference/operations.md, tests/integration/test_integration.py.

Traced the full key lifecycle: decode at the edge (crates/cf-workers/src/request.rs — unchanged, confirms keys reaching build_s3_operation are already percent-decoded, so validate_key's control-char/segment checks operate on the logical key as intended) → validate_key at parse time (crates/core/src/api/request.rs:59-79) → the three builders (build_object_path, build_backend_url, UnsignedUrlSigner::signed_url). Checked:

• validate_key's empty-key-first-check (request.rs:60) correctly short-circuits before .split('/'), so bucket-level ops (key == "") aren't misflagged by the [""] split result.
• Segment/control-char logic correctly catches a//b, /a, dir/, a/./b, a/../b, bare ./.., and control bytes (byte-level check is safe against UTF-8 continuation bytes, which are always ≥0x80).
• build_backend_url's backstop call (multipart.rs:58) runs before the key is prefixed/spliced, closing the hand-built-operation gap described in the PR body.
• UnsignedUrlSigner now percent-encodes with the same S3_PATH_ENCODE_SET shared from multipart.rs, and only ever receives a Path that already passed Path::parse/validate_key, so it isn't a separate bypass path.
• New tests (unit, contract, integration) look correctly wired to real helpers/imports already present in the files.

No correctness bugs, security issues, or best-practice problems found.

Simplify (ponytail)

• crates/core/src/api/request.rs:65-68 — validate_key hand-rolls the empty/./.. segment check that object_store::path::Path::parse already performs (per this PR's own doc comment at proxy.rs:1557-1561). Delegating to Path::parse(key).is_err() plus the leading/trailing-slash check would avoid maintaining a second implementation of a rule object_store owns — the exact kind of two-implementations-drift this PR is fixing elsewhere.

---

💰 Estimated review cost: $1.31 · 3m21s · 35 turns