Fix all formal review-set findings: REQTEST traceability, colour safety, doc accuracy, PathHelpers test, review-set consolidation

.reviewmark.yaml

@@ -18,14 +18,19 @@ evidence-source:
 # Review sets grouping files by logical unit of review.
 reviews:
   - id: SarifMark-CLI-Review
-    title: Review of SarifMark Command Line
+    title: Review of SarifMark Command Line and Integration Tests
     paths:
       - "docs/reqstream/command-line.yaml"
+      - "docs/reqstream/platform.yaml"
+      - "docs/reqstream/ots-software.yaml"
       - "docs/design/command-line.md"
       - "src/**/Program.cs"
       - "src/**/Context.cs"
       - "test/**/ProgramTests.cs"
       - "test/**/ContextTests.cs"
+      - "test/**/IntegrationTests.cs"
+      - "test/**/Runner.cs"
+      - "test/**/AssemblyInfo.cs"
 
   - id: SarifMark-SARIF-Review
     title: Review of SarifMark SARIF and Reporting
@@ -51,12 +56,3 @@ reviews:
       - "docs/design/utilities.md"
       - "src/**/PathHelpers.cs"
       - "test/**/PathHelpersTests.cs"
-
-  - id: SarifMark-Integration-Review
-    title: Review of SarifMark Integration Tests
-    paths:
-      - "docs/reqstream/platform.yaml"
-      - "docs/reqstream/ots-software.yaml"
-      - "test/**/IntegrationTests.cs"
-      - "test/**/Runner.cs"
-      - "test/**/AssemblyInfo.cs"

docs/design/command-line.md

@@ -83,7 +83,7 @@ arguments throw `ArgumentException`, satisfying `SarifMark-Cli-InvalidArgs`.
 
 `WriteLine` writes to `Console.Out` unless `Silent` is set, and also writes to the log
 file if one was opened. `WriteError` additionally sets `_hasErrors = true` (making
-`ExitCode` return 1) and writes to `Console.Error` in red. This satisfies
+`ExitCode` return 1) and, unless `Silent` is set, writes to `Console.Error` in red. This satisfies
 `SarifMark-Cli-Silent` and `SarifMark-Enf-ExitCode`.
 
 ### Log File

docs/design/sarif.md

@@ -60,6 +60,9 @@ The static `Read` method loads and parses a SARIF file:
 6. Delegates to `ParseResults` to iterate and parse all non-suppressed results. This
    satisfies `SarifMark-Sarif-Results` and `SarifMark-Sarif-Reading`.
 
+Together, steps 1–6 form the complete pipeline for processing a valid SARIF file, satisfying
+`SarifMark-Sarif-Processing`.
+
 ### Version Extraction
 
 `ExtractToolVersion` checks three fields in priority order: `version`,
@@ -95,3 +98,12 @@ satisfies `SarifMark-Rpt-Locations`.
 
 Each result is formatted as a single line ending with two trailing spaces (`  `), which
 forces a hard line break in rendered markdown. This satisfies `SarifMark-Rpt-LineBreaks`.
+
+## CLI Integration
+
+The requirement `SarifMark-Sarif-Required` (the tool shall require the `--sarif` parameter
+for analysis) is enforced at the command-line layer rather than within this library. The
+`ProcessSarifAnalysis` method in `Program.cs` validates that `--sarif` is provided before
+invoking the SARIF reading layer. See [command-line.md] for full details.
+
+[command-line.md]: command-line.md

docs/reqstream/validation.yaml

@@ -18,23 +18,24 @@ sections:
               Writing test results to files enables integration with CI/CD systems and provides persistent
               records of validation outcomes for tracking quality trends and compliance.
             tests:
-              - IntegrationTest_ValidateFlag_RunsSelfValidation
+              - Validation_Run_WithTrxResultsFile_WritesResultsFile
+              - Validation_Run_WithXmlResultsFile_WritesResultsFile
 
           - id: SarifMark-Val-TrxFormat
             title: The tool shall support TRX format for test results.
             justification: >-
               TRX format support enables integration with Microsoft testing ecosystems and Azure DevOps,
               providing native compatibility with Visual Studio and .NET tooling.
             tests:
-              - IntegrationTest_ValidateFlag_RunsSelfValidation
+              - Validation_Run_WithTrxResultsFile_WritesResultsFile
 
           - id: SarifMark-Val-JUnitFormat
             title: The tool shall support JUnit format for test results.
             justification: >-
               JUnit format support enables broad compatibility with CI/CD platforms like Jenkins, GitHub Actions,
               and GitLab CI, which commonly use this standard format for test reporting.
             tests:
-              - IntegrationTest_ValidateFlag_RunsSelfValidation
+              - Validation_Run_WithXmlResultsFile_WritesResultsFile
 
       - title: Quality Enforcement
         requirements:

src/DemaConsulting.SarifMark/Context.cs

@@ -365,9 +365,15 @@ public void WriteError(string message)
         if (!Silent)
         {
             var previousColor = Console.ForegroundColor;
-            Console.ForegroundColor = ConsoleColor.Red;
-            Console.Error.WriteLine(message);
-            Console.ForegroundColor = previousColor;
+            try
+            {
+                Console.ForegroundColor = ConsoleColor.Red;
+                Console.Error.WriteLine(message);
+            }
+            finally
+            {
+                Console.ForegroundColor = previousColor;
+            }
         }
 
         // Write to log file if logging is enabled

src/DemaConsulting.SarifMark/Program.cs

@@ -46,7 +46,7 @@ public static string Version
     /// </summary>
     /// <param name="args">Command-line arguments.</param>
     /// <returns>Exit code: 0 for success, non-zero for failure.</returns>
-    private static int Main(string[] args)
+    internal static int Main(string[] args)
     {
         try
         {

test/DemaConsulting.SarifMark.Tests/PathHelpersTests.cs

@@ -100,6 +100,25 @@ public void PathHelpers_SafePathCombine_PathWithDoubleDots_ThrowsArgumentExcepti
         Assert.Contains("Invalid path component", exception.Message);
     }
 
+    /// <summary>
+    ///     Test that SafePathCombine throws ArgumentException for a filename that contains ".." as a substring.
+    ///     The check uses a strict Contains("..") comparison, so names like "v1..0.sarif" are rejected
+    ///     even though they do not represent path traversal.  This is intentional: the design favours
+    ///     rejecting a small set of unusual but valid names over risking traversal edge-cases.
+    /// </summary>
+    [TestMethod]
+    public void PathHelpers_SafePathCombine_FilenameWithEmbeddedDots_ThrowsArgumentException()
+    {
+        // Arrange - "v1..0.sarif" contains ".." but is not a path traversal
+        var basePath = "/home/user";
+        var relativePath = "v1..0.sarif";
+
+        // Act & Assert - the strict substring check rejects this name
+        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>

test/DemaConsulting.SarifMark.Tests/ProgramTests.cs

@@ -18,8 +18,6 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
 // SOFTWARE.
 
-using System.Reflection;
-
 namespace DemaConsulting.SarifMark.Tests;
 
 /// <summary>
@@ -45,7 +43,7 @@ public void Program_Main_NoArguments_ReturnsError()
             Console.SetError(errWriter);
 
             // Act
-            var result = InvokeMain([]);
+            var result = Program.Main([]);
 
             // Assert
             Assert.AreEqual(1, result);
@@ -73,7 +71,7 @@ public void Program_Main_VersionFlag_DisplaysVersionOnly()
             Console.SetOut(outWriter);
 
             // Act
-            var result = InvokeMain(["--version"]);
+            var result = Program.Main(["--version"]);
 
             // Assert
             Assert.AreEqual(0, result);
@@ -101,7 +99,7 @@ public void Program_Main_HelpFlag_DisplaysHelp()
             Console.SetOut(outWriter);
 
             // Act
-            var result = InvokeMain(["--help"]);
+            var result = Program.Main(["--help"]);
 
             // Assert
             Assert.AreEqual(0, result);
@@ -130,7 +128,7 @@ public void Program_Main_UnknownArgument_ReturnsError()
             Console.SetError(errWriter);
 
             // Act
-            var result = InvokeMain(["--unknown"]);
+            var result = Program.Main(["--unknown"]);
 
             // Assert
             Assert.AreEqual(1, result);
@@ -141,17 +139,4 @@ public void Program_Main_UnknownArgument_ReturnsError()
             Console.SetError(originalError);
         }
     }
-
-    /// <summary>
-    ///     Invokes the Main method using reflection.
-    /// </summary>
-    /// <param name="args">Command-line arguments.</param>
-    /// <returns>The exit code returned by Main.</returns>
-    private static int InvokeMain(string[] args)
-    {
-        var programType = typeof(Program).Assembly.GetType("DemaConsulting.SarifMark.Program");
-        var mainMethod = programType?.GetMethod("Main", BindingFlags.Static | BindingFlags.NonPublic);
-        var result = mainMethod?.Invoke(null, [args]);
-        return result is int exitCode ? exitCode : -1;
-    }
 }

test/DemaConsulting.SarifMark.Tests/ValidationTests.cs

@@ -165,6 +165,42 @@ public void Validation_Run_WithTrxResultsFile_WritesResultsFile()
         }
     }
 
+    /// <summary>
+    ///     Tests that when a results file path with an unsupported extension is supplied, an error is reported.
+    /// </summary>
+    [TestMethod]
+    public void Validation_Run_WithUnsupportedResultsFileExtension_WritesError()
+    {
+        // Arrange - supply a .json results path (not a supported format)
+        var logFile = CreateTempFile(".log");
+        var jsonFile = CreateTempFile(".json");
+        try
+        {
+            int exitCode;
+            using (var context = Context.Create(
+                ["--silent", "--log", logFile, "--results", jsonFile]))
+            {
+                // Act
+                Validation.Run(context);
+                exitCode = context.ExitCode;
+            }
+
+            // Assert - error must be reported and no results file created
+            Assert.AreEqual(1, exitCode,
+                "Exit code should be 1 when an unsupported results file extension is used");
+            var logContent = File.ReadAllText(logFile);
+            Assert.Contains("Unsupported results file format", logContent,
+                "Log should contain the unsupported format error message");
+            Assert.IsFalse(File.Exists(jsonFile),
+                "Results file should not be created for an unsupported extension");
+        }
+        finally
+        {
+            SafeDeleteFile(logFile);
+            SafeDeleteFile(jsonFile);
+        }
+    }
+
     /// <summary>
     ///     Tests that when a .xml results file path is supplied the file is created and contains JUnit XML content.
     /// </summary>