Execute approved exec-approval commands as resolved argv (#799)

- Adds shell-free direct-argv execution payloads for approved exec-approval commands.
- Fails closed for unresolved executables, batch scripts, modified env wrappers, nested modifiers, and sandbox transports that cannot carry argv faithfully.
- Maintainer hardening added: timeout clamp, immutable payload collections, and canonical ToCommandRequest mapping.

Validation:
- Dual-model adversarial review: no blocking issues after maintainer hardening
- Autoreview: clean
- ./build.ps1
- dotnet test ./tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore
- dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --no-restore
- GitHub Build and Test checks passed, including test, e2e, win-x64, and win-arm64

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Author: AlexAlves87

src/OpenClaw.Shared/ExecApprovals/ExecApprovalV2Result.cs

@@ -1,5 +1,60 @@
+using System;
+using System.Collections.Generic;
+using System.Collections.ObjectModel;
+using System.Linq;
+using OpenClaw.Shared;
+
 namespace OpenClaw.Shared.ExecApprovals;
 
+/// <summary>
+/// What the caller must execute after an allow. <see cref="Argv"/> is the validated
+/// argument vector and <see cref="Env"/> is the sanitized environment built during
+/// evaluation. Neither may be re-derived from the raw request, and the argv must
+/// reach the process without a shell re-parsing it. The constructor takes defensive
+/// copies so the approved argv/env cannot be mutated through an aliased reference
+/// between approval and execution.
+/// </summary>
+public sealed record ExecApprovedExecution
+{
+    public const int MaxTimeoutMs = 600_000;
+
+    public IReadOnlyList<string> Argv { get; }
+    public string? Cwd { get; }
+    public int TimeoutMs { get; }
+    public IReadOnlyDictionary<string, string>? Env { get; }
+
+    public ExecApprovedExecution(
+        IReadOnlyList<string> argv,
+        string? cwd,
+        int timeoutMs,
+        IReadOnlyDictionary<string, string>? env)
+    {
+        ArgumentNullException.ThrowIfNull(argv);
+        if (argv.Count == 0)
+            throw new ArgumentException("Approved execution requires a non-empty argv.", nameof(argv));
+        if (timeoutMs <= 0)
+            throw new ArgumentOutOfRangeException(nameof(timeoutMs), timeoutMs, "Approved execution timeout must be positive.");
+
+        Argv = Array.AsReadOnly(argv.ToArray());
+        Cwd = cwd;
+        TimeoutMs = Math.Min(timeoutMs, MaxTimeoutMs);
+        Env = env is null
+            ? null
+            : new ReadOnlyDictionary<string, string>(
+                new Dictionary<string, string>(env, StringComparer.OrdinalIgnoreCase));
+    }
+
+    public CommandRequest ToCommandRequest() => new()
+    {
+        Argv = Argv,
+        Cwd = Cwd,
+        TimeoutMs = TimeoutMs,
+        Env = Env is null
+            ? null
+            : new Dictionary<string, string>(Env, StringComparer.OrdinalIgnoreCase),
+    };
+}
+
 /// <summary>
 /// Stable result codes for the V2 exec approval path.
 /// </summary>
@@ -25,10 +80,25 @@ public sealed class ExecApprovalV2Result
     public ExecApprovalV2Code Code { get; }
     public string Reason { get; }
 
-    private ExecApprovalV2Result(ExecApprovalV2Code code, string reason)
+    /// <summary>
+    /// The command to execute. Non-null only on <see cref="ExecApprovalV2Code.Allow"/>.
+    /// Carries the validated argv and sanitized env so the caller never re-derives
+    /// them from the raw request.
+    /// </summary>
+    public ExecApprovedExecution? Execution { get; }
+
+    private ExecApprovalV2Result(ExecApprovalV2Code code, string reason, ExecApprovedExecution? execution = null)
     {
+        // Invariant: Allow must carry a payload; non-Allow must not. A null payload
+        // on Allow (or a payload on a deny) is a bug, not a representable state.
+        if (code == ExecApprovalV2Code.Allow && execution is null)
+            throw new ArgumentNullException(nameof(execution), "Allow result requires an execution payload.");
+        if (code != ExecApprovalV2Code.Allow && execution is not null)
+            throw new ArgumentException("Non-allow result must not carry an execution payload.", nameof(execution));
+
         Code = code;
         Reason = reason;
+        Execution = execution;
     }
 
     public static ExecApprovalV2Result Unavailable(string reason = "Handler not available")
@@ -55,8 +125,16 @@ public static ExecApprovalV2Result ResolutionFailed(string reason)
     public static ExecApprovalV2Result InternalError(string reason)
         => new(ExecApprovalV2Code.InternalError, reason);
 
-    public static ExecApprovalV2Result Allow()
-        => new(ExecApprovalV2Code.Allow, "approved");
+    /// <summary>
+    /// Approve the command and carry the execution payload the caller must run.
+    /// An allow without a payload is not a valid state — there is intentionally
+    /// no parameterless allow: the approved argv must reach the process verbatim.
+    /// </summary>
+    public static ExecApprovalV2Result Allow(ExecApprovedExecution execution)
+    {
+        ArgumentNullException.ThrowIfNull(execution);
+        return new(ExecApprovalV2Code.Allow, "approved", execution);
+    }
 
     public bool IsAllow => Code == ExecApprovalV2Code.Allow;
 

src/OpenClaw.Shared/ExecApprovals/ExecApprovalsCoordinator.cs

@@ -100,13 +100,19 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
         if (pass1 is ExecHostPolicyDecision.AllowOutcome)
         {
             // Pre-approved path (security=Full, ask=Off or allowlist satisfied): skip prompt.
+            // Fail closed if the approved executable cannot be pinned to a resolved path.
+            var preApprovedExecution = BuildApprovedExecution(identity, sanitizedEnv);
+            if (preApprovedExecution is null)
+                return LogAndReturn(ExecApprovalV2Result.InternalError("unresolved-executable-on-allow"),
+                    correlationId, promptAttempted: false, fallbackUsed: false, canonical: context.DisplayCommand);
+
             // Side effects are best-effort: a metadata write failure must not flip an allow to a deny.
             try { await RecordAllowlistUsageAsync(context).ConfigureAwait(false); }
             catch (Exception ex) { _logger.Warn($"[EXEC-APPROVALS] [{correlationId}] side-effect: record-usage failed (non-fatal): {ex.Message}"); }
             _logger.Info($"[EXEC-APPROVALS] [{correlationId}] path=new " +
                 $"canonical=\"{SanitizeForLog(context.DisplayCommand)}\" decision=allow " +
                 $"reason=approved fallbackUsed=false promptAttempted=false");
-            return ExecApprovalV2Result.Allow();
+            return ExecApprovalV2Result.Allow(preApprovedExecution);
         }
         // RequiresPromptOutcome → continue to prompt/fallback block
 
@@ -186,7 +192,14 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
             _promptLock.Release();
         }
 
-        // Step 8: side effects — strictly after the final allow decision.
+        // Step 8: build payload before any store writes — a fail-closed payload result
+        // must not leave persistent allowlist state behind.
+        var execution = BuildApprovedExecution(identity, sanitizedEnv);
+        if (execution is null)
+            return LogAndReturn(ExecApprovalV2Result.InternalError("unresolved-executable-on-allow"),
+                correlationId, promptAttempted, fallbackUsed, canonical: context.DisplayCommand);
+
+        // Step 9: side effects — only reached when the payload is valid.
         // Each side effect is independently best-effort so a failure in one does not skip the other.
         if (persistAllowlistEntry && context.Security == ExecSecurity.Allowlist)
         {
@@ -196,13 +209,13 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
         try { await RecordAllowlistUsageAsync(context).ConfigureAwait(false); }
         catch (Exception ex) { _logger.Warn($"[EXEC-APPROVALS] [{correlationId}] side-effect: record-usage failed (non-fatal): {ex.Message}"); }
 
-        // Step 9: final allow log
+        // Step 10: final allow log
         _logger.Info($"[EXEC-APPROVALS] [{correlationId}] path=new " +
             $"canonical=\"{SanitizeForLog(context.DisplayCommand)}\" decision=allow " +
             $"reason=approved fallbackUsed={fallbackUsed} promptAttempted={promptAttempted}");
 
         // Step 10: return Allow
-        return ExecApprovalV2Result.Allow();
+        return ExecApprovalV2Result.Allow(execution);
         }
         catch (Exception ex)
         {
@@ -217,6 +230,50 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
         }
     }
 
+    // Builds the approved execution payload from the RESOLVED executable path, never
+    // the raw argv[0]. The command must execute with the same canonical identity it
+    // was evaluated under: a relative argv[0] in the payload would let Windows
+    // re-resolve it against PATH/cwd at execution time (a hijack), and the
+    // direct-argv runner rejects non-absolute executables anyway. Returns null when
+    // the executable could not be resolved to a path — the caller fails closed
+    // rather than execute a command whose identity we cannot pin.
+    internal static ExecApprovedExecution? BuildApprovedExecution(
+        CanonicalCommandIdentity identity,
+        IReadOnlyDictionary<string, string>? sanitizedEnv)
+    {
+        var resolvedPath = identity.Resolution?.ResolvedPath;
+        if (string.IsNullOrEmpty(resolvedPath))
+            return null;
+
+        // A batch script (.bat/.cmd) cannot run without cmd.exe, which re-parses the
+        // arguments and breaks the verbatim-argv guarantee. The direct-argv runner
+        // rejects these too; reject here as well so the fail-closed result is reached
+        // before any approval state is written, not after.
+        if (resolvedPath.EndsWith(".bat", StringComparison.OrdinalIgnoreCase)
+            || resolvedPath.EndsWith(".cmd", StringComparison.OrdinalIgnoreCase))
+            return null;
+
+        // If any env wrapper in the chain carries modifiers (VAR=val assignments or
+        // flags), the direct-argv payload cannot faithfully carry those semantics: the
+        // modifier would be silently dropped, and the process would run in a different
+        // environment than the one that was approved. This walks the full unwrap chain
+        // so a nested form such as `env env FOO=bar node` is caught, not just the outer
+        // wrapper. Fail closed rather than execute a command that differs from what was
+        // evaluated.
+        if (ExecEnvInvocationUnwrapper.AnyWrapperHasModifiers(identity.Command))
+            return null;
+
+        // Transparent env wrappers (no modifiers) are safe to unwrap: the inner
+        // command is the real executable and the args are preserved verbatim.
+        var effective = ExecEnvInvocationUnwrapper.UnwrapForResolution(identity.Command);
+        var argv = new string[effective.Count];
+        argv[0] = resolvedPath;
+        for (var i = 1; i < effective.Count; i++)
+            argv[i] = effective[i];
+
+        return new ExecApprovedExecution(argv, identity.Cwd, identity.TimeoutMs, sanitizedEnv);
+    }
+
     // Persists allowAlways patterns after an AllowAlways prompt decision (non-empty only).
     // Caller guarantees Security == Allowlist (guard is in HandleAsync step 8).
     private async Task PersistAllowlistEntriesAsync(ExecApprovalEvaluation context)

src/OpenClaw.Shared/ExecApprovals/ExecEnvInvocationUnwrapper.cs

@@ -81,6 +81,27 @@ internal static bool HasModifiers(IReadOnlyList<string> command)
         return false;
     }
 
+    // Returns true when any env wrapper in the full unwrap chain carries modifiers
+    // (VAR=val assignments or flags), including nested forms such as `env env FOO=bar node`.
+    // UnwrapForResolution strips every level for resolution, so checking only the outer
+    // wrapper would let an inner modifier slip through and be dropped from execution.
+    internal static bool AnyWrapperHasModifiers(IReadOnlyList<string> command)
+    {
+        var current = command;
+        for (var depth = 0; depth < MaxWrapperDepth; depth++)
+        {
+            if (current.Count == 0) break;
+            var token = current[0].Trim();
+            if (token.Length == 0) break;
+            if (!ExecCommandToken.IsEnv(token)) break;
+            if (HasModifiers(current)) return true;
+            var unwrapped = Unwrap(current);
+            if (unwrapped is null || unwrapped.Count == 0) break;
+            current = unwrapped;
+        }
+        return false;
+    }
+
     // Iteratively strips env wrappers for executable resolution only.
     internal static IReadOnlyList<string> UnwrapForResolution(IReadOnlyList<string> command)
     {

src/OpenClaw.Shared/ICommandRunner.cs

@@ -12,7 +12,15 @@ public class CommandRequest
 {
     /// <summary>The command to execute (e.g., "echo hello" or "Get-Process")</summary>
     public string Command { get; set; } = "";
-    
+
+    /// <summary>
+    /// When set, execute this argv directly with no shell between policy and the
+    /// process: FileName = Argv[0], the rest go through ProcessStartInfo.ArgumentList
+    /// verbatim. Takes precedence over Command/Args/Shell, which are ignored.
+    /// Null = legacy shell-wrapped path.
+    /// </summary>
+    public IReadOnlyList<string>? Argv { get; set; }
+
     /// <summary>Optional arguments array</summary>
     public string[]? Args { get; set; }