diff --git a/.github/workflows/sprig.yml b/.github/workflows/sprig.yml index 5e50808b3b..e381814157 100644 --- a/.github/workflows/sprig.yml +++ b/.github/workflows/sprig.yml @@ -1,10 +1,9 @@ name: Sprig # Builds and publishes Sprig — one deploy-anywhere Linux multicall binary for: -# buzz-acp ACP harness that bridges Buzz events to the LLM agent +# buzz-acp ACP harness (multicall Git helpers) that bridges Buzz events to the LLM agent # buzz-agent ACP-compliant agent (spawns MCP, calls LLMs) -# buzz-dev-mcp Developer MCP server (multicall: rg, tree, buzz, -# git-credential-nostr, git-sign-nostr) +# buzz-dev-mcp Developer MCP server (multicall: rg, tree, buzz) # # Targets: x86_64-unknown-linux-musl and aarch64-unknown-linux-musl (static # musl so the tarball runs on any modern Linux without libc surprises). diff --git a/Cargo.lock b/Cargo.lock index 421642e526..207fc06b72 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -896,6 +896,8 @@ dependencies = [ "clap", "evalexpr", "futures-util", + "git-credential-nostr", + "git-sign-nostr", "hex", "httparse", "nix 0.31.3", @@ -905,6 +907,7 @@ dependencies = [ "serde", "serde_json", "sha2 0.11.0", + "tempfile", "thiserror 2.0.18", "tokio", "tokio-tungstenite 0.29.0", @@ -914,6 +917,7 @@ dependencies = [ "tracing-subscriber", "url", "uuid", + "zeroize", ] [[package]] @@ -1167,8 +1171,6 @@ dependencies = [ "base64 0.22.1", "buzz-cli", "buzz-core", - "git-credential-nostr", - "git-sign-nostr", "ignore", "image", "nix 0.31.3", @@ -1186,7 +1188,6 @@ dependencies = [ "tracing", "tracing-subscriber", "windows-sys 0.61.2", - "zeroize", ] [[package]] diff --git a/Dockerfile.sprig b/Dockerfile.sprig index 160e0b5662..ee8ac1a64f 100644 --- a/Dockerfile.sprig +++ b/Dockerfile.sprig @@ -23,11 +23,7 @@ FROM alpine:3.22@sha256:14358309a308569c32bdc37e2e0e9694be33a9d99e68afb0f5ff33cc RUN apk add --no-cache bash ca-certificates curl git \ && adduser -D -h /home/agent agent \ - && install -d -o agent -g agent /workspace /home/agent \ - && git config --system gpg.format x509 \ - && git config --system gpg.x509.program /usr/local/bin/git-sign-nostr \ - && git config --system commit.gpgSign true \ - && git config --system tag.gpgSign true + && install -d -o agent -g agent /workspace /home/agent COPY --from=builder --chmod=0755 /build/target/sprig/sprig /usr/local/bin/sprig COPY --chmod=0755 scripts/sprig-entrypoint.sh /usr/local/bin/sprig-entrypoint diff --git a/crates/buzz-acp/Cargo.toml b/crates/buzz-acp/Cargo.toml index d047849806..07955ebbeb 100644 --- a/crates/buzz-acp/Cargo.toml +++ b/crates/buzz-acp/Cargo.toml @@ -21,6 +21,12 @@ buzz-core = { workspace = true } buzz-sdk = { workspace = true } buzz-persona = { path = "../buzz-persona" } +# Git multicall helpers and ephemeral key ownership +git-credential-nostr = { path = "../git-credential-nostr" } +git-sign-nostr = { path = "../git-sign-nostr" } +tempfile = "3" +zeroize = { workspace = true } + # Nostr nostr = { workspace = true } diff --git a/crates/buzz-acp/README.md b/crates/buzz-acp/README.md index 3d011eb8eb..76f73dbf4e 100644 --- a/crates/buzz-acp/README.md +++ b/crates/buzz-acp/README.md @@ -355,3 +355,25 @@ See the [root TESTING.md](../../TESTING.md) for the full integration testing gui ## License Apache-2.0 + +## Git in coding runtimes + +The harness configures agent authorship, Nostr commit/tag signing, and Git +credentials for native runtime shells and declared MCP servers. Author names +use `BUZZ_ACP_DISPLAY_NAME` (sanitized, with an npub fallback); email retains +the public key and relay host. Inherited author/committer name and email +overrides are cleared so native shells use the same agent attribution as MCP. +Credential helpers are scoped to the selected +relay's `/git` URLs. Existing `GIT_CONFIG_*` entries are preserved before the +harness's overrides, and the complete block is forwarded in `mcpServers[].env` +for agents that clear their MCP child environment. + +`buzz-acp` includes both Git helpers as multicall personalities, so standalone +and remote launches need no separate signer installation. The harness creates +private helper aliases and a 0600 keyfile, keeps them alive across adapter +respawns, and removes them when it exits normally or completes graceful +shutdown. As with other temporary files, SIGKILL or a machine crash cannot run +cleanup. `BUZZ_PRIVATE_KEY` remains available to the Buzz CLI; adapters do not +receive the redundant `NOSTR_PRIVATE_KEY` variable. No global Git config is +modified. Standalone `buzz-dev-mcp` supplies utility aliases only; a non-Buzz +ACP client must supply any desired Git environment itself. diff --git a/crates/buzz-acp/TESTING.md b/crates/buzz-acp/TESTING.md index 06fad05153..528bac2138 100644 --- a/crates/buzz-acp/TESTING.md +++ b/crates/buzz-acp/TESTING.md @@ -21,9 +21,41 @@ Make sure `pi` and `buzz-pi-acp` are on PATH, then restart Buzz. cargo test -p buzz-acp ``` +Managed agent sessions may already export harness options. Clear them when +running the package suite: three CLI parsing tests assert the unset defaults, +and inherited values would change the inputs those tests exercise. Running the +package serially also avoids scheduling flakes in existing short-deadline tests. + +```sh +env -u BUZZ_ACP_ALLOWED_RESPOND_TO \ + -u BUZZ_ACP_LAZY_POOL \ + -u BUZZ_ACP_IDLE_POOL_SLEEP \ + cargo test -p buzz-acp -- --test-threads=1 +``` + Run the ignored real-adapter test with a built fork checkout: ```sh BUZZ_TEST_PI_ACP=/absolute/buzz-pi-acp/dist/index.js \ cargo test -p buzz-acp real_pi_preserves -- --ignored ``` + +## Git bootstrap + +`cargo test -p buzz-acp --test git_bootstrap` starts the actual harness with a +probe adapter, runs real signed commits/tags and scoped credential resolution, +and verifies key cleanup on startup failure and SIGTERM. No relay is contacted. + +To exercise the real runtime boundaries on Unix: + +```sh +cargo build -p buzz-acp -p buzz-agent -p buzz-dev-mcp +BUZZ_TEST_BIN_DIR="$PWD/target/debug" cargo test -p buzz-acp git_runtime_tests -- --ignored --nocapture +``` + +The Buzz Agent test uses a deterministic local OpenAI-compatible response to +invoke the actual MCP shell. The Goose test requires an installed, configured +Goose and uses its provider to invoke the native developer shell. Both operate +only on temporary local repositories, verify commit/tag signatures and identity, +check unrelated-remote credential scoping, and assert keyfile removal. They do +not replace authenticated relay clone/push/readback testing. diff --git a/crates/buzz-acp/src/acp.rs b/crates/buzz-acp/src/acp.rs index cd04983068..82444152e6 100644 --- a/crates/buzz-acp/src/acp.rs +++ b/crates/buzz-acp/src/acp.rs @@ -528,6 +528,32 @@ impl AcpClient { cmd.env("CODEX_CONFIG", merged); } + // The harness composes the entire Git config block once. It must replace + // inherited counts/PATH rather than mixing two independently indexed blocks. + for (name, value) in extra_env + .iter() + .filter(|(name, _)| crate::git::is_managed_env(name)) + { + cmd.env(name, value); + } + // Git applies this older injection channel after GIT_CONFIG_COUNT. + // Keeping it would let an ambient user.name or signing setting win + // over the harness-owned agent identity in native shells. + cmd.env_remove("GIT_CONFIG_PARAMETERS"); + cmd.env_remove("NOSTR_PRIVATE_KEY"); + if extra_env.iter().any(|(name, _)| name == "GIT_CONFIG_COUNT") { + // Native shells inherit these overrides, while buzz-agent clears + // them for MCP. Let both paths use the harness's agent identity. + for name in [ + "GIT_AUTHOR_NAME", + "GIT_AUTHOR_EMAIL", + "GIT_COMMITTER_NAME", + "GIT_COMMITTER_EMAIL", + ] { + cmd.env_remove(name); + } + } + // 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). diff --git a/crates/buzz-acp/src/git.rs b/crates/buzz-acp/src/git.rs new file mode 100644 index 0000000000..081a18714b --- /dev/null +++ b/crates/buzz-acp/src/git.rs @@ -0,0 +1,610 @@ +//! Harness-owned, ephemeral Git identity, credentials and signing for every runtime. +use nostr::ToBech32; +use std::path::Path; +use tempfile::TempDir; +use zeroize::Zeroizing; + +/// Keep this guard alive until the adapter pool has shut down. +pub(crate) struct GitEnvironment { + _dir: TempDir, + pub(crate) env: Vec<(String, String)>, +} + +impl GitEnvironment { + pub(crate) fn install( + keys: &nostr::Keys, + relay_url: &str, + executable: &Path, + ) -> anyhow::Result { + let dir = tempfile::Builder::new().prefix("buzz-acp-git-").tempdir()?; + set_owner_only(dir.path())?; + for name in ["git-credential-nostr", "git-sign-nostr"] { + symlink(executable, &dir.path().join(name))?; + } + let keyfile = dir.path().join(".nostr-key"); + let secret = Zeroizing::new(keys.secret_key().to_secret_hex()); + write_keyfile_atomic(&keyfile, secret.as_bytes())?; + let info = KeyInfo { + keyfile_path: keyfile + .to_str() + .ok_or_else(|| anyhow::anyhow!("Git keyfile path is not UTF-8"))? + .to_owned(), + pubkey_hex: keys.public_key().to_hex(), + npub: keys.public_key().to_bech32()?, + }; + let mut relay = url::Url::parse(relay_url)?; + let scheme = match relay.scheme() { + "wss" | "https" => "https", + "ws" | "http" => "http", + _ => anyhow::bail!("unsupported Git relay URL scheme"), + }; + relay + .set_scheme(scheme) + .map_err(|_| anyhow::anyhow!("invalid Git relay URL"))?; + relay.set_query(None); + relay.set_fragment(None); + let inherited = inherited_config()?; + let display_name = std::env::var("BUZZ_ACP_DISPLAY_NAME").ok(); + let mut env = build_git_env( + &info, + relay.as_str().trim_end_matches('/'), + display_name.as_deref(), + inherited, + ); + let mut paths = vec![dir.path().to_path_buf()]; + paths.extend(std::env::split_paths( + &std::env::var_os("PATH").unwrap_or_default(), + )); + env.push(( + "PATH".into(), + std::env::join_paths(paths)? + .into_string() + .map_err(|_| anyhow::anyhow!("Git PATH is not UTF-8"))?, + )); + env.push(("GIT_TERMINAL_PROMPT".into(), "0".into())); + // CLI flags must work even when the caller did not export these variables. + env.push(("BUZZ_PRIVATE_KEY".into(), secret.to_string())); + env.push(("BUZZ_RELAY_URL".into(), relay_url.to_owned())); + Ok(Self { _dir: dir, env }) + } +} + +struct KeyInfo { + keyfile_path: String, + pubkey_hex: String, + npub: String, +} + +fn inherited_config() -> anyhow::Result> { + let count = match std::env::var("GIT_CONFIG_COUNT") { + Ok(value) => value.parse::()?, + Err(std::env::VarError::NotPresent) => 0, + Err(error) => return Err(error.into()), + }; + anyhow::ensure!(count <= 1024, "too many inherited Git config entries"); + (0..count) + .map(|i| { + Ok(( + std::env::var(format!("GIT_CONFIG_KEY_{i}"))?, + std::env::var(format!("GIT_CONFIG_VALUE_{i}"))?, + )) + }) + .collect() +} + +/// 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) +} + +/// 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. +/// +/// 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) +} + +/// Compose a complete config block, including the caller's entries, for env-cleared MCP children. +fn build_git_env( + info: &KeyInfo, + relay: &str, + display_name: Option<&str>, + mut entries: Vec<(String, String)>, +) -> Vec<(String, String)> { + let host = url::Url::parse(relay) + .ok() + .and_then(|url| url.host_str().map(str::to_owned)) + .filter(|host| !host.starts_with("localhost") && !host.starts_with("127.")) + .unwrap_or_else(|| "buzz".into()); + let scope = format!("credential.{relay}/git"); + entries.extend([ + ( + "user.name".into(), + display_name + .and_then(sanitize_git_user_name) + .unwrap_or_else(|| info.npub.clone()), + ), + ("user.email".into(), format!("{}@{host}", info.pubkey_hex)), + // Reset helpers only inside this relay's Git URL namespace. Unrelated + // remotes retain their own helper chain and never receive this key. + (format!("{scope}.helper"), String::new()), + (format!("{scope}.helper"), "nostr".into()), + (format!("{scope}.useHttpPath"), "true".into()), + ("nostr.keyfile".into(), info.keyfile_path.clone()), + ("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(), info.pubkey_hex.clone()), + ]); + let mut env = vec![("GIT_CONFIG_COUNT".into(), entries.len().to_string())]; + for (i, (key, value)) in entries.into_iter().enumerate() { + env.push((format!("GIT_CONFIG_KEY_{i}"), key)); + env.push((format!("GIT_CONFIG_VALUE_{i}"), value)); + } + env +} + +/// Variables owned by the harness and forwarded across the MCP env-clear boundary. +pub(crate) fn is_managed_env(name: &str) -> bool { + name.starts_with("GIT_CONFIG_") + || matches!( + name, + "PATH" | "GIT_TERMINAL_PROMPT" | "BUZZ_PRIVATE_KEY" | "BUZZ_RELAY_URL" + ) +} + +#[cfg(unix)] +fn set_owner_only(path: &Path) -> std::io::Result<()> { + use std::os::unix::fs::PermissionsExt; + let mut perms = std::fs::metadata(path)?.permissions(); + perms.set_mode(0o700); + std::fs::set_permissions(path, perms) +} + +#[cfg(not(unix))] +fn set_owner_only(_: &Path) -> std::io::Result<()> { + Ok(()) +} + +#[cfg(unix)] +fn symlink(src: &Path, dst: &Path) -> std::io::Result<()> { + std::os::unix::fs::symlink(src, dst) +} + +#[cfg(not(unix))] +fn symlink(src: &Path, dst: &Path) -> std::io::Result<()> { + // No symlinks without elevation on Windows; copy instead. The target needs + // a .exe extension or PATH lookup (via PATHEXT) won't treat it as runnable. + let dst = dst.with_extension("exe"); + std::fs::copy(src, dst).map(|_| ()) +} + +#[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, + }; + + 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(), + } + } + + /// 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()) + ); + } + + #[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" + ); + } + } + + #[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()) + ); + assert_eq!( + sanitize_git_user_name("Dun\u{202E}can Idaho"), + Some("Duncan Idaho".into()) + ); + } + + #[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()) + ); + } + + #[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 env = build_git_env( + &key_info(), + "https://localhost:3000", + Some("Duncan"), + vec![], + ); + + 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_eq!( + git_config(&env, "user.signingkey").as_deref(), + Some(PUBKEY_HEX) + ); + } + + #[test] + fn test_build_git_env_falls_back_to_npub_when_display_name_unset() { + let env = build_git_env(&key_info(), "https://localhost:3000", None, vec![]); + + // Without a display name, attribution falls back to the npub. + 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()) + ); + } + + #[test] + fn test_build_git_env_falls_back_to_npub_when_display_name_is_unusable() { + // 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}"] { + let env = build_git_env(&key_info(), "https://localhost:3000", Some(raw), vec![]); + assert_eq!( + git_config(&env, "user.name").as_deref(), + Some(NPUB), + "unusable display name {raw:?} must reach git as the npub" + ); + } + } + + #[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"); + } + } +} diff --git a/crates/buzz-acp/src/git_runtime_tests.rs b/crates/buzz-acp/src/git_runtime_tests.rs new file mode 100644 index 0000000000..4d5a89be36 --- /dev/null +++ b/crates/buzz-acp/src/git_runtime_tests.rs @@ -0,0 +1,182 @@ +//! Opt-in real runtime checks. Uses real ACP, MCP and Git processes, with a +//! deterministic local model for buzz-agent and the installed Goose provider. +use super::*; +use std::path::PathBuf; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; + +async fn scripted_model(command: String) -> (String, tokio::task::JoinHandle<()>) { + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let url = format!("http://{}", listener.local_addr().unwrap()); + let task = tokio::spawn(async move { + for round in 0..4 { + let (mut socket, _) = listener.accept().await.unwrap(); + let mut request = Vec::new(); + let mut buf = [0; 8192]; + let headers_end = loop { + let n = socket.read(&mut buf).await.unwrap(); + assert_ne!(n, 0); + request.extend_from_slice(&buf[..n]); + assert!(request.len() < 2_000_000); + if let Some(end) = request.windows(4).position(|w| w == b"\r\n\r\n") { + break end + 4; + } + }; + let headers = String::from_utf8_lossy(&request[..headers_end]); + let length: usize = headers + .lines() + .find_map(|line| { + line.to_lowercase() + .strip_prefix("content-length:") + .map(|s| s.trim().parse().unwrap()) + }) + .unwrap(); + assert!(length < 2_000_000); + while request.len() < headers_end + length { + let n = socket.read(&mut buf).await.unwrap(); + assert_ne!(n, 0); + request.extend_from_slice(&buf[..n]); + } + let request: serde_json::Value = + serde_json::from_slice(&request[headers_end..]).unwrap(); + let (message, reason) = if round == 0 { + let name = request["tools"] + .as_array() + .unwrap() + .iter() + .find_map(|tool| { + tool["function"]["name"] + .as_str() + .filter(|name| name.ends_with("__shell")) + }) + .unwrap(); + ( + serde_json::json!({"role":"assistant", "content":null,"tool_calls":[{"id":"git-probe","type":"function","function":{"name":name,"arguments":serde_json::json!({"command":command}).to_string()}}]}), + "tool_calls", + ) + } else { + ( + serde_json::json!({"role":"assistant","content":"done"}), + "stop", + ) + }; + let body = serde_json::json!({"id":"probe","object":"chat.completion","model":"probe","choices":[{"index":0,"message":message,"finish_reason":reason}]}).to_string(); + let response = format!("HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\nConnection: close\r\n\r\n{body}",body.len()); + socket.write_all(response.as_bytes()).await.unwrap(); + } + }); + (url, task) +} + +async fn check_runtime(goose: bool) { + let binaries = PathBuf::from( + std::env::var("BUZZ_TEST_BIN_DIR").expect("set BUZZ_TEST_BIN_DIR to built binaries"), + ); + let temp = tempfile::tempdir().unwrap(); + let mut config = build_mcp_servers_tests::test_config(); + config.relay_url = "https://relay.invalid".into(); + config.mcp_command = if goose { + String::new() + } else { + binaries.join("buzz-dev-mcp").to_str().unwrap().into() + }; + let git = + git::GitEnvironment::install(&config.keys, &config.relay_url, &binaries.join("buzz-acp")) + .unwrap(); + config.persona_env_vars.extend(git.env.iter().cloned()); + let keyfile = git + .env + .windows(2) + .find(|pair| pair[0].1 == "nostr.keyfile") + .unwrap()[1] + .1 + .clone(); + let script = temp.path().join("probe.sh"); + std::fs::write( + &script, + r#"set -eu +cd "$(dirname "$0")" +test -z "${NOSTR_PRIVATE_KEY:-}" +git config user.name > identity +git config user.email >> identity +test "$(git config --get-urlmatch credential.helper https://relay.invalid/git/test/repo)" = nostr +! git config --get-urlmatch credential.helper https://unrelated.invalid/git/test/repo +git init -q repo +cd repo +git -c core.hooksPath=/dev/null commit -s --allow-empty -qm 'runtime Git probe' +git tag -m 'runtime tag probe' probe +git verify-commit HEAD +git verify-tag probe +git config nostr.keyfile > ../keyfile +printf 'passed' > ../result +"#, + ) + .unwrap(); + let command = format!("sh '{}'", script.display()); + let (url, model) = scripted_model(command.clone()).await; + let (runtime, args) = if goose { + config + .persona_env_vars + .push(("GOOSE_MODE".into(), "auto".into())); + ( + "goose".into(), + vec!["acp".into(), "--with-builtin".into(), "developer".into()], + ) + } else { + config.persona_env_vars.extend([ + ("BUZZ_AGENT_PROVIDER".into(), "openai".into()), + ("OPENAI_COMPAT_BASE_URL".into(), url), + ("OPENAI_COMPAT_API_KEY".into(), "test".into()), + ("OPENAI_COMPAT_MODEL".into(), "probe".into()), + ]); + ( + binaries.join("buzz-agent").to_string_lossy().into_owned(), + vec![], + ) + }; + // Isolate the probes from operator/repository Git configuration. + config.persona_env_vars.extend([ + ("GIT_CONFIG_GLOBAL".into(), "/dev/null".into()), + ("GIT_CONFIG_NOSYSTEM".into(), "1".into()), + ]); + let mut client = AcpClient::spawn(&runtime, &args, &config.persona_env_vars, false) + .await + .unwrap(); + let result = tokio::time::timeout(Duration::from_secs(120), async { + client.initialize().await.unwrap(); + let session = client.session_new_full(temp.path().to_str().unwrap(), build_mcp_servers(&config), None, None).await.unwrap(); + client.session_prompt_with_idle_timeout(&session.session_id, &format!("Run exactly this command using your shell tool: {command}. It is a local disposable Git test. Do not edit it, read credentials, or contact any remote. Report the exit code."), Duration::from_secs(60), Duration::from_secs(100)).await.unwrap(); + }).await; + client.shutdown().await; + model.abort(); + result.unwrap(); + assert_eq!( + std::fs::read_to_string(temp.path().join("result")).unwrap(), + "passed" + ); + let identity = std::fs::read_to_string(temp.path().join("identity")).unwrap(); + assert!(identity.contains(&format!( + "{}@relay.invalid", + config.keys.public_key().to_hex() + ))); + assert_eq!( + std::fs::read_to_string(temp.path().join("keyfile")) + .unwrap() + .trim(), + keyfile + ); + assert!(std::path::Path::new(&keyfile).exists()); + drop(git); + assert!(!std::path::Path::new(&keyfile).exists()); +} + +#[tokio::test] +#[ignore = "requires built buzz-acp, buzz-agent and buzz-dev-mcp; BUZZ_TEST_BIN_DIR"] +async fn real_buzz_agent_git_shell() { + check_runtime(false).await; +} + +#[tokio::test] +#[ignore = "requires installed/configured Goose and built buzz-acp; BUZZ_TEST_BIN_DIR"] +async fn real_goose_native_git_shell() { + check_runtime(true).await; +} diff --git a/crates/buzz-acp/src/lib.rs b/crates/buzz-acp/src/lib.rs index 3ff1dc3989..cf723bcfd5 100644 --- a/crates/buzz-acp/src/lib.rs +++ b/crates/buzz-acp/src/lib.rs @@ -1,5 +1,9 @@ #![deny(unsafe_code)] +mod git; +#[cfg(all(test, unix))] +mod git_runtime_tests; + mod acp; mod config; mod engram_fetch; @@ -2445,6 +2449,15 @@ mod replay_floor_tests { } pub fn run() -> Result<()> { + let argv0 = std::env::args().next().unwrap_or_default(); + match std::path::Path::new(&argv0) + .file_stem() + .and_then(|name| name.to_str()) + { + Some("git-credential-nostr") => std::process::exit(git_credential_nostr::run()), + Some("git-sign-nostr") => std::process::exit(git_sign_nostr::run()), + _ => {} + } config::propagate_legacy_env_vars(); tokio_main() } @@ -2494,7 +2507,7 @@ async fn tokio_main() -> Result<()> { .compact() .init(); - let mut config = Config::from_cli().map_err(|e| anyhow::anyhow!("configuration error: {e}"))?; + let config = Config::from_cli().map_err(|e| anyhow::anyhow!("configuration error: {e}"))?; // ── Setup-mode early branch ─────────────────────────────────────────────── // @@ -2508,6 +2521,57 @@ async fn tokio_main() -> Result<()> { return setup_mode::run_setup_listener(config, payload).await; } + // Register termination before creating temporary key material or spawning + // adapters. During startup cancellation drops the pool and key guard; once + // ready, the existing main-loop shutdown drains active work first. + let (shutdown_tx, mut shutdown_rx) = watch::channel(()); + #[cfg(unix)] + let signals = { + use tokio::signal::unix::{signal, SignalKind}; + ( + signal(SignalKind::interrupt())?, + signal(SignalKind::terminate())?, + ) + }; + let tx = shutdown_tx.clone(); + let signal_task = tokio::spawn(async move { + #[cfg(unix)] + { + let (mut interrupt, mut terminate) = signals; + tokio::select! { _ = interrupt.recv() => {}, _ = terminate.recv() => {} } + } + #[cfg(not(unix))] + let _ = tokio::signal::ctrl_c().await; + let _ = tx.send(()); + }); + let (ready_tx, mut ready_rx) = tokio::sync::oneshot::channel(); + let harness = run_harness(config, shutdown_tx, shutdown_rx.clone(), ready_tx); + tokio::pin!(harness); + let result = tokio::select! { + biased; + _ = shutdown_rx.changed() => Ok(()), + result = &mut harness => result, + _ = &mut ready_rx => harness.await, + }; + signal_task.abort(); + result +} + +async fn run_harness( + mut config: Config, + shutdown_tx: watch::Sender<()>, + mut shutdown_rx: watch::Receiver<()>, + startup_ready: tokio::sync::oneshot::Sender<()>, +) -> Result<()> { + let git_environment = + git::GitEnvironment::install(&config.keys, &config.relay_url, &std::env::current_exe()?)?; + config + .persona_env_vars + .retain(|(name, _)| !git::is_managed_env(name)); + config + .persona_env_vars + .extend(git_environment.env.iter().cloned()); + tracing::info!("buzz-acp starting: {}", config.summary()); let observer = config @@ -2902,25 +2966,8 @@ async fn tokio_main() -> Result<()> { // `IN_FLIGHT_DEADLINE_SECS` expires. let (steer_ack_tx, mut steer_ack_rx) = mpsc::unbounded_channel::(); - // ── Step 7: Shutdown signal ─────────────────────────────────────────────── - let (shutdown_tx, mut shutdown_rx) = watch::channel(()); - - let tx = shutdown_tx.clone(); - tokio::spawn(async move { - tokio::signal::ctrl_c().await.ok(); - let _ = tx.send(()); - }); - - #[cfg(unix)] - { - let tx = shutdown_tx.clone(); - tokio::spawn(async move { - use tokio::signal::unix::{signal, SignalKind}; - let mut sigterm = signal(SignalKind::terminate()).expect("SIGTERM handler"); - sigterm.recv().await; - let _ = tx.send(()); - }); - } + // Startup is complete; the main loop now owns graceful shutdown. + let _ = startup_ready.send(()); // Track the newest membership notification timestamp per channel. // On reconnect the relay replays events newest-first, so the first event @@ -5908,10 +5955,8 @@ fn build_mcp_servers(config: &Config) -> Vec { }); } } - // Forward the agent's display name so dev-mcp can use it as the git - // author name instead of the raw npub. Read from the process env - // rather than Config: this is a pass-through of a contract owned - // upstream, and absent simply means dev-mcp falls back to the npub. + // Preserve the display-name contract for tools. Git authorship is + // already normalized by the harness bootstrap. if let Ok(display_name) = std::env::var("BUZZ_ACP_DISPLAY_NAME") { if !display_name.is_empty() { env.push(EnvVar { @@ -5920,6 +5965,15 @@ fn build_mcp_servers(config: &Config) -> Vec { }); } } + for (name, value) in &config.persona_env_vars { + if git::is_managed_env(name) { + env.retain(|entry| entry.name != *name); + env.push(EnvVar { + name: name.clone(), + value: value.clone(), + }); + } + } env }, }] @@ -9129,7 +9183,7 @@ mod build_mcp_servers_tests { /// Env-var-touching tests must run serially — env vars are process-global. static ENV_LOCK: Mutex<()> = Mutex::new(()); - fn test_config() -> Config { + pub(super) fn test_config() -> Config { Config { keys: nostr::Keys::generate(), relay_url: "ws://localhost:3000".into(), @@ -9179,6 +9233,44 @@ mod build_mcp_servers_tests { } } + #[test] + fn session_new_forwards_complete_git_block_without_duplicate_names() { + let mut config = test_config(); + let git = git::GitEnvironment::install( + &config.keys, + &config.relay_url, + &std::env::current_exe().unwrap(), + ) + .unwrap(); + config.persona_env_vars.extend(git.env.iter().cloned()); + let servers = build_mcp_servers(&config); + let env = &servers[0].env; + for (name, value) in &git.env { + let entries: Vec<_> = env.iter().filter(|entry| entry.name == *name).collect(); + assert_eq!(entries.len(), 1, "{name} must appear exactly once"); + assert_eq!(entries[0].value, *value); + } + assert!(!env.iter().any(|entry| entry.name == "NOSTR_PRIVATE_KEY")); + let keyfile = git + .env + .windows(2) + .find(|pair| pair[0].1 == "nostr.keyfile") + .unwrap()[1] + .1 + .clone(); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + assert_eq!( + std::fs::metadata(&keyfile).unwrap().permissions().mode() & 0o777, + 0o600 + ); + } + assert!(std::path::Path::new(&keyfile).exists()); + drop(git); + assert!(!std::path::Path::new(&keyfile).exists()); + } + #[test] fn session_new_mcp_server_has_required_fields() { let config = test_config(); diff --git a/crates/buzz-acp/tests/git_bootstrap.rs b/crates/buzz-acp/tests/git_bootstrap.rs new file mode 100644 index 0000000000..dc6d25d04c --- /dev/null +++ b/crates/buzz-acp/tests/git_bootstrap.rs @@ -0,0 +1,168 @@ +//! Run the installed harness/helper personalities across a real process boundary. +#![cfg(unix)] + +use base64::Engine; +use std::os::unix::fs::PermissionsExt; +use std::process::{Command, Stdio}; +use std::time::{Duration, Instant}; + +#[test] +fn harness_native_git_and_startup_shutdown_cleanup() { + let temp = tempfile::tempdir().unwrap(); + let workspace = temp.path(); + let adapter = workspace.join("adapter"); + // Nothing in this script configures Git: all settings/helpers must come + // from the production harness spawn, including a CLI-only identity. + std::fs::write(&adapter, r#"#!/bin/sh +set -eu +cd "$PROBE_DIR" +test -z "${NOSTR_PRIVATE_KEY:-}" +test "$(git config test.inherited)" = preserved +test "$(git config user.name)" = 'Git Probe' +test "$(git config --get-urlmatch credential.helper https://relay.invalid/git/test/repo)" = nostr +! git config --get-urlmatch credential.helper https://unrelated.invalid/git/test/repo +! git config --get-urlmatch credential.helper https://relay.invalid/other/repo +! git config --get-urlmatch credential.helper https://relay.invalid/git-other/repo +git config nostr.keyfile > keyfile +command -v git-sign-nostr > signer +command -v git-credential-nostr > credential +git init -q repo +cd repo +git -c core.hooksPath=/dev/null commit -s --allow-empty -qm 'bootstrap probe' +git tag -m 'signed tag probe' probe +git verify-commit HEAD +git verify-tag probe +git show -s --format='%an%n%ae%n%cn%n%ce' HEAD > ../identity +printf 'capability[]=authtype\nprotocol=https\nhost=relay.invalid\npath=git/test/repo\nwwwauth[]=Nostr method="GET"\n\n' | git credential fill > ../credential-result +printf 'done' > ../done +# Stay alive until the harness is terminated during ACP initialization. +exec sleep 60 +"#).unwrap(); + std::fs::set_permissions(&adapter, std::fs::Permissions::from_mode(0o700)).unwrap(); + let keys = nostr::Keys::generate(); + let log = std::fs::File::create(workspace.join("harness.log")).unwrap(); + let mut child = Command::new(env!("CARGO_BIN_EXE_buzz-acp")) + .env_clear() + .env("PATH", std::env::var_os("PATH").unwrap_or_default()) + .args([ + "--private-key", + &keys.secret_key().to_secret_hex(), + "--relay-url", + "wss://relay.invalid", + "--agent-command", + adapter.to_str().unwrap(), + "--agent-args", + "", + ]) + .env("PROBE_DIR", workspace) + .env("BUZZ_ACP_DISPLAY_NAME", "Git Probe") + .env("GIT_AUTHOR_NAME", "Inherited Human") + .env("GIT_AUTHOR_EMAIL", "human@example.invalid") + .env("GIT_COMMITTER_NAME", "Inherited Human") + .env("GIT_COMMITTER_EMAIL", "human@example.invalid") + .env("NOSTR_PRIVATE_KEY", "must-not-reach-adapter") + .env("GIT_CONFIG_COUNT", "3") + .env("GIT_CONFIG_KEY_0", "test.inherited") + .env("GIT_CONFIG_VALUE_0", "preserved") + // Emulate an old Desktop launcher's scoped entries. These must be + // carried through and overridden without duplicating config counts. + .env( + "GIT_CONFIG_KEY_1", + "credential.https://relay.invalid/git.helper", + ) + .env("GIT_CONFIG_VALUE_1", "/nonexistent/old-helper") + .env( + "GIT_CONFIG_KEY_2", + "credential.https://relay.invalid/git.useHttpPath", + ) + .env("GIT_CONFIG_VALUE_2", "true") + .env("GIT_CONFIG_NOSYSTEM", "1") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_PARAMETERS", "'user.name=Inherited Human'") + .env_remove("BUZZ_ACP_SETUP_PAYLOAD") + .env_remove("BUZZ_AUTH_TAG") + .stdin(Stdio::null()) + .stdout(log.try_clone().unwrap()) + .stderr(log) + .spawn() + .unwrap(); + let deadline = Instant::now() + Duration::from_secs(20); + while !workspace.join("done").exists() && Instant::now() < deadline { + if child.try_wait().unwrap().is_some() { + break; + } + std::thread::sleep(Duration::from_millis(50)); + } + // Always terminate, even on assertion failure, so regressions don't leave + // the subprocess or its ephemeral signing key behind. + nix::sys::signal::kill( + nix::unistd::Pid::from_raw(child.id() as i32), + nix::sys::signal::Signal::SIGTERM, + ) + .ok(); + let exit_deadline = Instant::now() + Duration::from_secs(5); + while child.try_wait().unwrap().is_none() && Instant::now() < exit_deadline { + std::thread::sleep(Duration::from_millis(20)); + } + if child.try_wait().unwrap().is_none() { + child.kill().unwrap(); + } + let status = child.wait().unwrap(); + let logs = std::fs::read_to_string(workspace.join("harness.log")).unwrap(); + assert!(workspace.join("done").exists(), "probe failed: {logs}"); + assert!(status.success(), "startup shutdown failed: {logs}"); + let identity = std::fs::read_to_string(workspace.join("identity")).unwrap(); + assert_eq!( + identity, + format!( + "Git Probe\n{0}@relay.invalid\nGit Probe\n{0}@relay.invalid\n", + keys.public_key().to_hex() + ) + ); + let credential = std::fs::read_to_string(workspace.join("credential-result")).unwrap(); + assert!(credential.contains("authtype=Nostr")); + let encoded = credential + .lines() + .find_map(|line| line.strip_prefix("credential=")) + .unwrap(); + let event: nostr::Event = serde_json::from_slice( + &base64::engine::general_purpose::STANDARD + .decode(encoded) + .unwrap(), + ) + .unwrap(); + event.verify().unwrap(); + assert_eq!(event.pubkey, keys.public_key()); + assert!(event + .tags + .iter() + .any(|tag| tag.as_slice() == ["u", "https://relay.invalid/git/test/repo"])); + for file in ["keyfile", "signer", "credential"] { + let path = std::fs::read_to_string(workspace.join(file)).unwrap(); + assert!( + !std::path::Path::new(path.trim()).exists(), + "{file} survived harness shutdown" + ); + } +} + +#[test] +fn keyfile_removed_after_adapter_failure() { + let temp = tempfile::tempdir().unwrap(); + let output = Command::new(env!("CARGO_BIN_EXE_buzz-acp")) + .args([ + "--private-key", + &nostr::Keys::generate().secret_key().to_secret_hex(), + "--relay-url", + "ws://localhost:1", + "--agent-command", + "/missing-buzz-adapter", + ]) + .env("TMPDIR", temp.path()) + .env_remove("BUZZ_ACP_SETUP_PAYLOAD") + .env_remove("BUZZ_AUTH_TAG") + .output() + .unwrap(); + assert!(!output.status.success()); + assert_eq!(std::fs::read_dir(temp.path()).unwrap().count(), 0); +} diff --git a/crates/buzz-agent/src/mcp.rs b/crates/buzz-agent/src/mcp.rs index a848557ae2..c9f1c92488 100644 --- a/crates/buzz-agent/src/mcp.rs +++ b/crates/buzz-agent/src/mcp.rs @@ -80,16 +80,11 @@ const PASSTHROUGH_ENV: &[&str] = &[ // was configured correctly and the child could not see it. "SSL_CERT_FILE", "SSL_CERT_DIR", - // Buzz identity — dev-mcp writes NOSTR_PRIVATE_KEY to a keyfile then - // removes it from its own env (children never see it). BUZZ_PRIVATE_KEY - // and BUZZ_RELAY_URL are kept for the buzz CLI. BUZZ_AUTH_TAG is a - // non-secret signed ownership attestation needed by portable owner-scoped - // CLI operations; MCP subprocesses are trusted like the agent runtime. - "NOSTR_PRIVATE_KEY", + // Git config/keyfile paths arrive explicitly in session/new mcpServers[].env. "BUZZ_PRIVATE_KEY", "BUZZ_RELAY_URL", "BUZZ_AUTH_TAG", - // Agent display name — dev-mcp uses it as the git author name. On the + // Agent display name for tools. On the // Desktop path this arrives via the wire `mcpServers[].env` declaration // (which wins here anyway); the allowlist entry covers ACP clients that // spawn buzz-agent without declaring it. diff --git a/crates/buzz-dev-mcp/Cargo.toml b/crates/buzz-dev-mcp/Cargo.toml index 8b711b8063..08d1ffabe3 100644 --- a/crates/buzz-dev-mcp/Cargo.toml +++ b/crates/buzz-dev-mcp/Cargo.toml @@ -15,10 +15,7 @@ path = "src/main.rs" [dependencies] buzz-cli = { path = "../buzz-cli" } -git-credential-nostr = { path = "../git-credential-nostr" } -git-sign-nostr = { path = "../git-sign-nostr" } nostr = { workspace = true } -zeroize = { workspace = true } tokio = { workspace = true } tokio-util = { workspace = true } serde = { workspace = true } diff --git a/crates/buzz-dev-mcp/src/lib.rs b/crates/buzz-dev-mcp/src/lib.rs index d555b6ea54..837ec292ab 100644 --- a/crates/buzz-dev-mcp/src/lib.rs +++ b/crates/buzz-dev-mcp/src/lib.rs @@ -148,8 +148,6 @@ pub fn run() -> Result<(), Box> { match cmd.as_str() { "rg" => std::process::exit(rg::run(std::env::args().skip(1).collect())), "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()), _ => {} } diff --git a/crates/buzz-dev-mcp/src/shell.rs b/crates/buzz-dev-mcp/src/shell.rs index 140d3c44cc..13e350689d 100644 --- a/crates/buzz-dev-mcp/src/shell.rs +++ b/crates/buzz-dev-mcp/src/shell.rs @@ -168,11 +168,6 @@ pub async fn run( cmd.arg(shell_arg).arg(&p.command); cmd.current_dir(&workdir); cmd.env("PATH", &state.shim.path_env); - // NOSTR_PRIVATE_KEY is already removed from this process's env (shim.rs). - // BUZZ_PRIVATE_KEY is intentionally inherited — the buzz CLI needs it. - for (k, v) in &state.shim.git_env { - cmd.env(k, v); - } cmd.stdin(Stdio::null()); cmd.stdout(Stdio::piped()); cmd.stderr(Stdio::piped()); diff --git a/crates/buzz-dev-mcp/src/shim.rs b/crates/buzz-dev-mcp/src/shim.rs index cccf0e6eca..0d985831fe 100644 --- a/crates/buzz-dev-mcp/src/shim.rs +++ b/crates/buzz-dev-mcp/src/shim.rs @@ -1,331 +1,34 @@ -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. -/// -/// On install: -/// 1. Creates a 0700 tempdir with symlinks back to our binary (multicall) -/// 2. If `NOSTR_PRIVATE_KEY` is set: writes a 0600 keyfile, derives the pubkey, -/// builds ephemeral `GIT_CONFIG_*` env vars, then removes the env var -/// 3. Prepends the shim dir to PATH -/// -/// Shell children receive `path_env`, `git_env`, and `BUZZ_PRIVATE_KEY` (for -/// 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). +/// Session-scoped utility aliases. Git is configured by the ACP harness. pub struct Shim { _dir: TempDir, pub path_env: String, - pub git_env: Vec<(String, String)>, } impl Shim { pub fn install() -> std::io::Result { 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", - ] { + for name in ["rg", "tree", "buzz"] { symlink(&self_exe, &dir.path().join(name))?; } - let original = std::env::var_os("PATH").unwrap_or_default(); let mut entries = vec![PathBuf::from(dir.path())]; entries.extend(std::env::split_paths(&original)); - // join_paths uses the platform separator (':' on Unix, ';' on Windows). let path_env = std::env::join_paths(entries) .map_err(|e| std::io::Error::new(std::io::ErrorKind::InvalidInput, e))? .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), - None => Vec::new(), - }; - if let Some(ref mut k) = nostr_key { - k.zeroize(); - } - Ok(Self { _dir: dir, path_env, - git_env, }) } } -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; @@ -357,339 +60,3 @@ pub fn artifact_dir(session_root: &Path) -> PathBuf { let _ = std::fs::create_dir_all(&p); 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, - }; - 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 key_info() -> KeyInfo { - KeyInfo { - keyfile_path: "/tmp/.nostr-key".into(), - pubkey_hex: PUBKEY_HEX.into(), - npub: NPUB.into(), - } - } - - /// 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()) - ); - } - - #[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" - ); - } - } - - #[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()) - ); - assert_eq!( - sanitize_git_user_name("Dun\u{202E}can Idaho"), - Some("Duncan Idaho".into()) - ); - } - - #[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()) - ); - } - - #[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_eq!( - git_config(&env, "user.signingkey").as_deref(), - Some(PUBKEY_HEX) - ); - } - - #[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()) - ); - } - - #[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"); - } - - #[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"); - } - } -} diff --git a/crates/sprig/src/main.rs b/crates/sprig/src/main.rs index 672a5a5f37..8676d1dcb9 100644 --- a/crates/sprig/src/main.rs +++ b/crates/sprig/src/main.rs @@ -14,7 +14,9 @@ fn dispatch() -> Result<(), String> { .to_ascii_lowercase(); match cmd.as_str() { - "buzz-acp" => buzz_acp::run().map_err(|e| e.to_string()), + "buzz-acp" | "git-credential-nostr" | "git-sign-nostr" => { + buzz_acp::run().map_err(|e| e.to_string()) + } "buzz-agent" => buzz_agent::run().map_err(|e| e.to_string()), "sprig" => match std::env::args().nth(1).as_deref() { Some("-V") | Some("--version") => { @@ -36,8 +38,7 @@ fn dispatch() -> Result<(), String> { )) } }, - // buzz-dev-mcp also handles its own multicall names: rg, tree, - // buzz, git-credential-nostr, and git-sign-nostr. + // buzz-dev-mcp handles its utility multicall names: rg, tree, buzz. _ => buzz_dev_mcp::run().map_err(|e| e.to_string()), } } @@ -47,7 +48,7 @@ fn print_usage() { "Sprig — all-in-one Buzz ACP harness, agent, and developer MCP\n\n\ Sprig is a multicall binary. Invoke it through one of the personality names:\n\n\ buzz-acp ACP harness\n buzz-agent ACP-compliant agent\n buzz-dev-mcp Developer MCP server\n\n\ -Developer MCP helper names are also supported: rg, tree, buzz, git-credential-nostr, git-sign-nostr.\n\n\ +Helper names are also supported: rg, tree, buzz, git-credential-nostr, git-sign-nostr.\n\n\ Installers can create links with:\n ln -s sprig buzz-acp\n ln -s sprig buzz-agent\n ln -s sprig buzz-dev-mcp" ); } diff --git a/desktop/src-tauri/src/managed_agents/runtime.rs b/desktop/src-tauri/src/managed_agents/runtime.rs index 5b44f95de9..07690ff5a8 100644 --- a/desktop/src-tauri/src/managed_agents/runtime.rs +++ b/desktop/src-tauri/src/managed_agents/runtime.rs @@ -19,6 +19,43 @@ mod path; pub(in crate::managed_agents) use path::build_augmented_path; pub(crate) use path::{compose_path_entries, should_skip_claude_executable, should_use_inherited}; +/// Custom ACP harnesses do not run buzz-acp's Git bootstrap. Preserve the +/// Desktop-provided relay credential helper for those commands. +fn apply_custom_acp_git_credentials( + command: &mut std::process::Command, + acp_command: &str, + private_key: &str, + relay_url: &str, + credential_helper: Option<&std::path::Path>, +) { + if acp_command == super::DEFAULT_ACP_COMMAND { + return; + } + let Some(helper) = credential_helper else { + eprintln!( + "buzz-desktop: git-credential-nostr not found — custom ACP command will not have automatic Buzz git auth" + ); + return; + }; + let relay_http_url = crate::relay::relay_http_base_url(relay_url); + command.env("NOSTR_PRIVATE_KEY", private_key); + command.env("GIT_TERMINAL_PROMPT", "0"); + command.env("GIT_CONFIG_COUNT", "2"); + command.env( + "GIT_CONFIG_KEY_0", + format!("credential.{relay_http_url}/git.helper"), + ); + command.env( + "GIT_CONFIG_VALUE_0", + helper.to_string_lossy().replace('\\', "/"), + ); + command.env( + "GIT_CONFIG_KEY_1", + format!("credential.{relay_http_url}/git.useHttpPath"), + ); + command.env("GIT_CONFIG_VALUE_1", "true"); +} + pub(crate) use super::access_policy::{build_respond_to_env_with_policy, RespondToEnv}; mod metadata; @@ -724,29 +761,16 @@ pub fn spawn_agent_child( command.env("BUZZ_ACP_RELAY_OBSERVER", "true"); - // Git credential helper: NIP-98 auth for Buzz relay git via git-credential-nostr. - // Ephemeral GIT_CONFIG_COUNT env vars scoped to relay HTTP URL; NOSTR_PRIVATE_KEY mirrors BUZZ_PRIVATE_KEY. - if let Some(cred_helper) = resolve_command("git-credential-nostr") { - let relay_http_url = crate::relay::relay_http_base_url(&effective_relay_url); - - command.env("NOSTR_PRIVATE_KEY", &record.private_key_nsec); - command.env("GIT_TERMINAL_PROMPT", "0"); - command.env("GIT_CONFIG_COUNT", "2"); - command.env( - "GIT_CONFIG_KEY_0", - format!("credential.{relay_http_url}/git.helper"), - ); - let helper = cred_helper.to_string_lossy().replace('\\', "/"); - command.env("GIT_CONFIG_VALUE_0", helper); - command.env( - "GIT_CONFIG_KEY_1", - format!("credential.{relay_http_url}/git.useHttpPath"), - ); - command.env("GIT_CONFIG_VALUE_1", "true"); - } else { - eprintln!( - "buzz-desktop: git-credential-nostr not found — agent {} will not have automatic Buzz git auth", - record.name, + // buzz-acp owns Git identity, scoped credentials, signing and key cleanup. + // An advanced custom ACP command bypasses that harness, so retain the + // earlier Desktop credential setup for that supported override. + if record.acp_command != super::DEFAULT_ACP_COMMAND { + apply_custom_acp_git_credentials( + &mut command, + &record.acp_command, + &record.private_key_nsec, + &effective_relay_url, + resolve_command("git-credential-nostr").as_deref(), ); } diff --git a/desktop/src-tauri/src/managed_agents/runtime/tests.rs b/desktop/src-tauri/src/managed_agents/runtime/tests.rs index 68d8ad7047..39247f7a15 100644 --- a/desktop/src-tauri/src/managed_agents/runtime/tests.rs +++ b/desktop/src-tauri/src/managed_agents/runtime/tests.rs @@ -594,6 +594,44 @@ fn codex_spawn_does_not_set_a_claude_executable() { .any(|(key, _)| key == "CLAUDE_CODE_EXECUTABLE")); } +#[test] +fn custom_acp_command_keeps_relay_git_credentials() { + let helper = std::path::Path::new("/opt/buzz/git-credential-nostr"); + let mut custom = std::process::Command::new("custom-acp"); + super::apply_custom_acp_git_credentials( + &mut custom, + "custom-acp", + "nsec-test", + "wss://relay.example", + Some(helper), + ); + let env: std::collections::HashMap<_, _> = custom + .get_envs() + .filter_map(|(key, value)| Some((key.to_str()?, value?.to_str()?))) + .collect(); + assert_eq!(env.get("NOSTR_PRIVATE_KEY"), Some(&"nsec-test")); + assert_eq!(env.get("GIT_CONFIG_COUNT"), Some(&"2")); + assert_eq!( + env.get("GIT_CONFIG_KEY_0"), + Some(&"credential.https://relay.example/git.helper") + ); + assert_eq!( + env.get("GIT_CONFIG_VALUE_0"), + Some(&"/opt/buzz/git-credential-nostr") + ); + assert_eq!(env.get("GIT_CONFIG_VALUE_1"), Some(&"true")); + + let mut standard = std::process::Command::new("buzz-acp"); + super::apply_custom_acp_git_credentials( + &mut standard, + super::super::DEFAULT_ACP_COMMAND, + "nsec-test", + "wss://relay.example", + Some(helper), + ); + assert_eq!(standard.get_envs().count(), 0); +} + /// On Windows, `.cmd` and `.bat` batch shims must NOT be assigned to /// `CLAUDE_CODE_EXECUTABLE` — `CreateProcess` cannot exec them directly and /// returns EINVAL (issue #2397). The adapter must fall back to its own PATH diff --git a/docs/remote-agents.md b/docs/remote-agents.md index 45289ef910..56de86bade 100644 --- a/docs/remote-agents.md +++ b/docs/remote-agents.md @@ -1021,12 +1021,11 @@ run — it MUST NOT fall back to `default`. `ghcr.io/block/buzz-sprig`: Alpine base + `bash` (required by the dev-MCP shell tool) + `git` + CA certificates + the static musl `sprig` multicall binary with its personality links (`buzz-acp`, `buzz-agent`, `buzz-dev-mcp`, -`rg`, `tree`, `buzz`, `git-credential-nostr`, `git-sign-nostr`) + a baked -system gitconfig wiring the nostr signing and credential helpers. The baked -credential-helper config MUST be scoped to the relay's git URL — mirroring -the local spawn's `credential./git.helper` scoping — never a -global `credential.helper`: a global nostr helper would answer for every -remote, including github.com. ~15–25MB; +`rg`, `tree`, `buzz`, `git-credential-nostr`, `git-sign-nostr`). The harness +provides ephemeral Git identity, signing and URL-scoped credentials to every +adapter and its MCP servers, exactly as on Desktop. No global or system Git +configuration is written. Credentials are restricted to the relay's `/git` +namespace; unrelated remotes retain their own helpers. ~15–25MB; not FROM-scratch (bash and git preclude it). Sprig-only: alternate-harness dependencies (node for Claude Code / Codex) come via the `image` override field, not a fatter default. Tagging follows the relay image's matrix — diff --git a/scripts/build-sprig.sh b/scripts/build-sprig.sh index 77ee6e5832..070fb19b4e 100755 --- a/scripts/build-sprig.sh +++ b/scripts/build-sprig.sh @@ -3,10 +3,10 @@ # harness, agent, and developer MCP. The archive exposes these command names: # # sprig implementation binary -# buzz-acp link to sprig (ACP harness) +# buzz-acp link to sprig (ACP harness and Git helper personalities) # buzz-agent link to sprig (ACP-compliant agent) # buzz-dev-mcp link to sprig (developer MCP server; also dispatches -# rg/tree/buzz/git-credential-nostr/git-sign-nostr) +# rg/tree/buzz) # # Usage: # ./scripts/build-sprig.sh [version] [target] @@ -140,12 +140,12 @@ to one multicall binary so shared Rust runtime/TLS code is stored only once. Commands: - `sprig` — prints usage/version. Invoke a personality by one of the links below. -- `buzz-acp` — ACP harness that bridges Buzz channel events to an +- `buzz-acp` — ACP harness and `git-credential-nostr` / `git-sign-nostr` + multicall entrypoint that bridges Buzz channel events to an ACP-compliant agent over stdio. - `buzz-agent` — ACP-compliant agent (spawns MCP servers, calls LLMs). - `buzz-dev-mcp` — Developer MCP server (shell, str_replace, todo) and - multicall entrypoint for `rg`, `tree`, `buzz`, `git-credential-nostr`, - `git-sign-nostr`. + multicall entrypoint for `rg`, `tree`, and `buzz`. See `sprig.json` for SHA-256s, sizes, target, and source git SHA. diff --git a/scripts/sprig-entrypoint.sh b/scripts/sprig-entrypoint.sh index 16abf7bc51..be73ce7d38 100755 --- a/scripts/sprig-entrypoint.sh +++ b/scripts/sprig-entrypoint.sh @@ -1,16 +1,6 @@ #!/bin/bash set -euo pipefail -# Match desktop's URL-scoped git credential configuration without installing a -# helper globally (which would make it answer for unrelated remotes). -if [[ -n "${BUZZ_RELAY_URL:-}" ]]; then - relay_http_url="${BUZZ_RELAY_URL/#ws:/http:}" - relay_http_url="${relay_http_url/#wss:/https:}" - relay_http_url="${relay_http_url%/}" - git config --global "credential.${relay_http_url}/git.helper" \ - /usr/local/bin/git-credential-nostr - git config --global "credential.${relay_http_url}/git.useHttpPath" true -fi - +# buzz-acp owns ephemeral Git setup for both native and MCP shells. # The harness must receive Kubernetes' termination signal directly. exec buzz-acp "$@" diff --git a/scripts/test-sprig-image.sh b/scripts/test-sprig-image.sh index 4ca42fdb4d..7bcab3faf8 100755 --- a/scripts/test-sprig-image.sh +++ b/scripts/test-sprig-image.sh @@ -16,28 +16,85 @@ assert_run ' for name in buzz-agent buzz-dev-mcp rg tree buzz git-credential-nostr git-sign-nostr; do test "$(readlink "/usr/local/bin/$name")" = sprig done - test "$(git config --system gpg.x509.program)" = /usr/local/bin/git-sign-nostr + ! git config --system --get gpg.x509.program ! git config --system --get-all credential.helper test "$HOME" = /home/agent test "$(pwd)" = /home/agent ' -assert_run ' - grep -Eq "^[[:space:]]*exec buzz-acp" /usr/local/bin/sprig-entrypoint - ! grep -Eq "^[[:space:]]*(buzz-acp|bash -c .*buzz-acp)" /usr/local/bin/sprig-entrypoint -' - docker run --rm --entrypoint /bin/bash \ -e BUZZ_RELAY_URL=wss://relay.example.test/ "$IMAGE" -ceu ' - /usr/local/bin/sprig-entrypoint --help >/dev/null 2>&1 & pid=$! - for _ in 1 2 3 4 5; do - git config --global --get credential.https://relay.example.test/git.helper >/dev/null 2>&1 && break - sleep 0.1 - done - test "$(git config --global --get credential.https://relay.example.test/git.helper)" = /usr/local/bin/git-credential-nostr - test "$(git config --global --get credential.https://relay.example.test/git.useHttpPath)" = true + /usr/local/bin/sprig-entrypoint --help >/dev/null + ! git config --global --get credential.https://relay.example.test/git.helper ! git config --global --get-all credential.helper - wait "$pid" || true ' +# Exercise the image entrypoint, ACP harness, and both Git helper personalities +# without contacting a relay. +docker run --rm -i --entrypoint /bin/bash "$IMAGE" -seuo pipefail <<'SCRIPT' +probe_dir=$(mktemp -d) +trap 'rm -rf "$probe_dir"' EXIT +export PROBE_DIR="$probe_dir" +cat > "$probe_dir/adapter" <<'ADAPTER' +#!/bin/bash +set -euo pipefail +cd "$PROBE_DIR" +git init -q repo +cd repo +git -c core.hooksPath=/dev/null commit -s --allow-empty -qm probe +git tag -m probe probe +git verify-commit HEAD +git verify-tag probe +test "$(git show -s --format=%an HEAD)" = "$(git config user.name)" +test "$(git show -s --format=%cn HEAD)" = "$(git config user.name)" +git config nostr.keyfile > ../keyfile +test "$(git config --get-urlmatch credential.helper https://relay.invalid/git/repo)" = nostr +! git config --get-urlmatch credential.helper https://unrelated.invalid/git/repo +printf 'capability[]=authtype\nprotocol=https\nhost=relay.invalid\npath=git/repo\nwwwauth[]=Nostr method="GET"\n\n' | + git credential fill > ../credential-result +grep -q '^authtype=Nostr$' ../credential-result +printf done > ../done +exec sleep 30 +ADAPTER +chmod 700 "$probe_dir/adapter" +# Disposable test key; never used with any relay. +GIT_AUTHOR_NAME='Inherited Human' GIT_AUTHOR_EMAIL=human@example.invalid \ +GIT_COMMITTER_NAME='Inherited Human' GIT_COMMITTER_EMAIL=human@example.invalid \ +BUZZ_PRIVATE_KEY=0000000000000000000000000000000000000000000000000000000000000001 \ +BUZZ_RELAY_URL=wss://relay.invalid \ +BUZZ_ACP_AGENT_COMMAND="$probe_dir/adapter" BUZZ_ACP_AGENT_ARGS= \ + /usr/local/bin/sprig-entrypoint > "$probe_dir/log" 2>&1 & +pid=$! +for _ in {1..100}; do + [[ -f "$probe_dir/done" ]] && break + sleep 0.1 +done +if ! kill -TERM "$pid" 2>/dev/null; then + wait "$pid" || true + cat "$probe_dir/log" + echo "harness exited before SIGTERM" >&2 + exit 1 +fi +for _ in {1..50}; do + kill -0 "$pid" 2>/dev/null || break + sleep 0.1 +done +if kill -0 "$pid" 2>/dev/null; then + kill -KILL "$pid" + wait "$pid" || true + cat "$probe_dir/log" + exit 1 +fi +if ! wait "$pid"; then + cat "$probe_dir/log" + exit 1 +fi +if [[ ! -f "$probe_dir/done" ]]; then + cat "$probe_dir/log" + exit 1 +fi +keyfile=$(cat "$probe_dir/keyfile") +test ! -e "$keyfile" +SCRIPT + echo "PASS: Sprig image runtime contract ($IMAGE)"