From fc14a5a587b18c45fdf183bc71bdccf846963421 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 12 Aug 2026 20:11:27 +0800 Subject: [PATCH] fix(sessions): reject missing explicit session targets (#122564) * fix(sessions): reject missing explicit session targets Punchcard-Session: brisk-willow-summit-k0 * fix(sessions): scope strict resolution to send and history Punchcard-Session: brisk-willow-summit-k0 --- src/agents/openclaw-tools.sessions.test.ts | 5 +- .../tools/sessions-history-tool.test.ts | 76 ++++- src/agents/tools/sessions-history-tool.ts | 19 +- src/agents/tools/sessions-resolution.test.ts | 302 ++++++++++++++---- src/agents/tools/sessions-resolution.ts | 137 +++++--- src/agents/tools/sessions-send-tool.ts | 112 ++++--- src/agents/tools/sessions.test.ts | 42 ++- src/gateway/server.sessions-send.test.ts | 56 +++- .../server.sessions.preview-resolve.test.ts | 12 + 9 files changed, 569 insertions(+), 192 deletions(-) diff --git a/src/agents/openclaw-tools.sessions.test.ts b/src/agents/openclaw-tools.sessions.test.ts index c0ad761cbf87..95b129a7a2c2 100644 --- a/src/agents/openclaw-tools.sessions.test.ts +++ b/src/agents/openclaw-tools.sessions.test.ts @@ -1164,6 +1164,9 @@ describe("sessions tools", () => { callGatewayMock.mockImplementation(async (opts: unknown) => { const request = opts as GatewayCall; calls.push(request); + if (request.method === "sessions.resolve") { + return { key: targetSessionKey }; + } if (request.method === "agent") { return { runId: "run-scoped", status: "accepted", acceptedAt: 1 }; } @@ -1193,8 +1196,8 @@ describe("sessions tools", () => { watched: false, }); expect(calls.map((call) => call.method)).toEqual([ - "sessions.list", "sessions.resolve", + "sessions.list", "agent", ]); } finally { diff --git a/src/agents/tools/sessions-history-tool.test.ts b/src/agents/tools/sessions-history-tool.test.ts index 7206a5bcc632..0d2341db8409 100644 --- a/src/agents/tools/sessions-history-tool.test.ts +++ b/src/agents/tools/sessions-history-tool.test.ts @@ -146,6 +146,76 @@ describe("sessions_history redaction", () => { ); }); + it("returns not-found for an unknown explicit key without reading history", async () => { + const requests: CallGatewayRequest[] = []; + const sessionKey = "agent:main:missing"; + const tool = createSessionsHistoryTool({ + config: { tools: { sessions: { visibility: "all" } } }, + callGateway: async >(request: CallGatewayRequest): Promise => { + requests.push(request); + if (request.method === "sessions.resolve") { + throw new Error(`No session found: ${sessionKey}`); + } + return { messages: [] } as T; + }, + }); + + const result = await tool.execute("missing-explicit-key", { sessionKey }); + + expect(result.details).toEqual({ + status: "error", + error: `No session found: ${sessionKey}`, + }); + expect(requests.map((request) => request.method)).toEqual(["sessions.resolve"]); + }); + + it("conceals missing explicit keys denied by session visibility", async () => { + const requests: CallGatewayRequest[] = []; + const tool = createSessionsHistoryTool({ + agentSessionKey: "agent:main:main", + config: { tools: { sessions: { visibility: "self" } } }, + callGateway: async >(request: CallGatewayRequest): Promise => { + requests.push(request); + throw new Error("No session found: agent:main:missing"); + }, + }); + + const result = await tool.execute("hidden-missing-key", { + sessionKey: "agent:main:missing", + }); + + expect(result.details).toMatchObject({ status: "forbidden" }); + expect(requests.map((request) => request.method)).toEqual(["sessions.resolve"]); + }); + + it("returns an empty history for an existing explicit key", async () => { + const requests: CallGatewayRequest[] = []; + const sessionKey = "agent:main:empty"; + const tool = createSessionsHistoryTool({ + config: { tools: { sessions: { visibility: "all" } } }, + callGateway: async >(request: CallGatewayRequest): Promise => { + requests.push(request); + if (request.method === "sessions.resolve") { + return { key: sessionKey } as T; + } + return { messages: [] } as T; + }, + }); + + const result = await tool.execute("existing-empty-key", { sessionKey }); + + expect(result.details).toMatchObject({ + sessionKey, + messages: [], + bytes: 2, + }); + expect(requests.map((request) => request.method)).toEqual([ + "sessions.resolve", + "sessions.list", + "chat.history", + ]); + }); + it("redacts recalled session text even when log redaction is disabled", async () => { // Recalled transcript content is model-visible, so it is always redacted // even when normal logging redaction is configured off. @@ -445,7 +515,11 @@ describe("sessions_history redaction", () => { sessionKey: targetSessionKey, messages: [{ role: "assistant", content: "visible" }], }); - expect(requests.map((request) => request.method)).toEqual(["sessions.list", "chat.history"]); + expect(requests.map((request) => request.method)).toEqual([ + "sessions.resolve", + "sessions.list", + "chat.history", + ]); } finally { unregister(); } diff --git a/src/agents/tools/sessions-history-tool.ts b/src/agents/tools/sessions-history-tool.ts index a84065478855..28b9f1a0161b 100644 --- a/src/agents/tools/sessions-history-tool.ts +++ b/src/agents/tools/sessions-history-tool.ts @@ -33,6 +33,7 @@ import { import { runWithScopedSessionAccess } from "./scoped-session-access.js"; import { createSessionVisibilityGuard, + createSessionVisibilityRowChecker, createAgentToAgentPolicy, resolveEffectiveSessionToolsVisibility, resolveSessionReference, @@ -398,12 +399,25 @@ export function createSessionsHistoryTool(opts?: { if (!resolvedSession.ok) { return jsonResult({ status: resolvedSession.status, error: resolvedSession.error }); } + const a2aPolicy = createAgentToAgentPolicy(cfg); + const visibility = resolveEffectiveSessionToolsVisibility({ + cfg, + sandboxed: opts?.sandboxed === true, + }); + const resolutionAccess = createSessionVisibilityRowChecker({ + action: "history", + defaultAgentId: resolveDefaultAgentId(cfg), + requesterSessionKey: effectiveRequesterKey, + visibility, + a2aPolicy, + }).check({ key: resolvedSession.key }); const visibleSession = await resolveVisibleSessionReference({ action: "history", resolvedSession, requesterSessionKey: effectiveRequesterKey, restrictToSpawned, visibilitySessionKey: sessionKeyParam, + concealResolutionError: resolutionAccess.allowed ? undefined : resolutionAccess.error, callGateway: gatewayCall, }); if (!visibleSession.ok) { @@ -416,11 +430,6 @@ export function createSessionsHistoryTool(opts?: { const resolvedKey = visibleSession.key; const displayKey = visibleSession.displayKey; - const a2aPolicy = createAgentToAgentPolicy(cfg); - const visibility = resolveEffectiveSessionToolsVisibility({ - cfg, - sandboxed: opts?.sandboxed === true, - }); const visibilityGuard = await createSessionVisibilityGuard({ action: "history", defaultAgentId: resolveDefaultAgentId(cfg), diff --git a/src/agents/tools/sessions-resolution.test.ts b/src/agents/tools/sessions-resolution.test.ts index 8e42750581ee..a2e345d0d081 100644 --- a/src/agents/tools/sessions-resolution.test.ts +++ b/src/agents/tools/sessions-resolution.test.ts @@ -160,7 +160,7 @@ describe("resolved session visibility checks", () => { await expect( resolveVisibleSessionReference({ - action: "history", + action: "status", resolvedSession: { ok: true, key: sessionKey, @@ -210,7 +210,7 @@ describe("resolved session visibility checks", () => { for (const testCase of cases) { callGatewayMock.mockResolvedValueOnce({ key: testCase.targetSessionKey }); const result = resolveVisibleSessionReference({ - action: "history", + action: "status", resolvedSession: { ok: true, key: testCase.targetSessionKey, @@ -253,7 +253,7 @@ describe("resolved session visibility checks", () => { await expect( resolveVisibleSessionReference({ - action: "history", + action: "status", resolvedSession: { ok: true, key: "agent:main:subagent:worker-999", @@ -281,7 +281,7 @@ describe("resolved session visibility checks", () => { await expect( resolveVisibleSessionReference({ - action: "history", + action: "status", resolvedSession: { ok: true, key: "agent:main:subagent:worker", @@ -362,71 +362,6 @@ describe("resolveSessionReference", () => { }); }); - it("retries literal current probes without allowMissing for older gateways", async () => { - const unsupportedAllowMissing = () => - new GatewayClientRequestError({ - code: "INVALID_REQUEST", - message: "invalid sessions.resolve params: at root: unexpected property 'allowMissing'", - }); - callGatewayMock - .mockRejectedValueOnce(unsupportedAllowMissing()) - .mockRejectedValueOnce( - new GatewayClientRequestError({ - code: "INVALID_REQUEST", - message: "No session found: current", - }), - ) - .mockRejectedValueOnce(unsupportedAllowMissing()) - .mockResolvedValueOnce({ key: "agent:ops:main" }); - - const result = await resolveSessionReference({ - sessionKey: "current", - alias: "main", - mainKey: "main", - requesterInternalKey: "agent:main:subagent:child", - restrictToSpawned: false, - }); - expectResolvedSessionReference(result, { - key: "agent:ops:main", - displayKey: "agent:ops:main", - resolvedViaSessionId: true, - }); - expect(callGatewayMock).toHaveBeenNthCalledWith(1, { - method: "sessions.resolve", - params: { - key: "current", - spawnedBy: undefined, - allowMissing: true, - }, - }); - expect(callGatewayMock).toHaveBeenNthCalledWith(2, { - method: "sessions.resolve", - params: { - key: "current", - spawnedBy: undefined, - }, - }); - expect(callGatewayMock).toHaveBeenNthCalledWith(3, { - method: "sessions.resolve", - params: { - sessionId: "current", - spawnedBy: undefined, - includeGlobal: true, - includeUnknown: true, - allowMissing: true, - }, - }); - expect(callGatewayMock).toHaveBeenNthCalledWith(4, { - method: "sessions.resolve", - params: { - sessionId: "current", - spawnedBy: undefined, - includeGlobal: true, - includeUnknown: true, - }, - }); - }); - it("does not compatibility-retry unrelated gateway failures", async () => { callGatewayMock.mockRejectedValueOnce(new Error("gateway timeout")).mockResolvedValueOnce({}); @@ -486,4 +421,233 @@ describe("resolveSessionReference", () => { }); expect(callGatewayMock).not.toHaveBeenCalled(); }); + + it("preserves the main alias without probing configured-main bootstrap", async () => { + const result = await resolveSessionReference({ + sessionKey: "main", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:dashboard:requester", + restrictToSpawned: false, + }); + + expectResolvedSessionReference(result, { + key: "main", + displayKey: "main", + resolvedViaSessionId: false, + }); + expect(callGatewayMock).not.toHaveBeenCalled(); + }); + + it("defers explicit-key lookup to action-aware visibility resolution", async () => { + const result = await resolveSessionReference({ + sessionKey: "agent:main:worker", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:main", + restrictToSpawned: false, + }); + + expect(result).toEqual({ + ok: true, + key: "agent:main:worker", + displayKey: "agent:main:worker", + resolvedViaSessionId: false, + }); + expect(callGatewayMock).not.toHaveBeenCalled(); + }); + + it("rejects an unknown explicit session key for history", async () => { + callGatewayMock.mockRejectedValueOnce( + new GatewayClientRequestError({ + code: "INVALID_REQUEST", + message: "No session found: agent:main:missing", + }), + ); + + const resolvedSession = await resolveSessionReference({ + sessionKey: "agent:main:missing", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:main", + restrictToSpawned: false, + }); + if (!resolvedSession.ok) { + throw new Error("Expected session reference"); + } + const result = await resolveVisibleSessionReference({ + action: "history", + resolvedSession, + requesterSessionKey: "agent:main:main", + restrictToSpawned: false, + visibilitySessionKey: "agent:main:missing", + }); + + expect(result).toEqual({ + ok: false, + status: "error", + error: "No session found: agent:main:missing", + displayKey: "agent:main:missing", + }); + expect(callGatewayMock).toHaveBeenCalledWith({ + method: "sessions.resolve", + params: { + key: "agent:main:missing", + spawnedBy: undefined, + }, + }); + }); + + it("canonicalizes an existing explicit session key", async () => { + callGatewayMock.mockResolvedValueOnce({ key: "agent:ops:main" }); + + const resolvedSession = await resolveSessionReference({ + sessionKey: "agent:OPS:main", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:main", + restrictToSpawned: false, + }); + if (!resolvedSession.ok) { + throw new Error("Expected session reference"); + } + const result = await resolveVisibleSessionReference({ + action: "send", + resolvedSession, + requesterSessionKey: "agent:main:main", + restrictToSpawned: false, + visibilitySessionKey: "agent:OPS:main", + }); + + expect(result).toEqual({ + ok: true, + key: "agent:ops:main", + displayKey: "agent:ops:main", + }); + }); + + it("rejects an explicit key that canonicalizes to an incognito session", async () => { + callGatewayMock.mockResolvedValueOnce({ key: "agent:ops:dashboard:incognito-private" }); + + const resolvedSession = await resolveSessionReference({ + sessionKey: "agent:OPS:dashboard:private", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:main", + restrictToSpawned: false, + }); + if (!resolvedSession.ok) { + throw new Error("Expected session reference"); + } + const result = await resolveVisibleSessionReference({ + action: "history", + resolvedSession, + requesterSessionKey: "agent:main:main", + restrictToSpawned: false, + visibilitySessionKey: "agent:OPS:dashboard:private", + }); + + expect(result).toEqual({ + ok: false, + status: "forbidden", + error: "Session not visible from session tools: agent:OPS:dashboard:private", + displayKey: "agent:ops:dashboard:incognito-private", + }); + }); + + it("conceals a missing explicit key from sandboxed callers", async () => { + callGatewayMock.mockRejectedValueOnce(new Error("No session found: agent:main:missing")); + + const resolvedSession = await resolveSessionReference({ + sessionKey: "agent:main:missing", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:subagent:child", + restrictToSpawned: true, + }); + if (!resolvedSession.ok) { + throw new Error("Expected session reference"); + } + const result = await resolveVisibleSessionReference({ + action: "history", + resolvedSession, + requesterSessionKey: "agent:main:subagent:child", + restrictToSpawned: true, + visibilitySessionKey: "agent:main:missing", + }); + + expect(result).toEqual({ + ok: false, + status: "forbidden", + error: "Session not visible from this sandboxed agent session: agent:main:missing", + displayKey: "agent:main:missing", + }); + }); + + it("propagates explicit-key gateway failures", async () => { + callGatewayMock.mockRejectedValueOnce(new Error("gateway unavailable")); + + const resolvedSession = await resolveSessionReference({ + sessionKey: "agent:main:worker", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:main", + restrictToSpawned: false, + }); + if (!resolvedSession.ok) { + throw new Error("Expected session reference"); + } + const result = await resolveVisibleSessionReference({ + action: "send", + resolvedSession, + requesterSessionKey: "agent:main:main", + restrictToSpawned: false, + visibilitySessionKey: "agent:main:worker", + }); + + expect(result).toEqual({ + ok: false, + status: "error", + error: "gateway unavailable", + displayKey: "agent:main:worker", + }); + }); + + it("reports an allowed missing explicit key for deliberate bootstrap", async () => { + callGatewayMock.mockResolvedValueOnce({}); + + const resolvedSession = await resolveSessionReference({ + sessionKey: "agent:main:main", + alias: "main", + mainKey: "main", + requesterInternalKey: "agent:main:dashboard:requester", + restrictToSpawned: false, + }); + if (!resolvedSession.ok) { + throw new Error("Expected session reference"); + } + const result = await resolveVisibleSessionReference({ + action: "send", + resolvedSession, + requesterSessionKey: "agent:main:dashboard:requester", + restrictToSpawned: false, + visibilitySessionKey: "agent:main:main", + allowMissingKey: true, + }); + + expect(result).toEqual({ + ok: true, + key: "agent:main:main", + displayKey: "agent:main:main", + missing: true, + }); + expect(callGatewayMock).toHaveBeenCalledWith({ + method: "sessions.resolve", + params: { + key: "agent:main:main", + spawnedBy: undefined, + allowMissing: true, + }, + }); + }); }); diff --git a/src/agents/tools/sessions-resolution.ts b/src/agents/tools/sessions-resolution.ts index be6ee23b114d..13c668c781f7 100644 --- a/src/agents/tools/sessions-resolution.ts +++ b/src/agents/tools/sessions-resolution.ts @@ -9,7 +9,6 @@ import { normalizeGatewayClientId, } from "../../../packages/gateway-protocol/src/client-info.js"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; -import { GatewayClientRequestError } from "../../gateway/client.js"; import { formatErrorMessage } from "../../infra/errors.js"; import { createSessionVisibilityChecker, @@ -164,10 +163,11 @@ type VisibleSessionReferenceResolution = ok: true; key: string; displayKey: string; + missing?: true; } | { ok: false; - status: "forbidden"; + status: "error" | "forbidden"; error: string; displayKey: string; }; @@ -190,36 +190,35 @@ function buildResolvedSessionReference(params: { }; } +function buildFailedSessionReference( + error: unknown, + raw: string, + restrictToSpawned: boolean, +): Extract { + return restrictToSpawned + ? { + ok: false, + status: "forbidden", + error: `Session not visible from this sandboxed agent session: ${raw}`, + } + : { + ok: false, + status: "error", + error: + formatErrorMessage(error) || + `Session not found: ${raw} (use the full sessionKey from sessions_list)`, + }; +} + async function requestResolvedSessionKey( params: Record & { allowMissing?: boolean }, callGateway: GatewayCaller, ): Promise { - try { - const result = await callGateway<{ key?: unknown }>({ - method: "sessions.resolve", - params, - }); - return normalizeOptionalString(result?.key); - } catch (error) { - const olderGatewayRejectedProbe = - params.allowMissing === true && - error instanceof GatewayClientRequestError && - error.gatewayCode === "INVALID_REQUEST" && - error.message.includes("invalid sessions.resolve params") && - error.message.includes("unexpected property 'allowMissing'"); - if (!olderGatewayRejectedProbe) { - throw error; - } - // Protocol v4 gateways predating allowMissing reject the additive field. - // Retry without it for mixed-version correctness; remove at the next protocol break. - const legacyParams: Record = { ...params }; - delete legacyParams.allowMissing; - const result = await callGateway<{ key?: unknown }>({ - method: "sessions.resolve", - params: legacyParams, - }); - return normalizeOptionalString(result?.key); - } + const result = await callGateway<{ key?: unknown }>({ + method: "sessions.resolve", + params, + }); + return normalizeOptionalString(result?.key); } function buildSessionResolveQuery(params: { @@ -310,20 +309,7 @@ export async function resolveSessionReference(params: { } return buildReference(key, true); } catch (error) { - if (params.restrictToSpawned) { - return { - ok: false, - status: "forbidden", - error: `Session not visible from this sandboxed agent session: ${raw}`, - }; - } - return { - ok: false, - status: "error", - error: - formatErrorMessage(error) || - `Session not found: ${raw} (use the full sessionKey from sessions_list)`, - }; + return buildFailedSessionReference(error, raw, params.restrictToSpawned); } } @@ -333,12 +319,7 @@ export async function resolveSessionReference(params: { mainKey: params.mainKey, requesterInternalKey: params.requesterInternalKey, }); - const displayKey = resolveDisplaySessionKey({ - key: resolvedKey, - alias: params.alias, - mainKey: params.mainKey, - }); - return { ok: true, key: resolvedKey, displayKey, resolvedViaSessionId: false }; + return buildReference(resolvedKey, false); } export async function resolveVisibleSessionReference(params: { @@ -347,10 +328,13 @@ export async function resolveVisibleSessionReference(params: { requesterSessionKey: string; restrictToSpawned: boolean; visibilitySessionKey: string; + allowMissingKey?: boolean; + concealResolutionError?: string; callGateway?: GatewayCaller; }): Promise { - const resolvedKey = params.resolvedSession.key; - const displayKey = params.resolvedSession.displayKey; + let resolvedKey = params.resolvedSession.key; + let displayKey = params.resolvedSession.displayKey; + let missing = false; // Cross-session tools persist their results into the caller transcript; an // incognito target must remain unreachable even from an incognito requester. if (isIncognitoSessionKey(resolvedKey)) { @@ -361,6 +345,57 @@ export async function resolveVisibleSessionReference(params: { displayKey, }; } + const input = params.visibilitySessionKey.trim(); + const isExplicitKey = + !params.resolvedSession.resolvedViaSessionId && + input !== "current" && + input !== "main" && + input !== "global" && + input !== "unknown" && + !shouldResolveSessionIdInput(input); + if (isExplicitKey && (params.action === "history" || params.action === "send")) { + try { + const key = await requestResolvedSessionKey( + buildSessionResolveQuery({ + input: resolvedKey, + kind: "key", + requesterInternalKey: params.requesterSessionKey, + restrictToSpawned: params.restrictToSpawned, + allowMissing: params.allowMissingKey, + }), + params.callGateway ?? callAgentToolGatewayRequest, + ); + if (key) { + resolvedKey = key; + displayKey = key; + } else if (params.allowMissingKey) { + missing = true; + } + } catch (error) { + if (params.concealResolutionError && !params.restrictToSpawned) { + return { + ok: false, + status: "forbidden", + error: params.concealResolutionError, + displayKey, + }; + } + const failed = buildFailedSessionReference( + error, + params.visibilitySessionKey, + params.restrictToSpawned, + ); + return { ...failed, displayKey }; + } + } + if (isIncognitoSessionKey(resolvedKey)) { + return { + ok: false, + status: "forbidden", + error: `Session not visible from session tools: ${params.visibilitySessionKey}`, + displayKey, + }; + } const shouldVerifySpawnedVisibility = params.restrictToSpawned && !params.resolvedSession.resolvedViaSessionId && @@ -389,5 +424,5 @@ export async function resolveVisibleSessionReference(params: { displayKey, }; } - return { ok: true, key: resolvedKey, displayKey }; + return { ok: true, key: resolvedKey, displayKey, ...(missing ? { missing: true } : {}) }; } diff --git a/src/agents/tools/sessions-send-tool.ts b/src/agents/tools/sessions-send-tool.ts index 1c5f6ee367f1..3496f90104aa 100644 --- a/src/agents/tools/sessions-send-tool.ts +++ b/src/agents/tools/sessions-send-tool.ts @@ -69,6 +69,7 @@ import { import { runWithScopedSessionAccess } from "./scoped-session-access.js"; import { createSessionVisibilityGuard, + createSessionVisibilityRowChecker, createAgentToAgentPolicy, resolveEffectiveSessionToolsVisibility, resolveSessionReference, @@ -205,57 +206,39 @@ function isConfiguredAgentMainSessionKey(params: { ); } -async function ensureConfiguredAgentMainSession(params: { +async function createConfiguredAgentMainSession(params: { cfg: OpenClawConfig; callGateway: GatewayCaller; sessionKey: string; - mainKey: string; requesterSessionKey?: string; useTrustedInProcessCreation: boolean; }): Promise<{ ok: true } | { ok: false; error: string }> { - if ( - !isConfiguredAgentMainSessionKey({ - cfg: params.cfg, - sessionKey: params.sessionKey, - mainKey: params.mainKey, - }) - ) { - return { ok: true }; - } - try { - await params.callGateway({ - method: "sessions.resolve", - params: { key: params.sessionKey }, - timeoutMs: 10_000, - }); - return { ok: true }; - } catch { - try { - const createParams = { - key: params.sessionKey, - agentId: resolveAgentIdFromSessionKey(params.sessionKey, resolveDefaultAgentId(params.cfg)), - }; - if ( - params.useTrustedInProcessCreation && - params.requesterSessionKey && - hasInProcessGatewayToolContext() - ) { - await callInProcessGatewayToolWithCreation("sessions.create", createParams, { - via: "internal", - actor: { type: "agent", id: params.requesterSessionKey }, - }); - } else { - await params.callGateway({ - method: "sessions.create", - params: createParams, - timeoutMs: 10_000, - }); - } - return { ok: true }; - } catch (err) { - return { ok: false, error: formatErrorMessage(err) }; + const createParams = { + key: params.sessionKey, + agentId: resolveAgentIdFromSessionKey(params.sessionKey, resolveDefaultAgentId(params.cfg)), + }; + if ( + params.useTrustedInProcessCreation && + params.requesterSessionKey && + hasInProcessGatewayToolContext() + ) { + // sessions.create serializes keyed creation and adopts an existing row, + // so concurrent first sends can safely race after the missing resolution. + await callInProcessGatewayToolWithCreation("sessions.create", createParams, { + via: "internal", + actor: { type: "agent", id: params.requesterSessionKey }, + }); + } else { + await params.callGateway({ + method: "sessions.create", + params: createParams, + timeoutMs: 10_000, + }); } + return { ok: true }; + } catch (err) { + return { ok: false, error: formatErrorMessage(err) }; } } @@ -591,6 +574,11 @@ export function createSessionsSendTool(opts?: { error: "Either sessionKey or label is required", }); } + const allowMissingKey = isConfiguredAgentMainSessionKey({ + cfg, + sessionKey, + mainKey, + }); const resolvedSession = await resolveSessionReference({ sessionKey, alias, @@ -606,12 +594,21 @@ export function createSessionsSendTool(opts?: { error: resolvedSession.error, }); } + const resolutionAccess = createSessionVisibilityRowChecker({ + action: "send", + defaultAgentId: resolveDefaultAgentId(cfg), + requesterSessionKey: effectiveRequesterKey, + visibility: sessionVisibility, + a2aPolicy, + }).check({ key: resolvedSession.key }); const visibleSession = await resolveVisibleSessionReference({ action: "send", resolvedSession, requesterSessionKey: effectiveRequesterKey, restrictToSpawned, visibilitySessionKey: sessionKey, + allowMissingKey, + concealResolutionError: resolutionAccess.allowed ? undefined : resolutionAccess.error, callGateway: gatewayCall, }); const unresolvedDisplayKey = sessionKey; @@ -772,21 +769,22 @@ export function createSessionsSendTool(opts?: { ...(opts?.signal ? { signal: opts.signal } : {}), targetSessionKey: resolvedKey, run: async () => { - const ensuredSession = await ensureConfiguredAgentMainSession({ - cfg, - callGateway: gatewayCall, - sessionKey: resolvedKey, - mainKey, - requesterSessionKey, - useTrustedInProcessCreation: opts?.callGateway === undefined, - }); - if (!ensuredSession.ok) { - return jsonResult({ - runId: crypto.randomUUID(), - status: "error", - error: ensuredSession.error, - sessionKey: displayKey, + if (visibleSession.missing) { + const createdSession = await createConfiguredAgentMainSession({ + cfg, + callGateway: gatewayCall, + sessionKey: resolvedKey, + requesterSessionKey, + useTrustedInProcessCreation: opts?.callGateway === undefined, }); + if (!createdSession.ok) { + return jsonResult({ + runId: crypto.randomUUID(), + status: "error", + error: createdSession.error, + sessionKey: displayKey, + }); + } } const requesterChannel = opts?.agentChannel; diff --git a/src/agents/tools/sessions.test.ts b/src/agents/tools/sessions.test.ts index e6b84993bb77..a108c9e6a9bd 100644 --- a/src/agents/tools/sessions.test.ts +++ b/src/agents/tools/sessions.test.ts @@ -838,6 +838,31 @@ describe("sessions_send gating", () => { expect(requireGatewayRequest().method).toBe("sessions.resolve"); }); + it("conceals missing explicit keys denied by session visibility", async () => { + callGatewayMock.mockRejectedValueOnce(new Error("No session found: agent:main:missing")); + const tool = createSessionsSendTool({ + agentSessionKey: MAIN_AGENT_SESSION_KEY, + callGateway: callGatewayMock, + config: { + session: { scope: "per-sender", mainKey: "main" }, + tools: { + agentToAgent: { enabled: false }, + sessions: { visibility: "self" }, + }, + } as never, + }); + + const result = await tool.execute("call-hidden-missing-key", { + sessionKey: "agent:main:missing", + message: "hi", + timeoutSeconds: 0, + }); + + expect(requireDetails(result).status).toBe("forbidden"); + expect(callGatewayMock).toHaveBeenCalledTimes(1); + expect(requireGatewayRequest().method).toBe("sessions.resolve"); + }); + it("prefers sessionKey over a redundant label", async () => { const tool = createMainSessionsSendTool(); @@ -989,8 +1014,9 @@ describe("sessions_send gating", () => { timeoutSeconds: 0, }); - expect(callGatewayMock).toHaveBeenCalledTimes(1); - expect(requireGatewayRequest().method).toBe("sessions.list"); + expect(callGatewayMock).toHaveBeenCalledTimes(2); + expect(requireGatewayRequest().method).toBe("sessions.resolve"); + expect(requireGatewayRequest(1).method).toBe("sessions.list"); expect(requireDetails(result).status).toBe("forbidden"); }); @@ -1017,7 +1043,8 @@ describe("sessions_send gating", () => { expect((result.details as { error?: string } | undefined)?.error ?? "").toContain( "cannot target a thread session", ); - expect(callGatewayMock).not.toHaveBeenCalled(); + expect(callGatewayMock).toHaveBeenCalledTimes(1); + expect(requireGatewayRequest().method).toBe("sessions.resolve"); }); it("rejects Telegram topic session targets before dispatching an agent run", async () => { @@ -1056,7 +1083,8 @@ describe("sessions_send gating", () => { expect((result.details as { error?: string } | undefined)?.error ?? "").toContain( "cannot target a thread session", ); - expect(callGatewayMock).not.toHaveBeenCalled(); + expect(callGatewayMock).toHaveBeenCalledTimes(1); + expect(requireGatewayRequest().method).toBe("sessions.resolve"); }); it("rejects label targets that resolve to canonical thread sessions", async () => { @@ -1083,8 +1111,9 @@ describe("sessions_send gating", () => { expect((result.details as { error?: string } | undefined)?.error ?? "").toContain( "cannot target a thread session", ); - expect(callGatewayMock).toHaveBeenCalledTimes(1); + expect(callGatewayMock).toHaveBeenCalledTimes(2); expect(requireGatewayRequest().method).toBe("sessions.resolve"); + expect(requireGatewayRequest(1).method).toBe("sessions.resolve"); }); it("does not disclose a resolved thread session key from a sessionId target", async () => { @@ -1653,8 +1682,7 @@ describe("sessions_send agent-main materialization provenance", () => { callGatewayMock.mockImplementation(async (opts: unknown) => { const request = opts as { method?: string }; if (request.method === "sessions.resolve") { - // Unmaterialized agent main: the probe fails, forcing creation. - throw new Error("unknown session: agent:main:main"); + return {}; } if (request.method === "sessions.create") { throw new Error("plain sessions.create must not be used for trusted materialization"); diff --git a/src/gateway/server.sessions-send.test.ts b/src/gateway/server.sessions-send.test.ts index 54b4b2429af3..cf12a8f209fe 100644 --- a/src/gateway/server.sessions-send.test.ts +++ b/src/gateway/server.sessions-send.test.ts @@ -3,7 +3,18 @@ import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; -import { afterAll, beforeAll, beforeEach, describe, expect, it, vi, type Mock } from "vitest"; +import { + afterAll, + afterEach, + beforeAll, + beforeEach, + describe, + expect, + it, + vi, + type Mock, +} from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; import { testing as agentStepTesting } from "../agents/tools/agent-step.test-support.js"; import { runSessionsSendA2AFlow } from "../agents/tools/sessions-send-tool.a2a.js"; import { @@ -33,6 +44,7 @@ let server: Awaited>; let gatewayPort: number; const gatewayToken = "test-gateway-token-1234567890"; let envSnapshot: ReturnType; +const tempDirs = useAutoCleanupTempDirTracker(afterEach); type SessionSendTool = ReturnType[number]; const SESSION_SEND_E2E_TIMEOUT_MS = 10_000; @@ -153,6 +165,48 @@ afterAll(async () => { }); describe("sessions_send gateway loopback", () => { + it("rejects a missing explicit key without creating or running a session", async () => { + const dir = tempDirs.make("openclaw-sessions-send-missing-"); + const missingKey = "agent:main:missing"; + const spy = agentCommandMock as unknown as Mock<(opts: unknown) => Promise>; + testState.sessionStorePath = path.join(dir, "sessions.json"); + try { + await writeSessionStore({ + entries: { + main: { + sessionId: "sess-main", + updatedAt: Date.now(), + }, + }, + }); + spy.mockClear(); + const tool = createOpenClawTools({ + agentSessionKey: "agent:main:main", + config: { tools: { sessions: { visibility: "all" } } }, + }).find((candidate) => candidate.name === "sessions_send"); + if (!tool) { + throw new Error("missing sessions_send tool"); + } + + const result = await tool.execute("call-missing-key", { + sessionKey: missingKey, + message: "ping", + timeoutSeconds: 0, + }); + + expect(result.details).toMatchObject({ + status: "error", + error: `No session found: ${missingKey}`, + }); + expect(spy).not.toHaveBeenCalled(); + expect( + loadSessionEntry({ sessionKey: missingKey, storePath: testState.sessionStorePath }), + ).toBe(undefined); + } finally { + testState.sessionStorePath = undefined; + } + }); + it("returns reply when lifecycle ends before agent.wait", async () => { const spy = agentCommandMock as unknown as Mock<(opts: unknown) => Promise>; spy.mockImplementation(async (opts: unknown) => diff --git a/src/gateway/server.sessions.preview-resolve.test.ts b/src/gateway/server.sessions.preview-resolve.test.ts index 2645da186ff6..266ab67f67d6 100644 --- a/src/gateway/server.sessions.preview-resolve.test.ts +++ b/src/gateway/server.sessions.preview-resolve.test.ts @@ -114,6 +114,18 @@ test("sessions.resolve can probe a missing selector without returning an RPC err expect(resolved.payload).toEqual({ ok: false }); }); +test("sessions.resolve rejects a missing key by default", async () => { + await createSessionStoreDir(); + const { ws } = await openClient(); + + const resolved = await rpcReq(ws, "sessions.resolve", { + key: "agent:main:missing", + }); + + expect(resolved.ok).toBe(false); + expect(resolved.error?.message).toBe("No session found: agent:main:missing"); +}); + test("sessions.resolve returns short-id ambiguity as a protocol-success result", async () => { await createSessionStoreDir(); await writeSessionStore({