diff --git a/.env.example b/.env.example index 81c60824d..50a72da0d 100644 --- a/.env.example +++ b/.env.example @@ -17,6 +17,31 @@ GITLAWB_PORT=7545 # ── Storage ─────────────────────────────────────────────────────────────── GITLAWB_REPOS_DIR=/data/repos +# ── Object storage (durable repo archives) ──────────────────────────────── +# Backend for whole-repo archives: s3 | fs | ipfs. Empty = auto-detect: +# s3 when a bucket is set, else fs when GITLAWB_STORAGE_FS_DIR is set, else +# local-only (repos live only on this node's disk). `ipfs` is never +# auto-selected — setting GITLAWB_IPFS_API alone keeps its pinning-only +# meaning; opt in explicitly with GITLAWB_STORAGE_BACKEND=ipfs. NOTE: the +# ipfs backend stores archives in the Kubo daemon's LOCAL MFS namespace, so +# every node must point at the SAME Kubo instance. +GITLAWB_STORAGE_BACKEND= +# Bucket for the s3 backend (Tigris, R2, AWS S3, MinIO, B2). +# GITLAWB_TIGRIS_BUCKET is honored as a legacy alias. +GITLAWB_S3_BUCKET= +# Endpoint URL override for the s3 backend (R2/MinIO). On Tigris/Fly the +# endpoint arrives via AWS_ENDPOINT_URL_S3 — leave empty. +GITLAWB_S3_ENDPOINT= +# Force path-style addressing (required by MinIO and some S3-compatibles). +GITLAWB_S3_FORCE_PATH_STYLE=false +# Directory for the fs (local filesystem) backend. +GITLAWB_STORAGE_FS_DIR= +# Ack pushes before the durable upload finishes (write-back). Lower latency; +# opt-in durability tradeoff — see --help for the full semantics. +GITLAWB_ASYNC_UPLOAD=false +# Dedicated DB pool for per-repo write locks; a push pins one connection for +# its lifetime, so this bounds per-node push concurrency. + # PostgreSQL connection URL. Required. # When using the bundled docker-compose, this is wired automatically. DATABASE_URL=postgresql://gitlawb:changeme@localhost:5432/gitlawb diff --git a/Cargo.lock b/Cargo.lock index 3f29b0767..a842049db 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3441,6 +3441,7 @@ dependencies = [ "async-compression", "async-graphql", "async-graphql-axum", + "async-trait", "aws-config", "aws-sdk-s3", "axum", @@ -3465,6 +3466,7 @@ dependencies = [ "libp2p-kad", "libp2p-quic", "libp2p-swarm", + "md-5", "mockito", "multiaddr", "prometheus", diff --git a/README.md b/README.md index 3a092bf21..578d2d53a 100644 --- a/README.md +++ b/README.md @@ -397,7 +397,7 @@ Important node settings: | `GITLAWB_AUTO_SYNC` | Enable automatic sync from known peers. | | `GITLAWB_MAX_PACK_BYTES` | Max git pack body size for smart-HTTP routes. | | `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` | Max seconds a served git upload-pack, receive-pack, or `info/refs` advertisement may run before it is aborted (504). Default 600. Also bounds the withheld-blob classification walk (on both the upload-pack serve and receive-pack replication paths) and the push-side pin-candidate discovery (`rev-list` / `cat-file`), each reaped via process-group teardown at the deadline. On the path-scoped upload-pack path the classification walk and the pack serve share ONE deadline, so this value bounds their combined duration rather than granting each stage a full budget: a walk that consumes it leaves the serve nothing and the clone gets a 504. Serving large path-scoped repos may therefore need a higher value than they did when each stage was budgeted separately. Accepted range is 1 to 3153600000 (100 years), since the node derives deadlines from this value and a larger one cannot be represented. | -| `GITLAWB_GIT_ACQUIRE_TIMEOUT_SECS` | Max seconds the storage-acquisition phase (Tigris HEAD/GET, push advisory-lock) of a served git op may run before the request is shed with a 503, separate from the git-run timeout. The concurrency permit is released on expiry so a stalled backend cannot pin the pool. Default 30. | +| `GITLAWB_GIT_ACQUIRE_TIMEOUT_SECS` | Max seconds the storage-acquisition phase (storage-backend HEAD/GET, push advisory-lock) of a served git op may run before the request is shed with a 503, separate from the git-run timeout. The concurrency permit is released on expiry so a stalled backend cannot pin the pool. Default 30. | | `GITLAWB_MAX_CONCURRENT_GIT_OPS` | Max concurrent served git READ ops (upload-pack and its `info/refs` advertisement) across all callers; over-cap sheds a 503 + Retry-After. Anonymous reads draw from this pool, so pair it with `GITLAWB_MAX_CONCURRENT_READS_PER_CALLER`. Pushes and the receive-pack advertisement have their own pools, so a read flood cannot shed an authenticated push. Default 128. | | `GITLAWB_MAX_CONCURRENT_GIT_PUSHES` | Max concurrent `git-receive-pack` POST operations, in a pool separate from the read pool. The anon receive-pack `info/refs` advertisement runs in a third pool of the same size, disjoint from both, so an advertisement flood cannot shed a push either. Two per-source push caps are derived from this value (`/8`, floor 1) and have no env var of their own. Over-cap sheds a 503 + Retry-After. Default 32. | | `GITLAWB_MAX_CONCURRENT_READS_PER_CALLER` | Max concurrent read ops a single caller may hold, so one caller cannot monopolize the read pool. Keyed on the resolved source IP, never the DID, and only as granular as `GITLAWB_TRUSTED_PROXY`: left unset, a node behind an edge or NAT keys every caller on the edge IP and this collapses to one global cap. Default 16. | @@ -412,7 +412,13 @@ Important node settings: | `GITLAWB_IPFS_REQUEST_BUDGET_SECS` | Absolute wall-clock budget for one admitted `/ipfs/{cid}` request's acquire+walk lifetime. Per-stage clamps bound the acquire and walk stages to the remaining budget, and no stage starts once it is exhausted; the scan then stops with a retryable 503. The object-type probe and content-read `cat-file` subprocesses are budget-checked before starting and each also run under their own deadline (the lesser of `GITLAWB_GIT_SERVICE_TIMEOUT_SECS` and the remaining budget), reaped via process-group teardown, so a hung `cat-file` cannot hold the request's walk slot past it. One hang path is still unbounded: the probe's object-store readability check is a plain filesystem sweep with nothing to reap, so a wedged filesystem can hold the slot past the deadline. Default 600. Accepted range is 1 to 3153600000 (100 years), since the node derives a deadline from this value and a larger one cannot be represented. | | `GITLAWB_IPFS_RESOLVE_BUDGET_SECS` | Shorter budget for the pre-walk CID resolve inside an admitted `/ipfs/{cid}` request: the lookup that maps the requested CID to its git oid(s), which runs while the scarce walk admission is already held. A well-formed CID with no pin row does no probe and no walk work, so without this it could hold a walk slot for the whole request budget while nothing walked, and enough such requests shed every real retrieval at admission. The effective deadline is the lesser of this and the remaining request budget, so a value above `GITLAWB_IPFS_REQUEST_BUDGET_SECS` degrades to the request budget. Only the resolve is on this clock; walk and probe work stay on the request budget, so a slow but progressing scan is never shed by it. Default 10. Accepted range is 1 to 3153600000 (100 years). | | `GITLAWB_IPFS_RATE_LIMIT` | Max `/ipfs/{cid}` requests per client IP per hour (route flood brake). 0 disables. Default 600. | -| `GITLAWB_TIGRIS_BUCKET` | Optional S3/Tigris shared repo storage bucket. | +| `GITLAWB_STORAGE_BACKEND` | Object-storage backend for repo archives: `s3`, `fs`, or `ipfs`. Empty = auto-detect (`s3` if a bucket is set, else `fs` if a dir is set, else local-only). `ipfs` is never auto-selected and requires all nodes to share one Kubo instance (MFS is daemon-local). | +| `GITLAWB_S3_BUCKET` | Bucket for the `s3` backend (Tigris, R2, AWS S3, MinIO, B2). | +| `GITLAWB_S3_ENDPOINT` | Endpoint URL override for the `s3` backend (R2/MinIO; empty on Tigris/Fly). | +| `GITLAWB_S3_FORCE_PATH_STYLE` | Force path-style S3 addressing (MinIO and some S3-compatibles). | +| `GITLAWB_STORAGE_FS_DIR` | Directory for the `fs` (local filesystem) backend. | +| `GITLAWB_ASYNC_UPLOAD` | Ack pushes before the durable storage upload (write-back). Lower latency, opt-in durability tradeoff. Default `false`. | +| `GITLAWB_TIGRIS_BUCKET` | Legacy alias for `GITLAWB_S3_BUCKET` (selects the `s3` backend). | | `GITLAWB_PINATA_JWT` | Optional Pinata/IPFS warm-storage pinning. | | `GITLAWB_IRYS_URL` | Optional Irys/Arweave permanent anchoring. | diff --git a/crates/gitlawb-node/Cargo.toml b/crates/gitlawb-node/Cargo.toml index c583569cb..901078611 100644 --- a/crates/gitlawb-node/Cargo.toml +++ b/crates/gitlawb-node/Cargo.toml @@ -33,9 +33,15 @@ sqlx = { version = "0.8", features = ["postgres", "runtime-tokio-rustls", "chron clap = { version = "4", features = ["derive", "env"] } bytes = "1" libc = "0.2" +async-trait = "0.1" cid = { workspace = true } hex = { workspace = true } sha2 = { workspace = true } +# Content etags for repo archives: S3 single-part ETags are the body MD5, so a +# client-side MD5 lets the uploader record its *intended* etag before the PUT +# (crash-recovery provenance for the pending-upload marker). Not used for +# anything security-sensitive. +md-5 = "0.10" hmac = { workspace = true } http-body-util = "0.1" tokio-util = { version = "0.7", features = ["io"] } @@ -57,7 +63,7 @@ aws-sdk-s3 = { version = "1", default-features = false, features = ["sigv4a", "d aws-config = { version = "1", features = ["behavior-version-latest"] } async-compression = { version = "0.4", features = ["tokio", "zstd"] } tar = "0.4" -zstd = "0.13" +zstd = { version = "0.13", features = ["zstdmt"] } # Prometheus metrics. Used to expose a /metrics endpoint for ops/observability # on the opt-in GITLAWB_METRICS_ADDR listener. The crate is also the de-facto # exposition format encoder in the Rust ecosystem. diff --git a/crates/gitlawb-node/src/api/ipfs.rs b/crates/gitlawb-node/src/api/ipfs.rs index 92d129803..6426b4f49 100644 --- a/crates/gitlawb-node/src/api/ipfs.rs +++ b/crates/gitlawb-node/src/api/ipfs.rs @@ -2333,7 +2333,7 @@ pub(crate) enum MarkerQuery { // Test-only fault-injection seam for the `needs_scan` marker pair // (`pin_sources_at_cap`, `pin_sources_incomplete`), same idea as -// `RepoStore::tigris_stall`: hold one specific await open so the clamp around it is +// `RepoStore::storage_stall`: hold one specific await open so the clamp around it is // the one observed to fire. // // A `LOCK TABLE` fixture cannot isolate these two. `pin_sources_at_cap` reads @@ -3757,10 +3757,12 @@ mod tests { // parallel test run); the silent local endpoint stalls the HEAD // deterministically. let endpoint = crate::test_support::silent_http_endpoint().await; - let tigris = - crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint) - .await; - state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool); + let blob: std::sync::Arc = std::sync::Arc::new( + crate::storage::s3::S3BlobStore::for_testing_with_endpoint("test-bucket", &endpoint) + .await, + ); + let archive = crate::storage::archive::RepoArchive::new(blob); + state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(archive), pool); state.push_limiter_trust = crate::rate_limit::TrustedProxy::None; let mut cfg = (*state.config).clone(); cfg.git_acquire_timeout_secs = 1; @@ -3830,10 +3832,12 @@ mod tests { // consults the silent local endpoint and stalls to the 1s timeout // (endpoint-pinned test client, no AWS_* env reads). let endpoint = crate::test_support::silent_http_endpoint().await; - let tigris = - crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint) - .await; - state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool); + let blob: std::sync::Arc = std::sync::Arc::new( + crate::storage::s3::S3BlobStore::for_testing_with_endpoint("test-bucket", &endpoint) + .await, + ); + let archive = crate::storage::archive::RepoArchive::new(blob); + state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(archive), pool); let mut cfg = (*state.config).clone(); cfg.git_acquire_timeout_secs = 1; state.config = Arc::new(cfg); @@ -7955,10 +7959,12 @@ mod tests { // consults the silent local endpoint and stalls past the budget // (endpoint-pinned test client, no AWS_* env reads). let endpoint = crate::test_support::silent_http_endpoint().await; - let tigris = - crate::git::tigris::TigrisClient::for_testing_with_endpoint("test-bucket", &endpoint) - .await; - state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(tigris), pool); + let blob: std::sync::Arc = std::sync::Arc::new( + crate::storage::s3::S3BlobStore::for_testing_with_endpoint("test-bucket", &endpoint) + .await, + ); + let archive = crate::storage::archive::RepoArchive::new(blob); + state.repo_store = crate::git::repo_store::RepoStore::new(repos_dir, Some(archive), pool); let mut cfg = (*state.config).clone(); cfg.ipfs_request_budget_secs = 1; cfg.git_acquire_timeout_secs = 2; diff --git a/crates/gitlawb-node/src/api/issues.rs b/crates/gitlawb-node/src/api/issues.rs index 0eacfa724..ad4b10234 100644 --- a/crates/gitlawb-node/src/api/issues.rs +++ b/crates/gitlawb-node/src/api/issues.rs @@ -73,10 +73,32 @@ pub async fn create_issue( let create_result = git_issues::create_issue(&disk_path, &issue_id, &json_str); - // Always release the advisory lock — even on error; upload to Tigris only on success. - guard.release(create_result.is_ok()).await; + // Always release the advisory lock — even on error; upload to storage only on success. + let release_result = guard.release(create_result.is_ok()).await; create_result.map_err(|e| AppError::Git(e.to_string()))?; + // A durable-upload failure is recoverable ONLY while the pending-upload + // marker protects the committed mutation (the next successful upload + // re-syncs storage). Verify the marker actually exists before choosing to + // succeed: if the marker write itself also failed, nothing protects the + // mutation from a stale-archive rollback, and the request must fail. + // (Succeeding here avoids non-idempotent retries: a retried create mints + // a second issue UUID and both eventually publish.) + if let Err(e) = release_result { + if state + .repo_store + .pending_marker_exists(&record.owner_did, &record.name) + { + tracing::error!(repo = %record.name, issue = %issue_id, err = %e, + "issue committed locally but durable upload failed — storage re-syncs on next upload"); + } else { + tracing::error!(repo = %record.name, issue = %issue_id, err = %e, + "issue committed locally with NO durable protection — failing the request"); + return Err(AppError::Git(format!( + "issue stored locally but durability could not be guaranteed: {e}" + ))); + } + } // Bump trust score for the issue author — increment current score by 0.05 // (avoids the push_count=0 stuck-at-0.05 bug for agents who only file issues) @@ -249,11 +271,11 @@ pub async fn close_issue( .ok() .and_then(|i| i.author), Ok(None) => { - guard.release(false).await; + let _ = guard.release(false).await; return Err(AppError::NotFound(format!("issue {issue_id} not found"))); } Err(e) => { - guard.release(false).await; + let _ = guard.release(false).await; return Err(AppError::Git(e.to_string())); } }; @@ -262,7 +284,7 @@ pub async fn close_issue( .as_deref() .is_some_and(|a| crate::api::did_matches(&auth.0, a)); if !is_owner && !is_author { - guard.release(false).await; + let _ = guard.release(false).await; return Err(AppError::Forbidden( "only the repo owner or the issue author can close this issue".into(), )); @@ -270,12 +292,29 @@ pub async fn close_issue( let close_result = git_issues::close_issue(&disk_path, &issue_id); - // Always release the advisory lock — even on error; upload to Tigris only on success. - guard.release(close_result.is_ok()).await; + // Always release the advisory lock — even on error; upload to storage only on success. + let release_result = guard.release(close_result.is_ok()).await; let updated = close_result .map_err(|e| AppError::Git(e.to_string()))? .ok_or_else(|| AppError::RepoNotFound(format!("issue {issue_id} not found")))?; + // Recoverable only while the marker protects the committed mutation + // (see create_issue). + if let Err(e) = release_result { + if state + .repo_store + .pending_marker_exists(&record.owner_did, &record.name) + { + tracing::error!(repo = %repo, issue = %issue_id, err = %e, + "issue close committed locally but durable upload failed — storage re-syncs on next upload"); + } else { + tracing::error!(repo = %repo, issue = %issue_id, err = %e, + "issue close committed locally with NO durable protection — failing the request"); + return Err(AppError::Git(format!( + "issue close stored locally but durability could not be guaranteed: {e}" + ))); + } + } let issue: serde_json::Value = serde_json::from_str(&updated) .map_err(|e| AppError::BadRequest(format!("invalid issue data: {e}")))?; @@ -382,7 +421,7 @@ mod lock_pool_shed_tests { // MUST-NOT: with the pool free again the call is not shed as capacity (it // fails later on the nonexistent on-disk repo, which is a git 500). - held.release(false).await; + held.release(false).await.ok(); let admitted = create_issue( State(state.clone()), Extension(AuthenticatedDid(owner.to_string())), @@ -431,7 +470,7 @@ mod lock_pool_shed_tests { let err = shed.expect_err("an exhausted lock pool must fail the call"); assert_sheds_503_with_retry_after(err, "close_issue"); - held.release(false).await; + held.release(false).await.ok(); let admitted = close_issue( State(state.clone()), Extension(AuthenticatedDid(owner.to_string())), diff --git a/crates/gitlawb-node/src/api/pulls.rs b/crates/gitlawb-node/src/api/pulls.rs index 6255ef246..925faf63e 100644 --- a/crates/gitlawb-node/src/api/pulls.rs +++ b/crates/gitlawb-node/src/api/pulls.rs @@ -227,10 +227,31 @@ pub async fn merge_pr( &pr.title, ); - // Always release the advisory lock — even on error; upload to Tigris only on success. - guard.release(merge_result.is_ok()).await; + // Always release the advisory lock — even on error; upload to storage only on success. + let release_result = guard.release(merge_result.is_ok()).await; let merge_sha = merge_result.map_err(|e| AppError::Git(e.to_string()))?; + // A durable-upload failure is recoverable ONLY while the pending-upload + // marker protects the merge commit; verify it exists before choosing to + // proceed (proceeding keeps the DB status consistent with the already + // merged ref, which a retry cannot un-merge). Without the marker nothing + // protects the merge from a stale-archive rollback — fail the request so + // the inconsistency is surfaced instead of silently losable. + if let Err(e) = release_result { + if state + .repo_store + .pending_marker_exists(&record.owner_did, &record.name) + { + tracing::error!(repo = %record.name, pr = %pr.id, err = %e, + "merge committed locally but durable upload failed — storage re-syncs on next upload"); + } else { + tracing::error!(repo = %record.name, pr = %pr.id, err = %e, + "merge committed locally with NO durable protection — failing the request"); + return Err(AppError::Git(format!( + "merge applied locally but durability could not be guaranteed: {e}" + ))); + } + } state.db.merge_pr(&pr.id, &merger_did).await?; let _ = state.db.touch_repo(&record.id).await; @@ -526,7 +547,7 @@ mod lock_pool_shed_tests { // MUST-NOT: with the pool free again the merge is not shed as capacity (it // fails later on the nonexistent on-disk repo, which is a git 500). - held.release(false).await; + held.release(false).await.ok(); let admitted = merge_pr( State(state.clone()), Extension(AuthenticatedDid(owner.to_string())), diff --git a/crates/gitlawb-node/src/api/repos.rs b/crates/gitlawb-node/src/api/repos.rs index 4e327c429..90195ea7c 100644 --- a/crates/gitlawb-node/src/api/repos.rs +++ b/crates/gitlawb-node/src/api/repos.rs @@ -253,17 +253,17 @@ pub async fn create_repo( // Request is admissible — spend the proof now, immediately before the write. let verified_proof = proof.consume(&state.db).await?; - let disk_path = state - .repo_store - .init(&owner_did, &req.name) - .await - .map_err(|e| { - // `{:#}` walks the anyhow chain to the leaf cause; the other git - // handlers log their failures, this one didn't. - tracing::error!(owner = %owner_did, repo = %req.name, err = %format!("{e:#}"), "repo create failed"); - AppError::Git(e.to_string()) - })?; - + // Claim-first ordering: insert the DB row before creating anything durable. + // Within one node's database the row is the claim on (owner, name) — a + // concurrent same-name create loses at the insert with nothing on disk or + // in storage yet. Across nodes (each fly app has its own Postgres) the + // insert arbitrates nothing; the cross-node safety property is that + // failure compensation below only ever touches state THIS attempt created + // (its own row by id, its own local dir) and never deletes a storage key. + // init() publishes under the per-repo advisory lock, so a push arriving + // through the just-visible row serializes behind publication and can + // never be destroyed by this compensation. + let disk_path = store::repo_disk_path(&state.config.repos_dir, &owner_did, &req.name); let now = Utc::now(); let record = crate::db::RepoRecord { id: Uuid::new_v4().to_string(), @@ -278,9 +278,23 @@ pub async fn create_repo( forked_from: None, machine_id: state.machine_id.clone(), }; - state.db.create_repo(&record).await?; + // Create the bare repo locally and publish the initial archive (under the + // advisory lock). On failure, compensate by removing our own just-inserted + // row (keyed by our id) so a retry starts clean; create_published removes + // its local dir itself. + if let Err(e) = state.repo_store.init(&owner_did, &req.name).await { + // `{:#}` walks the anyhow chain to the leaf cause; the other git + // handlers log their failures, this one didn't. + tracing::error!(owner = %owner_did, repo = %req.name, err = %format!("{e:#}"), "repo create failed"); + if let Err(db_err) = state.db.delete_repo_by_id(&record.id).await { + tracing::warn!(repo = %req.name, err = %db_err, + "failed to remove repo row after init failure"); + } + return Err(AppError::Git(e.to_string())); + } + // Persist the proof so it can travel with the repo and a mirroring peer can // re-verify it (enforce-mode origins only; off/shadow yield no proof here). if let Some(p) = verified_proof { @@ -631,10 +645,10 @@ pub async fn git_info_refs( } // Push flood brake on the advertisement phase. A push always hits this - // GET first, and for receive-pack it forces a fresh Tigris download below; + // GET first, and for receive-pack it forces a fresh storage download below; // throttling only the receive-pack POST would leave the expensive // fresh-acquire reachable unauthenticated and unlimited. Applied before the - // acquire so a rejected request does no Tigris work. Same per-IP limiter and + // acquire so a rejected request does no storage work. Same per-IP limiter and // trusted-proxy policy as the POST middleware (shared buckets). if service == "git-receive-pack" { if let Some(key) = crate::rate_limit::client_key(&headers, peer, state.push_limiter_trust) { @@ -692,7 +706,7 @@ pub async fn git_info_refs( git_permit(&state.git_read_semaphore)? }; - // For receive-pack (push), download the latest from Tigris so the client + // For receive-pack (push), download the latest from storage so the client // sees the same refs that acquire_write() will operate on. // // Bound the acquire under `git_acquire_timeout_secs`: the concurrency permit is @@ -2308,7 +2322,7 @@ pub async fn git_receive_pack( } // Always release the advisory lock — even on error — to prevent stale locks - // from blocking subsequent pushes. Only upload to Tigris when the push + // from blocking subsequent pushes. Only upload to storage when the push // succeeded; uploading a half-applied repo would propagate corruption. // Reclaim the write lock from the shared cell (#173 F2). This is only reachable // once `receive_pack` has returned, so the admission guard's copy can only ever @@ -2319,12 +2333,74 @@ pub async fn git_receive_pack( .expect("repo write-lock mutex poisoned") .take() .expect("the write lock is only taken here, and only once"); - reclaimed.release(push_succeeded).await; + // `Some` = the guard still needs a synchronous (strict) release; taken by + // the write-back path only once its intent marker is durably on disk. + let mut strict_guard = Some(reclaimed); + if push_succeeded && state.config.async_upload { + let guard = strict_guard.take().expect("guard present before release"); + // Write-back: ack the client now; the durable upload to object storage + // and the advisory-lock release run in the background. The lock is held + // until the upload finishes, so a concurrent writer on another machine + // can't observe a stale archive. If this detached task is cancelled by + // runtime shutdown mid-upload, the guard's lock connection is closed + // rather than repooled, so Postgres frees the advisory lock (see + // `LockedConn`). Durability tradeoff: if the upload fails (or the node + // stops first), storage stays stale until this repo's next successful + // upload. The persisted pending-upload marker keeps the local copy + // authoritative on THIS node in that window — no access rolls it back + // to the stale archive — but other nodes still serve the stale archive + // until the re-upload lands. Hence async_upload is opt-in. + // + // The intent marker must be on disk BEFORE the ack: the spawned task + // may never be polled if the process stops right after the response, + // and without the marker a restart would treat the stale storage + // archive as newer and roll the acked push back. If the marker itself + // cannot be persisted, do NOT ack early — fall back to the strict + // upload-before-ack path below. + match guard.mark_pending().await { + Ok(()) => { + let repo_label = name.to_string(); + tokio::spawn(async move { + if let Err(e) = guard.release(true).await { + tracing::error!(repo = %repo_label, err = %e, + "write-back durable upload failed after push was acked"); + } + }); + } + Err(e) => { + tracing::warn!(repo = %name, err = %e, + "pending-upload marker write failed — falling back to strict upload-before-ack"); + strict_guard = Some(guard); + } + } + } + if let Some(guard) = strict_guard { + // Strict path (failed push, async_upload off, or marker write failure): + // upload-before-ack. + if let Err(e) = guard.release(push_succeeded).await { + if push_succeeded { + // A successful push whose durable upload then failed — the + // client must know the push is not durably stored. The lease + // drops on this return, and the advisory lock was already + // released inside `release`. + tracing::error!(repo = %name, err = %e, "durable upload failed after push"); + return Err(AppError::Git(format!( + "push applied locally but durable upload to storage failed: {e}" + ))); + } + // The push itself failed; log the release error but fall through + // so the real git failure (below) is what the client sees. + tracing::error!(repo = %name, err = %e, "lock release failed after failed push"); + } + } // Clean path: clone (a) already dropped inside run_git_service when the receive-pack - // group was reaped; clone (b) held here spanned the success-only Tigris upload that - // ran inside release() above. Drop it now so a second same-repo push proceeds the - // moment this write is durable, rather than at end of the (longer) handler tail. On - // the disconnect path this line is never reached: clone (a) rides the reaper (F3). + // group was reaped; clone (b) held here spanned the success-only storage upload that + // ran inside release() above (or, under async_upload, only the marker write — the + // upload continues under the advisory lock, which is what keeps a second push from + // reading a stale archive). Drop it now so a second same-repo push proceeds the + // moment this write is durable (or acked), rather than at end of the (longer) + // handler tail. On the disconnect path this line is never reached: clone (a) rides + // the reaper (F3). drop(lease); let result = receive_result.map_err(|e| { @@ -3057,7 +3133,7 @@ pub async fn fork_repo( // Request is admissible — spend the proof now, immediately before the write. let verified_proof = proof.consume(&state.db).await?; - // Ensure source repo is on local disk (downloads from Tigris on cache miss) + // Ensure source repo is on local disk (downloads from storage on cache miss) let source_path = state .repo_store .acquire(&source.owner_did, &source.name) @@ -3066,30 +3142,14 @@ pub async fn fork_repo( let disk_path = store::repo_disk_path(&state.config.repos_dir, &forker_did, &fork_name); - // Clone the source repo as a mirror - let output = std::process::Command::new("git") - .args([ - "clone", - "--mirror", - source_path.to_str().unwrap_or(""), - disk_path.to_str().unwrap_or(""), - ]) - .output() - .map_err(|e| AppError::Git(format!("git clone --mirror failed: {e}")))?; - - if !output.status.success() { - let stderr = String::from_utf8_lossy(&output.stderr); - return Err(AppError::Git(format!( - "git clone --mirror failed: {stderr}" - ))); - } - - // Upload fork to Tigris - state - .repo_store - .release_after_write(&forker_did, &fork_name) - .await; - + // Claim-first ordering: insert the DB row before cloning or uploading + // anything. Within one node's database the row is the claim on (owner, + // name) — a concurrent same-name fork loses at the insert with nothing on + // disk or in storage. Across nodes (per-node Postgres) the insert + // arbitrates nothing; the cross-node safety properties are that + // publication runs under the per-repo advisory lock (below) and that the + // failure compensation only ever removes state THIS attempt created (its + // own row by id, its own clone dir) — never a storage archive. let now = Utc::now(); let record = crate::db::RepoRecord { id: Uuid::new_v4().to_string(), @@ -3104,9 +3164,42 @@ pub async fn fork_repo( forked_from: Some(source.id.clone()), machine_id: state.machine_id.clone(), }; - state.db.create_repo(&record).await?; + // The whole clone-and-publish lifecycle runs under the per-repo advisory + // lock inside `create_published`: pushes to the just-visible row serialize + // on the same lock, so none can execute (let alone be destroyed by the + // compensation below) until publication has succeeded or been unwound. + // On failure, compensation removes only what THIS attempt created: its + // clone dir (inside create_published) and its own row, keyed by our + // generated id. + let clone_result = state + .repo_store + .create_published(&forker_did, &fork_name, |dest| { + let output = std::process::Command::new("git") + .args([ + "clone", + "--mirror", + source_path.to_str().unwrap_or(""), + dest.to_str().unwrap_or(""), + ]) + .output() + .map_err(|e| anyhow::anyhow!("git clone --mirror failed: {e}"))?; + if !output.status.success() { + let stderr = String::from_utf8_lossy(&output.stderr); + anyhow::bail!("git clone --mirror failed: {stderr}"); + } + Ok(()) + }) + .await; + if let Err(e) = clone_result { + if let Err(db_err) = state.db.delete_repo_by_id(&record.id).await { + tracing::warn!(record_id = %record.id, err = %db_err, + "failed to remove fork row after fork error"); + } + return Err(AppError::Git(format!("fork failed: {e}"))); + } + // Persist the proof so the fork carries it when it propagates to peers. if let Some(p) = verified_proof { if let Err(e) = p.record_for_repo(&state.db, &record.id).await { @@ -4693,7 +4786,7 @@ mod tests { /// The receive-pack *advertisement* (`GET info/refs?service=git-receive-pack`) /// must be throttled by the per-IP push limiter BEFORE it does the fresh - /// Tigris acquire — otherwise the flood brake on the POST is bypassable via + /// storage acquire — otherwise the flood brake on the POST is bypassable via /// the cheaper unauthenticated GET (PR #152 review P1). Pre-filling the /// bucket makes the assertion deterministic and keeps the test off the /// acquire path entirely. @@ -4732,7 +4825,7 @@ mod tests { assert_eq!( status, StatusCode::TOO_MANY_REQUESTS, - "receive-pack advertisement must be throttled before the Tigris acquire" + "receive-pack advertisement must be throttled before the storage acquire" ); } @@ -6395,7 +6488,7 @@ mod tests { // observes the same counter at 1: without it, a zero here would pass on any build // where an upload is simply impossible. assert_eq!( - state.repo_store.tigris_upload_site_reached(), + state.repo_store.storage_upload_site_reached(), 0, "a push interrupted by a client disconnect must not reach the Tigris upload \ site: publishing a half-applied repo propagates it to every node that later \ @@ -6500,7 +6593,7 @@ mod tests { // not at least once: a retried exec race releases with success = false and must // not count. assert_eq!( - state.repo_store.tigris_upload_site_reached(), + state.repo_store.storage_upload_site_reached(), 1, "a completed push must reach the Tigris upload site once" ); @@ -6565,7 +6658,7 @@ mod tests { // MUST-NOT: with the pool free again, the push is not shed as capacity (it fails // later on the nonexistent on-disk repo, which is a git error, not Overloaded). - held.release(false).await; + held.release(false).await.ok(); let admitted = git_receive_pack( State(state.clone()), Path((owner.to_string(), name.to_string())), diff --git a/crates/gitlawb-node/src/config.rs b/crates/gitlawb-node/src/config.rs index 1fbf4376e..1beb42e55 100644 --- a/crates/gitlawb-node/src/config.rs +++ b/crates/gitlawb-node/src/config.rs @@ -172,11 +172,50 @@ pub struct Config { #[arg(long, env = "GITLAWB_HEARTBEAT_INTERVAL_HOURS", default_value_t = 20)] pub heartbeat_interval_hours: u64, - /// Tigris (S3-compatible) bucket for repo storage. + /// Tigris (S3-compatible) bucket for repo storage. Legacy alias for + /// `s3_bucket` — still honoured so existing deployments keep working. /// Leave empty to disable Tigris and use local-only storage. #[arg(long, env = "GITLAWB_TIGRIS_BUCKET", default_value = "")] pub tigris_bucket: String, + /// Object-storage backend: `s3` (any S3-compatible service), `fs` (local + /// directory), or `ipfs` (Kubo MFS). Empty = auto-detect: `s3` when a bucket + /// is set, else `fs` when a storage dir is set, else local-only. `ipfs` is + /// never auto-selected (`GITLAWB_IPFS_API` alone keeps its pinning-only + /// meaning) — set this explicitly to `ipfs` to opt in, and note the ipfs + /// backend requires ALL nodes to share one Kubo instance (MFS is + /// daemon-local, not shared network storage). + #[arg(long, env = "GITLAWB_STORAGE_BACKEND", default_value = "")] + pub storage_backend: String, + + /// Bucket for the `s3` backend (Tigris, R2, AWS S3, MinIO, B2). Falls back to + /// `tigris_bucket` when empty. + #[arg(long, env = "GITLAWB_S3_BUCKET", default_value = "")] + pub s3_bucket: String, + + /// Endpoint URL override for the `s3` backend (e.g. R2/MinIO). On Tigris/Fly + /// the endpoint is auto-provided via `AWS_ENDPOINT_URL_S3`, so leave empty. + #[arg(long, env = "GITLAWB_S3_ENDPOINT", default_value = "")] + pub s3_endpoint: String, + + /// Force path-style S3 addressing (required by MinIO and some S3-compatibles). + #[arg(long, env = "GITLAWB_S3_FORCE_PATH_STYLE", default_value_t = false)] + pub s3_force_path_style: bool, + + /// Directory for the `fs` (local filesystem) storage backend. + #[arg(long, env = "GITLAWB_STORAGE_FS_DIR", default_value = "")] + pub storage_fs_dir: String, + + /// Acknowledge a push to the client before the durable upload to object + /// storage finishes (write-back). Lowers push latency, but opens a + /// durability window: if the upload fails or the node stops first, storage + /// stays stale until the next successful upload. A persisted pending-upload + /// marker keeps the local copy authoritative on this node in that window + /// (no rollback of the acked push), but other nodes serve the stale archive + /// until the re-upload lands. Off by default (strict upload-before-ack). + #[arg(long, env = "GITLAWB_ASYNC_UPLOAD", default_value_t = false)] + pub async_upload: bool, + /// Maximum pack body size for git-receive-pack and git-upload-pack, in bytes. /// Applies only to git smart-HTTP routes — all other API routes keep the 2 MB default. /// Default: 2 GB. Set lower on resource-constrained nodes. diff --git a/crates/gitlawb-node/src/db/mod.rs b/crates/gitlawb-node/src/db/mod.rs index cc2cf0bd7..8b48749e6 100644 --- a/crates/gitlawb-node/src/db/mod.rs +++ b/crates/gitlawb-node/src/db/mod.rs @@ -1239,6 +1239,17 @@ impl Db { Ok(()) } + /// Compensating delete for creation flows: remove the row a failed + /// create/fork just inserted, keyed strictly by our own generated id so a + /// concurrent same-name winner's row can never be affected. + pub async fn delete_repo_by_id(&self, id: &str) -> Result<()> { + sqlx::query("DELETE FROM repos WHERE id = $1") + .bind(id) + .execute(&self.pool) + .await?; + Ok(()) + } + /// Register a mirrored repo from a peer in the local DB so git smart HTTP can serve it. /// Uses INSERT OR IGNORE (SQLite) / ON CONFLICT DO NOTHING (Postgres) so it's idempotent. pub async fn upsert_mirror_repo( diff --git a/crates/gitlawb-node/src/git/mod.rs b/crates/gitlawb-node/src/git/mod.rs index 59e34c843..eec7f23d3 100644 --- a/crates/gitlawb-node/src/git/mod.rs +++ b/crates/gitlawb-node/src/git/mod.rs @@ -3,5 +3,4 @@ pub mod push_delta; pub mod repo_store; pub mod smart_http; pub mod store; -pub mod tigris; pub mod visibility_pack; diff --git a/crates/gitlawb-node/src/git/repo_store.rs b/crates/gitlawb-node/src/git/repo_store.rs index 458207466..5cdd23cfd 100644 --- a/crates/gitlawb-node/src/git/repo_store.rs +++ b/crates/gitlawb-node/src/git/repo_store.rs @@ -1,14 +1,17 @@ -//! Centralized repo storage layer — local disk cache backed by Tigris (S3). +//! Centralized repo storage layer — local disk cache backed by a pluggable +//! object store (S3-compatible / filesystem / IPFS) via [`RepoArchive`]. //! //! Every handler that needs access to a git repo on disk goes through `RepoStore`: //! -//! - `acquire()` — ensures the repo is on local disk (downloads from Tigris on cache miss). -//! - `release_after_write()` — uploads the updated repo to Tigris after a write operation. -//! - `init()` — creates a new bare repo locally and uploads to Tigris. +//! - `acquire()` — ensures the repo is on local disk (downloads on cache miss). +//! - `acquire_write()` — write lock + ensures local matches storage (skips the +//! download when the cached etag already matches — the push-latency win). +//! - `release()` — upload the updated repo to storage and free the write lock. +//! - `init()` — creates a new bare repo locally and uploads to storage. //! -//! When Tigris is disabled (bucket empty), this is a simple passthrough to local disk. +//! When no backend is configured, this is a simple passthrough to local disk. -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Duration; @@ -18,45 +21,52 @@ use sqlx::pool::PoolConnection; use sqlx::postgres::PgPoolOptions; use sqlx::{PgPool, Postgres}; use tokio::sync::Mutex; -use tracing::{debug, info, warn}; +use tracing::{debug, warn}; use super::store; -use super::tigris::TigrisClient; +use crate::storage::archive::RepoArchive; -/// Centralized repo storage: local disk cache + optional Tigris backend. +/// Centralized repo storage: local disk cache + optional object-storage backend +/// (S3-compatible / filesystem / IPFS) behind the [`RepoArchive`] layer. #[derive(Clone)] pub struct RepoStore { repos_dir: PathBuf, - tigris: Option, - /// Dedicated Postgres pool for repo write advisory locks, built by - /// `build_lock_pool` (see there for why it is separate and why it carries an - /// `after_release` hook). Never use this for ordinary queries. + archive: Option, + /// Bounded pool dedicated to advisory-lock connections, built by + /// `build_lock_pool` (see there for the `after_release` cancellation + /// backstop and why it is separate from the handler pool). A push pins one + /// connection while it HOLDS the lock (across receive-pack and the + /// upload); WAITING for a contended lock occupies nothing — see + /// `LockedConn::acquire` (#173 F1). Never use this pool for ordinary + /// queries. lock_pool: PgPool, - /// Tracks repos already confirmed to exist in Tigris — avoids redundant + /// Tracks repos already confirmed to exist in storage — avoids redundant /// HEAD checks and background uploads for repos we've already migrated. migrated: Arc>>, - /// Test-only stall injected at the head of `acquire_write`'s Tigris phase, + /// Last-known archive etag per `owner_slug/repo` key. Lets a write skip the + /// pre-write download when our local copy already matches storage (the + /// common case under sticky routing) — the main push-latency win. + versions: Arc>>, + /// Test-only stall injected at the head of `acquire_write`'s storage phase, /// i.e. AFTER the advisory lock is taken and BEFORE the guard exists. That /// window is exactly where the outer `tokio::time::timeout` in - /// `api/repos.rs` can drop the future (#173). `TigrisClient` takes its - /// endpoint from process-wide AWS env vars and has no injectable seam, so - /// this flag is the smallest way to hold a real `acquire_write` open in that - /// window and cancel it there. + /// `api/repos.rs` can drop the future (#173). The S3 client takes its + /// endpoint from process-wide AWS env vars, so this flag is the smallest + /// way to hold a real `acquire_write` open in that window and cancel it + /// there. #[cfg(test)] - tigris_stall: Option, - /// Test-only counter of how many times a write guard from this store REACHED the - /// Tigris upload site in `release` (the point past the `success` check, where a - /// configured client would be uploaded to). It counts the decision, not a network - /// call: `TigrisClient` takes its endpoint from process-wide AWS env vars and has no - /// injectable seam, so every test runs with `tigris: None` and a counter inside the - /// `Some` arm could never move. Reaching the site is the property under test anyway: - /// an interrupted push must not publish a half-applied repo, and the disconnect path - /// must therefore never get here (#173 F2). + storage_stall: Option, + /// Test-only counter of how many times a write guard from this store REACHED + /// the storage upload site in `release` (the decision point past the + /// `success` check). It counts the decision, not a network call, so it moves + /// even when the store has no backend configured; reaching the site at all + /// is the property under test — an interrupted push must not publish a + /// half-applied repo (#173 F2). /// - /// Per store rather than a process global, so cases running in parallel do not see - /// each other's uploads, and an `Arc` rather than a `thread_local` because the guard - /// is released from a detached task on another worker thread. Same test-only counter - /// idiom as `ipfs_pin::note_legacy_repair_read`. + /// Per store rather than a process global, so cases running in parallel do + /// not see each other's uploads, and an `Arc` rather than a `thread_local` + /// because the guard is released from a detached task on another worker + /// thread. Same test-only counter idiom as `ipfs_pin::note_legacy_repair_read`. #[cfg(test)] upload_site_reached: Arc, /// Test-only seam: armed here, copied into every `RepoWriteGuard` this store @@ -98,31 +108,34 @@ impl RepoStore { &self.lock_pool } - /// Test-only: see `tigris_stall`. + /// Test-only: see `storage_stall`. #[cfg(test)] - pub fn with_tigris_stall(mut self, stall: Duration) -> Self { - self.tigris_stall = Some(stall); + pub fn with_storage_stall(mut self, stall: Duration) -> Self { + self.storage_stall = Some(stall); self } - /// Test-only: how many write guards from this store have reached the Tigris upload - /// site. See [`RepoStore::upload_site_reached`]. + /// Test-only: how many write guards from this store have reached the storage + /// upload site. See [`RepoStore::upload_site_reached`]. #[cfg(test)] - pub fn tigris_upload_site_reached(&self) -> usize { + pub fn storage_upload_site_reached(&self) -> usize { self.upload_site_reached .load(std::sync::atomic::Ordering::SeqCst) } - /// `lock_pool` must come from `build_lock_pool`; a plain pool leaks advisory - /// locks on cancellation. - pub fn new(repos_dir: PathBuf, tigris: Option, lock_pool: PgPool) -> Self { + /// `lock_pool` must come from `build_lock_pool`: its `after_release` hook is + /// the cancellation backstop behind `LockedConn`'s connection-affinity + /// discipline, and a plain pool would leak advisory locks on paths that + /// repool a connection. + pub fn new(repos_dir: PathBuf, archive: Option, lock_pool: PgPool) -> Self { Self { repos_dir, - tigris, + archive, lock_pool, migrated: Arc::new(Mutex::new(HashSet::new())), + versions: Arc::new(Mutex::new(HashMap::new())), #[cfg(test)] - tigris_stall: None, + storage_stall: None, #[cfg(test)] upload_site_reached: Arc::new(std::sync::atomic::AtomicUsize::new(0)), #[cfg(test)] @@ -130,65 +143,269 @@ impl RepoStore { } } - /// Ensure a repo is available on local disk, downloading from Tigris if needed. - /// If the repo exists locally but not yet in Tigris, a background upload is - /// spawned to lazily migrate it (on-demand migration for pre-Tigris repos). + /// Ensure the local copy matches storage, skipping the download when our + /// cached etag already equals the current archive etag. + /// + /// `require_fresh` selects the failure policy: + /// - `false` (read path, `acquire_fresh`): self-heal — if a storage HEAD or + /// download fails but a valid local copy exists, use it; a later upload + /// re-syncs storage. + /// - `true` (write path, `acquire_write`): fail closed — never fall back to + /// a possibly-stale local copy. The remote etag differs (remote is newer), + /// so uploading our stale copy after the write would clobber it (lost + /// update). Propagate the error so the write is rejected instead. + async fn sync_down_if_stale( + &self, + owner_slug: &str, + repo_name: &str, + local_path: &Path, + require_fresh: bool, + ) -> Result<()> { + let Some(ref archive) = self.archive else { + return Ok(()); + }; + + // The repo path arrived as a parameter, so re-establish the traversal + // barrier here, where the filesystem work happens, rather than relying + // on the caller having built it through `validated_repo_disk_path`. See + // `validated_repo_path_in`. + let local_path = &validated_repo_path_in(&self.repos_dir, local_path)?; + + let marker = pending_upload_marker(local_path)?; + // try_exists, not exists(): a transient EACCES/EIO must not read as + // "no marker" — that path downloads and can roll back a pending local + // write. Fail the write path closed; treat as present on the read path. + let marker_present = match marker.try_exists() { + Ok(present) => present, + Err(e) => { + if require_fresh { + return Err(e).context("probing pending-upload marker"); + } + warn!(repo = %repo_name, err = %e, + "pending-upload marker probe failed — treating as present"); + true + } + }; + if marker_present { + if local_path.exists() { + // The local copy has a write that storage never received (its + // upload failed, or the node stopped first). The marker records + // the storage etag that write was BASED on — and, if an upload + // was in flight, the etag it was going to produce — so we can + // tell "storage unchanged — local strictly ahead" and "that's + // our own completed upload" apart from "another node advanced + // storage — genuine divergence". + let pm = read_pending_marker(local_path); + let remote = match archive.head_etag(owner_slug, repo_name).await { + Ok(r) => r, + Err(e) => { + if require_fresh { + return Err(e).context("storage head while local pending upload"); + } + warn!(repo = %repo_name, err = %e, + "storage head failed while pending upload — using local copy"); + return Ok(()); + } + }; + match remote.as_deref() { + // Storage empty, or exactly the version our write built on: + // local is strictly ahead. Serve it; the next successful + // post-write upload re-syncs storage and clears the marker. + None => return Ok(()), + Some(r) if pm.matches_base(r) => { + warn!(repo = %repo_name, + "local copy ahead of storage (pending upload) — skipping download"); + return Ok(()); + } + Some(r) => { + // Unexplained remote: our own interrupted upload, or a + // genuine external writer. Validate by CONTENT — fetch + // the remote bytes and compare their MD5 to the + // marker's recorded in-flight hash. Never trust the + // backend etag for this: etag semantics vary (IPFS + // CIDs, SSE-KMS), and the fs backend can crash between + // publishing its etag and its bytes. + if self + .remote_matches_inflight(archive, owner_slug, repo_name, &pm) + .await + { + debug!(repo = %repo_name, + "storage content matches our own in-flight upload — marker cleared, synced"); + self.versions + .lock() + .await + .insert(format!("{owner_slug}/{repo_name}"), r.to_string()); + clear_pending_upload_after_success(local_path, Some(r)); + return Ok(()); + } + // Storage advanced past our base while this node held + // un-uploaded local changes: both sides have writes + // the other lacks. Overwriting either loses a push. + if require_fresh { + anyhow::bail!( + "storage for {owner_slug}/{repo_name} advanced while local \ + changes were pending upload — refusing to overwrite either \ + side; reconcile manually (fetch both, merge, remove the \ + pending-upload marker)" + ); + } + warn!(repo = %repo_name, + "storage diverged from pending local copy — serving local for read"); + return Ok(()); + } + } + } + // Marker without a local copy: the repo dir was removed out from + // under us, so the storage copy is the best remaining state. Drop + // the stale marker and fall through to the normal download. + let _ = std::fs::remove_file(&marker); + } + let key = format!("{owner_slug}/{repo_name}"); + + let remote_etag = match archive.head_etag(owner_slug, repo_name).await { + Ok(Some(etag)) => etag, + Ok(None) => return Ok(()), // not in storage yet — local is authoritative + Err(e) => { + // HEAD failed. Read path: fall back to a valid local copy if we + // have one. Write path: fail closed (see `require_fresh`). + if !require_fresh && local_path.exists() { + warn!(repo = %repo_name, err = %e, "storage head failed — using local copy"); + return Ok(()); + } + return Err(e).context("storage head before access"); + } + }; + + if local_path.exists() { + let known = self.versions.lock().await.get(&key).cloned(); + if known.as_deref() == Some(remote_etag.as_str()) { + debug!(repo = %repo_name, "local copy current (etag match) — skipping download"); + return Ok(()); + } + } + + // KNOWN LIMITATION (pre-dates this layer): read-path downloads and + // their swap-into-place are not serialized against the advisory write + // lock, so a slow in-flight download decided before a push began can + // swap a stale tree under a running receive-pack on the same node. + // Requires a cache-miss/stale read racing a same-repo write; the + // follow-up is to serialize download+swap with writers. + match archive.download(owner_slug, repo_name, local_path).await { + Ok(()) => { + self.versions.lock().await.insert(key, remote_etag); + Ok(()) + } + Err(e) => { + // Read path self-heal only: a corrupt/unreadable archive must not + // block access when a valid local copy exists. On the write path + // the remote etag differs (remote is newer), so falling back and + // later uploading our stale copy would clobber it — fail closed. + if !require_fresh && local_path.exists() { + warn!(repo = %repo_name, err = %e, + "archive download failed — falling back to local copy"); + Ok(()) + } else { + Err(e).context("downloading repo archive") + } + } + } + } + + /// Validate a heal candidate by CONTENT: does storage hold exactly the + /// bytes this node's interrupted upload was sending? Fetches the remote + /// object and compares its MD5 to the marker's recorded in-flight hash. + /// Deliberately never compares against the backend etag — etag semantics + /// vary (IPFS CIDs, SSE-KMS etags are not content MD5s) and the fs + /// backend can crash between publishing its etag and its bytes. A full + /// GET on this recovery-only path is an acceptable price for a check + /// that cannot false-positive on stale bytes. + async fn remote_matches_inflight( + &self, + archive: &RepoArchive, + owner_slug: &str, + repo_name: &str, + pm: &PendingMarker, + ) -> bool { + let Some(ref inflight) = pm.inflight else { + return false; + }; + match archive.fetch_raw(owner_slug, repo_name).await { + Ok(Some(bytes)) => { + norm_etag(&crate::storage::archive::content_md5_hex(&bytes)) == norm_etag(inflight) + } + _ => false, + } + } + + /// Ensure a repo is available on local disk, downloading from storage if needed. + /// If the repo exists locally but not yet in storage, a background upload is + /// spawned to lazily migrate it (on-demand migration for pre-storage repos). /// Returns the local path to the bare repo. pub async fn acquire(&self, owner_did: &str, repo_name: &str) -> Result { let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; // Fast path: repo exists locally if local_path.exists() { - // Lazy migration: if Tigris is enabled and we haven't confirmed this - // repo is in Tigris yet, check and upload in the background. - if let Some(ref tigris) = self.tigris { + // Lazy migration: if storage is enabled and we haven't confirmed this + // repo is in storage yet, check and upload in the background. + if self.archive.is_some() { let key = format!("{owner_slug}/{repo_name}"); let already_migrated = self.migrated.lock().await.contains(&key); - if !already_migrated { - let tigris = tigris.clone(); + // A pending-upload marker means the marker machinery already + // owns this repo's next upload (next write, or the startup + // retry). Migration must not steal it: `upload_under_lock` + // knows nothing about markers, so its upload would strand the + // marker with a base that no longer matches storage, wedging + // the repo's writes on a spurious divergence. + // A path the barrier rejects cannot carry a marker we wrote, so + // it reads as "not pending" and migration proceeds under the + // lock exactly as it would for an unmarked repo. + let marker_pending = + pending_upload_marker(&local_path).is_ok_and(|marker| marker.exists()); + if !already_migrated && !marker_pending { + let this = self.clone(); let slug = owner_slug.clone(); let name = repo_name.to_string(); let path = local_path.clone(); - let migrated = Arc::clone(&self.migrated); + let key = key.clone(); tokio::spawn(async move { - // Check if already in Tigris before uploading - match tigris.exists(&slug, &name).await { - Ok(true) => { - debug!(repo = %name, "repo already in tigris — skipping migration"); - } - Ok(false) => { - info!(repo = %name, "migrating local repo to tigris"); - if let Err(e) = tigris.upload(&slug, &name, &path).await { - warn!(repo = %name, err = %e, "lazy migration to tigris failed"); - return; - } - info!(repo = %name, "lazy migration to tigris complete"); + // Upload under the advisory lock (skip if already present) + // so this opportunistic migration can't clobber a + // concurrent locked push by landing a stale snapshot. + match this.upload_under_lock(&slug, &name, &path, true).await { + Ok(()) => { + this.migrated.lock().await.insert(key); + debug!(repo = %name, "lazy migration to storage complete (or already present)"); } Err(e) => { - warn!(repo = %name, err = %e, "tigris existence check failed"); - return; + warn!(repo = %name, err = %e, "lazy migration to storage failed"); } } - migrated.lock().await.insert(format!("{slug}/{name}")); }); } } return Ok(local_path); } - // Try downloading from Tigris - if let Some(ref tigris) = self.tigris { - if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) { - debug!(repo = %repo_name, "cache miss — downloading from tigris"); - tigris + // Try downloading from storage + if let Some(ref archive) = self.archive { + if let Some(remote_etag) = archive + .head_etag(&owner_slug, repo_name) + .await + .context("checking storage for repo")? + { + debug!(repo = %repo_name, "cache miss — downloading from storage"); + archive .download(&owner_slug, repo_name, &local_path) .await - .context("downloading repo from tigris")?; - // Mark as migrated since we just downloaded it - self.migrated - .lock() - .await - .insert(format!("{owner_slug}/{repo_name}")); + .context("downloading repo from storage")?; + // The local copy didn't exist, so any pending-upload marker + // here is stale litter — clear it or it would wrongly pin the + // just-downloaded copy as "ahead of storage". + clear_pending_upload(&local_path); + let key = format!("{owner_slug}/{repo_name}"); + self.migrated.lock().await.insert(key.clone()); + self.versions.lock().await.insert(key, remote_etag); return Ok(local_path); } } @@ -198,34 +415,14 @@ impl RepoStore { Ok(local_path) } - /// Ensure a repo is available on local disk with the **latest** Tigris state. + /// Ensure a repo is available on local disk with the **latest** storage state. /// Use this for operations that precede a write (e.g. `info/refs` for /// `git-receive-pack`) so the client sees the same refs that `acquire_write()` /// will operate on. pub async fn acquire_fresh(&self, owner_did: &str, repo_name: &str) -> Result { let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; - - if let Some(ref tigris) = self.tigris { - if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) { - debug!(repo = %repo_name, "acquire_fresh: downloading latest from tigris"); - if let Err(e) = tigris.download(&owner_slug, repo_name, &local_path).await { - // The Tigris archive is present (HEAD ok) but unreadable — a - // corrupt/partial upload, or a transient GET failure. If we have a - // valid local copy, proceed with it rather than blocking the write; - // the post-write upload re-syncs (self-heals) Tigris. Only hard-fail - // when there is no local copy to fall back to. - if local_path.exists() { - warn!(repo = %repo_name, err = %e, - "acquire_fresh: tigris download failed — falling back to local copy"); - return Ok(local_path); - } - return Err(e).context("downloading repo from tigris (fresh)"); - } - return Ok(local_path); - } - } - - // Tigris disabled or repo not in Tigris — fall back to local + self.sync_down_if_stale(&owner_slug, repo_name, &local_path, false) + .await?; Ok(local_path) } @@ -260,97 +457,41 @@ impl RepoStore { /// accepted-window path. pub async fn acquire_write(&self, owner_did: &str, repo_name: &str) -> Result { let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; - let lock_key = advisory_lock_key(&owner_slug, repo_name); - - // Acquire the Postgres advisory lock with retry, using pg_try_advisory_lock so a - // stale lock from a crashed connection can't block us indefinitely. - // - // The connection is checked out INSIDE the loop and RETURNED before each sleep. - // Only the connection that actually took the lock is retained. Two constraints - // pull in opposite directions here, and this is what satisfies both: - // - // * Session ownership. A session-level advisory lock belongs to the CONNECTION - // that took it, so the lock and its `pg_advisory_unlock` must run on the same - // one. Running them through the pool (`fetch_one(&self.pool)`) lets them land - // on different connections: the unlock silently returns false and the lock - // leaks, while a competing acquire that happens to draw the holding - // connection re-enters the lock and two pushes to one repo run concurrently. - // Hence: keep the connection that WON. - // * Occupancy. Holding a connection across the ~60 one-second sleeps would let - // one spinning acquire park a lock-pool connection for a minute. That is not - // just a push-path concern: `api/issues.rs` and `api/pulls.rs` reach - // acquire_write holding no concurrency permit at all, so a caller could park - // the whole pool and starve authenticated pushes on every repo (#173 F1). - // Hence: return the connection when we LOSE, before sleeping. - // - // Returning a losing connection is safe with respect to the cancellation design: - // `after_release` runs `pg_advisory_unlock_all()`, a no-op on a connection that - // took nothing, so it cannot disturb a lock held by any other connection - // (proven by `returning_an_unlocked_connection_does_not_clear_another_connections_lock`). - // - // Cancellation safety is unchanged: the future can only be dropped while a - // connection is checked out, and dropping it runs the same `after_release` hook, - // which clears whatever lock it had just taken (#173 U1). - let mut lock_conn = None; - for attempt in 0..60 { - let mut conn = self.lock_pool.acquire().await.map_err(|e| { - anyhow::Error::new(LockPoolBusy) - .context(format!("checking out a lock-pool connection: {e}")) - })?; - let row: (bool,) = sqlx::query_as("SELECT pg_try_advisory_lock($1)") - .bind(lock_key) - .fetch_one(&mut *conn) - .await - .context("trying advisory lock")?; - if row.0 { - lock_conn = Some(conn); - break; - } - // Lost the race: give the connection back so a spinning acquire occupies - // nothing while it waits. - drop(conn); - if attempt < 59 { - tokio::time::sleep(std::time::Duration::from_secs(1)).await; - } - } - let Some(lock_conn) = lock_conn else { - anyhow::bail!("could not acquire advisory lock after 60s — possible stale lock for {owner_slug}/{repo_name}"); - }; + let label = format!("{owner_slug}/{repo_name}"); + let lock = LockedConn::acquire( + &self.lock_pool, + advisory_lock_key(&owner_slug, repo_name), + &label, + ) + .await?; #[cfg(test)] - if let Some(stall) = self.tigris_stall { + if let Some(stall) = self.storage_stall { tokio::time::sleep(stall).await; } - // Always download the latest from Tigris before writing. Local disk may be - // stale if another machine pushed since our last access. The lock connection - // is already held, so a cancellation here returns it through `after_release`, - // which clears the lock. - if let Some(ref tigris) = self.tigris { - if tigris.exists(&owner_slug, repo_name).await.unwrap_or(false) { - debug!(repo = %repo_name, "write acquire: downloading latest from tigris"); - if let Err(e) = tigris.download(&owner_slug, repo_name, &local_path).await { - // Same self-healing fallback as acquire_fresh: a corrupt/unreadable - // Tigris archive must not block a write when a valid local copy - // exists — release(success) will re-upload a good archive. - if local_path.exists() { - warn!(repo = %repo_name, err = %e, - "write acquire: tigris download failed — falling back to local copy"); - } else { - return Err(e).context("downloading repo from tigris for write"); - } - } - } + // Ensure local matches the latest in storage before writing. The etag + // cache skips the full download when our copy is already current (the + // common single-machine case under sticky routing); a stale copy (another + // machine pushed since) still triggers a download. The advisory lock above + // serializes this so the post-write upload can't race a concurrent writer. + // A cancellation anywhere in here drops `lock`, whose backstop frees the + // advisory lock (see `LockedConn`). + if let Err(e) = self + .sync_down_if_stale(&owner_slug, repo_name, &local_path, true) + .await + { + lock.unlock().await; + return Err(e); } Ok(RepoWriteGuard { owner_slug, repo_name: repo_name.to_string(), local_path, - lock_key, - lock_conn: Some(lock_conn), - released: false, - tigris: self.tigris.clone(), + lock, + archive: self.archive.clone(), + versions: Arc::clone(&self.versions), #[cfg(test)] upload_site_reached: Arc::clone(&self.upload_site_reached), #[cfg(test)] @@ -358,43 +499,348 @@ impl RepoStore { }) } - /// Initialize a new bare repo on local disk and upload to Tigris. + /// Initialize a new bare repo on local disk and publish it to storage. pub async fn init(&self, owner_did: &str, repo_name: &str) -> Result { - let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; + self.create_published(owner_did, repo_name, |path| { + store::init_bare(path).context("initializing bare repo") + }) + .await + } - store::init_bare(&local_path).context("initializing bare repo")?; - - // Upload to Tigris in background - if let Some(ref tigris) = self.tigris { - let tigris = tigris.clone(); - let owner_slug = owner_slug.clone(); - let repo_name = repo_name.to_string(); - let path = local_path.clone(); - tokio::spawn(async move { - if let Err(e) = tigris.upload(&owner_slug, &repo_name, &path).await { - warn!(repo = %repo_name, err = %e, "failed to upload new repo to tigris"); + /// Create a new repo's on-disk content via `build` and publish its archive + /// to storage, holding the per-repo advisory lock for the WHOLE + /// claim-to-publication lifecycle. + /// + /// Callers insert the DB row (the claim) BEFORE calling this. Because + /// pushes serialize on the same advisory lock, no push can execute in the + /// window between the row becoming visible and publication finishing — so + /// a failure here, compensated by the caller deleting its own row, can + /// never destroy a concurrently accepted push. On failure the created + /// local dir is removed so a retry doesn't hit an existing destination. + /// + /// `build` runs inline while the lock is held (matching the pre-existing + /// pattern of running git plumbing on the handler task). + pub async fn create_published( + &self, + owner_did: &str, + repo_name: &str, + build: impl FnOnce(&Path) -> Result<()> + Send, + ) -> Result { + let (owner_slug, local_path) = self.local_path(owner_did, repo_name)?; + let label = format!("{owner_slug}/{repo_name}"); + let lock = LockedConn::acquire( + &self.lock_pool, + advisory_lock_key(&owner_slug, repo_name), + &label, + ) + .await?; + + let outcome: Result<()> = async { + build(&local_path)?; + // A marker left by a previous same-name repo (failed creation, + // deleted repo) describes THAT repo's history, not this fresh one + // — once this repo's archive exists, a stale marker would read as + // divergence and wedge its writes. + clear_pending_upload(&local_path); + if let Some(ref archive) = self.archive { + // Fail closed: a silent upload failure would leave the repo + // absent from storage while its row is live. + let etag = archive + .upload(&owner_slug, repo_name, &local_path) + .await + .context("uploading new repo to storage")?; + if let Some(etag) = etag { + self.versions.lock().await.insert(label.clone(), etag); } - }); + } + Ok(()) } + .await; + if let Err(e) = outcome { + if local_path.exists() { + if let Err(cleanup_err) = std::fs::remove_dir_all(&local_path) { + warn!(repo = %repo_name, err = %cleanup_err, + "failed to remove local repo dir after creation failure"); + } + } + clear_pending_upload(&local_path); + lock.unlock().await; + return Err(e); + } + lock.unlock().await; Ok(local_path) } - /// Upload a repo to Tigris after a write operation (push, merge, fork, etc.). - /// Call this after any operation that modifies the git repo on disk. - pub async fn release_after_write(&self, owner_did: &str, repo_name: &str) { - if let Some(ref tigris) = self.tigris { - let (owner_slug, local_path) = match self.local_path(owner_did, repo_name) { - Ok(p) => p, + /// Whether a pending-upload marker currently protects this repo's local + /// copy. Handlers use this after a failed `release()` to decide whether an + /// already-committed git mutation is recoverable (marker present: the next + /// upload re-syncs storage) or must fail the request (no marker: nothing + /// protects the mutation from a stale-archive rollback). + pub fn pending_marker_exists(&self, owner_did: &str, repo_name: &str) -> bool { + self.local_path(owner_did, repo_name) + .and_then(|(_, local_path)| pending_upload_marker(&local_path)) + .map(|marker| marker.try_exists().unwrap_or(false)) + .unwrap_or(false) + } + + /// Startup sweep re-attempting the durable upload for every repo whose + /// pending-upload marker survived a crash or a failed upload. Without this, + /// a repo that receives no further writes stays divergent from storage + /// indefinitely, visible only as one log line at failure time. + /// + /// Applies the same base-etag rule as `sync_down_if_stale`: a repo whose + /// storage advanced past the marker's base is left marked (its writes stay + /// wedged pending manual reconciliation) and only logged. Returns + /// `(reuploaded, still_pending)`. + pub async fn retry_pending_uploads(&self) -> (usize, usize) { + if self.archive.is_none() { + return (0, 0); + } + let mut reuploaded = 0usize; + let mut still_pending = 0usize; + + let mut markers: Vec<(String, String, PathBuf)> = Vec::new(); // (slug, repo, local) + // Scan failures are logged loudly: a sweep that scanned nothing must + // not look identical to a node with no pending markers — especially + // since the gauge below is seeded from this same scan. + let owners = match std::fs::read_dir(&self.repos_dir) { + Ok(owners) => owners, + Err(e) => { + warn!(dir = %self.repos_dir.display(), err = %e, + "pending-upload sweep: cannot read repos dir — sweep skipped"); + return (0, 0); + } + }; + for owner in owners.flatten() { + if !owner.path().is_dir() { + continue; + } + let slug = owner.file_name().to_string_lossy().into_owned(); + let entries = match std::fs::read_dir(owner.path()) { + Ok(entries) => entries, Err(e) => { - warn!(repo = %repo_name, err = %e, "rejected unsafe path in release_after_write"); - return; + warn!(dir = %owner.path().display(), err = %e, + "pending-upload sweep: cannot read owner dir — skipped"); + continue; } }; - if let Err(e) = tigris.upload(&owner_slug, repo_name, &local_path).await { - warn!(repo = %repo_name, err = %e, "failed to upload repo to tigris after write"); + for entry in entries.flatten() { + let name = entry.file_name().to_string_lossy().into_owned(); + // Marker-write temp litter (crash mid-rename): collect it. + if name.starts_with(".pending-upload.tmp-") { + let _ = std::fs::remove_file(entry.path()); + continue; + } + // Marker layout: `.{repo}.git.pending-upload` + let Some(repo_dir) = name + .strip_prefix('.') + .and_then(|n| n.strip_suffix(".pending-upload")) + else { + continue; + }; + let Some(repo_name) = repo_dir.strip_suffix(".git") else { + continue; + }; + // The repo dir name comes off the filesystem here rather than + // from a request, but it is a name this node wrote from + // user-provided data and the result is handed to the upload + // path's fs calls, so it goes through the same barrier as every + // other repo path. A sweep entry that fails it is litter no + // legitimate write could have produced: skip it rather than + // acting on it. + let repo_path = + match validated_repo_path_in(&self.repos_dir, &owner.path().join(repo_dir)) { + Ok(p) => p, + Err(e) => { + warn!(dir = %owner.path().display(), err = %e, + "pending-upload sweep: rejecting an unsafe repo path — skipped"); + continue; + } + }; + markers.push((slug.clone(), repo_name.to_string(), repo_path)); } } + + // Seed the gauge with the surviving-marker count before processing; + // the clears below (and all runtime marker churn) then keep it + // current via deltas. + crate::metrics::set_pending_upload_markers(markers.len() as i64); + + for (slug, repo_name, local_path) in markers { + if !local_path.exists() { + // Stale litter (repo dir gone) — storage is the best remaining + // state; drop the marker. + clear_pending_upload(&local_path); + continue; + } + let pm = read_pending_marker(&local_path); + // The marker-vs-remote decision and the upload both happen inside + // `upload_locked_with_marker`, UNDER the advisory lock: an + // unlocked pre-check here could pass, then block on a concurrent + // push's lock for that push's whole duration, and the stale + // decision would clobber the push's freshly-uploaded archive. + match self + .upload_locked_with_marker(&slug, &repo_name, &local_path, &pm) + .await + { + Ok(PendingUploadOutcome::Uploaded) => { + debug!(repo = %repo_name, "pending-upload retry: re-synced storage"); + reuploaded += 1; + } + Ok(PendingUploadOutcome::Diverged) => { + warn!(repo = %repo_name, + "pending-upload retry: storage diverged from marker base — \ + leaving marked; writes stay blocked pending manual reconciliation"); + still_pending += 1; + } + Err(e) => { + warn!(repo = %repo_name, err = %e, + "pending-upload retry: upload failed — will retry on next write"); + still_pending += 1; + } + } + } + (reuploaded, still_pending) + } + + /// Marker-protected upload: takes the per-repo advisory lock, re-checks + /// that storage still matches `base` UNDER the lock, and only then uploads, + /// updates the versions cache, and clears the marker — all before the lock + /// is released. + /// + /// Both halves of that ordering are load-bearing: + /// - The divergence check must run under the lock. An unlocked check can + /// pass just before a concurrent locked push advances storage (the check + /// then blocks on that push's lock), and blindly uploading afterwards + /// would clobber the acked push it lost the race to. + /// - The marker must be cleared before the lock is released, or a writer + /// queued on the lock could observe marker + fresh etag and fail with a + /// spurious "diverged — reconcile manually" on a consistent repo. + async fn upload_locked_with_marker( + &self, + owner_slug: &str, + repo_name: &str, + local_path: &Path, + marker: &PendingMarker, + ) -> Result { + let Some(ref archive) = self.archive else { + anyhow::bail!("upload_locked_with_marker called without a storage backend"); + }; + let label = format!("{owner_slug}/{repo_name}"); + let lock = LockedConn::acquire( + &self.lock_pool, + advisory_lock_key(owner_slug, repo_name), + &label, + ) + .await?; + + let outcome: Result = async { + let remote = archive + .head_etag(owner_slug, repo_name) + .await + .context("storage head under lock before pending upload")?; + match remote.as_deref() { + Some(r) if !marker.matches_base(r) => { + // Unexplained remote: validate by content whether it is + // exactly what this node was uploading when it died — + // synced, no PUT needed. Otherwise: divergence. + if self + .remote_matches_inflight(archive, owner_slug, repo_name, marker) + .await + { + self.versions + .lock() + .await + .insert(label.clone(), r.to_string()); + clear_pending_upload_after_success(local_path, Some(r)); + return Ok(PendingUploadOutcome::Uploaded); + } + return Ok(PendingUploadOutcome::Diverged); + } + None if !marker.matches_base("") => { + return Ok(PendingUploadOutcome::Diverged); + } + _ => {} + } + // Record the intended etag in the marker before the PUT: a crash + // after the PUT lands is then recognizable (above) as our own + // completed upload instead of wedging on false divergence. + let etag = archive + .upload_with_intent(owner_slug, repo_name, local_path, |intended| { + record_inflight_upload(local_path, intended) + }) + .await + .context("uploading repo to storage under lock")?; + if let Some(ref etag) = etag { + self.versions + .lock() + .await + .insert(label.clone(), etag.clone()); + } + clear_pending_upload_after_success(local_path, etag.as_deref()); + Ok(PendingUploadOutcome::Uploaded) + } + .await; + + lock.unlock().await; + outcome + } + + /// Upload `local_path` to storage while holding the per-repo advisory lock, + /// so a background or init-time upload can't clobber a concurrent locked + /// write by landing an older snapshot after it. With `skip_if_exists`, skips + /// the upload when the archive is already present (used by lazy migration). + async fn upload_under_lock( + &self, + owner_slug: &str, + repo_name: &str, + local_path: &Path, + skip_if_exists: bool, + ) -> Result<()> { + let Some(ref archive) = self.archive else { + return Ok(()); + }; + let label = format!("{owner_slug}/{repo_name}"); + let lock = LockedConn::acquire( + &self.lock_pool, + advisory_lock_key(owner_slug, repo_name), + &label, + ) + .await?; + + let outcome: Result> = async { + if skip_if_exists { + // Propagate a failed existence check instead of treating it as + // "absent": HEAD failing transiently while PUT would succeed + // must not let this node's cache overwrite a newer shared + // archive. The lazy-migration caller just retries later. + let exists = archive + .exists(owner_slug, repo_name) + .await + .context("checking storage before migration upload")?; + if exists { + return Ok(None); // already present — nothing to upload + } + } + archive.upload(owner_slug, repo_name, local_path).await + } + .await; + + // Release the lock on the same connection regardless of outcome. + lock.unlock().await; + + match outcome { + Ok(Some(etag)) => { + self.versions + .lock() + .await + .insert(format!("{owner_slug}/{repo_name}"), etag); + Ok(()) + } + Ok(None) => Ok(()), + Err(e) => Err(e).context("uploading repo to storage under lock"), + } } /// Compute the local disk path and owner slug for a repo. @@ -465,6 +911,109 @@ pub(crate) fn validated_repo_disk_path( Ok(local_path) } +/// Re-establish the traversal barrier on a repo path this function did not +/// build itself, and return the checked value. +/// +/// `sync_down_if_stale` and the sweep receive a `&Path` as a PARAMETER. The +/// caller derived it from [`validated_repo_disk_path`], but a barrier the +/// reader has to trace across a call boundary is not a barrier a static +/// analyser will honour: to CodeQL's `rust/path-injection` the parameter is +/// just a path with user-provided data in its history, and every `exists()`, +/// `read_dir` and `remove_file` reached from it is a sink. Re-running the +/// check where the filesystem work actually happens costs two string compares +/// on a path we already hold and makes the guarantee local to the code it +/// guards, so it survives future refactors that move a call site. +/// +/// Same three layers as [`validated_repo_disk_path`], minus the name allowlist +/// (the components are no longer separable once joined): containment under +/// `repos_dir`, then the explicit `Component::Normal` walk. +pub(crate) fn validated_repo_path_in(repos_dir: &Path, candidate: &Path) -> Result { + if !candidate.starts_with(repos_dir) { + anyhow::bail!("repo path escaped repos_dir: {}", candidate.display()); + } + + // Explicit component walk — sanitisation barrier that static analysers + // (CodeQL `rust/path-injection`) recognise. The path must be composed + // entirely of Normal segments after the root prefix; any ParentDir or + // CurDir component is a traversal attempt. + for component in candidate.components() { + use std::path::Component; + match component { + Component::Prefix(_) | Component::RootDir | Component::Normal(_) => {} + Component::ParentDir => { + anyhow::bail!("path contains parent-directory component"); + } + Component::CurDir => { + anyhow::bail!("path contains current-directory component"); + } + } + } + + Ok(candidate.to_path_buf()) +} + +/// The sibling-path counterpart to [`validated_repo_disk_path`], for the files +/// this layer writes NEXT TO a repo directory rather than inside it: the +/// pending-upload marker, its rename-temp, and the swap phase's `.bak-` and +/// `.tmp-extract.` work dirs. +/// +/// A validated repo path does not make its siblings validated. `with_file_name` +/// and `join` build a NEW path, and a new path is a new question: nothing in +/// the type system says the name handed in contributed no separator, no `..`, +/// and no absolute prefix (a `join` with an absolute component silently +/// DISCARDS everything accumulated before it). These names are assembled from +/// the repo's own file name, which carries user-provided data all the way from +/// the push URL, so each one is re-checked here before any filesystem call +/// touches it. +/// +/// Three layers, mirroring the repo-path barrier: the name must be a single +/// ordinary path segment, the result must stay under the repo's parent +/// directory, and the joined path must walk as `Component::Normal` throughout. +/// The returned `PathBuf` is the only value callers hand to the filesystem. +pub(crate) fn validated_sibling_path(local_path: &Path, file_name: &str) -> Result { + let parent = local_path + .parent() + .context("repo path has no parent directory")?; + + if file_name.is_empty() { + anyhow::bail!("sibling file name is empty"); + } + if file_name.len() > 255 { + anyhow::bail!("sibling file name exceeds the 255-byte filesystem name limit"); + } + if file_name == "." || file_name == ".." || file_name.contains("..") { + anyhow::bail!("sibling file name contains a parent-directory reference"); + } + if file_name.contains('/') || file_name.contains('\\') || file_name.contains('\0') { + anyhow::bail!("sibling file name contains a path separator or null byte"); + } + + let candidate = parent.join(file_name); + + if !candidate.starts_with(parent) { + anyhow::bail!("sibling path escaped the repo parent dir: {file_name}"); + } + + // Explicit component walk — sanitisation barrier that static analysers + // (CodeQL `rust/path-injection`) recognise. The path must be composed + // entirely of Normal segments after the root prefix; any ParentDir or + // CurDir component is a traversal attempt. + for component in candidate.components() { + use std::path::Component; + match component { + Component::Prefix(_) | Component::RootDir | Component::Normal(_) => {} + Component::ParentDir => { + anyhow::bail!("path contains parent-directory component"); + } + Component::CurDir => { + anyhow::bail!("path contains current-directory component"); + } + } + } + + Ok(candidate) +} + /// Strict allowlist validator for `owner_did` and `repo_name`. /// /// Rejects any character that isn't explicitly safe, plus length and @@ -657,46 +1206,12 @@ fn validate_repo_name(repo_name: &str) -> Result<()> { #[error("no lock-pool connection available")] pub struct LockPoolBusy; -/// Guard returned by `acquire_write()`. Holds the Postgres advisory lock and -/// uploads to Tigris + releases the lock on `release()`. -pub struct RepoWriteGuard { - owner_slug: String, - repo_name: String, - pub local_path: PathBuf, - lock_key: i64, - /// The lock-pool connection that TOOK the advisory lock. It must be the one - /// that releases it (session locks are owned by their connection), and - /// holding it here is also what makes a guard dropped without `release` - /// safe: the drop returns the connection through the pool's `after_release` - /// hook, which runs `pg_advisory_unlock_all()`. - /// - /// `Option` because that hook is not a complete answer. When the unlock ERRORS - /// on a live session (a statement timeout, an admin cancel, an aborted - /// transaction), `after_release` issues its `pg_advisory_unlock_all()` on the - /// SAME broken session and it fails too, so the connection goes back to the pool - /// still holding the lock and nothing ever clears it (measured: never freed in - /// 15s, #174 F3b). Those paths `take()` the connection and close it instead; - /// ending the session is what actually frees the lock. `None` only after such a - /// disposal, or after `Drop` has moved it into the detached unlock. - lock_conn: Option>, - /// Set once `release` has run its unlock, making the `Drop` backstop inert. A - /// guard is only ever constructed with the lock already held, so there is no - /// "never locked" state to track alongside it. - released: bool, - tigris: Option, - /// Shared with the store that handed this guard out; see - /// [`RepoStore::upload_site_reached`]. - #[cfg(test)] - upload_site_reached: Arc, - /// Test-only seam: when set, `release` parks on this gate at the exact point it - /// is about to await `pg_advisory_unlock` (connection still owned, not yet - /// returned to the lock pool). Dropping the `release` future while it is parked - /// reproduces a mid-unlock cancellation, so a test can assert the lock is still - /// freed: the drop returns the connection through the pool's `after_release` - /// hook, which runs `pg_advisory_unlock_all()`. Never set outside tests. - #[cfg(test)] - test_pre_unlock_gate: Option>, -} +/// How long to retry acquiring the per-repo advisory lock before giving up. +/// Matches the storage backends' total operation timeout (300s in `s3.rs` and +/// `ipfs.rs`): the writer holding the lock may legitimately be mid-upload of a +/// large archive, so a concurrent push must be willing to outwait the longest +/// possible upload rather than failing while the lock holder is still healthy. +pub(crate) const LOCK_ACQUIRE_TIMEOUT_SECS: u64 = 300; /// Deadline for tearing down the connection that saw a failing `pg_advisory_unlock`. /// Long enough that a healthy socket always finishes well inside it, short enough that @@ -734,54 +1249,129 @@ async fn close_conn_bounded( } } -impl RepoWriteGuard { - /// Path to the bare repo on local disk. - pub fn path(&self) -> &Path { - &self.local_path - } +/// A pool connection pinned for the lifetime of a session-scoped advisory lock. +/// +/// Postgres advisory locks bind to one backend connection and only release on +/// that same connection; with a pool, acquiring and releasing on different +/// checked-out connections means the unlock silently no-ops while the lock +/// lingers on the original. So the lock's whole HELD lifetime — the winning +/// try-lock, use, unlock — runs on this single pinned connection. +/// +/// `unlock()` is the graceful path: it releases the lock and returns the +/// connection to the pool (closing it instead when the unlock errors, since a +/// session whose unlock failed may still hold the lock). If the holder is +/// *dropped* while the lock is held — a cancelled `acquire_write`, a detached +/// write-back task cancelled by runtime shutdown mid upload — `Drop` runs a +/// detached unlock on the same session, disposing of the connection if that +/// errors; off a Tokio runtime it detaches and closes the connection, which +/// ends the Postgres session and frees the lock server-side. The one thing +/// this type never does is knowingly return a still-locked connection to the +/// pool — and the lock pool's `after_release` hook (`pg_advisory_unlock_all`) +/// backstops even the paths it cannot see (#173). +struct LockedConn { + /// The connection that TOOK the advisory lock, owned until the unlock await + /// RESOLVES. `Option` because the unlock is not always the end of it: when + /// `pg_advisory_unlock` errors on a live session, `after_release` fails + /// identically on that same broken session (#174 F3b), so those paths + /// `take()` the connection and close it instead — ending the session is what + /// actually frees the lock. `None` only after such a disposal, or after + /// `Drop` has moved it into the detached unlock. + conn: Option>, + lock_key: i64, + repo_label: String, + /// Set once `unlock()`'s await has resolved, making the `Drop` backstop + /// inert. A `LockedConn` only ever exists with the lock already held, so + /// there is no "never locked" state to track alongside it. + released: bool, +} - /// Upload to Tigris (only when the write succeeded) and release the advisory - /// lock. Pass `success = false` when the write operation failed — uploading a - /// half-applied or otherwise inconsistent repo would propagate corruption to - /// Tigris (and to every node that later downloads it). The lock is always - /// released regardless, to avoid stale locks blocking future writes. - pub async fn release(mut self, success: bool) { - // Upload to Tigris only on success. - if success { - // The upload site, recorded for tests before the client is consulted: with - // no injectable seam on `TigrisClient` a counter inside the arm below could - // never move, and it is reaching this point at all that an interrupted push - // must not do (#173 F2). - #[cfg(test)] - self.upload_site_reached - .fetch_add(1, std::sync::atomic::Ordering::SeqCst); - if let Some(ref tigris) = self.tigris { - if let Err(e) = tigris - .upload(&self.owner_slug, &self.repo_name, &self.local_path) - .await - { - warn!(repo = %self.repo_name, err = %e, "failed to upload repo to tigris after write"); +impl LockedConn { + /// Acquire `lock_key` on a pinned connection, polling `pg_try_advisory_lock` + /// once per second up to [`LOCK_ACQUIRE_TIMEOUT_SECS`]. Polling (rather than + /// the blocking `pg_advisory_lock`) keeps a stale lock from a crashed + /// session from wedging writers indefinitely. + /// + /// The connection is checked out INSIDE the loop and RETURNED before each + /// sleep; only the connection that actually took the lock is retained. Two + /// constraints pull in opposite directions here, and this is what satisfies + /// both (#173 F1): + /// + /// * Session ownership. A session-level advisory lock belongs to the + /// CONNECTION that took it, so the lock and its `pg_advisory_unlock` + /// must run on the same one. Hence: keep the connection that WON. + /// * Occupancy. Holding a connection across the ~300 one-second sleeps + /// would let one spinning acquire park a lock-pool connection for + /// minutes. `api/issues.rs` and `api/pulls.rs` reach `acquire_write` + /// holding no concurrency permit at all, so a caller could park the + /// whole pool and starve authenticated pushes on every repo. Hence: + /// return the connection when we LOSE, before sleeping. + /// + /// Returning a losing connection is safe with respect to the cancellation + /// design: `after_release` runs `pg_advisory_unlock_all()`, a no-op on a + /// connection that took nothing, so it cannot disturb a lock held by any + /// other connection. + /// + /// Pool exhaustion (no connection free within the pool's acquire timeout) + /// surfaces as a downcastable [`LockPoolBusy`] so the HTTP layer can shed a + /// 503 + Retry-After instead of a generic 500. + async fn acquire(pool: &PgPool, lock_key: i64, repo_label: &str) -> Result { + for attempt in 0..LOCK_ACQUIRE_TIMEOUT_SECS { + let mut conn = pool.acquire().await.map_err(|e| { + anyhow::Error::new(LockPoolBusy) + .context(format!("checking out a lock-pool connection: {e}")) + })?; + match sqlx::query_as::<_, (bool,)>("SELECT pg_try_advisory_lock($1)") + .bind(lock_key) + .fetch_one(&mut *conn) + .await + { + Ok((true,)) => { + return Ok(Self { + conn: Some(conn), + lock_key, + repo_label: repo_label.to_string(), + released: false, + }); + } + Ok((false,)) => { + // Lost the race: give the connection back so a spinning + // acquire occupies nothing while it waits. + drop(conn); + } + Err(e) => { + // The poll itself failing leaves the lock's server-side + // state unknown: if the query executed but the response was + // lost, this session HOLDS the lock, and repooling the + // connection would strand it behind `after_release`'s best + // effort. Close it deliberately; ending the session frees + // anything it took. + drop(conn.detach()); + return Err(e).context("trying advisory lock"); } } - } else { - warn!(repo = %self.repo_name, "write failed — skipping tigris upload to avoid propagating an inconsistent repo"); + if attempt < LOCK_ACQUIRE_TIMEOUT_SECS - 1 { + tokio::time::sleep(std::time::Duration::from_secs(1)).await; + } } + anyhow::bail!( + "could not acquire advisory lock for {repo_label} after {LOCK_ACQUIRE_TIMEOUT_SECS}s — \ + possible stale lock or a long-running upload" + ); + } - // Test-only: park right before the unlock await so a test can drop this - // future mid-unlock, with the connection still owned. - #[cfg(test)] - if let Some(gate) = self.test_pre_unlock_gate.clone() { - gate.notified().await; - } - // Release the advisory lock on the connection that took it. Anything else - // (a fresh `&pool` checkout) is a no-op that returns false: Postgres - // scopes a session lock to its owning connection. - // - // Unlock through the connection while it is STILL owned by `self`; do not - // `take()` it first. A cancellation during this await then drops `self` with - // the connection still in place, so it returns to the lock pool and - // `after_release` clears the lock (#174 F4). - let unlock = match self.lock_conn.as_deref_mut() { + /// Release the held lock on the pinned connection and return it to the + /// pool. An unlock that ERRORS is a live session that may still hold the + /// lock — `after_release` would fail identically on the same broken session + /// (#174 F3b) — so the connection is closed instead (bounded, #174 F3c): + /// ending the session is what actually frees the lock. + async fn unlock(mut self) { + // Unlock through the connection while it is STILL owned by `self`; do + // NOT `take()` it first. A cancellation during this await then drops + // `self` with the connection in place, so `Drop`'s detached backstop + // runs and the pool's `after_release` hook clears whatever the + // interrupted unlock did not (#174 F4). Taking it early would leave + // `Drop` with `conn == None` and strand the session lock. + let unlock = match self.conn.as_deref_mut() { Some(conn) => Some( sqlx::query("SELECT pg_advisory_unlock($1)") .bind(self.lock_key) @@ -790,45 +1380,51 @@ impl RepoWriteGuard { ), None => None, }; - // An unlock that ERRORS is a different failure from a cancellation: the await - // resolved, so the session is alive and still holds the lock. Returning that - // connection to the pool does NOT recover it, because `after_release` runs its - // `pg_advisory_unlock_all()` on the same broken session and fails identically - // (#174 F3b). Close it: ending the session is what frees the lock. + // An unlock that ERRORS is a different failure from a cancellation: the + // await resolved, so the session is alive and still holds the lock, and + // returning that connection to the pool does not recover it because + // `after_release` runs its `pg_advisory_unlock_all()` on the same broken + // session and fails identically (#174 F3b). Close it: ending the session + // is what frees the lock. if let Some(Err(e)) = unlock { - warn!(repo = %self.repo_name, err = %e, + warn!(repo = %self.repo_label, err = %e, "advisory unlock failed, closing the connection so the session ends and postgres drops the lock"); - if let Some(conn) = self.lock_conn.take() { - close_conn_bounded(&self.repo_name, conn.close()).await; + if let Some(conn) = self.conn.take() { + close_conn_bounded(&self.repo_label, conn.close()).await; } } - // On the clean path, dropping `self` returns the connection to the lock pool, - // where `after_release` sweeps anything the unlock above missed. + // Only now that the await has resolved: mark released so the `Drop` + // backstop below does not re-issue an unlock on a lock already freed. + // On the clean path, dropping `self` returns the connection to the lock + // pool, where `after_release` sweeps anything this missed. self.released = true; } } -impl Drop for RepoWriteGuard { - /// Backstop for a guard dropped WITHOUT `release` (a cancelled `acquire_write`, a - /// handler future dropped before the release call). The pool's `after_release` - /// hook covers the ordinary case on its own, but not one: if the detached unlock - /// ERRORS on a live session, the hook's `pg_advisory_unlock_all()` fails the same - /// way and the connection returns to the pool still holding the lock (#174 F3b). +impl Drop for LockedConn { + /// Backstop for a holder dropped WITHOUT `unlock` (a cancelled + /// `acquire_write`, a handler future dropped before release, a cancelled + /// write-back task). The pool's `after_release` hook covers the ordinary + /// case on its own, but not one: if the detached unlock ERRORS on a live + /// session, the hook's `pg_advisory_unlock_all()` fails the same way and + /// the connection returns to the pool still holding the lock (#174 F3b). /// So the unlock runs here and disposes of the connection when it errors. /// - /// `Drop` cannot await, so the unlock is spawned; it runs on the same session, - /// which is what makes it effective. With no runtime to spawn onto there is - /// nothing that can unlock, so the connection is detached and dropped instead: - /// closing the socket ends the session, and that frees the lock server-side. + /// `Drop` cannot await, so the unlock is spawned; it runs on the same + /// session, which is what makes it effective. With no runtime to spawn onto + /// there is nothing that can unlock, so the connection is detached and + /// dropped instead: closing the socket ends the session, and that frees the + /// lock server-side (and detaching first avoids sqlx's return-to-pool + /// spawn, which panics off-runtime). fn drop(&mut self) { if self.released { return; } - let Some(mut conn) = self.lock_conn.take() else { + let Some(mut conn) = self.conn.take() else { return; }; let lock_key = self.lock_key; - let repo_name = self.repo_name.clone(); + let repo_label = self.repo_label.clone(); match tokio::runtime::Handle::try_current() { Ok(handle) => { handle.spawn(async move { @@ -836,33 +1432,343 @@ impl Drop for RepoWriteGuard { .bind(lock_key) .execute(&mut *conn) .await; - // Same failure as `release`'s, one level down: the await RESOLVED - // with an error, so the session is alive and still holds the lock. - // Ending this block would drop `conn` and RETURN it to the pool, - // where `after_release` fails identically. Close it instead. if let Err(e) = unlock { - warn!(repo = %repo_name, err = %e, "detached advisory-unlock on write-guard drop failed, closing the connection so the session ends and postgres drops the lock"); - close_conn_bounded(&repo_name, conn.close()).await; + warn!(repo = %repo_label, err = %e, + "detached advisory-unlock on lock-holder drop failed, closing the connection so the session ends and postgres drops the lock"); + close_conn_bounded(&repo_label, conn.close()).await; } }); } Err(_) => { - // `PoolConnection`'s own drop spawns its return-to-pool task, which - // panics with no runtime. `detach` gives up the pool slot and yields a - // plain `PgConnection`; dropping that closes the socket, which ends the - // session and is what frees the lock. drop(conn.detach()); warn!( - repo = %repo_name, - "RepoWriteGuard dropped off a Tokio runtime; no detached unlock is \ - possible, so the pinned connection is disposed of instead: ending \ - the session is what releases the advisory lock" + repo = %repo_label, + "advisory-lock holder dropped off a Tokio runtime; no detached unlock is \ + possible, so the pinned connection is disposed of instead: ending the \ + session is what releases the advisory lock" ); } } } } +/// Guard returned by `acquire_write()`. Holds the Postgres advisory lock and +/// uploads to storage + releases the lock on `release()`. +/// +/// `#[must_use]`: dropping the guard without calling `release()` skips the +/// storage upload and force-closes the pinned lock connection to free the +/// advisory lock (see [`LockedConn`]) — safe, but never what a caller wants. +#[must_use = "call release() — dropping the guard skips the upload and force-closes the lock connection"] +pub struct RepoWriteGuard { + owner_slug: String, + repo_name: String, + pub local_path: PathBuf, + /// The pinned advisory-lock connection; freed on `release()`, or by + /// `LockedConn::drop` if the guard (or a write-back task driving it) is + /// dropped or cancelled mid-flight. + lock: LockedConn, + archive: Option, + versions: Arc>>, + /// Shared with the store that handed this guard out; see + /// [`RepoStore::upload_site_reached`]. + #[cfg(test)] + upload_site_reached: Arc, + /// Test-only seam: when set, `release` parks on this gate at the exact point it + /// is about to await the advisory unlock (connection still owned by the guard's + /// `LockedConn`). Dropping the `release` future while it is parked reproduces a + /// mid-unlock cancellation; the guard then drops with the lock connection still + /// pinned, and `LockedConn`'s backstop frees the lock. Never set outside tests. + #[cfg(test)] + test_pre_unlock_gate: Option>, +} + +impl RepoWriteGuard { + /// Path to the bare repo on local disk. + pub fn path(&self) -> &Path { + &self.local_path + } + + /// Durably record intent-to-upload NOW, before the caller acks the client. + /// Write-back callers must call this before spawning `release()` — the + /// spawned task may never be polled if the process stops right after the + /// ack, and without the marker already on disk a restart would treat the + /// stale storage archive as newer and roll the acked write back. On `Err` + /// the caller must NOT ack early; fall back to strict upload-before-ack. + /// Idempotent with the marker `release()` writes itself. No-op without a + /// storage backend (markers would be inert until a backend appears, then + /// wedge repos whose archives predate them). + pub async fn mark_pending(&self) -> Result<()> { + if self.archive.is_none() { + return Ok(()); + } + let key = format!("{}/{}", self.owner_slug, self.repo_name); + let base = self.versions.lock().await.get(&key).cloned(); + mark_pending_upload(&self.local_path, base.as_deref()) + } + + /// Upload to storage (only when the write succeeded) and release the advisory + /// lock. Pass `success = false` when the write operation failed — uploading a + /// half-applied or otherwise inconsistent repo would propagate corruption to + /// storage (and to every node that later downloads it). The lock is always + /// released regardless, to avoid stale locks blocking future writes. + /// + /// IMPORTANT: the advisory lock is held until the upload finishes, so a + /// concurrent writer on another machine cannot read a stale archive. When + /// callers want a fast client ack, they spawn this future as a background + /// task (write-back) — the lock + etag-cache update still complete in order. + pub async fn release(self, success: bool) -> Result<()> { + let key = format!("{}/{}", self.owner_slug, self.repo_name); + + // Upload to storage only on success. Capture the outcome so we can both + // release the lock unconditionally and propagate a durable-upload + // failure to the caller (a synchronous caller turns it into a client + // error; a write-back caller logs it). + let upload_result: Result<()> = if success { + // The upload site, recorded for tests before the backend is consulted: it + // counts the DECISION to upload, so it moves even with no backend + // configured, and reaching this point at all is what an interrupted push + // must never do (#173 F2). + #[cfg(test)] + self.upload_site_reached + .fetch_add(1, std::sync::atomic::Ordering::SeqCst); + if let Some(ref archive) = self.archive { + let base = self.versions.lock().await.get(&key).cloned(); + if let Err(e) = mark_pending_upload(&self.local_path, base.as_deref()) { + // Proceed with the upload anyway: if it succeeds, no marker + // is needed; if both fail, the error below reaches the + // caller (double-failure corner, same exposure as + // pre-marker behavior). + warn!(repo = %self.repo_name, err = %e, "failed to write pending-upload marker"); + } + match archive + .upload_with_intent( + &self.owner_slug, + &self.repo_name, + &self.local_path, + |intended| record_inflight_upload(&self.local_path, intended), + ) + .await + { + Ok(Some(etag)) => { + self.versions.lock().await.insert(key.clone(), etag.clone()); + clear_pending_upload_after_success(&self.local_path, Some(&etag)); + Ok(()) + } + Ok(None) => { + clear_pending_upload(&self.local_path); + Ok(()) + } + Err(e) => { + // Storage is now behind local (this holds even for an + // already-acked write-back push). Drop the cached etag, + // and leave the pending-upload marker so the next + // access serves the local copy instead of rolling it + // back to the stale archive; the next successful + // upload re-syncs storage and clears the marker. + self.versions.lock().await.remove(&key); + Err(e).context("uploading repo to storage after write") + } + } + } else { + Ok(()) + } + } else { + // Write failed: skip the upload (a half-applied repo must not reach + // storage) and invalidate the cached etag — the local copy may be + // dirty, so the next write must re-download instead of skipping on a + // now-misleading etag match. + warn!(repo = %self.repo_name, "write failed — skipping storage upload and invalidating etag cache"); + self.versions.lock().await.remove(&key); + Ok(()) + }; + + // Test-only: park right before the unlock await so a test can drop this + // future mid-unlock, with the connection still owned. + #[cfg(test)] + if let Some(gate) = self.test_pre_unlock_gate.clone() { + gate.notified().await; + } + // Release the advisory lock on the same connection it was taken on + // regardless of the upload outcome, then return it to the pool. + self.lock.unlock().await; + + upload_result + } +} + +/// Sibling marker file recording that `local_path` holds writes storage has +/// not received yet ("local is ahead"). Written before every post-write upload +/// and removed only when the upload succeeds, so it survives process death and +/// lets `sync_down_if_stale` distinguish "storage is ahead of local" (download) +/// from "local is ahead of storage" (never download — that would roll back an +/// acked write). Lives next to the repo dir, not inside it, so it is never +/// packed into the archive. +/// Fallible because it goes through [`validated_sibling_path`]: the marker name +/// is built from the repo's own file name, which carries user-provided data, so +/// the path is re-checked here rather than trusted from the repo path's own +/// earlier validation. Callers that cannot propagate the error (the `Drop`-like +/// cleanup paths) treat a rejected path as "no marker", which is the same +/// conservative answer they already give for an unreadable one. +fn pending_upload_marker(local_path: &Path) -> Result { + let name = local_path + .file_name() + .map(|n| n.to_string_lossy().into_owned()) + .unwrap_or_default(); + validated_sibling_path(local_path, &format!(".{name}.pending-upload")) +} + +/// The marker's rename-temp sibling. Same barrier, same reason; the UUID is +/// ours but the prefix is not, and one helper means the two cannot drift. +fn pending_upload_marker_tmp(local_path: &Path) -> Result { + validated_sibling_path( + local_path, + &format!(".pending-upload.tmp-{}", uuid::Uuid::new_v4()), + ) +} + +/// Persist the intent-to-upload marker. Fallible (write-back callers must NOT +/// ack the client if this fails) and atomic (tmp + rename, so a crash cannot +/// leave a torn marker). +/// +/// `base_etag` is the storage etag the local write was built on (empty when +/// storage held nothing). `sync_down_if_stale` compares it against the current +/// remote etag to distinguish "local strictly ahead" from cross-node +/// divergence. +/// +/// An existing marker is preserved untouched: its base is the last storage +/// etag this node confirmed, which stays correct for every further write +/// stacked on the same undiverged local copy. Re-marking would record the +/// CURRENT cache — emptied by the preceding upload failure — and a corrupted +/// (empty) base makes the next sync read unchanged storage as divergence, +/// wedging the repo's whole write surface after two consecutive upload +/// failures. +fn mark_pending_upload(local_path: &Path, base_etag: Option<&str>) -> Result<()> { + let marker = pending_upload_marker(local_path)?; + match marker.try_exists() { + Ok(true) => return Ok(()), // keep the original base + Ok(false) => {} + Err(e) => return Err(e).context("probing pending-upload marker"), + } + let tmp = pending_upload_marker_tmp(local_path)?; + std::fs::write(&tmp, base_etag.unwrap_or_default()).context("writing pending-upload marker")?; + std::fs::rename(&tmp, &marker) + .inspect_err(|_| { + let _ = std::fs::remove_file(&tmp); + }) + .context("publishing pending-upload marker")?; + crate::metrics::add_pending_upload_markers(1); + Ok(()) +} + +pub(crate) fn clear_pending_upload(local_path: &Path) { + // A path the barrier rejects is one we must never have written, so there is + // nothing to clear and nothing to report: the same no-op this already + // performs for a marker that is simply absent. + let Ok(marker) = pending_upload_marker(local_path) else { + return; + }; + if std::fs::remove_file(marker).is_ok() { + crate::metrics::add_pending_upload_markers(-1); + } +} + +/// Etags compared structurally: S3 returns them quoted, our recorded values +/// are bare, and whitespace can differ across the marker round-trip. +fn norm_etag(e: &str) -> &str { + e.trim().trim_matches('"') +} + +/// Parsed pending-upload marker. Line 1 is the storage etag the local write +/// was BASED on; optional line 2 is the etag the in-flight upload was going to +/// produce (the archive's content MD5, recorded just before the PUT). +struct PendingMarker { + base: String, + inflight: Option, +} + +impl PendingMarker { + /// Storage still holds exactly what the local write was based on: local + /// is strictly ahead. + fn matches_base(&self, remote: &str) -> bool { + norm_etag(remote) == norm_etag(&self.base) + } +} + +fn read_pending_marker(local_path: &Path) -> PendingMarker { + // A rejected path reads as an empty marker, which is what an absent or + // unreadable one already produces: an empty base matches only empty + // storage, so recovery stays conservative rather than claiming a base it + // never confirmed. + let content = pending_upload_marker(local_path) + .ok() + .and_then(|marker| std::fs::read_to_string(marker).ok()) + .unwrap_or_default(); + let mut lines = content.lines(); + let base = lines.next().unwrap_or("").trim().to_string(); + let inflight = lines + .next() + .map(|l| l.trim().to_string()) + .filter(|l| !l.is_empty()); + PendingMarker { base, inflight } +} + +/// Atomically rewrite the marker as `base\nintended` just before the PUT, so +/// a crash anywhere between the PUT landing and the post-upload clear leaves a +/// marker that names the uploaded content — recovery then recognizes storage +/// as this node's own completed upload instead of wedging on false divergence. +/// MUST be fallible: if the intent cannot be durably recorded, the PUT it +/// describes must not run (the caller aborts the upload), or a crash after +/// that PUT reads as external divergence. +fn record_inflight_upload(local_path: &Path, intended_etag: &str) -> Result<()> { + let marker = pending_upload_marker(local_path)?; + let base = read_pending_marker(local_path).base; + let tmp = pending_upload_marker_tmp(local_path)?; + std::fs::write(&tmp, format!("{base}\n{intended_etag}")) + .context("writing in-flight upload intent")?; + std::fs::rename(&tmp, &marker) + .inspect_err(|_| { + let _ = std::fs::remove_file(&tmp); + }) + .context("publishing in-flight upload intent") +} + +/// Remove the marker after a *successful* upload, first atomically rewriting +/// its base to the just-uploaded etag. A crash between the rewrite and the +/// unlink then reads as "local ahead, base matches" — which self-heals on the +/// next write or startup retry — instead of "base predates storage", which +/// would wedge the repo behind a spurious permanent divergence even though +/// local and storage are identical. +fn clear_pending_upload_after_success(local_path: &Path, new_etag: Option<&str>) { + // As in `clear_pending_upload`: a path the barrier rejects is one nothing + // ever wrote, so there is no marker to rewrite and none to unlink. + let Ok(marker) = pending_upload_marker(local_path) else { + return; + }; + if let Some(etag) = new_etag { + if marker.exists() { + if let Ok(tmp) = pending_upload_marker_tmp(local_path) { + if std::fs::write(&tmp, etag).is_ok() { + let _ = std::fs::rename(&tmp, &marker); + } else { + let _ = std::fs::remove_file(&tmp); + } + } + } + } + if std::fs::remove_file(&marker).is_ok() { + crate::metrics::add_pending_upload_markers(-1); + } +} + +/// Outcome of a marker-protected upload attempt. +enum PendingUploadOutcome { + /// Uploaded, versions cache updated, marker cleared — all under the lock. + Uploaded, + /// Storage no longer matches the marker's base: another writer advanced it. + /// Nothing was uploaded and the marker was left in place. + Diverged, +} + /// Build the dedicated advisory-lock pool a `RepoStore` runs its write locks on. /// Connect options are cloned off an existing pool so callers need not re-parse /// the database URL; the pool is lazy, so no connection is opened here. @@ -1037,19 +1943,19 @@ mod tests { // ── acquire_write cancellation safety (#173 U1) ──────────────────────── /// The reviewer's named regression. `api/repos.rs` wraps `acquire_write` in a - /// `tokio::time::timeout`; when that fires during the Tigris phase the future + /// `tokio::time::timeout`; when that fires during the storage phase the future /// is dropped after the advisory lock was taken and before `RepoWriteGuard` /// (the only thing that unlocks) exists. The lock then leaks and every later /// push to the same repo spins the 60-attempt / 60s ceiling and fails. #[sqlx::test] - async fn cancelled_acquire_write_mid_tigris_does_not_leak_the_lock(pool: PgPool) { + async fn cancelled_acquire_write_mid_storage_does_not_leak_the_lock(pool: PgPool) { let repos_dir = PathBuf::from("/tmp/gitlawb-test-repos"); let owner = "did:key:z6MkCancelMidTigris"; let repo = "cancel-mid-tigris"; let store_pool = sibling_pool(&pool, 8); let stalling = RepoStore::for_testing(repos_dir.clone(), store_pool.clone()) - .with_tigris_stall(Duration::from_secs(30)); + .with_storage_stall(Duration::from_secs(30)); let cancelled = tokio::time::timeout( Duration::from_millis(500), stalling.acquire_write(owner, repo), @@ -1057,7 +1963,7 @@ mod tests { .await; assert!( cancelled.is_err(), - "the acquire must still be inside the Tigris phase when the timeout fires" + "the acquire must still be inside the storage phase when the timeout fires" ); // Observed from an independent session, so the check cannot be satisfied @@ -1076,7 +1982,7 @@ mod tests { .await .expect("second acquire_write must not block on a leaked lock") .expect("second acquire_write must succeed"); - guard.release(false).await; + guard.release(false).await.ok(); } /// Cancellation BEFORE the lock is taken must leave nothing behind: no lock, @@ -1114,7 +2020,7 @@ mod tests { .await .expect("the lock-pool connection must have been returned") .expect("acquire after cancellation"); - guard.release(false).await; + guard.release(false).await.ok(); } /// Lock-pool exhaustion is a bounded wait and a clean error, never a panic and @@ -1155,7 +2061,7 @@ mod tests { "the error must name the lock-pool checkout, got: {err}" ); - held.release(false).await; + held.release(false).await.ok(); } /// #173 F1 (RED-before/GREEN-after). A contended `acquire_write` spins for up to @@ -1221,7 +2127,7 @@ mod tests { "the uncontended acquire must not queue behind the spinners for the pool \ acquire timeout; took {elapsed:?}" ); - free_guard.release(false).await; + free_guard.release(false).await.ok(); // The drop-and-retake cycle must still END in a real, exclusive lock: free // spin-a's key and the spinner that was cycling connections must take it. @@ -1240,7 +2146,7 @@ mod tests { !lock_is_free_elsewhere(&probe, advisory_lock_key(&owner_slug, "spin-a")).await, "the lock a spinner finally took must be observably held from another session" ); - winner.release(false).await; + winner.release(false).await.ok(); for s in spinners { s.abort(); @@ -1290,7 +2196,7 @@ mod tests { !lock_is_free_elsewhere(&probe, key).await, "a held write lock must survive other lock-pool connections being returned" ); - guard.release(true).await; + guard.release(true).await.ok(); assert!( lock_is_free_elsewhere(&probe, key).await, "release must still free the lock after the churn" @@ -1332,7 +2238,7 @@ mod tests { "a validation failure must not masquerade as lock-pool capacity, got: {other}" ); - held.release(false).await; + held.release(false).await.ok(); } /// Round trip: the lock is observably HELD between acquire and release, and @@ -1360,7 +2266,7 @@ mod tests { // session and return false. The `after_release` hook is a net for the // cancellation path and fires from a spawned task well after this point, so // it must not be what makes this assertion pass. - guard.release(true).await; + guard.release(true).await.ok(); assert!( lock_is_free_elsewhere(&probe, key).await, "release must free the lock as seen from another session" @@ -1378,7 +2284,7 @@ mod tests { let store = RepoStore::for_testing(PathBuf::from("/tmp/gitlawb-test-repos"), pool.clone()); let guard = store.acquire_write(owner, repo).await.expect("acquire"); - guard.release(false).await; + guard.release(false).await.ok(); assert!( wait_until_free(&probe, key, Duration::from_secs(5)).await, @@ -1407,7 +2313,7 @@ mod tests { .await .expect("a different repo must not wait on this lock") .expect("acquire other repo"); - other.release(false).await; + other.release(false).await.ok(); // Same repo: must not acquire while `first` is alive. let contender = tokio::spawn({ @@ -1420,13 +2326,13 @@ mod tests { "a second acquire for the same repo must block while the first guard lives" ); - first.release(false).await; + first.release(false).await.ok(); let second = tokio::time::timeout(Duration::from_secs(10), contender) .await .expect("contender must finish once the lock is free") .expect("contender task") .expect("contender acquire"); - second.release(false).await; + second.release(false).await.ok(); } // ── sync slug validation (#272) ──────────────────────────────────────── @@ -1934,7 +2840,7 @@ mod tests { let mut checker = pool.acquire().await.expect("checker connection"); let guard = store.acquire_write(owner, name).await.expect("acquire"); - guard.release(false).await; + guard.release(false).await.ok(); let (free,): (bool,) = sqlx::query_as("SELECT pg_try_advisory_lock($1)") .bind(key) @@ -2027,7 +2933,7 @@ mod tests { .acquire_write(owner, name) .await .expect("first acquire"); - guard.release(true).await; + guard.release(true).await.ok(); let again = tokio::time::timeout( std::time::Duration::from_secs(2), @@ -2036,7 +2942,7 @@ mod tests { .await .expect("second acquire_write must not hit the ~60s stale-lock retry loop") .expect("second acquire"); - again.release(true).await; + again.release(true).await.ok(); } // ── unlock error disposes the connection (#174 F3b, RED-before/GREEN-after) ─ @@ -2050,7 +2956,8 @@ mod tests { /// in this module and can reach `conn` directly. async fn poison_guard_connection(guard: &mut RepoWriteGuard) { let conn = guard - .lock_conn + .lock + .conn .as_deref_mut() .expect("guard holds its connection before release"); sqlx::query("BEGIN") @@ -2264,7 +3171,7 @@ mod tests { "the poisoned session must still hold the lock before release" ); - guard.release(false).await; + guard.release(false).await.ok(); // Postgres drops the lock when the disposed session's backend exits, which is // asynchronous to our socket close: poll for it rather than sleeping a @@ -2303,7 +3210,7 @@ mod tests { let size_before = lock_pool.size(); assert!(size_before > 0, "the lock pool owns the guard's connection"); - guard.release(false).await; + guard.release(false).await.ok(); // The pool's size drops when the closed connection's slot is given up, which // is not synchronous with `release` returning: poll rather than sleep. @@ -2333,7 +3240,7 @@ mod tests { let guard = store.acquire_write(owner, name).await.expect("acquire"); let size_before = pool.size(); - guard.release(false).await; + guard.release(false).await.ok(); tokio::time::sleep(std::time::Duration::from_millis(400)).await; assert_eq!( @@ -2455,4 +3362,600 @@ mod tests { .execute(&mut *checker) .await; } + + // ── sync_down_if_stale (fs-backed archive, lazy pool) ────────────────── + + /// A RepoStore over an fs-backed archive. `sync_down_if_stale` never touches + /// the pool, so a lazy (never-connected) pool is fine. + fn store_with_fs_archive(repos_dir: PathBuf, store_root: &Path) -> RepoStore { + let blob: Arc = + Arc::new(crate::storage::fs::FsBlobStore::new(store_root).unwrap()); + let archive = crate::storage::archive::RepoArchive::new(blob); + let pool = sqlx::PgPool::connect_lazy("postgres://invalid").unwrap(); + RepoStore::new(repos_dir, Some(archive), pool) + } + + #[tokio::test] + async fn sync_down_if_stale_downloads_then_skips_on_etag_match() { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let store = store_with_fs_archive(repos_dir.path().to_path_buf(), store_root.path()); + + // Seed the archive with a repo. + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + store + .archive + .as_ref() + .unwrap() + .upload("owner", "repo", seed.path()) + .await + .unwrap(); + + let local = repos_dir.path().join("owner").join("repo.git"); + + // First call downloads. + store + .sync_down_if_stale("owner", "repo", &local, false) + .await + .unwrap(); + assert_eq!(std::fs::read(local.join("HEAD")).unwrap(), b"v1\n"); + + // Locally mutate, then sync again: the cached etag still matches the + // remote, so the download is skipped and our local edit survives. + std::fs::write(local.join("HEAD"), b"LOCAL-EDIT\n").unwrap(); + store + .sync_down_if_stale("owner", "repo", &local, false) + .await + .unwrap(); + assert_eq!( + std::fs::read(local.join("HEAD")).unwrap(), + b"LOCAL-EDIT\n", + "etag match must skip the download (local copy preserved)" + ); + } + + // Needs a real pool: `release_after_write` uploads under the advisory lock. + #[sqlx::test] + async fn pending_marker_prevents_rollback_and_clears_on_next_upload(pool: PgPool) { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let blob: Arc = + Arc::new(crate::storage::fs::FsBlobStore::new(store_root.path()).unwrap()); + let store = RepoStore::new( + repos_dir.path().to_path_buf(), + Some(crate::storage::archive::RepoArchive::new(blob)), + pool, + ); + + // Storage holds v1; local downloads it. + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + store + .archive + .as_ref() + .unwrap() + .upload("owner", "repo", seed.path()) + .await + .unwrap(); + let local = repos_dir.path().join("owner").join("repo.git"); + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .unwrap(); + + // Simulate an acked write whose upload failed: local advances, the + // pending marker (recording the storage etag the write was based on) + // persists, and the in-memory cache was invalidated on failure. + let base = store + .archive + .as_ref() + .unwrap() + .head_etag("owner", "repo") + .await + .unwrap() + .unwrap(); + std::fs::write(local.join("HEAD"), b"ACKED-WRITE\n").unwrap(); + mark_pending_upload(&local, Some(&base)).unwrap(); + store.versions.lock().await.clear(); + + // Both the read and the write path must serve local, not roll it back. + for require_fresh in [false, true] { + store + .sync_down_if_stale("owner", "repo", &local, require_fresh) + .await + .unwrap(); + assert_eq!( + std::fs::read(local.join("HEAD")).unwrap(), + b"ACKED-WRITE\n", + "pending marker must prevent rollback (require_fresh={require_fresh})" + ); + } + + // The next successful write-path upload re-syncs storage and clears + // the marker. + let guard = store.acquire_write("owner", "repo").await.unwrap(); + guard.release(true).await.unwrap(); + assert!( + !pending_upload_marker(&local).unwrap().exists(), + "marker must be cleared by a successful upload" + ); + let out = tempfile::tempdir().unwrap(); + let restored = out.path().join("restored.git"); + store + .archive + .as_ref() + .unwrap() + .download("owner", "repo", &restored) + .await + .unwrap(); + assert_eq!( + std::fs::read(restored.join("HEAD")).unwrap(), + b"ACKED-WRITE\n" + ); + } + + #[tokio::test] + async fn pending_marker_detects_cross_node_divergence() { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let store = store_with_fs_archive(repos_dir.path().to_path_buf(), store_root.path()); + + // Storage v1; local synced, then advanced with a failed upload (marker + // records v1's etag as its base). + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + let archive = store.archive.as_ref().unwrap(); + archive.upload("owner", "repo", seed.path()).await.unwrap(); + let local = repos_dir.path().join("owner").join("repo.git"); + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .unwrap(); + let base = archive.head_etag("owner", "repo").await.unwrap().unwrap(); + std::fs::write(local.join("HEAD"), b"LOCAL-AHEAD\n").unwrap(); + mark_pending_upload(&local, Some(&base)).unwrap(); + store.versions.lock().await.clear(); + + // Another node advances storage past our base. + let seed2 = tempfile::tempdir().unwrap(); + std::fs::write(seed2.path().join("HEAD"), b"OTHER-NODE\n").unwrap(); + archive.upload("owner", "repo", seed2.path()).await.unwrap(); + + // Write path: refuse — proceeding would clobber one side or the other. + assert!( + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .is_err(), + "diverged marker must fail the write path closed" + ); + // Read path: serve local (read-only cannot propagate damage), and the + // local copy must be untouched either way. + store + .sync_down_if_stale("owner", "repo", &local, false) + .await + .unwrap(); + assert_eq!(std::fs::read(local.join("HEAD")).unwrap(), b"LOCAL-AHEAD\n"); + } + + #[tokio::test] + async fn stale_pending_marker_without_local_copy_is_dropped() { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let store = store_with_fs_archive(repos_dir.path().to_path_buf(), store_root.path()); + + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + store + .archive + .as_ref() + .unwrap() + .upload("owner", "repo", seed.path()) + .await + .unwrap(); + + // Marker exists but the repo dir does not (removed out from under us): + // the marker is stale — drop it and download normally. + let local = repos_dir.path().join("owner").join("repo.git"); + std::fs::create_dir_all(local.parent().unwrap()).unwrap(); + mark_pending_upload(&local, Some("whatever")).unwrap(); + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .unwrap(); + assert_eq!(std::fs::read(local.join("HEAD")).unwrap(), b"v1\n"); + assert!(!pending_upload_marker(&local).unwrap().exists()); + } + + #[tokio::test] + async fn sync_down_if_stale_require_fresh_fails_closed_on_bad_remote() { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let store = store_with_fs_archive(repos_dir.path().to_path_buf(), store_root.path()); + + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + store + .archive + .as_ref() + .unwrap() + .upload("owner", "repo", seed.path()) + .await + .unwrap(); + + let local = repos_dir.path().join("owner").join("repo.git"); + store + .sync_down_if_stale("owner", "repo", &local, false) + .await + .unwrap(); + + // Corrupt the stored archive: HEAD now succeeds with a *new* etag (so the + // cache no longer matches and a download is forced), but the download + // decompresses garbage and fails. + let blob_path = store_root.path().join("repos/v1/owner/repo.tar.zst"); + std::fs::write(&blob_path, b"corrupted not-a-tar-zst").unwrap(); + // The fs backend's etag lives in a sidecar, so a direct file overwrite + // must also bump it for the change to be visible (as any real writer's + // put() would). + std::fs::write( + store_root.path().join("repos/v1/owner/repo.tar.zst.etag"), + "corrupted-generation", + ) + .unwrap(); + + // Write path: must fail closed rather than fall back to the stale local + // copy (which a later upload would use to clobber the newer remote). + assert!( + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .is_err(), + "require_fresh=true must propagate the download error" + ); + + // Read path: self-heals — falls back to the valid local copy. + store + .sync_down_if_stale("owner", "repo", &local, false) + .await + .expect("require_fresh=false must fall back to the local copy"); + assert_eq!(std::fs::read(local.join("HEAD")).unwrap(), b"v1\n"); + } + + // ── failing-store double: exercises error branches no real backend can ── + + /// BlobStore wrapper whose `put`/`head` can be flipped to fail, unlocking + /// deterministic coverage of the upload-failure and head-failure branches. + struct FlakyStore { + inner: crate::storage::fs::FsBlobStore, + fail_put: std::sync::atomic::AtomicBool, + fail_head: std::sync::atomic::AtomicBool, + } + + impl FlakyStore { + fn new(root: &Path) -> Arc { + Arc::new(Self { + inner: crate::storage::fs::FsBlobStore::new(root).unwrap(), + fail_put: std::sync::atomic::AtomicBool::new(false), + fail_head: std::sync::atomic::AtomicBool::new(false), + }) + } + } + + #[async_trait::async_trait] + impl crate::storage::BlobStore for FlakyStore { + fn backend_name(&self) -> &'static str { + "flaky" + } + async fn get(&self, key: &str) -> Result> { + self.inner.get(key).await + } + async fn put(&self, key: &str, body: bytes::Bytes) -> Result { + if self.fail_put.load(std::sync::atomic::Ordering::Relaxed) { + anyhow::bail!("injected put failure"); + } + self.inner.put(key, body).await + } + async fn head(&self, key: &str) -> Result> { + if self.fail_head.load(std::sync::atomic::Ordering::Relaxed) { + anyhow::bail!("injected head failure"); + } + self.inner.head(key).await + } + async fn delete(&self, key: &str) -> Result<()> { + self.inner.delete(key).await + } + } + + fn store_with_flaky(repos_dir: PathBuf, flaky: Arc, pool: PgPool) -> RepoStore { + let blob: Arc = flaky; + let archive = crate::storage::archive::RepoArchive::new(blob); + RepoStore::new(repos_dir, Some(archive), pool) + } + + /// beardthelion P1 regression: two consecutive failed uploads must not + /// corrupt the marker's base — the second `release` re-marks while the + /// versions cache is empty, and overwriting the base with "" would make + /// the third write read unchanged storage as divergence and wedge the + /// repo's entire write surface. + #[sqlx::test] + async fn two_consecutive_failed_uploads_preserve_marker_base(pool: PgPool) { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let flaky = FlakyStore::new(store_root.path()); + let store = store_with_flaky(repos_dir.path().to_path_buf(), Arc::clone(&flaky), pool); + + // Storage v1, synced down. + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + store + .archive + .as_ref() + .unwrap() + .upload("owner", "repo", seed.path()) + .await + .unwrap(); + let local = repos_dir.path().join("owner").join("repo.git"); + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .unwrap(); + + // Write 1: mutate, upload fails. + flaky + .fail_put + .store(true, std::sync::atomic::Ordering::Relaxed); + std::fs::write(local.join("HEAD"), b"write-1\n").unwrap(); + let guard = store.acquire_write("owner", "repo").await.unwrap(); + assert!(guard.release(true).await.is_err(), "injected put must fail"); + let base_after_first = read_pending_marker(&local).base; + assert!(!base_after_first.is_empty(), "base must be recorded"); + + // Write 2: acquire must succeed (storage unchanged == local-ahead), + // and the second failed release must NOT re-mark with an empty base. + // (The in-flight line legitimately changes per attempt; the BASE is + // the invariant.) + let guard = store.acquire_write("owner", "repo").await.unwrap(); + std::fs::write(local.join("HEAD"), b"write-2\n").unwrap(); + assert!(guard.release(true).await.is_err()); + assert_eq!( + read_pending_marker(&local).base, + base_after_first, + "an existing marker's base must be preserved on re-mark" + ); + + // Write 3: still not wedged — and once the store heals, everything + // re-syncs and the marker clears. + let guard = store + .acquire_write("owner", "repo") + .await + .expect("repeated upload failures must not wedge the write surface"); + flaky + .fail_put + .store(false, std::sync::atomic::Ordering::Relaxed); + guard.release(true).await.unwrap(); + assert!(!pending_upload_marker(&local).unwrap().exists()); + } + + /// jatmn P1 regression: the write-back ack window. `mark_pending` runs + /// before the ack; if the process dies before the spawned release is ever + /// polled (simulated by dropping the guard), the marker alone must keep + /// the next sync from rolling the acked write back. + #[sqlx::test] + async fn mark_pending_alone_protects_the_ack_window(pool: PgPool) { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let flaky = FlakyStore::new(store_root.path()); + let store = store_with_flaky(repos_dir.path().to_path_buf(), Arc::clone(&flaky), pool); + + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + store + .archive + .as_ref() + .unwrap() + .upload("owner", "repo", seed.path()) + .await + .unwrap(); + let local = repos_dir.path().join("owner").join("repo.git"); + + let guard = store.acquire_write("owner", "repo").await.unwrap(); + std::fs::write(local.join("HEAD"), b"ACKED\n").unwrap(); + guard.mark_pending().await.unwrap(); + drop(guard); // crash before release() is ever polled + // A real restart loses the in-memory etag cache; without this the + // cache-hit skip masks the marker and the test passes with + // mark_pending gutted. + store.versions.lock().await.clear(); + + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .unwrap(); + assert_eq!( + std::fs::read(local.join("HEAD")).unwrap(), + b"ACKED\n", + "the pre-ack marker alone must prevent rollback" + ); + } + + /// beardthelion P1 regression: a crash between a successful upload and + /// the marker clear must NOT read as divergence. The marker records the + /// upload's intended etag (content MD5) before the PUT; recovery finding + /// storage at exactly that etag recognizes its own completed upload and + /// heals instead of wedging a byte-identical repo behind "reconcile + /// manually". + #[tokio::test] + async fn crash_after_upload_before_clear_heals_via_inflight_etag() { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let store = store_with_fs_archive(repos_dir.path().to_path_buf(), store_root.path()); + let archive = store.archive.as_ref().unwrap(); + + // Simulate the crash state: storage holds the content this node was + // uploading (etag E), while the marker still names the pre-upload + // base B plus the in-flight etag E. + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"uploaded\n").unwrap(); + archive.upload("owner", "repo", seed.path()).await.unwrap(); + let remote = archive.head_etag("owner", "repo").await.unwrap().unwrap(); + + let local = repos_dir.path().join("owner").join("repo.git"); + std::fs::create_dir_all(&local).unwrap(); + std::fs::write(local.join("HEAD"), b"uploaded\n").unwrap(); + mark_pending_upload(&local, Some("pre-upload-base")).unwrap(); + record_inflight_upload(&local, &remote).unwrap(); + + // Write path must heal, not wedge: marker cleared, cache adopted. + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .expect("own completed upload must not read as divergence"); + assert!( + !pending_upload_marker(&local).unwrap().exists(), + "marker must be cleared once storage is recognized as our upload" + ); + assert_eq!(std::fs::read(local.join("HEAD")).unwrap(), b"uploaded\n"); + // And subsequent syncs skip on the adopted etag. + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .unwrap(); + } + + /// The lazy-migration existence check must propagate failure instead of + /// reading it as "absent" and uploading over a possibly-newer archive. + #[sqlx::test] + async fn upload_under_lock_propagates_failed_existence_check(pool: PgPool) { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let flaky = FlakyStore::new(store_root.path()); + let store = store_with_flaky(repos_dir.path().to_path_buf(), Arc::clone(&flaky), pool); + + let local = repos_dir.path().join("owner").join("repo.git"); + std::fs::create_dir_all(&local).unwrap(); + std::fs::write(local.join("HEAD"), b"local\n").unwrap(); + + flaky + .fail_head + .store(true, std::sync::atomic::Ordering::Relaxed); + assert!( + store + .upload_under_lock("owner", "repo", &local, true) + .await + .is_err(), + "a failed existence check must not read as absent" + ); + assert!( + store + .archive + .as_ref() + .unwrap() + .head_etag("owner", "repo") + .await + .is_err(), + "sanity: head still failing" + ); + } + + /// init() must remove its local dir when the initial upload fails, so a + /// retry of the same name doesn't hit an existing destination. + #[sqlx::test] + async fn init_removes_local_dir_when_upload_fails(pool: PgPool) { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let flaky = FlakyStore::new(store_root.path()); + let store = store_with_flaky(repos_dir.path().to_path_buf(), Arc::clone(&flaky), pool); + + flaky + .fail_put + .store(true, std::sync::atomic::Ordering::Relaxed); + assert!(store.init("did:key:z6MkOwner", "newrepo").await.is_err()); + let local = repos_dir + .path() + .join("did_key_z6MkOwner") + .join("newrepo.git"); + assert!( + !local.exists(), + "failed init must not leave a local dir behind" + ); + } + + /// Marker + head failure: the write path fails closed, the read path + /// serves the local copy. + #[tokio::test] + async fn marker_with_failing_head_fails_write_closed_serves_read() { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let flaky = FlakyStore::new(store_root.path()); + // sync_down never touches the pool — lazy is fine here. + let pool = sqlx::PgPool::connect_lazy("postgres://invalid").unwrap(); + let store = store_with_flaky(repos_dir.path().to_path_buf(), Arc::clone(&flaky), pool); + + let local = repos_dir.path().join("owner").join("repo.git"); + std::fs::create_dir_all(&local).unwrap(); + std::fs::write(local.join("HEAD"), b"pending\n").unwrap(); + mark_pending_upload(&local, Some("base-etag")).unwrap(); + + flaky + .fail_head + .store(true, std::sync::atomic::Ordering::Relaxed); + assert!( + store + .sync_down_if_stale("owner", "repo", &local, true) + .await + .is_err(), + "write path must fail closed when freshness is unknowable" + ); + store + .sync_down_if_stale("owner", "repo", &local, false) + .await + .expect("read path serves the local copy"); + assert_eq!(std::fs::read(local.join("HEAD")).unwrap(), b"pending\n"); + } + + /// Startup sweep: re-uploads marked repos whose storage didn't move, and + /// leaves diverged ones marked. + #[sqlx::test] + async fn retry_pending_uploads_heals_and_respects_divergence(pool: PgPool) { + let store_root = tempfile::tempdir().unwrap(); + let repos_dir = tempfile::tempdir().unwrap(); + let flaky = FlakyStore::new(store_root.path()); + let store = store_with_flaky(repos_dir.path().to_path_buf(), Arc::clone(&flaky), pool); + let archive = store.archive.as_ref().unwrap(); + + // Repo A: storage v1, local ahead with matching base — heals. + let seed = tempfile::tempdir().unwrap(); + std::fs::write(seed.path().join("HEAD"), b"v1\n").unwrap(); + archive.upload("owner", "heals", seed.path()).await.unwrap(); + let base_a = archive.head_etag("owner", "heals").await.unwrap().unwrap(); + let local_a = repos_dir.path().join("owner").join("heals.git"); + std::fs::create_dir_all(&local_a).unwrap(); + std::fs::write(local_a.join("HEAD"), b"local-ahead\n").unwrap(); + mark_pending_upload(&local_a, Some(&base_a)).unwrap(); + + // Repo B: marker base predates current storage — stays marked. + archive + .upload("owner", "diverged", seed.path()) + .await + .unwrap(); + let local_b = repos_dir.path().join("owner").join("diverged.git"); + std::fs::create_dir_all(&local_b).unwrap(); + std::fs::write(local_b.join("HEAD"), b"local-b\n").unwrap(); + mark_pending_upload(&local_b, Some("stale-base")).unwrap(); + + let (reuploaded, still_pending) = store.retry_pending_uploads().await; + assert_eq!((reuploaded, still_pending), (1, 1)); + assert!(!pending_upload_marker(&local_a).unwrap().exists()); + assert!(pending_upload_marker(&local_b).unwrap().exists()); + + // A's local content is now durably in storage. + let out = tempfile::tempdir().unwrap(); + let restored = out.path().join("restored.git"); + archive.download("owner", "heals", &restored).await.unwrap(); + assert_eq!( + std::fs::read(restored.join("HEAD")).unwrap(), + b"local-ahead\n" + ); + } } diff --git a/crates/gitlawb-node/src/git/tigris.rs b/crates/gitlawb-node/src/git/tigris.rs deleted file mode 100644 index cf7abfd5f..000000000 --- a/crates/gitlawb-node/src/git/tigris.rs +++ /dev/null @@ -1,245 +0,0 @@ -//! Tigris (S3-compatible) storage client for git bare repos. -//! -//! Repos are stored as `repos/v1/{owner_slug}/{repo_name}.tar.zst` — a -//! zstd-compressed tar archive of the bare repo directory. - -use std::collections::HashMap; -use std::path::{Path, PathBuf}; -use std::sync::{Arc, Mutex, OnceLock}; - -use anyhow::{Context, Result}; -use aws_sdk_s3::Client as S3Client; -use tracing::{debug, info}; - -/// Wrapper around the S3 client with the configured bucket. -#[derive(Clone)] -pub struct TigrisClient { - s3: S3Client, - bucket: String, -} - -impl TigrisClient { - /// Create a new client. Uses AWS_ACCESS_KEY_ID, AWS_SECRET_ACCESS_KEY, and - /// AWS_ENDPOINT_URL_S3 env vars — all set automatically by Fly for Tigris buckets. - pub async fn new(bucket: &str) -> Result { - let config = aws_config::load_defaults(aws_config::BehaviorVersion::latest()).await; - let s3 = S3Client::new(&config); - info!(bucket = %bucket, "tigris storage client initialized"); - Ok(Self { - s3, - bucket: bucket.to_string(), - }) - } - - /// Test-only constructor with an explicit S3 endpoint, region, and static - /// credentials — no env-var reads, so parallel tests cannot race each other's - /// `AWS_*` environment the way the env-based `new` would. Lets a test point - /// the client at a non-routable endpoint to exercise acquire-stall paths. - #[cfg(test)] - pub(crate) async fn for_testing_with_endpoint(bucket: &str, endpoint_url: &str) -> Self { - let creds = aws_sdk_s3::config::Credentials::new("test", "test", None, None, "test"); - let config = aws_config::defaults(aws_config::BehaviorVersion::latest()) - .endpoint_url(endpoint_url) - .region(aws_config::Region::new("auto")) - .credentials_provider(creds) - .load() - .await; - Self { - s3: S3Client::new(&config), - bucket: bucket.to_string(), - } - } - - /// S3 key for a given repo: `repos/v1/{owner_slug}/{repo_name}.tar.zst` - fn repo_key(owner_slug: &str, repo_name: &str) -> String { - format!("repos/v1/{owner_slug}/{repo_name}.tar.zst") - } - - /// Check if a repo archive exists in Tigris. - pub async fn exists(&self, owner_slug: &str, repo_name: &str) -> Result { - let key = Self::repo_key(owner_slug, repo_name); - match self - .s3 - .head_object() - .bucket(&self.bucket) - .key(&key) - .send() - .await - { - Ok(_) => Ok(true), - Err(e) => { - if e.as_service_error().is_some_and(|e| e.is_not_found()) { - Ok(false) - } else { - Err(anyhow::anyhow!("tigris HEAD {key}: {e}")) - } - } - } - } - - /// Upload a local bare repo directory to Tigris as a tar.zst archive. - pub async fn upload(&self, owner_slug: &str, repo_name: &str, local_path: &Path) -> Result<()> { - let key = Self::repo_key(owner_slug, repo_name); - debug!(key = %key, path = %local_path.display(), "uploading repo to tigris"); - - // Create tar.zst in memory - let archive_bytes = tokio::task::spawn_blocking({ - let local_path = local_path.to_path_buf(); - move || compress_repo(&local_path) - }) - .await - .context("tar task panicked")? - .context("compressing repo")?; - - let body = aws_sdk_s3::primitives::ByteStream::from(archive_bytes); - - self.s3 - .put_object() - .bucket(&self.bucket) - .key(&key) - .body(body) - .content_type("application/zstd") - .send() - .await - .context(format!("tigris PUT {key}"))?; - - info!(key = %key, "uploaded repo to tigris"); - Ok(()) - } - - /// Download a repo archive from Tigris and extract to local disk. - pub async fn download( - &self, - owner_slug: &str, - repo_name: &str, - local_path: &Path, - ) -> Result<()> { - let key = Self::repo_key(owner_slug, repo_name); - debug!(key = %key, path = %local_path.display(), "downloading repo from tigris"); - - let resp = self - .s3 - .get_object() - .bucket(&self.bucket) - .key(&key) - .send() - .await - .context(format!("tigris GET {key}"))?; - - let data = resp - .body - .collect() - .await - .context("reading tigris response body")? - .into_bytes(); - - // Extract tar.zst to local path - tokio::task::spawn_blocking({ - let local_path = local_path.to_path_buf(); - move || decompress_repo(&data, &local_path) - }) - .await - .context("extract task panicked")? - .context("extracting repo")?; - - info!(key = %key, path = %local_path.display(), "downloaded repo from tigris"); - Ok(()) - } - - /// Delete a repo archive from Tigris. - #[allow(dead_code)] - pub async fn delete(&self, owner_slug: &str, repo_name: &str) -> Result<()> { - let key = Self::repo_key(owner_slug, repo_name); - self.s3 - .delete_object() - .bucket(&self.bucket) - .key(&key) - .send() - .await - .context(format!("tigris DELETE {key}"))?; - Ok(()) - } -} - -/// Compress a bare repo directory into a tar.zst byte vector. -fn compress_repo(repo_path: &Path) -> Result> { - let buf = Vec::new(); - let encoder = zstd::stream::Encoder::new(buf, 3)?; // level 3 = fast + decent ratio - let mut tar = tar::Builder::new(encoder); - - // Append the bare repo directory contents (not the directory itself) - tar.append_dir_all(".", repo_path) - .context("building tar archive")?; - - let encoder = tar.into_inner().context("finishing tar")?; - let compressed = encoder.finish().context("finishing zstd")?; - Ok(compressed) -} - -/// Per-repo-path lock serializing the publish (swap-into-place) step of -/// `decompress_repo`. Concurrent extractions unpack into isolated temp dirs in -/// parallel, but the final `remove_dir_all` + `rename` must not interleave for -/// the same `local_path`, or they race to a nondeterministic overwrite/failure. -fn publish_lock(local_path: &Path) -> Arc> { - // KNOWN LIMITATION: this map is never evicted — one (PathBuf, Arc) - // entry accrues per distinct repo path for the process lifetime. Bounded by - // the number of repos a node hosts, so it's negligible for normal use, but - // high-volume/churning deployments may want LRU or weak-ref eviction here. - static LOCKS: OnceLock>>>> = OnceLock::new(); - let locks = LOCKS.get_or_init(|| Mutex::new(HashMap::new())); - let mut map = locks.lock().expect("publish lock map poisoned"); - map.entry(local_path.to_path_buf()) - .or_insert_with(|| Arc::new(Mutex::new(()))) - .clone() -} - -/// Decompress a tar.zst byte vector into a local directory. -/// -/// Extraction is atomic with respect to `local_path`: the archive is unpacked -/// into a sibling temp directory first, and only swapped into place once it -/// fully succeeds. A corrupt or truncated archive therefore can never clobber a -/// good existing copy at `local_path` — on failure we discard the temp dir and -/// leave `local_path` exactly as it was. -fn decompress_repo(data: &[u8], local_path: &Path) -> Result<()> { - let parent = local_path.parent().context("repo path has no parent")?; - std::fs::create_dir_all(parent).context("creating parent dir")?; - - let file_name = local_path - .file_name() - .context("repo path has no file name")? - .to_string_lossy(); - // Unique per-extraction temp dir: a fixed name would let two concurrent - // extractions of the same repo share one dir and clobber each other's - // in-progress unpack. A fresh UUID also means it can't collide with a - // leftover dir from a previously-interrupted run. - let tmp_dir = parent.join(format!(".{file_name}.tmp-extract.{}", uuid::Uuid::new_v4())); - - std::fs::create_dir_all(&tmp_dir).context("creating temp extract dir")?; - - // Unpack into the temp dir; on any failure, clean up and bail without - // touching local_path. - let unpack = (|| -> Result<()> { - let decoder = zstd::stream::Decoder::new(data)?; - let mut archive = tar::Archive::new(decoder); - archive.unpack(&tmp_dir).context("unpacking tar.zst")?; - Ok(()) - })(); - if let Err(e) = unpack { - let _ = std::fs::remove_dir_all(&tmp_dir); - return Err(e); - } - - // Swap the freshly-extracted repo into place. rename within the same parent - // is effectively atomic, but most platforms refuse to rename onto a - // non-empty dir, so remove the old copy first. Serialize this per repo path: - // concurrent extractions unpack into isolated temp dirs, but their swaps - // must not interleave or they race to a nondeterministic overwrite/failure. - let lock = publish_lock(local_path); - let _publish = lock.lock().expect("publish lock poisoned"); - if local_path.exists() { - std::fs::remove_dir_all(local_path).context("removing stale repo dir")?; - } - std::fs::rename(&tmp_dir, local_path).context("swapping extracted repo into place")?; - - Ok(()) -} diff --git a/crates/gitlawb-node/src/main.rs b/crates/gitlawb-node/src/main.rs index 66bfa0961..4d71a5af5 100644 --- a/crates/gitlawb-node/src/main.rs +++ b/crates/gitlawb-node/src/main.rs @@ -18,6 +18,7 @@ mod pinata; mod rate_limit; mod server; mod state; +mod storage; mod sync; #[cfg(test)] mod test_support; @@ -299,22 +300,14 @@ async fn main() -> Result<()> { info!(" fly machine: {mid}"); } - // Initialize Tigris S3 client if bucket is configured - let tigris = if !config.tigris_bucket.is_empty() { - match git::tigris::TigrisClient::new(&config.tigris_bucket).await { - Ok(client) => { - info!(bucket = %config.tigris_bucket, "tigris storage enabled"); - Some(client) - } - Err(e) => { - tracing::warn!(err = %e, "failed to initialize Tigris client — using local-only storage"); - None - } - } - } else { - info!("tigris storage disabled (no bucket configured)"); - None - }; + // Initialize the storage-agnostic blob backend (S3-compatible / filesystem / + // IPFS), then wrap it in the repo-archive layer. `None` = local-only mode. + // Fail closed: a configured-but-unreachable backend aborts boot rather than + // silently running local-only and dropping durability. + let blob_store = storage::build(&config) + .await + .context("initializing object storage backend")?; + let archive = blob_store.map(storage::archive::RepoArchive::new); // Repo write locks run on their own pool, never the main query pool: each push // holds its connection for the whole receive-pack, so a burst of concurrent @@ -323,13 +316,42 @@ async fn main() -> Result<()> { // whatever the two pools are sized at, which is why the separation is // structural rather than a consequence of the defaults; config validate() // separately requires db_max_connections >= max_concurrent_git_pushes + 8. See - // build_lock_pool for the cancellation semantics (#173). + // build_lock_pool for the cancellation semantics (#173). The pool's + // acquire_timeout bounds only the checkout for a single try-lock round trip + // (waiting for a contended LOCK sleeps with no connection held), so it stays + // at the ordinary DB acquire timeout rather than the 300s lock-wait budget. let lock_pool = git::repo_store::build_lock_pool( db.pool(), lock_pool_size(config.max_concurrent_git_pushes), std::time::Duration::from_secs(config.db_acquire_timeout_secs), ); - let repo_store = git::repo_store::RepoStore::new(config.repos_dir.clone(), tigris, lock_pool); + + // Sweep swap-phase litter (`.tmp-extract.`/`.bak-` dirs) orphaned by a hard + // kill mid-extraction. Must run before any request can start an extraction, + // as a live swap owns exactly these names — synchronous here, and cheap: + // it's a two-level directory scan that removes only matching orphans. + { + let removed = storage::archive::sweep_orphaned_swap_dirs(&config.repos_dir); + if removed > 0 { + info!(removed, "swept orphaned repo swap dirs from previous run"); + } + } + + let repo_store = git::repo_store::RepoStore::new(config.repos_dir.clone(), archive, lock_pool); + + // Re-attempt uploads for repos whose pending-upload marker survived a + // crash or failed upload — otherwise a repo with no further writes stays + // divergent from storage indefinitely. Background task: it takes the + // per-repo advisory locks, so it serializes correctly with live pushes. + { + let store = repo_store.clone(); + tokio::spawn(async move { + let (reuploaded, still_pending) = store.retry_pending_uploads().await; + if reuploaded > 0 || still_pending > 0 { + info!(reuploaded, still_pending, "pending-upload marker sweep"); + } + }); + } // Per-DID limiter for the creation endpoints. Keyed on the authenticated // DID (attacker-varied), so bound its key set to cap memory. diff --git a/crates/gitlawb-node/src/metrics.rs b/crates/gitlawb-node/src/metrics.rs index c95ef1d18..1e540214e 100644 --- a/crates/gitlawb-node/src/metrics.rs +++ b/crates/gitlawb-node/src/metrics.rs @@ -15,6 +15,8 @@ //! `gitlawb_pack_size_bytes` //! * a single `gitlawb_info{version, did}` gauge = 1, for joins/dashboards //! * currently-connected peer count — `gitlawb_peers_connected` +//! * repos whose local copy is ahead of durable storage — +//! `gitlawb_pending_upload_markers` //! //! All metrics live in a single process-wide registry initialized by //! [`init`]. Increment helpers (`record_push`, `record_auth_failure`, ...) @@ -51,6 +53,7 @@ static SYNC_PROCESSED: OnceLock = OnceLock::new(); static WEBHOOK_DELIVERIES: OnceLock = OnceLock::new(); static PACK_SIZE: OnceLock = OnceLock::new(); static PEERS_CONNECTED: OnceLock = OnceLock::new(); +static PENDING_UPLOAD_MARKERS: OnceLock = OnceLock::new(); /// One-time initializer. Builds the registry, registers every metric, /// and sets the constant `gitlawb_info` gauge. Idempotent — calling @@ -202,6 +205,18 @@ fn init_inner(version: &str, node_did: &str) { .set(peers_connected) .expect("set PEERS_CONNECTED once"); + let pending_markers = IntGauge::with_opts(Opts::new( + "gitlawb_pending_upload_markers", + "Repos whose local copy is ahead of durable storage (pending-upload markers outstanding)", + )) + .expect("gitlawb_pending_upload_markers definition"); + registry + .register(Box::new(pending_markers.clone())) + .expect("register gitlawb_pending_upload_markers"); + PENDING_UPLOAD_MARKERS + .set(pending_markers) + .expect("set PENDING_UPLOAD_MARKERS once"); + REGISTRY .set(registry) .expect("set REGISTRY once (init must be called exactly once)"); @@ -284,6 +299,24 @@ pub fn set_peers_connected(count: i64) { } } +/// Update the outstanding pending-upload marker gauge (repos whose local copy +/// is ahead of durable storage). +pub fn set_pending_upload_markers(count: i64) { + if let Some(g) = PENDING_UPLOAD_MARKERS.get() { + g.set(count); + } +} + +/// Adjust the pending-upload marker gauge by a delta (marker created = +1, +/// marker removed = -1). The startup sweep seeds the absolute count via +/// [`set_pending_upload_markers`]; runtime marker churn keeps it current +/// through this. +pub fn add_pending_upload_markers(delta: i64) { + if let Some(g) = PENDING_UPLOAD_MARKERS.get() { + g.add(delta); + } +} + /// Encode the registry as the standard Prometheus text exposition format. /// Returns an error if `init` was never called. pub fn encode() -> Result { diff --git a/crates/gitlawb-node/src/state.rs b/crates/gitlawb-node/src/state.rs index 24607e5ad..642c31276 100644 --- a/crates/gitlawb-node/src/state.rs +++ b/crates/gitlawb-node/src/state.rs @@ -61,7 +61,7 @@ pub struct AppState { pub graphql_schema: Arc, /// Fly.io machine ID — used for fly-replay routing in multi-machine deployments pub machine_id: Option, - /// Centralized repo storage: local disk cache + optional Tigris backend + /// Centralized repo storage: local disk cache + optional object-store backend pub repo_store: RepoStore, /// Per-DID rate limiter for creation endpoints (repos, issues, PRs) pub rate_limiter: RateLimiter, diff --git a/crates/gitlawb-node/src/storage/archive.rs b/crates/gitlawb-node/src/storage/archive.rs new file mode 100644 index 000000000..9cd7910a9 --- /dev/null +++ b/crates/gitlawb-node/src/storage/archive.rs @@ -0,0 +1,446 @@ +//! Repo-archive layer: stores a bare git repo as a single +//! `repos/v1/{owner_slug}/{repo_name}.tar.zst` object on top of any +//! [`BlobStore`] backend. Backend-agnostic replacement for the old +//! single-backend (Tigris-only) client. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; +use std::sync::{Arc, Mutex, OnceLock}; + +use anyhow::{Context, Result}; +use bytes::Bytes; +use tracing::{debug, info}; + +use super::BlobStore; + +#[derive(Clone)] +pub struct RepoArchive { + store: Arc, +} + +impl RepoArchive { + pub fn new(store: Arc) -> Self { + Self { store } + } + + /// Object key for a repo archive. + fn key(owner_slug: &str, repo_name: &str) -> String { + format!("repos/v1/{owner_slug}/{repo_name}.tar.zst") + } + + /// Current archive etag, or `None` if the repo isn't in storage yet. + pub async fn head_etag(&self, owner_slug: &str, repo_name: &str) -> Result> { + let key = Self::key(owner_slug, repo_name); + Ok(self + .store + .head(&key) + .await? + .map(|m| m.etag.unwrap_or_else(|| format!("size:{}", m.size)))) + } + + /// Whether the repo archive exists in storage. + pub async fn exists(&self, owner_slug: &str, repo_name: &str) -> Result { + Ok(self + .store + .head(&Self::key(owner_slug, repo_name)) + .await? + .is_some()) + } + + /// Compress the bare repo and upload it. Returns the new etag (for the + /// skip-redundant-download cache). + pub async fn upload( + &self, + owner_slug: &str, + repo_name: &str, + local_path: &Path, + ) -> Result> { + self.upload_with_intent(owner_slug, repo_name, local_path, |_| Ok(())) + .await + } + + /// Like [`upload`](Self::upload), but calls `record_intent` with the + /// archive's content MD5 after compressing and BEFORE the PUT. On backends + /// whose etag is the body MD5 (S3-compatibles for single-part puts; the fs + /// backend by construction), that value equals the etag the PUT will + /// produce, letting crash recovery recognize "storage holds exactly what + /// this node was uploading" — its own completed upload, not divergence. + /// `record_intent` returning `Err` ABORTS the upload before the PUT: an + /// intent that could not be durably recorded must not be outrun by the + /// upload it describes, or a crash after the PUT reads as external + /// divergence. Recovery validates a candidate by fetching the remote + /// bytes and comparing their MD5, so the mechanism is backend-independent + /// (it does not assume the backend's etag is a content hash). + pub async fn upload_with_intent( + &self, + owner_slug: &str, + repo_name: &str, + local_path: &Path, + record_intent: F, + ) -> Result> + where + F: FnOnce(&str) -> Result<()> + Send, + { + let key = Self::key(owner_slug, repo_name); + let archive_bytes = tokio::task::spawn_blocking({ + let local_path = local_path.to_path_buf(); + move || compress_repo(&local_path) + }) + .await + .context("tar task panicked")? + .context("compressing repo")?; + + record_intent(&content_md5_hex(&archive_bytes)) + .context("recording upload intent before PUT")?; + + let meta = self + .store + .put(&key, Bytes::from(archive_bytes)) + .await + .context("uploading repo archive")?; + info!(key = %key, backend = self.store.backend_name(), "uploaded repo archive"); + Ok(meta.etag.or_else(|| Some(format!("size:{}", meta.size)))) + } + + /// Download the repo archive and extract it to `local_path` (atomic swap). + pub async fn download( + &self, + owner_slug: &str, + repo_name: &str, + local_path: &Path, + ) -> Result<()> { + let key = Self::key(owner_slug, repo_name); + debug!(key = %key, "downloading repo archive"); + let data = self + .store + .get(&key) + .await + .context("fetching repo archive")? + .ok_or_else(|| anyhow::anyhow!("repo archive missing: {key}"))?; + + tokio::task::spawn_blocking({ + let local_path = local_path.to_path_buf(); + move || decompress_repo(&data, &local_path) + }) + .await + .context("extract task panicked")? + .context("extracting repo")?; + info!(key = %key, path = %local_path.display(), "downloaded repo archive"); + Ok(()) + } + + /// Fetch the raw archive object bytes (no extraction). Recovery-path + /// helper: lets the caller validate a heal candidate by hashing the + /// actual remote content instead of trusting backend etag semantics. + pub async fn fetch_raw(&self, owner_slug: &str, repo_name: &str) -> Result> { + self.store.get(&Self::key(owner_slug, repo_name)).await + } + + /// Delete a repo archive. No production caller yet (creation flows are + /// claim-first, so they never need to delete an archive); kept for the + /// repo-deletion path that will need it. + #[allow(dead_code)] + pub async fn delete(&self, owner_slug: &str, repo_name: &str) -> Result<()> { + self.store.delete(&Self::key(owner_slug, repo_name)).await + } +} + +/// Hex MD5 of a byte body — the etag an S3-compatible single-part PUT (and +/// the fs backend, by construction) will assign to it. +pub(crate) fn content_md5_hex(bytes: &[u8]) -> String { + use md5::{Digest, Md5}; + format!("{:x}", Md5::digest(bytes)) +} + +/// Remove orphaned swap-phase work dirs (`.{repo}.tmp-extract.{uuid}` and +/// `.{repo}.bak-{uuid}`) left under `repos_dir/{owner_slug}/` by a process +/// killed mid-`decompress_repo`. Safe to run only at startup, before any +/// extraction is in flight: a live swap owns exactly these names. Deleting a +/// `.bak-` orphan loses no data — these dirs only exist on the download path, +/// so storage still holds the archive and the next `acquire()` re-downloads. +/// Returns the number of directories removed. +pub fn sweep_orphaned_swap_dirs(repos_dir: &Path) -> usize { + let mut removed = 0; + let Ok(owners) = std::fs::read_dir(repos_dir) else { + return 0; + }; + for owner in owners.flatten() { + let owner_path = owner.path(); + if !owner_path.is_dir() { + continue; + } + let Ok(entries) = std::fs::read_dir(&owner_path) else { + continue; + }; + for entry in entries.flatten() { + let name = entry.file_name(); + let name = name.to_string_lossy(); + let is_swap_litter = + name.starts_with('.') && (name.contains(".tmp-extract.") || name.contains(".bak-")); + if is_swap_litter && entry.path().is_dir() { + match std::fs::remove_dir_all(entry.path()) { + Ok(()) => { + info!(dir = %entry.path().display(), "removed orphaned swap dir"); + removed += 1; + } + Err(e) => { + tracing::warn!(dir = %entry.path().display(), err = %e, + "failed to remove orphaned swap dir"); + } + } + } + } + } + removed +} + +/// Compress a bare repo directory into a tar.zst byte vector. +fn compress_repo(repo_path: &Path) -> Result> { + let buf = Vec::new(); + // Level 3 = fast + decent ratio. Compression dominates the post-push upload + // for larger repos; zstd's multithreaded mode splits the stream across + // worker threads for a near-linear speedup at the same ratio. Capped so a + // single push can't monopolize a small node's cores. + let mut encoder = zstd::stream::Encoder::new(buf, 3)?; + let workers = std::thread::available_parallelism() + .map(|n| n.get().min(4) as u32) + .unwrap_or(1); + if workers > 1 { + encoder + .multithread(workers) + .context("enabling multithreaded zstd")?; + } + let mut tar = tar::Builder::new(encoder); + tar.append_dir_all(".", repo_path) + .context("building tar archive")?; + let encoder = tar.into_inner().context("finishing tar")?; + let compressed = encoder.finish().context("finishing zstd")?; + Ok(compressed) +} + +/// Per-repo-path lock serializing the publish (swap-into-place) step of +/// `decompress_repo`. Concurrent extractions unpack into isolated temp dirs in +/// parallel, but the final `remove_dir_all` + `rename` must not interleave for +/// the same `local_path`, or they race to a nondeterministic overwrite/failure. +fn publish_lock(local_path: &Path) -> Arc> { + // KNOWN LIMITATION: this map is never evicted — one (PathBuf, Arc) + // entry accrues per distinct repo path for the process lifetime. Bounded by + // the number of repos a node hosts, so it's negligible for normal use, but + // high-volume/churning deployments may want LRU or weak-ref eviction here. + static LOCKS: OnceLock>>>> = OnceLock::new(); + let locks = LOCKS.get_or_init(|| Mutex::new(HashMap::new())); + let mut map = locks.lock().expect("publish lock map poisoned"); + map.entry(local_path.to_path_buf()) + .or_insert_with(|| Arc::new(Mutex::new(()))) + .clone() +} + +/// Decompress a tar.zst byte vector into a local directory. +/// +/// Extraction is atomic with respect to `local_path`: the archive is unpacked +/// into a sibling temp directory first, and only swapped into place once it +/// fully succeeds. A corrupt or truncated archive therefore can never clobber a +/// good existing copy at `local_path` — on failure we discard the temp dir and +/// leave `local_path` exactly as it was. +fn decompress_repo(data: &[u8], local_path: &Path) -> Result<()> { + let parent = local_path.parent().context("repo path has no parent")?; + std::fs::create_dir_all(parent).context("creating parent dir")?; + + let file_name = local_path + .file_name() + .context("repo path has no file name")? + .to_string_lossy(); + // Unique per-extraction temp dir: a fixed name would let two concurrent + // extractions of the same repo share one dir and clobber each other's + // in-progress unpack. A fresh UUID also means it can't collide with a + // leftover dir from a previously-interrupted run. + // + // Built through the shared sibling barrier rather than a bare `join`: the + // name embeds `file_name`, which carries user-provided data from the repo + // name, and this path is handed straight to `create_dir_all`, `unpack` and + // `remove_dir_all`. See `repo_store::validated_sibling_path`. + let tmp_dir = crate::git::repo_store::validated_sibling_path( + local_path, + &format!(".{file_name}.tmp-extract.{}", uuid::Uuid::new_v4()), + )?; + + std::fs::create_dir_all(&tmp_dir).context("creating temp extract dir")?; + + let unpack = (|| -> Result<()> { + let decoder = zstd::stream::Decoder::new(data)?; + let mut archive = tar::Archive::new(decoder); + archive.unpack(&tmp_dir).context("unpacking tar.zst")?; + Ok(()) + })(); + if let Err(e) = unpack { + let _ = std::fs::remove_dir_all(&tmp_dir); + return Err(e); + } + + // Swap the freshly-extracted repo into place. rename within the same parent + // is effectively atomic, but most platforms refuse to rename onto a + // non-empty dir, so remove the old copy first. Serialize this per repo path: + // concurrent extractions unpack into isolated temp dirs, but their swaps + // must not interleave or they race to a nondeterministic overwrite/failure. + let lock = publish_lock(local_path); + let _publish = lock.lock().expect("publish lock poisoned"); + let swap = (|| -> Result<()> { + // Move any existing repo aside to a backup first, rather than deleting + // it up front: if the rename of the new copy then fails, we restore the + // backup so `local_path` is never left without a valid repo. (Most + // platforms refuse to rename onto a non-empty dir, hence the move-aside.) + let backup = if local_path.exists() { + // Same sibling barrier as the temp-extract dir above: the name + // embeds the user-provided repo file name, and this path is renamed + // onto and later removed recursively. + let b = crate::git::repo_store::validated_sibling_path( + local_path, + &format!(".{file_name}.bak-{}", uuid::Uuid::new_v4()), + )?; + std::fs::rename(local_path, &b).context("moving existing repo to backup")?; + Some(b) + } else { + None + }; + match std::fs::rename(&tmp_dir, local_path).context("swapping extracted repo into place") { + Ok(()) => { + if let Some(b) = backup { + let _ = std::fs::remove_dir_all(&b); + } + Ok(()) + } + Err(e) => { + // Restore the previous copy so the repo isn't left missing. + if let Some(b) = backup { + let _ = std::fs::rename(&b, local_path); + } + Err(e) + } + } + })(); + if swap.is_err() { + // Don't leak the extracted temp dir if the swap failed. + let _ = std::fs::remove_dir_all(&tmp_dir); + } + swap +} + +#[cfg(test)] +mod tests { + use super::*; + use std::fs; + + fn seed_repo(dir: &std::path::Path) { + fs::create_dir_all(dir.join("refs/heads")).unwrap(); + fs::write(dir.join("HEAD"), b"ref: refs/heads/main\n").unwrap(); + fs::write(dir.join("refs/heads/main"), b"abc123\n").unwrap(); + fs::write(dir.join("config"), b"[core]\n\tbare = true\n").unwrap(); + } + + #[test] + fn compress_decompress_round_trip_preserves_files() { + let src = tempfile::tempdir().unwrap(); + seed_repo(src.path()); + + let bytes = compress_repo(src.path()).unwrap(); + assert!(!bytes.is_empty()); + + let out_parent = tempfile::tempdir().unwrap(); + let out = out_parent.path().join("restored.git"); + decompress_repo(&bytes, &out).unwrap(); + + assert_eq!( + fs::read(out.join("HEAD")).unwrap(), + b"ref: refs/heads/main\n" + ); + assert_eq!(fs::read(out.join("refs/heads/main")).unwrap(), b"abc123\n"); + assert_eq!( + fs::read(out.join("config")).unwrap(), + b"[core]\n\tbare = true\n" + ); + } + + #[test] + fn decompress_swap_replaces_existing_dir_atomically() { + let src = tempfile::tempdir().unwrap(); + fs::write(src.path().join("HEAD"), b"new\n").unwrap(); + let bytes = compress_repo(src.path()).unwrap(); + + // Pre-existing copy with stale junk that the swap must fully replace. + let out_parent = tempfile::tempdir().unwrap(); + let out = out_parent.path().join("repo.git"); + fs::create_dir_all(&out).unwrap(); + fs::write(out.join("STALE"), b"old\n").unwrap(); + + decompress_repo(&bytes, &out).unwrap(); + assert_eq!(fs::read(out.join("HEAD")).unwrap(), b"new\n"); + assert!( + !out.join("STALE").exists(), + "stale content must be gone after the swap" + ); + } + + #[test] + fn decompress_corrupt_archive_leaves_existing_copy_untouched() { + let out_parent = tempfile::tempdir().unwrap(); + let out = out_parent.path().join("repo.git"); + fs::create_dir_all(&out).unwrap(); + fs::write(out.join("HEAD"), b"good\n").unwrap(); + + // Garbage is not a valid tar.zst: unpack fails before the swap, so the + // existing copy is preserved (atomicity claim). + assert!(decompress_repo(b"not a real archive", &out).is_err()); + assert_eq!(fs::read(out.join("HEAD")).unwrap(), b"good\n"); + } + + #[test] + fn sweep_removes_only_orphaned_swap_dirs() { + let repos = tempfile::tempdir().unwrap(); + let owner = repos.path().join("did_key_z6MkAlice"); + + // Litter from an interrupted swap... + let tmp_extract = owner.join(".repo.git.tmp-extract.1234-uuid"); + let bak = owner.join(".repo.git.bak-5678-uuid"); + // ...alongside things the sweep must not touch. + let live_repo = owner.join("repo.git"); + let dotfile = owner.join(".keep"); + fs::create_dir_all(&tmp_extract).unwrap(); + fs::create_dir_all(&bak).unwrap(); + fs::create_dir_all(&live_repo).unwrap(); + fs::write(&dotfile, b"").unwrap(); + + assert_eq!(sweep_orphaned_swap_dirs(repos.path()), 2); + assert!(!tmp_extract.exists()); + assert!(!bak.exists()); + assert!(live_repo.exists()); + assert!(dotfile.exists()); + + // Idempotent on a clean tree. + assert_eq!(sweep_orphaned_swap_dirs(repos.path()), 0); + } + + #[tokio::test] + async fn upload_download_round_trip_over_fs_backend() { + let store_dir = tempfile::tempdir().unwrap(); + let store: Arc = + Arc::new(crate::storage::fs::FsBlobStore::new(store_dir.path()).unwrap()); + let archive = RepoArchive::new(store); + + let src = tempfile::tempdir().unwrap(); + seed_repo(src.path()); + + assert!(!archive.exists("owner", "repo").await.unwrap()); + let etag = archive.upload("owner", "repo", src.path()).await.unwrap(); + assert!(etag.is_some()); + assert!(archive.exists("owner", "repo").await.unwrap()); + + let out_parent = tempfile::tempdir().unwrap(); + let out = out_parent.path().join("repo.git"); + archive.download("owner", "repo", &out).await.unwrap(); + assert_eq!( + fs::read(out.join("HEAD")).unwrap(), + b"ref: refs/heads/main\n" + ); + assert_eq!(fs::read(out.join("refs/heads/main")).unwrap(), b"abc123\n"); + } +} diff --git a/crates/gitlawb-node/src/storage/fs.rs b/crates/gitlawb-node/src/storage/fs.rs new file mode 100644 index 000000000..2f865f723 --- /dev/null +++ b/crates/gitlawb-node/src/storage/fs.rs @@ -0,0 +1,374 @@ +//! Local filesystem blob backend. +//! +//! Stores each object as a file under a configured root directory, using the +//! object key as a relative path. For self-hosters without S3 and for tests of +//! the storage abstraction. The etag is the body's MD5 (matching S3 semantics +//! for single-part puts) persisted in a `.etag` sidecar at write time, so the +//! skip-redundant-download optimization can rely on "etag unchanged ⇒ content +//! unchanged" even on filesystems with coarse timestamps, and an uploader can +//! predict the etag its PUT will produce (crash-recovery provenance). Objects +//! predating the sidecar fall back to a size-mtime fingerprint. + +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use async_trait::async_trait; +use bytes::Bytes; + +use super::{validate_key, BlobStore, ObjectMeta}; + +#[derive(Clone)] +pub struct FsBlobStore { + root: PathBuf, +} + +impl FsBlobStore { + pub fn new(root: impl AsRef) -> Result { + let root = root.as_ref().to_path_buf(); + std::fs::create_dir_all(&root) + .with_context(|| format!("creating storage dir {}", root.display()))?; + Ok(Self { root }) + } + + fn path_for(&self, key: &str) -> Result { + validate_key(key)?; + let path = self.root.join(key); + // Defence in depth: the resolved path must stay under root. + if !path.starts_with(&self.root) { + anyhow::bail!("blob key escaped storage root: {key}"); + } + Ok(path) + } + + /// Sidecar file persisting the object's etag: the body's MD5, computed at + /// write time. RepoStore treats etag equality as proof the local copy is + /// current, so different content must always yield a different token — a + /// `size-mtime` fingerprint cannot guarantee that on mounted filesystems + /// with coarse timestamp precision (two different same-size writes in one + /// tick collide); a content hash can. Identical content re-written yields + /// the same etag, which is semantically exact for a freshness token. + fn sidecar_of(path: &Path) -> PathBuf { + let mut os = path.as_os_str().to_owned(); + os.push(".etag"); + PathBuf::from(os) + } + + /// Striped lock serializing the sidecar+blob publish step of `put`. + /// The two renames are only pairwise-consistent when same-key puts don't + /// interleave: unserialized, put A's content can land under put B's etag, + /// and "etag unchanged ⇒ content unchanged" breaks in the direction that + /// serves stale data as current. A fixed stripe array (same-key puts + /// always hash to the same stripe) bounds memory regardless of how many + /// distinct keys the process ever writes; cross-key collisions only cost + /// a moment of extra serialization on the cheap rename pair. + fn put_publish_lock(path: &Path) -> &'static std::sync::Mutex<()> { + use std::hash::{Hash, Hasher}; + use std::sync::{Mutex, OnceLock}; + const STRIPES: usize = 64; + static LOCKS: OnceLock>> = OnceLock::new(); + let locks = LOCKS.get_or_init(|| (0..STRIPES).map(|_| Mutex::new(())).collect()); + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + path.hash(&mut hasher); + &locks[(hasher.finish() as usize) % STRIPES] + } + + /// Fallback fingerprint for objects written before the sidecar existed. + fn legacy_etag(md: &std::fs::Metadata) -> String { + let mtime = md + .modified() + .ok() + .and_then(|t| t.duration_since(std::time::UNIX_EPOCH).ok()) + .map(|d| d.as_nanos()) + .unwrap_or(0); + format!("{}-{}", md.len(), mtime) + } +} + +#[async_trait] +impl BlobStore for FsBlobStore { + fn backend_name(&self) -> &'static str { + "fs" + } + + async fn get(&self, key: &str) -> Result> { + let path = self.path_for(key)?; + match tokio::fs::read(&path).await { + Ok(data) => Ok(Some(Bytes::from(data))), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(e).context(format!("reading {}", path.display())), + } + } + + async fn put(&self, key: &str, body: Bytes) -> Result { + let path = self.path_for(key)?; + let parent = path + .parent() + .context("blob path has no parent")? + .to_path_buf(); + // Unique temp name per write: a fixed suffix would let concurrent puts + // to the same key overwrite each other's temp file and corrupt the blob. + let tmp = path.with_extension(format!("{}.tmp-put", uuid::Uuid::new_v4())); + let path2 = path.clone(); + // Atomic write: temp file in the same dir, then rename into place. On any + // failure, remove the temp file so a failed write can't leak it. The + // trailing stat + etag-sidecar write run inside the same blocking task — + // no synchronous fs call ever touches the async runtime. + tokio::task::spawn_blocking(move || -> Result { + std::fs::create_dir_all(&parent).context("creating blob parent dir")?; + let etag = crate::storage::archive::content_md5_hex(&body); + let sidecar = Self::sidecar_of(&path2); + let sidecar_tmp = sidecar.with_extension(format!("{}.tmp-put", uuid::Uuid::new_v4())); + // Remember the published etag so a failed blob rename can restore + // it: leaving the NEW etag over the OLD content is no longer a + // harmless redundant download — under the pending-marker rules an + // unexplained remote etag is terminal divergence. + let prev_etag = std::fs::read_to_string(&sidecar).ok(); + let write_and_swap = (|| -> Result<()> { + std::fs::write(&tmp, &body).context("writing temp blob")?; + // Publish the new etag BEFORE the blob (each via its own + // tmp+rename): a crash between the two renames then yields + // new-etag/old-content — a redundant download, or at worst a + // loud-and-safe pending-marker divergence — instead of + // old-etag/new-content, which etag-equality consumers would + // silently serve as current while stale. The publish pair is + // serialized per key so concurrent same-key puts can't + // interleave one put's etag with another's content. + let _guard = Self::put_publish_lock(&path2) + .lock() + .expect("put publish lock poisoned"); + std::fs::write(&sidecar_tmp, &etag).context("writing etag sidecar")?; + std::fs::rename(&sidecar_tmp, &sidecar).context("publishing etag sidecar")?; + std::fs::rename(&tmp, &path2).context("renaming blob into place")?; + Ok(()) + })(); + if let Err(e) = write_and_swap { + let _ = std::fs::remove_file(&tmp); + let _ = std::fs::remove_file(&sidecar_tmp); + // Roll the sidecar back to describe the content actually on + // disk (the sidecar may have been published before the blob + // rename failed). + match prev_etag { + Some(prev) => { + let _ = std::fs::write(&sidecar, prev); + } + None => { + let _ = std::fs::remove_file(&sidecar); + } + } + return Err(e); + } + let md = std::fs::metadata(&path2).context("stat blob after write")?; + Ok(ObjectMeta { + size: md.len(), + etag: Some(etag), + }) + }) + .await + .context("fs put task panicked")? + } + + async fn head(&self, key: &str) -> Result> { + let path = self.path_for(key)?; + // Probe existence by io error kind, not path.exists(): a permission/IO + // error must surface, not be silently reported as "not found". + let md = match tokio::fs::metadata(&path).await { + Ok(md) => md, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(e) => return Err(e).context(format!("stat {}", path.display())), + }; + let etag = match tokio::fs::read_to_string(Self::sidecar_of(&path)).await { + Ok(tag) => tag.trim().to_string(), + // Object written before the sidecar existed — legacy fingerprint. + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Self::legacy_etag(&md), + Err(e) => { + return Err(e).context(format!("reading etag sidecar for {}", path.display())) + } + }; + Ok(Some(ObjectMeta { + size: md.len(), + etag: Some(etag), + })) + } + + async fn delete(&self, key: &str) -> Result<()> { + let path = self.path_for(key)?; + match tokio::fs::remove_file(&path).await { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => return Err(e).context(format!("deleting {}", path.display())), + } + match tokio::fs::remove_file(Self::sidecar_of(&path)).await { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(e).context(format!("deleting etag sidecar for {}", path.display())), + } + } +} + +/// Test-only helper: enumerate stored keys. `list` was dropped from the +/// `BlobStore` trait until a production consumer (GC/admin/migration) exists; +/// the tests here still need it to assert on stored state. +#[cfg(test)] +impl FsBlobStore { + async fn list(&self, prefix: &str) -> Result> { + let root = self.root.clone(); + let prefix = prefix.to_string(); + tokio::task::spawn_blocking(move || -> Result> { + let mut keys = Vec::new(); + let mut stack = vec![root.clone()]; + while let Some(dir) = stack.pop() { + // Propagate read errors rather than skipping: a partial listing + // reported as success would mislead GC/admin/migration callers. + let rd = std::fs::read_dir(&dir) + .with_context(|| format!("listing {}", dir.display()))?; + for entry in rd { + // Propagate per-entry errors rather than dropping them via + // flatten(): a partial listing must not look like success. + let entry = + entry.with_context(|| format!("reading entry under {}", dir.display()))?; + let path = entry.path(); + if path.is_dir() { + stack.push(path); + } else if let Ok(rel) = path.strip_prefix(&root) { + let key = rel.to_string_lossy().replace('\\', "/"); + // Etag sidecars are backend metadata, not objects. + if key.starts_with(&prefix) && !key.ends_with(".etag") { + keys.push(key); + } + } + } + } + Ok(keys) + }) + .await + .context("fs list task panicked")? + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn put_get_head_delete_list_roundtrip() { + let dir = tempfile::tempdir().unwrap(); + let store = FsBlobStore::new(dir.path()).unwrap(); + + // Absent key + assert!(store.get("repos/v1/a/x.tar.zst").await.unwrap().is_none()); + assert!(store.head("repos/v1/a/x.tar.zst").await.unwrap().is_none()); + + // Put then get + let body = Bytes::from_static(b"hello blob"); + let meta = store + .put("repos/v1/a/x.tar.zst", body.clone()) + .await + .unwrap(); + assert_eq!(meta.size, body.len() as u64); + assert!(meta.etag.is_some()); + let got = store.get("repos/v1/a/x.tar.zst").await.unwrap().unwrap(); + assert_eq!(got, body); + + // Head returns matching etag (stable across reads) + let h = store.head("repos/v1/a/x.tar.zst").await.unwrap().unwrap(); + assert_eq!(h.etag, meta.etag); + + // List by prefix + store + .put("repos/v1/b/y.tar.zst", Bytes::from_static(b"y")) + .await + .unwrap(); + let mut keys = store.list("repos/v1/").await.unwrap(); + keys.sort(); + assert_eq!(keys, vec!["repos/v1/a/x.tar.zst", "repos/v1/b/y.tar.zst"]); + + // Delete is idempotent + store.delete("repos/v1/a/x.tar.zst").await.unwrap(); + store.delete("repos/v1/a/x.tar.zst").await.unwrap(); + assert!(store.get("repos/v1/a/x.tar.zst").await.unwrap().is_none()); + } + + #[tokio::test] + async fn etag_is_persisted_content_md5() { + let dir = tempfile::tempdir().unwrap(); + let store = FsBlobStore::new(dir.path()).unwrap(); + let key = "repos/v1/a/x.tar.zst"; + let body = Bytes::from_static(b"same bytes"); + + // Content-addressed: different same-size content must yield a + // different etag even inside one coarse-filesystem timestamp tick + // (the size-mtime failure mode); identical content is the same token. + let m1 = store.put(key, body.clone()).await.unwrap(); + let m2 = store + .put(key, Bytes::from_static(b"diff bytes")) + .await + .unwrap(); + assert_ne!(m1.etag, m2.etag, "different content ⇒ different etag"); + let m3 = store.put(key, body).await.unwrap(); + assert_eq!(m1.etag, m3.etag, "identical content ⇒ identical etag"); + assert_eq!( + m3.etag.as_deref(), + Some(crate::storage::archive::content_md5_hex(b"same bytes").as_str()), + "etag must be the body MD5 (predictable pre-PUT)" + ); + + // head() reports the persisted sidecar etag, not a recomputed + // fingerprint. + let h = store.head(key).await.unwrap().unwrap(); + assert_eq!(h.etag, m3.etag); + let sidecar = dir.path().join(format!("{key}.etag")); + assert_eq!( + std::fs::read_to_string(&sidecar).unwrap(), + m3.etag.clone().unwrap(), + "etag must come from the persisted sidecar" + ); + + // delete() removes the sidecar with the object. + store.delete(key).await.unwrap(); + assert!(store.head(key).await.unwrap().is_none()); + assert!(store.list("repos/v1/").await.unwrap().is_empty()); + } + + #[tokio::test] + async fn rejects_key_traversal() { + let dir = tempfile::tempdir().unwrap(); + let store = FsBlobStore::new(dir.path()).unwrap(); + assert!(store.get("../escape").await.is_err()); + assert!(store.put("a/../../etc/passwd", Bytes::new()).await.is_err()); + // Backslashes are separators on Windows — must be rejected as keys. + assert!(store.get("a\\..\\escape").await.is_err()); + assert!(store.put("repos\\v1\\x", Bytes::new()).await.is_err()); + } + + #[tokio::test] + async fn concurrent_puts_same_key_do_not_corrupt_or_leak_temps() { + let dir = tempfile::tempdir().unwrap(); + let store = FsBlobStore::new(dir.path()).unwrap(); + let key = "repos/v1/a/x.tar.zst"; + let body = Bytes::from_static(b"the-one-true-blob"); + + // Many concurrent writers of the same key: with a fixed temp name they + // would clobber each other's temp file mid-write and corrupt the result. + let mut handles = Vec::new(); + for _ in 0..16 { + let store = store.clone(); + let body = body.clone(); + handles.push(tokio::spawn(async move { store.put(key, body).await })); + } + for h in handles { + h.await.unwrap().unwrap(); + } + + // Final content is intact... + assert_eq!(store.get(key).await.unwrap().unwrap(), body); + // ...and no unique-suffixed temp files were left behind. + let leftovers: Vec = store + .list("repos/v1/") + .await + .unwrap() + .into_iter() + .filter(|k| k.contains("tmp-put")) + .collect(); + assert!(leftovers.is_empty(), "leaked temp files: {leftovers:?}"); + } +} diff --git a/crates/gitlawb-node/src/storage/ipfs.rs b/crates/gitlawb-node/src/storage/ipfs.rs new file mode 100644 index 000000000..ed10aa646 --- /dev/null +++ b/crates/gitlawb-node/src/storage/ipfs.rs @@ -0,0 +1,173 @@ +//! IPFS (content-addressed) blob backend over a Kubo node's Mutable File System. +//! +//! Kubo's MFS (`/api/v0/files/*`) provides a path-addressed, mutable namespace +//! backed by content-addressed IPFS objects — a natural fit for a key→blob store. +//! Each object's etag is its IPFS CID (from `files/stat`), giving true +//! content-addressing for the skip-redundant-download optimization. +//! +//! Requires a reachable Kubo HTTP API (`GITLAWB_IPFS_API`, e.g. +//! `http://127.0.0.1:5001`). Objects written here are also retrievable by CID +//! from the wider IPFS network once pinned/announced by the node. +//! +//! DEPLOYMENT RESTRICTION: MFS is the mutable namespace of ONE Kubo daemon — +//! nothing here publishes a shared CID registry or IPNS mapping. Every +//! gitlawb node in a network using this backend MUST point at the same Kubo +//! instance; separate daemons produce disjoint namespaces where a push +//! accepted on one node is invisible to the others. `storage::build` logs a +//! startup warning to the same effect. + +use anyhow::{Context, Result}; +use async_trait::async_trait; +use bytes::Bytes; + +use super::{validate_key, BlobStore, ObjectMeta}; + +#[derive(Clone)] +pub struct IpfsBlobStore { + api: String, + client: reqwest::Client, +} + +impl IpfsBlobStore { + pub fn new(api: &str) -> Result { + // Bound requests so an unresponsive Kubo API can't hang push/write flows + // indefinitely. connect_timeout guards the dial; the generous total + // timeout still allows large repo-archive transfers. + let client = reqwest::Client::builder() + .connect_timeout(std::time::Duration::from_secs(5)) + .timeout(std::time::Duration::from_secs(300)) + .build() + .context("building IPFS HTTP client")?; + Ok(Self { + api: api.trim_end_matches('/').to_string(), + client, + }) + } + + /// MFS path for a key: `/gitlawb/` (namespaced to avoid clobbering + /// other MFS users on a shared node). + fn mfs_path(key: &str) -> String { + format!("/gitlawb/{key}") + } +} + +#[async_trait] +impl BlobStore for IpfsBlobStore { + fn backend_name(&self) -> &'static str { + "ipfs" + } + + async fn get(&self, key: &str) -> Result> { + validate_key(key)?; + let url = format!("{}/api/v0/files/read", self.api); + let resp = self + .client + .post(&url) + .query(&[("arg", Self::mfs_path(key).as_str())]) + .send() + .await + .context("IPFS files/read")?; + if resp.status().is_success() { + Ok(Some(resp.bytes().await.context("reading IPFS body")?)) + } else { + // Kubo returns 500 with a JSON message when the path is absent. + let body = resp.text().await.unwrap_or_default(); + if body.contains("does not exist") || body.contains("no link named") { + Ok(None) + } else { + anyhow::bail!("IPFS files/read {key}: {body}") + } + } + } + + async fn put(&self, key: &str, body: Bytes) -> Result { + validate_key(key)?; + let size = body.len() as u64; + let url = format!("{}/api/v0/files/write", self.api); + // Stream the body instead of copying it via to_vec — avoids doubling + // peak memory for large archives. Length is known, so set it explicitly. + let part = reqwest::multipart::Part::stream_with_length(reqwest::Body::from(body), size) + .file_name("blob"); + let form = reqwest::multipart::Form::new().part("data", part); + let resp = self + .client + .post(&url) + .query(&[ + ("arg", Self::mfs_path(key).as_str()), + ("create", "true"), + ("parents", "true"), + ("truncate", "true"), + ]) + .multipart(form) + .send() + .await + .context("IPFS files/write")?; + if !resp.status().is_success() { + let status = resp.status(); + let b = resp.text().await.unwrap_or_default(); + anyhow::bail!("IPFS files/write {key} returned {status}: {b}"); + } + // etag = CID from stat. The write already succeeded, so a failed stat + // must not fail the put — just return without an etag (callers treat a + // missing etag as "always re-check", never as a lost write). + let etag = match self.head(key).await { + Ok(m) => m.and_then(|m| m.etag), + Err(e) => { + tracing::warn!(key = %key, err = %e, "IPFS stat after write failed — returning no etag"); + None + } + }; + Ok(ObjectMeta { size, etag }) + } + + async fn head(&self, key: &str) -> Result> { + validate_key(key)?; + let url = format!("{}/api/v0/files/stat", self.api); + let resp = self + .client + .post(&url) + .query(&[("arg", Self::mfs_path(key).as_str())]) + .send() + .await + .context("IPFS files/stat")?; + if resp.status().is_success() { + let v: serde_json::Value = resp.json().await.context("parsing files/stat")?; + Ok(Some(ObjectMeta { + size: v.get("Size").and_then(|s| s.as_u64()).unwrap_or(0), + etag: v + .get("Hash") + .and_then(|h| h.as_str()) + .map(|s| s.to_string()), + })) + } else { + let body = resp.text().await.unwrap_or_default(); + if body.contains("does not exist") || body.contains("no link named") { + Ok(None) + } else { + anyhow::bail!("IPFS files/stat {key}: {body}") + } + } + } + + async fn delete(&self, key: &str) -> Result<()> { + validate_key(key)?; + let url = format!("{}/api/v0/files/rm", self.api); + let resp = self + .client + .post(&url) + .query(&[("arg", Self::mfs_path(key).as_str()), ("force", "true")]) + .send() + .await + .context("IPFS files/rm")?; + if resp.status().is_success() { + Ok(()) + } else { + let body = resp.text().await.unwrap_or_default(); + if body.contains("does not exist") || body.contains("no link named") { + Ok(()) + } else { + anyhow::bail!("IPFS files/rm {key}: {body}") + } + } + } +} diff --git a/crates/gitlawb-node/src/storage/mod.rs b/crates/gitlawb-node/src/storage/mod.rs new file mode 100644 index 000000000..59d89c659 --- /dev/null +++ b/crates/gitlawb-node/src/storage/mod.rs @@ -0,0 +1,164 @@ +//! Storage-agnostic blob layer. +//! +//! Repos are persisted to a pluggable object store behind the [`BlobStore`] +//! trait. Backends: +//! - [`s3::S3BlobStore`] — any S3-compatible service (Tigris, Cloudflare R2, +//! AWS S3, MinIO, Backblaze B2). Selected by default when a bucket is set. +//! - [`fs::FsBlobStore`] — a local/mounted directory; for self-hosters & tests. +//! - [`ipfs::IpfsBlobStore`] — content-addressed storage over a Kubo (IPFS) node +//! using its Mutable File System (MFS) for a key→blob namespace. +//! +//! Higher layers ([`archive::RepoArchive`]) compose a bare repo into a single +//! `repos/v1/{slug}/{repo}.tar.zst` object on top of whichever backend is active, +//! so the repo-storage semantics are identical regardless of backend. + +use std::sync::Arc; + +use anyhow::{Context, Result}; +use async_trait::async_trait; +use bytes::Bytes; +use tracing::info; + +use crate::config::Config; + +pub mod archive; +pub mod fs; +pub mod ipfs; +pub mod s3; + +/// Metadata about a stored object. `etag` is an opaque change-detection token +/// (S3 ETag, IPFS CID, or the persisted content MD5 for the filesystem +/// backend). +#[derive(Debug, Clone)] +pub struct ObjectMeta { + pub size: u64, + pub etag: Option, +} + +/// A backend-agnostic key→bytes object store. +/// +/// Keys are forward-slash-delimited paths (e.g. `repos/v1/slug/repo.tar.zst`). +/// Implementations must reject `..` traversal in keys. +#[async_trait] +pub trait BlobStore: Send + Sync { + /// Short backend name, for logs. + fn backend_name(&self) -> &'static str; + + /// Fetch an object. Returns `None` if the key does not exist. + async fn get(&self, key: &str) -> Result>; + + /// Store an object, returning its metadata (including the new etag). + async fn put(&self, key: &str, body: Bytes) -> Result; + + /// Fetch object metadata without the body. Returns `None` if absent. + async fn head(&self, key: &str) -> Result>; + + /// Delete an object. Succeeds (no-op) if the key does not exist. + async fn delete(&self, key: &str) -> Result<()>; +} + +/// Build the configured blob store. +/// +/// Returns `Ok(None)` only when no backend is configured at all (local-only +/// passthrough mode). A misconfigured backend (missing required setting, or a +/// client that fails to construct) returns `Err`: we fail closed rather than +/// silently degrading to local-only, which would accept writes without the +/// intended durable backend and risk cross-node persistence drift. +/// +/// Note: this validates configuration and client construction, not live +/// connectivity — e.g. the S3 client builds successfully against an unreachable +/// or wrong bucket, and that surfaces as an error on the first real request. +/// +/// Selection order: +/// 1. Explicit `GITLAWB_STORAGE_BACKEND` (`s3` | `fs` | `ipfs`). +/// 2. Auto: `s3` if a bucket is configured (incl. legacy `GITLAWB_TIGRIS_BUCKET`), +/// else `fs` if `GITLAWB_STORAGE_FS_DIR` is set, +/// else local-only. +/// +/// The `ipfs` backend is never auto-selected: `GITLAWB_IPFS_API` predates this +/// layer and configures the per-object encrypted pinning path, so treating its +/// presence as "store repo archives in IPFS" would silently repurpose an +/// existing pinning config on upgrade. Routing repo archives into IPFS MFS +/// requires the explicit `GITLAWB_STORAGE_BACKEND=ipfs` opt-in. +pub async fn build(config: &Config) -> Result>> { + let bucket = if !config.s3_bucket.is_empty() { + config.s3_bucket.clone() + } else { + config.tigris_bucket.clone() + }; + + let backend = if !config.storage_backend.is_empty() { + config.storage_backend.to_ascii_lowercase() + } else if !bucket.is_empty() { + "s3".to_string() + } else if !config.storage_fs_dir.is_empty() { + "fs".to_string() + } else { + info!("object storage disabled (no backend configured) — local-only mode"); + return Ok(None); + }; + + // A backend was selected (explicitly or by auto-detection); fail closed from + // here — a missing required setting or an init failure is a hard error. + match backend.as_str() { + "s3" => { + if bucket.is_empty() { + anyhow::bail!( + "storage backend=s3 but no bucket configured (set GITLAWB_S3_BUCKET)" + ); + } + let endpoint = (!config.s3_endpoint.is_empty()).then(|| config.s3_endpoint.clone()); + let s = s3::S3BlobStore::new(&bucket, endpoint, config.s3_force_path_style) + .await + .context("initializing S3 storage")?; + info!(bucket = %bucket, backend = "s3", "object storage enabled"); + Ok(Some(Arc::new(s) as Arc)) + } + "fs" => { + if config.storage_fs_dir.is_empty() { + anyhow::bail!("storage backend=fs but GITLAWB_STORAGE_FS_DIR is empty"); + } + let s = fs::FsBlobStore::new(&config.storage_fs_dir) + .context("initializing filesystem storage")?; + info!(dir = %config.storage_fs_dir, backend = "fs", "object storage enabled"); + Ok(Some(Arc::new(s) as Arc)) + } + "ipfs" => { + if config.ipfs_api.is_empty() { + anyhow::bail!("storage backend=ipfs but GITLAWB_IPFS_API is empty"); + } + let s = + ipfs::IpfsBlobStore::new(&config.ipfs_api).context("initializing IPFS storage")?; + // MFS is a namespace of ONE Kubo daemon, not a shared network + // pointer: no CID registry or IPNS mapping is published, so two + // nodes pointed at separate daemons read and write disjoint + // storage and every cross-node archive assumption breaks. + tracing::warn!(api = %config.ipfs_api, backend = "ipfs", + "IPFS backend stores archives in the Kubo daemon's LOCAL MFS namespace — \ + ALL nodes sharing this network must point at the SAME Kubo instance; \ + separate daemons produce disjoint, silently-diverging storage"); + info!(api = %config.ipfs_api, backend = "ipfs", "object storage enabled"); + Ok(Some(Arc::new(s) as Arc)) + } + other => { + anyhow::bail!("unknown GITLAWB_STORAGE_BACKEND: {other}"); + } + } +} + +/// Reject keys that could escape the namespace (`..`) or are absolute. +pub(crate) fn validate_key(key: &str) -> Result<()> { + if key.is_empty() { + anyhow::bail!("blob key is empty"); + } + if key.split('/').any(|seg| seg == ".." || seg == ".") { + anyhow::bail!("blob key contains traversal segment: {key}"); + } + // Backslash is rejected outright: keys are forward-slash-delimited, and on + // Windows `PathBuf::join` treats `\` as a separator, which would let a key + // smuggle path components past the segment checks above. + if key.starts_with('/') || key.contains('\\') || key.contains('\0') { + anyhow::bail!("blob key is absolute, contains '\\', or contains null byte: {key}"); + } + Ok(()) +} diff --git a/crates/gitlawb-node/src/storage/s3.rs b/crates/gitlawb-node/src/storage/s3.rs new file mode 100644 index 000000000..3c3339deb --- /dev/null +++ b/crates/gitlawb-node/src/storage/s3.rs @@ -0,0 +1,167 @@ +//! S3-compatible blob backend. +//! +//! Works with any S3 API implementation: Tigris, Cloudflare R2, AWS S3, MinIO, +//! Backblaze B2. Credentials and (for Tigris on Fly) the endpoint are read from +//! the standard AWS env vars (`AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, +//! `AWS_ENDPOINT_URL_S3`, `AWS_REGION`). `endpoint`/`force_path_style` override +//! those for self-hosted services like MinIO. + +use anyhow::{Context, Result}; +use async_trait::async_trait; +use aws_sdk_s3::Client as S3Client; +use bytes::Bytes; +use tracing::debug; + +use super::{validate_key, BlobStore, ObjectMeta}; + +#[derive(Clone)] +pub struct S3BlobStore { + s3: S3Client, + bucket: String, +} + +impl S3BlobStore { + /// Build a client. `endpoint` overrides `AWS_ENDPOINT_URL_S3` (for R2/MinIO); + /// `force_path_style` is required by MinIO and some S3-compatibles. + pub async fn new( + bucket: &str, + endpoint: Option, + force_path_style: bool, + ) -> Result { + let shared = aws_config::load_defaults(aws_config::BehaviorVersion::latest()).await; + let mut builder = aws_sdk_s3::config::Builder::from(&shared); + if let Some(ep) = endpoint { + builder = builder.endpoint_url(ep); + } + if force_path_style { + builder = builder.force_path_style(true); + } + // Bound requests so a hung endpoint can't block the calling task forever + // (under async_upload it would hold the advisory lock and stall every + // later push). attempt_timeout bounds a single try; operation_timeout + // bounds the whole call incl. retries — generous enough for large + // archive transfers. + let timeouts = aws_sdk_s3::config::timeout::TimeoutConfig::builder() + .operation_attempt_timeout(std::time::Duration::from_secs(60)) + .operation_timeout(std::time::Duration::from_secs(300)) + .build(); + builder = builder.timeout_config(timeouts); + let s3 = S3Client::from_conf(builder.build()); + Ok(Self { + s3, + bucket: bucket.to_string(), + }) + } + + /// Test-only constructor with an explicit S3 endpoint and static + /// credentials — no env-var reads, so parallel tests cannot race each + /// other's `AWS_*` environment the way the env-based `new` would. Lets a + /// test point the client at a non-routable endpoint to exercise + /// acquire-stall paths; deliberately no operation timeouts, so the stall + /// is bounded only by the caller's own deadline. + #[cfg(test)] + pub(crate) async fn for_testing_with_endpoint(bucket: &str, endpoint_url: &str) -> Self { + let creds = aws_sdk_s3::config::Credentials::new("test", "test", None, None, "test"); + let config = aws_config::defaults(aws_config::BehaviorVersion::latest()) + .endpoint_url(endpoint_url) + .region(aws_config::Region::new("auto")) + .credentials_provider(creds) + .load() + .await; + Self { + s3: S3Client::new(&config), + bucket: bucket.to_string(), + } + } +} + +#[async_trait] +impl BlobStore for S3BlobStore { + fn backend_name(&self) -> &'static str { + "s3" + } + + async fn get(&self, key: &str) -> Result> { + validate_key(key)?; + match self + .s3 + .get_object() + .bucket(&self.bucket) + .key(key) + .send() + .await + { + Ok(resp) => { + let data = resp + .body + .collect() + .await + .context("reading S3 response body")? + .into_bytes(); + Ok(Some(data)) + } + Err(e) => { + if e.as_service_error().is_some_and(|e| e.is_no_such_key()) { + Ok(None) + } else { + Err(anyhow::anyhow!("S3 GET {key}: {e}")) + } + } + } + } + + async fn put(&self, key: &str, body: Bytes) -> Result { + validate_key(key)?; + let size = body.len() as u64; + let resp = self + .s3 + .put_object() + .bucket(&self.bucket) + .key(key) + .body(aws_sdk_s3::primitives::ByteStream::from(body)) + .send() + .await + .context(format!("S3 PUT {key}"))?; + debug!(key = %key, size, "s3 put"); + Ok(ObjectMeta { + size, + etag: resp.e_tag().map(|s| s.to_string()), + }) + } + + async fn head(&self, key: &str) -> Result> { + validate_key(key)?; + match self + .s3 + .head_object() + .bucket(&self.bucket) + .key(key) + .send() + .await + { + Ok(resp) => Ok(Some(ObjectMeta { + size: resp.content_length().unwrap_or(0).max(0) as u64, + etag: resp.e_tag().map(|s| s.to_string()), + })), + Err(e) => { + if e.as_service_error().is_some_and(|e| e.is_not_found()) { + Ok(None) + } else { + Err(anyhow::anyhow!("S3 HEAD {key}: {e}")) + } + } + } + } + + async fn delete(&self, key: &str) -> Result<()> { + validate_key(key)?; + self.s3 + .delete_object() + .bucket(&self.bucket) + .key(key) + .send() + .await + .context(format!("S3 DELETE {key}"))?; + Ok(()) + } +} diff --git a/crates/gitlawb-node/src/test_support.rs b/crates/gitlawb-node/src/test_support.rs index 430c06002..bcab1172e 100644 --- a/crates/gitlawb-node/src/test_support.rs +++ b/crates/gitlawb-node/src/test_support.rs @@ -10687,6 +10687,9 @@ mod tests { "TigrisClient", ".download(", "tigris", + "RepoArchive", + "BlobStore", + "fetch_raw", ] { assert!( !code.contains(shape), diff --git a/crates/gitlawb-node/tests/inv22_gates.rs b/crates/gitlawb-node/tests/inv22_gates.rs index 48381e3dc..c619a6c5d 100644 --- a/crates/gitlawb-node/tests/inv22_gates.rs +++ b/crates/gitlawb-node/tests/inv22_gates.rs @@ -210,14 +210,21 @@ fn inv22_concurrency_gates_present_and_not_bypassed() { } } -/// F4 (repo_store advisory-unlock cancellation safety): `RepoWriteGuard::release` -/// must await `pg_advisory_unlock` while `self` still owns the pooled connection, -/// and must not mark itself `released` until that await resolves. Either shape, +/// F4 (repo_store advisory-unlock cancellation safety): the advisory unlock must +/// be awaited while its owner still holds the pooled connection, and the owner +/// must not mark itself `released` until that await resolves. Either shape, /// reintroduced, re-opens the mid-unlock cancellation leak: taking the connection /// early leaves `Drop` with `conn == None`, and setting `released = true` early /// leaves the `Drop` backstop inert — both strand the session lock on cancellation. /// -/// Scoped to the `release` fn body: the `Drop` impl legitimately takes the +/// The guarded code MOVED with the storage-abstraction layer: the unlock used to +/// live in `RepoWriteGuard::release`, and now lives in `LockedConn::unlock`, the +/// type that owns a lock's whole pinned-connection lifetime (the guard delegates +/// to it, and the write-back path drives it from a detached task, which is the +/// cancellation shape this gate exists for). The invariant is unchanged, so this +/// gate follows the code rather than being deleted with it. +/// +/// Scoped to the `unlock` fn body: the `Drop` impl legitimately takes the /// connection and unlocks, so a whole-file scan would match it and read as a false /// pass. Reverting either ordering turns this red (proven load-bearing). #[test] @@ -225,12 +232,12 @@ fn f4_release_keeps_conn_owned_until_unlock_resolves() { let repo_store = src("git/repo_store.rs"); let rel_start = repo_store - .find("pub async fn release(mut self") - .expect("F4 gate: repo_store.rs no longer defines RepoWriteGuard::release"); + .find("async fn unlock(mut self") + .expect("F4 gate: repo_store.rs no longer defines LockedConn::unlock"); let rel_end = repo_store[rel_start..] - .find("impl Drop for RepoWriteGuard") + .find("impl Drop for LockedConn") .map(|off| rel_start + off) - .expect("F4 gate: release fn / Drop impl markers moved — update this guard"); + .expect("F4 gate: unlock fn / Drop impl markers moved — update this guard"); let release_body = &repo_store[rel_start..rel_end]; let unlock = release_body @@ -241,7 +248,7 @@ fn f4_release_keeps_conn_owned_until_unlock_resolves() { // (a) the connection must still be owned by `self` at the unlock await. assert!( !before_unlock.contains("self.conn.take()"), - "F4 regression: RepoWriteGuard::release takes self.conn BEFORE awaiting \ + "F4 regression: LockedConn::unlock takes self.conn BEFORE awaiting \ pg_advisory_unlock. A cancellation during the unlock await then strands the \ session advisory lock (Drop sees conn == None and skips its backstop). \ Unlock through the still-owned connection instead." @@ -250,7 +257,7 @@ fn f4_release_keeps_conn_owned_until_unlock_resolves() { // reintroduction shape a single-reorder check on (a) alone is blind to. assert!( !before_unlock.contains("released = true"), - "F4 regression: RepoWriteGuard::release sets `released = true` BEFORE awaiting \ + "F4 regression: LockedConn::unlock sets `released = true` BEFORE awaiting \ pg_advisory_unlock. A cancellation during the await then leaves the Drop \ backstop inert (it early-returns on released). Set released only AFTER the \ unlock await resolves."