Merge local-only Git transport base

This commit is contained in:
Chris Bookholt
2026-06-30 11:59:24 -07:00
16 changed files with 1044 additions and 30 deletions
+2 -1
View File
@@ -1090,7 +1090,7 @@ Notes:
- Empty `command` arrays are rejected.
- Prefer `permissionProfile` for command permission overrides. It selects an active profile by id (for example `:read-only`, `:workspace`, or a user-defined `[permissions.<id>]` profile) rather than accepting low-level filesystem/network permissions. The legacy `sandboxPolicy` field accepts the same shape used by `turn/start` (e.g., `dangerFullAccess`, `readOnly`, `workspaceWrite` with flags, `externalSandbox` with `networkAccess` `restricted|enabled`), but cannot be combined with `permissionProfile`.
- `env` merges into the environment produced by the server's shell environment policy. Matching names are overridden; unspecified variables are left intact.
- `env` merges into the environment produced by the server's shell environment policy. Matching names are overridden; unspecified variables are left intact. On Windows, ASCII names are matched case-insensitively, and requests containing multiple ASCII-case-equivalent names are rejected.
- When omitted, `timeoutMs` falls back to the server default.
- When omitted, `outputBytesCap` falls back to the server default of 1 MiB per stream.
- `disableOutputCap: true` disables stdout/stderr capture truncation for that `command/exec` request. It cannot be combined with `outputBytesCap`.
@@ -1225,6 +1225,7 @@ For interactive or streaming processes, set `tty: true` or `streamStdoutStderr:
- Empty `command` arrays and empty `processHandle` strings are rejected.
- `cwd` is required and must be absolute.
- `process/spawn` is intentionally unsandboxed and does not define sandbox-selection fields such as `sandboxPolicy` or `permissionProfile`.
- `env` overrides and unsets inherited variables by exact name on Unix. On Windows, ASCII names are matched case-insensitively, and requests containing multiple ASCII-case-equivalent names are rejected.
- Duplicate active `processHandle` values are rejected on the same connection; the same handle can be reused after the prior process exits.
- `tty: true` implies PTY mode plus `streamStdin: true` and `streamStdoutStderr: true`.
- `process/writeStdin` accepts either `deltaBase64`, `closeStdin`, or both.
@@ -493,6 +493,7 @@ mod apps_processor;
mod catalog_processor;
mod command_exec_processor;
mod config_processor;
mod environment_overrides;
mod environment_processor;
mod external_agent_config_processor;
mod external_agent_session_import;
@@ -1,3 +1,4 @@
use super::environment_overrides::apply_environment_overrides;
use super::*;
#[derive(Clone)]
@@ -152,16 +153,8 @@ impl CommandExecRequestProcessor {
/*thread_id*/ None,
);
if let Some(env_overrides) = env_overrides {
for (key, value) in env_overrides {
match value {
Some(value) => {
env.insert(key, value);
}
None => {
env.remove(&key);
}
}
}
apply_environment_overrides(&mut env, env_overrides)
.map_err(|error| invalid_params(format!("command/exec env: {error}")))?;
}
let timeout_ms = match timeout_ms {
Some(timeout_ms) => match u64::try_from(timeout_ms) {
@@ -0,0 +1,98 @@
use std::collections::HashMap;
use std::fmt;
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
enum EnvironmentNameSemantics {
CaseSensitive,
WindowsAsciiCaseInsensitive,
}
impl EnvironmentNameSemantics {
const fn for_target() -> Self {
if cfg!(windows) {
Self::WindowsAsciiCaseInsensitive
} else {
Self::CaseSensitive
}
}
}
#[derive(Clone, Debug, Eq, PartialEq)]
pub(super) struct EnvironmentOverrideError {
first_key: String,
second_key: String,
}
impl fmt::Display for EnvironmentOverrideError {
fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(
formatter,
"environment override keys '{}' and '{}' are equivalent on Windows",
self.first_key, self.second_key
)
}
}
pub(super) fn apply_environment_overrides(
env: &mut HashMap<String, String>,
overrides: HashMap<String, Option<String>>,
) -> Result<(), EnvironmentOverrideError> {
apply_environment_overrides_with_semantics(
env,
overrides,
EnvironmentNameSemantics::for_target(),
)
}
fn apply_environment_overrides_with_semantics(
env: &mut HashMap<String, String>,
overrides: HashMap<String, Option<String>>,
semantics: EnvironmentNameSemantics,
) -> Result<(), EnvironmentOverrideError> {
match semantics {
EnvironmentNameSemantics::CaseSensitive => {
for (key, value) in overrides {
match value {
Some(value) => {
env.insert(key, value);
}
None => {
env.remove(&key);
}
}
}
}
EnvironmentNameSemantics::WindowsAsciiCaseInsensitive => {
validate_windows_override_keys(&overrides)?;
for (key, value) in overrides {
env.retain(|existing_key, _| !existing_key.eq_ignore_ascii_case(&key));
if let Some(value) = value {
env.insert(key, value);
}
}
}
}
Ok(())
}
fn validate_windows_override_keys(
overrides: &HashMap<String, Option<String>>,
) -> Result<(), EnvironmentOverrideError> {
let mut keys = overrides.keys().collect::<Vec<_>>();
keys.sort_unstable();
for (index, first_key) in keys.iter().enumerate() {
for second_key in &keys[index + 1..] {
if first_key.eq_ignore_ascii_case(second_key) {
return Err(EnvironmentOverrideError {
first_key: (*first_key).clone(),
second_key: (*second_key).clone(),
});
}
}
}
Ok(())
}
#[cfg(test)]
#[path = "environment_overrides_tests.rs"]
mod tests;
@@ -0,0 +1,180 @@
use super::*;
use pretty_assertions::assert_eq;
fn string_map<const N: usize>(entries: [(&str, &str); N]) -> HashMap<String, String> {
entries
.into_iter()
.map(|(key, value)| (key.to_string(), value.to_string()))
.collect()
}
fn override_map<const N: usize>(
entries: [(&str, Option<&str>); N],
) -> HashMap<String, Option<String>> {
entries
.into_iter()
.map(|(key, value)| (key.to_string(), value.map(std::string::ToString::to_string)))
.collect()
}
fn apply_legacy_case_sensitive_overrides(
env: &mut HashMap<String, String>,
overrides: HashMap<String, Option<String>>,
) {
for (key, value) in overrides {
match value {
Some(value) => {
env.insert(key, value);
}
None => {
env.remove(&key);
}
}
}
}
#[test]
fn windows_present_override_replaces_every_case_variant() {
let mut env = string_map([
("git_allow_protocol", "ext"),
("Git_Allow_Protocol", "file"),
("OTHER", "keep"),
]);
apply_environment_overrides_with_semantics(
&mut env,
override_map([("GIT_ALLOW_PROTOCOL", Some(""))]),
EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
)
.expect("Windows override should be unambiguous");
assert_eq!(
env,
string_map([("GIT_ALLOW_PROTOCOL", ""), ("OTHER", "keep")])
);
}
#[test]
fn windows_unset_removes_every_case_variant() {
let mut env = string_map([("Path", "first"), ("PATH", "second"), ("OTHER", "keep")]);
apply_environment_overrides_with_semantics(
&mut env,
override_map([("path", None)]),
EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
)
.expect("Windows unset should be unambiguous");
assert_eq!(env, string_map([("OTHER", "keep")]));
}
#[test]
fn windows_local_only_overrides_replace_both_permissive_controls() {
let mut env = string_map([
("git_allow_protocol", "ext"),
("git_no_lazy_fetch", "0"),
("OTHER", "keep"),
]);
apply_environment_overrides_with_semantics(
&mut env,
override_map([
("GIT_ALLOW_PROTOCOL", Some("")),
("GIT_NO_LAZY_FETCH", Some("1")),
]),
EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
)
.expect("local-only overrides should be unambiguous");
assert_eq!(
env,
string_map([
("GIT_ALLOW_PROTOCOL", ""),
("GIT_NO_LAZY_FETCH", "1"),
("OTHER", "keep"),
])
);
}
#[test]
fn windows_rejects_equivalent_request_keys_before_mutation() {
let initial = string_map([("Path", "old"), ("OTHER", "keep")]);
let mut env = initial.clone();
let error = apply_environment_overrides_with_semantics(
&mut env,
override_map([("PATH", Some("first")), ("Path", Some("second"))]),
EnvironmentNameSemantics::WindowsAsciiCaseInsensitive,
)
.expect_err("equivalent Windows request keys should be rejected");
assert_eq!(
error,
EnvironmentOverrideError {
first_key: "PATH".to_string(),
second_key: "Path".to_string(),
}
);
assert_eq!(env, initial);
}
#[test]
fn case_sensitive_mode_preserves_differently_cased_names() {
let mut env = string_map([("Path", "old"), ("PATH", "replace"), ("OTHER", "keep")]);
apply_environment_overrides_with_semantics(
&mut env,
override_map([("PATH", None), ("path", Some("new"))]),
EnvironmentNameSemantics::CaseSensitive,
)
.expect("case-sensitive overrides should not conflict");
assert_eq!(
env,
string_map([("Path", "old"), ("path", "new"), ("OTHER", "keep")])
);
}
#[test]
fn case_sensitive_mode_matches_legacy_merge_behavior() {
let fixtures = [
(
string_map([("Path", "old"), ("PATH", "replace"), ("OTHER", "keep")]),
override_map([("PATH", None), ("path", Some("new"))]),
),
(
string_map([("FOO", "old"), ("BAR", "keep"), ("EMPTY", "old")]),
override_map([
("FOO", Some("new")),
("ADDED", Some("value")),
("EMPTY", None),
]),
),
];
for (initial, overrides) in fixtures {
let mut expected = initial.clone();
apply_legacy_case_sensitive_overrides(&mut expected, overrides.clone());
let mut actual = initial;
apply_environment_overrides_with_semantics(
&mut actual,
overrides,
EnvironmentNameSemantics::CaseSensitive,
)
.expect("case-sensitive overrides should not conflict");
assert_eq!(actual, expected);
}
}
#[test]
fn target_semantics_matches_platform() {
let expected = if cfg!(windows) {
EnvironmentNameSemantics::WindowsAsciiCaseInsensitive
} else {
EnvironmentNameSemantics::CaseSensitive
};
assert_eq!(EnvironmentNameSemantics::for_target(), expected);
}
@@ -42,6 +42,7 @@ use crate::error_code::invalid_request;
use crate::outgoing_message::ConnectionId;
use crate::outgoing_message::ConnectionRequestId;
use crate::outgoing_message::OutgoingMessageSender;
use crate::request_processors::environment_overrides::apply_environment_overrides;
const EXEC_TIMEOUT_EXIT_CODE: i32 = 124;
const OUTPUT_CHUNK_SIZE_HINT: usize = 64 * 1024;
@@ -96,16 +97,8 @@ impl ProcessExecRequestProcessor {
}
let mut env = std::env::vars().collect::<HashMap<_, _>>();
if let Some(env_overrides) = env_overrides {
for (key, value) in env_overrides {
match value {
Some(value) => {
env.insert(key, value);
}
None => {
env.remove(&key);
}
}
}
apply_environment_overrides(&mut env, env_overrides)
.map_err(|error| invalid_params(format!("{method_name} env: {error}")))?;
}
let expiration = match timeout_ms {
Some(Some(timeout_ms)) => match u64::try_from(timeout_ms) {
@@ -37,6 +37,17 @@ use super::connection_handling_websocket::send_initialize_request;
use super::connection_handling_websocket::send_request;
use super::connection_handling_websocket::spawn_websocket_server;
#[cfg(windows)]
fn require_absent_windows_environment(names: &[&str]) -> Result<()> {
for name in names {
anyhow::ensure!(
std::env::var_os(name).is_none(),
"cannot establish lowercase Windows environment fixture because {name} is already set"
);
}
Ok(())
}
#[tokio::test]
async fn command_exec_without_streams_can_be_terminated() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
@@ -193,6 +204,268 @@ async fn command_exec_env_overrides_merge_with_server_environment_and_support_un
Ok(())
}
#[cfg(windows)]
#[tokio::test]
async fn command_exec_env_overrides_use_windows_case_folding() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
require_absent_windows_environment(&[
"GIT_ALLOW_PROTOCOL",
"GIT_NO_LAZY_FETCH",
"CASEFOLD_REMOVE_ME",
])?;
let mut mcp = TestAppServer::new_with_env(
codex_home.path(),
&[
("git_allow_protocol", Some("ext")),
("git_no_lazy_fetch", Some("0")),
("casefold_remove_me", Some("inherited")),
],
)
.await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let system_root = std::env::var_os("SystemRoot").context("SystemRoot should be set")?;
let powershell = Path::new(&system_root)
.join("System32")
.join("WindowsPowerShell")
.join("v1.0")
.join("powershell.exe");
let inherited_script = r#"
$entries = [System.Environment]::GetEnvironmentVariables('Process')
function Exact-Keys([string] $name) {
@($entries.Keys | Where-Object {
[string]::Equals(
[string] $_,
$name,
[System.StringComparison]::Ordinal
)
})
}
$allowLower = @(Exact-Keys 'git_allow_protocol')
$allowUpper = @(Exact-Keys 'GIT_ALLOW_PROTOCOL')
$noLazyLower = @(Exact-Keys 'git_no_lazy_fetch')
$noLazyUpper = @(Exact-Keys 'GIT_NO_LAZY_FETCH')
$removedLower = @(Exact-Keys 'casefold_remove_me')
$removedUpper = @(Exact-Keys 'CASEFOLD_REMOVE_ME')
if ($allowLower.Count -ne 1 -or [string] $entries[$allowLower[0]] -ne 'ext') { exit 21 }
if ($allowUpper.Count -ne 0) { exit 22 }
if ($noLazyLower.Count -ne 1 -or [string] $entries[$noLazyLower[0]] -ne '0') { exit 23 }
if ($noLazyUpper.Count -ne 0) { exit 24 }
if ($removedLower.Count -ne 1 -or [string] $entries[$removedLower[0]] -ne 'inherited') { exit 25 }
if ($removedUpper.Count -ne 0) { exit 26 }
[Console]::Out.Write('lowercase-inherited')
"#;
let inherited_request_id = mcp
.send_command_exec_request(CommandExecParams {
command: vec![
powershell.display().to_string(),
"-NoProfile".to_string(),
"-NonInteractive".to_string(),
"-Command".to_string(),
inherited_script.to_string(),
],
process_id: None,
tty: false,
stream_stdin: false,
stream_stdout_stderr: false,
output_bytes_cap: None,
disable_output_cap: false,
disable_timeout: false,
timeout_ms: None,
cwd: None,
env: None,
size: None,
sandbox_policy: None,
permission_profile: None,
})
.await?;
let inherited_response = mcp
.read_stream_until_response_message(RequestId::Integer(inherited_request_id))
.await?;
let inherited_response: CommandExecResponse = to_response(inherited_response)?;
assert_eq!(
inherited_response,
CommandExecResponse {
exit_code: 0,
stdout: "lowercase-inherited".to_string(),
stderr: String::new(),
}
);
let overridden_script = r#"
$entries = [System.Environment]::GetEnvironmentVariables('Process')
function Matching-Keys([string] $name) {
@($entries.Keys | Where-Object {
[string]::Equals(
[string] $_,
$name,
[System.StringComparison]::OrdinalIgnoreCase
)
})
}
$allow = @(Matching-Keys 'GIT_ALLOW_PROTOCOL')
$noLazy = @(Matching-Keys 'GIT_NO_LAZY_FETCH')
$removed = @(Matching-Keys 'CASEFOLD_REMOVE_ME')
if ($allow.Count -ne 1 -or [string] $entries[$allow[0]] -ne '') { exit 11 }
if ($noLazy.Count -ne 1 -or [string] $entries[$noLazy[0]] -ne '1') { exit 12 }
if ($removed.Count -ne 0) { exit 13 }
[Console]::Out.Write('casefold-ok')
"#;
let command_request_id = mcp
.send_command_exec_request(CommandExecParams {
command: vec![
powershell.display().to_string(),
"-NoProfile".to_string(),
"-NonInteractive".to_string(),
"-Command".to_string(),
overridden_script.to_string(),
],
process_id: None,
tty: false,
stream_stdin: false,
stream_stdout_stderr: false,
output_bytes_cap: None,
disable_output_cap: false,
disable_timeout: false,
timeout_ms: None,
cwd: None,
env: Some(HashMap::from([
("GIT_ALLOW_PROTOCOL".to_string(), Some(String::new())),
("GIT_NO_LAZY_FETCH".to_string(), Some("1".to_string())),
("CASEFOLD_REMOVE_ME".to_string(), None),
])),
size: None,
sandbox_policy: None,
permission_profile: None,
})
.await?;
let response = mcp
.read_stream_until_response_message(RequestId::Integer(command_request_id))
.await?;
let response: CommandExecResponse = to_response(response)?;
assert_eq!(
response,
CommandExecResponse {
exit_code: 0,
stdout: "casefold-ok".to_string(),
stderr: String::new(),
}
);
Ok(())
}
#[cfg(windows)]
#[tokio::test]
async fn command_exec_empty_git_allowlist_blocks_local_file_transport_on_windows() -> Result<()> {
let repository_root = TempDir::new()?;
let repository = repository_root.path().join("remote.git");
let init = std::process::Command::new("git")
.args(["init", "-q", "--bare"])
.arg(&repository)
.output()
.context("initialize bare Git repository")?;
anyhow::ensure!(
init.status.success(),
"git init failed: {}",
String::from_utf8_lossy(&init.stderr)
);
let repository_url = url::Url::from_file_path(&repository)
.map_err(|()| anyhow::anyhow!("convert repository path to file URL"))?
.to_string();
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
let codex_home = TempDir::new()?;
create_config_toml(codex_home.path(), &server.uri(), "never")?;
require_absent_windows_environment(&["GIT_ALLOW_PROTOCOL", "GIT_NO_LAZY_FETCH"])?;
let mut mcp = TestAppServer::new_with_env(
codex_home.path(),
&[
("git_allow_protocol", Some("file")),
("git_no_lazy_fetch", Some("0")),
],
)
.await?;
timeout(DEFAULT_READ_TIMEOUT, mcp.initialize()).await??;
let control_request_id = mcp
.send_command_exec_request(CommandExecParams {
command: vec![
"git".to_string(),
"ls-remote".to_string(),
repository_url.clone(),
],
process_id: None,
tty: false,
stream_stdin: false,
stream_stdout_stderr: false,
output_bytes_cap: None,
disable_output_cap: false,
disable_timeout: false,
timeout_ms: None,
cwd: None,
env: None,
size: None,
sandbox_policy: None,
permission_profile: None,
})
.await?;
let control_response = mcp
.read_stream_until_response_message(RequestId::Integer(control_request_id))
.await?;
let control_response: CommandExecResponse = to_response(control_response)?;
assert_eq!(
control_response,
CommandExecResponse {
exit_code: 0,
stdout: String::new(),
stderr: String::new(),
},
"inherited lowercase allowlist should permit file:// through command/exec"
);
let command_request_id = mcp
.send_command_exec_request(CommandExecParams {
command: vec!["git".to_string(), "ls-remote".to_string(), repository_url],
process_id: None,
tty: false,
stream_stdin: false,
stream_stdout_stderr: false,
output_bytes_cap: None,
disable_output_cap: false,
disable_timeout: false,
timeout_ms: None,
cwd: None,
env: Some(HashMap::from([
("GIT_ALLOW_PROTOCOL".to_string(), Some(String::new())),
("GIT_NO_LAZY_FETCH".to_string(), Some("1".to_string())),
])),
size: None,
sandbox_policy: None,
permission_profile: None,
})
.await?;
let response = mcp
.read_stream_until_response_message(RequestId::Integer(command_request_id))
.await?;
let response: CommandExecResponse = to_response(response)?;
assert_ne!(response.exit_code, 0, "empty allowlist should deny file://");
assert_eq!(response.stdout, "");
assert!(
response.stderr.contains("transport 'file' not allowed"),
"expected Git transport denial, got: {}",
response.stderr
);
Ok(())
}
#[tokio::test]
async fn command_exec_accepts_permission_profile() -> Result<()> {
let server = create_mock_responses_server_sequence_unchecked(Vec::new()).await;
+13 -3
View File
@@ -124,7 +124,7 @@ pub fn apply_git_patch(req: &ApplyGitRequest) -> io::Result<ApplyGitResult> {
}
fn resolve_git_root(cwd: &Path) -> io::Result<PathBuf> {
let out = std::process::Command::new("git")
let out = local_git_command()
.arg("rev-parse")
.arg("--show-toplevel")
.current_dir(cwd)
@@ -149,7 +149,7 @@ fn write_temp_patch(diff: &str) -> io::Result<(tempfile::TempDir, PathBuf)> {
}
fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32, String, String)> {
let mut cmd = std::process::Command::new("git");
let mut cmd = local_git_command();
for p in git_cfg {
cmd.arg(p);
}
@@ -163,6 +163,12 @@ fn run_git(cwd: &Path, git_cfg: &[String], args: &[String]) -> io::Result<(i32,
Ok((code, stdout, stderr))
}
fn local_git_command() -> std::process::Command {
let mut command = std::process::Command::new("git");
command.envs(crate::local_only_git_env());
command
}
fn quote_shell(s: &str) -> String {
let simple = s
.chars()
@@ -329,7 +335,7 @@ pub fn stage_paths(git_root: &Path, diff: &str) -> io::Result<()> {
if existing.is_empty() {
return Ok(());
}
let mut cmd = std::process::Command::new("git");
let mut cmd = local_git_command();
cmd.arg("add");
cmd.arg("--");
for p in &existing {
@@ -592,6 +598,10 @@ fn regex_ci(pat: &str) -> Regex {
Regex::new(&format!("(?i){pat}")).unwrap_or_else(|e| panic!("invalid regex: {e}"))
}
#[cfg(all(test, unix))]
#[path = "apply_transport_tests.rs"]
mod transport_tests;
#[cfg(test)]
mod tests {
use super::*;
@@ -0,0 +1,154 @@
use super::ApplyGitRequest;
use super::apply_git_patch;
use pretty_assertions::assert_eq;
use std::fs;
use std::os::unix::fs::PermissionsExt;
use std::path::Path;
use std::process::Command;
use std::process::Output;
#[test]
fn three_way_apply_does_not_lazy_fetch_promisor_objects() {
let temp_dir = tempfile::tempdir().expect("create temp dir");
let source = temp_dir.path().join("source");
let partial = temp_dir.path().join("partial");
fs::create_dir(&source).expect("create source repository");
run_git(&source, &["init", "-q", "--initial-branch=main"]);
run_git(&source, &["config", "uploadpack.allowFilter", "true"]);
fs::write(source.join("data.txt"), "one\nbase\nthree\n").expect("write base blob");
run_git(&source, &["add", "data.txt"]);
commit_all(&source, "base");
let base_blob = run_git_stdout(&source, &["rev-parse", "HEAD:data.txt"]);
fs::write(source.join("data.txt"), "one\nbase\npatched\n").expect("write patch blob");
let patch = String::from_utf8(run_git(&source, &["diff", "--full-index", "--binary"]).stdout)
.expect("patch should be UTF-8");
run_git(&source, &["checkout", "-q", "--", "data.txt"]);
fs::write(source.join("data.txt"), "current\nbase\nthree\n").expect("write head blob");
commit_all(&source, "head");
let complete_result = apply_git_patch(&ApplyGitRequest {
cwd: source.clone(),
diff: patch.clone(),
revert: false,
preflight: false,
})
.expect("apply patch in complete clone");
assert_eq!(
(
complete_result.exit_code,
fs::read_to_string(source.join("data.txt")).expect("read complete result"),
),
(0, "current\nbase\npatched\n".to_string()),
"three-way apply should retain normal behavior when every object is local; stderr: {}",
complete_result.stderr,
);
run_git(&source, &["reset", "-q", "--hard", "HEAD"]);
let source_url = format!("file://{}", source.display());
run_git(
temp_dir.path(),
&[
"-c",
"protocol.file.allow=always",
"clone",
"-q",
"--no-local",
"--filter=blob:none",
"--no-checkout",
&source_url,
partial.to_str().expect("partial path"),
],
);
run_git(&partial, &["checkout", "-q", "main"]);
let missing = run_git_stdout(
&partial,
&["rev-list", "--objects", "--all", "--missing=print"],
);
assert!(
missing.lines().any(|line| line == format!("?{base_blob}")),
"expected patch base blob {base_blob} to remain missing:\n{missing}",
);
let helper = temp_dir.path().join("transport-helper.sh");
fs::write(&helper, "#!/bin/sh\nprintf ran >\"$0.ran\"\nexit 1\n")
.expect("write transport helper");
let mut permissions = fs::metadata(&helper)
.expect("read transport helper metadata")
.permissions();
permissions.set_mode(/*mode*/ 0o755);
fs::set_permissions(&helper, permissions).expect("make transport helper executable");
let helper_url = format!("ext::{}", helper.display());
run_git(&partial, &["config", "remote.origin.url", &helper_url]);
run_git(&partial, &["config", "protocol.ext.allow", "always"]);
let patch_path = temp_dir.path().join("change.patch");
fs::write(&patch_path, &patch).expect("write attack reproduction patch");
let unprotected = Command::new("git")
.args(["apply", "--3way", patch_path.to_str().expect("patch path")])
.env_remove("GIT_ALLOW_PROTOCOL")
.env_remove("GIT_NO_LAZY_FETCH")
.current_dir(&partial)
.output()
.expect("run unprotected Git apply");
let helper_marker = helper.with_extension("sh.ran");
assert_eq!(
(unprotected.status.success(), helper_marker.exists()),
(false, true),
"unprotected three-way apply should reach the selected promisor transport",
);
fs::remove_file(&helper_marker).expect("remove reproduction marker");
run_git(&partial, &["reset", "-q", "--hard", "HEAD"]);
let partial_result = apply_git_patch(&ApplyGitRequest {
cwd: partial,
diff: patch,
revert: false,
preflight: false,
})
.expect("run fail-closed apply");
assert_eq!(
(partial_result.exit_code == 0, helper_marker.exists(),),
(false, false),
"local-only apply must fail without invoking the promisor transport",
);
}
fn run_git(cwd: &Path, args: &[&str]) -> Output {
let output = Command::new("git")
.args(args)
.current_dir(cwd)
.output()
.unwrap_or_else(|error| panic!("run git {args:?}: {error}"));
assert!(
output.status.success(),
"git {args:?} failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr),
);
output
}
fn run_git_stdout(cwd: &Path, args: &[&str]) -> String {
String::from_utf8_lossy(&run_git(cwd, args).stdout)
.trim()
.to_string()
}
fn commit_all(cwd: &Path, message: &str) {
run_git(
cwd,
&[
"-c",
"user.name=Codex Test",
"-c",
"user.email=codex@example.com",
"commit",
"-qam",
message,
],
);
}
+120 -1
View File
@@ -424,7 +424,11 @@ impl crate::FsmonitorProbeRunner for LocalFsmonitorProbeRunner<'_> {
// Both probes are fast, bounded metadata queries that do not inspect the
// worktree or index, so do not reduce the requested command's timeout.
let mut command = Command::new(self.git);
command.args(args).current_dir(self.cwd).kill_on_drop(true);
command
.envs(crate::local_only_git_env())
.args(args)
.current_dir(self.cwd)
.kill_on_drop(true);
match timeout(GIT_COMMAND_TIMEOUT, command.output()).await {
Ok(Ok(output)) if output.status.success() => Some(output.stdout),
_ => None,
@@ -445,6 +449,7 @@ async fn run_git_command_with_timeout_from(
) -> Option<std::process::Output> {
let mut command = Command::new(git);
command
.envs(crate::local_only_git_env())
.env("GIT_OPTIONAL_LOCKS", "0")
// Keep internal Git commands independent of repository-selected hooks
// and fsmonitor helpers while preserving built-in fsmonitor acceleration.
@@ -908,6 +913,120 @@ mod tests {
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
#[cfg(unix)]
fn run_git(cwd: &Path, args: &[&str]) -> std::process::Output {
let output = std::process::Command::new("git")
.args(args)
.current_dir(cwd)
.output()
.expect("run Git command");
assert!(
output.status.success(),
"git {args:?} failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
output
}
#[cfg(unix)]
fn run_git_stdout(cwd: &Path, args: &[&str]) -> String {
String::from_utf8(run_git(cwd, args).stdout)
.expect("Git output should be UTF-8")
.trim()
.to_string()
}
#[cfg(unix)]
fn commit_all(cwd: &Path, message: &str) {
run_git(
cwd,
&[
"-c",
"user.name=Codex Test",
"-c",
"user.email=codex@example.com",
"commit",
"-qam",
message,
],
);
}
#[cfg(unix)]
#[tokio::test]
async fn diff_against_sha_does_not_lazy_fetch_promisor_objects() {
let temp_dir = tempfile::tempdir().expect("create temp dir");
let source = temp_dir.path().join("source");
let clone = temp_dir.path().join("clone");
std::fs::create_dir(&source).expect("create source repository");
run_git(&source, &["init", "-q", "--initial-branch=main"]);
run_git(&source, &["config", "uploadpack.allowFilter", "true"]);
std::fs::write(source.join("data.txt"), "before\n").expect("write initial blob");
run_git(&source, &["add", "data.txt"]);
commit_all(&source, "initial");
let base_sha = run_git_stdout(&source, &["rev-parse", "HEAD"]);
let base_blob = run_git_stdout(&source, &["rev-parse", "HEAD:data.txt"]);
std::fs::write(source.join("data.txt"), "after\n").expect("write current blob");
commit_all(&source, "current");
let complete_diff = diff_against_sha(&source, &GitSha::new(&base_sha))
.await
.expect("diff complete repository");
assert!(
complete_diff.contains("-before") && complete_diff.contains("+after"),
"complete-repository diff should remain available:\n{complete_diff}"
);
let source_url = format!("file://{}", source.display());
run_git(
temp_dir.path(),
&[
"-c",
"protocol.file.allow=always",
"clone",
"-q",
"--no-local",
"--filter=blob:none",
"--no-checkout",
&source_url,
clone.to_str().expect("clone path"),
],
);
run_git(&clone, &["checkout", "-q", "main"]);
let missing = run_git_stdout(
&clone,
&["rev-list", "--objects", "--all", "--missing=print"],
);
assert!(
missing.lines().any(|line| line == format!("?{base_blob}")),
"expected historical blob {base_blob} to remain missing:\n{missing}"
);
let helper = temp_dir.path().join("transport-helper.sh");
std::fs::write(&helper, "#!/bin/sh\nprintf ran >\"$0.ran\"\nexit 1\n")
.expect("write transport helper");
let mut permissions = std::fs::metadata(&helper)
.expect("read transport helper metadata")
.permissions();
permissions.set_mode(/*mode*/ 0o755);
std::fs::set_permissions(&helper, permissions).expect("make transport helper executable");
let helper_url = format!("ext::{}", helper.display());
run_git(&clone, &["config", "remote.origin.url", &helper_url]);
run_git(&clone, &["config", "protocol.ext.allow", "always"]);
let diff = diff_against_sha(&clone, &GitSha::new(&base_sha)).await;
assert_eq!(
(diff, helper.with_extension("sh.ran").exists()),
(None, false),
"local-only diff must fail without invoking the promisor transport"
);
}
#[test]
fn canonicalize_git_remote_url_normalizes_github_variants() {
for remote in [
+2
View File
@@ -4,6 +4,7 @@ mod branch;
mod errors;
mod fsmonitor;
mod info;
mod local_only;
mod operations;
mod platform;
@@ -42,4 +43,5 @@ pub use info::git_diff_to_remote;
pub use info::local_git_branches;
pub use info::recent_commits;
pub use info::resolve_root_git_project_for_trust;
pub use local_only::local_only_git_env;
pub use platform::create_symlink;
+8
View File
@@ -0,0 +1,8 @@
/// Environment overrides for Git commands that must only use local repository data.
///
/// An empty `GIT_ALLOW_PROTOCOL` list denies every Git transport, including implicit fetches from
/// promisor remotes. `GIT_NO_LAZY_FETCH` provides defense in depth on Git versions that support it;
/// older versions safely ignore the unknown variable.
pub fn local_only_git_env() -> impl Iterator<Item = (&'static str, &'static str)> {
[("GIT_ALLOW_PROTOCOL", ""), ("GIT_NO_LAZY_FETCH", "1")].into_iter()
}
+1
View File
@@ -117,6 +117,7 @@ where
command.env(key, value);
}
}
command.envs(crate::local_only_git_env());
command.args(&args_vec);
let output = command.output()?;
if !output.status.success() {
+156 -1
View File
@@ -440,7 +440,7 @@ async fn run_git_command(
argv.extend(args.iter().map(|arg| (*arg).to_string()));
runner
.run(
WorkspaceCommand::new(argv)
WorkspaceCommand::local_only_git(argv)
.cwd(cwd.to_path_buf())
.env("GIT_OPTIONAL_LOCKS", "0"),
)
@@ -474,10 +474,85 @@ mod tests {
use super::*;
use crate::workspace_command::WorkspaceCommand;
use pretty_assertions::assert_eq;
#[cfg(unix)]
use std::fs;
use std::future::Future;
#[cfg(unix)]
use std::os::unix::fs::PermissionsExt;
use std::pin::Pin;
#[cfg(unix)]
use std::process::Command as ProcessCommand;
use std::sync::Mutex;
#[cfg(unix)]
#[tokio::test]
async fn branch_diff_stats_do_not_lazy_fetch_promisor_objects() {
let temp_dir = tempfile::tempdir().expect("create temp dir");
let source = temp_dir.path().join("source");
let clone = temp_dir.path().join("clone");
fs::create_dir(&source).expect("create source repository");
run_git(&source, &["init", "-q", "--initial-branch=main"]);
run_git(&source, &["config", "uploadpack.allowFilter", "true"]);
fs::write(source.join("data.txt"), "before\n").expect("write initial blob");
run_git(&source, &["add", "data.txt"]);
commit_all(&source, "initial");
let base_sha = run_git_stdout(&source, &["rev-parse", "HEAD"]);
let base_blob = run_git_stdout(&source, &["rev-parse", "HEAD:data.txt"]);
fs::write(source.join("data.txt"), "after\n").expect("write current blob");
commit_all(&source, "current");
let source_url = format!("file://{}", source.display());
run_git(
temp_dir.path(),
&[
"-c",
"protocol.file.allow=always",
"clone",
"-q",
"--no-local",
"--filter=blob:none",
"--no-checkout",
&source_url,
clone.to_str().expect("clone path"),
],
);
run_git(&clone, &["checkout", "-q", "main"]);
let missing = run_git_stdout(
&clone,
&["rev-list", "--objects", "--all", "--missing=print"],
);
assert!(
missing.lines().any(|line| line == format!("?{base_blob}")),
"expected historical blob {base_blob} to remain missing:\n{missing}"
);
run_git(
&clone,
&["update-ref", "refs/remotes/origin/main", &base_sha],
);
let helper = temp_dir.path().join("transport-helper.sh");
fs::write(&helper, "#!/bin/sh\nprintf ran >\"$0.ran\"\nexit 1\n")
.expect("write transport helper");
let mut permissions = fs::metadata(&helper)
.expect("read transport helper metadata")
.permissions();
permissions.set_mode(/*mode*/ 0o755);
fs::set_permissions(&helper, permissions).expect("make transport helper executable");
let helper_url = format!("ext::{}", helper.display());
run_git(&clone, &["config", "remote.origin.url", &helper_url]);
run_git(&clone, &["config", "protocol.ext.allow", "always"]);
let stats = branch_diff_stats_to_default_branch(&LocalRunner, &clone).await;
assert_eq!(
(stats, helper.with_extension("sh.ran").exists()),
(None, false),
"local-only branch stats must fail without invoking the promisor transport"
);
}
#[tokio::test]
async fn branch_diff_stats_prefers_remote_default_ref_over_stale_local_branch() {
let runner = FakeRunner::new(vec![
@@ -880,4 +955,84 @@ mod tests {
})
}
}
#[cfg(unix)]
fn run_git(cwd: &Path, args: &[&str]) -> std::process::Output {
let output = ProcessCommand::new("git")
.args(args)
.current_dir(cwd)
.output()
.expect("run Git command");
assert!(
output.status.success(),
"git {args:?} failed\nstdout:\n{}\nstderr:\n{}",
String::from_utf8_lossy(&output.stdout),
String::from_utf8_lossy(&output.stderr)
);
output
}
#[cfg(unix)]
fn run_git_stdout(cwd: &Path, args: &[&str]) -> String {
String::from_utf8(run_git(cwd, args).stdout)
.expect("Git output should be UTF-8")
.trim()
.to_string()
}
#[cfg(unix)]
fn commit_all(cwd: &Path, message: &str) {
run_git(
cwd,
&[
"-c",
"user.name=Codex Test",
"-c",
"user.email=codex@example.com",
"commit",
"-qam",
message,
],
);
}
#[cfg(unix)]
struct LocalRunner;
#[cfg(unix)]
impl WorkspaceCommandExecutor for LocalRunner {
fn run(
&self,
command: WorkspaceCommand,
) -> Pin<
Box<
dyn Future<Output = Result<WorkspaceCommandOutput, WorkspaceCommandError>>
+ Send
+ '_,
>,
> {
Box::pin(async move {
let mut process = ProcessCommand::new(&command.argv[0]);
process
.args(&command.argv[1..])
.current_dir(command.cwd.expect("test command cwd"));
for (key, value) in command.env {
match value {
Some(value) => {
process.env(key, value);
}
None => {
process.env_remove(key);
}
}
}
let output = process.output().expect("run test command");
Ok(WorkspaceCommandOutput {
exit_code: output.status.code().expect("test command exit code"),
stdout: String::from_utf8(output.stdout).expect("utf8 stdout"),
stderr: String::from_utf8(output.stderr).expect("utf8 stderr"),
})
})
}
}
}
+21 -4
View File
@@ -34,7 +34,7 @@ struct WorkspaceFsmonitorProbeRunner<'a> {
impl FsmonitorProbeRunner for WorkspaceFsmonitorProbeRunner<'_> {
async fn run_probe(&mut self, args: &[&str]) -> Option<Vec<u8>> {
let argv = ["git"].into_iter().chain(args.iter().copied());
let command = WorkspaceCommand::new(argv).cwd(self.cwd.to_path_buf());
let command = WorkspaceCommand::local_only_git(argv).cwd(self.cwd.to_path_buf());
match self.runner.run(command).await {
Ok(output) if output.success() => Some(output.stdout.into_bytes()),
_ => None,
@@ -238,7 +238,7 @@ async fn run_git_command(
]
.into_iter()
.chain(args.iter().copied());
let mut command = WorkspaceCommand::new(argv)
let mut command = WorkspaceCommand::local_only_git(argv)
.cwd(cwd.to_path_buf())
.timeout(DIFF_COMMAND_TIMEOUT)
.disable_output_cap();
@@ -764,7 +764,8 @@ mod tests {
}
fn filter_override_env(driver: &str) -> HashMap<String, Option<String>> {
HashMap::from([
let mut env = expected_local_only_git_env();
env.extend([
("GIT_CONFIG_COUNT".to_string(), Some("3".to_string())),
(
"GIT_CONFIG_KEY_0".to_string(),
@@ -781,6 +782,14 @@ mod tests {
Some(format!("{driver}.required")),
),
("GIT_CONFIG_VALUE_2".to_string(), Some("false".to_string())),
]);
env
}
fn expected_local_only_git_env() -> HashMap<String, Option<String>> {
HashMap::from([
("GIT_ALLOW_PROTOCOL".to_string(), Some(String::new())),
("GIT_NO_LAZY_FETCH".to_string(), Some("1".to_string())),
])
}
@@ -833,11 +842,19 @@ mod tests {
command.argv.get(1).map(String::as_str),
Some("config" | "version")
) {
assert_eq!(command.env, HashMap::new());
assert_eq!(command.env, expected_local_only_git_env());
assert_eq!(command.timeout, Duration::from_secs(/*secs*/ 5));
assert_eq!(command.output_bytes_cap, 64 * 1024);
assert_eq!(command.disable_output_cap, false);
} else {
assert_eq!(
command.env.get("GIT_ALLOW_PROTOCOL"),
Some(&Some(String::new()))
);
assert_eq!(
command.env.get("GIT_NO_LAZY_FETCH"),
Some(&Some("1".to_string()))
);
assert_eq!(command.timeout, DIFF_COMMAND_TIMEOUT);
assert_eq!(command.disable_output_cap, true);
}
+9
View File
@@ -62,6 +62,15 @@ impl WorkspaceCommand {
}
}
/// Creates a Git command that is restricted to already-local repository data.
pub(crate) fn local_only_git(argv: impl IntoIterator<Item = impl Into<String>>) -> Self {
let mut command = Self::new(argv);
for (key, value) in codex_git_utils::local_only_git_env() {
command = command.env(key, value);
}
command
}
/// Sets the command working directory.
pub(crate) fn cwd(mut self, cwd: impl Into<PathBuf>) -> Self {
self.cwd = Some(cwd.into());