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