fix(agents): enforce sandboxed session_status visibility (#43754)

* agents: guard sandboxed session_status access

* test(agents): cover sandboxed session_status scope

* docs(changelog): credit session_status hardening

* agents: preflight sandboxed session_status checks

* test(agents): cover session_status existence oracle

* agents: preserve legacy session_status tree keys

* test(agents): cover legacy session_status tree keys

* Update CHANGELOG.md
This commit is contained in:
Vincent Koc
2026-03-12 02:54:25 -04:00
committed by GitHub
parent 12dc299cde
commit 99ec687d7a
5 changed files with 271 additions and 23 deletions

View File

@@ -10,6 +10,7 @@ Docs: https://docs.openclaw.ai
- Security/device pairing: cap issued and verified device-token scopes to each paired device's approved scope baseline so stale or overbroad tokens cannot exceed approved access. (`GHSA-2pwv-x786-56f8`)(#43686) Thanks @tdjackey and @vincentkoc. - Security/device pairing: cap issued and verified device-token scopes to each paired device's approved scope baseline so stale or overbroad tokens cannot exceed approved access. (`GHSA-2pwv-x786-56f8`)(#43686) Thanks @tdjackey and @vincentkoc.
- Security/proxy attachments: restore the shared media-store size cap for persisted browser proxy files so oversized payloads are rejected instead of overriding the intended 5 MB limit. (`GHSA-6rph-mmhp-h7h9`)(#43684) Thanks @tdjackey and @vincentkoc. - Security/proxy attachments: restore the shared media-store size cap for persisted browser proxy files so oversized payloads are rejected instead of overriding the intended 5 MB limit. (`GHSA-6rph-mmhp-h7h9`)(#43684) Thanks @tdjackey and @vincentkoc.
- 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/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.
### Changes ### Changes
@@ -102,7 +103,6 @@ Docs: https://docs.openclaw.ai
- Security/system.run: fail closed for approval-backed interpreter/runtime commands when OpenClaw cannot bind exactly one concrete local file operand, while extending best-effort direct-file binding to additional runtime forms. Thanks @tdjackey for reporting. - Security/system.run: fail closed for approval-backed interpreter/runtime commands when OpenClaw cannot bind exactly one concrete local file operand, while extending best-effort direct-file binding to additional runtime forms. Thanks @tdjackey for reporting.
- Gateway/session reset auth: split conversation `/new` and `/reset` handling away from the admin-only `sessions.reset` control-plane RPC so write-scoped gateway callers can no longer reach the privileged reset path through `agent`. Thanks @tdjackey for reporting. - Gateway/session reset auth: split conversation `/new` and `/reset` handling away from the admin-only `sessions.reset` control-plane RPC so write-scoped gateway callers can no longer reach the privileged reset path through `agent`. Thanks @tdjackey for reporting.
- Security/plugin runtime: stop unauthenticated plugin HTTP routes from inheriting synthetic admin gateway scopes when they call `runtime.subagent.*`, so admin-only methods like `sessions.delete` stay blocked without gateway auth. - Security/plugin runtime: stop unauthenticated plugin HTTP routes from inheriting synthetic admin gateway scopes when they call `runtime.subagent.*`, so admin-only methods like `sessions.delete` stay blocked without gateway auth.
- 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`.
- Security/nodes: treat the `nodes` agent tool as owner-only fallback policy so non-owner senders cannot reach paired-node approval or invoke paths through the shared tool set. - Security/nodes: treat the `nodes` agent tool as owner-only fallback policy so non-owner senders cannot reach paired-node approval or invoke paths through the shared tool set.
- Security/external content: treat whitespace-delimited `EXTERNAL UNTRUSTED CONTENT` boundary markers like underscore-delimited variants so prompt wrappers cannot bypass marker sanitization. (#35983) Thanks @urianpaul94. - Security/external content: treat whitespace-delimited `EXTERNAL UNTRUSTED CONTENT` boundary markers like underscore-delimited variants so prompt wrappers cannot bypass marker sanitization. (#35983) Thanks @urianpaul94.
- Telegram/exec approvals: reject `/approve` commands aimed at other bots, keep deterministic approval prompts visible when tool-result delivery fails, and stop resolved exact IDs from matching other pending approvals by prefix. (#37233) Thanks @huntharo. - Telegram/exec approvals: reject `/approve` commands aimed at other bots, keep deterministic approval prompts visible when tool-result delivery fails, and stop resolved exact IDs from matching other pending approvals by prefix. (#37233) Thanks @huntharo.

View File

@@ -2,6 +2,22 @@ import { describe, expect, it, vi } from "vitest";
const loadSessionStoreMock = vi.fn(); const loadSessionStoreMock = vi.fn();
const updateSessionStoreMock = vi.fn(); const updateSessionStoreMock = vi.fn();
const callGatewayMock = vi.fn();
const createMockConfig = () => ({
session: { mainKey: "main", scope: "per-sender" },
agents: {
defaults: {
model: { primary: "anthropic/claude-opus-4-5" },
models: {},
},
},
tools: {
agentToAgent: { enabled: false },
},
});
let mockConfig: Record<string, unknown> = createMockConfig();
vi.mock("../config/sessions.js", async (importOriginal) => { vi.mock("../config/sessions.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../config/sessions.js")>(); const actual = await importOriginal<typeof import("../config/sessions.js")>();
@@ -22,19 +38,15 @@ vi.mock("../config/sessions.js", async (importOriginal) => {
}; };
}); });
vi.mock("../gateway/call.js", () => ({
callGateway: (opts: unknown) => callGatewayMock(opts),
}));
vi.mock("../config/config.js", async (importOriginal) => { vi.mock("../config/config.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("../config/config.js")>(); const actual = await importOriginal<typeof import("../config/config.js")>();
return { return {
...actual, ...actual,
loadConfig: () => ({ loadConfig: () => mockConfig,
session: { mainKey: "main", scope: "per-sender" },
agents: {
defaults: {
model: { primary: "anthropic/claude-opus-4-5" },
models: {},
},
},
}),
}; };
}); });
@@ -82,13 +94,17 @@ import { createOpenClawTools } from "./openclaw-tools.js";
function resetSessionStore(store: Record<string, unknown>) { function resetSessionStore(store: Record<string, unknown>) {
loadSessionStoreMock.mockClear(); loadSessionStoreMock.mockClear();
updateSessionStoreMock.mockClear(); updateSessionStoreMock.mockClear();
callGatewayMock.mockClear();
loadSessionStoreMock.mockReturnValue(store); loadSessionStoreMock.mockReturnValue(store);
callGatewayMock.mockResolvedValue({});
mockConfig = createMockConfig();
} }
function getSessionStatusTool(agentSessionKey = "main") { function getSessionStatusTool(agentSessionKey = "main", options?: { sandboxed?: boolean }) {
const tool = createOpenClawTools({ agentSessionKey }).find( const tool = createOpenClawTools({
(candidate) => candidate.name === "session_status", agentSessionKey,
); sandboxed: options?.sandboxed,
}).find((candidate) => candidate.name === "session_status");
expect(tool).toBeDefined(); expect(tool).toBeDefined();
if (!tool) { if (!tool) {
throw new Error("missing session_status tool"); throw new Error("missing session_status tool");
@@ -176,6 +192,153 @@ describe("session_status tool", () => {
); );
}); });
it("blocks sandboxed child session_status access outside its tree before store lookup", async () => {
resetSessionStore({
"agent:main:subagent:child": {
sessionId: "s-child",
updatedAt: 20,
},
"agent:main:main": {
sessionId: "s-parent",
updatedAt: 10,
},
});
mockConfig = {
session: { mainKey: "main", scope: "per-sender" },
tools: {
sessions: { visibility: "all" },
agentToAgent: { enabled: true, allow: ["*"] },
},
agents: {
defaults: {
model: { primary: "anthropic/claude-opus-4-5" },
models: {},
sandbox: { sessionToolsVisibility: "spawned" },
},
},
};
callGatewayMock.mockImplementation(async (opts: unknown) => {
const request = opts as { method?: string; params?: Record<string, unknown> };
if (request.method === "sessions.list") {
return { sessions: [] };
}
return {};
});
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", {
sessionKey: "agent:main:main",
model: "anthropic/claude-sonnet-4-5",
}),
).rejects.toThrow(expectedError);
await expect(
tool.execute("call7", {
sessionKey: "agent:main:subagent:missing",
}),
).rejects.toThrow(expectedError);
expect(loadSessionStoreMock).not.toHaveBeenCalled();
expect(updateSessionStoreMock).not.toHaveBeenCalled();
expect(callGatewayMock).toHaveBeenCalledTimes(2);
expect(callGatewayMock).toHaveBeenNthCalledWith(1, {
method: "sessions.list",
params: {
includeGlobal: false,
includeUnknown: false,
limit: 500,
spawnedBy: "agent:main:subagent:child",
},
});
expect(callGatewayMock).toHaveBeenNthCalledWith(2, {
method: "sessions.list",
params: {
includeGlobal: false,
includeUnknown: false,
limit: 500,
spawnedBy: "agent:main:subagent:child",
},
});
});
it("keeps legacy main requester keys for sandboxed session tree checks", async () => {
resetSessionStore({
"agent:main:main": {
sessionId: "s-main",
updatedAt: 10,
},
"agent:main:subagent:child": {
sessionId: "s-child",
updatedAt: 20,
},
});
mockConfig = {
session: { mainKey: "main", scope: "per-sender" },
tools: {
sessions: { visibility: "all" },
agentToAgent: { enabled: true, allow: ["*"] },
},
agents: {
defaults: {
model: { primary: "anthropic/claude-opus-4-5" },
models: {},
sandbox: { sessionToolsVisibility: "spawned" },
},
},
};
callGatewayMock.mockImplementation(async (opts: unknown) => {
const request = opts as { method?: string; params?: Record<string, unknown> };
if (request.method === "sessions.list") {
return {
sessions:
request.params?.spawnedBy === "main" ? [{ key: "agent:main:subagent:child" }] : [],
};
}
return {};
});
const tool = getSessionStatusTool("main", {
sandboxed: true,
});
const mainResult = await tool.execute("call8", {});
const mainDetails = mainResult.details as { ok?: boolean; sessionKey?: string };
expect(mainDetails.ok).toBe(true);
expect(mainDetails.sessionKey).toBe("agent:main:main");
const childResult = await tool.execute("call9", {
sessionKey: "agent:main:subagent:child",
});
const childDetails = childResult.details as { ok?: boolean; sessionKey?: string };
expect(childDetails.ok).toBe(true);
expect(childDetails.sessionKey).toBe("agent:main:subagent:child");
expect(callGatewayMock).toHaveBeenCalledTimes(2);
expect(callGatewayMock).toHaveBeenNthCalledWith(1, {
method: "sessions.list",
params: {
includeGlobal: false,
includeUnknown: false,
limit: 500,
spawnedBy: "main",
},
});
expect(callGatewayMock).toHaveBeenNthCalledWith(2, {
method: "sessions.list",
params: {
includeGlobal: false,
includeUnknown: false,
limit: 500,
spawnedBy: "main",
},
});
});
it("scopes bare session keys to the requester agent", async () => { it("scopes bare session keys to the requester agent", async () => {
loadSessionStoreMock.mockClear(); loadSessionStoreMock.mockClear();
updateSessionStoreMock.mockClear(); updateSessionStoreMock.mockClear();

View File

@@ -200,6 +200,7 @@ export function createOpenClawTools(
createSessionStatusTool({ createSessionStatusTool({
agentSessionKey: options?.agentSessionKey, agentSessionKey: options?.agentSessionKey,
config: options?.config, config: options?.config,
sandboxed: options?.sandboxed,
}), }),
...(webSearchTool ? [webSearchTool] : []), ...(webSearchTool ? [webSearchTool] : []),
...(webFetchTool ? [webFetchTool] : []), ...(webFetchTool ? [webFetchTool] : []),

View File

@@ -19,6 +19,7 @@ import {
import { import {
buildAgentMainSessionKey, buildAgentMainSessionKey,
DEFAULT_AGENT_ID, DEFAULT_AGENT_ID,
parseAgentSessionKey,
resolveAgentIdFromSessionKey, resolveAgentIdFromSessionKey,
} from "../../routing/session-key.js"; } from "../../routing/session-key.js";
import { applyModelOverrideToSessionEntry } from "../../sessions/model-overrides.js"; import { applyModelOverrideToSessionEntry } from "../../sessions/model-overrides.js";
@@ -36,10 +37,12 @@ import {
import type { AnyAgentTool } from "./common.js"; import type { AnyAgentTool } from "./common.js";
import { readStringParam } from "./common.js"; import { readStringParam } from "./common.js";
import { import {
createSessionVisibilityGuard,
shouldResolveSessionIdInput, shouldResolveSessionIdInput,
resolveInternalSessionKey,
resolveMainSessionAlias,
createAgentToAgentPolicy, createAgentToAgentPolicy,
resolveEffectiveSessionToolsVisibility,
resolveInternalSessionKey,
resolveSandboxedSessionToolContext,
} from "./sessions-helpers.js"; } from "./sessions-helpers.js";
const SessionStatusToolSchema = Type.Object({ const SessionStatusToolSchema = Type.Object({
@@ -175,6 +178,7 @@ async function resolveModelOverride(params: {
export function createSessionStatusTool(opts?: { export function createSessionStatusTool(opts?: {
agentSessionKey?: string; agentSessionKey?: string;
config?: OpenClawConfig; config?: OpenClawConfig;
sandboxed?: boolean;
}): AnyAgentTool { }): AnyAgentTool {
return { return {
label: "Session Status", label: "Session Status",
@@ -185,18 +189,70 @@ export function createSessionStatusTool(opts?: {
execute: async (_toolCallId, args) => { execute: async (_toolCallId, args) => {
const params = args as Record<string, unknown>; const params = args as Record<string, unknown>;
const cfg = opts?.config ?? loadConfig(); const cfg = opts?.config ?? loadConfig();
const { mainKey, alias } = resolveMainSessionAlias(cfg); const { mainKey, alias, effectiveRequesterKey } = resolveSandboxedSessionToolContext({
cfg,
agentSessionKey: opts?.agentSessionKey,
sandboxed: opts?.sandboxed,
});
const a2aPolicy = createAgentToAgentPolicy(cfg); const a2aPolicy = createAgentToAgentPolicy(cfg);
const requesterAgentId = resolveAgentIdFromSessionKey(
opts?.agentSessionKey ?? effectiveRequesterKey,
);
const visibilityRequesterKey = effectiveRequesterKey.trim();
const usesLegacyMainAlias = alias === mainKey;
const isLegacyMainVisibilityKey = (sessionKey: string) => {
const trimmed = sessionKey.trim();
return usesLegacyMainAlias && (trimmed === "main" || trimmed === mainKey);
};
const resolveVisibilityMainSessionKey = (sessionAgentId: string) => {
const requesterParsed = parseAgentSessionKey(visibilityRequesterKey);
if (
resolveAgentIdFromSessionKey(visibilityRequesterKey) === sessionAgentId &&
(requesterParsed?.rest === mainKey || isLegacyMainVisibilityKey(visibilityRequesterKey))
) {
return visibilityRequesterKey;
}
return buildAgentMainSessionKey({
agentId: sessionAgentId,
mainKey,
});
};
const normalizeVisibilityTargetSessionKey = (sessionKey: string, sessionAgentId: string) => {
const trimmed = sessionKey.trim();
if (!trimmed) {
return trimmed;
}
if (trimmed.startsWith("agent:")) {
const parsed = parseAgentSessionKey(trimmed);
if (parsed?.rest === mainKey) {
return resolveVisibilityMainSessionKey(sessionAgentId);
}
return trimmed;
}
// Preserve legacy bare main keys for requester tree checks.
if (isLegacyMainVisibilityKey(trimmed)) {
return resolveVisibilityMainSessionKey(sessionAgentId);
}
return trimmed;
};
const visibilityGuard =
opts?.sandboxed === true
? await createSessionVisibilityGuard({
action: "status",
requesterSessionKey: visibilityRequesterKey,
visibility: resolveEffectiveSessionToolsVisibility({
cfg,
sandboxed: true,
}),
a2aPolicy,
})
: null;
const requestedKeyParam = readStringParam(params, "sessionKey"); const requestedKeyParam = readStringParam(params, "sessionKey");
let requestedKeyRaw = requestedKeyParam ?? opts?.agentSessionKey; let requestedKeyRaw = requestedKeyParam ?? opts?.agentSessionKey;
if (!requestedKeyRaw?.trim()) { if (!requestedKeyRaw?.trim()) {
throw new Error("sessionKey required"); throw new Error("sessionKey required");
} }
const requesterAgentId = resolveAgentIdFromSessionKey(
opts?.agentSessionKey ?? requestedKeyRaw,
);
const ensureAgentAccess = (targetAgentId: string) => { const ensureAgentAccess = (targetAgentId: string) => {
if (targetAgentId === requesterAgentId) { if (targetAgentId === requesterAgentId) {
return; return;
@@ -213,7 +269,14 @@ export function createSessionStatusTool(opts?: {
}; };
if (requestedKeyRaw.startsWith("agent:")) { if (requestedKeyRaw.startsWith("agent:")) {
ensureAgentAccess(resolveAgentIdFromSessionKey(requestedKeyRaw)); const requestedAgentId = resolveAgentIdFromSessionKey(requestedKeyRaw);
ensureAgentAccess(requestedAgentId);
const access = visibilityGuard?.check(
normalizeVisibilityTargetSessionKey(requestedKeyRaw, requestedAgentId),
);
if (access && !access.allowed) {
throw new Error(access.error);
}
} }
const isExplicitAgentKey = requestedKeyRaw.startsWith("agent:"); const isExplicitAgentKey = requestedKeyRaw.startsWith("agent:");
@@ -258,6 +321,15 @@ export function createSessionStatusTool(opts?: {
throw new Error(`Unknown ${kind}: ${requestedKeyRaw}`); throw new Error(`Unknown ${kind}: ${requestedKeyRaw}`);
} }
if (visibilityGuard && !requestedKeyRaw.startsWith("agent:")) {
const access = visibilityGuard.check(
normalizeVisibilityTargetSessionKey(resolved.key, agentId),
);
if (!access.allowed) {
throw new Error(access.error);
}
}
const configured = resolveDefaultModelForAgent({ cfg, agentId }); const configured = resolveDefaultModelForAgent({ cfg, agentId });
const modelRaw = readStringParam(params, "model"); const modelRaw = readStringParam(params, "model");
let changedModel = false; let changedModel = false;

View File

@@ -14,7 +14,7 @@ export type AgentToAgentPolicy = {
isAllowed: (requesterAgentId: string, targetAgentId: string) => boolean; isAllowed: (requesterAgentId: string, targetAgentId: string) => boolean;
}; };
export type SessionAccessAction = "history" | "send" | "list"; export type SessionAccessAction = "history" | "send" | "list" | "status";
export type SessionAccessResult = export type SessionAccessResult =
| { allowed: true } | { allowed: true }
@@ -130,6 +130,9 @@ function actionPrefix(action: SessionAccessAction): string {
if (action === "send") { if (action === "send") {
return "Session send"; return "Session send";
} }
if (action === "status") {
return "Session status";
}
return "Session list"; return "Session list";
} }
@@ -140,6 +143,9 @@ function a2aDisabledMessage(action: SessionAccessAction): string {
if (action === "send") { if (action === "send") {
return "Agent-to-agent messaging is disabled. Set tools.agentToAgent.enabled=true to allow cross-agent sends."; return "Agent-to-agent messaging is disabled. Set tools.agentToAgent.enabled=true to allow cross-agent sends.";
} }
if (action === "status") {
return "Agent-to-agent status is disabled. Set tools.agentToAgent.enabled=true to allow cross-agent access.";
}
return "Agent-to-agent listing is disabled. Set tools.agentToAgent.enabled=true to allow cross-agent visibility."; return "Agent-to-agent listing is disabled. Set tools.agentToAgent.enabled=true to allow cross-agent visibility.";
} }
@@ -150,6 +156,9 @@ function a2aDeniedMessage(action: SessionAccessAction): string {
if (action === "send") { if (action === "send") {
return "Agent-to-agent messaging denied by tools.agentToAgent.allow."; return "Agent-to-agent messaging denied by tools.agentToAgent.allow.";
} }
if (action === "status") {
return "Agent-to-agent status denied by tools.agentToAgent.allow.";
}
return "Agent-to-agent listing denied by tools.agentToAgent.allow."; return "Agent-to-agent listing denied by tools.agentToAgent.allow.";
} }
@@ -160,6 +169,9 @@ function crossVisibilityMessage(action: SessionAccessAction): string {
if (action === "send") { if (action === "send") {
return "Session send visibility is restricted. Set tools.sessions.visibility=all to allow cross-agent access."; return "Session send visibility is restricted. Set tools.sessions.visibility=all to allow cross-agent access.";
} }
if (action === "status") {
return "Session status visibility is restricted. Set tools.sessions.visibility=all to allow cross-agent access.";
}
return "Session list visibility is restricted. Set tools.sessions.visibility=all to allow cross-agent access."; return "Session list visibility is restricted. Set tools.sessions.visibility=all to allow cross-agent access.";
} }