diff --git a/guard/guard.go b/guard/guard.go index 03c0179..fc85608 100644 --- a/guard/guard.go +++ b/guard/guard.go @@ -227,6 +227,7 @@ func (g *packageManagerGuard) continueExecution(ctx context.Context, pc *package if err != nil { return fmt.Errorf("failed to apply sandbox: %w", err) } + defer result.Close() // Clean up sandbox resources // Only run the command if the sandbox didn't already execute it if result.ShouldRun() { diff --git a/internal/flows/proxy_flow.go b/internal/flows/proxy_flow.go index d80abe0..917ef10 100644 --- a/internal/flows/proxy_flow.go +++ b/internal/flows/proxy_flow.go @@ -248,6 +248,7 @@ func (f *proxyFlow) executeWithProxy(ctx context.Context, parsedCmd *packagemana if err != nil { return fmt.Errorf("failed to apply sandbox: %w", err) } + defer result.Close() // Clean up sandbox resources log.Debugf("Executing command: %s %v", parsedCmd.Command.Exe, parsedCmd.Command.Args) log.Debugf("Proxy environment: HTTP_PROXY=%s, HTTPS_PROXY=%s, NODE_EXTRA_CA_CERTS=%s", proxyURL, proxyURL, caCertPath) diff --git a/sandbox/executor/apply.go b/sandbox/executor/apply.go index 57c1e0b..e11342e 100644 --- a/sandbox/executor/apply.go +++ b/sandbox/executor/apply.go @@ -84,5 +84,6 @@ func ApplySandbox(ctx context.Context, cmd *exec.Cmd, pmName string) (*sandbox.E return nil, fmt.Errorf("failed to setup sandbox: %w", err) } - return result, nil + // Return result with sandbox reference so caller can defer result.Close() + return sandbox.NewExecutionResultWithSandbox(result.WasExecuted(), sb), nil } diff --git a/sandbox/platform/seatbelt_darwin.go b/sandbox/platform/seatbelt_darwin.go index b8934ff..7896c67 100644 --- a/sandbox/platform/seatbelt_darwin.go +++ b/sandbox/platform/seatbelt_darwin.go @@ -15,7 +15,9 @@ import ( // seatbeltSandbox implements the Sandbox interface using macOS Seatbelt (sandbox-exec). type seatbeltSandbox struct { - translator *policyTranslator + translator *policyTranslator + tempProfilePath string // Path to temporary .sb file, cleaned up in Close() + cleanupCompleted bool // Track if cleanup already happened (idempotent Close) } // newSeatbeltSandbox creates a new Seatbelt sandbox instance. @@ -39,20 +41,25 @@ func (s *seatbeltSandbox) Execute(ctx context.Context, cmd *exec.Cmd, policy *sa } // Write Seatbelt profile to temporary file + // The file will be cleaned up when Close() is called tmpFile, err := os.CreateTemp("", "pmg-sandbox-*.sb") if err != nil { return nil, fmt.Errorf("failed to create temporary sandbox profile: %w", err) } - defer os.Remove(tmpFile.Name()) + // Store the path for cleanup in Close() + s.tempProfilePath = tmpFile.Name() if _, err := tmpFile.WriteString(sbProfile); err != nil { tmpFile.Close() + // Clean up on error + os.Remove(s.tempProfilePath) + s.tempProfilePath = "" return nil, fmt.Errorf("failed to write sandbox profile: %w", err) } tmpFile.Close() - log.Debugf("Seatbelt profile written to %s", tmpFile.Name()) + log.Debugf("Seatbelt profile written to %s", s.tempProfilePath) log.Debugf("Seatbelt profile content:\n%s", sbProfile) // Modify command to run via sandbox-exec @@ -63,7 +70,7 @@ func (s *seatbeltSandbox) Execute(ctx context.Context, cmd *exec.Cmd, policy *sa cmd.Path = "/usr/bin/sandbox-exec" cmd.Args = []string{ "sandbox-exec", - "-f", tmpFile.Name(), + "-f", s.tempProfilePath, originalPath, } @@ -88,3 +95,24 @@ func (s *seatbeltSandbox) IsAvailable() bool { _, err := exec.LookPath("sandbox-exec") return err == nil } + +// Close cleans up the temporary seatbelt profile file. +// Safe to call multiple times (idempotent). +func (s *seatbeltSandbox) Close() error { + // Idempotent - return early if already cleaned up or no file to clean + if s.cleanupCompleted || s.tempProfilePath == "" { + return nil + } + + log.Debugf("Cleaning up seatbelt profile: %s", s.tempProfilePath) + + err := os.Remove(s.tempProfilePath) + s.cleanupCompleted = true + + if err != nil && !os.IsNotExist(err) { + // Only return error if it's not "file doesn't exist" + return fmt.Errorf("failed to remove seatbelt profile %s: %w", s.tempProfilePath, err) + } + + return nil +} diff --git a/sandbox/profiles/npm-restrictive.yml b/sandbox/profiles/npm-restrictive.yml index d3d7bf7..26f19ba 100644 --- a/sandbox/profiles/npm-restrictive.yml +++ b/sandbox/profiles/npm-restrictive.yml @@ -79,10 +79,11 @@ process: - ${HOME}/.bun/install/cache/** - /usr/bin/git - /usr/local/bin/git + - /bin/bash + - /bin/sh + - /usr/bin/env deny_exec: - /usr/bin/curl - /usr/bin/wget - - /bin/bash - - /bin/sh - /usr/bin/python* diff --git a/sandbox/sandbox.go b/sandbox/sandbox.go index 7505ac8..0bf5916 100644 --- a/sandbox/sandbox.go +++ b/sandbox/sandbox.go @@ -8,8 +8,10 @@ import ( // ExecutionResult represents the result of applying a sandbox to a command. // It encapsulates the execution state and allows for future extension with // additional metadata (e.g., exit codes, resource usage, violation events). +// Callers must call Close() after cmd.Run() completes to clean up resources. type ExecutionResult struct { executed bool + sandbox Sandbox // Reference to sandbox for cleanup // Future fields can be added here without breaking the API: // - exitCode int // - resourceUsage ResourceStats @@ -19,9 +21,20 @@ type ExecutionResult struct { // NewExecutionResult creates a new ExecutionResult. // If executed is true, it indicates the sandbox executed the command directly. // If executed is false, the sandbox only modified the command and the caller must execute it. +// The sandbox parameter can be nil if no sandbox was applied. func NewExecutionResult(executed bool) *ExecutionResult { return &ExecutionResult{ executed: executed, + sandbox: nil, + } +} + +// NewExecutionResultWithSandbox creates a new ExecutionResult with a sandbox reference. +// The sandbox's Close() method will be called when result.Close() is called. +func NewExecutionResultWithSandbox(executed bool, sb Sandbox) *ExecutionResult { + return &ExecutionResult{ + executed: executed, + sandbox: sb, } } @@ -37,6 +50,16 @@ func (r *ExecutionResult) ShouldRun() bool { return !r.executed } +// Close cleans up any resources allocated by the sandbox. +// Must be called after cmd.Run() completes. Safe to call multiple times (idempotent). +// Safe to call even if no sandbox was applied (sandbox is nil). +func (r *ExecutionResult) Close() error { + if r.sandbox != nil { + return r.sandbox.Close() + } + return nil +} + // Sandbox represents a platform-specific sandbox executor that isolates // package manager processes with controlled access to filesystem, network, // and process execution resources. @@ -61,6 +84,10 @@ type Sandbox interface { // IsAvailable returns true if the sandbox is available and functional on this platform. IsAvailable() bool + + // Close cleans up any resources allocated by the sandbox (e.g., temporary files). + // Must be called after cmd.Run() completes. Idempotent - safe to call multiple times. + Close() error }