Skip to content

Commit 305cc3b

Browse files
authored
fix: replace fetch() with SDK HTTP client in checkMalware and route by component count (#544)
1 parent 7a22968 commit 305cc3b

File tree

7 files changed

+222
-165
lines changed

7 files changed

+222
-165
lines changed

CLAUDE.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,13 @@ All `logger.error()` and `logger.log()` calls include empty string:
151151
- **Type imports**: Always `import type`
152152
- **Nullish values**: Prefer `undefined` over `null` - use `undefined` for absent/missing values
153153

154+
#### HTTP Requests
155+
156+
- **🚨 NEVER use `fetch()`** - use `createGetRequest`/`createRequestWithJson` from `src/http-client.ts`
157+
- `fetch()` bypasses the SDK's HTTP stack (retries, timeouts, hooks, agent config)
158+
- `fetch()` cannot be intercepted by nock in tests, forcing c8 ignore blocks
159+
- For external URLs (e.g., firewall API), pass a different `baseUrl` to `createGetRequest`
160+
154161
#### Working Directory
155162

156163
- **🚨 NEVER use `process.chdir()`** - use `{ cwd }` options and absolute paths instead

src/constants.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@ export const MAX_STREAM_SIZE = 100 * 1024 * 1024
5252
// Public blob store URL for patch downloads
5353
export const SOCKET_PUBLIC_BLOB_STORE_URL = 'https://socketusercontent.com'
5454

55+
// Max components to check via parallel firewall API before switching to batch.
56+
export const MAX_FIREWALL_COMPONENTS = 8
57+
5558
// Public firewall API URL for per-package lookups (unauthenticated, fast).
5659
export const SOCKET_FIREWALL_API_URL = 'https://firewall-api.socket.dev/purl'
5760

src/socket-sdk-class.ts

Lines changed: 20 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import {
2929
DEFAULT_RETRY_DELAY,
3030
DEFAULT_USER_AGENT,
3131
httpAgentNames,
32+
MAX_FIREWALL_COMPONENTS,
3233
MAX_HTTP_TIMEOUT,
3334
MAX_STREAM_SIZE,
3435
MIN_HTTP_TIMEOUT,
@@ -966,52 +967,48 @@ export class SocketSdk {
966967
/**
967968
* Check packages for malware and security alerts.
968969
*
969-
* For public tokens, uses the firewall API (per-package, unauthenticated)
970-
* which returns full artifact data including score and alert details.
971-
* Alerts are filtered using the client-side publicPolicy map.
970+
* For small sets (≤ MAX_FIREWALL_COMPONENTS), uses parallel firewall API
971+
* requests which return full artifact data including score and alert details.
972972
*
973-
* For org tokens, uses the batch PURL API with full org policy.
974-
* Alerts are filtered using the server-assigned action.
973+
* For larger sets, uses the batch PURL API for efficiency.
975974
*
976-
* Both paths return the same normalized result shape.
975+
* Both paths normalize alerts through publicPolicy and only return
976+
* malware-relevant results.
977977
*
978978
* @param components - Array of package URLs to check
979979
* @returns Normalized results with policy-filtered alerts per package
980980
*/
981981
async checkMalware(
982982
components: Array<{ purl: string }>,
983983
): Promise<SocketSdkGenericResult<MalwareCheckResult>> {
984-
const isPublicToken = this.#apiToken === SOCKET_PUBLIC_API_TOKEN
985-
/* c8 ignore next 3 - c8 ignored: because public token path uses global fetch() to SOCKET_FIREWALL_API_URL which cannot be intercepted by nock or local HTTP servers; tested via the isolated checkMalware test with mocked fetch */
986-
if (isPublicToken) {
984+
if (components.length <= MAX_FIREWALL_COMPONENTS) {
987985
return this.#checkMalwareFirewall(components)
988986
}
989987
return this.#checkMalwareBatch(components)
990988
}
991989

992-
// Public token path: parallel firewall API requests per PURL.
990+
// Small-set path: parallel firewall API requests per PURL.
993991
// Returns full artifact data (score, alert props, categories, fix info).
994-
/* c8 ignore start - c8 ignored: because #checkMalwareFirewall uses global fetch() to an external URL (firewall-api.socket.dev) that cannot be intercepted in the main test suite */
995992
async #checkMalwareFirewall(
996993
components: Array<{ purl: string }>,
997994
): Promise<SocketSdkGenericResult<MalwareCheckResult>> {
998995
const packages: MalwareCheckPackage[] = []
999996
const results = await Promise.allSettled(
1000997
components.map(async ({ purl }) => {
1001-
const url = `${SOCKET_FIREWALL_API_URL}/${encodeURIComponent(purl)}`
1002-
const resp = await fetch(url, {
1003-
signal: AbortSignal.timeout(
1004-
this.#reqOptions.timeout ?? DEFAULT_HTTP_TIMEOUT,
1005-
),
1006-
})
1007-
if (!resp.ok) return undefined
1008-
return (await resp.json()) as Record<string, unknown>
998+
const urlPath = `/${encodeURIComponent(purl)}`
999+
const response = await createGetRequest(
1000+
SOCKET_FIREWALL_API_URL,
1001+
urlPath,
1002+
this.#reqOptions,
1003+
)
1004+
if (!isResponseOk(response)) return undefined
1005+
const json = await getResponseJson(response)
1006+
return json as unknown as SocketArtifact
10091007
}),
10101008
)
10111009
for (const settled of results) {
10121010
if (settled.status === 'rejected' || !settled.value) continue
1013-
const artifact = settled.value as SocketArtifact
1014-
packages.push(SocketSdk.#normalizeArtifact(artifact, publicPolicy))
1011+
packages.push(SocketSdk.#normalizeArtifact(settled.value, publicPolicy))
10151012
}
10161013
return {
10171014
cause: undefined,
@@ -1021,9 +1018,8 @@ export class SocketSdk {
10211018
success: true,
10221019
}
10231020
}
1024-
/* c8 ignore stop */
10251021

1026-
// Org token path: single batch PURL API request.
1022+
// Multi-component path: batch PURL API request, normalized to publicPolicy.
10271023
async #checkMalwareBatch(
10281024
components: Array<{ purl: string }>,
10291025
): Promise<SocketSdkGenericResult<MalwareCheckResult>> {
@@ -1042,7 +1038,7 @@ export class SocketSdk {
10421038
}
10431039
const packages: MalwareCheckPackage[] = []
10441040
for (const artifact of result.data as SocketArtifact[]) {
1045-
packages.push(SocketSdk.#normalizeArtifact(artifact))
1041+
packages.push(SocketSdk.#normalizeArtifact(artifact, publicPolicy))
10461042
}
10471043
return {
10481044
cause: undefined,

test/unit/coverage-non-error-paths.test.mts

Lines changed: 61 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
* #executeWithRetry onRetry branches (401/403, 429 with Retry-After),
1313
* #getResponseText 50MB size limit,
1414
* #getTtlForEndpoint / cache config (number, object with endpoint, default),
15-
* #checkMalwareFirewall internals (rejected/undefined settled),
15+
* #checkMalwareBatch normalize with publicPolicy,
1616
* downloadOrgFullScanFilesAsTar streaming,
1717
* streamFullScan data/error/end handlers,
1818
* uploadManifestFiles edge case
@@ -27,6 +27,7 @@ import { PassThrough } from 'node:stream'
2727

2828
import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest'
2929

30+
import { MAX_FIREWALL_COMPONENTS } from '../../src/constants.js'
3031
import {
3132
createRequestBodyForFilepaths,
3233
createUploadRequest,
@@ -641,67 +642,70 @@ describe('SocketSdk - #parseRetryAfter via retry behavior', () => {
641642
})
642643

643644
// =============================================================================
644-
// 4e. socket-sdk-class.ts — #checkMalwareFirewall internals (lines 990-1018)
645-
// Specifically: rejected promise and resp.ok=false branches
645+
// 4e. socket-sdk-class.ts — #checkMalwareBatch normalize with publicPolicy
646+
// Specifically: alerts with/without fix, ignore actions filtered
646647
// =============================================================================
647648

648-
describe('SocketSdk - checkMalwareFirewall internals', () => {
649+
describe('SocketSdk - checkMalware batch normalize with publicPolicy', () => {
650+
const artifact = {
651+
alerts: [
652+
{
653+
category: 'supplyChainRisk',
654+
fix: { description: 'Remove package', type: 'remove' },
655+
key: 'mal-1',
656+
props: { note: 'data exfil' },
657+
severity: 'critical',
658+
type: 'malware',
659+
},
660+
{
661+
// Alert without fix property — criticalCVE is 'warn' in publicPolicy
662+
category: 'quality',
663+
key: 'cve-1',
664+
props: {},
665+
severity: 'high',
666+
type: 'criticalCVE',
667+
},
668+
{
669+
// deprecated is 'ignore' in publicPolicy — should be filtered out
670+
category: 'misc',
671+
key: 'dep-1',
672+
props: {},
673+
severity: 'low',
674+
type: 'deprecated',
675+
},
676+
],
677+
name: 'evil-pkg',
678+
namespace: undefined,
679+
score: {
680+
license: 0.9,
681+
maintenance: 0.8,
682+
overall: 0.1,
683+
quality: 0.7,
684+
supplyChain: 0.0,
685+
vulnerability: 0.0,
686+
},
687+
type: 'npm',
688+
version: '1.0.0',
689+
}
690+
649691
const getBaseUrl = setupLocalHttpServer(
650692
(req: IncomingMessage, res: ServerResponse) => {
651693
const url = req.url || ''
652694

653-
// Batch purl path (org token) — exercises #normalizeArtifact.
695+
// Batch purl path — exercises #normalizeArtifact with publicPolicy.
654696
let body = ''
655697
req.on('data', (chunk: Buffer) => {
656698
body += chunk.toString()
657699
})
658700
req.on('end', () => {
659701
if (url.includes('/purl') && req.method === 'POST') {
660-
const artifact = {
661-
alerts: [
662-
{
663-
action: 'error',
664-
category: 'supplyChainRisk',
665-
fix: { description: 'Remove package', type: 'remove' },
666-
key: 'mal-1',
667-
props: { note: 'data exfil' },
668-
severity: 'critical',
669-
type: 'malware',
670-
},
671-
{
672-
// Alert without fix property
673-
action: 'warn',
674-
category: 'quality',
675-
key: 'cve-1',
676-
props: {},
677-
severity: 'high',
678-
type: 'criticalCVE',
679-
},
680-
{
681-
// Alert with ignore action — should be filtered out
682-
action: 'ignore',
683-
category: 'misc',
684-
key: 'dep-1',
685-
props: {},
686-
severity: 'low',
687-
type: 'deprecated',
688-
},
689-
],
690-
name: 'evil-pkg',
691-
namespace: undefined,
692-
score: {
693-
license: 0.9,
694-
maintenance: 0.8,
695-
overall: 0.1,
696-
quality: 0.7,
697-
supplyChain: 0.0,
698-
vulnerability: 0.0,
699-
},
700-
type: 'npm',
701-
version: '1.0.0',
702-
}
702+
const parsed = JSON.parse(body)
703+
const count = parsed.components?.length ?? 0
704+
const lines = Array.from({ length: count }, () =>
705+
JSON.stringify(artifact),
706+
).join('\n')
703707
res.writeHead(200, { 'Content-Type': 'application/x-ndjson' })
704-
res.end(`${JSON.stringify(artifact)}\n`)
708+
res.end(`${lines}\n`)
705709
} else {
706710
res.writeHead(404)
707711
res.end()
@@ -711,21 +715,23 @@ describe('SocketSdk - checkMalwareFirewall internals', () => {
711715
)
712716

713717
it('should normalize artifact with fix and without fix, filtering ignore actions', async () => {
714-
const client = new SocketSdk('org-api-token', {
718+
const count = MAX_FIREWALL_COMPONENTS + 1
719+
const client = new SocketSdk('test-api-token', {
715720
baseUrl: `${getBaseUrl()}/v0/`,
716721
retries: 0,
717722
})
718723

719-
const result = await client.checkMalware([
720-
{ purl: 'pkg:npm/evil-pkg@1.0.0' },
721-
])
724+
const components = Array.from({ length: count }, (_, i) => ({
725+
purl: `pkg:npm/evil-pkg@${i + 1}.0.0`,
726+
}))
727+
const result = await client.checkMalware(components)
722728

723729
expect(result.success).toBe(true)
724730
if (!result.success) return
725-
expect(result.data).toHaveLength(1)
731+
expect(result.data).toHaveLength(count)
726732
const pkg = result.data[0]!
727733

728-
// Two alerts should remain (error + warn), ignore is filtered
734+
// Two alerts should remain (error + warn via publicPolicy), deprecated is filtered
729735
expect(pkg.alerts).toHaveLength(2)
730736

731737
// First alert has fix

0 commit comments

Comments
 (0)