test(15-04): add fail-closed Phase 15 gate and ASVS L1 review
scripts/check-phase15.sh --all refuses skip/no-tests/race/dirty PHP pin; the review closes T-15-01..15 and T-15-SC with executed TestNames. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Jakub Zych
201
.planning/phases/15-journal-plugin/15-SECURITY-REVIEW.md
Normal file
201
.planning/phases/15-journal-plugin/15-SECURITY-REVIEW.md
Normal file
@@ -0,0 +1,201 @@
|
||||
---
|
||||
phase: 15
|
||||
slug: journal-plugin
|
||||
status: verified
|
||||
threats_total: 16
|
||||
threats_closed: 16
|
||||
threats_open: 0
|
||||
accepted_risks: 0
|
||||
asvs_level: 1
|
||||
block_on: high
|
||||
created: 2026-10-06
|
||||
verified: 2026-10-06
|
||||
reviewer: gsd-executor (15-04 Task 3, self-performed -- see Reviewer Note)
|
||||
---
|
||||
|
||||
# Phase 15 — Security Review
|
||||
|
||||
> Lean Journal plugin (`sm-journal-plugin`) and the proof-host boot of
|
||||
> user+translate+journal. Every Phase 15 threat locked in plans 01–04 is
|
||||
> mapped below to executed, named Go evidence. Unmapped IDs would be a
|
||||
> review gap, not an accepted risk; none exist.
|
||||
|
||||
**Date:** 2026-10-06
|
||||
**Scope:** Plans 15-01 through 15-04; `sm-journal-plugin`; proof host
|
||||
`sm-grzybyfunkcjonalne-app`; `scripts/check-phase15.sh`.
|
||||
**Repos grepped:** `sm-journal-plugin`, `summercms.go` (excluding
|
||||
`.planning/` except this review), `sm-grzybyfunkcjonalne-app`.
|
||||
|
||||
## Reviewer Note
|
||||
|
||||
15-04-PLAN.md Task 3 calls for an independent `gsd-security-auditor`
|
||||
agent pass. This Cursor session has no dedicated security-auditor
|
||||
subagent (same fallback as 14.2.1-04): the 15-04 executor performed the
|
||||
review directly. Every high threat below is closed with source citations
|
||||
and named tests **re-executed during this review** (2026-10-06 plugin
|
||||
`go test ./... -race` and host `go test ./... -race`), not merely
|
||||
inherited from earlier plans.
|
||||
|
||||
No external API integration: this phase ports a compiled plugin and local
|
||||
host contracts only. No external SaaS SDK this phase (Typesense stays
|
||||
behind a default-off gate; TestSearchGateOff recorded zero HTTP).
|
||||
|
||||
---
|
||||
|
||||
## Verdict Summary
|
||||
|
||||
The register contains **16 total threats: 16 closed, 0 open, 0 accepted
|
||||
risks**. High findings block phase completion; all high rows are mitigate
|
||||
with executed named tests. PHP pin SHA `02110eb1c0c3861370b0b9b47b209a0702ac5d88`
|
||||
is unchanged.
|
||||
|
||||
---
|
||||
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description | Data Crossing |
|
||||
|----------|-------------|----------------|
|
||||
| anonymous GET → published posts | public `/_journal/api/v1` | published rows only; drafts 404 without `data` |
|
||||
| backend JWT → writes / media | HS256 `aud=backend` | title/content/files; never frontend audience |
|
||||
| Fillable / API assigns → GORM | untrusted JSON | nest_*, redactor_id, user_id must not persist from maps |
|
||||
| markdown → stored HTML | FormatHTML rejectUnsafe | script/iframe/event/js schemes |
|
||||
| test fixture → production binary | process-local plugins | must not appear in host `plugins.gen.go` |
|
||||
| gate → production claims | skipped containers / dirty PHP | named PASS + final marker |
|
||||
|
||||
---
|
||||
|
||||
## Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Proof |
|
||||
|-----------|----------|-----------|----------|-------------|-------|
|
||||
| T-15-01 | Spoofing | POST `/_journal/api/v1/posts` | high | mitigate | `journal_api_writes_test.go:TestJournalWriteUnauthenticated`; frontend audience 401 |
|
||||
| T-15-02 | Information Disclosure | GET posts/{slug} drafts | high | mitigate | `TestJournal005DraftShow` 404 without `data`; owner/`access_other_posts` 200 |
|
||||
| T-15-03 | Tampering | POST `/media/upload` | high | mitigate | `TestJournal006MediaUpload` 403 without `access_posts`; folder `..` 422; `/journal/` prefix |
|
||||
| T-15-04 | Elevation of Privilege | Category/Tag/Post Fillable | high | mitigate | `models/fillable_test.go:TestFillable`; `TestJournalAPIMassAssignRedactor` |
|
||||
| T-15-05 | Tampering | gormigrate DDL | high | mitigate | `TestJournalTables`; `TestJournalMigrationsRollbackAndRemigrate`; no AutoMigrate |
|
||||
| T-15-06 | Tampering | MorphName | high | mitigate | `TestTranslatable`; `TestPostTranslatableSmoke` PHP class strings |
|
||||
| T-15-07 | Elevation of Privilege | Posts admin | high | mitigate | `TestPostsAdminForbidden` 403 without `access_posts`; owner scope in Plan 02 |
|
||||
| T-15-08 | Tampering | FormatHTML | high | mitigate | `classes/format_html_test.go:TestFormatHTMLRejectsUnsafeHTML` |
|
||||
| T-15-09 | Elevation of Privilege | access_publish | high | mitigate | `TestJournalWriteUnauthenticated` publish 403; `TestPostsAdminCreateSmoke/publish_without_access_publish` |
|
||||
| T-15-10 | Spoofing | write API tokens | high | mitigate | `TestJournalWriteUnauthenticated` / `TestJournalWriteFrontendAudience` reject `aud=user` |
|
||||
| T-15-11 | Information Disclosure | Typesense sync | high | mitigate | `search_test.go:TestSearchGateOff` zero HTTP; unpublished `ShouldBeSearchable` false with gate flipped |
|
||||
| T-15-12 | Denial of Service | X-Forwarded-For | medium | mitigate | `plugin.go` buckets use `surf.ClientIP` + `TrustedProxies`; `TestJournalBuckets` |
|
||||
| T-15-13 | Tampering | error envelope | high | mitigate | `TestJournalWriteUnauthenticated` PHP `{error}` string, no cabana admin envelope |
|
||||
| T-15-14 | Repudiation | phase gate | high | mitigate | `scripts/check-phase15.sh` detector refuses skip/no-tests/race; `--self-test` |
|
||||
| T-15-15 | Information Disclosure | unpublished title prefix | medium | mitigate | `TestJournalAPIShowNeighbors` JSON title omits `UnpublishedTitlePrefix` |
|
||||
| T-15-SC | Tampering | package installs | high | mitigate | plugin `replace` is only `summercms => ../summercms.go`; goldmark already in the graph; no new SaaS SDK |
|
||||
|
||||
---
|
||||
|
||||
## Findings by Threat
|
||||
|
||||
### T-15-01 — unauthenticated and frontend-audience writes
|
||||
|
||||
- **Source:** `controllers/api/auth.go` `requireBackendPrincipal`; PHP `{error:"Authentication required"}`.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournalWriteUnauthenticated` PASS; `TestJournalWriteFrontendAudience` PASS; featured-image POST/DELETE 401 in `TestJournalFeaturedImageUnauthenticated` PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-02 — draft enumeration
|
||||
|
||||
- **Source:** `controllers/api/posts.go` Show; 404 without `data` unless owner or `access_other_posts`.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournal005DraftShow` PASS; `TestJournalEndToEnd` anonymous draft 404 PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-03 — media traversal
|
||||
|
||||
- **Source:** `controllers/api/media.go` folder regex, `..` reject, forced `/journal/` prefix.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournal006MediaUpload` PASS; `TestMediaObjectPath` PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-04 — mass assignment
|
||||
|
||||
- **Source:** Tag/Category `Fillable`; Post API `buildNewPost` field-by-field (never `lagoon.Fill` of `redactor_id`/`user_id`).
|
||||
- **Test evidence (re-run 2026-10-06):** `TestFillable` PASS; `TestJournalAPIMassAssignRedactor` PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-05 — schema / AutoMigrate
|
||||
|
||||
- **Source:** gormigrate IDs `202610060001`–`007`; production plugin has no `AutoMigrate(`.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournalTables` PASS; `TestJournalMigrationsRollbackAndRemigrate` PASS. Gate `--forbidden` refuses production AutoMigrate.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-06 — MorphName
|
||||
|
||||
- **Source:** hard-coded `Golem15\Journal\Models\Post` / `Category` / `Tag`.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestTranslatable` PASS; `TestPostTranslatableSmoke` PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-07 — admin access_posts
|
||||
|
||||
- **Source:** Posts controller `RequiredPermissions`; List/FormExtendQuery owner scope without `access_other_posts`.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestPostsAdminForbidden` PASS (403 without grant).
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-08 — stored XSS in content_html
|
||||
|
||||
- **Source:** `classes/format_html.go` goldmark without unsafe HTML; `rejectUnsafe` for script/iframe/event/js/vbscript/data.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestFormatHTMLRejectsUnsafeHTML` and subtests script/iframe/event/javascript/vbscript/data PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-09 — publish permission
|
||||
|
||||
- **Source:** Store/Update refuse `published` without `golem15.journal.access_publish`; admin `ForbiddenError`.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournalWriteUnauthenticated` publish 403 PASS; `TestPostsAdminCreateSmoke/publish_without_access_publish` PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-10 — frontend token on writes
|
||||
|
||||
- **Source:** backend JWT audience only; no Apparatus personal tokens.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournalWriteUnauthenticated` frontend-audience POST 401 PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-11 — Typesense leak
|
||||
|
||||
- **Source:** `search_use_typesense` default false; `ShouldBeSearchable` false when unpublished or gate off.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestSearchGateOff` PASS (zero HTTP; must not skip).
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-12 — rate-limit XFF
|
||||
|
||||
- **Source:** `Plugin.Buckets` keys `surf.ClientIP` with `TrustedProxies`.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournalBuckets` PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-13 — envelope mixup
|
||||
|
||||
- **Source:** journal `writeAPIError` PHP `{error}` string; must not use cabana admin `{error:{code}}` on public API.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournalWriteUnauthenticated` PASS (string error, no `data`).
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-14 — gate repudiation
|
||||
|
||||
- **Source:** `scripts/check-phase15.sh` JSON detector.
|
||||
- **Test evidence:** `--self-test` (fail/skip/zero/no-tests/race/missing-named) executed as the first `--all` stage.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-15 — unpublished lock prefix
|
||||
|
||||
- **Source:** API serialize uses raw `Title`; `console.UnpublishedTitlePrefix` is import-only.
|
||||
- **Test evidence (re-run 2026-10-06):** `TestJournalAPIShowNeighbors` PASS.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
### T-15-SC — package installs
|
||||
|
||||
- **Source:** plugin `go.mod` replace of summercms only; goldmark v1.8.6 already required for FormatHTML.
|
||||
- **Test evidence:** `--layout` replace check; no `go get` of a new SaaS SDK this plan.
|
||||
- **Disposition:** closed / mitigate.
|
||||
|
||||
---
|
||||
|
||||
## Submodule provenance
|
||||
|
||||
Host gitlinks `plugins/golem15/{user,translate,journal}` are mode `160000`.
|
||||
`TestBootUserTranslateJournal` PASS (re-run 2026-10-06). `--layout` requires
|
||||
the three production IDs and CORS `_journal/api/*`.
|
||||
|
||||
---
|
||||
|
||||
## API-coverage declaration
|
||||
|
||||
No external SaaS SDK this phase. Typesense is optional and default-off;
|
||||
`TestSearchGateOff` observed zero outbound HTTP.
|
||||
Reference in New Issue
Block a user