Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 4 additions & 0 deletions src-rust/crates/tools/src/apply_patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,10 @@ impl Tool for ApplyPatchTool {
let path = ctx.resolve_path(&fp.path);
debug!(path = %path.display(), "ApplyPatch processing file");

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &path) {
return ToolResult::error(e.to_string());
}

// Read current content (or empty string for new files).
let original_content = if path.exists() {
match tokio::fs::read_to_string(&path).await {
Expand Down
5 changes: 5 additions & 0 deletions src-rust/crates/tools/src/batch_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,11 @@ impl Tool for BatchEditTool {
let path = ctx.resolve_path(&edit.file_path);
debug!(path = %path.display(), index = i, "BatchEdit pre-check");

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &path) {
pre_check_errors.push(format!("Edit {}: {}", i, e));
continue;
}

let content = match tokio::fs::read_to_string(&path).await {
Ok(c) => c,
Err(e) => {
Expand Down
4 changes: 4 additions & 0 deletions src-rust/crates/tools/src/file_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl Tool for FileEditTool {
let path = ctx.resolve_path(&params.file_path);
debug!(path = %path.display(), "Editing file");

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &path) {
return ToolResult::error(e.to_string());
}

// Permission check
if let Err(e) =
ctx.check_permission(self.name(), &format!("Edit {}", path.display()), false)
Expand Down
4 changes: 4 additions & 0 deletions src-rust/crates/tools/src/file_read.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ impl Tool for FileReadTool {
let path = ctx.resolve_path(&params.file_path);
debug!(path = %path.display(), "Reading file");

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &path) {
return ToolResult::error(e.to_string());
}

// Permission check
if let Err(e) = ctx.check_permission_for_path(
self.name(),
Expand Down
4 changes: 4 additions & 0 deletions src-rust/crates/tools/src/file_write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ impl Tool for FileWriteTool {
let path = ctx.resolve_path(&params.file_path);
debug!(path = %path.display(), "Writing file");

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &path) {
return ToolResult::error(e.to_string());
}

// Permission check
if let Err(e) =
ctx.check_permission(self.name(), &format!("Write {}", path.display()), false)
Expand Down
7 changes: 7 additions & 0 deletions src-rust/crates/tools/src/glob_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ impl Tool for GlobTool {
.map(|p| ctx.resolve_path(p))
.unwrap_or_else(|| ctx.working_dir.clone());

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &base_dir) {
return ToolResult::error(e.to_string());
}

if let Err(e) = ctx.check_permission_for_path(
self.name(),
&format!("Glob {} in {}", params.pattern, base_dir.display()),
Expand All @@ -88,6 +92,9 @@ impl Tool for GlobTool {
Ok(paths) => {
let mut out = Vec::new();
for path in paths.filter_map(|p| p.ok()) {
if let Err(e) = ctx.check_hosted_repair_path(self.name(), &path) {
return ToolResult::error(e.to_string());
}
if !ctx.path_is_within_workspace(&path) {
if let Err(e) = ctx.check_permission_for_path(
self.name(),
Expand Down
8 changes: 8 additions & 0 deletions src-rust/crates/tools/src/grep_tool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,10 @@ impl Tool for GrepTool {
.map(|p| ctx.resolve_path(p))
.unwrap_or_else(|| ctx.working_dir.clone());

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &search_path) {
return ToolResult::error(e.to_string());
}

if let Err(e) = ctx.check_permission_for_path(
self.name(),
&format!("Grep {} in {}", params.pattern, search_path.display()),
Expand Down Expand Up @@ -219,6 +223,10 @@ impl Tool for GrepTool {

let path = entry.path();

if let Err(e) = ctx.check_hosted_repair_path(self.name(), path) {
return ToolResult::error(e.to_string());
}

if !ctx.path_is_within_workspace(path) {
if let Err(e) = ctx.check_permission_for_path(
self.name(),
Expand Down
137 changes: 136 additions & 1 deletion src-rust/crates/tools/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use claurst_core::permissions::{PermissionDecision, PermissionHandler, Permissio
use claurst_core::types::ToolDefinition;
use serde_json::Value;
use std::collections::{HashMap, VecDeque};
use std::path::PathBuf;
use std::path::{Component, Path, PathBuf};
use std::sync::atomic::{AtomicUsize, Ordering};
use std::sync::Arc;

Expand Down Expand Up @@ -413,6 +413,28 @@ impl ToolContext {
self.request_permission_inner(request)
}

/// Hosted repair runs GitHub-supplied prompts with permission bypass, so
/// file tools must enforce the repository boundary themselves.
pub fn check_hosted_repair_path(
&self,
tool_name: &str,
path: &Path,
) -> Result<(), claurst_core::error::ClaudeError> {
if !self.config.hosted_review.allow_file_write_tools {
return Ok(());
}

if self.path_is_within_working_dir(path) {
return Ok(());
}

Err(claurst_core::error::ClaudeError::PermissionDenied(format!(
"Permission denied for tool '{}': hosted repair file tools are limited to {}",
tool_name,
self.working_dir.display()
)))
}

/// Like `check_permission` but also passes structured `details` text
/// (e.g. a risk explanation) that the TUI permission dialog can display.
pub fn check_permission_with_details(
Expand Down Expand Up @@ -450,6 +472,13 @@ impl ToolContext {
roots.iter().any(|root| resolved.starts_with(root))
}

fn path_is_within_working_dir(&self, path: &Path) -> bool {
let root = std::fs::canonicalize(&self.working_dir)
.unwrap_or_else(|_| normalize_path(&self.working_dir));
let resolved = canonicalize_existing_prefix(path);
resolved.starts_with(root)
}
Comment thread
BunsDev marked this conversation as resolved.

pub fn check_permission_with_details_and_path(
&self,
tool_name: &str,
Expand Down Expand Up @@ -494,6 +523,45 @@ impl ToolContext {
}
}

fn normalize_path(path: &Path) -> PathBuf {
let mut normalized = PathBuf::new();
for component in path.components() {
match component {
Component::CurDir => {}
Component::ParentDir => {
normalized.pop();
}
other => normalized.push(other.as_os_str()),
}
}
normalized
}
Comment thread
BunsDev marked this conversation as resolved.
Outdated

fn canonicalize_existing_prefix(path: &Path) -> PathBuf {
let normalized = normalize_path(path);
let mut ancestor = normalized.as_path();
let mut suffix = Vec::new();

loop {
if ancestor.exists() {
let mut resolved =
std::fs::canonicalize(ancestor).unwrap_or_else(|_| ancestor.to_path_buf());
for component in suffix.iter().rev() {
resolved.push(component);
}
return normalize_path(&resolved);
}

match (ancestor.parent(), ancestor.file_name()) {
(Some(parent), Some(file_name)) => {
suffix.push(file_name.to_os_string());
ancestor = parent;
}
_ => return normalized,
}
}
}

/// The trait every tool must implement.
#[async_trait]
pub trait Tool: Send + Sync {
Expand Down Expand Up @@ -936,6 +1004,73 @@ mod tests {
assert!(error.contains("generic reason"));
}

#[test]
fn hosted_repair_path_check_allows_working_dir_files() {
use claurst_core::permissions::AutoPermissionHandler;

let temp = tempfile::tempdir().expect("tempdir");
let mut ctx = test_tool_context(Arc::new(AutoPermissionHandler {
mode: claurst_core::config::PermissionMode::BypassPermissions,
}));
ctx.working_dir = temp.path().to_path_buf();
ctx.config.hosted_review.enabled = true;
ctx.config.hosted_review.allow_file_write_tools = true;

let path = ctx.resolve_path("src/new.rs");
assert!(ctx.check_hosted_repair_path("Write", &path).is_ok());
}

#[test]
fn hosted_repair_path_check_rejects_parent_escape() {
use claurst_core::permissions::AutoPermissionHandler;

let temp = tempfile::tempdir().expect("tempdir");
let workspace = temp.path().join("workspace");
std::fs::create_dir(&workspace).expect("workspace dir");
let mut ctx = test_tool_context(Arc::new(AutoPermissionHandler {
mode: claurst_core::config::PermissionMode::BypassPermissions,
}));
ctx.working_dir = workspace;
ctx.config.hosted_review.enabled = true;
ctx.config.hosted_review.allow_file_write_tools = true;

let path = ctx.resolve_path("../outside.txt");
let error = ctx
.check_hosted_repair_path("Read", &path)
.expect_err("parent escape must be rejected")
.to_string();

assert!(error.contains("hosted repair file tools are limited"));
}

#[cfg(unix)]
#[test]
fn hosted_repair_path_check_rejects_symlink_escape() {
use claurst_core::permissions::AutoPermissionHandler;
use std::os::unix::fs::symlink;

let temp = tempfile::tempdir().expect("tempdir");
let workspace = temp.path().join("workspace");
let outside = temp.path().join("outside");
std::fs::create_dir(&workspace).expect("workspace dir");
std::fs::create_dir(&outside).expect("outside dir");
symlink(&outside, workspace.join("link")).expect("symlink");
let mut ctx = test_tool_context(Arc::new(AutoPermissionHandler {
mode: claurst_core::config::PermissionMode::BypassPermissions,
}));
ctx.working_dir = workspace;
ctx.config.hosted_review.enabled = true;
ctx.config.hosted_review.allow_file_write_tools = true;

let path = ctx.resolve_path("link/new.txt");
let error = ctx
.check_hosted_repair_path("Write", &path)
.expect_err("symlink escape must be rejected")
.to_string();

assert!(error.contains("hosted repair file tools are limited"));
}

// ---- PermissionLevel tests ---------------------------------------------

#[test]
Expand Down
4 changes: 4 additions & 0 deletions src-rust/crates/tools/src/notebook_edit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,10 @@ impl Tool for NotebookEditTool {
return ToolResult::error("File must have .ipynb extension".to_string());
}

if let Err(e) = ctx.check_hosted_repair_path(self.name(), &path) {
return ToolResult::error(e.to_string());
}

// Permission check
if let Err(e) = ctx.check_permission(
self.name(),
Expand Down