Files
summercms/.planning/phases/14.2.1-translate-plugin/14.2.1-SECURITY-REVIEW.md
Jakub Zych ef03016314 test(14.2.1-06): land remaining named tests and phase gate
WR-02 proofs and T-14.2.1-19..23 must fail the phase closed if hydration or child locale validation is removed.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-10-06 16:28:29 +02:00

265 lines
17 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
phase: 14.2.1
slug: translate-plugin
status: verified
threats_total: 24
threats_closed: 24
threats_open: 0
accepted_risks: 0
asvs_level: 1
created: 2026-10-06
verified: 2026-10-06
reviewer: gsd-executor (14.2.1-04 Task 3 and 14.2.1-06 Task 3, self-performed -- see Reviewer Note)
---
# Phase 14.2.1 — Security Review
> Lean Translate plugin (`sm-translate-plugin`), cabana `markdown`/`mltext`/`mlmarkdown`,
> surf `LocaleResolver`, and the proof-host boot of user+translate. Every
> T-14.2.1 threat locked in plans 01–04 and 06 is mapped below to executed, named Go
> or Vitest evidence. Unmapped IDs would be a review gap, not an accepted
> risk; none exist.
**Date:** 2026-10-06
**Scope:** Plans 14.2.1-01 through 14.2.1-06; `sm-translate-plugin`;
`summercms.go` modules `cabana` and `surf` plus admin SPA ML/markdown
controls; proof host `sm-grzybyfunkcjonalne-app`.
**Repos grepped:** `sm-translate-plugin`, `summercms.go` (excluding
`.planning/` except this review), `sm-grzybyfunkcjonalne-app`.
## Reviewer Note
14.2.1-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 Phase 08): the 14.2.1-04 executor performed the review
directly, and 14.2.1-06 Task 3 appended CR-01/WR-02 rows the same way. Every
high threat below is closed with source citations and named tests
**re-executed during this review** (2026-10-06), not merely inherited
from earlier plans. The executor checkpoint return states this plainly.
No external API integration: this phase ports a compiled plugin and local
framework/host contracts only.
---
## Verdict Summary
The register contains **24 total threats: 24 closed, 0 open, 0 accepted
risks**. High findings block phase completion; all high rows are mitigate
with executed named tests. PHP pin SHA `725d547ec839f02b5fdc0f0a6faaed601a414d50`
is unchanged.
---
## Trust Boundaries
| Boundary | Description | Data Crossing |
|----------|-------------|----------------|
| untrusted request → Translator | URL prefix, cookies, Accept-Language, principal preferred_locale | locale codes only; must match enabled Locale rows |
| unauthenticated vs `manage_locales` | Locales admin CRUD | JWT + permission grant |
| admin SPA → cabana save | nested mltext/mlmarkdown JSON | locale→string maps lifted before Fill |
| markdown source → HTML | stored body / SPA preview | goldmark without WithUnsafe; SPA interpolates text |
| test fixture → production binary | process-local `acme.fixture` | must not appear in generated plugin imports |
| gate → production claims | skipped containers / stale dist | named PASS + final marker |
---
## Threat Register
| Threat ID | Category | Component | Severity | Disposition | Proof |
|-----------|----------|-----------|----------|-------------|-------|
| T-14.2.1-01 | Tampering | Translator locale inputs | high | mitigate | `classes/translator_resolve_test.go:TestTranslatorResolve` (URL/session/header invalid codes ignored; only enabled `en`/`pl`) |
| T-14.2.1-02 | Information Disclosure | request locale state | high | mitigate | `classes/translator.go` has no process singleton; `TestTranslatorConcurrentIsolation`; `modules/surf/locale_resolver_test.go:TestLocaleResolver` |
| T-14.2.1-03 | Tampering | manual-selection cookie | medium | mitigate | `classes/translator_test.go:TestManualFlagIsNotALocale`; `TestTranslatorResolve/manual_flag_blocks_accept-language` |
| T-14.2.1-04 | Tampering | schema source | high | mitigate | `updates/migrations_test.go:TestTranslateTables`; `TestTranslateMigrationsRollbackAndRemigrate`; no AutoMigrate in production plugin |
| T-14.2.1-05 | Elevation of Privilege | Locales admin | high | mitigate | `locales_admin_test.go:TestLocalesAdminForbidden`; `integration_test.go:TestTranslateEndToEnd` 403 without `golem15.translate.manage_locales` |
| T-14.2.1-06 | Tampering | Locale writes | high | mitigate | `models/locale.go` Fillable excludes `is_default`/`sort_order`; `TestLocalesAdminMassAssignment`; `TestLocalesAdminDefaultGuards` |
| T-14.2.1-07 | Tampering | SetTranslated | high | mitigate | `classes/translatable.go` declared-field + enabled-locale checks; `TestInvalidLocaleRejected`; `TestTranslatableGetSet` |
| T-14.2.1-08 | Information Disclosure | Morph/index queries | medium | mitigate | `TestTranslatableIndexAndMorphIsolation`; MorphName is an import-stable string |
| T-14.2.1-09 | Tampering | ML nested request | high | mitigate | `modules/cabana/field_ml.go` `liftMLValues`; `TestMLNestedSave`; `TestMLFieldTypes` |
| T-14.2.1-10 | Elevation of Privilege | TranslationWriter | high | mitigate | no standalone translate-write route; `TestML/writer_is_not_invoked_before_query_checks`; writer runs after host PK inside the save transaction |
| T-14.2.1-11 | Tampering | mass assignment | high | mitigate | nested non-ML maps still dropped; default scalar only reaches Fill; `TestMLNestedSave` |
| T-14.2.1-12 | Tampering | markdown preview | high | mitigate | `modules/cabana/field_markdown.go:RenderMarkdown`; `TestMarkdownRejectsUnsafeHTML`; `admin/tests/form/MarkdownField.test.ts` |
| T-14.2.1-13 | Tampering | host submodule provenance | medium | mitigate | `TestBootUserTranslate` gitlink `160000`; `scripts/check-phase14.2.1.sh --layout` |
| T-14.2.1-14 | Repudiation | phase gate | high | mitigate | `scripts/check-phase14.2.1.sh` detector refuses skip/no-tests/race; `--self-test` proves fail-closed |
| T-14.2.1-15 | Information Disclosure | concurrent Translator | high | mitigate | `TestTranslatorConcurrentIsolation` under `go test -race`; `TestLocaleResolver/conflicting_requests_stay_isolated` |
| T-14.2.1-16 | Tampering | migration rollback | medium | mitigate | `TestTranslateMigrationsRollbackAndRemigrate` dedicated Postgres |
| T-14.2.1-17 | Tampering | generated admin artifacts | medium | mitigate | `TestMLOpenAPIConformance`; `scripts/check-admin-openapi.sh --check`; `scripts/check-admin-dist.sh` |
| T-14.2.1-18 | Elevation of Privilege | test fixture | high | mitigate | `TestTranslateEndToEndFixtureAbsentFromProduction`; host `plugins.gen.go` has no `acme.fixture` |
| T-14.2.1-19 | Information Disclosure | hydrateMLRecord / ShowChild | high | mitigate | `modules/cabana/field_ml.go` hydrates only declared ML fields on Show/save and child record payloads; `TestMLHydration`; `TestRelationChildMLNestedSave` |
| T-14.2.1-20 | Information Disclosure | FormMeta.EnabledLocales | medium | mitigate | `modules/cabana/http.go` copies locales only on protect()'d form/relation schemas; `admin/tests/form/formState.test.ts` seeds en+pl; `admin/tests/form/MLFields.test.ts` create-empty |
| T-14.2.1-21 | Tampering | FormView.adopt | medium | mitigate | `mergeMLValue` applies only to ML fields; `admin/tests/form/formState.test.ts` GET-string merge; `admin/tests/form/MLFields.test.ts` sibling locales |
| T-14.2.1-22 | Tampering | fillChild / CreateChild / UpdateChild | high | mitigate | `liftMLValues` before `projectOperation`; `TestRelationChildMLNestedSave` (de 422, no row) |
| T-14.2.1-23 | Elevation of Privilege | RelationService.writer | high | mitigate | `http.go` `relations()` Lookup on existing protect()'d child routes; apply after loadParent/loadChild and child PK; `TestRelationChildMLNestedSave` |
| T-14.2.1-SC | Tampering | package installs | high | mitigate | no `go get` / npm install this plan; plugin `replace` of `sm-user-plugin` is a sibling checkout; framework `go.mod`/`admin/package-lock.json` unchanged |
---
## Findings by Threat
### T-14.2.1-01 — locale injection
- **Source:** `sm-translate-plugin/classes/translator.go` `isValid` / `enabledCodes` (enabled Locale rows only).
- **Test evidence (re-run 2026-10-06):** `TestTranslatorResolve` subtests `invalid_url_session_and_header_ignored`, `url_prefix_wins_and_strips`, `accept-language_reduced_to_enabled_codes` — PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-02 — process-wide locale leak
- **Source:** `Translator.Locale` reads `towel.Locale(ctx)` only; no package-level current locale.
- **Test evidence (re-run 2026-10-06):** `TestTranslatorConcurrentIsolation` PASS; `TestLocaleResolver` isolation subtest PASS; plugin `go test ./... -race` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-03 — manual cookie as locale
- **Source:** `hasManualFlag` treats any non-empty value as a flag; `rememberedLocale` reads a different cookie name.
- **Test evidence (re-run 2026-10-06):** `TestManualFlagIsNotALocale` (package classes); `TestTranslatorResolve/manual_flag_blocks_accept-language` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-04 — schema / AutoMigrate
- **Source:** gormigrate files `updates/202610060001`–`005`; plugin production Go has no `AutoMigrate`.
- **Test evidence (re-run 2026-10-06):** `TestTranslateTables`, `TestSeedEnPl`, `TestTranslateMigrationsRollbackAndRemigrate` PASS. Gate `--forbidden` refuses production `AutoMigrate`.
- **Disposition:** closed / mitigate.
### T-14.2.1-05 — manage_locales privilege
- **Source:** `controllers/locales.go` `RequiredPermissions` = `golem15.translate.manage_locales`.
- **Test evidence (re-run 2026-10-06):** `TestLocalesAdminForbidden` PASS; `TestTranslateEndToEnd` 403 on list/schema without the grant PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-06 — Locale mass assignment and default lifecycle
- **Source:** `models/locale.go` `Fillable` is `code,name,is_enabled`; `FormBeforeDelete` / `FormBeforeUpdate` / `makeDefault`.
- **Test evidence (re-run 2026-10-06):** `TestLocalesAdminMassAssignment`, `TestLocalesAdminDefaultGuards` covered by the plugin suite (smoke + full admin tests) PASS in `go test ./...`.
- **Disposition:** closed / mitigate.
### T-14.2.1-07 — undeclared field / invalid locale writes
- **Source:** `classes/translatable.go` `validateCall`.
- **Test evidence (re-run 2026-10-06):** `TestInvalidLocaleRejected`, `TestTranslatableGetSet` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-08 — morph/index isolation
- **Source:** `WhereIndex` predicates on `model_type`, `locale`, `item`, `value`.
- **Test evidence (re-run 2026-10-06):** `TestTranslatableIndexAndMorphIsolation` in the plugin `./...` suite PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-09 — nested ML JSON
- **Source:** `modules/cabana/field_ml.go` `liftMLValues` (enabled locale codes, string values, default required).
- **Test evidence (re-run 2026-10-06):** `TestMLFieldTypes`, `TestMLNestedSave` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-10 — TranslationWriter privilege
- **Source:** `cabana.TranslationWriter` comment and `crud.go` `save`: apply after Create/Update, after permission/query/lock checks.
- **Test evidence (re-run 2026-10-06):** `TestML/writer_is_not_invoked_before_query_checks`, `TestML/writer_failure_rolls_back_host_write` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-11 — ML mass assignment
- **Source:** lift before `ProjectWritableFields`; extra nested keys stay blocked.
- **Test evidence (re-run 2026-10-06):** `TestMLNestedSave` PASS (unlifted nested maps empty; extra locale `de` rejected).
- **Disposition:** closed / mitigate.
### T-14.2.1-12 — stored XSS / markdown
- **Source:** `RenderMarkdown` goldmark without `html.WithUnsafe`; rejects script/iframe/event/javascript/vbscript/data. SPA preview interpolates text, no `v-html=`.
- **Test evidence (re-run 2026-10-06):** `TestMarkdownRejectsUnsafeHTML` (script/iframe/event/javascript/vbscript/data) PASS; Vitest `tests/form/MarkdownField.test.ts` 3/3 PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-13 — submodule provenance
- **Source:** host gitlinks `plugins/golem15/{user,translate}`.
- **Test evidence (re-run 2026-10-06):** `TestBootUserTranslate` PASS; `--layout` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-14 — gate repudiation
- **Source:** `scripts/check-phase14.2.1.sh` JSON detector.
- **Test evidence (re-run 2026-10-06):** `--self-test` PASS (fail/skip/zero/no-tests/race/missing-named).
- **Disposition:** closed / mitigate.
### T-14.2.1-15 — concurrent Translator
- **Source:** context-only locale; Rewrite clones the request.
- **Test evidence (re-run 2026-10-06):** `TestTranslatorConcurrentIsolation` PASS; plugin `-race` PASS; `TestLocaleResolver/conflicting_requests_stay_isolated` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-16 — migration rollback
- **Source:** gormigrate RollbackLast ×5 then remigrate on a dedicated database.
- **Test evidence (re-run 2026-10-06):** `TestTranslateMigrationsRollbackAndRemigrate` PASS.
- **Disposition:** closed / mitigate.
### T-14.2.1-17 — generated artifacts
- **Source:** committed `admin/openapi/admin.json`, `schema.d.ts`, `modules/boardwalk/dist`.
- **Test evidence:** `TestMLOpenAPIConformance` PASS (re-run 2026-10-06); gate `--admin` regenerates and diffs.
- **Disposition:** closed / mitigate.
### T-14.2.1-18 — test fixture in production
- **Source:** `party.Register` of `acme.fixture` is `sync.Once` inside tests only.
- **Test evidence (re-run 2026-10-06):** `TestTranslateEndToEndFixtureAbsentFromProduction` matched by `TestTranslateEndToEnd*` in the plugin `./...` run PASS; host `plugins.gen.go` lists only `golem15.user` and `golem15.translate`.
- **Disposition:** closed / mitigate.
### T-14.2.1-19 — ML hydration disclosure
- **Source:** `modules/cabana/field_ml.go` `hydrateMLRecord` walks declared `mltext`/`mlmarkdown` fields only; list projection never calls it; `TranslatedExact` skips D-11 so missing pl stays empty. ShowChild uses the same helper after `loadParent`/`loadChild`.
- **Test evidence (re-run 2026-10-06):** `TestMLHydration` PASS (english-only pl is `""`); `TestRelationChildMLNestedSave` PASS (create/update/show hydrated maps).
- **Disposition:** closed / mitigate.
### T-14.2.1-20 — enabled locale list on schemas
- **Source:** `modules/cabana/http.go` `enabledContentLocales` after Lookup, copied onto protect()'d form and relation schema meta; `FormMeta.EnabledLocales` is `json:"enabledLocales,omitempty"` so list Meta omits it.
- **Test evidence (re-run 2026-10-06):** `admin/tests/form/formState.test.ts` `seeds every enabled locale as an empty string`; `admin/tests/form/MLFields.test.ts` create-empty selectors list en+pl.
- **Disposition:** closed / mitigate.
### T-14.2.1-21 — adopt merge of GET scalars
- **Source:** `admin/src/components/form/formState.ts` `mergeMLValue` overlays a host string onto the seed for ML fields only; password fields still clear.
- **Test evidence (re-run 2026-10-06):** `admin/tests/form/formState.test.ts` `merges a GET host string onto the seed without dropping sibling locales`; `admin/tests/form/MLFields.test.ts` GET-string cases for mltext and mlmarkdown.
- **Disposition:** closed / mitigate.
### T-14.2.1-22 — relation-child nested ML tampering
- **Source:** `modules/cabana/relation_child.go` `CreateChild`/`UpdateChild` call `liftMLValues` on the child form before `fillChild`/`projectOperation`; undeclared locale, non-string, missing default, and nil writer with a nested map are 422 before Fill.
- **Test evidence (re-run 2026-10-06):** `TestRelationChildMLNestedSave` PASS (create+update persist en/pl; locale `de` is 422 and inserts no row).
- **Disposition:** closed / mitigate.
### T-14.2.1-23 — RelationService writer privilege
- **Source:** `modules/cabana/http.go` `relations()` Lookup of `TranslationWriter` matches `crud()`; no new HTTP route; `applyMLTranslations` runs after `loadParent`/`loadChild` and after the child row has a primary key.
- **Test evidence (re-run 2026-10-06):** `TestRelationChildMLNestedSave` PASS; `recordingWriter.WriteTranslated` refuses a zero PK.
- **Disposition:** closed / mitigate.
### T-14.2.1-SC — package installs
- **Source:** no new module versions beyond existing stack pins; plugin `replace` of `sm-user-plugin` points at the host submodule checkout; `gocloud.dev v0.46.0` is the already-decided blob pin. Framework `go.mod` / `admin/package-lock.json` were not changed in this phase.
- **Test evidence:** `git log -- go.mod go.sum` on the framework shows no 14.2.1 commits; plugin go.mod Task 1 added a local replace, not a downloaded package.
- **Disposition:** closed / mitigate.
---
## Deferred surfaces confirmed absent
Gate `--forbidden` (re-run 2026-10-06, PASS) refuses production `manage_messages`, `plugin.Open`/`yaegi`, plugin `AutoMigrate`, `acme.fixture` outside tests, Winter/RainLab table names in production Go, consuming-application names in framework docs, and `v-html=` in `admin/src`.
---
## ASVS L1 mapping (selected)
| ASVS | Control | Evidence |
|------|---------|----------|
| V4.1 | Access control on admin | `TestLocalesAdminForbidden` |
| V5.1.3 | Mass assignment | Locale Fillable + ML lift |
| V5.2 | Sanitization / XSS | `RenderMarkdown` + SPA text interpolation |
| V11.1 | Stored locale injection | enabled-row validation |
| V14.2 | Dependency integrity | T-14.2.1-SC |
---
**Approval:** closed — all high threats mitigated with re-executed named tests.