Skip to content

[Repo Assist] feat: implement ExecApprovalV2PolicyHandler (PR7 coordinator) #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
github-actions wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

[Repo Assist] feat: implement ExecApprovalV2PolicyHandler (PR7 coordinator) #261

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
174 changes: 124 additions & 50 deletions src/OpenClaw.Shared/Capabilities/SystemCapability.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,9 +275,13 @@ private async Task<NodeInvokeResponse> HandleRunAsync(NodeInvokeRequest request)
}

Logger.Info($"[system.run] corr={correlationId} decision={v2Result.Code} reason={v2Result.Reason}");
if (v2Result.Code == ExecApprovalV2Code.Allowed)
{
// V2 coordinator already evaluated policy — execute directly.
Logger.Info($"[system.run] corr={correlationId} v2-approved executing");
return await ExecuteRunRequestAsync(request, correlationId);
}
// Rail 1: no silent fallback to legacy regardless of result code.
// In PR1 only ExecApprovalV2NullHandler exists (always unavailable); the real
// coordinator that can produce an allow decision is wired in PR7/PR8.
return Error($"exec-approvals-v2: {v2Result.Code} ({v2Result.Reason})");
}

Expand All @@ -288,13 +292,83 @@ private async Task<NodeInvokeResponse> HandleRunAsync(NodeInvokeRequest request)
{
return Error("Command execution not available");
}


// Parse argv and parameters from raw request.
var legacyParsed = ParseRunRequest(request);
if (legacyParsed.Error != null)
return Error(legacyParsed.Error);

var (legacyCommand, legacyArgs, legacyShell, legacyCwd, legacyTimeoutMs, legacyEnv) = legacyParsed;

var fullCommand = legacyArgs != null
? FormatExecCommand([legacyCommand!, ..legacyArgs])
: legacyCommand;

Logger.Info($"system.run: {fullCommand} (shell={legacyShell ?? "auto"}, timeout={legacyTimeoutMs}ms)");

// Check exec approval policy
if (_approvalPolicy != null)
{
var approval = _approvalPolicy.Evaluate(fullCommand!, legacyShell);
if (!await EnsureApprovedAsync(fullCommand!, legacyShell, approval))
{
Logger.Warn($"system.run DENIED: {fullCommand} ({approval.Reason})");
return Error($"Command denied by exec policy: {approval.Reason}");
}

var parseResult = ExecShellWrapperParser.Expand(fullCommand!, legacyShell);
if (!string.IsNullOrWhiteSpace(parseResult.Error))
{
Logger.Warn($"system.run DENIED: {fullCommand} ({parseResult.Error})");
return Error($"Command denied by exec policy: {parseResult.Error}");
}

foreach (var target in parseResult.Targets)
{
var innerApproval = _approvalPolicy.Evaluate(target.Command, target.Shell);
if (!await EnsureApprovedAsync(target.Command, target.Shell, innerApproval))
{
Logger.Warn($"system.run DENIED: {target.Command} ({innerApproval.Reason})");
return Error($"Command denied by exec policy: {innerApproval.Reason}");
}
}
}

return await RunCommandAsync(legacyCommand!, legacyArgs, legacyShell, legacyCwd, legacyTimeoutMs, legacyEnv);
}

/// <summary>
/// Executes a V2-approved request without re-checking policy.
/// Called by the V2 routing path after the handler returns <see cref="ExecApprovalV2Code.Allowed"/>.
/// </summary>
private async Task<NodeInvokeResponse> ExecuteRunRequestAsync(NodeInvokeRequest request, string correlationId)
{
if (_commandRunner == null)
return Error("Command execution not available");

var parsed = ParseRunRequest(request);
if (parsed.Error != null)
return Error(parsed.Error);

var (command, args, shell, cwd, timeoutMs, env) = parsed;
var fullCommand = args != null ? FormatExecCommand([command!, ..args]) : command;
Logger.Info($"[system.run] corr={correlationId} v2-execute cmd={fullCommand} shell={shell ?? "auto"} timeout={timeoutMs}ms");

return await RunCommandAsync(command!, args, shell, cwd, timeoutMs, env);
}

/// <summary>
/// Parses the raw NodeInvokeRequest fields common to both the legacy and V2 execution paths.
/// Returns a <see cref="ParsedRunRequest"/> with <see cref="ParsedRunRequest.Error"/> set on failure.
/// </summary>
private ParsedRunRequest ParseRunRequest(NodeInvokeRequest request)
{
// Per OpenClaw spec, "command" is an argv array (e.g. ["echo","Hello"]).
// Also accept a plain string for backward compatibility.
var argv = TryParseArgv(request.Args);
string? command = argv?[0];
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) &&
argsEl.ValueKind == System.Text.Json.JsonValueKind.Array)
Expand All @@ -308,12 +382,10 @@ private async Task<NodeInvokeResponse> HandleRunAsync(NodeInvokeRequest request)
if (list.Count > 0)
args = list.ToArray();
}

if (string.IsNullOrWhiteSpace(command))
{
return Error("Missing command parameter");
}

return ParsedRunRequest.Fail("Missing command parameter");

var shell = GetStringArg(request.Args, "shell");
var cwd = GetStringArg(request.Args, "cwd");
var timeoutMs = GetIntArg(request.Args, "timeoutMs",
Expand All @@ -325,7 +397,7 @@ private async Task<NodeInvokeResponse> HandleRunAsync(NodeInvokeRequest request)
// from accidentally outliving the tray.
if (timeoutMs <= 0) timeoutMs = DefaultRunTimeoutMs;
if (timeoutMs > MaxRunTimeoutMs) timeoutMs = MaxRunTimeoutMs;

// Parse env dict if present
Dictionary<string, string>? env = null;
if (request.Args.ValueKind != System.Text.Json.JsonValueKind.Undefined &&
Expand All @@ -347,51 +419,24 @@ private async Task<NodeInvokeResponse> HandleRunAsync(NodeInvokeRequest request)
Array.Sort(blockedNames, StringComparer.OrdinalIgnoreCase);
var blockedList = string.Join(", ", blockedNames);
Logger.Warn($"system.run DENIED: blocked environment overrides [{blockedList}]");
return Error($"Unsafe environment variable override blocked: {blockedList}");
return ParsedRunRequest.Fail($"Unsafe environment variable override blocked: {blockedList}");
}
env = envResult.Allowed;

// Build the full command string for policy evaluation and logging.
// 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([command!, ..args])
: command;

Logger.Info($"system.run: {fullCommand} (shell={shell ?? "auto"}, timeout={timeoutMs}ms)");

// Check exec approval policy
if (_approvalPolicy != null)
{
var approval = _approvalPolicy.Evaluate(fullCommand, shell);
if (!await EnsureApprovedAsync(fullCommand, shell, approval))
{
Logger.Warn($"system.run DENIED: {fullCommand} ({approval.Reason})");
return Error($"Command denied by exec policy: {approval.Reason}");
}

var parseResult = ExecShellWrapperParser.Expand(fullCommand, shell);
if (!string.IsNullOrWhiteSpace(parseResult.Error))
{
Logger.Warn($"system.run DENIED: {fullCommand} ({parseResult.Error})");
return Error($"Command denied by exec policy: {parseResult.Error}");
}
return ParsedRunRequest.Ok(command!, args, shell, cwd, timeoutMs, env);
}

foreach (var target in parseResult.Targets)
{
var innerApproval = _approvalPolicy.Evaluate(target.Command, target.Shell);
if (!await EnsureApprovedAsync(target.Command, target.Shell, innerApproval))
{
Logger.Warn($"system.run DENIED: {target.Command} ({innerApproval.Reason})");
return Error($"Command denied by exec policy: {innerApproval.Reason}");
}
}
}

private async Task<NodeInvokeResponse> RunCommandAsync(
string command,
string[]? args,
string? shell,
string? cwd,
int timeoutMs,
Dictionary<string, string>? env)
{
try
{
var result = await _commandRunner.RunAsync(new CommandRequest
var result = await _commandRunner!.RunAsync(new CommandRequest
{
Command = command,
Args = args,
Expand All @@ -400,7 +445,7 @@ private async Task<NodeInvokeResponse> HandleRunAsync(NodeInvokeRequest request)
TimeoutMs = timeoutMs,
Env = env
});

return Success(new
{
stdout = result.Stdout,
Expand All @@ -417,6 +462,35 @@ private async Task<NodeInvokeResponse> HandleRunAsync(NodeInvokeRequest request)
}
}

/// <summary>
/// Parsed, sanitized system.run parameters — shared between legacy and V2 execution paths.
/// </summary>
private sealed class ParsedRunRequest
{
public string? Command { get; private init; }
public string[]? Args { get; private init; }
public string? Shell { get; private init; }
public string? Cwd { get; private init; }
public int TimeoutMs { get; private init; }
public Dictionary<string, string>? Env { get; private init; }
public string? Error { get; private init; }

public void Deconstruct(
out string? command, out string[]? args, out string? shell,
out string? cwd, out int timeoutMs, out Dictionary<string, string>? env)
{
command = Command; args = Args; shell = Shell;
cwd = Cwd; timeoutMs = TimeoutMs; env = Env;
}

public static ParsedRunRequest Ok(
string command, string[]? args, string? shell,
string? cwd, int timeoutMs, Dictionary<string, string>? env)
=> new() { Command = command, Args = args, Shell = shell, Cwd = cwd, TimeoutMs = timeoutMs, Env = env };

public static ParsedRunRequest Fail(string error) => new() { Error = error };
}

private async Task<bool> EnsureApprovedAsync(
string command,
string? shell,
Expand Down
73 changes: 73 additions & 0 deletions src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2PolicyHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
using System.Threading.Tasks;

namespace OpenClaw.Shared.ExecApprovals;

/// <summary>
/// V2 exec approval handler: validates input and evaluates the configured
/// ExecApprovalPolicy to decide Allow / SecurityDeny / AllowlistMiss.
///
/// This is the "PR7 coordinator" referred to in SystemCapability comments.
/// When installed via SetV2Handler and a command is approved, SystemCapability
/// executes it via the command runner already wired on the legacy path.
///
/// Shell-wrapper expansion mirrors the legacy path: after the top-level policy
/// check passes, each shell-wrapped sub-command is also evaluated so that
/// `cmd /c "rm /"` is not approved by a rule for `cmd`.
/// </summary>
public sealed class ExecApprovalV2PolicyHandler : IExecApprovalV2Handler
{
private readonly ExecApprovalPolicy _policy;
private readonly IOpenClawLogger _logger;

public ExecApprovalV2PolicyHandler(ExecApprovalPolicy policy, IOpenClawLogger logger)
{
_policy = policy;
_logger = logger;
}

public Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, string correlationId)
{
// Step 1: Structural input validation.
var validation = ExecApprovalV2InputValidator.Validate(request);
if (!validation.IsValid)
{
_logger.Info($"[exec-v2] corr={correlationId} validation-failed reason={validation.Error!.Reason}");
return Task.FromResult(validation.Error!);
}

var validated = validation.Request!;
var commandString = ShellQuoting.FormatExecCommand(validated.Argv);

// Step 2: Top-level policy check.
var topResult = _policy.Evaluate(commandString, validated.Shell);
_logger.Info($"[exec-v2] corr={correlationId} policy={topResult.Action} pattern={topResult.MatchedPattern ?? "(default)"}");

if (topResult.Action == ExecApprovalAction.Deny)
return Task.FromResult(ExecApprovalV2Result.SecurityDeny(topResult.Reason ?? "policy-deny"));

if (topResult.Action == ExecApprovalAction.Prompt)
return Task.FromResult(ExecApprovalV2Result.AllowlistMiss("prompt-required"));

// Step 3: Shell-wrapper expansion — ensure wrapped sub-commands also pass policy.
var parseResult = ExecShellWrapperParser.Expand(commandString, validated.Shell);
if (!string.IsNullOrWhiteSpace(parseResult.Error))
{
_logger.Warn($"[exec-v2] corr={correlationId} shell-parse-denied reason={parseResult.Error}");
return Task.FromResult(ExecApprovalV2Result.SecurityDeny(parseResult.Error));
}

foreach (var target in parseResult.Targets)
{
var innerResult = _policy.Evaluate(target.Command, target.Shell);
if (innerResult.Action == ExecApprovalAction.Deny)
{
_logger.Warn($"[exec-v2] corr={correlationId} inner-policy-deny cmd={target.Command}");
return Task.FromResult(ExecApprovalV2Result.SecurityDeny(innerResult.Reason ?? "inner-policy-deny"));
}
if (innerResult.Action == ExecApprovalAction.Prompt)
return Task.FromResult(ExecApprovalV2Result.AllowlistMiss("inner-prompt-required"));
}

return Task.FromResult(ExecApprovalV2Result.Allowed());
}
}
4 changes: 4 additions & 0 deletions src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2Result.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ namespace OpenClaw.Shared.ExecApprovals;
public enum ExecApprovalV2Code
{
Unavailable,
Allowed,
SecurityDeny,
AllowlistMiss,
UserDenied,
Expand All @@ -31,6 +32,9 @@ private ExecApprovalV2Result(ExecApprovalV2Code code, string reason)
public static ExecApprovalV2Result Unavailable(string reason = "Handler not available")
=> new(ExecApprovalV2Code.Unavailable, reason);

public static ExecApprovalV2Result Allowed(string reason = "policy-allow")
=> new(ExecApprovalV2Code.Allowed, reason);

public static ExecApprovalV2Result SecurityDeny(string reason)
=> new(ExecApprovalV2Code.SecurityDeny, reason);

Expand Down
Loading
Loading