fix(cli): error on a mistyped command instead of silently scanning and exiting 0#719
Conversation
sonukapoor
left a comment
There was a problem hiding this comment.
Good fix - the silent exit 0 on a mistyped command is a genuine footgun for a security tool and this is the right place to catch it. A few things to address before it lands, two of which become active regressions after the required rebase against main.
The branch is behind main (which now includes the overrides command from #718) - please rebase before pushing the fixes, as two of the issues below are directly caused by that gap.
…xiting 0 `cve-lite frobnicate` (a typo'd command) resolved "frobnicate" as a scan path, found no packages, and exited 0 - a silent clean result on a typo, which for a security tool reads as "nothing wrong" when the intended check never ran. The guard is scoped to the scan command and fires only when the first argument is neither an existing path nor a known command; it errors with EXIT_ERROR and a Levenshtein "did you mean" suggestion (advisories / install-skill / config / overrides). Subcommands like `overrides /not-yet-existing` are unaffected, and existing directories still scan unchanged. nearestCommand/levenshtein live in src/utils/string.ts (index.ts stays free of logic). Covered by unit tests for the helpers plus CLI/e2e tests for the guard.
dac498c to
2da7a5e
Compare
|
@sonukapoor all four addressed in 2da7a5e, rebased onto current main (post-#718). Guard scoped to scan, EXIT_ERROR constant, overrides added to the suggestion list, and levenshtein/nearestCommand moved to utils/string.ts with unit + CLI + e2e coverage. Full suite green at 851. Also updated the upstream e2e test that pinned the old exit-0 behavior. Ready for another look. |
sonukapoor
left a comment
There was a problem hiding this comment.
All 6 issues resolved - guard correctly scoped to scan only, overrides in the known-commands list, levenshtein/nearestCommand moved to utils/string.ts, EXIT_ERROR constant in place, branch current with main, and the e2e regression test covers the overrides path directly. One minor thing: tests/utils/string.test.ts is redundant alongside tests/string.test.ts - worth deleting in a follow-on chore but not a blocker. Merging.
|
Merged - thank you @alamb-hex! |
A small, focused CLI-robustness fix (the standalone change mentioned in #718).
Problem
cve-lite frobnicate(a typo'd command) resolvesfrobnicateas a scan path, finds no packages, and exits 0. For a security tool, a clean exit on a typo reads as "nothing wrong" when the intended check never ran.Fix
Known commands (
advisories,install-skill,config) are dispatched before the scan branch, so an explicit first argument that does not resolve to an existing path is almost certainly a mistyped command. In that case, error with a clear message, exit code 3, and a Levenshtein "did you mean" suggestion:Existing directories still scan unchanged - the guard only fires on nonexistent paths, so a real (empty) project still exits 0 as before.
Tests
Two cases added to
tests/cli-integration.test.ts(nonexistent arg -> exit 3 + message; typo'd subcommand -> suggestion). Full suite green.