From cab0abf52ac91e12ea7a0cf04fff315cf0c94d64 Mon Sep 17 00:00:00 2001 From: Robby Date: Sat, 14 Feb 2026 19:44:51 +0100 Subject: [PATCH] fix(sessions): resolve transcript paths with explicit agent context (#16288) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: 7cbe9deca9b7fc9efa5d2320acb058bc9fbea48c Co-authored-by: robbyczgw-cla <239660374+robbyczgw-cla@users.noreply.github.com> Co-authored-by: gumadeiras <5599352+gumadeiras@users.noreply.github.com> Reviewed-by: @gumadeiras --- CHANGELOG.md | 1 + src/agents/subagent-announce.ts | 2 + src/agents/tools/sessions-list-tool.ts | 5 +- src/config/sessions/paths.test.ts | 72 ++++++++++++++++++++++- src/config/sessions/paths.ts | 81 ++++++++++++++++++++++++-- src/config/sessions/transcript.ts | 1 + src/gateway/server-methods/chat.ts | 11 +++- src/gateway/session-utils.fs.test.ts | 68 +++++++++++++++++++++ src/gateway/session-utils.fs.ts | 4 +- src/gateway/session-utils.test.ts | 19 ++++++ 10 files changed, 252 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 52cdc1022b..cea0af080f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Sessions/Agents: harden transcript path resolution for mismatched agent context by preserving explicit store roots and adding safe absolute-path fallback to the correct agent sessions directory. (#16288) Thanks @robbyczgw-cla. - BlueBubbles: include sender identity in group chat envelopes and pass clean message text to the agent prompt, aligning with iMessage/Signal formatting. (#16210) Thanks @zerone0x. - WhatsApp: honor per-account `dmPolicy` overrides (account-level settings now take precedence over channel defaults for inbound DMs). (#10082) Thanks @mcaxtr. - Security/Node Host: enforce `system.run` rawCommand/argv consistency to prevent allowlist/approval bypass. Thanks @christos-eth. diff --git a/src/agents/subagent-announce.ts b/src/agents/subagent-announce.ts index a3093c7b90..98bf961052 100644 --- a/src/agents/subagent-announce.ts +++ b/src/agents/subagent-announce.ts @@ -230,10 +230,12 @@ async function buildSubagentStatsLine(params: { }); const sessionId = entry?.sessionId; + const agentId = resolveAgentIdFromSessionKey(params.sessionKey); let transcriptPath: string | undefined; if (sessionId && storePath) { try { transcriptPath = resolveSessionFilePath(sessionId, entry, { + agentId, sessionsDir: path.dirname(storePath), }); } catch { diff --git a/src/agents/tools/sessions-list-tool.ts b/src/agents/tools/sessions-list-tool.ts index e98be654f9..70fc97e074 100644 --- a/src/agents/tools/sessions-list-tool.ts +++ b/src/agents/tools/sessions-list-tool.ts @@ -161,7 +161,10 @@ export function createSessionsListTool(opts?: { transcriptPath = resolveSessionFilePath( sessionId, sessionFile ? { sessionFile } : undefined, - { sessionsDir: path.dirname(storePath) }, + { + agentId: resolveAgentIdFromSessionKey(key), + sessionsDir: path.dirname(storePath), + }, ); } catch { transcriptPath = undefined; diff --git a/src/config/sessions/paths.test.ts b/src/config/sessions/paths.test.ts index baa45079bf..443b7791b8 100644 --- a/src/config/sessions/paths.test.ts +++ b/src/config/sessions/paths.test.ts @@ -96,30 +96,96 @@ describe("session path safety", () => { expect(resolved).toBe(path.resolve(sessionsDir, "abc-123-topic-42.jsonl")); }); - it("rejects absolute sessionFile paths outside the sessions dir", () => { + it("rejects absolute sessionFile paths outside known agent sessions dirs", () => { const sessionsDir = "/tmp/openclaw/agents/main/sessions"; expect(() => resolveSessionFilePath( "sess-1", - { sessionFile: "/tmp/openclaw/agents/work/sessions/abc-123.jsonl" }, + { sessionFile: "/tmp/openclaw/agents/work/not-sessions/abc-123.jsonl" }, { sessionsDir }, ), ).toThrow(/within sessions directory/); }); + it("uses explicit agentId fallback for absolute sessionFile outside sessionsDir", () => { + const mainSessionsDir = path.dirname(resolveStorePath(undefined, { agentId: "main" })); + const opsSessionsDir = path.dirname(resolveStorePath(undefined, { agentId: "ops" })); + const opsSessionFile = path.join(opsSessionsDir, "abc-123.jsonl"); + + const resolved = resolveSessionFilePath( + "sess-1", + { sessionFile: opsSessionFile }, + { sessionsDir: mainSessionsDir, agentId: "ops" }, + ); + + expect(resolved).toBe(path.resolve(opsSessionFile)); + }); + + it("uses absolute path fallback when sessionFile includes a different agent dir", () => { + const mainSessionsDir = path.dirname(resolveStorePath(undefined, { agentId: "main" })); + const opsSessionsDir = path.dirname(resolveStorePath(undefined, { agentId: "ops" })); + const opsSessionFile = path.join(opsSessionsDir, "abc-123.jsonl"); + + const resolved = resolveSessionFilePath( + "sess-1", + { sessionFile: opsSessionFile }, + { sessionsDir: mainSessionsDir }, + ); + + expect(resolved).toBe(path.resolve(opsSessionFile)); + }); + + it("uses sibling fallback for custom per-agent store roots", () => { + const mainSessionsDir = "/srv/custom/agents/main/sessions"; + const opsSessionFile = "/srv/custom/agents/ops/sessions/abc-123.jsonl"; + + const resolved = resolveSessionFilePath( + "sess-1", + { sessionFile: opsSessionFile }, + { sessionsDir: mainSessionsDir, agentId: "ops" }, + ); + + expect(resolved).toBe(path.resolve(opsSessionFile)); + }); + + it("uses extracted agent fallback for custom per-agent store roots", () => { + const mainSessionsDir = "/srv/custom/agents/main/sessions"; + const opsSessionFile = "/srv/custom/agents/ops/sessions/abc-123.jsonl"; + + const resolved = resolveSessionFilePath( + "sess-1", + { sessionFile: opsSessionFile }, + { sessionsDir: mainSessionsDir }, + ); + + expect(resolved).toBe(path.resolve(opsSessionFile)); + }); + it("uses agent sessions dir fallback for transcript path", () => { const resolved = resolveSessionTranscriptPath("sess-1", "main"); expect(resolved.endsWith(path.join("agents", "main", "sessions", "sess-1.jsonl"))).toBe(true); }); - it("prefers storePath when resolving session file options", () => { + it("keeps storePath and agentId when resolving session file options", () => { const opts = resolveSessionFilePathOptions({ storePath: "/tmp/custom/agent-store/sessions.json", agentId: "ops", }); expect(opts).toEqual({ sessionsDir: path.resolve("/tmp/custom/agent-store"), + agentId: "ops", + }); + }); + + it("keeps custom per-agent store roots when agentId is provided", () => { + const opts = resolveSessionFilePathOptions({ + storePath: "/srv/custom/agents/ops/sessions/sessions.json", + agentId: "ops", + }); + expect(opts).toEqual({ + sessionsDir: path.resolve("/srv/custom/agents/ops/sessions"), + agentId: "ops", }); }); diff --git a/src/config/sessions/paths.ts b/src/config/sessions/paths.ts index a630f68c2f..9d5ea3b06b 100644 --- a/src/config/sessions/paths.ts +++ b/src/config/sessions/paths.ts @@ -42,11 +42,12 @@ export function resolveSessionFilePathOptions(params: { agentId?: string; storePath?: string; }): SessionFilePathOptions | undefined { + const agentId = params.agentId?.trim(); const storePath = params.storePath?.trim(); if (storePath) { - return { sessionsDir: path.dirname(path.resolve(storePath)) }; + const sessionsDir = path.dirname(path.resolve(storePath)); + return agentId ? { sessionsDir, agentId } : { sessionsDir }; } - const agentId = params.agentId?.trim(); if (agentId) { return { agentId }; } @@ -71,7 +72,51 @@ function resolveSessionsDir(opts?: SessionFilePathOptions): string { return resolveAgentSessionsDir(opts?.agentId); } -function resolvePathWithinSessionsDir(sessionsDir: string, candidate: string): string { +function resolvePathFromAgentSessionsDir( + agentSessionsDir: string, + candidateAbsPath: string, +): string | undefined { + const agentBase = path.resolve(agentSessionsDir); + const relative = path.relative(agentBase, candidateAbsPath); + if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) { + return undefined; + } + return path.resolve(agentBase, relative); +} + +function resolveSiblingAgentSessionsDir( + baseSessionsDir: string, + agentId: string, +): string | undefined { + const resolvedBase = path.resolve(baseSessionsDir); + if (path.basename(resolvedBase) !== "sessions") { + return undefined; + } + const baseAgentDir = path.dirname(resolvedBase); + const baseAgentsDir = path.dirname(baseAgentDir); + if (path.basename(baseAgentsDir) !== "agents") { + return undefined; + } + const rootDir = path.dirname(baseAgentsDir); + return path.join(rootDir, "agents", normalizeAgentId(agentId), "sessions"); +} + +function extractAgentIdFromAbsoluteSessionPath(candidateAbsPath: string): string | undefined { + const normalized = path.normalize(path.resolve(candidateAbsPath)); + const parts = normalized.split(path.sep).filter(Boolean); + const sessionsIndex = parts.lastIndexOf("sessions"); + if (sessionsIndex < 2 || parts[sessionsIndex - 2] !== "agents") { + return undefined; + } + const agentId = parts[sessionsIndex - 1]; + return agentId || undefined; +} + +function resolvePathWithinSessionsDir( + sessionsDir: string, + candidate: string, + opts?: { agentId?: string }, +): string { const trimmed = candidate.trim(); if (!trimmed) { throw new Error("Session file path must not be empty"); @@ -81,6 +126,34 @@ function resolvePathWithinSessionsDir(sessionsDir: string, candidate: string): s // Older versions stored absolute sessionFile paths in sessions.json; // convert them to relative so the containment check passes. const normalized = path.isAbsolute(trimmed) ? path.relative(resolvedBase, trimmed) : trimmed; + if (normalized.startsWith("..") && path.isAbsolute(trimmed)) { + const tryAgentFallback = (agentId: string): string | undefined => { + const normalizedAgentId = normalizeAgentId(agentId); + const siblingSessionsDir = resolveSiblingAgentSessionsDir(resolvedBase, normalizedAgentId); + if (siblingSessionsDir) { + const siblingResolved = resolvePathFromAgentSessionsDir(siblingSessionsDir, trimmed); + if (siblingResolved) { + return siblingResolved; + } + } + return resolvePathFromAgentSessionsDir(resolveAgentSessionsDir(normalizedAgentId), trimmed); + }; + + const explicitAgentId = opts?.agentId?.trim(); + if (explicitAgentId) { + const resolvedFromAgent = tryAgentFallback(explicitAgentId); + if (resolvedFromAgent) { + return resolvedFromAgent; + } + } + const extractedAgentId = extractAgentIdFromAbsoluteSessionPath(trimmed); + if (extractedAgentId) { + const resolvedFromPath = tryAgentFallback(extractedAgentId); + if (resolvedFromPath) { + return resolvedFromPath; + } + } + } if (!normalized || normalized.startsWith("..") || path.isAbsolute(normalized)) { throw new Error("Session file path must be within sessions directory"); } @@ -122,7 +195,7 @@ export function resolveSessionFilePath( const sessionsDir = resolveSessionsDir(opts); const candidate = entry?.sessionFile?.trim(); if (candidate) { - return resolvePathWithinSessionsDir(sessionsDir, candidate); + return resolvePathWithinSessionsDir(sessionsDir, candidate, { agentId: opts?.agentId }); } return resolveSessionTranscriptPathInDir(sessionId, sessionsDir); } diff --git a/src/config/sessions/transcript.ts b/src/config/sessions/transcript.ts index dabed46c8a..659c089fce 100644 --- a/src/config/sessions/transcript.ts +++ b/src/config/sessions/transcript.ts @@ -106,6 +106,7 @@ export async function appendAssistantMessageToSessionTranscript(params: { let sessionFile: string; try { sessionFile = resolveSessionFilePath(entry.sessionId, entry, { + agentId: params.agentId, sessionsDir: path.dirname(storePath), }); } catch (err) { diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index 28ea99b60b..4966e15753 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -53,8 +53,9 @@ function resolveTranscriptPath(params: { sessionId: string; storePath: string | undefined; sessionFile?: string; + agentId?: string; }): string | null { - const { sessionId, storePath, sessionFile } = params; + const { sessionId, storePath, sessionFile, agentId } = params; if (!storePath && !sessionFile) { return null; } @@ -63,7 +64,7 @@ function resolveTranscriptPath(params: { return resolveSessionFilePath( sessionId, sessionFile ? { sessionFile } : undefined, - sessionsDir ? { sessionsDir } : undefined, + sessionsDir || agentId ? { sessionsDir, agentId } : undefined, ); } catch { return null; @@ -99,12 +100,14 @@ function appendAssistantTranscriptMessage(params: { sessionId: string; storePath: string | undefined; sessionFile?: string; + agentId?: string; createIfMissing?: boolean; }): TranscriptAppendResult { const transcriptPath = resolveTranscriptPath({ sessionId: params.sessionId, storePath: params.storePath, sessionFile: params.sessionFile, + agentId: params.agentId, }); if (!transcriptPath) { return { ok: false, error: "transcript path not resolved" }; @@ -572,6 +575,7 @@ export const chatHandlers: GatewayRequestHandlers = { sessionId, storePath: latestStorePath, sessionFile: latestEntry?.sessionFile, + agentId, createIfMissing: true, }); if (appended.ok) { @@ -666,7 +670,7 @@ export const chatHandlers: GatewayRequestHandlers = { // Load session to find transcript file const rawSessionKey = p.sessionKey; - const { storePath, entry } = loadSessionEntry(rawSessionKey); + const { cfg, storePath, entry } = loadSessionEntry(rawSessionKey); const sessionId = entry?.sessionId; if (!sessionId || !storePath) { respond(false, undefined, errorShape(ErrorCodes.INVALID_REQUEST, "session not found")); @@ -679,6 +683,7 @@ export const chatHandlers: GatewayRequestHandlers = { sessionId, storePath, sessionFile: entry?.sessionFile, + agentId: resolveSessionAgentId({ sessionKey: rawSessionKey, config: cfg }), createIfMissing: false, }); if (!appended.ok || !appended.messageId || !appended.message) { diff --git a/src/gateway/session-utils.fs.test.ts b/src/gateway/session-utils.fs.test.ts index 0e324f78d5..d7cf781f5f 100644 --- a/src/gateway/session-utils.fs.test.ts +++ b/src/gateway/session-utils.fs.test.ts @@ -475,6 +475,58 @@ describe("readSessionMessages", () => { expect(marker.__openclaw?.id).toBe("comp-1"); expect(typeof marker.timestamp).toBe("number"); }); + + test("reads cross-agent absolute sessionFile when storePath points to another agent dir", () => { + const sessionId = "cross-agent-default-root"; + const sessionFile = path.join(tmpDir, "agents", "ops", "sessions", `${sessionId}.jsonl`); + fs.mkdirSync(path.dirname(sessionFile), { recursive: true }); + fs.writeFileSync( + sessionFile, + [ + JSON.stringify({ type: "session", version: 1, id: sessionId }), + JSON.stringify({ message: { role: "user", content: "from-ops" } }), + ].join("\n"), + "utf-8", + ); + + const wrongStorePath = path.join(tmpDir, "agents", "main", "sessions", "sessions.json"); + const out = readSessionMessages(sessionId, wrongStorePath, sessionFile); + + expect(out).toEqual([{ role: "user", content: "from-ops" }]); + }); + + test("reads cross-agent absolute sessionFile for custom per-agent store roots", () => { + const sessionId = "cross-agent-custom-root"; + const sessionFile = path.join( + tmpDir, + "custom", + "agents", + "ops", + "sessions", + `${sessionId}.jsonl`, + ); + fs.mkdirSync(path.dirname(sessionFile), { recursive: true }); + fs.writeFileSync( + sessionFile, + [ + JSON.stringify({ type: "session", version: 1, id: sessionId }), + JSON.stringify({ message: { role: "assistant", content: "from-custom-ops" } }), + ].join("\n"), + "utf-8", + ); + + const wrongStorePath = path.join( + tmpDir, + "custom", + "agents", + "main", + "sessions", + "sessions.json", + ); + const out = readSessionMessages(sessionId, wrongStorePath, sessionFile); + + expect(out).toEqual([{ role: "assistant", content: "from-custom-ops" }]); + }); }); describe("readSessionPreviewItemsFromTranscript", () => { @@ -594,6 +646,22 @@ describe("resolveSessionTranscriptCandidates", () => { }); describe("resolveSessionTranscriptCandidates safety", () => { + test("keeps cross-agent absolute sessionFile when storePath agent context differs", () => { + const storePath = "/tmp/openclaw/agents/main/sessions/sessions.json"; + const sessionFile = "/tmp/openclaw/agents/ops/sessions/sess-safe.jsonl"; + const candidates = resolveSessionTranscriptCandidates("sess-safe", storePath, sessionFile); + + expect(candidates.map((value) => path.resolve(value))).toContain(path.resolve(sessionFile)); + }); + + test("keeps cross-agent absolute sessionFile for custom per-agent store roots", () => { + const storePath = "/srv/custom/agents/main/sessions/sessions.json"; + const sessionFile = "/srv/custom/agents/ops/sessions/sess-safe.jsonl"; + const candidates = resolveSessionTranscriptCandidates("sess-safe", storePath, sessionFile); + + expect(candidates.map((value) => path.resolve(value))).toContain(path.resolve(sessionFile)); + }); + test("drops unsafe session IDs instead of producing traversal paths", () => { const candidates = resolveSessionTranscriptCandidates( "../etc/passwd", diff --git a/src/gateway/session-utils.fs.ts b/src/gateway/session-utils.fs.ts index 032e3686a5..d9987e8f22 100644 --- a/src/gateway/session-utils.fs.ts +++ b/src/gateway/session-utils.fs.ts @@ -131,7 +131,9 @@ export function resolveSessionTranscriptCandidates( if (storePath) { const sessionsDir = path.dirname(storePath); if (sessionFile) { - pushCandidate(() => resolveSessionFilePath(sessionId, { sessionFile }, { sessionsDir })); + pushCandidate(() => + resolveSessionFilePath(sessionId, { sessionFile }, { sessionsDir, agentId }), + ); } pushCandidate(() => resolveSessionTranscriptPathInDir(sessionId, sessionsDir)); } else if (sessionFile) { diff --git a/src/gateway/session-utils.test.ts b/src/gateway/session-utils.test.ts index aa0d518712..e57ea027a3 100644 --- a/src/gateway/session-utils.test.ts +++ b/src/gateway/session-utils.test.ts @@ -70,6 +70,25 @@ describe("gateway session utils", () => { ); }); + test("resolveSessionStoreKey falls back to first list entry when no agent is marked default", () => { + const cfg = { + session: { mainKey: "main" }, + agents: { list: [{ id: "ops" }, { id: "review" }] }, + } as OpenClawConfig; + expect(resolveSessionStoreKey({ cfg, sessionKey: "main" })).toBe("agent:ops:main"); + expect(resolveSessionStoreKey({ cfg, sessionKey: "discord:group:123" })).toBe( + "agent:ops:discord:group:123", + ); + }); + + test("resolveSessionStoreKey falls back to main when agents.list is missing", () => { + const cfg = { + session: { mainKey: "work" }, + } as OpenClawConfig; + expect(resolveSessionStoreKey({ cfg, sessionKey: "main" })).toBe("agent:main:work"); + expect(resolveSessionStoreKey({ cfg, sessionKey: "thread-1" })).toBe("agent:main:thread-1"); + }); + test("resolveSessionStoreKey normalizes session key casing", () => { const cfg = { session: { mainKey: "main" },