Skip to content

Commit a761575

Browse files
author
Yogthos
committed
Fix apply-patch: description, rename permission, create size limit
- Fix misleading 'atomically' in description — operations execute in order, prior ops remain applied on failure - Add permission check for rename new_path (previously only checked source) - Add 1MB limit on create content to prevent runaway file writes
1 parent b7094f6 commit a761575

1 file changed

Lines changed: 17 additions & 2 deletions

File tree

src/agent/tools/apply_patch.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,9 @@ use std::path::Path;
55

66
use crate::agent::tools::{AskSender, PermCheck, ToolError, check_perm_path};
77

8+
/// Max content size for a single create operation (1MB).
9+
const MAX_CREATE_SIZE: usize = 1_048_576;
10+
811
#[derive(Deserialize, Debug, Clone)]
912
#[serde(tag = "action")]
1013
pub enum PatchOp {
@@ -100,7 +103,7 @@ impl Tool for ApplyPatchTool {
100103
async fn definition(&self, _prompt: String) -> ToolDefinition {
101104
ToolDefinition {
102105
name: "apply_patch".to_string(),
103-
description: "Apply multiple file operations atomically in a single call. Supports create, update (by exact text match), delete, and rename. Operations execute in order and stop on first failure."
106+
description: "Apply multiple file operations in a single call. Supports create, update (by exact text match), delete, and rename. Operations execute in order and stop on first failure — prior operations that succeeded remain applied."
104107
.to_string(),
105108
parameters: serde_json::json!({
106109
"type": "object",
@@ -154,7 +157,7 @@ impl Tool for ApplyPatchTool {
154157
let mut results = Vec::new();
155158

156159
for op in &args.operations {
157-
// Permission check for the path
160+
// Permission check for the target path
158161
match op {
159162
PatchOp::Create { path, .. }
160163
| PatchOp::Update { path, .. }
@@ -164,6 +167,18 @@ impl Tool for ApplyPatchTool {
164167
.await?;
165168
}
166169
}
170+
// Rename also requires permission on the new path
171+
if let PatchOp::Rename { new_path, .. } = op {
172+
check_perm_path(&self.permission, &self.ask_tx, "apply_patch", new_path)
173+
.await?;
174+
}
175+
// Validate create content size
176+
if let PatchOp::Create { content, .. } = op {
177+
if content.len() > MAX_CREATE_SIZE {
178+
results.push(format!("FAILED: create content exceeds {} bytes ({} bytes provided)", MAX_CREATE_SIZE, content.len()));
179+
break;
180+
}
181+
}
167182

168183
let result = match op {
169184
PatchOp::Create { path, content } => apply_create(path, content),

0 commit comments

Comments
 (0)