From 8c4df9bd3cba37a0c315c9fb05bd3dbd791826b3 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Thu, 29 Oct 2026 23:46:07 +0800 Subject: [PATCH] Resolve Project and roster ids from their own keys only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The id tables are plain objects, so `state.projects['__proto__']` and `state.bots['__proto__']` answer `Object.prototype`, which `isRecord` accepts: `project-read` answered `{}`, `project-bind` wrote `projectId` onto `Object.prototype` (after which every unbound Bot and group reported that Project, while `state.json` carried no own key and a restart cleared it), and `findBot` / `findRooms` returned it as a member and as a group. One `ownEntry` read now feeds `projectOfId`, `projectMemberOf`, `findBot` and `findRooms`, and the one binding a request writes (`state.projectRuns`) goes through `setEntry`, so no id can reach a prototype and answer `ok: true` without storing anything. `state.projectRuns` has its own regression test: `project-bind {kind:'run', id:'__proto__'}` must leave an own `__proto__` key holding the Project and an untouched `Object.prototype`, so a plain assignment (or `Reflect.set`) in `setEntry` fails it even though the response still says `ok: true`. Docs-not-needed: the documented Project and roster behaviour does not change — an id that is not a member resolves null / not-found, which is what `docs/subsystems/core-host.md` already says; this makes the code match it instead of resolving through `Object.prototype`. Fixes #1704 --- .../1704-proto-safe-project-ids.fixed.md | 1 + core/src/host/store.js | 29 +++++-- core/test/projects.test.js | 66 ++++++++++++++++ core/test/store.test.js | 18 +++++ patches/ds-bot/0002-flybot-api-host.patch | 76 ++++++++++++------- patches/ds-bot/0014-flybot-branding.patch | 22 +++++- patches/ds-bot/files/test/projects.test.js | 66 ++++++++++++++++ 7 files changed, 241 insertions(+), 37 deletions(-) create mode 100644 changelog.d/1704-proto-safe-project-ids.fixed.md diff --git a/changelog.d/1704-proto-safe-project-ids.fixed.md b/changelog.d/1704-proto-safe-project-ids.fixed.md new file mode 100644 index 000000000..b53d10383 --- /dev/null +++ b/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)). diff --git a/core/src/host/store.js b/core/src/host/store.js index 480e9db6c..172f54f1c 100644 --- a/core/src/host/store.js +++ b/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,7 +722,8 @@ 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) @@ -717,7 +731,8 @@ export function install(rt) { // 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) } diff --git a/core/test/projects.test.js b/core/test/projects.test.js index d01f89baf..4c6e29242 100644 --- a/core/test/projects.test.js +++ b/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() } +}) diff --git a/core/test/store.test.js b/core/test/store.test.js index 04c1920f9..45c6841df 100644 --- a/core/test/store.test.js +++ b/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') +}) diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index 280840973..a4075bb63 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -620,7 +620,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,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 @@ 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 }) + export function install(rt) { - const { ctx, home, statePath } = rt @@ -664,7 +671,7 @@ diff --git a/src/host/store.js b/src/host/store.js 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 @@ diff --git a/src/host/store.js b/src/host/store.js 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,10 +777,9 @@ diff --git a/src/host/store.js b/src/host/store.js + 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) { @@ -781,14 +787,15 @@ diff --git a/src/host/store.js b/src/host/store.js + 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 @@ diff --git a/src/host/store.js b/src/host/store.js 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 @@ diff --git a/src/host/store.js b/src/host/store.js 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 @@ diff --git a/src/host/store.js b/src/host/store.js // 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 @@ diff --git a/src/host/store.js b/src/host/store.js + 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 @@ diff --git a/src/host/store.js b/src/host/store.js + 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 @@ diff --git a/src/host/store.js b/src/host/store.js 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 @@ diff --git a/src/host/store.js b/src/host/store.js warnedReadOnly = false adopt(await readFileState()) return -@@ -235,11 +235,37 @@ export function install(rt) { +@@ -235,11 +235,37 @@ return warnReadOnly() } @@ -1075,7 +1088,7 @@ diff --git a/src/host/store.js b/src/host/store.js 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 @@ diff --git a/src/host/store.js b/src/host/store.js throw error } }) -@@ -273,6 +273,30 @@ export function install(rt) { +@@ -273,6 +273,30 @@ }) } @@ -1118,7 +1131,7 @@ diff --git a/src/host/store.js b/src/host/store.js 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 @@ diff --git a/src/host/store.js b/src/host/store.js // 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,12 +1154,13 @@ diff --git a/src/host/store.js b/src/host/store.js 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. @@ -1154,7 +1168,13 @@ diff --git a/src/host/store.js b/src/host/store.js } // 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, { diff --git a/patches/ds-bot/0014-flybot-branding.patch b/patches/ds-bot/0014-flybot-branding.patch index 407acef33..00b630900 100644 --- a/patches/ds-bot/0014-flybot-branding.patch +++ b/patches/ds-bot/0014-flybot-branding.patch @@ -1840,7 +1840,7 @@ diff --git a/test/secrets.test.js b/test/secrets.test.js diff --git a/test/store.test.js b/test/store.test.js --- a/test/store.test.js +++ b/test/store.test.js -@@ -139,7 +139,7 @@ test('a state from a newer version remains untouched', async (t) => { +@@ -139,7 +139,7 @@ const original = JSON.stringify({ version: 999, revision: 77, recoveryMarker: 'keep' }) const { rt, statePath } = runtime(t, original) await rt.load() @@ -1849,7 +1849,7 @@ diff --git a/test/store.test.js b/test/store.test.js rt.state.prefs.marker = 'not saved' await rt.save() assert.equal(fs.readFileSync(statePath, 'utf8'), original) -@@ -285,3 +285,20 @@ test('dispose waits for a pending save and prevents later writes', async (t) => +@@ -285,3 +285,38 @@ restore() } }) @@ -1870,6 +1870,24 @@ diff --git a/test/store.test.js b/test/store.test.js +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') ++}) diff --git a/test/update.test.js b/test/update.test.js --- a/test/update.test.js +++ b/test/update.test.js diff --git a/patches/ds-bot/files/test/projects.test.js b/patches/ds-bot/files/test/projects.test.js index d01f89baf..4c6e29242 100644 --- a/patches/ds-bot/files/test/projects.test.js +++ b/patches/ds-bot/files/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() } +})