mirror of
https://github.com/openclaw/openclaw.git
synced 2026-02-19 18:39:20 -05:00
fix(sessions): resolve transcript paths with explicit agent context (#16288)
Merged via /review-pr -> /prepare-pr -> /merge-pr.
Prepared head SHA: 7cbe9deca9
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
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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",
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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" },
|
||||
|
||||
Reference in New Issue
Block a user