Skip to content

Commit 000c379

Browse files
committed
fix(cdxgen): report why cdxgen failed
When `socket cdxgen` failed on a CI runner it could exit 1 and print nothing at all: no error, no hint, no way to tell whether cdxgen was never downloaded, never started, or ran and died. The command armed `process.exitCode = 1` before starting cdxgen, then handled only the signal and numeric-exit-code cases. There was no `else`, so a child that reported neither left the armed 1 standing and printed nothing. Because cdxgen is spawned with `stdio: 'inherit'`, a rejected spawn also reached the top level with an empty stderr, leaving the shared formatter nothing to attach beyond a generic line. Every way out of the run now either exits with cdxgen's own code or prints where the CLI looked for cdxgen and what to try next. A new `util/dlx/cdxgen-diagnostics.mts` builds those messages so the command and the spawn helper share one wording. `spawnCdxgenDlx` also checks `SOCKET_CLI_CDXGEN_LOCAL_PATH` on disk before spawning, so a wrong path says so instead of surfacing as a bare ENOENT that never names the variable. An `InputError` message is passed through untouched rather than nested inside a second Where/Saw/Fix block, which previously produced an outer `Fix:` telling the user to set the very variable they had already set. The underlying error is logged via `debugNs('error', ...)` so the message's advice to re-run with `SOCKET_CLI_DEBUG=1` leads somewhere. The successful path is unchanged: cdxgen's output still streams through and its exit code is still forwarded exactly as before. Refs SURF-1045.
1 parent 0bd8b9e commit 000c379

6 files changed

Lines changed: 505 additions & 3 deletions

File tree

packages/cli/src/commands/manifest/cmd-manifest-cdxgen.mts

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,15 @@ import terminalLink from 'terminal-link'
66
import yargsParse from 'yargs-parser'
77

88
import { joinAnd } from '@socketsecurity/lib-stable/arrays/join'
9+
import { debugNs } from '@socketsecurity/lib-stable/debug/output'
910
import { getDefaultLogger } from '@socketsecurity/lib-stable/logger/default'
1011
import { isPath } from '@socketsecurity/lib-stable/paths/normalize'
1112
import { pluralize } from '@socketsecurity/lib-stable/words/pluralize'
1213

14+
import {
15+
describeCdxgenSource,
16+
formatCdxgenFailureMessage,
17+
} from '../../util/dlx/cdxgen-diagnostics.mts'
1318
import {
1419
detectNodejsCdxgenSources,
1520
isNodejsCdxgenType,
@@ -336,15 +341,36 @@ export async function run(
336341
}
337342
}
338343

344+
// Assume failure until cdxgen reports otherwise, so an unexpected early exit
345+
// cannot be mistaken for a successful scan.
339346
process.exitCode = 1
340347

341-
const { spawnPromise } = await runCdxgen(yargv)
348+
let result
349+
try {
350+
const { spawnPromise } = await runCdxgen(yargv)
351+
result = await spawnPromise
352+
} catch (e) {
353+
// cdxgen runs with stdio: 'inherit', so a failure to start it produces no
354+
// child output at all. Say where we looked and what to try next.
355+
// The message tells the user to re-run with SOCKET_CLI_DEBUG=1, so put the
356+
// underlying error on the 'error' category that flag turns on. Handling it
357+
// here means it never reaches the top level, and nothing else on this path
358+
// logs it, so without this the advice would lead nowhere.
359+
debugNs('error', `cdxgen failed to run while ${describeCdxgenSource()}`, e)
360+
logger.fail(formatCdxgenFailureMessage(e))
361+
return
362+
}
342363

343-
// Wait for the spawn promise to resolve and handle the result.
344-
const result = await spawnPromise
345364
if (result.signal) {
346365
process.kill(process.pid, result.signal)
347366
} else if (typeof result.code === 'number') {
348367
process.exit(result.code)
368+
} else {
369+
// Neither an exit code nor a signal. Without this branch the command
370+
// returns here with the exit code armed above and prints nothing at all.
371+
// There is no error to show, so the spawn result is what SOCKET_CLI_DEBUG=1
372+
// has to offer.
373+
debugNs('error', 'cdxgen returned no exit code and no signal', result)
374+
logger.fail(formatCdxgenFailureMessage())
349375
}
350376
}
Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,89 @@
1+
/**
2+
* Diagnostics for the `socket cdxgen` command.
3+
*
4+
* Cdxgen runs as a child process, so when it cannot start there is nothing on
5+
* stdout or stderr to explain why. These helpers turn that into a message that
6+
* names where the CLI looked for cdxgen and what to try next.
7+
*/
8+
9+
import { existsSync } from 'node:fs'
10+
11+
import { errorMessage } from '@socketsecurity/lib-stable/errors/message'
12+
13+
import { resolveCdxgen } from './resolve-binary.mjs'
14+
import { InputError } from '../error/errors-types.mts'
15+
16+
/**
17+
* Describe where the CLI resolved cdxgen from, so an error can say which of
18+
* the two sources actually failed.
19+
*/
20+
export function describeCdxgenSource(): string {
21+
const resolution = resolveCdxgen()
22+
if (resolution.type === 'local') {
23+
return `the local override SOCKET_CLI_CDXGEN_LOCAL_PATH=${resolution.path}`
24+
}
25+
if (resolution.type === 'dlx') {
26+
const { name, version } = resolution.details
27+
return `${name}@${version}, downloaded and cached by Socket's dlx`
28+
}
29+
return "a binary downloaded and cached by Socket's dlx"
30+
}
31+
32+
/**
33+
* Build the message shown when cdxgen fails to produce a result.
34+
*
35+
* Pass the underlying error when there is one. Pass nothing for the case where
36+
* the child process ended without reporting either an exit code or a signal,
37+
* which is the shape that used to exit 1 with no output at all.
38+
*/
39+
export function formatCdxgenFailureMessage(
40+
cause?: unknown | undefined,
41+
): string {
42+
// An InputError is raised with a message already written for the exact thing
43+
// that went wrong, so pass it through. Wrapping it would nest one
44+
// Where/Saw/Fix block inside another, and the generic Fix below would tell
45+
// the user to set SOCKET_CLI_CDXGEN_LOCAL_PATH when a bad value for that
46+
// very variable is what they are being told about.
47+
if (cause instanceof InputError) {
48+
return cause.message
49+
}
50+
51+
const detail = cause
52+
? errorMessage(cause)
53+
: 'the cdxgen process ended without reporting an exit code or a signal'
54+
55+
return [
56+
'socket cdxgen could not run cdxgen.',
57+
` Where: ${describeCdxgenSource()}`,
58+
` Saw: ${detail || 'no error message was reported'}`,
59+
' Fix: Re-run with SOCKET_CLI_DEBUG=1 to see the full error. If cdxgen',
60+
' cannot be downloaded on this machine, install it yourself and',
61+
' point SOCKET_CLI_CDXGEN_LOCAL_PATH at the binary.',
62+
].join('\n')
63+
}
64+
65+
/**
66+
* Build the message shown when SOCKET_CLI_CDXGEN_LOCAL_PATH points at
67+
* something that is not there.
68+
*
69+
* Without this check the override is silently ignored and the failure looks
70+
* identical to a download failure, which makes the override appear to have no
71+
* effect at all.
72+
*/
73+
export function formatMissingCdxgenLocalPathMessage(localPath: string): string {
74+
return [
75+
'SOCKET_CLI_CDXGEN_LOCAL_PATH points at a file that does not exist.',
76+
` Where: ${localPath}`,
77+
' Saw: nothing at that path',
78+
' Fix: Correct the path, or unset SOCKET_CLI_CDXGEN_LOCAL_PATH to let',
79+
' Socket download cdxgen itself. On a CI runner, check that the',
80+
' path exists in the same step that runs socket cdxgen.',
81+
].join('\n')
82+
}
83+
84+
/**
85+
* True when a local cdxgen override is configured but missing from disk.
86+
*/
87+
export function isMissingCdxgenLocalPath(localPath: string): boolean {
88+
return !existsSync(localPath)
89+
}

packages/cli/src/util/dlx/spawn-cdxgen.mts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,14 @@
1313
import { detectExecutableType } from '@socketsecurity/lib-stable/dlx/detect'
1414
import { spawn } from '@socketsecurity/lib-stable/process/spawn/child'
1515

16+
import {
17+
formatMissingCdxgenLocalPathMessage,
18+
isMissingCdxgenLocalPath,
19+
} from './cdxgen-diagnostics.mts'
1620
import { defineAutoDispatch, defineVfsSpawn } from './define-tool-spawn.mts'
1721
import { spawnDlx } from './spawn.mts'
1822
import { resolveCdxgen } from './resolve-binary.mjs'
23+
import { InputError } from '../error/errors.mts'
1924
import { buildSystemToolEnv } from '../spawn/system-tool.mts'
2025
import { resolveNodeExecutable } from '../spawn/spawn-node.mts'
2126

@@ -37,6 +42,12 @@ export async function spawnCdxgenDlx(
3742

3843
// Use local cdxgen if available.
3944
if (resolution.type === 'local') {
45+
// Check the override before spawning. Otherwise a wrong path surfaces as a
46+
// bare ENOENT that never mentions the environment variable, so the
47+
// override looks like it was ignored.
48+
if (isMissingCdxgenLocalPath(resolution.path)) {
49+
throw new InputError(formatMissingCdxgenLocalPathMessage(resolution.path))
50+
}
4051
const detection = detectExecutableType(resolution.path)
4152
const { env: spawnEnv, ...dlxOptions } = {
4253
__proto__: null,
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
/**
2+
* Unit tests for the cdxgen command's failure reporting.
3+
*
4+
* Purpose: The command arms process.exitCode = 1 before spawning cdxgen, and
5+
* cdxgen itself runs with stdio: 'inherit'. Together that means any path out
6+
* of the spawn that neither exits deliberately nor prints leaves the user with
7+
* exit code 1 and an empty log. These tests cover every one of those paths.
8+
*
9+
* Related Files: - src/commands/manifest/cmd-manifest-cdxgen.mts.
10+
*/
11+
12+
import { beforeEach, describe, expect, it, vi } from 'vitest'
13+
14+
import { cmdManifestCdxgen } from '../../../../src/commands/manifest/cmd-manifest-cdxgen.mts'
15+
import { formatMissingCdxgenLocalPathMessage } from '../../../../src/util/dlx/cdxgen-diagnostics.mts'
16+
import { InputError } from '../../../../src/util/error/errors-types.mts'
17+
18+
const mockLogger = vi.hoisted(() => ({
19+
error: vi.fn(),
20+
fail: vi.fn(),
21+
info: vi.fn(),
22+
log: vi.fn(),
23+
success: vi.fn(),
24+
warn: vi.fn(),
25+
}))
26+
27+
vi.mock(import('@socketsecurity/lib-stable/logger/default'), () => ({
28+
getDefaultLogger: () => mockLogger,
29+
}))
30+
31+
const mockDebugNs = vi.hoisted(() => vi.fn())
32+
33+
vi.mock(
34+
import('@socketsecurity/lib-stable/debug/output'),
35+
async importOriginal => ({
36+
...(await importOriginal()),
37+
debugNs: mockDebugNs,
38+
}),
39+
)
40+
41+
const mockRunCdxgen = vi.hoisted(() => vi.fn())
42+
const mockDetectNodejsCdxgenSources = vi.hoisted(() =>
43+
vi.fn().mockResolvedValue({ hasLockfile: true, hasNodeModules: true }),
44+
)
45+
const mockIsNodejsCdxgenType = vi.hoisted(() => vi.fn().mockReturnValue(true))
46+
47+
vi.mock(import('../../../../src/commands/manifest/run-cdxgen.mts'), () => ({
48+
detectNodejsCdxgenSources: mockDetectNodejsCdxgenSources,
49+
isNodejsCdxgenType: mockIsNodejsCdxgenType,
50+
runCdxgen: mockRunCdxgen,
51+
}))
52+
53+
describe('cmd-manifest-cdxgen failure reporting', () => {
54+
const importMeta = { url: 'file:///test/cmd-manifest-cdxgen.mts' }
55+
const context = { parentName: 'socket manifest' }
56+
57+
beforeEach(() => {
58+
vi.clearAllMocks()
59+
mockDetectNodejsCdxgenSources.mockResolvedValue({
60+
hasLockfile: true,
61+
hasNodeModules: true,
62+
})
63+
mockIsNodejsCdxgenType.mockReturnValue(true)
64+
process.exitCode = undefined
65+
})
66+
67+
describe('never fails silently', () => {
68+
// The command arms process.exitCode = 1 before spawning cdxgen. Every
69+
// way out of the spawn must therefore either exit deliberately or print
70+
// something, otherwise the user gets exit code 1 and an empty log.
71+
it('reports an actionable error when cdxgen cannot be started', async () => {
72+
mockRunCdxgen.mockRejectedValue(new Error('spawn cdxgen ENOENT'))
73+
const mockExit = vi
74+
.spyOn(process, 'exit')
75+
.mockImplementation((() => {}) as unknown)
76+
77+
await cmdManifestCdxgen.run(['.'], importMeta, context)
78+
79+
expect(mockLogger.fail).toHaveBeenCalledTimes(1)
80+
const message = String(mockLogger.fail.mock.calls[0]?.[0] ?? '')
81+
expect(message.trim()).not.toBe('')
82+
expect(message).toContain('socket cdxgen could not run cdxgen.')
83+
expect(message).toContain('spawn cdxgen ENOENT')
84+
expect(process.exitCode).toBe(1)
85+
mockExit.mockRestore()
86+
})
87+
88+
it('reports an actionable error when the spawn itself rejects', async () => {
89+
mockRunCdxgen.mockResolvedValue({
90+
spawnPromise: Promise.reject(new Error('cdxgen download failed')),
91+
})
92+
const mockExit = vi
93+
.spyOn(process, 'exit')
94+
.mockImplementation((() => {}) as unknown)
95+
96+
await cmdManifestCdxgen.run(['.'], importMeta, context)
97+
98+
expect(mockLogger.fail).toHaveBeenCalledTimes(1)
99+
const message = String(mockLogger.fail.mock.calls[0]?.[0] ?? '')
100+
expect(message).toContain('cdxgen download failed')
101+
expect(process.exitCode).toBe(1)
102+
mockExit.mockRestore()
103+
})
104+
105+
it('reports an actionable error when cdxgen ends with no exit code and no signal', async () => {
106+
mockRunCdxgen.mockResolvedValue({
107+
spawnPromise: Promise.resolve({ code: undefined, signal: undefined }),
108+
})
109+
const mockExit = vi
110+
.spyOn(process, 'exit')
111+
.mockImplementation((() => {}) as unknown)
112+
113+
await cmdManifestCdxgen.run(['.'], importMeta, context)
114+
115+
expect(mockExit).not.toHaveBeenCalled()
116+
expect(mockLogger.fail).toHaveBeenCalledTimes(1)
117+
const message = String(mockLogger.fail.mock.calls[0]?.[0] ?? '')
118+
expect(message).toContain('without reporting an exit code or a signal')
119+
expect(process.exitCode).toBe(1)
120+
mockExit.mockRestore()
121+
})
122+
123+
// The failure message tells the user to re-run with SOCKET_CLI_DEBUG=1.
124+
// That flag turns on the 'error' debug category, so the underlying detail
125+
// has to be emitted there or the advice sends them nowhere.
126+
it('makes SOCKET_CLI_DEBUG=1 worth running after a failed spawn', async () => {
127+
const cause = new Error('spawn cdxgen ENOENT')
128+
mockRunCdxgen.mockRejectedValue(cause)
129+
const mockExit = vi
130+
.spyOn(process, 'exit')
131+
.mockImplementation((() => {}) as unknown)
132+
133+
await cmdManifestCdxgen.run(['.'], importMeta, context)
134+
135+
const debugCall = mockDebugNs.mock.calls.find(call => call[0] === 'error')
136+
expect(debugCall).toBeDefined()
137+
expect(debugCall).toContain(cause)
138+
mockExit.mockRestore()
139+
})
140+
141+
it('makes SOCKET_CLI_DEBUG=1 worth running after a resultless exit', async () => {
142+
const spawnResult = { code: undefined, signal: undefined }
143+
mockRunCdxgen.mockResolvedValue({
144+
spawnPromise: Promise.resolve(spawnResult),
145+
})
146+
const mockExit = vi
147+
.spyOn(process, 'exit')
148+
.mockImplementation((() => {}) as unknown)
149+
150+
await cmdManifestCdxgen.run(['.'], importMeta, context)
151+
152+
const debugCall = mockDebugNs.mock.calls.find(call => call[0] === 'error')
153+
expect(debugCall).toBeDefined()
154+
expect(debugCall).toContain(spawnResult)
155+
mockExit.mockRestore()
156+
})
157+
158+
it('prints the missing-override message as written, without wrapping it', async () => {
159+
const explained = formatMissingCdxgenLocalPathMessage('/tmp/acme-cdxgen')
160+
mockRunCdxgen.mockRejectedValue(new InputError(explained))
161+
const mockExit = vi
162+
.spyOn(process, 'exit')
163+
.mockImplementation((() => {}) as unknown)
164+
165+
await cmdManifestCdxgen.run(['.'], importMeta, context)
166+
167+
expect(mockLogger.fail).toHaveBeenCalledTimes(1)
168+
const message = String(mockLogger.fail.mock.calls[0]?.[0] ?? '')
169+
expect(message).toBe(explained)
170+
expect(process.exitCode).toBe(1)
171+
mockExit.mockRestore()
172+
})
173+
174+
it('stays quiet on the success path', async () => {
175+
mockRunCdxgen.mockResolvedValue({
176+
spawnPromise: Promise.resolve({ code: 0, signal: undefined }),
177+
})
178+
const mockExit = vi
179+
.spyOn(process, 'exit')
180+
.mockImplementation((() => {}) as unknown)
181+
182+
await cmdManifestCdxgen.run(['.'], importMeta, context)
183+
184+
expect(mockLogger.fail).not.toHaveBeenCalled()
185+
expect(mockExit).toHaveBeenCalledWith(0)
186+
mockExit.mockRestore()
187+
})
188+
})
189+
})

0 commit comments

Comments
 (0)