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

src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs

@@ -1141,6 +1141,175 @@ public void ExtractCandidates_uses_redirect_target_and_invocation_working_direct
             candidate.Verb == "echo" && candidate.Directory == workingDirectory);
     }
 
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")]
+    public void ExtractCandidates_echo_text_question_mark_is_not_a_glob_scope()
+    {
+        // Regression for #1795: `echo "---try /stats?format=json---"` contains
+        // a `?`, which the parser classifies as a Glob token. The matcher then
+        // derives a covering directory from the static prefix (`---try`) —
+        // but the `?` is URL query syntax inside echo text, not a glob
+        // pattern, and `---try` is not a real directory. echo is a
+        // stdout-only side-effect verb, so the candidate must carry
+        // Directory == null (matching `echo "done"`). The phantom scope is
+        // what inflated the approval header to "Approve in 2 directories?".
+        var candidates = _matcher.ExtractCandidates(
+            new ToolName("shell_execute"),
+            new Dictionary<string, object?>
+            {
+                ["Command"] = "echo \"---try /stats?format=json---\"",
+                ["WorkingDirectory"] = "/home/user/repos/demo"
+            });
+
+        var echoCandidate = Assert.Single(candidates);
+        Assert.Equal("echo", echoCandidate.Verb);
+        Assert.Null(echoCandidate.Directory);
+    }
+
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")]
+    public void ExtractCandidates_bare_numeric_operand_is_not_a_path_scope()
+    {
+        // Regression for #1795: `head -c 2000` treats the bare numeric
+        // operand `2000` as a path arg (slash-free token branch of
+        // IsAuthorizationPathArg) and resolves it relative to cwd, producing
+        // the phantom scope `/cwd/2000`. `head -c 2000` touches no filesystem
+        // path; head is a read-only verb with no path argument, so its
+        // candidate must carry Directory == null (like `git status`).
+        var candidates = _matcher.ExtractCandidates(
+            new ToolName("shell_execute"),
+            new Dictionary<string, object?>
+            {
+                ["Command"] = "head -c 2000",
+                ["WorkingDirectory"] = "/home/user/repos/demo"
+            });
+
+        var headCandidate = Assert.Single(candidates);
+        Assert.Equal("head", headCandidate.Verb);
+        Assert.Null(headCandidate.Directory);
+    }
+
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")]
+    public void ExtractCandidates_printf_format_operand_is_not_a_path_scope()
+    {
+        // Regression for #1795: printf is a stdout-only side-effect verb. Its
+        // format string and value operands are literal text, not paths. The
+        // candidate must carry Directory == null.
+        var candidates = _matcher.ExtractCandidates(
+            new ToolName("shell_execute"),
+            new Dictionary<string, object?>
+            {
+                ["Command"] = "printf \"%d\" 5",
+                ["WorkingDirectory"] = "/home/user/repos/demo"
+            });
+
+        var printfCandidate = Assert.Single(candidates);
+        Assert.Equal("printf", printfCandidate.Verb);
+        Assert.Null(printfCandidate.Directory);
+    }
+
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")]
+    public void ExtractCandidates_echo_glob_char_text_is_not_a_scope()
+    {
+        // Regression for #1795: `echo "a?b"` contains a `?`, which the parser
+        // classifies as a Glob token. echo is a side-effect verb, so no
+        // arg-derived scope forms. The candidate must carry Directory == null.
+        var candidates = _matcher.ExtractCandidates(
+            new ToolName("shell_execute"),
+            new Dictionary<string, object?>
+            {
+                ["Command"] = "echo \"a?b\"",
+                ["WorkingDirectory"] = "/home/user/repos/demo"
+            });
+
+        var echoCandidate = Assert.Single(candidates);
+        Assert.Equal("echo", echoCandidate.Verb);
+        Assert.Null(echoCandidate.Directory);
+    }
+
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")]
+    public void ExtractCandidates_bare_numeric_flag_value_is_not_a_path_scope()
+    {
+        // Regression for #1795: `head -n 20` has a bare numeric operand `20`.
+        // A number is not a path, so no scope forms. The candidate must carry
+        // Directory == null.
+        var candidates = _matcher.ExtractCandidates(
+            new ToolName("shell_execute"),
+            new Dictionary<string, object?>
+            {
+                ["Command"] = "head -n 20",
+                ["WorkingDirectory"] = "/home/user/repos/demo"
+            });
+
+        var headCandidate = Assert.Single(candidates);
+        Assert.Equal("head", headCandidate.Verb);
+        Assert.Null(headCandidate.Directory);
+    }
+
+    [SlopwatchSuppress("SW001", "This test verifies Bash symlink path behavior, which does not apply to the Windows shell parser.")]
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")]
+    public void IsApproved_numeric_token_that_names_an_escaping_symlink_still_prompts()
+    {
+        // Security regression for the #1795 numeric guard. `cat 2000` where
+        // `2000` is a symlink out of the granted tree must NOT auto-approve.
+        // The guard drops a numeric operand only when no filesystem entry
+        // exists at its path. A real symlink named `2000` stays a path arg, so
+        // its scope survives and the symlink-segment check in
+        // 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}");
+        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");
+        Directory.CreateDirectory(projectDirectory);
+        Directory.CreateDirectory(externalDirectory);
+        File.WriteAllText(externalSecret, "secret");
+        File.CreateSymbolicLink(link, externalSecret);
+
+        try
+        {
+            var approved = new[] { new ApprovalEntry("cat") { Directory = projectDirectory } };
+            Assert.False(_matcher.IsApproved(
+                new ToolName("shell_execute"),
+                Args("cat 2000", projectDirectory),
+                approved,
+                cwd: projectDirectory));
+        }
+        finally
+        {
+            File.Delete(link);
+            Directory.Delete(root, recursive: true);
+        }
+    }
+
+    [Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only path semantics")]
+    public void IsApproved_numeric_token_that_names_a_real_in_tree_directory_is_covered_by_grant()
+    {
+        // Complement to the symlink case. `cat 2000` where `2000` is a real
+        // directory inside the granted tree keeps its scope and is covered by
+        // 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}");
+        var projectDirectory = Path.Combine(root, "project");
+        var numericDirectory = Path.Combine(projectDirectory, "2000");
+        Directory.CreateDirectory(numericDirectory);
+
+        try
+        {
+            var approved = new[] { new ApprovalEntry("cat") { Directory = projectDirectory } };
+            Assert.True(_matcher.IsApproved(
+                new ToolName("shell_execute"),
+                Args("cat 2000", projectDirectory),
+                approved,
+                cwd: projectDirectory));
+        }
+        finally
+        {
+            Directory.Delete(root, recursive: true);
+        }
+    }
+
     [Fact]
     public void IsApproved_treats_side_effect_candidates_as_authorized()
     {

src/Netclaw.Security/IToolApprovalMatcher.cs

@@ -269,27 +269,39 @@ private static IReadOnlyList<ApprovalCandidate> ExtractCandidatesViaBashAnalysis
         // Each parser path is an authorization scope. A grant must cover all
         // scopes, or a later external path could hide behind an earlier local
         // path. The resolved value also handles native forms such as @file.
-        foreach (var arg in clause.Args)
+        //
+        // Temporary containment for #1795. ShellSyntaxTree 0.2 tags an echo or
+        // printf text operand as a path arg or a glob arg. The matcher then
+        // makes a phantom directory scope from literal text. A pure side-effect
+        // verb writes only to stdout. Its operands are never filesystem paths.
+        // So this verb derives no arg-based scope. A redirect still gives a real
+        // scope below. This code only removes a false scope. It never grants a
+        // new one. ShellSyntaxTree v0.3.0 fixes the misclassification in the
+        // parser. Remove this guard then.
+        if (!isSideEffectVerb)
         {
-            if (arg.IsCwdAttribution || !IsAuthorizationPathArg(arg, clauseWorkingDirectory))
-                continue;
-
-            if (arg.Kind == ShellSyntaxTree.ArgKind.Glob)
+            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));
+            }
         }
 
         foreach (var redirect in clause.Redirects)
@@ -395,6 +407,28 @@ private static bool IsAuthorizationPathArg(
         if (!arg.IsPath)
             return false;
 
+        // Temporary containment for #1795. ShellSyntaxTree 0.2 tags a bare
+        // numeric operand such as `head -c 2000` as a path arg. The matcher
+        // then makes a phantom scope `/cwd/2000`. A token of only ASCII digits
+        // is normally a numeric value, not a path.
+        //
+        // Drop the token ONLY when no filesystem entry exists at its resolved
+        // path. A real file, directory, or symlink named `2000` stays a path
+        // arg, so its directory scope and the later symlink-segment check
+        // survive. Without this existence gate, `cat 2000` where `2000` is a
+        // symlink out of the granted tree would collapse to the cwd and skip
+        // the symlink check — a prompt-to-auto-approve escalation on the ACL
+        // perimeter. ShouldDropNumericToken safe-fails: it keeps the arg as a
+        // path on any doubt, so the gate prompts. This guard stays narrow:
+        // `2000.txt`, `file1234`, and `1234/x` still pass. This code only
+        // removes a false scope. It never grants a new one. ShellSyntaxTree
+        // v0.3.0 classifies the operand in the parser. Remove this guard then.
+        if (arg.Raw.Length > 0 && arg.Raw.All(char.IsAsciiDigit)
+            && ShouldDropNumericToken(arg, workingDirectory))
+        {
+            return false;
+        }
+
         if (ShellTokenizer.IsPathToken(arg.Raw) || !arg.Raw.Contains('/', StringComparison.Ordinal))
             return true;
 
@@ -428,6 +462,51 @@ or NotSupportedException
         }
     }
 
+    /// <summary>
+    /// Decides if an all-digit operand is a plain numeric value and not a real
+    /// filesystem object. This is temporary containment for #1795. It returns
+    /// <c>true</c> only when the matcher can prove no entry exists at the
+    /// resolved path. On any doubt it returns <c>false</c>, so the caller keeps
+    /// the token as a path and the gate prompts. It never fails toward an
+    /// automatic grant.
+    /// </summary>
+    private static bool ShouldDropNumericToken(ShellSyntaxTree.Arg arg, string? workingDirectory)
+    {
+        try
+        {
+            var token = string.IsNullOrWhiteSpace(arg.Resolved) ? arg.Raw : arg.Resolved;
+
+            string path;
+            if (Path.IsPathRooted(token))
+            {
+                path = token;
+            }
+            else if (!string.IsNullOrWhiteSpace(workingDirectory))
+            {
+                path = Path.Combine(workingDirectory, token);
+            }
+            else
+            {
+                // No absolute path to probe. Keep the token as a path.
+                return false;
+            }
+
+            // File.Exists and Directory.Exists follow a symlink to its target.
+            // A real symlink to an outside path returns true here, so the token
+            // stays a path and keeps its symlink-segment check downstream.
+            return !File.Exists(path) && !Directory.Exists(path);
+        }
+        catch (Exception ex) when (ex is ArgumentException
+                                      or IOException
+                                      or NotSupportedException
+                                      or UnauthorizedAccessException
+                                      or System.Security.SecurityException)
+        {
+            // The probe failed. Keep the token as a path so the gate prompts.
+            return false;
+        }
+    }
+
     private static string? ResolveRedirectDirectory(ShellSyntaxTree.Redirect redirect)
     {
         if (string.IsNullOrWhiteSpace(redirect.Target)