diff --git a/Cargo.lock b/Cargo.lock index 18c53c18ca0..2b7e937f430 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -831,12 +831,15 @@ dependencies = [ "anyhow", "base64 0.22.1", "buzz-core", + "buzz-git-identity", "buzz-persona", "buzz-sdk", "chrono", "clap", "evalexpr", "futures-util", + "git-credential-nostr", + "git-sign-nostr", "hex", "httparse", "nix 0.31.3", @@ -846,6 +849,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.11.0", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-tungstenite 0.29.0", @@ -1097,6 +1101,7 @@ dependencies = [ "base64 0.22.1", "buzz-cli", "buzz-core", + "buzz-git-identity", "git-credential-nostr", "git-sign-nostr", "ignore", @@ -1119,6 +1124,16 @@ dependencies = [ "zeroize", ] +[[package]] +name = "buzz-git-identity" +version = "0.1.0" +dependencies = [ + "nostr 0.44.7", + "tempfile", + "wait-timeout", + "zeroize", +] + [[package]] name = "buzz-media" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index 78816ff4827..c78d3488cfd 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,6 +24,7 @@ members = [ "crates/buzz-persona", "crates/git-credential-nostr", "crates/git-sign-nostr", + "crates/buzz-git-identity", "crates/buzz-pair-relay", "crates/buzz-relay-mesh", "crates/buzz-dev-mcp", diff --git a/crates/buzz-acp/Cargo.toml b/crates/buzz-acp/Cargo.toml index d047849806f..91382d63e5f 100644 --- a/crates/buzz-acp/Cargo.toml +++ b/crates/buzz-acp/Cargo.toml @@ -53,6 +53,10 @@ url = { workspace = true } sha2 = { workspace = true } base64 = "0.22" hex = { workspace = true } +buzz-git-identity = { path = "../buzz-git-identity" } +git-credential-nostr = { path = "../git-credential-nostr" } +git-sign-nostr = { path = "../git-sign-nostr" } +tempfile = "3" # Logging tracing = { workspace = true } diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index 0d87bca028c..cf3d90be73f 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -214,6 +214,12 @@ pub struct AcpClient { standard_usage: StandardUsageTracker, /// Known adapter identity for prompt-response usage mapping. standard_adapter: Option, + /// Session-scoped tempdir holding the agent's git identity keyfile and the + /// `git` enforcement-wrapper symlink prepended to the child's PATH. Present + /// only when `NOSTR_PRIVATE_KEY` was set at spawn. Deleted explicitly by + /// [`AcpClient::shutdown`] (the guaranteed-cleanup path); `Drop` is the + /// best-effort fallback for callers that never call `shutdown`. + _git_identity_dir: Option, } /// Recursively merge `overlay` into `base`, with `overlay` winning on scalar/shape @@ -413,6 +419,170 @@ fn build_client_capabilities() -> serde_json::Value { }) } +/// Install deterministic agent git identity onto the about-to-be-spawned agent +/// runtime child (L1b + the L2/L3 wrapper's PATH placement). +/// +/// Builds a session-scoped 0700 tempdir that holds: +/// - the agent's 0600 nostr keyfile (the signer/credential helper read it), +/// - the 0600 identity manifest the wrapper reads as its authority, and +/// - `git`, `git-sign-nostr`, `git-credential-nostr` symlinks back to this +/// binary's own exe, whose multicall dispatch (see [`crate::run`]) makes each +/// name resolve to the matching personality. +/// +/// It then prepends that dir to the child's `PATH` (so the wrapper `git` shadows +/// the real one and the nostr helpers are reachable) and applies the identity + +/// signing `GIT_CONFIG_*` env, composed over any config the caller already set +/// (the desktop's per-URL credential helper). The returned [`TempDir`] owns the +/// keyfile's lifetime and must be held for the life of the child. +/// +/// The agent secret is sourced from the canonical configured key with explicit +/// precedence: `BUZZ_PRIVATE_KEY` (the documented required secret) then the +/// `NOSTR_PRIVATE_KEY` the desktop stages. Eligibility does NOT depend on +/// independent `git-credential-nostr` discovery — this multicall binary IS that +/// helper — so a standard headless `BUZZ_PRIVATE_KEY=… buzz-acp …` launch gets +/// deterministic identity, closing the ambient-Will leak. `NOSTR_PRIVATE_KEY` +/// is also staged on the child (from the canonical key) so dev-mcp's shim, which +/// reads that var, installs the same identity for its own subtree. +/// +/// # Errors +/// +/// `Err` when a key is present but identity cannot be installed (tempdir/chmod/ +/// symlink/keyfile/manifest/PATH failure) — the caller MUST fail the managed +/// session closed rather than spawn an agent that commits as ambient Will. +/// `Ok(None)` only when no key is configured at all (test spawns, genuinely +/// unconfigured sessions): those legitimately spawn without injected identity, +/// and the wrapper (finding no manifest) passes through. +/// +/// Only wired on Unix. Windows agent hosting is not a supported harness surface; +/// the symlink/exec model the wrapper relies on does not exist there. +#[cfg(unix)] +fn install_git_identity( + cmd: &mut tokio::process::Command, +) -> std::io::Result> { + use std::io::{Error, ErrorKind}; + use std::os::unix::fs::{symlink, PermissionsExt}; + + // Operator's identity mode, read once here at spawn — never by the wrapper + // per-invocation, which would let any agent `export BUZZ_GIT_IDENTITY=user` + // mid-session and hollow out enforcement. A persona-staged value (on `cmd`) + // outranks the harness process env, so a per-agent `user` setting wins; + // absent both, the default is `agent`. An unrecognized value fails the + // spawn loudly rather than silently picking a mode. + let mode = buzz_git_identity::GitIdentityMode::from_value( + child_env(cmd, buzz_git_identity::GitIdentityMode::ENV_VAR) + .or_else(|| std::env::var_os(buzz_git_identity::GitIdentityMode::ENV_VAR)) + .as_deref(), + ) + .map_err(|e| Error::new(ErrorKind::InvalidInput, e))?; + + // `user` mode: install NO attribution machinery — no wrapper on PATH, no + // manifest, no keyfile, no injected identity/signing config. This is the + // existing, review-hardened unconfigured-session path: the child's git is + // vanilla git resolving the operator's own repo/global identity and + // signing. But it must still stage the canonical key below so the shim's + // credential-helper-only branch can authenticate to relay git — auth ≠ + // attribution holds on every launch path, not just desktop, and an operator + // who set `user` on a configured session has not unconfigured their auth. + + // Canonical agent key. `BUZZ_PRIVATE_KEY` (the documented secret) always + // outranks `NOSTR_PRIVATE_KEY`, at BOTH layers, so a stale or conflicting + // child-staged `NOSTR_PRIVATE_KEY` can never split identity away from the + // canonical `BUZZ_PRIVATE_KEY`. Within a var name, an explicitly cmd-staged + // value (a persona) wins over the ambient process env. Absent entirely → + // unconfigured session, spawn without identity (Ok(None)) in either mode. + let Some(raw_key) = child_env(cmd, "BUZZ_PRIVATE_KEY") + .or_else(|| std::env::var_os("BUZZ_PRIVATE_KEY")) + .or_else(|| child_env(cmd, "NOSTR_PRIVATE_KEY")) + .or_else(|| std::env::var_os("NOSTR_PRIVATE_KEY")) + else { + return Ok(None); + }; + let raw_key = raw_key + .into_string() + .map_err(|_| Error::new(ErrorKind::InvalidInput, "agent key is not valid UTF-8"))?; + + // Stage the canonical key as the child's `NOSTR_PRIVATE_KEY` unconditionally + // — overwriting any pre-staged value — so dev-mcp's shim (which reads that + // var) installs the SAME identity for its own subtree. Staging only when + // absent would let a conflicting child `NOSTR_PRIVATE_KEY` drive the shim to + // a different identity than the one enforced here. Staged in BOTH modes: + // `agent` needs it for the shim's full identity install, `user` needs it for + // the shim's credential-helper-only branch (relay git auth). + cmd.env("NOSTR_PRIVATE_KEY", &raw_key); + + // `user` mode stops here: the key is staged for the shim's credential + // helper, but no attribution machinery is installed — vanilla git resolves + // the operator's own identity. `Ok(None)` = no wrapper, manifest, or keyfile + // on the harness side. + if mode == buzz_git_identity::GitIdentityMode::User { + return Ok(None); + } + + let self_exe = std::env::current_exe()?; + + let dir = tempfile::Builder::new().prefix("buzz-acp-git-").tempdir()?; + std::fs::set_permissions(dir.path(), std::fs::Permissions::from_mode(0o700))?; + + // The wrapper `git` shadows the real one; the two nostr helpers back the + // signing + credential config. All resolve to this binary's multicall. + for name in ["git", "git-sign-nostr", "git-credential-nostr"] { + symlink(&self_exe, dir.path().join(name))?; + } + + // Persist the keyfile and derive the identity. An invalid/empty key here is + // a fatal misconfiguration for a managed session — signing would fail every + // commit — so surface it rather than spawn unattributed. + let id = buzz_git_identity::write_keyfile(dir.path(), &raw_key).ok_or_else(|| { + Error::new( + ErrorKind::InvalidData, + "agent nostr key is empty or invalid", + ) + })?; + + // Write the authoritative identity manifest the enforcement wrapper reads. + let identity = buzz_git_identity::identity_signing_entries(&id); + buzz_git_identity::write_identity_manifest(dir.path(), &identity)?; + + // Prepend the wrapper dir to the child's PATH. Prefer a cmd-staged PATH + // (a persona may set one), fall back to the process env, so we compose + // rather than clobber in both cases. + let base_path = child_env(cmd, "PATH") + .or_else(|| std::env::var_os("PATH")) + .unwrap_or_default(); + let mut entries = vec![dir.path().to_path_buf()]; + entries.extend(std::env::split_paths(&base_path)); + let joined = + std::env::join_paths(entries).map_err(|e| Error::new(ErrorKind::InvalidInput, e))?; + cmd.env("PATH", joined); + + // Identity + signing GIT_CONFIG_*, composed over any config already present + // (the desktop's per-URL credential helper at KEY_0/KEY_1). Our entries land + // at the next free indices so that helper is preserved. + for (key, value) in buzz_git_identity::to_git_config_env(&identity) { + cmd.env(key, value); + } + + Ok(Some(dir)) +} + +#[cfg(not(unix))] +fn install_git_identity( + _cmd: &mut tokio::process::Command, +) -> std::io::Result> { + Ok(None) +} + +/// Read a value previously staged on `cmd` via [`Command::env`], if any. Lets +/// the git-identity installer compose over a `PATH` the spawn path already put +/// on the child rather than clobbering it. +#[cfg(unix)] +fn child_env(cmd: &tokio::process::Command, key: &str) -> Option { + cmd.as_std() + .get_envs() + .find(|(k, _)| *k == std::ffi::OsStr::new(key)) + .and_then(|(_, v)| v.map(|v| v.to_owned())) +} + impl AcpClient { /// Kill the agent subprocess and wait for it to exit (no zombies). /// @@ -420,6 +590,19 @@ impl AcpClient { /// Call this when you need guaranteed cleanup — e.g., in `run_models` /// before process exit. pub async fn shutdown(&mut self) { + // Delete the git-identity tempdir (0600 nostr keyfile) explicitly and + // first. `TempDir`'s own `Drop` swallows its `remove_dir_all` error, and + // when the client is dropped right before `std::process::exit` on an + // error/timeout path that removal races the teardown and leaves the + // keyfile on disk ~80% of the time (the leak Gurney found). Closing it + // here — on the guaranteed-cleanup path, before the process-group kill — + // makes removal deterministic and surfaces any failure. + if let Some(dir) = self._git_identity_dir.take() { + let path = dir.path().to_path_buf(); + if let Err(e) = dir.close() { + tracing::warn!("git identity: keyfile cleanup failed for {path:?}: {e}"); + } + } // Kill the entire process group when possible. The child was spawned // with process_group(0), so its PID == its PGID. Killing the group // ensures subprocesses (MCP servers, tool processes) are cleaned up @@ -508,7 +691,14 @@ impl AcpClient { // Handled by build_codex_config_env; skip here to avoid double-setting. continue; } - if std::env::var_os(key).is_none() { + // `BUZZ_GIT_IDENTITY` is an operator-controlled per-agent exception to + // the parent-wins rule: a persona value must always reach the child so + // `install_git_identity`'s child-over-process lookup can honor a + // per-agent mode even when the harness process env sets a global one. + // Without this, a global `BUZZ_GIT_IDENTITY` silently defeats every + // per-agent override in both directions. + if key == buzz_git_identity::GitIdentityMode::ENV_VAR || std::env::var_os(key).is_none() + { cmd.env(key, value); } } @@ -516,6 +706,23 @@ impl AcpClient { cmd.env("CODEX_CONFIG", merged); } + // ── L1b: deterministic agent git identity for the whole runtime subtree ── + // + // buzz-dev-mcp's shim applies the identity+signing GIT_CONFIG_* only to + // its own shell-tool children. The native shells of claude-code / codex / + // goose never see it, so a bare `git commit` there resolves to whatever + // ambient identity the repo/global config carries (the leak that + // produced attribution gaps). Lift the same identity onto the agent + // runtime child so every native shell inherits it. + // + // Composed over the desktop's per-URL credential-helper GIT_CONFIG_* + // (base-offset preserved). NOSTR_PRIVATE_KEY is staged on the child so + // dev-mcp's shim installs the same identity for its subtree. When a key + // is configured but identity cannot be installed, fail the session + // closed (`?`) rather than spawn an agent that would commit as ambient + // Will; `Ok(None)` (no key at all) spawns unchanged. + let git_identity_dir = install_git_identity(&mut cmd)?; + // Spawn the agent in its own process group so SIGKILL doesn't propagate // to the harness's own process group on Unix. // tokio::process::Command::process_group is a stable tokio API (no extra imports needed). @@ -563,6 +770,7 @@ impl AcpClient { goose_usage: UsageTracker::default(), standard_usage: StandardUsageTracker::default(), standard_adapter, + _git_identity_dir: git_identity_dir, }) } @@ -3125,6 +3333,53 @@ mod tests { ); } + /// `BUZZ_GIT_IDENTITY` is an operator-controlled per-agent exception to the + /// parent-wins env merge: a persona value must always reach the child so the + /// child-over-process lookup in `install_git_identity` can honor a per-agent + /// mode even when the harness process env sets a conflicting global one. + /// Exercised through the real `AcpClient::spawn` merge loop (the unit tests + /// write directly to `Command` and bypass it). No agent key is set, so + /// `install_git_identity` returns early without installing anything — this + /// isolates the env-merge behavior. + #[cfg(unix)] + #[tokio::test] + async fn spawn_persona_git_identity_overrides_global_process_env() { + const VAR: &str = buzz_git_identity::GitIdentityMode::ENV_VAR; + + /// Restore the process-global `VAR` to its prior state on drop, so this + /// test neither clobbers a caller-supplied value nor leaks its own. + struct EnvGuard(Option); + impl Drop for EnvGuard { + fn drop(&mut self) { + match self.0.take() { + Some(prev) => std::env::set_var(VAR, prev), + None => std::env::remove_var(VAR), + } + } + } + let _guard = EnvGuard(std::env::var_os(VAR)); + + // parent=agent, persona=user → child sees the persona value. + std::env::set_var(VAR, "agent"); + let observed = + spawn_named_and_read_child_env("other-agent", VAR, &[(VAR.into(), "user".into())]) + .await; + assert_eq!( + observed, "user", + "persona `user` must override a global `agent` in the parent env" + ); + + // parent=user, persona=agent → child sees the persona value. + std::env::set_var(VAR, "user"); + let observed = + spawn_named_and_read_child_env("other-agent", VAR, &[(VAR.into(), "agent".into())]) + .await; + assert_eq!( + observed, "agent", + "persona `agent` must override a global `user` in the parent env" + ); + } + #[tokio::test] async fn idle_timeout_fires_on_silent_process() { let mut client = spawn_script("sleep 10").await; @@ -5027,4 +5282,135 @@ mod tests { "error must mention sandbox_workspace_write" ); } + + /// I5: `BUZZ_PRIVATE_KEY` (the documented secret) must win over a + /// conflicting `NOSTR_PRIVATE_KEY`, and the canonical key must be staged as + /// the child's `NOSTR_PRIVATE_KEY` — overwriting the conflicting value — so + /// the harness layer and dev-mcp's shim can never install split identities. + /// Both keys are staged on the command (which outranks the process env for + /// each name), so the test is deterministic regardless of ambient env. + #[cfg(unix)] + #[test] + fn install_git_identity_prefers_buzz_key_and_restages_nostr() { + use nostr::ToBech32; + + let buzz_keys = nostr::Keys::generate(); + let nostr_keys = nostr::Keys::generate(); + let buzz_nsec = buzz_keys.secret_key().to_bech32().unwrap(); + let nostr_nsec = nostr_keys.secret_key().to_bech32().unwrap(); + assert_ne!(buzz_nsec, nostr_nsec, "distinct conflicting keys"); + let buzz_hex = buzz_keys.public_key().to_hex(); + let nostr_hex = nostr_keys.public_key().to_hex(); + + let mut cmd = tokio::process::Command::new("true"); + cmd.env("BUZZ_PRIVATE_KEY", &buzz_nsec); + cmd.env("NOSTR_PRIVATE_KEY", &nostr_nsec); + + let dir = install_git_identity(&mut cmd) + .expect("install must succeed with a valid key") + .expect("a configured key must install identity"); + + // The manifest — the wrapper's authority — must name the BUZZ key. Assert + // on the pubkey (the stable identity), not the host, so the test does not + // depend on a process-global `BUZZ_RELAY_URL` other tests may mutate. + let entries = + buzz_git_identity::read_identity_manifest(dir.path()).expect("manifest written"); + let email = entries + .iter() + .find(|(k, _)| k == "user.email") + .map(|(_, v)| v.clone()) + .expect("manifest has user.email"); + assert!( + email.starts_with(&format!("{buzz_hex}@")), + "BUZZ_PRIVATE_KEY must outrank the conflicting NOSTR_PRIVATE_KEY; got {email:?}" + ); + assert!( + !email.contains(&nostr_hex), + "the conflicting NOSTR key must not have won; got {email:?}" + ); + + // The child's NOSTR_PRIVATE_KEY must have been overwritten to the + // canonical (BUZZ) key, not left at the conflicting staged value. + let staged = child_env(&cmd, "NOSTR_PRIVATE_KEY") + .and_then(|v| v.into_string().ok()) + .expect("NOSTR_PRIVATE_KEY staged on child"); + assert_eq!( + staged, buzz_nsec, + "child NOSTR_PRIVATE_KEY must be the canonical BUZZ key, so dev-mcp's shim \ + installs the same identity" + ); + } + + /// `BUZZ_GIT_IDENTITY=user` (staged per-agent on the command) installs no + /// attribution machinery even with a valid key present, so the child's git + /// resolves the operator's own identity — but the canonical key is still + /// staged as the child's `NOSTR_PRIVATE_KEY` so the shim's credential helper + /// can authenticate to relay git (auth ≠ attribution on every launch path). + /// The staged mode value outranks the harness process env, proving + /// per-agent control. + #[cfg(unix)] + #[test] + fn install_git_identity_user_mode_installs_nothing_but_stages_key() { + use nostr::ToBech32; + + let nsec = nostr::Keys::generate().secret_key().to_bech32().unwrap(); + let mut cmd = tokio::process::Command::new("true"); + cmd.env("BUZZ_PRIVATE_KEY", &nsec); + cmd.env("BUZZ_GIT_IDENTITY", "user"); + + let dir = install_git_identity(&mut cmd).expect("user mode must not error"); + assert!( + dir.is_none(), + "user mode must install no identity dir (unconfigured path)" + ); + // No identity/signing config staged: the child git is vanilla git. + assert!( + child_env(&cmd, "GIT_CONFIG_COUNT").is_none(), + "user mode must not inject GIT_CONFIG_* identity/signing config" + ); + // The canonical key IS staged so the shim's credential helper works. + let staged = child_env(&cmd, "NOSTR_PRIVATE_KEY") + .and_then(|v| v.into_string().ok()) + .expect("user mode must stage NOSTR_PRIVATE_KEY for the credential helper"); + assert_eq!( + staged, nsec, + "user mode must stage the canonical key so relay git auth still works" + ); + } + + /// `agent` (explicit) still enforces: a valid key installs the identity dir + /// and injects config, identical to the unset default. + #[cfg(unix)] + #[test] + fn install_git_identity_agent_mode_enforces() { + use nostr::ToBech32; + + let nsec = nostr::Keys::generate().secret_key().to_bech32().unwrap(); + let mut cmd = tokio::process::Command::new("true"); + cmd.env("BUZZ_PRIVATE_KEY", &nsec); + cmd.env("BUZZ_GIT_IDENTITY", "agent"); + + let dir = install_git_identity(&mut cmd) + .expect("agent mode must not error") + .expect("agent mode with a key must install identity"); + assert!( + buzz_git_identity::read_identity_manifest(dir.path()).is_some(), + "agent mode must write the enforcement manifest" + ); + } + + /// An unrecognized value fails the spawn loudly rather than silently + /// picking a mode — the #3140 failure class. + #[cfg(unix)] + #[test] + fn install_git_identity_rejects_invalid_mode() { + let mut cmd = tokio::process::Command::new("true"); + cmd.env("BUZZ_GIT_IDENTITY", "usr"); + let err = install_git_identity(&mut cmd).expect_err("invalid mode must error"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); + assert!( + err.to_string().contains("BUZZ_GIT_IDENTITY"), + "error must name the var; got {err}" + ); + } } diff --git a/crates/buzz-acp/src/base_prompt.md b/crates/buzz-acp/src/base_prompt.md index 7a979b62e0c..57298adf292 100644 --- a/crates/buzz-acp/src/base_prompt.md +++ b/crates/buzz-acp/src/base_prompt.md @@ -134,7 +134,7 @@ These are guidelines, not a fixed procedure — apply judgment to the task in fr - After selecting a repository or worktree, read its root `AGENTS.md` and any path-local `AGENTS.md` files that apply before planning or editing. The workspace-level file is team context; it does not replace repository-owned instructions. - Treat repository-owned product, architecture, and vision documents as design constraints, not optional background. Read the relevant documents before making non-trivial plans, and surface any intentional conflict with them. - Make file changes in a worktree, not on the default branch. When continuing recent work, reuse the existing one rather than creating another. -- Before committing, read the repo-local git `user.name` / `user.email`; if email is empty, stop and ask. Include the trailers the repo requires. +- Your commit author identity is machine-managed: every commit is automatically authored and signed as your agent identity (`@`). Never set `user.name`/`user.email`, and never pass `-c user.*`, `--author`, or `--reset-author` — the managed `git` rejects those. Credit the human operator with the `Co-authored-by` and `Signed-off-by` trailers the repo requires (add them to the commit message body); if you cannot determine the operator's email for those trailers, stop and ask. (When the operator sets `BUZZ_GIT_IDENTITY=user`, commits instead carry their own git identity and these trailers are redundant.) ## Autonomy diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 146214197a8..94ce5bfcb0f 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1893,10 +1893,47 @@ mod idle_pool_sleep_tests { } pub fn run() -> Result<()> { + // Multicall git-helper personalities — when the harness binary is invoked + // under one of these names (via the symlinks it installs on the + // agent-runtime child's PATH; see `AcpClient::install_git_identity`), it + // dispatches to that personality and exits before any harness setup. This + // makes buzz-acp self-contained for deterministic agent git identity: the + // enforcement wrapper AND the nostr signer/credential helper it configures + // are all reachable from a single binary, with no dependency on a separately + // resolvable buzz-dev-mcp. Mirrors buzz-dev-mcp's own shim dispatch so the + // native shells of every runtime hit the same identity-enforcing git. + match git_multicall_personality() { + Some(GitPersonality::Git) => std::process::exit(buzz_git_identity::git_wrapper::run()), + Some(GitPersonality::SignNostr) => std::process::exit(git_sign_nostr::run()), + Some(GitPersonality::CredentialNostr) => std::process::exit(git_credential_nostr::run()), + None => {} + } config::propagate_legacy_env_vars(); tokio_main() } +/// A git-helper multicall personality this binary can assume based on argv[0]. +enum GitPersonality { + Git, + SignNostr, + CredentialNostr, +} + +/// The multicall personality implied by argv[0]'s file stem, or `None` when the +/// binary was launched normally as `buzz-acp`. +fn git_multicall_personality() -> Option { + let stem = std::env::args_os() + .next() + .map(std::path::PathBuf::from) + .and_then(|p| p.file_stem().map(|s| s.to_ascii_lowercase()))?; + match stem.to_str()? { + "git" => Some(GitPersonality::Git), + "git-sign-nostr" => Some(GitPersonality::SignNostr), + "git-credential-nostr" => Some(GitPersonality::CredentialNostr), + _ => None, + } +} + #[tokio::main] async fn tokio_main() -> Result<()> { // Install the ring crypto provider for rustls (required for wss:// connections). @@ -4790,6 +4827,18 @@ fn extract_auth_methods(init_result: &serde_json::Value) -> Vec ! { + client.shutdown().await; + eprintln!("{msg}"); + std::process::exit(code) +} + /// `buzz-acp auth-methods` — spawn an adapter, initialize it, print authMethods. async fn run_auth_methods(args: AuthMethodsArgs) -> Result<()> { let mut client = match spawn_auth_client(&args.agent).await { @@ -4803,14 +4852,15 @@ async fn run_auth_methods(args: AuthMethodsArgs) -> Result<()> { let init_result = match tokio::time::timeout(MODELS_TIMEOUT, client.initialize()).await { Ok(Ok(result)) => result, Ok(Err(e)) => { - client.shutdown().await; - eprintln!("error: agent initialize failed: {e}"); - std::process::exit(1); + shutdown_and_exit(client, &format!("error: agent initialize failed: {e}"), 1).await; } Err(_) => { - client.shutdown().await; - eprintln!("error: agent timed out ({MODELS_TIMEOUT:?})"); - std::process::exit(1); + shutdown_and_exit( + client, + &format!("error: agent timed out ({MODELS_TIMEOUT:?})"), + 1, + ) + .await; } }; @@ -4851,14 +4901,15 @@ async fn run_authenticate(args: AuthenticateArgs) -> Result<()> { let init_result = match tokio::time::timeout(MODELS_TIMEOUT, client.initialize()).await { Ok(Ok(result)) => result, Ok(Err(e)) => { - client.shutdown().await; - eprintln!("error: agent initialize failed: {e}"); - std::process::exit(1); + shutdown_and_exit(client, &format!("error: agent initialize failed: {e}"), 1).await; } Err(_) => { - client.shutdown().await; - eprintln!("error: agent initialize timed out ({MODELS_TIMEOUT:?})"); - std::process::exit(1); + shutdown_and_exit( + client, + &format!("error: agent initialize timed out ({MODELS_TIMEOUT:?})"), + 1, + ) + .await; } }; @@ -4866,12 +4917,15 @@ async fn run_authenticate(args: AuthenticateArgs) -> Result<()> { .iter() .any(|method| method.get("id").and_then(|id| id.as_str()) == Some(args.method_id.as_str())); if !supports_method { - client.shutdown().await; - eprintln!( - "error: auth method '{}' is not advertised by this adapter", - args.method_id - ); - std::process::exit(1); + shutdown_and_exit( + client, + &format!( + "error: auth method '{}' is not advertised by this adapter", + args.method_id + ), + 1, + ) + .await; } let result = @@ -4883,14 +4937,15 @@ async fn run_authenticate(args: AuthenticateArgs) -> Result<()> { Ok(()) } Ok(Err(e)) => { - client.shutdown().await; - eprintln!("error: authenticate failed: {e}"); - std::process::exit(1); + shutdown_and_exit(client, &format!("error: authenticate failed: {e}"), 1).await; } Err(_) => { - client.shutdown().await; - eprintln!("error: authenticate timed out ({AUTHENTICATE_TIMEOUT:?})"); - std::process::exit(1); + shutdown_and_exit( + client, + &format!("error: authenticate timed out ({AUTHENTICATE_TIMEOUT:?})"), + 1, + ) + .await; } } } @@ -4926,14 +4981,20 @@ async fn run_models(args: ModelsArgs) -> Result<()> { let (init_result, session_resp) = match protocol_result { Ok(Ok(tuple)) => tuple, Ok(Err(e)) => { - client.shutdown().await; - eprintln!("error: agent communication failed: {e}"); - std::process::exit(1); + shutdown_and_exit( + client, + &format!("error: agent communication failed: {e}"), + 1, + ) + .await; } Err(_) => { - client.shutdown().await; - eprintln!("error: agent timed out ({MODELS_TIMEOUT:?})"); - std::process::exit(1); + shutdown_and_exit( + client, + &format!("error: agent timed out ({MODELS_TIMEOUT:?})"), + 1, + ) + .await; } }; diff --git a/crates/buzz-acp/tests/git_identity_enforcement.rs b/crates/buzz-acp/tests/git_identity_enforcement.rs new file mode 100644 index 00000000000..db76df0198f --- /dev/null +++ b/crates/buzz-acp/tests/git_identity_enforcement.rs @@ -0,0 +1,605 @@ +//! Process-level, mutation-sensitive regression for the deterministic +//! agent-git-identity enforcement wrapper. Unlike the unit tests in +//! `buzz-git-identity`, this exercises the REAL multicall binary: `buzz-acp` +//! symlinked as `git`, invoked exactly as an agent's shell would invoke it, +//! with a `.git-identity` manifest beside the symlink (the harness-owned +//! authority) and the real `git` reachable later on PATH. +//! +//! Each test targets one enforcement layer and is designed to go RED if that +//! layer is deleted: +//! * `enforce` — flag-based identity/signing override is rejected. +//! * `verify_push` — a human-authored outgoing commit cannot be pushed. +//! * `apply_authority_env`— the agent identity is re-applied over caller/repo +//! config (the env-var override vector), so commits land agent-authored +//! even when repo-local config names a human. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +#[cfg(unix)] +use nostr::ToBech32; + +const AGENT_EMAIL: &str = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@relay.test"; +const ALIAS_HOP_LIMIT: usize = 10; + +/// Directory of the first real `git` on PATH; the wrapper is installed ahead +/// of it so `find_real_git` skips our shim symlink and reaches this one. +fn real_git_dir() -> PathBuf { + for dir in std::env::split_paths(&std::env::var_os("PATH").unwrap_or_default()) { + let cand = dir.join("git"); + if cand.is_file() { + return dir; + } + } + panic!("no real git on PATH"); +} + +/// Build a shim dir containing `git` -> the buzz-acp multicall binary and a +/// `.git-identity` manifest, and a PATH with the shim ahead of real git. +/// Returns (shim TempDir, PATH string). +fn shim_env(manifest: &str) -> (tempfile::TempDir, String) { + let shim = tempfile::tempdir().unwrap(); + let git_link = shim.path().join("git"); + #[cfg(unix)] + std::os::unix::fs::symlink(env!("CARGO_BIN_EXE_buzz-acp"), &git_link).unwrap(); + #[cfg(not(unix))] + std::fs::copy(env!("CARGO_BIN_EXE_buzz-acp"), &git_link).unwrap(); + + std::fs::write(shim.path().join(".git-identity"), manifest).unwrap(); + + let real = real_git_dir(); + let path = std::env::join_paths([shim.path().to_path_buf(), real]) + .unwrap() + .into_string() + .unwrap(); + (shim, path) +} + +/// Standard managed manifest with signing OFF (the test box has no nostr +/// signer; signing enforcement is covered by unit tests). +fn manifest() -> String { + format!("user.name=Agent\nuser.email={AGENT_EMAIL}\ncommit.gpgSign=false\n") +} + +/// A git repo with one human-authored commit and human-named local config. +fn human_repo() -> tempfile::TempDir { + let d = tempfile::tempdir().unwrap(); + let p = d.path(); + let g = |args: &[&str]| { + let ok = Command::new("git") + .args(args) + .current_dir(p) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .status() + .unwrap() + .success(); + assert!(ok, "git {args:?} failed"); + }; + g(&["init", "-q", "-b", "main"]); + g(&["config", "user.name", "Human Dev"]); + g(&["config", "user.email", "human@example.com"]); + g(&["config", "commit.gpgSign", "false"]); + std::fs::write(p.join("f"), "one").unwrap(); + g(&["add", "f"]); + g(&["commit", "-qm", "human commit"]); + d +} + +/// A fresh repo with a staged file but no commit object. +fn unborn_repo() -> tempfile::TempDir { + let d = tempfile::tempdir().unwrap(); + let p = d.path(); + let g = |args: &[&str]| { + let ok = Command::new("git") + .args(args) + .current_dir(p) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .status() + .unwrap() + .success(); + assert!(ok, "git {args:?} failed"); + }; + g(&["init", "-q", "-b", "main"]); + g(&["config", "user.name", "Human Dev"]); + g(&["config", "user.email", "human@example.com"]); + g(&["config", "commit.gpgSign", "false"]); + std::fs::write(p.join("f"), "staged").unwrap(); + g(&["add", "f"]); + d +} + +/// Number of commit objects in `repo`, including unreachable objects. +fn commit_object_count(repo: &Path) -> usize { + let out = Command::new("git") + .args([ + "-C", + repo.to_str().unwrap(), + "cat-file", + "--batch-all-objects", + "--batch-check", + ]) + .output() + .unwrap(); + assert!(out.status.success(), "enumerating git objects failed"); + String::from_utf8_lossy(&out.stdout) + .lines() + .filter(|line| line.split_whitespace().nth(1) == Some("commit")) + .count() +} + +/// Invoke the wrapper (`git` on the shim PATH) with `args`, in `cwd`. +fn wrapper(path: &str, cwd: &Path, args: &[&str]) -> std::process::Output { + Command::new("git") + .args(args) + .current_dir(cwd) + .env("PATH", path) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .output() + .expect("run wrapper git") +} + +/// The current `HEAD` commit SHA of `repo`, via real git (empty if unborn). +fn head_sha(repo: &Path) -> String { + let out = Command::new("git") + .args(["-C", repo.to_str().unwrap(), "rev-parse", "HEAD"]) + .output() + .unwrap(); + if !out.status.success() { + return String::new(); + } + String::from_utf8_lossy(&out.stdout).trim().to_string() +} + +#[test] +fn wrapper_rejects_flag_based_identity_override() { + let (_shim, path) = shim_env(&manifest()); + let repo = human_repo(); + std::fs::write(repo.path().join("f"), "two").unwrap(); + wrapper(&path, repo.path(), &["add", "f"]); + + let out = wrapper( + &path, + repo.path(), + &["-c", "user.email=evil@example.com", "commit", "-m", "x"], + ); + assert!( + !out.status.success(), + "override commit should be rejected; stdout={} stderr={}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr), + ); + assert!( + String::from_utf8_lossy(&out.stderr).contains("machine-managed"), + "expected the loud enforce message; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); +} + +#[test] +fn wrapper_refuses_to_push_human_authored_commit() { + let (_shim, path) = shim_env(&manifest()); + let repo = human_repo(); + // A reachable bare remote so the dry-run plan resolves and HEAD (human + // authored) is examined as an offender. + let remote = tempfile::tempdir().unwrap(); + assert!(Command::new("git") + .args(["init", "-q", "--bare", remote.path().to_str().unwrap()]) + .status() + .unwrap() + .success()); + wrapper( + &path, + repo.path(), + &["remote", "add", "origin", remote.path().to_str().unwrap()], + ); + + let out = wrapper(&path, repo.path(), &["push", "origin", "main"]); + assert!( + !out.status.success(), + "pushing a human-authored commit must be refused; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + assert!( + String::from_utf8_lossy(&out.stderr).contains("not authored by your agent identity"), + "expected the push-gate rejection; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + // The bare remote must have received nothing. + let refs = Command::new("git") + .args(["-C", remote.path().to_str().unwrap(), "for-each-ref"]) + .output() + .unwrap(); + assert!( + refs.stdout.is_empty(), + "no ref should have reached the remote: {}", + String::from_utf8_lossy(&refs.stdout), + ); +} + +/// R4 real-wrapper regression for the allowlist alias guard. Two Thufir p3 +/// bypass probes must be refused through the actual `buzz-acp`-as-`git` +/// multicall, and neither may create a commit: +/// +/// (a) a *quoted* config alias — git's quote-aware parser dequotes `'-c'` +/// `'user.email=…'` into real `-c` config that the whitespace-naive +/// round-3 scan missed; +/// (b) a *shell* (`!`) commit alias — git runs it with real git ahead of the +/// wrapper on PATH, so its inner `-c` re-authors the commit. +/// +/// A plain-subcommand alias must still resolve and commit as the agent +/// identity, proving the allowlist did not over-reject Gurney's working shapes. +#[test] +fn wrapper_rejects_quoted_and_shell_aliases_and_allows_plain_alias() { + let (_shim, path) = shim_env(&manifest()); + let repo = human_repo(); + + // (a) quoted config alias — the parser-parity bypass. + wrapper( + &path, + repo.path(), + &[ + "config", + "alias.quoted", + "'-c' 'user.name=QuotedHuman' '-c' 'user.email=quoted@human.test' '-c' 'commit.gpgSign=false' commit", + ], + ); + // (b) shell commit alias — git prepends real git to PATH for `!` bodies. + wrapper( + &path, + repo.path(), + &[ + "config", + "alias.sc", + "!f(){ git -c user.name=ShellHuman -c user.email=shell@human.test -c commit.gpgSign=false commit \"$@\"; }; f", + ], + ); + // A plain-subcommand alias that must keep working. + wrapper(&path, repo.path(), &["config", "alias.ci", "commit"]); + + let head_before = head_sha(repo.path()); + std::fs::write(repo.path().join("f"), "two").unwrap(); + wrapper(&path, repo.path(), &["add", "f"]); + + // (a) refused, no commit created. + let out = wrapper(&path, repo.path(), &["quoted", "-m", "via quoted alias"]); + assert!( + !out.status.success(), + "quoted config alias must be refused; stdout={} stderr={}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr), + ); + assert_eq!( + head_sha(repo.path()), + head_before, + "the refused quoted alias must not create a commit" + ); + + // (b) refused, no commit created. + let out = wrapper(&path, repo.path(), &["sc", "-m", "via shell alias"]); + assert!( + !out.status.success(), + "shell commit alias must be refused; stdout={} stderr={}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr), + ); + assert!( + String::from_utf8_lossy(&out.stderr).contains("shell (`!`) git alias"), + "expected the shell-alias rejection; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + assert_eq!( + head_sha(repo.path()), + head_before, + "the refused shell alias must not create a commit" + ); + + // The plain alias must still resolve and commit as the agent identity. + let out = wrapper(&path, repo.path(), &["ci", "-m", "via plain alias"]); + assert!( + out.status.success(), + "plain-subcommand alias must still commit; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + let author = Command::new("git") + .args([ + "-C", + repo.path().to_str().unwrap(), + "show", + "-s", + "--format=%ae", + "HEAD", + ]) + .output() + .unwrap(); + assert_eq!( + String::from_utf8_lossy(&author.stdout).trim(), + AGENT_EMAIL, + "plain-alias commit must be authored as the agent identity" + ); +} + +/// R5 real-wrapper regression for the alias-unification fix (Thufir's rd-4 +/// IMPORTANT). A bare-word alias whose body carries identity/signing *flags* +/// passes the allowlist (every token is a plain bare word), but after the alias +/// is expanded the wrapper holds the expansion to the SAME `enforce`/author +/// policy as a directly-typed command — so the alias can do no more than its +/// expansion could. Three shapes, each of which is refused when typed directly, +/// must therefore be refused through the alias too, with `HEAD` unchanged: +/// +/// (a) Thufir's exact probe — `--author` (split form) plus `--no-gpg-sign`; +/// (b) `--no-gpg-sign` alone, pinning that the fix is not one hard-coded string; +/// (c) an alias *chain* that resolves to `commit --no-gpg-sign` through two +/// hops, pinning that unification applies to the final accumulated command. +#[test] +fn wrapper_rejects_bare_word_alias_carried_identity_and_signing_flags() { + let (_shim, path) = shim_env(&manifest()); + let repo = human_repo(); + + // (a) Thufir's exact bypass probe — bare-word `--author`/`--no-gpg-sign`. + wrapper( + &path, + repo.path(), + &[ + "config", + "alias.human", + "commit --author Human --no-gpg-sign", + ], + ); + // (b) `--no-gpg-sign` alone. + wrapper( + &path, + repo.path(), + &["config", "alias.unsign", "commit --no-gpg-sign"], + ); + // (c) an alias chain: `chain` → `co --no-gpg-sign` → `commit --no-gpg-sign`. + wrapper(&path, repo.path(), &["config", "alias.co", "commit"]); + wrapper( + &path, + repo.path(), + &["config", "alias.chain", "co --no-gpg-sign"], + ); + + let head_before = head_sha(repo.path()); + std::fs::write(repo.path().join("f"), "two").unwrap(); + wrapper(&path, repo.path(), &["add", "f"]); + + for (alias, label) in [ + ("human", "author+no-gpg-sign alias"), + ("unsign", "no-gpg-sign-only alias"), + ("chain", "chained no-gpg-sign alias"), + ] { + let out = wrapper(&path, repo.path(), &[alias, "-m", "leak"]); + assert!( + !out.status.success(), + "{label} must be refused; stdout={} stderr={}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr), + ); + assert!( + String::from_utf8_lossy(&out.stderr).contains("machine-managed"), + "{label} must give the identity/signing rejection; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + assert_eq!( + head_sha(repo.path()), + head_before, + "{label} must not create a commit" + ); + } +} + +#[test] +fn wrapper_refuses_alias_chain_beyond_limit_and_allows_exact_limit() { + let (_shim, path) = shim_env(&manifest()); + let repo = unborn_repo(); + + // Exactly ALIAS_HOP_LIMIT substitutions end at real `commit`, so the wrapper + // must preserve the boundary's useful side: it resolves and commits under + // the managed agent identity. + for index in 0..ALIAS_HOP_LIMIT { + let name = format!("at{index}"); + let next = if index + 1 == ALIAS_HOP_LIMIT { + "commit".to_string() + } else { + format!("at{}", index + 1) + }; + wrapper( + &path, + repo.path(), + &["config", &format!("alias.{name}"), &next], + ); + } + let out = wrapper(&path, repo.path(), &["at0", "-m", "at the alias limit"]); + assert!( + out.status.success(), + "chain at the limit must reach the real command; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + let author = Command::new("git") + .args([ + "-C", + repo.path().to_str().unwrap(), + "show", + "-s", + "--format=%ae", + "HEAD", + ]) + .output() + .unwrap(); + assert_eq!(String::from_utf8_lossy(&author.stdout).trim(), AGENT_EMAIL); + + // Thufir's limit+1 counterexample: the wrapper must not hand a partial + // expansion to git. A human-author `commit` beyond the bound is refused + // before git runs, leaving the fresh repo unborn with no commit objects. + let beyond = unborn_repo(); + for index in 0..=ALIAS_HOP_LIMIT { + let name = format!("a{index}"); + let next = if index == ALIAS_HOP_LIMIT { + "commit --author Human".to_string() + } else { + format!("a{}", index + 1) + }; + wrapper( + &path, + beyond.path(), + &["config", &format!("alias.{name}"), &next], + ); + } + let out = wrapper(&path, beyond.path(), &["a0", "-m", "leak"]); + assert!( + !out.status.success(), + "chain past the limit must be refused; stdout={} stderr={}", + String::from_utf8_lossy(&out.stdout), + String::from_utf8_lossy(&out.stderr), + ); + assert!( + String::from_utf8_lossy(&out.stderr) + .contains(&format!("after {ALIAS_HOP_LIMIT} expansions")), + "expected the alias-limit refusal; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + let beyond_head = head_sha(beyond.path()); + assert!( + beyond_head.is_empty(), + "HEAD must remain unborn; HEAD={beyond_head:?}; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + assert_eq!( + commit_object_count(beyond.path()), + 0, + "the refused chain must not create an unreachable commit object" + ); +} + +#[test] +fn wrapper_reapplies_agent_identity_over_repo_config() { + // The env-var / repo-config override vector: repo-local config names a + // human, yet the wrapper re-appends the agent identity at the highest + // GIT_CONFIG_* index, so the resulting commit is agent-authored. Deleting + // `apply_authority_env` makes this commit land as `human@example.com`. + let (_shim, path) = shim_env(&manifest()); + let repo = human_repo(); + std::fs::write(repo.path().join("f"), "two").unwrap(); + wrapper(&path, repo.path(), &["add", "f"]); + + let out = wrapper(&path, repo.path(), &["commit", "-m", "agent authored"]); + assert!( + out.status.success(), + "ordinary commit should succeed; stderr={}", + String::from_utf8_lossy(&out.stderr), + ); + + let author = Command::new("git") + .args([ + "-C", + repo.path().to_str().unwrap(), + "show", + "-s", + "--format=%ae", + "HEAD", + ]) + .output() + .unwrap(); + assert_eq!( + String::from_utf8_lossy(&author.stdout).trim(), + AGENT_EMAIL, + "commit must be authored as the agent identity, not the repo-local human" + ); +} + +/// I4: the spawn-path wiring — `AcpClient::spawn` → `install_git_identity` — +/// must actually install the wrapper + manifest onto the agent-runtime child. +/// +/// The tests above manufacture their own `.git-identity` manifest, so they stay +/// green even if the `install_git_identity(&mut cmd)?` call in `spawn` is +/// deleted. This one drives the REAL `buzz-acp` binary through `buzz-acp models` +/// (whose spawn path is the code under test) with a script agent that runs a +/// bare `git commit` in a human-configured repo and records the resulting +/// author. It passes only when the spawn path installed the wrapper `git` ahead +/// of real git AND wrote a manifest naming the configured key's identity — so +/// removing the `install_git_identity` call makes it go RED (the commit lands as +/// the repo-local human, or fails). +/// +/// `BUZZ_AUTH_TAG` is cleared so `git-sign-nostr` signs offline (no NIP-OA owner +/// attestation to verify against a relay); signing itself needs no network. +#[cfg(unix)] +#[test] +fn spawn_path_installs_identity_so_agent_commits_land_agent_authored() { + use std::os::unix::fs::PermissionsExt; + + // A configured agent key and its derived author email (the wrapper builds + // `@` from BUZZ_RELAY_URL). + let keys = nostr::Keys::generate(); + let nsec = keys.secret_key().to_bech32().unwrap(); + let pubkey_hex = keys.public_key().to_hex(); + let expected_email = format!("{pubkey_hex}@relay.test"); + + // A human-configured repo with a staged file, ready for one commit. + let work = tempfile::tempdir().unwrap(); + let repo = work.path().join("repo"); + std::fs::create_dir_all(&repo).unwrap(); + let g = |args: &[&str]| { + assert!(Command::new("git") + .args(args) + .current_dir(&repo) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .status() + .unwrap() + .success()); + }; + g(&["init", "-q", "-b", "main"]); + g(&["config", "user.name", "Human Dev"]); + g(&["config", "user.email", "human@example.com"]); + std::fs::write(repo.join("f"), "hi").unwrap(); + g(&["add", "f"]); + + // Script "agent": commit in the repo using whatever `git` its PATH resolves + // (the wrapper, if the spawn path installed it), record the author, exit. + let out_file = work.path().join("author.txt"); + let agent = work.path().join("agent.sh"); + std::fs::write( + &agent, + format!( + "#!/usr/bin/env bash\n\ + cd {repo:?}\n\ + git commit -m 'agent authored' >/dev/null 2>&1\n\ + git show -s --format=%ae HEAD > {out:?} 2>/dev/null\n\ + exit 0\n", + repo = repo, + out = out_file, + ), + ) + .unwrap(); + std::fs::set_permissions(&agent, std::fs::Permissions::from_mode(0o755)).unwrap(); + + // Drive the real binary. `models` spawns the agent (running install_git_identity), + // then fails init (the script exits) — expected; we assert on the side effect. + let output = Command::new(env!("CARGO_BIN_EXE_buzz-acp")) + .args([ + "models", + "--agent-command", + agent.to_str().unwrap(), + "--agent-args", + "", + ]) + .env("BUZZ_PRIVATE_KEY", &nsec) + .env("BUZZ_RELAY_URL", "wss://relay.test") + .env_remove("NOSTR_PRIVATE_KEY") + .env_remove("BUZZ_AUTH_TAG") + .output() + .expect("run buzz-acp models"); + + let author = std::fs::read_to_string(&out_file).unwrap_or_default(); + assert_eq!( + author.trim(), + expected_email, + "spawn path must install the wrapper + manifest so the agent's commit is \ + authored as the configured key's identity; got {author:?}. models stderr: {}", + String::from_utf8_lossy(&output.stderr), + ); +} diff --git a/crates/buzz-acp/tests/keyfile_lifecycle.rs b/crates/buzz-acp/tests/keyfile_lifecycle.rs new file mode 100644 index 00000000000..f459853f7c1 --- /dev/null +++ b/crates/buzz-acp/tests/keyfile_lifecycle.rs @@ -0,0 +1,67 @@ +//! Process-level regression for the git-identity keyfile leak Gurney found: +//! on an error/timeout exit `buzz-acp` calls `shutdown().await`, which now +//! deletes the 0600 nostr keyfile tempdir explicitly (relying on `TempDir`'s +//! `Drop` right before `std::process::exit` leaked it ~80% of the time). This +//! exercises the REAL binary on its error-exit path and asserts the keyfile +//! dir does not survive. + +use std::path::Path; +use std::process::Command; + +use nostr::ToBech32; + +/// Count `buzz-acp-git-*` identity tempdirs under `tmp`. +fn identity_dirs(tmp: &Path) -> Vec { + std::fs::read_dir(tmp) + .map(|rd| { + rd.filter_map(Result::ok) + .map(|e| e.path()) + .filter(|p| { + p.file_name() + .and_then(|n| n.to_str()) + .is_some_and(|n| n.starts_with("buzz-acp-git-")) + }) + .collect() + }) + .unwrap_or_default() +} + +/// `buzz-acp models` against an agent that exits immediately hits the +/// init-error exit path. With `NOSTR_PRIVATE_KEY` set, the identity keyfile is +/// installed at spawn; `shutdown()` must delete it before the destructor-less +/// exit. Repeated because the pre-fix leak was ~80% probabilistic — a single +/// run could pass spuriously against the broken code. +#[test] +fn error_exit_removes_identity_keyfile() { + for attempt in 0..8 { + // Isolate the tempdir so we only observe this run's identity dirs. + let tmp = tempfile::tempdir().unwrap(); + + // A generated key so `install_git_identity` actually writes a keyfile. + let nsec = nostr::Keys::generate().secret_key().to_bech32().unwrap(); + + // `false` spawns then exits, so `initialize()` fails fast and the + // harness takes the error exit through `shutdown_and_exit`. + let output = Command::new(env!("CARGO_BIN_EXE_buzz-acp")) + .args(["models", "--agent-command", "false", "--agent-args", ""]) + .env("TMPDIR", tmp.path()) + .env("NOSTR_PRIVATE_KEY", &nsec) + .output() + .expect("run buzz-acp models"); + + // The error exit is expected — we assert on the side effect, not success. + assert!( + !output.status.success(), + "attempt {attempt}: expected non-zero exit on the error path; stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let leaked = identity_dirs(tmp.path()); + assert!( + leaked.is_empty(), + "attempt {attempt}: identity keyfile dir leaked on the error-exit path: \ + {leaked:?}\nstderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + } +} diff --git a/crates/buzz-dev-mcp/Cargo.toml b/crates/buzz-dev-mcp/Cargo.toml index 8b711b80634..31650629482 100644 --- a/crates/buzz-dev-mcp/Cargo.toml +++ b/crates/buzz-dev-mcp/Cargo.toml @@ -15,6 +15,7 @@ path = "src/main.rs" [dependencies] buzz-cli = { path = "../buzz-cli" } +buzz-git-identity = { path = "../buzz-git-identity" } git-credential-nostr = { path = "../git-credential-nostr" } git-sign-nostr = { path = "../git-sign-nostr" } nostr = { workspace = true } diff --git a/crates/buzz-dev-mcp/src/lib.rs b/crates/buzz-dev-mcp/src/lib.rs index 87c3a119317..41712f0caf4 100644 --- a/crates/buzz-dev-mcp/src/lib.rs +++ b/crates/buzz-dev-mcp/src/lib.rs @@ -150,6 +150,7 @@ pub fn run() -> Result<(), Box> { "tree" => std::process::exit(tree::run(std::env::args().skip(1).collect())), "git-credential-nostr" => std::process::exit(git_credential_nostr::run()), "git-sign-nostr" => std::process::exit(git_sign_nostr::run()), + "git" => std::process::exit(buzz_git_identity::git_wrapper::run()), _ => {} } diff --git a/crates/buzz-dev-mcp/src/shim.rs b/crates/buzz-dev-mcp/src/shim.rs index cccf0e6eca1..f78dd2d23ee 100644 --- a/crates/buzz-dev-mcp/src/shim.rs +++ b/crates/buzz-dev-mcp/src/shim.rs @@ -1,7 +1,5 @@ -use nostr::ToBech32; use std::path::{Path, PathBuf}; use tempfile::TempDir; -use zeroize::Zeroize; /// Session-scoped shim directory providing tools and git config to shell children. /// @@ -15,6 +13,7 @@ use zeroize::Zeroize; /// the buzz CLI). `NOSTR_PRIVATE_KEY` is removed from the process env after /// the keyfile is written — git helpers read from the keyfile only. /// Cleaned up on drop (TempDir). +#[derive(Debug)] pub struct Shim { _dir: TempDir, pub path_env: String, @@ -23,19 +22,33 @@ pub struct Shim { impl Shim { pub fn install() -> std::io::Result { + // Read the operator's identity mode once, here at install. A bad value + // fails the session loudly rather than silently picking a mode — silent + // fallback in an identity control is the failure class this exists to + // close. The enforcement wrapper never reads this var (it would let an + // agent disable enforcement mid-session); the mode is fixed at install. + let mode = buzz_git_identity::GitIdentityMode::from_env() + .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))?; + let enforce = mode == buzz_git_identity::GitIdentityMode::Agent; + let dir = tempfile::Builder::new().prefix("buzz-dev-mcp-").tempdir()?; set_owner_only(dir.path())?; let self_exe = std::env::current_exe()?; - // Multicall symlinks — all resolve back to this binary. - for name in [ - "rg", - "tree", - "buzz", - "git-credential-nostr", - "git-sign-nostr", - ] { + // Multicall symlinks. `rg`/`tree`/`buzz` and the `git-credential-nostr` + // helper are present in both modes: relay git-over-HTTP auth is + // orthogonal to commit attribution (auth ≠ attribution). The `git` + // enforcement wrapper (see git_wrapper.rs) and the `git-sign-nostr` + // signer are installed only in `agent` mode — in `user` mode the agent + // uses vanilla git resolving the operator's own identity, so shadowing + // `git` with a wrapper that has no manifest would only fail closed. + let mut names = vec!["rg", "tree", "buzz", "git-credential-nostr"]; + if enforce { + names.push("git"); + names.push("git-sign-nostr"); + } + for name in names { symlink(&self_exe, &dir.path().join(name))?; } @@ -48,24 +61,39 @@ impl Shim { .to_string_lossy() .into_owned(); - // Read and unconditionally remove NOSTR_PRIVATE_KEY from this process's - // env. The key must never leak to child processes regardless of whether - // keyfile creation succeeds. - let mut nostr_key = std::env::var("NOSTR_PRIVATE_KEY").ok(); - std::env::remove_var("NOSTR_PRIVATE_KEY"); - - // Ephemeral git config: write key to 0600 keyfile, derive pubkey, build - // GIT_CONFIG_* env vars for nostr auth + signing. - let git_env = match nostr_key - .as_deref() - .and_then(|k| write_keyfile(dir.path(), k)) - { - Some(info) => build_git_env(&info), + // Ephemeral git config: NOSTR_PRIVATE_KEY → 0600 keyfile (and removed + // from this process's env so children never see it) → derive identity → + // build the GIT_CONFIG_* env. The identity primitives live in + // `buzz-git-identity`, shared with the harness so an agent commits under + // the same identity regardless of which surface applied it. + // + // `agent` mode installs authorship + NIP-GS signing + the credential + // helper, and writes the authoritative identity manifest the wrapper + // reads. `user` mode installs only the credential helper (plus the + // `nostr.keyfile` pointer it loads the key from, since NOSTR_PRIVATE_KEY + // was just scrubbed) — no authorship, no signing, no manifest, no + // wrapper — so commits carry the operator's own identity. + let git_env = match buzz_git_identity::take_key_and_write(dir.path()) { + Some(id) if enforce => { + let identity = buzz_git_identity::identity_signing_entries(&id); + // Write the authoritative identity manifest the enforcement + // wrapper reads (its expected author + the config it re-applies + // before exec). Without it the wrapper cannot fail closed on an + // env-scrubbed identity, so a manifest-write failure disables + // enforcement — treat it as fatal rather than ship a wrapper + // that silently trusts mutable env. + buzz_git_identity::write_identity_manifest(dir.path(), &identity)?; + let mut entries = identity; + entries.extend(buzz_git_identity::nostr_credential_entries()); + buzz_git_identity::to_git_config_env(&entries) + } + Some(id) => { + let mut entries = buzz_git_identity::nostr_credential_entries(); + entries.push(buzz_git_identity::keyfile_entry(&id)); + buzz_git_identity::to_git_config_env(&entries) + } None => Vec::new(), }; - if let Some(ref mut k) = nostr_key { - k.zeroize(); - } Ok(Self { _dir: dir, @@ -75,257 +103,6 @@ impl Shim { } } -struct KeyInfo { - keyfile_path: String, - pubkey_hex: String, - npub: String, -} - -/// Write the nostr private key to an owner-only file in the shim dir. -/// Returns key metadata or None if key is empty/invalid. -/// Warns to stderr if the key is invalid (operator mistake). -fn write_keyfile(shim_dir: &Path, raw: &str) -> Option { - if raw.is_empty() { - return None; - } - let keys = match nostr::Keys::parse(raw) { - Ok(k) => k, - Err(e) => { - eprintln!( - "buzz-dev-mcp: warning: NOSTR_PRIVATE_KEY is set but invalid ({e}); \ - git auth/signing will be disabled" - ); - return None; - } - }; - let pubkey_hex = keys.public_key().to_hex(); - let npub = keys - .public_key() - .to_bech32() - .unwrap_or_else(|_| pubkey_hex.clone()); - - let keyfile = shim_dir.join(".nostr-key"); - if write_keyfile_atomic(&keyfile, raw.as_bytes()).is_err() { - eprintln!( - "buzz-dev-mcp: warning: failed to write nostr keyfile; git auth/signing disabled" - ); - return None; - } - let keyfile_path = match keyfile.to_str() { - Some(s) => s.to_owned(), - None => { - eprintln!( - "buzz-dev-mcp: warning: tempdir path is not valid UTF-8; git auth/signing disabled" - ); - return None; - } - }; - - Some(KeyInfo { - keyfile_path, - pubkey_hex, - npub, - }) -} - -/// Write `data` to `path` with 0600 permissions set at creation time via -/// `OpenOptions::mode()` (no window where the file is world-readable). -/// Non-Unix: plain write — acceptable inside our 0700 tempdir. -#[cfg(unix)] -fn write_keyfile_atomic(path: &Path, data: &[u8]) -> std::io::Result<()> { - use std::io::Write; - use std::os::unix::fs::OpenOptionsExt; - let mut f = std::fs::OpenOptions::new() - .write(true) - .create_new(true) - .mode(0o600) - .open(path)?; - f.write_all(data) -} - -#[cfg(not(unix))] -fn write_keyfile_atomic(path: &Path, data: &[u8]) -> std::io::Result<()> { - std::fs::write(path, data) -} - -/// Derive a NIP-05-style email from the pubkey and relay URL. -/// Format: `@` (e.g., `ab12...cd@relay.buzz.dev`). -/// Falls back to `@buzz` if no relay URL is configured. -fn derive_git_email(pubkey_hex: &str) -> String { - let host = std::env::var("BUZZ_RELAY_URL") - .ok() - .and_then(|url| { - // Strip scheme, port, and trailing paths - let stripped = url - .strip_prefix("https://") - .or_else(|| url.strip_prefix("http://")) - .or_else(|| url.strip_prefix("wss://")) - .or_else(|| url.strip_prefix("ws://")) - .unwrap_or(&url); - let host_port = stripped.split('/').next()?; - // Strip port number (e.g., "localhost:3000" → "localhost") - Some(host_port.split(':').next().unwrap_or(host_port).to_owned()) - }) - .filter(|h| !h.is_empty() && !h.starts_with("localhost") && !h.starts_with("127.")) - .unwrap_or_else(|| "buzz".to_owned()); - format!("{pubkey_hex}@{host}") -} - -/// Stable identity contract for git attribution: the bare agent display name, -/// never channel-qualified, safe to embed in commit history. -/// -/// Deliberately distinct from `BUZZ_ACP_SESSION_TITLE`, which is per-session UI -/// chrome and may be composed (`Agent · #channel`) by consumers. Commits -/// outlive sessions, so git attribution must not follow a mutable title. -/// -/// Nothing writes this yet — when unset, [`build_git_env`] falls back to the -/// npub, which is byte-for-byte today's behavior. -const DISPLAY_NAME_ENV_VAR: &str = "BUZZ_ACP_DISPLAY_NAME"; - -/// Max characters in a git author name. Nostr display names are unbounded. -const MAX_GIT_USER_NAME_CHARS: usize = 80; - -/// Characters git's `ident.c` treats as "crud": stripped from both ends of a -/// name, and — when a name is *nothing but* these — rejected outright with -/// `fatal: name consists only of disallowed characters`. -/// -/// Verified empirically against git 2.54.0 by committing with each ASCII byte -/// 32..=126 as the entire `user.name`: exactly space, `"`, `'`, `,`, `:`, `;`, -/// `<`, `>`, and `\` abort. Control characters abort too (the predicate is -/// `c <= 32`). Note `.` is *not* crud in this version despite older lore. -fn is_git_crud(c: char) -> bool { - c <= ' ' || matches!(c, '"' | '\'' | ',' | ':' | ';' | '<' | '>' | '\\') -} - -/// Characters in Unicode general category `Cf` (format): zero-width space and -/// joiners, bidi embedding/override marks, invisible math operators, interlinear -/// annotations, and tag characters. -/// -/// `char::is_control` covers only `Cc`, so every one of these survives it — and -/// none is whitespace or [`is_git_crud`]. A display name of nothing but U+200B -/// ZERO WIDTH SPACE would therefore satisfy the "at least one non-crud -/// character" gate and hand git a visually blank author instead of falling back -/// to the npub. An embedded U+202E RIGHT-TO-LEFT OVERRIDE is worse: it makes a -/// commit's persisted author line render as something other than what it says, -/// the same confusion the angle-bracket filter exists to prevent. -/// -/// The whole category is rejected rather than the two known-bad marks, because -/// the boundary that matters is "invisible or reorders text", not "the codepoint -/// someone thought of". Ranges transcribed from the UCD's -/// `DerivedGeneralCategory.txt` (17.0.0) and independently cross-checked against -/// Python's `unicodedata` (16.0.0); both yield exactly these 21 ranges. Inlined -/// rather than taking a Unicode-tables dependency for one predicate. -fn is_unicode_format(c: char) -> bool { - matches!(c, - '\u{00AD}' - | '\u{0600}'..='\u{0605}' - | '\u{061C}' - | '\u{06DD}' - | '\u{070F}' - | '\u{0890}'..='\u{0891}' - | '\u{08E2}' - | '\u{180E}' - | '\u{200B}'..='\u{200F}' - | '\u{202A}'..='\u{202E}' - | '\u{2060}'..='\u{2064}' - | '\u{2066}'..='\u{206F}' - | '\u{FEFF}' - | '\u{FFF9}'..='\u{FFFB}' - | '\u{110BD}' - | '\u{110CD}' - | '\u{13430}'..='\u{1343F}' - | '\u{1BCA0}'..='\u{1BCA3}' - | '\u{1D173}'..='\u{1D17A}' - | '\u{E0001}' - | '\u{E0020}'..='\u{E007F}' - ) -} - -/// Normalize a Buzz display name into a git author name, or `None` to fall -/// back to the npub. -/// -/// Strips control and Unicode format characters plus angle brackets, collapses -/// whitespace runs, trims, and caps at [`MAX_GIT_USER_NAME_CHARS`] by `chars()` -/// so a multi-byte name cannot be split mid-UTF-8. Angle brackets go because git -/// silently drops them rather than erroring — `Duncan ` would -/// render as `Duncan evil@x.com `, which forges nothing but reads as -/// though it might. -/// -/// Returns `None` unless at least one non-crud character survives. A bare -/// emptiness check is not sufficient: git rejects a name built only of crud, -/// so a display name of `;;` or `""` would abort **every commit** the agent -/// makes. Falling back to the npub keeps the agent able to commit. -fn sanitize_git_user_name(raw: &str) -> Option { - let collapsed = raw - .split_whitespace() - .map(|word| { - word.chars() - .filter(|c| !c.is_control() && !is_unicode_format(*c) && *c != '<' && *c != '>') - .collect::() - }) - .filter(|word| !word.is_empty()) - .collect::>() - .join(" "); - let name: String = collapsed - .chars() - .take(MAX_GIT_USER_NAME_CHARS) - .collect::() - .trim_end() - .to_string(); - name.chars().any(|c| !is_git_crud(c)).then_some(name) -} - -/// Build GIT_CONFIG_COUNT/KEY/VALUE env vars for ephemeral nostr git config. -/// Composes with any existing GIT_CONFIG_COUNT in the environment. When launched -/// via buzz-agent (which clears env), the base is always 0 — composition only -/// matters when dev-mcp is run directly with pre-existing GIT_CONFIG vars. -fn build_git_env(info: &KeyInfo) -> Vec<(String, String)> { - let email = derive_git_email(&info.pubkey_hex); - // Display name for humans reading `git log`; the pubkey stays in the email, - // which is what NIP-98 auth, NIP-GS signing, and contributor matching key on. - let user_name = std::env::var(DISPLAY_NAME_ENV_VAR) - .ok() - .as_deref() - .and_then(sanitize_git_user_name) - .unwrap_or_else(|| info.npub.clone()); - let entries: Vec<(&str, String)> = vec![ - // Identity — Buzz display name (npub fallback), NIP-05-style email - ("user.name", user_name), - ("user.email", email), - // Nostr credential helper is additive — it silently declines non-Buzz - // remotes (exits 0, no credential), so git falls through to system - // helpers (osxkeychain, store, etc.) for GitHub/GitLab/etc. - ("credential.helper", "nostr".into()), - // Required: Buzz relay verifies NIP-98 against the full repo-root URL. - // Without useHttpPath, git only passes the host and auth is rejected. - ("credential.useHttpPath", "true".into()), - ("nostr.keyfile", info.keyfile_path.clone()), - ("gpg.format", "x509".into()), - ("gpg.x509.program", "git-sign-nostr".into()), - ("commit.gpgSign", "true".into()), - ("tag.gpgSign", "true".into()), - ("user.signingkey", info.pubkey_hex.clone()), - ]; - - // Compose with existing GIT_CONFIG_COUNT — don't clobber caller's config. - let base: usize = std::env::var("GIT_CONFIG_COUNT") - .ok() - .and_then(|v| v.parse().ok()) - .unwrap_or(0); - - let mut env = Vec::with_capacity(entries.len() * 2 + 1); - env.push(( - "GIT_CONFIG_COUNT".into(), - (base + entries.len()).to_string(), - )); - for (i, (key, val)) in entries.iter().enumerate() { - let idx = base + i; - env.push((format!("GIT_CONFIG_KEY_{idx}"), key.to_string())); - env.push((format!("GIT_CONFIG_VALUE_{idx}"), val.to_string())); - } - env -} - #[cfg(unix)] fn set_owner_only(path: &Path) -> std::io::Result<()> { use std::os::unix::fs::PermissionsExt; @@ -358,338 +135,101 @@ pub fn artifact_dir(session_root: &Path) -> PathBuf { p } -#[cfg(test)] -mod git_user_name_tests { - use super::{ - build_git_env, is_git_crud, is_unicode_format, sanitize_git_user_name, KeyInfo, - MAX_GIT_USER_NAME_CHARS, - }; +#[cfg(all(test, unix))] +mod tests { + use super::*; + use nostr::ToBech32; use std::sync::Mutex; - /// Env-var-touching tests must run serially — env vars are process-global. + /// `Shim::install` reads process-global env (BUZZ_GIT_IDENTITY, + /// NOSTR_PRIVATE_KEY); serialize these tests so they don't race. static ENV_LOCK: Mutex<()> = Mutex::new(()); - const PUBKEY_HEX: &str = "dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95"; - const NPUB: &str = "npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7"; - - fn key_info() -> KeyInfo { - KeyInfo { - keyfile_path: "/tmp/.nostr-key".into(), - pubkey_hex: PUBKEY_HEX.into(), - npub: NPUB.into(), + fn install_with(mode: Option<&str>) -> Shim { + let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let nsec = nostr::Keys::generate().secret_key().to_bech32().unwrap(); + std::env::set_var("NOSTR_PRIVATE_KEY", &nsec); + match mode { + Some(m) => std::env::set_var("BUZZ_GIT_IDENTITY", m), + None => std::env::remove_var("BUZZ_GIT_IDENTITY"), } + let shim = Shim::install().expect("install"); + std::env::remove_var("BUZZ_GIT_IDENTITY"); + // `take_key_and_write` already scrubbed NOSTR_PRIVATE_KEY, but be sure. + std::env::remove_var("NOSTR_PRIVATE_KEY"); + shim } - /// Read a git config value back out of the flat GIT_CONFIG_KEY_n/VALUE_n pairs. - fn git_config(env: &[(String, String)], key: &str) -> Option { - let idx = env - .iter() - .find(|(k, v)| k.starts_with("GIT_CONFIG_KEY_") && v == key)? - .0 - .strip_prefix("GIT_CONFIG_KEY_")? - .to_owned(); - env.iter() - .find(|(k, _)| *k == format!("GIT_CONFIG_VALUE_{idx}")) - .map(|(_, v)| v.clone()) - } - - #[test] - fn test_ordinary_name_passes_through_unchanged() { - assert_eq!(sanitize_git_user_name("Duncan"), Some("Duncan".into())); - } - - #[test] - fn test_angle_brackets_are_stripped_so_no_second_email_is_rendered() { - // git drops the brackets itself and renders `Duncan evil@x.com - // ` — no forgery, but a confusing author line. - assert_eq!( - sanitize_git_user_name("Duncan "), - Some("Duncan evil@x.com".into()) - ); - } - - #[test] - fn test_whitespace_control_characters_become_a_single_separator() { - // Newline, tab and carriage return are whitespace: they collapse to one - // space like any other run, so a multi-line name stays readable. - assert_eq!( - sanitize_git_user_name("Dun\ncan\tThe\r\nIdaho"), - Some("Dun can The Idaho".into()) - ); - } - - #[test] - fn test_non_whitespace_control_characters_are_dropped_outright() { - // NUL is the important one: an interior NUL makes `Command::env` fail - // the entire spawn upstream, so it must never survive to git config. - let got = sanitize_git_user_name("Idaho\0Blade\u{7}").expect("non-empty"); - assert_eq!(got, "IdahoBlade"); - assert!(!got.chars().any(char::is_control)); - } - - #[test] - fn test_internal_whitespace_runs_collapse_to_one_space() { - assert_eq!( - sanitize_git_user_name(" Duncan Idaho "), - Some("Duncan Idaho".into()) - ); - } - - #[test] - fn test_whitespace_only_name_falls_back_to_npub() { - assert_eq!(sanitize_git_user_name(" \t\n "), None); - } - - #[test] - fn test_empty_name_falls_back_to_npub() { - assert_eq!(sanitize_git_user_name(""), None); - } - - #[test] - fn test_crud_only_name_falls_back_rather_than_aborting_every_commit() { - // git rejects a name built only of crud with `fatal: name consists - // only of disallowed characters`, which would break EVERY commit the - // agent makes. Verified against git 2.54.0. - for raw in ["<>", ";;", "\"\"", "''", ",", ":", "\\", ",;:"] { - assert_eq!( - sanitize_git_user_name(raw), - None, - "crud-only name {raw:?} must fall back to the npub" - ); - } - } - - #[test] - fn test_crud_mixed_with_real_characters_is_kept() { - // Legitimate names contain crud; only an all-crud result is fatal. - assert_eq!(sanitize_git_user_name("O'Brien"), Some("O'Brien".into())); - assert_eq!( - sanitize_git_user_name("Smith, Jr."), - Some("Smith, Jr.".into()) - ); - } - - #[test] - fn test_over_length_name_is_truncated_to_the_cap() { - let long = "a".repeat(200); - let got = sanitize_git_user_name(&long).expect("non-empty"); - assert_eq!(got.chars().count(), MAX_GIT_USER_NAME_CHARS); - } - - #[test] - fn test_truncation_never_splits_a_multibyte_character() { - let long = "🐝".repeat(200); - let got = sanitize_git_user_name(&long).expect("non-empty"); - assert_eq!(got.chars().count(), MAX_GIT_USER_NAME_CHARS); - assert!(got.chars().all(|c| c == '🐝'), "no replacement chars"); - } - - #[test] - fn test_truncation_does_not_leave_a_trailing_space() { - // Cutting mid-word would otherwise strand the separator at the end. - let raw = format!("{} tail", "a".repeat(MAX_GIT_USER_NAME_CHARS - 1)); - let got = sanitize_git_user_name(&raw).expect("non-empty"); - assert!(!got.ends_with(' '), "got {got:?}"); - } - - #[test] - fn test_non_ascii_names_survive() { - assert_eq!( - sanitize_git_user_name("Élodie 🐝"), - Some("Élodie 🐝".into()) - ); + fn shim_dir(shim: &Shim) -> PathBuf { + std::env::split_paths(&shim.path_env) + .next() + .expect("shim dir is first PATH entry") } - #[test] - fn test_format_only_name_falls_back_to_npub() { - // U+200B is neither control, nor whitespace, nor crud, so before Cf - // filtering this passed the non-crud gate and handed git a visually - // blank author instead of falling back. - assert_eq!(sanitize_git_user_name("\u{200B}\u{200B}"), None); - // Same class, different marks: joiner, word joiner, BOM, bidi override. - for raw in ["\u{200D}", "\u{2060}", "\u{FEFF}", "\u{202E}", "\u{00AD}"] { - assert_eq!( - sanitize_git_user_name(raw), - None, - "format-only name {raw:?} must fall back to the npub" - ); - } + fn has(env: &[(String, String)], key: &str) -> bool { + env.iter() + .any(|(k, v)| k.starts_with("GIT_CONFIG_KEY_") && v == key) } #[test] - fn test_bidi_override_is_stripped_and_the_name_is_kept() { - // A trailing RLO would reorder everything after it in `git log`, so the - // mark goes and the readable name stays. - assert_eq!( - sanitize_git_user_name("Duncan\u{202E}"), - Some("Duncan".into()) + fn agent_mode_installs_wrapper_signer_and_manifest() { + let shim = install_with(Some("agent")); + let dir = shim_dir(&shim); + assert!(dir.join("git").exists(), "git wrapper must be installed"); + assert!(dir.join("git-sign-nostr").exists(), "signer installed"); + assert!( + dir.join("git-credential-nostr").exists(), + "credential helper installed" ); - assert_eq!( - sanitize_git_user_name("Dun\u{202E}can Idaho"), - Some("Duncan Idaho".into()) + assert!( + buzz_git_identity::read_identity_manifest(&dir).is_some(), + "manifest must be written" ); + assert!(has(&shim.git_env, "user.email"), "authorship injected"); + assert!(has(&shim.git_env, "credential.helper"), "helper injected"); } #[test] - fn test_zero_width_space_inside_a_word_is_removed_without_splitting_it() { - // U+200B is not whitespace, so it must not become a separator: the word - // rejoins rather than turning into "Dun can". - assert_eq!( - sanitize_git_user_name("Dun\u{200B}can"), - Some("Duncan".into()) + fn user_mode_keeps_credential_helper_but_no_wrapper_signing_or_manifest() { + let shim = install_with(Some("user")); + let dir = shim_dir(&shim); + // Auth survives: the credential helper and its keyfile pointer. + assert!( + dir.join("git-credential-nostr").exists(), + "credential helper must survive user mode (auth ≠ attribution)" ); - } - - #[test] - fn test_format_characters_do_not_consume_the_length_budget() { - // Filtering happens before truncation, so invisible padding cannot - // shorten the visible name. - let raw = format!("{}{}", "\u{200B}".repeat(200), "a".repeat(90)); - let got = sanitize_git_user_name(&raw).expect("non-empty"); - assert_eq!(got.chars().count(), MAX_GIT_USER_NAME_CHARS); - assert!(got.chars().all(|c| c == 'a'), "got {got:?}"); - } - - #[test] - fn test_unicode_format_covers_every_cf_range_and_nothing_adjacent() { - // Both endpoints of each of the 21 `Cf` ranges in UCD 17.0.0. Endpoints - // are what a transcription error moves, so they are what gets asserted. - for c in [ - '\u{00AD}', - '\u{0600}', - '\u{0605}', - '\u{061C}', - '\u{06DD}', - '\u{070F}', - '\u{0890}', - '\u{0891}', - '\u{08E2}', - '\u{180E}', - '\u{200B}', - '\u{200F}', - '\u{202A}', - '\u{202E}', - '\u{2060}', - '\u{2064}', - '\u{2066}', - '\u{206F}', - '\u{FEFF}', - '\u{FFF9}', - '\u{FFFB}', - '\u{110BD}', - '\u{110CD}', - '\u{13430}', - '\u{1343F}', - '\u{1BCA0}', - '\u{1BCA3}', - '\u{1D173}', - '\u{1D17A}', - '\u{E0001}', - '\u{E0020}', - '\u{E007F}', - ] { - assert!(is_unicode_format(c), "U+{:04X} is Cf", c as u32); - } - // Codepoints immediately outside those ranges, plus ordinary characters. - // U+2065 is the notable one: it sits *inside* the 2060..206F block but - // is unassigned, not `Cf`. - for c in [ - '\u{00AC}', - '\u{00AE}', - '\u{05FF}', - '\u{0606}', - '\u{061B}', - '\u{061D}', - '\u{200A}', - '\u{2010}', - '\u{2029}', - '\u{202F}', - '\u{2065}', - '\u{205F}', - '\u{2070}', - '\u{FEFE}', - '\u{FFF8}', - '\u{FFFC}', - '\u{110BC}', - '\u{1342F}', - '\u{E0000}', - '\u{E0080}', - 'a', - ' ', - '🐝', - 'É', - ] { - assert!(!is_unicode_format(c), "U+{:04X} is not Cf", c as u32); - } - } - - #[test] - fn test_build_git_env_uses_display_name_and_leaves_email_on_the_pubkey() { - let _guard = ENV_LOCK.lock().unwrap(); - std::env::set_var("BUZZ_ACP_DISPLAY_NAME", "Duncan"); - std::env::remove_var("BUZZ_RELAY_URL"); - std::env::remove_var("GIT_CONFIG_COUNT"); - let env = build_git_env(&key_info()); - std::env::remove_var("BUZZ_ACP_DISPLAY_NAME"); - - assert_eq!(git_config(&env, "user.name").as_deref(), Some("Duncan")); - // The pubkey — the thing NIP-98 auth, NIP-GS signing, and contributor - // matching key on — must stay in the email untouched. - assert_eq!( - git_config(&env, "user.email").as_deref(), - Some(format!("{PUBKEY_HEX}@buzz").as_str()) + assert!(has(&shim.git_env, "credential.helper"), "helper injected"); + assert!( + has(&shim.git_env, "nostr.keyfile"), + "keyfile pointer injected so the helper can load the key" ); - assert_eq!( - git_config(&env, "user.signingkey").as_deref(), - Some(PUBKEY_HEX) + // Attribution machinery is absent: no wrapper, signer, manifest, or + // authorship/signing config. + assert!( + !dir.join("git").exists(), + "no git wrapper — vanilla git resolves the operator's identity" ); - } - - #[test] - fn test_build_git_env_falls_back_to_npub_when_display_name_unset() { - let _guard = ENV_LOCK.lock().unwrap(); - std::env::remove_var("BUZZ_ACP_DISPLAY_NAME"); - std::env::remove_var("BUZZ_RELAY_URL"); - std::env::remove_var("GIT_CONFIG_COUNT"); - let env = build_git_env(&key_info()); - - // Today's behavior, and what every agent gets until a writer for - // BUZZ_ACP_DISPLAY_NAME lands on the Desktop side. - assert_eq!(git_config(&env, "user.name").as_deref(), Some(NPUB)); - assert_eq!( - git_config(&env, "user.email").as_deref(), - Some(format!("{PUBKEY_HEX}@buzz").as_str()) + assert!(!dir.join("git-sign-nostr").exists(), "no signer"); + assert!( + buzz_git_identity::read_identity_manifest(&dir).is_none(), + "no manifest in user mode" ); + assert!(!has(&shim.git_env, "user.email"), "no authorship"); + assert!(!has(&shim.git_env, "commit.gpgSign"), "no signing"); } #[test] - fn test_build_git_env_falls_back_to_npub_when_display_name_is_unusable() { - let _guard = ENV_LOCK.lock().unwrap(); - std::env::remove_var("BUZZ_RELAY_URL"); - std::env::remove_var("GIT_CONFIG_COUNT"); - - // Crud-only and format-only names both reach git as the npub — one - // would abort every commit, the other would render as blank. - for raw in ["<>", "\u{200B}"] { - std::env::set_var("BUZZ_ACP_DISPLAY_NAME", raw); - let env = build_git_env(&key_info()); - assert_eq!( - git_config(&env, "user.name").as_deref(), - Some(NPUB), - "unusable display name {raw:?} must reach git as the npub" - ); - } - std::env::remove_var("BUZZ_ACP_DISPLAY_NAME"); + fn unset_defaults_to_agent() { + let shim = install_with(None); + assert!(shim_dir(&shim).join("git").exists(), "default is agent"); } #[test] - fn test_git_crud_set_matches_observed_git_behavior() { - // Empirically derived from git 2.54.0: these bytes, alone, abort a commit. - for c in [' ', '"', '\'', ',', ':', ';', '<', '>', '\\', '\t', '\n'] { - assert!(is_git_crud(c), "{c:?} should be crud"); - } - for c in ['.', '-', '_', '@', '(', 'a', '🐝'] { - assert!(!is_git_crud(c), "{c:?} should not be crud"); - } + fn invalid_mode_fails_install() { + let _guard = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + std::env::set_var("BUZZ_GIT_IDENTITY", "usr"); + let err = Shim::install().expect_err("invalid mode must fail install"); + std::env::remove_var("BUZZ_GIT_IDENTITY"); + assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput); } } diff --git a/crates/buzz-git-identity/Cargo.toml b/crates/buzz-git-identity/Cargo.toml new file mode 100644 index 00000000000..25d69075924 --- /dev/null +++ b/crates/buzz-git-identity/Cargo.toml @@ -0,0 +1,19 @@ +[package] +name = "buzz-git-identity" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +description = "Deterministic agent git identity: author/email/signing GIT_CONFIG_* construction shared by the harness and dev-mcp" + +[lib] +name = "buzz_git_identity" +path = "src/lib.rs" + +[dependencies] +nostr = { workspace = true } +zeroize = { workspace = true } +wait-timeout = "0.2" + +[dev-dependencies] +tempfile = "3" diff --git a/crates/buzz-git-identity/src/git_wrapper.rs b/crates/buzz-git-identity/src/git_wrapper.rs new file mode 100644 index 00000000000..ac2b196a5a4 --- /dev/null +++ b/crates/buzz-git-identity/src/git_wrapper.rs @@ -0,0 +1,2114 @@ +//! Enforcement `git` wrapper — the L2/L3 half of deterministic agent identity. +//! +//! Installed on PATH (shim dir and the harness's agent-runtime PATH) as `git`, +//! ahead of the real binary. Every `git` an agent's shell runs lands here first. +//! The wrapper: +//! +//! 1. **Scrubs** `GIT_AUTHOR_NAME`/`GIT_AUTHOR_EMAIL` and the committer pair from +//! the child env — the env-var identity-override vector. +//! 2. **Rejects loudly** the flag-based override vectors: `-c user.name=`/ +//! `-c user.email=` (and `--config-env` for the same keys) in global position, +//! and `--author`/`--reset-author` on `commit`/`am`. +//! 3. On `push`, **verifies** that every outgoing commit not already on a remote +//! is authored by the agent identity, and fails the push otherwise. +//! 4. Execs the real `git` (found by skipping PATH entries that resolve back to +//! this binary), so nothing the agent can pass reaches git with a spoofed +//! identity on the default path. +//! +//! Exit codes: `1` for a rejected override, `1` for a failed push verification, +//! `127` when the real git cannot be found. Otherwise the real git's own status. + +use std::path::{Path, PathBuf}; + +/// Author/committer identity env vars the agent's shell must not use to override +/// the configured Buzz identity. DATE is intentionally left alone — it carries +/// no attribution signal and rebase/cherry-pick rely on it internally. +const SCRUBBED_ENV: &[&str] = &[ + "GIT_AUTHOR_NAME", + "GIT_AUTHOR_EMAIL", + "GIT_COMMITTER_NAME", + "GIT_COMMITTER_EMAIL", +]; + +/// Maximum number of ordinary alias substitutions the managed wrapper resolves. +/// The next command token is always inspected too, so a chain with exactly this +/// many aliases may terminate at a real subcommand; a further alias is refused. +const MAX_ALIAS_HOPS: usize = 10; + +/// Long global options that consume the *following* argv token as their value +/// (the `--opt value` form; the `--opt=value` form is self-contained). Needed +/// only to locate the subcommand correctly; agents almost never pass these. +const VALUE_LONG_OPTS: &[&str] = &[ + "--git-dir", + "--work-tree", + "--namespace", + "--super-prefix", + "--config-env", + "--attr-source", +]; + +/// The harness-owned identity authority: the identity/signing config the +/// wrapper re-applies before exec, and the agent author email push +/// verification checks against. Read from the 0600 manifest the harness/shim +/// wrote beside the keyfile — never from the caller-mutable `GIT_CONFIG_*` +/// environment the wrapper exists to constrain. +struct Authority { + /// Ordered identity + signing `(key, value)` git config entries. + entries: Vec<(String, String)>, + /// The expected commit author email (`<64-hex>@host`). + email: String, +} + +/// Result of locating the wrapper's identity authority. +enum AuthorityState { + /// The wrapper was not reached through an install symlink on `PATH`, so its + /// own dir (and any manifest) cannot be located. This is the accepted local + /// ceiling — e.g. the real `git` invoked by absolute path — so there is no + /// authority to enforce against: passthrough. + Unmanaged, + /// The install dir is located and holds a valid manifest: enforce. + Managed(Authority), + /// The install dir is located but its manifest is missing, empty, or lacks + /// a usable `user.email`. A managed install always writes a valid manifest, + /// so this means the authority was deleted or corrupted after install — + /// fail closed rather than silently drop enforcement. + Tampered, +} + +impl Authority { + /// Locate and classify the wrapper's identity authority. Distinguishing a + /// genuinely unmanaged wrapper (no install dir on `PATH`) from a located + /// install dir whose manifest was removed/corrupted is load-bearing: the + /// former passes through (accepted ceiling), the latter fails closed. + fn load() -> AuthorityState { + let Some(dir) = locate_install_dir() else { + return AuthorityState::Unmanaged; + }; + let Some(entries) = crate::read_identity_manifest(&dir) else { + return AuthorityState::Tampered; // manifest missing/unreadable + }; + let Some(email) = entries + .iter() + .find(|(k, _)| k == "user.email") + .map(|(_, v)| v.clone()) + else { + return AuthorityState::Tampered; // present but no usable identity + }; + AuthorityState::Managed(Self { entries, email }) + } +} + +/// Entry point for the `git` multicall personality. Never returns on success +/// (execs real git on Unix); returns the process exit code on Windows/error. +pub fn run() -> i32 { + let argv: Vec = std::env::args().skip(1).collect(); + + // Classify the authority. `Tampered` (install dir located but manifest + // missing/corrupt) fails closed for every command: a managed install always + // writes a valid manifest, so its absence means the authority was removed or + // damaged after install, and continuing would silently drop enforcement. + let authority = match Authority::load() { + AuthorityState::Managed(a) => Some(a), + AuthorityState::Unmanaged => None, + AuthorityState::Tampered => { + eprintln!( + "buzz git wrapper: refusing to run — this `git` is a managed enforcement \ + wrapper but its identity manifest is missing or unreadable. Enforcement fails \ + closed rather than fall back to an ambient identity." + ); + return 1; + } + }; + + if let Err(msg) = enforce(&argv, authority.as_ref()) { + eprintln!("{msg}"); + return 1; + } + + let real_git = match find_real_git() { + Some(p) => p, + None => { + eprintln!( + "buzz git wrapper: could not locate the real `git` binary on PATH. \ + Agent git identity enforcement is active but git is not installed." + ); + return 127; + } + }; + + let ctx = repo_context_args(&argv); + + // Alias preflight + unification. `verify_alias_safety` refuses every shell + // (`!`) alias and every non-shell alias carrying config/quoting the wrapper + // cannot classify; on success it returns the alias's fully-resolved bare-word + // expansion (or `None` when no alias was involved). We then hold that + // expansion to the SAME `enforce`/`verify_commit_author` policy as a directly + // typed command — keyed on the *expanded* subcommand — so an alias can never + // do more than its expansion could. `enforce`/`verify_commit_author` on the + // literal argv below cannot catch alias-carried flags (`git human` expands to + // `commit --author …`, but the literal subcommand is `human`); the expanded + // preflight closes that gap by construction, with no alias-specific flag list. + if let Some(auth) = &authority { + match verify_alias_safety(&real_git, &argv, &ctx) { + Ok(None) => {} + Ok(Some(expanded)) => { + if let Err(msg) = enforce(&expanded, Some(auth)) { + eprintln!("{msg}"); + return 1; + } + if let Err(msg) = verify_commit_author(&real_git, &expanded, &ctx, auth) { + eprintln!("{msg}"); + return 1; + } + } + Err(msg) => { + eprintln!("{msg}"); + return 1; + } + } + } + + // Author preflight (E): commit modes that reuse or preserve another author + // (`-C`/`-c `, `--amend`) create NEW commits stamped with that author. + // Re-applied identity config cannot fix this — git honours the reused + // author — so reject when the resulting author would not be the agent. This + // covers a directly-typed `commit`; an alias resolving to one is covered by + // the expanded-command preflight above. + if let Some(auth) = &authority { + if let Err(msg) = verify_commit_author(&real_git, &argv, &ctx, auth) { + eprintln!("{msg}"); + return 1; + } + } + + // Push verification (L3) runs before exec so a wrongly-authored commit + // cannot leave the machine. The effective command is resolved through git + // aliases (config-defined and inline `-c alias.*`), because `git pub` with + // `alias.pub = push` reaches the real push after we hand off — keying on the + // literal token alone would let an alias slip a wrong-authored commit past. + if let Some(auth) = &authority { + match is_push_command(&real_git, &argv, &ctx) { + PushKind::NotPush => {} + PushKind::Push => { + if let Err(msg) = verify_push(&real_git, &argv, &ctx, auth) { + eprintln!("{msg}"); + return 1; + } + } + } + } + + exec_real_git(&real_git, &argv, authority.as_ref()) +} + +/// The wrapper's own install dir: the first `PATH` entry whose `git` resolves +/// (through the install symlink) back to this binary. That dir holds the 0600 +/// identity manifest and keyfile. Located by canonicalization — the same +/// env-independent trust channel as [`find_real_git`] — so an agent cannot +/// point the wrapper at a forged authority by rewriting environment variables. +fn locate_install_dir() -> Option { + let self_canon = std::env::current_exe().ok()?.canonicalize().ok()?; + let git_name = if cfg!(windows) { "git.exe" } else { "git" }; + for dir in std::env::split_paths(&std::env::var_os("PATH").unwrap_or_default()) { + let candidate = dir.join(git_name); + if candidate.canonicalize().ok().as_ref() == Some(&self_canon) { + return Some(dir); + } + } + None +} + +/// Global options that carry the repository context real git would apply. The +/// verifier's internal `config`/`rev-list`/`show` probes must run under the +/// same context or they resolve against the wrapper's cwd instead — the +/// `git -C push` bypass. `-C` and its value are already paired into the +/// globals by [`split_globals`]. +fn repo_context_args(argv: &[String]) -> Vec { + let (globals, _) = split_globals(argv); + let mut ctx = Vec::new(); + let mut i = 0; + while i < globals.len() { + let g = globals[i].as_str(); + if matches!(g, "-C" | "--git-dir" | "--work-tree" | "--namespace") { + ctx.push(globals[i].clone()); + if i + 1 < globals.len() { + i += 1; + ctx.push(globals[i].clone()); + } + } else if g.starts_with("--git-dir=") + || g.starts_with("--work-tree=") + || g.starts_with("--namespace=") + { + ctx.push(globals[i].clone()); + } + i += 1; + } + ctx +} + +/// The effective-command classification of an invocation. +enum PushKind { + /// The effective command is not `push`. + NotPush, + /// The effective command resolves to `push` through ordinary (config/inline) + /// git aliases; its transport plan can be resolved safely with `--dry-run`. + Push, +} + +/// Classify the invocation's *effective* command, resolving ordinary git +/// aliases so `git pub` (with `alias.pub = push`) and `git -c alias.pub=push +/// pub` are both recognized. Config aliases are read under `ctx` so a +/// `-C ` push consults the target repo's aliases. +/// +/// This runs only in a managed session, *after* [`verify_alias_safety`] has +/// already refused every shell (`!`) alias and every non-shell alias that is +/// not a trivially-safe bare-word chain — so a shell alias never reaches here. +/// Recursion is bounded to defeat cyclic alias definitions. +fn is_push_command(real_git: &Path, argv: &[String], ctx: &[String]) -> PushKind { + let (globals, _) = split_globals(argv); + let inline = inline_aliases(&globals); + let mut name = match subcommand(argv) { + Some(s) => s, + None => return PushKind::NotPush, + }; + for _ in 0..10 { + if name == "push" { + return PushKind::Push; + } + let def = inline.get(&name).cloned().or_else(|| { + capture( + real_git, + ctx, + &["config", "--get", &format!("alias.{name}")], + ) + }); + let def = match def { + Some(d) => d, + None => return PushKind::NotPush, // not an alias — effective command + }; + if def.starts_with('!') { + return PushKind::NotPush; // shell alias — already refused upstream + } + match def.split_whitespace().next() { + Some(first) => name = first.to_string(), + None => return PushKind::NotPush, + } + } + PushKind::NotPush +} + +/// Map of inline `-c alias.NAME=BODY` definitions passed on the command line. +/// These win over config-file aliases, matching git's own precedence. +fn inline_aliases(globals: &[String]) -> std::collections::HashMap { + let mut map = std::collections::HashMap::new(); + for token in globals { + let cfg = token + .strip_prefix("-c") + .filter(|s| !s.is_empty()) + .unwrap_or(token); + if let Some(rest) = cfg.strip_prefix("alias.") { + if let Some((name, body)) = rest.split_once('=') { + map.insert(name.to_string(), body.to_string()); + } + } + } + map +} + +/// Refuse any alias the wrapper cannot *trivially* prove safe, and — on success +/// — return the alias's fully-resolved expansion so the caller can hold it to +/// the same policy as a directly-typed command. Git expands an alias in-process, +/// and its config-bearing globals land *after* the identity/signing `-c` options +/// this wrapper injects before the subcommand ([`inject_identity_args`]), so an +/// alias could otherwise plant higher-precedence config that re-authors or +/// unsigns the commit. It could equally carry identity/signing *flags* +/// (`--author`, `--no-gpg-sign`, `--amend`, commit reuse). [`enforce`] and +/// [`verify_commit_author`] cannot catch either on the literal argv: they key on +/// the typed subcommand, which for `git human` is the alias name `human`, never +/// the expanded `commit`. +/// +/// This is an allowlist, not a blocklist. Rather than model git's alias grammar +/// (whose quote-aware parser would let `'-c' 'user.email=…'` slip past a +/// naive token scan), it admits an alias only when every token of its body is a +/// trivially-safe bare word: no quote or backslash characters, no `-c`/ +/// `--config-env` config channel, and no `=`-valued option. Anything the +/// wrapper cannot classify at a glance is refused (favor-rejection). +/// +/// The returned expansion is exactly the token list git would run — the typed +/// globals, then the recursively-expanded command with accumulated body tokens +/// and the user's trailing argv. Holding it to [`enforce`]/[`verify_commit_author`] +/// keyed on the *expanded* subcommand means an alias can never do more than its +/// expansion could typed directly, so no alias-specific flag list exists to keep +/// in sync. `Ok(None)` means the typed subcommand was a real command (no alias). +/// +/// Shell (`!`) aliases are refused outright in a managed session. Git runs an +/// `!` alias with its own exec-path prepended to `PATH`, so the inner `git` is +/// the *real* binary, not this wrapper — its `-c` outranks the inherited env +/// authority and can commit as an arbitrary human, unsigned. Their bodies are +/// arbitrary shell, so there is no safe subset to allow. +/// +/// Allowed shapes stay working: `alias.ci = commit`, `alias.st = status`, +/// `alias.lg = log --oneline`, `alias.pub = push origin main`. Recursion is +/// bounded to defeat cyclic alias definitions. After the bound is reached, the +/// next command token must be a real subcommand or the wrapper refuses rather +/// than treating a partial expansion as resolved. +fn verify_alias_safety( + real_git: &Path, + argv: &[String], + ctx: &[String], +) -> Result>, String> { + let (globals, sub_idx) = split_globals(argv); + let inline = inline_aliases(&globals); + let Some(sub_idx) = sub_idx else { + return Ok(None); // no subcommand — nothing to expand + }; + // The command chain being expanded: the subcommand token and its trailing + // argv. Typed globals (argv[..sub_idx]) are prepended to the final result. + let mut chain: Vec = argv[sub_idx..].to_vec(); + let mut resolved_any = false; + for _ in 0..MAX_ALIAS_HOPS { + // The command word within the current chain (git re-parses leading + // options as globals after each expansion, so a body may begin with + // bare-word options before its subcommand). + let Some(cmd_idx) = split_globals(&chain).1 else { + break; // no command word left + }; + let name = &chain[cmd_idx]; + let def = inline.get(name).cloned().or_else(|| { + capture( + real_git, + ctx, + &["config", "--get", &format!("alias.{name}")], + ) + }); + let Some(def) = def else { + break; // real subcommand — chain is fully expanded + }; + if def.starts_with('!') { + return Err(shell_alias_reject_message(name)); + } + let body: Vec = def.split_whitespace().map(String::from).collect(); + if !body.iter().all(|t| is_safe_alias_token(t)) { + return Err(alias_reject_message(name)); + } + // Substitute the command word with its body, exactly as git does. + chain.splice(cmd_idx..=cmd_idx, body); + resolved_any = true; + } + let Some(cmd_idx) = split_globals(&chain).1 else { + return Err(alias_limit_reject_message()); + }; + let name = &chain[cmd_idx]; + if inline.contains_key(name) + || capture( + real_git, + ctx, + &["config", "--get", &format!("alias.{name}")], + ) + .is_some() + { + return Err(alias_limit_reject_message()); + } + if !resolved_any { + return Ok(None); + } + let mut expanded = argv[..sub_idx].to_vec(); + expanded.extend(chain); + Ok(Some(expanded)) +} + +/// A single alias-body token is safe only when it is a plain bare word that +/// introduces no configuration and needs no shell/quote interpretation. This is +/// deliberately conservative: git's quote-aware alias parser sees a token +/// differently from our whitespace split, so any token carrying a quote or +/// escape is refused rather than guessed at. +fn is_safe_alias_token(token: &str) -> bool { + // Quote/escape characters: git would dequote these, changing the token from + // what we scanned. Refuse — the allowlist never reasons about quoted forms. + if token.contains(['\'', '"', '\\']) { + return false; + } + // The config-injection channels, in any spelling. + if token == "-c" + || token.starts_with("-c") + || token == "--config-env" + || token.starts_with("--config-env") + { + return false; + } + // Any other option carrying a `=` value (e.g. `--author=…`, `--foo=bar`) + // can redirect identity/behaviour we cannot classify — refuse. + if token.starts_with('-') && token.contains('=') { + return false; + } + true +} + +fn alias_reject_message(name: &str) -> String { + format!( + "buzz git wrapper: refusing `{name}` — this git alias contains tokens the managed \ + wrapper cannot verify as safe (quoting/escaping, `-c`/`--config-env`, or a \ + value-bearing option). Aliases that could carry configuration are refused because \ + git applies alias config after the managed agent identity and signing config. Run \ + the underlying git command directly; agent commit identity and signing are \ + machine-managed." + ) +} + +fn alias_limit_reject_message() -> String { + format!( + "buzz git wrapper: refusing alias chain after {MAX_ALIAS_HOPS} expansions — the managed \ + wrapper only runs a command after proving its final command word is not another git alias. \ + Run the underlying git command directly; agent commit identity and signing are \ + machine-managed." + ) +} + +fn shell_alias_reject_message(name: &str) -> String { + format!( + "buzz git wrapper: refusing `{name}` — it is a shell (`!`) git alias. Git runs `!` \ + aliases with the real git ahead of this wrapper on PATH, so their body can commit \ + or push under an arbitrary identity, unsigned. Run the underlying git command \ + directly; agent commit identity and signing are machine-managed." + ) +} + +/// Reject the flag-based identity- and signing-override vectors. `Ok(())` means +/// the argv is clean and may proceed to the real git. +/// +/// Only enforces in a managed session (`authority` present): an unmanaged +/// session has no injected identity or signing config to protect, so rejecting +/// `--no-gpg-sign` there would break ordinary use. The env-var forms of these +/// overrides (`GIT_CONFIG_*`) are defeated separately by re-applying the +/// authoritative config at the highest index before exec; this covers the +/// command-line forms, which win over env config and so must be refused. +fn enforce(argv: &[String], authority: Option<&Authority>) -> Result<(), String> { + if authority.is_none() { + return Ok(()); + } + let (globals, sub_idx) = split_globals(argv); + + // Protected config keys set via `-c key=…`/`-ckey=…` or `--config-env=key=VAR` + // in global position. `-c` only ever appears as a git *global* option, so + // scanning globals both suffices and avoids misreading `git commit -c + // ` (reuse-message), where `-c` means something entirely different. + for token in &globals { + if let Some(key) = config_key_override(token) { + return Err(reject_message(&format!("-c {key}=…"))); + } + if let Some(key) = config_env_override(token) { + return Err(reject_message(&format!("--config-env={key}=…"))); + } + } + + // Subcommand-scoped identity/signing flags. `--author`/`--reset-author` + // carry identity for `commit`/`am`; `--no-gpg-sign` disables the signing + // the harness lifted. Scoping to the relevant subcommands is load-bearing: + // `git log --author=…` is a legitimate read filter that must keep working. + if let Some(sub) = sub_idx.map(|i| argv[i].as_str()) { + let is_commit_or_am = sub == "commit" || sub == "am"; + let signs = matches!( + sub, + "commit" | "am" | "tag" | "rebase" | "cherry-pick" | "revert" + ); + for token in &argv[sub_idx.unwrap() + 1..] { + if is_commit_or_am + && (token == "--author" + || token.starts_with("--author=") + || token == "--reset-author") + { + return Err(reject_message(token)); + } + if signs && token == "--no-gpg-sign" { + return Err(reject_message(token)); + } + } + } + + Ok(()) +} + +fn reject_message(what: &str) -> String { + format!( + "buzz git wrapper: refusing `{what}` — agent commit identity and signing are \ + machine-managed and cannot be overridden. Commits are automatically authored as your \ + agent identity (@) and signed. Credit the human operator with \ + `Co-authored-by`/`Signed-off-by` trailers instead." + ) +} + +/// If `token` is a `-c ` value (attached `-cuser.email=x` or the bare +/// `user.email=x` that follows a standalone `-c`) setting a protected identity +/// or signing key, return the normalized key; else `None`. +fn config_key_override(token: &str) -> Option<&'static str> { + // `-cuser.email=x` attached form, or the standalone value token that + // `split_globals` already paired with a preceding `-c`. + let cfg = token + .strip_prefix("-c") + .filter(|s| !s.is_empty()) + .unwrap_or(token); + matches_protected_key(cfg) +} + +/// If `token` is `--config-env==VAR` for a protected key, return it. +fn config_env_override(token: &str) -> Option<&'static str> { + let rest = token.strip_prefix("--config-env=")?; + matches_protected_key(rest) +} + +/// Normalize a `name.subname[=value]` config spec and return the canonical key +/// when it names a protected identity or signing setting (case-insensitive). +/// These are exactly the keys [`crate::identity_signing_entries`] injects: an +/// agent must not be able to redirect authorship or disable/redirect signing. +fn matches_protected_key(cfg: &str) -> Option<&'static str> { + let key = cfg.split('=').next().unwrap_or(cfg).to_ascii_lowercase(); + match key.as_str() { + "user.name" => Some("user.name"), + "user.email" => Some("user.email"), + "user.signingkey" => Some("user.signingkey"), + "commit.gpgsign" => Some("commit.gpgSign"), + "tag.gpgsign" => Some("tag.gpgSign"), + "gpg.format" => Some("gpg.format"), + "gpg.x509.program" => Some("gpg.x509.program"), + "nostr.keyfile" => Some("nostr.keyfile"), + _ => None, + } +} + +/// Split argv into the global-option tokens (including `-c` values) and the +/// index of the subcommand token, if any. Walks the same value-consuming rules +/// git uses so the subcommand is located correctly. +fn split_globals(argv: &[String]) -> (Vec, Option) { + let mut globals = Vec::new(); + let mut i = 0; + while i < argv.len() { + let arg = &argv[i]; + if !arg.starts_with('-') { + return (globals, Some(i)); // first non-option token = subcommand + } + globals.push(arg.clone()); + // `-c`/`-C` and the value-taking long options each consume the next + // token as their value; pull it into globals so the subcommand scan + // doesn't mistake a value for the subcommand. + let takes_value = arg == "-c" || arg == "-C" || VALUE_LONG_OPTS.contains(&arg.as_str()); + if takes_value && i + 1 < argv.len() { + i += 1; + globals.push(argv[i].clone()); + } + i += 1; + } + (globals, None) +} + +/// The git subcommand (first non-option token), or `None` for a bare `git` / +/// `git --version`-style invocation. +fn subcommand(argv: &[String]) -> Option { + let (_, idx) = split_globals(argv); + idx.map(|i| argv[i].clone()) +} + +/// Verify that every commit being pushed that is not already on a remote is +/// authored by the agent identity. `Ok(())` allows the push. +/// +/// The set of refs being pushed is git's own resolved update plan, obtained via +/// `push --no-verify --dry-run --porcelain` rather than reconstructed from a +/// partial argv grammar. That plan reflects `--all`/`--mirror`/`--tags`, +/// `remote..push`, `push.default`, wildcard refspecs, aliases, and `-C` +/// context exactly as git resolves them — the whole class of predictor gaps. +/// `--no-verify` on the *probe* skips the repo's own pre-push hook (the real +/// push still runs it); enforcement itself runs unconditionally, so a +/// `--no-verify` on the real push cannot bypass it. +/// +/// Scope guard against false positives: `rev-list --not --remotes` +/// yields only commits absent from every remote-tracking ref. Pre-existing +/// human commits pulled in by a plain merge are excluded (they are reachable +/// from `refs/remotes/*`). A commit that a cherry-pick or rebase *replayed* +/// gets a new SHA, so it is NOT reachable from a remote and would be flagged — +/// but its patch is identical to an upstream commit, so it is exempted by +/// patch-equivalence ([`patch_equivalent_upstream`]): only genuinely new +/// agent work is required to carry the agent identity. This is what lets a +/// branch carrying rebased/cherry-picked upstream human commits push cleanly. +fn verify_push( + real_git: &Path, + argv: &[String], + ctx: &[String], + authority: &Authority, +) -> Result<(), String> { + let expected = &authority.email; + + // git's resolved update plan. Unreachable remote / any dry-run failure = + // fail closed with the loud message: the real push would fail anyway, and + // an unverifiable plan must never be treated as "nothing to check". + let sources = match resolve_push_sources(real_git, argv) { + Some(s) => s, + None => { + return Err(String::from( + "buzz git wrapper: refusing to push — could not verify outgoing commits: \ + `git push --dry-run` failed (e.g. remote unreachable). Enforcement fails \ + closed rather than let an unverified commit leave the machine.", + )) + } + }; + + let mut offenders = Vec::new(); + for from in sources { + let shas = match rev_list_outgoing(real_git, ctx, &from) { + Some(s) => s, + // The plan named this ref as an update, so it resolves — an inability + // to compute its outgoing range is a verification failure, not an + // empty set. Fail closed. + None => { + return Err(format!( + "buzz git wrapper: refusing to push — could not verify the authorship of \ + outgoing commits for `{from}`. Enforcement fails closed rather than let \ + an unverified commit leave the machine." + )) + } + }; + // Patch-ids of commits on a remote but not on this tip — the pool a + // replayed (cherry-picked/rebased) upstream commit matches. Computed + // lazily and only when a non-agent author is actually found, so the + // ordinary all-agent push pays nothing. + let mut upstream: Option> = None; + for sha in shas { + match commit_author_email(real_git, ctx, &sha) { + Some(email) if &email == expected => {} + Some(email) => { + // A non-agent author is allowed only when this commit is a + // replay (same patch-id) of a commit already upstream — + // i.e. a cherry-picked/rebased human commit, which is + // correct attribution, not new agent work masquerading as + // someone else. Any other non-agent author is an offender. + let pool = + upstream.get_or_insert_with(|| upstream_patch_ids(real_git, ctx, &from)); + match commit_patch_id(real_git, ctx, &sha) { + Some(pid) if pool.contains(&pid) => {} // replayed upstream — exempt + Some(_) => offenders.push((sha, email)), + // No patch-id (e.g. a merge, or diff-tree failed) means + // we cannot prove it is a replay: fail closed on it. + None => offenders.push((sha, email)), + } + } + // Author lookup failed for a commit that rev-list just listed: + // fail closed rather than silently skip. + None => { + return Err(format!( + "buzz git wrapper: refusing to push — could not read the author of \ + outgoing commit `{}`. Enforcement fails closed.", + &sha[..sha.len().min(12)] + )) + } + } + } + } + + if offenders.is_empty() { + return Ok(()); + } + let mut msg = String::from( + "buzz git wrapper: refusing to push — these outgoing commits are not authored \ + by your agent identity (expected author email ", + ); + msg.push_str(expected); + msg.push_str("):\n"); + for (sha, email) in &offenders { + msg.push_str(&format!( + " {} authored by {}\n", + &sha[..sha.len().min(12)], + email + )); + } + msg.push_str( + "Re-author them as your agent identity (e.g. `git rebase` with `--reset-author`-free \ + re-commits under the managed identity) before pushing.", + ); + Err(msg) +} + +/// The local source refs a push will send, per git's own resolved plan. +/// +/// Runs the user's exact invocation with `--dry-run --porcelain --no-verify` +/// injected right after the subcommand token, so config aliases expand and +/// repository context (`-C`, `--git-dir`) applies exactly as in the real push. +/// Returns `None` on any dry-run failure (caller fails closed). Deletions and +/// up-to-date refs contribute no source; every other line's local ref (left of +/// `:` in the `from:to` field) is a source whose outgoing commits are checked. +fn resolve_push_sources(real_git: &Path, argv: &[String]) -> Option> { + let sub_idx = split_globals(argv).1?; + let mut full = argv.to_vec(); + // Inject after the subcommand token (`push` or an alias resolving to it). + full.splice( + sub_idx + 1..sub_idx + 1, + ["--dry-run", "--porcelain", "--no-verify"].map(String::from), + ); + let arg_refs: Vec<&str> = full.iter().map(String::as_str).collect(); + // Bounded: the probe contacts the remote, so an unresponsive remote must not + // hang the wrapper. Timeout returns `None`, which the caller fails closed on. + let out = capture_raw_bounded(real_git, &arg_refs, DRY_RUN_TIMEOUT)?; + if !out.status.success() { + return None; + } + Some(parse_porcelain_sources(&String::from_utf8_lossy( + &out.stdout, + ))) +} + +/// Parse `--porcelain` push output into the set of local source refs whose +/// outgoing commits must be verified. Each machine line is +/// `\t:\t`; header (`To …`) and trailer (`Done`) lines +/// lack the tab-delimited `from:to` field and are ignored. A `-` flag (deletion) +/// or empty `from` (deletion refspec) contributes nothing. +fn parse_porcelain_sources(stdout: &str) -> Vec { + let mut sources = Vec::new(); + for line in stdout.lines() { + let mut fields = line.split('\t'); + let flag = fields.next().unwrap_or(""); + let refspec = match fields.next() { + Some(r) if r.contains(':') => r, + _ => continue, // not a plan line + }; + if flag == "-" { + continue; // deletion + } + let from = refspec.split(':').next().unwrap_or(""); + if !from.is_empty() { + sources.push(from.to_string()); + } + } + sources +} + +/// Preflight the resulting author of a commit-creating invocation and reject it +/// when that author would not be the agent (E). Re-applied identity config +/// cannot fix modes that reuse or preserve another commit's author — +/// `commit -C/-c ` and `commit --amend` stamp the reused/original author +/// onto brand-new content. `--author`/`--reset-author` are already rejected in +/// [`enforce`]; this catches the reuse/amend forms that carry a human author +/// without naming one on the command line. +/// +/// History-preserving replays (`rebase`, `cherry-pick`, `am`) are intentionally +/// untouched: preserving an upstream human author there is correct attribution, +/// and the push gate lets those through because the commits already exist +/// upstream (reachable from `refs/remotes/*`). +fn verify_commit_author( + real_git: &Path, + argv: &[String], + ctx: &[String], + authority: &Authority, +) -> Result<(), String> { + let Some(sub_idx) = split_globals(argv).1 else { + return Ok(()); + }; + if argv[sub_idx] != "commit" { + return Ok(()); + } + let args = &argv[sub_idx + 1..]; + + // The reused/original author source, if any. `-C`/`-c ` reuse that + // commit's author; `--amend` (without a reuse flag) keeps HEAD's author. + let reuse_sha = reuse_commit_arg(args); + let source = if let Some(sha) = reuse_sha { + Some(sha) + } else if args.iter().any(|a| a == "--amend") { + Some("HEAD".to_string()) + } else { + None + }; + let Some(source) = source else { + return Ok(()); // ordinary commit — authored fresh as the agent + }; + + match commit_author_email(real_git, ctx, &source) { + // Reused author is the agent (e.g. amending the agent's own commit, the + // normal fixup flow) — allowed. + Some(email) if email == authority.email => Ok(()), + Some(email) => Err(format!( + "buzz git wrapper: refusing this commit — it would be authored by `{email}`, not \ + your agent identity (`{}`). `commit --amend`/`-c`/`-C` preserve the original \ + commit's author on new content. Make a fresh commit (it is authored as your agent \ + identity automatically) and credit the human with `Co-authored-by`/`Signed-off-by` \ + trailers.", + authority.email + )), + // Can't resolve the reuse source's author: fail closed. + None => Err(format!( + "buzz git wrapper: refusing this commit — could not determine the author that \ + `{source}` would stamp on it. Enforcement fails closed." + )), + } +} + +/// The commit named by a `-C `/`-c ` (or attached `-C`/`-c`) +/// author-and-message-reuse option on a `commit` invocation, if present. Unlike +/// the global `-c key=val` config flag, here `-c`/`-C` are `commit` options +/// whose value is a commit-ish; a value containing `=` is a config key, not a +/// commit, so it is ignored. +fn reuse_commit_arg(args: &[String]) -> Option { + let mut i = 0; + while i < args.len() { + let a = &args[i]; + if a == "-C" || a == "-c" { + return args.get(i + 1).cloned(); + } + if let Some(v) = a + .strip_prefix("-C") + .or_else(|| a.strip_prefix("-c")) + .filter(|v| !v.is_empty() && !v.contains('=')) + { + return Some(v.to_string()); + } + i += 1; + } + None +} + +fn commit_author_email(real_git: &Path, ctx: &[String], sha: &str) -> Option { + capture(real_git, ctx, &["show", "-s", "--format=%ae", sha]) +} + +/// The stable patch-id of a single commit, or `None` if it has no diff patch-id +/// (e.g. a merge commit, or `diff-tree` produced nothing). Used to recognize a +/// cherry-picked/rebased copy of an upstream commit by patch content rather than +/// SHA, which the replay rewrote. +fn commit_patch_id(real_git: &Path, ctx: &[String], sha: &str) -> Option { + let diff = { + let mut args = ctx.to_vec(); + args.extend(["diff-tree", "--root", "-p", sha].map(String::from)); + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let out = capture_raw(real_git, &arg_refs)?; + if !out.status.success() { + return None; + } + out.stdout + }; + let ids = patch_ids_from_diff(real_git, ctx, &diff); + ids.into_iter().next() +} + +/// Patch-ids of every commit reachable from a remote-tracking ref but not from +/// `from` — the pool a replayed upstream commit's patch-id must match. Bounded +/// to the divergence (`--remotes --not `), and computed in one +/// `diff-tree | patch-id` pipeline. Empty on any failure, so a commit can only +/// be *exempted* when a match is positively proven (fail-closed for the gate). +fn upstream_patch_ids( + real_git: &Path, + ctx: &[String], + from: &str, +) -> std::collections::HashSet { + let mut revs_args = ctx.to_vec(); + revs_args.extend(["rev-list", "--remotes", "--not", from].map(String::from)); + let revs_refs: Vec<&str> = revs_args.iter().map(String::as_str).collect(); + let revs = match capture_raw(real_git, &revs_refs) { + Some(o) if o.status.success() => o.stdout, + _ => return std::collections::HashSet::new(), + }; + // Feed the SHA list to `diff-tree --stdin -p`, whose diff stream goes to + // `patch-id`. Do it in two hops (diff-tree captured, then piped to + // patch-id) to reuse the stdin helper without a shell. + let diff = { + let mut args = ctx.to_vec(); + args.extend(["diff-tree", "--stdin", "--root", "-p"].map(String::from)); + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + match capture_raw_with_stdin(real_git, &arg_refs, &revs) { + Some(o) if o.status.success() => o.stdout, + _ => return std::collections::HashSet::new(), + } + }; + patch_ids_from_diff(real_git, ctx, &diff) + .into_iter() + .collect() +} + +/// Run `git patch-id --stable` over a diff stream and return each patch-id (the +/// first whitespace field of every output line). +fn patch_ids_from_diff(real_git: &Path, ctx: &[String], diff: &[u8]) -> Vec { + let mut args = ctx.to_vec(); + args.extend(["patch-id", "--stable"].map(String::from)); + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let out = match capture_raw_with_stdin(real_git, &arg_refs, diff) { + Some(o) if o.status.success() => o.stdout, + _ => return Vec::new(), + }; + String::from_utf8_lossy(&out) + .lines() + .filter_map(|l| l.split_whitespace().next().map(str::to_string)) + .collect() +} + +/// Commits reachable from `tip` but not from any remote-tracking ref. `None` +/// when `tip` does not resolve (rev-list exits non-zero). +fn rev_list_outgoing(real_git: &Path, ctx: &[String], tip: &str) -> Option> { + let mut args = ctx.to_vec(); + args.extend(["rev-list", tip, "--not", "--remotes"].map(String::from)); + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let out = capture_raw(real_git, &arg_refs)?; + if !out.status.success() { + return None; + } + Some( + String::from_utf8_lossy(&out.stdout) + .lines() + .map(str::to_string) + .collect(), + ) +} + +/// Run `git ` and capture trimmed stdout when it succeeds. +/// `ctx` carries repository-context globals (`-C`, `--git-dir`, …) so the probe +/// resolves against the same repository the user's command targets. +fn capture(real_git: &Path, ctx: &[String], args: &[&str]) -> Option { + let mut full = ctx.to_vec(); + full.extend(args.iter().map(|s| s.to_string())); + let arg_refs: Vec<&str> = full.iter().map(String::as_str).collect(); + let out = capture_raw(real_git, &arg_refs)?; + if !out.status.success() { + return None; + } + let s = String::from_utf8_lossy(&out.stdout).trim().to_string(); + (!s.is_empty()).then_some(s) +} + +fn capture_raw(real_git: &Path, args: &[&str]) -> Option { + let mut cmd = std::process::Command::new(real_git); + cmd.args(args); + scrub_env(&mut cmd); + cmd.output().ok() +} + +/// Run `git ` feeding `stdin` to its standard input and capture the +/// output. Used for the `diff-tree --stdin` / `patch-id` pipeline without a +/// shell. These operate on local objects only (no network), so no timeout. +fn capture_raw_with_stdin( + real_git: &Path, + args: &[&str], + stdin: &[u8], +) -> Option { + use std::io::Write; + use std::process::Stdio; + let mut child = { + let mut cmd = std::process::Command::new(real_git); + cmd.args(args); + scrub_env(&mut cmd); + cmd.stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + cmd.spawn().ok()? + }; + child.stdin.take()?.write_all(stdin).ok()?; + child.wait_with_output().ok() +} + +/// Hard ceiling on the push `--dry-run` probe. The probe contacts the remote to +/// resolve `old..new`, so an unresponsive remote could otherwise block the +/// wrapper — and therefore the agent's `git push` — indefinitely. A synchronous +/// unbounded subprocess in an enforcement path is a defect on its own; this +/// bounds it and the caller treats a timeout as fail-closed. +const DRY_RUN_TIMEOUT: std::time::Duration = std::time::Duration::from_secs(120); + +/// Like [`capture_raw`] but killed if it runs past `timeout`. Returns `None` on +/// spawn failure OR timeout (caller fails closed). The process is killed and +/// reaped on timeout so no zombie or detached network client survives. +fn capture_raw_bounded( + real_git: &Path, + args: &[&str], + timeout: std::time::Duration, +) -> Option { + use std::process::Stdio; + use wait_timeout::ChildExt; + let mut child = { + let mut cmd = std::process::Command::new(real_git); + cmd.args(args); + scrub_env(&mut cmd); + cmd.stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + cmd.spawn().ok()? + }; + match child.wait_timeout(timeout).ok()? { + Some(_status) => child.wait_with_output().ok(), + None => { + // Timed out: kill and reap, then report failure (None). + let _ = child.kill(); + let _ = child.wait(); + None + } + } +} + +fn scrub_env(cmd: &mut std::process::Command) { + for var in SCRUBBED_ENV { + cmd.env_remove(var); + } +} + +/// Locate the real `git`: the first PATH entry whose `git` does not resolve back +/// to this binary (the wrapper symlink). Canonicalization defeats the symlink so +/// we never exec ourselves. +fn find_real_git() -> Option { + let self_canon = std::env::current_exe() + .ok() + .and_then(|p| p.canonicalize().ok()); + let git_name = if cfg!(windows) { "git.exe" } else { "git" }; + + for dir in std::env::split_paths(&std::env::var_os("PATH").unwrap_or_default()) { + let candidate = dir.join(git_name); + if !candidate.is_file() { + continue; + } + let cand_canon = candidate.canonicalize().ok(); + if cand_canon.is_some() && cand_canon == self_canon { + continue; // this is our own wrapper symlink + } + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let executable = std::fs::metadata(&candidate) + .map(|m| m.permissions().mode() & 0o111 != 0) + .unwrap_or(false); + if !executable { + continue; + } + } + return Some(candidate); + } + None +} + +/// Build the real-git argv with the authoritative identity/signing config +/// injected as command-line `-c key=value` options placed immediately before +/// the subcommand — i.e. after every global option the caller passed. +/// +/// Command-line `-c` is git's highest-precedence configuration channel: it wins +/// over repo/global/system config files, over the `GIT_CONFIG_*` environment, +/// over `GIT_CONFIG_PARAMETERS`, and over `-c include.path=…`/`includeIf` +/// includes (whose settings enter at the position of their own `-c`, which the +/// caller can only place *before* ours). Placing our entries last among the +/// globals therefore makes them win regardless of what config channel the agent +/// used — the whole class of "some other channel outranks the appended env" +/// bypasses — without the wrapper having to enumerate or reject those channels. +/// +/// Author/committer env vars and the command-line `--author`/`--reset-author`/ +/// `--no-gpg-sign`/`-c ` forms outrank even command-line `-c`; those +/// are handled separately (scrubbed and rejected in [`enforce`]). +fn inject_identity_args(argv: &[String], authority: Option<&Authority>) -> Vec { + let Some(authority) = authority else { + return argv.to_vec(); + }; + // Splice point: the subcommand index (first non-option token), or the end + // for a bare `git`/`git --version`-style call where the position is moot. + let at = split_globals(argv).1.unwrap_or(argv.len()); + let mut out = argv[..at].to_vec(); + for (key, value) in &authority.entries { + out.push("-c".to_string()); + out.push(format!("{key}={value}")); + } + out.extend_from_slice(&argv[at..]); + out +} + +#[cfg(unix)] +fn exec_real_git(real_git: &Path, argv: &[String], authority: Option<&Authority>) -> i32 { + use std::os::unix::process::CommandExt; + let full = inject_identity_args(argv, authority); + let mut cmd = std::process::Command::new(real_git); + cmd.args(&full); + scrub_env(&mut cmd); + // exec replaces this process; on success it never returns. If it returns, + // the exec itself failed. + let err = cmd.exec(); + eprintln!("buzz git wrapper: failed to exec real git: {err}"); + 127 +} + +#[cfg(not(unix))] +fn exec_real_git(real_git: &Path, argv: &[String], authority: Option<&Authority>) -> i32 { + let full = inject_identity_args(argv, authority); + let mut cmd = std::process::Command::new(real_git); + cmd.args(&full); + scrub_env(&mut cmd); + match cmd.status() { + Ok(status) => status.code().unwrap_or(1), + Err(e) => { + eprintln!("buzz git wrapper: failed to run real git: {e}"); + 127 + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn v(args: &[&str]) -> Vec { + args.iter().map(|s| s.to_string()).collect() + } + + const AGENT_EMAIL: &str = + "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa@relay.test"; + + /// A managed-session authority with the standard identity/signing entries, + /// so `enforce`/`verify_*` behave as they do in a real managed session. + fn managed() -> Authority { + Authority { + entries: vec![ + ("user.name".into(), "Agent".into()), + ("user.email".into(), AGENT_EMAIL.into()), + ("commit.gpgSign".into(), "true".into()), + ], + email: AGENT_EMAIL.into(), + } + } + + /// Like [`managed`] but with signing disabled, for tests that create real + /// commits through the injected argv (no GPG key available in CI). + fn managed_nosign() -> Authority { + Authority { + entries: vec![ + ("user.name".into(), "Agent".into()), + ("user.email".into(), AGENT_EMAIL.into()), + ("commit.gpgSign".into(), "false".into()), + ], + email: AGENT_EMAIL.into(), + } + } + + // ── enforce: only enforces in a managed session ─────────────────────────── + + #[test] + fn enforce_is_a_noop_without_an_authority() { + // An unmanaged session (no manifest) must not reject anything. + for argv in [ + v(&["-c", "user.email=evil@x", "commit"]), + v(&["commit", "--author=Evil "]), + v(&["commit", "--no-gpg-sign"]), + ] { + assert!( + enforce(&argv, None).is_ok(), + "unmanaged must allow {argv:?}" + ); + } + } + + // ── enforce: -c identity/signing rejection ──────────────────────────────── + + #[test] + fn rejects_dash_c_protected_keys_in_global_position() { + let a = managed(); + for argv in [ + v(&["-c", "user.name=Evil", "commit"]), + v(&["-c", "user.email=evil@x.com", "commit"]), + v(&["-cuser.name=Evil", "commit"]), // attached form + v(&["-cuser.email=e@x", "commit"]), // attached form + v(&["-c", "USER.EMAIL=e@x", "commit"]), // case-insensitive key + v(&["-c", "commit.gpgSign=false", "commit"]), // signing disable (F) + v(&["-c", "user.signingkey=abc", "commit"]), + v(&["-c", "nostr.keyfile=/tmp/evil", "commit"]), + v(&["-c", "gpg.x509.program=/bin/false", "commit"]), + ] { + assert!(enforce(&argv, Some(&a)).is_err(), "must reject {argv:?}"); + } + } + + #[test] + fn allows_dash_c_for_unrelated_config_keys() { + let a = managed(); + for argv in [ + v(&["-c", "core.pager=less", "log"]), + v(&["-c", "http.proxy=x", "fetch"]), + ] { + assert!(enforce(&argv, Some(&a)).is_ok(), "must allow {argv:?}"); + } + } + + #[test] + fn commit_dash_c_reuse_message_is_not_a_config_override() { + // `git commit -c ` reuses a message; `-c` here is a commit + // option, not the global config flag. It must not be misread as one. + let a = managed(); + assert!(enforce(&v(&["commit", "-c", "HEAD~1"]), Some(&a)).is_ok()); + assert!(enforce(&v(&["commit", "-cuser.name=x"]), Some(&a)).is_ok()); + } + + // ── enforce: --config-env rejection ─────────────────────────────────────── + + #[test] + fn rejects_config_env_for_protected_keys() { + let a = managed(); + assert!(enforce(&v(&["--config-env=user.name=VAR", "commit"]), Some(&a)).is_err()); + assert!(enforce(&v(&["--config-env=user.email=VAR", "commit"]), Some(&a)).is_err()); + assert!(enforce(&v(&["--config-env=commit.gpgSign=VAR", "commit"]), Some(&a)).is_err()); + } + + #[test] + fn allows_config_env_for_unrelated_keys() { + let a = managed(); + assert!(enforce(&v(&["--config-env=http.proxy=PROXY", "fetch"]), Some(&a)).is_ok()); + } + + // ── enforce: --author / --reset-author / --no-gpg-sign scoping ──────────── + + #[test] + fn rejects_author_overrides_on_commit_and_am() { + let a = managed(); + for argv in [ + v(&["commit", "--author=Evil "]), + v(&["commit", "--author", "Evil "]), + v(&["commit", "--reset-author"]), + v(&["am", "--author=Evil "]), + ] { + assert!(enforce(&argv, Some(&a)).is_err(), "must reject {argv:?}"); + } + } + + #[test] + fn rejects_no_gpg_sign_on_signing_subcommands() { + let a = managed(); + for argv in [ + v(&["commit", "--no-gpg-sign"]), + v(&["tag", "-a", "v1", "--no-gpg-sign"]), + v(&["rebase", "--no-gpg-sign", "main"]), + ] { + assert!(enforce(&argv, Some(&a)).is_err(), "must reject {argv:?}"); + } + } + + #[test] + fn allows_author_filter_on_read_side_subcommands() { + // log/shortlog/blame --author are legitimate read filters. + let a = managed(); + for argv in [ + v(&["log", "--author=Duncan"]), + v(&["shortlog", "--author", "Duncan"]), + v(&["log", "--no-gpg-sign"]), // not a signing subcommand → allowed + ] { + assert!(enforce(&argv, Some(&a)).is_ok(), "must allow {argv:?}"); + } + } + + #[test] + fn author_override_after_global_options_is_still_rejected() { + let a = managed(); + assert!(enforce( + &v(&["-C", "/repo", "commit", "--author=Evil "]), + Some(&a) + ) + .is_err()); + } + + // ── split_globals / subcommand ──────────────────────────────────────────── + + #[test] + fn split_globals_locates_subcommand_after_value_consuming_options() { + assert_eq!(subcommand(&v(&["commit"])).as_deref(), Some("commit")); + assert_eq!( + subcommand(&v(&["-C", "/repo", "-c", "core.x=y", "push"])).as_deref(), + Some("push") + ); + assert_eq!( + subcommand(&v(&["--git-dir", "/g", "status"])).as_deref(), + Some("status") + ); + assert_eq!(subcommand(&v(&["--version"])), None); + assert_eq!(subcommand(&[]), None); + } + + // ── parse_porcelain_sources ─────────────────────────────────────────────── + + #[test] + fn porcelain_parse_extracts_update_sources_and_skips_deletes_and_headers() { + let stdout = "To ../remote.git\n\ + \trefs/heads/main:refs/heads/main\t4ab76d3..c0bab62\n\ + *\trefs/heads/newbr:refs/heads/newbr\t[new branch]\n\ + =\trefs/heads/up:refs/heads/up\t[up to date]\n\ + -\t:refs/heads/tokill\t[deleted]\n\ + Done\n"; + assert_eq!( + parse_porcelain_sources(stdout), + vec!["refs/heads/main", "refs/heads/newbr", "refs/heads/up"] + ); + } + + #[test] + fn porcelain_parse_ignores_lines_without_a_refspec_field() { + // Header/trailer and any stray non-tab lines contribute nothing. + assert!(parse_porcelain_sources("To origin\nDone\n").is_empty()); + assert!(parse_porcelain_sources("").is_empty()); + } + + // ── reuse_commit_arg (E) ────────────────────────────────────────────────── + + #[test] + fn reuse_commit_arg_detects_c_and_capital_c_forms() { + assert_eq!( + reuse_commit_arg(&v(&["-C", "HEAD~1"])).as_deref(), + Some("HEAD~1") + ); + assert_eq!( + reuse_commit_arg(&v(&["-c", "abc123"])).as_deref(), + Some("abc123") + ); + assert_eq!(reuse_commit_arg(&v(&["-CHEAD"])).as_deref(), Some("HEAD")); + // A `-c key=val` config value is not a commit reuse. + assert_eq!(reuse_commit_arg(&v(&["-cuser.name=x"])), None); + assert_eq!(reuse_commit_arg(&v(&["-m", "msg"])), None); + } + + // ── inline_aliases / repo_context_args ──────────────────────────────────── + + #[test] + fn inline_aliases_parses_dash_c_alias_definitions() { + let (globals, _) = split_globals(&v(&["-c", "alias.pub=push", "-calias.p=push", "pub"])); + let map = inline_aliases(&globals); + assert_eq!(map.get("pub").map(String::as_str), Some("push")); + assert_eq!(map.get("p").map(String::as_str), Some("push")); + } + + #[test] + fn repo_context_args_extracts_repository_context_globals() { + assert_eq!( + repo_context_args(&v(&["-C", "/repo", "-c", "core.x=y", "push"])), + v(&["-C", "/repo"]) + ); + assert_eq!( + repo_context_args(&v(&["--git-dir=/g", "status"])), + v(&["--git-dir=/g"]) + ); + assert!(repo_context_args(&v(&["push"])).is_empty()); + } + + // ── manifest round-trip ─────────────────────────────────────────────────── + + #[test] + fn authority_loads_identity_and_email_from_manifest() { + let dir = tempfile::tempdir().unwrap(); + let entries = vec![ + ("user.name".to_string(), "Agent".to_string()), + ("user.email".to_string(), AGENT_EMAIL.to_string()), + ("commit.gpgSign".to_string(), "true".to_string()), + ]; + crate::write_identity_manifest(dir.path(), &entries).unwrap(); + let parsed = crate::read_identity_manifest(dir.path()).unwrap(); + assert_eq!(parsed, entries); + let email = parsed + .iter() + .find(|(k, _)| k == "user.email") + .map(|(_, v)| v.clone()); + assert_eq!(email.as_deref(), Some(AGENT_EMAIL)); + } + + #[test] + fn read_identity_manifest_is_none_when_absent() { + let dir = tempfile::tempdir().unwrap(); + assert!(crate::read_identity_manifest(dir.path()).is_none()); + } + + // ── is_push_command / verify_push / verify_commit_author against real git ── + + /// Build a repo whose HEAD carries a deliberately human-authored commit and + /// return `(tempdir, repo_path)`. No remote, so `--not --remotes` yields the + /// full history — the commit shows as outgoing. + fn human_authored_repo() -> (tempfile::TempDir, PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path().to_path_buf(); + let git = |args: &[&str]| { + let ok = std::process::Command::new("git") + .args(args) + .current_dir(&repo) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .status() + .unwrap() + .success(); + assert!(ok, "git {args:?} failed"); + }; + git(&["init", "-q", "-b", "main"]); + git(&["config", "user.name", "Human"]); + git(&["config", "user.email", "human@example.com"]); + git(&["config", "commit.gpgSign", "false"]); + git(&["config", "alias.pub", "push"]); + std::fs::write(repo.join("f"), "x").unwrap(); + git(&["add", "f"]); + git(&["commit", "-qm", "human commit"]); + (dir, repo) + } + + fn real_git() -> PathBuf { + PathBuf::from("git") + } + + #[test] + fn config_alias_resolving_to_push_is_recognized() { + let (_d, repo) = human_authored_repo(); + let ctx = vec!["-C".to_string(), repo.to_string_lossy().into_owned()]; + // `git pub` → alias.pub = push. + assert!(matches!( + is_push_command( + &real_git(), + &v(&["-C", repo.to_str().unwrap(), "pub"]), + &ctx + ), + PushKind::Push + )); + // A non-push subcommand is not misclassified. + assert!(matches!( + is_push_command( + &real_git(), + &v(&["-C", repo.to_str().unwrap(), "status"]), + &ctx + ), + PushKind::NotPush + )); + } + + #[test] + fn inline_alias_resolving_to_push_is_recognized() { + let ctx: Vec = vec![]; + assert!(matches!( + is_push_command(&real_git(), &v(&["-c", "alias.pub=push", "pub"]), &ctx), + PushKind::Push + )); + } + + // ── verify_alias_safety (allowlist) ─────────────────────────────────────── + + #[test] + fn is_safe_alias_token_admits_only_trivial_bare_words() { + // Safe: bare subcommands and plain arguments. + assert!(is_safe_alias_token("commit")); + assert!(is_safe_alias_token("status")); + assert!(is_safe_alias_token("--oneline")); + assert!(is_safe_alias_token("origin")); + assert!(is_safe_alias_token("main")); + // Unsafe: config channels in any spelling. + assert!(!is_safe_alias_token("-c")); + assert!(!is_safe_alias_token("-cuser.email=x")); + assert!(!is_safe_alias_token("--config-env")); + assert!(!is_safe_alias_token("--config-env=user.name=VAR")); + // Unsafe: any quote/escape (git dequotes; we never guess). + assert!(!is_safe_alias_token("'-c'")); + assert!(!is_safe_alias_token("\"commit\"")); + assert!(!is_safe_alias_token("a\\b")); + // Unsafe: a value-bearing option. + assert!(!is_safe_alias_token("--author=Evil ")); + } + + #[test] + fn verify_alias_safety_rejects_config_and_quoted_aliases() { + let ctx: Vec = vec![]; + // Bare `-c` config channel. + assert!(verify_alias_safety( + &real_git(), + &v(&["-c", "alias.hc=-c user.email=e@x commit", "hc"]), + &ctx + ) + .is_err()); + // `--config-env` channel. + assert!(verify_alias_safety( + &real_git(), + &v(&["-c", "alias.hc=--config-env=user.name=V commit", "hc"]), + &ctx + ) + .is_err()); + // Quoted tokens — the parser-parity bypass; refused without dequoting. + assert!(verify_alias_safety( + &real_git(), + &v(&["-c", "alias.q='-c' 'user.email=q@x' commit", "q"]), + &ctx + ) + .is_err()); + } + + #[test] + fn verify_alias_safety_rejects_all_shell_aliases() { + let ctx: Vec = vec![]; + // A shell alias with no push and no config is still refused in managed mode. + assert!( + verify_alias_safety(&real_git(), &v(&["-c", "alias.sh=!git status", "sh"]), &ctx) + .is_err() + ); + // The commit-path shell bypass Thufir demonstrated. + assert!(verify_alias_safety( + &real_git(), + &v(&[ + "-c", + "alias.sc=!f(){ git -c user.email=shell@x commit \"$@\"; }; f", + "sc", + ]), + &ctx + ) + .is_err()); + } + + #[test] + fn verify_alias_safety_allows_bare_word_aliases() { + let ctx: Vec = vec![]; + // Gurney's certified working shapes must all stay allowed, and resolve to + // their expansion so the caller can hold it to the direct-command policy. + assert_eq!( + verify_alias_safety(&real_git(), &v(&["-c", "alias.ci=commit", "ci"]), &ctx).unwrap(), + Some(v(&["-c", "alias.ci=commit", "commit"])) + ); + assert_eq!( + verify_alias_safety(&real_git(), &v(&["-c", "alias.st=status", "st"]), &ctx).unwrap(), + Some(v(&["-c", "alias.st=status", "status"])) + ); + assert_eq!( + verify_alias_safety( + &real_git(), + &v(&["-c", "alias.lg=log --oneline", "lg"]), + &ctx + ) + .unwrap(), + Some(v(&["-c", "alias.lg=log --oneline", "log", "--oneline"])) + ); + assert_eq!( + verify_alias_safety( + &real_git(), + &v(&["-c", "alias.pub=push origin main", "pub"]), + &ctx + ) + .unwrap(), + Some(v(&[ + "-c", + "alias.pub=push origin main", + "push", + "origin", + "main" + ])) + ); + // A real (non-alias) subcommand resolves immediately with no expansion. + assert_eq!( + verify_alias_safety(&real_git(), &v(&["commit", "-m", "x"]), &ctx).unwrap(), + None + ); + } + + #[test] + fn verify_alias_safety_expands_bare_word_flags_and_appends_trailing_argv() { + let ctx: Vec = vec![]; + // Thufir's rd-4 bypass shape: every body token is a bare word, so the + // allowlist admits it — but the returned expansion carries the flags and + // the caller's trailing argv, so the direct-command preflight can catch + // `--author`/`--no-gpg-sign`. This is the unification contract. + assert_eq!( + verify_alias_safety( + &real_git(), + &v(&[ + "-c", + "alias.human=commit --author Human --no-gpg-sign", + "human", + "-m", + "leak", + ]), + &ctx + ) + .unwrap(), + Some(v(&[ + "-c", + "alias.human=commit --author Human --no-gpg-sign", + "commit", + "--author", + "Human", + "--no-gpg-sign", + "-m", + "leak", + ])) + ); + // A chain accumulates body tokens across hops onto the final command. + assert_eq!( + verify_alias_safety( + &real_git(), + &v(&[ + "-c", + "alias.chain=co --no-gpg-sign", + "-c", + "alias.co=commit", + "chain", + ]), + &ctx + ) + .unwrap(), + Some(v(&[ + "-c", + "alias.chain=co --no-gpg-sign", + "-c", + "alias.co=commit", + "commit", + "--no-gpg-sign", + ])) + ); + } + + #[test] + fn verify_alias_safety_walks_bare_word_chains_and_rejects_config_at_the_end() { + let ctx: Vec = vec![]; + // `a` → `b` (both bare-word) → allowed. + assert!(verify_alias_safety( + &real_git(), + &v(&["-c", "alias.a=b", "-c", "alias.b=commit", "a"]), + &ctx + ) + .is_ok()); + // `a` → `b` where `b` introduces config → refused via the chain. + assert!(verify_alias_safety( + &real_git(), + &v(&[ + "-c", + "alias.a=b", + "-c", + "alias.b=-c commit.gpgSign=false commit", + "a", + ]), + &ctx + ) + .is_err()); + } + + #[test] + fn verify_push_rejects_human_commit_via_git_resolved_plan() { + // No remote configured: the dry-run to a bogus remote fails, so the + // push fails closed. Point a real remote at a fresh bare repo so the + // plan resolves and HEAD (human-authored) shows as an offender. + let (_d, repo) = human_authored_repo(); + let remote = tempfile::tempdir().unwrap(); + let run = |args: &[&str]| { + std::process::Command::new("git") + .args(args) + .status() + .unwrap(); + }; + run(&["init", "-q", "--bare", remote.path().to_str().unwrap()]); + run(&[ + "-C", + repo.to_str().unwrap(), + "remote", + "add", + "origin", + remote.path().to_str().unwrap(), + ]); + let ctx = vec!["-C".to_string(), repo.to_string_lossy().into_owned()]; + let err = verify_push( + &real_git(), + &v(&["-C", repo.to_str().unwrap(), "push", "origin", "main"]), + &ctx, + &managed(), + ) + .expect_err("human-authored HEAD must be refused"); + assert!(err.contains("not authored by your agent identity"), "{err}"); + } + + #[test] + fn verify_push_fails_closed_when_remote_unreachable() { + let (_d, repo) = human_authored_repo(); + // origin points at a nonexistent path → dry-run fails → fail closed. + std::process::Command::new("git") + .args([ + "-C", + repo.to_str().unwrap(), + "remote", + "add", + "origin", + "/no/such/remote.git", + ]) + .status() + .unwrap(); + let ctx = vec!["-C".to_string(), repo.to_string_lossy().into_owned()]; + let err = verify_push( + &real_git(), + &v(&["-C", repo.to_str().unwrap(), "push", "origin", "main"]), + &ctx, + &managed(), + ) + .expect_err("unreachable remote must fail closed"); + assert!(err.contains("could not verify outgoing commits"), "{err}"); + } + + #[test] + fn verify_commit_author_rejects_reuse_of_human_author() { + // `commit -C ` would stamp the human author on new content. + let (_d, repo) = human_authored_repo(); + let ctx = vec!["-C".to_string(), repo.to_string_lossy().into_owned()]; + let err = verify_commit_author( + &real_git(), + &v(&["-C", repo.to_str().unwrap(), "commit", "-C", "HEAD"]), + &ctx, + &managed(), + ) + .expect_err("reusing a human author must be refused"); + assert!(err.contains("not"), "{err}"); + } + + #[test] + fn verify_commit_author_allows_ordinary_and_agent_amend() { + let (_d, repo) = human_authored_repo(); + let ctx = vec!["-C".to_string(), repo.to_string_lossy().into_owned()]; + // Ordinary commit (no reuse/amend) is authored fresh as the agent. + assert!(verify_commit_author( + &real_git(), + &v(&["-C", repo.to_str().unwrap(), "commit", "-m", "x"]), + &ctx, + &managed(), + ) + .is_ok()); + // Amending a commit already authored by the agent is the normal fixup + // flow and must be allowed. + let agent = managed(); + let agent_repo = tempfile::tempdir().unwrap(); + let ar = agent_repo.path(); + let g = |args: &[&str]| { + std::process::Command::new("git") + .args(args) + .current_dir(ar) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .status() + .unwrap(); + }; + g(&["init", "-q", "-b", "main"]); + g(&["config", "user.name", "Agent"]); + g(&["config", "user.email", AGENT_EMAIL]); + g(&["config", "commit.gpgSign", "false"]); + std::fs::write(ar.join("f"), "x").unwrap(); + g(&["add", "f"]); + g(&["commit", "-qm", "agent commit"]); + let ctx2 = vec!["-C".to_string(), ar.to_string_lossy().into_owned()]; + assert!(verify_commit_author( + &real_git(), + &v(&["-C", ar.to_str().unwrap(), "commit", "--amend", "--no-edit"]), + &ctx2, + &agent, + ) + .is_ok()); + } + + // ── helpers for exec-level identity/push tests ───────────────────────────── + + /// Run `git` in `repo` with the given argv and hermetic global/system config. + fn git_in(repo: &Path, args: &[&str]) -> std::process::Output { + std::process::Command::new("git") + .args(args) + .current_dir(repo) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .output() + .unwrap() + } + + /// Author email of `rev` in `repo`, trimmed. + fn author_email(repo: &Path, rev: &str) -> String { + let out = git_in(repo, &["show", "-s", "--format=%ae", rev]); + String::from_utf8_lossy(&out.stdout).trim().to_string() + } + + // ── C1: injected `-c` identity outranks every other config channel ───────── + + /// The wrapper's re-applied command-line `-c user.email=…` must dominate the + /// author even when the agent tries to smuggle a human identity in through a + /// lower-precedence channel: `GIT_CONFIG_PARAMETERS`, a `-c include.path` + /// include, and repo-file config. Exercised through `inject_identity_args` + /// (what `exec_real_git` splices) plus a real `git commit`. + #[test] + fn injected_identity_outranks_config_parameters_and_include_path() { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path(); + git_in(repo, &["init", "-q", "-b", "main"]); + // Repo-file config claims a human identity (a legitimate lower channel). + git_in(repo, &["config", "user.name", "Human"]); + git_in(repo, &["config", "user.email", "human@example.com"]); + git_in(repo, &["config", "commit.gpgSign", "false"]); + + // An include file that also tries to set a human identity. + let inc = repo.join("evil.inc"); + std::fs::write(&inc, "[user]\n\temail = include@evil.com\n").unwrap(); + + std::fs::write(repo.join("f"), "x").unwrap(); + git_in(repo, &["add", "f"]); + + // Caller argv smuggles identity via a `-c include.path` global. The + // wrapper splices its authoritative `-c user.email=` AFTER this, + // so command-line precedence (last `-c` wins) must make the agent win. + let caller = v(&[ + "-c", + &format!("include.path={}", inc.display()), + "commit", + "-qm", + "smuggled", + ]); + let full = inject_identity_args(&caller, Some(&managed_nosign())); + let refs: Vec<&str> = full.iter().map(String::as_str).collect(); + + // Also arm the env channel the wrapper re-append is meant to defeat. + let params = "'user.email=params@evil.com'"; + let out = std::process::Command::new("git") + .args(&refs) + .current_dir(repo) + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null") + .env("GIT_CONFIG_PARAMETERS", params) + .output() + .unwrap(); + assert!( + out.status.success(), + "commit failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + assert_eq!( + author_email(repo, "HEAD"), + AGENT_EMAIL, + "injected -c identity must beat include.path, GIT_CONFIG_PARAMETERS, and repo config" + ); + } + + // ── C2: shell (`!`) aliases are refused outright in a managed session ───── + + /// `verify_alias_safety` must refuse a `!`-shell alias (here one whose body + /// would push) without executing it. A sentinel file proves the body never + /// runs during classification — refusal is by source inspection only. + #[test] + fn shell_alias_is_refused_without_executing_its_body() { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path(); + git_in(repo, &["init", "-q", "-b", "main"]); + let sentinel = repo.join("ran"); + let body = format!("!touch {} && git push", sentinel.display()); + git_in(repo, &["config", "alias.deploy", &body]); + + let ctx = vec!["-C".to_string(), repo.to_string_lossy().into_owned()]; + let err = verify_alias_safety( + &real_git(), + &v(&["-C", repo.to_str().unwrap(), "deploy"]), + &ctx, + ); + assert!(err.is_err(), "shell alias must be refused"); + assert!( + err.unwrap_err().contains("shell (`!`) git alias"), + "expected the shell-alias rejection message" + ); + assert!( + !sentinel.exists(), + "classification must not execute the shell alias body" + ); + } + + /// A non-push `!`-shell alias is refused too — the ruling rejects ALL shell + /// aliases in a managed session, not only push-bearing ones. + #[test] + fn shell_alias_without_push_is_also_refused() { + let dir = tempfile::tempdir().unwrap(); + let repo = dir.path(); + git_in(repo, &["init", "-q", "-b", "main"]); + git_in(repo, &["config", "alias.st", "!git status"]); + let ctx = vec!["-C".to_string(), repo.to_string_lossy().into_owned()]; + assert!( + verify_alias_safety(&real_git(), &v(&["-C", repo.to_str().unwrap(), "st"]), &ctx) + .is_err() + ); + } + + // ── I3: cherry-picked / rebased upstream human commits are exempt ────────── + + /// Build `(dir, local, remote)` where `remote` (a real bare repo wired as + /// `origin` and fetched) carries a human-authored commit, and `local` is on + /// a branch forked from the shared base. Returns paths for building the two + /// rebase/cherry-pick shapes on top. + fn repo_with_upstream_human_commit() -> (tempfile::TempDir, PathBuf, PathBuf) { + let dir = tempfile::tempdir().unwrap(); + let remote = dir.path().join("remote.git"); + let local = dir.path().join("local"); + // Seed remote via a scratch working clone, then discard it. + git_in( + dir.path(), + &["init", "-q", "--bare", remote.to_str().unwrap()], + ); + let seed = dir.path().join("seed"); + git_in( + dir.path(), + &[ + "clone", + "-q", + remote.to_str().unwrap(), + seed.to_str().unwrap(), + ], + ); + git_in(&seed, &["config", "user.name", "Human"]); + git_in(&seed, &["config", "user.email", "human@example.com"]); + git_in(&seed, &["config", "commit.gpgSign", "false"]); + std::fs::write(seed.join("base"), "b").unwrap(); + git_in(&seed, &["add", "base"]); + git_in(&seed, &["commit", "-qm", "base"]); + std::fs::write(seed.join("human"), "h").unwrap(); + git_in(&seed, &["add", "human"]); + git_in(&seed, &["commit", "-qm", "human work"]); + git_in(&seed, &["push", "-q", "origin", "HEAD:main"]); + + // Local clone forked from the shared BASE (not the human tip). + git_in( + dir.path(), + &[ + "clone", + "-q", + remote.to_str().unwrap(), + local.to_str().unwrap(), + ], + ); + git_in(&local, &["config", "user.name", "Agent"]); + git_in(&local, &["config", "user.email", AGENT_EMAIL]); + git_in(&local, &["config", "commit.gpgSign", "false"]); + (dir, local, remote) + } + + /// I3 shape B (the shape that exercises the fix): the agent rebases/rewrites + /// the UPSTREAM human commit onto a new base, giving it a fresh SHA. That + /// new SHA is not reachable from `refs/remotes/*`, so the naive + /// `rev-list --not --remotes` flags it — but its patch-id matches the + /// upstream original, so the exemption must let the push through. + #[test] + fn verify_push_exempts_rebased_upstream_human_commit_by_patch_id() { + let (_d, local, _remote) = repo_with_upstream_human_commit(); + // Reset local to the shared base, then cherry-pick the upstream human + // commit — a replay that rewrites its SHA but preserves its patch and + // its human author. This is the correct-attribution case the gate must + // NOT refuse. + git_in(&local, &["reset", "-q", "--hard", "origin/main~1"]); + let human_sha = + String::from_utf8_lossy(&git_in(&local, &["rev-parse", "origin/main"]).stdout) + .trim() + .to_string(); + // Add an agent commit first, then replay the human commit on top so the + // outgoing range is {agent, replayed-human} — both must be allowed. + std::fs::write(local.join("agent"), "a").unwrap(); + git_in(&local, &["add", "agent"]); + git_in(&local, &["commit", "-qm", "agent work"]); + let cp = git_in(&local, &["cherry-pick", &human_sha]); + assert!( + cp.status.success(), + "cherry-pick failed: {}", + String::from_utf8_lossy(&cp.stderr) + ); + + let ctx = vec!["-C".to_string(), local.to_string_lossy().into_owned()]; + let res = verify_push( + &real_git(), + &v(&[ + "-C", + local.to_str().unwrap(), + "push", + "origin", + "HEAD:refs/heads/feature", + ]), + &ctx, + &managed_nosign(), + ); + assert!( + res.is_ok(), + "replayed upstream human commit must be exempt by patch-id, got: {res:?}" + ); + } + + /// I3 shape A (agent-onto-human, the ordinary rebase): the agent's own + /// commit sits on top of the upstream human tip. Only the agent commit is + /// outgoing; the human commit is already reachable from `refs/remotes/*`. + /// The push must be allowed, and it exercises the no-exemption-needed path. + #[test] + fn verify_push_allows_agent_commit_atop_upstream_human_tip() { + let (_d, local, _remote) = repo_with_upstream_human_commit(); + std::fs::write(local.join("agent"), "a").unwrap(); + git_in(&local, &["add", "agent"]); + git_in(&local, &["commit", "-qm", "agent work"]); + let ctx = vec!["-C".to_string(), local.to_string_lossy().into_owned()]; + let res = verify_push( + &real_git(), + &v(&[ + "-C", + local.to_str().unwrap(), + "push", + "origin", + "HEAD:refs/heads/feature", + ]), + &ctx, + &managed_nosign(), + ); + assert!( + res.is_ok(), + "agent commit atop upstream human tip must be allowed: {res:?}" + ); + } + + /// A genuinely NEW human-authored commit (no upstream patch-id match) is + /// still refused — the patch-id exemption must not become a blanket pass. + #[test] + fn verify_push_still_rejects_new_human_commit_without_upstream_match() { + let (_d, local, _remote) = repo_with_upstream_human_commit(); + // A fresh human-authored commit that exists nowhere upstream. + std::fs::write(local.join("new"), "n").unwrap(); + git_in(&local, &["add", "new"]); + git_in( + &local, + &[ + "-c", + "user.name=Human", + "-c", + "user.email=human@example.com", + "commit", + "-qm", + "brand-new human work", + ], + ); + let ctx = vec!["-C".to_string(), local.to_string_lossy().into_owned()]; + let err = verify_push( + &real_git(), + &v(&[ + "-C", + local.to_str().unwrap(), + "push", + "origin", + "HEAD:refs/heads/feature", + ]), + &ctx, + &managed_nosign(), + ) + .expect_err("a brand-new human commit must be refused"); + assert!(err.contains("not authored by your agent identity"), "{err}"); + } + + // ── I6: the dry-run probe is bounded and a timeout fails closed ──────────── + + /// `capture_raw_bounded` must kill and report failure (`None`) when the + /// child outlives the timeout, so a hung remote probe cannot block the + /// wrapper indefinitely. Uses a tiny timeout against a sleep to prove the + /// bound fires without depending on real network latency. + #[test] + fn capture_raw_bounded_times_out_and_fails_closed() { + // `sleep` via any binary on PATH would do; use the shell so the timeout + // is deterministic regardless of installed git. We invoke `sh -c sleep` + // as the "real git" stand-in — capture_raw_bounded only cares that the + // child runs longer than the timeout. + let start = std::time::Instant::now(); + let out = capture_raw_bounded( + Path::new("sh"), + &["-c", "sleep 5"], + std::time::Duration::from_millis(200), + ); + assert!( + out.is_none(), + "a child exceeding the timeout must yield None" + ); + assert!( + start.elapsed() < std::time::Duration::from_secs(3), + "the bound must fire well before the child would finish" + ); + } + + /// A child that completes within the timeout returns its output normally. + #[test] + fn capture_raw_bounded_returns_output_within_timeout() { + let out = capture_raw_bounded( + Path::new("sh"), + &["-c", "printf ok"], + std::time::Duration::from_secs(5), + ) + .expect("fast child must produce output"); + assert!(out.status.success()); + assert_eq!(String::from_utf8_lossy(&out.stdout), "ok"); + } +} diff --git a/crates/buzz-git-identity/src/lib.rs b/crates/buzz-git-identity/src/lib.rs new file mode 100644 index 00000000000..cef90061119 --- /dev/null +++ b/crates/buzz-git-identity/src/lib.rs @@ -0,0 +1,902 @@ +//! Deterministic agent git identity — the single source of truth for the +//! `GIT_CONFIG_*` environment that attributes and signs an agent's commits. +//! +//! Two processes build this env: `buzz-dev-mcp`'s shim (for its own shell-tool +//! children) and the `buzz-acp` harness (for the agent-runtime child, so every +//! native shell of claude-code/codex/goose inherits it too). Both call into +//! this crate so the identity an agent commits under is byte-for-byte identical +//! regardless of which surface applied it. +//! +//! The pieces are deliberately separated: +//! - [`write_keyfile`] persists the nostr secret to a 0600 file and returns the +//! derived public identity. +//! - [`identity_signing_entries`] is the author/email + NIP-GS signing config. +//! - [`nostr_credential_entries`] is the relay credential helper (applied +//! globally only by the shim; the desktop scopes it per-URL instead). +//! - [`to_git_config_env`] flattens entries into `GIT_CONFIG_*` env pairs, +//! composing over any `GIT_CONFIG_COUNT` already in the environment. + +use nostr::ToBech32; +use std::path::Path; +use zeroize::Zeroize; + +pub mod git_wrapper; + +/// Public identity derived from a nostr secret key. +pub struct KeyIdentity { + /// Absolute path to the 0600 keyfile holding the secret. + pub keyfile_path: String, + /// Lowercase hex public key — the stable attribution signal. + pub pubkey_hex: String, + /// `npub1…` bech32 form, used as the author-name fallback. + pub npub: String, +} + +/// Env var carrying the agent's Buzz display name, used as the git author name. +/// Distinct from the per-session UI title: commits outlive sessions, so +/// attribution must not follow a mutable, channel-qualified title. +pub const DISPLAY_NAME_ENV_VAR: &str = "BUZZ_ACP_DISPLAY_NAME"; + +/// Operator-controlled selector for whose identity an agent's commits carry. +/// +/// Read **once by the harness/shim at spawn** — never by the wrapper +/// per-invocation, which would let any agent `export BUZZ_GIT_IDENTITY=user` +/// mid-session and hollow out enforcement. Sovereignty lever, not an agent +/// escape hatch (VISION_SOVEREIGN.md: the operator overrides platform policy +/// on their own machine). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum GitIdentityMode { + /// Default. Commits are authored + NIP-GS-signed as the agent identity; + /// the full enforcement wrapper is installed. + Agent, + /// The operator's own git identity authors commits. No wrapper, manifest, + /// or injected authorship/signing config — vanilla git resolves the + /// operator's repo/global config. Relay git-over-HTTP auth (the nostr + /// credential helper) is unaffected: auth ≠ attribution. + User, +} + +impl GitIdentityMode { + /// The operator-facing environment variable that selects the mode. + pub const ENV_VAR: &'static str = "BUZZ_GIT_IDENTITY"; + + /// Resolve from an explicit optional raw value: unset (`None`) → [`Agent`]; + /// exactly `agent` or `user` (surrounding whitespace tolerated) → the + /// matching mode; anything else → `Err`. + /// + /// An unrecognized value **fails loudly** rather than silently falling back + /// to either mode — silent fallback in an identity control is the failure + /// class this whole feature exists to close. + /// + /// [`Agent`]: GitIdentityMode::Agent + pub fn from_value(raw: Option<&std::ffi::OsStr>) -> Result { + let Some(os) = raw else { + return Ok(Self::Agent); + }; + let value = os + .to_str() + .ok_or_else(|| format!("{} must be valid UTF-8 (`agent` or `user`)", Self::ENV_VAR))?; + match value.trim() { + "agent" => Ok(Self::Agent), + "user" => Ok(Self::User), + other => Err(format!( + "{} must be `agent` or `user`, got {other:?}", + Self::ENV_VAR + )), + } + } + + /// Resolve from this process's environment. See [`from_value`]. + /// + /// [`from_value`]: GitIdentityMode::from_value + pub fn from_env() -> Result { + Self::from_value(std::env::var_os(Self::ENV_VAR).as_deref()) + } +} + +/// Max characters in a git author name. Nostr display names are unbounded. +const MAX_GIT_USER_NAME_CHARS: usize = 80; + +/// Write the nostr private key to an owner-only file inside `dir` (which the +/// caller must have created 0700), then derive the public identity. +/// +/// Returns `None` — and warns to stderr — when the key is empty, unparseable, +/// or the file cannot be written or named. Callers treat `None` as "no identity +/// env", which keeps a mis-provisioned session able to run (unattributed) +/// rather than aborting every commit. +pub fn write_keyfile(dir: &Path, raw: &str) -> Option { + if raw.is_empty() { + return None; + } + let keys = match nostr::Keys::parse(raw) { + Ok(k) => k, + Err(e) => { + eprintln!( + "buzz-git-identity: warning: nostr key is set but invalid ({e}); \ + git auth/signing will be disabled" + ); + return None; + } + }; + let pubkey_hex = keys.public_key().to_hex(); + let npub = keys + .public_key() + .to_bech32() + .unwrap_or_else(|_| pubkey_hex.clone()); + + let keyfile = dir.join(".nostr-key"); + if write_keyfile_atomic(&keyfile, raw.as_bytes()).is_err() { + eprintln!( + "buzz-git-identity: warning: failed to write nostr keyfile; git auth/signing disabled" + ); + return None; + } + let keyfile_path = match keyfile.to_str() { + Some(s) => s.to_owned(), + None => { + eprintln!( + "buzz-git-identity: warning: keyfile path is not valid UTF-8; \ + git auth/signing disabled" + ); + return None; + } + }; + + Some(KeyIdentity { + keyfile_path, + pubkey_hex, + npub, + }) +} + +/// Write `data` to `path` with 0600 permissions set at creation time via +/// `OpenOptions::mode()` (no window where the file is world-readable). +/// `create_new` refuses to follow a pre-existing file or symlink. +#[cfg(unix)] +fn write_keyfile_atomic(path: &Path, data: &[u8]) -> std::io::Result<()> { + use std::io::Write; + use std::os::unix::fs::OpenOptionsExt; + let mut f = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(path)?; + f.write_all(data) +} + +#[cfg(not(unix))] +fn write_keyfile_atomic(path: &Path, data: &[u8]) -> std::io::Result<()> { + std::fs::write(path, data) +} + +/// Derive a NIP-05-style email from the pubkey and relay URL. +/// Format: `@`. Falls back to `@buzz` when +/// no usable relay host is configured (unset, or a loopback host that carries +/// no attribution meaning). The pubkey — not the display name — is the stable +/// key NIP-98 auth, NIP-GS signing, and contributor matching all read. +pub fn derive_git_email(pubkey_hex: &str) -> String { + let host = std::env::var("BUZZ_RELAY_URL") + .ok() + .and_then(|url| host_from_relay_url(&url)) + .filter(|h| !h.is_empty() && !h.starts_with("localhost") && !h.starts_with("127.")) + .unwrap_or_else(|| "buzz".to_owned()); + format!("{pubkey_hex}@{host}") +} + +fn host_from_relay_url(url: &str) -> Option { + let stripped = url + .strip_prefix("https://") + .or_else(|| url.strip_prefix("http://")) + .or_else(|| url.strip_prefix("wss://")) + .or_else(|| url.strip_prefix("ws://")) + .unwrap_or(url); + let host_port = stripped.split('/').next()?; + Some(host_port.split(':').next().unwrap_or(host_port).to_owned()) +} + +/// Resolve the git author name: the sanitized [`DISPLAY_NAME_ENV_VAR`], or the +/// `npub` when that env var is unset or sanitizes away to nothing usable. +pub fn resolve_user_name(npub: &str) -> String { + std::env::var(DISPLAY_NAME_ENV_VAR) + .ok() + .as_deref() + .and_then(sanitize_git_user_name) + .unwrap_or_else(|| npub.to_owned()) +} + +/// The author identity git config (`user.name` + `user.email`). Always safe to +/// apply: it introduces no dependency on an external signing program. +pub fn authorship_entries(id: &KeyIdentity) -> Vec<(String, String)> { + let email = derive_git_email(&id.pubkey_hex); + let user_name = resolve_user_name(&id.npub); + vec![ + ("user.name".into(), user_name), + ("user.email".into(), email), + ] +} + +/// The NIP-GS signing git config. Applying this makes git invoke +/// `git-sign-nostr` for every commit/tag, so the caller MUST ensure that +/// program is reachable on the child's PATH — otherwise every commit fails. +pub fn signing_entries(id: &KeyIdentity) -> Vec<(String, String)> { + vec![ + ("gpg.format".into(), "x509".into()), + ("gpg.x509.program".into(), "git-sign-nostr".into()), + ("commit.gpgSign".into(), "true".into()), + ("tag.gpgSign".into(), "true".into()), + ("user.signingkey".into(), id.pubkey_hex.clone()), + keyfile_entry(id), + ] +} + +/// The `nostr.keyfile` git config pointing at the 0600 keyfile. Both +/// `git-sign-nostr` and `git-credential-nostr` fall back to it to load the +/// secret when `NOSTR_PRIVATE_KEY` is scrubbed from the child env. It is part +/// of [`signing_entries`], but the credential helper needs it even in +/// `user`-identity mode (where signing/authorship are off) — auth ≠ +/// attribution — so it is exposed separately. +pub fn keyfile_entry(id: &KeyIdentity) -> (String, String) { + ("nostr.keyfile".into(), id.keyfile_path.clone()) +} + +/// The identity + signing git config for an agent, as ordered `(key, value)` +/// pairs. Excludes the credential helper — that is applied separately (globally +/// by the shim, per-URL by the desktop) so this set can be lifted onto the +/// agent-runtime child without duplicating the desktop's scoped helper. +pub fn identity_signing_entries(id: &KeyIdentity) -> Vec<(String, String)> { + let mut entries = authorship_entries(id); + entries.extend(signing_entries(id)); + entries +} + +/// Filename of the harness-owned identity manifest, written 0600 beside the +/// keyfile in the same 0700 install dir. It is the wrapper's authoritative +/// source for the identity/signing config it re-applies and the expected author +/// email it verifies pushes against — never the caller-mutable `GIT_CONFIG_*` +/// environment the wrapper is meant to constrain. +pub const IDENTITY_MANIFEST_NAME: &str = ".git-identity"; + +/// Serialize identity/signing `(key, value)` entries into the manifest and +/// write it 0600 into `dir` (which the caller created 0700). One `key=value` +/// per line; keys are fixed git config names (no `=`) so a first-`=` split +/// round-trips values that themselves contain `=`. Values never contain a +/// newline — [`sanitize_git_user_name`] strips control characters and the +/// keyfile path/email cannot — so lines are unambiguous. +pub fn write_identity_manifest(dir: &Path, entries: &[(String, String)]) -> std::io::Result<()> { + let mut body = String::new(); + for (key, value) in entries { + body.push_str(key); + body.push('='); + body.push_str(value); + body.push('\n'); + } + write_keyfile_atomic(&dir.join(IDENTITY_MANIFEST_NAME), body.as_bytes()) +} + +/// Parse the identity manifest in `dir`, or `None` when it is absent/unreadable. +/// A present-but-empty or entry-less manifest yields `Some(vec![])`, which the +/// wrapper treats as an inconsistent authority and fails closed on. +pub fn read_identity_manifest(dir: &Path) -> Option> { + let body = std::fs::read_to_string(dir.join(IDENTITY_MANIFEST_NAME)).ok()?; + Some( + body.lines() + .filter_map(|line| { + line.split_once('=') + .map(|(k, v)| (k.to_owned(), v.to_owned())) + }) + .collect(), + ) +} + +/// The nostr credential helper git config (relay git-over-HTTP auth). Additive: +/// the helper silently declines non-Buzz remotes, so git falls through to +/// system helpers for GitHub/GitLab/etc. +pub fn nostr_credential_entries() -> Vec<(String, String)> { + vec![ + ("credential.helper".into(), "nostr".into()), + ("credential.useHttpPath".into(), "true".into()), + ] +} + +/// Flatten `(key, value)` git config entries into `GIT_CONFIG_COUNT`/`KEY_n`/ +/// `VALUE_n` env pairs, composing over any `GIT_CONFIG_COUNT` already present +/// so a caller's existing config (e.g. the desktop's per-URL credential helper) +/// is preserved rather than clobbered. +pub fn to_git_config_env(entries: &[(String, String)]) -> Vec<(String, String)> { + let base: usize = std::env::var("GIT_CONFIG_COUNT") + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(0); + + let mut env = Vec::with_capacity(entries.len() * 2 + 1); + env.push(( + "GIT_CONFIG_COUNT".into(), + (base + entries.len()).to_string(), + )); + for (i, (key, val)) in entries.iter().enumerate() { + let idx = base + i; + env.push((format!("GIT_CONFIG_KEY_{idx}"), key.clone())); + env.push((format!("GIT_CONFIG_VALUE_{idx}"), val.clone())); + } + env +} + +/// Convenience for the common case: read the secret from `NOSTR_PRIVATE_KEY`, +/// remove it from this process's env so children never inherit it, write the +/// keyfile into `dir`, and return the derived identity. Mirrors the shim's +/// contract. `None` when the var is unset/empty/invalid or the keyfile fails. +pub fn take_key_and_write(dir: &Path) -> Option { + let mut raw = std::env::var("NOSTR_PRIVATE_KEY").ok(); + std::env::remove_var("NOSTR_PRIVATE_KEY"); + let id = raw.as_deref().and_then(|k| write_keyfile(dir, k)); + if let Some(ref mut k) = raw { + k.zeroize(); + } + id +} + +/// Like [`take_key_and_write`] but **leaves** `NOSTR_PRIVATE_KEY` in the env. +/// +/// The harness uses this: it lifts identity onto the agent-runtime child, but +/// that child still needs `NOSTR_PRIVATE_KEY` to reach `buzz-dev-mcp`, whose +/// shim performs the remove-from-env dance for its own subtree. Removing it at +/// the harness layer would blind dev-mcp's credential helper and signer. +pub fn read_key_and_write(dir: &Path) -> Option { + let mut raw = std::env::var("NOSTR_PRIVATE_KEY").ok(); + let id = raw.as_deref().and_then(|k| write_keyfile(dir, k)); + if let Some(ref mut k) = raw { + k.zeroize(); + } + id +} + +/// Characters git's `ident.c` treats as "crud": stripped from both ends of a +/// name, and — when a name is *nothing but* these — rejected outright with +/// `fatal: name consists only of disallowed characters`. +/// +/// Verified empirically against git 2.54.0 by committing with each ASCII byte +/// 32..=126 as the entire `user.name`: exactly space, `"`, `'`, `,`, `:`, `;`, +/// `<`, `>`, and `\` abort. Control characters abort too (the predicate is +/// `c <= 32`). Note `.` is *not* crud in this version despite older lore. +fn is_git_crud(c: char) -> bool { + c <= ' ' || matches!(c, '"' | '\'' | ',' | ':' | ';' | '<' | '>' | '\\') +} + +/// Characters in Unicode general category `Cf` (format): zero-width space and +/// joiners, bidi embedding/override marks, invisible math operators, interlinear +/// annotations, and tag characters. +/// +/// `char::is_control` covers only `Cc`, so every one of these survives it — and +/// none is whitespace or [`is_git_crud`]. A display name of nothing but U+200B +/// ZERO WIDTH SPACE would therefore satisfy the "at least one non-crud +/// character" gate and hand git a visually blank author instead of falling back +/// to the npub. An embedded U+202E RIGHT-TO-LEFT OVERRIDE is worse: it makes a +/// commit's persisted author line render as something other than what it says, +/// the same confusion the angle-bracket filter exists to prevent. +/// +/// The whole category is rejected rather than the two known-bad marks, because +/// the boundary that matters is "invisible or reorders text", not "the codepoint +/// someone thought of". Ranges transcribed from the UCD's +/// `DerivedGeneralCategory.txt` (17.0.0) and independently cross-checked against +/// Python's `unicodedata` (16.0.0); both yield exactly these 21 ranges. Inlined +/// rather than taking a Unicode-tables dependency for one predicate. +fn is_unicode_format(c: char) -> bool { + matches!(c, + '\u{00AD}' + | '\u{0600}'..='\u{0605}' + | '\u{061C}' + | '\u{06DD}' + | '\u{070F}' + | '\u{0890}'..='\u{0891}' + | '\u{08E2}' + | '\u{180E}' + | '\u{200B}'..='\u{200F}' + | '\u{202A}'..='\u{202E}' + | '\u{2060}'..='\u{2064}' + | '\u{2066}'..='\u{206F}' + | '\u{FEFF}' + | '\u{FFF9}'..='\u{FFFB}' + | '\u{110BD}' + | '\u{110CD}' + | '\u{13430}'..='\u{1343F}' + | '\u{1BCA0}'..='\u{1BCA3}' + | '\u{1D173}'..='\u{1D17A}' + | '\u{E0001}' + | '\u{E0020}'..='\u{E007F}' + ) +} + +/// Normalize a Buzz display name into a git author name, or `None` to fall +/// back to the npub. +/// +/// Strips control and Unicode format characters plus angle brackets, collapses +/// whitespace runs, trims, and caps at [`MAX_GIT_USER_NAME_CHARS`] by `chars()` +/// so a multi-byte name cannot be split mid-UTF-8. Angle brackets go because git +/// silently drops them rather than erroring — `Duncan ` would +/// render as `Duncan evil@x.com `, which forges nothing but reads as +/// though it might. +/// +/// Returns `None` unless at least one non-crud character survives. A bare +/// emptiness check is not sufficient: git rejects a name built only of crud, +/// so a display name of `;;` or `""` would abort **every commit** the agent +/// makes. Falling back to the npub keeps the agent able to commit. +pub fn sanitize_git_user_name(raw: &str) -> Option { + let collapsed = raw + .split_whitespace() + .map(|word| { + word.chars() + .filter(|c| !c.is_control() && !is_unicode_format(*c) && *c != '<' && *c != '>') + .collect::() + }) + .filter(|word| !word.is_empty()) + .collect::>() + .join(" "); + let name: String = collapsed + .chars() + .take(MAX_GIT_USER_NAME_CHARS) + .collect::() + .trim_end() + .to_string(); + name.chars().any(|c| !is_git_crud(c)).then_some(name) +} + +#[cfg(test)] +mod tests { + use super::*; + use std::sync::Mutex; + + /// Env-var-touching tests must run serially — env vars are process-global. + static ENV_LOCK: Mutex<()> = Mutex::new(()); + + const PUBKEY_HEX: &str = "dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95"; + const NPUB: &str = "npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7"; + + fn identity() -> KeyIdentity { + KeyIdentity { + keyfile_path: "/tmp/.nostr-key".into(), + pubkey_hex: PUBKEY_HEX.into(), + npub: NPUB.into(), + } + } + + /// Read a git config value back out of flattened GIT_CONFIG_KEY_n/VALUE_n pairs. + fn git_config(env: &[(String, String)], key: &str) -> Option { + let idx = env + .iter() + .find(|(k, v)| k.starts_with("GIT_CONFIG_KEY_") && v == key)? + .0 + .strip_prefix("GIT_CONFIG_KEY_")? + .to_owned(); + env.iter() + .find(|(k, _)| *k == format!("GIT_CONFIG_VALUE_{idx}")) + .map(|(_, v)| v.clone()) + } + + // ── sanitize_git_user_name ──────────────────────────────────────────────── + + #[test] + fn ordinary_name_passes_through_unchanged() { + assert_eq!(sanitize_git_user_name("Duncan"), Some("Duncan".into())); + } + + #[test] + fn angle_brackets_are_stripped_so_no_second_email_is_rendered() { + assert_eq!( + sanitize_git_user_name("Duncan "), + Some("Duncan evil@x.com".into()) + ); + } + + #[test] + fn whitespace_control_characters_become_a_single_separator() { + assert_eq!( + sanitize_git_user_name("Dun\ncan\tThe\r\nIdaho"), + Some("Dun can The Idaho".into()) + ); + } + + #[test] + fn non_whitespace_control_characters_are_dropped_outright() { + // NUL is the important one: an interior NUL makes `Command::env` fail + // the entire spawn upstream, so it must never survive to git config. + let got = sanitize_git_user_name("Idaho\0Blade\u{7}").expect("non-empty"); + assert_eq!(got, "IdahoBlade"); + assert!(!got.chars().any(char::is_control)); + } + + #[test] + fn internal_whitespace_runs_collapse_to_one_space() { + assert_eq!( + sanitize_git_user_name(" Duncan Idaho "), + Some("Duncan Idaho".into()) + ); + } + + #[test] + fn whitespace_only_name_falls_back_to_npub() { + assert_eq!(sanitize_git_user_name(" \t\n "), None); + } + + #[test] + fn empty_name_falls_back_to_npub() { + assert_eq!(sanitize_git_user_name(""), None); + } + + #[test] + fn crud_only_name_falls_back_rather_than_aborting_every_commit() { + for raw in ["<>", ";;", "\"\"", "''", ",", ":", "\\", ",;:"] { + assert_eq!( + sanitize_git_user_name(raw), + None, + "crud-only name {raw:?} must fall back to the npub" + ); + } + } + + #[test] + fn crud_mixed_with_real_characters_is_kept() { + assert_eq!(sanitize_git_user_name("O'Brien"), Some("O'Brien".into())); + assert_eq!( + sanitize_git_user_name("Smith, Jr."), + Some("Smith, Jr.".into()) + ); + } + + #[test] + fn over_length_name_is_truncated_to_the_cap() { + let long = "a".repeat(200); + let got = sanitize_git_user_name(&long).expect("non-empty"); + assert_eq!(got.chars().count(), MAX_GIT_USER_NAME_CHARS); + } + + #[test] + fn truncation_never_splits_a_multibyte_character() { + let long = "🐝".repeat(200); + let got = sanitize_git_user_name(&long).expect("non-empty"); + assert_eq!(got.chars().count(), MAX_GIT_USER_NAME_CHARS); + assert!(got.chars().all(|c| c == '🐝'), "no replacement chars"); + } + + #[test] + fn truncation_does_not_leave_a_trailing_space() { + let raw = format!("{} tail", "a".repeat(MAX_GIT_USER_NAME_CHARS - 1)); + let got = sanitize_git_user_name(&raw).expect("non-empty"); + assert!(!got.ends_with(' '), "got {got:?}"); + } + + #[test] + fn non_ascii_names_survive() { + assert_eq!( + sanitize_git_user_name("Élodie 🐝"), + Some("Élodie 🐝".into()) + ); + } + + #[test] + fn format_only_name_falls_back_to_npub() { + assert_eq!(sanitize_git_user_name("\u{200B}\u{200B}"), None); + for raw in ["\u{200D}", "\u{2060}", "\u{FEFF}", "\u{202E}", "\u{00AD}"] { + assert_eq!( + sanitize_git_user_name(raw), + None, + "format-only name {raw:?} must fall back to the npub" + ); + } + } + + #[test] + fn bidi_override_is_stripped_and_the_name_is_kept() { + assert_eq!( + sanitize_git_user_name("Duncan\u{202E}"), + Some("Duncan".into()) + ); + assert_eq!( + sanitize_git_user_name("Dun\u{202E}can Idaho"), + Some("Duncan Idaho".into()) + ); + } + + #[test] + fn zero_width_space_inside_a_word_is_removed_without_splitting_it() { + assert_eq!( + sanitize_git_user_name("Dun\u{200B}can"), + Some("Duncan".into()) + ); + } + + #[test] + fn format_characters_do_not_consume_the_length_budget() { + let raw = format!("{}{}", "\u{200B}".repeat(200), "a".repeat(90)); + let got = sanitize_git_user_name(&raw).expect("non-empty"); + assert_eq!(got.chars().count(), MAX_GIT_USER_NAME_CHARS); + assert!(got.chars().all(|c| c == 'a'), "got {got:?}"); + } + + #[test] + fn unicode_format_covers_every_cf_range_and_nothing_adjacent() { + for c in [ + '\u{00AD}', + '\u{0600}', + '\u{0605}', + '\u{061C}', + '\u{06DD}', + '\u{070F}', + '\u{0890}', + '\u{0891}', + '\u{08E2}', + '\u{180E}', + '\u{200B}', + '\u{200F}', + '\u{202A}', + '\u{202E}', + '\u{2060}', + '\u{2064}', + '\u{2066}', + '\u{206F}', + '\u{FEFF}', + '\u{FFF9}', + '\u{FFFB}', + '\u{110BD}', + '\u{110CD}', + '\u{13430}', + '\u{1343F}', + '\u{1BCA0}', + '\u{1BCA3}', + '\u{1D173}', + '\u{1D17A}', + '\u{E0001}', + '\u{E0020}', + '\u{E007F}', + ] { + assert!(is_unicode_format(c), "U+{:04X} is Cf", c as u32); + } + for c in [ + '\u{00AC}', + '\u{00AE}', + '\u{05FF}', + '\u{0606}', + '\u{061B}', + '\u{061D}', + '\u{200A}', + '\u{2010}', + '\u{2029}', + '\u{202F}', + '\u{2065}', + '\u{205F}', + '\u{2070}', + '\u{FEFE}', + '\u{FFF8}', + '\u{FFFC}', + '\u{110BC}', + '\u{1342F}', + '\u{E0000}', + '\u{E0080}', + 'a', + ' ', + '🐝', + 'É', + ] { + assert!(!is_unicode_format(c), "U+{:04X} is not Cf", c as u32); + } + } + + #[test] + fn git_crud_set_matches_observed_git_behavior() { + for c in [' ', '"', '\'', ',', ':', ';', '<', '>', '\\', '\t', '\n'] { + assert!(is_git_crud(c), "{c:?} should be crud"); + } + for c in ['.', '-', '_', '@', '(', 'a', '🐝'] { + assert!(!is_git_crud(c), "{c:?} should not be crud"); + } + } + + // ── identity_signing_entries / resolve_user_name ────────────────────────── + + #[test] + fn identity_uses_display_name_and_leaves_email_on_the_pubkey() { + let _guard = ENV_LOCK.lock().unwrap(); + std::env::set_var(DISPLAY_NAME_ENV_VAR, "Duncan"); + std::env::remove_var("BUZZ_RELAY_URL"); + let entries = identity_signing_entries(&identity()); + std::env::remove_var(DISPLAY_NAME_ENV_VAR); + + assert_eq!(entry(&entries, "user.name").as_deref(), Some("Duncan")); + assert_eq!( + entry(&entries, "user.email").as_deref(), + Some(format!("{PUBKEY_HEX}@buzz").as_str()) + ); + assert_eq!( + entry(&entries, "user.signingkey").as_deref(), + Some(PUBKEY_HEX) + ); + assert_eq!(entry(&entries, "gpg.format").as_deref(), Some("x509")); + assert_eq!( + entry(&entries, "gpg.x509.program").as_deref(), + Some("git-sign-nostr") + ); + assert_eq!(entry(&entries, "commit.gpgSign").as_deref(), Some("true")); + assert_eq!( + entry(&entries, "nostr.keyfile").as_deref(), + Some("/tmp/.nostr-key") + ); + // The credential helper is deliberately NOT part of this set. + assert_eq!(entry(&entries, "credential.helper"), None); + } + + #[test] + fn identity_falls_back_to_npub_when_display_name_unset() { + let _guard = ENV_LOCK.lock().unwrap(); + std::env::remove_var(DISPLAY_NAME_ENV_VAR); + std::env::remove_var("BUZZ_RELAY_URL"); + let entries = identity_signing_entries(&identity()); + assert_eq!(entry(&entries, "user.name").as_deref(), Some(NPUB)); + } + + #[test] + fn identity_falls_back_to_npub_when_display_name_is_unusable() { + let _guard = ENV_LOCK.lock().unwrap(); + std::env::remove_var("BUZZ_RELAY_URL"); + for raw in ["<>", "\u{200B}"] { + std::env::set_var(DISPLAY_NAME_ENV_VAR, raw); + let entries = identity_signing_entries(&identity()); + assert_eq!( + entry(&entries, "user.name").as_deref(), + Some(NPUB), + "unusable display name {raw:?} must reach git as the npub" + ); + } + std::env::remove_var(DISPLAY_NAME_ENV_VAR); + } + + fn entry(entries: &[(String, String)], key: &str) -> Option { + entries + .iter() + .find(|(k, _)| k == key) + .map(|(_, v)| v.clone()) + } + + // ── GitIdentityMode ─────────────────────────────────────────────────────── + + #[test] + fn mode_unset_defaults_to_agent() { + assert_eq!( + GitIdentityMode::from_value(None).unwrap(), + GitIdentityMode::Agent + ); + } + + #[test] + fn mode_parses_agent_and_user_tolerating_whitespace() { + use std::ffi::OsStr; + for raw in ["agent", " agent ", "agent\n"] { + assert_eq!( + GitIdentityMode::from_value(Some(OsStr::new(raw))).unwrap(), + GitIdentityMode::Agent, + "{raw:?} must parse as Agent" + ); + } + for raw in ["user", " user ", "user\n"] { + assert_eq!( + GitIdentityMode::from_value(Some(OsStr::new(raw))).unwrap(), + GitIdentityMode::User, + "{raw:?} must parse as User" + ); + } + } + + #[test] + fn mode_rejects_unrecognized_value_naming_var_and_accepted_values() { + use std::ffi::OsStr; + // No silent fallback in an identity control: typos, empty, and + // case-variants all fail loudly (git config values are case-sensitive). + for raw in ["usr", "Agent", "USER", "true", "", "1"] { + let err = GitIdentityMode::from_value(Some(OsStr::new(raw))) + .expect_err(&format!("{raw:?} must be rejected")); + assert!( + err.contains("BUZZ_GIT_IDENTITY") && err.contains("agent") && err.contains("user"), + "error must name the var and both values; got {err:?}" + ); + } + } + + // ── derive_git_email ────────────────────────────────────────────────────── + + #[test] + fn email_uses_relay_host_stripping_scheme_port_and_path() { + let _guard = ENV_LOCK.lock().unwrap(); + std::env::set_var("BUZZ_RELAY_URL", "wss://relay.example.com:443/path"); + assert_eq!( + derive_git_email(PUBKEY_HEX), + format!("{PUBKEY_HEX}@relay.example.com") + ); + std::env::remove_var("BUZZ_RELAY_URL"); + } + + #[test] + fn email_falls_back_to_buzz_for_loopback_and_unset() { + let _guard = ENV_LOCK.lock().unwrap(); + for url in ["http://localhost:3000", "ws://127.0.0.1:8080"] { + std::env::set_var("BUZZ_RELAY_URL", url); + assert_eq!(derive_git_email(PUBKEY_HEX), format!("{PUBKEY_HEX}@buzz")); + } + std::env::remove_var("BUZZ_RELAY_URL"); + assert_eq!(derive_git_email(PUBKEY_HEX), format!("{PUBKEY_HEX}@buzz")); + } + + // ── to_git_config_env composition ───────────────────────────────────────── + + #[test] + fn to_git_config_env_composes_over_existing_count() { + let _guard = ENV_LOCK.lock().unwrap(); + std::env::set_var("GIT_CONFIG_COUNT", "2"); + let env = to_git_config_env(&[("user.name".into(), "Duncan".into())]); + std::env::remove_var("GIT_CONFIG_COUNT"); + + assert_eq!(git_config_count(&env), 3); + // The new entry lands at index 2 (0 and 1 belong to the caller). + assert_eq!(git_config(&env, "user.name").as_deref(), Some("Duncan")); + assert!(env.iter().any(|(k, _)| k == "GIT_CONFIG_KEY_2")); + } + + #[test] + fn to_git_config_env_base_zero_when_unset() { + let _guard = ENV_LOCK.lock().unwrap(); + std::env::remove_var("GIT_CONFIG_COUNT"); + let env = to_git_config_env(&[ + ("user.name".into(), "Duncan".into()), + ("user.email".into(), "x@y".into()), + ]); + assert_eq!(git_config_count(&env), 2); + assert!(env.iter().any(|(k, _)| k == "GIT_CONFIG_KEY_0")); + assert!(env.iter().any(|(k, _)| k == "GIT_CONFIG_KEY_1")); + } + + fn git_config_count(env: &[(String, String)]) -> usize { + env.iter() + .find(|(k, _)| k == "GIT_CONFIG_COUNT") + .and_then(|(_, v)| v.parse().ok()) + .unwrap() + } + + // ── write_keyfile / take_key_and_write ──────────────────────────────────── + + #[test] + fn write_keyfile_rejects_empty_and_invalid() { + let dir = tempfile::tempdir().unwrap(); + assert!(write_keyfile(dir.path(), "").is_none()); + assert!(write_keyfile(dir.path(), "not-a-key").is_none()); + } + + #[test] + fn write_keyfile_persists_0600_and_derives_pubkey() { + let dir = tempfile::tempdir().unwrap(); + let nsec = nostr::Keys::generate().secret_key().to_bech32().unwrap(); + let id = write_keyfile(dir.path(), &nsec).expect("valid key"); + assert_eq!(id.pubkey_hex.len(), 64); + assert!(id.npub.starts_with("npub1")); + assert!(std::path::Path::new(&id.keyfile_path).exists()); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(&id.keyfile_path) + .unwrap() + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o600); + } + } + + #[test] + fn take_key_and_write_removes_var_from_env() { + let _guard = ENV_LOCK.lock().unwrap(); + let dir = tempfile::tempdir().unwrap(); + let nsec = nostr::Keys::generate().secret_key().to_bech32().unwrap(); + std::env::set_var("NOSTR_PRIVATE_KEY", &nsec); + let id = take_key_and_write(dir.path()); + assert!(id.is_some()); + assert!( + std::env::var("NOSTR_PRIVATE_KEY").is_err(), + "secret must be removed from the process env" + ); + } +} diff --git a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs index f3de11ad242..552dcd09faa 100644 --- a/desktop/src-tauri/src/managed_agents/env_vars/tests.rs +++ b/desktop/src-tauri/src/managed_agents/env_vars/tests.rs @@ -198,6 +198,50 @@ fn reserved_keys_include_relay_url() { assert!(merged.is_empty()); } +#[test] +fn reserved_keys_include_git_config_family() { + // Buzz stages the relay credential helper and the agent identity/signing + // config into the child through the GIT_CONFIG_* indexed env family. A + // user override lands after the helper on the spawn command, so a single + // GIT_CONFIG_COUNT=0 (or any index collision) silently orphans it. The + // whole family — the bare name and every GIT_CONFIG_* var — must be + // stripped from persona/agent/global overrides. + for key in [ + "GIT_CONFIG", + "GIT_CONFIG_COUNT", + "GIT_CONFIG_KEY_0", + "GIT_CONFIG_VALUE_0", + "GIT_CONFIG_GLOBAL", + "GIT_CONFIG_SYSTEM", + "GIT_CONFIG_NOSYSTEM", + "GIT_CONFIG_PARAMETERS", + // Case-insensitive: the shape the child getenv resolves is uppercase, + // but the validator/filter must not be fooled by a lowercased key. + "git_config_count", + ] { + assert!(is_reserved_env_key(key), "{key} should be reserved"); + let agent = map(&[(key, "0")]); + assert!( + merged_user_env(&BTreeMap::new(), &agent).is_empty(), + "{key} should be stripped from overrides" + ); + assert!( + validate_user_env_keys(&map(&[(key, "0")])).is_err(), + "{key} should be rejected at save time" + ); + } +} + +#[test] +fn git_config_reservation_does_not_catch_unrelated_names() { + // The prefix rule matches `GIT_CONFIG` and `GIT_CONFIG_*` only — a name + // that merely starts with those letters but is a distinct identifier + // (no underscore boundary) stays user-overridable. + for key in ["GIT_CONFIGURATION", "GIT_CONFIGX", "MY_GIT_CONFIG"] { + assert!(!is_reserved_env_key(key), "{key} must not be reserved"); + } +} + // ── validate_user_env_keys ───────────────────────────────────────── #[test] diff --git a/desktop/src-tauri/src/managed_agents/global_config/tests.rs b/desktop/src-tauri/src/managed_agents/global_config/tests.rs index 65cde47f26b..22a32a6f646 100644 --- a/desktop/src-tauri/src/managed_agents/global_config/tests.rs +++ b/desktop/src-tauri/src/managed_agents/global_config/tests.rs @@ -532,6 +532,64 @@ fn resolve_global_fallback_when_no_persona_linked() { assert_eq!(provider.as_deref(), Some("global-provider")); } +/// P1 (Carl): a user-supplied `GIT_CONFIG_*` entry — at any layer — must never +/// reach `descriptor.env`, which `spawn_agent_child` writes onto the child +/// *after* the relay credential-helper `GIT_CONFIG_*`. A surviving +/// `GIT_CONFIG_COUNT=0` would orphan the helper (and in `user` mode no identity +/// install re-stages it), erasing relay git auth. This drives the real +/// six-layer resolver — the same path spawn and remote-deploy both consume — +/// not a fresh `Command`, so it witnesses the actual Desktop→harness layering. +#[test] +fn git_config_stripped_from_every_env_layer_before_descriptor() { + // Seed the attacker value at all three user-settable layers: global, + // live persona, and per-agent overrides. + let mut persona = persona("p", Some("model"), Some("anthropic")); + persona.env_vars = [ + ("GIT_CONFIG_COUNT".to_string(), "0".to_string()), + ( + "GIT_CONFIG_KEY_0".to_string(), + "credential.helper".to_string(), + ), + ("BENIGN".to_string(), "persona".to_string()), + ] + .into_iter() + .collect(); + let personas = vec![persona]; + + let mut record = bare_record(); + record.persona_id = Some("p".to_string()); + record.env_vars = [("GIT_CONFIG_GLOBAL".to_string(), "/dev/null".to_string())] + .into_iter() + .collect(); + + let global = GlobalAgentConfig { + env_vars: [("GIT_CONFIG_SYSTEM".to_string(), "/dev/null".to_string())] + .into_iter() + .collect(), + ..Default::default() + }; + let runtime = super::super::known_acp_runtime("buzz-agent").expect("buzz-agent runtime"); + + let effective = super::super::readiness::resolve_effective_agent_env( + &record, + &personas, + Some(runtime), + &global, + ); + + for key in effective.env.keys() { + assert!( + !key.to_ascii_uppercase().starts_with("GIT_CONFIG"), + "descriptor env must not carry a user GIT_CONFIG* key, found `{key}`" + ); + } + // Non-reserved overrides still flow through — the strip is surgical. + assert_eq!( + effective.env.get("BENIGN").map(String::as_str), + Some("persona") + ); +} + /// All-None: no source provides model/provider → both must be None. /// Guards against a resolver that synthesizes phantom defaults. #[test] diff --git a/desktop/src-tauri/src/managed_agents/nest_agents.md b/desktop/src-tauri/src/managed_agents/nest_agents.md index 7cb7489b852..1f94bfc542c 100644 --- a/desktop/src-tauri/src/managed_agents/nest_agents.md +++ b/desktop/src-tauri/src/managed_agents/nest_agents.md @@ -46,13 +46,15 @@ created: 2026-01-15 ## Git Commit Identity -The human operator signs off for accountability. +Your commit **author** identity is machine-managed. Every commit is automatically authored and cryptographically signed as your agent identity (`@`) — you do not, and cannot, set it. The managed `git` rejects `user.name`/`user.email` config, `-c user.*`, `--author`, and `--reset-author`. The human operator is credited in the commit message trailers, which the author identity does not replace. -- **Human sign-off (required):** every commit MUST include a `Signed-off-by` trailer for the human operator who is responsible for the agent's work. Add via `git commit --trailer "Signed-off-by: Human Name "`. One blank line must separate trailers from the commit body. +> The operator can turn this off with `BUZZ_GIT_IDENTITY=user` (default `agent`, settable per-agent). In `user` mode your commits carry the operator's own git identity and signing config — no managed `git`, no signing enforcement — and the trailers below are redundant since the commit already *is* the operator's identity. The rest of this section describes the default `agent` mode. + +- **Human sign-off (required):** every commit MUST include a `Signed-off-by` trailer for the human operator responsible for the agent's work. Add via `git commit --trailer "Signed-off-by: Human Name "`. One blank line must separate trailers from the commit body. - **Human credit (`Co-authored-by`):** every commit MUST also include a `Co-authored-by` trailer for the same human operator, with identical name and email to the `Signed-off-by` line. GitHub parses `Co-authored-by` for contribution-graph credit; `Signed-off-by` alone does not grant it. Add via `git commit --trailer "Co-authored-by: Human Name "`. Place `Co-authored-by` before `Signed-off-by` in the trailer block. -- **Discovering the human's identity:** read `git config user.name` and `git config user.email` from the working repository. These reflect the human operator's configured identity for that repo (which may differ from their global config). Use these exact values for both trailers. Do NOT hardcode, guess, or prompt for the email — the repo config is the source of truth. If `git config user.email` returns empty, STOP and ask the human operator for their name and email before committing. -- **Signing:** if the agent has a registered signing key, sign commits. If not, commits will land unverified — this is acceptable until agent SSH keys are provisioned. Do NOT use the human's signing key. -- **Verify before pushing:** `git log -1` should show the human's `Signed-off-by` trailer. +- **Discovering the human's identity:** `git config user.name`/`user.email` now resolve to your machine-managed *agent* identity, NOT the operator's — do not use them for the trailers. Take the operator's name and email from the repository's `AGENTS.md` / contribution docs or from an explicit instruction. Do NOT hardcode or guess. If you cannot determine the operator's email, STOP and ask before committing. +- **Signing:** commits are signed with your agent nostr key automatically (NIP-GS). Do not configure a separate signing key and do not use the human's signing key. +- **Verify before pushing:** `git log -1 --format='%B' | git interpret-trailers --parse` should show the human's `Co-authored-by` and `Signed-off-by` trailers as a contiguous block. ## Active Agents diff --git a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs index afaaa2b4eb3..6dbd04afc10 100644 --- a/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs +++ b/desktop/src-tauri/src/managed_agents/reserved_env_keys.rs @@ -76,7 +76,28 @@ pub(crate) const RESERVED_ENV_KEYS: &[&str] = &[ ]; pub(crate) fn is_reserved_env_key(key: &str) -> bool { - RESERVED_ENV_KEYS - .iter() - .any(|reserved| reserved.eq_ignore_ascii_case(key)) + is_reserved_git_config_key(key) + || RESERVED_ENV_KEYS + .iter() + .any(|reserved| reserved.eq_ignore_ascii_case(key)) +} + +/// The complete `GIT_CONFIG*` env family — `GIT_CONFIG` plus every +/// `GIT_CONFIG_*` var (`GIT_CONFIG_COUNT`, the indexed `GIT_CONFIG_KEY_` / +/// `GIT_CONFIG_VALUE_` pairs, `GIT_CONFIG_GLOBAL` / `_SYSTEM` / `_NOSYSTEM`, +/// `GIT_CONFIG_PARAMETERS`) — is reserved. +/// +/// Buzz stages the relay git credential helper *and* the agent identity/signing +/// config into the child through these indexed vars (see `runtime.rs` and +/// `install_git_identity`). A user override is layered onto the spawn command +/// *after* the credential helper, so a single `GIT_CONFIG_COUNT=0` (or any +/// index collision) silently orphans the helper — breaking relay git auth in +/// `user` mode, where no identity install runs to re-stage it — and other +/// family members can redirect git's config resolution entirely. It is a +/// prefix rule because the indexed keys are unbounded; matching the exact +/// `GIT_CONFIG` name and the `GIT_CONFIG_` prefix covers the whole family +/// without catching unrelated names like `GIT_CONFIGURATION`. +fn is_reserved_git_config_key(key: &str) -> bool { + let upper = key.to_ascii_uppercase(); + upper == "GIT_CONFIG" || upper.starts_with("GIT_CONFIG_") }