From 5b97feaaa5a9547d382feca084971a9ea8110712 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 7 Jan 2026 23:35:04 +0100 Subject: [PATCH] fix: scope process sessions per agent --- CHANGELOG.md | 1 + docs/gateway/background-process.md | 1 + docs/tools/bash.md | 1 + docs/tools/index.md | 1 + src/agents/bash-process-registry.ts | 3 + src/agents/bash-tools.test.ts | 32 +++++ src/agents/bash-tools.ts | 177 ++++++++++++++++------------ src/agents/pi-tools.ts | 37 ++++-- 8 files changed, 166 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5387c0a20..fe76316a2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ - Sandbox: add `agent.sandbox.workspaceAccess` (`none`/`ro`/`rw`) to control agent workspace visibility inside the container; `ro` hard-disables `write`/`edit`. - Routing: allow per-agent sandbox overrides (including `workspaceAccess` and `sandbox.tools`) plus per-agent tool policies in multi-agent configs. Thanks @pasogott for PR #380. - Tools: make per-agent tool policies override global defaults and run bash synchronously when `process` is disallowed. +- Tools: scope `process` sessions per agent to prevent cross-agent visibility. - Cron: clamp timer delay to avoid TimeoutOverflowWarning. Thanks @emanuelst for PR #412. - Web UI: allow reconnect + password URL auth for the control UI and always scrub auth params from the URL. Thanks @oswalpalash for PR #414. - ClawdbotKit: fix SwiftPM resource bundling path for `tool-display.json`. Thanks @fcatuhe for PR #398. diff --git a/docs/gateway/background-process.md b/docs/gateway/background-process.md index 8658de949..3f97c844b 100644 --- a/docs/gateway/background-process.md +++ b/docs/gateway/background-process.md @@ -51,6 +51,7 @@ Notes: - Only backgrounded sessions are listed/persisted in memory. - Sessions are lost on process restart (no disk persistence). - Session logs are only saved to chat history if you run `process poll/log` and the tool result is recorded. +- `process` is scoped per agent; it only sees sessions started by that agent. - `process list` includes a derived `name` (command verb + target) for quick scans. - `process log` uses line-based `offset`/`limit` (omit `offset` to grab the last N lines). diff --git a/docs/tools/bash.md b/docs/tools/bash.md index 57095a6c2..73106a1e5 100644 --- a/docs/tools/bash.md +++ b/docs/tools/bash.md @@ -9,6 +9,7 @@ read_when: Run shell commands in the workspace. Supports foreground + background execution via `process`. If `process` is disallowed, `bash` runs synchronously and ignores `yieldMs`/`background`. +Background sessions are scoped per agent; `process` only sees sessions from the same agent. ## Parameters diff --git a/docs/tools/index.md b/docs/tools/index.md index 7965357e7..6e9d14daa 100644 --- a/docs/tools/index.md +++ b/docs/tools/index.md @@ -53,6 +53,7 @@ Core actions: Notes: - `poll` returns new output and exit status when complete. - `log` supports line-based `offset`/`limit` (omit `offset` to grab the last N lines). +- `process` is scoped per agent; sessions from other agents are not visible. ### `browser` Control the dedicated clawd browser. diff --git a/src/agents/bash-process-registry.ts b/src/agents/bash-process-registry.ts index d91081b6b..71c911376 100644 --- a/src/agents/bash-process-registry.ts +++ b/src/agents/bash-process-registry.ts @@ -18,6 +18,7 @@ export type ProcessStatus = "running" | "completed" | "failed" | "killed"; export interface ProcessSession { id: string; command: string; + scopeKey?: string; child?: ChildProcessWithoutNullStreams; pid?: number; startedAt: number; @@ -38,6 +39,7 @@ export interface ProcessSession { export interface FinishedSession { id: string; command: string; + scopeKey?: string; startedAt: number; endedAt: number; cwd?: string; @@ -126,6 +128,7 @@ function moveToFinished(session: ProcessSession, status: ProcessStatus) { finishedSessions.set(session.id, { id: session.id, command: session.command, + scopeKey: session.scopeKey, startedAt: session.startedAt, endedAt: Date.now(), cwd: session.cwd, diff --git a/src/agents/bash-tools.test.ts b/src/agents/bash-tools.test.ts index 7276803bb..9214ab2c7 100644 --- a/src/agents/bash-tools.test.ts +++ b/src/agents/bash-tools.test.ts @@ -185,4 +185,36 @@ describe("bash tool backgrounding", () => { const textBlock = log.content.find((c) => c.type === "text"); expect(textBlock?.text).toBe("beta"); }); + + it("scopes process sessions by scopeKey", async () => { + const bashA = createBashTool({ backgroundMs: 10, scopeKey: "agent:alpha" }); + const processA = createProcessTool({ scopeKey: "agent:alpha" }); + const bashB = createBashTool({ backgroundMs: 10, scopeKey: "agent:beta" }); + const processB = createProcessTool({ scopeKey: "agent:beta" }); + + const resultA = await bashA.execute("call1", { + command: 'node -e "setTimeout(() => {}, 50)"', + background: true, + }); + const resultB = await bashB.execute("call2", { + command: 'node -e "setTimeout(() => {}, 50)"', + background: true, + }); + + const sessionA = (resultA.details as { sessionId: string }).sessionId; + const sessionB = (resultB.details as { sessionId: string }).sessionId; + + const listA = await processA.execute("call3", { action: "list" }); + const sessionsA = ( + listA.details as { sessions: Array<{ sessionId: string }> } + ).sessions; + expect(sessionsA.some((s) => s.sessionId === sessionA)).toBe(true); + expect(sessionsA.some((s) => s.sessionId === sessionB)).toBe(false); + + const pollB = await processB.execute("call4", { + action: "poll", + sessionId: sessionA, + }); + expect(pollB.details.status).toBe("failed"); + }); }); diff --git a/src/agents/bash-tools.ts b/src/agents/bash-tools.ts index 6b146c7e5..bb4aff4c5 100644 --- a/src/agents/bash-tools.ts +++ b/src/agents/bash-tools.ts @@ -59,10 +59,12 @@ export type BashToolDefaults = { sandbox?: BashSandboxConfig; elevated?: BashElevatedDefaults; allowBackground?: boolean; + scopeKey?: string; }; export type ProcessToolDefaults = { cleanupMs?: number; + scopeKey?: string; }; export type BashSandboxConfig = { @@ -251,6 +253,7 @@ export function createBashTool( const session = { id: sessionId, command: params.command, + scopeKey: defaults?.scopeKey, child, pid: child?.pid, startedAt, @@ -471,6 +474,9 @@ export function createProcessTool( if (defaults?.cleanupMs !== undefined) { setJobTtlMs(defaults.cleanupMs); } + const scopeKey = defaults?.scopeKey; + const isInScope = (session?: { scopeKey?: string } | null) => + !scopeKey || session?.scopeKey === scopeKey; return { name: "process", @@ -488,32 +494,36 @@ export function createProcessTool( }; if (params.action === "list") { - const running = listRunningSessions().map((s) => ({ - sessionId: s.id, - status: "running", - pid: s.pid ?? undefined, - startedAt: s.startedAt, - runtimeMs: Date.now() - s.startedAt, - cwd: s.cwd, - command: s.command, - name: deriveSessionName(s.command), - tail: s.tail, - truncated: s.truncated, - })); - const finished = listFinishedSessions().map((s) => ({ - sessionId: s.id, - status: s.status, - startedAt: s.startedAt, - endedAt: s.endedAt, - runtimeMs: s.endedAt - s.startedAt, - cwd: s.cwd, - command: s.command, - name: deriveSessionName(s.command), - tail: s.tail, - truncated: s.truncated, - exitCode: s.exitCode ?? undefined, - exitSignal: s.exitSignal ?? undefined, - })); + const running = listRunningSessions() + .filter((s) => isInScope(s)) + .map((s) => ({ + sessionId: s.id, + status: "running", + pid: s.pid ?? undefined, + startedAt: s.startedAt, + runtimeMs: Date.now() - s.startedAt, + cwd: s.cwd, + command: s.command, + name: deriveSessionName(s.command), + tail: s.tail, + truncated: s.truncated, + })); + const finished = listFinishedSessions() + .filter((s) => isInScope(s)) + .map((s) => ({ + sessionId: s.id, + status: s.status, + startedAt: s.startedAt, + endedAt: s.endedAt, + runtimeMs: s.endedAt - s.startedAt, + cwd: s.cwd, + command: s.command, + name: deriveSessionName(s.command), + tail: s.tail, + truncated: s.truncated, + exitCode: s.exitCode ?? undefined, + exitSignal: s.exitSignal ?? undefined, + })); const lines = [...running, ...finished] .sort((a, b) => b.startedAt - a.startedAt) .map((s) => { @@ -547,34 +557,38 @@ export function createProcessTool( const session = getSession(params.sessionId); const finished = getFinishedSession(params.sessionId); + const scopedSession = isInScope(session) ? session : undefined; + const scopedFinished = isInScope(finished) ? finished : undefined; switch (params.action) { case "poll": { - if (!session) { - if (finished) { + if (!scopedSession) { + if (scopedFinished) { return { content: [ { type: "text", text: - (finished.tail || + (scopedFinished.tail || `(no output recorded${ - finished.truncated ? " — truncated to cap" : "" + scopedFinished.truncated ? " — truncated to cap" : "" })`) + `\n\nProcess exited with ${ - finished.exitSignal - ? `signal ${finished.exitSignal}` - : `code ${finished.exitCode ?? 0}` + scopedFinished.exitSignal + ? `signal ${scopedFinished.exitSignal}` + : `code ${scopedFinished.exitCode ?? 0}` }.`, }, ], details: { status: - finished.status === "completed" ? "completed" : "failed", + scopedFinished.status === "completed" + ? "completed" + : "failed", sessionId: params.sessionId, - exitCode: finished.exitCode ?? undefined, - aggregated: finished.aggregated, - name: deriveSessionName(finished.command), + exitCode: scopedFinished.exitCode ?? undefined, + aggregated: scopedFinished.aggregated, + name: deriveSessionName(scopedFinished.command), }, }; } @@ -588,7 +602,7 @@ export function createProcessTool( details: { status: "failed" }, }; } - if (!session.backgrounded) { + if (!scopedSession.backgrounded) { return { content: [ { @@ -599,17 +613,17 @@ export function createProcessTool( details: { status: "failed" }, }; } - const { stdout, stderr } = drainSession(session); - const exited = session.exited; - const exitCode = session.exitCode ?? 0; - const exitSignal = session.exitSignal ?? undefined; + const { stdout, stderr } = drainSession(scopedSession); + const exited = scopedSession.exited; + const exitCode = scopedSession.exitCode ?? 0; + const exitSignal = scopedSession.exitSignal ?? undefined; if (exited) { const status = exitCode === 0 && exitSignal == null ? "completed" : "failed"; markExited( - session, - session.exitCode ?? null, - session.exitSignal ?? null, + scopedSession, + scopedSession.exitCode ?? null, + scopedSession.exitSignal ?? null, status, ); } @@ -639,15 +653,15 @@ export function createProcessTool( status, sessionId: params.sessionId, exitCode: exited ? exitCode : undefined, - aggregated: session.aggregated, - name: deriveSessionName(session.command), + aggregated: scopedSession.aggregated, + name: deriveSessionName(scopedSession.command), }, }; } case "log": { - if (session) { - if (!session.backgrounded) { + if (scopedSession) { + if (!scopedSession.backgrounded) { return { content: [ { @@ -659,31 +673,31 @@ export function createProcessTool( }; } const { slice, totalLines, totalChars } = sliceLogLines( - session.aggregated, + scopedSession.aggregated, params.offset, params.limit, ); return { content: [{ type: "text", text: slice || "(no output yet)" }], details: { - status: session.exited ? "completed" : "running", + status: scopedSession.exited ? "completed" : "running", sessionId: params.sessionId, total: totalLines, totalLines, totalChars, - truncated: session.truncated, - name: deriveSessionName(session.command), + truncated: scopedSession.truncated, + name: deriveSessionName(scopedSession.command), }, }; } - if (finished) { + if (scopedFinished) { const { slice, totalLines, totalChars } = sliceLogLines( - finished.aggregated, + scopedFinished.aggregated, params.offset, params.limit, ); const status = - finished.status === "completed" ? "completed" : "failed"; + scopedFinished.status === "completed" ? "completed" : "failed"; return { content: [ { type: "text", text: slice || "(no output recorded)" }, @@ -694,10 +708,10 @@ export function createProcessTool( total: totalLines, totalLines, totalChars, - truncated: finished.truncated, - exitCode: finished.exitCode ?? undefined, - exitSignal: finished.exitSignal ?? undefined, - name: deriveSessionName(finished.command), + truncated: scopedFinished.truncated, + exitCode: scopedFinished.exitCode ?? undefined, + exitSignal: scopedFinished.exitSignal ?? undefined, + name: deriveSessionName(scopedFinished.command), }, }; } @@ -713,7 +727,7 @@ export function createProcessTool( } case "write": { - if (!session) { + if (!scopedSession) { return { content: [ { @@ -724,7 +738,7 @@ export function createProcessTool( details: { status: "failed" }, }; } - if (!session.backgrounded) { + if (!scopedSession.backgrounded) { return { content: [ { @@ -735,7 +749,10 @@ export function createProcessTool( details: { status: "failed" }, }; } - if (!session.child?.stdin || session.child.stdin.destroyed) { + if ( + !scopedSession.child?.stdin || + scopedSession.child.stdin.destroyed + ) { return { content: [ { @@ -747,13 +764,13 @@ export function createProcessTool( }; } await new Promise((resolve, reject) => { - session.child?.stdin.write(params.data ?? "", (err) => { + scopedSession.child?.stdin.write(params.data ?? "", (err) => { if (err) reject(err); else resolve(); }); }); if (params.eof) { - session.child.stdin.end(); + scopedSession.child.stdin.end(); } return { content: [ @@ -767,13 +784,15 @@ export function createProcessTool( details: { status: "running", sessionId: params.sessionId, - name: session ? deriveSessionName(session.command) : undefined, + name: scopedSession + ? deriveSessionName(scopedSession.command) + : undefined, }, }; } case "kill": { - if (!session) { + if (!scopedSession) { return { content: [ { @@ -784,7 +803,7 @@ export function createProcessTool( details: { status: "failed" }, }; } - if (!session.backgrounded) { + if (!scopedSession.backgrounded) { return { content: [ { @@ -795,21 +814,23 @@ export function createProcessTool( details: { status: "failed" }, }; } - killSession(session); - markExited(session, null, "SIGKILL", "failed"); + killSession(scopedSession); + markExited(scopedSession, null, "SIGKILL", "failed"); return { content: [ { type: "text", text: `Killed session ${params.sessionId}.` }, ], details: { status: "failed", - name: session ? deriveSessionName(session.command) : undefined, + name: scopedSession + ? deriveSessionName(scopedSession.command) + : undefined, }, }; } case "clear": { - if (finished) { + if (scopedFinished) { deleteSession(params.sessionId); return { content: [ @@ -830,20 +851,22 @@ export function createProcessTool( } case "remove": { - if (session) { - killSession(session); - markExited(session, null, "SIGKILL", "failed"); + if (scopedSession) { + killSession(scopedSession); + markExited(scopedSession, null, "SIGKILL", "failed"); return { content: [ { type: "text", text: `Removed session ${params.sessionId}.` }, ], details: { status: "failed", - name: session ? deriveSessionName(session.command) : undefined, + name: scopedSession + ? deriveSessionName(scopedSession.command) + : undefined, }, }; } - if (finished) { + if (scopedFinished) { deleteSession(params.sessionId); return { content: [ diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 449fd2068..5e67bab0c 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -432,6 +432,25 @@ function filterToolsByPolicy( }); } +function resolveEffectiveToolPolicy(params: { + config?: ClawdbotConfig; + sessionKey?: string; +}) { + const agentId = params.sessionKey + ? resolveAgentIdFromSessionKey(params.sessionKey) + : undefined; + const agentConfig = + params.config && agentId + ? resolveAgentConfig(params.config, agentId) + : undefined; + const hasAgentTools = agentConfig?.tools !== undefined; + const globalTools = params.config?.agent?.tools; + return { + agentId, + policy: hasAgentTools ? agentConfig?.tools : globalTools, + }; +} + function isToolAllowedByPolicy(name: string, policy?: SandboxToolPolicy) { if (!policy) return true; const deny = new Set(normalizeToolNames(policy.deny)); @@ -613,16 +632,12 @@ export function createClawdbotCodingTools(options?: { }): AnyAgentTool[] { const bashToolName = "bash"; const sandbox = options?.sandbox?.enabled ? options.sandbox : undefined; - const agentConfig = - options?.sessionKey && options?.config - ? resolveAgentConfig( - options.config, - resolveAgentIdFromSessionKey(options.sessionKey), - ) - : undefined; - const hasAgentTools = agentConfig?.tools !== undefined; - const globalTools = options?.config?.agent?.tools; - const effectiveToolsPolicy = hasAgentTools ? agentConfig?.tools : globalTools; + const { agentId, policy: effectiveToolsPolicy } = resolveEffectiveToolPolicy({ + config: options?.config, + sessionKey: options?.sessionKey, + }); + const scopeKey = + options?.bash?.scopeKey ?? (agentId ? `agent:${agentId}` : undefined); const subagentPolicy = isSubagentSessionKey(options?.sessionKey) && options?.sessionKey ? resolveSubagentToolPolicy(options.config) @@ -649,6 +664,7 @@ export function createClawdbotCodingTools(options?: { const bashTool = createBashTool({ ...options?.bash, allowBackground, + scopeKey, sandbox: sandbox ? { containerName: sandbox.containerName, @@ -660,6 +676,7 @@ export function createClawdbotCodingTools(options?: { }); const processTool = createProcessTool({ cleanupMs: options?.bash?.cleanupMs, + scopeKey, }); const tools: AnyAgentTool[] = [ ...base,