diff --git a/changelog.d/106-chip-resolution.fixed.md b/changelog.d/106-chip-resolution.fixed.md new file mode 100644 index 000000000..a120c77a6 --- /dev/null +++ b/changelog.d/106-chip-resolution.fixed.md @@ -0,0 +1 @@ +Chat-reference chips are author-checked on answer-question and schedule-create as well as send and room-post, schedule-create authorization keys off the Bot or room the endpoint uses, and Bot-authored group posts drop chips so a member Bot cannot tunnel a private chat into a shared room. ([#106](https://github.com/ThinkFlowLab/FlyBot/issues/106)) diff --git a/changelog.d/111-task-lifecycle.fixed.md b/changelog.d/111-task-lifecycle.fixed.md new file mode 100644 index 000000000..702b1c2fa --- /dev/null +++ b/changelog.d/111-task-lifecycle.fixed.md @@ -0,0 +1 @@ +A held send blocks only the named working task (or that Bot's only working task), a task waiting for your acceptance is not pruned, and task channel names are an explicit allow-list rather than any `task-*` prefix. ([#111](https://github.com/ThinkFlowLab/FlyBot/issues/111)) diff --git a/changelog.d/97-approval-binding.fixed.md b/changelog.d/97-approval-binding.fixed.md new file mode 100644 index 000000000..ff7ca2055 --- /dev/null +++ b/changelog.d/97-approval-binding.fixed.md @@ -0,0 +1 @@ +Held-send invalidation matches the contract: a hold addressed to a replaced Chief stays pending and decidable, unknown `invalidateFor` fields throw, and an in-flight approve is re-checked before dispatch so a concurrent Chief replacement cannot send on the old verdict. ([#97](https://github.com/ThinkFlowLab/FlyBot/issues/97)) diff --git a/core/src/host/authz.js b/core/src/host/authz.js index b16f959ea..ddb529c24 100644 --- a/core/src/host/authz.js +++ b/core/src/host/authz.js @@ -49,6 +49,12 @@ const ROOM_TARGETED = new Set(['update-room', 'delete-room', 'room-progress', 'c // Endpoints whose body names a Session. const SESSION_TARGETED = new Set(['earlier', 'send', 'answer-question', 'dismiss-question', 'approval-answer', 'exchange']) +// Task channel names. Requester checks live on the record in tasks.js; listing them +// here keeps a future `task-*` from auto-allowing for any signed-in actor (#111). +const TASK_ENDPOINTS = new Set([ + 'tasks', 'task-create', 'task-accept', 'task-start', 'task-return', 'task-accept-result', 'task-cancel', +]) + const deny = (code, message) => ({ ok: false, code, message }) const ALLOW = { ok: true } @@ -217,6 +223,16 @@ export function install(rt) { if (BOT_TARGETED.has(ep)) { if (ep === 'schedules' || ep === 'schedule-create' || ep === 'schedule-delete') { + // The create endpoint keys off `body.target` (Bot or room id); `sessionId` is + // the list/delete key. Checking only sessionId would allow a foreign target + // paired with an owned session (#106). + if (ep === 'schedule-create') { + const target = body?.target + if (typeof target === 'string' && target !== '') { + if (botOwnedBy(actor, target) || roomOwnedBy(actor, target) || roomMemberOf(actor, target)) return ALLOW + return deny('bot/forbidden', 'You do not own that Bot or group chat') + } + } if (ep === 'schedule-delete' && typeof body?.id === 'string' && !sessionOwnedBy(actor, body.id) && !sessionOwnedBy(actor, body.sessionId)) return deny('bot/forbidden', 'You do not own that schedule') return requireSession(body?.sessionId) } @@ -327,8 +343,9 @@ export function install(rt) { if (ep === 'share-brief') return ALLOW // Task transitions are requester-checked on the record in tasks.js (#54) — the // endpoint only needs a signed-in actor, like held-send decisions. Create is - // grant-aware there (owner, matching grant, or the named room). - if (ep === 'tasks' || ep.startsWith('task-')) return ALLOW + // grant-aware there (owner, matching grant, or the named room). Explicit set: + // a future `task-*` name must be listed here, not auto-allowed (#111). + if (TASK_ENDPOINTS.has(ep)) return ALLOW // Anything not in the table and not a known auth/read endpoint is denied. if (NO_ACTOR_ALLOWED.has(ep) || ep.startsWith('auth-')) return ALLOW diff --git a/core/src/host/channel.js b/core/src/host/channel.js index c9e6a2a08..279bbe260 100644 --- a/core/src/host/channel.js +++ b/core/src/host/channel.js @@ -119,11 +119,18 @@ export function install(rt) { const READS = new Set(['state', 'models', 'efforts', 'earlier', 'room-progress', 'room-history', 'exchange', 'usage', 'schedules', 'memory-view', 'browser-wait', 'browser-result', 'diagnostics', 'diagnostics-log', 'diagnostics-clear', 'update-check', 'update-install', 'dsh-update-install', 'repo-stars', 'approval-answer', 'auth-whoami']) // A chat reference chip names a Session whose snapshot rides to every recipient of - // the message, so the author must be able to read what they share (#53). + // the message, so the author must be able to read what they share (#53, #106). + // Actor-authored text that becomes a session user message: send, room-post, + // answer-question, schedule-create. Bot-authored room posts strip chips in groups.js. + // Menu chips encode the session id as base64url(JSON); raw ids still appear in tests. + const decodeSessionChip = (payload) => { + try { return JSON.parse(Buffer.from(payload, 'base64url').toString('utf8')) } catch { return undefined } + } const unshareableReference = (actor, text) => { if (!actor || !rt.authzEnforced?.() || typeof text !== 'string') return undefined for (const match of text.matchAll(/dsh-session:([A-Za-z0-9_-]+)/g)) { - if (!rt.sessionOwnedBy?.(actor, match[1])) return match[1] + const id = decodeSessionChip(match[1]) ?? match[1] + if (!rt.sessionOwnedBy?.(actor, id)) return id } return undefined } @@ -318,10 +325,15 @@ export function install(rt) { await rt.load() return ok(await rt.capabilityEndpoint(endpoint, body)) case 'schedules': - case 'schedule-create': case 'schedule-delete': await rt.load() return ok(await rt.scheduleEndpoint(endpoint, body)) + case 'schedule-create': { + await rt.load() + const leaked = unshareableReference(actor, [body.title, body.prompt].filter(part => typeof part === 'string').join('\n')) + if (leaked !== undefined) return fail('You do not own that chat', 'bot/forbidden') + return ok(await rt.scheduleEndpoint(endpoint, body)) + } case 'memory-view': case 'memory-ask': case 'memory-forget': @@ -336,6 +348,8 @@ export function install(rt) { if (question === undefined || question.id !== body.questionId) return fail('This question is no longer open', 'bot/stale') const reply = questionReply(question, body.answer) if (reply === undefined) return fail('Nothing to send', 'bot/invalid') + const leaked = unshareableReference(actor, reply.text) + if (leaked !== undefined) return fail('You do not own that chat', 'bot/forbidden') // Claim the question before queueing so a second answer is refused. delete state.questions[sessionId] try { diff --git a/core/src/host/groups.js b/core/src/host/groups.js index 81b4e800f..4949882f8 100644 --- a/core/src/host/groups.js +++ b/core/src/host/groups.js @@ -28,6 +28,23 @@ const ROOM_LOG_MAX = 60 const ROOM_FIRST_LOOK = 12 const ROOM_DELTA_MAX = 24 const ROOM_EARLIER_MAX = 8 +// Bot-authored group followups are createUserMessage; DSH may resolve `dsh-session:` +// chips there and attach the snapshot to the last user post. Drop chips so a member +// Bot cannot tunnel a private chat into the room (#106). Authorship stays the gate. +function stripSessionChips(text) { + if (typeof text !== 'string' || !text.includes('dsh-session:')) return text + // Loop until stable: a nested marker like dsh-sessiondsh-session:x:ImQi must not + // reconstitute a valid chip after one pass (#106 Codex review). + let out = text + for (let previous; previous !== out;) { + previous = out + out = out + .replace(/@\[((?:\\.|[^\]\\])*)\]\(dsh-session:[A-Za-z0-9_-]+\)/g, (_, label) => `@${String(label).replace(/\\([\\\]])/g, '$1')}`) + .replace(/dsh-session:[A-Za-z0-9_-]+/g, '') + } + if (out.includes('dsh-session:')) out = out.replace(/dsh-session:/g, '') + return out +} const ROOM_EXTRA_TURNS = 4 const ROOM_ADMIN_EXTRA_TURNS = 4 const ROOM_TURNS_EACH = 2 @@ -601,7 +618,7 @@ export function install(rt) { async function postToRoom(room, sender, text, extra = {}) { const agent = await rt.resolveAgent(room.id) agent.followup(createUserMessage({ - content: [{ type: 'text', text: `[Group post from ${sender.name}]\n${text}` }], + content: [{ type: 'text', text: `[Group post from ${sender.name}]\n${stripSessionChips(text)}` }], source: { kind: SOURCE_KIND, form: 'relay', role: 'post', exchangeId: randomUUID(), senderName: sender.name, senderSessionId: sender.id, roomId: room.id, hop: 0, ...extra }, })) await ctx.sessions.flush(agent.session) diff --git a/core/src/host/hooks.js b/core/src/host/hooks.js index 0910aee15..b4fc36fe7 100644 --- a/core/src/host/hooks.js +++ b/core/src/host/hooks.js @@ -328,10 +328,17 @@ export function install(rt) { // Queued work whose authority is gone does not wait forever: a hold naming one of // these Bots as sender or target is marked stale (still listed for the audit view). // `field` narrows the match ('from': sender-side only, e.g. a demoted Chief). + // Unknown values must throw: a silent no-op would skip invalidation (#97). const invalidateFor = async (botIds, reason, field) => { + if (field !== undefined && field !== 'from') { + throw new Error(`flybot: invalidateFor field must be 'from' or omitted, got ${JSON.stringify(field)}`) + } await rt.load() const ids = new Set(botIds) - const hit = Object.values(store()).filter(a => a.status === 'pending' + // Pending, and approved-but-not-yet-dispatched: otherwise a concurrent setMain / + // deleteBot during release() skips the record (status already `approved`) (#97). + const inFlight = a => a.status === 'pending' || (a.status === 'approved' && a.exchangeId === undefined) + const hit = Object.values(store()).filter(a => inFlight(a) && (ids.has(a.request?.from) && (field === undefined || field === 'from') || ids.has(a.request?.to) && field === undefined)) if (hit.length === 0) return @@ -415,6 +422,15 @@ export function install(rt) { } return } + // Re-check immediately before dispatch: evaluate() yielded, so a concurrent + // setMain/deleteBot may have marked this in-flight approve stale (#97). + if (approval.status === 'stale') return + const still = staleReason(approval) + if (still !== undefined) { + approval.status = 'stale' + approval.error = still + return + } try { approval.exchangeId = await dispatch(approval.request) } catch (error) { diff --git a/core/src/host/tasks.js b/core/src/host/tasks.js index cfadfca09..5b4e286bb 100644 --- a/core/src/host/tasks.js +++ b/core/src/host/tasks.js @@ -17,6 +17,8 @@ import { randomBytes } from 'node:crypto' import { currentParticipants } from './room-membership.js' const ACTIVE = new Set(['open', 'claimed', 'working', 'blocked']) +// result-ready still waits for the requester: prune must not evict it with terminals (#111). +const PROTECTED = new Set([...ACTIVE, 'result-ready']) const TRANSITIONS = { open: ['claimed', 'cancelled', 'failed'], claimed: ['working', 'cancelled', 'failed'], @@ -79,17 +81,25 @@ export function install(rt) { const view = task => ({ ...task, actors: [...task.actors ?? []] }) const prune = () => { const all = Object.values(store()) - const terminal = all.filter(task => !ACTIVE.has(task.status)).sort((a, b) => a.updatedAt - b.updatedAt) - for (const task of terminal.slice(0, Math.max(0, all.length - KEPT))) delete store()[task.id] + const droppable = all.filter(task => !PROTECTED.has(task.status)).sort((a, b) => a.updatedAt - b.updatedAt) + for (const task of droppable.slice(0, Math.max(0, all.length - KEPT))) delete store()[task.id] } const save = () => rt.save() // ---------------------------------------------------------------- host hooks // A consequential send from the responsible Bot was held (#52 card): the task - // blocks on that approval until the owner decides. + // blocks on that approval until the owner decides. Link one task, not every + // working task of that Bot: `meta.taskId` names it, else the Bot's only working + // task (several without a name stay working — they are not this send's) (#111). rt.taskHeld = (msg, approval) => { - for (const task of Object.values(store())) { - if (task.status !== 'working' || task.ownerBotId !== msg.from) continue + const namedId = typeof msg?.meta?.taskId === 'string' ? msg.meta.taskId : undefined + const named = namedId ? store()[namedId] : undefined + const ofSender = task => task.ownerBotId === msg.from || task.workBotId === msg.from + const working = Object.values(store()).filter(task => task.status === 'working' && ofSender(task)) + const targets = named && named.status === 'working' && ofSender(named) + ? [named] + : working.length === 1 ? working : [] + for (const task of targets) { task.approvalId = approval.id transition(task, 'blocked', null, { approvalId: approval.id }) } @@ -135,6 +145,7 @@ export function install(rt) { updatedAt: Date.now(), requesterId, ownerBotId: botId, + // workBotId is the specialist on the exchange; assigned with #55 wiring (#111). title: String(body.title ?? '').slice(0, 300), ...(typeof body.criteria === 'string' && body.criteria.trim() !== '' ? { criteria: body.criteria.slice(0, 2000) } : {}), origin: { ...(roomId ? { roomId } : {}), ...(body.sessionId ? { sessionId: String(body.sessionId) } : {}) }, diff --git a/core/test/approval-binding.test.js b/core/test/approval-binding.test.js index 61954e042..a13eb02f5 100644 --- a/core/test/approval-binding.test.js +++ b/core/test/approval-binding.test.js @@ -251,6 +251,47 @@ test('replacing a Chief invalidates the held sends written under it', async (t) assert.equal(approval.status, 'stale', JSON.stringify(approval)) }) +test('replacing a Chief leaves holds addressed to it pending and decidable', async (t) => { + const { fb, call, stop, received } = await start() + t.after(stop) + const alice = await signIn(call, 'Alice') + holdFlagged(fb) + // Sender-side only: a hold *to* the demoted Chief is still the sender's owner's to spend. + const id = await heldId(fb, { from: 'b', to: 'a', text: 'ask the chief', meta: { hold: true } }) + const swapped = await call('set-main', { id: 'b', main: true, replace: 'a' }, alice.sessionToken) + assert.equal(swapped.ok, true, JSON.stringify(swapped)) + const list = await call('flybot/approvals', { status: ['pending', 'failed', 'stale'] }, alice.sessionToken) + assert.equal(list.ok, true) + const approval = list.value.find(entry => entry.id === id) + assert.equal(approval.status, 'pending', JSON.stringify(approval)) + const decided = await call('flybot/decide', { id, decision: 'approve' }, alice.sessionToken) + assert.equal(decided.ok, true, JSON.stringify(decided)) + assert.equal(decided.value.status, 'approved', JSON.stringify(decided.value)) + assert.deepEqual(received('a'), ['[Message from Writer]\nask the chief']) +}) + +test('an in-flight approve is invalidated if the Chief is replaced before dispatch', async (t) => { + const { fb, call, stop, received } = await start() + t.after(stop) + const alice = await signIn(call, 'Alice') + holdFlagged(fb) + const id = await heldId(fb, { from: 'a', to: 'c', text: 'still in flight', meta: { hold: true } }) + // After the hold middleware, release re-evaluates the rest of the chain. Swap the + // Chief there so invalidateFor races the already-approved record (#97 TOCTOU). + fb.bot.send.use({ + name: 'swap-chief', + handle: async () => { + const swapped = await call('set-main', { id: 'b', main: true, replace: 'a' }, alice.sessionToken) + assert.equal(swapped.ok, true, JSON.stringify(swapped)) + return undefined + }, + }) + const decided = await call('flybot/decide', { id, decision: 'approve' }, alice.sessionToken) + assert.equal(decided.ok, true, JSON.stringify(decided)) + assert.equal(decided.value.status, 'stale', JSON.stringify(decided.value)) + assert.equal(received('c').length, 0) +}) + test('a member whose sender Bot is gone cannot decide leftovers with an old session', async (t) => { const { fb, call, stop, statePath } = await start() t.after(stop) diff --git a/core/test/authz.test.js b/core/test/authz.test.js index 731f9828f..a745262a6 100644 --- a/core/test/authz.test.js +++ b/core/test/authz.test.js @@ -195,6 +195,35 @@ test('a member reaches their own Bots sessions, groups and schedules', async (t) // A group Session follows its room's rule; so does a schedule on it. assert.equal(rt.authorize(actor, 'send', { sessionId: 'gs-a1' }).ok, true) assert.equal(rt.authorize(actor, 'schedule-create', { sessionId: 'gs-a1' }).ok, true) + // The create endpoint keys off `target` (Bot or room id), not sessionId (#106). + assert.equal(rt.authorize(actor, 'schedule-create', { target: 'a1' }).ok, true) + assert.equal(rt.authorize(actor, 'schedule-create', { target: 'ra' }).ok, true) +}) + +test('schedule-create cannot hide a foreign target behind an owned sessionId', async (t) => { + const { rt } = runtime(t, TEAM) + await rt.load() + const sessionA = await redeemOwner(rt) + await redeemOwner(rt) + rt.state.bots.a1 = { id: 'a1', sessionId: 'chat-a1', name: 'A', instructions: '', createdAt: 2, ownerId: 'owner-1' } + rt.state.bots.b1 = { id: 'b1', sessionId: 'chat-b1', name: 'B', instructions: '', createdAt: 3, ownerId: 'owner-2' } + const actorA = await rt.authenticate(sessionA.sessionToken) + assert.equal(rt.authorize(actorA, 'schedule-create', { target: 'b1' }).ok, false) + assert.equal(rt.authorize(actorA, 'schedule-create', { target: 'b1', sessionId: 'chat-a1' }).code, 'bot/forbidden') + assert.equal(rt.authorize(actorA, 'schedule-create', { target: 'a1' }).ok, true) +}) + +test('task endpoints are an explicit allow-list, not a task-* prefix', async (t) => { + const { rt } = runtime(t, TEAM) + await rt.load() + const sessionA = await redeemOwner(rt) + const actorA = await rt.authenticate(sessionA.sessionToken) + for (const ep of ['tasks', 'task-create', 'task-accept', 'task-start', 'task-return', 'task-accept-result', 'task-cancel']) { + assert.equal(rt.authorize(actorA, ep, {}).ok, true, ep) + } + const unknown = rt.authorize(actorA, 'task-explode', {}) + assert.equal(unknown.ok, false, JSON.stringify(unknown)) + assert.equal(unknown.code, 'bot/not-found') }) test('approval-answer resolves the approval, not caller-chosen fields', async (t) => { diff --git a/core/test/context-sharing.test.js b/core/test/context-sharing.test.js index fbc08ca44..38654c730 100644 --- a/core/test/context-sharing.test.js +++ b/core/test/context-sharing.test.js @@ -230,6 +230,83 @@ test('a chat reference chip must name a chat the author may read', async (t) => void bob }) +test('answer-question and schedule-create chips must name a chat the author may read', async (t) => { + const state = team() + state.questions.a = { id: 'q1', question: 'What next?', options: [{ label: 'Go' }], callId: 'c1' } + const { call, signIn } = await start(t, { state }) + const alice = await signIn('Alice') + const bob = await signIn('Bob', alice.sessionToken) + const foreignAnswer = await call('answer-question', { + sessionId: 'a', questionId: 'q1', + answer: { custom: 'look at @[that](dsh-session:d) please' }, + }, alice.sessionToken) + assert.equal(foreignAnswer.ok, false, JSON.stringify(foreignAnswer)) + assert.equal(foreignAnswer.error.code, 'bot/forbidden') + const ownAnswer = await call('answer-question', { + sessionId: 'a', questionId: 'q1', + answer: { custom: 'look at @[this](dsh-session:a) please' }, + }, alice.sessionToken) + assert.equal(ownAnswer.ok, true, JSON.stringify(ownAnswer)) + + const foreignSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: 'look at @[that](dsh-session:d)', + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + assert.equal(foreignSchedule.ok, false, JSON.stringify(foreignSchedule)) + assert.equal(foreignSchedule.error.code, 'bot/forbidden') + const ownSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: 'look at @[this](dsh-session:a)', + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + // Own chip is allowed through the author gate; automation may still be off in this host. + if (ownSchedule.ok === false) assert.notEqual(ownSchedule.error.code, 'bot/forbidden', JSON.stringify(ownSchedule)) + void bob +}) + +test('answer-question and schedule-create accept menu-encoded session chips the author owns', async (t) => { + // Menu chips encode JSON.stringify(sessionId) as base64url (`a` → ImEi). + const ownEncoded = 'ImEi' + const foreignEncoded = Buffer.from(JSON.stringify('d')).toString('base64url') + const state = team() + state.questions.a = { id: 'q1', question: 'What next?', options: [{ label: 'Go' }], callId: 'c1' } + const { call, signIn } = await start(t, { state }) + const alice = await signIn('Alice') + const ownAnswer = await call('answer-question', { + sessionId: 'a', questionId: 'q1', + answer: { custom: `look at @[this](dsh-session:${ownEncoded}) please` }, + }, alice.sessionToken) + assert.equal(ownAnswer.ok, true, JSON.stringify(ownAnswer)) + const foreignSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: `look at @[that](dsh-session:${foreignEncoded})`, + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + assert.equal(foreignSchedule.ok, false, JSON.stringify(foreignSchedule)) + assert.equal(foreignSchedule.error.code, 'bot/forbidden') + const ownSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: `look at @[this](dsh-session:${ownEncoded})`, + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + if (ownSchedule.ok === false) assert.notEqual(ownSchedule.error.code, 'bot/forbidden', JSON.stringify(ownSchedule)) +}) + +test('a Bot post into a group drops chat-reference chips so they cannot tunnel a private chat', async (t) => { + const { tool, agentOf, signIn } = await start(t) + const alice = await signIn('Alice') + await signIn('Bob', alice.sessionToken) + const posted = await tool('post_to_group', { group: 'War', message: 'see @[secret](dsh-session:a) and dsh-session:d' }, 'a') + assert.match(String(posted), /Posted/, String(posted)) + const text = agentOf('war').calls.map(message => (message.content ?? []).map(block => block.text ?? '').join('\n')).join('\n') + assert.ok(text.includes('see @secret'), 'the rest of the post still lands') + assert.ok(!text.includes('dsh-session:'), 'chips do not enter the group Session as user text') + // Nested marker must not reconstitute a valid chip after one strip pass. + const nested = await tool('post_to_group', { group: 'War', message: 'nested @[secret](dsh-sessiondsh-session:x:ImQi) end' }, 'a') + assert.match(String(nested), /Posted/, String(nested)) + const again = agentOf('war').calls.map(message => (message.content ?? []).map(block => block.text ?? '').join('\n')).join('\n') + assert.ok(!again.includes('dsh-session:'), 'nested chips do not reconstitute') + assert.ok(!/@\[secret\]\(dsh-session:/.test(again), 'no valid reconstituted chip') + void alice +}) + test('a granted remote Bot in the room counts as another owner for the brief', async (t) => { diff --git a/core/test/tasks.test.js b/core/test/tasks.test.js index 650bc9e5d..2471efe24 100644 --- a/core/test/tasks.test.js +++ b/core/test/tasks.test.js @@ -259,6 +259,70 @@ test('rejecting the blocked approval cancels the task', async (t) => { assert.equal(resume.error.code, 'bot/stale') }) +test('a held send blocks only the named working task, not every working task of that Bot', async (t) => { + const { fb, call, signIn, createTask } = await start(t) + const alice = await signIn('Alice') + const first = await createTask(alice.sessionToken, { botId: 'a', title: 'Note one' }) + const second = await createTask(alice.sessionToken, { botId: 'a', title: 'Note two' }) + for (const id of [first.value.id, second.value.id]) { + await call('task-accept', { id }, alice.sessionToken) + await call('task-start', { id }, alice.sessionToken) + } + fb.bot.send.use({ name: 'gate', handle: async msg => (msg.meta?.hold ? { action: 'hold', reason: 'held by test' } : undefined) }) + const held = await fb.bot.send({ from: 'a', to: 's', text: 'send this out', meta: { hold: true, taskId: first.value.id } }) + assert.equal(held.action, 'hold', JSON.stringify(held)) + assert.equal((await statusOf(call, alice.sessionToken, first.value.id)).status, 'blocked') + assert.equal((await statusOf(call, alice.sessionToken, second.value.id)).status, 'working') + const rejected = await call('flybot/decide', { id: held.approvalId, decision: 'reject' }, alice.sessionToken) + assert.equal(rejected.ok, true, JSON.stringify(rejected)) + assert.equal((await statusOf(call, alice.sessionToken, first.value.id)).status, 'cancelled') + assert.equal((await statusOf(call, alice.sessionToken, second.value.id)).status, 'working') +}) + +test('an untagged hold does not block every working task when the Bot has several', async (t) => { + const { fb, call, signIn, createTask } = await start(t) + const alice = await signIn('Alice') + const first = await createTask(alice.sessionToken, { botId: 'a', title: 'Note one' }) + const second = await createTask(alice.sessionToken, { botId: 'a', title: 'Note two' }) + for (const id of [first.value.id, second.value.id]) { + await call('task-accept', { id }, alice.sessionToken) + await call('task-start', { id }, alice.sessionToken) + } + fb.bot.send.use({ name: 'gate', handle: async msg => (msg.meta?.hold ? { action: 'hold', reason: 'held by test' } : undefined) }) + const held = await fb.bot.send({ from: 'a', to: 's', text: 'untagged send', meta: { hold: true } }) + assert.equal(held.action, 'hold', JSON.stringify(held)) + assert.equal((await statusOf(call, alice.sessionToken, first.value.id)).status, 'working') + assert.equal((await statusOf(call, alice.sessionToken, second.value.id)).status, 'working') +}) + +test('a result-ready task is not pruned while it waits for acceptance', async (t) => { + const { call, signIn, createTask, stop, home } = await start(t) + const alice = await signIn('Alice') + const created = await createTask(alice.sessionToken) + const task = created.value + await call('task-accept', { id: task.id }, alice.sessionToken) + await call('task-start', { id: task.id }, alice.sessionToken) + await call('task-return', { id: task.id, summary: 'waiting' }, alice.sessionToken) + await stop({ keepHome: true }) + const saved = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')) + for (let i = 0; i < 200; i++) { + const id = `task-old${i.toString(16).padStart(8, '0')}deadbeef` + saved.tasks[id] = { + id, status: 'cancelled', createdAt: 1, updatedAt: i + 2, + requesterId: alice.memberId, ownerBotId: 'a', title: 'old', origin: {}, actors: [], + } + } + saved.tasks[task.id].updatedAt = 1 + writeFileSync(join(home, 'state.json'), JSON.stringify(saved)) + const again = await start(t, { home }) + t.after(() => again.stop()) + const extra = await again.createTask(alice.sessionToken, { title: 'new' }) + assert.equal(extra.ok, true, JSON.stringify(extra)) + const waiting = await statusOf(again.call, alice.sessionToken, task.id) + assert.ok(waiting, 'the result-ready task is still listed') + assert.equal(waiting.status, 'result-ready', JSON.stringify(waiting)) +}) + test('a grantee may create a task on a granted child, not on an ungranted sibling', async (t) => { const { fb, signIn, createTask } = await start(t) const alice = await signIn('Alice') diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 85890a39a..ea02dc28c 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -48,13 +48,13 @@ Add a new invariant under the heading it belongs to (or a new heading), not at t ### Team state and access -- The saved team (`state.json`) is versioned. Version 2 (#47) carries the identity schema — `members` records, a per-Bot `ownerId`, the owner's `designatedChiefId`. A legacy version-1 file is migrated on load with a one-time `state.json.v1` backup; an existing backup counts only when it is a readable file holding a parseable version-1 state, and anything else (or any malformed ownership) fails closed (read-only). An older binary refuses a newer version read-only. This is schema foundation only — team mode stays disabled, see [team](team.md). On top of it, member auth (#48) adds host module `src/host/member-auth.js`, installed after `store` in `MODULES` (`core/index.js`): `invitations` and `authSessions` maps live on state (no `STATE_VERSION` bump), the channel resolves the actor from the `x-flybot-member-session` header, and endpoints are `auth-invite` / `auth-redeem` / `auth-revoke` / `auth-whoami` / `auth-remove-member` / `auth-revoke-invite` with `body.actorId` ignored; `auth-invite` alone also opens for an anonymous caller on a host with no sessions yet (bootstrap first invite), `auth-revoke-invite` kills an unredeemed invitation (redeeming it afterwards fails with `revoked`), `removeMember` refuses a member that still owns a Bot (`has_bots`), and the auth mutations are serialized behind an in-process lock so concurrent `removeMember` calls cannot both pass the last-owner check. Sign-in surface only — still not team mode; details in [team](team.md). On top of it, owner-scoped access (#49) adds host module `src/host/authz.js`, installed after member-auth: when any `authSessions` exist, the channel `authorize()` default-denies cross-owner Bot/room/session access, blocks shared non-isolatable resources (secrets, connectors, folder picks, browser bridge, update-install, diagnostics, set-prefs, `add-agent` — the DSH Agent is one per deployment and carries no owner) with `bot/blocked-shared` when more than one member exists, stamps `create-bot` with `actor.memberId`, and filters `view()`/`live()` to the actor; with no sessions the single-human behavior is unchanged. Session targets resolve through the store's owner maps — a Bot id via `botOf`, current chats and group Sessions and earlier parts via `selfOf`/`groupOwners`/`pastOwners` (a shared room Session also resolves for its current participants, #50) — and unresolvable ids deny. `approval-answer` gates on the approval's own Bot or room (resolved by approval id), never on caller-supplied session fields. `flybot/*` plugin endpoints (src/host/hooks.js) reach any signed-in actor for reads; plugin writes follow the shared rule. `view()`/`live()` narrowing (`rt.ownedView`/`rt.ownedLive` in authz.js) keeps per-session entries (`questions`, `exchangeTurns`) the actor owns, and deployment-wide surfaces (agents, secrets, connectors, memory reviews) only while a single member exists — the same rule as the shared writes; rooms the actor belongs to and the Bots in them (#50) stay visible. `create-room` refuses a request with neither a member Bot nor any participant. On top of it, room membership (#50) adds host module `src/host/room-membership.js`, installed after groups: a room may carry typed `participants` (`human` / local `bot` / granted remote `bot` with `handle` + owner fields, roles `admin` / `member`, `joinedSeq` / `leftSeq`); log lines carry `authorId` + `authorKind`; history is the membership window (current members and the owner of an invited Bot see `seq >=` their interval; leave withdraws access; rejoin starts a new interval); local cross-owner rooms accept each side's *granted* Bots (#90: default deny, Chief included); a friend's Bot from `social/remote-bots` (`cleanRemote`) may be added and ungranted Bots stay private, and a room with a friend's Bot holds only granted local Bots (a Bot that joined by room code, `viaCode`, only for that room); `view()` labels remotes with an owner badge and shows an invited owner **Shared room from \**. Face-to-face rooms (#83) reuse the same `participants` / membership-window / owner-badge presentation with `origin: 'face-to-face'`: room-only remotes may sit in the room without a person-grant or a friendship, `presentRoom` treats the viewer as a member (not Shared room from), leave skips the two-Bot floor, and `fanoutRoomLine` goes to `rt.faceRoomFanout` instead of friend `flybotSend`. Still not team mode — see [team](team.md). Per-Bot grants (#90) add host module `src/host/reach.js`, installed after room-membership: `state.grants[botId]` is the list of independent grants (one Bot → one grantee, optionally room-scoped); default deny, Chief included; a Bot whose owner is not a member has none. Grants are set with `rt.setGrants`, exposed as `BotView.granted`, `flybot.bot.hasGrant` / `hasAnyGrant`, `flybot.grants` and the `bot.grants` event. `reach()` is the owner policy: a friend's Bot may reach only local Bots granted to that sender, only a granted Bot (or one code-joined in that room) may message a friend's Bot, another owner's Bot is reached only if both sides have matching grants or both are members of the room the message belongs to. It is not a middleware: hooks.js runs it before the `bot.send` chain, again on approval release, and on every deliver with a sender or remote origin (resumed, retried, deferred, re-sent), so no verdict or `bypass` can skip it and a queued message is judged by what is true when it is delivered. Revoking a grant, or replacing its Chief, removes it from rooms that have friends' Bots or several owners at once (`rt.evictUnshared`). Approval binding (#52) completes the held-send half of this: `state.flybot.approvals` records carry `requestId`, a sha256 `version` over the request content, server-derived `ownerId` and `origin` (`via`/`hop`/audience) — claims in the message (`meta`, text) never confer authority. `flybot/decide` is allowed through authz for any signed-in actor and the per-record check happens in hooks (`decide` requires the sender Bot's owner's session, `bot/forbidden` otherwise, `bot/unauthorized` without a session); `release()` revalidates before dispatch and marks the record `stale` when the request changed, the sender or target Bot is gone (also invalidated eagerly on `deleteBot`), the Bot changed owners, or its Chief was demoted (`setMain` wrapper, sender side). `flybot/approvals` lists only the actor's own holds while enforcement is on; decided stale records keep their attribution in state but appear in no member's list. +- The saved team (`state.json`) is versioned. Version 2 (#47) carries the identity schema — `members` records, a per-Bot `ownerId`, the owner's `designatedChiefId`. A legacy version-1 file is migrated on load with a one-time `state.json.v1` backup; an existing backup counts only when it is a readable file holding a parseable version-1 state, and anything else (or any malformed ownership) fails closed (read-only). An older binary refuses a newer version read-only. This is schema foundation only — team mode stays disabled, see [team](team.md). On top of it, member auth (#48) adds host module `src/host/member-auth.js`, installed after `store` in `MODULES` (`core/index.js`): `invitations` and `authSessions` maps live on state (no `STATE_VERSION` bump), the channel resolves the actor from the `x-flybot-member-session` header, and endpoints are `auth-invite` / `auth-redeem` / `auth-revoke` / `auth-whoami` / `auth-remove-member` / `auth-revoke-invite` with `body.actorId` ignored; `auth-invite` alone also opens for an anonymous caller on a host with no sessions yet (bootstrap first invite), `auth-revoke-invite` kills an unredeemed invitation (redeeming it afterwards fails with `revoked`), `removeMember` refuses a member that still owns a Bot (`has_bots`), and the auth mutations are serialized behind an in-process lock so concurrent `removeMember` calls cannot both pass the last-owner check. Sign-in surface only — still not team mode; details in [team](team.md). On top of it, owner-scoped access (#49) adds host module `src/host/authz.js`, installed after member-auth: when any `authSessions` exist, the channel `authorize()` default-denies cross-owner Bot/room/session access, blocks shared non-isolatable resources (secrets, connectors, folder picks, browser bridge, update-install, diagnostics, set-prefs, `add-agent` — the DSH Agent is one per deployment and carries no owner) with `bot/blocked-shared` when more than one member exists, stamps `create-bot` with `actor.memberId`, and filters `view()`/`live()` to the actor; with no sessions the single-human behavior is unchanged. Session targets resolve through the store's owner maps — a Bot id via `botOf`, current chats and group Sessions and earlier parts via `selfOf`/`groupOwners`/`pastOwners` (a shared room Session also resolves for its current participants, #50) — and unresolvable ids deny. `approval-answer` gates on the approval's own Bot or room (resolved by approval id), never on caller-supplied session fields. `flybot/*` plugin endpoints (src/host/hooks.js) reach any signed-in actor for reads; plugin writes follow the shared rule. `view()`/`live()` narrowing (`rt.ownedView`/`rt.ownedLive` in authz.js) keeps per-session entries (`questions`, `exchangeTurns`) the actor owns, and deployment-wide surfaces (agents, secrets, connectors, memory reviews) only while a single member exists — the same rule as the shared writes; rooms the actor belongs to and the Bots in them (#50) stay visible. `create-room` refuses a request with neither a member Bot nor any participant. On top of it, room membership (#50) adds host module `src/host/room-membership.js`, installed after groups: a room may carry typed `participants` (`human` / local `bot` / granted remote `bot` with `handle` + owner fields, roles `admin` / `member`, `joinedSeq` / `leftSeq`); log lines carry `authorId` + `authorKind`; history is the membership window (current members and the owner of an invited Bot see `seq >=` their interval; leave withdraws access; rejoin starts a new interval); local cross-owner rooms accept each side's *granted* Bots (#90: default deny, Chief included); a friend's Bot from `social/remote-bots` (`cleanRemote`) may be added and ungranted Bots stay private, and a room with a friend's Bot holds only granted local Bots (a Bot that joined by room code, `viaCode`, only for that room); `view()` labels remotes with an owner badge and shows an invited owner **Shared room from \**. Face-to-face rooms (#83) reuse the same `participants` / membership-window / owner-badge presentation with `origin: 'face-to-face'`: room-only remotes may sit in the room without a person-grant or a friendship, `presentRoom` treats the viewer as a member (not Shared room from), leave skips the two-Bot floor, and `fanoutRoomLine` goes to `rt.faceRoomFanout` instead of friend `flybotSend`. Still not team mode — see [team](team.md). Per-Bot grants (#90) add host module `src/host/reach.js`, installed after room-membership: `state.grants[botId]` is the list of independent grants (one Bot → one grantee, optionally room-scoped); default deny, Chief included; a Bot whose owner is not a member has none. Grants are set with `rt.setGrants`, exposed as `BotView.granted`, `flybot.bot.hasGrant` / `hasAnyGrant`, `flybot.grants` and the `bot.grants` event. `reach()` is the owner policy: a friend's Bot may reach only local Bots granted to that sender, only a granted Bot (or one code-joined in that room) may message a friend's Bot, another owner's Bot is reached only if both sides have matching grants or both are members of the room the message belongs to. It is not a middleware: hooks.js runs it before the `bot.send` chain, again on approval release, and on every deliver with a sender or remote origin (resumed, retried, deferred, re-sent), so no verdict or `bypass` can skip it and a queued message is judged by what is true when it is delivered. Revoking a grant, or replacing its Chief, removes it from rooms that have friends' Bots or several owners at once (`rt.evictUnshared`). Approval binding (#52) completes the held-send half of this: `state.flybot.approvals` records carry `requestId`, a sha256 `version` over the request content, server-derived `ownerId` and `origin` (`via`/`hop`/audience) — claims in the message (`meta`, text) never confer authority. `flybot/decide` is allowed through authz for any signed-in actor and the per-record check happens in hooks (`decide` requires the sender Bot's owner's session, `bot/forbidden` otherwise, `bot/unauthorized` without a session); `release()` revalidates before dispatch and marks the record `stale` when the request changed, the sender or target Bot is gone (also invalidated eagerly on `deleteBot`), or the Bot changed owners; a demoted Chief's sender-side holds are invalidated eagerly on `setMain` (not inside `staleReason`), and `release()` re-checks immediately before dispatch so an in-flight approve cannot go out after a concurrent invalidation. Unknown `invalidateFor` field values throw. `flybot/approvals` lists only the actor's own holds while enforcement is on; decided stale records keep their attribution in state but appear in no member's list. -- Context sharing (#53) is specified in [team](team.md), **Context sharing foundation**: the own-chat brief is share-gated (`share-brief` / `briefShared`), a memory review reads only its owner as the user (`isUserLine`) and labels another human as `peer` (not `bot`), and keeps cross-owner entries out of the team scope (`teamAllowedFrom`), `list_bots` lists only the caller's owner's Bots (a granted child: that child's `createdBy` subtree), Bots granted to the same owner, and Bots in the current room — another owner's folder and instructions stay private — and a `dsh-session:` chip in `send` / `room-post` must resolve for the author (`sessionOwnedBy`). Host modules: `src/host/groups.js`, `src/host/memory.js`, `src/host/tools.js`, `src/host/channel.js`, `src/host/room-membership.js`, `src/host/authz.js`. Team mode stays disabled. +- Context sharing (#53) is specified in [team](team.md), **Context sharing foundation**: the own-chat brief is share-gated (`share-brief` / `briefShared`), a memory review reads only its owner as the user (`isUserLine`) and labels another human as `peer` (not `bot`), and keeps cross-owner entries out of the team scope (`teamAllowedFrom`), `list_bots` lists only the caller's owner's Bots (a granted child: that child's `createdBy` subtree), Bots granted to the same owner, and Bots in the current room — another owner's folder and instructions stay private — and a `dsh-session:` chip in actor-authored `send` / `room-post` / `answer-question` / `schedule-create` text must resolve for the author (`sessionOwnedBy`; `schedule-create` authz keys off `body.target`). Bot-authored group posts drop chips so they cannot tunnel a private chat into the room. Host modules: `src/host/groups.js`, `src/host/memory.js`, `src/host/tools.js`, `src/host/channel.js`, `src/host/room-membership.js`, `src/host/authz.js`. Team mode stays disabled. ### Tasks -A tracked request with a stable id (`task-`) and a small state machine, owned by `src/host/tasks.js` with `state.tasks` (additive; corrupt values reset, no version bump). States: `open → claimed → working → blocked ⇄ working → result-ready → accepted`, with `cancelled`/`failed` terminal and `accepted` reachable only from `result-ready` — a failed or cancelled task can never display as accepted (#54). Transitions after create are the requester's (server-derived `requesterId`, the signed-in actor or the single owner; `bot/forbidden` otherwise, `bot/stale` on invalid moves), recorded with per-status actor entries. Create does not require owning the Bot: a matching per-Bot grant, or both the requester and the Bot as current members of the named room, is enough; outgoing holds stay the owner's (#52 card). A working task's held send blocks it with the approval id; the owner's decide resumes (approve) or cancels (reject) it; deleting the responsible Bot fails its open tasks. Endpoints `tasks` / `task-create` / `task-accept` / `task-start` / `task-return` / `task-accept-result` / `task-cancel` pass authz for any signed-in actor and are record-checked in tasks.js. Records store pointers and a copied summary, never private transcripts. Team mode stays disabled. +A tracked request with a stable id (`task-`) and a small state machine, owned by `src/host/tasks.js` with `state.tasks` (additive; corrupt values reset, no version bump). States: `open → claimed → working → blocked ⇄ working → result-ready → accepted`, with `cancelled`/`failed` terminal and `accepted` reachable only from `result-ready` — a failed or cancelled task can never display as accepted (#54). Transitions after create are the requester's (server-derived `requesterId`, the signed-in actor or the single owner; `bot/forbidden` otherwise, `bot/stale` on invalid moves), recorded with per-status actor entries. Create does not require owning the Bot: a matching per-Bot grant, or both the requester and the Bot as current members of the named room, is enough; outgoing holds stay the owner's (#52 card). A working task's held send blocks **that** task (`meta.taskId`, or the Bot's only working task) with the approval id; the owner's decide resumes (approve) or cancels (reject) it; deleting the responsible Bot fails its open tasks. `result-ready` is protected from prune while it waits for acceptance. Endpoints `tasks` / `task-create` / `task-accept` / `task-start` / `task-return` / `task-accept-result` / `task-cancel` are an explicit authz allow-list and are record-checked in tasks.js. Records store pointers and a copied summary, never private transcripts. Clarification and `workBotId` wait on #55's exchange wiring. Team mode stays disabled. ### Inbound shared rooms diff --git a/docs/subsystems/ds-bot-core.md b/docs/subsystems/ds-bot-core.md index 3b5760e3a..2e80fe968 100644 --- a/docs/subsystems/ds-bot-core.md +++ b/docs/subsystems/ds-bot-core.md @@ -34,7 +34,7 @@ This page covers the pin, the patch format and the core's dev tooling. The gener | `ds-bot.pin.json` | Upstream pin. | | `patches/ds-bot/` | Ordered patch topics. | | `scripts/ds-bot-patches.mjs` | generate / check patches. | -| `core/src/host/hooks.js` (approvals) | Held sends bound to their owner — provenance, owner-bound decisions, stale revalidation. The authority contract lives in [team](team.md) (#52). | +| `core/src/host/hooks.js` (approvals) | Held sends bound to their owner — provenance, owner-bound decisions, stale revalidation. Chief replacement invalidates sender-side holds eagerly on `setMain` (not via `staleReason`). The authority contract lives in [team](team.md) (#52). | ## Entry points diff --git a/docs/subsystems/plugin-runtime.md b/docs/subsystems/plugin-runtime.md index 3a62af593..5a4bc9161 100644 --- a/docs/subsystems/plugin-runtime.md +++ b/docs/subsystems/plugin-runtime.md @@ -51,8 +51,8 @@ FlyBot's plugin API surface on top of the patched core: host hooks, client slots 7. `cleanRemote` is the record boundary for a friend's granted Bot: `name`, `homeName` and `owner` strip `@` (so they cannot look like a mention of another Bot in `nameMentions` chips/previews) and control characters/newlines, then cap at 60 characters. Handles stay on the `handle` field. Those labels then flow into delivery `fromName` (still `cleanName`'d and the body fenced) and into `list_bots` friends lines via the remote-chat roster. Group room logs use local team names, not peer labels. 8. A user's @ of a friend's Bot in a group gets a receipt line in that group (`hooks.js` `mentionReceipt`, relay role `mention`): where the reply lands, or why the send was denied, held, or failed. The receipt's role stays outside the room round trigger (`groups.js` `opens`), so it never opens a round; peer names go through `cleanName`. 9. `flybot.rooms` hosts a Bot Social face-to-face matching room as a native room (`upsertFace` / `applyFaceLine` / `ofFace` / `dropFace` / `onFaceLine` / `postNotice` / `postRemote`). Slot `flybot.room.overview` is a block at the top of a group's overview: while joining, a tile grid (avatar above name + owner) with **Leave** and **Enter chat** on one row; once open, compact **Leave** and Auto-send — not Open/Start/Leave together. Slot `flybot.new-chat.action` adds a row to the sidebar **+** short action menu (next to Create group chat; `label` / `hint` go through the same `t()` map). Still not team mode. -10. Held sends are owner-bound (#52): a hold stamps server-derived provenance on the record — `requestId`, a sha256 `version` over the request content, `origin` (`via`/`hop`/audience) and the sender Bot's `ownerId`; message claims (`meta`, text) never confer authority. `flybot/decide` passes authz for any signed-in actor and hooks binds the decision to the sender Bot's owner's session; `release` revalidates and marks records `stale` (changed request, deleted sender/target Bot, changed owner, demoted Chief — `deleteBot`/`setMain` also invalidate eagerly), and the pending list is filtered per actor under member enforcement. The default approval card shows `senderName`/`targetName` and a delegated marker (`Approval.origin`). -11. Task lifecycle (#54): a working task's held send calls `rt.taskHeld` (blocks on that approval); the owner's `flybot/decide` calls `rt.taskDecided` (approve resumes, reject cancels); `deleteBot` of a local Bot calls `rt.taskBotDeleted` (open tasks fail). Incoming holds stay the owner's. The state machine lives in `tasks.js`; hooks only fire those three seams. +10. Held sends are owner-bound (#52): a hold stamps server-derived provenance on the record — `requestId`, a sha256 `version` over the request content, `origin` (`via`/`hop`/audience) and the sender Bot's `ownerId`; message claims (`meta`, text) never confer authority. `flybot/decide` passes authz for any signed-in actor and hooks binds the decision to the sender Bot's owner's session; `release` revalidates and marks records `stale` (changed request, deleted sender/target Bot, changed owner — `deleteBot` invalidates eagerly; a demoted Chief's sender-side holds are invalidated eagerly on `setMain`, not inside `staleReason`). `release()` re-checks immediately before dispatch so an in-flight approve cannot go out after a concurrent invalidation. Unknown `invalidateFor` field values throw. The pending list is filtered per actor under member enforcement. The default approval card shows `senderName`/`targetName` and a delegated marker (`Approval.origin`). +11. Task lifecycle (#54): a working task's held send calls `rt.taskHeld` (blocks **that** task: `meta.taskId`, or the Bot's only working task); the owner's `flybot/decide` calls `rt.taskDecided` (approve resumes, reject cancels); `deleteBot` of a local Bot calls `rt.taskBotDeleted` (open tasks fail). Incoming holds stay the owner's. The state machine lives in `tasks.js`; hooks only fire those three seams. Clarification and `workBotId` wait on #55. ### Chat-row pending badge diff --git a/docs/subsystems/team.md b/docs/subsystems/team.md index 54dee3a4f..e4d3425a0 100644 --- a/docs/subsystems/team.md +++ b/docs/subsystems/team.md @@ -93,7 +93,7 @@ The held-send approval (`state.flybot.approvals` in `core/src/host/hooks.js`) no - **Provenance is server-derived.** A hold stamps `requestId` (the send's `exchangeId`), a sha256 `version` over the request content (from/to/text/role/attachment names), `origin` (`via`, `hop`, room audience when known), and `ownerId` — the sender Bot's owner, resolved from the team state. Claims inside the message (`meta`, body text such as "my owner approved") are data on the record, never authority. - **Decisions are owner-bound.** `flybot/decide` passes authz for any signed-in actor; hooks.js then requires the deciding session to belong to the sender Bot's owner (`bot/forbidden` otherwise, `bot/unauthorized` with no session). One owner cannot approve, reject or retry another owner's held send, and the pending list shows a member only their own holds. -- **Revalidation at approve/execute time.** Releasing a hold re-checks the record: a changed request content, a deleted sender or target Bot, a Bot that changed owners, or a demoted Chief marks the record `stale` — it is never dispatched. `deleteBot` and Chief replacement (`setMain`) invalidate affected pending holds eagerly; a removed member's sessions cannot decide, and their holds were already stale when their last Bot was deleted (`removeMember` refuses a member that still owns one). +- **Revalidation at approve/execute time.** Releasing a hold re-checks the record: a changed request content, a deleted sender or target Bot, or a Bot that changed owners marks the record `stale` — it is never dispatched. A demoted Chief's sender-side holds are invalidated eagerly on `setMain` (not inside `staleReason`); `deleteBot` invalidates holds that name that Bot as sender or target. `release()` re-checks immediately before dispatch so an in-flight approve cannot go out after a concurrent invalidation. A removed member's sessions cannot decide, and their holds were already stale when their last Bot was deleted (`removeMember` refuses a member that still owns one). Unknown `invalidateFor` field values throw rather than no-op. - **Legacy records keep working.** Holds saved before this change have no `version`; they skip only the tamper check, and their owner is derived at decide time. - **Not done here:** the social plugin's server-side approval holds (`social/decide-approval`) stay a deployment-wide plugin write (blocked while more than one member exists); the in-chat approval cards and task-level approvals are #90/#54 work. Gate 3 is satisfied for held sends only. @@ -104,12 +104,12 @@ Private context crosses owners only through an explicit, owner-granted share; ro - **The own-chat brief is share-gated.** A member's group Session starts with its owner's own-chat brief only when the room has a single owner, or that owner called `share-brief` for this room (the `briefShared` flag lives on the room participant, so leaving drops it; `rt.setBriefShare` in `core/src/host/room-membership.js` owner-checks, the endpoint only needs a signed-in actor). A group Session created after the share is taken back carries no brief. A granted remote Bot in the room counts as another owner (its handle stands for its home), so a friend's Bot's presence also suppresses the brief. A brief already delivered is not retroactively unseen — document that to owners. - **Memory review is owner-aware.** In `core/src/host/memory.js`, "the user" of a group review is the member Bot's owner (`isUserLine`); another owner's human is a named `peer` (not `bot`) and does not get user-line retention. A review that reads a room with more than one owner cannot write the team scope (`teamAllowedFrom`) — entries land in the Bot's own memory. Entries already written are not retroactively unseen. - **Roster disclosure narrows.** `list_bots` lists the caller's owner's Bots, Bots granted to that owner, and Bots in the current room — never an ungranted Bot of another owner. A granted child triggered by a grantee is scoped to its own `createdBy` subtree and does not dump the owner's roster with `workDir` / instructions. Another owner's folder and instructions stay private when that Bot is listed at all. -- **Chat references stay owner-scoped.** A `dsh-session:` reference chip in `send` or `room-post` must name a Session the author may read (`sessionOwnedBy`); the snapshot rides to every recipient, so authorship is the gate. -- **Not done here:** reference chips are author-checked on `send` and `room-post` only — whether DSH resolves chips in `answer-question`/`schedule-create` texts and in Bot-authored messages is an open DSH-side question (tracked follow-up); the secrets surface stays `SHARED_BLOCKED` for >1 member; previously received messages cannot be retroactively unseen (stated, not engineered); no per-owner file-sandbox changes — a room `workDir` remains readable by its member Bots under DSH's own policy. **Follow-up (brief share):** `briefAllowed` still withholds the owner's own-chat brief in any multi-owner room, including face-to-face rooms where someone brings their own Bot; the intended model is a grant-tied share (a flag on `state.grants` or the room-scoped grant) rather than a participant `briefShared` flag lost on leave. `setBriefShare` already requires the Bot to be a current room participant. That redesign is out of this #54 PR. +- **Chat references stay owner-scoped.** A `dsh-session:` reference chip in actor-authored text that becomes a session user message (`send`, `room-post`, `answer-question`, `schedule-create`) must name a Session the author may read (`sessionOwnedBy`); the snapshot rides to every recipient, so authorship is the gate. `schedule-create` authz keys off `body.target` (the Bot or room the endpoint uses), not a caller-chosen `sessionId`. Bot-authored group posts (`postToRoom`) drop chips so a member Bot cannot tunnel a private chat into the room. DSH resolves chips on user-shaped followups in the group's own Session (`session-reference` / `attachReference`) and attaches the snapshot only to the last user post, not a Bot's post. +- **Not done here:** bot→bot own-chat followups are not stripped (the snapshot stays in that Bot's chat; not a shared-room leak); the secrets surface stays `SHARED_BLOCKED` for >1 member; previously received messages cannot be retroactively unseen (stated, not engineered); no per-owner file-sandbox changes — a room `workDir` remains readable by its member Bots under DSH's own policy. **Follow-up (brief share):** `briefAllowed` still withholds the owner's own-chat brief in any multi-owner room, including face-to-face rooms where someone brings their own Bot; the intended model is a grant-tied share (a flag on `state.grants` or the room-scoped grant) rather than a participant `briefShared` flag lost on leave. `setBriefShare` already requires the Bot to be a current room participant. That redesign is out of this #54 PR. ## Task lifecycle foundation (landed, #54) -`state.tasks` (`core/src/host/tasks.js`): a request becomes a tracked task — `open → claimed → working → blocked ⇄ working → result-ready → accepted`, `cancelled`/`failed` terminal, `accepted` only from `result-ready`. Only the requester's session moves their task (`bot/forbidden` otherwise). Create does not require owning the Bot: a matching per-Bot grant, or both the requester and the Bot as current members of the named room, is enough — a granted child can take a handoff within its authority. Outgoing holds stay the owner's: a held send (#52) blocks the task until that owner decides. Records carry per-status actor entries, pointers to the originating conversation, and a summary — never private transcripts. The in-room task view and audit trail are #57's; per-task hold linkage, prune window, authz prefix, clarification and `workBotId` are #111's. Team mode stays disabled. +`state.tasks` (`core/src/host/tasks.js`): a request becomes a tracked task — `open → claimed → working → blocked ⇄ working → result-ready → accepted`, `cancelled`/`failed` terminal, `accepted` only from `result-ready`. Only the requester's session moves their task (`bot/forbidden` otherwise). Create does not require owning the Bot: a matching per-Bot grant, or both the requester and the Bot as current members of the named room, is enough — a granted child can take a handoff within its authority. Outgoing holds stay the owner's: a held send (#52) blocks **that** task (`meta.taskId`, or the Bot's only working task) until that owner decides — several working tasks without a name stay working. `result-ready` is protected from prune while it waits for acceptance. Task channel names are an explicit allow-list (`tasks` / `task-create` / `task-accept` / `task-start` / `task-return` / `task-accept-result` / `task-cancel`). Records carry per-status actor entries, pointers to the originating conversation, and a summary — never private transcripts. The in-room task view and audit trail are #57's; clarification (ask the requester) and `workBotId` (the specialist on the exchange) wait on #55's exchange wiring. Team mode stays disabled. ## Identity model @@ -227,10 +227,10 @@ Owner-scoped access (#49) tests in `core/test/authz.test.js`: cross-owner denial **Room membership foundation (#50)** has tests in `core/test/room-membership.test.js`: Alice, Bob and both Chiefs keep distinct authorship; concurrent posts keep order and identity; unauthorized history reads fail; join/leave follows the membership-window rule; non-members cannot inject (forged `authorId` is ignored); existing single-owner groups still display and accept posts; a specialist cannot join a cross-owner room. -**Approval binding (#52)** has tests in `core/test/approval-binding.test.js`: the hold stamps server-derived provenance and a request-content hash while forged `meta` claims stay data; only the sender Bot's owner's session may decide (another member gets `bot/forbidden`, no session gets `bot/unauthorized`, a body `actorId` grants nothing); the pending list is per-actor; a request that changed after it was held goes `stale` and never dispatches; same-owner deciding survives a restart; deleting a sender or target Bot or replacing the Chief invalidates pending holds; and a record whose sender Bot and owner are both gone stays in state with its attribution but in no member's list. +**Approval binding (#52)** has tests in `core/test/approval-binding.test.js`: the hold stamps server-derived provenance and a request-content hash while forged `meta` claims stay data; only the sender Bot's owner's session may decide (another member gets `bot/forbidden`, no session gets `bot/unauthorized`, a body `actorId` grants nothing); the pending list is per-actor; a request that changed after it was held goes `stale` and never dispatches; same-owner deciding survives a restart; deleting a sender or target Bot or replacing the Chief invalidates pending *sender-side* holds, while a hold *addressed to* the demoted Chief stays pending and decidable; an in-flight approve is invalidated if the Chief is replaced before dispatch; and a record whose sender Bot and owner are both gone stays in state with its attribution but in no member's list. -**Context sharing (#53)** has tests in `core/test/context-sharing.test.js` (brief gate per room ownership, the owner-only `share-brief` round trip, the single-owner guard, `list_bots` narrowing to own / granted / current-room Bots and granted-child subtree scoping, reference-chip authorization) and in `core/test/memory.test.js` (owner-as-user classification, the team-write gate out of a cross-owner room). +**Context sharing (#53)** has tests in `core/test/context-sharing.test.js` (brief gate per room ownership, the owner-only `share-brief` round trip, the single-owner guard, `list_bots` narrowing to own / granted / current-room Bots and granted-child subtree scoping, reference-chip authorization on `send`/`room-post`/`answer-question`/`schedule-create`, and chip stripping on Bot group posts) and in `core/test/memory.test.js` (owner-as-user classification, the team-write gate out of a cross-owner room). -**Task lifecycle (#54)** has tests in `core/test/tasks.test.js`: the happy path (open → claimed → working → result-ready → accepted, then stale), wrong-member accept/claim/cancel forbidden, accept-before-result and post-terminal moves stale, held-send block → owner approve resumes, owner reject cancels, responsible-Bot deletion fails the task, restart persistence, corrupt `state.tasks` resets, and a grantee may create a task on a granted child but not on an ungranted sibling. +**Task lifecycle (#54)** has tests in `core/test/tasks.test.js`: the happy path (open → claimed → working → result-ready → accepted, then stale), wrong-member accept/claim/cancel forbidden, accept-before-result and post-terminal moves stale, held-send block → owner approve resumes, owner reject cancels, a named hold blocks only that working task (an untagged hold does not cancel every sibling), `result-ready` survives prune, responsible-Bot deletion fails the task, restart persistence, corrupt `state.tasks` resets, and a grantee may create a task on a granted child but not on an ungranted sibling. The routing-matrix rows above remain untested because the behavior is not shipped. When enforcement work starts (#48+), each row becomes a test first, per [fail-without-change](../code-standards.md): same-owner delegation allowed (a), Chief-to-Chief allowed (b), cross-owner specialist denied (c), team-admin vs private-owner denied (d), forwarded approval rejected (e). Each must fail on the unbroken tip before implementation lands. diff --git a/patches/ds-bot/0005-social-seams.patch b/patches/ds-bot/0005-social-seams.patch index e7a0a59a1..654bd8b46 100644 --- a/patches/ds-bot/0005-social-seams.patch +++ b/patches/ds-bot/0005-social-seams.patch @@ -181,7 +181,7 @@ diff --git a/src/host/channel.js b/src/host/channel.js defaultModel: rt.cachedDefault() ?? null, permissions: rt.permissionCatalog?.() ?? null, modelsKey: modelsKey(), -@@ -65,22 +65,40 @@ export function install(rt) { +@@ -65,22 +65,47 @@ export function install(rt) { memory: rt.memoryEnabled !== false, // Set while another dsh host holds the team (store.js): why this one cannot change it. readOnly: rt.readOnly() ?? null, @@ -221,11 +221,18 @@ diff --git a/src/host/channel.js b/src/host/channel.js + const READS = new Set(['state', 'models', 'efforts', 'earlier', 'room-progress', 'room-history', 'exchange', 'usage', 'schedules', 'memory-view', 'browser-wait', 'browser-result', 'diagnostics', 'diagnostics-log', 'diagnostics-clear', 'update-check', 'update-install', 'dsh-update-install', 'repo-stars', 'approval-answer', 'auth-whoami']) + + // A chat reference chip names a Session whose snapshot rides to every recipient of -+ // the message, so the author must be able to read what they share (#53). ++ // the message, so the author must be able to read what they share (#53, #106). ++ // Actor-authored text that becomes a session user message: send, room-post, ++ // answer-question, schedule-create. Bot-authored room posts strip chips in groups.js. ++ // Menu chips encode the session id as base64url(JSON); raw ids still appear in tests. ++ const decodeSessionChip = (payload) => { ++ try { return JSON.parse(Buffer.from(payload, 'base64url').toString('utf8')) } catch { return undefined } ++ } + const unshareableReference = (actor, text) => { + if (!actor || !rt.authzEnforced?.() || typeof text !== 'string') return undefined + for (const match of text.matchAll(/dsh-session:([A-Za-z0-9_-]+)/g)) { -+ if (!rt.sessionOwnedBy?.(actor, match[1])) return match[1] ++ const id = decodeSessionChip(match[1]) ?? match[1] ++ if (!rt.sessionOwnedBy?.(actor, id)) return id + } + return undefined + } @@ -316,6 +323,32 @@ diff --git a/src/host/channel.js b/src/host/channel.js } case 'folders': case 'folder-pick': +@@ -242,10 +242,15 @@ export function install(rt) { + await rt.load() + return ok(await rt.capabilityEndpoint(endpoint, body)) + case 'schedules': +- case 'schedule-create': + case 'schedule-delete': + await rt.load() + return ok(await rt.scheduleEndpoint(endpoint, body)) ++ case 'schedule-create': { ++ await rt.load() ++ const leaked = unshareableReference(actor, [body.title, body.prompt].filter(part => typeof part === 'string').join('\n')) ++ if (leaked !== undefined) return fail('You do not own that chat', 'bot/forbidden') ++ return ok(await rt.scheduleEndpoint(endpoint, body)) ++ } + case 'memory-view': + case 'memory-ask': + case 'memory-forget': +@@ -260,6 +260,8 @@ export function install(rt) { + if (question === undefined || question.id !== body.questionId) return fail('This question is no longer open', 'bot/stale') + const reply = questionReply(question, body.answer) + if (reply === undefined) return fail('Nothing to send', 'bot/invalid') ++ const leaked = unshareableReference(actor, reply.text) ++ if (leaked !== undefined) return fail('You do not own that chat', 'bot/forbidden') + // Claim the question before queueing so a second answer is refused. + delete state.questions[sessionId] + try { @@ -278,9 +278,36 @@ export function install(rt) { const text = String(body.text ?? '').trim() if (text === '' || typeof body.sessionId !== 'string') return fail('Nothing to send', 'bot/invalid') diff --git a/patches/ds-bot/0012-room-membership.patch b/patches/ds-bot/0012-room-membership.patch index be3560405..6c7d568ff 100644 --- a/patches/ds-bot/0012-room-membership.patch +++ b/patches/ds-bot/0012-room-membership.patch @@ -320,7 +320,7 @@ diff --git a/src/client/sources.js b/src/client/sources.js diff --git a/src/host/groups.js b/src/host/groups.js --- a/src/host/groups.js +++ b/src/host/groups.js -@@ -22,6 +22,7 @@ import { escapeRegExp, splitRef, textOf } from './text.js' +@@ -22,11 +22,29 @@ import { escapeRegExp, splitRef, textOf } from './text.js' import { mentionSpans } from '../shared/mentions.js' import { GROUP_REF, output } from './tools.js' import { failureLine } from './turns.js' @@ -328,6 +328,28 @@ diff --git a/src/host/groups.js b/src/host/groups.js const ROOM_LOG_MAX = 60 const ROOM_FIRST_LOOK = 12 + const ROOM_DELTA_MAX = 24 + const ROOM_EARLIER_MAX = 8 ++// Bot-authored group followups are createUserMessage; DSH may resolve `dsh-session:` ++// chips there and attach the snapshot to the last user post. Drop chips so a member ++// Bot cannot tunnel a private chat into the room (#106). Authorship stays the gate. ++function stripSessionChips(text) { ++ if (typeof text !== 'string' || !text.includes('dsh-session:')) return text ++ // Loop until stable: a nested marker like dsh-sessiondsh-session:x:ImQi must not ++ // reconstitute a valid chip after one pass (#106 Codex review). ++ let out = text ++ for (let previous; previous !== out;) { ++ previous = out ++ out = out ++ .replace(/@\[((?:\\.|[^\]\\])*)\]\(dsh-session:[A-Za-z0-9_-]+\)/g, (_, label) => `@${String(label).replace(/\\([\\\]])/g, '$1')}`) ++ .replace(/dsh-session:[A-Za-z0-9_-]+/g, '') ++ } ++ if (out.includes('dsh-session:')) out = out.replace(/dsh-session:/g, '') ++ return out ++} + const ROOM_EXTRA_TURNS = 4 + const ROOM_ADMIN_EXTRA_TURNS = 4 + const ROOM_TURNS_EACH = 2 @@ -41,9 +41,12 @@ const ensureRoomLog = (room) => { room.seen ??= {} } @@ -454,6 +476,15 @@ diff --git a/src/host/groups.js b/src/host/groups.js return lines.length === 0 ? `"${room.name}" has no messages yet.` : `Latest in "${room.name}" (oldest first):\n${transcript(lines)}` }, })) +@@ -566,7 +566,7 @@ export function install(rt) { + async function postToRoom(room, sender, text, extra = {}) { + const agent = await rt.resolveAgent(room.id) + agent.followup(createUserMessage({ +- content: [{ type: 'text', text: `[Group post from ${sender.name}]\n${text}` }], ++ content: [{ type: 'text', text: `[Group post from ${sender.name}]\n${stripSessionChips(text)}` }], + source: { kind: SOURCE_KIND, form: 'relay', role: 'post', exchangeId: randomUUID(), senderName: sender.name, senderSessionId: sender.id, roomId: room.id, hop: 0, ...extra }, + })) + await ctx.sessions.flush(agent.session) @@ -615,7 +615,10 @@ export function install(rt) { const member = rt.botOf(owner?.botId) if (room === undefined || member === undefined || !room.members.includes(member.id)) return diff --git a/patches/ds-bot/files/src/host/authz.js b/patches/ds-bot/files/src/host/authz.js index b16f959ea..ddb529c24 100644 --- a/patches/ds-bot/files/src/host/authz.js +++ b/patches/ds-bot/files/src/host/authz.js @@ -49,6 +49,12 @@ const ROOM_TARGETED = new Set(['update-room', 'delete-room', 'room-progress', 'c // Endpoints whose body names a Session. const SESSION_TARGETED = new Set(['earlier', 'send', 'answer-question', 'dismiss-question', 'approval-answer', 'exchange']) +// Task channel names. Requester checks live on the record in tasks.js; listing them +// here keeps a future `task-*` from auto-allowing for any signed-in actor (#111). +const TASK_ENDPOINTS = new Set([ + 'tasks', 'task-create', 'task-accept', 'task-start', 'task-return', 'task-accept-result', 'task-cancel', +]) + const deny = (code, message) => ({ ok: false, code, message }) const ALLOW = { ok: true } @@ -217,6 +223,16 @@ export function install(rt) { if (BOT_TARGETED.has(ep)) { if (ep === 'schedules' || ep === 'schedule-create' || ep === 'schedule-delete') { + // The create endpoint keys off `body.target` (Bot or room id); `sessionId` is + // the list/delete key. Checking only sessionId would allow a foreign target + // paired with an owned session (#106). + if (ep === 'schedule-create') { + const target = body?.target + if (typeof target === 'string' && target !== '') { + if (botOwnedBy(actor, target) || roomOwnedBy(actor, target) || roomMemberOf(actor, target)) return ALLOW + return deny('bot/forbidden', 'You do not own that Bot or group chat') + } + } if (ep === 'schedule-delete' && typeof body?.id === 'string' && !sessionOwnedBy(actor, body.id) && !sessionOwnedBy(actor, body.sessionId)) return deny('bot/forbidden', 'You do not own that schedule') return requireSession(body?.sessionId) } @@ -327,8 +343,9 @@ export function install(rt) { if (ep === 'share-brief') return ALLOW // Task transitions are requester-checked on the record in tasks.js (#54) — the // endpoint only needs a signed-in actor, like held-send decisions. Create is - // grant-aware there (owner, matching grant, or the named room). - if (ep === 'tasks' || ep.startsWith('task-')) return ALLOW + // grant-aware there (owner, matching grant, or the named room). Explicit set: + // a future `task-*` name must be listed here, not auto-allowed (#111). + if (TASK_ENDPOINTS.has(ep)) return ALLOW // Anything not in the table and not a known auth/read endpoint is denied. if (NO_ACTOR_ALLOWED.has(ep) || ep.startsWith('auth-')) return ALLOW diff --git a/patches/ds-bot/files/src/host/hooks.js b/patches/ds-bot/files/src/host/hooks.js index 0910aee15..b4fc36fe7 100644 --- a/patches/ds-bot/files/src/host/hooks.js +++ b/patches/ds-bot/files/src/host/hooks.js @@ -328,10 +328,17 @@ export function install(rt) { // Queued work whose authority is gone does not wait forever: a hold naming one of // these Bots as sender or target is marked stale (still listed for the audit view). // `field` narrows the match ('from': sender-side only, e.g. a demoted Chief). + // Unknown values must throw: a silent no-op would skip invalidation (#97). const invalidateFor = async (botIds, reason, field) => { + if (field !== undefined && field !== 'from') { + throw new Error(`flybot: invalidateFor field must be 'from' or omitted, got ${JSON.stringify(field)}`) + } await rt.load() const ids = new Set(botIds) - const hit = Object.values(store()).filter(a => a.status === 'pending' + // Pending, and approved-but-not-yet-dispatched: otherwise a concurrent setMain / + // deleteBot during release() skips the record (status already `approved`) (#97). + const inFlight = a => a.status === 'pending' || (a.status === 'approved' && a.exchangeId === undefined) + const hit = Object.values(store()).filter(a => inFlight(a) && (ids.has(a.request?.from) && (field === undefined || field === 'from') || ids.has(a.request?.to) && field === undefined)) if (hit.length === 0) return @@ -415,6 +422,15 @@ export function install(rt) { } return } + // Re-check immediately before dispatch: evaluate() yielded, so a concurrent + // setMain/deleteBot may have marked this in-flight approve stale (#97). + if (approval.status === 'stale') return + const still = staleReason(approval) + if (still !== undefined) { + approval.status = 'stale' + approval.error = still + return + } try { approval.exchangeId = await dispatch(approval.request) } catch (error) { diff --git a/patches/ds-bot/files/src/host/tasks.js b/patches/ds-bot/files/src/host/tasks.js index cfadfca09..5b4e286bb 100644 --- a/patches/ds-bot/files/src/host/tasks.js +++ b/patches/ds-bot/files/src/host/tasks.js @@ -17,6 +17,8 @@ import { randomBytes } from 'node:crypto' import { currentParticipants } from './room-membership.js' const ACTIVE = new Set(['open', 'claimed', 'working', 'blocked']) +// result-ready still waits for the requester: prune must not evict it with terminals (#111). +const PROTECTED = new Set([...ACTIVE, 'result-ready']) const TRANSITIONS = { open: ['claimed', 'cancelled', 'failed'], claimed: ['working', 'cancelled', 'failed'], @@ -79,17 +81,25 @@ export function install(rt) { const view = task => ({ ...task, actors: [...task.actors ?? []] }) const prune = () => { const all = Object.values(store()) - const terminal = all.filter(task => !ACTIVE.has(task.status)).sort((a, b) => a.updatedAt - b.updatedAt) - for (const task of terminal.slice(0, Math.max(0, all.length - KEPT))) delete store()[task.id] + const droppable = all.filter(task => !PROTECTED.has(task.status)).sort((a, b) => a.updatedAt - b.updatedAt) + for (const task of droppable.slice(0, Math.max(0, all.length - KEPT))) delete store()[task.id] } const save = () => rt.save() // ---------------------------------------------------------------- host hooks // A consequential send from the responsible Bot was held (#52 card): the task - // blocks on that approval until the owner decides. + // blocks on that approval until the owner decides. Link one task, not every + // working task of that Bot: `meta.taskId` names it, else the Bot's only working + // task (several without a name stay working — they are not this send's) (#111). rt.taskHeld = (msg, approval) => { - for (const task of Object.values(store())) { - if (task.status !== 'working' || task.ownerBotId !== msg.from) continue + const namedId = typeof msg?.meta?.taskId === 'string' ? msg.meta.taskId : undefined + const named = namedId ? store()[namedId] : undefined + const ofSender = task => task.ownerBotId === msg.from || task.workBotId === msg.from + const working = Object.values(store()).filter(task => task.status === 'working' && ofSender(task)) + const targets = named && named.status === 'working' && ofSender(named) + ? [named] + : working.length === 1 ? working : [] + for (const task of targets) { task.approvalId = approval.id transition(task, 'blocked', null, { approvalId: approval.id }) } @@ -135,6 +145,7 @@ export function install(rt) { updatedAt: Date.now(), requesterId, ownerBotId: botId, + // workBotId is the specialist on the exchange; assigned with #55 wiring (#111). title: String(body.title ?? '').slice(0, 300), ...(typeof body.criteria === 'string' && body.criteria.trim() !== '' ? { criteria: body.criteria.slice(0, 2000) } : {}), origin: { ...(roomId ? { roomId } : {}), ...(body.sessionId ? { sessionId: String(body.sessionId) } : {}) }, diff --git a/patches/ds-bot/files/test/approval-binding.test.js b/patches/ds-bot/files/test/approval-binding.test.js index 61954e042..a13eb02f5 100644 --- a/patches/ds-bot/files/test/approval-binding.test.js +++ b/patches/ds-bot/files/test/approval-binding.test.js @@ -251,6 +251,47 @@ test('replacing a Chief invalidates the held sends written under it', async (t) assert.equal(approval.status, 'stale', JSON.stringify(approval)) }) +test('replacing a Chief leaves holds addressed to it pending and decidable', async (t) => { + const { fb, call, stop, received } = await start() + t.after(stop) + const alice = await signIn(call, 'Alice') + holdFlagged(fb) + // Sender-side only: a hold *to* the demoted Chief is still the sender's owner's to spend. + const id = await heldId(fb, { from: 'b', to: 'a', text: 'ask the chief', meta: { hold: true } }) + const swapped = await call('set-main', { id: 'b', main: true, replace: 'a' }, alice.sessionToken) + assert.equal(swapped.ok, true, JSON.stringify(swapped)) + const list = await call('flybot/approvals', { status: ['pending', 'failed', 'stale'] }, alice.sessionToken) + assert.equal(list.ok, true) + const approval = list.value.find(entry => entry.id === id) + assert.equal(approval.status, 'pending', JSON.stringify(approval)) + const decided = await call('flybot/decide', { id, decision: 'approve' }, alice.sessionToken) + assert.equal(decided.ok, true, JSON.stringify(decided)) + assert.equal(decided.value.status, 'approved', JSON.stringify(decided.value)) + assert.deepEqual(received('a'), ['[Message from Writer]\nask the chief']) +}) + +test('an in-flight approve is invalidated if the Chief is replaced before dispatch', async (t) => { + const { fb, call, stop, received } = await start() + t.after(stop) + const alice = await signIn(call, 'Alice') + holdFlagged(fb) + const id = await heldId(fb, { from: 'a', to: 'c', text: 'still in flight', meta: { hold: true } }) + // After the hold middleware, release re-evaluates the rest of the chain. Swap the + // Chief there so invalidateFor races the already-approved record (#97 TOCTOU). + fb.bot.send.use({ + name: 'swap-chief', + handle: async () => { + const swapped = await call('set-main', { id: 'b', main: true, replace: 'a' }, alice.sessionToken) + assert.equal(swapped.ok, true, JSON.stringify(swapped)) + return undefined + }, + }) + const decided = await call('flybot/decide', { id, decision: 'approve' }, alice.sessionToken) + assert.equal(decided.ok, true, JSON.stringify(decided)) + assert.equal(decided.value.status, 'stale', JSON.stringify(decided.value)) + assert.equal(received('c').length, 0) +}) + test('a member whose sender Bot is gone cannot decide leftovers with an old session', async (t) => { const { fb, call, stop, statePath } = await start() t.after(stop) diff --git a/patches/ds-bot/files/test/authz.test.js b/patches/ds-bot/files/test/authz.test.js index 731f9828f..a745262a6 100644 --- a/patches/ds-bot/files/test/authz.test.js +++ b/patches/ds-bot/files/test/authz.test.js @@ -195,6 +195,35 @@ test('a member reaches their own Bots sessions, groups and schedules', async (t) // A group Session follows its room's rule; so does a schedule on it. assert.equal(rt.authorize(actor, 'send', { sessionId: 'gs-a1' }).ok, true) assert.equal(rt.authorize(actor, 'schedule-create', { sessionId: 'gs-a1' }).ok, true) + // The create endpoint keys off `target` (Bot or room id), not sessionId (#106). + assert.equal(rt.authorize(actor, 'schedule-create', { target: 'a1' }).ok, true) + assert.equal(rt.authorize(actor, 'schedule-create', { target: 'ra' }).ok, true) +}) + +test('schedule-create cannot hide a foreign target behind an owned sessionId', async (t) => { + const { rt } = runtime(t, TEAM) + await rt.load() + const sessionA = await redeemOwner(rt) + await redeemOwner(rt) + rt.state.bots.a1 = { id: 'a1', sessionId: 'chat-a1', name: 'A', instructions: '', createdAt: 2, ownerId: 'owner-1' } + rt.state.bots.b1 = { id: 'b1', sessionId: 'chat-b1', name: 'B', instructions: '', createdAt: 3, ownerId: 'owner-2' } + const actorA = await rt.authenticate(sessionA.sessionToken) + assert.equal(rt.authorize(actorA, 'schedule-create', { target: 'b1' }).ok, false) + assert.equal(rt.authorize(actorA, 'schedule-create', { target: 'b1', sessionId: 'chat-a1' }).code, 'bot/forbidden') + assert.equal(rt.authorize(actorA, 'schedule-create', { target: 'a1' }).ok, true) +}) + +test('task endpoints are an explicit allow-list, not a task-* prefix', async (t) => { + const { rt } = runtime(t, TEAM) + await rt.load() + const sessionA = await redeemOwner(rt) + const actorA = await rt.authenticate(sessionA.sessionToken) + for (const ep of ['tasks', 'task-create', 'task-accept', 'task-start', 'task-return', 'task-accept-result', 'task-cancel']) { + assert.equal(rt.authorize(actorA, ep, {}).ok, true, ep) + } + const unknown = rt.authorize(actorA, 'task-explode', {}) + assert.equal(unknown.ok, false, JSON.stringify(unknown)) + assert.equal(unknown.code, 'bot/not-found') }) test('approval-answer resolves the approval, not caller-chosen fields', async (t) => { diff --git a/patches/ds-bot/files/test/context-sharing.test.js b/patches/ds-bot/files/test/context-sharing.test.js index fbc08ca44..38654c730 100644 --- a/patches/ds-bot/files/test/context-sharing.test.js +++ b/patches/ds-bot/files/test/context-sharing.test.js @@ -230,6 +230,83 @@ test('a chat reference chip must name a chat the author may read', async (t) => void bob }) +test('answer-question and schedule-create chips must name a chat the author may read', async (t) => { + const state = team() + state.questions.a = { id: 'q1', question: 'What next?', options: [{ label: 'Go' }], callId: 'c1' } + const { call, signIn } = await start(t, { state }) + const alice = await signIn('Alice') + const bob = await signIn('Bob', alice.sessionToken) + const foreignAnswer = await call('answer-question', { + sessionId: 'a', questionId: 'q1', + answer: { custom: 'look at @[that](dsh-session:d) please' }, + }, alice.sessionToken) + assert.equal(foreignAnswer.ok, false, JSON.stringify(foreignAnswer)) + assert.equal(foreignAnswer.error.code, 'bot/forbidden') + const ownAnswer = await call('answer-question', { + sessionId: 'a', questionId: 'q1', + answer: { custom: 'look at @[this](dsh-session:a) please' }, + }, alice.sessionToken) + assert.equal(ownAnswer.ok, true, JSON.stringify(ownAnswer)) + + const foreignSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: 'look at @[that](dsh-session:d)', + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + assert.equal(foreignSchedule.ok, false, JSON.stringify(foreignSchedule)) + assert.equal(foreignSchedule.error.code, 'bot/forbidden') + const ownSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: 'look at @[this](dsh-session:a)', + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + // Own chip is allowed through the author gate; automation may still be off in this host. + if (ownSchedule.ok === false) assert.notEqual(ownSchedule.error.code, 'bot/forbidden', JSON.stringify(ownSchedule)) + void bob +}) + +test('answer-question and schedule-create accept menu-encoded session chips the author owns', async (t) => { + // Menu chips encode JSON.stringify(sessionId) as base64url (`a` → ImEi). + const ownEncoded = 'ImEi' + const foreignEncoded = Buffer.from(JSON.stringify('d')).toString('base64url') + const state = team() + state.questions.a = { id: 'q1', question: 'What next?', options: [{ label: 'Go' }], callId: 'c1' } + const { call, signIn } = await start(t, { state }) + const alice = await signIn('Alice') + const ownAnswer = await call('answer-question', { + sessionId: 'a', questionId: 'q1', + answer: { custom: `look at @[this](dsh-session:${ownEncoded}) please` }, + }, alice.sessionToken) + assert.equal(ownAnswer.ok, true, JSON.stringify(ownAnswer)) + const foreignSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: `look at @[that](dsh-session:${foreignEncoded})`, + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + assert.equal(foreignSchedule.ok, false, JSON.stringify(foreignSchedule)) + assert.equal(foreignSchedule.error.code, 'bot/forbidden') + const ownSchedule = await call('schedule-create', { + target: 'a', title: 'Ping', prompt: `look at @[this](dsh-session:${ownEncoded})`, + when: { kind: 'every', minutes: 30 }, + }, alice.sessionToken) + if (ownSchedule.ok === false) assert.notEqual(ownSchedule.error.code, 'bot/forbidden', JSON.stringify(ownSchedule)) +}) + +test('a Bot post into a group drops chat-reference chips so they cannot tunnel a private chat', async (t) => { + const { tool, agentOf, signIn } = await start(t) + const alice = await signIn('Alice') + await signIn('Bob', alice.sessionToken) + const posted = await tool('post_to_group', { group: 'War', message: 'see @[secret](dsh-session:a) and dsh-session:d' }, 'a') + assert.match(String(posted), /Posted/, String(posted)) + const text = agentOf('war').calls.map(message => (message.content ?? []).map(block => block.text ?? '').join('\n')).join('\n') + assert.ok(text.includes('see @secret'), 'the rest of the post still lands') + assert.ok(!text.includes('dsh-session:'), 'chips do not enter the group Session as user text') + // Nested marker must not reconstitute a valid chip after one strip pass. + const nested = await tool('post_to_group', { group: 'War', message: 'nested @[secret](dsh-sessiondsh-session:x:ImQi) end' }, 'a') + assert.match(String(nested), /Posted/, String(nested)) + const again = agentOf('war').calls.map(message => (message.content ?? []).map(block => block.text ?? '').join('\n')).join('\n') + assert.ok(!again.includes('dsh-session:'), 'nested chips do not reconstitute') + assert.ok(!/@\[secret\]\(dsh-session:/.test(again), 'no valid reconstituted chip') + void alice +}) + test('a granted remote Bot in the room counts as another owner for the brief', async (t) => { diff --git a/patches/ds-bot/files/test/tasks.test.js b/patches/ds-bot/files/test/tasks.test.js index 650bc9e5d..2471efe24 100644 --- a/patches/ds-bot/files/test/tasks.test.js +++ b/patches/ds-bot/files/test/tasks.test.js @@ -259,6 +259,70 @@ test('rejecting the blocked approval cancels the task', async (t) => { assert.equal(resume.error.code, 'bot/stale') }) +test('a held send blocks only the named working task, not every working task of that Bot', async (t) => { + const { fb, call, signIn, createTask } = await start(t) + const alice = await signIn('Alice') + const first = await createTask(alice.sessionToken, { botId: 'a', title: 'Note one' }) + const second = await createTask(alice.sessionToken, { botId: 'a', title: 'Note two' }) + for (const id of [first.value.id, second.value.id]) { + await call('task-accept', { id }, alice.sessionToken) + await call('task-start', { id }, alice.sessionToken) + } + fb.bot.send.use({ name: 'gate', handle: async msg => (msg.meta?.hold ? { action: 'hold', reason: 'held by test' } : undefined) }) + const held = await fb.bot.send({ from: 'a', to: 's', text: 'send this out', meta: { hold: true, taskId: first.value.id } }) + assert.equal(held.action, 'hold', JSON.stringify(held)) + assert.equal((await statusOf(call, alice.sessionToken, first.value.id)).status, 'blocked') + assert.equal((await statusOf(call, alice.sessionToken, second.value.id)).status, 'working') + const rejected = await call('flybot/decide', { id: held.approvalId, decision: 'reject' }, alice.sessionToken) + assert.equal(rejected.ok, true, JSON.stringify(rejected)) + assert.equal((await statusOf(call, alice.sessionToken, first.value.id)).status, 'cancelled') + assert.equal((await statusOf(call, alice.sessionToken, second.value.id)).status, 'working') +}) + +test('an untagged hold does not block every working task when the Bot has several', async (t) => { + const { fb, call, signIn, createTask } = await start(t) + const alice = await signIn('Alice') + const first = await createTask(alice.sessionToken, { botId: 'a', title: 'Note one' }) + const second = await createTask(alice.sessionToken, { botId: 'a', title: 'Note two' }) + for (const id of [first.value.id, second.value.id]) { + await call('task-accept', { id }, alice.sessionToken) + await call('task-start', { id }, alice.sessionToken) + } + fb.bot.send.use({ name: 'gate', handle: async msg => (msg.meta?.hold ? { action: 'hold', reason: 'held by test' } : undefined) }) + const held = await fb.bot.send({ from: 'a', to: 's', text: 'untagged send', meta: { hold: true } }) + assert.equal(held.action, 'hold', JSON.stringify(held)) + assert.equal((await statusOf(call, alice.sessionToken, first.value.id)).status, 'working') + assert.equal((await statusOf(call, alice.sessionToken, second.value.id)).status, 'working') +}) + +test('a result-ready task is not pruned while it waits for acceptance', async (t) => { + const { call, signIn, createTask, stop, home } = await start(t) + const alice = await signIn('Alice') + const created = await createTask(alice.sessionToken) + const task = created.value + await call('task-accept', { id: task.id }, alice.sessionToken) + await call('task-start', { id: task.id }, alice.sessionToken) + await call('task-return', { id: task.id, summary: 'waiting' }, alice.sessionToken) + await stop({ keepHome: true }) + const saved = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')) + for (let i = 0; i < 200; i++) { + const id = `task-old${i.toString(16).padStart(8, '0')}deadbeef` + saved.tasks[id] = { + id, status: 'cancelled', createdAt: 1, updatedAt: i + 2, + requesterId: alice.memberId, ownerBotId: 'a', title: 'old', origin: {}, actors: [], + } + } + saved.tasks[task.id].updatedAt = 1 + writeFileSync(join(home, 'state.json'), JSON.stringify(saved)) + const again = await start(t, { home }) + t.after(() => again.stop()) + const extra = await again.createTask(alice.sessionToken, { title: 'new' }) + assert.equal(extra.ok, true, JSON.stringify(extra)) + const waiting = await statusOf(again.call, alice.sessionToken, task.id) + assert.ok(waiting, 'the result-ready task is still listed') + assert.equal(waiting.status, 'result-ready', JSON.stringify(waiting)) +}) + test('a grantee may create a task on a granted child, not on an ungranted sibling', async (t) => { const { fb, signIn, createTask } = await start(t) const alice = await signIn('Alice')