Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 9 additions & 7 deletions src/agent/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -249,16 +249,18 @@ pub async fn build_agent_inner<M: CompletionModel + 'static>(
vec![enter, exit]
});

// Web tools: gated on config + env var
let websearch_enabled = cfg
.tools
.as_ref()
.and_then(|t| t.websearch)
.unwrap_or(false)
// Web tools: gated on config + (for websearch) a separate
// env-var escape hatch. CONFIG.md documents both keys as
// defaulting to `true`; the previous `unwrap_or(false)`
// disabled them by default contrary to the docs. Now
// matches the documented behavior — explicit `false` in
// config disables; absent or `true` enables (the runtime
// API-key check still has to pass for websearch).
let websearch_enabled = cfg.tools.as_ref().and_then(|t| t.websearch).unwrap_or(true)
|| std::env::var("WEBSEARCH_ENABLED")
.map(|v| v == "true" || v == "1")
.unwrap_or(false);
let webfetch_enabled = cfg.tools.as_ref().and_then(|t| t.webfetch).unwrap_or(false);
let webfetch_enabled = cfg.tools.as_ref().and_then(|t| t.webfetch).unwrap_or(true);

let websearch_tool = websearch_enabled
.then(|| std::env::var("EXA_API_KEY").ok())
Expand Down
8 changes: 7 additions & 1 deletion src/agent/tools/find_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use rig::tool::Tool;

use crate::agent::tools::cache::ToolCache;
use crate::agent::tools::{
AskSender, FindFilesArgs, MAX_FIND_RESULTS, PermCheck, ToolError, check_perm, is_skip_dir,
AskSender, FindFilesArgs, MAX_FIND_RESULTS, PermCheck, ToolError, check_perm, check_perm_path,
is_skip_dir,
};

pub struct FindFilesTool {
Expand Down Expand Up @@ -71,6 +72,11 @@ impl Tool for FindFilesTool {

async fn call(&self, args: FindFilesArgs) -> Result<String, ToolError> {
check_perm(&self.permission, &self.ask_tx, "find_files", &args.pattern).await?;
// Path-side check: external_directory rules + Accept-mode
// gating live in check_perm_path. Without this find_files
// over `/etc` skipped the rules entirely.
let perm_path = args.path.as_deref().unwrap_or(".");
check_perm_path(&self.permission, &self.ask_tx, "find_files", perm_path).await?;

let cache_key = format!(
"find_files:{}:{}:hidden={}",
Expand Down
7 changes: 6 additions & 1 deletion src/agent/tools/glob.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::path::Path;

use crate::agent::tools::MAX_FIND_RESULTS;
use crate::agent::tools::cache::ToolCache;
use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm};
use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm, check_perm_path};

pub struct GlobTool {
pub permission: Option<PermCheck>,
Expand Down Expand Up @@ -128,6 +128,11 @@ impl Tool for GlobTool {
&format!("pattern:{}", args.pattern),
)
.await?;
// Path-side check: external_directory rules + Accept-mode
// working-dir gating live in check_perm_path. Without this
// a glob over `/etc` or `~/.ssh` skipped the rules entirely.
let perm_path = args.path.as_deref().unwrap_or(".");
check_perm_path(&self.permission, &self.ask_tx, "glob", perm_path).await?;

let cache_key = format!(
"glob:{}:{}:hidden={}",
Expand Down
10 changes: 9 additions & 1 deletion src/agent/tools/grep.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use rig::tool::Tool;

use crate::agent::tools::cache::ToolCache;
use crate::agent::tools::{
AskSender, GrepArgs, MAX_GREP_RESULTS, PermCheck, ToolError, check_perm, is_skip_dir,
AskSender, GrepArgs, MAX_GREP_RESULTS, PermCheck, ToolError, check_perm, check_perm_path,
is_skip_dir,
};

pub struct GrepTool {
Expand Down Expand Up @@ -99,6 +100,13 @@ impl Tool for GrepTool {

async fn call(&self, args: GrepArgs) -> Result<String, ToolError> {
check_perm(&self.permission, &self.ask_tx, "grep", &args.pattern).await?;
// Path-side check: previously grep accepted any path
// (`grep("x", "/etc")`) because only the pattern was
// permission-checked. external_directory rules + the
// working-dir Accept-mode logic live in check_perm_path,
// so an extra call here closes the bypass.
let perm_path = args.path.as_deref().unwrap_or(".");
check_perm_path(&self.permission, &self.ask_tx, "grep", perm_path).await?;

let cache_key = format!(
"grep:{}:{}:{}:{}:hidden={}",
Expand Down
6 changes: 4 additions & 2 deletions src/agent/tools/semantic/find_callees.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rig::completion::ToolDefinition;
use rig::tool::Tool;
use serde::Deserialize;

use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm};
use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm_path};
use crate::semantic::SymbolIndex;

pub struct FindCalleesTool {
Expand Down Expand Up @@ -63,7 +63,9 @@ impl Tool for FindCalleesTool {
}

async fn call(&self, args: Args) -> Result<String, ToolError> {
check_perm(&self.permission, &self.ask_tx, "find_callees", &args.path).await?;
// `args.path` is a real file path; use the path-aware
// permission check so external_directory rules apply.
check_perm_path(&self.permission, &self.ask_tx, "find_callees", &args.path).await?;

let file_path = PathBuf::from(&args.path);

Expand Down
6 changes: 4 additions & 2 deletions src/agent/tools/semantic/get_symbol_body.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rig::completion::ToolDefinition;
use rig::tool::Tool;
use serde::Deserialize;

use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm};
use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm_path};
use crate::semantic::SymbolIndex;

pub struct GetSymbolBodyTool {
Expand Down Expand Up @@ -63,7 +63,9 @@ impl Tool for GetSymbolBodyTool {
}

async fn call(&self, args: Args) -> Result<String, ToolError> {
check_perm(
// Path-aware check so external_directory rules apply —
// `args.path` is the real file path we'll read symbols from.
check_perm_path(
&self.permission,
&self.ask_tx,
"get_symbol_body",
Expand Down
7 changes: 5 additions & 2 deletions src/agent/tools/semantic/list_symbols.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rig::completion::ToolDefinition;
use rig::tool::Tool;
use serde::Deserialize;

use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm};
use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm_path};
use crate::semantic::SymbolIndex;
use crate::semantic::types::SymbolKind;

Expand Down Expand Up @@ -63,7 +63,10 @@ impl Tool for ListSymbolsTool {
}

async fn call(&self, args: Args) -> Result<String, ToolError> {
check_perm(
// Path-aware check so external_directory rules apply.
// `args.path` is None when scanning the whole project — pass
// "." which check_perm_path resolves against the working dir.
check_perm_path(
&self.permission,
&self.ask_tx,
"list_symbols",
Expand Down
53 changes: 44 additions & 9 deletions src/agent/tools/webfetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,20 @@ fn html_to_markdown(html: &str) -> String {
html2text::from_read(html.as_bytes(), 100).unwrap_or_else(|_| html.to_string())
}

/// True if `url` has an explicit `http://` or `https://` scheme,
/// case-insensitively. URL schemes are case-insensitive per RFC
/// 3986; checking only the lowercase form let `HTTP://...` and
/// other case variants bypass scheme + SSRF defenses entirely.
fn has_http_scheme(url: &str) -> bool {
let prefix = url.get(..7).map(str::to_ascii_lowercase);
let prefix8 = url.get(..8).map(str::to_ascii_lowercase);
matches!(prefix.as_deref(), Some("http://")) || matches!(prefix8.as_deref(), Some("https://"))
}

/// Normalize a URL. Respects explicit http:// (localhost, internal services).
/// Only prepends https:// when no scheme is present.
fn normalize_url(url: &str) -> String {
if url.starts_with("http://") || url.starts_with("https://") {
if has_http_scheme(url) {
url.to_string()
} else {
format!("https://{}", url)
Expand All @@ -47,7 +57,7 @@ fn normalize_url(url: &str) -> String {
/// explicit at the dirge boundary rather than relying on the HTTP
/// client's policy.
fn validate_url_scheme(url: &str) -> Result<(), String> {
if url.starts_with("http://") || url.starts_with("https://") {
if has_http_scheme(url) {
Ok(())
} else {
Err(format!(
Expand All @@ -71,13 +81,18 @@ fn validate_url_host_safety(url: &str) -> Result<(), String> {
if std::env::var("DIRGE_WEBFETCH_ALLOW_PRIVATE").as_deref() == Ok("1") {
return Ok(());
}
// Strip scheme to extract host. We don't pull in a URL parser
// crate here; webfetch's URL handling is already string-based
// and this is one more check at the boundary.
let after_scheme = url
.strip_prefix("https://")
.or_else(|| url.strip_prefix("http://"))
.unwrap_or(url);
// Strip scheme to extract host. Case-insensitive — URL schemes
// are case-insensitive per RFC 3986, and an attacker using
// `HTTPS://1.2.3.4/` would otherwise skip past the strip and
// get the scheme treated as host text.
let scheme_len = if url.len() >= 8 && url[..8].eq_ignore_ascii_case("https://") {
8
} else if url.len() >= 7 && url[..7].eq_ignore_ascii_case("http://") {
7
} else {
0
};
let after_scheme = &url[scheme_len..];
// Host extraction handles bracketed IPv6 (`[::1]`) before
// falling back to the bare host:port form. Without the
// bracket-aware path, `rsplit_once(':')` would chop `[::1]`
Expand Down Expand Up @@ -394,6 +409,26 @@ mod tests {
assert!(validate_url_scheme("").is_err());
}

/// Regression: scheme matching must be case-insensitive (RFC
/// 3986). Previously `starts_with("http://")` only matched
/// lowercase, so `HTTP://169.254.169.254` bypassed scheme
/// + SSRF defenses entirely.
#[test]
fn scheme_matching_is_case_insensitive() {
// Accepted forms.
assert!(validate_url_scheme("HTTP://example.com").is_ok());
assert!(validate_url_scheme("HTTPS://example.com").is_ok());
assert!(validate_url_scheme("Http://Example.Com").is_ok());
assert!(validate_url_scheme("HtTpS://x").is_ok());
// Rejected (no http/https scheme prefix).
assert!(validate_url_scheme("FILE:///etc/passwd").is_err());
// SSRF defense must still trigger for case-variant schemes.
if std::env::var("DIRGE_WEBFETCH_ALLOW_PRIVATE").as_deref() != Ok("1") {
assert!(validate_url_host_safety("HTTP://169.254.169.254/").is_err());
assert!(validate_url_host_safety("HTTPS://127.0.0.1/").is_err());
}
}

/// SSRF defense: AWS metadata + private + loopback + link-local
/// IPs are refused unless the env opt-in is set. Pin the exact
/// hosts that bug bounty reports keep hitting.
Expand Down
3 changes: 0 additions & 3 deletions src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ pub struct Cli {
#[arg(long = "temperature", help = "Model temperature (0.0 to 2.0)")]
pub temperature: Option<f64>,

#[arg(short = 't', long = "tools", help = "Allowlist specific tools")]
pub tools: Vec<String>,

#[arg(long = "no-tools", help = "Disable all tools")]
pub no_tools: bool,

Expand Down
Loading