fix(sync): refuse URL rewrites from the folder, keep the user's remotes safe

Follow-up to the independent review of this branch.

- A url.<base>.insteadOf entry in the folder's own configuration redirected
  the fetch of the listed URL to another repository. It is now refused
  before anything is fetched, and so is the folder's core.sshCommand when
  the fetch is not HTTPS. The user's global configuration still applies.
- The temporary remote is marked as gclo's; a remote of that name the user
  created is never removed, and cleanup can no longer mask the fetch's own
  error. A marked leftover is removed on any later pull. The temporary
  fetch follows origin's tag option.
- The mismatch message gives the git remote set-url command for a
  repository that was renamed or moved.
- RemoteUrl: redaction rebuilds the URL from scheme, host, port and path,
  so no shape leaves user information, query or fragment behind; a
  non-default web port distinguishes two servers; an unreadable host is
  nobody's instead of an exception; IPv6 hosts keep their brackets.
- The WSL script compares the stored origin URL, not the rewritten one.
- The credential callback is tested directly. The threat model and README
  no longer claim more than the code does about redirects.

Refs #139

Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
Signed-off-by: James Maes <james@kof22.com>
This commit is contained in:
james.maesandClaude Fable 5.2 committed 2026-11-10 12:54:51 -06:00
1 parent d5da49b2a2
commit 510d3e0561
9 files changed
+522 -85

No files matched your search

+1 -1
View File
@@ -186,7 +186,7 @@ Full reference — all options, output streams, Ctrl+C behavior, and more exampl
## How it works
The engine lists every repository through the GitHub REST API (Octokit, paginated 100 per page), then runs the set through `Parallel.ForEachAsync` bounded by the parallelism setting. Git operations use LibGit2Sharp: missing repos are cloned with object-transfer progress; existing repos get a fetch from `origin` followed by a **fast-forward-only** merge — if a local branch has diverged from its upstream, the repo is reported Failed rather than auto-merged, so gclo never manufactures merge commits. An existing folder is only updated when its `origin` is the repository gclo listed: a same-named folder that is a clone of something else (a fork, another owner's repository, another host) is reported Failed and left untouched. Archived repositories are included and sync normally. Every repo ends in exactly one terminal state — Done, Failed (with the error), or Canceled — and per-repo exceptions are contained so the rest of the run continues. The PAT is the only credential; on the git side it is sent as HTTPS basic credentials with username `x-access-token`, and only to the host the repository was listed on.
The engine lists every repository through the GitHub REST API (Octokit, paginated 100 per page), then runs the set through `Parallel.ForEachAsync` bounded by the parallelism setting. Git operations use LibGit2Sharp: missing repos are cloned with object-transfer progress; existing repos get a fetch from `origin` followed by a **fast-forward-only** merge — if a local branch has diverged from its upstream, the repo is reported Failed rather than auto-merged, so gclo never manufactures merge commits. An existing folder is only updated when its `origin` is the repository gclo listed: a same-named folder that is a clone of something else (a fork, another owner's repository, another host) is reported Failed and left untouched. A repository that was renamed or moved to another owner looks the same to gclo; the failure message gives the `git remote set-url` command that points the folder at its new URL. Archived repositories are included and sync normally. Every repo ends in exactly one terminal state — Done, Failed (with the error), or Canceled — and per-repo exceptions are contained so the rest of the run continues. The PAT is the only credential; on the git side it is sent as HTTPS basic credentials with username `x-access-token`, and only to the host the repository was listed on.
## Build from source
+4 -4
View File
@@ -86,16 +86,16 @@ flowchart LR
| T1 | Update feed | Tampering | Attacker serves a malicious update | Low | Critical | HTTPS to this repository's Releases only; Velopack hash verification; immutable releases; provenance and SBOM attestations; Scorecard; Authenticode pending (EX-0002) | Mitigated (signing pending) |
| T2 | Package managers / downloads | Tampering | Modified installer or CLI zip on a mirror | Low | High | `SHA256SUMS`; Scoop and Chocolatey manifests pin the hash; "Verifying a release" instructions | Mitigated |
| T3 | Git remotes | Tampering / EoP | Repository with Windows-invalid or traversal paths (`..`, reserved names, case collisions) writes outside or corrupts the target | Medium | High | Pre-checkout validation of every incoming path; `RepositoryPathResolver` containment; recovery by rename/skip or clone in WSL; `core.longpaths` | Mitigated |
| T4 | Git remotes | Tampering | Hostile `.git` config or hooks in a pre-existing target repository | Low | High | gclo runs no hooks (libgit2, no shell); only fetch and fast-forward; `gclo.checkoutpending` marks incomplete trees; a folder is fetched only when its `origin` names the listed repository, and always over the listed URL, so the folder's configuration chooses neither the remote nor the transport (`RemoteUrl`, #139) | Mitigated for the remote and transport; other `.git` content of an existing folder is still trusted |
| T4 | Git remotes | Tampering | Hostile `.git` config or hooks in a pre-existing target repository | Low | High | gclo runs no hooks (libgit2, no shell); only fetch and fast-forward; `gclo.checkoutpending` marks incomplete trees; a folder is fetched only when its `origin` names the listed repository, and the remote contacted is the listed URL, not origin's spelling of it; a `url.<base>.insteadOf` rewrite in the folder's own configuration is refused, and so is its `core.sshCommand` when the fetch is not HTTPS (`RemoteUrl`, #139). The user's global git configuration still applies, as it does to git itself | Mitigated for the remote and the transport program; other `.git` content of an existing folder is still trusted |
| T5 | PAT handling | Information disclosure | Token leaks to disk, logs, process listings, or UI | Medium | Critical | Memory-only in Quick Sync; Credential Manager otherwise; accounts may reference the default token (`TokenSource`), resolved at use time through one resolver and never copied; logs record lengths; no `--token`; WSL via env only; secret scanning + gitleaks + CodeQL in CI | Mitigated |
| T6 | Credential Manager | Information disclosure | Another process as the same user reads the stored token | Medium | High | Documented boundary (SECURITY.md); vault scoped per data directory so test or portable instances never see the default profile's tokens | Accepted (OS boundary) |
| T7 | GitHub API | Denial of service | Rate limits or abuse detection stop a sync | Medium | Low | Secondary-rate-limit and abuse handling with backoff; per-repo failure isolation; cancellation | Mitigated |
| T8 | GitHub API / remotes | Spoofing | A MITM impersonates GitHub | Low | Critical | HTTPS with system trust store (Octokit, libgit2); no HTTP fallback: an existing folder whose origin is `http://` or SSH is fetched over the listed HTTPS URL instead | Mitigated |
| T8 | GitHub API / remotes | Spoofing | A MITM impersonates GitHub | Low | Critical | HTTPS with system trust store (Octokit, libgit2); no HTTP fallback: an existing folder whose origin is `http://` or SSH is fetched through the listed HTTPS URL instead (unless the user's own global configuration rewrites that URL) | Mitigated |
| T9 | WSL recovery | Elevation of privilege | Hostile org/repo names injected into the shell script, or used to leave `~/gclo` | Low | High | Script fed on stdin with positional parameters; organization and repository each validated as one plain name inside `WslCloner` (and the organization again before any listing request); the URL must be HTTPS and is passed after `--`; no string interpolation into the script | Mitigated |
| T10 | Logs and crash output | Information disclosure | Exception text carries a token or private path | Low | Medium | Crash net logs messages only; log review in incident handling; tokens never in exception messages (tests) | Mitigated |
| T11 | Release pipeline | Tampering | Compromised action or cache poisons a release | Low | Critical | Build runs in the standards' shared release workflow (SLSA Build L3 provenance, `--signer-repo KofTwentyTwo/standards`), SHA-pinned actions, no caches, read-only tokens, publishing tokens only in the tag-restricted `release` environment, `protect-release-tags`, required gates re-run on the tag | Mitigated |
| T12 | Repository | Repudiation | Unattributed change reaches `main` | Low | High | Signed, signed-off commits; squash-only through gated pull requests; no bypass actors | Mitigated |
| T14 | PAT handling | Information disclosure | The token is offered to a host other than the one the repository was listed on: an existing folder whose `origin` points elsewhere, a redirect, or a server named by the repository's own configuration during the WSL clone (for example an LFS endpoint) | Low | Critical | The git credential callback answers only HTTPS requests for the listed URL's host and port, and refuses everything else with a clear error; a folder whose origin names another repository fails before any request; the WSL credential helper is scoped to the listed host (`credential.https://<host>.helper`) and an existing WSL destination is pulled only when its origin is the listed URL (#139) | Mitigated |
| T14 | PAT handling | Information disclosure | The token is offered to a host other than the one the repository was listed on: an existing folder whose `origin` points elsewhere, or a server named by the repository's own configuration during the WSL clone (for example an LFS endpoint) | Low | Critical | The git credential callback answers only a remote whose URL is HTTPS on the listed URL's host and port, and refuses every other remote with a clear error (a redirect that the listed host itself issues is followed by libgit2; that host already holds the token); a folder whose origin names another repository fails before any request; the WSL credential helper is scoped to the listed host (`credential.https://<host>.helper`) and an existing WSL destination is pulled only when its stored origin is the listed URL (#139) | Mitigated |
| T13 | CLI / data directory | Tampering | A malicious `accounts.json` or `settings.json` redirects targets | Low | Medium | Files live under the user profile; sanitized on load; target paths resolved and contained | Accepted (same-user boundary) |
## 7. Accepted risks
@@ -116,4 +116,4 @@ flowchart LR
| 2026-10-10 | v1.0.2 | James Maes (@KofTwentyTwo) | Local data directory moved to `%LOCALAPPDATA%\KofTwentyTwo\gclo`: the Velopack install root is deleted by Setup.exe, which destroyed accounts.json and settings.json (#91) |
| 2026-10-10 | v1.0.2 | James Maes (@KofTwentyTwo) | T2/T11: releases are Authenticode-signed with Azure Artifact Signing over GitHub OIDC (no signing secret; federated credential bound to this repository's `release` environment; signer role scoped to the certificate profile) and the published assets are verified with `signtool verify /pa /all` (#57, ADR 0008) |
| 2026-10-10 | v1.1.0 | James Maes (@KofTwentyTwo) | T5: accounts can reference the default token (`TokenSource = Default`) instead of holding a copy; one `AccountTokenResolver` serves the app and the CLI, so rotating the default token in Settings rotates every such account (#102, docs/plans/default-token-ux.md) |
| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T14 added, T4/T8/T9 updated: an existing folder is fetched only when its origin names the listed repository and always over the listed URL; the git credential callback and the WSL credential helper answer only for the listed host over HTTPS; WSL organization and repository names validated as single names (#139) |
| 2026-11-10 | v1.1.1 | James Maes (@KofTwentyTwo) | T14 added, T4/T8/T9 updated: an existing folder is fetched only when its origin names the listed repository, through the listed URL, and the folder's own `url.*.insteadOf` and (for a non-HTTPS fetch) `core.sshCommand` are refused; the git credential callback and the WSL credential helper answer only for the listed host over HTTPS; WSL organization and repository names validated as single names (#139) |
+114
View File
@@ -0,0 +1,114 @@
/*
* Copyright (c) 2026 James Maes (KofTwentyTwo)
* SPDX-License-Identifier: MIT
*/
using LibGit2Sharp;
using LibGit2Sharp.Handlers;
namespace gclo.Engine.Tests;
/// <summary>
/// The credential callback <see cref="LibGit2GitClient"/> hands to libgit2 (#139). The
/// offline fixtures are local paths, which never ask for credentials, so the callback
/// is called here the way libgit2 calls it: with the URL of the remote that asks.
/// </summary>
public sealed class GitCredentialsTests
{
private const string Token = "ghp_secret_token_value_1234567890";
private const string Listed = "https://github.com/acme/repo.git";
[Theory]
[InlineData("https://github.com/acme/repo.git")]
[InlineData("https://GitHub.com/acme/repo.git")]
public void ListedHostOverHttps_GetsTheTokenAsThePassword(string asking)
{
CredentialsHandler provider = LibGit2GitClient.MakeCredentialsProvider(Token, Listed);
UsernamePasswordCredentials credentials = Assert.IsType<UsernamePasswordCredentials>(provider(asking, null, SupportedCredentialTypes.UsernamePassword));
Assert.Equal("x-access-token", credentials.Username);
Assert.Equal(Token, credentials.Password);
}
[Theory]
[InlineData("https://evil.example/acme/repo.git")]
[InlineData("https://github.com.evil.example/acme/repo.git")]
[InlineData("https://github.com:8443/acme/repo.git")]
[InlineData("http://github.com/acme/repo.git")]
[InlineData("ssh://git@github.com/acme/repo.git")]
[InlineData("git@github.com:acme/repo.git")]
public void AnyOtherRemote_GetsNothing_AndTheErrorDoesNotCarryTheToken(string asking)
{
CredentialsHandler provider = LibGit2GitClient.MakeCredentialsProvider(Token, Listed);
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(
() => provider(asking, null, SupportedCredentialTypes.UsernamePassword));
Assert.Contains("Nothing was sent", ex.Message);
Assert.DoesNotContain(Token, ex.Message);
}
[Fact]
public void ListedUrlThatIsNotHttps_NobodyGetsTheToken()
{
CredentialsHandler provider = LibGit2GitClient.MakeCredentialsProvider(Token, "http://git.example.com/acme/repo.git");
Assert.Throws<InvalidOperationException>(
() => provider("http://git.example.com/acme/repo.git", null, SupportedCredentialTypes.UsernamePassword));
}
[Fact]
public void ErrorForAnotherRemote_DoesNotShowCredentialsInItsUrl()
{
CredentialsHandler provider = LibGit2GitClient.MakeCredentialsProvider(Token, Listed);
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(
() => provider("https://someone:hunter2@evil.example/acme/repo.git", null, SupportedCredentialTypes.UsernamePassword));
Assert.Contains("https://evil.example/acme/repo.git", ex.Message);
Assert.DoesNotContain("hunter2", ex.Message);
}
[Fact]
public void ThirdRequest_MeansTheTokenWasRejected_NotAnEndlessReplay()
{
CredentialsHandler provider = LibGit2GitClient.MakeCredentialsProvider(Token, Listed);
provider(Listed, null, SupportedCredentialTypes.UsernamePassword);
provider(Listed, null, SupportedCredentialTypes.UsernamePassword);
InvalidOperationException ex = Assert.Throws<InvalidOperationException>(
() => provider(Listed, null, SupportedCredentialTypes.UsernamePassword));
Assert.Contains("rejected the token", ex.Message);
Assert.DoesNotContain(Token, ex.Message);
}
[Fact]
public void RefusedRequests_DoNotCountTowardTheReplayLimit()
{
CredentialsHandler provider = LibGit2GitClient.MakeCredentialsProvider(Token, Listed);
for(int i = 0; i < 5; i++)
{
Assert.Throws<InvalidOperationException>(
() => provider("https://evil.example/x.git", null, SupportedCredentialTypes.UsernamePassword));
}
Assert.IsType<UsernamePasswordCredentials>(provider(Listed, null, SupportedCredentialTypes.UsernamePassword));
}
}
+118 -2
View File
@@ -55,6 +55,9 @@ public sealed class OriginVerificationTests : IDisposable
Assert.Contains("clone of a different repository", ex.Message);
Assert.Contains(unrelated, ex.Message);
Assert.Contains(listed, ex.Message);
// A renamed or transferred repository looks exactly like this; the message says
// how to point the folder at the new URL.
Assert.Contains($"git -C \"{target}\" remote set-url origin {listed}", ex.Message);
Assert.Equal(tipBefore, HeadSha(target));
Assert.Equal(trackingBefore, TrackingTip(target));
}
@@ -82,11 +85,16 @@ public sealed class OriginVerificationTests : IDisposable
Assert.Equal("new content", File.ReadAllText(Path.Combine(target, "update.txt")));
using(var repo = new Repository(target))
{
// The folder's configuration is as the user left it: origin keeps its own
// spelling, and the remote used for the fetch is gone.
// Origin keeps the spelling the user gave it, and the remote used for the
// fetch is gone.
Assert.Equal(new Uri(source).AbsoluteUri, repo.Network.Remotes["origin"].Url);
Assert.Equal("origin", Assert.Single(repo.Network.Remotes).Name);
}
// What was contacted is the listed URL, not origin's spelling of it: FETCH_HEAD
// records the URL of the remote a fetch used.
string fetchHead = File.ReadAllText(Path.Combine(target, ".git", "FETCH_HEAD"));
Assert.Contains(source.Replace('\\', '/'), fetchHead.Replace('\\', '/'));
Assert.DoesNotContain("file://", fetchHead);
}
@@ -137,6 +145,7 @@ public sealed class OriginVerificationTests : IDisposable
repo.Network.Remotes.Update("origin", r => r.Url = new Uri(source).AbsoluteUri);
// What a process killed mid-fetch leaves behind, pointing somewhere stale.
repo.Network.Remotes.Add("gclo-sync", Path.Combine(_root, "gone"));
repo.Config.Set("remote.gclo-sync.gclo-temporary", true, ConfigurationLevel.Local);
}
string newSha = CommitFile(source, "update.txt", "new content", "second commit");
@@ -207,6 +216,113 @@ public sealed class OriginVerificationTests : IDisposable
[Fact]
public async Task Pull_ARemoteLeftByAKilledRun_IsAlsoRemoved_WhenOriginIsTheListedUrl()
{
string source = CreateSourceRepo("source");
string target = Path.Combine(_root, "clone");
await _client.CloneAsync(source, target, Token, null, CancellationToken.None);
using(var repo = new Repository(target))
{
repo.Network.Remotes.Add("gclo-sync", Path.Combine(_root, "gone"));
repo.Config.Set("remote.gclo-sync.gclo-temporary", true, ConfigurationLevel.Local);
}
await _client.PullAsync(target, Token, CancellationToken.None);
using(var repo2 = new Repository(target))
{
Assert.Equal("origin", Assert.Single(repo2.Network.Remotes).Name);
}
}
[Fact]
public async Task Pull_TheUsersOwnRemoteNamedGcloSync_IsNeverRemoved()
{
string source = CreateSourceRepo("source");
string elsewhere = CreateSourceRepo("elsewhere");
string target = Path.Combine(_root, "clone");
await _client.CloneAsync(source, target, Token, null, CancellationToken.None);
using(var repo = new Repository(target))
{
repo.Network.Remotes.Add("gclo-sync", elsewhere); // theirs: no gclo marker
}
// Origin is the listed URL: the name is not needed, and their remote stays.
await _client.PullAsync(target, Token, CancellationToken.None);
using(var repo = new Repository(target))
{
Assert.Equal(elsewhere, repo.Network.Remotes["gclo-sync"].Url);
repo.Network.Remotes.Update("origin", r => r.Url = new Uri(source).AbsoluteUri);
}
// Origin is spelled differently: the name is needed, so the pull says so instead
// of taking it over.
InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => _client.FetchAndPullAsync(source, target, Token, CancellationToken.None));
Assert.Contains("remote of its own named 'gclo-sync'", ex.Message);
using(var repo = new Repository(target))
{
Assert.Equal(elsewhere, repo.Network.Remotes["gclo-sync"].Url);
}
}
[Fact]
public async Task Pull_FolderConfigurationRewritesUrls_IsRefused_BeforeAnythingIsFetched()
{
string source = CreateSourceRepo("source");
string other = CreateSourceRepo("other");
string target = Path.Combine(_root, "clone");
await _client.CloneAsync(source, target, Token, null, CancellationToken.None);
string trackingBefore = TrackingTip(target);
using(var repo = new Repository(target))
{
// url.<other>.insteadOf = <source>: every fetch of the listed URL would
// really contact the other repository.
repo.Config.Set($"url.{other}.insteadof", source, ConfigurationLevel.Local);
}
CommitFile(source, "update.txt", "new content", "second commit");
CommitFile(other, "planted.txt", "not what was listed", "planted commit");
InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => _client.FetchAndPullAsync(source, target, Token, CancellationToken.None));
Assert.Contains("url.<base>.insteadOf", ex.Message);
Assert.Equal(trackingBefore, TrackingTip(target));
Assert.False(File.Exists(Path.Combine(target, "planted.txt")));
}
[Fact]
public async Task Pull_FolderConfigurationNamesAnSshProgram_IsRefused_WhenTheFetchIsNotHttps()
{
// The fixtures are local paths, which stand in here for "not HTTPS": the case
// where the user's global configuration sends the fetch over SSH.
string source = CreateSourceRepo("source");
string target = Path.Combine(_root, "clone");
await _client.CloneAsync(source, target, Token, null, CancellationToken.None);
string trackingBefore = TrackingTip(target);
using(var repo = new Repository(target))
{
repo.Config.Set("core.sshCommand", "calc.exe", ConfigurationLevel.Local);
}
CommitFile(source, "update.txt", "new content", "second commit");
InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
() => _client.PullAsync(target, Token, CancellationToken.None));
Assert.Contains("core.sshCommand", ex.Message);
Assert.Equal(trackingBefore, TrackingTip(target));
}
// ---------------------------------------------------------------- helpers
private string CreateSourceRepo(string name)
+40 -5
View File
@@ -42,6 +42,8 @@ public sealed class RemoteUrlTests
[InlineData("https://github.com/acme/repo.git/extra")]
[InlineData("https://github.example.com/acme/repo.git")]
[InlineData("https://gitlab.com/acme/repo.git")]
[InlineData("https://github.com:8443/acme/repo.git")]
[InlineData("https://\u00fc-/acme/repo.git")]
[InlineData("git@bitbucket.org:acme/repo.git")]
[InlineData("ssh://git@old-host.example/acme/repo.git")]
[InlineData(@"C:\src\acme\repo")]
@@ -101,6 +103,8 @@ public sealed class RemoteUrlTests
[Theory]
[InlineData("https://evil.example/acme/repo.git")]
[InlineData("https://github.com.evil.example/acme/repo.git")]
[InlineData("https://github.com@evil.example/acme/repo.git")]
[InlineData("https://\u00fc-/acme/repo.git")]
[InlineData("https://github.com:8443/acme/repo.git")]
[InlineData("http://github.com/acme/repo.git")]
[InlineData("ssh://git@github.com/acme/repo.git")]
@@ -141,6 +145,8 @@ public sealed class RemoteUrlTests
[InlineData("https://github.com/acme/repo.git", "github.com")]
[InlineData("https://GitHub.com:443/acme/repo.git", "github.com")]
[InlineData("https://git.example.com:3000/acme/repo.git", "git.example.com:3000")]
[InlineData("https://[::1]:3000/acme/repo.git", "[::1]:3000")]
[InlineData("https://\u00fc-/acme/repo.git", null)]
[InlineData("http://github.com/acme/repo.git", null)]
[InlineData("git@github.com:acme/repo.git", null)]
[InlineData("", null)]
@@ -157,16 +163,45 @@ public sealed class RemoteUrlTests
[Theory]
[InlineData("https://someone:hunter2@github.com/acme/repo.git", "https://github.com/acme/repo.git")]
[InlineData("https://token@github.com/acme/repo.git", "https://github.com/acme/repo.git")]
[InlineData("ssh://git@github.com", "ssh://github.com")]
[InlineData(" https://someone:hunter2@github.com/acme/repo.git ", "https://github.com/acme/repo.git")]
[InlineData("https://someone:hun@ter2@github.com/acme/repo.git", "https://github.com/acme/repo.git")]
[InlineData("https://someone:hun/ter2@github.com/acme/repo.git", "https://github.com/acme/repo.git")]
[InlineData("https://github.com/acme/repo.git?access_token=hunter2", "https://github.com/acme/repo.git")]
[InlineData("https://github.com/acme/repo.git#hunter2", "https://github.com/acme/repo.git")]
[InlineData("https://someone:hunter2@git.example.com:3000/acme/repo.git", "https://git.example.com:3000/acme/repo.git")]
[InlineData("https://someone:hunter2@[::1]:3000/acme/repo.git", "https://[::1]:3000/acme/repo.git")]
[InlineData("ssh://git@github.com", "ssh://github.com/")]
[InlineData("https://github.com/acme/repo.git", "https://github.com/acme/repo.git")]
[InlineData("https://github.com/acme/user@example.git", "https://github.com/acme/user@example.git")]
[InlineData("https://github.com", "https://github.com")]
[InlineData("https://github.com", "https://github.com/")]
[InlineData("https://", "https://")]
[InlineData(@"C:\src\acme\repo", @"C:\src\acme\repo")]
[InlineData("git@github.com:acme/repo.git", "git@github.com:acme/repo.git")]
[InlineData("git@github.com:acme/repo.git", "github.com:acme/repo.git")]
[InlineData("someone:hunter2@github.com:acme/repo.git", "github.com:acme/repo.git")]
[InlineData("", "")]
[InlineData(null, "")]
public void Redact_DropsCredentialsFromTheAuthority_AndNothingElse(string? url, string expected)
public void Redact_NeverShowsUserInformationQueryOrFragment(string? url, string expected)
{
Assert.Equal(expected, RemoteUrl.Redact(url));
string redacted = RemoteUrl.Redact(url);
Assert.Equal(expected, redacted);
Assert.DoesNotContain("hunter2", redacted);
Assert.DoesNotContain("ter2", redacted);
}
[Theory]
[InlineData("https://github.com/acme/repo.git", true)]
[InlineData("HTTPS://github.com", true)]
[InlineData("http://github.com/acme/repo.git", false)]
[InlineData("ssh://git@github.com/acme/repo.git", false)]
[InlineData("git@github.com:acme/repo.git", false)]
[InlineData(@"C:\src\acme\repo", false)]
[InlineData("https://\u00fc-/acme/repo.git", false)]
[InlineData(null, false)]
public void IsHttps_OnlyAnAbsoluteHttpsUrlWithAReadableHost(string? url, bool expected)
{
Assert.Equal(expected, RemoteUrl.IsHttps(url));
}
}
+5 -2
View File
@@ -281,9 +281,12 @@ public sealed class WslClonerTests
// "--" ends option parsing, so a URL can never be read as an option.
Assert.Contains("clone --quiet -- \"$url\" \"$dest\"", script);
// An existing destination is only fast-forwarded when it is a clone of this URL.
Assert.Contains("remote get-url origin)\" != \"$url\"", script);
// The stored URL is compared, not "remote get-url": that one applies the user's
// url.*.insteadOf rewrites and would refuse a destination gclo itself cloned.
Assert.Contains("config --get remote.origin.url)\" != \"$url\"", script);
Assert.DoesNotContain("remote get-url", script);
Assert.True(
script.IndexOf("remote get-url origin", StringComparison.Ordinal) < script.IndexOf("pull --ff-only", StringComparison.Ordinal),
script.IndexOf("config --get remote.origin.url", StringComparison.Ordinal) < script.IndexOf("pull --ff-only", StringComparison.Ordinal),
"the origin check must come before the pull");
}
+105 -16
View File
@@ -66,6 +66,12 @@ public sealed class LibGit2GitClient : IGitClient
/// </summary>
private const string SyncRemoteRefSpec = "+refs/heads/*:refs/remotes/" + SyncRemoteName + "/*";
/// <summary>
/// Marks the sync remote as gclo's, so cleanup never removes a remote of the same
/// name that the user created. It goes away with the remote.
/// </summary>
private const string SyncRemoteMarkerKey = "remote." + SyncRemoteName + ".gclo-temporary";
/// <summary>
/// Test seam: invoked right after a fetch completed, before the cancellation
/// check that precedes any working-tree work. Local-transport fixtures fire no
@@ -363,45 +369,123 @@ public sealed class LibGit2GitClient : IGitClient
/// <summary>
/// Fetches the listed <paramref name="url"/> into <paramref name="origin"/>'s own
/// refspecs, so the tracking refs land exactly where a fetch of origin puts them
/// while the transport is always the URL the listing returned.
/// while the remote that is contacted is the one the listing returned.
/// </summary>
/// <remarks>
/// When origin's URL is the listed URL, which is every clone gclo made itself, this
/// is a plain fetch of origin. Otherwise the origin names the same repository in
/// another spelling: switched to SSH for pushing, plain HTTP, credentials typed
/// into it. Fetching that origin would let the folder's configuration choose the
/// transport (and, for SSH, the program that provides it). LibGit2Sharp cannot
/// fetch a bare URL, so the listed URL is fetched through a remote that exists
/// only for this call; origin itself is never rewritten (#139).
/// transport. LibGit2Sharp cannot fetch a bare URL, so the listed URL is fetched
/// through a remote that exists only for this call; origin itself is never
/// rewritten. The user's own (global) git configuration still applies to that
/// remote, as it does to everything git does for them (#139).
/// </remarks>
private static void FetchListedUrl(Repository repo, Remote origin, string url, FetchOptions options)
{
TryRemoveSyncRemote(repo); // left behind by a run that was killed mid-fetch
var refSpecs = origin.FetchRefSpecs.Select(s => s.Specification).ToList();
if(string.Equals(origin.Url, url, StringComparison.Ordinal))
{
RefuseSshCommandOfTheFolder(repo, origin.Url);
Commands.Fetch(repo, origin.Name, refSpecs, options, logMessage: null);
return;
}
RemoveSyncRemote(repo); // left behind by a run that was killed mid-fetch
repo.Network.Remotes.Add(SyncRemoteName, url, SyncRemoteRefSpec);
if(repo.Network.Remotes[SyncRemoteName] is not null)
{
// Not marked as gclo's, or TryRemoveSyncRemote would have removed it.
throw new InvalidOperationException(
$"This repository has a remote of its own named '{SyncRemoteName}', a name gclo needs for a moment "
+ "to fetch the listed URL. Rename that remote, then sync again.");
}
Remote sync = repo.Network.Remotes.Add(SyncRemoteName, url, SyncRemoteRefSpec);
repo.Config.Set(SyncRemoteMarkerKey, true, ConfigurationLevel.Local);
try
{
RefuseSshCommandOfTheFolder(repo, sync.Url);
// The fetch behaves like a fetch of origin in every respect but the URL.
options.TagFetchMode = origin.TagFetchMode;
Commands.Fetch(repo, SyncRemoteName, refSpecs, options, logMessage: null);
}
finally
{
RemoveSyncRemote(repo);
TryRemoveSyncRemote(repo);
}
}
private static void RemoveSyncRemote(Repository repo)
/// <summary>
/// Removes the remote <see cref="FetchListedUrl"/> adds, when it is there and carries
/// gclo's marker; a remote of that name the user made themselves is left alone.
/// Best effort: a failure here (another program holding .git/config open) must not
/// replace the outcome of the fetch, and the next sync removes what is left.
/// </summary>
private static void TryRemoveSyncRemote(Repository repo)
{
if(repo.Network.Remotes[SyncRemoteName] is not null)
try
{
repo.Network.Remotes.Remove(SyncRemoteName);
if(repo.Network.Remotes[SyncRemoteName] is not null && repo.Config.Get<bool>(SyncRemoteMarkerKey)?.Value == true)
{
repo.Network.Remotes.Remove(SyncRemoteName);
}
}
catch(LibGit2SharpException)
{
// See the summary: never mask the fetch's own result.
}
}
/// <summary>
/// A folder's own git configuration must not decide where a sync's fetch goes. Two
/// settings could: <c>url.&lt;base&gt;.insteadOf</c>, which rewrites the URL of any
/// remote, and (see <see cref="RefuseSshCommandOfTheFolder"/>) the program used for
/// SSH. Both are refused when they come from the folder's own configuration; the
/// user's global configuration is theirs and is honored (#139).
/// </summary>
private static void RefuseUrlRewritesOfTheFolder(Repository repo)
{
foreach(ConfigurationEntry<string> entry in repo.Config)
{
if(entry.Level >= ConfigurationLevel.Local
&& entry.Key.StartsWith("url.", StringComparison.OrdinalIgnoreCase)
&& entry.Key.EndsWith(".insteadof", StringComparison.OrdinalIgnoreCase))
{
throw new InvalidOperationException(
"This folder's own git configuration rewrites remote URLs (a url.<base>.insteadOf entry in its .git/config), "
+ "so a fetch could go somewhere other than the listed repository. Remove that entry, then sync again.");
}
}
}
/// <summary>
/// When the URL a fetch will really use is not HTTPS (the user's global git
/// configuration rewrites it to SSH), the transport is an external SSH program.
/// Which program must not be up to the folder: a <c>core.sshCommand</c> in its own
/// configuration is refused (#139).
/// </summary>
private static void RefuseSshCommandOfTheFolder(Repository repo, string effectiveUrl)
{
if(RemoteUrl.IsHttps(effectiveUrl))
{
return;
}
foreach(ConfigurationEntry<string> entry in repo.Config)
{
if(entry.Level >= ConfigurationLevel.Local && string.Equals(entry.Key, "core.sshcommand", StringComparison.OrdinalIgnoreCase))
{
throw new InvalidOperationException(
"This folder's own git configuration sets core.sshCommand, and this fetch would go over SSH. "
+ "gclo does not run a program chosen by the folder it is syncing. Remove that entry from its .git/config, then sync again.");
}
}
}
@@ -417,14 +501,18 @@ public sealed class LibGit2GitClient : IGitClient
/// </summary>
private static Remote OriginOfListedRepository(Repository repo, string url)
{
RefuseUrlRewritesOfTheFolder(repo);
Remote remote = repo.Network.Remotes["origin"]
?? throw new InvalidOperationException("Repository has no 'origin' remote.");
if(!RemoteUrl.IsSameRepository(url, remote.Url))
{
string folder = Path.TrimEndingDirectorySeparator(repo.Info.WorkingDirectory ?? repo.Info.Path);
throw new InvalidOperationException(
$"This folder is a clone of a different repository: its origin is '{RemoteUrl.Redact(remote.Url)}', not '{url}'. "
+ "Move or delete the folder, or point its origin at the listed repository, then sync again.");
+ "If the repository was renamed or moved to another owner, point the folder at it with "
+ $"'git -C \"{folder}\" remote set-url origin {url}'. Otherwise move or delete the folder. Then sync again.");
}
return remote;
@@ -889,12 +977,13 @@ public sealed class LibGit2GitClient : IGitClient
/// up with "too many redirects or authentication replays", once per repository.
/// A correct token is never asked for a third time, so the third request means the
/// token is bad and gets the same actionable message the API listers produce (#31).
/// The token is for <paramref name="expectedUrl"/>'s host only: a request for any
/// other host (a redirect, a folder configured for another server) or over anything
/// but HTTPS is refused, so the token never leaves for somewhere the repository was
/// not listed on (#139).
/// The token is for <paramref name="expectedUrl"/>'s host only: a remote on any
/// other host, or one reached over anything but HTTPS, is refused, so the token is
/// never offered to a server the repository was not listed on (#139). libgit2 asks
/// with the remote's own URL; a redirect the listed host itself issues is that
/// host's doing, and it already holds the token.
/// </summary>
private static CredentialsHandler MakeCredentialsProvider(string token, string expectedUrl)
internal static CredentialsHandler MakeCredentialsProvider(string token, string expectedUrl)
{
int requests = 0;
return (requestedUrl, _, _) =>
+133 -53
View File
@@ -13,21 +13,25 @@ namespace gclo.Engine;
/// <summary>
/// Decides two things about git remote URLs, both from the clone URL a listing returned
/// (the "expected" URL): whether an existing folder's <c>origin</c> names that same
/// repository, and whether a host that asks for credentials is the one the token is
/// repository, and whether a remote that asks for credentials is one the token is
/// meant for. A sync must never report a different repository as updated, and must
/// never hand the token to a host the listing did not come from (#139).
/// never hand the token to a remote on another host (#139).
/// </summary>
internal static class RemoteUrl
{
private static readonly char[] s_separators = ['/', '\\'];
/// <summary>
/// True when <paramref name="actual"/> (a folder's configured origin) names the same
/// repository as <paramref name="expected"/> (the listed clone URL). Network URLs
/// are compared by host and path, ignoring scheme, credentials, port, letter case,
/// a trailing slash and a trailing <c>.git</c>, so an origin switched to SSH still
/// matches its HTTPS listing. Local paths (the offline test fixtures, or a caller's
/// own mirror) are compared as full paths. A network URL never matches a local path.
/// are compared by host and path, ignoring scheme, credentials, letter case, a
/// trailing slash and a trailing <c>.git</c>, so an origin switched to SSH still
/// matches its HTTPS listing; the port counts only between two HTTP(S) URLs, where
/// it tells two servers on one host apart. Local paths (the offline test fixtures,
/// or a caller's own mirror) are compared as full paths. A network URL never
/// matches a local path, and a URL that cannot be read matches nothing.
/// </summary>
public static bool IsSameRepository(string expected, string actual)
{
@@ -37,50 +41,72 @@ internal static class RemoteUrl
return false;
}
(string Host, string Path)? expectedRemote = ParseNetwork(expected);
(string Host, string Path)? actualRemote = ParseNetwork(actual);
if(expectedRemote is null || actualRemote is null)
Location expectedLocation = Parse(expected);
Location actualLocation = Parse(actual);
if(expectedLocation.Kind != actualLocation.Kind || expectedLocation.Kind == LocationKind.Unreadable)
{
return expectedRemote is null
&& actualRemote is null
&& string.Equals(NormalizeLocal(expected), NormalizeLocal(actual), StringComparison.OrdinalIgnoreCase);
return false;
}
return string.Equals(expectedRemote.Value.Host, actualRemote.Value.Host, StringComparison.OrdinalIgnoreCase)
&& string.Equals(expectedRemote.Value.Path, actualRemote.Value.Path, StringComparison.OrdinalIgnoreCase);
if(expectedLocation.Kind == LocationKind.Local)
{
return string.Equals(
expectedLocation.Path, actualLocation.Path,
OperatingSystem.IsWindows() ? StringComparison.OrdinalIgnoreCase : StringComparison.Ordinal);
}
return string.Equals(expectedLocation.Host, actualLocation.Host, StringComparison.OrdinalIgnoreCase)
&& string.Equals(expectedLocation.Path, actualLocation.Path, StringComparison.OrdinalIgnoreCase)
&& (expectedLocation.WebPort is null || actualLocation.WebPort is null || expectedLocation.WebPort == actualLocation.WebPort);
}
/// <summary>
/// True when a credential request for <paramref name="requested"/> may be answered
/// with the token of a sync whose listed clone URL is <paramref name="expected"/>:
/// both must be HTTPS and name the same host and port. Anything else (another host
/// after a redirect, plain HTTP, SSH) gets no token.
/// True when a credential request from a remote whose URL is
/// <paramref name="requested"/> may be answered with the token of a sync whose listed
/// clone URL is <paramref name="expected"/>: both must be HTTPS and name the same
/// host and port. A remote on another host, plain HTTP, and SSH get no token.
/// </summary>
public static bool MayReceiveCredentials(string expected, string? requested)
{
ArgumentException.ThrowIfNullOrWhiteSpace(expected);
return TryParseHttps(expected, out Uri? expectedUri)
&& TryParseHttps(requested, out Uri? requestedUri)
&& string.Equals(expectedUri.IdnHost, requestedUri.IdnHost, StringComparison.OrdinalIgnoreCase)
return TryParseHttps(expected, out Uri? expectedUri, out string? expectedHost)
&& TryParseHttps(requested, out Uri? requestedUri, out string? requestedHost)
&& string.Equals(expectedHost, requestedHost, StringComparison.OrdinalIgnoreCase)
&& expectedUri.Port == requestedUri.Port;
}
/// <summary>True when <paramref name="url"/> is an absolute HTTPS URL with a readable host.</summary>
public static bool IsHttps(string? url) => TryParseHttps(url, out _, out _);
/// <summary>
/// The host of an HTTPS <paramref name="url"/>, with its port when that is not 443;
/// null when the URL is not absolute HTTPS. This is the scope a token may be used in.
/// The host of an HTTPS <paramref name="url"/>, with its port when that is not 443
/// and with the brackets of an IPv6 literal; null when the URL is not absolute HTTPS.
/// This is the scope a token may be used in.
/// </summary>
public static string? HttpsAuthority(string? url)
=> TryParseHttps(url, out Uri? uri) ? uri.IdnHost + (uri.IsDefaultPort ? "" : ":" + uri.Port.ToString(CultureInfo.InvariantCulture)) : null;
{
if(!TryParseHttps(url, out Uri? uri, out string? host))
{
return null;
}
string authority = uri.HostNameType == UriHostNameType.IPv6 ? $"[{host}]" : host;
return uri.IsDefaultPort ? authority : authority + ":" + uri.Port.ToString(CultureInfo.InvariantCulture);
}
/// <summary>
/// <paramref name="url"/> without any <c>user:password@</c> part, for error messages
/// and logs: an origin may carry credentials someone typed into it.
/// <paramref name="url"/> in a form that is safe to show and log: an origin may
/// carry credentials someone typed into it. A readable network URL is rebuilt from
/// its scheme, host, port and path, so user information, query and fragment are
/// gone; anything else loses everything up to its last <c>@</c>.
/// </summary>
public static string Redact(string? url)
{
@@ -89,58 +115,103 @@ internal static class RemoteUrl
return "";
}
int scheme = url.IndexOf("://", StringComparison.Ordinal);
if(scheme < 0)
string trimmed = url.Trim();
int scheme = trimmed.IndexOf("://", StringComparison.Ordinal);
if(scheme > 0 && Uri.TryCreate(trimmed, UriKind.Absolute, out Uri? uri) && !uri.IsFile && TryGetHost(uri, out string? host))
{
return url;
string authority = uri.HostNameType == UriHostNameType.IPv6 ? $"[{host}]" : host;
string port = uri.IsDefaultPort ? "" : ":" + uri.Port.ToString(CultureInfo.InvariantCulture);
return $"{uri.Scheme}://{authority}{port}{uri.AbsolutePath}";
}
int authorityStart = scheme + 3;
int authorityEnd = url.IndexOf('/', authorityStart);
int at = url.IndexOf('@', authorityStart);
return at >= 0 && (authorityEnd < 0 || at < authorityEnd)
? string.Concat(url.AsSpan(0, authorityStart), url.AsSpan(at + 1))
: url;
int at = trimmed.LastIndexOf('@');
if(at < 0)
{
return trimmed;
}
return scheme > 0 ? string.Concat(trimmed.AsSpan(0, scheme + 3), trimmed.AsSpan(at + 1)) : trimmed[(at + 1)..];
}
private static bool TryParseHttps(string? url, [NotNullWhen(true)] out Uri? uri)
=> Uri.TryCreate(url, UriKind.Absolute, out uri)
&& string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase);
private static bool TryParseHttps(string? url, [NotNullWhen(true)] out Uri? uri, [NotNullWhen(true)] out string? host)
{
host = null;
return Uri.TryCreate(url, UriKind.Absolute, out uri)
&& string.Equals(uri.Scheme, Uri.UriSchemeHttps, StringComparison.OrdinalIgnoreCase)
&& TryGetHost(uri, out host);
}
/// <summary>Host and normalized path of a network git URL, or null for a local path.</summary>
private static (string Host, string Path)? ParseNetwork(string url)
/// <summary>
/// The host in its ASCII (punycode) form. <see cref="Uri.IdnHost"/> throws for a
/// host that parses as a URI but is not a valid internationalized name; such a
/// host is nobody's, not an error.
/// </summary>
private static bool TryGetHost(Uri uri, [NotNullWhen(true)] out string? host)
{
try
{
host = uri.IdnHost;
return host.Length > 0;
}
catch(UriFormatException)
{
host = null;
return false;
}
}
private static Location Parse(string url)
{
string trimmed = url.Trim();
int scheme = trimmed.IndexOf("://", StringComparison.Ordinal);
if(scheme > 0)
{
return Uri.TryCreate(trimmed, UriKind.Absolute, out Uri? uri) && !uri.IsFile && uri.IdnHost.Length > 0
? (uri.IdnHost, NormalizePath(Uri.UnescapeDataString(uri.AbsolutePath)))
: null;
if(!Uri.TryCreate(trimmed, UriKind.Absolute, out Uri? uri))
{
return new Location(LocationKind.Unreadable, "", "", null);
}
if(uri.IsFile)
{
return Local(uri.LocalPath);
}
bool web = uri.Scheme is "http" or "https";
return TryGetHost(uri, out string? host)
? new Location(LocationKind.Network, host, NormalizePath(Uri.UnescapeDataString(uri.AbsolutePath)), web ? (uri.IsDefaultPort ? 0 : uri.Port) : null)
: new Location(LocationKind.Unreadable, "", "", null);
}
// The scp-like form git accepts for SSH: [user@]host:path. A single letter before
// the colon is a Windows drive ("C:\src\repo"), not a host, and a separator
// before the colon means the colon belongs to a local path.
int colon = trimmed.IndexOf(':', StringComparison.Ordinal);
if(colon < 0)
if(colon >= 0)
{
return null;
string host = trimmed[(trimmed.LastIndexOf('@', colon) + 1)..colon];
string path = trimmed[(colon + 1)..];
if(host.Length >= 2 && host.IndexOfAny(s_separators) < 0 && path.Length > 0 && path[0] != '\\')
{
return new Location(LocationKind.Network, host, NormalizePath(path), null);
}
}
string host = trimmed[(trimmed.LastIndexOf('@', colon) + 1)..colon];
string path = trimmed[(colon + 1)..];
return host.Length >= 2 && host.IndexOfAny(s_separators) < 0 && path.Length > 0 && path[0] != '\\'
? (host, NormalizePath(path))
: null;
return Local(trimmed);
}
private static Location Local(string path)
=> new(LocationKind.Local, "", Path.TrimEndingDirectorySeparator(Path.GetFullPath(path)), null);
private static string NormalizePath(string path)
{
string trimmed = path.Trim('/');
@@ -149,10 +220,19 @@ internal static class RemoteUrl
private static string NormalizeLocal(string path)
private enum LocationKind
{
string local = Uri.TryCreate(path.Trim(), UriKind.Absolute, out Uri? uri) && uri.IsFile ? uri.LocalPath : path.Trim();
return Path.TrimEndingDirectorySeparator(Path.GetFullPath(local));
Network,
Local,
Unreadable,
}
/// <summary>Where a git URL points: a host and path, or a local full path.</summary>
/// <param name="Kind">Network, local, or not readable as either.</param>
/// <param name="Host">ASCII host of a network URL; empty otherwise.</param>
/// <param name="Path">Normalized repository path, or the local full path.</param>
/// <param name="WebPort">Port of an HTTP(S) URL, 0 for the scheme's default (so HTTP and HTTPS on their usual ports agree); null for SSH, scp-like and local.</param>
private readonly record struct Location(LocationKind Kind, string Host, string Path, int? WebPort);
}
+2 -2
View File
@@ -35,8 +35,8 @@ public sealed class WslCloner : IWslCloner
+ "dest=\"$HOME/gclo/$org/$repo\"\n"
+ "helper='!f() { echo username=x-access-token; echo \"password=$" + TokenVariable + "\"; }; f'\n"
+ "if [ -d \"$dest/.git\" ]; then\n"
+ " if [ \"$(git -C \"$dest\" remote get-url origin)\" != \"$url\" ]; then\n"
+ " echo \"$dest is a clone of a different repository; move or delete it, then try again\" >&2\n"
+ " if [ \"$(git -C \"$dest\" config --get remote.origin.url)\" != \"$url\" ]; then\n"
+ " echo \"$dest exists but is not a clone of this repository; move or delete it, then try again\" >&2\n"
+ " exit 1\n"
+ " fi\n"
+ " git -C \"$dest\" -c credential.helper= -c \"credential.https://$host.helper=$helper\" pull --ff-only --quiet\n"