[Repo Assist] security: add ExecEnvSanitizer to block env-var injection in system.run#186
Closed
github-actions[bot] wants to merge 1 commit intomasterfrom
Closed
Conversation
Fixes the environment-variable injection vector described in issue #184. An agent can supply an 'env' dict that overrides variables like PATH, NODE_OPTIONS, GIT_SSH_COMMAND, or LD_PRELOAD, silently redirecting execution of an otherwise-approved command. ExecEnvSanitizer filters these variables before they reach the process, preserving the intent of the exec approval policy. Changes: - New ExecEnvSanitizer (OpenClaw.Shared): static class with an exact-match FrozenSet denylist + LD_/DYLD_ prefix rules; blocks PATH, PATHEXT, ComSpec, NODE_OPTIONS, GIT_SSH_COMMAND, LD_PRELOAD, DYLD_*, BASH_ENV, PYTHONPATH, RUBYOPT, PERL5OPT, and related variables; also rejects empty names and names containing '=' or control characters. - SystemCapability.HandleRunAsync: parse raw env dict first, call ExecEnvSanitizer.Sanitize(), warn about blocked vars, forward only approved vars (null when none survive). - ExecEnvSanitizerTests (53 new tests): IsBlocked unit tests for all denylist entries (case-insensitive), safe-var allowlist, invalid names, Sanitize() mixed-input scenarios, LD_ prefix variants, and two end-to-end integration tests through SystemCapability. 639 Shared + 122 Tray tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
github-actions
bot
added
automation
bug
10 tasks
shanselman
added a commit
that referenced
this pull request
Apr 20, 2026
Closes #184 by blocking dangerous environment overrides and by re-evaluating nested shell-wrapper payloads and chained commands against the exec approval policy. This extends the partial env-only approach discussed in PR #186 so the Windows node closes both vectors called out in the issue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
shanselman
added a commit
that referenced
this pull request
Apr 20, 2026
Closes #184 by blocking dangerous environment overrides and by re-evaluating nested shell-wrapper payloads and chained commands against the exec approval policy. This extends the partial env-only approach discussed in PR #186 so the Windows node closes both vectors called out in the issue. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
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.
🤖 This is an automated PR from Repo Assist.
Closes #184 (partially — addresses the env-var injection vector; the shell wrapper bypass is a separate, larger change).
Summary
Adds
ExecEnvSanitizer, a new static class that strips dangerous environment-variable overrides before they reach the process, protecting against the injection attack described in issue #184.Attack scenario: An agent submits a
system.runrequest for an approved command (e.g.git status) but includes anenvfield overridingGIT_SSH_COMMAND,PATH,NODE_OPTIONS, orLD_PRELOAD. The outer command passes the exec approval policy unchanged, but execution is silently hijacked by the injected variable.Root Cause
SystemCapabilityparsed theenvdict and forwarded it toLocalCommandRunnerwithout any filtering.LocalCommandRunnercopied all values directly intoProcessStartInfo.Environment.Fix
New class
ExecEnvSanitizer(src/OpenClaw.Shared/ExecEnvSanitizer.cs):PATH,PATHEXT,ComSpec,PSModulePath,NODE_OPTIONS,NODE_PATH,PYTHONPATH,PYTHONSTARTUP,PYTHONUSERBASE,RUBYOPT,RUBYLIB,BASH_ENV,ENV,ZDOTDIR,GIT_SSH,GIT_SSH_COMMAND,GIT_EXEC_PATH,GIT_PROXY_COMMAND,GIT_ASKPASS,LD_PRELOAD,LD_LIBRARY_PATH,LD_AUDIT,DYLD_INSERT_LIBRARIES,DYLD_LIBRARY_PATH,PERL5OPT,PERL5LIB,PERLIOLD_orDYLD_=or control charactersSanitizeResultwithAlloweddict andBlockedlist for audit loggingIntegration in
SystemCapability.HandleRunAsync: callsExecEnvSanitizer.Sanitize()after parsing the env dict; logs a warning listing each blocked variable name; passes only approved variables (ornullif none survive) to the runner.Remaining Gap from #184
The shell wrapper approval bypass (item 2 in #184) — where
powershell -Command Remove-Item...evades a rule onRemove-Item *— is a separate, more complex change involving command parsing. This PR does not address it.Test Status
53 new tests in
ExecEnvSanitizerTests.cs:IsBlockedunit tests for all denylist entries, case-insensitive variants, safe-var allowlist (LOG_LEVEL, APP_ENV, TZ, LANG, LC_ALL, HOME, RAILS_ENV...), invalid namesSanitize()scenarios: all-safe, all-dangerous, mixed, empty input, LD_ prefix variantsSystemCapability: verifies blocked vars don't reach the runner, safe vars pass through, and env is set tonullwhen all vars are blockedTotals: