fix: isolate artifact broker namespaces

[steipete]

Closes #702.

Summary

• Namespace new brokered artifact grants by authenticated organization and owner.
• Encode both identities as base64url UTF-8 under an explicit v2/org/.../owner/... key contract, preventing cross-org and owner/prefix boundary collisions.
• Preserve existing objects and URLs: this changes only newly signed upload grants and needs no legacy lookup or dual-write path.
• Preserve exact authenticated identity bytes before opaque encoding; cover cross-org,
  slash-boundary, backslash, Unicode, and composed/decomposed Unicode identities.
• Add an Unreleased changelog entry thanking @coygeek.

Verification

• npm run format:check --prefix worker
• npm run lint --prefix worker
• npm run check --prefix worker
• npm test --prefix worker -- artifacts.test.ts fleet.test.ts — 307 tests passed.
• npm run build --prefix worker — Wrangler dry-run passed.
• git diff --check
• .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --parallel-tests 'npm test --prefix worker -- artifacts.test.ts fleet.test.ts && npm run check --prefix worker' --stream-engine-output — clean, no accepted/actionable findings; final patch correct at 0.88 confidence.

The route-level integration proof shows identical owner/prefix/name tuples produce different keys across orgs, and an owner containing a path separator cannot collide with the same separator moved into the caller prefix.

Fresh-checkout remote proof

• Exact PR head 89572cbe5b8f93003385b541cc68a0ddf417d2c7 was checked out on a fresh brokered AWS c7a.8xlarge lease after merging current main.
• Run run_45dc16dc41c5 installed Node.js 22.22.1/npm 9.2.0 and passed both focused files: 2 files, 307 tests.
• Lease cbx_df7bf89adf79 finished in 1m13s and is verified released with no cleanup error.

[clawsweeper]

Codex review: needs changes before merge. Reviewed July 2, 2026, 6:12 AM ET / 10:12 UTC.

Summary
The PR changes Worker brokered artifact upload signing to use v2 base64url org/owner namespaces, updates artifact/security docs, and adds Worker regression coverage.

Reproducibility: yes. Source inspection on current main shows two authenticated POST /v1/artifacts/uploads requests with the same owner, prefix, and file name can still reach an owner-only key builder and derive the same key across orgs.

Review metrics: 3 noteworthy metrics.

• Changed surface: 8 files, +204/-11. The diff is focused but touches signer code, route wiring, docs, tests, and release-note surface.
• Artifact key contract: 1 namespace shape changed. New brokered grants move from owner-only paths to v2 encoded org/owner paths, which is the compatibility-sensitive surface.
• Proof coverage: 3 route collision cases, 307 focused tests. The contributor proof exercises the real route/broker path for the core namespace collisions and reports focused Worker tests passing.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #702
Summary: This PR is the candidate fix for the canonical artifact broker namespace security issue, while the related closed reports were consolidated into that tracker.

Members:

• canonical: [security] Artifact broker keys omit org from upload namespace #702 - The PR body uses closing syntax for this open issue, and the issue discussion defines the v2 opaque org/owner namespace implemented here.
• superseded: [security] Brokered artifact keys can collide across identity path segments #758 - This closed issue was consolidated into the canonical issue as the owner/request-prefix collision case covered by the same v2 key contract.
• duplicate: [security] Brokered artifact uploads omit org from object namespace #726 - This closed issue describes the same missing-org artifact broker namespace problem and was closed as a duplicate of the canonical tracker.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

• Move the release-note text out of CHANGELOG.md or get explicit release-owner acceptance for that file change.

Risk before merge

• [P1] Merging changes the returned prefix, key, upload URL path, and read URL path for every newly signed brokered artifact grant; consumers that treated owner-only object paths as stable must accept the new-write-only v2 contract.
• [P1] CHANGELOG.md remains touched in this normal PR even though this review workflow treats the changelog as release-owned unless a release owner explicitly accepts that file change.

Maintainer options:

1. Land the new-write-only v2 namespace (recommended)
   The canonical issue discussion records a v2 new-write-only decision, so maintainers can accept the returned key and URL shape change if broker consumers treat responses as opaque.
2. Add compatibility handling first
   If owner-only artifact keys are considered external API surface, add explicit legacy-read, translation, migration, or upgrade tests before merge.
3. Pause for key-contract design
   If the artifact object path contract is still unsettled, pause this PR and keep the canonical issue open until the migration policy is explicit.

Next step before merge

• [P2] A repair lane can handle the release-owned CHANGELOG.md cleanup; maintainer review still owns the compatibility-sensitive artifact key contract.

Security
Cleared: The diff narrows brokered artifact grant authority and adds no dependency, secret-handling, or supply-chain execution concern.

Review findings

• [P3] Move the changelog note out of this PR — CHANGELOG.md:26

Review details

Best possible solution:

Land the v2 new-write-only org/owner namespace after maintainer acceptance of the key-contract compatibility impact and the release-owned changelog handling is resolved.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection on current main shows two authenticated POST /v1/artifacts/uploads requests with the same owner, prefix, and file name can still reach an owner-only key builder and derive the same key across orgs.

Is this the best way to solve the issue?

Mostly yes. The v2 base64url org/owner namespace preserves exact identity bytes and covers both collision classes; compatibility handling only needs to expand if maintainers reject the documented new-write-only contract.

Full review comments:

• [P3] Move the changelog note out of this PR — CHANGELOG.md:26
  This review workflow treats CHANGELOG.md as release-owned for normal PRs. The PR body already carries the release-note context, so leave this file for release prep unless a release owner explicitly asks for the entry here.
  Confidence: 0.88

Overall correctness: patch is correct
Overall confidence: 0.88

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 098225a7d964.

Label changes

Label justifications:

• P2: This is a bounded authenticated artifact broker security fix with limited blast radius to deployments using brokered uploads.
• merge-risk: 🚨 compatibility: Merging changes the returned object key and URL namespace for every newly signed brokered artifact grant.
• rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
• status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): The PR body and comments include live route/broker output for the implementation path plus an exact-head fresh brokered AWS focused test run; this is sufficient real behavior proof.
• proof: sufficient: Contributor real behavior proof is sufficient. The PR body and comments include live route/broker output for the implementation path plus an exact-head fresh brokered AWS focused test run; this is sufficient real behavior proof.

Evidence reviewed

Acceptance criteria:

• [P1] git diff --check.

What I checked:

• Repository policy read: AGENTS.md was read fully; its neutral-example guidance was relevant to the test fixture review, and the latest PR head now uses alice@example.com instead of the project-specific fixture. (AGENTS.md:9, 098225a7d964)
• Current main still drops org: Current main still calls artifactUploadResponse with requestOwner(request) only, so the authenticated org does not reach brokered artifact key construction. (worker/src/fleet.ts:9395, 098225a7d964)
• Current main key builder is owner-only: Current main builds the brokered artifact prefix from config prefix, owner, and caller prefix, with no org component. (worker/src/artifacts.ts:57, 098225a7d964)
• Latest release is not fixed: v0.33.0 still has artifactUploadResponse(env, request, owner) and artifactPrefix(config.prefix, owner, request.prefix), so this PR is not obsolete because the latest release lacks the fix. (worker/src/artifacts.ts:57, 966e99599db4)
• PR head adds v2 org/owner scope: The PR head validates org and owner, then builds keys under v2/org//owner/ before the caller prefix. (worker/src/artifacts.ts:63, 89572cbe5b8f)
• PR head preserves exact auth org bytes: The PR passes requestOwner plus the raw x-crabbox-org header or configured default into the artifact signer, avoiding the lossy usage-org normalizer for key bytes. (worker/src/fleet.ts:9396, 89572cbe5b8f)

Likely related people:

• steipete: Peter Steinberger introduced brokered artifact publishing in b568298 and has the largest sampled history on the artifact/fleet/test/docs paths touched by this PR. (role: feature-history owner and recent area contributor; confidence: high; commits: b56829801970, d9b520cd4d27, c99a6dc66f94; files: worker/src/artifacts.ts, worker/src/fleet.ts, worker/test/fleet.test.ts)
• Vincent Koc: git blame attributes the current main owner-only signer, prefix builder, upload route, and brokered-upload test shape to e3e14ef, though that commit appears to be a broad import/refactor snapshot. (role: current-line provenance; confidence: low; commits: e3e14ef0588f; files: worker/src/artifacts.ts, worker/src/fleet.ts, worker/test/fleet.test.ts)
• coygeek: Coy Geek reported the canonical and related artifact namespace collision cases that this PR implements and tests against. (role: security reporter and acceptance-case provider; confidence: medium; files: worker/src/artifacts.ts, worker/src/fleet.ts, worker/test/fleet.test.ts)

What the crustacean ranks mean

• 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
• 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
• 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
• 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
• 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
• 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
• 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works

• ClawSweeper keeps one durable marker-backed review comment per issue or PR.
• Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
• A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
• PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
• Maintainers can also comment @clawsweeper review to request a fresh review only.
• Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
• Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
• Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3de6b185ee

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[steipete]

@clawsweeper re-review

Exact-head remote proof is now in the PR body: fresh brokered AWS checkout of 3de6b185ee6b35ee23a31374df03d58048bdfa46, 307 focused tests passed, and the disposable lease is verified released. Maintainer accepts the documented new-write-only v2 key contract; existing URLs remain unchanged. The changelog entry remains because repository policy requires maintainer-authored changelog coverage for user-facing fixes.

[clawsweeper]

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

[steipete]

Exact-head route/broker proof for 3de6b185ee6b35ee23a31374df03d58048bdfa46:

• Started the real Worker locally through Wrangler with an isolated Durable Object and test-only R2 signing configuration.
• Sent authenticated HTTP POST /v1/artifacts/uploads requests through the coordinator entrypoint for three collision cases; all returned 201.
• org/team and org\team produced distinct prefixes, keys, upload URL paths, and read URL paths.
• owner=alice/team,prefix=run-1 and owner=alice,prefix=team/run-1 likewise produced distinct prefixes, keys, upload URL paths, and read URL paths.
• An explicitly empty authenticated org returned 400 with artifact upload organization identity is required.

Redacted namespace output:

crabbox-artifacts/v2/org/b3JnL3RlYW0/owner/YWxpY2UvdGVhbQ/run-1
crabbox-artifacts/v2/org/b3JnXHRlYW0/owner/YWxpY2UvdGVhbQ/run-1
crabbox-artifacts/v2/org/b3JnL3RlYW0/owner/YWxpY2U/team/run-1

This exercises the real HTTP auth, route, Durable Object, signer, and response serialization path. It intentionally does not mutate an external object store. The exact head also passed the fresh brokered AWS run already recorded in the PR body, and that lease is verified released.

[steipete]

Landed as d0dedfcda2f3d5887ac0d4ae5b7c1d4bb2e6c660.

Proof on exact PR head 89572cbe5b8f93003385b541cc68a0ddf417d2c7:

• GitHub checks all green: Go, Worker, Apple VZ, Scripts, Docs, Release Check, CodeQL, and all analysis jobs.
• npm test --prefix worker -- artifacts.test.ts fleet.test.ts: 307 tests passed; typecheck passed.
• Final branch-mode autoreview: no accepted/actionable findings; patch correct at 0.88 confidence.
• Fresh brokered AWS run run_45dc16dc41c5: exact-head checkout, 307 tests passed; lease cbx_df7bf89adf79 verified released with no cleanup error.

The landed contract is new-write-only v2/org/<opaque>/owner/<opaque> namespacing. Existing object URLs remain unchanged; no dual-write or legacy lookup path was added.