Fix Path.Combine security warnings and MSTest assertion code smells

.github/codeql-config.yml

@@ -11,3 +11,8 @@ query-filters:
     paths:
       - src/DemaConsulting.TemplateDotNetTool/Context.cs
       - src/DemaConsulting.TemplateDotNetTool/Validation.cs
+  # Exclude path-combine warnings in PathHelpers (validated safe usage)
+  - exclude:
+      id: cs/path-combine
+    paths:
+      - src/DemaConsulting.TemplateDotNetTool/PathHelpers.cs

src/DemaConsulting.TemplateDotNetTool/PathHelpers.cs

@@ -0,0 +1,60 @@
+// Copyright (c) DEMA Consulting
+// 
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+// 
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+
+namespace DemaConsulting.TemplateDotNetTool;
+
+/// <summary>
+///     Helper utilities for safe path operations.
+/// </summary>
+internal static class PathHelpers
+{
+    /// <summary>
+    ///     Safely combines two paths, ensuring the second path doesn't contain path traversal sequences.
+    /// </summary>
+    /// <param name="basePath">The base path.</param>
+    /// <param name="relativePath">The relative path to combine.</param>
+    /// <returns>The combined path.</returns>
+    /// <exception cref="ArgumentException">Thrown when relativePath contains invalid characters or path traversal sequences.</exception>
+    internal static string SafePathCombine(string basePath, string relativePath)
+    {
+        // Ensure the relative path doesn't contain path traversal sequences
+        if (relativePath.Contains("..") || Path.IsPathRooted(relativePath))
+        {
+            throw new ArgumentException($"Invalid path component: {relativePath}", nameof(relativePath));
+        }
+
+        // This call to Path.Combine is safe because we've validated that:
+        // 1. relativePath doesn't contain ".." (path traversal)
+        // 2. relativePath is not an absolute path (IsPathRooted check)
+        // This ensures the combined path will always be under basePath
+        var combinedPath = Path.Combine(basePath, relativePath);
+
+        // Additional validation: ensure the combined path is still under the base path
+        var fullBasePath = Path.GetFullPath(basePath);
+        var fullCombinedPath = Path.GetFullPath(combinedPath);
+
+        if (!fullCombinedPath.StartsWith(fullBasePath, StringComparison.OrdinalIgnoreCase))
+        {
+            throw new ArgumentException($"Invalid path component: {relativePath}", nameof(relativePath));
+        }
+
+        return combinedPath;
+    }
+}

src/DemaConsulting.TemplateDotNetTool/Validation.cs

@@ -103,7 +103,7 @@ private static void RunVersionTest(Context context, DemaConsulting.TestResults.T
         try
         {
             using var tempDir = new TemporaryDirectory();
-            var logFile = Path.Combine(tempDir.DirectoryPath, "version-test.log");
+            var logFile = PathHelpers.SafePathCombine(tempDir.DirectoryPath, "version-test.log");
 
             // Build command line arguments
             var args = new List<string>
@@ -171,7 +171,7 @@ private static void RunHelpTest(Context context, DemaConsulting.TestResults.Test
         try
         {
             using var tempDir = new TemporaryDirectory();
-            var logFile = Path.Combine(tempDir.DirectoryPath, "help-test.log");
+            var logFile = PathHelpers.SafePathCombine(tempDir.DirectoryPath, "help-test.log");
 
             // Build command line arguments
             var args = new List<string>
@@ -329,7 +329,7 @@ private sealed class TemporaryDirectory : IDisposable
         /// </summary>
         public TemporaryDirectory()
         {
-            DirectoryPath = Path.Combine(Path.GetTempPath(), $"templatetool_validation_{Guid.NewGuid()}");
+            DirectoryPath = PathHelpers.SafePathCombine(Path.GetTempPath(), $"templatetool_validation_{Guid.NewGuid()}");
 
             try
             {

test/DemaConsulting.TemplateDotNetTool.Tests/ContextTests.cs

@@ -160,7 +160,7 @@ public void Context_Create_LogFlag_OpensLogFile()
             // Verify log file was written
             Assert.IsTrue(File.Exists(logFile));
             var logContent = File.ReadAllText(logFile);
-            Assert.IsTrue(logContent.Contains("Test message"));
+            Assert.Contains("Test message", logContent);
         }
         finally
         {
@@ -178,7 +178,7 @@ public void Context_Create_LogFlag_OpensLogFile()
     public void Context_Create_UnknownArgument_ThrowsArgumentException()
     {
         var exception = Assert.Throws<ArgumentException>(() => Context.Create(["--unknown"]));
-        Assert.IsTrue(exception.Message.Contains("Unsupported argument"));
+        Assert.Contains("Unsupported argument", exception.Message);
     }
 
     /// <summary>
@@ -197,7 +197,7 @@ public void Context_WriteLine_NotSilent_WritesToConsole()
             context.WriteLine("Test message");
 
             var output = outWriter.ToString();
-            Assert.IsTrue(output.Contains("Test message"));
+            Assert.Contains("Test message", output);
         }
         finally
         {
@@ -221,7 +221,7 @@ public void Context_WriteLine_Silent_DoesNotWriteToConsole()
             context.WriteLine("Test message");
 
             var output = outWriter.ToString();
-            Assert.IsFalse(output.Contains("Test message"));
+            Assert.DoesNotContain("Test message", output);
         }
         finally
         {

test/DemaConsulting.TemplateDotNetTool.Tests/IntegrationTests.cs

@@ -37,7 +37,7 @@ public void TestInitialize()
         // The DLL should be in the same directory as the test assembly
         // because the test project references the main project
         var baseDir = AppContext.BaseDirectory;
-        _dllPath = Path.Combine(baseDir, "DemaConsulting.TemplateDotNetTool.dll");
+        _dllPath = PathHelpers.SafePathCombine(baseDir, "DemaConsulting.TemplateDotNetTool.dll");
 
         Assert.IsTrue(File.Exists(_dllPath), $"Could not find Template DotNet Tool DLL at {_dllPath}");
     }
@@ -60,8 +60,8 @@ public void IntegrationTest_VersionFlag_OutputsVersion()
 
         // Verify version is output
         Assert.IsFalse(string.IsNullOrWhiteSpace(output));
-        Assert.IsFalse(output.Contains("Error"));
-        Assert.IsFalse(output.Contains("Copyright"));
+        Assert.DoesNotContain("Error", output);
+        Assert.DoesNotContain("Copyright", output);
     }
 
     /// <summary>
@@ -81,9 +81,9 @@ public void IntegrationTest_HelpFlag_OutputsUsageInformation()
         Assert.AreEqual(0, exitCode);
 
         // Verify usage information is output
-        Assert.IsTrue(output.Contains("Usage:"));
-        Assert.IsTrue(output.Contains("Options:"));
-        Assert.IsTrue(output.Contains("--version"));
+        Assert.Contains("Usage:", output);
+        Assert.Contains("Options:", output);
+        Assert.Contains("--version", output);
     }
 
     /// <summary>
@@ -103,8 +103,8 @@ public void IntegrationTest_ValidateFlag_RunsValidation()
         Assert.AreEqual(0, exitCode);
 
         // Verify validation output
-        Assert.IsTrue(output.Contains("Total Tests:"));
-        Assert.IsTrue(output.Contains("Passed:"));
+        Assert.Contains("Total Tests:", output);
+        Assert.Contains("Passed:", output);
     }
 
     /// <summary>
@@ -135,8 +135,8 @@ public void IntegrationTest_ValidateWithResults_GeneratesTrxFile()
 
             // Verify TRX file contains expected content
             var trxContent = File.ReadAllText(resultsFile);
-            Assert.IsTrue(trxContent.Contains("<TestRun"));
-            Assert.IsTrue(trxContent.Contains("</TestRun>"));
+            Assert.Contains("<TestRun", trxContent);
+            Assert.Contains("</TestRun>", trxContent);
         }
         finally
         {
@@ -192,7 +192,7 @@ public void IntegrationTest_LogFlag_WritesOutputToFile()
 
             // Verify log file contains output
             var logContent = File.ReadAllText(logFile);
-            Assert.IsTrue(logContent.Contains("Template DotNet Tool version"));
+            Assert.Contains("Template DotNet Tool version", logContent);
         }
         finally
         {
@@ -218,6 +218,6 @@ public void IntegrationTest_UnknownArgument_ReturnsError()
 
         // Verify error
         Assert.AreNotEqual(0, exitCode);
-        Assert.IsTrue(output.Contains("Error"));
+        Assert.Contains("Error", output);
     }
 }

test/DemaConsulting.TemplateDotNetTool.Tests/PathHelpersTests.cs

@@ -0,0 +1,101 @@
+// Copyright (c) DEMA Consulting
+// 
+// Permission is hereby granted, free of charge, to any person obtaining a copy
+// of this software and associated documentation files (the "Software"), to deal
+// in the Software without restriction, including without limitation the rights
+// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+// copies of the Software, and to permit persons to whom the Software is
+// furnished to do so, subject to the following conditions:
+// 
+// The above copyright notice and this permission notice shall be included in all
+// copies or substantial portions of the Software.
+// 
+// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+// SOFTWARE.
+
+namespace DemaConsulting.TemplateDotNetTool.Tests;
+
+/// <summary>
+///     Tests for the PathHelpers class.
+/// </summary>
+[TestClass]
+public class PathHelpersTests
+{
+    /// <summary>
+    ///     Test that SafePathCombine correctly combines valid paths.
+    /// </summary>
+    [TestMethod]
+    public void PathHelpers_SafePathCombine_ValidPaths_CombinesCorrectly()
+    {
+        // Arrange
+        var basePath = "/home/user/project";
+        var relativePath = "subfolder/file.txt";
+
+        // Act
+        var result = PathHelpers.SafePathCombine(basePath, relativePath);
+
+        // Assert
+        Assert.AreEqual(Path.Combine(basePath, relativePath), result);
+    }
+
+    /// <summary>
+    ///     Test that SafePathCombine throws ArgumentException for path traversal with double dots.
+    /// </summary>
+    [TestMethod]
+    public void PathHelpers_SafePathCombine_PathTraversalWithDoubleDots_ThrowsArgumentException()
+    {
+        // Arrange
+        var basePath = "/home/user/project";
+        var relativePath = "../etc/passwd";
+
+        // Act & Assert
+        var exception = Assert.Throws<ArgumentException>(() =>
+            PathHelpers.SafePathCombine(basePath, relativePath));
+        Assert.Contains("Invalid path component", exception.Message);
+    }
+
+    /// <summary>
+    ///     Test that SafePathCombine throws ArgumentException for path with double dots in middle.
+    /// </summary>
+    [TestMethod]
+    public void PathHelpers_SafePathCombine_DoubleDotsInMiddle_ThrowsArgumentException()
+    {
+        // Arrange
+        var basePath = "/home/user/project";
+        var relativePath = "subfolder/../../../etc/passwd";
+
+        // Act & Assert
+        var exception = Assert.Throws<ArgumentException>(() =>
+            PathHelpers.SafePathCombine(basePath, relativePath));
+        Assert.Contains("Invalid path component", exception.Message);
+    }
+
+    /// <summary>
+    ///     Test that SafePathCombine throws ArgumentException for absolute paths.
+    /// </summary>
+    [TestMethod]
+    public void PathHelpers_SafePathCombine_AbsolutePath_ThrowsArgumentException()
+    {
+        // Test Unix absolute path
+        var unixBasePath = "/home/user/project";
+        var unixRelativePath = "/etc/passwd";
+        var unixException = Assert.Throws<ArgumentException>(() =>
+            PathHelpers.SafePathCombine(unixBasePath, unixRelativePath));
+        Assert.Contains("Invalid path component", unixException.Message);
+
+        // Test Windows absolute path (only on Windows since Windows paths may not be rooted on Unix)
+        if (OperatingSystem.IsWindows())
+        {
+            var windowsBasePath = "C:\\Users\\project";
+            var windowsRelativePath = "C:\\Windows\\System32\\file.txt";
+            var windowsException = Assert.Throws<ArgumentException>(() =>
+                PathHelpers.SafePathCombine(windowsBasePath, windowsRelativePath));
+            Assert.Contains("Invalid path component", windowsException.Message);
+        }
+    }
+}

test/DemaConsulting.TemplateDotNetTool.Tests/ProgramTests.cs

@@ -42,8 +42,8 @@ public void Program_Run_WithVersionFlag_DisplaysVersionOnly()
             Program.Run(context);
 
             var output = outWriter.ToString();
-            Assert.IsFalse(output.Contains("Copyright"));
-            Assert.IsFalse(output.Contains("Template DotNet Tool version"));
+            Assert.DoesNotContain("Copyright", output);
+            Assert.DoesNotContain("Template DotNet Tool version", output);
         }
         finally
         {
@@ -67,10 +67,10 @@ public void Program_Run_WithHelpFlag_DisplaysUsageInformation()
             Program.Run(context);
 
             var output = outWriter.ToString();
-            Assert.IsTrue(output.Contains("Usage:"));
-            Assert.IsTrue(output.Contains("Options:"));
-            Assert.IsTrue(output.Contains("--version"));
-            Assert.IsTrue(output.Contains("--help"));
+            Assert.Contains("Usage:", output);
+            Assert.Contains("Options:", output);
+            Assert.Contains("--version", output);
+            Assert.Contains("--help", output);
         }
         finally
         {
@@ -94,7 +94,7 @@ public void Program_Run_WithValidateFlag_RunsValidation()
             Program.Run(context);
 
             var output = outWriter.ToString();
-            Assert.IsTrue(output.Contains("Total Tests:"));
+            Assert.Contains("Total Tests:", output);
         }
         finally
         {
@@ -118,8 +118,8 @@ public void Program_Run_NoArguments_DisplaysDefaultBehavior()
             Program.Run(context);
 
             var output = outWriter.ToString();
-            Assert.IsTrue(output.Contains("Template DotNet Tool version"));
-            Assert.IsTrue(output.Contains("Copyright"));
+            Assert.Contains("Template DotNet Tool version", output);
+            Assert.Contains("Copyright", output);
         }
         finally
         {