diff --git a/CHANGELOG.md b/CHANGELOG.md index 6fed6da..0830ce6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,31 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Fixed + +- `doctor` no longer reports every routed directory as differing from its + canonical path on Windows (it now compares in the same forward-slash, + de-UNC'd form routes are stored in). +- `create`/`edit` reject control characters in a signing key, and rendered + fragments escape control characters, closing a gitconfig-section injection + via a newline-bearing `--signing-key`. +- `init`, `doctor` and `uninstall` find the git-id include and + `user.useConfigOnly` even when they live in `$XDG_CONFIG_HOME/git/config` and + a `~/.gitconfig` is later created; `uninstall` now removes them from whichever + global config file holds them. +- `use` warns when routing a linked worktree's or submodule's own path, where + git matches against the main repository and the route would never apply. +- `edit` validates all fields before writing any, so a rejected edit no longer + leaves an earlier field already changed. +- `delete` rewrites the routes file before removing the fragment, so an + interruption cannot leave a route pointing at a deleted fragment. +- `doctor` also flags a duplicate `gitdir` route when one side is a hand-added + (preserved) `[includeIf]` block. +- `init` no longer overwrites an earlier same-second backup of the global + config. + ## [0.2.0] ### Added @@ -65,5 +90,6 @@ Initial release. formula pushed to the tap. - Dual licensed under MIT OR Apache-2.0. +[Unreleased]: https://github.com/jmarette/git-id/compare/v0.2.0...HEAD [0.2.0]: https://github.com/jmarette/git-id/compare/v0.1.0...v0.2.0 [0.1.0]: https://github.com/jmarette/git-id/releases/tag/v0.1.0 diff --git a/README.md b/README.md index df681fe..515691d 100644 --- a/README.md +++ b/README.md @@ -166,8 +166,9 @@ $ git-id completions nushell | save -f ~/.config/nushell/completions/git-id.nu `.git` directory, which lives under the *main* repository. A worktree placed under a routed directory whose main repo lives elsewhere keeps the main repo's identity. `git id which` detects and explains this. -- **Bare repository exactly at a routed root** is not matched (repositories - *below* the routed directory, bare or not, are fine). +- **Bare repositories** route like any other: because routes are stored with a + trailing slash, a route matches a bare repo whose git directory *is* the + routed directory itself, as well as every repository below it. - **Symlinked paths** are stored resolved (e.g. `/tmp/...` becomes `/private/tmp/...` on macOS) because that is what Git matches against. - `git id --help` tries to open a `git-id` man page and fails; use @@ -221,8 +222,8 @@ Contributions are welcome. Commit messages follow [Conventional Commits](https://www.conventionalcommits.org) (`feat:`, `fix:`, `docs:`, `refactor:`, `test:`, `chore:`). Pull requests are -squash-merged, so only the PR title has to follow the convention — your own -commits can stay messy. +merged with a merge commit (not squashed), so every commit is preserved in +`master` — keep each commit focused and give it a Conventional Commits message. Contributions are dual-licensed under MIT OR Apache-2.0; see [License](#license). diff --git a/src/commands/create.rs b/src/commands/create.rs index 8c475a8..945ee8a 100644 --- a/src/commands/create.rs +++ b/src/commands/create.rs @@ -44,10 +44,17 @@ pub fn run(env: &Env, args: &CreateArgs) -> Result { } }; let signing_key = match &args.signing_key { - Some(v) if !v.is_empty() => Some(v.clone()), + Some(v) if !v.is_empty() => { + store::validate_signing_key(v)?; + Some(v.clone()) + } Some(_) => None, None if pure_interactive => { - let v = prompt::ask("Signing key (user.signingkey)", true, |_| Ok(()))?; + let v = prompt::ask( + "Signing key (user.signingkey)", + true, + store::validate_signing_key, + )?; if v.is_empty() { None } else { Some(v) } } None => None, diff --git a/src/commands/delete.rs b/src/commands/delete.rs index b58ed1e..28164f7 100644 --- a/src/commands/delete.rs +++ b/src/commands/delete.rs @@ -40,13 +40,17 @@ pub fn run(env: &Env, args: &DeleteArgs) -> Result { } } - if fragment_exists { - store::remove(env, &args.name)?; - } + // Rewrite routes first (atomic_write makes this step crash-safe), then + // remove the fragment: a mid-operation failure leaves a present-but-unrouted + // fragment (harmless; doctor reports it as info) rather than a route + // pointing at a deleted fragment. let removed = model.remove_identity(&args.name); if !removed.is_empty() { routes::save(env, &model)?; } + if fragment_exists { + store::remove(env, &args.name)?; + } if removed.is_empty() { println!("Deleted identity `{}`.", args.name); } else { diff --git a/src/commands/doctor.rs b/src/commands/doctor.rs index a6a26f6..e671c13 100644 --- a/src/commands/doctor.rs +++ b/src/commands/doctor.rs @@ -69,14 +69,26 @@ pub fn run(env: &Env) -> Result { )); } + // Hand-added `[includeIf "gitdir:..."]` blocks live in `preserved`; count + // them alongside managed entries so a duplicate split across a managed + // route and a preserved block (git applies both) is still flagged. + let preserved_gitdirs: Vec = model + .preserved + .iter() + .filter_map(|block| block.lines().next()) + .filter_map(routes::parse_gitdir_header) + .collect(); let mut counts: HashMap<&str, u32> = HashMap::new(); for entry in &model.entries { *counts.entry(entry.gitdir.as_str()).or_insert(0) += 1; } + for gitdir in &preserved_gitdirs { + *counts.entry(gitdir.as_str()).or_insert(0) += 1; + } for (gitdir, n) in counts { if n > 1 { d.error(&format!( - "{n} routes exist for {gitdir} — run `git-id use` on it again to deduplicate" + "{n} routes exist for {gitdir} — git applies all of them (last wins); remove the duplicate `[includeIf]` block(s) from routes.gitconfig" )); } } @@ -114,7 +126,11 @@ pub fn run(env: &Env) -> Result { } else { let (canon, _) = paths::canonicalize_anchored(Path::new(entry.gitdir.trim_end_matches('/'))); - let canon_slash = paths::ensure_trailing_slash(canon.to_string_lossy().into_owned()); + // Compare in git-path form (forward slashes, de-UNC'd) so the stored + // gitdir is not falsely flagged as non-canonical on Windows, where + // fs::canonicalize yields a `\\?\C:\...` path. Identity on Unix. + let canon_slash = + paths::ensure_trailing_slash(paths::to_git_path(&canon.to_string_lossy())); if canon_slash != entry.gitdir { d.warn(&format!( "route {pretty} differs from the directory's canonical path {canon_slash} — git matches canonical paths, re-run `git-id use` on it" @@ -149,7 +165,7 @@ pub fn run(env: &Env) -> Result { } } - if gitcfg::global_get("user.useConfigOnly")?.as_deref() == Some("true") { + if init::useconfigonly_is_enabled(env)? { d.ok("user.useConfigOnly is enabled — commits require an explicit identity"); } else { d.info( diff --git a/src/commands/init.rs b/src/commands/init.rs index 21b7f66..fa53207 100644 --- a/src/commands/init.rs +++ b/src/commands/init.rs @@ -10,14 +10,53 @@ use crate::env::Env; use crate::paths::display_pretty; use crate::{gitcfg, paths, prompt, routes}; +/// The global-level config files git actually reads and that git-id may have +/// written to, in increasing precedence order, existing files only. Mirrors +/// git: an explicit `GIT_CONFIG_GLOBAL` is the *only* global file; otherwise +/// git reads BOTH `$XDG_CONFIG_HOME/git/config` and `~/.gitconfig` (the latter +/// wins). We scan these directly rather than `git config --global`, because +/// `--global` resolves to a single file (`~/.gitconfig` once it exists) and so +/// goes blind to an include we legitimately wrote into the XDG file before +/// `~/.gitconfig` came to exist. +pub fn global_config_files(env: &Env) -> Vec { + let candidates = if let Some(global) = &env.git_config_global { + vec![global.clone()] + } else { + vec![ + env.config_base.join("git").join("config"), + env.home.join(".gitconfig"), + ] + }; + candidates.into_iter().filter(|p| p.exists()).collect() +} + /// Whether the global git config already includes our routes file. pub fn include_is_installed(env: &Env) -> Result { // Compare in git-path form so a path Git normalized to forward slashes on // Windows still matches what we wrote (and stays byte-identical on Unix). let want = paths::to_git_path(routes_path_str(env)?); - Ok(gitcfg::global_get_all("include.path")?.iter().any(|p| { - paths::to_git_path(p) == want || paths::expand_tilde(p, &env.home) == env.routes_file - })) + for file in global_config_files(env) { + for p in gitcfg::get_file_all(&file, "include.path")? { + if paths::to_git_path(&p) == want + || paths::expand_tilde(&p, &env.home) == env.routes_file + { + return Ok(true); + } + } + } + Ok(false) +} + +/// Whether `user.useConfigOnly` resolves to true across the global config +/// files (later files win, mirroring git's precedence). +pub fn useconfigonly_is_enabled(env: &Env) -> Result { + let mut enabled = false; + for file in global_config_files(env) { + if let Some(value) = gitcfg::get_file_bool(&file, "user.useConfigOnly")? { + enabled = value; + } + } + Ok(enabled) } pub fn routes_path_str(env: &Env) -> Result<&str> { @@ -47,7 +86,15 @@ impl Backup { .file_name() .and_then(|n| n.to_str()) .unwrap_or("gitconfig"); - let bak = target.with_file_name(format!("{name}.bak-{}", timestamp_utc()?)); + // The timestamp has 1-second resolution, so find a free name rather than + // letting fs::copy silently clobber a same-second backup. + let ts = timestamp_utc()?; + let mut bak = target.with_file_name(format!("{name}.bak-{ts}")); + let mut n = 1; + while bak.exists() { + bak = target.with_file_name(format!("{name}.bak-{ts}-{n}")); + n += 1; + } fs::copy(&target, &bak) .with_context(|| format!("cannot back up {} to {}", target.display(), bak.display()))?; self.path = Some(bak); @@ -84,7 +131,7 @@ pub fn run(env: &Env, args: &InitArgs) -> Result { Declined, SkippedNonInteractive, } - let already_on = gitcfg::global_get("user.useConfigOnly")?.as_deref() == Some("true"); + let already_on = useconfigonly_is_enabled(env)?; let uco = if already_on { Uco::AlreadyEnabled } else { diff --git a/src/commands/uninstall.rs b/src/commands/uninstall.rs index dfc1a3d..e2e0e6d 100644 --- a/src/commands/uninstall.rs +++ b/src/commands/uninstall.rs @@ -18,7 +18,7 @@ const ROUTES_INCLUDE_REGEX: &str = r"git-id/routes\.gitconfig$"; pub fn run(env: &Env, args: &UninstallArgs) -> Result { let include = init::include_is_installed(env)?; let dir_exists = env.config_dir.exists(); - let use_config_only = gitcfg::global_get("user.useConfigOnly")?.as_deref() == Some("true"); + let use_config_only = init::useconfigonly_is_enabled(env)?; if !include && !dir_exists && !use_config_only { println!("git-id is not set up — nothing to remove."); @@ -47,11 +47,16 @@ pub fn run(env: &Env, args: &UninstallArgs) -> Result { } } - if include { - gitcfg::global_unset_all_matching("include.path", ROUTES_INCLUDE_REGEX)?; - } - if use_config_only { - gitcfg::global_unset("user.useConfigOnly")?; + // Clean every global config file git reads, not just the one `--global` + // resolves to: the include (or useConfigOnly) may live in the XDG file even + // when ~/.gitconfig now exists, and `--global` would never reach it. + for file in init::global_config_files(env) { + if include { + gitcfg::unset_all_matching_file(&file, "include.path", ROUTES_INCLUDE_REGEX)?; + } + if use_config_only { + gitcfg::unset_file(&file, "user.useConfigOnly")?; + } } if dir_exists { fs::remove_dir_all(&env.config_dir) diff --git a/src/commands/use.rs b/src/commands/use.rs index df8ddd3..8016f7e 100644 --- a/src/commands/use.rs +++ b/src/commands/use.rs @@ -32,13 +32,28 @@ pub fn run(env: &Env, args: &UseArgs) -> Result { eprintln!( "warning: {pretty_dir} does not exist yet — the route is recorded and will apply once it is created" ); - } else if let Some(top) = gitcfg::show_toplevel(&nd.path)? { - let top_slash = paths::ensure_trailing_slash(top.to_string_lossy().into_owned()); - if top_slash != nd.gitdir { - eprintln!( - "note: {pretty_dir} is inside the repository {} — the route affects repositories under it, not that repository itself", - display_pretty(&top_slash, &env.home) - ); + } else { + let git_dir = gitcfg::absolute_git_dir(&nd.path)?; + let top = gitcfg::show_toplevel(&nd.path)?; + // Linked worktrees and submodules: git matches `gitdir:` against the + // real .git directory, which lives under the *main* repository — so a + // route on this directory's own path is recorded but never applied. + if let (Some(gd), Some(top)) = (&git_dir, &top) { + if !gd.starts_with(top) { + eprintln!( + "warning: {pretty_dir} is a linked worktree or submodule whose git directory is {} — git matches routes against the main repository's location, so this route will NOT apply here; route the main repository instead", + display_pretty(&gd.to_string_lossy(), &env.home) + ); + } + } + if let Some(top) = &top { + let top_slash = paths::ensure_trailing_slash(top.to_string_lossy().into_owned()); + if top_slash != nd.gitdir { + eprintln!( + "note: {pretty_dir} is inside the repository {} — the route affects repositories under it, not that repository itself", + display_pretty(&top_slash, &env.home) + ); + } } } diff --git a/src/gitcfg.rs b/src/gitcfg.rs index 9d64ef3..0f7caac 100644 --- a/src/gitcfg.rs +++ b/src/gitcfg.rs @@ -105,28 +105,21 @@ pub fn unset_file(file: &Path, key: &str) -> Result<()> { } } -/// `git config --global --get ` -> None when absent (or when no global -/// config file exists at all). -pub fn global_get(key: &str) -> Result> { - let out = run(Command::new("git").args(["config", "--global", "--get", key]))?; - match out.status.code() { - Some(0) => Ok(Some(stdout_trimmed(&out))), - Some(1) => Ok(None), - _ => bail!( - "`git config --global --get {key}` failed: {}", - stderr_trimmed(&out) - ), - } -} - -/// All values of a multi-valued global key; empty when absent. -pub fn global_get_all(key: &str) -> Result> { - let out = run(Command::new("git").args(["config", "--global", "--get-all", key]))?; +/// All values of a multi-valued key in a specific file; empty when the key is +/// absent (exit 1). The file is expected to exist — callers filter to existing +/// files so a missing-file read error is never silently swallowed. +pub fn get_file_all(file: &Path, key: &str) -> Result> { + let out = run(Command::new("git") + .arg("config") + .arg("--file") + .arg(file) + .args(["--get-all", key]))?; match out.status.code() { Some(0) => Ok(stdout_trimmed(&out).lines().map(str::to_string).collect()), Some(1) => Ok(Vec::new()), _ => bail!( - "`git config --global --get-all {key}` failed: {}", + "`git config --file {} --get-all {key}` failed: {}", + file.display(), stderr_trimmed(&out) ), } @@ -144,27 +137,19 @@ pub fn global_set(key: &str, value: &str) -> Result<()> { check(&out, &format!("`git config --global {key}`")) } -/// Unset a single-valued global key; a missing key (exit 5) is fine. -pub fn global_unset(key: &str) -> Result<()> { - let out = run(Command::new("git").args(["config", "--global", "--unset", key]))?; - match out.status.code() { - Some(0) | Some(5) => Ok(()), - _ => bail!( - "`git config --global --unset {key}` failed: {}", - stderr_trimmed(&out) - ), - } -} - -/// Remove every global value of `key` matching `value_regex`; no match -/// (exit 5) is fine. -pub fn global_unset_all_matching(key: &str, value_regex: &str) -> Result<()> { - let out = - run(Command::new("git").args(["config", "--global", "--unset-all", key, value_regex]))?; +/// Remove every value of `key` in a specific file matching `value_regex`; no +/// match (exit 5) is fine. +pub fn unset_all_matching_file(file: &Path, key: &str, value_regex: &str) -> Result<()> { + let out = run(Command::new("git") + .arg("config") + .arg("--file") + .arg(file) + .args(["--unset-all", key, value_regex]))?; match out.status.code() { Some(0) | Some(5) => Ok(()), _ => bail!( - "`git config --global --unset-all {key}` failed: {}", + "`git config --file {} --unset-all {key}` failed: {}", + file.display(), stderr_trimmed(&out) ), } diff --git a/src/paths.rs b/src/paths.rs index 556b97f..ca69cdb 100644 --- a/src/paths.rs +++ b/src/paths.rs @@ -287,6 +287,19 @@ mod tests { assert_eq!(git_path_windows("C:/Users/x"), "C:/Users/x"); } + #[test] + fn doctor_canonical_comparison_uses_git_path_form() { + // doctor recomputes a route's canonical path and compares it to the + // stored gitdir. On Windows fs::canonicalize yields `\\?\C:\...`, so the + // comparison must go through to_git_path (here: git_path_windows) or a + // correct route is falsely flagged as non-canonical. This mirrors the + // exact transformation doctor applies. + assert_eq!( + ensure_trailing_slash(git_path_windows("\\\\?\\C:\\Users\\jane\\dev")), + "C:/Users/jane/dev/" + ); + } + #[test] #[cfg(not(windows))] fn to_git_path_is_identity_on_unix() { diff --git a/src/routes.rs b/src/routes.rs index 9df3746..6285fc0 100644 --- a/src/routes.rs +++ b/src/routes.rs @@ -231,7 +231,7 @@ fn is_substantive(line: &str) -> bool { /// `[includeIf "gitdir:"]` -> `` (with subsection /// unescaping). Conditions other than plain case-sensitive `gitdir:` — /// e.g. `gitdir/i:`, `onbranch:`, `hasconfig:` — are treated as foreign. -fn parse_gitdir_header(line: &str) -> Option { +pub(crate) fn parse_gitdir_header(line: &str) -> Option { let t = line.trim(); let inner = t.strip_prefix('[')?.strip_suffix(']')?.trim(); let (section, rest) = inner.split_once('"')?; diff --git a/src/store.rs b/src/store.rs index 4a8a093..e3c2120 100644 --- a/src/store.rs +++ b/src/store.rs @@ -97,6 +97,19 @@ pub fn validate_user_name(name: &str) -> Result<()> { Ok(()) } +/// A signing key is free-form (GPG id, SSH key, `key::` spec…), but it must not +/// carry control characters: a newline in particular would let the value break +/// out of its line in the rendered fragment and inject an arbitrary gitconfig +/// section (e.g. `core.sshCommand`). Callers pass only non-empty keys here; +/// an empty value means "no key" / "remove the key". +pub fn validate_signing_key(key: &str) -> Result<()> { + ensure!( + !key.chars().any(char::is_control), + "signing key cannot contain control characters" + ); + Ok(()) +} + pub fn fragment_path(env: &Env, name: &str) -> PathBuf { env.identities_dir.join(format!("{name}.{FRAGMENT_EXT}")) } @@ -172,12 +185,17 @@ pub fn render_fragment(id: &Identity) -> String { } /// Quote a gitconfig value only when needed (`#`, `;`, quotes, backslashes, -/// or leading/trailing whitespace would otherwise be mangled by git). +/// control characters, or leading/trailing whitespace would otherwise be +/// mangled by — or break out of — the line git reads back). Control characters +/// are escaped to git's own forms (`\n`, `\t`, `\b`) so the value round-trips +/// and can never inject a new section; this is defense in depth on top of the +/// `validate_*` checks, so no field rendered through here can ever inject. fn quote_cfg_value(value: &str) -> String { let needs_quoting = value.is_empty() || value.starts_with([' ', '\t']) || value.ends_with([' ', '\t']) - || value.contains(['#', ';', '"', '\\']); + || value.contains(['#', ';', '"', '\\']) + || value.chars().any(char::is_control); if !needs_quoting { return value.to_string(); } @@ -186,6 +204,9 @@ fn quote_cfg_value(value: &str) -> String { .flat_map(|c| match c { '\\' => vec!['\\', '\\'], '"' => vec!['\\', '"'], + '\n' => vec!['\\', 'n'], + '\t' => vec!['\\', 't'], + '\u{8}' => vec!['\\', 'b'], c => vec![c], }) .collect(); @@ -196,6 +217,9 @@ pub fn write_new(env: &Env, id: &Identity, force: bool) -> Result<()> { validate_slug(&id.name)?; validate_user_name(&id.user_name)?; validate_email(&id.email)?; + if let Some(key) = &id.signing_key { + validate_signing_key(key)?; + } let path = fragment_path(env, &id.name); if path.exists() && !force { bail!( @@ -215,12 +239,23 @@ pub fn apply_patch(env: &Env, name: &str, patch: &IdentityPatch) -> Result<()> { path.is_file(), "identity `{name}` does not exist (see `git-id list`)" ); + // Validate every present field before writing any, so a rejected patch is a + // true no-op instead of leaving an earlier field already committed. if let Some(user_name) = &patch.user_name { validate_user_name(user_name)?; - gitcfg::set_file(&path, "user.name", user_name)?; } if let Some(email) = &patch.email { validate_email(email)?; + } + if let Some(key) = &patch.signing_key { + if !key.is_empty() { + validate_signing_key(key)?; + } + } + if let Some(user_name) = &patch.user_name { + gitcfg::set_file(&path, "user.name", user_name)?; + } + if let Some(email) = &patch.email { gitcfg::set_file(&path, "user.email", email)?; } if let Some(key) = &patch.signing_key { @@ -319,6 +354,39 @@ mod tests { assert_eq!(quote_cfg_value("plain name"), "plain name"); } + #[test] + fn signing_key_rejects_control_chars() { + for ok in ["ABCDEF12", "key::ssh-ed25519 AAAAC3Nza", "0x1234"] { + assert!(validate_signing_key(ok).is_ok(), "{ok} should be valid"); + } + assert!(validate_signing_key("KEY\n[core]\n\tsshCommand = x").is_err()); + assert!(validate_signing_key("a\tb").is_err()); + } + + #[test] + fn render_does_not_inject_a_section_via_control_chars() { + // Even if a control-laden value reaches the renderer (validation is the + // first line of defense), quoting/escaping must keep it on one line so + // git reads it back verbatim and no extra section leaks in. + let tmp = tempfile::TempDir::new().unwrap(); + let path = tmp.path().join("x.gitconfig"); + let id = Identity { + name: "x".into(), + user_name: "Jane".into(), + email: "j@b.co".into(), + signing_key: Some("KEY\n[core]\n\tsshCommand = touch PWNED".into()), + sign: false, + }; + atomic_write(&path, &render_fragment(&id)).unwrap(); + assert_eq!( + gitcfg::get_file(&path, "user.signingkey") + .unwrap() + .as_deref(), + Some("KEY\n[core]\n\tsshCommand = touch PWNED") + ); + assert_eq!(gitcfg::get_file(&path, "core.sshCommand").unwrap(), None); + } + #[test] fn rendered_fragment_roundtrips_through_git() { let tmp = tempfile::TempDir::new().unwrap(); diff --git a/tests/cli.rs b/tests/cli.rs index e66ea1d..e862ae2 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -886,3 +886,275 @@ fn completions_install_detects_shell_from_env() { "bash completion was not written via $SHELL detection" ); } + +// --------------------------------------------------------------------------- +// input validation / injection + +#[test] +fn create_rejects_control_chars_in_signing_key() { + let t = TestEnv::new(); + t.ok(&["init"]); + // A newline in the signing key would otherwise inject a gitconfig section + // (e.g. core.sshCommand) into the fragment git includes for routed repos. + t.cmd() + .args([ + "create", + "work", + "--name", + "Jane", + "--email", + "jane@work.example", + "--signing-key", + "KEY\n[core]\n\tsshCommand = touch PWNED", + ]) + .assert() + .failure() + .stderr(contains("control characters")); + assert!( + !t.fragment("work").exists(), + "no fragment must be written when validation fails" + ); +} + +// --------------------------------------------------------------------------- +// edit: signing-key removal, sign toggles, and atomic validation + +#[test] +fn edit_removes_signing_key_and_toggles_sign_preserving_manual_keys() { + let t = TestEnv::new(); + t.ok(&["init"]); + t.ok(&[ + "create", + "work", + "--name", + "Jane Doe", + "--email", + "jane@work.example", + "--signing-key", + "ABCDEF12", + "--sign", + ]); + // A hand-added key must survive edits. + let frag = t.fragment("work"); + t.git_ok( + &t.home, + &[ + "config", + "--file", + frag.to_str().unwrap(), + "core.sshCommand", + "ssh -i ~/.ssh/work", + ], + ); + + t.ok(&["edit", "work", "--signing-key", ""]); // remove the key + t.ok(&["edit", "work", "--no-sign"]); // turn signing off + + let json: serde_json::Value = serde_json::from_str(&t.ok(&["show", "work", "--json"])).unwrap(); + assert_eq!(json["user"]["signing_key"], serde_json::Value::Null); + assert_eq!(json["user"]["sign"], false); + + let content = t.read(&frag); + assert!( + !content.contains("signingkey"), + "signing key not removed:\n{content}" + ); + assert!( + content.contains("sshCommand"), + "manual key lost:\n{content}" + ); +} + +#[test] +fn edit_rejects_invalid_field_without_partial_write() { + let t = TestEnv::new(); + t.ok(&["init"]); + t.ok(&[ + "create", + "work", + "--name", + "Old Name", + "--email", + "old@work.example", + ]); + + // Valid name + invalid email: the whole patch must be rejected as a no-op, + // not leave the name already written. + t.cmd() + .args([ + "edit", + "work", + "--name", + "New Name", + "--email", + "not-an-email", + ]) + .assert() + .failure() + .stderr(contains("email")); + + let content = t.read(&t.fragment("work")); + assert!( + content.contains("Old Name") && !content.contains("New Name"), + "partial write occurred:\n{content}" + ); + assert!(content.contains("old@work.example")); +} + +// --------------------------------------------------------------------------- +// include detection across global config files (XDG vs ~/.gitconfig) + +#[test] +fn include_in_xdg_is_found_and_removed_after_home_gitconfig_appears() { + let t = TestEnv::new(); + // Force the include into the XDG git config: no ~/.gitconfig, but an + // existing $XDG_CONFIG_HOME/git/config makes that the --global write target. + let xdg_git = t.mkdirs(".config/git").join("config"); + fs::write(&xdg_git, "[init]\n\tdefaultBranch = main\n").unwrap(); + t.ok(&["init", "--no-use-config-only"]); + assert!(!t.gitconfig().exists()); + assert!(t.read(&xdg_git).contains("routes.gitconfig")); + + // A ~/.gitconfig appears later (e.g. another tool). `git config --global` + // would now go blind to the XDG include; git-id scans the files directly. + fs::write(t.gitconfig(), "[user]\n\tname = Someone\n").unwrap(); + + t.cmd() + .arg("doctor") + .assert() + .success() + .stdout(contains("includes the routes file")); + + t.ok(&["uninstall", "--yes"]); + let after = t.read(&xdg_git); + assert!( + !after.contains("routes.gitconfig"), + "the XDG include must be removed:\n{after}" + ); + assert!( + after.contains("defaultBranch"), + "unrelated XDG content must be preserved:\n{after}" + ); +} + +// --------------------------------------------------------------------------- +// init edge cases + +#[test] +fn init_useconfigonly_already_enabled_is_idempotent() { + let t = TestEnv::new(); + t.ok(&["init", "--use-config-only"]); + assert!(t.backups_of(&t.gitconfig()).is_empty()); + + let out = t.ok(&["init"]); + assert!(out.contains("already enabled"), "{out}"); + assert!( + t.backups_of(&t.gitconfig()).is_empty(), + "a no-op re-run must not take a backup" + ); +} + +#[test] +fn init_with_nonexistent_git_config_global_creates_it_without_backup() { + let t = TestEnv::new(); + let custom = t.home.join("fresh-global.gitconfig"); + t.cmd() + .env("GIT_CONFIG_GLOBAL", &custom) + .args(["init", "--no-use-config-only"]) + .assert() + .success(); + assert!(custom.exists(), "the override file should be created"); + assert!(t.read(&custom).contains("routes.gitconfig")); + assert!( + t.backups_of(&custom).is_empty(), + "a fresh (nonexistent) target must not be backed up" + ); + assert!(!t.gitconfig().exists(), "~/.gitconfig must not be created"); +} + +// --------------------------------------------------------------------------- +// doctor: path-form and duplicate diagnostics + +#[test] +fn doctor_flags_noncanonical_route() { + let t = TestEnv::new(); + t.setup_work(); + // Hand-write a route whose gitdir is non-canonical (contains `..`) but still + // resolves to an existing directory: doctor must flag it via the canonical + // comparison (which now runs in git-path form). + let home = t.git_path(&t.home); + let frag = t.git_path(&t.fragment("work")); + let routes = + format!("# header\n[includeIf \"gitdir:{home}/dev/work/../work/\"]\n\tpath = \"{frag}\"\n"); + fs::write(t.routes_file(), routes).unwrap(); + + t.cmd() + .arg("doctor") + .assert() + .stdout(contains("differs from the directory's canonical path")); +} + +#[test] +fn doctor_flags_duplicate_gitdir_across_preserved_and_managed() { + let t = TestEnv::new(); + let dir = t.setup_work(); + let gitdir = format!("{}/", t.git_path(&dir)); + let frag = t.git_path(&t.fragment("work")); + // Append a hand-edited includeIf for the SAME gitdir with an extra key, so + // it is bucketed as `preserved`, not a managed entry. git applies both. + let mut routes = t.read(&t.routes_file()); + routes.push_str(&format!( + "\n[includeIf \"gitdir:{gitdir}\"]\n\tpath = \"{frag}\"\n\tsshCommand = ssh -i x\n" + )); + fs::write(t.routes_file(), routes).unwrap(); + + t.cmd() + .arg("doctor") + .assert() + .failure() + .stdout(contains("routes exist for")); +} + +// --------------------------------------------------------------------------- +// use: linked worktree + +#[test] +fn use_warns_when_routing_a_linked_worktree() { + let t = TestEnv::new(); + t.ok(&["init"]); + t.ok(&[ + "create", + "work", + "--name", + "Jane", + "--email", + "jane@work.example", + ]); + + let main = t.init_repo("dev/main"); + // dev/main is not routed, so give the setup commit an explicit identity: + // CI runners (unlike a dev macOS box) have no auto-guessable git ident. + t.git_ok( + &main, + &[ + "-c", + "user.name=Test", + "-c", + "user.email=test@example.com", + "commit", + "--allow-empty", + "-m", + "init", + ], + ); + let wt = t.home.join("dev/wt"); + t.git_ok(&main, &["worktree", "add", wt.to_str().unwrap()]); + + // git matches routes against the main repo, so a route on the worktree's own + // path would never apply — `use` must say so instead of silently succeeding. + t.cmd() + .args(["use", "work", wt.to_str().unwrap()]) + .assert() + .success() + .stderr(contains("linked worktree")); +}