Repository navigation
ci(security): OSV-Scanner gate and weekly report, plus a GitHub button in the title bar - #108
Conversation
…rity A third merge gate in security.yml, beside Semgrep and Dependency review, and a second weekly job in audit.yml, beside the RustSec check. On a pull request the gate scans the merge commit and the base it was built on with the same arguments, and blocks on a vulnerability the pull request brings in. One already on the base is left to the weekly run, which blocks on everything it finds and uploads SARIF to code scanning, so a finding stays in the Security tab until a later run no longer reports it. The verdict comes from .github/scripts/osv_gate.py, not from the scanner's exit code, for the reason security.yml gives for Semgrep. Measured on 2026-10-14 with osv-scanner 2.7.0: a clean scan and a scan that read nothing write the same empty report. The scans run with --all-packages, and the gate checks that crates.io, NuGet and PyPI were each read with at least a floor of packages (today 51, 59 and 1). The scanner is the published binary, pinned to a version and its checksum as cargo-deny is, because Google's actions pull a container by a tag. The check name is ours, so a required check in the ruleset does not depend on a job name in someone else's workflow. --no-resolve, measured: by default the scanner guesses what pip installs under the Semgrep pin and reported eleven vulnerabilities against anyio 4.9.0, idna 3.9.0 and python-multipart 0.0.9. The Semgrep job's log shows pip installing 4.15.1, 3.20 and 0.0.32, which the scanner reports clean. With the flag it reads what the files declare. The packages under the Semgrep pin are therefore not covered by this gate, which is said in the script. Measured before the change on a clean export of main: Cargo.lock and the NuGet lock files report nothing. A canary lock file with anyio 4.9.0 is caught. Nineteen checks of the gate script against synthetic and real reports, and nine rollback probes, each red in the checks it was meant to turn red (floors ignored, aliases ignored, version or name case in the package key, identifiers matched across packages, the base ignored, an unreadable report passing, the checksum unchecked, nothing ever blocking). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📜 Recent review details
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Tests For Changed Behavior |  |
The PR adds non-UI runtime behavior in .github/scripts/osv_gate.py: scanner installation, report parsing, package-count floors, vulnerability inheritance, and CLI exit codes. The workflows invoke th… |
Add committed automated tests for .github/scripts/osv_gate.py. Cover clean reports, each ecosystem floor, blocking findings, inherited OSV IDs and aliases with ecosystem/name matching, mismatches that must block, unreadable or invalid rep… |
| Safe File Parsing | |
The PR adds load() with unbounded json.load(handle) at .github/scripts/osv_gate.py:97-99. It then assumes a valid object schema in packages() and findings() (report.get, result.get, and … |
Replace json.load() with bounded reading, such as handle.read(MAX_REPORT_BYTES + 1) followed by json.loads(), and reject oversized input. Validate the report object and every traversed results, source, packages, package, `vuln… |
| Clear User-Facing Text | |
New user-facing error output at .github/scripts/osv_gate.py:206-209 prints the caught exception directly (%s), which can expose raw OSError or JSON parse details. It does not tell the user how t… |
Report a clear action without interpolating the exception, for example: Cannot read JSON report '<path>'. Ensure the scan produced a valid JSON report, then rerun the scan. Handle the new and old paths separately so the failing report is … |
| Scope, Duplication And Docs | |
The feature scope matches the title and PR description, and the new OSV gate follows the repository's separate gate-script pattern. However, the PR changes security behavior and permissions without up… | Update SECURITY.md to document the OSV pull-request gate, the weekly OSV report, SARIF reporting, and the additional security-events: write job. Correct the workflow/job permission count and the dependency-check description. Add a `CHAN… |
-
Check name Status Explanation Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. No Secrets Or Debug Leftovers PASS. The PR changes only .github/scripts/osv_gate.pyand two workflow files. It adds noCLAUDE.md,AGENTS.md,.claude/, or.envpath. Added-content scans found no credentials, personal emai…No Hardcoded Ui Styling The pull request changes only a Python CI script and GitHub Actions workflow files. It does not add or modify XAML, Slint, Fyne, Tkinter, or WPF UI code. The hardcoded UI styling check is not applicab… No Obvious Performance Problems No clear performance problem is introduced. The new Python report processing makes linear passes over scanner results and uses dictionary/set lookups. The workflows run the scanner as bounded CI jobs,… Desktop Robustness PASS: The PR changes only GitHub Actions workflows and .github/scripts/osv_gate.py; it adds no desktop window, settings, culture-sensitive formatting, elevation, destructive action, or background UI…System Changes Are Reversible The pull request does not modify the system-state categories covered by this check. It downloads a scanner to the GitHub Actions runner temp directory, writes a report, changes the downloaded file mod… No Resource Leaks No resource leak is introduced. urllib.request.urlopen(...)is used in a context manager, and both downloaded-binary and report files are opened withwith, so handles close on success and errors. …Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly identifies the OSV-Scanner merge gate and weekly report, which are the main changes. The reference to a GitHub button in the title bar is not supported by the changeset, but the titl…
Full details: Tests For Changed Behavior
-
Explanation
The PR adds non-UI runtime behavior in
.github/scripts/osv_gate.py: scanner installation, report parsing, package-count floors, vulnerability inheritance, and CLI exit codes. The workflows invoke this behavior as merge and weekly gates. The authoritative diff changes only.github/scripts/osv_gate.pyand two workflow files; it adds or updates no test file. The PR description claims manual checks, but it does not add executable tests covering this behavior. No existing tests were deleted or weakened.Resolution
Add committed automated tests for
.github/scripts/osv_gate.py. Cover clean reports, each ecosystem floor, blocking findings, inherited OSV IDs and aliases with ecosystem/name matching, mismatches that must block, unreadable or invalid reports, CLI exit codes, and checksum failures. Run these tests in CI.
Full details: Safe File Parsing
-
Explanation
The PR adds
load()with unboundedjson.load(handle)at.github/scripts/osv_gate.py:97-99. It then assumes a valid object schema inpackages()andfindings()(report.get,result.get, andentry.get) at lines 104-128. Valid JSON with the wrong shape, such as[],null, or a scalarresultsvalue, raisesAttributeErroroutside themain()exception handler. A sufficiently large scanner report can also cause excessive memory use because no input-size limit exists. This parser is new in the PR and is used by both workflow verdicts.Resolution
Replace
json.load()with bounded reading, such ashandle.read(MAX_REPORT_BYTES + 1)followed byjson.loads(), and reject oversized input. Validate the report object and every traversedresults,source,packages,package,vulnerabilities, and vulnerability value before traversal. Convert schema failures toValueErrorsomain()returns its existing failure code instead of crashing. A pinned JSON-schema validator is suitable if the dependency is acceptable; otherwise use explicit type checks.
Full details: Clear User-Facing Text
-
Explanation
New user-facing error output at
.github/scripts/osv_gate.py:206-209prints the caught exception directly (%s), which can expose rawOSErroror JSON parse details. It does not tell the user how to recover. The same OSV job also has inconsistent labels:OSV advisoriesinaudit.yml:42andOSV-Scannerinsecurity.yml:109.Resolution
Report a clear action without interpolating the exception, for example:
Cannot read JSON report '<path>'. Ensure the scan produced a valid JSON report, then rerun the scan.Handle the new and old paths separately so the failing report is identified. Add an action to checksum failures, such as verifying the pinned release asset and checksum. Use one job label in both workflows, preferablyOSV-Scanner.
Full details: Scope, Duplication And Docs
-
Explanation
The feature scope matches the title and PR description, and the new OSV gate follows the repository's separate gate-script pattern. However, the PR changes security behavior and permissions without updating repository documentation.
security.ymladds theOSV-Scannerpull-request gate, andaudit.ymladds the weekly OSV report withsecurity-events: write.SECURITY.mdis unchanged and still says that only two jobs raise permissions (lines 195-199) and describes only the fourcargo-denychecks (lines 205-209).CHANGELOG.mdis also unchanged.Resolution
Update
SECURITY.mdto document the OSV pull-request gate, the weekly OSV report, SARIF reporting, and the additionalsecurity-events: writejob. Correct the workflow/job permission count and the dependency-check description. Add aCHANGELOG.mdentry if CI security changes are release-noted by project policy.
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
CodeRabbit's review found no inline issues and raised four pre-merge warnings. Each was checked against the code, three were taken. - Tests for the gate script. The nineteen checks and nine rollback probes behind the first commit lived outside the repository. They are now .github/scripts/test_osv_gate.py (stdlib unittest, sixteen tests on reports shaped as osv-scanner 2.7.0 writes them), and both OSV jobs run it before they scan, so a gate that stopped blocking fails as itself. - A report of the wrong shape. It used to end in a traceback, red but unexplained. It is refused now with exit code 2, naming the first part that is not shaped as the scanner writes it. A field that is absent or null means none, any other wrong type is refused, because an empty dict and an empty list are both falsy and must not read as empty. The size limit the warning asked for is not added: the report is not outside input, the pinned scanner writes it a step earlier in the same job. - The failure message names which report could not be read (the pull request's, the base's or the tree's) and says where to look. The exception text stays, because this is a CI log and that is the cause. - SECURITY.md said five workflows and two jobs with raised permissions. Measured on this branch: nine workflows, all with contents: read at the top, and five jobs that raise it. The paragraph now says so, and the dependency checks name the dependency review, OSV on pull requests and the weekly RustSec and OSV runs. No CHANGELOG entry for the gate, as for Semgrep and Dependency review before it: it changes CI, not the product. Rollback probes against the committed tests, each measured: floors ignored, aliases ignored, version in the package key, name case kept, identifiers matched across packages, the base ignored, an unreadable report passing, the shape unchecked, a falsy field read as missing, the base not named, the checksum unchecked, nothing ever blocking. Twelve of twelve red, each in the tests it was meant to turn red. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The window's title bar carries a third action between Donate and About: a code mark and the word GitHub. It hands the repository address to the shell exactly as Donate hands the support page, with the same promise in the tooltip and the same failure notice when Windows has no browser for the address. The address is a constant in ExternalLinks, like every other one the application can open. The mark is E943 "Code" from Segoe MDL2 Assets, the icon face the window already uses. It is referenced by name and never shipped: Microsoft's font redistribution FAQ lets any Windows application render the system fonts, the symbol fonts included, and forbids only copying or converting the files. Nothing is added to the component register. What the two links share is decided once. The mark and the word beside it are named styles (TitleBarActionMark, TitleBarActionIcon, TitleBarActionLabel), the glyphs are values (GlyphHeart, GlyphCode), and one handler shows the failure notice for both. The title-bar button style gains the disabled state it never had, and the component catalogue gains the title-bar action it never showed, in every state with a long label. Texts, accepted before the code: the label GitHub in every language, the name "Chrono Mock on GitHub" / "Chrono Mock na GitHubie", and a tooltip that names the address and what is there. README and the FAQ in all 22 languages said the window has one control that leads outside it, which stops being true here, and now name both. The network register's reason for the shell-open file names all three addresses it can open. Measured on the render rather than read off the markup: the three actions stand in order and fit at the window's narrowest width, the code mark is a glyph its face actually has, both links keep the same gap between mark and word, and a disabled action is drawn at the disabled opacity. Live UIA on the dev window reads the button's name and tooltip. Rollback probes, each measured: the glyph swapped for one the face lacks, the icon face dropped, an own gap on one label, the address changed, the disabled state removed, the button renamed, the destination dropped. Seven of seven red, each in the tests it was meant to turn red. C# tests 874, all passing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
donislawdev
changed the title
What
OSV-Scanner joins the vulnerability checks twice:
security.yml(OSV-Scanner), beside Semgrep and Dependency review. It scans the pull request's merge commit and the base that merge was built on, with the same arguments, and blocks on a vulnerability the pull request brings in.audit.yml(OSV advisories), beside the RustSec check. It scans the tree, uploads SARIF to code scanning (categoryosv-scanner), and fails on anything it finds. A finding stays in the Security tab until a later run no longer reports it.A vulnerability already on the base does not block a pull request that did not add it: that is what the weekly run is for. A version bump that keeps a package vulnerable is not "brought in", and one that makes it vulnerable to something else is. Identity is the OSV id or any alias (CVE, GHSA) against the same package.
Why it is built this way
.github/scripts/osv_gate.py), as with Semgrep. Measured with osv-scanner 2.7.0: a clean scan and a scan that read nothing write the same"results": []. The scans run with--all-packages, and the gate checks that crates.io, NuGet and PyPI were each read with at least a floor of packages (today 51, 59 and 1). A scan exit code above one fails the step.cargo-deny.ps1does. Google's actions pull a container by a tag, which pins nothing. The check name is ours, so a required check in the ruleset does not depend on a job name in someone else's reusable workflow. Dependabot cannot see this pin, so a newer scanner is taken by hand (stated in the script).--no-resolve, measured. By default the scanner guesses what pip installs under the Semgrep pin and reported eleven vulnerabilities against anyio 4.9.0, idna 3.9.0 and python-multipart 0.0.9. The Semgrep job's own log shows pip installing 4.15.1, 3.20 and 0.0.32, which the same scanner reports clean. With the flag it reads what the files declare.Cargo.lockand the NuGet lock files are complete, so nothing changes for them.Not covered, said plainly
OSV-Scanneris a required check in the branch ruleset. That is added after this pull request's first run shows the exact check name.Evidence
main(git archive), scanned locally with osv-scanner 2.7.0 (checksum checked against the release):Cargo.lock(51 packages) and the NuGet files (59) report nothing.hygiene,names,shape,componentsguards green. Both workflows parse.Also in this pull request: a GitHub button in the title bar (
77899f0)Added here rather than in a pull request of its own, to keep within the review limit. This commit came after CodeRabbit's review, which is not incremental, so no automated review has read it.
E943"Code", Segoe MDL2 Assets) and the word GitHub. It handshttps://github.com/donislawdev/ChronoMockto the shell exactly as Donate hands the support page, with the same promise in the tooltip and the same failure notice. The address is a constant inExternalLinks.TitleBarActionMark,TitleBarActionIcon,TitleBarActionLabel), the glyphs are values (GlyphHeart,GlyphCode), one handler shows the failure notice for both links. The title-bar button style gains a disabled state, and the component catalogue gains the title-bar action.Review round (
43c7290)CodeRabbit's review of
e403fa5: no inline comments, LGTM on all three files, four pre-merge warnings. Each was checked against the code..github/scripts/test_osv_gate.py(stdlibunittest, 16 tests on reports shaped as osv-scanner 2.7.0 writes them), and both OSV jobs run it before they scan. 12 of 12 rollback probes red in the intended tests.SECURITY.md, not forCHANGELOG.md.SECURITY.mdsaid five workflows and two jobs with raised permissions. Measured: nine workflows, allcontents: readat the top, five jobs that raise it, now stated, and the dependency checks name OSV. No changelog entry for the gate itself, as for Semgrep and Dependency review: it changes CI, not the product. The button has one.🤖 Generated with Claude Code
Summary by CodeRabbit