fix(host): resolve wire ids from their table's own keys only

changelog.d/1711-own-key-lookups.security.md

@@ -0,0 +1 @@
+Host lookups that index a saved table with an id or name from the request now read only that table's own keys, so `__proto__` is no longer a Bot, a conversation, a task or a capability: `update-bot`, `task-update` and `set-flags` can no longer write a field onto `Object.prototype`, `set-main` no longer persists it as a Main Bot, and `capability-delete` refuses a name that does not exist instead of answering success, an invitation revoke no longer writes `revokedAt` onto `Object.prototype`, a member removal refuses an id that is not an own member, and a grants write for a nonexistent Bot is refused instead of re-pointing the grants table ([#1711](https://github.com/ThinkFlowLab/FlyBot/issues/1711)).

core/src/host/capabilities.js

@@ -1,5 +1,6 @@
 import { createHash } from 'node:crypto'
 import { RESERVED_NAMES } from './connectors.js'
+import { ownEntry } from './store.js'
 
 const MCP_CLIENT = '@deepseek-ai/dsh-mcp-client'
 const NAME = /^[a-z][a-z0-9]*(?:-[a-z0-9]+)*$/
@@ -140,7 +141,7 @@ export function install(rt) {
     await rt.load()
     writeCheck()
     const items = ensureLibrary()
-    const item = items[String(name ?? '')]
+    const item = ownEntry(items, String(name ?? ''))
     if (!item) throw new Error(`No capability named ${name}.`)
     if (!ui) editCheck(item, botId, 'delete')
     delete items[item.name]
@@ -157,7 +158,7 @@ export function install(rt) {
     const target = targetRef ? rt.findBot(targetRef) : caller
     if (!target) throw new Error(`No Bot named ${targetRef}.`)
     if (target.id !== caller.id && !rt.isMain(callerId)) throw new Error('Only a Main Bot can change another Bot’s capabilities.')
-    const item = ensureLibrary()[String(name ?? '')]
+    const item = ownEntry(ensureLibrary(), String(name ?? ''))
     if (!item) throw new Error(`No capability named ${name}.`)
     await setAttached(target, item.name, endpoint === 'attach')
     return { bot: target.name, attached: endpoint === 'attach' }
@@ -455,7 +456,7 @@ export function install(rt) {
     if (endpoint === 'capability-approve') {
       await rt.load()
       writeCheck()
-      const item = library()[String(body.name ?? '')]
+      const item = ownEntry(library(), String(body.name ?? ''))
       if (!item || item.kind !== 'mcp' || item.transport !== 'stdio') throw new Error(`No unapproved stdio MCP named ${body.name}.`)
       item.approved = true
       item.updatedAt = Date.now()
@@ -468,7 +469,7 @@ export function install(rt) {
       const bot = rt.botOf(String(body.botId ?? ''))
       if (!bot) throw new Error('Unknown Bot')
       const name = String(body.name ?? '')
-      if (!library()[name]) throw new Error(`No capability named ${name}.`)
+      if (!ownEntry(library(), name)) throw new Error(`No capability named ${name}.`)
       return setAttached(bot, name, body.attached === true)
     }
     throw new Error('Unknown capability operation.')

core/src/host/member-auth.js

@@ -5,6 +5,7 @@
 // This is not team mode: it is the sign-in surface the gates in docs/subsystems/team.md
 // need, and nothing more.
 import { createHash, randomBytes } from 'node:crypto'
+import { ownEntry } from './store.js'
 
 const DEFAULT_TTL_MS = 7 * 24 * 60 * 60 * 1000
 
@@ -63,7 +64,7 @@ export function install(rt) {
 
   rt.revokeInvitation = async function revokeInvitation(invitationId) {
     return withAuthLock(async () => {
-      const inv = state.invitations[invitationId]
+      const inv = ownEntry(state.invitations, invitationId)
       if (!inv) return { ok: false, error: 'not_found' }
       if (inv.redeemedAt) return { ok: false, error: 'already_redeemed' }
       if (inv.revokedAt) return { ok: false, error: 'already_revoked' }
@@ -162,7 +163,7 @@ export function install(rt) {
   rt.removeMember = async function removeMember(memberId) {
     return withAuthLock(async () => {
       const members = rt.state.members || {}
-      const member = members[memberId]
+      const member = ownEntry(members, memberId)
       if (!member) return { ok: false, error: 'not_found' }
       if (lastOwner(members)) return { ok: false, error: 'last_owner' }
       const bots = Object.values(rt.state.bots || {})

core/src/host/reach.js

@@ -1,3 +1,5 @@
+import { ownEntry } from './store.js'
+
 // Reach policy (#90): who may be reached from outside their owner. One owner- and
 // grant-aware check that sits OUTSIDE the plugin send chain, so a middleware's
 // `allow` or `bypass` verdict can never skip it. hooks.js runs it before bot.send's
@@ -73,7 +75,8 @@ function peerKeys(peer) {
 }
 
 export function grantsOf(state, botId) {
-  const list = state?.grants?.[botId]
+  const table = state?.grants
+  const list = table ? ownEntry(table, botId) : undefined
   return Array.isArray(list) ? list : []
 }
 
@@ -389,7 +392,7 @@ export function install(rt) {
   rt.ticketPeers = () => ticketPeers(state)
 
   rt.setGrants = async (botId, list) => {
-    const bot = state.bots?.[botId]
+    const bot = ownEntry(state.bots ?? {}, botId)
     if (bot === undefined || bot.remote !== undefined) throw Object.assign(new Error('Unknown Bot'), { code: 'bot/not-found' })
     if (!Array.isArray(list)) throw Object.assign(new Error('grants must be an array'), { code: 'bot/invalid' })
     const next = list.map((grant, i) => {

core/src/host/roster.js

@@ -6,6 +6,7 @@ import { MAX_AVATAR_IMAGE, ROOM_MAX_MEMBERS, ROOM_MODES, ROOM_NAME_MAX, ROOM_NOT
 import { currentColor, isColor, unquote } from './text.js'
 import { nameProblem } from '../shared/mentions.js'
 import { cleanExternal } from './room-participant-external.js'
+import { ownEntry } from './store.js'
 
 export function install(rt) {
   const { ctx, config, home, defaults, state } = rt
@@ -240,7 +241,7 @@ export function install(rt) {
   }
 
   async function setFlags(id, { pinned, hidden }) {
-    const entry = rt.botOf(id) ?? rt.roomOf(id) ?? state.agents[id]
+    const entry = rt.botOf(id) ?? rt.roomOf(id) ?? ownEntry(state.agents, id)
     if (entry === undefined) throw new Error('Unknown conversation')
     if (typeof pinned === 'boolean') entry.pinned = pinned
     if (typeof hidden === 'boolean') entry.hidden = hidden

core/src/host/store.js

@@ -18,13 +18,12 @@ const STATE_VERSION = 2
 // The immutable id of the one owner a fresh or legacy team starts with.
 const OWNER_ID = 'owner-1'
 const isRecord = value => value !== null && typeof value === 'object' && !Array.isArray(value)
-// Ids arrive from the wire and the saved tables are plain objects, so `table[id]` answers
-// `Object.prototype` for `__proto__` (and `isRecord` accepts it): one own-key read for every
-// id lookup in this file, and a write can never reach a prototype either (#1704).
-const ownEntry = (table, id) => (Object.hasOwn(table, id) ? table[id] : undefined)
-// `table['__proto__'] = value` is swallowed by the prototype setter, so a binding written that
-// way would answer `ok: true` and store nothing: a write defines an own entry instead.
-const setEntry = (table, id, value) => Object.defineProperty(table, id, { value, writable: true, enumerable: true, configurable: true })
+// Ids and names arrive from the wire, and the saved tables are plain objects, so `table[id]`
+// answers `Object.prototype` for `__proto__` (and `isRecord` accepts it): every id lookup in
+// the host reads its table through this one own-key check, and a write defines an own entry so
+// the prototype setter never swallows it (#1704, #1711).
+export const ownEntry = (table, id) => (Object.hasOwn(table, id) ? table[id] : undefined)
+export const setEntry = (table, id, value) => Object.defineProperty(table, id, { value, writable: true, enumerable: true, configurable: true })
 
 export function install(rt) {
   const { ctx, home, statePath, config = {} } = rt
@@ -656,8 +655,8 @@ export function install(rt) {
     return writing.finally(() => lock.release())
   }, 'ds-bot: state lock')
 
-  const botOf = id => (id === undefined ? undefined : state.bots[id])
-  const roomOf = id => (id === undefined ? undefined : state.rooms[id])
+  const botOf = id => (id === undefined ? undefined : ownEntry(state.bots, id))
+  const roomOf = id => (id === undefined ? undefined : ownEntry(state.rooms, id))
   const isMain = id => id !== undefined && state.mainBotIds.includes(id)
 
   // Group Sessions. A member speaks in each group from a hidden Session of its own,

core/src/host/tasks.js

@@ -19,6 +19,7 @@ import { randomBytes } from 'node:crypto'
 import { columnOf, compareForQueue, compareForWork, sameRole } from '../shared/task-board.js'
 import { currentParticipants, deriveParticipants } from './room-membership.js'
 import { cardNamesIssue } from '../shared/issue-ref.js'
+import { ownEntry } from './store.js'
 
 const ACTIVE = new Set(['open', 'claimed', 'working', 'blocked'])
 // result-ready still waits for the requester: prune must not evict it with terminals (#111).
@@ -174,7 +175,7 @@ export function install(rt) {
     return () => { projectProviders.delete(fn) }
   }
   rt.projectRunRefs = ({ hasTask } = {}) => {
-    const exists = typeof hasTask === 'function' ? hasTask : taskId => store()[String(taskId ?? '')] !== undefined
+    const exists = typeof hasTask === 'function' ? hasTask : taskId => ownEntry(store(), String(taskId ?? '')) !== undefined
     const refs = []
     const seen = new Set()
     for (const fn of [...projectProviders]) {
@@ -203,7 +204,7 @@ export function install(rt) {
     if (!rt.authzEnforced?.()) return true
     return actor !== null && actor !== undefined && task.requesterId === actor.memberId
   }
-  const find = id => store()[String(id ?? '')]
+  const find = id => ownEntry(store(), String(id ?? ''))
   // Only the record's own requesters may move their tasks; the responsible Bot is
   // checked at create time (owner, grant, or shared-room membership).
   const requireRequester = (task, actor) => {

core/test/own-keys.test.js

@@ -0,0 +1,199 @@
+// Host lookups that index a plain state table with a wire id (#1711). `table[id]` answers
+// `Object.prototype` for `__proto__`, which a request can then write through — so an id that is
+// not an own key must resolve as absent, and a refusal must not touch `Object.prototype`.
+// The assertions read the prototype itself: `Object.hasOwn(Object.prototype, key)` and
+// `key in {}`. `Object.hasOwn({}, key)` is false in the polluted state too, so it is no evidence.
+import assert from 'node:assert/strict'
+import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'
+import { tmpdir } from 'node:os'
+import { join } from 'node:path'
+import { test } from 'node:test'
+import { apply } from '../index.js'
+import { install as installMemberAuth } from '../src/host/member-auth.js'
+import { install as installReach } from '../src/host/reach.js'
+import { fakeContext } from './fake-ctx.js'
+
+const seed = () => ({
+  version: 2,
+  revision: 1,
+  mainBotIds: ['a'],
+  members: { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } },
+  bots: {
+    a: { id: 'a', name: 'Chief', role: 'Boss', sessionId: 'a', createdAt: 1, ownerId: 'owner-1' },
+    b: { id: 'b', name: 'Writer', role: 'Write', sessionId: 'b', createdAt: 2, ownerId: 'owner-1' },
+  },
+  rooms: { r1: { id: 'r1', name: 'Ops', members: ['a', 'b'], mode: 'admin', admin: 'a', named: true, sessions: {}, log: [], seq: 0, seen: {} } },
+  agents: {},
+  capabilities: {
+    items: {
+      echo: { name: 'echo', kind: 'mcp', description: 'Echo service', transport: 'stdio', command: 'node', args: [], env: {}, approved: true, createdAt: 1, updatedAt: 1 },
+    },
+  },
+  tasks: {},
+  projects: {},
+  projectRuns: {},
+  exchanges: [],
+  exchangeTurns: {},
+  questions: {},
+  prefs: {},
+  grants: {},
+})
+
+async function start(home = mkdtempSync(join(tmpdir(), 'ds-bot-own-keys-'))) {
+  if (!existsSync(join(home, 'state.json'))) writeFileSync(join(home, 'state.json'), JSON.stringify(seed()))
+  const ctx = fakeContext()
+  const agents = new Map()
+  const agentOf = (sessionId) => {
+    if (!agents.has(sessionId)) agents.set(sessionId, { session: { id: sessionId }, followup() {}, steer() {}, inject() {}, cancel() {}, whenIdle() {}, inbox: { nextStep: [], nextTurn: [], remove() {} } })
+    return agents.get(sessionId)
+  }
+  let route
+  Object.assign(ctx, {
+    llm: { listProviders: () => [], listModels: async () => [], resolveModelInfo: async () => ({}) },
+    workspaceRegistry: { get: () => undefined, resolveByPath: async () => undefined, create: async path => ({ id: 'w1', path }), archiveSession: async () => {} },
+    sessionController: { create: async () => ({ sessionId: 's1' }), resolveAgent: async id => ({ agent: agentOf(id) }), selectModel: async () => {} },
+    sessionTitle: { rename: async () => {} },
+    sessions: { flush: async () => {} },
+  })
+  const register = ctx.connection.fetch.register
+  ctx.connection.fetch.register = (options) => { route = options; return register(options) }
+  apply(ctx, { home })
+  const call = async (endpoint, payload) => (await route.fetch(new Request('http://localhost/api/bot', { method: 'POST', body: JSON.stringify({ endpoint, payload }) }))).json()
+  const saved = () => JSON.parse(readFileSync(join(home, 'state.json'), 'utf8'))
+  const stop = async ({ keepHome = false } = {}) => {
+    await ctx.dispose()
+    if (!keepHome) rmSync(home, { recursive: true, force: true, maxRetries: 5 })
+  }
+  return { call, saved, stop, home }
+}
+
+const value = (response) => {
+  assert.equal(response.ok, true, JSON.stringify(response))
+  return response.value
+}
+
+// Reads the pollution a call left, then removes it so one red test cannot fail the next.
+const leak = (key) => {
+  const own = Object.hasOwn(Object.prototype, key)
+  const written = Object.prototype[key]
+  delete Object.prototype[key]
+  return { own, written, inherited: key in {} }
+}
+
+const noLeak = (key, { own, written, inherited }) => {
+  assert.equal(own, false, `Object.prototype has own '${key}' = ${JSON.stringify(written)} after the call`)
+  assert.equal(inherited, false, `'${key}' in {} after the call`)
+}
+
+test('update-bot with a wire id writes nothing onto Object.prototype (#1711)', async () => {
+  const { call, stop } = await start()
+  try {
+    const response = await call('update-bot', { id: '__proto__', role: 'Intruder' })
+    noLeak('role', leak('role'))
+    assert.equal(response.ok, false, `update-bot for '__proto__' must answer a refusal, got ${JSON.stringify(response)}`)
+    assert.match(String(response.error?.message), /Unknown Bot/)
+
+    // A real Bot still updates.
+    assert.equal(value(await call('update-bot', { id: 'b', role: 'Editor' })).role, 'Editor')
+  } finally { await stop() }
+})
+
+test('task-update with a wire id writes nothing onto Object.prototype (#1711)', async () => {
+  const { call, stop } = await start()
+  try {
+    const response = await call('task-update', { id: '__proto__', title: 'Stolen title' })
+    noLeak('title', leak('title'))
+    assert.equal(response.ok, false, `task-update for '__proto__' must answer a refusal, got ${JSON.stringify(response)}`)
+
+    // A real task still updates.
+    const created = value(await call('task-create', { botId: 'b', title: 'Write it down' }))
+    const updated = value(await call('task-update', { id: created.id, title: 'Write it down twice' }))
+    assert.equal(updated.title, 'Write it down twice')
+  } finally { await stop() }
+})
+
+test('capability-delete refuses a name that is not an own key (#1711)', async () => {
+  const { call, stop } = await start()
+  try {
+    for (const name of ['__proto__', 'constructor']) {
+      const response = await call('capability-delete', { name })
+      assert.equal(response.ok, false, `capability-delete ${name} must answer a refusal, got ${JSON.stringify(response)}`)
+      assert.match(String(response.error?.message), /No capability named/)
+    }
+    // A real capability still deletes.
+    assert.equal(value(await call('capability-delete', { name: 'echo' })), true)
+  } finally { await stop() }
+})
+
+test('set-main with a wire id is not a Main Bot and is not persisted (#1711)', async () => {
+  const { call, saved, stop } = await start()
+  try {
+    const response = await call('set-main', { id: '__proto__', main: true })
+    assert.equal(response.ok, false, `set-main for '__proto__' must answer a refusal, got ${JSON.stringify(response)}`)
+    assert.match(String(response.error?.message), /Unknown Bot/)
+    assert.deepEqual(saved().mainBotIds, ['a'], 'a wire id must not reach state.mainBotIds')
+
+    // A real Bot still becomes a Main Bot.
+    value(await call('set-main', { id: 'b', main: true }))
+    assert.deepEqual(saved().mainBotIds, ['a', 'b'])
+  } finally { await stop() }
+})
+
+test('set-flags with a wire id writes nothing onto Object.prototype (#1711)', async () => {
+  const { call, stop } = await start()
+  try {
+    const response = await call('set-flags', { id: '__proto__', pinned: true })
+    noLeak('pinned', leak('pinned'))
+    assert.equal(response.ok, false, `set-flags for '__proto__' must answer a refusal, got ${JSON.stringify(response)}`)
+
+    // A real conversation still takes its flags.
+    assert.equal(value(await call('set-flags', { id: 'b', pinned: true })).pinned, true)
+  } finally { await stop() }
+})
+
+// The member-auth and reach tables are written by the same wire ids (#1711, Security
+// advisory follow-up): `state.invitations[id]` and `state.members[id]` answer
+// `Object.prototype`, so a revoke lands `revokedAt` on the prototype and a remove answers
+// `ok` for a member that does not exist; `state.grants[id]` writes the grants table.
+
+const authHost = (members = { 'owner-1': { id: 'owner-1', createdAt: 1, designatedChiefId: 'a' } }) => {
+  const state = { invitations: {}, authSessions: {}, members, bots: {} }
+  const rt = { state, save: async () => {} }
+  installMemberAuth(rt)
+  return rt
+}
+
+test('auth-revoke-invite with a wire id writes nothing onto Object.prototype (#1711)', async () => {
+  const rt = authHost()
+  const response = await rt.revokeInvitation('__proto__')
+  noLeak('revokedAt', leak('revokedAt'))
+  assert.deepEqual(response, { ok: false, error: 'not_found' }, `revoke for '__proto__' must be not_found, got ${JSON.stringify(response)}`)
+
+  // A real invitation still revokes.
+  const invite = await rt.createInvitation()
+  assert.deepEqual(await rt.revokeInvitation(invite.id), { ok: true })
+})
+
+test('auth-remove-member refuses a member id that is not an own key (#1711)', async () => {
+  const rt = authHost({ 'owner-1': { id: 'owner-1' }, 'owner-2': { id: 'owner-2' } })
+  const response = await rt.removeMember('__proto__')
+  assert.deepEqual(response, { ok: false, error: 'not_found' }, `remove of '__proto__' must be not_found, got ${JSON.stringify(response)}`)
+  assert.deepEqual(Object.keys(rt.state.members).sort(), ['owner-1', 'owner-2'])
+
+  // A real member still removes.
+  assert.deepEqual(await rt.removeMember('owner-2'), { ok: true })
+})
+
+test('setGrants with a wire id refuses instead of writing the grants table (#1711)', async () => {
+  const state = { bots: { a: { id: 'a', name: 'Chief', ownerId: 'owner-1' } }, grants: {} }
+  const rt = { state, save: async () => {}, reachChanged: async () => {}, refreshConnectorTools: () => {} }
+  installReach(rt)
+  let refused
+  try { await rt.setGrants('__proto__', [{ grantee: '@friend.home' }]) } catch (error) { refused = error }
+  assert.equal(refused?.code, 'bot/not-found', `grants for '__proto__' must be refused, got ${refused ? refused.message : 'a stored list'}`)
+  assert.equal(Object.hasOwn(state.grants, '__proto__'), false)
+  assert.equal(Object.getPrototypeOf(state.grants), Object.prototype, "the grants table must keep Object.prototype, not become the wire value")
+
+  // A real Bot still stores its grants.
+  assert.equal((await rt.setGrants('a', [{ grantee: '@friend.home' }])).length, 1)
+})