docs(19): plan gap closure for DM send IDOR and GDPR CASCADE
Two gap-closure plans close CR-01 participant-only send and CR-02 created_by reassignment. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Jakub Zych
@@ -1198,7 +1198,7 @@ Plans:
|
||||
3. Golden tests show that the serializers (`Message.toApiArray` with attachments, thread summaries, channel, category, pref, pin and mention arrays) and every realtime event (name, channel and payload) match PHP output captured from Wavepath. Route-level parity of `/_wave` is checked at the Wavepath port.
|
||||
4. The new code has unit tests, delivered in the phase's last plan.
|
||||
|
||||
**Plans:** 5/5 plans executed
|
||||
**Plans:** 5/7 plans executed
|
||||
|
||||
Plans:
|
||||
**Wave 1**
|
||||
@@ -1214,6 +1214,12 @@ Plans:
|
||||
**Wave 4** *(blocked on Wave 3)*
|
||||
- [x] 19-05-PLAN.md — Unit tests LAST: goldens, PHP method map (59 chat + 106 chatchannels), scripts/check-phase19.sh
|
||||
|
||||
**Wave 5** *(gap closure)*
|
||||
- [ ] 19-06-PLAN.md — Participant-only canSend (CR-01) and created_by reassignment on user.Deleting (CR-02)
|
||||
|
||||
**Wave 6** *(gap closure, tests last)*
|
||||
- [ ] 19-07-PLAN.md — TestTeammateDMWriteIDOR, TestDeletingCascade creator path, check-phase19.sh names
|
||||
|
||||
### Phase 19.1: Chat satellites
|
||||
|
||||
**Goal:** The remaining shared plugins that Wavepath's app plugin requires run on SummerCMS: chatbots, chatvideo, forum, herald and userfriends.
|
||||
|
||||
239
.planning/phases/19-chat-and-chatchannels/19-06-PLAN.md
Normal file
239
.planning/phases/19-chat-and-chatchannels/19-06-PLAN.md
Normal file
@@ -0,0 +1,239 @@
|
||||
---
|
||||
phase: 19-chat-and-chatchannels
|
||||
plan: 06
|
||||
type: execute
|
||||
wave: 5
|
||||
depends_on: ["19-05"]
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- ../sm-chat-plugin/classes/service.go
|
||||
- ../sm-chat-plugin/boot.go
|
||||
- ../sm-chat-plugin/gap_closure_test.go
|
||||
- ../sm-chat-plugin/README.md
|
||||
- ../sm-chatchannels-plugin/README.md
|
||||
autonomous: true
|
||||
requirements: [SC-19-1, D-08, D-13]
|
||||
estimate:
|
||||
tokens: 140000
|
||||
raw_tokens: 140000
|
||||
tasks: 3
|
||||
confidence: low
|
||||
must_haves:
|
||||
truths:
|
||||
- "D-08 / SC-19-1: conversation- and message-level routes check participation on every call; ChatService.sendMessage ports 1:1 (PHP requires hasParticipant). canSend returns only whether the sender is a participant; a third context member POSTing to a two-person DM conversation id is refused as ErrNotParticipant (HTTP 404). GetOrCreateDm still creates a two-person DM. GetGroupChat still lazy-joins via addParticipant so a group member can join then send. Context-level GetGroupChat / GetOrCreateDm still refuse when ChatEnabled is false."
|
||||
- "D-13 GDPR: deleting a user severs that user from channels without CASCADE-wiping conversations remaining members still use. Before the user row is deleted, chat's user.Deleting handler reassigns golem15_chat_conversations.created_by to another remaining participant on every conversation that user created (channels AND DMs). Remaining members keep the conversation row and, for a channel, the channel row. If no other participant remains, CASCADE wipe of an empty conversation is acceptable. The IF NOT EXISTS baseline is not ALTER'd; PHP constraint name golem15_chat_conversations_created_by_foreign and ON DELETE CASCADE stay as shipped."
|
||||
- "WR-01, WR-02, WR-03, WR-04, IN-01 and IN-02 stay open in this gap round; this plan does not change search scoping, JoinChannel, PinMessage, Content-Disposition, addParticipant error handling, or FormAfterDelete fire sites."
|
||||
artifacts:
|
||||
- path: "../sm-chat-plugin/classes/service.go"
|
||||
provides: "participant-only canSend (D-08 / CR-01) and ReassignCreatedBy / WithDB for user.Deleting (D-13 / CR-02)"
|
||||
contains: "isParticipant"
|
||||
- path: "../sm-chat-plugin/boot.go"
|
||||
provides: "chat-owned user.Deleting listener so DMs reassign too"
|
||||
contains: "DeletingEvent"
|
||||
- path: "../sm-chat-plugin/gap_closure_test.go"
|
||||
provides: "19-06 smoke of both post-fix behaviours (not a check-phase19.sh required name)"
|
||||
contains: "TestGapClosureSmoke"
|
||||
key_links:
|
||||
- from: "../sm-chat-plugin/classes/service.go"
|
||||
to: "../sm-chat-plugin/classes/service.go"
|
||||
via: "canSend returns s.isParticipant; SendMessage still maps a false result to ErrNotParticipant"
|
||||
pattern: "return s.isParticipant"
|
||||
- from: "../sm-chat-plugin/boot.go"
|
||||
to: "../fonoteka.go/plugins/golem15/user/classes/events.go"
|
||||
via: "Listen user.Deleting then ReassignCreatedBy on the event Tx"
|
||||
pattern: "DeletingEvent"
|
||||
- from: "../sm-chat-plugin/api/errors.go"
|
||||
to: "../sm-chat-plugin/classes/service.go"
|
||||
via: "ErrNotParticipant already maps to HTTP 404; do not change the mapping"
|
||||
pattern: "ErrNotParticipant"
|
||||
prohibitions:
|
||||
- statement: "Do not ALTER the IF NOT EXISTS chat or chatchannels baseline; do not add a new gormigrate ID; keep PHP constraint names including golem15_chat_conversations_created_by_foreign ON DELETE CASCADE."
|
||||
status: unresolved
|
||||
verification: grep
|
||||
- statement: "Do not push any remote (sm-chat-plugin, sm-chatchannels-plugin, sm-user-plugin, summercms.go, fonoteka.go). Commits in the two plugin repos are allowed after verify. Do not bump fonoteka submodule pointers. Do not force-push sm-user-plugin. Do not edit summercms.go modules."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "WR-01, WR-02, WR-03, WR-04, IN-01 and IN-02 stay open. Do not change JoinChannel, PinMessage, search.go, download Content-Disposition, GetGroupChat ignoring addParticipant errors, or user FormAfterDelete."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "Named postgres tests TestTeammateDMWriteIDOR / TestDeletingCascade creator-delete assertions and scripts/check-phase19.sh list updates belong in 19-07, not this plan."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "Production sources never contain the four D-02 tokens. No /_wave routes. No new third-party require."
|
||||
status: unresolved
|
||||
verification: grep
|
||||
---
|
||||
|
||||
## Phase Goal
|
||||
|
||||
**As a** application developer, **I want to** have Chat and ChatChannels run on SummerCMS as shared plugins with realtime through lighthouse, **so that** they are ready for the Wavepath port.
|
||||
|
||||
This plan's slice (plan 6 of 7, gap closure, wave 5): production fixes for CR-01 (participant-only send) and CR-02 (reassign `created_by` on `user.Deleting`) so Wavepath can thin-alias without a DM write-IDOR or a GDPR CASCADE wipe.
|
||||
|
||||
## Gap being closed (19-VERIFICATION.md `gaps`)
|
||||
|
||||
1. CR-01 / D-08 / SC-19-1 — `canSend` in `../sm-chat-plugin/classes/service.go` currently counts participants then falls back to the host context. `GetOrCreateDm` stores that host alias on the DM row, so any teammate can POST into another pair's DM. PHP `ChatService::sendMessage` requires `$conversation->hasParticipant($sender)` with no host-member fallback. Missing: participation-only `canSend`; keep `GetGroupChat` lazy-join via `addParticipant`.
|
||||
2. CR-02 / D-13 GDPR — `golem15_chat_conversations.created_by` is NOT NULL ON DELETE CASCADE. `SeverUserFromContext` removes members/prefs/participants and does not reassign `created_by`. Chat has no `user.Deleting` listener. Deleting the creator CASCADE-wipes the conversation and then the channel. Missing: before the user row is deleted, reassign `created_by` to another remaining participant on every conversation that user created (channels AND DMs). Prefer a chat-owned listener so DMs survive too.
|
||||
|
||||
Deferred VERIFICATION items (`/_wave` parity, Vue/satellites/`chat:migrate-attachments`) are not this round. WR-01..WR-04 and IN-01..IN-02 stay open.
|
||||
|
||||
## Spec-less probe fallback
|
||||
|
||||
Skipped visibly: see 19-01. `must_haves` derive from the two VERIFICATION.md gaps (D-08 / SC-19-1 and D-13 GDPR).
|
||||
|
||||
## Decisions taken in this plan (Claude's discretion)
|
||||
|
||||
- Chat owns `user.Deleting` reassignment (`ReassignCreatedBy`) so DMs survive even when chatchannels is not mounted. chatchannels keeps `SeverUserFromContext` as shipped; it does not call into chat for this. Festival order either way is correct: the next `created_by` is a participant whose `user_id` is not the deleting user.
|
||||
- `canSend` becomes participation-only. Host `ChatEnabled` stays on context-level `GetGroupChat` and `GetOrCreateDm` (and their HTTP doors). Do not fold `ChatEnabled` into `canSend`.
|
||||
- `GetGroupChat` keeps `_ = addParticipant(...)` on an existing group (IN-01 stays open).
|
||||
- Smoke is `TestGapClosureSmoke` in sm-chat-plugin (postgres, skipped under `-short`). It is not added to `scripts/check-phase19.sh`. Named HTTP/channel cases and the gate list are 19-07.
|
||||
- No new packages. Analog `ChannelService.WithDB` for binding the deleting event Tx.
|
||||
|
||||
## Artifacts this phase produces (this plan)
|
||||
|
||||
- Participant-only `canSend` and chat-owned `user.Deleting` reassignment in sm-chat-plugin.
|
||||
- `TestGapClosureSmoke` proving both post-fix behaviours on one path.
|
||||
- README notes for send rights and GDPR reassignment.
|
||||
- Commits in the plugin repos; no remotes pushed.
|
||||
|
||||
<objective>
|
||||
Close CR-01 and CR-02 in production code (D-08 / SC-19-1 and D-13 GDPR) with a tracer smoke of both post-fix behaviours.
|
||||
|
||||
Purpose: Wavepath clients that assume PHP participant-only send and remaining members keeping a shared channel cannot ship on the current tree.
|
||||
Output: canSend is participation-only; created_by is reassigned on user.Deleting; TestGapClosureSmoke is green on a Docker host; plugin repos committed, not pushed.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/19-chat-and-chatchannels/19-CONTEXT.md
|
||||
@.planning/phases/19-chat-and-chatchannels/19-VERIFICATION.md
|
||||
@.planning/phases/19-chat-and-chatchannels/19-REVIEW.md
|
||||
@.planning/phases/19-chat-and-chatchannels/19-02-SUMMARY.md
|
||||
@.planning/phases/19-chat-and-chatchannels/19-04-SUMMARY.md
|
||||
|
||||
Concurrency: other sessions may edit STATE.md. Do not stage those. Do not edit modules/lighthouse, attach, beachcomber, festival or conga. Stage by explicit path. No co-author tags. Do not push. Do not rewrite 19-01..19-05.
|
||||
|
||||
<interfaces>
|
||||
- ../sm-chat-plugin/classes/service.go canSend (today: participant count, then host ChatEnabled + host membership), SendMessage (false canSend -> ErrNotParticipant), GetOrCreateDm (two participants, host ChatEnabled), isParticipant, addParticipant
|
||||
- ../sm-chat-plugin/classes/writes.go GetGroupChat (host ChatEnabled + host membership; on an existing group calls addParticipant and ignores the error)
|
||||
- ../sm-chat-plugin/api/errors.go writeChatError maps ErrNotParticipant to 404
|
||||
- ../sm-chat-plugin/boot.go registerBootStep; Plugin.service; analog ../sm-chatchannels-plugin/boot.go bootListeners + WithDB + Listen[*userclasses.DeletingEvent]
|
||||
- ../sm-chatchannels-plugin/classes/channel_service.go WithDB and SeverUserFromContext (do not add created_by writes there)
|
||||
- ../fonoteka.go/plugins/golem15/user/classes/events.go DeletingEvent {User, Tx}; DeletingEventName user.Deleting
|
||||
- ../fonoteka.go/plugins/golem15/user/controllers/users_admin_controller.go FormAfterDelete: FireDeletingEvent then Unscoped delete
|
||||
- ../sm-chat-plugin/updates/202610160001_create_golem15_chat_baseline.go CONSTRAINT golem15_chat_conversations_created_by_foreign REFERENCES users (id) ON DELETE CASCADE
|
||||
- ../sm-chat-plugin/harness_test.go chatEnv, user, team, conversation, gdb, svc; fixture teamsAlias
|
||||
- Frozen PHP ChatService::sendMessage requires hasParticipant (D-01 tree at 943b712)
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer">
|
||||
<name>Task 1: End-to-end CR-01 and CR-02 — participant-only send plus created_by reassignment</name>
|
||||
<files>../sm-chat-plugin/classes/service.go, ../sm-chat-plugin/boot.go, ../sm-chat-plugin/gap_closure_test.go</files>
|
||||
<read_first>../sm-chat-plugin/classes/service.go, ../sm-chat-plugin/classes/writes.go, ../sm-chat-plugin/boot.go, ../sm-chat-plugin/api/errors.go, ../sm-chat-plugin/harness_test.go, ../sm-chatchannels-plugin/boot.go, ../sm-chatchannels-plugin/classes/channel_service.go, ../fonoteka.go/plugins/golem15/user/classes/events.go, ../fonoteka.go/plugins/golem15/user/controllers/users_admin_controller.go, ../sm-chat-plugin/updates/202610160001_create_golem15_chat_baseline.go</read_first>
|
||||
<action>Per D-08 / SC-19-1 (CR-01) and D-13 (CR-02), land both production fixes on one path and prove them with one postgres smoke. Do not edit check-phase19.sh. Do not alter any updates/*.go SQL.
|
||||
|
||||
CR-01: change canSend so send rights are participation only. After the nil/zero-id guards, return s.isParticipant(ctx, conv.ID, senderID). Do not load the conversation host from canSend. SendMessage already maps a false result to ErrNotParticipant; leave that mapping and api/errors.go 404 mapping alone. Keep GetOrCreateDm creating exactly the actor and the other user as participants, still refusing when the host is missing or ChatEnabled is false. Keep GetGroupChat lazy-join: on an existing group conversation it still calls addParticipant (IN-01 stays: continue to ignore that call's error). Do not drop ChatEnabled checks on GetGroupChat or GetOrCreateDm.
|
||||
|
||||
CR-02: add a chat-owned user.Deleting handler so DMs survive too. Export (*Service).WithDB analog to ChannelService.WithDB (clone, bind db). Export (*Service).ReassignCreatedBy(ctx, userID): load every golem15_chat_conversations row whose created_by is userID; for each, pick one other participant (user_id <> userID, limit 1); if none, skip (empty conversation may CASCADE when the user row goes); else Update created_by to that participant. Use s.db so the event Tx is the session. In boot.go, register a listeners step after service (chat already Requires golem15.user). Listen[*userclasses.DeletingEvent] like chatchannels: nil-safe, bind WithDB(ev.Tx) when Tx is set, call ReassignCreatedBy. Do not import chatchannels. Do not GORM-hook User. Do not rewrite SeverUserFromContext.
|
||||
|
||||
Smoke ../sm-chat-plugin/gap_closure_test.go TestGapClosureSmoke (package chat_test, postgres via existing TestMain skip under -short): three members of one acme.teams host. Alice GetOrCreateDm Bob — the DM has two participants. Carol SendMessage on that conversation id returns ErrNotParticipant. Alice SendMessage on the same DM succeeds. Carol GetGroupChat then SendMessage on the group succeeds (lazy-join). Then Fire userclasses.DeletingEvent{User: Alice, Tx: env.gdb} and Unscoped-delete Alice the way FormAfterDelete does; the DM row still exists; created_by is Bob (or another remaining participant), not Alice.
|
||||
|
||||
Do not add TestTeammateDMWriteIDOR or extend TestDeletingCascade here (19-07). Do not push.</action>
|
||||
<verify>
|
||||
<automated>go -C ../sm-chat-plugin vet ./... && go -C ../sm-chat-plugin test -count=1 -short ./...</automated>
|
||||
<fails_when>non-zero exit, or a package reports FAIL, or "ok" lines absent</fails_when>
|
||||
<automated>docker info >/dev/null 2>&1 && go -C ../sm-chat-plugin test -count=1 -run '^TestGapClosureSmoke$' -v .</automated>
|
||||
<fails_when>non-zero exit: no Docker daemon (record NOT RUN in the SUMMARY), or TestGapClosureSmoke is missing from the PASS list, fails, or "no tests to run"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- grep -n 'return s.isParticipant' ../sm-chat-plugin/classes/service.go prints at least one line.
|
||||
- grep -n 'func (s \*Service) ReassignCreatedBy' ../sm-chat-plugin/classes/service.go prints one line.
|
||||
- grep -n 'func (s \*Service) WithDB' ../sm-chat-plugin/classes/service.go prints one line.
|
||||
- grep -n 'DeletingEvent' ../sm-chat-plugin/boot.go prints at least one line.
|
||||
- grep -n 'func TestGapClosureSmoke' ../sm-chat-plugin/gap_closure_test.go prints one line.
|
||||
- grep -n 'golem15_chat_conversations_created_by_foreign' ../sm-chat-plugin/updates/202610160001_create_golem15_chat_baseline.go prints one line.
|
||||
- grep -n 'ON DELETE CASCADE' ../sm-chat-plugin/updates/202610160001_create_golem15_chat_baseline.go prints at least one line.
|
||||
</acceptance_criteria>
|
||||
<done>A teammate who is not a DM participant cannot SendMessage; deleting the DM creator leaves the conversation with created_by on a remaining participant; group lazy-join still lets a member send.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: README send-rights and GDPR notes; leave WR/IN and baselines untouched</name>
|
||||
<files>../sm-chat-plugin/README.md, ../sm-chatchannels-plugin/README.md</files>
|
||||
<read_first>../sm-chat-plugin/README.md, ../sm-chatchannels-plugin/README.md, ../sm-chat-plugin/classes/service.go, ../sm-chat-plugin/boot.go</read_first>
|
||||
<action>Per D-08 and D-13, document the two closed gaps without expanding API surface beyond identifiers Task 1 exported. Chat README Features: SendMessage requires the sender to be a conversation participant (PHP hasParticipant); GetGroupChat still lazy-joins a context member via addParticipant; Boot Listens user.Deleting and ReassignCreatedBy moves created_by to another remaining participant (DMs and group rows) so the PHP ON DELETE CASCADE constraint can stay. Add WithDB and ReassignCreatedBy to the API table only if those exact identifiers are exported (docs checker). Chatchannels README GDPR bullet: remaining members keep the channel because chat reassigns created_by; SeverUserFromContext still only removes that user's members/prefs/participants/personal categories. Do not claim search/join/pin/download work changed.
|
||||
|
||||
Confirm by read, do not "fix": GetGroupChat still ignores addParticipant errors (IN-01); api/download.go Content-Disposition (WR-04); JoinChannel / PinMessage / search.go (WR-01..WR-03); FormAfterDelete is still the only production FireDeletingEvent site (IN-02). Do not ALTER baselines. Do not add a chatchannels created_by update. Do not edit scripts/check-phase19.sh.</action>
|
||||
<verify>
|
||||
<automated>go -C ../sm-chat-plugin test -count=1 -short ./...</automated>
|
||||
<fails_when>non-zero exit or a package reports FAIL</fails_when>
|
||||
<automated>go -C ../sm-chatchannels-plugin test -count=1 -short ./...</automated>
|
||||
<fails_when>non-zero exit or a package reports FAIL</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- grep -n 'ReassignCreatedBy' ../sm-chat-plugin/README.md prints at least one line.
|
||||
- grep -n 'participant' ../sm-chat-plugin/README.md prints at least one line.
|
||||
- grep -n 'created_by' ../sm-chatchannels-plugin/README.md prints at least one line.
|
||||
- grep -n '202610160001_create_golem15_chat_baseline' ../sm-chat-plugin/updates/202610160001_create_golem15_chat_baseline.go prints at least one line.
|
||||
</acceptance_criteria>
|
||||
<done>READMEs state participant-only send and created_by reassignment; review warnings and the baseline CASCADE are unchanged.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Commit the plugin repos; do not push</name>
|
||||
<files>../sm-chat-plugin/classes/service.go, ../sm-chat-plugin/boot.go, ../sm-chat-plugin/gap_closure_test.go, ../sm-chat-plugin/README.md, ../sm-chatchannels-plugin/README.md</files>
|
||||
<read_first>../sm-chat-plugin/classes/service.go, ../sm-chat-plugin/boot.go</read_first>
|
||||
<action>After Task 1 and Task 2 verify is green, commit in the sibling plugin git roots only. Stage explicit paths (never git add -A). No co-author tags. Do not skip hooks. Do not push sm-chat-plugin, sm-chatchannels-plugin, sm-user-plugin, summercms.go, or fonoteka.go. Do not bump fonoteka submodule pointers. Do not force-push sm-user-plugin (it is ahead 3 behind 1). Do not commit summercms.go planning files from this task.
|
||||
|
||||
Suggested messages (one logical change per repo): sm-chat-plugin — participant-only send and reassign created_by on user.Deleting so DMs and shared channels survive GDPR delete. sm-chatchannels-plugin — document that chat reassigns created_by; skip the commit if README is unchanged. Record SHAs in the SUMMARY. If Docker was down for TestGapClosureSmoke, record NOT RUN and still commit the production fix plus the smoke file.</action>
|
||||
<verify>
|
||||
<automated>git -C ../sm-chat-plugin status --porcelain -- classes/service.go boot.go gap_closure_test.go README.md</automated>
|
||||
<fails_when>non-zero exit or any porcelain line (working tree still dirty for those paths)</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- git -C ../sm-chat-plugin log -1 --oneline prints a commit that mentions send or created_by or Deleting.
|
||||
- git -C ../sm-chat-plugin status -sb does not list classes/service.go as modified.
|
||||
</acceptance_criteria>
|
||||
<done>Plugin repos have local commits for the two fixes; no remote was updated.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| JWT actor → POST /conversations/{id}/messages | a context member who is not a DM participant must not write |
|
||||
| Admin FormAfterDelete → user.Deleting → Postgres CASCADE | deleting the creator must not wipe remaining members' conversation/channel rows |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
Threat IDs continue after T-19-14 (highest in 19-01..19-05). T-19-SC is reserved.
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19-15 | Elevation of privilege | classes/service.go canSend, POST /conversations/{id}/messages | high | mitigate | Participant-only canSend (CR-01); ErrNotParticipant stays HTTP 404; TestGapClosureSmoke now, TestTeammateDMWriteIDOR in 19-07 |
|
||||
| T-19-16 | Tampering | user.Deleting → created_by CASCADE | high | mitigate | Chat ReassignCreatedBy before the user row is deleted; do not ALTER baseline CASCADE; TestGapClosureSmoke now, TestDeletingCascade creator path in 19-07 |
|
||||
| T-19-SC | Tampering | go.mod requires | high | mitigate | No new packages; analog WithDB / Listen only |
|
||||
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
go -C ../sm-chat-plugin test -count=1 -short ./... stays green. On a Docker host, go -C ../sm-chat-plugin test -count=1 -run '^TestGapClosureSmoke$' -v . proves both post-fix behaviours. Baselines are not ALTER'd. No remotes pushed.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
CR-01: send is participation-only; a third teammate cannot SendMessage into a two-person DM. CR-02: deleting a conversation creator reassigns created_by so remaining participants keep the row. WR-01..WR-04 and IN-01..IN-02 remain open. Named gate tests wait for 19-07.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19-chat-and-chatchannels/19-06-SUMMARY.md` when done
|
||||
</output>
|
||||
234
.planning/phases/19-chat-and-chatchannels/19-07-PLAN.md
Normal file
234
.planning/phases/19-chat-and-chatchannels/19-07-PLAN.md
Normal file
@@ -0,0 +1,234 @@
|
||||
---
|
||||
phase: 19-chat-and-chatchannels
|
||||
plan: 07
|
||||
type: execute
|
||||
wave: 6
|
||||
depends_on: ["19-06"]
|
||||
gap_closure: true
|
||||
files_modified:
|
||||
- ../sm-chat-plugin/http_tracer_test.go
|
||||
- ../sm-chatchannels-plugin/http_tracer_test.go
|
||||
- scripts/check-phase19.sh
|
||||
autonomous: true
|
||||
requirements: [SC-19-1, D-08, D-13]
|
||||
estimate:
|
||||
tokens: 120000
|
||||
raw_tokens: 120000
|
||||
tasks: 3
|
||||
confidence: low
|
||||
must_haves:
|
||||
truths:
|
||||
- "D-08 / SC-19-1: conversation- and message-level routes check participation on every call; ChatService.sendMessage ports 1:1 (PHP requires hasParticipant). Postgres named test: two context members open a DM; a third context member POSTs to that conversation id and gets 404. GetOrCreateDm still creates a two-person DM. The case is a required name in scripts/check-phase19.sh PG_CHAT_TESTS (TestTeammateDMWriteIDOR; TestChatHTTPTracer stays required)."
|
||||
- "D-13 GDPR: deleting a user severs that user from channels without CASCADE-wiping conversations remaining members still use. TestDeletingCascade still asserts Bob-as-member. It also asserts: Alice creates the channel, Alice is deleted (Fire user.Deleting then unscoped delete of Alice like FormAfterDelete), Bob's member row AND the channel row AND the conversation row still exist, and created_by is Bob (or another remaining participant), not Alice. TestDeletingCascade remains in PG_CHATCHANNELS_TESTS."
|
||||
- "scripts/check-phase19.sh --self-test still passes; --all on a Docker host requires the new postgres names. WR-01..WR-04 and IN-01..IN-02 stay open."
|
||||
artifacts:
|
||||
- path: "../sm-chat-plugin/http_tracer_test.go"
|
||||
provides: "HTTP 404 when a teammate POSTs into another pair's DM (CR-01)"
|
||||
contains: "TestTeammateDMWriteIDOR"
|
||||
- path: "../sm-chatchannels-plugin/http_tracer_test.go"
|
||||
provides: "creator-delete path of TestDeletingCascade (CR-02)"
|
||||
contains: "TestDeletingCascade"
|
||||
- path: "scripts/check-phase19.sh"
|
||||
provides: "fail-closed postgres list including TestTeammateDMWriteIDOR"
|
||||
contains: "TestTeammateDMWriteIDOR"
|
||||
key_links:
|
||||
- from: "../sm-chat-plugin/http_tracer_test.go"
|
||||
to: "../sm-chat-plugin/api/dm.go"
|
||||
via: "POST /contexts/{type}/{id}/dm then POST /conversations/{id}/messages as a third teammate is 404"
|
||||
pattern: "TestTeammateDMWriteIDOR"
|
||||
- from: "../sm-chatchannels-plugin/http_tracer_test.go"
|
||||
to: "../fonoteka.go/plugins/golem15/user/controllers/users_admin_controller.go"
|
||||
via: "Fire DeletingEvent then Unscoped delete of the creator; remaining member/channel/conversation rows persist"
|
||||
pattern: "Unscoped"
|
||||
- from: "scripts/check-phase19.sh"
|
||||
to: "../sm-chat-plugin/http_tracer_test.go"
|
||||
via: "PG_CHAT_TESTS requires TestTeammateDMWriteIDOR by name (go_named slash-free top-level)"
|
||||
pattern: "TestTeammateDMWriteIDOR"
|
||||
prohibitions:
|
||||
- statement: "Do not ALTER the IF NOT EXISTS baselines or PHP constraint names. Do not add a new gormigrate ID."
|
||||
status: unresolved
|
||||
verification: grep
|
||||
- statement: "Do not push remotes unless a human later asks. Do not bump fonoteka submodule pointers. Do not force-push sm-user-plugin (ahead 3 behind 1). Do not edit summercms.go modules. Never git add -A. Never --no-verify."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "WR-01, WR-02, WR-03, WR-04, IN-01 and IN-02 stay open. Do not 'fix' them in this tests-last plan."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "Do not put a slash-containing subtest name in PG_CHAT_TESTS or PG_CHATCHANNELS_TESTS: go_named builds a -run regex that splits on /."
|
||||
status: unresolved
|
||||
verification: judgment
|
||||
- statement: "Production sources never contain the four D-02 tokens. No /_wave routes. No new third-party require."
|
||||
status: unresolved
|
||||
verification: grep
|
||||
---
|
||||
|
||||
## Phase Goal
|
||||
|
||||
**As a** application developer, **I want to** have Chat and ChatChannels run on SummerCMS as shared plugins with realtime through lighthouse, **so that** they are ready for the Wavepath port.
|
||||
|
||||
This plan's slice (plan 7 of 7, gap closure, wave 6, LAST): named postgres tests for CR-01 and CR-02 and the fail-closed `scripts/check-phase19.sh` list. Unit tests are the last plan (CLAUDE.md).
|
||||
|
||||
## Gap being closed (19-VERIFICATION.md `gaps` missing tests)
|
||||
|
||||
1. CR-01 missing: "Postgres case: two context members open a DM; a third context member POSTs to that conversation id and must get 404."
|
||||
2. CR-02 missing: "Extend TestDeletingCascade so Alice creates the channel, Alice is deleted, and Bob's member row and the channel row still exist" — also the conversation row, and `created_by` is a remaining participant.
|
||||
|
||||
Production fixes shipped in 19-06. This plan only adds the required tests and the gate names.
|
||||
|
||||
## Spec-less probe fallback
|
||||
|
||||
Skipped visibly: see 19-01. `must_haves` are the two VERIFICATION.md gaps plus the phase gate.
|
||||
|
||||
## Decisions taken in this plan (Claude's discretion)
|
||||
|
||||
- New top-level `TestTeammateDMWriteIDOR` (not a `/` subtest) so `go_named` `-run` regex stays valid. Keep `TestChatHTTPTracer` in PG_CHAT_TESTS.
|
||||
- Extend `TestDeletingCascade` in place (keep the Bob-as-member block). Add a second setup in the same function for Alice-as-creator. Do not add a slash name to PG_CHATCHANNELS_TESTS.
|
||||
- Proven verify commands from Phase 19 (do not copy Phase 18.1 cabana commands).
|
||||
- No push checkpoint: commit plugin repos locally after green; leave remotes alone.
|
||||
|
||||
## Artifacts this phase produces (this plan)
|
||||
|
||||
- `TestTeammateDMWriteIDOR` in sm-chat-plugin.
|
||||
- Creator-delete assertions in `TestDeletingCascade`.
|
||||
- `scripts/check-phase19.sh` PG_CHAT_TESTS includes `TestTeammateDMWriteIDOR`.
|
||||
- Local plugin-repo commits; no remotes, no fonoteka pointer bump.
|
||||
|
||||
<objective>
|
||||
Lock CR-01 and CR-02 with named postgres tests the phase gate requires, then keep check-phase19.sh fail-closed.
|
||||
|
||||
Purpose: 19-06 smoke is not a required gate name; verification cannot seal until the teammate DM 404 and creator-delete CASCADE cases are required by check-phase19.sh.
|
||||
Output: TestTeammateDMWriteIDOR and extended TestDeletingCascade pass on Docker; --self-test and --all (or plugins when Docker is down) are green.
|
||||
</objective>
|
||||
|
||||
<execution_context>
|
||||
@~/.claude/msd-core/workflows/execute-plan.md
|
||||
@~/.claude/msd-core/templates/summary.md
|
||||
</execution_context>
|
||||
|
||||
<context>
|
||||
@.planning/phases/19-chat-and-chatchannels/19-CONTEXT.md
|
||||
@.planning/phases/19-chat-and-chatchannels/19-VERIFICATION.md
|
||||
@.planning/phases/19-chat-and-chatchannels/19-06-PLAN.md
|
||||
@.planning/phases/19-chat-and-chatchannels/19-05-SUMMARY.md
|
||||
scripts/check-phase19.sh
|
||||
|
||||
Concurrency: other sessions may edit STATE.md. Do not stage those. Stage by explicit path. No co-author tags. Do not push. Do not rewrite 19-01..19-06.
|
||||
|
||||
<interfaces>
|
||||
- ../sm-chat-plugin/http_tracer_test.go bootWeb, call, frontendToken, TestChatHTTPTracer, TestDMThrottle (POST /contexts/{type}/{id}/dm)
|
||||
- ../sm-chat-plugin/api/dm.go StartDM; ../sm-chat-plugin/api/messages.go PostMessage; ../sm-chat-plugin/api/errors.go ErrNotParticipant -> 404
|
||||
- ../sm-chatchannels-plugin/http_tracer_test.go TestDeletingCascade (today deletes Bob as member of Alice's channel; Fire DeletingEvent; does not Unscoped-delete)
|
||||
- ../fonoteka.go/plugins/golem15/user/controllers/users_admin_controller.go FormAfterDelete: FireDeletingEvent then tx.Unscoped().Where("id = ?", user.ID).Delete(&models.User{})
|
||||
- scripts/check-phase19.sh PG_CHAT_TESTS and PG_CHATCHANNELS_TESTS; go_named regex ^((names))$ ; self-test token loop
|
||||
- 19-06 TestGapClosureSmoke (keep; not a required gate name)
|
||||
</interfaces>
|
||||
</context>
|
||||
|
||||
<tasks>
|
||||
|
||||
<task type="tracer">
|
||||
<name>Task 1: End-to-end teammate DM write is HTTP 404</name>
|
||||
<files>../sm-chat-plugin/http_tracer_test.go</files>
|
||||
<read_first>../sm-chat-plugin/http_tracer_test.go, ../sm-chat-plugin/api/dm.go, ../sm-chat-plugin/api/messages.go, ../sm-chat-plugin/api/errors.go, ../sm-chat-plugin/gap_closure_test.go</read_first>
|
||||
<action>Per D-08 / SC-19-1 (CR-01), add a top-level postgres test named TestTeammateDMWriteIDOR in ../sm-chat-plugin/http_tracer_test.go (package chat_test, same bootWeb helpers as TestChatHTTPTracer / TestDMThrottle). Do not name it with a slash. Do not edit check-phase19.sh in this task (Task 3 adds the required name).
|
||||
|
||||
Three users A, B, C are members of one acme.teams host (env.team with all three ids). A frontend JWT POSTs /contexts/{teamsAlias}/{teamID}/dm with body user_id = B and expects 200 and a conversation id. Count golem15_chat_participants for that id: exactly two rows (A and B). C's JWT POSTs /conversations/{id}/messages with content — expect 404 (ErrNotParticipant mapping; not 201). A's JWT POSTs the same conversation id and is 200 or 201. Keep TestChatHTTPTracer's stranger-of-context 404 case as-is.
|
||||
|
||||
Reuse env.call and frontendToken. Skip under -short via existing TestMain. Do not weaken GetOrCreateDm. Do not "fix" WR/IN findings.</action>
|
||||
<verify>
|
||||
<automated>go -C ../sm-chat-plugin vet ./... && go -C ../sm-chat-plugin test -count=1 -short ./...</automated>
|
||||
<fails_when>non-zero exit or a package reports FAIL</fails_when>
|
||||
<automated>docker info >/dev/null 2>&1 && go -C ../sm-chat-plugin test -count=1 -run '^TestTeammateDMWriteIDOR$' -v .</automated>
|
||||
<fails_when>non-zero exit: no Docker daemon (record NOT RUN in the SUMMARY), or TestTeammateDMWriteIDOR is missing from the PASS list, fails, or "no tests to run"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- grep -n 'func TestTeammateDMWriteIDOR' ../sm-chat-plugin/http_tracer_test.go prints one line.
|
||||
- grep -n 'func TestChatHTTPTracer' ../sm-chat-plugin/http_tracer_test.go prints one line.
|
||||
</acceptance_criteria>
|
||||
<done>A third context member who POSTs into another pair's DM conversation id gets 404; the two participants can still send.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 2: Extend TestDeletingCascade for Alice-as-creator</name>
|
||||
<files>../sm-chatchannels-plugin/http_tracer_test.go</files>
|
||||
<read_first>../sm-chatchannels-plugin/http_tracer_test.go, ../fonoteka.go/plugins/golem15/user/controllers/users_admin_controller.go, ../fonoteka.go/plugins/golem15/user/classes/events.go, ../sm-chat-plugin/classes/service.go, ../sm-chatchannels-plugin/classes/channel_service.go</read_first>
|
||||
<action>Per D-13 (CR-02), extend TestDeletingCascade. Keep the existing Bob-as-member assertions (Fire DeletingEvent for Bob; Bob's members/prefs/personal categories are gone; Alice's member row remains). Do not replace that block.
|
||||
|
||||
Add a second setup in the same function (after the Bob block, with a fresh CreateChannel): Alice creates a private channel with Bob in participantIDs. Confirm Bob has a member row and the channel has a conversation_id. Then match FormAfterDelete: env.app.Events.Fire DeletingEvent{User: alice, Tx: env.gdb}, then env.gdb.Unscoped().Where("id = ?", alice.ID).Delete the user model (same Unscoped delete as users_admin_controller FormAfterDelete). After that: Bob's golem15_chatchannels_channel_members row for that channel still exists; the golem15_chatchannels_channels row still exists; the golem15_chat_conversations row for conversation_id still exists; that conversation's created_by is Bob (or another remaining participant), not Alice.
|
||||
|
||||
Use a second channel so the first block (which severs Bob from the first channel) cannot empty the only remaining participant set. Do not ALTER baselines. Do not add a slash-named required test. Do not edit check-phase19.sh in this task.</action>
|
||||
<verify>
|
||||
<automated>go -C ../sm-chatchannels-plugin vet ./... && go -C ../sm-chatchannels-plugin test -count=1 -short ./...</automated>
|
||||
<fails_when>non-zero exit or a package reports FAIL</fails_when>
|
||||
<automated>docker info >/dev/null 2>&1 && go -C ../sm-chatchannels-plugin test -count=1 -run '^TestDeletingCascade$' -v .</automated>
|
||||
<fails_when>non-zero exit: no Docker daemon (record NOT RUN in the SUMMARY), or TestDeletingCascade is missing from the PASS list, fails, or "no tests to run"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- grep -n 'func TestDeletingCascade' ../sm-chatchannels-plugin/http_tracer_test.go prints one line.
|
||||
- grep -n 'Unscoped' ../sm-chatchannels-plugin/http_tracer_test.go prints at least one line.
|
||||
- grep -n 'created_by' ../sm-chatchannels-plugin/http_tracer_test.go prints at least one line.
|
||||
</acceptance_criteria>
|
||||
<done>Deleting Alice the creator leaves Bob's member row, the channel row, and the conversation row, with created_by on a remaining participant; Bob-as-member assertions still pass.</done>
|
||||
</task>
|
||||
|
||||
<task type="auto">
|
||||
<name>Task 3: Require the new names in check-phase19.sh; commit plugins; do not push</name>
|
||||
<files>scripts/check-phase19.sh, ../sm-chat-plugin/http_tracer_test.go, ../sm-chatchannels-plugin/http_tracer_test.go</files>
|
||||
<read_first>scripts/check-phase19.sh, ../sm-chat-plugin/http_tracer_test.go, ../sm-chatchannels-plugin/http_tracer_test.go</read_first>
|
||||
<action>Per SC-19-4, add TestTeammateDMWriteIDOR to PG_CHAT_TESTS in scripts/check-phase19.sh (slash-free). Keep TestChatHTTPTracer, TestChatTracer, TestDMThrottle, TestAttachmentDownload, TestThreadMute. Keep TestDeletingCascade in PG_CHATCHANNELS_TESTS (do not add a slash subtest name). Add TestTeammateDMWriteIDOR to the run_self_test token loop that greps the script so a missing token fails self-test. Self-test must still pass.
|
||||
|
||||
Proven commands (reuse, do not invent cabana paths): go -C ../sm-chat-plugin test -count=1 -short ./... ; on Docker, go -C ../sm-chat-plugin test -count=1 -run '^TestChatHTTPTracer$' -v . and go -C ../sm-chat-plugin test -count=1 -run '^TestTeammateDMWriteIDOR$' -v . ; go -C ../sm-chatchannels-plugin test -count=1 -run '^TestDeletingCascade$' -v . ; bash scripts/check-phase19.sh --self-test ; bash scripts/check-phase19.sh --all when Docker is available (plugins + docs + postgres). If Docker is down, --all prints NOT RUN postgres unless PHASE19_REQUIRE_POSTGRES=1.
|
||||
|
||||
After tests green, commit in the plugin repos (http_tracer_test.go paths) and commit scripts/check-phase19.sh in summercms.go only if this session is allowed to touch that file as a code artifact of this plan — stage that path explicitly, never git add -A, never --no-verify. Do not push any remote. Do not bump fonoteka submodule pointers. Do not force-push sm-user-plugin. Do not rewrite 19-01..19-06. WR-01..WR-04 and IN-01..IN-02 stay open.
|
||||
|
||||
If Docker was down, still add the names and commit the tests; record NOT RUN postgres in the SUMMARY.</action>
|
||||
<verify>
|
||||
<automated>bash -n scripts/check-phase19.sh && bash scripts/check-phase19.sh --self-test</automated>
|
||||
<fails_when>non-zero exit, or stdout does not contain "phase19 self-test passed"</fails_when>
|
||||
<automated>go -C ../sm-chat-plugin test -count=1 -short ./...</automated>
|
||||
<fails_when>non-zero exit or a package reports FAIL</fails_when>
|
||||
<automated>docker info >/dev/null 2>&1 && bash scripts/check-phase19.sh --all</automated>
|
||||
<fails_when>non-zero exit: no Docker daemon (record NOT RUN postgres in the SUMMARY), or a stage prints FAIL, or postgres does not print "phase19 postgres passed"</fails_when>
|
||||
</verify>
|
||||
<acceptance_criteria>
|
||||
- grep -n 'TestTeammateDMWriteIDOR' scripts/check-phase19.sh prints at least one line.
|
||||
- grep -n 'TestDeletingCascade' scripts/check-phase19.sh prints at least one line.
|
||||
- grep -n 'TestChatHTTPTracer' scripts/check-phase19.sh prints at least one line.
|
||||
- grep -nE '^run_admin|^run_layers' scripts/check-phase19.sh prints nothing.
|
||||
</acceptance_criteria>
|
||||
<done>The phase gate requires TestTeammateDMWriteIDOR and TestDeletingCascade by name; self-test passes; plugin tests are committed locally and not pushed.</done>
|
||||
</task>
|
||||
|
||||
</tasks>
|
||||
|
||||
<threat_model>
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description |
|
||||
|----------|-------------|
|
||||
| JWT teammate → POST /conversations/{id}/messages | gate must keep requiring the 404 case or CR-01 can regress silently |
|
||||
| FormAfterDelete sequence → channel rows | gate must keep requiring creator-delete or CR-02 can regress silently |
|
||||
|
||||
## STRIDE Threat Register
|
||||
|
||||
Threat IDs continue after T-19-16 (19-06). Do not reuse T-19-01..T-19-16. T-19-SC is reserved.
|
||||
|
||||
| Threat ID | Category | Component | Severity | Disposition | Mitigation Plan |
|
||||
|-----------|----------|-----------|----------|-------------|-----------------|
|
||||
| T-19-17 | Elevation of privilege | TestTeammateDMWriteIDOR + PG_CHAT_TESTS | high | mitigate | Required slash-free postgres name so CR-01 cannot regress without failing check-phase19.sh |
|
||||
| T-19-18 | Tampering | TestDeletingCascade creator path | high | mitigate | Fire Deleting then Unscoped delete of Alice; remaining member/channel/conversation persist; TestDeletingCascade stays in PG_CHATCHANNELS_TESTS |
|
||||
| T-19-SC | Tampering | scripts/check-phase19.sh | high | mitigate | No package installs; --self-test plants failures; new token grepped |
|
||||
|
||||
</threat_model>
|
||||
|
||||
<verification>
|
||||
go -C ../sm-chat-plugin test -count=1 -short ./... stays green. Docker host: go -C ../sm-chat-plugin test -count=1 -run '^TestChatHTTPTracer$' -v . ; go -C ../sm-chat-plugin test -count=1 -run '^TestTeammateDMWriteIDOR$' -v . ; go -C ../sm-chatchannels-plugin test -count=1 -run '^TestDeletingCascade$' -v . ; bash scripts/check-phase19.sh --self-test ; bash scripts/check-phase19.sh --all when Docker is available.
|
||||
</verification>
|
||||
|
||||
<success_criteria>
|
||||
Named postgres tests prove teammate DM write-IDOR is 404 and creator-delete does not wipe remaining members' channel. check-phase19.sh requires those names. Unit tests were the last plan. No remotes pushed.
|
||||
</success_criteria>
|
||||
|
||||
<output>
|
||||
Create `.planning/phases/19-chat-and-chatchannels/19-07-SUMMARY.md` when done
|
||||
</output>
|
||||
Reference in New Issue
Block a user