From a8a34c92f397b55f81ca09cbacc3bc2469a2d427 Mon Sep 17 00:00:00 2001 From: Shakker Date: Tue, 12 May 2026 00:23:09 +0100 Subject: [PATCH] fix: validate literal tilde exec scripts --- .../bash-tools.exec.script-preflight.test.ts | 6 +- src/agents/bash-tools.exec.ts | 73 ++++++++++++++++--- 2 files changed, 65 insertions(+), 14 deletions(-) diff --git a/src/agents/bash-tools.exec.script-preflight.test.ts b/src/agents/bash-tools.exec.script-preflight.test.ts index 727b6ac08f0..e53a05dcf24 100644 --- a/src/agents/bash-tools.exec.script-preflight.test.ts +++ b/src/agents/bash-tools.exec.script-preflight.test.ts @@ -487,10 +487,8 @@ describeNonWin("exec script preflight", () => { workdir: tmp, }), ).resolves.toBeUndefined(); - expect(scriptOpenFlags.length).toBeGreaterThan(0); - expect(scriptOpenFlags.filter((flags) => (flags & fsConstants.O_NONBLOCK) !== 0)).not.toEqual( - [], - ); + expect(scriptOpenFlags).not.toStrictEqual([]); + expect(scriptOpenFlags.every((flags) => (flags & fsConstants.O_NONBLOCK) !== 0)).toBe(true); }); }); diff --git a/src/agents/bash-tools.exec.ts b/src/agents/bash-tools.exec.ts index ce2cb125ca1..0b069534edd 100644 --- a/src/agents/bash-tools.exec.ts +++ b/src/agents/bash-tools.exec.ts @@ -1,3 +1,5 @@ +import { constants as fsConstants } from "node:fs"; +import fs from "node:fs/promises"; import path from "node:path"; import type { AgentToolResult } from "@earendil-works/pi-agent-core"; import { buildCommandPayloadCandidates } from "../infra/command-analysis/risks.js"; @@ -113,6 +115,12 @@ const SKIPPABLE_SCRIPT_PREFLIGHT_FS_ERROR_CODES = new Set([ "ENOTDIR", "EPERM", ]); +const SCRIPT_PREFLIGHT_MAX_BYTES = 512 * 1024; +const FS_CONSTANTS_WITH_OPTIONAL_NONBLOCK = fsConstants as typeof fsConstants & { + O_NONBLOCK?: number; +}; +const SCRIPT_PREFLIGHT_OPEN_FLAGS = + fsConstants.O_RDONLY | (FS_CONSTANTS_WITH_OPTIONAL_NONBLOCK.O_NONBLOCK ?? 0); function getNodeErrorCode(error: unknown): string | undefined { if (typeof error !== "object" || error === null || !("code" in error)) { @@ -149,9 +157,46 @@ function resolvePreflightRelativePath(params: { rootDir: string; absPath: string if (/^\.\.(?:[\\/]|$)/u.test(relative) || path.isAbsolute(relative)) { return null; } - // Preserve literal "~" path segments under the workdir. Root reads - // expand home prefixes for relative paths, so normalize `~/...` to `./~/...`. - return /^~(?:$|[\\/])/u.test(relative) ? `.${path.sep}${relative}` : relative; + return relative; +} + +function hasLeadingTildePathSegment(relativePath: string): boolean { + return /^~(?:$|[\\/])/u.test(relativePath); +} + +async function readLiteralTildePreflightScript(params: { + absPath: string; + fsSafe: FsSafeModule; + workspaceRoot: Awaited>; +}): Promise { + let handle: fs.FileHandle | undefined; + try { + handle = await fs.open(params.absPath, SCRIPT_PREFLIGHT_OPEN_FLAGS); + const stat = await handle.stat(); + if (!stat.isFile()) { + throw new params.fsSafe.FsSafeError("not-file", "not a file"); + } + if (stat.size > SCRIPT_PREFLIGHT_MAX_BYTES) { + throw new params.fsSafe.FsSafeError( + "too-large", + `file exceeds limit of ${SCRIPT_PREFLIGHT_MAX_BYTES} bytes (got ${stat.size})`, + ); + } + const realPath = await params.fsSafe.resolveOpenedFileRealPathForHandle(handle, params.absPath); + if (!params.fsSafe.isPathInside(params.workspaceRoot.rootReal, realPath)) { + throw new params.fsSafe.FsSafeError("outside-workspace", "file is outside workspace root"); + } + const buffer = await handle.readFile(); + if (buffer.byteLength > SCRIPT_PREFLIGHT_MAX_BYTES) { + throw new params.fsSafe.FsSafeError( + "too-large", + `file exceeds limit of ${SCRIPT_PREFLIGHT_MAX_BYTES} bytes (got ${buffer.byteLength})`, + ); + } + return buffer.toString("utf-8"); + } finally { + await handle?.close().catch(() => undefined); + } } function isShellEnvAssignmentToken(token: string): boolean { @@ -967,7 +1012,8 @@ async function validateScriptFileForShellBleed(params: { return; } - const { FsSafeError, root: fsRoot } = await loadFsSafeModule(); + const fsSafe = await loadFsSafeModule(); + const { FsSafeError, root: fsRoot } = fsSafe; const workspaceRoot = await fsRoot(params.workdir); for (const relOrAbsPath of target.relOrAbsPaths) { const absPath = path.isAbsolute(relOrAbsPath) @@ -987,12 +1033,19 @@ async function validateScriptFileForShellBleed(params: { // Use non-blocking open to avoid stalls if a path is swapped to a FIFO. let content: string; try { - const safeRead = await workspaceRoot.read(relativePath, { - nonBlockingRead: true, - symlinks: "follow-within-root", - maxBytes: 512 * 1024, - }); - content = safeRead.buffer.toString("utf-8"); + content = hasLeadingTildePathSegment(relativePath) + ? await readLiteralTildePreflightScript({ + absPath, + fsSafe, + workspaceRoot, + }) + : ( + await workspaceRoot.read(relativePath, { + nonBlockingRead: true, + symlinks: "follow-within-root", + maxBytes: SCRIPT_PREFLIGHT_MAX_BYTES, + }) + ).buffer.toString("utf-8"); } catch (error) { if (shouldSkipScriptPreflightPathError(error, FsSafeError)) { // Preflight validation is best-effort: skip path/read failures and