--- phase: "12.2" reviewed: "2026-10-02" threats_open: 0 reviewer: "gsd-executor (plan 12.2-05; no separate security agent was available, as in Phase 08)" gate: "scripts/check-phase12.2.sh --all" --- # Phase 12.2 Security Review This review covers the Phase 12.2 admin surface after plan 12.2-05: - datepicker and fileupload fields - the seven record file routes and the seven child file routes - relation child CRUD and pivot editing - deferred binding with commit and purge - the SPA controls that drive them Each row names the control as built and the test that proves it. The executor wrote this review, because no separate security agent was available (the same arrangement as Phase 08). Every named test was re-run in this plan, either through `bash scripts/check-phase12.2.sh --all` (all nine stages PASS on 2026-10-02) or by its own `go test -run` line. A high threat counts as mitigated only when a named test fails once its control is removed. For D-15 this was shown directly. With the parent predicate in `loadChild` (modules/cabana/relation_child.go) dropped, `go test ./modules/cabana -run '^TestRelationChildScope' -count=1` fails with: ``` relation_child_scope_test.go:97: GET records/{child} through G1: status=200 want 404 body={"data":{"due_on":null,"id":2,"label":"p2"},"meta":{"labels":{}}} ``` The same run also failed `TestRelationChildScopeSessionKey/foreign_admin` (admin B read admin A's pending part). The file was restored with `git checkout`, and `git diff --quiet` confirmed it. Paths below are relative to the summercms.go root. `cabana/`, `lagoon/`, `attach/`, `conga/` and `pact/` mean `modules/cabana/`, `modules/lagoon/`, `modules/lagoon/attach/`, `modules/conga/` and `modules/pact/`. ## Threat register | Threat | Severity | Disposition | Mitigation as built (file, function) | Executable evidence (file, test) | Result | |--------|----------|-------------|---------------------------------------|----------------------------------|--------| | T-12.2-01 | high | mitigated | `attach/store.go` `Store`, `writeBlob`: 1 MiB bounded peek refuses an over-limit head before any write; `LimitReader(MaxBytes+1)` while streaming; the key is deleted on overflow | `attach/store_test.go` `TestStore/size` (exact, +1, past the read-ahead, 0 = no limit), `TestStoreSmokeRefusals` | pass | | T-12.2-02 | high | mitigated | `attach/guard.go` `IsAllowedImage` (sniff, `image.DecodeConfig`, positive size, 4096x4096 ceiling, fail closed); `attach/store.go` `DefaultFileExtensions` without svg, js, map, css, less, scss, swf, xml | `attach/guard_test.go` `TestIsAllowedImageRefuses` (SVG, HTML, GIF and PNG signature polyglots, truncated, empty, over the ceiling, zero width, BMP), `TestIsAllowedImageAccepts`; `attach/store_test.go` `TestStore/default_lists`; `cabana/fileupload_test.go` `TestFileuploadUpload` (polyglot 422 on the field) | pass; residual noted below | | T-12.2-03 | high | mitigated | `attach/store.go` `newDiskName` (22 hex + validated extension), `clientBaseName` (base name only in file_name) | `attach/store_test.go` `TestStore/extensions` (a `..\..\Uploads/PHOTO.PNG` name: disk name shape, no client path in the key) | pass | | T-12.2-04 | high | mitigated | `lagoon/purge.go` `PurgeDeferred`, `purgeBinding`, `deleteCreatedChild` (created envelope only, unattached files only, SKIP LOCKED, unresolvable types skipped) | `lagoon/purge_test.go` `TestPurgeDeferredRules`, `TestPurgeDeferredSkipsLocked`, `TestPurgeDeferredCutoff` | pass | | T-12.2-05 | medium | mitigated | `lagoon/purge.go` (keys collected, `AfterCommit` + `attach.DeleteKeys`); `cabana/field_file.go` `deleteBlobsAfterCommit`; `cabana/deferred.go` `deleteFile` | `lagoon/purge_test.go` `TestPurgeDeferredBlobsAfterCommit` (blob kept inside the transaction and after a rollback); `cabana/fileupload_test.go` `TestFileuploadRemove`, `TestFileuploadAttachOneReplace` | pass | | T-12.2-06 | high | mitigated | `lagoon/deferred.go` `DeferredKey.validate`; `lagoon/deferred_migrations.go` `backend_user_id INTEGER NOT NULL` | `lagoon/deferred_test.go` `TestDeferredStore/refusals`, `TestDeferredMigrations` (NOT NULL insert refused) | pass | | T-12.2-07 | medium | mitigated | `conga/scheduler.go` `scheduleEntries` (framework entry in the compiled table); `runScheduled` matches exact entries | `conga/schedule_test.go` `TestFrameworkScheduleForgedArgs` (other args, other command, other index skipped; the exact entry runs), `TestFrameworkScheduleEntries` | pass | | T-12.2-08 | low | accepted | Text parsing runs only on string sources already bounded by the request body cap; the parsers are linear stdlib parsers | `lagoon/fill_test.go` `TestFillTextDateTypes`, `TestFillTextKeepsEarlierConversions` (behaviour, not DoS) | accepted | | T-12.2-09 | high | mitigated | `cabana/deferred.go` `sessionKeyFrom`, `commitDeferred` (key, bouncer admin id, morph type); `cabana/field_file.go` `parentFileScope`, `findFile`; `cabana/relation_child.go` `loadParent` | `cabana/relation_child_scope_test.go` `TestRelationChildScopeSessionKey/foreign_admin`; `cabana/protected_file_test.go` `TestProtectedFileScope/pending_file_of_another_admin`; `cabana/deferred_commit_test.go` `TestDeferredCommitAppliedOnly` (another admin's binding with the same key stays) | pass | | T-12.2-10 | high | mitigated | `cabana/deferred.go` `commitDeferred` (controller morph type, declared fields and relations allowed in the operation context) | `cabana/deferred_commit_test.go` `TestDeferredCommitAppliedOnly` (undeclared field, foreign morph type and update-only field bindings ignored, slaves untouched) | pass | | T-12.2-11 | high | mitigated | `cabana/field_file.go` `fileScope.findFile` (one parent-scoped query), `parentFileScope` (owner through `loadRecord`/FormExtendQuery) | `cabana/protected_file_test.go` `TestProtectedFileScope` (download, thumb, caption, remove through another gadget: 404, no change; reorder: the same 422 as an unknown id) | pass | | T-12.2-12 | high | mitigated | `cabana/field_file.go` `serveProtectedFileOn` (inline only for `attach.AllowedImageMIMEs`, else octet-stream attachment; nosniff, `private, no-store`, `default-src 'none'; sandbox`) | `cabana/protected_file_test.go` `TestProtectedFileHeaders` (SVG, HTML, text as attachments; PNG, GIF, JPEG, WebP inline; headers on every response and thumb) | pass | | T-12.2-13 | medium | mitigated | `cabana/field_file.go` `fileItem` (no `url`/`thumb_url` for a protected relation); `attach/static.go` `StaticHandlerPublic` | `cabana/protected_file_test.go` `TestProtectedFileListHasNoURLs`, `TestProtectedFileScope/public_rows`; `attach/static_test.go` `TestStaticHandlerPublicGate` | pass | | T-12.2-14 | high | mitigated | `cabana/field_file.go` `uploadCap` (min of upload_bytes and maxFilesize + 64 KiB), `writeFileError` (413); one `file_data` part | `cabana/fileupload_test.go` `TestFileuploadUpload` (413 past the cap, 422 between maxFilesize and the cap, state unchanged) | pass | | T-12.2-15 | high | mitigated | `cabana/csrf.go` `requireAjax` on upload, reorder, caption, remove and every child write route | `cabana/phase10_csrf_test.go` `TestPhase10CSRF`; `cabana/security_coverage_test.go` `TestPhase09PermissionMatrix`; `cabana/phase10_coverage_test.go` `TestPhase10Coverage` | pass | | T-12.2-16 | medium | mitigated | `lagoon/deferred.go` `DeferredBindings` (FOR UPDATE); `cabana/deferred.go` `commitDeferred` (applied rows forgotten in the same transaction) | `cabana/deferred_commit_test.go` `TestDeferredCommitConcurrent` (two concurrent saves with one key: file, part and pivot applied once, to one gadget); `lagoon/deferred_test.go` `TestDeferredConcurrentFirstBind` (documents the duplicate first-bind gap, below) | pass | | T-12.2-17 | medium | mitigated | `cabana/field_date.go` `dateBoundDetails` (in the save and the child save) | `cabana/datepicker_test.go` `TestDatepickerBounds` (inclusive edges, UTC date of a datetime, wall-clock date with ignoreTimezone, create and update); `cabana/deferred_commit_test.go` `TestDeferredCommitRollback/datepicker_bound` | pass | | T-12.2-18 | medium | mitigated | `cabana/field_file.go` `decodeStrictBody` (default_bytes cap, DisallowUnknownFields, trailing data); `cabana/relation_child.go` `decodeCappedObject` | `cabana/fileupload_test.go` `TestFileuploadCaption` (unknown key 422), `TestFileuploadReorder`; `cabana/relation_child_test.go` `TestRelationChildCRUD/body_cap` (413 past default_bytes on a child body) | pass | | T-12.2-19 | high | mitigated | `cabana/relation_child.go` `loadChild` (FK, pivot EXISTS or the admin's bound slaves in the same query), `childFileScope` | `cabana/relation_child_scope_test.go` `TestRelationChildScope` (all 12 child routes, the seven child file routes among them, through another parent: 404 `not_found`, database, files and blobs unchanged); the removal check above | pass | | T-12.2-20 | high | mitigated | `cabana/relation_form.go` `compileRelationForm` (pivot keys, timestamps and HookPivotColumns refused at boot); `cabana/relation.go` `fillPivot` (unknown keys 422), `insertPivot` (RelationBeforeLink stamps) | `cabana/relation_child_scope_test.go` `TestRelationChildScopePivotWhitelist` (gadget_id, member_id, id, created_at, role: 422, row unchanged); `cabana/relation_child_test.go` `TestRelationChildForms` (pivot foreign key and hook column stop boot), `TestRelationChildCRUD/belongsToMany` (role stamped) | pass | | T-12.2-21 | high | mitigated | `cabana/relation_child.go` `relationAllowed`/`relationButton`, `pivotAllowed`; `cabana/http.go` `relationMutation` (toolbar button before any SQL) | `cabana/relation_child_scope_test.go` `TestRelationChildScopeToolbar` (11 routes 403 on a parent id that does not exist, nothing changed; declared buttons reach the 404) | pass | | T-12.2-22 | medium | mitigated | `cabana/relation.go` `excludePendingCreated`, hasMany candidates with a NULL key | `cabana/relation_child_test.go` `TestRelationChildCRUD/hasMany` (owned part not a candidate, link refused), `TestRelationChildDeferred` (pending part excluded); `cabana/relation_child_smoke_test.go` `TestRelationChildSmokeDeferredCreate` | pass | | T-12.2-23 | high | mitigated | `cabana/relation_child.go` `loadParent` through `loadRecord` (FormExtendQuery) | `cabana/relation_child_scope_test.go` `TestRelationChildScope/hidden_parent` (every child route 404, unchanged) | pass | | T-12.2-24 | high | mitigated | `cabana/relation_child.go` `loadParent` (id 0 needs key, deferrable relation, create, create context, backend admin), `relationParent.boundSlaves` | `cabana/relation_child_scope_test.go` `TestRelationChildScopeSessionKey` (no key 404 on 13 routes; malformed 422 `session_key`; admin B: empty lists, 404 on show, update, delete, pivot and every child file route; B's save adopts nothing) | pass | | T-12.2-25 | medium | mitigated | `cabana/deferred.go` `applyRelationBinding` (bind of an existing record re-runs `linkRelated` with the saved parent) | `cabana/deferred_commit_test.go` `TestDeferredCommitRollback/ineligible_link` (422 on members, state unchanged); `cabana/relation_child_smoke_test.go` `TestRelationChildSmokeDeferredRollback` | pass | | T-12.2-26 | medium | mitigated | `cabana/relation_form.go` `compileRelationForm` via `assetPath` (`$/` only inside the plugin) | `cabana/relation_child_test.go` `TestRelationChildForms/cross-plugin_path` | pass | | T-12.2-27 | medium | mitigated | `cabana/relation_form.go` `relationFormRefusedTypes` | `cabana/relation_child_test.go` `TestRelationChildForms` (relation, relation-manager, widget and partial fields stop boot) | pass | | T-12.2-28 | high | mitigated | `cabana/relation_child.go` `CreateChild` (`setModelColumn` from the scoped parent), `relation_form.go` (a field named like the ForeignKey stops boot), `ProjectWritableFields` | `cabana/relation_child_test.go` `TestRelationChildCRUD/hasMany` (body `gadget_id` of another gadget ignored), `TestRelationChildForms/foreign_key_field`; `cabana/relation_child_smoke_test.go` `TestRelationChildSmokeScope` | pass | | T-12.2-29 | medium | mitigated | `admin/src/app/sessionKey.ts` `newSessionKey` (32 bytes, `crypto.getRandomValues`, base64url) | `admin/tests/app/sessionKey.test.ts` (43 chars, alphabet, crypto source used, 200 unique keys); `RelationChildModal.test.ts` (new child key per open) | pass | | T-12.2-30 | high | mitigated | Text interpolation only in the new components; no raw-HTML sink in `admin/src` | `scripts/check-phase12.2.sh --hygiene` (raw-HTML sink grep); `admin/tests/list/CellValue.test.ts` (never renders markup) | pass | | T-12.2-31 | low | mitigated | `FileuploadField.vue` `loadThumb`, `forgetThumb`, `onBeforeUnmount` (object URLs revoked) | `admin/tests/form/FileuploadField.test.ts` "protected thumbnails (backstop 3)", "file rows of a protected field" (download object URL revoked on unmount) | pass | | T-12.2-32 | high | mitigated | `admin/src/api/files.ts` `uploadWithProgress` (X-Requested-With on every upload) | `admin/tests/smoke/deferred.smoke.test.ts` "uploads to record 0 with the form key" (header asserted); server side `TestPhase10CSRF` | pass | | T-12.2-33 | low | mitigated | Keys travel in `X-Session-Key` and `X-Child-Session-Key` headers only | `scripts/check-phase12.2.sh --hygiene` (no key query parameter in admin/src or cabana); `FileuploadField.test.ts` (no query string on the thumb request); `deferred.smoke.test.ts` | pass | | T-12.2-34 | low | mitigated | Fixture plugin `acme.deferred` only in `cabana/phase122_fixture_test.go` and `cabana/testdata/deferred` | `scripts/check-phase12.2.sh --hygiene` (no `acme.deferred`/`dfPlugin` in non-test Go files) | pass | | T-12.2-35 | high | mitigated | Named security tests run by the gate's `--security` stage, which refuses a missing, renamed or skipped test | `scripts/check-phase12.2.sh --security` and `--self-test` (detector refuses skip, zero tests, no tests to run, a subtest-only match and a missing named prefix); planted missing name refused with exit 1; the removal check above | pass | | T-12.2-36 | medium | mitigated | The executor does not tag or push. The user tags at the blocking-human checkpoint of plan 05 Task 4 | `git tag --list v0.1.1` is empty when the checkpoint is presented | pending user (checkpoint) | | T-12.2-SC (plans 01, 02, 03, 05) | low | accepted | No Go module and no npm package added in these plans; go.mod and go.sum unchanged | `git diff 79e2a43..HEAD -- go.mod go.sum` shows no change in this phase | accepted | | T-12.2-SC (plan 04) | high | mitigated | `@internationalized/date` 3.12.4 added only after the user approved it at a blocking-human decision checkpoint (STATE.md decision); exact pin; integrity hash in the committed package-lock.json | `admin/package.json` pin `3.12.4`; `scripts/check-admin-dist.sh` (rebuild from the committed lockfile) | pass | ## Residual risk - **Header-only image guard (T-12.2-02).** `IsAllowedImage` checks the type sniff and the decoded header only. A GIF with a valid 1x1 header followed by HTML is accepted and stored as `image/gif`. This was probed in this plan and is not pinned by a test. The protected route serves it as `image/gif` with nosniff and a sandbox CSP, and browsers do not sniff image types into HTML, so no script runs. The public `StaticHandler`/`StaticHandlerPublic` sends the stored content type without `X-Content-Type-Options: nosniff`. That handler predates this phase. It is recorded in `deferred-items.md` as a hardening follow-up and was not changed here. - **Concurrent first binds (T-12.2-16).** Two transactions that bind the same slave for the first time can both insert a binding, because no unique index exists. WinterCMS has the same gap. `TestDeferredConcurrentFirstBind` documents this. The duplicates are harmless: a save reads both, applies them idempotently and forgets both. A unique index needs a decision and a migration, so it is left to the user (see the 12.2-05 summary). - **Browser checks.** Calendar keyboard use and visual fit, drag reorder and progress feel, and the modal flow are manual checks for `/gsd-verify-work 12.2`. This review does not claim them.