Skip to content

fix(ratchet): gate like --fail-on when no baseline exists - #1314

Open
theluckystrike wants to merge 5 commits into
OWASP:mainfrom
theluckystrike:fix/1293-ratchet-no-baseline-gate
Open

theluckystrike wants to merge 5 commits into
OWASP:mainfrom
theluckystrike:fix/1293-ratchet-no-baseline-gate

Conversation

@theluckystrike

Copy link
Copy Markdown
Contributor

Fixes #1293

With no committed baseline, --ratchet wrote one and exited 0, so a CI job that checks out fresh every run couldn't ever fail. This follows the decision in the issue body. When no baseline exists, --fail-on applies exactly as if --ratchet were absent, the baseline's still written, and a line says that committing .cve-lite/baseline.json is how you accept the existing debt.

Two integration tests cover it. With no baseline and a finding at or above --fail-on, the scan exits 1, writes the baseline and prints the gate line and the commit line. On a clean tree it exits 0 and prints no gate line. The first one fails on main with exit 0.

npm run lint:tests passes, and the full suite passed after advisories sync (2314 tests), the same steps ci.yml runs.

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. The fix itself is right, and I checked it does what #1293 asks on a single folder: with no baseline and a high finding it now exits 1, writes the baseline and prints both lines, and with a baseline present nothing changes. I ran the repro from the issue body on examples/direct-fixable.

One thing needs sorting before it can land, and it is an easy one to miss. --ratchet has a second implementation in src/scan/multi-folder-scan.ts. handleMultiFolderRatchet writes a baseline per folder and then continues, so the gate never runs there either. Same tree, same flags, both built from your branch:

mode exit code
single folder 1
multi folder 0, still a silent pass

So as it stands this fixes half of #1293 while carrying Fixes #1293, which would close the issue with the monorepo path still passing silently on every fresh CI checkout.

I am happy either way on how you want to handle that. You can extend this PR to cover handleMultiFolderRatchet, which is the same shape you have already written, or we split #1293 and you keep the credit for this half. If you would rather split it, drop the Fixes line here so the issue stays open.

Three smaller things inline.

Comment thread src/scan/single-scan.ts
Comment thread src/scan/single-scan.ts
Comment thread src/scan/single-scan.ts
@theluckystrike

theluckystrike commented Oct 14, 2026 via email •

Copy link
Copy Markdown
Contributor Author

@sonukapoor sonukapoor left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The multi-folder half is right. Same two-folder fixture, no baselines, --fail-on high: main exits 0, your branch exits 1, and a clean tree still exits 0 on both. Single folder behaves the same. Full suite green on the branch, 168 suites and 2371 tests, and the branch is current with main, so that is the merge result.

Your tests discriminate too: reverting the multi-folder return kills six of them, dropping the count > 0 guard kills two, dropping the --json guard kills two.

My third ask last time was wrong and I am withdrawing it. I asked you to pin the !options.fix check on the exit expression. That condition cannot be reached. The whole ratchet block sits in the else arm of if (options.fix), so options.fix is always false there, and under --fix no baseline is written at all, on main as well as here. There is nothing to pin, and both !options.fix checks are dead rather than load-bearing. Removing them would be a fair tidy-up but I am not asking for it.

One thing left before this lands, and it is docs rather than code. website/docs/ratcheting.md still says of the first run:

This creates .cve-lite/baseline.json in your project root and exits 0.

That is now false whenever the tree carries a finding at or above --fail-on, which is the exact case this PR fixes. Could you update that sentence here? src/cli/help.ts and website/docs/cli-reference.md both describe --ratchet without mentioning that the save run gates, so a few words in each would be welcome, but the ratcheting.md sentence is the one that contradicts what ships.

Separately, and not yours to fix: a first run with --ratchet --json now exits 1 while printing no reason, because the gate line is --json-suppressed. That suppression is what I asked for last time, so revisiting it is my call rather than a problem with your work. It also writes no JSON file at all, but main does the same on that path, so that half is pre-existing and the same family as #1315. I am filing it separately.

Four small things inline, all optional.

Comment thread tests/cli-integration.test.ts Outdated
}
});

it("--ratchet with no baseline and a clean tree exits 0 and writes the baseline", async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing: this is titled "a clean tree" but it seeds createFinding({ severity: "low" }), so the tree is not clean, it just carries nothing at or above high. The behaviour is worth covering and the assertions are right, it is the name that will mislead whoever reads it next, especially since the genuinely clean case is the first row of the it.each below. Something like "below the threshold" would say what it does.

expect(writeBaselineMock).toHaveBeenCalledWith("/project/a", expect.any(Array));
expect(writeBaselineMock).toHaveBeenCalledWith("/project/b", expect.any(Array));
expect(exitCode).toBe(EXIT_OK);
expect(exitCode).toBe(EXIT_FINDINGS);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This expectation and the matching one further down both assert EXIT_FINDINGS now, and they reach it through baseOptions, which sets failOn: "none". Since none ranks 0 and the gate counts findings >= failLevel, --fail-on none gates on every finding, so these two pass for an incidental reason rather than because the threshold logic works.

Your new it.each above covers the real thresholds properly, so nothing is genuinely untested. I would just pass an explicit failOn: "high" in these two so they say what they mean.

Comment thread src/scan/multi-folder-scan.ts Outdated
console.log(chalk.red(`${r.subfolder}/: ${gateLine}`));
}
}
if (count > 0 && !options.json) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This prints once per folder, so a monorepo with twenty subfolders gets the same sentence twenty times with only the path changing. The gate line above it genuinely is per folder because the counts differ, but the instruction does not vary. Collecting the paths and printing one line after the loop would read better, or printing the hint once without a path. Not a blocker.

Comment thread src/scan/multi-folder-scan.ts Outdated
),
);
const gateLine = failingGateSummary(
[{ label: "vulnerability", findings: r.sorted }],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

label: "vulnerability" is at four sites across the two scan files now, two of them added here. #1288 is touching these same files, so this is the kind of literal that quietly diverges in a merge. A shared constant beside RATCHET_BASELINE_COMMIT_HINT would settle it. Entirely optional, and say if you would rather leave it for #1288 to pick up.

…un gate

Address review on OWASP#1314:

- Multi-folder --ratchet collects the saved baselines that carry findings
  and prints a single commit hint after the loop instead of one per folder.
- Build the first-run gate input from GATE_CLASS_LABELS (OWASP#1288) in both
  scan paths, so no "vulnerability" literal is left.
- Rename the single-folder test that seeds a low finding to say it is
  below the threshold, and pass an explicit failOn "high" to the two
  multi-folder tests that relied on baseOptions' "none".
- ratcheting.md no longer says the first run exits 0; it gates on
  --fail-on. Note the same in cli-reference.md and --help.
@theluckystrike

theluckystrike commented Oct 15, 2026 via email •

Copy link
Copy Markdown
Contributor Author

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(ratchet): --ratchet with no committed baseline is a silent permanent pass in CI

2 participants