diff --git a/changelog.d/1711-own-key-lookups.security.md b/changelog.d/1711-own-key-lookups.security.md new file mode 100644 index 000000000..9866f8cad --- /dev/null +++ b/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)). diff --git a/core/src/host/capabilities.js b/core/src/host/capabilities.js index 9446b42c0..054b593af 100644 --- a/core/src/host/capabilities.js +++ b/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.') diff --git a/core/src/host/member-auth.js b/core/src/host/member-auth.js index 1fa95d1e9..006def9b7 100644 --- a/core/src/host/member-auth.js +++ b/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 || {}) diff --git a/core/src/host/reach.js b/core/src/host/reach.js index 22bdfd36e..8814d2894 100644 --- a/core/src/host/reach.js +++ b/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) => { diff --git a/core/src/host/roster.js b/core/src/host/roster.js index 75fa28ec3..e23862f1e 100644 --- a/core/src/host/roster.js +++ b/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 diff --git a/core/src/host/store.js b/core/src/host/store.js index 172f54f1c..a47b882ef 100644 --- a/core/src/host/store.js +++ b/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, diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index 9fe944378..d676222ce 100644 --- a/core/src/host/tasks.js +++ b/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) => { diff --git a/core/test/own-keys.test.js b/core/test/own-keys.test.js new file mode 100644 index 000000000..42e7ca536 --- /dev/null +++ b/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) +}) diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index bea3c42ce..d378df6fc 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -2987,13 +2987,14 @@ diff --git a/src/host/approvals.js b/src/host/approvals.js diff --git a/src/host/capabilities.js b/src/host/capabilities.js --- a/src/host/capabilities.js +++ b/src/host/capabilities.js -@@ -1,4 +1,5 @@ +@@ -1,4 +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]+)*$/ -@@ -7,8 +7,8 @@ const ENSURE_TIMEOUT_MS = 15_000 +@@ -7,8 +7,8 @@ export function validateCapabilityName(value) { const name = String(value ?? '').trim() @@ -3004,7 +3005,25 @@ diff --git a/src/host/capabilities.js b/src/host/capabilities.js } return name } -@@ -264,7 +264,7 @@ export function install(rt) { +@@ -139,7 +139,7 @@ + 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] +@@ -156,7 +156,7 @@ + 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' } +@@ -264,7 +264,7 @@ return { transport: 'streamable-http', serverName: item.name, url: item.url, headers: resolveValues(item.headers, botId), failOnStartupError: true } } const disposeMount = async mount => { @@ -3013,6 +3032,24 @@ diff --git a/src/host/capabilities.js b/src/host/capabilities.js } const mountOne = async (agent, bot, agentMounts, name) => { const sessionId = agent.session.id +@@ -454,7 +454,7 @@ + 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() +@@ -467,7 +467,7 @@ + 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.') diff --git a/src/host/chat-index.js b/src/host/chat-index.js --- a/src/host/chat-index.js +++ b/src/host/chat-index.js diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index d2768b383..08aeb8e26 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -137,15 +137,16 @@ diff --git a/src/host/home-lock.js b/src/host/home-lock.js diff --git a/src/host/roster.js b/src/host/roster.js --- a/src/host/roster.js +++ b/src/host/roster.js -@@ -5,6 +5,7 @@ import { join } from 'node:path' +@@ -5,6 +5,8 @@ import { MAX_AVATAR_IMAGE, ROOM_MAX_MEMBERS, ROOM_MODES, ROOM_NAME_MAX, ROOM_NOTICE_MAX } from './constants.js' 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 -@@ -15,7 +15,7 @@ export function install(rt) { +@@ -15,7 +15,7 @@ if (state.workspaceId && ctx.workspaceRegistry.get(state.workspaceId)) return state.workspaceId const path = config.workspace ?? join(home, 'workspace') await mkdir(path, { recursive: true }) @@ -154,7 +155,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js state.workspaceId = workspace.id await rt.save() return workspace.id -@@ -56,18 +56,49 @@ export function install(rt) { +@@ -56,18 +56,49 @@ return `${stem} ${index}` } @@ -212,7 +213,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js const id = await createSession(cleanName, { dir }) try { state.bots[id] = { -@@ -80,45 +80,48 @@ export function install(rt) { +@@ -80,45 +80,48 @@ ...(modelRef ? { model: modelRef } : {}), ...(effortId ? { effort: effortId } : {}), ...(dir ? { workDir: dir } : {}), @@ -285,7 +286,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js // A save that another change queued meanwhile still carries this Bot; a later // one writes the team without it. void rt.save().catch(() => {}) -@@ -128,24 +128,12 @@ export function install(rt) { +@@ -128,24 +128,12 @@ } } @@ -312,7 +313,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js name: freshName(`${bot.name} copy`), role: bot.role, instructions: bot.instructions, -@@ -155,7 +155,6 @@ export function install(rt) { +@@ -155,7 +155,6 @@ effort: served ? bot.effort : undefined, // A folder that went away since leaves the copy in the team workspace. workDir: bot.workDir !== undefined && await rt.isFolder(bot.workDir) ? bot.workDir : undefined, @@ -320,7 +321,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js }) } -@@ -189,6 +189,8 @@ export function install(rt) { +@@ -189,6 +189,8 @@ if (avatar) bot.avatar = avatar else delete bot.avatar } @@ -329,7 +330,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js if (patch.model !== undefined) { if (patch.model === null || patch.model === '') delete bot.model else bot.model = await rt.resolveModel(patch.model) -@@ -212,12 +212,6 @@ export function install(rt) { +@@ -212,17 +212,11 @@ await rt.save() if (modelChanged) await rt.applyModel(bot, { force: true }) if (moved) void rt.followFolder(bot.id) @@ -342,7 +343,13 @@ diff --git a/src/host/roster.js b/src/host/roster.js return bot } -@@ -235,7 +235,14 @@ 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 +@@ -235,7 +235,14 @@ // admin may change its own group but not delete it. Only a member can be admin. const roomBusy = id => [...rt.roomProgress.values()].some(round => round.roomId === id) @@ -358,7 +365,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js const cleanNotice = text => String(text ?? '').trim().slice(0, ROOM_NOTICE_MAX) function cleanRoomName(text, selfId) { -@@ -257,7 +257,7 @@ export function install(rt) { +@@ -257,7 +257,7 @@ async function followMemberNames(rooms = Object.values(state.rooms)) { for (const room of rooms) { if (room.named) continue @@ -367,7 +374,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js if (next === room.name) continue room.name = next await retitleRoom(room) -@@ -269,9 +269,24 @@ export function install(rt) { +@@ -269,9 +269,24 @@ if (members.length > ROOM_MAX_MEMBERS) throw new Error(`A group chat holds at most ${ROOM_MAX_MEMBERS} Bots`) } @@ -395,7 +402,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js const custom = cleanRoomName(roomName ?? '') // A Main Bot leads the groups it makes; a group the user makes is led by a Main // Bot member when it has one. -@@ -294,7 +294,19 @@ export function install(rt) { +@@ -294,7 +294,19 @@ createdAt: Date.now(), createdBy: createdBy ?? null, } @@ -415,7 +422,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js return state.rooms[id] } catch (error) { // The group's Session was already made: archive it instead of leaving an orphan. -@@ -306,13 +306,19 @@ export function install(rt) { +@@ -306,13 +306,19 @@ } } @@ -438,7 +445,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js let members = Array.isArray(patch.members) ? [...new Set(patch.members)] : [...room.members] for (const member of patch.add ?? []) if (!members.includes(member)) members.push(member) members = members.filter(member => !(patch.remove ?? []).includes(member) && rt.botOf(member)) -@@ -328,6 +328,14 @@ export function install(rt) { +@@ -328,6 +328,14 @@ admin = null } if (patch.mode !== undefined && !ROOM_MODES.includes(patch.mode)) throw new Error(`Reply mode must be one of ${ROOM_MODES.join(', ')}`) @@ -453,7 +460,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js const name = patch.name === undefined ? undefined : cleanRoomName(patch.name, room.id) const notice = patch.notice === undefined ? undefined : cleanNotice(patch.notice) const moving = patch.workDir !== undefined -@@ -340,14 +340,24 @@ export function install(rt) { +@@ -340,14 +340,24 @@ const removed = room.members.filter(member => !members.includes(member)) if (added.length > 0) changes.push(`added ${names(added)}`) if (removed.length > 0) changes.push(`removed ${names(removed)}`) @@ -483,7 +490,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js if (patch.mode !== undefined && patch.mode !== room.mode) { room.mode = patch.mode changes.push(patch.mode === 'admin' ? 'plain messages now go to the admin first' : 'every member now replies in turn') -@@ -371,7 +371,13 @@ export function install(rt) { +@@ -371,7 +371,13 @@ delete room.named } } @@ -498,7 +505,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js if (room.name !== before) { changes.push(`renamed it "${room.name}"`) await retitleRoom(room) -@@ -383,8 +383,11 @@ export function install(rt) { +@@ -383,8 +383,11 @@ async function deleteRoom(id) { const room = rt.roomOf(id) if (room === undefined) throw new Error('Unknown group chat') @@ -510,7 +517,7 @@ 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) { +@@ -409,6 +409,10 @@ } else if (replace !== undefined && replace !== id) { const at = others.indexOf(replace) if (at === -1) throw new Error('Only a Main Bot can be replaced') @@ -521,7 +528,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js next = others.with(at, id) } else if (primary) { next = [id, ...others] -@@ -417,12 +417,30 @@ export function install(rt) { +@@ -417,12 +417,30 @@ } // The sidebar shows every Main Bot, so a hidden Bot would vanish again once demoted. if (main) delete bot.hidden @@ -556,7 +563,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js } async function deleteBot(id) { -@@ -434,22 +434,42 @@ export function install(rt) { +@@ -434,22 +434,42 @@ const current = rt.chatOf(id) // Questions and exchange turns are kept per Session, so every part of the chat is cleared. const parts = new Set([id, current, ...(bot.segments ?? []).map(segment => segment.sessionId)]) @@ -599,7 +606,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js const failed = await saveForDelete() // Same as deleteRoom: stopActivity removes the Session's armed scheduled tasks. // Past parts are archived too: a part move can leave tasks on the old Session, -@@ -481,7 +481,9 @@ export function install(rt) { +@@ -481,7 +481,9 @@ if (state.mainBotIds.some(id => rt.botOf(id))) return // Only the team's first Main Bot is stored as the DeepSeek whale; other Bots get a // look from their id, and a Bot made Main Bot later keeps its own. @@ -610,7 +617,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js .finally(() => { bootstrapping = null }) await bootstrapping } -@@ -489,6 +489,6 @@ export function install(rt) { +@@ -489,6 +489,6 @@ Object.assign(rt, { ensureWorkspace, createSession, createBot, duplicateBot, updateBot, setFlags, roomBusy, autoRoomName, retitleRoom, createRoom, updateRoom, deleteRoom, @@ -621,7 +628,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js 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,46 @@ +@@ -1,21 +1,45 @@ // 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. @@ -643,13 +650,12 @@ diff --git a/src/host/store.js b/src/host/store.js +// 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 } = rt @@ -1101,7 +1107,7 @@ diff --git a/src/host/store.js b/src/host/store.js throw error } }) -@@ -273,6 +273,30 @@ +@@ -273,13 +273,37 @@ }) } @@ -1132,6 +1138,15 @@ diff --git a/src/host/store.js b/src/host/store.js ctx.effect(() => () => { stopped = true 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, @@ -313,6 +313,13 @@ } for (const [botId, sessionId] of Object.entries(room.sessions ?? {})) groupOwners.set(sessionId, { botId, roomId: room.id }) diff --git a/patches/ds-bot/files/src/host/member-auth.js b/patches/ds-bot/files/src/host/member-auth.js index 1fa95d1e9..006def9b7 100644 --- a/patches/ds-bot/files/src/host/member-auth.js +++ b/patches/ds-bot/files/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 || {}) diff --git a/patches/ds-bot/files/src/host/reach.js b/patches/ds-bot/files/src/host/reach.js index 22bdfd36e..8814d2894 100644 --- a/patches/ds-bot/files/src/host/reach.js +++ b/patches/ds-bot/files/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) => { diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index 9fe944378..d676222ce 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/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) => { diff --git a/patches/ds-bot/files/test/own-keys.test.js b/patches/ds-bot/files/test/own-keys.test.js new file mode 100644 index 000000000..42e7ca536 --- /dev/null +++ b/patches/ds-bot/files/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) +})