Add --exclude glob filter for CodeQL compiled-language false positives - #141
Conversation
CodeQL's paths-ignore config only affects extraction/build scope for compiled languages (C#, Java, C++, Go); findings from generated code in bin/obj still appear in the raw SARIF output regardless of config. This adds a repeatable --exclude <glob> CLI option so SarifMark can filter findings by file-path glob pattern natively, without requiring an extra Action/step (e.g. advanced-security/filter-sarif) in CI workflows. - New repeatable --exclude <glob> option filters SarifFinding entries by Uri using Microsoft.Extensions.FileSystemGlobbing, applied after reading the SARIF file and before --enforce/--report processing. - Findings with no physical location (null Uri) are never excluded. - Added Microsoft.Extensions.FileSystemGlobbing as a 12th OTS item with full design/verification/reqstream documentation and SysML2 model updates, following the existing DemaConsulting.TestResults pattern. - Added feature requirements, design docs, and verification docs for the new CLI option across the Cli, Cli.Context, Sarif, and Sarif.Results units, plus a new system-level end-to-end integration test. - Updated README and user guide (usage.md, faq.md) with --exclude documentation and a CodeQL bin/obj example. - Fixed a pre-existing README --help transcript inaccuracy (--depth range qualifier) and a misleading OTS verification-strategy sentence found during formal review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
A critical CLI parsing issue and several requested test and documentation updates remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR adds repeatable --exclude <glob> filtering for SARIF findings before enforcement and reporting, with tests, dependency updates, documentation, and traceability artifacts.
Changes:
- Added URI glob filtering via
Microsoft.Extensions.FileSystemGlobbing. - Added unit, integration, and end-to-end coverage.
- Updated user documentation, requirements, design, verification, SysML2, and review metadata.
Review findings:
- Critical: Reject recognized options used as a missing
--excludevalue and add regression coverage. - Moderate: Add multi-run count, help, URI-format, and metadata test cases.
- Nit: Align documented processing order and include SysML2Tools in the OTS verification list.
File summaries
| File | Summary |
|---|---|
test/DemaConsulting.SarifMark.Tests/Sarif/SarifResultsTests.cs |
Tests filtering and metadata preservation. |
test/DemaConsulting.SarifMark.Tests/ProgramTests.cs |
Tests filtering, reporting, and enforcement. |
test/DemaConsulting.SarifMark.Tests/IntegrationTests.cs |
Tests compiled-language filtering end to end. |
test/DemaConsulting.SarifMark.Tests/Cli/ContextTests.cs |
Tests exclusion context parsing. |
test/DemaConsulting.SarifMark.Tests/Cli/CliTests.cs |
Tests CLI exclusion handling. |
src/DemaConsulting.SarifMark/Sarif/SarifResults.cs |
Implements URI glob filtering. |
src/DemaConsulting.SarifMark/Program.cs |
Applies filtering and reports results. |
src/DemaConsulting.SarifMark/DemaConsulting.SarifMark.csproj |
Adds the globbing dependency. |
src/DemaConsulting.SarifMark/Cli/Context.cs |
Parses repeatable exclusion patterns. |
requirements.yaml |
Registers related requirements and OTS coverage. |
README.md |
Documents the new option and usage. |
docs/verification/sarifmark/sarif/sarif-results.md |
Defines SARIF-results verification scenarios. |
docs/verification/sarifmark/sarif.md |
Documents SARIF verification. |
docs/verification/sarifmark/program.md |
Documents program-level verification. |
docs/verification/sarifmark/cli/context.md |
Documents context verification. |
docs/verification/sarifmark/cli.md |
Documents CLI verification. |
docs/verification/sarifmark.md |
Updates system verification coverage. |
docs/verification/ots/filesystem-globbing.md |
Defines globbing dependency verification. |
docs/verification/ots.md |
Updates the OTS verification strategy. |
docs/user_guide/usage.md |
Adds exclusion usage guidance. |
docs/user_guide/faq.md |
Explains CodeQL generated-code filtering. |
docs/sysml2/model/sarifmark.sysml |
Adds the dependency to the system model. |
docs/sysml2/model/ots.sysml |
Models the globbing dependency. |
docs/reqstream/sarifmark/sarif/sarif-results.yaml |
Adds SARIF-results requirements. |
docs/reqstream/sarifmark/sarif.yaml |
Adds SARIF filtering requirements. |
docs/reqstream/sarifmark/program.yaml |
Adds program requirements. |
docs/reqstream/sarifmark/cli/context.yaml |
Adds context requirements. |
docs/reqstream/sarifmark/cli.yaml |
Adds CLI requirements. |
docs/reqstream/sarifmark.yaml |
Adds system-level requirements. |
docs/reqstream/ots/filesystem-globbing.yaml |
Defines dependency requirements. |
docs/design/sarifmark/sarif/sarif-results.md |
Documents filtering design. |
docs/design/sarifmark/sarif.md |
Documents SARIF integration. |
docs/design/sarifmark/program.md |
Documents processing flow. |
docs/design/sarifmark/cli/context.md |
Documents exclusion state. |
docs/design/sarifmark/cli.md |
Documents the CLI contract. |
docs/design/sarifmark.md |
Updates system design and data flow. |
docs/design/ots/filesystem-globbing.md |
Documents globbing integration. |
docs/design/ots.md |
Registers the new OTS dependency. |
docs/design/introduction.md |
Updates the design index. |
.reviewmark.yaml |
Adds OTS review coverage. |
Review details
Suppressed comments (5)
docs/design/sarifmark.md:152
- The sequence here still documents report generation before enforcement, but
ProcessSarifAnalysischeckscontext.Enforcebefore writing the report. Please reorder these steps so the design flow matches the actual control flow.
7. If `--report` was supplied, the markdown string is written to the specified file with
`File.WriteAllText`.
8. If `--enforce` is set and issues were found, `Context.WriteError` sets the exit code to 1.
9. `Program.Main` returns `Context.ExitCode` to the shell.
docs/verification/ots.md:9
- The updated list of tools exposing
--validatestill omits SysML2Tools, although this document later describes adotnet sysml2tools --validateCI step at lines 41-42. Include SysML2Tools here so the verification strategy is internally consistent.
1. **Self-validation output**: Tools that expose a `--validate` flag (BuildMark, FileAssert, ReqStream, ReviewMark,
VersionMark) are exercised through their own `--validate` self-validation mechanism as separate CI pipeline
steps; passing output confirms the tool is installed and all advertised features are operational.
src/DemaConsulting.SarifMark/Program.cs:237
- The excluded-count summary is specified as the total removed across all runs, but the Program-level test only uses a single-run SARIF with one finding. Add a multi-run case with matches in more than one run to verify that this
Sum-based count remains correct.
src/DemaConsulting.SarifMark/Sarif/SarifResults.cs:437 - This fixed-root call is documented as handling relative,
file://, Unix-absolute, and Windows-drive URIs, but the tests only cover relative paths and a simple Unix-stylefile:///URI. Add cases for absolute and Windows-drive forms (including separator/drive behavior), otherwise a platform-specific mismatch could silently leave CodeQL findings unfiltered.
test/DemaConsulting.SarifMark.Tests/Sarif/SarifResultsTests.cs:1548 - This test uses only one run even though its summary and the design contract say metadata is preserved for every run. A regression that drops or changes metadata on a later run would still pass; use a multi-run fixture and assert
ToolName,ToolVersion, andFileCountfor both runs (the filtering test does not check metadata).
- Files reviewed: 40/40 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Fix GetRequiredStringArgument (shared by --log, --sarif, --report, --heading, --results, and --exclude) to reject a following token that is itself a recognized option, instead of silently consuming it as the value (e.g. --exclude --enforce no longer swallows --enforce). - Apply the same fix to GetRequiredIntArgument (--depth/--report-depth) for structural consistency, using the shared KnownOptionTokens set. - Add regression tests for --exclude, --sarif, and --depth followed by another option. - Add an assertion to Program_Main_HelpFlag_DisplaysHelp confirming the --exclude help line is present. - Link new test names to existing ReqStream requirements (no new requirement IDs). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟢 Approval recommended
No blocking issues were identified; only a minor README command-example correction was noted.
Review details
Suppressed comments (1)
README.md:140
- The PR description's example uses
sarifmark results.sarif ..., but this parser accepts the SARIF path only after--sarif; a positionalresults.sarifis rejected as an unsupported argument. Please update that example to match the runnable form shown here, e.g.sarifmark --sarif results.sarif ....
sarifmark --sarif analysis.sarif --report report.md --exclude "**/bin/**" --exclude "**/obj/**"
- Files reviewed: 40/40 changed files
- Comments generated: 0 new
- Review effort level: Lite
Summary
CodeQL's
paths-ignoreconfiguration only affects the extraction/build scope for compiled languages (C#, Java, C++, Go) — it does not filter the raw SARIF findings. Any file the compiler touches (including generated code underbin/obj) still appears in results regardless of config. Actions likeadvanced-security/filter-sarifexist to post-process SARIF and drop findings by glob pattern; this PR adds that capability natively to SarifMark so users don't need an extra workflow step.Changes
--exclude <glob>CLI option that filtersSarifFindingentries by theirUriusingMicrosoft.Extensions.FileSystemGlobbing, applied after reading the SARIF file and before--enforce/--reportprocessing (excluded findings don't affect the exit code or the generated report).nullUri) are never excluded.Microsoft.Extensions.FileSystemGlobbingas a 12th OTS item, with design/verification/reqstream documentation and SysML2 model updates following the existingDemaConsulting.TestResultspattern (verified via SarifMark's own tests, no--validatehook).Cli,Cli.Context,Sarif, andSarif.SarifResultsunits, plus a new system-level end-to-end integration test.README.mdand the user guide (usage.md,faq.md) with--excludedocumentation and abin/objCodeQL example.--helptranscript inaccuracy (missing--depthrange qualifier) and a misleadingdocs/verification/ots.mdsentence about how several OTS tools are verified.Example
Validation
pwsh ./build.ps1— build succeeds, 0 warnings, 663/663 tests pass across net8.0/net9.0/net10.0.pwsh ./fix.ps1/pwsh ./lint.ps1— clean (yamllint, cspell, markdownlint-cli2, dotnet format, sysml2tools lint).dotnet reqstream --lint/--enforce --root-tags public,quality— no orphaned or unsatisfied requirements introduced by this change.dotnet reviewmark --lint/--plan --enforce— full coverage for the new OTS item.dotnet versionmark --lint— clean.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com