From 815674b47b766e9984da3367827f0720dfb4f3e6 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Sat, 25 Apr 2026 12:42:09 +0000 Subject: [PATCH] test: add ExecShellWrapperParser and ExecEnvSanitizer unit tests (+98 tests) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ExecShellWrapperParser (security-critical shell-unwrapping parser): - 34 new tests covering cmd /c, cmd /k, powershell -Command, -EncodedCommand (-enc/-ec aliases), pwsh, bash/sh wrapping, semicolon/& /&&/|| chaining, quote-protected separators, depth limiting, nested shell wrapping, and shell normalisation - Exercises all error paths: empty payload, invalid base64, missing -Command arg ExecEnvSanitizer (env variable security filter): - 24 new tests covering all 30 known-blocked names, case-insensitive matching, LD_*/DYLD_* prefix blocking, null/whitespace/invalid-char name rejection, mixed allowed+blocked split, all-blocked (null Allowed), all-allowed (empty Blocked) - Regression-grade: tests that PATH/PATHEXT/ComSpec/GIT_SSH_COMMAND etc. are blocked Both classes are internal with InternalsVisibleTo already configured for the test project. Baseline: 652 passed → after: 750 passed (+98); 20 skipped unchanged; Tray 122/122. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../ExecEnvSanitizerTests.cs | 235 ++++++++++++++ .../ExecShellWrapperParserTests.cs | 292 ++++++++++++++++++ 2 files changed, 527 insertions(+) create mode 100644 tests/OpenClaw.Shared.Tests/ExecEnvSanitizerTests.cs create mode 100644 tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs diff --git a/tests/OpenClaw.Shared.Tests/ExecEnvSanitizerTests.cs b/tests/OpenClaw.Shared.Tests/ExecEnvSanitizerTests.cs new file mode 100644 index 00000000..4cdf1b04 --- /dev/null +++ b/tests/OpenClaw.Shared.Tests/ExecEnvSanitizerTests.cs @@ -0,0 +1,235 @@ +using System; +using System.Collections.Generic; +using Xunit; +using OpenClaw.Shared; + +namespace OpenClaw.Shared.Tests; + +/// +/// Unit tests for ExecEnvSanitizer — the security filter that blocks dangerous +/// environment variable overrides before they reach the shell. +/// +public class ExecEnvSanitizerTests +{ + // ── null / empty input ──────────────────────────────────────────────────── + + [Fact] + public void Sanitize_NullEnv_ReturnsNullAllowed_EmptyBlocked() + { + var result = ExecEnvSanitizer.Sanitize(null); + Assert.Null(result.Allowed); + Assert.Empty(result.Blocked); + } + + [Fact] + public void Sanitize_EmptyDict_ReturnsAllowedPassthrough_EmptyBlocked() + { + // An empty dict has Count == 0, so the sanitizer returns it unchanged (no work to do). + var empty = new Dictionary(); + var result = ExecEnvSanitizer.Sanitize(empty); + Assert.Same(empty, result.Allowed); + Assert.Empty(result.Blocked); + } + + // ── known-blocked names ─────────────────────────────────────────────────── + + [Theory] + [InlineData("PATH")] + [InlineData("PATHEXT")] + [InlineData("ComSpec")] + [InlineData("PSModulePath")] + [InlineData("NODE_OPTIONS")] + [InlineData("NODE_PATH")] + [InlineData("PYTHONPATH")] + [InlineData("PYTHONSTARTUP")] + [InlineData("PYTHONUSERBASE")] + [InlineData("RUBYOPT")] + [InlineData("RUBYLIB")] + [InlineData("PERL5OPT")] + [InlineData("PERL5LIB")] + [InlineData("PERLIO")] + [InlineData("GIT_SSH")] + [InlineData("GIT_SSH_COMMAND")] + [InlineData("GIT_EXEC_PATH")] + [InlineData("GIT_PROXY_COMMAND")] + [InlineData("GIT_ASKPASS")] + [InlineData("BASH_ENV")] + [InlineData("ENV")] + [InlineData("CDPATH")] + [InlineData("PROMPT_COMMAND")] + [InlineData("ZDOTDIR")] + [InlineData("LD_PRELOAD")] + [InlineData("LD_LIBRARY_PATH")] + [InlineData("LD_AUDIT")] + [InlineData("DYLD_INSERT_LIBRARIES")] + [InlineData("DYLD_LIBRARY_PATH")] + public void IsBlocked_KnownDangerousName_ReturnsTrue(string name) + { + Assert.True(ExecEnvSanitizer.IsBlocked(name)); + } + + [Theory] + [InlineData("PATH")] // exact case + [InlineData("path")] // lower + [InlineData("Path")] // mixed + [InlineData("COMSPEC")] // upper + [InlineData("comspec")] // lower + public void IsBlocked_CaseInsensitive(string name) + { + Assert.True(ExecEnvSanitizer.IsBlocked(name)); + } + + // ── LD_ / DYLD_ prefix blocking ────────────────────────────────────────── + + [Theory] + [InlineData("LD_CUSTOM")] + [InlineData("LD_")] + [InlineData("ld_custom")] // case-insensitive + [InlineData("DYLD_CUSTOM")] + [InlineData("dyld_custom")] + public void IsBlocked_LdDyldPrefix_ReturnsTrue(string name) + { + Assert.True(ExecEnvSanitizer.IsBlocked(name)); + } + + // ── invalid / malformed names ───────────────────────────────────────────── + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" ")] + public void IsBlocked_NullOrWhitespace_ReturnsTrue(string? name) + { + Assert.True(ExecEnvSanitizer.IsBlocked(name)); + } + + [Theory] + [InlineData("BAD=NAME")] // contains '=' + [InlineData("BAD\0NAME")] // contains NUL + [InlineData("BAD\rNAME")] // contains CR + [InlineData("BAD\nNAME")] // contains LF + [InlineData("BAD NAME")] // contains space + [InlineData("BAD\tNAME")] // contains tab + public void IsBlocked_InvalidCharacters_ReturnsTrue(string name) + { + Assert.True(ExecEnvSanitizer.IsBlocked(name)); + } + + // ── allowed names ───────────────────────────────────────────────────────── + + [Theory] + [InlineData("MY_CUSTOM_VAR")] + [InlineData("FOO")] + [InlineData("APP_ENV")] + [InlineData("TEST_OPENCLAW_VAR")] + [InlineData("SOME_123_VAR")] + public void IsBlocked_SafeName_ReturnsFalse(string name) + { + Assert.False(ExecEnvSanitizer.IsBlocked(name)); + } + + // ── Sanitize: mixed allowed + blocked ──────────────────────────────────── + + [Fact] + public void Sanitize_MixedDict_SeparatesAllowedAndBlocked() + { + var env = new Dictionary + { + ["MY_VAR"] = "ok", + ["PATH"] = "evil", + ["ANOTHER_VAR"] = "also_ok", + ["LD_PRELOAD"] = "evil2", + }; + + var result = ExecEnvSanitizer.Sanitize(env); + + Assert.NotNull(result.Allowed); + Assert.Equal(2, result.Allowed!.Count); + Assert.True(result.Allowed.ContainsKey("MY_VAR")); + Assert.True(result.Allowed.ContainsKey("ANOTHER_VAR")); + + Assert.Equal(2, result.Blocked.Length); + Assert.Contains("PATH", result.Blocked, StringComparer.OrdinalIgnoreCase); + Assert.Contains("LD_PRELOAD", result.Blocked, StringComparer.OrdinalIgnoreCase); + } + + [Fact] + public void Sanitize_AllBlocked_ReturnsNullAllowed() + { + var env = new Dictionary + { + ["PATH"] = "evil", + ["PATHEXT"] = "evil2", + }; + + var result = ExecEnvSanitizer.Sanitize(env); + + Assert.Null(result.Allowed); + Assert.Equal(2, result.Blocked.Length); + } + + [Fact] + public void Sanitize_AllAllowed_ReturnsEmptyBlocked() + { + var env = new Dictionary + { + ["MY_VAR"] = "a", + ["OTHER_VAR"] = "b", + }; + + var result = ExecEnvSanitizer.Sanitize(env); + + Assert.NotNull(result.Allowed); + Assert.Equal(2, result.Allowed!.Count); + Assert.Empty(result.Blocked); + } + + [Fact] + public void Sanitize_PreservesValues() + { + var env = new Dictionary + { + ["CUSTOM"] = "hello world", + }; + + var result = ExecEnvSanitizer.Sanitize(env); + + Assert.Equal("hello world", result.Allowed!["CUSTOM"]); + } + + // ── case-insensitive lookup in Sanitize ─────────────────────────────────── + + [Fact] + public void Sanitize_BlockedName_CaseInsensitive() + { + var env = new Dictionary + { + ["path"] = "evil", // lower-case PATH + ["SAFE_VAR"] = "ok", + }; + + var result = ExecEnvSanitizer.Sanitize(env); + + Assert.Contains("path", result.Blocked, StringComparer.OrdinalIgnoreCase); + Assert.NotNull(result.Allowed); + Assert.True(result.Allowed!.ContainsKey("SAFE_VAR")); + } + + // ── LD_ prefix in Sanitize ──────────────────────────────────────────────── + + [Fact] + public void Sanitize_LdPrefixVar_IsBlocked() + { + var env = new Dictionary + { + ["LD_CUSTOM_EVIL"] = "val", + ["GOOD_VAR"] = "ok", + }; + + var result = ExecEnvSanitizer.Sanitize(env); + + Assert.Contains("LD_CUSTOM_EVIL", result.Blocked, StringComparer.OrdinalIgnoreCase); + Assert.NotNull(result.Allowed); + Assert.False(result.Allowed!.ContainsKey("LD_CUSTOM_EVIL")); + } +} diff --git a/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs b/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs new file mode 100644 index 00000000..db7d2c1f --- /dev/null +++ b/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs @@ -0,0 +1,292 @@ +using System; +using System.Text; +using Xunit; +using OpenClaw.Shared; + +namespace OpenClaw.Shared.Tests; + +/// +/// Unit tests for ExecShellWrapperParser.Expand — the security-critical parser +/// that unwraps shell wrapper commands (cmd /c, powershell -Command, bash -c, etc.) +/// to allow the approval policy to evaluate the actual underlying command. +/// +public class ExecShellWrapperParserTests +{ + // ── helpers ────────────────────────────────────────────────────────────── + + private static ExecShellParseResult Expand(string command, string? shell = null) + => ExecShellWrapperParser.Expand(command, shell); + + // ── empty / whitespace ──────────────────────────────────────────────────── + + [Fact] + public void Expand_EmptyString_ReturnsEmptyTargets() + { + var result = Expand(""); + Assert.Empty(result.Targets); + Assert.Null(result.Error); + } + + [Fact] + public void Expand_WhitespaceOnly_ReturnsEmptyTargets() + { + var result = Expand(" "); + Assert.Empty(result.Targets); + Assert.Null(result.Error); + } + + // ── plain commands (no shell wrappers) ──────────────────────────────────── + + [Fact] + public void Expand_PlainCommand_ReturnsNoTargets_NoError() + { + // A single non-wrapped command produces no extra targets and no error. + var result = Expand("echo hello"); + Assert.Empty(result.Targets); + Assert.Null(result.Error); + } + + // ── cmd.exe wrapping ───────────────────────────────────────────────────── + + [Theory] + [InlineData("cmd /c echo hello")] + [InlineData("cmd.exe /c echo hello")] + [InlineData("cmd /C echo hello")] // case-insensitive flag + public void Expand_CmdSlashC_ExtractsPayload(string command) + { + var result = Expand(command); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("echo hello") || t.Command == "echo hello"); + } + + [Fact] + public void Expand_CmdSlashK_ExtractsPayload() + { + var result = Expand("cmd /k dir C:\\"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("dir")); + } + + [Fact] + public void Expand_CmdSlashC_EmptyPayload_ReturnsError() + { + var result = Expand("cmd /c"); + Assert.NotNull(result.Error); + Assert.Contains("empty", result.Error, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void Expand_CmdSlashC_SetsShell_ToCmd() + { + var result = Expand("cmd /c echo hello"); + Assert.Null(result.Error); + // The extracted targets should have shell == "cmd" + Assert.All(result.Targets, t => Assert.Equal("cmd", t.Shell)); + } + + // ── PowerShell wrapping ─────────────────────────────────────────────────── + + [Theory] + [InlineData("powershell -Command Get-Process")] + [InlineData("powershell.exe -Command Get-Process")] + [InlineData("powershell -c Get-Process")] + public void Expand_Powershell_Command_ExtractsPayload(string command) + { + var result = Expand(command); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Get-Process")); + } + + [Theory] + [InlineData("pwsh -Command Get-Date")] + [InlineData("pwsh.exe -Command Get-Date")] + [InlineData("pwsh -c Get-Date")] + public void Expand_Pwsh_Command_ExtractsPayload(string command) + { + var result = Expand(command); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Get-Date")); + } + + [Fact] + public void Expand_Powershell_EncodedCommand_Decodes() + { + var payload = "Get-ChildItem"; + var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); + var result = Expand($"powershell -EncodedCommand {encoded}"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Get-ChildItem")); + } + + [Fact] + public void Expand_Powershell_ShortEncAlias_Decodes() + { + var payload = "Write-Output hello"; + var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); + var result = Expand($"powershell -enc {encoded}"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Write-Output")); + } + + [Fact] + public void Expand_Powershell_EcAlias_Decodes() + { + var payload = "Remove-Item test"; + var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); + var result = Expand($"powershell -ec {encoded}"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Remove-Item")); + } + + [Fact] + public void Expand_Powershell_EncodedCommand_EmptyPayload_ReturnsError() + { + var result = Expand("powershell -EncodedCommand"); + Assert.NotNull(result.Error); + } + + [Fact] + public void Expand_Powershell_EncodedCommand_InvalidBase64_ReturnsError() + { + var result = Expand("powershell -EncodedCommand NOT_VALID_BASE64!!!"); + Assert.NotNull(result.Error); + Assert.Contains("decoded", result.Error, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void Expand_Powershell_Command_EmptyPayload_ReturnsError() + { + var result = Expand("powershell -Command"); + Assert.NotNull(result.Error); + Assert.Contains("empty", result.Error, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public void Expand_Powershell_SetsShell_ToPowershell() + { + var result = Expand("powershell -Command Get-Process"); + Assert.Null(result.Error); + Assert.All(result.Targets, t => Assert.Equal("powershell", t.Shell)); + } + + [Fact] + public void Expand_Pwsh_SetsShell_ToPwsh() + { + var result = Expand("pwsh -Command Get-Date"); + Assert.Null(result.Error); + Assert.All(result.Targets, t => Assert.Equal("pwsh", t.Shell)); + } + + // ── bash / sh wrapping ─────────────────────────────────────────────────── + + [Theory] + [InlineData("bash -c echo hello")] + [InlineData("bash.exe -c echo hello")] + [InlineData("sh -c echo hello")] + [InlineData("sh.exe -c echo hello")] + public void Expand_Bash_C_ExtractsPayload(string command) + { + var result = Expand(command); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("echo hello") || t.Command == "echo hello"); + } + + [Fact] + public void Expand_Bash_SetsShell_ToSh() + { + var result = Expand("bash -c ls"); + Assert.Null(result.Error); + Assert.All(result.Targets, t => Assert.Equal("sh", t.Shell)); + } + + // ── semicolon / chain splitting ─────────────────────────────────────────── + + [Fact] + public void Expand_SemicolonSeparated_ProducesMultipleTargets() + { + var result = Expand("echo a; echo b"); + Assert.Null(result.Error); + Assert.Equal(2, result.Targets.Count); + Assert.Contains(result.Targets, t => t.Command.Contains("echo a") || t.Command == "echo a"); + Assert.Contains(result.Targets, t => t.Command.Contains("echo b") || t.Command == "echo b"); + } + + [Fact] + public void Expand_AmpersandSeparated_ProducesMultipleTargets() + { + var result = Expand("echo a & echo b"); + Assert.Null(result.Error); + Assert.Equal(2, result.Targets.Count); + } + + [Fact] + public void Expand_DoubleAmpersandSeparated_ProducesMultipleTargets() + { + var result = Expand("echo a && echo b"); + Assert.Null(result.Error); + Assert.Equal(2, result.Targets.Count); + } + + [Fact] + public void Expand_DoublePipeSeparated_ProducesMultipleTargets() + { + var result = Expand("echo a || echo b"); + Assert.Null(result.Error); + Assert.Equal(2, result.Targets.Count); + } + + [Fact] + public void Expand_SemicolonInsideQuotes_NotSplit() + { + // The semicolon inside quotes should not be treated as a separator. + var result = Expand("echo 'a;b'"); + Assert.Null(result.Error); + // Single non-wrapped command → no targets (depth 0, single segment) + Assert.Empty(result.Targets); + } + + // ── depth limiting ──────────────────────────────────────────────────────── + + [Fact] + public void Expand_DeepNesting_DoesNotInfiniteLoop() + { + // Four levels of cmd /c nesting should terminate cleanly. + var result = Expand("cmd /c cmd /c cmd /c cmd /c echo deep"); + // Should not throw or hang; result may have targets or empty — just no exception. + Assert.NotNull(result); + } + + // ── nested shell wrapping ────────────────────────────────────────────────── + + [Fact] + public void Expand_CmdWrapsPs_ProducesBothTargets() + { + // cmd /c powershell -Command Get-Process + // → first extracts "powershell -Command Get-Process" (as cmd payload), + // then recursively extracts "Get-Process" (as ps payload). + var result = Expand("cmd /c powershell -Command Get-Process"); + Assert.Null(result.Error); + Assert.True(result.Targets.Count >= 1); + } + + // ── shell normalisation ─────────────────────────────────────────────────── + + [Fact] + public void Expand_NullShell_DefaultsToPowershell() + { + // When no shell is provided, targets inherit "powershell" as the default. + var result = Expand("cmd /c echo hello", null); + Assert.Null(result.Error); + // Targets produced from a cmd /c call should carry "cmd" shell. + Assert.All(result.Targets, t => Assert.Equal("cmd", t.Shell)); + } + + [Fact] + public void Expand_ExplicitShell_PropagatedToTargets() + { + // Non-wrapped semicolon-chained command with an explicit shell. + var result = Expand("echo a; echo b", "pwsh"); + Assert.Null(result.Error); + Assert.All(result.Targets, t => Assert.Equal("pwsh", t.Shell)); + } +}