fix(cdxgen): report why cdxgen failed instead of exiting 1 in silence

jdalton · jdalton · commit cbabc524df46 · 2026-08-03T15:08:58.000-04:00
diff --git a/packages/cli/src/commands/manifest/cmd-manifest-cdxgen.mts b/packages/cli/src/commands/manifest/cmd-manifest-cdxgen.mts
@@ -10,6 +10,7 @@ import { getDefaultLogger } from '@socketsecurity/lib-stable/logger/default'
 import { isPath } from '@socketsecurity/lib-stable/paths/normalize'
 import { pluralize } from '@socketsecurity/lib-stable/words/pluralize'
 
+import { formatCdxgenFailureMessage } from '../../util/dlx/cdxgen-diagnostics.mts'
 import {
   detectNodejsCdxgenSources,
   isNodejsCdxgenType,
@@ -336,15 +337,28 @@ export async function run(
     }
   }
 
+  // Assume failure until cdxgen reports otherwise, so an unexpected early exit
+  // cannot be mistaken for a successful scan.
   process.exitCode = 1
 
-  const { spawnPromise } = await runCdxgen(yargv)
+  let result
+  try {
+    const { spawnPromise } = await runCdxgen(yargv)
+    result = await spawnPromise
+  } catch (e) {
+    // cdxgen runs with stdio: 'inherit', so a failure to start it produces no
+    // child output at all. Say where we looked and what to try next.
+    logger.fail(formatCdxgenFailureMessage(e))
+    return
+  }
 
-  // Wait for the spawn promise to resolve and handle the result.
-  const result = await spawnPromise
   if (result.signal) {
     process.kill(process.pid, result.signal)
   } else if (typeof result.code === 'number') {
     process.exit(result.code)
+  } else {
+    // Neither an exit code nor a signal. Without this branch the command
+    // returns here with the exit code armed above and prints nothing at all.
+    logger.fail(formatCdxgenFailureMessage())
   }
 }
diff --git a/packages/cli/src/util/dlx/cdxgen-diagnostics.mts b/packages/cli/src/util/dlx/cdxgen-diagnostics.mts
@@ -0,0 +1,79 @@
+/**
+ * Diagnostics for the `socket cdxgen` command.
+ *
+ * Cdxgen runs as a child process, so when it cannot start there is nothing on
+ * stdout or stderr to explain why. These helpers turn that into a message that
+ * names where the CLI looked for cdxgen and what to try next.
+ */
+
+import { existsSync } from 'node:fs'
+
+import { errorMessage } from '@socketsecurity/lib-stable/errors/message'
+
+import { resolveCdxgen } from './resolve-binary.mjs'
+
+/**
+ * Describe where the CLI resolved cdxgen from, so an error can say which of
+ * the two sources actually failed.
+ */
+export function describeCdxgenSource(): string {
+  const resolution = resolveCdxgen()
+  if (resolution.type === 'local') {
+    return `the local override SOCKET_CLI_CDXGEN_LOCAL_PATH=${resolution.path}`
+  }
+  if (resolution.type === 'dlx') {
+    const { name, version } = resolution.details
+    return `${name}@${version}, downloaded and cached by Socket's dlx`
+  }
+  return "a binary downloaded and cached by Socket's dlx"
+}
+
+/**
+ * Build the message shown when cdxgen fails to produce a result.
+ *
+ * Pass the underlying error when there is one. Pass nothing for the case where
+ * the child process ended without reporting either an exit code or a signal,
+ * which is the shape that used to exit 1 with no output at all.
+ */
+export function formatCdxgenFailureMessage(
+  cause?: unknown | undefined,
+): string {
+  const detail = cause
+    ? errorMessage(cause)
+    : 'the cdxgen process ended without reporting an exit code or a signal'
+
+  return [
+    'socket cdxgen could not run cdxgen.',
+    `  Where: ${describeCdxgenSource()}`,
+    `  Saw:   ${detail || 'no error message was reported'}`,
+    '  Fix:   Re-run with SOCKET_CLI_DEBUG=1 to see the full error. If cdxgen',
+    '         cannot be downloaded on this machine, install it yourself and',
+    '         point SOCKET_CLI_CDXGEN_LOCAL_PATH at the binary.',
+  ].join('\n')
+}
+
+/**
+ * Build the message shown when SOCKET_CLI_CDXGEN_LOCAL_PATH points at
+ * something that is not there.
+ *
+ * Without this check the override is silently ignored and the failure looks
+ * identical to a download failure, which makes the override appear to have no
+ * effect at all.
+ */
+export function formatMissingCdxgenLocalPathMessage(localPath: string): string {
+  return [
+    'SOCKET_CLI_CDXGEN_LOCAL_PATH points at a file that does not exist.',
+    `  Where: ${localPath}`,
+    '  Saw:   nothing at that path',
+    '  Fix:   Correct the path, or unset SOCKET_CLI_CDXGEN_LOCAL_PATH to let',
+    '         Socket download cdxgen itself. On a CI runner, check that the',
+    '         path exists in the same step that runs socket cdxgen.',
+  ].join('\n')
+}
+
+/**
+ * True when a local cdxgen override is configured but missing from disk.
+ */
+export function isMissingCdxgenLocalPath(localPath: string): boolean {
+  return !existsSync(localPath)
+}
diff --git a/packages/cli/src/util/dlx/spawn-cdxgen.mts b/packages/cli/src/util/dlx/spawn-cdxgen.mts
@@ -13,9 +13,14 @@
 import { detectExecutableType } from '@socketsecurity/lib-stable/dlx/detect'
 import { spawn } from '@socketsecurity/lib-stable/process/spawn/child'
 
+import {
+  formatMissingCdxgenLocalPathMessage,
+  isMissingCdxgenLocalPath,
+} from './cdxgen-diagnostics.mts'
 import { defineAutoDispatch, defineVfsSpawn } from './define-tool-spawn.mts'
 import { spawnDlx } from './spawn.mts'
 import { resolveCdxgen } from './resolve-binary.mjs'
+import { InputError } from '../error/errors.mts'
 import { buildSystemToolEnv } from '../spawn/system-tool.mts'
 import { resolveNodeExecutable } from '../spawn/spawn-node.mts'
 
@@ -37,6 +42,12 @@ export async function spawnCdxgenDlx(
 
   // Use local cdxgen if available.
   if (resolution.type === 'local') {
+    // Check the override before spawning. Otherwise a wrong path surfaces as a
+    // bare ENOENT that never mentions the environment variable, so the
+    // override looks like it was ignored.
+    if (isMissingCdxgenLocalPath(resolution.path)) {
+      throw new InputError(formatMissingCdxgenLocalPathMessage(resolution.path))
+    }
     const detection = detectExecutableType(resolution.path)
     const { env: spawnEnv, ...dlxOptions } = {
       __proto__: null,
diff --git a/packages/cli/test/unit/commands/manifest/cmd-manifest-cdxgen-failure-reporting.test.mts b/packages/cli/test/unit/commands/manifest/cmd-manifest-cdxgen-failure-reporting.test.mts
@@ -0,0 +1,126 @@
+/**
+ * Unit tests for the cdxgen command's failure reporting.
+ *
+ * Purpose: The command arms process.exitCode = 1 before spawning cdxgen, and
+ * cdxgen itself runs with stdio: 'inherit'. Together that means any path out
+ * of the spawn that neither exits deliberately nor prints leaves the user with
+ * exit code 1 and an empty log. These tests cover every one of those paths.
+ *
+ * Related Files: - src/commands/manifest/cmd-manifest-cdxgen.mts.
+ */
+
+import { beforeEach, describe, expect, it, vi } from 'vitest'
+
+import { cmdManifestCdxgen } from '../../../../src/commands/manifest/cmd-manifest-cdxgen.mts'
+
+const mockLogger = vi.hoisted(() => ({
+  error: vi.fn(),
+  fail: vi.fn(),
+  info: vi.fn(),
+  log: vi.fn(),
+  success: vi.fn(),
+  warn: vi.fn(),
+}))
+
+vi.mock(import('@socketsecurity/lib-stable/logger/default'), () => ({
+  getDefaultLogger: () => mockLogger,
+}))
+
+const mockRunCdxgen = vi.hoisted(() => vi.fn())
+const mockDetectNodejsCdxgenSources = vi.hoisted(() =>
+  vi.fn().mockResolvedValue({ hasLockfile: true, hasNodeModules: true }),
+)
+const mockIsNodejsCdxgenType = vi.hoisted(() => vi.fn().mockReturnValue(true))
+
+vi.mock(import('../../../../src/commands/manifest/run-cdxgen.mts'), () => ({
+  detectNodejsCdxgenSources: mockDetectNodejsCdxgenSources,
+  isNodejsCdxgenType: mockIsNodejsCdxgenType,
+  runCdxgen: mockRunCdxgen,
+}))
+
+describe('cmd-manifest-cdxgen failure reporting', () => {
+  const importMeta = { url: 'file:///test/cmd-manifest-cdxgen.mts' }
+  const context = { parentName: 'socket manifest' }
+
+  beforeEach(() => {
+    vi.clearAllMocks()
+    mockDetectNodejsCdxgenSources.mockResolvedValue({
+      hasLockfile: true,
+      hasNodeModules: true,
+    })
+    mockIsNodejsCdxgenType.mockReturnValue(true)
+    process.exitCode = undefined
+  })
+
+  describe('never fails silently', () => {
+    // The command arms process.exitCode = 1 before spawning cdxgen. Every
+    // way out of the spawn must therefore either exit deliberately or print
+    // something, otherwise the user gets exit code 1 and an empty log.
+    it('reports an actionable error when cdxgen cannot be started', async () => {
+      mockRunCdxgen.mockRejectedValue(new Error('spawn cdxgen ENOENT'))
+      const mockExit = vi
+        .spyOn(process, 'exit')
+        .mockImplementation((() => {}) as unknown)
+
+      await cmdManifestCdxgen.run(['.'], importMeta, context)
+
+      expect(mockLogger.fail).toHaveBeenCalledTimes(1)
+      const message = String(mockLogger.fail.mock.calls[0]?.[0] ?? '')
+      expect(message.trim()).not.toBe('')
+      expect(message).toContain('socket cdxgen could not run cdxgen.')
+      expect(message).toContain('spawn cdxgen ENOENT')
+      expect(process.exitCode).toBe(1)
+      mockExit.mockRestore()
+    })
+
+    it('reports an actionable error when the spawn itself rejects', async () => {
+      mockRunCdxgen.mockResolvedValue({
+        spawnPromise: Promise.reject(new Error('cdxgen download failed')),
+      })
+      const mockExit = vi
+        .spyOn(process, 'exit')
+        .mockImplementation((() => {}) as unknown)
+
+      await cmdManifestCdxgen.run(['.'], importMeta, context)
+
+      expect(mockLogger.fail).toHaveBeenCalledTimes(1)
+      const message = String(mockLogger.fail.mock.calls[0]?.[0] ?? '')
+      expect(message).toContain('cdxgen download failed')
+      expect(process.exitCode).toBe(1)
+      mockExit.mockRestore()
+    })
+
+    it('reports an actionable error when cdxgen ends with no exit code and no signal', async () => {
+      mockRunCdxgen.mockResolvedValue({
+        spawnPromise: Promise.resolve({ code: undefined, signal: undefined }),
+      })
+      const mockExit = vi
+        .spyOn(process, 'exit')
+        .mockImplementation((() => {}) as unknown)
+
+      await cmdManifestCdxgen.run(['.'], importMeta, context)
+
+      expect(mockExit).not.toHaveBeenCalled()
+      expect(mockLogger.fail).toHaveBeenCalledTimes(1)
+      const message = String(mockLogger.fail.mock.calls[0]?.[0] ?? '')
+      expect(message).toContain('without reporting an exit code or a signal')
+      expect(process.exitCode).toBe(1)
+      mockExit.mockRestore()
+    })
+
+    it('stays quiet on the success path', async () => {
+      mockRunCdxgen.mockResolvedValue({
+        spawnPromise: Promise.resolve({ code: 0, signal: undefined }),
+      })
+      const mockExit = vi
+        .spyOn(process, 'exit')
+        .mockImplementation((() => {}) as unknown)
+
+      await cmdManifestCdxgen.run(['.'], importMeta, context)
+
+      expect(mockLogger.fail).not.toHaveBeenCalled()
+      expect(mockExit).toHaveBeenCalledWith(0)
+      mockExit.mockRestore()
+    })
+  })
+})
diff --git a/packages/cli/test/unit/util/dlx/cdxgen-diagnostics.test.mts b/packages/cli/test/unit/util/dlx/cdxgen-diagnostics.test.mts
@@ -0,0 +1,123 @@
+/**
+ * Unit tests for the cdxgen failure diagnostics.
+ *
+ * Purpose: cdxgen runs with stdio: 'inherit', so when it cannot start there is
+ * no child output to explain the failure. These tests pin the property that
+ * matters to a user staring at a CI log: the message is non-empty, names where
+ * the CLI looked, and says what to do next.
+ *
+ * Related Files: - src/util/dlx/cdxgen-diagnostics.mts (implementation)
+ */
+
+import { afterEach, beforeEach, describe, expect, it } from 'vitest'
+
+import {
+  describeCdxgenSource,
+  formatCdxgenFailureMessage,
+  formatMissingCdxgenLocalPathMessage,
+  isMissingCdxgenLocalPath,
+} from '../../../../src/util/dlx/cdxgen-diagnostics.mts'
+
+describe('formatCdxgenFailureMessage', () => {
+  it('is never empty, even with no underlying error', () => {
+    const message = formatCdxgenFailureMessage()
+
+    expect(message.trim()).not.toBe('')
+    expect(message.length).toBeGreaterThan(40)
+  })
+
+  it('explains the case where the process reported no exit code and no signal', () => {
+    const message = formatCdxgenFailureMessage()
+
+    expect(message).toContain('without reporting an exit code or a signal')
+  })
+
+  it('quotes the underlying error when there is one', () => {
+    const message = formatCdxgenFailureMessage(new Error('spawn cdxgen ENOENT'))
+
+    expect(message).toContain('spawn cdxgen ENOENT')
+  })
+
+  it('survives a thrown non-Error value', () => {
+    const message = formatCdxgenFailureMessage('just a string')
+
+    expect(message.trim()).not.toBe('')
+    expect(message).toContain('socket cdxgen could not run cdxgen.')
+  })
+
+  it('names where cdxgen was looked for and how to get more detail', () => {
+    const message = formatCdxgenFailureMessage(new Error('boom'))
+
+    expect(message).toContain('Where:')
+    expect(message).toContain('Fix:')
+    expect(message).toContain('SOCKET_CLI_DEBUG=1')
+    expect(message).toContain('SOCKET_CLI_CDXGEN_LOCAL_PATH')
+  })
+})
+
+describe('describeCdxgenSource', () => {
+  const originalPath = process.env['SOCKET_CLI_CDXGEN_LOCAL_PATH']
+
+  afterEach(() => {
+    if (originalPath === undefined) {
+      delete process.env['SOCKET_CLI_CDXGEN_LOCAL_PATH']
+    } else {
+      process.env['SOCKET_CLI_CDXGEN_LOCAL_PATH'] = originalPath
+    }
+  })
+
+  it('names the dlx package when no override is configured', () => {
+    delete process.env['SOCKET_CLI_CDXGEN_LOCAL_PATH']
+
+    expect(describeCdxgenSource()).toContain('@cyclonedx/cdxgen')
+  })
+})
+
+describe('SOCKET_CLI_CDXGEN_LOCAL_PATH validation', () => {
+  it('reports a path that is not on disk as missing', () => {
+    expect(isMissingCdxgenLocalPath('/definitely/not/here/acme-cdxgen')).toBe(
+      true,
+    )
+  })
+
+  it('does not report an existing file as missing', () => {
+    expect(isMissingCdxgenLocalPath(process.execPath)).toBe(false)
+  })
+
+  it('names the variable and the offending path in the message', () => {
+    const message = formatMissingCdxgenLocalPathMessage(
+      '/definitely/not/here/acme-cdxgen',
+    )
+
+    expect(message).toContain('SOCKET_CLI_CDXGEN_LOCAL_PATH')
+    expect(message).toContain('/definitely/not/here/acme-cdxgen')
+    expect(message).toContain('Fix:')
+  })
+})
+
+describe('message shape', () => {
+  let messages: string[] = []
+
+  beforeEach(() => {
+    messages = [
+      formatCdxgenFailureMessage(),
+      formatCdxgenFailureMessage(new Error('boom')),
+      formatMissingCdxgenLocalPathMessage('/tmp/acme-cdxgen'),
+    ]
+  })
+
+  it('emits no ANSI escape codes, so a CI log stays clean', () => {
+    for (let i = 0, { length } = messages; i < length; i += 1) {
+      // oxlint-disable-next-line no-control-regex -- matching escapes is the point.
+      expect(messages[i]!).not.toMatch(/\[[0-9;]*m/)
+    }
+  })
+
+  it('leads with what went wrong before any detail', () => {
+    for (let i = 0, { length } = messages; i < length; i += 1) {
+      const firstLine = messages[i]!.split('\n')[0]!
+      expect(firstLine).not.toMatch(/^\s/)
+      expect(firstLine.length).toBeGreaterThan(20)
+    }
+  })
+})
diff --git a/packages/cli/test/unit/util/dlx/spawn-cdxgen.test.mts b/packages/cli/test/unit/util/dlx/spawn-cdxgen.test.mts
