From 1a7ef24f8388139a8caca13c008efbc6956b3ee8 Mon Sep 17 00:00:00 2001
From: Simon Cropp <simon.cropp@gmail.com>
Date: Sat, 22 Aug 2026 13:22:37 +1000
Subject: [PATCH] Validate the path, not the parameter name

Guard.FileExists called AgainstEmpty(argumentName, path) - arguments reversed,
so the empty check ran against the literal parameter name, which is never empty.
An empty path fell straight through to ArgumentException("File not found. Path:
") with no ParamName, which names neither the argument nor the file.

The same commit stops OsSettingsResolver dereferencing a null PATH. `env -i` and
some service launchers do start a process without one, and this runs in a static
constructor, so the NullReferenceException is permanent for the process. An
empty PATH just means no tool is found that way, which is a perfectly good
answer.

No test for the PATH half: it is read once by a type initialiser, so it cannot
be exercised twice in one process.
---
 src/DiffEngine.Tests/GuardTests.cs   | 27 +++++++++++++++++++++++++++
 src/DiffEngine/Guard.cs              |  4 +++-
 src/DiffEngine/OsSettingsResolver.cs |  5 ++++-
 3 files changed, 34 insertions(+), 2 deletions(-)
 create mode 100644 src/DiffEngine.Tests/GuardTests.cs

diff --git a/src/DiffEngine.Tests/GuardTests.cs b/src/DiffEngine.Tests/GuardTests.cs
new file mode 100644
index 00000000..9b834514
--- /dev/null
+++ b/src/DiffEngine.Tests/GuardTests.cs
@@ -0,0 +1,27 @@
+public class GuardTests
+{
+    /// <summary>
+    /// FileExists passed its arguments to AgainstEmpty the wrong way round, so the empty check was
+    /// run against the literal parameter name - which is never empty. An empty path therefore fell
+    /// through to "File not found. Path: " with no ParamName on it, naming nothing at all.
+    /// </summary>
+    [Test]
+    [Arguments("")]
+    [Arguments("   ")]
+    public async Task FileExistsRejectsAnEmptyPathByName(string path)
+    {
+        var exception = await Assert.That(() => Guard.FileExists(path, "tempFile"))
+            .Throws<ArgumentNullException>();
+
+        await Assert.That(exception!.ParamName).IsEqualTo("tempFile");
+    }
+
+    [Test]
+    public async Task FileExistsStillReportsAMissingFile()
+    {
+        var missing = Path.Combine(Path.GetTempPath(), $"missing{Guid.NewGuid():N}.txt");
+
+        await Assert.That(() => Guard.FileExists(missing, "tempFile"))
+            .Throws<ArgumentException>();
+    }
+}
diff --git a/src/DiffEngine/Guard.cs b/src/DiffEngine/Guard.cs
index b6360b70..615bdf85 100644
--- a/src/DiffEngine/Guard.cs
+++ b/src/DiffEngine/Guard.cs
@@ -10,7 +10,9 @@ public static void AgainstNegative(int value, string argumentName)
 
     public static void FileExists(string path, string argumentName)
     {
-        AgainstEmpty(argumentName, path);
+        // (value, name), not (name, value). Reversed, this validated the literal parameter name -
+        // never empty - so an empty path fell through to the message below with no ParamName on it
+        AgainstEmpty(path, argumentName);
         if (!File.Exists(path))
         {
             throw new ArgumentException($"File not found. Path: {path}");
diff --git a/src/DiffEngine/OsSettingsResolver.cs b/src/DiffEngine/OsSettingsResolver.cs
index b1585ed1..8d5fb518 100644
--- a/src/DiffEngine/OsSettingsResolver.cs
+++ b/src/DiffEngine/OsSettingsResolver.cs
@@ -4,7 +4,10 @@ static class OsSettingsResolver
 
     static OsSettingsResolver()
     {
-        var pathVariable = Environment.GetEnvironmentVariable("PATH")!;
+        // An unset PATH is a NullReferenceException in a static constructor, and so permanent
+        // for the process. `env -i` and some service launchers really do start a process without
+        // one; nothing on PATH simply means no tool is found that way
+        var pathVariable = Environment.GetEnvironmentVariable("PATH") ?? "";
 
         if (RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
         {