ref(fs): add safeReadFile helper for FIFO-safe config reads (#819)

## Summary

Extends the FIFO-safety pattern from #806 beyond DSN detection. All
remaining single-file reads of user-controlled paths now have a
`stat.isFile()` guard (either via a new `safeReadFile` helper or
inline), eliminating the FIFO-hang bug class across the codebase.

## Why

PR #806 fixed `Bun.file().text()` hanging on 1Password-style symlinked
FIFOs for DSN detection (4 sites in `src/lib/dsn/`). But the CLI has ~4
more read sites under user-controlled paths with the same vulnerability:

- **`src/lib/sentryclirc.ts`** — reads `.sentryclirc` at every directory
during walk-up (hit on every CLI invocation).
- **`src/lib/init/tools/read-files.ts`** — LLM-driven reads of arbitrary
project files during init wizard.
- **`src/lib/init/tools/apply-patchset.ts`** — reads modify-target files
during init wizard.
- **`src/lib/init/workflow-inputs.ts::preReadCommonFiles`** — scans a
fixed allowlist of common config filenames.

All four would hang on a 1Password-symlinked `.env`, `.sentryclirc`,
`package.json`, etc. An audit pass also surfaced a subtle bug in the two
sites that already had a `stat()`-based size check: `stat` on a FIFO
returns successfully with `size=0`, so the size-check passes and the
subsequent read still hangs. Adding `stat.isFile()` alongside the size
check fixes this without a second `stat` call.

## Design

Two distinct policies, corresponding to two different read contexts:

### `src/lib/safe-read.ts::safeReadFile` — opportunistic best-effort
reads

```ts
safeReadFile(path, operation) → Promise<string | null>
```

Chains `isRegularFile` (stat + FIFO guard) with `Bun.file(path).text()`.
Returns `null` for any expected error (ENOENT, EACCES, EPERM, EISDIR,
ENOTDIR, non-regular file, etc.); unexpected errors route through the
existing `handleFileError` → Sentry pipeline and also return `null`.
Used by `apply-patchset.ts` (where null is translated to a targeted
throw).

### `sentryclirc.ts::tryReadSentryCliRc` — committed config load

Direct `fs.promises.stat` + narrow `try/catch` that returns `null` only
on ENOENT/EACCES; re-throws every other error. A user with a broken
`.sentryclirc` (EPERM at the dir level, EISDIR, EIO, etc.) sees the
error clearly rather than getting confusing downstream "no auth token"
failures.

The two sites with size-gated reads (`read-files.ts`,
`workflow-inputs.ts::preReadCommonFiles`) keep their existing
`fs.promises.stat` call but now check `stat.isFile()` alongside
`stat.size`. No extra stat calls.

## Per-site changes

| Site | Before | After |
|---|---|---|
| `sentryclirc.ts::tryApplyFile` | Local `tryReadFile` (18 LOC,
ENOENT/EACCES-only) | `tryReadSentryCliRc` — adds `stat.isFile()` FIFO
guard, keeps the narrow ENOENT/EACCES re-throw policy intact |
| `init/tools/read-files.ts` | `fs.promises.stat` + size gate + read |
Same flow + explicit `stat.isFile()` check before the read |
| `init/tools/apply-patchset.ts` | Direct
`fs.promises.readFile(absPath)` | `safeReadFile` → throws targeted "not
a regular file or read failed" on null |
| `init/workflow-inputs.ts::preReadCommonFiles` | `stat` + size gate +
read | Same + `stat.isFile()` check; sets `cache[filePath] = null` on
non-regular |

## Regression tests

`test/lib/safe-read.test.ts` — 11 tests covering every migration site:

- `safeReadFile` on regular file / missing / FIFO / symlink→FIFO /
directory
- `sentryclirc` loader on a FIFO-backed `.sentryclirc` → empty config,
no hang
- `readFiles` tool on FIFO and symlink→FIFO paths (both test cases)
- `applyPatchset` tool on FIFO and symlink→FIFO targets (both test
cases)
- `preReadCommonFiles` on a FIFO-backed common-config file

**Verified negatively**: for each migrated site, temporarily removing
the FIFO guard causes the corresponding regression test to time out at
Bun's 5s default, confirming the guards are genuinely the condition
being exercised.

## Review cycle

The PR went through multiple rounds of self-review + bot review after
the initial post. Key fixes from review:

- **Seer (HIGH)** flagged that my first pass at migrating sentryclirc
silently swallowed EPERM/EISDIR/EIO errors. Fixed by reverting
sentryclirc to direct `fs.promises.stat` with the narrow original error
policy (not via `isRegularFile`, which had a broader `handleFileError`
swallow-list).
- **Cursor (Medium)** caught that my second pass *still* had the bug
because `isRegularFile` internally calls `handleFileError`. Fixed by
calling `stat` directly.
- **Cursor (Low)** caught unused re-exports in `safe-read.ts` after the
sentryclirc-goes-direct change removed the last consumer. Dropped.
- **Cursor (Low)** caught an orphaned JSDoc block after a
helper-extraction mid-file. Swapped order.
- **Self-review subagent** caught that one test was calling
`precomputeDirListing` (a listing op) instead of `preReadCommonFiles`
(the function with the new guard). Fixed to call the right function and
negative-verified.

## Test plan

- [x] `bunx tsc --noEmit` — clean
- [x] `bun run lint` — clean (1 pre-existing markdown.ts warning)
- [x] `bun test test/lib/safe-read.test.ts` — **11 pass**
- [x] `bun test --timeout 15000 test/lib test/commands test/types` —
**5687 pass, 0 fail**
- [x] `bun test test/isolated` — 138 pass
- [x] Negative verification for each guarded site — confirmed genuine
regression coverage

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: BYK

src/lib/init/tools/apply-patchset.ts

@@ -1,5 +1,6 @@
 import fs from "node:fs";
 import path from "node:path";
+import { safeReadFile } from "../../safe-read.js";
 import { replace } from "../replacers.js";
 import type {
   ApplyPatchsetPatch,
@@ -149,7 +150,20 @@ async function applyEdits(
   filePath: string,
   edits: Array<{ oldString: string; newString: string }>
 ): Promise<string> {
-  let content = await fs.promises.readFile(absPath, "utf-8");
+  const initialContent = await safeReadFile(absPath, "apply-patchset.read");
+  if (initialContent === null) {
+    // `applyPatchset`'s earlier `access()` call only verifies
+    // existence — it follows symlinks and succeeds on FIFOs/sockets,
+    // so this branch is the primary guard against non-regular files
+    // (FIFO, socket, symlink → FIFO) that would otherwise hang
+    // `readFile` indefinitely, plus any other expected I/O failure
+    // (permission, transient read error) routed through
+    // `safeReadFile`.
+    throw new Error(
+      `Cannot read "${filePath}": not a regular file or read failed`
+    );
+  }
+  let content = initialContent;
 
   for (let i = 0; i < edits.length; i += 1) {
     const edit = edits[i];

src/lib/init/tools/read-files.ts

@@ -36,6 +36,12 @@ async function readSingleFile(
   try {
     const absPath = safePath(cwd, filePath);
     const stat = await fs.promises.stat(absPath);
+    // Guard against FIFOs / sockets / devices — both `readFile` and
+    // `open("r")` block indefinitely on a FIFO waiting for a writer.
+    // `stat` follows symlinks, so symlink → FIFO is caught too.
+    if (!stat.isFile()) {
+      return null;
+    }
     if (stat.size <= maxBytes) {
       return await fs.promises.readFile(absPath, "utf-8");
     }

src/lib/init/workflow-inputs.ts

@@ -121,6 +121,13 @@ export async function preReadCommonFiles(
     try {
       const absPath = path.join(directory, filePath);
       const stat = await fs.promises.stat(absPath);
+      // Guard against FIFOs / sockets / devices — `fs.readFile` on a
+      // FIFO blocks indefinitely waiting for a writer. `stat` follows
+      // symlinks, so a symlink → FIFO is also caught here.
+      if (!stat.isFile()) {
+        cache[filePath] = null;
+        continue;
+      }
       if (stat.size > MAX_FILE_BYTES) {
         continue;
       }

src/lib/safe-read.ts

@@ -0,0 +1,45 @@
+/**
+ * FIFO-safe file-read helper for user-controlled paths.
+ *
+ * Named pipes (FIFOs), commonly created by 1Password's `.env` symlink
+ * integration, cause `Bun.file(path).text()` to block indefinitely
+ * waiting for a writer. Any read of a path under the user's project
+ * tree or home directory needs a `stat`-based regular-file check
+ * first.
+ *
+ * Prefer this helper over calling `isRegularFile` + `Bun.file().text()`
+ * by hand: a single call, consistent error handling, no way to forget
+ * the guard.
+ */
+
+import { handleFileError, isRegularFile } from "./dsn/fs-utils.js";
+
+/**
+ * Read a file's text content, returning `null` when the path is:
+ * - missing / inaccessible (ENOENT / EACCES / EPERM / EISDIR / ENOTDIR)
+ * - not a regular file (FIFO, socket, device, symlink to any of these)
+ * - any other expected I/O-level failure routed through
+ *   {@link handleFileError}
+ *
+ * Unexpected errors are captured to Sentry via `handleFileError` and
+ * also return `null`, so callers don't need their own try/catch.
+ *
+ * @param filePath - Absolute path to read.
+ * @param operation - Logical operation name, reported to Sentry when a
+ *   stat or read fails unexpectedly. Keep it short and specific (e.g.,
+ *   `"sentryclirc.read"`, `"read-files.tool"`).
+ */
+export async function safeReadFile(
+  filePath: string,
+  operation: string
+): Promise<string | null> {
+  if (!(await isRegularFile(filePath, `${operation}.stat`))) {
+    return null;
+  }
+  try {
+    return await Bun.file(filePath).text();
+  } catch (error) {
+    handleFileError(error, { operation, path: filePath });
+    return null;
+  }
+}

src/lib/sentryclirc.ts

@@ -16,6 +16,7 @@
  * `resolve-target.ts`.
  */
 
+import { stat } from "node:fs/promises";
 import { homedir } from "node:os";
 import { join } from "node:path";
 import { getConfigDir } from "./db/index.js";
@@ -57,25 +58,6 @@ export type SentryCliRcConfig = {
  */
 const cache = new Map<string, Promise<SentryCliRcConfig>>();
 
-/**
- * Read a file's text content, returning null for expected I/O errors.
- * ENOENT (missing) and EACCES (permission denied) return null.
- * All other errors propagate.
- */
-async function tryReadFile(filePath: string): Promise<string | null> {
-  try {
-    return await Bun.file(filePath).text();
-  } catch (error: unknown) {
-    if (error instanceof Error && "code" in error) {
-      const { code } = error as NodeJS.ErrnoException;
-      if (code === "ENOENT" || code === "EACCES") {
-        return null;
-      }
-    }
-    throw error;
-  }
-}
-
 /**
  * Fields we extract from an INI config, keyed by section.field.
  *
@@ -125,6 +107,67 @@ function isComplete(result: SentryCliRcConfig): boolean {
   );
 }
 
+/**
+ * True for the two I/O errors we treat as "file effectively absent"
+ * — a missing path and a permission-denied read. Any other error
+ * code signals something worth surfacing to the user.
+ */
+function isNarrowAbsenceError(error: unknown): boolean {
+  if (error instanceof Error && "code" in error) {
+    const { code } = error as NodeJS.ErrnoException;
+    return code === "ENOENT" || code === "EACCES";
+  }
+  return false;
+}
+
+/**
+ * Read a `.sentryclirc` file's text content. Returns `null` when
+ * the file:
+ *
+ * - is absent (ENOENT)
+ * - is unreadable for a common reason (EACCES)
+ * - is a non-regular file (FIFO / socket / symlink → FIFO — the
+ *   1Password pattern, which would otherwise block `text()`
+ *   indefinitely)
+ *
+ * Re-throws every other I/O error (EPERM, EISDIR, EIO,
+ * ENOTDIR, etc.). A `.sentryclirc` is a committed config file — a
+ * user with `chmod 000` at the directory level (EPERM), a typo
+ * pointing at a directory (EISDIR), or a disk throwing EIO needs
+ * to see that clearly, not have it silently suppressed and surface
+ * downstream as a confusing "no auth token" error.
+ *
+ * Note: cannot use {@link safeReadFile} / `isRegularFile` from
+ * `safe-read.ts` — their `handleFileError` treats EPERM and
+ * EISDIR as ignorable, swallowing them during the stat phase.
+ * That broader policy is correct for opportunistic DSN scans; not
+ * for this committed config load.
+ */
+async function tryReadSentryCliRc(filePath: string): Promise<string | null> {
+  let statResult: Awaited<ReturnType<typeof stat>>;
+  try {
+    statResult = await stat(filePath);
+  } catch (error) {
+    if (isNarrowAbsenceError(error)) {
+      return null;
+    }
+    throw error;
+  }
+  // Non-regular file (FIFO, socket, device, symlink → any of these)
+  // — `text()` would block. Return null so the walk-up moves on.
+  if (!statResult.isFile()) {
+    return null;
+  }
+  try {
+    return await Bun.file(filePath).text();
+  } catch (error) {
+    if (isNarrowAbsenceError(error)) {
+      return null;
+    }
+    throw error;
+  }
+}
+
 /**
  * Try to read and apply a `.sentryclirc` file to the result.
  * No-op if the file doesn't exist or can't be read.
@@ -134,7 +177,7 @@ async function tryApplyFile(
   filePath: string,
   isGlobal: boolean
 ): Promise<void> {
-  const content = await tryReadFile(filePath);
+  const content = await tryReadSentryCliRc(filePath);
   if (content !== null) {
     log.debug(
       `Found ${isGlobal ? "global" : "local"} ${CONFIG_FILENAME} at ${filePath}`