asepharyana
1d50b94eec
- Updated README.md to reflect the addition of 3 new built-in tools, bringing the total to 37. - Revised architecture documentation to indicate the increase in tool count. - Enhanced backend documentation with updated line counts for various modules. - Modified data documentation to change edit log format from JSON to JSONL. - Updated dependencies documentation to reflect version upgrades for several crates. - Improved prompts for auto-reviewer, division implementer, planner, tester, and quality reviewer to enforce stricter coding standards regarding linter bypasses. - Refactored code in various modules to improve clarity and performance, including updates to error handling and tool execution logic. - Added comprehensive tests for IPC frame serialization and deserialization.
496 lines
18 KiB
Rust
496 lines
18 KiB
Rust
//! Tool-call gating: decides whether a risky tool call is allowed to run
|
|
//! before it executes. Implements hooks-style pre-checks for write/edit/delete
|
|
//! and bash tools so the agent cannot silently introduce stubs, denial
|
|
//! patterns, assumption language, or destructive commands.
|
|
|
|
/// Outcome of gating a tool call: whether it's allowed to run.
|
|
#[derive(Debug, Clone, PartialEq)]
|
|
pub enum Verdict {
|
|
Allow,
|
|
Block(String),
|
|
}
|
|
|
|
/// Gatekeeper that decides whether a tool call may proceed before execution.
|
|
pub struct Harness;
|
|
|
|
/// Stub / placeholder / denial / assumption patterns that should never reach
|
|
/// a file in real code. Detected in write/edit content and bash heredocs.
|
|
const STUB_PATTERNS: &[&str] = &[
|
|
"todo!()",
|
|
"todo!(",
|
|
"unimplemented!()",
|
|
"unimplemented!(",
|
|
"todo_macro",
|
|
"FIXME",
|
|
"fixme:",
|
|
"XXX:",
|
|
"PLACEHOLDER",
|
|
"REPLACE_ME",
|
|
"stub_value",
|
|
"stub_function",
|
|
"fake_response",
|
|
"fake_data",
|
|
"not implemented",
|
|
"not yet implemented",
|
|
"to be implemented",
|
|
"to be done",
|
|
];
|
|
|
|
/// Language patterns indicating the AI is denying responsibility or
|
|
/// punting the work ("I'll skip this", "for now just", etc).
|
|
const DENIAL_PATTERNS: &[&str] = &[
|
|
"// skip",
|
|
"// skipping",
|
|
"// skipping for now",
|
|
"// for now just",
|
|
"// punt",
|
|
"// punted",
|
|
"// hack:",
|
|
"// hacky",
|
|
"// hack workaround",
|
|
"// workaround:",
|
|
"// cba",
|
|
"// later",
|
|
"// do later",
|
|
"// ignore for now",
|
|
"// disable",
|
|
"// disabled",
|
|
"// bypass",
|
|
"// quick fix",
|
|
"// temp fix",
|
|
"// temporary fix",
|
|
"// temp:",
|
|
"// temporary:",
|
|
"// noop",
|
|
];
|
|
|
|
/// Assumption-language patterns: words/phrases that indicate the code is
|
|
/// reasoning based on guesswork rather than data.
|
|
const ASSUMPTION_PATTERNS: &[&str] = &[
|
|
"// assume",
|
|
"// assuming",
|
|
"// probably",
|
|
"// maybe",
|
|
"// might",
|
|
"// should work",
|
|
"// hopefully",
|
|
"// guess",
|
|
"// i think",
|
|
"// should be fine",
|
|
"// should be",
|
|
"// likely",
|
|
"// ought to",
|
|
];
|
|
|
|
/// Network-exfiltration and credential-disclosure patterns for bash.
|
|
const EXFIL_PATTERNS: &[&str] = &[
|
|
"curl ", "wget ", "nc -e ", "ncat ", "/dev/tcp/",
|
|
"base64 -d |", "base64 --decode |",
|
|
"openssl s_client", "ssh -R ",
|
|
"scp /", "rsync /",
|
|
];
|
|
|
|
/// Substrings of well-known credential / secret files that bash must not read.
|
|
const SENSITIVE_PATH_PATTERNS: &[&str] = &[
|
|
".ssh/id_rsa",
|
|
".ssh/id_ed25519",
|
|
".ssh/authorized_keys",
|
|
".aws/credentials",
|
|
".aws/config",
|
|
".netrc",
|
|
".pypirc",
|
|
".npmrc",
|
|
".kube/config",
|
|
".docker/config.json",
|
|
".gnupg/",
|
|
"/etc/shadow",
|
|
"/etc/passwd",
|
|
"/proc/self/environ",
|
|
];
|
|
|
|
/// Minimum character length of a `reason` argument to be considered meaningful.
|
|
const MIN_REASON_LEN: usize = 8;
|
|
|
|
impl Harness {
|
|
/// Decide whether a tool call is allowed to execute.
|
|
///
|
|
/// Flow: ALL tools are gated (not just risky ones), closing the bypass
|
|
/// for MCP tools (which are never in the risky list). Basic path
|
|
/// traversal and reason validation applies to any tool with a `path`
|
|
/// argument. Heavy content scanning (stub/denial/assumption/exfiltration)
|
|
/// only applies to risky tools. MCP tools (mcp__ prefix) are treated
|
|
/// as risky because their behaviour is unknown.
|
|
///
|
|
/// Return: `Verdict::Allow` or `Verdict::Block(reason)`.
|
|
#[allow(clippy::too_many_lines, clippy::unnecessary_debug_formatting)]
|
|
pub fn gate_tool_call(
|
|
tool_name: &str,
|
|
args: &serde_json::Value,
|
|
workspace_roots: &[&std::path::Path],
|
|
) -> Verdict {
|
|
|
|
let is_risky = crate::tool::tool_is_risky(tool_name);
|
|
let is_mcp = tool_name.starts_with("mcp__");
|
|
|
|
// ── Universal checks applied to EVERY tool ──
|
|
|
|
// Path traversal: check ANY tool that accepts a path argument,
|
|
// not just write/edit/delete, so tools like read, MCP tools,
|
|
// and future tools are also protected.
|
|
if let Some(path) = args.get("path").and_then(|v| v.as_str()) {
|
|
if path.contains("..") {
|
|
return Verdict::Block(
|
|
"path traversal detected in 'path' argument".to_string(),
|
|
);
|
|
}
|
|
if !workspace_roots.is_empty() {
|
|
let abs_check = std::path::PathBuf::from(path);
|
|
if abs_check.is_absolute()
|
|
&& !workspace_roots.iter().any(|r| abs_check.starts_with(r))
|
|
{
|
|
return Verdict::Block(format!(
|
|
"absolute path '{path}' is outside all workspace roots"
|
|
));
|
|
}
|
|
}
|
|
}
|
|
|
|
// Workspace-root validation for output path.
|
|
if let Some(out_path) = Self::find_output_path(tool_name, args) {
|
|
if !workspace_roots.is_empty()
|
|
&& !out_path.starts_with("/tmp")
|
|
&& !out_path.is_absolute()
|
|
{
|
|
let allowed = workspace_roots.iter().any(|r| out_path.starts_with(r));
|
|
if !allowed {
|
|
return Verdict::Block(format!(
|
|
"output path '{out_path:?}' is outside all workspace roots"
|
|
));
|
|
}
|
|
}
|
|
}
|
|
|
|
// ── Risky / MCP tool checks ──
|
|
// Non-risky, non-MCP tools (read, grep, glob, recall, etc.) are
|
|
// allowed after universal checks above.
|
|
if !is_risky && !is_mcp {
|
|
return Verdict::Allow;
|
|
}
|
|
|
|
// File-mutating tools: write / edit / delete
|
|
if matches!(tool_name, "write" | "edit" | "delete") {
|
|
match Self::validate_reason(tool_name, args) {
|
|
Ok(()) => {}
|
|
Err(msg) => return Verdict::Block(msg),
|
|
}
|
|
}
|
|
|
|
// write / edit content must not contain stubs, denial language, or
|
|
// assumption language.
|
|
if matches!(tool_name, "write" | "edit") {
|
|
if let Some(content) = Self::extract_content(tool_name, args) {
|
|
if let Some(pat) = Self::first_match(&content, STUB_PATTERNS) {
|
|
return Verdict::Block(format!(
|
|
"content contains stub/placeholder pattern '{pat}'; \
|
|
production code must be fully implemented — \
|
|
replace the stub with a real implementation"
|
|
));
|
|
}
|
|
if let Some(pat) = Self::first_match(&content, DENIAL_PATTERNS) {
|
|
return Verdict::Block(format!(
|
|
"content contains denial/punt pattern '{pat}'; \
|
|
implement the change properly instead of skipping"
|
|
));
|
|
}
|
|
if let Some(pat) = Self::first_match(&content, ASSUMPTION_PATTERNS) {
|
|
return Verdict::Block(format!(
|
|
"content contains assumption pattern '{pat}'; \
|
|
verify against data/tests instead of guessing"
|
|
));
|
|
}
|
|
}
|
|
}
|
|
|
|
// Bash: destructive patterns, exfiltration (ALL commands checked,
|
|
// no safe-command whitelist), sensitive-path reads.
|
|
if tool_name == "bash" {
|
|
let cmd = args.get("command").and_then(|v| v.as_str()).unwrap_or("");
|
|
if cmd.contains("..") {
|
|
return Verdict::Block(
|
|
"path traversal detected in bash command".to_string(),
|
|
);
|
|
}
|
|
// Exfiltration patterns are checked on EVERY bash command,
|
|
// regardless of prefix. The safe-command whitelist was removed
|
|
// because it could be bypassed with command chaining.
|
|
for pat in EXFIL_PATTERNS {
|
|
if cmd.contains(pat) {
|
|
return Verdict::Block(format!(
|
|
"potential data-exfiltration command blocked (matched '{pat}')"
|
|
));
|
|
}
|
|
}
|
|
for pat in SENSITIVE_PATH_PATTERNS {
|
|
if cmd.contains(pat) {
|
|
return Verdict::Block(format!(
|
|
"refused to read/write sensitive path '{pat}'"
|
|
));
|
|
}
|
|
}
|
|
let dangerous_patterns = [
|
|
"rm -rf /", "rm -rf --no-preserve-root",
|
|
"rm -rf ~", "rm -fr /", "mkfs.", "dd if=",
|
|
":(){", "> /dev/sda", "chmod -R 000 /",
|
|
"shutdown ", "poweroff ", "reboot ", "halt ",
|
|
];
|
|
for pat in &dangerous_patterns {
|
|
if cmd.contains(pat) {
|
|
return Verdict::Block(format!(
|
|
"destructive command pattern blocked: {pat}"
|
|
));
|
|
}
|
|
}
|
|
// Also scan heredocs / -c / inline content for stub/denial
|
|
// language (e.g. `bash -c 'echo todo!()'`)
|
|
if let Some(pat) = Self::first_match(cmd, STUB_PATTERNS) {
|
|
return Verdict::Block(format!(
|
|
"bash command contains stub pattern '{pat}'"
|
|
));
|
|
}
|
|
}
|
|
|
|
// git_operator: require a non-trivial reason as well.
|
|
if tool_name == "git_operator" {
|
|
if let Some(reason) = args.get("reason").and_then(|v| v.as_str()) {
|
|
if reason.trim().len() < MIN_REASON_LEN {
|
|
return Verdict::Block(format!(
|
|
"git_operator requires a non-trivial 'reason' \
|
|
(>= {MIN_REASON_LEN} chars) explaining the operation"
|
|
));
|
|
}
|
|
} else {
|
|
return Verdict::Block(
|
|
"git_operator requires a 'reason' argument explaining the operation"
|
|
.to_string(),
|
|
);
|
|
}
|
|
}
|
|
|
|
// MCP tools: unknown behaviour — require a reason if they take
|
|
// arguments, to discourage lazy invocations.
|
|
if is_mcp {
|
|
if let Some(reason) = args.get("reason").and_then(|v| v.as_str()) {
|
|
if reason.trim().len() < MIN_REASON_LEN {
|
|
return Verdict::Block(format!(
|
|
"MCP tool '{tool_name}' requires a non-trivial 'reason' \
|
|
(>= {MIN_REASON_LEN} chars) explaining why it is needed"
|
|
));
|
|
}
|
|
} else if args.as_object().is_some_and(|m| !m.is_empty()) {
|
|
// Only require reason when there are meaningful arguments
|
|
return Verdict::Block(format!(
|
|
"MCP tool '{tool_name}' requires a 'reason' argument \
|
|
explaining the operation"
|
|
));
|
|
}
|
|
}
|
|
|
|
Verdict::Allow
|
|
}
|
|
|
|
/// Validate the `reason` argument for a mutating tool.
|
|
///
|
|
/// Flow: require the field to exist and be a non-empty string ≥
|
|
/// `MIN_REASON_LEN` chars after trimming.
|
|
///
|
|
/// Why: hook-style gates force the agent to articulate the *why* of
|
|
/// every change, which both deters lazy writes and produces a useful
|
|
/// audit trail in the edit log.
|
|
fn validate_reason(tool_name: &str, args: &serde_json::Value) -> Result<(), String> {
|
|
let reason = match args.get("reason") {
|
|
None => {
|
|
return Err(format!(
|
|
"{tool_name} requires a non-empty 'reason' argument \
|
|
explaining why the change is being made"
|
|
));
|
|
}
|
|
Some(v) => match v.as_str() {
|
|
Some(s) => s,
|
|
None => {
|
|
return Err(format!(
|
|
"{tool_name} 'reason' must be a string"
|
|
));
|
|
}
|
|
},
|
|
};
|
|
let trimmed = reason.trim();
|
|
if trimmed.is_empty() {
|
|
return Err(format!(
|
|
"{tool_name} 'reason' must not be empty"
|
|
));
|
|
}
|
|
if trimmed.len() < MIN_REASON_LEN {
|
|
return Err(format!(
|
|
"{tool_name} 'reason' must be at least {MIN_REASON_LEN} chars \
|
|
(got {}) — explain WHY, not just WHAT",
|
|
trimmed.len()
|
|
));
|
|
}
|
|
// Reject generic non-answers
|
|
let lower = trimmed.to_lowercase();
|
|
let non_answers = [
|
|
"fix", "update", "change", "edit", "modify",
|
|
"implement", "add", "remove", "delete",
|
|
"make it work", "make work", "test", "wip", "tbd",
|
|
];
|
|
if non_answers.iter().any(|n| lower == *n) {
|
|
return Err(format!(
|
|
"{tool_name} 'reason' '{trimmed}' is too generic — \
|
|
describe what changes and why (e.g. 'switch to Result<T> for \
|
|
safer error propagation per user request')"
|
|
));
|
|
}
|
|
Ok(())
|
|
}
|
|
|
|
/// Extract the textual content of a write/edit call, if any.
|
|
fn extract_content(tool_name: &str, args: &serde_json::Value) -> Option<String> {
|
|
match tool_name {
|
|
"write" => args.get("content").and_then(|v| v.as_str()).map(String::from),
|
|
"edit" => {
|
|
let old = args.get("old").and_then(|v| v.as_str()).unwrap_or("");
|
|
let new = args.get("new").and_then(|v| v.as_str()).unwrap_or("");
|
|
Some(format!("{old}\n{new}"))
|
|
}
|
|
_ => None,
|
|
}
|
|
}
|
|
|
|
/// Return the first pattern (case-insensitive substring) that matches
|
|
/// `text`, or `None` if no pattern matched.
|
|
fn first_match(text: &str, patterns: &'static [&'static str]) -> Option<&'static str> {
|
|
let lower = text.to_lowercase();
|
|
let iter: std::slice::Iter<'static, &'static str> = patterns.iter();
|
|
iter.copied().find(|p| lower.contains(&p.to_lowercase()))
|
|
}
|
|
|
|
/// Extract a candidate output path from a tool call, if one exists.
|
|
fn find_output_path(tool_name: &str, args: &serde_json::Value) -> Option<std::path::PathBuf> {
|
|
match tool_name {
|
|
"write" | "edit" | "delete" | "read" => {
|
|
args.get("path").and_then(|v| v.as_str()).map(std::path::PathBuf::from)
|
|
}
|
|
"bash" => {
|
|
let cmd = args.get("command").and_then(|v| v.as_str())?;
|
|
let lower = cmd.to_lowercase();
|
|
for prefix in &["cp ", "mv ", "install ", "ln -s ", "cat >", "cat >>"] {
|
|
if let Some(rest) = lower.strip_prefix(prefix) {
|
|
if let Some(target) = rest.split_whitespace().last() {
|
|
if !target.starts_with('-') {
|
|
return Some(std::path::PathBuf::from(target));
|
|
}
|
|
}
|
|
}
|
|
}
|
|
None
|
|
}
|
|
_ => None,
|
|
}
|
|
}
|
|
|
|
}
|
|
|
|
impl Default for Harness {
|
|
fn default() -> Self {
|
|
Harness
|
|
}
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
use serde_json::json;
|
|
|
|
fn parse_verdict(text: &str) -> Option<Verdict> {
|
|
let trimmed = text.trim();
|
|
if let Ok(v) = serde_json::from_str::<serde_json::Value>(trimmed) {
|
|
if let Some(verdict) = v.get("verdict").and_then(|v| v.as_str()) {
|
|
return match verdict.to_lowercase().as_str() {
|
|
"allow" => Some(Verdict::Allow),
|
|
"block" => Some(Verdict::Block(
|
|
v.get("reason").and_then(|r| r.as_str()).unwrap_or("blocked").to_string()
|
|
)),
|
|
_ => None,
|
|
};
|
|
}
|
|
}
|
|
for line in trimmed.lines() {
|
|
let l = line.trim().to_lowercase();
|
|
if l.starts_with("verdict: allow") {
|
|
return Some(Verdict::Allow);
|
|
}
|
|
if l.starts_with("verdict: block") {
|
|
let reason = line.split_once(':').map_or("blocked", |x| x.1).trim().to_string();
|
|
return Some(Verdict::Block(reason));
|
|
}
|
|
}
|
|
if trimmed.to_lowercase().contains("allow") {
|
|
return Some(Verdict::Allow);
|
|
}
|
|
if trimmed.to_lowercase().contains("block") {
|
|
return Some(Verdict::Block("blocked by classifier".to_string()));
|
|
}
|
|
None
|
|
}
|
|
|
|
#[test]
|
|
fn test_gate_tool_non_risky_always_allows() {
|
|
let roots: &[&std::path::Path] = &[];
|
|
let result = Harness::gate_tool_call("read", &json!({"path": "test.txt"}), roots);
|
|
assert_eq!(result, Verdict::Allow);
|
|
}
|
|
|
|
|
|
#[test]
|
|
fn test_parse_verdict_json_allow() {
|
|
let v = parse_verdict(r#"{"verdict": "allow"}"#);
|
|
assert_eq!(v, Some(Verdict::Allow));
|
|
}
|
|
|
|
#[test]
|
|
fn test_parse_verdict_json_block() {
|
|
let v = parse_verdict(r#"{"verdict": "block", "reason": "dangerous operation"}"#);
|
|
assert_eq!(v, Some(Verdict::Block("dangerous operation".to_string())));
|
|
}
|
|
|
|
#[test]
|
|
fn test_parse_verdict_text_allow() {
|
|
let v = parse_verdict("Verdict: Allow");
|
|
assert_eq!(v, Some(Verdict::Allow));
|
|
}
|
|
|
|
#[test]
|
|
fn test_parse_verdict_text_block() {
|
|
let v = parse_verdict("Verdict: Block - this operation is not allowed");
|
|
assert!(matches!(v, Some(Verdict::Block(_))));
|
|
}
|
|
|
|
#[test]
|
|
fn test_parse_verdict_fallback_allow() {
|
|
let v = parse_verdict("I think we should allow this operation");
|
|
assert_eq!(v, Some(Verdict::Allow));
|
|
}
|
|
|
|
#[test]
|
|
fn test_parse_verdict_fallback_block() {
|
|
let v = parse_verdict("This request should be blocked");
|
|
assert!(matches!(v, Some(Verdict::Block(_))));
|
|
}
|
|
|
|
#[test]
|
|
fn test_parse_verdict_unparseable() {
|
|
let v = parse_verdict("completely unrelated text with no keywords");
|
|
assert_eq!(v, None);
|
|
}
|
|
}
|