fix(local-runtime): stop inheriting daemon environment in plugin subprocesses - #799
Open
SashaMIT wants to merge 1 commit into
Open
fix(local-runtime): stop inheriting daemon environment in plugin subprocesses#799SashaMIT wants to merge 1 commit into
SashaMIT wants to merge 1 commit into
Conversation
…rocesses getInstanceCmd built plugin processes with cmd.Environ(), copying the daemon's full environment (DB_PASSWORD, SERVER_KEY, DIFY_INNER_API_KEY, Redis and cloud storage credentials) into every plugin subprocess, where any installed plugin could read and exfiltrate it over the network. Replace inheritance with an explicit allowlist builder, BuildPluginCommandEnv, mirroring the existing buildUVCommandEnv pattern used for the uv installer child process. The allowlist passes through what plugins legitimately need (PATH, HOME, locale variables, temp directories, TZ, CA bundle and proxy variables), daemon config proxy settings take precedence over inherited ones, and INSTALL_METHOD=local is set as before. The slim CLI local mode used the same os.Environ() pattern for marketplace-downloaded plugins and now shares the builder.
dosubot
Bot
added
size:M
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
1 participant
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.
Plugin subprocesses no longer receive a copy of the daemon's own environment variables.
What
getInstanceCmdbuilt plugin processes withcmd.Environ(), which copies the daemon process's entire environment into every plugin subprocess. In every real deployment that environment carries the daemon's full credential set:DB_PASSWORD/DB_USERNAME(direct Postgres access to all tenant plugin data),SERVER_KEY(the shared key protecting the daemon's management and dispatch routes),DIFY_INNER_API_KEY(the daemon's identity for Dify's internal API), Redis and cloud storage credentials,ADMIN_API_KEY, and anything else the operator places in the container env. Any installed plugin could read all of it fromos.environand exfiltrate it over the network, since plugins make arbitrary outbound calls by design. The slim CLI's local mode had the same pattern (os.Environ()), running marketplace-downloaded plugin code with the developer's full shell environment.Why
The local runtime's design (separate process per plugin, dedicated working directory, stdio-only IPC, heartbeat watchdog) exists to run third-party marketplace code at reduced trust. Inheriting the daemon environment defeats the confidentiality half of that isolation, and plugins need none of these variables to function: daemon to plugin traffic is stdio-framed, and plugin configuration arrives per-invocation rather than through the environment. The repo already recognized this pattern for the short-lived
uvinstaller child process (buildUVCommandEnv, with a test assertingUNRELATED_SECRETis not inherited); the long-lived plugin process was missed.How
Replace inheritance with an explicit allowlist builder,
BuildPluginCommandEnv, mirroringbuildUVCommandEnv. It passes through what plugins legitimately need:PATH,HOME, locale variables (LANG,LC_ALL,LC_CTYPE), temp directories (TMPDIR,TEMP,TMP),TZ, custom CA bundles (SSL_CERT_FILE,REQUESTS_CA_BUNDLE), and proxy variables in both cases, with the daemon config's proxy settings (HTTP_PROXY/HTTPS_PROXY/NO_PROXY, themselves loaded from env or config) taking precedence.INSTALL_METHOD=localis set exactly as before. Everything else, including all*_KEY/*_SECRET/*_PASSWORD/*_TOKENstyle credentials, never reaches plugin code. The slim CLI local mode now uses the same builder.How verified
internal/core/local_runtime/subprocess_test.go:TestBuildPluginCommandEnvasserts allowlisted variables pass through, config proxy settings win over inherited ones, andDB_PASSWORD/SERVER_KEY/DIFY_INNER_API_KEY/AWS_*/REDIS_PASSWORD/ADMIN_API_KEYare absent.TestGetInstanceCmdDoesNotInheritDaemonEnvbuilds the real plugin command against a fake venv and asserts the same oncmd.Env, proving the spawn site is wired to the allowlist.go buildpasses for the touched packages, andgo test ./internal/core/local_runtime/... ./pkg/slim/...passes in full. (A whole-repogo build ./...additionally requires the gitignoredpkg/license/private_key/PRIVATE_KEY.pem, unrelated to this change.)Same disclosure class as our merged #796: a one-line omission visible in the public source, fixed directly via public PR.
Made with Cursor