From e82cf652df97ff979d09b569315499cf451ddcba Mon Sep 17 00:00:00 2001 From: JY Tan Date: Sun, 22 Mar 2026 17:12:41 -0700 Subject: [PATCH 1/4] Commit --- cmd/fence/main.go | 63 ++++++++++------ cmd/fence/main_test.go | 26 +++++++ docs/configuration.md | 5 +- docs/linux-bwrap-mount-sequence.md | 5 +- docs/schema/fence.schema.json | 6 ++ internal/config/config.go | 17 +++-- internal/config/config_file.go | 18 +++-- internal/config/config_file_test.go | 5 ++ internal/config/config_test.go | 17 +++++ internal/sandbox/integration_linux_test.go | 76 +++++++++++++++++++ internal/sandbox/linux.go | 60 ++++++++++++++- internal/sandbox/linux_seccomp.go | 85 ++++++++++++++++++---- internal/sandbox/linux_seccomp_test.go | 55 ++++++++++++++ internal/sandbox/linux_test.go | 51 +++++++++++++ 14 files changed, 428 insertions(+), 61 deletions(-) create mode 100644 internal/sandbox/linux_seccomp_test.go diff --git a/cmd/fence/main.go b/cmd/fence/main.go index cd9e869..95de22b 100644 --- a/cmd/fence/main.go +++ b/cmd/fence/main.go @@ -32,18 +32,19 @@ var ( ) var ( - debug bool - monitor bool - settingsPath string - templateName string - listTemplates bool - cmdString string - exposePorts []string - shellMode string - shellLogin bool - exitCode int - showVersion bool - linuxFeatures bool + debug bool + monitor bool + settingsPath string + templateName string + listTemplates bool + cmdString string + exposePorts []string + shellMode string + shellLogin bool + forceNewSession bool + exitCode int + showVersion bool + linuxFeatures bool ) func main() { @@ -106,6 +107,7 @@ Configuration file format: rootCmd.Flags().StringArrayVarP(&exposePorts, "port", "p", nil, "Expose port for inbound connections (can be used multiple times)") rootCmd.Flags().StringVar(&shellMode, "shell", sandbox.ShellModeDefault, "Shell mode for command execution: default (bash) or user ($SHELL)") rootCmd.Flags().BoolVar(&shellLogin, "shell-login", false, "Run shell as login shell (-lc). Use with --shell user for shell init compatibility") + rootCmd.Flags().BoolVar(&forceNewSession, "force-new-session", false, "Linux only: force bubblewrap --new-session even for interactive PTY sessions") rootCmd.Flags().BoolVarP(&showVersion, "version", "v", false, "Show version information") rootCmd.Flags().BoolVar(&linuxFeatures, "linux-features", false, "Show available Linux security features and exit") @@ -215,6 +217,11 @@ func runCommand(cmd *cobra.Command, args []string) error { } } + if cmd.Flags().Changed("force-new-session") { + value := forceNewSession + cfg.ForceNewSession = &value + } + manager := sandbox.NewManager(cfg, debug, monitor) manager.SetExposedPorts(ports) manager.SetShellOptions(shellMode, shellLogin) @@ -255,11 +262,10 @@ func runCommand(cmd *cobra.Command, args []string) error { execCmd := exec.Command("sh", "-c", sandboxedCommand) //nolint:gosec // sandboxedCommand is constructed from user input - intentional execCmd.Env = hardenedEnv - // On Linux, bubblewrap runs with --new-session. That breaks the normal TTY - // SIGWINCH delivery behavior for interactive TUIs unless we relay it. - // - // PTY relay is only enabled for interactive sessions to avoid surprising - // behavior changes (e.g., piping output through fence). + // On Linux, PTY relay is only enabled for interactive sessions to avoid + // surprising behavior changes (e.g., piping output through fence). When PTY + // relay is active, the Linux wrapper can keep bwrap in the current session + // so the inner shell retains normal job control. usePTY := cfg != nil && cfg.AllowPty && platform.Detect() == platform.Linux && @@ -273,12 +279,7 @@ func runCommand(cmd *cobra.Command, args []string) error { // "cannot set terminal process group: Operation not permitted" // "no job control in this shell" isTTY := term.IsTerminal(int(os.Stdin.Fd())) - if isTTY { - execCmd.SysProcAttr = &syscall.SysProcAttr{ - Setpgid: true, - Pgid: 0, // child gets its own process group (pgid = child pid) - } - } + configureHostTTYChildProcessGroup(execCmd, isTTY, usePTY) if !usePTY { execCmd.Stdin = os.Stdin @@ -297,7 +298,7 @@ func runCommand(cmd *cobra.Command, args []string) error { // call tcsetpgrp. We ignore SIGTTOU so we don't get stopped when we // later reclaim the foreground (at that point we'll be in the background // process group). - if isTTY && execCmd.Process != nil { + if shouldManageHostTTYForeground(isTTY, usePTY) && execCmd.Process != nil { stdinFd := int(os.Stdin.Fd()) savedFgPgrp, err := unix.IoctlGetInt(stdinFd, unix.TIOCGPGRP) @@ -364,6 +365,20 @@ func startCommand(execCmd *exec.Cmd, usePTY bool) (func(), error) { return startCommandWithSignalProxy(execCmd) } +func configureHostTTYChildProcessGroup(execCmd *exec.Cmd, isTTY bool, usePTY bool) { + if !shouldManageHostTTYForeground(isTTY, usePTY) { + return + } + execCmd.SysProcAttr = &syscall.SysProcAttr{ + Setpgid: true, + Pgid: 0, // child gets its own process group (pgid = child pid) + } +} + +func shouldManageHostTTYForeground(isTTY bool, usePTY bool) bool { + return isTTY && !usePTY +} + func startCommandWithSignalProxy(execCmd *exec.Cmd) (func(), error) { sigChan := make(chan os.Signal, 1) signal.Notify(sigChan, syscall.SIGINT, syscall.SIGTERM, syscall.SIGWINCH) diff --git a/cmd/fence/main_test.go b/cmd/fence/main_test.go index 31b42d5..357d834 100644 --- a/cmd/fence/main_test.go +++ b/cmd/fence/main_test.go @@ -185,3 +185,29 @@ func TestStartCommandWithSignalProxy_CleanupIsIdempotent(t *testing.T) { cleanup() cleanup() } + +func TestConfigureHostTTYChildProcessGroup_DirectTTY(t *testing.T) { + execCmd := exec.Command("sh", "-c", "exit 0") + + configureHostTTYChildProcessGroup(execCmd, true, false) + + if execCmd.SysProcAttr == nil { + t.Fatal("expected SysProcAttr to be configured for direct TTY sessions") + } + if !execCmd.SysProcAttr.Setpgid { + t.Fatal("expected Setpgid to be enabled for direct TTY sessions") + } + if execCmd.SysProcAttr.Pgid != 0 { + t.Fatalf("expected Pgid=0, got %d", execCmd.SysProcAttr.Pgid) + } +} + +func TestConfigureHostTTYChildProcessGroup_PTYRelay(t *testing.T) { + execCmd := exec.Command("sh", "-c", "exit 0") + + configureHostTTYChildProcessGroup(execCmd, true, true) + + if execCmd.SysProcAttr != nil { + t.Fatal("expected PTY relay sessions to leave SysProcAttr unset") + } +} diff --git a/docs/configuration.md b/docs/configuration.md index 42b1a92..d697abf 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -398,12 +398,15 @@ SSH host patterns support wildcards anywhere: | Field | Description | |-------|-------------| -| `allowPty` | Enable interactive PTY behavior. On macOS this allows PTY access in sandbox policy; on Linux this enables a PTY relay mode for interactive TUIs/editors while keeping `bwrap --new-session` enabled. | +| `allowPty` | Enable interactive PTY behavior. On macOS this allows PTY access in sandbox policy; on Linux this enables a PTY relay mode for interactive TUIs/editors. | +| `forceNewSession` | Linux only. Force `bwrap --new-session` even for interactive PTY sessions. Leave unset to use Fence's default Linux PTY session policy. | ### `allowPty` notes (Linux) - Use `allowPty: true` for interactive terminal apps (TUIs/editors) that need proper resize redraw behavior. - PTY relay is only used when stdin/stdout are both terminals (non-interactive pipes keep the normal stdio behavior). +- By default, Linux interactive PTY sessions skip `bwrap --new-session` so shells keep normal job control. +- If you need the stricter Bubblewrap session split, set `forceNewSession: true` or pass `--force-new-session`. - Resize handling relays `SIGWINCH` to the PTY foreground process group so terminal apps can redraw after window size changes. ## Importing from Claude Code diff --git a/docs/linux-bwrap-mount-sequence.md b/docs/linux-bwrap-mount-sequence.md index dee6bf9..2297a22 100644 --- a/docs/linux-bwrap-mount-sequence.md +++ b/docs/linux-bwrap-mount-sequence.md @@ -50,17 +50,16 @@ The mount sequence is easiest to understand as a set of phases. ### 1. Base `bwrap` Flags -Fence always starts with: +Fence starts with: ```text bwrap ---new-session --die-with-parent ``` These are not mounts, but they shape the rest of the runtime: -- `--new-session` isolates the sandboxed process tree into a new session +- `--new-session` is added in the normal Linux path, and also in interactive PTY sessions when `forceNewSession` is enabled - `--die-with-parent` ensures the sandbox dies when Fence dies ### 2. Namespace Isolation diff --git a/docs/schema/fence.schema.json b/docs/schema/fence.schema.json index 745c892..73a8f39 100644 --- a/docs/schema/fence.schema.json +++ b/docs/schema/fence.schema.json @@ -106,6 +106,12 @@ }, "type": "object" }, + "forceNewSession": { + "type": [ + "boolean", + "null" + ] + }, "network": { "additionalProperties": false, "properties": { diff --git a/internal/config/config.go b/internal/config/config.go index f73f2bc..a2b3677 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -16,13 +16,14 @@ import ( // Config is the main configuration for fence. type Config struct { - Extends string `json:"extends,omitempty"` - Network NetworkConfig `json:"network"` - Filesystem FilesystemConfig `json:"filesystem"` - Devices DevicesConfig `json:"devices,omitempty"` - Command CommandConfig `json:"command"` - SSH SSHConfig `json:"ssh"` - AllowPty bool `json:"allowPty,omitempty"` + Extends string `json:"extends,omitempty"` + Network NetworkConfig `json:"network"` + Filesystem FilesystemConfig `json:"filesystem"` + Devices DevicesConfig `json:"devices,omitempty"` + Command CommandConfig `json:"command"` + SSH SSHConfig `json:"ssh"` + AllowPty bool `json:"allowPty,omitempty"` + ForceNewSession *bool `json:"forceNewSession,omitempty"` } // NetworkConfig defines network restrictions. @@ -520,6 +521,8 @@ func Merge(base, override *Config) *Config { result := &Config{ // AllowPty: true if either config enables it AllowPty: base.AllowPty || override.AllowPty, + // Pointer field: override wins if set, otherwise base + ForceNewSession: mergeOptionalBool(base.ForceNewSession, override.ForceNewSession), Network: NetworkConfig{ // Append slices (base first, then override additions) diff --git a/internal/config/config_file.go b/internal/config/config_file.go index 5597ddf..dadcd1d 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -57,20 +57,22 @@ type cleanSSHConfig struct { // cleanConfig is used for JSON output with fields in desired order and omitempty. type cleanConfig struct { - Extends string `json:"extends,omitempty"` - AllowPty bool `json:"allowPty,omitempty"` - Network *cleanNetworkConfig `json:"network,omitempty"` - Filesystem *cleanFilesystemConfig `json:"filesystem,omitempty"` - Command *cleanCommandConfig `json:"command,omitempty"` - SSH *cleanSSHConfig `json:"ssh,omitempty"` + Extends string `json:"extends,omitempty"` + AllowPty bool `json:"allowPty,omitempty"` + ForceNewSession *bool `json:"forceNewSession,omitempty"` + Network *cleanNetworkConfig `json:"network,omitempty"` + Filesystem *cleanFilesystemConfig `json:"filesystem,omitempty"` + Command *cleanCommandConfig `json:"command,omitempty"` + SSH *cleanSSHConfig `json:"ssh,omitempty"` } // MarshalConfigJSON marshals a fence config to clean JSON, omitting empty arrays // and with fields in a logical order (extends first). func MarshalConfigJSON(cfg *Config) ([]byte, error) { clean := cleanConfig{ - Extends: cfg.Extends, - AllowPty: cfg.AllowPty, + Extends: cfg.Extends, + AllowPty: cfg.AllowPty, + ForceNewSession: cfg.ForceNewSession, } // Network config - only include if non-empty diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index 09c907a..17938ec 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -56,7 +56,10 @@ func TestWriteConfigFile(t *testing.T) { func TestMarshalConfigJSON_IncludesExtendedFilesystemAndSSH(t *testing.T) { wslInterop := false + forceNewSession := true cfg := &Config{} + cfg.AllowPty = true + cfg.ForceNewSession = &forceNewSession cfg.Filesystem.DefaultDenyRead = true cfg.Filesystem.WSLInterop = &wslInterop cfg.Filesystem.AllowRead = []string{"/workspace"} @@ -69,6 +72,8 @@ func TestMarshalConfigJSON_IncludesExtendedFilesystemAndSSH(t *testing.T) { require.NoError(t, err) output := string(data) + assert.Contains(t, output, `"allowPty": true`) + assert.Contains(t, output, `"forceNewSession": true`) assert.Contains(t, output, `"defaultDenyRead": true`) assert.Contains(t, output, `"wslInterop": false`) assert.Contains(t, output, `"allowRead": [`) diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 3cb45e4..f35b4bb 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -757,6 +757,23 @@ func TestMerge(t *testing.T) { } }) + t.Run("merge forceNewSession override wins", func(t *testing.T) { + base := &Config{ + ForceNewSession: boolPtr(true), + } + override := &Config{ + ForceNewSession: boolPtr(false), + } + result := Merge(base, override) + + if result.ForceNewSession == nil { + t.Fatal("expected ForceNewSession to be non-nil") + } + if *result.ForceNewSession { + t.Error("expected ForceNewSession to be false (override wins)") + } + }) + t.Run("merge command config", func(t *testing.T) { base := &Config{ Command: CommandConfig{ diff --git a/internal/sandbox/integration_linux_test.go b/internal/sandbox/integration_linux_test.go index 88d6cb7..908d37b 100644 --- a/internal/sandbox/integration_linux_test.go +++ b/internal/sandbox/integration_linux_test.go @@ -15,6 +15,8 @@ import ( "syscall" "testing" "time" + + "github.com/Use-Tusk/fence/internal/config" ) // ============================================================================ @@ -54,6 +56,35 @@ func assertNetworkBlocked(t *testing.T, result *SandboxTestResult) { result.Stdout, result.Stderr) } +func runUnderLinuxSandboxDirect(t *testing.T, cfg *config.Config, command string, workDir string) *SandboxTestResult { + t.Helper() + skipIfAlreadySandboxed(t) + + if workDir == "" { + var err error + workDir, err = os.Getwd() + if err != nil { + return &SandboxTestResult{Error: err} + } + } + + wrappedCmd, err := WrapCommandLinuxWithOptions(cfg, command, nil, nil, LinuxSandboxOptions{ + UseLandlock: false, + UseSeccomp: false, + UseEBPF: false, + ShellMode: ShellModeDefault, + }) + if err != nil { + return &SandboxTestResult{ + ExitCode: 1, + Stderr: err.Error(), + Error: err, + } + } + + return executeShellCommand(t, wrappedCmd, workDir) +} + // TestLinux_LandlockBlocksWriteOutsideWorkspace verifies that Landlock prevents // writes to locations outside the allowed workspace. func TestLinux_LandlockBlocksWriteOutsideWorkspace(t *testing.T) { @@ -675,6 +706,51 @@ func TestLinux_PythonGetpwuidWorks(t *testing.T) { } } +func TestLinux_XDGRuntimeDirFallsBackWhenInheritedPathIsUnavailable(t *testing.T) { + skipIfAlreadySandboxed(t) + + workspace := createTempWorkspace(t) + cfg := testConfigWithWorkspace(workspace) + + t.Setenv("XDG_RUNTIME_DIR", "/run/user/fence-test-missing") + t.Setenv("TMPDIR", "/run/user/fence-test-missing/tmp") + + result := runUnderLinuxSandboxDirect(t, cfg, `printf 'XDG=%s\nTMP=%s\n' "$XDG_RUNTIME_DIR" "$TMPDIR" && test -n "$XDG_RUNTIME_DIR" && test -d "$XDG_RUNTIME_DIR" && test -w "$XDG_RUNTIME_DIR" && touch "$XDG_RUNTIME_DIR/fence-runtime-probe" && stat -c 'MODE=%a' "$XDG_RUNTIME_DIR" && test -d "$TMPDIR" && test -w "$TMPDIR" && touch "$TMPDIR/fence-tmp-probe" && echo OK`, workspace) + + assertAllowed(t, result) + assertContains(t, result.Stdout, "XDG=/tmp/fence-runtime-") + assertContains(t, result.Stdout, "TMP=/tmp") + assertContains(t, result.Stdout, "MODE=700") + assertContains(t, result.Stdout, "OK") +} + +func TestLinux_XDGRuntimeDirPreservedWhenWritable(t *testing.T) { + skipIfAlreadySandboxed(t) + + workspace := createTempWorkspace(t) + runtimeDir := filepath.Join(workspace, "runtime") + if err := os.MkdirAll(runtimeDir, 0o700); err != nil { + t.Fatalf("failed to create runtime dir: %v", err) + } + tmpDir := filepath.Join(workspace, "tmp") + if err := os.MkdirAll(tmpDir, 0o700); err != nil { + t.Fatalf("failed to create tmp dir: %v", err) + } + + t.Setenv("XDG_RUNTIME_DIR", runtimeDir) + t.Setenv("TMPDIR", tmpDir) + + cfg := testConfigWithWorkspace(workspace) + result := runUnderLinuxSandboxDirect(t, cfg, `printf 'XDG=%s\nTMP=%s\n' "$XDG_RUNTIME_DIR" "$TMPDIR" && touch "$XDG_RUNTIME_DIR/fence-runtime-probe" && touch "$TMPDIR/fence-tmp-probe" && echo OK`, workspace) + + assertAllowed(t, result) + assertContains(t, result.Stdout, "XDG="+runtimeDir) + assertContains(t, result.Stdout, "TMP="+tmpDir) + assertContains(t, result.Stdout, "OK") + assertFileExists(t, filepath.Join(runtimeDir, "fence-runtime-probe")) + assertFileExists(t, filepath.Join(tmpDir, "fence-tmp-probe")) +} + // ============================================================================ // Security Edge Case Tests // ============================================================================ diff --git a/internal/sandbox/linux.go b/internal/sandbox/linux.go index 4839c04..e13a9da 100644 --- a/internal/sandbox/linux.go +++ b/internal/sandbox/linux.go @@ -16,6 +16,7 @@ import ( "time" "github.com/Use-Tusk/fence/internal/config" + "golang.org/x/term" ) // LinuxBridge holds the socat bridge processes for Linux sandboxing (outbound). @@ -415,6 +416,41 @@ func WrapCommandLinuxWithShell(cfg *config.Config, command string, bridge *Linux }) } +func linuxRuntimeEnvScript() string { + return ` +fence_dir_is_usable() { + dir=$1 + [ -n "$dir" ] && [ -d "$dir" ] && [ -w "$dir" ] +} + +fence_prepare_private_runtime_dir() { + dir=$1 + mkdir -p "$dir" 2>/dev/null || return 1 + chmod 700 "$dir" 2>/dev/null || true + fence_dir_is_usable "$dir" +} + +if ! fence_dir_is_usable "${TMPDIR:-}"; then + export TMPDIR=/tmp +fi + +fence_runtime_dir=${XDG_RUNTIME_DIR:-} +if ! fence_dir_is_usable "$fence_runtime_dir"; then + fence_runtime_dir="/tmp/fence-runtime-$(id -u)-$$" + if ! fence_prepare_private_runtime_dir "$fence_runtime_dir"; then + fence_runtime_dir= + fi +fi + +if [ -n "$fence_runtime_dir" ]; then + export XDG_RUNTIME_DIR="$fence_runtime_dir" +else + unset XDG_RUNTIME_DIR +fi + +` +} + func buildLinuxBootstrapScript( cfg *config.Config, command string, @@ -475,6 +511,7 @@ cleanup() { } trap cleanup EXIT `) + script.WriteString(linuxRuntimeEnvScript()) if bridge != nil { _, _ = fmt.Fprintf(&script, ` @@ -573,6 +610,17 @@ func bootstrapSocketPaths(reverseBridge *ReverseBridge) []string { return reverseBridge.SocketPaths } +func useLinuxInteractivePTYSession(cfg *config.Config, stdinIsTTY bool, stdoutIsTTY bool) bool { + return cfg != nil && cfg.AllowPty && stdinIsTTY && stdoutIsTTY +} + +func effectiveLinuxForceNewSession(cfg *config.Config, stdinIsTTY bool, stdoutIsTTY bool) bool { + if cfg != nil && cfg.ForceNewSession != nil { + return *cfg.ForceNewSession + } + return !useLinuxInteractivePTYSession(cfg, stdinIsTTY, stdoutIsTTY) +} + func effectiveLinuxDeviceMode(cfg *config.Config, bwrapPath string) config.DeviceMode { requested := config.DeviceModeAuto if cfg != nil && cfg.Devices.Mode != "" { @@ -662,6 +710,10 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, bridge *Lin fmt.Fprintf(os.Stderr, "[fence:linux] Available features: %s\n", features.Summary()) } + stdinIsTTY := term.IsTerminal(int(os.Stdin.Fd())) + stdoutIsTTY := term.IsTerminal(int(os.Stdout.Fd())) + forceNewSession := effectiveLinuxForceNewSession(cfg, stdinIsTTY, stdoutIsTTY) + deviceMode := effectiveLinuxDeviceMode(cfg, bwrapPath) if opts.Debug { fmt.Fprintf(os.Stderr, "[fence:linux] Device mode: %s\n", deviceMode) @@ -679,9 +731,13 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, bridge *Lin // Build bwrap args with filesystem restrictions bwrapArgs := []string{ "bwrap", - "--new-session", "--die-with-parent", } + if forceNewSession { + bwrapArgs = append(bwrapArgs, "--new-session") + } else if opts.Debug { + fmt.Fprintf(os.Stderr, "[fence:linux] Interactive PTY session detected - skipping --new-session and relying on seccomp TIOCSTI filtering\n") + } // Only use --unshare-net if: // 1. The environment supports it (has CAP_NET_ADMIN) @@ -707,7 +763,7 @@ func WrapCommandLinuxWithOptions(cfg *config.Config, command string, bridge *Lin } else { seccompFilterPath = filterPath if opts.Debug { - fmt.Fprintf(os.Stderr, "[fence:linux] Seccomp filter enabled (blocking %d dangerous syscalls)\n", len(DangerousSyscalls)) + fmt.Fprintf(os.Stderr, "[fence:linux] Seccomp filter enabled (blocking dangerous syscalls and ioctl(TIOCSTI))\n") } // Add seccomp filter via fd 3 (will be set up via shell redirection) bwrapArgs = append(bwrapArgs, "--seccomp", "3") diff --git a/internal/sandbox/linux_seccomp.go b/internal/sandbox/linux_seccomp.go index ce28016..6dbd531 100644 --- a/internal/sandbox/linux_seccomp.go +++ b/internal/sandbox/linux_seccomp.go @@ -15,6 +15,12 @@ type SeccompFilter struct { debug bool } +const ( + seccompDataSyscallOffset = 0 + seccompDataArgsOffset = 16 + seccompArgSize = 8 +) + // NewSeccompFilter creates a new seccomp filter generator. func NewSeccompFilter(debug bool) *SeccompFilter { return &SeccompFilter{debug: debug} @@ -94,6 +100,28 @@ func (s *SeccompFilter) writeBPFProgram(path string) error { // 2. For each dangerous syscall: if match, return ERRNO(EPERM) or LOG+ERRNO // 3. Default: allow + program, err := s.buildBPFProgram() + if err != nil { + return err + } + + // Write the program to file + f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) //nolint:gosec // path is controlled + if err != nil { + return err + } + defer func() { _ = f.Close() }() + + for _, inst := range program { + if err := inst.writeTo(f); err != nil { + return err + } + } + + return nil +} + +func (s *SeccompFilter) buildBPFProgram() ([]bpfInstruction, error) { // Get syscall numbers for the current architecture syscallNums := make(map[string]int) for _, name := range DangerousSyscalls { @@ -104,7 +132,7 @@ func (s *SeccompFilter) writeBPFProgram(path string) error { if len(syscallNums) == 0 { // No syscalls to block (unknown architecture?) - return fmt.Errorf("no syscall numbers found for dangerous syscalls") + return nil, fmt.Errorf("no syscall numbers found for dangerous syscalls") } // Build BPF program @@ -114,15 +142,47 @@ func (s *SeccompFilter) writeBPFProgram(path string) error { // BPF_LD | BPF_W | BPF_ABS: load word from absolute offset program = append(program, bpfInstruction{ code: BPF_LD | BPF_W | BPF_ABS, - k: 0, // offsetof(struct seccomp_data, nr) + k: seccompDataSyscallOffset, }) - // For each dangerous syscall, add a comparison and block // Note: SECCOMP_RET_ERRNO returns -1 with errno in the low 16 bits // SECCOMP_RET_LOG means "log and allow" which is NOT what we want // We use SECCOMP_RET_ERRNO to block with EPERM action := SECCOMP_RET_ERRNO | (unix.EPERM & 0xFFFF) + // Allow interactive PTY sessions without bwrap --new-session, but block + // TIOCSTI specifically so sandboxed processes cannot inject keystrokes into + // the caller's terminal. Only the low 32 bits are relevant for ioctl cmds. + if ioctlNum, ok := getSyscallNumber("ioctl"); ok { + program = append(program, + bpfInstruction{ + code: BPF_JMP | BPF_JEQ | BPF_K, + jt: 0, + jf: 4, + k: uint32(ioctlNum), //nolint:gosec // syscall numbers fit in uint32 + }, + bpfInstruction{ + code: BPF_LD | BPF_W | BPF_ABS, + k: seccompArgLow32Offset(1), + }, + bpfInstruction{ + code: BPF_JMP | BPF_JEQ | BPF_K, + jt: 0, + jf: 1, + k: uint32(unix.TIOCSTI), + }, + bpfInstruction{ + code: BPF_RET | BPF_K, + k: uint32(action), + }, + bpfInstruction{ + code: BPF_RET | BPF_K, + k: SECCOMP_RET_ALLOW, + }, + ) + } + + // For each dangerous syscall, add a comparison and block for _, name := range DangerousSyscalls { num, ok := syscallNums[name] if !ok { @@ -150,20 +210,11 @@ func (s *SeccompFilter) writeBPFProgram(path string) error { k: SECCOMP_RET_ALLOW, }) - // Write the program to file - f, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, 0o600) //nolint:gosec // path is controlled - if err != nil { - return err - } - defer func() { _ = f.Close() }() - - for _, inst := range program { - if err := inst.writeTo(f); err != nil { - return err - } - } + return program, nil +} - return nil +func seccompArgLow32Offset(argIndex int) uint32 { + return seccompDataArgsOffset + uint32(argIndex*seccompArgSize) } // CleanupFilter removes a generated filter file. @@ -240,6 +291,7 @@ func getSyscallNumber(name string) (int, bool) { "ptrace": 117, "process_vm_readv": 270, "process_vm_writev": 271, + "ioctl": 29, "keyctl": 219, "add_key": 217, "request_key": 218, @@ -270,6 +322,7 @@ func getSyscallNumber(name string) (int, bool) { "ptrace": 101, "process_vm_readv": 310, "process_vm_writev": 311, + "ioctl": 16, "keyctl": 250, "add_key": 248, "request_key": 249, diff --git a/internal/sandbox/linux_seccomp_test.go b/internal/sandbox/linux_seccomp_test.go new file mode 100644 index 0000000..34a94be --- /dev/null +++ b/internal/sandbox/linux_seccomp_test.go @@ -0,0 +1,55 @@ +//go:build linux + +package sandbox + +import ( + "testing" + + "golang.org/x/sys/unix" +) + +func TestSeccompArgLow32Offset(t *testing.T) { + if got := seccompArgLow32Offset(1); got != 24 { + t.Fatalf("seccompArgLow32Offset(1) = %d, want 24", got) + } +} + +func TestBuildBPFProgram_BlocksTIOCSTI(t *testing.T) { + filter := NewSeccompFilter(false) + program, err := filter.buildBPFProgram() + if err != nil { + t.Fatalf("buildBPFProgram() error = %v", err) + } + + ioctlNum, ok := getSyscallNumber("ioctl") + if !ok { + t.Skip("ioctl syscall number unavailable on this architecture") + } + + wantAction := uint32(SECCOMP_RET_ERRNO | (unix.EPERM & 0xFFFF)) + found := false + + for i := 0; i+4 < len(program); i++ { + if program[i].code != BPF_JMP|BPF_JEQ|BPF_K || program[i].k != uint32(ioctlNum) { + continue + } + if program[i+1].code != BPF_LD|BPF_W|BPF_ABS || program[i+1].k != seccompArgLow32Offset(1) { + continue + } + if program[i+2].code != BPF_JMP|BPF_JEQ|BPF_K || program[i+2].k != uint32(unix.TIOCSTI) { + continue + } + if program[i+3].code != BPF_RET|BPF_K || program[i+3].k != wantAction { + continue + } + if program[i+4].code != BPF_RET|BPF_K || program[i+4].k != SECCOMP_RET_ALLOW { + continue + } + found = true + break + } + + if !found { + t.Fatal("expected seccomp program to contain an ioctl(TIOCSTI) blocking rule") + } +} diff --git a/internal/sandbox/linux_test.go b/internal/sandbox/linux_test.go index 90011bf..8bb98a4 100644 --- a/internal/sandbox/linux_test.go +++ b/internal/sandbox/linux_test.go @@ -185,6 +185,32 @@ func TestResolveLinuxDeviceMode(t *testing.T) { } } +func TestEffectiveLinuxForceNewSession(t *testing.T) { + t.Run("defaults to strict outside interactive pty", func(t *testing.T) { + if !effectiveLinuxForceNewSession(&config.Config{}, false, false) { + t.Fatal("expected new-session to remain enabled outside interactive PTY sessions") + } + }) + + t.Run("defaults off for interactive pty sessions", func(t *testing.T) { + cfg := &config.Config{AllowPty: true} + if effectiveLinuxForceNewSession(cfg, true, true) { + t.Fatal("expected interactive PTY sessions to skip new-session by default") + } + }) + + t.Run("explicit override wins", func(t *testing.T) { + value := true + cfg := &config.Config{ + AllowPty: true, + ForceNewSession: &value, + } + if !effectiveLinuxForceNewSession(cfg, true, true) { + t.Fatal("expected explicit forceNewSession override to win") + } + }) +} + func TestWrapCommandLinuxWithOptions_UsesMinimalDevMode(t *testing.T) { if _, err := exec.LookPath("bwrap"); err != nil { t.Skip("bwrap not available") @@ -234,6 +260,31 @@ func TestWrapCommandLinuxWithOptions_UsesMinimalDevMode(t *testing.T) { } } +func TestWrapCommandLinuxWithOptions_RespectsForceNewSessionOverride(t *testing.T) { + if _, err := exec.LookPath("bwrap"); err != nil { + t.Skip("bwrap not available") + } + + forceNewSession := false + cfg := &config.Config{ + ForceNewSession: &forceNewSession, + } + + cmd, err := WrapCommandLinuxWithOptions(cfg, "echo ok", nil, nil, LinuxSandboxOptions{ + UseLandlock: false, + UseSeccomp: false, + UseEBPF: false, + ShellMode: ShellModeDefault, + }) + if err != nil { + t.Fatalf("WrapCommandLinuxWithOptions failed: %v", err) + } + + if strings.Contains(cmd, ShellQuote([]string{"--new-session"})) { + t.Fatalf("did not expect --new-session when explicitly disabled: %s", cmd) + } +} + func TestWrapCommandLinuxWithOptions_UsesHostDevMode(t *testing.T) { if _, err := exec.LookPath("bwrap"); err != nil { t.Skip("bwrap not available") From 7bc9be8a1aa03c5154a5ba1f10c87f7f50cf0ac7 Mon Sep 17 00:00:00 2001 From: JY Tan Date: Sun, 22 Mar 2026 17:57:45 -0700 Subject: [PATCH 2/4] Fix --- cmd/fence/main.go | 16 ++++++++--- cmd/fence/main_test.go | 17 ++++++++++++ internal/sandbox/integration_linux_test.go | 31 ++++++++++++++++++++++ internal/sandbox/linux.go | 30 ++++++++++++++++----- internal/sandbox/linux_seccomp.go | 28 +++++++++---------- internal/sandbox/linux_seccomp_test.go | 2 +- 6 files changed, 99 insertions(+), 25 deletions(-) diff --git a/cmd/fence/main.go b/cmd/fence/main.go index 95de22b..d700b3a 100644 --- a/cmd/fence/main.go +++ b/cmd/fence/main.go @@ -217,10 +217,7 @@ func runCommand(cmd *cobra.Command, args []string) error { } } - if cmd.Flags().Changed("force-new-session") { - value := forceNewSession - cfg.ForceNewSession = &value - } + cfg = applyCLIConfigOverrides(cmd, cfg, forceNewSession) manager := sandbox.NewManager(cfg, debug, monitor) manager.SetExposedPorts(ports) @@ -358,6 +355,17 @@ func runCommand(cmd *cobra.Command, args []string) error { return nil } +func applyCLIConfigOverrides(cmd *cobra.Command, cfg *config.Config, forceNewSessionValue bool) *config.Config { + if cfg == nil { + cfg = config.Default() + } + if cmd.Flags().Changed("force-new-session") { + value := forceNewSessionValue + cfg.ForceNewSession = &value + } + return cfg +} + func startCommand(execCmd *exec.Cmd, usePTY bool) (func(), error) { if usePTY { return startCommandWithPTY(execCmd) diff --git a/cmd/fence/main_test.go b/cmd/fence/main_test.go index 357d834..9e2ff70 100644 --- a/cmd/fence/main_test.go +++ b/cmd/fence/main_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/Use-Tusk/fence/internal/config" + "github.com/spf13/cobra" ) func TestBuildInitConfig_DefaultTemplate(t *testing.T) { @@ -211,3 +212,19 @@ func TestConfigureHostTTYChildProcessGroup_PTYRelay(t *testing.T) { t.Fatal("expected PTY relay sessions to leave SysProcAttr unset") } } + +func TestApplyCLIConfigOverrides_NilConfigWithForceNewSessionFlag(t *testing.T) { + cmd := &cobra.Command{Use: "test"} + cmd.Flags().Bool("force-new-session", false, "") + if err := cmd.Flags().Set("force-new-session", "true"); err != nil { + t.Fatalf("failed to set force-new-session flag: %v", err) + } + + cfg := applyCLIConfigOverrides(cmd, nil, true) + if cfg == nil { + t.Fatal("expected config to be initialized when nil") + } + if cfg.ForceNewSession == nil || !*cfg.ForceNewSession { + t.Fatal("expected ForceNewSession override to be applied") + } +} diff --git a/internal/sandbox/integration_linux_test.go b/internal/sandbox/integration_linux_test.go index 908d37b..345a74a 100644 --- a/internal/sandbox/integration_linux_test.go +++ b/internal/sandbox/integration_linux_test.go @@ -724,6 +724,37 @@ func TestLinux_XDGRuntimeDirFallsBackWhenInheritedPathIsUnavailable(t *testing.T assertContains(t, result.Stdout, "OK") } +func TestLinux_XDGRuntimeDirFallbackIsCleanedUpOnExit(t *testing.T) { + skipIfAlreadySandboxed(t) + + workspace := createTempWorkspace(t) + cfg := testConfigWithWorkspace(workspace) + cfg.Filesystem.AllowWrite = append(cfg.Filesystem.AllowWrite, "/tmp") + + t.Setenv("XDG_RUNTIME_DIR", "/run/user/fence-test-missing") + t.Setenv("TMPDIR", "/run/user/fence-test-missing/tmp") + + result := runUnderLinuxSandboxDirect(t, cfg, `printf 'XDG=%s\n' "$XDG_RUNTIME_DIR" && touch "$XDG_RUNTIME_DIR/fence-runtime-probe" && echo OK`, workspace) + + assertAllowed(t, result) + assertContains(t, result.Stdout, "XDG=/tmp/fence-runtime-") + assertContains(t, result.Stdout, "OK") + + var runtimeDir string + for _, line := range strings.Split(result.Stdout, "\n") { + if strings.HasPrefix(line, "XDG=") { + runtimeDir = strings.TrimPrefix(line, "XDG=") + break + } + } + if runtimeDir == "" { + t.Fatal("expected sandbox output to include XDG runtime dir") + } + if _, err := os.Stat(runtimeDir); !os.IsNotExist(err) { + t.Fatalf("expected fallback runtime dir %q to be cleaned up, stat err=%v", runtimeDir, err) + } +} + func TestLinux_XDGRuntimeDirPreservedWhenWritable(t *testing.T) { skipIfAlreadySandboxed(t) diff --git a/internal/sandbox/linux.go b/internal/sandbox/linux.go index e13a9da..2b1026e 100644 --- a/internal/sandbox/linux.go +++ b/internal/sandbox/linux.go @@ -418,16 +418,27 @@ func WrapCommandLinuxWithShell(cfg *config.Config, command string, bridge *Linux func linuxRuntimeEnvScript() string { return ` +fence_runtime_dir_cleanup= + fence_dir_is_usable() { dir=$1 - [ -n "$dir" ] && [ -d "$dir" ] && [ -w "$dir" ] + probe= + [ -n "$dir" ] && [ -d "$dir" ] || return 1 + probe="$dir/.fence-write-test-$$" + (umask 077 && : > "$probe") 2>/dev/null || return 1 + rm -f "$probe" 2>/dev/null || true + return 0 } fence_prepare_private_runtime_dir() { - dir=$1 - mkdir -p "$dir" 2>/dev/null || return 1 + dir= + dir=$(mktemp -d "/tmp/fence-runtime-$(id -u)-XXXXXX" 2>/dev/null) || return 1 chmod 700 "$dir" 2>/dev/null || true - fence_dir_is_usable "$dir" + if ! fence_dir_is_usable "$dir"; then + rm -rf -- "$dir" 2>/dev/null || true + return 1 + fi + printf '%s\n' "$dir" } if ! fence_dir_is_usable "${TMPDIR:-}"; then @@ -436,9 +447,11 @@ fi fence_runtime_dir=${XDG_RUNTIME_DIR:-} if ! fence_dir_is_usable "$fence_runtime_dir"; then - fence_runtime_dir="/tmp/fence-runtime-$(id -u)-$$" - if ! fence_prepare_private_runtime_dir "$fence_runtime_dir"; then + fence_runtime_dir=$(fence_prepare_private_runtime_dir 2>/dev/null || true) + if [ -z "$fence_runtime_dir" ]; then fence_runtime_dir= + else + fence_runtime_dir_cleanup="$fence_runtime_dir" fi fi @@ -508,6 +521,11 @@ fence_wait_for_helpers() { # Cleanup function cleanup() { jobs -p | xargs -r kill >>"$fence_bootstrap_log" 2>&1 || true + case "${fence_runtime_dir_cleanup:-}" in + /tmp/fence-runtime-*) + rm -rf -- "$fence_runtime_dir_cleanup" >>"$fence_bootstrap_log" 2>&1 || true + ;; + esac } trap cleanup EXIT `) diff --git a/internal/sandbox/linux_seccomp.go b/internal/sandbox/linux_seccomp.go index 6dbd531..1a28472 100644 --- a/internal/sandbox/linux_seccomp.go +++ b/internal/sandbox/linux_seccomp.go @@ -16,9 +16,9 @@ type SeccompFilter struct { } const ( - seccompDataSyscallOffset = 0 - seccompDataArgsOffset = 16 - seccompArgSize = 8 + seccompDataSyscallOffset uint32 = 0 + seccompDataArgsOffset uint32 = 16 + seccompArgSize uint32 = 8 ) // NewSeccompFilter creates a new seccomp filter generator. @@ -123,7 +123,7 @@ func (s *SeccompFilter) writeBPFProgram(path string) error { func (s *SeccompFilter) buildBPFProgram() ([]bpfInstruction, error) { // Get syscall numbers for the current architecture - syscallNums := make(map[string]int) + syscallNums := make(map[string]uint32) for _, name := range DangerousSyscalls { if num, ok := getSyscallNumber(name); ok { syscallNums[name] = num @@ -159,7 +159,7 @@ func (s *SeccompFilter) buildBPFProgram() ([]bpfInstruction, error) { code: BPF_JMP | BPF_JEQ | BPF_K, jt: 0, jf: 4, - k: uint32(ioctlNum), //nolint:gosec // syscall numbers fit in uint32 + k: ioctlNum, }, bpfInstruction{ code: BPF_LD | BPF_W | BPF_ABS, @@ -192,9 +192,9 @@ func (s *SeccompFilter) buildBPFProgram() ([]bpfInstruction, error) { // BPF_JMP | BPF_JEQ | BPF_K: if A == K, jump jt else jump jf program = append(program, bpfInstruction{ code: BPF_JMP | BPF_JEQ | BPF_K, - jt: 0, // if match, go to next instruction (block) - jf: 1, // if not match, skip the block instruction - k: uint32(num), //nolint:gosec // syscall numbers fit in uint32 + jt: 0, // if match, go to next instruction (block) + jf: 1, // if not match, skip the block instruction + k: num, }) // Return action (block with EPERM) @@ -213,8 +213,8 @@ func (s *SeccompFilter) buildBPFProgram() ([]bpfInstruction, error) { return program, nil } -func seccompArgLow32Offset(argIndex int) uint32 { - return seccompDataArgsOffset + uint32(argIndex*seccompArgSize) +func seccompArgLow32Offset(argIndex uint32) uint32 { + return seccompDataArgsOffset + argIndex*seccompArgSize } // CleanupFilter removes a generated filter file. @@ -266,7 +266,7 @@ func (i *bpfInstruction) writeTo(f *os.File) error { } // getSyscallNumber returns the syscall number for the current architecture. -func getSyscallNumber(name string) (int, bool) { +func getSyscallNumber(name string) (uint32, bool) { // Detect architecture using uname var utsname unix.Utsname if err := unix.Uname(&utsname); err != nil { @@ -283,11 +283,11 @@ func getSyscallNumber(name string) (int, bool) { } } - var syscallMap map[string]int + var syscallMap map[string]uint32 if machine == "aarch64" || machine == "arm64" { // ARM64 syscall numbers (from asm-generic/unistd.h) - syscallMap = map[string]int{ + syscallMap = map[string]uint32{ "ptrace": 117, "process_vm_readv": 270, "process_vm_writev": 271, @@ -318,7 +318,7 @@ func getSyscallNumber(name string) (int, bool) { } } else { // x86_64 syscall numbers - syscallMap = map[string]int{ + syscallMap = map[string]uint32{ "ptrace": 101, "process_vm_readv": 310, "process_vm_writev": 311, diff --git a/internal/sandbox/linux_seccomp_test.go b/internal/sandbox/linux_seccomp_test.go index 34a94be..e292b8a 100644 --- a/internal/sandbox/linux_seccomp_test.go +++ b/internal/sandbox/linux_seccomp_test.go @@ -30,7 +30,7 @@ func TestBuildBPFProgram_BlocksTIOCSTI(t *testing.T) { found := false for i := 0; i+4 < len(program); i++ { - if program[i].code != BPF_JMP|BPF_JEQ|BPF_K || program[i].k != uint32(ioctlNum) { + if program[i].code != BPF_JMP|BPF_JEQ|BPF_K || program[i].k != ioctlNum { continue } if program[i+1].code != BPF_LD|BPF_W|BPF_ABS || program[i+1].k != seccompArgLow32Offset(1) { From 76c4698187a44b3a213df6f8b2c2859552232cf4 Mon Sep 17 00:00:00 2001 From: JY Tan Date: Sun, 22 Mar 2026 18:14:11 -0700 Subject: [PATCH 3/4] Fix test --- internal/sandbox/integration_linux_test.go | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/internal/sandbox/integration_linux_test.go b/internal/sandbox/integration_linux_test.go index 345a74a..01f1a73 100644 --- a/internal/sandbox/integration_linux_test.go +++ b/internal/sandbox/integration_linux_test.go @@ -776,10 +776,20 @@ func TestLinux_XDGRuntimeDirPreservedWhenWritable(t *testing.T) { assertAllowed(t, result) assertContains(t, result.Stdout, "XDG="+runtimeDir) - assertContains(t, result.Stdout, "TMP="+tmpDir) assertContains(t, result.Stdout, "OK") assertFileExists(t, filepath.Join(runtimeDir, "fence-runtime-probe")) - assertFileExists(t, filepath.Join(tmpDir, "fence-tmp-probe")) + + // Some environments normalize inherited TMPDIR to the sandbox's /tmp even + // when the original path is otherwise writable. The behavior we require is + // that TMPDIR remains usable, not that it always keeps the exact host path. + switch { + case strings.Contains(result.Stdout, "TMP="+tmpDir): + assertFileExists(t, filepath.Join(tmpDir, "fence-tmp-probe")) + case strings.Contains(result.Stdout, "TMP=/tmp"): + // The in-sandbox touch above already proved TMPDIR is usable. + default: + t.Fatalf("expected sandbox TMPDIR to remain %q or fall back to /tmp, got %q", tmpDir, result.Stdout) + } } // ============================================================================ From d62116febba50301043bbd2e206b912615900d95 Mon Sep 17 00:00:00 2001 From: JY Tan Date: Sun, 22 Mar 2026 18:18:55 -0700 Subject: [PATCH 4/4] Apply suggestion --- internal/sandbox/integration_linux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/sandbox/integration_linux_test.go b/internal/sandbox/integration_linux_test.go index 01f1a73..b57417c 100644 --- a/internal/sandbox/integration_linux_test.go +++ b/internal/sandbox/integration_linux_test.go @@ -785,7 +785,7 @@ func TestLinux_XDGRuntimeDirPreservedWhenWritable(t *testing.T) { switch { case strings.Contains(result.Stdout, "TMP="+tmpDir): assertFileExists(t, filepath.Join(tmpDir, "fence-tmp-probe")) - case strings.Contains(result.Stdout, "TMP=/tmp"): + case strings.Contains(result.Stdout, "TMP=/tmp\n"): // The in-sandbox touch above already proved TMPDIR is usable. default: t.Fatalf("expected sandbox TMPDIR to remain %q or fall back to /tmp, got %q", tmpDir, result.Stdout)