Repository navigation
The lazy-migration background upload tars a live repo while a push writes it, holding no lease or lock #343
Description
Activity
- addedsev:mediumDegraded but workaround existsDegraded but workaround existscrate:nodegitlawb-node — the serving node and REST APIgitlawb-node — the serving node and REST APIkind:bugDefect fix — wrong or unsafe behaviorDefect fix — wrong or unsafe behaviorsubsystem:storageBlob/object store, Arweave, IPFS, archivesBlob/object store, Arweave, IPFS, archives
on Aug 15, 2026 sync.rsis a third unexcluded writer ofrepos_dir, filed separately as #351. Cross-referencing because the survey here is worth widening: the paths named in this issue are the upload and extraction sides, and the sync worker is the write side.grep -c "repo_write_leases\|acquire_write\|publish_lock" crates/gitlawb-node/src/sync.rsreturns0atorigin/main50d3cbb. It deriveslocal_pathatsync.rs:243-245and then runsgit fetch --prune origin(orgit clone --mirror) straight into the live bare repo at:355-359, no temp dir and no rename, while the push path takes a lease and then the advisory lock.The observable consequence is sharper than the upload race, because a mirror's refspec is
+refs/*:refs/*:refs after push to mirror: refs/heads/feature, refs/heads/main refs after the worker's 'git fetch --prune': refs/heads/mainAn acked push is silently discarded, surviving only as a dangling object until gc.
On reachability, being precise about the two halves since they differ:
/api/v1/sync/notifyis inpeer_write_routes, which is wrapped byadd_auth_layersonly underif state.config.require_signed_peer_writes(server.rs:334), and that flag isdefault_value_t = false(config.rs:74-77). So by default thenode_didis self-asserted. It is not fully open though:notify_syncrejects a DID absent from the peers table (peers.rs:429-432), so a caller has to appear as a peer row first, which/api/v1/peers/announcepermits unsigned under that same conditional. Worth contrasting with/api/v1/sync/trigger, which is unconditionally signed (server.rs:311).Different from this issue in actor, direction, and fix: that is a detached upload snapshotting a repo under a push, triggered by an anonymous
GET /info/refs; this is a fetch writing into it, triggered by a notify. Same missing-exclusion class though, so if either gets fixed by adopting the push path's lease rather than a local temp-dir swap, the other should ride along on the same mechanism.
Two paths touch the same repo directory and neither excludes the other.
Writer,
api/repos.rs:1881-1946:POST .../git-receive-packtakes the per-repo lease(
state.repo_write_leases.acquire), thenrepo_store.acquire_write, which takes the advisory lock onits own pinned connection, runs
git receive-pack, thenguard.release(true)uploads.Background upload,
git/repo_store.rs:85-115: any read handler callingacquire()spawns a detachedtask that tars the live directory and uploads it. It takes no lease, no advisory lock, and no
snapshot.
repo_store.rsnever referencesrepo_write_leasesat all, andacquire()is the onlyfunction in the file that spawns an upload without going through a guard.
compress_repo(git/tigris.rs:165-176) walks the live bare repo withtar.append_dir_all(".", repo_path).No snapshot, no clone, no barrier. And
put_object(tigris.rs:96-104) carries noif_matchand nogeneration, so the last PUT to land wins regardless of which snapshot is older.
Two bad outcomes, both persisted
PUT, and the bucket ends up holding the pre-push repo. Internally consistent, but it is the
canonical copy, and the next download destroys the good local tree too, because
tigris.rs:239-242doesremove_dir_all(local_path)thenrename. That is silent loss of anacknowledged push.
not contain. Also persisted, also later served.
init()(repo_store.rs:289-299) has the same shape: a detached upload of the just-created repo withno exclusion against a push arriving before the PUT completes.
Reachability
Requires Tigris enabled and the repo not yet in the bucket, since
exists()returningOk(true)skips the upload. That state is exactly what this code is for (the doc comment at
repo_store.rs:75-76),and also arises when an
init()upload failed, which only warns (:296), or after a bucket-sidedelete.
Given that, the trigger is an anonymous permissionless
GET /:owner/:repo/info/refs?service=git-upload-pack,an ordinary clone, landing while an authenticated push is in flight. The window is the whole
compress plus PUT, which for a large repo is seconds to minutes of zstd over the full tree.
Not verified by execution: driving it needs a stallable S3 endpoint plus a compress-timing seam.
TigrisClient::for_testing_with_endpointexists but there is no upload hook. Established by reading,the same standing accepted for #283.
Not a duplicate
#283 is the mirror image: extraction clobbering a repo under a reader or writer. This is upload
capturing a repo under a writer. Opposite direction, different fix. #279 and PR #285 are
writer-versus-writer advisory-lock correctness and do not reach this, because the background upload
never asks for the lock, so even a perfectly session-affine lock excludes nothing here. #300 shares the
exists()call but is a read-rendering bug.Fix direction
Put the migration and
inituploads behind the same write exclusionrelease_after_writealreadyuses, or make the PUT conditional.
One correction: the obvious-looking fix of not inserting into
migrateduntil the upload completes isalready the behavior.
repo_store.rs:102-113returns early on upload error, so the insert is onlyreached after a successful PUT. The exclusion is the load-bearing part.