fix: resolve passthrough env vars through secret scope under multiplexing - #76199
fix: resolve passthrough env vars through secret scope under multiplexing#76199JonthanaHanh wants to merge 1 commit into
Conversation
…xing Under gateway.multiplex_profiles, terminal/execute_code environments were built from os.environ directly, bypassing the active secret scope. This caused routed profiles to receive the default profile's credentials for any key configured via terminal.env_passthrough. Fix: after building the subprocess env, re-resolve passthrough keys through get_secret() when a profile secret scope is active. This applies to both the local backend (_make_run_env) and the Docker backend (_build_init_env_args). When no scope is installed (single-profile deployment), behavior is unchanged - get_secret falls through to os.environ transparently. Fixes NousResearch#76163
64f7e2b to
10b4455
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the multiplex secret-scope gap; current main does have the reported raw-environment reads in tools/environments/local.py:1226 and tools/environments/docker.py:1525.
Problems
- In the added local branch, a scope miss leaves the old
run_env[k]value intact. Under multiplexing,get_secret()returnsNonefor a missing scoped key rather than readingos.environ(agent/secret_scope.py:150-156), so this still forwards the default-profile credential. - The stated
execute_codecoverage is not implemented:tools/code_execution_tool.py:1371still calls_scrub_child_env(os.environ), and its passthrough branch copies the raw value at:240-243. Background/PTY terminal launches similarly use_sanitize_subprocess_env(os.environ, ...)intools/process_registry.py:735,777. - The PR diff contains no regression tests for the two-profile and scoped-missing cases.
Suggested changes
- Centralize scope-aware passthrough resolution for all four child-environment paths; omit a key on a scoped miss and let unscoped multiplex reads fail closed.
- Add behavioral tests proving routed-profile selection and no fallback to the default profile.
Automated hermes-sweeper review.
| for k in list(run_env): | ||
| if _is_passthrough(k): | ||
| scoped_val = get_secret(k) | ||
| if scoped_val is not None: |
There was a problem hiding this comment.
A scope miss leaves the existing run_env[k] value from os.environ intact. In multiplex mode get_secret() returns None for an absent scoped key (agent/secret_scope.py:150-156), so this still forwards the default-profile credential. Remove/omit the key on a scope miss, and avoid bypassing get_secret() when multiplexing is active so unscoped reads fail closed.
|
Closing in favor of #76213, which implements the same fix correctly. The diagnosis here (passthrough vars resolve from the default profile's environ under multiplexing) is right — thanks for surfacing it. But the implementation has three lens violations our review flagged: on a scoped miss the pre-populated default-profile value stays in run_env (the exact leak this targets), the except-Exception blocks swallow UnscopedSecretError (fail-closed must propagate), and only 2 of 4 child-env paths are covered (code_execution_tool + process_registry untouched), with no regression tests. #76213 centralizes resolution in resolve_passthrough_value, covers all four paths, handles scope-change staleness, and ships two-profile tests. Thanks @JonthanaHanh. |
Summary
Under
gateway.multiplex_profiles,terminalandexecute_codeenvironments are built fromos.environdirectly, bypassing the active profile secret scope. This causes routed profiles to receive the default profile's credentials for any key configured viaterminal.env_passthrough.Since the default profile is typically the broadest, every narrower profile is silently promoted to its authority. The call succeeds with the wrong credentials, and nothing logs the mismatch — it fails open.
Changes
tools/environments/local.py::_make_run_env: After building the subprocess env, re-resolve passthrough keys throughget_secret()when a profile secret scope is active.tools/environments/docker.py::_build_init_env_args: Same treatment for Docker forwarded env keys.When no scope is installed (single-profile deployment), behavior is unchanged —
get_secret()falls through toos.environtransparently.Test Plan
tests/agent/test_secret_scope.py— 12 passedtests/tools/test_local_env_blocklist.py— 30 passedtests/tools/test_env_passthrough.py— 24 passedFixes #76163