fix(lfs): stop endpoint and path quirks from failing repository backups by neurekadev · Pull Request #29 · neurekadev/git-backup

neurekadev · 2026-09-10T14:30:52Z

Description

Fixes a set of Git LFS backup failures caused by how forges and hosts serve their LFS endpoints, so repositories are no longer reported as LFS-free (or fail outright) when their content is actually reachable.

Endpoint discovery and selection

Adds a new internal/lfs/endpoint.go that derives the API roots a repository's LFS objects may live under: a safe lfs.url override from .lfsconfig, otherwise the remote's standard /info/lfs path with and without the repository's .git suffix.
Before fetching, the client now probes each candidate once for a single object to determine which endpoint actually serves the LFS API. This separates "the API answered," "LFS is switched off," "nothing is mounted at this path," and endpoint failure.
The path the remote itself configures takes precedence over the derived .git guess, so a guess reporting "LFS off" or failing never masks the configured path's real answer. When no candidate serves the API, the result is reported as the existing expected-skip verdict (ErrDisabled).

Credentials

Credentials are now offered on the first request rather than waiting for a 401. This prevents forges that answer 403 to anonymous requests from being misread as repositories with LFS disabled, which would silently back up the repo without its LFS content.

Batch and object handling

Introduced error sentinels (errBatchRejected, errObjectUnavailable, errNoEndpoint, etc.) to distinguish endpoint failures from object-level unavailability.
A batch request rejected with a shape-related status (e.g. object-limit, unprocessable entity) is now narrowed by halving down to the offending object, so one stale/oversized pointer no longer costs the whole chunk's LFS content.
Objects whose pre-signed download URL has already expired are re-requested for a fresh URL instead of failing the transfer.
Cached objects are skipped based on the pointer's authoritative size, and objects already in the store are settled before expiry handling.
Response OIDs are validated and folded to lowercase; malformed, uppercase, or unrequested OIDs can no longer panic, escape the store path, or cause duplicate writes.

Repository scan

collectPointers now returns a scanResult including subtrees it could not read (skippedSubtree), and FetchAll reports failures when part of the repository could not be scanned rather than reporting a silently incomplete mirror.
Tree walking was rewritten to enumerate tree objects directly instead of using the working-tree walker, so repositories containing paths illegal on the host (e.g. backslashes on Windows) no longer abort the whole backup.

Tests

Extensive test coverage was added for endpoint fallback, disabled/forbidden/unmounted paths, credential handling, expired URLs, batch rejection narrowing, malformed/unrequested OIDs, chunking of large pointer sets, and scan failures.

Overall, the changes make LFS backups resilient to common endpoint and path quirks while reporting genuinely missing content instead of either failing the repository or recording it as complete when it isn't.

Three LFS conditions aborted a whole repository snapshot even though the git mirror itself cloned and uploaded fine: - Forges route only the suffixed repository path to their LFS service, so requesting /info/lfs without .git made GitHub and GitLab answer with an HTML 422 page ("batch request failed with status 422"). The endpoint now carries the repository's .git suffix whenever the configured remote omits it. - The pointer scan used object.TreeWalker, which rejects any tree entry whose name is unsuitable for materialising a working tree on the host, such as a path containing a backslash on Windows. Trees are now enumerated as raw tree objects, so an unusual file name can no longer abort the scan. - A batch the endpoint rejects outright failed the entire LFS mirror. Pointers are now submitted in batches of 100 and a rejected batch is retried object by object: one object the server refuses, or a request above its object limit, costs that object alone, while objects the endpoint will not serve are reported as skipped so the rest of the mirror still reaches storage.

kody-ai

Found critical issues please review the requested changes

defaultEndpoint reconstructs the URL by string concatenation, dropping userinfo and mis-glueing an empty path

kody-ai

Found critical issues please review the requested changes

The endpoint now keeps embedded URL credentials and newly added log lines print them verbatim.

kody-ai

Found critical issues please review the requested changes

probe's signature changed but its caller in endpoint.go still passes the old username/password args, breaking the build.
Changed selectEndpoint call passes creds but endpoint.go still takes username/password and calls probe with the old signature.

…jects named Review of the credential work found three defects, all in the new code: - Deferring the credentials to a 401 retry regressed authenticated forges. The probe and the batch both sent basic auth up front before, and a forge that answers 403 to an anonymous request — which is also how a repository with LFS disabled answers — was read as LFS-free, so the repository was recorded as having no LFS content instead of failing. Both requests now offer the credentials when there are any, and a 401 is reported as the refusal it is rather than retried with the same credentials, which is what the retry did. - An object already in the store was checked for expiry before it was checked for being cached, so a second run over mirrored content sent it back for rescheduling and could fail over a fresh URL the content never needed. The cache is now consulted first. - Objects whose refresh failed were counted with a bare, unnamed error, so a caller learned how many objects were missing but not which. Both refresh branches now name the object, and the endpoint's refusal is carried as that record's reason instead of being appended as a second, chunk-level failure over objects that had already been accounted for. Coverage: a forge that refuses anonymous requests with 403, cached content whose scheduled URL has lapsed, and both refresh-failure branches.

kody-ai

Found critical issues please review the requested changes

The new cache check slices an unvalidated endpoint-supplied OID, panicking on an OID shorter than four characters.

A batch response's OIDs are used as path components and sliced for the shard directories, but only the response's object count was checked against the pointers sent. An endpoint returning a short OID panicked the slice and took the whole backup with it, and one returning a crafted OID named a path outside the store. Neither is the client's data to trust: every response object is now checked to be a sha256 digest that was actually requested, and anything else is reported as an unserved object. The cached-object check also accepted any file at the object's path as proof the content was mirrored, which skipped the digest verification a download performs. A cached entry now has to be a regular file of the length the endpoint reported, so a partial or unrelated file falls through to a download that verifies it. Coverage: malformed and unrequested OIDs in a batch response, including the short-OID case that used to panic.

…tated size Two ways the new batch-response checks could fail content that is already mirrored: - The requested-object check compared the response OID with the pointer's OID byte for byte, while the digest check beside it accepts upper case. The OID is a path component and the store is keyed by it, so an endpoint echoing a valid digest in upper case would be written to a second directory, miss the cache it should hit, and be reported as an object that was never requested. Response OIDs are now folded to lower case, which is the spelling the pointers and the store already use. - The cached-object check treated the endpoint's size as authoritative, so an endpoint that omits it — reporting zero — made content the store already holds look stale. Re-downloading it is wasteful at best and, for an object whose scheduled URL has lapsed, escalates into failing a backup whose LFS content is complete. A size of zero now leaves the length unstated rather than contradicted, so the cache is trusted; a size the endpoint does state is still enforced, and a file of the wrong length falls through to a verified download. Coverage: an upper-case digested response matching cached content, and an endpoint that omits the size, including that a stated size is still enforced.

Treating a size of zero as "unstated" left the length unchecked whenever the endpoint omitted it, so any regular file at the object's path counted as mirrored. A truncated leftover or an interrupted copy would then be reported as a complete backup while the real content was missing — the failure this whole package exists to prevent, reached by trusting the wrong field. The pointer file is what the repository records about the object, and it is already in hand as the pointer being fetched, so its size is now authoritative; the endpoint's stands in only when the pointer states none. A file whose length contradicts the recorded size falls through to a download that verifies the digest, and only a length that is unknown on both sides is left unchecked. Coverage: a truncated cached file behind an endpoint that reports no size, which the previous check accepted.

An object whose scheduled URL had lapsed was rescheduled with the endpoint's size rather than the resolved one, so the pointer's authoritative length was dropped at exactly the point it was needed. For an endpoint that states no size, the refresh request asked about a zero-length object and the pass over its answer saw a zero size, which the cache predicate reads as unstated — accepting any cached file, truncation included, and reporting an incomplete mirror as complete. The resolved size now travels with the rescheduled pointer, so both the refresh request and the pass over its response keep the length the pointer records. Coverage: a lapsed object on an endpoint that states no size, asserting the rescheduled pointer carries the pointer file's size rather than zero.

kody-ai · 2026-09-10T23:39:28Z

Kody Review Complete

Great news! 🎉
No issues were found that match your current review configurations.

Keep up the excellent work! 🚀

Kody Guide: Usage and Configuration

Interacting with Kody

Request a Review: Ask Kody to review your PR manually by adding a comment with the @kody start-review command at the root of your PR.
Validate Business Logic: Ask Kody to validate your code against business rules by adding a comment with the @kody -v business-logic command.
Provide Feedback: Help Kody learn and improve by reacting to its comments with a 👍 for helpful suggestions or a 👎 if improvements are needed.

Current Kody Configuration

Review Options

The following review options are enabled or disabled:

Options	Enabled
Bug	✅
Performance	✅
Security	✅
Business Logic	✅

Access your configuration settings here.

This comment has been minimized.

Sign in to view