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
This commit is contained in:
Jakub Zych
2026-09-18 20:54:10 +02:00
parent e0a11c3ea0
commit ba3efccadd

View File

@@ -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*