fix: OCP code-audit P1+P2 hardening (crash bugs + multi-tenant gates) (#106)

* fix(daemon): P1-1 guard proc.stdin against EPIPE crash

In spawnClaudeProcess, attach an error listener on proc.stdin BEFORE
the write/end calls so an EPIPE (child closed stdin mid-write) is
swallowed and logged rather than thrown as an unhandled exception.

The existing proc.on("error") listener is on the ChildProcess object,
NOT on the stdin Writable — it does not catch stdin write errors.

Hardening per OCP code audit; entry-surface contract unchanged for
single-user default path.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(tui): P1-3 remove tool_use from isTerminalLine; only turn_duration is terminal

In interactive TUI mode, stop_reason=tool_use does NOT mean the turn is
complete. Claude handles the tool call internally and continues generating —
the transcript advances to another assistant entry. Treating tool_use as
terminal truncated tool-using turns mid-flight.

Only {type:"system", subtype:"turn_duration"} is the authoritative
completion marker (claude CLI v2.1.157+ interactive session transcript).

Updated two unit tests that previously asserted tool_use → true; they now
assert false (the correct behaviour). The real-fixture terminal detection
test is unaffected because the fixture uses turn_duration.

Hardening per OCP code audit; TUI path behaviour fix.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(tui): P2-5 add -l (literal) flag to send-keys prompt paste

A prompt that equals a tmux key token (e.g. "C-c", "Escape") would be
interpreted as that key binding rather than typed as literal text.

The -l flag forces literal character-by-character input. The separate
Enter key event afterward deliberately omits -l so tmux sends a real
carriage-return keypress to submit the prompt line.

Authority: tmux send-keys(1) § -l flag. Hardening per OCP code audit.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(audit): String-coerce parsed.error + stale tool_use comment (review fast-follow)

Folds the 2 minor findings from the independent review of the audit fixes:
- String(parsed.error) before .slice/message in callClaude + callClaudeStreaming
  (defensive: claude could emit a non-string result/error_message).
- correct the readTuiTranscript comment that still listed tool_use as terminal.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

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

lib/tui/session.mjs

@@ -223,9 +223,15 @@ export async function runTuiTurn({
 
     // 2. Submit prompt body via `"$(cat file)"` — byte-safe for any content —
     //    then settle, then send a SEPARATE Enter key event to submit the line.
+    //
+    //    The `-l` (literal) flag is required on the paste send-keys call so that
+    //    a prompt that happens to equal a tmux key token (e.g. "C-c", "Escape")
+    //    is typed literally as text rather than being interpreted as a key binding.
+    //    The SEPARATE Enter event below deliberately omits -l so that tmux sends a
+    //    real keypress (carriage return) to submit the prompt line.
     spawnSync(
       "sh",
-      ["-c", `${shq(TMUX)} send-keys -t ${shq(tmuxName)} -- "$(cat ${shq(promptFile)})"`],
+      ["-c", `${shq(TMUX)} send-keys -t ${shq(tmuxName)} -l -- "$(cat ${shq(promptFile)})"`],
       { env, encoding: "utf8" },
     );
     await sleep(PASTE_SETTLE_MS);

lib/tui/transcript.mjs

@@ -52,12 +52,18 @@ export function parseTranscriptLines(text) {
 }
 
 // A line marks the assistant turn complete when it is the turn_duration system
-// event, or an assistant message that stopped to hand off to a tool.
+// event. That is the ONLY reliable terminal marker in interactive TUI mode.
+//
+// Why tool_use is NOT a terminal marker:
+//   In interactive claude, when the model decides to call a tool (stop_reason=
+//   "tool_use"), claude handles the tool call internally and then continues
+//   generating — the turn is NOT complete. The transcript advances to another
+//   assistant entry after the tool result. Only {type:"system",
+//   subtype:"turn_duration"} signals that claude has fully finished the turn.
+//   Treating tool_use as terminal would truncate tool-using turns mid-flight.
 export function isTerminalLine(obj) {
   if (!obj || typeof obj !== "object") return false;
-  if (obj.type === "system" && obj.subtype === "turn_duration") return true;
+  return obj.type === "system" && obj.subtype === "turn_duration";
-  const sr = (obj.message && obj.message.stop_reason) || obj.stop_reason;
-  return sr === "tool_use";
 }
 
 // Text of the LAST assistant turn: concatenate its text content blocks
@@ -96,7 +102,7 @@ export function verifyEntrypoint(events) {
   return null;
 }
 
-// Block until the session transcript is terminal (turn_duration / tool_use) or
+// Block until the session transcript is terminal (turn_duration) or
 // the wall-clock cap elapses, polling the file (no fs.watch — robust over NFS /
 // editors). Returns the latest assistant text. On cap with text, returns the
 // partial text; on cap with no text at all, throws.

server.mjs

@@ -293,10 +293,15 @@ const TUI_CWD  = process.env.OCP_TUI_CWD  || `${process.env.HOME}/.ocp-tui/work`
 const TUI_HOME = process.env.OCP_TUI_HOME  || process.env.HOME;
 const TUI_ENTRYPOINT = process.env.OCP_TUI_ENTRYPOINT || "cli"; // cli|auto|off — see ADR 0007
 
-// SECURITY fail-loud: TUI-mode is incompatible with multi-user auth. Under TUI a
+// SECURITY fail-loud: TUI-mode is incompatible with any configuration that allows
-// guest/anonymous prompt would run interactive claude with the OPERATOR's full
+// non-operator prompts to reach the interactive claude session. Three cases:
-// filesystem access (home is NOT isolation). Refuse to boot until B-path isolation
+//   1. AUTH_MODE=multi — guest/anonymous keys can submit prompts.
-// (tools-off + per-key ephemeral home + sandbox) lands. See ADR 0007.
+//   2. BIND_ADDRESS=0.0.0.0 — server is LAN-exposed; any LAN peer can send prompts
+//      unless per-request trust is in place. Override with OCP_TUI_ALLOW_LAN=1
+//      ONLY if you have a separate network-layer trust (firewall, VPN).
+//   3. PROXY_ANONYMOUS_KEY set — anonymous callers can submit prompts without a key.
+// In all three cases TUI runs interactive claude with the OPERATOR's full filesystem
+// access — home is NOT isolation. Refuse to boot. See ADR 0007.
 if (TUI_MODE && AUTH_MODE === "multi") {
   console.error(
     "FATAL: CLAUDE_TUI_MODE=true is incompatible with CLAUDE_AUTH_MODE=multi.\n" +
@@ -306,6 +311,25 @@ if (TUI_MODE && AUTH_MODE === "multi") {
   );
   process.exit(1);
 }
+if (TUI_MODE && BIND_ADDRESS === "0.0.0.0" && process.env.OCP_TUI_ALLOW_LAN !== "1") {
+  console.error(
+    "FATAL: CLAUDE_TUI_MODE=true with CLAUDE_BIND=0.0.0.0 is unsafe.\n" +
+    "  TUI runs interactive claude with operator filesystem access; LAN-exposed without\n" +
+    "  per-request isolation means any LAN peer could drive the operator's claude session.\n" +
+    "  Either bind to 127.0.0.1 (default) or set OCP_TUI_ALLOW_LAN=1 if you have a\n" +
+    "  separate network-layer trust (firewall/VPN). See docs/adr/0007-tui-interactive-mode.md."
+  );
+  process.exit(1);
+}
+if (TUI_MODE && PROXY_ANONYMOUS_KEY) {
+  console.error(
+    "FATAL: CLAUDE_TUI_MODE=true with PROXY_ANONYMOUS_KEY set is unsafe.\n" +
+    "  TUI runs interactive claude with operator filesystem access; anonymous callers\n" +
+    "  could drive the operator's claude session without a named key.\n" +
+    "  Remove PROXY_ANONYMOUS_KEY or disable TUI-mode. See docs/adr/0007-tui-interactive-mode.md."
+  );
+  process.exit(1);
+}
 
 if (PROXY_ANONYMOUS_KEY && AUTH_MODE !== "multi") {
   console.warn("WARNING: PROXY_ANONYMOUS_KEY is set but AUTH_MODE is not 'multi' — anonymous key will be ignored");
@@ -566,7 +590,27 @@ function buildCliArgs(cliModel, systemPrompt) {
   ];
 
   // Permissions
-  if (SKIP_PERMISSIONS) {
+  // ADR 0007 B-path: in multi-tenant mode, suppress operator-FS tools so a guest
+  // prompt cannot drive Bash/Read/Write/Edit/etc. on the operator's filesystem.
+  // For AUTH_MODE !== "multi" (none/shared — single-operator/trusted), preserve
+  // existing behaviour unchanged.
+  if (AUTH_MODE === "multi") {
+    // Disallow the full operator-FS + web + agent surface. "--disallowedTools" may
+    // be repeated; claude accepts multiple occurrences (TUI path already uses it).
+    args.push(
+      "--disallowedTools", "Bash",
+      "--disallowedTools", "Read",
+      "--disallowedTools", "Write",
+      "--disallowedTools", "Edit",
+      "--disallowedTools", "Glob",
+      "--disallowedTools", "Grep",
+      "--disallowedTools", "WebFetch",
+      "--disallowedTools", "WebSearch",
+      "--disallowedTools", "Agent",
+      "--disallowedTools", "mcp__*",
+    );
+    // Do NOT push --allowedTools in multi mode.
+  } else if (SKIP_PERMISSIONS) {
     args.push("--dangerously-skip-permissions");
   } else if (ALLOWED_TOOLS.length > 0) {
     args.push("--allowedTools", ...ALLOWED_TOOLS);
@@ -730,6 +774,11 @@ function spawnClaudeProcess(model, messages, conversationId, keyName) {
     }
   }
 
+  // Guard stdin writes against EPIPE (child may close stdin before we finish
+  // writing, e.g. early exit on bad model). The ChildProcess "error" event is on
+  // the spawned process, NOT on the stdin Writable — it does not catch this.
+  proc.stdin.on("error", (e) => logEvent("warn", "stdin_write_error", { error: e.message }));
+
   // Write prompt to stdin immediately
   proc.stdin.write(prompt);
   proc.stdin.end();
@@ -794,7 +843,7 @@ function callClaude(model, messages, conversationId, keyName) {
           resultEventSeen = true;
         } else if (parsed.error) {
           // is_error result — treat as process error
-          reject(new Error(parsed.error));
+          reject(new Error(String(parsed.error)));
         }
       }
     });
@@ -916,6 +965,10 @@ function callClaudeStreaming(model, messages, conversationId, res, authInfo = {}
   let lineBuffer = "";
   let isFirstDelta = true;
   let resultEventSeen = false;
+  // Separate flag for is_error result — must NOT be conflated with resultEventSeen.
+  // If errored===true the close handler must not cache the response or record success
+  // (mirrors callClaude which rejects and never caches on is_error).
+  let errored = false;
 
   function ensureHeaders() {
     if (res.writableEnded || res.destroyed) return false;
@@ -977,12 +1030,14 @@ function callClaudeStreaming(model, messages, conversationId, res, authInfo = {}
         }
 
       } else if (parsed.error) {
-        // is_error result — emit error stop
+        // is_error result — emit error stop; do NOT set resultEventSeen (that would
-        resultEventSeen = true;
+        // cause the close handler to record success + write cache). Set errored instead.
-        logEvent("error", "claude_result_error", { model: cliModel, error: parsed.error.slice(0, 200) });
+        errored = true;
-        trackError(parsed.error.slice(0, 200));
+        const errStr = String(parsed.error);
+        logEvent("error", "claude_result_error", { model: cliModel, error: errStr.slice(0, 200) });
+        trackError(errStr.slice(0, 200));
         if (!headersSent && !res.writableEnded && !res.destroyed) {
-          jsonResponse(res, 500, { error: { message: parsed.error, type: "provider_error" } });
+          jsonResponse(res, 500, { error: { message: errStr, type: "provider_error" } });
         } else if (!res.writableEnded && !res.destroyed) {
           sendSSE(res, {
             id, object: "chat.completion.chunk", created, model,
@@ -1005,13 +1060,17 @@ function callClaudeStreaming(model, messages, conversationId, res, authInfo = {}
 
     // Tolerate null exit code when result event was seen (sandbox-wrap noise, same
     // as OLP commit 2864275 — bwrap shell exits null after model completes).
-    if (code !== 0 && !resultEventSeen) {
+    // Also route to the error path when errored===true (is_error result received):
+    // never record success or write cache for an errored response.
+    if ((code !== 0 && !resultEventSeen) || errored) {
       recordModelError(cliModel, false);
       try { recordUsage({ keyId: authInfo.keyId, keyName: authInfo.keyName, model, promptChars: messages.reduce((a, m) => a + (typeof m.content === "string" ? m.content.length : JSON.stringify(m.content).length), 0), responseChars: 0, elapsedMs: elapsed, success: false }); } catch (e) { logEvent("error", "usage_record_failed", { error: e.message }); }
-      logEvent("error", "claude_exit", { model: cliModel, code, signal: signal || "none", elapsed, stderr: stderr.slice(0, 300) });
+      logEvent("error", "claude_exit", { model: cliModel, code, signal: signal || "none", elapsed, errored, stderr: stderr.slice(0, 300) });
       trackError(stderr.slice(0, 300) || `claude exit ${code}`);
       handleSessionFailure();
 
+      // If the error was already sent inline (parsed.error branch above), the
+      // response may be writableEnded — nothing more to send.
       if (!headersSent && !res.writableEnded && !res.destroyed) {
         jsonResponse(res, 500, { error: { message: stderr.slice(0, 300) || `claude exit ${code}`, type: "proxy_error" } });
       } else if (!res.writableEnded && !res.destroyed) {
@@ -1027,7 +1086,7 @@ function callClaudeStreaming(model, messages, conversationId, res, authInfo = {}
       breakerRecordSuccess(cliModel);
       try { recordUsage({ keyId: authInfo.keyId, keyName: authInfo.keyName, model, promptChars: messages.reduce((a, m) => a + (typeof m.content === "string" ? m.content.length : JSON.stringify(m.content).length), 0), responseChars: totalChars, elapsedMs: elapsed, success: true }); } catch (e) { logEvent("error", "usage_record_failed", { error: e.message }); }
       logEvent("info", "claude_ok", { model: cliModel, chars: totalChars, elapsed, session: convId ? convId.slice(0, 12) + "..." : "none" });
-      // Cache write-back for streaming
+      // Cache write-back for streaming — only on true success (not errored)
       if (CACHE_TTL > 0 && authInfo.cacheHash) {
         try { setCachedResponse(authInfo.cacheHash, model, cachedContent); } catch (e) { logEvent("error", "cache_write_failed", { error: e.message }); }
       }
@@ -1507,9 +1566,16 @@ async function handleSettings(req, res) {
 
   // PATCH
   let body = "";
-  for await (const chunk of req) {
+  try {
-    body += chunk;
+    for await (const chunk of req) {
-    if (body.length > 10000) return jsonResponse(res, 413, { error: "Body too large" });
+      body += chunk;
+      if (body.length > 10000) return jsonResponse(res, 413, { error: "Body too large" });
+    }
+  } catch (e) {
+    if (!res.headersSent && !res.writableEnded) {
+      try { return jsonResponse(res, 400, { error: { message: "request aborted", type: "invalid_request_error" } }); } catch {}
+    }
+    return;
   }
   let updates;
   try { updates = JSON.parse(body); } catch { return jsonResponse(res, 400, { error: "Invalid JSON" }); }
@@ -1546,11 +1612,18 @@ const VALID_MODELS = new Set(Object.keys(MODEL_MAP));
 
 async function handleChatCompletions(req, res) {
   let body = "";
-  for await (const chunk of req) {
+  try {
-    body += chunk;
+    for await (const chunk of req) {
-    if (body.length > MAX_BODY_SIZE) {
+      body += chunk;
-      return jsonResponse(res, 413, { error: { message: "Request body too large (max 5MB)", type: "invalid_request_error" } });
+      if (body.length > MAX_BODY_SIZE) {
+        return jsonResponse(res, 413, { error: { message: "Request body too large (max 5MB)", type: "invalid_request_error" } });
+      }
     }
+  } catch (e) {
+    if (!res.headersSent && !res.writableEnded) {
+      try { return jsonResponse(res, 400, { error: { message: "request aborted", type: "invalid_request_error" } }); } catch {}
+    }
+    return;
   }
 
   let parsed;
@@ -1796,6 +1869,12 @@ const server = createServer(async (req, res) => {
   req._authKeyName = authKeyName;
   req._authKeyId = authKeyId;
 
+  // isAdmin computed here (early, before any admin-gated handler) so that
+  // DELETE /sessions, GET /logs, GET /usage, GET /status, PATCH /settings
+  // can all gate on it.  Localhost and explicit admin key are always admin;
+  // in multi-tenant mode only the "admin" named key qualifies.
+  const isAdmin = AUTH_MODE !== "multi" || authKeyName === "admin" || isLocalhost;
+
   // GET /v1/models
   if (req.url === "/v1/models" && req.method === "GET") {
     return jsonResponse(res, 200, {
@@ -1857,15 +1936,17 @@ const server = createServer(async (req, res) => {
     });
   }
 
-  // DELETE /sessions — clear all sessions
+  // DELETE /sessions — clear all sessions (mutating; admin only)
   if (req.url === "/sessions" && req.method === "DELETE") {
+    if (!isAdmin) return jsonResponse(res, 403, { error: { message: "admin only", type: "auth_error" } });
     const count = sessions.size;
     sessions.clear();
     return jsonResponse(res, 200, { cleared: count });
   }
 
-  // GET /sessions — list active sessions
+  // GET /sessions — list active sessions (operator data; admin only)
   if (req.url === "/sessions" && req.method === "GET") {
+    if (!isAdmin) return jsonResponse(res, 403, { error: { message: "admin only", type: "auth_error" } });
     const list = [];
     for (const [id, s] of sessions) {
       // id is "${keyName}|${conversationId}"; expose only the public-facing conversationId
@@ -1875,34 +1956,45 @@ const server = createServer(async (req, res) => {
     return jsonResponse(res, 200, { sessions: list });
   }
 
-  // GET /usage — fetch plan usage limits from Anthropic API
+  // GET /usage — fetches plan usage from Anthropic API with operator token; admin only
   if (req.url === "/usage" && req.method === "GET") {
+    if (!isAdmin) return jsonResponse(res, 403, { error: { message: "admin only", type: "auth_error" } });
     return handleUsage(req, res);
   }
 
-  // GET /logs — recent proxy log entries (errors and key events)
+  // GET /logs — recent proxy log entries (errors and key events); admin only
   if (req.url?.startsWith("/logs") && req.method === "GET") {
+    if (!isAdmin) return jsonResponse(res, 403, { error: { message: "admin only", type: "auth_error" } });
     return handleLogs(req, res);
   }
 
-  // GET /status — combined usage + health summary
+  // GET /status — combined usage + health summary; uses operator token; admin only
   if (req.url === "/status" && req.method === "GET") {
+    if (!isAdmin) return jsonResponse(res, 403, { error: { message: "admin only", type: "auth_error" } });
     return handleStatus(req, res);
   }
 
-  // GET /settings — view current tunable settings
+  // GET /settings — view current tunable settings (admin only)
-  // PATCH /settings — update settings at runtime (JSON body)
+  // PATCH /settings — update settings at runtime (JSON body; admin only, mutating)
   if (req.url === "/settings" && (req.method === "GET" || req.method === "PATCH")) {
+    if (!isAdmin) return jsonResponse(res, 403, { error: { message: "admin only", type: "auth_error" } });
     return handleSettings(req, res);
   }
 
   // ── Key management API ──
-  const isAdmin = AUTH_MODE !== "multi" || authKeyName === "admin" || isLocalhost;
+  // (isAdmin is computed early in the request handler, before the admin-gated routes)
 
   if (req.url === "/api/keys" && req.method === "POST") {
     if (!isAdmin) return jsonResponse(res, 403, { error: "Admin access required" });
     let body = "";
-    for await (const chunk of req) body += chunk;
+    try {
+      for await (const chunk of req) { body += chunk; if (body.length > 10000) return jsonResponse(res, 413, { error: "Body too large" }); }
+    } catch (e) {
+      if (!res.headersSent && !res.writableEnded) {
+        try { return jsonResponse(res, 400, { error: { message: "request aborted", type: "invalid_request_error" } }); } catch {}
+      }
+      return;
+    }
     let parsed;
     try { parsed = JSON.parse(body); } catch { return jsonResponse(res, 400, { error: "Invalid JSON" }); }
     const name = parsed.name || `key-${Date.now()}`;
@@ -1928,7 +2020,14 @@ const server = createServer(async (req, res) => {
     if (!isAdmin) return jsonResponse(res, 403, { error: "Admin access required" });
     const idOrName = decodeURIComponent(req.url.split("/api/keys/")[1].replace("/quota", ""));
     let body = "";
-    for await (const chunk of req) { body += chunk; if (body.length > 10000) return jsonResponse(res, 413, { error: "Body too large" }); }
+    try {
+      for await (const chunk of req) { body += chunk; if (body.length > 10000) return jsonResponse(res, 413, { error: "Body too large" }); }
+    } catch (e) {
+      if (!res.headersSent && !res.writableEnded) {
+        try { return jsonResponse(res, 400, { error: { message: "request aborted", type: "invalid_request_error" } }); } catch {}
+      }
+      return;
+    }
     let quotaBody;
     try { quotaBody = JSON.parse(body); } catch { return jsonResponse(res, 400, { error: "Invalid JSON" }); }
     // Validate quota values: must be positive integers or null
@@ -2032,6 +2131,20 @@ const server = createServer(async (req, res) => {
 });
 
 
+// ── Process-level safety nets ────────────────────────────────────────────
+// Prevent unhandled async rejections and synchronous exceptions from crashing
+// the daemon. Each registers once at module level so they are installed before
+// the first request arrives. These are global no-ops on the happy path.
+process.on("unhandledRejection", (e) =>
+  logEvent("error", "unhandled_rejection", { error: e && e.message ? e.message : String(e) })
+);
+process.on("uncaughtException", (e) =>
+  logEvent("error", "uncaught_exception", { error: e && e.message ? e.message : String(e) })
+);
+// Destroy the socket on low-level HTTP parse errors so broken connections
+// don't accumulate as open file descriptors.
+server.on("clientError", (err, socket) => { try { socket.destroy(); } catch {} });
+
 // ── Graceful shutdown ────────────────────────────────────────────────────
 let shuttingDown = false;
 

test-features.mjs

@@ -1309,11 +1309,11 @@ test("parseTranscriptLines skips blank + malformed/partial lines", () => {
 test("isTerminalLine true on turn_duration", () => {
   assert.equal(isTerminalLine({ type: "system", subtype: "turn_duration" }), true);
 });
-test("isTerminalLine true on stop_reason tool_use (message-wrapped)", () => {
+test("isTerminalLine false on stop_reason tool_use (message-wrapped) — tool_use is mid-turn in TUI mode", () => {
-  assert.equal(isTerminalLine({ type: "assistant", message: { stop_reason: "tool_use" } }), true);
+  assert.equal(isTerminalLine({ type: "assistant", message: { stop_reason: "tool_use" } }), false);
 });
-test("isTerminalLine true on stop_reason tool_use (flat)", () => {
+test("isTerminalLine false on stop_reason tool_use (flat) — claude continues after tool, turn not done", () => {
-  assert.equal(isTerminalLine({ stop_reason: "tool_use" }), true);
+  assert.equal(isTerminalLine({ stop_reason: "tool_use" }), false);
 });
 test("isTerminalLine false on ordinary assistant text line", () => {
   assert.equal(isTerminalLine({ type: "assistant", message: { content: [{ type: "text", text: "hi" }] } }), false);