From 9b48f0937f2ae6ef1f506d8836c672b9a6116520 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Fri, 30 Oct 2026 01:03:08 +0800 Subject: [PATCH] core: paint only the Project cards `tasks` would let a member open The `project-board` case in store.js passed only the Project id, so the board answered with every card under the Project while `tasks` refused the same member with `bot/forbidden`. A signed-in member could read another member's Project cards, and the #1715 view made those rows clickable into the refusal. Pass the authenticated actor down, and drop rows in project-board.js through the one predicate tasks.js already owns (`rt.taskVisible`): the requester, the owner of the card's Bot or an ancestor, or an admin of the card's room. A host without the predicate paints nothing, an empty board is still `[]`, and the endpoint name, the authz allow-list and the row shape are unchanged. Test: core/test/project-board.test.js, which fails on the base with `expected [] / actual [task-b1]` when member A reads member B's Project. --- .../1723-project-board-actor.security.md | 1 + core/src/host/project-board.js | 7 +- core/src/host/store.js | 5 +- core/src/host/tasks.js | 9 +++ core/test/project-board.test.js | 38 ++++++++++ docs/subsystems/core-host.md | 6 ++ patches/ds-bot/0002-flybot-api-host.patch | 75 ++++++++++--------- .../ds-bot/files/src/host/project-board.js | 7 +- patches/ds-bot/files/src/host/tasks.js | 9 +++ .../ds-bot/files/test/project-board.test.js | 38 ++++++++++ 10 files changed, 154 insertions(+), 41 deletions(-) create mode 100644 changelog.d/1723-project-board-actor.security.md diff --git a/changelog.d/1723-project-board-actor.security.md b/changelog.d/1723-project-board-actor.security.md new file mode 100644 index 00000000..e315681f --- /dev/null +++ b/changelog.d/1723-project-board-actor.security.md @@ -0,0 +1 @@ +A Project board now paints only the cards the asking member could open through the task list: `project-board` carries the signed-in actor into the board and drops every card the same member would be refused there, so one member can no longer read another member's Project cards while `tasks` already refuses them. A card stays visible to its requester, to the owner of its Bot or an ancestor of it, and to the admin of the room it came from; an empty board is still an answer, not an error ([#1723](https://github.com/ThinkFlowLab/FlyBot/issues/1723)). diff --git a/core/src/host/project-board.js b/core/src/host/project-board.js index 71a211af..78db0acc 100644 --- a/core/src/host/project-board.js +++ b/core/src/host/project-board.js @@ -22,7 +22,7 @@ const boardOf = (task) => { export function install(rt) { // Rows for one Project, deduped by task id. An unknown Project and a Project with nothing // under it both answer `[]`: reading a board is not a reason to throw. - const projectBoard = (projectId) => { + const projectBoard = (projectId, actor) => { const id = text(projectId) if (id === '') return [] if (!rt.readProject?.(id)) return [] @@ -31,9 +31,14 @@ export function install(rt) { for (const ref of rt.projectRunRefs?.({ hasTask: taskId => Object.hasOwn(tasks, taskId) }) ?? []) { refs.set(ref.taskId, ref) } + // #1723: a row is painted only when the caller could open that card through `tasks`. The + // predicate lives in tasks.js (`rt.taskVisible`); a host that has not installed it shows + // nothing, because an unreadable rule is not a reason to widen a board. + const visible = typeof rt.taskVisible === 'function' ? rt.taskVisible : () => false const rows = [] for (const task of Object.values(tasks)) { if (task === null || typeof task !== 'object') continue + if (!visible(task, actor)) continue const board = boardOf(task) // The Project of the card's own board. A card that has a board already belongs somewhere; a // board whose member is unbound names no Project, and only then may a run carry the card. diff --git a/core/src/host/store.js b/core/src/host/store.js index cc485e67..11449408 100644 --- a/core/src/host/store.js +++ b/core/src/host/store.js @@ -474,8 +474,9 @@ export function install(rt) { } case 'project-of': return { project: projectOfMember(projectText(body.kind, 20), projectText(body.id, 200)) } // The read-only board (#1696): Project id in, its tasks out. The aggregate lives in - // project-board.js; it reads and returns, and writes nothing. - case 'project-board': return { tasks: rt.projectBoard?.(projectText(body.id, 200)) ?? [] } + // project-board.js; it reads and returns, and writes nothing. The actor goes with the id + // so the board applies the same visibility rule `tasks` does (#1723). + case 'project-board': return { tasks: rt.projectBoard?.(projectText(body.id, 200), actor) ?? [] } default: throw projectRefusal(`Unknown endpoint ${endpoint}`, 'bot/not-found') } } diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index d676222c..4b94494c 100644 --- a/core/src/host/tasks.js +++ b/core/src/host/tasks.js @@ -313,6 +313,15 @@ export function install(rt) { if (!rt.authzEnforced?.()) return true return mayAct(task, actor) || owns(actor, task.ownerBotId) } + // One visibility rule for a card row (#1723): the requester, the owner of the Bot (or an + // ancestor), or a group admin of the card's room. It is the rule `tasks` answers with, and it + // is exported so the Project board cannot paint a card the same actor would be refused there. + const taskVisible = (task, actor) => { + if (mayAct(task, actor) || ancestorOwned(actor, task.ownerBotId)) return true + const roomId = typeof task?.origin?.roomId === 'string' ? task.origin.roomId : '' + return roomId !== '' && roomAdmin(actor, state.rooms?.[roomId]) + } + rt.taskVisible = taskVisible const view = (task) => { const parent = task.parentId ? store()[task.parentId] : undefined // `claimCue` is the board's own memory of the line it posted (#1253); no reader of a card needs diff --git a/core/test/project-board.test.js b/core/test/project-board.test.js index e19029e8..b9f7c3e4 100644 --- a/core/test/project-board.test.js +++ b/core/test/project-board.test.js @@ -191,3 +191,41 @@ test('#1713: a run binding never carries another Project’s card onto a board', await stop() } }) + +// #1723: the board is the one Project read that never carried the caller, so it painted a card +// that `tasks` refuses to the same member. A second owner with its own Bot, room and Project is +// what the leak needs; nothing here belongs in `seed()`. +const crossSeed = () => { + const team = seed() + team.members['owner-2'] = { id: 'owner-2', createdAt: 2, designatedChiefId: null } + team.bots.b = { id: 'b', name: 'Writer', role: 'Write', sessionId: 'b', createdAt: 3, ownerId: 'owner-2' } + team.rooms.r2 = { id: 'r2', name: 'Garden', members: ['b'], mode: 'admin', admin: 'b', named: true, sessions: {}, log: [], seq: 0, seen: {}, projectId: 'project-p2' } + team.projects['project-p2'] = { id: 'project-p2', name: 'P2', repository: 'ThinkFlowLab/FlyBot', defaultBranch: 'main', ownerUserId: 'owner-2', createdAt: 1, updatedAt: 1, status: 'active' } + team.tasks['task-b1'] = { id: 'task-b1', kind: 'group', status: 'working', ownerBotId: 'b', origin: { roomId: 'r2' }, assignee: 'Writer', title: 'Write it', createdAt: 6, updatedAt: 10 } + return team +} + +test('#1723: a Project board paints only the cards `tasks` would let the member open', async () => { + const { rt, callAs, stop } = await start(undefined, crossSeed()) + try { + rt.authenticate = async token => (token === 'tok-1' ? { memberId: 'owner-1' } : token === 'tok-2' ? { memberId: 'owner-2' } : null) + rt.state.authSessions = { 's-1': { memberId: 'owner-1', tokenHash: 'a' }, 's-2': { memberId: 'owner-2', tokenHash: 'b' } } + // The card `tasks` refuses to member A is the one the board must not paint for it. + const refused = await callAs('tasks', { botId: 'b' }, 'tok-1') + assert.equal(refused.ok, false, JSON.stringify(refused)) + assert.equal(refused.error.code, 'bot/forbidden', JSON.stringify(refused)) + const theirs = await callAs('project-board', { id: 'project-p2' }, 'tok-1') + assert.equal(theirs.ok, true, JSON.stringify(theirs)) + assert.deepEqual(theirs.value.tasks ?? [], [], 'member A reads nothing of member B’s board') + // Member B reads the same board and still gets its card. + const own = await callAs('project-board', { id: 'project-p2' }, 'tok-2') + assert.equal(own.ok, true, JSON.stringify(own)) + assert.deepEqual(own.value.tasks.map(row => row.id), ['task-b1']) + // #1716's same-owner ALLOW and #1715's empty board stay as they were. + const p1 = await callAs('project-board', { id: 'project-p1' }, 'tok-1') + assert.deepEqual(p1.value.tasks.map(row => row.id), ['task-group']) + const empty = await callAs('project-board', { id: 'project-empty' }, 'tok-1') + assert.equal(empty.ok, true, JSON.stringify(empty)) + assert.deepEqual(empty.value.tasks, []) + } finally { await stop() } +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index b17a4376..187016eb 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -571,5 +571,11 @@ the owner — or for every caller on a Project whose record names no owner. `pro by the host with `bot/forbidden` unless the caller may manage the Project the binding names (or the one it is leaving, when unbinding); the refusal writes nothing, so hiding a button is never the gate. Reading the board (`project-board`) stays open to a non-owner: the same rows, read-only. +Those rows are the ones the caller could open through `tasks` (#1723): `store.js` passes the +authenticated actor into `projectBoard(projectId, actor)`, and the board drops every card +`rt.taskVisible` refuses — a card is visible to its requester, to the owner of its Bot or an +ancestor of it, and to the admin of the room it came from. The predicate stays in `tasks.js`, one +copy of the rule `tasks` itself answers with, so the board cannot paint a card the same member is +refused there; a board with nothing to paint is still `[]`, not an error. `project-board` is listed in `authz.js`'s `TASK_ENDPOINTS`, so a signed-in deployment does not refuse it as an unknown endpoint. diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index f1159d56..33abfdc8 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -137,7 +137,7 @@ 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,8 @@ +@@ -5,6 +5,8 @@ import { join } from 'node:path' 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' @@ -146,7 +146,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js export function install(rt) { const { ctx, config, home, defaults, state } = rt -@@ -15,7 +15,7 @@ +@@ -15,7 +15,7 @@ export function install(rt) { if (state.workspaceId && ctx.workspaceRegistry.get(state.workspaceId)) return state.workspaceId const path = config.workspace ?? join(home, 'workspace') await mkdir(path, { recursive: true }) @@ -155,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 @@ +@@ -56,18 +56,49 @@ export function install(rt) { return `${stem} ${index}` } @@ -213,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 @@ +@@ -80,45 +80,48 @@ export function install(rt) { ...(modelRef ? { model: modelRef } : {}), ...(effortId ? { effort: effortId } : {}), ...(dir ? { workDir: dir } : {}), @@ -286,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 @@ +@@ -128,24 +128,12 @@ export function install(rt) { } } @@ -313,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 @@ +@@ -155,7 +155,6 @@ export function install(rt) { 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, @@ -321,7 +321,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js }) } -@@ -189,6 +189,8 @@ +@@ -189,6 +189,8 @@ export function install(rt) { if (avatar) bot.avatar = avatar else delete bot.avatar } @@ -330,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,17 +212,11 @@ +@@ -212,17 +212,11 @@ export function install(rt) { await rt.save() if (modelChanged) await rt.applyModel(bot, { force: true }) if (moved) void rt.followFolder(bot.id) @@ -349,7 +349,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js 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 @@ +@@ -235,7 +235,14 @@ export function install(rt) { // 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) @@ -365,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 @@ +@@ -257,7 +257,7 @@ export function install(rt) { async function followMemberNames(rooms = Object.values(state.rooms)) { for (const room of rooms) { if (room.named) continue @@ -374,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 @@ +@@ -269,9 +269,24 @@ export function install(rt) { if (members.length > ROOM_MAX_MEMBERS) throw new Error(`A group chat holds at most ${ROOM_MAX_MEMBERS} Bots`) } @@ -402,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 @@ +@@ -294,7 +294,19 @@ export function install(rt) { createdAt: Date.now(), createdBy: createdBy ?? null, } @@ -422,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 @@ +@@ -306,13 +306,19 @@ export function install(rt) { } } @@ -445,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 @@ +@@ -328,6 +328,14 @@ export function install(rt) { admin = null } if (patch.mode !== undefined && !ROOM_MODES.includes(patch.mode)) throw new Error(`Reply mode must be one of ${ROOM_MODES.join(', ')}`) @@ -460,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 @@ +@@ -340,14 +340,24 @@ export function install(rt) { 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)}`) @@ -490,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 @@ +@@ -371,7 +371,13 @@ export function install(rt) { delete room.named } } @@ -505,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 @@ +@@ -383,8 +383,11 @@ export function install(rt) { async function deleteRoom(id) { const room = rt.roomOf(id) if (room === undefined) throw new Error('Unknown group chat') @@ -517,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 @@ +@@ -409,6 +409,10 @@ export function install(rt) { } else if (replace !== undefined && replace !== id) { const at = others.indexOf(replace) if (at === -1) throw new Error('Only a Main Bot can be replaced') @@ -528,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 @@ +@@ -417,12 +417,30 @@ export function install(rt) { } // The sidebar shows every Main Bot, so a hidden Bot would vanish again once demoted. if (main) delete bot.hidden @@ -563,7 +563,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js } async function deleteBot(id) { -@@ -434,22 +434,42 @@ +@@ -434,22 +434,42 @@ export function install(rt) { 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)]) @@ -606,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 @@ +@@ -481,7 +481,9 @@ export function install(rt) { 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. @@ -617,7 +617,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js .finally(() => { bootstrapping = null }) await bootstrapping } -@@ -489,6 +489,6 @@ +@@ -489,6 +489,6 @@ export function install(rt) { Object.assign(rt, { ensureWorkspace, createSession, createBot, duplicateBot, updateBot, setFlags, roomBusy, autoRoomName, retitleRoom, createRoom, updateRoom, deleteRoom, @@ -678,7 +678,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 @@ +@@ -50,12 +50,88 @@ export function install(rt) { } } const readOnly = () => { @@ -768,7 +768,7 @@ diff --git a/src/host/store.js b/src/host/store.js const readFileState = async () => { let contents try { -@@ -87,10 +87,32 @@ +@@ -87,10 +87,32 @@ export function install(rt) { return undefined } if (parsed?.version === 1) { @@ -802,7 +802,7 @@ diff --git a/src/host/store.js b/src/host/store.js return undefined } -@@ -104,6 +104,14 @@ +@@ -104,6 +104,14 @@ export function install(rt) { // 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 @@ -817,7 +817,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 @@ +@@ -112,6 +112,11 @@ export function install(rt) { migrated = true } for (const bot of Object.values(state.bots)) { @@ -829,7 +829,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 @@ +@@ -120,6 +120,11 @@ export function install(rt) { bot.color = currentColor(bot.color) migrated = true } @@ -841,7 +841,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,214 @@ +@@ -161,12 +161,215 @@ export function install(rt) { migrated = true } // Unnamed groups from before names followed their members may still list old names. @@ -1019,8 +1019,9 @@ diff --git a/src/host/store.js b/src/host/store.js + } + case 'project-of': return { project: projectOfMember(projectText(body.kind, 20), projectText(body.id, 200)) } + // The read-only board (#1696): Project id in, its tasks out. The aggregate lives in -+ // project-board.js; it reads and returns, and writes nothing. -+ case 'project-board': return { tasks: rt.projectBoard?.(projectText(body.id, 200)) ?? [] } ++ // project-board.js; it reads and returns, and writes nothing. The actor goes with the id ++ // so the board applies the same visibility rule `tasks` does (#1723). ++ case 'project-board': return { tasks: rt.projectBoard?.(projectText(body.id, 200), actor) ?? [] } + default: throw projectRefusal(`Unknown endpoint ${endpoint}`, 'bot/not-found') + } + } @@ -1057,7 +1058,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 @@ +@@ -185,14 +185,24 @@ export function install(rt) { for (const room of stale) void rt.retitleRoom(room) })() @@ -1083,7 +1084,7 @@ diff --git a/src/host/store.js b/src/host/store.js warnedReadOnly = false adopt(await readFileState()) return -@@ -235,11 +235,37 @@ +@@ -235,11 +235,37 @@ export function install(rt) { return warnReadOnly() } @@ -1121,7 +1122,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 @@ +@@ -260,9 +260,10 @@ export function install(rt) { ensureLock() await rename(temp, statePath) seen = stamp() @@ -1133,7 +1134,7 @@ diff --git a/src/host/store.js b/src/host/store.js throw error } }) -@@ -273,13 +273,37 @@ +@@ -273,13 +273,37 @@ export function install(rt) { }) } @@ -1173,7 +1174,7 @@ diff --git a/src/host/store.js b/src/host/store.js 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 @@ +@@ -313,6 +313,13 @@ export function install(rt) { } for (const [botId, sessionId] of Object.entries(room.sessions ?? {})) groupOwners.set(sessionId, { botId, roomId: room.id }) } @@ -1187,7 +1188,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 @@ +@@ -328,6 +328,8 @@ export function install(rt) { delete room.sessions[botId] if (room.segments !== undefined) delete room.segments[botId] groupOwners.delete(sessionId) @@ -1196,7 +1197,7 @@ 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,23 +335,28 @@ +@@ -335,23 +335,28 @@ export function install(rt) { } const findBot = (ref) => { if (ref === undefined) return undefined diff --git a/patches/ds-bot/files/src/host/project-board.js b/patches/ds-bot/files/src/host/project-board.js index 71a211af..78db0acc 100644 --- a/patches/ds-bot/files/src/host/project-board.js +++ b/patches/ds-bot/files/src/host/project-board.js @@ -22,7 +22,7 @@ const boardOf = (task) => { export function install(rt) { // Rows for one Project, deduped by task id. An unknown Project and a Project with nothing // under it both answer `[]`: reading a board is not a reason to throw. - const projectBoard = (projectId) => { + const projectBoard = (projectId, actor) => { const id = text(projectId) if (id === '') return [] if (!rt.readProject?.(id)) return [] @@ -31,9 +31,14 @@ export function install(rt) { for (const ref of rt.projectRunRefs?.({ hasTask: taskId => Object.hasOwn(tasks, taskId) }) ?? []) { refs.set(ref.taskId, ref) } + // #1723: a row is painted only when the caller could open that card through `tasks`. The + // predicate lives in tasks.js (`rt.taskVisible`); a host that has not installed it shows + // nothing, because an unreadable rule is not a reason to widen a board. + const visible = typeof rt.taskVisible === 'function' ? rt.taskVisible : () => false const rows = [] for (const task of Object.values(tasks)) { if (task === null || typeof task !== 'object') continue + if (!visible(task, actor)) continue const board = boardOf(task) // The Project of the card's own board. A card that has a board already belongs somewhere; a // board whose member is unbound names no Project, and only then may a run carry the card. diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index d676222c..4b94494c 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/src/host/tasks.js @@ -313,6 +313,15 @@ export function install(rt) { if (!rt.authzEnforced?.()) return true return mayAct(task, actor) || owns(actor, task.ownerBotId) } + // One visibility rule for a card row (#1723): the requester, the owner of the Bot (or an + // ancestor), or a group admin of the card's room. It is the rule `tasks` answers with, and it + // is exported so the Project board cannot paint a card the same actor would be refused there. + const taskVisible = (task, actor) => { + if (mayAct(task, actor) || ancestorOwned(actor, task.ownerBotId)) return true + const roomId = typeof task?.origin?.roomId === 'string' ? task.origin.roomId : '' + return roomId !== '' && roomAdmin(actor, state.rooms?.[roomId]) + } + rt.taskVisible = taskVisible const view = (task) => { const parent = task.parentId ? store()[task.parentId] : undefined // `claimCue` is the board's own memory of the line it posted (#1253); no reader of a card needs diff --git a/patches/ds-bot/files/test/project-board.test.js b/patches/ds-bot/files/test/project-board.test.js index e19029e8..b9f7c3e4 100644 --- a/patches/ds-bot/files/test/project-board.test.js +++ b/patches/ds-bot/files/test/project-board.test.js @@ -191,3 +191,41 @@ test('#1713: a run binding never carries another Project’s card onto a board', await stop() } }) + +// #1723: the board is the one Project read that never carried the caller, so it painted a card +// that `tasks` refuses to the same member. A second owner with its own Bot, room and Project is +// what the leak needs; nothing here belongs in `seed()`. +const crossSeed = () => { + const team = seed() + team.members['owner-2'] = { id: 'owner-2', createdAt: 2, designatedChiefId: null } + team.bots.b = { id: 'b', name: 'Writer', role: 'Write', sessionId: 'b', createdAt: 3, ownerId: 'owner-2' } + team.rooms.r2 = { id: 'r2', name: 'Garden', members: ['b'], mode: 'admin', admin: 'b', named: true, sessions: {}, log: [], seq: 0, seen: {}, projectId: 'project-p2' } + team.projects['project-p2'] = { id: 'project-p2', name: 'P2', repository: 'ThinkFlowLab/FlyBot', defaultBranch: 'main', ownerUserId: 'owner-2', createdAt: 1, updatedAt: 1, status: 'active' } + team.tasks['task-b1'] = { id: 'task-b1', kind: 'group', status: 'working', ownerBotId: 'b', origin: { roomId: 'r2' }, assignee: 'Writer', title: 'Write it', createdAt: 6, updatedAt: 10 } + return team +} + +test('#1723: a Project board paints only the cards `tasks` would let the member open', async () => { + const { rt, callAs, stop } = await start(undefined, crossSeed()) + try { + rt.authenticate = async token => (token === 'tok-1' ? { memberId: 'owner-1' } : token === 'tok-2' ? { memberId: 'owner-2' } : null) + rt.state.authSessions = { 's-1': { memberId: 'owner-1', tokenHash: 'a' }, 's-2': { memberId: 'owner-2', tokenHash: 'b' } } + // The card `tasks` refuses to member A is the one the board must not paint for it. + const refused = await callAs('tasks', { botId: 'b' }, 'tok-1') + assert.equal(refused.ok, false, JSON.stringify(refused)) + assert.equal(refused.error.code, 'bot/forbidden', JSON.stringify(refused)) + const theirs = await callAs('project-board', { id: 'project-p2' }, 'tok-1') + assert.equal(theirs.ok, true, JSON.stringify(theirs)) + assert.deepEqual(theirs.value.tasks ?? [], [], 'member A reads nothing of member B’s board') + // Member B reads the same board and still gets its card. + const own = await callAs('project-board', { id: 'project-p2' }, 'tok-2') + assert.equal(own.ok, true, JSON.stringify(own)) + assert.deepEqual(own.value.tasks.map(row => row.id), ['task-b1']) + // #1716's same-owner ALLOW and #1715's empty board stay as they were. + const p1 = await callAs('project-board', { id: 'project-p1' }, 'tok-1') + assert.deepEqual(p1.value.tasks.map(row => row.id), ['task-group']) + const empty = await callAs('project-board', { id: 'project-empty' }, 'tok-1') + assert.equal(empty.ok, true, JSON.stringify(empty)) + assert.deepEqual(empty.value.tasks, []) + } finally { await stop() } +})