VC-801: Bound read: refuse non-regular files, name binaries, stream text

packages/agent-runtime/UPSTREAM.md

@@ -181,7 +181,10 @@ request: those endpoints carry prompts in their own `instructions`/`system` and
 JSON-only (`JsonValue` arrays readonly). Runtime tools already produced JSON,
 so only test fixtures needed retyping. `ExecutionEnv` gained
 `openTextLineReader`; `ScopedExecutionEnv` returns `not_supported`, as it does
-for `readTextLines`.
+for `readTextLines`. (VC-801 later added a Volli-owned `openFileReader`, which
+`read` uses; `ScopedExecutionEnv` guards and delegates it like
+`readBinaryFile`, and `refusingCredentialReads` refuses it for credential
+files.)
 
 The optional `Model.inputLimits` and `Model.promptCache` fields (0.87) are used
 only by Pi's coding-agent for image resizing and cache warming.

packages/agent-runtime/src/pi/credential-env.ts

@@ -39,7 +39,13 @@ export function credentialPath(path: string): boolean {
 }
 
 export function refusingCredentialReads(env: ExecutionEnv, workspacePath: string): ExecutionEnv {
-  const reads = new Set(["openTextLineReader", "readTextFile", "readTextLines", "readBinaryFile"]);
+  const reads = new Set([
+    "openFileReader",
+    "openTextLineReader",
+    "readTextFile",
+    "readTextLines",
+    "readBinaryFile",
+  ]);
   return new Proxy(env, {
     get(target, property) {
       const member: unknown = Reflect.get(target, property);

packages/agent-runtime/src/pi/pi-vendor-tools-coverage.test.ts

@@ -283,7 +283,8 @@ describe("vendored read contracts", () => {
       read.execute("read", { path: null as unknown as string }, () => {}, { env }, ...inert),
     ).rejects.toThrow(TypeError);
     const failure = new FileError("permission_denied", "binary denied");
-    const binary = vi.spyOn(env, "readBinaryFile").mockResolvedValue(err(failure));
+    // VC-801: `read` opens a bounded reader rather than reading the file whole.
+    const binary = vi.spyOn(env, "openFileReader").mockResolvedValue(err(failure));
     await expect(
       read.execute("read", { path: "note.txt" }, () => {}, { env }, ...inert),
     ).rejects.toBe(failure);

packages/agent-runtime/src/pi/scoped-execution-env.ts

@@ -595,6 +595,12 @@ export class ScopedExecutionEnv implements ExecutionEnv {
     return guarded.ok ? this.#delegate.readBinaryFile(guarded.value, context) : guarded;
   }
 
+  /** `read`'s bounded way in (VC-801): the same guard as {@link readBinaryFile}. */
+  async openFileReader(path: string, context: Context = BACKGROUND_CONTEXT) {
+    const guarded = await this.#guard(path, context);
+    return guarded.ok ? this.#delegate.openFileReader(guarded.value, context) : guarded;
+  }
+
   async fileInfo(path: string, context: Context = BACKGROUND_CONTEXT) {
     const guarded = await this.#guard(path, context);
     return guarded.ok ? this.#delegate.fileInfo(guarded.value, context) : guarded;

packages/agent-runtime/src/pi/vendor/pi-harness/README.md

@@ -122,6 +122,23 @@ branch-summary execution. `Agent`/agent loop are **not** vendored.
    (upstream `applyEditsToNormalizedContent`) takes the original text, and the
    reported `patch` is built from the original and written text so it applies
    to the file's bytes. `detectLineEnding` and `restoreLineEndings` are removed.
+7. **Bounded read (VC-801):** `read` no longer calls `readBinaryFile`. It opens
+   the file through a new `FileSystem.openFileReader` (Node: opened
+   `O_NONBLOCK | O_NOCTTY` and judged by `fstat` on the descriptor, so a
+   directory, FIFO, device or socket is refused at once without a pathname
+   check/use window, symlinks followed; see
+   `docs/development/codeql-triage.md`) and reads
+   the first 8 KiB. An image signature there still reads the whole file into
+   the unchanged image path; a NUL byte there answers a binary notice (a
+   UTF-16 notice when a UTF-16 BOM leads) instead of decoded text; anything
+   else streams in 256 KiB reads into one reused buffer, finding lines by
+   their LF byte and decoding only the prefix the 2,000-line / 50 KiB cut can
+   show. Every byte is still read, so the hint can name the line count. Text,
+   continuation offsets and truncation wording are unchanged;
+   `details.truncation.totalBytes` and an oversized first line's size are
+   counted in the file's bytes (less a leading BOM), which differs from
+   decoding everything only for invalid UTF-8. A negative `limit` now selects
+   no lines where `Array.prototype.slice` counted from the end.
 
 ## Compatibility evidence
 

packages/agent-runtime/src/pi/vendor/pi-harness/env/nodejs.ts

@@ -13,6 +13,7 @@ import {
   realpath,
   rename,
   rm,
+  stat,
   writeFile,
 } from "node:fs/promises";
 import { homedir, constants as osConstants, tmpdir } from "node:os";
@@ -24,6 +25,7 @@ import {
   type ForegroundShellExecutor,
   ExecutionError,
   err,
+  type FileByteReader,
   FileError,
   type FileInfo,
   type FileKind,
@@ -464,6 +466,108 @@ class NodeTextLineReader implements TextLineReader {
   }
 }
 
+/**
+ * Positional reads over a regular file opened by {@link NodeExecutionEnv.openFileReader}
+ * (VC-801). Each read is bounded by its caller; nothing here buffers the file.
+ */
+class NodeFileByteReader implements FileByteReader {
+  readonly size: number;
+  private readonly file: Awaited<ReturnType<typeof openFile>>;
+  private readonly path: string;
+  private closed = false;
+
+  constructor(file: Awaited<ReturnType<typeof openFile>>, path: string, size: number) {
+    this.file = file;
+    this.path = path;
+    this.size = size;
+  }
+
+  async read(
+    position: number,
+    length: number,
+    context: Context,
+    into?: Uint8Array,
+  ): Promise<Result<Uint8Array, FileError>> {
+    const aborted = abortResult<Uint8Array>(context.abortSignal, this.path);
+    if (aborted) return aborted;
+    if (this.closed) return err(new FileError("invalid", "File reader is closed", this.path));
+    try {
+      const buffer = into ?? Buffer.allocUnsafe(length);
+      let filled = 0;
+      while (filled < length) {
+        const { bytesRead } = await this.file.read(
+          buffer,
+          filled,
+          length - filled,
+          position + filled,
+        );
+        const afterReadAbort = abortResult<Uint8Array>(context.abortSignal, this.path);
+        if (afterReadAbort) return afterReadAbort;
+        if (bytesRead === 0) break;
+        filled += bytesRead;
+      }
+      return ok(buffer.subarray(0, filled));
+    } catch (error) {
+      return err(toFileError(error, this.path));
+    }
+  }
+
+  async close(_context: Context): Promise<void> {
+    if (this.closed) return;
+    this.closed = true;
+    try {
+      await this.file.close();
+    } catch {
+      // Closing is best-effort, including after cancellation or an earlier I/O failure.
+    }
+  }
+}
+
+const NOT_A_REGULAR_FILE =
+  "Not a regular file (a FIFO, device or socket); only regular files can be read";
+
+/** Why a file of this kind cannot be read, or nothing when it is a regular file. */
+function notRegularFile(
+  stats: { isFile(): boolean; isDirectory(): boolean },
+  path: string,
+): FileError | undefined {
+  if (stats.isFile()) return undefined;
+  return stats.isDirectory()
+    ? new FileError("is_directory", "Path is a directory", path)
+    : new FileError("invalid", NOT_A_REGULAR_FILE, path);
+}
+
+/**
+ * Why an open failed. A socket cannot be opened at all, and its errno differs
+ * by platform (ENXIO, or macOS's EOPNOTSUPP, which libuv leaves unnamed), so
+ * the path's kind is looked up after the failure, to choose the words only.
+ */
+async function openFailure(error: unknown, path: string): Promise<FileError> {
+  try {
+    const refusal = notRegularFile(await stat(path), path);
+    if (refusal) return refusal;
+  } catch {
+    // The open's own error says it better.
+  }
+  return toFileError(error, path);
+}
+
+async function regularFileReader(
+  file: Awaited<ReturnType<typeof openFile>>,
+  path: string,
+  context: Context,
+): Promise<Result<FileByteReader, FileError>> {
+  const aborted = abortResult<FileByteReader>(context.abortSignal, path);
+  if (aborted) return aborted;
+  try {
+    const stats = await file.stat();
+    const refusal = notRegularFile(stats, path);
+    return refusal ? err(refusal) : ok(new NodeFileByteReader(file, path, stats.size));
+  } catch (error) {
+    return err(toFileError(error, path));
+  }
+}
+
 export class NodeExecutionEnv implements ExecutionEnv {
   cwd: string;
   private shellPath?: string;
@@ -887,6 +991,32 @@ export class NodeExecutionEnv implements ExecutionEnv {
     }
   }
 
+  /**
+   * Judged on the open descriptor rather than on the path, so there is no
+   * check-then-use window for a swap to slip a device past (VC-801; see
+   * docs/development/codeql-triage.md). Opened non-blocking, so a FIFO cannot
+   * hold the open, and without a controlling terminal, so a terminal cannot
+   * become one; neither flag changes anything for a regular file. A socket
+   * cannot be opened at all, and is refused in the same words.
+   */
+  async openFileReader(path: string, context: Context): Promise<Result<FileByteReader, FileError>> {
+    const resolved = resolvePath(this.cwd, path);
+    const aborted = abortResult<FileByteReader>(context.abortSignal, resolved);
+    if (aborted) return aborted;
+    let file: Awaited<ReturnType<typeof openFile>>;
+    try {
+      file = await openFile(
+        resolved,
+        constants.O_RDONLY | constants.O_NONBLOCK | constants.O_NOCTTY,
+      );
+    } catch (error) {
+      return err(await openFailure(error, resolved));
+    }
+    const opened = await regularFileReader(file, resolved, context);
+    if (!opened.ok) await file.close().catch(() => undefined);
+    return opened;
+  }
+
   async writeFile(
     path: string,
     content: string | Uint8Array,