From ba3efccadd0382ad894db5d11e2dd5aee867a1c5 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Fri, 18 Sep 2026 20:54:10 +0200 Subject: [PATCH] docs(05-06): complete fill-fuzz hidden-marshal security-review plan - Record fuzz, hidden-marshal, public_token, attach, and security-review outcomes - Note module-boundary placement of the models.All() walk --- .../05-06-SUMMARY.md | 175 ++++++++++++++++++ 1 file changed, 175 insertions(+) create mode 100644 .planning/phases/05-data-layer-full-fidelity/05-06-SUMMARY.md diff --git a/.planning/phases/05-data-layer-full-fidelity/05-06-SUMMARY.md b/.planning/phases/05-data-layer-full-fidelity/05-06-SUMMARY.md new file mode 100644 index 0000000..ceeaeb2 --- /dev/null +++ b/.planning/phases/05-data-layer-full-fidelity/05-06-SUMMARY.md @@ -0,0 +1,175 @@ +--- +phase: 05-data-layer-full-fidelity +plan: 06 +subsystem: testing +tags: [fuzz, fillable, hidden, encrypted, attach, security-review, postgres] + +requires: + - phase: 05-data-layer-full-fidelity + provides: Winter-directory registries, lagoon.Fill, write services, Encrypted, attach, 25-model schema +provides: + - FuzzFill plus Album/Collection/4-credential fill-boundary fuzz against real Postgres + - Hidden-marshal coverage of every registered model in both plugins + - Collection.public_token soft-delete UNIQUE parity with PHP + - Collection attach photos/image/thumb/delete smoke test + - 05-SECURITY-REVIEW.md mapping T-05-01 through T-05-24 +affects: [06-http, 12-api-parity] + +tech-stack: + added: [] + patterns: + - Two-layer fillable proven by fuzz: service list is the working boundary, Fillable() is the backstop + - Hidden-marshal enumerates models.All() counts so a skipped model fails + - Framework tests stay app-agnostic; registry walks live in fonoteka.go + +key-files: + created: + - lagoon/fill_fuzz_test.go + - lagoon/hidden_marshal_test.go + - .planning/phases/05-data-layer-full-fidelity/05-SECURITY-REVIEW.md + - plugins/golem15/fonoteka/classes/postgres_test.go + - plugins/golem15/fonoteka/classes/album_write_service_fuzz_test.go + - plugins/golem15/fonoteka/classes/collection_write_service_fuzz_test.go + - plugins/golem15/fonoteka/classes/credential_write_service_fuzz_test.go + - plugins/golem15/fonoteka/classes/hidden_marshal_test.go + - plugins/golem15/fonoteka/classes/collection_public_token_test.go + - plugins/golem15/fonoteka/classes/attach_smoke_test.go + modified: + - plugins/golem15/fonoteka/go.mod + - plugins/golem15/fonoteka/go.sum + +key-decisions: + - "Hidden-marshal registry walk lives in fonoteka.go/classes because summercms.go must not import the app" + - "Credential fuzz excludes owner FKs from the working allow-list, matching D-05 two-layer even without a write-service file" + - "classes TestMain is the real-Postgres harness; parity activateAppPlugins/parityDB cannot be imported from package main" + +patterns-established: + - "Phase-ending coverage plan fuzzes existing fill boundaries rather than reimplementing them" + - "Security review greps .Reveal(), Association(, and AutoMigrate and maps every T-05-xx by name" + +requirements-completed: [DATA-06, DATA-07, DATA-09] + +duration: 23min +completed: 2026-09-18 +--- + +# Phase 5 Plan 06: Full-fidelity unit/fuzz coverage and security review Summary + +**lagoon.Fill and six D-07 write paths fuzzed against real Postgres; every registered model hidden-marshaled; Collection.public_token matches PHP's plain UNIQUE; attach photos/image/thumb/delete smoked; T-05-01..24 mapped in 05-SECURITY-REVIEW.md** + +## Performance + +- **Duration:** 23 min +- **Started:** 2026-09-18T18:30:26Z +- **Completed:** 2026-09-18T18:53:12Z +- **Tasks:** 3 +- **Files modified:** 12 + +## Accomplishments + +- `FuzzFill` ran 20s with zero crashers; non-allow-listed fixture fields never change. `FuzzSaveAlbum` / `FuzzSaveCollection` (20s each) and four credential fuzz targets persist against testcontainers Postgres and reject server-owned keys +- `TestHiddenNeverMarshals` enumerates 26 Fonoteka + 2 user models (28 total) and asserts every `Hidden()` sentinel and `json:"-"` field is absent from JSON +- `TestCollectionPublicTokenDeleteThenRecreate` gets a real unique-violation after soft-delete and succeeds after hard-delete +- `TestAttachSmoke` covers ordered photos, attachOne image, Winter thumb filename, soft-delete keep, and two-phase force-delete against memblob +- `05-SECURITY-REVIEW.md` maps T-05-01 through T-05-24; `.Reveal()` exists only on the method and in tests; no production `AutoMigrate`; no Association Mode pivot writes + +## Task Commits + +Each task was committed atomically: + +1. **Task 1: Fill boundary fuzz (lagoon)** - `7ead5e0` (test) in summercms.go +2. **Task 1: Fill boundary fuzz (write services)** - `836da04` (test) in fonoteka.go +3. **Task 2: Hidden-marshal fixture** - `1fcf5ab` (test) in summercms.go +4. **Task 2: Hidden-marshal all models + public_token** - `1ecd660` (test) in fonoteka.go +5. **Task 3: Attach smoke test** - `4f88c2f` (test) in fonoteka.go +6. **Task 3: Security review** - `e0a11c3` (docs) in summercms.go + +**Plan metadata:** pending (this file) + +## Files Created/Modified + +- `lagoon/fill_fuzz_test.go` — `FuzzFill` plus nil/empty map smoke +- `lagoon/hidden_marshal_test.go` — framework HasHidden fixture +- `.planning/phases/05-data-layer-full-fidelity/05-SECURITY-REVIEW.md` — threat map and greps +- `fonoteka.go/plugins/golem15/fonoteka/classes/postgres_test.go` — testcontainers harness (parity helpers are package main) +- `classes/album_write_service_fuzz_test.go`, `collection_write_service_fuzz_test.go`, `credential_write_service_fuzz_test.go` +- `classes/hidden_marshal_test.go`, `collection_public_token_test.go`, `attach_smoke_test.go` +- `plugins/golem15/fonoteka/go.mod`, `go.sum` — testcontainers-go v0.44.0, gocloud.dev (test imports) + +## Decisions Made + +- The All() hidden-marshal walk lives in `fonoteka.go` classes tests; putting it in `lagoon` would import the app into the framework module (CLAUDE.md two-repo rule). The plan's own fallback is used. `lagoon.TestHiddenNeverMarshals` still exists so the verify command passes +- Credential models have no write-service file; the fuzz path uses `Fillable()` minus `user_id`/`organisation_id`/`api_key`/`token` so a fuzzed owner-FK override cannot stick (D-05 two-layer, matching AlbumFillFields excluding `collection_id`) +- `classes.TestMain` starts ICU pl-PL Postgres and migrates both plugin sets; `activateAppPlugins`/`parityDB` cannot be imported from `package main` + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 3 - Blocking] classes tests cannot import parity helpers** +- **Found during:** Task 1 +- **Issue:** `activateAppPlugins`/`parityDB` live in `fonoteka.go/parity` `package main` +- **Fix:** `classes/postgres_test.go` TestMain + `lagoon.Migrate` of both plugin sets +- **Files modified:** `fonoteka.go/plugins/golem15/fonoteka/classes/postgres_test.go` +- **Verification:** fuzz seeds and 20s runs +- **Committed in:** `836da04` (Task 1) + +**2. [Rule 2 - Missing Critical] credential Fillable() includes owner FKs** +- **Found during:** Task 1 +- **Issue:** Using `Fillable()` as the allow-list would let a fuzzed `user_id`/`organisation_id` change the owner, contradicting the acceptance criterion +- **Fix:** Working allow-list drops owner FKs and Encrypted columns; secrets are set via `NewEncrypted` then `Save` +- **Files modified:** `credential_write_service_fuzz_test.go` +- **Verification:** four credential fuzz seed corpora +- **Committed in:** `836da04` (Task 1) + +**3. [Rule 3 - Blocking] Hidden-marshal All() cannot live in lagoon** +- **Found during:** Task 2 +- **Issue:** Importing both plugin `models.All()` from `summercms.go` would make the framework know about Płytarium +- **Fix:** Fixture test in `lagoon/hidden_marshal_test.go`; registry walk in `classes/hidden_marshal_test.go` (plan-allowed fallback) +- **Files modified:** both hidden_marshal_test.go files +- **Verification:** `go test ./lagoon/... -run TestHiddenNeverMarshals` and `go test ./classes/ -run TestHiddenNeverMarshals` +- **Committed in:** `1fcf5ab`, `1ecd660` (Task 2) + +--- + +**Total deviations:** 3 auto-fixed (1 missing critical, 2 blocking) +**Impact on plan:** Required for compiling tests, the owner-FK assertion, and the two-repo rule. No production-code scope creep. + +## Issues Encountered + +`Album.BeforeSave.stampMarketPrice` clears `market_price_source` when stored money is blank. The album fuzz seeds a real price and strips `market_price_stored` from the fuzzed map so the assertion measures mass-assignment, not money-normalization (already covered by `TestSaveAlbumMoneyNormalization`). + +Production code already existed; TDD tasks wrote tests that pass against it. A leak of `collection_id`/`market_price_source` would still fail the fuzz. + +## Authentication Gates + +None. + +## TDD Gate Compliance + +Plan type is `execute`, not `type: tdd`. Tasks 1–2 have `tdd="true"` but cover existing code from 05-01..05-05. No RED commit: a passing test here means the fillable/hidden invariants hold, not that the test is wrong. GREEN is the test commit itself. + +## Known Stubs + +None that prevent this plan's goal. + +## User Setup Required + +None - no external service configuration required. + +## Next Phase Readiness + +Phase 5 is complete. Fill, hidden, encrypted, attach, and schema-diff invariants have closing tests and a committed security review. Ready for Phase 6 (HTTP routing, auth groups, rate limiting). HTTP DTO fuzz remains Phase 12 (D-07). `StaticHandler` `is_public` gate remains Phase 6/12 (T-05-16). + +## Self-Check: PASSED + +- Key files exist on disk (`lagoon/fill_fuzz_test.go`, `lagoon/hidden_marshal_test.go`, `05-SECURITY-REVIEW.md`, classes fuzz/hidden/public_token/attach/postgres tests) +- Commits `7ead5e0`, `1fcf5ab`, `e0a11c3` (summercms.go) and `836da04`, `1ecd660`, `4f88c2f` (fonoteka.go) exist +- `go vet ./...` and `go test ./...` green in summercms.go +- `go vet ./...` and `go test ./...` plus plugin modules green in fonoteka.go +- `FuzzFill` / `FuzzSaveAlbum` / `FuzzSaveCollection` each ran `-fuzztime=20s` with zero crashers +- `grep AutoMigrate` on `fonoteka.go/plugins` and `summercms.go/lagoon` production code is zero (test-only mentions) + +--- +*Phase: 05-data-layer-full-fidelity* +*Completed: 2026-09-18*