fix(node): validate the fork name before spending the proof

The barrier landed after `proof.consume` and after `repo_store.acquire`, so a
name the character allowlist admits but `validate_repo_name` refuses burned a
valid iCaptcha proof and paid a source-repo download before its 400. The
handler's own header comment promises the opposite: a fork rejected for a bad
name never burns a proof. Reported by CodeRabbit on the review of a7257dc.

It now runs with the other admissibility checks, above both.

The regression test pins the ORDER without iCaptcha plumbing. Seed a repo row
whose bytes are not on disk: late validation lets the acquire fail first and
the caller sees a 500 from git, so only early validation can produce the 400
the test asserts. Verified load-bearing by degrading the early call to
`unwrap_or_else`, which reddens it.

Both name rules are kept. CodeRabbit also suggested deleting the character
allowlist as a strict subset of `validate_repo_name`, and that is not the
relation between them: the allowlist rejects a dot, so `v1.2.3` and `my.repo`
are refused today while `validate_repo_name` accepts both, and the allowlist
accepts a non-ASCII alphanumeric that `validate_repo_name` rejects. Removing
it would newly admit dotted fork names, which is a behaviour change and not a
cleanup. A comment now records why the two coexist.

The empty-name fixture also generates its owner DID per run. It writes to a
fixed `/tmp/<owner_slug>`, so a shared constant let two concurrent `cargo
test` processes delete each other's repo mid-test (reported by Greptile).

Author: beardthelion

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

@@ -3046,6 +3046,25 @@ pub async fn fork_repo(
         ));
     }
 
+    // Same barrier every other repo-creation route goes through. The character
+    // allowlist above is vacuously true on an empty name, and `repo_disk_path`
+    // is a raw join, so without this a `{"name":""}` fork lands a row with an
+    // empty name at `<repos_dir>/<owner_slug>/.git`. Closed #272 fixed this
+    // class on the sync route and never scoped fork.
+    //
+    // It runs with the other admissibility checks, above the proof spend and the
+    // source acquire, because the header comment promises that a fork rejected
+    // for a bad name never burns a valid proof. The two name rules are kept
+    // separate on purpose: neither is a subset of the other, since the allowlist
+    // above rejects a dot (`v1.2.3`) that `validate_repo_name` accepts, and
+    // accepts a non-ASCII alphanumeric that it rejects.
+    let disk_path = crate::git::repo_store::validated_repo_disk_path(
+        &state.config.repos_dir,
+        &forker_did,
+        &fork_name,
+    )
+    .map_err(|e| AppError::BadRequest(e.to_string()))?;
+
     // Check no name conflict under the forker's ownership
     let forker_short = crate::db::normalize_owner_key(&forker_did);
     if state.db.get_repo(forker_short, &fork_name).await?.is_some() {
@@ -3064,18 +3083,6 @@ pub async fn fork_repo(
         .await
         .map_err(|e| AppError::Git(e.to_string()))?;
 
-    // Same barrier every other repo-creation route goes through. The character
-    // allowlist above is vacuously true on an empty name, and `repo_disk_path`
-    // is a raw join, so without this a `{"name":""}` fork lands a row with an
-    // empty name at `<repos_dir>/<owner_slug>/.git`. Closed #272 fixed this
-    // class on the sync route and never scoped fork.
-    let disk_path = crate::git::repo_store::validated_repo_disk_path(
-        &state.config.repos_dir,
-        &forker_did,
-        &fork_name,
-    )
-    .map_err(|e| AppError::BadRequest(e.to_string()))?;
-
     // Clone the source repo as a mirror
     let output = std::process::Command::new("git")
         .args([

crates/gitlawb-node/src/test_support.rs

@@ -573,7 +573,17 @@ mod tests {
         // `repo_store::for_testing` pins its on-disk root to /tmp, so the config
         // the handler reads at the raw join must name the same root or the two
         // halves of this test look at different directories.
-        let owner = "did:key:zFORKEMPTYNAMEAAAAAAAAAAAAAAAAAAAAAAAAAA";
+        // A per-run owner: the fixture path is a fixed /tmp/<slug>, so a shared
+        // constant lets two concurrent `cargo test` PROCESSES delete each
+        // other's repo mid-test.
+        let owner = format!(
+            "did:key:zFORKEMPTY{}",
+            gitlawb_core::identity::Keypair::generate()
+                .did()
+                .to_string()
+                .replace("did:key:", "")
+        );
+        let owner = owner.as_str();
         let owner_slug = owner.replace([':', '/'], "_");
         let repos_dir = std::path::PathBuf::from("/tmp");
         let owner_dir = repos_dir.join(&owner_slug);
@@ -774,6 +784,56 @@ mod tests {
         );
     }
 
+    /// The name barrier must run BEFORE the proof spend and the source acquire.
+    /// `fork_repo`'s header comment promises that a fork rejected for a bad name
+    /// never burns a valid proof, and the first version of this fix validated
+    /// after both, so an empty name paid a Tigris download and spent the proof
+    /// before its 400.
+    ///
+    /// The seam that pins the order without iCaptcha plumbing: seed a repo ROW
+    /// whose bytes are not on disk. If validation still ran after the acquire,
+    /// the acquire fails first and the caller sees a 500 from git. A 400 proves
+    /// the refusal came first.
+    #[sqlx::test]
+    async fn fork_refuses_a_bad_name_before_acquiring_the_source(pool: PgPool) {
+        let owner = "did:key:zFORKORDERAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+        let repos_dir = std::path::PathBuf::from("/tmp");
+        let state = test_state_with(pool, |cfg| cfg.repos_dir = repos_dir.clone()).await;
+
+        // Row only. Nothing is written to disk, so `repo_store.acquire` cannot
+        // succeed and any 500 here means validation ran too late.
+        let repo = seed_repo(owner, "absent-source");
+        state.db.create_repo(&repo).await.expect("seed source row");
+
+        let router = Router::new()
+            .route(
+                "/api/v1/repos/{owner}/{repo}/fork",
+                axum::routing::post(crate::api::repos::fork_repo),
+            )
+            .with_state(state.clone());
+        let uri = format!("/api/v1/repos/{owner}/absent-source/fork");
+        let resp = router
+            .oneshot(signed_request_as(
+                owner,
+                Method::POST,
+                &uri,
+                Body::from(r#"{"name":""}"#),
+            ))
+            .await
+            .unwrap();
+
+        let status = resp.status();
+        let body = axum::body::to_bytes(resp.into_body(), usize::MAX)
+            .await
+            .expect("read body");
+        let body = String::from_utf8_lossy(&body);
+        assert_eq!(
+            status,
+            StatusCode::BAD_REQUEST,
+            "a bad fork name must be refused before the source acquire, not after; body={body}"
+        );
+    }
+
     /// Fixed-path temp cleanup that survives a panic (the suite's convention;
     /// `TempDir` cannot own a path this test must compute up front).
     struct OwnerDirGuard(std::path::PathBuf);