fix(copy): authorize a versioned copy-source against its version

[alukach]

What I'm changing

x-amz-copy-source may carry ?versionId=, and that version is faithfully copied. But the synthetic GetObject the gateway builds to authorize the source carried no version — and a plain GET through the proxy drops ?versionId= entirely, serving the current object.

That combination made CopyObject the only operation able to address a non-current object version, and it did so without the registry ever being told which version. A caller with read access on a product could copy an older version into a location they control and read it back. For a deployment that treats "overwritten" as "no longer readable", there was no way to refuse: the version never reached the policy.

Found while reviewing the authorization path for #128. It arrived with the CopyObject feature (#121), not with that PR, so it is split out here rather than folded into a bugfix.

How I did it

• crates/core/src/types.rs — S3Operation::GetObject gains version: Option<String>: the version the backend read will actually return, None for current. The doc comment states the invariant that keeps the field meaningful and tells registries to deny a Some(_) they do not recognize rather than ignore it — it names bytes that are otherwise unreachable through the proxy.
• crates/core/src/proxy.rs — the copy's source authorization carries src_version, so the read that gets authorized is the read that gets performed.
• crates/core/src/api/request.rs — a plain GET leaves version: None even when the client sends ?versionId=. This is deliberate: the read path does not address versions, so marking it versioned would describe a read that never happens, and would let a policy authorize v1 while the backend serves current. The field describes the read, not the request shape.
• docs/reference/operations.md — documents both halves.

Alternative considered

Rejecting ?versionId= on a copy-source outright (501) would also close the hole and is a smaller diff, but it removes a working capability and still leaves version-scoped policy inexpressible. Threading it through keeps the capability and puts the decision where it belongs — with the registry.

Test plan

Four tests. Verified the two that pin the hole fail when src_version is not threaded through (version: None in its place):

• copy_source_version_reaches_the_source_authorization — the registry sees Some("v42") on the source read

• registry_can_deny_a_versioned_copy_source — a non-version-aware policy can refuse, and the refusal precedes any backend call, so no versioned bytes move

• unversioned_copy_source_authorizes_an_unversioned_read — an ordinary copy is unaffected by a version-denying policy

• plain_get_with_version_id_authorizes_an_unversioned_read — pins the deliberate None on the read path

• 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

• Stacked on fix(copy): resolve CopyObject sources on path-mapped and OIDC deployments #128 (fix/copyobject-path-mapping), which it shares test scaffolding with. Review or merge that one first; I'll rebase onto main if you'd rather take this independently.
• Breaking: adding a field to S3Operation::GetObject breaks external construction sites. Six in-tree sites updated; downstream custom resolvers (e.g. Source Cooperative) will need version: None added.
• Not addressed here: a plain GET with ?versionId= still silently returns the current object instead of erroring. That is a read-path correctness question rather than an authorization one, and changing it would alter behavior for existing clients — happy to take it separately if you want it.

🤖 Generated with Claude Code

[claude]

Claude finished @alukach's task in 2m 21s —— View job

---

• Gather context (diff, changed files)
• Review diff for correctness/security/best-practice issues
• Apply ponytail-review for over-engineering
• Post final review

---

✅ No blocking issues — safe to merge.

The fix closes the hole cleanly: src_version now flows from CopyObject into the synthetic GetObject used to authorize the source (proxy.rs:1084-1087), S3Operation::action() maps version: Some(_) to a distinct Action::GetObjectVersion (types.rs:472-475), and auth::authorize's anonymous-access branch only ever grants GetObject | HeadObject | ListBucket (authorize.rs:35-40) — so a versioned read is deny-by-default for both anonymous and scoped callers unless get_object_version is explicitly listed. Traced all four call sites of S3Operation::GetObject { .. } construction (types.rs, proxy.rs, api/request.rs, authorize.rs) — all updated, compiler-enforced by the field addition. The plain-GET-ignores-?versionId= behavior is unchanged and correctly left out of scope.

One non-blocking documentation nit:

• docs/configuration/roles.md:95 — the new row describes get_object_version as covering "GET/copy of a specific object version," but today only the copy-source read exercises this action; a plain GET with ?versionId= still silently serves the current object and never reaches this action (as the PR body itself notes under "Not addressed here"). Worth a parenthetical ("currently only exercised by copy-source reads; plain GET does not yet honor ?versionId=") so a reader configuring roles today doesn't infer that granting this action enables version-scoped GET.

Simplify (ponytail)

Nothing to cut — the diff is a tight, minimal fix. The RecordingRegistry test-helper additions (versions field, deny_versioned_reads bool) are proportionate to the four new tests they support, Action::GetObjectVersion isn't a speculative abstraction (it mirrors a real S3 action and is the mechanism that makes the fix enforceable rather than advisory), and no new dependencies or reinvented stdlib are present.

---

💰 Estimated review cost: $1.01 · 2m21s · 35 turns

[github-actions]

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

• Date: 2026-07-27T21:47:56Z
• Commit: c884d03

[github-actions]

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

• Date: 2026-07-27T21:47:56Z
• Commit: c884d03

[alukach]

Both findings are correct. Fixed in f2e1e6f.

The main finding

Verified before acting: static-config/src/lib.rs:215 does call auth::authorize, and that function matches on action + bucket + prefix with no reference to version. So the doc I added told registries to deny a Some(_) they don't recognize while the reference implementation shipped in this repo did exactly the ignoring it warned against. The scenario in the description was still open on the bundled registry. Good catch.

I fixed it differently from the suggested snippet, and it's worth saying why. A hardcoded return Err(AccessDenied) for any versioned read closes the hole, but it also makes version-scoped reads unreachable on the bundled registry — no configuration could enable them. That is fine while CopyObject is the only producer of a versioned read, but #130 makes plain GET/HEAD honor ?versionId=, and a blanket denial would land that feature dead on arrival for exactly the deployments the crate is meant to serve.

Instead a version-scoped read now resolves to its own action:

S3Operation::GetObject { version: None, .. } => Action::GetObject,
S3Operation::GetObject { version: Some(_), .. } => Action::GetObjectVersion,

The existing action match in authorize then does the work — a scope granting get_object doesn't match, so the default is deny, and no new branch is needed. This mirrors S3's own split between s3:GetObject and s3:GetObjectVersion, which exists for this reason: reading what an object used to be is a distinct privilege from reading what it is. And unlike a hardcoded rejection, a deployment that wants versioned reads can grant get_object_version.

Consequences, stated rather than buried:

• Anonymous callers can never read a previous version. The anonymous grant in authorize covers GetObject/HeadObject/ListBucket only, and GetObjectVersion is deliberately not added. A public bucket serving previous versions anonymously needs a custom registry.
• Existing configs keep working. They simply don't carry the new action, which is the safe direction — no config silently gains a capability.

Three tests, mutation-checked (they fail if action() maps versioned reads back to GetObject): a get_object holder is denied a versioned read, a get_object_version holder is allowed inside their prefix and denied outside it, and anonymous is denied on an anonymous-readable bucket. Also updated the action table in docs/configuration/roles.md and the copy section in docs/reference/operations.md.

Simplify

Agreed — VersionRecordingRegistry duplicated the whole trait impl to capture one field. Merged into RecordingRegistry as a versions: Arc<Mutex<Vec<Option<String>>>>, and the second type is gone.

#130 has been rebased on this and updated: a plain versioned GET now authorizes as get_object_version too, which is documented there.

[alukach]

Fixed in dbde69c, and thanks — this was more than a stale sentence.

That paragraph was supposed to have been rewritten in f2e1e6f alongside the Action::GetObjectVersion change. The edit silently applied nowhere: I wrote the replacement against wording the file didn't hold, so it matched nothing and passed without complaint. roles.md got its table row, operations.md kept the original text, and the result read exactly as you describe — advice to registry implementors for a rule the bundled policy now enforces itself. Same miss on #130, which had inherited the paragraph; both are corrected, and I checked for other copies of the old framing (none).

The paragraph now says what ships:

…which makes the authorization action get_object_version rather than get_object — mirroring S3's own split between s3:GetObject and s3:GetObjectVersion. This is not merely advice to registry implementors: the bundled policy (auth::authorize, which multistore-static-config calls) enforces it, because a version-scoped read resolves to its own action. A caller granted get_object over a prefix therefore cannot read — or copy out — an older version of an object in it, and an anonymous caller never can; the scope must list get_object_version explicitly. Prefix-scoped policy cannot express "may read version X", so the default is to deny rather than silently permit. A registry with its own policy should draw the same distinction.

It links the roles action table, so the two halves you noted were documented separately now point at each other.

#130 rebased on top; its copy paragraph carries the same text, and its "Version-scoped reads" section already stated the get_object_version requirement for plain reads. Full suite green on both (285 / 290).