scopeの再設計
This commit is contained in:
@@ -158,7 +158,10 @@ mod tests {
|
||||
|
||||
fn setup() -> (TempDir, ScopedFs, Tracker) {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
(dir, fs, Tracker::new())
|
||||
}
|
||||
|
||||
|
||||
@@ -16,6 +16,9 @@ pub enum ToolsError {
|
||||
#[error("path is outside allowed scope: {}", .0.display())]
|
||||
OutOfScope(PathBuf),
|
||||
|
||||
#[error("path is read-only in this scope: {}", .0.display())]
|
||||
ReadOnly(PathBuf),
|
||||
|
||||
#[error("path is a directory: {}", .0.display())]
|
||||
IsDirectory(PathBuf),
|
||||
|
||||
@@ -70,6 +73,7 @@ impl From<ToolsError> for ToolError {
|
||||
match err {
|
||||
RelativePath(_)
|
||||
| OutOfScope(_)
|
||||
| ReadOnly(_)
|
||||
| IsDirectory(_)
|
||||
| NotRead(_)
|
||||
| ExternallyModified(_)
|
||||
|
||||
@@ -6,6 +6,7 @@ use std::time::SystemTime;
|
||||
|
||||
use async_trait::async_trait;
|
||||
use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput};
|
||||
use manifest::Scope;
|
||||
use serde::Deserialize;
|
||||
|
||||
use crate::error::ToolsError;
|
||||
@@ -47,12 +48,13 @@ impl Tool for GlobTool {
|
||||
let base = params
|
||||
.path
|
||||
.clone()
|
||||
.unwrap_or_else(|| self.fs.scope().root().to_path_buf());
|
||||
.unwrap_or_else(|| self.fs.pwd().to_path_buf());
|
||||
let pattern = params.pattern.clone();
|
||||
let scope = self.fs.scope().clone();
|
||||
|
||||
// ignore::Walk is synchronous; run it on a blocking thread so we
|
||||
// don't stall the runtime for large trees.
|
||||
let results = tokio::task::spawn_blocking(move || run_glob(&base, &pattern))
|
||||
let results = tokio::task::spawn_blocking(move || run_glob(&base, &pattern, &scope))
|
||||
.await
|
||||
.map_err(|e| ToolError::Internal(format!("spawn_blocking failed: {e}")))??;
|
||||
|
||||
@@ -92,7 +94,7 @@ impl Tool for GlobTool {
|
||||
}
|
||||
}
|
||||
|
||||
fn run_glob(base: &Path, pattern: &str) -> Result<Vec<PathBuf>, ToolsError> {
|
||||
fn run_glob(base: &Path, pattern: &str, scope: &Scope) -> Result<Vec<PathBuf>, ToolsError> {
|
||||
if !base.is_absolute() {
|
||||
return Err(ToolsError::RelativePath(base.to_path_buf()));
|
||||
}
|
||||
@@ -131,6 +133,9 @@ fn run_glob(base: &Path, pattern: &str) -> Result<Vec<PathBuf>, ToolsError> {
|
||||
if !glob.is_match(rel) {
|
||||
continue;
|
||||
}
|
||||
if !scope.is_readable(entry.path()) {
|
||||
continue;
|
||||
}
|
||||
let mtime = entry
|
||||
.metadata()
|
||||
.ok()
|
||||
@@ -164,7 +169,10 @@ mod tests {
|
||||
|
||||
fn setup() -> (TempDir, ScopedFs) {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
(dir, fs)
|
||||
}
|
||||
|
||||
@@ -237,6 +245,43 @@ mod tests {
|
||||
assert!(matches!(err, ToolError::InvalidArgument(_)));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn glob_filters_results_by_scope_readability() {
|
||||
use manifest::{Permission, ScopeConfig, ScopeRule};
|
||||
|
||||
let dir = TempDir::new().unwrap();
|
||||
let secret_dir = dir.path().join("secret");
|
||||
std::fs::create_dir(&secret_dir).unwrap();
|
||||
touch(&dir.path().join("visible.rs"), "");
|
||||
touch(&secret_dir.join("hidden.rs"), "");
|
||||
|
||||
let cfg = ScopeConfig {
|
||||
allow: vec![ScopeRule {
|
||||
target: dir.path().to_path_buf(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
}],
|
||||
deny: vec![ScopeRule {
|
||||
target: secret_dir.clone(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
}],
|
||||
};
|
||||
let scope = Scope::from_config(&cfg, dir.path()).unwrap();
|
||||
let fs = ScopedFs::new(scope, dir.path().to_path_buf());
|
||||
|
||||
let def = glob_tool(fs);
|
||||
let (_, tool) = def();
|
||||
let inp = serde_json::json!({ "pattern": "**/*.rs" });
|
||||
let out = tool.execute(&inp.to_string()).await.unwrap();
|
||||
let body = out.content.unwrap_or_default();
|
||||
assert!(body.contains("visible.rs"));
|
||||
assert!(
|
||||
!body.contains("hidden.rs"),
|
||||
"scope-denied file leaked into glob output: {body}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn glob_honors_hidden_files() {
|
||||
let (dir, fs) = setup();
|
||||
|
||||
@@ -11,6 +11,7 @@ use ignore::WalkBuilder;
|
||||
use ignore::overrides::OverrideBuilder;
|
||||
use ignore::types::TypesBuilder;
|
||||
use llm_worker::tool::{Tool, ToolDefinition, ToolError, ToolMeta, ToolOutput};
|
||||
use manifest::Scope;
|
||||
use serde::Deserialize;
|
||||
|
||||
use crate::error::ToolsError;
|
||||
@@ -91,8 +92,9 @@ impl Tool for GrepTool {
|
||||
"Grep"
|
||||
);
|
||||
|
||||
let default_base = self.fs.scope().root().to_path_buf();
|
||||
let report = tokio::task::spawn_blocking(move || run_grep(default_base, params))
|
||||
let default_base = self.fs.pwd().to_path_buf();
|
||||
let scope = self.fs.scope().clone();
|
||||
let report = tokio::task::spawn_blocking(move || run_grep(default_base, params, &scope))
|
||||
.await
|
||||
.map_err(|e| ToolError::Internal(format!("spawn_blocking failed: {e}")))??;
|
||||
|
||||
@@ -228,7 +230,7 @@ impl GrepReport {
|
||||
}
|
||||
}
|
||||
|
||||
fn run_grep(default_base: PathBuf, p: GrepParams) -> Result<GrepReport, ToolsError> {
|
||||
fn run_grep(default_base: PathBuf, p: GrepParams, scope: &Scope) -> Result<GrepReport, ToolsError> {
|
||||
let matcher = RegexMatcherBuilder::new()
|
||||
.case_insensitive(p.case_insensitive)
|
||||
.multi_line(p.multiline)
|
||||
@@ -309,6 +311,9 @@ fn run_grep(default_base: PathBuf, p: GrepParams) -> Result<GrepReport, ToolsErr
|
||||
continue;
|
||||
}
|
||||
let path = entry.path();
|
||||
if !scope.is_readable(path) {
|
||||
continue;
|
||||
}
|
||||
|
||||
match mode {
|
||||
GrepOutputMode::FilesWithMatches => {
|
||||
@@ -472,7 +477,10 @@ mod tests {
|
||||
|
||||
fn setup() -> (TempDir, ScopedFs) {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
(dir, fs)
|
||||
}
|
||||
|
||||
@@ -483,6 +491,43 @@ mod tests {
|
||||
fs::write(path, content).unwrap();
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn grep_filters_results_by_scope_readability() {
|
||||
use manifest::{Permission, ScopeConfig, ScopeRule};
|
||||
|
||||
let dir = TempDir::new().unwrap();
|
||||
let secret_dir = dir.path().join("secret");
|
||||
fs::create_dir(&secret_dir).unwrap();
|
||||
touch(&dir.path().join("visible.txt"), "needle\n");
|
||||
touch(&secret_dir.join("hidden.txt"), "needle\n");
|
||||
|
||||
let cfg = ScopeConfig {
|
||||
allow: vec![ScopeRule {
|
||||
target: dir.path().to_path_buf(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
}],
|
||||
deny: vec![ScopeRule {
|
||||
target: secret_dir.clone(),
|
||||
permission: Permission::Read,
|
||||
recursive: true,
|
||||
}],
|
||||
};
|
||||
let scope = Scope::from_config(&cfg, dir.path()).unwrap();
|
||||
let scoped = ScopedFs::new(scope, dir.path().to_path_buf());
|
||||
|
||||
let def = grep_tool(scoped);
|
||||
let (_, tool) = def();
|
||||
let inp = serde_json::json!({ "pattern": "needle" });
|
||||
let out = tool.execute(&inp.to_string()).await.unwrap();
|
||||
let body = out.content.unwrap_or_default();
|
||||
assert!(body.contains("visible.txt"));
|
||||
assert!(
|
||||
!body.contains("hidden.txt"),
|
||||
"scope-denied file leaked into grep output: {body}"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn grep_files_with_matches_default() {
|
||||
let (dir, fs) = setup();
|
||||
|
||||
@@ -137,7 +137,10 @@ mod tests {
|
||||
|
||||
fn setup() -> (TempDir, ScopedFs, Tracker) {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
(dir, fs, Tracker::new())
|
||||
}
|
||||
|
||||
|
||||
@@ -1,15 +1,16 @@
|
||||
//! Scope-aware filesystem primitive.
|
||||
//!
|
||||
//! `ScopedFs` represents **only** the write-block boundary: it knows a
|
||||
//! [`manifest::Scope`] and refuses writes outside of it. It carries no
|
||||
//! per-session state and is cheap to clone (pod-lifetime, reusable across
|
||||
//! sessions). The read-before-edit policy lives separately in
|
||||
//! [`crate::Tracker`].
|
||||
//! `ScopedFs` is the write/read gate layered on top of a [`manifest::Scope`]
|
||||
//! and a Pod's working directory. The scope decides which paths are
|
||||
//! readable and writable; the pwd is carried alongside for convenience
|
||||
//! (Glob/Grep default their search base to it).
|
||||
//!
|
||||
//! Reads are unrestricted by design (see `tickets/builtin-tools.md`).
|
||||
//! `ScopedFs` is cheap to clone (`Arc` inside) and carries no per-session
|
||||
//! state — the read-before-edit policy lives separately in
|
||||
//! [`crate::Tracker`].
|
||||
|
||||
use std::io::Write as _;
|
||||
use std::path::Path;
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
use manifest::Scope;
|
||||
@@ -19,6 +20,7 @@ use crate::error::ToolsError;
|
||||
#[derive(Debug)]
|
||||
struct ScopedFsInner {
|
||||
scope: Scope,
|
||||
pwd: PathBuf,
|
||||
}
|
||||
|
||||
/// Scope-aware filesystem handle. Clone-cheap (`Arc` inside).
|
||||
@@ -35,10 +37,10 @@ pub struct WriteOutcome {
|
||||
}
|
||||
|
||||
impl ScopedFs {
|
||||
/// Create a new [`ScopedFs`] wrapping the given [`Scope`].
|
||||
pub fn new(scope: Scope) -> Self {
|
||||
/// Create a new [`ScopedFs`] wrapping the given [`Scope`] and pwd.
|
||||
pub fn new(scope: Scope, pwd: PathBuf) -> Self {
|
||||
Self {
|
||||
inner: Arc::new(ScopedFsInner { scope }),
|
||||
inner: Arc::new(ScopedFsInner { scope, pwd }),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,18 +49,27 @@ impl ScopedFs {
|
||||
&self.inner.scope
|
||||
}
|
||||
|
||||
/// The Pod's working directory. Glob/Grep default their search base
|
||||
/// to this path when callers omit an explicit `path` parameter.
|
||||
pub fn pwd(&self) -> &Path {
|
||||
&self.inner.pwd
|
||||
}
|
||||
|
||||
// =========================================================================
|
||||
// Read — unrestricted
|
||||
// Read — scope-checked against readability
|
||||
// =========================================================================
|
||||
|
||||
/// Read the full contents of `path` as raw bytes.
|
||||
///
|
||||
/// Follows symlinks. Rejects directories, relative paths, and missing
|
||||
/// files. No scope check.
|
||||
/// Follows symlinks. Rejects directories, relative paths, paths not
|
||||
/// readable by the scope, and missing files.
|
||||
pub fn read_bytes(&self, path: &Path) -> Result<Vec<u8>, ToolsError> {
|
||||
if !path.is_absolute() {
|
||||
return Err(ToolsError::RelativePath(path.to_path_buf()));
|
||||
}
|
||||
if !self.inner.scope.is_readable(path) {
|
||||
return Err(ToolsError::OutOfScope(path.to_path_buf()));
|
||||
}
|
||||
let meta = std::fs::metadata(path).map_err(|e| match e.kind() {
|
||||
std::io::ErrorKind::NotFound => ToolsError::NotFound(path.to_path_buf()),
|
||||
_ => ToolsError::io(path, e),
|
||||
@@ -75,9 +86,10 @@ impl ScopedFs {
|
||||
|
||||
/// Atomically write `content` to `path`, creating or overwriting it.
|
||||
///
|
||||
/// - `path` must be absolute and inside the scope (delegates to
|
||||
/// [`Scope::contains`]).
|
||||
/// - Missing parent directories inside the scope are created.
|
||||
/// - `path` must be absolute and writable under the scope.
|
||||
/// - Paths that are readable but not writable return [`ToolsError::ReadOnly`].
|
||||
/// - Paths outside the scope entirely return [`ToolsError::OutOfScope`].
|
||||
/// - Missing parent directories are created.
|
||||
/// - The actual write uses a sibling tempfile + `persist`, so the
|
||||
/// target file transitions atomically between states.
|
||||
///
|
||||
@@ -88,8 +100,12 @@ impl ScopedFs {
|
||||
if !path.is_absolute() {
|
||||
return Err(ToolsError::RelativePath(path.to_path_buf()));
|
||||
}
|
||||
if !self.inner.scope.contains(path) {
|
||||
return Err(ToolsError::OutOfScope(path.to_path_buf()));
|
||||
if !self.inner.scope.is_writable(path) {
|
||||
return Err(if self.inner.scope.is_readable(path) {
|
||||
ToolsError::ReadOnly(path.to_path_buf())
|
||||
} else {
|
||||
ToolsError::OutOfScope(path.to_path_buf())
|
||||
});
|
||||
}
|
||||
|
||||
// Reject existing directory targets.
|
||||
@@ -138,11 +154,15 @@ impl ScopedFs {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
use manifest::{Permission, ScopeConfig, ScopeRule};
|
||||
use std::fs;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn make_fs(dir: &TempDir) -> ScopedFs {
|
||||
ScopedFs::new(Scope::new(dir.path()).unwrap())
|
||||
ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
)
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -183,15 +203,15 @@ mod tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn read_bytes_allows_paths_outside_scope() {
|
||||
// Reads are unrestricted — scope only gates writes.
|
||||
fn read_bytes_rejects_paths_outside_scope() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let outside = TempDir::new().unwrap();
|
||||
let outside_file = outside.path().join("x.txt");
|
||||
fs::write(&outside_file, b"hi").unwrap();
|
||||
|
||||
let scoped = make_fs(&dir);
|
||||
assert_eq!(scoped.read_bytes(&outside_file).unwrap(), b"hi");
|
||||
let err = scoped.read_bytes(&outside_file).unwrap_err();
|
||||
assert!(matches!(err, ToolsError::OutOfScope(_)));
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
@@ -229,6 +249,32 @@ mod tests {
|
||||
assert!(matches!(err, ToolsError::OutOfScope(_)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_rejects_readonly_path() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let sub = dir.path().join("sub");
|
||||
fs::create_dir(&sub).unwrap();
|
||||
let cfg = ScopeConfig {
|
||||
allow: vec![ScopeRule {
|
||||
target: dir.path().to_path_buf(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
}],
|
||||
deny: vec![ScopeRule {
|
||||
target: sub.clone(),
|
||||
permission: Permission::Write,
|
||||
recursive: true,
|
||||
}],
|
||||
};
|
||||
let scope = Scope::from_config(&cfg, dir.path()).unwrap();
|
||||
let scoped = ScopedFs::new(scope, dir.path().to_path_buf());
|
||||
let err = scoped.write(&sub.join("locked.txt"), b"x").unwrap_err();
|
||||
assert!(
|
||||
matches!(err, ToolsError::ReadOnly(_)),
|
||||
"expected ReadOnly, got {err:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn write_rejects_relative_path() {
|
||||
let dir = TempDir::new().unwrap();
|
||||
|
||||
@@ -25,10 +25,11 @@
|
||||
//! the Pod wires them together when registering builtin tools.
|
||||
//!
|
||||
//! ```no_run
|
||||
//! # use std::path::PathBuf;
|
||||
//! # use manifest::Scope;
|
||||
//! # use tools::{ScopedFs, Tracker, builtin_tools};
|
||||
//! let scope = Scope::new("/workspace").unwrap();
|
||||
//! let fs = ScopedFs::new(scope); // pod lifetime
|
||||
//! let scope = Scope::writable("/workspace").unwrap();
|
||||
//! let fs = ScopedFs::new(scope, PathBuf::from("/workspace")); // pod lifetime
|
||||
//! let tracker = Tracker::new(); // session lifetime
|
||||
//! let defs = builtin_tools(fs, tracker);
|
||||
//! ```
|
||||
|
||||
@@ -99,7 +99,10 @@ mod tests {
|
||||
|
||||
fn setup() -> (TempDir, ScopedFs, Tracker) {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
(dir, fs, Tracker::new())
|
||||
}
|
||||
|
||||
|
||||
@@ -29,7 +29,10 @@ impl Registry {
|
||||
|
||||
fn setup() -> (TempDir, Registry) {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
let tracker = Tracker::new();
|
||||
(dir, Registry::new(builtin_tools(fs, tracker)))
|
||||
}
|
||||
@@ -76,14 +79,19 @@ async fn symlink_to_outside_scope_is_rejected_for_write() {
|
||||
let link = dir.path().join("linked.txt");
|
||||
symlink(&outside_target, &link).unwrap();
|
||||
|
||||
// Read tool must work against the symlink (read is unrestricted).
|
||||
// Read through the symlink must be rejected because the resolved
|
||||
// target sits outside the scope.
|
||||
let read = reg.get("Read");
|
||||
read.execute(&json!({ "file_path": link.to_str().unwrap() }).to_string())
|
||||
let read_err = read
|
||||
.execute(&json!({ "file_path": link.to_str().unwrap() }).to_string())
|
||||
.await
|
||||
.unwrap();
|
||||
.unwrap_err();
|
||||
assert!(
|
||||
format!("{read_err}").contains("outside allowed scope"),
|
||||
"symlink read escape not rejected: {read_err}"
|
||||
);
|
||||
|
||||
// Write through the symlink must be rejected because canonicalization
|
||||
// resolves it to outside the scope.
|
||||
// Write through the symlink must be rejected for the same reason.
|
||||
let write = reg.get("Write");
|
||||
let err = write
|
||||
.execute(
|
||||
|
||||
@@ -38,7 +38,10 @@ impl Registry {
|
||||
|
||||
fn setup() -> (TempDir, Registry) {
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
let tracker = Tracker::new();
|
||||
let reg = Registry::new(builtin_tools(fs, tracker));
|
||||
(dir, reg)
|
||||
@@ -275,7 +278,10 @@ fn tool_names_match_reference_spec() {
|
||||
async fn tracker_recent_files_tracks_read_write_edit() {
|
||||
// Build a fresh registry that shares a tracker we can query afterwards.
|
||||
let dir = TempDir::new().unwrap();
|
||||
let fs = ScopedFs::new(Scope::new(dir.path()).unwrap());
|
||||
let fs = ScopedFs::new(
|
||||
Scope::writable(dir.path()).unwrap(),
|
||||
dir.path().to_path_buf(),
|
||||
);
|
||||
let tracker = Tracker::new();
|
||||
let reg = Registry::new(builtin_tools(fs, tracker.clone()));
|
||||
|
||||
|
||||
Reference in New Issue
Block a user