From 2e1f5b1a3656bfb81021c2df3c0d80d08f0fec7c Mon Sep 17 00:00:00 2001 From: Abhisek Datta Date: Wed, 14 Jan 2026 10:50:39 +0530 Subject: [PATCH] feat: Add support for policy inheritence (#113) * feat: Add support for policy inheritence * fix: Linter fixes * Update docs/sandbox.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Abhisek Datta * fix: Handle boolean inheritence * ci: Add linter * Update sandbox/policy_test.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Abhisek Datta * fix: Linter fixes * fix: Linter fixes * fix: Sandbox rule regex format --------- Signed-off-by: Abhisek Datta Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .github/workflows/golangci-lint.yml | 27 ++ config/config.template.yml | 7 +- docs/sandbox.md | 2 +- go.mod | 2 + go.sum | 3 + internal/alias/alias.go | 24 +- internal/eventlog/eventlog.go | 29 +- internal/eventlog/eventlog_test.go | 27 +- internal/flows/proxy_flow.go | 4 +- internal/pty/session.go | 6 +- .../platform/seatbelt_translator_darwin.go | 23 +- .../seatbelt_translator_darwin_test.go | 25 +- sandbox/policy.go | 102 ++++++- sandbox/policy_test.go | 288 ++++++++++++++++++ sandbox/profiles/npx.yml | 24 ++ sandbox/registry.go | 75 +++++ sandbox/registry_test.go | 205 +++++++++++++ 17 files changed, 815 insertions(+), 58 deletions(-) create mode 100644 .github/workflows/golangci-lint.yml create mode 100644 sandbox/policy_test.go create mode 100644 sandbox/profiles/npx.yml diff --git a/.github/workflows/golangci-lint.yml b/.github/workflows/golangci-lint.yml new file mode 100644 index 0000000..e7fdda3 --- /dev/null +++ b/.github/workflows/golangci-lint.yml @@ -0,0 +1,27 @@ +name: Go Linter +on: + pull_request: + branches: + - main +permissions: + contents: read + pull-requests: read + +jobs: + golangci: + name: lint + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 + + - uses: actions/setup-go@44694675825211faa026b3c33043df3e48a5fa00 # v6 + with: + go-version-file: go.mod + + - name: golangci-lint + uses: golangci/golangci-lint-action@4afd733a84b1f43292c63897423277bb7f4313a9 # v8 + with: + version: latest + args: --issues-exit-code=1 --timeout=10m + only-new-issues: true diff --git a/config/config.template.yml b/config/config.template.yml index 98c2506..66473d9 100644 --- a/config/config.template.yml +++ b/config/config.template.yml @@ -65,6 +65,9 @@ sandbox: # Policy templates define policy profiles by name and path. # They can be used to override a built-in profile or create a custom profile. + # Note: Custom profiles loaded via policy_templates can inherit from built-in + # profiles using the 'inherits' field in the YAML (e.g., inherits: npm-restrictive). + # Inheritance allows you to extend a base profile with additional permissions. policy_templates: # Name for the template. Can be used to override a built-in profile or create a custom profile. # Path is the path to the template file. @@ -86,11 +89,11 @@ sandbox: npx: enabled: true - profile: npm-restrictive + profile: npx pnpx: enabled: true - profile: npm-restrictive + profile: npx yarn: enabled: true diff --git a/docs/sandbox.md b/docs/sandbox.md index d71d9b7..22c1989 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -94,7 +94,7 @@ To manually investigate sandbox policy violations, you can use the following com APP_LOG_LEVEL=debug APP_LOG_FILE=/tmp/pmg-debug.log pmg --sandbox --sandbox-profile=npm-restrictive npm install express ``` -Find the log tag in the log file and use it to investigate the sandbox policy violation. +Find the log tag in the debug log file and use it to investigate the sandbox policy violation. ```bash grep "PMG_SBX_" /tmp/pmg-debug.log diff --git a/go.mod b/go.mod index a08a3a9..ba62b5c 100644 --- a/go.mod +++ b/go.mod @@ -91,6 +91,7 @@ require ( github.com/go-xmlfmt/xmlfmt v1.1.3 // indirect github.com/gobwas/glob v0.2.3 // indirect github.com/gofrs/flock v0.12.1 // indirect + github.com/golang/protobuf v1.5.4 // indirect github.com/golangci/dupl v0.0.0-20250308024227-f665c8d69b32 // indirect github.com/golangci/go-printf-func-name v0.1.0 // indirect github.com/golangci/gofmt v0.0.0-20250106114630-d62b90e6713d // indirect @@ -147,6 +148,7 @@ require ( github.com/nishanths/exhaustive v0.12.0 // indirect github.com/nishanths/predeclared v0.2.2 // indirect github.com/nunnatsa/ginkgolinter v0.19.1 // indirect + github.com/oklog/ulid/v2 v2.1.0 // indirect github.com/olekukonko/tablewriter v0.0.5 // indirect github.com/package-url/packageurl-go v0.1.3 // indirect github.com/pelletier/go-toml/v2 v2.2.4 // indirect diff --git a/go.sum b/go.sum index bfc0e50..9dcc456 100644 --- a/go.sum +++ b/go.sum @@ -327,6 +327,8 @@ github.com/nishanths/predeclared v0.2.2 h1:V2EPdZPliZymNAn79T8RkNApBjMmVKh5XRpLm github.com/nishanths/predeclared v0.2.2/go.mod h1:RROzoN6TnGQupbC+lqggsOlcgysk3LMK/HI84Mp280c= github.com/nunnatsa/ginkgolinter v0.19.1 h1:mjwbOlDQxZi9Cal+KfbEJTCz327OLNfwNvoZ70NJ+c4= github.com/nunnatsa/ginkgolinter v0.19.1/go.mod h1:jkQ3naZDmxaZMXPWaS9rblH+i+GWXQCaS/JFIWcOH2s= +github.com/oklog/ulid/v2 v2.1.0 h1:+9lhoxAP56we25tyYETBBY1YLA2SaoLvUFgrP2miPJU= +github.com/oklog/ulid/v2 v2.1.0/go.mod h1:rcEKHmBBKfef9DhnvX7y1HZBYxjXb0cP5ExxNsTT1QQ= github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec= github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY= github.com/onsi/ginkgo/v2 v2.22.2 h1:/3X8Panh8/WwhU/3Ssa6rCKqPLuAkVY2I0RoyDLySlU= @@ -343,6 +345,7 @@ github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT9 github.com/otiai10/mint v1.3.1/go.mod h1:/yxELlJQ0ufhjUwhshSj+wFjZ78CnZ48/1wtmBH1OTc= github.com/package-url/packageurl-go v0.1.3 h1:4juMED3hHiz0set3Vq3KeQ75KD1avthoXLtmE3I0PLs= github.com/package-url/packageurl-go v0.1.3/go.mod h1:nKAWB8E6uk1MHqiS/lQb9pYBGH2+mdJ2PJc2s50dQY0= +github.com/pborman/getopt v0.0.0-20170112200414-7148bc3a4c30/go.mod h1:85jBQOZwpVEaDAr341tbn15RS4fCAsIst0qp7i8ex1o= github.com/pelletier/go-toml/v2 v2.2.4 h1:mye9XuhQ6gvn5h28+VilKrrPoQVanw5PMw/TB0t5Ec4= github.com/pelletier/go-toml/v2 v2.2.4/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0= diff --git a/internal/alias/alias.go b/internal/alias/alias.go index c235c86..ac9ce26 100644 --- a/internal/alias/alias.go +++ b/internal/alias/alias.go @@ -211,6 +211,7 @@ func (a *AliasManager) removeSourceLinesFromShells() error { if os.IsNotExist(err) { continue } + log.Warnf("Warning: skipping %s (%s)", shell.Name(), err) continue } @@ -218,12 +219,14 @@ func (a *AliasManager) removeSourceLinesFromShells() error { // Get original file permissions info, err := os.Stat(configPath) if err != nil { + log.Warnf("Warning: skipping %s (%s)", shell.Name(), err) continue } // Create temp file tempFile, err := os.CreateTemp(filepath.Dir(configPath), ".tmp-"+filepath.Base(configPath)) if err != nil { + log.Warnf("Warning: failed to create temporary file for %s: %s", configPath, err) continue } @@ -242,16 +245,27 @@ func (a *AliasManager) removeSourceLinesFromShells() error { continue } - writer.WriteString(line + "\n") + if _, err := writer.WriteString(line + "\n"); err != nil { + log.Warnf("Warning: failed to write to temporary file: %s", err) + } } - writer.Flush() - tempFile.Close() + if err := writer.Flush(); err != nil { + log.Warnf("Warning: failed to flush temporary file: %s", err) + } + + if err := tempFile.Close(); err != nil { + log.Warnf("Warning: failed to close temporary file: %s", err) + } + + // Set permissions on temporary file to match original file. + if err := os.Chmod(tempPath, info.Mode()); err != nil { + log.Warnf("Warning: failed to set permissions on temporary file for %s: %s", configPath, err) + } // Replace original file - os.Chmod(tempPath, info.Mode()) if err := os.Rename(tempPath, configPath); err != nil { - os.Remove(tempPath) + _ = os.Remove(tempPath) log.Warnf("Warning: failed to update %s: %s", configPath, err) } } diff --git a/internal/eventlog/eventlog.go b/internal/eventlog/eventlog.go index 7d8d4d5..1f51d7f 100644 --- a/internal/eventlog/eventlog.go +++ b/internal/eventlog/eventlog.go @@ -248,7 +248,9 @@ func (l *fileWithRotationLogger) Log(event Event) error { // Flush to ensure data is written if l.file != nil { - l.file.Sync() + if err := l.file.Sync(); err != nil { + return fmt.Errorf("failed to sync file: %w", err) + } } return nil @@ -267,6 +269,7 @@ func (l *fileWithRotationLogger) Close() error { if l.file != nil { return l.file.Close() } + return nil } @@ -303,7 +306,9 @@ func LogMalwareBlocked(packageName, version, ecosystem, reason string, details m } event.Details["reason"] = reason - LogEvent(event) + if err := LogEvent(event); err != nil { + log.Warnf("failed to log malware blocked event: %s", err) + } } // LogMalwareConfirmed logs when user confirms installation despite warning @@ -316,7 +321,9 @@ func LogMalwareConfirmed(packageName, version, ecosystem string) { Ecosystem: ecosystem, } - LogEvent(event) + if err := LogEvent(event); err != nil { + log.Warnf("failed to log malware confirmed event: %s", err) + } } // LogInstallAllowed logs when an installation is allowed @@ -332,7 +339,9 @@ func LogInstallAllowed(packageName, version, ecosystem string, packageCount int) }, } - LogEvent(event) + if err := LogEvent(event); err != nil { + log.Warnf("failed to log install allowed event: %s", err) + } } // LogInstallTrustedAllowed logs when an installation is allowed for a trusted package @@ -345,7 +354,9 @@ func LogInstallTrustedAllowed(packageName, version, ecosystem string) { Ecosystem: ecosystem, } - LogEvent(event) + if err := LogEvent(event); err != nil { + log.Warnf("failed to log install trusted allowed event: %s", err) + } } // LogInstallStarted logs when an installation starts @@ -359,7 +370,9 @@ func LogInstallStarted(packageManager string, args []string) { }, } - LogEvent(event) + if err := LogEvent(event); err != nil { + log.Warnf("failed to log install started event: %s", err) + } } // LogError logs an error event @@ -372,7 +385,9 @@ func LogError(message string, err error) { }, } - LogEvent(event) + if err := LogEvent(event); err != nil { + log.Warnf("failed to log error event: %s", err) + } } // Close closes the global logger diff --git a/internal/eventlog/eventlog_test.go b/internal/eventlog/eventlog_test.go index 4b6f831..c1ddc3a 100644 --- a/internal/eventlog/eventlog_test.go +++ b/internal/eventlog/eventlog_test.go @@ -27,7 +27,10 @@ func TestLoggerInitialization(t *testing.T) { // Initialize logger err := InitializeWithDir(logDir) assert.NoError(t, err, "Failed to initialize logger") - defer Close() + defer func() { + err := Close() + assert.NoError(t, err) + }() // Check that directory was created _, err = os.Stat(logDir) @@ -47,7 +50,10 @@ func TestLogEvent(t *testing.T) { // Initialize logger err := reinitializeForTest(logDir) assert.NoError(t, err, "Failed to initialize logger") - defer Close() + defer func() { + err := Close() + assert.NoError(t, err) + }() // Log an event event := Event{ @@ -87,7 +93,10 @@ func TestLogMalwareBlocked(t *testing.T) { // Initialize logger err := reinitializeForTest(logDir) assert.NoError(t, err, "Failed to initialize logger") - defer Close() + defer func() { + err := Close() + assert.NoError(t, err) + }() // Log malware blocked event LogMalwareBlocked("malicious-pkg", "2.0.0", "pypi", "Contains known malware", nil) @@ -114,14 +123,17 @@ func TestInitializeWithFile(t *testing.T) { // Initialize logger with custom file err := reinitializeForTest("") if err == nil { - Close() + _ = Close() } // Reset for custom file once = sync.Once{} err = InitializeWithFile(logFile) assert.NoError(t, err, "Failed to initialize logger with file") - defer Close() + defer func() { + err := Close() + assert.NoError(t, err) + }() // Log an event event := Event{ @@ -177,7 +189,10 @@ func TestCleanupOldLogs(t *testing.T) { err = logger.init(logDir) assert.NoError(t, err, "Failed to initialize logger") - defer logger.Close() + defer func() { + err := logger.Close() + assert.NoError(t, err) + }() // Give cleanup goroutine time to run time.Sleep(100 * time.Millisecond) diff --git a/internal/flows/proxy_flow.go b/internal/flows/proxy_flow.go index 1325ca5..d2faaa8 100644 --- a/internal/flows/proxy_flow.go +++ b/internal/flows/proxy_flow.go @@ -368,7 +368,9 @@ func (f *proxyFlow) executeWithProxy( var wg sync.WaitGroup wg.Go(func() { - io.Copy(outputRouter, sess.PtyReader()) + if _, err := io.Copy(outputRouter, sess.PtyReader()); err != nil { + log.Errorf("failed to copy output: %v", err) + } }) inputRouter, err := pty.NewInputRouter(sess.PtyWriter()) diff --git a/internal/pty/session.go b/internal/pty/session.go index 1b69580..f092307 100644 --- a/internal/pty/session.go +++ b/internal/pty/session.go @@ -101,8 +101,10 @@ func NewSession(ctx context.Context, cfg SessionConfig) (InteractiveSession, err Env: cfg.Env, }) if err != nil { - c.Restore(oldState) - c.Close() + // We are already in error state, restore and close is best effort. + _ = c.Restore(oldState) + _ = c.Close() + return nil, fmt.Errorf("failed to spawn: %w", err) } diff --git a/sandbox/platform/seatbelt_translator_darwin.go b/sandbox/platform/seatbelt_translator_darwin.go index d6e043b..cc0c3e5 100644 --- a/sandbox/platform/seatbelt_translator_darwin.go +++ b/sandbox/platform/seatbelt_translator_darwin.go @@ -9,6 +9,7 @@ import ( "path/filepath" "strings" + "github.com/safedep/dry/utils" "github.com/safedep/pmg/sandbox" "github.com/safedep/pmg/sandbox/util" ) @@ -138,7 +139,7 @@ func generateMoveBlockingRules(pathPatterns []string, logTag string) []string { if util.ContainsGlob(pathPattern) { // For glob patterns, use regex matching for precise pattern enforcement regexPattern := util.GlobToRegex(pathPattern) - rules = append(rules, fmt.Sprintf("(deny file-write-unlink (regex \"%s\") (with message \"%s\"))", regexPattern, logTag)) + rules = append(rules, fmt.Sprintf("(deny file-write-unlink (regex #\"%s\") (with message \"%s\"))", regexPattern, logTag)) // Also block moving the base directory to prevent bypass baseDir := extractBaseDir(pathPattern) @@ -358,7 +359,7 @@ func (t *seatbeltPolicyTranslator) translate(policy *sandbox.SandboxPolicy) (str } // PTY support (optional) - if policy.AllowPTY { + if utils.SafelyGetValue(policy.AllowPTY) { sb.WriteString(";; Pseudo-terminal (PTY) support\n") sb.WriteString("(allow pseudo-tty)\n") sb.WriteString("(allow file-ioctl\n") @@ -390,7 +391,7 @@ func (t *seatbeltPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPo globDoubleStarAutoAllowParentDirIfNeeded(sb, pattern, expanded, "file-read*") regexPattern := util.GlobToRegex(expanded) - sb.WriteString(fmt.Sprintf("(allow file-read* (regex \"%s\"))\n", regexPattern)) + sb.WriteString(fmt.Sprintf("(allow file-read* (regex #\"%s\"))\n", regexPattern)) } else { sb.WriteString(fmt.Sprintf("(allow file-read* (subpath \"%s\"))\n", expanded)) } @@ -424,7 +425,7 @@ func (t *seatbeltPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPo globDoubleStarAutoAllowParentDirIfNeeded(sb, pattern, expanded, "file-write*") regexPattern := util.GlobToRegex(expanded) - sb.WriteString(fmt.Sprintf("(allow file-write* (regex \"%s\"))\n", regexPattern)) + sb.WriteString(fmt.Sprintf("(allow file-write* (regex #\"%s\"))\n", regexPattern)) } else { sb.WriteString(fmt.Sprintf("(allow file-write* (subpath \"%s\"))\n", expanded)) } @@ -444,7 +445,7 @@ func (t *seatbeltPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPo // Use regex matching for glob patterns, subpath for literals if util.ContainsGlob(expanded) { regexPattern := util.GlobToRegex(expanded) - sb.WriteString(fmt.Sprintf("(deny file-read* (regex \"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) + sb.WriteString(fmt.Sprintf("(deny file-read* (regex #\"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) } else { sb.WriteString(fmt.Sprintf("(deny file-read* (subpath \"%s\") (with message \"%s\"))\n", expanded, t.logTag)) } @@ -471,7 +472,7 @@ func (t *seatbeltPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPo // Use regex matching for glob patterns, subpath for literals if util.ContainsGlob(expanded) { regexPattern := util.GlobToRegex(expanded) - sb.WriteString(fmt.Sprintf("(deny file-write* (regex \"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) + sb.WriteString(fmt.Sprintf("(deny file-write* (regex #\"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) } else { sb.WriteString(fmt.Sprintf("(deny file-write* (subpath \"%s\") (with message \"%s\"))\n", expanded, t.logTag)) } @@ -483,7 +484,7 @@ func (t *seatbeltPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPo if t.enableDangerousFileBlocking { // Add mandatory deny patterns for security (credentials, git hooks, etc.) sb.WriteString(";; Mandatory security denies (credentials, git hooks, etc.)\n") - mandatoryDenies := util.GetMandatoryDenyPatterns(policy.AllowGitConfig) + mandatoryDenies := util.GetMandatoryDenyPatterns(utils.SafelyGetValue(policy.AllowGitConfig)) for _, pattern := range mandatoryDenies { // Expand variables if needed expanded, err := util.ExpandVariables(pattern) @@ -494,8 +495,8 @@ func (t *seatbeltPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPo // Use regex matching for glob patterns, subpath for literals if util.ContainsGlob(expanded) { regexPattern := util.GlobToRegex(expanded) - sb.WriteString(fmt.Sprintf("(deny file-write* (regex \"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) - sb.WriteString(fmt.Sprintf("(deny file-read* (regex \"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) + sb.WriteString(fmt.Sprintf("(deny file-write* (regex #\"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) + sb.WriteString(fmt.Sprintf("(deny file-read* (regex #\"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) } else { sb.WriteString(fmt.Sprintf("(deny file-write* (subpath \"%s\") (with message \"%s\"))\n", expanded, t.logTag)) sb.WriteString(fmt.Sprintf("(deny file-read* (subpath \"%s\") (with message \"%s\"))\n", expanded, t.logTag)) @@ -571,7 +572,7 @@ func (t *seatbeltPolicyTranslator) translateProcess(policy *sandbox.SandboxPolic if util.ContainsGlob(expanded) { // For glob patterns, use regex matching for precise control regexPattern := util.GlobToRegex(expanded) - sb.WriteString(fmt.Sprintf("(allow process-exec* (regex \"%s\"))\n", regexPattern)) + sb.WriteString(fmt.Sprintf("(allow process-exec* (regex #\"%s\"))\n", regexPattern)) } else { sb.WriteString(fmt.Sprintf("(allow process-exec* (literal \"%s\"))\n", expanded)) } @@ -589,7 +590,7 @@ func (t *seatbeltPolicyTranslator) translateProcess(policy *sandbox.SandboxPolic if util.ContainsGlob(expanded) { // For glob patterns, use regex matching for precise control regexPattern := util.GlobToRegex(expanded) - sb.WriteString(fmt.Sprintf("(deny process-exec* (regex \"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) + sb.WriteString(fmt.Sprintf("(deny process-exec* (regex #\"%s\") (with message \"%s\"))\n", regexPattern, t.logTag)) } else { sb.WriteString(fmt.Sprintf("(deny process-exec* (literal \"%s\") (with message \"%s\"))\n", expanded, t.logTag)) } diff --git a/sandbox/platform/seatbelt_translator_darwin_test.go b/sandbox/platform/seatbelt_translator_darwin_test.go index 0e36f12..d251be2 100644 --- a/sandbox/platform/seatbelt_translator_darwin_test.go +++ b/sandbox/platform/seatbelt_translator_darwin_test.go @@ -8,6 +8,7 @@ import ( "os" "testing" + "github.com/safedep/dry/utils" "github.com/safedep/pmg/sandbox" "github.com/stretchr/testify/assert" ) @@ -89,7 +90,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { // Should generate literal rule for parent directory assert.Contains(t, actual, `(allow file-read* (literal "/path/to/dir"))`) // Should use regex matching for glob patterns - assert.Contains(t, actual, "(allow file-read* (regex") + assert.Contains(t, actual, "(allow file-read* (regex #") assert.Contains(t, actual, "^/path/to/dir/.*$") }, }, @@ -103,7 +104,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { assert: func(t *testing.T, actual string, err error) { assert.NoError(t, err) // Should use regex matching for glob patterns - assert.Contains(t, actual, "(allow file-read* (regex") + assert.Contains(t, actual, "(allow file-read* (regex #") assert.Contains(t, actual, `^/path/to/[^/]*\.txt$`) }, }, @@ -117,7 +118,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { assert: func(t *testing.T, actual string, err error) { assert.NoError(t, err) // Should use regex matching for glob patterns - assert.Contains(t, actual, "(allow file-write* (regex") + assert.Contains(t, actual, "(allow file-write* (regex #") assert.Contains(t, actual, `^/path/to/file[^/]\.log$`) }, }, @@ -131,7 +132,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { assert: func(t *testing.T, actual string, err error) { assert.NoError(t, err) // Should use regex matching for glob patterns - assert.Contains(t, actual, "(deny file-read* (regex") + assert.Contains(t, actual, "(deny file-read* (regex #") assert.Contains(t, actual, `^/path/[^/]*/subdir$`) }, }, @@ -145,7 +146,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { assert: func(t *testing.T, actual string, err error) { assert.NoError(t, err) // Should use regex matching for glob patterns - assert.Contains(t, actual, "(allow file-read* (regex") + assert.Contains(t, actual, "(allow file-read* (regex #") assert.Contains(t, actual, `^/tmp/test[123]\.txt$`) }, }, @@ -161,7 +162,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { // Should generate literal rule for parent directory assert.Contains(t, actual, `(allow file-write* (literal "/path/to/dir"))`) // Should also generate regex rule for contents - assert.Contains(t, actual, `(allow file-write* (regex "^/path/to/dir/.*$"))`) + assert.Contains(t, actual, `(allow file-write* (regex #"^/path/to/dir/.*$"))`) }, }, { @@ -174,7 +175,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { assert: func(t *testing.T, actual string, err error) { assert.NoError(t, err) // Should only generate regex rule, no literal parent - assert.Contains(t, actual, `(allow file-write* (regex`) + assert.Contains(t, actual, `(allow file-write* (regex #`) assert.NotContains(t, actual, `(allow file-write* (literal "/path")`) }, }, @@ -224,7 +225,7 @@ func TestSeatbeltTranslatorDarwinFilesystemTranslation(t *testing.T) { assert.NoError(t, err) // Should allow writing to root (empty string after trimming becomes "/") assert.Contains(t, actual, `(allow file-write* (literal "/"))`) - assert.Contains(t, actual, `(allow file-write* (regex "^/.*$"))`) + assert.Contains(t, actual, `(allow file-write* (regex #"^/.*$"))`) }, }, { @@ -288,7 +289,7 @@ func TestSeatbeltTranslatorDarwinProcessTranslation(t *testing.T) { assert: func(t *testing.T, actual string, err error) { assert.NoError(t, err) // Should use regex matching for glob patterns - assert.Contains(t, actual, "(allow process-exec* (regex") + assert.Contains(t, actual, "(allow process-exec* (regex #") assert.Contains(t, actual, "^/usr/local/bin/.*$") }, }, @@ -302,7 +303,7 @@ func TestSeatbeltTranslatorDarwinProcessTranslation(t *testing.T) { assert: func(t *testing.T, actual string, err error) { assert.NoError(t, err) // Should use regex matching for glob patterns - assert.Contains(t, actual, "(allow process-exec* (regex") + assert.Contains(t, actual, "(allow process-exec* (regex #") assert.Contains(t, actual, `^/usr/bin/python[^/]*$`) }, }, @@ -537,7 +538,7 @@ func TestPTYSupport(t *testing.T) { Name: "test", Description: "test without PTY", PackageManagers: []string{"npm"}, - AllowPTY: false, + AllowPTY: utils.PtrTo(false), } translator := newSeatbeltPolicyTranslator() @@ -554,7 +555,7 @@ func TestPTYSupport(t *testing.T) { Name: "test", Description: "test with PTY", PackageManagers: []string{"npm"}, - AllowPTY: true, + AllowPTY: utils.PtrTo(true), } translator := newSeatbeltPolicyTranslator() diff --git a/sandbox/policy.go b/sandbox/policy.go index dfd8893..8aa1d16 100644 --- a/sandbox/policy.go +++ b/sandbox/policy.go @@ -3,23 +3,37 @@ package sandbox import ( "fmt" "strings" + + "github.com/safedep/dry/utils" ) // SandboxPolicy represents a parsed and validated sandbox policy that defines // filesystem, network, and process execution restrictions for package managers. -// Policy violations will block execution. +// Policy violations will block execution. Policy supports inheritance from parent policies. type SandboxPolicy struct { - Name string `yaml:"name" json:"name"` - Description string `yaml:"description" json:"description"` - PackageManagers []string `yaml:"package_managers" json:"package_managers"` - Filesystem FilesystemPolicy `yaml:"filesystem" json:"filesystem"` - Network NetworkPolicy `yaml:"network" json:"network"` - Process ProcessPolicy `yaml:"process" json:"process"` + // These fields are not affected by inheritance and are set from the child policy. + Name string `yaml:"name" json:"name"` + Description string `yaml:"description" json:"description"` + Inherits string `yaml:"inherits,omitempty" json:"inherits,omitempty"` + PackageManagers []string `yaml:"package_managers" json:"package_managers"` + + // These fields are affected by inheritance and are merged with the parent policy. + // Any new values added here should be handled in the MergeWithParent method. + Filesystem FilesystemPolicy `yaml:"filesystem" json:"filesystem"` + Network NetworkPolicy `yaml:"network" json:"network"` + Process ProcessPolicy `yaml:"process" json:"process"` + + // The boolean fields are pointers to allow for nil values so that the YAML parser + // can set the values from the child policy if present. We can differentiate between + // nil and false values. Any new values added here should be handled in the + // MergeWithParent method. When present in the child policy, the parent value is ignored. + // When not present in the child policy, the parent value is used. // AllowGitConfig allows write access to .git/config file. - AllowGitConfig bool `yaml:"allow_git_config" json:"allow_git_config"` + AllowGitConfig *bool `yaml:"allow_git_config" json:"allow_git_config"` + // AllowPTY allows pseudo-terminal (PTY) operations. - AllowPTY bool `yaml:"allow_pty" json:"allow_pty"` + AllowPTY *bool `yaml:"allow_pty" json:"allow_pty"` } // FilesystemPolicy defines allowed and denied filesystem access patterns. @@ -45,8 +59,10 @@ type ProcessPolicy struct { DenyExec []string `yaml:"deny_exec" json:"deny_exec"` } -// Validate validates the sandbox policy for correctness. +// Validate validates the sandbox policy for correctness before inheritance resolution. // Returns an error if the policy is invalid. +// Note: Validation for "at least one rule" check is deferred to ValidateResolved(), +// since a child policy might have no rules of its own but inherit rules from its parent. func (p *SandboxPolicy) Validate() error { if p.Name == "" { return fmt.Errorf("policy name is required") @@ -56,6 +72,17 @@ func (p *SandboxPolicy) Validate() error { return fmt.Errorf("policy must specify at least one package manager") } + return nil +} + +// ValidateResolved validates a policy after inheritance has been resolved. +// This is called after MergeWithParent to ensure the final policy is valid. +func (p *SandboxPolicy) ValidateResolved() error { + if err := p.Validate(); err != nil { + return err + } + + // Check that at least one access rule is defined (after inheritance) hasRules := len(p.Filesystem.AllowRead) > 0 || len(p.Filesystem.AllowWrite) > 0 || len(p.Filesystem.DenyRead) > 0 || @@ -66,7 +93,7 @@ func (p *SandboxPolicy) Validate() error { len(p.Process.DenyExec) > 0 if !hasRules { - return fmt.Errorf("policy must define at least one access rule") + return fmt.Errorf("policy must define at least one access rule (after inheritance resolution)") } return nil @@ -83,3 +110,56 @@ func (p *SandboxPolicy) AppliesToPackageManager(pm string) bool { return false } + +// MergeWithParent merges the child policy with its parent policy. +// Lists are unioned (additive), package_managers are replaced, booleans are overridden. +// This method modifies the receiver (child policy) in place. +func (child *SandboxPolicy) MergeWithParent(parent *SandboxPolicy) { + // Union filesystem lists + child.Filesystem.AllowRead = unionStringSlices(parent.Filesystem.AllowRead, child.Filesystem.AllowRead) + child.Filesystem.AllowWrite = unionStringSlices(parent.Filesystem.AllowWrite, child.Filesystem.AllowWrite) + child.Filesystem.DenyRead = unionStringSlices(parent.Filesystem.DenyRead, child.Filesystem.DenyRead) + child.Filesystem.DenyWrite = unionStringSlices(parent.Filesystem.DenyWrite, child.Filesystem.DenyWrite) + + // Union network lists + child.Network.AllowOutbound = unionStringSlices(parent.Network.AllowOutbound, child.Network.AllowOutbound) + child.Network.DenyOutbound = unionStringSlices(parent.Network.DenyOutbound, child.Network.DenyOutbound) + + // Union process lists + child.Process.AllowExec = unionStringSlices(parent.Process.AllowExec, child.Process.AllowExec) + child.Process.DenyExec = unionStringSlices(parent.Process.DenyExec, child.Process.DenyExec) + + // Set boolean fields by duplicating the parent value if not present in the child. + if child.AllowPTY == nil { + child.AllowPTY = utils.PtrTo(utils.SafelyGetValue(parent.AllowPTY)) + } + + if child.AllowGitConfig == nil { + child.AllowGitConfig = utils.PtrTo(utils.SafelyGetValue(parent.AllowGitConfig)) + } +} + +// unionStringSlices returns a new slice containing all unique elements from both slices. +// Order is preserved: parent entries first, then child entries (excluding duplicates). +func unionStringSlices(parent, child []string) []string { + seen := make(map[string]bool) + result := make([]string, 0, len(parent)+len(child)) + + // Add all parent entries + for _, item := range parent { + if !seen[item] { + seen[item] = true + result = append(result, item) + } + } + + // Add child entries that aren't duplicates + for _, item := range child { + if !seen[item] { + seen[item] = true + result = append(result, item) + } + } + + return result +} diff --git a/sandbox/policy_test.go b/sandbox/policy_test.go new file mode 100644 index 0000000..1aa153b --- /dev/null +++ b/sandbox/policy_test.go @@ -0,0 +1,288 @@ +package sandbox + +import ( + "testing" + + "github.com/safedep/dry/utils" + "github.com/stretchr/testify/assert" +) + +func TestUnionStringSlices(t *testing.T) { + tests := []struct { + name string + parent []string + child []string + expected []string + }{ + { + name: "empty slices", + parent: []string{}, + child: []string{}, + expected: []string{}, + }, + { + name: "parent empty, child has values", + parent: []string{}, + child: []string{"a", "b"}, + expected: []string{"a", "b"}, + }, + { + name: "parent has values, child empty", + parent: []string{"a", "b"}, + child: []string{}, + expected: []string{"a", "b"}, + }, + { + name: "no overlapping entries", + parent: []string{"a", "b"}, + child: []string{"c", "d"}, + expected: []string{"a", "b", "c", "d"}, + }, + { + name: "overlapping entries (deduplication)", + parent: []string{"a", "b", "c"}, + child: []string{"b", "c", "d"}, + expected: []string{"a", "b", "c", "d"}, + }, + { + name: "all duplicate entries", + parent: []string{"a", "b"}, + child: []string{"a", "b"}, + expected: []string{"a", "b"}, + }, + { + name: "parent has duplicates", + parent: []string{"a", "a", "b"}, + child: []string{"c"}, + expected: []string{"a", "b", "c"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := unionStringSlices(tt.parent, tt.child) + assert.Equal(t, tt.expected, result) + }) + } +} + +func TestMergeWithParent(t *testing.T) { + cases := []struct { + name string + parent *SandboxPolicy + child *SandboxPolicy + assert func(t *testing.T, parent, child *SandboxPolicy) + }{ + { + name: "merge when all fields are present", + parent: &SandboxPolicy{ + Name: "parent", + Description: "Parent policy", + PackageManagers: []string{"npm", "yarn"}, + AllowPTY: utils.PtrTo(true), + AllowGitConfig: utils.PtrTo(false), + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/usr/**", "/home/**"}, + AllowWrite: []string{"/tmp/**"}, + DenyRead: []string{"/etc/shadow"}, + DenyWrite: []string{"/etc/**"}, + }, + Network: NetworkPolicy{ + AllowOutbound: []string{"registry.npmjs.org:443"}, + DenyOutbound: []string{"*:*"}, + }, + Process: ProcessPolicy{ + AllowExec: []string{"/usr/bin/node"}, + DenyExec: []string{"/usr/bin/curl"}, + }, + }, + child: &SandboxPolicy{ + Name: "child", + Description: "Child policy", + PackageManagers: []string{"npx"}, + AllowPTY: utils.PtrTo(false), + AllowGitConfig: utils.PtrTo(true), + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/var/**"}, + AllowWrite: []string{"/home/**"}, + DenyRead: []string{}, + DenyWrite: []string{"/usr/**"}, + }, + Network: NetworkPolicy{ + AllowOutbound: []string{"github.com:443"}, + DenyOutbound: []string{}, + }, + Process: ProcessPolicy{ + AllowExec: []string{"/usr/bin/git"}, + DenyExec: []string{}, + }, + }, + assert: func(t *testing.T, parent, child *SandboxPolicy) { + // Test that name and description are preserved from child + assert.Equal(t, "child", child.Name) + assert.Equal(t, "Child policy", child.Description) + + // Test that package managers are replaced (not merged) + assert.Equal(t, []string{"npx"}, child.PackageManagers) + + // Test that boolean flags are overridden + assert.False(t, *child.AllowPTY) + assert.True(t, *child.AllowGitConfig) + + // Test filesystem lists are unioned + assert.ElementsMatch(t, []string{"/usr/**", "/home/**", "/var/**"}, child.Filesystem.AllowRead) + assert.ElementsMatch(t, []string{"/tmp/**", "/home/**"}, child.Filesystem.AllowWrite) + assert.ElementsMatch(t, []string{"/etc/shadow"}, child.Filesystem.DenyRead) + assert.ElementsMatch(t, []string{"/etc/**", "/usr/**"}, child.Filesystem.DenyWrite) + + // Test network lists are unioned + assert.ElementsMatch(t, []string{"registry.npmjs.org:443", "github.com:443"}, child.Network.AllowOutbound) + assert.ElementsMatch(t, []string{"*:*"}, child.Network.DenyOutbound) + + // Test process lists are unioned + assert.ElementsMatch(t, []string{"/usr/bin/node", "/usr/bin/git"}, child.Process.AllowExec) + assert.ElementsMatch(t, []string{"/usr/bin/curl"}, child.Process.DenyExec) + }, + }, + { + name: "merge when child has no boolean fields", + parent: &SandboxPolicy{ + Name: "parent", + Description: "Parent policy", + PackageManagers: []string{"npm", "yarn"}, + AllowPTY: utils.PtrTo(true), + AllowGitConfig: utils.PtrTo(false), + }, + child: &SandboxPolicy{ + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/usr/**"}, + }, + }, + assert: func(t *testing.T, parent, child *SandboxPolicy) { + // Child inherits boolean fields from parent if not present in child + assert.True(t, *child.AllowPTY) + assert.False(t, *child.AllowGitConfig) + + // Still invalid because child has no name and package managers + assert.Error(t, child.Validate()) + assert.Error(t, child.ValidateResolved()) + }, + }, + } + + for _, tt := range cases { + t.Run(tt.name, func(t *testing.T) { + tt.child.MergeWithParent(tt.parent) + tt.assert(t, tt.parent, tt.child) + }) + } +} + +func TestValidate(t *testing.T) { + tests := []struct { + name string + policy *SandboxPolicy + expectErr bool + }{ + { + name: "valid policy without inheritance", + policy: &SandboxPolicy{ + Name: "test", + PackageManagers: []string{"npm"}, + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/usr/**"}, + }, + }, + expectErr: false, + }, + { + name: "missing name", + policy: &SandboxPolicy{ + PackageManagers: []string{"npm"}, + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/usr/**"}, + }, + }, + expectErr: true, + }, + { + name: "missing package managers", + policy: &SandboxPolicy{ + Name: "test", + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/usr/**"}, + }, + }, + expectErr: true, + }, + { + name: "policy with inherits but no rules is valid", + policy: &SandboxPolicy{ + Name: "test", + Inherits: "parent", + PackageManagers: []string{"npm"}, + }, + expectErr: false, // Basic validation allows this + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.policy.Validate() + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestValidateResolved(t *testing.T) { + tests := []struct { + name string + policy *SandboxPolicy + expectErr bool + }{ + { + name: "valid resolved policy", + policy: &SandboxPolicy{ + Name: "test", + PackageManagers: []string{"npm"}, + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/usr/**"}, + }, + }, + expectErr: false, + }, + { + name: "resolved policy with no rules", + policy: &SandboxPolicy{ + Name: "test", + PackageManagers: []string{"npm"}, + }, + expectErr: true, // Should fail because no rules after resolution + }, + { + name: "missing name in resolved policy", + policy: &SandboxPolicy{ + PackageManagers: []string{"npm"}, + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/usr/**"}, + }, + }, + expectErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := tt.policy.ValidateResolved() + if tt.expectErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + }) + } +} diff --git a/sandbox/profiles/npx.yml b/sandbox/profiles/npx.yml new file mode 100644 index 0000000..8c110ef --- /dev/null +++ b/sandbox/profiles/npx.yml @@ -0,0 +1,24 @@ +name: npx +description: Profile for npx/pnpx generators with write access to current directory +inherits: npm-restrictive + +package_managers: + - npx + - pnpx + +# npx and pnpx require PTY access to work, especially for commands like: +# npx create-next-app@latest +# +# Explicitly setting this so that it is enabled even if we decide to turn this off +# in parent profiles in the future. +allow_pty: true + +filesystem: + # Add write permissions for common generator outputs + allow_write: + - ${CWD}/** + + # Additional deny rules for extra security + deny_write: + - ${CWD}/.env + - ${CWD}/.env.* diff --git a/sandbox/registry.go b/sandbox/registry.go index 8592c71..06c6bc7 100644 --- a/sandbox/registry.go +++ b/sandbox/registry.go @@ -32,12 +32,14 @@ func newDefaultProfileRegistry() (*defaultProfileRegistry, error) { } // loadBuiltinProfiles loads all built-in YAML profiles from the embedded filesystem. +// Inheritance is resolved in a second pass after all profiles are loaded. func (r *defaultProfileRegistry) loadBuiltinProfiles() error { entries, err := profilesFS.ReadDir("profiles") if err != nil { return fmt.Errorf("failed to read profiles directory: %w", err) } + // First pass: load all profiles without resolving inheritance for _, entry := range entries { if entry.IsDir() || !strings.HasSuffix(entry.Name(), ".yml") { continue @@ -54,6 +56,7 @@ func (r *defaultProfileRegistry) loadBuiltinProfiles() error { return fmt.Errorf("failed to parse profile %s: %w", entry.Name(), err) } + // Basic validation (without inheritance resolution) if err := policy.Validate(); err != nil { return fmt.Errorf("invalid profile %s: %w", entry.Name(), err) } @@ -63,6 +66,51 @@ func (r *defaultProfileRegistry) loadBuiltinProfiles() error { r.mu.Unlock() } + // Second pass: resolve inheritance and validate + r.mu.Lock() + defer r.mu.Unlock() + + for name, policy := range r.profiles { + if policy.Inherits != "" { + if err := r.resolveInheritance(policy); err != nil { + return fmt.Errorf("failed to resolve inheritance for profile %s: %w", name, err) + } + + // Validate after inheritance resolution + if err := policy.ValidateResolved(); err != nil { + return fmt.Errorf("invalid profile %s after inheritance: %w", name, err) + } + } + } + + return nil +} + +// resolveInheritance resolves the inheritance chain for a policy. +// This function is called during registry initialization and modifies the policy in place. +// Assumes registry mutex is already held. +func (r *defaultProfileRegistry) resolveInheritance(child *SandboxPolicy) error { + if child.Inherits == "" { + return nil + } + + // Look up parent profile (must be a built-in profile) + parent, exists := r.profiles[child.Inherits] + if !exists { + return fmt.Errorf("parent profile '%s' not found (only built-in profiles can be inherited)", child.Inherits) + } + + // Prevent inheritance chains (parent must not itself inherit) + if parent.Inherits != "" { + return fmt.Errorf("inheritance chains not allowed: parent profile '%s' inherits from '%s'", parent.Name, parent.Inherits) + } + + // Merge parent into child + child.MergeWithParent(parent) + + // Clear the inherits field after resolution to indicate it's been processed + child.Inherits = "" + return nil } @@ -83,6 +131,7 @@ func (r *defaultProfileRegistry) GetProfile(name string) (*SandboxPolicy, error) } // LoadCustomProfile loads a policy from a custom YAML file path. +// Inheritance is resolved if the profile inherits from a built-in profile. func (r *defaultProfileRegistry) LoadCustomProfile(path string) (*SandboxPolicy, error) { data, err := os.ReadFile(path) if err != nil { @@ -94,10 +143,36 @@ func (r *defaultProfileRegistry) LoadCustomProfile(path string) (*SandboxPolicy, return nil, fmt.Errorf("failed to parse custom profile %s: %w", path, err) } + // Basic validation if err := policy.Validate(); err != nil { return nil, fmt.Errorf("invalid custom profile %s: %w", path, err) } + // Resolve inheritance if present + if policy.Inherits != "" { + r.mu.RLock() + parent, exists := r.profiles[policy.Inherits] + r.mu.RUnlock() + + if !exists { + return nil, fmt.Errorf("custom profile %s inherits from unknown profile '%s' (only built-in profiles can be inherited)", path, policy.Inherits) + } + + // Prevent inheritance chains + if parent.Inherits != "" { + return nil, fmt.Errorf("custom profile %s: parent profile '%s' inherits from '%s' (chains not allowed)", path, parent.Name, parent.Inherits) + } + + // Merge parent into child + policy.MergeWithParent(parent) + policy.Inherits = "" + } + + // Validate after inheritance resolution + if err := policy.ValidateResolved(); err != nil { + return nil, fmt.Errorf("invalid custom profile %s after inheritance: %w", path, err) + } + r.mu.Lock() r.profiles[path] = policy r.mu.Unlock() diff --git a/sandbox/registry_test.go b/sandbox/registry_test.go index 4939070..863b243 100644 --- a/sandbox/registry_test.go +++ b/sandbox/registry_test.go @@ -90,3 +90,208 @@ func TestLoadCustomProfile(t *testing.T) { }) } } + +func TestBuiltinProfileInheritance(t *testing.T) { + registry, err := newDefaultProfileRegistry() + assert.NoError(t, err) + assert.NotNil(t, registry) + + // Get the npx profile which inherits from npm-restrictive + npxProfile, err := registry.GetProfile("npx") + assert.NoError(t, err) + assert.NotNil(t, npxProfile) + + // Verify the inherits field is cleared after resolution + assert.Empty(t, npxProfile.Inherits) + + // Verify package managers are from child (not parent) + assert.ElementsMatch(t, []string{"npx", "pnpx"}, npxProfile.PackageManagers) + + // Get the parent profile for comparison + npmRestrictive, err := registry.GetProfile("npm-restrictive") + assert.NoError(t, err) + assert.NotNil(t, npmRestrictive) + + // Verify that child has parent's rules + // Check that some parent rules are present + assert.Contains(t, npxProfile.Filesystem.AllowRead, "/") + assert.Contains(t, npxProfile.Filesystem.AllowRead, "/usr/**") + assert.Contains(t, npxProfile.Filesystem.AllowWrite, "/tmp/**") + + // Verify that child has its own rules + assert.Contains(t, npxProfile.Filesystem.AllowWrite, "${CWD}/**") + assert.Contains(t, npxProfile.Filesystem.DenyWrite, "${CWD}/.env") +} + +func TestLoadCustomProfileWithInheritance(t *testing.T) { + tests := []struct { + name string + policy *SandboxPolicy + assert func(t *testing.T, policy *SandboxPolicy, err error) + }{ + { + name: "custom profile inherits from npm-restrictive", + policy: &SandboxPolicy{ + Name: "custom-npm", + Description: "Custom npm policy", + Inherits: "npm-restrictive", + PackageManagers: []string{"npm"}, + Filesystem: FilesystemPolicy{ + AllowWrite: []string{"/custom/path/**"}, + }, + }, + assert: func(t *testing.T, policy *SandboxPolicy, err error) { + assert.NoError(t, err) + assert.NotNil(t, policy) + + // Verify inheritance was resolved + assert.Empty(t, policy.Inherits) + + // Verify parent rules are present + assert.Contains(t, policy.Filesystem.AllowRead, "/") + assert.Contains(t, policy.Filesystem.AllowRead, "/usr/**") + + // Verify child rules are added + assert.Contains(t, policy.Filesystem.AllowWrite, "/custom/path/**") + assert.Contains(t, policy.Filesystem.AllowWrite, "/tmp/**") + }, + }, + { + name: "custom profile inherits from already-resolved profile (npx)", + policy: &SandboxPolicy{ + Name: "custom-chain", + Description: "Inherits from npx which already resolved its inheritance", + Inherits: "npx", + PackageManagers: []string{"npm"}, + Filesystem: FilesystemPolicy{ + AllowWrite: []string{"/custom/**"}, + }, + }, + assert: func(t *testing.T, policy *SandboxPolicy, err error) { + // This should succeed because npx has already resolved its inheritance + // and no longer has an Inherits field set + assert.NoError(t, err) + assert.NotNil(t, policy) + assert.Empty(t, policy.Inherits) + }, + }, + { + name: "custom profile inherits from non-existent profile", + policy: &SandboxPolicy{ + Name: "custom-bad", + Description: "Should fail", + Inherits: "does-not-exist", + PackageManagers: []string{"npm"}, + }, + assert: func(t *testing.T, policy *SandboxPolicy, err error) { + // This should fail because parent doesn't exist + assert.Error(t, err) + assert.Nil(t, policy) + assert.Contains(t, err.Error(), "inherits from unknown profile") + }, + }, + { + name: "custom profile with multiple inherited and own rules", + policy: &SandboxPolicy{ + Name: "custom-extended", + Description: "Extended npm profile", + Inherits: "npm-restrictive", + PackageManagers: []string{"npm", "yarn"}, + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/opt/**"}, + AllowWrite: []string{"/data/**", "/cache/**"}, + DenyWrite: []string{"/data/secrets/**"}, + }, + Network: NetworkPolicy{ + AllowOutbound: []string{"custom.registry.io:443"}, + }, + }, + assert: func(t *testing.T, policy *SandboxPolicy, err error) { + assert.NoError(t, err) + assert.NotNil(t, policy) + + // Verify inheritance resolved + assert.Empty(t, policy.Inherits) + + // Verify parent and child filesystem rules are merged + assert.Contains(t, policy.Filesystem.AllowRead, "/") + assert.Contains(t, policy.Filesystem.AllowRead, "/usr/**") + assert.Contains(t, policy.Filesystem.AllowRead, "/opt/**") + assert.Contains(t, policy.Filesystem.AllowWrite, "/tmp/**") + assert.Contains(t, policy.Filesystem.AllowWrite, "/data/**") + assert.Contains(t, policy.Filesystem.AllowWrite, "/cache/**") + assert.Contains(t, policy.Filesystem.DenyWrite, "/etc/**") + assert.Contains(t, policy.Filesystem.DenyWrite, "/data/secrets/**") + + // Verify network rules are merged + assert.Contains(t, policy.Network.AllowOutbound, "registry.npmjs.org:443") + assert.Contains(t, policy.Network.AllowOutbound, "custom.registry.io:443") + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + registry, err := newDefaultProfileRegistry() + assert.NoError(t, err) + assert.NotNil(t, registry) + + tempFile, err := os.CreateTemp(t.TempDir(), "custom-policy-*.yml") + assert.NoError(t, err) + defer func() { + assert.NoError(t, tempFile.Close()) + }() + + err = yaml.NewEncoder(tempFile).Encode(tt.policy) + assert.NoError(t, err) + + loadedPolicy, err := registry.LoadCustomProfile(tempFile.Name()) + tt.assert(t, loadedPolicy, err) + }) + } +} + +func TestResolveInheritance(t *testing.T) { + registry, err := newDefaultProfileRegistry() + assert.NoError(t, err) + assert.NotNil(t, registry) + + parent := &SandboxPolicy{ + Name: "test-parent", + PackageManagers: []string{"npm"}, + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/parent/**"}, + AllowWrite: []string{"/parent/write/**"}, + }, + } + + child := &SandboxPolicy{ + Name: "test-child", + Inherits: "test-parent", + PackageManagers: []string{"npx"}, + Filesystem: FilesystemPolicy{ + AllowRead: []string{"/child/**"}, + AllowWrite: []string{"/child/write/**"}, + }, + } + + // Add parent to registry + registry.mu.Lock() + registry.profiles["test-parent"] = parent + registry.mu.Unlock() + + // Resolve inheritance + registry.mu.Lock() + err = registry.resolveInheritance(child) + registry.mu.Unlock() + + assert.NoError(t, err) + assert.Empty(t, child.Inherits) + + // Verify lists are merged + assert.ElementsMatch(t, []string{"/parent/**", "/child/**"}, child.Filesystem.AllowRead) + assert.ElementsMatch(t, []string{"/parent/write/**", "/child/write/**"}, child.Filesystem.AllowWrite) + + // Verify package managers are from child + assert.Equal(t, []string{"npx"}, child.PackageManagers) +}