Skip to content

refactor+test: extract advisory mapping into a pure, unit-tested module#20

Merged
John-David Dalton (jdalton) merged 2 commits into
mainfrom
surf-bun-scanner-modernize
Jul 25, 2026
Merged

refactor+test: extract advisory mapping into a pure, unit-tested module#20
John-David Dalton (jdalton) merged 2 commits into
mainfrom
surf-bun-scanner-modernize

Conversation

@jdalton

Copy link
Copy Markdown
Collaborator

What this does

Finishes the bun-security-scanner modernization. Three of the four goals were already delivered by earlier commits on main (rolldown bundling, SDK in authenticated mode, a conforming scanner shape); this PR verifies and documents those against the current upstream contracts and delivers the missing piece — real offline test coverage of the core scan logic.

The one behavior change is a refactor: the scan() alert→advisory mapping and the purl parsing move out of src/index.ts (which runs a token bootstrap at import) into a side-effect-free src/advisories.ts, so they can be unit tested without network or a token. Runtime behavior is identical.

Coverage: offline unit suite 16 → 44 tests. The core mapping in src/index.ts went from 0% offline coverage (only the network/token-gated live/dist suites touched it) to fully exercised.

file funcs lines
src/advisories.ts (new) 100% 100%
src/index.ts 100% 74.6%
src/modes/authenticated.ts 100% 95.7%
src/modes/unauthenticated.ts 100% 100%
src/scanner-factory.ts 100% 100%

(The uncovered index.ts lines are the win32 LOCALAPPDATA-missing throw and the no-XDG_DATA_HOME ~/Library / .local/share fallback — platform branches that don't run on a CI runner with XDG_DATA_HOME set.)

Goal 1 — conforms to the current Bun.Security.Scanner API

Checked the scanner against the installed bun-types@1.3.14 bun/security.d.ts and the official oven-sh/security-scanner-template. No API delta — the contract has not broken, and src/index.ts already conforms:

  • version: '1' (string literal, required discriminator).
  • scan(info: { packages: Package[] }): Promise<Advisory[]>.
  • Advisory = { level: 'fatal' | 'warn'; package: string; url: string | null; description: string | null }. The scanner maps alert.action === 'error'fatal, everything else → warn, which matches Bun's documented behavior (any fatal cancels the install with a non-zero exit; otherwise warn prompts in a TTY / exits in CI).
  • bunfig.toml contract is [install.security] scanner = "...", unchanged.

tsc --noEmit passes with the export typed as Bun.Security.Scanner, and a new conformance test asserts version === '1' and that scan is a function.

Goal 2 — @socketsecurity/sdk usage (and why free mode stays a bounded fetch)

Authenticated mode already uses the SDK (SocketSdk.batchPackageStream, actions=error,warn) — unchanged here.

For free mode I evaluated SDK 4.0.3's checkMalware(), which hits the same firewall-api.socket.dev/purl endpoint token-lessly (it strips the Authorization header). It is not a drop-in replacement:

  • It runs publicPolicy normalization and, per its own docs, "only return[s] malware-relevant results" — it would drop the non-malware alerts the scanner surfaces (e.g. didYouMean typo-squatting).
  • The MalwareCheckAlert shape it returns has no action field, so the scanner could no longer distinguish fatal from warn.

So free mode keeps its minimal, bounded fetch to the firewall API, which returns the raw artifact alerts (with action) the scanner needs. This is the "no SDK-supported unauthenticated path, keep a documented bounded fetch" outcome.

Goal 3 — rolldown bundle

Already wired (.config/repo/rolldown.config.mts, scripts/repo/build.mts): single ESM entry, every runtime dep vendored, only bun + node builtins external. bun run build produces dist/index.js + dist/index.d.ts. Verified the new src/advisories.ts is pulled into the bundle via the import graph (no config change needed), and test/dist.test.ts smoke-tests the built bundle in both modes.

Goal 4 — tests + coverage (the substance of this PR)
  • test/advisories.test.ts (new): parseNpmPurl (unscoped, scoped, percent-encoded scope, missing version, malformed) and artifactsToAdvisories (fatal/warn mapping, typo-squat hint, description/note/fix assembly, exact description formatting, unparseable-purl skip, one-advisory-per-alert, multi-artifact flattening). Covers 100% of the module.
  • test/index.test.ts (new): scanner conformance; a deterministic free-mode scan() end-to-end that doubles only global fetch (the genuine external HTTP boundary) — free mode is forced by pointing XDG_DATA_HOME/LOCALAPPDATA at an empty temp dir so no real developer token can leak in; and the settings-file token fallback (valid base64 apiToken → authenticated SDK path; unreadable file → throws error reading Socket settings).

No owned-infrastructure mocking: doubles are limited to global fetch and, for the settings-file authenticated-path assertion, a spyOn of the SDK method (matching the existing authenticated.test.ts convention).

Verification

  • bun test — 49 pass, 0 fail across 8 files (the live suite self-skips without a token; dist smoke-tests the built bundle).
  • bun run build — succeeds (rolldown bundle + d.ts).
  • tsc --noEmit, fleet oxlint + oxfmt — clean.

Deferred / not included

The scan() body inlined the Socket-artifact→Bun-advisory mapping (level,
description assembly, overview URL) and the purl parsing. Both are pure, but
they lived next to index.ts's module-init token bootstrap (env + settings-file
read behind a top-level await), so they could only be exercised through the
network/token-gated live and dist suites.

Move parseNpmPurl and the new artifactsToAdvisories into a side-effect-free
./advisories module. index.ts re-exports parseNpmPurl for compatibility and
scan() now just drains each yielded batch through artifactsToAdvisories.
Behavior is identical: an artifact with no alerts or an unparseable inputPurl
contributes nothing, action:'error' maps to fatal and anything else to warn.
Adds offline unit coverage for logic that previously only ran through the
network/token-gated live and dist suites.

- advisories.test.ts: parseNpmPurl (scoped, percent-encoded, missing-version,
  malformed) and artifactsToAdvisories (fatal/warn mapping, typo-squat hint,
  description/note/fix assembly, unparseable-purl skip, multi-alert and
  multi-artifact flattening). 100% of the module.
- index.test.ts: Bun.Security.Scanner conformance (version '1', async scan(),
  parseNpmPurl re-export), a deterministic free-mode scan() end-to-end that
  doubles only global fetch, and the settings-file token fallback (valid
  base64 apiToken enters authenticated mode; unreadable file throws). Free
  mode is forced deterministically by pointing XDG_DATA_HOME/LOCALAPPDATA at
  an empty temp dir so no real developer token can leak in.

Offline unit suite goes 16 -> 44 tests; src/index.ts core mapping goes from
0% offline coverage to fully exercised (advisories 100%, index funcs 100%).
@jdalton
John-David Dalton (jdalton) merged commit 69d09aa into main Jul 25, 2026
4 checks passed
@jdalton
John-David Dalton (jdalton) deleted the surf-bun-scanner-modernize branch July 25, 2026 04:34
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.

1 participant