fix: Misc cleanup fixes

sandbox/executor/apply.go

@@ -14,67 +14,55 @@ import (
 // ApplySandbox applies sandbox isolation to the command if sandbox mode is enabled.
 // This is a helper function used by both guard and proxy flows to avoid code duplication.
 //
-// Parameters:
+// This is a security sensitive operation. If sandbox is enabled via. config but not available on the platform,
-//   - ctx: Context for the sandbox execution
+// it will return an error to avoid running the command without sandbox protection.
-//   - cmd: The exec.Cmd to be sandboxed (will be modified in place)
-//   - pmName: Package manager name (e.g., "npm", "pip") used to determine the sandbox policy to apply
-//   - mode: Optional mode description for logging (e.g., "proxy mode", empty for default)
-//
-// Returns:
-//   - ExecutionResult: Contains execution state. Callers must check result.ShouldRun() before calling cmd.Run().
-//   - error: Non-nil if sandbox setup fails
-//
-// If sandbox is not enabled/available, returns a result indicating the caller should run the command.
-// Gracefully degrades with warnings if sandbox is unavailable on the platform.
 func ApplySandbox(ctx context.Context, cmd *exec.Cmd, pmName string) (*sandbox.ExecutionResult, error) {
 	cfg := config.Get()
 
 	if !cfg.Config.Sandbox.Enabled {
-		return sandbox.NewExecutionResult(false), nil
+		return sandbox.NewExecutionResult(), nil
 	}
 
 	registry := sandbox.NewProfileRegistry()
 	var policy *sandbox.SandboxPolicy
 	var err error
 
-	// Check for runtime profile override first (--sandbox-profile flag)
 	if cfg.SandboxProfileOverride != "" {
 		log.Debugf("Using sandbox profile override: %s", cfg.SandboxProfileOverride)
+
 		policy, err = registry.GetProfile(cfg.SandboxProfileOverride)
 		if err != nil {
 			return nil, fmt.Errorf("failed to load override sandbox policy %s: %w", cfg.SandboxProfileOverride, err)
 		}
 	} else {
-		// Use configured per-package-manager policy
+		log.Debugf("Looking up sandbox policy for %s", pmName)
+
 		policyRef, exists := cfg.Config.Sandbox.Policies[pmName]
 		if !exists || !policyRef.Enabled {
-			log.Debugf("No sandbox policy enabled for %s", pmName)
+			return nil, fmt.Errorf("no sandbox policy enabled for %s", pmName)
-			return sandbox.NewExecutionResult(false), nil
 		}
 
+		log.Debugf("Loading sandbox policy %s", policyRef.Profile)
+
 		policy, err = registry.GetProfile(policyRef.Profile)
 		if err != nil {
 			return nil, fmt.Errorf("failed to load sandbox policy %s: %w", policyRef.Profile, err)
 		}
 	}
 
-	// Validate that the loaded policy applies to this package manager
+	log.Debugf("Loaded sandbox policy %s", policy.Name)
+
 	if !policy.AppliesToPackageManager(pmName) {
-		log.Warnf("Sandbox policy %s does not apply to %s", policy.Name, pmName)
+		return nil, fmt.Errorf("sandbox policy %s does not apply to %s", policy.Name, pmName)
-		return sandbox.NewExecutionResult(false), nil
 	}
 
-	// Create platform-specific sandbox
 	sb, err := platform.NewSandbox()
 	if err != nil {
-		log.Warnf("Sandbox not available on this platform: %v", err)
+		return nil, fmt.Errorf("sandbox not available on this platform: %v", err)
-		log.Warnf("Continuing without sandbox protection")
-		return sandbox.NewExecutionResult(false), nil
 	}
 
 	if !sb.IsAvailable() {
-		log.Warnf("Sandbox %s not available, running without sandbox", sb.Name())
+		return nil, fmt.Errorf("sandbox %s not available, running without sandbox", sb.Name())
-		return sandbox.NewExecutionResult(false), nil
 	}
 
 	log.Debugf("Running %s in %s sandbox with policy %s", pmName, sb.Name(), policy.Name)
@@ -84,6 +72,5 @@ func ApplySandbox(ctx context.Context, cmd *exec.Cmd, pmName string) (*sandbox.E
 		return nil, fmt.Errorf("failed to setup sandbox: %w", err)
 	}
 
-	// Return result with sandbox reference so caller can defer result.Close()
+	return result, nil
-	return sandbox.NewExecutionResultWithSandbox(result.WasExecuted(), sb), nil
 }

sandbox/platform/seatbelt_darwin.go

@@ -81,8 +81,7 @@ func (s *seatbeltSandbox) Execute(ctx context.Context, cmd *exec.Cmd, policy *sa
 
 	log.Debugf("Sandboxed command: %s %v", cmd.Path, cmd.Args)
 
-	// Return ExecutionResult indicating we only modified cmd, didn't execute it
+	return sandbox.NewExecutionResult(sandbox.WithExecutionResultSandbox(s)), nil
-	return sandbox.NewExecutionResult(false), nil
 }
 
 // Name returns the name of this sandbox implementation.
@@ -97,9 +96,7 @@ func (s *seatbeltSandbox) IsAvailable() bool {
 }
 
 // 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
 	}
@@ -110,7 +107,6 @@ func (s *seatbeltSandbox) Close() error {
 	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)
 	}
 

sandbox/sandbox.go

@@ -5,54 +5,49 @@ import (
 	"os/exec"
 )
 
-// ExecutionResult represents the result of applying a sandbox to a command.
+// ExecutionResult represents the result of executing a command in a sandbox.
-// It encapsulates the execution state and allows for future extension with
+// It contains sandbox internal 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
+	sandbox  Sandbox
-	// Future fields can be added here without breaking the API:
+}
-	// - exitCode int
+
-	// - resourceUsage ResourceStats
+// ExecutionResultOpt is a function that can be used to configure an ExecutionResult.
-	// - violations []ViolationEvent
+type ExecutionResultOpt func(*ExecutionResult)
+
+// WithSandbox sets the sandbox for the ExecutionResult.
+func WithExecutionResultSandbox(sb Sandbox) ExecutionResultOpt {
+	return func(r *ExecutionResult) {
+		r.sandbox = sb
+	}
+}
+
+// WithExecuted sets the executed flag for the ExecutionResult.
+func WithExecutionResultExecuted(executed bool) ExecutionResultOpt {
+	return func(r *ExecutionResult) {
+		r.executed = executed
+	}
 }
 
 // NewExecutionResult creates a new ExecutionResult.
-// If executed is true, it indicates the sandbox executed the command directly.
+func NewExecutionResult(opts ...ExecutionResultOpt) *ExecutionResult {
-// If executed is false, the sandbox only modified the command and the caller must execute it.
+	r := &ExecutionResult{}
-// The sandbox parameter can be nil if no sandbox was applied.
+	for _, opt := range opts {
-func NewExecutionResult(executed bool) *ExecutionResult {
+		opt(r)
-	return &ExecutionResult{
-		executed: executed,
-		sandbox:  nil,
 	}
-}
 
-// NewExecutionResultWithSandbox creates a new ExecutionResult with a sandbox reference.
+	return r
-// 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,
-	}
-}
-
-// WasExecuted returns true if the sandbox executed the command directly.
-// If false, the caller must execute the command using cmd.Run().
-func (r *ExecutionResult) WasExecuted() bool {
-	return r.executed
 }
 
 // ShouldRun returns true if the caller should execute cmd.Run().
-// This is the inverse of WasExecuted() and may be more intuitive at call sites.
 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).
+// Must be called after cmd.Run() completes.
-// 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()
@@ -90,7 +85,6 @@ type Sandbox interface {
 	Close() error
 }
 
-
 // ProfileRegistry manages built-in and custom sandbox policies.
 type ProfileRegistry interface {
 	// GetProfile retrieves a policy by name.