diff --git a/docs/editing.md b/docs/editing.md index 2ac704d..c339f31 100644 --- a/docs/editing.md +++ b/docs/editing.md @@ -382,6 +382,8 @@ GitHub, GitLab merge requests, and Gitea are supported. API reads require `curl` Standard GitHub/GitLab/Gitea hosts and Codeberg are detected from HTTPS or SSH remote URLs. For another host, set `remote..vamp-forge` to `github`, `gitlab`, or `gitea` in the repository's Git configuration, for example `git config remote.origin.vamp-forge gitea`. For a non-colocated jj repository, use `git --git-dir .jj/repo/store/git config remote.origin.vamp-forge gitea`. +SSH aliases are resolved with `ssh -G` before provider detection and API authentication, honoring the effective `HostName` and user-specific `Match` settings. SSH ports do not become HTTPS API ports. Git fetches retain the original remote URL, alias, and SSH identity configuration. A failed API load shows its error without also claiming that the search has no matches; the empty-result message appears only after a successful load. + The picker loads every API page and searches locally without additional network requests when changing tabs or typing. GitLab also reads each request's historical diff metadata; large histories can take longer, and Escape cancels outstanding work. Fork heads are fetched through the target repository's PR refs, into the existing private comparison store, without changing active refs, jj operations, or checkout files. Closed/merged requests retain historical bases and heads rather than comparing against today's branch tips. If the server no longer retains the required commits, opening the request reports an error instead of displaying a different diff. Review submission and comments are separate, not part of this picker. Requests with missing/unprepared historical metadata remain listed with an `unavailable` reason. Selecting one reports that error without blocking other requests or substituting a live branch tip. diff --git a/src/forge.rs b/src/forge.rs index bd6c4f0..0252e35 100644 --- a/src/forge.rs +++ b/src/forge.rs @@ -64,8 +64,33 @@ fn decode(path: &str) -> Result { String::from_utf8(decoded).map_err(|_| "Remote path is not UTF-8".into()) } +fn ssh_hostname( + mut command: Command, + destination: &str, + port: &str, + cancelled: &AtomicBool, +) -> Result { + command.arg("-G"); + if !port.is_empty() { + command.args(["-p", &port[1..]]); + } + command.args(["--", destination]); + let output = checked(run(command, vec![], cancelled)?) + .map_err(|error| format!("Cannot resolve SSH remote configuration: {error}"))?; + let output = std::str::from_utf8(&output).map_err(|_| "SSH configuration is not UTF-8")?; + output + .lines() + .find_map(|line| line.strip_prefix("hostname ")) + .map(str::to_owned) + .ok_or_else(|| "SSH configuration returned no hostname".into()) +} + impl Forge { - fn parse(url: &str, provider: Option<&str>) -> Result { + fn parse( + url: &str, + provider: Option<&str>, + resolve_ssh: impl FnOnce(&str, &str) -> Result, + ) -> Result { if url.bytes().any(|b| b <= b' ' || b == 127) || url.contains(['?', '#', '\\']) { return Err("Invalid forge remote URL".into()); } @@ -89,7 +114,15 @@ impl Forge { } (&url[..separator], &url[separator + 1..], true) }; - // Never reuse URL userinfo as an API credential or include it in arguments/errors. + let destination = authority; + // Git URL userinfo must never become an API credential. + if ssh + && authority + .rsplit_once('@') + .is_some_and(|(user, _)| user.contains(':')) + { + return Err("SSH remote URLs must not contain passwords".into()); + } let authority = authority.rsplit('@').next().unwrap(); let (host, port) = if authority.starts_with('[') { let end = authority.find(']').ok_or("Invalid remote host")?; @@ -119,6 +152,21 @@ impl Forge { return Err("Invalid remote port".into()); } let mut host = host.to_ascii_lowercase(); + if ssh { + host = resolve_ssh(&destination[..destination.len() - port.len()], port)? + .to_ascii_lowercase(); + let ipv6 = host.trim_start_matches('[').trim_end_matches(']'); + if ipv6.parse::().is_ok() { + host = format!("[{ipv6}]"); + } else if host.is_empty() + || host.starts_with('-') + || !host + .bytes() + .all(|b| b.is_ascii_alphanumeric() || b".-".contains(&b)) + { + return Err("SSH configuration returned an invalid hostname".into()); + } + } if ssh && host == "ssh.github.com" { host = "github.com".into(); } @@ -487,7 +535,11 @@ pub(crate) fn list( root: &Path, cancelled: &AtomicBool, ) -> Result, String> { - let forge = Forge::parse(url, provider)?; + let forge = Forge::parse(url, provider, |destination, port| { + let mut command = Command::new("ssh"); + command.current_dir(root); + ssh_hostname(command, destination, port, cancelled) + })?; let token = forge.token(root, cancelled)?; let mut gitea_heads = BTreeMap::new(); let query = match forge.provider { @@ -594,22 +646,94 @@ mod tests { use super::*; use serde_json::json; + fn parse(url: &str, provider: Option<&str>) -> Result { + Forge::parse(url, provider, |destination, _| { + Ok(destination.rsplit('@').next().unwrap().to_owned()) + }) + } + + #[test] + fn ssh_aliases_use_effective_hostname_and_preserve_user_matching() { + use std::{fs, io::Write, path::PathBuf}; + struct Config(PathBuf); + impl Drop for Config { + fn drop(&mut self) { + let _ = fs::remove_file(&self.0); + } + } + let path = std::env::temp_dir().join(format!("vamp-forge-ssh-{}.conf", std::process::id())); + let mut file = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + .unwrap(); + let config = Config(path); + file.write_all(b"Host gitea-personal\n HostName gitea.example.org\n Port 2222\nHost github-personal\n HostName ssh.github.com\nHost user-match\n Port 2222\nMatch originalhost user-match user git\n HostName gitlab.example.org\n").unwrap(); + for (url, expected) in [ + ( + "git@gitea-personal:owner/repo.git", + "https://gitea.example.org/api/v1/repos/owner/repo/pulls", + ), + ( + "ssh://git@gitea-personal:2200/owner/repo.git", + "https://gitea.example.org/api/v1/repos/owner/repo/pulls", + ), + ( + "git@github-personal:owner/repo.git", + "https://api.github.com/repos/owner/repo/pulls", + ), + ( + "git@user-match:group/sub/repo.git", + "https://gitlab.example.org/api/v4/projects/group%2Fsub%2Frepo/merge_requests", + ), + ] { + let forge = Forge::parse(url, None, |destination, port| { + let mut command = Command::new("ssh"); + command.arg("-F").arg(&config.0); + ssh_hostname(command, destination, port, &AtomicBool::new(false)) + }) + .unwrap(); + assert_eq!(forge.endpoint, expected); + } + assert!( + Forge::parse("git@alias:owner/repo", Some("gitea"), |_, _| Ok( + "evil.example/path".into() + )) + .is_err() + ); + assert!( + Forge::parse( + "ssh://user:secret@alias/owner/repo", + Some("gitea"), + |_, _| { panic!("Password-bearing SSH URLs must be rejected before starting SSH") } + ) + .is_err() + ); + assert_eq!( + Forge::parse("https://github.com/owner/repo", None, |_, _| panic!( + "HTTPS must not invoke SSH" + )) + .unwrap() + .host, + "github.com" + ); + } + #[test] fn remote_urls_preserve_namespaces_and_keep_ssh_ports_out_of_https() { - let gitlab = - Forge::parse("ssh://git@gitlab.example.org:2222/group/sub/repo.git", None).unwrap(); + let gitlab = parse("ssh://git@gitlab.example.org:2222/group/sub/repo.git", None).unwrap(); assert_eq!( gitlab.endpoint, "https://gitlab.example.org/api/v4/projects/group%2Fsub%2Frepo/merge_requests" ); assert_eq!( - Forge::parse("git@github.com:owner/repo.git", None) + parse("git@github.com:owner/repo.git", None) .unwrap() .endpoint, "https://api.github.com/repos/owner/repo/pulls" ); assert_eq!( - Forge::parse( + parse( "https://user:secret@forge.example:8443/owner/repo.git", Some("gitea") ) @@ -618,7 +742,7 @@ mod tests { "https://forge.example:8443/api/v1/repos/owner/repo/pulls" ); assert!( - Forge::parse("https://forge.example/owner/repo", None) + parse("https://forge.example/owner/repo", None) .err() .unwrap() .contains("git config remote..vamp-forge github") @@ -630,7 +754,7 @@ mod tests { "https://github.com:0/owner/repo", "file:///owner/repo", ] { - assert!(Forge::parse(url, Some("github")).is_err(), "{url}"); + assert!(parse(url, Some("github")).is_err(), "{url}"); } } diff --git a/src/tui.rs b/src/tui.rs index b2e7c4d..fcfcdbc 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -2790,7 +2790,7 @@ fn draw_diff( )); } } - if lines.is_empty() && !diff.loading { + if lines.is_empty() && !diff.loading && diff.error.is_none() { let empty = if pull_requests { "No matching pull requests" } else { @@ -6751,6 +6751,7 @@ mod tests { diff.apply_result(1, Err("offline".into())); render_diff(&mut terminal, &diff, &mut panels); assert!(screen_text(&terminal).contains("offline")); + assert!(!screen_text(&terminal).contains("No matching pull requests")); diff.handle(Input::Enter); diff.take_request(); diff.apply_result(