examples: add tool execution safety guard#2193
Open
March-77 wants to merge 3 commits into
Open
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughEnglish
Compatibility / behavioral risks
Recommended validation steps
中文
兼容性 / 行为风险
建议验证步骤
WalkthroughAdds a standalone Go tool-safety guard example with configurable policy scanning, redaction, audit middleware, CLI-generated reports, sample requests, tests, and usage documentation. ChangesTool Safety Guard
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Flash-LHR
reviewed
Jul 12, 2026
| if regexp.MustCompile(`\b(?:sh|bash|cmd|powershell)\s+(?:-c|/c|-command)\b|\beval\b`).MatchString(lower) { | ||
| add("ask", "high", "SHELL_WRAPPER", req.Command, "expand and review the wrapped command before execution") | ||
| } | ||
| if strings.ContainsAny(req.Command, "`$") || strings.Contains(req.Command, "|") || strings.ContainsAny(req.Command, ";><") { |
Collaborator
There was a problem hiding this comment.
Newline separators bypass denied command checks, so go test ./... followed by sudo returns allow. Treat newlines as command separators before returning allow.
中文
换行分隔符会绕过禁用命令检查，因此 `go test ./...` 后接 `sudo` 会返回 `allow`。请在返回 `allow` 前将换行视为命令分隔符。
|
All contributors have signed the CLA ✍️ ✅ |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2193 +/- ##
===================================================
+ Coverage 89.86563% 89.88162% +0.01599%
===================================================
Files 1144 1144
Lines 198483 198520 +37
===================================================
+ Hits 178368 178433 +65
+ Misses 12602 12579 -23
+ Partials 7513 7508 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
examples/tool_safety_guard/main.go (2)
45-45: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse keyed struct fields for instantiation.
Using unkeyed struct literals is fragile. If fields are added or reordered in the structs later, these initializations will silently break or compile with errors. It is best practice to use keyed fields, especially in example code that users may use as a reference.
中文
使用带键的结构体字段进行实例化。
使用未命名的结构体字面量是很脆弱的。如果在未来的版本中向结构体添加或重新排列字段，这些初始化将失败或引发编译错误。建议使用带键的字段，特别是在用户可能作为参考的示例代码中。
examples/tool_safety_guard/main.go#L45-L45: Use keyed fields forSafetyReport.examples/tool_safety_guard/main.go#L53-L53: Use keyed fields forAuditEvent.♻️ Proposed fixes
For
SafetyReport:report := SafetyReport{ GeneratedAt: time.Now().UTC().Format(time.RFC3339Nano), Policy: *policyPath, Samples: len(samples), MatchedExpected: 0, Results: nil, }For
AuditEvent:event := AuditEvent{ Timestamp: time.Now().UTC().Format(time.RFC3339Nano), ToolName: sample.Request.ToolName, Decision: result.Decision, RiskLevel: result.RiskLevel, RuleID: result.RuleID, Backend: sample.Request.Backend, DurationMicros: result.DurationMicros, Redacted: result.Redacted, Blocked: result.Blocked, }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/tool_safety_guard/main.go` at line 45, Replace the unkeyed SafetyReport literal at examples/tool_safety_guard/main.go:45 with keyed fields matching GeneratedAt, Policy, Samples, MatchedExpected, and Results; likewise update the AuditEvent literal at examples/tool_safety_guard/main.go:53 to use its named fields, preserving all existing values.
54-58: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHandle
json.Marshalerrors.While
json.Marshalis unlikely to fail with the current types, it is good practice to handle its errors, especially in example code that users might copy and modify. You can leverage the existingmust()helper.中文
处理
json.Marshal错误。尽管
json.Marshal在当前类型下不太可能失败，但处理其错误是一个好习惯，特别是在用户可能会复制和修改的示例代码中。你可以利用现有的must()辅助函数。♻️ Proposed fix
- line, _ := json.Marshal(event) + line, e := json.Marshal(event) + must(e) _, _ = writer.Write(append(line, '\n')) } - out, _ := json.MarshalIndent(report, "", " ") + out, e := json.MarshalIndent(report, "", " ") + must(e) must(os.WriteFile(*reportPath, append(out, '\n'), 0o644))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@examples/tool_safety_guard/main.go` around lines 54 - 58, Handle the error returned by the report serialization call in the shown flow by routing json.MarshalIndent through the existing must() helper, while preserving the current output formatting and WriteFile behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@examples/tool_safety_guard/guard_test.go`:
- Around line 76-84: Replace TestPerformance500Commands with a Go benchmark
using testing.B and benchmark iterations, removing the hardcoded one-second
assertion while repeatedly measuring the guard.Scan call. Update loadTest to
accept testing.TB so the benchmark and existing tests can share it, preserving
the current test setup and scan request.
In `@examples/tool_safety_guard/guard.go`:
- Line 116: In examples/tool_safety_guard/guard.go at lines 116, 124, 127, 138,
144, 147, and 150, move the static regexp.MustCompile patterns used by Scan to
package-level compiled regex variables and reuse them on each request. At line
112, compile the DeniedCommands-dependent regexes once during NewGuard
initialization and store them on the guard for Scan to reuse; remove all
per-request compilation while preserving the existing matching behavior.
- Line 112: Update the denied-command regex in the guard check to include
newline as a valid command separator alongside semicolons, ampersands, and
pipes. Preserve the existing command matching and boundary behavior so chained
commands such as “go test\nsudo” are detected.
---
Nitpick comments:
In `@examples/tool_safety_guard/main.go`:
- Line 45: Replace the unkeyed SafetyReport literal at
examples/tool_safety_guard/main.go:45 with keyed fields matching GeneratedAt,
Policy, Samples, MatchedExpected, and Results; likewise update the AuditEvent
literal at examples/tool_safety_guard/main.go:53 to use its named fields,
preserving all existing values.
- Around line 54-58: Handle the error returned by the report serialization call
in the shown flow by routing json.MarshalIndent through the existing must()
helper, while preserving the current output formatting and WriteFile behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c844a7c1-53a6-45e6-a1db-a9bcdf7c4ee0
📒 Files selected for processing (9)
examples/tool_safety_guard/README.mdexamples/tool_safety_guard/go.modexamples/tool_safety_guard/guard.goexamples/tool_safety_guard/guard_test.goexamples/tool_safety_guard/main.goexamples/tool_safety_guard/samples.jsonexamples/tool_safety_guard/tool_safety_audit.jsonl.exampleexamples/tool_safety_guard/tool_safety_policy.jsonexamples/tool_safety_guard/tool_safety_report.json.example
Author
|
I have read the CLA Document and I hereby sign the CLA |
Author
|
recheck |
WineChord
reviewed
Jul 15, 2026
| if destructiveCommandRE.MatchString(lower) { | ||
| add("deny", "critical", "DESTRUCTIVE_COMMAND", req.Command, "do not run destructive filesystem commands") | ||
| } | ||
| for _, path := range g.policy.ForbiddenPaths { |
Collaborator
There was a problem hiding this comment.
ForbiddenPaths only checks req.Command, so WorkingDir: "~/.ssh" with a harmless command can be allowed even though the guard says it protects working directories. Normalize and check req.WorkingDir against ForbiddenPaths as well, then add a sample/test for protected working dirs.
中文
`ForbiddenPaths` 现在只检查 `req.Command`，所以即使 `WorkingDir` 是 `~/.ssh`，只要命令本身看起来无害就可能被放行，这和 guard 宣称会保护工作目录不一致。建议把 `req.WorkingDir` 规范化后也纳入 `ForbiddenPaths` 检查，并补一个受保护工作目录的样例/测试。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements #2002 with a policy-driven Tool Execution Safety Guard.
allow,deny, oraskdecisions with risk, rule, evidence, and recommendationtool.PermissionPolicy: audit first, block deny/ask, call the executor only for allowValidation
go test ./...inexamples/tool_safety_guardgo vet ./...inexamples/tool_safety_guardgo testand allowlisted GitHub API request: allowedUsage
The samples are scanned only; dangerous commands are never executed. This guard is a policy layer and intentionally does not claim to replace container/E2B sandbox isolation, clean environments, output bounds, or process-tree cleanup.