fix(worker): wrap Claude CLI in PTY to fix stdout buffering hang

[j-bloggs]

Summary

• Replace script -qfc shell-string approach with pty-process crate for PTY allocation
• Revert to Command::arg() chaining (injection-safe by construction via execve)
• Eliminates all shell injection surfaces: prompt, model, session_id
• Keep stderr on separate pipe to prevent NDJSON parse breakage
• Gate PTY behind #[cfg(unix)] with direct-spawn fallback for Windows CI
• Remove shell_escape() function and tests (no longer needed)

Change Type

• Bug fix (non-breaking change which fixes an issue)

Linked Issue

Addresses review feedback from zmanian and gemini-code-assist on this PR.

Validation

• cargo fmt --all -- --check: pass
• cargo clippy (all-features, default, libsql): pass (zero warnings)
• cargo test --lib -- claude_bridge: 23 tests pass
• pty-process crate: MIT licensed, depends on nix (MIT)

Security Impact

• Eliminates entire class of shell injection vulnerabilities by removing the shell interpretation layer
• Previously: prompt, model name, and session_id were interpolated into a shell string for script -qfc
• Now: all arguments passed via execve (Command::arg), never shell-interpreted

Database Impact

None

Blast Radius

• New dependency: pty-process 0.5 (MIT, Unix only via cfg gate)
• Claude bridge stdout now read from PTY master instead of child.stdout
• Windows builds use non-PTY fallback (compile-only, bridge runs in Linux Docker)

Rollback Plan

Revert to staging Command::new("claude").arg() without PTY (re-introduces buffering hang but eliminates security risk).

Review Track

Track C - runtime changes in src/worker/, new dependency

Feature Parity

No FEATURE_PARITY.md changes needed.

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a critical bug where the Claude bridge would hang due to Node.js full stdout buffering when interacting with the Claude CLI via non-TTY pipes. By introducing a pseudo-terminal wrapper, the output is now line-buffered, ensuring proper streaming. Concurrently, this change enhances security by implementing necessary shell escaping for user-provided inputs, mitigating potential command injection risks within the worker.

Highlights

• PTY Allocation: The Claude CLI invocation is now wrapped in script -qfc to allocate a pseudo-terminal (PTY).
• Stdout Buffering Fix: This change resolves an issue where Node.js full-buffered stdout on non-TTY pipes, which previously caused the Claude bridge to hang indefinitely.
• Shell Escaping: Robust shell escaping has been implemented for the prompt and session ID to prevent command injection vulnerabilities.
• Regression Test: A new test case (prompt_shell_escaping) was added to validate the correctness and security of the shell escaping mechanism.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This pull request refactors the execution of the claude command by wrapping it in script -qfc to allocate a PTY, addressing Node.js stdout buffering issues. It also introduces shell escaping for the prompt argument to prevent command injection, and a new test prompt_shell_escaping to verify this. The review identifies a potential command injection vulnerability with resume_session_id due to a lack of proper escaping and suggests adding test cases for resume_session_id escaping to improve robustness.

[zmanian]

Review: fix(worker): wrap Claude CLI in PTY to fix stdout buffering hang

PTY is the right fix for Node.js stdout buffering, but the implementation introduces security risk.

Critical

1. Shell injection via unescaped self.config.model: Interpolated directly into the shell command string. Must be escaped with shell_escape() like prompt and session_id are.

High

2. Security regression from Command::arg() to shell string: Previous code passed args via execve (injection-safe by construction). New code goes through script -qfc which invokes /bin/sh -c. Consider using the pty-process crate for PTY allocation while keeping Command::arg().

Medium

3. script -qfc is GNU-only: macOS script has different syntax. Documented as Docker-only but a maintenance trap for local testing.

4. stdout/stderr separation through PTY: script can merge streams. Verify NDJSON parsing still works when stderr lines from Claude CLI are potentially interleaved into stdout.

Positive

• Thorough escaping tests covering quotes, metacharacters, adversarial session IDs.

Fix the model escaping at minimum. Strongly consider the PTY crate approach to avoid the shell interpretation layer entirely.

[zmanian]

Re-review: shell injection via script -qfc is fully resolved -- replaced with pty-process crate using execve directly. No shell involved. Arguments passed via Command::arg(). Stderr kept separate from PTY. Excellent fix. Approve.