mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 11:41:24 +00:00
fix: harden session transcript path resolution
This commit is contained in:
@@ -30,7 +30,7 @@ describe("gateway chat.inject transcript writes", () => {
|
||||
return {
|
||||
...original,
|
||||
loadSessionEntry: () => ({
|
||||
storePath: "/tmp/store.json",
|
||||
storePath: path.join(dir, "sessions.json"),
|
||||
entry: {
|
||||
sessionId: "sess-1",
|
||||
sessionFile: transcriptPath,
|
||||
|
||||
@@ -9,6 +9,7 @@ import { resolveAgentTimeoutMs } from "../../agents/timeout.js";
|
||||
import { dispatchInboundMessage } from "../../auto-reply/dispatch.js";
|
||||
import { createReplyDispatcher } from "../../auto-reply/reply/reply-dispatcher.js";
|
||||
import { createReplyPrefixOptions } from "../../channels/reply-prefix.js";
|
||||
import { resolveSessionFilePath } from "../../config/sessions.js";
|
||||
import { resolveSendPolicy } from "../../sessions/send-policy.js";
|
||||
import { INTERNAL_MESSAGE_CHANNEL } from "../../utils/message-channel.js";
|
||||
import {
|
||||
@@ -54,13 +55,19 @@ function resolveTranscriptPath(params: {
|
||||
sessionFile?: string;
|
||||
}): string | null {
|
||||
const { sessionId, storePath, sessionFile } = params;
|
||||
if (sessionFile) {
|
||||
return sessionFile;
|
||||
}
|
||||
if (!storePath) {
|
||||
if (!storePath && !sessionFile) {
|
||||
return null;
|
||||
}
|
||||
try {
|
||||
const sessionsDir = storePath ? path.dirname(storePath) : undefined;
|
||||
return resolveSessionFilePath(
|
||||
sessionId,
|
||||
sessionFile ? { sessionFile } : undefined,
|
||||
sessionsDir ? { sessionsDir } : undefined,
|
||||
);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
return path.join(path.dirname(storePath), `${sessionId}.jsonl`);
|
||||
}
|
||||
|
||||
function ensureTranscriptFile(params: { transcriptPath: string; sessionId: string }): {
|
||||
|
||||
@@ -107,40 +107,73 @@ describe("sessions.usage", () => {
|
||||
|
||||
it("resolves store entries by sessionId when queried via discovered agent-prefixed key", async () => {
|
||||
const storeKey = "agent:opus:slack:dm:u123";
|
||||
const tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-usage-test-"));
|
||||
const sessionFile = path.join(tempDir, "s-opus.jsonl");
|
||||
fs.writeFileSync(sessionFile, "", "utf-8");
|
||||
const stateDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-usage-test-"));
|
||||
const previousStateDir = process.env.OPENCLAW_STATE_DIR;
|
||||
process.env.OPENCLAW_STATE_DIR = stateDir;
|
||||
|
||||
try {
|
||||
const agentSessionsDir = path.join(stateDir, "agents", "opus", "sessions");
|
||||
fs.mkdirSync(agentSessionsDir, { recursive: true });
|
||||
const sessionFile = path.join(agentSessionsDir, "s-opus.jsonl");
|
||||
fs.writeFileSync(sessionFile, "", "utf-8");
|
||||
const respond = vi.fn();
|
||||
|
||||
// Swap the store mock for this test: the canonical key differs from the discovered key
|
||||
// but points at the same sessionId.
|
||||
vi.mocked(loadCombinedSessionStoreForGateway).mockReturnValue({
|
||||
storePath: "(multiple)",
|
||||
store: {
|
||||
[storeKey]: {
|
||||
sessionId: "s-opus",
|
||||
sessionFile: "s-opus.jsonl",
|
||||
label: "Named session",
|
||||
updatedAt: 999,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Query via discovered key: agent:<id>:<sessionId>
|
||||
await usageHandlers["sessions.usage"]({
|
||||
respond,
|
||||
params: {
|
||||
startDate: "2026-02-01",
|
||||
endDate: "2026-02-02",
|
||||
key: "agent:opus:s-opus",
|
||||
limit: 10,
|
||||
},
|
||||
} as unknown as Parameters<(typeof usageHandlers)["sessions.usage"]>[0]);
|
||||
|
||||
expect(respond).toHaveBeenCalledTimes(1);
|
||||
expect(respond.mock.calls[0]?.[0]).toBe(true);
|
||||
const result = respond.mock.calls[0]?.[1] as unknown as { sessions: Array<{ key: string }> };
|
||||
expect(result.sessions).toHaveLength(1);
|
||||
expect(result.sessions[0]?.key).toBe(storeKey);
|
||||
} finally {
|
||||
if (previousStateDir === undefined) {
|
||||
delete process.env.OPENCLAW_STATE_DIR;
|
||||
} else {
|
||||
process.env.OPENCLAW_STATE_DIR = previousStateDir;
|
||||
}
|
||||
fs.rmSync(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects traversal-style keys in specific session usage lookups", async () => {
|
||||
const respond = vi.fn();
|
||||
|
||||
// Swap the store mock for this test: the canonical key differs from the discovered key
|
||||
// but points at the same sessionId.
|
||||
vi.mocked(loadCombinedSessionStoreForGateway).mockReturnValue({
|
||||
storePath: "(multiple)",
|
||||
store: {
|
||||
[storeKey]: {
|
||||
sessionId: "s-opus",
|
||||
sessionFile,
|
||||
label: "Named session",
|
||||
updatedAt: 999,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
// Query via discovered key: agent:<id>:<sessionId>
|
||||
await usageHandlers["sessions.usage"]({
|
||||
respond,
|
||||
params: {
|
||||
startDate: "2026-02-01",
|
||||
endDate: "2026-02-02",
|
||||
key: "agent:opus:s-opus",
|
||||
key: "agent:opus:../../etc/passwd",
|
||||
limit: 10,
|
||||
},
|
||||
} as unknown as Parameters<(typeof usageHandlers)["sessions.usage"]>[0]);
|
||||
|
||||
expect(respond).toHaveBeenCalledTimes(1);
|
||||
expect(respond.mock.calls[0]?.[0]).toBe(true);
|
||||
const result = respond.mock.calls[0]?.[1] as unknown as { sessions: Array<{ key: string }> };
|
||||
expect(result.sessions).toHaveLength(1);
|
||||
expect(result.sessions[0]?.key).toBe(storeKey);
|
||||
expect(respond.mock.calls[0]?.[0]).toBe(false);
|
||||
const error = respond.mock.calls[0]?.[2] as { message?: string } | undefined;
|
||||
expect(error?.message).toContain("Invalid session reference");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import fs from "node:fs";
|
||||
import path from "node:path";
|
||||
import type { SessionEntry, SessionSystemPromptReport } from "../../config/sessions/types.js";
|
||||
import type {
|
||||
CostUsageSummary,
|
||||
@@ -291,7 +292,7 @@ export const usageHandlers: GatewayRequestHandlers = {
|
||||
const specificKey = typeof p.key === "string" ? p.key.trim() : null;
|
||||
|
||||
// Load session store for named sessions
|
||||
const { store } = loadCombinedSessionStoreForGateway(config);
|
||||
const { storePath, store } = loadCombinedSessionStoreForGateway(config);
|
||||
const now = Date.now();
|
||||
|
||||
// Merge discovered sessions with store entries
|
||||
@@ -331,9 +332,21 @@ export const usageHandlers: GatewayRequestHandlers = {
|
||||
const sessionId = storeEntry?.sessionId ?? keyRest;
|
||||
|
||||
// Resolve the session file path
|
||||
const sessionFile = resolveSessionFilePath(sessionId, storeEntry, {
|
||||
agentId: agentIdFromKey,
|
||||
});
|
||||
let sessionFile: string;
|
||||
try {
|
||||
const pathOpts =
|
||||
storePath && storePath !== "(multiple)"
|
||||
? { sessionsDir: path.dirname(storePath) }
|
||||
: { agentId: agentIdFromKey };
|
||||
sessionFile = resolveSessionFilePath(sessionId, storeEntry, pathOpts);
|
||||
} catch {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, `Invalid session reference: ${specificKey}`),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
const stats = fs.statSync(sessionFile);
|
||||
@@ -756,15 +769,25 @@ export const usageHandlers: GatewayRequestHandlers = {
|
||||
}
|
||||
|
||||
const config = loadConfig();
|
||||
const { entry } = loadSessionEntry(key);
|
||||
const { entry, storePath } = loadSessionEntry(key);
|
||||
|
||||
// For discovered sessions (not in store), try using key as sessionId directly
|
||||
const parsed = parseAgentSessionKey(key);
|
||||
const agentId = parsed?.agentId;
|
||||
const rawSessionId = parsed?.rest ?? key;
|
||||
const sessionId = entry?.sessionId ?? rawSessionId;
|
||||
const sessionFile =
|
||||
entry?.sessionFile ?? resolveSessionFilePath(rawSessionId, entry, { agentId });
|
||||
let sessionFile: string;
|
||||
try {
|
||||
const pathOpts = storePath ? { sessionsDir: path.dirname(storePath) } : { agentId };
|
||||
sessionFile = resolveSessionFilePath(sessionId, entry, pathOpts);
|
||||
} catch {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, `Invalid session key: ${key}`),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const timeseries = await loadSessionUsageTimeSeries({
|
||||
sessionId,
|
||||
@@ -798,15 +821,25 @@ export const usageHandlers: GatewayRequestHandlers = {
|
||||
: 200;
|
||||
|
||||
const config = loadConfig();
|
||||
const { entry } = loadSessionEntry(key);
|
||||
const { entry, storePath } = loadSessionEntry(key);
|
||||
|
||||
// For discovered sessions (not in store), try using key as sessionId directly
|
||||
const parsed = parseAgentSessionKey(key);
|
||||
const agentId = parsed?.agentId;
|
||||
const rawSessionId = parsed?.rest ?? key;
|
||||
const sessionId = entry?.sessionId ?? rawSessionId;
|
||||
const sessionFile =
|
||||
entry?.sessionFile ?? resolveSessionFilePath(rawSessionId, entry, { agentId });
|
||||
let sessionFile: string;
|
||||
try {
|
||||
const pathOpts = storePath ? { sessionsDir: path.dirname(storePath) } : { agentId };
|
||||
sessionFile = resolveSessionFilePath(sessionId, entry, pathOpts);
|
||||
} catch {
|
||||
respond(
|
||||
false,
|
||||
undefined,
|
||||
errorShape(ErrorCodes.INVALID_REQUEST, `Invalid session key: ${key}`),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
const { loadSessionLogs } = await import("../../infra/session-cost-usage.js");
|
||||
const logs = await loadSessionLogs({
|
||||
|
||||
@@ -507,3 +507,26 @@ describe("resolveSessionTranscriptCandidates", () => {
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveSessionTranscriptCandidates safety", () => {
|
||||
test("drops unsafe session IDs instead of producing traversal paths", () => {
|
||||
const candidates = resolveSessionTranscriptCandidates(
|
||||
"../etc/passwd",
|
||||
"/tmp/openclaw/agents/main/sessions/sessions.json",
|
||||
);
|
||||
|
||||
expect(candidates).toEqual([]);
|
||||
});
|
||||
|
||||
test("drops unsafe sessionFile candidates and keeps safe fallbacks", () => {
|
||||
const storePath = "/tmp/openclaw/agents/main/sessions/sessions.json";
|
||||
const candidates = resolveSessionTranscriptCandidates(
|
||||
"sess-safe",
|
||||
storePath,
|
||||
"../../etc/passwd",
|
||||
);
|
||||
|
||||
expect(candidates.some((value) => value.includes("etc/passwd"))).toBe(false);
|
||||
expect(candidates).toContain(path.join(path.dirname(storePath), "sess-safe.jsonl"));
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,7 +2,11 @@ import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { SessionPreviewItem } from "./session-utils.types.js";
|
||||
import { resolveSessionTranscriptPath } from "../config/sessions.js";
|
||||
import {
|
||||
resolveSessionFilePath,
|
||||
resolveSessionTranscriptPath,
|
||||
resolveSessionTranscriptPathInDir,
|
||||
} from "../config/sessions.js";
|
||||
import { resolveRequiredHomeDir } from "../infra/home-dir.js";
|
||||
import { extractToolCallNames, hasToolCall } from "../utils/transcript-tools.js";
|
||||
import { stripEnvelope } from "./chat-sanitize.js";
|
||||
@@ -61,19 +65,40 @@ export function resolveSessionTranscriptCandidates(
|
||||
agentId?: string,
|
||||
): string[] {
|
||||
const candidates: string[] = [];
|
||||
if (sessionFile) {
|
||||
candidates.push(sessionFile);
|
||||
}
|
||||
const pushCandidate = (resolve: () => string): void => {
|
||||
try {
|
||||
candidates.push(resolve());
|
||||
} catch {
|
||||
// Ignore invalid paths/IDs and keep scanning other safe candidates.
|
||||
}
|
||||
};
|
||||
|
||||
if (storePath) {
|
||||
const dir = path.dirname(storePath);
|
||||
candidates.push(path.join(dir, `${sessionId}.jsonl`));
|
||||
const sessionsDir = path.dirname(storePath);
|
||||
if (sessionFile) {
|
||||
pushCandidate(() => resolveSessionFilePath(sessionId, { sessionFile }, { sessionsDir }));
|
||||
}
|
||||
pushCandidate(() => resolveSessionTranscriptPathInDir(sessionId, sessionsDir));
|
||||
} else if (sessionFile) {
|
||||
if (agentId) {
|
||||
pushCandidate(() => resolveSessionFilePath(sessionId, { sessionFile }, { agentId }));
|
||||
} else {
|
||||
const trimmed = sessionFile.trim();
|
||||
if (trimmed) {
|
||||
candidates.push(path.resolve(trimmed));
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (agentId) {
|
||||
candidates.push(resolveSessionTranscriptPath(sessionId, agentId));
|
||||
pushCandidate(() => resolveSessionTranscriptPath(sessionId, agentId));
|
||||
}
|
||||
|
||||
const home = resolveRequiredHomeDir(process.env, os.homedir);
|
||||
candidates.push(path.join(home, ".openclaw", "sessions", `${sessionId}.jsonl`));
|
||||
return candidates;
|
||||
const legacyDir = path.join(home, ".openclaw", "sessions");
|
||||
pushCandidate(() => resolveSessionTranscriptPathInDir(sessionId, legacyDir));
|
||||
|
||||
return Array.from(new Set(candidates));
|
||||
}
|
||||
|
||||
export function archiveFileOnDisk(filePath: string, reason: string): string {
|
||||
|
||||
Reference in New Issue
Block a user