Skip to content

ci(pr): gates that cannot run now fail instead of passing (#308) #356

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Chris-Wolfgang wants to merge 1 commit into
base: security/mcp-override-ghsa-84m7
Choose a base branch
from
Open

ci(pr): gates that cannot run now fail instead of passing (#308) #356

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
96 changes: 95 additions & 1 deletion .github/workflows/pr.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -366,10 +366,33 @@ jobs:

- name: Gate on error-severity findings
run: |
# This step had no `set -e`, and that made it PASS when it could not do its job: with
# jq absent (or the SARIF missing or malformed) `count` came back EMPTY,
# `[ "" -gt 0 ]` failed with "integer expected", that failure was swallowed because it
# sat in an `if` condition, and the last statement was the "No error-severity findings"
# echo - so the step exited 0 and the gate reported clean without reading anything.
# Reproduced locally 2026-09-28 on a machine without jq. GitHub's runners do preinstall
# jq, so this was not failing in CI; it was still a gate that could not tell "clean"
# from "could not look".
set -euo pipefail

if [ ! -s inspect.sarif ]; then
echo "::error::inspect.sarif is missing or empty - InspectCode produced no report, so the gate cannot run. Treating as a failure, not a pass."
exit 1
fi
if ! command -v jq > /dev/null 2>&1; then
echo "::error::jq is not available, so the SARIF cannot be read. Treating as a failure, not a pass."
exit 1
fi

# SARIF level=error fails the job; warnings still upload (visible in
# Security → Code scanning) but don't gate merge — raise to `error`
# later when the noise floor is acceptable.
count=$(jq '[.runs[].results[] | select(.level=="error")] | length' inspect.sarif)
if ! printf '%s' "$count" | grep -qE '^[0-9]+$'; then
echo "::error::Could not read a finding count from inspect.sarif (got '$count'). Treating as a failure, not a pass."
exit 1
fi
if [ "$count" -gt 0 ]; then
echo "::error::$count InspectCode error-severity finding(s) — see Security → Code scanning"
exit 1
Expand All @@ -388,10 +411,15 @@ jobs:
needs: inspectcode
# A pull_request run from a fork (or by Dependabot) carries a read-only token that cannot write
# Code Scanning; those PRs still get the InspectCode gate above, just no SARIF upload.

# The Dependabot test reads the PR AUTHOR, not github.actor: re-running or reopening a
# Dependabot PR makes the actor a maintainer while the token stays Dependabot's read-only
# one, so an actor test would let this write-scoped job run and then fail. Every other
# Dependabot guard in this file already tests the author.
if: >-
always() && (needs.inspectcode.result == 'success' || needs.inspectcode.result == 'failure')
&& (github.event_name == 'push'
|| (github.event.pull_request.head.repo.full_name == github.repository && github.actor != 'dependabot[bot]'))
|| (github.event.pull_request.head.repo.full_name == github.repository && github.event.pull_request.user.login != 'dependabot[bot]'))
timeout-minutes: 10
permissions:
contents: read
Expand Down Expand Up @@ -1005,6 +1033,13 @@ jobs:
$testProjects | ForEach-Object { Write-Host $_.FullName -ForegroundColor White }
Write-Host ""

# The Stage 1 ledger, on this stage. #654 added it to Stage 1 only, so a test assembly
# that ran here and recorded nothing still dropped out of the coverage gate silently.
# Recorded only for projects that ran a COVERAGE-COLLECTING framework: net4x and
# netcoreapp3.1 are tested below without the XPlat collector by design, so a project
# targeting only those must not be expected to have a coverage row.
$ranAssemblies = @()

foreach ($testProj in $testProjects) {
Write-Host "==========================================" -ForegroundColor Cyan
Write-Host "Testing project: $($testProj.FullName)" -ForegroundColor Cyan
Expand Down Expand Up @@ -1095,6 +1130,7 @@ jobs:
# netcoreapp3.1 and net4x are tested but excluded from coverage:
# netcoreapp3.1 has no matching test TFM on Linux (Stage 1) so its numbers
# would not be comparable; net4x cannot use the XPlat collector on Windows.
$collectedCoverage = $false
foreach ($fw in $frameworks) {
Write-Host "Testing framework: $fw" -ForegroundColor Yellow

Expand All @@ -1114,6 +1150,7 @@ jobs:
--settings coverlet.runsettings `
--results-directory "./TestResults" `
--logger "console;verbosity=normal" 2>&1 | Tee-Object -FilePath $testLog
$collectedCoverage = $true
} else {
dotnet test $testProj.FullName `
--configuration Release `
Expand All @@ -1133,15 +1170,33 @@ jobs:
exit 1
}
}

if ($collectedCoverage) {
$asmMatch = [regex]::Match((Get-Content $testProj.FullName -Raw), '<AssemblyName>([^<]+)</AssemblyName>')
$ranAssemblies += if ($asmMatch.Success) { $asmMatch.Groups[1].Value.Trim() } else { $testProj.BaseName }
}
Write-Host ""
}

$ranFile = Join-Path $env:RUNNER_TEMP 'stage2-ran-test-assemblies.txt'
Set-Content -Path $ranFile -Value $ranAssemblies
Write-Host "Recorded $($ranAssemblies.Count) test assembly/assemblies with coverage: $($ranAssemblies -join ', ')"

- name: Check for coverage files
id: check-coverage
run: |
if (Get-ChildItem -Path TestResults -Recurse -Filter coverage.cobertura.xml -ErrorAction SilentlyContinue) {
echo "has-coverage=true" >> $env:GITHUB_OUTPUT
Write-Host "✅ Coverage files found"
} elseif ((Test-Path (Join-Path $env:RUNNER_TEMP 'stage2-ran-test-assemblies.txt')) -and
@(Get-Content (Join-Path $env:RUNNER_TEMP 'stage2-ran-test-assemblies.txt') | Where-Object { $_.Trim() }).Count -gt 0) {
# Do NOT skip. Every later coverage step is gated on has-coverage == 'true', so a
# dropped coverlet.collector reference used to skip the whole coverage gate and report
# green. #654 closed this on Stage 1; this stage kept shrugging. Mirrors Stage 1's
# wording: tests ran, so absence of coverage means collection broke.
Write-Host "::error::Tests ran but produced no coverage files - the coverage gate cannot be evaluated. Test assemblies that ran:"
Get-Content (Join-Path $env:RUNNER_TEMP 'stage2-ran-test-assemblies.txt')
exit 1
} else {
echo "has-coverage=false" >> $env:GITHUB_OUTPUT
Write-Host "ℹ️ No coverage files found - skipping coverage report generation"
Expand All @@ -1162,6 +1217,45 @@ jobs:
-targetdir:"CoverageReport" `
-reporttypes:"Html;TextSummary;MarkdownSummaryGithub;CsvSummary"

- name: Every test assembly that ran must be in the coverage report
# The coverage gate can only judge modules that APPEAR in the report, so an assembly that
# ran and recorded nothing drops out of it and the gate passes while measuring less than
# it did yesterday. #654 added this check to Stage 1; this is the same check for the
# stage that runs a different subset of the suite.
if: steps.check-coverage.outputs.has-coverage == 'true'
shell: pwsh
run: |
$ranFile = Join-Path $env:RUNNER_TEMP 'stage2-ran-test-assemblies.txt'
if (-not (Test-Path $ranFile)) {
Write-Host "::error::The ran-assemblies ledger is missing, so the row check cannot run."
exit 1
}
$ran = @(Get-Content $ranFile | Where-Object { $_.Trim() } | Sort-Object -Unique)
if ($ran.Count -eq 0) {
Write-Host "::error::Coverage files exist but no test assembly was recorded as having collected coverage - refusing to pass a check that measured nothing."
exit 1
}
if (-not (Test-Path 'CoverageReport/Summary.txt')) {
Write-Host "::error::CoverageReport/Summary.txt is missing, so the row check cannot run."
exit 1
}

# Module rows start in column 1; every other line in Summary.txt is indented.
$rows = @(Get-Content 'CoverageReport/Summary.txt' |
Where-Object { $_ -match '^\S' } |
ForEach-Object { ($_ -split '\s+')[0] })

$missing = @($ran | Where-Object { $rows -notcontains $_ })
foreach ($asm in $ran) {
if ($missing -contains $asm) { Write-Host " FAIL $asm ran but recorded no coverage" }
else { Write-Host " OK $asm has a coverage row" }
}
if ($missing.Count -gt 0) {
Write-Host "::error::These test assemblies ran but produced no coverage row: $($missing -join ', '). Is coverlet.collector still referenced?"
exit 1
}
Write-Host "Every test assembly that collected coverage is present in the report."

- name: Enforce 90% coverage threshold
if: steps.check-coverage.outputs.has-coverage == 'true'
shell: pwsh
Expand Down
Loading