Add typed shell approval store schema 3 by Aaronontheweb · Pull Request #1890 · netclaw-dev/netclaw

Aaronontheweb · 2026-08-11T23:02:34Z

Summary

add closed schema-3 forms for Bash and PowerShell token prefixes, exact legacy shell grants, and exact non-shell grants
convert valid schema-2 stores under one cross-process lock with byte-identical backup, source CAS, atomic replace, and fail-closed status
carry ShellSyntaxTree 0.3.1 token facts through policy checks, one-time keys, actor state, protobuf events, and subagent prompts
preserve each complete command occurrence even when legacy verb projections are equal
update CLI, TUI, runbook, smoke tape, and OpenSpec artifacts for typed list, revoke, trust, and recovery behavior

Security boundary

ShellSyntaxTree supplies syntax facts; Netclaw owns authority and scope
no executable-private parser or command-specific grant parser was added
malformed, partial, future, or unavailable stores cannot authorize
schema-2 shell grants remain exact until the user approves a new token prefix
dynamic identities, redirects, path checks, shell boundaries, and hard-deny rules remain strict

Validation

adversarial review: PASS with no blocker or warning
Release build: 0 warnings, 0 errors
full suite: 6,983 passed; 15 expected platform or external-service skips
post-rebase focused suites: 414 passed
strict OpenSpec: both active changes pass
headers, PII scan, diff check, and native approvals tape pass
Slopwatch: one prior SW004 warning in PowerShellHostProbeTests; this PR does not touch that file

+        foreach (var segment in normalized[rootLength..].Split(
+                     '\\',
+                     StringSplitOptions.RemoveEmptyEntries))
+        {
+            if (segment == ".")
+                continue;
+
+            if (segment == "..")
+            {
+                if (segments.Count == 0)
+                    return null;
+
+                segments.RemoveAt(segments.Count - 1);
+                continue;
+            }
+
+            segments.Add(segment);
+        }


        foreach (var audience in audiences)
-            totalRemoved += store.RemoveAllForTool(audience, canonicalTool);
+        {
+            var change = store.TryRemoveAllForTool(audience, canonicalTool);
+            if (change is ApprovalStoreChangeResult.Unavailable unavailable)
+            {
+                WriteStoreError(unavailable.Failure, store, writer);
+                return 1;
+            }
+
+            totalRemoved += ((ApprovalStoreChangeResult.Completed)change).ChangeCount;
+        }


Aaronontheweb

Inline guide to the main authority boundaries in this pull request.

Aaronontheweb · 2026-08-12T01:54:20Z

+    /// <summary>
+    /// Creates a typed token-prefix shell entry.
+    /// </summary>
+    public static ApprovalEntry CreateTokenPrefix(


This factory creates new shell authority as immutable whole tokens. The separate LegacyExact factory prevents old free-form grants from gaining prefix authority.

Aaronontheweb · 2026-08-12T01:54:20Z

+    private static readonly HashSet<string> AllowedMembers =
+        ["shell", "match", "verbTokens", "verb", "directory", "createdAt"];
+
+    internal static ApprovalEntry ReadVersion3(JsonElement element)


This reader validates one complete version 3 entry shape. A mixed or malformed entry makes the store unavailable instead of salvaging nearby authority.

Aaronontheweb · 2026-08-12T01:54:20Z

+        if (context is not null &&
+            string.Equals(toolName, context.ShellToolName, StringComparison.Ordinal))
+        {
+            return ApprovalEntry.CreateLegacyExact(


Version 2 shell entries become LegacyExact. Migration preserves their prior exact authority and never turns them into token-prefix grants.

Aaronontheweb · 2026-08-12T01:54:20Z

+            sourceBytes,
+            contents);
+        LastMigrationOmittedEntryCount = omittedEntries;
+        UpdateCache(data, Encoding.UTF8.GetBytes(contents));


The store publishes its cache only after the version 3 replacement succeeds. A failed write cannot create authority that exists only in memory.

Aaronontheweb · 2026-08-12T01:54:20Z

+            return false;
+        }
+
+        for (var index = 0; index < entry.VerbTokens.Count; index++)


Token-prefix comparison uses whole parser tokens and the selected shell case rule. It does not use a raw string prefix.

Aaronontheweb · 2026-08-12T01:54:20Z

+                    return;
+                }
+
+                var change = _persistentStore.TryAddApprovals(


The actor writes all structured grants as one store operation. A partial persistent batch cannot survive a later entry failure.

Aaronontheweb · 2026-08-12T01:54:20Z

-            writer.WriteLine("the persisted tool-approvals.json directly.");
-            return 1;
+            var shell = opts.Shell ?? NativeShell;
+            if (!ShellApprovalPhraseParser.TryCreateTokenPrefix(


For shell tools, trust-verb asks ShellSyntaxTree for the canonical phrase. Extra operands, redirects, or shell effects cannot silently become broader reusable authority.

Aaronontheweb · 2026-08-12T01:54:20Z

                || finite.Values.Distinct(StringComparer.Ordinal).Count() != finite.Values.Count,
+            // ShellSyntaxTree proves these domains are bounded. They remain
+            // data only and cannot establish path or execution authority.
+            ShellValueDomain.IntegerRange => argument.Argument.IsPath,


ShellSyntaxTree 0.3.1 proves these scalar domains are bounded. Netclaw accepts them only as non-path data; paths, redirects, identities, and command payloads remain strict.

Aaronontheweb · 2026-08-12T02:05:59Z

TL;DR; the code for interpreting ShellSytnaxTree output was getting gross, so we formalized the policy layer into its own type system in order to make it easier to enforce and reason about.

Aaronontheweb

Not quite done reviewing but I have some immediate questions that will inform my next round

Aaronontheweb · 2026-08-12T02:07:08Z

 {
+    public static IReadOnlyList<ToolApprovalGrant> BuildGrants(
+        IReadOnlyList<ApprovalCandidate> candidates,
+        bool persistent,


should make these value objects to prevent primitive confusion

Fixed in . now receives one . The context carries the reusable decision, working directory, and session directory. Its factory rejects , , and , so callers cannot swap Boolean flags or create a reusable grant from a non-reusable decision.

Aaronontheweb · 2026-08-12T02:08:06Z

+    InvalidData = 1,
+
+    /// <summary>The file used a schema newer than this process supports.</summary>
+    UnsupportedVersion = 2,


Do we have a dedicated schema file for approvals?

No dedicated JSON Schema file exists today. and are the executable schema. They reject unknown members, mixed entry forms, duplicate keys, bad enums, invalid paths, and spoof characters before any authority becomes available. A separate JSON Schema could help editors, but it would duplicate the security contract unless tests generated or compared it against the executable reader. I left that out of this PR.

Aaronontheweb · 2026-08-12T02:08:55Z

+            {
+                var powerShell7 = ParsePowerShell(source, PwshDialect.PowerShell7);
+                var windowsPowerShell = ParsePowerShell(source, PwshDialect.WindowsPowerShell51);
+                if (!TryCreateTokenPrefix(


attempt both powershells - take the results of the newer one if it's available.

Fixed in . An abstract PowerShell grant now tries both dialects. It uses a valid PowerShell 7 result first and uses Windows PowerShell 5.1 only when the preferred parse fails. The resolved-environment overload still uses only the selected installed host dialect. Tests cover the PowerShell 7 preference with and the 5.1 fallback with .

Aaronontheweb · 2026-08-12T02:10:37Z

+        }
+    }
+
+    private static bool TryCreateTokenPrefix(


not sure why this method needs to exist - couldn't we just jump straight to TryCreateTokenPrefixCore ?

Agreed. The private wrapper duplicated exception handling that the two public entry points already provide. removes it and calls directly.

Aaronontheweb · 2026-08-12T02:11:36Z

+            parsed.Syntax.Statements.Count != 1 ||
+            parsed.Syntax.Statements[0] is not SimpleCommandSyntax simple)
+        {
+            error = "The shell phrase must contain one complete static command.";


I'll need to find this upstream, but who does the parsing into multiple commands if they're strung together? Or is that not the shape being extracted by the ParsedCommand output here?

does contain every executable occurrence from a complete shell program. This type deliberately rejects that shape because it creates one stored grant from one operator phrase. Runtime commands use , which iterates every . I renamed this type to and clarified that boundary in its public summary.

Aaronontheweb · 2026-08-12T02:12:40Z

+            clause.Elements.Any(static element =>
+                element.Role != ClauseElementRole.Verb || element.IsFlag))
+        {
+            error = "The shell phrase must have no argument, flag, assignment, redirect, or control effect.";


the idea being that you can't have any of these positionals BEFORE the first command? Are there any valid counter-examples?

Valid shell commands can have an assignment or redirect before the executable, such as or . This grant parser rejects them intentionally. must store exactly the phrase the operator entered, with no extra shell effect. Runtime command analysis still supports those legal forms through .

Aaronontheweb · 2026-08-12T02:13:13Z

+        var canonicalSource = string.Join(" ", tokens);
+        if (!string.Equals(source, canonicalSource, StringComparison.Ordinal))
+        {
+            error = $"The shell phrase must equal its canonical form: {canonicalSource}";


are we sure this holds up under all valid / legal conditions?

No. This equality is intentionally stricter than the complete shell language. Legal quotes, escapes, aliases, extra whitespace, arguments, redirects, and assignments can fail here. The CLI accepts only one canonical phrase because a silent reduction could create broader authority than the operator entered. The renamed type and summary now state this narrow contract.

Aaronontheweb · 2026-08-12T02:13:44Z

+            return false;
+        }
+
+        entry = ApprovalEntry.CreateTokenPrefix(shell, tokens);


again, where does this get called n times for each valid command in a command string? This looks like a per-verb function

It is not called once per runtime command. It creates one stored grant for Usage: netclaw approvals trust-verb [--audience personal|team|public] [--tool ]

Adds a global-wildcard '(verb, null)' approval entry — the verb runs in any cwd
without prompting. Used to pre-approve verbs for unattended/scheduled tasks. and reconstructs one legacy compatibility pattern. Runtime analysis calls , which iterates once per occurrence. The old name hid this difference, so renames it to .

Aaronontheweb · 2026-08-12T02:14:30Z

-                argument.Argument.Kind != ArgKind.Glob
-                && (Environment.Grammar != ShellGrammar.Bash
-                    || argument.Argument.Kind != ArgKind.EnvVar),
+            ShellValueDomain.Unknown => argument.Argument.Kind != ArgKind.Glob,


we tolerate globs as the only "unknown" type of domain

Yes. Raw authored globs are the only value domain accepted here. Netclaw does not trust that unknown value directly. The parser-owned glob source must pass the separate covering-directory and descendant-glob checks. Every other value remains complex.

Aaronontheweb

Left some more comments - still going through this

Aaronontheweb · 2026-08-12T14:46:33Z

+    /// <summary>
+    /// Creates an exact non-shell approval entry.
+    /// </summary>
+    public static ApprovalEntry CreateNonShell(


Non-shell is for things like MCP servers or file access, which can require approval too

Aaronontheweb · 2026-08-12T14:47:43Z

+        {
+            Shell = shell,
+            Match = ApprovalMatchKind.TokenPrefix,
+            VerbTokens = Array.AsReadOnly(verbTokens.ToArray()),


keep the original verbTokens and only string.Join it once IMHO.

Aaronontheweb · 2026-08-12T14:48:06Z

+    public static ApprovalEntry CreateLegacyExact(
+        ApprovalShell shell,
+        string verb,
+        string? directory = null,


some commands have global approval, so not tied to a specific directory

+        foreach (var candidate in _armed)
+        {
+            if (!HasSameExecutionSemantics(candidate.Call, call)
+                || !_consumed.TryAdd(candidate, 0))
+                continue;
+
+            key = candidate;
+            return true;
+        }


+        foreach (var candidate in candidates)
+        {
+            if (candidate.Directory is null)
+                continue;
+
+            if (!IsSafeTemporaryPath(candidate.Directory))
+                return false;
+        }


+            foreach (var argument in command.Clause.Args)
+            {
+                if (!argument.IsPath && !argument.IsCwdAttribution)
+                    continue;
+
+                if (string.IsNullOrWhiteSpace(argument.Resolved)
+                    || !IsSafeTemporaryPath(argument.Resolved))
+                {
+                    return false;
+                }
+            }


+        foreach (var candidate in candidates)
+        {
+            if (string.IsNullOrWhiteSpace(candidate.Verb) || !_safeVerbs.Contains(candidate.Verb))
+                return false;
+        }


+        foreach (var segment in path.Split(separator, StringSplitOptions.RemoveEmptyEntries))
+        {
+            if (segment == ".")
+                continue;
+
+            if (segment == "..")
+            {
+                if (segments.Count == 0)
+                    return string.Empty;
+
+                segments.RemoveAt(segments.Count - 1);
+                continue;
+            }
+
+            segments.Add(segment);
+        }


+            foreach (var redirect in command.Redirects)
+            {
+                var safe = redirect switch
+                {
+                    FileRedirectAnalysis file => HasSafeRedirectTarget(file.Target),
+                    DescriptorDuplicateRedirectAnalysis => true,
+                    DescriptorMoveRedirectAnalysis => true,
+                    DescriptorCloseRedirectAnalysis => true,
+                    HereDocumentRedirectAnalysis => true,
+                    HereStringRedirectAnalysis => true,
+                    _ => false
+                };
+                if (!safe)
+                {
+                    return false;
+                }
+            }


+        foreach (var candidate in candidates)
+        {
+            var effectiveDirectory = candidate.Directory ?? fullCwd;
+            if (string.IsNullOrWhiteSpace(effectiveDirectory))
+                return false;
+
+            string fullDirectory;
+            try
+            {
+                fullDirectory = Path.GetFullPath(effectiveDirectory);
+            }
+            catch (Exception ex) when (ex is ArgumentException or NotSupportedException or PathTooLongException)
+            {
+                return false;
+            }
+
+            if (!PathUtility.IsWithinRoot(fullDirectory, fullCwd)
+                || PathUtility.ContainsSymlinkSegment(fullCwd, fullDirectory))
+            {
+                return false;
+            }
+        }


* Sync delta specs for the last two completed changes Apply the redirect-shared-temp-to-session-scratch delta to session-cwd and tool-approval-gates. The OpenSpec CLI applied both files. Merge the surface-reminder-schedule-failures delta into netclaw-scheduling by hand. PR #2024 already corrected that requirement against the merged code, so a programmatic apply would undo those corrections. Keep the corrected text and add the scheduling-failure content: - An execution failure and a scheduling failure share one ConsecutiveFailures count. - A successful reschedule alone does not reset the count. - An unattended reschedule site emits ReminderScheduleFailed at Warning severity, and ReminderAutoDisabled at Critical severity on the threshold. - The set_reminder create path and update path return the error to the caller and emit no alert. Drop three stale delta claims. The merged code disables a reminder. It does not set a "paused" status and it does not write tasks.json. ReminderExecutionActor has no ExecutionTimeoutSeconds constant, so the one-hour absolute limit stands. * Archive the last two completed OpenSpec changes Move each change to openspec/changes/archive/2026-08-19-<name>/. The code of both changes is merged on dev. - surface-reminder-schedule-failures: PR #1886. - redirect-shared-temp-to-session-scratch: PRs #1890, #1925, #1926, #1945, and #1956. Tick task 4.3 of redirect-shared-temp-to-session-scratch. The full pr_validation suite ran green on Windows against current dev. Leave the other boxes unticked. The maintainer accepts tasks 4.5 and 4.6 of surface-reminder-schedule-failures as guaranteed by construction. The maintainer accepts task 6.4 of that change and task 4.4 of redirect-shared-temp-to-session-scratch as an eval-credentials gap.

Aaronontheweb enabled auto-merge (squash) August 11, 2026 23:02

github-code-quality Bot found potential problems Aug 11, 2026

View reviewed changes

Comment thread src/Netclaw.Cli.Tests/Approvals/ApprovalsCommandTests.cs Fixed

Comment thread src/Netclaw.Cli.Tests/Tui/ApprovalsManagerPageTests.cs Fixed

Comment thread src/Netclaw.Configuration.Tests/ToolApprovalStoreTests.cs Fixed

Aaronontheweb force-pushed the feat/structured-shell-approval-policy branch from 7946956 to 2bdce84 Compare August 12, 2026 01:53

Aaronontheweb commented Aug 12, 2026

View reviewed changes

Aaronontheweb disabled auto-merge August 12, 2026 02:06

Aaronontheweb added the shell Issues related to the shell tool, since it has the largest security perimeter. label Aug 12, 2026

Aaronontheweb commented Aug 12, 2026

View reviewed changes

Aaronontheweb mentioned this pull request Aug 12, 2026

Extend approval grants into typed allow, prompt, and deny rules #1898

Open

github-code-quality Bot found potential problems Aug 12, 2026

View reviewed changes

Comment thread src/Netclaw.Configuration.Tests/ApprovalEntryWireCodecTests.cs Fixed

Aaronontheweb commented Aug 12, 2026

View reviewed changes

Aaronontheweb mentioned this pull request Aug 13, 2026

evals: score MCP prompt activation from structured per-run events #1910

Open

Aaronontheweb force-pushed the feat/structured-shell-approval-policy branch from b288541 to 1d1f8a6 Compare August 13, 2026 00:22

github-code-quality Bot found potential problems Aug 13, 2026

View reviewed changes

Aaronontheweb mentioned this pull request Aug 13, 2026

build: pin patched SSH.NET transitive dependency #1911

Closed

Aaronontheweb added 9 commits August 13, 2026 00:31

feat(approvals): add typed schema 3 policy

ccfe4cb

test(approvals): use native shell fixtures

508510e

fix(approvals): accept bounded shell data

8916bbb

refactor(approvals): clarify grant creation

c303a37

refactor(approvals): source generate store JSON

09d6bce

Guide temporary work into session scratch

3bb3d65

Preserve shell API and canonicalize temp roots

6c9afd2

Preserve scratch guidance across recovery

084265e

Preserve subagent scratch denial guidance

f83b3b7

Aaronontheweb force-pushed the feat/structured-shell-approval-policy branch from 1d1f8a6 to f83b3b7 Compare August 13, 2026 00:31

fix(approvals): canonicalize platform temp advice

e35444c

Aaronontheweb merged commit b183288 into dev Aug 13, 2026
23 checks passed

Aaronontheweb deleted the feat/structured-shell-approval-policy branch August 13, 2026 01:40

Aaronontheweb mentioned this pull request Aug 13, 2026

fix(evals): capture stderr separately so diagnostics cannot corrupt the json envelope #1896

Merged

This was referenced Aug 13, 2026

test: bind shell approval policy evidence #1912

Merged

feat(sessions): resume the turn after a mid-stream LLM timeout #1893

Open

Archive completed OpenSpec changes and sync their specs #2024

Merged

Archive the last two completed OpenSpec changes #2031

Merged

Uh oh!

Conversation

Aaronontheweb commented Aug 11, 2026

Summary

Security boundary

Validation

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Aaronontheweb commented Aug 12, 2026

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!