refactor(browser): split act route modules and dedupe path guards

This commit is contained in:
Peter Steinberger
2026-02-26 01:21:20 +01:00
parent 046feb6b0e
commit f41715a18f
7 changed files with 319 additions and 275 deletions

View File

@@ -9,6 +9,58 @@ export const DEFAULT_TRACE_DIR = DEFAULT_BROWSER_TMP_DIR;
export const DEFAULT_DOWNLOAD_DIR = path.join(DEFAULT_BROWSER_TMP_DIR, "downloads");
export const DEFAULT_UPLOAD_DIR = path.join(DEFAULT_BROWSER_TMP_DIR, "uploads");
type InvalidPathResult = { ok: false; error: string };
function invalidPath(scopeLabel: string): InvalidPathResult {
return {
ok: false,
error: `Invalid path: must stay within ${scopeLabel}`,
};
}
async function resolveRealPathIfExists(targetPath: string): Promise<string | undefined> {
try {
return await fs.realpath(targetPath);
} catch {
return undefined;
}
}
async function resolveTrustedRootRealPath(rootDir: string): Promise<string | undefined> {
try {
const rootLstat = await fs.lstat(rootDir);
if (!rootLstat.isDirectory() || rootLstat.isSymbolicLink()) {
return undefined;
}
return await fs.realpath(rootDir);
} catch {
return undefined;
}
}
async function validateCanonicalPathWithinRoot(params: {
rootRealPath: string;
candidatePath: string;
expect: "directory" | "file";
}): Promise<"ok" | "not-found" | "invalid"> {
try {
const candidateLstat = await fs.lstat(params.candidatePath);
if (candidateLstat.isSymbolicLink()) {
return "invalid";
}
if (params.expect === "directory" && !candidateLstat.isDirectory()) {
return "invalid";
}
if (params.expect === "file" && !candidateLstat.isFile()) {
return "invalid";
}
const candidateRealPath = await fs.realpath(params.candidatePath);
return isPathInside(params.rootRealPath, candidateRealPath) ? "ok" : "invalid";
} catch (err) {
return isNotFoundPathError(err) ? "not-found" : "invalid";
}
}
export function resolvePathWithinRoot(params: {
rootDir: string;
requestedPath: string;
@@ -42,51 +94,30 @@ export async function resolveWritablePathWithinRoot(params: {
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 rootRealPath = await resolveTrustedRootRealPath(rootDir);
if (!rootRealPath) {
return invalidPath(params.scopeLabel);
}
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();
const parentStatus = await validateCanonicalPathWithinRoot({
rootRealPath,
candidatePath: parentDir,
expect: "directory",
});
if (parentStatus !== "ok") {
return invalidPath(params.scopeLabel);
}
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();
}
const targetStatus = await validateCanonicalPathWithinRoot({
rootRealPath,
candidatePath: requestedPath,
expect: "file",
});
if (targetStatus === "invalid") {
return invalidPath(params.scopeLabel);
}
return lexical;
@@ -141,13 +172,8 @@ async function resolveCheckedPathsWithinRoot(params: {
allowMissingFallback: boolean;
}): Promise<{ ok: true; paths: string[] } | { ok: false; error: string }> {
const rootDir = path.resolve(params.rootDir);
let rootRealPath: string | undefined;
try {
rootRealPath = await fs.realpath(rootDir);
} catch {
// Keep historical behavior for missing roots and rely on openFileWithinRoot for final checks.
rootRealPath = undefined;
}
// Keep historical behavior for missing roots and rely on openFileWithinRoot for final checks.
const rootRealPath = await resolveRealPathIfExists(rootDir);
const isInRoot = (relativePath: string) =>
Boolean(relativePath) && !relativePath.startsWith("..") && !path.isAbsolute(relativePath);

View File

@@ -0,0 +1,97 @@
import type { BrowserRouteContext } from "../server-context.js";
import { readBody, resolveTargetIdFromBody, withPlaywrightRouteContext } from "./agent.shared.js";
import { ensureOutputRootDir, resolveWritableOutputPathOrRespond } from "./output-paths.js";
import { DEFAULT_DOWNLOAD_DIR } from "./path-output.js";
import type { BrowserRouteRegistrar } from "./types.js";
import { jsonError, toNumber, toStringOrEmpty } from "./utils.js";
function buildDownloadRequestBase(cdpUrl: string, targetId: string, timeoutMs: number | undefined) {
return {
cdpUrl,
targetId,
timeoutMs: timeoutMs ?? undefined,
};
}
export function registerBrowserAgentActDownloadRoutes(
app: BrowserRouteRegistrar,
ctx: BrowserRouteContext,
) {
app.post("/wait/download", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const out = toStringOrEmpty(body.path) || "";
const timeoutMs = toNumber(body.timeoutMs);
await withPlaywrightRouteContext({
req,
res,
ctx,
targetId,
feature: "wait for download",
run: async ({ cdpUrl, tab, pw }) => {
await ensureOutputRootDir(DEFAULT_DOWNLOAD_DIR);
let downloadPath: string | undefined;
if (out.trim()) {
const resolvedDownloadPath = await resolveWritableOutputPathOrRespond({
res,
rootDir: DEFAULT_DOWNLOAD_DIR,
requestedPath: out,
scopeLabel: "downloads directory",
});
if (!resolvedDownloadPath) {
return;
}
downloadPath = resolvedDownloadPath;
}
const requestBase = buildDownloadRequestBase(cdpUrl, tab.targetId, timeoutMs);
const result = await pw.waitForDownloadViaPlaywright({
...requestBase,
path: downloadPath,
});
res.json({ ok: true, targetId: tab.targetId, download: result });
},
});
});
app.post("/download", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const ref = toStringOrEmpty(body.ref);
const out = toStringOrEmpty(body.path);
const timeoutMs = toNumber(body.timeoutMs);
if (!ref) {
return jsonError(res, 400, "ref is required");
}
if (!out) {
return jsonError(res, 400, "path is required");
}
await withPlaywrightRouteContext({
req,
res,
ctx,
targetId,
feature: "download",
run: async ({ cdpUrl, tab, pw }) => {
await ensureOutputRootDir(DEFAULT_DOWNLOAD_DIR);
const downloadPath = await resolveWritableOutputPathOrRespond({
res,
rootDir: DEFAULT_DOWNLOAD_DIR,
requestedPath: out,
scopeLabel: "downloads directory",
});
if (!downloadPath) {
return;
}
const requestBase = buildDownloadRequestBase(cdpUrl, tab.targetId, timeoutMs);
const result = await pw.downloadViaPlaywright({
...requestBase,
ref,
path: downloadPath,
});
res.json({ ok: true, targetId: tab.targetId, download: result });
},
});
});
}

View File

@@ -0,0 +1,100 @@
import type { BrowserRouteContext } from "../server-context.js";
import { readBody, resolveTargetIdFromBody, withPlaywrightRouteContext } from "./agent.shared.js";
import { DEFAULT_UPLOAD_DIR, resolveExistingPathsWithinRoot } from "./path-output.js";
import type { BrowserRouteRegistrar } from "./types.js";
import { jsonError, toBoolean, toNumber, toStringArray, toStringOrEmpty } from "./utils.js";
export function registerBrowserAgentActHookRoutes(
app: BrowserRouteRegistrar,
ctx: BrowserRouteContext,
) {
app.post("/hooks/file-chooser", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const ref = toStringOrEmpty(body.ref) || undefined;
const inputRef = toStringOrEmpty(body.inputRef) || undefined;
const element = toStringOrEmpty(body.element) || undefined;
const paths = toStringArray(body.paths) ?? [];
const timeoutMs = toNumber(body.timeoutMs);
if (!paths.length) {
return jsonError(res, 400, "paths are required");
}
await withPlaywrightRouteContext({
req,
res,
ctx,
targetId,
feature: "file chooser hook",
run: async ({ cdpUrl, tab, pw }) => {
const uploadPathsResult = await resolveExistingPathsWithinRoot({
rootDir: DEFAULT_UPLOAD_DIR,
requestedPaths: paths,
scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`,
});
if (!uploadPathsResult.ok) {
res.status(400).json({ error: uploadPathsResult.error });
return;
}
const resolvedPaths = uploadPathsResult.paths;
if (inputRef || element) {
if (ref) {
return jsonError(res, 400, "ref cannot be combined with inputRef/element");
}
await pw.setInputFilesViaPlaywright({
cdpUrl,
targetId: tab.targetId,
inputRef,
element,
paths: resolvedPaths,
});
} else {
await pw.armFileUploadViaPlaywright({
cdpUrl,
targetId: tab.targetId,
paths: resolvedPaths,
timeoutMs: timeoutMs ?? undefined,
});
if (ref) {
await pw.clickViaPlaywright({
cdpUrl,
targetId: tab.targetId,
ref,
});
}
}
res.json({ ok: true });
},
});
});
app.post("/hooks/dialog", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const accept = toBoolean(body.accept);
const promptText = toStringOrEmpty(body.promptText) || undefined;
const timeoutMs = toNumber(body.timeoutMs);
if (accept === undefined) {
return jsonError(res, 400, "accept is required");
}
await withPlaywrightRouteContext({
req,
res,
ctx,
targetId,
feature: "dialog hook",
run: async ({ cdpUrl, tab, pw }) => {
await pw.armDialogViaPlaywright({
cdpUrl,
targetId: tab.targetId,
accept,
promptText,
timeoutMs: timeoutMs ?? undefined,
});
res.json({ ok: true });
},
});
});
}

View File

@@ -1,6 +1,7 @@
import fs from "node:fs/promises";
import type { BrowserFormField } from "../client-actions-core.js";
import type { BrowserRouteContext } from "../server-context.js";
import { registerBrowserAgentActDownloadRoutes } from "./agent.act.download.js";
import { registerBrowserAgentActHookRoutes } from "./agent.act.hooks.js";
import {
type ActKind,
isActKind,
@@ -13,43 +14,9 @@ import {
withPlaywrightRouteContext,
SELECTOR_UNSUPPORTED_MESSAGE,
} from "./agent.shared.js";
import {
DEFAULT_DOWNLOAD_DIR,
DEFAULT_UPLOAD_DIR,
resolveWritablePathWithinRoot,
resolveExistingPathsWithinRoot,
} from "./path-output.js";
import type { BrowserResponse, BrowserRouteRegistrar } from "./types.js";
import type { BrowserRouteRegistrar } from "./types.js";
import { jsonError, toBoolean, toNumber, toStringArray, toStringOrEmpty } from "./utils.js";
async function resolveDownloadPathOrRespond(
res: BrowserResponse,
requestedPath: string,
): Promise<string | null> {
const downloadPathResult = await resolveWritablePathWithinRoot({
rootDir: DEFAULT_DOWNLOAD_DIR,
requestedPath,
scopeLabel: "downloads directory",
});
if (!downloadPathResult.ok) {
res.status(400).json({ error: downloadPathResult.error });
return null;
}
return downloadPathResult.path;
}
function buildDownloadRequestBase(cdpUrl: string, targetId: string, timeoutMs: number | undefined) {
return {
cdpUrl,
targetId,
timeoutMs: timeoutMs ?? undefined,
};
}
function respondWithDownloadResult(res: BrowserResponse, targetId: string, result: unknown) {
res.json({ ok: true, targetId, download: result });
}
export function registerBrowserAgentActRoutes(
app: BrowserRouteRegistrar,
ctx: BrowserRouteContext,
@@ -367,163 +334,8 @@ export function registerBrowserAgentActRoutes(
});
});
app.post("/hooks/file-chooser", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const ref = toStringOrEmpty(body.ref) || undefined;
const inputRef = toStringOrEmpty(body.inputRef) || undefined;
const element = toStringOrEmpty(body.element) || undefined;
const paths = toStringArray(body.paths) ?? [];
const timeoutMs = toNumber(body.timeoutMs);
if (!paths.length) {
return jsonError(res, 400, "paths are required");
}
await withPlaywrightRouteContext({
req,
res,
ctx,
targetId,
feature: "file chooser hook",
run: async ({ cdpUrl, tab, pw }) => {
const uploadPathsResult = await resolveExistingPathsWithinRoot({
rootDir: DEFAULT_UPLOAD_DIR,
requestedPaths: paths,
scopeLabel: `uploads directory (${DEFAULT_UPLOAD_DIR})`,
});
if (!uploadPathsResult.ok) {
res.status(400).json({ error: uploadPathsResult.error });
return;
}
const resolvedPaths = uploadPathsResult.paths;
if (inputRef || element) {
if (ref) {
return jsonError(res, 400, "ref cannot be combined with inputRef/element");
}
await pw.setInputFilesViaPlaywright({
cdpUrl,
targetId: tab.targetId,
inputRef,
element,
paths: resolvedPaths,
});
} else {
await pw.armFileUploadViaPlaywright({
cdpUrl,
targetId: tab.targetId,
paths: resolvedPaths,
timeoutMs: timeoutMs ?? undefined,
});
if (ref) {
await pw.clickViaPlaywright({
cdpUrl,
targetId: tab.targetId,
ref,
});
}
}
res.json({ ok: true });
},
});
});
app.post("/hooks/dialog", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const accept = toBoolean(body.accept);
const promptText = toStringOrEmpty(body.promptText) || undefined;
const timeoutMs = toNumber(body.timeoutMs);
if (accept === undefined) {
return jsonError(res, 400, "accept is required");
}
await withPlaywrightRouteContext({
req,
res,
ctx,
targetId,
feature: "dialog hook",
run: async ({ cdpUrl, tab, pw }) => {
await pw.armDialogViaPlaywright({
cdpUrl,
targetId: tab.targetId,
accept,
promptText,
timeoutMs: timeoutMs ?? undefined,
});
res.json({ ok: true });
},
});
});
app.post("/wait/download", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const out = toStringOrEmpty(body.path) || "";
const timeoutMs = toNumber(body.timeoutMs);
await withPlaywrightRouteContext({
req,
res,
ctx,
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 = await resolveDownloadPathOrRespond(res, out);
if (!resolvedDownloadPath) {
return;
}
downloadPath = resolvedDownloadPath;
}
const requestBase = buildDownloadRequestBase(cdpUrl, tab.targetId, timeoutMs);
const result = await pw.waitForDownloadViaPlaywright({
...requestBase,
path: downloadPath,
});
respondWithDownloadResult(res, tab.targetId, result);
},
});
});
app.post("/download", async (req, res) => {
const body = readBody(req);
const targetId = resolveTargetIdFromBody(body);
const ref = toStringOrEmpty(body.ref);
const out = toStringOrEmpty(body.path);
const timeoutMs = toNumber(body.timeoutMs);
if (!ref) {
return jsonError(res, 400, "ref is required");
}
if (!out) {
return jsonError(res, 400, "path is required");
}
await withPlaywrightRouteContext({
req,
res,
ctx,
targetId,
feature: "download",
run: async ({ cdpUrl, tab, pw }) => {
await fs.mkdir(DEFAULT_DOWNLOAD_DIR, { recursive: true });
const downloadPath = await resolveDownloadPathOrRespond(res, out);
if (!downloadPath) {
return;
}
const requestBase = buildDownloadRequestBase(cdpUrl, tab.targetId, timeoutMs);
const result = await pw.downloadViaPlaywright({
...requestBase,
ref,
path: downloadPath,
});
respondWithDownloadResult(res, tab.targetId, result);
},
});
});
registerBrowserAgentActHookRoutes(app, ctx);
registerBrowserAgentActDownloadRoutes(app, ctx);
app.post("/response/body", async (req, res) => {
const body = readBody(req);

View File

@@ -1,5 +1,4 @@
import crypto from "node:crypto";
import fs from "node:fs/promises";
import path from "node:path";
import type { BrowserRouteContext } from "../server-context.js";
import {
@@ -8,7 +7,8 @@ import {
resolveTargetIdFromQuery,
withPlaywrightRouteContext,
} from "./agent.shared.js";
import { DEFAULT_TRACE_DIR, resolveWritablePathWithinRoot } from "./path-output.js";
import { resolveWritableOutputPathOrRespond } from "./output-paths.js";
import { DEFAULT_TRACE_DIR } from "./path-output.js";
import type { BrowserRouteRegistrar } from "./types.js";
import { toBoolean, toStringOrEmpty } from "./utils.js";
@@ -120,19 +120,17 @@ export function registerBrowserAgentDebugRoutes(
feature: "trace stop",
run: async ({ cdpUrl, tab, pw }) => {
const id = crypto.randomUUID();
const dir = DEFAULT_TRACE_DIR;
await fs.mkdir(dir, { recursive: true });
const tracePathResult = await resolveWritablePathWithinRoot({
rootDir: dir,
const tracePath = await resolveWritableOutputPathOrRespond({
res,
rootDir: DEFAULT_TRACE_DIR,
requestedPath: out,
scopeLabel: "trace directory",
defaultFileName: `browser-trace-${id}.zip`,
ensureRootDir: true,
});
if (!tracePathResult.ok) {
res.status(400).json({ error: tracePathResult.error });
if (!tracePath) {
return;
}
const tracePath = tracePathResult.path;
await pw.traceStopViaPlaywright({
cdpUrl,
targetId: tab.targetId,

View File

@@ -0,0 +1,31 @@
import fs from "node:fs/promises";
import { resolveWritablePathWithinRoot } from "./path-output.js";
import type { BrowserResponse } from "./types.js";
export async function ensureOutputRootDir(rootDir: string): Promise<void> {
await fs.mkdir(rootDir, { recursive: true });
}
export async function resolveWritableOutputPathOrRespond(params: {
res: BrowserResponse;
rootDir: string;
requestedPath: string;
scopeLabel: string;
defaultFileName?: string;
ensureRootDir?: boolean;
}): Promise<string | null> {
if (params.ensureRootDir) {
await ensureOutputRootDir(params.rootDir);
}
const pathResult = await resolveWritablePathWithinRoot({
rootDir: params.rootDir,
requestedPath: params.requestedPath,
scopeLabel: params.scopeLabel,
defaultFileName: params.defaultFileName,
});
if (!pathResult.ok) {
params.res.status(400).json({ error: pathResult.error });
return null;
}
return pathResult.path;
}

View File

@@ -75,37 +75,17 @@ export function resolvePreferredOpenClawTmpDir(
return st.isDirectory() && !st.isSymbolicLink() && isSecureDirForUser(st);
};
const resolvePreferredState = (
const resolveDirState = (
candidatePath: string,
requireWritableAccess: boolean,
): "available" | "missing" | "invalid" => {
try {
const preferred = lstatSync(POSIX_OPENCLAW_TMP_DIR);
if (!isTrustedPreferredDir(preferred)) {
return "invalid";
}
if (requireWritableAccess) {
accessSync(POSIX_OPENCLAW_TMP_DIR, fs.constants.W_OK | fs.constants.X_OK);
}
return "available";
} catch (err) {
if (isNodeErrorWithCode(err, "ENOENT")) {
return "missing";
}
return "invalid";
}
};
const resolveFallbackState = (
fallbackPath: string,
requireWritableAccess: boolean,
): "available" | "missing" | "invalid" => {
try {
const candidate = lstatSync(fallbackPath);
const candidate = lstatSync(candidatePath);
if (!isTrustedPreferredDir(candidate)) {
return "invalid";
}
if (requireWritableAccess) {
accessSync(fallbackPath, fs.constants.W_OK | fs.constants.X_OK);
accessSync(candidatePath, fs.constants.W_OK | fs.constants.X_OK);
}
return "available";
} catch (err) {
@@ -118,7 +98,7 @@ export function resolvePreferredOpenClawTmpDir(
const ensureTrustedFallbackDir = (): string => {
const fallbackPath = fallback();
const state = resolveFallbackState(fallbackPath, true);
const state = resolveDirState(fallbackPath, true);
if (state === "available") {
return fallbackPath;
}
@@ -130,13 +110,13 @@ export function resolvePreferredOpenClawTmpDir(
} catch {
throw new Error(`Unable to create fallback OpenClaw temp dir: ${fallbackPath}`);
}
if (resolveFallbackState(fallbackPath, true) !== "available") {
if (resolveDirState(fallbackPath, true) !== "available") {
throw new Error(`Unsafe fallback OpenClaw temp dir: ${fallbackPath}`);
}
return fallbackPath;
};
const existingPreferredState = resolvePreferredState(true);
const existingPreferredState = resolveDirState(POSIX_OPENCLAW_TMP_DIR, true);
if (existingPreferredState === "available") {
return POSIX_OPENCLAW_TMP_DIR;
}
@@ -148,7 +128,7 @@ export function resolvePreferredOpenClawTmpDir(
accessSync("/tmp", fs.constants.W_OK | fs.constants.X_OK);
// Create with a safe default; subsequent callers expect it exists.
mkdirSync(POSIX_OPENCLAW_TMP_DIR, { recursive: true, mode: 0o700 });
if (resolvePreferredState(true) !== "available") {
if (resolveDirState(POSIX_OPENCLAW_TMP_DIR, true) !== "available") {
return ensureTrustedFallbackDir();
}
return POSIX_OPENCLAW_TMP_DIR;