diff --git a/cmd/toolsrv/internal/exec/exec.go b/cmd/toolsrv/internal/exec/exec.go index 1fd6bc7..a9cc474 100644 --- a/cmd/toolsrv/internal/exec/exec.go +++ b/cmd/toolsrv/internal/exec/exec.go @@ -21,6 +21,12 @@ import ( "ollie/util" ) +// StartResult reports whether an external command started. +type StartResult struct { + Process *os.Process + Err error +} + // Config contains all context needed for tool execution. type Config struct { CWD string @@ -28,7 +34,7 @@ type Config struct { Yolo bool Timeout int // 0 = no timeout Output io.Writer // if set, stream stdout/stderr here in real-time - Started chan *os.Process // if set, receives the os.Process after Start() + Started chan StartResult // receives exactly one startup result if set } // StreamFunc returns the streaming output function from context, if any. @@ -94,7 +100,7 @@ func ExecuteTool(ctx context.Context, info protocol.ToolInfo, args json.RawMessa var result string if bypassed { bypassCode := fmt.Sprintf("cat <<'OLLIE_EOF' | %s\n%s\nOLLIE_EOF", toolPath, stdinData) - result, err = executeBypassDirect(ctx, bypassCode, cwd, cfg.Env, timeout) + result, err = executeBypassDirect(ctx, bypassCode, cwd, cfg.Env, timeout, cfg.Started) } else { result, err = executeSandboxed(ctx, toolPath, stdinData, cwd, cfg.Env, timeout, cfg.Yolo, cfg.Output, cfg.Started) } @@ -111,7 +117,14 @@ func ExecuteTool(ctx context.Context, info protocol.ToolInfo, args json.RawMessa } // executeSandboxed runs a tool script inside the sandbox. -func executeSandboxed(ctx context.Context, toolPath, stdinData, cwd string, envExtra map[string]string, timeout int, yolo bool, streamOut io.Writer, started chan *os.Process) (string, error) { +func executeSandboxed(ctx context.Context, toolPath, stdinData, cwd string, envExtra map[string]string, timeout int, yolo bool, streamOut io.Writer, started chan StartResult) (string, error) { + startedSent := false + defer func() { + if started != nil && !startedSent { + started <- StartResult{Err: fmt.Errorf("execution did not start")} + } + }() + var sandboxCfg *sandbox.Config if !yolo { cfgPath := filepath.Join(util.CfgDir(), "sandbox.yaml") @@ -205,13 +218,15 @@ func executeSandboxed(ctx context.Context, toolPath, stdinData, cwd string, envE if err := cmd.Start(); err != nil { if started != nil { - close(started) + started <- StartResult{Err: fmt.Errorf("execution failed: %w", err)} + startedSent = true } return "", fmt.Errorf("execution failed: %w", err) } if started != nil { - started <- cmd.Process + started <- StartResult{Process: cmd.Process} + startedSent = true } err := cmd.Wait() @@ -236,7 +251,7 @@ func executeSandboxed(ctx context.Context, toolPath, stdinData, cwd string, envE // executeBypassDirect requests bypass approval and executes the command directly. // The approval comes from olliesrv via the 9P bypass/pending and bypass/resolve files. -func executeBypassDirect(ctx context.Context, cmd, cwd string, envExtra map[string]string, timeout int) (string, error) { +func executeBypassDirect(ctx context.Context, cmd, cwd string, envExtra map[string]string, timeout int, started chan StartResult) (string, error) { // Build environment map envMap := make(map[string]string) for _, kv := range os.Environ() { @@ -251,9 +266,15 @@ func executeBypassDirect(ctx context.Context, cmd, cwd string, envExtra map[stri // Submit bypass request and wait for approval approved, err := bypass.Submit(ctx, cmd, cwd, envMap) if err != nil { + if started != nil { + started <- StartResult{Err: fmt.Errorf("bypass request failed: %w", err)} + } return "", fmt.Errorf("bypass request failed: %w", err) } if !approved { + if started != nil { + started <- StartResult{Err: fmt.Errorf("bypass denied")} + } return "", fmt.Errorf("bypass denied") } @@ -264,11 +285,11 @@ func executeBypassDirect(ctx context.Context, cmd, cwd string, envExtra map[stri defer cancel() } - return executeDirectUnsandboxed(ctx, cmd, cwd, envMap) + return executeDirectUnsandboxed(ctx, cmd, cwd, envMap, started) } // executeDirectUnsandboxed runs a command without any sandbox. -func executeDirectUnsandboxed(ctx context.Context, cmd, cwd string, env map[string]string) (string, error) { +func executeDirectUnsandboxed(ctx context.Context, cmd, cwd string, env map[string]string, started chan StartResult) (string, error) { execCmd := osExec.CommandContext(ctx, "bash", "-c", cmd) execCmd.Dir = cwd if len(env) > 0 { @@ -288,7 +309,16 @@ func executeDirectUnsandboxed(ctx context.Context, cmd, cwd string, env map[stri execCmd.Stderr = io.MultiWriter(&stderr, &streamWriter{stream}) } - err := execCmd.Run() + if err := execCmd.Start(); err != nil { + if started != nil { + started <- StartResult{Err: fmt.Errorf("bypass execution failed: %w", err)} + } + return "", err + } + if started != nil { + started <- StartResult{Process: execCmd.Process} + } + err := execCmd.Wait() output := stdout.String() if stderr.Len() > 0 { if output != "" { diff --git a/cmd/toolsrv/internal/server/proc.go b/cmd/toolsrv/internal/server/proc.go index 1564371..4a155ca 100644 --- a/cmd/toolsrv/internal/server/proc.go +++ b/cmd/toolsrv/internal/server/proc.go @@ -294,11 +294,11 @@ func (st *State) NewProc(ctx context.Context, payload string, background bool) ( // For background procs, stream output directly into the proc buffer. // For foreground, use nil (local buffer in exec path). var outputWriter io.Writer - var startedCh chan *os.Process + var startedCh chan exec.StartResult timeout := 30 // default for foreground if background { outputWriter = &procWriter{proc: proc} - startedCh = make(chan *os.Process, 1) + startedCh = make(chan exec.StartResult, 1) timeout = 0 // background procs never timeout } @@ -334,9 +334,21 @@ func (st *State) NewProc(ctx context.Context, payload string, background bool) ( // For background procs, wait for the process to start and store its reference. if background { - if p, ok := <-startedCh; ok && p != nil { + started := <-startedCh + if started.Err != nil { proc.mu.Lock() - proc.proc = p + proc.Exited = true + proc.ExitCode = 1 + proc.EndTime = time.Now() + proc.mu.Unlock() + st.procMu.Lock() + delete(st.procs, proc.ID) + st.procMu.Unlock() + return "", 0, started.Err + } + if started.Process != nil { + proc.mu.Lock() + proc.proc = started.Process proc.mu.Unlock() } return fmt.Sprintf("%d", proc.ID), proc.ID, nil @@ -378,7 +390,7 @@ func (st *State) NewProc(ctx context.Context, payload string, background bool) ( // executeTool runs a tool and returns output + exit code. // timeout: 0 = no timeout, >0 = seconds. -func (st *State) executeTool(ctx context.Context, info protocol.ToolInfo, args map[string]string, cwd string, envExtra map[string]string, yolo bool, timeout int, output io.Writer, started chan *os.Process) (string, int) { +func (st *State) executeTool(ctx context.Context, info protocol.ToolInfo, args map[string]string, cwd string, envExtra map[string]string, yolo bool, timeout int, output io.Writer, started chan exec.StartResult) (string, int) { // Convert args map to JSON for the execution path jsonArgs := argsToJSON(args)