From 58e1bc9d6567bc1a6551a28e0558cf338a174c19 Mon Sep 17 00:00:00 2001 From: Dennison Date: Tue, 8 Sep 2026 22:24:29 -0400 Subject: [PATCH] fix(cli): refuse extra prompt arguments instead of dropping them MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `go-code explain this repo` — the natural unquoted form — ran against `-prompt explain` and silently discarded the rest, exit 0. A truncated prompt often still produces plausible output, so the loss was invisible. The prompt branch assigned `prompt="$1"` and never read `$2` onward, while the CLI branch beside it captures the rest with `cli_args=("$@")` and the flag branches die on anything unknown. Prompt mode was the only path that accepted input and threw part of it away. It now refuses and reconstructs the correct command: unexpected extra argument: this. Quote the whole prompt as one argument, e.g. go-code "explain this repo" Joining the words was considered and rejected — it guesses at intent and silently reinterprets a shell-splitting mistake, where refusing teaches the rule once. Found by the untrusted external reviewer reading only the script, then confirmed by running it. Closes #1435 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01WJGxhoFhA8JjkwZFcLGdS5 --- cmd/harnesscli/go_code_script_test.go | 72 +++++++++++++++++++++++++++ docs/logs/engineering-log.md | 30 +++++++++++ scripts/go-code.sh | 10 ++++ 3 files changed, 112 insertions(+) diff --git a/cmd/harnesscli/go_code_script_test.go b/cmd/harnesscli/go_code_script_test.go index 30846ab8..4a5a8583 100644 --- a/cmd/harnesscli/go_code_script_test.go +++ b/cmd/harnesscli/go_code_script_test.go @@ -528,3 +528,75 @@ func describeStuckProcesses(wrapperPID int) string { } return b.String() } + +// TestGoCodeScriptRejectsExtraPromptArguments pins issue #1435: the wrapper +// must not silently discard part of the user's prompt. +// +// `prompt="$1"` took only the first positional, so `go-code explain this repo` +// — the natural unquoted form — ran against `-prompt explain` and threw the +// rest away with no warning and exit 0. The run looked successful and a short +// prompt often still produces plausible output, so the truncation could go +// unnoticed indefinitely. +// +// The assertion is on the user-visible consequence: either the whole prompt +// reaches harnesscli, or the wrapper refuses and says why. +func TestGoCodeScriptRejectsExtraPromptArguments(t *testing.T) { + scriptPath, err := filepath.Abs(filepath.Join("..", "..", "scripts", "go-code.sh")) + if err != nil { + t.Fatalf("resolve go-code script path: %v", err) + } + + newStubs := func(t *testing.T, recordFile string) string { + t.Helper() + binDir := t.TempDir() + writeExecutable(t, filepath.Join(binDir, "curl"), "#!/usr/bin/env bash\nexit 0\n") + writeExecutable(t, filepath.Join(binDir, "harnessd"), "#!/usr/bin/env bash\nexit 0\n") + writeExecutable(t, filepath.Join(binDir, "harnesscli"), "#!/usr/bin/env bash\nprintf '%s\\n' \"$*\" >> \"$RECORD_FILE\"\n") + return binDir + } + + t.Run("extra positional arguments are refused", func(t *testing.T) { + tmp := t.TempDir() + recordFile := filepath.Join(tmp, "harnesscli.args") + binDir := newStubs(t, recordFile) + + cmd := exec.Command("bash", scriptPath, "explain this repo", "and also this") + cmd.Env = append(os.Environ(), + "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "HARNESS_ADDR=:19910", "RECORD_FILE="+recordFile) + out, err := cmd.CombinedOutput() + + if err == nil { + t.Fatalf("expected a non-zero exit when extra prompt arguments are given, got success\n%s", out) + } + if raw, statErr := os.ReadFile(recordFile); statErr == nil && len(raw) > 0 { + t.Fatalf("harnesscli was invoked with a truncated prompt instead of the wrapper refusing: %q", raw) + } + if !bytes.Contains(out, []byte("and also this")) { + t.Errorf("refusal should name the offending argument so the user can see what was dropped, got:\n%s", out) + } + }) + + // Control: the normal path must keep working, or a fix that refuses + // everything would satisfy the assertion above. + t.Run("a single quoted prompt still passes through whole", func(t *testing.T) { + tmp := t.TempDir() + recordFile := filepath.Join(tmp, "harnesscli.args") + binDir := newStubs(t, recordFile) + + cmd := exec.Command("bash", scriptPath, "explain this repo") + cmd.Env = append(os.Environ(), + "PATH="+binDir+string(os.PathListSeparator)+os.Getenv("PATH"), + "HARNESS_ADDR=:19911", "RECORD_FILE="+recordFile) + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("single quoted prompt should succeed: %v\n%s", err, out) + } + raw, err := os.ReadFile(recordFile) + if err != nil { + t.Fatalf("read record file: %v", err) + } + if !bytes.Contains(raw, []byte("-prompt explain this repo")) { + t.Errorf("whole prompt should reach harnesscli, got %q", raw) + } + }) +} diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index 2aa892d2..44cf43d9 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -1,5 +1,35 @@ # Engineering Log +## 2026-09-08 — Issue #1435 go-code silently dropped prompt arguments + +- Symptom: `go-code explain this repo` — the natural unquoted form — ran against + `-prompt explain` and discarded the rest, with no warning and exit 0. Proven + with stubs: `go-code "explain this repo" "and also this"` reached harnesscli + as `-prompt explain this repo`. +- Cause: the prompt branch of the argument `case` assigned `prompt="$1"` and + never read `$2` onward. The CLI branch beside it captures the rest with + `cli_args=("$@")`, and the flag branches `die` on anything unknown — prompt + mode was the only path that accepted input and threw part of it away. +- Why it stayed hidden: the run looks successful, and a truncated prompt often + still produces plausible output, so nothing signals the loss. No test covered + multi-argument prompt mode. +- Fix: refuse, and name the fix. `go-code explain this repo` now exits non-zero + with `unexpected extra argument: this. Quote the whole prompt as one argument, + e.g. go-code "explain this repo"` — the message reconstructs the correct + command rather than only complaining. +- Joining the arguments was considered and rejected: it guesses at intent and + silently reinterprets a shell-splitting mistake, where refusing teaches the + rule once. +- Test: `TestGoCodeScriptRejectsExtraPromptArguments`, with a control asserting + a single quoted prompt still passes through whole — otherwise a fix that + refused everything would satisfy the first assertion. +- Provenance: found by the untrusted external reviewer (`gpt-6-astra` via the + Surplus proxy) reading only the script, then confirmed by running the stub + reproduction rather than by reading. Six findings on this file, of which two + were confirmed, three were legitimate low-severity hardening (PID reuse in + `stop_server`, predictable tmp paths, unvalidated `HARNESS_ADDR`), and one was + rejected as speculative. + ## 2026-09-08 — Issue #1432 wide labels destroyed the cancel hint - Symptom: `shortenLabel` guarantees the `(esc to interrupt)` hint survives at diff --git a/scripts/go-code.sh b/scripts/go-code.sh index 3c196f3f..5e4274a8 100755 --- a/scripts/go-code.sh +++ b/scripts/go-code.sh @@ -405,6 +405,16 @@ main() { *) mode="prompt" prompt="$1" + shift + # Refuse rather than silently truncate. `prompt="$1"` alone discarded + # every later positional, so the natural unquoted form — + # `go-code explain this repo` — ran against `-prompt explain` and threw + # the rest away with exit 0. A short prompt still produces plausible + # output, so the loss was invisible. Joining the words instead would guess + # at intent; refusing teaches the rule once. Issue #1435. + if [[ $# -gt 0 ]]; then + die "unexpected extra argument: $1. Quote the whole prompt as one argument, e.g. go-code \"${prompt} $*\"" + fi ;; esac