diff --git a/.planning/quick-batches/261105-oc4/BATCH.json b/.planning/quick-batches/261105-oc4/BATCH.json new file mode 100644 index 00000000..5765d4cb --- /dev/null +++ b/.planning/quick-batches/261105-oc4/BATCH.json @@ -0,0 +1,189 @@ +{ + "schema_version": 1, + "batch_id": "261105-oc4", + "created_at": "2026-11-05T16:31:21.798Z", + "options": {}, + "base_revision": "5aa846c5e85368b534e03a1d19f441bb34de193c", + "items": [ + { + "quick_id": "261105-oc5", + "client_id": null, + "description": "2FA hardening (todos 2026-10-26-activate-by-code-second-factor.md and 2026-10-26-mfa-factor-changes-need-reauth.md): sm-user-plugin ActivateByCode runs CheckLoginGates after VerifyActivationCode (finding F-1); sm-mfa-plugin requires password confirmation on factor-changing routes (WR-05) and vue-demo-app security.vue asks for the password", + "status": "complete", + "depends_on": [], + "planned_files": [ + "../sm-user-plugin/controllers/api_controller.go", + "../sm-user-plugin/controllers/login_gates.go", + "../sm-user-plugin/classes/login_gate.go", + "../sm-user-plugin/login_gate_test.go", + "../sm-user-plugin/README.md", + "../sm-mfa-plugin/go.work", + "../sm-mfa-plugin/go.work.sum", + "../sm-mfa-plugin/classes/config.go", + "../sm-mfa-plugin/config/config.yaml", + "../sm-mfa-plugin/controllers/management.go", + "../sm-mfa-plugin/controllers/webauthn.go", + "../sm-mfa-plugin/factor_password_test.go", + "../sm-mfa-plugin/login_paths_test.go", + "../sm-mfa-plugin/settings_test.go", + "../sm-mfa-plugin/tracer_smoke_test.go", + "../sm-mfa-plugin/webauthn_smoke_test.go", + "../sm-mfa-plugin/webauthn_test.go", + "../sm-mfa-plugin/php_bodies_test.go", + "../sm-mfa-plugin/challenge_smoke_test.go", + "../sm-mfa-plugin/recovery_test.go", + "../sm-mfa-plugin/email_test.go", + "../sm-mfa-plugin/gate_test.go", + "../sm-mfa-plugin/trusted_test.go", + "../sm-mfa-plugin/passwordless_smoke_test.go", + "../sm-mfa-plugin/README.md", + "../sm-demo-app/vue-demo-app/app/lib/twoFactor.ts", + "../sm-demo-app/vue-demo-app/app/lib/twoFactor.test.ts", + "../sm-demo-app/vue-demo-app/app/composables/useTwoFactor.ts", + "../sm-demo-app/vue-demo-app/app/pages/account/security.vue", + "../sm-demo-app/vue-demo-app/i18n/locales/en.json", + "../sm-demo-app/vue-demo-app/i18n/locales/pl.json", + "../sm-demo-app/vue-demo-app" + ], + "directory": null, + "worktree": null, + "dispatched_worktree": null, + "dispatched_branch": null, + "dispatched_base": null, + "wave": 0, + "commit": "b179e6b", + "failure_reason": null + }, + { + "quick_id": "261105-oc6", + "client_id": null, + "description": "Chatchannels pin policy (todo chatchannels-pin-policy.md): sm-chatchannels-plugin PinMessage/UnpinMessage require CanManageContext or an optional contracts.PinPolicy, and verify the message belongs to the channel conversation; sm-demo-app answers the policy from its moderator group", + "status": "complete", + "depends_on": [], + "planned_files": [ + "../sm-chatchannels-plugin/contracts/pin_policy.go", + "../sm-chatchannels-plugin/contracts/access_resolver.go", + "../sm-chatchannels-plugin/classes/channel_service.go", + "../sm-chatchannels-plugin/api/pins.go", + "../sm-chatchannels-plugin/pins_test.go", + "../sm-chatchannels-plugin/testdata/PHP_METHOD_MAP.md", + "../sm-chatchannels-plugin/README.md", + "../sm-demo-app/plugins/demo/classes/chat_context.go", + "../sm-demo-app/plugins/demo/classes/chat_context_test.go", + "../sm-demo-app/plugins/demo/chat.go", + "../sm-demo-app/reset_test.go", + "../sm-demo-app/README.md", + "../sm-demo-app/plugins/golem15/chatchannels", + "../sm-demo-app/chat_test.go" + ], + "directory": null, + "worktree": null, + "dispatched_worktree": null, + "dispatched_branch": null, + "dispatched_base": null, + "wave": 0, + "commit": "a042536", + "failure_reason": null + }, + { + "quick_id": "261105-oc7", + "client_id": null, + "description": "Chatvideo hardening (todos chatvideo-live-membership.md, chatvideo-dm-only-host-hook.md, chatvideo-incoming-handle-leak.md): sm-chatvideo-plugin honours LiveMembership and removes banned participants from the LiveKit room, adds an optional CanStartCall host hook for a DM-only rule; vue-chatvideo-layer useIncomingCalls closes stale session handles", + "status": "complete", + "depends_on": [], + "planned_files": [ + "../sm-chatvideo-plugin/classes/video_service.go", + "../sm-chatvideo-plugin/classes/authorizer.go", + "../sm-chatvideo-plugin/classes/livekit.go", + "../sm-chatvideo-plugin/classes/revoked_access.go", + "../sm-chatvideo-plugin/boot.go", + "../sm-chatvideo-plugin/live_membership_test.go", + "../sm-chatvideo-plugin/contracts/call_policy.go", + "../sm-chatvideo-plugin/contracts/call_policy_test.go", + "../sm-chatvideo-plugin/api/errors.go", + "../sm-chatvideo-plugin/lang/en/lang.yaml", + "../sm-chatvideo-plugin/lang/pl/lang.yaml", + "../sm-chatvideo-plugin/call_policy_test.go", + "../sm-chatvideo-plugin/README.md", + "../sm-chatvideo-plugin/plugin.yaml", + "../sm-demo-app/plugins/golem15/chatvideo", + "../sm-demo-app/plugins/demo/video.go", + "../sm-demo-app/plugins/demo/plugin.go", + "../sm-demo-app/video_test.go", + "../sm-demo-app/config/golem15.chatvideo.yaml", + "../sm-demo-app/README.md", + "../vue-chatvideo-layer/app/composables/useIncomingCalls.ts", + "../vue-chatvideo-layer/vitest.config.ts", + "../vue-chatvideo-layer/tests/useIncomingCalls.test.ts", + "../vue-chatvideo-layer/README.md" + ], + "directory": null, + "worktree": null, + "dispatched_worktree": null, + "dispatched_branch": null, + "dispatched_base": null, + "wave": 1, + "commit": "f996156", + "failure_reason": null + }, + { + "quick_id": "261105-oc8", + "client_id": null, + "description": "lagoon/attach StaticHandler and StaticHandlerPublic send the stored Content-Disposition and X-Content-Type-Options nosniff (todo 2026-10-28-attach-static-handler-disposition.md)", + "status": "complete", + "depends_on": [], + "planned_files": [ + "modules/lagoon/attach/static.go", + "modules/lagoon/attach/static_test.go", + "modules/lagoon/attach/lifecycle_test.go", + "modules/lagoon/README.md", + "docs/services/storage.md", + "docs/database/attachments.md", + "modules/surf/uploads.go" + ], + "directory": null, + "worktree": null, + "dispatched_worktree": null, + "dispatched_branch": null, + "dispatched_base": null, + "wave": 0, + "commit": "27c98bf", + "failure_reason": null + }, + { + "quick_id": "261105-oc9", + "client_id": null, + "description": "sm-demo-app hardening (todos check-local-jwt-out-of-argv.md, demo-chat-members-read-only.md, demo-nginx-livekit-rtc-only.md, demo-seed-moderation-lists-once.md, demo-vapid-runbook-pipefail.md)", + "status": "complete", + "depends_on": [], + "planned_files": [ + "../sm-demo-app/plugins/demo/chat_api.go", + "../sm-demo-app/plugins/demo/classes/members.go", + "../sm-demo-app/plugins/demo/console/seed.go", + "../sm-demo-app/plugins/demo/console/seed_chat.go", + "../sm-demo-app/plugins/demo/console/seed_friends.go", + "../sm-demo-app/plugins/demo/plugin.go", + "../sm-demo-app/plugins/demo/updates/updates.go", + "../sm-demo-app/go.mod", + "../sm-demo-app/chat_test.go", + "../sm-demo-app/chat_smoke_test.go", + "../sm-demo-app/seed_chat_test.go", + "../sm-demo-app/README.md", + "../sm-demo-app/scripts/check-moderation.sh", + "../sm-demo-app/scripts/check-local.sh", + "../sm-demo-app/deploy/nginx/demo.summercms.io.conf", + "../sm-demo-app/scripts/check-deploy.sh", + "../sm-demo-app/scripts/check-live.sh", + "../sm-demo-app/DEPLOY.md" + ], + "directory": null, + "worktree": null, + "dispatched_worktree": null, + "dispatched_branch": null, + "dispatched_base": null, + "wave": 2, + "commit": "9595606", + "failure_reason": null + } + ] +} diff --git a/.planning/quick/261105-oc5-2fa-hardening/261105-oc5-PLAN.md b/.planning/quick/261105-oc5-2fa-hardening/261105-oc5-PLAN.md new file mode 100644 index 00000000..b523ab87 --- /dev/null +++ b/.planning/quick/261105-oc5-2fa-hardening/261105-oc5-PLAN.md @@ -0,0 +1,288 @@ +--- +phase: quick-261105-oc5 +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - ../sm-user-plugin/controllers/api_controller.go + - ../sm-user-plugin/controllers/login_gates.go + - ../sm-user-plugin/classes/login_gate.go + - ../sm-user-plugin/login_gate_test.go + - ../sm-user-plugin/README.md + - ../sm-mfa-plugin/go.work + - ../sm-mfa-plugin/go.work.sum + - ../sm-mfa-plugin/classes/config.go + - ../sm-mfa-plugin/config/config.yaml + - ../sm-mfa-plugin/controllers/management.go + - ../sm-mfa-plugin/controllers/webauthn.go + - ../sm-mfa-plugin/factor_password_test.go + - ../sm-mfa-plugin/login_paths_test.go + - ../sm-mfa-plugin/settings_test.go + - ../sm-mfa-plugin/tracer_smoke_test.go + - ../sm-mfa-plugin/webauthn_smoke_test.go + - ../sm-mfa-plugin/webauthn_test.go + - ../sm-mfa-plugin/php_bodies_test.go + - ../sm-mfa-plugin/challenge_smoke_test.go + - ../sm-mfa-plugin/recovery_test.go + - ../sm-mfa-plugin/email_test.go + - ../sm-mfa-plugin/gate_test.go + - ../sm-mfa-plugin/trusted_test.go + - ../sm-mfa-plugin/passwordless_smoke_test.go + - ../sm-mfa-plugin/README.md + - ../sm-demo-app/vue-demo-app/app/lib/twoFactor.ts + - ../sm-demo-app/vue-demo-app/app/lib/twoFactor.test.ts + - ../sm-demo-app/vue-demo-app/app/composables/useTwoFactor.ts + - ../sm-demo-app/vue-demo-app/app/pages/account/security.vue + - ../sm-demo-app/vue-demo-app/i18n/locales/en.json + - ../sm-demo-app/vue-demo-app/i18n/locales/pl.json + - ../sm-demo-app/vue-demo-app +autonomous: true +requirements: + - quick-261105-oc5 + +must_haves: + truths: + - "With a login gate that challenges, POST /_user/api/v1/activate-by-code with a valid code answers the gate's status and body (for golem15.mfa: 200 {two_factor_required, challenge_token, available_methods, expires_at}) and no token or session cookie; the account is activated either way (F-1, per D-01)" + - "Without a gate, or when every gate allows, activate-by-code answers exactly as before ({message: Account activated, user, token} or the HttpOnly cookie with golem15.user.jwt.token_cookie); a gate error answers the opaque 500 with no token and the account stays activated (D-01)" + - "In sm-mfa-plugin, DELETE /2fa/totp, DELETE /2fa/email, DELETE /2fa/webauthn/{id}, POST /2fa/webauthn/register, and POST /2fa/totp/confirm for a user who already has an enabled second factor answer 403 {\"error\":\"Password confirmation required.\"} without the right password and change nothing (WR-05, per D-02, D-03)" + - "A wrong password on those routes is a failed login attempt in golem15.user's throttle, and a suspended or banned account is refused even the right password, exactly as on the recovery-code routes (D-04)" + - "The first totp/confirm of a user with no enabled factor, email/enable, totp/setup, webauthn/register/options and the trusted-device routes keep their current contract (no password); validation 422 bodies keep PHP's shape and order (D-03)" + - "golem15.mfa.factor_changes_require_password (default true) set to false restores PHP's password-free contract on all five routes (D-05)" + - "Two concurrent password-free totp/confirm calls of a user with no factor end with exactly one 200; the other answers 403 and the first secret stays stored (D-03)" + - "The demo site's /account/security asks for the password before turning the authenticator app off, sends it in the JSON body of DELETE /2fa/totp (never in the query string), shows a localized wrong-password message on the 403, and asks for it in the setup form when another factor is already on (D-06)" + artifacts: + - path: ../sm-user-plugin/controllers/api_controller.go + provides: "ActivateByCode consults CheckLoginGates after VerifyActivationCode and before mintFor" + - path: ../sm-user-plugin/login_gate_test.go + provides: "TestActivateByCodeGates (Postgres)" + - path: ../sm-mfa-plugin/controllers/management.go + provides: "password helper split (passwordFrom, checkPassword) reused by confirmPassword and the five factor-changing handlers" + - path: ../sm-mfa-plugin/classes/config.go + provides: "Config.FactorChangesRequirePassword (factor_changes_require_password, default true)" + - path: ../sm-mfa-plugin/factor_password_test.go + provides: "TestFactorChangesNeedPassword (Postgres)" + - path: ../sm-mfa-plugin/login_paths_test.go + provides: "activation_by_code_is_challenged subtest and the updated tokenMints reason" + - path: ../sm-demo-app/vue-demo-app/app/pages/account/security.vue + provides: "password step for turning TOTP off and for confirming TOTP beside another factor" + key_links: + - from: "sm-user-plugin controllers.ActivateByCode" + to: "controllers.CheckLoginGates with classes.LoginSourcePassword" + via: "the gate answer replaces the token response; sm-mfa-plugin's existing Gate challenges source password" + - from: "sm-mfa-plugin TOTPDisable/EmailDisable/WebAuthnRemove/WebAuthnRegister/TOTPConfirm" + to: "userclasses.CheckAndRecordLogin and userclasses.ThrottleStates" + via: "the shared checkPassword helper (the body of today's confirmPassword)" + - from: "vue-demo-app useTwoFactor.disableTotp(password)" + to: "DELETE /_user/api/v1/2fa/totp JSON body {password}" + via: "$fetch body" +--- + + +Close the two deferred 2FA hardening items: finding F-1 (sm-user-plugin's activate-by-code mints a session without asking the login gates, so with `require_activation: false` the mailed activation code stands in for an enabled second factor) and review finding WR-05 (sm-mfa-plugin lets a bare session JWT remove, replace or add second factors). Then the demo frontend asks for the password where the plugin now requires it. + +Purpose: a stolen session or a mailbox alone must not get past or rewrite a user's second factor. Both plugins are shared core plugins (sm-demo-app and sm-bm-app mount golem15.mfa; fonoteka.go, sm-demo-app and sm-bm-app mount golem15.user), so the fix is additive where it can be, and the one contract change (WR-05) is explicit in the todo and comes with a config knob. The only frontend that calls the changed routes is sm-demo-app's vue-demo-app (checked at planning: sm-bm-app's frontend and fonoteka call none of them). + +Output: one commit in sm-user-plugin, two in sm-mfa-plugin (workspace repoint, then the fix), one in vue-demo-app, one vue-demo-app pointer bump in sm-demo-app. No tag, push, deploy, go.mod version or plugin submodule bump. + +Source todos (read them first): `.planning/todos/pending/2026-10-26-activate-by-code-second-factor.md` (g15office yfadd1mb) and `.planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md` (g15office krt3vrts). Background: `.planning/phases/19.6-two-factor-auth-and-device-flow-in-sm-user-plugin/19.6-SECURITY-REVIEW.md` "F-1" (option (a) is the one implemented here) and `19.6-REVIEW.md` "WR-05". + +Decisions for this item (from the item description, the todos and the coordinator notes), cited below as D-01..D-07: +- D-01 (F-1): `ActivateByCode` calls `CheckLoginGates` after `VerifyActivationCode` succeeds, with `Source: classes.LoginSourcePassword` (the mailed code is the first factor here), and answers the gate's status and body instead of the token. The account stays activated. No new login source: sm-mfa-plugin's existing gate (every released version) already challenges source password, so the user-plugin patch alone closes F-1 in every app, and no cross-plugin compile dependency appears (19.6-SECURITY-REVIEW F-1 option (a)). +- D-02 (WR-05): password confirmation on `DELETE /2fa/totp`, `DELETE /2fa/email`, `DELETE /2fa/webauthn/{id}`, `POST /2fa/webauthn/register` (every registration, passkey or not) and `POST /2fa/totp/confirm` when it would replace or add beside an enabled factor. The refusal is the existing 403 `{"error":"Password confirmation required."}`. +- D-03: contract edges. Validation (422) and "method not available" (403) answers come before the password check, so PHP's bodies for malformed requests stay the same. Routes not named in D-02 are unchanged. Claude's discretion: `totp/confirm` asks for the password when the user has ANY enabled second factor (not only TOTP). The todo says "when TOTP is already on"; this is a superset of it. Adding a TOTP secret beside an enabled email or WebAuthn factor gives a session thief who also phished the password a factor they control, which is the same risk as replacing one. A user with no factor still enables the first one password-free, as before (their account is protected only by the password anyway). The decision and the factor write run in one transaction under a per-user row lock, so a race cannot slip a second secret past the check. +- D-04: the check is today's `confirmPassword` path: every non-empty password is recorded through `userclasses.CheckAndRecordLogin` for the client address, a banned or suspended account is refused even the right password, and a missing password is refused without recording (WR-06 behaviour, 0a08070). +- D-05 (config knob, Claude's discretion following the coordinator's "keep default behaviour where a config knob makes sense"): new config-only key `golem15.mfa.factor_changes_require_password`, default `true` (the todo asks for the requirement), env `SUMMER_GOLEM15__MFA__FACTOR_CHANGES_REQUIRE_PASSWORD`. `false` restores PHP's password-free contract for an application whose ported PHP frontend cannot send the password yet. The README states that this re-opens WR-05. It is not on the admin settings row (no migration), like `failure_cap.*`. +- D-06 (frontend): vue-demo-app is the submodule checkout `../sm-demo-app/vue-demo-app` (remote golem15/vue-demo-app, HEAD fe5d38d at planning). The sibling `../vue-demo-app` is a stale older checkout: do not edit it. The page asks for the password in the page, not through window.confirm, sends it only in the JSON body, and clears it after every attempt. Sending a password to today's released sm-mfa-plugin v0.1.3 is harmless (it ignores the field), so the frontend change is safe to ship ahead of the plugin release. +- D-07 (release discipline): each repo commits on its own, with explicit paths and no co-author trailer. Commit messages carry no planning IDs (they feed the changelog). No tag, push or deploy. sm-demo-app's README requires `go.mod` to pin tags and each plugin submodule to sit on its pinned tag (`scripts/check-pins.sh`), so the plugin submodule and go.mod bumps in sm-demo-app, sm-bm-app and fonoteka.go wait until the user tags the plugins. They are listed as follow-ups in the SUMMARY. Only the vue-demo-app gitlink is bumped here. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/CLAUDE.md +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/todos/pending/2026-10-26-activate-by-code-second-factor.md +@/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md + +Repos (absolute; paths in files_modified are relative to summercms.go): +- U = /media/nvme/dev/golem15/summercms.io/summercms/sm-user-plugin (master, HEAD 97f83cc at planning, clean) +- M = /media/nvme/dev/golem15/summercms.io/summercms/sm-mfa-plugin (master, HEAD 544017c at planning, clean) +- F = /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/vue-demo-app (master, HEAD fe5d38d, clean) +- D = /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app (clean; vue-demo-app is its submodule) +Several sessions work in these repos at once (batch items oc6 and oc9 also touch sm-demo-app). Run `git status --short` in a repo before editing it and again before staging, and stage only the paths this plan changes. + +Facts gathered during planning (do not re-derive): +- U `controllers/api_controller.go` `ActivateByCode` (~line 594): sessionDeps, sessionCookie, readFields, validate code, splitCode, IsAlreadyActivated, `VerifyActivationCode(codeContext(...), gdb, id, code)` returns `(*models.User, bool)`, then `mintFor`, `apiArray`, `writeSession(w, app, cc, token, {"message":"Account activated","user":…})`. `Login` (~lines 133-160) shows the gate-call pattern to copy: `CheckLoginGates(ctx, app, classes.LoginAttempt{User, Source, Fields, Request})`; on error `pluginLog.Error("golem15.user: login gate", "user_id", …, "err", err)` and `writeOpaque500`; on a result, status 0 means 200 and `writeJSON(w, status, gate.Body)`. +- U `controllers/login_gates.go` `CheckLoginGates` doc says "Password Login calls it; a social-login plugin calls it". U `classes/login_gate.go` documents `LoginSourcePassword` as "POST /_user/api/v1/login" and `LoginAttempt.Fields` as "it holds the password on a password login". +- U test harness: `login_gate_test.go` has `countingGate` (records attempts, answers its result) and `gatedEnv(t, result) (*adminEnv, *countingGate)`; `login_gate_smoke_test.go` has `gateFunc` (a func implementing LoginGate, for an error gate). `adminEnv` (admin_harness_test.go) has `gdb`, `seedUser(t, label, mutate)` (password "correct-horse"), `call(t, method, path, token, body)`. `account_test.go TestActivateByCode` packs the code as `strconv.FormatUint(uint64(user.ID), 10) + "!" + code` from `classes.IssueActivationCode(ctx, db, user)`. `token_cookie_test.go TestTokenCookie` covers activate-by-code with the cookie. +- U baseline (planning run): `go vet ./...` clean; `go test -short -count=1 ./...` fails only `TestRegisterCORSPath` ("config/http.yaml not found"; it needs an application checkout, known). Docker works on this host; `-short` still runs some Postgres tests. +- M `go.work` replaces sm-user-plugin with `../fonoteka.go/plugins/golem15/user`, a stale checkout (a1b78b8). M's tests (`TestEveryLoginPathIsGated` compiles the user plugin and scans its source through `go list -m`) therefore never see a U change. At planning, M built, vetted and passed its full suite (`go test -count=1 ./...`, about 55 s with Docker) against U's current checkout through a temporary GOWORK file that replaced sm-user-plugin with ../sm-user-plugin. +- M `controllers/management.go`: `confirmPassword(w, r, app, db, user)` reads `password` from the query, else from `readFields(r)["password"].(string)` (raw, never trimmed), refuses an empty one with 403, refuses banned/suspended (`userclasses.ThrottleStates`), checks `bouncer.CheckPassword`, records through `userclasses.CheckAndRecordLogin(ctx, db, user, classes.ClientIP(app, r), ok)` with `ctx := userclasses.ContextWithThrottle(r.Context(), app)`. `readFields` consumes the body, so a handler that already read its fields must not call it again. `field()` trims, so never use it for the password. `disableHandler` serves TOTPDisable and EmailDisable. `TOTPConfirm` runs signedIn, readFields, validate(totpConfirmRules), MethodAvailable, VerifyTOTP, then a transaction with `classes.EnableMethod` (an upsert that overwrites an enabled secret), `EnsureRecoveryCodes`, `Fire(EnabledEvent)`. +- M `controllers/webauthn.go`: `WebAuthnRegister` (~line 125) runs signedIn, readFields, validate(webAuthnRegisterRules), MethodAvailable, jsonField attestation, cache, a transaction with FinishRegistration, EnableMethod, EnsureRecoveryCodes, Fire. `WebAuthnRemove` (~line 202) parses `{id}` (404 on bad id) then a transaction. +- M `classes/methods.go`: `EnabledMethods(ctx, db, userID) ([]string, error)`, `MethodEnabled(ctx, db, userID, method)`. M `classes/config.go`: `Config` struct plus `LoadConfig(app)`. Booleans are read with `c.Bool`, which gives false for a missing key, so a default-true key needs `c.Has` first. `Effective` embeds `Config`; `overlay` lays the settings row over it and must not touch the new field. +- M tests that call the five routes today (all must pass after the change): `tracer_smoke_test.go` (`enableTOTP` helper ~line 73, a direct confirm ~line 129), `webauthn_smoke_test.go` (`registerKey` helper ~line 173 and direct register/delete calls ~220-328), `webauthn_test.go` (~296-341, ~454), `php_bodies_test.go` (~116-154), `challenge_smoke_test.go:315`, `recovery_test.go` (~90-113: a second confirm and a confirm after email), `email_test.go:162`, `gate_test.go:171`, `trusted_test.go` (~388-426), `passwordless_smoke_test.go:143`. Harness users all have password `testPassword` (harness_test.go:38). `settings_test.go TestSettings` covers LoadConfig through `configApp(t, map)`. +- M `login_paths_test.go`: `tokenMints` has the entry `"controllers/api_controller.go ActivateByCode mintFor": "accepted debt of plan 01; see 19.6-SECURITY-REVIEW.md finding F-1"`; `TestEveryLoginPathIsGated` has subtests mint_sites, password_is_challenged, social_sign_in, other_sources_carry_their_own_proof, passkey_only_with_its_assertion, device_poll_only_after_an_authorize, registration_mints_without_a_gate. Helpers: `e.user(t)`, `e.enableTOTP(t, user)`, `e.call`, `e.verify(t, challenge, "totp", nextCode(t, secret))`, `freshIP()`, `mustStatus`. +- M README: Routes table rows ~60-70, Passkeys ~line 87 and Security notes ~line 164 say the management routes need only the session token (PHP's contract), Configuration table ~89-108 ("The settings screen overrides all of them but `issuer` and `failure_cap.*`"), Testing ~181-189. +- F: `app/composables/useTwoFactor.ts` (`confirmTotp(secret, code)`, `disableTotp()` with DELETE and no body), `app/lib/twoFactor.ts` (pure helpers, tests in `app/lib/twoFactor.test.ts`, 14 passing), `app/pages/account/security.vue` (turnOff uses `window.confirm(t('security.turnOffConfirm'))`; the setup form shows only while TOTP is off; errors go through `authErrors(e, t, fallback)` from `app/composables/useDemoAuth.ts`, which surfaces `data.error` verbatim, so the English 403 text would show on /pl), `i18n/locales/en.json` and `pl.json` "security" blocks (~line 174). `pnpm test` = vitest in Node (pure libs only); `pnpm build` = nuxt build. +- D README: "Push vue-demo-app before the pointer bump in this repository"; a pointer bump commit looks like `6686216 chore: bump vue-demo-app (SEO layers, Polish under /pl/)`. + + + + + + Task 1: sm-user-plugin: activate-by-code asks the login gates after the code verifies (F-1) + ../sm-user-plugin/controllers/api_controller.go, ../sm-user-plugin/controllers/login_gates.go, ../sm-user-plugin/classes/login_gate.go, ../sm-user-plugin/login_gate_test.go, ../sm-user-plugin/README.md + U controllers/api_controller.go (Login ~40-185 and ActivateByCode ~590-645), controllers/login_gates.go, classes/login_gate.go, login_gate_test.go (countingGate, gatedEnv, TestLoginGateOrder), login_gate_smoke_test.go (gateFunc), account_test.go TestActivateByCode (~95-137), README.md bullets "Registration and activation" (line 19), "Login gates" (line 21), the classes/controllers API reference lines on CheckLoginGates and the LoginSource constants (~236, ~256) and the Testing paragraph on the login-gate seam (~360) + + - TestActivateByCodeGates (Postgres, skipped under -short like TestLoginGateOrder), each case with its own not-activated user (seedUser with IsActivated false) and a fresh activation code: + - challenge gate (gatedEnv with a {two_factor_required…} result): POST /_user/api/v1/activate-by-code answers 200 with exactly the gate body, no "token" key and no Set-Cookie; the gate ran once and saw Source == classes.LoginSourcePassword, the right User.ID, Fields["code"] == the packed code, a non-nil Request; the users row is_activated is true afterwards. + - gate with Status 401 and its own body: the answer carries that status and body, and the account is still activated. + - allowing gate (nil result): the answer is 200 {message: "Account activated", user, token} as before and the gate ran once. + - erroring gate (gateFunc returning an error): opaque 500, no token, account still activated, and a second activate-by-code with the same code answers the Winter error page because the account is already activated (unchanged IsAlreadyActivated behaviour). + - invalid or expired code: the gate never runs (count 0) and the 422 body is unchanged. + - existing TestActivateByCode, TestTokenCookie and TestLoginGateOrder stay green unchanged. + + +Write TestActivateByCodeGates in login_gate_test.go first, run it red, then implement (D-01). + +In ActivateByCode, after `VerifyActivationCode` returns a verified user and before `mintFor`, call `CheckLoginGates(r.Context(), app, classes.LoginAttempt{User: user, Source: classes.LoginSourcePassword, Fields: fields, Request: r})` (the fields the handler already read; do not read the body again). On an error, log it with `pluginLog.Error("golem15.user: login gate", "user_id", user.ID, "err", err)` and answer `writeOpaque500`. On a non-nil result, answer `writeJSON(w, status, gate.Body)` with status 0 mapped to 200, the same as Login. On both paths, mint nothing and write no session cookie. The activation `VerifyActivationCode` already committed stays as it is (the account is activated whatever the gate answers). With no result, the handler continues exactly as today. Update ActivateByCode's doc comment: it activates, consults the login gates (as password Login does, with source password, because the mailed code is the first factor here), then mints; a gate answer replaces the token response. + +Doc comments: in classes/login_gate.go, `LoginSourcePassword` covers POST /_user/api/v1/login and POST /_user/api/v1/activate-by-code (the mailed activation code), and `LoginAttempt.Fields` holds the password on a password login and the activation code on an activation. In controllers/login_gates.go, `CheckLoginGates` is called by password Login, activate-by-code and a social-login plugin. + +README.md (same commit, per CLAUDE.md documentation rules): in "Registration and activation", `POST /activate-by-code` answers with a token unless a login gate holds the sign-in back (for example a second-factor challenge, which the user completes through the gate's plugin), and the account is activated either way. In "Login gates", list activate-by-code as a caller (after the code verifies, source password), and add the reason: with `require_activation: false` an account can enable a second factor before it activates, so the mailed code must not stand in for that factor. Update the CheckLoginGates API reference line and the Testing paragraph to name TestActivateByCodeGates. + +Commit in U (D-07): `git -C U status --short` first, then stage the five paths explicitly and commit with a message like "fix: activate-by-code asks the login gates before it signs in". No planning IDs, no trailer. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-user-plugin && go build ./... && go vet ./... && go test -count=1 -run 'TestActivateByCodeGates|TestActivateByCode$|TestTokenCookie|TestLoginGates$|TestLoginGateOrder|TestLoginGateSmoke|TestLoginGatesSmoke' . && N=$(go test -short -count=1 ./... 2>&1 | grep -E '^--- FAIL' | grep -vc 'TestRegisterCORSPath'); test "$N" -eq 0 + + TestActivateByCodeGates passes with Docker and covers the challenge, refusal, allow, error and bad-code cases. The existing activate-by-code, token-cookie and gate tests pass unchanged. `go test -short ./...` has no failure other than the known TestRegisterCORSPath. One commit in sm-user-plugin with code, doc comments and README. + + + + Task 2: sm-mfa-plugin: factor changes ask for the password (WR-05) and activate-by-code is pinned as gated (F-1) + ../sm-mfa-plugin/go.work, ../sm-mfa-plugin/go.work.sum, ../sm-mfa-plugin/classes/config.go, ../sm-mfa-plugin/config/config.yaml, ../sm-mfa-plugin/controllers/management.go, ../sm-mfa-plugin/controllers/webauthn.go, ../sm-mfa-plugin/factor_password_test.go, ../sm-mfa-plugin/login_paths_test.go, ../sm-mfa-plugin/settings_test.go, ../sm-mfa-plugin/tracer_smoke_test.go, ../sm-mfa-plugin/webauthn_smoke_test.go, ../sm-mfa-plugin/webauthn_test.go, ../sm-mfa-plugin/php_bodies_test.go, ../sm-mfa-plugin/challenge_smoke_test.go, ../sm-mfa-plugin/recovery_test.go, ../sm-mfa-plugin/email_test.go, ../sm-mfa-plugin/gate_test.go, ../sm-mfa-plugin/trusted_test.go, ../sm-mfa-plugin/passwordless_smoke_test.go, ../sm-mfa-plugin/README.md + M go.work, controllers/management.go (whole file), controllers/webauthn.go (WebAuthnRegister, WebAuthnRemove), controllers/helpers.go (readFields, field, signedIn, writeError), classes/config.go (Config, LoadConfig, Effective, overlay), classes/methods.go (EnabledMethods, EnableMethod), config/config.yaml, login_paths_test.go, settings_test.go TestSettings, harness_test.go (bootMFA, user, sessionToken, call), the confirm-password throttle test (grep TestPasswordConfirmationThrottle) for how a recorded failed attempt is asserted, README.md sections Routes, Passkeys, Configuration, Security notes, Testing + + - Workspace: after the repoint, `go list -m -f '{{.Dir}}' git.golem15.com/golem15/sm-user-plugin` in M prints the sm-user-plugin checkout, and the whole suite passes before any WR-05 change (baseline). + - TestSettings additions (no Docker): LoadConfig without the key gives FactorChangesRequirePassword true; key false gives false; env SUMMER_GOLEM15__MFA__FACTOR_CHANGES_REQUIRE_PASSWORD=false gives false; a settings row never changes it. + - TestFactorChangesNeedPassword (Postgres), one user per case, enabled through the existing helpers: + - DELETE /2fa/totp, DELETE /2fa/email and DELETE /2fa/webauthn/{id} without a password: 403 {"error":"Password confirmation required."}, and the method or credential is still there. With a wrong JSON-body password: the same 403, a failed attempt recorded for the user (assert as TestPasswordConfirmationThrottle does), and nothing changed. With the right password: the PHP 200 body ({message, status}) and the factor gone. + - POST /2fa/webauthn/register without a password: 403 and no new credential, also when the user has no factor at all and also with passwordless true. With the right password: 200 {credential, message, status}. + - POST /2fa/totp/confirm: a user with no factor enables TOTP without a password (200 with recovery_codes, unchanged). With TOTP on, a new secret without a password gives 403 and the stored secret is unchanged (decrypt or compare the ciphertext before and after); with the password gives 200. A user with only email on adding TOTP without a password gives 403, with the password 200. + - Ordering: a malformed totp/confirm or webauthn/register body without a password still answers the PHP 422 body. A method that is not available still answers its 403 "… is not available." body. + - Suspended account: after attempt_limit wrong passwords, the right password on DELETE /2fa/totp answers 403 and TOTP stays on. + - Race: two parallel password-free totp/confirm calls (different secrets, each with a valid code) for a user with no factor end with exactly one 200 and one 403, and the stored secret is the winner's. + - Knob: with golem15.mfa.factor_changes_require_password false (boot env or Config.Set), every route above works without a password (PHP contract). + - TestEveryLoginPathIsGated gains activation_by_code_is_challenged: a user with TOTP is set is_activated false and gets a code from userclasses.IssueActivationCode. POST /_user/api/v1/activate-by-code with "{id}!{code}" answers 200 two_factor_required true with no "token". The users row is activated. Completing the challenge with e.verify and nextCode mints a token. The tokenMints reason for ActivateByCode no longer says accepted debt. + + +Step A, workspace (own commit): `git -C M status --short`. In M go.work, change the sm-user-plugin replacement from ../fonoteka.go/plugins/golem15/user to ../sm-user-plugin, the plugin's own repository, which now holds Task 1. The fonoteka checkout is stale and would hide Task 1 from M's tests. Run `go build ./... && go vet ./... && go test -count=1 ./...` in M (the baseline must be green at this point, as it was at planning). Commit go.work (and go.work.sum only if Go changed it) as "chore: develop against the sm-user-plugin checkout". + +Step B, tests first: add the TestSettings rows, write factor_password_test.go (TestFactorChangesNeedPassword) and the login_paths_test.go subtest and reason, and run them red. + +Step C, implementation (D-02..D-05): +- classes/config.go: add `FactorChangesRequirePassword bool` to Config with a doc comment naming the key, its default and what false re-opens. LoadConfig defaults it to true and reads `c.Bool` only when `c.Has(ConfigPrefix + "factor_changes_require_password")`. Do not touch overlay (config-only key). config/config.yaml: add the key, default true, with a comment in the file's style ("(new)"; the env var name; false restores PHP's password-free factor changes for a frontend that cannot send the password, and re-opens the stolen-session exposure the README describes). +- controllers/management.go: split confirmPassword without changing its behaviour for the recovery-code routes. `passwordFrom(r, fields)` returns the query password, else the raw `fields["password"]` string (never trimmed). `checkPassword(w, r, app, db, user, password) bool` holds today's body: refuse empty, refuse banned or suspended, CheckPassword, CheckAndRecordLogin, refuse 403. confirmPassword becomes `checkPassword(w, r, app, db, user, passwordFrom(r, readFields(r)))`, keeping today's laziness: read the body only when the query has no password. Add a small helper `requiresPassword(eff)` that reports `eff.FactorChangesRequirePassword`. +- disableHandler (TOTP, email): after signedIn, if required, `confirmPassword`; then the existing transaction. +- TOTPConfirm: after validate and the availability check, and before VerifyTOTP: if required, load `classes.EnabledMethods(ctx, db, user.ID)`. When any is enabled, run checkPassword with passwordFrom(r, fields) (the fields already read) and remember confirmed. Inside the existing transaction, before EnableMethod, when required and not confirmed: lock the user's row (`SELECT id FROM users WHERE id = ? FOR UPDATE` on tx, the pattern the user plugin already uses) and re-read `EnabledMethods` on tx. If any method is now enabled, return a package-level sentinel error (for example errPasswordNeeded), roll back, and answer the 403 "Password confirmation required." (not a 500). When confirmed, take the same lock first too, so that factor writes of one user serialize. +- controllers/webauthn.go WebAuthnRegister: after validate and the availability check, and before attestation and FinishRegistration: if required, checkPassword with passwordFrom(r, fields). WebAuthnRemove: if required, confirmPassword right after signedIn, before the id parse. +- Update the handlers' doc comments: each names the password requirement and the knob. +- Existing tests: send `"password": testPassword` wherever a call now needs it. That is the `enableTOTP` and `registerKey` helpers and the direct calls listed in the context section, with the body field, never the query. `enableTOTP` may always send it, because a password the route does not need is ignored. Keep every PHP-shape assertion as it is. Only add the password field, and for the 422 cases leave it out so they still prove validation comes first. +- login_paths_test.go: set the ActivateByCode tokenMints reason to "activation code as first factor: consults the login gates first (F-1)" and add the activation_by_code_is_challenged subtest described in behavior. + +README.md (same commit): Routes rows for the five routes gain `password` (JSON body; the query also works, as on the recovery-code routes) and the 403. Add a Configuration row for factor_changes_require_password and note it among the keys the settings screen does not override. Rewrite the Passkeys and Security notes paragraphs that say the management routes need only the session token: the five routes need the password (totp/confirm only when the user already has a second factor), the confirmations go through golem15.user's failed-login throttle, the knob restores PHP's contract and re-opens the exposure, and the remaining password-free paths are the first factor of an account with none, email/enable (codes go to the account's own address) and register/options (stores nothing). Add TestFactorChangesNeedPassword and the new subtest to Testing. + +Commit (D-07): `git -C M status --short`, stage exactly the changed paths of this step, message like "fix: changing a second factor asks for the password". No planning IDs, no trailer. This is the contract change of the todo. The SUMMARY flags it for the release note and suggests a minor version (v0.2.0) when the user tags. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-mfa-plugin && test "$(go list -m -f '{{.Dir}}' git.golem15.com/golem15/sm-user-plugin)" = /media/nvme/dev/golem15/summercms.io/summercms/sm-user-plugin && go build ./... && go vet ./... && go test -short -count=1 ./... && go test -count=1 ./... + + M builds against ../sm-user-plugin. The full suite passes with Docker: TestFactorChangesNeedPassword, TestEveryLoginPathIsGated/activation_by_code_is_challenged, the TestSettings rows and every existing test (the PHP body tests unchanged except for the added password fields). Two commits in sm-mfa-plugin: the workspace repoint, then the fix with README. + + + + Task 3: vue-demo-app: the security page asks for the password; bump the pointer in sm-demo-app + ../sm-demo-app/vue-demo-app/app/lib/twoFactor.ts, ../sm-demo-app/vue-demo-app/app/lib/twoFactor.test.ts, ../sm-demo-app/vue-demo-app/app/composables/useTwoFactor.ts, ../sm-demo-app/vue-demo-app/app/pages/account/security.vue, ../sm-demo-app/vue-demo-app/i18n/locales/en.json, ../sm-demo-app/vue-demo-app/i18n/locales/pl.json, ../sm-demo-app/vue-demo-app + F app/pages/account/security.vue, app/composables/useTwoFactor.ts, app/lib/twoFactor.ts, app/lib/twoFactor.test.ts, app/composables/useDemoAuth.ts (formErrors, authErrors), i18n/locales/en.json and pl.json "security" blocks, README.md "Test and build"; D README.md "Remotes" bullet + + - twoFactor.test.ts: a new pure helper `isPasswordRefusal(e)` is true for a fetch-error-like object with statusCode (or status) 403 and data.error "Password confirmation required." It is false for a 403 with another error (for example "TOTP is not available."), for a 401 or 422, for null, a string or an object without data. + - Existing 14 tests stay green. + + +Write the helper tests first, then implement (D-06). `git -C F status --short` before editing. Edit only F, never the stale sibling ../vue-demo-app. + +app/lib/twoFactor.ts: export the constant PASSWORD_CONFIRMATION_REQUIRED (the plugin's exact 403 error text) and `isPasswordRefusal(e: unknown): boolean` as described in behavior. Read `statusCode ?? status ?? response?.status` and `data?.error` defensively, as ofetch's FetchError carries them. + +app/composables/useTwoFactor.ts: `disableTotp(password: string)` sends DELETE with `body: { password }` (JSON body, never the query string). `confirmTotp(secret, code, password?: string)` adds `password` to the body only when it is given. Update the comments: golem15.mfa asks for the password to turn a factor off, and to confirm TOTP when another factor is already on. + +app/pages/account/security.vue: +- Turning off: replace the browser confirm dialog in turnOff with an in-page step (no browser dialog at all). "Turn off" opens a small form showing the existing turnOffConfirm text, a password input (type password, autocomplete current-password, required), a submit button (turnOff) and a cancel link-button. Submit calls disableTotp(password). +- Setup: when `state.methods.length > 0` (another factor is on; the server asks for the password then), the setup form shows the same password input and passes it to confirmTotp. +- Errors: `isPasswordRefusal(e)` shows t('security.passwordWrong'); other errors keep authErrors. +- Password handling: clear the password ref after every attempt, success or failure, and on cancel. Never log it, never put it in the URL, never store it beyond the ref. Keep `busy` handling and the existing markup classes (form, button, link-button, muted, form__error). +- Update the header comment to mention the password step. + +i18n: add to the security block of en.json and pl.json, with the same keys in both: `passwordLabel` ("Your password" / "Twoje hasło"), `passwordWrong` ("Wrong password. Several wrong attempts block the account for a while." / "Nieprawidłowe hasło. Kilka błędnych prób blokuje konto na jakiś czas."), `passwordForSetup` ("Your account already has a second factor: confirm the change with your password." / "Konto ma już drugi składnik: potwierdź zmianę hasłem."). Keep turnOffConfirm as the prompt text. Valid JSON, no other key changes. + +Commit in F (D-07): stage the six paths, message like "feat: the security page asks for the password to change a second factor". Then, in D: `git -C D status --short`. Stage only the gitlink `vue-demo-app` (if D shows other sessions' changes, leave them alone) and commit "chore: bump vue-demo-app (password for second-factor changes)". Do not touch D's plugin submodules, go.mod, go.work or plugins.gen.go (D-07: they wait for the plugin tags). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app/vue-demo-app && pnpm test && node -e "const en=require('./i18n/locales/en.json').security, pl=require('./i18n/locales/pl.json').security; for (const k of ['passwordLabel','passwordWrong','passwordForSetup']) { if (!en[k]||!pl[k]) { console.error('missing', k); process.exit(1) } } if (JSON.stringify(Object.keys(en).sort())!==JSON.stringify(Object.keys(pl).sort())) process.exit(1)" && grep -q "isPasswordRefusal" app/pages/account/security.vue && ! grep -q "window.confirm" app/pages/account/security.vue && grep -q "body: { password }" app/composables/useTwoFactor.ts + + vitest passes, including the new isPasswordRefusal cases. en/pl security keys match. security.vue opens no browser dialog and sends the password in the body. `pnpm build` completes. One commit in vue-demo-app and one gitlink-only bump commit in sm-demo-app. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| mailbox → activate-by-code | A mailed activation code is a public-route credential (untrusted holder) | +| session bearer → 2fa management routes | A JWT may be stolen (XSS, leaked token); it must not suffice to change factors | +| browser → password field | The password crosses the page into a JSON body | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-oc5-01 | Elevation of privilege | U ActivateByCode | medium | mitigate | CheckLoginGates after VerifyActivationCode with source password; a gate answer replaces the token (Task 1, TestActivateByCodeGates, M activation_by_code_is_challenged) | +| T-oc5-02 | Tampering | M TOTPDisable, EmailDisable, WebAuthnRemove, WebAuthnRegister, TOTPConfirm | high | mitigate | checkPassword on the five routes per D-02, factors unchanged on refusal (TestFactorChangesNeedPassword) | +| T-oc5-03 | Information disclosure | M checkPassword as a password oracle | high | mitigate | Every non-empty password is recorded through userclasses.CheckAndRecordLogin, and banned or suspended accounts are refused (D-04; suspended case in the test) | +| T-oc5-04 | Tampering | M TOTPConfirm race | medium | mitigate | Users-row FOR UPDATE and an EnabledMethods re-check inside the write transaction; sentinel answers 403 (parallel test) | +| T-oc5-05 | Information disclosure | password in query strings and access logs | low | mitigate | The frontend sends it only in the JSON body. The server's query fallback stays for parity with the existing recovery-code route and is documented | +| T-oc5-06 | Tampering | factor_changes_require_password false | medium | accept | Opt-out for ported PHP frontends only. Default true, and the README says it re-opens WR-05 | +| T-oc5-07 | Information disclosure | F password ref | low | mitigate | Cleared after every attempt and on cancel, autocomplete current-password, never logged or put in a URL | +| T-oc5-SC | Tampering | npm/pip/cargo installs | high | accept | No package is added or installed by this plan (no package.json, go.mod require or lockfile change) | + + + +- U: `go vet ./...`; `go test -count=1 -run 'TestActivateByCodeGates|TestActivateByCode$|TestTokenCookie|TestLoginGate' .`; `go test -short -count=1 ./...` has only the known TestRegisterCORSPath failure. +- M: `go vet ./...`; `go test -count=1 ./...` (Docker) green against ../sm-user-plugin. +- F: `pnpm test`; `pnpm build` completes. +- Each repo's `git log -1 --format=%B` has no Co-Authored-By and no planning ID; `git status --short` shows nothing this plan left unstaged. +- SUMMARY records the commit hashes per repo, the tests that ran (and which needed Docker), and the follow-ups that need the user (D-07): + 1. Push and tag sm-user-plugin (next patch, v0.1.20 if free) and sm-mfa-plugin (v0.2.0 suggested: contract change). + 2. Push vue-demo-app before sm-demo-app's pointer bump. + 3. Bump in D-17 order, separate commits each: sm-demo-app (go.mod plus the user and mfa submodules, check-pins), sm-bm-app (user and mfa), fonoteka.go (user only). + 4. Changelog posts for the plugin tags. + 5. Move both todos to done and update g15office yfadd1mb and krt3vrts. + 6. Note F-1 and WR-05 as resolved in 19.6-SECURITY-REVIEW.md if the user wants that record. + + + +- With a challenging gate, activate-by-code no longer mints a token. The account is still activated, and every gate-free answer is unchanged. +- No second factor can be removed, replaced, added beside an existing one, or registered as a passkey with a session alone, unless the application opts out through factor_changes_require_password. +- Wrong confirmations count toward the login throttle, and malformed requests keep PHP's 422 bodies. +- The demo security page works against both the current v0.1.3 and the new sm-mfa-plugin. +- All suites named above are green, and every repo has only its own explicit-path commits. + + + +Create `/media/nvme/dev/golem15/summercms.io/summercms/summercms.go/.planning/quick/261105-oc5-2fa-hardening/261105-oc5-SUMMARY.md` when done + diff --git a/.planning/quick/261105-oc5-2fa-hardening/261105-oc5-SUMMARY.md b/.planning/quick/261105-oc5-2fa-hardening/261105-oc5-SUMMARY.md new file mode 100644 index 00000000..f15dcb78 --- /dev/null +++ b/.planning/quick/261105-oc5-2fa-hardening/261105-oc5-SUMMARY.md @@ -0,0 +1,156 @@ +--- +phase: quick-261105-oc5 +plan: 01 +subsystem: auth (sm-user-plugin, sm-mfa-plugin, vue-demo-app) +tags: [security, 2fa, mfa, activation, login-gates, webauthn, totp] +status: complete +requires: + - sm-user-plugin login gate registry (CheckLoginGates) + - sm-mfa-plugin Gate (challenges source password) +provides: + - activate-by-code consults the login gates before it mints (F-1 closed) + - password confirmation on the five factor-changing mfa routes (WR-05 closed) + - golem15.mfa.factor_changes_require_password config key (default true) + - vue-demo-app /account/security password step +affects: + - sm-demo-app, sm-bm-app, fonoteka.go (plugin bumps pending tags) +tech-stack: + added: [] + patterns: + - "users-row FOR UPDATE + EnabledMethods re-check inside the factor write (race-safe first factor)" + - "password helper split: passwordFrom / checkPassword / confirmPassword" +key-files: + created: + - ../sm-mfa-plugin/factor_password_test.go + modified: + - ../sm-user-plugin/controllers/api_controller.go + - ../sm-user-plugin/controllers/login_gates.go + - ../sm-user-plugin/classes/login_gate.go + - ../sm-user-plugin/login_gate_test.go + - ../sm-user-plugin/README.md + - ../sm-mfa-plugin/go.work + - ../sm-mfa-plugin/classes/config.go + - ../sm-mfa-plugin/config/config.yaml + - ../sm-mfa-plugin/controllers/management.go + - ../sm-mfa-plugin/controllers/webauthn.go + - ../sm-mfa-plugin/login_paths_test.go + - ../sm-mfa-plugin/settings_test.go + - ../sm-mfa-plugin/README.md + - ../sm-mfa-plugin/{tracer_smoke,webauthn_smoke,webauthn,php_bodies,challenge_smoke,email,gate,trusted,passwordless_smoke}_test.go + - ../sm-demo-app/vue-demo-app/app/lib/twoFactor.ts + - ../sm-demo-app/vue-demo-app/app/lib/twoFactor.test.ts + - ../sm-demo-app/vue-demo-app/app/composables/useTwoFactor.ts + - ../sm-demo-app/vue-demo-app/app/pages/account/security.vue + - ../sm-demo-app/vue-demo-app/i18n/locales/en.json + - ../sm-demo-app/vue-demo-app/i18n/locales/pl.json + - ../sm-demo-app (vue-demo-app gitlink) +decisions: + - "F-1: ActivateByCode calls CheckLoginGates with LoginSourcePassword after VerifyActivationCode; a gate answer or error replaces the token, the activation stays committed" + - "WR-05: password on DELETE totp/email/webauthn/{id}, every webauthn/register, and totp/confirm when ANY factor is enabled; validation 422 and availability 403 come first" + - "golem15.mfa.factor_changes_require_password (default true, config-only, not on the settings row); false restores the PHP password-free contract" + - "sm-mfa-plugin go.work now replaces sm-user-plugin with ../sm-user-plugin instead of the stale fonoteka checkout" +metrics: + duration: ~40 min + completed: 2026-11-05 +actuals: + tokens: 26000 + tasks: 3 + commits: 5 +commits_by_repo: + sm-user-plugin: [b179e6b] + sm-mfa-plugin: [357e85d, 3aa52c6] + vue-demo-app: [848ee1e] + sm-demo-app: [7b75d2c] +plan_head_before: "n/a (no commits in summercms.go; sibling repos: sm-user-plugin 97f83cc, sm-mfa-plugin 544017c, vue-demo-app fe5d38d, sm-demo-app da03c06)" +plan_head_after: "sm-user-plugin b179e6b, sm-mfa-plugin 3aa52c6, vue-demo-app 848ee1e, sm-demo-app 7b75d2c" +--- + +# Quick 261105-oc5: 2FA hardening Summary + +activate-by-code now goes through the login gates, so a mailed code no longer gets a user past an enabled second factor (F-1). golem15.mfa's five factor-changing routes now need the user's password and record wrong guesses in the throttle, with a config opt-out (WR-05). The demo security page asks for the password where the plugin now needs it. + +## Commits + +| Repo | Hash | Message | +|------|------|---------| +| sm-user-plugin | b179e6b | fix: activate-by-code asks the login gates before it signs in | +| sm-mfa-plugin | 357e85d | chore: develop against the sm-user-plugin checkout | +| sm-mfa-plugin | 3aa52c6 | fix: changing a second factor asks for the password | +| vue-demo-app (sm-demo-app/vue-demo-app) | 848ee1e | feat: the security page asks for the password to change a second factor | +| sm-demo-app | 7b75d2c | chore: bump vue-demo-app (password for second-factor changes) | + +None of the commits has a co-author trailer or a planning ID. Nothing was pushed or tagged. None of the five repos has uncommitted changes from this plan. summercms.go has no commits; the orchestrator commits this SUMMARY. + +## What changed + +**Task 1, sm-user-plugin (F-1).** After `VerifyActivationCode`, `ActivateByCode` calls `CheckLoginGates` with `LoginSourcePassword`, the fields it already read and the request. If a gate answers, its status and body go back to the client (status 0 means 200), with no token and no session cookie. If a gate fails, the client gets the opaque 500. Either way the account stays activated. With no gate, or when every gate allows, the answer is the same as before. The doc comments on `LoginSourcePassword`, `LoginAttempt.Fields`, `LoginGates` and `CheckLoginGates` now cover activate-by-code. The README sections "Registration and activation", "Login gates", the API reference and Testing are updated. + +**Task 2, sm-mfa-plugin (WR-05).** +- `go.work` now points sm-user-plugin at `../sm-user-plugin`, the plugin's own repo, instead of the stale fonoteka checkout. The full suite passed against it before any WR-05 change. +- `management.go` splits the password check into `passwordFrom`, `checkPassword` and `confirmPassword`, and adds `requiresPassword(eff)`. The recovery-code routes behave exactly as before. +- `disableHandler` (TOTP and email) and `WebAuthnRemove` call `confirmPassword` right after `signedIn`. In `WebAuthnRemove` that is before the id is parsed. +- `WebAuthnRegister` checks the password after validation and the availability check, and before it reads the attestation. This applies to every registration. +- `TOTPConfirm` loads `EnabledMethods` after validation and the availability check. If any factor is enabled, it checks the password before `VerifyTOTP`. Inside the write transaction it locks the user's row (`SELECT id FROM users WHERE id = ? FOR UPDATE`). On the path that skipped the password, it re-reads the enabled methods; if one exists now, `errPasswordNeeded` rolls back and the client gets the 403. +- New `Config.FactorChangesRequirePassword`, from `golem15.mfa.factor_changes_require_password`: default true, env `SUMMER_GOLEM15__MFA__FACTOR_CHANGES_REQUIRE_PASSWORD`, config-only (`overlay` is unchanged). It is documented in `config/config.yaml`. +- README: Features, the Routes table (password and 403 on the five routes, plus the order of answers), Passkeys, the Configuration row and the note that the settings screen does not override it, Security notes (contract, password-free paths, the knob re-opening WR-05, the activate-by-code challenge), and Testing. + +**Task 3, vue-demo-app (D-06).** +- New `PASSWORD_CONFIRMATION_REQUIRED` constant and `isPasswordRefusal(e)` helper, with 5 new vitest cases. +- `disableTotp(password)` sends `DELETE` with the JSON body `{ password }`. `confirmTotp(secret, code, password?)` adds the password only when one is given. +- `security.vue`: "Turn off" opens a form on the page (the `turnOffConfirm` text, a password field with `current-password`, submit and cancel). `window.confirm` is gone. The setup form shows the password field when `state.methods.length > 0`. A password refusal shows the localized `passwordWrong` message. The password is cleared after every attempt and on cancel. +- New `security.passwordLabel`, `passwordWrong` and `passwordForSetup` strings in both en and pl. + +## Verification run + +- **sm-user-plugin:** `go vet ./...` is clean. `TestActivateByCodeGates` (new, Postgres), `TestActivateByCode`, `TestTokenCookie`, `TestLoginGates`, `TestLoginGateOrder`, `TestLoginGateSmoke` and `TestLoginGatesSmoke` pass. `go test -short -count=1 ./...` and the full `go test -count=1 ./...` (Docker) fail only the known `TestRegisterCORSPath` (it needs `config/http.yaml` from an application checkout). +- **sm-mfa-plugin:** `go list -m` reports `../sm-user-plugin`. `gofmt`, `go build` and `go vet` are clean. `go test -short -count=1 ./...` and the full `go test -count=1 ./...` (Docker, about 39 s) pass. `TestFactorChangesNeedPassword` also passed under `-race`, and its race subtest passed 3 more runs. + - New tests: `TestFactorChangesNeedPassword` (disable routes, register, totp_confirm, validation and availability first, suspended account, first-factor race, knob off), the `TestSettings` cases for the knob (default, config, env, and the settings row never changing it), and `TestEveryLoginPathIsGated/activation_by_code_is_challenged`. + - The new tests were red before the handler changes, apart from the activation subtest, which already passed because Task 1 was committed first. +- **vue-demo-app:** `pnpm test` passes 230 tests, including the 5 new ones. The plan's i18n/grep check passes. `pnpm build` completes. +- Docker worked directly; `sudo -u golem` was not needed. + +## Deviations from Plan + +**1. [Rule 1, test fixes beyond the files the plan listed] More existing calls needed the password.** In `php_bodies_test.go` (management_group), `webauthn_test.go` (registration and delete_scoped_to_the_owner) and `webauthn_smoke_test.go`, some calls now hit the password check before the answer they assert: +- `totp/confirm 422 code` (wrong code) +- `webauthn/register 422 failed` (unknown challenge) +- `DELETE webauthn/999999 404`, and the 404 for another user's key +- registrations whose attestation fails verification + +These calls now send `"password": testPassword`. Every PHP body assertion is unchanged, and the pure validation 422 cases still send no password. All the files were already in the plan's list; only these extra call sites were new. + +**2. [Discretion] Order of answers in the Routes table.** For `webauthn/register` and `webauthn/{id}`, the 403 for a missing password comes before the 422 "Registration failed: …" and before the 404 for an unknown id. That follows from D-03 (the password check runs after validation and availability, before the attestation or the id is read), and the README Routes table says so. + +**3. [Discretion] mfa README states the user plugin version activate-by-code depends on.** The Security note says the challenge needs "sm-user-plugin after v0.1.19". go.mod still requires sm-user-plugin v0.1.12, because no code depends on the new behaviour; only the new `activation_by_code_is_challenged` test needs it (see follow-up 3). + +Nothing else deviated. The plan ran as written. + +## Contract change (for the release note) + +sm-mfa-plugin's management API changes: DELETE `2fa/totp`, DELETE `2fa/email`, DELETE `2fa/webauthn/{id}`, every POST `2fa/webauthn/register`, and POST `2fa/totp/confirm` when the user already has a second factor now need `password` and answer 403 `{"error":"Password confirmation required."}` without it. To restore the old contract, set `golem15.mfa.factor_changes_require_password: false`. A **minor** version, **v0.2.0**, is suggested. sm-user-plugin's change is additive (the answer changes only when a gate challenges), so a patch, **v0.1.20**, fits. + +## Follow-ups for the user (not done here: no push, tag, deploy or bumps) + +1. Push and tag **sm-user-plugin** (next patch, `v0.1.20` if free) and **sm-mfa-plugin** (`v0.2.0` suggested, because of the contract change). Then post on https://summercms.io/changelog/plugins for both tags (`scripts/changelog-post.py publish … --repo ../sm--plugin`). +2. Push **vue-demo-app** (`848ee1e`) before pushing sm-demo-app's pointer bump (`7b75d2c`), as sm-demo-app's README requires. +3. After the tags, bump in this order, one commit each: + - **sm-demo-app:** go.mod plus the `user` and `mfa` plugin submodules, then `scripts/check-pins.sh`. + - **sm-bm-app:** user and mfa. + - **fonoteka.go:** user only. + - Optionally raise sm-mfa-plugin's own `go.mod` require of sm-user-plugin to the new tag (and its `plugin.yaml` `requires`), so `GOWORK=off` builds see the activate-by-code gate the new login-paths subtest pins. +4. Any application that mounts golem15.mfa with a frontend other than vue-demo-app must send `password` on these routes, or set `factor_changes_require_password: false` until it can. At planning, sm-bm-app's frontend and fonoteka called none of these routes. +5. Move both todos to done (`2026-10-26-activate-by-code-second-factor.md`, `2026-10-26-mfa-factor-changes-need-reauth.md`) and update g15office yfadd1mb and krt3vrts. The orchestrator owns these. +6. Optionally mark F-1 and WR-05 as resolved in `19.6-SECURITY-REVIEW.md` and `19.6-REVIEW.md`. + +## Known Stubs + +None. + +## Threat Flags + +None. No new endpoints were added. The changes tighten existing routes as the plan's threat register describes (T-oc5-01..07 mitigated as planned; T-oc5-06 accepted, documented in the README and config.yaml). + +## Self-Check: PASSED + +- Files exist: `../sm-mfa-plugin/factor_password_test.go`, and every modified path listed above. +- Commits found: sm-user-plugin b179e6b; sm-mfa-plugin 357e85d, 3aa52c6; vue-demo-app 848ee1e; sm-demo-app 7b75d2c. diff --git a/.planning/quick/261105-oc6-chatchannels-pin-policy/261105-oc6-PLAN.md b/.planning/quick/261105-oc6-chatchannels-pin-policy/261105-oc6-PLAN.md new file mode 100644 index 00000000..9f27efd2 --- /dev/null +++ b/.planning/quick/261105-oc6-chatchannels-pin-policy/261105-oc6-PLAN.md @@ -0,0 +1,297 @@ +--- +phase: quick-261105-oc6 +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - ../sm-chatchannels-plugin/contracts/pin_policy.go + - ../sm-chatchannels-plugin/contracts/access_resolver.go + - ../sm-chatchannels-plugin/classes/channel_service.go + - ../sm-chatchannels-plugin/api/pins.go + - ../sm-chatchannels-plugin/pins_test.go + - ../sm-chatchannels-plugin/testdata/PHP_METHOD_MAP.md + - ../sm-chatchannels-plugin/README.md + - ../sm-demo-app/plugins/demo/classes/chat_context.go + - ../sm-demo-app/plugins/demo/classes/chat_context_test.go + - ../sm-demo-app/plugins/demo/chat.go + - ../sm-demo-app/reset_test.go + - ../sm-demo-app/README.md + - ../sm-demo-app/plugins/golem15/chatchannels + - ../sm-demo-app/chat_test.go +autonomous: true +requirements: + - quick-261105-oc6 + - WR-02 + +must_haves: + truths: + - "In sm-chatchannels-plugin, a user whose installed resolver grants channel access but answers CanManageContext false (and either does not implement contracts.PinPolicy or answers CanPin false) gets 404 from POST /_chatchannels/api/v1/contexts/{type}/{id}/channels/{channelId}/pins and from DELETE .../pins/{messageId}. No pin row is written or removed. GET .../pins still answers 200 for them." + - "A user with CanManageContext, or whose resolver also implements contracts.PinPolicy and answers CanPin true, pins and unpins. A second pin of the same message is a no-op that keeps the first pinner, and an allowed user unpins a pin someone else made. A user without channel access is refused (ErrForbiddenAccess) even when CanPin would answer true, and CanPin is not consulted for them." + - "PinMessage refuses with ErrNotFound (HTTP 404, no row) a message id that is unknown, soft-deleted, or in another conversation (another channel or a DM), and refuses any message for a channel with no conversation_id." + - "Hosts that do not implement PinPolicy compile unchanged: contracts.AccessResolver and every existing exported signature in the plugin stay the same, go.mod and go.sum are unchanged, and the plugin's go vet, go test -short and Docker-backed go test pass." + - "In sm-demo-app, with the chatchannels submodule at the new plugin commit, a member of the demo-moderators group pins and unpins in #general, including a seeded welcome pin. A plain member gets 404 for both and the seeded pin survives. The moderator cannot pin an #announcements message or a DM message through #general's path. Nobody can create a channel (CanManageContext stays false)." + - "Commits: one on sm-chatchannels-plugin main, and three on sm-demo-app master in this order: host policy, then the pointer bump alone, then the end-to-end test. None has an attribution trailer. Nothing is pushed or tagged, and go.mod/go.sum are unchanged in both repos." + artifacts: + - path: "../sm-chatchannels-plugin/contracts/pin_policy.go" + provides: "contracts.PinPolicy, the optional host hook CanPin(ctx, userID, contextType, contextID, channelID) bool" + - path: "../sm-chatchannels-plugin/classes/channel_service.go" + provides: "CanPinInChannel, the pin gate used by PinMessage/UnpinMessage, and the message-in-channel check" + - path: "../sm-chatchannels-plugin/pins_test.go" + provides: "TestPinPolicy (Docker) and TestDenyAllHasNoPinPolicy (-short)" + - path: "../sm-demo-app/plugins/demo/classes/chat_context.go" + provides: "ChatAccess.CanPin answering from the moderator group" + - path: "../sm-demo-app/chat_test.go" + provides: "TestDemoPinPolicy end to end over HTTP" + key_links: + - from: "../sm-chatchannels-plugin/classes/channel_service.go PinMessage/UnpinMessage" + to: "contracts.PinPolicy" + via: "type assertion on s.Resolver() after requireChannelAccess and a false CanManageContext" + - from: "../sm-demo-app/plugins/demo/chat.go registerChat" + to: "../sm-demo-app/plugins/demo/classes/chat_context.go NewChatAccess" + via: "svc.SetAccessResolver(classes.NewChatAccess(p.db, p.chatModeratorGroup, nil))" + - from: "../sm-demo-app go.work use ./plugins/golem15/chatchannels" + to: "../sm-chatchannels-plugin main (Task 1 commit)" + via: "submodule gitlink bumped in its own commit" +--- + + +Pinning in chat channels becomes a manage action (todo `chatchannels-pin-policy.md`, phase 20 review WR-02). Today any visitor of the demo can unpin the seeded welcome pins or flood the pins panel, because sm-chatchannels-plugin gates PinMessage and UnpinMessage on channel access only, and the demo grants that access to every member. PinMessage also never checks that the message belongs to the channel's conversation. + +Purpose: the server enforces who may pin, through an additive and optional host contract, so shared-plugin hosts keep compiling. The demo answers that contract from its existing moderator group (`golem15.demo.chat.moderator_group`, default `demo-moderators`). + +Output: +- One commit on sm-chatchannels-plugin main: `contracts.PinPolicy`, the pin gate, the message-in-channel check, tests and README. +- Three commits on sm-demo-app master: the host policy, the submodule pointer bump on its own, and the end-to-end pin test. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@.planning/STATE.md +@CLAUDE.md +@.planning/todos/pending/chatchannels-pin-policy.md + +Repos and their state at planning time (2026-11-05). Use absolute paths in commands; the shell cwd resets between calls. +- Plugin: /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin, branch `main` at 017843a (= origin/main = tag v0.1.0), clean. Leave the `backup/manifest-pre-rebase` branch alone. Its go.work resolves summercms to ../summercms.go, sm-chat-plugin to ../sm-chat-plugin and sm-user-plugin to ../fonoteka.go/plugins/golem15/user. If main has moved past 017843a, build on top of it and record `git log --oneline 017843a..main` in the SUMMARY. If the tree is dirty, stop: another session is working there. +- Demo: /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app, branch `master` at da03c06, clean (sibling batch items oc5/oc9 may have committed there before you; build on top). The submodule plugins/golem15/chatchannels is detached at 017843a (v0.1.0), and go.mod requires sm-chatchannels-plugin v0.1.0. go.work `use`s the submodule checkout, so development and tests compile the submodule. `go list ./...` at the demo root covers only the app's own 7 packages. +- Baselines measured during planning, all green: the plugin's `go vet ./...`, `go test -short ./...` and Docker-backed `go test -count=1 .` (~36 s); the demo's `go vet ./plugins/demo/...`, `go test -short ./plugins/demo/...` and Docker-backed `TestDemoModeratorDelete` + `TestResetKeepsPlans` (~35 s). Docker is reachable for this user. If the socket is denied, run the tests through `sudo -u golem` (summercms.go CLAUDE.md). + +Plugin code facts (sm-chatchannels-plugin): +- contracts/access_resolver.go: the package doc says the package holds the ChannelAccessResolver seam. `AccessResolver` has `CanAccessContext` and `CanManageContext(ctx context.Context, userID uint, contextType string, contextID uint) bool`. +- classes/channel_service.go: the sentinels at :26-35 (`ErrForbidden` = "not allowed to manage this channel context", `ErrForbiddenAccess`, `ErrNotFound`). `Resolver()` at :101 never returns nil. `requireManage` at :293 and `requireAccessContext` at :303. `CanAccessChannel` at :463 checks resolver access plus membership for private channels. `requireChannelAccess` at :481. `PinMessage` at :1332 calls requireChannelAccess, then FirstOrCreate on (channel_id, message_id) with Attrs ConversationID/PinnedBy. `UnpinMessage` at :1348 calls requireChannelAccess, then deletes. `PinsFor` at :1356 orders by created_at DESC, id DESC. The file already imports `chatmodels "git.golem15.com/golem15/sm-chat-plugin/models"`. +- sm-chat-plugin models/message.go: `chatmodels.Message` (table golem15_chat_messages) has `ConversationID uint`, `SenderID`, `Type` (`chatmodels.TypeText` = "text"), `Content *string` and `DeletedAt gorm.DeletedAt`. A GORM query through `Model(&chatmodels.Message{})` therefore excludes soft-deleted rows. +- api/pins.go: `ListPins`, `PinMessage` and `UnpinMessage` call `channelActor` (api/auth.go:74). It looks the service up per request and returns 404 for an unknown, foreign or invisible channel. Errors go through `writeChannelError` (api/errors.go), which maps ErrForbidden, ErrForbiddenAccess and ErrNotFound to 404 `{"error":"Not found"}`. No mapping change is needed. +- Tests (package chatchannels_test): harness_test.go has `bootChannels(t, nil)` returning `*channelEnv` with fields app, gdb, plugins, svc and rt, plus `env.user(t)` and `env.count(t, table, where, args...)`. fixture_plugin_test.go:188 has `env.team(t, name, members...)`, which inserts acme_teams_* rows, and `teamsAlias` = "teams". http_tracer_test.go has `bootWeb(t)` returning `*webEnv` (a surf handler), `env.frontendToken(t, user)`, `env.call(t, method, path, token, body)` (path relative to chatchannels.APIPrefix), `decodeObject`, and `teamAccessResolver`, whose CanManageContext equals CanAccessContext. `channelActor` looks the service up per request, so `env.svc.SetAccessResolver(...)` after bootWeb applies to HTTP calls. TestMain (testmain_test.go) skips the container under -short. +- testdata/PHP_METHOD_MAP.md rows 37-42 map the six PHP pin tests to TestChannelHTTPTracer/TestAccessResolverDefault. TestPHPMethodMap requires status `ported` and an existing Go test name. +- The PHP original (wavepath ChannelService.php pinMessage/unpinMessage, D-57-02) let any member pin and unpin. This change deliberately tightens that per the todo (WR-02). + +Demo code facts (sm-demo-app): +- plugins/demo/classes/chat_context.go: the interface assertions var block is at :18-26. `IsModerator(ctx, db, userID, group)` at :90 returns false without querying when the user is 0, the group is "" or db is nil, and otherwise calls userclasses.HasGroupCode. `ChatAccess` at :239-275 has the fields `db func() *gorm.DB` and `isMember MemberFunc`, plus `NewChatAccess(db, isMember)`, `CanAccessContext` (demo/1 + member) and `CanManageContext`, which is always false (D-07: channels come from demo:seed only, and TestDemoChannelsSeededOnly relies on it). +- plugins/demo/classes/forum_access.go is the pattern to mirror: `NewForumAccess(db func() *gorm.DB, moderatorGroup func() string, isMember MemberFunc)`, a `member` helper and a `moderator` helper. +- plugins/demo/chat.go:68: `svc.SetAccessResolver(classes.NewChatAccess(p.db, nil))`. `p.chatModeratorGroup` (:93) is a `func() string` method value. +- plugins/demo/classes/chat_context_test.go: TestDemoChatAccess (~:177) calls `NewChatAccess(someDB, m.fn)` four times. `someDB()` is an empty `&gorm.DB{}`: any real query on it panics, so unit tests must never reach IsModerator with a non-empty group for a community member. +- reset_test.go `addChatActivity` (:155-210) has the plain walker POST a pin in #general (:187, `ok("pin", ...)` fails on status >= 300). Under the new rule the walker must be a moderator for that step. `makeModerator(t, a *apiDemo, userID)` lives in chat_test.go:19 (same package main), and `a := &apiDemo{Demo: d, h: h}` already exists inside addChatActivity. demo:reset empties users_groups entirely (plugins/demo/console/reset.go:31), so the extra membership does not disturb the reset assertions. +- Root test helpers (package main): `bootAPI(t, nil, nil)` boots and seeds (the demo-moderators group exists). Also `a.member(t, email)` returning (id, token), `a.openChannel(t, token, slug)` returning the conversation id, `a.bootstrapChat(t, token).Channels` ({ID, Slug, ConversationID}), `a.postMessage(t, token, convID, content)` returning (status, id), `a.openDM(t, token, userID)` returning (status, convID), `a.call(t, method, path, token, body)` and `scalar[T](t, a.Demo, query, args...)`. The seed inserts one pin each in #general, #announcements and #ogólny by SQL (seed_chat.go:279). TestSeedChatSmoke counts 3 pinned channels, which is unaffected. +- README.md "Moderation." paragraph (~:186-188) lists what demo-moderators may do. +- scripts/check-pins.sh (run by build.sh and check-all.sh) requires each plugin submodule HEAD to equal the commit of the tag go.mod requires. After the untagged pointer bump it will report chatchannels until the user tags the plugin and bumps go.mod. That is expected; do not run it as a gate and do not change go.mod. +- Frontend: vue-chat-layer's useChat `togglePin` already hides the pin action for the session after a 4xx refusal (`pinAllowed`). No frontend change is in scope. + + + + + + Task 1: sm-chatchannels-plugin: pin and unpin need CanManageContext or a host PinPolicy, and the pinned message must be in the channel + ../sm-chatchannels-plugin/contracts/pin_policy.go, ../sm-chatchannels-plugin/contracts/access_resolver.go, ../sm-chatchannels-plugin/classes/channel_service.go, ../sm-chatchannels-plugin/api/pins.go, ../sm-chatchannels-plugin/pins_test.go, ../sm-chatchannels-plugin/testdata/PHP_METHOD_MAP.md, ../sm-chatchannels-plugin/README.md + /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin is on branch main with a clean working tree, and `docker info` succeeds (or the tests run through sudo -u golem). + + - A member with channel access, but neither manage nor PinPolicy permission, gets ErrForbidden from PinMessage and UnpinMessage (HTTP 404). No pin row is written and an existing row survives. PinsFor still works for them (HTTP GET 200). + - A manager (CanManageContext true) pins: the row has pinned_by = manager, and a second pin of the same message keeps one row and the first pinner. A manager unpins, which removes the row. + - With a resolver that also implements contracts.PinPolicy and answers CanPin true for a non-manager, that user pins and unpins, including a pin another user made. CanPin receives the channel's context type, context id and channel id. + - A user without channel access gets ErrForbiddenAccess even though CanPin would answer true for them, and CanPin is not called for them. + - PinMessage returns ErrNotFound and writes no row for an unknown message id, a soft-deleted message, a message of another channel's conversation, and a channel copy whose ConversationID is nil. Over HTTP the foreign message is 404. + - PinsFor lists pins newest first and PinCountFor counts them (so the PHP map rows can point at this test). + - Under -short: classes.DenyAll does not implement contracts.PinPolicy, and CanPinInChannel on a nil service or a nil channel is false. + + +Work in /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin. All changes are additive (a shared plugin used by several apps). No existing exported signature changes, contracts.AccessResolver stays as it is, and no dependency is added. + +1. Create contracts/pin_policy.go with the optional interface `PinPolicy`. It has one method: `CanPin(ctx context.Context, userID uint, contextType string, contextID, channelID uint) bool`. Write the doc comment in the style of sm-chat-plugin's contracts.MessageModerator: + - It is an optional interface the host's AccessResolver may also implement. + - golem15.chatchannels consults it only for PinMessage and UnpinMessage, after the channel access check passed and only when CanManageContext answered false. True lets userID pin and unpin messages in channel channelID of that host context. + - It never grants access to the channel. + - A host that does not implement it lets only CanManageContext pin. + - Any member could pin before this rule (v0.1.0). A host that wants that behaviour back implements CanPin by answering its own CanAccessContext. + + In contracts/access_resolver.go, update only the package doc comment so it names both seams: the required AccessResolver and the optional PinPolicy. + +2. In classes/channel_service.go, next to the pin methods: + - Add an unexported helper `pinAllowed(ctx, actorID, ch) bool`. It is true when `s.Resolver().CanManageContext(ctx, actorID, ch.ContextType, ch.ContextID)` is true. Otherwise, when `s.Resolver()` also satisfies `contracts.PinPolicy` (type assertion), it returns `CanPin(ctx, actorID, ch.ContextType, ch.ContextID, ch.ID)`. Otherwise it is false. + - Add an exported `CanPinInChannel(ctx context.Context, actorID uint, ch *models.Channel) bool`. It is nil-safe: a nil service or channel is false, matching CanAccessChannel. It returns CanAccessChannel && pinAllowed. Hosts and bootstraps can use it to show or hide the pin action. + - Add an unexported `requirePin(ctx, actorID, ch) error`. It runs requireChannelAccess first and returns that error unchanged (ErrNotFound or ErrForbiddenAccess). It then returns ErrForbidden when pinAllowed is false. + - Add an unexported `requireMessageInChannel(ctx, ch, messageID) error`. It returns ErrNotFound when ch.ConversationID is nil or zero. Otherwise it counts `Model(&chatmodels.Message{})` rows where id = messageID and conversation_id = *ch.ConversationID, which excludes soft-deleted rows. Zero is ErrNotFound and a query error is returned as is. + - PinMessage: requirePin, then requireMessageInChannel, then the existing FirstOrCreate, unchanged. + - UnpinMessage: requirePin, then the existing delete, unchanged. Do not add a message check to unpin: a stale pin of a deleted message must stay removable. + - Update both doc comments to say "PHP successor, tightened by WR-02" and state the rule. PinsFor, IsPinned, PinCountFor and RemovePinsForMessage stay unchanged. + +3. In api/pins.go, update only the doc comments of the PinMessage and UnpinMessage handlers. They now say: 404 also when the caller may not pin (CanManageContext or contracts.PinPolicy) and, for POST, when message_id is not a live message of this channel's conversation. Do not change the status mapping in api/errors.go. Forbidden and not-found both stay 404, the plugin's existing anti-enumeration rule. + +4. Create pins_test.go (package chatchannels_test): + - A resolver type `pinAccess` with access for team members, using the acme_teams_members count in the same way as teamAccessResolver, and CanManageContext only for user ids in a managers set. + - A pointer type `pinAccessWithPolicy`. It embeds pinAccess, adds a pinners set, and records the arguments of each CanPin call plus a call count. Its CanPin deliberately ignores access, which proves the service checks access first. + - Compile-time assertions that both satisfy contracts.AccessResolver and that *pinAccessWithPolicy satisfies contracts.PinPolicy. + - `TestPinPolicy` (Docker; the harness skips it under -short): + - Use bootWeb(t). Create users manager, member, pinner and stranger. The team holds manager, member and pinner. + - Install pinAccess with managers = {manager}. + - CreateChannel "general" and "random" as manager with member and pinner as participants. + - Insert messages with env.gdb.Create(&chatmodels.Message{ConversationID, SenderID, Type: chatmodels.TypeText, Content}): two in general, one in random, and one in general that you then soft-delete with env.gdb.Delete. + - Cover every behavior bullet at the service level, checking errors with errors.Is and row counts with env.count on golem15_chatchannels_channel_pins. + - Cover over HTTP with env.call on /contexts/teams/{teamID}/channels/{channelID}/pins: member POST is 404, manager POST is 200 with message_id and channel_id, manager POST of the random message on general's path is 404, member GET is 200, member DELETE is 404, and manager DELETE is 200. + - Then install pinAccessWithPolicy with pinners = {pinner, stranger}. Cover: pinner pins and unpins (including the manager's pin); the recorded CanPin args are (teams, teamID, general.ID); member is still refused; stranger gets ErrForbiddenAccess and the CanPin call count does not change. + - Also cover newest-first PinsFor and PinCountFor with two pins. + - `TestDenyAllHasNoPinPolicy` (runs under -short): DenyAll does not satisfy contracts.PinPolicy, and `(*classes.ChannelService)(nil).CanPinInChannel(ctx, 1, nil)` is false. + +5. In testdata/PHP_METHOD_MAP.md, re-point the six pin rows to TestPinPolicy: test_pin_message_creates_row_keyed_on_channel_and_message, test_second_pin_of_same_message_is_idempotent_noop_first_pinner_wins, test_any_member_can_unpin_regardless_of_who_pinned, test_pins_for_returns_newest_first, test_pin_unpin_list_throw_channel_forbidden_when_resolver_denies and test_pin_count_for_counts_distinct_pinned_messages. Keep status `ported`, and leave the row count and PHP names unchanged. + +6. README.md: same commit, per the summercms.go documentation rules. + - Add a Features bullet "**Pin policy.**" stating the rule. Pin and unpin need channel access plus CanManageContext, or a contracts.PinPolicy the installed resolver also implements. A pinned message must be a live message of the channel's conversation (otherwise ErrNotFound, 404). Listing pins needs channel access only. + - Add a sentence in the same bullet that this tightens the v0.1.0 rule (any member pinned), and that a host that wants it back answers CanPin with its access check. + - In Usage, add a short Go example of a resolver that also implements CanPin. + - In the API reference, add a `contracts.PinPolicy` row and add `CanPinInChannel` to the ChannelService method list. + - In Testing, add the TestPinPolicy Docker line and TestDenyAllHasNoPinPolicy to the -short line. + +7. Run gofmt, go vet, go test -short and the full Docker-backed go test. Then commit on main, staging the seven paths explicitly (never `git add -A`). Suggested subject: "fix: pinning needs CanManageContext or a host PinPolicy, and the message must be in the channel". The body has two short paragraphs, WR-02 and the additive contract. Add no attribution trailer of any kind: the user's global CLAUDE.md overrides any harness attribution reminder. Do not push or tag. Record the commit SHA for Task 3. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin && test -z "$(gofmt -l .)" && go vet ./... && go test -short -count=1 ./... && go test -count=1 ./... && go test -count=1 -run '^TestPinPolicy$' -v . 2>&1 | grep -qF -- '--- PASS: TestPinPolicy' && go test -short -count=1 -run '^TestDenyAllHasNoPinPolicy$|^TestPHPMethodMap$' -v . 2>&1 | grep -qF -- '--- PASS: TestDenyAllHasNoPinPolicy' && grep -qF 'contracts.PinPolicy' README.md && grep -qF 'CanPinInChannel' README.md && awk -F'|' '$3 ~ /TestPinPolicy/ && $4 ~ /ported/ {n++} END {exit (n < 6)}' testdata/PHP_METHOD_MAP.md && S=$(git status --short) && test -z "$S" && test "$(git rev-parse --abbrev-ref HEAD)" = main && git diff --quiet 017843a main -- go.mod go.sum && M=$(git log -1 --format=%B main) && ! grep -qi 'co-authored-by' <<<"$M" && T=$(git tag --points-at main) && test -z "$T" && R=$(git branch -r --contains main) && test -z "$R" + + sm-chatchannels-plugin main has one new commit with the contract, the gate, the message check, the handler docs, pins_test.go, the PHP map re-pointing and the README. TestPinPolicy passes against Postgres (not skipped) and TestDenyAllHasNoPinPolicy passes under -short. go vet, go test -short and the full go test are green. go.mod and go.sum are unchanged. The commit has no trailer, no tag and no remote branch. The SHA is recorded. + + + + Task 2: sm-demo-app: demo moderators answer the pin policy (compatible with the current v0.1.0 pointer) + ../sm-demo-app/plugins/demo/classes/chat_context.go, ../sm-demo-app/plugins/demo/classes/chat_context_test.go, ../sm-demo-app/plugins/demo/chat.go, ../sm-demo-app/reset_test.go, ../sm-demo-app/README.md + The sm-demo-app submodule plugins/golem15/chatchannels is still at 017843a (v0.1.0), and sm-demo-app has no uncommitted edits in the five files above. + + - NewChatAccess(db, moderatorGroup, isMember) keeps CanAccessContext as before (demo/1 members only), and CanManageContext stays false for everyone, moderators included. + - CanPin is false for: user 0, a non-member, any context other than demo/1, a nil db func, a nil database, a membership error, a nil moderatorGroup func, and an empty group code. The unit test never lets a community member reach IsModerator with a non-empty group, because someDB() is an empty handle. + - The walker in addChatActivity is made a moderator before the pin step, so TestResetKeepsPlans passes both before and after the pointer bump. + + +Work in /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app. Keep this commit compiling against the current submodule (v0.1.0, which has no contracts.PinPolicy). Therefore do NOT reference chancontracts.PinPolicy in this task. ChatAccess satisfies it structurally, and Task 3 adds the compile-time assertion after the bump. + +1. plugins/demo/classes/chat_context.go: + - Give ChatAccess a `moderatorGroup func() string` field. Change the constructor to `NewChatAccess(db func() *gorm.DB, moderatorGroup func() string, isMember MemberFunc)`, mirroring NewForumAccess. db and moderatorGroup are resolved per check, so a config change needs no re-install. + - Factor the existing demo/1 membership check into a `member` helper like ForumAccess.member, and keep CanAccessContext's behaviour identical. + - Add `CanPin(ctx context.Context, userID uint, contextType string, contextID, channelID uint) bool`. It is true only for a community member (via the member helper) when moderatorGroup is non-nil and `IsModerator(ctx, db, userID, moderatorGroup())` returns true with no error. channelID is unused, because every demo channel lives in demo/1 and chatchannels has already resolved the channel in that context. Say so in the doc comment. + - CanManageContext stays always false, with its comment unchanged in meaning (D-07: channels come from demo:seed only). + - Update the ChatAccess type comment: members access demo/1; nobody manages it; members of the moderator group pin and unpin channel messages (golem15.chatchannels' optional PinPolicy, review WR-02). + +2. plugins/demo/chat.go:68: install `classes.NewChatAccess(p.db, p.chatModeratorGroup, nil)`. Extend the chatModeratorGroup doc comment ("moderate the channels and the forum") to mention pins. + +3. plugins/demo/classes/chat_context_test.go: update the four NewChatAccess calls in TestDemoChatAccess to the new signature, passing a group func returning "" for the table cases. Extend the test to assert CanPin is false in each case listed in the behavior block. Use groups "" and "g" only where the membership func denies, errors or is never reached (wrong context, user 0, nil db). Also cover a nil moderatorGroup func with a member (must be false without a query). Update the test's doc comment to mention pins. The positive moderator case needs a real database and lives in Task 3's TestDemoPinPolicy. + +4. reset_test.go addChatActivity: right before the "pin" step, call makeModerator(t, a, userID). Add a one-line comment that pinning in the demo channels is a moderator action (chatchannels PinPolicy). Leave every other step as it is. + +5. README.md "Moderation." paragraph: the moderator group now also pins and unpins channel messages; other members only read the pins. Keep the paragraph's style. No new config key is added. + +6. Run gofmt, go vet ./..., go test -short ./..., and the Docker-backed TestResetKeepsPlans, TestDemoModeratorDelete and TestDemoChannelsSeededOnly. They pass on v0.1.0, where any member may still pin. Commit only these five paths, staged explicitly. Suggested subject: "feat: demo moderators answer chatchannels' pin policy". No trailer, no push. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app && test "$(git -C plugins/golem15/chatchannels rev-parse HEAD)" = 017843a6d8191d743a910ea20b4899f894aad837 && test -z "$(gofmt -l plugins/demo reset_test.go)" && go vet ./... && go test -short -count=1 ./... && go test -short -count=1 -run '^TestDemoChatAccess$' -v ./plugins/demo/classes 2>&1 | grep -qF -- '--- PASS: TestDemoChatAccess' && go test -count=1 -run '^TestResetKeepsPlans$|^TestDemoModeratorDelete$|^TestDemoChannelsSeededOnly$' . && git diff --quiet HEAD -- plugins/demo reset_test.go README.md && C=$(git rev-parse HEAD) && N=$(git diff --name-only "$C^" "$C") && test "$(sort <<<"$N" | tr '\n' ' ')" = "README.md plugins/demo/chat.go plugins/demo/classes/chat_context.go plugins/demo/classes/chat_context_test.go reset_test.go " && M=$(git log -1 --format=%B) && ! grep -qi 'co-authored-by' <<<"$M" && grep -qF 'NewChatAccess(p.db, p.chatModeratorGroup, nil)' plugins/demo/chat.go + + sm-demo-app master has one new commit touching exactly the five files. ChatAccess.CanPin answers from the moderator group, CanManageContext is still false, and NewChatAccess takes the moderator group func. The walker is a moderator before pinning in the reset test. go vet, go test -short and the three Docker tests pass against the unchanged v0.1.0 submodule. No trailer, not pushed. + + + + Task 3: sm-demo-app: bump the chatchannels submodule (own commit), then prove the moderator-only pin rule end to end + ../sm-demo-app/plugins/golem15/chatchannels, ../sm-demo-app/plugins/demo/classes/chat_context.go, ../sm-demo-app/chat_test.go + Task 1's commit is on /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin main (contracts/pin_policy.go exists there), and Task 2's commit is on sm-demo-app master. + + - With the submodule at the Task 1 commit, TestResetKeepsPlans, TestDemoModeratorDelete, TestDemoChannelsSeededOnly, TestDemoChatTracer and TestSeedChatSmoke stay green. + - TestDemoPinPolicy (Docker): a plain member's POST pin in #general is 404 and writes no row. A plain member's DELETE of the seeded #general pin is 404 and the row survives. A plain member's GET pins is 200. + - A demo-moderators member's POST pin of a #general message is 200 with pinned_by = moderator. Their POST of an #announcements message or of a DM message through #general's pins path is 404 with no row. Their DELETE of their own pin and of the seeded welcome pin is 200 and both rows are gone. + + +Work in /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app. + +1. Pointer bump, as its own commit: + - Fetch the plugin commit from the local sibling clone, not from origin, since nothing is pushed: `git -C plugins/golem15/chatchannels fetch /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin main`, then `git -C plugins/golem15/chatchannels checkout -q --detach FETCH_HEAD`. + - Check that the submodule HEAD equals `git -C /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin rev-parse main`. + - Leave go.mod (still v0.1.0) and go.sum unchanged; tagging v0.1.1 and requiring it is the user's release step. + - Run go vet ./..., go test -short ./..., and the Docker-backed TestResetKeepsPlans, TestDemoModeratorDelete, TestDemoChannelsSeededOnly, TestDemoChatTracer and TestSeedChatSmoke. Task 2 makes these pass with the stricter plugin. + - Stage only the gitlink (`git add plugins/golem15/chatchannels`) and commit. Suggested subject: "chore: bump chatchannels to (pin policy, untagged)". The body notes that check-pins.sh reports chatchannels until sm-chatchannels-plugin is tagged and go.mod requires the tag. No trailer. + +2. End-to-end proof, as a second commit: + - In plugins/demo/classes/chat_context.go, add `_ chancontracts.PinPolicy = (*ChatAccess)(nil)` to the interface assertions var block. This guards against signature drift: a mismatched CanPin would otherwise silently fail closed. + - In chat_test.go, add `TestDemoPinPolicy` next to TestDemoModeratorDelete, with a doc comment naming WR-02. + - Setup: bootAPI(t, nil, nil); a plain member and a moderator via a.member, with makeModerator for the moderator. Both open #general with a.openChannel. The moderator opens #announcements. Take the channel ids from a.bootstrapChat(...).Channels by slug. + - Messages: the plain member posts a #general message. The moderator posts an #announcements message. The plain member opens a DM with the moderator and posts in it. + - Read the seeded #general pin's message_id with scalar from golem15_chatchannels_channel_pins. + - Assert every behavior bullet through a.call on /_chatchannels/api/v1/contexts/demo/1/channels/{generalID}/pins and .../pins/{messageID}, checking rows with scalar counts. + - Run gofmt, go vet ./..., go test -short ./..., TestDemoPinPolicy -v (must PASS, not SKIP) and the five Docker tests above. Commit chat_test.go and chat_context.go only. Suggested subject: "test: pinning in the demo channels is a moderator action". No trailer. + +3. Do not run scripts/build.sh or scripts/check-pins.sh as gates: check-pins.sh is expected to flag the untagged chatchannels pointer. Do not push, tag or deploy. + +4. Record these follow-ups in the SUMMARY for the user (do not act on them): + - Release: tag sm-chatchannels-plugin (v0.1.1) on main and push it, then bump sm-demo-app go.mod to that tag; check-pins then passes again. The release also needs a changelog post at /changelog/plugins. + - sm-bm-app vendors chatchannels too, and its ChatAccess.CanManageContext is false. When it bumps past this commit its members stop pinning unless it implements CanPin. + - vue-chat-layer could hide the pin action up front from the bootstrap's `moderator` flag; today it hides the action after the first refusal. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app && P=$(git -C /media/nvme/dev/golem15/summercms.io/summercms/sm-chatchannels-plugin rev-parse main) && test "$(git -C plugins/golem15/chatchannels rev-parse HEAD)" = "$P" && L=$(git ls-tree HEAD plugins/golem15/chatchannels) && test "$(awk '{print $3}' <<<"$L")" = "$P" && test -z "$(gofmt -l chat_test.go plugins/demo)" && go vet ./... && go test -short -count=1 ./... && go test -count=1 -run '^TestDemoPinPolicy$' -v . 2>&1 | grep -qF -- '--- PASS: TestDemoPinPolicy' && go test -count=1 -run '^TestResetKeepsPlans$|^TestDemoModeratorDelete$|^TestDemoChannelsSeededOnly$|^TestDemoChatTracer$|^TestSeedChatSmoke$' . && grep -qF 'chancontracts.PinPolicy' plugins/demo/classes/chat_context.go && git diff --quiet HEAD -- go.mod go.sum chat_test.go plugins/demo plugins/golem15/chatchannels && B=$(git log -1 --format=%H -- plugins/golem15/chatchannels) && test "$(git diff --name-only "$B^" "$B")" = "plugins/golem15/chatchannels" && git diff --quiet "$B^" HEAD -- go.mod go.sum && N=$(git diff --name-only HEAD^ HEAD) && test "$(sort <<<"$N" | tr '\n' ' ')" = "chat_test.go plugins/demo/classes/chat_context.go " && M=$(git log -2 --format=%B) && ! grep -qi 'co-authored-by' <<<"$M" + + The sm-demo-app gitlink for plugins/golem15/chatchannels points at the Task 1 commit, in a commit that changes nothing else. A following commit adds the PinPolicy compile-time assertion and TestDemoPinPolicy, which passes against Postgres. The reset, moderator-delete, seeded-only, chat-tracer and seed-smoke tests stay green, and go.mod/go.sum are unchanged. No trailers, nothing pushed or tagged. The SUMMARY lists the three follow-ups (tag plus go.mod bump plus changelog, sm-bm-app impact, optional up-front hiding in the frontend). + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| browser -> /_chatchannels/api/v1 pins routes | An authenticated (jwt.auth) frontend user sends an untrusted channelId, messageId and body message_id | +| chatchannels -> host resolver (AccessResolver / PinPolicy) | The plugin trusts the host's answers; the host decides who manages and who pins | +| demo -> user groups (users_groups / user_groups) | The moderator role is read from the configured group code on every check | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-oc6-01 | Elevation of privilege | ChannelService.PinMessage / UnpinMessage (WR-02) | high | mitigate | requirePin: channel access, then CanManageContext or PinPolicy.CanPin, else ErrForbidden (404). TestPinPolicy and TestDemoPinPolicy prove a plain member can neither pin nor unpin a seeded pin | +| T-oc6-02 | Tampering | PinMessage message_id (cross-conversation pin) | medium | mitigate | requireMessageInChannel counts live golem15_chat_messages rows with conversation_id = channel.conversation_id. Foreign (other channel, DM), unknown and soft-deleted ids are ErrNotFound with no row (tests in both repos) | +| T-oc6-03 | Information disclosure | pins routes error responses | low | mitigate | Forbidden, not-found and foreign-message all answer the same 404 `{"error":"Not found"}` through the unchanged writeChannelError, so the routes are no oracle for message ids in other conversations | +| T-oc6-04 | Elevation of privilege | PinPolicy granting a non-member (host bug) | medium | mitigate | requireChannelAccess runs before PinPolicy and its error is returned unchanged. TestPinPolicy asserts the stranger gets ErrForbiddenAccess and CanPin is never called for them | +| T-oc6-05 | Denial of service | pins panel flooding | low | mitigate | Only managers or policy-approved users pin (T-oc6-01). The existing chatchannels-api bucket (120/min) still applies | +| T-oc6-06 | Elevation of privilege | demo ChatAccess.CanPin / CanManageContext | medium | mitigate | CanPin requires community membership plus the moderator group, and fails closed on an empty group, nil func, nil db or any error. CanManageContext stays false, so moderators still cannot create channels (TestDemoChannelsSeededOnly) | +| T-oc6-07 | Spoofing / Repudiation | pin attribution | low | accept | pinned_by keeps the first pinner (idempotent FirstOrCreate, PHP D-57-02). An unpin is not attributed, the same as before; only moderators can unpin now | +| T-oc6-SC | Tampering | Go module installs | low | accept | No dependency is added. go.mod and go.sum are unchanged in both repos (Task 1 and Task 3 verify). The submodule moves to a local commit fetched from the sibling clone, not from a remote | + + + +- Plugin: gofmt clean; go vet ./...; go test -short ./...; full go test ./... with Docker; TestPinPolicy PASS (not SKIP); TestPHPMethodMap green with the six pin rows on TestPinPolicy. +- Demo: go vet ./...; go test -short ./...; Docker tests TestDemoPinPolicy, TestResetKeepsPlans, TestDemoModeratorDelete, TestDemoChannelsSeededOnly, TestDemoChatTracer and TestSeedChatSmoke green with the submodule at the plugin commit. +- Git: one plugin commit on main; three demo commits in the order host policy, then pointer bump alone, then e2e test; no trailers, no tags, no pushes; go.mod/go.sum untouched in both repos. + + + +- A channel member who is neither a manager nor approved by the host's PinPolicy cannot pin or unpin. The server enforces it and answers 404. +- A pin always references a live message of the channel's own conversation. +- Existing chatchannels hosts compile without change. The new rule is documented in the plugin README together with the way to restore the old behaviour. +- In the demo, only members of the configured moderator group pin and unpin. Every other member still reads the pins, and channel creation stays closed. + + + +Create `.planning/quick/261105-oc6-chatchannels-pin-policy/261105-oc6-SUMMARY.md` in summercms.go when done. Include: +- the plugin commit SHA and base; +- the three demo commit SHAs; +- the test results, including that TestPinPolicy and TestDemoPinPolicy ran against Postgres; +- confirmation that nothing was pushed or tagged and that go.mod/go.sum are unchanged; +- the three follow-ups from Task 3 step 4. + +Do not stage or commit anything in summercms.go; the batch coordinator commits the planning docs (SUMMARY, todo move, STATE). + diff --git a/.planning/quick/261105-oc6-chatchannels-pin-policy/261105-oc6-SUMMARY.md b/.planning/quick/261105-oc6-chatchannels-pin-policy/261105-oc6-SUMMARY.md new file mode 100644 index 00000000..c82b823d --- /dev/null +++ b/.planning/quick/261105-oc6-chatchannels-pin-policy/261105-oc6-SUMMARY.md @@ -0,0 +1,126 @@ +--- +phase: quick-261105-oc6 +plan: 01 +subsystem: sm-chatchannels-plugin, sm-demo-app +tags: [chatchannels, pins, security, WR-02, demo] +status: complete +requires: [] +provides: + - contracts.PinPolicy (optional CanPin host hook) in sm-chatchannels-plugin + - ChannelService.CanPinInChannel + - demo ChatAccess.CanPin answering from the moderator group +affects: [sm-bm-app (when it bumps chatchannels), vue-chat-layer (optional up-front pin hiding)] +tech-stack: + added: [] + patterns: [optional host interface discovered by type assertion, like sm-chat-plugin contracts.MessageModerator] +key-files: + created: + - ../sm-chatchannels-plugin/contracts/pin_policy.go + - ../sm-chatchannels-plugin/pins_test.go + modified: + - ../sm-chatchannels-plugin/contracts/access_resolver.go + - ../sm-chatchannels-plugin/classes/channel_service.go + - ../sm-chatchannels-plugin/api/pins.go + - ../sm-chatchannels-plugin/testdata/PHP_METHOD_MAP.md + - ../sm-chatchannels-plugin/README.md + - ../sm-demo-app/plugins/demo/classes/chat_context.go + - ../sm-demo-app/plugins/demo/classes/chat_context_test.go + - ../sm-demo-app/plugins/demo/chat.go + - ../sm-demo-app/reset_test.go + - ../sm-demo-app/README.md + - ../sm-demo-app/plugins/golem15/chatchannels (gitlink) + - ../sm-demo-app/chat_test.go +decisions: + - Pinning in chatchannels is manage-only unless the host resolver also implements contracts.PinPolicy (user-confirmed; sm-bm-app behaviour change accepted as a follow-up). + - The sm-demo-app pointer bump is committed untagged; check-pins.sh flags it until the plugin is tagged. +completed: 2026-11-05 +actuals: + tasks: 3 + commits: 4 + tokens: 9000 +--- + +# Quick 261105-oc6: chatchannels pin policy Summary + +Pinning and unpinning in chatchannels now need `CanManageContext` or an optional host `contracts.PinPolicy`, and a pinned message must be a live message of the channel's conversation. In the demo, only `demo-moderators` members pin and unpin. Every other member can still read the pins. + +## Commits + +Commits are in sibling repos, so the summercms.go ledger (`commits:` measured against summercms.go HEAD) does not apply. The hashes below are measured in each repo. + +**sm-chatchannels-plugin** (`main`, base `017843a` = v0.1.0; main had not moved past it) +- `a042536` fix: pinning needs CanManageContext or a host PinPolicy, and the message must be in the channel + +**sm-demo-app** (`master`, base `7b75d2c`, in this order) +- `ce800f2` feat: demo moderators answer chatchannels' pin policy (5 files) +- `7e4b9d3` chore: bump chatchannels to a042536 (pin policy, untagged) (gitlink only) +- `11d3afc` test: pinning in the demo channels is a moderator action (chat_test.go and chat_context.go) + +No commit has an attribution trailer. Nothing was pushed or tagged: `git branch -r --contains` and `git tag --points-at` are empty in both repos. go.mod and go.sum are unchanged in both repos (`017843a..main` in the plugin, `7b75d2c..HEAD` in the demo). + +## What changed + +- **Plugin** + - `contracts.PinPolicy` adds `CanPin(ctx, userID, contextType, contextID, channelID)`. It is optional and found by type assertion on the installed resolver. + - `ChannelService.requirePin` checks channel access first and returns its error unchanged, so `CanPin` is never asked for a user without access. It then requires `CanManageContext` or `CanPin`, else `ErrForbidden` (404). + - `requireMessageInChannel` counts live `golem15_chat_messages` rows in the channel's conversation (soft-deleted rows excluded). An unknown, deleted or foreign message, or a channel with no conversation, gets `ErrNotFound`. + - Unpinning does not check the message, so a stale pin stays removable. + - New exported nil-safe `CanPinInChannel`. The `AccessResolver` interface and every existing signature are unchanged. + - Handler docs updated, the PHP map's six pin rows now point at `TestPinPolicy`, and the README has the rule, a Usage example, API reference rows and Testing lines. +- **Demo** + - `NewChatAccess(db, moderatorGroup, isMember)` mirrors `NewForumAccess`, with a shared `member` helper. + - `CanPin` is true only for a community member in the moderator group, and fails closed on any error, empty group, nil func or nil db. `CanManageContext` stays false (D-07). + - `chat.go` installs the resolver with `p.chatModeratorGroup`. + - The reset test's walker is made a moderator before it pins. + - The README "Moderation." paragraph is updated. + - After the pointer bump, a compile-time `chancontracts.PinPolicy` assertion was added. + +## Test results + +- **Plugin** + - `go vet ./...` passes. + - `go test -short -count=1 ./...` passes. + - The full Docker-backed `go test -count=1 ./...` passes (~19 s). + - `TestPinPolicy` passed against Postgres, not skipped (1.45 s). `TestDenyAllHasNoPinPolicy` and `TestPHPMethodMap` pass. + - Mutation check: with the message-in-channel check disabled, `TestPinPolicy` fails ("pin unknown message: , want ErrNotFound"). The code was restored before committing. +- **Demo, Task 2** (submodule still at v0.1.0) + - `go vet ./...` and `go test -short ./...` pass, and `TestDemoChatAccess` passes. + - Docker tests `TestResetKeepsPlans`, `TestDemoModeratorDelete` and `TestDemoChannelsSeededOnly` pass. +- **Demo, Task 3** (submodule at a042536) + - `go vet ./...` and `go test -short ./...` pass. + - `TestDemoPinPolicy` passed against Postgres (2.74 s). + - `TestResetKeepsPlans`, `TestDemoModeratorDelete`, `TestDemoChannelsSeededOnly`, `TestDemoChatTracer` and `TestSeedChatSmoke` pass. + - The full Docker-backed `go test -count=1 ./...` passes (~57 s). + +## Deviations from Plan + +**1. [Scope note] The plan's Task 1 verify gate `test -z "$(gofmt -l .)"` fails in the plugin because of a formatting issue that was already there.** +- `fixture_plugin_test.go` (lines 86-89) has mis-aligned one-line method bodies at base `017843a`. This plan did not touch the file. +- The changed and new files are all gofmt-clean. I left the existing issue alone because it is out of scope, and the plan stages exactly seven paths. +- Fixing it is a one-line `gofmt -w fixture_plugin_test.go` in a later plugin commit. + +**2. [Test design] TestPinPolicy proves "first pinner wins" by briefly making `pinner` a manager for the second pin.** +- The first pinner is the manager. Then the base resolver is restored. +- The plan text did not fix who makes the second pin. Every other behaviour bullet is covered as written. + +Otherwise the plan was executed as written. + +## Follow-ups for the user (not acted on) + +1. **Release:** + - Tag sm-chatchannels-plugin `v0.1.1` on `main` (a042536) and push it. + - Bump sm-demo-app `go.mod` to require that tag. `scripts/check-pins.sh` reports chatchannels until then. + - Publish a changelog post at /changelog/plugins (`scripts/changelog-post.py publish v0.1.1 notes.md --excerpt "…" --repo ../sm-chatchannels-plugin`). + - Push the three sm-demo-app commits after that. +2. **sm-bm-app** also vendors chatchannels, and its `ChatAccess.CanManageContext` is false. When it bumps past a042536, its members stop pinning unless it implements `CanPin`. To keep the old behaviour, answer `CanPin` with its own `CanAccessContext`. +3. **vue-chat-layer** could hide the pin action up front from the bootstrap's `moderator` flag. Today `togglePin` hides it only after the first 4xx refusal. + +## Threat Flags + +None: the change narrows existing surface and adds no endpoint. + +## Self-Check: PASSED + +- Files exist: `sm-chatchannels-plugin/contracts/pin_policy.go`, `sm-chatchannels-plugin/pins_test.go`, `sm-demo-app/chat_test.go` (TestDemoPinPolicy). +- Commits exist: plugin `a042536`; demo `ce800f2`, `7e4b9d3`, `11d3afc`. +- The demo gitlink at HEAD equals the plugin `main` (a0425366487b1c8e8a9566a74fe6b35613f529da). diff --git a/.planning/quick/261105-oc7-chatvideo-hardening/261105-oc7-PLAN.md b/.planning/quick/261105-oc7-chatvideo-hardening/261105-oc7-PLAN.md new file mode 100644 index 00000000..552cbdc8 --- /dev/null +++ b/.planning/quick/261105-oc7-chatvideo-hardening/261105-oc7-PLAN.md @@ -0,0 +1,440 @@ +--- +phase: quick-261105-oc7 +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - ../sm-chatvideo-plugin/classes/video_service.go + - ../sm-chatvideo-plugin/classes/authorizer.go + - ../sm-chatvideo-plugin/classes/livekit.go + - ../sm-chatvideo-plugin/classes/revoked_access.go + - ../sm-chatvideo-plugin/boot.go + - ../sm-chatvideo-plugin/live_membership_test.go + - ../sm-chatvideo-plugin/contracts/call_policy.go + - ../sm-chatvideo-plugin/contracts/call_policy_test.go + - ../sm-chatvideo-plugin/api/errors.go + - ../sm-chatvideo-plugin/lang/en/lang.yaml + - ../sm-chatvideo-plugin/lang/pl/lang.yaml + - ../sm-chatvideo-plugin/call_policy_test.go + - ../sm-chatvideo-plugin/README.md + - ../sm-chatvideo-plugin/plugin.yaml + - ../sm-demo-app/plugins/golem15/chatvideo + - ../sm-demo-app/plugins/demo/video.go + - ../sm-demo-app/plugins/demo/plugin.go + - ../sm-demo-app/video_test.go + - ../sm-demo-app/config/golem15.chatvideo.yaml + - ../sm-demo-app/README.md + - ../vue-chatvideo-layer/app/composables/useIncomingCalls.ts + - ../vue-chatvideo-layer/vitest.config.ts + - ../vue-chatvideo-layer/tests/useIncomingCalls.test.ts + - ../vue-chatvideo-layer/README.md +autonomous: true +requirements: + - todo:chatvideo-live-membership + - todo:chatvideo-dm-only-host-hook + - todo:chatvideo-incoming-handle-leak + +must_haves: + truths: + - "With a host chat context that implements golem15.chat's contracts.LiveMembership (RequireLiveMembership true), a conversation participant whose HasMember answers false gets 404 from POST /_chatvideo/api/v1/conversations/{id}/video/join, /leave, /end, GET /video/status and GET /call-links/{code}, and the registered video authorizer denies them video:session:{id}. When HasMember answers true again, access comes back. Start neither invites nor rings such a member: no participant row, and they are not in the Incoming recipient list" + - "A context that does not implement LiveMembership (or answers false) and a conversation with no context keep the participant-row behaviour unchanged. The existing tests (TestHTTPTracer, TestVideoAuthorizer, TestReconcileEmptyRoom, TestUserDeletionReassignsSession, TestGoldenEvents) pass without edits to their assertions" + - "A ban (golem15.user AccessRevokedEvent with Reason userclasses.AccessRevokedBan) or a deactivation (DeactivatingEvent) marks the user's joined participant rows of live sessions left, with left_at set, inside the event's transaction. Only after the commit does it remove the user's LiveKit identity from every live room where they were joined or had left, and publish participant-left for the rows that were joined. The call goes on for everyone else. A rolled-back ban changes nothing. AccessRevokedEvent with any other reason (password reset or change) leaves calls alone" + - "VideoService.SetCallPolicy installs an optional contracts.CallPolicy. With no policy installed, every member may start a call, as before. With contracts.DMOnly installed, POST .../video/start in a group conversation answers 403 {\"error\":\"Video calls are not available in this conversation\"}, and it creates no session, room or participant row and rings nobody. A DM start is unchanged. A policy error answers 500 and creates nothing. Joining a call that is already running (POST .../video/join) is never checked against the policy. A non-participant still gets 404, and the policy is not consulted for them" + - "sm-demo-app installs contracts.DMOnly at boot. TestDemoVideoStart asserts that a channel start is refused with 403 whether or not the channel fits max_participants: the rule no longer depends on seed data. DM calls behave as before, and the config comment and README say the server enforces the rule" + - "useIncomingCalls never keeps more than one session handle. A subscribe that resolves after a newer ring, a dismiss, accept, decline or the ring timeout has its handle closed at once. Only the latest ring's handle is kept, and clear() closes it. The new test fails on the code before the fix" + - "Additive: no exported sm-chatvideo-plugin identifier is removed or changes its signature (AuthorizeVideo, AuthorizeVideoCheck, AuthorizeVideoLookup, NewVideoService, VideoOptions, VideoService methods). No go.mod changes in any repo. No framework (summercms.go) code changes. Nothing is pushed or tagged. No commit has a co-author trailer" + artifacts: + - path: ../sm-chatvideo-plugin/classes/video_service.go + provides: "MembershipCheck type, VideoOptions.Membership and VideoOptions.CallPolicy, SetMembership, SetCallPolicy, ErrCallNotAllowed; requireParticipant goes through the membership check; Start filters invitees and applies the policy" + - path: ../sm-chatvideo-plugin/classes/authorizer.go + provides: "AuthorizeVideoMembership(getDB, member); AuthorizeVideoLookup delegates with a nil check" + - path: ../sm-chatvideo-plugin/classes/revoked_access.go + provides: "VideoService.RemoveFromCalls(ctx, tx, userIDs) with after-commit LiveKit removal" + - path: ../sm-chatvideo-plugin/contracts/call_policy.go + provides: "contracts.CallPolicy, contracts.CallPolicyFunc, contracts.DMOnly" + - path: ../sm-demo-app/plugins/demo/video.go + provides: "registerVideo installs contracts.DMOnly on the published *classes.VideoService" + - path: ../vue-chatvideo-layer/app/composables/useIncomingCalls.ts + provides: "a sequence number per watchSession, bumped by clear(); stale handles are closed" + key_links: + - from: "../sm-chatvideo-plugin/boot.go chatMembership(app)" + to: "golem15.chat classes.Service.IsParticipant (participant row + contracts.LiveMembership)" + via: "classes.MembershipCheck, used by bootService (VideoOptions.Membership) and bootRealtime (AuthorizeVideoMembership)" + - from: "../sm-chatvideo-plugin/boot.go bootListeners" + to: "classes.VideoService.RemoveFromCalls -> lagoon.AfterCommit -> RoomAPI.RemoveParticipant" + via: "app.Events.Listen for *userclasses.AccessRevokedEvent (reason ban) and *userclasses.DeactivatingEvent" + - from: "../sm-demo-app/plugins/demo/video.go" + to: "classes.VideoService.SetCallPolicy(contracts.DMOnly)" + via: "lagoon.OnDatabase after chatvideo publishes its service" + - from: "../sm-chatvideo-plugin/api/errors.go ErrCallNotAllowed -> 403" + to: "vue-chatvideo-layer callMachine failureReason(403) = 'forbidden'" + via: "existing frontend mapping; no layer change needed for the refusal" +--- + + +Close the three chatvideo todos from the phase 20 code review in three sibling repos, with additive changes only: IN-02 (`chatvideo-live-membership`), IN-01 (`chatvideo-dm-only-host-hook`) and IN-07 (`chatvideo-incoming-handle-leak`). + +1. **Live membership and bans (sm-chatvideo-plugin).** chatvideo checks participant rows only. Change `requireParticipant` and the video authorizer to go through golem15.chat's `Service.IsParticipant`, which also asks the host context about current membership when the context implements `contracts.LiveMembership`. When a user is banned (or deactivated), chatvideo removes them from every live LiveKit room after the transaction commits. +2. **DM-only host hook (sm-chatvideo-plugin, then sm-demo-app).** Add an optional `contracts.CallPolicy` with `CanStartCall`, a ready-made `contracts.DMOnly` policy and `VideoService.SetCallPolicy`. The demo installs `DMOnly`, so the server refuses channel calls whatever the seeded participant count is. +3. **Stale handles (vue-chatvideo-layer).** `useIncomingCalls` keeps a sequence number per `watchSession` and closes every handle that is not the latest. + +Live reads at planning time (all four repos clean; sm-chatvideo-plugin on `main` at 0f08c35 = v0.1.1, vue-chatvideo-layer on `main` at 2313a4e = v0.1.1, sm-demo-app on `master` at da03c06): +- `classes/video_service.go`: `requireParticipant` (lines 425-443) loads the conversation (missing: ErrNotFound) and then counts `golem15_chat_participants` rows (none: ErrNotParticipant). Start, Status, CallLink and `sessionForActor` (and through it Join, Leave and End) all call it. `Start` (84-168) plucks every participant's user_id into memberIDs, refuses with ErrTooMany when `len(memberIDs) > max_participants`, and passes memberIDs to `invite` and to `notify.Incoming`. `Leave` (207-228) shows the left pattern: Updates status `models.ParticipantLeft` and `left_at`, then calls `rooms.RemoveParticipant(ctx, room, strconvUint(actorID))`, then `notify.ParticipantLeft`. +- `classes/authorizer.go`: `AuthorizeVideoLookup(getDB)` (54-79) loads the session and counts participant rows of `session.ConversationID`. `bootRealtime` registers it under the namespace "video". +- `classes/user_deletion.go` is the pattern for work that has to wait for the commit: `q()` builds sessions on tx with `Session(&gorm.Session{NewDB: true})`; then `lagoon.AfterCommit(ctx, q(), fn)`, `context.WithoutCancel`, `roomCallTimeout` (10 s), and a `pluginLog.Warn` unless the error is ErrNotConfigured. A nil tx falls back to the service DB. +- golem15.chat: `(*classes.Service).IsParticipant(ctx, conversationID, userID) (bool, error)` exists in the required v0.1.3 and in the workspace checkout (`../sm-chat-plugin`, v0.1.5). It answers whether a participant row exists and, if the conversation's context implements `contracts.LiveMembership` and `RequireLiveMembership()` is true, whether `HasMember` holds. Chat publishes `*classes.Service` from its own `lagoon.OnDatabase` hook. Chat boots first (chatvideo `Requires()` golem15.chat), and OnDatabase hooks run in FIFO order. +- golem15.user: `AccessRevokedEvent{UserIDs []uint, Before time.Time, Reason string, Tx *gorm.DB}`, the `AccessRevokedBan` reason constant and `DeactivatingEvent{User *models.User, Tx *gorm.DB}` exist in the required v0.1.13 and in the workspace user plugin (fonoteka.go/plugins/golem15/user, v0.1.14). The admin ban action fires AccessRevokedEvent (reason "ban") inside its transaction (users_admin_controller.go:678). sm-mfa-plugin and sm-oauthserver-plugin already listen to it. No go.mod change is needed anywhere. +- lighthouse has no server-side unsubscribe or disconnect API. A Centrifugo subscription token expires (realtime.centrifugo.token_ttl, default 3600 s). Refreshing it needs the user's JWT (revoked by a ban) and runs the authorizer again. +- The production `sdkRooms.RemoveParticipant` already swallows a missing room or participant (`isMissingRoom`). `FakeRooms` has no way to put a named identity in a room: `SetCount` creates identities "0".."n-1". +- The HTTP error mapping (`api/errors.go` writeVideoError) has no 403 today. vue-chatvideo-layer `failureReason` maps 403 to the reason key 'forbidden' ("You can't join this call.") and 422 to 'tooMany'. A policy refusal must therefore be a 403 to read correctly in the existing UI. +- `TestFixtureAbsentFromProduction` fails when a production file (.go, .yaml or .md, including README.md) contains `acme.teams`, `teamsPlugin`, or any of the words queststream, family, parent, child, wavepath or fonoteka. Do not use those words in production comments or the README. +- sm-demo-app: `plugins/golem15/chatvideo` is a submodule detached at v0.1.1 (0f08c35), and `go.mod` requires sm-chatvideo-plugin v0.1.1. `go.work` uses the submodule checkout, so dev builds and `go test` compile the submodule. `scripts/check-pins.sh` and the release build (`scripts/build.sh`, GOWORK=off) use the go.mod version. `TestDemoVideoStart` (video_test.go:97) pins "no type rule on the server": a channel start is refused with 422 only because of `max_participants: 4`, and is accepted (201) after the channel is trimmed to 2 participants. The demo plugin installs other plugins' host policies from `lagoon.OnDatabase` hooks (chat.go registerChat/registerForum); Boot runs a list of steps in plugin.go. +- vue-chatvideo-layer `useIncomingCalls.ts` (88 lines): `IncomingRuntime {registered, channel, timer}` sits on `nuxtApp._chatvideoIncoming`. `watchSession` closes `rt.channel`, awaits `subscribe(videoSessionChannel(id), {onPublication})`, and keeps the handle when `pending.value?.sessionId === call.sessionId`. Two rings for the same session both pass that check, so the earlier handle leaks. Registration is guarded by `import.meta.client`, which is undefined under vitest. A scratchpad check confirmed that vitest 3.2.7 replaces it with `define: { 'import.meta.client': 'true' }`. The layer has no vitest config. Baseline: `pnpm test` gives 127 tests green; chatvideo `go vet` and `go test -short ./...` are green. + +Commits (four, in this order, each staged by explicit path, with no co-author trailer and nothing pushed or tagged): +1. sm-chatvideo-plugin: `fix: honour chat live membership and remove banned users from live calls` (Task 1) +2. sm-chatvideo-plugin: `feat: optional CallPolicy host hook and a DM-only policy` (Task 2, first part) +3. sm-demo-app: `feat: video calls in DMs only, enforced on the server by chatvideo's CallPolicy` (Task 2, second part: submodule pointer bump and wiring in one commit, so the demo compiles at every commit) +4. vue-chatvideo-layer: `fix: the incoming-call runtime closes stale session handles` (Task 3) + +Release state after this plan (put this in the SUMMARY): the demo's `plugins/golem15/chatvideo` pointer is ahead of its go.mod pin (still v0.1.1). Dev builds and tests compile through go.work, but `scripts/check-pins.sh` reports the mismatch and the GOWORK=off release build does not compile until the user tags sm-chatvideo-plugin (next v0.1.x), pushes it before the demo, and commits `chore: require sm-chatvideo-plugin vX` (go.mod and go.sum). The same goes for a vue-chatvideo-layer tag and a vue-demo-app bump. The executor does none of these. + +Not in scope (the orchestrator records them as follow-ups): +- Ending an already open `video:session` Centrifugo subscription on a ban. lighthouse has no unsubscribe API (a framework change). The subscription's refresh needs the revoked JWT and re-runs the now live-aware authorizer. +- Revoking an already minted LiveKit join token. LiveKit has no revocation. A removed user could reconnect with the token until token_ttl (900 s) runs out. Hosts may lower `golem15.chatvideo.token_ttl`. +- Removing a user from live rooms when they leave a host context without a ban. There is no generic event for that. Live checks still refuse every new join, status, call link and subscribe. +- Ending a call whose last joined participant was banned. The reconciler already ends empty rooms within 5 minutes. +- The logout half of IN-07 (fixed in phase 20 plan 08). +- Tags, pushes, go.mod bumps, the vue-demo-app layer bump, moving the three todos to completed, and the g15office comment. The orchestrator's closing docs commit handles the todos and g15office. + +Purpose: a ban ends call access at once, as it already ends chat access. The demo's DM-only promise holds on the server, not only in the seed data and the UI. The incoming-call prompt no longer leaks realtime subscriptions. +Output: four commits in three repos and 261105-oc7-SUMMARY.md. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@.planning/STATE.md +@./CLAUDE.md +@.planning/todos/pending/chatvideo-live-membership.md +@.planning/todos/pending/chatvideo-dm-only-host-hook.md +@.planning/todos/pending/chatvideo-incoming-handle-leak.md +@.planning/phases/20-demo-instance-with-every-shared-plugin/20-REVIEW.md + + +Existing code, verified at planning time. Paths are relative to /media/nvme/dev/golem15/summercms.io/summercms. + +sm-chatvideo-plugin (package classes) +- Errors: ErrNotConfigured, ErrNotFound, ErrNotParticipant, ErrAlreadyEnded, ErrTooMany (video_service.go:20-31). +- type VideoOptions struct { DB *gorm.DB; Rooms RoomAPI; Cfg *compass.Config; Now func() time.Time } +- type VideoService struct { db, rooms, cfg, now, notify } ; func NewVideoService(opt VideoOptions) *VideoService ; SetRooms(RoomAPI) ; SetNotifier(callNotify) +- func (s *VideoService) Start(ctx, actorID, conversationID uint) (*StartResult, error) ; Join / Leave / End / Status / CallLink / Reconcile / OnUserDeleting(ctx, tx *gorm.DB, userID uint) error +- func (s *VideoService) requireParticipant(ctx, conversationID, userID uint) error (unexported) +- func (s *VideoService) invite(ctx, sessionID, starter uint, memberIDs []uint, now time.Time) error (unexported) +- callNotify interface: Incoming(ctx, *models.Session, callerID uint, recipientIDs []uint), Accepted, Declined, Ended(ctx, session, endedBy), ParticipantJoined(ctx, session, userID), ParticipantLeft(ctx, session, userID) +- authorizer.go: type ConversationCheck func(ctx, userID, sessionID uint) bool ; AuthorizeVideo(db *gorm.DB) ; AuthorizeVideoCheck(check ConversationCheck) ; AuthorizeVideoLookup(getDB func() *gorm.DB) ; allowVideo(userID, sessionID) ; parseVideoSessionChannel(channel) (uint, bool) +- livekit.go: RoomAPI { CreateRoom(ctx, name) error; DeleteRoom(ctx, name) error; RemoveParticipant(ctx, room, identity string) error; ParticipantCount(ctx, name) (int, error); ActiveRoomNames(ctx) ([]string, error) } ; FakeRooms { mu sync.Mutex; rooms map[string]map[string]struct{}; created []string } with CreateRoom, DeleteRoom, RemoveParticipant, ParticipantCount, ActiveRoomNames, CreatedNames, SetCount(name, n, create), Drop(name) +- user_deletion.go: const roomCallTimeout = 10 * time.Second ; the AfterCommit pattern described in the objective. +- logger.go: var pluginLog = diary.For("chatvideo") +- models: StatusPending/StatusActive/StatusEnded ; ParticipantInvited/ParticipantJoined/ParticipantDeclined/ParticipantLeft ; Session{ID, ConversationID, ActiveConversationID *uint, LivekitRoomName, Status, StartedBy, StartedAt, ...} ; participant columns video_session_id, user_id, status, joined_at, left_at. +- strconvUint(n uint) string (video_service.go:564), the LiveKit identity of a user. + +sm-chatvideo-plugin (package chatvideo, root) +- boot.go: init registers boot steps env, realtime, service, listeners. bootRealtime registers classes.AuthorizeVideoLookup(func() *gorm.DB { app.Lookup[*gorm.DB]() }) under "video". bootService (inside lagoon.OnDatabase) builds classes.NewVideoService(VideoOptions{DB, Rooms, Cfg}), SetNotifier, then publishes p and svc. bootListeners does app.Events.Listen[*userclasses.DeletingEvent](PluginID, p.onUserDeleting). Imports userclasses "git.golem15.com/golem15/sm-user-plugin/classes". +- api/errors.go: string constants errNotFound ... errTooMany ; writeVideoError(w, what, err) switch: NotFound/NotParticipant → 404, NotConfigured → 503, AlreadyEnded/TooMany → 422, default → 500. +- lang/en/lang.yaml and lang/pl/lang.yaml: errors.{auth_required, not_found, not_configured, not_participant, session_not_found, already_ended, too_many}. +- Tests (package chatvideo_test): ensurePostgres(t) ; bootVideo(t, ids []string, extraEnv ...string) *videoEnv{app, gdb, plugins, svc, rooms *classes.FakeRooms, rt *lighthouse.Service} ; (e) user(t) *usermodels.User (activated) ; (e) conversation(t, createdBy, participants ...uint) *chatmodels.Conversation (Type group, no context) ; (e) count(t, table, where, args...) int64 ; (e) session(t, id) models.Session ; const participantsTable ; bootWeb(t) *webEnv{*videoEnv, h} ; (e *webEnv) frontendToken(t, user) string ; (e *webEnv) call(t, method, path, token, body) *httptest.ResponseRecorder (path below chatvideo.APIPrefix) ; decodeObject(t, rec) map[string]any ; deleteUserLikeAdmin shows the lagoon.Transaction + app.Events.Fire pattern ; const fixturePluginID = "acme.teams". + +golem15.chat (sm-chat-plugin) +- classes.Service: IsParticipant(ctx, conversationID, userID uint) (bool, error). +- contracts.Context: ID() uint ; Type() string ; Members(ctx) ([]uint, error) ; HasMember(ctx, userID) (bool, error) ; CanOverseeChats(ctx, userID) (bool, error) ; ChatEnabled() bool ; GroupName() string ; CanActFor(ctx, actorID, memberID) bool ; BroadcastChannel() string (two or three colon-separated segments). +- contracts.LiveMembership: RequireLiveMembership() bool. +- contracts.Registry (published as *contracts.Registry): Add(pluginID string, reg Registration) error ; Registration{Alias string; Loader Loader; LegacyPHPClass string} ; type Loader func(ctx, db *gorm.DB, id uint) (Context, error). +- models.Conversation{Type string; ContextType *string; ContextID *uint; ...} with columns context_type and context_id ; models.TypeDM = "dm", models.TypeGroup = "group". + +golem15.user (sm-user-plugin classes) +- AccessRevokedEvent{UserIDs []uint; Before time.Time; Reason string; Tx *gorm.DB} ; const AccessRevokedBan = "ban", AccessRevokedPasswordChanged = "password_changed" (plus password_reset, password_set_by_admin). +- DeactivatingEvent{User *models.User; Tx *gorm.DB}. + +lighthouse (framework, unchanged) +- (*lighthouse.Service).Registry() *Registry ; (*Registry).Get(namespace) (Authorizer, bool) ; Authorizer.Authorize(ctx, userID uint, channel string) Result ; Result.Allowed bool, Result.Reason() string ; lighthouse.Allowed(info) / lighthouse.Denied(reason). + +sm-demo-app +- plugins/demo/plugin.go: Boot runs p.wireRealtime then the steps p.registerChat, p.registerForum, p.registerCommands, p.registerModeration. Requires() already lists golem15.chatvideo. +- plugins/demo/chat.go registerForum is the template: app.HasPlugin guard, lagoon.OnDatabase, app.Lookup of the service, error "golem15.demo: golem15.forum must be listed before golem15.demo in summer.yaml", then Set...(policy). +- video_test.go TestDemoVideoStart: bootAPI(t, env, set) ; a.member(t, email) (id, token) ; a.openDM(t, token, otherID) ; a.openChannel(t, token, "general") ; a.call(t, method, path, token, body) ; scalar[T](t, a.Demo, sql, args) ; exec(t, a.DB, sql, args) ; fakeRoomService.called("CreateRoom"). +- config/golem15.chatvideo.yaml lines 1-15: a comment block ending "The server does not restrict calls by conversation type; the site offers them in DMs only." README.md lines 199-201: the "**Calls.**" paragraph says the same. + +vue-chatvideo-layer +- app/composables/useIncomingCalls.ts: export const RING_MS = 45_000 ; interface IncomingRuntime { registered: boolean; channel: ChannelHandle | null; timer: ReturnType | null } ; export function useIncomingCalls() returning { pending, accept(), decline(), dismiss }. It imports onUserEvent, subscribe and ChannelHandle from 'vue-chat-layer/app/lib/realtime', parseIncomingCall, videoEvent and videoSessionChannel from '../lib/videoClient', and useVideoClient from './useCall' (which returns nuxtApp._chatvideoClient when set). +- ChannelHandle { close(): void; resubscribe(): void }. onUserEvent(handler) returns an unsubscribe function. parseIncomingCall reads {event: 'video:call:incoming', payload: {session_id, conversation_id, caller_name}}. +- tests/useCall.test.ts shows the stubbing style: vi.mock('vue-chat-layer/app/lib/realtime', ...), vi.stubGlobal('useNuxtApp', () => ({ _chatvideoClient: client })), vi.unstubAllGlobals() in afterEach. +- package.json: "test": "vitest run" ; devDependencies vitest 3.2.7, vue 3.5.35. + + + + + + + Task 1: chatvideo honours live membership end to end and removes banned users from live rooms + ../sm-chatvideo-plugin/classes/video_service.go, ../sm-chatvideo-plugin/classes/authorizer.go, ../sm-chatvideo-plugin/classes/livekit.go, ../sm-chatvideo-plugin/classes/revoked_access.go, ../sm-chatvideo-plugin/boot.go, ../sm-chatvideo-plugin/live_membership_test.go, ../sm-chatvideo-plugin/README.md + + - ../sm-chatvideo-plugin/classes/video_service.go (whole file; requireParticipant, Start, Leave) + - ../sm-chatvideo-plugin/classes/authorizer.go + - ../sm-chatvideo-plugin/classes/user_deletion.go (the AfterCommit pattern to copy) + - ../sm-chatvideo-plugin/boot.go + - ../sm-chatvideo-plugin/harness_test.go, http_tracer_test.go (lines 1-80), user_deletion_test.go (lines 1-75) + - ../sm-chat-plugin/classes/service.go lines 480-560 (IsParticipant, liveHost, stillMember) + - ../sm-user-plugin/classes/access_revoked.go and classes/events.go lines 70-120 + + + - Live context (RequireLiveMembership true, members {A, B}), conversation with participants A, B and C: A's HTTP start answers 201. A's row is joined, B's is invited, and C has no row (not invited, not rung). + - After B leaves the member set: B's POST join answers 404 and B's GET status answers 404. The registered "video" authorizer denies B on video:session:{id} and allows A. After B is added back: B's join answers 200 and the authorizer allows B. + - A context with RequireLiveMembership false: a participant outside the member set still starts, joins and is authorized (participant-row behaviour). + - A ban: A starts and B joins, and the fake room holds identities A and B. An AccessRevokedEvent{Reason: ban, UserIDs: [B]} fired inside a lagoon.Transaction leaves B in the room until the commit. After the commit B is gone from the room, A is still in it, B's row is left with left_at set, and the session is still active. + - AccessRevokedEvent with Reason password_changed for A: A stays in the room and A's row stays joined. + - A ban for A in a transaction that then returns an error (rolled back): A stays in the room and A's row stays joined. + - DeactivatingEvent{User: A} committed: A is removed from the room and A's row is left. + + +Work in /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin. First run `git status --short`. If any file this task touches has uncommitted edits from another session, stop and report instead of editing. + +RED first. Add two methods to `FakeRooms` in classes/livekit.go. `AddParticipant(room, identity string)` puts identity into an existing fake room (a missing room stays missing, as SetCount without create). `HasParticipant(room, identity string) bool`. Both take the mutex. Then write live_membership_test.go (package chatvideo_test, Docker-backed: call ensurePostgres(t) first so it skips under -short) with the cases in ``, in three test functions: + +- **TestVideoLiveMembership** (bootWeb). Add a test-only type implementing chat's contracts.Context and contracts.LiveMembership. It holds a mutex-guarded member set and a `live` flag; HasMember reads the set, RequireLiveMembership returns the flag, ChatEnabled is true, BroadcastChannel is "acme:team:1", CanActFor uses chatcontracts.DefaultCanActFor, CanOverseeChats is false. Look up `*chatcontracts.Registry` from env.app and Add(fixturePluginID, Registration{Alias "acme_team", Loader returning that context for id 1, nil otherwise}). Point the test conversation at it with an Updates of context_type "acme_team" and context_id 1. Check rows with env.count on participantsTable. Get the authorizer with `env.rt.Registry().Get("video")`. +- **TestVideoMembershipWithoutLiveCheck**: same setup with live false. +- **TestVideoBanRemovesFromCall** (bootVideo). Fire the events through env.app.Events.Fire inside lagoon.Transaction, as deleteUserLikeAdmin does. Seed the fake room with AddParticipant using the users' decimal ids as identities. + +Run the tests against the unchanged service and record that they fail for the membership, ban and deactivation assertions, not only for compile errors. + +GREEN, in classes/video_service.go: +- Add the exported type `MembershipCheck`: a func of (ctx context.Context, conversationID, userID uint) returning (bool, error). Its doc comment says golem15.chat's Service.IsParticipant has this shape. +- Add `Membership MembershipCheck` to VideoOptions and a `membership` field to VideoService, copied by NewVideoService. Add `SetMembership(m MembershipCheck)`, nil-receiver safe like SetNotifier, where nil restores the participant-row check. +- requireParticipant: keep the conversation load and its ErrNotFound. Then, when s.membership is set, call it. An error is returned as is, and false is ErrNotParticipant. When it is not set, keep today's participant-row count exactly. Status, CallLink, Join, Leave and End inherit this through requireParticipant and sessionForActor; do not change their signatures. +- Start: keep the ErrTooMany check on the raw memberIDs, unchanged and still before any per-member work. Then, when s.membership is set, build the recipient list from memberIDs in order. Keep actorID unconditionally (it already passed requireParticipant) and drop every other id for which the check answers false; a check error aborts Start with that error. Pass this list to invite and to notify.Incoming instead of memberIDs. With no membership set, the list is memberIDs. + +In classes/authorizer.go, add the exported `AuthorizeVideoMembership(getDB func() *gorm.DB, member MembershipCheck) lighthouse.AuthorizerFunc`. It is AuthorizeVideoLookup's body, except that after loading the session it calls member(ctx, session.ConversationID, userID) when member is non-nil. An error or false is Denied("not a conversation participant"). With a nil member it keeps the participant-row count. AuthorizeVideoLookup becomes a one-line delegation with a nil member. AuthorizeVideo and AuthorizeVideoCheck stay as they are. + +Create classes/revoked_access.go with the exported `(s *VideoService) RemoveFromCalls(ctx context.Context, tx *gorm.DB, userIDs []uint) error`. Its doc comment says it removes the users from every live call, for a ban or a deactivation. It mirrors OnUserDeleting's structure: nil and empty guards; a nil tx falls back to s.db and a nil ctx to Background; the q() helper on tx. Steps: +1. Load the live sessions (status pending or active, active_conversation_id NOT NULL) in which any of the users has a participant row with status joined or left. +2. Inside tx, update the users' rows with status joined in those sessions to status left with left_at now (s.now().UTC()). +3. Register one lagoon.AfterCommit callback on q(). It removes each (room, strconvUint(userID)) pair whose row was joined or left through s.rooms.RemoveParticipant, using roomCallTimeout per call and a Warn log with op remove_participant unless the error is ErrNotConfigured. It then calls s.notify.ParticipantLeft(base, session, userID) for the rows that were joined. +4. Return nil without registering a callback when there is nothing to do. +It never ends a session and never deletes rows. + +In boot.go: +- Add an unexported `chatMembership(app *backpack.App) classes.MembershipCheck`. On each call it looks up `*chatclasses.Service` (import "git.golem15.com/golem15/sm-chat-plugin/classes" as chatclasses) and returns its IsParticipant answer. A missing or nil service returns false with an unexported error naming golem15.chat: fail closed, never fall back to the participant row. +- bootRealtime registers classes.AuthorizeVideoMembership with the existing getDB closure and chatMembership(app). +- bootService sets Membership: chatMembership(app) in VideoOptions. +- bootListeners adds two listeners next to the DeletingEvent one. `app.Events.Listen[*userclasses.AccessRevokedEvent](PluginID, p.onAccessRevoked)` calls p.service.RemoveFromCalls(ctx, ev.Tx, ev.UserIDs) only when ev.Reason == userclasses.AccessRevokedBan (other reasons return nil). `app.Events.Listen[*userclasses.DeactivatingEvent](PluginID, p.onUserDeactivating)` calls it with the single user id. Both return nil on a nil plugin, event, user or service, as onUserDeleting does. + +Update README.md in the same commit: +- Overview: production registers AuthorizeVideoMembership with chat's Service.IsParticipant, so a host context implementing chat's contracts.LiveMembership ends call access as soon as the user leaves it. +- Features: a "Live membership." bullet (requireParticipant, the authorizer, invitees filtered at start) and a "Bans." bullet (AccessRevokedEvent with reason ban and DeactivatingEvent; rows left in the transaction; LiveKit removal and participant-left after the commit; the residual window up to token_ttl because LiveKit tokens cannot be revoked). +- API reference rows: classes.MembershipCheck, classes.VideoService.SetMembership, classes.VideoService.RemoveFromCalls, classes.AuthorizeVideoMembership, classes.FakeRooms.AddParticipant, classes.FakeRooms.HasParticipant. +- Dependencies: chat for Service.IsParticipant; the user plugin for AccessRevokedEvent and DeactivatingEvent (both in v0.1.13). +- Testing: the three new test names in the `go test -run` line. + +Keep the forbidden fixture words out of every production file and the README (see objective). Run gofmt. + +Commit in sm-chatvideo-plugin, staging these seven paths explicitly: `fix: honour chat live membership and remove banned users from live calls`. Add a short body naming IN-02. No co-author trailer. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin && test -z "$(gofmt -l .)" && go vet ./... && go test -short ./... && go test -count=1 -run '^(TestVideoLiveMembership|TestVideoMembershipWithoutLiveCheck|TestVideoBanRemovesFromCall|TestHTTPTracer|TestUserDeletionReassignsSession|TestReconcileEmptyRoom|TestGoldenEvents|TestFixtureAbsentFromProduction)$' . + + + - The three new tests run (not skipped) and pass. They failed before the implementation for the membership, ban and deactivation assertions. + - `gofmt -l .` prints nothing. `go vet ./...` and `go test -short ./...` are green. The full `go test -count=1 ./...` (Docker; if the socket is denied, run it through `sudo -u golem` as CLAUDE.md says) is green. + - `git -C ../sm-chatvideo-plugin diff HEAD~1 --stat` lists exactly the seven task files. go.mod and go.sum are unchanged. + - `git -C ../sm-chatvideo-plugin log -1 --format=%B | grep -ci 'co-authored'` prints 0. + + chatvideo consults chat's live membership for every participant check, the authorizer and the start invitees. A ban or deactivation removes the user from live rooms after the commit. Existing behaviour is unchanged for hosts without LiveMembership. One commit in sm-chatvideo-plugin. + + + + Task 2: optional CallPolicy host hook with a DM-only policy, installed by the demo + ../sm-chatvideo-plugin/contracts/call_policy.go, ../sm-chatvideo-plugin/contracts/call_policy_test.go, ../sm-chatvideo-plugin/classes/video_service.go, ../sm-chatvideo-plugin/api/errors.go, ../sm-chatvideo-plugin/lang/en/lang.yaml, ../sm-chatvideo-plugin/lang/pl/lang.yaml, ../sm-chatvideo-plugin/call_policy_test.go, ../sm-chatvideo-plugin/README.md, ../sm-chatvideo-plugin/plugin.yaml, ../sm-demo-app/plugins/golem15/chatvideo, ../sm-demo-app/plugins/demo/video.go, ../sm-demo-app/plugins/demo/plugin.go, ../sm-demo-app/video_test.go, ../sm-demo-app/config/golem15.chatvideo.yaml, ../sm-demo-app/README.md + + - ../sm-chatvideo-plugin/classes/video_service.go (after Task 1) + - ../sm-chatvideo-plugin/api/errors.go, lang/en/lang.yaml, lang/pl/lang.yaml + - ../sm-chatchannels-plugin/contracts/access_resolver.go (package doc and comment style of a host seam) + - ../sm-chat-plugin/contracts/moderator.go (comment style of an optional hook) + - ../sm-demo-app/plugins/demo/chat.go lines 73-89 (registerForum template) and plugins/demo/plugin.go lines 80-100 + - ../sm-demo-app/video_test.go lines 88-200 + + + - contracts (-short): DMOnly answers true for "dm" and false for "group" and "". CallPolicyFunc passes userID, conversationID and conversationType through unchanged. + - No policy installed: a group-conversation start answers 201 (unchanged). + - SetCallPolicy(contracts.DMOnly): a group start answers 403 with error "Video calls are not available in this conversation". The conversation has zero sessions and zero chatvideo participant rows, and env.rooms.CreatedNames() did not grow. A DM start (conversation Type "dm") answers 201. + - A recording CallPolicyFunc sees the actor id, the conversation id and "group". + - A policy that returns an error: start answers 500 and creates no session. + - A non-participant's start answers 404 and the recording policy is not called. + - A group session started before the policy was installed: with DMOnly installed, start answers 403 and another member's POST join answers 200. + - SetCallPolicy(nil): a group start answers 201 again. + - Demo: with the demo's boot, a start in #general answers 403 both with 5 participants and after trimming the channel to 2. No session exists for #general and CreateRoom was called once (for the DM only). DM starts and the stranger 404 are unchanged. + + +**Part A, sm-chatvideo-plugin** (/media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin; check `git status --short` first, as in Task 1). + +RED: write contracts/call_policy_test.go (package contracts, -short) and call_policy_test.go (package chatvideo_test, Docker; ensurePostgres first; bootWeb) with the plugin cases in ``. Create the DM conversation in the test with gdb.Create of a chatmodels.Conversation with Type chatmodels.TypeDM plus two chatmodels.Participant rows, the same way the harness's conversation helper does it. Install policies through env.svc.SetCallPolicy. Run them and record the failure: compile errors are expected until the identifiers exist, so add the contracts types first and then see the 403 assertions fail. + +GREEN: +- Create contracts/call_policy.go: package contracts, with a package doc comment saying it holds golem15.chatvideo's host seams. Declare: + - The interface `CallPolicy` with the single method `CanStartCall(ctx context.Context, userID, conversationID uint, conversationType string) (bool, error)`. Its doc comment: consulted when a conversation participant starts a call, after the participant check; false refuses the start; never consulted for joining a running call. + - `CallPolicyFunc`, a func type with that signature implementing CallPolicy. + - The exported var `DMOnly CallPolicy`, a CallPolicyFunc answering conversationType == chatmodels.TypeDM (import the chat models package). +- classes/video_service.go: + - Add `ErrCallNotAllowed` (text "video calls are not allowed in this conversation"), `CallPolicy contracts.CallPolicy` in VideoOptions, a `policy` field copied by NewVideoService, and `SetCallPolicy(p contracts.CallPolicy)`, nil-receiver safe, where nil allows every start (today's behaviour). + - Change requireParticipant to return the conversation it loaded, `(*chatmodels.Conversation, error)`. Other callers discard it. + - In Start, right after requireParticipant and before activeSession, when a policy is set: call CanStartCall(ctx, actorID, conversationID, conv.Type). An error is returned as is; false returns ErrCallNotAllowed. Join, Leave, End, Status and CallLink do not consult the policy. +- api/errors.go: add the constant errCallNotAllowed = "Video calls are not available in this conversation" and a writeVideoError case mapping classes.ErrCallNotAllowed to 403. A 422 would show the frontend's "too many people" text. +- lang en and pl: add errors.not_allowed with "Video calls are not available in this conversation." and "Połączenia wideo nie są dostępne w tej rozmowie." +- README.md: + - Features: a "Call policy." bullet (optional, default allows all, DMOnly, 403 answer, join unaffected). + - Usage: a short paragraph saying a host installs a policy from a lagoon.OnDatabase hook by looking up *classes.VideoService and calling SetCallPolicy(contracts.DMOnly). Keep it in prose plus the existing code-block style. + - API reference rows: contracts.CallPolicy, contracts.CallPolicyFunc, contracts.DMOnly, classes.VideoService.SetCallPolicy, classes.ErrCallNotAllowed. + - Overview or Features: the start route's possible 403. + - Testing: add TestVideoCallPolicy to the run line. +- Run `go test -short -run TestPluginManifest .`. Only if it reports a mismatch, regenerate plugin.yaml with `go run git.golem15.com/golem15/summercms/cmd/summer plugin:manifest` (or the installed `summer plugin:manifest`) and include it. Otherwise leave plugin.yaml untouched. + +Commit in sm-chatvideo-plugin, staging the changed plugin paths explicitly: `feat: optional CallPolicy host hook and a DM-only policy`. Add a body naming IN-01. No co-author trailer. + +**Part B, sm-demo-app** (/media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app; check `git status --short` first; other batch items also commit here, so stage only these paths). + +- Move the submodule to the new plugin HEAD with `git -C plugins/golem15/chatvideo fetch /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin main`, then `git -C plugins/golem15/chatvideo checkout --detach FETCH_HEAD`. Confirm `git -C plugins/golem15/chatvideo rev-parse HEAD` equals `git -C ../sm-chatvideo-plugin rev-parse HEAD`. +- Create plugins/demo/video.go with `(p *Plugin) registerVideo(app *backpack.App) error`, modelled on registerForum: + - It returns nil without golem15.chatvideo. + - Inside lagoon.OnDatabase it looks up *chatvideoclasses.VideoService (import "git.golem15.com/golem15/sm-chatvideo-plugin/classes" and ".../contracts"). + - A missing service returns the error "golem15.demo: golem15.chatvideo must be listed before golem15.demo in summer.yaml". + - Otherwise it calls SetCallPolicy(chatvideocontracts.DMOnly). + - Its doc comment cites D-23 (calls in DMs only), now enforced on the server. +- plugins/demo/plugin.go: append p.registerVideo to the Boot step list and mention the video call policy in Boot's doc comment. +- video_test.go: rewrite TestDemoVideoStart's doc comment ("the server refuses a call outside a DM through chatvideo's DM-only CallPolicy") and its channel section: + - The #general start answers 403 with the error text above, while participants is still 5. + - After the existing trim to 2 participants it still answers 403. Replace the old 201 assertion and its "no type rule" message. + - #general has 0 sessions, and fake.called("CreateRoom") has length 1. + - The DM part and the stranger 404 stay as they are. +- config/golem15.chatvideo.yaml: replace the last comment sentence with one saying golem15.demo installs chatvideo's DM-only CallPolicy, so the server refuses calls outside DMs (403) whatever max_participants allows. Leave the values unchanged. +- README.md "**Calls.**" paragraph: the same correction. + +Run the demo gates in ``. Then commit, staging exactly plugins/golem15/chatvideo, plugins/demo/video.go, plugins/demo/plugin.go, video_test.go, config/golem15.chatvideo.yaml and README.md: `feat: video calls in DMs only, enforced on the server by chatvideo's CallPolicy`. The body names IN-01 and states that the chatvideo pointer is ahead of the go.mod pin (v0.1.1) until the plugin is tagged. No co-author trailer, no go.mod change. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-chatvideo-plugin && go vet ./... && go test -short ./... && go test -count=1 -run '^(TestVideoCallPolicy|TestVideoLiveMembership|TestHTTPTracer)$' . && cd ../sm-demo-app && go vet ./... && go test -short ./... && go test -count=1 -run '^(TestDemoVideoStart|TestBootAllPlugins|TestDemoDMs|TestDemoChatContextMembership|TestRealtimeMemberToken|TestGoModRequiresTags)$' . + + + - TestVideoCallPolicy and the contracts unit test pass. They failed on the 403 assertions before the Start change. + - The full `go test -count=1 ./...` in sm-chatvideo-plugin is green (Docker; `sudo -u golem` if the socket is denied). + - The demo's targeted Docker tests above pass, with TestDemoVideoStart asserting 403 for both channel starts. `go test ./...` in sm-demo-app is green (Docker); if a test unrelated to video fails, check against the base commit and report it rather than editing it. + - `git -C ../sm-demo-app show --stat HEAD` lists exactly the six demo paths. `git -C ../sm-demo-app diff HEAD~1 -- go.mod go.sum` is empty. + - Neither of the two new commits has a co-author trailer. + + Hosts can install contracts.CallPolicy, and DMOnly ships with the plugin. The demo enforces calls in DMs only on the server, and its test proves it no longer depends on max_participants or seed data. Two commits: plugin, then demo (pointer bump and wiring). + + + + Task 3: useIncomingCalls closes stale session handles + ../vue-chatvideo-layer/app/composables/useIncomingCalls.ts, ../vue-chatvideo-layer/vitest.config.ts, ../vue-chatvideo-layer/tests/useIncomingCalls.test.ts, ../vue-chatvideo-layer/README.md + + - ../vue-chatvideo-layer/app/composables/useIncomingCalls.ts + - ../vue-chatvideo-layer/tests/useCall.test.ts lines 1-140 (mock and stub style) + - ../vue-chatvideo-layer/app/lib/videoClient.ts lines 195-220 (parseIncomingCall) + - ../vue-chat-layer/vitest.config.ts (the sibling layer's config shape) + - ../vue-chatvideo-layer/README.md "Composables" and "Testing" sections + + + - Two video:call:incoming events for session 5 while both subscribes are pending. When the first resolves before the second, the first handle is closed and the second is kept (not closed). When the second resolves first, the first, resolving later, is closed and the second is kept. + - Rings for session 5 and then session 6 while both subscribes are pending: the session 5 handle is closed whatever the order; the session 6 handle is kept and pending is session 6. + - A subscribe still pending when dismiss() runs: its handle is closed when it resolves, and pending stays null. + - The kept handle is closed by dismiss(), by accept() (which returns the conversation id) and by decline() (which calls client.leave with the conversation id). + - A video:call:ended publication on the kept handle clears pending and closes it. + - With fake timers, after RING_MS the prompt clears and the kept handle is closed. + + +Work in /media/nvme/dev/golem15/summercms.io/summercms/vue-chatvideo-layer; check `git status --short` first. + +RED: +- Add vitest.config.ts using defineConfig from 'vitest/config': `define` maps 'import.meta.client' to 'true'; test.environment is 'node'; test.include is ['tests/**/*.test.ts']. Add a short header comment saying the define lets the client-only registration in useIncomingCalls run under vitest. Confirm the existing 127 tests still pass with it. +- Write tests/useIncomingCalls.test.ts with the cases in ``: + - vi.mock 'vue-chat-layer/app/lib/realtime'. onUserEvent records the handler and returns a no-op unsubscribe. subscribe returns a promise the test resolves by hand and records the channel, the hooks and a handle with vi.fn close and resubscribe. + - In beforeEach, stub the globals: useState backed by a Map of vue refs keyed by the state key, and useNuxtApp returning one fresh object per test that carries _chatvideoClient with a vi.fn leave. A fresh object per test means each test registers its own handler. + - Import the composable dynamically after the mocks, as useCall.test.ts does. Deliver rings by calling the recorded handler with {event: 'video:call:incoming', payload: {session_id, conversation_id, caller_name}}. Flush microtasks after resolving a subscribe. + - vi.unstubAllGlobals and vi.useRealTimers in afterEach. +- Run `pnpm test` and record that the same-session cases fail on the current code (a stale handle is never closed). + +GREEN, in app/composables/useIncomingCalls.ts: +- Add `seq: number` to IncomingRuntime, initialised to 0 in the nuxtApp default object. +- clear() increments rt.seq, alongside its current work, so a subscribe in flight at that moment is invalidated. +- watchSession takes `const seq = ++rt.seq` after closing the current channel. After the await it keeps the handle only when seq still equals rt.seq and pending.value?.sessionId === call.sessionId; otherwise it calls handle.close() immediately. +- The onUserEvent handler keeps its order (clear, set pending, start the timer, watchSession). The ring timer and the ended-publication check stay as they are. +- Update the file's header comment: at most one session handle is open, and a handle that resolves after a newer ring or a dismiss is closed. + +README.md: +- The useIncomingCalls() bullet: one clause saying a newer ring or a dismiss closes a session subscription that is still opening. +- Testing: add a `tests/useIncomingCalls.test.ts` bullet listing what it covers, and mention vitest.config.ts (the import.meta.client define) and the updated `pnpm test` comment. + +Do not bump package.json. + +Commit in vue-chatvideo-layer, staging the four paths explicitly: `fix: the incoming-call runtime closes stale session handles`. Add a body naming IN-07. No co-author trailer. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/vue-chatvideo-layer && pnpm test + + + - `pnpm test` is green: the 127 previous tests plus the new file. The new same-session tests failed on the unchanged composable. + - `git -C ../vue-chatvideo-layer show --stat HEAD` lists exactly the four paths. package.json is unchanged. + - The commit has no co-author trailer. + + The incoming-call runtime keeps only the latest ring's session handle and closes every stale one. Covered by a dedicated vitest file. One commit in vue-chatvideo-layer. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| browser → chatvideo JSON API | A JWT-authenticated member asks to start, join, leave or end calls and reads status and call links. The actor id comes from the token | +| browser → lighthouse subscribe (video:session:{id}) | Subscription authorization runs the plugin's "video" authorizer | +| LiveKit join token holder → LiveKit SFU | A minted HS256 token (token_ttl 900 s) admits its holder to the room until it expires. The server can only remove a connected identity | +| host application → chatvideo | Host contexts (chat's contracts.Context, LiveMembership) and the new CallPolicy decide who takes part and where calls may start | +| user plugin admin actions → chatvideo | Ban and deactivation events are fired inside the admin transaction | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-q261105oc7-01 | Elevation of privilege | requireParticipant and the video authorizer accepted a stale participant row after the user left the host context | medium | mitigate | Task 1: both go through chat's Service.IsParticipant (LiveMembership). TestVideoLiveMembership covers join, status and authorizer denial and re-admission | +| T-q261105oc7-02 | Elevation of privilege | A banned or deactivated user keeps a live LiveKit connection | medium | mitigate | Task 1: RemoveFromCalls on AccessRevokedEvent (ban) and DeactivatingEvent removes the identity after the commit and marks rows left. TestVideoBanRemovesFromCall, including the rollback and password-reason cases | +| T-q261105oc7-03 | Elevation of privilege | A removed user reconnects with the still-valid LiveKit token | medium | accept | LiveKit has no token revocation. The window is bounded by golem15.chatvideo.token_ttl (900 s), which hosts can lower. The JWT is revoked, so no new token can be minted. Documented in the README Bans bullet and the SUMMARY follow-ups | +| T-q261105oc7-04 | Information disclosure | An open video:session subscription survives a ban until its token refresh | low | accept | lighthouse has no server-side unsubscribe (a framework follow-up). The refresh needs the revoked JWT and re-runs the live-aware authorizer. The channel carries call events and user ids only | +| T-q261105oc7-05 | Elevation of privilege | A member rings everyone in a channel once max_participants allows it (IN-01) | medium | mitigate | Task 2: contracts.CallPolicy with DMOnly; the demo installs it. TestVideoCallPolicy and TestDemoVideoStart assert 403 for both channel cases and no session or room | +| T-q261105oc7-06 | Elevation of privilege | A fail-open path when the chat service or the policy errors | medium | mitigate | chatMembership returns false with an error when chat's service is missing: the authorizer denies and the API answers 500, never a participant-row fallback. A policy error aborts Start with nothing created (asserted) | +| T-q261105oc7-07 | Spoofing | Ringing (video:call:incoming on user:{id}) of users who are no longer members | low | mitigate | Task 1: Start filters invitees through the membership check before invite and notify.Incoming; asserted by the missing participant row | +| T-q261105oc7-08 | Denial of service | Per-invitee membership queries at start | low | accept | The ErrTooMany check runs first on the raw list, so at most max_participants (default 8) checks run, and only when a membership check is installed | +| T-q261105oc7-09 | Denial of service | Leaked realtime subscriptions in the browser (IN-07) | low | mitigate | Task 3: a sequence number per watchSession; stale handles are closed; tests/useIncomingCalls.test.ts | +| T-q261105oc7-SC | Tampering | npm/pip/cargo installs | low | accept | No install and no new dependency in any repo. vitest/config comes from the existing vitest 3.2.7 devDependency; the Go imports are already required modules (chat v0.1.3, user v0.1.13) | + + + +- sm-chatvideo-plugin: gofmt clean; `go vet ./...`, `go test -short ./...` and the full `go test -count=1 ./...` (Docker) green after each of its two commits. +- sm-demo-app: `go vet ./...`, `go test -short ./...` and `go test -count=1 ./...` (Docker) green, TestDemoVideoStart asserting 403 for channel starts. The submodule pointer equals the plugin HEAD. go.mod and go.sum are unchanged. +- vue-chatvideo-layer: `pnpm test` green with the new file. +- Four commits total (two plugin, one demo, one layer), each staging only its listed paths, none with a co-author trailer. Nothing pushed or tagged; no summercms.go code changed. + + + +- A ban ends call access at once: no join, status, call link or subscribe for a member who left a live-membership context; banned or deactivated users are removed from live LiveKit rooms after the commit. +- Hosts can restrict where calls start through an optional CallPolicy (default unchanged). The demo enforces calls in DMs only on the server, independent of seed data. +- The incoming-call runtime never leaks a session handle. +- The SUMMARY lists the release steps (tag and push sm-chatvideo-plugin before the demo, require the tag in the demo's go.mod so check-pins passes, tag vue-chatvideo-layer and bump vue-demo-app) and the Not-in-scope follow-ups. + + + +Create `.planning/quick/261105-oc7-chatvideo-hardening/261105-oc7-SUMMARY.md` when done. Include the four commit hashes, the release state of the demo pointer against its go.mod pin, and the follow-ups from the objective's "Not in scope" list so the orchestrator can file them. + diff --git a/.planning/quick/261105-oc7-chatvideo-hardening/261105-oc7-SUMMARY.md b/.planning/quick/261105-oc7-chatvideo-hardening/261105-oc7-SUMMARY.md new file mode 100644 index 00000000..b7d63143 --- /dev/null +++ b/.planning/quick/261105-oc7-chatvideo-hardening/261105-oc7-SUMMARY.md @@ -0,0 +1,149 @@ +--- +phase: quick-261105-oc7 +plan: 01 +subsystem: chatvideo (sm-chatvideo-plugin, sm-demo-app, vue-chatvideo-layer) +tags: [chatvideo, security, live-membership, bans, call-policy, realtime] +status: complete +requirements: + - todo:chatvideo-live-membership + - todo:chatvideo-dm-only-host-hook + - todo:chatvideo-incoming-handle-leak +requires: + - golem15.chat Service.IsParticipant + contracts.LiveMembership (v0.1.3) + - golem15.user AccessRevokedEvent / DeactivatingEvent (v0.1.13) +provides: + - classes.MembershipCheck, VideoOptions.Membership, VideoService.SetMembership + - classes.AuthorizeVideoMembership + - classes.VideoService.RemoveFromCalls (ban / deactivation) + - contracts.CallPolicy, contracts.CallPolicyFunc, contracts.DMOnly, VideoService.SetCallPolicy, classes.ErrCallNotAllowed (403) + - demo registerVideo installing DMOnly + - useIncomingCalls sequence-numbered session handles +affects: [sm-demo-app go.mod pin of sm-chatvideo-plugin, vue-demo-app layer pin] +tech-stack: + added: [] + patterns: [lagoon.AfterCommit for LiveKit side effects, optional host hook with nil default, fail-closed membership lookup] +key-files: + created: + - ../sm-chatvideo-plugin/classes/revoked_access.go + - ../sm-chatvideo-plugin/live_membership_test.go + - ../sm-chatvideo-plugin/contracts/call_policy.go + - ../sm-chatvideo-plugin/contracts/call_policy_test.go + - ../sm-chatvideo-plugin/call_policy_test.go + - ../sm-demo-app/plugins/demo/video.go + - ../vue-chatvideo-layer/vitest.config.ts + - ../vue-chatvideo-layer/tests/useIncomingCalls.test.ts + modified: + - ../sm-chatvideo-plugin/classes/video_service.go + - ../sm-chatvideo-plugin/classes/authorizer.go + - ../sm-chatvideo-plugin/classes/livekit.go + - ../sm-chatvideo-plugin/boot.go + - ../sm-chatvideo-plugin/api/errors.go + - ../sm-chatvideo-plugin/lang/en/lang.yaml + - ../sm-chatvideo-plugin/lang/pl/lang.yaml + - ../sm-chatvideo-plugin/README.md + - ../sm-demo-app/plugins/golem15/chatvideo (submodule pointer) + - ../sm-demo-app/plugins/demo/plugin.go + - ../sm-demo-app/video_test.go + - ../sm-demo-app/config/golem15.chatvideo.yaml + - ../sm-demo-app/README.md + - ../vue-chatvideo-layer/app/composables/useIncomingCalls.ts + - ../vue-chatvideo-layer/README.md +decisions: + - "chatvideo's membership check is golem15.chat's Service.IsParticipant, looked up per call; a missing chat service fails closed (false + error), never a participant-row fallback" + - "The CallPolicy runs right after the participant check and before the active-session lookup and max_participants, so a refused start creates nothing and a running group call cannot be re-started under DMOnly (joining it still works)" + - "A policy refusal is 403 so vue-chatvideo-layer's existing failureReason(403) = 'forbidden' reads correctly; 422 would show 'too many people'" +metrics: + duration: ~50 min + completed: 2026-11-05 +actuals: + tasks: 3 + commits: 4 +--- + +# Quick 261105-oc7: chatvideo hardening Summary + +chatvideo now checks every participant, the `video:session` authorizer and start invitees through chat's live-membership-aware `Service.IsParticipant`. Bans and deactivations drop the user from live LiveKit rooms after the commit. An optional `contracts.CallPolicy` with a ready-made `DMOnly` lets the demo enforce DM-only calls on the server. `useIncomingCalls` closes every stale session handle. + +## Commits + +| # | Repo | Hash | Message | +|---|------|------|---------| +| 1 | sm-chatvideo-plugin (`main`) | `f996156` | fix: honour chat live membership and remove banned users from live calls | +| 2 | sm-chatvideo-plugin (`main`) | `11a9607` | feat: optional CallPolicy host hook and a DM-only policy | +| 3 | sm-demo-app (`master`) | `fbfdcf6` | feat: video calls in DMs only, enforced on the server by chatvideo's CallPolicy | +| 4 | vue-chatvideo-layer (`main`) | `6d5784d` | fix: the incoming-call runtime closes stale session handles | + +None has a co-author trailer. Nothing was pushed or tagged. No go.mod or go.sum changed in any repo, and no summercms.go code changed. Each commit staged only its listed paths. plugin.yaml was left alone because `TestPluginManifest` passed without regenerating it. + +## What was built + +**Task 1 (IN-02), live membership and bans.** +- `classes.MembershipCheck`, `VideoOptions.Membership` and `VideoService.SetMembership` were added. +- `requireParticipant` uses the membership check when one is set. With none set, the participant-row count is unchanged. +- `Start` keeps the raw `ErrTooMany` check. It then drops the participants who fail the check from the invite list and from `notify.Incoming`. The actor is always kept. +- `AuthorizeVideoMembership(getDB, member)` was added, and `AuthorizeVideoLookup` now delegates to it with a nil member. +- Boot installs `chatMembership(app)` in the service and the authorizer. It looks up `*chatclasses.Service` on each call and fails closed. +- Boot also listens for `AccessRevokedEvent`, but only acts on the reason `ban`, and for `DeactivatingEvent`. Both call `RemoveFromCalls`, which: + - marks the user's joined rows of live sessions left, with `left_at`, inside the transaction + - after the commit, removes the user's LiveKit identity from rooms where they were joined or had left, and publishes participant-left for the rows that were joined + - never ends a session or deletes a row +- `FakeRooms.AddParticipant` and `FakeRooms.HasParticipant` were added for tests. + +**Task 2 (IN-01), CallPolicy.** +- New `contracts` package with `CallPolicy`, `CallPolicyFunc` and `DMOnly`. +- `VideoOptions.CallPolicy`, `VideoService.SetCallPolicy` and `ErrCallNotAllowed` were added. A refusal maps to 403 `"Video calls are not available in this conversation"`, with lang `errors.not_allowed` in en and pl. +- `requireParticipant` now also returns the conversation it loaded. It is unexported. +- The demo's `registerVideo` installs `DMOnly` from a `lagoon.OnDatabase` hook (D-23). +- `TestDemoVideoStart` now expects 403 for #general with 5 participants and again after trimming to 2. It also checks there are 0 sessions for #general and exactly 1 CreateRoom call (the DM's). +- The config comment and the README "Calls." paragraph now say the server enforces the rule. + +**Task 3 (IN-07), stale handles.** +- `IncomingRuntime.seq` was added. `clear()` increments it, and `watchSession` takes `const seq = ++rt.seq`. When the subscribe resolves, the handle is kept only if `seq` still matches and the session is still the pending one. Otherwise it is closed. +- New `vitest.config.ts` (defines `import.meta.client`) and `tests/useIncomingCalls.test.ts` (10 tests). + +## TDD evidence + +- **Task 1.** On the unchanged service, `TestVideoLiveMembership` failed on "C (not a member) has 1 participant rows". `TestVideoBanRemovesFromCall` failed on "banned B still in the room after the commit" and "deactivated A still in the room after the commit". The first ban RED run was not valid, because the helper passed the event as `any` and the generic `festival.Fire[T]` matched no listener. After fixing the helper, I confirmed the ban and deactivation failures again with the new listeners commented out. +- **Task 2.** With the contracts types and SetCallPolicy in place but not used by `Start`, `TestVideoCallPolicy` failed four subtests. Group start under DMOnly gave 201. The recording policy was never called. A policy error gave 201. A start of a running call gave 200. `TestDemoVideoStart` with `registerVideo` taken out failed with 422, then 201, 1 session and 2 CreateRoom calls. +- **Task 3.** On the unchanged composable, both same-session tests failed with "expected spy to be called 1 times, but got 0 times". The other 8 already passed and now guard against regressions. + +## Verification + +- **sm-chatvideo-plugin:** `go vet ./...`, `GOWORK=off go vet ./...` (against the pinned chat v0.1.3 and user v0.1.13), `go test -short ./...` and the full `go test -count=1 ./...` (Docker) all pass after each commit. `TestFixtureAbsentFromProduction` and `TestPluginManifest` pass. +- **sm-demo-app:** `go vet ./...`, `go test -short ./...`, the targeted Docker tests from the plan and the full `go test -count=1 ./...` all pass. The submodule pointer equals the plugin HEAD (`11a9607`). +- **vue-chatvideo-layer:** `pnpm test` passes 137 tests (the 127 existing plus 10 new). package.json is unchanged. + +## Deviations from Plan + +1. **[Rule 1 - test bug] Event firing in the ban test.** `festival.Bus.Fire` is generic (`Fire[T]`), so firing through an `any`-typed helper reached no listener. `fireInTx` now takes a fire closure that calls `env.app.Events.Fire` with the concrete event type. The fix is in the test only. +2. **Skipping under -short.** The plan says to call `ensurePostgres(t)` "so it skips under -short". In this harness `ensurePostgres` starts a container even under `-short`, so the new Docker tests call `t.Skip` under `testing.Short()` first and then `ensurePostgres`. +3. **gofmt gate.** `gofmt -l .` in sm-chatvideo-plugin already listed `harness_test.go` and `http_tracer_test.go` before this plan (import order and map alignment). Those files are outside this task and were not changed, so the plan's `test -z "$(gofmt -l .)"` gate fails on the baseline. Every file this plan touched is gofmt-clean. + +## Release state (for the orchestrator / user) + +- sm-demo-app `plugins/golem15/chatvideo` is at `11a9607`, ahead of its go.mod pin (`sm-chatvideo-plugin v0.1.1` = `0f08c35`). `scripts/check-pins.sh` reports exactly this. It also reports the earlier chatchannels pointer from a sibling item. +- Dev builds and tests compile through go.work. The GOWORK=off release build (`scripts/build.sh`) does not compile until these steps are done in order: + 1. Tag sm-chatvideo-plugin (the next v0.1.x, at `11a9607` or later on `main`). + 2. Push that tag before the demo. + 3. Commit `chore: require sm-chatvideo-plugin vX` in sm-demo-app (go.mod and go.sum). +- vue-chatvideo-layer also needs a tag at `6d5784d` or later, followed by a vue-demo-app layer bump. +- I did none of these. + +## Follow-ups (not in scope) + +- **Open subscriptions after a ban.** A ban does not end an already open `video:session` Centrifugo subscription, because lighthouse has no server-side unsubscribe (that is a framework change). Refreshing the subscription needs the revoked JWT and re-runs the now live-aware authorizer, so access lasts until `realtime.centrifugo.token_ttl` (3600 s by default). +- **Join tokens after a ban.** An already minted LiveKit join token cannot be revoked, so a removed user could reconnect until `golem15.chatvideo.token_ttl` (900 s) runs out. Hosts may lower it. This is documented in the README "Bans." bullet. +- **Leaving a host context without a ban.** The user is not removed from live rooms, because there is no generic event for it. The live checks still refuse every new join, status read, call link and subscribe. +- **Banned last participant.** A call whose last joined participant was banned is not ended at once. The reconciler ends empty rooms within 5 minutes. +- The logout half of IN-07 was already fixed in phase 20 plan 08. +- **Pre-existing gofmt drift.** `sm-chatvideo-plugin/harness_test.go` and `http_tracer_test.go` need gofmt. +- **Orchestrator tasks.** Tags, pushes, the go.mod bump, the vue-demo-app layer bump, moving the three todos to completed and the g15office comment. + +## Threat Flags + +None. The only new surface is the ban/deactivation listeners and the 403 path, both in the plan's threat model (T-q261105oc7-02, -05, -06). + +## Self-Check: PASSED + +- Files: every created file listed above exists on disk. +- Commits: `f996156` and `11a9607` (sm-chatvideo-plugin), `fbfdcf6` (sm-demo-app) and `6d5784d` (vue-chatvideo-layer) are in their repos' logs. diff --git a/.planning/quick/261105-oc8-lagoon-attach-statichandler-and-statichandlerpublic-send-the/261105-oc8-PLAN.md b/.planning/quick/261105-oc8-lagoon-attach-statichandler-and-statichandlerpublic-send-the/261105-oc8-PLAN.md new file mode 100644 index 00000000..d1727e5b --- /dev/null +++ b/.planning/quick/261105-oc8-lagoon-attach-statichandler-and-statichandlerpublic-send-the/261105-oc8-PLAN.md @@ -0,0 +1,294 @@ +--- +phase: quick-261105-oc8 +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - modules/lagoon/attach/static.go + - modules/lagoon/attach/static_test.go + - modules/lagoon/attach/lifecycle_test.go + - modules/lagoon/README.md + - docs/services/storage.md + - docs/database/attachments.md + - modules/surf/uploads.go +autonomous: true +requirements: + - todo:2026-10-28-attach-static-handler-disposition + +must_haves: + truths: + - "attach.StaticHandler and attach.StaticHandlerPublic send X-Content-Type-Options: nosniff on every response, GET and HEAD, 200 and 404 alike" + - "A served blob whose stored Content-Disposition is non-empty and parses with mime.ParseMediaType gets that value verbatim (trimmed) as the response's Content-Disposition, on GET and on HEAD. A blob attach.Store wrote for text/html (stored disposition attachment) is therefore served with Content-Disposition: attachment by both handlers" + - "A stored disposition that mime.ParseMediaType rejects (for example a value carrying CR/LF or a broken parameter) is sent as the bare value attachment: the handler fails closed and never forwards malformed metadata" + - "Additive: a blob with no stored disposition (raster images, thumbnails, blobs written before quick-261028-rqe or outside attach.Store) is served exactly as before, with its stored Content-Type and no Content-Disposition. The handlers do not derive a disposition from the type and add no Content-Security-Policy; the surf uploads route keeps doing both" + - "Ordering and 404s are unchanged: the is_public gate still runs before any bucket access, a missing blob, an invalid path and a denied row still answer 404, and a 404 never carries a Content-Disposition" + - "Non-breaking: no exported identifier is added, removed or changes its signature in modules/lagoon or modules/surf. The existing static, lifecycle and surf uploads tests (TestStaticHandler, TestStaticHandlerServesThumbURL, TestStaticHandlerPublicGate, TestUploadsMount, TestUploadsResponseHeaders) pass without edits to their existing assertions" + - "The lagoon README, docs/services/storage.md and docs/database/attachments.md describe the new headers and what the handlers still do not do; the surf uploads.go doc comment no longer says StaticHandlerPublic sends only the stored type. go vet ./..., go test -short ./..., go test ./cmd/summer -run TestDocsTree and go run ./cmd/summer docs:build --check are green at the commit, and the Docker-backed TestStaticHandlerPublicLooksUpIsPublic passes" + artifacts: + - path: modules/lagoon/attach/static.go + provides: "servePublicBlobs reads the blob's Attributes after the gate, sets nosniff, the stored Content-Type and the validated stored Content-Disposition; unexported storedDisposition helper; updated StaticHandler and StaticHandlerPublic doc comments" + contains: "X-Content-Type-Options" + - path: modules/lagoon/attach/static_test.go + provides: "TestStoredDisposition (table) and TestStaticHandlerSendsStoredDisposition (memblob and fileblob sidecar, writeBlob round trip, GET and HEAD, ungated and gated), Docker-free" + contains: "TestStaticHandlerSendsStoredDisposition" + - path: modules/lagoon/attach/lifecycle_test.go + provides: "TestStaticHandlerPublicLooksUpIsPublic also asserts the headers through the real is_public gate on Postgres" + contains: "Content-Disposition" + - path: modules/lagoon/README.md + provides: "Features bullet and the attach.StaticHandler table row name the stored disposition and nosniff" + - path: docs/services/storage.md + provides: "Serving files (mounting the handler yourself) and the deploy note describe what the handlers send and what they do not" + - path: docs/database/attachments.md + provides: "The StaticHandler paragraph and the Store disposition paragraph name the handlers" + - path: modules/surf/uploads.go + provides: "mountUploads doc comment corrected; no code change" + key_links: + - from: modules/lagoon/attach/static.go servePublicBlobs + to: gocloud.dev/blob (*Bucket).Attributes + via: "bucket.Attributes(r.Context(), key) after the allow gate and before NewReader; its ContentDisposition goes through storedDisposition" + pattern: "bucket\\.Attributes\\(" + - from: modules/lagoon/attach/static.go storedDisposition + to: modules/lagoon/attach/inline.go attachmentDisposition + via: "the fail-closed value for an unparsable stored disposition is the existing attachmentDisposition constant, the same bare value attach.Store stores" + pattern: "attachmentDisposition" + - from: modules/lagoon/attach/store.go writeBlob + to: modules/lagoon/attach/static.go servePublicBlobs + via: "TestStaticHandlerSendsStoredDisposition writes through writeBlob (Store's real writer) and reads the headers back through StaticHandler" + pattern: "writeBlob\\(" +--- + + +Close the todo `2026-10-28-attach-static-handler-disposition` in the framework (summercms.go only). Quick 261028-rqe (ba8432c, 4957e5c) made `attach.Store` store `Content-Disposition: attachment` with every blob whose type `attach.IsInlineContentType` rejects, and the surf uploads route sends a disposition. `attach.StaticHandler` and `attach.StaticHandlerPublic` still send only the stored Content-Type, so an application that mounts either handler directly serves a stored HTML body inline. The fix, from the todo's Solution: both handlers send the stored disposition and `X-Content-Type-Options: nosniff`, additively. + +Live reads at planning time: +- `modules/lagoon/attach/static.go` `servePublicBlobs` (lines 38-80): method check, nil bucket, prefix strip, `parsePublicBlobPath`, the optional `allow` gate, then `bucket.NewReader(r.Context(), key, nil)`. It sets Content-Type from `reader.ContentType()` (falling back to `defaultStaticContentType`), returns for HEAD, else `io.Copy`. Both exported handlers are thin wrappers over it. +- gocloud.dev v0.46.0: `*blob.Reader` exposes only `ContentType`, `ModTime`, `Size`, `As`; it does not expose the disposition. `(*blob.Bucket).Attributes(ctx, key)` returns `*blob.Attributes` with `ContentDisposition` and `ContentType`. memblob keeps the disposition, fileblob keeps it in its `.attrs` sidecar (the default behind `file://`), s3blob reads it with a HEAD. Reading it costs one `Attributes` call per request; the rqe plan named that cost and deferred it here. +- `modules/lagoon/attach/inline.go` already has the unexported constant `attachmentDisposition` (the bare value attachment) and the exported `IsInlineContentType`. inline.go is not edited. +- `net/http.Error` (Go 1.27) sets `X-Content-Type-Options: nosniff` and does not touch Content-Disposition, so every `http.NotFound` and 405 from the handler already carries nosniff; a disposition set before an error path would leak into the 404. +- `mime.ParseMediaType` accepts `attachment`, `inline`, `attachment; filename="a.png"` and RFC 5987 `filename*=` values; it rejects `attachment;;`, `attachment; filename` and a value containing CR/LF (verified in the scratchpad). +- `modules/surf/uploads.go` wraps `attach.StaticHandlerPublic`. `serveUploads` sets nosniff and the CSP before calling it, and `uploadsDispositionWriter.decide` sets `Content-Disposition: attachment` at WriteHeader/Write time for a non-inline 2xx, i.e. after the handler has set its headers, so the route's output for non-inline types is unchanged and its tests write blobs without a stored disposition. Only its doc comment (lines 36-40, "attach.StaticHandlerPublic sends only the stored type") becomes stale. +- `TestStaticHandlerPublicLooksUpIsPublic` (lifecycle_test.go:190, package attach_test, skips under -short) is the only test of `StaticHandlerPublic` with a real `is_public` lookup. +- No other caller of the handlers exists in the repo (examples/ included). None of the seven files has uncommitted edits from another session at planning time. + +Decomposition: two tasks, one commit, run in order. +- Task 1 is the tracer: the header change in servePublicBlobs plus Docker-free tests that write through `writeBlob` (Store's real writer) and read the headers back through both handlers on memblob and fileblob. +- Task 2 adds the Postgres-backed gated assertion, updates the lagoon README, the two docs pages and the surf doc comment, and commits Tasks 1 and 2 together (CLAUDE.md: a change to a module's exported behaviour updates its README and the affected docs pages in the same commit). + +Not in scope (the orchestrator records these as follow-ups; do not edit sibling repos or consumer apps): +- Blob writers that bypass `attach.Store` adopting `attach.IsInlineContentType`: fonoteka.go `storeUpload` and `StorePublicFile` (see todo `2026-10-28-fonoteka-uploads-use-disk-extension.md`), the sm-user-plugin avatar writer, the sm-feedback-plugin screenshot writer. +- Adding the disposition to objects already in a bucket: an operator task, already documented in `docs/services/storage.md#deploy-note`. +- A type-derived disposition or a CSP in the handlers. The todo asks for the stored disposition only; deciding from the type would change how existing blobs without a disposition are served (a PDF opened inline would start downloading), which is not additive. The surf uploads route keeps that role. +- Moving the todo to completed and the g15office comment (`knuu7fil`): the orchestrator's closing docs commit handles both. + +Purpose: an application that mounts `attach.StaticHandler` or `attach.StaticHandlerPublic` directly gets the same protection a public bucket gives since rqe: an HTML, SVG, PDF or text upload stored by `attach.Store` downloads instead of rendering, and nothing is content-sniffed. +Output: one commit in summercms.go and 261105-oc8-SUMMARY.md. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@.planning/STATE.md +@./CLAUDE.md +@.planning/todos/pending/2026-10-28-attach-static-handler-disposition.md + + +Existing code, verified at planning time. + +modules/lagoon/attach/static.go +- const defaultStaticContentType = "application/octet-stream" +- func StaticHandler(bucket *blob.Bucket, prefix string) http.Handler — signature stays. +- func StaticHandlerPublic(bucket *blob.Bucket, prefix string, db *gorm.DB) http.Handler — signature stays. +- func servePublicBlobs(bucket *blob.Bucket, prefix string, allow func(context.Context, string) (bool, error)) http.Handler — signature stays (TestStaticHandlerPublicGate calls it directly). +- Imports today: context, io, net/http, path, strings, gocloud.dev/blob, gorm.io/gorm. The change adds mime. + +modules/lagoon/attach/inline.go (not edited) +- const attachmentDisposition = "attachment" (unexported, bare value, no filename parameter). +- func IsInlineContentType(contentType string) bool. +- func uploadWriterOptions(contentType string) *blob.WriterOptions — ContentType plus ContentDisposition attachment for non-inline types. + +modules/lagoon/attach/store.go (not edited) +- func writeBlob(ctx context.Context, bucket *blob.Bucket, key string, r io.Reader, contentType string, limit int64) (int64, error) — writes through uploadWriterOptions. inline_test.go's TestWriteBlobDisposition already calls it with limit 0. +- func BlobKey(diskName string) string and func PartitionDirectory(diskName string) string: the partition is the first nine characters in three groups ("abc123xyz.jpg" -> "abc/123/xyz/"). + +modules/lagoon/attach/static_test.go (package attach): TestStaticHandler, TestStaticHandlerServesThumbURL, TestStaticHandlerPublicGate; imports context, io, net/http, net/http/httptest, testing, gocloud.dev/blob, gocloud.dev/blob/memblob. inline_test.go already imports gocloud.dev/blob/fileblob, so a test import adds no dependency. + +modules/lagoon/attach/lifecycle_test.go (package attach_test): TestStaticHandlerPublicLooksUpIsPublic at line 190 creates a public jpeg row abc123xyz.jpg and a private one def456uvw.jpg via gdb.Create, writes their blobs and thumbs with bucket.WriteAll, builds h := attach.StaticHandlerPublic(bucket, "/storage/uploads", gdb) and checks status codes through a local get(path) int helper. + +modules/surf/uploads.go: mountUploads doc comment lines 29-45; lines 36-40 read "attach.Store also stores the attachment disposition with the blob, but the route still decides from the type on every response: attach.StaticHandlerPublic sends only the stored type, and blobs written earlier or outside attach.Store carry no disposition." Code is not edited. + +gocloud.dev v0.46.0 (go.mod) +- (*blob.Bucket).Attributes(ctx context.Context, key string) (*blob.Attributes, error); Attributes.ContentDisposition string. +- fileblob.OpenBucket(dir string, opts *fileblob.Options); nil opts writes the .attrs sidecar. + + + + + + + Task 1: StaticHandler and StaticHandlerPublic send nosniff and the stored Content-Disposition (tracer: Store's writer to the handler's response) + modules/lagoon/attach/static.go, modules/lagoon/attach/static_test.go + + - modules/lagoon/attach/static.go (whole file, 129 lines) + - modules/lagoon/attach/static_test.go (whole file, 174 lines) + - modules/lagoon/attach/inline.go (whole file, 44 lines: attachmentDisposition, uploadWriterOptions) + - modules/lagoon/attach/inline_test.go lines 40-120 (how TestWriteBlobDisposition builds memblob and fileblob buckets and calls writeBlob) + + + - storedDisposition("") and storedDisposition(" ") return "". + - storedDisposition returns its trimmed input for "attachment", "inline", `attachment; filename="a.png"` and " attachment ". + - storedDisposition returns "attachment" for "attachment;;", "attachment; filename" and "attachment\r\nX-Evil: 1". + - On memblob and on fileblob with sidecar metadata: a blob written with writeBlob(ctx, bucket, key, body, "text/html", 0) and fetched with GET through StaticHandler answers 200 with Content-Type text/html, Content-Disposition attachment, X-Content-Type-Options nosniff and the unchanged body. + - The same blob fetched with HEAD answers 200 with the same three headers and an empty body. + - A blob written with writeBlob(..., "image/png", 0) is served with Content-Type image/png, nosniff and no Content-Disposition header at all. + - On memblob, a blob written with bucket.WriteAll and ContentDisposition `attachment; filename="a.png"` (ContentType image/png) is served with that exact disposition; one written with ContentDisposition "attachment;;" (ContentType text/html) is served with Content-Disposition attachment. + - Through servePublicBlobs with an allow func that returns true, the text/html blob gets the same headers as through StaticHandler (the gated path shares the change). + - A missing blob answers 404 with nosniff and no Content-Disposition. + + +This task implements the todo's Solution, first bullet: both handlers send the stored disposition and X-Content-Type-Options nosniff, additively. Write the tests from the behavior block first and watch them fail to compile or fail, then implement. + +1. In modules/lagoon/attach/static.go, add an unexported helper storedDisposition(raw string) string. It trims surrounding whitespace; an empty result returns "". If mime.ParseMediaType rejects the trimmed value, it returns attachmentDisposition (the existing constant in inline.go: fail closed, so malformed or injected metadata never reaches a response header). Otherwise it returns the trimmed value verbatim, parameters included, so the handler behaves like a public bucket that sends the stored header. Doc comment: the value of the stored Content-Disposition a handler sends; empty for none; attachment for a value mime.ParseMediaType rejects. + +2. In servePublicBlobs, keep the method check, nil-bucket check, prefix strip, parsePublicBlobPath and the allow gate exactly where they are; the gate must still run before any bucket access (TestStaticHandlerPublicGate relies on it). Then: + - Right after the method check passes, set X-Content-Type-Options to nosniff on w.Header(), so every response the handler writes carries it (http.Error sets it on the error paths too; setting it up front keeps the success path explicit). + - After the gate and before NewReader, call bucket.Attributes(r.Context(), key). On error answer http.NotFound and return, the same as a NewReader error today. + - Keep the NewReader call, its 404 on error, the deferred Close and the Content-Type logic (reader.ContentType() with the defaultStaticContentType fallback) unchanged. + - Only after both Attributes and NewReader have succeeded, set Content-Disposition to storedDisposition(attrs.ContentDisposition) when that is non-empty, next to the Content-Type header and before the HEAD return, so GET and HEAD both carry it and no 404 ever does (http.Error does not remove a Content-Disposition set earlier). + - Leave stripStaticPrefix and parsePublicBlobPath untouched. + +3. Update the doc comments. StaticHandler: after the existing description, say that every response carries X-Content-Type-Options: nosniff, and a served file gets its stored Content-Type and, when the blob has one, its stored Content-Disposition (attach.Store stores attachment for every type IsInlineContentType rejects; a stored value mime.ParseMediaType rejects is sent as attachment). Say that the handler derives no disposition from the type and sends no Content-Security-Policy, so a blob stored without a disposition is served inline with its type, and that reading the disposition costs one Attributes call per request. Keep the existing warning not to mount the ungated handler on the app origin. StaticHandlerPublic: add that it sends the same headers as StaticHandler. Signatures of StaticHandler, StaticHandlerPublic and servePublicBlobs do not change; no exported identifier is added. + +4. In modules/lagoon/attach/static_test.go (package attach, Docker-free, must not skip under -short): + - TestStoredDisposition: a table over the storedDisposition cases in the behavior block, failure message naming the input (use %q). + - TestStaticHandlerSendsStoredDisposition: build memblob.OpenBucket(nil) and fileblob.OpenBucket(t.TempDir(), nil), each closed through t.Cleanup, and run the same subtests on both (t.Run per bucket name). Use disk names whose partition matches PartitionDirectory, for example "abc123xyz.html" and "def456uvw.png", keyed with BlobKey, and write them with writeBlob and the types text/html and image/png, limit 0, body such as and a few bytes for the png. Serve through StaticHandler(bucket, "/storage/uploads") with httptest.NewRecorder for GET and HEAD, and through servePublicBlobs(bucket, "/storage/uploads", func that returns true, nil) for GET of the html blob. Assert the headers and bodies from the behavior block. For the png assert that w.Result().Header has no Content-Disposition key at all (Values returns an empty slice), not merely an empty string. + - On the memblob bucket only, write two more blobs with bucket.WriteAll and explicit blob.WriterOptions: an image/png with ContentDisposition `attachment; filename="a.png"` and a text/html with ContentDisposition "attachment;;", each under its own valid partition key, and assert the verbatim and the fail-closed value. + - A GET for a valid-shaped key that was never written (for example "/storage/uploads/mis/sin/g12/missing123.html" — check that its partition matches PartitionDirectory of its file name, or derive the path from BlobKey) answers 404 with nosniff and no Content-Disposition. + - Add the fileblob import; no new module dependency (inline_test.go already imports it). + +Do not commit at the end of this task; Task 2 commits code, tests and docs together. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./modules/lagoon/... ./modules/surf/... && go test -short -count=1 -v -run 'TestStoredDisposition|TestStaticHandler' ./modules/lagoon/attach && go test -short -count=1 ./modules/lagoon/... ./modules/surf/... + + + - `grep -c 'func storedDisposition(raw string) string' modules/lagoon/attach/static.go` prints 1. + - `grep -c 'bucket.Attributes(' modules/lagoon/attach/static.go` prints 1. + - `grep -c '"X-Content-Type-Options", "nosniff"' modules/lagoon/attach/static.go` prints 1. + - `git diff --quiet -- modules/lagoon/attach/inline.go modules/lagoon/attach/store.go modules/surf/uploads_test.go` exits 0: the writer side and the surf tests are untouched. + - `git diff -U0 -- modules/lagoon/attach/static.go > "$SCRATCH/oc8-static.diff"` exits 0 ($SCRATCH is the session scratchpad), and `grep -E '^[-+]func ' "$SCRATCH/oc8-static.diff"` prints only the added storedDisposition line. + - The -v run reports PASS (not SKIP) for TestStoredDisposition, TestStaticHandlerSendsStoredDisposition with both bucket subtests, TestStaticHandler, TestStaticHandlerServesThumbURL and TestStaticHandlerPublicGate. + - `go test -short ./modules/surf/...` passes with no surf edit, so TestUploadsMount and TestUploadsResponseHeaders still hold. + + Both handlers send nosniff on every response and the validated stored disposition on GET and HEAD of a served blob; blobs without one are served as before. Store's writer and the handler are proven end to end on memblob and fileblob under -short. Nothing is committed yet. + + + + Task 2: Gated Postgres assertion, lagoon README, storage and attachments docs, surf doc comment; commit Tasks 1 and 2 + modules/lagoon/attach/lifecycle_test.go, modules/lagoon/README.md, docs/services/storage.md, docs/database/attachments.md, modules/surf/uploads.go + The Docker daemon is reachable for testcontainers (docker info succeeds). If the socket is denied although the user is in the docker group, run the Docker-backed test through sudo -u golem, as CLAUDE.md says. + + - modules/lagoon/attach/lifecycle_test.go lines 1-30 (imports) and 190-245 (TestStaticHandlerPublicLooksUpIsPublic) + - modules/lagoon/README.md line 44 (Features, attach bullet; the clause "static serving with an optional `is_public` gate (`attach.StaticHandlerPublic`)") and lines 207-208 (the two handler rows) + - docs/services/storage.md lines 98-165 (Serving files and Deploy note) + - docs/database/attachments.md lines 84-92 and 136-141 + - modules/surf/uploads.go lines 29-45 (mountUploads doc comment) + + + - Through attach.StaticHandlerPublic with a real Postgres is_public lookup, a public row whose blob was written with ContentType text/html and ContentDisposition attachment answers GET 200 with Content-Disposition attachment and X-Content-Type-Options nosniff. + - Through the same handler, the existing public jpeg original answers 200 with nosniff and no Content-Disposition, and the private original still answers 404. + + +This task covers the gated half of the todo with a real database and the documentation rule from CLAUDE.md. + +1. In modules/lagoon/attach/lifecycle_test.go, TestStaticHandlerPublicLooksUpIsPublic: keep every existing assertion. Add a third public row, for example DiskName "fed654cba.txt", FileName "note.txt", ContentType "text/html", IsPublic &pub, created with gdb.Create, and write its blob at attach.BlobKey of that disk name with bucket.WriteAll and blob.WriterOptions{ContentType: "text/html", ContentDisposition: "attachment"} (what attach.Store writes for an HTML body uploaded as note.txt). Serve GET for its URL and for the existing public jpeg original through h with httptest.NewRecorder and assert the headers from the behavior block (a small local helper returning the recorder is fine; leave the existing get helper's signature as it is). Add a one-line comment that the stored disposition and nosniff go through the is_public gate. + +2. modules/lagoon/README.md: + - (a) Features, attach bullet: extend the clause about static serving so it says the handlers send each file's stored Content-Type and Content-Disposition with X-Content-Type-Options: nosniff, for example: static serving (`attach.StaticHandler`, with an optional `is_public` gate in `attach.StaticHandlerPublic`) that sends each file's stored Content-Type and Content-Disposition with `X-Content-Type-Options: nosniff`. + - (b) API table, `attach.StaticHandler` row: Serves stored files and thumbnails under a URL prefix with their stored Content-Type and Content-Disposition and `X-Content-Type-Options: nosniff`. The `attach.StaticHandlerPublic` row stays as it is (it already says "attach.StaticHandler plus"). + +3. docs/services/storage.md: + - Serving files, the paragraph that starts "Without the key, the application decides where files are served": after "mount `attach.StaticHandlerPublic` itself", add that it sends the stored Content-Type, the stored Content-Disposition (so a file `attach.Store` stored as an attachment downloads) and `X-Content-Type-Options: nosniff`, but not the route's sandboxing Content-Security-Policy, and it adds no disposition to a file stored without one (see the deploy note). Keep the separate-origin advice. + - Deploy note, the paragraph that starts "Objects written before this change": add that `attach.StaticHandler` and `attach.StaticHandlerPublic` send the stored disposition too, so the same caveat applies to them, while the built-in route also decides from the type. + - Deploy note, the file:// sentence at the end of the section: add that `attach.StaticHandler`, `attach.StaticHandlerPublic` and the built-in route read the sidecar, so the static-server case applies only to a web server reading the directory. + - Keep the `#deploy-note` heading and anchor, the nginx example and every existing fact. + +4. docs/database/attachments.md: + - Line 87, after "`attach.StaticHandler` serves originals and thumbnails under that prefix; `attach.StaticHandlerPublic` does the same and answers 404 for a row whose `is_public` flag is false.": add that both send `X-Content-Type-Options: nosniff` and each file's stored Content-Type and Content-Disposition, and send no Content-Security-Policy, which is why the warning below still applies. + - Line 140 (the paragraph about attach.Store writing Content-Disposition: attachment): extend "downloads it instead of rendering it" to cover a browser that opens it from a public bucket or through `attach.StaticHandler` and `attach.StaticHandlerPublic`. + +5. modules/surf/uploads.go, mountUploads doc comment only (no code change): replace the clause saying attach.StaticHandlerPublic sends only the stored type. The new wording: attach.Store also stores the attachment disposition with the blob and attach.StaticHandlerPublic sends it, but the route still decides from the type on every response, because blobs written earlier or outside attach.Store carry no disposition. + +6. Run the docs checkers and fix any identifier, link or anchor they report. Use only identifiers that exist: attach.StaticHandler, attach.StaticHandlerPublic, attach.Store, attach.IsInlineContentType. Config keys are not checked automatically; keep storage.uploads.serve and storage.uploads.public_url spelled exactly as the pages already have them. + +7. Commit Tasks 1 and 2 as one commit, staging exactly these paths by name: + - modules/lagoon/attach/static.go + - modules/lagoon/attach/static_test.go + - modules/lagoon/attach/lifecycle_test.go + - modules/lagoon/README.md + - docs/services/storage.md + - docs/database/attachments.md + - modules/surf/uploads.go + + Message subject: fix(lagoon/attach): StaticHandler sends the stored Content-Disposition and nosniff (quick-261105-oc8). Body: a short bullet list (both handlers set X-Content-Type-Options nosniff; the stored disposition is read with one Attributes call after the is_public gate and sent on GET and HEAD, an unparsable value as attachment; blobs without one are served as before; README, storage and attachments pages, surf comment). Never stage everything at once. Leave other sessions' STATE.md, state.json, G15OFFICE.md and the untracked .gitkeep, .msd and quick-batches files alone. Add no co-author tag (user and project CLAUDE.md override the harness attribution reminder). Do not push or tag. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/summercms.go && go vet ./... && go test -short ./... && go test ./cmd/summer -run TestDocsTree && go run ./cmd/summer docs:build --check && go test -count=1 -v -run 'TestStaticHandlerPublicLooksUpIsPublic$' ./modules/lagoon/attach && go test -count=1 -run 'TestUploads' ./modules/surf + + + - `grep -c 'nosniff' modules/lagoon/README.md` prints at least 2 (Features bullet and the StaticHandler row). + - `grep -c 'attach.StaticHandler' docs/services/storage.md` prints at least 2, and `grep -c 'nosniff' docs/database/attachments.md` prints at least 2. + - `grep -c '^### Deploy note' docs/services/storage.md` prints 1 (the anchor survives). + - The Docker-backed -v run reports PASS, not SKIP, for TestStaticHandlerPublicLooksUpIsPublic. If the socket is denied, rerun through `sudo -u golem`. + - After committing, `SHA1=$(git log -1 --format=%H)` and `git show --name-only --format= "$SHA1"` list exactly the 7 paths above. + - `git show "$SHA1" -- modules/surf/uploads.go > "$SCRATCH/oc8-surf.diff"` exits 0, and every changed line in it (lines starting with a single + or -, excluding the +++/--- headers) starts with `//` after the sign: the surf change is comment-only. + - `git show "$SHA1" -- modules/lagoon/ modules/surf/ > "$SCRATCH/oc8-api.diff"` exits 0, and `grep -e '^[+-]func [A-Z]' -e '^[+-]type [A-Z]' -e '^[+-]var [A-Z]' -e '^[+-]const [A-Z]' "$SCRATCH/oc8-api.diff"` prints nothing: no exported identifier is added, removed or changed. + - `git log -1 --format=%B "$SHA1" > "$SCRATCH/oc8-msg.txt"` exits 0, and `grep -ci 'co-authored-by' "$SCRATCH/oc8-msg.txt"` prints 0. + + One commit. Both handlers send nosniff and the stored disposition, proven Docker-free on memblob and fileblob and through the real is_public gate on Postgres. The lagoon README, both docs pages and the surf comment describe the behaviour and its limits. go vet, go test -short ./..., the docs checkers, the Docker-backed handler test and the surf uploads tests are green. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| bucket attributes → response headers | Blob metadata is written by attach.Store, but any process with bucket write access (or an S3 console user) can set an arbitrary Content-Disposition. The handler forwards it into a response header | +| attach.StaticHandler(Public) → browser | An application mounting a handler directly gets no CSP and no type-derived disposition. A rendering type served inline on a trusted origin is stored XSS | +| surf uploads route → attach.StaticHandlerPublic | The route wraps the handler and sets its own nosniff, CSP and type-derived disposition after the handler's headers | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-q261105oc8-01 | Elevation of privilege | An HTML, SVG or text upload stored by attach.Store and served inline by a directly mounted StaticHandler or StaticHandlerPublic | medium | mitigate | Task 1: servePublicBlobs sends the stored Content-Disposition (attachment for every non-inline type Store wrote). TestStaticHandlerSendsStoredDisposition writes through writeBlob and asserts the header on GET and HEAD; Task 2 asserts it through the real is_public gate | +| T-q261105oc8-02 | Tampering | Header injection or malformed values through a stored Content-Disposition | low | mitigate | storedDisposition sends a value only when mime.ParseMediaType accepts it and otherwise the bare attachment constant; a CR/LF value is a TestStoredDisposition case. Go's header writer also replaces CR/LF | +| T-q261105oc8-03 | Information disclosure | Content sniffing of a served blob turning a non-HTML type into a rendered document | low | mitigate | Task 1 sets X-Content-Type-Options: nosniff on every response of both handlers; asserted on 200 GET, 200 HEAD and 404 | +| T-q261105oc8-04 | Elevation of privilege | Blobs written before quick-261028-rqe or by writers that bypass attach.Store carry no disposition and are still served inline by the handlers | medium | transfer | Task 2's docs say the handlers send only the stored disposition, keep the separate-origin warning and the deploy-note backfill advice; the surf route keeps its type-derived disposition. Direct writers (fonoteka.go, sm-user-plugin avatar, sm-feedback-plugin screenshot) are recorded as follow-ups | +| T-q261105oc8-05 | Information disclosure | Probing private files through a new bucket call before the gate | low | mitigate | The Attributes call runs after the allow gate, so a denied or missing row still 404s without touching the bucket; TestStaticHandlerPublicGate (seen-before-NewReader) stays unmodified | +| T-q261105oc8-06 | Denial of service | One extra Attributes call per request (an S3 HEAD) on the handlers and the built-in route | low | accept | The reader does not expose the disposition in gocloud.dev v0.46.0; the cost is documented in the StaticHandler doc comment. Production deployments are advised to serve uploads from a separate origin or CDN | +| T-q261105oc8-SC | Tampering | npm/pip/cargo installs | low | accept | No new dependency. mime is stdlib; gocloud.dev/blob/fileblob is already a module dependency and already imported by tests in the package | + + + +- go vet ./... and go test -short ./... are green at the commit. +- go test ./cmd/summer -run TestDocsTree and go run ./cmd/summer docs:build --check are green. +- Docker-backed: TestStaticHandlerPublicLooksUpIsPublic passes (not skipped), and go test -run 'TestUploads' ./modules/surf passes without -short. +- One commit containing exactly the 7 files_modified paths, no co-author tag, nothing pushed or tagged. + + + +- A blob attach.Store wrote for a non-inline type is served by attach.StaticHandler and attach.StaticHandlerPublic with Content-Disposition: attachment, on GET and HEAD. +- Every handler response carries X-Content-Type-Options: nosniff. +- A blob without a stored disposition is served exactly as before; no exported identifier changes; the surf route's observable headers are unchanged. +- The lagoon README, docs/services/storage.md, docs/database/attachments.md and the surf doc comment match the new behaviour. + + + +Create `.planning/quick/261105-oc8-lagoon-attach-statichandler-and-statichandlerpublic-send-the/261105-oc8-SUMMARY.md` when done. List the follow-ups from the objective's "Not in scope" block in it so the orchestrator can file them. + diff --git a/.planning/quick/261105-oc8-lagoon-attach-statichandler-and-statichandlerpublic-send-the/261105-oc8-SUMMARY.md b/.planning/quick/261105-oc8-lagoon-attach-statichandler-and-statichandlerpublic-send-the/261105-oc8-SUMMARY.md new file mode 100644 index 00000000..76dc0b6e --- /dev/null +++ b/.planning/quick/261105-oc8-lagoon-attach-statichandler-and-statichandlerpublic-send-the/261105-oc8-SUMMARY.md @@ -0,0 +1,75 @@ +--- +phase: quick-261105-oc8 +plan: 01 +subsystem: lagoon/attach +status: complete +tags: [security, uploads, headers, content-disposition, nosniff] +requires: [quick-261028-rqe] +provides: + - attach.StaticHandler and attach.StaticHandlerPublic send X-Content-Type-Options nosniff and the validated stored Content-Disposition +affects: [modules/surf uploads route (doc comment only)] +tech-stack: + added: [] + patterns: ["fail-closed header forwarding: stored metadata validated with mime.ParseMediaType, else the bare attachment constant"] +key-files: + created: [] + modified: + - modules/lagoon/attach/static.go + - modules/lagoon/attach/static_test.go + - modules/lagoon/attach/lifecycle_test.go + - modules/lagoon/README.md + - docs/services/storage.md + - docs/database/attachments.md + - modules/surf/uploads.go +decisions: + - "Handlers forward only the stored disposition; no type-derived disposition and no CSP (additive, the surf route keeps that role)" + - "Attributes call placed after the is_public gate and before NewReader; disposition set only after both succeed so a 404 never carries it" +requirements: [todo:2026-10-28-attach-static-handler-disposition] +duration: ~15min +completed: 2026-11-05 +commits: 1 +plan_head_before: 5aa846c5e85368b534e03a1d19f441bb34de193c +plan_head_after: 27c98bf22aa61add1b90904456dc37fec4310ca8 +actuals: + tokens: 9000 + tasks: 2 + commits: 1 +--- + +# Quick 261105-oc8: attach.StaticHandler sends stored Content-Disposition and nosniff Summary + +Both `attach.StaticHandler` and `attach.StaticHandlerPublic` now set `X-Content-Type-Options: nosniff` on every response and forward the blob's stored Content-Disposition (read with one `bucket.Attributes` call after the `is_public` gate, validated by `mime.ParseMediaType`, fail-closed to `attachment`) on GET and HEAD, so an HTML/SVG/PDF/text upload stored by `attach.Store` downloads instead of rendering when an app mounts the handlers directly. + +## Commit + +- `27c98bf` fix(lagoon/attach): StaticHandler sends the stored Content-Disposition and nosniff (quick-261105-oc8) — exactly the 7 planned paths, no co-author tag, not pushed or tagged. + +## Tasks + +1. **Task 1 (tracer, TDD):** RED: `TestStoredDisposition` and `TestStaticHandlerSendsStoredDisposition` failed to compile (`undefined: storedDisposition`). GREEN: unexported `storedDisposition` helper, nosniff after the method check, `Attributes` after the gate, disposition set next to Content-Type before the HEAD return. Tests run on memblob and fileblob (sidecar) through `writeBlob`, GET and HEAD, ungated and gated (`servePublicBlobs` with an allow-all func), verbatim `attachment; filename="a.png"`, fail-closed `attachment;;`, png without any Content-Disposition key, 404 with nosniff and no disposition. Tracer gate re-run passed. +2. **Task 2:** `TestStaticHandlerPublicLooksUpIsPublic` adds a public `text/html` row (`fed654cba.txt`) with stored `attachment` and asserts disposition + nosniff through the real Postgres `is_public` gate; the public jpeg gets nosniff and no disposition; existing assertions untouched. Lagoon README (Features bullet, `attach.StaticHandler` row), `docs/services/storage.md` (Serving files paragraph, deploy-note caveat, `file://` sidecar sentence), `docs/database/attachments.md` (handler paragraph, Store disposition paragraph), surf `mountUploads` doc comment (comment-only diff). + +## Verification + +- `go vet ./...`, `go test -short ./...`: green. +- `go test ./cmd/summer -run TestDocsTree`, `go run ./cmd/summer docs:build --check`: green ("no problems found"). +- Docker-backed `TestStaticHandlerPublicLooksUpIsPublic`: PASS (not skipped). `go test -run TestUploads ./modules/surf` (no -short): PASS with no surf test edit. +- `inline.go`, `store.go`, `uploads_test.go` untouched; static.go diff adds only `func storedDisposition`; surf diff is comment-only; no exported identifier added/removed/changed in modules/lagoon or modules/surf (the only `func [A-Z]` lines in the diff are the two new `Test*` functions in `static_test.go`). + +## Deviations from Plan + +None - plan executed exactly as written. (The missing-blob test uses `BlobKey("mis123sin.html")` so its partition matches.) + +## Threat Flags + +None beyond the plan's threat model. Note for the surf route: a blob stored with a disposition on an inline raster type (which `attach.Store` never writes) would now have that stored disposition forwarded through the route too; the route's own type-derived `attachment` for non-inline types is unchanged. + +## Follow-ups (not in scope; for the orchestrator to file) + +- Blob writers that bypass `attach.Store` should adopt `attach.IsInlineContentType` / store the disposition: fonoteka.go `storeUpload` and `StorePublicFile` (see todo `2026-10-28-fonoteka-uploads-use-disk-extension.md`), the sm-user-plugin avatar writer, the sm-feedback-plugin screenshot writer. +- Backfilling the disposition on objects already in a bucket is an operator task (documented in `docs/services/storage.md#deploy-note`). +- Move todo `2026-10-28-attach-static-handler-disposition` to completed and post the g15office comment (`knuu7fil`). + +## Self-Check: PASSED + +- All 7 modified files exist; commit `27c98bf` present in `git log`. diff --git a/.planning/quick/261105-oc9-sm-demo-app-hardening/261105-oc9-PLAN.md b/.planning/quick/261105-oc9-sm-demo-app-hardening/261105-oc9-PLAN.md new file mode 100644 index 00000000..10ab6fe8 --- /dev/null +++ b/.planning/quick/261105-oc9-sm-demo-app-hardening/261105-oc9-PLAN.md @@ -0,0 +1,249 @@ +--- +phase: quick-261105-oc9 +plan: 01 +type: execute +wave: 1 +depends_on: [] +files_modified: + - ../sm-demo-app/plugins/demo/chat_api.go + - ../sm-demo-app/plugins/demo/classes/members.go + - ../sm-demo-app/plugins/demo/console/seed.go + - ../sm-demo-app/plugins/demo/console/seed_chat.go + - ../sm-demo-app/plugins/demo/console/seed_friends.go + - ../sm-demo-app/plugins/demo/plugin.go + - ../sm-demo-app/plugins/demo/updates/updates.go + - ../sm-demo-app/go.mod + - ../sm-demo-app/chat_test.go + - ../sm-demo-app/chat_smoke_test.go + - ../sm-demo-app/seed_chat_test.go + - ../sm-demo-app/README.md + - ../sm-demo-app/scripts/check-moderation.sh + - ../sm-demo-app/scripts/check-local.sh + - ../sm-demo-app/deploy/nginx/demo.summercms.io.conf + - ../sm-demo-app/scripts/check-deploy.sh + - ../sm-demo-app/scripts/check-live.sh + - ../sm-demo-app/DEPLOY.md +autonomous: true +requirements: + - quick-261105-oc9 + +must_haves: + truths: + - "GET /_demo/api/v1/chat/members writes nothing: a member without a handle is listed with handle \"\" and its users.handle is still NULL after the call (no handle assignment in a GET, so two concurrent first loads cannot race on the same rows)." + - "GET /_demo/api/v1/chat/members answers 200 for a community of more than 65535 members: the query filters users with the community's membership condition as a subquery (WHERE id <> caller AND ), never an id list bound as parameters." + - "demo:seed has a step \"member handles\" right after \"chat personas\" that gives every user without a handle one (the visitor, rows created outside registration): \"updated\" when it assigned any, \"exists\" otherwise." + - "The default moderation lists are written once, by golem15.demo's own migration, which runs right after golem15.moderation's migration creates the settings row and fills the lists only while both are empty. demo:seed no longer touches golem15_moderation_settings, so an admin who empties both lists keeps them empty across demo:seed and demo:reset." + - "scripts/check-local.sh passes every member JWT to curl through a config on stdin (curl -K -) from request(); no curl argv carries a bearer token. The visitor's login body goes to curl from a file in the 0700 temp directory, not argv." + - "deploy/nginx/demo.summercms.io.conf proxies only /livekit/rtc (and the paths below it, /rtc/validate and /rtc/v1 included) to LiveKit's /rtc on 127.0.0.1:7890; every other /livekit/ path answers 404 from nginx, so LiveKit's Twirp RoomService is reachable on loopback only. check-deploy.sh enforces the shape, check-live.sh probes it. The vhost is committed only, not installed on rome." + - "DEPLOY.md's VAPID step runs with set -eo pipefail, generates the pair into a variable, refuses to write unless both keys are non-empty, and appends all three lines with one tee; no key is ever printed." + - "go vet ./... and go test -short ./... in sm-demo-app are green at every commit; the Docker-backed tests named in each task pass; nothing is pushed, tagged or deployed; commits carry no co-author tag." + artifacts: + - path: ../sm-demo-app/plugins/demo/classes/members.go + provides: "OtherMembers(ctx, db, exceptID): the community's members except one user, ordered by id, read-only, membership as a subquery" + - path: ../sm-demo-app/plugins/demo/chat_api.go + provides: "chatMembers reads through classes.OtherMembers; no MemberIDs id list, no handle writes" + - path: ../sm-demo-app/plugins/demo/updates/updates.go + provides: "All(): the golem15.demo migration that fills golem15.moderation's default lists once" + - path: ../sm-demo-app/plugins/demo/plugin.go + provides: "Plugin implements pact.HasMigrations (Migrations returns updates.All())" + - path: ../sm-demo-app/deploy/nginx/demo.summercms.io.conf + provides: "location ^~ /livekit/rtc to http://127.0.0.1:7890/rtc; location ^~ /livekit/ answers 404" + key_links: + - from: "GET /_demo/api/v1/chat/members (plugins/demo/chat_api.go chatMembers)" + to: "plugins/demo/classes/members.go OtherMembers" + via: "the unexported memberWhere condition of the classes package (chat_context.go, not edited)" + pattern: "OtherMembers" + - from: "lagoon.Migrate / summer migrate (party activation order: golem15.demo Requires golem15.moderation)" + to: "plugins/demo/updates All() via Plugin.Migrations()" + via: "history table summer_migrations_golem15_demo" + pattern: "HasMigrations" + - from: "scripts/check-local.sh request()" + to: "curl -K - (config on stdin)" + via: "printf of the header line piped into curl" + pattern: "--jwt" +--- + + +Close the five sm-demo-app todos from the phase 20 code review in the sibling repo /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app, inside that repo only: + +1. demo-chat-members-read-only.md (IN-09): GET /_demo/api/v1/chat/members becomes read-only and selects members with the membership subquery instead of an id list; missing handles are assigned in demo:seed. +2. demo-seed-moderation-lists-once.md (IN-05): the default moderation lists are seeded once, by a golem15.demo migration, not on every demo:seed. +3. check-local-jwt-out-of-argv.md (IN-03): check-local.sh passes member JWTs to curl through a config on stdin. +4. demo-nginx-livekit-rtc-only.md (IN-06): nginx proxies only LiveKit's /rtc paths; check-deploy and check-live follow. +5. demo-vapid-runbook-pipefail.md (IN-08): the VAPID runbook step uses pipefail, generates into a variable, checks both keys and appends with one tee. + +Purpose: remove a write-in-GET race and a 500 past Postgres' 65535 bind parameters on a growing demo, stop deploys from undoing an admin's moderation choice, keep tokens out of /proc/*/cmdline, shrink LiveKit's public surface and make the VAPID rebuild step fail safe. +Output: five sm-demo-app commits (one per todo), no framework change, no shared-plugin change, no frontend change (the members response shape is unchanged). + +Claude's discretion choices (no CONTEXT.md exists for a quick item): +- "Seed only once" is implemented as a golem15.demo data migration (the todo's first option, "insert the lists when the settings row is created"): gormigrate records it in summer_migrations_golem15_demo, demo:reset keeps settings and migrations, and no flag table is needed. This is the demo plugin's first migration; the plugin doc comment and README change accordingly. +- The members helper goes into a NEW file plugins/demo/classes/members.go (same package as the unexported memberWhere), so this item does not edit plugins/demo/classes/chat_context.go, which item 261105-oc6 edits for its pin policy. +- The "member handles" seed step covers every user without a handle (as user:backfill-handles does), not only members. +- check-local.sh's visitor login body (it carries the local visitor password) also leaves argv, through a file in the script's 0700 temp directory: same leak class, same fix, two lines. + +Closing the five todos (moving them to .planning/todos/done/ with a "## Closed" note and syncing g15office) is the orchestrator's closing docs step, not part of the executor's sm-demo-app commits. + + + +@~/.claude/msd-core/workflows/execute-plan.md +@~/.claude/msd-core/templates/summary.md + + + +@.planning/STATE.md +@CLAUDE.md +@.planning/todos/pending/demo-chat-members-read-only.md +@.planning/todos/pending/demo-seed-moderation-lists-once.md +@.planning/todos/pending/check-local-jwt-out-of-argv.md +@.planning/todos/pending/demo-nginx-livekit-rtc-only.md +@.planning/todos/pending/demo-vapid-runbook-pipefail.md +@../sm-demo-app/README.md +@../sm-demo-app/plugins/demo/chat_api.go +@../sm-demo-app/plugins/demo/classes/chat_context.go +@../sm-demo-app/plugins/demo/console/seed.go +@../sm-demo-app/plugins/demo/console/seed_friends.go + +Facts established while planning (sm-demo-app at da03c06, working tree clean): +- chat_api.go chatMembers (lines 319-367) calls classes.MemberIDs, binds every id into "id IN ? AND id <> ?", then calls the user plugin's handle assignment for each member lacking a handle. memberWhere (classes/chat_context.go) is an unexported const: activated, not deleted, no user_throttle row with is_banned. classes.MemberIDs stays (community.Members uses it). +- Registration already assigns handles (sm-user-plugin controllers/registration.go, used by social login too) and so does the admin create. Rows without a handle come from direct inserts: demo:seed's visitor (seed.go visitor.seed), legacy rows, and test helpers (demo_api_test.go apiDemo.member uses a.DB.Create without a handle). +- seed_chat.go seedPersonas already assigns persona handles at creation. +- golem15.moderation's migration 202610280001 creates golem15_moderation_settings and inserts row id 1 with NULL lists. lagoon runs each plugin's set in party activation order with history table summer_migrations_; golem15.demo Requires golem15.moderation. internal/demotest migrates a template database with every plugin (lagoon.Migrate), so test databases already carry the demo migration's effect. +- pact.HasMigrations is Migrations() []*gormigrate.Migration. gormigrate v2.1.7 is in sm-demo-app go.mod as indirect only. +- Tests that pin the current lists behaviour: seed_chat_test.go TestSeedChat (lines 181-219), chat_smoke_test.go chatSeedSteps (line 226-230, includes "moderation lists") and TestSeedChatSmoke (line 301). TestDemoChatMembers (chat_test.go 320-372) asserts every listed member has a handle, which members created by apiDemo.member only had because the GET wrote one. +- check-local.sh runs under set -euo pipefail; request() is at lines 124-130; 15 calls pass a bearer header literally and 12 more through local auth variables (check_quizzes, check_forum, check_call); visitor_jwt (lines 325-336) puts the login JSON in -d. check-moderation.sh request() (lines 80-93) is the pattern to copy. +- nginx: location ^~ /livekit/ at conf lines 355-375 proxies the whole LiveKit HTTP surface. check-deploy.sh lines 26-31 (header) and 329-341 (python block) pin that location; check-live.sh lines 40-42 (header) and 292-298 probe /livekit/rtc/validate. DEPLOY.md mentions /livekit/ at lines 19, 364, 370, 510, 839, 916; README lines 280 and 282 describe check-deploy and check-live. +- DEPLOY.md VAPID step: lines 533-547; other remote snippets in DEPLOY.md already use set -euo pipefail. +- Baseline: go vet ./... and go test -short ./... green; scripts/check-deploy.sh prints "check-deploy: ok"; shellcheck -S error is clean on check-local.sh, check-live.sh, check-deploy.sh; Docker is reachable for the testcontainers tests (if the socket is denied, run them through sudo -u golem, per the framework CLAUDE.md). + + + + + + Task 1: GET /_demo/api/v1/chat/members reads only, through the membership subquery; demo:seed assigns missing handles (IN-09) + ../sm-demo-app/plugins/demo/classes/members.go, ../sm-demo-app/plugins/demo/chat_api.go, ../sm-demo-app/plugins/demo/console/seed.go, ../sm-demo-app/plugins/demo/console/seed_chat.go, ../sm-demo-app/chat_test.go, ../sm-demo-app/seed_chat_test.go, ../sm-demo-app/README.md + + - TestDemoChatMembers (rewritten): the four personas are listed with their handles (ada, ben, ola, tomek); the other member created by apiDemo.member (no handle) is listed with handle "" and, after the GET, SELECT handle FROM users for that id is still NULL; the caller and the banned user are not listed; a banned caller gets 403; after mustSeed runs again, the same GET lists the other member with a non-empty handle. Red on the current code: the GET writes the handle. + - TestDemoChatMembersManyMembers (new): after bootAPI, one INSERT ... SELECT FROM generate_series(1, 66000) adds 66000 activated users (email 'bulk-' || g || '@example.com', password 'x', is_activated true, created_at and updated_at now()); a member's GET answers 200 and lists at least 66000 members. Red on the current code: 500 from the bind-parameter limit. + - TestSeedChat: with the visitor configured, the first demo:seed reports "member handles" as updated and the visitor's users.handle is non-empty; the second run reports exists. TestSeedChatWithoutVisitor: "member handles" reports exists (personas get handles at creation). + + +Work in /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app. RED first: change the tests in the behavior block (TestDemoChatMembers and the new TestDemoChatMembersManyMembers in chat_test.go, the step assertions in seed_chat_test.go; do not change apiDemo.member in demo_api_test.go, its handle-less rows are what the test needs), run them and note the failures for the SUMMARY. Do not commit red tests: the repo keeps go test green at every commit. + +GREEN: +1. New file plugins/demo/classes/members.go (package classes; do not edit chat_context.go, item 261105-oc6 edits it). Add OtherMembers(ctx context.Context, db *gorm.DB, exceptID uint) ([]usermodels.User, error): nil db gives an empty slice; otherwise query the user plugin's User model with Select of id, name, surname and handle, Where "users.id <> ?" exceptID, Where(memberWhere), Order "users.id", Find. It never writes. Doc comment: the community's members except one user, ordered by id, read-only; the membership condition stays in SQL, so no id list is bound however large the community grows. +2. plugins/demo/chat_api.go chatMembers: keep the caller resolution and the IsMember 403; replace the MemberIDs call, the id-list query and the per-member handle assignment with one OtherMembers call (500 on error); build chatMemberJSON from each row with deref as now (an absent handle is ""). Drop the imports that become unused. Rewrite its doc comment: every member but the caller, ordered by id, read-only; the handle is the user plugin's, assigned at registration or by demo:seed, empty for a row that has none yet; is_bot always false. +3. plugins/demo/console/seed_chat.go: add seedMemberHandles(ctx) returning func(*gorm.DB) (string, error). It finds every user whose handle IS NULL or trims to '' (the GORM User model, ordered by id) and calls the user plugin's handle assignment on each inside the step's transaction, exactly as seedPersonas does for a new persona; it returns "updated" when it assigned at least one handle, "exists" otherwise. +4. plugins/demo/console/seed.go: in runSeed's steps insert {"member handles", seedMemberHandles(ctx)} directly after the "chat personas" step. Add "member handles" to SeedCommand's Description (after "the chat personas"). +5. README.md: the GET /_demo/api/v1/chat/members row (line 173) says the call reads only and a member without a handle shows an empty one until demo:seed assigns it; the Seed list (around line 123) mentions the handles for users without one; the Testing paragraph (lines 342-353) lists TestDemoChatMembersManyMembers next to TestDemoChatMembers. + +Run gofmt on the touched Go files, go vet ./..., go test -short ./..., then the Docker tests below. Commit in sm-demo-app, staging these paths explicitly, message "fix: GET /_demo/api/v1/chat/members reads only and selects members with a subquery", body naming IN-09 and the new demo:seed step "member handles". No co-author tag. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app && test -z "$(gofmt -l plugins/demo *.go)" && go vet ./... && go test -short ./... && go test -count=1 -run 'TestDemoChatMembers|TestSeedChat|TestDemoDMs|TestDemoChatBootstrap|TestDemoChatTracer' . 2>&1 | tail -5 && git log -1 --format='%h %s' && git show --stat --format= HEAD + + GET /_demo/api/v1/chat/members answers from one read-only query with the membership subquery: a handle-less member stays NULL in the database and shows "" in the answer, 66000 members answer 200; demo:seed's "member handles" step assigns the missing handles (updated, then exists); README updated; vet, short tests and the named Docker tests pass; one sm-demo-app commit. + + + + Task 2: the default moderation lists are seeded once, by a golem15.demo migration (IN-05) + ../sm-demo-app/plugins/demo/updates/updates.go, ../sm-demo-app/plugins/demo/plugin.go, ../sm-demo-app/plugins/demo/console/seed_friends.go, ../sm-demo-app/plugins/demo/console/seed.go, ../sm-demo-app/go.mod, ../sm-demo-app/seed_chat_test.go, ../sm-demo-app/chat_smoke_test.go, ../sm-demo-app/scripts/check-moderation.sh, ../sm-demo-app/README.md + + - TestSeedChat: before the first demo:seed, golem15_moderation_settings row 1 already holds the defaults (buy followers, viagra, bit.ly) and summer_migrations_golem15_demo has the migration's id; demo:seed's output has no "moderation lists" line. + - TestSeedChat: after an admin sets both lists to '' (and again with both NULL), two demo:seed runs leave both lists empty. Red on the current code: the seed refills them. The existing "an admin's own list survives a later seed" check stays. + - chatSeedSteps no longer lists "moderation lists"; TestSeedChatSmoke's "default moderation lists were filled" assertion still holds (the migration filled them) and its settings fingerprint across demo:reset still matches. + + +Work in /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app. RED first: change seed_chat_test.go and chatSeedSteps in chat_smoke_test.go as in the behavior block, run TestSeedChat and note the failure; do not commit red tests. + +GREEN: +1. New package plugins/demo/updates, file updates.go. Move defaultBannedWords and defaultBannedLinks (same values, same comment about mild defaults) here from console/seed_friends.go. Define one gormigrate migration with ID "202611050001_seed_golem15_moderation_lists" whose Migrate runs the exact UPDATE statement seedModerationLists runs today (moderation settings row id 1, only while both lists trim to empty), moved here unchanged; its Rollback returns nil (from then on the lists belong to the admin). Export All() []*gormigrate.Migration returning it. Doc comment: golem15.demo's migrations; this one gives golem15.moderation's settings row the default lists once, right after golem15.moderation's own migration creates it (golem15.demo requires golem15.moderation, so its set runs later), and only while both lists are empty, so on an existing database whose lists were already seeded it changes nothing and is just recorded. +2. plugins/demo/plugin.go: add pact.HasMigrations to the interface assertion block and func (p *Plugin) Migrations() []*gormigrate.Migration returning updates.All(). Replace "It adds no admin screens or migrations of its own." in the package doc with: no admin screens; one migration, which fills golem15.moderation's default word and link lists once. +3. plugins/demo/console/seed_friends.go: delete seedModerationLists and the two constants. plugins/demo/console/seed.go: delete the {"moderation lists", ...} step and drop "the moderation lists" from SeedCommand's Description. +4. go.mod: gormigrate is now imported directly. Hand-move the github.com/go-gormigrate/gormigrate/v2 v2.1.7 line from the indirect block into the first require block without the indirect marker (keep the version; do not run go mod tidy, the workspace replaces would make it reach for the network). go.sum does not change. +5. scripts/check-moderation.sh header (line 10): the lists the controls guard against are those that differ from the demo migration's defaults, no longer demo:seed's. Comment only. +6. README.md: the Seed list (line 124) drops "the default moderation lists"; the Moderation paragraph (around line 192) says golem15.demo's migration fills a short default list once, when the settings row is new, and an admin who empties both lists keeps them empty; the Reset paragraph's "(the moderation lists included)" stays true. + +Run gofmt, go vet ./..., go test -short ./..., then the Docker tests below. Commit in sm-demo-app, staging these paths explicitly, message "fix: seed the default moderation lists once, in a golem15.demo migration", body naming IN-05 and that an admin's emptied lists now survive deploys. No co-author tag. + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app && test -z "$(gofmt -l plugins/demo *.go)" && go vet ./... && go test -short ./... && go test -count=1 -run 'TestSeedChat|TestDemoModeration|TestResetKeepsPlans|TestBootAllPlugins|TestGoModRequiresTags' . 2>&1 | tail -5 && test "$(grep -hv '^[[:space:]]*//' plugins/demo/console/*.go | grep -c 'golem15_moderation_settings')" = 0 && grep -q 'func (p \*Plugin) Migrations()' plugins/demo/plugin.go && git log -1 --format='%h %s' && git show --stat --format= HEAD + + golem15.demo carries one migration that fills the default lists once; demo:seed and the console package no longer touch golem15_moderation_settings; lists an admin emptied stay empty after demo:seed; tests, check-moderation comment and README updated; vet, short tests and the named Docker tests pass; one sm-demo-app commit. + + + + Task 3: check-local JWTs through a curl config (IN-03), nginx proxies only LiveKit's /rtc (IN-06), VAPID runbook with pipefail (IN-08) + ../sm-demo-app/scripts/check-local.sh, ../sm-demo-app/deploy/nginx/demo.summercms.io.conf, ../sm-demo-app/scripts/check-deploy.sh, ../sm-demo-app/scripts/check-live.sh, ../sm-demo-app/DEPLOY.md, ../sm-demo-app/README.md + +Work in /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app. Three commits, in this order, each staging only its own paths. No co-author tags. Nothing is pushed and nothing is installed on rome. + +Commit A, scripts/check-local.sh (IN-03): +- request() takes an optional leading pair --jwt JWT before METHOD URL. With a non-empty JWT it pipes one config line, header = "Authorization: Bearer JWT" written with printf (a builtin, so the token never reaches an argv), into curl with -K - and otherwise the same curl flags, output files and the same fallback to 000 as now (the script runs under pipefail, so a curl failure still prints 000). Without --jwt, or with an empty JWT, it runs exactly as now. Copy the shape of check-moderation.sh's request() (lines 80-93). Update the comment above it to "request [--jwt JWT] METHOD URL [curl args...]" and say the token goes through a curl config on stdin, never argv. +- Every call that today passes a bearer header as a -H argument, literally or through the auth locals of check_quizzes, check_forum and check_call (27 calls: realtime, decks, quizzes, forum, friends, call, herald), passes --jwt with the matching variable ($jwt, $a_jwt, $b_jwt) instead. Remove the auth locals and their declarations. +- visitor_jwt: write the login JSON (the python3 one-liner's output, unchanged) to "$TMP/login.json" and pass -d @"$TMP/login.json", so the local visitor password leaves argv too ($TMP is the script's mktemp -d directory, mode 0700). +- If the local stack happens to be up (scripts/dev.sh up), also run scripts/check-local.sh realtime decks quizzes forum friends call herald and record the result; it is not required. +- Commit "test: check-local passes member JWTs to curl through a config on stdin" (body: IN-03). + +Commit B, LiveKit signalling only (IN-06): +- deploy/nginx/demo.summercms.io.conf: replace the location ^~ /livekit/ block (lines 355-375) with two blocks. First location ^~ /livekit/rtc with proxy_pass http://127.0.0.1:7890/rtc; and every directive the old block had (proxy_http_version 1.1, Upgrade and Connection upgrade, Host, X-Real-IP, X-Forwarded-For, X-Forwarded-Proto, proxy_read_timeout 7d, proxy_send_timeout 7d, proxy_buffering off). Then location ^~ /livekit/ with only return 404;. Keep tab indentation and the closing brace on its own tab-indented line (check-deploy's regexes rely on it). Rewrite the comment: livekit-client appends /rtc (and /rtc/validate, /rtc/v1) to livekit_url wss://demo.summercms.io/livekit, so only that prefix reaches LiveKit's /rtc; the rest of LiveKit's HTTP surface, the Twirp RoomService included, stays on loopback for the Go binary (livekit_admin_url), and nginx answers 404 for any other /livekit/ path. Keep the media and timeout sentences. +- scripts/check-deploy.sh: in the python block (lines 329-341) require the location ^~ /livekit/rtc block to contain proxy_pass http://127.0.0.1:{lk_port}/rtc; and each WebSocket directive, require the location ^~ /livekit/ block to contain return 404; and no proxy_pass, and require exactly one proxy_pass to the LiveKit port in the whole file; every mismatch is its own problems line as now. Update the header comment (lines 28-30) to describe the two locations. The production livekit_url check stays wss://demo.summercms.io/livekit. +- scripts/check-live.sh: keep the /livekit/rtc/validate probe; add a probe that POSTs {} as JSON to /livekit/twirp/livekit.RoomService/ListRooms through the get helper (extra curl args go before the URL) and requires 404 (LiveKit itself would answer 401), with its own bad line. Update the header comment (lines 40-42). Until the new vhost is installed on rome this probe fails there; that is expected and documented below. +- DEPLOY.md: the Overview tree (line 19) shows /livekit/rtc to 127.0.0.1:7890 and the rest of /livekit/ as 404; the port table row for 7890 (line 364) says browsers reach only the signal server through nginx /livekit/rtc and the RoomService is reachable by the Go binary on loopback only; lines 510 and 839 name /livekit/rtc; the check-live description (around line 916) adds the 404 for the rest of /livekit/. Add a short pending note in the "When deploy/nginx/demo.summercms.io.conf changed" procedure (or next to the "Applied on rome" record): the /livekit/rtc-only vhost is committed but not applied on rome; installing it needs its own go-ahead; until then check-live.sh reports the RoomService probe. Keep "Disallow: /livekit" in robots. +- README.md lines 280 and 282: check-deploy covers "the LiveKit /rtc-only location", check-live covers "/livekit/rtc/validate from LiveKit, the rest of /livekit/ 404". +- Run scripts/check-deploy.sh (must print check-deploy: ok), bash -n and shellcheck -S error on check-deploy.sh and check-live.sh. Commit "fix(deploy): nginx proxies only LiveKit's /rtc paths" (body: IN-06; the vhost is not installed on rome). + +Commit C, DEPLOY.md VAPID step (IN-08), lines 533-547: +- Keep the build-machine part (source the secrets file, printf the address into $RSH $ROME) and the remote script's single-quoted form. The remote script starts with set -eo pipefail (if rome's login shell lacked pipefail it would stop before writing anything, which is still safe), reads the address, keeps the empty-address guard and the "already in .env" guard, then assigns the output of the herald:vapid-generate --json -q run (same sudo -u summercms-demo SUMMER_ENV=production ./bin/demo.new) to a variable, extracts .public_key and .private_key with jq -r and a // empty fallback into two variables, exits with "herald:vapid-generate returned no key pair" unless both are non-empty, and only then appends all three lines (PUBLIC_KEY, PRIVATE_KEY, SUBJECT=mailto:address) with one printf piped into one sudo tee -a .en[v] >/dev/null. No echo, no set -x, nothing prints a key; the final grep -c check (expects 3) stays. One sentence under the block: a failed generation now writes nothing, so a re-run needs no hand edit. +- Dry-run it in the scratchpad before committing: copy the remote script text into a file, run it with bash against a temp directory holding an .env and a stub bin/demo.new, with a sudo shim on PATH that drops -u USER and runs the rest through env. Check: the stub printing a JSON pair appends exactly three lines; a failing stub, and a stub printing JSON without private_key, both exit non-zero and leave .env byte-identical; a second run stops at "already in .env". Record the outcome in the SUMMARY. +- Commit "docs: the VAPID step runs with pipefail and appends the keys only when both exist" (body: IN-08). + + + cd /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app && bash -n scripts/check-local.sh && bash -n scripts/check-live.sh && bash -n scripts/check-deploy.sh && shellcheck -S error scripts/check-local.sh scripts/check-live.sh scripts/check-deploy.sh && test "$(grep -v '^[[:space:]]*#' scripts/check-local.sh | grep -c 'Authorization')" = 1 && test "$(grep -c -- '--jwt' scripts/check-local.sh)" -ge 27 && grep -q 'curl .*-K -' scripts/check-local.sh && grep -q 'location \^~ /livekit/rtc {' deploy/nginx/demo.summercms.io.conf && grep -q 'proxy_pass http://127.0.0.1:7890/rtc;' deploy/nginx/demo.summercms.io.conf && scripts/check-deploy.sh 2>&1 | tail -1 | grep -qx 'check-deploy: ok' && grep -q 'livekit.RoomService/ListRooms' scripts/check-live.sh && awk '/^The VAPID keys, after/,/^After the release, `GET \/_herald/' DEPLOY.md | grep -q 'set -eo pipefail' && test "$(awk '/^The VAPID keys, after/,/^After the release, `GET \/_herald/' DEPLOY.md | grep -c 'tee -a')" = 1 && go vet ./... && go test -short ./... && git log -3 --format='%h %s' && git status --short + + check-local.sh carries no bearer token in any curl argv (one Authorization line, inside request()'s printf) and the visitor login body comes from a file; nginx proxies only /livekit/rtc to LiveKit and answers 404 for the rest of /livekit/, check-deploy enforces it and passes, check-live probes the RoomService 404; the VAPID step uses pipefail, a variable, a both-keys check and one tee, dry-run evidence recorded; DEPLOY.md and README describe the new shape and the pending vhost go-ahead; three sm-demo-app commits; working tree clean apart from other sessions' files. + + + + + +## Trust Boundaries + +| Boundary | Description | +|----------|-------------| +| internet -> nginx (demo.summercms.io) | anonymous traffic reaches every proxied path, /livekit/* included | +| member JWT -> Go (/_demo/api/v1/chat/members) | any authenticated member triggers the query; the community grows without a reset | +| admin settings -> deploy (demo:seed) | release automation runs against admin-owned settings rows | +| local user -> /proc/*/cmdline | other local users read process arguments of check-local's curl calls | +| build machine -> rome over SSH | the VAPID step writes secrets into /srv/summercms-demo/.env | + +## STRIDE Threat Register + +| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan | +|-----------|----------|-----------|----------|-------------|-----------------| +| T-oc9-01 | D | chat_api.go chatMembers | medium | mitigate | membership as a SQL subquery (OtherMembers), no bound id list; 66000-member test answers 200 (Task 1) | +| T-oc9-02 | T | chat_api.go chatMembers | low | mitigate | the GET writes nothing; handles are assigned at registration and in demo:seed's "member handles" step; test proves the row stays NULL after a GET (Task 1) | +| T-oc9-03 | T | console/seed.go, updates/updates.go | medium | mitigate | lists seeded once by a recorded migration; demo:seed no longer writes golem15_moderation_settings; test proves emptied lists survive demo:seed (Task 2) | +| T-oc9-04 | I | scripts/check-local.sh | low | mitigate | JWTs to curl through a config on stdin (printf builtin), visitor login body from a 0700 temp file; gate: one non-comment Authorization line (Task 3) | +| T-oc9-05 | E | nginx /livekit/ (LiveKit Twirp RoomService) | medium | mitigate | only ^~ /livekit/rtc is proxied; the rest of /livekit/ answers 404; check-deploy enforces, check-live probes ListRooms for 404 (Task 3) | +| T-oc9-06 | I | DEPLOY.md VAPID step | medium | mitigate | keys only in shell variables and one printf piped to sudo tee; no echo or set -x; pipefail plus both-keys check means a failed generation writes nothing (Task 3) | +| T-oc9-07 | T | rome vhost | low | accept | the nginx change is committed, not installed; installation is a separate go-ahead, documented in DEPLOY.md | +| T-oc9-SC | T | package installs | low | accept | no install: gormigrate v2.1.7 is already in go.mod and go.sum, only its indirect marker moves | + + + +- In /media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app: go vet ./... and go test -short ./... green after each of the five commits; the Docker tests TestDemoChatMembers, TestDemoChatMembersManyMembers, TestSeedChat, TestSeedChatWithoutVisitor, TestSeedChatSmoke, TestDemoModeration, TestResetKeepsPlans pass. +- scripts/check-deploy.sh prints check-deploy: ok; shellcheck -S error clean on the three scripts. +- git log shows five new commits (Task 1, Task 2, Task 3 A/B/C), none with a co-author tag; git status shows no leftover sm-demo-app change from this item; nothing pushed or tagged; nothing applied on rome. +- No file outside sm-demo-app changed by the executor (the plan and SUMMARY under summercms.go/.planning/quick are the orchestrator's). + + + +- All five todos' Solution sections are implemented as written, with the discretion choices stated in the objective. +- The members endpoint keeps its response shape (no vue-demo-app or vue-chat-layer change needed) and works past 65535 members. +- An admin's moderation lists are never overwritten by a deploy. +- No bearer token in check-local's curl argv; LiveKit's public surface is signalling only; the VAPID rebuild step cannot leave a half-written .env. + + + +Create `.planning/quick/261105-oc9-sm-demo-app-hardening/261105-oc9-SUMMARY.md` when done, listing the five sm-demo-app commit hashes, the red-test evidence for Tasks 1 and 2, the VAPID dry-run outcome, and that the /livekit/rtc vhost awaits a go-ahead on rome. + diff --git a/.planning/quick/261105-oc9-sm-demo-app-hardening/261105-oc9-SUMMARY.md b/.planning/quick/261105-oc9-sm-demo-app-hardening/261105-oc9-SUMMARY.md new file mode 100644 index 00000000..a8d366b1 --- /dev/null +++ b/.planning/quick/261105-oc9-sm-demo-app-hardening/261105-oc9-SUMMARY.md @@ -0,0 +1,159 @@ +--- +phase: quick-261105-oc9 +plan: 01 +subsystem: sm-demo-app (golem15.demo plugin, deploy files, runbook) +status: complete +tags: [security, hardening, chat, moderation, nginx, livekit, vapid, scripts] +requires: [] +provides: + - "classes.OtherMembers: read-only community member listing with the membership subquery" + - "demo:seed step \"member handles\"" + - "golem15.demo migration 202611050001_seed_golem15_moderation_lists (Plugin implements pact.HasMigrations)" + - "check-local.sh request --jwt (token through curl -K - on stdin)" + - "nginx /livekit/rtc-only vhost (committed, not installed)" + - "VAPID runbook step with pipefail and both-keys guard" +affects: [sm-demo-app] +tech-stack: + added: [] + patterns: + - "Data seeded once via a plugin gormigrate migration instead of an idempotent seed step" + - "Secrets to curl via -K - config on stdin, never argv" +key-files: + created: + - ../sm-demo-app/plugins/demo/classes/members.go + - ../sm-demo-app/plugins/demo/updates/updates.go + modified: + - ../sm-demo-app/plugins/demo/chat_api.go + - ../sm-demo-app/plugins/demo/plugin.go + - ../sm-demo-app/plugins/demo/console/seed.go + - ../sm-demo-app/plugins/demo/console/seed_chat.go + - ../sm-demo-app/plugins/demo/console/seed_friends.go + - ../sm-demo-app/go.mod + - ../sm-demo-app/chat_test.go + - ../sm-demo-app/chat_smoke_test.go + - ../sm-demo-app/seed_chat_test.go + - ../sm-demo-app/README.md + - ../sm-demo-app/DEPLOY.md + - ../sm-demo-app/scripts/check-local.sh + - ../sm-demo-app/scripts/check-moderation.sh + - ../sm-demo-app/scripts/check-deploy.sh + - ../sm-demo-app/scripts/check-live.sh + - ../sm-demo-app/deploy/nginx/demo.summercms.io.conf +decisions: + - "Default moderation lists live in a golem15.demo gormigrate migration (recorded in summer_migrations_golem15_demo); demo:seed no longer touches golem15_moderation_settings" + - "OtherMembers lives in a new file classes/members.go so chat_context.go (edited by 261105-oc6) stays untouched" + - "The member handles seed step covers every user without a handle, not only members" + - "check-local.sh's visitor login body also leaves argv (file in the 0700 temp dir)" +metrics: + duration: "~20 min" + completed: 2026-11-05 +actuals: + tokens: 15396 + tasks: 3 + commits: 5 +plan_head_before: fbfdcf6 +plan_head_after: 054cda58485d9d7d96be794cf19f7af812050ef2 +--- + +# Quick 261105-oc9: sm-demo-app hardening Summary + +GET /_demo/api/v1/chat/members is now read-only and selects members through a SQL subquery (it answers 200 for 66000 members). The default moderation lists are written once, by a golem15.demo migration. check-local.sh passes JWTs to curl through a config on stdin. The nginx vhost proxies only LiveKit's /rtc. The VAPID runbook step uses pipefail and appends the keys only when both exist. + +All work is in `/media/nvme/dev/golem15/summercms.io/summercms/sm-demo-app` on `master`, starting from `fbfdcf6`. Nothing was pushed, tagged, deployed or installed on rome, and no ssh was used. + +## Commits (sm-demo-app) + +| # | Todo | Hash | Message | +|---|------|------|---------| +| 1 | IN-09 demo-chat-members-read-only | `9595606` | fix: GET /_demo/api/v1/chat/members reads only and selects members with a subquery | +| 2 | IN-05 demo-seed-moderation-lists-once | `5630d92` | fix: seed the default moderation lists once, in a golem15.demo migration | +| 3A | IN-03 check-local-jwt-out-of-argv | `55620dc` | test: check-local passes member JWTs to curl through a config on stdin | +| 3B | IN-06 demo-nginx-livekit-rtc-only | `6c52514` | fix(deploy): nginx proxies only LiveKit's /rtc paths | +| 3C | IN-08 demo-vapid-runbook-pipefail | `054cda5` | docs: the VAPID step runs with pipefail and appends the keys only when both exist | + +None of the commits has a co-author tag (`git log fbfdcf6..HEAD | grep -ci co-authored` returns 0). + +## Task 1: members endpoint is read-only (IN-09) + +- New `plugins/demo/classes/members.go`: `OtherMembers(ctx, db, exceptID)` selects id, name, surname and handle with `users.id <> ?` plus the unexported `memberWhere`, ordered by id. It never writes. `chat_context.go` is not touched. +- `chatMembers` makes one `OtherMembers` call. The `MemberIDs` id list and the `AssignHandle` call inside the GET are gone, along with the now-unused user plugin imports. The response shape is unchanged. +- New demo:seed step `member handles`, placed right after `chat personas`, runs `AssignHandle` for every user whose handle is NULL or blank. It reports `updated` when it assigned at least one handle and `exists` otherwise. The SeedCommand description mentions it. +- README: the members row, the Seed list and the Testing paragraph now mention `TestDemoChatMembersManyMembers`. + +**RED evidence (before the implementation):** +- `TestDemoChatMembers`: "the member without a handle is listed with \"member\", want empty"; "the GET wrote handles: 1 of the caller and the other member are still NULL, want 2" +- `TestDemoChatMembersManyMembers`: `members = 500 {"error":true,"message":"Internal server error"}` (bind parameter limit) +- `TestSeedChat` / `TestSeedChatWithoutVisitor`: "demo:seed printed no step \"member handles\"" + +**GREEN:** these tests pass, and so do TestDemoDMs, TestDemoChatBootstrap, vet and the short tests. + +## Task 2: moderation lists seeded once (IN-05) + +- New `plugins/demo/updates/updates.go` holds `defaultBannedWords` and `defaultBannedLinks` (moved here, same values) and the migration `202611050001_seed_golem15_moderation_lists`. Its UPDATE statement is the same one the seed step ran, and its Rollback is a no-op. `All()` returns it. +- `plugin.go` asserts `pact.HasMigrations` and adds `Migrations()`, which returns `updates.All()`. The package doc now says the plugin has one migration. +- `seedModerationLists` and the `moderation lists` step are removed, and the description no longer mentions them. No non-comment line in the console package refers to `golem15_moderation_settings` any more. +- `go.mod`: I moved gormigrate v2.1.7 by hand into the direct require block. I did not run `go mod tidy`, and go.sum is unchanged. +- I updated the `check-moderation.sh` header comment and the README's Seed list and Moderation paragraph. + +**RED evidence:** `TestSeedChat` failed with "the migrated database has no default moderation lists" and `relation "summer_migrations_golem15_demo" does not exist`. + +**GREEN:** TestSeedChat passes, including the new check that lists emptied to `''` and to `NULL` stay empty across two demo:seed runs. TestSeedChatSmoke (the lists are filled by the migration, and the settings fingerprint survives demo:reset), TestDemoModeration, TestResetKeepsPlans, TestBootAllPlugins and TestGoModRequiresTags also pass. + +## Task 3 + +**A, check-local (IN-03).** The new signature is `request [--jwt JWT] METHOD URL [curl args...]`. With a JWT, the function pipes `printf 'header = "Authorization: Bearer %s"\n'` into `curl -K -`. Without one, it behaves as before. All 27 authenticated calls (15 literal, 12 through the `auth` locals) now use `--jwt`, and the `auth` locals are gone. The visitor login JSON is written to `$TMP/login.json` and sent with `-d @file`. Only one non-comment `Authorization` line remains. +- I checked request() against a throwaway local echo server. The header arrives with `--jwt`, there is no header without it or with an empty JWT, `-d @file` bodies arrive, and an unreachable URL gives the same fallback as before. The local stack was stopped (`scripts/dev.sh status`), so I did not do the optional full `check-local.sh` run. + +**B, LiveKit /rtc only (IN-06).** The vhost now has `location ^~ /livekit/rtc`, which proxies to `http://127.0.0.1:7890/rtc` with all the WebSocket and timeout directives, and `location ^~ /livekit/`, which only does `return 404;`. +- check-deploy.sh enforces the /rtc block and its directives, a 404 block with no proxy_pass, and exactly one proxy_pass to the LiveKit port. It prints `check-deploy: ok`. As a negative check, I temporarily put the catch-all proxy back, and it produced three separate problem lines and failed. Then I restored the file. +- check-live.sh gains a probe: `POST /livekit/twirp/livekit.RoomService/ListRooms` must return 404. +- DEPLOY.md: I updated the overview tree, the 7890 port row, the chat-family list, the first-release note and the check-live description. I also added a note, **"Pending on rome: the `/livekit/rtc`-only vhost"**, after the "When `deploy/nginx/demo.summercms.io.conf` changed" procedure. README lines 280 and 282 are updated. + +**C, VAPID step (IN-08).** The step now uses `set -eo pipefail`. The pair is generated into `pair`, and `jq -r '.x // empty'` fills `pub` and `priv`. The step exits with "herald:vapid-generate returned no key pair" unless both are non-empty. It then appends all three lines with one `printf | sudo tee -a`, and the final `grep -c` check (expects 3) is kept. A sentence below the block explains that a failed generation writes nothing. + +**VAPID dry-run** (scratchpad, the remote script extracted from DEPLOY.md, a `sudo` shim that drops `-u USER` and runs `env "$@"`, stub `bin/demo.new`). Because a hook blocks reading files named `.env`, the copy under test wrote to `envfile` instead of `.en[v]`. The logic is otherwise identical. + +| Case | Exit | Output | Env file | +|------|------|--------|----------| +| stub prints a JSON pair | 0 | `3` | exactly 3 lines appended (PUBLIC_KEY, PRIVATE_KEY, SUBJECT=mailto:) | +| stub fails (exit 3, partial JSON) | 3 | none | byte-identical | +| JSON without private_key | 1 | "herald:vapid-generate returned no key pair" | byte-identical | +| non-JSON output | 5 (jq) | jq parse error | byte-identical | +| second run after success | 1 | "VAPID keys are already in .env" | byte-identical | +| empty address | 1 | "DEMO_ADMIN_EMAIL is empty" | unchanged | + +No case printed a key. + +## Verification + +- At each commit, gofmt is clean, `go vet ./...` passes and `go test -short ./...` is green. +- Final `go test -count=1 ./...` with Docker: every package is ok, root package 51s. +- `scripts/check-deploy.sh` prints `check-deploy: ok`. `shellcheck -S error` is clean on check-local.sh, check-live.sh, check-deploy.sh and check-moderation.sh. +- `scripts/check-pins.sh` reports only the chatchannels (`a042536`) and chatvideo (`11a9607`) submodules ahead of their go.mod pins. That was accepted up front and is not addressed here. +- The sm-demo-app working tree was clean after the last commit. + +## Deviations from Plan + +**1. [Rule 3 - Blocking] VAPID dry-run target file renamed.** A PreToolUse secret-read guard blocks any Bash command that references a path matching `.env`. The extracted copy under test therefore replaced `.en[v]` with `envfile`. The committed DEPLOY.md text is unchanged by this. + +Otherwise the plan was executed as written. + +## Follow-ups + +- **The `/livekit/rtc`-only vhost is waiting for a go-ahead to install on rome.** It is committed only. Until it is installed, `scripts/check-live.sh https://demo.summercms.io` will report the RoomService probe (LiveKit answers 401, not 404). The install steps are in DEPLOY.md, "When `deploy/nginx/demo.summercms.io.conf` changed". +- The orchestrator's closing docs step still has to do these: move the five todos (demo-chat-members-read-only, demo-seed-moderation-lists-once, check-local-jwt-out-of-argv, demo-nginx-livekit-rtc-only, demo-vapid-runbook-pipefail) to done with a "## Closed" note, and sync g15office. +- On rome's existing database, the next `summer migrate` records the new golem15.demo migration. Since the lists there are already seeded, the UPDATE changes nothing. +- Pre-existing, not changed: check-local.sh's `request` prints `000000` for an unreachable URL (curl's own `000` plus the `|| echo 000` fallback). Callers compare against specific codes, so this is harmless. + +## Known Stubs + +None. + +## Threat Flags + +None. No new surface: the changes shrink the public LiveKit surface and remove a write from a GET. + +## Self-Check: PASSED + +- FOUND: plugins/demo/classes/members.go, plugins/demo/updates/updates.go +- FOUND commits: 9595606, 5630d92, 55620dc, 6c52514, 054cda5 (`git rev-list --count fbfdcf6..HEAD` = 5) diff --git a/.planning/todos/pending/2026-10-26-activate-by-code-second-factor.md b/.planning/todos/done/2026-10-26-activate-by-code-second-factor.md similarity index 92% rename from .planning/todos/pending/2026-10-26-activate-by-code-second-factor.md rename to .planning/todos/done/2026-10-26-activate-by-code-second-factor.md index d2069265..e2785fef 100644 --- a/.planning/todos/pending/2026-10-26-activate-by-code-second-factor.md +++ b/.planning/todos/done/2026-10-26-activate-by-code-second-factor.md @@ -17,3 +17,7 @@ To fix later: - Add a test in sm-user-plugin (a gate that challenges, then activate-by-code answers the challenge) and an end-to-end case in sm-mfa-plugin's `TestEveryLoginPathIsGated`, and update its `tokenMints` reason. - Release sm-user-plugin as a patch and bump sm-demo-app, sm-bm-app and fonoteka.go in separate commits (D-17 order). - Until then, `TestEveryLoginPathIsGated/mint_sites` lists `ActivateByCode` with a pointer to F-1, so any change to the mint sites fails until the list is updated. + +## Closed 2026-11-05 + +Fixed in quick 261105-oc5: sm-user-plugin b179e6b (activate-by-code asks the login gates). Untagged; release sm-user-plugin v0.1.20 and bump the apps. diff --git a/.planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md b/.planning/todos/done/2026-10-26-mfa-factor-changes-need-reauth.md similarity index 85% rename from .planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md rename to .planning/todos/done/2026-10-26-mfa-factor-changes-need-reauth.md index 3b591bdb..977fc147 100644 --- a/.planning/todos/pending/2026-10-26-mfa-factor-changes-need-reauth.md +++ b/.planning/todos/done/2026-10-26-mfa-factor-changes-need-reauth.md @@ -14,3 +14,7 @@ Full fix (a contract change, user deferred it on 2026-10-26): - Require a `password` field (or a current factor) on those routes, refused with the existing 403 `{"error":"Password confirmation required."}` and counted in the login throttle like the recovery-code routes (WR-06, 0a08070). - Update the frontends that call them (sm-demo-app's vue-demo-app `pages/account/security.vue` and any app frontend) to ask for the password. - Release sm-mfa-plugin as a minor/patch together with the frontend change, and bump the apps in separate commits. + +## Closed 2026-11-05 + +Fixed in quick 261105-oc5: sm-mfa-plugin 3aa52c6 (golem15.mfa.factor_changes_require_password, default true), vue-demo-app 848ee1e, sm-demo-app 7b75d2c. Untagged; release sm-mfa-plugin v0.2.0. diff --git a/.planning/todos/pending/2026-10-28-attach-static-handler-disposition.md b/.planning/todos/done/2026-10-28-attach-static-handler-disposition.md similarity index 86% rename from .planning/todos/pending/2026-10-28-attach-static-handler-disposition.md rename to .planning/todos/done/2026-10-28-attach-static-handler-disposition.md index b10a5fda..dbbf6123 100644 --- a/.planning/todos/pending/2026-10-28-attach-static-handler-disposition.md +++ b/.planning/todos/done/2026-10-28-attach-static-handler-disposition.md @@ -19,3 +19,7 @@ Follow-up from quick 261028-rqe (ba8432c, 4957e5c). `attach.Store` saves `Conten - Make both handlers send the stored disposition (and `X-Content-Type-Options: nosniff`), additive. - Blob writers that bypass `attach.Store` can adopt `attach.IsInlineContentType`: fonoteka.go `storeUpload`/`StorePublicFile` (see `2026-10-28-fonoteka-uploads-use-disk-extension.md`), the sm-user-plugin avatar writer, the sm-feedback-plugin screenshot writer. - Adding the disposition to objects already in a bucket stays an operator task (documented in `docs/services/storage.md#deploy-note`). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc8: summercms.go 27c98bf. Writers that bypass attach.Store (fonoteka.go, sm-user avatar, sm-feedback screenshot) remain follow-ups. diff --git a/.planning/todos/pending/chatchannels-pin-policy.md b/.planning/todos/done/chatchannels-pin-policy.md similarity index 87% rename from .planning/todos/pending/chatchannels-pin-policy.md rename to .planning/todos/done/chatchannels-pin-policy.md index 838722de..4df916ec 100644 --- a/.planning/todos/pending/chatchannels-pin-policy.md +++ b/.planning/todos/done/chatchannels-pin-policy.md @@ -19,3 +19,7 @@ chatchannels gates PinMessage and UnpinMessage only with channel access, which t Additively in chatchannels: PinMessage and UnpinMessage require CanManageContext, or an optional contracts.PinPolicy the host implements; the demo answers it from its moderator group. Verify message.conversation_id equals the channel's conversation before creating the pin. Meanwhile the demo layer can hide pin actions for non-moderators, but the server must enforce the rule. Source: phase 20 code review (20-REVIEW.md WR-02). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc6: sm-chatchannels-plugin a042536, sm-demo-app ce800f2/7e4b9d3/11d3afc. Untagged; sm-bm-app needs CanPin before it bumps. diff --git a/.planning/todos/pending/chatvideo-dm-only-host-hook.md b/.planning/todos/done/chatvideo-dm-only-host-hook.md similarity index 86% rename from .planning/todos/pending/chatvideo-dm-only-host-hook.md rename to .planning/todos/done/chatvideo-dm-only-host-hook.md index e2ca24da..ad35ba6d 100644 --- a/.planning/todos/pending/chatvideo-dm-only-host-hook.md +++ b/.planning/todos/done/chatvideo-dm-only-host-hook.md @@ -18,3 +18,7 @@ The server has no DM-only rule; channel calls are refused today only because the Add an optional host hook in chatvideo (for example CanStartCall(conversationType)), or check conversation.type == dm in the demo, so the rule does not depend on seed data. Source: phase 20 code review (20-REVIEW.md IN-01). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc7: sm-chatvideo-plugin 11a9607 (contracts.CallPolicy, DMOnly), sm-demo-app fbfdcf6. diff --git a/.planning/todos/pending/chatvideo-incoming-handle-leak.md b/.planning/todos/done/chatvideo-incoming-handle-leak.md similarity index 89% rename from .planning/todos/pending/chatvideo-incoming-handle-leak.md rename to .planning/todos/done/chatvideo-incoming-handle-leak.md index c895aec6..4f068343 100644 --- a/.planning/todos/pending/chatvideo-incoming-handle-leak.md +++ b/.planning/todos/done/chatvideo-incoming-handle-leak.md @@ -17,3 +17,7 @@ Two video:call:incoming events for the same session, both pending in subscribe, Keep a sequence number per watchSession and close the handle unless it is the latest. Source: phase 20 code review (20-REVIEW.md IN-07). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc7: vue-chatvideo-layer 6d5784d. Untagged. diff --git a/.planning/todos/pending/chatvideo-live-membership.md b/.planning/todos/done/chatvideo-live-membership.md similarity index 90% rename from .planning/todos/pending/chatvideo-live-membership.md rename to .planning/todos/done/chatvideo-live-membership.md index 4aa0a058..d30c13a1 100644 --- a/.planning/todos/pending/chatvideo-live-membership.md +++ b/.planning/todos/done/chatvideo-live-membership.md @@ -18,3 +18,7 @@ chatvideo checks participant rows only. A ban revokes the user's JWTs, but an al Have chatvideo honour LiveMembership (through chat's Service.IsParticipant) in requireParticipant and the authorizer, and remove the participant from the LiveKit room on a ban. Source: phase 20 code review (20-REVIEW.md IN-02). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc7: sm-chatvideo-plugin f996156. Untagged. diff --git a/.planning/todos/pending/check-local-jwt-out-of-argv.md b/.planning/todos/done/check-local-jwt-out-of-argv.md similarity index 91% rename from .planning/todos/pending/check-local-jwt-out-of-argv.md rename to .planning/todos/done/check-local-jwt-out-of-argv.md index 4b0bfdb8..7969d558 100644 --- a/.planning/todos/pending/check-local-jwt-out-of-argv.md +++ b/.planning/todos/done/check-local-jwt-out-of-argv.md @@ -17,3 +17,7 @@ check-local still puts member JWTs into -H "Authorization: Bearer ..." arguments Give request() an optional JWT that it writes as header = "Authorization: Bearer ..." into a curl config on stdin (curl -K -), as check-moderation.sh does, and drop every -H Authorization argument. Source: phase 20 code review (20-REVIEW.md IN-03). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc9: sm-demo-app 55620dc. diff --git a/.planning/todos/pending/demo-chat-members-read-only.md b/.planning/todos/done/demo-chat-members-read-only.md similarity index 91% rename from .planning/todos/pending/demo-chat-members-read-only.md rename to .planning/todos/done/demo-chat-members-read-only.md index 24c252bb..00ce4d79 100644 --- a/.planning/todos/pending/demo-chat-members-read-only.md +++ b/.planning/todos/done/demo-chat-members-read-only.md @@ -17,3 +17,7 @@ The GET assigns handles to members that lack one (two concurrent first loads rac Select with the memberWhere subquery (WHERE id <> ? AND ) instead of an id list, and assign missing handles at registration or in demo:seed, not in a GET. Source: phase 20 code review (20-REVIEW.md IN-09). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc9: sm-demo-app 9595606. diff --git a/.planning/todos/pending/demo-nginx-livekit-rtc-only.md b/.planning/todos/done/demo-nginx-livekit-rtc-only.md similarity index 85% rename from .planning/todos/pending/demo-nginx-livekit-rtc-only.md rename to .planning/todos/done/demo-nginx-livekit-rtc-only.md index 68a55c70..62564c07 100644 --- a/.planning/todos/pending/demo-nginx-livekit-rtc-only.md +++ b/.planning/todos/done/demo-nginx-livekit-rtc-only.md @@ -18,3 +18,7 @@ Browsers need only /rtc and /rtc/validate, but the /livekit/ location also expos Use location ^~ /livekit/rtc { proxy_pass http://127.0.0.1:7890/rtc; ... } and answer 404 for the rest of /livekit/; adjust check-deploy and check-live. A vhost change on rome needs its own go-ahead. Source: phase 20 code review (20-REVIEW.md IN-06). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc9: sm-demo-app 6c52514. Committed only; installing the vhost on rome needs its own go-ahead. diff --git a/.planning/todos/pending/demo-seed-moderation-lists-once.md b/.planning/todos/done/demo-seed-moderation-lists-once.md similarity index 86% rename from .planning/todos/pending/demo-seed-moderation-lists-once.md rename to .planning/todos/done/demo-seed-moderation-lists-once.md index cf85cd09..b56f3a29 100644 --- a/.planning/todos/pending/demo-seed-moderation-lists-once.md +++ b/.planning/todos/done/demo-seed-moderation-lists-once.md @@ -17,3 +17,7 @@ seedModerationLists refills both lists whenever both are empty, and every releas Seed only once: insert the lists when the settings row is created, or record a seeded flag. Source: phase 20 code review (20-REVIEW.md IN-05). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc9: sm-demo-app 5630d92 (golem15.demo migration). diff --git a/.planning/todos/pending/demo-vapid-runbook-pipefail.md b/.planning/todos/done/demo-vapid-runbook-pipefail.md similarity index 91% rename from .planning/todos/pending/demo-vapid-runbook-pipefail.md rename to .planning/todos/done/demo-vapid-runbook-pipefail.md index 7fcd42ac..f9b6a7c2 100644 --- a/.planning/todos/pending/demo-vapid-runbook-pipefail.md +++ b/.planning/todos/done/demo-vapid-runbook-pipefail.md @@ -17,3 +17,7 @@ The remote script uses set -e without pipefail. If herald:vapid-generate fails, Use set -eo pipefail, generate into a variable first, check both keys are non-empty, then append all three lines in one tee. Source: phase 20 code review (20-REVIEW.md IN-08). + +## Closed 2026-11-05 + +Fixed in quick 261105-oc9: sm-demo-app 054cda5.