From 63e635a98c32873f890ee5ea545a10fadc588a8a Mon Sep 17 00:00:00 2001 From: Graeme Foster <80714+GraemeF@users.noreply.github.com> Date: Wed, 19 Aug 2026 16:02:07 +0100 Subject: [PATCH] An upload is claimed against the message sender, so it must go out under the bound bot (comms-qpup) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `uploadFile` posted `/api/v1/user_uploads` through `minterHttp` while `post` went out through `boundHttp()`. Zulip's `do_claim_attachments` validates each attachment against the MESSAGE SENDER, so the claim was skipped, the row that grants read access was never written, and the message sent anyway. The link rendered and nobody could open it — including the realm owner. `uploadFile` now runs through `boundHttp()`, which put it on the mint seam with the other attribution-producing verbs and pulled four things after it: - `BotHttp` had no `uploadRaw`, so the bound client could not upload at all. Widened by that one member, passed through `wrapBotHttp` untouched. The wall that wrapper enforces is a recipient rule on `POST /messages`, and an upload addresses no recipient. `bot-dm-guard.test.ts` now asserts the same wrapped client that just uploaded still refuses a bot-to-bot direct message. - `upload_file` became a binding tool, so it takes the host-supplied `session_id` and joins the PreToolUse matcher. Without it a seat whose first commy call is an upload gets `UnboundEphemeralSession` instead of a file. - `hooks-manifest.test.ts` traced a tool's bound path through `adapter.publisher.*` and `adapter.inbox.*` only. `upload_file` reaches the adapter through a dep closure, so the guard would have reported coverage it did not have. It now resolves the dep alias and covers the attachment receiver too. - A live test in `realm.live.test.ts` is the proof: bot A uploads and posts a four-byte file; a second freshly-minted bot — owns nothing, subscribed to nothing — reads it back. Measured 403 before the change, bytes after. The reader is a second bot rather than `adapter.downloadFile` because `downloadFile` reads through the minter, and the minter is subscribed to every channel on this realm. Its read succeeds on a ground this change does not touch, so it cannot tell a claimed attachment from an unclaimed one. `downloadFile` stays on `minterHttp`, so a seat that only ever reads still never binds. That now rests on two pieces of realm state this repo does not control — no invite-only channels, and the minter's blanket subscription — and the call site records both, says plainly that neither is established by a discriminating test, and names what would discriminate. --- clients/claude-code/hooks-manifest.test.ts | 176 +++++++++++++++++++-- clients/claude-code/hooks/hooks.json | 2 +- docs/self-hosting.md | 7 +- packages/mcp/tools.ts | 13 +- packages/zulip/adapter.ts | 35 +++- packages/zulip/bot-dm-guard.test.ts | 38 +++++ packages/zulip/bot-dm-guard.ts | 12 +- packages/zulip/realm.live.test.ts | 69 ++++++++ 8 files changed, 329 insertions(+), 23 deletions(-) diff --git a/clients/claude-code/hooks-manifest.test.ts b/clients/claude-code/hooks-manifest.test.ts index ccfd326..037dc48 100644 --- a/clients/claude-code/hooks-manifest.test.ts +++ b/clients/claude-code/hooks-manifest.test.ts @@ -72,6 +72,7 @@ const BOUND_HTTP_CALLERS = [ 'subscriptions', 'unreact', 'unsubscribe', + 'uploadFile', ] as const /** @@ -95,6 +96,37 @@ const BOUND_HTTP_CALLERS = [ */ const BOUND_INBOX_VERBS = ['subscribe', 'subscriptions', 'unsubscribe'] as const +/** + * Adapter ATTACHMENT verbs that reach `boundHttp` (comms-qpup). An upload + * writes an `Attachment` row with an owner, and Zulip grants a reader access to + * it only when that owner is also the sender of the referencing message — so + * the upload goes out under the seat's own bot, like every other write. + * + * Held apart from the two sets above for the same reason they are held apart + * from each other: the tool layer reaches these through a THIRD receiver. It + * never names `adapter.uploadFile` — it receives a closure as a dep — which is + * why the trace needs {@link ATTACHMENT_DEP_ADAPTER_MEMBER} to see this path at + * all. + */ +const BOUND_ATTACHMENT_VERBS = ['uploadFile'] as const + +/** + * Which adapter member backs each attachment dep in `RegisterToolsDeps`, as + * `server.ts` wires them. + * + * PINNED, NOT PARSED, and the limit of that is worth stating: nothing here + * reads `server.ts`, so a rewire of `upload` onto a different member would + * leave this map lying. Every other pin in this file carries the same limit + * (see `BOUND_VERBS`), and resolving a closure across a second source file is a + * different kind of test than this one. What the map buys is the thing that was + * missing: a tool whose only route to `boundHttp` is a dep now sits INSIDE the + * compared set instead of outside it. + */ +const ATTACHMENT_DEP_ADAPTER_MEMBER: Readonly> = { + upload: 'uploadFile', + downloadFile: 'downloadFile', +} + /** * Tools that reach `boundHttp` through an inbox verb while sitting outside the * matcher, so the hook never stamps them and the bind seam sees no session id. @@ -126,9 +158,10 @@ const G5ZH3_MATCHER_PENDING = [] as const const TWW6_EXCEPTIONS = ['resolve_thread', 'set_channel_description', 'unresolve_thread'] as const /** - * Every tool that accepts a host-supplied `session_id`. SEVEN, which is wider - * than the five in the PreToolUse matcher: `subscribe` and `unsubscribe` are - * here for a non-CC ephemeral host that supplies the UUID itself, and a + * Every tool that accepts a host-supplied `session_id`. EIGHT, the same eight + * the PreToolUse matcher stamps today — but the two sets answer different + * questions and are allowed to diverge again: `subscribe` and `unsubscribe` + * are here for a non-CC ephemeral host that supplies the UUID itself, and a * listen-first seat reaches an identity through no other tool. Do not read a * statement about one of these sets as a statement about the other. * @@ -143,6 +176,7 @@ const SESSION_ID_RECEIVING_TOOLS = [ 'subscribe', 'unreact', 'unsubscribe', + 'upload_file', ] as const /** Enclosing declaration names in the adapter source that call `boundHttp()`. */ @@ -170,15 +204,35 @@ function adapterVerbsReachingBoundHttp(source: string): ReadonlySet { interface ToolFacts { readonly verbs: ReadonlySet readonly inboxVerbs: ReadonlySet + readonly attachmentVerbs: ReadonlySet readonly receivesSessionId: boolean readonly advertisesSessionId: boolean } /** - * Per-tool: which publisher verbs and which inbox verbs its handler calls, - * whether it accepts a host-supplied `session_id`, and whether it advertises - * one on its `inputSchema`. The last is traced only so the assertions can show - * it is nowhere — the two facts are separate and stay separately measured. + * `const = deps.` bindings in the tools source — the hop that + * hides an attachment tool's adapter path from a line-wise scan. `upload_file`'s + * handler calls `upload(path)`; only this binding says `upload` is + * `deps.upload`, and only {@link ATTACHMENT_DEP_ADAPTER_MEMBER} says + * `deps.upload` is `adapter.uploadFile`. + */ +function depAliases(source: string): ReadonlyMap { + const aliases = new Map() + for (const line of source.split('\n')) { + const m = line.match(/^ {4}const ([a-zA-Z]+) = deps\.([a-zA-Z]+)$/) + const alias = m?.[1] + const field = m?.[2] + if (alias !== undefined && field !== undefined) aliases.set(alias, field) + } + return aliases +} + +/** + * Per-tool: which publisher verbs, inbox verbs and attachment verbs its handler + * reaches, whether it accepts a host-supplied `session_id`, and whether it + * advertises one on its `inputSchema`. The last is traced only so the + * assertions can show it is nowhere — the two facts are separate and stay + * separately measured. */ function toolFactsFromToolsSource(source: string): ReadonlyMap { const facts = new Map< @@ -186,10 +240,12 @@ function toolFactsFromToolsSource(source: string): ReadonlyMap inboxVerbs: Set + attachmentVerbs: Set receivesSessionId: boolean advertisesSessionId: boolean } >() + const aliases = depAliases(source) let current: string | undefined for (const line of source.split('\n')) { const named = line.match(/^ {6}name: '([a-z_]+)',$/)?.[1] @@ -198,6 +254,7 @@ function toolFactsFromToolsSource(source: string): ReadonlyMap [name, { ...e, verbs: e.verbs, inboxVerbs: e.inboxVerbs }]), + [...facts].map(([name, e]) => [ + name, + { ...e, verbs: e.verbs, inboxVerbs: e.inboxVerbs, attachmentVerbs: e.attachmentVerbs }, + ]), ) } @@ -266,7 +339,7 @@ test('the set of adapter declarations reaching boundHttp is the pinned one', asy // them reads "no tool violates this", which a scan that matches nothing // satisfies trivially — so assert first that the scan finds the set it is // supposed to find. -test('the tools accepting a host-supplied session_id are exactly the pinned seven', async () => { +test('the tools accepting a host-supplied session_id are exactly the pinned eight', async () => { const facts = toolFactsFromToolsSource(await toolsSource()) const receiving = [...facts] .filter(([, f]) => f.receivesSessionId) @@ -312,6 +385,55 @@ test('every tool whose adapter path reaches boundHttp is in the PreToolUse match expect(missing).toEqual([]) }) +// The same rule again, over the ATTACHMENT verbs that began binding with +// comms-qpup. Its own assertion with its own named set, for the reason the +// inbox rule has one: a rule that compares over a set which no longer covers +// every binding path goes green by not looking. +test('every tool whose adapter path reaches boundHttp via an attachment dep receives session_id', async () => { + const facts = toolFactsFromToolsSource(await toolsSource()) + const offenders = [...facts] + .filter( + ([, f]) => + [...f.attachmentVerbs].some((v) => + (BOUND_ATTACHMENT_VERBS as ReadonlyArray).includes(v), + ) && !f.receivesSessionId, + ) + .map(([name]) => name) + .sort() + expect(offenders).toEqual([]) +}) + +test('every tool whose adapter path reaches boundHttp via an attachment dep is in the matcher', async () => { + const facts = toolFactsFromToolsSource(await toolsSource()) + const matched = alternationToolsFromMatcher(injectSessionIdMatcher(hooksManifest)) + const missing = [...facts] + .filter(([, f]) => + [...f.attachmentVerbs].some((v) => + (BOUND_ATTACHMENT_VERBS as ReadonlyArray).includes(v), + ), + ) + .map(([name]) => name) + .filter((name) => !matched.has(name)) + .sort() + expect(missing).toEqual([]) +}) + +// The pin that keeps the two rules above from holding by not looking. The +// attachment trace runs through an alias hop, so it has more ways to stop +// matching than the other two — assert it still finds the tool it is for. +test('the tools reaching a bound attachment verb are exactly upload_file', async () => { + const facts = toolFactsFromToolsSource(await toolsSource()) + const reaching = [...facts] + .filter(([, f]) => + [...f.attachmentVerbs].some((v) => + (BOUND_ATTACHMENT_VERBS as ReadonlyArray).includes(v), + ), + ) + .map(([name]) => name) + .sort() + expect(reaching).toEqual(['upload_file']) +}) + // The same rule, stated over the INBOX verbs that began binding with // comms-g5zh.2/.3. Kept as its own assertion with its own named list so the // publisher-side rule above cannot go green on a set that no longer covers @@ -354,12 +476,15 @@ test('the matcher carries no tool that never reaches boundHttp and never binds', .filter((name) => { const f = facts.get(name) if (f === undefined) return true - // Either receiver counts. A tool binds through the publisher verbs or - // through the inbox verbs; asking only about the first would call a - // legitimately-stamped `subscribe` an orphan. + // Any receiver counts. A tool binds through the publisher verbs, the + // inbox verbs or an attachment dep; asking only about the first would + // call a legitimately-stamped `subscribe` an orphan. return ( ![...f.verbs].some((v) => (BOUND_VERBS as ReadonlyArray).includes(v)) && - ![...f.inboxVerbs].some((v) => (BOUND_INBOX_VERBS as ReadonlyArray).includes(v)) + ![...f.inboxVerbs].some((v) => (BOUND_INBOX_VERBS as ReadonlyArray).includes(v)) && + ![...f.attachmentVerbs].some((v) => + (BOUND_ATTACHMENT_VERBS as ReadonlyArray).includes(v), + ) ) }) .sort() @@ -398,17 +523,25 @@ test('toolFactsFromToolsSource attributes verbs and session_id to the enclosing session_id: sessionIdField, }, }, + const upload = deps.upload + name: 'epsilon', + hostSuppliedArgs: hostSuppliedSessionId, + handler: async (args) => { + const result = await run(upload(path)) + }, ` const facts = toolFactsFromToolsSource(synthetic) expect(facts.get('alpha')).toEqual({ verbs: new Set(['post']), inboxVerbs: new Set(), + attachmentVerbs: new Set(), receivesSessionId: true, advertisesSessionId: false, }) expect(facts.get('beta')).toEqual({ verbs: new Set(), inboxVerbs: new Set(), + attachmentVerbs: new Set(), receivesSessionId: false, advertisesSessionId: false, }) @@ -417,17 +550,32 @@ test('toolFactsFromToolsSource attributes verbs and session_id to the enclosing expect(facts.get('gamma')).toEqual({ verbs: new Set(), inboxVerbs: new Set(['subscribe']), + attachmentVerbs: new Set(), receivesSessionId: false, advertisesSessionId: false, }) // The advertise-side trace catches a schema property coming back, and does - // not confuse it with the accept-side marker. + // not confuse it with the accept-side marker. `delta` also sits immediately + // before the `const upload = deps.upload` binding, so its empty + // `attachmentVerbs` is what proves the binding line is not attributed to + // whichever tool the scan is currently inside. expect(facts.get('delta')).toEqual({ verbs: new Set(), inboxVerbs: new Set(), + attachmentVerbs: new Set(), receivesSessionId: false, advertisesSessionId: true, }) + // The alias hop, end to end: `upload` resolves through the binding to + // `deps.upload`, and `ATTACHMENT_DEP_ADAPTER_MEMBER` resolves that to + // `adapter.uploadFile` — the member the tool source never names. + expect(facts.get('epsilon')).toEqual({ + verbs: new Set(), + inboxVerbs: new Set(), + attachmentVerbs: new Set(['uploadFile']), + receivesSessionId: true, + advertisesSessionId: false, + }) }) test('alternationToolsFromMatcher splits the trailing parenthesised group', () => { diff --git a/clients/claude-code/hooks/hooks.json b/clients/claude-code/hooks/hooks.json index 21e9ec3..09f6200 100644 --- a/clients/claude-code/hooks/hooks.json +++ b/clients/claude-code/hooks/hooks.json @@ -2,7 +2,7 @@ "hooks": { "PreToolUse": [ { - "matcher": "mcp__plugin_commy_commy__(post|edit_message|react|unreact|current_identity|subscribe|unsubscribe)", + "matcher": "mcp__plugin_commy_commy__(post|edit_message|react|unreact|current_identity|subscribe|unsubscribe|upload_file)", "hooks": [ { "type": "command", diff --git a/docs/self-hosting.md b/docs/self-hosting.md index 739456f..41af7ff 100644 --- a/docs/self-hosting.md +++ b/docs/self-hosting.md @@ -244,9 +244,10 @@ Two supported ways to avoid it, both already documented above: - Pass a UUID `session_id` in the tool-call arguments, which is the binding an ephemeral non-CC host has (`docs/claude-channel-inbound-contract.md`). The tools accepting it are `post`, `edit_message`, `react`, `unreact`, - `current_identity`, `subscribe` and `unsubscribe`. It is **not** advertised on - their `inputSchema` — a session id is the host's to supply, not something a - model could fill, so it is documented here rather than shown to the agent. + `current_identity`, `subscribe`, `unsubscribe` and `upload_file`. It is + **not** advertised on their `inputSchema` — a session id is the host's to + supply, not something a model could fill, so it is documented here rather + than shown to the agent. Anything that fails UUID validation is treated as missing: the server returns the unbound-stub error rather than minting a malformed `cc-*` identity. diff --git a/packages/mcp/tools.ts b/packages/mcp/tools.ts index 09a5635..c6e3e49 100644 --- a/packages/mcp/tools.ts +++ b/packages/mcp/tools.ts @@ -505,9 +505,10 @@ const buildToolDefs = (deps: RegisterToolsDeps, cache: InternalCache): ReadonlyA const projectForCwd = deps.projectForCwd ?? (() => Effect.succeed(undefined)) /** * The tools that accept a host-supplied `session_id` (see - * {@link ToolDef.hostSuppliedArgs}). SEVEN tools carry it, not the five in - * Claude Code's PreToolUse matcher: `subscribe` and `unsubscribe` are here - * for the non-CC ephemeral host that supplies the UUID itself, which is a + * {@link ToolDef.hostSuppliedArgs}). EIGHT tools carry it. That is the same + * eight Claude Code's PreToolUse matcher stamps today, but the two sets are + * separate questions and are allowed to differ: this one also serves the + * non-CC ephemeral host that supplies the UUID itself, which is a * listen-first seat's only route to an identity. A host stamping the arg on * a tool CC never stamps must not be told the argument is unknown. * @@ -1293,10 +1294,16 @@ const buildToolDefs = (deps: RegisterToolsDeps, cache: InternalCache): ReadonlyA type: 'string', description: 'Absolute path to the local file to upload (e.g. /tmp/chart.png)', }, + cwd: cwdField, }, required: ['path'], additionalProperties: false, }, + // An upload binds (comms-qpup). The file it writes carries an owner, and + // Zulip grants readers access to it only when that owner is also the + // account that sends the referencing message — so the upload has to go out + // under the calling session's own bot, which needs the session id here. + hostSuppliedArgs: hostSuppliedSessionId, handler: async (args) => { const run = runFor(args) const { path } = await run(Schema.decodeUnknown(UploadFileArgs)(args)) diff --git a/packages/zulip/adapter.ts b/packages/zulip/adapter.ts index a24f68c..6e5347e 100644 --- a/packages/zulip/adapter.ts +++ b/packages/zulip/adapter.ts @@ -2194,6 +2194,31 @@ export const zulipAdapter = ( inbox, history, directory, + // Stays on the MINTER, so a seat that only ever reads never binds. Once + // `uploadFile` moved to the bound bot below, the minter stopped owning the + // files it reads, so this now rests on two grounds — NEITHER OF WHICH THIS + // REPO CONTROLS, and both of which are realm state rather than code. + // + // Zulip's `validate_attachment_request` admits a reader on the first of + // three tests it passes: owns it, `is_realm_public`, or holds a + // `UserMessage` for a message referencing it. The two grounds map to the + // last two. `is_realm_public` is stamped onto the `Attachment` row at claim + // time from `stream.is_public()`, so it holds while no channel is + // invite-only. The `UserMessage` test holds while the minter is subscribed + // to the channel the attachment was posted in — measured by + // `cc-homelab-cbb5b9c3` on 2026-08-19 as 1194 active subscriptions against + // 1182 non-deactivated channels, i.e. everything. + // + // BOTH GROUNDS ARE ARGUED FROM REALM STATE AND ZULIP'S SOURCE, NOT + // ESTABLISHED BY A DISCRIMINATING TEST. They overlap completely on this + // realm, so no download run against it can say which one admitted the + // reader, or whether either would still admit it alone. What would + // discriminate: an invite-only channel the minter is NOT subscribed to, + // carrying an attachment posted by another account. Building that mutates + // the realm, so it is the operator's call, not this test suite's. + // + // If either ground goes away, this read fails and the fix is to move it + // onto `boundHttp()` — which costs the never-bind property above. downloadFile: (ref: AttachmentRef) => decodeUserUploadPath(ref).pipe( Effect.flatMap((urlPath) => @@ -2203,8 +2228,16 @@ export const zulipAdapter = ( ), Effect.mapError((cause) => new AttachmentError({ operation: 'download', cause })), ), + // An upload writes an `Attachment` row with an owner, so it belongs on + // the mint seam with the other attribution-producing verbs. It also has + // to be there for the reference to work at all: `do_claim_attachments` + // validates each attachment against the MESSAGE SENDER, so an upload + // owned by the minter and posted by the bound bot fails that check. Zulip + // logs a warning, skips the row that grants read access, and sends the + // message anyway — the link renders and nobody can open it. uploadFile: (filename: string, data: Uint8Array) => - minterHttp.uploadRaw(filename, data).pipe( + boundHttp().pipe( + Effect.flatMap((http) => http.uploadRaw(filename, data)), Effect.flatMap((upload) => decodeAttachmentRef(upload.url).pipe( Effect.map((ref) => ({ diff --git a/packages/zulip/bot-dm-guard.test.ts b/packages/zulip/bot-dm-guard.test.ts index e3184bd..dda1fe3 100644 --- a/packages/zulip/bot-dm-guard.test.ts +++ b/packages/zulip/bot-dm-guard.test.ts @@ -257,3 +257,41 @@ effectTest( expect(yield* messagesPosted(stub)).toBe(0) }), ) + +/** + * `BotHttp` carries `uploadRaw` because an upload has to go out under the same + * account that sends the referencing message — Zulip validates a message's + * attachments against the SENDER (comms-qpup). Widening a security-adjacent + * type is the move that gets accepted once and then cited for the next member, + * so the reasoning is pinned here rather than left in a PR body: THE WALL THIS + * WRAPPER ENFORCES IS A RECIPIENT RULE ON `POST /messages`, and an upload + * addresses no recipient. A member that can address one does not belong here on + * this precedent. + * + * One test, two assertions, and the second is the one that matters: the same + * wrapped client that just uploaded still refuses a bot-to-bot direct message. + */ +effectTest('uploadRaw passes through, and the DM wall still stands on the same client', () => + Effect.gen(function* () { + const stub = yield* makeStubHttpClient + yield* stub.respond('POST', '/api/v1/user_uploads', { + body: { url: '/user_uploads/2/ab/token/chart.svg', filename: 'chart.svg' }, + }) + const http = wrapBotHttp(yield* buildHttp(stub), directoryStub, SELF_ID) + + const uploaded = yield* http + .uploadRaw('chart.svg', new Uint8Array([1, 2, 3])) + .pipe(Effect.orDie) + expect(uploaded.filename).toBe('chart.svg') + expect(yield* requestsTo(stub, 'POST', '/api/v1/user_uploads')).toBe(1) + + yield* expectBotDirectMessageDefect( + http.post('/messages', sentMessageSchema, { + type: 'private', + to: JSON.stringify([OTHER_BOT_ID]), + content: 'hi', + }), + ) + expect(yield* messagesPosted(stub)).toBe(0) + }), +) diff --git a/packages/zulip/bot-dm-guard.ts b/packages/zulip/bot-dm-guard.ts index aadd1f0..789d27a 100644 --- a/packages/zulip/bot-dm-guard.ts +++ b/packages/zulip/bot-dm-guard.ts @@ -32,7 +32,14 @@ export interface RecipientDirectory { readonly byId: ReadonlyMap } -export type BotHttp = Pick +/** + * The bot-authenticated slice of {@link ZulipHttp}. `uploadRaw` is here + * because an upload writes an `Attachment` row owned by the account that + * sends the request, and Zulip's `do_claim_attachments` validates a message's + * attachments against the SENDER — so the upload has to go out under the same + * credential as the post that references it. + */ +export type BotHttp = Pick const decodeUserIds = Schema.decodeUnknownEither(Schema.parseJson(Schema.Array(Schema.Int))) @@ -99,4 +106,7 @@ export const wrapBotHttp = ( inner.patch(path, schema, body), delete: (path: string, schema: Schema.Schema, body?: ZulipParams) => inner.delete(path, schema, body), + // Nothing to guard: the wall this wrapper enforces is a recipient rule on + // `POST /messages`, and an upload addresses no recipient. + uploadRaw: (filename: string, data: Uint8Array) => inner.uploadRaw(filename, data), }) diff --git a/packages/zulip/realm.live.test.ts b/packages/zulip/realm.live.test.ts index fb70498..60d055b 100644 --- a/packages/zulip/realm.live.test.ts +++ b/packages/zulip/realm.live.test.ts @@ -712,3 +712,72 @@ describeLive('zulip live upload round-trip — zulip.example.com', () => { }), )) }) + +describeLiveChannel('zulip live attachment claim — zulip.example.com', () => { + // The regression comms-qpup exists to catch. `uploadFile` used to go out + // through the MINTER while `post` went out through the bound bot, so Zulip's + // `do_claim_attachments` — which validates each attachment against the + // MESSAGE SENDER — skipped the row that grants read access, logged a warning, + // and sent the message anyway. The link rendered and nobody could open it. + // + // WHY THE READER IS A SECOND BOT AND NOT THE ADAPTER'S OWN `downloadFile`. + // `downloadFile` reads through the minter, and the minter is subscribed to + // every channel on this realm, so it holds a `UserMessage` for the + // referencing message and its read succeeds on a ground this fix does not + // touch. That instrument cannot tell a claimed attachment from an unclaimed + // one. A FRESHLY MINTED BOT CAN: it owns nothing and is subscribed to + // nothing, so it fails the ownership test and the `UserMessage` test, and + // `is_realm_public` is its only remaining route — and that flag is stamped + // onto the `Attachment` row only when the claim SUCCEEDS. + // + // So this reads 403 before the fix and returns the bytes after it. + test( + 'a file uploaded and posted by one bot is readable by a second bot that owns nothing and is subscribed to nothing', + () => + Effect.runPromise( + Effect.gen(function* () { + const channel = decodeChannelNameSync(liveChannelName ?? '') + const thread = decodeThreadNameSync( + `cc-live-attach-${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 8)}`, + ) + // Four bytes. The upload is permanent realm state, so the test keeps + // its own footprint to the smallest thing that still proves a + // round-trip. + const bytes = new Uint8Array([0x71, 0x70, 0x75, 0x70]) + const filename = `cc-live-attach-${Date.now()}.bin` + + const owner = yield* buildAdapter() + const uploaded = yield* Effect.acquireUseRelease( + pacedAcquire(owner, decodeBotNameSync(uniqueName('attach-owner'))), + () => + Effect.gen(function* () { + const up = yield* owner.uploadFile(filename, bytes) + // The post is what triggers the claim, so it is part of the + // setup rather than an assertion — an upload nobody references + // is never claimed by anyone. + yield* owner.publisher.post( + channel, + decodeMessageBodySync(`attachment claim probe ${up.reference}`), + { thread }, + ) + return up + }), + () => pacedRelease(owner), + ) + + const reader = yield* buildAdapter() + yield* Effect.acquireUseRelease( + pacedAcquire(reader, decodeBotNameSync(uniqueName('attach-reader'))), + (acquired) => + Effect.gen(function* () { + const http = yield* botHttp(liveEnv(), credentialsOf(acquired.credentials)) + const back = yield* http.downloadRaw(uploaded.ref) + expect(new Uint8Array(back.data)).toEqual(bytes) + }), + () => pacedRelease(reader), + ) + }), + ), + 60_000, + ) +})