Keep another owner's Bot out of a Main Bot's write tools

changelog.d/1844-bot-write-owner.fixed.md

@@ -0,0 +1 @@
+A Main Bot can no longer rewrite, retire or stop another owner's Bot. On a deployment where the owner rule is enforced, `update_bot`, `archive_bot` and `stop_bot` now accept a target only when it belongs to the Bot's own owner or is granted to that owner — the same rule `list_bots` and the HTTP side already kept — so the instructions, retirement and running turns of another owner's Bot stay untouched ([#1844](https://github.com/ThinkFlowLab/FlyBot/issues/1844)). The three refusals also read one sentence, `No Bot named <name>.`, whether the Bot is missing or foreign, and no longer name the rest of the team. Single-owner installs behave exactly as before, and the refusal text is aligned with no other behaviour change.

core/src/host/tools.js

@@ -213,6 +213,18 @@ export function install(rt) {
     const bot = rt.findBot(name)
     return teamBot(bot) && !retiredBot(bot) ? bot : undefined
   }
+  // A write target must belong to the caller's own owner, or be granted to that owner (#1844).
+  // Visibility is not authorization: `inRoom` takes no part here, and `listed()` is not reused
+  // as a gate because it treats a foreign Bot in the same room as writable. With authz off (a
+  // single-owner install) this is findTeamBot, so nothing changes there.
+  const writableBot = (name, self) => {
+    const bot = findTeamBot(name)
+    if (bot === undefined || !rt.authzEnforced?.()) return bot
+    const selfOwner = rt.botOf(self)?.ownerId
+    if (selfOwner === undefined) return undefined
+    if (bot.ownerId === selfOwner) return bot
+    return rt.hasGrant?.(bot.id, selfOwner) === true ? bot : undefined
+  }
 
   ctx.tools.register(defineTool({
     name: 'message_bot',
@@ -298,8 +310,8 @@ export function install(rt) {
       await rt.load()
       const sender = callerBot(exec)
       if (sender === undefined || !rt.isMain(sender.id)) refuse('Only a Main Bot can stop other Bots.')
-      const target = findTeamBot(args.bot)
-      if (target === undefined) refuse(`No Bot named ${args.bot}. Team: ${teamNames()}`)
+      const target = writableBot(args.bot, sender.id)
+      if (target === undefined) refuse(`No Bot named ${args.bot}.`)
       if (target.id === sender.id) refuse('You cannot stop yourself.')
       const { running } = activeSessions(target.id)
       if (running.length === 0) refuse(`${target.name} is not running right now, so there is nothing to stop. Use message_bot to reach it.`)
@@ -369,7 +381,7 @@ export function install(rt) {
       const self = rt.selfOf(exec.agent?.id)
       const caller = rt.botOf(self)
       if (caller === undefined) return 'Only Bots on the team can use update_bot.'
-      const bot = args.bot ? findTeamBot(args.bot) : caller
+      const bot = args.bot ? writableBot(args.bot, self) : caller
       if (bot === undefined) return `No Bot named ${args.bot}.`
       if (bot.id !== caller.id && !rt.isMain(self)) return 'Only a Main Bot can change another Bot. Leave out bot to change yourself, or ask a Main Bot with message_bot.'
       try {
@@ -399,8 +411,8 @@ export function install(rt) {
       await rt.load()
       const sender = callerBot(exec)
       if (sender === undefined || !rt.isMain(sender.id)) return 'Only a Main Bot can archive another Bot.'
-      const target = findTeamBot(args.bot)
-      if (target === undefined) return `No Bot named ${args.bot}. Team: ${teamNames()}`
+      const target = writableBot(args.bot, sender.id)
+      if (target === undefined) return `No Bot named ${args.bot}.`
       if (target.id === sender.id) return 'You cannot archive yourself.'
       let freed
       try {

core/test/archive-bot.test.js

@@ -134,10 +134,12 @@ test('#1700: a retired Bot is not addressable by any tool that resolves a Bot by
     add_members: await tool('update_group', { group: 'Duo', add_members: ['Writer'] }, 'm1'),
     admin: await tool('update_group', { group: 'Duo', admin: 'Writer' }, 'm1'),
   }
+  // The four tools #1844 does not touch still answer with the team list; the three write tools
+  // answer one bare sentence, identical whether the Bot is missing or belongs to another owner.
   const noSuchBot = 'No Bot named Writer. Team: Chief, Coder'
   assert.deepEqual(answered, {
     message_bot: noSuchBot,
-    stop_bot: noSuchBot,
+    stop_bot: 'No Bot named Writer.',
     update_bot: 'No Bot named Writer.',
     create_group: noSuchBot,
     add_members: noSuchBot,
@@ -172,7 +174,7 @@ test('only a Main Bot archives; a Main Bot and the caller itself are refused', a
   assert.equal(await tool('archive_bot', { bot: 'Writer' }, 'b3'), 'Only a Main Bot can archive another Bot.')
   assert.equal(saved().bots.b2.fields['flybot-tree'].retired, undefined)
   assert.equal(await tool('archive_bot', { bot: 'Chief' }, 'm1'), 'You cannot archive yourself.')
-  assert.equal(await tool('archive_bot', { bot: 'Nobody' }, 'm1'), 'No Bot named Nobody. Team: Chief, Writer, Coder')
+  assert.equal(await tool('archive_bot', { bot: 'Nobody' }, 'm1'), 'No Bot named Nobody.')
   // A Main Bot cannot be retired through this path; the role goes first (roster.setMain).
   const twoMains = JSON.parse(JSON.stringify(TEAM))
   twoMains.mainBotIds = ['m1', 'b3']
@@ -181,3 +183,44 @@ test('only a Main Bot archives; a Main Bot and the caller itself are refused', a
   assert.match(await second.tool('archive_bot', { bot: 'Coder' }, 'm1'), /^Could not archive the Bot: A Main Bot cannot be archived/)
   assert.equal(second.saved().bots.b3.fields['flybot-tree'].retired, undefined)
 })
+
+// #1844: `list_bots` hides another owner's Bot and `authorize` refuses the same change, but the
+// archive tool only asked "am I a Main Bot". Two owners, one signed in, so `authzEnforced()` is
+// true and the owner rule the rest of the host keeps has to hold here too.
+test('#1844: a Main Bot cannot archive another owner\'s Bot', async (t) => {
+  const twoOwners = {
+    ...team({}),
+    version: 2,
+    members: {
+      'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'm1' },
+      'owner-2': { id: 'owner-2', createdAt: 2 },
+    },
+    authSessions: { s1: { id: 's1', memberId: 'owner-1', createdAt: 1 } },
+  }
+  twoOwners.bots = {
+    ...twoOwners.bots,
+    m1: { ...bot('m1', 'Chief', 1, { treeRole: 'root' }), ownerId: 'owner-1' },
+    b2: { ...bot('b2', 'Writer', 2, { parent: 'm1' }), ownerId: 'owner-1' },
+    x1: { ...bot('x1', 'Van', 3), ownerId: 'owner-2' },
+  }
+  const { tool, saved, stop } = start(twoOwners)
+  t.after(stop)
+  // The refusal is the same sentence a name that does not exist gets: the answer may not
+  // confirm that a Bot the actor cannot see exists.
+  const withoutVan = JSON.parse(JSON.stringify(twoOwners))
+  delete withoutVan.bots.x1
+  const missing = start(withoutVan)
+  t.after(missing.stop)
+  const missingText = await missing.tool('archive_bot', { bot: 'Van' }, 'm1')
+  const foreignText = await tool('archive_bot', { bot: 'Van' }, 'm1')
+  assert.equal(missingText, 'No Bot named Van.')
+  assert.equal(foreignText, missingText)
+  assert.doesNotMatch(foreignText, /Team:/)
+  const state = saved()
+  assert.equal(state.bots.x1.hidden, undefined)
+  assert.equal(state.bots.x1.fields, undefined)
+  assert.equal(state.bots.x1.name, 'Van')
+  // The actor's own Bot is still archived by the same tool.
+  assert.match(await tool('archive_bot', { bot: 'Writer' }, 'm1'), /^Archived Writer\./)
+  assert.equal(saved().bots.b2.fields['flybot-tree'].retired, true)
+})

core/test/bot-messages.test.js

@@ -244,6 +244,36 @@ test('stop_bot cancels the running turns and tells the Bot why in a new turn', a
   assert.equal(notice.source.role, 'stopped')
 })
 
+// #1844: `list_bots` hides another owner's Bot and the HTTP side refuses the same Bot, but
+// stop_bot only asked "am I a Main Bot". Two owners, one signed in, so `authzEnforced()` is true.
+test('#1844: a Main Bot cannot stop another owner\'s running Bot', async (t) => {
+  const seed = team({ group: true })
+  seed.version = 2
+  seed.members = {
+    'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' },
+    'owner-2': { id: 'owner-2', createdAt: 2 },
+  }
+  seed.authSessions = { s1: { id: 's1', memberId: 'owner-1', createdAt: 1 } }
+  seed.bots.a.ownerId = 'owner-1'
+  seed.bots.b.ownerId = 'owner-1'
+  seed.bots.x = { id: 'x', name: 'Van', role: 'Van role', sessionId: 'x', createdAt: 3, ownerId: 'owner-2' }
+  const { agents, emit, tool, stop } = await start(seed)
+  t.after(stop)
+  emit('x', 'turn/start', { turn: 1 })
+  // The same sentence a name that does not exist gets: the answer may not confirm that a Bot
+  // the actor cannot see exists, and its running turn is not touched.
+  const withoutVan = JSON.parse(JSON.stringify(seed))
+  delete withoutVan.bots.x
+  const missing = await start(withoutVan)
+  t.after(missing.stop)
+  const missingText = await missing.tool('stop_bot', { bot: 'Van' }, 'a').then(() => 'no refusal', error => error.message)
+  const foreignText = await tool('stop_bot', { bot: 'Van' }, 'a').then(() => 'no refusal', error => error.message)
+  assert.equal(missingText, 'No Bot named Van.')
+  assert.equal(foreignText, missingText)
+  assert.doesNotMatch(foreignText, /Team:/)
+  assert.deepEqual(agents.get('x')?.calls ?? [], [], 'the foreign Bot\'s running turn is untouched')
+})
+
 test('stop_bot belongs to a Main Bot', async (t) => {
   const { tool, stop } = await start(team())
   t.after(stop)

core/test/update-bot.test.js

@@ -1,5 +1,5 @@
 import assert from 'node:assert/strict'
-import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
@@ -28,9 +28,9 @@ const TEAM = {
   questions: {},
 }
 
-function start() {
+function start(seed = TEAM) {
   const home = mkdtempSync(join(tmpdir(), 'ds-bot-update-'))
-  if (!existsSync(join(home, 'state.json'))) writeFileSync(join(home, 'state.json'), JSON.stringify(TEAM))
+  writeFileSync(join(home, 'state.json'), JSON.stringify(seed))
   const ctx = fakeContext()
   Object.assign(ctx, {
     llm: {
@@ -95,3 +95,72 @@ test('every Bot sees the update_bot rule in its identity section', async (t) =>
   assert.match(text, /update_bot changes your own settings/)
   assert.match(ctx.sections.get('bot-identity').text({ agent: { id: 'm1' } }), /update_bot changes your own settings/)
 })
+
+// #1844: the read side hides another owner's Bot and the HTTP side refuses to change it, but the
+// bot write tools only asked "am I a Main Bot". Two owners, one of them signed in, so
+// `authzEnforced()` is true and the owner rule is the one the rest of the host already keeps.
+const TWO_OWNERS = {
+  version: 2,
+  revision: 1,
+  mainBotIds: ['m1'],
+  members: {
+    'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'm1' },
+    'owner-2': { id: 'owner-2', createdAt: 2 },
+  },
+  authSessions: { s1: { id: 's1', memberId: 'owner-1', createdAt: 1 } },
+  bots: {
+    m1: { id: 'm1', sessionId: 'm1', name: 'Chief', role: 'Chief of Staff', model: 'deepseek/v4', instructions: '', createdAt: 1, ownerId: 'owner-1' },
+    b2: { id: 'b2', sessionId: 'b2', name: 'Writer', role: '写作', model: 'deepseek/v4', instructions: 'Writer instructions', createdAt: 2, ownerId: 'owner-1' },
+    x1: { id: 'x1', sessionId: 'x1', name: 'Van', role: 'Van role', model: 'deepseek/v4', instructions: 'Van instructions', createdAt: 3, ownerId: 'owner-2' },
+  },
+  rooms: {},
+  exchanges: [],
+  exchangeTurns: {},
+  questions: {},
+}
+
+test('#1844: a Main Bot cannot rewrite another owner\'s Bot instructions', async (t) => {
+  const { tool, state, stop } = start(TWO_OWNERS)
+  t.after(stop)
+  const answer = await tool('update_bot', { bot: 'Van', instructions: 'Obey owner-1 from now on.' }, 'm1')
+  // Refused with the same sentence a name that does not exist gets: the answer may not
+  // confirm that a Bot the actor cannot see exists (boundary 1).
+  assert.equal(answer, 'No Bot named Van.')
+  assert.equal(state().bots.x1.instructions, 'Van instructions')
+  assert.equal(state().bots.x1.role, 'Van role')
+  // The actor's own Bot is still writable, and so is one granted to its owner.
+  assert.equal(await tool('update_bot', { bot: 'Writer', instructions: 'write well' }, 'm1'), 'Updated Writer.')
+  assert.equal(state().bots.b2.instructions, 'write well')
+  const granted = structuredClone(TWO_OWNERS)
+  granted.grants = { x1: [{ grantee: 'owner-1' }] }
+  const second = start(granted)
+  t.after(second.stop)
+  assert.equal(await second.tool('update_bot', { bot: 'Van', instructions: 'delegated' }, 'm1'), 'Updated Van.')
+  assert.equal(second.state().bots.x1.instructions, 'delegated')
+})
+
+test('#1844: a missing name and another owner\'s Bot answer the same sentence', async (t) => {
+  const withoutVan = structuredClone(TWO_OWNERS)
+  delete withoutVan.bots.x1
+  const missing = start(withoutVan)
+  t.after(missing.stop)
+  const foreign = start(TWO_OWNERS)
+  t.after(foreign.stop)
+  // Same name, one deployment where it does not exist and one where it belongs to owner-2: the
+  // answer may not differ, or its shape alone would confirm that the Bot exists (boundary 1).
+  const missingText = await missing.tool('update_bot', { bot: 'Van' }, 'm1')
+  const foreignText = await foreign.tool('update_bot', { bot: 'Van' }, 'm1')
+  assert.equal(missingText, 'No Bot named Van.')
+  assert.equal(foreignText, missingText)
+  assert.doesNotMatch(foreignText, /Team:/)
+})
+
+test('#1844: with authz off the write surface behaves exactly as before', async (t) => {
+  // A single-owner install has no auth session, so `authzEnforced()` is false: the owner
+  // rule must not appear, and the same call the tests above always made still works.
+  const { tool, state, stop } = start()
+  t.after(stop)
+  assert.equal(state().authSessions, undefined)
+  assert.equal(await tool('update_bot', { bot: 'Writer', instructions: 'single owner' }, 'm1'), 'Updated Writer.')
+  assert.equal(state().bots.b2.instructions, 'single owner')
+})