diff --git a/codex-rs/core/src/responses_metadata.rs b/codex-rs/core/src/responses_metadata.rs index 795cbf1eca..e939afc2da 100644 --- a/codex-rs/core/src/responses_metadata.rs +++ b/codex-rs/core/src/responses_metadata.rs @@ -47,6 +47,9 @@ pub(crate) const ANALYTICS_ENABLED_KEY: &str = "analytics_enabled"; pub(crate) const HISTORY_INITIALIZATION_KEY: &str = "history_initialization"; pub(crate) const MCP_ATTRIBUTION_CLIENT_METADATA_KEY: &str = "mcp_attribution"; pub(crate) const MAX_MCP_ATTRIBUTION_BYTES: usize = 16 * 1024; +// ResponsesAPI rejects a whole client_metadata value above this character limit. +// Turn metadata is ASCII JSON, so its byte and character lengths are identical. +const MAX_TURN_METADATA_BYTES: usize = 8096; pub(crate) const FORKED_FROM_THREAD_ID_KEY: &str = "forked_from_thread_id"; pub(crate) const FORKED_FROM_ORDINAL_EXCLUSIVE_KEY: &str = "forked_from_ordinal_exclusive"; @@ -328,7 +331,7 @@ impl CodexResponsesMetadata { } pub(crate) fn turn_metadata_json(&self) -> Option { - to_ascii_json_string(&self.turn_metadata_payload()).ok() + bounded_turn_metadata_json(self.turn_metadata_payload()).ok() } pub(crate) fn turn_metadata_value(&self) -> Option { @@ -393,7 +396,7 @@ impl CodexResponsesMetadata { // Direct x-codex-turn-metadata is compatibility output. Keep the unbounded tool inventory // in client_metadata only so HTTP and WebSocket compatibility headers remain bounded. if self.has_turn_metadata() - && let Ok(turn_metadata_json) = to_ascii_json_string(&CodexTurnMetadataPayload { + && let Ok(turn_metadata_json) = bounded_turn_metadata_json(CodexTurnMetadataPayload { tool_namespaces_info: None, ..self.turn_metadata_payload() }) @@ -555,7 +558,78 @@ fn non_empty_workspaces( (!workspaces.is_empty()).then_some(workspaces) } -#[derive(Serialize)] +fn bounded_turn_metadata_json(payload: CodexTurnMetadataPayload<'_>) -> serde_json::Result { + let full = to_ascii_json_string(&payload)?; + if full.len() <= MAX_TURN_METADATA_BYTES { + return Ok(full); + } + let Some(original_workspaces) = payload.workspaces else { + return Ok(full); + }; + + // Reserve all non-remote metadata before fitting complete URLs. A remote must + // never be shortened into a different repository identity. + let mut workspaces = original_workspaces.clone(); + let mut candidates = Vec::new(); + for path in original_workspaces.keys() { + let Some(workspace) = workspaces.get_mut(path) else { + continue; + }; + let Some(remotes) = workspace.associated_remote_urls.take() else { + continue; + }; + if remotes.is_empty() { + workspace.associated_remote_urls = Some(remotes); + continue; + } + let without_remotes_len = to_ascii_json_string(workspace)?.len(); + workspace.associated_remote_urls = Some(BTreeMap::new()); + let first_map_overhead = to_ascii_json_string(workspace)?.len() - without_remotes_len; + workspace.associated_remote_urls = None; + for (name, url) in remotes { + // [name,url] and name:url differ by the two array delimiters. + let entry_len = to_ascii_json_string(&(&name, &url))?.len() - 2; + candidates.push((path.as_str(), name, url, entry_len, first_map_overhead)); + } + } + let mut projected_len = to_ascii_json_string(&CodexTurnMetadataPayload { + workspaces: Some(&workspaces), + ..payload.clone() + })? + .len(); + if projected_len > MAX_TURN_METADATA_BYTES || candidates.is_empty() { + return Ok(full); + } + candidates.sort_unstable_by(|a, b| { + (a.1 != "origin") + .cmp(&(b.1 != "origin")) + .then_with(|| a.3.cmp(&b.3)) + .then_with(|| a.0.cmp(b.0)) + .then_with(|| a.1.cmp(&b.1)) + }); + for (path, name, url, entry_len, first_map_overhead) in candidates { + let Some(workspace) = workspaces.get_mut(path) else { + continue; + }; + let overhead = match &workspace.associated_remote_urls { + Some(_) => 1, + None => first_map_overhead, + }; + if entry_len + overhead <= MAX_TURN_METADATA_BYTES - projected_len { + projected_len += entry_len + overhead; + workspace + .associated_remote_urls + .get_or_insert_default() + .insert(name, url); + } + } + to_ascii_json_string(&CodexTurnMetadataPayload { + workspaces: Some(&workspaces), + ..payload + }) +} + +#[derive(Clone, Serialize)] struct CodexTurnMetadataPayload<'a> { #[serde(default, skip_serializing_if = "Option::is_none")] guardian_recorded_items: Option, @@ -620,3 +694,7 @@ struct CodexTurnMetadataPayload<'a> { #[serde(flatten)] extra: &'a BTreeMap, } + +#[cfg(test)] +#[path = "responses_metadata_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/responses_metadata_tests.rs b/codex-rs/core/src/responses_metadata_tests.rs new file mode 100644 index 0000000000..654a8e7da3 --- /dev/null +++ b/codex-rs/core/src/responses_metadata_tests.rs @@ -0,0 +1,98 @@ +#![cfg(test)] + +use super::*; +use pretty_assertions::assert_eq; +use serde_json::json; + +fn metadata_with_remote(url: &str) -> CodexResponsesMetadata { + let mut metadata = CodexResponsesMetadata::new( + "install".into(), + "session".into(), + "thread".into(), + "window".into(), + ); + metadata.request_kind = Some(CodexResponsesRequestKind::Turn); + metadata.turn_id = Some("turn".into()); + metadata.workspaces.insert( + "/workspace/東京".into(), + TurnMetadataWorkspace { + associated_remote_urls: Some(BTreeMap::from([( + "origin".into(), + SanitizedGitUrl::try_from(url).unwrap(), + )])), + latest_git_commit_hash: Some("abc123".into()), + has_changes: Some(false), + }, + ); + metadata +} + +#[test] +fn turn_metadata_keeps_the_exact_boundary_and_omits_an_overlong_remote() { + let prefix = "https://example.invalid/"; + let initial_len = metadata_with_remote(prefix) + .turn_metadata_json() + .unwrap() + .len(); + let padding = "x".repeat(MAX_TURN_METADATA_BYTES - initial_len); + let boundary_url = format!("{prefix}{padding}"); + let boundary = metadata_with_remote(&boundary_url); + let expected = to_ascii_json_string(&boundary.turn_metadata_payload()).unwrap(); + assert_eq!(expected.len(), MAX_TURN_METADATA_BYTES); + assert_eq!(boundary.turn_metadata_json().unwrap(), expected); + + let over = metadata_with_remote(&format!("{boundary_url}x")); + let body = over.client_metadata(/*include_internal*/ false); + let canonical = &body[X_CODEX_TURN_METADATA_HEADER]; + assert_eq!( + canonical, + over.compatibility_headers()[X_CODEX_TURN_METADATA_HEADER] + .to_str() + .unwrap() + ); + assert_eq!( + serde_json::from_str::(canonical).unwrap()["workspaces"]["/workspace/東京"], + json!({"latest_git_commit_hash":"abc123","has_changes":false}) + ); +} + +#[test] +fn oversized_remote_preserves_other_metadata_and_short_remote_urls() { + let mut metadata = + metadata_with_remote(&format!("https://example.invalid/{}é", "o".repeat(5000))); + let workspace = metadata.workspaces.get_mut("/workspace/東京").unwrap(); + let remotes = workspace.associated_remote_urls.as_mut().unwrap(); + remotes.insert( + "large".into(), + SanitizedGitUrl::try_from(format!("https://example.invalid/{}", "x".repeat(5000)).as_str()) + .unwrap(), + ); + remotes.insert( + "upstream".into(), + SanitizedGitUrl::try_from("git@example.invalid:upstream.git").unwrap(), + ); + metadata.extra.insert("custom".into(), "retained".into()); + metadata.tool_namespaces_info = Some(BTreeMap::from([( + "shell".into(), + TurnToolNamespaceInfo { + name: "shell".into(), + functions: BTreeMap::new(), + }, + )])); + let body = metadata.client_metadata(/*include_internal*/ true); + let canonical = &body[X_CODEX_TURN_METADATA_HEADER]; + assert!(canonical.len() <= MAX_TURN_METADATA_BYTES); + assert!(canonical.is_ascii()); + let actual: Value = serde_json::from_str(canonical).unwrap(); + let mut expected = metadata.turn_metadata_value().unwrap(); + expected["workspaces"]["/workspace/東京"]["associated_remote_urls"] + .as_object_mut() + .unwrap() + .remove("large"); + assert_eq!(actual, expected); + let header = metadata.compatibility_headers(); + let header: Value = + serde_json::from_str(header[X_CODEX_TURN_METADATA_HEADER].to_str().unwrap()).unwrap(); + assert_eq!(header["workspaces"], expected["workspaces"]); + assert!(header.get("tool_namespaces_info").is_none()); +} diff --git a/codex-rs/git-utils/Cargo.toml b/codex-rs/git-utils/Cargo.toml index c9409d41e0..cacddbc3ac 100644 --- a/codex-rs/git-utils/Cargo.toml +++ b/codex-rs/git-utils/Cargo.toml @@ -26,7 +26,7 @@ serde = { workspace = true, features = ["derive"] } similar = { workspace = true } tempfile = { workspace = true } thiserror = { workspace = true } -tokio = { workspace = true, features = ["macros", "process", "rt", "time"] } +tokio = { workspace = true, features = ["macros", "process", "rt", "sync", "time"] } ts-rs = { workspace = true, features = [ "uuid-impl", "serde-json-impl", diff --git a/codex-rs/git-utils/src/info.rs b/codex-rs/git-utils/src/info.rs index 3465016aa9..875677449c 100644 --- a/codex-rs/git-utils/src/info.rs +++ b/codex-rs/git-utils/src/info.rs @@ -1,4 +1,9 @@ -use std::collections::BTreeMap; +mod remotes; + +pub use remotes::get_git_remote_urls; +pub use remotes::get_git_remote_urls_assume_git_repo; +pub use remotes::observe_git_remote_urls; + use std::collections::HashSet; use std::path::Path; use std::path::PathBuf; @@ -122,32 +127,6 @@ pub async fn collect_git_info(cwd: &Path) -> Option { Some(git_info) } -/// Collect fetch remotes in a multi-root-friendly format: {"origin": "https://..."}. -pub async fn get_git_remote_urls(cwd: &Path) -> Option> { - let is_git_repo = run_git_command_with_timeout(&["rev-parse", "--git-dir"], cwd) - .await? - .status - .success(); - if !is_git_repo { - return None; - } - - get_git_remote_urls_assume_git_repo(cwd).await -} - -/// Collect fetch remotes without checking whether `cwd` is in a git repo. -pub async fn get_git_remote_urls_assume_git_repo( - cwd: &Path, -) -> Option> { - let output = run_git_command_with_timeout(&["remote", "-v"], cwd).await?; - if !output.status.success() { - return None; - } - - let stdout = String::from_utf8(output.stdout).ok()?; - parse_git_remote_urls(stdout.as_str()) -} - /// Return the current HEAD commit hash without checking whether `cwd` is in a git repo. pub async fn get_head_commit_hash(cwd: &Path) -> Option { let output = run_git_command_with_timeout(&["rev-parse", "HEAD"], cwd).await?; @@ -266,35 +245,6 @@ fn trim_git_suffix(value: &str) -> &str { value.strip_suffix(".git").unwrap_or(value) } -fn parse_git_remote_urls(stdout: &str) -> Option> { - let mut remotes = BTreeMap::new(); - for line in stdout.lines() { - let Some(fetch_line) = line.strip_suffix(" (fetch)") else { - continue; - }; - - let Some((name, url_part)) = fetch_line - .split_once('\t') - .or_else(|| fetch_line.split_once(' ')) - else { - continue; - }; - - let url = url_part.trim_start(); - if !url.is_empty() - && let Ok(url) = SanitizedGitUrl::try_from(url) - { - remotes.insert(name.to_string(), url); - } - } - - if remotes.is_empty() { - None - } else { - Some(remotes) - } -} - /// A minimal commit summary entry used for pickers (subject + timestamp + sha). #[derive(Clone, Debug, Serialize, Deserialize)] pub struct CommitLogEntry { @@ -417,6 +367,16 @@ pub(crate) async fn run_git_command_with_timeout_from( cwd: &Path, fsmonitor: crate::FsmonitorOverride, ) -> Option { + let mut command = local_git_command(git, args, cwd, fsmonitor); + run_git_command_with_timeout_output(&mut command, GIT_COMMAND_TIMEOUT).await +} + +pub(crate) fn local_git_command( + git: &Path, + args: &[&str], + cwd: &Path, + fsmonitor: crate::FsmonitorOverride, +) -> Command { let mut command = Command::new(git); command .env("GIT_OPTIONAL_LOCKS", "0") @@ -427,7 +387,7 @@ pub(crate) async fn run_git_command_with_timeout_from( .args(["-c", fsmonitor.git_config_arg()]) .args(args) .current_dir(cwd); - run_git_command_with_timeout_output(&mut command, GIT_COMMAND_TIMEOUT).await + command } async fn get_git_remotes(cwd: &Path) -> Option> { @@ -847,6 +807,7 @@ pub async fn current_branch_name(cwd: &Path) -> Option { mod tests { use super::*; use pretty_assertions::assert_eq; + use std::collections::BTreeMap; use std::process::Stdio; #[tokio::test] @@ -938,50 +899,6 @@ mod tests { } } - /// Fetch remotes must be sanitized before they enter workspace metadata. - #[test] - fn parse_git_remote_urls_sanitizes_fetch_credentials() { - let remotes = parse_git_remote_urls( - "origin\thttps://alice:secret@github.com/org/repo.git (fetch)\n\ - origin\thttps://alice:secret@github.com/org/repo.git (push)\n\ - upstream\tgit@github.com:org/upstream.git (fetch)\n", - ); - - assert_eq!( - remotes, - Some(BTreeMap::from([ - ( - "origin".to_string(), - SanitizedGitUrl::try_from("https://github.com/org/repo.git") - .expect("parse expected remote URL"), - ), - ( - "upstream".to_string(), - SanitizedGitUrl::try_from("git@github.com:org/upstream.git") - .expect("parse expected remote URL"), - ), - ])) - ); - } - - /// Malformed remote entries are omitted instead of leaking their raw contents. - #[test] - fn parse_git_remote_urls_skips_malformed_entries() { - let remotes = parse_git_remote_urls( - "bad\thttps://alice:secret@[invalid (fetch)\n\ - origin\thttps://github.com/org/repo.git (fetch)\n", - ); - - assert_eq!( - remotes, - Some(BTreeMap::from([( - "origin".to_string(), - SanitizedGitUrl::try_from("https://github.com/org/repo.git") - .expect("parse expected remote URL"), - )])) - ); - } - /// Both metadata collection paths must sanitize credentials introduced by `insteadOf`. #[tokio::test] async fn git_metadata_collection_sanitizes_rewritten_remote_credentials() { diff --git a/codex-rs/git-utils/src/info/remotes.rs b/codex-rs/git-utils/src/info/remotes.rs new file mode 100644 index 0000000000..144a3a3396 --- /dev/null +++ b/codex-rs/git-utils/src/info/remotes.rs @@ -0,0 +1,207 @@ +//! Collects sanitized fetch URLs from NUL-delimited Git configuration without contacting remotes. +//! Git resolves rewritten or reset URLs within the same collection deadline. + +use std::collections::BTreeMap; +use std::path::Path; + +use futures::FutureExt; +use futures::StreamExt; +use futures::stream; + +use super::DISABLED_HOOKS_PATH; +use super::GIT_COMMAND_TIMEOUT; +use super::local_git_command; +use super::run_git_command_with_timeout; +use crate::SanitizedGitUrl; +use crate::git_process::run_git_command_with_timeout_output; + +const MAX_CONCURRENT_REMOTE_LOOKUPS: usize = 8; + +/// Collect fetch remotes in a multi-root-friendly format: {"origin": "https://..."}. +pub async fn get_git_remote_urls(cwd: &Path) -> Option> { + let is_git_repo = run_git_command_with_timeout(&["rev-parse", "--git-dir"], cwd) + .await? + .status + .success(); + if !is_git_repo { + return None; + } + + get_git_remote_urls_assume_git_repo(cwd).await +} + +/// Collect fetch remotes without checking whether `cwd` is in a git repo. +pub async fn get_git_remote_urls_assume_git_repo( + cwd: &Path, +) -> Option> { + get_git_remote_urls_with_progress(cwd, |_| {}).await +} + +/// Publishes verified fetch URLs as they become available, keeping previously resolved names. +pub async fn observe_git_remote_urls( + cwd: &Path, + updates: &tokio::sync::watch::Sender>, +) { + get_git_remote_urls_with_progress(cwd, |urls| { + updates.send_modify(|known| known.extend(urls.clone())); + }) + .await; +} + +/// Publishes unambiguous configured URLs, then each successful native resolution. +async fn get_git_remote_urls_with_progress( + cwd: &Path, + mut publish_verified: impl FnMut(&BTreeMap), +) -> Option> { + let deadline = tokio::time::Instant::now() + GIT_COMMAND_TIMEOUT; + // `git remote` ignores GIT_CONFIG, but `git config` treats it as an explicit + // file. Clear it so the machine-readable query observes the same configuration. + let mut command = local_git_command( + Path::new("git"), + &["config", "--null", "--get-regexp", r"^(remote\.|url\.)"], + cwd, + crate::FsmonitorOverride::Disabled, + ); + command.env_remove("GIT_CONFIG"); + let config = run_git_command_with_timeout_output(&mut command, GIT_COMMAND_TIMEOUT).await?; + if !config.status.success() { + return None; + } + let mut configured = BTreeMap::<&str, Vec<&[u8]>>::new(); + let mut rewrite_prefixes = Vec::new(); + for entry in config.stdout.split(|byte| *byte == b'\0') { + if entry.is_empty() { + continue; + } + let separator = entry + .iter() + .position(|byte| *byte == b'\n') + .unwrap_or(entry.len()); + let (key, value) = entry.split_at(separator); + if let Some(name) = key + .strip_prefix(b"remote.") + .and_then(|key| key.strip_suffix(b".url")) + .and_then(|name| std::str::from_utf8(name).ok()) + { + // Git interprets an empty remote subsection as the name `.url`. + let name = if name.is_empty() { ".url" } else { name }; + configured + .entry(name) + .or_default() + .push(value.strip_prefix(b"\n")?); + } else if key + .strip_prefix(b"url.") + .and_then(|key| key.strip_suffix(b".insteadof")) + .is_some() + { + rewrite_prefixes.push(value.strip_prefix(b"\n")?); + } + } + let mut remotes = BTreeMap::new(); + let mut pending = BTreeMap::new(); + for (name, values) in configured { + let first = *values.first()?; + let has_empty = values.iter().any(|value| value.is_empty()); + let rewritten = rewrite_prefixes + .iter() + .any(|prefix| first.starts_with(prefix)); + if has_empty || rewritten { + pending.insert( + name.to_owned(), + values.last().is_some_and(|value| value.is_empty()), + ); + continue; + } + if let Ok(raw) = std::str::from_utf8(first) + && !raw.contains(['\n', '\r']) + && !raw.trim().is_empty() + && let Ok(url) = SanitizedGitUrl::try_from(raw.trim_start()) + { + remotes.insert(name.to_owned(), url); + } + } + if !remotes.is_empty() { + publish_verified(&remotes); + } + // Let Git implement insteadOf precedence and version-dependent URL resets. + let origin = pending.remove_entry("origin"); + let (known, ambiguous): (Vec<_>, Vec<_>) = pending.into_iter().partition(|(_, empty)| !empty); + let resets = git_resets_empty_fetch_url(cwd).shared(); + let resolved = stream::iter(origin.into_iter().chain(known).chain(ambiguous)) + .map(|(name, last_empty)| { + let resets = resets.clone(); + async move { + // A trailing empty URL clears the list on newer Git. Older Git + // resolves its first configured URL, including insteadOf rewrites. + if last_empty && resets.await? { + return None; + } + // --get-url expands the configured fetch URL locally. + let output = + run_git_command_with_timeout(&["ls-remote", "--get-url", "--", &name], cwd) + .await?; + if !output.status.success() { + return None; + } + let stdout = String::from_utf8(output.stdout).ok()?; + let mut lines = stdout.lines(); + let url = lines.next()?; + if lines.next().is_some() || url.contains('\r') || url.trim().is_empty() { + return None; + } + let url = SanitizedGitUrl::try_from(url.trim_start()).ok()?; + Some((name, url)) + } + }) + .buffer_unordered(MAX_CONCURRENT_REMOTE_LOOKUPS) + .take_until(tokio::time::sleep_until(deadline)) + .filter_map(std::future::ready); + let mut resolved = std::pin::pin!(resolved); + while let Some(remote) = resolved.next().await { + let update = BTreeMap::from([remote]); + publish_verified(&update); + remotes.extend(update); + } + (!remotes.is_empty()).then_some(remotes) +} + +pub(crate) async fn git_resets_empty_fetch_url(cwd: &Path) -> Option { + let null_device = DISABLED_HOOKS_PATH; + let mut command = local_git_command( + Path::new("git"), + &[ + "--git-dir", + null_device, + "-c", + "remote.codex-reset-probe.url=before-reset", + "-c", + "remote.codex-reset-probe.url=", + "ls-remote", + "--get-url", + "--", + "codex-reset-probe", + ], + cwd, + crate::FsmonitorOverride::Disabled, + ); + command + .env("GIT_CONFIG_GLOBAL", null_device) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_COUNT", "0") + .env_remove("GIT_CONFIG_PARAMETERS") + .env_remove("GIT_COMMON_DIR") + .env_remove("GIT_WORK_TREE"); + let output = run_git_command_with_timeout_output(&mut command, GIT_COMMAND_TIMEOUT).await?; + if !output.status.success() { + return None; + } + match std::str::from_utf8(&output.stdout).ok()?.trim_end() { + "before-reset" => Some(false), + "codex-reset-probe" => Some(true), + _ => None, + } +} + +#[cfg(test)] +#[path = "remotes_tests.rs"] +mod tests; diff --git a/codex-rs/git-utils/src/info/remotes_tests.rs b/codex-rs/git-utils/src/info/remotes_tests.rs new file mode 100644 index 0000000000..7da1fd7607 --- /dev/null +++ b/codex-rs/git-utils/src/info/remotes_tests.rs @@ -0,0 +1,275 @@ +#![cfg(test)] + +use super::*; +use pretty_assertions::assert_eq; +use std::io::Write; +use tokio::process::Command; + +#[tokio::test] +async fn git_remote_urls_keep_filtered_fetch_remotes() { + const CHILD_ENV: &str = "CODEX_GIT_UTILS_FILTERED_REMOTES_CHILD"; + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let root = temp_dir.path(); + if std::env::var_os(CHILD_ENV).is_none() { + let status = Command::new(std::env::current_exe().expect("find test executable")) + .args([ + "--exact", + "info::remotes::tests::git_remote_urls_keep_filtered_fetch_remotes", + "--nocapture", + ]) + .env(CHILD_ENV, "1") + .env("GIT_CONFIG_GLOBAL", root.join("empty.gitconfig")) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env_remove("GIT_CONFIG") + .env_remove("GIT_CONFIG_COUNT") + .env_remove("GIT_CONFIG_PARAMETERS") + .status() + .await + .expect("run test with isolated Git configuration"); + assert!(status.success()); + return; + } + let git = |args: &[&str]| { + crate::operations::run_git_for_status(root, args, /*env*/ None) + .expect("configure test repository") + }; + git(&["init", "--bare", "source"]); + git(&[ + "--git-dir=source", + "config", + "uploadpack.allowFilter", + "true", + ]); + git(&[ + "clone", + "--no-local", + "--filter=blob:none", + "source", + "partial", + ]); + let repo = root.join("partial"); + let config = |key: &str, value: &str| git(&["-C", "partial", "config", key, value]); + let add_config = + |key: &str, value: &str| git(&["-C", "partial", "config", "--add", key, value]); + let native_fetch_url = |name: &str| { + let url = crate::operations::run_git_for_stdout( + &repo, + ["ls-remote", "--get-url", "--", name], + /*env*/ None, + ) + .expect("read Git's effective fetch URL"); + SanitizedGitUrl::try_from(url.as_str()).expect("parse native fetch URL") + }; + config( + "url.https://alice:secret@example.invalid/.insteadOf", + "short:", + ); + config("url.https://unused.invalid/.insteadOf", "short"); + config("remote.origin.url", "short:repo.git"); + add_config("remote.origin.url", "https://other.invalid/unused"); + config("remote.origin.pushurl", "https://push.invalid/other.git"); + config("remote.pushonly.pushurl", "https://push.invalid/only.git"); + config("remote.upstream.url", "git@github.com:org/upstream.git"); + config("remote.bad.url", "https://alice:secret@[invalid"); + config("remote.reset.url", "https://old.invalid/repo"); + add_config("remote.reset.url", ""); + config("remote.middle-reset.url", "https://old.invalid/before"); + add_config("remote.middle-reset.url", ""); + add_config("remote.middle-reset.url", "https://new.invalid/after"); + config("remote.literal.url", "literal"); + config("remote.rewritten.url", "alias:repo"); + config("url.rewritten.insteadOf", "alias:repo"); + config("url.poisoned.insteadOf", "codex-reset-probe"); + config("remote.empty=fetch.url", ""); + config("remote.local.url", "/a repo (fetch) [archive]"); + config( + "remote.multiline.url", + "https://other.invalid/first\nsecond", + ); + let mut expected = BTreeMap::from([ + ( + "origin".to_string(), + SanitizedGitUrl::try_from("https://example.invalid/repo.git") + .expect("parse expected remote URL"), + ), + ( + "local".to_string(), + SanitizedGitUrl::try_from("/a repo (fetch) [archive]") + .expect("parse expected remote path"), + ), + ( + "literal".to_string(), + SanitizedGitUrl::try_from("literal").expect("parse expected local URL"), + ), + ( + "rewritten".to_string(), + SanitizedGitUrl::try_from("rewritten").expect("parse expected rewritten URL"), + ), + ( + "upstream".to_string(), + SanitizedGitUrl::try_from("git@github.com:org/upstream.git") + .expect("parse expected upstream URL"), + ), + ("middle-reset".to_owned(), native_fetch_url("middle-reset")), + ]); + // Older Git retains the first URL when followed by an empty entry. + let verbose = || { + crate::operations::run_git_for_stdout(&repo, ["remote", "-v"], /*env*/ None) + .expect("read installed Git's fetch remotes") + }; + if verbose() + .lines() + .any(|line| line == "reset\thttps://old.invalid/repo (fetch)") + { + expected.insert( + "reset".to_string(), + SanitizedGitUrl::try_from("https://old.invalid/repo").expect("parse reset URL"), + ); + } + // Equally specific rewrite rules leave Git to choose the effective destination. + config("url.https://first.invalid/.insteadOf", "tied:"); + config("url.https://second.invalid/.insteadOf", "tied:"); + let fallback_names = ["first-fallback", "second-fallback"]; + for name in fallback_names { + config(&format!("remote.{name}.url"), &format!("tied:{name}")); + expected.insert(name.to_owned(), native_fetch_url(name)); + } + let append_raw_config = |bytes: &[u8]| { + std::fs::OpenOptions::new() + .append(true) + .open(repo.join(".git/config")) + .expect("open repository config") + .write_all(bytes) + .expect("append non-UTF-8 config"); + }; + // Unrelated non-UTF-8 configuration must not discard valid fetch URLs. + append_raw_config( + b"\n[url \"https://unused-invalid-utf8.invalid/\"]\n\tinsteadOf = \xff\n\n[remote \"unused\"]\n\tpromisor\n\n[remote \"bad\xff\"]\n\turl = https://invalid-name.invalid/repo\n", + ); + let mut updates = Vec::new(); + let complete = + get_git_remote_urls_with_progress(&repo, |urls| updates.push(urls.clone())).await; + assert_eq!(complete.as_ref(), Some(&expected)); + let verified = updates.first().expect("publish unambiguous fetch URLs"); + assert_eq!(verified.get("upstream"), expected.get("upstream")); + assert!(!verified.contains_key("origin")); + assert!(!verified.contains_key("reset")); + assert!(!verified.contains_key("middle-reset")); + assert!( + verified + .iter() + .all(|(name, url)| expected.get(name) == Some(url)) + ); + for name in ["origin", "rewritten"].into_iter().chain(fallback_names) { + assert!(!verified.contains_key(name)); + assert!( + updates + .iter() + .any(|urls| urls.len() == 1 && urls.get(name) == expected.get(name)) + ); + } + assert_eq!( + updates.into_iter().flatten().collect::>(), + expected + ); + config( + "remote.origin.partialclonefilter", + "blob:none]\nforged\thttps://other.invalid/repo (fetch)\nforged\thttps://other.invalid/repo (push)\n[tree:0", + ); + let spawned_repo = repo.clone(); + assert_eq!( + tokio::spawn(async move { get_git_remote_urls_assume_git_repo(&spawned_repo).await }) + .await + .expect("join remote lookup"), + Some(expected.clone()) + ); + // Every legal remote, including a long URL, is retained without a count cap. + let long_url = format!("https://example.invalid/{}", "p".repeat(5000)); + for index in 0..20 { + let name = format!("a{index:02}"); + config(&format!("remote.{name}.url"), &long_url); + expected.insert(name, SanitizedGitUrl::try_from(long_url.as_str()).unwrap()); + } + assert_eq!( + get_git_remote_urls_assume_git_repo(&repo).await, + Some(expected.clone()) + ); + // A rewrite can split a UTF-8 character. Filter text must not forge its URL. + config("remote.a-victim.url", "érepo"); + append_raw_config(b"\n[url \"https://effective.invalid/\xc3\"]\n\tinsteadOf = \xc3\n"); + config( + "remote.origin.partialclonefilter", + "blob:none]\na-victim\térepo (fetch)\n[tree:0", + ); + expected.insert("a-victim".to_owned(), native_fetch_url("a-victim")); + assert_eq!( + get_git_remote_urls_assume_git_repo(&repo).await, + Some(expected) + ); + config("url.https://default.invalid/.insteadOf", ""); + let native = Command::new("git") + .args(["remote", "-v"]) + .current_dir(&repo) + .output() + .await + .expect("read installed Git's fetch remotes"); + assert!(native.status.success()); + let native_has_empty = native + .stdout + .split(|byte| *byte == b'\n') + .any(|line| line.starts_with(b"empty=fetch\t") && line.ends_with(b" (fetch)")); + let collected = get_git_remote_urls_assume_git_repo(&repo) + .await + .expect("collect rewritten remotes"); + assert_eq!(collected.contains_key("empty=fetch"), native_has_empty); +} + +#[tokio::test] +async fn git_remote_urls_ignore_git_config_file_override() { + const CHILD_REPO: &str = "CODEX_GIT_UTILS_REMOTE_CONFIG_CHILD_REPO"; + if let Some(repo) = std::env::var_os(CHILD_REPO) { + assert_eq!( + get_git_remote_urls_assume_git_repo(Path::new(&repo)).await, + Some(BTreeMap::from([( + "origin".to_string(), + SanitizedGitUrl::try_from("https://repository.invalid/repo.git") + .expect("parse repository remote"), + )])) + ); + return; + } + + let temp_dir = tempfile::tempdir().expect("create temp dir"); + let root = temp_dir.path(); + crate::operations::run_git_for_status(root, ["init", "repo"], /*env*/ None) + .expect("initialize test repository"); + let repo = root.join("repo"); + std::fs::OpenOptions::new() + .append(true) + .open(repo.join(".git/config")) + .expect("open repository config") + .write_all(b"\n[remote \"origin\"]\n\turl = https://repository.invalid/repo.git\n") + .expect("configure repository remote"); + let foreign = root.join("foreign.gitconfig"); + std::fs::write( + &foreign, + b"[remote \"foreign\"]\n\turl = https://foreign.invalid/repo.git\n", + ) + .expect("write foreign config"); + let status = Command::new(std::env::current_exe().expect("find test executable")) + .args([ + "--exact", + "info::remotes::tests::git_remote_urls_ignore_git_config_file_override", + "--nocapture", + ]) + .env(CHILD_REPO, &repo) + .env("GIT_CONFIG", &foreign) + .env("GIT_CONFIG_GLOBAL", root.join("empty.gitconfig")) + .env("GIT_CONFIG_NOSYSTEM", "1") + .env_remove("GIT_CONFIG_COUNT") + .env_remove("GIT_CONFIG_PARAMETERS") + .status() + .await + .expect("run test with foreign config override"); + assert!(status.success()); +} diff --git a/codex-rs/git-utils/src/lib.rs b/codex-rs/git-utils/src/lib.rs index 1224d2ab03..1beb1f7e5c 100644 --- a/codex-rs/git-utils/src/lib.rs +++ b/codex-rs/git-utils/src/lib.rs @@ -48,6 +48,7 @@ pub use info::get_git_repo_root; pub use info::get_head_commit_hash; pub use info::git_diff_to_remote; pub use info::local_git_branches; +pub use info::observe_git_remote_urls; pub use info::recent_commits; pub use operations::git_config_override_env; pub use platform::create_symlink;