Repository navigation
feat(authz): scope bot resources and administration to their owner - #64
Conversation
Fixes #49. Enforced only once a member session exists. Cross-owner Bot/room/session access is denied; shared non-isolatable resources (secrets, connectors, folders, browser, update-install, diagnostics, set-prefs) are blocked with bot/blocked-shared while more than one member exists. create-bot stamps actor.memberId; view()/live() filter to the actor. Unenforced (no sessions) behavior is unchanged. Team mode stays disabled; #16 stays open.
Magpie Verifier (Codex) — FAILBlocking findings (summary)
Full report: executor workspace Next: OpenCode fix loop on this tip → Codex re-review. Not merge authority. Team mode stays disabled; #16 untouched. |
hsliuustc0106
left a comment
There was a problem hiding this comment.
Verifier review
Verdict: FAIL at tip f822243aef189a68df2aff1baa3e3922da001d1b. The suite is green — I reproduced 616/616 at this tip — but the enforced path is broken in the real runtime: the tests never assert the positive session path, so a load-bearing bug ships green (details in P1-1).
Tip SHA: f822243aef189a68df2aff1baa3e3922da001d1b
Ran:
- core `npm test` at this tip → 616/616 pass
- `node scripts/ds-bot-patches.mjs check` → ok (10 patches apply cleanly, core = ds-bot + patches)
- `node scripts/build-client.mjs --check` → ok
- `npm run lint` → 0 errors; 21 warnings vs 20 on main (one new, cosmetic — spread-to-array in the update-room ids build)
- Executed `src/host/authz.js` from this tip on a runtime with store + member-auth + authz
installed (same modules as the core/test/authz.test.js fixture; `installStore` provides
`rt.botOf`, and no other module overrides it) → reproductions below
Not run:
- smoke bench / Desktop / real-model steward / browser multi-member UI
- plugins (social/tree) suites — green in CI at this tip; the findings below are in the
enforced channel path those suites do not cover
- arch-docs — verified only via CI (green); not runnable outside a git checkout
OpenCode↔Codex: n/a
Verdict: FAIL
P1-1 — Members cannot send to their own Bots (core/src/host/authz.js:70)
rt.botOf (src/host/store.js:466) is id => state.bots[id] — it returns the bot object, not an id. sessionOwnedBy treats the return value as an id and hands it to botOwnedBy, whose typeof botId === 'string' guard rejects objects, so every bot-chat session resolves to "not owned":
// at this tip, one member signed in, bot a1 owned by the actor:
rt.authorize(actor, 'send', { sessionId: 'a1' })
// → { ok: false, code: 'bot/forbidden', message: 'You do not own that session' }That denies all of SESSION_TARGETED — send, earlier, answer-question, dismiss-question, approval-answer, exchange — including the owner's own Bots. Enforcement starts with the first member session, so once it is on, chat is unusable for everyone. Fix direction: resolve the bot id (rt.botOf(id)?.id, or via chatOwners/selfOf like the rest of the host) and add the positive test — "owner can send to their own Bot" must fail on the unbroken tip per fail-without-change. The 7 current tests only assert cross-owner denials, which pass for the wrong reason; that is how this slipped through green.
P1-2 — All flybot/* plugin endpoints default-deny
The channel's default branch routes flybot/… to rt.flybotEndpoint (src/host/hooks.js), but authorize() runs before the switch and the namespace is in no allow table:
rt.authorize(actor, 'flybot/sidebar/structure', {}) // → { ok: false, code: 'bot/not-found' }
rt.authorize(actor, 'flybot/decide', { id: 'x' }) // → { ok: false, code: 'bot/not-found' }Once anyone signs in this breaks the tree sidebar's structure polling, the social panel (invites etc.), plugin attachment fetches (core/src/client/cells.js:229), every client plugin-slot call (core/src/client/flybot-slots.js:26), and flybot/decide. The namespace needs an explicit rule — e.g. allow flybotIsRead reads to a signed-in actor and define the write policy per plugin.
P1-3 — approval-answer gate does not match its payload (core/src/host/authz.js:153)
The client sends { id, choice } where id is an approval id, not a session id (core/src/client/index.js:543); the approval's session id is a different field the client never sends. Demonstrated: the rightful owner is denied answering their own Bot's approval (bot/forbidden). The sessionId || id OR is also a bypass in the other direction: answerApproval acts on body.id, so pairing one's own session id with someone else's approval id passes the gate. The gate should resolve the approval by id (the pending-approvals map carries sessionId/botId) and check ownership of that.
P2-1 — The sole signed-in owner loses first-party UI surfaces
view()/live() blank questions, exchangeTurns, agents, secrets, secretRequests, connectors, memoryReviews for any actor once enforced — including a single-member deployment, where SHARED_BLOCKED deliberately still allows secret-set/folders. Net effect today: the owner can configure secrets but can no longer see them, and their own Bots' questions disappear from view() (state.questions is keyed by sessionId and could be filtered with sessionOwnedBy instead of dropped). Enforcement begins with the very first member session, so this hits the primary single-user flow, not just multi-human setups.
P2-2 — add-agent allowed for every member (core/src/host/authz.js:117)
rt.addAgent() takes no arguments and stamps no owner; the created DSH Agent is deployment-wide. It belongs in SHARED_BLOCKED with the other one-per-deployment resources. Related dead check: the agent-* byAgent branch reads state.agents[id].ownerId, a field nothing ever sets.
P3 / notes
sessionOwnedByconsultspastOwnersbut notrt.groupOwners, whichsrc/host/schedules.jsandsrc/host/memory.jstreat as the primary resolver for group session ids — inconsistent resolution for schedule/memory targets on group sessions.create-roomwith an emptymemberslist is allowed (core/src/host/authz.js:135); the resulting room is owned by nobody (roomOwnedByrequires ≥1 member) and can never be updated, deleted or viewed afterwards.- Docs / #16 conformance: good — no
create_child_bot, team mode stays disabled, and the "Not done here" list in team.md is accurate. One watch item:set-mainnow lets any owner promote their own Bot to Main; that touches the planned authority contract's edges without deciding #16 — worth a sentence in team.md. - Process: team.md says the routing rows (a)–(e) become fail-without-change tests "when enforcement work starts" — this PR is that start, and neither those rows nor any positive session path is tested. P1-1 is exactly the class that gap misses.
Happy to re-review on a new tip; per code-standards each fix should land with the positive test that fails on the current tip.
This is a Verifier report, not merge authority.
Fixes #49.
Owner-scoped access foundation on #48 member auth. Enforced only once a member session exists (`authzEnforced`). Team mode stays disabled; #16 open.
Verified
Not run