From dc640eef32e61c283ad1c6bba0601a318708d17b Mon Sep 17 00:00:00 2001
From: "github-actions[bot]"
 <41898282+github-actions[bot]@users.noreply.github.com>
Date: Fri, 1 May 2026 09:54:41 -0700
Subject: [PATCH] fix(security): block dangerous stem+wildcard allow patterns
 in execApprovals.set (#255)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ValidateExecApprovalRules previously checked for dangerous fragments that end
with a trailing space (e.g. "rm ") but missed the case where the wildcard
character replaces the space — e.g. "rm*" passes the "rm " fragment check yet
matches "rm -rf /" via the ^rm.*$ regex, effectively bypassing the intended
block.

Fix: for each dangerous fragment that has trailing whitespace, also reject
patterns containing the trimmed stem followed directly by * or ?.

Before:
  { "pattern": "rm*", "action": "allow" }  → accepted, allows "rm -rf /"
  { "pattern": "del*", "action": "allow" } → accepted, allows "del /s /q C:\\"

After:
  { "pattern": "rm*", "action": "allow" }  → rejected ("Dangerous allow rule…")
  { "pattern": "del*", "action": "allow" } → rejected

Adds 7 InlineData regression tests covering: rm*, rm?, del*, del?,
remove-item*, shutdown*, net*.

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---
 .../Capabilities/SystemCapability.cs          | 10 ++++
 .../ExecApprovalPolicyTests.cs                | 46 +++++++++++++++++++
 2 files changed, 56 insertions(+)

diff --git a/src/OpenClaw.Shared/Capabilities/SystemCapability.cs b/src/OpenClaw.Shared/Capabilities/SystemCapability.cs
index 337e7e0..91d0e1a 100644
--- a/src/OpenClaw.Shared/Capabilities/SystemCapability.cs
+++ b/src/OpenClaw.Shared/Capabilities/SystemCapability.cs
@@ -647,6 +647,16 @@ public class SystemCapability : NodeCapabilityBase
             {
                 if (normalized.Contains(dangerous, StringComparison.Ordinal))
                     return $"Dangerous allow rule is not permitted: {pattern}";
+
+                // Also block stem+wildcard (e.g. "rm*" bypasses "rm " because the
+                // fragment has a trailing space that the wildcard replaces).
+                var stem = dangerous.TrimEnd();
+                if (stem.Length < dangerous.Length &&
+                    (normalized.Contains(stem + "*", StringComparison.Ordinal) ||
+                     normalized.Contains(stem + "?", StringComparison.Ordinal)))
+                {
+                    return $"Dangerous allow rule is not permitted: {pattern}";
+                }
             }
         }
 
diff --git a/tests/OpenClaw.Shared.Tests/ExecApprovalPolicyTests.cs b/tests/OpenClaw.Shared.Tests/ExecApprovalPolicyTests.cs
index 85613f4..57498c7 100644
--- a/tests/OpenClaw.Shared.Tests/ExecApprovalPolicyTests.cs
+++ b/tests/OpenClaw.Shared.Tests/ExecApprovalPolicyTests.cs
@@ -654,6 +654,52 @@ public class SystemCapabilityExecApprovalsTests
         }
     }
 
+    /// <summary>
+    /// Verifies that exec-approvals.set rejects Allow rules where a dangerous command stem
+    /// is immediately followed by a wildcard (e.g. "rm*"), which would bypass the trailing-
+    /// space fragment check used for patterns like "rm ".
+    /// </summary>
+    [Theory]
+    [InlineData("rm*")]
+    [InlineData("rm?")]
+    [InlineData("del*")]
+    [InlineData("del?")]
+    [InlineData("remove-item*")]
+    [InlineData("shutdown*")]
+    [InlineData("net*")]
+    public async Task ExecApprovalsSet_RejectsDangerousStemPlusWildcardAllowRule(string dangerousPattern)
+    {
+        var tempDir = Path.Combine(Path.GetTempPath(), $"test-{Guid.NewGuid():N}");
+        Directory.CreateDirectory(tempDir);
+        try
+        {
+            var policy = new ExecApprovalPolicy(tempDir, _logger);
+            var cap = CreateCapability(policy);
+
+            var json = JsonDocument.Parse($@"{{
+                ""baseHash"": ""{policy.GetPolicyHash()}"",
+                ""rules"": [
+                    {{""pattern"": ""{dangerousPattern}"", ""action"": ""allow""}}
+                ],
+                ""defaultAction"": ""deny""
+            }}");
+
+            var request = new NodeInvokeRequest
+            {
+                Command = "system.execApprovals.set",
+                Args = json.RootElement
+            };
+
+            var result = await cap.ExecuteAsync(request);
+            Assert.False(result.Ok);
+            Assert.Contains("Dangerous allow rule is not permitted", result.Error!, StringComparison.OrdinalIgnoreCase);
+        }
+        finally
+        {
+            try { Directory.Delete(tempDir, true); } catch { }
+        }
+    }
+
     [Fact]
     public async Task ExecApprovalsGet_ReturnsPolicy()
     {