From 06d829e5a51cad12ef78dea64504196e74a0ce6b Mon Sep 17 00:00:00 2001 From: Martin Torp Date: Wed, 29 Jul 2026 09:52:40 +0200 Subject: [PATCH 1/5] fix(fix): propagate Coana discovery failures instead of reporting success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit socket fix silently exited 0 with "Finished!" whenever the GHSA discovery step failed, because discoverGhsaIds collapsed every failure mode — Coana exiting non-zero, the dlx launcher failing to fetch @coana-tech/cli, empty stdout, or an unparseable final line — to an empty GHSA list, indistinguishable from "nothing to fix". discoverGhsaIds now returns CResult: a failed Coana spawn propagates its error (exit code and stderr intact), and empty, non-JSON, or wrongly-shaped output fails with a specific message. A genuine empty array still exits 0. Both call sites (local and CI/PR mode) propagate the failure so outputFixResult sets a non-zero exit code and the real reason reaches the user. --- src/commands/fix/coana-fix.mts | 108 ++++++++++++----- src/commands/fix/handle-fix-limit.test.mts | 134 +++++++++++++++++++++ 2 files changed, 210 insertions(+), 32 deletions(-) diff --git a/src/commands/fix/coana-fix.mts b/src/commands/fix/coana-fix.mts index 61ed607313..ea38d14c00 100644 --- a/src/commands/fix/coana-fix.mts +++ b/src/commands/fix/coana-fix.mts @@ -72,7 +72,7 @@ async function discoverGhsaIds( orgSlug: string, tarHash: string, options?: DiscoverGhsaIdsOptions | undefined, -): Promise { +): Promise> { const { cwd = process.cwd(), ecosystems, @@ -104,16 +104,43 @@ 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) { + return foundCResult + } + + // Coana prints ghsaIds as json-formatted string on the final line of the output. + const lastLine = foundCResult.data.trim().split('\n').pop() + if (!lastLine) { + return { + ok: false, + message: 'Coana returned no output while discovering vulnerabilities', + cause: + 'Expected a JSON array of GHSA IDs on the final line of `find-vulnerabilities` output.', + } + } + + let parsed: unknown + try { + parsed = JSON.parse(lastLine) + } catch { + return { + ok: false, + message: 'Could not parse the vulnerability list returned by Coana', + cause: + 'Expected a JSON array of GHSA IDs on the final line of `find-vulnerabilities` output, got:\n' + + ` ${lastLine}`, + } + } + + if (!Array.isArray(parsed) || parsed.some(id => typeof id !== 'string')) { + return { + ok: false, + message: 'Coana returned an unexpected vulnerability list', + cause: `Expected a JSON array of GHSA ID strings, got:\n ${lastLine}`, + } } - return [] + + return { ok: true, data: parsed } } export async function coanaFix( @@ -267,16 +294,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 +440,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..87de391651 100644 --- a/src/commands/fix/handle-fix-limit.test.mts +++ b/src/commands/fix/handle-fix-limit.test.mts @@ -383,6 +383,140 @@ 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('fails when discovery returns no output', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: '', + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/no output/i) + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + }) + + it('fails when the final discovery output line is not JSON', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: '["GHSA-aaaa-aaaa-aaaa"]\nnpm notice: a new version is available', + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/could not parse/i) + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + }) + + it('fails when discovery output parses to a non-array', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: '{}', + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/unexpected vulnerability list/i) + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + }) + + it('fails when discovery output is an array of non-strings', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: '[123]', + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/unexpected vulnerability list/i) + }) + + it('still succeeds when warnings precede the JSON array on the final line', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: 'some warning\n["GHSA-aaaa-aaaa-aaaa"]', + }) + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: 'fix applied', + }) + + const result = await coanaFix({ + ...baseConfig, + ghsas: [], + }) + + expect(result.ok).toBe(true) + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(2) + }) + + 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('--id filtering in local mode', () => { it('should process all provided GHSA IDs in local mode (prLimit ignored)', async () => { const ghsas = [ From e1a4cd43a95810d0f385a094339a84ddfbef7de6 Mon Sep 17 00:00:00 2001 From: Martin Torp Date: Wed, 29 Jul 2026 11:20:43 +0200 Subject: [PATCH 2/5] feat(fix): consume Coana's structured discovery result when available Coana 15.9.7 adds `find-vulnerabilities --output-file`, writing a structured JSON result ({ ghsaIds, artifactCount, filteredArtifactCount }) that replaces the brittle "JSON array on the final stdout line" contract (coana-tech/coana-package-manager#2327). discoverGhsaIds now prefers that file when the resolved Coana version supports it (or when using a local Coana build), falling back to strict stdout parsing for older versions. A missing, unparseable, or wrongly shaped result file fails the run with a specific message instead of silently skipping fixes. The artifact counts also close the remaining diagnostic hole: when the backend resolved 0 artifacts, an empty result is almost certainly a server-side resolve problem rather than "nothing to fix", so the CLI now warns instead of only printing "Finished!". --- src/commands/fix/coana-fix.mts | 82 ++++++++++++ src/commands/fix/handle-fix-limit.test.mts | 137 +++++++++++++++++++++ 2 files changed, 219 insertions(+) diff --git a/src/commands/fix/coana-fix.mts b/src/commands/fix/coana-fix.mts index ea38d14c00..cc3c8a9473 100644 --- a/src/commands/fix/coana-fix.mts +++ b/src/commands/fix/coana-fix.mts @@ -2,6 +2,8 @@ import { promises as fs } from 'node:fs' import os from 'node:os' import path from 'node:path' +import semver from 'semver' + import { joinAnd } from '@socketsecurity/registry/lib/arrays' import { debugDir, debugFn } from '@socketsecurity/registry/lib/debug' import { readJsonSync } from '@socketsecurity/registry/lib/fs' @@ -64,6 +66,64 @@ type DiscoverGhsaIdsOptions = { spinner?: Spinner | undefined } +// First Coana version with `find-vulnerabilities --output-file`, which writes +// a structured JSON result ({ ghsaIds, artifactCount, filteredArtifactCount }) +// instead of relying on the final stdout line. +const COANA_STRUCTURED_DISCOVERY_MIN_VERSION = '15.9.7' + +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)}`, + } + } + + 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. @@ -84,12 +144,26 @@ async function discoverGhsaIds( ...options, } as DiscoverGhsaIdsOptions + const resolvedCoanaVersion = + options?.coanaVersion ?? + process.env['INLINED_SOCKET_CLI_COANA_TECH_CLI_VERSION'] + // Local Coana builds (SOCKET_CLI_COANA_LOCAL_PATH) are assumed current. + const useStructuredOutput = + !!process.env['SOCKET_CLI_COANA_LOCAL_PATH'] || + (!!resolvedCoanaVersion && + !!semver.valid(resolvedCoanaVersion) && + semver.gte(resolvedCoanaVersion, COANA_STRUCTURED_DISCOVERY_MIN_VERSION)) + const outputFile = useStructuredOutput + ? path.join(os.tmpdir(), `socket-fix-discovery-${Date.now()}.json`) + : undefined + const foundCResult = await spawnCoanaDlx( [ 'find-vulnerabilities', cwd, '--manifests-tar-hash', tarHash, + ...(outputFile ? ['--output-file', outputFile] : []), ...(ecosystems?.length ? ['--purl-types', ...ecosystems] : []), ...(packageManagers?.length ? ['--package-managers', ...packageManagers] @@ -108,6 +182,14 @@ async function discoverGhsaIds( return foundCResult } + if (outputFile) { + try { + return await readStructuredDiscoveryResult(outputFile, silence) + } finally { + await fs.rm(outputFile, { force: true }).catch(() => {}) + } + } + // Coana prints ghsaIds as json-formatted string on the final line of the output. const lastLine = foundCResult.data.trim().split('\n').pop() if (!lastLine) { diff --git a/src/commands/fix/handle-fix-limit.test.mts b/src/commands/fix/handle-fix-limit.test.mts index 87de391651..234902a716 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' @@ -517,6 +521,139 @@ describe('socket fix --pr-limit behavior verification', () => { }) }) + describe('structured discovery output (--output-file)', () => { + it('passes --output-file and reads the result file when Coana supports it', async () => { + 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({ + ghsaIds: ['GHSA-aaaa-aaaa-aaaa'], + artifactCount: 3, + filteredArtifactCount: 3, + }), + ) + return { ok: true, data: '' } + } + return { ok: true, data: 'fix applied' } + }) + + const result = await coanaFix({ + ...baseConfig, + coanaVersion: '15.9.7', + ghsas: [], + }) + + expect(result.ok).toBe(true) + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(2) + }) + + it('does not pass --output-file to older Coana versions', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: JSON.stringify(['GHSA-aaaa-aaaa-aaaa']), + }) + mockSpawnCoanaDlx.mockResolvedValueOnce({ + ok: true, + data: 'fix applied', + }) + + const result = await coanaFix({ + ...baseConfig, + coanaVersion: '15.9.6', + ghsas: [], + }) + + expect(result.ok).toBe(true) + const discoveryArgs = mockSpawnCoanaDlx.mock.calls[0]?.[0] as string[] + expect(discoveryArgs).not.toContain('--output-file') + }) + + it('fails when Coana does not write the result file', async () => { + mockSpawnCoanaDlx.mockResolvedValueOnce({ ok: true, data: '' }) + + const result = await coanaFix({ + ...baseConfig, + coanaVersion: '15.9.7', + 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, + coanaVersion: '15.9.7', + 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, + coanaVersion: '15.9.7', + ghsas: [], + }) + + expect(result.ok).toBe(false) + expect(result.message).toMatch(/unexpected vulnerability discovery/i) + }) + + it('warns when the backend resolved zero artifacts', async () => { + const warnSpy = vi.spyOn(logger, 'warn') + mockSpawnCoanaDlx.mockImplementation(async (args: string[]) => { + const idx = args.indexOf('--output-file') + await fs.writeFile( + args[idx + 1]!, + JSON.stringify({ + ghsaIds: [], + artifactCount: 0, + filteredArtifactCount: 0, + }), + ) + return { ok: true, data: '' } + }) + + try { + const result = await coanaFix({ + ...baseConfig, + coanaVersion: '15.9.7', + 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 = [ From 645d428370ddea4bdf22d5d942a1b2aa5add4c23 Mon Sep 17 00:00:00 2001 From: Martin Torp Date: Wed, 29 Jul 2026 14:02:28 +0200 Subject: [PATCH 3/5] feat(fix): always use Coana's structured discovery result (coana 15.9.7) The pinned @coana-tech/cli is now 15.9.7, which always supports `find-vulnerabilities --output-file`, so the Coana version is known at build time and no backward compatibility is needed: drop the version gate and the stdout final-line parsing fallback. Discovery now always passes --output-file and reads the structured result; a missing, unparseable, or wrongly shaped result file fails the run with a specific message. --- CHANGELOG.md | 9 + package.json | 4 +- pnpm-lock.yaml | 10 +- src/commands/fix/coana-fix.mts | 71 ++----- src/commands/fix/handle-fix-limit.test.mts | 234 +++++---------------- 5 files changed, 80 insertions(+), 248 deletions(-) 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/coana-fix.mts b/src/commands/fix/coana-fix.mts index cc3c8a9473..fedec9f075 100644 --- a/src/commands/fix/coana-fix.mts +++ b/src/commands/fix/coana-fix.mts @@ -2,8 +2,6 @@ import { promises as fs } from 'node:fs' import os from 'node:os' import path from 'node:path' -import semver from 'semver' - import { joinAnd } from '@socketsecurity/registry/lib/arrays' import { debugDir, debugFn } from '@socketsecurity/registry/lib/debug' import { readJsonSync } from '@socketsecurity/registry/lib/fs' @@ -66,11 +64,6 @@ type DiscoverGhsaIdsOptions = { spinner?: Spinner | undefined } -// First Coana version with `find-vulnerabilities --output-file`, which writes -// a structured JSON result ({ ghsaIds, artifactCount, filteredArtifactCount }) -// instead of relying on the final stdout line. -const COANA_STRUCTURED_DISCOVERY_MIN_VERSION = '15.9.7' - type StructuredDiscoveryResult = { ghsaIds?: unknown artifactCount?: unknown @@ -144,18 +137,13 @@ async function discoverGhsaIds( ...options, } as DiscoverGhsaIdsOptions - const resolvedCoanaVersion = - options?.coanaVersion ?? - process.env['INLINED_SOCKET_CLI_COANA_TECH_CLI_VERSION'] - // Local Coana builds (SOCKET_CLI_COANA_LOCAL_PATH) are assumed current. - const useStructuredOutput = - !!process.env['SOCKET_CLI_COANA_LOCAL_PATH'] || - (!!resolvedCoanaVersion && - !!semver.valid(resolvedCoanaVersion) && - semver.gte(resolvedCoanaVersion, COANA_STRUCTURED_DISCOVERY_MIN_VERSION)) - const outputFile = useStructuredOutput - ? path.join(os.tmpdir(), `socket-fix-discovery-${Date.now()}.json`) - : undefined + // 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( [ @@ -163,7 +151,8 @@ async function discoverGhsaIds( cwd, '--manifests-tar-hash', tarHash, - ...(outputFile ? ['--output-file', outputFile] : []), + '--output-file', + outputFile, ...(ecosystems?.length ? ['--purl-types', ...ecosystems] : []), ...(packageManagers?.length ? ['--package-managers', ...packageManagers] @@ -182,47 +171,11 @@ async function discoverGhsaIds( return foundCResult } - if (outputFile) { - try { - return await readStructuredDiscoveryResult(outputFile, silence) - } finally { - await fs.rm(outputFile, { force: true }).catch(() => {}) - } - } - - // Coana prints ghsaIds as json-formatted string on the final line of the output. - const lastLine = foundCResult.data.trim().split('\n').pop() - if (!lastLine) { - return { - ok: false, - message: 'Coana returned no output while discovering vulnerabilities', - cause: - 'Expected a JSON array of GHSA IDs on the final line of `find-vulnerabilities` output.', - } - } - - let parsed: unknown try { - parsed = JSON.parse(lastLine) - } catch { - return { - ok: false, - message: 'Could not parse the vulnerability list returned by Coana', - cause: - 'Expected a JSON array of GHSA IDs on the final line of `find-vulnerabilities` output, got:\n' + - ` ${lastLine}`, - } + return await readStructuredDiscoveryResult(outputFile, silence) + } finally { + await fs.rm(outputFile, { force: true }).catch(() => {}) } - - if (!Array.isArray(parsed) || parsed.some(id => typeof id !== 'string')) { - return { - ok: false, - message: 'Coana returned an unexpected vulnerability list', - cause: `Expected a JSON array of GHSA ID strings, got:\n ${lastLine}`, - } - } - - return { ok: true, data: parsed } } export async function coanaFix( diff --git a/src/commands/fix/handle-fix-limit.test.mts b/src/commands/fix/handle-fix-limit.test.mts index 234902a716..2a2f118d30 100644 --- a/src/commands/fix/handle-fix-limit.test.mts +++ b/src/commands/fix/handle-fix-limit.test.mts @@ -74,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, @@ -211,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, @@ -230,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({ @@ -284,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({ @@ -321,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' }, @@ -335,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({ @@ -407,88 +403,6 @@ describe('socket fix --pr-limit behavior verification', () => { expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) }) - it('fails when discovery returns no output', async () => { - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: '', - }) - - const result = await coanaFix({ - ...baseConfig, - ghsas: [], - }) - - expect(result.ok).toBe(false) - expect(result.message).toMatch(/no output/i) - expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) - }) - - it('fails when the final discovery output line is not JSON', async () => { - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: '["GHSA-aaaa-aaaa-aaaa"]\nnpm notice: a new version is available', - }) - - const result = await coanaFix({ - ...baseConfig, - ghsas: [], - }) - - expect(result.ok).toBe(false) - expect(result.message).toMatch(/could not parse/i) - expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) - }) - - it('fails when discovery output parses to a non-array', async () => { - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: '{}', - }) - - const result = await coanaFix({ - ...baseConfig, - ghsas: [], - }) - - expect(result.ok).toBe(false) - expect(result.message).toMatch(/unexpected vulnerability list/i) - expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) - }) - - it('fails when discovery output is an array of non-strings', async () => { - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: '[123]', - }) - - const result = await coanaFix({ - ...baseConfig, - ghsas: [], - }) - - expect(result.ok).toBe(false) - expect(result.message).toMatch(/unexpected vulnerability list/i) - }) - - it('still succeeds when warnings precede the JSON array on the final line', async () => { - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: 'some warning\n["GHSA-aaaa-aaaa-aaaa"]', - }) - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: 'fix applied', - }) - - const result = await coanaFix({ - ...baseConfig, - ghsas: [], - }) - - expect(result.ok).toBe(true) - expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(2) - }) - it('propagates discovery failures in PR mode', async () => { mockGetFixEnv.mockResolvedValue({ baseBranch: 'main', @@ -521,28 +435,16 @@ describe('socket fix --pr-limit behavior verification', () => { }) }) - describe('structured discovery output (--output-file)', () => { - it('passes --output-file and reads the result file when Coana supports it', async () => { - 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({ - ghsaIds: ['GHSA-aaaa-aaaa-aaaa'], - artifactCount: 3, - filteredArtifactCount: 3, - }), - ) - return { ok: true, data: '' } - } - return { ok: true, data: 'fix applied' } + 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, - coanaVersion: '15.9.7', ghsas: [], }) @@ -550,33 +452,11 @@ describe('socket fix --pr-limit behavior verification', () => { expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(2) }) - it('does not pass --output-file to older Coana versions', async () => { - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: JSON.stringify(['GHSA-aaaa-aaaa-aaaa']), - }) - mockSpawnCoanaDlx.mockResolvedValueOnce({ - ok: true, - data: 'fix applied', - }) - - const result = await coanaFix({ - ...baseConfig, - coanaVersion: '15.9.6', - ghsas: [], - }) - - expect(result.ok).toBe(true) - const discoveryArgs = mockSpawnCoanaDlx.mock.calls[0]?.[0] as string[] - expect(discoveryArgs).not.toContain('--output-file') - }) - it('fails when Coana does not write the result file', async () => { mockSpawnCoanaDlx.mockResolvedValueOnce({ ok: true, data: '' }) const result = await coanaFix({ ...baseConfig, - coanaVersion: '15.9.7', ghsas: [], }) @@ -594,7 +474,6 @@ describe('socket fix --pr-limit behavior verification', () => { const result = await coanaFix({ ...baseConfig, - coanaVersion: '15.9.7', ghsas: [], }) @@ -611,7 +490,6 @@ describe('socket fix --pr-limit behavior verification', () => { const result = await coanaFix({ ...baseConfig, - coanaVersion: '15.9.7', ghsas: [], }) @@ -621,23 +499,15 @@ describe('socket fix --pr-limit behavior verification', () => { it('warns when the backend resolved zero artifacts', async () => { const warnSpy = vi.spyOn(logger, 'warn') - mockSpawnCoanaDlx.mockImplementation(async (args: string[]) => { - const idx = args.indexOf('--output-file') - await fs.writeFile( - args[idx + 1]!, - JSON.stringify({ - ghsaIds: [], - artifactCount: 0, - filteredArtifactCount: 0, - }), - ) - return { ok: true, data: '' } + mockDiscoveryEnvelope({ + ghsaIds: [], + artifactCount: 0, + filteredArtifactCount: 0, }) try { const result = await coanaFix({ ...baseConfig, - coanaVersion: '15.9.7', ghsas: [], }) From fe7b74b77893c4190d170dbce75334f107bf34be Mon Sep 17 00:00:00 2001 From: Martin Torp Date: Wed, 29 Jul 2026 15:09:36 +0200 Subject: [PATCH 4/5] fix(fix): plug discovery result edge cases from review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Guard readStructuredDiscoveryResult against non-object JSON bodies (null, arrays, scalars): destructuring null threw a TypeError instead of the intended ok:false result. Also delete the --output-file temp path when the Coana spawn fails — cleanup previously only ran on the success path. --- src/commands/fix/coana-fix.mts | 9 +++++ src/commands/fix/handle-fix-limit.test.mts | 38 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/src/commands/fix/coana-fix.mts b/src/commands/fix/coana-fix.mts index fedec9f075..7d2258782d 100644 --- a/src/commands/fix/coana-fix.mts +++ b/src/commands/fix/coana-fix.mts @@ -97,6 +97,14 @@ async function readStructuredDiscoveryResult( } } + 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 { @@ -168,6 +176,7 @@ async function discoverGhsaIds( ) if (!foundCResult.ok) { + await fs.rm(outputFile, { force: true }).catch(() => {}) return foundCResult } diff --git a/src/commands/fix/handle-fix-limit.test.mts b/src/commands/fix/handle-fix-limit.test.mts index 2a2f118d30..e6ad6af39a 100644 --- a/src/commands/fix/handle-fix-limit.test.mts +++ b/src/commands/fix/handle-fix-limit.test.mts @@ -497,6 +497,44 @@ describe('socket fix --pr-limit behavior verification', () => { 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({ From f2dd7bfe2c53f61cc7cb5efd5df76f751b1a6c92 Mon Sep 17 00:00:00 2001 From: Martin Torp Date: Wed, 29 Jul 2026 15:17:27 +0200 Subject: [PATCH 5/5] test(fix): log CLI output on any e2e failure, not just non-zero exits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A silent no-op exits 0, so the `if (code !== 0)` guard hid all CLI output for exactly the failure class that is hardest to diagnose (e.g. discovery returning an empty id list). The catch block now logs unconditionally, so the next occurrence shows what Coana and the backend actually reported — including the zero-artifact warning. --- src/commands/fix/cmd-fix.e2e.test.mts | 48 +++++++++++++-------------- 1 file changed, 24 insertions(+), 24 deletions(-) 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()