From 995d85131edb14943c389443ab83f932ff1e2cbd Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Wed, 16 Apr 2025 12:56:34 -0700
Subject: [PATCH 01/11] wip

---
 run/abc_version.go      | 178 ++++++++++++++++++++++++++++++++++++++++
 run/exec.go             |  88 ++++++++++++++++++++
 run/guardian_version.go | 148 +++++++++++++++++++++++++++++++++
 run/sys_default.go      |  26 ++++++
 run/sys_linux.go        |  40 +++++++++
 5 files changed, 480 insertions(+)
 create mode 100644 run/abc_version.go
 create mode 100644 run/exec.go
 create mode 100644 run/guardian_version.go
 create mode 100644 run/sys_default.go
 create mode 100644 run/sys_linux.go

diff --git a/run/abc_version.go b/run/abc_version.go
new file mode 100644
index 00000000..7f76525d
--- /dev/null
+++ b/run/abc_version.go
@@ -0,0 +1,178 @@
+// Copyright 2023 The Authors (see AUTHORS file)
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package run
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"os/exec"
+	"time"
+)
+
+// DefaultRunTimeout is how long we'll wait for commands to run in the case
+// where the context doesn't already have a timeout. This was chosen
+// arbitrarily.
+const DefaultRunTimeout = time.Minute
+
+// Simple is a wrapper around [Run] that captures stdout and stderr as strings.
+// This is intended to be used for commands that run non-interactively then
+// exit.
+//
+// If the command exits with a nonzero status code, an *exec.ExitError will be
+// returned.
+//
+// If the command fails, the error message will include the contents of stdout
+// and stderr. This saves boilerplate in the caller.
+func Simple(ctx context.Context, args ...string) (stdout, stderr string, _ error) {
+	var stdoutBuf, stderrBuf bytes.Buffer
+	_, err := Run(ctx, []*Option{
+		WithStdout(&stdoutBuf),
+		WithStderr(&stderrBuf),
+	}, args...)
+	return stdoutBuf.String(), stderrBuf.String(), err
+}
+
+// Run runs the command provided by args, using the options in opts. By default,
+// if the command returns a nonzero exit code, an error is returned, but this
+// behavior may be overridden by the AllowNonzeroExit option.
+//
+// If the incoming context doesn't already have a timeout, then a default
+// timeout will be added (see DefaultRunTimeout).
+//
+// If the command fails, the error message will include the contents of stdout
+// and stderr. This saves boilerplate in the caller.
+//
+// The input args must have len>=1. opts may be nil if no special options are
+// needed.
+//
+// This doesn't execute a shell (unless of course args[0] is the name of a shell
+// binary).
+func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ error) {
+	if _, ok := ctx.Deadline(); !ok {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, DefaultRunTimeout)
+		defer cancel()
+	}
+
+	cmd := exec.CommandContext(ctx, args[0], args[1:]...) //nolint:gosec // run'ing the input args is fundamentally the whole point
+
+	// any of these can be nil
+	compiledOpts := compileOpts(opts)
+	cmd.Stdout = compiledOpts.stdout
+	cmd.Stderr = compiledOpts.stderr
+	cmd.Stdin = compiledOpts.stdin
+	cmd.Dir = compiledOpts.cwd
+
+	err := cmd.Run()
+	if err != nil {
+		// Don't return error if both (a) the caller indicated they're OK with a
+		// nonzero exit code and (b) the error is of a type that means the only
+		// problem was a nonzero exit code.
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) && compiledOpts.allowNonZeroExit {
+			err = nil
+		} else {
+			err = fmt.Errorf(`run of %v failed: error was "%w", context error was "%w"\nstdout: %s\nstderr: %s`,
+				args, err, ctx.Err(), cmd.Stdout, cmd.Stderr)
+		}
+	}
+	return cmd.ProcessState.ExitCode(), err
+}
+
+// Many calls [Simple] for each command in args. If any command returns error,
+// then no further commands will be run, and that error will be returned. For
+// any commands that were actually executed (not aborted by a previous error),
+// their stdout and stderr will be returned. It's guaranteed that
+// len(stdouts)==len(stderrs).
+func Many(ctx context.Context, args ...[]string) (stdouts, stderrs []string, _ error) {
+	for _, cmd := range args {
+		stdout, stderr, err := Simple(ctx, cmd...)
+		stdouts = append(stdouts, stdout)
+		stderrs = append(stderrs, stderr)
+		if err != nil {
+			return stdouts, stderrs, err
+		}
+	}
+	return stdouts, stderrs, nil
+}
+
+// Option implements the functional options pattern for [Run].
+type Option struct {
+	allowNonZeroExit bool
+	cwd              string
+	stdin            io.Reader
+	stdout           io.Writer
+	stderr           io.Writer
+}
+
+// AllowNonzeroExit is an option that will NOT treat a nonzero exit code from
+// the command as an error (so [Run] won't return error). The default behavior
+// of [Run] is that if a command exits with a nonzero status code, then that
+// becomes a Go error.
+func AllowNonzeroExit() *Option {
+	return &Option{allowNonZeroExit: true}
+}
+
+// WithStdin passes the given reader as the command's standard input.
+func WithStdin(stdin io.Reader) *Option {
+	return &Option{stdin: stdin}
+}
+
+// WithStdinStr is a convenient wrapper around WithStdin that passes the given
+// string as the command's standard input.
+func WithStdinStr(stdin string) *Option {
+	return WithStdin(bytes.NewBufferString(stdin))
+}
+
+// WithStdout writes the command's standard output to the given writer.
+func WithStdout(stdout io.Writer) *Option {
+	return &Option{stdout: stdout}
+}
+
+// WithStderr writes the command's standard error to the given writer.
+func WithStderr(stderr io.Writer) *Option {
+	return &Option{stderr: stderr}
+}
+
+// WithCwd runs the command in the given working directory.
+func WithCwd(cwd string) *Option {
+	return &Option{cwd: cwd}
+}
+
+func compileOpts(opts []*Option) *Option {
+	var out Option
+	for _, opt := range opts {
+		if opt.allowNonZeroExit {
+			out.allowNonZeroExit = true
+		}
+		if opt.stdin != nil {
+			out.stdin = opt.stdin
+		}
+		if opt.stdout != nil {
+			out.stdout = opt.stdout
+		}
+		if opt.stderr != nil {
+			out.stderr = opt.stderr
+		}
+		if opt.cwd != "" {
+			out.cwd = opt.cwd
+		}
+	}
+
+	return &out
+}
diff --git a/run/exec.go b/run/exec.go
new file mode 100644
index 00000000..b83f256b
--- /dev/null
+++ b/run/exec.go
@@ -0,0 +1,88 @@
+// Copyright 2025 The Authors (see AUTHORS file)
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package run
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"io"
+	"os/exec"
+)
+
+// ExecConfig are the inputs for a run operation.
+type Option struct {
+	stdin      io.Writer
+	stdout     io.Writer
+	stderr     io.Writer
+	workingDir string
+
+	// allowedEnvKeys and deniedEnvKeys respectively define an allow/deny list of
+	// patterns for environment variable keys. Keys are matched using
+	// [filepath.Match].
+	allowedEnvKeys []string
+	deniedEnvKeys  []string
+
+	// overrideEnvVars are the environment variables to inject into the child
+	// process, no matter what the user configured. These take precedence over all
+	// other configurables.
+	overrideEnvVars []string
+}
+
+// Run runs the command provided by args, using the options in opts. Error
+// is only returned if something external to the process fails (timeout,
+// command not found, ect). It is up to user to check status code.
+//
+// If the incoming context doesn't already have a timeout, then a default
+// timeout will be added (see DefaultRunTimeout).
+//
+// opts may be nil if no special options are needed.
+//
+// This doesn't execute a shell (unless of course command is the name of a shell
+// binary).
+
+// TODO: what are the ergonomics like with opts not being varadic operator? I'm not sure I'm happy with this
+func Run(ctx context.Context, opts []*Option, command string, args ...string) (exitCode int, _ error) {
+	if _, ok := ctx.Deadline(); !ok {
+		var cancel context.CancelFunc
+		ctx, cancel = context.WithTimeout(ctx, DefaultRunTimeout)
+		defer cancel()
+	}
+
+	cmd := exec.CommandContext(ctx, command, args...) //nolint:gosec // run'ing the input args is fundamentally the whole point
+	setSysProcAttr(cmd)
+	setCancel(cmd)
+	// any of these can be nil
+	compiledOpts := compileOpts(opts)
+	cmd.Stdout = compiledOpts.stdout
+	cmd.Stderr = compiledOpts.stderr
+	cmd.Stdin = compiledOpts.stdin
+	cmd.Dir = compiledOpts.cwd
+
+	err := cmd.Run()
+	if err != nil {
+		// Don't return error if both (a) the caller indicated they're OK with a
+		// nonzero exit code and (b) the error is of a type that means the only
+		// problem was a nonzero exit code.
+		var exitErr *exec.ExitError
+		if errors.As(err, &exitErr) && compiledOpts.allowNonZeroExit {
+			err = nil
+		} else {
+			err = fmt.Errorf(`run of %v failed: error was "%w", context error was "%w"\nstdout: %s\nstderr: %s`,
+				args, err, ctx.Err(), cmd.Stdout, cmd.Stderr)
+		}
+	}
+	return cmd.ProcessState.ExitCode(), err
+}
diff --git a/run/guardian_version.go b/run/guardian_version.go
new file mode 100644
index 00000000..82c3cf35
--- /dev/null
+++ b/run/guardian_version.go
@@ -0,0 +1,148 @@
+// Copyright 2023 The Authors (see AUTHORS file)
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+// Package child provides the functionality to execute child command line processes.
+package run
+
+import (
+	"context"
+	"fmt"
+	"io"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/abcxyz/pkg/logging"
+)
+
+// RunConfig are the inputs for a run operation.
+type RunConfig struct {
+	Stdout     io.Writer
+	Stderr     io.Writer
+	WorkingDir string
+	Command    string
+	Args       []string
+
+	// AllowedEnvKeys and DeniedEnvKeys respectively define an allow/deny list of
+	// patterns for environment variable keys. Keys are matched using
+	// [filepath.Match].
+	AllowedEnvKeys []string
+	DeniedEnvKeys  []string
+
+	// OverrideEnvVars are the environment variables to inject into the child
+	// process, no matter what the user configured. These take precedence over all
+	// other configurables.
+	OverrideEnvVars []string
+}
+
+// Run executes a child process with the provided arguments.
+func Run(ctx context.Context, cfg *RunConfig) (int, error) {
+	logger := logging.FromContext(ctx).
+		With("working_dir", cfg.WorkingDir).
+		With("command", cfg.Command).
+		With("args", cfg.Args)
+
+	path, err := exec.LookPath(cfg.Command)
+	if err != nil {
+		return -1, fmt.Errorf("failed to locate command run path: %w", err)
+	}
+
+	cmd := exec.CommandContext(ctx, path)
+	setSysProcAttr(cmd)
+	setCancel(cmd)
+
+	if v := cfg.WorkingDir; v != "" {
+		cmd.Dir = v
+	}
+
+	stdout := cfg.Stdout
+	if stdout == nil {
+		stdout = os.Stdout
+	}
+
+	stderr := cfg.Stderr
+	if stderr == nil {
+		stderr = os.Stderr
+	}
+
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
+	cmd.Args = append(cmd.Args, cfg.Args...)
+
+	// Compute and set a custom environment for the child process.
+	env := environ(os.Environ(), cfg.AllowedEnvKeys, cfg.DeniedEnvKeys, cfg.OverrideEnvVars)
+	logger.DebugContext(ctx, "computed environment", "env", env)
+	cmd.Env = env
+
+	// add small wait delay to kill subprocesses if context is canceled
+	// https://github.com/golang/go/issues/23019
+	// https://github.com/golang/go/issues/50436
+	cmd.WaitDelay = 2 * time.Second
+
+	logger.DebugContext(ctx, "command started")
+
+	if err := cmd.Start(); err != nil {
+		return cmd.ProcessState.ExitCode(), fmt.Errorf("failed to start command: %w", err)
+	}
+
+	if err := cmd.Wait(); err != nil {
+		return cmd.ProcessState.ExitCode(), fmt.Errorf("failed to run command: %w", err)
+	}
+
+	exitCode := cmd.ProcessState.ExitCode()
+
+	logger.DebugContext(ctx, "command completed", "exit_code", exitCode)
+
+	return exitCode, nil
+}
+
+// environ compiles the appropriate environment to pass to the child process.
+// The overridden environment is always added, even if not explicitly
+// allowed/denied.
+func environ(osEnv, allowedKeys, deniedKeys, overrideEnv []string) []string {
+	finalEnv := make([]string, 0, len(osEnv)+len(overrideEnv))
+
+	// Select keys that match the allow filter (if given) but not the deny filter.
+	for _, v := range osEnv {
+		k := strings.SplitN(v, "=", 2)[0]
+		if (len(allowedKeys) == 0 || anyGlobMatch(k, allowedKeys)) &&
+			!anyGlobMatch(k, deniedKeys) {
+			finalEnv = append(finalEnv, v)
+		}
+	}
+
+	// Add overrides at the end, after any filtering
+	finalEnv = append(finalEnv, overrideEnv...)
+
+	return finalEnv
+}
+
+func anyGlobMatch(s string, patterns []string) bool {
+	// Short-circuit path matching logic for match-all.
+	for _, p := range patterns {
+		if p == "*" || p == ".*" {
+			return true
+		}
+	}
+
+	// Now do the slower lookup.
+	for _, p := range patterns {
+		if ok, _ := filepath.Match(p, s); ok {
+			return true
+		}
+	}
+	return false
+}
diff --git a/run/sys_default.go b/run/sys_default.go
new file mode 100644
index 00000000..0073365e
--- /dev/null
+++ b/run/sys_default.go
@@ -0,0 +1,26 @@
+// Copyright 2023 The Authors (see AUTHORS file)
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build !linux
+// +build !linux
+
+package run
+
+import "os/exec"
+
+// setSysProcAttr sets the sysProcAttr.
+func setSysProcAttr(cmd *exec.Cmd) {}
+
+// setCancel sets the Cancel behavior for a child process.
+func setCancel(cmd *exec.Cmd) {}
diff --git a/run/sys_linux.go b/run/sys_linux.go
new file mode 100644
index 00000000..3be22c00
--- /dev/null
+++ b/run/sys_linux.go
@@ -0,0 +1,40 @@
+// Copyright 2023 The Authors (see AUTHORS file)
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+//go:build linux
+// +build linux
+
+package run
+
+import (
+	"os/exec"
+	"syscall"
+)
+
+// setSysProcAttr sets the sysProcAttr.
+func setSysProcAttr(cmd *exec.Cmd) {
+	cmd.SysProcAttr = &syscall.SysProcAttr{
+		// kill children if parent is dead
+		Pdeathsig: syscall.SIGKILL,
+		// set process group ID
+		Setpgid: true,
+	}
+}
+
+// setCancel sets the Cancel behavior for a child process.
+func setCancel(cmd *exec.Cmd) {
+	cmd.Cancel = func() error {
+		return cmd.Process.Signal(syscall.SIGQUIT) // Want passthrough
+	}
+}

From 2dc5e04ebd948abf55894a7be5dfcf6f82a32d75 Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 15:18:32 -0700
Subject: [PATCH 02/11] WIP:

Simple() tested, looking for overall feedback before committing more thorough testing.

Signed-off-by: pdewilde <pdewilde@google.com>
---
 run/exec.go      | 342 +++++++++++++++++++++++++++++++++++++++++------
 run/exec_test.go |  61 +++++++++
 2 files changed, 359 insertions(+), 44 deletions(-)
 create mode 100644 run/exec_test.go

diff --git a/run/exec.go b/run/exec.go
index b83f256b..1bdd0bdb 100644
--- a/run/exec.go
+++ b/run/exec.go
@@ -1,10 +1,10 @@
-// Copyright 2025 The Authors (see AUTHORS file)
+// Copyright 2023 The Authors (see AUTHORS file)
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
 // You may obtain a copy of the License at
 //
-//      http://www.apache.org/licenses/LICENSE-2.0
+//     http://www.apache.org/licenses/LICENSE-2.0
 //
 // Unless required by applicable law or agreed to in writing, software
 // distributed under the License is distributed on an "AS IS" BASIS,
@@ -12,77 +12,331 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+// Package run provides enhanced functionality for executing external commands,
+// offering control over environment variables, process attributes, I/O,
+// timeouts, and integrated logging, using a functional options pattern.
 package run
 
 import (
+	"bytes"
 	"context"
 	"errors"
 	"fmt"
 	"io"
+	"os"
 	"os/exec"
+	"path/filepath"
+	"strings"
+	"time"
+
+	"github.com/abcxyz/pkg/logging" // Import the specific logging package
 )
 
-// ExecConfig are the inputs for a run operation.
-type Option struct {
-	stdin      io.Writer
-	stdout     io.Writer
-	stderr     io.Writer
-	workingDir string
-
-	// allowedEnvKeys and deniedEnvKeys respectively define an allow/deny list of
-	// patterns for environment variable keys. Keys are matched using
-	// [filepath.Match].
-	allowedEnvKeys []string
-	deniedEnvKeys  []string
-
-	// overrideEnvVars are the environment variables to inject into the child
-	// process, no matter what the user configured. These take precedence over all
-	// other configurables.
-	overrideEnvVars []string
+// DefaultRunTimeout is how long commands wait if the context doesn't have a deadline.
+const DefaultRunTimeout = time.Minute
+
+// DefaultWaitDelay is how long after context cancellation processes are actually killed
+// if another value isn't set. See exec.Cmd.WaitDelay for more information.
+const DefaultWaitDelay = time.Second
+
+// Simple is a wrapper around [Run] that captures stdout and stderr as strings.
+// This is intended to be used for commands that run non-interactively then
+// exit.
+//
+// If the command exits with a nonzero status code, an *exec.ExitError will be
+// returned.
+//
+// If the command fails, the error message will include the contents of stdout
+// and stderr. This saves boilerplate in the caller.
+func Simple(ctx context.Context, args ...string) (stdout, stderr string, exitCode int, _ error) {
+	var stdoutBuf, stderrBuf bytes.Buffer
+	opts := []*Option{
+		WithStdout(&stdoutBuf),
+		WithStderr(&stderrBuf),
+	}
+
+	exitCode, err := Run(ctx, opts, args...)
+
+	return stdoutBuf.String(), stderrBuf.String(), exitCode, err
 }
 
-// Run runs the command provided by args, using the options in opts. Error
-// is only returned if something external to the process fails (timeout,
-// command not found, ect). It is up to user to check status code.
+// Run executes the command specified by args, applying configurations from opts.
+//
+// By default, a non-zero exit code results in an error (*exec.ExitError),
+// unless the AllowNonzeroExit option is used.
+// The error message includes stdout/stderr content if they are captured to buffers.
 //
-// If the incoming context doesn't already have a timeout, then a default
-// timeout will be added (see DefaultRunTimeout).
+// If the context doesn't have a deadline, DefaultRunTimeout is applied.
 //
-// opts may be nil if no special options are needed.
+// The input args must have len>=1. opts may be nil if no special options are
+// needed.
 //
-// This doesn't execute a shell (unless of course command is the name of a shell
+// This doesn't execute a shell (unless of course args[0] is the name of a shell
 // binary).
+func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ error) {
+	logger := logging.FromContext(ctx)
 
-// TODO: what are the ergonomics like with opts not being varadic operator? I'm not sure I'm happy with this
-func Run(ctx context.Context, opts []*Option, command string, args ...string) (exitCode int, _ error) {
+	if len(args) == 0 {
+		logger.DebugContext(ctx, "run called with no arguments")
+		return -1, errors.New("run: must provide at least one argument (the command)")
+	}
+
+	// Apply default timeout if none exists on the context.
 	if _, ok := ctx.Deadline(); !ok {
 		var cancel context.CancelFunc
 		ctx, cancel = context.WithTimeout(ctx, DefaultRunTimeout)
-		defer cancel()
+		defer cancel() // Ensure the derived context is cancelled
 	}
 
-	cmd := exec.CommandContext(ctx, command, args...) //nolint:gosec // run'ing the input args is fundamentally the whole point
-	setSysProcAttr(cmd)
-	setCancel(cmd)
-	// any of these can be nil
 	compiledOpts := compileOpts(opts)
-	cmd.Stdout = compiledOpts.stdout
-	cmd.Stderr = compiledOpts.stderr
-	cmd.Stdin = compiledOpts.stdin
+
+	// #nosec G204 -- Execution of external commands is the purpose of this package. Inputs must be trusted by the caller.
+	cmd := exec.CommandContext(ctx, args[0], args[1:]...)
+
 	cmd.Dir = compiledOpts.cwd
+	cmd.Stdin = compiledOpts.stdin
+	// Handle stdout/stderr, capturing output if writers are bytes.Buffer for error reporting.
+	var stdoutBuf, stderrBuf *bytes.Buffer
+	if compiledOpts.stdout != nil {
+		cmd.Stdout = compiledOpts.stdout
+		if bb, ok := compiledOpts.stdout.(*bytes.Buffer); ok {
+			stdoutBuf = bb
+		}
+	} else {
+		cmd.Stdout = os.Stdout
+	}
+	if compiledOpts.stderr != nil {
+		cmd.Stderr = compiledOpts.stderr
+		if bb, ok := compiledOpts.stderr.(*bytes.Buffer); ok {
+			stderrBuf = bb
+		}
+	} else {
+		cmd.Stderr = os.Stderr
+	}
+
+	useCustomEnv := len(compiledOpts.allowedEnvKeys) > 0 || len(compiledOpts.deniedEnvKeys) > 0 || len(compiledOpts.additionalEnv) > 0
+	if useCustomEnv {
+		currentEnv := os.Environ()
+		logger.DebugContext(ctx, "calculating custom environment",
+			"inherited_count", len(currentEnv),
+			"allowed_patterns", compiledOpts.allowedEnvKeys,
+			"denied_patterns", compiledOpts.deniedEnvKeys,
+			"additional_vars_count", len(compiledOpts.additionalEnv))
+		env := environ(currentEnv, compiledOpts.allowedEnvKeys, compiledOpts.deniedEnvKeys, compiledOpts.additionalEnv)
+		logger.DebugContext(ctx, "computed environment", "env", env)
+		cmd.Env = env
+	} else {
+		logger.DebugContext(ctx, "using inherited environment")
+	}
+
+	if compiledOpts.waitDelay != nil {
+		cmd.WaitDelay = *compiledOpts.waitDelay
+	} else {
+		cmd.WaitDelay = DefaultWaitDelay
+	}
+	logger.DebugContext(ctx, "set wait delay", "delay", cmd.WaitDelay)
+
+	logger.DebugContext(ctx, "starting command", "args", args, "options", compiledOpts)
+	err := cmd.Run() // This blocks until the command exits or context is cancelled
+
+	// Exit code -1 if ProcessState is nil (e.g., start failed)
+	exitCode = -1
+	if cmd.ProcessState != nil {
+		exitCode = cmd.ProcessState.ExitCode()
+	}
 
-	err := cmd.Run()
 	if err != nil {
-		// Don't return error if both (a) the caller indicated they're OK with a
-		// nonzero exit code and (b) the error is of a type that means the only
-		// problem was a nonzero exit code.
 		var exitErr *exec.ExitError
-		if errors.As(err, &exitErr) && compiledOpts.allowNonZeroExit {
+		isExitError := errors.As(err, &exitErr)
+
+		if isExitError && compiledOpts.allowNonZeroExit {
+			logger.DebugContext(ctx, "command exited non-zero, but allowed by option", "exit_code", exitCode)
 			err = nil
+		} else if isExitError {
+			stdoutContent := "[stdout not captured]"
+			if stdoutBuf != nil {
+				stdoutContent = stdoutBuf.String()
+			}
+			stderrContent := "[stderr not captured]"
+			if stderrBuf != nil {
+				stderrContent = stderrBuf.String()
+			}
+			err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
+				args, exitCode, err, ctx.Err(), stdoutContent, stderrContent)
+			logger.DebugContext(ctx, "command exited non-zero", "exit_code", exitCode, "error", err)
 		} else {
-			err = fmt.Errorf(`run of %v failed: error was "%w", context error was "%w"\nstdout: %s\nstderr: %s`,
-				args, err, ctx.Err(), cmd.Stdout, cmd.Stderr)
+			// It's an actual execution error (e.g., command not found, context cancelled early)
+			stdoutContent := "[stdout not captured]"
+			if stdoutBuf != nil {
+				stdoutContent = stdoutBuf.String()
+			}
+			stderrContent := "[stderr not captured]"
+			if stderrBuf != nil {
+				stderrContent = stderrBuf.String()
+			}
+			err = fmt.Errorf("command %v failed: %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
+				args, err, ctx.Err(), stdoutContent, stderrContent)
+			logger.DebugContext(ctx, "command failed with execution error", "exit_code", exitCode, "error", err)
+		}
+	} else {
+		// Command ran successfully (exit code 0)
+		logger.DebugContext(ctx, "command finished successfully", "exit_code", exitCode)
+	}
+
+	return exitCode, err
+}
+
+// Option implements the functional options pattern for [Run].
+// It holds all configurable settings for executing a command.
+type Option struct {
+	// Basic execution control
+	allowNonZeroExit bool           // If true, non-zero exit codes are not treated as errors.
+	cwd              string         // Working directory for the command.
+	stdin            io.Reader      // Standard input for the command.
+	stdout           io.Writer      // Standard output destination.
+	stderr           io.Writer      // Standard error destination.
+	waitDelay        *time.Duration // Grace period after context cancel before killing.
+
+	// Environment control
+	allowedEnvKeys []string // Glob patterns for inherited env vars to allow.
+	deniedEnvKeys  []string // Glob patterns for inherited env vars to deny.
+	additionalEnv  []string // Env vars ("KEY=VALUE") to add/override.
+}
+
+// AllowNonzeroExit prevents Run from returning an error
+// when the command exits with a non-zero status code.
+func AllowNonzeroExit() *Option {
+	return &Option{allowNonZeroExit: true}
+}
+
+// WithStdin provides the given reader as the command's
+// standard input.
+func WithStdin(stdin io.Reader) *Option {
+	return &Option{stdin: stdin}
+}
+
+// WithStdinStr is a convenience option that uses the given string as the
+// command's standard input.
+func WithStdinStr(stdin string) *Option {
+	return WithStdin(bytes.NewBufferString(stdin))
+}
+
+// WithStdout directs the command's standard output
+// to the given writer.
+func WithStdout(stdout io.Writer) *Option {
+	return &Option{stdout: stdout}
+}
+
+// WithStderr directs the command's standard error
+// to the given writer.
+func WithStderr(stderr io.Writer) *Option {
+	return &Option{stderr: stderr}
+}
+
+// WithCwd runs the command in the specified working directory.
+func WithCwd(cwd string) *Option {
+	return &Option{cwd: cwd}
+}
+
+// WithFilteredEnv returns an option that filters the inherited environment variables.
+// allowed is a list of glob patterns for keys to keep (empty means allow all initially).
+// denied is a list of glob patterns for keys to remove (takes precedence).
+func WithFilteredEnv(allowed, denied []string) *Option {
+	return &Option{allowedEnvKeys: allowed, deniedEnvKeys: denied}
+}
+
+// WithAdditionalEnv returns an option that adds or overrides environment variables.
+// vars is a slice of strings in "KEY=VALUE" format.can
+func WithAdditionalEnv(vars []string) *Option {
+	return &Option{additionalEnv: vars}
+}
+
+// WithWaitDelay returns an option that sets a grace period for the command to
+// exit after the context is cancelled before being forcefully killed.
+// Default: DefautWaitDelay.
+// See exec.Cmd.WaitDelay for more information.
+func WithWaitDelay(d time.Duration) *Option {
+	return &Option{waitDelay: &d}
+}
+
+// TODO: handle process group and cancel semantics.
+// WithProcessGroup returns an option that attempts to run the command in a new
+// process group (primarily effective on Unix-like systems).
+//func WithProcessGroup(set bool) *Option {
+//	return &Option{setProcessGroup: set}
+//}
+
+// compileOpts merges multiple options into a single configuration struct.
+// The last option specified for most fields takes precedence.
+// additionalEnv is appended across options.
+func compileOpts(opts []*Option) *Option {
+	out := Option{}
+
+	for _, opt := range opts {
+		if opt.allowNonZeroExit {
+			out.allowNonZeroExit = true
+		}
+		if opt.cwd != "" {
+			out.cwd = opt.cwd
+		}
+		if opt.stdin != nil {
+			out.stdin = opt.stdin
+		}
+		if opt.stdout != nil {
+			out.stdout = opt.stdout
+		}
+		if opt.stderr != nil {
+			out.stderr = opt.stderr
+		}
+		if opt.waitDelay != nil {
+			out.waitDelay = opt.waitDelay
+		}
+		if opt.allowedEnvKeys != nil {
+			out.allowedEnvKeys = opt.allowedEnvKeys
+		}
+		if opt.deniedEnvKeys != nil {
+			out.deniedEnvKeys = opt.deniedEnvKeys
+		}
+		if len(opt.additionalEnv) > 0 {
+			out.additionalEnv = append(out.additionalEnv, opt.additionalEnv...)
+		}
+	}
+	return &out
+}
+
+// anyGlobMatch checks if string s matches any of the glob patterns.
+func anyGlobMatch(s string, patterns []string) bool {
+	for _, p := range patterns {
+		if p == "*" || p == ".*" { // Common fast-path wildcards
+			return true
 		}
 	}
-	return cmd.ProcessState.ExitCode(), err
+	for _, p := range patterns {
+		if ok, _ := filepath.Match(p, s); ok { // Ignore Match errors
+			return true
+		}
+	}
+	return false
+}
+
+// environ compiles the appropriate environment to pass to the child process.
+// The overridden environment is always added, even if not explicitly
+// allowed/denied.
+func environ(osEnv, allowedKeys, deniedKeys, overrideEnv []string) []string {
+	finalEnv := make([]string, 0, len(osEnv)+len(overrideEnv))
+
+	// Select keys that match the allow filter (if given) but not the deny filter.
+	for _, v := range osEnv {
+		k := strings.SplitN(v, "=", 2)[0]
+		if (len(allowedKeys) == 0 || anyGlobMatch(k, allowedKeys)) &&
+			!anyGlobMatch(k, deniedKeys) {
+			finalEnv = append(finalEnv, v)
+		}
+	}
+
+	// Add overrides at the end, after any filtering. os/exec has "last wins"
+	// semantics, so appending is an overriding action.
+	finalEnv = append(finalEnv, overrideEnv...)
+
+	return finalEnv
 }
diff --git a/run/exec_test.go b/run/exec_test.go
new file mode 100644
index 00000000..aaf13a1b
--- /dev/null
+++ b/run/exec_test.go
@@ -0,0 +1,61 @@
+package run
+
+import (
+	"github.com/abcxyz/pkg/testutil"
+	"github.com/google/go-cmp/cmp"
+	"testing"
+)
+
+// this may not be portable
+func TestSimple(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name       string
+		args       []string
+		wantOut    string
+		wantStdErr string
+		wantCode   int
+		wantErr    string
+	}{
+		{
+			name:    "happy_path",
+			args:    []string{"echo", "foo", "bar   bazz"},
+			wantOut: "foo bar   bazz\n",
+		},
+		{
+			name:     "no_command_error",
+			args:     []string{"echoooocrapimistyped", "foo", "bar   bazz"},
+			wantCode: -1,
+			wantErr:  "not found",
+		},
+		{
+			name:       "exit_code_error",
+			args:       []string{"cat", "/fake/file/path/should/fail"},
+			wantCode:   1,
+			wantStdErr: "cat: /fake/file/path/should/fail: No such file or directory\n",
+			wantErr:    "exited non-zero (1): exit status 1 (context error: <nil>)\nstdout:\n\nstderr:\ncat:",
+		},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+			ctx := t.Context()
+			stdout, stderr, exitCode, err := Simple(ctx, tc.args...)
+
+			if diff := cmp.Diff(stdout, tc.wantOut); diff != "" {
+				t.Errorf("stdout was not as expected(-got,+want): %s", diff)
+			}
+			if diff := cmp.Diff(stderr, tc.wantStdErr); diff != "" {
+				t.Errorf("stderr was not as expected(-got,+want): %s", diff)
+			}
+			if got, want := exitCode, tc.wantCode; got != want {
+				t.Errorf("exit code was not as expected, got: %d, want: %d", got, want)
+			}
+			if diff := testutil.DiffErrString(err, tc.wantErr); diff != "" {
+				t.Error(diff)
+			}
+		})
+	}
+}

From f3bf48981c39dbf96503168bab376cd935433c04 Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 15:22:07 -0700
Subject: [PATCH 03/11] remove copied files

---
 run/abc_version.go      | 178 ----------------------------------------
 run/guardian_version.go | 148 ---------------------------------
 2 files changed, 326 deletions(-)
 delete mode 100644 run/abc_version.go
 delete mode 100644 run/guardian_version.go

diff --git a/run/abc_version.go b/run/abc_version.go
deleted file mode 100644
index 7f76525d..00000000
--- a/run/abc_version.go
+++ /dev/null
@@ -1,178 +0,0 @@
-// Copyright 2023 The Authors (see AUTHORS file)
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package run
-
-import (
-	"bytes"
-	"context"
-	"errors"
-	"fmt"
-	"io"
-	"os/exec"
-	"time"
-)
-
-// DefaultRunTimeout is how long we'll wait for commands to run in the case
-// where the context doesn't already have a timeout. This was chosen
-// arbitrarily.
-const DefaultRunTimeout = time.Minute
-
-// Simple is a wrapper around [Run] that captures stdout and stderr as strings.
-// This is intended to be used for commands that run non-interactively then
-// exit.
-//
-// If the command exits with a nonzero status code, an *exec.ExitError will be
-// returned.
-//
-// If the command fails, the error message will include the contents of stdout
-// and stderr. This saves boilerplate in the caller.
-func Simple(ctx context.Context, args ...string) (stdout, stderr string, _ error) {
-	var stdoutBuf, stderrBuf bytes.Buffer
-	_, err := Run(ctx, []*Option{
-		WithStdout(&stdoutBuf),
-		WithStderr(&stderrBuf),
-	}, args...)
-	return stdoutBuf.String(), stderrBuf.String(), err
-}
-
-// Run runs the command provided by args, using the options in opts. By default,
-// if the command returns a nonzero exit code, an error is returned, but this
-// behavior may be overridden by the AllowNonzeroExit option.
-//
-// If the incoming context doesn't already have a timeout, then a default
-// timeout will be added (see DefaultRunTimeout).
-//
-// If the command fails, the error message will include the contents of stdout
-// and stderr. This saves boilerplate in the caller.
-//
-// The input args must have len>=1. opts may be nil if no special options are
-// needed.
-//
-// This doesn't execute a shell (unless of course args[0] is the name of a shell
-// binary).
-func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ error) {
-	if _, ok := ctx.Deadline(); !ok {
-		var cancel context.CancelFunc
-		ctx, cancel = context.WithTimeout(ctx, DefaultRunTimeout)
-		defer cancel()
-	}
-
-	cmd := exec.CommandContext(ctx, args[0], args[1:]...) //nolint:gosec // run'ing the input args is fundamentally the whole point
-
-	// any of these can be nil
-	compiledOpts := compileOpts(opts)
-	cmd.Stdout = compiledOpts.stdout
-	cmd.Stderr = compiledOpts.stderr
-	cmd.Stdin = compiledOpts.stdin
-	cmd.Dir = compiledOpts.cwd
-
-	err := cmd.Run()
-	if err != nil {
-		// Don't return error if both (a) the caller indicated they're OK with a
-		// nonzero exit code and (b) the error is of a type that means the only
-		// problem was a nonzero exit code.
-		var exitErr *exec.ExitError
-		if errors.As(err, &exitErr) && compiledOpts.allowNonZeroExit {
-			err = nil
-		} else {
-			err = fmt.Errorf(`run of %v failed: error was "%w", context error was "%w"\nstdout: %s\nstderr: %s`,
-				args, err, ctx.Err(), cmd.Stdout, cmd.Stderr)
-		}
-	}
-	return cmd.ProcessState.ExitCode(), err
-}
-
-// Many calls [Simple] for each command in args. If any command returns error,
-// then no further commands will be run, and that error will be returned. For
-// any commands that were actually executed (not aborted by a previous error),
-// their stdout and stderr will be returned. It's guaranteed that
-// len(stdouts)==len(stderrs).
-func Many(ctx context.Context, args ...[]string) (stdouts, stderrs []string, _ error) {
-	for _, cmd := range args {
-		stdout, stderr, err := Simple(ctx, cmd...)
-		stdouts = append(stdouts, stdout)
-		stderrs = append(stderrs, stderr)
-		if err != nil {
-			return stdouts, stderrs, err
-		}
-	}
-	return stdouts, stderrs, nil
-}
-
-// Option implements the functional options pattern for [Run].
-type Option struct {
-	allowNonZeroExit bool
-	cwd              string
-	stdin            io.Reader
-	stdout           io.Writer
-	stderr           io.Writer
-}
-
-// AllowNonzeroExit is an option that will NOT treat a nonzero exit code from
-// the command as an error (so [Run] won't return error). The default behavior
-// of [Run] is that if a command exits with a nonzero status code, then that
-// becomes a Go error.
-func AllowNonzeroExit() *Option {
-	return &Option{allowNonZeroExit: true}
-}
-
-// WithStdin passes the given reader as the command's standard input.
-func WithStdin(stdin io.Reader) *Option {
-	return &Option{stdin: stdin}
-}
-
-// WithStdinStr is a convenient wrapper around WithStdin that passes the given
-// string as the command's standard input.
-func WithStdinStr(stdin string) *Option {
-	return WithStdin(bytes.NewBufferString(stdin))
-}
-
-// WithStdout writes the command's standard output to the given writer.
-func WithStdout(stdout io.Writer) *Option {
-	return &Option{stdout: stdout}
-}
-
-// WithStderr writes the command's standard error to the given writer.
-func WithStderr(stderr io.Writer) *Option {
-	return &Option{stderr: stderr}
-}
-
-// WithCwd runs the command in the given working directory.
-func WithCwd(cwd string) *Option {
-	return &Option{cwd: cwd}
-}
-
-func compileOpts(opts []*Option) *Option {
-	var out Option
-	for _, opt := range opts {
-		if opt.allowNonZeroExit {
-			out.allowNonZeroExit = true
-		}
-		if opt.stdin != nil {
-			out.stdin = opt.stdin
-		}
-		if opt.stdout != nil {
-			out.stdout = opt.stdout
-		}
-		if opt.stderr != nil {
-			out.stderr = opt.stderr
-		}
-		if opt.cwd != "" {
-			out.cwd = opt.cwd
-		}
-	}
-
-	return &out
-}
diff --git a/run/guardian_version.go b/run/guardian_version.go
deleted file mode 100644
index 82c3cf35..00000000
--- a/run/guardian_version.go
+++ /dev/null
@@ -1,148 +0,0 @@
-// Copyright 2023 The Authors (see AUTHORS file)
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// Package child provides the functionality to execute child command line processes.
-package run
-
-import (
-	"context"
-	"fmt"
-	"io"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"strings"
-	"time"
-
-	"github.com/abcxyz/pkg/logging"
-)
-
-// RunConfig are the inputs for a run operation.
-type RunConfig struct {
-	Stdout     io.Writer
-	Stderr     io.Writer
-	WorkingDir string
-	Command    string
-	Args       []string
-
-	// AllowedEnvKeys and DeniedEnvKeys respectively define an allow/deny list of
-	// patterns for environment variable keys. Keys are matched using
-	// [filepath.Match].
-	AllowedEnvKeys []string
-	DeniedEnvKeys  []string
-
-	// OverrideEnvVars are the environment variables to inject into the child
-	// process, no matter what the user configured. These take precedence over all
-	// other configurables.
-	OverrideEnvVars []string
-}
-
-// Run executes a child process with the provided arguments.
-func Run(ctx context.Context, cfg *RunConfig) (int, error) {
-	logger := logging.FromContext(ctx).
-		With("working_dir", cfg.WorkingDir).
-		With("command", cfg.Command).
-		With("args", cfg.Args)
-
-	path, err := exec.LookPath(cfg.Command)
-	if err != nil {
-		return -1, fmt.Errorf("failed to locate command run path: %w", err)
-	}
-
-	cmd := exec.CommandContext(ctx, path)
-	setSysProcAttr(cmd)
-	setCancel(cmd)
-
-	if v := cfg.WorkingDir; v != "" {
-		cmd.Dir = v
-	}
-
-	stdout := cfg.Stdout
-	if stdout == nil {
-		stdout = os.Stdout
-	}
-
-	stderr := cfg.Stderr
-	if stderr == nil {
-		stderr = os.Stderr
-	}
-
-	cmd.Stdout = stdout
-	cmd.Stderr = stderr
-	cmd.Args = append(cmd.Args, cfg.Args...)
-
-	// Compute and set a custom environment for the child process.
-	env := environ(os.Environ(), cfg.AllowedEnvKeys, cfg.DeniedEnvKeys, cfg.OverrideEnvVars)
-	logger.DebugContext(ctx, "computed environment", "env", env)
-	cmd.Env = env
-
-	// add small wait delay to kill subprocesses if context is canceled
-	// https://github.com/golang/go/issues/23019
-	// https://github.com/golang/go/issues/50436
-	cmd.WaitDelay = 2 * time.Second
-
-	logger.DebugContext(ctx, "command started")
-
-	if err := cmd.Start(); err != nil {
-		return cmd.ProcessState.ExitCode(), fmt.Errorf("failed to start command: %w", err)
-	}
-
-	if err := cmd.Wait(); err != nil {
-		return cmd.ProcessState.ExitCode(), fmt.Errorf("failed to run command: %w", err)
-	}
-
-	exitCode := cmd.ProcessState.ExitCode()
-
-	logger.DebugContext(ctx, "command completed", "exit_code", exitCode)
-
-	return exitCode, nil
-}
-
-// environ compiles the appropriate environment to pass to the child process.
-// The overridden environment is always added, even if not explicitly
-// allowed/denied.
-func environ(osEnv, allowedKeys, deniedKeys, overrideEnv []string) []string {
-	finalEnv := make([]string, 0, len(osEnv)+len(overrideEnv))
-
-	// Select keys that match the allow filter (if given) but not the deny filter.
-	for _, v := range osEnv {
-		k := strings.SplitN(v, "=", 2)[0]
-		if (len(allowedKeys) == 0 || anyGlobMatch(k, allowedKeys)) &&
-			!anyGlobMatch(k, deniedKeys) {
-			finalEnv = append(finalEnv, v)
-		}
-	}
-
-	// Add overrides at the end, after any filtering
-	finalEnv = append(finalEnv, overrideEnv...)
-
-	return finalEnv
-}
-
-func anyGlobMatch(s string, patterns []string) bool {
-	// Short-circuit path matching logic for match-all.
-	for _, p := range patterns {
-		if p == "*" || p == ".*" {
-			return true
-		}
-	}
-
-	// Now do the slower lookup.
-	for _, p := range patterns {
-		if ok, _ := filepath.Match(p, s); ok {
-			return true
-		}
-	}
-	return false
-}

From 0ab0b1aedbffc2dba3e8b1775fc1a0872687f30a Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 15:22:47 -0700
Subject: [PATCH 04/11] remove copied code

---
 run/sys_default.go | 26 --------------------------
 run/sys_linux.go   | 40 ----------------------------------------
 2 files changed, 66 deletions(-)
 delete mode 100644 run/sys_default.go
 delete mode 100644 run/sys_linux.go

diff --git a/run/sys_default.go b/run/sys_default.go
deleted file mode 100644
index 0073365e..00000000
--- a/run/sys_default.go
+++ /dev/null
@@ -1,26 +0,0 @@
-// Copyright 2023 The Authors (see AUTHORS file)
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//go:build !linux
-// +build !linux
-
-package run
-
-import "os/exec"
-
-// setSysProcAttr sets the sysProcAttr.
-func setSysProcAttr(cmd *exec.Cmd) {}
-
-// setCancel sets the Cancel behavior for a child process.
-func setCancel(cmd *exec.Cmd) {}
diff --git a/run/sys_linux.go b/run/sys_linux.go
deleted file mode 100644
index 3be22c00..00000000
--- a/run/sys_linux.go
+++ /dev/null
@@ -1,40 +0,0 @@
-// Copyright 2023 The Authors (see AUTHORS file)
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//      http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-//go:build linux
-// +build linux
-
-package run
-
-import (
-	"os/exec"
-	"syscall"
-)
-
-// setSysProcAttr sets the sysProcAttr.
-func setSysProcAttr(cmd *exec.Cmd) {
-	cmd.SysProcAttr = &syscall.SysProcAttr{
-		// kill children if parent is dead
-		Pdeathsig: syscall.SIGKILL,
-		// set process group ID
-		Setpgid: true,
-	}
-}
-
-// setCancel sets the Cancel behavior for a child process.
-func setCancel(cmd *exec.Cmd) {
-	cmd.Cancel = func() error {
-		return cmd.Process.Signal(syscall.SIGQUIT) // Want passthrough
-	}
-}

From 8672cdbd05784da8542e5722a8cba82b2358072a Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 15:35:04 -0700
Subject: [PATCH 05/11] Remove exitcode from simple

---
 run/exec.go      |  6 +++---
 run/exec_test.go | 13 ++++---------
 2 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/run/exec.go b/run/exec.go
index 1bdd0bdb..4358060c 100644
--- a/run/exec.go
+++ b/run/exec.go
@@ -48,16 +48,16 @@ const DefaultWaitDelay = time.Second
 //
 // If the command fails, the error message will include the contents of stdout
 // and stderr. This saves boilerplate in the caller.
-func Simple(ctx context.Context, args ...string) (stdout, stderr string, exitCode int, _ error) {
+func Simple(ctx context.Context, args ...string) (stdout, stderr string, _ error) {
 	var stdoutBuf, stderrBuf bytes.Buffer
 	opts := []*Option{
 		WithStdout(&stdoutBuf),
 		WithStderr(&stderrBuf),
 	}
 
-	exitCode, err := Run(ctx, opts, args...)
+	_, err := Run(ctx, opts, args...)
 
-	return stdoutBuf.String(), stderrBuf.String(), exitCode, err
+	return stdoutBuf.String(), stderrBuf.String(), err
 }
 
 // Run executes the command specified by args, applying configurations from opts.
diff --git a/run/exec_test.go b/run/exec_test.go
index aaf13a1b..c6d423e4 100644
--- a/run/exec_test.go
+++ b/run/exec_test.go
@@ -24,15 +24,13 @@ func TestSimple(t *testing.T) {
 			wantOut: "foo bar   bazz\n",
 		},
 		{
-			name:     "no_command_error",
-			args:     []string{"echoooocrapimistyped", "foo", "bar   bazz"},
-			wantCode: -1,
-			wantErr:  "not found",
+			name:    "no_command_error",
+			args:    []string{"echoooocrapimistyped", "foo", "bar   bazz"},
+			wantErr: "not found",
 		},
 		{
 			name:       "exit_code_error",
 			args:       []string{"cat", "/fake/file/path/should/fail"},
-			wantCode:   1,
 			wantStdErr: "cat: /fake/file/path/should/fail: No such file or directory\n",
 			wantErr:    "exited non-zero (1): exit status 1 (context error: <nil>)\nstdout:\n\nstderr:\ncat:",
 		},
@@ -42,7 +40,7 @@ func TestSimple(t *testing.T) {
 		t.Run(tc.name, func(t *testing.T) {
 			t.Parallel()
 			ctx := t.Context()
-			stdout, stderr, exitCode, err := Simple(ctx, tc.args...)
+			stdout, stderr, err := Simple(ctx, tc.args...)
 
 			if diff := cmp.Diff(stdout, tc.wantOut); diff != "" {
 				t.Errorf("stdout was not as expected(-got,+want): %s", diff)
@@ -50,9 +48,6 @@ func TestSimple(t *testing.T) {
 			if diff := cmp.Diff(stderr, tc.wantStdErr); diff != "" {
 				t.Errorf("stderr was not as expected(-got,+want): %s", diff)
 			}
-			if got, want := exitCode, tc.wantCode; got != want {
-				t.Errorf("exit code was not as expected, got: %d, want: %d", got, want)
-			}
 			if diff := testutil.DiffErrString(err, tc.wantErr); diff != "" {
 				t.Error(diff)
 			}

From 018e6ed2a10fe53b1eb3c8a8bc968c2e42d5af94 Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 15:40:00 -0700
Subject: [PATCH 06/11] clean up

---
 run/exec.go | 30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/run/exec.go b/run/exec.go
index 4358060c..1cdcc219 100644
--- a/run/exec.go
+++ b/run/exec.go
@@ -190,17 +190,16 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 // It holds all configurable settings for executing a command.
 type Option struct {
 	// Basic execution control
-	allowNonZeroExit bool           // If true, non-zero exit codes are not treated as errors.
-	cwd              string         // Working directory for the command.
-	stdin            io.Reader      // Standard input for the command.
-	stdout           io.Writer      // Standard output destination.
-	stderr           io.Writer      // Standard error destination.
-	waitDelay        *time.Duration // Grace period after context cancel before killing.
-
-	// Environment control
-	allowedEnvKeys []string // Glob patterns for inherited env vars to allow.
-	deniedEnvKeys  []string // Glob patterns for inherited env vars to deny.
-	additionalEnv  []string // Env vars ("KEY=VALUE") to add/override.
+	allowNonZeroExit bool
+	cwd              string
+	stdin            io.Reader
+	stdout           io.Writer
+	stderr           io.Writer
+	waitDelay        *time.Duration
+
+	allowedEnvKeys []string
+	deniedEnvKeys  []string
+	additionalEnv  []string
 }
 
 // AllowNonzeroExit prevents Run from returning an error
@@ -238,7 +237,7 @@ func WithCwd(cwd string) *Option {
 	return &Option{cwd: cwd}
 }
 
-// WithFilteredEnv returns an option that filters the inherited environment variables.
+// WithFilteredEnv filters the inherited environment variables.
 // allowed is a list of glob patterns for keys to keep (empty means allow all initially).
 // denied is a list of glob patterns for keys to remove (takes precedence).
 func WithFilteredEnv(allowed, denied []string) *Option {
@@ -246,13 +245,13 @@ func WithFilteredEnv(allowed, denied []string) *Option {
 }
 
 // WithAdditionalEnv returns an option that adds or overrides environment variables.
-// vars is a slice of strings in "KEY=VALUE" format.can
+// vars is a slice of strings in "KEY=VALUE" format. Can be added multiple times.
 func WithAdditionalEnv(vars []string) *Option {
 	return &Option{additionalEnv: vars}
 }
 
-// WithWaitDelay returns an option that sets a grace period for the command to
-// exit after the context is cancelled before being forcefully killed.
+// WithWaitDelay sets a grace period for the command to exit after the context
+// is cancelled before being forcefully killed.
 // Default: DefautWaitDelay.
 // See exec.Cmd.WaitDelay for more information.
 func WithWaitDelay(d time.Duration) *Option {
@@ -266,7 +265,6 @@ func WithWaitDelay(d time.Duration) *Option {
 //	return &Option{setProcessGroup: set}
 //}
 
-// compileOpts merges multiple options into a single configuration struct.
 // The last option specified for most fields takes precedence.
 // additionalEnv is appended across options.
 func compileOpts(opts []*Option) *Option {

From f59a43e334b922a5f98017f21c2e8340a5bb92cf Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 15:46:51 -0700
Subject: [PATCH 07/11] Do some delinting

---
 run/exec.go      | 19 ++++++++++++++-----
 run/exec_test.go |  6 ++++--
 2 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/run/exec.go b/run/exec.go
index 1cdcc219..e6754438 100644
--- a/run/exec.go
+++ b/run/exec.go
@@ -136,7 +136,10 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 	}
 	logger.DebugContext(ctx, "set wait delay", "delay", cmd.WaitDelay)
 
-	logger.DebugContext(ctx, "starting command", "args", args, "options", compiledOpts)
+	logger.DebugContext(ctx, "starting command",
+		"args", args,
+		"options", compiledOpts,
+	)
 	err := cmd.Run() // This blocks until the command exits or context is cancelled
 
 	// Exit code -1 if ProcessState is nil (e.g., start failed)
@@ -161,9 +164,12 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 			if stderrBuf != nil {
 				stderrContent = stderrBuf.String()
 			}
-			err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
+			err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %w)\nstdout:\n%s\nstderr:\n%s",
 				args, exitCode, err, ctx.Err(), stdoutContent, stderrContent)
-			logger.DebugContext(ctx, "command exited non-zero", "exit_code", exitCode, "error", err)
+			logger.DebugContext(ctx, "command exited non-zero",
+				"exit_code", exitCode,
+				"error", err,
+			)
 		} else {
 			// It's an actual execution error (e.g., command not found, context cancelled early)
 			stdoutContent := "[stdout not captured]"
@@ -174,9 +180,12 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 			if stderrBuf != nil {
 				stderrContent = stderrBuf.String()
 			}
-			err = fmt.Errorf("command %v failed: %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
+			err = fmt.Errorf("command %v failed: %w (context error: %w)\nstdout:\n%s\nstderr:\n%s",
 				args, err, ctx.Err(), stdoutContent, stderrContent)
-			logger.DebugContext(ctx, "command failed with execution error", "exit_code", exitCode, "error", err)
+			logger.DebugContext(ctx, "command failed with execution error",
+				"exit_code", exitCode,
+				"error", err,
+			)
 		}
 	} else {
 		// Command ran successfully (exit code 0)
diff --git a/run/exec_test.go b/run/exec_test.go
index c6d423e4..214b0bf4 100644
--- a/run/exec_test.go
+++ b/run/exec_test.go
@@ -1,9 +1,11 @@
 package run
 
 import (
-	"github.com/abcxyz/pkg/testutil"
-	"github.com/google/go-cmp/cmp"
 	"testing"
+
+	"github.com/google/go-cmp/cmp"
+
+	"github.com/abcxyz/pkg/testutil"
 )
 
 // this may not be portable

From 060154081ec2f611431578302d7cce21acffbb23 Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 16:06:10 -0700
Subject: [PATCH 08/11] delint

---
 run/exec_test.go | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/run/exec_test.go b/run/exec_test.go
index 214b0bf4..f353c7e8 100644
--- a/run/exec_test.go
+++ b/run/exec_test.go
@@ -1,3 +1,17 @@
+// Copyright 2025 The Authors (see AUTHORS file)
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
 package run
 
 import (
@@ -8,7 +22,7 @@ import (
 	"github.com/abcxyz/pkg/testutil"
 )
 
-// this may not be portable
+// This may not be portable due to depending on behavior of external programs.
 func TestSimple(t *testing.T) {
 	t.Parallel()
 

From 31f55941eccd32dbfe2fa1f6802ee95ec0684f40 Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 16:07:56 -0700
Subject: [PATCH 09/11] linter is full of it

---
 run/exec.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/run/exec.go b/run/exec.go
index e6754438..1b26d14f 100644
--- a/run/exec.go
+++ b/run/exec.go
@@ -164,7 +164,7 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 			if stderrBuf != nil {
 				stderrContent = stderrBuf.String()
 			}
-			err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %w)\nstdout:\n%s\nstderr:\n%s",
+			err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
 				args, exitCode, err, ctx.Err(), stdoutContent, stderrContent)
 			logger.DebugContext(ctx, "command exited non-zero",
 				"exit_code", exitCode,
@@ -180,7 +180,7 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 			if stderrBuf != nil {
 				stderrContent = stderrBuf.String()
 			}
-			err = fmt.Errorf("command %v failed: %w (context error: %w)\nstdout:\n%s\nstderr:\n%s",
+			err = fmt.Errorf("command %v failed: %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
 				args, err, ctx.Err(), stdoutContent, stderrContent)
 			logger.DebugContext(ctx, "command failed with execution error",
 				"exit_code", exitCode,

From 6a3140161cb4d893da1338c182f094abf610f3c3 Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Mon, 21 Apr 2025 16:11:07 -0700
Subject: [PATCH 10/11] BE GONE EVIL LINTER

---
 run/exec.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/run/exec.go b/run/exec.go
index 1b26d14f..9bac2633 100644
--- a/run/exec.go
+++ b/run/exec.go
@@ -165,7 +165,7 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 				stderrContent = stderrBuf.String()
 			}
 			err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
-				args, exitCode, err, ctx.Err(), stdoutContent, stderrContent)
+				args, exitCode, err, ctx.Err(), stdoutContent, stderrContent) //nolint:errorlint
 			logger.DebugContext(ctx, "command exited non-zero",
 				"exit_code", exitCode,
 				"error", err,
@@ -181,7 +181,7 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 				stderrContent = stderrBuf.String()
 			}
 			err = fmt.Errorf("command %v failed: %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
-				args, err, ctx.Err(), stdoutContent, stderrContent)
+				args, err, ctx.Err(), stdoutContent, stderrContent) //nolint:errorlint
 			logger.DebugContext(ctx, "command failed with execution error",
 				"exit_code", exitCode,
 				"error", err,

From bc9aa3d522b58c7b2df73b6730073b078f3a8e8d Mon Sep 17 00:00:00 2001
From: pdewilde <pdewilde@google.com>
Date: Tue, 22 Apr 2025 16:39:04 -0700
Subject: [PATCH 11/11] Address pr feedback

---
 run/exec.go | 234 ++++++++++++----------------------------------------
 1 file changed, 52 insertions(+), 182 deletions(-)

diff --git a/run/exec.go b/run/exec.go
index 9bac2633..7acddbf3 100644
--- a/run/exec.go
+++ b/run/exec.go
@@ -26,6 +26,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"slices"
 	"strings"
 	"time"
 
@@ -43,6 +44,8 @@ const DefaultWaitDelay = time.Second
 // This is intended to be used for commands that run non-interactively then
 // exit.
 //
+// Sub-command inherits env variables.
+//
 // If the command exits with a nonzero status code, an *exec.ExitError will be
 // returned.
 //
@@ -50,9 +53,10 @@ const DefaultWaitDelay = time.Second
 // and stderr. This saves boilerplate in the caller.
 func Simple(ctx context.Context, args ...string) (stdout, stderr string, _ error) {
 	var stdoutBuf, stderrBuf bytes.Buffer
-	opts := []*Option{
-		WithStdout(&stdoutBuf),
-		WithStderr(&stderrBuf),
+	opts := &Option{
+		Stdout: &stdoutBuf,
+		Stderr: &stderrBuf,
+		Env:    os.Environ(),
 	}
 
 	_, err := Run(ctx, opts, args...)
@@ -73,7 +77,7 @@ func Simple(ctx context.Context, args ...string) (stdout, stderr string, _ error
 //
 // This doesn't execute a shell (unless of course args[0] is the name of a shell
 // binary).
-func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ error) {
+func Run(ctx context.Context, opts *Option, args ...string) (exitCode int, _ error) {
 	logger := logging.FromContext(ctx)
 
 	if len(args) == 0 {
@@ -88,49 +92,27 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 		defer cancel() // Ensure the derived context is cancelled
 	}
 
-	compiledOpts := compileOpts(opts)
-
 	// #nosec G204 -- Execution of external commands is the purpose of this package. Inputs must be trusted by the caller.
 	cmd := exec.CommandContext(ctx, args[0], args[1:]...)
 
-	cmd.Dir = compiledOpts.cwd
-	cmd.Stdin = compiledOpts.stdin
-	// Handle stdout/stderr, capturing output if writers are bytes.Buffer for error reporting.
-	var stdoutBuf, stderrBuf *bytes.Buffer
-	if compiledOpts.stdout != nil {
-		cmd.Stdout = compiledOpts.stdout
-		if bb, ok := compiledOpts.stdout.(*bytes.Buffer); ok {
-			stdoutBuf = bb
-		}
+	cmd.Dir = opts.Cwd
+	cmd.Stdin = opts.Stdin
+
+	if opts.Stdout != nil {
+		cmd.Stdout = opts.Stdout
 	} else {
 		cmd.Stdout = os.Stdout
 	}
-	if compiledOpts.stderr != nil {
-		cmd.Stderr = compiledOpts.stderr
-		if bb, ok := compiledOpts.stderr.(*bytes.Buffer); ok {
-			stderrBuf = bb
-		}
+	if opts.Stderr != nil {
+		cmd.Stderr = opts.Stderr
 	} else {
 		cmd.Stderr = os.Stderr
 	}
 
-	useCustomEnv := len(compiledOpts.allowedEnvKeys) > 0 || len(compiledOpts.deniedEnvKeys) > 0 || len(compiledOpts.additionalEnv) > 0
-	if useCustomEnv {
-		currentEnv := os.Environ()
-		logger.DebugContext(ctx, "calculating custom environment",
-			"inherited_count", len(currentEnv),
-			"allowed_patterns", compiledOpts.allowedEnvKeys,
-			"denied_patterns", compiledOpts.deniedEnvKeys,
-			"additional_vars_count", len(compiledOpts.additionalEnv))
-		env := environ(currentEnv, compiledOpts.allowedEnvKeys, compiledOpts.deniedEnvKeys, compiledOpts.additionalEnv)
-		logger.DebugContext(ctx, "computed environment", "env", env)
-		cmd.Env = env
-	} else {
-		logger.DebugContext(ctx, "using inherited environment")
-	}
+	cmd.Env = opts.Env
 
-	if compiledOpts.waitDelay != nil {
-		cmd.WaitDelay = *compiledOpts.waitDelay
+	if opts.WaitDelay != nil {
+		cmd.WaitDelay = *opts.WaitDelay
 	} else {
 		cmd.WaitDelay = DefaultWaitDelay
 	}
@@ -138,7 +120,7 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 
 	logger.DebugContext(ctx, "starting command",
 		"args", args,
-		"options", compiledOpts,
+		"options", opts,
 	)
 	err := cmd.Run() // This blocks until the command exits or context is cancelled
 
@@ -150,42 +132,19 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 
 	if err != nil {
 		var exitErr *exec.ExitError
-		isExitError := errors.As(err, &exitErr)
 
-		if isExitError && compiledOpts.allowNonZeroExit {
-			logger.DebugContext(ctx, "command exited non-zero, but allowed by option", "exit_code", exitCode)
-			err = nil
-		} else if isExitError {
-			stdoutContent := "[stdout not captured]"
-			if stdoutBuf != nil {
-				stdoutContent = stdoutBuf.String()
-			}
-			stderrContent := "[stderr not captured]"
-			if stderrBuf != nil {
-				stderrContent = stderrBuf.String()
-			}
-			err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
-				args, exitCode, err, ctx.Err(), stdoutContent, stderrContent) //nolint:errorlint
-			logger.DebugContext(ctx, "command exited non-zero",
-				"exit_code", exitCode,
-				"error", err,
-			)
-		} else {
-			// It's an actual execution error (e.g., command not found, context cancelled early)
-			stdoutContent := "[stdout not captured]"
-			if stdoutBuf != nil {
-				stdoutContent = stdoutBuf.String()
+		if errors.As(err, &exitErr) {
+			if opts.AllowedExitCodes[0] == -1 || slices.Contains(opts.AllowedExitCodes, exitCode) {
+				logger.DebugContext(ctx, "command exited non-zero, but allowed by option", "exit_code", exitCode)
+				err = nil
+			} else {
+				logger.DebugContext(ctx, "command exited non-zero",
+					"exit_code", exitCode,
+					"error", err,
+				)
+				err = fmt.Errorf("command %v exited non-zero (%d): %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
+					args, exitCode, err, ctx.Err(), cmd.Stdout, cmd.Stderr) //nolint:errorlint
 			}
-			stderrContent := "[stderr not captured]"
-			if stderrBuf != nil {
-				stderrContent = stderrBuf.String()
-			}
-			err = fmt.Errorf("command %v failed: %w (context error: %v)\nstdout:\n%s\nstderr:\n%s",
-				args, err, ctx.Err(), stdoutContent, stderrContent) //nolint:errorlint
-			logger.DebugContext(ctx, "command failed with execution error",
-				"exit_code", exitCode,
-				"error", err,
-			)
 		}
 	} else {
 		// Command ran successfully (exit code 0)
@@ -198,118 +157,29 @@ func Run(ctx context.Context, opts []*Option, args ...string) (exitCode int, _ e
 // Option implements the functional options pattern for [Run].
 // It holds all configurable settings for executing a command.
 type Option struct {
-	// Basic execution control
-	allowNonZeroExit bool
-	cwd              string
-	stdin            io.Reader
-	stdout           io.Writer
-	stderr           io.Writer
-	waitDelay        *time.Duration
-
-	allowedEnvKeys []string
-	deniedEnvKeys  []string
-	additionalEnv  []string
-}
-
-// AllowNonzeroExit prevents Run from returning an error
-// when the command exits with a non-zero status code.
-func AllowNonzeroExit() *Option {
-	return &Option{allowNonZeroExit: true}
-}
-
-// WithStdin provides the given reader as the command's
-// standard input.
-func WithStdin(stdin io.Reader) *Option {
-	return &Option{stdin: stdin}
-}
-
-// WithStdinStr is a convenience option that uses the given string as the
-// command's standard input.
-func WithStdinStr(stdin string) *Option {
-	return WithStdin(bytes.NewBufferString(stdin))
-}
-
-// WithStdout directs the command's standard output
-// to the given writer.
-func WithStdout(stdout io.Writer) *Option {
-	return &Option{stdout: stdout}
-}
-
-// WithStderr directs the command's standard error
-// to the given writer.
-func WithStderr(stderr io.Writer) *Option {
-	return &Option{stderr: stderr}
-}
-
-// WithCwd runs the command in the specified working directory.
-func WithCwd(cwd string) *Option {
-	return &Option{cwd: cwd}
-}
-
-// WithFilteredEnv filters the inherited environment variables.
-// allowed is a list of glob patterns for keys to keep (empty means allow all initially).
-// denied is a list of glob patterns for keys to remove (takes precedence).
-func WithFilteredEnv(allowed, denied []string) *Option {
-	return &Option{allowedEnvKeys: allowed, deniedEnvKeys: denied}
-}
-
-// WithAdditionalEnv returns an option that adds or overrides environment variables.
-// vars is a slice of strings in "KEY=VALUE" format. Can be added multiple times.
-func WithAdditionalEnv(vars []string) *Option {
-	return &Option{additionalEnv: vars}
-}
-
-// WithWaitDelay sets a grace period for the command to exit after the context
-// is cancelled before being forcefully killed.
-// Default: DefautWaitDelay.
-// See exec.Cmd.WaitDelay for more information.
-func WithWaitDelay(d time.Duration) *Option {
-	return &Option{waitDelay: &d}
+	// AllowNonzeroExitCodes prevents Run from returning error when one of
+	// the listed non-zero status codes appear. If AllowedExitCodes[0] == -1,
+	// all exit codes are allowed.
+	AllowedExitCodes []int
+
+	// Runs command in specified working directory.
+	Cwd string
+	// Env contains the list of env vars in KEY=VALUE form. Default is no env vars.
+	Env []string
+	// Sets the cmd's Stdin. Defaults is no Stdin.
+	Stdin io.Reader
+	// Sets the cmd's Stdout. Default is os.Stdout.
+	Stdout io.Writer
+	// Sets the cmd's Stderr. Default is os.Stderr.
+	Stderr io.Writer
+	// Sets a grace period for the command to exit after the context
+	// is cancelled before being forcefully killed.
+	// Default: DefautWaitDelay. 0 is no wait delay.
+	// See exec.Cmd.WaitDelay for more information.
+	WaitDelay *time.Duration
 }
 
 // TODO: handle process group and cancel semantics.
-// WithProcessGroup returns an option that attempts to run the command in a new
-// process group (primarily effective on Unix-like systems).
-//func WithProcessGroup(set bool) *Option {
-//	return &Option{setProcessGroup: set}
-//}
-
-// The last option specified for most fields takes precedence.
-// additionalEnv is appended across options.
-func compileOpts(opts []*Option) *Option {
-	out := Option{}
-
-	for _, opt := range opts {
-		if opt.allowNonZeroExit {
-			out.allowNonZeroExit = true
-		}
-		if opt.cwd != "" {
-			out.cwd = opt.cwd
-		}
-		if opt.stdin != nil {
-			out.stdin = opt.stdin
-		}
-		if opt.stdout != nil {
-			out.stdout = opt.stdout
-		}
-		if opt.stderr != nil {
-			out.stderr = opt.stderr
-		}
-		if opt.waitDelay != nil {
-			out.waitDelay = opt.waitDelay
-		}
-		if opt.allowedEnvKeys != nil {
-			out.allowedEnvKeys = opt.allowedEnvKeys
-		}
-		if opt.deniedEnvKeys != nil {
-			out.deniedEnvKeys = opt.deniedEnvKeys
-		}
-		if len(opt.additionalEnv) > 0 {
-			out.additionalEnv = append(out.additionalEnv, opt.additionalEnv...)
-		}
-	}
-	return &out
-}
 
 // anyGlobMatch checks if string s matches any of the glob patterns.
 func anyGlobMatch(s string, patterns []string) bool {
@@ -326,10 +196,10 @@ func anyGlobMatch(s string, patterns []string) bool {
 	return false
 }
 
-// environ compiles the appropriate environment to pass to the child process.
+// Environ is a utility to compile environment variables to pass to the child.
 // The overridden environment is always added, even if not explicitly
 // allowed/denied.
-func environ(osEnv, allowedKeys, deniedKeys, overrideEnv []string) []string {
+func Environ(osEnv, allowedKeys, deniedKeys, overrideEnv []string) []string {
 	finalEnv := make([]string, 0, len(osEnv)+len(overrideEnv))
 
 	// Select keys that match the allow filter (if given) but not the deny filter.