Skip to content

Commit 4eb0c44

Browse files
committed
fix(node): route fork disk paths through validated_repo_disk_path
CodeQL flagged remove_dir_all on a user-derived path in fork_repo. Use the same three-layer barrier as RepoStore::local_path and reject names the validator refuses before clone.
1 parent 4f1c1bb commit 4eb0c44

3 files changed

Lines changed: 36 additions & 3 deletions

File tree

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3146,7 +3146,12 @@ pub async fn fork_repo(
31463146
.await
31473147
.map_err(|e| AppError::Git(e.to_string()))?;
31483148

3149-
let disk_path = store::repo_disk_path(&state.config.repos_dir, &forker_did, &fork_name);
3149+
let disk_path = crate::git::repo_store::validated_repo_disk_path(
3150+
&state.config.repos_dir,
3151+
&forker_did,
3152+
&fork_name,
3153+
)
3154+
.map_err(|e| AppError::BadRequest(e.to_string()))?;
31503155

31513156
// Clone the source repo as a mirror
31523157
let output = std::process::Command::new("git")

‎crates/gitlawb-node/src/git/repo_store.rs‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -710,8 +710,7 @@ pub(crate) fn validated_repo_disk_path(
710710
) -> Result<PathBuf> {
711711
validate_path_components(owner_did, repo_name)?;
712712

713-
let owner_slug = owner_did.replace([':', '/'], "_");
714-
let local_path = repos_dir.join(&owner_slug).join(format!("{repo_name}.git"));
713+
let local_path = store::repo_disk_path(repos_dir, owner_did, repo_name);
715714

716715
if !local_path.starts_with(repos_dir) {
717716
anyhow::bail!(

‎crates/gitlawb-node/src/test_support.rs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -557,6 +557,35 @@ mod tests {
557557
);
558558
}
559559

560+
/// Fork disk paths must go through `validated_repo_disk_path` so a user-supplied
561+
/// name cannot reach `remove_dir_all` on an escaped path (CodeQL path-injection).
562+
#[sqlx::test]
563+
async fn fork_rejects_name_that_fails_validated_disk_path(pool: PgPool) {
564+
let owner = "did:key:zFORKOWNERAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
565+
let state = test_state(pool).await;
566+
let repo = seed_repo(owner, "fork-src");
567+
state.db.create_repo(&repo).await.expect("seed repo");
568+
569+
let router = Router::new()
570+
.route(
571+
"/api/v1/repos/{owner}/{repo}/fork",
572+
axum::routing::post(crate::api::repos::fork_repo),
573+
)
574+
.with_state(state.clone());
575+
let too_long = "a".repeat(101);
576+
let uri = format!("/api/v1/repos/{owner}/fork-src/fork");
577+
let body = Body::from(format!(r#"{{"name":"{too_long}"}}"#));
578+
let resp = router
579+
.oneshot(signed_request_as(owner, Method::POST, &uri, body))
580+
.await
581+
.unwrap();
582+
assert_eq!(
583+
resp.status(),
584+
StatusCode::BAD_REQUEST,
585+
"fork names that fail validated_repo_disk_path must be rejected before clone"
586+
);
587+
}
588+
560589
/// N13: the task handlers bind the acting DID to the signer. A caller signed
561590
/// as B claiming delegator_did A is rejected before any DB write (DB-free).
562591
#[sqlx::test]

0 commit comments

Comments
 (0)