fix: OCP code-audit P1+P2 hardening (crash bugs + multi-tenant gates)#106
Merged
Conversation
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>
…n 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>
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>
…ew 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the 3 P1 + 5 P2 findings from the multi-agent OCP code audit (23/24 confirmed after adversarial verification). Independently reviewed: APPROVE-WITH-MINOR (default-path-sacred verified by diffing
buildCliArgs+ theisAdminmove against the parent; 2 minor findings folded).P1 — availability / correctness (the reason to patch before any durable deploy)
proc.stdinhad no'error'listener → an EPIPE from a fast-failing child (auth error, bad model, big prompt) crashed the single-process daemon. Now swallowed+logged.unhandledRejection/uncaughtException/clientErrornets + unguarded body-read loops → a normal client aborting mid-upload crashed the daemon. Added process safety nets + try/catch on all 4 body loops.stop_reason:"tool_use"as turn-complete → silently truncated any TUI turn that used a tool. Now onlyturn_durationis terminal.P2 — security gates / cache integrity
AUTH_MODE=multi, force--disallowedTools(Bash/Read/Write/Edit/…) so a guest prompt can't drive operator-FS tools. Single-user (none/shared) path unchanged./sessions(DELETE),/settings(PATCH),/logs,/usage,/statuswere dispatched before theisAdmincheck → now admin-gated.is_errorresponse as success → cache poisoning. Now anerroredflag routes it torecordModelError, no cache write.none+0.0.0.0(unlessOCP_TUI_ALLOW_LAN=1) and+ PROXY_ANONYMOUS_KEY.send-keyspaste now uses-l(literal) → a prompt equal to a tmux key token (e.g.C-c) is typed, not interpreted.Invariants
Default single-user path (
AUTH_MODE=none, no TUI) is behavior-identical (buildCliArgsdiff confirmed byte-for-byte; the new code is no-op on the happy path).node test-features.mjs→ 136/0.Follow-up (not blocking)
--disallowedToolsdenial on PI231 before any production multi deployment.🤖 Generated with Claude Code