diff --git a/docs/README.md b/docs/README.md index d9ca27b0..578cbba0 100644 --- a/docs/README.md +++ b/docs/README.md @@ -26,6 +26,7 @@ prerequisites for using the plugin. | [Minimal integration example](agent-consumption.md#try-a-minimal-integration) | A bootstrap prompt connecting your agent to a BCQuality checkout and an app folder. | | [How agents consume BCQuality](agent-consumption.md) | Architecture, repository structure, routing, and delivery of findings. | | [Standalone runner](standalone-runner.md) | Optional model selection, scheduling, retries, and telemetry. | +| [SARIF export](standalone-runner.md#publish-findings-as-sarif) | Show accepted findings as code scanning annotations in GitHub or Azure DevOps. | | [Global skills](../skills/README.md) | Host adapters versus internal protocol files. | | [Entry](../skills/entry.md) | Task context and skill dispatch. | | [READ](../skills/read.md) | Knowledge schema, applicability, and precedence. | diff --git a/docs/standalone-runner.md b/docs/standalone-runner.md index 8980b034..edb18536 100644 --- a/docs/standalone-runner.md +++ b/docs/standalone-runner.md @@ -186,3 +186,42 @@ A compatible runner: A CI integration, custom agent, or small host-native plugin can implement this runner contract. These remain optional consumers: BCQuality's knowledge and skills stay independent of their orchestration choices. + +## Publish findings as SARIF + +To show findings as annotations in GitHub code scanning, Azure DevOps, or +another SARIF 2.1.0 viewer, convert the **accepted** report after the +acceptance gate: + +```powershell +pwsh ./tools/ConvertTo-Sarif.ps1 -ReportPath ./accepted-report.json ` + -OutputPath ./bcquality.sarif -SourcePathPrefix 'apps/MyApp' +``` + +`-SourcePathPrefix` is the reviewed app folder relative to the repository root, +needed when the app is not the repository root. Source paths and the prefix +are filesystem paths, not pre-encoded URIs. The converter URI-encodes each +combined path segment while preserving `/` separators, so spaces, `#`, literal +`%` characters, and non-ASCII filenames retain their file binding. The report's +raw source paths are not modified. The converter maps +`blocker`/`major` to `error`, `minor` to `warning`, and `info` to `note`, links +each knowledge-backed rule to its article, and converts only top-level findings +so a super-skill's copied leaf findings are not duplicated. A `partial` or +`failed` outcome sets `executionSuccessful` to `false` with a notification, +so an incomplete review is never published as a clean one. Confidence, domain, +references, and code suggestions are kept in each result's `properties`. + +GitHub code scanning requires a source location for every result. A top-level +finding without a location is therefore emitted as a +`invocations[].toolExecutionNotifications` entry instead of a `results` entry. +The notification includes the finding ID and full message, links to its rule +through `associatedRule`, uses the same severity mapping, and retains the +finding metadata in `properties`. `run.properties.locationlessFindingCount` +records how many findings were represented this way (zero when none). +No fallback file is invented and no finding is silently discarded. A report +containing only location-less findings has an empty `results` array but retains +all those findings as notifications. This does not change `executionSuccessful`, +which still reflects the report's outcome; these findings are not file annotations. + +The converter validates the report schema but is not an acceptance gate; run +`tools/Validate-FindingsReport.ps1` first. diff --git a/tools/ConvertTo-Sarif.ps1 b/tools/ConvertTo-Sarif.ps1 new file mode 100644 index 00000000..4b4d08ea --- /dev/null +++ b/tools/ConvertTo-Sarif.ps1 @@ -0,0 +1,246 @@ +#Requires -Version 7.5 +<# +.SYNOPSIS + Converts a BCQuality findings-report into a SARIF 2.1.0 log. + +.DESCRIPTION + Reads one findings-report (leaf or super-skill), validates it against + schemas/findings-report.schema.json, and writes a SARIF 2.1.0 log that code + scanning hosts such as GitHub and Azure DevOps can display as annotations. + + The converter is a presentation step. It does not replace the DO acceptance + gate: run tools/Validate-FindingsReport.ps1 first and convert only accepted + reports. The report is not modified. + + Mapping: + - Only top-level findings are converted. A super-skill already copies + accepted leaf findings to its top-level findings[], so converting + sub-results as well would duplicate them. + - findings[].id becomes the SARIF ruleId. Knowledge-backed IDs are article + paths; their rule links to the article. Agent findings (agent:...) get a + rule without a help link. + - severity blocker/major -> error, minor -> warning, info -> note. + - location.file/line/range -> physicalLocation relative to SRCROOT, prefixed + with -SourcePathPrefix when the reviewed app is a sub-folder of the + repository that the SARIF consumer analyzes. + - severity, confidence, domain, references, from-sub-skill, and the code + suggestion fields are kept in result.properties. + - Findings without a location become tool execution notifications, not + results, so GitHub code scanning can accept every emitted result. + Notifications retain the finding ID, message, severity, and properties; + run.properties.locationlessFindingCount records their count. + - A partial or failed outcome sets executionSuccessful = false and is + reported as a tool execution notification, so the log is never mistaken + for a clean result. + +.EXAMPLE + pwsh ./tools/ConvertTo-Sarif.ps1 -ReportPath ./report.json -OutputPath ./bcquality.sarif +#> +[CmdletBinding()] +param( + [Parameter(Mandatory)] + [string] $ReportPath, + [string] $OutputPath, + [string] $BCQualityRoot, + [string] $SourcePathPrefix = '', + [string] $KnowledgeBaseUrl = 'https://github.com/microsoft/BCQuality/blob/main/' +) + +Set-StrictMode -Version Latest +$ErrorActionPreference = 'Stop' + +if (-not $BCQualityRoot) { + $BCQualityRoot = (Resolve-Path (Join-Path $PSScriptRoot '..')).Path +} +$BCQualityRoot = (Resolve-Path -LiteralPath $BCQualityRoot).Path +$schemaPath = Join-Path $BCQualityRoot 'schemas/findings-report.schema.json' + +$raw = Get-Content -LiteralPath $ReportPath -Raw +try { + if (-not ($raw | Test-Json -SchemaFile $schemaPath -ErrorAction Stop)) { + throw 'Report does not satisfy schemas/findings-report.schema.json.' + } + $report = $raw | ConvertFrom-Json -Depth 100 -DateKind String +} +catch { + throw "Invalid findings-report JSON or schema: $($_.Exception.Message)" +} + +function Get-OptionalValue { + param([object] $Object, [string] $Name) + if ($null -ne $Object -and $Object.PSObject.Properties.Name -contains $Name) { + return $Object.$Name + } + return $null +} + +function Get-SarifLevel { + param([string] $Severity) + switch ($Severity) { + 'blocker' { 'error' } + 'major' { 'error' } + 'minor' { 'warning' } + default { 'note' } + } +} + +function Get-ArticleTitle { + param([string] $ArticlePath) + $fullPath = Join-Path $BCQualityRoot $ArticlePath + if (-not (Test-Path -LiteralPath $fullPath -PathType Leaf)) { + return $null + } + foreach ($line in Get-Content -LiteralPath $fullPath) { + if ($line -match '^#\s+(.+)$') { + return $Matches[1].Trim() + } + } + return $null +} + +$prefix = $SourcePathPrefix.Replace('\', '/').Trim('/') +$knowledgePattern = '^(microsoft|community|custom)/knowledge/[^/]+/[^/]+\.md$' + +$rules = [Collections.Generic.List[object]]::new() +$ruleIndex = [Collections.Generic.Dictionary[string, int]]::new([StringComparer]::Ordinal) +$results = [Collections.Generic.List[object]]::new() +$notifications = [Collections.Generic.List[object]]::new() +$locationlessFindingCount = 0 + +foreach ($finding in @($report.findings)) { + $ruleId = [string]$finding.id + if (-not $ruleIndex.ContainsKey($ruleId)) { + $rule = [ordered]@{ id = $ruleId } + if ($ruleId -match $knowledgePattern) { + $rule.name = [IO.Path]::GetFileNameWithoutExtension($ruleId) + $title = Get-ArticleTitle $ruleId + if ($title) { + $rule.shortDescription = [ordered]@{ text = $title } + } + $rule.helpUri = $KnowledgeBaseUrl + $ruleId + } + else { + $rule.name = 'agent-finding' + $rule.shortDescription = [ordered]@{ text = 'Advisory finding without a BCQuality knowledge article.' } + } + $ruleIndex[$ruleId] = $rules.Count + $rules.Add($rule) | Out-Null + } + + $result = [ordered]@{ + ruleId = $ruleId + ruleIndex = $ruleIndex[$ruleId] + level = Get-SarifLevel ([string]$finding.severity) + message = [ordered]@{ text = [string]$finding.message } + } + + $location = Get-OptionalValue $finding 'location' + if ($null -ne $location) { + $file = ([string]$location.file).Replace('\', '/').TrimStart('/') + if ($prefix) { + $file = "$prefix/$file" + } + $fileUri = (($file.Split('/') | ForEach-Object { [uri]::EscapeDataString($_) }) -join '/') + $region = [ordered]@{ startLine = [int]$location.line } + $range = Get-OptionalValue $location 'range' + if ($null -ne $range) { + $region.startLine = [int]$range.'start-line' + $region.endLine = [int]$range.'end-line' + } + $result.locations = @( + [ordered]@{ + physicalLocation = [ordered]@{ + artifactLocation = [ordered]@{ uri = $fileUri; uriBaseId = 'SRCROOT' } + region = $region + } + } + ) + } + + $properties = [ordered]@{ + severity = [string]$finding.severity + confidence = [string]$finding.confidence + references = @(@($finding.references) | ForEach-Object { [string]$_.path }) + } + foreach ($name in 'domain', 'from-sub-skill', 'suggested-code', 'suggested-code-omission-reason') { + $value = Get-OptionalValue $finding $name + if ($null -ne $value) { + $properties[$name] = [string]$value + } + } + if ($null -eq $location) { + $notifications.Add([ordered]@{ + level = $result.level + message = [ordered]@{ text = "${ruleId}: $($finding.message)" } + associatedRule = [ordered]@{ id = $ruleId; index = $ruleIndex[$ruleId] } + properties = $properties + }) | Out-Null + $locationlessFindingCount++ + } + else { + $result.properties = $properties + $results.Add($result) | Out-Null + } +} + +function Add-OutcomeNotifications { + param([object] $SkillReport) + $outcome = [string]$SkillReport.outcome + if ($outcome -in @('partial', 'failed')) { + $reason = [string](Get-OptionalValue $SkillReport 'outcome-reason') + $notifications.Add([ordered]@{ + level = if ($outcome -eq 'failed') { 'error' } else { 'warning' } + message = [ordered]@{ text = "$($SkillReport.skill.id) v$($SkillReport.skill.version): $outcome. $reason".Trim() } + }) | Out-Null + } + foreach ($subResult in @(Get-OptionalValue $SkillReport 'sub-results')) { + if ($null -ne $subResult) { + Add-OutcomeNotifications $subResult + } + } +} +Add-OutcomeNotifications $report + +$invocation = [ordered]@{ + executionSuccessful = ([string]$report.outcome) -in @('completed', 'not-applicable', 'no-knowledge') +} +if ($notifications.Count -gt 0) { + $invocation.toolExecutionNotifications = @($notifications) +} + +$sarif = [ordered]@{ + '$schema' = 'https://json.schemastore.org/sarif-2.1.0.json' + version = '2.1.0' + runs = @( + [ordered]@{ + tool = [ordered]@{ + driver = [ordered]@{ + name = 'BCQuality' + informationUri = 'https://github.com/microsoft/BCQuality' + rules = @($rules) + } + } + invocations = @($invocation) + originalUriBaseIds = [ordered]@{ + SRCROOT = [ordered]@{ description = [ordered]@{ text = 'Root of the analyzed source repository.' } } + } + results = @($results) + properties = [ordered]@{ + skill = [ordered]@{ id = [string]$report.skill.id; version = [int]$report.skill.version } + outcome = [string]$report.outcome + locationlessFindingCount = $locationlessFindingCount + suppressed = @(@($report.suppressed) | ForEach-Object { + [ordered]@{ path = [string]$_.reference.path; reason = [string]$_.reason } + }) + } + } + ) +} + +$json = $sarif | ConvertTo-Json -Depth 30 +if ($OutputPath) { + Set-Content -LiteralPath $OutputPath -Value $json -Encoding utf8NoBOM +} +else { + $json +} diff --git a/tools/Test-ReviewContract.ps1 b/tools/Test-ReviewContract.ps1 index a01e283d..82137412 100644 --- a/tools/Test-ReviewContract.ps1 +++ b/tools/Test-ReviewContract.ps1 @@ -240,6 +240,118 @@ try { -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath Assert-True (-not $accepted.normalized) 'valid report is accepted without normalization' + $sarifConverter = Join-Path $Root 'tools/ConvertTo-Sarif.ps1' + Assert-True (Test-Path -LiteralPath $sarifConverter -PathType Leaf) 'SARIF converter exists' + $sarif = (& $sarifConverter -ReportPath $reportPath -BCQualityRoot $Root -SourcePathPrefix 'apps/demo') | ConvertFrom-Json -Depth 30 + $sarifRun = $sarif.runs[0] + $sarifResult = $sarifRun.results[0] + Assert-True ($sarif.version -ceq '2.1.0') 'SARIF log declares version 2.1.0' + Assert-True ($sarifRun.invocations[0].executionSuccessful) 'completed report converts to a successful invocation' + Assert-True ($sarifResult.ruleId -ceq $articlePath) 'SARIF ruleId is the knowledge article path' + Assert-True ($sarifResult.level -ceq 'warning') 'minor severity maps to SARIF warning' + Assert-True ($sarifRun.tool.driver.rules[0].helpUri.EndsWith($articlePath)) 'knowledge rule links to its article' + $sarifLocation = $sarifResult.locations[0].physicalLocation + Assert-True ($sarifLocation.artifactLocation.uri -ceq "apps/demo/$sourcePath") 'SARIF location applies the source path prefix' + Assert-True ($sarifLocation.region.startLine -eq 2 -and $sarifLocation.region.endLine -eq 3) 'SARIF region uses the finding range' + Assert-True ($sarifResult.properties.confidence -ceq 'high') 'SARIF result keeps the finding confidence' + Assert-True ($sarifRun.properties.locationlessFindingCount -eq 0) 'located-only report has zero location-less findings' + + foreach ($severity in 'blocker', 'major', 'minor', 'info') { + foreach ($includeLocatedFinding in @($true, $false)) { + $unlocatedReport = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $unlocatedFinding = $unlocatedReport.findings[0] | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $unlocatedFinding.PSObject.Properties.Remove('location') + $unlocatedFinding.severity = $severity + $unlocatedFinding.message = 'Repository-level finding without a source location.' + $unlocatedReport.findings = @( + if ($includeLocatedFinding) { $unlocatedReport.findings[0] } + $unlocatedFinding + ) + foreach ($level in 'blocker', 'major', 'minor', 'info') { + $unlocatedReport.summary.counts.$level = @($unlocatedReport.findings | Where-Object severity -eq $level).Count + } + $unlocatedReportPath = Join-Path $tmp 'unlocated-report.json' + Set-Content -LiteralPath $unlocatedReportPath -Value ($unlocatedReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $acceptedUnlocated = & $validator -ReportPath $unlocatedReportPath -BCQualityRoot $Root -SourceRoot $tmp ` + -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath + Assert-True (-not $acceptedUnlocated.normalized) 'location-less finding is accepted' + $unlocatedSarif = (& $sarifConverter -ReportPath $unlocatedReportPath -BCQualityRoot $Root) | ConvertFrom-Json -Depth 30 + $unlocatedRun = $unlocatedSarif.runs[0] + Assert-True (@($unlocatedRun.results).Count -eq [int]$includeLocatedFinding) 'only located findings are emitted as results' + foreach ($locatedResult in $unlocatedRun.results) { + Assert-True (@($locatedResult.locations).Count -gt 0) 'every emitted result has a location' + Assert-True (-not [string]::IsNullOrWhiteSpace($locatedResult.locations[0].physicalLocation.artifactLocation.uri)) 'every emitted result has a non-empty artifact URI' + } + $notification = $unlocatedRun.invocations[0].toolExecutionNotifications[0] + Assert-Contains $notification.message.text $articlePath 'location-less notification names the finding ID' + Assert-Contains $notification.message.text $unlocatedFinding.message 'location-less finding message remains visible' + $expectedLevel = switch ($severity) { 'blocker' { 'error' }; 'major' { 'error' }; 'minor' { 'warning' }; 'info' { 'note' } } + Assert-True ($notification.level -ceq $expectedLevel) 'location-less notification preserves severity mapping' + Assert-True ($notification.properties.confidence -ceq 'high') 'location-less notification retains finding properties' + Assert-True ($unlocatedRun.properties.locationlessFindingCount -eq 1) 'location-less finding count is explicit' + Assert-True ($unlocatedRun.invocations[0].executionSuccessful) 'location-less findings do not change a completed invocation outcome' + } + } + + $uriSourcePaths = @( + 'src/Price list.al' + 'src/Price #1.al' + 'src/Price %23 %25.al' + "src/$([char]0x00E8) prezzo $([char]0x65E5).al" + ) + if (-not $IsWindows) { + $uriSourcePaths += 'src/Price?1.al' + } + foreach ($uriSourcePath in $uriSourcePaths) { + $uriSourceFile = Join-Path $tmp $uriSourcePath + Set-Content -LiteralPath $uriSourceFile -Value @('line one', 'line two', 'line three') -Encoding utf8NoBOM + $uriReport = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $uriReport.findings[0].location.file = $uriSourcePath + $uriReportPath = Join-Path $tmp 'uri-report.json' + $uriReportJson = $uriReport | ConvertTo-Json -Depth 20 + Set-Content -LiteralPath $uriReportPath -Value $uriReportJson -Encoding utf8NoBOM + $acceptedUriReport = & $validator -ReportPath $uriReportPath -BCQualityRoot $Root -SourceRoot $tmp ` + -SourcePaths $uriSourcePath -RetrievedArticlePaths $articlePath + Assert-True (-not $acceptedUriReport.normalized) "source path is accepted unchanged: $uriSourcePath" + + foreach ($uriPrefix in @('', "apps/My App #1 %23 $([char]0x00E8)")) { + $uriSarif = (& $sarifConverter -ReportPath $uriReportPath -BCQualityRoot $Root -SourcePathPrefix $uriPrefix) | + ConvertFrom-Json -Depth 30 + $artifactUri = $uriSarif.runs[0].results[0].locations[0].physicalLocation.artifactLocation.uri + $expectedPath = if ($uriPrefix) { "$uriPrefix/$uriSourcePath" } else { $uriSourcePath } + $resolvedUri = [uri]::new([uri]'https://example.invalid/repository/', $artifactUri) + Assert-True ($resolvedUri.Fragment -ceq '') "source URI has no fragment: $expectedPath" + Assert-True ($resolvedUri.Query -ceq '') "source URI has no query: $expectedPath" + Assert-True ([uri]::UnescapeDataString($artifactUri) -ceq $expectedPath) "source URI decodes losslessly: $expectedPath" + Assert-True ([uri]::UnescapeDataString($resolvedUri.AbsolutePath) -ceq "/repository/$expectedPath") "source URI resolves to the original filename: $expectedPath" + Assert-True (-not $artifactUri.Contains(' ')) "source URI encodes spaces: $expectedPath" + } + Assert-True ((Get-Content -LiteralPath $uriReportPath -Raw).TrimEnd() -ceq $uriReportJson.TrimEnd()) 'SARIF conversion leaves the raw report unchanged' + } + + $failedReport = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $failedReport.outcome = 'failed' + $failedReport | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Index unavailable.' + $failedReport.findings = @() + $failedReport.summary.counts.minor = 0 + $failedReportPath = Join-Path $tmp 'failed-report.json' + Set-Content -LiteralPath $failedReportPath -Value ($failedReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $failedSarif = (& $sarifConverter -ReportPath $failedReportPath -BCQualityRoot $Root) | ConvertFrom-Json -Depth 30 + $failedInvocation = $failedSarif.runs[0].invocations[0] + Assert-True (-not $failedInvocation.executionSuccessful) 'failed report never converts to a successful invocation' + Assert-True ($failedInvocation.toolExecutionNotifications[0].level -ceq 'error') 'failed outcome is reported as an error notification' + + $partialUnlocatedReport = $unlocatedReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $partialUnlocatedReport.outcome = 'partial' + $partialUnlocatedReport.summary.coverage.'worklist-size' = 2 + $partialUnlocatedReport | Add-Member -NotePropertyName 'outcome-reason' -NotePropertyValue 'Stopped early.' + Set-Content -LiteralPath $unlocatedReportPath -Value ($partialUnlocatedReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $partialUnlocatedSarif = (& $sarifConverter -ReportPath $unlocatedReportPath -BCQualityRoot $Root) | ConvertFrom-Json -Depth 30 + $partialUnlocatedRun = $partialUnlocatedSarif.runs[0] + Assert-True (-not $partialUnlocatedRun.invocations[0].executionSuccessful) 'location-less notification never masks an incomplete outcome' + Assert-True ($partialUnlocatedRun.invocations[0].toolExecutionNotifications.Count -eq 2) 'finding and partial-outcome notifications are both retained' + Assert-True ($partialUnlocatedRun.properties.locationlessFindingCount -eq 1) 'outcome notifications are not counted as location-less findings' + $invalidCounts = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $invalidCounts.summary.counts.minor = 0 Set-Content -LiteralPath $reportPath -Value ($invalidCounts | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM