Restrict git transports for every child skillz runs and refuse malformed owner/repo sources

crates/skillz-harness/src/clone.rs

@@ -17,6 +17,24 @@ fn check_source(source: &str) -> anyhow::Result<()> {
     Ok(())
 }
 
+/// A GitHub shorthand is exactly `<owner>/<repo>`: anything else (`../x`, `~/x`, `a/b/c`) is
+/// not a repository name and must not reach git.
+fn check_repo_name(source: &str) -> anyhow::Result<()> {
+    let is_url = source.contains("://") || source.starts_with("git@");
+    let is_name = |part: &str| {
+        !part.is_empty() && part != "." && part != ".." && !part.starts_with(['-', '~'])
+    };
+    let is_owner_and_repo = source
+        .split_once('/')
+        .is_some_and(|(owner, repo)| is_name(owner) && is_name(repo) && !repo.contains('/'));
+    anyhow::ensure!(
+        is_url || (is_owner_and_repo && !source.contains('\\')),
+        "invalid repository name: {}",
+        redact_text(source)
+    );
+    Ok(())
+}
+
 /// Resolve a local directory or inspect a temporary clone. The callback receives the stable
 /// install source as well as the fetched tree, so temporary paths are never persisted.
 pub fn with_source<T>(
@@ -40,6 +58,7 @@ pub fn with_source<T>(
         Err(e) if e.kind() == std::io::ErrorKind::NotFound => {}
         Err(e) => return Err(e).with_context(|| format!("reading source {}", redact_text(source))),
     }
+    check_repo_name(source)?;
     let temporary = tempfile::Builder::new()
         .prefix("skillz-install-")
         .tempdir()?;
@@ -147,6 +166,77 @@ mod source_tests {
         assert!(runner.calls().is_empty());
     }
 
+    #[test]
+    fn anything_but_owner_slash_repo_is_refused_and_runs_nothing() {
+        let fx = project_with(|_| {});
+        let runner = FakeRunner::new(|_| {});
+        let sources = [
+            "../x",
+            "a/../b",
+            "./a/b",
+            "~/x",
+            "~o/r",
+            "o/~r",
+            "o/-r",
+            "o/r/extra",
+            "o",
+            "o//r",
+            "/o/r",
+            "o\\r",
+            "o/r\\x",
+            "o\\x/r",
+            "o/",
+            "/r",
+            "./o",
+        ];
+        for source in sources {
+            let res = with_source(source, &fx.env.project, &runner, |_, _| Ok(()));
+            assert!(res.is_err(), "{source}");
+        }
+        assert!(runner.calls().is_empty());
+    }
+
+    #[test]
+    fn owner_slash_repo_and_urls_are_cloned_unchanged() {
+        let fx = project_with(|_| {});
+        let runner = FakeRunner::new(|_| {});
+        let sources = ["o/r", "https://x.dev/o/r.git", "git@x.dev:o/r.git"];
+        for source in sources {
+            with_source(source, &fx.env.project, &runner, |_, _| Ok(())).unwrap();
+        }
+        let cloned: Vec<String> = runner
+            .calls()
+            .iter()
+            .map(|(_, args, _)| args[4].clone())
+            .collect();
+        assert_eq!(
+            cloned,
+            [
+                "https://github.com/o/r.git",
+                "https://x.dev/o/r.git",
+                "git@x.dev:o/r.git"
+            ]
+        );
+    }
+
+    #[test]
+    fn a_local_directory_at_a_deep_relative_path_is_used_in_place() {
+        let directory = tempfile::tempdir().unwrap();
+        let deep = directory.path().join("a/b/c");
+        std::fs::create_dir_all(&deep).unwrap();
+        let runner = FakeRunner::new(|_| {});
+        let mut used = None;
+
+        with_source("a/b/c", directory.path(), &runner, |tree, _| {
+            used = Some(tree.to_path_buf());
+            Ok(())
+        })
+        .unwrap();
+
+        assert_eq!(used, Some(deep.canonicalize().unwrap()));
+        assert!(runner.calls().is_empty());
+    }
+
     #[test]
     fn clone_failure_is_err_and_skips_install() {
         let fx = project_with(|_| {});

crates/skillz-harness/src/runner.rs

@@ -55,6 +55,10 @@ impl CommandRunner for NoCommands {
     }
 }
 
+/// Transports git may use in every child skillz runs except a launched agent; blocks `ext::` and
+/// other remote helpers whatever the user's `protocol.*.allow` says.
+pub const GIT_ALLOW_PROTOCOL: (&str, &str) = ("GIT_ALLOW_PROTOCOL", "http:https:ssh:git:file");
+
 /// Spawns real processes in `cwd` with stdin and stdout inherited (npx and claude print
 /// prompts and errors to stdout) and stderr captured for the error message. `envs` are set on
 /// every child, so a config dir skillz was pointed at (`CLAUDE_CONFIG_DIR`, `CODEX_HOME`)
@@ -73,6 +77,17 @@ impl SystemRunner {
             .current_dir(cwd);
         command
     }
+
+    fn command_restricting_git_transports(
+        &self,
+        program: &str,
+        args: &[String],
+        cwd: &Path,
+    ) -> Command {
+        let mut command = self.command(program, args, cwd);
+        command.env(GIT_ALLOW_PROTOCOL.0, GIT_ALLOW_PROTOCOL.1);
+        command
+    }
 }
 
 impl CommandRunner for SystemRunner {
@@ -95,7 +110,7 @@ impl CommandRunner for SystemRunner {
 
     fn run(&self, program: &str, args: &[String], cwd: &Path) -> anyhow::Result<Output> {
         let out = self
-            .command(program, args, cwd)
+            .command_restricting_git_transports(program, args, cwd)
             .stdin(Stdio::inherit())
             .stdout(Stdio::inherit())
             .output()
@@ -108,7 +123,7 @@ impl CommandRunner for SystemRunner {
 
     fn run_quiet(&self, program: &str, args: &[String], cwd: &Path) -> anyhow::Result<Output> {
         let out = self
-            .command(program, args, cwd)
+            .command_restricting_git_transports(program, args, cwd)
             .stdin(Stdio::null())
             .output()
             .with_context(|| format!("failed to run `{program}` in {}", cwd.display()))?;
@@ -126,7 +141,7 @@ impl CommandRunner for SystemRunner {
 
     fn capture(&self, program: &str, args: &[String], cwd: &Path) -> anyhow::Result<String> {
         let out = self
-            .command(program, args, cwd)
+            .command_restricting_git_transports(program, args, cwd)
             .stdin(Stdio::null())
             .output()
             .with_context(|| format!("failed to run `{program}` in {}", cwd.display()))?;
@@ -249,6 +264,71 @@ mod tests {
         let res = SystemRunner::default().run("skillz-definitely-not-a-program", &[], dir.path());
         assert!(res.is_err());
     }
+
+    fn plain_and_redirected_runners() -> Vec<Box<dyn CommandRunner>> {
+        let redirected = SystemRunner::default()
+            .with_envs(vec![("SKILLZ_KEEP", "/keep".into())])
+            .unwrap();
+        vec![Box::new(SystemRunner::default()), redirected]
+    }
+
+    fn sh_args(script: String) -> [String; 2] {
+        ["-c".into(), script]
+    }
+
+    #[test]
+    fn captured_children_may_only_use_the_allowed_git_transports() {
+        let dir = tempfile::tempdir().unwrap();
+        let args = sh_args("printf %s \"$GIT_ALLOW_PROTOCOL\"".into());
+
+        for runner in plain_and_redirected_runners() {
+            let seen = runner.capture("sh", &args, dir.path()).unwrap();
+            assert_eq!(seen, "http:https:ssh:git:file");
+        }
+    }
+
+    #[test]
+    fn run_and_run_quiet_children_may_only_use_the_allowed_git_transports() {
+        let dir = tempfile::tempdir().unwrap();
+        let args = sh_args(format!(
+            "test \"$GIT_ALLOW_PROTOCOL\" = {}",
+            GIT_ALLOW_PROTOCOL.1
+        ));
+
+        for runner in plain_and_redirected_runners() {
+            assert!(runner.run("sh", &args, dir.path()).unwrap().success);
+            assert!(runner.run_quiet("sh", &args, dir.path()).unwrap().success);
+        }
+    }
+
+    #[test]
+    fn a_launched_agent_keeps_its_own_git_allow_protocol() {
+        let dir = tempfile::tempdir().unwrap();
+        let args = sh_args("test \"$GIT_ALLOW_PROTOCOL\" = sentinel".into());
+        let runner = SystemRunner::default()
+            .with_envs(vec![(GIT_ALLOW_PROTOCOL.0, "sentinel".into())])
+            .unwrap();
+
+        assert_eq!(runner.launch("sh", &args, dir.path()).unwrap(), 0);
+    }
+
+    #[test]
+    fn the_transport_restriction_overrides_the_users_git_config() {
+        let dir = tempfile::tempdir().unwrap();
+        let args = [
+            "-c",
+            "protocol.allow=always",
+            "ls-remote",
+            "ext::sh -c touch% marker",
+        ]
+        .map(String::from);
+
+        let out = SystemRunner::default().run_quiet("git", &args, dir.path());
+
+        assert!(!out.unwrap().success);
+        assert!(!dir.path().join("marker").exists());
+    }
+
     #[test]
     fn child_redirects_preserve_other_configured_environment_variables() {
         let directory = tempfile::tempdir().unwrap();