Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/613-main-bot-owner.security.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
`set-main` no longer lets a member take another owner's Main Bot's place: the endpoint checks the Bot named in `replace` too, and `setMain` refuses a `replace` across owners, so a member cannot demote another owner's Main Bot or promote their own Bot over it ([#613](https://github.com/ThinkFlowLab/FlyBot/issues/613)).
11 changes: 10 additions & 1 deletion core/src/host/authz.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,16 @@ export function install(rt) {
return requireSession(body?.sessionId)
}
if (ep === 'usage') return body?.botId === undefined ? ALLOW : requireBot(body.botId)
if (ep === 'set-main') return requireBot(body?.id)
if (ep === 'set-main') {
// `replace` names the Main Bot whose place the target takes, and the Main list is
// deployment-wide: checking only `body.id` let a member swap another owner's Main Bot
// for their own and demote it (#613).
if (typeof body?.replace === 'string' && body.replace !== '' && body.replace !== body?.id) {
const replaceGate = requireBot(body.replace)
if (!replaceGate.ok) return replaceGate
}
return requireBot(body?.id)
}
if (ep === 'set-flags') {
const id = body?.id ?? body?.botId
if (botOwnedBy(actor, id) || roomOwnedBy(actor, id) || roomMemberOf(actor, id)) return ALLOW
Expand Down
4 changes: 4 additions & 0 deletions core/src/host/roster.js
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,10 @@ export function install(rt) {
} else if (replace !== undefined && replace !== id) {
const at = others.indexOf(replace)
if (at === -1) throw new Error('Only a Main Bot can be replaced')
// The Main list is deployment-wide, so a swap is only within one owner: taking another
// member's Main Bot's place demotes it for them (#613). authz.js refuses the endpoint for
// the same reason; this keeps the state invariant for every caller.
if ((rt.botOf(replace)?.ownerId ?? null) !== (bot.ownerId ?? null)) throw new Error('Only a Bot of the same owner can take a Main Bot’s place')
next = others.with(at, id)
} else if (primary) {
next = [id, ...others]
Expand Down
25 changes: 25 additions & 0 deletions core/test/authz.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -433,3 +433,28 @@ test('a group\'s question card is its Bot owner\'s to see and answer, not every
assert.deepEqual(Object.keys(rt.ownedView(full, actorA).questions), ['rx'])
assert.deepEqual(Object.keys(rt.ownedView(full, actorB).questions), [])
})

// #613: the Main Bot list is deployment-wide, so `replace` crosses the owner boundary unless the
// actor owns both the Bot it promotes and the Main Bot whose place it takes.
test('#613: a member cannot take another owner’s Main Bot’s place', async (t) => {
const { rt } = runtime(t, { ...TEAM, mainBotIds: ['m1'] })
await rt.load()
const sessionA = await redeemOwner(rt)
const sessionB = await redeemOwner(rt)
assert.equal(sessionB.memberId, 'owner-2')
rt.state.bots.a1 = { id: 'a1', sessionId: 'a1', name: 'A', instructions: '', createdAt: 2, ownerId: 'owner-1' }
rt.state.bots.b1 = { id: 'b1', sessionId: 'b1', name: 'B', instructions: '', createdAt: 3, ownerId: 'owner-2' }
const actorA = await rt.authenticate(sessionA.sessionToken)
const actorB = await rt.authenticate(sessionB.sessionToken)

// Bob's own Bot may not take Alice's Main Bot's place: promoting it that way demotes hers.
const escalation = rt.authorize(actorB, 'set-main', { id: 'b1', main: true, replace: 'm1' })
assert.equal(escalation.ok, false)
assert.equal(escalation.code, 'bot/forbidden')
// Demoting hers by naming it as the target must not work either.
assert.equal(rt.authorize(actorB, 'set-main', { id: 'm1', main: false }).ok, false)
// A swap inside one owner's team stays allowed.
assert.equal(rt.authorize(actorA, 'set-main', { id: 'a1', main: true, replace: 'm1' }).ok, true)
// And Bob's own Bot keeps the plain promotion the deployment-wide list gives it.
assert.equal(rt.authorize(actorB, 'set-main', { id: 'b1', main: true }).ok, true)
})
31 changes: 31 additions & 0 deletions core/test/roster.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -593,3 +593,34 @@ test('a failed Main Bot create does not leave a dangling designated Chief', asyn
assert.equal(saved.members['owner-1'].designatedChiefId, null)
assert.equal(Object.keys(saved.bots || {}).length, 0)
})

// #613: `setMain` is the layer that writes `mainBotIds`, so the same-owner rule is enforced here
// too, not only at the endpoint: a `replace` across owners would demote another member's Bot.
test('#613: a Main Bot’s place is only taken by a Bot of the same owner', async (t) => {
const { call, stop, home } = start({
version: 2,
revision: 1,
members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'chief-a' }, 'owner-2': { id: 'owner-2', createdAt: 2, designatedChiefId: 'chief-b' } },
rooms: {},
exchanges: [],
exchangeTurns: {},
questions: {},
prefs: {},
mainBotIds: ['chief-a'],
bots: {
'chief-a': { id: 'chief-a', name: 'Alice Chief', ownerId: 'owner-1' },
'chief-b': { id: 'chief-b', name: 'Bob Chief', ownerId: 'owner-2' },
'helper-b': { id: 'helper-b', name: 'Bob Helper', ownerId: 'owner-2' },
},
})
t.after(stop)
const refused = await call('set-main', { id: 'chief-b', main: true, replace: 'chief-a' })
assert.equal(refused.ok, false)
assert.match(refused.error.message, /same owner/)
assert.deepEqual(JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).mainBotIds, ['chief-a'])
// A swap within Bob's own team still works.
await call('set-main', { id: 'chief-b', main: true })
const swapped = await call('set-main', { id: 'helper-b', main: true, replace: 'chief-b' })
assert.equal(swapped.ok, true)
assert.deepEqual(swapped.value.mainBotIds, ['chief-a', 'helper-b'])
})
1 change: 1 addition & 0 deletions docs/subsystems/core-host.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ external backend or task-specific session mapping; see the
### Team state and access

- The saved team (`state.json`) is versioned. Version 2 (#47) carries the identity schema — `members` records, a per-Bot `ownerId`, the owner's `designatedChiefId`. A legacy version-1 file is migrated on load with a one-time `state.json.v1` backup; an existing backup counts only when it is a readable file holding a parseable version-1 state, and anything else (or any malformed ownership) fails closed (read-only). An older binary refuses a newer version read-only. This is schema foundation only — team mode stays disabled, see [team](team.md). On top of it, member auth (#48) adds host module `src/host/member-auth.js`, installed after `store` in `MODULES` (`core/index.js`): `invitations` and `authSessions` maps live on state (no `STATE_VERSION` bump), the channel resolves the actor from the `x-flybot-member-session` header, and endpoints are `auth-invite` / `auth-redeem` / `auth-revoke` / `auth-whoami` / `auth-remove-member` / `auth-revoke-invite` with `body.actorId` ignored; `auth-invite` alone also opens for an anonymous caller on a host with no sessions yet (bootstrap first invite), `auth-revoke-invite` kills an unredeemed invitation (redeeming it afterwards fails with `revoked`), `removeMember` refuses a member that still owns a Bot (`has_bots`), and the auth mutations are serialized behind an in-process lock so concurrent `removeMember` calls cannot both pass the last-owner check. Sign-in surface only — still not team mode; details in [team](team.md). On top of it, owner-scoped access (#49) adds host module `src/host/authz.js`, installed after member-auth: when any `authSessions` exist, the channel `authorize()` default-denies cross-owner Bot/room/session access, blocks shared non-isolatable resources (secrets, connectors, folder picks, browser bridge, update-install, diagnostics, set-prefs, and the DSH Agent with its Sessions — `add-agent`, `agent-session`, `agent-adopt`, `agent-archive`, `agent-rename`, `remove-agent`, which name an ownerless `agentId` and are one per deployment) with `bot/blocked-shared` when more than one member exists, stamps `create-bot` with `actor.memberId`, and filters `view()`/`live()` to the actor — every answer that returns a view, `delete-bot`, `set-main`, `remove-agent` and `delete-room` included, passes the actor so it stays scoped; with no sessions the single-human behavior is unchanged. Session targets resolve through the store's owner maps — a Bot id via `botOf`, current chats and group Sessions and earlier parts via `selfOf`/`groupOwners`/`pastOwners` (a shared room Session also resolves for its current participants, #50) — and unresolvable ids deny. `approval-answer` gates on the approval's own Bot or room (resolved by approval id), never on caller-supplied session fields. `flybot/*` plugin endpoints (src/host/hooks.js) reach any signed-in actor for reads; plugin writes follow the shared rule. `view()`/`live()` narrowing (`rt.ownedView`/`rt.ownedLive` in authz.js) keeps per-session entries (`questions`, `exchangeTurns`) the actor owns, and deployment-wide surfaces (agents, secrets, connectors, memory reviews) only while a single member exists — the same rule as the shared writes; rooms the actor belongs to and the Bots in them (#50) stay visible. `create-room` refuses a request with neither a member Bot nor any participant. On top of it, room membership (#50) adds host module `src/host/room-membership.js`, installed after groups: a room may carry typed `participants` (`human` / local `bot` / granted remote `bot` with `handle` + owner fields, roles `admin` / `member`, `joinedSeq` / `leftSeq`); log lines carry `authorId` + `authorKind`; history is the membership window (current members and the owner of an invited Bot see `seq >=` their interval; leave withdraws access; rejoin starts a new interval); local cross-owner rooms accept each side's *granted* Bots (#90: default deny, Chief included); a friend's Bot from `social/remote-bots` (`cleanRemote`) may be added and ungranted Bots stay private, and a room with a friend's Bot holds only granted local Bots (a Bot that joined by room code, `viaCode`, only for that room); `view()` labels remotes with an owner badge and shows an invited owner **Shared room from \<inviter\>**. Face-to-face rooms (#83) reuse the same `participants` / membership-window / owner-badge presentation with `origin: 'face-to-face'`: room-only remotes may sit in the room without a person-grant or a friendship, `presentRoom` treats the viewer as a member (not Shared room from), leave skips the two-Bot floor, and `fanoutRoomLine` goes to `rt.faceRoomFanout` instead of friend `flybotSend`. Still not team mode — see [team](team.md). Per-Bot grants (#90) add host module `src/host/reach.js`, installed after room-membership: `state.grants[botId]` is the list of independent grants (one Bot → one grantee, optionally room-scoped); default deny, Chief included; a Bot whose owner is not a member has none. Grants are set with `rt.setGrants`, exposed as `BotView.granted`, `flybot.bot.hasGrant` / `hasAnyGrant`, `flybot.grants` and the `bot.grants` event. `reach()` is the owner policy: a friend's Bot may reach only local Bots granted to that sender, only a granted Bot (or one code-joined in that room) may message a friend's Bot, another owner's Bot is reached only if both sides have matching grants or both are members of the room the message belongs to. It is not a middleware: hooks.js runs it before the `bot.send` chain, again on approval release, and on every deliver with a sender or remote origin (resumed, retried, deferred, re-sent), so no verdict or `bypass` can skip it and a queued message is judged by what is true when it is delivered. Revoking a grant, or replacing its Chief, removes it from rooms that have friends' Bots or several owners at once (`rt.evictUnshared`). Approval binding (#52) completes the held-send half of this: `state.flybot.approvals` records carry `requestId`, a sha256 `version` over the request content, server-derived `ownerId` and `origin` (`via`/`hop`/audience) — claims in the message (`meta`, text) never confer authority. `flybot/decide` is allowed through authz for any signed-in actor and the per-record check happens in hooks (`decide` requires the sender Bot's owner's session, `bot/forbidden` otherwise, `bot/unauthorized` without a session); `release()` revalidates before dispatch and marks the record `stale` when the request changed, the sender or target Bot is gone (also invalidated eagerly on `deleteBot`), or the Bot changed owners; a demoted Chief's sender-side holds are invalidated eagerly on `setMain` (not inside `staleReason`), and `release()` re-checks immediately before dispatch so an in-flight approve cannot go out after a concurrent invalidation. Unknown `invalidateFor` field values throw. `flybot/approvals` lists only the actor's own holds while enforcement is on; decided stale records keep their attribution in state but appear in no member's list.
- The Main Bot list is deployment-wide (#613): `set-main` requires the actor to own `body.id` and, when `body.replace` names the Main Bot whose place the target takes, that Bot too, so a member cannot swap another owner's Main Bot for their own or demote it (both are `bot/forbidden`); `roster.setMain` keeps the same invariant for every caller, refusing a `replace` across owners and leaving `state.mainBotIds` unchanged. A swap inside one owner's team, the plain promotion of a Bot the caller owns, and the deployment-wide list itself are unchanged.

- Context sharing (#53) is specified in [team](team.md), **Context sharing foundation**: the own-chat brief is share-gated (`share-brief` / `briefShared`), a memory review reads only its owner as the user (`isUserLine`) and labels another human as `peer` (not `bot`), and keeps cross-owner entries out of the team scope (`teamAllowedFrom`), `list_bots` lists only the caller's owner's Bots (a granted child: that child's `createdBy` subtree), Bots granted to the same owner, and Bots in the current room — another owner's folder and instructions stay private — and a `dsh-session:` chip in actor-authored `send` / `room-post` / `answer-question` / `schedule-create` text must resolve for the author (`sessionOwnedBy`; `schedule-create` authz keys off `body.target`). Bot-authored group posts and Bot-to-Bot / own-chat followups drop chips so they cannot tunnel a private chat into another Session. Host modules: `src/host/groups.js`, `src/host/memory.js`, `src/host/tools.js`, `src/host/channel.js`, `src/host/room-membership.js`, `src/host/authz.js`. Team mode stays disabled.

Expand Down
11 changes: 11 additions & 0 deletions patches/ds-bot/0002-flybot-api-host.patch
Original file line number Diff line number Diff line change
Expand Up @@ -370,6 +370,17 @@ diff --git a/src/host/roster.js b/src/host/roster.js
rt.roomInbox.delete(id)
const failed = await saveForDelete()
// Armed scheduled tasks refuse a plain archive and would keep delivering into
@@ -409,6 +409,10 @@ export function install(rt) {
} else if (replace !== undefined && replace !== id) {
const at = others.indexOf(replace)
if (at === -1) throw new Error('Only a Main Bot can be replaced')
+ // The Main list is deployment-wide, so a swap is only within one owner: taking another
+ // member's Main Bot's place demotes it for them (#613). authz.js refuses the endpoint for
+ // the same reason; this keeps the state invariant for every caller.
+ if ((rt.botOf(replace)?.ownerId ?? null) !== (bot.ownerId ?? null)) throw new Error('Only a Bot of the same owner can take a Main Bot’s place')
next = others.with(at, id)
} else if (primary) {
next = [id, ...others]
@@ -422,6 +422,7 @@ export function install(rt) {
if (rt.botOf(other)?.kickoff?.main) delete rt.botOf(other).kickoff
}
Expand Down
33 changes: 32 additions & 1 deletion patches/ds-bot/0009-identity-schema.patch
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ diff --git a/test/roster.test.js b/test/roster.test.js
test('a new team starts with a DeepSeek-blue whale as its Main Bot', async (t) => {
const { call, stop } = start()
t.after(stop)
@@ -526,3 +526,34 @@ test('a write that fails at save time archives the Session it made', async (t) =
@@ -526,3 +526,65 @@ test('a write that fails at save time archives the Session it made', async (t) =
const saved = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8'))
assert.equal(Object.values(saved.bots).some(bot => bot.name === 'Phantom'), false)
})
Expand Down Expand Up @@ -88,3 +88,34 @@ diff --git a/test/roster.test.js b/test/roster.test.js
+ assert.equal(saved.members['owner-1'].designatedChiefId, null)
+ assert.equal(Object.keys(saved.bots || {}).length, 0)
+})
+
+// #613: `setMain` is the layer that writes `mainBotIds`, so the same-owner rule is enforced here
+// too, not only at the endpoint: a `replace` across owners would demote another member's Bot.
+test('#613: a Main Bot’s place is only taken by a Bot of the same owner', async (t) => {
+ const { call, stop, home } = start({
+ version: 2,
+ revision: 1,
+ members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'chief-a' }, 'owner-2': { id: 'owner-2', createdAt: 2, designatedChiefId: 'chief-b' } },
+ rooms: {},
+ exchanges: [],
+ exchangeTurns: {},
+ questions: {},
+ prefs: {},
+ mainBotIds: ['chief-a'],
+ bots: {
+ 'chief-a': { id: 'chief-a', name: 'Alice Chief', ownerId: 'owner-1' },
+ 'chief-b': { id: 'chief-b', name: 'Bob Chief', ownerId: 'owner-2' },
+ 'helper-b': { id: 'helper-b', name: 'Bob Helper', ownerId: 'owner-2' },
+ },
+ })
+ t.after(stop)
+ const refused = await call('set-main', { id: 'chief-b', main: true, replace: 'chief-a' })
+ assert.equal(refused.ok, false)
+ assert.match(refused.error.message, /same owner/)
+ assert.deepEqual(JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')).mainBotIds, ['chief-a'])
+ // A swap within Bob's own team still works.
+ await call('set-main', { id: 'chief-b', main: true })
+ const swapped = await call('set-main', { id: 'helper-b', main: true, replace: 'chief-b' })
+ assert.equal(swapped.ok, true)
+ assert.deepEqual(swapped.value.mainBotIds, ['chief-a', 'helper-b'])
+})
11 changes: 10 additions & 1 deletion patches/ds-bot/files/src/host/authz.js
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,16 @@ export function install(rt) {
return requireSession(body?.sessionId)
}
if (ep === 'usage') return body?.botId === undefined ? ALLOW : requireBot(body.botId)
if (ep === 'set-main') return requireBot(body?.id)
if (ep === 'set-main') {
// `replace` names the Main Bot whose place the target takes, and the Main list is
// deployment-wide: checking only `body.id` let a member swap another owner's Main Bot
// for their own and demote it (#613).
if (typeof body?.replace === 'string' && body.replace !== '' && body.replace !== body?.id) {
const replaceGate = requireBot(body.replace)
if (!replaceGate.ok) return replaceGate
}
return requireBot(body?.id)
}
if (ep === 'set-flags') {
const id = body?.id ?? body?.botId
if (botOwnedBy(actor, id) || roomOwnedBy(actor, id) || roomMemberOf(actor, id)) return ALLOW
Expand Down
Loading
Loading