feat: guard nested repository creation and tighten push detection
commit
15e742dfeat: guard nested repository creation and tighten push detection
Repository auto-creation now refuses paths that collide with an existing repository (one nested inside a repo, or a name already used as a namespace) and serializes creation so two concurrent first pushes cannot race. Push detection matches the receive-pack service parameter exactly rather than by substring, so a read or an unrelated query is no longer mistaken for a push.
feat: reject pushes that collide with an existing repository as 409 fix: serialize repository creation behind a lock to close a first-push race fix: detect pushes by exact service parameter instead of substring match test: add rstest table-driven coverage for path parsing, nesting, collisions Assisted-by: Claude:claude-opus-4-8
Reviews
No reviews of this commit yet — record a verdict below.
Start a review
Cargo.lock
@@ -2,6 +2,15 @@
# It is not intended for manual editing.
version = 4
+[[package]]
+name = "aho-corasick"
+version = "1.1.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301"
+dependencies = [
+ "memchr",
+]
+
[[package]]
name = "anstream"
version = "1.0.0"
@@ -184,6 +193,12 @@
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570"
+[[package]]
+name = "equivalent"
+version = "1.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "877a4ace8713b0bcf2a4e7eec82529c029f1d0619886d18145fea96c3ffe5c0f"
+
[[package]]
name = "errno"
version = "0.3.14"
@@ -224,12 +239,29 @@
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7e3450815272ef58cec6d564423f6e755e25379b217b0bc688e295ba24df6b1d"
+[[package]]
+name = "futures-macro"
+version = "0.3.32"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e835b70203e41293343137df5c0664546da5745f82ec9b84d40be8336958447b"
+dependencies = [
+ "proc-macro2",
+ "quote",
+ "syn",
+]
+
[[package]]
name = "futures-task"
version = "0.3.32"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "037711b3d59c33004d3856fbdc83b99d4ff37a24768fa1be9ce3538a1cde4393"
+[[package]]
+name = "futures-timer"
+version = "3.0.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "af43fadb8a98512d547e37b4e92e0ced13e205c061b87b4623eff01d918d6968"
+
[[package]]
name = "futures-util"
version = "0.3.32"
@@ -237,6 +269,7 @@
checksum = "389ca41296e6190b48053de0321d02a77f32f8a5d2461dd38762c0593805c6d6"
dependencies = [
"futures-core",
+ "futures-macro",
"futures-task",
"pin-project-lite",
"slab",
@@ -264,10 +297,23 @@
"axum",
"clap",
"clap_mangen",
+ "rstest",
"tempfile",
"tokio",
]
+[[package]]
+name = "glob"
+version = "0.3.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0cc23270f6e1808e30a928bdc84dea0b9b4136a8bc82338574f23baf47bbd280"
+
+[[package]]
+name = "hashbrown"
+version = "0.17.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ed5909b6e89a2db4456e54cd5f673791d7eca6732202bbf2a9cc504fe2f9b84a"
+
[[package]]
name = "heck"
version = "0.5.0"
@@ -354,6 +400,16 @@
"tower-service",
]
+[[package]]
+name = "indexmap"
+version = "2.14.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d466e9454f08e4a911e14806c24e16fba1b4c121d1ea474396f396069cf949d9"
+dependencies = [
+ "equivalent",
+ "hashbrown",
+]
+
[[package]]
name = "is_terminal_polyfill"
version = "1.70.2"
@@ -437,6 +493,15 @@
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd"
+[[package]]
+name = "proc-macro-crate"
+version = "3.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "e67ba7e9b2b56446f1d419b1d807906278ffa1a658a8a5d8a39dcb1f5a78614f"
+dependencies = [
+ "toml_edit",
+]
+
[[package]]
name = "proc-macro2"
version = "1.0.106"
@@ -461,12 +526,85 @@
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "f8dcc9c7d52a811697d2151c701e0d08956f92b0e24136cf4cf27b57a6a0d9bf"
+[[package]]
+name = "regex"
+version = "1.12.4"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f1292b7759ae1cb9ec195452d1390a074f0cd8541ab7a5a8c31cd6db45d4a6ba"
+dependencies = [
+ "aho-corasick",
+ "memchr",
+ "regex-automata",
+ "regex-syntax",
+]
+
+[[package]]
+name = "regex-automata"
+version = "0.4.14"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "6e1dd4122fc1595e8162618945476892eefca7b88c52820e74af6262213cae8f"
+dependencies = [
+ "aho-corasick",
+ "memchr",
+ "regex-syntax",
+]
+
+[[package]]
+name = "regex-syntax"
+version = "0.8.11"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4"
+
+[[package]]
+name = "relative-path"
+version = "1.9.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "ba39f3699c378cd8970968dcbff9c43159ea4cfbd88d43c00b22f2ef10a435d2"
+
[[package]]
name = "roff"
version = "1.1.1"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "323c417e1d9665a65b263ec744ba09030cfb277e9daa0b018a4ab62e57bc8189"
+[[package]]
+name = "rstest"
+version = "0.26.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "f5a3193c063baaa2a95a33f03035c8a72b83d97a54916055ba22d35ed3839d49"
+dependencies = [
+ "futures-timer",
+ "futures-util",
+ "rstest_macros",
+]
+
+[[package]]
+name = "rstest_macros"
+version = "0.26.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "9c845311f0ff7951c5506121a9ad75aec44d083c31583b2ea5a30bcb0b0abba0"
+dependencies = [
+ "cfg-if",
+ "glob",
+ "proc-macro-crate",
+ "proc-macro2",
+ "quote",
+ "regex",
+ "relative-path",
+ "rustc_version",
+ "syn",
+ "unicode-ident",
+]
+
+[[package]]
+name = "rustc_version"
+version = "0.4.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cfcb3a22ef46e85b45de6ee7e79d063319ebb6594faafcf1c225ea92ab6e9b92"
+dependencies = [
+ "semver",
+]
+
[[package]]
name = "rustix"
version = "1.1.4"
@@ -486,6 +624,12 @@
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "9774ba4a74de5f7b1c1451ed6cd5285a32eddb5cccb8cc655a4e50009e06477f"
+[[package]]
+name = "semver"
+version = "1.0.28"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8a7852d02fc848982e0c167ef163aaff9cd91dc640ba85e263cb1ce46fae51cd"
+
[[package]]
name = "serde"
version = "1.0.228"
@@ -646,6 +790,36 @@
"syn",
]
+[[package]]
+name = "toml_datetime"
+version = "1.1.1+spec-1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "3165f65f62e28e0115a00b2ebdd37eb6f3b641855f9d636d3cd4103767159ad7"
+dependencies = [
+ "serde_core",
+]
+
+[[package]]
+name = "toml_edit"
+version = "0.25.12+spec-1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "d2153edc6955a6c354fad8f5efd38b6a8769bdccf9fe50f8e1329f81b0baa5d7"
+dependencies = [
+ "indexmap",
+ "toml_datetime",
+ "toml_parser",
+ "winnow",
+]
+
+[[package]]
+name = "toml_parser"
+version = "1.1.2+spec-1.1.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a2abe9b86193656635d2411dc43050282ca48aa31c2451210f4202550afb7526"
+dependencies = [
+ "winnow",
+]
+
[[package]]
name = "tower"
version = "0.5.3"
@@ -727,6 +901,15 @@
"windows-link",
]
+[[package]]
+name = "winnow"
+version = "1.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "0592e1c9d151f854e6fd382574c3a0855250e1d9b2f99d9281c6e6391af352f1"
+dependencies = [
+ "memchr",
+]
+
[[package]]
name = "zmij"
version = "1.0.21"
Cargo.toml
@@ -16,6 +16,7 @@
clap = { version = "4.5.60", features = ["derive"] }
clap_mangen = "0.2.31"
gix = { version = "0.83", features = ["no-default-features"] }
+rstest = "0.26"
tempfile = "3"
thiserror = "2"
tokio = { version = "1", features = [
crates/git-ents-server/Cargo.toml
@@ -12,6 +12,7 @@
tokio = { workspace = true }
[dev-dependencies]
+rstest = { workspace = true }
tempfile = { workspace = true }
[lints]
crates/git-ents-server/src/http.rs
@@ -39,12 +39,12 @@
return (StatusCode::BAD_REQUEST, "bad request").into_response();
}
- // A push begins with `info/refs?service=git-receive-pack`; auto-init the
- // bare repo so the very first request finds it. An unacceptable repository
- // path is rejected here rather than handed to the backend.
- let is_push = query_string.contains("service=git-receive-pack")
- || path_info.ends_with("/git-receive-pack");
- let push_repo = if is_push {
+ // A push uses exactly two endpoints: the receive-pack advertisement
+ // (`GET /<repo>/info/refs?service=git-receive-pack`) and the receive-pack
+ // RPC (`POST /<repo>/git-receive-pack`). Recognize the target so the bare
+ // repo can be auto-created on the very first request; reject an
+ // unacceptable repository path here rather than handing it to the backend.
+ let push_repo = if is_receive_pack(&path_info, &query_string) {
match repo_path(&path_info) {
Some(relative) => Some(state.data_dir.join(relative)),
None => return (StatusCode::BAD_REQUEST, "invalid repository path").into_response(),
@@ -53,14 +53,9 @@
None
};
if let Some(repo) = &push_repo
- && !repo.exists()
- && let Err(e) = init_bare_repo(repo).await
+ && let Err(response) = ensure_repo(&state, repo).await
{
- return (
- StatusCode::INTERNAL_SERVER_ERROR,
- format!("init failed: {e}"),
- )
- .into_response();
+ return response;
}
let content_type = header_value(&headers, "Content-Type");
@@ -178,6 +173,82 @@
/// Greatest repository nesting depth: `repo`, `org/repo`, or `org/team/repo`.
const MAX_REPO_DEPTH: usize = 3;
+/// Whether this request is a push: the smart-HTTP receive-pack advertisement
+/// (`/info/refs?service=git-receive-pack`) or the receive-pack RPC itself.
+///
+/// The `service` parameter is matched exactly, so a request for the read-only
+/// `git-upload-pack` service (or an unrelated parameter that merely contains
+/// the string) is not mistaken for a push.
+fn is_receive_pack(path_info: &str, query: &str) -> bool {
+ path_info.ends_with("/git-receive-pack")
+ || (path_info.ends_with("/info/refs") && query_service(query) == Some("git-receive-pack"))
+}
+
+/// The value of the `service` query parameter, if present.
+fn query_service(query: &str) -> Option<&str> {
+ query
+ .split('&')
+ .find_map(|pair| pair.strip_prefix("service="))
+}
+
+/// Ensure `repo` exists as a bare repository, creating it on first push.
+///
+/// Holds [`AppState::init_lock`] across the whole check-and-create so two
+/// concurrent first pushes to the same name cannot both initialize it, and
+/// refuses paths that collide with an existing repository: one nested inside a
+/// repo, or one that already exists as a namespace directory.
+async fn ensure_repo(state: &AppState, repo: &Path) -> Result<(), Response> {
+ let _guard = state.init_lock.lock().await;
+ if enclosing_repo(&state.data_dir, repo).is_some() {
+ return Err((
+ StatusCode::CONFLICT,
+ "repository path is nested inside an existing repository",
+ )
+ .into_response());
+ }
+ if repo.exists() {
+ return if is_bare_repo(repo) {
+ Ok(())
+ } else {
+ Err((
+ StatusCode::CONFLICT,
+ "repository path already exists as a namespace",
+ )
+ .into_response())
+ };
+ }
+ init_bare_repo(repo).await.map_err(|e| {
+ (
+ StatusCode::INTERNAL_SERVER_ERROR,
+ format!("init failed: {e}"),
+ )
+ .into_response()
+ })
+}
+
+/// The ancestor of `repo` (below `data_dir`) that is itself a bare repository,
+/// if any. Used to refuse creating a repository inside another one.
+fn enclosing_repo(data_dir: &Path, repo: &Path) -> Option<PathBuf> {
+ let relative = repo.strip_prefix(data_dir).ok()?;
+ let mut current = data_dir.to_path_buf();
+ let mut components = relative.components().peekable();
+ while let Some(component) = components.next() {
+ if components.peek().is_none() {
+ break;
+ }
+ current.push(component);
+ if is_bare_repo(¤t) {
+ return Some(current);
+ }
+ }
+ None
+}
+
+/// Whether `path` is the root of a bare git repository.
+fn is_bare_repo(path: &Path) -> bool {
+ path.join("HEAD").is_file() && path.join("objects").is_dir()
+}
+
/// The target repository of a push, as a validated path relative to the data
/// directory, or `None` if the request does not name an acceptable repository.
///
@@ -339,3 +410,49 @@
}
value
}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+ use rstest::rstest;
+
+ #[rstest]
+ #[case("repo", true)]
+ #[case("repo.git", true)]
+ #[case("My-Repo_1.git", true)]
+ #[case("", false)]
+ #[case(".", false)]
+ #[case("..", false)]
+ #[case(".hidden", false)]
+ #[case("a/b", false)]
+ #[case("a b", false)]
+ #[case("a%2eb", false)]
+ fn validates_segments(#[case] segment: &str, #[case] expected: bool) {
+ assert_eq!(valid_segment(segment), expected);
+ }
+
+ #[rstest]
+ #[case("/repo.git/git-receive-pack", Some("repo.git"))]
+ #[case("/org/repo.git/git-receive-pack", Some("org/repo.git"))]
+ #[case("/org/team/repo.git/git-receive-pack", Some("org/team/repo.git"))]
+ #[case("/repo.git/info/refs", Some("repo.git"))]
+ #[case("/a/b/c/d.git/git-receive-pack", None)]
+ #[case("/../etc/git-receive-pack", None)]
+ #[case("/.ssh/git-receive-pack", None)]
+ #[case("/git-receive-pack", None)]
+ fn extracts_repo_path(#[case] path: &str, #[case] expected: Option<&str>) {
+ assert_eq!(repo_path(path).as_deref(), expected.map(Path::new));
+ }
+
+ #[rstest]
+ #[case("/repo.git/git-receive-pack", "", true)]
+ #[case("/repo.git/info/refs", "service=git-receive-pack", true)]
+ #[case("/repo.git/info/refs", "service=git-upload-pack", false)]
+ #[case("/repo.git/info/refs", "", false)]
+ #[case("/repo.git/info/refs", "x=service=git-receive-pack", false)]
+ #[case("/repo.git/info/refs", "a=b&service=git-receive-pack", true)]
+ #[case("/repo.git/objects/info/packs", "", false)]
+ fn detects_pushes(#[case] path: &str, #[case] query: &str, #[case] expected: bool) {
+ assert_eq!(is_receive_pack(path, query), expected);
+ }
+}
crates/git-ents-server/src/main.rs
@@ -12,7 +12,7 @@
use axum::extract::DefaultBodyLimit;
use axum::routing::get;
use clap::{CommandFactory, Parser};
-use tokio::sync::Notify;
+use tokio::sync::{Mutex, Notify};
#[derive(Parser)]
#[command(
@@ -37,10 +37,12 @@
max_requests: Option<usize>,
}
-/// Shared handler state: where the bare repositories live.
+/// Shared handler state: where the bare repositories live, plus a lock that
+/// serializes repository creation so concurrent first pushes cannot race.
#[derive(Clone)]
pub(crate) struct AppState {
pub(crate) data_dir: PathBuf,
+ pub(crate) init_lock: Arc<Mutex<()>>,
}
fn main() -> ExitCode {
@@ -69,6 +71,7 @@
async fn serve(args: Args) -> ExitCode {
let state = AppState {
data_dir: args.data_dir,
+ init_lock: Arc::new(Mutex::new(())),
};
// The git smart-HTTP protocol streams whole packfiles through the request
crates/git-ents-server/tests/server.rs
@@ -11,6 +11,8 @@
use std::path::Path;
use std::process::Command;
+use rstest::rstest;
+
#[test]
fn responds_and_shuts_down() {
let probe = std::net::TcpListener::bind("127.0.0.1:0").unwrap();
@@ -87,6 +89,91 @@
assert_eq!(pushed, cloned, "cloned HEAD must match pushed HEAD");
}
+#[rstest]
+#[case("org/repo")]
+#[case("org/team/repo")]
+fn nested_push_then_clone_round_trip(#[case] name: &str) {
+ let data = tempfile::tempdir().unwrap();
+ let port = free_port();
+
+ let mut child = Command::new(env!("CARGO_BIN_EXE_git-ents-server"))
+ .arg("--port")
+ .arg(port.to_string())
+ .arg("--data-dir")
+ .arg(data.path())
+ .spawn()
+ .unwrap();
+
+ wait_for_port(port);
+
+ let url = format!("http://127.0.0.1:{port}/{name}.git");
+
+ let src = tempfile::tempdir().unwrap();
+ run_git(Some(src.path()), &["init", "-q", "-b", "main"]);
+ std::fs::write(src.path().join("README.md"), "hello ents\n").unwrap();
+ run_git(Some(src.path()), &["add", "."]);
+ run_git(Some(src.path()), &["commit", "-q", "-m", "initial"]);
+ run_git(Some(src.path()), &["push", "-q", &url, "main"]);
+ let pushed = rev_parse(src.path());
+
+ let dst = tempfile::tempdir().unwrap();
+ let clone_path = dst.path().join("clone");
+ run_git(None, &["clone", "-q", &url, clone_path.to_str().unwrap()]);
+ let cloned = rev_parse(&clone_path);
+
+ child.kill().unwrap();
+ let _wait = child.wait();
+
+ assert_eq!(
+ pushed, cloned,
+ "cloned HEAD must match pushed HEAD for {name}"
+ );
+}
+
+#[rstest]
+#[case("org/repo.git/deep.git")] // nested inside an existing repository
+#[case("org")] // already exists as a namespace
+fn rejects_colliding_pushes(#[case] collide: &str) {
+ let data = tempfile::tempdir().unwrap();
+ let port = free_port();
+
+ let mut child = Command::new(env!("CARGO_BIN_EXE_git-ents-server"))
+ .arg("--port")
+ .arg(port.to_string())
+ .arg("--data-dir")
+ .arg(data.path())
+ .spawn()
+ .unwrap();
+
+ wait_for_port(port);
+
+ let src = tempfile::tempdir().unwrap();
+ run_git(Some(src.path()), &["init", "-q", "-b", "main"]);
+ std::fs::write(src.path().join("README.md"), "hello ents\n").unwrap();
+ run_git(Some(src.path()), &["add", "."]);
+ run_git(Some(src.path()), &["commit", "-q", "-m", "initial"]);
+
+ // Claim `org/repo.git`, which also makes `org` a namespace directory.
+ let base = format!("http://127.0.0.1:{port}/org/repo.git");
+ run_git(Some(src.path()), &["push", "-q", &base, "main"]);
+
+ // A push that collides with that repository must be refused.
+ let url = format!("http://127.0.0.1:{port}/{collide}");
+ let rejected = !git_command(Some(src.path()), &["push", "-q", &url, "main"])
+ .output()
+ .unwrap()
+ .status
+ .success();
+
+ child.kill().unwrap();
+ let _wait = child.wait();
+
+ assert!(
+ rejected,
+ "push colliding with an existing repo ({collide}) must fail"
+ );
+}
+
fn free_port() -> u16 {
let probe = TcpListener::bind("127.0.0.1:0").unwrap();
let port = probe.local_addr().unwrap().port();