fix(id): confine REPL match-derived download paths (#2307)

pkgs/id/src/commands/find.rs

@@ -78,8 +78,8 @@ use iroh_base::EndpointId;
 
 use crate::{
     CLIENT_KEY_FILE, FindMatch, META_ALPN, MetaRequest, MetaResponse, TaggedMatch, cmd_get_one,
-    cmd_get_one_remote, load_or_create_keypair, match_kind, open_store, print_match_cli,
-    print_matches_cli,
+    cmd_get_one_remote, helpers::download_output_path, load_or_create_keypair, match_kind,
+    open_store, print_match_cli, print_matches_cli,
 };
 
 /// Options for filtering and limiting search results.
@@ -149,39 +149,6 @@ impl SearchOptions {
     }
 }
 
-/// Build the local write path for a downloaded find/search match.
-///
-/// A match name is untrusted input: in remote mode both the name and the
-/// blob content come from the queried node, so the name must never be used
-/// as a filesystem path verbatim (`../` and absolute names would overwrite
-/// arbitrary local files — issue #1614). The write path is derived from the
-/// name's final path component only (`Path::file_name` ruling), optionally
-/// under `dir`. A name with no final component (`..`, `/`, a trailing
-/// separator, empty) is refused and nothing is written. Ordinary
-/// single-segment names produce byte-identical paths to the previous
-/// verbatim behaviour.
-fn download_output_path(dir: Option<&str>, name: &str) -> Result<String> {
-    if name.is_empty() || name.ends_with('/') {
-        bail!(
-            "refusing download of match with unsafe name {:?}: no final path component",
-            name
-        );
-    }
-    let Some(file_name) = std::path::Path::new(name)
-        .file_name()
-        .and_then(|c| c.to_str())
-    else {
-        bail!(
-            "refusing download of match with unsafe name {:?}: no final path component",
-            name
-        );
-    };
-    Ok(match dir {
-        Some(dir) => format!("{}/{}", dir, file_name),
-        None => file_name.to_owned(),
-    })
-}
-
 /// Plan all bulk-download destinations before creating directories or writing files.
 ///
 /// Match names are deduplicated the same way as the output loops. Distinct matches
@@ -1083,6 +1050,19 @@ mod tests {
         assert_eq!(download_output_path(None, "/tmp/x").unwrap(), "x");
     }
 
+    #[test]
+    #[test]
+    fn test_download_output_path_dash_name_is_not_the_stdout_sentinel() {
+        // A match-derived "-" must name a file, not the stdout sentinel;
+        // only an explicit user-supplied "-" means stdout.
+        assert_eq!(download_output_path(None, "-").unwrap(), "./-");
+        assert_eq!(download_output_path(None, "nested/-").unwrap(), "./-");
+        assert_eq!(
+            download_output_path(Some("out"), "nested/-").unwrap(),
+            "out/-"
+        );
+    }
+
     #[test]
     fn test_download_output_path_refuses_names_without_final_component() {
         for name in ["..", "/", "", "foo/", "../", "."] {

pkgs/id/src/helpers.rs

@@ -34,6 +34,7 @@
 
 use crate::commands::text::{plain_field, tsv_row};
 use crate::protocol::{FindMatch, MatchKind, TaggedMatch};
+use anyhow::{Result, bail};
 
 /// Parses a put specification into path and optional name.
 ///
@@ -292,6 +293,48 @@ fn format_match_repl(query: &str, m: &FindMatch, format: &str) -> String {
 /// backwards compatibility.
 pub use crate::match_kind;
 
+/// Build the local write path for a downloaded find/search match.
+///
+/// A match name is untrusted input: in remote mode both the name and the
+/// blob content come from the queried node, so the name must never be used
+/// as a filesystem path verbatim (`../` and absolute names would overwrite
+/// arbitrary local files — issues #1614, #2307). The write path is derived
+/// from the name's final path component only (`Path::file_name` ruling),
+/// optionally under `dir`. A name with no final component (`..`, `/`, a
+/// trailing separator, empty) is refused and nothing is written. Ordinary
+/// single-segment names produce byte-identical paths to the previous
+/// verbatim behaviour.
+///
+/// This is the single shared implementation used by both the CLI
+/// (`commands/find.rs`) and the REPL (`repl/runner.rs`); do not copy this
+/// rule elsewhere.
+pub fn download_output_path(dir: Option<&str>, name: &str) -> Result<String> {
+    if name.is_empty() || name.ends_with('/') {
+        bail!(
+            "refusing download of match with unsafe name {:?}: no final path component",
+            name
+        );
+    }
+    let Some(file_name) = std::path::Path::new(name)
+        .file_name()
+        .and_then(|c| c.to_str())
+    else {
+        bail!(
+            "refusing download of match with unsafe name {:?}: no final path component",
+            name
+        );
+    };
+    Ok(match dir {
+        Some(dir) => format!("{}/{}", dir, file_name),
+        // A derived bare "-" would collide with the stdout sentinel in
+        // export_blob ("-" means stdout there); write the file as "./-"
+        // instead. An explicit user-supplied "-" is untouched and still
+        // means stdout (Codex review on PR #4472).
+        None if file_name == "-" => "./-".to_owned(),
+        None => file_name.to_owned(),
+    })
+}
+
 /// Parse a duration such as `45s`, `90m`, `12h` or `3d` into seconds.
 ///
 /// # Errors

pkgs/id/src/lib.rs

@@ -286,7 +286,8 @@ pub use discovery::{
     STALE_CHECK_INTERVAL, STALE_THRESHOLD, defaults, parse_defaults, resolve_config,
 };
 pub use helpers::{
-    parse_get_spec, parse_put_spec, print_match_cli, print_match_repl, print_matches_cli,
+    download_output_path, parse_get_spec, parse_put_spec, print_match_cli, print_match_repl,
+    print_matches_cli,
 };
 pub use protocol::{FindMatch, MatchKind, MetaProtocol, MetaRequest, MetaResponse, TaggedMatch};
 pub use repl::run_repl;