Skip to content

Fix: agent not able to execute shell command added to customAllowPatterns - #3314

Open
j-v wants to merge 1 commit into
sipeed:mainfrom
j-v:fix/custom-allow-pattern-exempts-deny
Open

Fix: agent not able to execute shell command added to customAllowPatterns#3314
j-v wants to merge 1 commit into
sipeed:mainfrom
j-v:fix/custom-allow-pattern-exempts-deny

Conversation

@j-v

@j-v j-v commented Aug 3, 2026

Copy link
Copy Markdown

📝 Description

Bug: My agent was not able to execute 'git push' despite adding it to the exec allow list. According to the tests it should have worked.

Fixed customAllowPatterns not working: default deny patterns always took precedence in guardCommand, so a command like git push could never be permitted via a custom allow pattern. Separated user-specified CustomDenyPatterns from the built-in defaults so security-critical custom deny rules (e.g. jq env-access checks) still always apply, while built-in deny patterns can be exempted by a matching custom allow pattern.

Also fixed TestShellTool_CustomAllowPatterns, which previously passed vacuously: it never set an internal channel context, so the channel-restriction check blocked the command before the guard ran. The test now reaches guardCommand and genuinely asserts the allow/deny behavior (allowed with pattern, blocked without, blocked for non-matching remote).

🗣️ Type of Change

  • 🐞 Bug fix (non-breaking change which fixes an issue)
  • ✨ New feature (non-breaking change which adds functionality)
  • 📖 Documentation update
  • ⚡ Code refactoring (no functional changes, no api changes)

🤖 AI Code Generation

  • 🤖 Fully AI-generated (100% AI, 0% Human)
  • 🛠️ Mostly AI-generated (AI draft, Human verified/modified)
  • 👨‍💻 Mostly Human-written (Human lead, AI assisted or none)

🔗 Related Issue

N/A

📚 Technical Context (Skip for Docs)

  • Reference URL: N/A
  • Reasoning: guardCommand checked all deny patterns (built-in + user CustomDenyPatterns) before any allow logic, and customAllowPatterns were only consulted by commandMatchesAllowPattern, which is gated on allowPatterns being non-empty (never populated in practice) — making custom allow rules dead code. The fix splits deny patterns into built-in (denyPatterns) and user-specified (customDenyPatterns) lists. Built-in deny patterns (e.g. \bgit\s+push\b) can now be exempted by a matching custom allow pattern, but custom deny patterns always apply so operators can't weaken explicit security rules (see [Security] PicoClaw exec command whitelist allows jq environment disclosure by skipping deny-pattern enforcement #3079). The existing test appeared to pass but never exercised the guard due to the channel check firing first on an empty channel context.

🧪 Test Environment

  • Hardware: PC (Apple Silicon Mac)
  • OS: macOS
  • Model/Provider: DeepSeek-V3
  • Channels: CLI

📸 Evidence (Optional)

Click to view Logs/Screenshots Without the fix (shell.go reverted), `TestShellTool_CustomAllowPatterns` fails: ``` === RUN TestShellTool_CustomAllowPatterns shell_test.go:682: custom allow pattern should exempt 'git push origin main', got: Command blocked by safety guard (dangerous pattern detected) --- FAIL: TestShellTool_CustomAllowPatterns (0.00s) ``` With the fix: ``` === RUN TestShellTool_CustomAllowPatterns --- PASS: TestShellTool_CustomAllowPatterns (1.33s) === RUN TestShellTool_CustomAllowDoesNotBypassDenyPatterns --- PASS: TestShellTool_CustomAllowDoesNotBypassDenyPatterns (0.00s) === RUN TestShellTool_CustomAllowStillPermitsSafeMatch --- PASS: TestShellTool_CustomAllowStillPermitsSafeMatch (0.00s) === RUN TestShellTool_CustomAllowDoesNotBecomeStrictAllowlist --- PASS: TestShellTool_CustomAllowDoesNotBecomeStrictAllowlist (0.00s) ```

☑️ Checklist

  • My code/docs follow the style of this project.
  • I have performed a self-review of my own changes.
  • I have updated the documentation accordingly.

@github-actions

Copy link
Copy Markdown

This PR has had no activity for 7 days and has been marked as stale. If you are still working on it, please push an update or leave a comment; otherwise it will be closed automatically in 7 days.

@github-actions github-actions Bot added the stale label Aug 10, 2026
@j-v

j-v commented Aug 10, 2026

Copy link
Copy Markdown
Author

Still active, waiting for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant