diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md b/docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md index 37451f0..708af24 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/07-bio2-compliance.md @@ -1,9 +1,1128 @@ -## Scope: [to be filled by agent] +## Scope: apps/ssp (auth, registratie, herregistratie, brief, showcase+shell+root), apps/behandelportal (auth, behandeling, shell+root), libs/shared (per layer), libs/beheer, backend (Program.cs, Domain, Data, Zgw, Contracts, Stamdata) — controls 5.12, 5.13, 8.15, 8.16, 8.24, 8.25, 8.26, 8.28, 8.29, 8.32, 9.1, 9.2, 9.4 -## Status: not_started +## Status: complete -## Last updated: - +## Last updated: 2026-08-27 -## Depends on: [see agent prompt] +## Depends on: 00-baseline.md, 02-testability.md, 04-cqrs-light.md, 06-adr-conformance.md ## --- + +# 07 — BIO2 / ISO 27002:2022 compliance + +**Scope, in full.** Frontend: `ssp/auth`, `ssp/registratie`, `ssp/herregistratie`, `ssp/brief`, +`ssp/showcase+shell+root`, `bhp/auth`, `bhp/behandeling`, `bhp/shell+root`, `libs/shared` +(domain, application, infrastructure, ui, layout, kernel, upload, testing, environments), +`libs/beheer`. Backend: `Program.cs`, `Domain`, `Data`, `Zgw`, `Contracts`, `Stamdata`. +Controls: 5.12, 5.13, 8.15, 8.16, 8.24, 8.25, 8.26, 8.28, 8.29, 8.32, 9.1, 9.2, 9.4. + +--- + +## 0. The control-set assumption, stated for the record + +**No explicit control list was supplied to this agent.** The seven areas below were selected +for privacy and security relevance to a BIG-register portal handling BSN, diploma and +health-professional registration data: + +| # | Area | ISO 27002:2022 / BIO2 | +| --- | ------------------------------ | --------------------- | +| 1 | Access control | 9.1, 9.2, 9.4 | +| 2 | Logging & monitoring | 8.15, 8.16 | +| 3 | Data classification & handling | 5.12, 5.13 | +| 4 | Cryptography | 8.24 | +| 5 | Secure development | 8.25, 8.28, 8.29 | +| 6 | Change control | 8.32 | +| 7 | Input validation | 8.26 | + +**Flag if a different set should apply.** Three plausible narrowings/widenings a reviewer +should decide on before this file is treated as authoritative: + +- **BIO 2.0 thema-uitwerkingen** rather than raw ISO 27002 — a Dutch government system would + normally be assessed against the BIO's own thematic elaborations (toegangsbeveiliging, + logging & monitoring), which are stricter on logging retention and on the "verwerking van + bijzondere persoonsgegevens" than the bare ISO controls used here. Nothing below would be + withdrawn under that set; several items would rise in severity. +- **NEN 7510** (Dutch healthcare information security) is arguably the governing standard for + a register of healthcare professionals. Not applied here. +- **AVG/GDPR obligations proper** (art. 5 minimisation, art. 9 special-category, art. 30 + register of processing, art. 32 measures) are referenced only where the code itself invokes + them. A DPIA is out of scope for this pass and is listed in the pre-production checklist. + +**Framing, per the brief.** This POC has deliberately faked authentication; CLAUDE.md's +"Out of scope" excludes real auth/DigiD and PRD-0002 §3 excludes real AD/OIDC/SAML. **No +finding below asks for real DigiD or employee SSO.** Findings are split: + +- **Defect now** — wrong even for a POC. Typically: PII reaching a store or a log that the + code's own contract says holds none, or a missing authorization check that has nothing to + do with the identity stub. +- **Production gate** — correct for a POC, must be true before production. These are the + pre-production checklist at the end. + +Nine of the twenty findings are **defect now**. Where a dev-only affordance is genuinely +stripped from a production build, that is said plainly and no finding is filed +(see §1.4 and the module notes). + +--- + +## 1. Control area: access control (9.1, 9.2, 9.4) + +### BIO-001 — the backend trusts client-asserted identity headers in every environment + +- **Control:** 9.2 (user access provisioning), 9.4 (least privilege / secure log-on) +- **Class:** **production gate** +- **Severity: high** — a single request header grants the full admin capability set. It is + high not because it is unknown (it is documented in three places) but because it is the + item on which every other authorization control in the system rests: `Authz`, the + capability model, the four-eyes rule and the audit trail are all correct _given_ a + trustworthy `Principal`, and all worthless without one. +- **Evidence (read):** + - `backend/src/BigRegister.Api/Domain/Authorization/StubIdentityProvider.cs:23-27` — + `ctx.Request.Headers["X-Role"]` maps directly to `PrincipalRole.Admin`. No verification, + no signature, no environment guard. + - `:29-31` — `X-Medewerker` present ⇒ `MedewerkerCaller`, id taken verbatim from the header; + `:38-46` `X-Rollen` likewise. + - `:33-35` — `X-Subject` sets the caller's BSN, i.e. the ownership key every owner-scoped + store reads. + - `backend/src/BigRegister.Api/Program.cs:53` registers this as the only `IIdentityProvider` + unconditionally; `:114-119` runs it as middleware for every request in every environment. + - `Domain/Authorization/Authz.cs:12-16` and `StubIdentityProvider.cs:7-9` both label it + "dev stub — NOT a security boundary". The labelling is accurate and complete; **nothing + in the build enforces it.** +- **Baseline citation:** §7 Backend pattern inventory — "Single-impl interface | + `IIdentityProvider` → `StubIdentityProvider`"; **BL-006** (the backend has zero automated + architecture enforcement, so nothing would fail a build that shipped this stub). +- **Remediation, minimal:** do not replace the stub in this backlog. Two cheap, in-scope + steps: (a) fail fast — throw at startup when + `builder.Environment.IsProduction() && provider is StubIdentityProvider`, so the stub can + never boot outside Development; (b) give `IIdentityProvider.Resolve` a way to say "no + identity" (see BIO-002), so the production swap is a drop-in rather than a redesign. +- **Effort:** S (a), S (b). The real provider is out of scope and is a checklist item, not a + ticket. + +### BIO-002 — in a production build the backoffice has no identity, and the default is a citizen + +_Agent 06 handed this over explicitly (`06-adr-conformance.md`, "Observation for agent 07"). +Here is what it actually means for 9.4._ + +- **Control:** 9.4 (least privilege), 9.2 (provisioning); PRD-0002 §4 goal 4 (deny-by-default) +- **Class:** **production gate** +- **Severity: high** — the failure mode is an _identity substitution_, not merely a missing + identity, and it fails open in the direction nobody checked. +- **Evidence (read), and the answer to "what identity does a production backoffice user get":** + - `apps/behandelportal/src/app/app.config.ts:57-63` — `medewerkerInterceptor` is inside the + `isDevMode()` array. A production bundle sends **no** `X-Medewerker` / `X-Rollen`. + - `StubIdentityProvider.cs:29-37` — with no `X-Medewerker` and no `X-Subject`, the provider + falls through to `new ZorgverlenerCaller(DocumentStore.DemoOwner, SeedData.Registration.Naam, +PrincipalRole.Drafter)`. `DocumentStore.cs:48` — `DemoOwner = "123456782"`, the single + seeded citizen's BSN. + - **So a production backoffice user authenticates to the backend as the seeded citizen, + role `drafter`.** Concretely: + - **Fails closed, correctly, on the backoffice capability.** + `Authz.CanBeoordelen(caller)` is `caller is MedewerkerCaller m && …`, so a zorgverlener + is `false` regardless of `X-Role`. `GET /werkvoorraad`, `GET /beoordeling/{id}` and + `POST /beoordeling/{id}/besluit` all 403 through the `Beoordelen` gate + (`Program.cs:814-820`) and write a deny audit row. `GET /me` returns an empty capability + list, so `capabilityGuard('aanvraag:beoordelen')` (`app.routes.ts:25`) denies too. This + part of the design is right and should be recorded as such. + - **Fails open on the citizen's own rights.** The same user _is_ the seeded citizen for + every citizen-scoped endpoint: `GET /applications`, `GET /applications/{id}`, + `PUT/DELETE /applications/{id}`, `POST /applications/{id}/submit`, `DELETE /uploads/{id}`, + `GET|PUT /brief`, `POST /brief/submit|send|reset` (`Program.cs:281-372`, `:253`, + `:603-656`) all resolve `ctx.Zorgverlener().Bsn` to `123456782`. An employee with no + employee identity is granted a **citizen's** read and write rights over that citizen's + aanvragen, uploads and letters. + - **Holds the PII-reveal capability.** `Authz.CanRevealBigNummer(principal)` is + `principal.Role == PrincipalRole.Drafter` — and `drafter` is exactly the role the + no-header default produces. See BIO-006. + - **Root cause, and why it is worth a ticket now:** `IIdentityProvider.Resolve` returns a + non-nullable `CallerIdentity` (`IIdentityProvider.cs:12`). The interface **cannot express + "no identity"**, so any implementation — stub or real — is forced to invent one for an + unauthenticated request. `CallerIdentityHttpContextExtensions.Caller()` + (`CallerIdentity.cs:44-50`) already throws rather than defaulting when the middleware did + not run, i.e. the codebase reaches for fail-loud one layer up and then defaults one layer + down. +- **Baseline citation:** §7 Backend — "Single-impl interface | `IIdentityProvider` → + `StubIdentityProvider`"; §2 size inventory (`apps/behandelportal` 29 src files, 1 309 lines + — a whole app with no non-dev identity path). +- **Remediation, minimal:** change `Resolve` to `CallerIdentity?` and have the middleware + either reject (401) or set an explicit `AnonymousCaller` when it returns null; keep + `StubIdentityProvider` returning the current default **only** under + `IHostEnvironment.IsDevelopment()`. That is the smallest change that makes "unauthenticated" + representable, and it is the natural home for agent 06's **ADR-C-004** (`Session → Principal`). +- **Effort:** S for the interface + middleware; the behandelportal's real login is out of scope. + +### BIO-003 — `X-Admin` is a second authorization gate, outside `Authz`, unaudited, with no caller + +- **Control:** 9.4; 8.15 (audit trail); PRD-0002 §7 ("a single shared authorization helper … + used on every endpoint … the helper makes emit and enforce the same code path") +- **Class:** **defect now** +- **Severity: medium** — a destructive cross-owner delete behind the weakest gate in the + system, leaving no audit record. Not high only because the stronger gate it should use is + itself header-asserted today (BIO-001). +- **Evidence (read):** + - `Program.cs:773` — `static bool IsAdmin(HttpContext ctx) => ctx.Request.Headers["X-Admin"] == "true";` + - `Program.cs:268-270` — `DELETE /admin/uploads/{documentId}` is gated by `IsAdmin` alone, + not by `Authz.CanManageCases` / the `CasesAdmin` wrapper the four sibling admin surfaces + use (`:801-808`). + - `Data/DocumentStore.cs:165-179` — `AdminDelete` "bypasses ownership", deletes the row and + its bytes, and writes only a `DocumentStore.Audit("delete-admin", …)` metadata row — **no** + `AuthzAuditStore` entry, so the action never appears on `/beheer/audit`. + - `grep -rn "X-Admin"` over `apps`, `libs`, `backend`, `e2e`: the only sender is + `backend/tests/BigRegister.Tests/EndpointTests.cs:231`. **No frontend uses this endpoint.** + It is an orphaned gate, not a live seam. +- **Baseline citation:** **BL-003** (940 lines, 48 endpoint mappings in one file, read/write + separated only by a comment banner — the structural condition under which one endpoint keeps + a superseded gate); §7 Backend CQRS-light row (the local helpers `Submit`, `StamdataAdmin`, + `CasesAdmin`, `Beoordelen`, `OrgAdmin`, `FlagsAdmin` are "authorization/idempotency wrappers" + — `IsAdmin` is the one that never became a wrapper). +- **Remediation, minimal:** route the endpoint through `CasesAdmin` (or a new + `Authz.CanDeleteAnyDocument`) and delete `IsAdmin`; add the `AuditAuthz` call the sibling + gates make. One test (`EndpointTests.cs:231`) changes its header. +- **Effort:** S + +### BIO-004 — two upload endpoints have no authorization check at all + +- **Control:** 9.4 (broken object-level authorization); 5.12 (the objects are diploma and + identity scans) +- **Class:** **defect now** +- **Severity: high** — it is the only place in the backend where AVG-relevant _content_ is + served with no owner and no capability test, and the codebase demonstrably knows the + pattern: the sibling `DELETE` on the same resource is owner-scoped, and submit validates + foreign ids. Mitigating factor, stated honestly: document ids are `Guid.NewGuid()` + (`DocumentStore.cs:54`) and local ids are `crypto.randomUUID()` on the client, so this is a + capability-URL exposure rather than an enumerable one. +- **Evidence (read):** + - `Program.cs:231-237` — `GET /uploads/{documentId}/content` calls + `DocumentStore.Get(documentId)` and streams `doc.Content`. The lambda does not take + `HttpContext`; it cannot check anything. + - `Data/DocumentStore.cs:65-72` — `Get` has no owner parameter. + - Contrast, one screen away: `Program.cs:253-254` `DELETE /uploads/{documentId}` → + `DocumentStore.DeleteOwned(documentId, ctx.Zorgverlener().Bsn)` (`:146-153`, explicit + `d.Owner != owner` check), and `Program.cs:367` → + `DocumentStore.ForeignIds(documentIds, ctx.Zorgverlener().Bsn)` (`:103-113`), whose own + docstring says it "guards submit/draft-sync against a citizen attaching another citizen's + upload to their own aanvraag". + - `Program.cs:242-250` — `GET /uploads/status?localIds=` maps client local ids to document + ids for **any** caller, with no owner filter (`DocumentStore.ByLocalIds`, `:76-84`). + - **Why it is unscoped is legible:** `Program.cs:451-452` (the beoordeling detail) hands a + behandelaar the `DocumentId`s of another citizen's uploads, so a cross-owner read is a + genuine requirement. The defect is that the requirement was met by removing the check + rather than by widening it. +- **Baseline citation:** §7 Backend — `DocumentStore` listed among the 7 stores "Not behind + any port"; §3c `backend/Data` **75.5% branch** against 99.0% line (**BL-005**), the exact + signature of "the unit is entered, the guard branches are not there to enter". +- **Remediation, minimal:** take `HttpContext` in both lambdas and allow when + `doc.Owner == ctx.Caller().SubjectId` **or** `Authz.CanBeoordelen(ctx.Caller())` **or** + `Authz.CanManageCases(Authz.ResolvePrincipal(ctx))`; 404 (not 403) otherwise, per PRD-0002 + §8's "avoid resource-existence enumeration". Same predicate for `/uploads/status`. +- **Effort:** S + +### BIO-005 — `POST /registrations` links arbitrary document ids with no ownership check + +- **Control:** 9.4; 8.26 (unvalidated request field driving a state change) +- **Class:** **defect now** +- **Severity: medium** — integrity/availability, not confidentiality: the caller cannot read + another citizen's document, only permanently mark it `Linked`, which blocks that citizen + from ever deleting it (`DocumentStore.DeleteOwned` returns `Linked`, `Program.cs:257-259`). +- **Evidence (read):** + - `Program.cs:187-190` — `POST /registrations` passes `req.Documents` straight to `Submit`. + - `Program.cs:919-925` (`Submit`) — `DocumentStore.Link(documents.Where(…).Select(d => d.DocumentId!))` + and `DocumentStore.Audit("post-delivery", …)`. Neither is owner-scoped; + `DocumentStore.Link` (`Data/DocumentStore.cs:129-141`) takes no owner at all. + - Contrast `Program.cs:367`, the newer submit path, which rejects foreign ids before linking. + - **The endpoint has no frontend caller.** `grep` over `apps`/`libs` for the generated + client's `registrations` method returns nothing outside `api-client.ts` itself; only + `/change-requests` is still called (`registratie/infrastructure/change-request.adapter.ts:17`). +- **Baseline citation:** **BL-003** (48 endpoint mappings in one 940-line file — the condition + under which a guard added on one submit path is not added to the other); §7 Backend + CQRS-light row, which names `Submit` as one of the cross-cutting wrappers. +- **Remediation, minimal:** either delete the endpoint (it is dead, and WP-72 already removed + its siblings), or add the one `ForeignIds` guard the other submit path uses. Deleting is + smaller and removes the surface entirely. +- **Effort:** S + +### BIO-006 — the PII-reveal capability belongs to the default role, and its step-up is a client-asserted constant + +- **Control:** 9.4 (least privilege); PRD-0002 §5d ("the server re-checks the environment + attribute before permitting") +- **Class:** **production gate** (the role mapping is a documented POC choice) with one + **defect-now** sub-item (the step-up check is a no-op as wired) +- **Severity: medium** +- **Evidence (read):** + - `Domain/Authorization/Authz.cs` — `CanRevealBigNummer(principal) => principal.Role == PrincipalRole.Drafter`. + - `StubIdentityProvider.cs:23-27` — the `_ =>` arm of the role switch is `Drafter`. So the + **absence** of any role header yields the role that holds the PII reveal. `roles-and-access.md` + states this as "`drafter` … the only role that may reveal a BSN" without noting that it is + also the default. + - `Program.cs:668-681` — the reveal requires `canReveal && ctx.Request.Headers["X-Step-Up"] == "true"`. + - `apps/ssp/src/app/brief/infrastructure/reveal-bignummer.adapter.ts:26` — the client sends + `'X-Step-Up': 'true'` **unconditionally, as a literal**. The precondition is therefore + satisfied by every call that reaches the endpoint; it constrains nothing. The endpoint's + own comment at `Program.cs:665` calls it "stubbed here as the X-Step-Up header", which is + accurate, but the FE side turns the stub into a constant rather than into a gesture-gated + value. +- **Baseline citation:** §7 Backend — `Domain/.../Authz.cs` listed among the pure rule classes + with co-located tests; §3b `ssp/brief` **42% spec reach** (11 of 26 files), with + `reveal-bignummer.adapter.ts` among the 15 unreached and **not** a `ui/` file, so BL-004's + Storybook carve-out does not cover it. +- **Remediation, minimal:** (a) send `X-Step-Up` only from the confirm handler, not from the + adapter's literal — one line, and it makes the stub behave like the control it stands in + for; (b) record in `roles-and-access.md` that `drafter` is the default role, so the + least-privilege consequence is visible; (c) production gate: bind the reveal to an app-overlay + attribute rather than the coarse role, as PRD-0002 §5c already says ("Role-based in the POC; + a real system resolves it from the app overlay independent of role"). +- **Effort:** S for (a)+(b); the overlay is a checklist item. + +### BIO-013 — the seeded-citizen endpoints ignore the caller entirely + +- **Control:** 9.4 (row-level scoping); 5.12 +- **Class:** **production gate** — explicitly acknowledged as unbuilt in PRD-0002 §9 P2 + ("Row-level scoping (§5b) still unbuilt") +- **Severity: medium** — with one seeded citizen it is invisible; the moment a second identity + exists (which `?subject=` already creates in e2e) every citizen reads the seeded citizen's + BRP address, birthdate and registration. +- **Evidence (read):** `Program.cs:135-155` — `GET /dashboard-view`, `GET /notes`, + `GET /brp/address`, `GET /duo/diplomas` take no `HttpContext` and return `SeedData.Registration` + / `SeedData.Person` / `SeedData.BrpAddress` regardless of the resolved caller. The + identity middleware runs (`:114-119`) and its result is discarded. +- **Baseline citation:** §7 Backend — the read side is "screen-shaped reads. Decisions are + computed here"; **BL-003** (all 48 mappings in one file). §3c `Program.cs` 97.4% line — + these endpoints are covered, so the gap is by design, not by omission. +- **Remediation, minimal:** none proposed for the POC. The checklist item is: every read that + returns person data must take the caller and scope on it, and the acceptance test is a + second seeded citizen who cannot see the first's data. +- **Effort:** M (out of this backlog) + +### BIO-018 — `IdempotencyStore` is keyed on a client-supplied string with no caller scoping, TTL or bound + +- **Control:** 9.4; 8.26 +- **Class:** **defect now** +- **Severity: low** — the cached values are only a `ReferentieResponse` or a ProblemDetails, so + a cross-caller replay leaks a reference number, not personal data. Filed because it is an + unscoped shared cache in an access-control path and the fix is trivial. +- **Evidence (read):** `Data/IdempotencyStore.cs:11-27` — a process-global + `Dictionary` keyed on the header alone; the file's own `ponytail:` comment + concedes "no TTL/eviction … an unbounded dictionary keyed on client-supplied strings is a + memory leak at scale". `Program.cs:901-909` reads and writes it with the raw header value, + never composed with the caller's `SubjectId`. +- **Baseline citation:** §7 Backend — `IdempotencyStore` listed among the 7 stores "Not behind + any port"; agent 02's `backend/Data` note ("the only store that is purely in-memory with no + `Reset()` and no TTL … shared by every test class in the process"). +- **Remediation, minimal:** key on `$"{ctx.Caller().SubjectId}:{idemKey}"`. One line, and it + also removes the cross-test-class bleed agent 02 flagged. +- **Effort:** S + +--- + +## 2. Control area: logging & monitoring (8.15, 8.16) + +### BIO-007 — only _denied_ authorization decisions are audited; successful admin and approval actions are not + +- **Control:** 8.15 (logging), 8.16 (monitoring activities); PRD-0002 §8 ("Audit log of + authorization-relevant events — **denials, PII reveals, approvals/rejections**, step-up, + break-glass") +- **Class:** **defect now** +- **Severity: medium** — the queryable trail the product ships as its audit surface + (`/beheer/audit`) cannot answer "who changed this", only "who was turned away". For a + register whose integrity is the product, that is the wrong half. +- **Evidence (read) — every `AuditAuthz` call site, checked:** + - `Program.cs:783` `OrgAdmin` → `allowed: false`. `:794` `StamdataAdmin` → `false`. + `:805` `CasesAdmin` → `false`. `:817` `Beoordelen` → `false`. `:827` `FlagsAdmin` → `false`. + All five gates audit **only** the denial branch; the allow branch calls `action()` and + returns. + - The one exception is `Program.cs:674` (the reveal), which passes the real `allowed`. + `:539` audits the NRC notification. `:856` audits a ZGW divergence. + - **Therefore the following leave no row in `AuthzAuditStore`:** `PUT /admin/flags/{key}` + (`:592`, and `Data/FeatureFlagStore.cs:54-66` writes nothing either — the endpoint does + not even emit a log line), `PUT /admin/org-template/{subOrgId}` (`:739`), + `POST /admin/org-template/{subOrgId}/rollback/{version}` (`:764`), + `DELETE /admin/cases/{id}` (`:554`, log line only at `:557`), + `DELETE /admin/uploads/{documentId}` (`:268`, see BIO-003), + `POST /brief/approve` / `/reject` / `/send` (`:631-658`, `LogBrief` writes a log line with + no actor), and `POST /beoordeling/{id}/besluit` (`:473`). + - The comment at `Program.cs:777-778` states the intent — "the allow path is left un-logged + (the endpoints log their own effect, e.g. publish)" — and the intent is only half met: + publish (`:755`) and admin case delete (`:557`) log; the other six do not log at all. +- **Baseline citation:** §7 Backend — `AuthzAuditStore` listed among the 7 static stores; + §3c `backend/Program.cs` 97.4% line / **84.8% branch** — the allow branches run constantly + and simply have nothing in them. +- **Remediation, minimal:** move the `AuditAuthz` call from each gate's deny branch to the + gate itself, passing the real boolean and wrapping `action()`: + `var ok = Authz.CanX(p); AuditAuthz(ctx, "x", resource, ok, p); return ok ? action() : Forbidden(...)`. + Five identical edits in `Program.cs:779-830`, no signature change, no new concept. Add + `AuditAuthz` to the three brief transitions and the besluit. +- **Effort:** S for the five gates; M including the brief/besluit sites and their tests. + +### BIO-008 — the BSN is written into the authz audit trail's `Resource` column + +- **Control:** 8.15; 5.12 (classification of special-category data) +- **Class:** **defect now** +- **Severity: high** — this is the single clearest "wrong even for a POC" item in the file: + a national identifier is persisted to a store whose own type documentation, class + documentation and endpoint documentation all state it holds none, and it is then served over + an API and rendered in a UI that repeat the same claim. +- **Evidence (read), the whole chain:** + 1. `Program.cs:674` — + `AuditAuthz(ctx, "brief:reveal-bignummer", "brief/" + ctx.Zorgverlener().Bsn, allowed, principal);` + The BSN is concatenated into the `resource` argument. + 2. `Program.cs:840-844` — that argument is logged + (`"authz action={Action} resource={Resource} …"`) **and** passed to + `AuthzAuditStore.Record(action, resource, …)`. + 3. `Data/AuthzAuditStore.cs:30-38` — `Record` inserts it as `AuthzAuditEntry.Resource` into + SQLite. The entity's doc comment (`:5-8`) says "**never** a name, BSN, or the value that + was (or wasn't) revealed"; the class comment (`:22-23`) says "Holds NO PII by construction + (see the entity); the schema test asserts it". + 4. `Program.cs:566-569` — `GET /admin/audit` returns `a.Resource` verbatim in `AuthzAuditDto`. + 5. `libs/beheer/src/ui/audit.page.ts:10-12` — the page's own header comment reads + "data-minimised, no PII", and its table renders the resource column. + 6. `Program.cs:666-667` — the endpoint comment: "every attempt — allow or deny — is audited + with **NO PII** (AuditAuthz)". +- **The false assurance, named:** `backend/tests/BigRegister.Tests/AuthzAuditTests.cs:51-53` + asserts over **column names** — + `Assert.DoesNotContain(names, n => Regex.IsMatch(n, "naam|name|bsn|value|waarde", …))` — + not over values. The BSN travels in a column called `Resource`, which the regex cannot see. + Four documents claim the control; the test that is cited as enforcing it does not enforce it. +- **Baseline citation:** §7 Backend — the 7 static stores, `AuthzAuditStore` among them; + §3c `backend/Data` **75.5% branch** (**BL-005**). +- **Remediation, minimal:** the resource ref for a per-owner brief does not need the BSN — use + the brief's own id, or `"brief/" + MaskTail(bsn, 3)` (the masker already exists at + `Program.cs:861-863` and is already used for the BIG-nummer at `:878`). Then extend + `AuthzAuditTests` to assert on **values**: seed a reveal attempt and assert no stored + `Resource` matches `\d{9}`. +- **Effort:** S + +### BIO-009 — the BSN is the `Actor` on every document audit row + +- **Control:** 8.15; 5.12 +- **Class:** **defect now** +- **Severity: medium** — same class as BIO-008 but a narrower blast radius: this table is not + exposed by any endpoint (`grep`: `DocumentStore.AuditLog` has no caller in `Program.cs`), so + it is a storage-side leak only. +- **Evidence (read):** + - `Data/DocumentStore.cs:52-63` — `Add(..., string owner)` ends with + `Audit("upload", doc.DocumentId, categoryId, owner)`, and `owner` is the caller's BSN + (`Program.cs:224` passes `ctx.Zorgverlener()`, whose `Bsn` becomes `StoredDocument.Owner`). + - `:159` — `DeleteOwned` likewise: `Audit("delete-user", documentId, categoryId, owner)`. + - `:181-189` — `Audit` persists it as `AuditEntry.Actor`. + - The class doc comment (`:30-36`) states "The audit log holds metadata only (never file + content **or other PII**)." + - `StoredDocument.Owner` itself is the BSN by design (it is the ownership key) — that is + correct and is **not** the finding; the finding is the _audit_ row, which needs only a + pseudonymous actor. +- **Baseline citation:** §7 Backend — `DocumentStore` among the 7 static stores; §3c + `backend/Data` 99.0% line / 75.5% branch. +- **Remediation, minimal:** pass `MaskTail(owner, 3)` (or a per-session pseudonym) as the + `actor` argument at `:61` and `:159`; the ownership column is untouched. +- **Effort:** S + +### BIO-010 — the BSN reaches logs and a persisted aanvraag field through the ZGW error path + +- **Control:** 8.15; 5.12 +- **Class:** **defect now**, conditional on `Zgw:Enabled=true` (off by default, + `appsettings.json`) — which is why the severity is medium and not high today, and why it + becomes high the moment OpenZaak is switched on. +- **Severity: medium** +- **Evidence (read):** + - `Zgw/OpenZaakZaakSource.cs:52` — the citizen-scoped list builds + `url += $"?rol__betrokkeneIdentificatie__natuurlijkPersoon__inpBsn={Uri.EscapeDataString(bsn)}"`. + The BSN is in the request URI. + - `Zgw/ZgwHttpClient.cs:76-81` — on a non-transient failure the client throws + `new HttpRequestException($"ZGW {req.Method} {req.RequestUri} failed: {(int)res.StatusCode} {snippet}")`, + i.e. the exception message carries **both** the BSN-bearing URI **and** up to 500 characters + of the OpenZaak response body. `Zgw/OpenZaakZaakSource.cs:246` shows the write side posts + `inpBsn` in the body, and `Data/DocumentStore.cs:44-47` records that OpenZaak's own + validation errors for that field are a real, observed failure mode — so the body snippet + is a live BSN-echo path, not a hypothetical one. + - `Program.cs:851-856` (`RecordZgwDivergence`) — `app.Logger.LogError(ex, …)` logs that + message, **and** `ApplicationStore.SetZgwError(id, ex.Message)` persists it to the aanvraag + row (`Data/ApplicationStore.cs:286-294`). `Data/AanvraagMapper.cs:25,50,63,78,96` carry it + onward through the mapper. + - Mitigating, verified: `ZgwError` is **not** on any DTO in `Contracts/` and does not appear + anywhere in the frontend — the value is stored and logged, not served. + - Also verified and **not** a finding: `Zgw/ZgwDiagnosticHandler.cs:17-24` logs only method, + URI and byte counts, and only when `request.Content is not null` — so the BSN-bearing GET + URI is never reached by it, and no body is logged. It is opt-in behind `ZGW_DEBUG_HTTP=1` + (`Program.cs:73-78`). That hatch is clean. +- **Baseline citation:** §7 Backend — "ZGW anti-corruption layer | Fully built"; §3c + `backend/Zgw` 98.1% line / **85.5% branch — the strongest branch figure on the backend**, + so this is a design gap, not a test gap. +- **Remediation, minimal:** in `ZgwHttpClient.SendWithRetryAsync`, build the message from + `req.RequestUri.GetLeftPart(UriPartial.Path)` (drop the query) and omit the body snippet + from the _message_, logging it separately at Debug if the diagnostic value is wanted. +- **Effort:** S + +--- + +## 3. Control area: data classification & handling (5.12, 5.13) + +### BIO-011 — cross-owner list endpoints ship unmasked BSNs while the detail endpoint masks + +- **Control:** 5.12 (classification), 5.13 (labelling/handling); PRD-0002 §5c ("Default DTO + carries a **masked** BSN … or omits it entirely") +- **Class:** **defect now** +- **Severity: medium** — the correct behaviour is implemented one file away, so this is an + inconsistency rather than a missing capability, and the list is the _wider_ exposure (every + open case, not one). +- **Evidence (read):** + - `Contracts/Mappers.cs:72-74` — `ToAdminSummaryDto(now) => a.ToSummaryDto(now) with { Owner = a.Owner }`. + `a.Owner` is the raw BSN. `Contracts/Dtos.cs:107` documents the field as "populated for the + admin cross-owner list (WP-36)". + - `Data/LocalZaakSource.cs:15-16` — `ListCases` maps every row through `ToAdminSummaryDto`. + - `Program.cs:425-427` — `GET /admin/cases` returns `zaken.ListCases(...)` unmodified. + `Program.cs:434-438` — `GET /werkvoorraad` returns the same list, filtered by status only. + - **Contrast, in the same file:** `Program.cs:453` — the beoordeling detail does + `var masked = c with { Owner = MaskTail(c.Owner!, 3) };` before returning. The detail + masks; the list that leads to it does not. + - Rendered unmasked in both UIs: + `apps/behandelportal/src/app/behandeling/domain/werkvoorraad-item-view.ts:28` + (`subtitle: $localize\`:@@werkvoorraad.row.bsn:BSN ${item.owner}\``) and +`apps/ssp/src/app/registratie/ui/admin-cases.page.ts:91`("Eigenaar (BSN)").`behandeling/domain/beoordeling.ts:26`correctly documents its own field as "masked by the +server";`werkvoorraad-item.ts:20` documents its as "always populated" with no masking note. +- **Baseline citation:** §7 Backend — "Mapping | `Contracts/Mappers.cs` (`.ToDto()`, + `.ToDetailDto()`), `Data/AanvraagMapper.cs`" (the exact seam); §3a `bhp/behandeling` + 91.6% line / 81.5% branch — well tested, so nothing here is accidental. +- **Remediation, minimal:** apply `MaskTail(a.Owner, 3)` in `ToAdminSummaryDto` + (`Mappers.cs:74`) — one expression, and both list endpoints inherit it. If an admin genuinely + needs the full value, that is a reveal action with the existing gated+audited shape, not a + default list field. +- **Effort:** S + +### BIO-012 — `?role=` and `?subject=` are **not** stripped from production builds on three hand-written `fetch` paths + +- **Control:** 5.13 (handling — a BSN written to web storage and onto the wire), 9.4 +- **Class:** **defect now** — the defect is that a documented control does not hold, not that + the residual exposure is large. Stated plainly: an attacker who can send a header does not + need this path (see BIO-001), so the incremental attack value is low. The value of the + finding is that the docs and CLAUDE.md assert a production property the code does not have, + and anyone reasoning about production risk from those docs will get it wrong. +- **Severity: medium** +- **Evidence (read):** + - The interceptor chain **is** correctly gated: `apps/ssp/src/app/app.config.ts:58-62` and + `apps/behandelportal/src/app/app.config.ts:57-63` both register + `[scenarioInterceptor, roleInterceptor, subjectInterceptor(, medewerkerInterceptor)]` + only when `isDevMode()`. That much of the claim is true. + - But three adapters bypass `HttpClient` entirely and set the headers themselves, with **no** + `isDevMode()` guard: + - `apps/ssp/src/app/brief/infrastructure/reveal-bignummer.adapter.ts:26` — + `headers: { 'X-Role': currentRole(), 'X-Step-Up': 'true' }` + - `apps/ssp/src/app/brief/infrastructure/letter-preview.adapter.ts:43-46` — + `headers: { 'X-Role': currentRole(), ...(subject ? { 'X-Subject': subject } : {}) }` + - `apps/ssp/src/app/brief/infrastructure/org-template.adapter.ts:79` — + `headers: { 'X-Role': currentRole() }` + - The readers are ungated too: `libs/shared/src/infrastructure/role.ts:23-31` and + `libs/shared/src/infrastructure/subject.ts:23-29` both read the query param and **write it + into `sessionStorage`** on any navigation, in any build. For `?subject=` that value is a + **BSN** — persisted to web storage under `dev-subject`, then sent as `X-Subject` on + `/brief/preview`. `subject.interceptor.ts:14-29` argues at length that the BSN must not + leave `SessionStore` ("`SessionStore`'s G1 comment is explicit that the BSN … is never + persisted or otherwise handed outward, by design") and then routes it through + `sessionStorage` instead. + - The claim being contradicted: `docs/reference/roles-and-access.md:23` — "Both are wired + only under `isDevMode()` — **they do not exist in a production build**"; CLAUDE.md's + "Scenario toggle (**dev-only**, not wired in prod builds)" and "Dev role stand-in + (**dev-only**)". `?scenario=` genuinely is stripped (its only consumer is the gated + interceptor plus `upload.adapter.ts`'s simulator); `?role=` and `?subject=` are not. + - BSN-in-URL has its own consequences independent of the header: browser history, `Referer`, + and any reverse-proxy access log. +- **Baseline citation:** §3b `ssp/brief` **42% spec reach** (11 of 26 files) — all three + hand-written `fetch` adapters are among the 15 unreached, and none is a `ui/` file, so + **BL-004**'s Storybook carve-out does not cover them; §3a `ssp/brief` 68.8% branch. +- **Remediation, minimal:** guard the three adapters — + `...(isDevMode() ? { 'X-Role': currentRole() } : {})` — or, cleaner, have `currentRole()` + and `currentSubject()` return `undefined` outside `isDevMode()` so every caller inherits the + guard and the sessionStorage write disappears with it. Then correct + `docs/reference/roles-and-access.md:23`. +- **Effort:** S + +### BIO-017 — two PII guards have no executable test + +- **Control:** 8.29 (security testing in development); 5.13 +- **Class:** **defect now** +- **Severity: low** — both guards were verified correct by reading them; the finding is that + nothing would catch a regression. +- **Evidence (read):** + - `apps/ssp/src/app/auth/application/session.store.ts` — **G1 holds on every path, + verified:** `restore()` (`:12-21`) returns `{ bsn: '', naam }` and never reads a stored + BSN; the constructor `effect()` (`:43-48`) writes only `{ naam: s.naam }`; `login()` + (`:52-56`) sets the signal in memory only; `logout()` removes the key. There is no path + that writes the BSN to `localStorage`. Identical in + `apps/behandelportal/src/app/auth/application/session.store.ts`. + - But agent 02's **TE-001** is right that the guard is untestable as written: `restore()` + is module-private and reads `localStorage` in a field initializer. Per-file lcov (agent 02, + from §3a): **LH 2 / LF 20 (10.0% line), BRH 3 / BRF 13**. + - `apps/ssp/src/app/shell/debug-state/mask.ts:13-25` — `redactProfile` is a pure, exported, + directly callable PII-redaction function with **no spec** (agent 02's "missing test, not + blocked test"). It redacts name, birthdate and address and masks the BIG-nummer; verified + correct by reading. +- **Baseline citation:** §3a `ssp/auth` · `bhp/auth` **42.9% line / 46.2% branch — jointly the + worst line coverage in the frontend** (§8 ranking); **BL-009** (no coverage threshold is + enforced anywhere, so nothing ratchets this). +- **Remediation, minimal:** take agent 02's TE-001 seam (`parseStoredSession` in + `auth/domain/session.ts`, which already has a spec file) and add the three cases — + absent, non-JSON, wrong shape — plus one that asserts a stored `{"bsn":"…","naam":"…"}` + yields `bsn: ''`. Add a five-line spec for `redactProfile`. +- **Effort:** S (this is TE-001 plus one assertion; it does not need its own ticket if TE-001 + is scheduled — but the BSN assertion must be in TE-001's acceptance criteria, which today + it is not). + +--- + +## 4. Control area: cryptography (8.24) + +### BIO-014 — nothing is encrypted at rest: document bytes, BSNs and the audit trail sit in a plaintext SQLite file + +- **Control:** 8.24 (use of cryptography) +- **Class:** **production gate** +- **Severity: high** — the file contains the highest-classification data in the system in one + place, unprotected. +- **Evidence (read) — what is at rest, and what class of data:** + + | Table | Contains | Class | + | ----------------------------------------- | ------------------------------------------------------------------------------ | --------------------------------------- | + | `Documents` (`DocumentStore.cs:9-20`) | `byte[] Content` — the uploaded diploma / identity / language-proficiency scan | AVG art. 9-adjacent; identity documents | + | `Documents.Owner` | BSN | AVG art. 87 national identifier | + | `Applications.Owner` (`ApplicationStore`) | BSN | same | + | `Applications.ZgwError` | may contain a BSN (BIO-010) | same | + | `AuthzAudit.Resource` | contains a BSN today (BIO-008) | same | + | `AuditEntries.Actor` | BSN (BIO-009) | same | + | `Briefs` / `Briefs.ArchivedHtml` | the rendered letter, incl. name, BIG-nummer, address | personal data | + - `Data/Db.cs:16-18` — `UseSqlite(ConnectionString)`, default + `"Data Source=bigregister.db"`. No key, no `SQLCipher`, no page encryption. + - `DocumentStore.cs:3-8` records the intent — "a real backend persists them to blob storage + keyed by DocumentId" — so the shape is a known stand-in. + - The file lands in the app's working directory and, under `docker-compose.yml`, on the host + through the `./backend:/src:z` bind mount. **Verified: it is gitignored** + (`backend/.gitignore:5`, `git check-ignore` confirms both copies), and `git ls-files` shows + zero tracked `.db` files — so there is no committed-database finding. + - No transparent-data-encryption, no filesystem-level requirement, and no key management + exists anywhere in the repo. + - Also verified and **not** a finding: no secrets are committed. `appsettings.json`'s `Zgw` + block ships every credential field empty, `backend/openzaak/setup_configuration/data.prod.yaml` + and `seeded.env` are both gitignored (`.gitignore:57`, `:60`), and + `ZgwOptions.Secret`'s doc comment correctly states it is "Held only by the BFF, never the + browser". `ZgwTokenProvider.cs` mints a short-lived HS256 JWT per call with no refresh + token to store — sound for what it is. + +- **Baseline citation:** §2 size inventory — `backend/Data` (excl. Migrations) 1 697 lines, + the largest backend folder after `Program.cs`; §7 Backend — the 7 static stores, "each opens + a short-lived context via `Db.Create()` under its own lock". +- **Remediation, minimal:** none for the POC. Checklist items: move document bytes out of the + relational store to encrypted object storage keyed by `DocumentId` (the code already says + this is the target); require encryption at rest for the database (managed-service TDE or + SQLCipher); define key custody and rotation. **Prerequisite:** BIO-008/009/010 first, so the + BSN is not in three places that do not need it before deciding what must be encrypted. +- **Effort:** L (infrastructure), but S to write the requirement down and to stop widening it. + +### BIO-015 — no transport security, no security response headers, Swagger and `AllowedHosts: *` unconditional + +- **Control:** 8.24 (cryptography in transit), 8.28 (secure coding — attack surface) +- **Class:** **production gate** +- **Severity: medium** +- **Evidence (read):** `Program.cs:86-131` is the whole pipeline. It contains no + `UseHttpsRedirection`, no `UseHsts`, no `UseAuthentication`/`UseAuthorization`, and no + response-header middleware — so no `Strict-Transport-Security`, no + `X-Content-Type-Options: nosniff`, no `Content-Security-Policy`, no `Referrer-Policy`. + `app.UseSwagger(); app.UseSwaggerUI();` (`:121-122`) run in every environment, with no + `app.Environment.IsDevelopment()` guard. `appsettings.json` sets `"AllowedHosts": "*"`. + CORS (`:37-39`) allows only `http://localhost:4200` — which is tight, and worth noting is + effectively unused since both the dev servers and `docker-compose.yml` proxy `/api` + same-origin (`API_PROXY_TARGET`); the behandelportal's `:4201` is not in the list and does + not need to be. + - **`nosniff` in context:** the upload allow-list is `application/pdf`, `image/jpeg`, + `image/png` only (`Domain/Documents/DocumentCategory.cs:20-22`, enforced at + `Program.cs:216-217`), and `GET /uploads/{id}/content` serves `inline` only for pdf and + `image/*` (`:235`). No script-capable type (SVG, HTML) can be uploaded, so the usual + stored-XSS-via-inline-attachment path is **closed by the allow-list**. The content type is + nevertheless client-declared rather than sniffed from magic bytes, which is why `nosniff` + belongs on the checklist rather than being a finding today. +- **Baseline citation:** **BL-003** (`Program.cs` 940 lines, file CC 78 — the one place every + pipeline decision lives); §3c `Program.cs` 97.4% line / 84.8% branch. +- **Remediation, minimal:** wrap Swagger in `if (app.Environment.IsDevelopment())` — one line, + and it is a genuine attack-surface reduction with no POC cost. The rest are checklist items, + most of which belong to the reverse proxy rather than the app. +- **Effort:** S for Swagger; the rest is deployment configuration. + +--- + +## 5. Control area: secure development (8.25, 8.28, 8.29) + +### BIO-016 — what the security gates cover, and what they do not + +- **Control:** 8.25 (secure development lifecycle), 8.28 (secure coding), 8.29 (security + testing in development and acceptance) +- **Class:** **production gate** +- **Severity: medium** +- **Evidence (read) — `.github/workflows/ci.yml`, in full:** + + **Present, and genuinely blocking:** + - `semgrep scan --config p/default --config p/csharp --metrics=off --error` (`:254-279`) — + SAST on both sides, `--error` makes it a gate, telemetry off, prior findings triaged + rather than suppressed wholesale. + - `npm audit --omit=dev` (`:128`) — the shipped bundle must audit clean. + - All actions pinned to full SHAs (`:32-33`, `:64`, `:70`, …) — supply-chain hygiene. + - `permissions: contents: read` at workflow level (`:9-11`), plus per-ref concurrency cancel. + - `npm run dep:check` (`:109`) — 11 `severity: error` architecture rules, **frontend only**. + - `dotnet format --verify-no-changes` + `dotnet test --filter "Category!=Integration"` + (`:199-204`) — 241 backend tests. + - `api-client-drift` (`:281-317`) — the wire contract cannot drift unnoticed. + + **Absent:** + - **No dependency vulnerability scan on the backend.** There is no + `dotnet list package --vulnerable --include-transitive` step; `npm audit` covers only the + frontend. The .NET dependency tree is unscanned. + - **No secret scanning** (gitleaks/trufflehog). Today nothing is committed (verified in + BIO-014), so this is prevention, not repair. + - **No authorization regression suite as a gate.** `AuthzTests.cs`, `AuthzAuditTests.cs`, + `WerkvoorraadTests.cs` and `StubIdentityProviderTests.cs` exist and run inside + `dotnet test`, but nothing asserts the _set_ of gated endpoints — so an endpoint added + without a gate (BIO-004's shape) fails no test. + - **No backend architecture enforcement at all — BL-006.** `Domain/` purity holds by + convention. The property ADR-0005 depends on ("ZGW shapes never leave `Zgw/`") and the + property this file depends on ("authorization lives in `Authz`") are both review-maintained. + - **No coverage ratchet — BL-009.** Nothing can regress-test a security fix by CI number. + - No DAST, no container image scan, no SBOM. Reasonable omissions for a POC; listed so the + production decision is explicit. + +- **Baseline citation:** **BL-006** (verbatim: "the backend has zero automated architecture + enforcement … `Domain/` purity currently holds by convention") and **BL-009** (no coverage + threshold is enforced anywhere). +- **Remediation, minimal:** two cheap additions with real value here — + (a) `dotnet list package --vulnerable --include-transitive` as a failing step in the + `backend` job; (b) one endpoint-inventory test that enumerates the app's route table and + asserts every route outside a small allow-list passes through one of the six authorization + wrappers. (b) is the test that would have caught BIO-003, BIO-004 and BIO-005. +- **Effort:** S for (a), M for (b). + +--- + +## 6. Control area: change control (8.32) + +### BIO-020 — the only deployment artifact in the repo builds development bundles + +- **Control:** 8.32 (change management); 8.25 +- **Class:** **production gate** +- **Severity: medium** — there is no release path, so there is no gate at which any of the + production-gate items in this file would be checked. +- **Evidence (read):** `docker-compose.yml` — the `api` service sets + `ASPNETCORE_ENVIRONMENT=Development` and runs `dotnet run`; both `web` and + `web-behandelportal` run `npx ng build … --configuration development --localize`, and the + file's own header comment states why: "development config keeps `isDevMode()=true` so the + dev tools render". The image is `mcr.microsoft.com/dotnet/sdk:10.0` / `node:24-slim`, and + the header opens "dev-server images (not multi-stage prod builds) — this is a demo". + **Consequence for every dev hatch in this file:** in the one containerised deployment the + repo ships, `isDevMode()` is `true`, so `roleInterceptor`, `subjectInterceptor`, + `medewerkerInterceptor`, `scenarioInterceptor` and the `⚙ state` debug panel are all live. + That is correct for a demo and must not be mistaken for a production deployment. +- **Baseline citation:** §2 size inventory (two apps + one backend, all deployed by this one + file); **BL-006** (no automated enforcement that would distinguish the two). +- **Remediation, minimal:** none for the POC. Checklist: a separate production compose/Helm + artifact with `--configuration production`, `ASPNETCORE_ENVIRONMENT=Production`, and a + release checklist that names this file's production gates. +- **Effort:** M (out of this backlog) + +**Positive finding, recorded rather than ticketed.** Change control over the _business rules_ +is genuinely strong and is the model the rest should follow: stamdata is config-as-code +(ADR-0004), validated at build by `StamdataValidationTests`, with **no runtime write +endpoint at all** — verified: `Program.cs:164` and `:173` are both GETs behind +`StamdataAdmin`, and `libs/beheer`'s editor downloads a JSON file for a reviewed PR +(`libs/beheer/src/infrastructure/stamdata.adapter.ts:16-20`, "There is no write method"). A +bad reference-data edit fails CI, never production. The two sanctioned runtime-editable +surfaces (`OrgTemplateStore`, `FeatureFlagStore`) both keep their catalog in code and fail +closed on an unknown key (`Data/FeatureFlagStore.cs:56`) — but see BIO-007: their **writes are +not audited**, which fails clause (4) of the four-part test agent 06's ADR-C-009 proposes. + +--- + +## 7. Control area: input validation (8.26) + +**The server is the authority, and it is.** Verified against ADR-0001's rule ("the FE renders +decisions, it does not recompute business rules"): `SubmissionRules.RejectPhoneChange` +(`Domain/Submissions/SubmissionRules.cs:36-42`) re-validates the phone number server-side +with the same normalisation the FE's `parseTelefoonnummer` applies, and says so; +`DocumentRules.RejectUpload` (`Domain/Documents/DocumentCategory.cs:82-90`) authoritatively +enforces the content-type allow-list and the size cap before any byte is stored; +`Program.cs:208-217` validates multipart shape and required fields before reading the file; +`Program.cs:367` validates document ownership on submit. `IntakePolicy.ScholingThreshold` is +shipped to the FE as a _value_ for instant feedback and re-validated server-side — the +config-value shape ADR-0001 prescribes. The 30 FE `parse*` boundaries (baseline §7) are +defence in depth on the response direction, not a substitute; CLAUDE.md explicitly puts +runtime DTO validation on every endpoint out of scope, and this pass does not reopen that. + +Two gaps, both narrow: + +### BIO-019 — `GET /stamdata/{table}?peildatum=` 500s on unparseable input + +- **Control:** 8.26 +- **Class:** **defect now** +- **Severity: low** — admin-gated, and the failure is a 500 rather than a leak. Filed because + an unhandled exception on a user-supplied string is exactly what 8.26 exists to prevent, and + in a Development environment the exception detail is returned to the caller. +- **Evidence (read):** `Program.cs:178` — + `var rows = peildatum is { Length: > 0 } p ? t.RowsOn(DateOnly.Parse(p)) : t.Rows();`. + `DateOnly.Parse` throws `FormatException` on anything unparseable; there is no + `TryParse`, no 400 path, and `.Produces` on the endpoint declares only 200/403/404. +- **Baseline citation:** §3c `backend/Stamdata` 96.8% line but **71.7% branch** — named in + **BL-005** as the second-weakest branch axis; this is one of the unentered branches. +- **Remediation, minimal:** `DateOnly.TryParse(p, out var d) ? … : Results.Problem(statusCode: 400)`. +- **Effort:** S + +### Noted, not filed + +`GET /uploads/status?localIds=` splits an unbounded comma-separated list into a single +`WHERE IN` query (`Program.cs:242-248`, `DocumentStore.ByLocalIds`). No cap. Real but +negligible at POC scale; it belongs with BIO-018's "bound the client-supplied inputs" theme +rather than as a ticket of its own. + +--- + +# Module sections + +Every module in scope gets a section. "No findings" is a result, not an omission. + +## apps/ssp — auth + +**BIO-017** (the G1 guard has no executable test). **The guard itself holds on every path** — +`restore()`, the persistence `effect()`, `login()` and `logout()` were each read and none +writes the BSN to storage. No other findings: `DigidAdapter` is the faked login CLAUDE.md +places out of scope, and `Session.bsn` never leaves the in-memory signal (verified: the one +place that needed it, `subject.interceptor.ts`, deliberately does **not** reach into +`SessionStore` — see BIO-012 for what it does instead). + +## apps/ssp — registratie + +**No findings of its own.** It is the consumer side of **BIO-011**: +`ui/admin-cases.page.ts:91` renders the unmasked BSN column the backend ships; the fix is +server-side. `application/draft-sync.ts` and the five value objects +(`domain/value-objects/`) do format validation only and never act as authority — correct per +ADR-0001. `parseBsn` (`libs/shared/src/kernel/bsn.ts`) is checksum-only and says so. + +## apps/ssp — herregistratie + +**No findings.** The scholing threshold arrives from the server as a config value +(`domain/intake.machine.ts:52`) with the offline fallback ADR-0001 sanctions; nothing in this +context touches PII, authorization or the network directly. + +## apps/ssp — brief + +**BIO-012** (the three hand-written `fetch` adapters carry `?role=`/`?subject=` into +production builds) and **BIO-006** (the client sends `X-Step-Up: 'true'` as a literal). The +context is otherwise the best-behaved consumer of the decision-DTO pattern in the repo: +`application/brief.store.ts:31,103-108` derives every gate from `BriefState.loaded.decisions` +and states "this store never computes them itself" — verified true. + +## apps/ssp — showcase, shell, root + +**BIO-017** (`shell/debug-state/mask.ts::redactProfile` has no spec; verified correct by +reading). **The dev panel is genuinely gated and is not a finding** — +`libs/shared/src/layout/shell/shell.component.ts:73,79` renders it only under +`@if (isDev && debugPanel)` with `isDev = isDevMode()`, and +`shell/debug-state/debug-state.component.ts:176` masks the BSN even there. The `showcase` +context is a teaching page and reads every context by sanction; it introduces no data path. + +## apps/behandelportal — auth + +**BIO-002** — this is where it lands. `app.config.ts:57-63` is the gate; +`auth/infrastructure/medewerker.ts:14` is the fixed stand-in id. Also relevant and already +owned by agent 06: `auth/ui/login.page.ts:31` logs a backoffice user in through DigiD with a +BSN (ADR-C-004). + +## apps/behandelportal — behandeling + +**No findings of its own.** Consumer side of **BIO-011**: +`domain/werkvoorraad-item-view.ts:28` renders the unmasked BSN for every queue row while +`domain/beoordeling.ts:26` correctly documents its own field as server-masked. Positively: +`infrastructure/beoordeling.adapter.ts:87-97` rejects the payload at the parse boundary if +`decisions.canBesluiten` is absent — deny-by-default at the wire, which is the right reflex. + +## apps/behandelportal — shell, root + +**BIO-002** (the `app.config.ts` interceptor gate). No other findings — routing, providers and +nav config only. + +## libs/shared — domain + +**No findings.** `capability.ts`, `role.ts`, `feature-flag.ts` — 30 lines of type declarations +with no executable statement (agent 02's verified correction to BL-004). The capability names +are enforced at runtime by `parseMe`, which is exported and spec'd. + +## libs/shared — application + +**No findings.** Both access-control primitives were read and are correct: +`access.store.ts:34-37` is deny-by-default (`rd.tag === 'Success' && rd.value.includes(cap)`), +and `whenReady()` (`:50-53`) exists specifically so the guard cannot read `can()` mid-load and +deny an entitled user. `auth.guard.ts:22-47` (already moved here per agent 06's ADR-C-006) +reads only `SESSION_PORT` and `AccessStore` and documents itself accurately as "the UX +pre-gate. The backend re-enforces regardless (403)". **That claim was verified endpoint by +endpoint for the admin surfaces and holds:** `/beheer/stamdata` → `StamdataAdmin` +(`Program.cs:164,173`), `/beheer/zaken` → `CasesAdmin` (`:425,554`), `/beheer/audit` → +`CasesAdmin` (`:566`), `/beheer/functies` → `FlagsAdmin` (`:592`), `/brief/huisstijl` → +`OrgAdmin` (`:726,732,739,751,764,703`), `/aanvraag/:id` → `Beoordelen` (`:446,473`). Every +capability the guard checks has a server-side twin. The exceptions are in `Program.cs`, not +here: BIO-003 (a gate outside `Authz`), BIO-004 and BIO-005 (endpoints with no gate at all) — +and none of those has a `capabilityGuard` claiming to front it. + +## libs/shared — infrastructure + +**BIO-012** — `role.ts:23-31` and `subject.ts:23-29` are the ungated readers, and +`subject.ts` is where a BSN reaches `sessionStorage`. Otherwise clean: +`api-client.provider.ts:58` attaches the Idempotency-Key only when `method !== 'GET'`, and +`:66`'s `retry({count: 2})` is GET-only. + +## libs/shared — ui + +**No findings.** No network, no storage, no PII decision. The `masked-value` atom renders what +it is given; the masking itself lives in `kernel/pii.ts`. + +## libs/shared — layout + +**No findings.** `shell.component.ts:73,79` is the dev-panel gate and it is correct +(see ssp/shell above). + +## libs/shared — kernel + +**No findings, and this is the reference standard.** `bsn.ts` is a "parse, don't validate" +value object whose doc comment correctly classifies a BSN as "art. 9 GDPR/AVG +special-category data" and correctly scopes itself to format+elfproef ("identity is still +faked in this POC"). `pii.ts`'s `maskTail`/`maskBsn` are pure, and the backend keeps a +verified twin (`Program.cs:861-863`) so wire redaction and UI redaction agree. §3a: 96.4% +line / 90.0% branch, §3b **100% reach** — the best-covered module in the repo. + +## libs/shared — upload + +**No findings in this remit**, but two dependencies to record: the document bytes this module +moves are the objects **BIO-004** serves without a check and **BIO-014** stores unencrypted. +`upload.adapter.ts:216-217`'s server-side counterpart enforces the content-type allow-list +authoritatively, so the FE's accept filter is UX only — the correct division. Agent 02's +TE-005 (`uploadOutcome`) is an 8.26-relevant boundary; see the compliance-review section. + +## libs/shared — testing + +**No findings.** Kept out of production by the `no-testing-in-production` dependency-cruiser +rule (§6, 0 violations). + +## libs/shared — environments + +**No findings.** `apiBaseUrl` only; no credentials, no keys. + +## libs/beheer + +**No findings of its own.** It is the display surface for **BIO-008**: `ui/audit.page.ts:10-12` +describes the trail as "data-minimised, no PII" and renders the `resource` column that today +contains a BSN. `ui/feature-flags.page.ts:93` is the caller of the unaudited admin write in +**BIO-007**. `infrastructure/stamdata.adapter.ts:16-20` is genuinely read-only, which is the +positive ADR-0004 note in §6. + +## backend/Program.cs + +**BIO-003, BIO-004, BIO-005, BIO-006, BIO-007, BIO-013, BIO-015, BIO-019.** Eight of the +twenty findings land in one 940-line file, which is itself the observation: **BL-003** records +it as "the single largest complexity concentration in the repo", and the six authorization +wrappers (`OrgAdmin`, `StamdataAdmin`, `CasesAdmin`, `Beoordelen`, `FlagsAdmin`, plus the +orphaned `IsAdmin`) sit 500 lines below the endpoints they gate, with nothing that enumerates +which endpoint uses which. That is the structural condition under which BIO-003/004/005 exist, +and it is why BIO-016's proposed route-table test is worth more than three individual fixes. + +## backend/Domain + +**BIO-001, BIO-002, BIO-006** — all three live in `Domain/Authorization/`. The rest of the +folder is clean and its design is right: `Authz` is a single source of truth where it is used, +the same function both emits the decision flag and gates the mutation +(`Authz.Decisions` ↔ `Authz.CanActOn` ↔ `BriefStore.Review`), and the four-eyes rule +(`CanActOn(Approve, …) => principal.Role == Approver && ActingId(principal) != drafterId`) is +a real segregation-of-duty control, correctly ordered Forbidden-before-Conflict. +`CanBeoordelen(CallerIdentity)` is deliberately caller-kind-derived rather than role-derived +and is the one capability that a forged `X-Role` cannot reach — the right instinct, and the +reason BIO-002 fails closed in that direction. `Domain/` is EF-free and ASP-free (§7), though +nothing enforces it (**BL-006**). + +## backend/Data + +**BIO-008, BIO-009, BIO-014, BIO-018.** All four are storage-side data-handling items rather +than logic defects; the store shape itself (static, `Db.Create()`-per-call, documented in +`Data/Db.cs:6-12`) is out of scope here and is deliberately not challenged — agent 02 reaches +the same conclusion from the testability side. + +## backend/Zgw + +**BIO-010** only. Otherwise the strongest module in the backend for this pass: the ACL is +complete (ADR-0005, fully conformed per agent 06), the client secret is BFF-only and never +reaches the browser, tokens are short-lived and minted per call with no stored refresh +credential, the inbound notification webhook fails closed on an unconfigured secret +(`ZgwOptions.NotificatieAuthorization`: "Empty (the default) means every notification is +rejected — an unconfigured secret must never mean 'accept anything'"), and the diagnostic +handler logs no bodies and is opt-in. §3c: 98.1% line / **85.5% branch, the best on the +backend**. + +## backend/Contracts + +**BIO-011** — `Mappers.cs:74` is the single line that decides whether a cross-owner list ships +a raw BSN, so the fix is one expression here rather than at each endpoint. No other findings: +`Dtos.cs` contains no `Bsn` field by name (verified by grep) — the BSN travels only as +`Owner`, which is exactly the field BIO-011 addresses. + +## backend/Stamdata + +**BIO-019** only, and it is a low-severity parse gap. The module is otherwise the best +change-control story in the repo — see the positive finding in §6. + +--- + +# Compliance review required + +**This is the mandatory flag. Agent 08 must carry every row into `99-backlog.md`, attached to +the finding, not filed as a separate ticket.** These are findings from agents 02, 04 and 06 +that touch a control area in §0. A flag is not a rejection: each of these should proceed, with +a compliance acceptance criterion added to its definition of done. + +| Their ID | Their title (abbrev.) | Control touched | Why it is flagged, and what the added acceptance criterion must be | +| ------------------- | ---------------------------------------------------------------- | ---------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| **TE-001** | `SessionStore.restore()` reads `localStorage` inline | 5.13, 8.29 | The extracted `parseStoredSession` **is** the G1 PII guard. Ship it with an explicit assertion that a stored `{"bsn":…}` yields `bsn: ''` — see **BIO-017**. Without that assertion the ticket moves the guard without testing it. Lands twice (both apps). | +| **TE-002** | Trust boundary hidden in a global-`fetch` method | 5.12, 8.26, 8.15 | The boundary guards a **PII reveal**. `reveal-bignummer.adapter.ts` is also the site of **BIO-012** (`X-Role` ungated) and **BIO-006** (`X-Step-Up: 'true'` literal). Fix all three in one touch of the file, or the next reviewer will re-open it. | +| **TE-003** | `UploadTransport` port declared, concrete class injected | 5.12, 8.26 | The transport carries identity-document bytes. Introducing `UPLOAD_TRANSPORT` must not make it possible to substitute a transport in a production build — token factory only, no route/query override. | +| **TE-004** | `createUploadController` injects + binds at call time | 8.26 | `planFileSelection` is the client-side accept/reject decision. It is **UX only**; the ticket must not let it read as authority — the server allow-list (`DocumentRules.RejectUpload`) stays the authority. State that in the moved function's docstring. | +| **TE-005** | `xhrUpload` interprets responses inside an XHR closure | 8.26 | `uploadOutcome` is a trust boundary on an untrusted response. Also: the extraction must not move `parseError`'s ProblemDetails text into a place where it can be logged with a filename. | +| **TE-006** | Blob-to-browser handoff inlined in 3 commands | 5.13 | The blobs are **rendered letters containing name, BIG-nummer and address**, and the stamdata download is an admin export. `BLOB_PRESENTER` must not gain any persistence or caching; recording fakes belong in specs only. | +| **TE-008** | Brief transition rules live inside store methods | **9.4** | The five guards being extracted include the authorization ones, and `Authz.CanActOn` (the **four-eyes / SoD** rule) sits one line away at `BriefStore.cs:163`. Acceptance: `BriefRules` must **call** `Authz`, never re-implement it, and `tests/Domain/BriefRuleTests.cs` must cover approver == drafter. | +| **CQ-002** | 2 read stores write without the fold; errors dropped | 8.15, 8.16 | One of the two is `AdminCasesStore.delete` — a **destructive admin action** whose failure is currently silent. The error surfacing this ticket adds is the FE half of **BIO-007**'s server-side audit gap. Ship them aware of each other. | +| **CQ-003 / CQ-005** | `runSubmit` (write fold + idempotency mint) used for reads | 9.4, 8.26 | Splitting `runResult`/`runSubmit` narrows who mints an `Idempotency-Key`, which is the key **BIO-018** shows is an unscoped cache key. Sequence CQ-003 before BIO-018 so the scoping change lands on a smaller call set. | +| **CQ-004** | `FeatureFlagStore.set` skips the fold, drops the error | 8.15, 8.32 | A `flags:manage` admin write that fails silently **and** writes no audit row (**BIO-007**). This is the concrete case that fails clause (4) of agent 06's **ADR-C-009** four-part test. Fix the FE error and the BE audit row together. | +| **CQ-006** | Read/write banner split abandoned in 5 of 7 sections | **9.4** | A large-diff, zero-semantic-change reordering across **all 48 endpoint mappings**, including every authorization wrapper call site. Acceptance: `AuthzTests`, `AuthzAuditTests`, `WerkvoorraadTests`, `OrgTemplateEndpointTests`, `StamdataEndpointTests` and `AdminCasesTests` all green, **and** a reviewer confirms each moved endpoint kept its gate. Land it alone, as CQ-006 already says. | +| **CQ-007** | `GET /brief` creates a row | 8.26, 9.4 | A GET with a persisted side effect, auto-retried by the FE. Either fix is acceptable to compliance; the documentation-only alternative is **not** — a non-idempotent GET must be visible in the code, not only in a ticket. | +| **ADR-C-002** | `libs/shared/src/upload/` does network outside `infrastructure/` | 5.12 | Moving the adapter that carries identity documents. Mechanical, but the acceptance criterion (deleting the depcruise carve-out) must not be met by widening a different rule. | +| **ADR-C-004** | The `Principal` union never landed | **9.2, 9.4** | This is the natural home for **BIO-002**. Acceptance must include: the behandelportal's identity works in a **production** build, and `IIdentityProvider` can express "no identity". Landing `Principal` on the FE alone would close the ADR and leave BIO-002 open. | +| **ADR-C-006** | Extract the actor-agnostic route guards to `libs/shared` | **9.4** | Already present in the tree at `libs/shared/src/application/auth.guard.ts` — verify the state before ticketing. Any future change to `authGuard`/`capabilityGuard` is an access-control change and needs the guard spec re-run for both apps. | +| **ADR-C-009** | Generalise the runtime-editable-config exception | **8.32, 9.4** | Agent 06's proposed clause (4) is "writes are admin-capability-gated **and audited**". Today they are gated but **not audited** (**BIO-007**). Either the ADR amendment lands with BIO-007, or the amendment ratifies a control the code does not implement. | + +**Not flagged** (read and judged to touch no control in §0): TE-007, TE-009, CQ-001, +ADR-C-001, ADR-C-003, ADR-C-005, ADR-C-007, ADR-C-008, ADR-C-010, ADR-C-011. + +--- + +# Pre-production compliance checklist + +The **production gate** items, as a checklist. None of these is a defect in the POC; every one +must be true before this system holds real BSNs. Ordered by dependency, not by severity. + +**Identity and access (9.1, 9.2, 9.4)** + +- [ ] **Replace `StubIdentityProvider`** with a provider built from verified DigiD claims + (zorgverlener) and verified employee-SSO/eHerkenning claims (medewerker). `X-Role`, + `X-Subject`, `X-Medewerker`, `X-Rollen` and `X-Admin` are removed as inputs, not merely + ignored. — **BIO-001** +- [ ] **`IIdentityProvider` can express "no identity"**, and an unauthenticated request is + rejected rather than defaulted. No code path may resolve a caller from a constant. — + **BIO-002** +- [ ] **The behandelportal has a non-dev identity path.** Verify by building both apps with + `--configuration production` and confirming the backoffice cannot act as a citizen. — + **BIO-002** (see also ADR-C-004) +- [ ] **A startup assertion** fails the app in Production if the resolved `IIdentityProvider` + is the stub. — **BIO-001** +- [ ] **Row-level scoping** (PRD-0002 §5b) on every read that returns person data. Acceptance: + a second seeded citizen cannot see the first's dashboard, notes, BRP address or diplomas. + — **BIO-013** +- [ ] **The PII-reveal capability comes from the app overlay, not the coarse role**, and is not + held by the default role. — **BIO-006** +- [ ] **Real step-up.** `X-Step-Up` is replaced by a server-verified assurance/recency + attribute; no client may satisfy it with a constant. — **BIO-006** + +**Cryptography (8.24)** + +- [ ] **Encryption at rest** for the database, with documented key custody and rotation. — + **BIO-014** +- [ ] **Document bytes move to encrypted object storage** keyed by `DocumentId` (the code + already names this as the target). — **BIO-014** +- [ ] **TLS everywhere**: `UseHttpsRedirection` + HSTS at the edge, and no plaintext listener. + — **BIO-015** +- [ ] **Security response headers**: `X-Content-Type-Options: nosniff`, CSP, + `Referrer-Policy`, and a real `AllowedHosts`. — **BIO-015** +- [ ] **Swagger and the OpenAPI document are Development-only.** — **BIO-015** + +**Logging, monitoring and retention (8.15, 8.16)** + +- [ ] **Every authorization-relevant event is audited on the allow path too** — admin + mutations, brief approvals/rejections, besluiten, PII reveals. — **BIO-007** +- [ ] **No BSN in any audit row, log line or persisted error field**, enforced by a test that + asserts on **values**, not column names. — **BIO-008, BIO-009, BIO-010** +- [ ] **Audit retention, integrity and access** are defined: how long, append-only, who may + read `/beheer/audit` (today it reuses `cases:manage`, which `Program.cs:565` already + flags as a placeholder for a dedicated `audit:read`). +- [ ] **Log shipping and alerting** — the audit trail is a SQLite table with no export path + today. + +**Data protection (5.12, 5.13)** + +- [ ] **A DPIA** covering BSN, uploaded identity documents and the BIG register, with a + documented lawful basis and retention schedule. Nothing in the repo covers this. +- [ ] **Data minimisation on every list endpoint** — no unmasked BSN as a default field. — + **BIO-011** +- [ ] **Deletion / retention** for uploaded documents and the audit trail. `AdminDelete` + exists; no retention policy does. + +**Secure development (8.25, 8.28, 8.29)** + +- [ ] **Backend dependency vulnerability scanning** in CI. — **BIO-016** +- [ ] **Secret scanning** in CI. — **BIO-016** +- [ ] **An authorization regression gate**: a test that enumerates the route table and asserts + every route passes an authorization wrapper or is on an explicit allow-list. — **BIO-016** +- [ ] **Backend architecture enforcement** (NetArchTest/ArchUnitNET) so `Domain/` purity, the + "ZGW shapes stay in `Zgw/`" property (ADR-0005) and "authorization lives in `Authz`" are + CI-maintained rather than review-maintained. — **BL-006** +- [ ] **A coverage ratchet**, so a security fix can be verified as not regressed by CI. — + **BL-009** +- [ ] **Penetration test / DAST** before go-live, with BIO-004's object-level authorization and + BIO-005's document linking as named test cases. + +**Change control (8.32)** + +- [ ] **A production build and deployment artifact exists** (`--configuration production`, + `ASPNETCORE_ENVIRONMENT=Production`), separate from the demo compose file, and its + release checklist references this list. — **BIO-020** +- [ ] **Verify by build, not by reading**: in a production bundle, `?role=`, `?subject=`, + `?scenario=`, `?rollen=` and the `⚙ state` panel are all inert — including on the three + hand-written `fetch` paths. — **BIO-012** + +--- + +## Summary + +| ID | Title | Control | Class | Sev. | Module | Effort | +| ----------- | -------------------------------------------------------------------- | ---------- | --------------- | ------ | ----------------------- | ------ | +| **BIO-001** | Backend trusts client-asserted identity headers in every environment | 9.2, 9.4 | production gate | high | BE/Domain+Program | S | +| **BIO-002** | Production backoffice has no identity; default is the seeded citizen | 9.4 | production gate | high | bhp/auth+root, BE | S | +| **BIO-003** | `X-Admin` is a second, unaudited gate outside `Authz` | 9.4, 8.15 | **defect now** | medium | BE/Program | S | +| **BIO-004** | `GET /uploads/{id}/content` + `/uploads/status` have no authz check | 9.4, 5.12 | **defect now** | high | BE/Program+Data | S | +| **BIO-005** | `POST /registrations` links arbitrary documents, unscoped | 9.4, 8.26 | **defect now** | medium | BE/Program | S | +| **BIO-006** | Reveal capability on the default role; step-up is a client constant | 9.4 | production gate | medium | BE/Domain, ssp/brief | S | +| **BIO-007** | Only denied decisions are audited; admin writes leave no trail | 8.15, 8.16 | **defect now** | medium | BE/Program | S–M | +| **BIO-008** | BSN written into the authz audit trail's `Resource` column | 8.15, 5.12 | **defect now** | high | BE/Program+Data | S | +| **BIO-009** | BSN is the `Actor` on every document audit row | 8.15, 5.12 | **defect now** | medium | BE/Data | S | +| **BIO-010** | BSN reaches logs + a persisted field via the ZGW error path | 8.15, 5.12 | **defect now** | medium | BE/Zgw+Program | S | +| **BIO-011** | Cross-owner lists ship unmasked BSNs; the detail masks | 5.12, 5.13 | **defect now** | medium | BE/Contracts | S | +| **BIO-012** | `?role=` / `?subject=` reach production on 3 hand-written fetches | 5.13, 9.4 | **defect now** | medium | ssp/brief, shared/infra | S | +| **BIO-013** | Seeded-citizen endpoints ignore the caller (no row scoping) | 9.4, 5.12 | production gate | medium | BE/Program | M | +| **BIO-014** | No encryption at rest for bytes, BSNs and the audit trail | 8.24 | production gate | high | BE/Data | L | +| **BIO-015** | No TLS/HSTS/nosniff/CSP; Swagger + `AllowedHosts:*` unconditional | 8.24, 8.28 | production gate | medium | BE/Program | S | +| **BIO-016** | CI security gates: semgrep+audit present; 5 gaps | 8.25/28/29 | production gate | medium | repo (CI) | S–M | +| **BIO-017** | Two PII guards have no executable test | 8.29, 5.13 | **defect now** | low | ssp/auth, ssp/shell | S | +| **BIO-018** | `IdempotencyStore` unscoped by caller, no TTL, unbounded | 9.4, 8.26 | **defect now** | low | BE/Data | S | +| **BIO-019** | `?peildatum=` 500s on unparseable input | 8.26 | **defect now** | low | BE/Stamdata | S | +| **BIO-020** | The only deployment artifact builds development bundles | 8.32 | production gate | medium | repo (compose) | M | + +**Twelve defect-now findings** (BIO-003, 004, 005, 007, 008, 009, 010, 011, 012, 017, 018, 019 +— BIO-006's step-up sub-item is counted inside its production gate), **eight production gates**. +Fifteen of the twenty are effort **S**. + +**Modules with no findings of their own:** ssp/registratie · ssp/herregistratie · +bhp/behandeling · libs/shared/{domain, application, ui, layout, kernel, upload, testing, +environments} · libs/beheer. + +**Recorded as correct, so a later pass does not "fix" them:** the deny-by-default +`AccessStore.can()` + `whenReady()` pair; `capabilityGuard`'s "UX pre-gate, backend +re-enforces" claim, verified endpoint by endpoint for all six admin surfaces; +`Authz.CanBeoordelen`'s caller-kind derivation (the one capability a forged `X-Role` cannot +reach); the four-eyes rule in `Authz.CanActOn` with Forbidden-before-Conflict ordering; the +`isDevMode()` gate on the debug panel and the interceptor chain; the ZGW secret never reaching +the browser and the notification webhook failing closed on an unset secret; the upload +content-type allow-list enforced server-side; stamdata having no runtime write endpoint at +all; and `libs/shared/src/kernel/{bsn,pii}.ts`, which are the standard the rest should be +measured against. diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md index e69de29..8c954a0 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/99-backlog.md @@ -0,0 +1,421 @@ +## Scope: all findings from 00-baseline, 02-testability, 04-cqrs-light, 06-adr-conformance, 07-bio2-compliance — deduplicated, scored, CD-sequenced + +## Status: complete + +## Last updated: 2026-08-27 + +## Depends on: 00-baseline.md, 02-testability.md, 04-cqrs-light.md, 06-adr-conformance.md, 07-bio2-compliance.md + +## --- + +# 99 — Consolidated refactoring backlog + +**47 findings in, 33 open tickets + 5 ADR-fixes + 1 shipped set out.** Everything below +traces to at least one `TE-`/`CQ-`/`ADR-C-`/`BIO-` finding and cites a baseline metric. + +**HALT.** This file is the deliverable. No Implementation Agent starts until a human has +approved it. Nothing in this pass was implemented; no source file was modified. + +--- + +## Coverage of this backlog — read this before treating it as complete + +Three of the seven Phase 1 agents were **deliberately skipped** by the operator +(reasons recorded in `_status.md`). This backlog therefore contains **no findings of the +following kinds**, and their absence is not evidence that none exist: + +| Agent not run | Category of finding that is absent | +| ---------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **01 — readability** | Function/file length, naming, nesting depth, comment quality, dead code, test readability. No ticket below is a "this is too long/unclear" ticket. | +| **03 — DDD/hexagonal** | Backend layering, vertical-slice structure, port extraction, module boundaries. The backend's structure is untouched except where CQRS-light reached it. | +| **05 — BDD** | Nothing material — the agent self-reduced to a structural note; `gen:behaviour-spec` already covers the intent. | + +Concrete consequences, so nobody assumes these were considered and dismissed: + +- **`createDraftSync` (143 lines, the longest function in the repo, §4a) is only partly + addressed.** RB-21 splits its read half out on CQRS grounds. Whether the remainder is + still too long was never assessed. +- **The other named length/complexity candidates have no owner:** + `api-client.provider.ts:49 fetch` (CC 19) and `rich-text-dom.ts:130 collect` (CC 11) — + the only two CC>10 functions outside the mandated idioms per **BL-001**; the 293-line + CC-20 test method in `OpenZaakZaakSourceTests.cs`; and the six files over 400 lines + (§9). RB-19 reorders `Program.cs` but does not shorten it. +- **Backend structure was assessed only through the CQRS-light lens.** **BL-003**'s + invitation (940 lines → `Features/`) is filed as out-of-mandate **OOM-A**, not a ticket. + **BL-010** (`libs/shared/upload/` outside the layer convention) is resolved only + incidentally, by RB-24, which came from the ADR agent rather than the structure agent. +- **Two baseline observations remain unowned by any agent:** **BL-005** (backend branch + coverage 18 points behind line coverage; `Contracts` 65.0%, `Stamdata` 71.7%, `Data` + 75.5% — `backend/tests/` has no `Contracts/` folder at all) and **BL-009** (no coverage + ratchet anywhere). Neither is a testability _blocker_, so agent 02 correctly declined + both; they are coverage work with no seam to add, and no ticket below covers them. + +--- + +## Already done — implemented and committed, do not re-file + +Branch `refactor/adr-c-006-shared-route-guards`, five commits. + +| Finding | Commit subject | Status | Residual | +| ------------- | ----------------------------------------------------------------------------------- | --------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **ADR-C-005** | `docs(adr-0002): accept, and record the unbuilt Principal union as debt` | **implemented** | ADR-0002 is now `Accepted`, so **RB-13 (ADR-C-004) now stands on a correct ADR** — that was the whole point of the gate. | +| **ADR-C-006** | `refactor(auth): share the actor-agnostic route guards (ADR-C-006)` | **implemented** | Auth duplication **211 → 151 lines**. §5's `ssp/auth 100% / bhp/auth 86.8%` rows and the `auth.guard*` clone pairs in the baseline are now **stale** — re-measure before citing them. Standing compliance criterion from agent 07: any future change to `authGuard`/`capabilityGuard` is an access-control change and must re-run the guard spec for both apps. | +| **CQ-004** | `fix(flags): surface a failed admin toggle instead of swallowing it` | **implemented** | **Half of its compliance criterion is unmet.** Agent 07 required "fix the FE error **and** the BE audit row together". The FE error shipped; `PUT /admin/flags/{key}` still writes **no** audit row. That half is carried by **RB-07**, and it is why **ADR-C-009** must not be signed off before RB-07 lands. | +| **TE-009** | `fix(stamdata): evaluate the profession validity window per call, not at type-load` | **implemented** | Also closed the latent dead-`ActiveOn`-branch bug. Not compliance-flagged. | +| **BL-008** | `build: make coverageExclude actually exclude the generated API client` | **implemented** | The reported `libs/shared/infrastructure` figure should now read ≈94.7%, not 6.9%. §3a is stale on that row. | + +**Correction to the hand-off.** The brief listed "CQ-002/004 (`FeatureFlagStore.set`)" as +fixed. Only **CQ-004** was — `FeatureFlagStore.set` is the CQ-004 subject. **CQ-002** +(`ApplicationsStore.cancel`, `AdminCasesStore.delete`) is **verified still open**: both +still do `try { await this.adapter.x(id) } catch { this.state.set(before) }` with no +`runSubmit`, no `Result`, and no error channel. It is filed below as **RB-20**. + +--- + +# The backlog + +**How to read the CD batch column.** A batch is a _suggested ordering wave_, not a release +train. Every ticket in the table ships **alone**, on its own merge, without any other +ticket in its batch. Where a ticket genuinely cannot ship alone it was split into a chain +(RB-22/RB-23) — see "Tickets that were rejected and split". `Depends on` means _must be +deployed first_, not _must ship together_. + +**Compliance column.** `SIGN-OFF` = requires compliance sign-off before merge, per rule 4. +Every ticket tracing to a `BIO-` finding, plus every row on agent 07's authoritative +16-row "Compliance review required" list, carries it — regardless of priority. + +| ID | Module | Category | Description | Baseline metric improved | Effort | Risk | Priority | CD batch # | Depends on | Compliance | Status | +| --------- | -------------------------------- | ------------- | ------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------- | ------ | -------- | -------- | ---------- | ---------- | ------------ | ------ | +| **RB-01** | backend/Program.cs + Data | security | Add an owner/capability check to `GET /uploads/{id}/content` and `/uploads/status`; 404 not 403 | §3c Data 75.5% branch vs 99.0% line (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | open | +| **RB-02** | backend/Program.cs + Data | privacy | Stop concatenating the BSN into `AuthzAudit.Resource`; assert on **values** in the test | §3c Data 75.5% branch (BL-005) | S | Low | **P1** | 1 | — | **SIGN-OFF** | open | +| **RB-03** | backend/Contracts | privacy | `MaskTail(a.Owner, 3)` in `ToAdminSummaryDto` — both cross-owner lists inherit it | §3a bhp/behandeling 91.6%/81.5%; §7 Mapping row | S | Low | **P1** | 1 | — | **SIGN-OFF** | open | +| **RB-04** | backend/Data | privacy | Mask the BSN used as `AuditEntry.Actor` on document audit rows (ownership column untouched) | §3c Data 99.0% line / 75.5% branch | S | Low | **P1** | 1 | — | **SIGN-OFF** | open | +| **RB-05** | backend/Zgw | privacy | Drop the BSN-bearing query + body snippet from the `ZgwHttpClient` exception message | §3c Zgw 98.1%/85.5% (best backend branch) — a design gap, not a test gap | S | Low | **P1** | 1 | — | **SIGN-OFF** | open | +| **RB-06** | backend/Program.cs | security | Delete the dead `POST /registrations` (no FE caller) — or add the `ForeignIds` guard | BL-003 (48 mappings in 940 lines, file CC 78) | S | Low | **P1** | 1 | — | **SIGN-OFF** | open | +| **RB-07** | backend/Program.cs | audit | Audit the **allow** path in all five authz gates + the 3 brief transitions and the besluit | §3c Program.cs 84.8% branch; BL-003 | S–M | Med | **P1** | 2 | — | **SIGN-OFF** | open | +| **RB-08** | backend/Program.cs | security | Route `DELETE /admin/uploads/{id}` through `CasesAdmin`; delete the orphaned `IsAdmin` gate | BL-003; §7 CQRS-light wrappers row | S | Low | **P1** | 2 | RB-07 | **SIGN-OFF** | open | +| **RB-09** | backend/Domain + Program.cs | security | `IIdentityProvider` can express "no identity"; stub Development-only; fail fast in Production | §7 "Single-impl interface `IIdentityProvider`"; BL-006 | S | Med | **P1** | 2 | — | **SIGN-OFF** | open | +| **RB-10** | ssp/auth + bhp/auth + ssp/shell | testability | Extract `parseStoredSession` (×2 apps) + spec `redactProfile`; assert a stored BSN yields `''` | §3a auth 42.9%/46.2% (worst FE line, §8); file LH 2/LF 20, BRH 3/BRF 13 | S | Low | **P1** | 2 | — | **SIGN-OFF** | open | +| **RB-11** | ssp/brief + libs/shared/infra | security | Dev hatches out of prod on the 3 hand-written `fetch` paths; export their parse boundaries; fix the doc | §3b ssp/brief 42% reach (11/26, none `ui/`); §3a 68.8% branch | M | Med | **P1** | 2 | — | **SIGN-OFF** | open | +| **RB-12** | backend/tests (CI) | security gate | One test enumerating the route table; every route hits an authz wrapper or an explicit allow-list | BL-006 (zero backend architecture enforcement) | M | Low | **P1** | 3 | — | **SIGN-OFF** | open | +| **RB-13** | ssp/auth + bhp/auth | ADR execution | Land `Session → Principal`; `MedewerkerAdapter`; backoffice login stops being a DigiD/BSN form | BL-002 (211→151 dup after ADR-C-006; expected <40 after this) | M | Med | **P1** | 3 | RB-09 | **SIGN-OFF** | open | +| **RB-14** | repo (CI) | security gate | `dotnet list package --vulnerable --include-transitive` as a failing step | BL-006; §7 (the .NET tree is entirely unscanned today) | S | Low | P2 | 3 | — | **SIGN-OFF** | open | +| **RB-15** | backend/Program.cs | security | Wrap Swagger + the OpenAPI document in `if (app.Environment.IsDevelopment())` | BL-003; §3c Program.cs 97.4%/84.8% | S | Low | P2 | 3 | — | **SIGN-OFF** | open | +| **RB-16** | backend/Stamdata | input valid. | `DateOnly.TryParse` on `?peildatum=` → 400 instead of an unhandled 500 | §3c Stamdata 96.8% line / **71.7% branch** (BL-005) | S | Low | P2 | 3 | — | **SIGN-OFF** | open | +| **RB-17** | libs/shared/app + brief + beheer | CQRS-light | Split `runResult` (fold) from `runSubmit` (fold + idempotency mint); point the 5 reads at it | BL-007; §7 "read adapters 20 / mutations inline ~13" | S | Low | P2 | 3 | — | **SIGN-OFF** | open | +| **RB-18** | backend/Data | security | Key `IdempotencyStore` on `{SubjectId}:{idemKey}` | §7 stores "Not behind any port"; agent 02's Data note (no TTL, no reset) | S | Low | P2 | 3 | RB-17 | **SIGN-OFF** | open | +| **RB-19** | backend/Program.cs | structure | Reorder all 48 endpoints under read/write sub-banners; regroup admin-cases + org-template preview | BL-003 (940 lines, file CC 78 vs next-highest 27) | S | **High** | P2 | 4 | RB-12 | **SIGN-OFF** | open | +| **RB-20** | ssp/registratie | CQRS-light | `ApplicationsStore.cancel` / `AdminCasesStore.delete` through `runSubmit`; surface the error | BL-007; §7 "Command factories 3" | S | Low | P2 | 4 | — | **SIGN-OFF** | open | +| **RB-21** | ssp/registratie | CQRS-light | Extract the read half of `createDraftSync` into `application/find-concept.ts` | §4a `createDraftSync` 143 lines — longest fn in the repo; §9 (>40) | M | Med | P2 | 4 | — | — | open | +| **RB-22** | ssp/brief | CQRS-light | _(expand)_ `BriefStore.load()` tolerates a 404 by calling the existing `reset()` once | BL-003; §7 Backend CQRS-light row | S | Low | P2 | 4 | — | **SIGN-OFF** | open | +| **RB-23** | backend/Program.cs + Data | CQRS-light | _(contract)_ `GET /brief` 404s when absent; `GetOrCreate` → `Get` | BL-003; §7 Backend CQRS-light row | S | Med | P2 | 4 | RB-22 | **SIGN-OFF** | open | +| **RB-24** | libs/shared/upload | ADR conform. | Move `upload/` into `infrastructure`/`domain`/`application`; **delete** the depcruise carve-out | BL-010; §7 "+1 adapter outside `infrastructure/`", "8 of 9 machines in `domain/`"; §3b shared/domain 0% reach | M | Med | P2 | 5 | — | **SIGN-OFF** | open | +| **RB-25** | libs/shared/upload | testability | `UPLOAD_TRANSPORT` injection token (the `SESSION_PORT` shape) instead of `inject(KeepaliveTransport)` | §3a upload 52.0%/50.0%; §3b file unreached, non-`ui/` | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | open | +| **RB-26** | libs/shared/upload | testability | Move the accept/reject decision to `planFileSelection` in `upload.machine.ts` | §3a upload 52.0%/50.0%; §4a module max CC 27 | S | Low | P2 | 5 | RB-24 | **SIGN-OFF** | open | +| **RB-27** | libs/shared/upload | testability | Extract `uploadOutcome(status, responseText)` out of the XHR closure | file LH 5/64 (**7.8% line**), BRH 3/57 (**5.3% branch**) | S–M | Low | P2 | 5 | RB-25 | **SIGN-OFF** | open | +| **RB-28** | libs/beheer + ssp/brief | testability | `BLOB_PRESENTER` token; the 3 commands' success paths become assertable | §3a beheer/application **40.5% branch — worst FE**; brief.store BRH 32/64 | S–M | Low | P2 | 5 | — | **SIGN-OFF** | open | +| **RB-29** | backend/Domain | testability | Thread the existing `at` through `LetterHtml.ResolveAuto` instead of reading `UtcNow` | §3c Domain 82.0% branch; §4b `LetterHtml.cs` CC 21 | S | Low | P2 | 5 | — | — | open | +| **RB-30** | backend/Data + Domain | testability | Extract 5 brief guards into `Domain/Letters/BriefRules.cs`; add `tests/Domain/BriefRuleTests.cs` | §3c Data **75.5% branch** (BL-005); §4b `BriefStore.cs` CC 17, `ToDto` CC 16 | M | Med | P2 | 5 | — | **SIGN-OFF** | open | +| **RB-31** | 4 app contexts (specs only) | ADR conform. | Replace hand-rolled state literals with `given(reduce, initial)` replays in 4 machine specs | §7 Elm machines 9 (1 has a `*.testing.ts`); §3a herreg 67.8% / brief 68.8% branch | M | Low | P2 | 6 | — | — | open | +| **RB-32** | libs/shared/docs | ADR conform. | Add the missing `language-switcher` row to the CIBG gap register (9 markers vs 8 rows) | §2 libs/shared 86 files / 5 194 lines; §6 layout Ca 22 | S | Low | P3 | 6 | — | — | open | +| **RB-33** | libs/shared/testing | ADR conform. | Adopt `unwrapOk` at its one call site — **or delete it**; both satisfy ADR-0006 §3 | BL-004; §3a libs/shared/testing 3 files, 100% line | S | Low | P3 | 6 | — | — | open | + +--- + +## Notes on the table + +**Why P1 is not simply "everything".** Rule 2's P1 definition ("violates a correct ADR, +blocks testability, or is a BIO2 compliance risk") would catch nearly every finding, which +would make the score useless. It is applied as: **P1 = a control is broken, an accepted +ADR's decision is unexecuted, or a security-relevant guard has no test today.** A ticket +that is merely _flagged because it touches a control_ (TE-003/4/5/6/8, CQ-006, ADR-C-002) +is **P2 with mandatory sign-off** — the compliance risk is one the ticket could introduce, +not one that exists. That distinction is the whole reason rule 4 is orthogonal to rule 2. + +**RB-01 and RB-02 sort above every structural ticket** regardless of effort. Both are live +production-shaped defects, independently verified: a BSN concatenated into the persisted +authz audit `Resource` (`Program.cs:674`) and an unauthorized document-content endpoint +(`GET /uploads/{documentId}/content`). Four documents claim the audit trail holds no PII +and the test cited as enforcing it (`AuthzAuditTests.cs:51-53`) asserts on **column +names**, so the BSN travels in a column called `Resource` that the regex cannot see — the +value-asserting test is part of RB-02's definition of done, not a follow-up. + +**RB-11 ships the doc correction in the same diff as the code.** `?role=` and `?subject=` +are _not_ stripped from production builds on three hand-written `fetch` adapters, while +`docs/reference/roles-and-access.md:23` says "they do not exist in a production build". +Correcting the doc without the code, or the code without the doc, both leave the repo +lying about itself. `?subject=` additionally writes a **BSN into `sessionStorage`** in any +build, which is the specific thing `SessionStore`'s G1 comment promises never happens. + +**RB-12 before RB-19, deliberately.** Agent 07 flags CQ-006 as needing the authz suites as +its safety net; agent 04 flags it as the prerequisite for OOM-A. RB-12's route-table test +is the check that "each moved endpoint kept its gate" is verified by CI rather than by a +reviewer's eye across a 900-line diff. RB-19 carries the only **High** risk in the table +for exactly that reason and must land alone, never mixed with a behaviour change. + +**RB-07 gates ADR-C-009, not the other way round.** Agent 06's proposed four-part test for +runtime-editable config includes "writes are admin-capability-gated **and audited**". +Today they are gated and not audited. Signing the ADR amendment first would ratify a +control the code does not implement. + +**RB-13's dependency on RB-09 is real, not stylistic.** Landing `Principal` on the +frontend alone closes ADR-C-004 and leaves BIO-002 wide open: a production behandelportal +build still resolves to the seeded **zorgverlener** — failing closed on backoffice +capabilities (correctly) but **open on every citizen-scoped endpoint** and holding +`CanRevealBigNummer`, because `drafter` is the no-header default. RB-09 makes "no +identity" representable at the interface; RB-13 is the FE half. + +--- + +## Merges — what was deduplicated, and how confident each merge is + +| Merged ticket | Findings folded in | Confidence | Reasoning | +| --------------- | ------------------------------------------ | ------------------ | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| **RB-10** | TE-001 + BIO-017 | **Certain** | Agent 07 says outright: "this is TE-001 plus one assertion; it does not need its own ticket if TE-001 is scheduled". BIO-017's second half (`redactProfile` spec) is a five-line spec in the same PII-guard category, so it rides along. | +| **RB-11** | BIO-012 + TE-002 + BIO-006(a) + BIO-006(b) | **Certain** | Agent 07 instructs: "Fix all three in one touch of the file, or the next reviewer will re-open it." All four land in the same three `fetch` adapters plus `role.ts`/`subject.ts` plus one doc line. BIO-006(b) is the same doc edit as BIO-012's. | +| **RB-09** | BIO-001(a) + BIO-001(b) + BIO-002 | **Certain** | BIO-001's own remediation _is_ (a) fail-fast + (b) "give `Resolve` a way to say no identity (see BIO-002)". BIO-002's root cause is the same non-nullable `Resolve`. One change, one file pair. | +| **RB-17** | CQ-003 + CQ-005 | **Certain** | Agent 04: "Fix them in one ticket; they are listed separately only because the module scope requires it." One shared-file split, five call sites. | +| **RB-14/12** | BIO-016 split into (a) and (b) | **Certain** | Two unrelated CI changes of different size and different value; the rest of BIO-016's "Absent" list is genuinely a production gate and stays on the checklist. | +| **RB-08** | BIO-003, sequenced behind RB-07 | High | Routing through `CasesAdmin` gives BIO-003's missing audit row for free **once** RB-07 has moved auditing to the allow path. Shipping BIO-003 first would mean writing the audit call twice. It can ship standalone if RB-07 slips. | +| **RB-18** | BIO-018, sequenced behind RB-17 | High | Agent 07: "Sequence CQ-003 before BIO-018 so the scoping change lands on a smaller call set." Not a merge, an ordering constraint. | +| **RB-25/26/27** | TE-003/004/005, sequenced behind RB-24 | **Judgement call** | Agent 04 argued BL-010 must be resolved before anything is layered onto the upload folder, and RB-24 (ADR-C-002) is the ticket that resolves it. But the three seams are each independently shippable **today**, against the current paths. If RB-24 is deferred or rejected, unblock all three — the dependency is hygiene, not correctness. | + +**Merges considered and rejected:** + +- **BIO-008 / BIO-009 / BIO-010 kept as three tickets (RB-02/04/05).** They share a theme + ("no BSN in any audit row, log line or persisted error field") and a shared acceptance + criterion (assert on **values**, e.g. no stored string matching `\d{9}`). They were not + merged because they sit in three modules with three different test suites, and BIO-010 + is conditional on `Zgw:Enabled` (off by default) which gives it a different risk profile. + Three one-line fixes that each ship alone beat one cross-module sweep. **If a reviewer + prefers one ticket, merging them is defensible** — this is the least settled call here. +- **CQ-002 not merged into BIO-007 (RB-07).** They are the two halves of the same + admin-mutation-observability gap, but one is FE error surfacing and the other is BE + auditing. Agent 07 asked only that they "ship aware of each other". Cross-referenced, + not merged. +- **`SessionStore` not merged across the TE-001 / residual-auth-duplication overlap.** + Both touch `session.store.ts`, but agent 06 is explicit that merging the two apps' + session stores now would cement a citizen DigiD/BSN login as the backoffice's login — + the exact outcome ADR-0002 §3 exists to prevent. RB-10 lands the same seam **twice**, on + purpose. The duplication question reopens only after RB-13, on re-measurement. +- **ADR-C-004 not merged into BIO-002.** Split into RB-09 (BE, S) → RB-13 (FE, M) instead, + because a single ticket spanning both would not be independently deployable. + +--- + +## Tickets that were rejected and split (rule 3) + +**CQ-007 → RB-22 then RB-23.** As filed, CQ-007 is the one finding agent 04 marked +"**no** — FE+BE together": the FE must handle a 404 that the BE does not yet return. +Shipping it as one ticket is a coordinated release. Split into the standard +expand/contract pair: + +1. **RB-22 (expand, FE).** `BriefStore.load()` tolerates a 404 by calling the existing + `reset()` command once. Deploys against today's backend as a **no-op** — the BE never + 404s, so the branch is dead on arrival and provably safe. +2. **RB-23 (contract, BE).** `GET /brief` returns 404 when no brief exists; + `BriefStore.GetOrCreate` splits into `Get` + the already-existing `ResetAndCreate`. + Deploys only once RB-22 is live. + +Agent 07 rejected CQ-007's documentation-only alternative outright: "a non-idempotent GET +must be visible in the code, not only in a ticket". That alternative is therefore **not** +on the table. + +**No other ticket failed the single-deploy test.** TE-001 lands in two apps but in one +merge; RB-24 touches 30 dependents but is one atomic move; RB-19 is a 900-line diff but +zero-semantic-change. + +--- + +# ADR-fix tickets — architect approval required before any dependent code ticket + +None of these five is a code change. All five change what the repo's architecture +documents _claim_. **Three of them require a matching CLAUDE.md correction in the same +diff** (CLAUDE.md's own precedence rule: "the docs win — update this file"). + +| ID | ADR | What the amendment does | Gates / blocks | CLAUDE.md edit? | Effort | Compliance | Status | +| ------------- | ---- | -------------------------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | --------------- | ------ | ------------ | -------- | +| **ADR-C-001** | 0001 | Rewrite the worked example against the real backend; drop the 2 discharged out-of-scope bullets (every path it names no longer exists) | nothing | no | S | — | pending | +| **ADR-C-003** | 0001 | State that the generated client **is** the wire contract post-codegen; scope `contracts/` to codegen gaps | any ticket deleting the 4 surviving `contracts/*.dto.ts`, or adding a hand-written DTO for a generated endpoint. **No open ticket below is blocked today** — recorded so a future one is. | **yes (§4)** | S | — | pending | +| **ADR-C-007** | 0003 | Repoint 5 WP-67-stale paths; replace the **factually false** `app-alert` hand-rolled example (it wraps vendored `.feedback` classes) | nothing | **yes (§2)** | S | — | pending | +| **ADR-C-009** | 0004 | Generalise "the org-templates exception" into a stated four-part test; list both passing surfaces | **RB-07.** Clause (4) is "writes are admin-capability-gated **and** audited". Today they are gated and _not_ audited — sign this before RB-07 and the ADR ratifies a control the code does not implement. | **yes (§4)** | S | **SIGN-OFF** | pending | +| **ADR-C-005** | 0002 | _(already landed — see "Already done")_ | was the gate on RB-13; now cleared | — | — | — | **done** | + +**No ADR-fix is proposed against ADR-0002 §3's non-sharing rule.** Agent 06 considered it +as instructed and rejected it with evidence: `grep -rn "Principal" apps libs` returns one +comment and no type, so the rule was never _tested_, only _unexecuted_. Amending it now +would ratify the omission rather than the evidence. The correct sequence is +ADR-C-005 (done) → **RB-13** → **re-measure BL-002**; agent 06's expectation is that the +residual duplication drops from 151 lines to under 40 on its own. If RB-13 is still +unstarted at the next backlog cycle, _that_ is when the ADR-fix conversation becomes +legitimate. + +--- + +# Production gates — a release checklist, not tickets + +These are **correct for a POC** and must be true before the system holds real BSNs. They +are deliberately kept out of the ticket table: they are acceptance criteria for a release +that does not exist yet (there is no production build artifact at all — **BIO-020**), not +work that can be merged and deployed this week. Where a _part_ of a production-gate +finding was shippable now, that part was pulled out as a ticket and is named below. + +**Identity and access (9.1, 9.2, 9.4)** + +- [ ] Replace `StubIdentityProvider` with verified DigiD / employee-SSO claims. `X-Role`, + `X-Subject`, `X-Medewerker`, `X-Rollen`, `X-Admin` removed as **inputs**, not ignored. — BIO-001 +- [ ] Verify by building both apps `--configuration production` that the backoffice cannot + act as a citizen. — BIO-002 _(the interface half is **RB-09**; the FE half is **RB-13**)_ +- [ ] Row-level scoping on every read returning person data; acceptance = a second seeded + citizen cannot see the first's dashboard, notes, BRP address or diplomas. — BIO-013 +- [ ] The PII-reveal capability comes from the app overlay, not the coarse role, and is + **not held by the default role**. — BIO-006 _(the `X-Step-Up` literal is in **RB-11**)_ +- [ ] Real step-up: a server-verified assurance/recency attribute no client can satisfy + with a constant. — BIO-006 + +**Cryptography (8.24)** + +- [ ] Encryption at rest with documented key custody and rotation. — BIO-014 + **Prerequisite: RB-02/04/05 first**, so the BSN is not in three places that do not + need it before deciding what must be encrypted. +- [ ] Document bytes move to encrypted object storage keyed by `DocumentId`. — BIO-014 +- [ ] TLS everywhere: `UseHttpsRedirection` + HSTS at the edge. — BIO-015 +- [ ] Security response headers (`nosniff`, CSP, `Referrer-Policy`) and a real + `AllowedHosts`. — BIO-015 _(the Swagger gate is **RB-15**)_ + +**Logging, monitoring and retention (8.15, 8.16)** + +- [ ] Audit retention, integrity and access defined — how long, append-only, and who may + read `/beheer/audit` (it reuses `cases:manage`, which `Program.cs:565` already flags + as a placeholder for a dedicated `audit:read`). +- [ ] Log shipping and alerting — the audit trail is a SQLite table with no export path. +- [ ] _(Covered by tickets: allow-path auditing = **RB-07**; no BSN in any audit row, log + line or persisted error field = **RB-02/04/05**.)_ + +**Data protection (5.12, 5.13)** + +- [ ] A DPIA covering BSN, uploaded identity documents and the register, with lawful basis + and retention schedule. Nothing in the repo covers this. +- [ ] Deletion / retention policy for uploaded documents and the audit trail. +- [ ] _(Covered: data minimisation on list endpoints = **RB-03**.)_ + +**Secure development (8.25, 8.28, 8.29)** + +- [ ] Secret scanning in CI (prevention — nothing is committed today, verified). — BIO-016 +- [ ] Backend architecture enforcement (NetArchTest/ArchUnitNET) so `Domain/` purity, ZGW + containment (ADR-0005) and "authorization lives in `Authz`" are CI- rather than + review-maintained. — BL-006 +- [ ] A coverage ratchet, so a security fix can be verified as not regressed by CI. — BL-009 +- [ ] Penetration test / DAST, with BIO-004's object-level authorization and BIO-005's + document linking as named cases. +- [ ] _(Covered: backend dependency scanning = **RB-14**; the authorization regression gate + = **RB-12**.)_ + +**Change control (8.32)** + +- [ ] A production build and deployment artifact exists, separate from the demo compose + file, and its release checklist references this list. — BIO-020 +- [ ] Verify **by build, not by reading**: in a production bundle `?role=`, `?subject=`, + `?scenario=`, `?rollen=` and the `⚙ state` panel are all inert — including on the + three hand-written `fetch` paths. — BIO-012 _(the code fix is **RB-11**; this box is + the build-time proof)_ + +--- + +# Verified clean — do not "fix" + +Each of these was read and judged correct by the agent named. Re-checking them is wasted +effort; "simplifying" them is a regression. + +**Security and access control** (agent 07, verified endpoint by endpoint) + +- `AccessStore.can()` deny-by-default + `whenReady()` — the pair exists so the guard cannot + read `can()` mid-load and deny an entitled user. +- `capabilityGuard`'s "UX pre-gate, the backend re-enforces" claim — verified true for all + six admin surfaces; every capability the guard checks has a server-side twin. +- `Authz.CanBeoordelen`'s caller-kind derivation — the one capability a forged `X-Role` + cannot reach, and the reason BIO-002 fails _closed_ in that direction. +- The four-eyes rule in `Authz.CanActOn`, Forbidden-before-Conflict ordering. +- The `isDevMode()` gate on the debug panel and on the interceptor chain (the _interceptor_ + chain is correctly gated — RB-11 is about the three adapters that bypass it). +- The ZGW client secret never reaching the browser; the notification webhook failing closed + on an unset secret; `ZgwDiagnosticHandler` logging no bodies and being opt-in. +- The upload content-type allow-list enforced **server-side** — which is also why + `nosniff` is a checklist item and not a finding. +- Stamdata having no runtime write endpoint at all. +- `libs/shared/src/kernel/{bsn,pii}.ts` — the standard the rest should be measured against. +- No secrets committed; no `.db` file tracked (both verified by `git check-ignore`/`ls-files`). + +**Architecture and structure** + +- **ADR-0005 is fully conformed — zero findings** (agent 06). The ZGW anti-corruption layer + is the repo's worked example; the ADR even predicted its own remaining gap and the gap + stayed where predicted. +- **`bhp/behandeling` is the CQRS-light reference implementation** (agent 04). Query + adapters, command adapter and command factory in separate files, write-free read stores. + Do not "clean it up". +- **The FE dependency structure is not a problem area** (baseline §6): 0 violations across + 11 `severity: error` rules, textbook instability gradient (`kernel` I=5%, contexts I≥83%). + Do not spend tickets here. +- `BigProfileStore` — the reference implementation of the read/write split (agent 04). +- The `ToDetailDto(now)` / `ToDto(now)` status projection — a real read-model derivation; + do not let a future ticket "simplify" it into a stored status column (agent 04). +- The 7 static backend stores and `[assembly: DisableTestParallelization]` — deliberate, + documented in `Data/Db.cs`, and explicitly _not_ challenged by agents 02, 04 or 07. + RB-30 works **because** the rules never needed the DbContext, not by redesigning stores. + +**Baseline rows closed as false gaps** (agent 02, verified — do not ticket them) + +- `libs/shared/domain` 0% reach / 3 files, and `libs/beheer/contracts` 0% reach / 1 file. + Both are pure type declarations with **zero executable statements**; 0% is correct and + unimprovable. BL-004 named both as "genuine gaps"; that part of BL-004 is superseded. +- 23 of the 25 CC>10 TS functions are reducers / `parse*` / `validate*` — mandated house + idioms (**BL-001**). A bare CC number is not grounds for a ticket against any of them. +- `createDraftSync` is **acquitted on testability** (explicit deps object, optional + injection, `enabled()` escape hatch, has a spec). RB-21 is a CQRS split, not a fix. +- `httpClientFetch`, `Contracts/Mappers.cs`, `submit-besluit.ts`, `breadcrumb-trail.ts`, + `route-focus.ts`, `AccessStore.can()` — all "missing test, not blocked test", or a seam + that costs more than it returns. Filing them would be volume, not quality. + +--- + +# Out of mandate — recorded so a later phase does not read this file as a step toward them + +- **OOM-A — `Program.cs` → `Features/` folders with handler types.** BL-003's most obvious + invitation, and out of mandate because §7 is explicit that the backend has "no handler + types, no mediator, no `Features/` folders" — there is no structure to extend, only one + to introduce. **RB-19 is a strict prerequisite** if it is ever taken: you cannot cut a + 940-line file into vertical slices while five of its seven sections interleave + directions. Agent 03, which would have owned this, did not run. +- **OOM-B — read/write repository split in `backend/Data`.** Would introduce the pattern + where §7 records it absent, and collides with the documented static/no-DI design. +- **OOM-C — no read model, no event sourcing, and none proposed.** +- **OOM-D — BL-011: the FE suite is flaky under parallel load, and BL-009 means nothing + ratchets.** "CI green" alone does not verify any ticket in this backlog. Verify against + `00-baseline.md`'s numbers — **and note that §3a, §3b and §5 are already partly stale** + after the five shipped commits (auth duplication 211→151; `libs/shared/infrastructure` + coverage no longer dragged down by the generated client). **Re-run the baseline before + using it as the before-picture for any ticket below.** + +--- + +## Provenance + +| Source finding | Where it went | +| ------------------------------------------------------- | -------------------------------------------------------------------- | +| TE-001…008 | RB-10, RB-11, RB-25, RB-26, RB-27, RB-28, RB-29, RB-30 | +| TE-009 | **shipped** | +| CQ-001, 002, 003+005, 006, 007 | RB-21, RB-20, RB-17, RB-19, RB-22+RB-23 | +| CQ-004 | **shipped** (BE audit half outstanding → RB-07) | +| ADR-C-001, 003, 007, 009 | ADR-fix table | +| ADR-C-002, 004, 008, 010, 011 | RB-24, RB-13, RB-32, RB-31, RB-33 | +| ADR-C-005, 006 | **shipped** | +| BIO-001, 002 | RB-09 + checklist | +| BIO-003, 004, 005, 007, 008, 009, 010, 011, 018, 019 | RB-08, RB-01, RB-06, RB-07, RB-02, RB-04, RB-05, RB-03, RB-18, RB-16 | +| BIO-006 | RB-11 (a+b) + checklist (c) | +| BIO-012, 017 | RB-11, RB-10 | +| BIO-015, 016 | RB-15 + checklist; RB-14 + RB-12 + checklist | +| BIO-013, 014, 020 | checklist only | +| BL-008 | **shipped** | +| BL-005, BL-009, BL-011 | **unowned** — see "Coverage of this backlog" and OOM-D | +| BL-001, BL-002, BL-004 (partly), BL-006, BL-007, BL-010 | absorbed into the tickets/checklist above | diff --git a/docs/project/refactor-backlog-setup/refactor-backlog/_status.md b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md index e952668..88b4e89 100644 --- a/docs/project/refactor-backlog-setup/refactor-backlog/_status.md +++ b/docs/project/refactor-backlog-setup/refactor-backlog/_status.md @@ -1,13 +1,13 @@ # Agent run status -| Agent | Status | Last module processed | Last updated | Notes | -| --------------- | ----------- | -------------------------- | ------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| baseline | complete | all (18 FE + 7 BE modules) | 2026-08-26 | 0 layering violations. 11 observations BL-001..BL-011. Thresholds for Phase 1 in §9 — agent 01's `[N] lines` = 40 (TS fn) / 60 (C# method) / 400 (file). Read BL-001, BL-002 and BL-004 before filing complexity, duplication or coverage findings. | -| readability | not_started | - | - | | -| testability | complete | all 24 modules | 2026-08-26 | 9 findings TE-001..TE-009; 15 modules explicit "no findings". Corrected BL-004 — `libs/shared/domain` and `libs/beheer/contracts` are pure type declarations, 0% is unimprovable (amendment note in 00-baseline.md §10). TE-009 is also a latent correctness bug (dead `ActiveOn` branches). Acquitted `createDraftSync`, `httpClientFetch`, `Contracts/Mappers.cs` in writing. | -| ddd-hexagonal | not_started | - | - | | -| cqrs-light | complete | all (16 FE + 6 BE modules) | 2026-08-26 | 7 findings CQ-001..CQ-007; 12 modules clean. Corrected BL-007 (see the amendment note in 00-baseline.md §10) and found 3 mutations the baseline missed. `Program.cs` → `Features/`+handlers filed as out-of-mandate OOM-A, not a ticket; CQ-006 is its prerequisite. `bhp/behandeling` named the reference implementation — do not "clean it up". | -| bdd | not_started | - | - | | -| adr-conformance | complete | all 6 ADRs | 2026-08-26 | 11 findings: 6 code, **5 ADR-fix (architect approval required)**. Sharpened BL-002 — `Principal` was never built, so ADR-0002 was untested not falsified (amendment note in 00-baseline.md §10). ADR-0005 fully conformed. Gates: ADR-C-005→ADR-C-004; ADR-C-003 gates contracts/ cleanup. 3 ADR-fixes need a matching CLAUDE.md correction in the same diff. | -| bio2-compliance | not_started | - | - | | -| consolidation | not_started | - | - | | +| Agent | Status | Last module processed | Last updated | Notes | +| --------------- | -------------------------- | ----------------------------- | ------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| baseline | complete | all (18 FE + 7 BE modules) | 2026-08-26 | 0 layering violations. 11 observations BL-001..BL-011. Thresholds for Phase 1 in §9 — agent 01's `[N] lines` = 40 (TS fn) / 60 (C# method) / 400 (file). Read BL-001, BL-002 and BL-004 before filing complexity, duplication or coverage findings. | +| readability | skipped | n/a | 2026-08-27 | **skipped** — deliberate. BL-001: 23 of the 25 TS functions over CC 10 are reducers / `parse*` boundaries / `validate*`, all mandated house idioms; TS fn-length p99 is 34 with only 2 functions over 75 lines. Little left for this agent to find that is not a false positive. Revisit if the CC>10 population grows outside those three shapes. | +| testability | complete | all 24 modules | 2026-08-26 | 9 findings TE-001..TE-009; 15 modules explicit "no findings". Corrected BL-004 — `libs/shared/domain` and `libs/beheer/contracts` are pure type declarations, 0% is unimprovable (amendment note in 00-baseline.md §10). TE-009 is also a latent correctness bug (dead `ActiveOn` branches). Acquitted `createDraftSync`, `httpClientFetch`, `Contracts/Mappers.cs` in writing. | +| ddd-hexagonal | skipped | n/a | 2026-08-27 | **skipped** — deliberate. FE layering is clean (baseline §6: 0 violations, healthy instability gradient, `kernel` I=5% vs contexts I>=83%); backend `Domain/` is verified EF/ASP-free. The agent may only _extend_ existing hexagonal structure, and the one real target (`Program.cs`) has no `Features/` folder to extend — agent 04 already filed that as out-of-mandate OOM-A. | +| cqrs-light | complete | all (16 FE + 6 BE modules) | 2026-08-26 | 7 findings CQ-001..CQ-007; 12 modules clean. Corrected BL-007 (see the amendment note in 00-baseline.md §10) and found 3 mutations the baseline missed. `Program.cs` → `Features/`+handlers filed as out-of-mandate OOM-A, not a ticket; CQ-006 is its prerequisite. `bhp/behandeling` named the reference implementation — do not "clean it up". | +| bdd | skipped | n/a | 2026-08-27 | **skipped** — deliberate. No BDD tooling present, and the prompt forbids proposing any; it self-reduces to a single structural note. `gen:behaviour-spec` already extracts behaviours from spec names into `libs/shared/docs/behaviour-spec.mdx`, which covers the intent. | +| adr-conformance | complete | all 6 ADRs | 2026-08-26 | 11 findings: 6 code, **5 ADR-fix (architect approval required)**. Sharpened BL-002 — `Principal` was never built, so ADR-0002 was untested not falsified (amendment note in 00-baseline.md §10). ADR-0005 fully conformed. Gates: ADR-C-005→ADR-C-004; ADR-C-003 gates contracts/ cleanup. 3 ADR-fixes need a matching CLAUDE.md correction in the same diff. | +| bio2-compliance | complete | all modules + 7 control areas | 2026-08-27 | 20 findings BIO-001..BIO-020 (12 **defect now**, 8 **production gate**). High: BIO-008 BSN concatenated into the authz audit `Resource` (`Program.cs:674`, verified); BIO-004 `GET /uploads/{documentId}/content` has no authz at all (verified). Answered agent 06's handoff as BIO-002 — a production behandelportal build resolves to the seeded **zorgverlener**, failing closed on backoffice caps but open on citizen-scoped ones incl. `CanRevealBigNummer`. Carries the mandatory **"compliance review required"** list: 16 rows over agents 02/04/06. Also a pre-production checklist (~25 boxes). | +| consolidation | halted (awaiting approval) | all inputs | 2026-08-27 | **HALTED for human approval** (per spec) — `99-backlog.md` written, nothing implemented. 33 open tickets RB-01..RB-33 + 5 ADR-fixes + 5 already-shipped, from 47 findings. RB-01 (no authz on upload content) and RB-02 (BSN in the audit `Resource`) sort above all structural work. Gate relaxed to the 4 agents that ran; a "Coverage of this backlog" note records what the 3 skips leave unowned. Caught two orchestrator errors: **CQ-002 is NOT fixed** (verified — `ApplicationsStore.cancel`/`AdminCasesStore.delete` still swallow errors → RB-20), and **CQ-004 shipped with half its compliance criterion unmet** (no audit row on `PUT /admin/flags/{key}`, verified → RB-07, which blocks signing ADR-C-009). OOM-D: re-run the baseline before using it to verify any ticket — ADR-C-006 and BL-008 moved it. |