diff --git a/CHANGELOG.md b/CHANGELOG.md index f9a9ba2f70..1cb099541f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). +## [1.1.150](https://github.com/SocketDev/socket-cli/releases/tag/v1.1.150) - 2026-07-29 + +### Changed +- Updated the Coana CLI to v `15.9.7`. +- `socket fix` vulnerability discovery now reads Coana's structured `--output-file` JSON result instead of parsing stdout, and warns when the Socket backend resolved 0 artifacts so an incomplete server-side resolve is surfaced instead of silently reporting "Finished!". + +### Fixed +- `socket fix` no longer reports success when the Coana vulnerability-discovery step fails — Coana errors and unreadable discovery output now exit non-zero with the underlying reason instead of printing "Finished!" with nothing fixed. + ## [1.1.149](https://github.com/SocketDev/socket-cli/releases/tag/v1.1.149) - 2026-07-29 ### Changed diff --git a/package.json b/package.json index c472677a82..c175ffa944 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "socket", - "version": "1.1.149", + "version": "1.1.150", "description": "CLI for Socket.dev", "homepage": "https://github.com/SocketDev/socket-cli", "license": "MIT", @@ -97,7 +97,7 @@ "@babel/preset-typescript": "7.27.1", "@babel/runtime": "7.28.4", "@biomejs/biome": "2.2.4", - "@coana-tech/cli": "15.9.6", + "@coana-tech/cli": "15.9.7", "@cyclonedx/cdxgen": "12.1.2", "@dotenvx/dotenvx": "1.49.0", "@eslint/compat": "1.3.2", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 245f5ae6ef..a57398c341 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -132,8 +132,8 @@ importers: specifier: 2.2.4 version: 2.2.4 '@coana-tech/cli': - specifier: 15.9.6 - version: 15.9.6 + specifier: 15.9.7 + version: 15.9.7 '@cyclonedx/cdxgen': specifier: 12.1.2 version: 12.1.2 @@ -806,8 +806,8 @@ packages: resolution: {integrity: sha512-hAs5PPKPCQ3/Nha+1fo4A4/gL85fIfxZwHPehsjCJ+BhQH2/yw6/xReuaPA/RfNQr6iz1PcD7BZcE3ctyyl3EA==} cpu: [x64] - '@coana-tech/cli@15.9.6': - resolution: {integrity: sha512-c4bTWc+fgKuyKVc9p2qxly6JNqwzF0L/UrDctMSTupbKeoELkL6nZw4TCdaAUr8Xd28AW46UxucMFOd4QevXUA==} + '@coana-tech/cli@15.9.7': + resolution: {integrity: sha512-1CGis51nRr3Sl3uKYLd7/c4L+vEtYlJUiGYh1TL58WtD4Ojd+qaticSFDa+mywbKLRBXpAPwPWBOes0OQZNP1g==} hasBin: true '@colors/colors@1.5.0': @@ -5509,7 +5509,7 @@ snapshots: '@cdxgen/cdxgen-plugins-bin@2.0.2': optional: true - '@coana-tech/cli@15.9.6': {} + '@coana-tech/cli@15.9.7': {} '@colors/colors@1.5.0': optional: true diff --git a/src/commands/fix/cmd-fix.e2e.test.mts b/src/commands/fix/cmd-fix.e2e.test.mts index bbaea5a8c1..ac4b7f5a89 100644 --- a/src/commands/fix/cmd-fix.e2e.test.mts +++ b/src/commands/fix/cmd-fix.e2e.test.mts @@ -196,9 +196,9 @@ describe('socket fix (E2E tests)', async () => { `\nSuccessfully upgraded lodash from ${beforeVersion} to ${afterVersion}`, ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() @@ -276,9 +276,9 @@ describe('socket fix (E2E tests)', async () => { `\nSuccessfully upgraded lodash from ${beforeVersion} to ${afterVersion} and wrote output file`, ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() @@ -334,9 +334,9 @@ describe('socket fix (E2E tests)', async () => { `\nSuccessfully fixed GHSA-35jh-r3h4-6jhm by upgrading lodash from ${beforeVersion} to ${afterVersion}`, ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() @@ -392,9 +392,9 @@ describe('socket fix (E2E tests)', async () => { `\nSuccessfully converted CVE-2021-23337 to GHSA and fixed by upgrading lodash from ${beforeVersion} to ${afterVersion}`, ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() @@ -458,9 +458,9 @@ describe('socket fix (E2E tests)', async () => { '\nSuccessfully verified --silence --json outputs only JSON', ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() @@ -525,9 +525,9 @@ describe('socket fix (E2E tests)', async () => { `\n--package-managers NPM upgraded npm-app lodash to ${afterLodash} and left pnpm-app axios untouched`, ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() @@ -590,9 +590,9 @@ describe('socket fix (E2E tests)', async () => { `\n--package-managers PNPM upgraded pnpm-app axios to ${afterAxios} and left npm-app lodash untouched`, ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() @@ -692,9 +692,9 @@ describe('socket fix (E2E tests)', async () => { `\nSuccessfully upgraded django from ${beforeVersion} to ${afterVersion}`, ) } catch (e) { - if (code !== 0) { - logCommandOutput(code, stdout, stderr) - } + // Log output on any failure — including exit-0 runs whose later + // assertions fail (e.g. a silent no-op upgrade). + logCommandOutput(code, stdout, stderr) throw e } finally { await tempFixture.cleanup() diff --git a/src/commands/fix/coana-fix.mts b/src/commands/fix/coana-fix.mts index 61ed607313..7d2258782d 100644 --- a/src/commands/fix/coana-fix.mts +++ b/src/commands/fix/coana-fix.mts @@ -64,6 +64,67 @@ type DiscoverGhsaIdsOptions = { spinner?: Spinner | undefined } +type StructuredDiscoveryResult = { + ghsaIds?: unknown + artifactCount?: unknown + filteredArtifactCount?: unknown +} + +async function readStructuredDiscoveryResult( + outputFile: string, + silence: boolean, +): Promise> { + let raw: string + try { + raw = await fs.readFile(outputFile, 'utf8') + } catch { + return { + ok: false, + message: 'Coana did not write a vulnerability discovery result file', + cause: `Expected \`find-vulnerabilities --output-file\` to create ${outputFile}.`, + } + } + + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch { + return { + ok: false, + message: + 'Could not parse the vulnerability discovery result written by Coana', + cause: `Expected JSON in ${outputFile}, got:\n ${raw.slice(0, 500)}`, + } + } + + if (parsed === null || typeof parsed !== 'object') { + return { + ok: false, + message: 'Coana wrote an unexpected vulnerability discovery result', + cause: `Expected a JSON object with a ghsaIds string array, got:\n ${raw.slice(0, 500)}`, + } + } + + const { artifactCount, ghsaIds } = parsed as StructuredDiscoveryResult + if (!Array.isArray(ghsaIds) || ghsaIds.some(id => typeof id !== 'string')) { + return { + ok: false, + message: 'Coana wrote an unexpected vulnerability discovery result', + cause: `Expected a JSON object with a ghsaIds string array, got:\n ${raw.slice(0, 500)}`, + } + } + + // Zero artifacts means the backend resolved nothing — suspicious when + // manifests were uploaded, so warn even though discovery itself succeeded. + if (artifactCount === 0 && !silence) { + logger.warn( + 'The Socket backend resolved 0 artifacts, so vulnerability discovery may be incomplete.', + ) + } + + return { ok: true, data: ghsaIds } +} + /** * Discovers GHSA IDs by running coana without applying fixes. * Returns a list of GHSA IDs, optionally limited. @@ -72,7 +133,7 @@ async function discoverGhsaIds( orgSlug: string, tarHash: string, options?: DiscoverGhsaIdsOptions | undefined, -): Promise { +): Promise> { const { cwd = process.cwd(), ecosystems, @@ -84,12 +145,22 @@ async function discoverGhsaIds( ...options, } as DiscoverGhsaIdsOptions + // Coana writes a structured JSON result ({ ghsaIds, artifactCount, + // filteredArtifactCount }) via --output-file (available since coana 15.9.7, + // older than the pinned @coana-tech/cli version). + const outputFile = path.join( + os.tmpdir(), + `socket-fix-discovery-${Date.now()}.json`, + ) + const foundCResult = await spawnCoanaDlx( [ 'find-vulnerabilities', cwd, '--manifests-tar-hash', tarHash, + '--output-file', + outputFile, ...(ecosystems?.length ? ['--purl-types', ...ecosystems] : []), ...(packageManagers?.length ? ['--package-managers', ...packageManagers] @@ -104,16 +175,16 @@ async function discoverGhsaIds( { stdio: 'pipe' }, ) - if (foundCResult.ok) { - try { - // Coana prints ghsaIds as json-formatted string on the final line of the output. - const ghsaIdsRaw = foundCResult.data.trim().split('\n').pop() - if (ghsaIdsRaw) { - return JSON.parse(ghsaIdsRaw) - } - } catch {} + if (!foundCResult.ok) { + await fs.rm(outputFile, { force: true }).catch(() => {}) + return foundCResult + } + + try { + return await readStructuredDiscoveryResult(outputFile, silence) + } finally { + await fs.rm(outputFile, { force: true }).catch(() => {}) } - return [] } export async function coanaFix( @@ -267,16 +338,26 @@ export async function coanaFix( } // In local mode, process all discovered/provided IDs (no limit). - const ids: string[] = shouldDiscoverGhsaIds - ? await discoverGhsaIds(orgSlug, tarHash, { - coanaVersion, - cwd, - ecosystems, - packageManagers, - silence, - spinner, - }) - : ghsas + let ids: string[] + if (shouldDiscoverGhsaIds) { + const discoverCResult = await discoverGhsaIds(orgSlug, tarHash, { + coanaVersion, + cwd, + ecosystems, + packageManagers, + silence, + spinner, + }) + if (!discoverCResult.ok) { + if (!silence) { + spinner?.stop() + } + return discoverCResult + } + ids = discoverCResult.data + } else { + ids = ghsas + } if (ids.length === 0) { if (!silence) { @@ -403,18 +484,25 @@ export async function coanaFix( let ids: string[] | undefined if (shouldSpawnCoana) { - ids = ( - shouldDiscoverGhsaIds - ? await discoverGhsaIds(orgSlug, tarHash, { - coanaVersion, - cwd, - ecosystems, - packageManagers, - silence, - spinner, - }) - : ghsas - ).slice(0, adjustedPrLimit) + if (shouldDiscoverGhsaIds) { + const discoverCResult = await discoverGhsaIds(orgSlug, tarHash, { + coanaVersion, + cwd, + ecosystems, + packageManagers, + silence, + spinner, + }) + if (!discoverCResult.ok) { + if (!silence) { + spinner?.stop() + } + return discoverCResult + } + ids = discoverCResult.data.slice(0, adjustedPrLimit) + } else { + ids = ghsas.slice(0, adjustedPrLimit) + } } if (!ids?.length) { diff --git a/src/commands/fix/handle-fix-limit.test.mts b/src/commands/fix/handle-fix-limit.test.mts index 8676cf3010..e6ad6af39a 100644 --- a/src/commands/fix/handle-fix-limit.test.mts +++ b/src/commands/fix/handle-fix-limit.test.mts @@ -1,5 +1,9 @@ +import { promises as fs } from 'node:fs' + import { beforeEach, describe, expect, it, vi } from 'vitest' +import { logger } from '@socketsecurity/registry/lib/logger' + import { coanaFix } from './coana-fix.mts' import type { FixConfig } from './types.mts' @@ -70,6 +74,32 @@ vi.mock('./branch-cleanup.mts', () => ({ cleanupSuccessfulPrLocalBranch: vi.fn(), })) +// Discovery always uses `find-vulnerabilities --output-file`: mock Coana by +// writing the structured result file on the discovery call and succeeding on +// the fix call. +function mockDiscoveryEnvelope(envelope: { + ghsaIds: string[] + artifactCount?: number + filteredArtifactCount?: number +}) { + mockSpawnCoanaDlx.mockImplementation(async (args: string[]) => { + if (args[0] === 'find-vulnerabilities') { + const idx = args.indexOf('--output-file') + expect(idx).toBeGreaterThan(-1) + await fs.writeFile( + args[idx + 1]!, + JSON.stringify({ + artifactCount: envelope.ghsaIds.length, + filteredArtifactCount: envelope.ghsaIds.length, + ...envelope, + }), + ) + return { ok: true, data: '' } + } + return { ok: true, data: 'fix applied' } + }) +} + describe('socket fix --pr-limit behavior verification', () => { const baseConfig: FixConfig = { all: false, @@ -207,11 +237,7 @@ describe('socket fix --pr-limit behavior verification', () => { }) it('should return early when no GHSAs are provided and none are discovered', async () => { - // Discovery returns empty array. - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: JSON.stringify([]), - }) + mockDiscoveryEnvelope({ ghsaIds: [] }) const result = await coanaFix({ ...baseConfig, @@ -226,16 +252,8 @@ describe('socket fix --pr-limit behavior verification', () => { }) it('should discover vulnerabilities when no GHSAs are provided', async () => { - // First call is for discovery (returns vulnerability IDs). - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: JSON.stringify(['GHSA-aaaa-aaaa-aaaa', 'GHSA-bbbb-bbbb-bbbb']), - }) - - // Second call is to apply fixes to the discovered IDs. - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: 'fix applied', + mockDiscoveryEnvelope({ + ghsaIds: ['GHSA-aaaa-aaaa-aaaa', 'GHSA-bbbb-bbbb-bbbb'], }) const result = await coanaFix({ @@ -280,23 +298,13 @@ describe('socket fix --pr-limit behavior verification', () => { }) it('should process only N GHSAs when --pr-limit N is specified in PR mode', async () => { - const ghsas = [ - 'GHSA-aaaa-aaaa-aaaa', - 'GHSA-bbbb-bbbb-bbbb', - 'GHSA-cccc-cccc-cccc', - 'GHSA-dddd-dddd-dddd', - ] - - // First call discovers vulnerabilities. - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: JSON.stringify(ghsas), - }) - - // Subsequent calls are for individual GHSA fixes. - mockSpawnCoanaDlx.mockResolvedValue({ - ok: true, - data: 'fix applied', + mockDiscoveryEnvelope({ + ghsaIds: [ + 'GHSA-aaaa-aaaa-aaaa', + 'GHSA-bbbb-bbbb-bbbb', + 'GHSA-cccc-cccc-cccc', + 'GHSA-dddd-dddd-dddd', + ], }) mockGitUnstagedModifiedFiles.mockResolvedValue({ @@ -317,12 +325,6 @@ describe('socket fix --pr-limit behavior verification', () => { }) it('should adjust prLimit based on existing open PRs', async () => { - const ghsas = [ - 'GHSA-aaaa-aaaa-aaaa', - 'GHSA-bbbb-bbbb-bbbb', - 'GHSA-cccc-cccc-cccc', - ] - // Mock 1 existing open PR. mockGetSocketFixPrs.mockResolvedValueOnce([ { number: 123, state: 'OPEN' }, @@ -331,14 +333,12 @@ describe('socket fix --pr-limit behavior verification', () => { // Second call returns no open PRs for specific GHSAs. mockGetSocketFixPrs.mockResolvedValue([]) - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: JSON.stringify(ghsas), - }) - - mockSpawnCoanaDlx.mockResolvedValue({ - ok: true, - data: 'fix applied', + mockDiscoveryEnvelope({ + ghsaIds: [ + 'GHSA-aaaa-aaaa-aaaa', + 'GHSA-bbbb-bbbb-bbbb', + 'GHSA-cccc-cccc-cccc', + ], }) mockGitUnstagedModifiedFiles.mockResolvedValue({ @@ -383,6 +383,185 @@ describe('socket fix --pr-limit behavior verification', () => { }) }) + describe('GHSA discovery failure propagation', () => { + it('propagates a failed discovery spawn instead of reporting success', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: false, + message: 'Coana exited with code 1', + cause: + 'Socket compute-artifacts failed: upstream timeout (code=timeout)', + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toBe('Coana exited with code 1') + // Discovery failed, so no fix call should follow. + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + }) + + it('propagates discovery failures in PR mode', async () => { + mockGetFixEnv.mockResolvedValue({ + baseBranch: 'main', + githubToken: 'test-token', + gitEmail: 'test@example.com', + gitUser: 'test-user', + isCi: true, + repoInfo: { + defaultBranch: 'main', + owner: 'test-owner', + repo: 'test-repo', + }, + }) + mockGetSocketFixPrs.mockResolvedValue([]) + + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: false, + message: 'Coana exited with code 1', + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + prLimit: 2, + }) + + expect(result.ok).toBe(false) + expect(result.message).toBe('Coana exited with code 1') + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + }) + }) + + describe('structured discovery result (--output-file)', () => { + it('reads discovered GHSA IDs from the result file', async () => { + mockDiscoveryEnvelope({ + ghsaIds: ['GHSA-aaaa-aaaa-aaaa'], + artifactCount: 3, + filteredArtifactCount: 3, + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(true) + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(2) + }) + + it('fails when Coana does not write the result file', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ ok: true, data: '' }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/did not write/i) + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + }) + + it('fails when the result file is not valid JSON', async () => { + mockSpawnCoanaDlx.mockImplementation(async (args: string[]) => { + const idx = args.indexOf('--output-file') + await fs.writeFile(args[idx + 1]!, 'not json') + return { ok: true, data: '' } + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/could not parse/i) + }) + + it('fails when ghsaIds in the result file is not a string array', async () => { + mockSpawnCoanaDlx.mockImplementation(async (args: string[]) => { + const idx = args.indexOf('--output-file') + await fs.writeFile(args[idx + 1]!, JSON.stringify({ ghsaIds: [123] })) + return { ok: true, data: '' } + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/unexpected vulnerability discovery/i) + }) + + it.each(['null', '[1, 2]', '42', '"text"'])( + 'fails cleanly when the result file is %s instead of an object', + async body => { + mockSpawnCoanaDlx.mockImplementation(async (args: string[]) => { + const idx = args.indexOf('--output-file') + await fs.writeFile(args[idx + 1]!, body) + return { ok: true, data: '' } + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/unexpected vulnerability discovery/i) + }, + ) + + it('removes the discovery temp file when the spawn fails', async () => { + let outputFile = '' + mockSpawnCoanaDlx.mockImplementation(async (args: string[]) => { + const idx = args.indexOf('--output-file') + outputFile = args[idx + 1]! + await fs.writeFile(outputFile, JSON.stringify({ ghsaIds: [] })) + return { ok: false, message: 'Coana exited with code 1' } + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(outputFile).not.toBe('') + await expect(fs.stat(outputFile)).rejects.toThrow() + }) + + it('warns when the backend resolved zero artifacts', async () => { + const warnSpy = vi.spyOn(logger, 'warn') + mockDiscoveryEnvelope({ + ghsaIds: [], + artifactCount: 0, + filteredArtifactCount: 0, + }) + + try { + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(true) + expect(result.data?.fixedAll).toBe(false) + // Discovery succeeded with an empty list, so no fix call follows. + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + expect(warnSpy).toHaveBeenCalledWith( + expect.stringMatching(/0 artifacts/i), + ) + } finally { + warnSpy.mockRestore() + } + }) + }) + describe('--id filtering in local mode', () => { it('should process all provided GHSA IDs in local mode (prLimit ignored)', async () => { const ghsas = [