Merge pull request #124 from dzerik/feat/popen-go

feat(go): streaming-stdio Popen — Process with per-stream StdioMode (RFC #67)

Author: congwang-mk

go/README.md

@@ -119,6 +119,7 @@ func (s *Sandbox) Run(ctx context.Context, cmd ...string) (*Result, error)
 func (s *Sandbox) RunInteractive(ctx context.Context, cmd ...string) (int, error)
 func (s *Sandbox) DryRun(ctx context.Context, cmd ...string) (*DryRunResult, error)
 func (s *Sandbox) Spawn(cmd ...string) (*Process, error)
+func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error)
 ```
 
 - **Run** captures stdout/stderr and waits. A `ctx` deadline kills the process
@@ -129,6 +130,25 @@ func (s *Sandbox) Spawn(cmd ...string) (*Process, error)
   filesystem `Changes` it would have made, and discards them. Requires
   `Workdir`.
 - **Spawn** starts a process without waiting, returning a `*Process`.
+- **Popen** is the streaming counterpart of Spawn: each stream set to
+  `StdioPiped` is handed back on the `*Process` as an `*os.File`
+  (`Stdin`/`Stdout`/`Stderr`) you read/write while the child runs. The zero
+  `Stdio` inherits all three (identical to Spawn). Close a piped `Stdin` before
+  `Wait` (or let `Wait` close it for you) so a reader child sees EOF; drain a
+  piped `Stdout`/`Stderr` before `Wait`, or `Kill` from another goroutine to
+  interrupt a blocked `Wait`. `Wait` returns a `Result` with the exit status
+  only — a Popen'd process sends piped output to the `Stdout`/`Stderr` fields
+  (inherited/null streams go to the parent fd or `/dev/null`), so (unlike `Run`)
+  `Result.Stdout`/`Result.Stderr` are always empty.
+
+```go
+p, _ := sb.Popen(sandlock.Stdio{Stdin: sandlock.StdioPiped, Stdout: sandlock.StdioPiped}, "cat")
+defer p.Close()
+p.Stdin.Write([]byte("hi\n"))
+p.Stdin.Close()                 // EOF so cat exits
+out, _ := io.ReadAll(p.Stdout)  // "hi\n"
+res, _ := p.Wait()
+```
 
 ### Dynamic policy callbacks
 
@@ -177,7 +197,12 @@ func (p *Process) Pause() error           // SIGSTOP to the process group
 func (p *Process) Resume() error          // SIGCONT
 func (p *Process) Kill() error            // SIGKILL
 func (p *Process) Ports() (map[int]int, error) // virtual→real, with PortRemap
-func (p *Process) Close() error           // release the handle (kills if running)
+func (p *Process) Close() error           // release the handle (kills if running), close piped streams
+
+// Popen only: caller-owned pipe ends, non-nil per stream wired StdioPiped.
+p.Stdin  // *os.File
+p.Stdout // *os.File
+p.Stderr // *os.File
 ```
 
 ### Confine the current process

go/sandbox.go

@@ -285,6 +285,29 @@ type Result struct {
 	Stderr   []byte // captured standard error
 }
 
+// StdioMode selects how one of a Popen'd process's standard streams is wired.
+// The values are the stable ABI discriminants shared with the C/Rust core.
+type StdioMode uint32
+
+const (
+	// StdioInherit shares the parent's fd (the child writes to the same
+	// terminal/file). It is the zero value, so an unset stream inherits.
+	StdioInherit StdioMode = 0
+	// StdioPiped connects the stream to a pipe; Popen hands the caller the
+	// owning end as an *os.File on the returned Process.
+	StdioPiped StdioMode = 1
+	// StdioNull connects the stream to /dev/null.
+	StdioNull StdioMode = 2
+)
+
+// Stdio selects the wiring of a Popen'd process's three standard streams. The
+// zero value wires all three as StdioInherit, matching Spawn.
+type Stdio struct {
+	Stdin  StdioMode
+	Stdout StdioMode
+	Stderr StdioMode
+}
+
 // ChangeKind classifies a filesystem change observed during a dry run.
 type ChangeKind byte
 

go/sandlock_linux.go

@@ -22,6 +22,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"os"
 	"runtime"
 	"strings"
 	"sync"
@@ -799,6 +800,13 @@ type Process struct {
 	h       *C.sandlock_handle_t
 	pid     int
 	waiting bool // a Wait owns the handle; other handle ops must defer to it
+
+	// Caller-owned stdio for a process started by Popen. Each is non-nil only
+	// for a stream wired StdioPiped; it owns the pipe fd (closing the file
+	// closes the fd). Nil for Spawn'd processes and for inherit/null streams.
+	Stdin  *os.File
+	Stdout *os.File
+	Stderr *os.File
 }
 
 // Spawn forks the sandboxed child, installs the policy, and releases it to
@@ -838,10 +846,78 @@ func (s *Sandbox) Spawn(cmd ...string) (*Process, error) {
 	return p, nil
 }
 
+// fdFile wraps an fd owned by the caller (handed back by sandlock_popen) as an
+// *os.File that closes the fd when closed or finalized, or nil for the -1
+// sentinel of an inherit/null stream.
+func fdFile(fd C.int, name string) *os.File {
+	if fd < 0 {
+		return nil
+	}
+	return os.NewFile(uintptr(fd), name)
+}
+
+// Popen forks the sandboxed child with per-stream stdio and returns a live
+// Process without waiting — the streaming counterpart of Spawn. For each stream
+// set to StdioPiped, the matching Process field (Stdin/Stdout/Stderr) is an
+// *os.File the caller reads/writes while the child runs; inherit/null streams
+// leave it nil. The zero Stdio inherits all three (identical to Spawn).
+//
+// Deadlock warning (as with os/exec pipes): a piped Stdin is yours — close it
+// before Wait, or a child that reads to EOF (e.g. cat) never exits and Wait
+// blocks. Likewise drain a piped Stdout/Stderr before Wait, or a child that
+// fills the pipe buffer blocks on write. As an escape hatch, Kill from another
+// goroutine interrupts a blocked Wait; Close reaps the child and closes the
+// streams. Wait closes a still-open piped Stdin for you to deliver EOF.
+func (s *Sandbox) Popen(stdio Stdio, cmd ...string) (*Process, error) {
+	if len(cmd) == 0 {
+		return nil, fmt.Errorf("sandlock: empty command")
+	}
+	for _, m := range []StdioMode{stdio.Stdin, stdio.Stdout, stdio.Stderr} {
+		if m > StdioNull {
+			return nil, fmt.Errorf("sandlock: invalid StdioMode %d", uint32(m))
+		}
+	}
+	policyPtr, err := s.buildPolicy()
+	if err != nil {
+		return nil, err
+	}
+	defer C.sandlock_sandbox_free(policyPtr)
+
+	argv, err := cArgv(cmd)
+	if err != nil {
+		return nil, err
+	}
+	defer freeArgv(argv)
+	ap, ac := argvPtr(argv)
+	name := s.cName()
+	defer freeName(name)
+
+	fdIn, fdOut, fdErr := C.int(-1), C.int(-1), C.int(-1)
+	h := C.sandlock_popen(
+		policyPtr, name, ap, ac,
+		C.uint32_t(stdio.Stdin), C.uint32_t(stdio.Stdout), C.uint32_t(stdio.Stderr),
+		&fdIn, &fdOut, &fdErr,
+	)
+	if h == nil {
+		return nil, fmt.Errorf("sandlock: popen failed")
+	}
+	p := &Process{
+		h:      h,
+		pid:    int(C.sandlock_handle_pid(h)),
+		Stdin:  fdFile(fdIn, "sandlock-stdin"),
+		Stdout: fdFile(fdOut, "sandlock-stdout"),
+		Stderr: fdFile(fdErr, "sandlock-stderr"),
+	}
+	runtime.SetFinalizer(p, (*Process).finalize)
+	return p, nil
+}
+
 // finalize is the SetFinalizer cleanup for a Process abandoned without
 // Wait/Close. It can only run once the Process is unreachable, which implies
 // no Wait is in flight (a blocked Wait keeps the Process reachable), so the
-// handle is not concurrently borrowed and is safe to free here.
+// handle is not concurrently borrowed and is safe to free here. The piped
+// streams are intentionally left alone: once the Process is unreachable so are
+// they, and each *os.File's own finalizer closes its fd — no fd leaks.
 func (p *Process) finalize() {
 	p.mu.Lock()
 	defer p.mu.Unlock()
@@ -862,23 +938,39 @@ func (p *Process) Pid() int {
 	return p.pid
 }
 
-// Wait blocks until the process exits, returns its captured Result, and
-// releases the handle. After Wait the Process is no longer running.
+// Wait blocks until the process exits, returns its Result, and releases the
+// handle. After Wait the Process is no longer running.
+//
+// The Result carries the exit status only. Unlike Run, a Popen'd process sends
+// any StdioPiped output to the caller through the Stdout/Stderr fields (and
+// StdioInherit/StdioNull output to the inherited fd or /dev/null), so it is
+// never captured into Result.Stdout/Result.Stderr — read piped bytes off
+// p.Stdout/p.Stderr, not off the Result.
 //
 // The blocking native wait runs without holding the mutex so that Kill (and
 // Pause/Resume), which signal the process group by PID, can run concurrently
 // and interrupt it. The waiting flag reserves exclusive use of the handle for
 // the duration, so no other handle operation aliases it.
+//
+// Wait closes a still-open piped Stdin to deliver EOF, so do not write to Stdin
+// from another goroutine while Wait runs — the write would race a closing fd.
 func (p *Process) Wait() (*Result, error) {
 	p.mu.Lock()
 	if p.h == nil || p.waiting {
 		p.mu.Unlock()
 		return nil, ErrNotRunning
 	}
 	h := p.h
+	stdin := p.Stdin
 	p.waiting = true
 	p.mu.Unlock()
 
+	// Deliver EOF to a still-open piped stdin so a reader child (e.g. cat) can
+	// exit; otherwise the native wait blocks forever. Harmless if already closed.
+	if stdin != nil {
+		_ = stdin.Close()
+	}
+
 	r := C.sandlock_handle_wait(h)
 
 	p.mu.Lock()
@@ -911,11 +1003,29 @@ func (p *Process) Pause() error { return p.signal(syscall.SIGSTOP) }
 // Resume sends SIGCONT to the sandbox process group.
 func (p *Process) Resume() error { return p.signal(syscall.SIGCONT) }
 
-// Kill sends SIGKILL to the sandbox process group.
+// Kill sends SIGKILL to the sandbox process group. It is idempotent: a process
+// that already exited (ESRCH) or was already reaped by Wait (ErrNotRunning) is
+// not an error — killing it is a no-op success, matching the FFI
+// (sandlock_handle_kill returns 0) and the Python binding (silent no-op after
+// wait()). This keeps the "Kill from another goroutine" escape hatch race-free:
+// a Kill that fires just as Wait reaps the child still returns nil.
 func (p *Process) Kill() error {
 	err := p.signal(syscall.SIGKILL)
 	if err == syscall.ESRCH {
-		return nil
+		return nil // child already exited (still unreaped) — no-op
+	}
+	if err == ErrNotRunning {
+		// signal() returns ErrNotRunning for two causes: the handle was already
+		// reaped by Wait (h == nil) — an idempotent no-op — or a live handle with
+		// no pid. Only the reaped case is a success; a missing pid is a genuine
+		// error we must surface (and must never turn into killpg(-0), which would
+		// signal our own process group).
+		p.mu.Lock()
+		reaped := p.h == nil
+		p.mu.Unlock()
+		if reaped {
+			return nil
+		}
 	}
 	return err
 }
@@ -957,6 +1067,13 @@ func (p *Process) Ports() (map[int]int, error) {
 func (p *Process) Close() error {
 	p.mu.Lock()
 	defer p.mu.Unlock()
+	// Release caller-owned pipe fds regardless of handle state: a Close after
+	// Wait still needs to close the piped stdout/stderr the caller drained.
+	for _, f := range []*os.File{p.Stdin, p.Stdout, p.Stderr} {
+		if f != nil {
+			_ = f.Close()
+		}
+	}
 	if p.h == nil {
 		return nil
 	}