diff --git a/crates/bitbygit-gh/src/lib.rs b/crates/bitbygit-gh/src/lib.rs index 32d7973..c68e7ab 100644 --- a/crates/bitbygit-gh/src/lib.rs +++ b/crates/bitbygit-gh/src/lib.rs @@ -9,6 +9,7 @@ pub const INSTALL_GH_GUIDANCE: &str = "GitHub CLI (gh) is not installed. Install it from https://cli.github.com/."; pub const AUTHENTICATE_GH_GUIDANCE: &str = "GitHub CLI is not authenticated. Run `gh auth login` and try again."; +const DEFAULT_HOSTNAME: &str = "github.com"; #[derive(Debug, Clone)] pub struct GitHub { @@ -16,6 +17,7 @@ pub struct GitHub { executable: PathBuf, #[cfg(test)] executable_args: Vec, + hostname: String, repository: Option, } @@ -30,6 +32,7 @@ impl GitHub { executable: executable.into(), #[cfg(test)] executable_args: Vec::new(), + hostname: DEFAULT_HOSTNAME.to_owned(), repository: None, } } @@ -37,6 +40,7 @@ impl GitHub { pub fn with_executable_and_repository( cwd: impl Into, executable: impl Into, + hostname: impl Into, repository: impl Into, ) -> Self { Self { @@ -44,6 +48,7 @@ impl GitHub { executable: executable.into(), #[cfg(test)] executable_args: Vec::new(), + hostname: hostname.into(), repository: Some(repository.into()), } } @@ -66,7 +71,7 @@ impl GitHub { "status".to_owned(), "--active".to_owned(), "--hostname".to_owned(), - "github.com".to_owned(), + self.hostname.clone(), ]) { Ok(()) => Ok(GhSetupStatus::Ready), Err(GhError::CommandFailed { .. }) => Ok(GhSetupStatus::NotAuthenticated), @@ -111,7 +116,7 @@ impl GitHub { "-f".to_owned(), "per_page=100".to_owned(), "--hostname".to_owned(), - "github.com".to_owned(), + self.hostname.clone(), ])?; let pages: Vec> = parse_json(&output, "pull request query")?; Ok(pages.into_iter().flatten().map(PullRequest::from).collect()) @@ -145,7 +150,7 @@ impl GitHub { url_path_component(branch) ), "--hostname".to_owned(), - "github.com".to_owned(), + self.hostname.clone(), ])?; let pages: Vec> = parse_json(&output, "branch query")?; let expected = format!("refs/heads/{branch}"); @@ -158,7 +163,7 @@ impl GitHub { fn repository_after_ready(&self) -> Result { let mut args = vec!["repo".to_owned(), "view".to_owned()]; if let Some(repository) = &self.repository { - args.push(repository.clone()); + args.push(self.qualified_repository(repository)); } args.extend([ "--json".to_owned(), @@ -213,18 +218,24 @@ impl GitHub { match self.setup_status()? { GhSetupStatus::Ready => Ok(()), GhSetupStatus::MissingCli => Err(GhError::MissingCli), - GhSetupStatus::NotAuthenticated => Err(GhError::NotAuthenticated), + GhSetupStatus::NotAuthenticated => Err(GhError::NotAuthenticated { + hostname: self.hostname.clone(), + }), } } fn with_repository(&self, mut args: Vec) -> Vec { if let Some(repository) = &self.repository { args.push("--repo".to_owned()); - args.push(repository.clone()); + args.push(self.qualified_repository(repository)); } args } + fn qualified_repository(&self, repository: &str) -> String { + format!("{}/{repository}", self.hostname) + } + fn run_status(&self, args: Vec) -> Result<(), GhError> { let output = self.command(&args).output().map_err(|source| { if source.kind() == io::ErrorKind::NotFound { @@ -264,6 +275,7 @@ impl GitHub { let mut command = Command::new(&self.executable); command .current_dir(&self.cwd) + .env("GH_HOST", &self.hostname) .env("GH_PROMPT_DISABLED", "1"); #[cfg(test)] command.args(&self.executable_args); @@ -392,7 +404,7 @@ pub struct CreatedPullRequest { #[derive(Debug)] pub enum GhError { MissingCli, - NotAuthenticated, + NotAuthenticated { hostname: String }, Io { source: io::Error }, CommandFailed { status: ExitStatus }, InvalidInput { name: &'static str }, @@ -403,7 +415,10 @@ impl Display for GhError { fn fmt(&self, formatter: &mut Formatter<'_>) -> fmt::Result { match self { Self::MissingCli => formatter.write_str(INSTALL_GH_GUIDANCE), - Self::NotAuthenticated => formatter.write_str(AUTHENTICATE_GH_GUIDANCE), + Self::NotAuthenticated { hostname } => write!( + formatter, + "GitHub CLI is not authenticated for {hostname}. Run `gh auth login --hostname {hostname}` and try again." + ), Self::Io { source } => write!(formatter, "failed to run GitHub CLI: {source}"), Self::CommandFailed { status } => { write!(formatter, "GitHub CLI failed with status {status}") @@ -426,7 +441,7 @@ impl Error for GhError { match self { Self::Io { source } => Some(source), Self::MissingCli - | Self::NotAuthenticated + | Self::NotAuthenticated { .. } | Self::CommandFailed { .. } | Self::InvalidInput { .. } | Self::InvalidOutput { .. } => None, @@ -560,9 +575,13 @@ mod tests { let fake = FakeGh::new( "case \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 0 ;;\nrepo:view) [ \"$3\" = github.com/octo/repo ] && [ \"$4\" = --json ] && [ \"$5\" = nameWithOwner,defaultBranchRef ] || exit 1; printf '%s\\n' '{\"nameWithOwner\":\"octo/repo\",\"defaultBranchRef\":{\"name\":\"main\"}}' ;;\napi:--method) [ \"$3\" = GET ] && [ \"$4\" = --paginate ] && [ \"$5\" = --slurp ] && [ \"$6\" = repos/octo/repo/pulls ] && [ \"$7\" = -f ] && [ \"$8\" = state=open ] && [ \"$9\" = -f ] && [ \"${10}\" = head=octo:feature ] && [ \"${11}\" = -f ] && [ \"${12}\" = per_page=100 ] && [ \"${13}\" = --hostname ] && [ \"${14}\" = github.com ] || exit 1; printf '%s\\n' '[[{\"number\":42,\"html_url\":\"https://github.com/octo/repo/pull/42\",\"title\":\"Existing PR\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature\",\"repo\":{\"full_name\":\"octo/repo\"}}}]]' ;;\n*) exit 1 ;;\nesac", )?; - let github = - GitHub::with_executable_and_repository(&fake.path, "/bin/sh", "github.com/octo/repo") - .with_executable_args(vec![fake.executable.display().to_string()]); + let github = GitHub::with_executable_and_repository( + &fake.path, + "/bin/sh", + "github.com", + "octo/repo", + ) + .with_executable_args(vec![fake.executable.display().to_string()]); let repository = github.repository()?; let pull_requests = github.existing_pull_requests("feature")?; @@ -616,6 +635,61 @@ mod tests { Ok(()) } + #[test] + fn enterprise_commands_are_scoped_to_the_selected_host() -> Result<(), Box> { + let fake = FakeGh::new( + "[ \"$GH_HOST\" = git.example.com ] || exit 1\ncase \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) [ \"$3:$4:$5\" = --active:--hostname:git.example.com ] || exit 1 ;;\nrepo:view) [ \"$3\" = git.example.com/octo/repo ] || exit 1; printf '%s\\n' '{\"nameWithOwner\":\"octo/repo\",\"defaultBranchRef\":{\"name\":\"main\"}}' ;;\napi:--method) case \"$6\" in\n*/pulls) [ \"${13}:${14}\" = --hostname:git.example.com ] || exit 1; printf '%s\\n' '[[]]' ;;\n*/git/matching-refs/heads/*) [ \"$7:$8\" = --hostname:git.example.com ] || exit 1; printf '%s\\n' '[[]]' ;;\nesac ;;\npr:create) case \"$*\" in *'--repo git.example.com/octo/repo'*) printf '%s\\n' 'https://git.example.com/octo/repo/pull/43' ;; *) exit 1 ;; esac ;;\n*) exit 1 ;;\nesac", + )?; + let github = GitHub::with_executable_and_repository( + &fake.path, + "/bin/sh", + "git.example.com", + "octo/repo", + ) + .with_executable_args(vec![fake.executable.display().to_string()]); + + assert_eq!(github.repository()?.name_with_owner, "octo/repo"); + assert!(github.existing_pull_requests("feature")?.is_empty()); + assert!(!github.branch_exists("main")?); + assert_eq!( + github + .create_pull_request(&CreatePullRequest { + title: "Feature".to_owned(), + body: String::new(), + base: "main".to_owned(), + head: "feature".to_owned(), + })? + .url, + "https://git.example.com/octo/repo/pull/43" + ); + Ok(()) + } + + #[test] + fn enterprise_auth_failure_names_the_selected_host() -> Result<(), Box> { + let fake = FakeGh::new( + "case \"$1:$2\" in\n--version:*) exit 0 ;;\nauth:status) exit 1 ;;\nesac\nexit 1", + )?; + let github = GitHub::with_executable_and_repository( + &fake.path, + "/bin/sh", + "git.example.com", + "octo/repo", + ) + .with_executable_args(vec![fake.executable.display().to_string()]); + + let error = match github.repository() { + Ok(_) => return Err(io::Error::other("authentication must fail").into()), + Err(error) => error, + }; + + assert_eq!( + error.to_string(), + "GitHub CLI is not authenticated for git.example.com. Run `gh auth login --hostname git.example.com` and try again." + ); + Ok(()) + } + #[test] fn creates_pull_request_with_literal_shell_metacharacters_in_title() -> Result<(), Box> { diff --git a/crates/bitbygit-tui/src/lib.rs b/crates/bitbygit-tui/src/lib.rs index c076f9d..976bb99 100644 --- a/crates/bitbygit-tui/src/lib.rs +++ b/crates/bitbygit-tui/src/lib.rs @@ -599,6 +599,29 @@ struct QueuedPromptSequence { remaining_requests: Vec, } +#[derive(Debug, Clone, PartialEq, Eq)] +struct GitHubRepository(String); + +impl GitHubRepository { + fn new(hostname: &str, name_with_owner: &str) -> Self { + Self(format!("{hostname}/{name_with_owner}")) + } + + fn hostname(&self) -> &str { + self.0.split_once('/').map_or("", |(hostname, _)| hostname) + } + + fn name_with_owner(&self) -> &str { + self.0 + .split_once('/') + .map_or("", |(_, name_with_owner)| name_with_owner) + } + + fn eq_ignore_ascii_case(&self, other: &Self) -> bool { + self.0.eq_ignore_ascii_case(&other.0) + } +} + impl QueuedPromptSequence { fn new(first: PreparedOperation, remaining_requests: Vec) -> Self { Self { @@ -686,8 +709,8 @@ enum PendingPayload { base: String, title: String, repository: String, - github_repository: String, - head_github_repository: String, + github_repository: GitHubRepository, + head_github_repository: GitHubRepository, head_repository: String, head: String, github_executable: Option, @@ -1210,12 +1233,13 @@ fn open_pull_request_plan( head: &str, base: &str, title: &str, - repository: &str, + repository: &GitHubRepository, existing_url: Option<&str>, ) -> OperationPlan { let target = existing_url.map(ToOwned::to_owned).unwrap_or_else(|| { format!( - "https://github.com/{repository}/compare/{}...{}?expand=1", + "https://{}/compare/{}...{}?expand=1", + repository.0, compare_url_ref(base), compare_url_ref(head) ) @@ -1929,11 +1953,8 @@ impl OperationPlanner { .remote_push_urls(&remote) .map_err(|error| format!("Unable to prepare pull request remote: {error}"))?; let push_url = single_pull_request_push_url(&remote, &remote_urls)?; - let head_github_repository = github_repository_from_push_url(push_url).ok_or_else(|| { - format!( - "Open pull request blocked: remote {remote} does not identify a GitHub repository." - ) - })?; + let head_github_repository = github_repository_from_push_url(push_url) + .map_err(|reason| format!("Open pull request blocked: remote {remote} {reason}"))?; let target = git .head_target() .map_err(|error| format!("Unable to snapshot pull request branch: {error}"))?; @@ -1942,6 +1963,15 @@ impl OperationPlanner { .to_owned() })?; let github_repository = github_base_repository(&git, &remote, &head_github_repository)?; + if !head_github_repository + .hostname() + .eq_ignore_ascii_case(github_repository.hostname()) + { + return Err( + "Open pull request blocked: source and upstream remotes use different GitHub hosts." + .to_owned(), + ); + } let github = self.github(&github_repository); let repository = github.repository().map_err(open_pull_request_gh_error)?; let head_github = self.github(&head_github_repository); @@ -1960,8 +1990,10 @@ impl OperationPlanner { return Err("Open pull request blocked: current branch is not pushed to its upstream. Push it with `push` first.".to_owned()); } let head_repository = canonical_head_repository.name_with_owner; - let canonical_head_github_repository = format!("github.com/{head_repository}"); - let canonical_github_repository = format!("github.com/{}", repository.name_with_owner); + let canonical_head_github_repository = + GitHubRepository::new(head_github_repository.hostname(), &head_repository); + let canonical_github_repository = + GitHubRepository::new(github_repository.hostname(), &repository.name_with_owner); let head = pull_request_head( &canonical_head_github_repository, &canonical_github_repository, @@ -2002,7 +2034,7 @@ impl OperationPlanner { &head, &base, &title, - &repository.name_with_owner, + &canonical_github_repository, existing_url, ), ExecutionContext::from_payload(PendingPayload::OpenPullRequest { @@ -2055,12 +2087,20 @@ impl OperationPlanner { Git::new(self.repo_root.clone()) } - fn github(&self, repository: &str) -> GitHub { + fn github(&self, repository: &GitHubRepository) -> GitHub { match &self.github_executable { - Some(executable) => { - GitHub::with_executable_and_repository(&self.repo_root, executable, repository) - } - None => GitHub::with_executable_and_repository(&self.repo_root, "gh", repository), + Some(executable) => GitHub::with_executable_and_repository( + &self.repo_root, + executable, + repository.hostname(), + repository.name_with_owner(), + ), + None => GitHub::with_executable_and_repository( + &self.repo_root, + "gh", + repository.hostname(), + repository.name_with_owner(), + ), } } } @@ -2093,12 +2133,14 @@ fn single_pull_request_push_url<'a>(remote: &str, urls: &'a [String]) -> Result< } } -fn github_repository_from_push_url(url: &str) -> Option { +fn github_repository_from_push_url(url: &str) -> Result { let (host, path) = if let Some((scheme, rest)) = url.split_once("://") { if !scheme.eq_ignore_ascii_case("https") && !scheme.eq_ignore_ascii_case("ssh") { - return None; + return Err("uses an unsupported URL; configure an HTTPS or SSH GitHub remote URL."); } - let (authority, path) = rest.split_once('/')?; + let (authority, path) = rest + .split_once('/') + .ok_or("has an invalid URL; use HOST/OWNER/REPOSITORY form.")?; ( authority .rsplit_once('@') @@ -2106,9 +2148,11 @@ fn github_repository_from_push_url(url: &str) -> Option { path, ) } else { - let (authority, path) = url.split_once(':')?; - if authority.contains('/') { - return None; + let (authority, path) = url + .split_once(':') + .ok_or("uses an unsupported URL; configure an HTTPS or SSH GitHub remote URL.")?; + if authority.contains('/') || path.starts_with(':') { + return Err("uses an unsupported URL; configure an HTTPS or SSH GitHub remote URL."); } ( authority @@ -2117,45 +2161,82 @@ fn github_repository_from_push_url(url: &str) -> Option { path, ) }; - if !host.eq_ignore_ascii_case("github.com") { - return None; + if host.contains(':') { + return Err( + "uses a custom port, which is not supported by the GitHub CLI integration; configure a hostname-only remote URL.", + ); + } + let hostname = host.to_ascii_lowercase(); + if !valid_github_hostname(&hostname) { + return Err( + "has an invalid GitHub hostname; configure a valid HTTPS or SSH GitHub remote URL.", + ); } let mut components = path.trim_end_matches('/').split('/'); - let owner = components.next()?; - let repository = components.next()?; + let owner = components + .next() + .ok_or("does not identify a GitHub OWNER/REPOSITORY.")?; + let repository = components + .next() + .ok_or("does not identify a GitHub OWNER/REPOSITORY.")?; let repository = repository.strip_suffix(".git").unwrap_or(repository); - if host.is_empty() - || owner.is_empty() + if owner.is_empty() || repository.is_empty() || components.next().is_some() || owner.contains(['?', '#']) || repository.contains(['?', '#']) { - return None; + return Err("does not identify a GitHub OWNER/REPOSITORY."); } - Some(format!("github.com/{owner}/{repository}")) + Ok(GitHubRepository::new( + &hostname, + &format!("{owner}/{repository}"), + )) +} + +fn valid_github_hostname(hostname: &str) -> bool { + !hostname.is_empty() + && hostname.len() <= 253 + && hostname.split('.').all(|label| { + !label.is_empty() + && label.len() <= 63 + && label + .bytes() + .all(|byte| byte.is_ascii_alphanumeric() || byte == b'-') + && label + .as_bytes() + .first() + .is_some_and(u8::is_ascii_alphanumeric) + && label + .as_bytes() + .last() + .is_some_and(u8::is_ascii_alphanumeric) + }) } fn github_base_repository( git: &Git, push_remote: &str, - head_repository: &str, -) -> Result { + head_repository: &GitHubRepository, +) -> Result { let remotes = git .remotes() .map_err(|error| format!("Unable to prepare pull request base remote: {error}"))?; - if let Some(fetch_repository) = remotes + if let Some(fetch_url) = remotes .iter() .find(|remote| remote.name == push_remote) .and_then(|remote| remote.fetch_url.as_deref()) - .and_then(github_repository_from_push_url) - .filter(|repository| !repository.eq_ignore_ascii_case(head_repository)) { - return Ok(fetch_repository); + let fetch_repository = github_repository_from_push_url(fetch_url).map_err(|reason| { + format!("Open pull request blocked: remote {push_remote} fetch URL {reason}") + })?; + if !fetch_repository.eq_ignore_ascii_case(head_repository) { + return Ok(fetch_repository); + } } let upstream = remotes.into_iter().find(|remote| remote.name == "upstream"); let Some(upstream) = upstream else { - return Ok(head_repository.to_owned()); + return Ok(head_repository.clone()); }; let url = upstream .fetch_url @@ -2165,36 +2246,34 @@ fn github_base_repository( "Open pull request blocked: upstream remote has no URL; configure the base repository or remove the remote." .to_owned() })?; - github_repository_from_push_url(url).ok_or_else(|| { - "Open pull request blocked: upstream remote does not identify a GitHub repository." - .to_owned() - }) + github_repository_from_push_url(url) + .map_err(|reason| format!("Open pull request blocked: upstream remote {reason}")) } fn pull_request_head( - head_repository: &str, - base_repository: &str, + head_repository: &GitHubRepository, + base_repository: &GitHubRepository, branch: &str, ) -> Result { if head_repository.eq_ignore_ascii_case(base_repository) { return Ok(branch.to_owned()); } - let (head_host, head_name) = head_repository.split_once('/').ok_or_else(|| { - "Open pull request blocked: source remote does not identify a GitHub repository.".to_owned() - })?; - let (base_host, _) = base_repository.split_once('/').ok_or_else(|| { - "Open pull request blocked: upstream remote does not identify a GitHub repository." - .to_owned() - })?; - if !head_host.eq_ignore_ascii_case(base_host) { + if !head_repository + .hostname() + .eq_ignore_ascii_case(base_repository.hostname()) + { return Err( "Open pull request blocked: source and upstream remotes use different GitHub hosts." .to_owned(), ); } - let (owner, _) = head_name.split_once('/').ok_or_else(|| { - "Open pull request blocked: source remote does not identify a GitHub repository.".to_owned() - })?; + let (owner, _) = head_repository + .name_with_owner() + .split_once('/') + .ok_or_else(|| { + "Open pull request blocked: source remote does not identify a GitHub repository." + .to_owned() + })?; Ok(format!("{owner}:{branch}")) } @@ -2961,11 +3040,15 @@ impl PlanExecutor { Some(executable) => GitHub::with_executable_and_repository( &self.repo_root, executable, - github_repository, + github_repository.hostname(), + github_repository.name_with_owner(), + ), + None => GitHub::with_executable_and_repository( + &self.repo_root, + "gh", + github_repository.hostname(), + github_repository.name_with_owner(), ), - None => { - GitHub::with_executable_and_repository(&self.repo_root, "gh", github_repository) - } }; let current_repository = github.repository().map_err(StepExecutionError::GitHub)?; if !current_repository @@ -2981,12 +3064,14 @@ impl PlanExecutor { Some(executable) => GitHub::with_executable_and_repository( &self.repo_root, executable, - head_github_repository, + head_github_repository.hostname(), + head_github_repository.name_with_owner(), ), None => GitHub::with_executable_and_repository( &self.repo_root, "gh", - head_github_repository, + head_github_repository.hostname(), + head_github_repository.name_with_owner(), ), }; let current_head_repository = @@ -3021,8 +3106,14 @@ impl PlanExecutor { )); } let current_head = pull_request_head( - &format!("github.com/{}", current_head_repository.name_with_owner), - &format!("github.com/{}", current_repository.name_with_owner), + &GitHubRepository::new( + head_github_repository.hostname(), + ¤t_head_repository.name_with_owner, + ), + &GitHubRepository::new( + github_repository.hostname(), + ¤t_repository.name_with_owner, + ), upstream_branch, ) .map_err(StepExecutionError::Blocked)?; @@ -3751,7 +3842,7 @@ fn sanitized_git_error(error: &GitError) -> String { fn sanitized_github_error(error: &GhError) -> String { match error { - GhError::MissingCli | GhError::NotAuthenticated | GhError::InvalidInput { .. } => { + GhError::MissingCli | GhError::NotAuthenticated { .. } | GhError::InvalidInput { .. } => { error.to_string() } GhError::CommandFailed { status } => format!("GitHub CLI failed with status {status}"), @@ -4279,6 +4370,104 @@ mod tests { Ok(()) } + #[test] + fn enterprise_https_pull_request_scopes_all_gh_commands_to_remote_host() + -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-enterprise-https")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "https://git.example.com/octo/repo.git", + ], + )?; + let fake_gh = fake_gh("open-pr-enterprise-https", false)?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!( + operation + .plan + .preview_text() + .contains("https://git.example.com/octo/repo/compare/main...feature/open-pr") + ); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-enterprise-https-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + assert!(execution.succeeded(), "{}", execution.message()); + assert_eq!( + execution.message(), + "Pull request created: https://git.example.com/octo/repo/pull/43" + ); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + assert!(invocations.contains("auth status --active --hostname git.example.com")); + assert!(invocations.contains("repo view git.example.com/octo/repo")); + assert!(invocations.contains("--hostname git.example.com")); + assert!(invocations.contains("pr create")); + assert!(invocations.contains("--repo git.example.com/octo/repo")); + Ok(()) + } + + #[test] + fn enterprise_ssh_remote_surfaces_existing_pull_request() -> Result<(), Box> { + let repo = pushed_branch_repo("open-pr-enterprise-ssh-existing")?; + git_stdout( + &repo, + &[ + "remote", + "set-url", + "origin", + "git@git.example.com:octo/repo.git", + ], + )?; + let fake_gh = fake_gh_with_pull_requests( + "open-pr-enterprise-ssh-existing", + "[{\"number\":42,\"html_url\":\"https://git.example.com/octo/repo/pull/42\",\"title\":\"Existing PR\",\"base\":{\"ref\":\"main\"},\"head\":{\"ref\":\"feature/open-pr\",\"repo\":{\"full_name\":\"octo/repo\"}}}]", + true, + )?; + let planner = OperationPlanner { + repo_root: repo.clone(), + github_executable: Some(fake_gh.clone()), + ssh_executable: Some(test_ssh_command()?), + }; + + let operation = planner + .plan_request(OperationRequest::OpenPullRequest { base: None }) + .map_err(std::io::Error::other)?; + + assert!( + operation + .plan + .preview_text() + .contains("https://git.example.com/octo/repo/pull/42") + ); + let execution = PlanExecutor::with_audit_paths_and_ssh( + &repo, + isolated_store_paths("open-pr-enterprise-ssh-existing-audit")?, + test_ssh_command()?, + ) + .execute(&operation.plan, operation.context); + assert!(execution.succeeded(), "{}", execution.message()); + assert!(execution.message().contains("Pull request already open")); + let invocations = std::fs::read_to_string(fake_gh.with_file_name("invocations"))?; + assert!(invocations.contains("head=octo:feature/open-pr")); + assert!(invocations.contains("--hostname git.example.com")); + assert!(!invocations.contains("pr:create")); + Ok(()) + } + #[test] fn pull_request_uses_differently_named_tracked_remote_branch() -> Result<(), Box> { let repo = pushed_branch_repo("open-pr-different-upstream-branch")?; @@ -4558,7 +4747,7 @@ mod tests { Err(error) => error, }; - assert!(error.contains("does not identify a GitHub repository")); + assert!(error.contains("unsupported URL")); assert!(!marker.exists()); Ok(()) } @@ -4912,7 +5101,7 @@ mod tests { "feature%ready", "release#candidate", "feature%ready", - "octo/repo", + &GitHubRepository::new("github.com", "octo/repo"), None, ); @@ -4921,6 +5110,39 @@ mod tests { )); } + #[test] + fn github_remote_parser_supports_github_and_enterprise_ssh_forms() { + for (url, hostname) in [ + ("https://github.com/octo/repo.git", "github.com"), + ("ssh://git@github.com/octo/repo.git", "github.com"), + ("https://git.example.com/octo/repo.git", "git.example.com"), + ("ssh://git@git.example.com/octo/repo.git", "git.example.com"), + ("git@git.example.com:octo/repo.git", "git.example.com"), + ] { + assert_eq!( + github_repository_from_push_url(url), + Ok(GitHubRepository::new(hostname, "octo/repo")) + ); + } + } + + #[test] + fn github_remote_parser_rejects_unsupported_or_invalid_targets() { + for url in [ + "http://git.example.com/octo/repo.git", + "https://git.example.com:8443/octo/repo.git", + "ssh://git@-git.example.com/octo/repo.git", + "https://git.example.com/octo", + "https://git.example.com/octo/repo/extra.git", + "file:///octo/repo.git", + ] { + assert!( + github_repository_from_push_url(url).is_err(), + "{url} must fail closed" + ); + } + } + #[test] fn open_pull_request_blocks_missing_upstream_and_matching_base() -> Result<(), Box> { let repo = isolated_git_repo("open-pr-invalid-state")?; @@ -5839,7 +6061,7 @@ mod tests { let base_missing = root.join("base-missing"); let head_missing = root.join("head-missing"); let create_response = if create_succeeds { - "printf '%s\\n' 'https://github.com/octo/repo/pull/43' ;;" + "printf 'https://%s/%s/pull/43\\n' \"$GH_HOST\" \"$repository\" ;;" } else { "exit 1 ;;" };