diff --git a/config/config.go b/config/config.go index 124b347..8e75197 100644 --- a/config/config.go +++ b/config/config.go @@ -298,6 +298,7 @@ const ( SandboxAllowExec SandboxAllowType = "exec" SandboxAllowNetConnect SandboxAllowType = "net-connect" SandboxAllowNetBind SandboxAllowType = "net-bind" + SandboxAllowEnv SandboxAllowType = "env" ) // SandboxAllowOverride represents a single --sandbox-allow flag value. diff --git a/config/sandbox_allow.go b/config/sandbox_allow.go index 89b4eee..e3c2213 100644 --- a/config/sandbox_allow.go +++ b/config/sandbox_allow.go @@ -16,6 +16,7 @@ var validSandboxAllowTypes = map[SandboxAllowType]bool{ SandboxAllowExec: true, SandboxAllowNetConnect: true, SandboxAllowNetBind: true, + SandboxAllowEnv: true, } // parseSandboxAllowOverrides parses raw --sandbox-allow flag values into validated overrides. @@ -69,7 +70,7 @@ func parseSingleOverride(raw string) (SandboxAllowOverride, error) { } if !validSandboxAllowTypes[allowType] { - return SandboxAllowOverride{}, fmt.Errorf("unknown type %q, valid types: read, write, exec, net-connect, net-bind", typStr) + return SandboxAllowOverride{}, fmt.Errorf("unknown type %q, valid types: read, write, exec, net-connect, net-bind, env", typStr) } resolved, err := validateAndResolveValue(allowType, value) @@ -95,11 +96,26 @@ func validateAndResolveValue(typ SandboxAllowType, value string) (string, error) return validateNetConnect(value) case SandboxAllowNetBind: return validateNetBind(value) + case SandboxAllowEnv: + return validateEnvName(value) default: return "", fmt.Errorf("unhandled type: %s", typ) } } +// validateEnvName validates an env allow value. The value is an environment +// variable name or name glob (e.g. NPM_TOKEN, npm_config_*) and is kept +// verbatim — unlike filesystem/exec values it is NOT path-resolved, since it +// matches a variable name and not a filesystem location. Matching is +// case-insensitive at scrub time, so the value is not normalized here. +func validateEnvName(value string) (string, error) { + if strings.ContainsAny(value, "=/") || strings.ContainsAny(value, " \t") { + return "", fmt.Errorf("invalid env variable name %q (expected a name or name glob, e.g. NPM_TOKEN or npm_config_*)", value) + } + + return value, nil +} + // resolveFilesystemPath resolves a filesystem path for read/write overrides. // Supports glob patterns. Resolves relative paths to absolute via CWD. func resolveFilesystemPath(value string) (string, error) { diff --git a/config/sandbox_allow_test.go b/config/sandbox_allow_test.go index ea9765c..be2ae00 100644 --- a/config/sandbox_allow_test.go +++ b/config/sandbox_allow_test.go @@ -94,6 +94,37 @@ func TestParseSandboxAllowOverrides_ValidFormats(t *testing.T) { } } +func TestParseSandboxAllowOverrides_Env(t *testing.T) { + tests := []struct { + name string + raw string + expectedValue string + }{ + {name: "exact name", raw: "env=NPM_TOKEN", expectedValue: "NPM_TOKEN"}, + {name: "glob name kept verbatim", raw: "env=npm_config_*", expectedValue: "npm_config_*"}, + {name: "not path resolved", raw: "env=AWS_PROFILE", expectedValue: "AWS_PROFILE"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + overrides, err := parseSandboxAllowOverrides([]string{tt.raw}) + require.NoError(t, err) + require.Len(t, overrides, 1) + + assert.Equal(t, SandboxAllowEnv, overrides[0].Type) + // Value is kept verbatim — no CWD/path resolution. + assert.Equal(t, tt.expectedValue, overrides[0].Value) + }) + } +} + +func TestParseSandboxAllowOverrides_EnvInvalid(t *testing.T) { + for _, raw := range []string{"env=NPM/TOKEN", "env=FOO=BAR", "env=HAS SPACE"} { + _, err := parseSandboxAllowOverrides([]string{raw}) + assert.Error(t, err, "expected error for %q", raw) + } +} + func TestParseSandboxAllowOverrides_MultipleValues(t *testing.T) { raw := []string{ "write=./.gitignore", diff --git a/docs/sandbox.md b/docs/sandbox.md index 89aaf86..b8bb6f9 100644 --- a/docs/sandbox.md +++ b/docs/sandbox.md @@ -46,6 +46,46 @@ read=./.env` is enough to read `${CWD}/.env`. Suppression is exact post-expansio like `${HOME}/**` do not opt out of `${HOME}/.aws`. The unnamed absolute form stays denied. `.git/hooks` does not accept opt-outs because hooks can execute arbitrary code. +### Environment Variable Protection + +Many supply chain attacks steal credentials from the **process environment** rather than from files +(e.g. `AWS_SECRET_ACCESS_KEY`, `GITHUB_TOKEN`, `NPM_TOKEN`, `TWINE_PASSWORD`). When the sandbox is +enabled, PMG scrubs a default-deny list of credential-bearing variables from the package manager +child process before it is spawned. The built-in list is an explicit, curated set of **known** secret +names — see [`DANGEROUS_ENV_VARS`](../sandbox/util/dangerous.go). There are deliberately no generic +`*_TOKEN` / `*_SECRET` catch-alls in the default, because broad wildcards there would risk clipping +legitimate build variables. + +Scrubbing is platform-independent (it filters the environment regardless of the OS sandbox driver) +and runs as the last step before launch, after project overlays and `--sandbox-allow` overrides are +merged. Scrubbed variable **names** (never values) are written to the event log so you can see what +was removed. + +Each profile re-allows the variables its ecosystem legitimately needs via an `environment.allow` +block, so package managers keep working: + +```yaml +environment: + # Re-permit only what this ecosystem needs; everything else in the default + # deny list stays scrubbed. allow always wins over deny. + allow: + - NPM_TOKEN + - npm_config_* + # Optionally scrub more than the default. Glob patterns are supported here + # (they are intentionally not in the built-in default). + deny: + - MY_CUSTOM_SECRET + - "*_TOKEN" +``` + +**Accepted trade-off**: a profile re-allows its own ecosystem's publishing token, so a malicious +package executed during `npm install` can read `NPM_TOKEN` — but not a PyPI token, AWS key, or other +cloud/secret-manager credential, which stay scrubbed. The reverse holds for the PyPI profile. This is +deliberate: the package manager needs its own auth token to function. + +Matching is on the variable name, case-insensitive, and supports the same glob syntax as filesystem +rules. A small set of core variables (`PATH`, `HOME`, `LC_*`, `TZ`, ...) is never scrubbed. + ## Requirements - Linux kernel 5.13+ with Landlock enabled (default, no external dependencies) @@ -162,6 +202,9 @@ pmg --sandbox-allow net-connect=npm.internal.corp:443 npm install @corp/private- # Allow a dev server to bind to a local port pmg --sandbox-allow net-bind=127.0.0.1:3000 npx some-dev-tool +# Re-allow a sensitive environment variable that the profile scrubs by default +pmg --sandbox-allow env=AWS_PROFILE aws-cdk-using-package install + # Multiple overrides pmg \ --sandbox-allow write=./.gitignore \ @@ -169,7 +212,13 @@ pmg \ npm install some-package ``` -Supported types: `read`, `write`, `exec`, `net-connect`, `net-bind`. +Supported types: `read`, `write`, `exec`, `net-connect`, `net-bind`, `env`. + +For `env`, the value is an environment variable **name** or name glob (e.g. `NPM_TOKEN`, +`npm_config_*`) and is kept verbatim — it is not path-resolved. It is an allow-only override that +re-permits a variable the profile would otherwise scrub; allow always wins, so there is no deny list +to edit. If a command fails with an auth error, check the event log for a "scrubbed" line naming the +variable, then re-allow it with `--sandbox-allow env=NAME`. Overrides are non-persistent (apply to current invocation only) and logged in the event log for auditing. An override adds the path to the allow list and removes an exact match entry from the diff --git a/sandbox/executor/apply.go b/sandbox/executor/apply.go index 2117476..8ba77b5 100644 --- a/sandbox/executor/apply.go +++ b/sandbox/executor/apply.go @@ -6,6 +6,7 @@ import ( "os" "os/exec" "path/filepath" + "strings" "github.com/safedep/dry/log" "github.com/safedep/dry/usefulerror" @@ -15,6 +16,7 @@ import ( "github.com/safedep/pmg/internal/audit" "github.com/safedep/pmg/sandbox" "github.com/safedep/pmg/sandbox/platform" + "github.com/safedep/pmg/sandbox/util" ) type applySandboxConfig struct { @@ -164,6 +166,12 @@ func ApplySandbox(ctx context.Context, cmd *exec.Cmd, pmName string, opts ...app log.Debugf("Running %s in %s sandbox with policy %s", pmName, sb.Name(), policy.Name) + // Scrub sensitive environment variables before the child is spawned. This + // runs after overlay and runtime overrides are merged into the policy so + // user allowances are honored, and is platform-independent (it filters the + // env slice regardless of the OS sandbox driver). + scrubEnv(cmd, policy) + result, err := sb.Execute(ctx, cmd, policy) if err != nil { return nil, fmt.Errorf("failed to setup sandbox: %w", err) @@ -205,10 +213,40 @@ func applyRuntimeOverrides(policy *sandbox.SandboxPolicy, overrides []config.San // Enable AllowNetworkBind so the translator emits bind rules. // Without this, AllowBind entries would be ignored on some platforms. policy.AllowNetworkBind = utils.PtrTo(true) + + case config.SandboxAllowEnv: + // Allow-wins: appending to Allow un-scrubs the variable regardless + // of whether it was denied by the built-in list or a profile deny + // glob, so (unlike the filesystem cases) there is no deny list to + // remove an exact match from. + log.Infof("Sandbox override: allowing environment variable %s", override.Value) + policy.Environment.Allow = append(policy.Environment.Allow, override.Value) } } } +// scrubEnv removes sensitive environment variables from cmd.Env per the +// resolved policy's environment section. It runs after project overlay and +// runtime overrides are merged into the policy, so user allowances take effect. +// A nil cmd.Env would mean "inherit the parent environment", which would defeat +// scrubbing, so it is populated from os.Environ() first. +func scrubEnv(cmd *exec.Cmd, policy *sandbox.SandboxPolicy) { + if cmd.Env == nil { + cmd.Env = os.Environ() + } + + result := util.ScrubEnv(cmd.Env, util.EnvScrubOptions{ + Allow: policy.Environment.Allow, + Deny: policy.Environment.Deny, + }) + cmd.Env = result.Env + + if len(result.Removed) > 0 { + log.Infof("Sandbox: scrubbed %d sensitive environment variable(s) from %s: %s", + len(result.Removed), policy.Name, strings.Join(result.Removed, ", ")) + } +} + // removeExactMatch removes entries from the slice that exactly match the given value. // Glob patterns and wildcards in the slice are never matched. Only literal string // equality is used. This keeps broad deny rules intact while allowing targeted overrides. diff --git a/sandbox/executor/apply_test.go b/sandbox/executor/apply_test.go index f0928f1..8bea86b 100644 --- a/sandbox/executor/apply_test.go +++ b/sandbox/executor/apply_test.go @@ -2,6 +2,7 @@ package executor import ( "os" + "os/exec" "path/filepath" "testing" @@ -57,6 +58,72 @@ func TestApplyRuntimeOverrides_Exec(t *testing.T) { assert.Contains(t, policy.Process.AllowExec, "/usr/bin/curl") } +func TestApplyRuntimeOverrides_Env(t *testing.T) { + policy := &sandbox.SandboxPolicy{ + Environment: sandbox.EnvironmentPolicy{ + Allow: []string{"NPM_TOKEN"}, + }, + } + + applyRuntimeOverrides(policy, []config.SandboxAllowOverride{ + {Type: config.SandboxAllowEnv, Value: "AWS_PROFILE", Raw: "env=AWS_PROFILE"}, + }) + + assert.Contains(t, policy.Environment.Allow, "NPM_TOKEN") + assert.Contains(t, policy.Environment.Allow, "AWS_PROFILE") +} + +func TestScrubEnv_RemovesDeniedKeepsAllowed(t *testing.T) { + policy := &sandbox.SandboxPolicy{ + Name: "test", + Environment: sandbox.EnvironmentPolicy{ + Allow: []string{"NPM_TOKEN"}, + }, + } + + cmd := &exec.Cmd{Env: []string{ + "PATH=/usr/bin", + "NPM_TOKEN=keep-me", + "AWS_SECRET_ACCESS_KEY=scrub-me", + "GITHUB_TOKEN=scrub-me-too", + }} + + scrubEnv(cmd, policy) + + assert.Contains(t, cmd.Env, "PATH=/usr/bin") + assert.Contains(t, cmd.Env, "NPM_TOKEN=keep-me") + assert.NotContains(t, cmd.Env, "AWS_SECRET_ACCESS_KEY=scrub-me") + assert.NotContains(t, cmd.Env, "GITHUB_TOKEN=scrub-me-too") +} + +func TestScrubEnv_AllowOverrideUnscrubs(t *testing.T) { + policy := &sandbox.SandboxPolicy{Name: "test"} + + // Simulate a --sandbox-allow env=AWS_PROFILE override having been merged. + applyRuntimeOverrides(policy, []config.SandboxAllowOverride{ + {Type: config.SandboxAllowEnv, Value: "AWS_SESSION_TOKEN", Raw: "env=AWS_SESSION_TOKEN"}, + }) + + cmd := &exec.Cmd{Env: []string{"AWS_SESSION_TOKEN=kept"}} + scrubEnv(cmd, policy) + + assert.Contains(t, cmd.Env, "AWS_SESSION_TOKEN=kept") +} + +func TestScrubEnv_NilEnvPopulatedThenScrubbed(t *testing.T) { + t.Setenv("GITHUB_TOKEN", "should-be-scrubbed") + t.Setenv("PMG_ENV_SCRUB_MARKER", "kept") + + policy := &sandbox.SandboxPolicy{Name: "test"} + cmd := &exec.Cmd{Env: nil} + + scrubEnv(cmd, policy) + + require.NotNil(t, cmd.Env) + assert.Contains(t, cmd.Env, "PMG_ENV_SCRUB_MARKER=kept") + assert.NotContains(t, cmd.Env, "GITHUB_TOKEN=should-be-scrubbed") +} + func TestApplyRuntimeOverrides_NetConnect(t *testing.T) { policy := &sandbox.SandboxPolicy{ Network: sandbox.NetworkPolicy{ @@ -250,7 +317,6 @@ func TestApplyRuntimeOverrides_VariableDenyNotRemovedByAbsoluteOverride(t *testi assert.Equal(t, []string{"${CWD}/blocked.txt"}, policy.Filesystem.DenyWrite) } - func TestApplyProjectOverlayAppendsEntries(t *testing.T) { dir := t.TempDir() repo := "/repo/example" diff --git a/sandbox/policy.go b/sandbox/policy.go index 7955703..ef0220f 100644 --- a/sandbox/policy.go +++ b/sandbox/policy.go @@ -19,9 +19,10 @@ type SandboxPolicy struct { // 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"` + Filesystem FilesystemPolicy `yaml:"filesystem" json:"filesystem"` + Network NetworkPolicy `yaml:"network" json:"network"` + Process ProcessPolicy `yaml:"process" json:"process"` + Environment EnvironmentPolicy `yaml:"environment" json:"environment"` // 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 @@ -63,6 +64,15 @@ type ProcessPolicy struct { DenyExec []string `yaml:"deny_exec" json:"deny_exec"` } +// EnvironmentPolicy controls which environment variables are scrubbed from the +// child process. Deny extends the built-in util.DANGEROUS_ENV_VARS list; Allow +// suppresses matching denies (allow wins). Patterns are case-insensitive name +// globs. Enforcement happens in sandbox/util.ScrubEnv at process spawn. +type EnvironmentPolicy struct { + Allow []string `yaml:"allow" json:"allow"` + Deny []string `yaml:"deny" json:"deny"` +} + // 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(), @@ -134,6 +144,10 @@ func (child *SandboxPolicy) MergeWithParent(parent *SandboxPolicy) { child.Process.AllowExec = unionStringSlices(parent.Process.AllowExec, child.Process.AllowExec) child.Process.DenyExec = unionStringSlices(parent.Process.DenyExec, child.Process.DenyExec) + // Union environment lists + child.Environment.Allow = unionStringSlices(parent.Environment.Allow, child.Environment.Allow) + child.Environment.Deny = unionStringSlices(parent.Environment.Deny, child.Environment.Deny) + // 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)) diff --git a/sandbox/policy_env_test.go b/sandbox/policy_env_test.go new file mode 100644 index 0000000..f8ae1b3 --- /dev/null +++ b/sandbox/policy_env_test.go @@ -0,0 +1,42 @@ +package sandbox + +import ( + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestMergeWithParent_Environment(t *testing.T) { + parent := &SandboxPolicy{ + Environment: EnvironmentPolicy{ + Allow: []string{"NPM_TOKEN"}, + Deny: []string{"PARENT_SECRET"}, + }, + } + child := &SandboxPolicy{ + Environment: EnvironmentPolicy{ + Allow: []string{"NODE_AUTH_TOKEN"}, + Deny: []string{"CHILD_SECRET"}, + }, + } + + child.MergeWithParent(parent) + + assert.Equal(t, []string{"NPM_TOKEN", "NODE_AUTH_TOKEN"}, child.Environment.Allow) + assert.Equal(t, []string{"PARENT_SECRET", "CHILD_SECRET"}, child.Environment.Deny) +} + +func TestResolveProfile_DeepCopiesEnvironment(t *testing.T) { + r, err := newDefaultProfileRegistry() + assert.NoError(t, err) + + resolved, err := r.ResolveProfile("npm-restrictive", ResolveOptions{}) + assert.NoError(t, err) + + // Mutating the resolved copy must not corrupt the registry-cached policy. + resolved.Environment.Allow = append(resolved.Environment.Allow, "MUTATED") + + again, err := r.ResolveProfile("npm-restrictive", ResolveOptions{}) + assert.NoError(t, err) + assert.NotContains(t, again.Environment.Allow, "MUTATED") +} diff --git a/sandbox/profiles/npm-restrictive.yml b/sandbox/profiles/npm-restrictive.yml index 16cdd83..02fce2f 100644 --- a/sandbox/profiles/npm-restrictive.yml +++ b/sandbox/profiles/npm-restrictive.yml @@ -93,6 +93,22 @@ network: deny_outbound: - "*:*" +environment: + # Credential-bearing variables are scrubbed by default (see the built-in + # DANGEROUS_ENV_VARS list). Re-allow only the variables the npm ecosystem + # legitimately needs for auth, registry config, and TLS. + # + # Accepted trade-off: a malicious JS package executed during install can read + # the npm publishing token below, but NOT PyPI tokens, AWS keys, or other + # cloud/secret-manager credentials, which remain scrubbed. + allow: + - NPM_TOKEN + - NPM_AUTH_TOKEN + - NODE_AUTH_TOKEN + - npm_config_* + - NPM_CONFIG_* + - NODE_EXTRA_CA_CERTS + process: allow_exec: - /usr/bin/node diff --git a/sandbox/profiles/pypi-restrictive.yml b/sandbox/profiles/pypi-restrictive.yml index abdfe22..e3c0ded 100644 --- a/sandbox/profiles/pypi-restrictive.yml +++ b/sandbox/profiles/pypi-restrictive.yml @@ -71,6 +71,22 @@ network: deny_outbound: - "*:*" +environment: + # Credential-bearing variables are scrubbed by default (see the built-in + # DANGEROUS_ENV_VARS list). Re-allow only the variables the PyPI ecosystem + # legitimately needs for index auth, publishing, and TLS. + # + # Accepted trade-off: a malicious Python package executed during install can + # read the PyPI publishing credentials below, but NOT npm tokens, AWS keys, or + # other cloud/secret-manager credentials, which remain scrubbed. + allow: + - TWINE_USERNAME + - TWINE_PASSWORD + - TWINE_REPOSITORY* + - PIP_* + - UV_* + - POETRY_* + process: allow_exec: - /usr/bin/python* diff --git a/sandbox/profiles_env_test.go b/sandbox/profiles_env_test.go new file mode 100644 index 0000000..8bfac63 --- /dev/null +++ b/sandbox/profiles_env_test.go @@ -0,0 +1,61 @@ +package sandbox + +import ( + "testing" + + "github.com/safedep/pmg/sandbox/util" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestProfileEnvContract pins the §2 accepted-risk trade-off as a regression +// test: each ecosystem profile re-allows its own publishing token but keeps +// other ecosystems' and cloud credentials scrubbed. +func TestProfileEnvContract(t *testing.T) { + r, err := newDefaultProfileRegistry() + require.NoError(t, err) + + env := []string{ + "NPM_TOKEN=x", + "NODE_AUTH_TOKEN=x", + "TWINE_PASSWORD=x", + "AWS_SECRET_ACCESS_KEY=x", + "GITHUB_TOKEN=x", + } + + tests := []struct { + profile string + wantKept []string + wantScrubbed []string + }{ + { + profile: "npm-restrictive", + wantKept: []string{"NPM_TOKEN", "NODE_AUTH_TOKEN"}, + wantScrubbed: []string{"TWINE_PASSWORD", "AWS_SECRET_ACCESS_KEY", "GITHUB_TOKEN"}, + }, + { + profile: "pypi-restrictive", + wantKept: []string{"TWINE_PASSWORD"}, + wantScrubbed: []string{"NPM_TOKEN", "NODE_AUTH_TOKEN", "AWS_SECRET_ACCESS_KEY", "GITHUB_TOKEN"}, + }, + } + + for _, tt := range tests { + t.Run(tt.profile, func(t *testing.T) { + policy, err := r.ResolveProfile(tt.profile, ResolveOptions{}) + require.NoError(t, err) + + result := util.ScrubEnv(env, util.EnvScrubOptions{ + Allow: policy.Environment.Allow, + Deny: policy.Environment.Deny, + }) + + for _, name := range tt.wantKept { + assert.Contains(t, result.Env, name+"=x", "%s should keep %s", tt.profile, name) + } + for _, name := range tt.wantScrubbed { + assert.Contains(t, result.Removed, name, "%s should scrub %s", tt.profile, name) + } + }) + } +} diff --git a/sandbox/resolve.go b/sandbox/resolve.go index 665630f..b3e8901 100644 --- a/sandbox/resolve.go +++ b/sandbox/resolve.go @@ -81,6 +81,13 @@ func expandPolicyPaths(p *SandboxPolicy, opts ResolveOptions) (*SandboxPolicy, e AllowBind: append([]string(nil), p.Network.AllowBind...), } + // Environment entries are variable-name globs, not paths, so they are + // deep-copied without expansion so the caller can safely mutate the result. + out.Environment = EnvironmentPolicy{ + Allow: append([]string(nil), p.Environment.Allow...), + Deny: append([]string(nil), p.Environment.Deny...), + } + out.PackageManagers = append([]string(nil), p.PackageManagers...) return &out, nil diff --git a/sandbox/util/dangerous.go b/sandbox/util/dangerous.go index 5d967ca..02598dd 100644 --- a/sandbox/util/dangerous.go +++ b/sandbox/util/dangerous.go @@ -24,6 +24,92 @@ var DANGEROUS_FILES = []string{ ".config/gh", } +// DANGEROUS_ENV_VARS are credential-bearing environment variables scrubbed from +// the child process by default when the sandbox is enabled (see ScrubEnv). This +// is an explicit, curated list of known secret names — there are deliberately +// no generic "*_TOKEN" / "*_SECRET" catch-alls here, because broad wildcards in +// the default would risk clipping legitimate build variables for every user. +// The matcher (ScrubEnv) does support glob patterns, so users who want broader +// coverage opt into it per profile via environment.deny. +// +// Matching is case-insensitive (see ScrubEnv). A package manager's own +// publishing token is intentionally left scrubbable here and re-allowed per +// ecosystem in the profile's environment.allow (e.g. npm re-allows NPM_TOKEN). +var DANGEROUS_ENV_VARS = []string{ + // Cloud providers + "AWS_ACCESS_KEY_ID", + "AWS_SECRET_ACCESS_KEY", + "AWS_SESSION_TOKEN", + "AWS_SECURITY_TOKEN", + "AZURE_CLIENT_SECRET", + "AZURE_CLIENT_ID", + "AZURE_TENANT_ID", + "ARM_CLIENT_SECRET", + "GOOGLE_APPLICATION_CREDENTIALS", + "GCP_SERVICE_ACCOUNT_KEY", + "CLOUDSDK_AUTH_ACCESS_TOKEN", + "DIGITALOCEAN_ACCESS_TOKEN", + + // Package registry / publishing tokens + "NPM_TOKEN", + "NPM_AUTH_TOKEN", + "NODE_AUTH_TOKEN", + "NPM_CONFIG__AUTH", + "TWINE_USERNAME", + "TWINE_PASSWORD", + "PYPI_TOKEN", + "UV_PUBLISH_TOKEN", + "FLIT_PASSWORD", + "POETRY_PYPI_TOKEN_PYPI", + "POETRY_HTTP_BASIC_PYPI_PASSWORD", + "GEM_HOST_API_KEY", + "CARGO_REGISTRY_TOKEN", + + // VCS / CI + "GITHUB_TOKEN", + "GH_TOKEN", + "GH_ENTERPRISE_TOKEN", + "GITLAB_TOKEN", + "CI_JOB_TOKEN", + + // Secrets managers + "VAULT_TOKEN", + + // Misc high-value + "DOCKER_PASSWORD", + "DOCKER_AUTH_CONFIG", + "SNYK_TOKEN", + "CODECOV_TOKEN", + "OPENAI_API_KEY", + "ANTHROPIC_API_KEY", + "HUGGING_FACE_HUB_TOKEN", +} + +// ProtectedEnvVars are core process variables never scrubbed, regardless of +// deny patterns. They are matched case-insensitively as globs (see ScrubEnv), +// so "LC_*" covers the whole locale family. This is a safety net so that a +// profile opting into a broad deny glob (e.g. "*_TOKEN") cannot break process +// startup. The built-in DANGEROUS_ENV_VARS list never touches these names. +var ProtectedEnvVars = []string{ + "PATH", + "HOME", + "USER", + "LOGNAME", + "SHELL", + "PWD", + "OLDPWD", + "TERM", + "TMPDIR", + "TEMP", + "TMP", + "LANG", + "LC_*", + "TZ", + "DISPLAY", + "HOSTNAME", + "NODE_ENV", +} + // MandatoryDenyOptions configures GetMandatoryDenyPatterns. AllowRead and // AllowWrite must be already expanded (post-ExpandVariables); the function // does not call ExpandVariables itself. diff --git a/sandbox/util/env.go b/sandbox/util/env.go new file mode 100644 index 0000000..41843eb --- /dev/null +++ b/sandbox/util/env.go @@ -0,0 +1,108 @@ +package util + +import ( + "regexp" + "strings" + "sync" +) + +// EnvScrubOptions configures ScrubEnv. Allow and Deny are variable-name glob +// patterns sourced from the resolved sandbox policy's environment section +// (already merged with inheritance, project overlay, and --sandbox-allow env= +// overrides by the caller). Deny extends the built-in DANGEROUS_ENV_VARS; +// Allow suppresses any matching deny (allow wins). +type EnvScrubOptions struct { + Allow []string + Deny []string +} + +// EnvScrubResult is the outcome of ScrubEnv. Env holds the kept "KEY=VALUE" +// entries; Removed holds the NAMES (never values) of scrubbed variables, for +// audit logging. +type EnvScrubResult struct { + Env []string + Removed []string +} + +// ScrubEnv removes sensitive variables from env. A variable is removed iff its +// name matches the effective deny set (built-in DANGEROUS_ENV_VARS plus +// opts.Deny) AND does not match opts.Allow AND is not a ProtectedEnvVars entry. +// Matching is on the variable name (left of the first '=') and is +// case-insensitive glob (see GlobToRegex). Removal (not blanking) is +// intentional: absence is the cleanest "not set" signal for downstream tools. +func ScrubEnv(env []string, opts EnvScrubOptions) EnvScrubResult { + deny := make([]string, 0, len(DANGEROUS_ENV_VARS)+len(opts.Deny)) + deny = append(deny, DANGEROUS_ENV_VARS...) + deny = append(deny, opts.Deny...) + + kept := make([]string, 0, len(env)) + var removed []string + + for _, entry := range env { + name := entry + if i := strings.IndexByte(entry, '='); i >= 0 { + name = entry[:i] + } + + if shouldScrubEnvVar(name, deny, opts.Allow) { + removed = append(removed, name) + continue + } + + kept = append(kept, entry) + } + + return EnvScrubResult{Env: kept, Removed: removed} +} + +// shouldScrubEnvVar reports whether a variable named name should be removed. +// Protected variables and allow matches are kept; otherwise a deny match +// scrubs. Allow wins over deny by construction (checked first). +func shouldScrubEnvVar(name string, deny, allow []string) bool { + if matchAnyEnvPattern(name, ProtectedEnvVars) { + return false + } + + if matchAnyEnvPattern(name, allow) { + return false + } + + return matchAnyEnvPattern(name, deny) +} + +func matchAnyEnvPattern(name string, patterns []string) bool { + for _, pattern := range patterns { + if envNameRegex(pattern).MatchString(name) { + return true + } + } + + return false +} + +var ( + envRegexMu sync.Mutex + envRegexCache = map[string]*regexp.Regexp{} +) + +// envNameRegex compiles pattern into a case-insensitive anchored regex for +// matching environment variable names, caching the result. GlobToRegex escapes +// all regex specials, so compilation does not fail in practice; on the +// unexpected error we fall back to a literal case-insensitive name match so a +// deny pattern is never silently dropped. +func envNameRegex(pattern string) *regexp.Regexp { + envRegexMu.Lock() + defer envRegexMu.Unlock() + + if re, ok := envRegexCache[pattern]; ok { + return re + } + + re, err := regexp.Compile("(?i)" + GlobToRegex(pattern)) + if err != nil { + re = regexp.MustCompile("(?i)^" + regexp.QuoteMeta(pattern) + "$") + } + + envRegexCache[pattern] = re + return re +} diff --git a/sandbox/util/env_test.go b/sandbox/util/env_test.go new file mode 100644 index 0000000..a1db245 --- /dev/null +++ b/sandbox/util/env_test.go @@ -0,0 +1,96 @@ +package util + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestScrubEnv(t *testing.T) { + tests := []struct { + name string + env []string + opts EnvScrubOptions + wantKept []string + wantRemoved []string + }{ + { + name: "built-in deny scrubs known secret", + env: []string{"PATH=/usr/bin", "AWS_SECRET_ACCESS_KEY=abc"}, + wantKept: []string{"PATH=/usr/bin"}, + wantRemoved: []string{"AWS_SECRET_ACCESS_KEY"}, + }, + { + name: "case-insensitive match", + env: []string{"aws_secret_access_key=abc"}, + wantKept: []string{}, + wantRemoved: []string{"aws_secret_access_key"}, + }, + { + name: "allow suppresses built-in deny", + env: []string{"NPM_TOKEN=secret"}, + opts: EnvScrubOptions{Allow: []string{"NPM_TOKEN"}}, + wantKept: []string{"NPM_TOKEN=secret"}, + wantRemoved: nil, + }, + { + name: "profile deny glob scrubs", + env: []string{"MY_CUSTOM_TOKEN=x", "OTHER=y"}, + opts: EnvScrubOptions{Deny: []string{"*_TOKEN"}}, + wantKept: []string{"OTHER=y"}, + wantRemoved: []string{"MY_CUSTOM_TOKEN"}, + }, + { + name: "allow glob wins over profile deny glob", + env: []string{"npm_config_registry=x", "npm_config_token=y"}, + opts: EnvScrubOptions{Deny: []string{"*_TOKEN", "npm_config_*"}, Allow: []string{"npm_config_*"}}, + wantKept: []string{"npm_config_registry=x", "npm_config_token=y"}, + wantRemoved: nil, + }, + { + name: "protected essential never scrubbed even under broad deny", + env: []string{"PATH=/usr/bin", "HOME=/home/u", "LC_ALL=en_US.UTF-8"}, + opts: EnvScrubOptions{Deny: []string{"*"}}, + wantKept: []string{"PATH=/usr/bin", "HOME=/home/u", "LC_ALL=en_US.UTF-8"}, + wantRemoved: nil, + }, + { + name: "non-sensitive variables are kept", + env: []string{"FOO=bar", "EDITOR=vim"}, + wantKept: []string{"FOO=bar", "EDITOR=vim"}, + wantRemoved: nil, + }, + { + name: "entry without equals is treated as a name", + env: []string{"GITHUB_TOKEN", "PLAINNAME"}, + wantKept: []string{"PLAINNAME"}, + wantRemoved: []string{"GITHUB_TOKEN"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := ScrubEnv(tt.env, tt.opts) + assert.Equal(t, tt.wantKept, got.Env) + assert.Equal(t, tt.wantRemoved, got.Removed) + }) + } +} + +func TestScrubEnv_RemovesEntirelyNotBlanked(t *testing.T) { + got := ScrubEnv([]string{"GITHUB_TOKEN=secret"}, EnvScrubOptions{}) + + require.Empty(t, got.Env) + assert.NotContains(t, got.Env, "GITHUB_TOKEN=") + assert.Equal(t, []string{"GITHUB_TOKEN"}, got.Removed) +} + +func TestScrubEnv_NoCatchAllsInBuiltinList(t *testing.T) { + // A novel token name must NOT be scrubbed by the default list (catch-alls + // are opt-in per profile, never built in). + got := ScrubEnv([]string{"SOME_RANDOM_TOKEN=x"}, EnvScrubOptions{}) + + assert.Equal(t, []string{"SOME_RANDOM_TOKEN=x"}, got.Env) + assert.Empty(t, got.Removed) +}