From fdbcfced844e52fedc9dea304415bccc537dbf96 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Fri, 27 Mar 2026 11:49:24 -0700 Subject: [PATCH] Agents: enforce session status visibility (#55904) * Agents: enforce session_status visibility * Agents: preserve sandboxed session_status visibility checks --- .../openclaw-tools.session-status.test.ts | 253 ++++++++++++++++++ src/agents/tools/session-status-tool.ts | 29 +- 2 files changed, 267 insertions(+), 15 deletions(-) diff --git a/src/agents/openclaw-tools.session-status.test.ts b/src/agents/openclaw-tools.session-status.test.ts index e8d892ffcdf..9a11fdbe032 100644 --- a/src/agents/openclaw-tools.session-status.test.ts +++ b/src/agents/openclaw-tools.session-status.test.ts @@ -386,6 +386,19 @@ describe("session_status tool", () => { updatedAt: 20, }, }); + mockConfig = { + session: { mainKey: "main", scope: "per-sender" }, + tools: { + sessions: { visibility: "all" }, + agentToAgent: { enabled: true, allow: ["*"] }, + }, + agents: { + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: {}, + }, + }, + }; const tool = getSessionStatusTool(); @@ -583,6 +596,19 @@ describe("session_status tool", () => { updatedAt: 100, }, }); + mockConfig = { + session: { mainKey: "main", scope: "per-sender" }, + tools: { + sessions: { visibility: "all" }, + agentToAgent: { enabled: true, allow: ["*"] }, + }, + agents: { + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: {}, + }, + }, + }; const tool = getSessionStatusTool(); @@ -623,6 +649,193 @@ describe("session_status tool", () => { ); }); + it("blocks unsandboxed same-agent session_status outside self visibility", async () => { + resetSessionStore({ + "agent:main:main": { + sessionId: "s-parent", + updatedAt: 10, + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4-6", + }, + "agent:main:subagent:child": { + sessionId: "s-child", + updatedAt: 20, + }, + }); + mockConfig = { + session: { mainKey: "main", scope: "per-sender" }, + tools: { + sessions: { visibility: "self" }, + agentToAgent: { enabled: true, allow: ["*"] }, + }, + agents: { + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: {}, + }, + }, + }; + + const tool = getSessionStatusTool("agent:main:subagent:child"); + + await expect( + tool.execute("call-self-visibility", { + sessionKey: "agent:main:main", + model: "default", + }), + ).rejects.toThrow( + "Session status visibility is restricted to the current session (tools.sessions.visibility=self).", + ); + + expect(loadSessionStoreMock).not.toHaveBeenCalled(); + expect(updateSessionStoreMock).not.toHaveBeenCalled(); + }); + + it("blocks unsandboxed same-agent session_status outside tree visibility before mutation", async () => { + resetSessionStore({ + "agent:main:main": { + sessionId: "s-parent", + updatedAt: 10, + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4-6", + }, + "agent:main:subagent:child": { + sessionId: "s-child", + updatedAt: 20, + }, + }); + mockConfig = { + session: { mainKey: "main", scope: "per-sender" }, + tools: { + sessions: { visibility: "tree" }, + agentToAgent: { enabled: true, allow: ["*"] }, + }, + agents: { + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: {}, + }, + }, + }; + mockSpawnedSessionList(() => []); + + const tool = getSessionStatusTool("agent:main:subagent:child"); + + await expect( + tool.execute("call-tree-visibility", { + sessionKey: "agent:main:main", + model: "default", + }), + ).rejects.toThrow( + "Session status visibility is restricted to the current session tree (tools.sessions.visibility=tree).", + ); + + expect(loadSessionStoreMock).not.toHaveBeenCalled(); + expect(updateSessionStoreMock).not.toHaveBeenCalled(); + expect(callGatewayMock).toHaveBeenCalledTimes(1); + expect(callGatewayMock).toHaveBeenCalledWith({ + method: "sessions.list", + params: { + includeGlobal: false, + includeUnknown: false, + spawnedBy: "agent:main:subagent:child", + }, + }); + }); + + it("allows unsandboxed same-agent session_status under agent visibility", async () => { + resetSessionStore({ + "agent:main:main": { + sessionId: "s-parent", + updatedAt: 10, + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4-6", + }, + "agent:main:subagent:child": { + sessionId: "s-child", + updatedAt: 20, + }, + }); + mockConfig = { + session: { mainKey: "main", scope: "per-sender" }, + tools: { + sessions: { visibility: "agent" }, + agentToAgent: { enabled: true, allow: ["*"] }, + }, + agents: { + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: {}, + }, + }, + }; + + const tool = getSessionStatusTool("agent:main:subagent:child"); + + const result = await tool.execute("call-agent-visibility", { + sessionKey: "agent:main:main", + model: "default", + }); + const details = result.details as { ok?: boolean; sessionKey?: string }; + expect(details.ok).toBe(true); + expect(details.sessionKey).toBe("agent:main:main"); + expect(updateSessionStoreMock).toHaveBeenCalled(); + }); + + it("blocks unsandboxed sessionId session_status outside tree visibility before mutation", async () => { + resetSessionStore({ + "agent:main:main": { + sessionId: "s-parent", + updatedAt: 10, + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4-6", + }, + "agent:main:subagent:child": { + sessionId: "s-child", + updatedAt: 20, + }, + }); + mockConfig = { + session: { mainKey: "main", scope: "per-sender" }, + tools: { + sessions: { visibility: "tree" }, + agentToAgent: { enabled: true, allow: ["*"] }, + }, + agents: { + defaults: { + model: { primary: "openai/gpt-5.4" }, + models: {}, + }, + }, + }; + callGatewayMock.mockImplementation(async (opts: unknown) => { + const request = opts as { method?: string; params?: Record }; + if (request.method === "sessions.resolve") { + if (request.params?.sessionId === "s-parent") { + return { key: "agent:main:main" }; + } + return {}; + } + if (request.method === "sessions.list") { + return { sessions: [] }; + } + return {}; + }); + + const tool = getSessionStatusTool("agent:main:subagent:child"); + + await expect( + tool.execute("call-tree-session-id-visibility", { + sessionKey: "s-parent", + model: "default", + }), + ).rejects.toThrow( + "Session status visibility is restricted to the current session tree (tools.sessions.visibility=tree).", + ); + + expect(updateSessionStoreMock).not.toHaveBeenCalled(); + }); + it("blocks sandboxed child session_status access outside its tree before store lookup", async () => { resetSessionStore({ "agent:main:subagent:child": { @@ -660,6 +873,46 @@ describe("session_status tool", () => { expectSpawnedSessionLookupCalls("agent:main:subagent:child"); }); + it("blocks sandboxed child bare main session_status access outside its tree", async () => { + resetSessionStore({ + "agent:main:subagent:child": { + sessionId: "s-child", + updatedAt: 20, + }, + "agent:main:main": { + sessionId: "s-parent", + updatedAt: 10, + providerOverride: "anthropic", + modelOverride: "claude-sonnet-4-6", + }, + }); + installSandboxedSessionStatusConfig(); + mockSpawnedSessionList(() => []); + + const tool = getSessionStatusTool("agent:main:subagent:child", { + sandboxed: true, + }); + const expectedError = "Session status visibility is restricted to the current session tree"; + + await expect( + tool.execute("call6-bare-main", { + sessionKey: "main", + model: "default", + }), + ).rejects.toThrow(expectedError); + + expect(updateSessionStoreMock).not.toHaveBeenCalled(); + expect(callGatewayMock).toHaveBeenCalledTimes(1); + expect(callGatewayMock).toHaveBeenCalledWith({ + method: "sessions.list", + params: { + includeGlobal: false, + includeUnknown: false, + spawnedBy: "agent:main:subagent:child", + }, + }); + }); + it("blocks sandboxed child session_status sessionId access outside its tree before store lookup", async () => { resetSessionStore({ "agent:main:subagent:child": { diff --git a/src/agents/tools/session-status-tool.ts b/src/agents/tools/session-status-tool.ts index 8550b2c8047..e6be3fb103d 100644 --- a/src/agents/tools/session-status-tool.ts +++ b/src/agents/tools/session-status-tool.ts @@ -242,21 +242,19 @@ export function createSessionStatusTool(opts?: { } return trimmed; }; - const visibilityGuard = - opts?.sandboxed === true - ? await createSessionVisibilityGuard({ - action: "status", - requesterSessionKey: visibilityRequesterKey, - visibility: resolveEffectiveSessionToolsVisibility({ - cfg, - sandboxed: true, - }), - a2aPolicy, - }) - : null; + const visibilityGuard = await createSessionVisibilityGuard({ + action: "status", + requesterSessionKey: visibilityRequesterKey, + visibility: resolveEffectiveSessionToolsVisibility({ + cfg, + sandboxed: opts?.sandboxed === true, + }), + a2aPolicy, + }); const requestedKeyParam = readStringParam(params, "sessionKey"); let requestedKeyRaw = requestedKeyParam ?? opts?.agentSessionKey; + let resolvedTargetViaSessionId = false; if (!requestedKeyRaw?.trim()) { throw new Error("sessionKey required"); } @@ -278,10 +276,10 @@ export function createSessionStatusTool(opts?: { if (requestedKeyRaw.startsWith("agent:")) { const requestedAgentId = resolveAgentIdFromSessionKey(requestedKeyRaw); ensureAgentAccess(requestedAgentId); - const access = visibilityGuard?.check( + const access = visibilityGuard.check( normalizeVisibilityTargetSessionKey(requestedKeyRaw, requestedAgentId), ); - if (access && !access.allowed) { + if (!access.allowed) { throw new Error(access.error); } } @@ -331,6 +329,7 @@ export function createSessionStatusTool(opts?: { } // If resolution points at another agent, enforce A2A policy before switching stores. ensureAgentAccess(resolveAgentIdFromSessionKey(visibleSession.key)); + resolvedTargetViaSessionId = true; requestedKeyRaw = visibleSession.key; agentId = resolveAgentIdFromSessionKey(visibleSession.key); storePath = resolveStorePath(cfg.session?.store, { agentId }); @@ -368,7 +367,7 @@ export function createSessionStatusTool(opts?: { throw new Error(`Unknown ${kind}: ${requestedKeyRaw}`); } - if (visibilityGuard && !isExplicitAgentKey) { + if (resolvedTargetViaSessionId || (opts?.sandboxed === true && !isExplicitAgentKey)) { const access = visibilityGuard.check( normalizeVisibilityTargetSessionKey(resolved.key, agentId), );