fix(security): scope plugin subprocess env to an allow-list by default#7
Merged
Merged
Conversation
Manager.Start ran every plugin with the full parent environment inherited via os.Environ(), so a plugin could read any ambient secret enowX's own process happened to have in its environment. Default to a minimal allow-list (PATH/HOME/USERPROFILE/TMPDIR/TEMP/TMP/SystemRoot); a plugin that declares "env:full" in its manifest keeps today's full inheritance.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
os.Environ()inherited wholesale from enowX's own process, so a plugin could read any ambient secret enowX's process happened to have —Manifest.Permissionsexisted in the manifest struct but was never read by anything."env:full"permission for a plugin that needs the old behavior.Problem
core/plugins/manager.go'sStartbuiltcmd.Envfromappend(os.Environ(), ...)for every plugin runtime (go/python/node/bin), with no restriction.core/plugins/manifest.go'sPermissions []stringfield was parsed fromplugin.jsonbut never checked anywhere — no official plugin and no part of the UI/marketplace sets or surfaces it.Solution
minimalEnv(): a fixed allow-list (PATH,HOME,USERPROFILE,TMPDIR,TEMP,TMP,SystemRoot) built viaos.LookupEnv, so it's correct on every OS without aruntime.GOOSbranch (a var simply isn't present on platforms where it doesn't apply).PATH/HOME/TMPDIRare also exactly what node/python/go need to locate their toolchain, caches, and temp files, so switching to this allow-list doesn't break plugins that only need to run their own runtime.buildEnv(man, id, port, enowxPort): starts fromminimalEnv(), or fromos.Environ()if the manifest lists"env:full"inPermissions— preserving today's behavior for a plugin that explicitly opts in (e.g. one that shells out togit/gcloud/awsrelying on the user's own env). Either way, the plugin's ownPORT/ENOWX_PLUGIN_ID/ENOWX_APIvars are appended, unchanged from before.Changed files
core/plugins/manager.goenvAllowList,minimalEnv,buildEnv;Startnow callsbuildEnvinstead of inliningos.Environ().core/plugins/manager_test.gominimalEnv/buildEnv, covering the default-scoped case and theenv:fullopt-in.core/plugins/manifest.goPermissionsdocumenting the recognized"env:full"value.Known limitation / out of scope
permissionsyet — a plugin author sets it directly inplugin.json.Test plan
go test ./core/plugins/... -run 'TestMinimalEnv|TestBuildEnv' -vpassesgo build ./...andgo test ./...pass with no regressionsgo runplugin subprocesses viaplugins.Managerdirectly (a freshenxinstance can't start any plugin without a cloud entitlement, an unrelated pre-existing gate — seeSetGate/authorizedinmanager.go, only wired up incmd/enowx/main.go): a plugin withoutenv:fullnever sees a canary env var set on the parent process, while PATH is still available so its owngo runstill works; a plugin withenv:fulldoes see the canary — confirming the allow-list is enforced and the opt-in still works end-to-end