fix(host): stop Project ids from resolving through the prototype chain

changelog.d/1704-proto-safe-project-ids.fixed.md

@@ -0,0 +1 @@
+Project and roster ids resolve only from their table's own keys: `project-read {id: "__proto__"}`, `project-bind {kind: "bot", id: "__proto__"}` and `project-of` no longer answer from `Object.prototype`, a binding request can no longer write `projectId` onto it (which made every unbound Bot and group report one Project until the next restart), and `findBot` / `findRooms` stop returning it as a member or a group ([#1704](https://github.com/ThinkFlowLab/FlyBot/issues/1704)).

core/src/host/store.js

@@ -18,6 +18,13 @@ 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 })
 
 export function install(rt) {
   const { ctx, home, statePath, config = {} } = rt
@@ -361,20 +368,26 @@ export function install(rt) {
     error.code = code
     return error
   }
-  const projectOfId = id => (typeof id === 'string' && isRecord(state.projects[id]) ? state.projects[id] : null)
+  const projectOfId = (id) => {
+    if (typeof id !== 'string') return null
+    const project = ownEntry(state.projects, id)
+    return isRecord(project) ? project : null
+  }
   const requireProject = (id) => {
     const project = projectOfId(id)
     if (project === null) throw projectRefusal(`Unknown Project ${String(id ?? '')}`, 'bot/not-found')
     return project
   }
   const projectMemberOf = (kind, id) => {
-    if (kind === 'bot') return isRecord(state.bots[id]) ? state.bots[id] : null
-    if (kind === 'group') return isRecord(state.rooms[id]) ? state.rooms[id] : null
+    if (kind === 'bot' || kind === 'group') {
+      const member = ownEntry(kind === 'bot' ? state.bots : state.rooms, id)
+      return isRecord(member) ? member : null
+    }
     if (kind === 'run') return null
     throw projectRefusal(`Unknown Project member kind ${String(kind ?? '')}`)
   }
   const projectOfMember = (kind, id) => {
-    if (kind === 'run') return projectOfId(state.projectRuns[String(id ?? '')])
+    if (kind === 'run') return projectOfId(ownEntry(state.projectRuns, String(id ?? '')))
     const member = projectMemberOf(kind, id)
     return member === null ? null : projectOfId(member.projectId)
   }
@@ -410,7 +423,7 @@ export function install(rt) {
     const project = body.projectId === undefined || body.projectId === null ? null : requireProject(projectText(body.projectId, 200))
     if (kind === 'run') {
       if (project === null) delete state.projectRuns[id]
-      else state.projectRuns[id] = project.id
+      else setEntry(state.projectRuns, id, project.id)
       return project
     }
     const member = projectMemberOf(kind, id)
@@ -709,15 +722,17 @@ export function install(rt) {
   }
   const findBot = (ref) => {
     if (ref === undefined) return undefined
-    if (state.bots[ref]) return state.bots[ref].remote === undefined ? state.bots[ref] : undefined
+    const direct = ownEntry(state.bots, ref)
+    if (direct) return direct.remote === undefined ? direct : undefined
     const wanted = normalize(ref).replace(/^@/, '')
     // A friend's Bot (remote chat) is no team member, so a name never finds it.
     return Object.values(state.bots).find(bot => bot.remote === undefined && normalize(bot.name) === wanted)
   }
   // Every group chat whose id or name matches; callers refuse an ambiguous name.
   const findRooms = (ref) => {
     if (ref === undefined || ref === null) return []
-    if (state.rooms[ref]) return [state.rooms[ref]]
+    const direct = ownEntry(state.rooms, ref)
+    if (direct) return [direct]
     const wanted = normalize(unquote(ref))
     return Object.values(state.rooms).filter(room => normalize(room.name) === wanted)
   }

core/test/projects.test.js

@@ -143,3 +143,69 @@ test('the record and its bindings survive a store re-open', async () => {
     rmSync(home, { recursive: true, force: true, maxRetries: 5 })
   }
 })
+
+// #1704: the id tables are plain objects, so `state.projects['__proto__']` and
+// `state.bots['__proto__']` answered `Object.prototype` (which `isRecord` accepts). An id that
+// is not an own key must resolve as absent, and no request may write `Object.prototype`.
+test('an id that is not an own key resolves as absent and never writes Object.prototype (#1704)', async () => {
+  const { call, stop, home } = await start()
+  try {
+    const project = created(await call('project-create', { name: 'FlyBot', repository: 'ThinkFlowLab/FlyBot' }))
+    const protoRead = await call('project-read', { id: '__proto__' })
+    const protoBind = await call('project-bind', { kind: 'bot', id: '__proto__', projectId: project.id })
+    const protoOf = await call('project-of', { kind: 'bot', id: '__proto__' })
+    const unboundBot = await call('project-of', { kind: 'bot', id: 'b' })
+    const unboundGroup = await call('project-of', { kind: 'group', id: 'r1' })
+    const saved = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8'))
+    const observed = JSON.stringify({
+      'project-read __proto__': protoRead.value?.project ?? protoRead,
+      'project-bind __proto__': protoBind,
+      'project-of bot __proto__': protoOf.value?.project ?? protoOf,
+      'project-of unbound bot b': unboundBot.value?.project ?? unboundBot,
+      'project-of unbound group r1': unboundGroup.value?.project ?? unboundGroup,
+      'Object.prototype has own projectId': Object.hasOwn(Object.prototype, 'projectId'),
+      '({}).projectId': ({}).projectId ?? null,
+      'state.json bots.b has own projectId': Object.hasOwn(saved.bots.b, 'projectId'),
+    })
+    assert.deepEqual(protoRead.value, { project: null }, observed)
+    assert.equal(protoBind.ok, false, observed)
+    assert.equal(protoBind.error?.code, 'bot/not-found', observed)
+    assert.deepEqual(protoOf.value, { project: null }, observed)
+    assert.deepEqual(unboundBot.value, { project: null }, observed)
+    assert.deepEqual(unboundGroup.value, { project: null }, observed)
+    assert.equal(Object.hasOwn(Object.prototype, 'projectId'), false, observed)
+    assert.equal(({}).projectId, undefined, observed)
+    assert.equal(Object.hasOwn(saved.bots.b, 'projectId'), false, observed)
+  } finally { await stop() }
+})
+
+// #1704, the run half (#1708): a run's binding lives in `state.projectRuns`, a plain object, so
+// `state.projectRuns['__proto__'] = id` was taken by the `__proto__` setter: the request answered
+// `ok: true` and nothing was stored, and `project-of {kind:'run'}` kept answering null. The write
+// now defines an own key, which is what the assertions below read; a plain assignment (or
+// `Reflect.set`) fails them even though the response still says ok.
+test('a run binds under an own key, not through the prototype chain (#1704)', async () => {
+  const { call, stop, home } = await start()
+  try {
+    const project = created(await call('project-create', { name: 'FlyBot', repository: 'ThinkFlowLab/FlyBot' }))
+    const bound = await call('project-bind', { kind: 'run', id: '__proto__', projectId: project.id })
+    const of = await call('project-of', { kind: 'run', id: '__proto__' })
+    const saved = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8'))
+    const observed = JSON.stringify({
+      'project-bind run __proto__': bound,
+      'project-of run __proto__': of,
+      'state.projectRuns own __proto__': Object.hasOwn(saved.projectRuns, '__proto__'),
+      'state.projectRuns __proto__': saved.projectRuns['__proto__'] ?? null,
+      'state.projectRuns prototype is Object.prototype': Object.getPrototypeOf(saved.projectRuns) === Object.prototype,
+      'Object.prototype has own projectId': Object.hasOwn(Object.prototype, 'projectId'),
+      "'projectId' in {}": 'projectId' in {},
+    })
+    assert.equal(bound.ok, true, observed)
+    assert.equal(Object.hasOwn(saved.projectRuns, '__proto__'), true, observed)
+    assert.equal(saved.projectRuns['__proto__'], project.id, observed)
+    assert.equal(Object.getPrototypeOf(saved.projectRuns), Object.prototype, observed)
+    assert.equal(Object.hasOwn(Object.prototype, 'projectId'), false, observed)
+    assert.equal('projectId' in {}, false, observed)
+    assert.equal(value(of).project?.id, project.id, observed)
+  } finally { await stop() }
+})

core/test/store.test.js

@@ -302,3 +302,21 @@ test('a member that loses its seat closes its external process (#198)', async (t
 function ctxRegistry(rt) {
   rt.ctx.workspaceRegistry = { archiveSession: async () => {} }
 }
+
+// #1704: a roster id that is not an own key of its table is not a member either.
+test('a roster id that is not an own key finds no Bot and no group (#1704)', async (t) => {
+  const { rt } = runtime(t, {
+    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' } },
+    rooms: { r1: { id: 'r1', name: 'Ops', members: ['a'], mode: 'admin', admin: 'a', named: true, sessions: {}, log: [], seq: 0, seen: {} } },
+    agents: {}, exchanges: [],
+  })
+  await rt.load()
+  assert.equal(rt.findBot('__proto__'), undefined, 'findBot(__proto__) answered Object.prototype')
+  assert.deepEqual(rt.findRooms('__proto__'), [], 'findRooms(__proto__) answered Object.prototype')
+  assert.equal(rt.findBot('a').name, 'Chief')
+  assert.equal(rt.findRooms('Ops')[0].id, 'r1')
+})

patches/ds-bot/0002-flybot-api-host.patch

@@ -1,3 +1,3 @@
 FlyBot patch 0002-flybot-api-host on Fei2411/DeepSeek-Bot@2b10c70 (ds-bot/). Generated by scripts/ds-bot-patches.mjs; edits to upstream files only (files FlyBot adds are in files/).
 
 diff --git a/index.js b/index.js
@@ -620,7 +620,7 @@
 diff --git a/src/host/store.js b/src/host/store.js
 --- a/src/host/store.js
 +++ b/src/host/store.js
-@@ -1,21 +1,39 @@
+@@ -1,21 +1,46 @@
  // Store: the saved team (state.json), its migrations, the lock that keeps a second dsh
  // host on the same data from writing it, and the indexes that map a Session to the Bot
  // or group chat it belongs to.
@@ -642,6 +642,13 @@
 +// 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 })
 +
  export function install(rt) {
 -  const { ctx, home, statePath } = rt
@@ -664,7 +671,7 @@
 
    const EMPTY = JSON.stringify(state)
    // The other dsh host that holds state.lock while this one only reads (see home-lock.js);
-@@ -50,12 +50,88 @@ export function install(rt) {
+@@ -50,12 +50,88 @@
      }
    }
    const readOnly = () => {
@@ -754,7 +761,7 @@
    const readFileState = async () => {
      let contents
      try {
-@@ -87,10 +87,32 @@ export function install(rt) {
+@@ -87,10 +87,32 @@
        return undefined
      }
      if (parsed?.version === 1) {
@@ -770,25 +777,25 @@
 +        warn('ds-bot: unreadable state %s: %s', statePath, backupProblem)
 +        return undefined
 +      }
-       stuck = undefined
-       return parsed
-     }
--    stuck = 'saved by a newer DS Bot; update DS Bot'
++      stuck = undefined
++      return parsed
++    }
 +    if (parsed?.version === STATE_VERSION) {
 +      const problem = ownershipProblem(parsed)
 +      if (problem !== undefined) {
 +        stuck = `malformed ownership: ${problem}`
 +        warn('ds-bot: state %s was refused: %s', statePath, problem)
 +        return undefined
 +      }
-+      stuck = undefined
-+      return parsed
-+    }
+       stuck = undefined
+       return parsed
+     }
+-    stuck = 'saved by a newer DS Bot; update DS Bot'
 +    stuck = 'saved by a newer FlyBot; update FlyBot'
      return undefined
    }
 
-@@ -104,6 +104,14 @@ export function install(rt) {
+@@ -104,6 +104,14 @@
      // Teams saved before the DSH Agent existed carry no `agents`; corrupt values reset.
      if (state.agents === null || typeof state.agents !== 'object' || Array.isArray(state.agents)) state.agents = {}
      let migrated = !hadCapabilities
@@ -803,7 +810,7 @@
      if (state.capabilities === null || typeof state.capabilities !== 'object' || Array.isArray(state.capabilities)) {
        state.capabilities = { items: {} }
        migrated = true
-@@ -112,6 +112,11 @@ export function install(rt) {
+@@ -112,6 +112,11 @@
        migrated = true
      }
      for (const bot of Object.values(state.bots)) {
@@ -815,7 +822,7 @@
        if (!Array.isArray(bot.capabilities)) {
          bot.capabilities = []
          migrated = true
-@@ -120,6 +120,11 @@ export function install(rt) {
+@@ -120,6 +120,11 @@
          bot.color = currentColor(bot.color)
          migrated = true
        }
@@ -827,7 +834,7 @@
        // LEGACY: Bots saved with a lab ran on that lab's default model; the model they
        // last ran on becomes their own, so dropping the lab changes nothing for them.
        if ('lab' in bot) {
-@@ -161,12 +161,182 @@ export function install(rt) {
+@@ -161,12 +161,188 @@
        migrated = true
      }
      // Unnamed groups from before names followed their members may still list old names.
@@ -898,20 +905,26 @@
 +    error.code = code
 +    return error
 +  }
-+  const projectOfId = id => (typeof id === 'string' && isRecord(state.projects[id]) ? state.projects[id] : null)
++  const projectOfId = (id) => {
++    if (typeof id !== 'string') return null
++    const project = ownEntry(state.projects, id)
++    return isRecord(project) ? project : null
++  }
 +  const requireProject = (id) => {
 +    const project = projectOfId(id)
 +    if (project === null) throw projectRefusal(`Unknown Project ${String(id ?? '')}`, 'bot/not-found')
 +    return project
 +  }
 +  const projectMemberOf = (kind, id) => {
-+    if (kind === 'bot') return isRecord(state.bots[id]) ? state.bots[id] : null
-+    if (kind === 'group') return isRecord(state.rooms[id]) ? state.rooms[id] : null
++    if (kind === 'bot' || kind === 'group') {
++      const member = ownEntry(kind === 'bot' ? state.bots : state.rooms, id)
++      return isRecord(member) ? member : null
++    }
 +    if (kind === 'run') return null
 +    throw projectRefusal(`Unknown Project member kind ${String(kind ?? '')}`)
 +  }
 +  const projectOfMember = (kind, id) => {
-+    if (kind === 'run') return projectOfId(state.projectRuns[String(id ?? '')])
++    if (kind === 'run') return projectOfId(ownEntry(state.projectRuns, String(id ?? '')))
 +    const member = projectMemberOf(kind, id)
 +    return member === null ? null : projectOfId(member.projectId)
 +  }
@@ -947,7 +960,7 @@
 +    const project = body.projectId === undefined || body.projectId === null ? null : requireProject(projectText(body.projectId, 200))
 +    if (kind === 'run') {
 +      if (project === null) delete state.projectRuns[id]
-+      else state.projectRuns[id] = project.id
++      else setEntry(state.projectRuns, id, project.id)
 +      return project
 +    }
 +    const member = projectMemberOf(kind, id)
@@ -1011,7 +1024,7 @@
    const load = () => loaded ??= (async () => {
      claimLock()
      if (holder !== undefined) warn('ds-bot: %s', readOnly())
-@@ -185,14 +185,24 @@ export function install(rt) {
+@@ -185,14 +185,24 @@
      for (const room of stale) void rt.retitleRoom(room)
    })()
 
@@ -1037,7 +1050,7 @@
          warnedReadOnly = false
          adopt(await readFileState())
          return
-@@ -235,11 +235,37 @@ export function install(rt) {
+@@ -235,11 +235,37 @@
      return warnReadOnly()
    }
 
@@ -1075,7 +1088,7 @@
      state.revision += 1
      state.exchanges = state.exchanges.slice(-MAX_EXCHANGES)
      const snapshot = JSON.stringify(state, null, 2)
-@@ -260,9 +260,10 @@ export function install(rt) {
+@@ -260,9 +260,10 @@
          ensureLock()
          await rename(temp, statePath)
          seen = stamp()
@@ -1087,7 +1100,7 @@
          throw error
        }
      })
-@@ -273,6 +273,30 @@ export function install(rt) {
+@@ -273,6 +273,30 @@
      })
    }
 
@@ -1118,7 +1131,7 @@
    ctx.effect(() => () => {
      stopped = true
      return writing.finally(() => lock.release())
-@@ -313,6 +313,13 @@ export function install(rt) {
+@@ -313,6 +313,13 @@
        }
        for (const [botId, sessionId] of Object.entries(room.sessions ?? {})) groupOwners.set(sessionId, { botId, roomId: room.id })
      }
@@ -1132,7 +1145,7 @@
      // The agents module owns the DSH Agent session index; it installs after this.
      rt.indexAgents?.()
    }
-@@ -328,6 +328,8 @@ export function install(rt) {
+@@ -328,6 +328,8 @@
      delete room.sessions[botId]
      if (room.segments !== undefined) delete room.segments[botId]
      groupOwners.delete(sessionId)
@@ -1141,20 +1154,27 @@
      rt.cancelApprovals?.(sessionId)
      // A plain archive refuses a running Session, which would go on working for a group
      // that no longer has it.
-@@ -335,9 +335,10 @@ export function install(rt) {
+@@ -335,23 +335,28 @@
    }
    const findBot = (ref) => {
      if (ref === undefined) return undefined
 -    if (state.bots[ref]) return state.bots[ref]
-+    if (state.bots[ref]) return state.bots[ref].remote === undefined ? state.bots[ref] : undefined
++    const direct = ownEntry(state.bots, ref)
++    if (direct) return direct.remote === undefined ? direct : undefined
      const wanted = normalize(ref).replace(/^@/, '')
 -    return Object.values(state.bots).find(bot => normalize(bot.name) === wanted)
 +    // A friend's Bot (remote chat) is no team member, so a name never finds it.
 +    return Object.values(state.bots).find(bot => bot.remote === undefined && normalize(bot.name) === wanted)
    }
    // Every group chat whose id or name matches; callers refuse an ambiguous name.
    const findRooms = (ref) => {
-@@ -349,9 +349,11 @@ export function install(rt) {
+     if (ref === undefined || ref === null) return []
+-    if (state.rooms[ref]) return [state.rooms[ref]]
++    const direct = ownEntry(state.rooms, ref)
++    if (direct) return [direct]
+     const wanted = normalize(unquote(ref))
+     return Object.values(state.rooms).filter(room => normalize(room.name) === wanted)
+   }
    const isAdminOf = (room, id) => id !== undefined && room.admin === id
 
    Object.assign(rt, {