From 0ddf7f8f68ed31d3b16ccc734aa480da1200224e Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 29 Sep 2026 03:14:47 +0200 Subject: [PATCH] docs(10.1-04): complete unit tests, gate and security evidence plan --- .../10.1-04-SUMMARY.md | 302 ++++++++++++++++++ 1 file changed, 302 insertions(+) create mode 100644 .planning/phases/10.1-runtime-admin-extension-point/10.1-04-SUMMARY.md diff --git a/.planning/phases/10.1-runtime-admin-extension-point/10.1-04-SUMMARY.md b/.planning/phases/10.1-runtime-admin-extension-point/10.1-04-SUMMARY.md new file mode 100644 index 0000000..a237e5a --- /dev/null +++ b/.planning/phases/10.1-runtime-admin-extension-point/10.1-04-SUMMARY.md @@ -0,0 +1,302 @@ +--- +phase: 10.1-runtime-admin-extension-point +plan: 04 +subsystem: testing +tags: [go, testcontainers, postgres, vitest, happy-dom, cabana, boardwalk, admin-spa, gate, security-review] + +requires: + - phase: 10.1-runtime-admin-extension-point + provides: "10.1-01 widget, toolbar and partial routes, plugin asset route and sanitizer; 10.1-02 SPA WidgetField, PartialHost, pluginAssets and toolbar actions; 10.1-03 Albums stats strip, Discogs widget and sync stubs" + - phase: 10-admin-vue-spa + provides: check-phase10.sh detector and hygiene rules, cabana test harnesses (adminGorm, conform env, handlerRouter), Vitest helpers +provides: + - modules/cabana/testdata/extension acme fixture plugin (gadgets controller, header and form partials, lookup widget, JS, CSS, en/pl lang) + - TestPhase101FormExtensionSchema, TestPhase101PartialSchema, TestPhase101Toolbar, TestPhase101PartialSanitizer, TestPhase101Assets, TestPhase101Actions (cabana), TestPhase101BoardwalkExports (boardwalk) + - TestPhase101AlbumsExtension (fonoteka.go, PostgreSQL, assembled router at /plytadmin) + - Vitest suites pluginAssets, WidgetField, PartialField, PartialHost; extended ListToolbar, ListView, registry, formState, FormField, FormView + - assetAllowed refuses percent-encoded dot segments; rebuilt modules/boardwalk/dist + - scripts/check-phase10.1.sh with --self-test, --go, --security, --postgres, --spa, --openapi, --dist, --hygiene, --evidence, --all + - 10.1-SECURITY-REVIEW.md with 23 threat rows and 23 removal checks; validated 10.1-VALIDATION.md +affects: [10.1 verify-work, 11.1 documentation, 14 Discogs client] + +actuals: + tokens: 51000 + tasks: 3 + commits: 7 +plan_head_before: c3c547c394308441eea4113e055919674394a679 +plan_head_after: bbceeb957f0988ea2cb129fcbb14d1206c2a18cb +fonoteka_head_before: 9868a0a26957d4657e7084755655dd3a6f6c8f11 +fonoteka_head_after: c72c10662fb22517dbda6b72c1b8eb4837358764 + +tech-stack: + added: [] + patterns: + - "Fixture plugin trees under modules//testdata loaded with os.DirFS, copied to fstest.MapFS so each boot-error case edits one file and fails when its anchor text is missing" + - "Removal checks driven by an anchor-exact mutation harness that restores the file byte for byte; a high threat counts as mitigated only when its test fails with the protection removed" + - "Gate hygiene rules each carry their own refusal reason, and the self-test checks every plant is refused for that reason and no other" + +key-files: + created: + - modules/cabana/testdata/extension/controllers/gadgets/config_list.yaml + - modules/cabana/testdata/extension/controllers/gadgets/_stats.htm + - modules/cabana/testdata/extension/assets/js/lookup.js + - modules/cabana/phase101_schema_test.go + - modules/cabana/phase101_render_test.go + - modules/cabana/phase101_assets_test.go + - modules/cabana/phase101_actions_test.go + - ../fonoteka.go/plugins/golem15/fonoteka/admin_phase101_albums_test.go + - admin/tests/app/pluginAssets.test.ts + - admin/tests/form/WidgetField.test.ts + - admin/tests/form/PartialField.test.ts + - admin/tests/list/PartialHost.test.ts + - scripts/check-phase10.1.sh + - .planning/phases/10.1-runtime-admin-extension-point/10.1-SECURITY-REVIEW.md + modified: + - modules/boardwalk/boardwalk_test.go + - admin/src/app/pluginAssets.ts + - modules/boardwalk/dist/index.html + - admin/tests/list/ListToolbar.test.ts + - admin/tests/list/ListView.test.ts + - admin/tests/form/registry.test.ts + - admin/tests/form/formState.test.ts + - admin/tests/form/FormField.test.ts + - admin/tests/form/FormView.test.ts + - scripts/check-phase10.sh + - .planning/phases/10.1-runtime-admin-extension-point/10.1-VALIDATION.md + - .planning/phases/10-admin-vue-spa/deferred-items.md + +key-decisions: + - "A percent-encoded dot segment counts as a dot segment in assetAllowed: the URL parser resolves %2e%2e like .., which let a schema URL climb out of {base}/assets/" + - "Both gates allow-list no fonoteka failure: the two parity tests pass since fonoteka.go 21c0f12, and the detector refuses an allow-listed failure that passes" + - "Removal-check rows use their own RC-NN ids so the review's threat-row count stays one row per threat; --evidence requires an RC row naming every high mitigated threat" + - "hygiene_101 also refuses sendBeacon, WebSocket and EventSource in application plugin JS, next to the planned fetch, XMLHttpRequest, cookie and storage rules" + - "Action boot errors (reserved name, missing Run, duplicate) name the plugin and controller but no file, because actions are Go registrations; the tests assert exactly that" + +patterns-established: + - "An external-package PostgreSQL test can drive cookie transport by setting the summer_admin cookie to the Bearer JWT, which is what the cookie login stores" + - "SPA suites that touch pluginAssets use fresh asset URLs per test, because the loader remembers URLs for the lifetime of the test file" + +requirements-completed: [ADMIN-07] + +coverage: + - id: D1 + description: "Every Phase 10.1 Go path has branch-level tests: widget, partial and toolbar schema rules, the sanitizer and caps, the asset route and its boot checks, the action and partial routes on PostgreSQL, and the boardwalk exports" + requirement: ADMIN-07 + verification: + - kind: unit + ref: "modules/cabana/phase101_schema_test.go#TestPhase101FormExtensionSchema" + status: pass + - kind: unit + ref: "modules/cabana/phase101_schema_test.go#TestPhase101PartialSchema" + status: pass + - kind: unit + ref: "modules/cabana/phase101_schema_test.go#TestPhase101Toolbar" + status: pass + - kind: unit + ref: "modules/cabana/phase101_render_test.go#TestPhase101PartialSanitizer" + status: pass + - kind: unit + ref: "modules/cabana/phase101_assets_test.go#TestPhase101Assets" + status: pass + - kind: integration + ref: "modules/cabana/phase101_actions_test.go#TestPhase101Actions" + status: pass + - kind: unit + ref: "modules/boardwalk/boardwalk_test.go#TestPhase101BoardwalkExports" + status: pass + human_judgment: false + - id: D2 + description: "The three Albums surfaces proven on PostgreSQL through the assembled router: stats scoped over two collections, Discogs widget fill and save, sync toast, Genres-only 403, assets, pl/en labels, route templates in admin.json" + requirement: ADMIN-07 + verification: + - kind: integration + ref: "../fonoteka.go/plugins/golem15/fonoteka/admin_phase101_albums_test.go#TestPhase101AlbumsExtension" + status: pass + human_judgment: false + - id: D3 + description: "Every new SPA module and changed component has a Vitest suite asserting the UI-SPEC states and accessibility attributes, offline" + requirement: ADMIN-07 + verification: + - kind: automated_ui + ref: "admin/tests/app/pluginAssets.test.ts" + status: pass + - kind: automated_ui + ref: "admin/tests/form/WidgetField.test.ts" + status: pass + - kind: automated_ui + ref: "admin/tests/list/PartialHost.test.ts" + status: pass + - kind: automated_ui + ref: "admin/tests/form/PartialField.test.ts" + status: pass + - kind: automated_ui + ref: "admin/tests/list/ListView.test.ts#list view extension points" + status: pass + - kind: automated_ui + ref: "admin/tests/form/FormView.test.ts#form view extension context" + status: pass + human_judgment: false + - id: D4 + description: "One fail-closed Phase 10.1 gate whose self-test proves each detector and hygiene rule, passing end to end, with the Phase 10 gate still green" + requirement: ADMIN-07 + verification: + - kind: other + ref: "scripts/check-phase10.1.sh --self-test" + status: pass + - kind: other + ref: "scripts/check-phase10.1.sh --all" + status: pass + - kind: other + ref: "scripts/check-phase10.sh --all" + status: pass + human_judgment: false + - id: D5 + description: "Security review with a failing-when-broken test and a removal check per high threat, and a validated per-task verification map" + requirement: ADMIN-07 + verification: + - kind: other + ref: "scripts/check-phase10.1.sh --evidence" + status: pass + human_judgment: false + - id: D6 + description: "Real-browser behaviour of the extension point: CSP module loading, element upgrade, 768px layout, Vite /assets proxy" + verification: [] + human_judgment: true + rationale: "happy-dom neither enforces CSP nor loads module scripts or lays out; collected from the human-check blocks of 10.1-02 Task 3 and 10.1-03 Task 3 at /gsd-verify-work" + +duration: 42min +completed: 2026-09-29 +status: complete +--- + +# Phase 10.1 Plan 04: Unit tests, gate and security evidence Summary + +**The Phase 10.1 extension point now has full test coverage. Eight named Go tests run on an acme fixture plugin and on PostgreSQL, one assembled Albums acceptance test runs in fonoteka.go, and ten Vitest suites cover the SPA side. One fail-closed `check-phase10.1.sh --all` gate passes, and a security review records 23 removal checks. Along the way the review found and fixed a percent-encoded `..` bypass of the plugin asset URL check.** + +## Performance + +- **Duration:** 42 min +- **Started:** 2026-09-29T00:30:57Z +- **Completed:** 2026-09-29T01:13:27Z +- **Tasks:** 3 +- **Files modified:** 31 in summercms.go (plus the rebuilt dist), 1 in fonoteka.go + +## Accomplishments + +- Schema, sanitizer, asset and action tests on the `testdata/extension` fixture. The boot-error tables cover 20 widget rules, 12 partial rules and 6 toolbar rules, and each one asserts the plugin, controller and file named in the error. The sanitizer covers 19 dropped tags, the attribute table, 16 URL cases, per-request `trans` in en and pl on one compiled template, all three caps at and just past their limit, and the view-model guard both directly and through the route. +- `TestPhase101Actions` runs on PostgreSQL through the assembled router. It covers the scoped record, the fill filter in both directions, 8 malformed bodies, the action permission on top of the controller's, ValidationError versus plain error mapping, the CSRF header on both routes, and the toolbar and partial 404/422 rules. +- `TestPhase101AlbumsExtension` covers these cases: + - An empty admin collection shows `All albums 0` while a second collection holds four albums. + - "No shelf" appears only when its count is above zero. + - The widget fill of 1977/LP is saved and reloads. + - The sync toast appears in en and pl. + - A Genres-only admin gets 403 on all three routes, and each action declares its own permission. + - The assets are served with the right headers. + - Every called route template appears in admin.json. +- The Vitest suites for WidgetField (fake-timer 5000 ms timeout, `whenDefined`, attributes only, one POST while busy, fill-key-only patch), PartialHost and partialNodes (exhaustive tag and attribute tables including `javascript:`, depth and node caps, busy refetch), pluginAssets (18 refused URL shapes, retry after error, per-controller links), plus ListToolbar, ListView, registry, formState, FormField and FormView extensions. 677 tests pass offline. +- `scripts/check-phase10.1.sh`: the detector with 14 synthetic cases, plus `hygiene_101` with three named rules proven by eight plants and one clean look-alike. `--evidence` requires a removal-check row for every high threat. Both `check-phase10.1.sh --all` ("phase10.1 all passed") and `check-phase10.sh --all` ("phase10 all passed") pass. +- `10.1-SECURITY-REVIEW.md` has 23 threat rows and 23 removal checks (RC-01 to RC-23), all failing as required. `10.1-VALIDATION.md` is validated with `nyquist_compliant: true`. + +## Task Commits + +summercms.go: +1. **Task 1: Go coverage and fixture tree** - `7eed4ac` (test) +2. **Task 2 (security fix found by the new suite): percent-encoded dot segments** - `6b0ac15` (fix) +3. **Task 2: SPA Vitest suites** - `11c4e54` (test) +4. **Task 3 (blocking fix): stale parity allow-list in the Phase 10 gate** - `9aeb0e1` (fix) +5. **Task 3: Phase 10.1 gate** - `02df0a8` (feat) +6. **Task 3: security review and validation map** - `bbceeb9` (docs) + +fonoteka.go: +1. **Task 1: Albums acceptance test** - `c72c106` (test) + +Commit count: `git rev-list --count c3c547c..bbceeb9` is 6 in summercms.go, plus 1 in fonoteka.go, for 7 plan commits. + +## Files Created/Modified + +- `modules/cabana/testdata/extension/**`: the acme fixture plugin, with the gadgets controller, stats and summary partials, lookup widget, JS, CSS and lang files +- `modules/cabana/phase101_schema_test.go`: form extension, partial schema and toolbar tests, plus the shared fixture helpers +- `modules/cabana/phase101_render_test.go`: sanitizer, escaping, caps and view-model guard tests +- `modules/cabana/phase101_assets_test.go`: tests of the asset route through a mux that mirrors `service.mount`, backed by the real SPA handler +- `modules/cabana/phase101_actions_test.go`: the PostgreSQL action and partial route tests (external package) +- `modules/boardwalk/boardwalk_test.go`: `TestPhase101BoardwalkExports` +- `../fonoteka.go/plugins/golem15/fonoteka/admin_phase101_albums_test.go`: `TestPhase101AlbumsExtension` +- `admin/src/app/pluginAssets.ts`: rejects percent-encoded dot segments; `modules/boardwalk/dist` rebuilt +- `admin/tests/{app,form,list}/*.test.ts`: the new and extended suites +- `scripts/check-phase10.1.sh`: the gate. `scripts/check-phase10.sh`: allow-list emptied +- `10.1-SECURITY-REVIEW.md`, `10.1-VALIDATION.md`, and the Phase 10 `deferred-items.md` entry marked resolved + +## Decisions Made + +- The client asset check treats a segment as a dot segment after decoding `%2e`, whatever its case. Browsers resolve `%2e%2e` like `..`. +- The gates allow-list nothing. Keeping the two parity names made `check-phase10.sh --go` fail with exit 6, because both tests now pass. +- Removal-check rows are `| RC-NN | T-10.1-XX | … |`. The acceptance grep then counts exactly 23 threat rows, and `--evidence` can still tie every high threat to a removal check. +- `hygiene_101` refuses `sendBeacon`, `WebSocket` and `EventSource` alongside the planned network, cookie and storage patterns. These are the other ways plugin JS could reach the network. +- Action registration errors name the plugin and controller only. They come from Go code, not a YAML file, so the tests do not expect a file name there. + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 2 - Security] Percent-encoded dot segments bypassed `assetAllowed` (T-10.1-13)** +- **Found during:** Task 2 (the pluginAssets suite) +- **Issue:** `/admin-test/assets/%2e%2e/api/v1/x.js` passed the check, but `new URL()` resolves it to `/admin-test/api/v1/x.js`, outside the asset prefix. +- **Fix:** A segment counts as `.` or `..` after lowercasing and replacing `%2e`. Test cases were added for the encoded, mixed-case and single-dot forms. The dist was rebuilt, and `check-admin-dist.sh` passes. +- **Files modified:** admin/src/app/pluginAssets.ts, modules/boardwalk/dist/index.html, modules/boardwalk/dist/assets/index-*.js +- **Verification:** RED first (3 URL cases and the loadScript refusal failed), then 30/30 green. Full Vitest run: 677 pass. +- **Committed in:** 6b0ac15 + +**2. [Rule 3 - Blocking] Stale parity allow-list made the Phase 10 gate fail** +- **Found during:** Task 3 (`--go` stage, detector exit 6) +- **Issue:** `TestMigrateSeedsCanonicalGenres` and `TestSchemaMatchesPHPSnapshot` pass since fonoteka.go `21c0f12`. The plan said to reuse the same allow-list, and `check-phase10.sh --all`, which the plan requires to pass, refused. +- **Fix:** `KNOWN_APP_FAILURES=""` in both gates, with the header comment rewritten. The Phase 10 deferred item is marked `status: resolved`. +- **Files modified:** scripts/check-phase10.sh, scripts/check-phase10.1.sh, .planning/phases/10-admin-vue-spa/deferred-items.md +- **Verification:** `check-phase10.sh --all` prints "phase10 all passed"; `check-phase10.1.sh --all` prints "phase10.1 all passed" +- **Committed in:** 9aeb0e1, 02df0a8, bbceeb9 + +**3. [Rule 1 - Test bug] Two removal checks initially survived** +- **Found during:** Task 1 removal checks +- **Issue:** Disabling the model-type guard still returned 500, because the header template failed on the model. Removing the Discogs action permission went unnoticed, because the controller permission already refused the Genres-only admin. +- **Fix:** The guard test now renders the form partial, whose template reads `.Data.Name`, a field the model also has. The acceptance test now asserts each Discogs action's own `Permissions`. +- **Files modified:** modules/cabana/phase101_render_test.go, ../fonoteka.go/.../admin_phase101_albums_test.go +- **Verification:** RC-11 and RC-18 now fail as required +- **Committed in:** 7eed4ac, c72c106 + +--- + +**Total deviations:** 3 auto-fixed (1 security, 1 blocking, 1 test bug) +**Impact on plan:** The asset fix touches admin/src and the dist, which are outside the plan's file list. It is a one-function hardening of an existing mitigation with its own test. The allow-list change was needed for the plan's own acceptance command. No scope creep beyond that. + +## TDD Notes + +This plan is test coverage for code that plans 10.1-01 to 10.1-03 had already shipped, so most suites passed on their first run by design. Their failing-when-broken evidence comes from the 23 removal checks in the security review, not from RED commits. The one behaviour change, the encoded dot segment, followed RED then GREEN: the new cases failed on their assertions before the fix. It shipped as a single `fix` commit, because a failing test commit would have broken the green-at-every-commit rule. + +## Issues Encountered + +- `pluginAssets` keeps loaded script URLs and owned links for the lifetime of a test file. Two new ListView and FormView tests got cached promises until they switched to fresh URLs. +- The admin cookie is only set by a login that sends `X-Requested-With`. The external cabana test uses the Bearer JWT as the `summer_admin` value, which is what that login stores. +- `admin/src/api/schema.d.ts` changed in Phase 10.1, but no app/form/list suite imports it. It is generated and type-only, and the Phase 10 hygiene rule exempts it too. `tests/fixtures/typed.ts` imports it. + +## Known Stubs + +None added. The Discogs stubs from 10.1-03 are unchanged and still tracked in `.planning/WINDOWS.md` (Phase 14). + +## User Setup Required + +None. No external service configuration required. + +## Next Phase Readiness + +- Phase 10.1 is complete in code and evidence. `/gsd-verify-work 10.1` should collect the real-browser human checks from 10.1-02 Task 3 and 10.1-03 Task 3. These are D6 above: CSP module loading, element upgrade, the 768px layout, and the Vite `/assets` proxy. +- `scripts/check-phase10.1.sh --all` is the phase acceptance command. It runs `npm ci` and every PostgreSQL suite in both repositories, and takes several minutes. + +## Self-Check: PASSED + +- FOUND: modules/cabana/phase101_{schema,render,assets,actions}_test.go, modules/cabana/testdata/extension/assets/js/lookup.js, modules/cabana/testdata/extension/controllers/gadgets/_stats.htm, ../fonoteka.go/plugins/golem15/fonoteka/admin_phase101_albums_test.go, admin/tests/app/pluginAssets.test.ts, admin/tests/form/WidgetField.test.ts, admin/tests/form/PartialField.test.ts, admin/tests/list/PartialHost.test.ts, scripts/check-phase10.1.sh (mode 100755), 10.1-SECURITY-REVIEW.md +- FOUND commits: 7eed4ac, 6b0ac15, 11c4e54, 9aeb0e1, 02df0a8, bbceeb9 (summercms.go); c72c106 (fonoteka.go) +- Acceptance: review threat rows 23; `nyquist_compliant: true` 1; pending table rows 0; ADMIN-07 mentions 25; `whenDefined|5000` in WidgetField.test.ts 3; `javascript:` in PartialHost.test.ts 3; admin/package.json and package-lock.json unchanged since b2845e0 +- Plan verification: `scripts/check-phase10.1.sh --all` ("phase10.1 all passed") and `scripts/check-phase10.sh --all` ("phase10 all passed") + +--- +*Phase: 10.1-runtime-admin-extension-point* +*Completed: 2026-09-29*