fix(health,mcp): a security score from 3% of the repo, and the last of the fake totals #5611
ccantynzAI Reviewcommented 1h ago
AI Triage
(no summary)
Priority: medium Risk area: mixed
Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)
Suggestions only — nothing has been applied. The PR author stays in control.
gluecron[bot]🤖 botAI Reviewcommented 1h ago
AI review unavailable
The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.
Cross-repo impact
See what breaks downstream if this PR merges.
⮌ Merged
This pull request was merged into main.
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts
Third wave from the cross-platform session. Both headline items are the same defect in different clothes: a number computed from partial input, presented with the confidence of a complete one.
Security scored 88/100 having read 3.2% of the source
The scanner ran one
git showper file, so covering a 3,090-file repo meant 3,090 subprocesses. It was capped at 100 files — and then scored the result as a measurement of the repository.The coverage block did disclose it (
sourceFilesScanned: 100, truncated: true), which is better than silence and still not enough. As the GateTest session put it: a score computed from 3% of a repo is not a truncated measurement, it's a different measurement wearing the same name — and nobody discounts a number that says 88.Fixed by making full coverage affordable rather than by better-labelling the absence. One
git grep -nreturns every candidate line in the whole tree in a single subprocess.Only files containing an
innerHTMLassignment get read in full, because those are judged as whole statements — 19 files here.The token list is deliberately a superset of every rule, so a line it doesn't return cannot contain a finding. Leading letters are dropped where that buys case-insensitivity (
asswordmatches both spellings;-iisn't on the git argument allowlist). Broad costs output; narrow costs a missed vulnerability.When git grep can't answer it returns
null, the bounded scan runs, and the response saysmethod: "capped"withtruncated: true. "No candidates" must never be arrived at by way of "the tool failed" — and those two cases were previously indistinguishable from the numbers alone.The last of the fake totals
The previous wave fixed four listing tools and missed both repo searches. GateTest measured it:
Worse than the
list_prscase it matches:total: 50is a round number that invites suspicion, whiletotal: 5when you asked for 5 reads as a complete and precise answer. Nothing signals the 31 repos the caller can't see. Both now return the true count and page properly. Nototal: rows.lengthremains in either tool file.Ancestor trees counted as contents
list_treewith a path +recursive: trueincluded the ancestor trees leading to that path —totalCount: 10for 8 actual entries (Vapron's #5). Verified against real git output: scoping tosrc/lib/selfcheckemitssrc,src/lib,src/lib/selfcheckahead of 52 real entries. Exactly those three are dropped now, from the count as well as the list.Degraded AI output was indistinguishable from real output
generate_commit_messagefell back to a heuristic on three paths — no API key, empty model response, any network/API failure — returning a result shaped identically to a real generation. A caller couldn't tellchore(src): update 1 filefrom a model's considered summary. Now carriessource: "ai" | "heuristic"and adegradedReason, and the failure path logs rather than swallowing.A test heuristic that punished its own documentation
git-arg-injectioncollects every dash-prefixed literal ingit/repository.tsand requires each to be an allowed git flag. It caught"--- "(a diff header matched withstartsWith) and then`-i`inside a comment explaining why that flag is not on the allowlist — so documenting the guard failed the guard's own test.Narrowed by context both times, not by exceptions: literals consumed by a string predicate, and literals on a comment line, are not arguments. Anything that could reach git as an argument still must be allowlisted. Detected by line position rather than by stripping comments, which would have to parse strings containing
//correctly to avoid dropping a real flag.Verification
git ls-treeoutput.