diff --git a/packages/cli/src/commands/scan/handle-create-new-scan.mts b/packages/cli/src/commands/scan/handle-create-new-scan.mts index bcdd558a1..5beab9e87 100644 --- a/packages/cli/src/commands/scan/handle-create-new-scan.mts +++ b/packages/cli/src/commands/scan/handle-create-new-scan.mts @@ -208,6 +208,11 @@ export async function handleCreateNewScan({ let scanPaths: string[] = packagePaths let tier1ReachabilityScanId: string | undefined let reachabilityReport: string | undefined + // Whether this run generated the .socket.facts.json itself. We only clean + // up a facts file we produced — a pre-existing one (the user pre-generated + // it, or --reach-use-only-pregenerated-sboms points at their own artifacts) + // is left untouched. + let generatedFactsFile = false // If reachability is enabled, perform reachability analysis. if (reach.runReachabilityAnalysis) { @@ -229,6 +234,15 @@ export async function handleCreateNewScan({ debugNs('notice', 'Reachability analysis enabled') debugDir('inspect', { reachabilityOptions: mergedReachabilityOptions }) + // Record whether the facts file was already on disk before the analysis + // ran. The create flow always writes coana's report to the default + // .socket.facts.json in cwd, so a file present now was not produced by + // this run and must be preserved. --reach-use-only-pregenerated-sboms + // likewise signals the user is managing their own artifacts. + const factsFilePreExisted = existsSync( + path.resolve(cwd, DOT_SOCKET_DOT_FACTS_JSON), + ) + spinner.start() const reachResult = await performReachabilityAnalysis({ @@ -252,6 +266,8 @@ export async function handleCreateNewScan({ logger.success('Reachability analysis completed successfully') reachabilityReport = reachResult.data?.reachabilityReport + generatedFactsFile = + !factsFilePreExisted && !reach.reachUseOnlyPregeneratedSboms scanPaths = [ ...excludeFactsJson(packagePaths), @@ -349,9 +365,12 @@ export async function handleCreateNewScan({ await finalizeTier1Scan(tier1ReachabilityScanId, scanId) } - if (fullScanCResult.ok && reachabilityReport) { + if (fullScanCResult.ok && reachabilityReport && generatedFactsFile) { // The facts file is an upload artifact, not user-facing output — remove - // it once the scan is submitted so it doesn't linger in the project. + // it once the scan is submitted so it doesn't linger in the project. Only + // a file we generated this run is removed; a pre-existing or + // user-pre-generated facts file is left in place (see generatedFactsFile). + // On submission failure we intentionally keep the file for debuggability. await safeDelete(path.resolve(cwd, reachabilityReport), { force: true }) } diff --git a/packages/cli/test/unit/commands/scan/handle-create-new-scan-features.test.mts b/packages/cli/test/unit/commands/scan/handle-create-new-scan-features.test.mts index bd900c3ed..2f764e9f5 100644 --- a/packages/cli/test/unit/commands/scan/handle-create-new-scan-features.test.mts +++ b/packages/cli/test/unit/commands/scan/handle-create-new-scan-features.test.mts @@ -125,13 +125,6 @@ vi.mock(import('../../../../src/util/basics/spawn.mts'), () => ({ runSocketBasics: mockRunSocketBasics, })) -const mockSafeDelete = vi.hoisted(() => vi.fn()) -// Keep safeDeleteSync real — tests below use it to clean up tmp dirs. -vi.mock(import('@socketsecurity/lib-stable/fs/safe'), async importOriginal => ({ - ...(await importOriginal()), - safeDelete: mockSafeDelete, -})) - describe('handleCreateNewScan', () => { const mockConfig = { autoManifest: false, @@ -207,65 +200,6 @@ describe('handleCreateNewScan', () => { expect(finalizeTier1Scan).toHaveBeenCalledWith('tier1-scan-456', 'scan-789') }) - it('deletes the reachability facts file after a successful submission', async () => { - mockFetchSupportedScanFileNames.mockResolvedValue( - createSuccessResult(new Set(['package.json'])), - ) - mockGetPackageFilesForScan.mockResolvedValue(['/test/project/package.json']) - mockCheckCommandInput.mockReturnValue(true) - mockPerformReachabilityAnalysis.mockResolvedValue( - createSuccessResult({ - reachabilityReport: '/test/project/.socket.facts.json', - tier1ReachabilityScanId: 'tier1-scan-456', - }), - ) - mockFetchCreateOrgFullScan.mockResolvedValue( - createSuccessResult({ id: 'scan-789' }), - ) - - await handleCreateNewScan({ - ...mockConfig, - reach: { - excludePaths: [], - reachExcludePaths: [], - runReachabilityAnalysis: true, - }, - }) - - expect(mockSafeDelete).toHaveBeenCalledWith( - '/test/project/.socket.facts.json', - { force: true }, - ) - }) - - it('keeps the reachability facts file when submission fails', async () => { - mockFetchSupportedScanFileNames.mockResolvedValue( - createSuccessResult(new Set(['package.json'])), - ) - mockGetPackageFilesForScan.mockResolvedValue(['/test/project/package.json']) - mockCheckCommandInput.mockReturnValue(true) - mockPerformReachabilityAnalysis.mockResolvedValue( - createSuccessResult({ - reachabilityReport: '/test/project/.socket.facts.json', - tier1ReachabilityScanId: 'tier1-scan-456', - }), - ) - mockFetchCreateOrgFullScan.mockResolvedValue( - createErrorResult('upload failed'), - ) - - await handleCreateNewScan({ - ...mockConfig, - reach: { - excludePaths: [], - reachExcludePaths: [], - runReachabilityAnalysis: true, - }, - }) - - expect(mockSafeDelete).not.toHaveBeenCalled() - }) - it('handles scan report generation', async () => { await import('../../../../src/commands/scan/fetch-supported-scan-file-names.mts') await import('../../../../src/util/fs/path-resolve.mts') diff --git a/packages/cli/test/unit/commands/scan/handle-create-new-scan.test.mts b/packages/cli/test/unit/commands/scan/handle-create-new-scan.test.mts index 439553ce4..46c32501e 100644 --- a/packages/cli/test/unit/commands/scan/handle-create-new-scan.test.mts +++ b/packages/cli/test/unit/commands/scan/handle-create-new-scan.test.mts @@ -23,6 +23,7 @@ import { createSuccessResult, } from '../../../helpers/mocks.mts' import { handleCreateNewScan } from '../../../../src/commands/scan/handle-create-new-scan.mts' +import { safeDeleteSync } from '@socketsecurity/lib-stable/fs/safe' // Mock all the dependencies. const mockLogger = vi.hoisted(() => ({ @@ -313,4 +314,150 @@ describe('handleCreateNewScan', () => { }, ) }) + + describe('reachability facts file cleanup', () => { + // These run against a real tmp cwd and the real safeDelete so the + // assertions observe actual on-disk state after submission. + async function setupTmpProject() { + const path = await import('node:path') + const os = await import('node:os') + const fs = await import('node:fs') + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'cli-scan-facts-')) + const factsPath = path.join(tmpDir, '.socket.facts.json') + mockFetchSupportedScanFileNames.mockResolvedValue( + createSuccessResult(new Set(['package.json'])), + ) + mockGetPackageFilesForScan.mockResolvedValue([ + path.join(tmpDir, 'package.json'), + ]) + mockCheckCommandInput.mockReturnValue(true) + return { factsPath, fs, tmpDir } + } + + it('deletes a facts file it generated after a successful submission', async () => { + const { factsPath, fs, tmpDir } = await setupTmpProject() + try { + // Coana writes the facts file during the analysis, not before it. + mockPerformReachabilityAnalysis.mockImplementation(async () => { + fs.writeFileSync(factsPath, '{}', 'utf8') + return createSuccessResult({ + reachabilityReport: factsPath, + tier1ReachabilityScanId: 'tier1-scan-456', + }) + }) + mockFetchCreateOrgFullScan.mockResolvedValue( + createSuccessResult({ id: 'scan-789' }), + ) + + await handleCreateNewScan({ + ...mockConfig, + cwd: tmpDir, + reach: { + excludePaths: [], + reachExcludePaths: [], + runReachabilityAnalysis: true, + }, + }) + + expect(fs.existsSync(factsPath)).toBe(false) + } finally { + safeDeleteSync(tmpDir) + } + }) + + it('keeps a facts file it generated when the submission fails', async () => { + const { factsPath, fs, tmpDir } = await setupTmpProject() + try { + mockPerformReachabilityAnalysis.mockImplementation(async () => { + fs.writeFileSync(factsPath, '{}', 'utf8') + return createSuccessResult({ + reachabilityReport: factsPath, + tier1ReachabilityScanId: 'tier1-scan-456', + }) + }) + mockFetchCreateOrgFullScan.mockResolvedValue( + createErrorResult('upload failed'), + ) + + await handleCreateNewScan({ + ...mockConfig, + cwd: tmpDir, + reach: { + excludePaths: [], + reachExcludePaths: [], + runReachabilityAnalysis: true, + }, + }) + + expect(fs.existsSync(factsPath)).toBe(true) + } finally { + safeDeleteSync(tmpDir) + } + }) + + it('keeps a facts file that pre-existed the run', async () => { + const { factsPath, fs, tmpDir } = await setupTmpProject() + try { + // The file is on disk before the analysis, so it was not produced by + // this run (e.g. the user pre-generated it) and must be preserved. + fs.writeFileSync(factsPath, '{}', 'utf8') + mockPerformReachabilityAnalysis.mockResolvedValue( + createSuccessResult({ + reachabilityReport: factsPath, + tier1ReachabilityScanId: 'tier1-scan-456', + }), + ) + mockFetchCreateOrgFullScan.mockResolvedValue( + createSuccessResult({ id: 'scan-789' }), + ) + + await handleCreateNewScan({ + ...mockConfig, + cwd: tmpDir, + reach: { + excludePaths: [], + reachExcludePaths: [], + runReachabilityAnalysis: true, + }, + }) + + expect(fs.existsSync(factsPath)).toBe(true) + } finally { + safeDeleteSync(tmpDir) + } + }) + + it('keeps the facts file when --reach-use-only-pregenerated-sboms is set', async () => { + const { factsPath, fs, tmpDir } = await setupTmpProject() + try { + // In pregenerated-SBOMs mode the user manages their own artifacts, so + // the facts file is left in place even though this run wrote it. + mockPerformReachabilityAnalysis.mockImplementation(async () => { + fs.writeFileSync(factsPath, '{}', 'utf8') + return createSuccessResult({ + reachabilityReport: factsPath, + tier1ReachabilityScanId: 'tier1-scan-456', + }) + }) + mockFetchCreateOrgFullScan.mockResolvedValue( + createSuccessResult({ id: 'scan-789' }), + ) + + await handleCreateNewScan({ + ...mockConfig, + cwd: tmpDir, + reach: { + excludePaths: [], + reachExcludePaths: [], + reachUseOnlyPregeneratedSboms: true, + runReachabilityAnalysis: true, + }, + }) + + expect(fs.existsSync(factsPath)).toBe(true) + } finally { + safeDeleteSync(tmpDir) + } + }) + }) })