stackchain/timmy-talking-turd
2
0 Fork 0
Code Issues 21 Pull Requests 7 Actions 4 Packages Projects Releases 41 Wiki Activity

Harden image ingress: sniff the container, refuse polyglots and metadata, and bound the request (#16) #82

Merged
rockachopa merged 5 commits from timmy/16-image-ingress-hardening into main 2026-10-31 12:06:27 +00:00
Conversation 2 Commits 5 Files Changed 21
+1094 -30
timmy commented 2026-10-20 08:21:37 +00:00
Owner
Copy link
Copy source

Closes #16

What this does

/api/analyze decided whether an upload was a photo from the data-URL MIME type the caller supplied. That claim is now ignored: every decision comes from the bytes, and every limit fails before the provider is called.

Control Module Refusal
Container sniffed from magic bytes; mismatch refused src/image-ingress.js 400
Container walked to its own end marker, trailing payload refused (polyglot) src/image-ingress.js 400
JPEG APP1/APP13/COM and PNG eXIf/tEXt/zTXt/iTXt/tIME refused src/image-ingress.js 400
Dimensions read from the header; >4096px edge or >16MP refused src/image-ingress.js 400
Per-caller sliding-window budget, 12/60s default, retry-after src/rate-limit.js 429
6MB ceiling and 15s body deadline, half-read socket dropped src/http-body.js 413 / 408
No bytes, hash, or address in any log line or error body server.mjs + tests —

The app's own upload is a canvas re-encode (JPEG q0.7, longest edge 1200px) and carries no EXIF, PNG, or WebP metadata, so the accepted path is already metadata-free. A file the browser cannot decode now has its own state: "That file isn’t a photo Timmy can read" + "Nothing was uploaded", instead of the failed-analysis copy.

Evidence

  • npm test — 225 pass, 0 fail (32 new).
  • tests/analyze-ingress.acceptance.test.js runs the shipping server against synthetic bytes with a counting provider stub: non-image, polyglot, flood (429 + retry-after), and stalled body (408) all refused with provider calls unmoved; a clean re-encoded image still reaches the provider exactly once; the log carries no base64.
  • tests/ingress-capture.acceptance.mjs (new browser suite, wired into CI and the release builder): the app refuses the fake file, never posts it, keeps manual entry one tap away, then asks the running server to refuse the same bytes (400) and still reaches consent with a real synthetic photo.
  • Browser gates re-run green: ui, photo, mobile-capture, sleek, manual-fallback, provisional, followup, vision-refusal, worker-readiness, pattern-summary, grounded-chat, photo-lifecycle.
  • Release demo re-recorded end to end: 63/63 captions measured as painted bars, including the new refusal scene (A file that only claims to be a photo is refused on the device — nothing is uploaded); the builder now requires that caption and records image_ingress_refusal: passed.

Notes

  • Supersedes the unmerged free/hy3-timmy-16 branch (PR #63): that branch has 6 conflicts against current main and 31 commits of drift, so this is a fresh, tested implementation of the same acceptance criteria on today's tree.
  • No real medical images anywhere: every fixture is synthetic bytes built in the test file, plus a committed text file that claims to be a .jpg.
  • Residual risks stated in the threat model, not glossed: no server-side transcode (unusual-but-valid containers are refused, not repaired), PNG CRCs not validated, budget is per process.
  • Not merged, per the release job's contract.
Closes #16 ## What this does `/api/analyze` decided whether an upload was a photo from the data-URL MIME type the caller supplied. That claim is now ignored: every decision comes from the bytes, and every limit fails before the provider is called. | Control | Module | Refusal | |---|---|---| | Container sniffed from magic bytes; mismatch refused | `src/image-ingress.js` | 400 | | Container walked to its own end marker, trailing payload refused (polyglot) | `src/image-ingress.js` | 400 | | JPEG APP1/APP13/COM and PNG eXIf/tEXt/zTXt/iTXt/tIME refused | `src/image-ingress.js` | 400 | | Dimensions read from the header; >4096px edge or >16MP refused | `src/image-ingress.js` | 400 | | Per-caller sliding-window budget, 12/60s default, retry-after | `src/rate-limit.js` | 429 | | 6MB ceiling and 15s body deadline, half-read socket dropped | `src/http-body.js` | 413 / 408 | | No bytes, hash, or address in any log line or error body | `server.mjs` + tests | — | The app's own upload is a canvas re-encode (JPEG q0.7, longest edge 1200px) and carries no EXIF, PNG, or WebP metadata, so the accepted path is already metadata-free. A file the browser cannot decode now has its own state: "That file isn’t a photo Timmy can read" + "Nothing was uploaded", instead of the failed-analysis copy. ## Evidence - `npm test` — 225 pass, 0 fail (32 new). - `tests/analyze-ingress.acceptance.test.js` runs the shipping server against synthetic bytes with a counting provider stub: non-image, polyglot, flood (429 + retry-after), and stalled body (408) all refused with provider calls unmoved; a clean re-encoded image still reaches the provider exactly once; the log carries no base64. - `tests/ingress-capture.acceptance.mjs` (new browser suite, wired into CI and the release builder): the app refuses the fake file, never posts it, keeps manual entry one tap away, then asks the running server to refuse the same bytes (400) and still reaches consent with a real synthetic photo. - Browser gates re-run green: ui, photo, mobile-capture, sleek, manual-fallback, provisional, followup, vision-refusal, worker-readiness, pattern-summary, grounded-chat, photo-lifecycle. - Release demo re-recorded end to end: 63/63 captions measured as painted bars, including the new refusal scene (`A file that only claims to be a photo is refused on the device — nothing is uploaded`); the builder now requires that caption and records `image_ingress_refusal: passed`. ## Notes - Supersedes the unmerged `free/hy3-timmy-16` branch (PR #63): that branch has 6 conflicts against current `main` and 31 commits of drift, so this is a fresh, tested implementation of the same acceptance criteria on today's tree. - No real medical images anywhere: every fixture is synthetic bytes built in the test file, plus a committed text file that claims to be a `.jpg`. - Residual risks stated in the threat model, not glossed: no server-side transcode (unusual-but-valid containers are refused, not repaired), PNG CRCs not validated, budget is per process. - Not merged, per the release job's contract.
timmy added 3 commits 2026-10-20 08:21:38 +00:00
feat(ingress): decide photo validity from bytes, budget, and header before any provider work (#16) ba9afe0a1d 
The declared data-URL MIME was a claim the boundary believed. Every image decision now comes from
the bytes: the container is sniffed, walked to its own end marker, refused when metadata segments
remain, and refused above a 4096px edge or 16MP. The request path adds a per-caller sliding-window
budget (429 + Retry-After) and a body deadline (408), so a flood, a stalled body, or a decode bomb
fails before a provider call.

Log hygiene is part of the contract: refusal copy is fixed prose, the bucket key is a salted hash
kept in memory only, and the acceptance test asserts no image bytes reach the process log.
feat(photo): give an unreadable file its own upload-free state instead of a failed analysis (#16) 9b72872466 
Selecting a file the browser cannot decode used to render "Timmy couldn't analyze that safely",
which described an upload that never happened. It now states that the file isn't a photo Timmy can
read and that nothing was uploaded, with manual entry and the picker both one tap away. The state is
asserted in a new browser suite that also asks the running server to refuse the same bytes, and the
existing mobile capture suite asserts the new copy across both viewports.
test(release): record the ingress refusal on camera and gate its caption (#16)
Quality gates / quality (pull_request) Successful in 2m57s
Details
2b773ba58c 
The demo now picks a committed text fixture that only claims to be a photo, asserts on camera that
nothing was posted to /api/analyze, and keeps manual entry in the same scene. The builder requires
that caption to have painted and records image_ingress_refusal in the manifest; the threat model
names each implemented control, its module, and the tests that enforce it.
rockachopa commented 2026-10-20 12:03:53 +00:00
Member
Copy link
Copy source

Release-blocking finding on the release-builder gate this PR adds (not merged today).

scripts/build_release.py starts the acceptance server with TIMMY_VISION_ENABLED=false, and server.mjs returns 503 from the !visionConfig.enabled branch before ingressGuard runs. tests/ingress-capture.acceptance.mjs then asserts 400 for the same bytes and fails:

$ HOST=127.0.0.1 PORT=4173 TIMMY_VISION_ENABLED=false node server.mjs &
$ curl -s -X POST localhost:4173/api/analyze -H 'content-type: application/json'     -d '{"consent":true,"imageDataUrl":"data:image/jpeg;base64,PGh0bWw+..."}'
{"error":"AI analysis is disabled. Continue manually."}   # HTTP 503
$ npm run test:ingress-capture
AssertionError: the server must refuse non-image bytes, saw 503

CI is green only because .gitea/workflows/quality.yml does not set TIMMY_VISION_ENABLED=false, so the endpoint reaches ingress there. The builder's own new gate is therefore red on this branch: merging as-is makes npm run test:ingress-capture fail the release build.

Suggested fix in this branch: move the per-caller budget and ingressGuard(payload) ahead of the visionConfig.enabled early return, so invalid bytes are refused with 400 in every configuration while a well-formed photo still gets 503 when the provider is off. Then re-run the suite against a server started with TIMMY_VISION_ENABLED=false. The rest of the diff (byte sniffing, polyglot/trailing-data refusal, metadata refusal, header-bounded dimensions, 6MB/15s body limits, no-bytes-in-logs) tested 225/225 locally and read correctly.

Release-blocking finding on the release-builder gate this PR adds (not merged today). `scripts/build_release.py` starts the acceptance server with `TIMMY_VISION_ENABLED=false`, and `server.mjs` returns 503 from the `!visionConfig.enabled` branch *before* `ingressGuard` runs. `tests/ingress-capture.acceptance.mjs` then asserts 400 for the same bytes and fails: ``` $ HOST=127.0.0.1 PORT=4173 TIMMY_VISION_ENABLED=false node server.mjs & $ curl -s -X POST localhost:4173/api/analyze -H 'content-type: application/json' -d '{"consent":true,"imageDataUrl":"data:image/jpeg;base64,PGh0bWw+..."}' {"error":"AI analysis is disabled. Continue manually."} # HTTP 503 $ npm run test:ingress-capture AssertionError: the server must refuse non-image bytes, saw 503 ``` CI is green only because `.gitea/workflows/quality.yml` does not set `TIMMY_VISION_ENABLED=false`, so the endpoint reaches ingress there. The builder's own new gate is therefore red on this branch: merging as-is makes `npm run test:ingress-capture` fail the release build. Suggested fix in this branch: move the per-caller budget and `ingressGuard(payload)` ahead of the `visionConfig.enabled` early return, so invalid bytes are refused with 400 in every configuration while a well-formed photo still gets 503 when the provider is off. Then re-run the suite against a server started with `TIMMY_VISION_ENABLED=false`. The rest of the diff (byte sniffing, polyglot/trailing-data refusal, metadata refusal, header-bounded dimensions, 6MB/15s body limits, no-bytes-in-logs) tested 225/225 locally and read correctly.
rockachopa added 1 commit 2026-10-31 11:17:47 +00:00
Merge main into the #16 ingress branch and close what the RC gate caught
Quality gates / quality (pull_request) Canceled after 0s
Details
bc64055196 
Brings #82 up to today's `main` (1bbcc10) and resolves the two conflicts as ordered
unions: `package.json` `test`/`check:syntax` and the release builder's syntax allow-list
and evidence keys. Both sides only appended, so nothing was dropped.

Verifying the merge against main's RC attack suite (#40) found two real defects, each
fixed test-first:

- An oversized body destroyed the socket mid-upload, so the caller saw `fetch failed`
  instead of the 413 it had already earned, and `security_attack_suite.mjs` could not
  complete at all. New test in `tests/analyze-ingress.acceptance.test.js` failed with
  `fetch failed` before the fix; `src/http-body.js` now keeps draining without buffering
  and refuses on `end`, so the ceiling still holds and the refusal is readable.
- #16's per-caller budget turned the suite's own load phase into 8/8 429s, which would
  have published refusals as analysis latency. The load phase now runs against a second
  permissive target and the suite refuses to publish a load figure if any sample was
  refused. `buildReport` also publishes `evidenceVerified`, so the verdict is
  re-derivable from the report — asserted by #40's acceptance test, which only trips once
  the verdict is GO.

Evidence on the merged tree: `npm test` 277/277, `test:security-attack` 8/8, the RC attack
suite returns VERDICT: GO with both P0 ingress cases passing (main is BLOCKED on exactly
those two), 14 browser acceptance suites green, and the release demo records end to end
(8:13, 97 captions) including the on-camera refusal scene.
rockachopa commented 2026-10-31 11:18:13 +00:00
Member
Copy link
Copy source

Brought this PR up to main (1bbcc10) and closed what verifying it caught. It is mergeable again; still not merged, per the release contract.

Conflicts — both were pure appends, resolved as ordered unions: package.json test/check:syntax, and the release builder's syntax allow-list plus evidence keys. Nothing from either side was dropped.

Two defects found by running main's own RC attack suite (#40) against the merged tree, each fixed test-first:

Defect Failing test first Fix
An oversized body destroyed the socket mid-upload, so the caller got fetch failed instead of the 413, and the attack suite could not complete tests/analyze-ingress.acceptance.test.js — refuses an oversized body with a refusal the caller can read (failed with fetch failed) src/http-body.js drains without buffering and refuses on end; the 6 MB ceiling still holds
The per-caller budget turned the suite's own load phase into 8/8 429s, i.e. refusals published as analysis latency scripts/security_attack_suite.mjs — load phase now throws if any sample was refused (failed with 8/8 refusals) Load latency is measured against a second target with the budget opened up
The report dropped evidenceVerified, so the verdict was not re-derivable from what it published #40's existing acceptance test (test:security-attack) buildReport publishes the flag

Evidence on this head (bc64055):

  • npm test — 277/277, 0 fail, 0 skipped (run on the committed tree, clean status).
  • npm run test:security-attack — 8/8.
  • node scripts/security_attack_suite.mjs — VERDICT: GO, ingress-oversized-body, ingress-polyglot-html, ingress-garbage-bytes all pass with providerCalls 0; load 8 requests, p50 39 ms, p95 169 ms, errors 0, abstentions 8. On main the same command is BLOCKED on exactly those two ingress cases.
  • 14 browser acceptance suites green against a live server on this tree (ui, photo, mobile-capture, ingress-capture, sleek, manual-fallback, provisional, followup, vision-refusal, worker-readiness, pattern-summary, photo-lifecycle, training-contribution, portability).
  • scripts/record_release_demo.mjs records end to end on this tree — 8:13, 97 captions — including "A file that only claims to be a photo is refused on the device — nothing is uploaded" and the security scene that previously aborted at the attack suite.

docs/RC-SECURITY-EVIDENCE.md no longer predicts BLOCKED for #16; it states the run and leaves queue bounding (#17) as the open P1. No real medical image, secret, generated artifact, or node_modules entry is in the diff.

**Brought this PR up to `main` (1bbcc10) and closed what verifying it caught.** It is mergeable again; still not merged, per the release contract. **Conflicts** — both were pure appends, resolved as ordered unions: `package.json` `test`/`check:syntax`, and the release builder's syntax allow-list plus evidence keys. Nothing from either side was dropped. **Two defects found by running `main`'s own RC attack suite (#40) against the merged tree**, each fixed test-first: | Defect | Failing test first | Fix | |---|---|---| | An oversized body destroyed the socket mid-upload, so the caller got `fetch failed` instead of the 413, and the attack suite could not complete | `tests/analyze-ingress.acceptance.test.js` — *refuses an oversized body with a refusal the caller can read* (failed with `fetch failed`) | `src/http-body.js` drains without buffering and refuses on `end`; the 6 MB ceiling still holds | | The per-caller budget turned the suite's own load phase into 8/8 429s, i.e. refusals published as analysis latency | `scripts/security_attack_suite.mjs` — load phase now throws if any sample was refused (failed with `8/8 refusals`) | Load latency is measured against a second target with the budget opened up | | The report dropped `evidenceVerified`, so the verdict was not re-derivable from what it published | #40's existing acceptance test (`test:security-attack`) | `buildReport` publishes the flag | **Evidence on this head (bc64055):** - `npm test` — 277/277, 0 fail, 0 skipped (run on the committed tree, clean status). - `npm run test:security-attack` — 8/8. - `node scripts/security_attack_suite.mjs` — **VERDICT: GO**, `ingress-oversized-body`, `ingress-polyglot-html`, `ingress-garbage-bytes` all pass with `providerCalls 0`; load 8 requests, p50 39 ms, p95 169 ms, **errors 0**, abstentions 8. On `main` the same command is BLOCKED on exactly those two ingress cases. - 14 browser acceptance suites green against a live server on this tree (ui, photo, mobile-capture, ingress-capture, sleek, manual-fallback, provisional, followup, vision-refusal, worker-readiness, pattern-summary, photo-lifecycle, training-contribution, portability). - `scripts/record_release_demo.mjs` records end to end on this tree — 8:13, 97 captions — including *"A file that only claims to be a photo is refused on the device — nothing is uploaded"* and the security scene that previously aborted at the attack suite. `docs/RC-SECURITY-EVIDENCE.md` no longer predicts BLOCKED for #16; it states the run and leaves queue bounding (#17) as the open P1. No real medical image, secret, generated artifact, or `node_modules` entry is in the diff.
rockachopa added 1 commit 2026-10-31 12:06:16 +00:00
fix(ingress): refuse hostile bytes before the disabled-model shortcut (#16)
Quality gates / quality (pull_request) Canceled after 0s
Details
8ce5bc174b 
The release builder runs its browser gates with TIMMY_VISION_ENABLED=false, and
the 503 "AI analysis is disabled" branch ran before the request budget, body
limits, and byte-level ingress guard. Under builder conditions every hostile
upload answered 503, so the refusal boundary was never exercised and the
ingress-capture gate this same PR added failed:

  AssertionError: the server must refuse non-image bytes, saw 503

Refusals now run in cost order regardless of model availability: budget, body,
bytes, then the disabled switch. Verified with vision disabled: hostile bytes
-> 400 not_an_image, a valid synthetic photo -> 503 with the existing copy.
npm test 277 pass; ingress-capture and the other browser gates green under the
builder's env.
rockachopa merged commit c31d0a510f into main 2026-10-31 12:06:27 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
area/backend
Application API and storage
 area/data
Dataset and provenance
 area/frontend
PWA and interaction design
 area/model
Vision model and inference
 area/privacy
Privacy and consent
 area/release
Release engineering and distribution
 area/safety
Clinical safety boundary
 blocked
Blocked on an explicit dependency or decision
 human-gate
Requires explicit human approval
 priority/P0
Release-critical
 priority/P1
High priority
 priority/P2
Planned
 state:review
type/docs
Documentation and evidence
 type/epic
Roadmap epic
 type/feature
Product feature
 type/ops
Infrastructure and operations
 type/research
Research or evaluation
 type/security
Security hardening
 type/test
Verification and quality gate
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
timmy (Timmy · the Wizard Mind) zephyrus-sparkborn (Zephyrus Sparkborn)
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: stackchain/timmy-talking-turd#82