feat(read): honor ?versionId= on GET and HEAD

[alukach]

What I'm changing

A GET or HEAD carrying ?versionId= silently returned the current object: the parameter was dropped at parse time and never reached the backend. A client asking for a specific version got the wrong bytes with a 200 — worse than an error, because nothing signals the mismatch.

It also left CopyObject as the only operation able to reach a non-current version (the gap #129 closes on the authorization side), so the natural workaround for "read an old version" was to copy it somewhere readable and fetch it back.

How I did it

• crates/core/src/types.rs — HeadObject gains version to match GetObject. Supporting GET alone would be a sharper inconsistency than today's uniform silence: head_object(VersionId=…) would describe a different object than get_object(VersionId=…) returns.
• crates/core/src/api/request.rs — parse versionId for both. An empty ?versionId= is treated as absent rather than as a version named "": S3 has no such version, so reading it as one would convert a malformed request into a read that can never succeed.
• crates/core/src/backend/multipart.rs — build_backend_url appends ?versionId= for a version-scoped read, alongside the existing multipart query arms.
• crates/core/src/proxy.rs — build_versioned_read_forward: a versioned read is signed with an Authorization header rather than presigned, and READ_FORWARD_HEADERS is factored out of the two read arms.

Why a versioned read can't be presigned

The query string is part of a presigned request's canonical form, so versionId must be present when the signature is computed. The Signer interface presigns a bare object_store::Path with no query, so the parameter can be neither included before signing nor appended after — appending invalidates the signature. Signing with an Authorization header puts versionId in the URL before signing instead. The runtime streams the response body identically for both kinds of ForwardRequest; this reuses the same mechanism as the existing aws-chunked streaming PUT re-sign.

Client read headers (Range, If-*) are attached after signing, so they travel unsigned — matching the presigned path. That is deliberate: SigV4 only requires host and x-amz-* to be signed, and a runtime that normalizes a forwarded Range (Cloudflare may) would break a signature that covered it.

Object versioning is an S3 concept, so a version-scoped read of a non-S3 backend returns 501 NotImplemented — not the require_s3_backend 400, since the request is valid S3 and it is the backend that cannot serve it.

Test plan

Six tests. Verified the three that pin the dispatch change fail when the version is parsed but ignored at dispatch (the pre-change behavior):

• plain_get_with_version_id_parses_as_a_versioned_read — GET and HEAD both parse a version; absent and empty both mean current

• versioned_get_forwards_a_header_signed_url_carrying_the_version — versionId in the URL, Authorization header present, no X-Amz-Signature in the URL

• unversioned_get_still_uses_a_presigned_url — the ordinary path is untouched

• versioned_read_forwards_range_unsigned — Range/If-None-Match forwarded, absent from SignedHeaders, and a ranged read still bypasses the full-object cache

• versioned_read_of_a_non_s3_backend_is_rejected — 501

• versioned_get_is_authorized_against_its_version — the version reaches the registry, so a version-aware policy can refuse it

• cargo test (full workspace, all green)

• cargo clippy --all-targets (clean; one pre-existing warning in an untouched test helper)

• cargo fmt

• cargo check -p multistore-cf-workers --target wasm32-unknown-unknown

Notes for the reviewer

• This inverts a test from fix(copy): authorize a versioned copy-source against its version #129. That PR pinned the read path's version-blindness as deliberate (plain_get_with_version_id_authorizes_an_unversioned_read) — correct while reads ignored versions, since authorizing a version the backend would not return describes a read that never happens. The invariant is that version names the read that actually occurs; this PR makes it occur, so the assertion flips. Called out because a reviewer seeing a just-added test replaced deserves the reasoning.
• Stacked on fix(copy): authorize a versioned copy-source against its version #129 (which is stacked on fix(copy): resolve CopyObject sources on path-mapped and OIDC deployments #128). Happy to rebase onto main if you'd rather take these independently.
• Breaking: S3Operation::HeadObject gains a field, as GetObject did in fix(copy): authorize a versioned copy-source against its version #129. Downstream custom resolvers need version: None added.
• Cacheability difference: a versioned read's backend URL is stable across requests (the signature lives in a header), where a presigned URL varies per request. Cloudflare's subrequest cache can therefore actually hit for versioned reads. That is sound — a version is immutable, the URL is unique per bucket/key/version, and a hit only occurs after the proxy has authorized that exact object — but it is a real behavioral difference worth a reviewer's eye.
• Not addressed: ?versionId= on DELETE is still ignored (documented). Versioned delete needs MFA-delete semantics and a policy story of its own.

🤖 Generated with Claude Code

[claude]

Claude finished @alukach's task in 4m 12s —— View job

---

Reviewing PR #130

• Gather context (diff, files)
• Correctness / security review of diff
• Ponytail over-engineering pass
• Post final review

---

❌ Changes requested — see findings below.

• crates/core/src/types.rs:479 — versioned HeadObject authorizes under the same action as an unversioned one, unlike GetObject. action() gives GetObject a version-aware split (version: None → Action::GetObject, version: Some(_) → Action::GetObjectVersion, lines 475-478), but HeadObject { .. } => Action::HeadObject ignores version entirely (line 479). Since HeadObject::version is documented as having "Same contract as GetObject::version" (types.rs:382) and docs/reference/operations.md's new "Version-scoped reads" section states outright that "Such a read authorizes as get_object_version, not get_object" for both GET and HEAD — the code doesn't back that claim for HEAD.

  Concretely, this is exploitable through the production registry: crates/static-config/src/lib.rs:215 calls auth::authorize(), whose scope check is scope.actions.contains(&action) (crates/core/src/auth/authorize.rs:78). A credential holding only head_object (not get_object_version) can now issue HEAD /bucket/key?versionId=xxx and read metadata (existence, size, ETag, last-modified) of a non-current version it was never granted access to. Worse, authorize()'s anonymous-access allowlist (crates/core/src/auth/authorize.rs:37, Action::GetObject | Action::HeadObject | Action::ListBucket) already excludes GetObjectVersion for anonymous callers — but not this, so a public/anonymous-read bucket now leaks version metadata via HEAD too. This directly undermines the invariant this PR (and fix(copy): authorize a versioned copy-source against its version #129) sets up for GET.

  // crates/core/src/types.rs
  pub enum Action {
      GetObject,
      GetObjectVersion,
      HeadObject,
      HeadObjectVersion, // add
      ...
  }
  
  // action()
  S3Operation::HeadObject { version: None, .. } => Action::HeadObject,
  S3Operation::HeadObject { version: Some(_), .. } => Action::HeadObjectVersion,

  Also needs a head_object_version row in docs/configuration/roles.md (mirroring get_object_version) and a HEAD-equivalent of the versioned_get_is_authorized_against_its_version test.

No other correctness or security issues found — authorization for the GET path is wired correctly (version reaches bucket_registry.get_bucket before dispatch), the empty-versionId-as-absent handling is correct, the non-S3-backend 501 is checked before URL building, and the presigned-vs-header-signed split (and the resulting unsigned Range/If-* forwarding) matches SigV4's actual signing requirements.

No ponytail (over-engineering) findings — version_id() and READ_FORWARD_HEADERS are small, real deduplications, and build_versioned_read_forward is justified by the presigned-URL constraint it documents.

---

💰 Estimated review cost: $1.57 · 4m12s · 44 turns

[github-actions]

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

• Date: 2026-07-27T22:17:21Z
• Commit: 1e54706

[github-actions]

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

• Date: 2026-07-27T22:17:21Z
• Commit: 1e54706