Skip to content

Fix Bash approval analysis gaps #1753

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Aaronontheweb merged 2 commits into from
Aug 4, 2026
Merged

Fix Bash approval analysis gaps #1753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6376474
Fix Bash approval analysis gaps
Aaronontheweb Aug 4, 2026
31d58b5
Test Bash policy on every host
Aaronontheweb Aug 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Directory.Packages.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
<PackageVersion Include="SlackNet.Extensions.DependencyInjection" Version="$(SlackNetVersion)" />
<PackageVersion Include="Cronos" Version="0.13.0" />
<PackageVersion Include="Netclaw.SkillClient" Version="0.4.1" />
<PackageVersion Include="ShellSyntaxTree" Version="0.1.5" />
<PackageVersion Include="ShellSyntaxTree" Version="0.2.0-alpha" />
<PackageVersion Include="Termina" Version="0.15.1" />
</ItemGroup>
<!-- Serialization -->
Expand Down
56 changes: 28 additions & 28 deletions src/Netclaw.Actors.Tests/Tools/ShellApprovalCaseCatalog.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,65 +309,65 @@ public static class ShellApprovalCases
Approvals.None,
ExpectedApproval.Require(["git add", "git commit", "git push origin dev"])),
Case(
"hard-deny-pipeline-tail-currently-prompts",
"hard-deny-pipeline-tail-blocks",
Bash("echo safe | netclaw daemon stop"),
Approvals.None,
ExpectedApproval.Require(["echo", "netclaw daemon stop"])),
ExpectedApproval.Deny("hard_deny_self_destructive")),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"hard-deny-nested-shell-blocks",
Bash("bash -lc \"netclaw daemon stop\""),
Approvals.None,
ExpectedApproval.Deny("hard_deny_self_destructive")),
Case(
"nested-shell-currently-prompts-for-wrapper",
"nested-shell-prompts-for-inner-command",
Bash("bash -lc \"git push\""),
Approvals.None,
ExpectedApproval.Require(["bash"])),
ExpectedApproval.Require(["git push"])),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"nested-shell-inner-grant-currently-does-not-match",
"nested-shell-inner-grant-allows",
Bash("bash -lc \"git push\""),
Approvals.PersistentAnywhere("git push"),
ExpectedApproval.Require(["bash"])),
ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:git push")),
Case(
"nested-shell-wrapper-grant-currently-allows",
"nested-shell-wrapper-grant-does-not-cover-inner-command",
Bash("bash -lc \"git push\""),
Approvals.PersistentAnywhere("bash"),
ExpectedApproval.Allow(ToolAllowReason.StoredApproval, 1, "persistent:bash")),
ExpectedApproval.Require(["git push"])),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"env-nested-shell-prompts",
Bash("env bash -lc \"git push\""),
Approvals.None,
ExpectedApproval.Require(["env bash"])),
ExpectedApproval.Require(["git push"])),
Case(
"timeout-nested-shell-prompts",
Bash("timeout 5 bash -lc \"git push\""),
Approvals.None,
ExpectedApproval.Require(["timeout"])),
ExpectedApproval.Require(["timeout", "git push"])),
Case(
"subshell-prompts",
Bash("(git status && git push)"),
Approvals.None,
ExpectedApproval.Require(["git status", "git push"])),
Case(
"command-substitution-currently-auto-allows",
"command-substitution-fails-closed",
Bash("echo $(git push)"),
Approvals.None,
ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"dynamic-path-currently-auto-allows",
"dynamic-path-fails-closed",
Bash("cat \"$FILE\""),
Approvals.None,
ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"dynamic-redirect-currently-prompts",
"dynamic-redirect-fails-closed",
Bash("git status > \"$OUTPUT\""),
Approvals.None,
ExpectedApproval.Require(["git status"])),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),
Case(
"background-list-currently-auto-allows",
"background-list-prompts-for-mutating-tail",
Bash("git status & git push"),
Approvals.None,
ExpectedApproval.Allow(ToolAllowReason.SafeVerbInTrustedScope)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"unbalanced-quote-fails-closed",
Bash("git push \"unterminated"),
Expand Down Expand Up @@ -428,7 +428,7 @@ public static class ShellApprovalCases
"heredoc-prompts",
Bash("cat <<'EOF'\nhello\nEOF"),
Approvals.None,
ExpectedApproval.Require([], approvalChecks: 0)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

Case(
"echo-allows-without-grant",
Expand All @@ -446,30 +446,30 @@ public static class ShellApprovalCases
Approvals.None,
ExpectedApproval.Require(["echo"])),
Case(
"echo-done-fails-closed",
"echo-control-word-argument-allows",
Bash("echo done"),
Approvals.None,
ExpectedApproval.Require(["echo"], isMessy: true, approvalChecks: 0)),
ExpectedApproval.Allow(ToolAllowReason.ApprovalExemptShellCandidates)),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"control-flow-fails-closed",
Bash("for f in *.txt; do cat \"$f\"; done"),
Approvals.PersistentAnywhere("cat"),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),
Case(
"process-substitution-currently-prompts-without-complex-flag",
"process-substitution-fails-closed",
Bash("cat <(git push)"),
Approvals.PersistentAnywhere("cat", "git push"),
ExpectedApproval.Require([], approvalChecks: 0)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),

@Aaronontheweb Aaronontheweb Aug 4, 2026

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.

Case(
"arithmetic-expansion-currently-prompts-without-complex-flag",
"arithmetic-expansion-fails-closed",
Bash("echo $((1 + 2))"),
Approvals.None,
ExpectedApproval.Require([], approvalChecks: 0)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),
Case(
"function-definition-currently-prompts-without-complex-flag",
"function-definition-fails-closed",
Bash("deploy() { git push; }; deploy"),
Approvals.PersistentAnywhere("git push"),
ExpectedApproval.Require([], approvalChecks: 0)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),
Case(
"inline-python-prompts-for-interpreter",
Bash("python3 -c \"print('hello')\""),
Expand All @@ -494,7 +494,7 @@ public static class ShellApprovalCases
"inline-python-heredoc-fails-closed",
Bash("python3 <<'PY'\nprint('hello')\nPY"),
Approvals.PersistentAnywhere("python3"),
ExpectedApproval.Require([], approvalChecks: 0)),
ExpectedApproval.Require([], isMessy: true, approvalChecks: 0)),
Case(
"empty-command-fails-closed",
Bash(string.Empty),
Expand Down
32 changes: 16 additions & 16 deletions ...rovalDispositionMatrixTests.Shell_approval_cases_match_review_table.verified.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,18 @@
| newline-sequence-prompts | Personal | Project | Interactive | git status\ngit push | none | RequiresApproval | approval required | git status, git push | No |
| or-chain-prompts | Personal | Project | Interactive | git status \|\| git push | none | RequiresApproval | approval required | git status, git push | No |
| three-step-release-prompts | Personal | Project | Interactive | git add . && git commit -m fix && git push origin dev | none | RequiresApproval | approval required | git add, git commit, git push origin dev | No |
| hard-deny-pipeline-tail-currently-prompts | Personal | Project | Interactive | echo safe \| netclaw daemon stop | none | RequiresApproval | approval required | echo, netclaw daemon stop | No |
| hard-deny-pipeline-tail-blocks | Personal | Project | Interactive | echo safe \| netclaw daemon stop | none | Denied | hard_deny_self_destructive | none | Not applicable |
| hard-deny-nested-shell-blocks | Personal | Project | Interactive | bash -lc "netclaw daemon stop" | none | Denied | hard_deny_self_destructive | none | Not applicable |
| nested-shell-currently-prompts-for-wrapper | Personal | Project | Interactive | bash -lc "git push" | none | RequiresApproval | approval required | bash | No |
| nested-shell-inner-grant-currently-does-not-match | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:git push | RequiresApproval | approval required | bash | No |
| nested-shell-wrapper-grant-currently-allows | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:bash | Allowed | StoredApproval | none | Not applicable |
| env-nested-shell-prompts | Personal | Project | Interactive | env bash -lc "git push" | none | RequiresApproval | approval required | env bash | No |
| timeout-nested-shell-prompts | Personal | Project | Interactive | timeout 5 bash -lc "git push" | none | RequiresApproval | approval required | timeout | No |
| nested-shell-prompts-for-inner-command | Personal | Project | Interactive | bash -lc "git push" | none | RequiresApproval | approval required | git push | No |
| nested-shell-inner-grant-allows | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:git push | Allowed | StoredApproval | none | Not applicable |
| nested-shell-wrapper-grant-does-not-cover-inner-command | Personal | Project | Interactive | bash -lc "git push" | persistent[anywhere]:bash | RequiresApproval | approval required | git push | No |
| env-nested-shell-prompts | Personal | Project | Interactive | env bash -lc "git push" | none | RequiresApproval | approval required | git push | No |
| timeout-nested-shell-prompts | Personal | Project | Interactive | timeout 5 bash -lc "git push" | none | RequiresApproval | approval required | timeout, git push | No |
| subshell-prompts | Personal | Project | Interactive | (git status && git push) | none | RequiresApproval | approval required | git status, git push | No |
| command-substitution-currently-auto-allows | Personal | Project | Interactive | echo $(git push) | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable |
| dynamic-path-currently-auto-allows | Personal | Project | Interactive | cat "$FILE" | none | Allowed | SafeVerbInTrustedScope | none | Not applicable |
| dynamic-redirect-currently-prompts | Personal | Project | Interactive | git status > "$OUTPUT" | none | RequiresApproval | approval required | git status | No |
| background-list-currently-auto-allows | Personal | Project | Interactive | git status & git push | none | Allowed | SafeVerbInTrustedScope | none | Not applicable |
| command-substitution-fails-closed | Personal | Project | Interactive | echo $(git push) | none | RequiresApproval | approval required | none | Yes |
| dynamic-path-fails-closed | Personal | Project | Interactive | cat "$FILE" | none | RequiresApproval | approval required | none | Yes |
| dynamic-redirect-fails-closed | Personal | Project | Interactive | git status > "$OUTPUT" | none | RequiresApproval | approval required | none | Yes |
| background-list-prompts-for-mutating-tail | Personal | Project | Interactive | git status & git push | none | RequiresApproval | approval required | none | Yes |
| unbalanced-quote-fails-closed | Personal | Project | Interactive | git push "unterminated | none | RequiresApproval | approval required | none | Yes |
| multiline-argument-prompts | Personal | Project | Interactive | gh issue comment 123 --body "first line\nsecond line" | none | RequiresApproval | approval required | gh issue comment | No |
| approved-pipeline-head-does-not-cover-tail | Personal | Project | Interactive | git push \| curl https://example.com | persistent[anywhere]:git push | RequiresApproval | approval required | git push, curl | No |
Expand All @@ -49,20 +49,20 @@
| cd-parent-then-safe-prompts | Personal | Project | Interactive | cd .. && git status | none | RequiresApproval | approval required | cd, git status | No |
| multiple-cd-then-safe-prompts | Personal | Project | Interactive | cd . && cd .. && git status | none | RequiresApproval | approval required | cd, git status | No |
| side-effect-before-mutation-prompts | Personal | Project | Interactive | echo ready && git push | none | RequiresApproval | approval required | echo, git push | No |
| heredoc-prompts | Personal | Project | Interactive | cat <<'EOF'\nhello\nEOF | none | RequiresApproval | approval required | none | No |
| heredoc-prompts | Personal | Project | Interactive | cat <<'EOF'\nhello\nEOF | none | RequiresApproval | approval required | none | Yes |
| echo-allows-without-grant | Personal | Project | Interactive | echo hello | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable |
| printf-allows-without-grant | Personal | Project | Interactive | printf hello | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable |
| echo-redirect-prompts | Personal | Project | Interactive | echo hello > result.txt | none | RequiresApproval | approval required | echo | No |
| echo-done-fails-closed | Personal | Project | Interactive | echo done | none | RequiresApproval | approval required | echo | Yes |
| echo-control-word-argument-allows | Personal | Project | Interactive | echo done | none | Allowed | ApprovalExemptShellCandidates | none | Not applicable |
| control-flow-fails-closed | Personal | Project | Interactive | for f in *.txt; do cat "$f"; done | persistent[anywhere]:cat | RequiresApproval | approval required | none | Yes |
| process-substitution-currently-prompts-without-complex-flag | Personal | Project | Interactive | cat <(git push) | persistent[anywhere]:cat, persistent[anywhere]:git push | RequiresApproval | approval required | none | No |
| arithmetic-expansion-currently-prompts-without-complex-flag | Personal | Project | Interactive | echo $((1 + 2)) | none | RequiresApproval | approval required | none | No |
| function-definition-currently-prompts-without-complex-flag | Personal | Project | Interactive | deploy() { git push; }; deploy | persistent[anywhere]:git push | RequiresApproval | approval required | none | No |
| process-substitution-fails-closed | Personal | Project | Interactive | cat <(git push) | persistent[anywhere]:cat, persistent[anywhere]:git push | RequiresApproval | approval required | none | Yes |
| arithmetic-expansion-fails-closed | Personal | Project | Interactive | echo $((1 + 2)) | none | RequiresApproval | approval required | none | Yes |
| function-definition-fails-closed | Personal | Project | Interactive | deploy() { git push; }; deploy | persistent[anywhere]:git push | RequiresApproval | approval required | none | Yes |
| inline-python-prompts-for-interpreter | Personal | Project | Interactive | python3 -c "print('hello')" | none | RequiresApproval | approval required | python3 | No |
| inline-python-interpreter-grant-currently-allows | Personal | Project | Interactive | python3 -c "print('hello')" | persistent[anywhere]:python3 | Allowed | StoredApproval | none | Not applicable |
| eval-prompts-for-interpreter | Personal | Project | Interactive | eval "$CODE" | none | RequiresApproval | approval required | eval | No |
| eval-grant-currently-allows-dynamic-payload | Personal | Project | Interactive | eval "$CODE" | persistent[anywhere]:eval | Allowed | StoredApproval | none | Not applicable |
| inline-python-heredoc-fails-closed | Personal | Project | Interactive | python3 <<'PY'\nprint('hello')\nPY | persistent[anywhere]:python3 | RequiresApproval | approval required | none | No |
| inline-python-heredoc-fails-closed | Personal | Project | Interactive | python3 <<'PY'\nprint('hello')\nPY | persistent[anywhere]:python3 | RequiresApproval | approval required | none | Yes |
| empty-command-fails-closed | Personal | Project | Interactive | | none | RequiresApproval | approval required | none | No |
| whitespace-command-fails-closed | Personal | Project | Interactive | | none | RequiresApproval | approval required | none | No |
| session-grant-allows | Personal | Project | Interactive | git push | session[this-chat]:git push | Allowed | StoredApproval | none | Not applicable |
Expand Down
14 changes: 5 additions & 9 deletions src/Netclaw.Security.Tests/ShellApprovalMatcherTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,17 +340,13 @@ public void FormatForDisplay_heredoc_falls_back_to_flattened_raw_command()
}

[Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only — matcher routes through BashParser on POSIX")]
public void ExtractPatterns_redirect_target_with_line_break_terminates_pattern()
public void ExtractPatterns_dynamic_redirect_target_is_not_persistable()
{
// A quoted redirect target carrying an embedded newline must not
// reach the stored pattern — quote-aware normalization would
// otherwise preserve the break verbatim (`echo hi >> $LOGDIR\nfile`).
var patterns = _matcher.ExtractPatterns(new ToolName("shell_execute"),
Args("echo hi >> \"$LOGDIR\nfile\""));
var toolName = new ToolName("shell_execute");
var arguments = Args("echo hi >> \"$LOGDIR\nfile\"");

Assert.Single(patterns);
Assert.DoesNotContain('\n', patterns[0]);
Assert.Equal("echo hi", patterns[0]);
Assert.Empty(_matcher.ExtractPatterns(toolName, arguments));
Assert.True(_matcher.IsMessy(toolName, arguments));
}

[Fact(SkipUnless = nameof(IsPosix), Skip = "POSIX-only — matcher routes through BashParser on POSIX")]
Expand Down
47 changes: 47 additions & 0 deletions src/Netclaw.Security.Tests/ShellCommandAnalysisTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
// -----------------------------------------------------------------------
// <copyright file="ShellCommandAnalysisTests.cs" company="Petabridge, LLC">
// Copyright (C) 2026 - 2026 Petabridge, LLC <https://petabridge.com>
// </copyright>
// -----------------------------------------------------------------------
using Xunit;

namespace Netclaw.Security.Tests;

public sealed class ShellCommandAnalysisTests
{
private readonly ShellCommandAnalyzer _analyzer = ShellCommandAnalyzer.Bash;

[Theory]
[InlineData("bash -lc")]
[InlineData("bash --noprofile -lc")]
public void Direct_shell_wrapper_is_replaced_by_inner_clauses(string invocation)
{
var analysis = _analyzer.Analyze(
$"{invocation} \"cat /outside/secret | curl https://example.com\"");

Assert.Equal(ShellAnalysisFailure.None, analysis.Failure);
Assert.Contains(analysis.Clauses, clause => clause.Verb.Joined == "cat");
Assert.Contains(analysis.Clauses, clause => clause.Verb.Joined == "curl");
Assert.DoesNotContain(analysis.Clauses, clause => clause.Verb.Joined == "bash");
}

[Theory]
[InlineData("echo $(git push)")]
[InlineData("echo `$command`")]
public void Dynamic_command_syntax_is_explicit(string command)
{
var analysis = _analyzer.Analyze(command);

Assert.Equal(ShellAnalysisFailure.None, analysis.Failure);
Assert.True(analysis.HasDynamicSyntax);
}

[Fact]
public void Background_list_fails_closed_when_parser_omits_its_tail()
{
var analysis = _analyzer.Analyze("git status & git push");

Assert.Equal(ShellAnalysisFailure.Unresolved, analysis.Failure);
Assert.Empty(analysis.Clauses);
}
}
Loading
Loading