From 14bbeb7e482afe5a8cc9f8f24ad1e77701de34b6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 00:09:45 +0000 Subject: [PATCH 01/13] Initial plan From fdfbf50f0fc3a0c605137090585d74253ba84501 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 00:35:31 +0000 Subject: [PATCH 02/13] Fix duplicate link references in docs/design/introduction.md Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- docs/design/introduction.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/docs/design/introduction.md b/docs/design/introduction.md index 644775c..271c3e9 100644 --- a/docs/design/introduction.md +++ b/docs/design/introduction.md @@ -30,8 +30,7 @@ architecture, and the requirements it satisfies — followed by **class-level do that describe each implementing class in detail. This document does not cover installation, end-user usage patterns, or the CI/CD pipeline -configuration. Those topics are addressed in the [User Guide][user-guide] and the -[Requirements document][requirements-doc]. +configuration. Those topics are addressed in the [SarifMark repository][sarifmark-repo]. ## Audience @@ -55,5 +54,4 @@ This document describes the intent and structure of that code; any discrepancy b this document and the code should be resolved by updating this document to reflect the actual implementation, or by raising a defect against the code. -[user-guide]: https://github.com/demaconsulting/SarifMark -[requirements-doc]: https://github.com/demaconsulting/SarifMark +[sarifmark-repo]: https://github.com/demaconsulting/SarifMark From 8bdcd571ef8bf1506c66f450b891965248f2e2bb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 00:42:51 +0000 Subject: [PATCH 03/13] Update introduction.md scope wording per review feedback Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- docs/design/introduction.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/design/introduction.md b/docs/design/introduction.md index 271c3e9..d5036a0 100644 --- a/docs/design/introduction.md +++ b/docs/design/introduction.md @@ -30,7 +30,7 @@ architecture, and the requirements it satisfies — followed by **class-level do that describe each implementing class in detail. This document does not cover installation, end-user usage patterns, or the CI/CD pipeline -configuration. Those topics are addressed in the [SarifMark repository][sarifmark-repo]. +configuration. Those topics are addressed in other [SarifMark repository][sarifmark-repo] documentation. ## Audience From 7d9e6cb96590634cdaaa4a5388b693d09da06947 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 01:00:06 +0000 Subject: [PATCH 04/13] Fix all 5 review findings from reviewmark review-sets Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- docs/design/validation.md | 20 ++- src/DemaConsulting.SarifMark/SarifResult.cs | 2 +- src/DemaConsulting.SarifMark/SarifResults.cs | 2 +- src/DemaConsulting.SarifMark/Validation.cs | 131 +++++++----------- .../IntegrationTests.cs | 4 +- .../ProgramTests.cs | 1 + 6 files changed, 62 insertions(+), 98 deletions(-) diff --git a/docs/design/validation.md b/docs/design/validation.md index 37fb003..4b27705 100644 --- a/docs/design/validation.md +++ b/docs/design/validation.md @@ -52,14 +52,11 @@ The test name is `SarifMark_MarkdownReportGeneration`, satisfying `SarifMark-Rep ### RunEnforcementTest -`RunEnforcementTest` verifies enforcement mode: +`RunEnforcementTest` verifies enforcement mode by delegating to `RunValidationTest` with +`--enforce` as an extra argument and a validator that: -1. Creates a `TemporaryDirectory`. -2. Writes the mock SARIF file. -3. Constructs a `Context` with `--silent`, `--log `, `--sarif `, and - `--enforce`. -4. Calls `Program.Run` and verifies exit code is non-zero. -5. Checks the log contains `"Error: Issues found in SARIF file"`. +1. Verifies exit code is non-zero. +2. Checks the log contains `"Error: Issues found in SARIF file"`. The test name is `SarifMark_Enforcement`, satisfying `SarifMark-Enforce-Mode` and `SarifMark-Enforce-ExitCode`. @@ -68,15 +65,16 @@ The test name is `SarifMark_Enforcement`, satisfying `SarifMark-Enforce-Mode` an `RunValidationTest` is a private shared helper used by `RunSarifReadingTest`, `RunMarkdownReportGenerationTest`, and `RunEnforcementTest`. It accepts a test name, an -optional report file name, and a caller-supplied `validator` function, and: +optional report file name, a caller-supplied `validator` function, and an optional +`extraArgs` collection, and: 1. Creates a `TemporaryDirectory`. 2. Creates a mock SARIF file and builds a command-line argument list with `--silent`, `--log`, and `--sarif`. If a `reportFileName` is provided, adds `--report` to the - argument list. + argument list. Any `extraArgs` are appended last. 3. Constructs a `Context` and calls `Program.Run`, capturing the exit code. -4. Reads the log and (if present) report file contents and passes them to the `validator` - function. +4. Reads the log and (if present) report file contents and passes the exit code, log + content, and report content to the `validator` function. 5. Records the test as passed or failed in the `TestResults` collection and prints a `✓` or `✗` status line to the context output. diff --git a/src/DemaConsulting.SarifMark/SarifResult.cs b/src/DemaConsulting.SarifMark/SarifResult.cs index a6e25a0..60f14bc 100644 --- a/src/DemaConsulting.SarifMark/SarifResult.cs +++ b/src/DemaConsulting.SarifMark/SarifResult.cs @@ -51,7 +51,7 @@ public record SarifResult public int? StartLine { get; } /// - /// Internal constructor for testing purposes. + /// Internal constructor to enforce that instances are only created through the validated parsing pipeline. /// /// The rule identifier. /// The level of the result. diff --git a/src/DemaConsulting.SarifMark/SarifResults.cs b/src/DemaConsulting.SarifMark/SarifResults.cs index 47d0e12..19873a3 100644 --- a/src/DemaConsulting.SarifMark/SarifResults.cs +++ b/src/DemaConsulting.SarifMark/SarifResults.cs @@ -49,7 +49,7 @@ public record SarifResults public int ResultCount => Results.Count; /// - /// Internal constructor for testing purposes. + /// Internal constructor to enforce that instances are only created through the validated parsing pipeline. /// /// The name of the analysis tool. /// The version of the analysis tool. diff --git a/src/DemaConsulting.SarifMark/Validation.cs b/src/DemaConsulting.SarifMark/Validation.cs index 156fd6b..cf11034 100644 --- a/src/DemaConsulting.SarifMark/Validation.cs +++ b/src/DemaConsulting.SarifMark/Validation.cs @@ -106,8 +106,13 @@ private static void RunSarifReadingTest(Context context, DemaConsulting.TestResu testResults, "SarifMark_SarifReading", null, - (logContent, _) => + (exitCode, logContent, _) => { + if (exitCode != 0) + { + return $"Program exited with code {exitCode}"; + } + if (logContent.Contains("Tool: MockTool 1.0.0") && logContent.Contains("Results: 2")) { @@ -130,8 +135,13 @@ private static void RunMarkdownReportGenerationTest(Context context, DemaConsult testResults, "SarifMark_MarkdownReportGeneration", "sarif-report.md", - (logContent, reportContent) => + (exitCode, _, reportContent) => { + if (exitCode != 0) + { + return $"Program exited with code {exitCode}"; + } + if (reportContent == null) { return "Report file not created"; @@ -154,68 +164,26 @@ private static void RunMarkdownReportGenerationTest(Context context, DemaConsult /// The test results collection. private static void RunEnforcementTest(Context context, DemaConsulting.TestResults.TestResults testResults) { - var startTime = DateTime.UtcNow; - var test = CreateTestResult("SarifMark_Enforcement"); - - try - { - using var tempDir = new TemporaryDirectory(); - var logFile = PathHelpers.SafePathCombine(tempDir.DirectoryPath, "enforcement.log"); - var sarifFile = PathHelpers.SafePathCombine(tempDir.DirectoryPath, "test.sarif"); - - // Create mock SARIF file - CreateMockSarifFile(sarifFile); - - // Build command line arguments - var args = new List - { - "--silent", - "--log", logFile, - "--sarif", sarifFile, - "--enforce" - }; - - // Run the program - int exitCode; - using (var testContext = Context.Create([.. args])) - { - Program.Run(testContext); - exitCode = testContext.ExitCode; - } - - // Check if execution failed as expected (should return non-zero when issues found) - if (exitCode != 0) + RunValidationTest( + context, + testResults, + "SarifMark_Enforcement", + null, + (exitCode, logContent, _) => { - // Read log content - var logContent = File.ReadAllText(logFile); - - if (logContent.Contains("Error: Issues found in SARIF file")) + if (exitCode == 0) { - test.Outcome = DemaConsulting.TestResults.TestOutcome.Passed; - context.WriteLine($"✓ SarifMark_Enforcement - Passed"); + return "Program should have exited with non-zero code"; } - else + + if (logContent.Contains("Error: Issues found in SARIF file")) { - test.Outcome = DemaConsulting.TestResults.TestOutcome.Failed; - test.ErrorMessage = "Expected error message not found"; - context.WriteError($"✗ SarifMark_Enforcement - Failed: Expected error message not found"); + return null; } - } - else - { - test.Outcome = DemaConsulting.TestResults.TestOutcome.Failed; - test.ErrorMessage = "Program should have exited with non-zero code"; - context.WriteError($"✗ SarifMark_Enforcement - Failed: Program should have exited with non-zero code"); - } - } - // Catch all exceptions as this is a test framework - any exception should be recorded as a test failure. - // This is intentional to ensure robust test execution and reporting regardless of exception type. - catch (Exception ex) - { - HandleTestException(test, context, "SarifMark_Enforcement", ex); - } - FinalizeTestResult(test, startTime, testResults); + return "Expected error message not found"; + }, + ["--enforce"]); } /// @@ -225,13 +193,15 @@ private static void RunEnforcementTest(Context context, DemaConsulting.TestResul /// The test results collection. /// The name of the test. /// Optional report file name to generate. - /// Function to validate test results. Returns null on success or error message on failure. + /// Function to validate test results. Receives exit code, log content, and report content. Returns null on success or error message on failure. + /// Optional extra arguments to append to the command line. private static void RunValidationTest( Context context, DemaConsulting.TestResults.TestResults testResults, string testName, string? reportFileName, - Func validator) + Func validator, + IEnumerable? extraArgs = null) { var startTime = DateTime.UtcNow; var test = CreateTestResult(testName); @@ -260,6 +230,11 @@ private static void RunValidationTest( args.Add(reportFile); } + if (extraArgs != null) + { + args.AddRange(extraArgs); + } + // Run the program int exitCode; using (var testContext = Context.Create([.. args])) @@ -268,35 +243,25 @@ private static void RunValidationTest( exitCode = testContext.ExitCode; } - // Check if execution succeeded - if (exitCode == 0) - { - // Read log and report contents - var logContent = File.ReadAllText(logFile); - var reportContent = reportFile != null && File.Exists(reportFile) - ? File.ReadAllText(reportFile) - : null; + // Read log and report contents + var logContent = File.ReadAllText(logFile); + var reportContent = reportFile != null && File.Exists(reportFile) + ? File.ReadAllText(reportFile) + : null; - // Validate the results - var errorMessage = validator(logContent, reportContent); + // Validate the results + var errorMessage = validator(exitCode, logContent, reportContent); - if (errorMessage == null) - { - test.Outcome = DemaConsulting.TestResults.TestOutcome.Passed; - context.WriteLine($"✓ {testName} - Passed"); - } - else - { - test.Outcome = DemaConsulting.TestResults.TestOutcome.Failed; - test.ErrorMessage = errorMessage; - context.WriteError($"✗ {testName} - Failed: {errorMessage}"); - } + if (errorMessage == null) + { + test.Outcome = DemaConsulting.TestResults.TestOutcome.Passed; + context.WriteLine($"✓ {testName} - Passed"); } else { test.Outcome = DemaConsulting.TestResults.TestOutcome.Failed; - test.ErrorMessage = $"Program exited with code {exitCode}"; - context.WriteError($"✗ {testName} - Failed: Exit code {exitCode}"); + test.ErrorMessage = errorMessage; + context.WriteError($"✗ {testName} - Failed: {errorMessage}"); } } // Catch all exceptions as this is a test framework - any exception should be recorded as a test failure. diff --git a/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs b/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs index e832e58..5060e69 100644 --- a/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs @@ -106,8 +106,8 @@ public void IntegrationTest_ValidateFlag_RunsSelfValidation() _dllPath, "--validate"); - // Assert - Validation runs (exit code may be 0 or 1 depending on test results) - Assert.IsTrue(exitCode is 0 or 1); + // Assert + Assert.AreEqual(0, exitCode); Assert.Contains("SarifMark version", output); Assert.Contains("Total Tests:", output); } diff --git a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs index 25dce71..71708fa 100644 --- a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs @@ -76,6 +76,7 @@ public void Program_Main_VersionFlag_DisplaysVersionOnly() // Assert Assert.AreEqual(0, result); var output = outWriter.ToString(); + Assert.IsFalse(string.IsNullOrWhiteSpace(output)); Assert.DoesNotContain("Copyright", output); Assert.DoesNotContain("SarifMark version", output); } From 269d71d96cf0fb93ee8de5451ab64ea41d36f4ba Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 01:06:49 +0000 Subject: [PATCH 05/13] Strengthen VersionFlag test: assert output matches semver regex Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- test/DemaConsulting.SarifMark.Tests/ProgramTests.cs | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs index 71708fa..6519b7a 100644 --- a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs @@ -18,6 +18,8 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +using System.Text.RegularExpressions; + namespace DemaConsulting.SarifMark.Tests; /// @@ -75,10 +77,10 @@ public void Program_Main_VersionFlag_DisplaysVersionOnly() // Assert Assert.AreEqual(0, result); - var output = outWriter.ToString(); - Assert.IsFalse(string.IsNullOrWhiteSpace(output)); - Assert.DoesNotContain("Copyright", output); - Assert.DoesNotContain("SarifMark version", output); + var output = outWriter.ToString().Trim(); + Assert.IsTrue( + Regex.IsMatch(output, @"^\d+\.\d+\.\d+(?:-[\w.-]+)?(?:\+[\w.-]+)?$"), + $"Version output '{output}' does not match semantic version pattern"); } finally { From d02d1eec630f09cd22272d86a85789d96001f5b2 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 01:08:05 +0000 Subject: [PATCH 06/13] Use source-generated non-backtracking regex for semver version check Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- test/DemaConsulting.SarifMark.Tests/ProgramTests.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs index 6519b7a..1669209 100644 --- a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs @@ -26,8 +26,15 @@ namespace DemaConsulting.SarifMark.Tests; /// Unit tests for the Program class. /// [TestClass] -public class ProgramTests +public partial class ProgramTests { + /// + /// Source-generated regex matching a semantic version string (e.g. 1.2.3, 1.2.3-alpha.1, 1.2.3+build.42). + /// Uses the non-backtracking engine to eliminate ReDoS risk. + /// + [GeneratedRegex(@"^\d+\.\d+\.\d+(?:-[\w.-]+)?(?:\+[\w.-]+)?$", RegexOptions.NonBacktracking)] + private static partial Regex SemanticVersionRegex(); + /// /// Test that Main with no arguments returns error due to missing sarif parameter. /// @@ -79,7 +86,7 @@ public void Program_Main_VersionFlag_DisplaysVersionOnly() Assert.AreEqual(0, result); var output = outWriter.ToString().Trim(); Assert.IsTrue( - Regex.IsMatch(output, @"^\d+\.\d+\.\d+(?:-[\w.-]+)?(?:\+[\w.-]+)?$"), + SemanticVersionRegex().IsMatch(output), $"Version output '{output}' does not match semantic version pattern"); } finally From c1cfd60ed2e156e10be5ff7873abbed3002c4f75 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 02:24:35 +0000 Subject: [PATCH 07/13] Fix all review FAIL findings and observations from 11 review-sets Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- .reviewmark.yaml | 3 + docs/design/program.md | 2 +- docs/design/validation.md | 6 +- docs/reqstream/platform.yaml | 4 +- docs/reqstream/program.yaml | 3 + docs/reqstream/sarif-results.yaml | 1 + src/DemaConsulting.SarifMark/SarifResults.cs | 6 +- .../AssemblyInfo.cs | 2 + .../ContextTests.cs | 95 ++++++++++++++++++- .../PathHelpersTests.cs | 4 + .../ProgramTests.cs | 11 +++ .../ValidationTests.cs | 6 ++ 12 files changed, 132 insertions(+), 11 deletions(-) diff --git a/.reviewmark.yaml b/.reviewmark.yaml index f221116..34eedd8 100644 --- a/.reviewmark.yaml +++ b/.reviewmark.yaml @@ -32,6 +32,8 @@ reviews: - "docs/reqstream/context.yaml" - "docs/design/introduction.md" - "docs/design/command-line.md" + - "docs/design/program.md" + - "docs/design/context.md" - "src/**/Program.cs" - "src/**/Context.cs" - "test/**/ProgramTests.cs" @@ -58,6 +60,7 @@ reviews: - "docs/reqstream/report.yaml" - "docs/reqstream/sarif-results.yaml" - "docs/design/sarif.md" + - "docs/design/sarif-results.md" - "src/**/SarifResults.cs" - "test/**/SarifResultsTests.cs" - "test/**/IntegrationTests.cs" diff --git a/docs/design/program.md b/docs/design/program.md index 1d5c241..5f89232 100644 --- a/docs/design/program.md +++ b/docs/design/program.md @@ -9,7 +9,7 @@ appropriate subsystem, and handles top-level exception translation. ## Version Property The static `Version` property reads the assembly's `AssemblyInformationalVersionAttribute` at -runtime. If that attribute is absent it falls back to the `AssemblyVersion`; if that is also +runtime. If that attribute is absent, it falls back to the `AssemblyVersion`; if that is also unavailable it returns `"0.0.0"`. This satisfies requirement `SarifMark-Program-Version`. ## Main Method diff --git a/docs/design/validation.md b/docs/design/validation.md index 4b27705..86a5b13 100644 --- a/docs/design/validation.md +++ b/docs/design/validation.md @@ -14,7 +14,11 @@ organizes all test execution internally. ### Run Method -`Run` orchestrates the self-validation sequence: +Before executing the sequence, `Run` validates its input by calling +`ArgumentNullException.ThrowIfNull(context)`, throwing `ArgumentNullException` immediately if +`context` is null. This satisfies requirement `SarifMark-Validation-NullCheck`. + +`Run` then orchestrates the self-validation sequence: 1. Calls `PrintValidationHeader` to emit a markdown table with tool version, machine name, OS version, .NET runtime, and timestamp. diff --git a/docs/reqstream/platform.yaml b/docs/reqstream/platform.yaml index 469592c..42bacea 100644 --- a/docs/reqstream/platform.yaml +++ b/docs/reqstream/platform.yaml @@ -77,8 +77,8 @@ sections: - id: SarifMark-Plt-Net10 title: The tool shall support .NET 10.0 runtime. justification: >- - .NET 10.0 support provides forward compatibility and ensures the tool remains viable as users adopt - future .NET versions, extending the tool's lifespan and relevance. + .NET 10.0 support provides forward compatibility and ensures the tool remains viable as + adoption grows, extending the tool's lifespan and relevance. tags: [public] tests: - dotnet10.x@SarifMark_SarifReading diff --git a/docs/reqstream/program.yaml b/docs/reqstream/program.yaml index fe7cce2..5d08677 100644 --- a/docs/reqstream/program.yaml +++ b/docs/reqstream/program.yaml @@ -76,3 +76,6 @@ sections: tags: [internal] tests: - Program_Main_NoArguments_ReturnsError + - IntegrationTest_ValidSarifFile_ProcessesSuccessfully + - IntegrationTest_EnforceFlagWithIssues_ReturnsError + - IntegrationTest_GenerateReport_CreatesReportFile diff --git a/docs/reqstream/sarif-results.yaml b/docs/reqstream/sarif-results.yaml index 20076fe..7dd4157 100644 --- a/docs/reqstream/sarif-results.yaml +++ b/docs/reqstream/sarif-results.yaml @@ -139,3 +139,4 @@ sections: tests: - SarifResults_ToMarkdown_NoResults_ShowsFoundNoResults - SarifResults_ToMarkdown_OneResult_UsesSingularForm + - SarifResults_ToMarkdown_Depth1_ProducesCorrectOutput diff --git a/src/DemaConsulting.SarifMark/SarifResults.cs b/src/DemaConsulting.SarifMark/SarifResults.cs index 19873a3..29c2284 100644 --- a/src/DemaConsulting.SarifMark/SarifResults.cs +++ b/src/DemaConsulting.SarifMark/SarifResults.cs @@ -117,13 +117,13 @@ private static JsonElement ValidateSarifStructure(JsonElement root) throw new InvalidOperationException("Invalid SARIF file: missing or invalid 'runs' array."); } - var runs = runsElement.EnumerateArray(); - if (!runs.Any()) + var runs = runsElement.EnumerateArray().ToList(); + if (runs.Count == 0) { throw new InvalidOperationException("Invalid SARIF file: 'runs' array is empty."); } - return runs.First(); + return runs[0]; } /// diff --git a/test/DemaConsulting.SarifMark.Tests/AssemblyInfo.cs b/test/DemaConsulting.SarifMark.Tests/AssemblyInfo.cs index 495b4f8..71fe7b2 100644 --- a/test/DemaConsulting.SarifMark.Tests/AssemblyInfo.cs +++ b/test/DemaConsulting.SarifMark.Tests/AssemblyInfo.cs @@ -18,4 +18,6 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. +// Disable parallel test execution to prevent Console.Out/Console.Error redirection conflicts +// in ProgramTests, which captures console output for assertion purposes. [assembly: DoNotParallelize] diff --git a/test/DemaConsulting.SarifMark.Tests/ContextTests.cs b/test/DemaConsulting.SarifMark.Tests/ContextTests.cs index 2e17e1f..32c3daa 100644 --- a/test/DemaConsulting.SarifMark.Tests/ContextTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ContextTests.cs @@ -357,11 +357,11 @@ public void Context_Dispose_ProperlyClosesLogFile() public void Context_ExitCode_StartsAtZero_ChangesToOneAfterError() { // Arrange - var originalOut = Console.Out; + var originalError = Console.Error; try { - using var outWriter = new StringWriter(); - Console.SetOut(outWriter); + using var errWriter = new StringWriter(); + Console.SetError(errWriter); using var context = Context.Create([]); @@ -376,7 +376,7 @@ public void Context_ExitCode_StartsAtZero_ChangesToOneAfterError() } finally { - Console.SetOut(originalOut); + Console.SetError(originalError); } } @@ -520,4 +520,91 @@ public void Context_Create_ResultsParameter_SetsResultsFile() // Assert Assert.AreEqual("results.trx", context.ResultsFile); } + + /// + /// Test that WriteLine writes to the log file when it is open. + /// + [TestMethod] + public void Context_WriteLine_WithLogFile_WritesToLog() + { + // Arrange + var logFile = PathHelpers.SafePathCombine(Path.GetTempPath(), $"test-log-{Guid.NewGuid()}.log"); + try + { + using (var context = Context.Create(["--silent", "--log", logFile])) + { + // Act + context.WriteLine("LogLine message"); + } + + // Assert - message must appear in the log file even though --silent suppresses console + var logContent = File.ReadAllText(logFile); + Assert.Contains("LogLine message", logContent); + } + finally + { + if (File.Exists(logFile)) + { + File.Delete(logFile); + } + } + } + + /// + /// Test that WriteLine writes to the log file in silent mode. + /// + [TestMethod] + public void Context_WriteLine_SilentModeWithLogFile_WritesToLog() + { + // Arrange + var logFile = PathHelpers.SafePathCombine(Path.GetTempPath(), $"test-log-{Guid.NewGuid()}.log"); + try + { + using (var context = Context.Create(["--silent", "--log", logFile])) + { + // Act + context.WriteLine("Silent log message"); + } + + // Assert - message must appear in the log even in silent mode + var logContent = File.ReadAllText(logFile); + Assert.Contains("Silent log message", logContent); + } + finally + { + if (File.Exists(logFile)) + { + File.Delete(logFile); + } + } + } + + /// + /// Test that WriteError writes to the log file when it is open. + /// + [TestMethod] + public void Context_WriteError_WithLogFile_WritesToLog() + { + // Arrange + var logFile = PathHelpers.SafePathCombine(Path.GetTempPath(), $"test-log-{Guid.NewGuid()}.log"); + try + { + using (var context = Context.Create(["--silent", "--log", logFile])) + { + // Act + context.WriteError("Error log message"); + } + + // Assert - error message must appear in the log file + var logContent = File.ReadAllText(logFile); + Assert.Contains("Error log message", logContent); + } + finally + { + if (File.Exists(logFile)) + { + File.Delete(logFile); + } + } + } } diff --git a/test/DemaConsulting.SarifMark.Tests/PathHelpersTests.cs b/test/DemaConsulting.SarifMark.Tests/PathHelpersTests.cs index 8a6d4a5..ae10037 100644 --- a/test/DemaConsulting.SarifMark.Tests/PathHelpersTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/PathHelpersTests.cs @@ -98,6 +98,7 @@ public void PathHelpers_SafePathCombine_PathWithDoubleDots_ThrowsArgumentExcepti var exception = Assert.Throws(() => PathHelpers.SafePathCombine(basePath, relativePath)); Assert.Contains("Invalid path component", exception.Message); + Assert.AreEqual("relativePath", exception.ParamName); } /// @@ -117,6 +118,7 @@ public void PathHelpers_SafePathCombine_FilenameWithEmbeddedDots_ThrowsArgumentE var exception = Assert.Throws(() => PathHelpers.SafePathCombine(basePath, relativePath)); Assert.Contains("Invalid path component", exception.Message); + Assert.AreEqual("relativePath", exception.ParamName); } /// @@ -131,6 +133,7 @@ public void PathHelpers_SafePathCombine_AbsolutePath_ThrowsArgumentException() var unixException = Assert.Throws(() => PathHelpers.SafePathCombine(unixBasePath, unixRelativePath)); Assert.Contains("Invalid path component", unixException.Message); + Assert.AreEqual("relativePath", unixException.ParamName); // Test Windows absolute path (only on Windows since Windows paths may not be rooted on Unix) if (OperatingSystem.IsWindows()) @@ -140,6 +143,7 @@ public void PathHelpers_SafePathCombine_AbsolutePath_ThrowsArgumentException() var windowsException = Assert.Throws(() => PathHelpers.SafePathCombine(windowsBasePath, windowsRelativePath)); Assert.Contains("Invalid path component", windowsException.Message); + Assert.AreEqual("relativePath", windowsException.ParamName); } } diff --git a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs index 1669209..cc45de2 100644 --- a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs @@ -117,6 +117,17 @@ public void Program_Main_HelpFlag_DisplaysHelp() Assert.Contains("SarifMark version", output); Assert.Contains("Copyright", output); Assert.Contains("Usage:", output); + Assert.Contains("--version", output); + Assert.Contains("--help", output); + Assert.Contains("--silent", output); + Assert.Contains("--validate", output); + Assert.Contains("--results", output); + Assert.Contains("--enforce", output); + Assert.Contains("--log", output); + Assert.Contains("--sarif", output); + Assert.Contains("--report", output); + Assert.Contains("--report-depth", output); + Assert.Contains("--heading", output); } finally { diff --git a/test/DemaConsulting.SarifMark.Tests/ValidationTests.cs b/test/DemaConsulting.SarifMark.Tests/ValidationTests.cs index 89f2d24..ac47561 100644 --- a/test/DemaConsulting.SarifMark.Tests/ValidationTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ValidationTests.cs @@ -61,6 +61,12 @@ public void Validation_Run_ValidContext_PrintsValidationHeader() "Log should contain the 'SarifMark Version' header row"); Assert.Contains("Machine Name", logContent, "Log should contain the 'Machine Name' header row"); + Assert.Contains("OS Version", logContent, + "Log should contain the 'OS Version' header row"); + Assert.Contains("DotNet Runtime", logContent, + "Log should contain the 'DotNet Runtime' header row"); + Assert.Contains("Time Stamp", logContent, + "Log should contain the 'Time Stamp' header row"); } finally { From 155a8206d1a8be3275ec61c97e70d752c5c8da53 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Mon, 16 Mar 2026 22:32:04 -0400 Subject: [PATCH 08/13] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/DemaConsulting.SarifMark/SarifResults.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/DemaConsulting.SarifMark/SarifResults.cs b/src/DemaConsulting.SarifMark/SarifResults.cs index 29c2284..88ae485 100644 --- a/src/DemaConsulting.SarifMark/SarifResults.cs +++ b/src/DemaConsulting.SarifMark/SarifResults.cs @@ -117,13 +117,13 @@ private static JsonElement ValidateSarifStructure(JsonElement root) throw new InvalidOperationException("Invalid SARIF file: missing or invalid 'runs' array."); } - var runs = runsElement.EnumerateArray().ToList(); - if (runs.Count == 0) + var runsEnumerator = runsElement.EnumerateArray(); + if (!runsEnumerator.MoveNext()) { throw new InvalidOperationException("Invalid SARIF file: 'runs' array is empty."); } - return runs[0]; + return runsEnumerator.Current; } /// From d39a857a6e18767fc504366064a07c77723137d8 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Mon, 16 Mar 2026 22:32:36 -0400 Subject: [PATCH 09/13] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- test/DemaConsulting.SarifMark.Tests/ContextTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/DemaConsulting.SarifMark.Tests/ContextTests.cs b/test/DemaConsulting.SarifMark.Tests/ContextTests.cs index 32c3daa..54a00a6 100644 --- a/test/DemaConsulting.SarifMark.Tests/ContextTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ContextTests.cs @@ -531,7 +531,7 @@ public void Context_WriteLine_WithLogFile_WritesToLog() var logFile = PathHelpers.SafePathCombine(Path.GetTempPath(), $"test-log-{Guid.NewGuid()}.log"); try { - using (var context = Context.Create(["--silent", "--log", logFile])) + using (var context = Context.Create(["--log", logFile])) { // Act context.WriteLine("LogLine message"); From f5d63ec2edb9d33ff88bb45fa061a27333ba8bbc Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 02:35:15 +0000 Subject: [PATCH 10/13] Fix stale comment in Context_WriteLine_WithLogFile_WritesToLog Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- test/DemaConsulting.SarifMark.Tests/ContextTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/DemaConsulting.SarifMark.Tests/ContextTests.cs b/test/DemaConsulting.SarifMark.Tests/ContextTests.cs index 54a00a6..3a56423 100644 --- a/test/DemaConsulting.SarifMark.Tests/ContextTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ContextTests.cs @@ -537,7 +537,7 @@ public void Context_WriteLine_WithLogFile_WritesToLog() context.WriteLine("LogLine message"); } - // Assert - message must appear in the log file even though --silent suppresses console + // Assert - message must appear in the log file var logContent = File.ReadAllText(logFile); Assert.Contains("LogLine message", logContent); } From 6deb032010c6404d32f502492c07199273716ac8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 03:17:46 +0000 Subject: [PATCH 11/13] Fix Report-Subsystem FAIL and close traceability gaps from review-set runs Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- docs/design/sarif-results.md | 3 ++- docs/reqstream/context.yaml | 3 +++ docs/reqstream/sarif-results.yaml | 1 + 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/design/sarif-results.md b/docs/design/sarif-results.md index a987b15..1c618c7 100644 --- a/docs/design/sarif-results.md +++ b/docs/design/sarif-results.md @@ -93,7 +93,8 @@ and `SarifMark-SarifResults-FilterSuppressions`. The sub-heading level is `min(depth + 1, 6)`. 3. **Issues section** — calls `AppendIssuesSection` to emit the `Issues` sub-heading, the result count formatted by `FormatFoundText`, and one line per result formatted by - `FormatLocation`. + `FormatLocation`. Each result line is appended with a trailing two-space markdown hard + line break (` `) before the newline, satisfying requirement `SarifMark-Report-LineBreaks`. This satisfies requirement `SarifMark-SarifResults-ToMarkdown`. diff --git a/docs/reqstream/context.yaml b/docs/reqstream/context.yaml index c95f8c1..b65e75b 100644 --- a/docs/reqstream/context.yaml +++ b/docs/reqstream/context.yaml @@ -150,6 +150,8 @@ sections: tests: - Context_WriteLine_WritesToConsole - Context_WriteLine_SilentMode_DoesNotWriteToConsole + - Context_WriteLine_WithLogFile_WritesToLog + - Context_WriteLine_SilentModeWithLogFile_WritesToLog - id: SarifMark-Context-WriteError title: >- @@ -163,6 +165,7 @@ sections: tests: - Context_WriteError_WritesToErrorAndSetsExitCode - Context_WriteError_SilentMode_DoesNotWriteToConsoleButSetsExitCode + - Context_WriteError_WithLogFile_WritesToLog - id: SarifMark-Context-ExitCode title: The ExitCode property shall start at zero and change to one after the first call to WriteError. diff --git a/docs/reqstream/sarif-results.yaml b/docs/reqstream/sarif-results.yaml index 7dd4157..3086637 100644 --- a/docs/reqstream/sarif-results.yaml +++ b/docs/reqstream/sarif-results.yaml @@ -13,6 +13,7 @@ sections: tags: [internal] tests: - SarifResults_Read_NoResults_ReturnsValidResults + - SarifResults_InternalConstructor_CreatesValidInstance - id: SarifMark-SarifResults-ValidatePath title: The Read method shall validate the file path is non-null, non-empty, and the file exists. From 02973c5af6c3c6a2503c10b83e247d2204147617 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 03:27:33 +0000 Subject: [PATCH 12/13] Remove redundant utilities.md and clean up all references to it Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- .reviewmark.yaml | 1 - docs/design/definition.yaml | 1 - docs/design/introduction.md | 3 +-- docs/design/utilities.md | 25 ------------------------- 4 files changed, 1 insertion(+), 29 deletions(-) delete mode 100644 docs/design/utilities.md diff --git a/.reviewmark.yaml b/.reviewmark.yaml index 34eedd8..a47bafa 100644 --- a/.reviewmark.yaml +++ b/.reviewmark.yaml @@ -132,7 +132,6 @@ reviews: title: Review of SarifMark PathHelpers Software Unit paths: - "docs/reqstream/path-helpers.yaml" - - "docs/design/utilities.md" - "docs/design/path-helpers.md" - "src/**/PathHelpers.cs" - "test/**/PathHelpersTests.cs" diff --git a/docs/design/definition.yaml b/docs/design/definition.yaml index 34d08e6..e065ba2 100644 --- a/docs/design/definition.yaml +++ b/docs/design/definition.yaml @@ -7,7 +7,6 @@ input-files: - docs/design/introduction.md - docs/design/command-line.md - docs/design/sarif.md - - docs/design/utilities.md - docs/design/validation.md - docs/design/program.md - docs/design/context.md diff --git a/docs/design/introduction.md b/docs/design/introduction.md index d5036a0..b8ced6f 100644 --- a/docs/design/introduction.md +++ b/docs/design/introduction.md @@ -15,13 +15,12 @@ The purpose of this document is to: ## Scope -This document covers the design of four primary functional layers within SarifMark: +This document covers the design of three primary functional layers within SarifMark: - The **command-line layer**: the `Program` entry point and `Context` class that handle argument parsing, output routing, and program flow control - The **SARIF and reporting layer**: the `SarifResult` and `SarifResults` classes that read SARIF files and generate markdown reports -- The **utilities layer**: the `PathHelpers` class providing safe path operations - The **self-validation layer**: the `Validation` class that provides built-in verification of the tool's core functionality diff --git a/docs/design/utilities.md b/docs/design/utilities.md deleted file mode 100644 index 27547c6..0000000 --- a/docs/design/utilities.md +++ /dev/null @@ -1,25 +0,0 @@ -# Utilities - -## Overview - -The utilities layer provides internal support classes used across the tool. It currently -consists of a single class: `PathHelpers`, which defends against -path-traversal vulnerabilities when constructing file paths from partially-trusted input. - -## PathHelpers - -`PathHelpers` is a static internal class exposing a single method, `SafePathCombine`. It -is used by `Validation.TemporaryDirectory` when constructing paths inside a temporary -directory from `Guid`-based file names. - -`SafePathCombine` applies two layers of validation to guard against path-traversal attacks: - -1. **Pre-combination check** — rejects paths containing `".."` or rooted (absolute) paths. -2. **Post-combination check** — resolves both paths with `Path.GetFullPath` and verifies - the result still sits under the base path. - -See the PathHelpers Class document for class-level details. - -## Class Details - -- **PathHelpers class** — safe path combination with traversal defense From e1eda9ebb8c8527a75604a0a85549cfe52bc182f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Mar 2026 10:35:04 +0000 Subject: [PATCH 13/13] Use Assert.MatchesRegex for --report and semver assertions; fix parameter order Co-authored-by: Malcolmnixon <1863707+Malcolmnixon@users.noreply.github.com> --- .../IntegrationTests.cs | 2 +- .../ProgramTests.cs | 17 +++-------------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs b/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs index 5060e69..8896cf0 100644 --- a/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs @@ -88,7 +88,7 @@ public void IntegrationTest_HelpFlag_OutputsUsageInformation() Assert.Contains("--version", output); Assert.Contains("--help", output); Assert.Contains("--sarif", output); - Assert.Contains("--report", output); + Assert.MatchesRegex(@"--report(?!-)", output); } /// diff --git a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs index cc45de2..e36f477 100644 --- a/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs +++ b/test/DemaConsulting.SarifMark.Tests/ProgramTests.cs @@ -18,23 +18,14 @@ // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE // SOFTWARE. -using System.Text.RegularExpressions; - namespace DemaConsulting.SarifMark.Tests; /// /// Unit tests for the Program class. /// [TestClass] -public partial class ProgramTests +public class ProgramTests { - /// - /// Source-generated regex matching a semantic version string (e.g. 1.2.3, 1.2.3-alpha.1, 1.2.3+build.42). - /// Uses the non-backtracking engine to eliminate ReDoS risk. - /// - [GeneratedRegex(@"^\d+\.\d+\.\d+(?:-[\w.-]+)?(?:\+[\w.-]+)?$", RegexOptions.NonBacktracking)] - private static partial Regex SemanticVersionRegex(); - /// /// Test that Main with no arguments returns error due to missing sarif parameter. /// @@ -85,9 +76,7 @@ public void Program_Main_VersionFlag_DisplaysVersionOnly() // Assert Assert.AreEqual(0, result); var output = outWriter.ToString().Trim(); - Assert.IsTrue( - SemanticVersionRegex().IsMatch(output), - $"Version output '{output}' does not match semantic version pattern"); + Assert.MatchesRegex(@"^\d+\.\d+\.\d+(?:-[\w.-]+)?(?:\+[\w.-]+)?$", output); } finally { @@ -125,7 +114,7 @@ public void Program_Main_HelpFlag_DisplaysHelp() Assert.Contains("--enforce", output); Assert.Contains("--log", output); Assert.Contains("--sarif", output); - Assert.Contains("--report", output); + Assert.MatchesRegex(@"--report(?!-)", output); Assert.Contains("--report-depth", output); Assert.Contains("--heading", output); }