a73x

5149aa12

Honour core.sshCommand and GIT_SSH_COMMAND in the release CLI

a73x   2026-08-08 18:04

Commit message
Honour core.sshCommand and GIT_SSH_COMMAND in the release CLI
src/release.rs
Old New
@@ -109,12 +109,51 @@ fn validate_remote_path(path: &str) -> Result<(), Error> {
109 Ok(()) 109 Ok(())
110 } 110 }
111 111
112 /// Build the ssh invocation for a remote, honoring GIT_COLLAB_SSH_COMMAND 112 /// The candidate ssh command strings, in the order they were read from their
113 /// (like git's GIT_SSH_COMMAND: extra words become leading arguments). Unlike 113 /// respective sources. Named fields (rather than positional `Option<String>`
114 /// git's GIT_SSH_COMMAND, which is run through `sh -c` and supports shell 114 /// args) so a call site can't silently pass them in the wrong order.
115 /// quoting, this is split on whitespace only — no quoting support. 115 struct SshCommandSources {
116 fn ssh_command(remote: &SshRemote) -> Command { 116 collab_env: Option<String>,
117 let base = std::env::var("GIT_COLLAB_SSH_COMMAND").unwrap_or_else(|_| "ssh".to_string()); 117 git_env: Option<String>,
118 config: Option<String>,
119 }
120
121 /// Pick the ssh command string to use, first match wins: `collab_env`
122 /// (GIT_COLLAB_SSH_COMMAND), then `git_env` (GIT_SSH_COMMAND), then `config`
123 /// (git's `core.sshCommand`), then a bare `"ssh"`. Env overrides config for
124 /// the last two — matching git itself, whose docs say `core.sshCommand` "is
125 /// overridden when the GIT_SSH_COMMAND environment variable is set". A
126 /// candidate that is empty or whitespace-only is treated as absent rather
127 /// than winning with a blank program name.
128 fn resolve_ssh_command(sources: SshCommandSources) -> String {
129 [sources.collab_env, sources.git_env, sources.config]
130 .into_iter()
131 .flatten()
132 .find(|value| !value.trim().is_empty())
133 .unwrap_or_else(|| "ssh".to_string())
134 }
135
136 /// Build the ssh invocation for a remote. The command is resolved in order:
137 /// `GIT_COLLAB_SSH_COMMAND` (env, this tool's own escape hatch) > `GIT_SSH_COMMAND`
138 /// (env) > `core.sshCommand` (git config, including inherited global/system
139 /// config) > bare `ssh`. Env overriding config for the last two matches git's
140 /// own resolution, so `git-collab release` agrees with `git push` against the
141 /// same remote even when a user sets `core.sshCommand` globally but overrides
142 /// it with `GIT_SSH_COMMAND` in one shell. Unlike git, which runs these
143 /// through `sh -c` and supports shell quoting, this is split on whitespace
144 /// only — a path containing spaces cannot be expressed.
145 fn ssh_command(repo: &Repository, remote: &SshRemote) -> Command {
146 let env_collab = std::env::var("GIT_COLLAB_SSH_COMMAND").ok();
147 let env_git = std::env::var("GIT_SSH_COMMAND").ok();
148 let config_ssh = repo
149 .config()
150 .ok()
151 .and_then(|cfg| cfg.get_string("core.sshCommand").ok());
152 let base = resolve_ssh_command(SshCommandSources {
153 collab_env: env_collab,
154 git_env: env_git,
155 config: config_ssh,
156 });
118 let mut parts = base.split_whitespace(); 157 let mut parts = base.split_whitespace();
119 let mut cmd = Command::new(parts.next().unwrap_or("ssh")); 158 let mut cmd = Command::new(parts.next().unwrap_or("ssh"));
120 for part in parts { 159 for part in parts {
@@ -130,8 +169,13 @@ fn ssh_command(remote: &SshRemote) -> Command {
130 cmd 169 cmd
131 } 170 }
132 171
133 fn run_remote(remote: &SshRemote, remote_cmd: &str, stdin: Stdio) -> Result<Output, Error> { 172 fn run_remote(
134 let output = ssh_command(remote) 173 repo: &Repository,
174 remote: &SshRemote,
175 remote_cmd: &str,
176 stdin: Stdio,
177 ) -> Result<Output, Error> {
178 let output = ssh_command(repo, remote)
135 .arg(remote_cmd) 179 .arg(remote_cmd)
136 .stdin(stdin) 180 .stdin(stdin)
137 .output() 181 .output()
@@ -196,7 +240,7 @@ pub fn publish(
196 if force { 240 if force {
197 remote_cmd.push_str(" --force"); 241 remote_cmd.push_str(" --force");
198 } 242 }
199 let output = run_remote(&remote, &remote_cmd, Stdio::from(handle))?; 243 let output = run_remote(repo, &remote, &remote_cmd, Stdio::from(handle))?;
200 let stdout = String::from_utf8_lossy(&output.stdout); 244 let stdout = String::from_utf8_lossy(&output.stdout);
201 let sha = stdout.trim().strip_prefix("ok ").unwrap_or("").to_string(); 245 let sha = stdout.trim().strip_prefix("ok ").unwrap_or("").to_string();
202 println!("Published {}/{} (sha256 {})", version, filename, sha); 246 println!("Published {}/{} (sha256 {})", version, filename, sha);
@@ -207,7 +251,7 @@ pub fn publish(
207 pub fn list(repo: &Repository, remote_name: &str, json: bool) -> Result<(), Error> { 251 pub fn list(repo: &Repository, remote_name: &str, json: bool) -> Result<(), Error> {
208 let remote = ssh_remote(repo, remote_name)?; 252 let remote = ssh_remote(repo, remote_name)?;
209 let remote_cmd = format!("collab-release list '{}'", remote.path); 253 let remote_cmd = format!("collab-release list '{}'", remote.path);
210 let output = run_remote(&remote, &remote_cmd, Stdio::null())?; 254 let output = run_remote(repo, &remote, &remote_cmd, Stdio::null())?;
211 let stdout = String::from_utf8_lossy(&output.stdout); 255 let stdout = String::from_utf8_lossy(&output.stdout);
212 if json { 256 if json {
213 print!("{}", stdout); 257 print!("{}", stdout);
@@ -256,7 +300,7 @@ pub fn delete(
256 if let Some(name) = filename { 300 if let Some(name) = filename {
257 remote_cmd.push_str(&format!(" '{}'", name)); 301 remote_cmd.push_str(&format!(" '{}'", name));
258 } 302 }
259 run_remote(&remote, &remote_cmd, Stdio::null())?; 303 run_remote(repo, &remote, &remote_cmd, Stdio::null())?;
260 match filename { 304 match filename {
261 Some(name) => println!("Deleted {}/{}", version, name), 305 Some(name) => println!("Deleted {}/{}", version, name),
262 None => println!("Deleted {}", version), 306 None => println!("Deleted {}", version),
@@ -387,4 +431,84 @@ mod tests {
387 let remote = parse_ssh_remote("ssh://git@example.com/myrepo.git").unwrap(); 431 let remote = parse_ssh_remote("ssh://git@example.com/myrepo.git").unwrap();
388 assert!(validate_remote_path(&remote.path).is_ok()); 432 assert!(validate_remote_path(&remote.path).is_ok());
389 } 433 }
434
435 #[test]
436 fn resolve_ssh_command_prefers_collab_env_over_all_others() {
437 assert_eq!(
438 resolve_ssh_command(SshCommandSources {
439 collab_env: Some("collab-ssh".to_string()),
440 git_env: Some("git-ssh".to_string()),
441 config: Some("config-ssh".to_string()),
442 }),
443 "collab-ssh"
444 );
445 }
446
447 /// Regression guard: git's own resolution has GIT_SSH_COMMAND (env)
448 /// override core.sshCommand (config) — the reverse of what an earlier
449 /// version of this function did. A user with core.sshCommand set
450 /// globally who exports GIT_SSH_COMMAND for one shell must see it win,
451 /// matching `git push` against the same remote.
452 ///
453 /// This only guards resolve_ssh_command's own logic; it cannot catch a
454 /// mis-wired call site (e.g. ssh_command() passing git_env and config in
455 /// the wrong struct fields) — see the e2e tests in
456 /// tests/release_cli_test.rs for that.
457 #[test]
458 fn resolve_ssh_command_git_env_overrides_config_when_both_set() {
459 assert_eq!(
460 resolve_ssh_command(SshCommandSources {
461 collab_env: None,
462 git_env: Some("git-ssh".to_string()),
463 config: Some("config-ssh".to_string()),
464 }),
465 "git-ssh"
466 );
467 }
468
469 #[test]
470 fn resolve_ssh_command_falls_back_to_config() {
471 assert_eq!(
472 resolve_ssh_command(SshCommandSources {
473 collab_env: None,
474 git_env: None,
475 config: Some("config-ssh".to_string()),
476 }),
477 "config-ssh"
478 );
479 }
480
481 #[test]
482 fn resolve_ssh_command_defaults_to_bare_ssh() {
483 assert_eq!(
484 resolve_ssh_command(SshCommandSources {
485 collab_env: None,
486 git_env: None,
487 config: None,
488 }),
489 "ssh"
490 );
491 }
492
493 #[test]
494 fn resolve_ssh_command_ignores_empty_and_whitespace_values() {
495 // An empty or whitespace-only override must not win and must not
496 // produce an empty program name — fall through to the next source.
497 assert_eq!(
498 resolve_ssh_command(SshCommandSources {
499 collab_env: Some("".to_string()),
500 git_env: Some(" ".to_string()),
501 config: Some("config-ssh".to_string()),
502 }),
503 "config-ssh"
504 );
505 assert_eq!(
506 resolve_ssh_command(SshCommandSources {
507 collab_env: Some("\t\n".to_string()),
508 git_env: None,
509 config: None,
510 }),
511 "ssh"
512 );
513 }
390 } 514 }
tests/release_cli_test.rs
Old New
@@ -63,6 +63,87 @@ fn publish_list_delete_roundtrip() {
63 assert_eq!(after["versions"].as_array().unwrap().len(), 0); 63 assert_eq!(after["versions"].as_array().unwrap().len(), 0);
64 } 64 }
65 65
66 /// `core.sshCommand` (git's own config for a custom ssh invocation — custom
67 /// keys, ports, jump hosts) must be honored on its own, without
68 /// GIT_COLLAB_SSH_COMMAND set, so `git-collab release` agrees with `git push`
69 /// against the same remote.
70 #[test]
71 fn core_ssh_command_alone_is_sufficient() {
72 let harness = setup("cli-core-ssh-command");
73 harness
74 .work_repo()
75 .git(&["config", "core.sshCommand", &harness.ssh_command_string()]);
76
77 let mut cmd = harness.work_repo().cli_command();
78 cmd.env_remove("GIT_COLLAB_SSH_COMMAND");
79 cmd.args(["release", "list", "--remote", "srv"]);
80 let output = cmd.output().expect("failed to run git-collab release");
81
82 assert!(
83 output.status.success(),
84 "list failed: {}{}",
85 String::from_utf8_lossy(&output.stdout),
86 String::from_utf8_lossy(&output.stderr)
87 );
88 assert!(String::from_utf8_lossy(&output.stdout).contains("No releases."));
89 }
90
91 /// The unit tests for resolve_ssh_command() are purely positional and cannot
92 /// catch a mis-wired call site (e.g. ssh_command() swapping GIT_SSH_COMMAND
93 /// and core.sshCommand when constructing SshCommandSources) — every unit
94 /// test would still pass unchanged since it calls the function directly with
95 /// literal values. These two tests exercise the real call site instead: only
96 /// one of GIT_SSH_COMMAND / core.sshCommand is set to a command that actually
97 /// works, so the test can only pass if the CLI picked the right one.
98 #[test]
99 fn git_ssh_command_env_beats_core_ssh_command() {
100 let harness = setup("cli-git-env-beats-config");
101 // core.sshCommand points at a command that is not ssh at all and will
102 // always fail — if it wins, the release command fails.
103 harness
104 .work_repo()
105 .git(&["config", "core.sshCommand", "/bin/false"]);
106
107 let mut cmd = harness.work_repo().cli_command();
108 cmd.env_remove("GIT_COLLAB_SSH_COMMAND");
109 cmd.env("GIT_SSH_COMMAND", harness.ssh_command_string());
110 cmd.args(["release", "list", "--remote", "srv"]);
111 let output = cmd.output().expect("failed to run git-collab release");
112
113 assert!(
114 output.status.success(),
115 "expected GIT_SSH_COMMAND to win over a failing core.sshCommand: {}{}",
116 String::from_utf8_lossy(&output.stdout),
117 String::from_utf8_lossy(&output.stderr)
118 );
119 }
120
121 /// Inverse of `git_ssh_command_env_beats_core_ssh_command`: this is the
122 /// assertion that actually catches a call-site re-swap, since the other
123 /// direction could pass for the wrong reason if both sources happened to
124 /// work. Here GIT_SSH_COMMAND is the one that always fails, so the command
125 /// only succeeds if core.sshCommand wrongly wins.
126 #[test]
127 fn core_ssh_command_does_not_beat_git_ssh_command_env() {
128 let harness = setup("cli-config-does-not-beat-git-env");
129 harness
130 .work_repo()
131 .git(&["config", "core.sshCommand", &harness.ssh_command_string()]);
132
133 let mut cmd = harness.work_repo().cli_command();
134 cmd.env_remove("GIT_COLLAB_SSH_COMMAND");
135 cmd.env("GIT_SSH_COMMAND", "/bin/false");
136 cmd.args(["release", "list", "--remote", "srv"]);
137 let output = cmd.output().expect("failed to run git-collab release");
138
139 assert!(
140 !output.status.success(),
141 "expected GIT_SSH_COMMAND=/bin/false to win over a working core.sshCommand (i.e. fail), but it succeeded: {}{}",
142 String::from_utf8_lossy(&output.stdout),
143 String::from_utf8_lossy(&output.stderr)
144 );
145 }
146
66 #[test] 147 #[test]
67 fn duplicate_publish_needs_force_flag() { 148 fn duplicate_publish_needs_force_flag() {
68 let harness = setup("cli-force"); 149 let harness = setup("cli-force");