fix(hub): note kan het deny-citaat niet sluiten + bytegrens op de intake-context #181

Merged

janpeter merged 1 commit from fix/hub-ingest-hardening into main 2026-08-19 19:25:13 +02:00

Conversation 1 Commits 1 Files changed 6 +451 -2

janpeter commented 2026-08-19 18:57:29 +02:00

Owner

Copy link

Twee van de drie securitypunten die bij de review van de AskUserQuestion-brug buiten scope vielen, plus een plan voor de derde laag. Het derde punt (plain http op loopback in isSecureHubUrl) blijft bewust ongewijzigd — zie onderaan.

1. Een note kan het deny-citaat niet meer sluiten

userNoteReason in scripts/hooks/hub-permission-hook.mjs omkadert JP's toelichting als geciteerde tekst. Wat de oorspronkelijke review niet vaststelde maar wel bepalend is: bij allow is die string alleen voor de gebruiker zichtbaar, bij deny gaat hij als reden naar Claude. Daar telt de markering dus, en daar had een note met een » erin exact het gat dat eerder in hub-askuserquestion-hook.mjs gedicht is — tekst die ná het citaat lijkt te staan.

Zelfde fix, één regel: het sluitteken wordt geneutraliseerd (» → >>), de markeringszin blijft ongewijzigd. Restrisico dat blijft: newlines in de note blijven newlines. Het kanaal is HMAC-gebonden en komt van het gekoppelde toestel, dus dit is verdediging in de diepte, geen gat.

2. Bytegrens op context in de CUSTOM-intake

context was z.record(z.string(), z.unknown()) — elke diepte, elke omvang. De hook begrenst zijn eigen descriptions al, maar dat is cliëntdiscipline; het schema is het enige dat óók toekomstige intake-bronnen bindt, en dat veld is expres generiek.

Grens: 16 KiB, gemeten in bytes (dat is wat de database en het detailscherm werkelijk dragen, niet tekens). Ruim boven het zwaarste bekende geval: volledige vraagtekst + 8 descriptions van elk max 500 tekens ≈ 6 KB.

Eén koppeling om te kennen, die ook in de code als commentaar staat: de ask-hook laat een vraag tot 100 000 tekens toe en zet de volledige tekst in de context. Een vraag boven ~15 KB krijgt nu een 422, waarna de hook zwijgt en de vraag gewoon in de terminal verschijnt. Dat is de bedoelde degradatie, maar wel een gedragsverandering voor een pathologisch lange vraag.

3. Plan voor de request-size-guard

docs/superpowers/plans/2026-08-19-hub-ingest-request-size-guard.md — 4 taken, TDD, met exacte code.

De eerlijke grens van fix 2: Zod draait pas ná request.json(), dus een payload van 50 MB is al geparsed voordat hij wordt afgewezen. Het plan legt de grens daarom in withIngestSecret — de helper die alle vier de ingest-routes al aanroepen — en laat die de body zélf begrensd lezen en teruggeven, precies zoals withSignedDevice dat al doet met rawBody. Twee dingen die het plan expliciet benoemt in plaats van weglaat:

• Niet op Content-Length vertrouwen. Die header is optioneel (chunked) en door de cliënt te vervalsen; de guard telt de werkelijke bytestroom en gebruikt de header hooguit als goedkope voorcontrole. Er staat een test voor een leugenachtige header in.
• withSignedDevice blijft onbegrensd (await request.text()). Die routes zijn device-ondertekend en de handtekening wordt pas ná het lezen gecontroleerd; dat omdraaien is een grotere ingreep en een apart voorstel. Task 4 meet het en legt het vast in het REST-contract.

Het plan bevat ook een zusterbevinding die ik tijdens dit werk tegenkwam en bewust niet in deze PR heb opgelost, om binnen de goedgekeurde scope te blijven: de permissie-intake (app/api/hub/permissions/route.ts) heeft host, sessionLabel, toolName, toolInputSummary en cwd als kale z.string() — onbegrensd, terwijl het zusterschema overal grenzen heeft. Dat is Task 3 van het plan, met grenzen die aantoonbaar boven liggen wat de hook vandaag verstuurt.

Waarom isSecureHubUrl ongewijzigd blijft

De loopback-uitzondering op de https-eis is in productie ongebruikt (beide hosts wijzen naar https://thuis.jp-visser.nl), maar hij is wél wat het testen van de hele keten zonder telefoon mogelijk maakt — het hookscript tegen een neptaub op 127.0.0.1. Weghalen kost die testroute. De uitzondering bijt alleen op een host waar HUB_URL naar loopback wijst én een andere gebruiker een poort kan bezetten; dat is geen van beide fleet-hosts. Wil je hem tóch expliciet maken, dan is een opt-in (HUB_ALLOW_INSECURE_LOOPBACK=1) de vorm die het testpad heel laat.

Verificatie

TDD: eerst drie falende tests (RED aangetoond), daarna de fixes. npm run verify groen — 272 bestanden, 2201 tests (was 2197). npm run docs groen, 224 docs geïndexeerd.

Twee van de drie securitypunten die bij de review van de AskUserQuestion-brug buiten scope vielen, plus een plan voor de derde laag. Het derde punt (plain `http` op loopback in `isSecureHubUrl`) blijft bewust ongewijzigd — zie onderaan. ## 1. Een note kan het deny-citaat niet meer sluiten `userNoteReason` in `scripts/hooks/hub-permission-hook.mjs` omkadert JP's toelichting als geciteerde tekst. Wat de oorspronkelijke review niet vaststelde maar wel bepalend is: **bij `allow` is die string alleen voor de gebruiker zichtbaar, bij `deny` gaat hij als reden naar Claude.** Daar telt de markering dus, en daar had een note met een `»` erin exact het gat dat eerder in `hub-askuserquestion-hook.mjs` gedicht is — tekst die ná het citaat lijkt te staan. Zelfde fix, één regel: het sluitteken wordt geneutraliseerd (`»` → `>>`), de markeringszin blijft ongewijzigd. Restrisico dat blijft: newlines in de note blijven newlines. Het kanaal is HMAC-gebonden en komt van het gekoppelde toestel, dus dit is verdediging in de diepte, geen gat. ## 2. Bytegrens op `context` in de CUSTOM-intake `context` was `z.record(z.string(), z.unknown())` — elke diepte, elke omvang. De hook begrenst zijn eigen descriptions al, maar dat is cliëntdiscipline; het schema is het enige dat óók **toekomstige** intake-bronnen bindt, en dat veld is expres generiek. Grens: 16 KiB, gemeten in **bytes** (dat is wat de database en het detailscherm werkelijk dragen, niet tekens). Ruim boven het zwaarste bekende geval: volledige vraagtekst + 8 descriptions van elk max 500 tekens ≈ 6 KB. Eén koppeling om te kennen, die ook in de code als commentaar staat: de ask-hook laat een vraag tot 100 000 tekens toe en zet de **volledige** tekst in de context. Een vraag boven ~15 KB krijgt nu een 422, waarna de hook zwijgt en de vraag gewoon in de terminal verschijnt. Dat is de bedoelde degradatie, maar wel een gedragsverandering voor een pathologisch lange vraag. ## 3. Plan voor de request-size-guard `docs/superpowers/plans/2026-08-19-hub-ingest-request-size-guard.md` — 4 taken, TDD, met exacte code. De eerlijke grens van fix 2: **Zod draait pas ná `request.json()`**, dus een payload van 50 MB is al geparsed voordat hij wordt afgewezen. Het plan legt de grens daarom in `withIngestSecret` — de helper die alle vier de ingest-routes al aanroepen — en laat die de body zélf begrensd lezen en teruggeven, precies zoals `withSignedDevice` dat al doet met `rawBody`. Twee dingen die het plan expliciet benoemt in plaats van weglaat: - **Niet op `Content-Length` vertrouwen.** Die header is optioneel (chunked) en door de cliënt te vervalsen; de guard telt de werkelijke bytestroom en gebruikt de header hooguit als goedkope voorcontrole. Er staat een test voor een leugenachtige header in. - **`withSignedDevice` blijft onbegrensd** (`await request.text()`). Die routes zijn device-ondertekend en de handtekening wordt pas ná het lezen gecontroleerd; dat omdraaien is een grotere ingreep en een apart voorstel. Task 4 meet het en legt het vast in het REST-contract. Het plan bevat ook een zusterbevinding die ik tijdens dit werk tegenkwam en bewust **niet** in deze PR heb opgelost, om binnen de goedgekeurde scope te blijven: de permissie-intake (`app/api/hub/permissions/route.ts`) heeft `host`, `sessionLabel`, `toolName`, `toolInputSummary` en `cwd` als kale `z.string()` — onbegrensd, terwijl het zusterschema overal grenzen heeft. Dat is Task 3 van het plan, met grenzen die aantoonbaar boven liggen wat de hook vandaag verstuurt. ## Waarom `isSecureHubUrl` ongewijzigd blijft De loopback-uitzondering op de https-eis is in productie ongebruikt (beide hosts wijzen naar `https://thuis.jp-visser.nl`), maar hij is wél wat het testen van de hele keten zonder telefoon mogelijk maakt — het hookscript tegen een neptaub op `127.0.0.1`. Weghalen kost die testroute. De uitzondering bijt alleen op een host waar `HUB_URL` naar loopback wijst én een andere gebruiker een poort kan bezetten; dat is geen van beide fleet-hosts. Wil je hem tóch expliciet maken, dan is een opt-in (`HUB_ALLOW_INSECURE_LOOPBACK=1`) de vorm die het testpad heel laat. ## Verificatie TDD: eerst drie falende tests (RED aangetoond), daarna de fixes. `npm run verify` groen — 272 bestanden, **2201 tests** (was 2197). `npm run docs` groen, 224 docs geïndexeerd.

janpeter added 1 commit 2026-08-19 18:57:29 +02:00

fix(hub): note kan het deny-citaat niet sluiten + bytegrens op de intake-context ... 26e278ba8d

Twee bevindingen uit de securityreview van de AskUserQuestion-brug die daar
buiten scope vielen, plus een plan voor de derde laag.

1. userNoteReason: bij een deny gaat deze reden als tekst naar Claude. Een note
   die zelf een sluitteken bevat produceerde tekst die ná het citaat lijkt te
   staan — precies wat de markering hoort te voorkomen. Zelfde fix als eerder in
   hub-askuserquestion-hook.mjs: het sluitteken wordt geneutraliseerd, de
   markeringszin blijft ongewijzigd.

2. De CUSTOM-intake accepteerde een onbegrensde `context` (z.record van
   z.unknown). De cliënten begrenzen zichzelf al, maar het schema is het enige
   dat óók toekomstige bronnen bindt. Grens op 16 KiB, in bytes gemeten. Let op
   de koppeling: de ask-hook laat een vraag tot 100_000 tekens toe en zet de
   volledige tekst in de context — boven ~15 KB volgt nu een 422, waarna de hook
   zwijgt en de vraag in de terminal verschijnt (de bedoelde degradatie).

Plus docs/superpowers/plans/2026-08-19-hub-ingest-request-size-guard.md: het
plan voor een begrensde bodyread in withIngestSecret, inclusief de zusterbevinding
(onbegrensde strings in de permissie-intake) en een expliciete notitie over wat
het níét dekt (withSignedDevice leest nog steeds onbegrensd).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

s4m-codex-reviewer approved these changes 2026-08-19 19:01:05 +02:00

s4m-codex-reviewer left a comment

Copy link

APPROVED

geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden.

Findings

• Geen blokkerende of error-severity findings.

Review-notities

• app/api/hub/approvals/intake/route.ts:21 introduceert een expliciete MAX_CONTEXT_BYTES-grens en valideert op UTF-8 bytes via Buffer.byteLength(JSON.stringify(v)), met tests voor overschrijding, meerbyte-tekens en een normale hook-context.
• scripts/hooks/hub-permission-hook.mjs:526 neutraliseert » in user notes voordat de note in geciteerde tekst naar Claude gaat; de toegevoegde test dekt dat de note het citaat niet meer kan sluiten.
• Het toegevoegde planbestand beschrijft een bredere request-size guard, maar is niet als linked_plan bij deze review aangeleverd en wordt daarom niet als acceptatiecriterium voor deze PR afgedwongen.

# APPROVED geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. ## Findings - Geen blokkerende of error-severity findings. ## Review-notities - `app/api/hub/approvals/intake/route.ts:21` introduceert een expliciete `MAX_CONTEXT_BYTES`-grens en valideert op UTF-8 bytes via `Buffer.byteLength(JSON.stringify(v))`, met tests voor overschrijding, meerbyte-tekens en een normale hook-context. - `scripts/hooks/hub-permission-hook.mjs:526` neutraliseert `»` in user notes voordat de note in geciteerde tekst naar Claude gaat; de toegevoegde test dekt dat de note het citaat niet meer kan sluiten. - Het toegevoegde planbestand beschrijft een bredere request-size guard, maar is niet als `linked_plan` bij deze review aangeleverd en wordt daarom niet als acceptatiecriterium voor deze PR afgedwongen.

janpeter merged commit 011b014f5a into main 2026-08-19 19:25:13 +02:00

janpeter referenced this pull request from a commit 2026-08-19 19:25:14 +02:00

Merge pull request 'fix(hub): note kan het deny-citaat niet sluiten + bytegrens op de intake-context' (#181) from fix/hub-ingest-hardening into main

Sign in to join this conversation.

Reviewers

No reviewers

s4m-codex-reviewer

Labels

Clear labels

No items

No labels

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

janpeter/Scrum4Me!181

No description provided.