fix: address review issues from formal review of all 18 review-sets

Agent-Logs-Url: https://github.com/demaconsulting/FileAssert/sessions/d0aab0b1-f98b-471e-b038-8888d3cb00f5

Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com>

Author: Copilot

docs/design/file-assert/utilities/path-helpers.md

@@ -20,7 +20,7 @@ the base directory.
 **Validation steps:**
 
 1. Reject null inputs via `ArgumentNullException.ThrowIfNull`.
-2. Reject `relativePath` values that contain `..` (path traversal).
+2. Reject `relativePath` values that contain `..` as a path component (path traversal).
 3. Reject `relativePath` values that are rooted (absolute paths).
 4. Combine the paths with `Path.Combine`.
 5. Compute the full (canonical) paths of both base and combined paths.

docs/reqstream/file-assert/configuration/configuration.yaml

@@ -28,3 +28,4 @@ sections:
           that filtering works correctly across the full load-and-run pipeline.
         tests:
           - ConfigurationSubsystem_RunWithFilter_ExecutesOnlyMatchingTests
+          - ConfigurationSubsystem_RunWithTagFilter_ExecutesOnlyMatchingTests

docs/reqstream/file-assert/modeling/file-assert-file.yaml

@@ -19,6 +19,7 @@ sections:
           - FileAssertFile_Create_ValidData_CreatesFile
           - FileAssertFile_Create_NullData_ThrowsArgumentNullException
           - FileAssertFile_Create_NullPattern_ThrowsInvalidOperationException
+          - FileAssertFile_Create_BlankPattern_ThrowsInvalidOperationException
 
       - id: FileAssert-FileAssertFile-CountConstraints
         title: The FileAssertFile class shall enforce optional minimum and maximum file count constraints.
@@ -42,6 +43,7 @@ sections:
         tests:
           - FileAssertFile_Run_WithContentRule_ContentContainsValue_NoError
           - FileAssertFile_Run_WithContentRule_ContentMissingValue_WritesError
+          - FileAssertFile_Run_MultipleFiles_OneFailsContentRule_WritesError
 
       - id: FileAssert-FileAssertFile-ExactCount
         title: The FileAssertFile class shall enforce an exact file count constraint when declared.
@@ -66,3 +68,4 @@ sections:
         tests:
           - FileAssertFile_Run_TooSmall_WritesError
           - FileAssertFile_Run_TooLarge_WritesError
+          - FileAssertFile_Run_MultipleFiles_OneViolatesSizeConstraint_WritesError

docs/reqstream/file-assert/selftest/selftest.yaml

@@ -17,3 +17,13 @@ sections:
           output all work together correctly.
         tests:
           - SelfTestSubsystem_Run_ExecutesBuiltInTestsAndProducesSummary
+
+      - id: FileAssert-SelfTestSubsystem-ResultsSerialization
+        title: The SelfTest subsystem shall write validation results to a file in TRX or JUnit XML format when requested.
+        justification: |
+          CI/CD pipelines and regulated environments require machine-readable test result files
+          to collect evidence that self-validation passed. Supporting both TRX and JUnit XML
+          formats ensures compatibility with common test reporting tools.
+        tests:
+          - Validation_Run_WithTrxResultsFile_WritesTrxFile
+          - Validation_Run_WithXmlResultsFile_WritesXmlFile

docs/reqstream/file-assert/utilities/path-helpers.yaml

@@ -22,6 +22,7 @@ sections:
           - PathHelpers_SafePathCombine_NestedPaths_CombinesCorrectly
           - PathHelpers_SafePathCombine_CurrentDirectoryReference_CombinesCorrectly
           - PathHelpers_SafePathCombine_EmptyRelativePath_ReturnsBasePath
+          - PathHelpers_SafePathCombine_DoubleDotInFilename_CombinesCorrectly
 
       - id: FileAssert-PathHelpers-NullValidation
         title: The PathHelpers class shall reject null base or relative path arguments.

src/DemaConsulting.FileAssert/Utilities/PathHelpers.cs

@@ -38,8 +38,13 @@ internal static string SafePathCombine(string basePath, string relativePath)
         ArgumentNullException.ThrowIfNull(basePath);
         ArgumentNullException.ThrowIfNull(relativePath);
 
-        // Ensure the relative path doesn't contain path traversal sequences
-        if (relativePath.Contains("..") || Path.IsPathRooted(relativePath))
+        // Ensure the relative path doesn't contain path traversal sequences.
+        // Split by directory separators and check each component to avoid false positives
+        // for filenames that contain ".." as a substring (e.g. "my..file.txt").
+        var pathComponents = relativePath.Split(
+            [Path.DirectorySeparatorChar, Path.AltDirectorySeparatorChar],
+            StringSplitOptions.None);
+        if (pathComponents.Any(c => c == "..") || Path.IsPathRooted(relativePath))
         {
             throw new ArgumentException($"Invalid path component: {relativePath}", nameof(relativePath));
         }

test/DemaConsulting.FileAssert.Tests/Configuration/ConfigurationSubsystemTests.cs

@@ -115,4 +115,50 @@ public void ConfigurationSubsystem_RunWithFilter_ExecutesOnlyMatchingTests()
             tempDir.Delete(recursive: true);
         }
     }
+
+    /// <summary>
+    ///     Verifies that the Configuration subsystem executes only tests whose tag matches
+    ///     the provided filter when running a configuration with multiple tests.
+    /// </summary>
+    [TestMethod]
+    public void ConfigurationSubsystem_RunWithTagFilter_ExecutesOnlyMatchingTests()
+    {
+        // Arrange - two tests with different tags; only one file exists so only that test passes
+        var tempDir = Directory.CreateTempSubdirectory("fileassert_config_");
+        try
+        {
+            var configPath = Path.Combine(tempDir.FullName, "config.yaml");
+            File.WriteAllText(configPath, """
+                tests:
+                  - name: "Alpha"
+                    tags:
+                      - smoke
+                    files:
+                      - pattern: "alpha.txt"
+                        min: 1
+                  - name: "Beta"
+                    tags:
+                      - regression
+                    files:
+                      - pattern: "beta.txt"
+                        min: 1
+                """);
+
+            // Create only alpha.txt so the Alpha test passes and Beta would fail
+            File.WriteAllText(Path.Combine(tempDir.FullName, "alpha.txt"), "content");
+
+            var config = FileAssertConfig.ReadFromFile(configPath);
+            using var context = Context.Create(["--silent"]);
+
+            // Act - run with the "smoke" tag filter only
+            config.Run(context, ["smoke"]);
+
+            // Assert - no errors because only Alpha ran (matching the smoke tag) and alpha.txt exists
+            Assert.AreEqual(0, context.ExitCode);
+        }
+        finally
+        {
+            tempDir.Delete(recursive: true);
+        }
+    }
 }

test/DemaConsulting.FileAssert.Tests/IntegrationTests.cs

@@ -148,15 +148,14 @@ public void IntegrationTest_SilentFlag_SuppressesOutput()
     {
         // Act
         var exitCode = Runner.Run(
-            out var _,
+            out var output,
             "dotnet",
             _dllPath,
             "--silent");
 
         // Assert
         Assert.AreEqual(0, exitCode);
-
-        // Output check removed since silent mode may still produce some output
+        Assert.AreEqual(string.Empty, output);
     }
 
     /// <summary>

test/DemaConsulting.FileAssert.Tests/Modeling/FileAssertFileTests.cs

@@ -73,6 +73,20 @@ public void FileAssertFile_Create_NullPattern_ThrowsInvalidOperationException()
         Assert.Contains("pattern", exception.Message);
     }
 
+    /// <summary>
+    ///     Verifies that Create throws <see cref="InvalidOperationException"/> when Pattern is blank.
+    /// </summary>
+    [TestMethod]
+    public void FileAssertFile_Create_BlankPattern_ThrowsInvalidOperationException()
+    {
+        // Arrange
+        var data = new FileAssertFileData { Pattern = "   " };
+
+        // Act & Assert
+        var exception = Assert.Throws<InvalidOperationException>(() => FileAssertFile.Create(data));
+        Assert.Contains("pattern", exception.Message);
+    }
+
     /// <summary>
     ///     Verifies that Run produces no error when there are no matching files and no constraints.
     /// </summary>
@@ -323,4 +337,64 @@ public void FileAssertFile_Run_TooLarge_WritesError()
             tempDir.Delete(recursive: true);
         }
     }
+
+    /// <summary>
+    ///     Verifies that Run checks size constraints against every matched file, not just the first.
+    /// </summary>
+    [TestMethod]
+    public void FileAssertFile_Run_MultipleFiles_OneViolatesSizeConstraint_WritesError()
+    {
+        // Arrange - create two files: one large enough, one too small
+        var tempDir = Directory.CreateTempSubdirectory("fileassert_test_");
+        try
+        {
+            File.WriteAllText(Path.Combine(tempDir.FullName, "ok.txt"), "enough content here");
+            File.WriteAllText(Path.Combine(tempDir.FullName, "small.txt"), string.Empty);
+            var data = new FileAssertFileData { Pattern = "*.txt", MinSize = 5 };
+            var file = FileAssertFile.Create(data);
+            using var context = Context.Create(["--silent"]);
+
+            // Act
+            file.Run(context, tempDir.FullName);
+
+            // Assert - the small file should trigger an error
+            Assert.AreEqual(1, context.ExitCode);
+        }
+        finally
+        {
+            tempDir.Delete(recursive: true);
+        }
+    }
+
+    /// <summary>
+    ///     Verifies that Run applies content rules to every matched file, not just the first.
+    /// </summary>
+    [TestMethod]
+    public void FileAssertFile_Run_MultipleFiles_OneFailsContentRule_WritesError()
+    {
+        // Arrange - create two files: one with required content, one without
+        var tempDir = Directory.CreateTempSubdirectory("fileassert_test_");
+        try
+        {
+            File.WriteAllText(Path.Combine(tempDir.FullName, "good.txt"), "expected content here");
+            File.WriteAllText(Path.Combine(tempDir.FullName, "bad.txt"), "unrelated content");
+            var data = new FileAssertFileData
+            {
+                Pattern = "*.txt",
+                Rules = [new FileAssertRuleData { Contains = "expected content" }]
+            };
+            var file = FileAssertFile.Create(data);
+            using var context = Context.Create(["--silent"]);
+
+            // Act
+            file.Run(context, tempDir.FullName);
+
+            // Assert - the bad file should trigger an error
+            Assert.AreEqual(1, context.ExitCode);
+        }
+        finally
+        {
+            tempDir.Delete(recursive: true);
+        }
+    }
 }

test/DemaConsulting.FileAssert.Tests/Utilities/PathHelpersTests.cs

@@ -152,6 +152,23 @@ public void PathHelpers_SafePathCombine_EmptyRelativePath_ReturnsBasePath()
         Assert.AreEqual(Path.Combine(basePath, relativePath), result);
     }
 
+    /// <summary>
+    ///     Test that SafePathCombine allows filenames that contain ".." as a substring but not as a path component.
+    /// </summary>
+    [TestMethod]
+    public void PathHelpers_SafePathCombine_DoubleDotInFilename_CombinesCorrectly()
+    {
+        // Arrange - filename with ".." as substring, not a path traversal component
+        var basePath = "/home/user/project";
+        var relativePath = "my..file.txt";
+
+        // Act
+        var result = PathHelpers.SafePathCombine(basePath, relativePath);
+
+        // Assert
+        Assert.AreEqual(Path.Combine(basePath, relativePath), result);
+    }
+
     /// <summary>
     ///     Test that SafePathCombine throws ArgumentNullException when base path is null.
     /// </summary>