From 15a0fc03e06aaeea2a805d5a8a79d563cfb390e5 Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Fri, 28 Aug 2026 14:18:13 -0700 Subject: [PATCH] Fix clone authentication when includeIf is in git config 'gvfs clone' failed to authenticate when the user's git config contained an 'includeIf "gitdir:..."' section: Cannot clone because authentication failed: fatal: Invalid path '/src': No such file or directory Clone authenticates before it creates the enlistment, but the credential verbs ran against the enlistment's .git folder through --git-dir. Git resolves that path before it evaluates an includeIf condition, so git aborted instead of asking for credentials. The condition did not need to match. Any 'gitdir:' condition was enough. Pass --git-dir only when .git exists. Credential helpers still see repo-local configuration in an established enlistment, and there is no local configuration to read before clone creates the repository. A linked worktree stores .git as a file rather than a folder, so accept both. Git resolves the 'gitdir:' pointer in that file, so the shared repository's configuration is still used. Route the other two pre-repository git calls the same way. Both hit the same failure during clone: - TryGetConfigUrlMatch runs from the GitAuthentication constructor. When it failed, GitSsl was never built and the user's client-certificate settings were silently dropped. - TryGetCertificatePassword is itself a credential fill. Record which route ran in the credential failure metadata, so a later report of "the wrong credentials were used" can be diagnosed from the logs. Add unit tests for all three states of .git (missing, folder, worktree file) across all three credential verbs, and functional tests that clone with and without an includeIf section. The functional tests assert that authentication was reached, not only that the failure message changed: a clone that cannot reach the server also exits with the same code and prints neither of the strings under test, so absence checks alone would pass with no network. Also correct two entries in AGENTS.md that this work proved wrong. GVFS.Payload has no ProjectReference items and never builds or publishes the projects layout.bat copies from, so a stale binary can be packaged and tested silently. And RunFunctionalTests-Dev.ps1 binds positional arguments to -Arch, so a bare 'Debug --test=...' fails validation. Assisted-by: Claude Opus 5 Signed-off-by: Tyrie Vella --- AGENTS.md | 41 +++- GVFS/GVFS.Common/Git/GitProcess.cs | 116 ++++++++-- .../Tests/CloneAuthTests.cs | 163 ++++++++++++++ .../Common/Git/GitProcessCredentialTests.cs | 209 ++++++++++++++++++ .../GVFS.UnitTests/Mock/Git/MockGitProcess.cs | 35 ++- 5 files changed, 538 insertions(+), 26 deletions(-) create mode 100644 GVFS/GVFS.FunctionalTests/Tests/CloneAuthTests.cs create mode 100644 GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs diff --git a/AGENTS.md b/AGENTS.md index e5dd769f3d..31ac6ef919 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -64,8 +64,8 @@ Skips `dotnet publish`, AOT, native C++ projects, payload assembly, installer. For changes that need the GVFS payload (`gvfs.exe`, hooks, service) but not an installer. `PublishAot=false` skips ilc (~3–4 min saved); `SkipCreateInstaller=true` skips Inno Setup (~95 s saved). -`GVFS.Payload` cascades to its dependencies (GVFS, GVFS.Mount, GVFS.Hooks, -GVFS.Service) via `ProjectReference`. +`GVFS.Payload` only assembles the payload directory — it does **not** build or +publish the projects it copies from (see the warning below). > **Prerequisite: the native C++ projects must already be built.** They are > `.vcxproj` (see [Native C++ projects](#native-c-projects-need-msbuild-not-dotnet-build) @@ -78,10 +78,13 @@ GVFS.Service) via `ProjectReference`. ```powershell dotnet publish src\GVFS\GVFS.FunctionalTests\GVFS.FunctionalTests.csproj ` -c Debug /p:PublishAot=false +dotnet publish src\GVFS\GVFS\GVFS.csproj ` + -c Debug /p:PublishAot=false dotnet publish src\GVFS\GVFS.Payload\GVFS.Payload.csproj ` -c Debug /p:PublishAot=false /p:SkipCreateInstaller=true -src\scripts\RunFunctionalTests-Dev.ps1 Debug --test=GVFS.FunctionalTests.Tests... +src\scripts\RunFunctionalTests-Dev.ps1 -Configuration Debug -Arch x64 ` + --test=GVFS.FunctionalTests.Tests... ``` `layout.bat` (invoked by GVFS.Payload) `xcopy`s from each project's `publish\` @@ -89,11 +92,39 @@ or native-output directory — the C# projects do not require AOT, so `PublishAot=false` produces a fully functional test payload. The native hook binaries are copied straight from the vcxproj output. +> **⚠️ Publish each changed project explicitly — `GVFS.Payload` does not do it +> for you.** `GVFS.Payload.csproj` is a `Microsoft.Build.NoTargets` project with +> **no `ProjectReference` items at all**; its `CreatePayload` target just runs +> `layout.bat`, which `xcopy`s from each project's existing output/publish +> directory. Nothing in that chain rebuilds or republishes those projects, so a +> project you did not publish contributes whatever was left there by the last +> `Build.bat` — an AOT binary that predates your edit. The build succeeds and the +> test then runs stale code, which looks exactly like "my fix did not work". +> +> Verify the payload actually changed before you trust a functional-test result: +> +> ```powershell +> Get-Item out\GVFS.Payload\bin\Debug\win-x64\GVFS.exe | +> Format-List LastWriteTime, Length +> ``` +> +> A ~27 MB `GVFS.exe` is the AOT build from `Build.bat`; a ~160 KB one is the +> `PublishAot=false` build from the command above. If the timestamp predates +> your edit, publish the project that owns the changed code and re-run the +> payload publish. The same applies to `GVFS.Mount`, `GVFS.Hooks`, and +> `GVFS.Service` when you change those. + `RunFunctionalTests-Dev.ps1` runs functional tests against the build output without requiring admin or a system-wide install. It launches the test service as a console process. Each invocation gets a unique service name and data dir, so concurrent runs from different worktrees don't collide. +> **Pass `-Configuration` and `-Arch` by name.** The script's first two +> positional parameters are `Configuration` and `Arch`, so a bare +> `RunFunctionalTests-Dev.ps1 Debug --test=...` binds `--test=...` to `-Arch` +> and fails its `ValidateSet`. Name both parameters, then let `--test=` fall +> through to `ExtraArgs`. + ### Path C — Installer build (~5 min — only when you need an installer) For producing an installable package (testing install/upgrade flows, or @@ -136,11 +167,11 @@ participate in the C# inner-loop paths above. ```powershell # ✅ Correct & "out\GVFS.UnitTests\bin\...\GVFS.UnitTests.exe" --test "GVFS.UnitTests.Common.WorktreeInfoTests" -src\scripts\RunFunctionalTests-Dev.ps1 Debug --test=GVFS.FunctionalTests.Tests.GVFSVerbTests.UnknownVerb +src\scripts\RunFunctionalTests-Dev.ps1 -Configuration Debug -Arch x64 --test=GVFS.FunctionalTests.Tests.GVFSVerbTests.UnknownVerb # ❌ Wrong — silently runs the entire suite & "out\GVFS.UnitTests\bin\...\GVFS.UnitTests.exe" --where "class =~ Worktree" -src\scripts\RunFunctionalTests-Dev.ps1 Debug --where "cat == Smoke" +src\scripts\RunFunctionalTests-Dev.ps1 -Configuration Debug -Arch x64 --where "cat == Smoke" ``` For unit tests, `--where` is merely annoying (the whole suite runs in diff --git a/GVFS/GVFS.Common/Git/GitProcess.cs b/GVFS/GVFS.Common/Git/GitProcess.cs index 03cc27b417..f18759ab9a 100644 --- a/GVFS/GVFS.Common/Git/GitProcess.cs +++ b/GVFS/GVFS.Common/Git/GitProcess.cs @@ -210,10 +210,9 @@ public virtual bool TryDeleteCredential(ITracer tracer, string repoUrl, string u string stdinConfig = sb.ToString(); - Result result = this.InvokeGitAgainstDotGitFolder( + Result result = this.InvokeGitAgainstDotGitFolderOrOutsideEnlistment( GenerateCredentialVerbCommand("reject"), stdin => stdin.Write(stdinConfig), - null, usePreCommandHook: false); if (result.ExitCodeIsFailure) @@ -238,10 +237,9 @@ public virtual bool TryStoreCredential(ITracer tracer, string repoUrl, string us string stdinConfig = sb.ToString(); - Result result = this.InvokeGitAgainstDotGitFolder( + Result result = this.InvokeGitAgainstDotGitFolderOrOutsideEnlistment( GenerateCredentialVerbCommand("approve"), stdin => stdin.Write(stdinConfig), - null, usePreCommandHook: false); if (result.ExitCodeIsFailure) @@ -275,10 +273,9 @@ public virtual bool TryGetCertificatePassword( { // See GetFromConfig for why pre-command hook is disabled // for bootstrap-time git operations. - Result gitCredentialOutput = this.InvokeGitAgainstDotGitFolder( + Result gitCredentialOutput = this.InvokeGitAgainstDotGitFolderOrOutsideEnlistment( "credential fill", stdin => stdin.Write("protocol=cert\npath=" + certificatePath + "\nusername=\n\n"), - parseStdOutLine: null, usePreCommandHook: false); if (gitCredentialOutput.ExitCodeIsFailure) @@ -328,12 +325,10 @@ public virtual bool TryGetCredential( using (ITracer activity = tracer.StartActivity(nameof(this.TryGetCredential), EventLevel.Informational)) { - // See GetFromConfig for why pre-command hook is disabled - // for bootstrap-time git operations. - Result gitCredentialOutput = this.InvokeGitAgainstDotGitFolder( + Result gitCredentialOutput = this.InvokeGitAgainstDotGitFolderOrOutsideEnlistment( GenerateCredentialVerbCommand("fill"), stdin => stdin.Write($"url={repoUrl}\n\n"), - parseStdOutLine: null, + out bool usedDotGitFolder, usePreCommandHook: false, timeoutMs: timeoutMs); @@ -341,6 +336,10 @@ public virtual bool TryGetCredential( { EventMetadata errorData = new EventMetadata(); + // Records whether repo-local configuration (and therefore a + // repo-local credential.helper) was visible to git. + errorData.Add(nameof(usedDotGitFolder), usedDotGitFolder); + if (gitCredentialOutput.Errors.StartsWith("Operation timed out")) { errorMessage = "Credential manager did not respond within " + (timeoutMs / 1000) + " seconds"; @@ -442,7 +441,13 @@ public Result SetInFileConfig(string configFile, string settingName, string valu public bool TryGetConfigUrlMatch(string section, string repositoryUrl, out Dictionary configSettings) { // See GetFromConfig for why pre-command hook is disabled. - Result result = this.InvokeGitAgainstDotGitFolder($"config --get-urlmatch {section} {repositoryUrl}", usePreCommandHook: false); + // This runs from the GitAuthentication constructor, which happens before + // clone creates the enlistment, so it must tolerate a missing .git folder. + Result result = this.InvokeGitAgainstDotGitFolderOrOutsideEnlistment( + $"config --get-urlmatch {section} {repositoryUrl}", + writeStdIn: null, + usePreCommandHook: false); + if (result.ExitCodeIsFailure) { configSettings = null; @@ -1109,7 +1114,8 @@ private Result InvokeGitOutsideEnlistment( string command, Action writeStdIn, Action parseStdOutLine, - int timeout = -1) + int timeout = -1, + bool usePreCommandHook = true) { return this.InvokeGitImpl( command, @@ -1118,7 +1124,91 @@ private Result InvokeGitOutsideEnlistment( useReadObjectHook: false, writeStdIn: writeStdIn, parseStdOutLine: parseStdOutLine, - timeoutMs: timeout); + timeoutMs: timeout, + usePreCommandHook: usePreCommandHook); + } + + /// + /// Invokes git.exe against an enlistment's .git folder when that folder exists, + /// and outside the enlistment when it does not. + /// + /// + /// For commands that prefer an enlistment's configuration but do not require a + /// repository to run. Naming a --git-dir that is absent is not merely redundant: + /// git resolves that path before it evaluates an 'includeIf "gitdir:..."' + /// condition, so a user who has such a section in their config gets + /// "fatal: Invalid path ...: No such file or directory" and the command never + /// runs. The condition does not have to match for this to happen. + /// + /// The credential verbs need this because 'gvfs clone' authenticates before it + /// creates the enlistment. This method should be used only with commands that + /// still behave correctly with no repository, because there is no repo-local + /// configuration to read on the fallback path. + /// + private Result InvokeGitAgainstDotGitFolderOrOutsideEnlistment( + string command, + Action writeStdIn, + Action parseStdOutLine = null, + bool usePreCommandHook = true, + int timeoutMs = -1) + { + return this.InvokeGitAgainstDotGitFolderOrOutsideEnlistment( + command, + writeStdIn, + out bool _, + parseStdOutLine, + usePreCommandHook, + timeoutMs); + } + + /// + /// Overload that reports which route was taken, so callers can record it. + /// + /// + /// True when --git-dir was passed and repo-local configuration was therefore + /// visible to git; false when the command ran with no repository. + /// + private Result InvokeGitAgainstDotGitFolderOrOutsideEnlistment( + string command, + Action writeStdIn, + out bool usedDotGitFolder, + Action parseStdOutLine = null, + bool usePreCommandHook = true, + int timeoutMs = -1) + { + // Evaluate once so the reported route always matches the route taken. + usedDotGitFolder = this.DotGitRootExists(); + + if (usedDotGitFolder) + { + return this.InvokeGitAgainstDotGitFolder( + command, + writeStdIn, + parseStdOutLine, + usePreCommandHook: usePreCommandHook, + timeoutMs: timeoutMs); + } + + return this.InvokeGitOutsideEnlistment( + command, + writeStdIn, + parseStdOutLine, + timeout: timeoutMs, + usePreCommandHook: usePreCommandHook); + } + + /// + /// Determines whether the enlistment's .git exists. In a linked worktree .git is a + /// file that points at the real git directory rather than a folder, so check for both. + /// + private bool DotGitRootExists() + { + if (string.IsNullOrEmpty(this.dotGitRoot)) + { + return false; + } + + return Directory.Exists(this.dotGitRoot) || File.Exists(this.dotGitRoot); } /// diff --git a/GVFS/GVFS.FunctionalTests/Tests/CloneAuthTests.cs b/GVFS/GVFS.FunctionalTests/Tests/CloneAuthTests.cs new file mode 100644 index 0000000000..88ce8f1ef2 --- /dev/null +++ b/GVFS/GVFS.FunctionalTests/Tests/CloneAuthTests.cs @@ -0,0 +1,163 @@ +using GVFS.FunctionalTests.Tools; +using GVFS.Tests.Should; +using NUnit.Framework; +using System; +using System.Diagnostics; +using System.IO; + +namespace GVFS.FunctionalTests.Tests +{ + /// + /// Tests for the authentication step of 'gvfs clone', which runs before the + /// enlistment's src folder exists. + /// + [TestFixture] + public class CloneAuthTests + { + private const int GVFSGenericError = 3; + + /// + /// The message GVFS reports when it cannot reach the server at all. It must not + /// appear in these tests: it means the clone failed before it asked for + /// credentials, so the test proved nothing. + /// + private const string ConfigQueryFailed = "Unable to query /gvfs/config"; + + private string testRoot; + + /// + /// A URL that always requires authentication, so that 'gvfs clone' runs + /// 'git credential fill' instead of succeeding anonymously. The project does + /// not exist, but Azure DevOps answers with 401 before it checks existence, + /// which is what this test needs. The host comes from the configured test + /// repo so that no new network dependency is introduced. + /// + private static string AuthRequiredRepoUrl + { + get + { + Uri repoToClone = new Uri(Properties.Settings.Default.RepoToClone); + return repoToClone.GetLeftPart(UriPartial.Authority) + "/NoSuchProject/_git/NoSuchRepo"; + } + } + + [SetUp] + public void CreateTestRoot() + { + this.testRoot = Path.Combine( + Properties.Settings.Default.EnlistmentRoot, + "CloneAuthTests_" + Guid.NewGuid().ToString("N").Substring(0, 8)); + Directory.CreateDirectory(this.testRoot); + } + + [TearDown] + public void DeleteTestRoot() + { + RepositoryHelpers.DeleteTestDirectory(this.testRoot); + } + + /// + /// 'gvfs clone' must reach the credential helper when the user's global config + /// contains an 'includeIf "gitdir:..."' section. Git resolves the --git-dir path + /// before it evaluates the condition, so passing the not-yet-created src folder + /// makes git fail with "Invalid path" instead of asking for credentials. + /// + [TestCase] + public void CloneAuthenticatesWhenGitConfigContainsIncludeIfGitDir() + { + string globalConfig = this.WriteGlobalConfig(includeIfGitDir: true); + string enlistmentRoot = Path.Combine(this.testRoot, "enlistment"); + + ProcessResult result = this.RunClone(enlistmentRoot, globalConfig); + + ShouldFailInTheCredentialHelper(result); + } + + /// + /// Control for . + /// Without the includeIf section the same clone must reach the credential helper. + /// + [TestCase] + public void CloneAuthenticatesWhenGitConfigHasNoIncludeIf() + { + string globalConfig = this.WriteGlobalConfig(includeIfGitDir: false); + string enlistmentRoot = Path.Combine(this.testRoot, "enlistment"); + + ProcessResult result = this.RunClone(enlistmentRoot, globalConfig); + + ShouldFailInTheCredentialHelper(result); + } + + /// + /// Asserts that the clone reached the credential helper and failed there. + /// + /// + /// The positive assertions matter as much as the negative ones. A clone that + /// cannot reach the server also exits with and + /// prints neither "Invalid path" nor "No such file or directory", so a test that + /// only checks for the absence of those strings passes when the network is down + /// and proves nothing. + /// + private static void ShouldFailInTheCredentialHelper(ProcessResult result) + { + string output = result.Output + result.Errors; + + // No credentials are available, so the clone must fail. + result.ExitCode.ShouldEqual(GVFSGenericError, output); + + // It must have got as far as authentication, and the server must have + // answered, otherwise the rest of this test is meaningless. + output.ShouldContain("Authenticating...Failed"); + output.ShouldNotContain(false, ConfigQueryFailed); + + // It must fail because the credential helper produced no password, not + // because git could not resolve the src folder that clone has not created + // yet. + output.ShouldNotContain(true, "Invalid path", "No such file or directory"); + } + + private string WriteGlobalConfig(bool includeIfGitDir) + { + // An empty helper value clears the credential helper list, so git fails + // immediately instead of showing an interactive credential prompt. + string contents = "[credential]\n\thelper =\n"; + + if (includeIfGitDir) + { + string includedConfig = Path.Combine(this.testRoot, "included.gitconfig"); + File.WriteAllText(includedConfig, string.Empty); + + // The condition never matches. Git still resolves the repository's + // gitdir before it compares the pattern, which is what triggers the + // failure this test guards against. + contents += + "[includeIf \"gitdir:NoSuchDirectory/\"]\n" + + "\tpath = " + includedConfig.Replace('\\', '/') + "\n"; + } + + string globalConfig = Path.Combine(this.testRoot, "global.gitconfig"); + File.WriteAllText(globalConfig, contents); + return globalConfig; + } + + private ProcessResult RunClone(string enlistmentRoot, string globalConfig) + { + ProcessStartInfo processInfo = new ProcessStartInfo(GVFSTestConfig.PathToGVFS); + processInfo.Arguments = $"clone {AuthRequiredRepoUrl} \"{enlistmentRoot}\" --no-mount --no-prefetch"; + processInfo.WindowStyle = ProcessWindowStyle.Hidden; + processInfo.CreateNoWindow = true; + processInfo.WorkingDirectory = this.testRoot; + processInfo.UseShellExecute = false; + processInfo.RedirectStandardOutput = true; + processInfo.RedirectStandardError = true; + + // Point git at the test's config instead of the config of the user running + // the tests, so the test controls whether includeIf is present. + processInfo.EnvironmentVariables["GIT_CONFIG_GLOBAL"] = globalConfig; + processInfo.EnvironmentVariables["GIT_CONFIG_SYSTEM"] = Path.Combine(this.testRoot, "system.gitconfig"); + processInfo.EnvironmentVariables["GIT_TERMINAL_PROMPT"] = "0"; + + return ProcessHelper.Run(processInfo); + } + } +} diff --git a/GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs b/GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs new file mode 100644 index 0000000000..2ba912cb1c --- /dev/null +++ b/GVFS/GVFS.UnitTests/Common/Git/GitProcessCredentialTests.cs @@ -0,0 +1,209 @@ +using GVFS.Common.Git; +using GVFS.Tests.Should; +using GVFS.UnitTests.Mock.Common; +using GVFS.UnitTests.Mock.Git; +using NUnit.Framework; +using System; +using System.IO; + +namespace GVFS.UnitTests.Common.Git +{ + /// + /// Verifies which repository the credential verbs run against. + /// + /// + /// Credential helpers must see repo-local configuration, so the credential verbs + /// normally run against the enlistment's .git folder. During 'gvfs clone' that + /// folder does not exist yet, and git refuses to resolve a --git-dir that is not + /// there when the user's config contains an 'includeIf "gitdir:..."' section. + /// + [TestFixture] + public class GitProcessCredentialTests + { + private const string CredentialFillCommandPrefix = "-c " + GitConfigSetting.CredentialUseHttpPath + "=true credential fill"; + private const string CredentialApproveCommandPrefix = "-c " + GitConfigSetting.CredentialUseHttpPath + "=true credential approve"; + private const string CredentialRejectCommandPrefix = "-c " + GitConfigSetting.CredentialUseHttpPath + "=true credential reject"; + private const string RepoUrl = "mock://repoUrl"; + + private string testRoot; + + [SetUp] + public void CreateTestRoot() + { + this.testRoot = Path.Combine(Path.GetTempPath(), "GitProcessCredentialTests_" + Guid.NewGuid().ToString("N").Substring(0, 8)); + Directory.CreateDirectory(this.testRoot); + } + + [TearDown] + public void DeleteTestRoot() + { + if (Directory.Exists(this.testRoot)) + { + Directory.Delete(this.testRoot, recursive: true); + } + } + + [TestCase] + public void CredentialVerbDoesNotUseGitDirWhenDotGitIsMissing() + { + // 'gvfs clone' authenticates before it creates the enlistment, so the + // credential verb must not name a .git folder that does not exist yet. + string workingDirectoryRoot = Path.Combine(this.testRoot, "src"); + + string dotGitDirectoryUsed = this.RunCredentialFill(workingDirectoryRoot); + + dotGitDirectoryUsed.ShouldBeNull(); + } + + [TestCase] + public void CredentialVerbUsesGitDirWhenDotGitIsAFolder() + { + // In an established enlistment the credential helper must still see the + // repository's local configuration. + string workingDirectoryRoot = Path.Combine(this.testRoot, "src"); + string dotGitRoot = Path.Combine(workingDirectoryRoot, ".git"); + Directory.CreateDirectory(dotGitRoot); + + string dotGitDirectoryUsed = this.RunCredentialFill(workingDirectoryRoot); + + dotGitDirectoryUsed.ShouldEqual(dotGitRoot); + } + + [TestCase] + public void CredentialVerbUsesGitDirWhenDotGitIsAWorktreeFile() + { + // A linked worktree has a .git file that points at the real git directory + // instead of a .git folder. That is still a valid repository. + string workingDirectoryRoot = Path.Combine(this.testRoot, "src"); + Directory.CreateDirectory(workingDirectoryRoot); + string dotGitRoot = Path.Combine(workingDirectoryRoot, ".git"); + File.WriteAllText(dotGitRoot, "gitdir: " + Path.Combine(this.testRoot, "worktrees", "src")); + + string dotGitDirectoryUsed = this.RunCredentialFill(workingDirectoryRoot); + + dotGitDirectoryUsed.ShouldEqual(dotGitRoot); + } + + [TestCase] + public void StoreCredentialDoesNotUseGitDirWhenDotGitIsMissing() + { + string workingDirectoryRoot = Path.Combine(this.testRoot, "src"); + + string dotGitDirectoryUsed = this.RunStoreCredential(workingDirectoryRoot); + + dotGitDirectoryUsed.ShouldBeNull(); + } + + [TestCase] + public void StoreCredentialUsesGitDirWhenDotGitIsAFolder() + { + string workingDirectoryRoot = Path.Combine(this.testRoot, "src"); + string dotGitRoot = Path.Combine(workingDirectoryRoot, ".git"); + Directory.CreateDirectory(dotGitRoot); + + string dotGitDirectoryUsed = this.RunStoreCredential(workingDirectoryRoot); + + dotGitDirectoryUsed.ShouldEqual(dotGitRoot); + } + + [TestCase] + public void DeleteCredentialDoesNotUseGitDirWhenDotGitIsMissing() + { + string workingDirectoryRoot = Path.Combine(this.testRoot, "src"); + + string dotGitDirectoryUsed = this.RunDeleteCredential(workingDirectoryRoot); + + dotGitDirectoryUsed.ShouldBeNull(); + } + + [TestCase] + public void DeleteCredentialUsesGitDirWhenDotGitIsAFolder() + { + string workingDirectoryRoot = Path.Combine(this.testRoot, "src"); + string dotGitRoot = Path.Combine(workingDirectoryRoot, ".git"); + Directory.CreateDirectory(dotGitRoot); + + string dotGitDirectoryUsed = this.RunDeleteCredential(workingDirectoryRoot); + + dotGitDirectoryUsed.ShouldEqual(dotGitRoot); + } + + private static MockGitProcess CreateGitProcess(string workingDirectoryRoot, string verbCommandPrefix) + { + MockGitProcess gitProcess = new MockGitProcess(Path.Combine("mock:", "git"), workingDirectoryRoot); + gitProcess.SetExpectedCommandResult( + verbCommandPrefix, + () => new GitProcess.Result("username=mockUser\npassword=mockPassword\n", string.Empty, GitProcess.Result.SuccessCode), + matchPrefix: true); + + return gitProcess; + } + + /// + /// Returns the --git-dir used by the single git invocation the caller triggered. + /// + /// + /// Takes the count from before the invocation so that the assertion stays valid + /// if a credential verb ever makes more than one git call. + /// + private static string SingleDotGitDirectoryUsed(MockGitProcess gitProcess, int countBeforeInvocation) + { + gitProcess.DotGitDirectoriesUsed.Count.ShouldEqual( + countBeforeInvocation + 1, + "Expected exactly one git invocation for the credential verb"); + + return gitProcess.DotGitDirectoriesUsed[countBeforeInvocation]; + } + + private string RunCredentialFill(string workingDirectoryRoot) + { + MockGitProcess gitProcess = CreateGitProcess(workingDirectoryRoot, CredentialFillCommandPrefix); + int countBeforeInvocation = gitProcess.DotGitDirectoriesUsed.Count; + + gitProcess.TryGetCredential( + new MockTracer(), + RepoUrl, + out string username, + out string password, + out string error) + .ShouldBeTrue(error); + + gitProcess.CommandsRun.ShouldContain(x => x.StartsWith(CredentialFillCommandPrefix, StringComparison.Ordinal)); + return SingleDotGitDirectoryUsed(gitProcess, countBeforeInvocation); + } + + private string RunStoreCredential(string workingDirectoryRoot) + { + MockGitProcess gitProcess = CreateGitProcess(workingDirectoryRoot, CredentialApproveCommandPrefix); + int countBeforeInvocation = gitProcess.DotGitDirectoriesUsed.Count; + + gitProcess.TryStoreCredential( + new MockTracer(), + RepoUrl, + "mockUser", + "mockPassword", + out string error) + .ShouldBeTrue(error); + + gitProcess.CommandsRun.ShouldContain(x => x.StartsWith(CredentialApproveCommandPrefix, StringComparison.Ordinal)); + return SingleDotGitDirectoryUsed(gitProcess, countBeforeInvocation); + } + + private string RunDeleteCredential(string workingDirectoryRoot) + { + MockGitProcess gitProcess = CreateGitProcess(workingDirectoryRoot, CredentialRejectCommandPrefix); + int countBeforeInvocation = gitProcess.DotGitDirectoriesUsed.Count; + + gitProcess.TryDeleteCredential( + new MockTracer(), + RepoUrl, + "mockUser", + "mockPassword", + out string error) + .ShouldBeTrue(error); + + gitProcess.CommandsRun.ShouldContain(x => x.StartsWith(CredentialRejectCommandPrefix, StringComparison.Ordinal)); + return SingleDotGitDirectoryUsed(gitProcess, countBeforeInvocation); + } + } +} diff --git a/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs b/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs index c9095cc2c8..10da74bb0a 100644 --- a/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs +++ b/GVFS/GVFS.UnitTests/Mock/Git/MockGitProcess.cs @@ -17,17 +17,26 @@ public class MockGitProcess : GitProcess public MockGitProcess() : base(new MockGVFSEnlistment()) { - this.CommandsRun = new List(); - this.StoredCredentials = new Dictionary(StringComparer.OrdinalIgnoreCase); - this.CredentialApprovals = new Dictionary>(); - this.CredentialRejections = new Dictionary>(); + this.Initialize(); } - public List CommandsRun { get; } + public MockGitProcess(string gitBinPath, string workingDirectoryRoot) + : base(gitBinPath, workingDirectoryRoot) + { + this.Initialize(); + } + + public List CommandsRun { get; private set; } public bool ShouldFail { get; set; } - public Dictionary StoredCredentials { get; } - public Dictionary> CredentialApprovals { get; } - public Dictionary> CredentialRejections { get; } + public Dictionary StoredCredentials { get; private set; } + public Dictionary> CredentialApprovals { get; private set; } + public Dictionary> CredentialRejections { get; private set; } + + /// + /// The value passed as --git-dir for each invocation, in the order the + /// invocations happened. An entry is null when no --git-dir was passed. + /// + public List DotGitDirectoriesUsed { get; private set; } public void SetExpectedCommandResult(string command, Func result, bool matchPrefix = false) { @@ -87,6 +96,7 @@ protected override Result InvokeGitImpl( bool usePrecommandHook = true) { this.CommandsRun.Add(command); + this.DotGitDirectoriesUsed.Add(dotGitDirectory); if (this.ShouldFail) { @@ -125,6 +135,15 @@ protected override Result InvokeGitImpl( return result; } + private void Initialize() + { + this.CommandsRun = new List(); + this.DotGitDirectoriesUsed = new List(); + this.StoredCredentials = new Dictionary(StringComparer.OrdinalIgnoreCase); + this.CredentialApprovals = new Dictionary>(); + this.CredentialRejections = new Dictionary>(); + } + public class Credential { public Credential(string username, string password)