fix(security): harden browser trace/download temp path handling

This commit is contained in:
Peter Steinberger
2026-02-26 01:01:17 +01:00
parent e56b0cf1a0
commit 496a76c03b
8 changed files with 322 additions and 40 deletions

View File

@@ -18,6 +18,7 @@ Docs: https://docs.openclaw.ai
- Security/macOS beta onboarding: remove Anthropic OAuth sign-in and the legacy `oauth.json` onboarding path that exposed the PKCE verifier via OAuth `state`; this impacted the macOS beta onboarding path only. Anthropic subscription auth is now setup-token-only and will ship in the next npm release (`2026.2.25`). Thanks @zdi-disclosures for reporting.
- Security/Nextcloud Talk: drop replayed signed webhook events with persistent per-account replay dedupe across restarts, and reject unexpected webhook backend origins when account base URL is configured. Thanks @aristorechina for reporting.
- Security/Gateway: harden `agents.files` path handling to block out-of-workspace symlink targets for `agents.files.get`/`agents.files.set`, keep in-workspace symlink targets supported, and add gateway regression coverage for both blocked escapes and allowed in-workspace symlinks. Thanks @tdjackey for reporting.
- Security/Browser temp paths: harden trace/download output-path handling against symlink-root and symlink-parent escapes with realpath-based write-path checks plus secure fallback tmp-dir validation that fails closed on unsafe fallback links. This ships in the next npm release (`2026.2.25`). Thanks @tdjackey for reporting.
- Gateway/Message media roots: thread `agentId` through gateway `send` RPC and prefer explicit `agentId` over session/default resolution so non-default agent workspace media sends no longer fail with `LocalMediaAccessError`; added regression coverage for agent precedence and blank-agent fallback. (#23249) Thanks @Sid-Qin.
- Cron/Model overrides: when isolated `payload.model` is no longer allowlisted, fall back to default model selection instead of failing the job, while still returning explicit errors for invalid model strings. (#26717) Thanks @Youyou972.
- Security/Gateway auth: require pairing for operator device-identity sessions authenticated with shared token auth so unpaired devices cannot self-assign operator scopes. Thanks @tdjackey for reporting.

View File

@@ -7,6 +7,7 @@ import {
resolvePathsWithinRoot,
resolvePathWithinRoot,
resolveStrictExistingPathsWithinRoot,
resolveWritablePathWithinRoot,
} from "./paths.js";
async function createFixtureRoot(): Promise<{ baseDir: string; uploadsDir: string }> {
@@ -245,6 +246,45 @@ describe("resolvePathWithinRoot", () => {
});
});
describe("resolveWritablePathWithinRoot", () => {
it("accepts a writable path under root when parent is a real directory", async () => {
await withFixtureRoot(async ({ uploadsDir }) => {
const result = await resolveWritablePathWithinRoot({
rootDir: uploadsDir,
requestedPath: "safe.txt",
scopeLabel: "uploads directory",
});
expect(result).toEqual({
ok: true,
path: path.resolve(uploadsDir, "safe.txt"),
});
});
});
it.runIf(process.platform !== "win32")(
"rejects write paths routed through a symlinked parent directory",
async () => {
await withFixtureRoot(async ({ baseDir, uploadsDir }) => {
const outsideDir = path.join(baseDir, "outside");
await fs.mkdir(outsideDir, { recursive: true });
const symlinkDir = path.join(uploadsDir, "escape-link");
await fs.symlink(outsideDir, symlinkDir);
const result = await resolveWritablePathWithinRoot({
rootDir: uploadsDir,
requestedPath: "escape-link/pwned.txt",
scopeLabel: "uploads directory",
});
expect(result.ok).toBe(false);
if (!result.ok) {
expect(result.error).toContain("must stay within uploads directory");
}
});
},
);
});
describe("resolvePathsWithinRoot", () => {
it("resolves all valid in-root paths", () => {
const result = resolvePathsWithinRoot({

View File

@@ -1,6 +1,7 @@
import fs from "node:fs/promises";
import path from "node:path";
import { SafeOpenError, openFileWithinRoot } from "../infra/fs-safe.js";
import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js";
import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js";
export const DEFAULT_BROWSER_TMP_DIR = resolvePreferredOpenClawTmpDir();
@@ -30,6 +31,67 @@ export function resolvePathWithinRoot(params: {
return { ok: true, path: resolved };
}
export async function resolveWritablePathWithinRoot(params: {
rootDir: string;
requestedPath: string;
scopeLabel: string;
defaultFileName?: string;
}): Promise<{ ok: true; path: string } | { ok: false; error: string }> {
const lexical = resolvePathWithinRoot(params);
if (!lexical.ok) {
return lexical;
}
const invalid = (): { ok: false; error: string } => ({
ok: false,
error: `Invalid path: must stay within ${params.scopeLabel}`,
});
const rootDir = path.resolve(params.rootDir);
let rootRealPath: string;
try {
const rootLstat = await fs.lstat(rootDir);
if (!rootLstat.isDirectory() || rootLstat.isSymbolicLink()) {
return invalid();
}
rootRealPath = await fs.realpath(rootDir);
} catch {
return invalid();
}
const requestedPath = lexical.path;
const parentDir = path.dirname(requestedPath);
try {
const parentLstat = await fs.lstat(parentDir);
if (!parentLstat.isDirectory() || parentLstat.isSymbolicLink()) {
return invalid();
}
const parentRealPath = await fs.realpath(parentDir);
if (!isPathInside(rootRealPath, parentRealPath)) {
return invalid();
}
} catch {
return invalid();
}
try {
const targetLstat = await fs.lstat(requestedPath);
if (targetLstat.isSymbolicLink() || !targetLstat.isFile()) {
return invalid();
}
const targetRealPath = await fs.realpath(requestedPath);
if (!isPathInside(rootRealPath, targetRealPath)) {
return invalid();
}
} catch (err) {
if (!isNotFoundPathError(err)) {
return invalid();
}
}
return lexical;
}
export function resolvePathsWithinRoot(params: {
rootDir: string;
requestedPaths: string[];

View File

@@ -1,3 +1,4 @@
import fs from "node:fs/promises";
import type { BrowserFormField } from "../client-actions-core.js";
import type { BrowserRouteContext } from "../server-context.js";
import {
@@ -15,14 +16,17 @@ import {
import {
DEFAULT_DOWNLOAD_DIR,
DEFAULT_UPLOAD_DIR,
resolvePathWithinRoot,
resolveWritablePathWithinRoot,
resolveExistingPathsWithinRoot,
} from "./path-output.js";
import type { BrowserResponse, BrowserRouteRegistrar } from "./types.js";
import { jsonError, toBoolean, toNumber, toStringArray, toStringOrEmpty } from "./utils.js";
function resolveDownloadPathOrRespond(res: BrowserResponse, requestedPath: string): string | null {
const downloadPathResult = resolvePathWithinRoot({
async function resolveDownloadPathOrRespond(
res: BrowserResponse,
requestedPath: string,
): Promise<string | null> {
const downloadPathResult = await resolveWritablePathWithinRoot({
rootDir: DEFAULT_DOWNLOAD_DIR,
requestedPath,
scopeLabel: "downloads directory",
@@ -466,9 +470,10 @@ export function registerBrowserAgentActRoutes(
targetId,
feature: "wait for download",
run: async ({ cdpUrl, tab, pw }) => {
await fs.mkdir(DEFAULT_DOWNLOAD_DIR, { recursive: true });
let downloadPath: string | undefined;
if (out.trim()) {
const resolvedDownloadPath = resolveDownloadPathOrRespond(res, out);
const resolvedDownloadPath = await resolveDownloadPathOrRespond(res, out);
if (!resolvedDownloadPath) {
return;
}
@@ -504,7 +509,8 @@ export function registerBrowserAgentActRoutes(
targetId,
feature: "download",
run: async ({ cdpUrl, tab, pw }) => {
const downloadPath = resolveDownloadPathOrRespond(res, out);
await fs.mkdir(DEFAULT_DOWNLOAD_DIR, { recursive: true });
const downloadPath = await resolveDownloadPathOrRespond(res, out);
if (!downloadPath) {
return;
}

View File

@@ -8,7 +8,7 @@ import {
resolveTargetIdFromQuery,
withPlaywrightRouteContext,
} from "./agent.shared.js";
import { DEFAULT_TRACE_DIR, resolvePathWithinRoot } from "./path-output.js";
import { DEFAULT_TRACE_DIR, resolveWritablePathWithinRoot } from "./path-output.js";
import type { BrowserRouteRegistrar } from "./types.js";
import { toBoolean, toStringOrEmpty } from "./utils.js";
@@ -122,7 +122,7 @@ export function registerBrowserAgentDebugRoutes(
const id = crypto.randomUUID();
const dir = DEFAULT_TRACE_DIR;
await fs.mkdir(dir, { recursive: true });
const tracePathResult = resolvePathWithinRoot({
const tracePathResult = await resolveWritablePathWithinRoot({
rootDir: dir,
requestedPath: out,
scopeLabel: "trace directory",

View File

@@ -1,7 +1,9 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { fetch as realFetch } from "undici";
import { describe, expect, it } from "vitest";
import { DEFAULT_UPLOAD_DIR } from "./paths.js";
import { DEFAULT_DOWNLOAD_DIR, DEFAULT_TRACE_DIR, DEFAULT_UPLOAD_DIR } from "./paths.js";
import {
installAgentContractHooks,
postJson,
@@ -16,6 +18,23 @@ import {
const state = getBrowserControlServerTestState();
const pwMocks = getPwMocks();
async function withSymlinkPathEscape<T>(params: {
rootDir: string;
run: (relativePath: string) => Promise<T>;
}): Promise<T> {
const outsideDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-route-escape-"));
const linkName = `escape-link-${Date.now()}-${Math.random().toString(16).slice(2)}`;
const linkPath = path.join(params.rootDir, linkName);
await fs.mkdir(params.rootDir, { recursive: true });
await fs.symlink(outsideDir, linkPath);
try {
return await params.run(`${linkName}/pwned.zip`);
} finally {
await fs.unlink(linkPath).catch(() => {});
await fs.rm(outsideDir, { recursive: true, force: true }).catch(() => {});
}
}
describe("browser control server", () => {
installAgentContractHooks();
@@ -268,6 +287,58 @@ describe("browser control server", () => {
expect(pwMocks.downloadViaPlaywright).not.toHaveBeenCalled();
});
it.runIf(process.platform !== "win32")(
"trace stop rejects symlinked write path escape under trace dir",
async () => {
const base = await startServerAndBase();
await withSymlinkPathEscape({
rootDir: DEFAULT_TRACE_DIR,
run: async (pathEscape) => {
const res = await postJson<{ error?: string }>(`${base}/trace/stop`, {
path: pathEscape,
});
expect(res.error).toContain("Invalid path");
expect(pwMocks.traceStopViaPlaywright).not.toHaveBeenCalled();
},
});
},
);
it.runIf(process.platform !== "win32")(
"wait/download rejects symlinked write path escape under downloads dir",
async () => {
const base = await startServerAndBase();
await withSymlinkPathEscape({
rootDir: DEFAULT_DOWNLOAD_DIR,
run: async (pathEscape) => {
const res = await postJson<{ error?: string }>(`${base}/wait/download`, {
path: pathEscape,
});
expect(res.error).toContain("Invalid path");
expect(pwMocks.waitForDownloadViaPlaywright).not.toHaveBeenCalled();
},
});
},
);
it.runIf(process.platform !== "win32")(
"download rejects symlinked write path escape under downloads dir",
async () => {
const base = await startServerAndBase();
await withSymlinkPathEscape({
rootDir: DEFAULT_DOWNLOAD_DIR,
run: async (pathEscape) => {
const res = await postJson<{ error?: string }>(`${base}/download`, {
ref: "e12",
path: pathEscape,
});
expect(res.error).toContain("Invalid path");
expect(pwMocks.downloadViaPlaywright).not.toHaveBeenCalled();
},
});
},
);
it("wait/download accepts in-root relative output path", async () => {
const base = await startServerAndBase();
const res = await postJson<{ ok?: boolean; download?: { path?: string } }>(

View File

@@ -8,24 +8,54 @@ function fallbackTmp(uid = 501) {
return path.join("/var/fallback", `openclaw-${uid}`);
}
function nodeErrorWithCode(code: string) {
const err = new Error(code) as Error & { code?: string };
err.code = code;
return err;
}
function secureDirStat(uid = 501) {
return {
isDirectory: () => true,
isSymbolicLink: () => false,
uid,
mode: 0o40700,
};
}
function resolveWithMocks(params: {
lstatSync: NonNullable<TmpDirOptions["lstatSync"]>;
fallbackLstatSync?: NonNullable<TmpDirOptions["lstatSync"]>;
accessSync?: NonNullable<TmpDirOptions["accessSync"]>;
uid?: number;
tmpdirPath?: string;
}) {
const uid = params.uid ?? 501;
const fallbackPath = fallbackTmp(uid);
const accessSync = params.accessSync ?? vi.fn();
const wrappedLstatSync = vi.fn((target: string) => {
if (target === POSIX_OPENCLAW_TMP_DIR) {
return params.lstatSync(target);
}
if (target === fallbackPath) {
if (params.fallbackLstatSync) {
return params.fallbackLstatSync(target);
}
return secureDirStat(uid);
}
return secureDirStat(uid);
}) as NonNullable<TmpDirOptions["lstatSync"]>;
const mkdirSync = vi.fn();
const getuid = vi.fn(() => params.uid ?? 501);
const getuid = vi.fn(() => uid);
const tmpdir = vi.fn(() => params.tmpdirPath ?? "/var/fallback");
const resolved = resolvePreferredOpenClawTmpDir({
accessSync,
lstatSync: params.lstatSync,
lstatSync: wrappedLstatSync,
mkdirSync,
getuid,
tmpdir,
});
return { resolved, accessSync, lstatSync: params.lstatSync, mkdirSync, tmpdir };
return { resolved, accessSync, lstatSync: wrappedLstatSync, mkdirSync, tmpdir };
}
describe("resolvePreferredOpenClawTmpDir", () => {
@@ -45,24 +75,12 @@ describe("resolvePreferredOpenClawTmpDir", () => {
});
it("prefers /tmp/openclaw when it does not exist but /tmp is writable", () => {
const lstatSyncMock = vi.fn<NonNullable<TmpDirOptions["lstatSync"]>>(() => {
const err = new Error("missing") as Error & { code?: string };
err.code = "ENOENT";
throw err;
});
// second lstat call (after mkdir) should succeed
lstatSyncMock.mockImplementationOnce(() => {
const err = new Error("missing") as Error & { code?: string };
err.code = "ENOENT";
throw err;
});
lstatSyncMock.mockImplementationOnce(() => ({
isDirectory: () => true,
isSymbolicLink: () => false,
uid: 501,
mode: 0o40700,
}));
const lstatSyncMock = vi
.fn<NonNullable<TmpDirOptions["lstatSync"]>>()
.mockImplementationOnce(() => {
throw nodeErrorWithCode("ENOENT");
})
.mockImplementationOnce(() => secureDirStat(501));
const { resolved, accessSync, mkdirSync, tmpdir } = resolveWithMocks({
lstatSync: lstatSyncMock,
@@ -84,7 +102,7 @@ describe("resolvePreferredOpenClawTmpDir", () => {
const { resolved, tmpdir } = resolveWithMocks({ lstatSync });
expect(resolved).toBe(fallbackTmp());
expect(tmpdir).toHaveBeenCalledTimes(1);
expect(tmpdir).toHaveBeenCalled();
});
it("falls back to os.tmpdir()/openclaw when /tmp is not writable", () => {
@@ -94,9 +112,7 @@ describe("resolvePreferredOpenClawTmpDir", () => {
}
});
const lstatSync = vi.fn(() => {
const err = new Error("missing") as Error & { code?: string };
err.code = "ENOENT";
throw err;
throw nodeErrorWithCode("ENOENT");
});
const { resolved, tmpdir } = resolveWithMocks({
accessSync,
@@ -104,7 +120,7 @@ describe("resolvePreferredOpenClawTmpDir", () => {
});
expect(resolved).toBe(fallbackTmp());
expect(tmpdir).toHaveBeenCalledTimes(1);
expect(tmpdir).toHaveBeenCalled();
});
it("falls back when /tmp/openclaw is a symlink", () => {
@@ -118,7 +134,7 @@ describe("resolvePreferredOpenClawTmpDir", () => {
const { resolved, tmpdir } = resolveWithMocks({ lstatSync });
expect(resolved).toBe(fallbackTmp());
expect(tmpdir).toHaveBeenCalledTimes(1);
expect(tmpdir).toHaveBeenCalled();
});
it("falls back when /tmp/openclaw is not owned by the current user", () => {
@@ -132,7 +148,7 @@ describe("resolvePreferredOpenClawTmpDir", () => {
const { resolved, tmpdir } = resolveWithMocks({ lstatSync });
expect(resolved).toBe(fallbackTmp());
expect(tmpdir).toHaveBeenCalledTimes(1);
expect(tmpdir).toHaveBeenCalled();
});
it("falls back when /tmp/openclaw is group/other writable", () => {
@@ -145,6 +161,51 @@ describe("resolvePreferredOpenClawTmpDir", () => {
const { resolved, tmpdir } = resolveWithMocks({ lstatSync });
expect(resolved).toBe(fallbackTmp());
expect(tmpdir).toHaveBeenCalledTimes(1);
expect(tmpdir).toHaveBeenCalled();
});
it("throws when fallback path is a symlink", () => {
const lstatSync = vi.fn(() => ({
isDirectory: () => true,
isSymbolicLink: () => true,
uid: 501,
mode: 0o120777,
}));
const fallbackLstatSync = vi.fn(() => ({
isDirectory: () => true,
isSymbolicLink: () => true,
uid: 501,
mode: 0o120777,
}));
expect(() =>
resolveWithMocks({
lstatSync,
fallbackLstatSync,
}),
).toThrow(/Unsafe fallback OpenClaw temp dir/);
});
it("creates fallback directory when missing, then validates ownership and mode", () => {
const lstatSync = vi.fn(() => ({
isDirectory: () => true,
isSymbolicLink: () => true,
uid: 501,
mode: 0o120777,
}));
const fallbackLstatSync = vi
.fn<NonNullable<TmpDirOptions["lstatSync"]>>()
.mockImplementationOnce(() => {
throw nodeErrorWithCode("ENOENT");
})
.mockImplementationOnce(() => secureDirStat(501));
const { resolved, mkdirSync } = resolveWithMocks({
lstatSync,
fallbackLstatSync,
});
expect(resolved).toBe(fallbackTmp());
expect(mkdirSync).toHaveBeenCalledWith(fallbackTmp(), { recursive: true, mode: 0o700 });
});
});

View File

@@ -95,12 +95,53 @@ export function resolvePreferredOpenClawTmpDir(
}
};
const resolveFallbackState = (
fallbackPath: string,
requireWritableAccess: boolean,
): "available" | "missing" | "invalid" => {
try {
const candidate = lstatSync(fallbackPath);
if (!isTrustedPreferredDir(candidate)) {
return "invalid";
}
if (requireWritableAccess) {
accessSync(fallbackPath, fs.constants.W_OK | fs.constants.X_OK);
}
return "available";
} catch (err) {
if (isNodeErrorWithCode(err, "ENOENT")) {
return "missing";
}
return "invalid";
}
};
const ensureTrustedFallbackDir = (): string => {
const fallbackPath = fallback();
const state = resolveFallbackState(fallbackPath, true);
if (state === "available") {
return fallbackPath;
}
if (state === "invalid") {
throw new Error(`Unsafe fallback OpenClaw temp dir: ${fallbackPath}`);
}
try {
mkdirSync(fallbackPath, { recursive: true, mode: 0o700 });
} catch {
throw new Error(`Unable to create fallback OpenClaw temp dir: ${fallbackPath}`);
}
if (resolveFallbackState(fallbackPath, true) !== "available") {
throw new Error(`Unsafe fallback OpenClaw temp dir: ${fallbackPath}`);
}
return fallbackPath;
};
const existingPreferredState = resolvePreferredState(true);
if (existingPreferredState === "available") {
return POSIX_OPENCLAW_TMP_DIR;
}
if (existingPreferredState === "invalid") {
return fallback();
return ensureTrustedFallbackDir();
}
try {
@@ -108,10 +149,10 @@ export function resolvePreferredOpenClawTmpDir(
// Create with a safe default; subsequent callers expect it exists.
mkdirSync(POSIX_OPENCLAW_TMP_DIR, { recursive: true, mode: 0o700 });
if (resolvePreferredState(true) !== "available") {
return fallback();
return ensureTrustedFallbackDir();
}
return POSIX_OPENCLAW_TMP_DIR;
} catch {
return fallback();
return ensureTrustedFallbackDir();
}
}