fix: typing UX — input/output separation + cleaner hint line#99
Conversation
… word-jump
Three UX fixes for the interactive prompt:
1. **Typing while agent works shows only one character** — the spinner's `\r`
line-rewrite was overwriting OS-echoed chars in canonical mode. Fixed by
switching stdin to half-raw mode (ICANON+ECHO off, OPOST kept so agent
streaming output isn't broken), managing echo manually, and telling the
spinner to pause its writes while the user is typing (`Terminal.userTyping`
atomic flag checked before each 80 ms tick).
2. **Enter can't interrupt thinking** — `StartQueueReader` now accepts a
`cancelFn` that is called when the user presses Enter with non-empty text.
For the direct API path this cancels the streaming context; for CC/Codex
the new `CLIAgent.Cancel()` method cancels the subprocess context. The
queued text is processed on the very next REPL iteration.
3. **Option+Arrow doesn't jump words on macOS** — macOS Terminal.app sends
ESC+b/f for Option+Left/Right; iTerm2 and xterm-compatible terminals send
`\x1b[1;3D`/`\x1b[1;3C` (`KeyLeft/Right{Alt:true}`). Both sequences now
invoke `previousWordBoundary` / `nextWordBoundary`.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…buf alloc - agent.go: protect all three a.cancel write sites with cancelMu so the CancelCurrent() goroutine can never race with RunStreaming/tool-call/ollama paths (was a data race flagged by security reviewer) - stdin_queue_unix.go: add ESC-sequence state machine so Option+Arrow during agent thinking no longer inserts stray 'b'/'f' characters into the typed line; also pre-allocate the read buffer outside the hot loop; fix utf8.RuneError size==1 check so genuine U+FFFD (size==3) passes through - input.go: mention Opt+←/→ in the keyboard hint line Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the user presses Enter to interrupt the running agent, Cancel() fires context.CancelFunc which sends SIGKILL to the CC/Codex subprocess. cmd.Wait() then returns "signal: killed", which was surfaced to the user as: ✗ claude exited with error: signal: killed Check ctx.Err() before treating a non-zero exit as an error — if the context was already cancelled the kill was intentional, so return whatever output was collected (possibly empty) without an error. The REPL then cleanly picks up the next queued prompt. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously the queue reader echoed each keystroke inline. When the agent
concurrently called StreamText / FinishMarkdown / PrintToolIcon, the writes
landed wherever the cursor happened to be, displacing the typing line into
new rows. FinishMarkdown's \033[1A\033[2K loop could even erase the typed
characters outright.
New approach: print a single static status line on the first keystroke
("⌨ typing — Enter to queue, Ctrl+C to cancel") and silently buffer the
runes with no per-character echo. The user's input has no visual presence
the agent can stomp on, and the full text reveals itself in the existing
"✓ queued [N]: …" confirmation when Enter is pressed.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Desperado
changed the title
![qualitymaxapp[bot]](https://avatars.githubusercontent.com/in/2748338?s=60&v=4){kind=small}
There was a problem hiding this comment.
QualityMax Security Scan
Found 1 security issue(s).
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax security findings in this PR:\n- injection in internal/agent/cc_agent.go:293: The ccSessionID is passed directly as a command-line argument via --resume without validation. If ccSessionID is user-controlled or derived from untrusted sources, it could be exploited for command injection."|
|
||
| // Expose cancel so Cancel() can interrupt from another goroutine. | ||
| a.runMu.Lock() | ||
| a.runCancel = cancel |
There was a problem hiding this comment.
HIGH: The ccSessionID is passed directly as a command-line argument via --resume without validation. If ccSessionID is user-controlled or derived from untrusted sources, it could be exploited for command injection.
Fix: Validate ccSessionID to ensure it matches an expected format (e.g., alphanumeric only). Implement strict whitelist validation before passing to exec.CommandContext.
CWE-78 | A03:2021-Injection
Fix with your LLM agent
<your-llm-agent> "Fix the injection issue in internal/agent/cc_agent.go at line 293: The ccSessionID is passed directly as a command-line argument via --resume without validation. If ccSessionID is user-controlled or derived from untrusted sources, it could be exploited for command injection."
✅ QualityMax BOLA Audit — No new HIGH risk routesNo new RED (missing authorization) findings introduced by this PR. |
|
There was a problem hiding this comment.
QualityMax Security Scan
Found 1 security issue(s).
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax security findings in this PR:\n- path-traversal in internal/agent/cc_agent.go:283: The mcpConfigPath is constructed using os.TempDir() and os.Getpid(), then written with os.WriteFile(). While the path construction itself is safe, the file is created with 0600 permissions in a world-writable temp directory. An attacker could potentially create a symlink at the expected path before the file is written, leading to arbitrary file write."| } | ||
| if ccSessionID != "" { | ||
| if !isSafeCCSessionID(ccSessionID) { | ||
| return "", fmt.Errorf("invalid Claude session ID") |
There was a problem hiding this comment.
HIGH: The mcpConfigPath is constructed using os.TempDir() and os.Getpid(), then written with os.WriteFile(). While the path construction itself is safe, the file is created with 0600 permissions in a world-writable temp directory. An attacker could potentially create a symlink at the expected path before the file is written, leading to arbitrary file write.
Fix: Use os.CreateTemp() instead of os.WriteFile() to atomically create the file with secure permissions, preventing symlink attacks. Example: f, err := os.CreateTemp(os.TempDir(), "qmax-mcp-*.json"); defer f.Close(); f.WriteString(string(data))
CWE-22 | A01:2021-Broken Access Control
Fix with your LLM agent
<your-llm-agent> "Fix the path-traversal issue in internal/agent/cc_agent.go at line 283: The mcpConfigPath is constructed using os.TempDir() and os.Getpid(), then written with os.WriteFile(). While the path construction itself is safe, the file is created with 0600 permissions in a world-writable temp directory. An attacker could potentially create a symlink at the expected path before the file is written, leading to arbitrary file write."
|
There was a problem hiding this comment.
QualityMax Security Scan
Found 1 security issue(s).
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax security findings in this PR:\n- injection in internal/agent/cc_agent.go:330: User-controlled ccSessionID is passed to exec.CommandContext without proper validation. The isSafeCCSessionID() check is called but its implementation is not visible, making it impossible to verify if it prevents command injection."| result := a.parseStream(stdout, term) | ||
|
|
||
| if err := cmd.Wait(); err != nil { | ||
| // Intentional cancel (user pressed Enter to interrupt) — not an error. |
There was a problem hiding this comment.
HIGH: User-controlled ccSessionID is passed to exec.CommandContext without proper validation. The isSafeCCSessionID() check is called but its implementation is not visible, making it impossible to verify if it prevents command injection.
Fix: Ensure isSafeCCSessionID() uses a strict whitelist (alphanumeric + hyphens/underscores only) and validate that the session ID matches expected format before appending to command arguments. Consider using a more restrictive approach such as UUID validation.
CWE-78 | A03:2021-Injection
Fix with your LLM agent
<your-llm-agent> "Fix the injection issue in internal/agent/cc_agent.go at line 330: User-controlled ccSessionID is passed to exec.CommandContext without proper validation. The isSafeCCSessionID() check is called but its implementation is not visible, making it impossible to verify if it prevents command injection."
There was a problem hiding this comment.
QualityMax Security Scan
Found 2 security issue(s).
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax security findings in this PR:\n- path-traversal in internal/agent/cc_agent.go:265: The mcpConfigPath is created in OS temp directory but passed to external 'claude' CLI. If an attacker can predict or influence the temp directory location, they could potentially swap the config file with a malicious one before it's used.\n- injection in internal/agent/cc_agent.go:330: User-controlled 'userMsg' is passed directly to 'claude --print' command without sanitization. An attacker can inject arbitrary command-line arguments or shell metacharacters."| for i, r := range id { | ||
| switch i { | ||
| case 8, 13, 18, 23: | ||
| if r != '-' { |
There was a problem hiding this comment.
HIGH: The mcpConfigPath is created in OS temp directory but passed to external 'claude' CLI. If an attacker can predict or influence the temp directory location, they could potentially swap the config file with a malicious one before it's used.
Fix: While os.CreateTemp() is used (which is good), ensure the file permissions (0600) are set before writing sensitive data. Additionally, validate the file hasn't been modified before passing it to the claude CLI by checking its inode/modification time.
CWE-22 | A01:2021-Broken Access Control
Fix with your LLM agent
<your-llm-agent> "Fix the path-traversal issue in internal/agent/cc_agent.go at line 265: The mcpConfigPath is created in OS temp directory but passed to external 'claude' CLI. If an attacker can predict or influence the temp directory location, they could potentially swap the config file with a malicious one before it's used."|
|
||
| // Expose cancel so Cancel() can interrupt from another goroutine. | ||
| a.runMu.Lock() | ||
| a.runCancel = cancel |
There was a problem hiding this comment.
HIGH: User-controlled 'userMsg' is passed directly to 'claude --print' command without sanitization. An attacker can inject arbitrary command-line arguments or shell metacharacters.
Fix: Use exec.Command with separate arguments array (already done correctly) but validate/sanitize userMsg content. Consider restricting special characters or using a whitelist of allowed input patterns.
CWE-78 | A03:2021-Injection
Fix with your LLM agent
<your-llm-agent> "Fix the injection issue in internal/agent/cc_agent.go at line 330: User-controlled 'userMsg' is passed directly to 'claude --print' command without sanitization. An attacker can inject arbitrary command-line arguments or shell metacharacters."
✅ QualityMax Pipeline
Powered by QualityMax — AI-Powered Test Automation |
# Conflicts: # internal/session/stdin_queue_unix.go # internal/tui/input.go
There was a problem hiding this comment.
QualityMax Security Scan
Found 1 security issue(s).
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax security findings in this PR:\n- injection in internal/agent/cc_agent.go:280: User input 'userMsg' is passed directly to exec.CommandContext as stdin without sanitization. While stdin is generally safer than command-line arguments, if the claude CLI or downstream tools parse stdin unsafely, this could enable command injection or other attacks."| } | ||
| return os.SameFile(expected, current) && | ||
| current.Mode().Perm() == 0600 && | ||
| current.Size() == expected.Size() && |
There was a problem hiding this comment.
HIGH: User input 'userMsg' is passed directly to exec.CommandContext as stdin without sanitization. While stdin is generally safer than command-line arguments, if the claude CLI or downstream tools parse stdin unsafely, this could enable command injection or other attacks.
Fix: Validate and sanitize userMsg before passing to the subprocess. Consider implementing input validation to reject or escape potentially dangerous patterns.
CWE-78 | A03:2021-Injection
Fix with your LLM agent
<your-llm-agent> "Fix the injection issue in internal/agent/cc_agent.go at line 280: User input 'userMsg' is passed directly to exec.CommandContext as stdin without sanitization. While stdin is generally safer than command-line arguments, if the claude CLI or downstream tools parse stdin unsafely, this could enable command injection or other attacks."
There was a problem hiding this comment.
QualityMax Security Scan
Found 1 security issue(s).
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax security findings in this PR:\n- path-traversal in internal/agent/cc_agent.go:335: The mcpPath variable (from WriteMCPConfig) is passed directly to exec.CommandContext as a command-line argument. While WriteMCPConfig creates the file securely, the path itself is not validated before being passed to the claude CLI, which could be exploited if the path contains special characters or is manipulated."| mcpInfo := a.mcpConfigInfo | ||
| a.mu.Unlock() | ||
| if !sameMCPConfigFile(mcpPath, mcpInfo) { | ||
| return "", fmt.Errorf("MCP config changed before claude launch") |
There was a problem hiding this comment.
HIGH: The mcpPath variable (from WriteMCPConfig) is passed directly to exec.CommandContext as a command-line argument. While WriteMCPConfig creates the file securely, the path itself is not validated before being passed to the claude CLI, which could be exploited if the path contains special characters or is manipulated.
Fix: Validate that mcpPath is an absolute path and contains only expected characters before passing to exec.CommandContext. Consider using filepath.Abs() and filepath.Clean().
CWE-22 | A01:2021-Broken Access Control
Fix with your LLM agent
<your-llm-agent> "Fix the path-traversal issue in internal/agent/cc_agent.go at line 335: The mcpPath variable (from WriteMCPConfig) is passed directly to exec.CommandContext as a command-line argument. While WriteMCPConfig creates the file securely, the path itself is not validated before being passed to the claude CLI, which could be exploited if the path contains special characters or is manipulated."
There was a problem hiding this comment.
QualityMax Security Scan
Found 2 security issue(s).
Fix all findings with your LLM agent
<your-llm-agent> "Fix all QualityMax security findings in this PR:\n- path-traversal in internal/agent/cc_agent.go:265: The validateMCPConfigPathForClaude function attempts to validate file paths but uses a character whitelist that may be insufficient. The check allows '/', '.', '_', '-' characters which could potentially be combined to create path traversal sequences like '../' or '/./'.\n- injection in internal/agent/cc_agent.go:330: User input 'userMsg' is passed to claude CLI via stdin without sufficient validation. While sanitizeCCUserPrompt filters some control characters, it does not prevent injection of command-line arguments or special sequences that could be interpreted by the shell or claude CLI."|
|
||
| a.mu.Lock() | ||
| a.mcpConfigPath = path | ||
| a.mcpConfigInfo = info |
There was a problem hiding this comment.
HIGH: The validateMCPConfigPathForClaude function attempts to validate file paths but uses a character whitelist that may be insufficient. The check allows '/', '.', '_', '-' characters which could potentially be combined to create path traversal sequences like '../' or '/./'.
Fix: Instead of character-by-character validation, use filepath.Abs() and filepath.Clean() (already done), then verify the result is within an expected directory. Additionally, ensure the path does not contain '..' sequences: add a check like if strings.Contains(clean, "..") { return "", fmt.Errorf("invalid path") }
CWE-22 | A01:2021-Broken Access Control
Fix with your LLM agent
<your-llm-agent> "Fix the path-traversal issue in internal/agent/cc_agent.go at line 265: The validateMCPConfigPathForClaude function attempts to validate file paths but uses a character whitelist that may be insufficient. The check allows '/', '.', '_', '-' characters which could potentially be combined to create path traversal sequences like '../' or '/./'."|
|
||
| func sanitizeCCUserPrompt(prompt string) (string, error) { | ||
| if strings.ContainsRune(prompt, '\x00') { | ||
| return "", fmt.Errorf("invalid prompt: contains NUL byte") |
There was a problem hiding this comment.
HIGH: User input 'userMsg' is passed to claude CLI via stdin without sufficient validation. While sanitizeCCUserPrompt filters some control characters, it does not prevent injection of command-line arguments or special sequences that could be interpreted by the shell or claude CLI.
Fix: The current approach of passing user input via stdin is relatively safe, but ensure that the claude binary itself cannot be manipulated. Consider using exec.Command with explicit argument arrays (already done correctly here) and verify that stdin-based input cannot trigger unintended CLI behavior. The sanitization function should be reviewed to ensure it blocks all potentially dangerous sequences.
CWE-78 | A03:2021-Injection
Fix with your LLM agent
<your-llm-agent> "Fix the injection issue in internal/agent/cc_agent.go at line 330: User input 'userMsg' is passed to claude CLI via stdin without sufficient validation. While sanitizeCCUserPrompt filters some control characters, it does not prevent injection of command-line arguments or special sequences that could be interpreted by the shell or claude CLI."1 participant
Two typing-related fixes bundled together.
1. Separate user input from agent output in the queue reader
Bug: When the user typed while the agent was working, the agent's concurrent stdout writes (
StreamText,FinishMarkdown,PrintToolIcon) would shove the typing line onto new rows or, worse, erase the typed characters entirely (FinishMarkdowndoes\033[1A\033[2Kper counted newline).Fix: Drop per-character echo. On the first keystroke, print a single static status line — "⌨ typing — Enter to queue, Ctrl+C to cancel" — and silently buffer runes. The user's input has no visual presence the agent can stomp on; the full text reveals itself in the existing "✓ queued [N]: …" confirmation when Enter is pressed.
Trade-off: you can't see typos as you type, but you also can't get the typing line mangled.
2. Hint line tidy-up
Below the slash prompt, the hint now reads:
`Ctrl+O output: compact • ↑↓ history • Ctrl+X×3 clear • Opt+←/→ words • Ctrl+C cancel • / commands`
↑↓ history— was supported but undiscoveredCtrl+←/→— still works, just not surfaced🤖 Generated with Claude Code