chore(tools): Suppress noisy/sensitive paths from fs tool results

.config/jp/tools/src/fs.rs

@@ -1,3 +1,5 @@
+use utils::suppress_matcher;
+
 use crate::{
     Context, Tool, to_xml,
     util::{OneOrMany, ToolResult},
@@ -21,12 +23,28 @@ use move_file::fs_move_file;
 use read_file::fs_read_file;
 
 pub async fn run(ctx: Context, t: Tool) -> ToolResult {
+    // Paths these tools may read but never return. Honored by the tools that exist
+    // to hand file contents or paths back; the write tools return confirmations
+    // rather than content, and what they may touch is the access policy's
+    // question.
+    //
+    // Parsed strictly rather than through `option_or`: a disclosure control that
+    // falls back to "suppress nothing" when its configuration is malformed hands
+    // over the very paths it was told to hold back, and does it silently.
+    let patterns: Vec<String> = match t.options.get("suppress") {
+        None => vec![],
+        Some(value) => serde_json::from_value(value.clone())
+            .map_err(|error| format!("Invalid `suppress` option for tool '{}': {error}", t.name))?,
+    };
+    let suppress = suppress_matcher(&ctx.root, &patterns)?;
+
     match t.name.trim_start_matches("fs_") {
         "list_files" => fs_list_files(
             &ctx.root,
             ctx.access.as_ref(),
             t.opt("prefixes")?,
             t.opt("extensions")?,
+            &suppress,
         )
         .await
         .and_then(to_xml)
@@ -35,6 +53,7 @@ pub async fn run(ctx: Context, t: Tool) -> ToolResult {
         "read_file" => {
             fs_read_file(
                 &ctx,
+                &suppress,
                 t.req("path")?,
                 t.opt("start_line")?,
                 t.opt("end_line")?,
@@ -49,6 +68,7 @@ pub async fn run(ctx: Context, t: Tool) -> ToolResult {
             t.opt("context")?,
             t.opt("paths")?,
             None,
+            &suppress,
         )
         .await
         .map(Into::into),
@@ -65,6 +85,7 @@ pub async fn run(ctx: Context, t: Tool) -> ToolResult {
             t.opt("context")?,
             Some(vec!["docs".to_owned()].into()),
             Some(vec!["md".to_owned()].into()),
+            &suppress,
         )
         .await
         .map(Into::into),
@@ -103,3 +124,7 @@ pub async fn run(ctx: Context, t: Tool) -> ToolResult {
         _ => Err(format!("Unknown tool '{}'", t.name).into()),
     }
 }
+
+#[cfg(test)]
+#[path = "fs_tests.rs"]
+mod tests;

.config/jp/tools/src/fs/delete_file_tests.rs

@@ -1,9 +1,20 @@
 use camino_tempfile::tempdir;
-use jp_tool::Outcome;
+use jp_tool::{AccessPolicy, FsRule, Outcome};
 use serde_json::Map;
 
 use super::*;
 
+/// A policy granting the whole workspace except `denied`.
+fn workspace_except(denied: &str) -> AccessPolicy {
+    AccessPolicy {
+        fs: vec![
+            FsRule::new("").with_read(true).with_write(true),
+            FsRule::new(denied).with_read(false).with_write(false),
+        ],
+        ..AccessPolicy::default()
+    }
+}
+
 fn no_answers() -> Map<String, serde_json::Value> {
     Map::new()
 }
@@ -178,6 +189,35 @@ async fn deleting_dangling_symlink_succeeds() {
     assert!(std::fs::symlink_metadata(root.join("broken")).is_err());
 }
 
+#[tokio::test]
+async fn refuses_to_delete_a_path_a_deny_rule_closes() {
+    let dir = tempdir().unwrap();
+    let root = dir.path();
+    std::fs::create_dir(root.join(".git")).unwrap();
+    std::fs::write(root.join(".git/HEAD"), "ref: refs/heads/main").unwrap();
+
+    let result = fs_delete_file(
+        root,
+        Some(&workspace_except(".git")),
+        &no_answers(),
+        ".git/HEAD".to_owned(),
+    )
+    .await
+    .unwrap();
+
+    let Outcome::Error { message, .. } = result else {
+        panic!("expected a refusal, got: {result:?}");
+    };
+    // Normalized: a resolved path carries native separators, so the message reads
+    // `.git\HEAD` on Windows.
+    assert_eq!(
+        message.replace('\\', "/"),
+        "Access denied: cannot delete '.git/HEAD'. Paths granting delete: [.]. If required, ask \
+         the user for explicit access."
+    );
+    assert!(root.join(".git/HEAD").exists(), "file was deleted anyway");
+}
+
 #[tokio::test]
 async fn deleting_missing_path_errors() {
     let dir = tempdir().unwrap();

.config/jp/tools/src/fs/grep_files.rs

@@ -2,6 +2,7 @@ use camino::{Utf8Path, Utf8PathBuf};
 use grep_printer::StandardBuilder;
 use grep_regex::RegexMatcher;
 use grep_searcher::SearcherBuilder;
+use ignore::gitignore::Gitignore;
 use jp_tool::AccessPolicy;
 
 use super::fs_list_files;
@@ -14,6 +15,7 @@ pub(crate) async fn fs_grep_files(
     context: Option<usize>,
     paths: Option<OneOrMany<String>>,
     extensions: Option<OneOrMany<String>>,
+    suppress: &Gitignore,
 ) -> std::result::Result<String, Error> {
     // Resolve the file set via `fs_list_files`, which always walks from the
     // workspace root. Anchoring the walk there is what makes the root
@@ -27,8 +29,10 @@ pub(crate) async fn fs_grep_files(
     // nothing, and `""`/`.` mean the workspace root. Escape attempts surface
     // as a hard error from the shared path validation. The access policy is
     // threaded through so an approved external mount can be searched.
-    let files: Vec<Utf8PathBuf> = fs_list_files(root, access, paths.clone(), extensions.clone())
-        .await?
+    let listing = fs_list_files(root, access, paths.clone(), extensions.clone(), suppress).await?;
+
+    let notes = listing.notes();
+    let files: Vec<Utf8PathBuf> = listing
         .into_files()
         .into_iter()
         .map(Utf8PathBuf::from)
@@ -63,28 +67,56 @@ pub(crate) async fn fs_grep_files(
     let matches = String::from_utf8(printer.into_inner().into_inner())?;
 
     let lines = matches.lines().count();
-    if matches.is_empty() {
-        Ok("No matches found. Broaden your search to see more.".to_owned())
+    let body = if matches.is_empty() {
+        // A search whose requested paths were skipped finds nothing for a
+        // completely different reason than a search that came up empty, and the
+        // notes below say which happened.
+        if notes.is_empty() {
+            "No matches found. Broaden your search to see more.".to_owned()
+        } else {
+            "No matches found in the paths that were searched.".to_owned()
+        }
     } else if lines > 200 && context.is_some() {
-        Box::pin(fs_grep_files(
-            root, access, pattern, None, paths, extensions,
+        // The inner call reproduces the notes, so they are not appended twice.
+        return Box::pin(fs_grep_files(
+            root, access, pattern, None, paths, extensions, suppress,
         ))
         .await
         .map(|v| {
             format!(
                 "{v}\n[Hidden contextual lines due to excessive number of lines returned. Narrow \
                  down your search to see more.]"
             )
-        })
+        });
     } else if lines > 100 {
-        Ok(indoc::formatdoc! {"
+        indoc::formatdoc! {"
             {}
 
             [Showing 100/{lines} lines of matches... Narrow down your search to see more.]
-        ", matches.lines().take(100).collect::<Vec<_>>().join("\n"),})
+        ", matches.lines().take(100).collect::<Vec<_>>().join("\n"),}
     } else {
-        Ok(matches)
+        matches
+    };
+
+    Ok(append_notes(body, &notes))
+}
+
+/// Append the listing's skip notes to a search result.
+///
+/// Without them, a search whose requested paths were skipped reports the same
+/// empty result as a search that genuinely found nothing.
+fn append_notes(body: String, notes: &[String]) -> String {
+    if notes.is_empty() {
+        return body;
     }
+
+    let notes = notes
+        .iter()
+        .map(|note| format!("Note: {note}"))
+        .collect::<Vec<_>>()
+        .join("\n");
+
+    format!("{body}\n\n{notes}")
 }
 
 #[cfg(test)]