fix: input hardening + silent-failure hygiene (AGENTS.md doctrine)

src/core/audit/log.ts

@@ -33,23 +33,47 @@ export function createAuditLog(ctx: PluginInput): AuditLog {
       rotateIfNeeded(logPath, AUDIT_MAX_BYTES)
     }
 
+    let diskWriteFailed = false
     try {
       appendFileSync(logPath, JSON.stringify(entry) + "\n")
-    } catch {
-      // Silent fail — don't break the plugin if logging fails
+    } catch (diskErr) {
+      // Disk write failed (e.g. .opencode/ directory removed while plugin is running).
+      // This is NOT provably irrelevant — the audit trail is silently lost otherwise.
+      // We do NOT throw here (that would crash the calling handler), but we surface
+      // the failure as a warn via ctx.client.app.log, which is the only logger
+      // available in core/ without importing infra/logger. The app.log call below
+      // is the deliberate fallback; we annotate it with a warn level so the TUI
+      // shows the disk-write failure instead of silently upgrading it to a normal
+      // debug entry.
+      diskWriteFailed = true
+      const reason = diskErr instanceof Error ? diskErr.message : String(diskErr)
+      ctx.client.app
+        .log({
+          body: {
+            service: "opencode-pilot",
+            level: "warn",
+            message: `[audit] disk write failed for action "${action}": ${reason}`,
+            extra: { action, path: logPath, error: reason },
+          },
+        })
+        .catch(() => {})
     }
 
-    ctx.client.app
-      .log({
-        body: {
-          service: "opencode-pilot",
-          level:
-            action.includes("error") || action.includes("failed") ? "warn" : "debug",
-          message: `[audit] ${action}`,
-          extra: details,
-        },
-      })
-      .catch(() => {})
+    // Only emit the normal app.log entry when the disk write succeeded.
+    // If it failed, the warn entry above already carries the action details.
+    if (!diskWriteFailed) {
+      ctx.client.app
+        .log({
+          body: {
+            service: "opencode-pilot",
+            level:
+              action.includes("error") || action.includes("failed") ? "warn" : "debug",
+            message: `[audit] ${action}`,
+            extra: details,
+          },
+        })
+        .catch(() => {})
+    }
   }
 
   return { log }

src/core/audit/rotation.ts

@@ -3,7 +3,7 @@
 // Keeps up to 3 rotated files: .log → .log.1 → .log.2 → .log.3 (then dropped).
 // Extracted so audit.ts can remain simple and this logic is independently testable.
 
-import { existsSync, statSync, renameSync, unlinkSync } from "fs"
+import { existsSync, statSync, renameSync } from "fs"
 
 /** Max rotated files to keep (not counting the active file). */
 const MAX_ROTATIONS = 3
@@ -29,18 +29,34 @@ export function rotateIfNeeded(logPath: string, thresholdBytes: number): void {
   if (size <= thresholdBytes) return
 
   // Shift existing rotated files: .2 → .3, .1 → .2
+  // If any shift rename fails we bail out of the entire rotation rather than
+  // continuing — a partial shift would cause renaming the active log to .1
+  // to overwrite .1 with data that .1 was already holding. Log the failure
+  // via console.error (core/ has no logger — this runs before one is available)
+  // so the problem is at least visible in OpenCode's stderr output.
   for (let i = MAX_ROTATIONS - 1; i >= 1; i--) {
     const from = `${logPath}.${i}`
     const to = `${logPath}.${i + 1}`
     if (existsSync(from)) {
       try {
         renameSync(from, to)
-      } catch {}
+      } catch (err) {
+        // Rename failed mid-shift: bail out to avoid a partial rotation that
+        // would overwrite .1 with wrong data on the next step.
+        const reason = err instanceof Error ? err.message : String(err)
+        console.error(`[opencode-pilot] warn: audit rotation aborted — rename ${from} → ${to} failed: ${reason}`)
+        return
+      }
     }
   }
 
   // Move active log → .1
   try {
     renameSync(logPath, `${logPath}.1`)
-  } catch {}
+  } catch (err) {
+    // Failed to move the active log to .1. Rotation did not complete; the
+    // active file will continue to grow until the next rotation attempt.
+    const reason = err instanceof Error ? err.message : String(err)
+    console.error(`[opencode-pilot] warn: audit rotation aborted — rename ${logPath} → ${logPath}.1 failed: ${reason}`)
+  }
 }

src/core/events/bus.ts

@@ -29,7 +29,11 @@ export function createEventBus(): EventBus {
     if (process.env.PILOT_DEBUG_BUS === "1") {
       try {
         console.error(`[pilot:bus-emit] ${event.type} clients=${clients.size}`)
-      } catch (_) {}
+      } catch (_) {
+        // Provably irrelevant: console.error() is a debug-trace-only call gated
+        // behind PILOT_DEBUG_BUS=1. A failure here (e.g. closed stderr fd in
+        // an exotic test harness) must not prevent the actual SSE emit below.
+      }
     }
 
     const data = `data: ${JSON.stringify(event)}\n\n`

src/infra/banner/writer.test.ts

@@ -69,8 +69,7 @@ describe("writeBanner — projectStateMode", () => {
     // Before the fix, join("", ".opencode", "pilot-banner.txt") resolved to a
     // relative path inside the process cwd. With shouldWriteProjectState the
     // empty-string directory short-circuits to false and nothing is written.
-    await expect(
-      writeBanner({ ...minimalOpts, directory: "", projectStateMode: "auto" }),
-    ).resolves.toBeString()
+    const result = await writeBanner({ ...minimalOpts, directory: "", projectStateMode: "auto" })
+    expect(typeof result.banner).toBe("string")
   })
 })

src/infra/banner/writer.ts

@@ -8,12 +8,23 @@ export function globalBannerPath(): string {
   return stateFile("pilot-banner.txt")
 }
 
-function safeWrite(path: string, content: string): void {
+/**
+ * Write content to path, creating the parent directory as needed.
+ * Returns a string describing the failure, or null on success.
+ *
+ * Use for the per-project write (best-effort, caller ignores the result).
+ * For the global write use writeGlobal() which returns an error the caller can log.
+ */
+function safeWrite(path: string, content: string): string | null {
   try {
     mkdirSync(dirname(path), { recursive: true })
     writeFileSync(path, content)
-  } catch {
-    // Silent fail — caller tolerates missing banner file.
+    return null
+  } catch (err) {
+    // Per-project banner write is best-effort: the project may not have an
+    // .opencode/ dir (shouldWriteProjectState guards most cases) and a failure
+    // here does not break the TUI — the global path is the authoritative source.
+    return err instanceof Error ? err.message : String(err)
   }
 }
 
@@ -43,11 +54,25 @@ function encodePwaConnect(serverUrl: string, token: string): string {
   return Buffer.from(`${serverUrl}|${token}`).toString("base64")
 }
 
+export interface WriteBannerResult {
+  banner: string
+  /**
+   * Non-null when the GLOBAL banner write failed (the path the TUI reads).
+   * This is significant — the TUI will not show the connect URL until the
+   * plugin restarts. The caller (server/index.ts) should log this via
+   * ctx.client.app.log at warn level so the user sees it in the TUI output.
+   *
+   * Per-project write failures are best-effort and not reported here —
+   * the global path is the authoritative source.
+   */
+  globalWriteError: string | null
+}
+
 /**
  * Generates the startup banner text, prints the QR code, and writes the
  * banner to `.opencode/pilot-banner.txt` for the TUI to read.
  */
-export async function writeBanner(opts: BannerOptions): Promise<string> {
+export async function writeBanner(opts: BannerOptions): Promise<WriteBannerResult> {
   const { localUrl, publicUrl, token, directory, pwaUrl, projectStateMode = "auto" } = opts
 
   // Direct dashboard link (tunnel URL or local)
@@ -106,12 +131,22 @@ export async function writeBanner(opts: BannerOptions): Promise<string> {
   // where directory is empty or otherwise not suitable for join().
   if (shouldWriteProjectState(directory, projectStateMode)) {
     try {
+      // Per-project write is best-effort — failure is logged inside safeWrite()
+      // but not surfaced to the caller because the global path is authoritative.
       safeWrite(join(directory, ".opencode", "pilot-banner.txt"), banner)
     } catch {
-      // Silent fail — same contract as safeWrite.
+      // Defensive guard around the write; failure here is non-fatal — the
+      // in-memory banner string is still returned. safeWrite itself does not
+      // throw, but future refactors might wrap or replace it with something
+      // that does (e.g. fs.mkdirSync), so the catch keeps the caller contract
+      // stable regardless.
     }
   }
-  safeWrite(globalBannerPath(), banner)
 
-  return banner
+  // Global write: this is the path the TUI reads. A failure here means the
+  // user will not see the connect URL. Return the error so the caller
+  // (server/index.ts) can log it via ctx.client.app.log.
+  const globalWriteError = safeWrite(globalBannerPath(), banner)
+
+  return { banner, globalWriteError }
 }

src/infra/dotenv/index.ts

@@ -60,7 +60,16 @@ export function loadDotEnv(): DotenvResult {
     let text: string
     try {
       text = readFileSync(path, "utf8")
-    } catch {
+    } catch (readErr) {
+      // The file exists but is unreadable (e.g. permission denied, symlink loop).
+      // Silently continuing here would leave the user's env vars unapplied with
+      // no indication of why — they would see mysterious "config not applied"
+      // behaviour at runtime. We write to stderr (console.error) because this
+      // is infra/ startup code that runs before any logger is available.
+      // "File absent" (existsSync === false) stays fully silent — only an
+      // existing-but-unreadable file emits this warning.
+      const reason = readErr instanceof Error ? readErr.message : String(readErr)
+      console.error(`[opencode-pilot] warn: .env file found but unreadable — env vars NOT applied. path=${path} reason=${reason}`)
       continue
     }
     const parsed = parseEnv(text)

src/infra/http/text.ts

@@ -25,14 +25,23 @@ export async function readBoundedText(req: Request, maxBytes: number): Promise<s
       if (done) break
       total += value.byteLength
       if (total > maxBytes) {
-        try { reader.cancel() } catch {}
+        try { reader.cancel() } catch {
+          // Provably irrelevant: cancel() is best-effort stream teardown after
+          // we have already decided to reject the body. Failure here (e.g. the
+          // request stream was already closed by the client) does not affect
+          // the null return or downstream handling.
+        }
         return null
       }
       result += decoder.decode(value, { stream: true })
     }
     result += decoder.decode()
   } finally {
-    try { reader.releaseLock() } catch {}
+    try { reader.releaseLock() } catch {
+      // Provably irrelevant: releaseLock() is finally-block cleanup. Failure
+      // (e.g. lock already released by a prior cancel()) does not affect the
+      // return value or any downstream processing.
+    }
   }
   return result
 }

src/integrations/codex/validators.test.ts

@@ -261,3 +261,93 @@ describe("validateStop", () => {
     if (result.ok) expect(result.data.last_assistant_message).toBe("done")
   })
 })
+
+// ─── B1: Input length caps ────────────────────────────────────────────────────
+// session_id/turn_id/tool_use_id ≤ 128, tool_name/cwd/model ≤ 512, prompt ≤ 50_000
+
+describe("validateSessionStart — length caps (B1)", () => {
+  const base = {
+    session_id: "sess-1",
+    cwd: "/tmp",
+    model: "gpt-4o",
+    permission_mode: "default",
+  }
+
+  test("rejects session_id > 128 chars", () => {
+    const result = validateSessionStart({ ...base, session_id: "x".repeat(129) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("session_id")
+  })
+
+  test("accepts session_id at limit (128 chars)", () => {
+    const result = validateSessionStart({ ...base, session_id: "x".repeat(128) })
+    expect(result.ok).toBe(true)
+  })
+
+  test("rejects cwd > 512 chars", () => {
+    const result = validateSessionStart({ ...base, cwd: "/".repeat(513) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("cwd")
+  })
+
+  test("rejects model > 512 chars", () => {
+    const result = validateSessionStart({ ...base, model: "m".repeat(513) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("model")
+  })
+
+  test("rejects turn_id > 128 chars when provided", () => {
+    const result = validateSessionStart({ ...base, turn_id: "t".repeat(129) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("turn_id")
+  })
+})
+
+describe("validateUserPromptSubmit — length caps (B1)", () => {
+  const base = { session_id: "sess-1", turn_id: "turn-1", prompt: "hello" }
+
+  test("rejects session_id > 128 chars", () => {
+    const result = validateUserPromptSubmit({ ...base, session_id: "x".repeat(129) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("session_id")
+  })
+
+  test("rejects turn_id > 128 chars", () => {
+    const result = validateUserPromptSubmit({ ...base, turn_id: "t".repeat(129) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("turn_id")
+  })
+
+  test("rejects prompt > 50_000 chars", () => {
+    const result = validateUserPromptSubmit({ ...base, prompt: "p".repeat(50_001) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("prompt")
+  })
+
+  test("accepts prompt at limit (50_000 chars)", () => {
+    const result = validateUserPromptSubmit({ ...base, prompt: "p".repeat(50_000) })
+    expect(result.ok).toBe(true)
+  })
+})
+
+describe("validatePreToolUse — length caps (B1)", () => {
+  const base = {
+    session_id: "sess-1",
+    turn_id: "turn-1",
+    tool_use_id: "tuid-1",
+    tool_name: "bash",
+    tool_input: {},
+  }
+
+  test("rejects tool_use_id > 128 chars", () => {
+    const result = validatePreToolUse({ ...base, tool_use_id: "u".repeat(129) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("tool_use_id")
+  })
+
+  test("rejects tool_name > 512 chars", () => {
+    const result = validatePreToolUse({ ...base, tool_name: "n".repeat(513) })
+    expect(result.ok).toBe(false)
+    if (!result.ok) expect(result.error).toContain("tool_name")
+  })
+})