oficina / sheets Public

The plaintext share-link token transits a GET URL Issue closed

claude agent, operated by paul@keymail.dev driven by paul@keymail.dev session a066678d · claude-code/2.1.252 · DOCS-T1

Found by the Fable review pass on core, recorded in discussion #2 and not actioned there. Re-verified present on c4a3954, which is what the canary is serving.

handleLinkCreate mints a share link and then hands the plaintext token to the browser in a redirect:

internal/sheets/handlers_link.go:145 "/share?notice=link&token="+token, http.StatusSeeOther

Everything else about the token is clean, and the review checked it: 32 bytes from crypto/rand, only the SHA-256 is persisted, unique index, constant-time compare, never in an error string, never in an app log line.

The leak is the request line itself. A query string reaches the platform edge's access logs, and AGENTS.md's own rule is that carlos logs is platform-held storage we cannot grep. So the one artefact that grants read access to a sealed sheet is written, in the clear, into a store we have no way to audit or redact. That is rule zero through a side door.

It is also the only one of the five findings that is a trust-model defect rather than a correctness or copy defect, which is why it goes first.

Fix: stash the token server-side for exactly one read and render the share page from that, or render the page directly from the POST. Either way the token must never appear in a URL.

Test that must fail first: create a link, assert the Location header carries no token, and assert the token is still shown once on the page the person lands on.

claude agent, operated by paul@keymail.dev driven by paul@keymail.dev session a066678d · claude-code/2.1.252 · DOCS-T1

Fixed on branch fix/link-token-not-in-url, commit 3b8870a, deployed to edge and verified on the process: GET /api/version returns 3b8870a.

The token now reaches the person in the response to their own POST, and renderShare no longer reads a token out of the query at all — that second half is what actually closes the channel. While the screen still echoed a token handed to it, the old URL stayed a working way to put one in an access log; a caller only had to build it.

A server-side stash was the other candidate and is worse here. A redirect landing on a second instance would find nothing, and since only the SHA-256 is stored the token would be gone with no way to recover it — a silent, permanent loss of a capability. The cost actually taken is that a resubmitted form mints a duplicate link, which is listed on the screen below and revocable from it. Visible and recoverable, against invisible and permanent.

Two tests moved, and one is deleted rather than adapted. TestTheTokenIsShownOnceAndNotAgain asserted the token was in the Location header, so it pinned the bug; it asserts the same intent against the response body now. TestTheShareScreenOnlyEchoesItsOwnToken is gone, with the reasoning left in a comment where it stood: with the query path removed, no production change could make it fail. It would have sat in the suite passing and proving nothing.

The promote-is-not-a-deploy gap measured about 130 seconds on this one — the edge header read 3b8870a on the very first poll while the process kept answering c4a3954 for thirteen consecutive polls. A single early read would have called this a failed deploy.

make test green including -race.

Closing. Not reviewed: I wrote it, and AGENTS.md §Review is explicit that a self-read is not a review pass.