fix: enforce sandbox visibility for session_status ids

This commit is contained in:
Tak Hoffman
2026-03-24 21:05:25 -05:00
parent 2c1d16e261
commit fb04801ed7
2 changed files with 111 additions and 31 deletions

View File

@@ -1,4 +1,6 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { SessionEntry } from "../config/sessions.js";
import { resolvePreferredSessionKeyForSessionIdMatches } from "../sessions/session-id-resolution.js";
const loadSessionStoreMock = vi.fn();
const updateSessionStoreMock = vi.fn();
@@ -188,7 +190,7 @@ async function loadFreshOpenClawToolsForSessionStatusTest() {
({ createSessionStatusTool } = await import("./tools/session-status-tool.js"));
}
function resetSessionStore(store: Record<string, unknown>) {
function resetSessionStore(store: Record<string, SessionEntry>) {
loadSessionStoreMock.mockClear();
updateSessionStoreMock.mockClear();
callGatewayMock.mockClear();
@@ -198,7 +200,35 @@ function resetSessionStore(store: Record<string, unknown>) {
storePath: "(multiple)",
store,
});
callGatewayMock.mockResolvedValue({});
callGatewayMock.mockImplementation(async (opts: unknown) => {
const request = opts as { method?: string; params?: Record<string, unknown> };
if (request.method === "sessions.resolve") {
const key = typeof request.params?.key === "string" ? request.params.key.trim() : "";
if (key && store[key]) {
return { key };
}
const sessionId =
typeof request.params?.sessionId === "string" ? request.params.sessionId.trim() : "";
if (!sessionId) {
return {};
}
const spawnedBy =
typeof request.params?.spawnedBy === "string" ? request.params.spawnedBy.trim() : "";
const matches = Object.entries(store).filter((entry): entry is [string, SessionEntry] => {
return (
entry[1].sessionId === sessionId &&
(!spawnedBy ||
entry[1].spawnedBy === spawnedBy ||
entry[1].parentSessionKey === spawnedBy)
);
});
return { key: resolvePreferredSessionKeyForSessionIdMatches(matches, sessionId) };
}
if (request.method === "sessions.list") {
return { sessions: [] };
}
return {};
});
mockConfig = createMockConfig();
}
@@ -237,7 +267,6 @@ function expectSpawnedSessionLookupCalls(spawnedBy: string) {
params: {
includeGlobal: false,
includeUnknown: false,
limit: 500,
spawnedBy,
},
};
@@ -502,6 +531,62 @@ describe("session_status tool", () => {
expectSpawnedSessionLookupCalls("agent:main:subagent:child");
});
it("blocks sandboxed child session_status sessionId 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,
},
"agent:other:main": {
sessionId: "s-other",
updatedAt: 30,
},
});
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-session-id", {
sessionKey: "s-other",
}),
).rejects.toThrow(expectedError);
expect(loadSessionStoreMock).toHaveBeenCalledTimes(1);
expect(loadSessionStoreMock).toHaveBeenCalledWith("/tmp/main/sessions.json");
expect(updateSessionStoreMock).not.toHaveBeenCalled();
expect(callGatewayMock).toHaveBeenCalledTimes(3);
expect(callGatewayMock.mock.calls).toContainEqual([
{
method: "sessions.resolve",
params: {
sessionId: "s-other",
spawnedBy: "agent:main:subagent:child",
includeGlobal: false,
includeUnknown: false,
},
},
]);
expect(callGatewayMock.mock.calls).toContainEqual([
{
method: "sessions.list",
params: {
includeGlobal: false,
includeUnknown: false,
spawnedBy: "agent:main:subagent:child",
},
},
]);
});
it("keeps legacy main requester keys for sandboxed session tree checks", async () => {
resetSessionStore({
"agent:main:main": {

View File

@@ -10,7 +10,6 @@ import {
type SessionEntry,
updateSessionStore,
} from "../../config/sessions.js";
import { loadCombinedSessionStoreForGateway } from "../../gateway/session-utils.js";
import {
formatUsageWindowSummary,
loadProviderUsageSummary,
@@ -23,7 +22,6 @@ import {
resolveAgentIdFromSessionKey,
} from "../../routing/session-key.js";
import { applyModelOverrideToSessionEntry } from "../../sessions/model-overrides.js";
import { resolvePreferredSessionKeyForSessionIdMatches } from "../../sessions/session-id-resolution.js";
import { resolveAgentDir } from "../agent-scope.js";
import { formatUserTime, resolveUserTimeFormat, resolveUserTimezone } from "../date-time.js";
import { resolveModelAuthLabel } from "../model-auth-label.js";
@@ -43,7 +41,9 @@ import {
createAgentToAgentPolicy,
resolveEffectiveSessionToolsVisibility,
resolveInternalSessionKey,
resolveSessionReference,
resolveSandboxedSessionToolContext,
resolveVisibleSessionReference,
} from "./sessions-helpers.js";
const SessionStatusToolSchema = Type.Object({
@@ -105,24 +105,6 @@ function resolveSessionEntry(params: {
return null;
}
function resolveSessionKeyFromSessionId(params: {
cfg: OpenClawConfig;
sessionId: string;
agentId?: string;
}): string | null {
const trimmed = params.sessionId.trim();
if (!trimmed) {
return null;
}
const { store } = loadCombinedSessionStoreForGateway(params.cfg);
const matches = Object.entries(store).filter(
(entry): entry is [string, SessionEntry] =>
entry[1]?.sessionId === trimmed &&
(!params.agentId || resolveAgentIdFromSessionKey(entry[0]) === params.agentId),
);
return resolvePreferredSessionKeyForSessionIdMatches(matches, trimmed) ?? null;
}
function resolveStoreScopedRequesterKey(params: {
requesterKey: string;
agentId: string;
@@ -329,16 +311,27 @@ export function createSessionStatusTool(opts?: {
!resolved &&
(requestedKeyRaw === "current" || shouldResolveSessionIdInput(requestedKeyRaw))
) {
const resolvedKey = resolveSessionKeyFromSessionId({
cfg,
sessionId: requestedKeyRaw,
agentId: a2aPolicy.enabled ? undefined : requesterAgentId,
const resolvedSession = await resolveSessionReference({
sessionKey: requestedKeyRaw,
alias,
mainKey,
requesterInternalKey: effectiveRequesterKey,
restrictToSpawned: opts?.sandboxed === true,
});
if (resolvedKey) {
if (resolvedSession.ok && resolvedSession.resolvedViaSessionId) {
const visibleSession = await resolveVisibleSessionReference({
resolvedSession,
requesterSessionKey: effectiveRequesterKey,
restrictToSpawned: opts?.sandboxed === true,
visibilitySessionKey: requestedKeyRaw,
});
if (!visibleSession.ok) {
throw new Error("Session status visibility is restricted to the current session tree.");
}
// If resolution points at another agent, enforce A2A policy before switching stores.
ensureAgentAccess(resolveAgentIdFromSessionKey(resolvedKey));
requestedKeyRaw = resolvedKey;
agentId = resolveAgentIdFromSessionKey(resolvedKey);
ensureAgentAccess(resolveAgentIdFromSessionKey(visibleSession.key));
requestedKeyRaw = visibleSession.key;
agentId = resolveAgentIdFromSessionKey(visibleSession.key);
storePath = resolveStorePath(cfg.session?.store, { agentId });
store = loadSessionStore(storePath);
storeScopedRequesterKey = resolveStoreScopedRequesterKey({
@@ -353,6 +346,8 @@ export function createSessionStatusTool(opts?: {
mainKey,
requesterInternalKey: storeScopedRequesterKey,
});
} else if (!resolvedSession.ok && opts?.sandboxed === true) {
throw new Error("Session status visibility is restricted to the current session tree.");
}
}