Skip to content

fix(scanner): isolate custom cliScannerSource binaries and test oldest supported scanner - #65

Merged
tembleking merged 2 commits into
mainfrom
fix/custom-scanner-source-and-oldest-scanner
Oct 7, 2026
Merged

tembleking merged 2 commits into
mainfrom
fix/custom-scanner-source-and-oldest-scanner

Conversation

@tembleking

Copy link
Copy Markdown
Member

Users can point cliScannerSource at any scanner build, including older ones. That path was broken and untested: the cached binary was always named after SCANNER_VERSION, so a custom binary and the default one overwrote each other. Custom sources are now cached under a name derived from a hash of their URL.

A new compatibility test downloads both the newest and the oldest supported scanner (the oldest release inside the 365-day window) through cliScannerSource. For each one it checks --version, and checks that every flag the VM and IaC runners pass is still accepted. It needs no token, so it runs in the existing CI. Both versions are tagged with markers and bumped by just update (update-cli-scanner / update-oldest-cli-scanner; update-scanner-version is kept as an alias), the same approach used in scan-action and the Jenkins plugin. This also bumps the default scanner from 1.27.1 to 1.30.1.

just test was broken before this change. VS Code 1.139 renamed its bundled binary from Electron to Code, and @vscode/test-electron 2.x still looks for Electron. This PR bumps @vscode/test-electron to 3.1.0 and @vscode/test-cli to 0.0.15. With 0.0.15, npm audit flagged the mocha it pulls in (serialize-javascript, diff) as fixable, which failed just lint. mocha is now forced to 12 through overrides, and npm audit fix took care of the rest (js-yaml, fast-uri), so the audit is clean.

@tembleking
tembleking requested a review from a team as a code owner September 24, 2026 14:59
mateobur
mateobur previously approved these changes Sep 24, 2026
@mateobur

Copy link
Copy Markdown

The custom-source cache isolation is a clean, well-tested fix. Before merging, please confirm the new scannerCompatibility test does not download a binary at CI time (flaky risk), and that CI and contributors are on Node 22+ given the test-tooling and mocha upgrades now require it.

@tembleking

Copy link
Copy Markdown
Member Author

Thanks!

  • The compat test does download the binaries, on purpose: the point is to execute the real oldest supported scanner, which can't be done without fetching it. This isn't a new dependency, since CI already downloads the scanner from download.sysdig.com in configScanner.test.ts (downloadBinary should download the binary file when it does not exist).
  • CI runs inside nix develop, which ships Node 24.15, so the Node 22+ requirement from the new test tooling is met. The flake is the supported dev setup for contributors too.

@tembleking
tembleking requested a review from mateobur September 25, 2026 10:56

@Jujuyeh Jujuyeh 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.

wow interesting

@airadier airadier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, approving. CI is green on all platforms. Nothing blocking; a few non-blocking follow-ups:

  1. configScanner.test.ts: the new test calls configuration.update(...) without await and never resets cliScannerSource in the shared test workspace. This can be order-dependent or flaky. Suggest awaiting the updates and resetting to undefined in a finally, or stubbing getConfiguration as scannerCompatibility.test.ts does.
  2. package.json: overrides.mocha ^12.0.2 goes beyond what @vscode/test-cli 0.0.15 declares (mocha ^11.7.6). Worth confirming this is intentional. test-cli 0.0.15 and c8 11 also now require Node >=22 / >=20, so Node 18 environments can no longer install or run the tests.
  3. justfile oldest-cli-scanner: any failed or non-200 HEAD request counts as a missing version, and two consecutive misses end the patch scan. A transient network or CDN error could therefore silently write a wrong oldest version. Consider distinguishing 404 from other errors and retrying. It also picks the first release inside the window rather than the release that was current at the cutoff, so the "oldest supported" guarantee is slightly weaker than documented.
  4. scannerCompatibility.test.ts: the suite downloads two real binaries from download.sysdig.com on every run, so it fails offline. Consider gating it behind an env var or a dedicated CI job. The flag check (${flag}\b) can also match prose in the help text; a stricter pattern like (^|\s)${flag}(\s|=|,|$) would be tighter.
  5. configScanner.ts: the custom-source cache key hashes only the URL, so a mutable URL (e.g. latest) is never re-downloaded once cached. This is not a regression, since the old key was just SCANNER_VERSION, but it may be worth documenting that the URL should be immutable.

@tembleking
tembleking merged commit 211c396 into main Oct 7, 2026
5 checks passed
@tembleking
tembleking deleted the fix/custom-scanner-source-and-oldest-scanner branch October 7, 2026 13:54
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.

4 participants