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() + }) +})