refactor: Misc cleanup

This commit is contained in:
Abhisek Datta
2026-01-15 08:44:26 +05:30
parent dce5220241
commit 5098649c72
3 changed files with 23 additions and 12 deletions
+1 -1
View File
@@ -19,7 +19,7 @@ See [example](https://safedep.io/malicious-npm-package-express-cookie-parser/)
- Maintains package installation event log for transparency and audit trail - Maintains package installation event log for transparency and audit trail
- Enforces least privilege and defense in depth using OS native sandboxing - Enforces least privilege and defense in depth using OS native sandboxing
PMG guarantees it's own artifact integrity using GitHub and npm attestations. Users can cryptographically prove that the binary they run PMG guarantees its own artifact integrity using GitHub and npm attestations. Users can cryptographically prove that the binary they run
matches the source code they reviewed, eliminating the risk of tampered or malicious builds. See [why and how to trust PMG](docs/trust.md). matches the source code they reviewed, eliminating the risk of tampered or malicious builds. See [why and how to trust PMG](docs/trust.md).
## PMG in Action ## PMG in Action
@@ -85,6 +85,9 @@ type seccompConfig struct {
// These defaults are based on: // These defaults are based on:
// - Common Linux filesystem layouts // - Common Linux filesystem layouts
// - Anthropic Sandbox Runtime implementation patterns // - Anthropic Sandbox Runtime implementation patterns
//
// This config is for maintaining safe defaults for the sandbox. Future enhancements
// will allow the user to override the config through policy or sandbox config available at PMG level.
func newDefaultBubblewrapConfig() *bubblewrapConfig { func newDefaultBubblewrapConfig() *bubblewrapConfig {
return &bubblewrapConfig{ return &bubblewrapConfig{
// Essential system paths (read-only) // Essential system paths (read-only)
@@ -195,6 +198,7 @@ func (c *bubblewrapConfig) getEssentialSystemPaths() []string {
existingPaths = append(existingPaths, path) existingPaths = append(existingPaths, path)
} }
} }
return existingPaths return existingPaths
} }
@@ -207,5 +211,6 @@ func (c *bubblewrapConfig) getEssentialDevices() []string {
existingDevices = append(existingDevices, device) existingDevices = append(existingDevices, device)
} }
} }
return existingDevices return existingDevices
} }
+17 -11
View File
@@ -155,6 +155,7 @@ func (t *bubblewrapPolicyTranslator) addIsolationNamespaces(policy *sandbox.Sand
// - --ro-bind: Read-only bind mount // - --ro-bind: Read-only bind mount
// - --bind: Read-write bind mount // - --bind: Read-write bind mount
// - --dev-bind: Device file bind mount // - --dev-bind: Device file bind mount
// - --tmpfs: Temporary file system mount (used to hide specific files/directories)
// - Paths not mounted are inaccessible (deny-by-default) // - Paths not mounted are inaccessible (deny-by-default)
// //
// Strategy: // Strategy:
@@ -168,18 +169,20 @@ func (t *bubblewrapPolicyTranslator) addIsolationNamespaces(policy *sandbox.Sand
func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPolicy) ([]string, error) { func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.SandboxPolicy) ([]string, error) {
args := []string{} args := []string{}
// Track paths we've already bound to avoid duplicates // Track paths we've already bound for read and write to avoid duplicates
boundPaths := make(map[string]bool) // Bubblewrap later mounts win, so we need to track both read and write bound paths.
readBoundPaths := make(map[string]bool)
writeBoundPaths := make(map[string]bool)
// Add essential system paths to bound paths (already handled separately) // Add essential system paths to bound paths (already handled separately)
for _, path := range t.config.getEssentialSystemPaths() { for _, path := range t.config.getEssentialSystemPaths() {
boundPaths[path] = true readBoundPaths[path] = true
} }
// Mark tmpdir as already bound (will be handled by addTmpdirSupport()) // Mark tmpdir as already bound (will be handled by addTmpdirSupport())
// This prevents conflicts from policy patterns like /tmp/** // This prevents conflicts from policy patterns like /tmp/**
tmpDir := os.TempDir() tmpDir := os.TempDir()
boundPaths[tmpDir] = true writeBoundPaths[tmpDir] = true
// 1. Process allow_read rules FIRST (read-only bind mounts) // 1. Process allow_read rules FIRST (read-only bind mounts)
// This establishes the base read-only filesystem view (including "/" if specified) // This establishes the base read-only filesystem view (including "/" if specified)
@@ -190,18 +193,19 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
continue continue
} }
readArgs, err := t.processReadRule(expanded, boundPaths) // Glob chars are handled by the processReadRule function.
readArgs, err := t.processReadRule(expanded, readBoundPaths)
if err != nil { if err != nil {
log.Warnf("Failed to process allow_read rule '%s': %v", expanded, err) log.Warnf("Failed to process allow_read rule '%s': %v", expanded, err)
continue continue
} }
args = append(args, readArgs...) args = append(args, readArgs...)
} }
// 2. Process allow_write rules SECOND (read-write bind mounts) // 2. Process allow_write rules SECOND (read-write bind mounts)
// These OVERRIDE earlier read-only binds (bwrap: later mounts win) // These OVERRIDE earlier read-only binds (bwrap: later mounts win)
// Use a separate map so we don't skip paths that need write access // Use a separate map so we don't skip paths that need write access
writeBoundPaths := make(map[string]bool)
writeBoundPaths[tmpDir] = true // tmpdir handled by addTmpdirSupport writeBoundPaths[tmpDir] = true // tmpdir handled by addTmpdirSupport
for _, pattern := range policy.Filesystem.AllowWrite { for _, pattern := range policy.Filesystem.AllowWrite {
expanded, err := util.ExpandVariables(pattern) expanded, err := util.ExpandVariables(pattern)
@@ -210,11 +214,13 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
continue continue
} }
// Glob chars are handled by the processWriteRule function.
writeArgs, err := t.processWriteRule(expanded, writeBoundPaths) writeArgs, err := t.processWriteRule(expanded, writeBoundPaths)
if err != nil { if err != nil {
log.Warnf("Failed to process allow_write rule '%s': %v", expanded, err) log.Warnf("Failed to process allow_write rule '%s': %v", expanded, err)
continue continue
} }
args = append(args, writeArgs...) args = append(args, writeArgs...)
} }
@@ -242,16 +248,16 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
denyArgs, err := t.processDenyRule(expanded, allowedWritePaths) denyArgs, err := t.processDenyRule(expanded, allowedWritePaths)
if err != nil { if err != nil {
// Deny rules failing is not critical (file may not exist yet)
log.Debugf("Deny rule '%s' skipped: %v", expanded, err) log.Debugf("Deny rule '%s' skipped: %v", expanded, err)
continue continue
} }
args = append(args, denyArgs...) args = append(args, denyArgs...)
} }
// 4. Process mandatory credential directories - completely hide them with tmpfs // 4. Process mandatory credential directories - completely hide them with tmpfs
// This blocks both read AND write access (more secure than read-only mount) // This blocks both read AND write access (more secure than read-only mount)
hiddenDirs := make(map[string]bool) // Track to avoid duplicates hiddenDirs := make(map[string]bool)
for _, pattern := range mandatoryDenies { for _, pattern := range mandatoryDenies {
expanded, err := util.ExpandVariables(pattern) expanded, err := util.ExpandVariables(pattern)
if err != nil { if err != nil {
@@ -260,11 +266,11 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
var dirsToHide []string var dirsToHide []string
if util.ContainsGlob(expanded) { if util.ContainsGlob(expanded) {
// Expand glob pattern to find matching directories
matches, err := filepath.Glob(expanded) matches, err := filepath.Glob(expanded)
if err != nil { if err != nil {
continue continue
} }
dirsToHide = matches dirsToHide = matches
} else { } else {
dirsToHide = []string{expanded} dirsToHide = []string{expanded}
@@ -274,9 +280,11 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
if hiddenDirs[dir] { if hiddenDirs[dir] {
continue continue
} }
if info, err := os.Stat(dir); err == nil && info.IsDir() { if info, err := os.Stat(dir); err == nil && info.IsDir() {
args = append(args, "--tmpfs", dir) args = append(args, "--tmpfs", dir)
hiddenDirs[dir] = true hiddenDirs[dir] = true
log.Debugf("Hiding credential directory '%s' with tmpfs", dir) log.Debugf("Hiding credential directory '%s' with tmpfs", dir)
} }
} }
@@ -304,7 +312,6 @@ func (t *bubblewrapPolicyTranslator) translateFilesystem(policy *sandbox.Sandbox
} }
} }
} else { } else {
// Literal path
if info, err := os.Stat(expanded); err == nil && !info.IsDir() { if info, err := os.Stat(expanded); err == nil && !info.IsDir() {
args = append(args, "--ro-bind", "/dev/null", expanded) args = append(args, "--ro-bind", "/dev/null", expanded)
log.Debugf("Blocked execution of '%s'", expanded) log.Debugf("Blocked execution of '%s'", expanded)
@@ -709,7 +716,6 @@ func (t *bubblewrapPolicyTranslator) addPTYSupport() []string {
// Package managers need writable temp space for downloads, extraction, etc. // Package managers need writable temp space for downloads, extraction, etc.
func (t *bubblewrapPolicyTranslator) addTmpdirSupport() []string { func (t *bubblewrapPolicyTranslator) addTmpdirSupport() []string {
args := []string{} args := []string{}
tmpDir := os.TempDir() tmpDir := os.TempDir()
// Bind tmp directory as writable // Bind tmp directory as writable