Skip to content

Fix Bash approval analysis gaps - #1753

Merged
Aaronontheweb merged 2 commits into
devfrom
fix/bash-ast-approval-security
Aug 4, 2026
Merged

Fix Bash approval analysis gaps#1753
Aaronontheweb merged 2 commits into
devfrom
fix/bash-ast-approval-security

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Collaborator

Summary

  • Update ShellSyntaxTree to 0.2.0-alpha.
  • Share one Bash analysis between approval checks and hard-deny checks.
  • Check each executable pipeline clause.
  • Resolve nested Bash command strings before approval checks.
  • Fail closed for dynamic syntax and unsupported background lists.
  • Update the Bash approval corpus and its review snapshot.

Security results

  • A hard-deny command in a pipeline tail now denies the full invocation.
  • A stored bash approval no longer authorizes a nested git push.
  • A stored git push approval now matches the nested command.
  • Command substitution and background lists no longer receive automatic access.

Scope

This pull request does not change the runtime shell selection.
It does not add PowerShell support.
Pull request #1733 retains that work.

Tests

  • dotnet test Netclaw.slnx
  • dotnet slopwatch analyze
  • pwsh ./scripts/Add-FileHeaders.ps1 -Verify
  • git diff --check

Closes #1693
Closes #1751

Copilot AI lite review requested due to automatic review settings August 4, 2026 04:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Comment thread src/Netclaw.Security/ShellCommandPolicy.cs
Comment thread src/Netclaw.Security/ShellCommandPolicy.cs
Comment thread src/Netclaw.Security/ShellCommandAnalysis.cs
Copilot AI review requested due to automatic review settings August 4, 2026 04:44

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 4, 2026 11:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI review requested due to automatic review settings August 4, 2026 13:08
@Aaronontheweb
Aaronontheweb force-pushed the fix/bash-ast-approval-security branch from aa6b5b0 to 31d58b5 Compare August 4, 2026 13:08

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Bash("echo safe | netclaw daemon stop"),
Approvals.None,
ExpectedApproval.Require(["echo", "netclaw daemon stop"])),
ExpectedApproval.Deny("hard_deny_self_destructive")),

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 now blocks the complete pipeline. Netclaw checks each pipeline command, so a safe first command cannot hide a blocked command.

Bash("bash -lc \"git push\""),
Approvals.None,
ExpectedApproval.Require(["bash"])),
ExpectedApproval.Require(["git push"])),

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.

Netclaw now checks the text inside bash -lc. The prompt and saved approvals now use git push, not the Bash wrapper.

Bash("bash -lc \"git push\""),
Approvals.PersistentAnywhere("bash"),
ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:bash")),
ExpectedApproval.Require(["git push"])),

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 saved bash approval no longer covers every command inside Bash. Netclaw now asks for approval for git push.

Bash("echo $(git push)"),
Approvals.None,
ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

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 command no longer runs without approval. Netclaw cannot safely identify all actions inside $(), so it asks for approval for this run.

Bash("cat \"$FILE\""),
Approvals.None,
ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

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.

The cat command is safe, but the file path is unknown. Netclaw now asks for approval instead of assuming a safe path.

Bash("git status & git push"),
Approvals.None,
ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

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.

The syntax parser does not return every command after &. Netclaw asks for approval and does not create a broad saved approval.

Bash("echo done"),
Approvals.None,
ExpectedApproval.Require(["echo"], isMessy: true, approvalChecks: 0)),
ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)),

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.

The syntax parser shows that done is plain text here. Netclaw now treats this as a safe echo command.

Bash("cat <(git push)"),
Approvals.PersistentAnywhere("cat", "git push"),
ExpectedApproval.Require([], approvalChecks: 0)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

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 shell form is not fully supported. Netclaw asks for approval for this run, even when saved approvals cover both named commands.

@Aaronontheweb Aaronontheweb added security Security-related changes shell Issues related to the shell tool, since it has the largest security perimeter. labels Aug 4, 2026
@Aaronontheweb
Aaronontheweb merged commit 150aa52 into dev Aug 4, 2026
21 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/bash-ast-approval-security branch August 4, 2026 13:46
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.

Close shell approval gaps exposed by the Bash corpus Underutilize ShellSyntaxTree: pipe-tail security gap and redundant IsMessyCompoundCommand

2 participants