Skip to content

Fix CLI scan completion, error handling, and advisory deduplication - #623

Open
PekSec wants to merge 1 commit into
RetireJS:masterfrom
PekSec:codex/cli-correctness
Open

PekSec wants to merge 1 commit into
RetireJS:masterfrom
PekSec:codex/cli-correctness

Conversation

@PekSec

@PekSec PekSec commented Sep 18, 2026 •

Copy link
Copy Markdown

CLI scans could close reports before asynchronous OSV findings arrived, and malformed inputs or repository responses could escape the intended error path. This PR waits for scan completion before finalizing output, validates terminal CLI input, handles malformed local/remote JSON with source context, and passes the insecure option through repository loading.

It also fixes the exclusive upper license boundary and platform-dependent Bower path handling, and preserves distinct CVEs sharing an issue while removing repeated advisories.

This is the CLI-only replacement for the relevant part of #622, as requested. It changes seven CLI/core/test files; extension rewrites, portable build scripts, and CycloneDX changes are excluded.

Validation: 70 tests pass, TypeScript checking and nonfixing ESLint pass, repository validation passes, and the full detection suite completes successfully across 48 packages. On Windows, the unchanged Unix-oriented npm scripts were run using equivalent direct Node/TypeScript commands against freshly compiled output.

AI-assisted implementation, with regression tests for the corrected behavior.

Related independent replacements: browser extensions #624 and CycloneDX #625.

@eoftedal

Copy link
Copy Markdown
Member

Why the full rewrite from eventemitter to async?
That makes this change very hard to review and makes in a serious architectural change.

@eoftedal

Copy link
Copy Markdown
Member

Tests should be split into test files per module. Example: license tests

@eoftedal

Copy link
Copy Markdown
Member

getIdentifiers has changed semantics. Why?

@eoftedal

Copy link
Copy Markdown
Member

Commits must have verified signatures.

@PekSec
PekSec force-pushed the codex/cli-correctness branch from 018599e to 703593f Compare September 24, 2026 10:36
@PekSec

PekSec commented Sep 24, 2026

Copy link
Copy Markdown
Author

The change addresses a completion-order bug introduced with OSV integration: directory traversal can finish while OSV requests are still pending. In an isolated reproduction using the old and new sources, the old CLI closed the report with no findings and exit code 0; the new code waited and closed with the finding and exit code 13. Scanner events remain intact. The waiting is necessary, although moving the entire CLI into async main() is not required to fix it.

@PekSec

PekSec commented Sep 24, 2026

Copy link
Copy Markdown
Author

Previously, any shared identifier could collapse two advisories, including a shared issue number. Bootstrap issue 20184 covers multiple distinct CVEs, so this dropped valid findings. The change prioritizes CVE/GHSA identifiers and namespaces fallback identifiers. The tradeoff is that an issue-only record may remain separate from a CVE-bearing record for the same vulnerability.

@PekSec

PekSec commented Sep 24, 2026

Copy link
Copy Markdown
Author

The tests are now split by module into CLI, scanner, repository, depsdev, and license specs. All 70 tests pass, along with the build, TypeScript checks, and ESLint. The updated commit, 703593f, has a verified SSH signature. Production code is unchanged from the previous PR revision.

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.

2 participants