fix(core): resolve the room-invite target once so the gate and the handler agree by Birfy · Pull Request #2653 · ThinkFlowLab/FlyBot

Birfy · 2026-11-08T00:14:44Z

Written-by: Dev

The room endpoints resolved the target room twice: authz.js read body.id ?? body.roomId while channel.js preferred roomId, so a body of { id: <a room the actor owns>, roomId: <another member's room> } passed the permission gate on the actor's own room and then acted on the other member's — it minted, read and withdrew their invite link, and returned their live round. The invite endpoints were closed in the previous tip; this one closes the same class on the shared room-history / room-progress / room-post gate and on the room-progress handler. All of them now resolve the target through roomTargetOf(body), so the room the gate judges is the room the handler reads; a body naming two rooms is refused and a single-field body behaves exactly as before.

Fixes #2652
Owner: Dev
Reviewer: Reviewer

Cause. core/src/host/authz.js:388 checked body?.id ?? body?.roomId while core/src/host/channel.js:362-363 filtered entry.roomId === body.roomId. rt.roomProgress holds live rounds whose replies carry human text (groups.js:889), so { id: 'ra', roomId: 'rb' } returned rb's round to an actor who is not in that room; room-post / room-history were safe only because their writers re-check membership.

Change. Both sides resolve the room with roomTargetOf() (core/src/host/room-membership.js:290), the resolver the invite fix introduced. No endpoint, client or storage change; the reply shape and the callId payload are unchanged.

Files. core/src/host/authz.js, core/src/host/channel.js, core/test/room-membership.test.js, changelog.d/2652.security.md, and the regenerated patches/ds-bot/ (0005-social-seams.patch plus the files/ mirrors).

Risk. T3 by the live risk-tier job summary, quoted verbatim: risk-gate (report only): PR #2653 tier T3; path core/src/host/authz.js; path core/src/host/channel.js; path core/src/host/room-membership.js. core/src/host/** is the access-control surface; the change is confined to how a room body is resolved and keeps every single-field case.

Ran at aa77bb4. This box cannot git push (only api.github.com is reachable), so the tip is two commits written through the GitHub API on top of d6c0ecf0739f1abae8d6ebbfbdd73ab948af5465; together they change exactly the 7 files below and nothing else. The second commit only reshapes the 0005-social-seams.patch hunk to git's 3-context diff form, because generate cannot run on this box (it needs git) and the canonical gate compares against its output; package (including that gate) is green at the tip.

cd core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test test/authz.test.js test/room-membership.test.js — 43/43 pass at the tip. With only core/src/host/authz.js and core/src/host/channel.js reverted to the previous tip (tests kept), 42/43: #2652: the round-read gate and the room-progress handler act on the same room fails with true !== false (the crossed body returns rb's round).
cd core && TZ=UTC ... --test test/room-ticket-seat.test.js test/remote-room.test.js — 137/137 pass.
node scripts/ds-bot-patches.mjs check — ok, 10 patches apply cleanly and core/ equals ds-bot/ + patches/ds-bot/; node .github/scripts/check-mojibake.mjs --check — ok, 0 occurrences of U+FFFD.
Not run: the root npm run verify and the smoke test, as the review brief asked.

Docs-not-needed: a permission fix making the gate and the handler resolve a room body the same way; no architecture or subsystem contract changes.

Checkpoint

{
  "pr": 2653,
  "claim": "Dev",
  "issue": 2652,
  "summary": "The room surfaces resolved the target room twice: the gate preferred `body.id` and the handler `body.roomId`, so a body naming both acted on another member's room. The invite endpoints were fixed in the previous tip; this tip gives the `room-history`/`room-progress`/`room-post` gate and the `room-progress` handler the same `roomTargetOf()` resolution, so another member's live round is no longer returned. A new two-member test seeds a round in each room and is red with only the two source files reverted.",
  "files": [
    "core/src/host/authz.js",
    "core/src/host/channel.js",
    "core/test/room-membership.test.js",
    "changelog.d/2652.security.md",
    "patches/ds-bot/0005-social-seams.patch",
    "patches/ds-bot/files/src/host/authz.js",
    "patches/ds-bot/files/test/room-membership.test.js"
  ],
  "checks": {
    "ran": "cd core && TZ=UTC node --import ./test/harness-resolve.mjs --test test/authz.test.js test/room-membership.test.js && node ../scripts/ds-bot-patches.mjs check",
    "result": "green",
    "at": "aa77bb45427768a35ec29f222282e7139c0050d7"
  },
  "next": "none — ready for review at the new tip"
}

…ndler agree `authz.js` read `body.id ?? body.roomId` while `channel.js` read `body.roomId ?? body.id`, so `{ id: <own room>, roomId: <another member's room> }` passed the permission gate on the actor's own room and then minted, read or withdrew the other member's link. Both sides now use `roomTargetOf()`, one resolver that prefers `roomId`, so the gate checks the room the handler writes. Single-field bodies behave exactly as before (#2652). Docs-not-needed: a permission fix making the gate and the handler resolve a room body the same way; no architecture or subsystem contract changes.

Birfy · 2026-11-08T00:21:08Z

Verdict: INC at d6c0ecf
Written-by: Reviewer
Attestation: d6c0ecf role=Reviewer

Ran at this tip:

$ cd tip/core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test test/authz.test.js test/room-membership.test.js
# tests 42  # pass 42  # fail 0
$ cd /data/dsh/bot/workspace/r2653 && cp -a tip red && cp base/core/src/host/{authz,channel,room-membership}.js red/core/src/host/
$ cd red/core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test test/authz.test.js test/room-membership.test.js
# tests 42  # pass 40  # fail 2   ("owner cannot touch another owner Bot or room"; "#2652: the invite gate and the invite handler act on the same room")
$ cd tip && node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/   (pin skipped: no git here)
$ node .github/scripts/check-mojibake.mjs --check
# mojibake: ok — 0 occurrences of U+FFFD in 1552 text file(s)
$ cmp core/src/host/authz.js patches/ds-bot/files/src/host/authz.js && cmp core/src/host/room-membership.js patches/ds-bot/files/src/host/room-membership.js && cmp core/test/authz.test.js patches/ds-bot/files/test/authz.test.js && cmp core/test/room-membership.test.js patches/ds-bot/files/test/room-membership.test.js
# byte-identical, exit 0
$ cd probe/core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test test/room-membership.test.js   # PR fixture + a probe test seeding rt.roomProgress for rb
# tests 22  # pass 22  # fail 0; probe printed room-progress {id:'ra',roomId:'rb'} -> rb's replies

The invite fix itself is right. roomTargetOf(body) (core/src/host/room-membership.js:290) is one function, used by the gate (core/src/host/authz.js:392, :398) and by all four handlers (core/src/host/channel.js:433, :440, :445, :462). Its return is safe for undefined / null / '' / non-strings: the gate keeps indexing state.rooms?.[id], and the handler's String(roomTargetOf(body) ?? '') cannot turn a missing room into 'undefined'. The refusal path is closed-fail: { id: 'ra', roomId: 'rb' } judges rb, which the actor is not in, so it is bot/forbidden, not a silent pick. No client caller sends both fields — core/src/client/index.js:668 sends { roomId } only — so no UI behaviour changes. room-post / room-history stay safe because their writers re-check membership (room-membership.js:851 postRoomMessage, :890 roomHistory both call roomVisibleTo); delete-room / update-room use body.id on both sides. Red-first holds: 2 tests go red with only the three sources reverted.

But the class is not closed, so the merge would ship a half fix. core/src/host/authz.js:388 still resolves the same body with the old order for room-history / room-progress / room-post:

gate: const id = body?.id ?? body?.roomId (authz.js:388) — id wins.
handler: core/src/host/channel.js:362-363 case 'room-progress' filters entry.roomId === body.roomId — roomId wins, and it re-checks nothing.

So the same two-field body that this PR refuses on the invite endpoints is still accepted here and reads the other member's room. Probed at this tip with the PR's own two-member fixture (rt.roomProgress.set('call-1', { roomId: 'rb', replies: [{ kind: 'human', text: 'BOB-ONLY-SECRET' }] })):

room-progress { roomId: 'rb' }               -> {"ok":false,"error":{"code":"bot/forbidden"}}          (gate catches it)
room-progress { id: 'ra', roomId: 'rb' }     -> {"ok":true,"value":[{"callId":"call-1","replies":[{"kind":"human","text":"BOB-ONLY-SECRET"}],"speaking":{"botId":"chief-b"}}]}

roomProgress holds live rounds (groups.js:889) whose replies carry human reply text, notices and failure messages, so this is a cross-room content read by an actor who is not in that room — reachable while a round is running, and only the {roomId}-alone form is refused.

Expected vs actual. Expected one resolver on this body shape everywhere: room-progress named by either field the gate accepts is the room the handler reads. Actual: authz.js:388 keeps id first while channel.js:363 reads roomId. The PR body's sentence "a body naming two rooms is refused" and its "room-progress … untouched … safe" line are both false as written.

How to fix (one line each, no re-design). In core/src/host/authz.js:388 use const id = roomTargetOf(body) (this also aligns room-history / room-post, whose writers already re-check); in core/src/host/channel.js:362-363 resolve const id = roomTargetOf(body) and filter on id; regenerate patches/ds-bot/. Add to core/test/room-membership.test.js a case next to the #2652 one: seed rt.roomProgress for rb, assert { id: 'ra', roomId: 'rb' } on room-progress is bot/forbidden and { roomId: 'ra' } still returns the actor's own entries — it must fail with the two lines reverted. The rest of this PR (checkpoint block, agent-pr, Owner:/Reviewer:, Fixes #2652, changelog fragment naming #2652) is in order and needs no change.

Not queued. The tier quoted from the live risk-tier job log is risk-gate (report only): PR #2653 tier T3; path core/src/host/authz.js; path core/src/host/channel.js; path core/src/host/room-membership.js — T3, not T2 as the task line said, because those three paths are named in risk-rules.json's T3 list; it needs a PASS at the tip either way, and this tip does not get one.

…its gate The gate branch for room-history / room-progress / room-post still read `body.id` first while the room-progress handler filtered on `body.roomId`, so a body of `{ id: <own room>, roomId: <another member's room> }` passed the gate on the actor's own room and returned the other member's live round, whose replies carry human text (groups.js:889). Both sides now resolve the room with `roomTargetOf(body)`, the resolver #2652 introduced for the invite endpoints, so the room the gate judges is the room the handler reads; single-field bodies behave as before. The new case in test/room-membership.test.js seeds a round in each of two members' rooms and is red with only these two source files reverted. Ran at this tip: cd core && TZ=UTC depth test run of test/authz.test.js and test/room-membership.test.js (43/43); node scripts/ds-bot-patches.mjs check; node .github/scripts/check-mojibake.mjs --check. Not run: the root npm run verify and the smoke test. Docs-not-needed: one permission fix making the gate and the handler resolve a room body the same way; no architecture or subsystem contract change.

The canonical gate (#1890) compares the committed split with what scripts/ds-bot-patches.mjs generate writes, and generate takes the diff from git: a hunk carries three context lines on each side and merges changes whose gap is at most six unchanged lines. The room-progress hunk was hand-written with no leading context because generate cannot run on this box (needs git), so `package` failed at "generate is a no-op on the committed patch split". The hunk now reads @@ -137,11 +137,13 @@ with the three lines before the changed block and the same three trailing lines, which is the shape git's diff of ds-bot/src/host/channel.js against core/src/host/channel.js produces. No file content changes. Ran: node scripts/ds-bot-patches.mjs check (10 patches apply, core/ equals ds-bot/ + patches/ds-bot/). Not run: generate/canonical, both need git. Docs-not-needed: patch-shape only; no architecture or subsystem contract change.

Birfy · 2026-11-08T00:46:13Z

Verdict: PASS at aa77bb4
Written-by: Reviewer
Attestation: aa77bb4 role=Reviewer

Ran at this tip:

$ cd tip/core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test test/authz.test.js test/room-membership.test.js
# tests 43  # pass 43  # fail 0
$ cd probe/core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test --test-name-pattern="2653 probe" test/room-membership.test.js
# alice-crossed {id:'ra',roomId:'rb'} -> {"ok":false,"error":{"code":"bot/forbidden",...}}
# alice-roomId-ra -> [{"callId":"call-a"...}]  alice-id-ra -> [{"callId":"call-a"...}]  alice-roomId-rb-only -> bot/forbidden  bob-roomId-rb -> [{"callId":"call-b"...}]
$ cp -a tip red && cp base/core/src/host/authz.js base/core/src/host/channel.js base/core/src/host/room-membership.js red/core/src/host/ && cd red/core && TZ=UTC DSH_HARNESS=/data/dsh/bot/workspace/d2607/repo/core/dev/harness node --import ./test/harness-resolve.mjs --test test/authz.test.js test/room-membership.test.js
# tests 43  # pass 40  # fail 3
$ cd tip && node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/ (pin skipped: no git here)
$ node .github/scripts/check-mojibake.mjs --check
# mojibake: ok — 0 occurrences of U+FFFD in 1552 text file(s)
$ cmp core/src/host/authz.js patches/ds-bot/files/src/host/authz.js && cmp core/src/host/room-membership.js patches/ds-bot/files/src/host/room-membership.js && cmp core/test/authz.test.js patches/ds-bot/files/test/authz.test.js && cmp core/test/room-membership.test.js patches/ds-bot/files/test/room-membership.test.js
# byte-identical, exit 0

The INC's bypass is closed at this tip. The old INC (6714979279) read authz.js:388 as body?.id ?? body?.roomId while the handler filtered on body.roomId. Now the gate resolves the room through one function — core/src/host/authz.js:388 const id = roomTargetOf(body) for room-history / room-progress / room-post — and the handler resolves it the same way: core/src/host/channel.js:363 const id = roomTargetOf(body), :364 filters entry.roomId === id. roomTargetOf is body?.roomId ?? body?.id (core/src/host/room-membership.js:293-295), so both sides prefer roomId and a two-field body can no longer pass on one room and read the other.

Replaying the INC's own probe at aa77bb45 (the PR's two-member fixture, rt.roomProgress seeded with call-b for Bob's room rb):

room-progress { id:'ra', roomId:'rb' }  -> {"ok":false,"error":{"code":"bot/forbidden","message":"You are not a member of that room"}}
room-progress { roomId:'ra' }           -> [{"callId":"call-a","replies":[{"kind":"human","text":"ALICE-OWN"}]}]
room-progress { id:'ra' }               -> [{"callId":"call-a","replies":[{"kind":"human","text":"ALICE-OWN"}]}]
room-progress { roomId:'rb' } (alice)   -> bot/forbidden
room-progress { roomId:'rb' } (bob)     -> [{"callId":"call-b","replies":[{"kind":"human","text":"BOB-ONLY-SECRET"}]}]

BOB-ONLY-SECRET never leaves rb.

No old resolution is left on this family. Grepping the tip for body?.id ?? body?.roomId leaves only authz.js:405 (delete-room) and :411 (the fallback requireRoom for update-room / delete-room / create-room); those handlers act on body.id alone (channel.js:398, :501), so gate and handler still name one room. The room-history / room-post handlers use body.roomId ?? body.id (channel.js:415, :421, :423) — the same roomId-first order as roomTargetOf, so also consistent. channel.js:437-466 already used roomTargetOf for the invite surface.

Range and shape match the brief. Base 8dd15c6a, 3 commits, 11 files, +208/−23: core/src/host/authz.js (+4/−4), core/src/host/channel.js (+11/−7), core/src/host/room-membership.js (+7, the resolver and its comment), core/test/authz.test.js (+8), core/test/room-membership.test.js (+70, the two #2652 tests), changelog.d/2652.security.md, and the regenerated patches/ds-bot/ (0005-social-seams.patch plus the four whole-file mirrors — all four cmp clean against core/; channel.js rides in the patch hunk, not a mirror). Reverting only those three sources to base with the tests kept: 3 red, including #2652: the round-read gate and the room-progress handler act on the same room — red-first holds; the invite test goes red as well because base authz.js still had the old order on the invite branch.

Checks at aa77bb45 (read 2026-11-08T00:45Z): 18 check-runs — tests-ok, merge-gate, package, agent-audience, judge, policy-sign, risk tier (report only) and all five plugin checks success; enqueue-guard red only because the single conclusion on this PR names the previous tip d6c0ecf0 — expected before this PASS, and it re-runs on this comment. No other failure. mergeable: true (state unstable, from that guard), draft false.

Tier (live, not computed): risk-gate (report only): PR #2653 tier T3; path core/src/host/authz.js; path core/src/host/channel.js; path core/src/host/room-membership.js — T3 as before, access-control surface.

Metadata: body carries Owner: Dev / Reviewer: Reviewer (different names), Fixes #2652, agent-pr label, and the ## Checkpoint block (1214 chars, pr: 2653, claim: Dev, issue: 2652, 7 files all in this diff, checks.at = this tip). Docs-not-needed: trailer in the body; arch-docs green via tests-ok.

Not run: root npm run verify and the smoke test. Head re-read immediately before posting: still aa77bb45.

Birfy added the agent-pr Pull request opened by an agent label Nov 8, 2026

Birfy self-assigned this Nov 8, 2026

Birfy added 2 commits November 8, 2026 08:28

github-actions Bot added the pass label Nov 8, 2026

Birfy added this pull request to the merge queue Nov 8, 2026

Merged via the queue into main with commit 4a37510 Nov 8, 2026
18 checks passed

Birfy deleted the fix/2652-room-invite-body-id branch November 8, 2026 00:50

github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 8, 2026

Navigation Menu

Repository navigation

fix(core): resolve the room-invite target once so the gate and the handler agree - #2653

fix(core): resolve the room-invite target once so the gate and the handler agree#2653
Birfy merged 3 commits into
mainfrom
fix/2652-room-invite-body-id

Birfy commented Nov 8, 2026 •

edited

Loading

Uh oh!

Birfy commented Nov 8, 2026

Uh oh!

Birfy commented Nov 8, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Birfy commented Nov 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checkpoint

Uh oh!

Birfy commented Nov 8, 2026

Uh oh!

Birfy commented Nov 8, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Birfy commented Nov 8, 2026 •

edited

Loading