Sessions: consolidate path hardening and fallback resilience (#24657)

* Changelog: credit session path fixes

* Sessions: harden path resolution for symlink and stale metadata

* Tests: cover fallback for invalid absolute sessionFile

* Tests: add symlink alias session path coverage

* Tests: guard symlink escape in sessionFile resolution
This commit is contained in:
Vincent Koc
2026-02-23 12:36:01 -05:00
committed by GitHub
parent ce1f12ff33
commit 6a0fcf6518
4 changed files with 103 additions and 24 deletions

View File

@@ -75,6 +75,8 @@ Docs: https://docs.openclaw.ai
### Fixes ### Fixes
- Sessions/Resilience: ignore invalid persisted `sessionFile` metadata and fall back to the derived safe transcript path instead of aborting session resolution for handlers and tooling. (#16061) Thanks @haoyifan and @vincentkoc.
- Sessions/Paths: resolve symlinked state-dir aliases during transcript-path validation while preserving safe cross-agent/state-root compatibility for valid `agents/<id>/sessions/**` paths. (#18593) Thanks @EpaL and @vincentkoc.
- Agents/Compaction: count auto-compactions only after a non-retry `auto_compaction_end`, keeping session `compactionCount` aligned to completed compactions. - Agents/Compaction: count auto-compactions only after a non-retry `auto_compaction_end`, keeping session `compactionCount` aligned to completed compactions.
- Security/CLI: redact sensitive values in `openclaw config get` output before printing config paths, preventing credential leakage to terminal output/history. (#13683) Thanks @SleuthCo. - Security/CLI: redact sensitive values in `openclaw config get` output before printing config paths, preventing credential leakage to terminal output/history. (#13683) Thanks @SleuthCo.
- Agents/Moonshot: force `supportsDeveloperRole=false` for Moonshot-compatible `openai-completions` models (provider `moonshot` and Moonshot base URLs), so initial runs no longer send unsupported `developer` roles that trigger `ROLE_UNSPECIFIED` errors. (#21060, #22194) Thanks @ShengFuC. - Agents/Moonshot: force `supportsDeveloperRole=false` for Moonshot-compatible `openai-completions` models (provider `moonshot` and Moonshot base URLs), so initial runs no longer send unsupported `developer` roles that trigger `ROLE_UNSPECIFIED` errors. (#21060, #22194) Thanks @ShengFuC.

View File

@@ -561,15 +561,22 @@ describe("sessions", () => {
}); });
}); });
it("rejects absolute sessionFile paths outside agent sessions directories", () => { it("falls back to derived transcript path when sessionFile is outside agent sessions directories", () => {
withStateDir(path.resolve("/home/user/.openclaw"), () => { withStateDir(path.resolve("/home/user/.openclaw"), () => {
expect(() => const sessionFile = resolveSessionFilePath(
resolveSessionFilePath( "sess-1",
"sess-1", { sessionFile: path.resolve("/etc/passwd") },
{ sessionFile: path.resolve("/etc/passwd") }, { agentId: "bot1" },
{ agentId: "bot1" }, );
expect(sessionFile).toBe(
path.join(
path.resolve("/home/user/.openclaw"),
"agents",
"bot1",
"sessions",
"sess-1.jsonl",
), ),
).toThrow(/within sessions directory/); );
}); });
}); });

View File

@@ -1,3 +1,4 @@
import fs from "node:fs";
import os from "node:os"; import os from "node:os";
import path from "node:path"; import path from "node:path";
import { expandHomePrefix, resolveRequiredHomeDir } from "../../infra/home-dir.js"; import { expandHomePrefix, resolveRequiredHomeDir } from "../../infra/home-dir.js";
@@ -76,8 +77,10 @@ function resolvePathFromAgentSessionsDir(
agentSessionsDir: string, agentSessionsDir: string,
candidateAbsPath: string, candidateAbsPath: string,
): string | undefined { ): string | undefined {
const agentBase = path.resolve(agentSessionsDir); const agentBase =
const relative = path.relative(agentBase, candidateAbsPath); safeRealpathSync(path.resolve(agentSessionsDir)) ?? path.resolve(agentSessionsDir);
const realCandidate = safeRealpathSync(candidateAbsPath) ?? candidateAbsPath;
const relative = path.relative(agentBase, realCandidate);
if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) { if (!relative || relative.startsWith("..") || path.isAbsolute(relative)) {
return undefined; return undefined;
} }
@@ -112,6 +115,14 @@ function extractAgentIdFromAbsoluteSessionPath(candidateAbsPath: string): string
return agentId || undefined; return agentId || undefined;
} }
function safeRealpathSync(filePath: string): string | undefined {
try {
return fs.realpathSync(filePath);
} catch {
return undefined;
}
}
function resolvePathWithinSessionsDir( function resolvePathWithinSessionsDir(
sessionsDir: string, sessionsDir: string,
candidate: string, candidate: string,
@@ -122,21 +133,28 @@ function resolvePathWithinSessionsDir(
throw new Error("Session file path must not be empty"); throw new Error("Session file path must not be empty");
} }
const resolvedBase = path.resolve(sessionsDir); const resolvedBase = path.resolve(sessionsDir);
const realBase = safeRealpathSync(resolvedBase) ?? resolvedBase;
// Normalize absolute paths that are within the sessions directory. // Normalize absolute paths that are within the sessions directory.
// Older versions stored absolute sessionFile paths in sessions.json; // Older versions stored absolute sessionFile paths in sessions.json;
// convert them to relative so the containment check passes. // convert them to relative so the containment check passes.
const normalized = path.isAbsolute(trimmed) ? path.relative(resolvedBase, trimmed) : trimmed; const realTrimmed = path.isAbsolute(trimmed) ? (safeRealpathSync(trimmed) ?? trimmed) : trimmed;
if (normalized.startsWith("..") && path.isAbsolute(trimmed)) { const normalized = path.isAbsolute(realTrimmed)
? path.relative(realBase, realTrimmed)
: realTrimmed;
if (normalized.startsWith("..") && path.isAbsolute(realTrimmed)) {
const tryAgentFallback = (agentId: string): string | undefined => { const tryAgentFallback = (agentId: string): string | undefined => {
const normalizedAgentId = normalizeAgentId(agentId); const normalizedAgentId = normalizeAgentId(agentId);
const siblingSessionsDir = resolveSiblingAgentSessionsDir(resolvedBase, normalizedAgentId); const siblingSessionsDir = resolveSiblingAgentSessionsDir(realBase, normalizedAgentId);
if (siblingSessionsDir) { if (siblingSessionsDir) {
const siblingResolved = resolvePathFromAgentSessionsDir(siblingSessionsDir, trimmed); const siblingResolved = resolvePathFromAgentSessionsDir(siblingSessionsDir, realTrimmed);
if (siblingResolved) { if (siblingResolved) {
return siblingResolved; return siblingResolved;
} }
} }
return resolvePathFromAgentSessionsDir(resolveAgentSessionsDir(normalizedAgentId), trimmed); return resolvePathFromAgentSessionsDir(
resolveAgentSessionsDir(normalizedAgentId),
realTrimmed,
);
}; };
const explicitAgentId = opts?.agentId?.trim(); const explicitAgentId = opts?.agentId?.trim();
@@ -146,7 +164,7 @@ function resolvePathWithinSessionsDir(
return resolvedFromAgent; return resolvedFromAgent;
} }
} }
const extractedAgentId = extractAgentIdFromAbsoluteSessionPath(trimmed); const extractedAgentId = extractAgentIdFromAbsoluteSessionPath(realTrimmed);
if (extractedAgentId) { if (extractedAgentId) {
const resolvedFromPath = tryAgentFallback(extractedAgentId); const resolvedFromPath = tryAgentFallback(extractedAgentId);
if (resolvedFromPath) { if (resolvedFromPath) {
@@ -156,13 +174,13 @@ function resolvePathWithinSessionsDir(
// Accept it even if the root directory differs from the current env // Accept it even if the root directory differs from the current env
// (e.g., OPENCLAW_STATE_DIR changed between session creation and resolution). // (e.g., OPENCLAW_STATE_DIR changed between session creation and resolution).
// The structural pattern provides sufficient containment guarantees. // The structural pattern provides sufficient containment guarantees.
return path.resolve(trimmed); return path.resolve(realTrimmed);
} }
} }
if (!normalized || normalized.startsWith("..") || path.isAbsolute(normalized)) { if (!normalized || normalized.startsWith("..") || path.isAbsolute(normalized)) {
throw new Error("Session file path must be within sessions directory"); throw new Error("Session file path must be within sessions directory");
} }
return path.resolve(resolvedBase, normalized); return path.resolve(realBase, normalized);
} }
export function resolveSessionTranscriptPathInDir( export function resolveSessionTranscriptPathInDir(
@@ -200,7 +218,11 @@ export function resolveSessionFilePath(
const sessionsDir = resolveSessionsDir(opts); const sessionsDir = resolveSessionsDir(opts);
const candidate = entry?.sessionFile?.trim(); const candidate = entry?.sessionFile?.trim();
if (candidate) { if (candidate) {
return resolvePathWithinSessionsDir(sessionsDir, candidate, { agentId: opts?.agentId }); try {
return resolvePathWithinSessionsDir(sessionsDir, candidate, { agentId: opts?.agentId });
} catch {
// Keep handlers alive when persisted metadata is stale/corrupt.
}
} }
return resolveSessionTranscriptPathInDir(sessionId, sessionsDir); return resolveSessionTranscriptPathInDir(sessionId, sessionsDir);
} }

View File

@@ -56,16 +56,64 @@ describe("session path safety", () => {
expect(resolved).toBe(path.resolve(sessionsDir, "sess-1-topic-topic%2Fa%2Bb.jsonl")); expect(resolved).toBe(path.resolve(sessionsDir, "sess-1-topic-topic%2Fa%2Bb.jsonl"));
}); });
it("rejects absolute sessionFile paths outside known agent sessions dirs", () => { it("falls back to derived path when sessionFile is outside known agent sessions dirs", () => {
const sessionsDir = "/tmp/openclaw/agents/main/sessions"; const sessionsDir = "/tmp/openclaw/agents/main/sessions";
expect(() => const resolved = resolveSessionFilePath(
resolveSessionFilePath( "sess-1",
{ sessionFile: "/tmp/openclaw/agents/work/not-sessions/abc-123.jsonl" },
{ sessionsDir },
);
expect(resolved).toBe(path.resolve(sessionsDir, "sess-1.jsonl"));
});
it("accepts symlink-alias session paths that resolve under the sessions dir", () => {
if (process.platform === "win32") {
return;
}
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-symlink-session-"));
const realRoot = path.join(tmpDir, "real-state");
const aliasRoot = path.join(tmpDir, "alias-state");
try {
const sessionsDir = path.join(realRoot, "agents", "main", "sessions");
fs.mkdirSync(sessionsDir, { recursive: true });
fs.symlinkSync(realRoot, aliasRoot, "dir");
const viaAlias = path.join(aliasRoot, "agents", "main", "sessions", "sess-1.jsonl");
fs.writeFileSync(path.join(sessionsDir, "sess-1.jsonl"), "");
const resolved = resolveSessionFilePath("sess-1", { sessionFile: viaAlias }, { sessionsDir });
expect(fs.realpathSync(resolved)).toBe(
fs.realpathSync(path.join(sessionsDir, "sess-1.jsonl")),
);
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it("falls back when sessionFile is a symlink that escapes sessions dir", () => {
if (process.platform === "win32") {
return;
}
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-symlink-escape-"));
const sessionsDir = path.join(tmpDir, "agents", "main", "sessions");
const outsideDir = path.join(tmpDir, "outside");
try {
fs.mkdirSync(sessionsDir, { recursive: true });
fs.mkdirSync(outsideDir, { recursive: true });
const outsideFile = path.join(outsideDir, "escaped.jsonl");
fs.writeFileSync(outsideFile, "");
const symlinkPath = path.join(sessionsDir, "escaped.jsonl");
fs.symlinkSync(outsideFile, symlinkPath, "file");
const resolved = resolveSessionFilePath(
"sess-1", "sess-1",
{ sessionFile: "/tmp/openclaw/agents/work/not-sessions/abc-123.jsonl" }, { sessionFile: symlinkPath },
{ sessionsDir }, { sessionsDir },
), );
).toThrow(/within sessions directory/); expect(fs.realpathSync(path.dirname(resolved))).toBe(fs.realpathSync(sessionsDir));
expect(path.basename(resolved)).toBe("sess-1.jsonl");
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
}); });
}); });