diff --git a/changelog.d/1555.fixed.md b/changelog.d/1555.fixed.md new file mode 100644 index 000000000..1405898bc --- /dev/null +++ b/changelog.d/1555.fixed.md @@ -0,0 +1 @@ +A run Bot started through a friend's Bot no longer reaches a connector: the two Workflow tools have no `visibleTo`, so a remote caller could start a run whose node `tools` alone decided what it reached, and one that started a run is now read as "every connector switched off" — the guard refuses with `… not available to a run started by a friend's Bot`, and the tool mask takes the connectors out of the run Bot's list. An in-team caller whose own switch for a connector is on, and whose node names it, is unchanged, and a run with no caller (the owner, the board) keeps the answer it had. ([#1555](https://github.com/ThinkFlowLab/FlyBot/issues/1555)) diff --git a/core/src/host/connector-access.js b/core/src/host/connector-access.js index 53b6e0b45..d5242f71e 100644 --- a/core/src/host/connector-access.js +++ b/core/src/host/connector-access.js @@ -9,7 +9,8 @@ // - The owner can turn a connector off for one Bot: `bot.connectors = { composio: false }`. Absent // means on, so every Bot, old or new, has the connected connector. // - A friend's Bot (`remote`) never gets them. A run Bot (`ephemeral`) gets a connector -// only when its `stepTools` name it (`github` covers `mcp__github__*`). A Session whose +// only when its `stepTools` name it (`github` covers `mcp__github__*`), and never when a +// friend's Bot is the one that started the run (#1555). A Session whose // Bot or parent chain cannot be resolved fails closed. Sessions that belong to no Bot // (the DSH Agent, plain DSH sessions) are not touched: they are the owner's own, and no // friend can reach them. @@ -53,7 +54,17 @@ export function install(rt) { const tools = bot?.stepTools ?? [] return tools.includes(id) || tools.includes('*') || tools.some(name => name === id || String(name).startsWith(`mcp__${id}__`)) } - const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) && (bot.ephemeral === undefined || stepGrants(bot, id)) + // #1555: `run_workflow`/`define_workflow` have no `visibleTo`, so a friend's Bot can call them and a run + // Bot can name it as the one that started the run (`createdBy`). A remote Bot has no switch table of ours + // to intersect with, so it reads as "every connector switched off": the node's `tools` cannot hand the run + // Bot a connector. A record with no `createdBy` (the owner, the board, an older run) keeps the answer it + // had, and a `createdBy` that names a Bot we do not have leaves it as it was too. + const remoteCaller = (bot) => { + const id = bot?.createdBy + return (typeof id === 'string' && id !== '' ? rt.botOf(id) : undefined)?.remote !== undefined + } + const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) + && (bot.ephemeral === undefined || (stepGrants(bot, id) && !remoteCaller(bot))) // The Bot an Agent works for: its own chat, a group Session and an earlier part resolve through // selfOf; a sub-agent Session is followed up to its parent. The answer has three shapes, and only @@ -126,7 +137,11 @@ export function install(rt) { const { bot } = who if (allowed(bot, id)) return undefined if (bot.remote !== undefined) return `The ${id} connector is not available to a friend's Bot.` - if (bot.ephemeral !== undefined) return `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + if (bot.ephemeral !== undefined) { + return remoteCaller(bot) + ? `The ${id} connector is not available to a run started by a friend's Bot.` + : `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + } return `The ${id} connector is turned off for ${bot.name}. Its owner can turn it on in this Bot's settings.` }) diff --git a/core/test/connector-access.test.js b/core/test/connector-access.test.js index 939e533b6..d0c731274 100644 --- a/core/test/connector-access.test.js +++ b/core/test/connector-access.test.js @@ -572,3 +572,32 @@ test('a run Bot gets a connector only when its step tools name it', async (t) => assert.deepEqual(sorted(hiddenFrom('bare')), sorted([...COMPOSIO, ...GITHUB]), 'no stepTools fails closed') assert.match(guard(GITHUB[0], 'bare'), /not granted to this run/) }) + +// #1555: `run_workflow` / `define_workflow` have no `visibleTo`, so a friend's Bot can call them. A run +// Bot then names that remote Bot as the one that started it, and a remote Bot has no switch table of ours +// to intersect with — so the caller reads as "every connector switched off", and the step list cannot hand +// the run Bot anything. Nothing changes for a team Bot: its own switch and the node's `tools` still decide. +test('a run Bot started by a friend\'s Bot reaches no connector', async (t) => { + const state = team() + state.bots.friend = { id: 'friend', sessionId: 'friend', name: 'Bob', role: 'Bot', instructions: '', createdAt: 5, remote: { handle: '@bob.home' } } + const run = (id, { createdBy, stepTools = ['github'] } = {}) => { + state.bots[id] = { + id, sessionId: id, name: id, role: 'Run', instructions: '', createdAt: 6, + hidden: true, ephemeral: { plugin: 'flybot-workflow' }, stepTools, + ...(createdBy === undefined ? {} : { createdBy }), + } + } + run('remote-run', { createdBy: 'friend' }) + run('team-run', { createdBy: 'chief' }) + const { agentFor, hiddenFrom, guard, stop } = await setup({ state }) + t.after(stop) + await Promise.all(['remote-run', 'team-run'].map(id => agentFor(id))) + + // Red before this change: the node names github, so the step list alone let the run Bot through. + assert.match(guard(GITHUB[0], 'remote-run'), /started by a friend's Bot/, 'the caller is the reason, not the step list') + assert.deepEqual(sorted(hiddenFrom('remote-run')), sorted([...COMPOSIO, ...GITHUB]), 'and both connectors are out of its list too') + assert.match(guard(COMPOSIO[0], 'remote-run'), /started by a friend's Bot/) + // The no-over-refusal case: a team Bot started this run, its switch is on, the node names github. + assert.equal(guard(GITHUB[0], 'team-run'), undefined, 'a team caller keeps what the node names') + assert.deepEqual(sorted(hiddenFrom('team-run')), sorted(COMPOSIO), 'only the connector the node did not name stays out') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 57308f714..01f065cb3 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -436,6 +436,7 @@ The connector tools are mounted once for the whole DSH profile, so which Bot may **Decision (the owner, 2026-10-22, #601: 「放开,主要应该用连接器工具」; it supersedes #369's "off by default for other people's use"; one exception, #1237, is the group room's composio and schedule tools, below).** Every turn of every one of the owner's own Bots may use the connected connectors, whatever started it: the owner, a group round of any origin (also a room with a friend or another person in it), a Bot-to-Bot relay, a scheduled turn, a friend talking to a shared Bot, a turn with no recorded origin. The connector tools no longer read `turn-origin.js`. - **Per-Bot record** (only what differs from the default is stored): `bot.connectors = { github?: false, composio?: false }` is an explicit off for that Bot (absent = on; a legacy `true` from #346 reads as on). `bot-connectors { botId, connector, enabled }` writes it; a `scope` an older client still sends is ignored. The retired "also when others use it" offer (`bot.connectorsShared`) means nothing any more and is dropped on the Bot's next switch or on a disconnect. +- **A run Bot started by a friend's Bot reaches no connector (#1555).** `run_workflow` and `define_workflow` have no `visibleTo`, so a friend's Bot can call them; the run Bot's `createdBy` then names that remote Bot, which has no switch table of ours to intersect with. The ephemeral branch reads that as "every connector switched off": `allowed()` requires the step tools to name the connector **and** a caller that is not remote, so the guard refuses with `The github connector is not available to a run started by a friend's Bot.` and the same `denied()` takes the connectors out of the run Bot's tool list. A run with no `createdBy` (the owner, the board, an older run) and a `createdBy` that names a Bot the team no longer has keep the answer they had, and an in-team caller whose own switch is on, whose node names the connector, still gets it. `visibleTo` on the two tools stays backlog. - **Boundaries that stay.** A friend's Bot (`bot.remote`) never gets the tools. An Agent whose Bot cannot be determined (a parent Session that is gone, or a parent chain deeper than eight) gets none either (fail closed), so an unresolved sub-agent never reads as a plain owner Session. A chain that really ends at a Session of no Bot (the DSH Agent, a plain Session) keeps them. A sub-agent Session is followed to its parent Bot (`whoseAgent`, also on `rt.whoseAgent` for `secrets.js`). - **Enforcement is two layers.** A `ctx.tools.guard` refuses every `mcp__github__*` / `mcp__composio__*` call that is not allowed, and `agent.ctx.tools.restrict({ deny })` takes the same tools out of that Agent's tool list. The mask is refreshed when the Agent is created, on every `resolveAgent` (delivery), when a switch changes, and on `tools/change` (a connector that mounts or goes away later). Turns and sharing no longer change it. DSH's hidden "Tool added/removed" notices for these tools are not drawn in the chat (`cells.js`). - **A mask that cannot be rebuilt keeps the previous one (#1271).** `hideConnectorTools` lifts the old mask before it reads `schemas()` again, so a `restrict()` that refuses a name would otherwise leave the Agent with no mask and every tool listed again. The rebuild now names only what the global view (`schemas()` with no Agent) can restrict, and if a restriction still fails the catch puts the previous deny list back: the guard refuses a call either way, and a list that widens by itself is a wrong answer. A denied name the Agent itself serves stays listed — `restrict()` cannot name a scope's own tool — and the warn line says so. diff --git a/patches/ds-bot/files/src/host/connector-access.js b/patches/ds-bot/files/src/host/connector-access.js index 53b6e0b45..d5242f71e 100644 --- a/patches/ds-bot/files/src/host/connector-access.js +++ b/patches/ds-bot/files/src/host/connector-access.js @@ -9,7 +9,8 @@ // - The owner can turn a connector off for one Bot: `bot.connectors = { composio: false }`. Absent // means on, so every Bot, old or new, has the connected connector. // - A friend's Bot (`remote`) never gets them. A run Bot (`ephemeral`) gets a connector -// only when its `stepTools` name it (`github` covers `mcp__github__*`). A Session whose +// only when its `stepTools` name it (`github` covers `mcp__github__*`), and never when a +// friend's Bot is the one that started the run (#1555). A Session whose // Bot or parent chain cannot be resolved fails closed. Sessions that belong to no Bot // (the DSH Agent, plain DSH sessions) are not touched: they are the owner's own, and no // friend can reach them. @@ -53,7 +54,17 @@ export function install(rt) { const tools = bot?.stepTools ?? [] return tools.includes(id) || tools.includes('*') || tools.some(name => name === id || String(name).startsWith(`mcp__${id}__`)) } - const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) && (bot.ephemeral === undefined || stepGrants(bot, id)) + // #1555: `run_workflow`/`define_workflow` have no `visibleTo`, so a friend's Bot can call them and a run + // Bot can name it as the one that started the run (`createdBy`). A remote Bot has no switch table of ours + // to intersect with, so it reads as "every connector switched off": the node's `tools` cannot hand the run + // Bot a connector. A record with no `createdBy` (the owner, the board, an older run) keeps the answer it + // had, and a `createdBy` that names a Bot we do not have leaves it as it was too. + const remoteCaller = (bot) => { + const id = bot?.createdBy + return (typeof id === 'string' && id !== '' ? rt.botOf(id) : undefined)?.remote !== undefined + } + const allowed = (bot, id) => bot.remote === undefined && !turnedOff(bot, id) + && (bot.ephemeral === undefined || (stepGrants(bot, id) && !remoteCaller(bot))) // The Bot an Agent works for: its own chat, a group Session and an earlier part resolve through // selfOf; a sub-agent Session is followed up to its parent. The answer has three shapes, and only @@ -126,7 +137,11 @@ export function install(rt) { const { bot } = who if (allowed(bot, id)) return undefined if (bot.remote !== undefined) return `The ${id} connector is not available to a friend's Bot.` - if (bot.ephemeral !== undefined) return `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + if (bot.ephemeral !== undefined) { + return remoteCaller(bot) + ? `The ${id} connector is not available to a run started by a friend's Bot.` + : `The ${id} connector is not granted to this run. The step's tools list is what allows it.` + } return `The ${id} connector is turned off for ${bot.name}. Its owner can turn it on in this Bot's settings.` }) diff --git a/patches/ds-bot/files/test/connector-access.test.js b/patches/ds-bot/files/test/connector-access.test.js index 939e533b6..d0c731274 100644 --- a/patches/ds-bot/files/test/connector-access.test.js +++ b/patches/ds-bot/files/test/connector-access.test.js @@ -572,3 +572,32 @@ test('a run Bot gets a connector only when its step tools name it', async (t) => assert.deepEqual(sorted(hiddenFrom('bare')), sorted([...COMPOSIO, ...GITHUB]), 'no stepTools fails closed') assert.match(guard(GITHUB[0], 'bare'), /not granted to this run/) }) + +// #1555: `run_workflow` / `define_workflow` have no `visibleTo`, so a friend's Bot can call them. A run +// Bot then names that remote Bot as the one that started it, and a remote Bot has no switch table of ours +// to intersect with — so the caller reads as "every connector switched off", and the step list cannot hand +// the run Bot anything. Nothing changes for a team Bot: its own switch and the node's `tools` still decide. +test('a run Bot started by a friend\'s Bot reaches no connector', async (t) => { + const state = team() + state.bots.friend = { id: 'friend', sessionId: 'friend', name: 'Bob', role: 'Bot', instructions: '', createdAt: 5, remote: { handle: '@bob.home' } } + const run = (id, { createdBy, stepTools = ['github'] } = {}) => { + state.bots[id] = { + id, sessionId: id, name: id, role: 'Run', instructions: '', createdAt: 6, + hidden: true, ephemeral: { plugin: 'flybot-workflow' }, stepTools, + ...(createdBy === undefined ? {} : { createdBy }), + } + } + run('remote-run', { createdBy: 'friend' }) + run('team-run', { createdBy: 'chief' }) + const { agentFor, hiddenFrom, guard, stop } = await setup({ state }) + t.after(stop) + await Promise.all(['remote-run', 'team-run'].map(id => agentFor(id))) + + // Red before this change: the node names github, so the step list alone let the run Bot through. + assert.match(guard(GITHUB[0], 'remote-run'), /started by a friend's Bot/, 'the caller is the reason, not the step list') + assert.deepEqual(sorted(hiddenFrom('remote-run')), sorted([...COMPOSIO, ...GITHUB]), 'and both connectors are out of its list too') + assert.match(guard(COMPOSIO[0], 'remote-run'), /started by a friend's Bot/) + // The no-over-refusal case: a team Bot started this run, its switch is on, the node names github. + assert.equal(guard(GITHUB[0], 'team-run'), undefined, 'a team caller keeps what the node names') + assert.deepEqual(sorted(hiddenFrom('team-run')), sorted(COMPOSIO), 'only the connector the node did not name stays out') +})