From 02f13456da8188a498c747997a5d0c7cf285f5b8 Mon Sep 17 00:00:00 2001 From: Bill Li Date: Thu, 16 Apr 2026 16:13:51 +0100 Subject: [PATCH 1/2] fix: race condition where artifacts from late resolving flights could be silently dropped --- bun.lock | 1 + src/modes/authenticated.ts | 4 +- src/modes/unauthenticated.ts | 8 +- src/scanner-factory.ts | 14 +- test/scanner-factory.test.ts | 242 +++++++++++++++++++++++++++++++++-- 5 files changed, 248 insertions(+), 21 deletions(-) diff --git a/bun.lock b/bun.lock index f2e1285..e0832bc 100644 --- a/bun.lock +++ b/bun.lock @@ -1,5 +1,6 @@ { "lockfileVersion": 1, + "configVersion": 0, "workspaces": { "": { "name": "@socketsecurity/bun-security-scanner", diff --git a/src/modes/authenticated.ts b/src/modes/authenticated.ts index 33df8c5..74c1bb5 100644 --- a/src/modes/authenticated.ts +++ b/src/modes/authenticated.ts @@ -12,7 +12,7 @@ export default function (apiKey: string): ScannerImplementation { return createScanner({ maxSending: 30, maxBatchLength: 1, - fetchStrategy: async (purls, artifacts) => { + fetchStrategy: async (purls) => { const body = JSON.stringify({ components: purls.map(purl => ({ purl })) } satisfies SocketBatchEndpointBody) @@ -33,7 +33,7 @@ export default function (apiKey: string): ScannerImplementation { const data = await res.text() - artifacts.push(...data.split('\n').filter(Boolean).map(line => JSON.parse(line))) + return data.split('\n').filter(Boolean).map(line => JSON.parse(line)) } }) } diff --git a/src/modes/unauthenticated.ts b/src/modes/unauthenticated.ts index ea41179..e8cce36 100644 --- a/src/modes/unauthenticated.ts +++ b/src/modes/unauthenticated.ts @@ -1,4 +1,4 @@ -import type { ScannerImplementation } from '../types' +import type { ScannerImplementation, SocketArtifact } from '../types' import { createScanner } from '../scanner-factory' import { userAgent } from './user-agent' @@ -6,8 +6,9 @@ export default function (): ScannerImplementation { return createScanner({ maxSending: 20, maxBatchLength: 50, - fetchStrategy: async (purls, artifacts) => { + fetchStrategy: async (purls) => { const urls = purls.map(purl => `https://firewall-api.socket.dev/purl/${encodeURIComponent(purl)}`) + const results: SocketArtifact[] = [] await Promise.all(urls.map(async url => { const res = await fetch(url, { headers: { @@ -18,8 +19,9 @@ export default function (): ScannerImplementation { throw new Error(`Socket Security Scanner: Received ${res.status} from server`) } const data = await res.text() - artifacts.push(...data.split('\n').filter(Boolean).map(line => JSON.parse(line))) + results.push(...data.split('\n').filter(Boolean).map(line => JSON.parse(line))) })) + return results } }) } diff --git a/src/scanner-factory.ts b/src/scanner-factory.ts index 22b6236..a6c1231 100644 --- a/src/scanner-factory.ts +++ b/src/scanner-factory.ts @@ -3,23 +3,23 @@ import type { SocketArtifact, ScannerImplementation } from './types' type ScannerConfig = { maxSending: number maxBatchLength: number - fetchStrategy: (purls: string[], artifacts: SocketArtifact[]) => Promise + fetchStrategy: (purls: string[]) => Promise } export function createScanner({ maxSending, maxBatchLength, fetchStrategy }: ScannerConfig): ScannerImplementation { return async function*(packages) { let artifacts: SocketArtifact[] = [] let batch: Bun.Security.Package[] = [] - let in_flight = 0 + let inFlight = 0 const pending: Set> = new Set() async function startFlight() { const purls = batch.map(p => `pkg:npm/${p.name}@${p.version}`) batch = [] - in_flight += purls.length + inFlight += purls.length - if (in_flight >= maxSending) { + if (inFlight >= maxSending) { if (pending.size !== 0) { await Promise.race([...pending]) } else { @@ -27,12 +27,14 @@ export function createScanner({ maxSending, maxBatchLength, fetchStrategy }: Sca } } - const flight = fetchStrategy(purls, artifacts) + const flight = fetchStrategy(purls).then(results => { + artifacts.push(...results) + }) pending.add(flight) flight.finally(() => { - in_flight -= purls.length + inFlight -= purls.length pending.delete(flight) }) } diff --git a/test/scanner-factory.test.ts b/test/scanner-factory.test.ts index 8de4e30..0c467e2 100644 --- a/test/scanner-factory.test.ts +++ b/test/scanner-factory.test.ts @@ -26,8 +26,9 @@ const mockPackages: Bun.Security.Package[] = [ describe('scanner-factory', () => { test('should call fetchStrategy with correct purls', async () => { const capturedPurls: string[] = [] - const fetchStrategy = async (purls: string[], artifacts: SocketArtifact[]) => { + const fetchStrategy = async (purls: string[]) => { capturedPurls.push(...purls) + return [] } const scanner = createScanner({ @@ -51,8 +52,9 @@ describe('scanner-factory', () => { test('should respect maxBatchLength', async () => { const batchSizes: number[] = [] - const fetchStrategy = async (purls: string[], artifacts: SocketArtifact[]) => { + const fetchStrategy = async (purls: string[]) => { batchSizes.push(purls.length) + return [] } const scanner = createScanner({ @@ -83,8 +85,8 @@ describe('scanner-factory', () => { } ] - const fetchStrategy = async (purls: string[], artifacts: SocketArtifact[]) => { - artifacts.push(...mockArtifacts) + const fetchStrategy = async (purls: string[]) => { + return [...mockArtifacts] } const scanner = createScanner({ @@ -105,9 +107,10 @@ describe('scanner-factory', () => { test('should not call fetchStrategy with empty batch', async () => { let callCount = 0 - const fetchStrategy = async (purls: string[], artifacts: SocketArtifact[]) => { + const fetchStrategy = async (purls: string[]) => { callCount++ expect(purls.length).toBeGreaterThan(0) + return [] } const scanner = createScanner({ @@ -128,8 +131,9 @@ describe('scanner-factory', () => { test('should handle empty package list', async () => { let callCount = 0 - const fetchStrategy = async (purls: string[], artifacts: SocketArtifact[]) => { + const fetchStrategy = async (purls: string[]) => { callCount++ + return [] } const scanner = createScanner({ @@ -152,7 +156,7 @@ describe('scanner-factory', () => { let currentInFlight = 0 const delays: Promise[] = [] - const fetchStrategy = async (purls: string[], artifacts: SocketArtifact[]) => { + const fetchStrategy = async (purls: string[]) => { currentInFlight += purls.length maxConcurrent = Math.max(maxConcurrent, currentInFlight) @@ -162,6 +166,7 @@ describe('scanner-factory', () => { await delay currentInFlight -= purls.length + return [] } const manyPackages = Array.from({ length: 50 }, (_, i) => ({ @@ -189,11 +194,11 @@ describe('scanner-factory', () => { test('should yield artifacts progressively with maxBatchLength', async () => { let batchIndex = 0 - const fetchStrategy = async (purls: string[], artifacts: SocketArtifact[]) => { - artifacts.push({ + const fetchStrategy = async (purls: string[]) => { + return [{ inputPurl: `batch-${batchIndex++}`, alerts: [] - }) + }] } const scanner = createScanner({ @@ -213,4 +218,221 @@ describe('scanner-factory', () => { // Should yield twice, once per batch expect(yielded.length).toEqual(2) }) + +}) + +// Helper to create a promise whose resolution is controlled externally +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise(r => { resolve = r }) + return { promise, resolve } +} + +function makePkg(name: string, version = '1.0.0'): Bun.Security.Package { + return { + name, + version, + requestedRange: `^${version}`, + tarball: `https://registry.npmjs.org/${name}/-/${name}-${version}.tgz`, + } +} + +function makeArtifact(purl: string): SocketArtifact { + return { inputPurl: purl, alerts: [{ action: 'error', type: 'test', props: {} }] } +} + +describe('scanner-factory race condition regression', () => { + test('late-resolving flight delivers artifacts after array swap', async () => { + // Flight 1 (pkg-a) is launched, doesn't resolve yet. + // Flight 2 (pkg-b) is launched and resolves immediately, triggering a yield + swap. + // Flight 1 then resolves. Its artifact must not be lost. + const gate = deferred() + let call = 0 + + const fetchStrategy = async (purls: string[]) => { + call++ + if (call === 1) { + // First flight: wait until we release the gate + return gate.promise + } + // Second flight: resolve immediately + return purls.map(p => makeArtifact(p)) + } + + const scanner = createScanner({ maxSending: 30, maxBatchLength: 1, fetchStrategy }) + const results = scanner([makePkg('pkg-a'), makePkg('pkg-b')]) + const allArtifacts: SocketArtifact[] = [] + + // Start consuming the async generator. After pkg-b's flight resolves + // immediately, the generator yields and swaps artifacts = []. + // Then release pkg-a's flight so it resolves into the new array. + const consumer = (async () => { + for await (const batch of results) { + allArtifacts.push(...batch) + // After the first yield, release the slow flight + if (call >= 2) { + gate.resolve([makeArtifact('pkg:npm/pkg-a@1.0.0')]) + } + } + })() + + await consumer + + const purls = allArtifacts.map(a => a.inputPurl).sort() + expect(purls).toEqual(['pkg:npm/pkg-a@1.0.0', 'pkg:npm/pkg-b@1.0.0']) + }) + + test('all flights pending until final drain collects every artifact', async () => { + // All flights stay pending until after the while-loop ends. + // Everything should be collected at the final `await Promise.all` + yield. + const gates: { resolve: (v: SocketArtifact[]) => void }[] = [] + + const fetchStrategy = async (purls: string[]) => { + const d = deferred() + gates.push(d) + return d.promise + } + + const packages = Array.from({ length: 5 }, (_, i) => makePkg(`pkg${i}`)) + + const scanner = createScanner({ maxSending: 30, maxBatchLength: 1, fetchStrategy }) + const results = scanner([...packages]) + const allArtifacts: SocketArtifact[] = [] + + // Start consuming — the generator will loop through all packages, launching + // flights that never resolve, so no intermediate yields happen. + // After all packages are dispatched, it hits `await Promise.all([...pending])`. + // We then resolve all gates. + const consumer = (async () => { + for await (const batch of results) { + allArtifacts.push(...batch) + } + })() + + // Wait a tick for all flights to be dispatched + await new Promise(r => setTimeout(r, 10)) + + // Now resolve all gates + for (let i = 0; i < gates.length; i++) { + gates[i]!.resolve([makeArtifact(`pkg:npm/pkg${i}@1.0.0`)]) + } + + await consumer + + expect(allArtifacts.length).toBe(5) + const purls = allArtifacts.map(a => a.inputPurl).sort() + expect(purls).toEqual( + Array.from({ length: 5 }, (_, i) => `pkg:npm/pkg${i}@1.0.0`).sort() + ) + }) + + test('staggered resolution across multiple yield cycles loses nothing', async () => { + // Odd-numbered flights resolve slowly, even-numbered resolve instantly. + // With maxBatchLength=1 this forces multiple yield+swap cycles with + // in-flight stragglers from previous cycles. + const gates: { resolve: (v: SocketArtifact[]) => void }[] = [] + + const fetchStrategy = async (purls: string[]) => { + const purl = purls[0]! + const idx = parseInt(purl.match(/pkg(\d+)/)![1]!) + if (idx % 2 === 1) { + // Odd: delay resolution + const d = deferred() + gates.push({ resolve: (v) => d.resolve(v) }) + return d.promise + } + // Even: resolve immediately + return [makeArtifact(purl)] + } + + const packages = Array.from({ length: 8 }, (_, i) => makePkg(`pkg${i}`)) + + const scanner = createScanner({ maxSending: 30, maxBatchLength: 1, fetchStrategy }) + const results = scanner([...packages]) + const allArtifacts: SocketArtifact[] = [] + + const consumer = (async () => { + for await (const batch of results) { + allArtifacts.push(...batch) + } + })() + + // Wait for all flights to be dispatched, then release the slow ones + await new Promise(r => setTimeout(r, 20)) + for (let i = 0; i < gates.length; i++) { + const oddIdx = i * 2 + 1 + gates[i]!.resolve([makeArtifact(`pkg:npm/pkg${oddIdx}@1.0.0`)]) + } + + await consumer + + expect(allArtifacts.length).toBe(8) + const purls = allArtifacts.map(a => a.inputPurl).sort() + expect(purls).toEqual( + Array.from({ length: 8 }, (_, i) => `pkg:npm/pkg${i}@1.0.0`).sort() + ) + }) + + test('authenticated-mode scenario: 50+ packages with maxBatchLength=1 and maxSending=30', async () => { + // Mirrors the real customer config that triggered the bug report. + // Variable latency with high concurrency — every artifact must survive. + const fetchStrategy = async (purls: string[]) => { + await new Promise(r => setTimeout(r, Math.random() * 30)) + return purls.map(p => makeArtifact(p)) + } + + const packages = Array.from({ length: 60 }, (_, i) => makePkg(`dep${i}`)) + + const scanner = createScanner({ maxSending: 30, maxBatchLength: 1, fetchStrategy }) + const results = scanner([...packages]) + const allArtifacts: SocketArtifact[] = [] + + for await (const batch of results) { + allArtifacts.push(...batch) + } + + expect(allArtifacts.length).toBe(60) + const purls = allArtifacts.map(a => a.inputPurl).sort() + const expected = packages.map(p => `pkg:npm/${p.name}@${p.version}`).sort() + expect(purls).toEqual(expected) + }) + + test('artifacts from a flight that resolves during yield are not lost', async () => { + // Specifically tests: flight 1 resolves, generator yields and swaps, + // and flight 2 resolves during that same tick. Flight 2's results + // must land in the new array, not the old one. + let flightCount = 0 + const slow = deferred() + + const fetchStrategy = async (purls: string[]) => { + flightCount++ + if (flightCount <= 2) { + // First two flights resolve immediately + return purls.map(p => makeArtifact(p)) + } + // Third flight: delayed + return slow.promise + } + + const scanner = createScanner({ maxSending: 30, maxBatchLength: 1, fetchStrategy }) + const results = scanner([makePkg('a'), makePkg('b'), makePkg('c')]) + const allArtifacts: SocketArtifact[] = [] + let yieldCount = 0 + + const consumer = (async () => { + for await (const batch of results) { + yieldCount++ + allArtifacts.push(...batch) + // After first yield, resolve the slow flight + if (yieldCount === 1) { + slow.resolve([makeArtifact('pkg:npm/c@1.0.0')]) + } + } + })() + + await consumer + + const purls = allArtifacts.map(a => a.inputPurl).sort() + expect(purls).toEqual(['pkg:npm/a@1.0.0', 'pkg:npm/b@1.0.0', 'pkg:npm/c@1.0.0']) + }) }) From e4f6eb540ab027fb4b597e7d2439068540455a49 Mon Sep 17 00:00:00 2001 From: Bill Li Date: Thu, 16 Apr 2026 17:17:09 +0100 Subject: [PATCH 2/2] fix: missing prop from purl endpoint --- .github/workflows/test.yml | 2 ++ src/index.ts | 6 +++--- src/types.ts | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index cace609..d2f56bc 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -29,6 +29,8 @@ jobs: - name: Setup Bun uses: oven-sh/setup-bun@22457c87c1b161cf7dde222c3e82b2b5f8d2bed2 + with: + no-cache: true - name: Install dependencies run: bun install diff --git a/src/index.ts b/src/index.ts index 552898a..4c01b76 100644 --- a/src/index.ts +++ b/src/index.ts @@ -62,15 +62,15 @@ export const scanner: Bun.Security.Scanner = { for (const alert of artifact.alerts) { const description = [''] - if (alert.type === 'didYouMean') { + if (alert.type === 'didYouMean' && alert.props?.alternatePackage) { description.push(`This package could be a typo-squatting attempt of another package (${alert.props.alternatePackage}).`) } - if (alert.props.description) { + if (alert.props?.description) { description.push(alert.props.description) } - if (alert.props.note) { + if (alert.props?.note) { description.push(alert.props.note) } diff --git a/src/types.ts b/src/types.ts index b5b8b4b..bc4601f 100644 --- a/src/types.ts +++ b/src/types.ts @@ -5,7 +5,7 @@ export type SocketArtifact = { alerts: { action: 'error' | 'warn' type: string, - props: { + props?: { note?: string, didYouMean?: string, } & Record