My code checker was wrong. How I turned its false positives into tests
DEV Community

My code checker was wrong. How I turned its false positives into tests

Disclosure: I'm the author of rebar, the open-source checker this post is about. It is alpha. rebar fails a CI job when a repository breaks a rule. Examples: formatting nobody checks, a hook git will never run, a credential in a tracked file. A checker like that can fail in one way that matters more than any other: accusing a repository that is doing the right thing. After a couple of those, people stop reading the output. Soon after, they turn it off. So I ran it against 28 public repositories and read what it said. Three of the accusations were wrong, and I could prove it. Each case below follows the same order: the accusation, the evidence it was wrong, the fix, and the test that keeps it fixed. The test has to work both ways. It must fail on the old code, and the real violation must still be caught. 1. "Git ignores this hook." Not with husky 9. rebar told mastra (commit 7e21ff5 ): hooks-executable ✗ no execute bit in the index, git ignores them on Linux: .husky/pre-commit - fix with: git update-index --chmod=+x .husky/pre-commit The file really is mode 100644, and on Linux git skips a hook that isn't executable. The accusation sounds right, and husky 8 users run into exactly that. mastra uses husky 9, which works differently. husky 9 makes three changes: - it points core.hooksPath at.husky/_ ; - it writes its own .husky//pre-commit with mode0o755 ; - that generated file calls a small script that runs yours with sh -e .husky/pre-commit . Git never executes the committed file, so its mode doesn't matter. I checked this by running git, not just by reading the husky source. I made a repository with .husky/pre-commit at 100644, cloned it, ran npm install , and ran git commit with GIT_TRACE=1 , which prints the file git runs: | husky 9.1.7 | husky 8.0.3 | | |---|---|---| | file git ran | .husky//pre-commit (generated) | .husky/pre-commit (committed) | | rebar before | ✗ | ✗ | | rebar after | not applicable | ✗ | I ran it on Windows because Docker wouldn't start on my machine. Windows shows which file git runs. That Linux skips a non-executable hook comes from git's documentation, not from this run. The fix exempts the hooks husky 9 manages. It doesn't switch the rule off. The tests: - husky-v9 has two sides with the same bytes and the same 100644 mode. The only difference is the husky version the repository declares. The new code passes one side and fails the other. The old code fails both, which is what makes it a regression test. - husky-own-hook declares husky 9, but also has a hook that git runs directly (tooling/hooks/pre-push ) at 100644. Declaring husky must not silence that one, and it doesn't. 2. "Rotate this credential." It's AWS's documentation example. rebar told dotenv (commit f6390d1 ): hardcoded-secret ✗ 10 credential(s) in tracked files … deleting the line is not enough, what is in history has to be ROTATED Six of the ten were not credentials: - AWS_ACCESS_KEY_ID=AKIAIOSF•••••••••••• , twice. That's the example key pair from AWS's own IAM documentation. - A PRIVATE_KEY in the README whose body isKh9NV... , twice. It was cut short on purpose. - PASSWORD: 'password' , in two tests. The value is just the word. A secret scanner that tells people to rotate AWS's example key loses its audience fast. The fix releases those three shapes and nothing near them. The tests are built so the fix can't turn into a hole: - vendor-example : the failing side is the same file, and it even keeps AWS's example secret key. Only the access key ID changes, to one that isn't a published example, and it must still be flagged. A check for "contains EXAMPLE" would let it through. - elided-pem : the failing side has an ellipsis too, but between real 64-character key lines. That's what someone does when they paste a real key into docs and cut out the middle. - value-is-the-word : the same test fixture and the same key, with a value that isn't the word. This fix nearly shipped wrong. Its first version removed 45 findings from mastra and added 6, and the total still went down. A count only shows the net change. So for this post I ran the scanner again with full lists on every repository, at the same commits, and compared finding by finding: - 151 findings removed, 0 added; - I read each removed line in the original repository; - 107 were the word itself, 28 were AWS's example pair, 9 were shortened keys, and 7 were placeholders like mypassword ; - none was a real credential. dotenv's other four findings are .env files used as test fixtures. rebar still flags them, on purpose, because it flags any committed .env whatever is in it. I haven't settled that design question, and I didn't touch it. 3. "Nothing fails when a file is out of format." The next step does. rebar told e2b (commit ccaf9fc ): formatter ✗ script format only rewrites - nothing fails when a file is out of format The format script is prettier --write , which only rewrites. But e2b's lint workflow runs pnpm run format and then fails the job if git status --porcelain isn't empty. Rewriting and then failing on any difference is a formatting check. The fix recognizes that pattern, but only inside a single job. A git diff --exit-code in another job looks at a different checkout. The tests: - write-then-verify must pass; - write-then-verify-other-job splits the same two commands across two jobs, and it must fail; - both of the sides that should pass fail on the old code. I also ran rebar's GitHub Action on a runner with only this rule. prettier --write . exits 1 and prettier --check . exits 0. Did the fixes generalize? Seven of the 28 repositories are where I found these problems, and I wrote the fixes looking at them, so of course they pass there. The other 21 came from a search rule I wrote before measuring anything. Those 21 are the real test. | 7 chosen, before → after | 21 searched, before → after | | |---|---|---| hooks-executable , repos flagged | 1 → 0 | 4 → 1 | hardcoded-secret , findings | 1319 → 1202 | 664 → 630 | formatter , repos flagged | 4 → 2 | 14 → 14 | - husky: 4 → 1. The one left, agentscope, is still a false positive. Its husky 9 hook lives in examples/web_ui/ , a package inside the repository, and rebar only looks for husky at the root. Still open. - Secrets: 34 fewer findings in the searched group, all placeholders, none added. I haven't classified the 1832 findings that remain, so I make no claim about them. - formatter: no. 14 → 14. I read all 16 repositories still flagged across both groups and followed everything their CI calls: - 12 are correct: nothing fails on bad formatting. - 4 are still wrong: - serena runs poe lint , which runsruff format --check ; - cognee runs ruff format --check ; - page-agent runs a scripts/ci.js that callsprettier --check ; - dotenv runs npm test , which runsstandard . That last one is arguable. Ten verdicts changed in total. Every one went from failing to passing or not applicable, and no repository was newly accused. The checker's checker was wrong too My first script for sorting those formatter results said "13 correct, 1 false positive". It looked for the check only on the workflow line itself, and didn't follow poe lint or node scripts/ci.js : the same blind spot as the rule it was auditing. The version before that read files through the API, swallowed an error, and reported "no check" for cognee. Both are fixed, and both now fail loudly. I mention them because they are the same bug as the rule: a checker that is wrong without saying so. What this does not show The searched repositories were picked partly by the mcp topic, and rebar has rules aimed at MCP server setups. This audit says nothing about those rules. I didn't classify their findings, and there's no comparison group of projects without MCP. So I can't say whether MCP projects are riskier, or whether those signatures catch a real attack. Answering that would need a study of its own. Three fixed false positives aren't an accuracy rate either. They are three bugs, and each one now has a test that fails on the old code. Run it on your project and send me a reproducible false positive npx github:Navesz/rebar#v0.1.0 . npx -p github:Navesz/rebar#v0.1.0 rebar-security . Or in CI, as a GitHub Action, pinned to the release commit. A tag can be moved; a commit can't: - uses: Navesz/rebar@9d39f1e3ed4cd3190b4635dccf22d7b815a74d5b # v0.1.0 with: ruler: both # check · security · both If it flags something that is correct, open a false-positive issue with the rule id, the line it printed, and a public commit or a minimal repository. Every case above started as one wrong line of output, and each one is now a test. Appendix: method and full tables Versions. Before: rebar 9f78d9b . After: 58aff2c . The v0.1.0 release (9d39f1e ) has the same rules: only the Action, its workflow and the changelog differ. The fixes landed in #41 (husky, formatter, ci-gates, env-example) and #40 (secrets). Sample. 28 repositories, fixed before any result was seen and not extended afterwards. - 7 chosen, because the fixes came from them: mastra, dotenv, stagehand, biome, e2b, serena, skyvern. - 21 from a GitHub search: - topic mcp ,ai-agents ordeveloper-tools ; - 300-40,000 stars, pushed after 2026-07-24; - not archived, not a fork; - TypeScript, JavaScript or Python; - not a collection (awesome lists, skills, templates); - at most 300 MB; - top by stars. - topic - mastra (1.8 GB) and skyvern (623 MB) break the size cap. I kept them because they're among the chosen seven. Measurement. Each repository was cloned at depth 50 and measured with both rulers and both versions: 112 runs, 0 errors. The commit measured for each repository is in the raw data. The secrets comparison ran the scanner again with full lists at those same commits, because the ruler's summary cuts each list at about 12 items. Other verdicts that changed (not among the three cases): - ci-gates on dotenv, becausenpm test chainsnpm run lint ; - ci-gates on stagehand, becausepnpm check runsturbo run fmt:check lint typecheck ;

Read on DEV Community ↗ ← Back to News

Comments

No comments yet. Start the discussion.