fix: Add support for sandbox cleanup

This commit is contained in:
Abhisek Datta
2026-01-08 14:30:33 +05:30
parent 9f77cca5e5
commit f07d8e6a38
6 changed files with 66 additions and 7 deletions
+1
View File
@@ -227,6 +227,7 @@ func (g *packageManagerGuard) continueExecution(ctx context.Context, pc *package
if err != nil { if err != nil {
return fmt.Errorf("failed to apply sandbox: %w", err) 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 // Only run the command if the sandbox didn't already execute it
if result.ShouldRun() { if result.ShouldRun() {
+1
View File
@@ -248,6 +248,7 @@ func (f *proxyFlow) executeWithProxy(ctx context.Context, parsedCmd *packagemana
if err != nil { if err != nil {
return fmt.Errorf("failed to apply sandbox: %w", err) 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("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) log.Debugf("Proxy environment: HTTP_PROXY=%s, HTTPS_PROXY=%s, NODE_EXTRA_CA_CERTS=%s", proxyURL, proxyURL, caCertPath)
+2 -1
View File
@@ -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 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
} }
+32 -4
View File
@@ -15,7 +15,9 @@ import (
// seatbeltSandbox implements the Sandbox interface using macOS Seatbelt (sandbox-exec). // seatbeltSandbox implements the Sandbox interface using macOS Seatbelt (sandbox-exec).
type seatbeltSandbox struct { 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. // 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 // Write Seatbelt profile to temporary file
// The file will be cleaned up when Close() is called
tmpFile, err := os.CreateTemp("", "pmg-sandbox-*.sb") tmpFile, err := os.CreateTemp("", "pmg-sandbox-*.sb")
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to create temporary sandbox profile: %w", err) 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 { if _, err := tmpFile.WriteString(sbProfile); err != nil {
tmpFile.Close() tmpFile.Close()
// Clean up on error
os.Remove(s.tempProfilePath)
s.tempProfilePath = ""
return nil, fmt.Errorf("failed to write sandbox profile: %w", err) return nil, fmt.Errorf("failed to write sandbox profile: %w", err)
} }
tmpFile.Close() 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) log.Debugf("Seatbelt profile content:\n%s", sbProfile)
// Modify command to run via sandbox-exec // 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.Path = "/usr/bin/sandbox-exec"
cmd.Args = []string{ cmd.Args = []string{
"sandbox-exec", "sandbox-exec",
"-f", tmpFile.Name(), "-f", s.tempProfilePath,
originalPath, originalPath,
} }
@@ -88,3 +95,24 @@ func (s *seatbeltSandbox) IsAvailable() bool {
_, err := exec.LookPath("sandbox-exec") _, err := exec.LookPath("sandbox-exec")
return err == nil 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
}
+3 -2
View File
@@ -79,10 +79,11 @@ process:
- ${HOME}/.bun/install/cache/** - ${HOME}/.bun/install/cache/**
- /usr/bin/git - /usr/bin/git
- /usr/local/bin/git - /usr/local/bin/git
- /bin/bash
- /bin/sh
- /usr/bin/env
deny_exec: deny_exec:
- /usr/bin/curl - /usr/bin/curl
- /usr/bin/wget - /usr/bin/wget
- /bin/bash
- /bin/sh
- /usr/bin/python* - /usr/bin/python*
+27
View File
@@ -8,8 +8,10 @@ import (
// ExecutionResult represents the result of applying a sandbox to a command. // ExecutionResult represents the result of applying a sandbox to a command.
// It encapsulates the execution state and allows for future extension with // It encapsulates the execution state and allows for future extension with
// additional metadata (e.g., exit codes, resource usage, violation events). // 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 { type ExecutionResult struct {
executed bool executed bool
sandbox Sandbox // Reference to sandbox for cleanup
// Future fields can be added here without breaking the API: // Future fields can be added here without breaking the API:
// - exitCode int // - exitCode int
// - resourceUsage ResourceStats // - resourceUsage ResourceStats
@@ -19,9 +21,20 @@ type ExecutionResult struct {
// NewExecutionResult creates a new ExecutionResult. // NewExecutionResult creates a new ExecutionResult.
// If executed is true, it indicates the sandbox executed the command directly. // 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. // 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 { func NewExecutionResult(executed bool) *ExecutionResult {
return &ExecutionResult{ return &ExecutionResult{
executed: executed, 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 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 // Sandbox represents a platform-specific sandbox executor that isolates
// package manager processes with controlled access to filesystem, network, // package manager processes with controlled access to filesystem, network,
// and process execution resources. // 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 returns true if the sandbox is available and functional on this platform.
IsAvailable() bool 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
} }