From c831cbfc848d18bd9c4c95cf38e2e82d0f2b0ac0 Mon Sep 17 00:00:00 2001 From: barslev Date: Sun, 31 May 2026 13:41:48 +0200 Subject: [PATCH] fix(scan): resolve .socket.facts.json against the scan cwd so tier1 finalize isn't silently skipped performReachabilityAnalysis spawns Coana with the scan cwd, so Coana writes /.socket.facts.json, but the tier1 scan id was read back via a relative path resolved against process.cwd(). When the scan cwd differed from process.cwd() (e.g. --cwd ) the read missed, the tier1 id came back undefined, and the finalize guard fell through with nothing logged, leaving the tier1 reachability row unlinked. extractReachabilityErrors (used by scan reach) shared the same bug. Resolve the facts path against the scan cwd at the read sites (extractTier1ReachabilityScanId and extractReachabilityErrors); the returned/uploaded report path stays cwd-relative so upload and cleanup behavior is unchanged. Warn when reachability ran and a scan was created but no tier1 id could be extracted, so the skip is never silent. Adds regression coverage for the cwd-mismatch case, the reachability-errors read, and the warning path. --- src/commands/scan/handle-create-new-scan.mts | 13 ++ .../scan/handle-create-new-scan.test.mts | 67 ++++++- src/commands/scan/handle-scan-reach.mts | 3 +- src/commands/scan/output-scan-reach.mts | 16 +- src/commands/scan/output-scan-reach.test.mts | 95 ++++++++++ .../scan/perform-reachability-analysis.mts | 14 +- .../perform-reachability-analysis.test.mts | 164 ++++++++++++++++++ 7 files changed, 366 insertions(+), 6 deletions(-) create mode 100644 src/commands/scan/output-scan-reach.test.mts create mode 100644 src/commands/scan/perform-reachability-analysis.test.mts diff --git a/src/commands/scan/handle-create-new-scan.mts b/src/commands/scan/handle-create-new-scan.mts index 1a944d625..68facffa4 100644 --- a/src/commands/scan/handle-create-new-scan.mts +++ b/src/commands/scan/handle-create-new-scan.mts @@ -314,6 +314,19 @@ export async function handleCreateNewScan({ if (reach && scanId && tier1ReachabilityScanId) { await finalizeTier1Scan(tier1ReachabilityScanId, scanId) + } else if ( + reach.runReachabilityAnalysis && + scanId && + !tier1ReachabilityScanId + ) { + // Reachability analysis ran and a scan was created, but no tier 1 + // reachability scan id was extracted from the facts file. Surface this + // instead of silently skipping finalize — otherwise the tier 1 row stays + // stuck (e.g. at COANA_DONE) and the full scan is never linked to its + // reachability report. + logger.warn( + 'Reachability analysis ran but no tier 1 reachability scan ID was found; skipping tier 1 finalize. The scan was created but its reachability report was not linked.', + ) } // On a successful scan, clean up the `.socket.facts.json` coana wrote at diff --git a/src/commands/scan/handle-create-new-scan.test.mts b/src/commands/scan/handle-create-new-scan.test.mts index 0998e9530..0b702f9f4 100644 --- a/src/commands/scan/handle-create-new-scan.test.mts +++ b/src/commands/scan/handle-create-new-scan.test.mts @@ -1,5 +1,8 @@ -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { logger } from '@socketsecurity/registry/lib/logger' + +import { finalizeTier1Scan } from './finalize-tier1-scan.mts' import { handleCreateNewScan } from './handle-create-new-scan.mts' import type { HandleCreateNewScanConfig } from './handle-create-new-scan.mts' @@ -480,3 +483,65 @@ describe('handleCreateNewScan excludePaths', () => { ) }) }) + +describe('handleCreateNewScan tier1 finalize', () => { + let warnSpy: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => logger) + mockFetchSupportedScanFileNames.mockResolvedValue({ + data: { size: 1 }, + ok: true, + }) + mockFindSocketYmlSync.mockReturnValue({ ok: false }) + mockGetPackageFilesForScan.mockResolvedValue(['package.json']) + mockFetchCreateOrgFullScan.mockResolvedValue({ + data: { id: 'scan-id' }, + ok: true, + }) + }) + + afterEach(() => { + warnSpy.mockRestore() + }) + + it('finalizes the tier 1 scan when a scan id and tier 1 id are present', async () => { + mockPerformReachabilityAnalysis.mockResolvedValue({ + data: { + reachabilityReport: '.socket.facts.json', + tier1ReachabilityScanId: 'tier1-id', + }, + ok: true, + }) + + const config = createConfig() + config.reach.runReachabilityAnalysis = true + + await handleCreateNewScan(config) + + expect(finalizeTier1Scan).toHaveBeenCalledWith('tier1-id', 'scan-id') + expect(warnSpy).not.toHaveBeenCalled() + }) + + it('warns instead of silently skipping finalize when no tier 1 id was extracted', async () => { + mockPerformReachabilityAnalysis.mockResolvedValue({ + data: { + reachabilityReport: '.socket.facts.json', + tier1ReachabilityScanId: undefined, + }, + ok: true, + }) + + const config = createConfig() + config.reach.runReachabilityAnalysis = true + + await handleCreateNewScan(config) + + expect(finalizeTier1Scan).not.toHaveBeenCalled() + expect(warnSpy).toHaveBeenCalledTimes(1) + expect(String(warnSpy.mock.calls[0]![0])).toMatch( + /tier 1 finalize|reachability report was not linked/i, + ) + }) +}) diff --git a/src/commands/scan/handle-scan-reach.mts b/src/commands/scan/handle-scan-reach.mts index 9df5c2a17..e16e4cc37 100644 --- a/src/commands/scan/handle-scan-reach.mts +++ b/src/commands/scan/handle-scan-reach.mts @@ -38,6 +38,7 @@ export async function handleScanReach({ const supportedFilesCResult = await fetchSupportedScanFileNames({ spinner }) if (!supportedFilesCResult.ok) { await outputScanReach(supportedFilesCResult, { + cwd, outputKind, outputPath, }) @@ -103,5 +104,5 @@ export async function handleScanReach({ spinner.stop() - await outputScanReach(result, { outputKind, outputPath }) + await outputScanReach(result, { cwd, outputKind, outputPath }) } diff --git a/src/commands/scan/output-scan-reach.mts b/src/commands/scan/output-scan-reach.mts index dc425e33f..652392c7f 100644 --- a/src/commands/scan/output-scan-reach.mts +++ b/src/commands/scan/output-scan-reach.mts @@ -1,3 +1,5 @@ +import path from 'node:path' + import { logger } from '@socketsecurity/registry/lib/logger' import { pluralize } from '@socketsecurity/registry/lib/words' @@ -11,7 +13,11 @@ import type { CResult, OutputKind } from '../../types.mts' export async function outputScanReach( result: CResult, - { outputKind, outputPath }: { outputKind: OutputKind; outputPath: string }, + { + cwd, + outputKind, + outputPath, + }: { cwd: string; outputKind: OutputKind; outputPath: string }, ): Promise { if (!result.ok) { process.exitCode = result.code ?? 1 @@ -33,7 +39,13 @@ export async function outputScanReach( logger.info(`Reachability report has been written to: ${actualOutputPath}`) // Warn about individual vulnerabilities where reachability analysis errored. - const errors = extractReachabilityErrors(result.data.reachabilityReport) + // Resolve the report path against the scan `cwd` (not `process.cwd()`): + // Coana writes the facts file relative to `cwd` and `reachabilityReport` + // is a `cwd`-relative path, so reading the bare relative path would miss + // the file whenever `cwd !== process.cwd()` (e.g. `--cwd `). + const errors = extractReachabilityErrors( + path.resolve(cwd, result.data.reachabilityReport), + ) if (errors.length) { logger.log('') logger.warn( diff --git a/src/commands/scan/output-scan-reach.test.mts b/src/commands/scan/output-scan-reach.test.mts new file mode 100644 index 000000000..3ddb7548c --- /dev/null +++ b/src/commands/scan/output-scan-reach.test.mts @@ -0,0 +1,95 @@ +/** + * Regression tests for outputScanReach facts-file resolution. + * + * Test Coverage: + * - Per-vulnerability reachability errors must be read from the facts + * file Coana actually wrote at `/.socket.facts.json`, not from a + * relative path resolved against `process.cwd()`, so `--cwd ` runs + * surface their errors instead of silently reporting none. + * + * Related Files: + * - output-scan-reach.mts (implementation) + * - utils/coana.mts (extractReachabilityErrors — exercised for real) + */ + +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { logger } from '@socketsecurity/registry/lib/logger' + +import { outputScanReach } from './output-scan-reach.mts' + +import type { ReachabilityAnalysisResult } from './perform-reachability-analysis.mts' +import type { CResult } from '../../types.mts' + +vi.mock('../../constants.mts', () => ({ + default: { + DOT_SOCKET_DOT_FACTS_JSON: '.socket.facts.json', + }, +})) + +const errorComponentsBody = { + components: [ + { + name: 'lodash', + version: '4.17.21', + reachability: [ + { + ghsa_id: 'GHSA-aaaa-bbbb-cccc', + reachability: [{ type: 'error', subprojectPath: 'packages/web' }], + }, + ], + }, + ], +} + +describe('outputScanReach facts-file resolution', () => { + let scanCwd: string + let warnSpy: ReturnType + + beforeEach(() => { + scanCwd = mkdtempSync(path.join(tmpdir(), 'socket-rea495-out-')) + // Silence the rest of the text output; only assert on warnings. + vi.spyOn(logger, 'log').mockImplementation(() => logger) + vi.spyOn(logger, 'info').mockImplementation(() => logger) + vi.spyOn(logger, 'success').mockImplementation(() => logger) + warnSpy = vi.spyOn(logger, 'warn').mockImplementation(() => logger) + }) + + afterEach(() => { + rmSync(scanCwd, { force: true, recursive: true }) + vi.restoreAllMocks() + }) + + it('reads reachability errors from the facts file under the scan cwd, not process.cwd()', async () => { + writeFileSync( + path.join(scanCwd, '.socket.facts.json'), + JSON.stringify(errorComponentsBody), + ) + + expect(scanCwd).not.toBe(process.cwd()) + + const result: CResult = { + ok: true, + data: { + reachabilityReport: '.socket.facts.json', + tier1ReachabilityScanId: undefined, + }, + } + + await outputScanReach(result, { + cwd: scanCwd, + outputKind: 'text', + outputPath: '', + }) + + // The summary warning plus one per-vulnerability warning are emitted. + expect(warnSpy).toHaveBeenCalled() + const warned = warnSpy.mock.calls.map(c => String(c[0])).join('\n') + expect(warned).toContain('GHSA-aaaa-bbbb-cccc') + expect(warned).toContain('lodash@4.17.21') + }) +}) diff --git a/src/commands/scan/perform-reachability-analysis.mts b/src/commands/scan/perform-reachability-analysis.mts index aac692b29..0c623dab9 100644 --- a/src/commands/scan/perform-reachability-analysis.mts +++ b/src/commands/scan/perform-reachability-analysis.mts @@ -267,12 +267,22 @@ export async function performReachabilityAnalysis( return coanaResult } + // Coana writes the facts file relative to the scan `cwd` (it is spawned + // with `cwd` above), so resolve the read path against `cwd` too. Reading + // the bare relative path would resolve against `process.cwd()` and miss + // the file whenever `cwd !== process.cwd()` (e.g. `--cwd `), silently + // dropping the tier 1 scan id and skipping finalize downstream. + const resolvedReportPath = path.resolve(cwd, outputFilePath) + return { ok: true, data: { - // Use the actual output filename for the scan. + // Use the actual output filename for the scan. Keep this `cwd`-relative + // so the upload (which relativizes against `cwd`) and the post-success + // unlink (`path.resolve(cwd, reachabilityReport)`) keep working. reachabilityReport: outputFilePath, - tier1ReachabilityScanId: extractTier1ReachabilityScanId(outputFilePath), + tier1ReachabilityScanId: + extractTier1ReachabilityScanId(resolvedReportPath), }, } } diff --git a/src/commands/scan/perform-reachability-analysis.test.mts b/src/commands/scan/perform-reachability-analysis.test.mts new file mode 100644 index 000000000..d144c9fdd --- /dev/null +++ b/src/commands/scan/perform-reachability-analysis.test.mts @@ -0,0 +1,164 @@ +/** + * Regression tests for performReachabilityAnalysis facts-file resolution. + * + * Test Coverage: + * - When the scan `cwd` differs from `process.cwd()` (e.g. the + * `--cwd ` flag), the tier 1 reachability scan id must be read from the + * facts file Coana actually wrote at `/.socket.facts.json`, not from a + * relative path resolved against `process.cwd()`. + * + * Related Files: + * - perform-reachability-analysis.mts (implementation) + * - utils/coana.mts (extractTier1ReachabilityScanId — exercised for real) + */ + +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import path from 'node:path' + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { performReachabilityAnalysis } from './perform-reachability-analysis.mts' + +import type { ReachabilityOptions } from './perform-reachability-analysis.mts' + +const { mockFetchOrganization, mockHasEnterpriseOrgPlan, mockSpawnCoanaDlx } = + vi.hoisted(() => ({ + mockFetchOrganization: vi.fn(), + mockHasEnterpriseOrgPlan: vi.fn(), + mockSpawnCoanaDlx: vi.fn(), + })) + +vi.mock('../organization/fetch-organization-list.mts', () => ({ + fetchOrganization: mockFetchOrganization, +})) + +vi.mock('../../utils/organization.mts', () => ({ + hasEnterpriseOrgPlan: mockHasEnterpriseOrgPlan, +})) + +vi.mock('../../utils/dlx.mts', () => ({ + spawnCoanaDlx: mockSpawnCoanaDlx, +})) + +// Stubbed to keep the heavy SDK / API import chains out of the test; the +// happy path below skips the manifest-upload branch entirely. +vi.mock('../../utils/sdk.mts', () => ({ + setupSdk: vi.fn(), +})) + +vi.mock('../../utils/api.mts', () => ({ + handleApiCall: vi.fn(), +})) + +vi.mock('../../utils/terminal-link.mts', () => ({ + socketDevLink: vi.fn((text: string) => text), +})) + +vi.mock('../../constants.mts', () => ({ + default: { + DOT_SOCKET_DOT_FACTS_JSON: '.socket.facts.json', + ENV: { INLINED_SOCKET_CLI_COANA_TECH_CLI_VERSION: 'test' }, + HTTP_STATUS_UNAUTHORIZED: 401, + SOCKET_DEFAULT_BRANCH: 'socket-default-branch', + SOCKET_DEFAULT_REPOSITORY: 'socket-default-repository', + }, +})) + +vi.mock('@socketsecurity/registry/lib/logger', () => ({ + logger: { + error: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + }, +})) + +function makeReachabilityOptions(): ReachabilityOptions { + return { + excludePaths: [], + reachAnalysisMemoryLimit: 0, + reachAnalysisTimeout: 0, + reachConcurrency: 0, + reachContinueOnAnalysisErrors: false, + reachContinueOnInstallErrors: false, + reachContinueOnMissingLockFiles: false, + reachContinueOnNoSourceFiles: false, + reachDebug: false, + reachDetailedAnalysisLogFile: false, + reachDisableExternalToolChecks: false, + reachDisableAnalytics: false, + reachEcosystems: [], + reachEnableAnalysisSplitting: false, + reachExcludePaths: [], + reachLazyMode: false, + reachSkipCache: false, + reachUseOnlyPregeneratedSboms: false, + reachVersion: undefined, + } +} + +describe('performReachabilityAnalysis facts-file resolution', () => { + let scanCwd: string + + beforeEach(() => { + vi.clearAllMocks() + mockFetchOrganization.mockResolvedValue({ + ok: true, + data: { organizations: {} }, + }) + mockHasEnterpriseOrgPlan.mockReturnValue(true) + mockSpawnCoanaDlx.mockResolvedValue({ ok: true, data: '' }) + // A scan cwd that is intentionally NOT process.cwd(). + scanCwd = mkdtempSync(path.join(tmpdir(), 'socket-rea495-')) + }) + + afterEach(() => { + rmSync(scanCwd, { force: true, recursive: true }) + }) + + it('extracts the tier 1 scan id from the facts file under the scan cwd, not process.cwd()', async () => { + // Coana (mocked) is spawned with `cwd`, so it writes the facts file under + // the scan cwd. Pre-write it here to stand in for that output. + writeFileSync( + path.join(scanCwd, '.socket.facts.json'), + JSON.stringify({ tier1ReachabilityScanId: 'reach-scan-rea495' }), + ) + + expect(scanCwd).not.toBe(process.cwd()) + + const result = await performReachabilityAnalysis({ + cwd: scanCwd, + reachabilityOptions: makeReachabilityOptions(), + target: scanCwd, + }) + + expect(mockSpawnCoanaDlx).toHaveBeenCalledTimes(1) + // The Coana spawn must use the scan cwd so its write and our read agree. + expect(mockSpawnCoanaDlx.mock.calls[0]![2]).toMatchObject({ cwd: scanCwd }) + + expect(result.ok).toBe(true) + expect(result.ok && result.data.tier1ReachabilityScanId).toBe( + 'reach-scan-rea495', + ) + // The returned report path stays cwd-relative for upload / unlink. + expect(result.ok && result.data.reachabilityReport).toBe( + '.socket.facts.json', + ) + }) + + it('returns undefined tier 1 scan id when the facts file under cwd has none', async () => { + writeFileSync( + path.join(scanCwd, '.socket.facts.json'), + JSON.stringify({ components: [] }), + ) + + const result = await performReachabilityAnalysis({ + cwd: scanCwd, + reachabilityOptions: makeReachabilityOptions(), + target: scanCwd, + }) + + expect(result.ok).toBe(true) + expect(result.ok && result.data.tier1ReachabilityScanId).toBeUndefined() + }) +})