From 2d88937fbabf21013f28debb4d4f93270a4bb8cd Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 20:37:33 +0800 Subject: [PATCH 1/8] fix(gl): detect gitlawb remotes beyond origin in gl status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit gl init names its remote 'gitlawb', but detect_gitlawb_remote() only ever read origin — so gl status reported 'not in a gitlawb repo' in the very repo gl init had just set up. Scan all remotes, preferring origin (clones) then gitlawb, then any other gitlawb:// URL. (#231 acceptance test finding) --- crates/gl/src/status.rs | 58 ++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 13 deletions(-) diff --git a/crates/gl/src/status.rs b/crates/gl/src/status.rs index 2c77adab..c6acbc9a 100644 --- a/crates/gl/src/status.rs +++ b/crates/gl/src/status.rs @@ -59,7 +59,7 @@ pub async fn run(args: StatusArgs) -> Result<()> { println!(" repo {short_did}/{repo}"); } None => { - println!(" repo (not in a gitlawb repo — no gitlawb:// origin)"); + println!(" repo (not in a gitlawb repo — no gitlawb:// remote)"); } } @@ -143,29 +143,61 @@ fn trust_bar(score: f64) -> String { format!("{}{}", "█".repeat(filled), "░".repeat(empty)) } -/// Parse `gitlawb:///` from the git origin remote. +/// Find a `gitlawb:///` remote in the current repo. +/// +/// Checks `origin` first (clones), then `gitlawb` (the name `gl init` adds), +/// then any other remote with a gitlawb:// URL. fn detect_gitlawb_remote() -> Option<(String, String)> { let out = std::process::Command::new("git") - .args(["remote", "get-url", "origin"]) + .args(["remote"]) .stderr(std::process::Stdio::null()) .output() .ok()?; if !out.status.success() { return None; } - let url = String::from_utf8(out.stdout).ok()?; - let rest = url.trim().strip_prefix("gitlawb://")?; - let slash = rest.rfind('/')?; - let did = rest[..slash].to_string(); - let repo = rest[slash + 1..].to_string(); - if did.is_empty() || repo.is_empty() { - return None; + let names: Vec = String::from_utf8(out.stdout) + .ok()? + .lines() + .map(str::trim) + .filter(|l| !l.is_empty()) + .map(str::to_string) + .collect(); + + let preferred = ["origin", "gitlawb"]; + let ordered = preferred + .iter() + .filter(|p| names.iter().any(|n| n == *p)) + .map(|p| p.to_string()) + .chain( + names + .iter() + .filter(|n| !preferred.contains(&n.as_str())) + .cloned(), + ); + + for name in ordered { + let Ok(out) = std::process::Command::new("git") + .args(["remote", "get-url", &name]) + .stderr(std::process::Stdio::null()) + .output() + else { + continue; + }; + if !out.status.success() { + continue; + } + let Ok(url) = String::from_utf8(out.stdout) else { + continue; + }; + if let Some(parsed) = parse_gitlawb_url(&url) { + return Some(parsed); + } } - Some((did, repo)) + None } -/// Parse a gitlawb:// URL string into (did, repo) — extracted for testing. -#[cfg(test)] +/// Parse a gitlawb:// URL string into (did, repo). fn parse_gitlawb_url(url: &str) -> Option<(String, String)> { let rest = url.trim().strip_prefix("gitlawb://")?; let slash = rest.rfind('/')?; From cb42a4830d0d070165e2212f316d009ec716dbee Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 20:37:33 +0800 Subject: [PATCH 2/8] fix(gl): make gl init's push guidance match the repo's actual state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - git init -b main (plain-init fallback for git <2.28) so a fresh repo can't land on master while every hint says main. - The closing hint checks HEAD: with no commits it prints the add/commit step first — a bare 'git push gitlawb main' at that point fails with 'src refspec main does not match any', the exact trap the acceptance test walked into. The current branch name is used in the hint either way. --- crates/gl/src/init.rs | 47 +++++++++++++++++++++++++++++++++++++++---- 1 file changed, 43 insertions(+), 4 deletions(-) diff --git a/crates/gl/src/init.rs b/crates/gl/src/init.rs index d187c128..5a6a55fd 100644 --- a/crates/gl/src/init.rs +++ b/crates/gl/src/init.rs @@ -38,13 +38,23 @@ pub async fn run(args: InitArgs) -> Result<()> { let git_dir = cwd.join(".git"); if !git_dir.exists() { println!("Initializing git repository..."); + // -b main: the push flow targets `main`; without it a fresh repo uses + // the user's init.defaultBranch (often `master`). Older git (<2.28) + // lacks the flag, so fall back to a plain init. let status = std::process::Command::new("git") - .args(["init"]) + .args(["init", "-b", "main"]) .current_dir(&cwd) .status() .context("failed to run git init")?; if !status.success() { - anyhow::bail!("git init failed"); + let status = std::process::Command::new("git") + .args(["init"]) + .current_dir(&cwd) + .status() + .context("failed to run git init")?; + if !status.success() { + anyhow::bail!("git init failed"); + } } } else { println!("Git repository detected."); @@ -165,9 +175,38 @@ pub async fn run(args: InitArgs) -> Result<()> { } } + // The hint must match the repo's actual state: with no commits yet, a bare + // `git push` fails with "src refspec ... does not match any" — exactly the + // trap a zero-to-push command must not set. + let has_commits = std::process::Command::new("git") + .args(["rev-parse", "--verify", "HEAD"]) + .current_dir(&cwd) + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + .map(|s| s.success()) + .unwrap_or(false); + let branch = std::process::Command::new("git") + .args(["symbolic-ref", "--short", "HEAD"]) + .current_dir(&cwd) + .stderr(std::process::Stdio::null()) + .output() + .ok() + .filter(|o| o.status.success()) + .and_then(|o| String::from_utf8(o.stdout).ok()) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty()) + .unwrap_or_else(|| "main".to_string()); + println!(); - println!("Ready! Push with:"); - println!(" git push gitlawb main"); + if has_commits { + println!("Ready! Push with:"); + println!(" git push gitlawb {branch}"); + } else { + println!("Ready! Nothing is committed yet — commit, then push:"); + println!(" git add -A && git commit -m \"initial commit\""); + println!(" git push gitlawb {branch}"); + } Ok(()) } From b0a2ed830821b5a77b0dafdcf5ce4b1ad904ec25 Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 20:37:33 +0800 Subject: [PATCH 3/8] feat(gl): actually verify certificate signatures in gl cert show show printed the signing payload and told the user to verify offline themselves. It now rebuilds the node's canonical payload (field order mirrors gitlawb-node cert issuance), derives the Ed25519 key from the certificate's node DID, verifies, and prints a VALID/INVALID verdict; --verify makes the exit code reflect it for scripting. Round-trip + tamper + malformed-signature unit tests included. --- crates/gl/Cargo.toml | 1 + crates/gl/src/cert.rs | 161 +++++++++++++++++++++++++++++++++++++----- 2 files changed, 145 insertions(+), 17 deletions(-) diff --git a/crates/gl/Cargo.toml b/crates/gl/Cargo.toml index 9b11d573..d6e9fcf9 100644 --- a/crates/gl/Cargo.toml +++ b/crates/gl/Cargo.toml @@ -13,6 +13,7 @@ path = "src/main.rs" [dependencies] gitlawb-core = { path = "../gitlawb-core" } icaptcha-client = { path = "../icaptcha-client" } +base64 = { workspace = true } tokio = { workspace = true } serde = { workspace = true } serde_json = { workspace = true } diff --git a/crates/gl/src/cert.rs b/crates/gl/src/cert.rs index 09823d5b..95f39a2c 100644 --- a/crates/gl/src/cert.rs +++ b/crates/gl/src/cert.rs @@ -41,6 +41,9 @@ pub enum CertCmd { node: String, #[arg(long)] dir: Option, + /// Exit non-zero unless the Ed25519 signature verifies + #[arg(long)] + verify: bool, }, } @@ -52,7 +55,8 @@ pub async fn run(args: CertArgs) -> Result<()> { id, node, dir, - } => cmd_show(repo, id, node, dir).await, + verify, + } => cmd_show(repo, id, node, dir, verify).await, } } @@ -114,7 +118,13 @@ async fn cmd_list(repo: String, node: String, dir: Option) -> Result<() Ok(()) } -async fn cmd_show(repo: String, id: String, node: String, dir: Option) -> Result<()> { +async fn cmd_show( + repo: String, + id: String, + node: String, + dir: Option, + require_valid: bool, +) -> Result<()> { let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?; let client = signed_client(&node, dir.as_deref()); @@ -148,8 +158,26 @@ async fn cmd_show(repo: String, id: String, node: String, dir: Option) println!(" Signature: {signature}"); println!(); - // Reconstruct the signing payload and verify - // Fetch the node's current public key to verify + // Verify the Ed25519 signature: rebuild the exact canonical payload the + // node signed (see gitlawb-node/src/cert.rs::issue_ref_certificate) and + // check it against the public key embedded in the certificate's node DID. + // This proves the cert is internally authentic — signed by the key it + // names; the node-DID comparison below covers *which* node that is. + let repo_id = cert["repo_id"].as_str().unwrap_or(""); + let verdict = verify_signature( + repo_id, ref_name, old_sha, new_sha, pusher, node_did, issued_at, signature, + ); + + println!("Signature verification:"); + match &verdict { + Ok(()) => { + println!(" VALID — Ed25519 signature verified against the key in {node_did}"); + } + Err(reason) => { + println!(" INVALID — {reason}"); + } + } + let info: Value = client .get("/") .await? @@ -157,30 +185,69 @@ async fn cmd_show(repo: String, id: String, node: String, dir: Option) .await .context("failed to fetch node info")?; let current_node_did = info["did"].as_str().unwrap_or(""); - - println!("Signature verification:"); - println!(" Signing payload would be:"); - println!(" {{\"repo_id\": ..., \"ref\": \"{ref_name}\", \"old\": \"{old_sha}\","); - println!(" \"new\": \"{new_sha}\", \"pusher\": \"{pusher}\","); - println!(" \"node\": \"{node_did}\", \"ts\": \"{issued_at}\"}}"); - println!(); - if current_node_did == node_did { - println!(" Node DID matches current node. Signature is an Ed25519/base64url value."); - println!(" To verify offline, use the node's Ed25519 public key derived from:"); - println!(" did:key → {node_did}"); + println!(" Issuing node DID matches the node being queried."); } else { println!(" WARNING: Certificate node DID ({node_did}) does not match"); println!(" current node DID ({current_node_did})."); println!(" This certificate was issued by a different node."); } - println!(); - println!(" Signature (base64url): {signature}"); + if require_valid { + if let Err(reason) = verdict { + anyhow::bail!("certificate signature did not verify: {reason}"); + } + } Ok(()) } +/// Rebuild the node's canonical signing payload (field order must match +/// gitlawb-node/src/cert.rs::issue_ref_certificate exactly) and verify the +/// certificate's Ed25519 signature against the key embedded in `node_did`. +#[allow(clippy::too_many_arguments)] +fn verify_signature( + repo_id: &str, + ref_name: &str, + old_sha: &str, + new_sha: &str, + pusher: &str, + node_did: &str, + issued_at: &str, + signature_b64: &str, +) -> std::result::Result<(), String> { + use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine}; + use std::str::FromStr; + + let payload = serde_json::json!({ + "repo_id": repo_id, + "ref": ref_name, + "old": old_sha, + "new": new_sha, + "pusher": pusher, + "node": node_did, + "ts": issued_at, + }); + let payload_bytes = + serde_json::to_vec(&payload).map_err(|e| format!("could not serialize payload: {e}"))?; + + let did = + gitlawb_core::did::Did::from_str(node_did).map_err(|e| format!("bad node DID: {e}"))?; + let verifying_key = did + .to_verifying_key() + .map_err(|e| format!("cannot derive a public key from {node_did}: {e}"))?; + + let sig_vec = URL_SAFE_NO_PAD + .decode(signature_b64) + .map_err(|e| format!("signature is not valid base64url: {e}"))?; + let sig_bytes: [u8; 64] = sig_vec + .try_into() + .map_err(|_| "signature is not 64 bytes".to_string())?; + + gitlawb_core::identity::verify(&verifying_key, &payload_bytes, &sig_bytes) + .map_err(|_| "Ed25519 signature does not match the signed payload".to_string()) +} + async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str) -> Result { if id.len() >= 36 { return Ok(id.to_string()); @@ -209,3 +276,63 @@ async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str) _ => anyhow::bail!("certificate prefix {id} matches multiple certificates"), } } + +#[cfg(test)] +mod tests { + use super::*; + + /// Signing exactly as the node does must round-trip through + /// verify_signature; any field tampering must fail it. + #[test] + fn verify_signature_round_trip_and_tamper() { + let kp = gitlawb_core::identity::Keypair::generate(); + let node_did = kp.did().as_str().to_string(); + + let payload = serde_json::json!({ + "repo_id": "repo-1", + "ref": "refs/heads/main", + "old": "0".repeat(40), + "new": "a".repeat(40), + "pusher": "did:key:z6MkPusher", + "node": node_did, + "ts": "2026-07-22T00:00:00+00:00", + }); + let sig = kp.sign_b64(&serde_json::to_vec(&payload).unwrap()); + + let ok = verify_signature( + "repo-1", + "refs/heads/main", + &"0".repeat(40), + &"a".repeat(40), + "did:key:z6MkPusher", + &node_did, + "2026-07-22T00:00:00+00:00", + &sig, + ); + assert!(ok.is_ok(), "expected valid signature, got: {ok:?}"); + + let tampered = verify_signature( + "repo-1", + "refs/heads/main", + &"0".repeat(40), + &"b".repeat(40), // new_sha changed after signing + "did:key:z6MkPusher", + &node_did, + "2026-07-22T00:00:00+00:00", + &sig, + ); + assert!(tampered.is_err(), "tampered payload must not verify"); + + let garbage = verify_signature( + "repo-1", + "refs/heads/main", + &"0".repeat(40), + &"a".repeat(40), + "did:key:z6MkPusher", + &node_did, + "2026-07-22T00:00:00+00:00", + "not-base64url!!!", + ); + assert!(garbage.is_err(), "malformed signature must not verify"); + } +} From 94c61f79421fa26ecc1bdf7c255e0c48e66a2048 Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 20:37:33 +0800 Subject: [PATCH 4/8] fix(gl): doctor treats a reachable local node as configuration, not failure A localhost GITLAWB_NODE red-flagged every self-hosted and dev-harness setup even when the node was healthy; the connectivity check already fails loudly when the configured node is unreachable. The env check now passes with an explanatory note. --- crates/gl/src/doctor.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/crates/gl/src/doctor.rs b/crates/gl/src/doctor.rs index 1a5d96be..269f56a3 100644 --- a/crates/gl/src/doctor.rs +++ b/crates/gl/src/doctor.rs @@ -145,13 +145,15 @@ pub async fn run(args: DoctorArgs) -> Result<()> { Ok(v) if !v.is_empty() && !v.contains("127.0.0.1") && !v.contains("localhost") => { checks.push(Check::pass("GITLAWB_NODE", v.to_string())); } + // A local address is a legitimate setup (self-hosted node, dev + // harness) — the connectivity check below fails loudly if it is not + // actually reachable, so don't red-flag the configuration itself. Ok(v) if v.contains("127.0.0.1") || v.contains("localhost") => { - checks.push(Check::fail( + checks.push(Check::pass( "GITLAWB_NODE", format!( - "set to local address ({v}) — git push/clone will fail against remote nodes" + "{v} (local node — intentional for self-hosting/dev; unset to target the public network)" ), - "export GITLAWB_NODE=https://node.gitlawb.com", )); } _ => { From 0765c42feff47b29054b9a277881a7cd5bfd9135 Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 20:51:23 +0800 Subject: [PATCH 5/8] feat(gl): doctor warns when a shell alias shadows the gl binary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit oh-my-zsh's default git plugin ships gl='git pull', silently shadowing gl in every interactive zsh — the symptom is the baffling 'fatal: not a git repository'. Aliases beat PATH, so no installer can fix it; doctor now detects the setup (explicit alias gl= in ~/.zshrc, or the omz git plugin without an unalias) and prescribes the one-line fix. Best-effort heuristic, warning-level, covered by four unit tests. --- crates/gl/src/doctor.rs | 96 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) diff --git a/crates/gl/src/doctor.rs b/crates/gl/src/doctor.rs index 269f56a3..d2bf8e56 100644 --- a/crates/gl/src/doctor.rs +++ b/crates/gl/src/doctor.rs @@ -245,6 +245,15 @@ pub async fn run(args: DoctorArgs) -> Result<()> { )); } + // ── 5b. shell alias shadowing ───────────────────────────────────────── + // oh-my-zsh's default `git` plugin aliases gl='git pull', which silently + // shadows this binary in every interactive zsh — the classic symptom is + // `gl` printing "fatal: not a git repository". Aliases beat PATH, so no + // install method can fix it; the user's rc file has to unalias. + if let Some(check) = check_shell_alias_shadowing(dirs::home_dir()) { + checks.push(check); + } + // ── 6. git ──────────────────────────────────────────────────────────── match std::process::Command::new("git") .arg("--version") @@ -311,6 +320,51 @@ pub async fn run(args: DoctorArgs) -> Result<()> { } /// Check if a binary name exists anywhere on PATH. +/// Detect interactive-shell setups where an alias shadows `gl` (aliases beat +/// PATH, so no install method can fix this). Best-effort heuristic over +/// ~/.zshrc: an explicit `alias gl=`, or oh-my-zsh's `git` plugin (which +/// ships gl='git pull'). Returns None when nothing suspicious is found or the +/// rc file already contains an `unalias gl`. +fn check_shell_alias_shadowing(home: Option) -> Option { + let home = home?; + let rc = std::fs::read_to_string(home.join(".zshrc")).ok()?; + if rc.contains("unalias gl") { + return None; + } + + let explicit_alias = rc.lines().any(|l| l.trim_start().starts_with("alias gl=")); + // Single-line `plugins=(git ...)` is the overwhelmingly common form; a + // multi-line plugins array slips past this heuristic, which is acceptable + // for a warning-level check. + let omz_git_plugin = home.join(".oh-my-zsh/plugins/git").exists() + && rc.lines().any(|l| { + let l = l.trim_start(); + l.starts_with("plugins=") + && l.trim_start_matches("plugins=(") + .trim_end_matches(')') + .split_whitespace() + .any(|p| p == "git") + }); + + if explicit_alias || omz_git_plugin { + let source = if explicit_alias { + "~/.zshrc defines `alias gl=`" + } else { + "oh-my-zsh's git plugin aliases gl='git pull'" + }; + Some(Check::warn( + "shell alias", + format!( + "{source} — interactive shells run that instead of this binary \ + (symptom: `gl` prints \"fatal: not a git repository\")" + ), + "add `unalias gl` at the end of ~/.zshrc (after oh-my-zsh loads)", + )) + } else { + None + } +} + fn which_in_path(name: &str) -> bool { std::env::var_os("PATH") .map(|paths| { @@ -395,6 +449,48 @@ fn is_newer(latest: &str, current: &str) -> bool { mod tests { use super::*; + fn fake_home(zshrc: Option<&str>, with_omz_git: bool) -> tempfile::TempDir { + let home = tempfile::TempDir::new().unwrap(); + if let Some(rc) = zshrc { + std::fs::write(home.path().join(".zshrc"), rc).unwrap(); + } + if with_omz_git { + std::fs::create_dir_all(home.path().join(".oh-my-zsh/plugins/git")).unwrap(); + } + home + } + + #[test] + fn alias_shadowing_flags_omz_git_plugin() { + let home = fake_home(Some("plugins=(git z)\nsource $ZSH/oh-my-zsh.sh\n"), true); + let check = check_shell_alias_shadowing(Some(home.path().to_path_buf())); + assert!(check.is_some(), "omz git plugin without unalias must warn"); + } + + #[test] + fn alias_shadowing_flags_explicit_alias() { + let home = fake_home(Some("alias gl='git pull'\n"), false); + assert!(check_shell_alias_shadowing(Some(home.path().to_path_buf())).is_some()); + } + + #[test] + fn alias_shadowing_silent_when_unaliased() { + let home = fake_home( + Some("plugins=(git z)\nsource $ZSH/oh-my-zsh.sh\nunalias gl 2>/dev/null\n"), + true, + ); + assert!(check_shell_alias_shadowing(Some(home.path().to_path_buf())).is_none()); + } + + #[test] + fn alias_shadowing_silent_without_signals() { + let home = fake_home(Some("plugins=(z)\nexport EDITOR=vim\n"), false); + assert!(check_shell_alias_shadowing(Some(home.path().to_path_buf())).is_none()); + let no_rc = fake_home(None, false); + assert!(check_shell_alias_shadowing(Some(no_rc.path().to_path_buf())).is_none()); + assert!(check_shell_alias_shadowing(None).is_none()); + } + #[test] fn test_is_newer_minor_bump() { assert!(is_newer("0.2.0", "0.1.0")); From e3df51d76c0275fde33954ff0af0aab95fd0cb7d Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 20:51:23 +0800 Subject: [PATCH 6/8] fix: warn about the oh-my-zsh gl alias at install time install.sh detects the common shadowing setup (omz git plugin, no unalias in ~/.zshrc) and prints the fix right after installing; the generated Homebrew formula gains a caveats block saying the same, so brew install/upgrade surfaces it too. Catching this at install time beats letting the first gl invocation print git's error. --- .github/workflows/release.yml | 9 +++++++++ install.sh | 14 ++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 9fcb494d..0d0207a7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -591,6 +591,15 @@ jobs: bin.install "git-remote-gitlawb" end + def caveats + <<~CAVEATS + oh-my-zsh's git plugin aliases gl='git pull', which shadows this + binary in interactive shells. If \`gl\` prints "fatal: not a git + repository", run: + echo 'unalias gl' >> ~/.zshrc && source ~/.zshrc + CAVEATS + end + test do assert_match version.to_s, shell_output("#{bin}/gl --version") end diff --git a/install.sh b/install.sh index 80bd3601..93df7bf9 100644 --- a/install.sh +++ b/install.sh @@ -193,5 +193,19 @@ else echo " gl quickstart" fi +# oh-my-zsh's default git plugin aliases gl='git pull', which silently shadows +# the gl binary in every interactive zsh (aliases beat PATH). Detect the common +# setup and say so now, at install time, instead of letting the first `gl` +# print git's baffling "fatal: not a git repository". +if [ -f "$HOME/.zshrc" ] && [ -d "$HOME/.oh-my-zsh/plugins/git" ] \ + && grep -Eq '^[[:space:]]*plugins=\(([^)]*[[:space:]])?git([[:space:]][^)]*)?\)' "$HOME/.zshrc" \ + && ! grep -q 'unalias gl' "$HOME/.zshrc"; then + echo "" + echo "NOTE: oh-my-zsh's git plugin aliases gl='git pull', which will shadow" + echo "the gl command in interactive shells. To use gl by name, run:" + echo "" + echo " echo 'unalias gl' >> ~/.zshrc && source ~/.zshrc" +fi + echo "" echo "Docs: https://docs.gitlawb.com" From 3fa38dce4e435ef017f607d355266c2892161f8d Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 21:17:38 +0800 Subject: [PATCH 7/8] =?UTF-8?q?fix(gl):=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20non-fatal=20DID=20check,=20exact-host=20loopback,?= =?UTF-8?q?=20detached=20HEAD,=20multi-URL=20remotes,=20command-aware=20un?= =?UTF-8?q?alias?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - cert show: the node-DID comparison is contextual; a node-info fetch error now prints a note instead of failing the command after the certificate and verdict were already shown. - doctor: loopback detection parses the URL host exactly — substring matching misread localhost.example and localhost-in-path URLs. - init: a detached HEAD gets its own hint (switch to a branch first) instead of a guessed 'git push gitlawb main'. - status: every fetch AND push URL of each remote is inspected — a gitlawb:// push URL behind an https fetch URL now counts. - doctor/install.sh: unalias detection is command-aware — comments, 'unalias global', and gl-only-in-a-trailing-comment no longer suppress the warning (BSD-grep-compatible pattern; ordering vs oh-my-zsh load documented as accepted slack). Eight new test cases. --- crates/gl/src/cert.rs | 36 +++++++++++------- crates/gl/src/doctor.rs | 84 ++++++++++++++++++++++++++++++++++++++--- crates/gl/src/init.rs | 27 ++++++++----- crates/gl/src/status.rs | 37 ++++++++++-------- install.sh | 2 +- 5 files changed, 142 insertions(+), 44 deletions(-) diff --git a/crates/gl/src/cert.rs b/crates/gl/src/cert.rs index 95f39a2c..13de1945 100644 --- a/crates/gl/src/cert.rs +++ b/crates/gl/src/cert.rs @@ -178,19 +178,29 @@ async fn cmd_show( } } - let info: Value = client - .get("/") - .await? - .json() - .await - .context("failed to fetch node info")?; - let current_node_did = info["did"].as_str().unwrap_or(""); - if current_node_did == node_did { - println!(" Issuing node DID matches the node being queried."); - } else { - println!(" WARNING: Certificate node DID ({node_did}) does not match"); - println!(" current node DID ({current_node_did})."); - println!(" This certificate was issued by a different node."); + // Contextual only — the verdict above stands on its own, so a node-info + // hiccup here must not turn a successfully displayed certificate into an + // error exit. + let current_node_did = match client.get("/").await { + Ok(resp) => resp + .json::() + .await + .ok() + .and_then(|info| info["did"].as_str().map(str::to_string)), + Err(_) => None, + }; + match current_node_did.as_deref() { + Some(current) if current == node_did => { + println!(" Issuing node DID matches the node being queried."); + } + Some(current) => { + println!(" WARNING: Certificate node DID ({node_did}) does not match"); + println!(" current node DID ({current})."); + println!(" This certificate was issued by a different node."); + } + None => { + println!(" NOTE: could not fetch current node info — skipping node-DID comparison."); + } } if require_valid { diff --git a/crates/gl/src/doctor.rs b/crates/gl/src/doctor.rs index d2bf8e56..61140781 100644 --- a/crates/gl/src/doctor.rs +++ b/crates/gl/src/doctor.rs @@ -142,13 +142,10 @@ pub async fn run(args: DoctorArgs) -> Result<()> { // ── 3. GITLAWB_NODE env var ─────────────────────────────────────────── match std::env::var("GITLAWB_NODE") { - Ok(v) if !v.is_empty() && !v.contains("127.0.0.1") && !v.contains("localhost") => { - checks.push(Check::pass("GITLAWB_NODE", v.to_string())); - } - // A local address is a legitimate setup (self-hosted node, dev + // A loopback host is a legitimate setup (self-hosted node, dev // harness) — the connectivity check below fails loudly if it is not // actually reachable, so don't red-flag the configuration itself. - Ok(v) if v.contains("127.0.0.1") || v.contains("localhost") => { + Ok(v) if is_loopback_url(&v) => { checks.push(Check::pass( "GITLAWB_NODE", format!( @@ -156,6 +153,9 @@ pub async fn run(args: DoctorArgs) -> Result<()> { ), )); } + Ok(v) if !v.is_empty() => { + checks.push(Check::pass("GITLAWB_NODE", v.to_string())); + } _ => { checks.push(Check::fail( "GITLAWB_NODE", @@ -320,6 +320,38 @@ pub async fn run(args: DoctorArgs) -> Result<()> { } /// Check if a binary name exists anywhere on PATH. +/// True when the rc file contains a real `unalias` command naming `gl` — +/// not a comment, and not a longer word like `unalias global`. Ordering +/// relative to oh-my-zsh loading is not modeled (an unalias placed before the +/// plugin loads would be ineffective); acceptable slack for a warning check. +fn rc_unaliases_gl(rc: &str) -> bool { + rc.lines().any(|line| { + let line = line.trim_start(); + if line.starts_with('#') { + return false; + } + line.split([';', '&', '|']).any(|cmd| { + // A trailing comment must not count: `unalias foo # gl` + let cmd = cmd.split('#').next().unwrap_or(""); + let mut tokens = cmd.split_whitespace(); + tokens.next() == Some("unalias") && tokens.any(|t| t == "gl") + }) + }) +} + +/// True when the URL's host is exactly a loopback address. Substring checks +/// misclassify hosts like `localhost.example` or URLs with "localhost" in the +/// path, so parse and compare the actual host. +fn is_loopback_url(value: &str) -> bool { + reqwest::Url::parse(value) + .ok() + .and_then(|u| { + u.host_str() + .map(|h| h == "localhost" || h == "127.0.0.1" || h == "[::1]" || h == "::1") + }) + .unwrap_or(false) +} + /// Detect interactive-shell setups where an alias shadows `gl` (aliases beat /// PATH, so no install method can fix this). Best-effort heuristic over /// ~/.zshrc: an explicit `alias gl=`, or oh-my-zsh's `git` plugin (which @@ -328,7 +360,7 @@ pub async fn run(args: DoctorArgs) -> Result<()> { fn check_shell_alias_shadowing(home: Option) -> Option { let home = home?; let rc = std::fs::read_to_string(home.join(".zshrc")).ok()?; - if rc.contains("unalias gl") { + if rc_unaliases_gl(&rc) { return None; } @@ -482,6 +514,46 @@ mod tests { assert!(check_shell_alias_shadowing(Some(home.path().to_path_buf())).is_none()); } + #[test] + fn alias_shadowing_ignores_fake_unaliases() { + // None of these actually free `gl` — the warning must survive them. + for rc in [ + "plugins=(git)\n# unalias gl\n", // commented out + "plugins=(git)\nunalias global\n", // longer word + "plugins=(git)\nunalias foo # gl\n", // gl only in a comment + "plugins=(git)\necho unalias-gl-later\n", // not an unalias command + ] { + let home = fake_home(Some(rc), true); + assert!( + check_shell_alias_shadowing(Some(home.path().to_path_buf())).is_some(), + "must still warn for rc: {rc:?}" + ); + } + // Real unalias forms that do free it — all must silence the warning. + for rc in [ + "plugins=(git)\nunalias gl\n", + "plugins=(git)\nunalias gl 2>/dev/null\n", + "plugins=(git)\ntrue; unalias gl\n", + "plugins=(git)\nunalias glog gl gst\n", + ] { + let home = fake_home(Some(rc), true); + assert!( + check_shell_alias_shadowing(Some(home.path().to_path_buf())).is_none(), + "must be silent for rc: {rc:?}" + ); + } + } + + #[test] + fn loopback_url_detection_is_host_exact() { + assert!(is_loopback_url("http://127.0.0.1:7545")); + assert!(is_loopback_url("http://localhost:7545")); + assert!(is_loopback_url("http://[::1]:7545")); + assert!(!is_loopback_url("https://localhost.example")); + assert!(!is_loopback_url("https://node.gitlawb.com/localhost")); + assert!(!is_loopback_url("not a url")); + } + #[test] fn alias_shadowing_silent_without_signals() { let home = fake_home(Some("plugins=(z)\nexport EDITOR=vim\n"), false); diff --git a/crates/gl/src/init.rs b/crates/gl/src/init.rs index 5a6a55fd..1bc3c406 100644 --- a/crates/gl/src/init.rs +++ b/crates/gl/src/init.rs @@ -186,6 +186,8 @@ pub async fn run(args: InitArgs) -> Result<()> { .status() .map(|s| s.success()) .unwrap_or(false); + // symbolic-ref succeeds on a branch (including an unborn one) and fails + // on a detached HEAD — where guessing "main" could push the wrong ref. let branch = std::process::Command::new("git") .args(["symbolic-ref", "--short", "HEAD"]) .current_dir(&cwd) @@ -195,17 +197,24 @@ pub async fn run(args: InitArgs) -> Result<()> { .filter(|o| o.status.success()) .and_then(|o| String::from_utf8(o.stdout).ok()) .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) - .unwrap_or_else(|| "main".to_string()); + .filter(|s| !s.is_empty()); println!(); - if has_commits { - println!("Ready! Push with:"); - println!(" git push gitlawb {branch}"); - } else { - println!("Ready! Nothing is committed yet — commit, then push:"); - println!(" git add -A && git commit -m \"initial commit\""); - println!(" git push gitlawb {branch}"); + match (branch, has_commits) { + (Some(branch), true) => { + println!("Ready! Push with:"); + println!(" git push gitlawb {branch}"); + } + (Some(branch), false) => { + println!("Ready! Nothing is committed yet — commit, then push:"); + println!(" git add -A && git commit -m \"initial commit\""); + println!(" git push gitlawb {branch}"); + } + (None, _) => { + println!("Ready! HEAD is detached — create or switch to a branch, then push:"); + println!(" git switch -c main"); + println!(" git push gitlawb main"); + } } Ok(()) diff --git a/crates/gl/src/status.rs b/crates/gl/src/status.rs index c6acbc9a..cf477cc6 100644 --- a/crates/gl/src/status.rs +++ b/crates/gl/src/status.rs @@ -177,21 +177,28 @@ fn detect_gitlawb_remote() -> Option<(String, String)> { ); for name in ordered { - let Ok(out) = std::process::Command::new("git") - .args(["remote", "get-url", &name]) - .stderr(std::process::Stdio::null()) - .output() - else { - continue; - }; - if !out.status.success() { - continue; - } - let Ok(url) = String::from_utf8(out.stdout) else { - continue; - }; - if let Some(parsed) = parse_gitlawb_url(&url) { - return Some(parsed); + // A remote can carry several URLs, and the gitlawb:// one may be a + // push URL behind an https fetch URL — check every fetch and push URL. + for extra in [&["--all"][..], &["--push", "--all"][..]] { + let mut args = vec!["remote", "get-url"]; + args.extend_from_slice(extra); + args.push(&name); + let Ok(out) = std::process::Command::new("git") + .args(&args) + .stderr(std::process::Stdio::null()) + .output() + else { + continue; + }; + if !out.status.success() { + continue; + } + let Ok(urls) = String::from_utf8(out.stdout) else { + continue; + }; + if let Some(parsed) = urls.lines().find_map(parse_gitlawb_url) { + return Some(parsed); + } } } None diff --git a/install.sh b/install.sh index 93df7bf9..c6796f78 100644 --- a/install.sh +++ b/install.sh @@ -199,7 +199,7 @@ fi # print git's baffling "fatal: not a git repository". if [ -f "$HOME/.zshrc" ] && [ -d "$HOME/.oh-my-zsh/plugins/git" ] \ && grep -Eq '^[[:space:]]*plugins=\(([^)]*[[:space:]])?git([[:space:]][^)]*)?\)' "$HOME/.zshrc" \ - && ! grep -q 'unalias gl' "$HOME/.zshrc"; then + && ! grep -Eq '^([^#]*[[:space:];&|])?unalias[[:space:]]+([^;&|#]*[[:space:]])?gl([[:space:]]|$)' "$HOME/.zshrc"; then echo "" echo "NOTE: oh-my-zsh's git plugin aliases gl='git pull', which will shadow" echo "the gl command in interactive shells. To use gl by name, run:" From c681af70dba3a76b11795d324f2ca0fecf7e42fd Mon Sep 17 00:00:00 2001 From: Kevin Codex Date: Wed, 22 Jul 2026 21:43:44 +0800 Subject: [PATCH 8/8] fix(gl): anchor cert --verify to a trusted issuer; pin canonical payload form MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settles the review's design question: a valid signature alone proves internal consistency — the payload was signed by whatever key the certificate itself names — so a hostile source could self-sign and pass. --verify now also requires the issuing node_did to match a trusted anchor: --expect-node when given, else the DID of the node being queried; unanchorable (node info unreachable, no --expect-node) fails closed. The displayed verdict wording says what it proves. Also from review: - A frozen-literal test pins gl's payload serialization to the node's canonical byte form, so serialization drift (new field, a preserve_order feature unifying across the workspace) fails a test instead of silently rendering every real certificate INVALID. - The printed unalias remediation uses the idempotent-safe 'unalias gl 2>/dev/null' form everywhere (doctor, install.sh, brew caveats), matching what doctor already suggested. - Acknowledged, deliberately left: bash-side alias detection and 127/8-range loopback matching (noted as out of scope for a warning-level heuristic). --- .github/workflows/release.yml | 2 +- crates/gl/src/cert.rs | 58 +++++++++++++++++++++++++++++++++-- crates/gl/src/doctor.rs | 2 +- install.sh | 2 +- 4 files changed, 58 insertions(+), 6 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 0d0207a7..dbc7f852 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -596,7 +596,7 @@ jobs: oh-my-zsh's git plugin aliases gl='git pull', which shadows this binary in interactive shells. If \`gl\` prints "fatal: not a git repository", run: - echo 'unalias gl' >> ~/.zshrc && source ~/.zshrc + echo 'unalias gl 2>/dev/null' >> ~/.zshrc && source ~/.zshrc CAVEATS end diff --git a/crates/gl/src/cert.rs b/crates/gl/src/cert.rs index 13de1945..87ad5aec 100644 --- a/crates/gl/src/cert.rs +++ b/crates/gl/src/cert.rs @@ -41,9 +41,16 @@ pub enum CertCmd { node: String, #[arg(long)] dir: Option, - /// Exit non-zero unless the Ed25519 signature verifies + /// Exit non-zero unless the Ed25519 signature verifies AND the + /// issuing node matches the queried node (or --expect-node) #[arg(long)] verify: bool, + /// Expected issuing node DID for --verify. A valid signature alone + /// only proves the cert is internally consistent — signed by whatever + /// key it names — so --verify also anchors the issuer to a DID you + /// trust: this value when given, else the queried node's DID. + #[arg(long, requires = "verify")] + expect_node: Option, }, } @@ -56,7 +63,8 @@ pub async fn run(args: CertArgs) -> Result<()> { node, dir, verify, - } => cmd_show(repo, id, node, dir, verify).await, + expect_node, + } => cmd_show(repo, id, node, dir, verify, expect_node).await, } } @@ -124,6 +132,7 @@ async fn cmd_show( node: String, dir: Option, require_valid: bool, + expect_node: Option, ) -> Result<()> { let (owner, name) = resolve_repo(&repo, &node, dir.as_deref()).await?; @@ -171,7 +180,9 @@ async fn cmd_show( println!("Signature verification:"); match &verdict { Ok(()) => { - println!(" VALID — Ed25519 signature verified against the key in {node_did}"); + println!( + " VALID — Ed25519 signature verified against the key the certificate names ({node_did})" + ); } Err(reason) => { println!(" INVALID — {reason}"); @@ -207,6 +218,22 @@ async fn cmd_show( if let Err(reason) = verdict { anyhow::bail!("certificate signature did not verify: {reason}"); } + // A valid signature proves internal consistency only: the payload was + // signed by whatever key the certificate itself names. A hostile + // source can mint a keypair, put its DID in node_did, and self-sign. + // --verify therefore also anchors the issuer to a trusted DID: + // --expect-node when given, else the DID of the node being queried. + let expected = expect_node.as_deref().or(current_node_did.as_deref()); + match expected { + Some(expected) if expected == node_did => {} + Some(expected) => anyhow::bail!( + "certificate is signed by {node_did}, but the expected issuer is {expected} — \ + a valid signature alone proves internal consistency, not a trusted issuer" + ), + None => anyhow::bail!( + "cannot anchor the issuer: node info is unreachable and no --expect-node was given" + ), + } } Ok(()) @@ -291,6 +318,31 @@ async fn resolve_cert_id(client: &NodeClient, owner: &str, name: &str, id: &str) mod tests { use super::*; + /// Pins gl's payload reconstruction to the frozen canonical byte form the + /// node signs (default serde_json maps = alphabetically ordered keys). If + /// serialization drifts — a field added, or a preserve_order feature + /// landing anywhere in the workspace (feature unification flips every + /// crate at once) — this literal stops matching and the test fails, + /// instead of every real certificate silently rendering INVALID. + #[test] + fn payload_serialization_matches_frozen_canonical_form() { + let payload = serde_json::json!({ + "repo_id": "repo-1", + "ref": "refs/heads/main", + "old": "oldsha", + "new": "newsha", + "pusher": "did:key:z6MkPusher", + "node": "did:key:z6MkNode", + "ts": "2026-07-22T00:00:00+00:00", + }); + let frozen = concat!( + r#"{"new":"newsha","node":"did:key:z6MkNode","old":"oldsha","#, + r#""pusher":"did:key:z6MkPusher","ref":"refs/heads/main","#, + r#""repo_id":"repo-1","ts":"2026-07-22T00:00:00+00:00"}"#, + ); + assert_eq!(serde_json::to_string(&payload).unwrap(), frozen); + } + /// Signing exactly as the node does must round-trip through /// verify_signature; any field tampering must fail it. #[test] diff --git a/crates/gl/src/doctor.rs b/crates/gl/src/doctor.rs index 61140781..86f50334 100644 --- a/crates/gl/src/doctor.rs +++ b/crates/gl/src/doctor.rs @@ -390,7 +390,7 @@ fn check_shell_alias_shadowing(home: Option) -> Option { "{source} — interactive shells run that instead of this binary \ (symptom: `gl` prints \"fatal: not a git repository\")" ), - "add `unalias gl` at the end of ~/.zshrc (after oh-my-zsh loads)", + "echo 'unalias gl 2>/dev/null' >> ~/.zshrc && source ~/.zshrc (must come after oh-my-zsh loads)", )) } else { None diff --git a/install.sh b/install.sh index c6796f78..416e7c9f 100644 --- a/install.sh +++ b/install.sh @@ -204,7 +204,7 @@ if [ -f "$HOME/.zshrc" ] && [ -d "$HOME/.oh-my-zsh/plugins/git" ] \ echo "NOTE: oh-my-zsh's git plugin aliases gl='git pull', which will shadow" echo "the gl command in interactive shells. To use gl by name, run:" echo "" - echo " echo 'unalias gl' >> ~/.zshrc && source ~/.zshrc" + echo " echo 'unalias gl 2>/dev/null' >> ~/.zshrc && source ~/.zshrc" fi echo ""