Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import "strings"
// Backend abstracts an AI command backend (Claude, Codex, etc.).
type Backend interface {
// BuildCommand constructs the shell command string to execute.
BuildCommand(baseCmd, extraFlags, prompt string) string
// The prompt is intentionally not part of this string - it is passed to the
// process via stdin so that untrusted candidate/prompt content is never
// parsed by the shell (see RunAICommand).
BuildCommand(baseCmd, extraFlags string) string
// ProcessLine parses one line of JSON output from the backend.
// Returns text to stream to the terminal/log, and whether the session is complete.
ProcessLine(line string) (streamText string, sessionDone bool)
Expand Down
11 changes: 5 additions & 6 deletions src/backend_claude.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,16 +30,15 @@ type ClaudeBackend struct {
messageHasContent bool
}

func (b *ClaudeBackend) BuildCommand(baseCmd, extraFlags, prompt string) string {
const delimiter = "__NIGEL_PROMPT_EOF__"
func (b *ClaudeBackend) BuildCommand(baseCmd, extraFlags string) string {
jsonFlags := "--print --output-format stream-json --include-partial-messages --verbose"

// "-p" with no argument makes claude read the prompt from stdin, which
// RunAICommand supplies directly - the prompt never passes through the shell.
if extraFlags != "" {
return fmt.Sprintf("%s %s %s -p <<'%s'\n%s\n%s",
baseCmd, jsonFlags, extraFlags, delimiter, prompt, delimiter)
return fmt.Sprintf("%s %s %s -p", baseCmd, jsonFlags, extraFlags)
}
return fmt.Sprintf("%s %s -p <<'%s'\n%s\n%s",
baseCmd, jsonFlags, delimiter, prompt, delimiter)
return fmt.Sprintf("%s %s -p", baseCmd, jsonFlags)
}

func (b *ClaudeBackend) ProcessLine(line string) (string, bool) {
Expand Down
13 changes: 5 additions & 8 deletions src/backend_codex.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,21 +21,18 @@ type codexItem struct {
// CodexBackend implements Backend for the OpenAI Codex CLI.
type CodexBackend struct{}

func (b *CodexBackend) BuildCommand(baseCmd, extraFlags, prompt string) string {
const delimiter = "__NIGEL_PROMPT_EOF__"
func (b *CodexBackend) BuildCommand(baseCmd, extraFlags string) string {
cmd := strings.TrimSpace(baseCmd)
if cmd == "codex" {
cmd = "codex exec"
}

// codex exec --json reads the prompt from stdin when using "-"
// Heredoc avoids shell quoting issues
// "codex exec --json -" reads the prompt from stdin, which RunAICommand
// supplies directly - the prompt never passes through the shell.
if extraFlags != "" {
return fmt.Sprintf("%s --json %s - <<'%s'\n%s\n%s",
cmd, extraFlags, delimiter, prompt, delimiter)
return fmt.Sprintf("%s --json %s -", cmd, extraFlags)
}
return fmt.Sprintf("%s --json - <<'%s'\n%s\n%s",
cmd, delimiter, prompt, delimiter)
return fmt.Sprintf("%s --json -", cmd)
}

func (b *CodexBackend) ProcessLine(line string) (string, bool) {
Expand Down
18 changes: 14 additions & 4 deletions src/backend_codex_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,25 @@ import "strings"
import "testing"

func TestCodexBuildCommandUsesExecForBareCodex(t *testing.T) {
cmd := (&CodexBackend{}).BuildCommand("codex", "", "hello")
if !strings.HasPrefix(cmd, "codex exec --json - <<") {
cmd := (&CodexBackend{}).BuildCommand("codex", "")
if cmd != "codex exec --json -" {
t.Fatalf("BuildCommand() = %q, want codex exec invocation", cmd)
}
}

func TestCodexBuildCommandDoesNotDoubleExec(t *testing.T) {
cmd := (&CodexBackend{}).BuildCommand("codex exec", "--sandbox read-only", "hello")
if !strings.HasPrefix(cmd, "codex exec --json --sandbox read-only - <<") {
cmd := (&CodexBackend{}).BuildCommand("codex exec", "--sandbox read-only")
if cmd != "codex exec --json --sandbox read-only -" {
t.Fatalf("BuildCommand() = %q, want existing codex exec invocation preserved", cmd)
}
}

func TestCodexBuildCommandExcludesPromptFromShellString(t *testing.T) {
// The prompt must never be embedded in the returned shell string - it is
// sent via stdin instead, so untrusted candidate content can't be parsed
// by the shell (see the heredoc-injection issue this replaces).
cmd := (&CodexBackend{}).BuildCommand("codex", "")
if strings.Contains(cmd, "<<") {
t.Fatalf("BuildCommand() = %q, should not contain a heredoc", cmd)
}
}
11 changes: 8 additions & 3 deletions src/executor.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,18 +61,23 @@ func KillRunningProcess() {
// The streamCb callback is invoked for each chunk of text received.
// Returns the accumulated output (for rate limit detection) and any error.
func RunAICommand(backend Backend, baseCmd, extraFlags, prompt, workDir string, logWriter io.Writer, timeout time.Duration, streamCb StreamCallback) (string, error) {
// Build the command via the backend
cmdStr := backend.BuildCommand(baseCmd, extraFlags, prompt)
// Build the command via the backend. The prompt is deliberately excluded
// from this string - it previously was interpolated into a shell heredoc,
// which let candidate/prompt content containing a line matching the
// heredoc delimiter break out of the heredoc and inject arbitrary shell
// commands. Passing it via Stdin below avoids the shell parsing it at all.
cmdStr := backend.BuildCommand(baseCmd, extraFlags)

// Log the exact command being executed (for debugging hangs)
if logWriter != nil {
fmt.Fprintf(logWriter, "Command: %s\n", cmdStr)
fmt.Fprintf(logWriter, "Command: %s\nPrompt (sent via stdin):\n%s\n", cmdStr, prompt)
}

args := []string{"-c", cmdStr}

cmd := exec.Command("bash", args...)
cmd.Dir = workDir
cmd.Stdin = strings.NewReader(prompt)
cmd.SysProcAttr = &syscall.SysProcAttr{
Setpgid: true,
Pdeathsig: syscall.SIGTERM,
Expand Down
51 changes: 50 additions & 1 deletion src/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ func TestRunCommandShowOnFail(t *testing.T) {

type stderrBackend struct{}

func (b stderrBackend) BuildCommand(baseCmd, extraFlags, prompt string) string {
func (b stderrBackend) BuildCommand(baseCmd, extraFlags string) string {
return "echo hidden-reason >&2; exit 7"
}

Expand All @@ -613,3 +613,52 @@ func TestRunAICommandIncludesStderrOnFailure(t *testing.T) {
t.Fatalf("error = %q, want stderr included", err.Error())
}
}

// echoBackend runs "cat" so the test can assert exactly what the process
// received, independent of any AI CLI being installed.
type echoBackend struct{}

func (b echoBackend) BuildCommand(baseCmd, extraFlags string) string {
return "cat"
}

func (b echoBackend) ProcessLine(line string) (string, bool) {
return "", false
}

func (b echoBackend) RateLimitPhrases() []string {
return nil
}

func (b echoBackend) DisplayName() string {
return "Echo"
}

// TestRunAICommandPromptNeverParsedByShell is a regression test for a heredoc
// injection: the prompt used to be interpolated into a `bash -c` string inside
// a `<<'__NIGEL_PROMPT_EOF__'` heredoc. A prompt containing a line that
// happened to match the delimiter closed the heredoc early, so any text after
// it was parsed and executed as a normal shell command. Since candidates (and
// therefore prompt content) can come from untrusted repository data via
// candidate_source/$INPUT, this was a real command injection. The prompt is
// now sent via Stdin instead, so it can never be interpreted by the shell no
// matter what it contains.
func TestRunAICommandPromptNeverParsedByShell(t *testing.T) {
dir := t.TempDir()
markerPath := dir + "/pwned"

maliciousPrompt := "innocuous prefix\n__NIGEL_PROMPT_EOF__\ntouch " + markerPath + "\n"

output, err := RunAICommand(echoBackend{}, "", "", maliciousPrompt, dir, nil, 0, nil)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}

if _, statErr := os.Stat(markerPath); statErr == nil {
t.Fatal("prompt content was executed as a shell command instead of being passed through as data")
}

if !strings.Contains(output, "touch "+markerPath) {
t.Fatalf("expected prompt content to be echoed back verbatim via stdin, got %q", output)
}
}