Files
summercms/.planning/phases/12.2-admin-form-fields-date-file-upload-relation-editing-with-def/12.2-SECURITY-REVIEW.md
Jakub Zych 912a46603f docs(12.2-05): security review and validation map for phase 12.2
- 12.2-SECURITY-REVIEW.md maps T-12.2-01 to T-12.2-36 and the supply
  chain rows to the controls as built and their passing tests, with the
  parent-predicate removal check and the residual risks
- 12.2-VALIDATION.md: per-task map with real task ids, all green,
  nyquist_compliant and wave_0_complete set
- deferred-items.md: out-of-scope findings for follow-up
2026-10-02 20:54:14 +02:00

16 KiB

phase, reviewed, threats_open, reviewer, gate
phase reviewed threats_open reviewer gate
12.2 2026-10-02 0 gsd-executor (plan 12.2-05; no separate security agent was available, as in Phase 08) 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.