diff --git a/.planning/REQUIREMENTS.md b/.planning/REQUIREMENTS.md index 9b715c1..84b9093 100644 --- a/.planning/REQUIREMENTS.md +++ b/.planning/REQUIREMENTS.md @@ -75,11 +75,11 @@ Requirements for v1 (the Płytarium port). Each maps to roadmap phases. "User" b - [x] **API-01**: Collections: CRUD, active-context switch (me/context flags plus the opaque channel name from realtime/channels), editor invitations and acceptance, owner-only share link show/update/regenerate - [x] **API-02**: Albums: CRUD, ratings, photo upload and manual cover URL, Discogs cover import (cover_urls), artists/genres/styles lookups, search that treats Typesense as a pre-filter with both the items and the total re-gated in SQL -- [ ] **API-03**: Wishlist: items, subscriptions, public-wishlist/{token} views, album reservations (reserve/reveal), purchase and digest triggers (the wishlist Discogs match/apply-release routes are Phase 14, with INTG-01) -- [ ] **API-04**: Notifications: list, unread count, mark read (one and all); pruning is the Phase 14 `fonoteka:prune-notifications` console command; realtime token endpoint owned by the websockets plugin +- [x] **API-03**: Wishlist: items, subscriptions, public-wishlist/{token} views, album reservations (reserve/reveal), purchase and digest triggers (the wishlist Discogs match/apply-release routes are Phase 14, with INTG-01) +- [x] **API-04**: Notifications: list, unread count, mark read (one and all); pruning is the Phase 14 `fonoteka:prune-notifications` console command; realtime token endpoint owned by the websockets plugin - [x] **API-05**: CSV import as a multi-step session (store, show/poll, mapping patch, per-row edit, commit, cancel) and CSV export on both authenticated groups -- [ ] **API-06**: Per-user and per-org Discogs and AI credentials CRUD with encrypted storage, org-lock flag, and env-to-org-to-user resolution (the live ai-credential/test and discogs-credential/test routes are Phase 14, with INTG-01 and INTG-02) -- [ ] **API-07**: Onboarding, public and invitation inspection routes, including the anonymous collection public-token views (public/{token}, its albums and album detail), with their public rate-limit buckets +- [x] **API-06**: Per-user and per-org Discogs and AI credentials CRUD with encrypted storage, org-lock flag, and env-to-org-to-user resolution (the live ai-credential/test and discogs-credential/test routes are Phase 14, with INTG-01 and INTG-02) +- [x] **API-07**: Onboarding, public and invitation inspection routes, including the anonymous collection public-token views (public/{token}, its albums and album detail), with their public rate-limit buckets - [ ] **API-08**: Feedback submissions and sitemap output from the stack plugins - [ ] **API-09**: All 154 routes are registered on the correct groups with identical paths, methods, status codes and bodies @@ -214,11 +214,11 @@ Which phases cover which requirements. Updated during roadmap creation. | AUTH-08 | Phase 9 | Complete | | API-01 | Phase 12 | Complete | | API-02 | Phase 12 | Complete | -| API-03 | Phase 13 | Pending | -| API-04 | Phase 13 | Pending | +| API-03 | Phase 13 | Complete | +| API-04 | Phase 13 | Complete | | API-05 | Phase 13 | Complete | -| API-06 | Phase 13 | Pending | -| API-07 | Phase 13 | Pending | +| API-06 | Phase 13 | Complete | +| API-07 | Phase 13 | Complete | | API-08 | Phase 14 | Pending | | API-09 | Phase 15 | Pending | | JOBS-01 | Phase 11 | Gaps Found | diff --git a/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/13-SECURITY-REVIEW.md b/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/13-SECURITY-REVIEW.md new file mode 100644 index 0000000..8a9709b --- /dev/null +++ b/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/13-SECURITY-REVIEW.md @@ -0,0 +1,111 @@ +--- +phase: "13" +reviewed: "2026-10-03" +reviewer: "gsd-executor, plan 13-06 (self-performed code-and-test review of plans 13-01 to 13-06; no reviewer agent was spawned, per the 08-10 precedent)" +threats_open: 0 +gate: "scripts/check-phase13.sh --all" +removal_harness: "scripts/check-phase13.sh --removal" +--- + +# Phase 13 Security Review + +This is a code-and-test review of every threat in the registers of Plans 13-01 to 13-06: every id of the form T-13- (T-13-01 to T-13-36) and T-13-SC. Severity and disposition are copied from the originating plan. T-13-SC is declared by every plan and is listed once with the strictest entry: 13-04, medium, mitigate (the direct `golang.org/x/text` requirement). The executor performed the review itself, as plan 08-10 did, because no separate reviewer agent was spawned. + +A high threat counts as mitigated only when its named test fails with the protection removed. `scripts/check-phase13.sh --removal` does this for every high mitigated threat, and for several medium ones: +- it refuses a file with uncommitted changes; +- it applies an anchor-exact mutation that removes the protection; +- it runs the named test and requires it to fail on an assertion (a build failure does not count); +- it restores the file byte for byte, checked with `cmp`. + +The results are under "Removal checks". The two accepted threats keep their rationale from their originating plans. + +The review found one defect in Phase 13 code, fixed in plan 13-06 with a failing-when-broken test (see "Fixes made during the review"): the CSV export dropped a database error that struck after its headers were sent, with no log line. It also fixed one stale test: the Phase 9 admin route inventory, which predated the Phase 12.2 cabana routes, all of which carry the backend guard. + +Commands run from `summercms.go`; `../fonoteka.go` tests run inside that repository. Gate stages are modes of `scripts/check-phase13.sh`. `TestPhase13Threats/T-13-NN` is the subtest of `../fonoteka.go/plugins/golem15/fonoteka/phase13_security_test.go` for that threat. + +| Threat | Category | Component | Severity | Disposition | Production mitigation | Test or gate stage | Observed result | Residual risk | +|--------|----------|-----------|----------|-------------|-----------------------|--------------------|-----------------|---------------| +| T-13-01 | Information Disclosure | token guessing / enumeration | high | mitigate | `classes/public_share.go` `PubfailCounter.Begin`: 10 failed resolutions per client address per 60 s, shared by all six public routes of both kinds, checked before any query, the slot reserved so concurrent failures cannot overshoot; the 16-symbol shape check before any lookup; inline `throttle:10,1` on the resolves and the per-token and per-IP buckets on the albums routes | `TestPhase13Threats/T-13-01` (ten failures through the albums routes lock the address; a valid token from it is then 429, another address 200), `TestPubfailCounter`, `TestPublicBucketsPerRoute`, `TestRouteTablePhase13`, `TestFonotekaNuxtFlows` (public-pubfail) | pass; removal checks RC-08 (Begin never throttles) and RC-09 (TooMany always false) fail | The counter is per process (13-05 decision): N instances behind a balancer allow N x 10 guesses per minute; a 16-symbol token has about 95 bits | +| T-13-02 | Information Disclosure | public serializer and facets | high | mitigate | `classes/serialize_public_album.go` `PublicAlbumDTO` is its own twelve-key type; the public search drops rating and price sorts, refuses the rating filter with 422, searches only the public text fields and re-gates engine ids to the shared collection; zero-count facets are dropped; no reservation is loaded | `TestPhase13Threats/T-13-02` (exactly the twelve keys in PHP order, none of shelf, notes, barcode, Discogs id, price, condition, reservation or rating; rating filter 422), `TestPublicAlbumFieldSet`, `TestPublicAlbumsIndex`, `TestPublicAlbumsEngine` | pass; removal check RC-20 (the DTO gains `shelf`) fails | None known | +| T-13-03 | Spoofing | disabled or regenerated share | high | mitigate | `classes/share_service.go` `ResolvePublic` requires `public_enabled`, the route's kind and the exact current token | `TestPhase13Threats/T-13-03` (a disabled share and a regenerated share's old token answer the public 404; the new token 200), `TestPublicResolve`, `TestFonotekaNuxtFlows` (public-anonymous) | pass; removal check RC-06 (`public_enabled` dropped from the lookup) fails | None known | +| T-13-04 | Elevation of Privilege | reserve, cancel, reveal | high | mitigate | `classes/reservations.go` `ReserveAlbum` refuses the owner (422) and a disabled wishlist (409) and locks the album row; `CancelReservation` deletes only the caller's own row; reveal looks the album up in the caller's own wishlist | `TestPhase13Threats/T-13-04` (a peer reserves; the owner cannot; another peer cannot cancel; the reserver cannot reveal; an outsider cannot reach the album; the reservation is unchanged), `TestReserveConcurrent`, `TestRevealIdempotent` | pass; removal check RC-23 (cancel without the user condition) fails | None known | +| T-13-05 | Information Disclosure | reservation mask in every serializer path | high | mitigate | `ReservationStateForViewer` answers the owner `reserved`, `is_mine:false`, `revealed:false` and no `reserved_by` until the reveal, on show, index and peer routes | `TestPhase13Threats/T-13-05` (owner show and index carry neither `reserved_by` nor the reserver's name; the reserver sees herself), `TestReservationMask`, `TestPhase13ReservationMask`, `TestWishlistOwnListAndShow` | pass; removal check RC-05 fails | None known | +| T-13-06 | Information Disclosure / Tampering | peer wishlists, subscriptions, wishlist album ids | high | mitigate | `ReservableWishlistIDs`, `WishlistsVisibleTo` and `ActiveWishlist` scope every lookup; a foreign and a missing id answer the same Winter 404 page | `TestPhase13Threats/T-13-06` (an outsider's peer albums, peer album, collection subscribe and own-wishlist album lookups are identical to a missing id; no subscription is written), `TestWishlistSubscriptions` | pass; removal check RC-24 (the visible-wishlist scope dropped from collection subscriptions) fails | None known | +| T-13-07 | Elevation of Privilege | token group | high | mitigate | `routes.go` mounts on `/api/v1/fonoteka` only the four wishlist CRUD routes and the export, each with exactly one `inv.scope` | `TestRouteTablePhase13` (58 routes on routes.php's groups with their lines, one scope per token route, Phase 14 routes absent), `TestPhase13Threats/T-13-07` (social, credential, notification, import and public paths unmounted for a token; a read-only token cannot write) | pass; removal check RC-22 (a second `inv.scope:read` on the token wishlist list) fails | None known | +| T-13-08 | Information Disclosure | credential responses, logs, marshal | high | mitigate | `lagoon.Encrypted` columns with `json:"-"` models; show selects provider, model and base_url only; nothing logs a secret | `TestPhase13Threats/T-13-08` (store and show of AI, organisation AI and Discogs credentials plus me/context: no secret in any body, in the captured logs or in the stored columns), `TestCredentialSecretsNeverSerialized` | pass; removal check RC-21 (show echoes the key) fails | Whoever holds the app key can decrypt stored secrets | +| T-13-09 | Elevation of Privilege | org credential store/destroy, Discogs shared | high | mitigate | `controllers/api/credentials_controller.go` `mayManageOrg` (after provisioning an org-less caller) refuses with 403 `{"error":"Forbidden"}` or `shared_forbidden`, writing nothing | `TestPhase13Threats/T-13-09` (a plain member's store, delete and shared Discogs store are 403 and leave the owner's key and no Discogs mirror), `TestCredentialsCRUD`, `TestDiscogsSharedMirror` | pass; removal check RC-25 (store without the manage check) fails | None known | +| T-13-10 | Tampering | credential owner FK mass assignment | high | mitigate | `classes/credential_write_service.go` fills only `CredentialFillFields` (provider, model, base_url); the owner keys come from the session on create and update | `FuzzWriteEndpoints` (the 29 Phase 13 write routes beside the 41 of Phase 12, every server-owned key hostile, Postgres snapshots), `TestPhase13Threats/T-13-10` (create and update with a hostile `user_id` and `organisation_id`) | pass; removal checks RC-15 (FuzzWriteEndpoints) and RC-16 (T-13-10) fail with `user_id` in the fill list | None known | +| T-13-11 | Tampering | base_url as SSRF target | medium | accept | Phase 13 only stores `base_url` (`nullable|url`); no outbound call exists until Phase 14, which owns the guarded client (INTG-02). | none (accepted) | accepted | Phase 14 must route the AI client through fetchguard | +| T-13-12 | Elevation of Privilege | import routes by id | high | mitigate | `classes/csv_import_service.go` `CsvImportFor` requires `user_id` = caller and an accessible collection | `TestPhase13Threats/T-13-12` (an own import on an unreachable collection and another user's import answer the missing-id body; cancel too), `TestCsvImportScope` | pass; removal check RC-13 (the accessible-collection check dropped) fails | None known | +| T-13-13 | Information Disclosure | CSV upload storage | high | mitigate | `controllers/api/csv_import_controller.go` writes to the private bucket from `golem15.fonoteka.csv.bucket_url` under server keys `fonoteka-csv//.csv`, never the public uploads bucket | `TestPhase13Threats/T-13-13` (a `../../evil.csv` upload lands under the user's prefix in the private directory and not in the public bucket), `TestCsvStoreAndShow` | pass; removal check RC-26 (the upload bucket used instead) fails | None known | +| T-13-14 | Tampering | exported cells | medium | mitigate | `classes/csv` `FormulaSafe` prefixes an apostrophe to cells PHP's `FORMULA_PATTERN` flags; since 13-06 a failure after the headers is logged | `TestPhase13Threats/T-13-14`, `TestCsvExport`, `TestPHPFputcsv`, `TestPhase13HandlersFailClosed` (export-stream-failure) | pass | A failure mid-stream can only cut the file short, as in PHP | +| T-13-15 | Denial of Service | CSV parse | medium | mitigate | 5 MiB cap, 5000 rows, NUL rejection, `throttle:10,1` on store | `TestPhase13Threats/T-13-15` (NUL and 5001 rows refused, no import left), `TestPhase13Boundaries` (5242880 vs 5242881 bytes, 5000 vs 5001 rows), `TestCsvParserTruthTable` | pass | None known | +| T-13-16 | Tampering | row edit Discogs pick | high | mitigate | Candidate allow-list; the Phase 13 `ReleaseFetcher` always fails, so a pick answers `discogs_unavailable` and writes nothing | `TestPhase13Threats/T-13-16` (with Discogs allowed for the caller, a candidate pick is 422 and the row unchanged), `TestCsvRowPickSeam` | pass; removal check RC-18 (the fetcher returns data) fails | Phase 14 installs the real fetcher | +| T-13-17 | Tampering | double commit / double writer | high | mitigate | `CommitCsvImport` is one `UPDATE ... WHERE status = 'preview'`; only the winner dispatches; a replay answers the existing job | `TestPhase13Threats/T-13-17` (two commits answer one job id; one import job exists), `TestCsvCommitCAS` | pass; removal check RC-14 (the status condition dropped) fails | None known | +| T-13-18 | Elevation of Privilege | onboarding bootstrap | high | mitigate | `classes/onboarding.go` `BootstrapOwner`: 409 before validation when any live user exists, then an advisory transaction lock and a recount under it; users.email unique | `TestPhase13Threats/T-13-18` (a bootstrap waiting on the held lock answers 409 and writes nothing once a user appears), `TestBootstrapConcurrent` | pass; removal check RC-12 (the in-transaction recount dropped) fails | None known | +| T-13-19 | Spoofing | register listener invitation match | high | mitigate | `HandleRegisterEvent` holds a registrant only for a pending, unexpired, unrevoked, unaccepted invitation whose trimmed lowercased e-mail matches theirs | `TestPhase13Threats/T-13-19` (pending held; expired, revoked, accepted and another address not), `TestRegisterInvitationListener` | pass; removal check RC-27 (the e-mail condition made always true) fails | None known | +| T-13-20 | Information Disclosure | RegisterEvent.Payload | high | mitigate | sm-user-plugin `controllers/registration.go` `FireRegisterEvent` deletes `password` and `password_confirmation` from its copy of the input | `TestRegisterEventPayload`, `TestRegisterUserExports`, `TestPhase13Threats/T-13-20` (the bootstrap's register event carries the e-mail and no password key or value) | pass; removal checks RC-10 (TestRegisterEventPayload) and RC-11 (TestPhase13Threats/T-13-20) fail without the confirmation delete | None known | +| T-13-21 | Information Disclosure / Tampering | notifications read and mark-read | medium | mitigate | `classes/notifications.go` scopes every query by `user_id`; zero updated rows is the Winter 404 page | `TestPhase13Threats/T-13-21` (another user's mark-read is 404 and leaves the row unread; read-all and the list never touch it), `TestNotificationsRoutes`, `TestPhase13Boundaries` (the 50-row cap) | pass; removal check RC-17 (mark-read without the user condition) fails | None known | +| T-13-22 | Denial of Service / Repudiation | conga unregistered kinds | high | mitigate | `modules/conga/conga.go` `clientFor` sends unregistered kinds through the insert-only client and, while a worker runs, refuses a queue it serves | `TestUnregisteredKindWithWorker`, `TestUnregisteredKindRefusalAndDelay`, `TestJobContractDispatchWhileWorkerRuns` | pass; removal check RC-04 (unregistered kinds through the worker client) fails | None known | +| T-13-23 | Elevation of Privilege | surf overlap dispatch | high | mitigate | `modules/surf/overlap.go` tries family members in registration order on their literals and constraints; each member runs its own wrapped chain; no match is 404 | `TestOverlappingConstrainedRoutes`, `TestOverlapConstraintFallsThrough`, `TestRouteTablePhase13` (the four routes.php pairs through the real handlers, 404 and 405), `TestPhase13Threats/T-13-23` (a constraint miss is the router's 404 before any member's guard) | pass; removal checks RC-01 (constraints skipped), RC-02 and RC-03 (literals skipped) fail | None known | +| T-13-24 | Repudiation | tide date and publication masks | medium | mitigate | Each mask checks the masked value's shape and leaves every other path visible | `TestNormalizeContentDispositionDate`, `TestNormalizeNotificationPublication`, `TestNormalizePhase13Edges` (quoted and RFC 5987 names, three dates, a changed date count, a captured id beside the masked one) | pass | None known | +| T-13-25 | Tampering | lagoon prohibited rule | medium | mitigate | Laravel 9 semantics (`!validateRequired`), not implicit | `TestValidateRequestProhibited`, `TestValidateRequestProhibitedNested` (wildcards, dotted paths, after bail), `FuzzWriteEndpoints` (wishlist writes never persist condition or shelf) | pass | None known | +| T-13-26 | Information Disclosure | php_parity.sh rows and share capture | medium | mitigate | `rows` accepts one read-only SELECT under sqlite3 `-readonly -safe`; share tokens are captured as `{{share:wishlist}}` | `TestCheckCorpusPortedCaseStatus`; stage `check-phase13.sh --parity` (`check_corpus --require-recorded --check-secrets`) | pass | None known | +| T-13-27 | Information Disclosure | invitation inspection | medium | mitigate | `InspectInvitation` reveals the collection name only for the sha256 of a pending invitation's exact token | `TestPhase13Threats/T-13-27` (expired, upper-cased, truncated and hashed tokens answer unavailable), `TestInspectInvitation` | pass | Shares the anonymous `throttle:10,1` budget (T-13-32) | +| T-13-28 | Tampering / Repudiation | item-added and purchase side effects | medium | mitigate | Bell rows, the digest upsert (`RETURNING xmax = 0`, dispatch on insert only) and mail enqueues run on the write transaction; publications after commit | `TestPhase13Threats/T-13-28` (three items, one digest row at 3 and one digest job per subscriber), `TestDigestCoalescing`, `TestPurchaseMailAfterCommit` | pass; removal check RC-19 (dispatch on every upsert) fails | None known | +| T-13-29 | Information Disclosure | subscribe by token | medium | mitigate | `ResolvePublic` with kind wishlist and a constant-time exact token compare after the case-insensitive lookup | `TestPhase13Threats/T-13-29` (a case-flipped, a disabled and a regenerated token are 404 and write nothing; the current token 201), `TestWishlistSubscriptions` | pass; removal check RC-07 (the compare made case-insensitive) fails | None known | +| T-13-30 | Information Disclosure | purchase mail job args and logs | medium | mitigate | `WishlistPurchasedMailArgs` holds the subscriber id and two names; the worker logs the id only | `TestPhase13Threats/T-13-30` (args keys exactly album_name, subscriber_id, wishlist_name; no address), `TestPurchaseSideEffects` | pass | None known | +| T-13-31 | Repudiation | queued jobs without workers | medium | mitigate | The 13-01 contract's unserved queues; cancel stops summer_jobs rows and River jobs | `TestPhase13Threats/T-13-31` (match and import jobs available on their queues, unattempted; both cancelled), `TestCsvJobRows`, `TestCsvCancel` | pass | Phase 14 registers the workers | +| T-13-32 | Denial of Service | shared anonymous inline budget | low | accept | PHP shares one `throttle:10,1` guest key across onboarding, inspection and public resolves; Go mirrors it (`inline:domainless|ClientIP`) for parity; documented in parity/README.md. | none (accepted) | accepted | One address's onboarding traffic counts against its public resolves | +| T-13-33 | Elevation of Privilege | public route kind confusion | medium | mitigate | Every public handler passes its route's kind to `ResolvePublic` | `TestPhase13Threats/T-13-33` (a wishlist token on `public/` and a collection token on `public-wishlist/` answer 404; each on its own route 200), `TestPublicResolve`, `TestPublicAlbumsIndex` | pass | None known | +| T-13-34 | Tampering | gate --removal leaving mutated source | medium | mitigate | `check-phase13.sh --removal` refuses a dirty target file, mutates by exact anchor, restores in a `finally` and on SIGINT/SIGTERM, and checks the restore with `cmp`; it is not part of `--all` | `check-phase13.sh --self-test` (dirty file, non-unique anchor, build failure, surviving mutation, byte-identical restore) | pass; removal check RC-31 (the dirty-file refusal disabled) fails the self-test | None known | +| T-13-35 | Repudiation | security review claims without evidence | medium | mitigate | `check-phase13.sh --evidence` refuses a threat without one review row copying its strictest severity and disposition, a mitigated threat naming a test the `--named` stage does not run, and a high mitigated threat without a removal row | `check-phase13.sh --self-test` (missing row, wrong disposition, missing removal row, unrun test, pending row, missing Wave 0 flag, unnamed validation test) | pass; removal check RC-30 (the removal-row requirement disabled) fails the self-test | None known | +| T-13-36 | Information Disclosure | fuzz seed corpus | low | mitigate | The 70 seeds hold synthetic values only; the gate scans `testdata/fuzz` for 64-hex values, `inv_` tokens, JWTs and bearer headers | `check-phase13.sh --parity` (corpus scan), `check-phase13.sh --self-test` (each planted shape refused) | pass; removal check RC-29 (the scan never fails) fails the self-test | None known | +| T-13-SC | Tampering | package installs (golang.org/x/text v0.42.0) | medium | mitigate | `golang.org/x/text` is a direct requirement of the plugin module at v0.42.0, already in the graph through go-i18n, pinned by go.sum; plans 13-01 to 13-03, 13-05 and 13-06 add no dependency | `check-phase13.sh --go` (module pin), `check-phase13.sh --self-test` | pass; removal check RC-28 (the pin lookup replaced by a fixed line) fails the self-test | None known | + +## Removal checks + +Each row is one anchor-exact mutation from `scripts/check-phase13.sh --removal`. The anchor occurs exactly once in the file. The test must fail on an assertion, not a build failure. The file is restored byte for byte and checked with `cmp`. The script rows mutate a copy of the gate and run its `--self-test`. The run of 2026-10-03 passed every row (31 of 31). + +| Check | Threat | File | Anchor removed or changed | Replacement | Test run | Observed | +|-------|--------|------|---------------------------|-------------|----------|----------| +| RC-01 | T-13-23 | `modules/surf/overlap.go` | the constraint check in `familyDispatch.ServeHTTP` | removed | `go test ./modules/surf -run '^TestOverlapConstraintFallsThrough$'` | fails: TestOverlapConstraintFallsThrough; restored, cmp ok | +| RC-02 | T-13-23 | `modules/surf/overlap.go` | the literal check in `familyMember.pathMatches` | `if false && ...` | `go test ./modules/surf -run '^TestOverlappingConstrainedRoutes$'` | fails: TestOverlappingConstrainedRoutes, TestOverlappingConstrainedRoutes/dispatch, TestOverlappingConstrainedRoutes/constraint-404 (+more); restored, cmp ok | +| RC-03 | T-13-23 | `modules/surf/overlap.go` | the literal check in `familyMember.pathMatches` | `if false && ...` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestRouteTablePhase13$'` | fails: TestRouteTablePhase13, TestRouteTablePhase13/numeric-path-parameters, TestRouteTablePhase13/phase14-routes-answer-404 (+more); restored, cmp ok | +| RC-04 | T-13-22 | `modules/conga/conga.go` | the insert-only client for unregistered kinds in `clientFor` | `return m.insertClient()` (the worker client) | `go test ./modules/conga -run '^TestUnregisteredKindWithWorker$'` | fails: TestUnregisteredKindWithWorker, TestUnregisteredKindWithWorker/dispatch-unregistered, TestUnregisteredKindWithWorker/enqueue-delayed; restored, cmp ok | +| RC-05 | T-13-05 | `../fonoteka.go/plugins/golem15/fonoteka/classes/reservations.go` | `if rc.IsOwner && !revealed {` in `ReservationStateForViewer` | `if false && ...` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-05$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-05; restored, cmp ok | +| RC-06 | T-13-03 | `../fonoteka.go/plugins/golem15/fonoteka/classes/share_service.go` | `public_enabled = ?` in `ResolvePublic` | the bound `true` alone | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-03$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-03; restored, cmp ok | +| RC-07 | T-13-29 | `../fonoteka.go/plugins/golem15/fonoteka/classes/share_service.go` | the exact constant-time compare in `ResolvePublic` | a compare of the lower-cased tokens | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-29$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-29; restored, cmp ok | +| RC-08 | T-13-01 | `../fonoteka.go/plugins/golem15/fonoteka/classes/public_share.go` | the limit check in `PubfailCounter.Begin` | `if false && ...` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-01$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-01; restored, cmp ok | +| RC-09 | T-13-01 | `../fonoteka.go/plugins/golem15/fonoteka/classes/public_share.go` | `too := w.hits+w.inflight >= c.limit` in `TooMany` | `false && ...` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPubfailCounter$'` | fails: TestPubfailCounter, TestPubfailCounter/boundary, TestPubfailCounter/window (+more); restored, cmp ok | +| RC-10 | T-13-20 | `../fonoteka.go/plugins/golem15/user/controllers/registration.go` | `delete(payload, "password_confirmation")` in `FireRegisterEvent` | removed | `go -C ../fonoteka.go test ./plugins/golem15/user -run '^TestRegisterEventPayload$'` | fails: TestRegisterEventPayload; restored, cmp ok | +| RC-11 | T-13-20 | `../fonoteka.go/plugins/golem15/user/controllers/registration.go` | `delete(payload, "password_confirmation")` in `FireRegisterEvent` | removed | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-20$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-20; restored, cmp ok | +| RC-12 | T-13-18 | `../fonoteka.go/plugins/golem15/fonoteka/classes/onboarding.go` | the recount under the advisory lock in `BootstrapOwner` | removed | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-18$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-18; restored, cmp ok | +| RC-13 | T-13-12 | `../fonoteka.go/plugins/golem15/fonoteka/classes/csv_import_service.go` | the accessible-collection check in `CsvImportFor` | removed | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-12$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-12; restored, cmp ok | +| RC-14 | T-13-17 | `../fonoteka.go/plugins/golem15/fonoteka/classes/csv_import_service.go` | `WHERE id = ? AND status = ?` in the commit compare-and-swap | the status condition dropped | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-17$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-17; restored, cmp ok | +| RC-15 | T-13-10 | `../fonoteka.go/plugins/golem15/fonoteka/classes/credential_write_service.go` | `CredentialFillFields` | plus `"user_id"` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^FuzzWriteEndpoints$'` | fails: FuzzWriteEndpoints, FuzzWriteEndpoints/seed#22; restored, cmp ok | +| RC-16 | T-13-10 | `../fonoteka.go/plugins/golem15/fonoteka/classes/credential_write_service.go` | `CredentialFillFields` | plus `"user_id"` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-10$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-10; restored, cmp ok | +| RC-17 | T-13-21 | `../fonoteka.go/plugins/golem15/fonoteka/classes/notifications.go` | `WHERE user_id = ? AND id = ?` in `MarkNotificationRead` | the user condition dropped | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-21$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-21; restored, cmp ok | +| RC-18 | T-13-16 | `../fonoteka.go/plugins/golem15/fonoteka/classes/csv_import_service.go` | `return nil, ErrDiscogsUnavailable` in the Phase 13 `ReleaseFetcher` | returns a draft | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-16$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-16; restored, cmp ok | +| RC-19 | T-13-28 | `../fonoteka.go/plugins/golem15/fonoteka/classes/wishlist_notifications.go` | `if len(inserted) != 1 \|\| !inserted[0] {` in `EnqueueWishlistDigest` | dispatch on every upsert | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-28$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-28; restored, cmp ok | +| RC-20 | T-13-02 | `../fonoteka.go/plugins/golem15/fonoteka/classes/serialize_public_album.go` | the end of `PublicAlbumDTO` | plus a `shelf` field | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-02$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-02; restored, cmp ok | +| RC-21 | T-13-08 | `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/credentials_controller.go` | the column select and status body of `AiCredentialShow` | a body echoing the decrypted key | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-08$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-08; restored, cmp ok | +| RC-22 | T-13-07 | `../fonoteka.go/plugins/golem15/fonoteka/routes.go` | `g.Get("/wishlist/albums", wishlistIndex, "inv.scope:read")` on the token group | a second `inv.scope:read` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestRouteTablePhase13$'` | fails: TestRouteTablePhase13, TestRouteTablePhase13/group-middleware-and-scopes; restored, cmp ok | +| RC-23 | T-13-04 | `../fonoteka.go/plugins/golem15/fonoteka/classes/reservations.go` | `WHERE album_id = ? AND user_id = ?` in `CancelReservation` | the user condition dropped | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-04$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-04; restored, cmp ok | +| RC-24 | T-13-06 | `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/wishlist_subscriptions_controller.go` | `Scopes(classes.WishlistsVisibleTo(user.ID))` in `visibleWishlist` | any wishlist | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-06$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-06; restored, cmp ok | +| RC-25 | T-13-09 | `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/credentials_controller.go` | the `mayManageOrg` check in `OrgAiCredentialStore` | removed | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-09$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-09; restored, cmp ok | +| RC-26 | T-13-13 | `../fonoteka.go/plugins/golem15/fonoteka/controllers/api/csv_import_controller.go` | `csvBucket(app)` in `CsvImportStore` | the public uploads bucket | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-13$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-13; restored, cmp ok | +| RC-27 | T-13-19 | `../fonoteka.go/plugins/golem15/fonoteka/classes/onboarding.go` | `LOWER(email) = ?` in `HandleRegisterEvent` | `(LOWER(email) = ? OR TRUE)` | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^TestPhase13Threats$/^T-13-19$'` | fails: TestPhase13Threats, TestPhase13Threats/T-13-19; restored, cmp ok | +| RC-28 | T-13-SC | `scripts/check-phase13.sh` (mutated copy) | the `golang.org/x/text` lookup in `module_pin` | a fixed audited line | `bash --self-test` | fails: "refuse: self-test module_pin accepted golang.org/x/text v0.41.0"; original untouched | +| RC-29 | T-13-36 | `scripts/check-phase13.sh` (mutated copy) | `sys.exit(1)` after a corpus secret is found | `pass` | `bash --self-test` | fails: "refuse: self-test corpus_scan accepted a planted secret aaaaaaaaaaaa"; original untouched | +| RC-30 | T-13-35 | `scripts/check-phase13.sh` (mutated copy) | the removal-row requirement of high threats in `evidence_check` | `if False:` | `bash --self-test` | fails: "refuse: self-test evidence_check accepted the removal plant"; original untouched | +| RC-31 | T-13-34 | `scripts/check-phase13.sh` (mutated copy) | the dirty-file refusal of `removal_harness` | `if False:` | `bash --self-test` | fails: "refuse: self-test removal harness mutated a dirty file"; original untouched | + +Not every protection is removable on its own. Skipping the constraint check in surf's family dispatcher does not change any routes.php pair: each member's own handler is wrapped with its `Where` constraints too, and the routes.php pairs already differ in their literal segments. RC-01 is therefore caught by `TestOverlapConstraintFallsThrough`, a family where an earlier member matches the literals but not its constraint. The application-level checks (RC-02, RC-03) skip the literal match instead. Disabling only `PubfailCounter.TooMany` leaves the handlers locked, because they use `Begin`; RC-08 removes the check in `Begin` and RC-09 pins `TooMany` through its unit test. + +## Fixes made during the review + +| Defect | Threat | Fix | Failing-when-broken test | Commit | +|--------|--------|-----|--------------------------|--------| +| The CSV export, which streams like PHP's download, dropped a database error that struck after the BOM and the header row: the client got a short file with 200 and no log line, where Laravel reports the exception | T-13-14 (export component) | `CsvExport` logs the error with the user id; the status cannot change once the headers are sent, as in PHP | `TestPhase13HandlersFailClosed` (export-stream-failure: RED with no log line) | fonoteka.go `4dec779` | +| The Phase 9 admin route inventory predated the 19 Phase 12.2 cabana relation child, pivot and file routes, so `TestPhase09SecurityRoutes` failed on the current framework; every one carries the backend guard | — (test inventory, no production change) | The expected set lists them; the test still fails on any unguarded or unlisted admin route | `TestPhase09SecurityRoutes` | fonoteka.go `549840d` | diff --git a/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/13-VALIDATION.md b/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/13-VALIDATION.md index 5d25ae3..7f0c7d2 100644 --- a/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/13-VALIDATION.md +++ b/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/13-VALIDATION.md @@ -3,10 +3,12 @@ phase: "13" slug: "p-ytarium-api-wishlist-notifications-csv-credentials-public" # status lifecycle: draft (seeded by plan-phase) → validated (set by validate-phase §6) # audit-milestone §5.5 distinguishes NOT-VALIDATED (draft) from PARTIAL (validated + nyquist_compliant: false) (#2117) -status: draft -nyquist_compliant: false -wave_0_complete: false +status: validated +nyquist_compliant: true +wave_0_complete: true created: "2026-10-02" +validated: "2026-10-03" +gate: "scripts/check-phase13.sh --all" --- # Phase 13 — Validation Strategy @@ -22,10 +24,12 @@ created: "2026-10-02" | **Framework** | Go `testing` (+ testify, Go fuzzing), testcontainers Postgres | | **Config file** | none — `fonoteka.go/parity/parity_test.go` TestMain starts Postgres | | **Quick run command** | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka/... ./plugins/golem15/user/... -short -count=1` | -| **Full suite command** | `go vet ./... && go test ./... -count=1 && go -C ../fonoteka.go vet ./... && go -C ../fonoteka.go test ./... -count=1` | -| **Parity command** | `go -C ../fonoteka.go test ./parity -run 'TestParityCorpus|TestBroadcastGoldens|TestFonotekaNuxtFlows' -count=1` | -| **Phase gate** | `scripts/check-phase13.sh --self-test && scripts/check-phase13.sh --all` (run from summercms.go; the script lives in summercms.go/scripts like check-phase12.sh) | -| **Estimated runtime** | ~180 seconds (full suite with testcontainers) | +| **Full suite command** | `go vet ./... && go test ./... -count=1 && go -C ../fonoteka.go vet ./... && go -C ../fonoteka.go test ./... ./plugins/golem15/fonoteka/... ./plugins/golem15/user/... -count=1` | +| **Parity command** | `go -C ../fonoteka.go test ./parity -run 'TestParityCorpus|TestBroadcastGoldens|TestFonotekaNuxtFlows|TestUserAPINuxtFlows' -count=1` | +| **Phase gate** | `scripts/check-phase13.sh --self-test && scripts/check-phase13.sh --all` (run from summercms.go; the script lives in summercms.go/scripts like check-phase12.sh); `--removal` runs the RC mutations of 13-SECURITY-REVIEW.md | +| **Estimated runtime** | ~180 seconds (full suite with testcontainers); the gate's `--all` about 15 minutes, `--removal` about 20 minutes | + +The gate unsets `FORCE_COLOR` itself. On a host whose `/tmp` is a small tmpfs, run it with `TMPDIR` and `GOTMPDIR` on a larger disk (the link step of `go test ./...` needs space). --- @@ -33,50 +37,68 @@ created: "2026-10-02" - **After every task commit:** Run the quick run command plus `go vet` in the touched repo - **After every plan wave:** Run the full suite command, the parity command and the corpus check -- **Before `/gsd-verify-work`:** `scripts/check-phase13.sh --all` must be green +- **Before `/gsd-verify-work`:** `scripts/check-phase13.sh --all` must be green: vet and tests in both repos, the parity corpus (157 ported and passing, 14 recorded but not ported, read from the replay's own coverage line), the broadcast goldens including the three wishlist goldens, every TestFonotekaNuxtFlows subtest, TestUserAPINuxtFlows, `check_corpus --require-recorded --check-secrets`, TestDocsTree and `docs:build --check`, every named test by exact name, the coverage floors and this file - **Max feedback latency:** 180 seconds --- ## Per-Task Verification Map -Filled by the planner per task; the requirement → test map lives in `13-RESEARCH.md` § Validation Architecture. +Task IDs are `-T`. Every row's command was run on 2026-10-03 and passed; `scripts/check-phase13.sh --named` runs all of these tests by exact name and refuses a skip, a missing pass or "no tests to run" (the fonoteka plugin tests under `-race`). Framework commands run from `summercms.go`; application commands use `go -C ../fonoteka.go`. | Task ID | Plan | Wave | Requirement | Threat Ref | Secure Behavior | Test Type | Automated Command | File Exists | Status | |---------|------|------|-------------|------------|-----------------|-----------|-------------------|-------------|--------| -| 13-01-01 | 01 | 1 | API-03 (framework) | T-13-23 | Overlapping constrained routes dispatch to the right handler; app wishlist shapes dispatch | unit | `go test ./modules/surf -run '^(TestOverlappingConstrainedRoutes)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestWishlistOverlapPatternsDispatch)$' -count=1 -v` | ❌ W0 | ⬜ pending | -| 13-01-02 | 01 | 1 | API-03, API-05 (framework) | T-13-22 | Unregistered job kinds queue while a worker runs and are never discarded; job contract pinned | integration | `go test ./modules/conga -run '^(TestUnregisteredKindWithWorker)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka ./plugins/golem15/fonoteka/classes -run '^(TestJobContract|TestJobContractDispatchWhileWorkerRuns)$' -count=1 -v` | ❌ W0 | ⬜ pending | -| 13-01-03 | 01 | 1 | API-03, API-05 (framework) | T-13-24, T-13-25 | prohibited rule; Content-Disposition date and publication masks never hide a real diff | unit | `go test ./modules/lagoon ./modules/tide -run '^(TestValidateRequestProhibited|TestNormalizeContentDispositionDate|TestNormalizeNotificationPublication)$' -count=1 -v` | ❌ W0 | ⬜ pending | -| 13-01-04 | 01 | 1 | API-03..API-07 | T-13-26 | Queue override, rows dump, share:wishlist capture, ported case-status check, planning rewording | unit | `go -C ../fonoteka.go test ./parity -run '^(TestCheckCorpusPortedCaseStatus|TestParityCorpus)$' -count=1 -v` | ❌ W0 | ⬜ pending | -| 13-02-01 | 02 | 2 | API-04 | T-13-21 | Bell list newest 50, caller's rows only | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestNotificationsRoutes)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-02-02 | 02 | 2 | API-04, API-06 | T-13-08, T-13-09, T-13-10, T-13-21 | Notifications read; credentials CRUD encrypted, secret-free, org checks, AI/Discogs resolution order | parity + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestNotificationsRoutes|TestCredentialsCRUD|TestCredentialSecretsNeverSerialized|TestResolveAIConfigPrecedence|TestDiscogsSharedMirror)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-02-03 | 02 | 2 | API-07 | T-13-18, T-13-19, T-13-20, T-13-27 | Single first owner; register hook; payload without passwords; inspection | parity flow + integration | `go -C ../fonoteka.go test ./plugins/golem15/user -run '^(TestRegisterEventPayload)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestBootstrapConcurrent|TestRegisterInvitationListener|TestInspectInvitation)$' -count=1 -race -v` ; `TestFonotekaNuxtFlows/onboarding` | ❌ | ⬜ pending | -| 13-03-01 | 03 | 3 | API-03 | T-13-05 | Own wishlist list/show on both groups with the reservation mask | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestWishlistOwnListAndShow)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-03-02 | 03 | 3 | API-03 | T-13-28 | Item writes, prohibited 422, item-added once per path, digest coalescing, share/settings/household | parity + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestWishlistItemAddedOncePerPath|TestDigestCoalescing|TestWishlistShareSettingsHousehold)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-03-03 | 03 | 3 | API-03 | T-13-04, T-13-05, T-13-06, T-13-07, T-13-29, T-13-30 | Subscriptions, secret reservations, reveal, purchase with mail after commit, peers, overlap routes assembled | parity + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestReserveConcurrent|TestRevealIdempotent|TestReservationMask|TestWishlistSubscriptions|TestPurchaseSideEffects|TestPurchaseMailAfterCommit|TestWishlistOverlapRoutesAssembled)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-03-04 | 03 | 3 | API-03 | T-13-28 | nuxt-wishlist and mcp-wishlist flows, digest rows, publication goldens | parity flow | `go -C ../fonoteka.go test ./parity -run '^(TestFonotekaNuxtFlows|TestBroadcastGoldens)$' -count=1 -v` | ❌ | ⬜ pending | -| 13-04-01 | 04 | 4 | API-05 | T-13-14 | Export with PHP fputcsv quoting, BOM, header, formula guard | parity + unit | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka/classes/csv -run '^(TestPHPFputcsv)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestCsvExport)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-04-02 | 04 | 4 | API-05 | T-13-12, T-13-13, T-13-15 | Parser truth tables across encodings and limits; private storage; import scope | unit + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka/classes/csv -run '^(TestCsvParserTruthTable|TestCsvDetectorTruthTable)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestCsvStoreAndShow|TestCsvImportScope)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-04-03 | 04 | 4 | API-05 | T-13-16, T-13-17, T-13-31 | Commit CAS, job rows, cancel, Discogs seam, nuxt-csv flow | parity flow + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestCsvCommitCAS|TestCsvJobRows|TestCsvCancel|TestCsvRowPickSeam)$' -count=1 -race -v` ; `TestFonotekaNuxtFlows/nuxt-csv` | ❌ | ⬜ pending | -| 13-05-01 | 05 | 5 | API-07 | T-13-01, T-13-03, T-13-33 | Public resolve, exact headers, pubfail counter, D-14 layout | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestPublicResolve|TestPubfailCounter)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-05-02 | 05 | 5 | API-03, API-07 | T-13-01, T-13-02 | Public albums, facets, field set, per-route buckets | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestPublicBucketsPerRoute|TestPublicAlbumFieldSet)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-05-03 | 05 | 5 | API-07 | T-13-01, T-13-03 | public-anonymous and public-pubfail flows | parity flow | `go -C ../fonoteka.go test ./parity -run '^(TestFonotekaNuxtFlows)$' -count=1 -v` | ❌ | ⬜ pending | -| 13-06-01 | 06 | 6 | API-03..API-07 (C-01) | T-13-07, T-13-23 | Route table: groups, scopes, constraints, throttles, Phase 14 routes absent | unit | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestRouteTablePhase13)$' -count=1 -race -v` | ❌ | ⬜ pending | -| 13-06-02 | 06 | 6 | API-03..API-07 (C-04) | all mitigated T-13 | Request-DTO fuzz over every write route; one test per threat | fuzz + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(FuzzWriteEndpoints|TestPhase13Threats)$' -count=1 -race -v` | ✅ extend | ⬜ pending | -| 13-06-03 | 06 | 6 | API-03..API-07 | T-13-34, T-13-35, T-13-36 | Coverage, fail-closed gate, security review, validation sign-off | gate | `scripts/check-phase13.sh --self-test && scripts/check-phase13.sh --all && scripts/check-phase13.sh --removal` | ❌ | ⬜ pending | +| 13-01-T1 | 13-01 | 1 | API-03 (framework) | T-13-23 | Overlapping constrained routes dispatch to the right handler; app wishlist shapes dispatch | unit | `go test ./modules/surf -run '^(TestOverlappingConstrainedRoutes)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestWishlistOverlapPatternsDispatch)$' -count=1 -v` | ✅ `modules/surf/overlap_test.go`, `plugins/golem15/fonoteka/routes_overlap_test.go` | ✅ green | +| 13-01-T2 | 13-01 | 1 | API-03, API-05 (framework) | T-13-22 | Unregistered job kinds queue while a worker runs and are never discarded; job contract pinned | integration | `go test ./modules/conga -run '^(TestUnregisteredKindWithWorker)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka/classes -run '^(TestJobContract)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestJobContractDispatchWhileWorkerRuns)$' -count=1 -v` | ✅ `modules/conga/unregistered_kind_test.go`, `classes/job_contract_test.go`, `job_contract_worker_test.go` | ✅ green | +| 13-01-T3 | 13-01 | 1 | API-03, API-05 (framework) | T-13-24, T-13-25 | prohibited rule; Content-Disposition date and publication masks never hide a real diff | unit | `go test ./modules/lagoon ./modules/tide -run '^(TestValidateRequestProhibited\|TestNormalizeContentDispositionDate\|TestNormalizeNotificationPublication)$' -count=1 -v` | ✅ `modules/lagoon/validate_request_test.go`, `modules/tide/normalize_phase13_test.go` | ✅ green | +| 13-01-T4 | 13-01 | 1 | API-03..API-07 | T-13-26 | Queue override, rows dump, share:wishlist capture, ported case-status check, planning rewording | unit | `go -C ../fonoteka.go test ./parity -run '^(TestCheckCorpusPortedCaseStatus\|TestParityCorpus)$' -count=1 -v` | ✅ `parity/check_corpus_test.go` | ✅ green | +| 13-02-T1 | 13-02 | 2 | API-04 | T-13-21 | Bell list newest 50, caller's rows only | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestNotificationsRoutes)$' -count=1 -race -v` | ✅ `notifications_smoke_test.go` | ✅ green | +| 13-02-T2 | 13-02 | 2 | API-04, API-06 | T-13-08, T-13-09, T-13-10, T-13-21 | Notifications read; credentials CRUD encrypted, secret-free, org checks, AI/Discogs resolution order | parity + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestNotificationsRoutes\|TestCredentialsCRUD\|TestCredentialSecretsNeverSerialized\|TestResolveAIConfigPrecedence\|TestDiscogsSharedMirror)$' -count=1 -race -v` | ✅ `credentials_smoke_test.go` | ✅ green | +| 13-02-T3 | 13-02 | 2 | API-07 | T-13-18, T-13-19, T-13-20, T-13-27 | Single first owner; register hook; payload without passwords; inspection | parity flow + integration | `go -C ../fonoteka.go test ./plugins/golem15/user -run '^(TestRegisterEventPayload)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestBootstrapConcurrent\|TestRegisterInvitationListener\|TestInspectInvitation)$' -count=1 -race -v` ; `TestFonotekaNuxtFlows/onboarding` | ✅ `plugins/golem15/user/register_test.go`, `onboarding_smoke_test.go`, `parity/fixtures/nuxt/onboarding.yaml` | ✅ green | +| 13-03-T1 | 13-03 | 3 | API-03 | T-13-05 | Own wishlist list/show on both groups with the reservation mask | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestWishlistOwnListAndShow)$' -count=1 -race -v` | ✅ `wishlist_smoke_test.go` | ✅ green | +| 13-03-T2 | 13-03 | 3 | API-03 | T-13-28 | Item writes, prohibited 422, item-added once per path, digest coalescing, share/settings/household | parity + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestWishlistItemAddedOncePerPath\|TestDigestCoalescing\|TestWishlistShareSettingsHousehold)$' -count=1 -race -v` | ✅ `wishlist_notifications_test.go`, `wishlist_smoke_test.go` | ✅ green | +| 13-03-T3 | 13-03 | 3 | API-03 | T-13-04, T-13-05, T-13-06, T-13-07, T-13-29, T-13-30 | Subscriptions, secret reservations, reveal, purchase with mail after commit, peers, overlap routes assembled | parity + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestReserveConcurrent\|TestRevealIdempotent\|TestReservationMask\|TestWishlistSubscriptions\|TestPurchaseSideEffects\|TestPurchaseMailAfterCommit\|TestWishlistOverlapRoutesAssembled)$' -count=1 -race -v` | ✅ `wishlist_reservations_test.go`, `wishlist_notifications_test.go` | ✅ green | +| 13-03-T4 | 13-03 | 3 | API-03 | T-13-28 | nuxt-wishlist and mcp-wishlist flows, digest rows, publication goldens | parity flow | `go -C ../fonoteka.go test ./parity -run '^(TestFonotekaNuxtFlows\|TestBroadcastGoldens)$' -count=1 -v` | ✅ `parity/fixtures/nuxt/nuxt-wishlist.yaml`, `parity/fixtures/mcp/mcp-wishlist.yaml`, `parity/fixtures/broadcasts/` | ✅ green | +| 13-04-T1 | 13-04 | 4 | API-05 | T-13-14 | Export with PHP fputcsv quoting, BOM, header, formula guard | parity + unit | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka/classes/csv -run '^(TestPHPFputcsv)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestCsvExport)$' -count=1 -race -v` | ✅ `classes/csv/csv_test.go`, `csv_smoke_test.go` | ✅ green | +| 13-04-T2 | 13-04 | 4 | API-05 | T-13-12, T-13-13, T-13-15 | Parser truth tables across encodings and limits; private storage; import scope | unit + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka/classes/csv -run '^(TestCsvParserTruthTable\|TestCsvDetectorTruthTable)$' -count=1 -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestCsvStoreAndShow\|TestCsvImportScope)$' -count=1 -race -v` | ✅ `classes/csv/truth_table_test.go`, `csv_smoke_test.go` | ✅ green | +| 13-04-T3 | 13-04 | 4 | API-05 | T-13-16, T-13-17, T-13-31 | Commit CAS, job rows, cancel, Discogs seam, nuxt-csv flow | parity flow + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestCsvCommitCAS\|TestCsvJobRows\|TestCsvCancel\|TestCsvRowPickSeam)$' -count=1 -race -v` ; `TestFonotekaNuxtFlows/nuxt-csv` | ✅ `csv_smoke_test.go`, `parity/fixtures/nuxt/nuxt-csv.yaml` | ✅ green | +| 13-05-T1 | 13-05 | 5 | API-07 | T-13-01, T-13-03, T-13-33 | Public resolve, exact headers, pubfail counter, D-14 layout | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestPublicResolve\|TestPubfailCounter)$' -count=1 -race -v` | ✅ `public_share_smoke_test.go` | ✅ green | +| 13-05-T2 | 13-05 | 5 | API-03, API-07 | T-13-01, T-13-02 | Public albums, facets, field set, per-route buckets | parity + smoke | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestPublicBucketsPerRoute\|TestPublicAlbumFieldSet)$' -count=1 -race -v` | ✅ `routes_bucket_test.go`, `public_share_smoke_test.go` | ✅ green | +| 13-05-T3 | 13-05 | 5 | API-07 | T-13-01, T-13-03 | public-anonymous and public-pubfail flows | parity flow | `go -C ../fonoteka.go test ./parity -run '^(TestFonotekaNuxtFlows)$' -count=1 -v` | ✅ `parity/fixtures/nuxt/public-anonymous.yaml`, `public-pubfail.yaml` | ✅ green | +| 13-06-T1 | 13-06 | 6 | API-03..API-07 (C-01) | T-13-07, T-13-23 | Route table: groups, scopes, constraints, throttles, Phase 14 routes absent, overlap 404/405 | unit | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestRouteTablePhase13\|TestRouteTablePhase12\|TestFullRouteTableAuthGroupMutualExclusivity)$' -count=1 -race -v` | ✅ `routes_table_phase13_test.go` | ✅ green | +| 13-06-T2 | 13-06 | 6 | API-03..API-07 (C-04) | all mitigated T-13 | Request-DTO fuzz over every write route; one subtest per threat | fuzz + integration | `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(FuzzWriteEndpoints\|TestPhase13Threats)$' -count=1 -race -v` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^$' -fuzz '^FuzzWriteEndpoints$' -fuzztime 60s` | ✅ `write_endpoints_fuzz_test.go`, `testdata/fuzz/FuzzWriteEndpoints/` (70 seeds), `phase13_security_test.go` | ✅ green | +| 13-06-T3 | 13-06 | 6 | API-03..API-07 | T-13-34, T-13-35, T-13-36 | Unit coverage in both repos and the user plugin, empty bodies, boundaries, fail-closed handlers, the gate, security review, validation sign-off | unit + gate | `go test ./modules/surf ./modules/conga ./modules/lagoon ./modules/tide -run '^(TestOverlapConstraintFallsThrough\|TestOverlapFamilyOfThree\|TestOverlapHeadAndAllow\|TestOverlapFamilyAcrossPlugins\|TestOverlapUnsupportedShapes\|TestUnregisteredKindRefusalAndDelay\|TestValidateRequestProhibitedNested\|TestNormalizePhase13Edges)$' -count=1` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka -run '^(TestPhase13EmptyBodies\|TestPhase13Boundaries\|TestPhase13HandlersFailClosed\|TestPhase09SecurityRoutes)$' -count=1 -race` ; `go -C ../fonoteka.go test ./plugins/golem15/fonoteka/classes/... ./plugins/golem15/fonoteka/controllers/api ./plugins/golem15/fonoteka/middleware ./plugins/golem15/user -run '^(TestPhase13ReservationMask\|TestPhase13PubfailWindow\|TestPhase13SmallHelpers\|TestPhase13NilHandles\|TestCsvPHPCasts\|TestCsvOrderedMap\|TestCsvMappingInput\|TestPublicShareHeadersRewrites429\|TestPublicShareHeadersLeaves200Body\|TestPublicShareHeadersExactBytes\|TestRegisterUserExports)$' -count=1` ; `scripts/check-phase13.sh --self-test && scripts/check-phase13.sh --all && scripts/check-phase13.sh --removal` | ✅ `scripts/check-phase13.sh`, `13-SECURITY-REVIEW.md`, `phase13_controllers_test.go`, `classes/phase13_classes_test.go`, `classes/csv/csv_edges_test.go`, `controllers/api/phase13_controllers_test.go`, `plugins/golem15/user/registration_test.go` | ✅ green | -*Status: ⬜ pending · ✅ green · ❌ red · ⚠️ flaky* +*Status: ✅ green · ❌ red · ⚠️ flaky* + +### Coverage (scripts/check-phase13.sh --coverage, 2026-10-03) + +| Package | Statements | Floor | +|---------|------------|-------| +| summercms.go `modules/surf` | 85.8% | 80% | +| summercms.go `modules/conga` | 92.8% | 80% | +| summercms.go `modules/lagoon` | 85.1% | 80% | +| summercms.go `modules/tide` | 81.7% | 80% | +| fonoteka `classes` | 85.8% | 80% | +| fonoteka `classes/csv` | 94.5% | 80% | +| fonoteka `controllers/api` | 81.9% | 80% | +| fonoteka `middleware` | 100.0% | 80% | +| sm-user-plugin `classes` | 88.7% | 80% | +| sm-user-plugin `controllers` | 70.6% | 68.8% (its value before Phase 13) | +| sm-user-plugin `controllers/registration.go` | every function ≥ 87.5% (RegisterUser 87.5%, the other three 100%) | 80% per function | + +The fonoteka packages are measured with `-coverpkg` over every test of the plugin, as check-phase12.sh does; each framework package with its own tests. --- ## Wave 0 Requirements -- [ ] `surf` constraint-aware overlap dispatch, plus a test (blocks every wishlist route) -- [ ] `conga` unregistered-kind insert while a worker runs, plus a test -- [ ] `lagoon` `prohibited` rule; tide Content-Disposition date and notification publication masks -- [ ] `php_parity.sh` `QUEUE_CONNECTION` override; `capture-rules.yaml` `share:wishlist` capture -- [ ] `fonoteka_reset.php` + `seedFonotekaCase` states: `wishlist`, `csv`, `credentials`, `empty`, `invite-for-register` -- [ ] `scripts/check-phase13.sh` (copy of the check-phase12 structure) +- [x] `surf` constraint-aware overlap dispatch, plus a test (blocks every wishlist route) +- [x] `conga` unregistered-kind insert while a worker runs, plus a test +- [x] `lagoon` `prohibited` rule; tide Content-Disposition date and notification publication masks +- [x] `php_parity.sh` `QUEUE_CONNECTION` override; `capture-rules.yaml` `share:wishlist` capture +- [x] `fonoteka_reset.php` + `seedFonotekaCase` states: `wishlist`, `csv`, `credentials`, `empty`, `invite-for-register` +- [x] `scripts/check-phase13.sh` (copy of the check-phase12 structure) --- @@ -84,17 +106,17 @@ Filled by the planner per task; the requirement → test map lives in `13-RESEAR | Behavior | Requirement | Why Manual | Test Instructions | |----------|-------------|------------|-------------------| -| Re-recording PHP fixtures against the isolated PHP instance | API-03..API-07 | Needs the local PHP stack running | Run `php_parity.sh` recordings per D-12 with the database queue override | +| Re-recording PHP fixtures against the isolated PHP instance | API-03..API-07 | Needs the local PHP stack running | Run `php_parity.sh` recordings per D-12 with the database queue override (done in plans 13-02 to 13-05; replay is automated) | --- ## Validation Sign-Off -- [ ] All tasks have `` verify or Wave 0 dependencies -- [ ] Sampling continuity: no 3 consecutive tasks without automated verify -- [ ] Wave 0 covers all MISSING references -- [ ] No watch-mode flags -- [ ] Feedback latency < 180s -- [ ] `nyquist_compliant: true` set in frontmatter +- [x] All tasks have `` verify or Wave 0 dependencies +- [x] Sampling continuity: no 3 consecutive tasks without automated verify +- [x] Wave 0 covers all MISSING references +- [x] No watch-mode flags +- [x] Feedback latency < 180s +- [x] Nyquist compliance set in the frontmatter -**Approval:** pending +**Approval:** validated 2026-10-03 (plan 13-06; gate `scripts/check-phase13.sh --all` and `--removal` green) diff --git a/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/deferred-items.md b/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/deferred-items.md index 98a9d52..1eb05e3 100644 --- a/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/deferred-items.md +++ b/.planning/phases/13-p-ytarium-api-wishlist-notifications-csv-credentials-public/deferred-items.md @@ -19,3 +19,11 @@ Out-of-scope findings logged by plan executors. Not fixed by the plan that found - **The general `pathID` range gap is still open.** The 30 wishlist routes use `int4PathID` (`controllers/api/wishlist_albums_controller.go`), which treats ids above `math.MaxInt32` as no row, so they answer PHP's 404. Other `pathID` routes are unchanged (see the 13-02 entry). - **`cleanSession` handles carry the triggering write's statement until their first chained call.** `db.Session(&gorm.Session{NewDB: true})` keeps the callback's statement on the handle itself; only a chained call (`Where`, `Raw`, `Scopes`, ...) starts a fresh one, while `WithContext` clones the old one. 13-03 hit it when `conga.Dispatch` (which calls `WithContext` and then `Create`) received such a handle inside the album insert hook: every digest dispatch inserted two extra album rows. The wishlist code now hands job queues `jobDB` (`cleanSession(...).Scopes()`). `WriteNotification` passes the same kind of handle to `lighthouse.Service.Emit`; it works today because `Emit` does not create through it, but any future callee that does `WithContext(...).Create(...)` on a `cleanSession` handle has the same bug. Fix: make `cleanSession` return a chained (fresh-statement) handle. - **TestPhase09SecurityRoutes** (12.2 cabana routes) and the `household_smoke_test.go` gofmt drift are still open. + +## From 13-06 + +- **Resolved: `TestPhase09SecurityRoutes`.** The failure was an outdated route inventory: the 19 cabana relation child, pivot and file routes added by Phase 12.2 all carry the backend guard. The expected set now lists them (fonoteka.go `549840d`); the test still fails on any unguarded or unlisted admin route. +- **`classes.SetJobDispatcher` is process-wide.** Like the pubfail counter before 13-05 moved it into the app's service registry, the job dispatcher the album insert hook and the purchase use is installed per process at boot, so in a process that boots several apps the last one wins. Production runs one app per process; tests that boot more than one harness must boot the one whose writes dispatch jobs last (FuzzWriteEndpoints and TestPhase13Threats do). Moving it into the app registry would remove the ordering rule. +- **A JSON scalar body on `PATCH import/csv/{id}/mapping`.** Laravel's `json()` bag casts a decoded scalar with `(array)`, so a body of `5` becomes `[0 => 5]` in `$request->all()`; `mappingInput` drops a scalar body (it keeps lists and objects). No client sends a scalar body; the difference only changes which detected map a garbage request gets. +- **Hosts with a small `/tmp` tmpfs.** `go test ./...` in summercms.go failed to link with "disk quota exceeded" on this host while `/tmp` held stale caches of earlier sessions; the gate and the suites ran with `TMPDIR` and `GOTMPDIR` on the home disk. Environment, not code. +- **gofmt drift in `fonoteka.go/plugins/golem15/fonoteka/household_smoke_test.go`** is still open (13-06 did not touch the file).