diff --git a/crates/prek/src/cli/run/keeper.rs b/crates/prek/src/cli/run/keeper.rs index 4484bbe95..fd326a5e7 100644 --- a/crates/prek/src/cli/run/keeper.rs +++ b/crates/prek/src/cli/run/keeper.rs @@ -59,7 +59,8 @@ impl IntentToAddRestorer { fn restore(&self) -> Result<()> { // Restore the intent-to-add changes. if !self.0.is_empty() { - Command::new(GIT.as_ref()?) + let mut cmd = Command::new(GIT.as_ref()?); + git::apply_git_work_tree(&mut cmd) .arg("add") .arg("--intent-to-add") .arg("--") @@ -154,7 +155,8 @@ impl UnstagedChangesRestorer { } fn checkout_working_tree(root: &Path) -> Result<()> { - let output = Command::new(GIT.as_ref()?) + let mut cmd = Command::new(GIT.as_ref()?); + let output = git::apply_git_work_tree(&mut cmd) .arg("-c") .arg("submodule.recurse=0") .arg("checkout") @@ -173,7 +175,8 @@ impl UnstagedChangesRestorer { } fn git_apply(patch: &Path) -> Result<()> { - let output = Command::new(GIT.as_ref()?) + let mut cmd = Command::new(GIT.as_ref()?); + let output = git::apply_git_work_tree(&mut cmd) .arg("apply") .arg("--whitespace=nowarn") .arg(patch) diff --git a/crates/prek/src/cli/update/repository.rs b/crates/prek/src/cli/update/repository.rs index 3a6cdae5c..1bef19323 100644 --- a/crates/prek/src/cli/update/repository.rs +++ b/crates/prek/src/cli/update/repository.rs @@ -14,6 +14,7 @@ use semver::Version; use tracing::{debug, trace}; use crate::cli::update::{CommitPresence, RevisionSelection, SkippedDowngrade, TagTimestamp}; +use crate::git::GitCommandExt; use crate::{config, git}; /// Initializes a temporary git repo and fetches the remote HEAD plus tags. @@ -27,7 +28,7 @@ pub(super) async fn setup_and_fetch_repo(repo_url: &str, repo_path: &Path) -> Re .arg("--filter=blob:none") .arg("--tags") .current_dir(repo_path) - .remove_git_envs() + .isolate_from_git_env() .stdout(Stdio::null()) .stderr(Stdio::null()) .status() @@ -43,7 +44,7 @@ pub(super) async fn resolve_revision_to_commit(repo_path: &Path, rev: &str) -> R .arg(format!("{rev}^{{}}")) .check(true) .current_dir(repo_path) - .remove_git_envs() + .isolate_from_git_env() .output() .await?; @@ -83,7 +84,7 @@ pub(super) async fn is_commit_present(repo_path: &Path, commit: &str) -> Result< .env(EnvVars::LC_ALL, "C") .check(false) .current_dir(repo_path) - .remove_git_envs() + .isolate_from_git_env() .stdout(Stdio::null()) .output() .await?; @@ -127,7 +128,7 @@ pub(super) async fn resolve_bleeding_edge(repo_path: &Path) -> Result Result Result> .arg("refs/tags") .check(true) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await?; @@ -380,7 +381,7 @@ pub(super) async fn checkout_and_validate_manifest( .arg("show") .arg(format!("{rev}:{PRE_COMMIT_HOOKS_YAML}")) .current_dir(repo_path) - .remove_git_envs() + .isolate_from_git_env() .stdout(Stdio::null()) .stderr(Stdio::null()) .status() @@ -394,7 +395,7 @@ pub(super) async fn checkout_and_validate_manifest( .arg("--") .arg(PRE_COMMIT_HOOKS_YAML) .current_dir(repo_path) - .remove_git_envs() + .isolate_from_git_env() .stdout(Stdio::null()) .stderr(Stdio::null()) .status() @@ -431,6 +432,7 @@ mod tests { }; use crate::cli::update::{RevisionSelection, SkippedDowngrade}; use crate::git; + use crate::git::GitCommandExt; use crate::process::Cmd; use std::path::Path; use std::time::{SystemTime, UNIX_EPOCH}; @@ -443,7 +445,7 @@ mod tests { .unwrap() .arg("init") .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -452,7 +454,7 @@ mod tests { .unwrap() .args(["config", "user.email", "test@test.com"]) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -461,7 +463,7 @@ mod tests { .unwrap() .args(["config", "user.name", "Test"]) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -477,7 +479,7 @@ mod tests { "initial", ]) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -486,7 +488,7 @@ mod tests { .unwrap() .args(["branch", "-M", "trunk"]) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -522,7 +524,7 @@ mod tests { async fn create_commit(repo: &Path, message: &str) { git_cmd(repo) .args(["commit", "--allow-empty", "-m", message]) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -541,7 +543,7 @@ mod tests { .args(["commit", "--allow-empty", "-m", message]) .env("GIT_AUTHOR_DATE", &date_str) .env("GIT_COMMITTER_DATE", &date_str) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -551,7 +553,7 @@ mod tests { git_cmd(repo) .arg("tag") .arg(tag) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -573,7 +575,7 @@ mod tests { .arg(tag) .env("GIT_AUTHOR_DATE", &date_str) .env("GIT_COMMITTER_DATE", &date_str) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -611,7 +613,7 @@ mod tests { .unwrap() .args(["fetch", ".", "HEAD"]) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -631,7 +633,7 @@ mod tests { .unwrap() .args(["fetch", ".", "HEAD"]) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap(); @@ -642,7 +644,7 @@ mod tests { .unwrap() .args(["rev-parse", "HEAD"]) .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .output() .await .unwrap() diff --git a/crates/prek/src/git.rs b/crates/prek/src/git.rs index 596495c90..6df06d1f8 100644 --- a/crates/prek/src/git.rs +++ b/crates/prek/src/git.rs @@ -1,11 +1,11 @@ use std::collections::HashSet; use std::path::{Path, PathBuf}; -use std::process::Stdio; +use std::process::{Command, Stdio}; use std::str::Utf8Error; -use std::sync::LazyLock; +use std::sync::{LazyLock, OnceLock}; use anyhow::Result; -use prek_consts::env_vars::EnvVars; +use prek_consts::env_vars::{EnvVars, EnvVarsRead}; use rustc_hash::FxHashSet; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tracing::{debug, instrument, warn}; @@ -37,6 +37,30 @@ pub(crate) enum Error { pub(crate) static GIT: LazyLock> = LazyLock::new(|| which::which("git")); +// Git hooks can expose `GIT_DIR` without `GIT_WORK_TREE`. Keep the derived +// work tree scoped to prek's own git commands so user hooks do not inherit it. +static GIT_WORK_TREE: OnceLock> = OnceLock::new(); + +pub(crate) fn init_git_work_tree() -> Result<()> { + if !EnvVars.is_set(EnvVars::GIT_DIR) || EnvVars.is_set(EnvVars::GIT_WORK_TREE) { + let _ = GIT_WORK_TREE.set(None); + return Ok(()); + } + + let cwd = std::env::current_dir()?; + debug!( + "Using {} `{}` for git commands", + EnvVars::GIT_WORK_TREE, + cwd.display() + ); + let _ = GIT_WORK_TREE.set(Some(cwd)); + Ok(()) +} + +fn git_work_tree() -> Option<&'static Path> { + GIT_WORK_TREE.get().and_then(Option::as_deref) +} + pub(crate) static GIT_ROOT: LazyLock> = LazyLock::new(|| { get_root() .map(|root| dunce::canonicalize(&root).unwrap_or(root)) @@ -51,7 +75,7 @@ pub(crate) static GIT_ROOT: LazyLock> = LazyLock::new(|| /// and set `GIT_INDEX_FILE` to point to it. /// We need to keep the `GIT_INDEX_FILE` env var to make sure `git write-tree` works correctly. /// -pub(crate) static GIT_ENV_TO_REMOVE: LazyLock> = LazyLock::new(|| { +static GIT_ENVS_TO_REMOVE: LazyLock> = LazyLock::new(|| { let keep = &[ "GIT_EXEC_PATH", "GIT_SSH", @@ -75,9 +99,39 @@ pub(crate) static GIT_ENV_TO_REMOVE: LazyLock> = LazyLock: .collect() }); +pub(crate) trait GitCommandExt { + fn isolate_from_git_env(&mut self) -> &mut Self; +} + +pub(crate) fn apply_git_work_tree(cmd: &mut Command) -> &mut Command { + if let Some(work_tree) = git_work_tree() { + cmd.env(EnvVars::GIT_WORK_TREE, work_tree); + } + cmd +} + +impl GitCommandExt for Cmd { + fn isolate_from_git_env(&mut self) -> &mut Self { + // `git_cmd()` adds this synthetic value as a command-local env. Commands + // that call `isolate_from_git_env()` are intentionally detached from the + // current repo, so remove it here; inherited `GIT_WORK_TREE` is handled + // by `GIT_ENVS_TO_REMOVE`. + if git_work_tree().is_some() { + self.env_remove(EnvVars::GIT_WORK_TREE); + } + for (key, _) in GIT_ENVS_TO_REMOVE.iter() { + self.env_remove(key); + } + self + } +} + pub(crate) fn git_cmd() -> Result { let mut cmd = Cmd::new(GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?); cmd.hidden_args(["-c", "core.useBuiltinFSMonitor=false"]); + if let Some(work_tree) = git_work_tree() { + cmd.env(EnvVars::GIT_WORK_TREE, work_tree); + } Ok(cmd) } @@ -414,7 +468,8 @@ pub(crate) async fn write_tree() -> Result { #[instrument(level = "trace")] pub(crate) fn get_root() -> Result { let git = GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?; - let output = std::process::Command::new(git) + let mut cmd = Command::new(git); + let output = apply_git_work_tree(&mut cmd) .arg("rev-parse") .arg("--show-toplevel") .output()?; @@ -439,7 +494,7 @@ pub(crate) async fn init_repo(url: &str, path: &Path) -> Result<(), Error> { .arg("init") .arg("--template=") .arg(path) - .remove_git_envs() + .isolate_from_git_env() .check(true) .output() .await?; @@ -450,7 +505,7 @@ pub(crate) async fn init_repo(url: &str, path: &Path) -> Result<(), Error> { .arg("add") .arg("origin") .arg(url) - .remove_git_envs() + .isolate_from_git_env() .check(true) .output() .await?; @@ -513,7 +568,7 @@ async fn shallow_clone( .arg("origin") .arg(rev) .arg("--depth=1") - .remove_git_envs() + .isolate_from_git_env() .env(EnvVars::LC_ALL, "C") .env(EnvVars::GIT_TERMINAL_PROMPT, terminal_prompt.env_value()) .check(true) @@ -524,7 +579,7 @@ async fn shallow_clone( .current_dir(path) .arg("checkout") .arg("FETCH_HEAD") - .remove_git_envs() + .isolate_from_git_env() .env(EnvVars::PREK_INTERNAL__SKIP_POST_CHECKOUT, "1") .env(EnvVars::LC_ALL, "C") .env(EnvVars::GIT_TERMINAL_PROMPT, terminal_prompt.env_value()) @@ -543,7 +598,7 @@ async fn full_clone(rev: &str, path: &Path, terminal_prompt: TerminalPrompt) -> .arg("fetch") .arg("origin") .arg("--tags") - .remove_git_envs() + .isolate_from_git_env() .env(EnvVars::LC_ALL, "C") .env(EnvVars::GIT_TERMINAL_PROMPT, terminal_prompt.env_value()) .check(true) @@ -554,7 +609,7 @@ async fn full_clone(rev: &str, path: &Path, terminal_prompt: TerminalPrompt) -> .current_dir(path) .arg("checkout") .arg(rev) - .remove_git_envs() + .isolate_from_git_env() .env(EnvVars::PREK_INTERNAL__SKIP_POST_CHECKOUT, "1") .env(EnvVars::LC_ALL, "C") .env(EnvVars::GIT_TERMINAL_PROMPT, terminal_prompt.env_value()) @@ -586,7 +641,7 @@ async fn update_submodules( if shallow { cmd.arg("--depth=1"); } - cmd.remove_git_envs() + cmd.isolate_from_git_env() .env(EnvVars::LC_ALL, "C") .env(EnvVars::GIT_TERMINAL_PROMPT, terminal_prompt.env_value()) .check(true) @@ -606,7 +661,7 @@ async fn should_update_submodules(path: &Path) -> Result { .arg("ls-files") .arg("-z") .arg("-s") - .remove_git_envs() + .isolate_from_git_env() .env(EnvVars::LC_ALL, "C") .check(true) .output() @@ -912,7 +967,8 @@ pub(crate) fn list_submodules(git_root: &Path) -> Result, Error> { } let git = GIT.as_ref().map_err(|&e| Error::GitNotFound(e))?; - let output = std::process::Command::new(git) + let mut cmd = Command::new(git); + let output = apply_git_work_tree(&mut cmd) .current_dir(git_root) .arg("config") .arg("--file") diff --git a/crates/prek/src/languages/golang/golang.rs b/crates/prek/src/languages/golang/golang.rs index 35a57f691..4f680357a 100644 --- a/crates/prek/src/languages/golang/golang.rs +++ b/crates/prek/src/languages/golang/golang.rs @@ -9,6 +9,7 @@ use prek_consts::prepend_paths; use crate::cli::reporter::HookInstallReporter; use crate::cli::run::HookRunReporter; +use crate::git::GitCommandExt; use crate::hook::{Hook, InstallInfo, InstalledHook}; use crate::languages::LanguageBackend; use crate::languages::golang::GoRequest; @@ -88,7 +89,7 @@ impl LanguageBackend for Golang { go_install_cmd() .arg("./...") .current_dir(repo) - .remove_git_envs() + .isolate_from_git_env() .check(true) .output() .await?; @@ -98,7 +99,11 @@ impl LanguageBackend for Golang { if let Some(repo) = hook.repo_path() { cmd.current_dir(repo); } - cmd.arg(dep).remove_git_envs().check(true).output().await?; + cmd.arg(dep) + .isolate_from_git_env() + .check(true) + .output() + .await?; } info.persist_env_path(); diff --git a/crates/prek/src/languages/python/python.rs b/crates/prek/src/languages/python/python.rs index 9bdfdc5cc..6f660684b 100644 --- a/crates/prek/src/languages/python/python.rs +++ b/crates/prek/src/languages/python/python.rs @@ -13,6 +13,7 @@ use tracing::{debug, trace}; use crate::cli::reporter::HookInstallReporter; use crate::cli::run::HookRunReporter; +use crate::git::GitCommandExt; use crate::hook::InstalledHook; use crate::hook::{Hook, InstallInfo}; use crate::languages::LanguageBackend; @@ -272,7 +273,7 @@ impl Python { Self::remove_uv_python_override_envs(&mut cmd) // Remove GIT environment variables that may leak from git hooks (e.g., in worktrees). // These can break packages using setuptools_scm for file discovery. - .remove_git_envs() + .isolate_from_git_env() .check(true); cmd } diff --git a/crates/prek/src/languages/rust/rust.rs b/crates/prek/src/languages/rust/rust.rs index 6007a09a6..deab0b73e 100644 --- a/crates/prek/src/languages/rust/rust.rs +++ b/crates/prek/src/languages/rust/rust.rs @@ -15,6 +15,7 @@ use tracing::debug; use crate::cli::reporter::HookInstallReporter; use crate::cli::run::HookRunReporter; use crate::fs::is_executable; +use crate::git::GitCommandExt; use crate::hook::{Hook, InstallInfo, InstalledHook}; use crate::languages::LanguageBackend; use crate::languages::rust::RustRequest; @@ -275,7 +276,7 @@ async fn install_local_project( .current_dir(&package_dir) .env(EnvVars::PATH, new_path) .env(EnvVars::CARGO_HOME, cargo_home) - .remove_git_envs() + .isolate_from_git_env() .check(true) .output() .await?; @@ -324,7 +325,7 @@ async fn install_local_project( cmd.current_dir(&manifest_dir) .env(EnvVars::PATH, new_path) .env(EnvVars::CARGO_HOME, cargo_home) - .remove_git_envs() + .isolate_from_git_env() .check(true) .output() .await?; @@ -347,7 +348,7 @@ async fn install_local_project( cmd.current_dir(&package_dir) .env(EnvVars::PATH, new_path) .env(EnvVars::CARGO_HOME, cargo_home) - .remove_git_envs() + .isolate_from_git_env() .check(true) .output() .await?; @@ -380,7 +381,7 @@ async fn install_cli_dependency( cmd.env(EnvVars::PATH, new_path) .env(EnvVars::CARGO_HOME, cargo_home) - .remove_git_envs() + .isolate_from_git_env() .check(true) .output() .await?; diff --git a/crates/prek/src/main.rs b/crates/prek/src/main.rs index ee950e44c..49b663e92 100644 --- a/crates/prek/src/main.rs +++ b/crates/prek/src/main.rs @@ -9,7 +9,6 @@ use anyhow::{Context, Result}; use clap::{CommandFactory, Parser}; use clap_complete::CompleteEnv; use owo_colors::OwoColorize; -use prek_consts::env_vars::{EnvVars, EnvVarsRead}; use tracing::debug; use tracing::level_filters::LevelFilter; use tracing_subscriber::filter::Directive; @@ -198,21 +197,13 @@ async fn run(cli: Cli) -> Result { } } - // If `GIT_DIR` is set, prek may be running from a git hook. - // Git exports `GIT_DIR` but *not* `GIT_WORK_TREE`. Without `GIT_WORK_TREE`, git - // treats the current working directory as the working tree. If prek changes the current - // working directory (with `--cd`), git commands run by prek may behave unexpectedly. - // - // To make git behavior stable, we set `GIT_WORK_TREE` ourselves to where prek is run from. - // If `GIT_WORK_TREE` is already set, we leave it alone. - // If `GIT_DIR` is not set, we let git discover `.git` after an optional `cd`. + // Initialize before `--cd` so a synthetic work tree captures the directory where + // git launched the hook. Keep it in prek's own lazy state instead of the + // process environment, otherwise user hooks and their nested git commands + // would inherit a `GIT_WORK_TREE` that git itself did not expose. // See: https://www.spinics.net/lists/git/msg374197.html // https://github.com/pre-commit/pre-commit/issues/2295 - if EnvVars.is_set(EnvVars::GIT_DIR) && !EnvVars.is_set(EnvVars::GIT_WORK_TREE) { - let cwd = std::env::current_dir().context("Failed to get current directory")?; - debug!("Setting {} to `{}`", EnvVars::GIT_WORK_TREE, cwd.display()); - unsafe { std::env::set_var(EnvVars::GIT_WORK_TREE, cwd) } - } + git::init_git_work_tree()?; if let Some(dir) = cli.globals.cd.as_ref() { debug!("Changing current directory to: `{}`", dir.display()); diff --git a/crates/prek/src/process.rs b/crates/prek/src/process.rs index 3c8f78f75..e93d22829 100644 --- a/crates/prek/src/process.rs +++ b/crates/prek/src/process.rs @@ -501,14 +501,6 @@ impl Cmd { pub fn get_current_dir(&self) -> Option<&Path> { self.inner.as_std().get_current_dir() } - - /// Remove some git-specific environment variables to make git commands isolated. - pub fn remove_git_envs(&mut self) -> &mut Self { - for (key, _) in crate::git::GIT_ENV_TO_REMOVE.iter() { - self.inner.env_remove(key); - } - self - } } /// Diagnostic APIs used by execution methods and direct child-process callers. diff --git a/crates/prek/tests/hook_impl.rs b/crates/prek/tests/hook_impl.rs index 6c09b2513..c0d112498 100644 --- a/crates/prek/tests/hook_impl.rs +++ b/crates/prek/tests/hook_impl.rs @@ -612,6 +612,41 @@ fn git_dir_respected() { "); } +#[test] +fn git_dir_synthesized_git_work_tree_not_leaked_to_hook() { + let context = TestContext::new(); + context.init_project(); + context.write_pre_commit_config(indoc! { r#" + repos: + - repo: local + hooks: + - id: print-git-work-tree + name: Print Git Work Tree + language: system + entry: python3 -c 'import os, sys; print("GIT_DIR:", os.environ.get("GIT_DIR")); print("GIT_WORK_TREE:", os.environ.get("GIT_WORK_TREE")); sys.exit(1)' + pass_filenames: false + always_run: true + "#}); + context.git_add("."); + + let mut run = context.run(); + run.env(EnvVars::GIT_DIR, context.work_dir().join(".git")); + + cmd_snapshot!(context.filters(), run, @r" + success: false + exit_code: 1 + ----- stdout ----- + Print Git Work Tree......................................................Failed + - hook id: print-git-work-tree + - exit code: 1 + + GIT_DIR: [TEMP_DIR]/.git + GIT_WORK_TREE: None + + ----- stderr ----- + "); +} + #[test] fn workspace_hook_impl_root() -> anyhow::Result<()> { let context = TestContext::new();