fix(node): reject revision shorthands and split ref-validation errors

git check-ref-format accepts @ and pseudoref names that git later treats as
symbolic revisions, so merge could succeed without merging the source branch.
Reject those explicitly at the shared validator and add sink/boundary tests.

Spawn failures from check-ref-format now surface as git_error (500) instead of
bad_request (400) at create_pr/create_repo boundaries.

Author: beardthelion

crates/gitlawb-node/src/api/mod.rs

@@ -98,6 +98,17 @@ pub(crate) fn require_repo_owner(record: &RepoRecord, caller: &str) -> Result<()
 /// caller. Its unit tests live beside the definition in `git/store.rs`.
 pub(crate) use crate::git::store::validate_git_ref;
 
+/// 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;

crates/gitlawb-node/src/api/pulls.rs

@@ -57,8 +57,10 @@ pub async fn create_pr(
     // 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(&req.source_branch).map_err(AppError::BadRequest)?;
-    crate::api::validate_git_ref(&target_branch).map_err(AppError::BadRequest)?;
+    crate::api::validate_git_ref(&req.source_branch)
+        .map_err(crate::api::map_git_ref_validation_error)?;
+    crate::api::validate_git_ref(&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();
 

crates/gitlawb-node/src/api/repos.rs

@@ -245,7 +245,8 @@ 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(&req.default_branch).map_err(AppError::BadRequest)?;
+    crate::api::validate_git_ref(&req.default_branch)
+        .map_err(crate::api::map_git_ref_validation_error)?;
 
     // Owner is the authenticated agent's DID
     let owner_did = auth.0;

crates/gitlawb-node/src/git/store.rs

@@ -746,6 +746,46 @@ pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result<Option<(String,
     Ok(Some((obj_type, content)))
 }
 
+/// Outcome of validating a stored branch-field value.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum GitRefValidationError {
+    /// Caller supplied a name that is not a valid local branch name.
+    Invalid(String),
+    /// Git could not be executed to validate the name.
+    GitUnavailable(String),
+}
+
+impl std::fmt::Display for GitRefValidationError {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> 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",
+    ];
+    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 interpolated into a git
 /// argv element. Delegates to `git check-ref-format --branch`, then rejects
 /// fully qualified ref paths (`refs/heads/...`, `refs/tags/...`) that git would
@@ -754,6 +794,9 @@ pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result<Option<(String,
 ///
 /// Symbolic revision names (`HEAD`), option-shaped names (leading `-`), 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 at the
+/// sink, not as literal branch names.
 ///
 /// A `--` delimiter is not the fix at the sink: these arguments are revisions,
 /// and `--` there reinterprets them as pathspecs.
@@ -762,34 +805,59 @@ pub fn read_object(repo_path: &Path, sha256_hex: &str) -> Result<Option<(String,
 /// call it too via the `crate::api` re-export to fail fast with a 400, but the
 /// guard here is what makes the property hold 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<(), String> {
+pub fn validate_git_ref(name: &str) -> std::result::Result<(), GitRefValidationError> {
+    validate_git_ref_with_git("git", name)
+}
+
+fn validate_git_ref_with_git(
+    git_bin: &str,
+    name: &str,
+) -> std::result::Result<(), GitRefValidationError> {
     if name.is_empty() {
-        return Err("branch ref must not be empty".into());
+        return Err(GitRefValidationError::Invalid(
+            "branch ref must not be empty".into(),
+        ));
     }
     if name.starts_with("refs/") {
-        return Err("branch ref must be a local branch name, not a fully qualified ref".into());
+        return Err(GitRefValidationError::Invalid(
+            "branch ref must be a local branch name, not a fully qualified ref".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")
+    let output = Command::new(git_bin)
         .args(["check-ref-format", "--branch", name])
         .output()
-        .map_err(|e| format!("failed to run git check-ref-format: {e}"))?;
+        .map_err(|e| {
+            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(stderr.trim().to_string())
+        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| anyhow::anyhow!("invalid target branch ref: {e}"))?;
-    validate_git_ref(source_branch)
-        .map_err(|e| anyhow::anyhow!("invalid source branch ref: {e}"))?;
+    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(())
 }
 
@@ -928,7 +996,7 @@ pub fn repo_disk_path(repos_dir: &Path, owner_did: &str, repo_name: &str) -> Pat
 
 #[cfg(test)]
 mod tests {
-    use super::validate_git_ref;
+    use super::{validate_git_ref, validate_git_ref_with_git, GitRefValidationError};
 
     #[test]
     fn validate_git_ref_accepts_normal_branch_names() {
@@ -938,6 +1006,8 @@ mod tests {
             "release-1.2",
             "v1.0.0",
             "user/fix-bug",
+            "feature@",
+            "user@host",
         ] {
             assert!(
                 validate_git_ref(good).is_ok(),
@@ -951,6 +1021,10 @@ mod tests {
         for bad in [
             "",
             "HEAD",
+            "@",
+            "@{-1}",
+            "FETCH_HEAD",
+            "MERGE_HEAD",
             "feature.",
             "feature/x.",
             "refs/heads/main",
@@ -967,10 +1041,24 @@ mod tests {
             "trailing/",
             "a//b",
         ] {
-            assert!(validate_git_ref(bad).is_err(), "{bad:?} should be rejected");
+            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;

crates/gitlawb-node/src/test_support.rs

@@ -1040,10 +1040,15 @@ mod tests {
 
         for (field, bad) in [
             ("source_branch", "HEAD"),
+            ("source_branch", "@"),
+            ("source_branch", "@{-1}"),
+            ("source_branch", "FETCH_HEAD"),
             ("source_branch", "feature."),
             ("source_branch", "feature/x."),
             ("source_branch", "refs/tags/v1"),
             ("target_branch", "HEAD"),
+            ("target_branch", "@"),
+            ("target_branch", "@{-1}"),
             ("target_branch", "feature."),
             ("target_branch", "feature/x."),
             ("target_branch", "refs/heads/main"),
@@ -1087,6 +1092,9 @@ mod tests {
 
         for (name_suffix, bad) in [
             ("head", "HEAD"),
+            ("at-shorthand", "@"),
+            ("reflog-shorthand", "@{-1}"),
+            ("fetch-head", "FETCH_HEAD"),
             ("trail-dot", "feature."),
             ("trail-dot-comp", "feature/x."),
             ("tag-ref", "refs/tags/v1"),
@@ -1240,6 +1248,124 @@ mod tests {
         );
     }
 
+    /// 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 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) {