From 6e96752aa0b35fdb9bfc49ea202d610d90592b61 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Mon, 4 May 2026 13:08:07 +0000 Subject: [PATCH 1/2] fix(security): handle all PowerShell -EncodedCommand abbreviations and separator forms ExecShellWrapperParser.ParsePowerShellPayload previously only recognised -EncodedCommand, -enc, and -ec. PowerShell also accepts: * All unique prefix abbreviations from the minimum disambiguation point (-en, -enco, -encod, -encode, -encoded, ...) to the full flag name. * Inline separator forms: -enc:PAYLOAD and -enc=PAYLOAD (single token). * The same colon/equals separator forms for -Command / -c. Without these, a command like powershell -encode was not decoded by the wrapper parser, so the encoded inner command was never evaluated against the exec approval policy. The outer command 'powershell -encode ' might pass policy while executing arbitrary encoded code unchecked. Changes - Extract IsEncodedCommandFlag / IsCommandFlag helpers using prefix matching (-en minimum) and the explicit -ec alias. - Add IndexOfFlagSeparator to detect inline colon/equals forms. - Refactor decoding into DecodeEncodedPayload for reuse. - 22 new tests covering all abbreviation lengths, both separator forms, and the ambiguous -e-alone case (must NOT be treated as EncodedCommand). Pre-existing test baseline (unaffected by these changes): - Shared.Tests: 1172 pass / 2 fail (canvas JSONL, Linux-only) / 20 skip - Tray.Tests: 406 pass / 1 fail (DPAPI, Windows-only) / 0 skip Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/OpenClaw.Shared/ExecShellWrapperParser.cs | 91 +++++++++++++++---- .../ExecShellWrapperParserTests.cs | 83 +++++++++++++++++ 2 files changed, 154 insertions(+), 20 deletions(-) diff --git a/src/OpenClaw.Shared/ExecShellWrapperParser.cs b/src/OpenClaw.Shared/ExecShellWrapperParser.cs index b5640e3f..eb1e8c15 100644 --- a/src/OpenClaw.Shared/ExecShellWrapperParser.cs +++ b/src/OpenClaw.Shared/ExecShellWrapperParser.cs @@ -135,8 +135,26 @@ private static (string? Payload, string? Shell, string? Error) ParsePowerShellPa for (var i = 1; i < tokens.Length; i++) { var option = tokens[i]; - if (option.Equals("-Command", StringComparison.OrdinalIgnoreCase) || - option.Equals("-c", StringComparison.OrdinalIgnoreCase)) + + // Check for inline separator form first: -flag:value or -flag=value + var sepIdx = IndexOfFlagSeparator(option); + if (sepIdx > 0) + { + var flagPart = option[..sepIdx]; + var valuePart = option[(sepIdx + 1)..]; + + if (IsCommandFlag(flagPart)) + { + return string.IsNullOrWhiteSpace(valuePart) + ? ("", shell, "Shell wrapper payload was empty") + : (valuePart, shell, null); + } + + if (IsEncodedCommandFlag(flagPart)) + return DecodeEncodedPayload(valuePart, shell); + } + + if (IsCommandFlag(option)) { var payload = string.Join(" ", tokens, i + 1, tokens.Length - i - 1).Trim(); return string.IsNullOrWhiteSpace(payload) @@ -144,32 +162,65 @@ private static (string? Payload, string? Shell, string? Error) ParsePowerShellPa : (payload, shell, null); } - if (option.Equals("-EncodedCommand", StringComparison.OrdinalIgnoreCase) || - option.Equals("-enc", StringComparison.OrdinalIgnoreCase) || - option.Equals("-ec", StringComparison.OrdinalIgnoreCase)) + if (IsEncodedCommandFlag(option)) { var encoded = i + 1 < tokens.Length ? tokens[i + 1] : null; - if (string.IsNullOrWhiteSpace(encoded)) - return ("", shell, "Shell wrapper payload was empty"); - - try - { - var bytes = Convert.FromBase64String(encoded); - var payload = Encoding.Unicode.GetString(bytes).Trim(); - return string.IsNullOrWhiteSpace(payload) - ? ("", shell, "EncodedCommand decoded to an empty payload") - : (payload, shell, null); - } - catch (FormatException) - { - return ("", shell, "EncodedCommand could not be decoded"); - } + return DecodeEncodedPayload(encoded, shell); } } return default; } + // Returns the index of the first ':' or '=' in a flag token (after the leading '-'). + private static int IndexOfFlagSeparator(string token) + { + for (var i = 1; i < token.Length; i++) + { + if (token[i] == ':' || token[i] == '=') + return i; + } + return -1; + } + + // Matches -Command and -c (documented PowerShell -Command aliases). + private static bool IsCommandFlag(string flag) => + flag.Equals("-Command", StringComparison.OrdinalIgnoreCase) || + flag.Equals("-c", StringComparison.OrdinalIgnoreCase); + + // Matches -ec (explicit alias) and all unique prefix abbreviations of -EncodedCommand + // starting from the minimum disambiguating prefix -en (2 chars after '-'). + // -e alone is ambiguous with -ExecutionPolicy and is intentionally excluded. + private static bool IsEncodedCommandFlag(string flag) + { + if (flag.Equals("-ec", StringComparison.OrdinalIgnoreCase)) + return true; + + const string fullFlag = "-encodedcommand"; + return flag.Length >= 3 && // minimum: -en + flag.Length <= fullFlag.Length && + fullFlag.StartsWith(flag, StringComparison.OrdinalIgnoreCase); + } + + private static (string? Payload, string? Shell, string? Error) DecodeEncodedPayload(string? encoded, string shell) + { + if (string.IsNullOrWhiteSpace(encoded)) + return ("", shell, "Shell wrapper payload was empty"); + + try + { + var bytes = Convert.FromBase64String(encoded); + var payload = Encoding.Unicode.GetString(bytes).Trim(); + return string.IsNullOrWhiteSpace(payload) + ? ("", shell, "EncodedCommand decoded to an empty payload") + : (payload, shell, null); + } + catch (FormatException) + { + return ("", shell, "EncodedCommand could not be decoded"); + } + } + private static List SplitTopLevelCommands(string command) { var parts = new List(); diff --git a/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs b/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs index db7d2c1f..d925d500 100644 --- a/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs +++ b/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs @@ -138,6 +138,89 @@ public void Expand_Powershell_EcAlias_Decodes() Assert.Contains(result.Targets, t => t.Command.Contains("Remove-Item")); } + // All unique prefix abbreviations of -EncodedCommand beyond -enc/-ec + [Theory] + [InlineData("-en")] + [InlineData("-enco")] + [InlineData("-encod")] + [InlineData("-encode")] + [InlineData("-encoded")] + [InlineData("-encodedc")] + [InlineData("-encodedco")] + [InlineData("-encodedcom")] + [InlineData("-encodedcomm")] + [InlineData("-encodedcomma")] + [InlineData("-encodedcomman")] + [InlineData("-encodedcommand")] + public void Expand_Powershell_EncodedCommand_PrefixAbbreviation_Decodes(string flag) + { + var payload = "Get-ChildItem C:\\"; + var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); + var result = Expand($"powershell {flag} {encoded}"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Get-ChildItem")); + } + + // Inline separator forms: -enc:value and -enc=value + [Theory] + [InlineData("-enc")] + [InlineData("-EncodedCommand")] + [InlineData("-encodedcommand")] + public void Expand_Powershell_EncodedCommand_ColonSeparator_Decodes(string flagBase) + { + var payload = "Invoke-Something"; + var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); + var result = Expand($"powershell {flagBase}:{encoded}"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Invoke-Something")); + } + + [Theory] + [InlineData("-enc")] + [InlineData("-EncodedCommand")] + public void Expand_Powershell_EncodedCommand_EqualsSeparator_Decodes(string flagBase) + { + var payload = "Write-Host hi"; + var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); + var result = Expand($"powershell {flagBase}={encoded}"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Write-Host")); + } + + // -Command separator forms + [Theory] + [InlineData("-Command")] + [InlineData("-c")] + public void Expand_Powershell_Command_ColonSeparator_ExtractsPayload(string flagBase) + { + var result = Expand($"powershell {flagBase}:Get-Process"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Get-Process")); + } + + [Theory] + [InlineData("-Command")] + [InlineData("-c")] + public void Expand_Powershell_Command_EqualsSeparator_ExtractsPayload(string flagBase) + { + var result = Expand($"powershell {flagBase}=Get-Date"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Get-Date")); + } + + // Ambiguous -e alone must NOT be treated as -EncodedCommand + [Fact] + public void Expand_Powershell_SingleE_NotTreatedAsEncodedCommand() + { + // -e alone is ambiguous (-EncodedCommand vs -ExecutionPolicy); must not decode + var payload = "Get-ChildItem"; + var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); + var result = Expand($"powershell -e {encoded}"); + // Should not produce a decoded target from -e + Assert.True(result.Error != null || !result.Targets.Any(t => t.Command.Contains("Get-ChildItem")), + "Ambiguous -e flag must not be silently treated as -EncodedCommand"); + } + [Fact] public void Expand_Powershell_EncodedCommand_EmptyPayload_ReturnsError() { From 25a2a46678903e3305e2274de7d938f08a8feae1 Mon Sep 17 00:00:00 2001 From: Scott Hanselman Date: Thu, 7 May 2026 13:58:31 -0400 Subject: [PATCH 2/2] fix(security): treat PowerShell -e as EncodedCommand Windows PowerShell accepts -e as an EncodedCommand alias, so the shell-wrapper parser must decode it to keep approval evaluation fail-closed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/OpenClaw.Shared/ExecShellWrapperParser.cs | 9 ++++++--- .../ExecShellWrapperParserTests.cs | 14 +++++++------- 2 files changed, 13 insertions(+), 10 deletions(-) diff --git a/src/OpenClaw.Shared/ExecShellWrapperParser.cs b/src/OpenClaw.Shared/ExecShellWrapperParser.cs index eb1e8c15..10568eaa 100644 --- a/src/OpenClaw.Shared/ExecShellWrapperParser.cs +++ b/src/OpenClaw.Shared/ExecShellWrapperParser.cs @@ -188,11 +188,14 @@ private static bool IsCommandFlag(string flag) => flag.Equals("-Command", StringComparison.OrdinalIgnoreCase) || flag.Equals("-c", StringComparison.OrdinalIgnoreCase); - // Matches -ec (explicit alias) and all unique prefix abbreviations of -EncodedCommand - // starting from the minimum disambiguating prefix -en (2 chars after '-'). - // -e alone is ambiguous with -ExecutionPolicy and is intentionally excluded. + // Matches -e/-ec aliases and all unique prefix abbreviations of -EncodedCommand. + // Windows PowerShell accepts -e as EncodedCommand despite the apparent ambiguity with + // -ExecutionPolicy, so the parser must fail closed and decode it. private static bool IsEncodedCommandFlag(string flag) { + if (flag.Equals("-e", StringComparison.OrdinalIgnoreCase)) + return true; + if (flag.Equals("-ec", StringComparison.OrdinalIgnoreCase)) return true; diff --git a/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs b/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs index d925d500..af3724cc 100644 --- a/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs +++ b/tests/OpenClaw.Shared.Tests/ExecShellWrapperParserTests.cs @@ -138,8 +138,11 @@ public void Expand_Powershell_EcAlias_Decodes() Assert.Contains(result.Targets, t => t.Command.Contains("Remove-Item")); } - // All unique prefix abbreviations of -EncodedCommand beyond -enc/-ec + // All unique prefix abbreviations of -EncodedCommand beyond -enc/-ec. + // Windows PowerShell also accepts -e as EncodedCommand, so include it to + // keep the shell-wrapper parser fail-closed. [Theory] + [InlineData("-e")] [InlineData("-en")] [InlineData("-enco")] [InlineData("-encod")] @@ -208,17 +211,14 @@ public void Expand_Powershell_Command_EqualsSeparator_ExtractsPayload(string fla Assert.Contains(result.Targets, t => t.Command.Contains("Get-Date")); } - // Ambiguous -e alone must NOT be treated as -EncodedCommand [Fact] - public void Expand_Powershell_SingleE_NotTreatedAsEncodedCommand() + public void Expand_Powershell_SingleE_DecodesEncodedCommand() { - // -e alone is ambiguous (-EncodedCommand vs -ExecutionPolicy); must not decode var payload = "Get-ChildItem"; var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(payload)); var result = Expand($"powershell -e {encoded}"); - // Should not produce a decoded target from -e - Assert.True(result.Error != null || !result.Targets.Any(t => t.Command.Contains("Get-ChildItem")), - "Ambiguous -e flag must not be silently treated as -EncodedCommand"); + Assert.Null(result.Error); + Assert.Contains(result.Targets, t => t.Command.Contains("Get-ChildItem")); } [Fact]