diff --git a/crates/gitlawb-node/src/api/mod.rs b/crates/gitlawb-node/src/api/mod.rs index df10175a9..cecdcfb22 100644 --- a/crates/gitlawb-node/src/api/mod.rs +++ b/crates/gitlawb-node/src/api/mod.rs @@ -92,6 +92,25 @@ pub(crate) fn require_repo_owner(record: &RepoRecord, caller: &str) -> Result<() } } +/// Validate a branch name using the configured git binary (storage boundaries). +pub(crate) fn validate_git_ref_with_git( + git_bin: &str, + name: &str, +) -> std::result::Result<(), crate::git::store::GitRefValidationError> { + crate::git::store::validate_git_ref_with_git(git_bin, name) +} + +/// Map ref-validation failures to the correct HTTP surface: malformed caller +/// input stays 400; an inability to run git stays on the git/server-error path. +pub(crate) fn map_git_ref_validation_error( + err: crate::git::store::GitRefValidationError, +) -> AppError { + match err { + crate::git::store::GitRefValidationError::Invalid(msg) => AppError::BadRequest(msg), + crate::git::store::GitRefValidationError::GitUnavailable(msg) => AppError::Git(msg), + } +} + #[cfg(test)] mod did_tests { use super::did_matches; diff --git a/crates/gitlawb-node/src/api/pulls.rs b/crates/gitlawb-node/src/api/pulls.rs index 6255ef246..d1e0185ad 100644 --- a/crates/gitlawb-node/src/api/pulls.rs +++ b/crates/gitlawb-node/src/api/pulls.rs @@ -50,6 +50,17 @@ pub async fn create_pr( let target_branch = req .target_branch .unwrap_or_else(|| record.default_branch.clone()); + + // Validate both refs before they are stored, since both are later + // interpolated into git argv (git diff / worktree add / merge). Validate the + // RESOLVED target, not just a caller-supplied one: create_repo also gates + // default_branch, but validating here as well means a PR can never feed the + // git sink an unchecked ref even if a default was poisoned by an older row + // or a future path that skips create_repo's gate. + crate::api::validate_git_ref_with_git(&state.git_bin, &req.source_branch) + .map_err(crate::api::map_git_ref_validation_error)?; + crate::api::validate_git_ref_with_git(&state.git_bin, &target_branch) + .map_err(crate::api::map_git_ref_validation_error)?; let number = state.db.next_pr_number(&record.id).await?; let now = Utc::now().to_rfc3339(); diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index 4e327c429..1fb21c51a 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -242,6 +242,12 @@ pub async fn create_repo( )); } + // default_branch is caller-supplied and becomes a PR's target_branch when the + // PR omits one, which is interpolated into a git revision argument. Validate + // it as a ref so it cannot begin with '-' and inject a git option downstream. + crate::api::validate_git_ref_with_git(&state.git_bin, &req.default_branch) + .map_err(crate::api::map_git_ref_validation_error)?; + // Owner is the authenticated agent's DID let owner_did = auth.0; diff --git a/crates/gitlawb-node/src/git/store.rs b/crates/gitlawb-node/src/git/store.rs index 5617b4198..9a05573af 100644 --- a/crates/gitlawb-node/src/git/store.rs +++ b/crates/gitlawb-node/src/git/store.rs @@ -746,13 +746,149 @@ pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result) -> std::fmt::Result { + match self { + Self::Invalid(msg) | Self::GitUnavailable(msg) => f.write_str(msg), + } + } +} + +/// Reject revision shorthands and pseudoref names that `git check-ref-format +/// --branch` accepts but that git later interprets as symbolic revisions rather +/// than literal local branch names. +fn reject_revision_shorthand(name: &str) -> Option<&'static str> { + if name == "@" { + return Some("branch ref must not be a revision shorthand"); + } + if name.starts_with("@{") { + return Some("branch ref must not be a reflog revision expression"); + } + const PSEUDOREFS: &[&str] = &[ + "FETCH_HEAD", + "ORIG_HEAD", + "MERGE_HEAD", + "CHERRY_PICK_HEAD", + "RERERE_MERGE_HEAD", + "REBASE_HEAD", + "REVERT_HEAD", + "BISECT_HEAD", + "AUTO_MERGE", + ]; + if PSEUDOREFS.contains(&name) { + return Some("branch ref must not be a git pseudoref name"); + } + None +} + +/// Validate a git *branch name* before it is stored or passed to git subprocesses. +/// Delegates to `git check-ref-format --branch`, then rejects fully qualified +/// ref paths, revision-namespace shorthands (`heads/`, `tags/`, `remotes/`), and +/// symbolic names that git would reinterpret at the sink. +/// +/// At diff/merge sinks, validated short names are passed as `refs/heads/{name}` so +/// grammar-valid revision shorthands cannot retarget another ref namespace. The +/// merge worktree checks out the local branch name instead, so merge commits +/// advance refs/heads/{target}. Symbolic revision names (`HEAD`), option-shaped +/// trailing dots, and other invalid forms are rejected by check-ref-format itself. +/// Revision shorthands that pass the grammar check (`@`, `@{-1}`, pseudorefs) are +/// rejected explicitly because git treats them as symbolic revisions when bare. +/// +/// Storage boundaries call this via [`validate_git_ref_with_git`] with the configured +/// git binary. Sink functions call [`validate_git_ref`] (system `git`) then prefix +/// before building argv, so the property holds for every caller and every row, +/// including legacy rows and any writer that skipped the boundary check. +pub fn validate_git_ref(name: &str) -> std::result::Result<(), GitRefValidationError> { + validate_git_ref_with_git("git", name) +} + +pub(crate) fn validate_git_ref_with_git( + git_bin: &str, + name: &str, +) -> std::result::Result<(), GitRefValidationError> { + if name.is_empty() { + return Err(GitRefValidationError::Invalid( + "branch ref must not be empty".into(), + )); + } + if name.starts_with("refs/") { + return Err(GitRefValidationError::Invalid( + "branch ref must be a local branch name, not a fully qualified ref".into(), + )); + } + if name.starts_with("heads/") || name.starts_with("tags/") || name.starts_with("remotes/") { + return Err(GitRefValidationError::Invalid( + "branch ref must be a local branch name, not a revision shorthand".into(), + )); + } + if let Some(reason) = reject_revision_shorthand(name) { + return Err(GitRefValidationError::Invalid(reason.into())); + } + // allow-unbounded-git: stateless check-ref-format on a user-supplied name; no repo acquire or concurrency permit + let output = Command::new(git_bin) + .args(["check-ref-format", "--branch", name]) + .output() + .map_err(|e| { + // allow-unbounded-git: stateless check-ref-format on user-supplied name; no repo acquire + GitRefValidationError::GitUnavailable(format!( + "failed to run git check-ref-format: {e}" + )) + })?; + + if output.status.success() { + Ok(()) + } else { + let stderr = String::from_utf8_lossy(&output.stderr); + Err(GitRefValidationError::Invalid(stderr.trim().to_string())) + } +} + +/// Reject both refs at the sink, so an option-shaped ref can never reach a git +/// argv element regardless of how it was stored. +fn map_ref_validation_to_anyhow(which: &str, err: GitRefValidationError) -> anyhow::Error { + match err { + GitRefValidationError::Invalid(msg) => { + anyhow::anyhow!("invalid {which} branch ref: {msg}") + } + GitRefValidationError::GitUnavailable(msg) => anyhow::anyhow!("{msg}"), + } +} + +fn guard_refs(target_branch: &str, source_branch: &str) -> Result<()> { + validate_git_ref(target_branch).map_err(|e| map_ref_validation_to_anyhow("target", e))?; + validate_git_ref(source_branch).map_err(|e| map_ref_validation_to_anyhow("source", e))?; + Ok(()) +} + +/// Stored short branch names are passed to git as explicit local-branch refs so a +/// syntax-valid name cannot be reinterpreted as a revision shorthand at the sink. +fn local_branch_ref(short_name: &str) -> String { + format!("refs/heads/{short_name}") +} + /// Get the diff between two branches: changes on source_branch not in target_branch. pub fn branch_diff(repo_path: &Path, target_branch: &str, source_branch: &str) -> Result { + guard_refs(target_branch, source_branch)?; + let target = local_branch_ref(target_branch); + let source = local_branch_ref(source_branch); let output = Command::new("git") - .args(["diff", &format!("{target_branch}...{source_branch}")]) + .args(["diff", &format!("{target}...{source}")]) .current_dir(repo_path) .output() .context("failed to run git diff")?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + bail!("git diff failed: {stderr}"); + } Ok(String::from_utf8_lossy(&output.stdout).to_string()) } @@ -765,13 +901,11 @@ pub fn branch_diff_names( target_branch: &str, source_branch: &str, ) -> Result> { + guard_refs(target_branch, source_branch)?; + let target = local_branch_ref(target_branch); + let source = local_branch_ref(source_branch); let output = Command::new("git") - .args([ - "diff", - "--name-only", - "-z", - &format!("{target_branch}...{source_branch}"), - ]) + .args(["diff", "--name-only", "-z", &format!("{target}...{source}")]) .current_dir(repo_path) .output() .context("failed to run git diff --name-only")?; @@ -799,18 +933,51 @@ pub fn merge_branch( author_did: &str, pr_title: &str, ) -> Result { + guard_refs(target_branch, source_branch)?; + let source_ref = local_branch_ref(source_branch); + let target_ref = local_branch_ref(target_branch); let worktree_path = repo_path.join("_merge_worktree"); - // Clean up any leftover worktree - if worktree_path.exists() { + let remove_worktree = || { let _ = Command::new("git") .args(["worktree", "remove", "--force", "_merge_worktree"]) .current_dir(repo_path) .output(); let _ = std::fs::remove_dir_all(&worktree_path); + }; + + // Clean up any leftover worktree + if worktree_path.exists() { + remove_worktree(); + } + + // The merge must land on the LOCAL target branch, so the worktree has to end + // up attached to exactly refs/heads/{target}. validate_git_ref proves + // branch-name grammar only: it does not prove refs/heads/{target} exists, + // and it cannot stop git's revision DWIM rules from resolving the same + // short name in another namespace (refs/tags/{name}, refs/remotes/...). If + // DWIM won, `worktree add` would check out a detached HEAD at the foreign + // ref, the merge would succeed on that disposable HEAD, and cleanup would + // discard the commit without ever advancing refs/heads/{target}. + // + // (1) Require the exact local ref before creating the worktree. show-ref + // --verify matches the full refname literally — no DWIM, no abbreviation — + // so a same-named tag with no local branch fails here instead of being + // silently resolved. + let target_exists = Command::new("git") + .args(["show-ref", "--verify", "--quiet", &target_ref]) + .current_dir(repo_path) + .output() + .context("failed to run git show-ref")?; + if !target_exists.status.success() { + bail!("target branch {target_ref} does not exist as a local branch"); } - // Create worktree on target branch + // (2) Check out the local target branch in the worktree. The bare short + // name is what makes git attach HEAD to the branch (passing + // refs/heads/{name} would detach, so a successful merge would not advance + // refs/heads/{target}); step (1) guarantees the branch exists, so an + // attached checkout is the only acceptable outcome — verified right below. let wt = Command::new("git") .args(["worktree", "add", "_merge_worktree", target_branch]) .current_dir(repo_path) @@ -823,12 +990,28 @@ pub fn merge_branch( ); } + // The worktree's HEAD must be a symbolic ref to exactly refs/heads/{target}. + // A detached or differently-attached HEAD means git resolved the name as + // something other than the local target branch; a merge committed on it + // would be thrown away by cleanup, so refuse before merging rather than + // relying on any denylist of symbolic shapes. + let head_ref = Command::new("git") + .args(["symbolic-ref", "--quiet", "HEAD"]) + .current_dir(&worktree_path) + .output() + .context("failed to run git symbolic-ref")?; + let head_ref_name = String::from_utf8_lossy(&head_ref.stdout).trim().to_string(); + if !head_ref.status.success() || head_ref_name != target_ref { + remove_worktree(); + bail!("merge worktree is not attached to {target_ref} (HEAD is {head_ref_name:?})"); + } + // Run merge in worktree let merge = Command::new("git") .args([ "merge", "--no-ff", - source_branch, + &source_ref, "-m", &format!( "Merge branch '{}' into {} ({})", @@ -846,11 +1029,7 @@ pub fn merge_branch( let success = merge.status.success(); // Always remove worktree - let _ = Command::new("git") - .args(["worktree", "remove", "--force", "_merge_worktree"]) - .current_dir(repo_path) - .output(); - let _ = std::fs::remove_dir_all(&worktree_path); + remove_worktree(); if !success { bail!( @@ -859,12 +1038,24 @@ pub fn merge_branch( ); } - // Get new HEAD of target branch + // (3) The merge only counts if the exact target ref now points at a commit. + // Plain `rev-parse` echoes an unresolvable token back on stdout with a + // nonzero exit status, so an unchecked call here would hand the literal + // string "refs/heads/{target}" upward as an apparently valid merge SHA and + // the caller would mark the PR merged and fire the webhook. --verify plus + // ^{commit} plus an explicit status check turn "target ref missing or not a + // commit" into a hard failure before any of that happens. let head = Command::new("git") - .args(["rev-parse", &format!("refs/heads/{target_branch}")]) + .args(["rev-parse", "--verify", &format!("{target_ref}^{{commit}}")]) .current_dir(repo_path) .output() .context("failed to get merge commit")?; + if !head.status.success() { + bail!( + "merge completed but {target_ref} does not resolve to a commit: {}", + String::from_utf8_lossy(&head.stderr) + ); + } Ok(String::from_utf8_lossy(&head.stdout).trim().to_string()) } @@ -878,6 +1069,72 @@ pub fn repo_disk_path(repos_dir: &Path, owner_did: &str, repo_name: &str) -> Pat #[cfg(test)] mod tests { + use super::{validate_git_ref, validate_git_ref_with_git, GitRefValidationError}; + + #[test] + fn validate_git_ref_accepts_normal_branch_names() { + for good in [ + "main", + "feature/foo", + "release-1.2", + "v1.0.0", + "user/fix-bug", + "feature@", + "user@host", + ] { + assert!( + validate_git_ref(good).is_ok(), + "{good:?} should be accepted" + ); + } + } + + #[test] + fn validate_git_ref_rejects_option_injection_and_malformed_refs() { + for bad in [ + "", + "HEAD", + "@", + "@{-1}", + "FETCH_HEAD", + "MERGE_HEAD", + "feature.", + "feature/x.", + "refs/heads/main", + "refs/tags/v1", + "refs/heads/--output=/tmp/x", + "heads/main", + "tags/v1", + "remotes/origin/main", + "--output=/tmp/x", + "-rf", + "a b", + "a..b", + "a~b", + "refs/heads/@{x}", + "foo.lock", + "/leading", + "trailing/", + "a//b", + ] { + let err = validate_git_ref(bad).expect_err("{bad:?} should be rejected"); + assert!( + matches!(err, GitRefValidationError::Invalid(_)), + "{bad:?} should be an invalid-name rejection, got {err:?}" + ); + } + } + + #[test] + fn validate_git_ref_spawn_failure_is_git_unavailable_not_invalid() { + let err = validate_git_ref_with_git("/nonexistent/gitlawb-git", "main") + .expect_err("missing git binary should fail"); + assert!( + matches!(err, GitRefValidationError::GitUnavailable(_)), + "spawn failure must not be classified as invalid input, got {err:?}" + ); + } + use super::branch_diff_names; use std::path::Path; use std::process::Command; diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 430c06002..93d4744b7 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -188,7 +188,6 @@ mod tests { use axum::http::StatusCode; use chrono::Utc; use tower::ServiceExt; - fn seed_repo(owner_did: &str, name: &str) -> RepoRecord { let now = Utc::now(); RepoRecord { @@ -786,6 +785,1667 @@ mod tests { ); } + /// SINK GUARD (defense in depth): a poisoned PR row that bypassed the create_pr + /// (a pre-fix row, or any writer create_pr does not gate) still reach the git + /// sink? Inserts the row directly via db.create_pr and drives get_pr_diff + /// anonymously. If the attacker-named file appears, the storage-boundary fix + /// does not cover legacy/other-writer rows and the sink itself needs a guard. + #[sqlx::test] + async fn poisoned_pr_row_cannot_write_a_file_through_the_diff_sink(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let src = std::env::temp_dir().join(format!("gl-probe-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("f.txt"), b"hi").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("probe.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + + let mut repo = seed_repo(&owner_did, "probe"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pwn = std::env::temp_dir().join(format!("gl-PROBE-{short}")); + let _ = std::fs::remove_file(&pwn); + let pwn_glued: std::path::PathBuf = format!("{}...main", pwn.to_str().unwrap()).into(); + let _pg1 = DirGuard(pwn.clone()); + let _pg2 = DirGuard(pwn_glued.clone()); + + // Insert the poisoned row DIRECTLY, as a pre-fix row or an ungated writer would. + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "x".into(), + body: None, + author_did: owner_did.clone(), + source_branch: "main".into(), + target_branch: format!("--output={}", pwn.to_str().unwrap()), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert poisoned row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/diff", + axum::routing::get(crate::api::pulls::get_pr_diff), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/probe/pulls/1/diff"); + let resp = router.oneshot(anon_get(&uri)).await.unwrap(); + let st = resp.status(); + assert!( + !st.is_success(), + "get_pr_diff must not succeed for a poisoned row (got {st})" + ); + + let written = pwn.exists() || pwn_glued.exists(); + assert!( + !written, + "the sink must not write a file even for a poisoned row that bypassed the \ + create_pr boundary (get_pr_diff status {st})" + ); + } + + /// SECURITY (option injection, source arm): create_pr must reject an + /// option-shaped source_branch too. merge_branch interpolates source as its + /// own argv element (git merge {source}), so this is the merge-shaped twin of + /// the target test above. + #[sqlx::test] + async fn create_pr_rejects_option_injecting_source_branch(pool: PgPool) { + let owner = "did:key:zPRSRCINJAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let state = test_state(pool).await; + let repo = seed_repo(owner, "pub-src-inj-repo"); + state.db.create_repo(&repo).await.expect("seed repo"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls", + axum::routing::post(crate::api::pulls::create_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner}/pub-src-inj-repo/pulls"); + let body = Body::from( + r#"{"title":"x","source_branch":"--output=/tmp/gl-should-not-exist-src","target_branch":"main"}"#, + ); + let resp = router + .oneshot(signed_request_as(owner, Method::POST, &uri, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "an option-shaped source_branch must be rejected, got {}", + resp.status() + ); + let prs = state.db.list_prs(&repo.id).await.expect("list_prs"); + assert!(prs.is_empty(), "no PR row when source_branch is rejected"); + } + + /// SECURITY (option injection, second entry point): create_repo must reject a + /// default_branch that git would parse as an option. Otherwise an owner sets + /// default_branch = "--output=...", opens a PR omitting target_branch so the + /// stored target falls back to that default, and the diff/merge sink injects. + #[sqlx::test] + async fn create_repo_rejects_option_injecting_default_branch(pool: PgPool) { + let owner = "did:key:zRepoDEFINJAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let state = test_state(pool).await; + + let router = Router::new() + .route( + "/api/v1/repos", + axum::routing::post(crate::api::repos::create_repo), + ) + .with_state(state.clone()); + let body = Body::from( + r#"{"name":"inj-default","default_branch":"--output=/tmp/gl-should-not-exist-def"}"#, + ); + let resp = router + .oneshot(signed_request_as( + owner, + Method::POST, + "/api/v1/repos", + body, + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "an option-shaped default_branch must be rejected, got {}", + resp.status() + ); + + // Nothing stored: the boundary rejects before init/create_repo. + assert!( + state + .db + .get_repo(owner, "inj-default") + .await + .unwrap() + .is_none(), + "no repo row must be created when default_branch is rejected" + ); + } + + /// SECURITY (option injection): create_pr must reject a branch ref that git + /// would parse as an option, so a stored `--output=...` target cannot later + /// turn get_pr_diff / merge into an arbitrary file write. The caller here is + /// the owner (a reader) of a PUBLIC repo, i.e. the minimum access needed to + /// plant a PR, and the request must be refused before any row is written. + #[sqlx::test] + async fn create_pr_rejects_option_injecting_branch_ref(pool: PgPool) { + let owner = "did:key:zPRINJOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let state = test_state(pool).await; + let repo = seed_repo(owner, "pub-inj-repo"); // is_public = true + state.db.create_repo(&repo).await.expect("seed repo"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls", + axum::routing::post(crate::api::pulls::create_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner}/pub-inj-repo/pulls"); + let body = Body::from( + r#"{"title":"x","source_branch":"main","target_branch":"--output=/tmp/gl-should-not-exist-inj"}"#, + ); + + let resp = router + .oneshot(signed_request_as(owner, Method::POST, &uri, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "an option-shaped branch ref must be rejected, got {}", + resp.status() + ); + + // And nothing was stored: the boundary rejects before the write. + let prs = state.db.list_prs(&repo.id).await.expect("list_prs"); + assert!( + prs.is_empty(), + "no PR row must be created when the branch ref is rejected" + ); + } + + /// Branch-name contract (jatmn #379): create_pr must reject symbolic revision + /// names and trailing-dot components that `git check-ref-format --branch` refuses. + #[sqlx::test] + async fn create_pr_rejects_non_branch_ref_names(pool: PgPool) { + let owner = "did:key:zPRBRANCHREFAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let state = test_state(pool).await; + let repo = seed_repo(owner, "branch-ref-repo"); + state.db.create_repo(&repo).await.expect("seed repo"); + + let router = || { + Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls", + axum::routing::post(crate::api::pulls::create_pr), + ) + .with_state(state.clone()) + }; + let uri = format!("/api/v1/repos/{owner}/branch-ref-repo/pulls"); + + for (field, bad) in [ + ("source_branch", "HEAD"), + ("source_branch", "@"), + ("source_branch", "@{-1}"), + ("source_branch", "FETCH_HEAD"), + ("source_branch", "ORIG_HEAD"), + ("source_branch", "MERGE_HEAD"), + ("source_branch", "CHERRY_PICK_HEAD"), + ("source_branch", "RERERE_MERGE_HEAD"), + ("source_branch", "feature."), + ("source_branch", "feature/x."), + ("source_branch", "refs/tags/v1"), + ("source_branch", "heads/main"), + ("source_branch", "tags/v1"), + ("target_branch", "HEAD"), + ("target_branch", "@"), + ("target_branch", "@{-1}"), + ("target_branch", "FETCH_HEAD"), + ("target_branch", "ORIG_HEAD"), + ("target_branch", "feature."), + ("target_branch", "feature/x."), + ("target_branch", "refs/heads/main"), + ("target_branch", "heads/main"), + ("target_branch", "tags/v1"), + ] { + let body_json = if field == "source_branch" { + format!(r#"{{"title":"x","source_branch":"{bad}","target_branch":"main"}}"#) + } else { + format!(r#"{{"title":"x","source_branch":"main","target_branch":"{bad}"}}"#) + }; + let body = Body::from(body_json); + let resp = router() + .oneshot(signed_request_as(owner, Method::POST, &uri, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "{field}={bad:?} must be rejected, got {}", + resp.status() + ); + let prs = state.db.list_prs(&repo.id).await.expect("list_prs"); + assert!(prs.is_empty(), "no PR row when {field}={bad:?} is rejected"); + } + } + + /// Branch-name contract (jatmn #379): create_repo must reject a default_branch + /// that is not a real branch name. + #[sqlx::test] + async fn create_repo_rejects_non_branch_default_branch(pool: PgPool) { + let owner = "did:key:zREPOBRANCHREFAAAAAAAAAAAAAAAAAAAAAAAAA"; + let state = test_state(pool).await; + + let router = || { + Router::new() + .route( + "/api/v1/repos", + axum::routing::post(crate::api::repos::create_repo), + ) + .with_state(state.clone()) + }; + + for (name_suffix, bad) in [ + ("head", "HEAD"), + ("at-shorthand", "@"), + ("reflog-shorthand", "@{-1}"), + ("fetch-head", "FETCH_HEAD"), + ("orig-head", "ORIG_HEAD"), + ("merge-head", "MERGE_HEAD"), + ("cherry-head", "CHERRY_PICK_HEAD"), + ("rerere-head", "RERERE_MERGE_HEAD"), + ("trail-dot", "feature."), + ("trail-dot-comp", "feature/x."), + ("tag-ref", "refs/tags/v1"), + ("qualified-head", "refs/heads/main"), + ("heads-shorthand", "heads/main"), + ("tags-shorthand", "tags/v1"), + ("remotes-shorthand", "remotes/origin/main"), + ] { + let repo_name = format!("bad-def-{name_suffix}"); + let body = Body::from(format!( + r#"{{"name":"{repo_name}","default_branch":"{bad}"}}"# + )); + let resp = router() + .oneshot(signed_request_as( + owner, + Method::POST, + "/api/v1/repos", + body, + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "default_branch={bad:?} must be rejected, got {}", + resp.status() + ); + assert!( + state + .db + .get_repo(owner, &repo_name) + .await + .unwrap() + .is_none(), + "no repo row when default_branch={bad:?} is rejected" + ); + } + } + + /// SINK GUARD: a legacy row with source_branch=HEAD must be rejected before + /// merge_branch runs git. Without the sink guard, git merge HEAD succeeds on + /// the checked-out target and the PR is recorded as merged without merging + /// the submitted source branch. + #[sqlx::test] + async fn legacy_pr_row_with_head_source_is_rejected_at_merge_sink(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let src = std::env::temp_dir().join(format!("gl-head-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("f.txt"), b"hi").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + run(&["branch", "feature"], &src); + std::fs::write(src.join("f.txt"), b"changed").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "on feature"], &src); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("head-probe.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + + let mut repo = seed_repo(&owner_did, "head-probe"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "x".into(), + body: None, + author_did: owner_did.clone(), + source_branch: "HEAD".into(), + target_branch: "main".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert legacy row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/head-probe/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert!( + !resp.status().is_success(), + "merge must not succeed for source_branch=HEAD, got {}", + resp.status() + ); + + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!( + stored.status, "open", + "a rejected merge must not record pull_request.merged" + ); + } + + /// SINK GUARD: a legacy row with source_branch=@ must be rejected before + /// merge_branch runs git. Without the sink guard, git merge @ is a no-op on + /// the checked-out target and the PR is recorded as merged without merging + /// the submitted source branch. + #[sqlx::test] + async fn legacy_pr_row_with_at_shorthand_source_is_rejected_at_merge_sink(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let src = std::env::temp_dir().join(format!("gl-at-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("f.txt"), b"hi").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + run(&["branch", "feature"], &src); + std::fs::write(src.join("f.txt"), b"changed").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "on feature"], &src); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("at-probe.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + + let mut repo = seed_repo(&owner_did, "at-probe"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "x".into(), + body: None, + author_did: owner_did.clone(), + source_branch: "@".into(), + target_branch: "main".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert legacy row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/at-probe/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert!( + !resp.status().is_success(), + "merge must not succeed for source_branch=@, got {}", + resp.status() + ); + + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!( + stored.status, "open", + "a rejected merge must not record pull_request.merged" + ); + } + + /// SINK GUARD: legacy rows with reflog shorthands or pseudoref source branches + /// must be rejected at merge before git runs. + #[sqlx::test] + async fn legacy_pr_rows_with_shorthand_or_pseudoref_source_rejected_at_merge_sink( + pool: PgPool, + ) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + for (label, bad_source) in [ + ("reflog", "@{-1}"), + ("fetch-head", "FETCH_HEAD"), + ("merge-head", "MERGE_HEAD"), + ] { + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool.clone()).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let src = std::env::temp_dir().join(format!("gl-{label}-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("f.txt"), b"hi").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + run(&["branch", "feature"], &src); + std::fs::write(src.join("f.txt"), b"changed").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "on feature"], &src); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join(format!("{label}-probe.git")); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + + let repo_name = format!("{label}-probe"); + let mut repo = seed_repo(&owner_did, &repo_name); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "x".into(), + body: None, + author_did: owner_did.clone(), + source_branch: bad_source.into(), + target_branch: "main".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert legacy row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/{repo_name}/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::INTERNAL_SERVER_ERROR, + "merge must return git_error for source_branch={bad_source:?}, got {}", + resp.status() + ); + + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!( + stored.status, "open", + "merge must not record pull_request.merged for source_branch={bad_source:?}" + ); + } + } + + /// SINK GUARD: legacy rows with reflog/pseudoref refs must be rejected at diff + /// before git argv is built. + #[sqlx::test] + async fn legacy_pr_rows_with_shorthand_or_pseudoref_rejected_at_diff_sink(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + for (label, bad_source) in [("reflog", "@{-1}"), ("fetch-head", "FETCH_HEAD")] { + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool.clone()).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let src = std::env::temp_dir().join(format!("gl-diff-{label}-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("f.txt"), b"hi").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join(format!("diff-{label}.git")); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + + let repo_name = format!("diff-{label}"); + let mut repo = seed_repo(&owner_did, &repo_name); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "x".into(), + body: None, + author_did: owner_did.clone(), + source_branch: bad_source.into(), + target_branch: "main".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert legacy row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/diff", + axum::routing::get(crate::api::pulls::get_pr_diff), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/{repo_name}/pulls/1/diff"); + let resp = router.oneshot(anon_get(&uri)).await.unwrap(); + assert_eq!( + resp.status(), + StatusCode::INTERNAL_SERVER_ERROR, + "get_pr_diff must return git_error for source_branch={bad_source:?}, got {}", + resp.status() + ); + } + } + + /// When git cannot be spawned for check-ref-format, storage boundaries must + /// return git_error (500), not bad_request (400). + #[sqlx::test] + async fn create_pr_returns_git_error_when_validate_git_ref_cannot_spawn_git(pool: PgPool) { + let owner = "did:key:zPRGITSPAWNAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let mut state = test_state(pool).await; + state.git_bin = "/nonexistent/gitlawb-validate-git".into(); + let repo = seed_repo(owner, "git-spawn-pr"); + state.db.create_repo(&repo).await.expect("seed repo"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls", + axum::routing::post(crate::api::pulls::create_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner}/git-spawn-pr/pulls"); + let body = Body::from(r#"{"title":"x","source_branch":"main","target_branch":"main"}"#); + let resp = router + .oneshot(signed_request_as(owner, Method::POST, &uri, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::INTERNAL_SERVER_ERROR, + "spawn failure must not surface as 400, got {}", + resp.status() + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .expect("read body"); + let v: serde_json::Value = serde_json::from_slice(&bytes).expect("json body"); + assert_eq!( + v.get("error").and_then(|e| e.as_str()), + Some("git_error"), + "spawn failure must use git_error code, got {v}" + ); + let prs = state.db.list_prs(&repo.id).await.expect("list_prs"); + assert!(prs.is_empty(), "no PR row when git is unavailable"); + } + + #[sqlx::test] + async fn create_repo_returns_git_error_when_validate_git_ref_cannot_spawn_git(pool: PgPool) { + let mut state = test_state(pool).await; + state.git_bin = "/nonexistent/gitlawb-validate-git".into(); + let owner = "did:key:zREPOGITSPAWNAAAAAAAAAAAAAAAAAAAAAAAAA"; + + let router = Router::new() + .route( + "/api/v1/repos", + axum::routing::post(crate::api::repos::create_repo), + ) + .with_state(state.clone()); + let body = Body::from(r#"{"name":"git-spawn-repo","default_branch":"main"}"#); + let resp = router + .oneshot(signed_request_as( + owner, + Method::POST, + "/api/v1/repos", + body, + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::INTERNAL_SERVER_ERROR, + "spawn failure must not surface as 400, got {}", + resp.status() + ); + let bytes = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .expect("read body"); + let v: serde_json::Value = serde_json::from_slice(&bytes).expect("json body"); + assert_eq!( + v.get("error").and_then(|e| e.as_str()), + Some("git_error"), + "spawn failure must use git_error code, got {v}" + ); + assert!( + state + .db + .get_repo(owner, "git-spawn-repo") + .await + .unwrap() + .is_none(), + "no repo row when git is unavailable" + ); + } + + /// SINK GUARD: legacy target_branch=HEAD must be rejected before worktree add. + #[sqlx::test] + async fn legacy_pr_row_with_head_target_is_rejected_at_merge_sink(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let src = std::env::temp_dir().join(format!("gl-head-tgt-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("f.txt"), b"hi").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + run(&["branch", "feature"], &src); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("head-tgt-probe.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + + let mut repo = seed_repo(&owner_did, "head-tgt-probe"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "x".into(), + body: None, + author_did: owner_did.clone(), + source_branch: "feature".into(), + target_branch: "HEAD".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert legacy row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/head-tgt-probe/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert!( + !resp.status().is_success(), + "merge must not succeed for target_branch=HEAD, got {}", + resp.status() + ); + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!(stored.status, "open"); + } + + /// Successful merge path (jatmn #379): diverged branches must produce a merge + /// commit on refs/heads/{target}, not only on a detached worktree HEAD. + #[sqlx::test] + async fn merge_pr_advances_target_ref_with_diverged_branches(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + out + }; + let rev = |args: &[&str], cwd: &std::path::Path| { + String::from_utf8_lossy(&run(args, cwd).stdout) + .trim() + .to_string() + }; + + let src = std::env::temp_dir().join(format!("gl-merge-ok-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("base.txt"), b"base").unwrap(); + run(&["add", "base.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + run(&["checkout", "-b", "feature"], &src); + std::fs::write(src.join("feature.txt"), b"feature-only").unwrap(); + run(&["add", "feature.txt"], &src); + run(&["commit", "-q", "-m", "on feature"], &src); + run(&["checkout", "main"], &src); + std::fs::write(src.join("main.txt"), b"main-only").unwrap(); + run(&["add", "main.txt"], &src); + run(&["commit", "-q", "-m", "on main"], &src); + let feature_tip = rev(&["rev-parse", "feature"], &src); + + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("merge-success.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + let main_before = rev(&["rev-parse", "refs/heads/main"], &bare); + + let mut repo = seed_repo(&owner_did, "merge-success"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "merge feature".into(), + body: None, + author_did: owner_did.clone(), + source_branch: "feature".into(), + target_branch: "main".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert pr row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/merge-success/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "merge must succeed for diverged branches, got {}", + resp.status() + ); + + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).expect("json body"); + let merge_sha = json["merge_sha"].as_str().expect("merge_sha in response"); + assert_eq!(json["status"].as_str(), Some("merged")); + + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!(stored.status, "merged"); + assert!(stored.merged_by_did.is_some()); + + let main_after = rev(&["rev-parse", "refs/heads/main"], &bare); + assert_ne!( + main_after, main_before, + "refs/heads/main must advance after a successful merge" + ); + assert_eq!( + main_after, merge_sha, + "reported merge_sha must match refs/heads/main" + ); + run( + &["merge-base", "--is-ancestor", &feature_tip, &main_after], + &bare, + ); + run( + &["cat-file", "-e", &format!("{main_after}:feature.txt")], + &bare, + ); + } + + /// SINK GUARD (jatmn #379): a PR targeting a name that exists only as a tag + /// (refs/tags/{name}, no refs/heads/{name}) must fail closed. Without the + /// exact-ref checks, git's revision DWIM resolves the bare name to the tag, + /// `worktree add` checks out a detached HEAD there, the merge succeeds on + /// that disposable HEAD, cleanup discards the commit, and the unchecked + /// final rev-parse echoes "refs/heads/{name}" back as a bogus merge SHA — + /// so the PR is marked merged and pull_request.merged fires even though no + /// branch ever moved. + #[sqlx::test] + async fn merge_pr_fails_closed_when_target_is_only_a_tag(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + out + }; + let rev = |args: &[&str], cwd: &std::path::Path| { + String::from_utf8_lossy(&run(args, cwd).stdout) + .trim() + .to_string() + }; + + let src = std::env::temp_dir().join(format!("gl-tag-only-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("base.txt"), b"base").unwrap(); + run(&["add", "base.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + // The tag that shadows the missing target branch: "release" resolves via + // DWIM but refs/heads/release never exists. + run(&["tag", "release"], &src); + run(&["checkout", "-q", "-b", "feature"], &src); + std::fs::write(src.join("feature.txt"), b"feature-only").unwrap(); + run(&["add", "feature.txt"], &src); + run(&["commit", "-q", "-m", "on feature"], &src); + + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("tag-only.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + let tag_before = rev(&["rev-parse", "refs/tags/release"], &bare); + // Precondition of the whole scenario: the tag exists, the branch does not. + let no_branch = Command::new("git") + .args(["show-ref", "--verify", "--quiet", "refs/heads/release"]) + .current_dir(&bare) + .output() + .expect("git show-ref"); + assert!( + !no_branch.status.success(), + "fixture must not have a local release branch" + ); + + let mut repo = seed_repo(&owner_did, "tag-only"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "merge into tag-shadowed name".into(), + body: None, + author_did: owner_did.clone(), + source_branch: "feature".into(), + target_branch: "release".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert pr row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/tag-only/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert!( + !resp.status().is_success(), + "merge must fail when the target exists only as a tag, got {}", + resp.status() + ); + + // The handler updates the PR row and fires pull_request.merged only + // after merge_branch returns Ok, so an open row proves no merged event + // was emitted. + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!( + stored.status, "open", + "a failed merge must leave the PR open and emit no pull_request.merged" + ); + assert!(stored.merged_by_did.is_none()); + + // The repo itself must be untouched: still no local release branch, and + // the tag did not move. + let still_no_branch = Command::new("git") + .args(["show-ref", "--verify", "--quiet", "refs/heads/release"]) + .current_dir(&bare) + .output() + .expect("git show-ref"); + assert!( + !still_no_branch.status.success(), + "a failed merge must not create refs/heads/release" + ); + let tag_after = rev(&["rev-parse", "refs/tags/release"], &bare); + assert_eq!(tag_after, tag_before, "the tag must not move"); + } + + /// Companion to the tag-only case (jatmn #379): when a same-named tag AND a + /// local branch coexist, the merge must attach to and advance + /// refs/heads/{target} while refs/tags/{target} stays where it was — the + /// exact-ref binding must pick the branch, not merely fail less. + #[sqlx::test] + async fn merge_pr_advances_local_branch_shadowed_by_same_named_tag(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + out + }; + let rev = |args: &[&str], cwd: &std::path::Path| { + String::from_utf8_lossy(&run(args, cwd).stdout) + .trim() + .to_string() + }; + + let src = std::env::temp_dir().join(format!("gl-tag-shadow-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("base.txt"), b"base").unwrap(); + run(&["add", "base.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + // Tag "release" pinned at the seed commit; the branch of the same name + // moves past it. Revision DWIM prefers refs/tags/ over refs/heads/ for + // an ambiguous short name, so this fixture is the strongest shadowing + // shape: a naive resolution would merge onto the stale tag commit. + run(&["tag", "release"], &src); + run(&["checkout", "-q", "-b", "release"], &src); + std::fs::write(src.join("release.txt"), b"release-only").unwrap(); + run(&["add", "release.txt"], &src); + run(&["commit", "-q", "-m", "on release"], &src); + run(&["checkout", "-q", "-b", "feature"], &src); + std::fs::write(src.join("feature.txt"), b"feature-only").unwrap(); + run(&["add", "feature.txt"], &src); + run(&["commit", "-q", "-m", "on feature"], &src); + run(&["checkout", "-q", "release"], &src); + std::fs::write(src.join("release2.txt"), b"release-again").unwrap(); + run(&["add", "release2.txt"], &src); + run(&["commit", "-q", "-m", "diverge release"], &src); + let feature_tip = rev(&["rev-parse", "refs/heads/feature"], &src); + + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("tag-shadow.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + let tag_before = rev(&["rev-parse", "refs/tags/release"], &bare); + let branch_before = rev(&["rev-parse", "refs/heads/release"], &bare); + assert_ne!( + tag_before, branch_before, + "fixture must diverge the tag from the branch" + ); + + let mut repo = seed_repo(&owner_did, "tag-shadow"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "merge feature into shadowed release".into(), + body: None, + author_did: owner_did.clone(), + source_branch: "feature".into(), + target_branch: "release".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert pr row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/tag-shadow/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::OK, + "merge must succeed when the local branch exists alongside the tag, got {}", + resp.status() + ); + + let body = axum::body::to_bytes(resp.into_body(), usize::MAX) + .await + .unwrap(); + let json: serde_json::Value = serde_json::from_slice(&body).expect("json body"); + let merge_sha = json["merge_sha"].as_str().expect("merge_sha in response"); + assert_eq!(json["status"].as_str(), Some("merged")); + + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!(stored.status, "merged"); + + // The LOCAL BRANCH advanced to the reported merge SHA; the tag stayed put. + let branch_after = rev(&["rev-parse", "refs/heads/release"], &bare); + assert_ne!( + branch_after, branch_before, + "refs/heads/release must advance past the merge" + ); + assert_eq!( + branch_after, merge_sha, + "reported merge_sha must match refs/heads/release" + ); + let tag_after = rev(&["rev-parse", "refs/tags/release"], &bare); + assert_eq!(tag_after, tag_before, "the tag must not move"); + run( + &["merge-base", "--is-ancestor", &feature_tip, &branch_after], + &bare, + ); + run( + &["cat-file", "-e", &format!("{branch_after}:feature.txt")], + &bare, + ); + } + + /// SINK GUARD: a poisoned row with option-shaped source_branch must not reach + /// git merge argv (distinct from the diff revspec injection shape). + #[sqlx::test] + async fn poisoned_pr_row_option_source_is_rejected_at_merge_sink(pool: PgPool) { + use gitlawb_core::identity::Keypair; + use std::process::Command; + struct DirGuard(std::path::PathBuf); + impl Drop for DirGuard { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + let kp = Keypair::generate(); + let owner_did = kp.did().to_string(); + let short = owner_did.split(':').next_back().unwrap().to_string(); + let slug = owner_did.replace([':', '/'], "_"); + let state = test_state(pool).await; + let run = |args: &[&str], cwd: &std::path::Path| { + let out = Command::new("git") + .args(args) + .current_dir(cwd) + .output() + .expect("git"); + assert!( + out.status.success(), + "git {args:?}: {}", + String::from_utf8_lossy(&out.stderr) + ); + }; + let src = std::env::temp_dir().join(format!("gl-merge-src-{short}")); + let _ = std::fs::remove_dir_all(&src); + std::fs::create_dir_all(&src).unwrap(); + let _sg = DirGuard(src.clone()); + run(&["init", "-q", "-b", "main"], &src); + run(&["config", "user.email", "t@t"], &src); + run(&["config", "user.name", "t"], &src); + std::fs::write(src.join("f.txt"), b"hi").unwrap(); + run(&["add", "f.txt"], &src); + run(&["commit", "-q", "-m", "seed"], &src); + let bare = std::path::PathBuf::from("/tmp") + .join(&slug) + .join("merge-probe.git"); + let _ = std::fs::remove_dir_all(&bare); + std::fs::create_dir_all(bare.parent().unwrap()).unwrap(); + let _bg = DirGuard(bare.clone()); + run( + &[ + "clone", + "--bare", + "-q", + src.to_str().unwrap(), + bare.to_str().unwrap(), + ], + &std::env::temp_dir(), + ); + + let mut repo = seed_repo(&owner_did, "merge-probe"); + repo.is_public = true; + state.db.create_repo(&repo).await.expect("seed repo"); + + let pwn = std::env::temp_dir().join(format!("gl-MERGE-{short}")); + let _ = std::fs::remove_file(&pwn); + + let pr = crate::db::PullRequest { + id: uuid::Uuid::new_v4().to_string(), + repo_id: repo.id.clone(), + number: 1, + title: "x".into(), + body: None, + author_did: owner_did.clone(), + source_branch: format!("--output={}", pwn.to_str().unwrap()), + target_branch: "main".into(), + status: "open".into(), + merged_by_did: None, + merged_at: None, + created_at: chrono::Utc::now().to_rfc3339(), + updated_at: chrono::Utc::now().to_rfc3339(), + }; + state.db.create_pr(&pr).await.expect("insert poisoned row"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls/{number}/merge", + axum::routing::post(crate::api::pulls::merge_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner_did}/merge-probe/pulls/1/merge"); + let resp = router + .oneshot(signed_request_as( + &owner_did, + Method::POST, + &uri, + Body::empty(), + )) + .await + .unwrap(); + assert!( + !resp.status().is_success(), + "merge must not succeed for option-shaped source_branch, got {}", + resp.status() + ); + assert!( + !pwn.exists(), + "merge sink must not write via option-shaped source_branch" + ); + let stored = state + .db + .get_pr(&repo.id, 1) + .await + .expect("get_pr") + .expect("pr row"); + assert_eq!(stored.status, "open"); + } + + /// create_pr must validate the resolved target_branch even when the caller + /// omits target_branch and the repo row carries a poisoned default_branch. + #[sqlx::test] + async fn create_pr_rejects_resolved_target_from_poisoned_default_branch(pool: PgPool) { + let owner = "did:key:zPRDEFPOISONAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"; + let state = test_state(pool).await; + let repo = seed_repo(owner, "def-poison-repo"); + state.db.create_repo(&repo).await.expect("seed repo"); + sqlx::query("UPDATE repos SET default_branch = $1 WHERE id = $2") + .bind("HEAD") + .bind(&repo.id) + .execute(state.db.pool()) + .await + .expect("poison default_branch"); + + let router = Router::new() + .route( + "/api/v1/repos/{owner}/{repo}/pulls", + axum::routing::post(crate::api::pulls::create_pr), + ) + .with_state(state.clone()); + let uri = format!("/api/v1/repos/{owner}/def-poison-repo/pulls"); + let body = Body::from(r#"{"title":"x","source_branch":"feature"}"#); + let resp = router + .oneshot(signed_request_as(owner, Method::POST, &uri, body)) + .await + .unwrap(); + assert_eq!( + resp.status(), + StatusCode::BAD_REQUEST, + "resolved target from poisoned default_branch must be rejected, got {}", + resp.status() + ); + let prs = state.db.list_prs(&repo.id).await.expect("list_prs"); + assert!( + prs.is_empty(), + "no PR row when resolved target_branch is invalid" + ); + } + /// Adversarial-review GATE-2 (create_issue): filing an issue requires read /// access. A non-reader is denied on a private repo before any git work. #[sqlx::test]