From 7726d5d8d8dbcb146489f0e65e76e9cc9b976db0 Mon Sep 17 00:00:00 2001 From: Wenjie Fan <31087545+gggdttt@users.noreply.github.com> Date: Mon, 5 Oct 2026 15:01:04 +0200 Subject: [PATCH] Harden findings-report producer constraints (#218) Reject fragment-bearing finding IDs and cap uncited confidence and severity in the shared schema. Require final-source verification before emission and cover leaf/root compatibility and fail-closed source bounds. Co-authored-by: wenjiefan Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- schemas/findings-report.schema.json | 13 ++- skills/do.md | 24 +++++- tools/Test-ReviewContract.ps1 | 121 +++++++++++++++++++++++++++- 3 files changed, 153 insertions(+), 5 deletions(-) diff --git a/schemas/findings-report.schema.json b/schemas/findings-report.schema.json index 76712f4..f0cfd35 100644 --- a/schemas/findings-report.schema.json +++ b/schemas/findings-report.schema.json @@ -122,7 +122,7 @@ "additionalProperties": false, "required": ["id", "severity", "message", "references", "confidence"], "properties": { - "id": { "type": "string", "minLength": 1 }, + "id": { "type": "string", "minLength": 1, "pattern": "^[^#]+$" }, "severity": { "enum": ["blocker", "major", "minor", "info"] }, "message": { "type": "string", "minLength": 1 }, "location": { "$ref": "#/definitions/location" }, @@ -135,6 +135,17 @@ "domain": { "type": "string", "minLength": 1, "pattern": "^[^\\r\\n]+$" }, "suggested-code": { "type": "string", "minLength": 1 }, "suggested-code-omission-reason": { "type": "string", "minLength": 1 } + }, + "if": { + "properties": { + "references": { "maxItems": 0 } + } + }, + "then": { + "properties": { + "confidence": { "enum": ["medium", "low"] }, + "severity": { "enum": ["minor", "info"] } + } } }, "suppressed": { diff --git a/skills/do.md b/skills/do.md index e5aef5b..a10e5b6 100644 --- a/skills/do.md +++ b/skills/do.md @@ -128,8 +128,8 @@ source-scope locations, and article-body retrieval. "message": "string", "location": { "file": "string", - "line": 0, - "range": { "start-line": 0, "end-line": 0 } + "line": 1, + "range": { "start-line": 1, "end-line": 1 } }, "references": [ { "path": "string", "sha": "string" } @@ -165,6 +165,26 @@ The emitted document MUST be strict, valid JSON per [RFC 8259](https://www.rfc-e AL source is the common failure case. Quoted identifiers (for example `Rec."No."`) and multi-line snippets routinely appear in `message`, `suggested-code`, and `suggested-code-omission-reason`, and each embedded quote or newline MUST be escaped when placed in a string value. A `suggested-code` payload that spans several lines is a single JSON string with `\n` separators, not a literal multi-line block. Emit the document as one JSON value with no trailing commentary, and do not rely on the consumer to repair unescaped output. +### Producer pre-emission checklist + +Before emitting each leaf report or super-skill rollup: + +1. Copy every citation-based `findings[].id` verbatim from + `references[0].path`, with no `#` fragment or other suffix. Apply the + reference-integrity gate below. +2. For `references: []`, emit only `confidence: "medium"` or `"low"` and + `severity: "minor"` or `"info"`. Preserve the role-specific agent ID + prefixes defined below. +3. Open the final source snapshot for every `location.file`. Verify `line` + and any inclusive range bounds are 1-based final-file line numbers within + that file's length, never diff/patch-relative line numbers. If the source + snapshot cannot be verified, return `outcome: "failed"` with an + `outcome-reason`, not unverified locations. + +Validate the complete document against the schema and semantic rules before +returning it. Consumers MUST NOT strip ID suffixes, downgrade agent findings, +or clamp locations to make an invalid report pass the acceptance gate. + ### Consumer acceptance gate Capture the exact Task return as the immutable raw audit payload and primary diff --git a/tools/Test-ReviewContract.ps1 b/tools/Test-ReviewContract.ps1 index db1c1b1..a01e283 100644 --- a/tools/Test-ReviewContract.ps1 +++ b/tools/Test-ReviewContract.ps1 @@ -56,6 +56,14 @@ function Assert-ThrowsLike { throw "Expected error like '$Pattern', but no error was thrown." } +function Assert-ReportSchema { + param([object] $Report, [bool] $Expected, [string] $Message) + + $valid = $Report | ConvertTo-Json -Depth 30 | + Test-Json -SchemaFile (Join-Path $Root 'schemas/findings-report.schema.json') -ErrorAction SilentlyContinue + Assert-True ($valid -eq $Expected) $Message +} + function Test-PositiveInteger { param([object] $Value) @@ -116,6 +124,11 @@ foreach ($surface in @( $normalizedDoContract = $doContract -replace '\s+', ' ' foreach ($expected in @( + 'Copy every citation-based `findings[].id` verbatim from `references[0].path`, with no `#` fragment or other suffix.', + 'For `references: []`, emit only `confidence: "medium"` or `"low"` and `severity: "minor"` or `"info"`.', + 'Open the final source snapshot for every `location.file`.', + '1-based final-file line numbers within that file''s length, never diff/patch-relative line numbers.', + 'Consumers MUST NOT strip ID suffixes, downgrade agent findings, or clamp locations', 'positive integers', 'start-line <= line <= end-line', 'does not contain the `suggested-code` field', @@ -286,6 +299,66 @@ try { $acceptedSuper = & $validator -ReportPath $reportPath -BCQualityRoot $Root -SkillKind super Assert-True (-not $acceptedSuper.normalized) 'valid super-skill report is accepted' + foreach ($isAgent in @($false, $true)) { + foreach ($severity in 'blocker', 'major', 'minor', 'info') { + foreach ($confidence in 'high', 'medium', 'low') { + $schemaLeaf = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $schemaLeaf.findings[0].severity = $severity + $schemaLeaf.findings[0].confidence = $confidence + $schemaLeaf.summary.counts.minor = 0 + $schemaLeaf.summary.counts.$severity = 1 + if ($isAgent) { + $schemaLeaf.findings[0].id = 'agent:uncited-defect' + $schemaLeaf.findings[0].references = @() + } + $expected = -not $isAgent -or ($severity -in @('minor', 'info') -and $confidence -ne 'high') + $caseName = "agent=$isAgent severity=$severity confidence=$confidence" + Assert-ReportSchema $schemaLeaf $expected "leaf schema: $caseName" + + $schemaSuper = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $schemaSuper.'sub-results'[0] = $schemaLeaf + $schemaSuper.summary.counts = $schemaLeaf.summary.counts + $schemaSuper.findings = @($schemaLeaf.findings[0] | ConvertTo-Json -Depth 20 | ConvertFrom-Json) + $schemaSuper.findings[0] | Add-Member -NotePropertyName 'from-sub-skill' -NotePropertyValue 'al-style-review' + if ($isAgent) { + $schemaSuper.findings[0].id = "al-style-review:$($schemaLeaf.findings[0].id)" + } + Assert-ReportSchema $schemaSuper $expected "rolled-up schema: $caseName" + + if ($isAgent) { + $schemaSuper.'sub-results'[0] = $completedLeaf + $schemaSuper.findings[0].id = $schemaLeaf.findings[0].id + $schemaSuper.findings[0].'from-sub-skill' = 'agent' + $schemaSuper.findings[0].domain = 'Agent' + Assert-ReportSchema $schemaSuper $expected "root-owned agent schema: $caseName" + + $schemaSuper.findings = @() + $schemaSuper.summary.counts = $completedLeaf.summary.counts + $schemaSuper.'sub-results'[0] = $schemaLeaf + Assert-ReportSchema $schemaSuper $expected "nested leaf schema: $caseName" + } + } + } + } + + $citationSuper = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $citationSuper.'sub-results'[0] = $validReport + $citationSuper.summary.counts = $validReport.summary.counts + $citationSuper.findings = @($validReport.findings[0] | ConvertTo-Json -Depth 20 | ConvertFrom-Json) + $citationSuper.findings[0] | Add-Member -NotePropertyName 'from-sub-skill' -NotePropertyValue 'al-style-review' + foreach ($position in 'leaf', 'root', 'nested-leaf') { + $fragmentReport = $(if ($position -eq 'leaf') { $validReport } else { $citationSuper }) | + ConvertTo-Json -Depth 20 | ConvertFrom-Json + $fragmentFinding = if ($position -eq 'nested-leaf') { + $fragmentReport.'sub-results'[0].findings[0] + } + else { + $fragmentReport.findings[0] + } + $fragmentFinding.id = "$articlePath#location" + Assert-ReportSchema $fragmentReport $false "$position citation id cannot append a fragment" + } + $duplicateLeafReport = $validSuperReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json $duplicateLeafReport.'sub-results' = @($completedLeaf, $completedLeaf) Set-Content -LiteralPath $reportPath -Value ($duplicateLeafReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM @@ -897,8 +970,52 @@ try { $invalidAgent.findings[0].references = @() $invalidAgent.findings[0].confidence = 'high' Set-Content -LiteralPath $reportPath -Value ($invalidAgent | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM - Assert-ThrowsLike -Pattern '*AGENT_CONFIDENCE_INVALID*' -Action { - & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp -SourcePaths $sourcePath + $invalidAgentRaw = [IO.File]::ReadAllText($reportPath) + Assert-ThrowsLike -Pattern '*Invalid findings-report JSON or schema*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp -SourcePaths $sourcePath ` + -AllowBoundedNormalization + } + Assert-True ([IO.File]::ReadAllText($reportPath) -ceq $invalidAgentRaw) 'invalid agent payload is not silently repaired' + + $mismatchedCitation = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $mismatchedCitation.findings[0].id = "${articlePath}:location" + Assert-ReportSchema $mismatchedCitation $true 'cross-field citation equality still requires semantic validation' + Set-Content -LiteralPath $reportPath -Value ($mismatchedCitation | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + Assert-ThrowsLike -Pattern '*PRIMARY_REFERENCE_MISMATCH*' -Action { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp ` + -SourcePaths $sourcePath -RetrievedArticlePaths $articlePath -AllowBoundedNormalization + } + + $finalSourcePath = 'src/final-snapshot.al' + Set-Content -LiteralPath (Join-Path $tmp $finalSourcePath) -Value (1..22 | ForEach-Object { "line $_" }) -Encoding utf8NoBOM + foreach ($bounds in @( + @{ Line = 22; End = 22; Error = $null } + @{ Line = 44; End = $null; Error = '*SOURCE_LINE_INVALID*' } + @{ Line = 22; End = 44; Error = '*SOURCE_RANGE_INVALID*' } + )) { + $locationReport = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json + $locationReport.findings[0].location = @{ + file = $finalSourcePath + line = $bounds.Line + } + if ($bounds.End) { + $locationReport.findings[0].location.range = @{ 'start-line' = $bounds.Line; 'end-line' = $bounds.End } + } + Assert-ReportSchema $locationReport $true 'schema alone cannot verify final-file line bounds' + Set-Content -LiteralPath $reportPath -Value ($locationReport | ConvertTo-Json -Depth 20) -Encoding utf8NoBOM + $locationRaw = [IO.File]::ReadAllText($reportPath) + $validateLocation = { + & $validator -ReportPath $reportPath -BCQualityRoot $Root -SourceRoot $tmp ` + -SourcePaths $finalSourcePath -RetrievedArticlePaths $articlePath -AllowBoundedNormalization + } + if ($bounds.Error) { + Assert-ThrowsLike -Pattern $bounds.Error -Action $validateLocation + } + else { + $acceptedLocation = & $validateLocation + Assert-True (-not $acceptedLocation.normalized) 'the final source line is accepted without normalization' + } + Assert-True ([IO.File]::ReadAllText($reportPath) -ceq $locationRaw) 'source locations are never clamped in the raw payload' } $normalizable = $validReport | ConvertTo-Json -Depth 20 | ConvertFrom-Json