Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/1723-project-board-actor.security.md
Original file line number Diff line number Diff line change
@@ -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)).
7 changes: 6 additions & 1 deletion core/src/host/project-board.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
Expand All @@ -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.
Expand Down
5 changes: 3 additions & 2 deletions core/src/host/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -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')
}
}
Expand Down
9 changes: 9 additions & 0 deletions core/src/host/tasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
38 changes: 38 additions & 0 deletions core/test/project-board.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() }
})
6 changes: 6 additions & 0 deletions docs/subsystems/core-host.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Loading
Loading