The lazy-migration background upload tars a live repo while a push writes it, holding no lease or lock

[beardthelion]

Two paths touch the same repo directory and neither excludes the other.

Writer, api/repos.rs:1881-1946: POST .../git-receive-pack takes the per-repo lease
(state.repo_write_leases.acquire), then repo_store.acquire_write, which takes the advisory lock on
its own pinned connection, runs git receive-pack, then guard.release(true) uploads.

Background upload, git/repo_store.rs:85-115: any read handler calling acquire() spawns a detached
task that tars the live directory and uploads it. It takes no lease, no advisory lock, and no
snapshot. repo_store.rs never references repo_write_leases at all, and acquire() is the only
function in the file that spawns an upload without going through a guard.

tokio::spawn(async move {
    match tigris.exists(&slug, &name).await {
        ... Ok(false) => { ... tigris.upload(&slug, &name, &path).await ... }

compress_repo (git/tigris.rs:165-176) walks the live bare repo with tar.append_dir_all(".", repo_path).
No snapshot, no clone, no barrier. And put_object (tigris.rs:96-104) carries no if_match and no
generation, so the last PUT to land wins regardless of which snapshot is older.

Two bad outcomes, both persisted

1. Stale overwrites newer: the migration snapshot predates the push, its PUT lands after the guard's
   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-242 does remove_dir_all(local_path) then rename. That is silent loss of an
   acknowledged push.
2. Torn: the tar walk straddles the ref update, so the archive's refs point at objects the pack does
   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 with
no exclusion against a push arriving before the PUT completes.

Reachability

Requires Tigris enabled and the repo not yet in the bucket, since exists() returning Ok(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-side
delete.

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_endpoint exists 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 init uploads behind the same write exclusion release_after_write already
uses, or make the PUT conditional.

One correction: the obvious-looking fix of not inserting into migrated until the upload completes is
already the behavior. repo_store.rs:102-113 returns early on upload error, so the insert is only
reached after a successful PUT. The exclusion is the load-bearing part.

[beardthelion]

sync.rs is a third unexcluded writer of repos_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.rs returns 0 at origin/main 50d3cbb. It derives local_path at sync.rs:243-245 and then runs git fetch --prune origin (or git 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/main

An 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/notify is in peer_write_routes, which is wrapped by add_auth_layers only under if state.config.require_signed_peer_writes (server.rs:334), and that flag is default_value_t = false (config.rs:74-77). So by default the node_did is self-asserted. It is not fully open though: notify_sync rejects 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/announce permits 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.