oficina / suite Public

<task-notification> <task-id>a92ac58a7b7aa9a81</task-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-suite/df6721cd-97ce-45b2-b664-3fea70beada0/tasks/a92ac58a7b7aa9a81.output</output-file> <status>completed</status> <summary>Agent "Fable whole-branch review" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>Nothing further to fetch; the report above stands, with four spec cross-references the last agent surfaced: **Addendum (spec cross-checks)** - **Reinforces #1.** Spec §1 ruling 1 (line 60): "Sharing is membership; an outside guest is a **guest** member", and §8.2 says the same. The branch mints a full member. This is a spec deviation, not only a design risk. - **New NIT, tied to #1.** Spec §4 data model (line 145) defines `grants(doc_id | NULL, subject, role, created_by, created_at)`. `0005_grants.sql` drops `created_by` (it argues away the nullable `doc_id`, but never mentions `created_by`). Given transitive admission, `created_by` is the one column that would let an operator answer "who let this address in"; without it the chain in probe #1 (Alice → Bob → Mallory → Eve) is unreconstructable from the database. - **Reinforces #5.** Spec §5.2 (lines 272-279) already names "anyone holding a share link" as a denial-of-service vector — for *writes*, bounded by wasm fuel. `/p/{token}` is the read-side sibling (unbounded replays, no fuel argument applies to volume), and the spec's own framing supports treating it as a known class rather than a nicety. - **Citation hygiene.** `publish.go`, `links.go` and `0006_links.sql` cite "suite doctrine ruling 15" as the source of the read-only rule. Two independent reviews in `oficina/suite` (`FABLE-FINDINGS-R3.md` N1, `CODEX-FINDINGS-R3.md` R3-6) flag ruling 15 as a *proposal* whose "view or edit" wording contradicts Docs §8.2 / Sheets ruling 12 (Paul, 2026-08-30: "never editor"). The branch implements the settled rule (read-only), which is right; the comments should cite Docs §8.2 as the authority and ruling 15 only as context, or the next reader will think "edit link" is on the table. Verdict unchanged: not as-is. Blockers are #2 (reader editor) and a written decision on #1/#3 (guest vs member, session revocation); #4/#6 are minutes; 7c/7d/7e are the missing tests. The sharing worktree is clean.</result> <usage><subagent_tokens>325884</subagent_tokens><tool_uses>59</tool_uses><duration_ms>1013140</duration_ms></usage> </task-notification>

<task-notification> <task-id>a92ac58a7b7aa9a81</task-id> <tool-use-id>toolu_0168fTRxS6YWjz8icY3b1BUC</tool-use-id> <output-file>/home/paulca/.cache/tmp/claude-1001/-home-paulca-amadan-net-oficina-suite/df6721cd-97ce-45b2-b664-3fea70beada0/tasks/a92ac58a7b7aa9a81.output</output-file> <status>completed</status> <summary>Agent "Fable whole-branch review" finished</summary> <note>A task-notification fires each time this agent stops with no live background children of its own. The user can send it another message and resume it, so the same task-id may notify more than once.</note> <result>## Full findings report — `sharing` branch (`1ef53df`..`137cfd8`), read as one diff Method: every claim below was checked against source; the ones marked **confirmed by probe** were also executed via a temporary `internal/docs/zz_review_probe_test.go` (six tests, all run, file deleted; `git status --short` on the sharing worktree is empty). GORM behaviour was checked in `gorm.io/gorm@v1.31.2/callbacks/query.go`; the layout in `rastrillo@v0.22.0/ui/layouts/topbar.html`. --- ### #1 — RISK (security/design). A single reader grant is an unbounded, transitive key to the instance's front door. Confirmed by probe. **Where:** `internal/docs/auth.go:168-183` (`allowlist.granted` / `allows()`), `internal/docs/grants.go:159-171` (`HasGrant`), `internal/docs/handlers.go:290-310` (`create` uses `h.owner`, i.e. any session), `internal/docs/store.go:171-206` (`EnsurePerson` mints a full `Person` on first sign-in), `internal/docs/app.go:388`. **What is wrong:** `allows()` admits any address that holds *any* grant. Once admitted, the address is a full member: it can `POST /documents` and `POST /d/{sqid}/share` on its own documents, which mints grants to further addresses, which admit them, and so on. Nothing distinguishes an invited outsider from a team member. The spec says otherwise twice: §1 ruling 1 "Sharing is membership; an outside guest is a **guest** member", §8.2 "Outside guests are granted a specific document and become guest members." The branch has no guest kind. The `allows()` comment ("this only ever widens") reads as bounded-by-grants; it is bounded by nothing. `$DOCS_ALLOWED_ADDRESSES` silently stops being the membership list the operator configured. **Scenario (probe output):** Alice (allowlisted) shares one document read-only with `bob@example.com`. Bob signs in, creates his own document, shares it to `mallory@example.com` → `303`, `HasGrant(mallory)=true`. Mallory can now sign in and repeat. Alice then revokes Bob → `HasGrant(bob)=false`, but Bob's live session still shares to `eve@example.com` → `303`, `HasGrant(eve)=true`, and still `POST /documents` → `303`. Alice cannot undo any of it. **Decision I want written down (one of):** - (a) A guest kind: a person admitted only through a grant (not on the allowlist) cannot create documents or share — `create`/`sharePost`/`shareRevoke`/`shareLink` check `list.allows`-without-grants (i.e. the configured allowlist) or a `people.kind` column; guests read/write what they were granted and nothing else. This matches §1/§8.2. - (b) Explicit acceptance that a grant makes a full member, recorded in `auth.go`'s `granted` comment, `AGENTS.md`'s sign-in section, and the spec — naming the transitive consequence, not just "widens". I recommend (a). Either way the current silence is the defect. --- ### #2 — BUG (integrity/product). A reader grantee is handed a live editor whose every save is thrown away. Confirmed by probe. **Where:** `internal/docs/handlers.go:486-541` (`show` builds `showPage` with `CanShare` but no write flag); `internal/docs/views/show.html:1541` (`&lt;div docs-text docs-editor data-sqid="{{.Sqid}}"&gt;`), `:1606` (`&lt;script type="module" defer src="{{.EditorScript}}"&gt;`), `:1373-1517` (menu bar + toolbar, all enabled by the bundle); `fixtures/save-queue.mjs:82-87` (4xx → permanent drop, sticky `failed`). **What a reader can do that they must not:** open the document and *edit it locally*. `show` uses `GetFor` (correct, a reader may read) and then renders exactly the page an owner gets: ProseMirror mounts on `docs-editor`, the toolbar and menus enable, the document is contenteditable. The server correctly refuses the writes (`ApplyUpdate` → `ErrNotWritable` → 404), but the browser has already accepted the keystrokes. Probe: reader's page contains `docs-editor` and `data-sqid`. **Scenario:** Bob (reader) opens `/d/{sqid}`, types a paragraph. After the 2 s debounce the save queue POSTs, gets 404, classifies it as permanent, drops the batch, shows the red "save failed" notice, and keeps accepting typing. Every subsequent batch is dropped the same way. Meanwhile the live stream (which a reader may hold) keeps pushing the owner's edits into a document Bob has diverged locally. Bob's text is lost; the UI told him he was editing. **Minimal fix:** add `CanWrite bool` to `showPage` (`handlers.go`), computed in `show` from `writable(...)` (or a small `Store.CanWrite(ctx, actor, doc.ID)` that runs the same EXISTS clause); in `show.html` render `{{if .CanWrite}}&lt;div docs-text docs-editor data-sqid="{{.Sqid}}"&gt;{{.HTML}}&lt;/div&gt;{{else}}&lt;div docs-text&gt;{{.HTML}}&lt;/div&gt;{{end}}` and wrap the `&lt;script … EditorScript&gt;` and `[docs-save-status]` in the same `{{if .CanWrite}}`. The menu/toolbar can stay (their command items are `disabled` until the bundle enables them, and with no bundle they stay disabled) — or hide `[docs-chrome]` too for the cleaner page `public.html` already demonstrates. Add a test: reader's `GET /d/{sqid}` body must not contain `docs-editor` or `data-sqid`; writer's must. --- ### #3 — RISK. Revoke revokes the document, not the membership; `allows()` runs only at sign-in. **Where:** `internal/docs/auth.go:172` (`allows` is consulted by `gateBegin` and `rauth.Config.Authorize` — both sign-in time only); `internal/docs/grants.go:195-203` (`Revoke` deletes the grant row and nothing else); `rastrillo@v0.22.0/auth/handlers.go:191-201` (`RequireSession` checks only session validity; 30-day TTL). **What is wrong:** after the last grant is revoked and the address is not on the allowlist, the person keeps a valid session for up to 30 days, keeps their `Person` row and documents, and (per #1) keeps creating and sharing. "Stop sharing" is complete for the document and says nothing about the instance. **Scenario (probe output):** Alice revokes Bob; `HasGrant(bob)=false`; Bob's existing cookies still `POST /documents` → 303 and `POST /d/{bobDoc}/share` → 303. **Decision to write down:** whether revocation is meant to end admission. If yes: re-check `list.allows(id.Address)` per request inside the `RequireSession` group (a small middleware after `RequireSession`: `if !list.allows(addr) { redirect/403 }`), or revoke sessions for the address when its last grant goes. If no: say so in the share page copy ("they can still sign in") and in `auth.go`. Today it is neither. --- ### #4 — BUG (visible). Public and share pages render `&lt;title&gt;Hello&lt;/title&gt;`. Confirmed by probe. **Where:** `internal/docs/views/public.html` (defines only `brand` and `content`), `internal/docs/views/share.html` (defines only `account` and `content`); `rastrillo@v0.22.0/ui/layouts/topbar.html:6` is `&lt;title&gt;{{block "title" .}}Hello{{end}}&lt;/title&gt;`. `show.html:43` and `list.html:42` define `title`; these two do not. **Scenario:** a stranger opens a published link; the browser tab reads "Hello". Owner opens the share page; tab reads "Hello". Probe output: `PROBE public &lt;title&gt;: "&lt;title&gt;Hello&lt;/title&gt;"`, `PROBE share &lt;title&gt;: "&lt;title&gt;Hello&lt;/title&gt;"`. **Fix:** add `{{define "title"}}{{.Title}}{{end}}` to `public.html` (`publicPage.Title` already carries the document title) and `{{define "title"}}{{.Title}}{{end}}` to `share.html` (`sharePage.Title` is the catalog's "Share"; better: `{{.Title}} — {{.DocTitle}}` or just `.DocTitle`). Add an assertion to `TestAPublicLinkLetsASignedOutStrangerRead` and `TestSharePageListsGrantees` that the body contains `&lt;title&gt;Minutes&lt;/title&gt;` (or whatever is chosen). Also confirmed: the public page still ships the `rst-shell-account` disclosure (summary "Account", empty menu) — hidden by CSS only, as `public.html`'s own comment concedes. Not a security issue; a `{{define "account"}}{{end}}` changes nothing (the layout owns the `&lt;details&gt;`); leave it unless rastrillo grows a way to drop it. --- ### #5 — RISK. `/p/{token}` is unauthenticated CRDT replay with no cache and no rate limit. **Where:** `internal/docs/publish.go:458` (`h.store.HTML`) → `internal/docs/store.go:589-601` → `replay` (`:390-493`): a fresh wasm handle per request, snapshot + tail applied every time. **What is wrong:** before this branch every replay sat behind a session and a per-team allowlist. Now the URL is designed to be posted publicly. Spec §5.2 (lines 272-279) already names "anyone holding a share link" as a denial-of-service vector for *writes* and bounds them with fuel; this is the read-side sibling, where volume rather than a single pathological call is the lever. **Scenario:** a long document (many snapshots/tails, or one past `core.MaxRetainedUpdates`, which `replay` re-seeds per read — `store.go:436-478`) is published; the link is shared widely or hammered; every hit instantiates and replays. Nothing shares work between requests. **Fix (not blocking):** cache the rendered HTML keyed on `(doc.ID, doc.UpdatedAt)` the way `previews` (preview.go) caches thumbnails, invalidated by `updated_at` moving; a small LRU is enough. Optionally a per-IP limiter on `/p/`. --- ### #6 — RISK (low). Response headers on `/p/`. Confirmed by probe (only `Content-Type` is set). **Where:** `internal/docs/publish.go:465-469` (comment) and `:470` (render, no headers). **What is wrong:** the comment says that setting nothing keeps a shared cache from retaining a revoked copy. Absence of `Cache-Control` does not forbid caching; it leaves it to heuristics. `stream.go:91` and `search.go:248` already set `no-store` for exactly this reason. Nothing in the app or in rastrillo sets `X-Robots-Tag`; an "anyone with the link" page that leaks into a public place gets indexed. (Referrer leakage is covered in production by rastrillo `serve.go:398` `Referrer-Policy: strict-origin-when-cross-origin`, so I withdraw that half.) **Fix:** before `h.render.page(...)` in `publicShow`: `w.Header().Set("Cache-Control", "no-store")` and `w.Header().Set("X-Robots-Tag", "noindex, nofollow")`; correct the comment. Add a header assertion to `TestAPublicLinkLetsASignedOutStrangerRead`, as `stream_test.go:158` does for the stream. --- ### #7 — Tests that assert less than they read as asserting **7a — NIT.** `internal/docs/publiclink_test.go:71-80` (`TestAPublicLinkCarriesNoWrite`, the route loop): asserts anonymous GETs to `/`, `/d/{sqid}`, `/d/{sqid}/share`, `/d/{sqid}/updates` are not 200. That is true on `main` with no link feature; the token is never presented to those routes, so it proves nothing about the link. Also `:84` bans `"editor-entry"` from the public body — that string appears on **no** page (the bundle is served from `bundlePath("editor")`, `assets.go:115`; probe on the reader's *show* page: `editor script: false` while `docs-editor: true`). Only `"docs-editor"` in that list does work. Fix: drop `"editor-entry"`, keep `"docs-editor"` and `"data-sqid"`. **7b — RISK.** `publiclink_test.go:94-105` (`TestAWrongTokenIsANotFound`): asserts `!= 200`, so a 500 (a DB error, a panic recovered by chi) passes. The invariant the branch claims — wrong, revoked and deleted-document tokens answer identically — is asserted nowhere, and "deleted document" is never exercised. Probe: wrong/revoked/empty all answer `404 "404 page not found\n"`, which is correct but unpinned. **Test to add:** `TestPublicRefusalsAreIndistinguishable` — publish; capture `(status, body, headers-minus-Date)` for (i) a syntactically valid unknown token, (ii) the real token after `on=0`, (iii) the real token after `UPDATE documents SET deleted_at=now WHERE id=?`; assert all three are byte-equal and status is exactly 404. **7c — RISK.** `publiclink_test.go:43-58` (`TestAPublicLinkLetsASignedOutStrangerRead`) publishes an **empty** document. If `ByToken`'s `JOIN links` scanned `links.id` into `Document.ID`, `HTML()` would replay the wrong (empty) log and still return 200. I verified it is correct today — GORM with `Joins` and no `Select` emits `SELECT documents.&lt;every column&gt;` (`callbacks/query.go:109-115`), and the probe shows `hello` on the public page — but no test holds it. **Test to add:** `TestAPublicLinkShowsTheDocumentsContent` — `ApplyUpdate(ReadFragmentFixture("hello"))` as owner, publish, anonymous `GET /p/{token}`, assert body contains `hello` and does not contain the sqid. **7d — RISK.** No HTTP-level test that a **writer** grantee can post an update and the owner sees it. `grants_write_test.go:58-84` posts `[]byte{0}` at the store and asserts only "not `ErrNotWritable`"; `share_test.go:148` tests the reader refusal. Probe: writer `POST /d/{sqid}/updates` → 204 and owner's page shows `hello` — it works — but if `updatesPost` regressed to `h.owner`+`Get`, every existing test stays green. **Test to add:** `TestAWriterCanPostUpdatesOverHTTP` — grant writer, POST `ReadFragmentFixture("hello")` with Bob's cookies → assert 204 and a `Docs-Seq` header; then `GET /d/{sqid}` as Alice → body contains `hello`; and `GET /d/{sqid}/updates` as Bob → 200 with non-empty body (reader/writer read path over HTTP is also untested). **7e — RISK.** `grants_admission_test.go` tests `allows()` with a fake closure and `HasGrant` at the store. The wiring that makes them one thing — `list.granted = …` at `app.go:388`, which must precede `newAuthPlugin(… list …)` at `:410` and `gateBegin(…, list, …)` at `:436` because both copy `list` by value — is untested. `NewForTest` always builds with a nil allowlist (`testing.go:31-34`), so grant-based admission never happens end-to-end in the suite. Move that assignment below `:410` and you silently recreate the "unredeemable invitation" the comment warns about, green. **Test to add:** `TestAGrantAdmitsThroughTheRealSignIn` — build with `Config{Env:"development", AllowedAddresses:"alice@example.com"}` (NewForTest needs a variant that accepts a Config, or a `NewForTestWith(t, cfg)`); `TestSignIn("bob@example.com")` must fail (no link mailed) before a grant; Alice grants Bob; `TestSignIn("bob@example.com")` must now yield a link whose verify sets a cookie, and `GET /d/{sqid}` with it is 200. **7f — NIT.** `share_test.go:146-162` (`TestAReaderCannotPostUpdates`) says "The 404 is the same one a stranger gets" and asserts only the reader's status. It is not the same: reader → `http.NotFound` (`handlers.go:753-755`, body `"404 page not found\n"`); stranger → `http.Error(T("docs.error.notfound"))` (`:575`, body `"docs.error.notfound\n"`). Probe confirmed both. Harmless as an oracle (a reader already knows they can read) but `store.go:733-735` "refused identically" and the test's sentence are false. Fix: use the same `http.Error(w, T("docs.error.notfound"), 404)` in the `ErrNotWritable` branch, and assert body equality. **7g — NIT.** Grantee `POST /d/{sqid}/share/revoke` and `POST /d/{sqid}/share/link` (`on=0`) are untested. Probe: both 404, correct. Add them to `TestOnlyTheOwnerReachesTheSharePage`. --- ### #8 — NIT. Comments the code no longer matches - `app.go:442-444` — search "owner-scoped through Store.Search → Store.List"; it is `SearchFor`/`ListFor`. - `app.go:455-471` — exports and stream "owner-scoped through the same Store.Get"; they are `GetFor`. - `stream.go:58-60` — "Owner-scoped through getDocumentForUpdates". - `export.go:6-10` and `views/show.html:1326` — "a document the signed-in person already owns". - `handlers.go:477-485` (`show`: "Store.Get's owner-scoped query"), `:551-554` (`getDocumentForUpdates` header paragraph still says `Store.Get`), `:593` (`updatesGet`: "Owner-scoped exactly like show"). - `handlers.go:204-208` (`list`): "every row's 'who' is the same person reading the page" — now false, and it was the stated reason the row shows no owner (see #9). - `store.go:679-720` — `ApplyUpdate` doc comment still describes verifying `doc.OwnerID` through `owned(doc.OwnerID)`; the code verifies the actor through `writable`. `:857-865` — "ownership was already verified above" (it is writability now; the `owned(doc.OwnerID)` bump at `:866` still works for a writer grantee only because `doc.OwnerID` came from `GetFor`). - `grants.go:236-240` (`ListFor`): "Which are yours is a question the row can answer without reordering the list" — `docRow` has no such field. - Spec §4.2 line 206-208: "One function, tested directly, and no handler queries documents any other way" — now `owned`/`readable`/`writable` and `Get`/`GetFor`. Amend the spec or the comment on `readable`. --- ### #9 — RISK (UX). The list mixes shared documents into "mine" with no marker; invitees get seeded. `handlers.go:139-150` (`docRow` has `Href/Title/When/Thumb` only), `:268-274`. A grantee's list shows Alice's document between their own with nothing saying whose it is; combined with #2 they open it and start "editing". Separately, `store.go:204` seeds six sample documents on the `Person`-create branch, so a person invited to read one document signs in to seven — the outcome `0005_grants.sql`'s comment says the design avoids, only deferred to sign-in. Fix: add `Shared bool` (or owner address) to `docRow` and render a muted marker; consider skipping seeding when the address is admitted only by a grant (ties to #1's guest kind). --- ### #10 — NIT. Address handling. Confirmed by probe. `grants.go:126-133` accepts any non-empty string: `address=not-an-email` → 303, listed on the share page, `HasGrant("not-an-email")=true` (admitted at the front door; only rauth's own address parsing downstream would stop it). `address=alice@example.com` (self) → 303 and the owner appears on her own share list. `subjectOf` (`grants.go:87-89`) duplicates `normalizeAddress` (`auth.go:134-136`): the two halves of one key, in two files, free to drift. Fix: `subjectOf` calls `normalizeAddress`; reject an address without `@` (or reuse rauth's validator) and reject the owner's own address. --- ### #11 — NIT. Handler hygiene. - `share.go:640` — `h.render.page(w, r, "share", page)` return value ignored; every other handler logs and 500s. - `share.go:680-681` — `w.WriteHeader(400)` runs *before* `render.page` sets `Content-Type`, so the header is dropped; works today via sniffing (probe: `text/html; charset=utf-8`). Set the header before `WriteHeader`, or have `renderShare` take a status. - `share.go:577` + `:582` — `shareGet` runs `Get` twice (`Get`, then `ListGrants` → `Get`). - `handlers.go:562` and `:741` — `updatesPost` resolves the actor twice → two `EnsurePerson` round-trips per keystroke batch. Return the actor from `getDocumentForUpdates`. - `handlers.go:728-739` — up to 1 MB of body is read before `writable` is checked; check first, read second. - `HasGrant` (`grants.go:159`) is called with `context.Background()` (`app.go:389`); a slow query during sign-in cannot be cancelled. --- ### #12 — NIT. Scope edges. - `HasGrant` counts grants on soft-deleted or orphaned (`owner_id = 0`) documents: `readable` hides the document, but the grant still opens the front door. - `ByToken` (`links.go:393-396`) filters `deleted_at IS NULL` but lacks `readable`'s `owner_id &gt; 0`; unreachable today because `Publish` goes through owner-scoped `Get`, but it is the one document query without the guard. - `writable` (`grants.go:73-83`) has no `deleted_at` clause; pre-existing shape (`owned` didn't either), and there is no delete verb yet (confirmed: no `Store.Delete`/`Rename`). - `0005_grants.sql` drops the spec's `created_by` column (spec §4, line 145: `grants doc_id | NULL, subject, role, created_by, created_at`). With #1 in play, `created_by` is the one column that lets an operator trace who admitted an address; add it now while the table is new. --- ### #13 — Verified as claimed (no action) Every `RequireSession` route is on the right scope: `/`, `/documents/search`, `/d/{sqid}`, `/d/{sqid}/updates` GET, both exports, `/d/{sqid}/stream` → `GetFor`; `/d/{sqid}/updates` POST → `GetFor` then `writable`; `/d/{sqid}/share` GET/POST, `/share/revoke`, `/share/link` → owner-only `Get`; no delete/rename routes. `readable`/`writable` are EXISTS subqueries with `owner_id &gt; 0` leading (orphan test holds). `Publish` is a real transaction on the single writer connection; a token collision on `idx_links_token` rolls back and leaves the old link live with a 500 — the safe direction. 16 bytes from `crypto/rand`, `RawURLEncoding` → 22 chars. `First` orders by `documents.id`, no ambiguity. Rotation over a live link, off-is-revocation, `CanShare` = ownership, public page carries no sqid/addresses, `list.granted` wiring order — all correct today. The branch implements view-only links, consistent with Docs §8.2 / Sheets ruling 12; `publish.go`/`links.go`/`0006_links.sql` should cite §8.2 as the authority and suite ruling 15 (a contested proposal, per `FABLE-FINDINGS-R3.md` N1 and `CODEX-FINDINGS-R3.md` R3-6) only as context. --- ## On the token at rest (your ruling 16) I did not list it as a finding, and I should have: `links.token` (`0006_links.sql:777`, `links.go:273`) stores the bearer capability in plaintext, `LinkOf` (`links.go:360-374`) reads it back to re-display on every share-page load, and `linkFor` in the tests scrapes it off that page. My view: **hashing is right, don't retract — but write the reason honestly, because the obvious one is weak for Docs.** - The weak argument: "a database read reveals the tokens." In Docs today the same read reveals every document in plaintext (`store.go:556-560`: sealing is out of scope), which is strictly more than a token. Hashing buys no confidentiality against that reader *today*. - The strong arguments, which hold regardless of sealing: (1) a token is a **standing key** where a document is a **snapshot** — a leaked backup or a copied `.db` keeps opening the live, changing document until someone notices and rotates, whereas leaked content is what it was on the day; hashing turns a backup leak into a one-time disclosure instead of an ongoing capability. (2) Re-displaying the URL on the share page is itself an exposure surface (screenshots, shoulder-surfing, browser page cache, "copy link" in a shared screen); show-once removes it. (3) One rule across the suite (Sheets §8.3 already hashes), so a reviewer never has to ask which app is the exception. - Mechanics that matter for the branch: SHA-256 of the 128-bit token, **unsalted** (the entropy is the salt; bcrypt/argon would only add per-request cost for nothing); `UNIQUE(token_hash)`; lookup `WHERE links.token_hash = ?` stays indexed and single-row; SQL byte-equality timing is irrelevant at 128 bits. `Publish` returns the plaintext once; `LinkOf` returns only "published or not". The share page shows the URL **only in the response to the publish POST** — which collides with `share.html`'s "303 back here so a reload never re-posts" design (a reload of the POST response would rotate the link). Use a one-time flash (a short-lived cookie the GET consumes) or render the POST response directly with a "this is the only time you will see this" line and accept that reload re-posts; either is fine, but pick one and say why. The three `publiclink_test.go` tests that read the link off the share-page GET (`linkFor`) all change shape. - One caveat to put in the ruling: if the app ever logs request paths, `/p/&lt;token&gt;` lands in the access log in plaintext anyway (`carlos logs`). Hashing at rest does not cover that; the ruling should say the request-log boundary is a separate rule, not claim it is solved. ## Verdict Not mergeable as-is. Blockers: **#2** (a reader is given an editor that silently discards their work) and a written decision on **#1/#3** (guest vs member; whether revoke ends admission) — the front door changes meaning and nothing says so. **#4** and **#6** are minutes. **7c/7d/7e** are the three tests that would have caught the regressions this branch is most exposed to. Token hashing (ruling 16) is right and can land with the same branch; the sharing worktree is clean.</result> <usage><subagent_tokens>333870</subagent_tokens><tool_uses>59</tool_uses><duration_ms>1174433</duration_ms></usage> </task-notification>