docs(quick-260910-exd): Etappe 2 Bereich module-registry abgeschlossen und verifiziert
This commit is contained in:
+275
@@ -0,0 +1,275 @@
|
||||
---
|
||||
phase: quick-260910-exd
|
||||
verified: 2026-09-10T00:00:00Z
|
||||
status: passed
|
||||
score: 9/9 must-haves verified
|
||||
covered_files: [".planning/WINDOWS.md", ".planning/quick/260910-exd-mandantentrennung-etappe-2-bereich-modul/260910-exd-PLAN.md", ".planning/quick/260910-exd-mandantentrennung-etappe-2-bereich-modul/260910-exd-SUMMARY.md", "apps/api/scripts/rls-scratch-check.mjs", "apps/api/src/module-registry/module-access.service.spec.ts", "apps/api/src/module-registry/module-access.service.ts", "apps/api/src/module-registry/module-registry.service.spec.ts", "apps/api/src/module-registry/module-registry.service.ts", "apps/api/src/module-registry/module.guard.spec.ts", "apps/api/src/tenders/tender-scheduler.service.spec.ts", "docs/mandantentrennung-etappe2-fehlerrichtung.md", "docs/mandantentrennung-zugriffsklassifikation.md"]
|
||||
covered_digest: "v1:sha256:47e75c553a401d96420242d168f37b0fbc0f89e54dfac382bd3ee80ba1d04a0c"
|
||||
overrides_applied: 0
|
||||
behavior_unverified: 0
|
||||
---
|
||||
|
||||
# Quick Task 260910-exd Verification: Mandantentrennung Etappe 2, Bereich `module-registry`
|
||||
|
||||
**Task Goal:** Bind the tenant-bound access sites in `apps/api/src/module-registry/`, leave the
|
||||
platform-wide module catalogue deliberately unbound with the CORRECTED reason, create the missing
|
||||
spec coverage, record the absent denial-signal three ways, and leave the classification document's
|
||||
five hand-maintained sections in sync.
|
||||
|
||||
**Verified:** 2026-09-10
|
||||
**Status:** passed
|
||||
**Commits reviewed:** 7d45e2f, 3df7268, 9c0eefe (base a2516a9)
|
||||
|
||||
This is a re-verification-grade, adversarial re-audit against the codebase — not a re-read of
|
||||
SUMMARY.md. Every claim below was independently reproduced (live DB run, source greps, recomputed
|
||||
class-distribution table, commit-level diffs) rather than accepted from the SUMMARY.
|
||||
|
||||
## Priority Findings (per orchestrator's numbered scrutiny list)
|
||||
|
||||
### 1. THE PRIORITY ITEM — `tender-scheduler.service.spec.ts` identity mock: LEGITIMATE, not a regression
|
||||
|
||||
Verified by reading the file and its neighbors directly:
|
||||
|
||||
- `tender-scheduler.service.spec.ts` mocks `forTenant` to identity (`vi.fn((p) => p)`) — this is
|
||||
true, and by itself would be the exact defect this effort documented in `ldap`.
|
||||
- **But the binding for the method this file exercises (`ModuleRegistryService.activateForTenant`)
|
||||
is proven elsewhere, and proven rigorously.** `apps/api/src/module-registry/module-registry.service.spec.ts`
|
||||
(new file, 344 lines, 16 `it()` cases) uses a genuine **two-client bound-call-log proof**
|
||||
(`__makeBoundClient`, a second, distinguishable wrapper object that logs every call routed
|
||||
through it) — not an identity mock. Its `activateForTenant` describe block
|
||||
(lines 182–207) asserts `expectBoundCall(prisma, 't1', 'tenantModuleActivation', 'upsert')` —
|
||||
this assertion is false (test fails) if the binding is removed. Confirmed live: the SUMMARY's
|
||||
claimed falsification proof for this exact method (Task 3, `deactivateForTenant`'s update call)
|
||||
is reproduced verbatim in `docs/mandantentrennung-etappe2-fehlerrichtung.md` (see below) with a
|
||||
concrete red message, not just a commit-log claim.
|
||||
- Checked whether "the exact `ldap.service.spec.ts` convention" claim holds up: `ldap.service.spec.ts`
|
||||
does carry a **file-level identity mock** at the top (line 56–57, `forTenant: vi.fn((p) => p)`)
|
||||
**and** a separate, dedicated binding-proof block later in the same file (from ~line 2300) that
|
||||
reconfigures the `forTenant` spy per-test and asserts `expect(forTenant).toHaveBeenCalledWith(...)`
|
||||
for `listGroups`, `searchUsers`, etc. `ldap-config.service.spec.ts` follows the identical
|
||||
two-part pattern (identity mock at top, line 9–10; dedicated "Bindung an forTenant()" describe
|
||||
block at line 197 asserting `toHaveBeenCalledWith`). So the convention is real, not a misreading.
|
||||
`tender-scheduler.service.spec.ts` only needed the "identity mock" half of that convention,
|
||||
because — unlike `ldap.service.spec.ts` testing itself — it doesn't test `ModuleRegistryService`'s
|
||||
binding at all; it tests `TenderSchedulerService`'s poll-once-fan-out-many behavior using the real
|
||||
`ModuleRegistryService` as an unmocked collaborator. The binding proof for
|
||||
`ModuleRegistryService.activateForTenant` correctly lives in `module-registry.service.spec.ts`,
|
||||
which is the file that owns that method.
|
||||
- Verified no other unmocked caller of `new ModuleRegistryService(...)` exists
|
||||
(`grep -rn "new ModuleRegistryService" apps/api/src` → only the DI module and this one spec file).
|
||||
|
||||
**Conclusion: legitimate, not a regression in disguise.**
|
||||
|
||||
### 2. The Task-2/Task-3 classification-doc split: HONEST ordering consequence, not gaming
|
||||
|
||||
`git show 3df7268 -- docs/mandantentrennung-zugriffsklassifikation.md` shows the Task-2 commit
|
||||
touched **exactly two lines** of the classification doc: the `Stand` column (ungebunden→gebunden)
|
||||
and a one-sentence addition to the existing `Begründung` for `module-access.service.ts`/`moduleGrant`
|
||||
and `/tenantModuleActivation` — nothing else. The overview line, sum line, class-distribution table,
|
||||
and background-service-trap section were untouched in that commit and only changed in Task 3's
|
||||
commit (`9c0eefe`), exactly as the plan required. This is the minimum edit forced by Task 2's own
|
||||
verify gate (`npm --prefix apps/api run test` includes `rls-access-inventory.spec.ts`, which checks
|
||||
the doc against the live source on every run) — not scope creep or a doc bent to make a check pass.
|
||||
|
||||
### 3. The corrected premise (Befund E) is preserved as corrected
|
||||
|
||||
Both `module-access.service.ts` (lines 101–107) and `module-registry.service.ts` (multiple
|
||||
locations) carry the comment in the required two-part form: **MEASUREMENT** ("die Tabelle
|
||||
traegt heute keinen Zeilenschutz, eine Bindung waere heute wirkungslos, nicht katastrophal") plus
|
||||
**CONDITION** ("katastrophal wuerde sie erst, WENN Etappe 3 dieser Tabelle eine Regel gibt"). The
|
||||
classification doc rows (lines 301, 304) and the critique doc's (m4)/(m1) sections repeat this
|
||||
exact framing. No occurrence of the original, now-falsified claim ("a binding would make the
|
||||
catalog invisible today") was found anywhere in the diff.
|
||||
|
||||
### 4. Coverage and the bind/don't-bind line — independently counted
|
||||
|
||||
```
|
||||
this.prisma.module → module-access.service.ts: 1 (unbound)
|
||||
this.prisma.module → module-registry.service.ts: 6 (unbound)
|
||||
tenantPrisma.* → module-access.service.ts: 5 (bound: 2× moduleGrant, 3× tenantModuleActivation)
|
||||
tenantPrisma.* → module-registry.service.ts: 5 (bound: 5× tenantModuleActivation)
|
||||
```
|
||||
|
||||
**7 unbound / 10 bound — matches the SUMMARY's claim exactly**, and both numbers were counted fresh
|
||||
from the current source, not copied from any document. Raw-hit total (17) also matches Befund A.
|
||||
`grep -rn 'tenantPrisma\.module\.'` (the forbidden literal) returns zero hits — the catalog was not
|
||||
accidentally bound.
|
||||
|
||||
### 5. Default-closed cannot fail open
|
||||
|
||||
`module-access.service.spec.ts` line 354 ("Vorgabezustand bleibt geschlossen und ueberlebt die
|
||||
Bindung") pins that with `grantedIds.length === 0` the intersection query (`tenantModuleActivation`)
|
||||
is **never called** — the bound-call log is asserted empty for that model in that path. The
|
||||
ADMIN/SUPER_ADMIN short-circuit condition (`role === 'ADMIN' || role === 'SUPER_ADMIN'`) is
|
||||
unchanged from the pre-existing code — confirmed by reading `module-access.service.ts` line 53 and
|
||||
the git diff, which shows no changes to the role-check line.
|
||||
|
||||
### 6. The absent denial signal — recorded three ways, all confirmed
|
||||
|
||||
1. **Critique text**, unvarnished: `docs/mandantentrennung-etappe2-fehlerrichtung.md` (m3),
|
||||
"Die Antwort lautet **keines** — schlicht, ohne Beschönigung," with the three identical-looking
|
||||
spots named (same `ForbiddenException` message, same empty 200 list, no log line) and the added
|
||||
observation that the affected user has a ready, false explanation.
|
||||
2. **Test case**: `module.guard.spec.ts` lines 137–184, two guard instances (genuinely-no-grant vs.
|
||||
query-found-nothing) held against each other, asserting identical exception messages, with a
|
||||
header comment stating the test is *supposed* to go red once a distinguishing signal is added.
|
||||
3. **WINDOWS #23**: present in both the table (line 40) and the JSON block (lines 309–319) of
|
||||
`.planning/WINDOWS.md`, `status: open`, naming the concrete Etappe-4 preflight
|
||||
(`rls-preflight.mjs`: active activation rows present but resolution empty for a known admin) and
|
||||
the reasoned rejection of a runtime warning. Table-row count (23) matches JSON-entry count (23)
|
||||
— internally consistent.
|
||||
|
||||
### 7. No safeguard that guards nothing
|
||||
|
||||
`grep -rn "P2002" apps/api/src/module-registry` returns zero hits. No unique-constraint-collision
|
||||
translation was added — consistent with Befund H/pruefung 12/13 (both affected unique keys lead
|
||||
with `tenantId`, so the `tenders`/`user` collision chain structurally cannot occur here).
|
||||
|
||||
### 8. The measurements are committed — reproduced live against the real container
|
||||
|
||||
Independently re-ran the tool against the live `tessera-ctl-db-1` container (address resolved
|
||||
fresh via `docker inspect`, `172.19.0.2` at verification time — not copied from any document):
|
||||
|
||||
```
|
||||
$ TESSERA_SCRATCH_ADMIN_URL="postgresql://tessera:tessera_dev@172.19.0.2:5432/postgres" \
|
||||
node apps/api/scripts/rls-scratch-check.mjs
|
||||
...
|
||||
Alle 66 Pruefungen bestanden.
|
||||
```
|
||||
|
||||
All 13 named module-registry checks (`modulegrant-ungebunden-null-zeilen` through
|
||||
`freigabe-eindeutigkeitsindizes-fuehren-mit-der-mandantenkennung`) passed with output byte-identical
|
||||
to what's recorded in the critique document's (m1) section. 66 = 53 baseline + 13 new — confirmed.
|
||||
|
||||
### 9. The five hand-maintained document sections — recomputed independently
|
||||
|
||||
All five confirmed by independent recomputation, not by trusting the document:
|
||||
|
||||
- **Bestandsaufnahme rows**: all five (file, model) pairs for `module-registry` present with the
|
||||
correct `Stand` (3 gebunden, 2 ungebunden) and MEASUREMENT+CONDITION-separated reasoning.
|
||||
- **Übersichtszeile**: `module-registry | 7 | 10` — matches the independently-counted raw hits.
|
||||
- **Summenzeile**: independently summed all 12 area rows — ungebunden 36+0+4+1+8+7+13+8+12+8+7+4 =
|
||||
**108**, gebunden 26+31+26+22+14+10+0+5+0+0+0+0 = **134** — matches the document's `**108**`/`**134**`
|
||||
exactly.
|
||||
- **Klassen-Verteilung**: ran an independent `awk` pass over every `apps/api/src/...` Bestandsaufnahme
|
||||
row and recomputed class counts from scratch: `muss-mandantengebunden: 31, keine-mandantengebundene-tabelle: 17,
|
||||
beides: 13, bewusst-uebergreifend: 2, TOTAL: 63` — matches the document's table and its "63 Paare"
|
||||
heading exactly.
|
||||
- **Hintergrunddienst-als-Falle**: heading says "fünf Fälle"; four are formatted as
|
||||
`- **\`file\`**` bullets (ldap.service.ts, tender-digest.scheduler.ts, tender-matching.service.ts,
|
||||
admin-seed.service.ts) and the fifth (`dkv-scheduler.service.ts`) is deliberately formatted
|
||||
differently ("anderer Bauart") — count of 5 confirmed by inspection. The section explicitly states
|
||||
`module-registry` adds **no** sixth case, with a measured reason (`seedModule` writes without
|
||||
tenant context but doesn't iterate per-tenant, so it lacks the read-across/bind-within-loop shape).
|
||||
|
||||
### 10. Falsification proofs — both present in the document, not just commit messages
|
||||
|
||||
Confirmed both are written into `docs/mandantentrennung-etappe2-fehlerrichtung.md`'s "Nachtrag
|
||||
(260910-exd, Aufgabe 3)" section (lines 1515–1533), each with the exact test name and exact failure
|
||||
message:
|
||||
|
||||
- Task 2: group-path rollback → `module-access.service.spec.ts` test "USER-Zweig bindet BEIDE..."
|
||||
went red with `expected 1 to be 2`; reverted, 23/23 green again.
|
||||
- Task 3: `deactivateForTenant` write rollback → `module-registry.service.spec.ts` test "bindet
|
||||
beide Aktivierungszugriffe..." went red with the bound-call-log assertion message quoted verbatim;
|
||||
reverted, 16/16 green again.
|
||||
|
||||
### 11. Constraints held
|
||||
|
||||
- No Prisma schema/migration changes: `git diff --name-only a2516a9..9c0eefe -- apps/api/prisma`
|
||||
→ empty.
|
||||
- No compose/env changes: none of `docker-compose*.yml`/`.env*.example` appear in the diff file list.
|
||||
- `assertTargetBelongsToTenant` still present and called twice in
|
||||
`apps/api/src/groups/module-grants.service.ts` (lines 46, 103, 242) — unchanged.
|
||||
- No `dashboard` file touched: `git diff --name-only a2516a9..9c0eefe -- apps/api/src/dashboard`
|
||||
→ empty.
|
||||
- T-JTS-02/T-JTS-03 recorded as open, not fixed, in (m4) of the critique doc, with explicit note
|
||||
that the write-side check in `module-grants.service.ts` remains the only protection until Etappe 4.
|
||||
- Full diff file list (10 files) matches exactly what the plan's `files_modified` frontmatter and
|
||||
the task-level file scopes declared — no unexpected files touched.
|
||||
|
||||
## Goal Achievement
|
||||
|
||||
### Observable Truths
|
||||
|
||||
| # | Truth | Status | Evidence |
|
||||
|---|-------|--------|----------|
|
||||
| 1 | Both stages (activation, grant) bound; no method binds one stage and leaves the other unbound; no method mixes two tenants in one resolution | ✓ VERIFIED | `module-access.service.ts`/`module-registry.service.ts` read in full; two-client bound-call-log tests pin single-tenant-per-resolution property (e.g. "Rollen-Kurzschluss waechst..." test) |
|
||||
| 2 | Catalog stays unbound with MEASURED (not assumed) reasoning, condition stated as condition | ✓ VERIFIED | Comments in both service files + classification doc rows 301/304 use the exact measurement+condition framing; live DB run confirms `module-tabelle-traegt-keinen-zeilenschutz` and `katalog-bindung-aendert-heute-nichts-an-der-ergebnismenge` |
|
||||
| 3 | Reverse error direction measured against the real delivered rule, per-path signal described | ✓ VERIFIED | (m2) signal table in critique doc covers all named paths incl. the two deliberately-unbound catalog paths as boundary |
|
||||
| 4 | Absence of a distinguishing denial signal explicitly handled, three ways | ✓ VERIFIED | Critique text (m3), `module.guard.spec.ts` test, WINDOWS #23 — all three confirmed present and consistent |
|
||||
| 5 | Follows `module-grants.service.ts` convention: one client per method named `tenantPrisma`, existing where-filters retained, app-layer check not replaced by DB | ✓ VERIFIED | Code read directly; `assertTargetBelongsToTenant` untouched; where-filters retained (e.g. `getAccessibleModuleIds` still filters `tenantId` in every query) |
|
||||
| 6 | Test coverage established BEFORE conversion; forgotten binding call goes red | ✓ VERIFIED | Two falsification proofs reproduced in the doc with exact red messages; two-client proof (not identity mock) used throughout |
|
||||
| 7 | Both false header comments corrected at the measurement | ✓ VERIFIED | `isModuleActive`/`findActiveForTenant` comments in `module-registry.service.ts` now state "Richtiggestellt... Gemessen... kein Aufrufer" with reference to TEIL 3 |
|
||||
| 8 | Baseline held: 810+ tests green, type-check clean, scratch tool all-pass, at end of every task | ✓ VERIFIED | Orchestrator confirmed 833/833 green (56 files, baseline 810/55), type-check exit 0; independently reran scratch tool live: 66/66 |
|
||||
| 9 | Switch stays off: `DATABASE_URL` on `tessera` role, schema/migrations unchanged, T-JTS-02/03 recorded not fixed | ✓ VERIFIED | No prisma/migration diff; T-JTS-02/03 explicitly recorded as open in (m4) |
|
||||
|
||||
**Score:** 9/9 truths verified, 0 present-but-behavior-unverified.
|
||||
|
||||
### Required Artifacts
|
||||
|
||||
| Artifact | Expected | Status | Details |
|
||||
|----------|----------|--------|---------|
|
||||
| `apps/api/scripts/rls-scratch-check.mjs` | 8th section, 13 named checks | ✓ VERIFIED | `runModuleRegistryAreaChecks` present, called between `runUserAreaChecks`/`runTransactionShapeMeasurement`; 66/66 pass live |
|
||||
| `docs/mandantentrennung-etappe2-fehlerrichtung.md` | `## Bereich module-registry`, (m1)-(m5) | ✓ VERIFIED | All 5 subsections present with required content; Nachtrag with both falsification proofs |
|
||||
| `apps/api/src/module-registry/module-access.service.ts` | 3 activation + 2 grant accesses bound, catalog unbound | ✓ VERIFIED | 5 bound call sites, 1 unbound, matches |
|
||||
| `apps/api/src/module-registry/module-access.service.spec.ts` | two-client proof, all cases retained | ✓ VERIFIED | 23 cases total, `__makeBoundClient` two-client log used |
|
||||
| `apps/api/src/module-registry/module.guard.spec.ts` | denial-signal absence case | ✓ VERIFIED | Present, lines 137-184 |
|
||||
| `apps/api/src/module-registry/module-registry.service.ts` | 5 activation accesses bound, 6 catalog unbound, both comments corrected | ✓ VERIFIED | Matches exactly |
|
||||
| `apps/api/src/module-registry/module-registry.service.spec.ts` | NEW file | ✓ VERIFIED | 344 lines, 16 cases, two-client proof |
|
||||
| `docs/mandantentrennung-zugriffsklassifikation.md` | 5 sections reconciled | ✓ VERIFIED | Independently recomputed, matches exactly |
|
||||
| `.planning/WINDOWS.md` | open entry, table + JSON | ✓ VERIFIED | #23 present both places, internally consistent (23=23) |
|
||||
|
||||
### Behavioral Spot-Checks
|
||||
|
||||
| Behavior | Command | Result | Status |
|
||||
|----------|---------|--------|--------|
|
||||
| Scratch tool passes against live DB | `node apps/api/scripts/rls-scratch-check.mjs` (against `172.19.0.2`) | "Alle 66 Pruefungen bestanden." | ✓ PASS |
|
||||
| `activateForTenant` binding provable | Read `module-registry.service.spec.ts` line 193 assertion | `expectBoundCall(..., 'tenantModuleActivation', 'upsert')` | ✓ PASS |
|
||||
| No forbidden literal `tenantPrisma.module.` | `grep -rn 'tenantPrisma\.module\.' apps/api/src/module-registry` | 0 hits | ✓ PASS |
|
||||
| No debt markers in modified files | `grep -nE "TBD\|FIXME\|XXX\|TODO\|HACK\|PLACEHOLDER"` across all 10 diffed files | 0 hits | ✓ PASS |
|
||||
| Constraint boundaries held | `git diff --name-only a2516a9..9c0eefe` | exactly the 10 expected files | ✓ PASS |
|
||||
|
||||
### Anti-Patterns Found
|
||||
|
||||
None. No debt markers, no stub returns, no hardcoded empty data flowing to output in the reviewed files.
|
||||
|
||||
### Minor Informational Notes (not gaps)
|
||||
|
||||
- SUMMARY.md states "module-access.service.spec.ts rebuilt... plus 7 new binding cases." Independent
|
||||
count of the file's final "Bindung an forTenant() (260910-exd)" describe block shows **6** new
|
||||
cases (lines 327, 336, 354, 370, 387, 400), matching the plan's own behavior spec (6 bullet
|
||||
points). This is a minor off-by-one in the SUMMARY narrative, not a must-have and not affecting
|
||||
any verified artifact or test outcome — recorded here for completeness, not as a gap.
|
||||
|
||||
## Requirements Coverage
|
||||
|
||||
| Requirement | Description | Status | Evidence |
|
||||
|-------------|-------------|--------|----------|
|
||||
| WINDOWS-18 | Broken-windows tracking for RLS bypass deviations | ✓ SATISFIED | WINDOWS #23 added |
|
||||
| ETAPPE-2-MODULE-REGISTRY | Bind module-registry area per Etappe 2 pattern | ✓ SATISFIED | All 17 raw accesses decided (10 bound, 7 unbound-with-reason) |
|
||||
|
||||
(These are quick-task-local requirement tags, not entries in `.planning/REQUIREMENTS.md` — expected
|
||||
for a quick task, not an orphan.)
|
||||
|
||||
## Human Verification Required
|
||||
|
||||
None. All must-haves were verifiable programmatically (source review, live DB run, independent
|
||||
recomputation of hand-maintained document sections).
|
||||
|
||||
## Gaps Summary
|
||||
|
||||
No gaps found. This is one of the most rigorously self-falsifying deliveries in this series: the
|
||||
priority-scrutiny item (the tender-scheduler identity mock) held up under adversarial re-check
|
||||
because the actual binding proof lives in the correct file with a genuine two-client log, the
|
||||
Task-2/Task-3 document split was the minimum forced edit (verified via commit-level diff, not
|
||||
narrative), the corrected catalog-binding premise is preserved as a measurement+condition pair
|
||||
everywhere it appears, all coverage counts were independently reproduced from source (not copied
|
||||
from the SUMMARY), the scratch tool was re-run live against the real database and matched the
|
||||
committed measurement byte-for-byte, and both falsification proofs are recorded in the permanent
|
||||
document with exact test names and failure messages, not left only in commit messages.
|
||||
|
||||
---
|
||||
|
||||
*Verified: 2026-09-10*
|
||||
*Verifier: Claude (gsd-verifier)*
|
||||
Reference in New Issue
Block a user