From beffec715a1bf747b5485b7a2fa4607cbf562de5 Mon Sep 17 00:00:00 2001
From: Drew Newberry <anewberry@nvidia.com>
Date: Wed, 25 Feb 2026 07:31:39 -0800
Subject: [PATCH] fix(providers): prevent home path escape in expand_home (!54)

Closes #61

## Summary
- harden provider discovery home expansion to reject path-escape components like dot-dot
- reuse the same safe expansion logic in MockDiscoveryContext
- add regression tests for escaped paths and valid home expansions
- document the new config-path expansion safety rule

## Test Plan
- mise run pre-commit
---
 architecture/sandbox-providers.md             |  1 +
 crates/navigator-providers/src/context.rs     | 41 ++++++++++++++++---
 crates/navigator-providers/src/discovery.rs   | 26 ++++++++++++
 .../navigator-providers/src/test_helpers.rs   |  4 +-
 4 files changed, 66 insertions(+), 6 deletions(-)

diff --git a/architecture/sandbox-providers.md b/architecture/sandbox-providers.md
index b0b892beb..235df57c8 100644
--- a/architecture/sandbox-providers.md
+++ b/architecture/sandbox-providers.md
@@ -123,6 +123,7 @@ Discovery behavior is split into three layers:
 
 2. **Config file scan**: for each path in `spec.config_paths`:
    - expands `~/` via the context,
+   - rejects `~/` expansions that contain path-escape components (for example `..`),
    - checks file existence,
    - **only parses `.json` files** (`.yml`/`.yaml` are checked for existence but not read),
    - recursively collects JSON fields whose keys match credential patterns
diff --git a/crates/navigator-providers/src/context.rs b/crates/navigator-providers/src/context.rs
index 27c7e8160..f566aba40 100644
--- a/crates/navigator-providers/src/context.rs
+++ b/crates/navigator-providers/src/context.rs
@@ -1,4 +1,4 @@
-use std::path::{Path, PathBuf};
+use std::path::{Component, Path, PathBuf};
 
 pub trait DiscoveryContext {
     fn env_var(&self, key: &str) -> Option<String>;
@@ -16,10 +16,8 @@ impl DiscoveryContext for RealDiscoveryContext {
 
     fn expand_home(&self, path: &str) -> Option<PathBuf> {
         if let Some(stripped) = path.strip_prefix("~/") {
-            return std::env::var("HOME")
-                .ok()
-                .map(PathBuf::from)
-                .map(|home| home.join(stripped));
+            let home = std::env::var("HOME").ok()?;
+            return expand_home_relative(Path::new(&home), stripped);
         }
         Some(PathBuf::from(path))
     }
@@ -32,3 +30,36 @@ impl DiscoveryContext for RealDiscoveryContext {
         std::fs::read_to_string(path).ok()
     }
 }
+
+pub(crate) fn expand_home_relative(home: &Path, stripped: &str) -> Option<PathBuf> {
+    let relative = Path::new(stripped);
+    for component in relative.components() {
+        match component {
+            Component::Normal(_) | Component::CurDir => {}
+            Component::ParentDir | Component::RootDir | Component::Prefix(_) => return None,
+        }
+    }
+    Some(home.join(relative))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::expand_home_relative;
+    use std::path::{Path, PathBuf};
+
+    #[test]
+    fn rejects_parent_dir_escape_when_expanding_home() {
+        assert_eq!(
+            expand_home_relative(Path::new("/home/tester"), "../etc/passwd"),
+            None
+        );
+    }
+
+    #[test]
+    fn expands_safe_relative_home_path() {
+        assert_eq!(
+            expand_home_relative(Path::new("/home/tester"), ".config/codex/config.json"),
+            Some(PathBuf::from("/home/tester/.config/codex/config.json"))
+        );
+    }
+}
diff --git a/crates/navigator-providers/src/discovery.rs b/crates/navigator-providers/src/discovery.rs
index d64f52df5..d380bee84 100644
--- a/crates/navigator-providers/src/discovery.rs
+++ b/crates/navigator-providers/src/discovery.rs
@@ -92,3 +92,29 @@ fn collect_credential_fields(
         _ => {}
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::discover_with_spec;
+    use crate::ProviderDiscoverySpec;
+    use crate::test_helpers::{MockDiscoveryContext, home_join};
+
+    #[test]
+    fn ignores_home_escaping_config_paths() {
+        let spec = ProviderDiscoverySpec {
+            id: "test",
+            credential_env_vars: &[],
+            config_paths: &["~/../secrets.json"],
+        };
+
+        let ctx = MockDiscoveryContext::new()
+            .with_home("/mock/home")
+            .with_file(
+                home_join("/mock/home", "../secrets.json"),
+                r#"{"token":"secret"}"#,
+            );
+
+        let discovered = discover_with_spec(&spec, &ctx).expect("discovery");
+        assert!(discovered.is_none());
+    }
+}
diff --git a/crates/navigator-providers/src/test_helpers.rs b/crates/navigator-providers/src/test_helpers.rs
index b1526ff47..58445e5fa 100644
--- a/crates/navigator-providers/src/test_helpers.rs
+++ b/crates/navigator-providers/src/test_helpers.rs
@@ -1,4 +1,5 @@
 use crate::DiscoveryContext;
+use crate::context::expand_home_relative;
 use std::collections::{HashMap, HashSet};
 use std::path::{Path, PathBuf};
 
@@ -45,7 +46,8 @@ impl DiscoveryContext for MockDiscoveryContext {
 
     fn expand_home(&self, path: &str) -> Option<PathBuf> {
         if let Some(stripped) = path.strip_prefix("~/") {
-            return self.home.as_ref().map(|home| home.join(stripped));
+            let home = self.home.as_deref()?;
+            return expand_home_relative(home, stripped);
         }
         Some(PathBuf::from(path))
     }