Skip to content
Merged
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
27 changes: 26 additions & 1 deletion go/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions go/sandbox.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test gap: the zero-Stdio-equals-Spawn contract is stated in three places (here, the Popen doc, the README) but never exercised. TestPopenNullStdoutNoStream is the closest test. A cheap Popen(sandlock.Stdio{}, ...) test asserting all three stream fields are nil and the child runs to success would pin the ABI default (StdioInherit == 0) that the zero-value contract relies on.

Stdin StdioMode
Stdout StdioMode
Stderr StdioMode
}

// ChangeKind classifies a filesystem change observed during a dry run.
type ChangeKind byte

Expand Down
127 changes: 122 additions & 5 deletions go/sandlock_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"context"
"encoding/json"
"fmt"
"os"
"runtime"
"strings"
"sync"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kill-after-exit idempotency now differs across the three bindings. FFI sandlock_handle_kill returns 0 for an already-exited process, Python Process.kill() is a silent no-op after wait(), but Go's Kill() returns ErrNotRunning once the handle is freed (signal checks p.h == nil before the ESRCH mapping can apply). Since this doc actively promotes "Kill from another goroutine" as the escape hatch, the watchdog-races-normal-exit case (Wait returns just as the watchdog fires Kill) yields an error in Go where the other two bindings say success. Pre-existing signal semantics, but worth aligning while the contract is being written down: Kill on an already-reaped Process should arguably be nil too.

// 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()
Expand All @@ -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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doc gap: nothing says the Result of a Popen'd process carries no captured output. The Python binding documents this in both popen and Process.wait; here neither Wait's doc nor the README's Popen bullet mentions it, and the README still describes Wait as returning a "captured Result". A user coming from Run will read res.Stdout and get an empty slice with no hint why (the bytes went to p.Stdout, possibly already drained). One sentence here and one in the README fixes it.

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()
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
}
Expand Down
Loading
Loading