Conversation
This was referenced Sep 26, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
On 2026-09-13 a security pull request claimed it remediated 76 advisories. A rough sum by package name came to 87. Both numbers were arrived at honestly and only one was right.
The difference was precisely the set a careless count sweeps in: packages already resolved on the integration branch, a rejected major, a major-only advisory. The exclusions are what made the final figure credible - not the total.
Two facts were nearly reported wrongly that day and both must survive into any output: advisories attach to the DEFAULT branch, so a fix merged into an integration branch closes NONE of them until release; and a figure that is merely consistent with a plausible total is not a verified figure.
What Changed
bin/fm-alert-count.py <base-ref> <head-ref>. It pulls the live open Dependabot alerts for origin's GitHub repo throughgh-axi(all pages) and checks each alert'spackage-lock.jsonat both refs against every npm vulnerable range in the advisory, including copies installed under an alias name. An alert counts as remediated only when base has a vulnerable copy and head has none. Other alerts go into a per-alert exclusion list with a reason: already resolved on base, head reintroduces a vulnerable copy, no patched version published, the patch needs a rejected major, or head never reaches a patched version.packagesobject, non-semver or prerelease installed versions, unparseable ranges or patched versions, and malformed alert records are listed underNOT CHECKEDand make the script exit 1. They are never counted. Every non-empty run also prints aDEFAULT-BRANCH CAVEAT: advisories attach to the default branch, so fixes on a branch close none of them until they are released there. Output is empty only when the live alert list is empty.tests/fm-alert-count.test.shwith a stubbedgh-axiand fixture repos. It covers the verified count and exclusions, the silent empty list, a failed alert fetch that produces no count, and the unverifiable-evidence cases. The test is registered inbin/fm-test-run.sh, and the tool has a new row indocs/scripts.md.🤖 Generated with Claude Code
Risk Assessment
✅ Low: This is a new read-only script that fails closed. Every prior-round fix was checked in the current code: alias copies, zero-only partial bounds, the reintroduced-copy label, the malformed-record
#N, and removal of the duplicate section. The gh-axi output contract was confirmed from source: no line wrapping, a 10MB buffer limit, and a non-zero exit on error. The only issue left is a docs wording mislabel.Testing
I drove the CLI live with real gh-axi 0.1.35 and the real Dependabot API. The main run used a private work repo's 44 open alerts: I made local branches with lockfile-only edits (nothing pushed) and ran three ref pairs. All 44 verdicts matched what I derived by hand from the published advisory ranges. The branch counted 7 of the 21 alerts on packages it bumped. The run separately labeled already resolved on integration, rejected major, reintroduced, alias and nested copies left behind, and a bump that lands inside another advisory range. It sent pipuv.lockalerts and a missing lockfile to NOT CHECKED with exit 1, and printed the default-branch caveat every time. The 44-alert--fullresponse parsed without truncation. Real prerelease range bounds (>= 5.0.0-beta.1, < 5.0.0-rc.2) and<=bounds also parsed live. Other live checks: a forbidden alert list and a non-GitHub origin both went to NOT CHECKED with exit 1, a repo with no open alerts gave silent output and exit 0, and real multi-page gh-axi output (2 pages) parsed correctly. That paging check used a different endpoint, because the alert endpoint fit on one page. Some labels never came up live: no patched version published, unparseable or partial range, prerelease install, no packages object, malformed record, and a complete exit-0 count. Those two scenarios are marked untested here; the executable test covers them and passed through the project runner, and the coverage guard is ok. The work repo's name, npm package names and product paths are replaced with stable tokens in evidence files 04 and 05, because evidence goes to a public branch. Line shapes are unchanged. This is a CLI, so there is no visual surface; the evidence is CLI transcripts.fm-alert-count.py integration security)fm-alert-count.py main maingives VERIFIED BRANCH REMEDIATION COUNT: 0 (none)integration nolocklists 6 api alerts as 'unavailable at nolock', exit=1bin/fm-test-run.sh tests/fm-alert-count.test.shwith a PATH gh-axi stub, not the live product. No readable live repo pub…bin/fm-test-run.sh tests/fm-alert-count.test.sh(test_verified_remediation_and_exclusions_are_explicit, test_unverifiabl…Evidence: Live: inaccessible alert list goes to NOT CHECKED, exit 1
Source: Live: inaccessible alert list goes to NOT CHECKED, exit 1
Evidence: Live: non-GitHub origin goes to NOT CHECKED, exit 1
Source: Live: non-GitHub origin goes to NOT CHECKED, exit 1
Evidence: Live: real gh-axi multi-page TOON body parsed into per-page base64
Source: Live: real gh-axi multi-page TOON body parsed into per-page base64
Evidence: Live: 44 real alerts, verified count 7 with labeled exclusions, caveat and NOT CHECKED (redacted)
Source: Live: 44 real alerts, verified count 7 with labeled exclusions, caveat and NOT CHECKED (redacted)
Evidence: Live: empty open alert list prints nothing, exit 0 (redacted)
Source: Live: empty open alert list prints nothing, exit 0 (redacted)
Evidence: Verified count and caveat excerpt (redacted)
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
bin/fm-alert-count.py:151-version_statedecides safe/vulnerable only by checkingversion >= first_patched_version. It never reads the alert'ssecurity_vulnerability.vulnerable_version_range, and it ignores the advisory's other ranges insecurity_advisory.vulnerabilities. GitHub's alertsecurity_vulnerabilityholds ONE vulnerable range, so a version on a later, still-vulnerable release line counts as patched. That lets an unverified remediation into the VERIFIED count without any error, which is exactly what the intent forbids: 'a figure that is merely consistent with a plausible total is not a verified figure'. Concrete trace: an advisory forsemverhas ranges<5.7.2(patched 5.7.2) and>=7.0.0 <7.5.2(patched 7.5.2). The alert carries the<5.7.2range. Base lock has semver 5.7.1 and head has 7.0.0.requires_major_upgrade([5.7.1], 5.7.2)is False, base is vulnerable, and head 7.0.0 >= 5.7.2 comes back 'safe', so the alert is counted as remediated even though 7.0.0 is still vulnerable and the alert stays open after release. The same happens when a lock holds several copies (5.7.1 and 7.3.8) and head bumps only one. The reverse error also exists: a copy belowfirst_patchedbut outside every vulnerable range forces a false 'does not reach a patched version'. Separately, prerelease ordering collapses to a single flag (1.0.1-beta.1compares equal to1.0.1-beta.2), another silent overcount path. Fix: addsecurity_advisory.vulnerabilitiesto the jq projection. Treat an installed version as vulnerable if it falls inside any npm range for that package, and parse ranges fail-closed: an unparseable range goes to NOT CHECKED.bin/fm-alert-count.py:55- Any repo with more than 100 open alerts always ends in NOT CHECKED, so the--paginatepath the script opts into can never produce a count. gh applies--jq ... | @base64to each page separately and prints one base64 line per page. gh-axi can't JSON-parse that raw output, so it wraps it as a single string, and TOON encodes a string containing a newline asbody: "W3si...\nW3si..."(verified against the installed gh-axi 0.1.35 encoder). Thegh_axi_bodiesregex[A-Za-z0-9+/=]+can't match the literal backslash (verified: fullmatch is False), sobodiesis empty and the script prints 'gh-axi returned no readable alert data' and exits 1. It fails closed, but the multi-body:loop is dead code and the test suite has no multi-page fixture. Fix: accept the quoted scalar, unescape it (JSON string decoding), and split it on newlines into one base64 page each.bin/fm-alert-count.py:227-requires_major_upgrade(base_versions, patched)is checked beforehead_state, so a branch that actually takes the major fix is still excluded. Trace: base foo@1.0.0, patched 2.0.0, head foo@2.0.0. The alert is reported as 'major-only advisory, patch requires 2.0.0' even though the head lockfile verifiably reaches the patched version. The intent names 'a rejected major' as an exclusion, meaning a major the branch did not take, so whether an accepted major counts is a product decision. Two related label defects sit in the same component: (a) line 211 labelsfirst_patched_version: nullas 'major-only advisory, no non-major patched version', but GitHub returns null when no patched version exists at all, which has nothing to do with majors; (b) the module docstring (line 11) says these advisories are 'reported as not checked', but the code puts them inexcludedand exits 0. Please decide whether a taken major counts as remediated, and whether the null-patch case needs its own label (e.g. 'no patched version published').bin/fm-alert-count.py:236- Simplification: the 'OPEN DEFAULT-BRANCH ADVISORIES BY PACKAGE' section (per-package tallies built at line 205) isn't needed for the intent, which is about a verified remediation count, its exclusions, and the default-branch caveat. The per-advisory identifiers already appear under PER-ADVISORY VERDICTS. The section also brings back the 'rough sum by package name' shape the intent describes as the misleading figure, and it counts alerts (one per manifest) under an 'ADVISORIES' header. In a repo with several lockfiles, one GHSA shows up as several 'advisories'. Recommended remedy: remove the section (and its test assertion). If you keep it, relabel it as alerts.bin/fm-alert-count.py:124- Simplification: the lockfileVersion 1dependenciesfallback (visit) is a second parsing path that nothing in the intent requires. v2/v3 lockfiles always carrypackages. Recommended remedy: remove it and reject a lock without apackagesobject as NOT CHECKED. Don't just delete the branch:lock_versionswould then return an empty map, and empty head versions are treated as 'safe' (counted as remediated).🔧 Fix applied.
4 issues (1 error, 2 warnings, 1 info) still open:
bin/fm-alert-count.py:125-lock_versionsnames each copy by its path only, so an npm alias copy is filed under the alias and the vulnerable copy it holds is never checked. This breaks the docstring's rule that head must hold no vulnerable copy, and it can silently raise the VERIFIED count. In v2/v3 lockfiles an aliased install looks like"node_modules/strip-ansi-cjs": {"name": "strip-ansi", "version": "6.0.1"}. Every lockfile that pulls in@isaacs/cliui(glob@10+, rimraf@5) has these entries, and so does any repo with a user alias like"lodash4": "npm:lodash@4". Checked:lock_versionsreturns{'strip-ansi-cjs': ['6.0.1']}. Example: the advisory range is< Xfor package P. Base hasnode_modules/Pand aliasnode_modules/P-alias(name P), both in range. Head bumpsnode_modules/Ppast X and leaves the alias alone.head_copiesfor P is empty, so the alert is counted as remediated while head still ships a vulnerable P. Fix at line 125: keeppackage_name_from_path(path)as the node_modules gate, then useitem["name"]in its place when it is a string. Don't just prefernameeverywhere: the root""entry and workspacepackages/fooentries also carrynameand must stay excluded. If GitHub's dependency graph turns out to ignore aliases, the fix only undercounts, which is the fail-closed direction.bin/fm-alert-count.py:147-parse_rangeaccepts only full X.Y.Z bounds, but GitHub publishes shorter bounds for real npm advisories. A live pass over reviewed npm advisories found>= 099 times, plus< 8.0,>= 6.0,<= 1.3,= 12.0and>= 9.22, < 11.5.0. Every sampled malware advisory uses> 0. All of these raise 'unparseable advisory range', so the alert goes to NOT CHECKED and the run exits 1. This fails closed, so nothing is overcounted, but it has two effects. (a) A>= 0range withfirst_patched_version: nullis exactly the 'no patched version published' case you asked to label, and it never gets that label. (b) A repo with even one malware alert can never get a complete exit-0 count.vulnerable_rangesalso raises when any single range for the package fails to parse, and line 166-167 reports a badfirst_patched_version.identifieras a bad range. The remedy widens what the parser accepts, which is why this needs your call rather than an auto-fix. Narrowest safe option: accept a bare0bound, which covers the>= 0/> 0bulk. Zero-padding is exact for>=and<but ambiguous for<= 1.3and= 12.0(does 1.3.5 fall in?), so those should stay NOT CHECKED.bin/fm-alert-count.py:246-if not base_copies:prints 'already resolved on <base>' without looking at head, so a branch that brings a vulnerable copy back is described as resolved. Example: an alert is open on the default branch for foo< 1.0.1. The integration lock has foo 1.0.1. The security branch's lock ends up with a nested foo 1.0.0 after a dependency bump or dedupe change.base_copiesis empty, so the output says '#N foo: already resolved on integration', and a reviewer reading the verdicts won't see that the branch brings the vulnerability back. The count is unaffected because the alert is excluded, but the label is wrong. Fix: whenbase_copiesis empty andhead_copiesis not, add a distinct exclusion such as '<head> reintroduces a vulnerable copy'.bin/fm-alert-count.py:227-unchecked.append("malformed live alert record")drops the alert number even whenraw_alert.get("number")is an int. Every other NOT CHECKED line carries#N, which the intent needs as a per-advisory identifier. Example: an alert whosesecurity_advisory.vulnerabilitiesprojects to null prints an anonymous 'malformed live alert record', and the operator can't tell which alert was skipped. Fix: include#<number>when the number is an int.🔧 Fix applied.
2 warnings still open:
bin/fm-alert-count.py:155- The fix round now accepts partial bounds, but it rejects them only for<=and=. A partial>bound with a non-zero component is ambiguous in the same way, and it is accepted with zero padding. That can overcount on the base side. Example: advisory range> 8.0, < 9.0, patched9.0.0, base lock8.0.5, head lock9.0.0. Zero padding reads the bound as> 8.0.0, so base counts as vulnerable, head is clean, and the alert is added to VERIFIED. Under npm's partial-version reading,> 8.0means>= 8.1.0: base was never vulnerable, and the right label is 'already resolved on <base>'. This is the ambiguity the prior round cited to keep<= 1.3and= 12.0NOT CHECKED, and your instruction was to keep genuinely ambiguous syntax NOT CHECKED.> 0is unaffected, because nothing sorts below 0.0.0, so the malware and no-patch case you asked for still works. I can't confirm GitHub's range grammar from source here, so treat this as an ambiguity, not a proven miscount. Narrower form: accept a partial>bound only when it pads to0.0.0, and send any other partial>to NOT CHECKED.bin/fm-alert-count.py:269- Simplification: the 'PER-ADVISORY VERDICTS' section repeats every line already printed in the VERIFIED count, EXCLUDED and NOT CHECKED sections. The intent doesn't need a second copy, and the copy is labeled worse than the originals. (a) Remediated rows show only an identifier with no verdict (- #101 foo (package-lock.json)). (b) NOT CHECKED rows (- #201 java-lib (pom.xml): unsupported manifest) are mixed in with no NOT CHECKED marker. (c) Every row is a Dependabot alert, not an advisory: the query at line 70 never fetchesghsa_id. In a repo with two lockfiles, one GHSA fixed in both shows up as two 'per-advisory' verdicts and adds 2 to the count. That repeats the alert-vs-advisory mix-up that got the by-package section removed last round, and the intent's '76 advisories' story is exactly that kind of miscount. Recommended remedy: remove the section, since the three sections below already list every alert. If advisory-level accounting is wanted, that is a product call: fetchsecurity_advisory.ghsa_idand label rows and the count as alerts, or group them by GHSA.🔧 Fix applied.
1 info still open:
docs/scripts.md:44- The docs row says the tool counts 'with per-advisory identifiers', but every identifier it prints is a Dependabot alert number (#N). The jq query at bin/fm-alert-count.py:70 never fetchesghsa_id. Round 3 removed the PER-ADVISORY section for this same alert-vs-advisory mix-up, and the docs line still carries it. Example: a repo withpackage-lock.jsonandweb/package-lock.jsongets two alerts for one GHSA. A reader of the docs would takeVERIFIED BRANCH REMEDIATION COUNT: 2as two advisories, which is the '76 advisories' overclaim the intent warns about. Fix is a wording change only: 'per-alert identifiers'.✅ **Test** - passed
✅ No issues found.
fm-alert-count.py integration security)fm-alert-count.py main maingives VERIFIED BRANCH REMEDIATION COUNT: 0 (none)integration nolocklists 6 api alerts as 'unavailable at nolock', exit=1bin/fm-test-run.sh tests/fm-alert-count.test.shwith a PATH gh-axi stub, not the live product. No readable live repo pub…bin/fm-test-run.sh tests/fm-alert-count.test.sh(test_verified_remediation_and_exclusions_are_explicit, test_unverifiabl…bin/fm-alert-count.py b182d0f f2f2d70in the worktree (origin kunchenguid/firstmate, real gh-axi returns FORBIDDEN)bin/fm-alert-count.py main mainin a temp repo whose origin is a local path (non-GitHub origin)gh-axi api '~/repos?per_page=5' --paginate --jq '[.[] | {name}] | @base64' --fullfed through the script'sgh_axi_pages()and base64/JSON decoded page by pageShallowgh repo cloneof a private work repo with 44 real open alerts; local commitsintegration,securityandnolockedited only package-lock.json versions (nothing pushed)fm-alert-count.py integration securityagainst the live alert list (count 7, all exclusion labels, caveat, NOT CHECKED for pip, exit 1)fm-alert-count.py main mainagainst the live alert list (a branch that changes nothing counts 0)fm-alert-count.py integration nolockagainst the live alert list (missing head lockfile goes to NOT CHECKED)fm-alert-count.py main securityin a local repo whose origin is a private work repo with 0 open alerts (silent, exit 0)bin/fm-test-run.sh tests/fm-alert-count.test.sh(4 behavior tests pass)bin/fm-test-run.sh --check-coverageand--list --family pure-contract-unit(new test is registered)✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.