fix(approvals): stop fabricating phantom approval directory scopes (#1795) by Aaronontheweb · Pull Request #1799 · netclaw-dev/netclaw

Aaronontheweb · 2026-08-07T18:09:56Z

Summary

Fixes #1795. The shell approval matcher made phantom directory scopes from two
token shapes. Each phantom scope inflated the approval header to "Approve in N
directories?" and forced a re-prompt.

This PR started as tests only. It now ships the production fix. The regression
tests pass, plus coverage and security-hardening tests.

Root cause

ShellSyntaxTree 0.2 misclassifies two token shapes:

echo "---try /stats?format=json---" holds a ?. The parser tags the text
operand as a glob arg. The matcher then makes a covering directory from the
static prefix (---try).
head -c 2000 holds a bare number. The parser tags 2000 as a path arg. The
matcher then resolves it to /cwd/2000.

The fix

Two narrow guards in src/Netclaw.Security/IToolApprovalMatcher.cs:

A pure side-effect verb (echo, printf, :, true, false) derives no
arg-based directory scope. It writes only to stdout. Redirect handling stays
intact, so echo x > dir/file keeps dir.
A bare all-digit operand is dropped as a numeric value, but only after an
existence probe (see below).

A comment marks each guard as temporary containment. ShellSyntaxTree v0.3.0 fixes
the misclassification in the parser. Remove the guards then.

Security hardening (adversarial review)

The first numeric guard was purely syntactic: it dropped any all-digit token.
That could not tell the value in head -c 2000 from a real filesystem object
named 2000. It opened a prompt-to-auto-approve hole on the ACL perimeter:

cat 2000, where 2000 is a symlink out of the granted tree, plus a folder
grant (cat, <cwd>), dropped the path scope. The candidate collapsed to the
cwd. The symlink-segment check in MatchesShellApproval never ran. The read
auto-approved with no prompt.

The guard now gates on filesystem existence:

It drops the all-digit token only when no entry exists at its resolved path.
A real file, directory, or symlink named 2000 stays a path arg, so its scope
and the symlink-segment check survive. File.Exists/Directory.Exists follow
a symlink to its target, so a symlink to an outside secret returns true and is
kept.
SAFE-FAIL: if the path is unknown or the probe throws, the token stays a path,
so the gate prompts. The guard never fails toward an automatic grant.

Corrected security reasoning

An earlier version of this description claimed the change "neither grants,
widens, nor hides authorization." That was wrong for the first numeric guard,
which could hide a real path's symlink check. The corrected position:

The side-effect skip removes only a fabricated arg scope from echo/printf.
The candidate returns to Directory == null, the intended pure-side-effect
state. The verb writes only to stdout. A redirect still produces a real scope
through the untouched redirect path.
The existence-gated numeric guard drops a scope only when the matcher proves
no filesystem object exists at the path. It cannot drop a real path, so it
cannot hide that path's symlink-segment check. head -c 2000 with no file
named 2000 still drops to Directory == null and fixes the prompt spam.
IsMessy reads globs through ResolveGlobCoveringDirectory, not through the
two guarded paths, so its behavior does not change.

Tests

Two named Approval prompt shows phantom "N directories" scopes for commands with ? in echo text or bare numeric args #1795 regression tests pass:
ExtractCandidates_echo_text_question_mark_is_not_a_glob_scope,
ExtractCandidates_bare_numeric_operand_is_not_a_path_scope.
Security regression:
IsApproved_numeric_token_that_names_an_escaping_symlink_still_prompts asserts
IsApproved is false for the symlink attack.
Complement:
IsApproved_numeric_token_that_names_a_real_in_tree_directory_is_covered_by_grant
asserts a real in-tree directory named 2000 stays covered by the folder grant.
Extra coverage: printf "%d" 5, echo "a?b", head -n 20.
Netclaw.Security.Tests full run: 672 passed, 0 failed.
dotnet slopwatch analyze: 0 issues.
Add-FileHeaders.ps1 -Verify: all files have headers.

Two regression tests that fail on current dev and demonstrate #1795: - echo text containing a URL query '?' is classified as a glob token and gains a covering directory derived from its static prefix (cwd/---try) - a bare numeric operand (head -c 2000) is treated as a path arg and resolves relative to cwd (cwd/2000) Both cases assert Directory == null, matching how the same verbs behave with ordinary operands. No production fix in this change.

Aaronontheweb · 2026-08-07T20:16:36Z

Holding the fix for ShellSyntaxTree 0.3.0.

The ?-glob half of #1795 lives in ShellSyntaxTree's glob classification (BashResolver Step 5: any token containing *, ?, or [ becomes Kind=Glob, with no URL-query awareness). The 0.3.0 structured-analysis rework replaces that blanket heuristic with explicit proof domains (Exact / FiniteSet / Pattern + covering directory), so it is the right layer to fix URL-query ? classification. The repro tests in this PR stay as the regression gate; we flip them green when NetClaw consumes ShellSyntaxTree 0.3.0 (and tighten IsAuthorizationPathArg for the bare-numeric case if the new parser doesn't cover it).

…bs and numeric operands (#1795) The shell approval matcher made phantom directory scopes from two token shapes. Each phantom scope inflated the approval header to "Approve in N directories?" and forced a re-prompt. The root cause is a ShellSyntaxTree 0.2 misclassification: - `echo "---try /stats?format=json---"` holds a `?`. The parser tags the text operand as a glob arg. The matcher then makes a covering directory from the static prefix (`---try`). - `head -c 2000` holds a bare number. The parser tags `2000` as a path arg. The matcher then resolves it to `/cwd/2000`. This change adds two narrow guards in `IToolApprovalMatcher`: - A pure side-effect verb (echo, printf, :, true, false) derives no arg-based directory scope. It writes only to stdout. A redirect still gives a real scope, so `echo x > dir/file` keeps `dir`. - A token of only ASCII digits with no `/` is a numeric value, not a path arg. The guard stays narrow: `2000.txt`, `file1234`, and `1234/x` pass. Both guards only remove a false scope. Neither grants, widens, or hides authorization. ShellSyntaxTree v0.3.0 fixes the misclassification in the parser. A comment on each guard marks it for removal then. Adds five ExtractCandidates coverage cases for the two shapes.

+            foreach (var arg in clause.Args)
            {
-                var coveringDirectory = ResolveGlobCoveringDirectory(arg, clauseWorkingDirectory);
-                if (coveringDirectory is null)
-                    return null;
+                if (arg.IsCwdAttribution || !IsAuthorizationPathArg(arg, clauseWorkingDirectory))
+                    continue;

-                directories.Add(coveringDirectory);
-                continue;
-            }
+                if (arg.Kind == ShellSyntaxTree.ArgKind.Glob)
+                {
+                    var coveringDirectory = ResolveGlobCoveringDirectory(arg, clauseWorkingDirectory);
+                    if (coveringDirectory is null)
+                        return null;

-            // A parser path without a canonical value cannot use the broader
-            // cwd grant. Return no candidates so the command fails closed.
-            if (string.IsNullOrWhiteSpace(arg.Resolved))
-                return null;
+                    directories.Add(coveringDirectory);
+                    continue;
+                }
+
+                // A parser path without a canonical value cannot use the broader
+                // cwd grant. Return no candidates so the command fails closed.
+                if (string.IsNullOrWhiteSpace(arg.Resolved))
+                    return null;

-            directories.Add(ShellTokenizer.ApplyFileParentRule(arg.Resolved));
+                directories.Add(ShellTokenizer.ApplyFileParentRule(arg.Resolved));
+            }


#1795) The first #1795 numeric guard was purely syntactic. It dropped any all-digit token as a numeric value. It could not tell the value in `head -c 2000` from a real filesystem object named `2000`. This let an attacker escalate a prompt into an auto-approve. `cat 2000` where `2000` is a symlink out of the granted tree, plus a folder grant `(cat, <cwd>)`, dropped the path scope. The candidate collapsed to the cwd. The symlink-segment check in MatchesShellApproval never ran. The read auto-approved with no prompt. The fix gates the guard on filesystem existence: - Drop the all-digit token ONLY when no entry exists at its resolved path. - A real file, directory, or symlink named `2000` stays a path arg, so its scope and the symlink-segment check survive. - SAFE-FAIL: if the path is unknown or the probe throws, keep the token as a path. Never fail toward auto-approve. `head -c 2000` with no file named `2000` still drops to Directory == null, so the prompt-spam fix holds. `cat 2000` on a symlink keeps its scope, so the read prompts again. Adds two regression tests: the escaping-symlink attack asserts IsApproved is false, and a real in-tree directory named `2000` asserts the folder grant still covers it.

+            }
+            else if (!string.IsNullOrWhiteSpace(workingDirectory))
+            {
+                path = Path.Combine(workingDirectory, token);


+        // MatchesShellApproval refuses the folder grant. A purely syntactic
+        // guard would drop `2000`, collapse the scope to the cwd, skip the
+        // symlink check, and auto-approve a read outside the tree.
+        var root = Path.Combine(Path.GetTempPath(), $"netclaw-numeric-symlink-{Guid.NewGuid():N}");


+        // guard would drop `2000`, collapse the scope to the cwd, skip the
+        // symlink check, and auto-approve a read outside the tree.
+        var root = Path.Combine(Path.GetTempPath(), $"netclaw-numeric-symlink-{Guid.NewGuid():N}");
+        var projectDirectory = Path.Combine(root, "project");


+        // symlink check, and auto-approve a read outside the tree.
+        var root = Path.Combine(Path.GetTempPath(), $"netclaw-numeric-symlink-{Guid.NewGuid():N}");
+        var projectDirectory = Path.Combine(root, "project");
+        var externalDirectory = Path.Combine(root, "external");


+        var root = Path.Combine(Path.GetTempPath(), $"netclaw-numeric-symlink-{Guid.NewGuid():N}");
+        var projectDirectory = Path.Combine(root, "project");
+        var externalDirectory = Path.Combine(root, "external");
+        var externalSecret = Path.Combine(externalDirectory, "secret.txt");


+        var projectDirectory = Path.Combine(root, "project");
+        var externalDirectory = Path.Combine(root, "external");
+        var externalSecret = Path.Combine(externalDirectory, "secret.txt");
+        var link = Path.Combine(projectDirectory, "2000");


+        // the folder grant. This proves the existence gate does not over-block
+        // a legitimate in-tree entry, and that the numeric token stays a path
+        // when a real object exists.
+        var root = Path.Combine(Path.GetTempPath(), $"netclaw-numeric-dir-{Guid.NewGuid():N}");


+        // a legitimate in-tree entry, and that the numeric token stays a path
+        // when a real object exists.
+        var root = Path.Combine(Path.GetTempPath(), $"netclaw-numeric-dir-{Guid.NewGuid():N}");
+        var projectDirectory = Path.Combine(root, "project");


+        // when a real object exists.
+        var root = Path.Combine(Path.GetTempPath(), $"netclaw-numeric-dir-{Guid.NewGuid():N}");
+        var projectDirectory = Path.Combine(root, "project");
+        var numericDirectory = Path.Combine(projectDirectory, "2000");


Aaronontheweb

LGTM

Aaronontheweb added bug Something isn't working shell Issues related to the shell tool, since it has the largest security perimeter. tests All issues related to testing, quality assurance, and smoke testing. labels Aug 7, 2026

Aaronontheweb changed the title ~~test(approvals): failing tests reproduce phantom approval directory scopes (#1795)~~ fix(approvals): stop fabricating phantom approval directory scopes (#1795) Aug 8, 2026

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

View reviewed changes

Merge branch 'dev' into fix/approval-directories-prompt

dd07861

Aaronontheweb commented Aug 8, 2026

View reviewed changes

Aaronontheweb enabled auto-merge (squash) August 8, 2026 01:36

Aaronontheweb merged commit bdf28c0 into dev Aug 8, 2026
21 checks passed

Aaronontheweb deleted the fix/approval-directories-prompt branch August 8, 2026 02:04

Navigation Menu

Uh oh!

fix(approvals): stop fabricating phantom approval directory scopes (#1795) - #1799

fix(approvals): stop fabricating phantom approval directory scopes (#1795)#1799
Aaronontheweb merged 4 commits into
devfrom
fix/approval-directories-prompt

Aaronontheweb commented Aug 7, 2026 •

edited

Loading

Uh oh!

Aaronontheweb commented Aug 7, 2026

Uh oh!

Aaronontheweb left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Aaronontheweb commented Aug 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Root cause

The fix

Security hardening (adversarial review)

Corrected security reasoning

Tests

Uh oh!

Aaronontheweb commented Aug 7, 2026

Uh oh!

Aaronontheweb left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Aaronontheweb commented Aug 7, 2026 •

edited

Loading