diff --git a/CHANGELOG.md b/CHANGELOG.md index cf7a60071bf..0632ab5e271 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai - Security/host env: block inherited `GIT_EXEC_PATH` from sanitized host exec environments so Git helper resolution cannot be steered by host environment state. (`GHSA-jf5v-pqgw-gm5m`)(#43685) Thanks @zpbrent and @vincentkoc. - Security/session_status: enforce sandbox session-tree visibility and shared agent-to-agent access guards before reading or mutating target session state, so sandboxed subagents can no longer inspect parent session metadata or write parent model overrides via `session_status`. (`GHSA-wcxr-59v9-rxr8`)(#43754) Thanks @tdjackey and @vincentkoc. - Models/secrets: enforce source-managed SecretRef markers in generated `models.json` so runtime-resolved provider secrets are not persisted when runtime projection is skipped. (#43759) Thanks @joshavant. +- Security/agent: reject public spawned-run lineage fields and keep workspace inheritance on the internal spawned-session path so external `agent` callers can no longer override the gateway workspace boundary. (`GHSA-2rqg-gjgv-84jm`)(#43801) Thanks @tdjackey and @vincentkoc. - Security/exec allowlist: preserve POSIX case sensitivity and keep `?` within a single path segment so exact-looking allowlist patterns no longer overmatch executables across case or directory boundaries. (`GHSA-f8r2-vg7x-gh8m`)(#43798) Thanks @zpbrent and @vincentkoc. ### Changes diff --git a/src/agents/openclaw-tools.subagents.sessions-spawn-depth-limits.test.ts b/src/agents/openclaw-tools.subagents.sessions-spawn-depth-limits.test.ts index b9c86bf7472..34fcbfbafd4 100644 --- a/src/agents/openclaw-tools.subagents.sessions-spawn-depth-limits.test.ts +++ b/src/agents/openclaw-tools.subagents.sessions-spawn-depth-limits.test.ts @@ -85,7 +85,10 @@ describe("sessions_spawn depth + child limits", () => { }); it("rejects spawning when caller depth reaches maxSpawnDepth", async () => { - const tool = createSessionsSpawnTool({ agentSessionKey: "agent:main:subagent:parent" }); + const tool = createSessionsSpawnTool({ + agentSessionKey: "agent:main:subagent:parent", + workspaceDir: "/parent/workspace", + }); const result = await tool.execute("call-depth-reject", { task: "hello" }); expect(result.details).toMatchObject({ @@ -109,8 +112,13 @@ describe("sessions_spawn depth + child limits", () => { const calls = callGatewayMock.mock.calls.map( (call) => call[0] as { method?: string; params?: Record }, ); - const agentCall = calls.find((entry) => entry.method === "agent"); - expect(agentCall?.params?.spawnedBy).toBe("agent:main:subagent:parent"); + const spawnedByPatch = calls.find( + (entry) => + entry.method === "sessions.patch" && + entry.params?.spawnedBy === "agent:main:subagent:parent", + ); + expect(spawnedByPatch?.params?.key).toMatch(/^agent:main:subagent:/); + expect(typeof spawnedByPatch?.params?.spawnedWorkspaceDir).toBe("string"); const spawnDepthPatch = calls.find( (entry) => entry.method === "sessions.patch" && entry.params?.spawnDepth === 2, diff --git a/src/agents/sessions-spawn-hooks.test.ts b/src/agents/sessions-spawn-hooks.test.ts index e7abc2dba9f..89004289369 100644 --- a/src/agents/sessions-spawn-hooks.test.ts +++ b/src/agents/sessions-spawn-hooks.test.ts @@ -380,4 +380,36 @@ describe("sessions_spawn subagent lifecycle hooks", () => { emitLifecycleHooks: true, }); }); + + it("cleans up the provisional session when lineage patching fails after thread binding", async () => { + const callGatewayMock = getCallGatewayMock(); + callGatewayMock.mockImplementation(async (opts: unknown) => { + const request = opts as { method?: string; params?: Record }; + if (request.method === "sessions.patch" && typeof request.params?.spawnedBy === "string") { + throw new Error("lineage patch failed"); + } + if (request.method === "sessions.delete") { + return { ok: true }; + } + return {}; + }); + + const result = await executeDiscordThreadSessionSpawn("call9"); + + expect(result.details).toMatchObject({ + status: "error", + error: "lineage patch failed", + }); + expect(hookRunnerMocks.runSubagentSpawned).not.toHaveBeenCalled(); + expect(hookRunnerMocks.runSubagentEnded).not.toHaveBeenCalled(); + const methods = getGatewayMethods(); + expect(methods).toContain("sessions.delete"); + expect(methods).not.toContain("agent"); + const deleteCall = findGatewayRequest("sessions.delete"); + expect(deleteCall?.params).toMatchObject({ + key: (result.details as { childSessionKey?: string }).childSessionKey, + deleteTranscript: true, + emitLifecycleHooks: true, + }); + }); }); diff --git a/src/agents/subagent-spawn.attachments.test.ts b/src/agents/subagent-spawn.attachments.test.ts index b564e77a906..9fe774fa284 100644 --- a/src/agents/subagent-spawn.attachments.test.ts +++ b/src/agents/subagent-spawn.attachments.test.ts @@ -1,6 +1,7 @@ +import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { resetSubagentRegistryForTests } from "./subagent-registry.js"; import { decodeStrictBase64, spawnSubagentDirect } from "./subagent-spawn.js"; @@ -31,6 +32,7 @@ let configOverride: Record = { }, }, }; +let workspaceDirOverride = ""; vi.mock("../config/config.js", async (importOriginal) => { const actual = await importOriginal(); @@ -61,7 +63,7 @@ vi.mock("./agent-scope.js", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, - resolveAgentWorkspaceDir: () => path.join(os.tmpdir(), "agent-workspace"), + resolveAgentWorkspaceDir: () => workspaceDirOverride, }; }); @@ -145,6 +147,16 @@ describe("spawnSubagentDirect filename validation", () => { resetSubagentRegistryForTests(); callGatewayMock.mockClear(); setupGatewayMock(); + workspaceDirOverride = fs.mkdtempSync( + path.join(os.tmpdir(), `openclaw-subagent-attachments-${process.pid}-${Date.now()}-`), + ); + }); + + afterEach(() => { + if (workspaceDirOverride) { + fs.rmSync(workspaceDirOverride, { recursive: true, force: true }); + workspaceDirOverride = ""; + } }); const ctx = { @@ -210,4 +222,43 @@ describe("spawnSubagentDirect filename validation", () => { expect(result.status).toBe("error"); expect(result.error).toMatch(/attachments_invalid_name/); }); + + it("removes materialized attachments when lineage patching fails", async () => { + const calls: Array<{ method?: string; params?: Record }> = []; + callGatewayMock.mockImplementation(async (opts: unknown) => { + const request = opts as { method?: string; params?: Record }; + calls.push(request); + if (request.method === "sessions.patch" && typeof request.params?.spawnedBy === "string") { + throw new Error("lineage patch failed"); + } + if (request.method === "sessions.delete") { + return { ok: true }; + } + return {}; + }); + + const result = await spawnSubagentDirect( + { + task: "test", + attachments: [{ name: "file.txt", content: validContent, encoding: "base64" }], + }, + ctx, + ); + + expect(result).toMatchObject({ + status: "error", + error: "lineage patch failed", + }); + const attachmentsRoot = path.join(workspaceDirOverride, ".openclaw", "attachments"); + const retainedDirs = fs.existsSync(attachmentsRoot) + ? fs.readdirSync(attachmentsRoot).filter((entry) => !entry.startsWith(".")) + : []; + expect(retainedDirs).toHaveLength(0); + const deleteCall = calls.find((entry) => entry.method === "sessions.delete"); + expect(deleteCall?.params).toMatchObject({ + key: expect.stringMatching(/^agent:main:subagent:/), + deleteTranscript: true, + emitLifecycleHooks: false, + }); + }); }); diff --git a/src/agents/subagent-spawn.ts b/src/agents/subagent-spawn.ts index be5dac37f83..a4a6229c715 100644 --- a/src/agents/subagent-spawn.ts +++ b/src/agents/subagent-spawn.ts @@ -153,6 +153,25 @@ async function cleanupProvisionalSession( } } +async function cleanupFailedSpawnBeforeAgentStart(params: { + childSessionKey: string; + attachmentAbsDir?: string; + emitLifecycleHooks?: boolean; + deleteTranscript?: boolean; +}): Promise { + if (params.attachmentAbsDir) { + try { + await fs.rm(params.attachmentAbsDir, { recursive: true, force: true }); + } catch { + // Best-effort cleanup only. + } + } + await cleanupProvisionalSession(params.childSessionKey, { + emitLifecycleHooks: params.emitLifecycleHooks, + deleteTranscript: params.deleteTranscript, + }); +} + function resolveSpawnMode(params: { requestedMode?: SpawnSubagentMode; threadRequested: boolean; @@ -561,10 +580,32 @@ export async function spawnSubagentDirect( explicitWorkspaceDir: toolSpawnMetadata.workspaceDir, }), }); + const spawnLineagePatchError = await patchChildSession({ + spawnedBy: spawnedByKey, + ...(spawnedMetadata.workspaceDir ? { spawnedWorkspaceDir: spawnedMetadata.workspaceDir } : {}), + }); + if (spawnLineagePatchError) { + await cleanupFailedSpawnBeforeAgentStart({ + childSessionKey, + attachmentAbsDir, + emitLifecycleHooks: threadBindingReady, + deleteTranscript: true, + }); + return { + status: "error", + error: spawnLineagePatchError, + childSessionKey, + }; + } const childIdem = crypto.randomUUID(); let childRunId: string = childIdem; try { + const { + spawnedBy: _spawnedBy, + workspaceDir: _workspaceDir, + ...publicSpawnedMetadata + } = spawnedMetadata; const response = await callGateway<{ runId: string }>({ method: "agent", params: { @@ -581,7 +622,7 @@ export async function spawnSubagentDirect( thinking: thinkingOverride, timeout: runTimeoutSeconds, label: label || undefined, - ...spawnedMetadata, + ...publicSpawnedMetadata, }, timeoutMs: 10_000, }); diff --git a/src/config/sessions/types.ts b/src/config/sessions/types.ts index 817f9efc3d8..0ae44b2db7a 100644 --- a/src/config/sessions/types.ts +++ b/src/config/sessions/types.ts @@ -78,6 +78,8 @@ export type SessionEntry = { sessionFile?: string; /** Parent session key that spawned this session (used for sandbox session-tool scoping). */ spawnedBy?: string; + /** Workspace inherited by spawned sessions and reused on later turns for the same child session. */ + spawnedWorkspaceDir?: string; /** True after a thread/topic session has been forked from its parent transcript once. */ forkedFromParent?: boolean; /** Subagent spawn depth (0 = main, 1 = sub-agent, 2 = sub-sub-agent). */ diff --git a/src/gateway/protocol/schema/agent.ts b/src/gateway/protocol/schema/agent.ts index 75d560ba92b..eaa54860a10 100644 --- a/src/gateway/protocol/schema/agent.ts +++ b/src/gateway/protocol/schema/agent.ts @@ -110,8 +110,6 @@ export const AgentParamsSchema = Type.Object( ), idempotencyKey: NonEmptyString, label: Type.Optional(SessionLabelString), - spawnedBy: Type.Optional(Type.String()), - workspaceDir: Type.Optional(Type.String()), }, { additionalProperties: false }, ); diff --git a/src/gateway/protocol/schema/sessions.ts b/src/gateway/protocol/schema/sessions.ts index 83f09e8ecba..30595c15698 100644 --- a/src/gateway/protocol/schema/sessions.ts +++ b/src/gateway/protocol/schema/sessions.ts @@ -71,6 +71,7 @@ export const SessionsPatchParamsSchema = Type.Object( execNode: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), model: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), spawnedBy: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), + spawnedWorkspaceDir: Type.Optional(Type.Union([NonEmptyString, Type.Null()])), spawnDepth: Type.Optional(Type.Union([Type.Integer({ minimum: 0 }), Type.Null()])), subagentRole: Type.Optional( Type.Union([Type.Literal("orchestrator"), Type.Literal("leaf"), Type.Null()]), diff --git a/src/gateway/server-methods/agent.test.ts b/src/gateway/server-methods/agent.test.ts index fbc8b056c34..5dfa27b20ce 100644 --- a/src/gateway/server-methods/agent.test.ts +++ b/src/gateway/server-methods/agent.test.ts @@ -405,30 +405,53 @@ describe("gateway agent handler", () => { expect(callArgs.bestEffortDeliver).toBe(false); }); - it("only forwards workspaceDir for spawned subagent runs", async () => { + it("rejects public spawned-run metadata fields", async () => { primeMainAgentRun(); mocks.agentCommand.mockClear(); - - await invokeAgent( - { - message: "normal run", - sessionKey: "agent:main:main", - workspaceDir: "/tmp/ignored", - idempotencyKey: "workspace-ignored", - }, - { reqId: "workspace-ignored-1" }, - ); - await vi.waitFor(() => expect(mocks.agentCommand).toHaveBeenCalled()); - const normalCall = mocks.agentCommand.mock.calls.at(-1)?.[0] as { workspaceDir?: string }; - expect(normalCall.workspaceDir).toBeUndefined(); - mocks.agentCommand.mockClear(); + const respond = vi.fn(); await invokeAgent( { message: "spawned run", sessionKey: "agent:main:main", spawnedBy: "agent:main:subagent:parent", - workspaceDir: "/tmp/inherited", + workspaceDir: "/tmp/injected", + idempotencyKey: "workspace-rejected", + } as AgentParams, + { reqId: "workspace-rejected-1", respond }, + ); + + expect(mocks.agentCommand).not.toHaveBeenCalled(); + expect(respond).toHaveBeenCalledWith( + false, + undefined, + expect.objectContaining({ + message: expect.stringContaining("invalid agent params"), + }), + ); + }); + + it("only forwards workspaceDir for spawned sessions with stored workspace inheritance", async () => { + primeMainAgentRun(); + mockMainSessionEntry({ + spawnedBy: "agent:main:subagent:parent", + spawnedWorkspaceDir: "/tmp/inherited", + }); + mocks.updateSessionStore.mockImplementation(async (_path, updater) => { + const store: Record = { + "agent:main:main": buildExistingMainStoreEntry({ + spawnedBy: "agent:main:subagent:parent", + spawnedWorkspaceDir: "/tmp/inherited", + }), + }; + return await updater(store); + }); + mocks.agentCommand.mockClear(); + + await invokeAgent( + { + message: "spawned run", + sessionKey: "agent:main:main", idempotencyKey: "workspace-forwarded", }, { reqId: "workspace-forwarded-1" }, diff --git a/src/gateway/server-methods/agent.ts b/src/gateway/server-methods/agent.ts index a6d437e6792..98466f91044 100644 --- a/src/gateway/server-methods/agent.ts +++ b/src/gateway/server-methods/agent.ts @@ -190,24 +190,20 @@ export const agentHandlers: GatewayRequestHandlers = { timeout?: number; bestEffortDeliver?: boolean; label?: string; - spawnedBy?: string; inputProvenance?: InputProvenance; - workspaceDir?: string; }; const senderIsOwner = resolveSenderIsOwnerFromClient(client); const cfg = loadConfig(); const idem = request.idempotencyKey; const normalizedSpawned = normalizeSpawnedRunMetadata({ - spawnedBy: request.spawnedBy, groupId: request.groupId, groupChannel: request.groupChannel, groupSpace: request.groupSpace, - workspaceDir: request.workspaceDir, }); let resolvedGroupId: string | undefined = normalizedSpawned.groupId; let resolvedGroupChannel: string | undefined = normalizedSpawned.groupChannel; let resolvedGroupSpace: string | undefined = normalizedSpawned.groupSpace; - let spawnedByValue = normalizedSpawned.spawnedBy; + let spawnedByValue: string | undefined; const inputProvenance = normalizeInputProvenance(request.inputProvenance); const cached = context.dedupe.get(`agent:${idem}`); if (cached) { @@ -359,11 +355,7 @@ export const agentHandlers: GatewayRequestHandlers = { const sessionId = entry?.sessionId ?? randomUUID(); const labelValue = request.label?.trim() || entry?.label; const sessionAgent = resolveAgentIdFromSessionKey(canonicalKey); - spawnedByValue = canonicalizeSpawnedByForAgent( - cfg, - sessionAgent, - spawnedByValue || entry?.spawnedBy, - ); + spawnedByValue = canonicalizeSpawnedByForAgent(cfg, sessionAgent, entry?.spawnedBy); let inheritedGroup: | { groupId?: string; groupChannel?: string; groupSpace?: string } | undefined; @@ -400,6 +392,7 @@ export const agentHandlers: GatewayRequestHandlers = { providerOverride: entry?.providerOverride, label: labelValue, spawnedBy: spawnedByValue, + spawnedWorkspaceDir: entry?.spawnedWorkspaceDir, spawnDepth: entry?.spawnDepth, channel: entry?.channel ?? request.channel?.trim(), groupId: resolvedGroupId ?? entry?.groupId, @@ -628,7 +621,7 @@ export const agentHandlers: GatewayRequestHandlers = { // Internal-only: allow workspace override for spawned subagent runs. workspaceDir: resolveIngressWorkspaceOverrideForSpawnedRun({ spawnedBy: spawnedByValue, - workspaceDir: request.workspaceDir, + workspaceDir: sessionEntry?.spawnedWorkspaceDir, }), senderIsOwner, }, diff --git a/src/gateway/sessions-patch.test.ts b/src/gateway/sessions-patch.test.ts index 2249c7f5c77..79e332f23ba 100644 --- a/src/gateway/sessions-patch.test.ts +++ b/src/gateway/sessions-patch.test.ts @@ -265,6 +265,19 @@ describe("gateway sessions patch", () => { expect(entry.spawnedBy).toBe("agent:main:main"); }); + test("sets spawnedWorkspaceDir for subagent sessions", async () => { + const entry = expectPatchOk( + await runPatch({ + storeKey: "agent:main:subagent:child", + patch: { + key: "agent:main:subagent:child", + spawnedWorkspaceDir: "/tmp/subagent-workspace", + }, + }), + ); + expect(entry.spawnedWorkspaceDir).toBe("/tmp/subagent-workspace"); + }); + test("sets spawnDepth for ACP sessions", async () => { const entry = expectPatchOk( await runPatch({ @@ -282,6 +295,13 @@ describe("gateway sessions patch", () => { expectPatchError(result, "spawnDepth is only supported"); }); + test("rejects spawnedWorkspaceDir on non-subagent sessions", async () => { + const result = await runPatch({ + patch: { key: MAIN_SESSION_KEY, spawnedWorkspaceDir: "/tmp/nope" }, + }); + expectPatchError(result, "spawnedWorkspaceDir is only supported"); + }); + test("normalizes exec/send/group patches", async () => { const entry = expectPatchOk( await runPatch({ diff --git a/src/gateway/sessions-patch.ts b/src/gateway/sessions-patch.ts index 1bf79ba4edf..66010e4745c 100644 --- a/src/gateway/sessions-patch.ts +++ b/src/gateway/sessions-patch.ts @@ -128,6 +128,27 @@ export async function applySessionsPatchToStore(params: { } } + if ("spawnedWorkspaceDir" in patch) { + const raw = patch.spawnedWorkspaceDir; + if (raw === null) { + if (existing?.spawnedWorkspaceDir) { + return invalid("spawnedWorkspaceDir cannot be cleared once set"); + } + } else if (raw !== undefined) { + if (!supportsSpawnLineage(storeKey)) { + return invalid("spawnedWorkspaceDir is only supported for subagent:* or acp:* sessions"); + } + const trimmed = String(raw).trim(); + if (!trimmed) { + return invalid("invalid spawnedWorkspaceDir: empty"); + } + if (existing?.spawnedWorkspaceDir && existing.spawnedWorkspaceDir !== trimmed) { + return invalid("spawnedWorkspaceDir cannot be changed once set"); + } + next.spawnedWorkspaceDir = trimmed; + } + } + if ("spawnDepth" in patch) { const raw = patch.spawnDepth; if (raw === null) {