fix(git): retry the other URL form when a host answers with protocol v2 by neurekadev · Pull Request #31 · neurekadev/git-backup

neurekadev · 2026-09-11T00:38:27Z

Closes #30.

Summary

Fixes syncs that fail when a Git host serves Git protocol v2 at one form of a repository URL (with or without the .git suffix) while the same repository remains reachable at the other form, which this client can read. Also adds credential redaction for URLs written to logs.

Changes

Git sync: retry the alternate URL form on protocol v2

SyncBareRepository now runs the clone/fetch through a new mirrorRepository helper, so the mirror logic can be invoked more than once.
When the sync fails because the remote answered with a protocol v2 advertisement (which the go-git v5 client cannot decode), the sync is retried against the alternate spelling of the same URL — the path with its .git suffix flipped, case-insensitively.
On a successful retry, the alternate URL becomes the URL the rest of the sync uses, including the subsequent Git LFS fetch, so a host that serves LFS only on the answered form is not failed after the mirror succeeded.
The protocol v2 condition is detected by matching the two fragments go-git's v0/v1 decoder emits (pkt-line too short and cannot read hash); unrelated failures (e.g. connection or authentication errors) and URLs with no path to alternate are reported as before rather than retried.
freshClone/fetchMirror self-heal behavior was adjusted: a protocol v2 failure during an incremental fetch no longer triggers a destructive re-clone. Re-cloning would delete an intact mirror and then fail on the same form, turning a retry that could have fetched incrementally into a full clone every run.
URL alternation normalizes a trailing slash, drops the fragment (a client-side marker), preserves query, port, userinfo, nested paths, and escaping behavior.

URL redaction for logs

New paths.RedactURL masks any embedded password in a clone URL before it is logged, keeping the scheme, username, and host visible.
Values the URL parser rejects (e.g. hosts with spaces or malformed ports) are masked textually by locating the userinfo section instead of returning the raw value, so a credential cannot leak through an unparseable URL.
The LFS "disabled on remote" log line now uses RedactURL instead of logging the raw URL, and the new protocol v2 retry log line redacts both the original and the retried URL.

Tests

Added coverage for retrying the alternate URL form, reporting protocol v2 when there is no alternate, and retrying into an existing cached mirror without destroying it (verified via a marker file surviving the sync).
Added table tests for alternateURLForm and versionTwoAlternate, and for RedactURL across password-masking and unparseable-URL cases.

A host may serve a repository at its bare path and at the path ending in ".git", and some answer only one of the two with the Git wire protocol v2 advertisement. go-git v5 speaks v0/v1 only, so that answer is a hard failure: pkt-line 3: cannot read hash, pkt-line too short (version 2) The same repository is reachable at the other form, so a sync that fails for that reason is retried there instead of failing the repository outright. A URL with no path to alternate is reported as before, and no other failure is retried. The failure is recognised by its text because go-git raises it from inside the decoder with no error to match on. Both fragments of the message are required so an unrelated decode failure is not mistaken for it. The retry only costs anything on a failure this client cannot otherwise recover from, and the mirror is removed and re-cloned on that path, so nothing partial survives the attempt. Closes #30.

kody-ai

Found critical issues please review the requested changes

Redact credentials from the logged repository URLs before writing them to the log.

…depends on Review of the protocol v2 retry found ways it could go wrong. The v2 log line wrote the clone URL verbatim, and a clone URL may embed a password (`https://user:token@host/owner/repo`), which put a live credential in the logs. Both URLs now go through paths.RedactURL, which is the shared helper the URL parsing this package already uses lives beside; the LFS skip line had the same leak and is fixed with it. A cached mirror was deleted before the retry could use it. The self-heal treats every incremental fetch failure as a corrupt mirror and re-clones from scratch, so a host answering v2 destroyed an intact mirror and then failed on the same form — every scheduled run paid for a full clone. A protocol advertisement is not corruption, so it is no longer self-healed, and the retry now fetches incrementally into the mirror that is already there. The retry's URL was not carried to the LFS step, which still derived its batch endpoint from the configured form. On a host that serves LFS only on the form that answered, the mirror succeeded and the sync failed immediately after, so the recovery was half-applied. The URL that actually cloned is now the one LFS uses. The suffix was matched case-sensitively and only the exact path "/" was special, so `repo.GIT` gained a second suffix and `repo/` produced `repo/.git` — URLs that cannot exist, on which the retry silently did nothing. The suffix is now matched case-insensitively through paths.TrimGitSuffix, as the rest of the codebase does, and a trailing slash is normalised away first. A fragment is also dropped: it is a client-side marker that never forms part of what is fetched, and carrying it onto the other form appends it to a request the server never received one for. A query is deliberately kept, since it can be part of what the remote is asked for. Coverage: redaction including the unparseable case, an upper-case suffix, trailing slashes, fragments, escaped paths, and a cached mirror surviving a protocol v2 retry. The mirror test fails against the previous self-heal, and the suffix test fails against the previous case-sensitive match.

neurekadev · 2026-09-11T01:23:21Z

@kody — all six findings from your review are addressed in 7019a8e, and CI (Lint, Unit Tests, Build) is green on that head. Could you re-review?

Summary of what changed, including one finding I only partly applied:

Redaction: both URLs in the v2 log line now go through a new paths.RedactURL; the LFS skip line had the same leak and is fixed with it.
Self-heal: a protocol v2 advertisement no longer triggers the destructive re-clone, so the retry fetches incrementally into the cached mirror instead of deleting it first.
LFS URL: the URL that actually cloned is the one passed to fetchLFS.
Suffix handling: case-insensitive via the existing paths.TrimGitSuffix, with a trailing slash normalised first.
RawPath / query / fragment: I dropped the fragment, kept the query, and did not clear RawPath — I measured it, and url.URL.String() recomputes from Path when the two disagree, so clearing it changed nothing in any case. Details are in the thread reply.

Each reply in the resolved threads has the verification behind it, including the two behaviours confirmed by mutation testing.

kody-ai

Found critical issues please review the requested changes

RedactURL returns the unredacted input when url.Parse fails, so a malformed URL carrying userinfo can still leak its credential to logs.

RedactURL returned a value the parser rejected unchanged, so a credential in a host the parser refuses — a space in the hostname, a malformed port — was handed back verbatim to be written to a log. That is the one case the function exists for, so it failed open exactly where it mattered. Unparseable values are now masked by locating the userinfo section directly: everything between the scheme and the first separator is the authority, and the last "@" within it splits userinfo from host, since a password may itself contain one. The scheme, username and host are kept so a log line still says what was fetched and which credential was used; the path is dropped, because once the value has failed to parse the separator that would end the authority cannot be told apart from part of it. Coverage: a password in a rejected host, before a malformed port, containing a space, under a non-http scheme, and an unparseable value with no userinfo left alone. A regexp was tried first and abandoned — Go's regexp has no lookahead, so the last-"@" rule cannot be expressed with one.

maskUserinfo cut the authority at the first "/", "?" or "#" before looking for the "@" that ends the userinfo section. A password containing one of those hid that "@" from the search, and the unparseable value was returned with the credential intact: RedactURL("https://user:tok/en@host/repo") == "https://user:tok/en@host/repo" The "@" is now located in the whole remainder after the scheme, before anything is cut, so the separator only bounds the userinfo rather than hiding where it ends. Because nothing is discarded, the path, port and fragment of the rejected value survive in the masked form, which is more useful in a log line than the authority-only rendering this replaces. Coverage: passwords containing "/", "?" and "#", plus a host and port with no userinfo to ensure a ":" alone is not mistaken for one.

kody-ai · 2026-09-11T01:46:21Z

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.

neurekadev added Priority/Medium Type/Bug labels Sep 11, 2026

This comment has been minimized.

Sign in to view

neurekadev mentioned this pull request Sep 11, 2026

Sync fails when a host serves Git protocol v2 on the requested URL form (go-git v5 cannot read v2) #30

Closed

2 tasks