diff --git a/changelog.d/unreleased/3433.security.md b/changelog.d/unreleased/3433.security.md new file mode 100644 index 0000000000..c69566a92a --- /dev/null +++ b/changelog.d/unreleased/3433.security.md @@ -0,0 +1,16 @@ +--- +category: security +issues: + - 3433 +affected: + - src/CodeIndex/Cli/GitHelper.cs + - tests/CodeIndex.Tests/GitHelperTests.cs +--- + +## English + +- **GitHelper no longer resolves git through ambient PATH (#3433)** — git commands now use a trusted absolute executable path so indexing helpers do not inherit a caller-controlled `git` from `PATH`. + +## 日本語 + +- **GitHelper が環境 PATH 経由で git を解決しないようになりました (#3433)** — git コマンドは信頼済みの絶対実行ファイルパスを使うため、indexing helper が呼び出し元に制御された `PATH` 上の `git` を継承しなくなりました。 diff --git a/changelog.d/unreleased/3455.security.md b/changelog.d/unreleased/3455.security.md new file mode 100644 index 0000000000..890a5d38ff --- /dev/null +++ b/changelog.d/unreleased/3455.security.md @@ -0,0 +1,18 @@ +--- +category: security +issues: + - 3455 +affected: + - src/CodeIndex/Indexer/DotnetHostPathResolver.cs + - src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs + - src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs + - tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +--- + +## English + +- **Worker child processes no longer fall back to PATH-resolved dotnet (#3455)** — isolated symbol extraction and post-extraction hook workers now require a trusted absolute dotnet host path and return a structured diagnostic when one cannot be resolved. + +## 日本語 + +- **worker 子プロセスが PATH 解決の dotnet にフォールバックしなくなりました (#3455)** — isolated symbol extraction と post-extraction hook worker は信頼済みの絶対 dotnet host パスを必須にし、解決できない場合は構造化された診断を返します。 diff --git a/src/CodeIndex/Cli/GitHelper.cs b/src/CodeIndex/Cli/GitHelper.cs index bc928a1f2c..3cb062a6d6 100644 --- a/src/CodeIndex/Cli/GitHelper.cs +++ b/src/CodeIndex/Cli/GitHelper.cs @@ -63,8 +63,116 @@ internal static TimeSpan GitCommandTimeout set => GitCommandTimeoutOverride.Value = value; } + private static readonly Lazy TrustedGitExecutablePath = new(ResolveTrustedGitExecutablePathFromKnownLocations); + private static readonly AsyncLocal GitExecutablePathOverrideValue = new(); + internal static string? GitExecutablePathOverride + { + get => GitExecutablePathOverrideValue.Value; + set => GitExecutablePathOverrideValue.Value = value; + } + private static readonly TimeSpan GitKillWaitTimeout = TimeSpan.FromSeconds(5); private const int GitProcessFailureExitCode = -1; + private const string TrustedGitUnavailableMessage = + "Could not resolve a trusted git executable path. Install git in a standard system location. / 信頼済みの git 実行ファイルパスを解決できませんでした。標準のシステム場所に git をインストールしてください。"; + + private static ProcessStartInfo? TryCreateGitStartInfo(string projectRoot) + { + var gitExecutablePath = TryResolveGitExecutablePath(); + if (gitExecutablePath == null) + return null; + + return new ProcessStartInfo + { + FileName = gitExecutablePath, + WorkingDirectory = projectRoot, + RedirectStandardOutput = true, + RedirectStandardError = true, + UseShellExecute = false, + CreateNoWindow = true, + }; + } + + private static ProcessStartInfo CreateGitStartInfoOrThrow(string projectRoot) + => TryCreateGitStartInfo(projectRoot) ?? throw new InvalidOperationException(TrustedGitUnavailableMessage); + + private static string? TryResolveGitExecutablePath() + { + var overridePath = NormalizeTrustedGitExecutablePath(GitExecutablePathOverrideValue.Value); + return overridePath ?? TrustedGitExecutablePath.Value; + } + + private static string? ResolveTrustedGitExecutablePathFromKnownLocations() + { + foreach (var candidate in EnumerateTrustedGitExecutableCandidates()) + { + var normalized = NormalizeTrustedGitExecutablePath(candidate); + if (normalized != null) + return normalized; + } + + return null; + } + + private static IEnumerable EnumerateTrustedGitExecutableCandidates() + { + if (OperatingSystem.IsWindows()) + { + var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); + if (!string.IsNullOrWhiteSpace(programFiles)) + { + yield return Path.Combine(programFiles, "Git", "cmd", "git.exe"); + yield return Path.Combine(programFiles, "Git", "bin", "git.exe"); + } + + var programFilesX86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86); + if (!string.IsNullOrWhiteSpace(programFilesX86)) + { + yield return Path.Combine(programFilesX86, "Git", "cmd", "git.exe"); + yield return Path.Combine(programFilesX86, "Git", "bin", "git.exe"); + } + + var windows = Environment.GetFolderPath(Environment.SpecialFolder.Windows); + if (!string.IsNullOrWhiteSpace(windows)) + yield return Path.Combine(windows, "System32", "git.exe"); + yield break; + } + + if (OperatingSystem.IsMacOS()) + { + yield return "/Library/Developer/CommandLineTools/usr/bin/git"; + yield return "/Applications/Xcode.app/Contents/Developer/usr/bin/git"; + yield break; + } + + yield return "/usr/bin/git"; + yield return "/bin/git"; + } + + internal static IReadOnlyList TrustedGitExecutableCandidatePathsForTests() + => EnumerateTrustedGitExecutableCandidates().ToList(); + + private static string? NormalizeTrustedGitExecutablePath(string? path) + { + if (string.IsNullOrWhiteSpace(path)) + return null; + + try + { + if (!Path.IsPathFullyQualified(path)) + return null; + + var fullPath = Path.GetFullPath(path); + if (!string.Equals(Path.GetFileNameWithoutExtension(fullPath), "git", StringComparison.OrdinalIgnoreCase)) + return null; + + return File.Exists(LongPath.EnsureWindowsPrefix(fullPath)) ? fullPath : null; + } + catch + { + return null; + } + } /// /// Resolve the common git directory for a project root, handling both normal repos and worktrees. @@ -147,15 +255,7 @@ public static List GetChangedFilesFromCommit( { ValidateSingleCommitRef(projectRoot, commitId, cancellationToken); - var psi = new ProcessStartInfo - { - FileName = "git", - WorkingDirectory = projectRoot, - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - }; + var psi = CreateGitStartInfoOrThrow(projectRoot); psi.ArgumentList.Add("diff-tree"); psi.ArgumentList.Add("--no-commit-id"); psi.ArgumentList.Add("--root"); @@ -205,6 +305,9 @@ private static void ValidateSingleCommitRef( string commitId, CancellationToken cancellationToken = default) { + if (TryResolveGitExecutablePath() == null) + throw new InvalidOperationException(TrustedGitUnavailableMessage); + // Reject range/pathspec syntax before invoking git so --commits remains a list // of single commit-ish values, not revision-set expressions. if (string.IsNullOrWhiteSpace(commitId) @@ -246,15 +349,7 @@ public static List GetChangedFilesBetweenRefs( ValidateGitRef(oldRef, nameof(oldRef)); ValidateGitRef(newRef, nameof(newRef)); - var psi = new ProcessStartInfo - { - FileName = "git", - WorkingDirectory = projectRoot, - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - }; + var psi = CreateGitStartInfoOrThrow(projectRoot); psi.ArgumentList.Add("diff"); psi.ArgumentList.Add("--name-status"); psi.ArgumentList.Add("-M"); @@ -432,15 +527,10 @@ private static bool TryRunGitForExitCode(string projectRoot, params string[] arg { try { - var psi = new ProcessStartInfo - { - FileName = "git", - WorkingDirectory = projectRoot, - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - }; + var psi = TryCreateGitStartInfo(projectRoot); + if (psi == null) + return false; + foreach (var arg in args) psi.ArgumentList.Add(arg); @@ -636,15 +726,9 @@ private static bool HasGitMetadataEntry(string projectRoot) { try { - var psi = new ProcessStartInfo - { - FileName = "git", - WorkingDirectory = projectRoot, - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - }; + var psi = TryCreateGitStartInfo(projectRoot); + if (psi == null) + return null; foreach (var arg in args) psi.ArgumentList.Add(arg); @@ -685,15 +769,9 @@ private static GitCommandResult RunGitCapturingResult( { try { - var psi = new ProcessStartInfo - { - FileName = "git", - WorkingDirectory = projectRoot, - RedirectStandardOutput = true, - RedirectStandardError = true, - UseShellExecute = false, - CreateNoWindow = true, - }; + var psi = TryCreateGitStartInfo(projectRoot); + if (psi == null) + return new GitCommandResult(null, null, null, TrustedGitUnavailableMessage); foreach (var arg in args) psi.ArgumentList.Add(arg); diff --git a/src/CodeIndex/Indexer/DotnetHostPathResolver.cs b/src/CodeIndex/Indexer/DotnetHostPathResolver.cs new file mode 100644 index 0000000000..3c2f3f6ace --- /dev/null +++ b/src/CodeIndex/Indexer/DotnetHostPathResolver.cs @@ -0,0 +1,81 @@ +namespace CodeIndex.Indexer; + +internal static class DotnetHostPathResolver +{ + private static readonly AsyncLocal?> TrustedDotnetHostCandidatesOverrideValue = new(); + + internal static IReadOnlyList? TrustedDotnetHostCandidatesOverride + { + get => TrustedDotnetHostCandidatesOverrideValue.Value; + set => TrustedDotnetHostCandidatesOverrideValue.Value = value; + } + + internal static string? Resolve(string? currentProcessPath) + { + if (TryNormalizeDotnetHostPath(currentProcessPath, out var normalized)) + return normalized; + + foreach (var candidate in TrustedDotnetHostCandidatesOverrideValue.Value ?? EnumerateTrustedDotnetHostCandidates()) + { + if (TryNormalizeDotnetHostPath(candidate, out normalized)) + return normalized; + } + + return null; + } + + internal static bool IsDotnetHostPath(string? path) + { + if (string.IsNullOrWhiteSpace(path)) + return false; + + try + { + return string.Equals(Path.GetFileNameWithoutExtension(path), "dotnet", StringComparison.OrdinalIgnoreCase); + } + catch + { + return false; + } + } + + private static IEnumerable EnumerateTrustedDotnetHostCandidates() + { + if (OperatingSystem.IsWindows()) + { + var programFiles = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFiles); + if (!string.IsNullOrWhiteSpace(programFiles)) + yield return Path.Combine(programFiles, "dotnet", "dotnet.exe"); + + var programFilesX86 = Environment.GetFolderPath(Environment.SpecialFolder.ProgramFilesX86); + if (!string.IsNullOrWhiteSpace(programFilesX86)) + yield return Path.Combine(programFilesX86, "dotnet", "dotnet.exe"); + + yield break; + } + + yield return "/usr/bin/dotnet"; + yield return "/usr/share/dotnet/dotnet"; + } + + private static bool TryNormalizeDotnetHostPath(string? path, out string normalized) + { + normalized = string.Empty; + if (string.IsNullOrWhiteSpace(path) || !IsDotnetHostPath(path)) + return false; + + try + { + if (!Path.IsPathFullyQualified(path)) + return false; + + normalized = Path.GetFullPath(path); + return File.Exists(LongPath.EnsureWindowsPrefix(normalized)); + } + catch + { + normalized = string.Empty; + return false; + } + } +} diff --git a/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs b/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs index 7995b81591..85f92df8f3 100644 --- a/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs +++ b/src/CodeIndex/Indexer/Hooks/PostExtractionHookCallbackWorker.cs @@ -6,6 +6,7 @@ using System.Text; using System.Text.Json; using System.Text.Json.Serialization; +using CodeIndex.Indexer; using CodeIndex.Indexer.Extensibility; using CodeIndex.Models; @@ -434,7 +435,15 @@ internal static bool TryCreateStartInfo( return false; } - startInfo.FileName = ResolveDotnetHostPath(); + var dotnetHostPath = DotnetHostPathResolver.Resolve(currentProcessPath); + if (dotnetHostPath == null) + { + startInfo = new ProcessStartInfo(); + error = "could not resolve a trusted dotnet host path for isolated hook callback execution; run cdidx through an absolute dotnet host path or use a self-contained cdidx executable."; + return false; + } + + startInfo.FileName = dotnetHostPath; startInfo.ArgumentList.Add(runnerAssemblyPath); startInfo.ArgumentList.Add(CommandName); startInfo.ArgumentList.Add(hook.AssemblyPath); @@ -644,22 +653,6 @@ private static bool TryResolveProtocolLineLimit( return false; } - private static string ResolveDotnetHostPath() - { - var dotnetHostPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH"); - if (!string.IsNullOrWhiteSpace(dotnetHostPath)) - return dotnetHostPath; - - var processPath = Environment.ProcessPath; - if (!string.IsNullOrWhiteSpace(processPath) - && string.Equals(Path.GetFileNameWithoutExtension(processPath), "dotnet", StringComparison.OrdinalIgnoreCase)) - { - return processPath; - } - - return "dotnet"; - } - private static ProcessStartInfo CreateStartInfo() => new() { @@ -675,7 +668,7 @@ private static ProcessStartInfo CreateStartInfo() private static bool ShouldStartCurrentExecutable(string? currentProcessPath, string? runnerAssemblyPath) { - if (string.IsNullOrWhiteSpace(currentProcessPath) || IsDotnetHostPath(currentProcessPath)) + if (string.IsNullOrWhiteSpace(currentProcessPath) || DotnetHostPathResolver.IsDotnetHostPath(currentProcessPath)) return false; var processName = Path.GetFileNameWithoutExtension(currentProcessPath); @@ -699,9 +692,6 @@ private static bool ShouldStartCurrentExecutable(string? currentProcessPath, str return File.Exists(candidate) ? candidate : null; } - private static bool IsDotnetHostPath(string path) - => string.Equals(Path.GetFileNameWithoutExtension(path), "dotnet", StringComparison.OrdinalIgnoreCase); - private static void ApplyCurrentRuntimeRollForward(ProcessStartInfo startInfo) { var targetMajor = GetRunnerTargetFrameworkMajor(); diff --git a/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs b/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs index a4c7a921ef..50618b37a2 100644 --- a/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs +++ b/src/CodeIndex/Indexer/Symbols/SymbolExtractionWorker.cs @@ -413,7 +413,15 @@ internal static bool TryCreateStartInfo( return false; } - startInfo.FileName = ResolveDotnetHostPath(); + var dotnetHostPath = DotnetHostPathResolver.Resolve(currentProcessPath); + if (dotnetHostPath == null) + { + startInfo = new ProcessStartInfo(); + error = "could not resolve a trusted dotnet host path for isolated symbol extraction; run cdidx through an absolute dotnet host path or use a self-contained cdidx executable."; + return false; + } + + startInfo.FileName = dotnetHostPath; startInfo.ArgumentList.Add(runnerAssemblyPath); startInfo.ArgumentList.Add(CommandName); AddProtocolLineLimitArguments(startInfo, maxProtocolLineBytes); @@ -438,7 +446,7 @@ private static ProcessStartInfo CreateStartInfo() private static bool ShouldStartCurrentExecutable(string? currentProcessPath, string? runnerAssemblyPath) { - if (string.IsNullOrWhiteSpace(currentProcessPath) || IsDotnetHostPath(currentProcessPath)) + if (string.IsNullOrWhiteSpace(currentProcessPath) || DotnetHostPathResolver.IsDotnetHostPath(currentProcessPath)) return false; var processName = Path.GetFileNameWithoutExtension(currentProcessPath); @@ -642,21 +650,6 @@ private static bool TryResolveProtocolLineLimit( return false; } - private static string ResolveDotnetHostPath() - { - var dotnetHostPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH"); - if (!string.IsNullOrWhiteSpace(dotnetHostPath)) - return dotnetHostPath; - - var processPath = Environment.ProcessPath; - if (!string.IsNullOrWhiteSpace(processPath) && IsDotnetHostPath(processPath)) - { - return processPath; - } - - return "dotnet"; - } - private static string? ResolveCurrentRunnerAssemblyPath() { var assemblyName = typeof(SymbolExtractionWorker).Assembly.GetName().Name; @@ -667,9 +660,6 @@ private static string ResolveDotnetHostPath() return File.Exists(candidate) ? candidate : null; } - private static bool IsDotnetHostPath(string path) - => string.Equals(Path.GetFileNameWithoutExtension(path), "dotnet", StringComparison.OrdinalIgnoreCase); - private static void ApplyCurrentRuntimeRollForward(ProcessStartInfo startInfo) { var targetMajor = GetRunnerTargetFrameworkMajor(); diff --git a/tests/CodeIndex.Tests/GitHelperTests.cs b/tests/CodeIndex.Tests/GitHelperTests.cs index 170a8a1c16..ebdf921e47 100644 --- a/tests/CodeIndex.Tests/GitHelperTests.cs +++ b/tests/CodeIndex.Tests/GitHelperTests.cs @@ -353,8 +353,8 @@ public async Task GetChangedFilesFromCommit_DrainsLargeStderrWithoutDeadlock() Directory.CreateDirectory(fakeGitDir); WriteFakeGitThatEmitsLargeStderr(fakeGitDir); - var oldPath = Environment.GetEnvironmentVariable("PATH"); - Environment.SetEnvironmentVariable("PATH", fakeGitDir + Path.PathSeparator + oldPath); + var oldGitExecutablePath = GitHelper.GitExecutablePathOverride; + GitHelper.GitExecutablePathOverride = Path.Combine(fakeGitDir, "git"); try { var task = Task.Run(() => GitHelper.GetChangedFilesFromCommit(repoDir, "0123456789abcdef")); @@ -364,6 +364,38 @@ public async Task GetChangedFilesFromCommit_DrainsLargeStderrWithoutDeadlock() } finally { + GitHelper.GitExecutablePathOverride = oldGitExecutablePath; + } + } + + [Fact] + public void GetChangedFilesFromCommit_UsesTrustedGitExecutableInsteadOfPath_Issue3433() + { + if (OperatingSystem.IsWindows()) + return; + + var repoDir = Path.Combine(_tempDir, "repo-trusted-git"); + Directory.CreateDirectory(repoDir); + var trustedGitDir = Path.Combine(_tempDir, "trusted-git"); + var pathGitDir = Path.Combine(_tempDir, "path-git"); + Directory.CreateDirectory(trustedGitDir); + Directory.CreateDirectory(pathGitDir); + WriteFakeGitThatReturnsChangedFile(trustedGitDir, "trusted.txt"); + WriteFakeGitThatReturnsChangedFile(pathGitDir, "path.txt"); + + var oldPath = Environment.GetEnvironmentVariable("PATH"); + var oldGitExecutablePath = GitHelper.GitExecutablePathOverride; + Environment.SetEnvironmentVariable("PATH", pathGitDir + Path.PathSeparator + oldPath); + GitHelper.GitExecutablePathOverride = Path.Combine(trustedGitDir, "git"); + try + { + var changedFiles = GitHelper.GetChangedFilesFromCommit(repoDir, "0123456789abcdef"); + + Assert.Equal(["trusted.txt"], changedFiles); + } + finally + { + GitHelper.GitExecutablePathOverride = oldGitExecutablePath; Environment.SetEnvironmentVariable("PATH", oldPath); } } @@ -380,8 +412,8 @@ public void GetChangedFilesFromCommit_FailsWhenCapturedOutputExceedsLimit() Directory.CreateDirectory(fakeGitDir); WriteFakeGitThatExceedsStdoutLimit(fakeGitDir); - var oldPath = Environment.GetEnvironmentVariable("PATH"); - Environment.SetEnvironmentVariable("PATH", fakeGitDir + Path.PathSeparator + oldPath); + var oldGitExecutablePath = GitHelper.GitExecutablePathOverride; + GitHelper.GitExecutablePathOverride = Path.Combine(fakeGitDir, "git"); try { var ex = Assert.Throws( @@ -391,7 +423,7 @@ public void GetChangedFilesFromCommit_FailsWhenCapturedOutputExceedsLimit() } finally { - Environment.SetEnvironmentVariable("PATH", oldPath); + GitHelper.GitExecutablePathOverride = oldGitExecutablePath; } } @@ -407,8 +439,8 @@ public void GetChangedFilesFromCommit_FailsWhenNewlineFreeStdoutExceedsLimit_Iss Directory.CreateDirectory(fakeGitDir); WriteFakeGitThatExceedsStdoutLimitWithoutNewlines(fakeGitDir); - var oldPath = Environment.GetEnvironmentVariable("PATH"); - Environment.SetEnvironmentVariable("PATH", fakeGitDir + Path.PathSeparator + oldPath); + var oldGitExecutablePath = GitHelper.GitExecutablePathOverride; + GitHelper.GitExecutablePathOverride = Path.Combine(fakeGitDir, "git"); try { var ex = Assert.Throws( @@ -418,7 +450,7 @@ public void GetChangedFilesFromCommit_FailsWhenNewlineFreeStdoutExceedsLimit_Iss } finally { - Environment.SetEnvironmentVariable("PATH", oldPath); + GitHelper.GitExecutablePathOverride = oldGitExecutablePath; } } @@ -434,9 +466,9 @@ public void GetChangedFilesFromCommit_FailsWhenGitCommandTimesOut() Directory.CreateDirectory(fakeGitDir); WriteFakeGitThatHangsOnDiffTree(fakeGitDir); - var oldPath = Environment.GetEnvironmentVariable("PATH"); + var oldGitExecutablePath = GitHelper.GitExecutablePathOverride; var oldTimeout = GitHelper.GitCommandTimeout; - Environment.SetEnvironmentVariable("PATH", fakeGitDir + Path.PathSeparator + oldPath); + GitHelper.GitExecutablePathOverride = Path.Combine(fakeGitDir, "git"); GitHelper.GitCommandTimeout = TimeSpan.FromSeconds(1); try { @@ -448,7 +480,7 @@ public void GetChangedFilesFromCommit_FailsWhenGitCommandTimesOut() finally { GitHelper.GitCommandTimeout = oldTimeout; - Environment.SetEnvironmentVariable("PATH", oldPath); + GitHelper.GitExecutablePathOverride = oldGitExecutablePath; } } @@ -464,8 +496,8 @@ public void GetChangedFilesFromCommit_CancelDuringGitCommand_ThrowsOperationCanc Directory.CreateDirectory(fakeGitDir); WriteFakeGitThatHangsOnDiffTree(fakeGitDir); - var oldPath = Environment.GetEnvironmentVariable("PATH"); - Environment.SetEnvironmentVariable("PATH", fakeGitDir + Path.PathSeparator + oldPath); + var oldGitExecutablePath = GitHelper.GitExecutablePathOverride; + GitHelper.GitExecutablePathOverride = Path.Combine(fakeGitDir, "git"); try { using var cts = new CancellationTokenSource(); @@ -481,7 +513,7 @@ public void GetChangedFilesFromCommit_CancelDuringGitCommand_ThrowsOperationCanc } finally { - Environment.SetEnvironmentVariable("PATH", oldPath); + GitHelper.GitExecutablePathOverride = oldGitExecutablePath; } } @@ -497,8 +529,8 @@ public void ResolveIgnoreCase_CancelDuringGitCommand_ThrowsOperationCanceled() Directory.CreateDirectory(fakeGitDir); WriteFakeGitThatHangsOnRevParse(fakeGitDir); - var oldPath = Environment.GetEnvironmentVariable("PATH"); - Environment.SetEnvironmentVariable("PATH", fakeGitDir + Path.PathSeparator + oldPath); + var oldGitExecutablePath = GitHelper.GitExecutablePathOverride; + GitHelper.GitExecutablePathOverride = Path.Combine(fakeGitDir, "git"); try { using var cts = new CancellationTokenSource(); @@ -514,10 +546,51 @@ public void ResolveIgnoreCase_CancelDuringGitCommand_ThrowsOperationCanceled() } finally { - Environment.SetEnvironmentVariable("PATH", oldPath); + GitHelper.GitExecutablePathOverride = oldGitExecutablePath; } } + [Fact] + public void TrustedGitExecutableCandidates_OnMacOS_ExcludeDeveloperToolsShim_Issue3433() + { + if (!OperatingSystem.IsMacOS()) + return; + + var candidates = GitHelper.TrustedGitExecutableCandidatePathsForTests(); + + Assert.DoesNotContain("/usr/bin/git", candidates); + Assert.Contains("/Library/Developer/CommandLineTools/usr/bin/git", candidates); + Assert.Contains("/Applications/Xcode.app/Contents/Developer/usr/bin/git", candidates); + } + + [Fact] + public void TryGetHeadCommitResult_OnMacOS_DoesNotUseDeveloperDirShimGit_Issue3433() + { + if (!OperatingSystem.IsMacOS()) + return; + + var projectDir = Path.Combine(_tempDir, "developer-dir-project"); + Directory.CreateDirectory(projectDir); + var developerDir = Path.Combine(_tempDir, "FakeDeveloper"); + var developerGitDir = Path.Combine(developerDir, "usr", "bin"); + Directory.CreateDirectory(developerGitDir); + var markerPath = Path.Combine(_tempDir, "developer-dir-git-ran.txt"); + var fakeGitPath = Path.Combine(developerGitDir, "git"); + File.WriteAllText(fakeGitPath, $""" +#!/bin/sh +printf ran > "{markerPath.Replace("\"", "\\\"", StringComparison.Ordinal)}" +exit 7 +"""); + File.SetUnixFileMode(fakeGitPath, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + + using var env = EnvironmentVariableScope.Capture("DEVELOPER_DIR"); + env.Set("DEVELOPER_DIR", developerDir); + + _ = GitHelper.TryGetHeadCommitResult(projectDir); + + Assert.False(File.Exists(markerPath), "GitHelper must not execute git selected through DEVELOPER_DIR."); + } + [Theory] [InlineData("feature")] [InlineData("v1.0.0")] @@ -970,6 +1043,30 @@ exit 1 File.SetUnixFileMode(script, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); } + private static void WriteFakeGitThatReturnsChangedFile(string directory, string changedPath) + { + var script = Path.Combine(directory, "git"); + File.WriteAllText(script, """ +#!/bin/sh +if [ "$1" = "rev-parse" ]; then + if [ "$2" = "--symbolic-full-name" ]; then + exit 0 + fi + if [ "$2" = "--verify" ]; then + printf '%s\n' '0123456789abcdef0123456789abcdef01234567' + exit 0 + fi +fi +if [ "$1" = "diff-tree" ]; then + printf 'M\t__CHANGED_PATH__\n' + exit 0 +fi +exit 1 +""".Replace("__CHANGED_PATH__", changedPath, StringComparison.Ordinal)); + if (!OperatingSystem.IsWindows()) + File.SetUnixFileMode(script, UnixFileMode.UserRead | UnixFileMode.UserWrite | UnixFileMode.UserExecute); + } + private static void WriteFakeGitThatExceedsStdoutLimit(string directory) { var script = Path.Combine(directory, "git"); diff --git a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs index eb0502313c..eaac4a9950 100644 --- a/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs +++ b/tests/CodeIndex.Tests/IndexCommandRunnerTests.cs @@ -431,27 +431,116 @@ public void SymbolExtractionWorker_StartInfo_UsesCurrentCdidxExecutableWhenAvail } [Fact] - public void SymbolExtractionWorker_StartInfo_UsesFrameworkDependentDllWhenProcessIsNotCdidx() + public void SymbolExtractionWorker_StartInfo_UsesFrameworkDependentDllWithTrustedDotnetHost() + { + var currentProcessPath = CreateTemporaryDotnetHostPath(); + var runnerAssemblyPath = Path.Combine(Path.GetTempPath(), "cdidx.dll"); + + try + { + var created = SymbolExtractionWorker.TryCreateStartInfo( + currentProcessPath, + runnerAssemblyPath, + out var startInfo, + out var error); + + Assert.True(created, error); + Assert.Equal(currentProcessPath, startInfo.FileName); + Assert.Equal( + [ + runnerAssemblyPath, + SymbolExtractionWorker.CommandName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); + } + finally + { + DeleteTemporaryDotnetHostPath(currentProcessPath); + } + } + + [Fact] + public void SymbolExtractionWorker_StartInfo_UsesTrustedDotnetCandidateWhenCurrentProcessIsTestHost_Issue3455() { var currentProcessPath = Path.Combine(Path.GetTempPath(), OperatingSystem.IsWindows() ? "testhost.exe" : "testhost"); + var trustedDotnetPath = CreateTemporaryDotnetHostPath(); var runnerAssemblyPath = Path.Combine(Path.GetTempPath(), "cdidx.dll"); + var originalCandidatesOverride = DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride; - var created = SymbolExtractionWorker.TryCreateStartInfo( - currentProcessPath, - runnerAssemblyPath, - out var startInfo, - out var error); + try + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = [trustedDotnetPath]; - Assert.True(created, error); - Assert.NotEqual(currentProcessPath, startInfo.FileName); - Assert.Equal( - [ + var created = SymbolExtractionWorker.TryCreateStartInfo( + currentProcessPath, runnerAssemblyPath, - SymbolExtractionWorker.CommandName, - "--protocol-max-line-bytes", - WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), - ], - startInfo.ArgumentList); + out var startInfo, + out var error); + + Assert.True(created, error); + Assert.Equal(trustedDotnetPath, startInfo.FileName); + Assert.Equal( + [ + runnerAssemblyPath, + SymbolExtractionWorker.CommandName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); + } + finally + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = originalCandidatesOverride; + DeleteTemporaryDotnetHostPath(trustedDotnetPath); + } + } + + [Fact] + public void SymbolExtractionWorker_StartInfo_FailsWithoutTrustedDotnetHost_Issue3455() + { + var currentProcessPath = Path.Combine(Path.GetTempPath(), OperatingSystem.IsWindows() ? "testhost.exe" : "testhost"); + var runnerAssemblyPath = Path.Combine(Path.GetTempPath(), "cdidx.dll"); + var originalCandidatesOverride = DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride; + + try + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = []; + + var created = SymbolExtractionWorker.TryCreateStartInfo( + currentProcessPath, + runnerAssemblyPath, + out _, + out var error); + + Assert.False(created); + Assert.Contains("trusted dotnet host path", error); + } + finally + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = originalCandidatesOverride; + } + } + + [Fact] + public void DotnetHostPathResolver_RejectsMissingDotnetHost_Issue3455() + { + var missingDotnetPath = Path.Combine(Path.GetTempPath(), $"cdidx_missing_dotnet_{Guid.NewGuid():N}", OperatingSystem.IsWindows() ? "dotnet.exe" : "dotnet"); + var originalCandidatesOverride = DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride; + + try + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = []; + + var resolved = DotnetHostPathResolver.Resolve(missingDotnetPath); + + Assert.Null(resolved); + } + finally + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = originalCandidatesOverride; + } } [Fact] @@ -512,34 +601,115 @@ public void PostExtractionHookCallbackWorker_StartInfo_UsesCurrentCdidxExecutabl } [Fact] - public void PostExtractionHookCallbackWorker_StartInfo_UsesFrameworkDependentDllWhenProcessIsNotCdidx() + public void PostExtractionHookCallbackWorker_StartInfo_UsesFrameworkDependentDllWithTrustedDotnetHost() + { + var hook = new PostExtractionHookInfo( + "demo", + Path.Combine(Path.GetTempPath(), "demo-hook.dll"), + "Demo.Hook"); + var currentProcessPath = CreateTemporaryDotnetHostPath(); + var runnerAssemblyPath = Path.Combine(Path.GetTempPath(), "cdidx.dll"); + + try + { + var created = PostExtractionHookCallbackWorker.TryCreateStartInfo( + hook, + currentProcessPath, + runnerAssemblyPath, + out var startInfo, + out var error); + + Assert.True(created, error); + Assert.Equal(currentProcessPath, startInfo.FileName); + Assert.Equal( + [ + runnerAssemblyPath, + PostExtractionHookCallbackWorker.CommandName, + hook.AssemblyPath, + hook.TypeName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); + } + finally + { + DeleteTemporaryDotnetHostPath(currentProcessPath); + } + } + + [Fact] + public void PostExtractionHookCallbackWorker_StartInfo_UsesTrustedDotnetCandidateWhenCurrentProcessIsTestHost_Issue3455() { var hook = new PostExtractionHookInfo( "demo", Path.Combine(Path.GetTempPath(), "demo-hook.dll"), "Demo.Hook"); var currentProcessPath = Path.Combine(Path.GetTempPath(), OperatingSystem.IsWindows() ? "testhost.exe" : "testhost"); + var trustedDotnetPath = CreateTemporaryDotnetHostPath(); var runnerAssemblyPath = Path.Combine(Path.GetTempPath(), "cdidx.dll"); + var originalCandidatesOverride = DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride; - var created = PostExtractionHookCallbackWorker.TryCreateStartInfo( - hook, - currentProcessPath, - runnerAssemblyPath, - out var startInfo, - out var error); + try + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = [trustedDotnetPath]; - Assert.True(created, error); - Assert.NotEqual(currentProcessPath, startInfo.FileName); - Assert.Equal( - [ + var created = PostExtractionHookCallbackWorker.TryCreateStartInfo( + hook, + currentProcessPath, runnerAssemblyPath, - PostExtractionHookCallbackWorker.CommandName, - hook.AssemblyPath, - hook.TypeName, - "--protocol-max-line-bytes", - WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), - ], - startInfo.ArgumentList); + out var startInfo, + out var error); + + Assert.True(created, error); + Assert.Equal(trustedDotnetPath, startInfo.FileName); + Assert.Equal( + [ + runnerAssemblyPath, + PostExtractionHookCallbackWorker.CommandName, + hook.AssemblyPath, + hook.TypeName, + "--protocol-max-line-bytes", + WorkerProtocolLineLimits.MaxLineUtf8Bytes.ToString(CultureInfo.InvariantCulture), + ], + startInfo.ArgumentList); + } + finally + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = originalCandidatesOverride; + DeleteTemporaryDotnetHostPath(trustedDotnetPath); + } + } + + [Fact] + public void PostExtractionHookCallbackWorker_StartInfo_FailsWithoutTrustedDotnetHost_Issue3455() + { + var hook = new PostExtractionHookInfo( + "demo", + Path.Combine(Path.GetTempPath(), "demo-hook.dll"), + "Demo.Hook"); + var currentProcessPath = Path.Combine(Path.GetTempPath(), OperatingSystem.IsWindows() ? "testhost.exe" : "testhost"); + var runnerAssemblyPath = Path.Combine(Path.GetTempPath(), "cdidx.dll"); + var originalCandidatesOverride = DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride; + + try + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = []; + + var created = PostExtractionHookCallbackWorker.TryCreateStartInfo( + hook, + currentProcessPath, + runnerAssemblyPath, + out _, + out var error); + + Assert.False(created); + Assert.Contains("trusted dotnet host path", error); + } + finally + { + DotnetHostPathResolver.TrustedDotnetHostCandidatesOverride = originalCandidatesOverride; + } } [Fact] @@ -11008,6 +11178,22 @@ private static string CreateTempProject() return projectRoot; } + private static string CreateTemporaryDotnetHostPath() + { + var hostDir = Path.Combine(Path.GetTempPath(), $"cdidx_dotnet_host_{Guid.NewGuid():N}"); + Directory.CreateDirectory(hostDir); + var hostPath = Path.Combine(hostDir, OperatingSystem.IsWindows() ? "dotnet.exe" : "dotnet"); + File.WriteAllText(hostPath, string.Empty); + return hostPath; + } + + private static void DeleteTemporaryDotnetHostPath(string hostPath) + { + var hostDir = Path.GetDirectoryName(hostPath); + if (!string.IsNullOrWhiteSpace(hostDir) && Directory.Exists(hostDir)) + Directory.Delete(hostDir, recursive: true); + } + private static int CountOccurrences(string text, string value) { if (string.IsNullOrEmpty(value))