mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-25 14:53:34 +00:00
fix(security): unified sensitive path protection across shell and file tools
Add a shared SENSITIVE_PATH_PATTERNS list in path_utils.rs that protects credentials, secrets, and private keys consistently across all tool types: - Shell tool: command_references_sensitive_path() scans commands for references to sensitive files (cat ~/.ssh/id_rsa, etc.) - File tools: is_sensitive_path() blocks ReadFileTool, WriteFileTool, ListDirTool, and ApplyPatchTool from accessing sensitive paths - ListDirTool: skips sensitive subdirectories during recursive traversal Previously, the shell tool had a small hardcoded list (5 patterns) in DANGEROUS_PATTERNS while file tools had no sensitive path protection at all. This created an asymmetric security model where file tools were more permissive than the shell tool. The shared list covers: SSH keys, GPG, AWS/Azure/GCP credentials, Kubernetes config, GitHub CLI tokens, Terraform credentials, Docker config, Vault tokens, shell history, .env files, git credentials, system shadow files, and sensitive key file extensions (.pem, .key, .p12, .pfx, .jks, .keystore). Safe suffixes (.example, .sample, .template) are excluded. 21 tests covering path detection, command scanning, safe suffixes, and normal file allowlisting. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
@@ -117,6 +117,13 @@ impl Tool for ReadFileTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive path. Use the appropriate secrets management tool instead.",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
// Check file size
|
||||
let metadata = fs::metadata(&path)
|
||||
.await
|
||||
@@ -256,6 +263,13 @@ impl Tool for WriteFileTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive path",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
// Create parent directories
|
||||
if let Some(parent) = path.parent() {
|
||||
fs::create_dir_all(parent).await.map_err(|e| {
|
||||
@@ -364,6 +378,13 @@ impl Tool for ListDirTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive directory",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
let mut entries = Vec::new();
|
||||
list_dir_inner(&path, &path, recursive, max_depth, 0, &mut entries).await?;
|
||||
|
||||
@@ -447,6 +468,10 @@ async fn list_dir_inner(
|
||||
entries.push(display);
|
||||
|
||||
if recursive && is_dir && current_depth < max_depth {
|
||||
// Skip sensitive directories during recursive traversal
|
||||
if super::path_utils::is_sensitive_path(&entry_path) {
|
||||
continue;
|
||||
}
|
||||
// Skip common non-essential directories
|
||||
let name = entry.file_name();
|
||||
let name_str = name.to_string_lossy();
|
||||
@@ -561,6 +586,13 @@ impl Tool for ApplyPatchTool {
|
||||
|
||||
let path = validate_path(path_str, self.base_dir.as_deref())?;
|
||||
|
||||
if super::path_utils::is_sensitive_path(&path) {
|
||||
return Err(ToolError::NotAuthorized(format!(
|
||||
"Access denied: '{}' is a sensitive path",
|
||||
path_str
|
||||
)));
|
||||
}
|
||||
|
||||
// Read current content
|
||||
let content = fs::read_to_string(&path)
|
||||
.await
|
||||
|
||||
@@ -4,9 +4,121 @@
|
||||
//! attacks and ensure paths stay within allowed sandboxes.
|
||||
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::LazyLock;
|
||||
|
||||
use crate::tools::tool::ToolError;
|
||||
|
||||
/// Paths that contain credentials, secrets, or private keys.
|
||||
/// Used by both file tools (exact path check) and shell tool (substring scan).
|
||||
/// Keep sorted by category for readability.
|
||||
static SENSITIVE_PATH_PATTERNS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
|
||||
vec![
|
||||
// SSH
|
||||
"/.ssh/",
|
||||
"/id_rsa",
|
||||
"/id_ed25519",
|
||||
"/id_ecdsa",
|
||||
"/id_dsa",
|
||||
"/authorized_keys",
|
||||
"/known_hosts",
|
||||
// GPG
|
||||
"/.gnupg/",
|
||||
// AWS
|
||||
"/.aws/credentials",
|
||||
"/.aws/config",
|
||||
// Kubernetes
|
||||
"/.kube/config",
|
||||
// Cloud providers
|
||||
"/.azure/",
|
||||
"/.gcloud/",
|
||||
"/.config/gcloud/",
|
||||
// Terraform
|
||||
"/.terraform.d/credentials.tfrc.json",
|
||||
// GitHub CLI
|
||||
"/.config/gh/hosts.yml",
|
||||
// Docker
|
||||
"/.docker/config.json",
|
||||
// Vault
|
||||
"/.vault-token",
|
||||
// Shell history
|
||||
"/.bash_history",
|
||||
"/.zsh_history",
|
||||
"/.histfile",
|
||||
// Env files (may contain secrets)
|
||||
"/.env",
|
||||
// Git credentials
|
||||
"/.git-credentials",
|
||||
"/.netrc",
|
||||
"/.pgpass",
|
||||
// IronClaw's own secrets
|
||||
"/.ironclaw/secrets/",
|
||||
// System
|
||||
"/etc/shadow",
|
||||
"/etc/gshadow",
|
||||
]
|
||||
});
|
||||
|
||||
/// File extensions that are always sensitive regardless of location.
|
||||
static SENSITIVE_EXTENSIONS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
|
||||
vec![".pem", ".key", ".p12", ".pfx", ".jks", ".keystore"]
|
||||
});
|
||||
|
||||
/// Suffixes that indicate a file is safe despite matching a sensitive pattern
|
||||
/// (e.g., `.env.example`, `.env.sample`).
|
||||
static SAFE_SUFFIXES: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
|
||||
vec![".example", ".sample", ".template", ".dist", ".bak.example"]
|
||||
});
|
||||
|
||||
/// Check if a resolved file path points to a sensitive location.
|
||||
/// Used by file tools (read, write, list_dir, apply_patch).
|
||||
pub fn is_sensitive_path(path: &Path) -> bool {
|
||||
let path_str = match path.canonicalize() {
|
||||
Ok(p) => p.to_string_lossy().to_string(),
|
||||
Err(_) => path.to_string_lossy().to_string(),
|
||||
};
|
||||
|
||||
// Safe suffixes override sensitive patterns
|
||||
let lower = path_str.to_lowercase();
|
||||
if SAFE_SUFFIXES.iter().any(|s| lower.ends_with(s)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Check sensitive path patterns
|
||||
if SENSITIVE_PATH_PATTERNS.iter().any(|p| path_str.contains(p)) {
|
||||
return true;
|
||||
}
|
||||
|
||||
// Check sensitive file extensions
|
||||
if let Some(ext) = path.extension().and_then(|e| e.to_str()) {
|
||||
let dot_ext = format!(".{}", ext.to_lowercase());
|
||||
if SENSITIVE_EXTENSIONS.iter().any(|e| *e == dot_ext) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
false
|
||||
}
|
||||
|
||||
/// Scan a shell command string for references to sensitive paths.
|
||||
/// Returns the first matched pattern, or None if the command is clean.
|
||||
/// Used by the shell tool to block `cat ~/.ssh/id_rsa` etc.
|
||||
pub fn command_references_sensitive_path(command: &str) -> Option<&'static str> {
|
||||
let normalized = command.to_lowercase();
|
||||
|
||||
for pattern in SENSITIVE_PATH_PATTERNS.iter() {
|
||||
// For path patterns, check case-insensitively
|
||||
if normalized.contains(&pattern.to_lowercase()) {
|
||||
return Some(pattern);
|
||||
}
|
||||
}
|
||||
|
||||
// Check for sensitive extensions in file arguments
|
||||
SENSITIVE_EXTENSIONS
|
||||
.iter()
|
||||
.find(|ext| normalized.contains(*ext))
|
||||
.copied()
|
||||
}
|
||||
|
||||
/// Normalize a path by resolving `.` and `..` components lexically (no filesystem access).
|
||||
///
|
||||
/// This is critical for security: `std::fs::canonicalize` only works on paths that exist,
|
||||
@@ -236,4 +348,90 @@ mod tests {
|
||||
let result = validate_path("a/b/../c.txt", Some(dir.path()));
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
// ── sensitive path tests ──
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_ssh() {
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.ssh/id_rsa")));
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.ssh/authorized_keys")));
|
||||
assert!(is_sensitive_path(Path::new("/root/.ssh/config")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_cloud_credentials() {
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.aws/credentials")));
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.kube/config")));
|
||||
assert!(is_sensitive_path(Path::new("/home/user/.azure/some_token")));
|
||||
assert!(is_sensitive_path(Path::new(
|
||||
"/home/user/.config/gh/hosts.yml"
|
||||
)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_system_secrets() {
|
||||
assert!(is_sensitive_path(Path::new("/etc/shadow")));
|
||||
assert!(is_sensitive_path(Path::new("/etc/gshadow")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_key_files_by_extension() {
|
||||
assert!(is_sensitive_path(Path::new("/tmp/server.pem")));
|
||||
assert!(is_sensitive_path(Path::new("/app/certs/private.key")));
|
||||
assert!(is_sensitive_path(Path::new("/home/user/keystore.p12")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_allows_safe_suffixes() {
|
||||
assert!(!is_sensitive_path(Path::new("/app/.env.example")));
|
||||
assert!(!is_sensitive_path(Path::new("/app/.env.sample")));
|
||||
assert!(!is_sensitive_path(Path::new("/app/.env.template")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_allows_normal_files() {
|
||||
assert!(!is_sensitive_path(Path::new("/app/src/main.rs")));
|
||||
assert!(!is_sensitive_path(Path::new("/home/user/README.md")));
|
||||
assert!(!is_sensitive_path(Path::new("/tmp/output.json")));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_is_sensitive_path_blocks_env_files() {
|
||||
assert!(is_sensitive_path(Path::new("/app/.env")));
|
||||
assert!(is_sensitive_path(Path::new("/app/.env.local")));
|
||||
assert!(is_sensitive_path(Path::new("/app/.env.production")));
|
||||
}
|
||||
|
||||
// ── command scanning tests ──
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_cat_ssh() {
|
||||
assert!(command_references_sensitive_path("cat ~/.ssh/id_rsa").is_some());
|
||||
assert!(command_references_sensitive_path("head -n 5 /home/user/.ssh/authorized_keys").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_aws() {
|
||||
assert!(command_references_sensitive_path("cat ~/.aws/credentials").is_some());
|
||||
assert!(command_references_sensitive_path("grep key ~/.aws/config").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_etc_shadow() {
|
||||
assert!(command_references_sensitive_path("cat /etc/shadow").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_catches_key_extensions() {
|
||||
assert!(command_references_sensitive_path("cp server.pem /tmp/").is_some());
|
||||
assert!(command_references_sensitive_path("cat private.key").is_some());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_command_references_sensitive_path_allows_safe_commands() {
|
||||
assert!(command_references_sensitive_path("ls -la").is_none());
|
||||
assert!(command_references_sensitive_path("cargo build").is_none());
|
||||
assert!(command_references_sensitive_path("git status").is_none());
|
||||
assert!(command_references_sensitive_path("cat README.md").is_none());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -83,6 +83,9 @@ static BLOCKED_COMMANDS: LazyLock<HashSet<&'static str>> = LazyLock::new(|| {
|
||||
});
|
||||
|
||||
/// Patterns that indicate potentially dangerous commands.
|
||||
/// Note: sensitive file paths (/.ssh/, /etc/shadow, etc.) are now handled by
|
||||
/// `command_references_sensitive_path` in path_utils.rs for consistency with
|
||||
/// file tool protections. This list covers command-level dangers only.
|
||||
static DANGEROUS_PATTERNS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
|
||||
vec![
|
||||
"sudo ",
|
||||
@@ -93,11 +96,6 @@ static DANGEROUS_PATTERNS: LazyLock<Vec<&'static str>> = LazyLock::new(|| {
|
||||
"eval ",
|
||||
"$(curl",
|
||||
"$(wget",
|
||||
"/etc/passwd",
|
||||
"/etc/shadow",
|
||||
"~/.ssh",
|
||||
".bash_history",
|
||||
"id_rsa",
|
||||
]
|
||||
});
|
||||
|
||||
@@ -622,6 +620,11 @@ impl ShellTool {
|
||||
}
|
||||
}
|
||||
|
||||
// Block commands that reference sensitive file paths (shared with file tools)
|
||||
if super::path_utils::command_references_sensitive_path(cmd).is_some() {
|
||||
return Some("Command references sensitive file path");
|
||||
}
|
||||
|
||||
None
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user