Add administrator-configured Exec wrapper (#68)

README.md

@@ -22,3 +22,39 @@ To run all features appropriate for a given context, use component groups:
 * `--run-agent`
     * implies `--run-vdagent --run-rpc` 
     * example usage: [`tart-guest-agent.plist`](https://github.com/cirruslabs/macos-image-templates/blob/main/data/tart-guest-agent.plist)
+
+## Wrapping RPC commands
+
+An image administrator can configure a fixed command prefix with repeated
+`--exec-wrapper` flags. The guest agent appends the requested executable and its
+arguments without shell interpolation. The prefix applies to every Exec RPC,
+including interactive, PTY, detached, and user-override commands; clients cannot
+disable it. With no prefix, execution is unchanged.
+
+For example, a managed image can add an environment variable to every command:
+
+```sh
+tart-guest-agent --run-agent \
+  --exec-wrapper=/usr/bin/env \
+  --exec-wrapper=-- \
+  --exec-wrapper=MANAGED_IMAGE=example
+```
+
+The first argument must be an absolute path to a regular file that the guest
+agent's effective user can execute. Invalid configuration stops startup. A wrapper
+that cannot start never falls back to running the requested command directly.
+Attached commands return the wrapper's
+exit status; detached commands retain their existing process-start acknowledgment.
+Use a wrapper that replaces itself with the command so signals and exit handling
+retain their usual behavior.
+
+The wrapper receives the command name unchanged and handles its executable
+lookup. Include its end-of-options marker in the prefix when its interface
+requires one.
+
+The wrapper runs with the command's requested environment, working directory,
+and user. A requested user must also be able to execute the wrapper. Keep its
+executable, configuration, and launch settings under the image administrator's
+control, and choose a wrapper whose behavior remains correct
+under those overrides. Only commands started through the guest agent use this
+prefix.

internal/command/root.go

@@ -26,6 +26,7 @@ import (
 var resizeDisk bool
 var runVdagent bool
 var runRPC bool
+var execWrapper []string
 
 var runDaemon bool
 var runAgent bool
@@ -56,6 +57,8 @@ func NewRootCommand() *cobra.Command {
 	cmd.Flags().BoolVar(&runVdagent, "run-vdagent", false, "run vdagent")
 	cmd.Flags().BoolVar(&runRPC, "run-rpc", false, "run RPC service (currently required "+
 		"to support \"tart exec\" functionality)")
+	cmd.Flags().StringArrayVar(&execWrapper, "exec-wrapper", nil,
+		"argv prefix for every RPC command; repeat for each argument (first must be an absolute executable path)")
 
 	// Component groups
 	cmd.Flags().BoolVar(&runDaemon, "run-daemon", false, "identical to running the agent"+
@@ -90,6 +93,13 @@ func run(cmd *cobra.Command, args []string) error {
 		zap.S().Infof("running on Tart %s, proceeding...", version.String())
 	}
 
+	if len(execWrapper) > 0 && !runRPC {
+		return errors.New("--exec-wrapper requires --run-rpc or --run-agent")
+	}
+	if err := rpc.ValidateExecWrapper(execWrapper); err != nil {
+		return err
+	}
+
 	// Perform disk resizing
 	if resizeDisk {
 		zap.S().Info("attempting to resize disk...")
@@ -188,7 +198,7 @@ func runRPCOnce(ctx context.Context) error {
 	}
 	defer listener.Close()
 
-	rpcServer, err := rpc.New(listener)
+	rpcServer, err := rpc.New(listener, execWrapper...)
 	if err != nil {
 		zap.S().Errorf("failed to initialize RPC server: %v", err)
 

internal/command/root_test.go

@@ -0,0 +1,30 @@
+package command_test
+
+import (
+	"testing"
+
+	"github.com/cirruslabs/tart-guest-agent/internal/command"
+	"github.com/stretchr/testify/require"
+)
+
+func TestExecWrapperFlagPreservesArguments(t *testing.T) {
+	cmd := command.NewRootCommand()
+	require.NoError(t, cmd.ParseFlags([]string{
+		"--run-rpc",
+		"--exec-wrapper=/usr/bin/env",
+		"--exec-wrapper=--",
+		"--exec-wrapper=NAME=value,with spaces",
+		"--exec-wrapper=",
+		"--exec-wrapper=literal $HOME; $(false)",
+	}))
+	argv, err := cmd.Flags().GetStringArray("exec-wrapper")
+	require.NoError(t, err)
+	require.Equal(t, []string{
+		"/usr/bin/env", "--", "NAME=value,with spaces", "", "literal $HOME; $(false)",
+	}, argv)
+
+	defaultCommand := command.NewRootCommand()
+	argv, err = defaultCommand.Flags().GetStringArray("exec-wrapper")
+	require.NoError(t, err)
+	require.Empty(t, argv)
+}

internal/rpc/exec.go

@@ -62,8 +62,8 @@ func (rpc *RPC) Exec(stream grpc.BidiStreamingServer[v1.ExecRequest, v1.ExecResp
 		execCtx = context.Background()
 	}
 
-	cmd := exec.CommandContext(execCtx, firstExecRequestCommand.Command.Name,
-		firstExecRequestCommand.Command.Args...)
+	cmd := rpc.execCommand(execCtx, firstExecRequestCommand.Command.GetName(),
+		firstExecRequestCommand.Command.GetArgs())
 
 	cmd.SysProcAttr = &syscall.SysProcAttr{}
 

internal/rpc/exec_grpc_test.go

@@ -25,9 +25,24 @@ import (
 )
 
 func TestExecGRPCBackgroundProcessLifetime(t *testing.T) {
+	testExecGRPCBackgroundProcessLifetime(t)
+}
+
+func TestExecWrapperGRPCBackgroundProcessLifetime(t *testing.T) {
+	receipt := filepath.Join(t.TempDir(), "wrapper-ran")
+	wrapper := writeExecWrapper(t, `printf wrapped > "$1"
+shift
+exec "$@"
+`)
+	testExecGRPCBackgroundProcessLifetime(t, wrapper, receipt)
+	require.FileExists(t, receipt)
+}
+
+func testExecGRPCBackgroundProcessLifetime(t *testing.T, execWrapper ...string) {
+	t.Helper()
 	for _, mode := range []string{"normal exit", "cancel after exit", "cancel while running"} {
 		t.Run(mode, func(t *testing.T) {
-			client := v1.NewAgentClient(newExecGRPCTestConn(t))
+			client := v1.NewAgentClient(newExecGRPCTestConn(t, execWrapper...))
 			ctx, cancel := context.WithTimeout(t.Context(), execTestTimeout)
 			defer cancel()
 
@@ -93,9 +108,9 @@ while [ ! -f "$RELEASE_FILE" ]; do sleep 0.01; done`},
 	}
 }
 
-func newExecGRPCTestConn(t *testing.T) *grpc.ClientConn {
+func newExecGRPCTestConn(t *testing.T, execWrapper ...string) *grpc.ClientConn {
 	listener := bufconn.Listen(1024 * 1024)
-	agent, err := New(listener)
+	agent, err := New(listener, execWrapper...)
 	require.NoError(t, err)
 
 	serveResult := make(chan error, 1)

internal/rpc/exec_test.go

@@ -290,6 +290,18 @@ func startExecTest(
 	configure ...func(*execTestStream),
 ) (*RPC, *execTestStream, <-chan error) {
 	t.Helper()
+	rpc, err := New(nil)
+	require.NoError(t, err)
+	return startExecTestWithRPC(t, rpc, command, configure...)
+}
+
+func startExecTestWithRPC(
+	t *testing.T,
+	rpc *RPC,
+	command *v1.ExecRequest_Command,
+	configure ...func(*execTestStream),
+) (*RPC, *execTestStream, <-chan error) {
+	t.Helper()
 
 	ctx, cancel := context.WithCancel(context.Background())
 	t.Cleanup(cancel)
@@ -297,8 +309,6 @@ func startExecTest(
 	for _, configureStream := range configure {
 		configureStream(stream)
 	}
-	rpc, err := New(nil)
-	require.NoError(t, err)
 	result := make(chan error, 1)
 	go func() {
 		result <- rpc.Exec(stream)

internal/rpc/exec_wrapper.go

@@ -0,0 +1,60 @@
+package rpc
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+
+	"golang.org/x/sys/unix"
+)
+
+// ValidateExecWrapper checks the administrator-configured argv prefix.
+// The wrapper itself is responsible for performing its work and executing the
+// appended command. No request can change this prefix.
+func ValidateExecWrapper(argv []string) error {
+	if len(argv) == 0 {
+		return nil
+	}
+
+	if !filepath.IsAbs(argv[0]) {
+		return errors.New("exec wrapper executable must be an absolute path")
+	}
+
+	for _, arg := range argv {
+		if strings.ContainsRune(arg, '\x00') {
+			return errors.New("exec wrapper arguments must not contain NUL")
+		}
+	}
+
+	info, err := os.Stat(argv[0])
+	if err != nil {
+		return fmt.Errorf("invalid exec wrapper executable: %w", err)
+	}
+
+	if !info.Mode().IsRegular() {
+		return errors.New("exec wrapper must name an executable file")
+	}
+	if err := unix.Faccessat(unix.AT_FDCWD, argv[0], unix.X_OK, unix.AT_EACCESS); err != nil {
+		return fmt.Errorf("exec wrapper is not executable by the guest agent: %w", err)
+	}
+
+	return nil
+}
+
+func (rpc *RPC) execCommand(ctx context.Context, name string, args []string) *exec.Cmd {
+	if len(rpc.execWrapper) == 0 {
+		return exec.CommandContext(ctx, name, args...) // #nosec G204 -- Executing RPC commands is this service's purpose.
+	}
+
+	wrappedArgs := make([]string, 0, len(rpc.execWrapper)+len(args))
+	wrappedArgs = append(wrappedArgs, rpc.execWrapper[1:]...)
+	wrappedArgs = append(wrappedArgs, name)
+	wrappedArgs = append(wrappedArgs, args...)
+
+	// #nosec G204 -- Fixed, validated administrator prefix.
+	return exec.CommandContext(ctx, rpc.execWrapper[0], wrappedArgs...)
+}

internal/rpc/exec_wrapper_test.go

@@ -0,0 +1,181 @@
+//nolint:testpackage
+package rpc
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/cirruslabs/tart-guest-agent/pkg/v1"
+	"github.com/stretchr/testify/require"
+)
+
+func TestValidateExecWrapper(t *testing.T) {
+	notExecutable := filepath.Join(t.TempDir(), "not-executable")
+	require.NoError(t, os.WriteFile(notExecutable, nil, 0o600))
+
+	for _, argv := range [][]string{
+		{""}, {"sh"}, {"/missing/tart-exec-wrapper"}, {t.TempDir()},
+		{notExecutable}, {execTestShell, "bad\x00argument"},
+	} {
+		require.Error(t, ValidateExecWrapper(argv), "%q", argv)
+		_, err := New(nil, argv...)
+		require.Error(t, err)
+	}
+
+	require.NoError(t, ValidateExecWrapper(nil))
+	require.NoError(t, ValidateExecWrapper([]string{execTestShell, "", "literal,$value"}))
+}
+
+func TestValidateExecWrapperRequiresApplicableExecutePermission(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root can execute a file with any execute bit set")
+	}
+
+	path := filepath.Join(t.TempDir(), "other-executable")
+	contents, err := os.ReadFile("/usr/bin/true")
+	require.NoError(t, err)
+	require.NoError(t, os.WriteFile(path, contents, 0o600)) // #nosec G703 -- Destination is inside t.TempDir.
+	require.NoError(t, os.Chmod(path, 0o001))               // #nosec G302 -- Tests inapplicable execute permission.
+	// The current user owns this file, so its other-execute bit does not apply.
+	// #nosec G204 -- Path is an owned permission-test fixture.
+	require.ErrorIs(t, exec.CommandContext(t.Context(), path).Run(), os.ErrPermission)
+	require.ErrorIs(t, ValidateExecWrapper([]string{path}), os.ErrPermission)
+	_, err = New(nil, path)
+	require.ErrorIs(t, err, os.ErrPermission)
+}
+
+func TestExecWrapperPreservesArgumentsAndOverrides(t *testing.T) {
+	workdir := t.TempDir()
+	canonicalWorkdir, err := filepath.EvalSymlinks(workdir)
+	require.NoError(t, err)
+	wrapper := writeExecWrapper(t, `printf '%s\n' "$1"
+shift
+exec "$@"
+`)
+	prefix := []string{wrapper, "fixed,argument $HOME; $(false)"}
+	rpc, err := New(nil, prefix...)
+	require.NoError(t, err)
+	prefix[0] = "/missing/changed-after-startup"
+	prefix[1] = "changed-after-startup"
+
+	_, stream, result := startExecTestWithRPC(t, rpc, &v1.ExecRequest_Command{
+		Name:    execTestShell,
+		Args:    []string{"-c", `printf '%s\n' "$PWD" "$WRAPPER_TEST_VALUE" "$1"; id -u`, "test", "literal; $(false)"},
+		Env:     map[string]string{"WRAPPER_TEST_VALUE": "value with spaces"},
+		Workdir: workdir,
+		User:    strconv.Itoa(os.Getuid()),
+	})
+	output, exitCode := collectExecWrapperResult(t, stream)
+	require.Zero(t, exitCode)
+	require.Equal(t, fmt.Sprintf("fixed,argument $HOME; $(false)\n%s\nvalue with spaces\nliteral; $(false)\n%d\n",
+		canonicalWorkdir, os.Getuid()), output)
+	require.NoError(t, receiveExecResult(t, result))
+}
+
+func TestExecWrapperDeniesEveryExecutionMode(t *testing.T) {
+	const detachedMode = "detached"
+	for _, mode := range []string{"normal", "interactive", "pty", detachedMode} {
+		t.Run(mode, func(t *testing.T) {
+			directory := t.TempDir()
+			receipt := filepath.Join(directory, "wrapper-ran")
+			target := filepath.Join(directory, "target-ran")
+			wrapper := writeExecWrapper(t, `printf denied > "$WRAPPER_RECEIPT"
+exit 86
+`)
+			rpc, err := New(nil, wrapper)
+			require.NoError(t, err)
+			_, stream, result := startExecTestWithRPC(t, rpc, &v1.ExecRequest_Command{
+				Name:        execTestShell,
+				Args:        []string{"-c", `printf unrestricted > "$TARGET_RECEIPT"`},
+				Env:         map[string]string{"WRAPPER_RECEIPT": receipt, "TARGET_RECEIPT": target},
+				Interactive: mode == "interactive",
+				Tty:         mode == "pty",
+				Detach:      mode == detachedMode,
+			})
+			_, exitCode := collectExecWrapperResult(t, stream)
+			if mode == detachedMode {
+				require.Zero(t, exitCode) // Detached success acknowledges process creation.
+			} else {
+				require.EqualValues(t, 86, exitCode)
+			}
+			require.NoError(t, receiveExecResult(t, result))
+			require.Eventually(t, func() bool { _, err := os.Stat(receipt); return err == nil },
+				execTestTimeout, 10*time.Millisecond)
+			_, err = os.Stat(target)
+			require.ErrorIs(t, err, os.ErrNotExist)
+		})
+	}
+}
+
+func TestExecWrapperDisappearingDoesNotRunTarget(t *testing.T) {
+	wrapper := writeExecWrapper(t, "exec \"$@\"\n")
+	rpc, err := New(nil, wrapper)
+	require.NoError(t, err)
+	require.NoError(t, os.Remove(wrapper))
+
+	_, stream, result := startExecTestWithRPC(t, rpc, &v1.ExecRequest_Command{Name: "/usr/bin/true"})
+	response := receiveExecResponse(t, stream)
+	require.Nil(t, response.GetStarted())
+	require.EqualValues(t, execRuntimeFailureExitCode, response.GetExit().GetCode())
+	require.NoError(t, receiveExecResult(t, result))
+}
+
+func TestExecWrapperSignalsProcessGroup(t *testing.T) {
+	for _, tty := range []bool{false, true} {
+		t.Run(fmt.Sprintf("tty=%t", tty), func(t *testing.T) {
+			wrapper := writeExecWrapper(t, "exec \"$@\"\n")
+			rpc, err := New(nil, wrapper)
+			require.NoError(t, err)
+			_, stream, result := startExecTestWithRPC(t, rpc, &v1.ExecRequest_Command{
+				Name: execTestShell,
+				Args: []string{"-c", "sleep 30 & printf ready; wait"},
+				Tty:  tty,
+			})
+			started := receiveExecResponse(t, stream).GetStarted()
+			require.NotNil(t, started)
+			require.Equal(t, []byte("ready"), receiveExecResponse(t, stream).GetStandardOutput().GetData())
+
+			_, err = rpc.Signal(context.Background(), &v1.SignalRequest{
+				ExecId: started.GetExecId(),
+				Signal: v1.SignalRequest_SIGNAL_SIGTERM,
+			})
+			require.NoError(t, err)
+			response := receiveExecResponse(t, stream)
+			require.EqualValues(t, signalExitCodeOffset+syscall.SIGTERM, response.GetExit().GetCode())
+			require.NoError(t, receiveExecResult(t, result))
+		})
+	}
+}
+
+func writeExecWrapper(t *testing.T, body string) string {
+	path := filepath.Join(t.TempDir(), "wrapper")
+	require.NoError(t, os.WriteFile(path, []byte("#!/bin/sh\n"+body), 0o600))
+	require.NoError(t, os.Chmod(path, 0o700)) // #nosec G302 -- This fixture must be executable.
+	return path
+}
+
+func collectExecWrapperResult(t *testing.T, stream *execTestStream) (string, int32) {
+	require.NotNil(t, receiveExecResponse(t, stream).GetStarted())
+	var output strings.Builder
+	for {
+		response := receiveExecResponse(t, stream)
+		switch value := response.GetType().(type) {
+		case *v1.ExecResponse_StandardOutput:
+			output.Write(value.StandardOutput.GetData())
+		case *v1.ExecResponse_StandardError:
+			output.Write(value.StandardError.GetData())
+		case *v1.ExecResponse_Exit_:
+			return output.String(), value.Exit.GetCode()
+		default:
+			t.Fatalf("unexpected exec response %T", value)
+		}
+	}
+}

internal/rpc/rpc.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"net"
 	"os"
+	"slices"
 
 	"github.com/cirruslabs/tart-guest-agent/pkg/v1"
 	"github.com/puzpuzpuz/xsync/v4"
@@ -13,16 +14,22 @@ import (
 type RPC struct {
 	v1.UnimplementedAgentServer
 
-	grpcServer *grpc.Server
-	listener   net.Listener
-	execs      *xsync.Map[string, *os.Process]
+	grpcServer  *grpc.Server
+	listener    net.Listener
+	execs       *xsync.Map[string, *os.Process]
+	execWrapper []string
 }
 
-func New(listener net.Listener) (*RPC, error) {
+func New(listener net.Listener, execWrapper ...string) (*RPC, error) {
+	if err := ValidateExecWrapper(execWrapper); err != nil {
+		return nil, err
+	}
+
 	rpc := &RPC{
-		grpcServer: grpc.NewServer(),
-		listener:   listener,
-		execs:      xsync.NewMap[string, *os.Process](),
+		grpcServer:  grpc.NewServer(),
+		listener:    listener,
+		execs:       xsync.NewMap[string, *os.Process](),
+		execWrapper: slices.Clone(execWrapper),
 	}
 
 	v1.RegisterAgentServer(rpc.grpcServer, rpc)