Repository navigation
bugfix: Keep the user of ssh URLs and restore it on mirrors stripped by v1.0.31 to v1.0.34 - #163
Merged
Conversation
…pping credentials. The username of an SSH URL is part of the connection specification and required for authentication, not a secret. Previously normalize_url() dropped the 'user@' part before storing the upstream URL in the database, so mirrors created from SSH URLs failed to authenticate on later updates (e.g. 'gitcache -u') once the URL was read back from the database and strip_credentials() found nothing to restore. Now normalize_url() and strip_credentials() keep the username for ssh:// and scp-style URLs and only strip (or mask) a password for ssh:// URLs. SCP-style URLs (user@host:path) have no syntax for a password, so their username is kept as-is without any stripping logic. Behaviour for http(s)/ftp(s)/git URLs is unchanged, and the mirror path still ignores the username so cache sharing is unaffected. Fixes seeraven#159 (cherry picked from commit 50b2aac)
The functional tests derived the database URL by removing `git@` from the remote URL. With the user kept, the database URL is the remote URL without the `.git` suffix. Also cover strip_credentials() with unit tests and add the changelog entry for seeraven#159.
…0.34 Those versions removed the user of ssh URLs from the mirror remote and restored it from the URL of the current command before each update. Keeping the user means the remote is no longer rewritten, so a mirror created by those versions would stay without user and fail to authenticate for every command, including the ssh commands that worked before. Before an update, compare the mirror remote with the URL of the command: if the remote is an ssh URL of the same repository without a user while the command URL has one, set the remote to the command URL. Mirrors only ever updated through `gitcache -u` keep the user-less URL stored in the database and still need the remote fixed by hand or a fresh mirror.
seeraven
approved these changes
Sep 25, 2026
seeraven
left a comment
seeraven
left a comment
Owner
There was a problem hiding this comment.
Thanks for the PR! I've noticed that you haven't added yourself as an author in the CHANGELOG.md nor in the newly added files. Otherwise it's good to merge.
The repair of mirrors stripped by v1.0.31 to v1.0.34 only acted on a remote without a user. A remote with a different ssh user was left alone, so on a cache shared between ssh accounts every update ran as whoever created the mirror. Compare the users instead and set the remote to the command URL whenever they differ. Refresh the database URL at the same time, so that `gitcache -s` shows the repaired URL and `gitcache -d` accepts it. Also add myself as author of the new test files and the changelog entry, as requested in review.
_remote_user_differs() relied on _add_credentials_to_remote() returning early for every non-ssh URL with a user. Check the mirror URL itself so the function holds on its own. Also add unit tests for normalize_url(): the ssh functional tests are gated on specific hosts, so CI never exercised the ssh user being kept.
Contributor
Author
|
Hey @seeraven I have updated the PR with the credit and pushed some more tests. Let me know what you think |
Owner
|
Hi @Bilb , looks good to me! If it's ok with you I'll merge it. |
Contributor
Author
|
@seeraven yeah this is ready to be merged! |
Contributor
|
@Bilb thanks for working on this. Sorry was away last week. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #159.
Problem
strip_credentials()andnormalize_url()treat the user ofssh://and scp-style URLs as a credential and drop it. The mirror remote is stored asgithub.com:org/repoand only works while the user is restored from the URL of the current command. Two situations have nothing to restore it from:gitcache -u, which reads the URL back from the database. This is the case reported in SSH mirrors fail ongitcache -ubecause URL normalization drops the username #159.<local user>@github.comand is refused.Both stay invisible on machines where
~/.ssh/configsetsUser gitfor the host.Changes
gitcache -ubecause URL normalization drops the username #159: keep the user ofssh://and scp-style URLs when normalizing URLs and stripping credentials. The commit is kept with its original author. Tests and the changelog entry are added here.gitcache -ukeep the user-less database URL and still need a fresh mirror or a manualgit remote set-url.git@. They now use the remote URL without the.gitsuffix.Verification
strip_credentials(),get_user(),is_ssh_url(),Database.set_url()and the remote repair, including a remote with a different user, a remote of another repository and a failinggit remote set-url.Workhorse/hermes, run locally against fa7b35e withplatform.node()patched: 53 passed. The variants without a user (github.com:...,ssh://github.com/...) needUser gitin the ssh configuration, emulated withGIT_SSH_COMMAND="ssh -o User=git". The follow-up commit only changes the repair condition, the database refresh and the credits, and is covered by the unit tests.GITCACHE_DIR, comparing v1.0.34, v1.0.34 plus change 1 only, and fa7b35e:gitcache -u-u