From 176e5baef8e5907d0b2c7d35b6321c4b03c9a018 Mon Sep 17 00:00:00 2001 From: Edwin van den Houdt Date: Thu, 27 Aug 2026 09:52:13 +0200 Subject: [PATCH] docs: BIO2 compliance pass + consolidated backlog (agents 07, 08) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the pipeline's analysis phase. Agent 07 (BIO2/ISO 27002:2022, control set stated as an assumption since none was supplied) produced 20 findings — 12 "defect now", 8 "production gate" — and agent 08 consolidated all 47 findings across 00/02/04/06/07 into 33 tickets, 5 ADR-fixes and a release checklist. Two findings are live defects rather than refactoring candidates, both verified directly: - RB-01/BIO-004: GET /uploads/{documentId}/content takes only (string documentId) — no HttpContext, so no authorization is possible. It streams diploma and identity scans, protected by GUID unguessability alone, while DELETE on the same resource is owner-scoped. - RB-02/BIO-008: Program.cs:674 concatenates the caller's BSN into the authz audit Resource column, which is persisted to SQLite and rendered by the admin audit page. Four doc comments claim that store holds no PII; the test cited as enforcing it asserts on column names, so a BSN inside a column called Resource is invisible to it. 07 also answered the handoff from 06: in a production behandelportal build no X-Medewerker is sent, so StubIdentityProvider returns the seeded citizen. It fails closed on backoffice capabilities but open on citizen-scoped ones, including CanRevealBigNummer. Root cause is IIdentityProvider.Resolve returning a non-nullable CallerIdentity — the interface cannot express "no identity", so any provider must invent one. 08's gate was relaxed from all-seven to the four agents that ran; _status.md records why 01/03/05 were skipped, and the backlog carries a "Coverage" note naming what those skips leave unowned. It caught two errors in the orchestrator's handoff: CQ-002 is not fixed (ApplicationsStore.cancel and AdminCasesStore.delete still swallow errors -> RB-20), and CQ-004 shipped with half its compliance criterion unmet (PUT /admin/flags/{key} writes no audit row -> RB-07, which blocks signing ADR-C-009). Both agents preserved a "verified clean — do not fix" list, so a later pass does not re-spend effort on the controls that already hold. Consolidation halted for human approval per its spec. No source file changed. Co-Authored-By: Claude Opus 5 --- .../refactor-backlog/07-bio2-compliance.md | 1127 ++++++++++++++++- .../refactor-backlog/99-backlog.md | 421 ++++++ .../refactor-backlog/_status.md | 22 +- 3 files changed, 1555 insertions(+), 15 deletions(-) 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. |