feat(pages): serve a recipe's picture of the dish and state its absence

[kschlt]

Requested by Kornelius · project thread

Before: a recipe page never showed a picture, even for a recipe that had one, and it said nothing about the missing picture. The renderer could draw one, but no page handed it a way to name the image, and no address served the bytes.

After: a recipe that has a picture of the dish shows it on its page, on its library card, and on the page the cooking address falls back to. A recipe without one says so in a sentence that is true for its source:

• Photographed cookbook page: the photograph is kept as the source and is not a picture of the dish (ontology §7).
• Linked page or pasted text: none was kept.
• Source that cannot be read: the page says that. It does not guess either way.

Reading the source never costs the page. A snapshot the store refuses to hand back still leaves both addresses readable.

Nothing writes media.heroImage yet, so a running instance still shows no picture, and the README still says so. The follow-up PR takes the picture from the linked page's structured data when a URL is imported.

How

• GET /recipes/:id/picture is behind the library credential and reads from the same byte store the photo route keeps photographs in (ADR-0009). It addresses by recipe id only: nothing in the request chooses other bytes than the recipe's own.
• The content-type comes only from the bytes' own signature (src/http/image-signature.ts: JPEG, PNG, WebP). The response also carries x-content-type-options: nosniff and cache-control: no-store.
• Any other bytes, such as HTML or SVG stored as a "picture", get the one shared not-found answer. So do a missing credential, an unknown recipe, an identity the store does not hold, and a crafted identity.
• recordedIdentity() in src/storage reads back an identity a record kept, so no module outside storage casts to StorageIdentity. get still returns nothing for an identity the store could not have issued.
• The renderer's recipe view now carries picture: either shown, or absent with a reason (photographed_page, none_kept, source_unread, not_served). What the caller learned about the source is three-valued (photographed_page, not_photographed, unread), and absent means unread. "Not a photograph" and "nobody could read it" are different facts.

Proofs: tests/run/recipe-picture.test.ts runs fifteen proofs against the composed instance. Every image in it is generated by the test.

Nineteen violations were planted, and each was red at its own assertion:

• a guessed fallback type
• a fixed type
• an empty media store in the instance
• the page without a resolver
• the library without a resolver
• the degraded page without a resolver
• the photographed flag dropped
• the absence rendered as nothing
• the credential check removed
• any RIFF file served as WebP
• a shortened PNG signature
• the picture shown together with a "no picture" sentence
• an identity taken from ?identity=
• an identity taken from the first query value, whichever of the seven sent keys carries it (Object.values(c.req.query())[0])
• an identity taken from a header
• a snapshot read error rethrown
• a read error shown as "not a photograph"
• a missing snapshot shown as "not a photograph"
• the renderer's default shown as "not a photograph"

Review round 1 (BLOCK): two findings, both fixed in fe914a2.

1. The route's "by the recipe" claim was held by nothing. A planted route that read ?identity= passed every proof. A proof now names another stored image's identity under seven query keys and a header, and requires the recipe's own bytes back. It sees only the spellings it sends: measured in review round 2, a route reading c.req.query("image") stays green. A structural claim that the identity passed to mediaStore.get is unreachable from c.req would close that, and is left for the next change to this route.
2. An unreadable snapshot turned both pages into 500s. The read is now caught, and the page says that the source could not be read.

Measured along the way:

• The photographed-page sentence contains an apostrophe, which the page serves as '. A "not on the page" check against the raw string was therefore vacuous. The proofs now compare the escaped form.
• My first repository proxy for the unreadable-snapshot proof produced its own 500, because the store's private fields cannot be read through a proxy as this. Its methods are now bound to the store.

Also touched:

• the majors plant text for the library call
• the README guard: byte-store.ts is now loaded at runtime, and the row's reason is updated
• the README gap paragraph

npm run quality on the merge with main e1a05ac is green: 1396 passed.

🤖 Generated with Claude Code

https://claude.ai/code/session_0125aMujwwRnPLKYSpiW6uZw

---

Generated by Claude Code

[kschlt]

PR Review Verdict

Result: BLOCK

Deterministic: Measured on the merge result, not the head: head 9cc6051 merged with main at e1a05ac in a worktree of its own, giving tree e9ca311. The branch is behind by the #116 merge and the merge is clean. npm run quality exits 0 under Node 26.10.0 against a real PostgreSQL 16: tsc --noEmit clean, biome ci . clean over 206 files, 95 files, 1391 passed / 4 skipped, which is main's 1379 plus the twelve this PR adds. The one commit's subject is 71 characters. The PR is still a draft, so this verdict is a reading of the work rather than a gate on a merge. I planted five weakenings of my own and reproduced one regression. Four plants were red at their own named assertion, each one failure in 262: nosniff dropped from the picture headers, cache-control: no-store dropped, the credential check removed from the picture route, and the WebP form-type check removed so that any RIFF container is served as WebP. One plant was green, and it is the route's own central security claim: making the route read c.req.query("identity") in place of the identity the recipe names leaves all 262 proofs in scope passing and tsc clean. The regression is reproduced with a control. A repository whose loadSnapshot throws — which the Postgres store does by design when a stored snapshot no longer satisfies the contract, as its own comment says — turns GET /recipes/:id and the degraded GET /recipes/:id/cook into 500s, where on main the same reproduction passes both. I built the stub wrong the first time (a spread of a class instance, which drops its prototype methods) and the 500 I first saw was my own; the measurement above is from the corrected Proxy version, and the failure is the route's. Every plant restored with git reset --hard plus git clean, git status --porcelain empty after each. — Tree: e9ca3116255a758136c1f24a6838d64f8fe52cab — Head: 9cc6051

Acceptance Criteria

• [not met] The picture address serves that recipe's picture and nothing else the store holds — Evidence: the route's own comment makes this its central claim, in bold, and names what it protects: the kept photographs, which "a route taking an identity from the request would hand to anyone holding the library credential". I planted exactly that route and nothing noticed. See the first finding
• [not met] A snapshot that cannot be read changes one sentence and nothing else — Evidence: the comment above recipePage says so; measured, it changes the status code to 500 on both addresses that render a recipe page. The control on main passes. See the second finding
• [met] The picture address requires the library credential, and a miss is the one shared answer — Evidence: removing the credential check is red at is the shared not-found answer without the library credential, comparing status, headers and body as one object, so a weakening that kept the status and changed the shape would be caught too — proof: run/recipe-picture
• [met] The content type follows the bytes, and bytes that are none of the three are not served — Evidence: serving any RIFF container as WebP is red at reads the three formats from their signatures, and nothing else as any of them, which compares the whole table of eleven cases rather than one. The signatures themselves are the formats' own: FF D8 FF, PNG's eight bytes, and RIFF with WEBP at offset 8
• [met] The response's headers are pinned as a set, not sampled — Evidence: dropping nosniff and dropping no-store are each red at answers that address with the bytes the store holds, typed by those bytes, which compares the header list as a whole. That is the breadth that stops a fourth header being added or removed unnoticed
• [met] No module outside storage casts to StorageIdentity, and a crafted identity reaches nothing — Evidence: recordedIdentity keeps the cast inside src/storage, and FilesystemByteStore.get refuses anything that is not sha256: plus 64 hex before it builds a path, so no identity from a record can traverse out of the volume. Read rather than planted; the confinement proof is slice1/storage-identity-confinement
• [met] An absence is stated rather than left blank, with a sentence true for its source — Evidence: PictureView is a union for exactly the reason the PR gives, and the photographed-page sentence says the photograph is kept rather than that no picture exists, which is what docs/recipe-ontology.md §7 requires. The apostrophe finding in the PR body is the right kind of care: a "not on the page" check against a raw string passed whatever the page said
• [met] Nothing writes media.heroImage yet, and the README says so — Evidence: the README guard's row now carries that reason, and the gate includes it
• [met] The gate is green on the merge result — Evidence: 1391 passed, exactly twelve more than main

Findings

• [blocking][reproduced] The route's central security claim — by the recipe, never by the request — is held by nothing. I changed one expression so the route prefers an identity from the query string when one is present, left everything else alone, and ran the 262 proofs in scope: all green, tsc clean. So the property the route's own comment states in bold, and the reason it gives for stating it, would survive no regression at all. What it protects is not hypothetical: instance.ts hands this route the same store the photo route keeps photographs in, so a route that took its identity from the request would read any bytes on the volume by digest for anyone holding the library credential. Exploiting it needs a digest, which is why I call it a held-claim defect rather than a live hole — but the claim is the design, and an unproven design claim is how the design gets edited away. The closure is one proof and no production change: put two images in the store, make one the recipe's picture and the other a kept photograph, ask for /recipes/:id/picture with the other identity named in the query, and require the answer to be the recipe's own bytes. That proof fails on my plant and passes on this head.
• [blocking][reproduced] The recipe page no longer survives a snapshot it cannot read, and neither does the degradation that exists to keep it readable. recipePage awaits deps.repo.loadSnapshot with nothing around it, and that call does not only return undefined: the Postgres store validates the row on the way out and throws when a stored snapshot no longer satisfies the contract — its own comment says that is deliberate, because a reader must be told. Measured with a repository whose loadSnapshot throws: GET /recipes/:id answers 500, and so does GET /recipes/:id/cook for a recipe whose plan refuses to derive, which is the branch whose comment one line above says "whatever went wrong with the plan, the recipe is still readable". The same reproduction passes on main, so this is a regression of slice6/recipe-viewable-when-derivation-fails and not a pre-existing property. The closure has two halves and the second is the easy one to get wrong: the read must not be allowed to fail the page, and the sentence it feeds must not then assert something unknown as known. photographedSource is boolean | undefined with === true, so "not a photograph" and "nobody could look" are the same value, and catching the error without changing that would print "none was kept from the source" for a recipe whose photograph the instance is holding. That is the invention the project refuses; a gap gets stated as a gap.

Not Checked

• Eight of the twelve plants the PR reports: the guessed fallback type, the fixed type, the empty media store, the page, library and degraded page each without a resolver, the photographed flag forced false, and the picture shown beside a "no picture" sentence. I measured four of its claims and one it does not make; the eight left exercise assertions I reached from another side.
• Whether a picture is ever actually written. Nothing writes media.heroImage on this branch, so every proof of the serving path runs on bytes a test put there. That is correct for this PR and is the follow-up's subject.
• The cost of the extra snapshot read on every recipe page. It is one query per page and nothing measures page latency, so I have no figure either way; OQ-35 is the neighbouring record.
• The 1391 tests one at a time. The gate ran them green on the merge result.

Routed

none

---

Generated by Claude Code

[kschlt]

PR Review Verdict

Result: PASS

Deterministic: Round two, delta-scoped: did the two blocking findings' closures hold, and did anything new appear at blocking severity. Measured on head f77eebc merged with main 59c4b13 — tree 3269540. The head does not contain that main, which landed while this review ran, so the measurement is on the merge result and the branch needs an update before it can merge. In a fresh worktree, npm ci before the merge and again after it, npm run quality exits 0 against a real PostgreSQL 16 under Node 26.10.0: tsc --noEmit clean, biome ci . over 206 files with 5 warnings and 106 infos and no errors — the same counts main carries — and 95 files, 1396 passed / 4 skipped, against main's 1379. The delta over round one's head is the fix commit plus two base merges; no file outside src/http/pages-app.ts, src/render/ and tests/run/recipe-picture.test.ts is touched by it, and the longest of the four subjects is 93 characters, so the merge title lands at 78. Both closures hold, each red at its own sentence, one failure in 116 each time. The ?identity= route I planted in round one is now red at answers with the recipe's own picture, whatever identity the request names; so is the same read taken from an x-storage-identity header. Removing the try/catch around the snapshot read is red twice, at expected 500 to be 200 on the recipe page and on the page the cooking address degrades to — the exact regression round one measured. Defaulting the renderer's absent pictureSource to not_photographed is red at expected 'none_kept' to be 'source_unread', and so is mapping a missing snapshot to not_photographed: the three-value union is held at both ends, in the app's caller and in the renderer itself. One plant survives, and the proof says so itself: a route reading an identity from a query key the proof does not send (?image=) is green at 116 of 116. Every plant restored with git reset --hard plus git clean, git status --porcelain empty after each. — Tree: 32695407f29d7241cc27cb4f6db4067e954ee173 — Head: f77eebc

Acceptance Criteria

• [met] The route's "by the recipe, not by the bytes" claim is now held by something — Evidence: the route's code is unchanged; what was missing was the proof, and the new one bites. My round-one plant and a header variant are each red at the new assertion, and the proof cannot pass vacuously: it compares the returned bytes to the dish's, so a 404 or a wrong image fails it — proof: picture/the-address-is-the-librarys > answers with the recipe's own picture, whatever identity the request names
• [met] An unreadable source no longer costs the page, on either address — Evidence: both pages answer 200 with the gap sentence under a repository whose loadSnapshot throws the real validator's own error, and both answer 500 the moment the catch is removed. The fixture throws through a Proxy with every other method bound to the store, which is the trap that produced my own first 500 in round one and theirs in their first attempt — proof: picture/an-unreadable-source-costs-the-page-nothing, both tests
• [met] The page never says "none was kept" about a recipe whose photograph the instance holds — Evidence: this was the trap I named in round one, and it is closed at the type. photographedSource?: boolean is gone; PictureSource is photographed_page | not_photographed | unread and absent means unread in the renderer, so a future caller that forgets to look cannot state a fact either. Both the renderer default and the missing-snapshot mapping are red when flipped to not_photographed
• [met] The new sentence is true for what is known, rather than guessing — Evidence: source_unread reads "No picture of the dish is kept with this recipe, and its source could not be read to say why", and gapIn requires exactly one of the four gap sentences on the page, so a page saying two or none fails. r-orphan, whose snapshot is simply absent, now says source_unread where it said none_kept — the declared-gap form rather than an invention
• [met] The closure stayed the size of the findings — Evidence: one union type replacing a boolean, one sentence, one try/catch, one mapping table, and four proofs for three existing ones. No change to the route, to image-signature.ts, to the not-found answer or to the credential check
• [met] The feature's unreachability is declared where a reader meets it — Evidence: the README lists it under "Built and tested, but not reachable from a running instance yet" and again under "Not there yet", saying nothing gives a recipe a picture of the dish because a photographed page is deliberately not one. Nothing writes media.heroImage, and the PR says so

Findings

• [advisory][reproduced] The security proof's breadth is an enumeration, not a universal, and an eighth spelling would be silent. The claim the route makes in bold is that nothing in the request chooses other bytes than the recipe's own. The proof names the identity under seven query keys and one header; measured, a route reading c.req.query("image") instead passes 116 of 116. The test file declares exactly this limit in its own prose, which is the honest form and the reason this is not blocking — but the PR body's plant list reads "an identity taken from any query key", which a later reader will take as the stronger thing. What would pin the breadth rather than sample it is the shape this repository already uses elsewhere (tests/protections/url-capture-wiring.ts, ingest-entry-points.ts): a structural assertion that inside this handler the value handed to mediaStore.get is not reached from c.req at all, which is red for every spelling including ones nobody has thought of. Worth doing when the route next changes; worth correcting in the body now, since a body edit costs no commit and no CI run.
• [advisory][reasoned] "Missing" and "unreadable" are now one sentence, and that is a choice the page cannot unmake. A snapshot that was never there and a snapshot the store refuses both produce source_unread, which says the source could not be read. That is true of both and invents nothing, so it is the right default. The thing to know is that the page can no longer distinguish a recipe whose source was dropped from an instance whose store is failing, and nothing logs the difference either — catch {} with no observer is this repository's house style in ingest-app.ts too, so this is consistent rather than careless. If the sentence ever has to tell an operator which of the two is happening, that information no longer exists at the point where the page is rendered.

Not Checked

• The nineteen plants the PR body reports from round one, beyond the six I planted here. The ones I did plant are the two findings' closures and their variants; the type-signature and resolver plants are untouched by this delta and I took them from round one's measurement.
• Whether Hono exposes a request value under a name neither the proof nor I tried. That is the open end of the advisory above, and enumerating further spellings would only move the bound, not remove it.
• Anything about what the picture route does once something writes media.heroImage. Nothing does yet, which the README and the PR both state, so every proof here constructs the record itself.
• The 1396 tests outside this file and the renderer. The gate ran them green on the merge result at main's own counts; I did not reason about them.

Routed

none

---

Generated by Claude Code

[kschlt]

PR Review Verdict

Result: PASS

Deterministic: Round two, delta-scoped: did the two blocking findings' closures hold, and did anything new appear at blocking severity. Measured on head b368338 — tree 3269540. This is the same measurement the verdict published on f77eebc carries, re-pinned to the head that will merge rather than re-run: that review measured f77eebc merged with main 59c4b13, the branch has since been updated to exactly that merge, and the updated head's tree is 32695407f29d7241cc27cb4f6db4067e954ee173 — byte-identical to the tree measured, so only the commit object differs and every figure below transfers unchanged. main is now an ancestor, all 22 check runs are green on this head, and the PR body's one overstated plant line has been corrected to what was measured. In a fresh worktree, npm ci before the merge and again after it, npm run quality exits 0 against a real PostgreSQL 16 under Node 26.10.0: tsc --noEmit clean, biome ci . over 206 files with 5 warnings and 106 infos and no errors — the same counts main carries — and 95 files, 1396 passed / 4 skipped, against main's 1379. The delta over round one's head is the fix commit plus two base merges; no file outside src/http/pages-app.ts, src/render/ and tests/run/recipe-picture.test.ts is touched by it, and the longest of the four subjects is 93 characters, so the merge title lands at 78. Both closures hold, each red at its own sentence, one failure in 116 each time. The ?identity= route I planted in round one is now red at answers with the recipe's own picture, whatever identity the request names; so is the same read taken from an x-storage-identity header. Removing the try/catch around the snapshot read is red twice, at expected 500 to be 200 on the recipe page and on the page the cooking address degrades to — the exact regression round one measured. Defaulting the renderer's absent pictureSource to not_photographed is red at expected 'none_kept' to be 'source_unread', and so is mapping a missing snapshot to not_photographed: the three-value union is held at both ends, in the app's caller and in the renderer itself. One plant survives, and the proof says so itself: a route reading an identity from a query key the proof does not send (?image=) is green at 116 of 116. Every plant restored with git reset --hard plus git clean, git status --porcelain empty after each. — Tree: 32695407f29d7241cc27cb4f6db4067e954ee173 — Head: b368338

Acceptance Criteria

• [met] The route's "by the recipe, not by the bytes" claim is now held by something — Evidence: the route's code is unchanged; what was missing was the proof, and the new one bites. My round-one plant and a header variant are each red at the new assertion, and the proof cannot pass vacuously: it compares the returned bytes to the dish's, so a 404 or a wrong image fails it — proof: picture/the-address-is-the-librarys > answers with the recipe's own picture, whatever identity the request names
• [met] An unreadable source no longer costs the page, on either address — Evidence: both pages answer 200 with the gap sentence under a repository whose loadSnapshot throws the real validator's own error, and both answer 500 the moment the catch is removed. The fixture throws through a Proxy with every other method bound to the store, which is the trap that produced my own first 500 in round one and theirs in their first attempt — proof: picture/an-unreadable-source-costs-the-page-nothing, both tests
• [met] The page never says "none was kept" about a recipe whose photograph the instance holds — Evidence: this was the trap I named in round one, and it is closed at the type. photographedSource?: boolean is gone; PictureSource is photographed_page | not_photographed | unread and absent means unread in the renderer, so a future caller that forgets to look cannot state a fact either. Both the renderer default and the missing-snapshot mapping are red when flipped to not_photographed
• [met] The new sentence is true for what is known, rather than guessing — Evidence: source_unread reads "No picture of the dish is kept with this recipe, and its source could not be read to say why", and gapIn requires exactly one of the four gap sentences on the page, so a page saying two or none fails. r-orphan, whose snapshot is simply absent, now says source_unread where it said none_kept — the declared-gap form rather than an invention
• [met] The closure stayed the size of the findings — Evidence: one union type replacing a boolean, one sentence, one try/catch, one mapping table, and four proofs for three existing ones. No change to the route, to image-signature.ts, to the not-found answer or to the credential check
• [met] The feature's unreachability is declared where a reader meets it — Evidence: the README lists it under "Built and tested, but not reachable from a running instance yet" and again under "Not there yet", saying nothing gives a recipe a picture of the dish because a photographed page is deliberately not one. Nothing writes media.heroImage, and the PR says so

Findings

• [advisory][reproduced] The security proof's breadth is an enumeration, not a universal, and an eighth spelling would be silent. The claim the route makes in bold is that nothing in the request chooses other bytes than the recipe's own. The proof names the identity under seven query keys and one header; measured, a route reading c.req.query("image") instead passes 116 of 116. The test file declares exactly this limit in its own prose, which is the honest form and the reason this is not blocking — but the PR body's plant list reads "an identity taken from any query key", which a later reader will take as the stronger thing. What would pin the breadth rather than sample it is the shape this repository already uses elsewhere (tests/protections/url-capture-wiring.ts, ingest-entry-points.ts): a structural assertion that inside this handler the value handed to mediaStore.get is not reached from c.req at all, which is red for every spelling including ones nobody has thought of. Worth doing when the route next changes; worth correcting in the body now, since a body edit costs no commit and no CI run.
• [advisory][reasoned] "Missing" and "unreadable" are now one sentence, and that is a choice the page cannot unmake. A snapshot that was never there and a snapshot the store refuses both produce source_unread, which says the source could not be read. That is true of both and invents nothing, so it is the right default. The thing to know is that the page can no longer distinguish a recipe whose source was dropped from an instance whose store is failing, and nothing logs the difference either — catch {} with no observer is this repository's house style in ingest-app.ts too, so this is consistent rather than careless. If the sentence ever has to tell an operator which of the two is happening, that information no longer exists at the point where the page is rendered.

Not Checked

• The nineteen plants the PR body reports from round one, beyond the six I planted here. The ones I did plant are the two findings' closures and their variants; the type-signature and resolver plants are untouched by this delta and I took them from round one's measurement.
• Whether Hono exposes a request value under a name neither the proof nor I tried. That is the open end of the advisory above, and enumerating further spellings would only move the bound, not remove it.
• Anything about what the picture route does once something writes media.heroImage. Nothing does yet, which the README and the PR both state, so every proof here constructs the record itself.
• The 1396 tests outside this file and the renderer. The gate ran them green on the merge result at main's own counts; I did not reason about them.

Routed

none

---

Generated by Claude Code