fix(exec): migrate approvals to custom state dir (#744)

vincentkoc · web-flow · commit 913ba4e8f504 · 2026-06-11T17:35:34.000+09:00
diff --git a/src/OpenClaw.Shared/ExecApprovals/ExecApprovalEvaluation.cs b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalEvaluation.cs
@@ -24,6 +24,8 @@ public sealed class ExecApprovalEvaluation
     public IReadOnlyList<string> AllowAlwaysPatterns { get; }
     public IReadOnlyList<ExecAllowlistEntry> AllowlistMatches { get; }
 
+    public bool AllAllowlistResolutionsMatched { get; }
+
     // true iff security==allowlist && resolutions.Count>0 && matches.Count==resolutions.Count.
     // Research doc 06 derivation rule — must not be re-derived outside the constructor.
     public bool AllowlistSatisfied { get; }
@@ -70,9 +72,9 @@ public ExecApprovalEvaluation(
 
         Resolution = allowlistResolutions.Count > 0 ? allowlistResolutions[0] : (ExecCommandResolution?)null;
 
-        AllowlistSatisfied = security == ExecSecurity.Allowlist
-            && allowlistResolutions.Count > 0
+        AllAllowlistResolutionsMatched = allowlistResolutions.Count > 0
             && allowlistMatches.Count == allowlistResolutions.Count;
+        AllowlistSatisfied = security == ExecSecurity.Allowlist && AllAllowlistResolutionsMatched;
 
         AllowlistMatch = AllowlistSatisfied ? allowlistMatches[0] : null;
     }
diff --git a/src/OpenClaw.Shared/ExecApprovals/ExecApprovalsContracts.cs b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalsContracts.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Text.Json;
 using System.Text.Json.Serialization;
 
 namespace OpenClaw.Shared.ExecApprovals;
@@ -52,15 +53,17 @@ public sealed class ExecApprovalsDefaults
 {
     public ExecSecurity? Security { get; set; }
     public ExecAsk? Ask { get; set; }
-    public ExecAsk? AskFallback { get; set; }
+    [JsonConverter(typeof(ExecSecurityFallbackConverter))]
+    public ExecSecurity? AskFallback { get; set; }
     public bool? AutoAllowSkills { get; set; }
 }
 
 public sealed class ExecApprovalsAgent
 {
     public ExecSecurity? Security { get; set; }
     public ExecAsk? Ask { get; set; }
-    public ExecAsk? AskFallback { get; set; }
+    [JsonConverter(typeof(ExecSecurityFallbackConverter))]
+    public ExecSecurity? AskFallback { get; set; }
     public bool? AutoAllowSkills { get; set; }
     public List<ExecAllowlistEntry>? Allowlist { get; set; }
 }
@@ -73,13 +76,45 @@ public sealed class ExecApprovalsFile
     public Dictionary<string, ExecApprovalsAgent>? Agents { get; set; }
 }
 
+internal sealed class ExecSecurityFallbackConverter : JsonConverter<ExecSecurity?>
+{
+    public override ExecSecurity? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
+    {
+        if (reader.TokenType == JsonTokenType.Null) return null;
+        if (reader.TokenType != JsonTokenType.String) throw new JsonException("askFallback must be a string");
+        return reader.GetString()?.ToLowerInvariant() switch
+        {
+            "deny" or "always" => ExecSecurity.Deny,
+            "allowlist" or "on-miss" => ExecSecurity.Allowlist,
+            "full" or "off" => ExecSecurity.Full,
+            var value => throw new JsonException($"Unsupported askFallback value: {value}"),
+        };
+    }
+
+    public override void Write(Utf8JsonWriter writer, ExecSecurity? value, JsonSerializerOptions options)
+    {
+        if (value is null)
+        {
+            writer.WriteNullValue();
+            return;
+        }
+        writer.WriteStringValue(value.Value switch
+        {
+            ExecSecurity.Deny => "deny",
+            ExecSecurity.Allowlist => "allowlist",
+            ExecSecurity.Full => "full",
+            _ => throw new JsonException($"Unsupported askFallback value: {value}"),
+        });
+    }
+}
+
 // ── Resolved/runtime contracts (not serialized) ───────────────────────────────
 
 public sealed class ExecApprovalsResolvedDefaults
 {
     public ExecSecurity Security { get; init; }
     public ExecAsk Ask { get; init; }
-    public ExecAsk AskFallback { get; init; }
+    public ExecSecurity AskFallback { get; init; }
     public bool AutoAllowSkills { get; init; }
 }
 
diff --git a/src/OpenClaw.Shared/ExecApprovals/ExecApprovalsCoordinator.cs b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalsCoordinator.cs
@@ -75,7 +75,9 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
         }
 
         var sanitizedEnv = envResult.Allowed as IReadOnlyDictionary<string, string>;
-        IReadOnlyList<ExecAllowlistEntry> matches = resolved.Defaults.Security == ExecSecurity.Allowlist
+        var needsAllowlistMatches = resolved.Defaults.Security == ExecSecurity.Allowlist
+            || resolved.Defaults.AskFallback == ExecSecurity.Allowlist;
+        IReadOnlyList<ExecAllowlistEntry> matches = needsAllowlistMatches
             ? ExecAllowlistMatcher.MatchAll(resolved.Allowlist, identity.AllowlistResolutions)
             : [];
 
@@ -208,16 +210,16 @@ public async Task<ExecApprovalV2Result> HandleAsync(NodeInvokeRequest request, s
     // ask=Always → Deny: human approval is a precondition; without UI the only safe outcome is deny.
     private static ExecApprovalDecision FallbackDecision(
         ExecApprovalEvaluation context,
-        ExecAsk askFallback)
+        ExecSecurity askFallback)
     {
-        return askFallback switch
+        var effectiveFallback = (ExecSecurity)Math.Min((int)context.Security, (int)askFallback);
+        return effectiveFallback switch
         {
-            ExecAsk.Off => ExecApprovalDecision.AllowOnce,
-            ExecAsk.OnMiss => context.AllowlistSatisfied
+            ExecSecurity.Full => ExecApprovalDecision.AllowOnce,
+            ExecSecurity.Allowlist => context.AllAllowlistResolutionsMatched
                 ? ExecApprovalDecision.AllowOnce
                 : ExecApprovalDecision.Deny,
-            ExecAsk.Always => ExecApprovalDecision.Deny,
-            ExecAsk.Deny => ExecApprovalDecision.Deny,
+            ExecSecurity.Deny => ExecApprovalDecision.Deny,
             _ => ExecApprovalDecision.Deny,  // defensive
         };
     }
diff --git a/src/OpenClaw.Shared/ExecApprovals/ExecApprovalsStore.cs b/src/OpenClaw.Shared/ExecApprovals/ExecApprovalsStore.cs
@@ -24,9 +24,17 @@ public sealed class ExecApprovalsStore
     };
 
     private readonly string _filePath;
+    private readonly string? _legacyFilePath;
     private readonly IOpenClawLogger _logger;
     private readonly SemaphoreSlim _lock = new(1, 1);
 
+    private enum LegacyMigrationStatus
+    {
+        NotNeeded,
+        Migrated,
+        Blocked,
+    }
+
     private enum LoadFileStatus
     {
         Missing,
@@ -37,8 +45,31 @@ private enum LoadFileStatus
     private readonly record struct LoadFileResult(LoadFileStatus Status, ExecApprovalsFile? File);
 
     public ExecApprovalsStore(string dataPath, IOpenClawLogger logger)
+        : this(
+            dataPath,
+            logger,
+            Environment.GetEnvironmentVariable("OPENCLAW_STATE_DIR"),
+            Environment.GetEnvironmentVariable("OPENCLAW_HOME"),
+            FirstUsablePathValue(
+                Environment.GetEnvironmentVariable("HOME"),
+                Environment.GetEnvironmentVariable("USERPROFILE"),
+                Environment.GetFolderPath(Environment.SpecialFolder.UserProfile)))
     {
-        _filePath = Path.Combine(dataPath, "exec-approvals.json");
+    }
+
+    internal ExecApprovalsStore(
+        string dataPath,
+        IOpenClawLogger logger,
+        string? stateDirOverride,
+        string? openClawHomeOverride = null,
+        string? osHomeOverride = null)
+    {
+        var stateDir = string.IsNullOrWhiteSpace(stateDirOverride)
+            ? dataPath
+            : ResolveStateDirPath(stateDirOverride, openClawHomeOverride, osHomeOverride);
+        _filePath = Path.Combine(stateDir, "exec-approvals.json");
+        var legacyFilePath = Path.Combine(dataPath, "exec-approvals.json");
+        _legacyFilePath = PathsEqual(_filePath, legacyFilePath) ? null : legacyFilePath;
         _logger = logger;
     }
 
@@ -47,6 +78,9 @@ public ExecApprovalsStore(string dataPath, IOpenClawLogger logger)
     // No side effects; does not create the file. Used by the evaluator (PR5).
     public ExecApprovalsResolved ResolveReadOnly(string? agentId)
     {
+        if (_legacyFilePath is not null && File.Exists(_legacyFilePath) && !File.Exists(_filePath))
+            return UnmigratedLegacyFallback(agentId);
+
         var result = LoadFile();
         return result.Status != LoadFileStatus.Loaded || result.File is null
             ? DefaultResolved(NormalizeAgentId(agentId))
@@ -69,14 +103,19 @@ public async Task<ExecApprovalsResolved> ResolveAsync(string? agentId)
         }
     }
 
+    public void MigrateLegacyFileIfNeeded() => TryMigrateLegacyFile();
+
     // ── File I/O ──────────────────────────────────────────────────────────────
 
     private LoadFileResult LoadFile()
+        => LoadFile(_filePath);
+
+    private LoadFileResult LoadFile(string filePath)
     {
-        if (!File.Exists(_filePath)) return new LoadFileResult(LoadFileStatus.Missing, null);
+        if (!File.Exists(filePath)) return new LoadFileResult(LoadFileStatus.Missing, null);
         try
         {
-            var json = File.ReadAllText(_filePath);
+            var json = File.ReadAllText(filePath);
             var file = JsonSerializer.Deserialize<ExecApprovalsFile>(json, JsonOptions);
             if (file is null)
             {
@@ -105,6 +144,9 @@ private LoadFileResult LoadFile()
 
     private async Task<ExecApprovalsFile> EnsureFileAsync()
     {
+        if (TryMigrateLegacyFile() == LegacyMigrationStatus.Blocked)
+            return UnmigratedLegacyFallbackFile();
+
         var result = LoadFile();
         if (result.Status == LoadFileStatus.Loaded && result.File is not null)
         {
@@ -136,6 +178,118 @@ private async Task<ExecApprovalsFile> EnsureFileAsync()
         return newFile;
     }
 
+    private LegacyMigrationStatus TryMigrateLegacyFile()
+    {
+        if (_legacyFilePath is null || !File.Exists(_legacyFilePath) || File.Exists(_filePath))
+            return LegacyMigrationStatus.NotNeeded;
+
+        var legacyResult = LoadFile(_legacyFilePath);
+        if (legacyResult.Status != LoadFileStatus.Loaded || legacyResult.File is null)
+        {
+            _logger.Warn($"[EXEC-APPROVALS] Legacy approvals at {_legacyFilePath} could not be migrated; applying default-deny without creating {_filePath}");
+            return LegacyMigrationStatus.Blocked;
+        }
+
+        var targetDir = Path.GetDirectoryName(_filePath)!;
+        var archivePath = NextArchivePath(_legacyFilePath);
+        var tempPath = Path.Combine(targetDir, $".exec-approvals-migration-{Guid.NewGuid():N}.tmp");
+        try
+        {
+            Directory.CreateDirectory(targetDir);
+            var data = File.ReadAllBytes(_legacyFilePath);
+            using (var stream = new FileStream(tempPath, FileMode.CreateNew, FileAccess.Write, FileShare.None))
+            {
+                stream.Write(data);
+                stream.Flush(flushToDisk: true);
+            }
+            File.Move(tempPath, _filePath);
+            try
+            {
+                File.Move(_legacyFilePath, archivePath);
+            }
+            catch (Exception ex)
+            {
+                _logger.Warn($"[EXEC-APPROVALS] Migrated approvals to {_filePath}, but could not archive {_legacyFilePath} ({ex.Message})");
+                return LegacyMigrationStatus.Migrated;
+            }
+            _logger.Info($"[EXEC-APPROVALS] Migrated {_legacyFilePath} to {_filePath}; archived source as {archivePath}");
+            return LegacyMigrationStatus.Migrated;
+        }
+        catch (IOException) when (File.Exists(_filePath))
+        {
+            try { if (File.Exists(tempPath)) File.Delete(tempPath); } catch { }
+            return LegacyMigrationStatus.NotNeeded;
+        }
+        catch (Exception ex)
+        {
+            try { if (File.Exists(tempPath)) File.Delete(tempPath); } catch { }
+            _logger.Warn($"[EXEC-APPROVALS] Failed to migrate {_legacyFilePath} to {_filePath} ({ex.Message}); applying default-deny without creating a replacement file");
+            return LegacyMigrationStatus.Blocked;
+        }
+    }
+
+    private static string NextArchivePath(string legacyFilePath)
+    {
+        var archivePath = $"{legacyFilePath}.migrated";
+        return File.Exists(archivePath) ? $"{archivePath}-{Guid.NewGuid():N}" : archivePath;
+    }
+
+    private static bool PathsEqual(string left, string right) =>
+        string.Equals(Path.GetFullPath(left), Path.GetFullPath(right), StringComparison.OrdinalIgnoreCase);
+
+    private static string ResolveStateDirPath(
+        string stateDirOverride,
+        string? openClawHomeOverride,
+        string? osHomeOverride)
+    {
+        var osHome = NormalizePathValue(osHomeOverride) ?? Environment.CurrentDirectory;
+        var openClawHome = NormalizePathValue(openClawHomeOverride);
+        var effectiveHome = openClawHome is null
+            ? Path.GetFullPath(osHome)
+            : Path.GetFullPath(ExpandHomePrefix(openClawHome, osHome));
+        return Path.GetFullPath(ExpandHomePrefix(stateDirOverride.Trim(), effectiveHome));
+    }
+
+    private static string ExpandHomePrefix(string path, string home) =>
+        path == "~"
+            ? home
+            : path.StartsWith($"~{Path.DirectorySeparatorChar}", StringComparison.Ordinal)
+                || path.StartsWith($"~{Path.AltDirectorySeparatorChar}", StringComparison.Ordinal)
+                ? Path.Combine(home, path[2..])
+                : path;
+
+    private static string? NormalizePathValue(string? value)
+    {
+        var trimmed = value?.Trim();
+        return string.IsNullOrEmpty(trimmed) || trimmed is "undefined" or "null" ? null : trimmed;
+    }
+
+    private static string? FirstUsablePathValue(params string?[] values)
+    {
+        foreach (var value in values)
+        {
+            var normalized = NormalizePathValue(value);
+            if (normalized is not null) return normalized;
+        }
+        return null;
+    }
+
+    private static ExecApprovalsFile UnmigratedLegacyFallbackFile() =>
+        new()
+        {
+            Version = 1,
+            Defaults = new ExecApprovalsDefaults
+            {
+                Security = ExecSecurity.Deny,
+                Ask = ExecAsk.Always,
+                AskFallback = ExecSecurity.Deny,
+            },
+            Agents = [],
+        };
+
+    private static ExecApprovalsResolved UnmigratedLegacyFallback(string? agentId) =>
+        ResolveFromFile(UnmigratedLegacyFallbackFile(), agentId);
+
     private async Task SaveFileAsync(ExecApprovalsFile file)
     {
         var dir = Path.GetDirectoryName(_filePath)!;
@@ -276,7 +430,7 @@ private static ExecApprovalsResolved ResolveFromFile(ExecApprovalsFile file, str
         // Cascade: agentEntry → wildcard → defaults → systemDefault
         var security = agentEntry?.Security ?? wildcardEntry?.Security ?? defaults?.Security ?? ExecSecurity.Deny;
         var ask = agentEntry?.Ask ?? wildcardEntry?.Ask ?? defaults?.Ask ?? ExecAsk.OnMiss;
-        var askFallback = agentEntry?.AskFallback ?? wildcardEntry?.AskFallback ?? defaults?.AskFallback ?? ExecAsk.Deny;
+        var askFallback = agentEntry?.AskFallback ?? wildcardEntry?.AskFallback ?? defaults?.AskFallback ?? ExecSecurity.Deny;
         var autoAllowSkills = agentEntry?.AutoAllowSkills ?? wildcardEntry?.AutoAllowSkills ?? defaults?.AutoAllowSkills ?? false;
 
         // Allowlist: wildcard first, then agent; then normalize dropInvalid=true.
@@ -307,7 +461,7 @@ private static ExecApprovalsResolved DefaultResolved(string agentId) =>
             {
                 Security = ExecSecurity.Deny,
                 Ask = ExecAsk.OnMiss,
-                AskFallback = ExecAsk.Deny,
+                AskFallback = ExecSecurity.Deny,
                 AutoAllowSkills = false,
             },
             Allowlist = [],
diff --git a/src/OpenClaw.Tray.WinUI/Services/NodeService.cs b/src/OpenClaw.Tray.WinUI/Services/NodeService.cs
@@ -7,6 +7,7 @@
 using Microsoft.UI.Dispatching;
 using OpenClaw.Shared;
 using OpenClaw.Shared.Capabilities;
+using OpenClaw.Shared.ExecApprovals;
 using OpenClaw.Shared.Mcp;
 using OpenClaw.Shared.Mxc;
 using OpenClawTray.A2UI.Actions;
@@ -267,6 +268,8 @@ public async Task DisconnectAsync()
     
     private void RegisterCapabilities()
     {
+        new ExecApprovalsStore(_dataPath, _logger).MigrateLegacyFileIfNeeded();
+
         // Hold the lock across the entire rebuild. The body is sync construction
         // (no awaits), so the lock is held briefly and an MCP tools/list arriving
         // mid-rebuild waits for a consistent snapshot rather than seeing a half-
diff --git a/tests/OpenClaw.Shared.Tests/ExecApprovalsCoordinatorTests.cs b/tests/OpenClaw.Shared.Tests/ExecApprovalsCoordinatorTests.cs
diff --git a/tests/OpenClaw.Shared.Tests/ExecApprovalsStoreTests.cs b/tests/OpenClaw.Shared.Tests/ExecApprovalsStoreTests.cs
