janpeter/Scrum4Me
0
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(hub): device-handtekening onafhankelijk van wire-codering (IDEA-183, schema v2) #167

Merged
janpeter merged 18 commits from feat/hub-signature-canonicalisatie into main 2026-08-14 21:28:27 +02:00
Conversation 1 Commits 18 Files changed 17 +1794 -15
janpeter commented 2026-08-14 21:06:41 +02:00
Owner
Copy link

Het deelnemer-filter in de hub-inbox gaf altijd 401. Oorzaak: de canonieke string werd aan de twee kanten uit verschillende bronnen opgebouwd — de client tekende de letterlijke querystring die hij verstuurde, de server url.pathname + url.search van een heropgebouwde URL, en Next codeert daarbij : naar %3A. Elke query-waarde met zo'n teken faalde.

Gevonden tijdens de M33-e2e-gate (item 3), gediagnosticeerd met een tijdelijk testapparaat tegen de draaiende server: %3A tekenen en : versturen gaf 200, andersom 401 — dus de server rekent altijd met %3A. Direct en via Caddy identiek, dus de proxy was niet de oorzaak.

De fix

Schema v2: beide kanten percent-decoderen het request-target naar bytes en bouwen het opnieuw op volgens één opgeschreven regel. Daarmee is niet dit ene geval opgelost maar de hele klasse — hoe een laag onderweg normaliseert doet niet meer ter zake.

  • canonicalTarget(pathname, search) in TypeScript én Swift, gehouden door één gedeeld vectorbestand (tests/fixtures/hub-canonical-target.json, 21 vectoren) dat beide kanten lezen.
  • X-Hub-Sig-Version: 2 kiest het schema per verzoek; het versienummer staat óók ín de ondertekende tekst, dus sleutelen aan die header kan een verzoek alleen laten falen. v1 en v2 zijn per constructie disjunct (v1 begint met method.toUpperCase(), v2 met de kleine letters v2).
  • Een rauwe + in een v2-querystring geeft 400: url.searchParams leest + als spatie en %2B als plus terwijl beide dezelfde canonieke string opleveren — één handtekening zou anders twee betekenissen dekken.
  • De iOS-client codeert een letterlijke + nu als %2B op de lijn (handtekening-neutraal), zodat hij niet tegen die eigen 400 aanloopt.

Uitrolvolgorde — dwingend

Server eerst, iOS-build daarna. De server accepteert tijdelijk beide schema's; een ontbrekende header betekent v1, dus de nu geïnstalleerde app blijft ongestoord werken. De nieuwe build tekent uitsluitend v2 en heeft géén terugval — andersom uitrollen legt de hub stil.

Voorwaarde vóór TestFlight: aantonen dat X-Hub-Sig-Version door Caddy heen komt. Strippt een tussenlaag die header, dan valt de server terug op v1 en krijgt de nieuwe app op álles 401 — niet te onderscheiden van de bug die we repareren. De zevenrijige verificatiematrix (direct én via Caddy) staat in docs/runbooks/hub-device-signature.md.

Schema 1 verdwijnt in een aparte PR, pas ná bevestiging dat de nieuwe build draait.

Verificatie

  • Wegwerp-clone-gate groen op d0a823a: npm ci && npm run verify && npm run build && git diff --exit-code.
  • npm run verify: 2047 tests. npm run ios:test: 58/58.
  • Spec en plan zijn twee ronden adversarieel gereviewd (verdict GO). Elke taak apart gereviewd, plus een brede eindreview over de hele branch die de drie dragende claims onafhankelijk natrok: 180.105 differentiële gevallen door beide implementaties met nul verschillen, een botsingsscan over 150.101 gevallen met nul botsingen in de query, en de v1/v2-disjunctheid.

De unittests kunnen het productiedefect níét reproduceren — een kale new Request laat de dubbele punt ongemoeid. Dat staat expliciet in de spec en het runbook; het bewijs komt van de eigenschapstest en van de live verificatie ná deploy.

🤖 Generated with Claude Code

Het deelnemer-filter in de hub-inbox gaf altijd 401. Oorzaak: de canonieke string werd aan de twee kanten uit **verschillende bronnen** opgebouwd — de client tekende de letterlijke querystring die hij verstuurde, de server `url.pathname + url.search` van een heropgebouwde URL, en Next codeert daarbij `:` naar `%3A`. Elke query-waarde met zo'n teken faalde. Gevonden tijdens de M33-e2e-gate (item 3), gediagnosticeerd met een tijdelijk testapparaat tegen de draaiende server: `%3A` tekenen en `:` versturen gaf 200, andersom 401 — dus de server rekent altijd met `%3A`. Direct en via Caddy identiek, dus de proxy was niet de oorzaak. ## De fix Schema **v2**: beide kanten percent-decoderen het request-target naar bytes en bouwen het opnieuw op volgens één opgeschreven regel. Daarmee is niet dit ene geval opgelost maar de hele klasse — hoe een laag onderweg normaliseert doet niet meer ter zake. - `canonicalTarget(pathname, search)` in TypeScript én Swift, gehouden door één gedeeld vectorbestand (`tests/fixtures/hub-canonical-target.json`, 21 vectoren) dat beide kanten lezen. - `X-Hub-Sig-Version: 2` kiest het schema per verzoek; het versienummer staat óók ín de ondertekende tekst, dus sleutelen aan die header kan een verzoek alleen laten falen. v1 en v2 zijn per constructie disjunct (v1 begint met `method.toUpperCase()`, v2 met de kleine letters `v2`). - Een **rauwe `+`** in een v2-querystring geeft 400: `url.searchParams` leest `+` als spatie en `%2B` als plus terwijl beide dezelfde canonieke string opleveren — één handtekening zou anders twee betekenissen dekken. - De iOS-client codeert een letterlijke `+` nu als `%2B` op de lijn (handtekening-neutraal), zodat hij niet tegen die eigen 400 aanloopt. ## Uitrolvolgorde — dwingend **Server eerst, iOS-build daarna.** De server accepteert tijdelijk beide schema's; een ontbrekende header betekent v1, dus de nu geïnstalleerde app blijft ongestoord werken. De nieuwe build tekent uitsluitend v2 en heeft géén terugval — andersom uitrollen legt de hub stil. **Voorwaarde vóór TestFlight:** aantonen dat `X-Hub-Sig-Version` door Caddy heen komt. Strippt een tussenlaag die header, dan valt de server terug op v1 en krijgt de nieuwe app op álles 401 — niet te onderscheiden van de bug die we repareren. De zevenrijige verificatiematrix (direct én via Caddy) staat in `docs/runbooks/hub-device-signature.md`. Schema 1 verdwijnt in een aparte PR, pas ná bevestiging dat de nieuwe build draait. ## Verificatie - Wegwerp-clone-gate groen op `d0a823a`: `npm ci && npm run verify && npm run build && git diff --exit-code`. - `npm run verify`: 2047 tests. `npm run ios:test`: 58/58. - Spec en plan zijn twee ronden adversarieel gereviewd (verdict GO). Elke taak apart gereviewd, plus een brede eindreview over de hele branch die de drie dragende claims onafhankelijk natrok: 180.105 differentiële gevallen door beide implementaties met nul verschillen, een botsingsscan over 150.101 gevallen met nul botsingen in de query, en de v1/v2-disjunctheid. De unittests kunnen het productiedefect níét reproduceren — een kale `new Request` laat de dubbele punt ongemoeid. Dat staat expliciet in de spec en het runbook; het bewijs komt van de eigenschapstest en van de live verificatie ná deploy. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Ontwerp voor IDEA-183. De canonieke string wordt aan de twee kanten uit
verschillende bronnen opgebouwd: de client tekent de letterlijke querystring,
de server bouwt hem uit een opnieuw geserialiseerde URL. Next codeert daarbij
de dubbele punt naar %3A, waardoor elk device-signed verzoek met zo'n teken in
een query-waarde 401 geeft.

Opzet: beide kanten decoderen eerst en bouwen opnieuw op volgens één regel
(alleen RFC-3986-unreserved blijft staan, hoofdletters in de hex). Daarmee doet
de wire-codering er niet meer toe en is de hele klasse gesloten, niet alleen de
dubbele punt.

Vastgelegde keuzes uit de brainstorm: versiemarkering in de header én in de
ondertekende tekst (sleutelen aan de header kan dan alleen laten falen, nooit
laten slagen); dual-accept tot JP bevestigt dat de nieuwe build draait; pad plus
query, geen sortering; server eerst uitrollen.

Onderweg een tweede divergentie van dezelfde klasse gevonden en dichtgezet: de
`+` in een querywaarde is bij WHATWG een spatie en bij Foundation een plusteken.
Vastgelegd als letterlijk plusteken, dus geen formulier-decodering.

Drift tussen Swift en TypeScript wordt gevangen met gedeelde testvectoren —
Swift en TS passen niet in één testproces, dus de truc uit M34 (beide kanten in
één test laden) kan hier niet.

Bewijs uit de diagnose zit in de spec: de kruisproef "teken %3A, verstuur :" gaf
200 en de omgekeerde 401, en beide routes (direct en via Caddy) gedroegen zich
identiek.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Zes taken, TDD per taak. Task 1 legt het contract vast als gedeelde
testvectoren en implementeert de TS-kant; 2 zet er de versieregel en de
schemakeuze op; 3 en 4 doen de Swift-kant tegen exact dezelfde vectoren; 5 is
runbook plus live verificatie; 6 verwijdert schema 1 en start pas ná JP's
bevestiging dat de nieuwe build draait.

Twee dingen die ik in de zelfreview repareerde. Het plan verwees twee keer naar
"zoek zelf het juiste testbestand" — dat is precies wat een plan niet moet doen;
nu staan `__tests__/lib/hub/route-helpers.test.ts` en `DeviceSignerTests.swift`
er bij naam in, met de bestaande helpers erbij. En de uitrolvolgorde ontbrak als
harde stap: Task 4 levert een app die uitsluitend v2 tekent, dus daar staat nu
een hardstop dat de server eerst gedeployed moet zijn.

De Global Constraints bevatten de regel die het makkelijkst fout gaat: werk op
bytes, nooit op gedecodeerde Strings. Een tussenstap via String maakt ongeldige
UTF-8 links U+FFFD en rechts iets anders — dezelfde klasse divergentie als de
dubbele punt zelf.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
signedRequest zette components.percentEncodedQuery = query rechtstreeks;
Foundation codeert een letterlijke + in een al-percent-gecodeerde query
niet (het is een toegestaan sub-delim-teken). participant is vrije tekst
op iOS, dus een waarde met een + ging rauw over de lijn en de server
weigerde het v2-verzoek met 400 (InboxStore.fetch verdronk die status +
boodschap in "Inbox laden mislukt" — een stil kapotte filter).

Herschrijf de + naar %2B ná het berekenen van de handtekening, zodat
elke aanroeper via signedRequest profiteert. Signature-neutraal:
canonicalTarget codeert een rauwe + en %2B al naar dezelfde %2B, dus de
canonieke string blijft ongewijzigd. Repareert en passant een
pre-existing v1-misread waarbij de app "a+b" stuurde en de handler
"a b" las.

DeviceSignerTests pint beide kanten: de URL op de lijn draagt %2B en
geen kale +, en de handtekening verifieert nog steeds tegen de
canonieke tekst van de ORIGINELE, niet-herschreven query.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
De bestaande raw-+ test dekte alleen ?p=a+b (een waarde). De guard zelf
(url.search.includes('+')) dekt sleutels al door constructie, maar de
regel in route-helpers-server.ts spreekt expliciet over sleutels én
waarden ("?a+b=1 is hetzelfde defect op de naam") — dus hoort er ook een
test voor te zijn: ?p+q=a.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
De fixture-klim vanaf #filePath stopte bij de eerste ancestor met
tests/fixtures/hub-canonical-target.json. In een worktree-checkout loopt
die ouder-keten óók door de hoofdrepo, die zijn eigen kopie heeft — de
dichtstbijzijnde treffer wint, wat vandaag correct is, maar ontbrak de
worktree-kopie ooit, dan zou de test stilletjes tegen de vectoren van
een andere checkout valideren.

Assert nu dat het gevonden bestand onder de wortel van dít package valt
(de ouder van de bovenste ios-map in het pad naar #filePath), met een
duidelijke XCTFail als dat niet zo is.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
docs(hub): IDEA-183 review-fix — matrix in het runbook, status, gate-note
All checks were successful
CI / Lint, Typecheck, Test & Build (pull_request) Successful in 4m20s
Details
CI / Deploy Manual (workflow_dispatch) (pull_request) Has been skipped
Details
CI / Detect deploy-relevant changes (pull_request) Has been skipped
Details
CI / Deploy Preview (PR) (pull_request) Has been skipped
Details
CI / Deploy Production (main) (pull_request) Has been skipped
Details
d0a823ab2b 
- runbook §5: kopieer de 7-rijige verificatiematrix (direct + via Caddy)
  uit het plan (Task 5 Step 4) in plaats van te verwijzen naar spec §7,
  dat maar ~3 van de 7 rijen als prosa dekt (mist beide
  schema-kruisende 401's, de raw-+ 400 en de v1-met-: 401 — precies de
  rijen die een gestripte X-Hub-Sig-Version-header zouden opvangen).
  Het runbook is het duurzame artefact voor deze pre-TestFlight-gate,
  niet een plan of spec.
- spec: status draft → reviewed-go — twee onafhankelijke reviewrondes
  gaven al GO (waarde al elders in docs/superpowers/specs/ in gebruik).
  Runbook blijft op status: active (beschrijft as-built code).
- hub-inbox-e2e-gate.md item 3: korte NO-GO-notitie met verwijzing naar
  hub-device-signature.md — die 401 op het deelnemer-filter is precies
  wat IDEA-183 oplost.
- spec §3.5 + runbook §4: een tweede, ongerelateerde botsing van
  dezelfde klasse in het pad (/% en /%25 canonicaliseren beide naar
  /%25, terwijl decodeURIComponent ze verschillend behandelt: waarde
  versus URIError). Onschadelijk — alle 36 gevonden botsingsklassen
  waren "fout versus waarde", nooit "waarde versus andere waarde" — en
  vereist een on-path herschrijver binnen TLS om te bereiken.
- spec §3.5: het niet-overwogen alternatief vastgelegd — + ongemoeid
  laten in de canonieke vorm (in plaats van hercoderen) zou de
  dubbelzinnigheid ook oplossen, maar zou de canonieke vorm laten
  afhangen van één draadteken dat nooit herschreven wordt — precies de
  aanname die dit ontwerp voor elk ander teken vermijdt.

docs/INDEX.md geregenereerd via npm run docs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
s4m-codex-reviewer approved these changes 2026-08-14 21:11:27 +02:00
s4m-codex-reviewer left a comment
Copy link

Verdict: APPROVED

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

Findings

  • Geen blokkerende findings gevonden.

Review-notities

  • lib/hub/route-helpers-server.ts:60 behoudt backwards compatibility door schema 1 als default te laten werken wanneer X-Hub-Sig-Version ontbreekt, en schakelt schema 2 alleen expliciet in.
  • lib/hub/signature-server.ts:44 en ios/Hub/Sources/Hub/Core/CanonicalTarget.swift:1 implementeren dezelfde byte-gebaseerde canonicalisatie met gedeelde fixture-dekking.
  • __tests__/lib/hub/route-helpers.test.ts:120, __tests__/lib/hub/canonical-target.test.ts:1 en ios/Hub/Tests/HubTests/CanonicalTargetTests.swift:1 dekken regressies rond v1/v2-keuze, gecodeerde versus ongecodeerde :, rauwe +, en Swift/TypeScript-vectorpariteit.
  • De toegevoegde runbook-documentatie in docs/runbooks/hub-device-signature.md:1 legt de signing-invariant en route-handler-waarschuwingen vast, passend bij de product-doc standaarden voor Hub-werk.
# Verdict: APPROVED Geen gekoppeld plan gevonden — beoordeeld op codekwaliteit + product-standaarden. ## Findings - Geen blokkerende findings gevonden. ## Review-notities - `lib/hub/route-helpers-server.ts:60` behoudt backwards compatibility door schema 1 als default te laten werken wanneer `X-Hub-Sig-Version` ontbreekt, en schakelt schema 2 alleen expliciet in. - `lib/hub/signature-server.ts:44` en `ios/Hub/Sources/Hub/Core/CanonicalTarget.swift:1` implementeren dezelfde byte-gebaseerde canonicalisatie met gedeelde fixture-dekking. - `__tests__/lib/hub/route-helpers.test.ts:120`, `__tests__/lib/hub/canonical-target.test.ts:1` en `ios/Hub/Tests/HubTests/CanonicalTargetTests.swift:1` dekken regressies rond v1/v2-keuze, gecodeerde versus ongecodeerde `:`, rauwe `+`, en Swift/TypeScript-vectorpariteit. - De toegevoegde runbook-documentatie in `docs/runbooks/hub-device-signature.md:1` legt de signing-invariant en route-handler-waarschuwingen vast, passend bij de product-doc standaarden voor Hub-werk.
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
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!167
No description provided.