From 7160abd177e9629e936ffcf4969797b0e703400c Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Wed, 30 Sep 2026 11:04:12 -0700 Subject: [PATCH 1/6] Remove ineffective arbitrary-want setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? Partial-clone lazy fetches require direct object wants, while the configuration claimed they could be disabled even though both upload-pack engines always accepted them. ### What? Remove the inert `git.allow_any_sha1_in_want` option and update tests and integrity documentation to describe arbitrary object wants as part of partial-clone support. ## Test Plan ⚠️ Rust tests were not run because `cargo` is unavailable in this environment; `git diff --check` passes. ## Issue Closes #77 (cherry picked from commit 06a0ef3f440c498196bd22d14c79287e38f19a7f) Signed-off-by: Yordis Prieto --- crates/walgit-config/src/lib.rs | 2 -- crates/walgit-server/tests/e2e.rs | 2 +- crates/walgit-server/tests/maintain.rs | 1 - docs/INTEGRITY.md | 2 +- walgit.example.toml | 1 - 5 files changed, 2 insertions(+), 6 deletions(-) diff --git a/crates/walgit-config/src/lib.rs b/crates/walgit-config/src/lib.rs index 4b6659ad..68a92d37 100644 --- a/crates/walgit-config/src/lib.rs +++ b/crates/walgit-config/src/lib.rs @@ -588,7 +588,6 @@ pub struct GitConfig { pub binary: PathBuf, pub upload_pack_engine: UploadPackEngine, pub allow_filter: bool, - pub allow_any_sha1_in_want: bool, /// Default object format for new repos. pub object_format: ObjectFormat, /// Maintain a split commit-graph chain per local repo: tier-2 packs that @@ -994,7 +993,6 @@ impl Default for GitConfig { binary: PathBuf::from("git"), upload_pack_engine: UploadPackEngine::Auto, allow_filter: true, - allow_any_sha1_in_want: false, object_format: ObjectFormat::Sha1, commit_graph: true, commit_graph_changed_paths: false, diff --git a/crates/walgit-server/tests/e2e.rs b/crates/walgit-server/tests/e2e.rs index ba7f0628..0c1bf17b 100644 --- a/crates/walgit-server/tests/e2e.rs +++ b/crates/walgit-server/tests/e2e.rs @@ -1653,7 +1653,7 @@ async fn push_from_a_shallow_clone() -> TestResult { /// `allow-any-sha1-in-want`), plus `--depth` + filter together. #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn partial_clone_tree_zero_and_depth_with_filter() -> TestResult { - let server = Server::start_with_tweak(|c| c.git.allow_any_sha1_in_want = true).await?; + let server = Server::start().await?; server.put_repo("t", "tree0").await?; let src = TestRepo::synthetic(5, 4)?; git_in( diff --git a/crates/walgit-server/tests/maintain.rs b/crates/walgit-server/tests/maintain.rs index f7df8ca7..0c6205e2 100644 --- a/crates/walgit-server/tests/maintain.rs +++ b/crates/walgit-server/tests/maintain.rs @@ -330,7 +330,6 @@ async fn fsck_unit_records_missing_objects_and_repair_unit_fetches_them_from_ups let server = step!( "start", Server::start_with_tweak(|c| { - c.git.allow_any_sha1_in_want = true; c.maintenance.checkpoints = false; c.packs.enabled = false; c.maintenance.fsck_interval = std::time::Duration::from_hours(1); diff --git a/docs/INTEGRITY.md b/docs/INTEGRITY.md index 142a1982..62911d50 100644 --- a/docs/INTEGRITY.md +++ b/docs/INTEGRITY.md @@ -33,7 +33,7 @@ Due right after checkpoint when `fsck.pb` lists missing objects, `repaired_seq = **`upstream.git`** (D24 setting, `[upstream] git = "https://github.com/acme/monorepo.git"`, `token_env` shared with `upstream.lfs`, `docs/LFS.md`). Steps (`walgit_git::repair::fetch_objects_as_pack`): scratch bare repo under `cache.dir/repair/`, `git fetch --depth=1 …` in batches of 500 (GitHub serves commit, tree **and -blob** wants by SHA — verified 2026-08-21; walgit does with `git.allow_any_sha1_in_want`), `pack-objects` of exactly +blob** wants by SHA — verified 2026-08-21; walgit enables arbitrary object wants for partial-clone lazy fetches), `pack-objects` of exactly the requested oids, verify every oid is in the resulting idx (a refused want is an error, never a silent hole), then `RepoHandle::add_pack(tier 0)` → one **COMPACT entry superseding nothing** (what `walgit wal add-pack … --tier 0` did by hand for a large repository, seq 11). `fsck.pb.repaired_seq` is set so the next pass re-audits instead of repairing diff --git a/walgit.example.toml b/walgit.example.toml index 8f0e959d..9d81dba7 100644 --- a/walgit.example.toml +++ b/walgit.example.toml @@ -186,7 +186,6 @@ max_object_bytes = "16GiB" binary = "git" # path to upstream git binary (>= 2.47 recommended: pack.writeReverseIndex) upload_pack_engine = "auto" # "auto" = stock git when packs are local/mount-linked, gix only for remote-served bases | "git" | "gix" allow_filter = true # uploadpack.allowFilter -allow_any_sha1_in_want = false # uploadpack.allowAnySHA1InWant object_format = "sha1" # default for new repos: "sha1" | "sha256" commit_graph = true # maintain a split commit-graph chain per repo (tier-2 layer from the WAL + incremental) history_pack = true # base rebuild also publishes commits+trees as a local "history pack" From 804d5da769ffd2183f96d0ca20b1db6391841bb9 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Thu, 1 Oct 2026 11:02:52 -0700 Subject: [PATCH 2/6] Document arbitrary object read access Clarify that repository read access already permits fetching stored objects by ID, including unreachable objects, so removing the ineffective configuration switch does not widen authorization. (cherry picked from commit b0b18b1136946379ff05a3bcbaf2783a60d4acef) Signed-off-by: Yordis Prieto --- docs/INTEGRITY.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/INTEGRITY.md b/docs/INTEGRITY.md index 62911d50..2eff6769 100644 --- a/docs/INTEGRITY.md +++ b/docs/INTEGRITY.md @@ -41,6 +41,10 @@ again. Counter `walgit_repair_objects_total{repo}`. Test: `tests/maintain.rs fsck_unit_records_missing_objects_and_repair_unit_fetches_them_from_upstream` (hole → audit → repair over HTTP from a second repository → re-audit clean → idle). +This does not widen repository access: a principal with read access can already fetch any stored object by ID, +including objects unreachable from an advertised ref. Removing `git.allow_any_sha1_in_want` only removes a +non-functional configuration switch. + ## 4. Runbook (what was done for a large repository, 2026-08-21 03:40Z) 1. Enumerate: `git rev-list --objects --missing=print --not ` on the complete copy (or read `fsck.pb` / `/var/lib/walgit/monorepo-fsck.out`) → 1,952 blobs, 70 MB. From b2d6a0aafecafbc414479de13bc6cd07d5926a8a Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Wed, 30 Sep 2026 11:08:09 -0700 Subject: [PATCH 3/6] Remove ineffective global request limits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? The server exposed global concurrency and total request timeout settings without a safe way to apply them. Enforcing them around the whole router would queue health probes behind clones and terminate long streaming Git, LFS, and pack responses mid-transfer. ### What? Remove `server.max_concurrent_requests` and `server.request_timeout`, along with the unsafe middleware implementation and misleading documentation. Keep the existing per-repository Git concurrency limit, and reject the removed keys instead of silently accepting unsupported controls. ## Test Plan ✅ `cargo test -p walgit-config removed_global_request_limit_configuration_is_rejected` ✅ `cargo check -p walgit-server` ✅ `cargo clippy -p walgit-server --all-targets -- -D warnings` ✅ `cargo fmt --all -- --check` ✅ `git diff --check` ## Issue Closes #78 (cherry picked from commit c7ab81ace98ce8a30d2c2546b2e0573c17a6b944) Signed-off-by: Yordis Prieto --- crates/walgit-config/src/lib.rs | 16 +++++++++++----- crates/walgit-server/src/middleware.rs | 6 ++---- crates/walgit-server/tests/harness.rs | 1 - walgit.example.toml | 2 -- 4 files changed, 13 insertions(+), 12 deletions(-) diff --git a/crates/walgit-config/src/lib.rs b/crates/walgit-config/src/lib.rs index 68a92d37..f3e8959d 100644 --- a/crates/walgit-config/src/lib.rs +++ b/crates/walgit-config/src/lib.rs @@ -45,11 +45,8 @@ pub struct Config { pub struct ServerConfig { pub listen: SocketAddr, pub http2: bool, - pub max_concurrent_requests: usize, /// Per-repo cap on concurrent upload-pack/receive-pack processes. pub max_concurrent_per_repo: usize, - #[serde(with = "humantime_serde")] - pub request_timeout: Duration, /// Graceful drain after SIGTERM: new object work (fetch/push/LFS) is /// refused with 503 + Retry-After and `/readyz` turns 503 at once; in-flight /// requests and the running maintenance unit get this long to finish @@ -863,9 +860,7 @@ impl Default for ServerConfig { ServerConfig { listen: std::net::SocketAddr::from(([127, 0, 0, 1], 8080)), http2: true, - max_concurrent_requests: 512, max_concurrent_per_repo: 64, - request_timeout: Duration::from_hours(1), drain_timeout: Duration::from_secs(20), max_push_bytes: ByteSize::gib(64), roles: vec![], @@ -1562,6 +1557,17 @@ mod tests { assert!(!base.public_settings_toml().unwrap().contains("[bundles]")); } + #[test] + fn removed_global_request_limit_configuration_is_rejected() { + for input in [ + "[server]\nmax_concurrent_requests = 512\n", + "[server]\nrequest_timeout = \"1h\"\n", + ] { + assert!(Config::parse(input).is_err(), "{input}"); + assert!(Config::default().with_settings(input).is_err(), "{input}"); + } + } + #[test] fn removed_compaction_configuration_is_rejected() { for input in [ diff --git a/crates/walgit-server/src/middleware.rs b/crates/walgit-server/src/middleware.rs index 6150bc9a..690e9f25 100644 --- a/crates/walgit-server/src/middleware.rs +++ b/crates/walgit-server/src/middleware.rs @@ -1,7 +1,5 @@ -//! HTTP middleware: request id + tracing span. Request timeout, body limit and -//! tracing layers are applied in [`crate::router`] via `tower-http`. Per-repo -//! concurrency limiting lives in the handlers (they hold a repo-keyed semaphore -//! for the duration of the git operation). +//! HTTP middleware: request id + tracing span and per-repository concurrency +//! limits. Per-repository permits live in handlers for the full Git operation. use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; diff --git a/crates/walgit-server/tests/harness.rs b/crates/walgit-server/tests/harness.rs index 0af5816f..80fdc455 100644 --- a/crates/walgit-server/tests/harness.rs +++ b/crates/walgit-server/tests/harness.rs @@ -77,7 +77,6 @@ impl Server { cfg.cache.max_bytes = ByteSize::gib(2); cfg.server.listen = "127.0.0.1:0".parse().unwrap(); cfg.server.max_concurrent_per_repo = 8; - cfg.server.request_timeout = std::time::Duration::from_mins(10); cfg.server.max_push_bytes = ByteSize::gib(2); cfg.wal.fsck_objects = true; cfg.wal.check_connectivity = true; diff --git a/walgit.example.toml b/walgit.example.toml index 9d81dba7..f16cd797 100644 --- a/walgit.example.toml +++ b/walgit.example.toml @@ -8,9 +8,7 @@ [server] listen = "127.0.0.1:8080" # default; `mode = none` is refused unless this is loopback. Public bind: 0.0.0.0 with token/oidc. http2 = true # HTTP/2 (h2c when TLS is off, ALPN when on) -max_concurrent_requests = 512 # global cap on in-flight git requests max_concurrent_per_repo = 64 # per-repo cap (upload-pack / receive-pack) -request_timeout = "1h" drain_timeout = "20s" # after SIGTERM, phase 2: how long in-flight requests may finish once /readyz is 503 # and new fetch/push/LFS are refused (503 + Retry-After). The running maintenance unit # is interrupted at once in phase 1 while serving continues. From 1d7dbd5ddb84da0280435a19da51023f44eef268 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Wed, 30 Sep 2026 11:17:57 -0700 Subject: [PATCH 4/6] Remove ignored Git binary setting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? `git.binary` implied that every Git subprocess could use an operator-selected executable, but the implementation consistently invokes `git` from `PATH` and never read the setting. ### What? Remove the unsupported configuration field and example entry, and verify that the obsolete key is rejected instead of silently suggesting custom-binary support. ## Test Plan ✅ `cargo test -p walgit-config removed_git_binary_configuration_is_rejected` ✅ `cargo fmt --all -- --check` ✅ `git diff --check` ## Issue Closes #81 (cherry picked from commit 129fcc2a857c35cc3c021936cf61d18e6992feec) Signed-off-by: Yordis Prieto --- crates/walgit-config/src/lib.rs | 20 +++++++++++++------- walgit.example.toml | 1 - 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/crates/walgit-config/src/lib.rs b/crates/walgit-config/src/lib.rs index f3e8959d..ac3d7670 100644 --- a/crates/walgit-config/src/lib.rs +++ b/crates/walgit-config/src/lib.rs @@ -581,8 +581,6 @@ pub struct UpstreamConfig { reason = "Independent configuration switches, not mutually exclusive states" )] pub struct GitConfig { - /// Path to the upstream git binary (repack, optional upload-pack engine). - pub binary: PathBuf, pub upload_pack_engine: UploadPackEngine, pub allow_filter: bool, /// Default object format for new repos. @@ -985,7 +983,6 @@ impl Default for LfsConfig { impl Default for GitConfig { fn default() -> Self { GitConfig { - binary: PathBuf::from("git"), upload_pack_engine: UploadPackEngine::Auto, allow_filter: true, object_format: ObjectFormat::Sha1, @@ -1496,10 +1493,12 @@ mod tests { // `Config::parse` (startup) reads the process env through the same path. let mut c = Config::default(); assert!( - c.apply_env(vec![("WALGIT__GIT__BINARI".to_string(), "g".to_string())].into_iter()) - .unwrap_err() - .to_string() - .contains("did you mean WALGIT__GIT__BINARY?") + c.apply_env( + vec![("WALGIT__GIT__HISTORY_PAC".to_string(), "g".to_string())].into_iter() + ) + .unwrap_err() + .to_string() + .contains("did you mean WALGIT__GIT__HISTORY_PACK?") ); } @@ -1588,6 +1587,13 @@ mod tests { ); } + #[test] + fn removed_git_binary_configuration_is_rejected() { + let input = "[git]\nbinary = \"/opt/git/bin/git\"\n"; + assert!(Config::parse(input).is_err()); + assert!(Config::default().with_settings(input).is_err()); + } + #[test] fn settings_merge_over_config_and_are_restricted() { let mut base = Config::default(); diff --git a/walgit.example.toml b/walgit.example.toml index f16cd797..eb26612a 100644 --- a/walgit.example.toml +++ b/walgit.example.toml @@ -181,7 +181,6 @@ max_object_bytes = "16GiB" # maintenance.follow_interval, published through the WAL as a push; fast-forward only [git] -binary = "git" # path to upstream git binary (>= 2.47 recommended: pack.writeReverseIndex) upload_pack_engine = "auto" # "auto" = stock git when packs are local/mount-linked, gix only for remote-served bases | "git" | "gix" allow_filter = true # uploadpack.allowFilter object_format = "sha1" # default for new repos: "sha1" | "sha256" From 81ee560a89212b9ca08416bd6c0ad2bbf0fed8d9 Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Wed, 30 Sep 2026 11:23:38 -0700 Subject: [PATCH 5/6] Remove unused transport switches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `server.http2` and `store.gcs.direct_connectivity` were accepted configuration values with no runtime consumers, so changing them gave operators a false impression that transport behavior changed. Remove both unsupported switches from the configuration surface and example file, and add a regression test proving the obsolete keys are rejected. ✅ `cargo test -p walgit-config removed_transport_switches_are_rejected` ✅ `cargo fmt --all -- --check` ✅ `git diff --check` Closes #83 (cherry picked from commit a200384829b9f7d3b6e41394159a3996a89d2e2d) Signed-off-by: Yordis Prieto --- crates/walgit-config/src/lib.rs | 26 +++++++++++++++++++------- walgit.example.toml | 2 -- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/crates/walgit-config/src/lib.rs b/crates/walgit-config/src/lib.rs index ac3d7670..c4fbe04d 100644 --- a/crates/walgit-config/src/lib.rs +++ b/crates/walgit-config/src/lib.rs @@ -44,7 +44,6 @@ pub struct Config { #[serde(deny_unknown_fields, default)] pub struct ServerConfig { pub listen: SocketAddr, - pub http2: bool, /// Per-repo cap on concurrent upload-pack/receive-pack processes. pub max_concurrent_per_repo: usize, /// Graceful drain after SIGTERM: new object work (fetch/push/LFS) is @@ -265,7 +264,6 @@ pub enum StoreBackend { pub struct GcsConfig { /// gRPC endpoint. pub endpoint: String, - pub direct_connectivity: bool, /// Service account for signed URLs; None = ADC/IAM signBlob. pub signing_service_account: Option, /// Separate data clients (own channels) for bulk traffic — pack/idx/side- @@ -857,7 +855,6 @@ impl Default for ServerConfig { fn default() -> Self { ServerConfig { listen: std::net::SocketAddr::from(([127, 0, 0, 1], 8080)), - http2: true, max_concurrent_per_repo: 64, drain_timeout: Duration::from_secs(20), max_push_bytes: ByteSize::gib(64), @@ -911,7 +908,6 @@ impl Default for GcsConfig { fn default() -> Self { GcsConfig { endpoint: "https://storage.googleapis.com".into(), - direct_connectivity: true, signing_service_account: None, bulk_clients: 4, bulk_concurrency: 32, @@ -1527,17 +1523,23 @@ mod tests { let err = c .apply_env( vec![ - ("WALGIT__SERVER__HTTP2".to_string(), "1".to_string()), + ( + "WALGIT__SERVER__ACCEL_REDIRECT".to_string(), + "1".to_string(), + ), ("WALGIT__STORE__BUCKET__NAME".to_string(), "b".to_string()), ] .into_iter(), ) .unwrap_err() .to_string(); - assert!(err.contains("WALGIT__SERVER__HTTP2: "), "{err}"); + assert!(err.contains("WALGIT__SERVER__ACCEL_REDIRECT: "), "{err}"); assert!(err.contains("WALGIT__STORE__BUCKET__NAME: "), "{err}"); assert!(err.contains("not a table"), "{err}"); - assert_eq!(c.server.http2, Config::default().server.http2); + assert_eq!( + c.server.accel_redirect, + Config::default().server.accel_redirect + ); assert_eq!(c.store.bucket, "20260915"); } @@ -1594,6 +1596,16 @@ mod tests { assert!(Config::default().with_settings(input).is_err()); } + #[test] + fn removed_transport_switches_are_rejected() { + for input in [ + "[server]\nhttp2 = false\n", + "[store.gcs]\ndirect_connectivity = false\n", + ] { + assert!(Config::parse(input).is_err(), "{input}"); + } + } + #[test] fn settings_merge_over_config_and_are_restricted() { let mut base = Config::default(); diff --git a/walgit.example.toml b/walgit.example.toml index eb26612a..5259ada1 100644 --- a/walgit.example.toml +++ b/walgit.example.toml @@ -7,7 +7,6 @@ [server] listen = "127.0.0.1:8080" # default; `mode = none` is refused unless this is loopback. Public bind: 0.0.0.0 with token/oidc. -http2 = true # HTTP/2 (h2c when TLS is off, ALPN when on) max_concurrent_per_repo = 64 # per-repo cap (upload-pack / receive-pack) drain_timeout = "20s" # after SIGTERM, phase 2: how long in-flight requests may finish once /readyz is 503 # and new fetch/push/LFS are refused (503 + Retry-After). The running maintenance unit @@ -82,7 +81,6 @@ force_path_style = true # path-style `endpoint/bucket/key`, what rustf [store.gcs] endpoint = "https://storage.googleapis.com" # gRPC endpoint; credentials = Application Default Credentials -direct_connectivity = true # signing_service_account = "sa@project.iam.gserviceaccount.com" # for lfs serve_via = "signed_url" bulk_clients = 4 # separate data channels for pack/idx/side-file/LFS bytes + ranged reads bulk_concurrency = 32 # max concurrent bulk requests per process (control plane never queues behind them) From f7633ac841165f8c494b143b1aef6d4000d09a7d Mon Sep 17 00:00:00 2001 From: Preetam Dwivedi Date: Wed, 30 Sep 2026 11:24:10 -0700 Subject: [PATCH 6/6] Ignore macOS Finder metadata MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary ### Why? Finder-created `.DS_Store` files appeared as untracked changes throughout the repository and could be committed accidentally. ### What? Ignore `.DS_Store` recursively from the root ignore file. ## Test Plan ✅ `git check-ignore -v .DS_Store crates/.DS_Store crates/walgit-server/.DS_Store` ✅ `git diff --check` ## Issue Closes #84 (cherry picked from commit ef1120d1ba027bc67a9726e1567e5ea83914a77b) Signed-off-by: Yordis Prieto --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 2f18fa33..3b8633a0 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ target/ *.orig +.DS_Store # local secrets / environment — never in the repo .env