From 7d4e354d30ce58fac0134ec8623fc3f41d7571eb Mon Sep 17 00:00:00 2001 From: Casey Marshall Date: Tue, 4 Jun 2024 19:27:07 -0500 Subject: [PATCH] feat: consolidate json streaming --- src/cli/commands/test/iac/output.ts | 27 +++--- src/cli/commands/test/index.ts | 48 +++------- src/cli/commands/types.ts | 96 +++++-------------- src/cli/main.ts | 39 +++----- src/lib/ecosystems/test.ts | 16 ++-- .../formatters/test/format-test-results.ts | 18 +--- src/lib/iac/test/v2/output.ts | 4 +- src/lib/json-file-output.ts | 14 +-- src/lib/json.ts | 1 + src/lib/print-deps.ts | 6 +- src/lib/snyk-test/assemble-payloads.ts | 11 +-- src/lib/snyk-test/common.ts | 14 +++ src/lib/snyk-test/run-test.ts | 4 +- src/lib/types.ts | 7 +- 14 files changed, 104 insertions(+), 201 deletions(-) diff --git a/src/cli/commands/test/iac/output.ts b/src/cli/commands/test/iac/output.ts index 4bae1e867c..665e745a01 100644 --- a/src/cli/commands/test/iac/output.ts +++ b/src/cli/commands/test/iac/output.ts @@ -101,23 +101,21 @@ export function buildOutput({ const mappedResults = results.map(mapIacTestResult); const { - stdout: dataToSend, - stringifiedData, - stringifiedJsonData, - stringifiedSarifData, + dataToSend, + jsonData, + sarifData, } = extractDataToSendFromResults(results, mappedResults, options); if (options.json || options.sarif) { // if all results are ok (.ok == true) if (mappedResults.every((res) => res.ok)) { return TestCommandResult.createJsonTestCommandResult( - stringifiedData, - stringifiedJsonData, - stringifiedSarifData, + jsonData, + sarifData, ); } - const err = new Error(stringifiedData) as any; + const err = new Error() as any; if (foundVulnerabilities) { err.code = 'VULNS'; @@ -134,9 +132,8 @@ export function buildOutput({ // the first error. err.code = errorResults[0].code; } - err.json = stringifiedData; - err.jsonStringifiedResults = stringifiedJsonData; - err.sarifStringifiedResults = stringifiedSarifData; + err.jsonData = jsonData; + err.sarifData = sarifData; throw err; } @@ -228,15 +225,15 @@ export function buildOutput({ // first one error.code = vulnerableResults[0].code || 'VULNS'; error.userMessage = vulnerableResults[0].userMessage; - error.jsonStringifiedResults = stringifiedJsonData; - error.sarifStringifiedResults = stringifiedSarifData; + error.jsonData = jsonData; + error.sarifData = sarifData; throw error; } return TestCommandResult.createHumanReadableTestCommandResult( response, - stringifiedJsonData, - stringifiedSarifData, + jsonData, + sarifData, ); } diff --git a/src/cli/commands/test/index.ts b/src/cli/commands/test/index.ts index fb638f3c3f..db6d20f855 100644 --- a/src/cli/commands/test/index.ts +++ b/src/cli/commands/test/index.ts @@ -202,26 +202,20 @@ export default async function test( const mappedResults = createErrorMappedResultsForJsonOutput(results); const { - stdout: dataToSend, - stringifiedData, - stringifiedJsonData, - stringifiedSarifData, + dataToSend, + jsonData, + sarifData, } = extractDataToSendFromResults(results, mappedResults, options); - const jsonPayload = stringifiedJsonData.length === 0 ? dataToSend : null; - if (options.json || options.sarif) { // if all results are ok (.ok == true) if (mappedResults.every((res) => res.ok)) { return TestCommandResult.createJsonTestCommandResult( - stringifiedData, - stringifiedJsonData, - stringifiedSarifData, - jsonPayload, + jsonData, sarifData, ); } - const err = new Error(stringifiedData) as any; + const err = new Error() as any; if (foundVulnerabilities) { if (options.failOn) { @@ -229,10 +223,7 @@ export default async function test( if (!fail) { // return here to prevent failure return TestCommandResult.createJsonTestCommandResult( - stringifiedData, - stringifiedJsonData, - stringifiedSarifData, - jsonPayload, + jsonData, sarifData, ); } } @@ -249,13 +240,8 @@ export default async function test( // the first error. err.code = errorResults[0].code; } - err.json = stringifiedData; - err.jsonStringifiedResults = stringifiedJsonData; - err.sarifStringifiedResults = stringifiedSarifData; - // set jsonPayload if we failed to stringify it - if (jsonPayload) { - err.jsonPayload = jsonPayload; - } + err.jsonData = jsonData; + err.sarifData = sarifData; throw err; } @@ -315,10 +301,7 @@ export default async function test( ); return TestCommandResult.createHumanReadableTestCommandResult( - response, - stringifiedJsonData, - stringifiedSarifData, - jsonPayload, + response, jsonData, sarifData, ); } } @@ -339,12 +322,8 @@ export default async function test( // first one error.code = vulnerableResults[0].code || 'VULNS'; error.userMessage = vulnerableResults[0].userMessage; - error.jsonStringifiedResults = stringifiedJsonData; - error.sarifStringifiedResults = stringifiedSarifData; - // conditionally set jsonPayload for now, to determine whether to stream data to destination - if (stringifiedJsonData.length === 0) { - error.jsonPayload = dataToSend; - } + error.jsonData + error.sarifData throw error; } @@ -356,9 +335,8 @@ export default async function test( return TestCommandResult.createHumanReadableTestCommandResult( response, - stringifiedJsonData, - stringifiedSarifData, - jsonPayload, + jsonData, + sarifData, ); } diff --git a/src/cli/commands/types.ts b/src/cli/commands/types.ts index 9d27b3257f..635f9654b1 100644 --- a/src/cli/commands/types.ts +++ b/src/cli/commands/types.ts @@ -15,113 +15,65 @@ export class CommandResult { } } -export abstract class TestCommandResult extends CommandResult { - protected jsonResult = ''; - protected sarifResult = ''; - protected jsonData = {}; +export type JsonDocument = Record | Array; - public getJsonResult(): string { - return this.jsonResult; - } +export abstract class TestCommandResult extends CommandResult { + protected jsonData?: JsonDocument; + protected sarifData?: JsonDocument; - public getSarifResult(): string { - return this.sarifResult; + public getJsonData(): JsonDocument | undefined { + return this.jsonData; } - public getJsonData(): Record { - return this.jsonData; + public getSarifData(): JsonDocument | undefined { + return this.sarifData; } public static createHumanReadableTestCommandResult( humanReadableResult: string, - jsonResult: string, - sarifResult?: string, - jsonData?: Record, + jsonData?: JsonDocument, + sarifData?: JsonDocument, ): HumanReadableTestCommandResult { return new HumanReadableTestCommandResult( humanReadableResult, - jsonResult, - sarifResult, jsonData, + sarifData, ); } public static createJsonTestCommandResult( - stdout: string, - jsonResult?: string, - sarifResult?: string, - jsonPayload?: Record, + jsonData?: JsonDocument, + sarifData?: JsonDocument, ): JsonTestCommandResult { return new JsonTestCommandResult( - stdout, - jsonResult, - sarifResult, - jsonPayload, + '', + jsonData, + sarifData, ); } } class HumanReadableTestCommandResult extends TestCommandResult { - protected jsonResult = ''; - protected sarifResult = ''; - protected jsonData = {}; - constructor( humanReadableResult: string, - jsonResult: string, - sarifResult?: string, - jsonData?: Record, + jsonData?: JsonDocument, + sarifData?: JsonDocument, ) { super(humanReadableResult); - this.jsonResult = jsonResult; - if (sarifResult) { - this.sarifResult = sarifResult; - } - if (jsonData) { - this.jsonData = jsonData; - } - } - - public getJsonResult(): string { - return this.jsonResult; - } - - public getSarifResult(): string { - return this.sarifResult; - } - - public getJsonData(): Record { - return this.jsonData; + this.jsonData = jsonData; + this.sarifData = sarifData; } } class JsonTestCommandResult extends TestCommandResult { constructor( stdout: string, - jsonResult?: string, - sarifResult?: string, - jsonData?: Record, + jsonData?: JsonDocument, + sarifData?: JsonDocument, ) { super(stdout); - if (jsonResult) { - this.jsonResult = jsonResult; - } - if (sarifResult) { - this.sarifResult = sarifResult; - } else { - this.jsonResult = stdout; - } - if (jsonData) { - this.jsonData = jsonData; - } - } - - public getJsonResult(): string { - return this.jsonResult; - } - - public getSarifResult(): string { - return this.sarifResult; + this.jsonData = jsonData; + this.sarifData = sarifData; } } diff --git a/src/cli/main.ts b/src/cli/main.ts index 86d6d56070..45d8e66317 100755 --- a/src/cli/main.ts +++ b/src/cli/main.ts @@ -15,7 +15,7 @@ import * as runtime from './runtime'; import * as analytics from '../lib/analytics'; import * as alerts from '../lib/alerts'; import * as sln from '../lib/sln'; -import { TestCommandResult } from './commands/types'; +import { JsonDocument, TestCommandResult } from './commands/types'; import { copy } from './copy'; import { spinner } from '../lib/spinner'; import * as errors from '../lib/errors/legacy-errors'; @@ -36,7 +36,6 @@ import { modeValidation } from './modes'; import { JsonFileOutputBadInputError } from '../lib/errors/json-file-output-bad-input-error'; import { saveObjectToFile, - saveJsonToFileCreatingDirectoryIfRequired, } from '../lib/json-file-output'; import { Options, @@ -78,10 +77,9 @@ async function runCommand(args: Args) { // also save the json (in error.json) to file if option is set if (args.command === 'test') { - const jsonResults = (commandResult as TestCommandResult).getJsonResult(); const jsonPayload = (commandResult as TestCommandResult).getJsonData(); - await saveResultsToFile(args.options, 'json', jsonResults, jsonPayload); - const sarifResults = (commandResult as TestCommandResult).getSarifResult(); + await saveResultsToFile(args.options, 'json', jsonPayload); + const sarifResults = (commandResult as TestCommandResult).getSarifData(); await saveResultsToFile(args.options, 'sarif', sarifResults); } @@ -145,8 +143,8 @@ async function handleError(args, error) { const output = vulnsFound ? error.message : stripAnsi(error.json || error.stack); - if (error.jsonPayload) { - new JsonStreamStringify(error.jsonPayload, undefined, 2).pipe( + if (error.jsonData) { + new JsonStreamStringify(error.jsonData, undefined, 2).pipe( process.stdout, ); } else { @@ -169,14 +167,12 @@ async function handleError(args, error) { } } - if (error.jsonPayload) { - // send raw jsonPayload instead of stringified payload - await saveResultsToFile(args.options, 'json', '', error.jsonPayload); - } else { - // fallback to original behaviour - await saveResultsToFile(args.options, 'json', error.jsonStringifiedResults); + if (error.jsonData) { + await saveResultsToFile(args.options, 'json', error.jsonData); + } + if (error.sarifData) { + await saveResultsToFile(args.options, 'sarif', error.sarifData); } - await saveResultsToFile(args.options, 'sarif', error.sarifStringifiedResults); const analyticsError = vulnsFound ? { @@ -224,9 +220,8 @@ function getFullPath(filepathFragment: string): string { } async function saveJsonResultsToFile( - stringifiedJson: string, jsonOutputFile: string, - jsonPayload?: Record, + jsonPayload?: JsonDocument, ) { if (!jsonOutputFile) { console.error('empty jsonOutputFile'); @@ -238,14 +233,8 @@ async function saveJsonResultsToFile( return; } - // save to file with jsonPayload object instead of stringifiedJson if (jsonPayload && !isEmpty(jsonPayload)) { await saveObjectToFile(jsonOutputFile, jsonPayload); - } else { - await saveJsonToFileCreatingDirectoryIfRequired( - jsonOutputFile, - stringifiedJson, - ); } } @@ -461,16 +450,14 @@ function validateUnsupportedSarifCombinations(args) { async function saveResultsToFile( options: ArgsOptions, outputType: string, - jsonResults: string, - jsonPayload?: Record, + jsonPayload?: JsonDocument, ) { const flag = `${outputType}-file-output`; const outputFile = options[flag]; - if (outputFile && (jsonResults || !isEmpty(jsonPayload))) { + if (outputFile && !isEmpty(jsonPayload)) { const outputFileStr = outputFile as string; const fullOutputFilePath = getFullPath(outputFileStr); await saveJsonResultsToFile( - stripAnsi(jsonResults), fullOutputFilePath, jsonPayload, ); diff --git a/src/lib/ecosystems/test.ts b/src/lib/ecosystems/test.ts index c7f8b8e0c1..95f1997071 100644 --- a/src/lib/ecosystems/test.ts +++ b/src/lib/ecosystems/test.ts @@ -2,7 +2,7 @@ import config from '../config'; import { isCI } from '../is-ci'; import { makeRequest } from '../request/promise'; import { Options, PolicyOptions } from '../types'; -import { TestCommandResult } from '../../cli/commands/types'; +import { JsonDocument, TestCommandResult } from '../../cli/commands/types'; import { spinner } from '../../lib/spinner'; import { Ecosystem, ScanResult, TestResult } from './types'; import { getPlugin } from './plugins'; @@ -31,10 +31,11 @@ export async function testEcosystem( paths, options, ); + // TODO(cmars, 2024-06-04): what still uses this code path? Can we remove it? return TestCommandResult.createHumanReadableTestCommandResult( res, - '', - sarifRes, + undefined, + sarifRes ? JSON.parse(sarifRes) : undefined, ); } const results: ScanResultsByPath = {}; @@ -57,9 +58,8 @@ export async function testEcosystem( options, ); - const stringifiedData = JSON.stringify(testResults, null, 2); if (options.json) { - return TestCommandResult.createJsonTestCommandResult(stringifiedData); + return TestCommandResult.createJsonTestCommandResult(testResults as any as JsonDocument); } const emptyResults: ScanResult[] = []; const scanResults = emptyResults.concat(...Object.values(results)); @@ -73,7 +73,7 @@ export async function testEcosystem( return TestCommandResult.createHumanReadableTestCommandResult( readableResult, - stringifiedData, + testResults as any as JsonDocument, ); } @@ -94,8 +94,8 @@ export async function formatUnmanagedResults( const [result] = await getUnmanagedDepGraph(results); const depGraph = convertDepGraph(result); - return TestCommandResult.createJsonTestCommandResult( - depGraphToOutputString(depGraph, target), + return TestCommandResult.createHumanReadableTestCommandResult( + depGraphToOutputString(depGraph, target) ); } diff --git a/src/lib/formatters/test/format-test-results.ts b/src/lib/formatters/test/format-test-results.ts index dff3d27d18..0ad0777e9a 100644 --- a/src/lib/formatters/test/format-test-results.ts +++ b/src/lib/formatters/test/format-test-results.ts @@ -21,7 +21,6 @@ import { formatDockerBinariesIssues } from '../docker'; import { createSarifOutputForContainers } from '../sarif-output'; import { createSarifOutputForIac } from '../iac-output/sarif'; import { isNewVuln, isVulnFixable } from '../../vuln-helpers'; -import { jsonStringifyLargeObject } from '../../json'; import { createSarifOutputForOpenSource } from '../open-source-sarif-output'; import { getSeverityValue } from '../get-severity-value'; import { showFixTip } from '../show-fix-tip'; @@ -70,7 +69,6 @@ export function extractDataToSendFromResults( options: Options, ): OutputDataTypes { let sarifData = {}; - let stringifiedSarifData = ''; if (options.sarif || options['sarif-file-output']) { if (options.iac) { sarifData = createSarifOutputForIac(results); @@ -79,7 +77,6 @@ export function extractDataToSendFromResults( } else { sarifData = createSarifOutputForOpenSource(results); } - stringifiedSarifData = jsonStringifyLargeObject(sarifData); } const jsonResults = mappedResults.map((res) => @@ -100,21 +97,12 @@ export function extractDataToSendFromResults( jsonData['applications'] = appVulnsData; } - let stringifiedJsonData = ''; - if (options.json || options['json-file-output']) { - stringifiedJsonData = jsonStringifyLargeObject(jsonData); - } - const dataToSend = options.sarif ? sarifData : jsonData; - const stringifiedData = options.sarif - ? stringifiedSarifData - : stringifiedJsonData; return { - stdout: dataToSend, // this is for the human-readable stdout output and is set even if --json or --sarif is set - stringifiedData, // this will be used to display either the Snyk or SARIF format JSON to stdout if --json or --sarif is set - stringifiedJsonData, // this will be used for the --json-file-output= option - stringifiedSarifData, // this will be used for the --sarif-file-output= option + dataToSend, // this is for the human-readable stdout output and is set even if --json or --sarif is set + jsonData, + sarifData, }; } diff --git a/src/lib/iac/test/v2/output.ts b/src/lib/iac/test/v2/output.ts index e6463e92c3..b8e9dbc7a7 100644 --- a/src/lib/iac/test/v2/output.ts +++ b/src/lib/iac/test/v2/output.ts @@ -59,7 +59,6 @@ export function buildOutput({ if (options.json || options.sarif) { return TestCommandResult.createJsonTestCommandResult( - responseData, jsonData, sarifData, ); @@ -121,7 +120,8 @@ function buildTestCommandResultData({ }); } - return { responseData, jsonData, sarifData }; + // TODO(cmars, 2024-06-05): refactor iac to provide objects so we don't have to re-parse here + return { responseData, jsonData: JSON.parse(jsonData), sarifData: JSON.parse(sarifData) }; } const SEPARATOR = '\n-------------------------------------------------------\n'; diff --git a/src/lib/json-file-output.ts b/src/lib/json-file-output.ts index 4edb891452..5c26aa76c1 100644 --- a/src/lib/json-file-output.ts +++ b/src/lib/json-file-output.ts @@ -2,6 +2,7 @@ import { gte } from 'semver'; import { existsSync, mkdirSync, createWriteStream } from 'fs'; import * as path from 'path'; import { JsonStreamStringify } from 'json-stream-stringify'; +import { JsonDocument } from '../cli/commands/types'; export const MIN_VERSION_FOR_MKDIR_RECURSIVE = '10.12.0'; @@ -69,20 +70,9 @@ export async function writeContentsToFileSwallowingErrors( }); } -export async function saveJsonToFileCreatingDirectoryIfRequired( - jsonOutputFile: string, - contents: string, -): Promise { - const dirPath = path.dirname(jsonOutputFile); - const createDirSuccess = createDirectory(dirPath); - if (createDirSuccess) { - await writeContentsToFileSwallowingErrors(jsonOutputFile, contents); - } -} - export async function saveObjectToFile( jsonOutputFile: string, - jsonPayload: Record, + jsonPayload: JsonDocument, ): Promise { const dirPath = path.dirname(jsonOutputFile); const createDirSuccess = createDirectory(dirPath); diff --git a/src/lib/json.ts b/src/lib/json.ts index f72943e08c..26c91b4dc4 100644 --- a/src/lib/json.ts +++ b/src/lib/json.ts @@ -4,6 +4,7 @@ const debug = require('debug')('snyk-json'); * Attempt to json-stringify an object which is potentially very large and might exceed the string limit. * If it does exceed the string limit, return empty string. * @param obj the object from which you want to get a JSON string + * @deprecated Don't use this, it can exceed max string length. */ export function jsonStringifyLargeObject(obj: any): string { let res = ''; diff --git a/src/lib/print-deps.ts b/src/lib/print-deps.ts index 3e5a8a1382..5f5532bcee 100644 --- a/src/lib/print-deps.ts +++ b/src/lib/print-deps.ts @@ -3,7 +3,7 @@ import * as depGraphLib from '@snyk/dep-graph'; import { DepDict, Options, MonitorOptions } from './types'; import { legacyCommon as legacyApi } from '@snyk/cli-interface'; import { countPathsToGraphRoot } from './utils'; -import { jsonStringifyLargeObject } from './json'; +import { JsonStreamStringify } from 'json-stream-stringify'; export async function maybePrintDepGraph( options: Options | MonitorOptions, @@ -26,7 +26,7 @@ export async function maybePrintDepGraph( '--print-deps --json option not yet supported for large projects. Displaying graph json output instead', ); // TODO @boost: add as output graphviz 'dot' file to visualize? - console.log(jsonStringifyLargeObject(depGraph.toJSON())); + new JsonStreamStringify(depGraph).pipe(process.stdout); } else { console.warn( '--print-deps option not yet supported for large projects. Try with --json.', @@ -45,7 +45,7 @@ export function maybePrintDepTree( if (options['print-deps']) { if (options.json) { // Will produce 2 JSON outputs, one for the deps, one for the vuln scan. - console.log(jsonStringifyLargeObject(rootPackage)); + new JsonStreamStringify(rootPackage).pipe(process.stdout); } else { printDepsForTree({ [rootPackage.name!]: rootPackage }); } diff --git a/src/lib/snyk-test/assemble-payloads.ts b/src/lib/snyk-test/assemble-payloads.ts index 233b06be12..dedf36d300 100644 --- a/src/lib/snyk-test/assemble-payloads.ts +++ b/src/lib/snyk-test/assemble-payloads.ts @@ -5,7 +5,7 @@ import { getPlugin } from '../ecosystems'; import { Ecosystem, ContainerTarget, ScanResult } from '../ecosystems/types'; import { Options, PolicyOptions, TestOptions } from '../types'; import { Payload } from './types'; -import { assembleQueryString, depGraphToOutputString } from './common'; +import { assembleQueryString, depGraphToOutputStream } from './common'; import { spinner } from '../spinner'; import { findAndLoadPolicyForScanResult } from '../ecosystems/policy'; import { getAuthHeader } from '../../lib/api-token'; @@ -59,11 +59,10 @@ export async function assembleEcosystemPayloads( // those. const dg = scanResult.facts.find((dg) => dg.type === 'depGraph'); if (dg) { - console.log( - depGraphToOutputString( - dg.data.toJSON(), - constructProjectName(scanResult), - ), + depGraphToOutputStream( + dg.data.toJSON(), + constructProjectName(scanResult), + process.stdout, ); } } diff --git a/src/lib/snyk-test/common.ts b/src/lib/snyk-test/common.ts index bd158b4812..f5a43e9461 100644 --- a/src/lib/snyk-test/common.ts +++ b/src/lib/snyk-test/common.ts @@ -2,6 +2,7 @@ import config from '../config'; import { color } from '../theme'; import { DepGraphData } from '@snyk/dep-graph'; import { jsonStringifyLargeObject } from '../json'; +import { JsonStreamStringify } from 'json-stream-stringify'; export function assembleQueryString(options) { const org = options.org || config.org || null; @@ -83,3 +84,16 @@ DepGraph target: ${targetName} DepGraph end`; } + +export function depGraphToOutputStream( + dg: DepGraphData, + targetName: string, + out: NodeJS.WritableStream, +): void { + out.write(`DepGraph data:\n`); + new JsonStreamStringify(dg).pipe(out); + out.write(` +DepGraph target: +${targetName} +DepGraph end`); +} diff --git a/src/lib/snyk-test/run-test.ts b/src/lib/snyk-test/run-test.ts index 2f79de7301..99bc2b5378 100644 --- a/src/lib/snyk-test/run-test.ts +++ b/src/lib/snyk-test/run-test.ts @@ -768,9 +768,7 @@ async function assembleLocalPayloads( ); } - console.log( - common.depGraphToOutputString(root.toJSON(), targetFile || ''), - ); + common.depGraphToOutputStream(root.toJSON(), targetFile || '', process.stdout); } const body: PayloadBody = { diff --git a/src/lib/types.ts b/src/lib/types.ts index a8df7357ca..444c211b3a 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -222,10 +222,9 @@ export interface SpinnerOptions { } export interface OutputDataTypes { - stdout: any; - stringifiedData: string; - stringifiedJsonData: string; - stringifiedSarifData: string; + dataToSend: any; + jsonData: any; + sarifData: any; } export type SupportedProjectTypes = IacProjectTypes | SupportedPackageManagers;