improve: modernize HandleRunAsync syntax; add policy+args regression tests
- Use range slice argv[1..] instead of argv.Skip(1).ToArray() (LINQ-free) - Add SystemRun_SeparateArgsProperty_PolicyEvaluatesFullCommandLine: regression guard ensuring policy evaluates the full 'rm -rf /' when args come from the separate JSON 'args' property rather than the command argv array - Add SystemRun_ShellFilter_PolicySkipsRuleForWrongShell: verifies that shell- filtered rules (Shells=["pwsh"]) are not applied when a different shell (cmd) is requested 588 Shared + 20 skipped; Tray unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
parent
e46dfe7830
commit
68a18de623
@ -223,7 +223,7 @@ public class SystemCapability : NodeCapabilityBase
|
||||
// Also accept a plain string for backward compatibility.
|
||||
var argv = TryParseArgv(request.Args);
|
||||
string? command = argv?[0];
|
||||
string[]? args = argv?.Length > 1 ? argv.Skip(1).ToArray() : null;
|
||||
string[]? args = argv?.Length > 1 ? argv[1..] : null;
|
||||
|
||||
// When command is a string, also check for separate "args" array
|
||||
if (argv?.Length == 1 && request.Args.TryGetProperty("args", out var argsEl) &&
|
||||
@ -267,8 +267,8 @@ public class SystemCapability : NodeCapabilityBase
|
||||
// When command arrives as an argv array, we must evaluate the entire
|
||||
// command line — not just argv[0] — so policy rules like "rm *" correctly
|
||||
// match "rm -rf /".
|
||||
var fullCommand = args != null
|
||||
? FormatExecCommand(new[] { command }.Concat(args).ToArray())
|
||||
var fullCommand = args != null
|
||||
? FormatExecCommand([command!, ..args])
|
||||
: command;
|
||||
|
||||
Logger.Info($"system.run: {fullCommand} (shell={shell ?? "auto"}, timeout={timeoutMs}ms)");
|
||||
|
||||
@ -685,6 +685,98 @@ public class SystemCapabilityExecApprovalsTests
|
||||
try { Directory.Delete(tempDir, true); } catch { }
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SystemRun_SeparateArgsProperty_PolicyEvaluatesFullCommandLine()
|
||||
{
|
||||
// Regression guard: when "command" is a string and args come from the separate
|
||||
// "args" JSON property (e.g. {"command":"rm","args":["-rf","/"]}), the policy
|
||||
// must evaluate the full combined command "rm -rf /" — not just "rm".
|
||||
var tempDir = Path.Combine(Path.GetTempPath(), $"test-{Guid.NewGuid():N}");
|
||||
Directory.CreateDirectory(tempDir);
|
||||
try
|
||||
{
|
||||
var policy = new ExecApprovalPolicy(tempDir, _logger);
|
||||
policy.SetRules(new[]
|
||||
{
|
||||
new ExecApprovalRule { Pattern = "rm -rf *", Action = ExecApprovalAction.Deny },
|
||||
new ExecApprovalRule { Pattern = "rm *", Action = ExecApprovalAction.Allow },
|
||||
}, ExecApprovalAction.Deny);
|
||||
|
||||
var cap = CreateCapability(policy);
|
||||
|
||||
// {"command":"rm","args":["-rf","/"]} — full command "rm -rf /" must be denied
|
||||
var dangerousReq = new NodeInvokeRequest
|
||||
{
|
||||
Command = "system.run",
|
||||
Args = JsonDocument.Parse("{\"command\":\"rm\",\"args\":[\"-rf\",\"/\"]}").RootElement
|
||||
};
|
||||
var denied = await cap.ExecuteAsync(dangerousReq);
|
||||
Assert.False(denied.Ok);
|
||||
Assert.Contains("denied", denied.Error!, StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
// {"command":"rm","args":["safe.txt"]} — "rm safe.txt" matches "rm *" → allowed
|
||||
var safeReq = new NodeInvokeRequest
|
||||
{
|
||||
Command = "system.run",
|
||||
Args = JsonDocument.Parse("{\"command\":\"rm\",\"args\":[\"safe.txt\"]}").RootElement
|
||||
};
|
||||
var allowed = await cap.ExecuteAsync(safeReq);
|
||||
Assert.True(allowed.Ok);
|
||||
}
|
||||
finally
|
||||
{
|
||||
try { Directory.Delete(tempDir, true); } catch { }
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task SystemRun_ShellFilter_PolicySkipsRuleForWrongShell()
|
||||
{
|
||||
// A rule with Shells=["pwsh"] must not fire when shell="cmd",
|
||||
// ensuring shell-filtered rules are not applied across shell contexts.
|
||||
var tempDir = Path.Combine(Path.GetTempPath(), $"test-{Guid.NewGuid():N}");
|
||||
Directory.CreateDirectory(tempDir);
|
||||
try
|
||||
{
|
||||
var policy = new ExecApprovalPolicy(tempDir, _logger);
|
||||
policy.SetRules(new[]
|
||||
{
|
||||
// This allow rule only applies to pwsh
|
||||
new ExecApprovalRule
|
||||
{
|
||||
Pattern = "Get-Process *",
|
||||
Action = ExecApprovalAction.Allow,
|
||||
Shells = new[] { "pwsh" }
|
||||
},
|
||||
}, ExecApprovalAction.Deny);
|
||||
|
||||
var cap = CreateCapability(policy);
|
||||
|
||||
// shell=pwsh → rule fires → allowed
|
||||
var pwshReq = new NodeInvokeRequest
|
||||
{
|
||||
Command = "system.run",
|
||||
Args = JsonDocument.Parse("{\"command\":\"Get-Process explorer\",\"shell\":\"pwsh\"}").RootElement
|
||||
};
|
||||
var pwshResult = await cap.ExecuteAsync(pwshReq);
|
||||
Assert.True(pwshResult.Ok);
|
||||
|
||||
// shell=cmd → rule is skipped → denied by default
|
||||
var cmdReq = new NodeInvokeRequest
|
||||
{
|
||||
Command = "system.run",
|
||||
Args = JsonDocument.Parse("{\"command\":\"Get-Process explorer\",\"shell\":\"cmd\"}").RootElement
|
||||
};
|
||||
var cmdResult = await cap.ExecuteAsync(cmdReq);
|
||||
Assert.False(cmdResult.Ok);
|
||||
Assert.Contains("denied", cmdResult.Error!, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
finally
|
||||
{
|
||||
try { Directory.Delete(tempDir, true); } catch { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>Mock command runner that always succeeds</summary>
|
||||
|
||||
Loading…
Reference in New Issue
Block a user