diff --git a/packages/client/src/loadConfiguration.ts b/packages/client/src/loadConfiguration.ts index 5696f2e400..e7b02f33d6 100644 --- a/packages/client/src/loadConfiguration.ts +++ b/packages/client/src/loadConfiguration.ts @@ -1,6 +1,10 @@ import Configuration from './configuration'; +<<<<<<< HEAD export const DefaultURL = 'https://app.land'; +======= +const DefaultURL = 'https://app.land'; +>>>>>>> 701fa4d5 (feat: Recognize APPMAP_* env vars) class Settings { baseURL = DefaultURL; diff --git a/packages/scanner/src/cli.ts b/packages/scanner/src/cli.ts index 19b8a1a610..428537d255 100644 --- a/packages/scanner/src/cli.ts +++ b/packages/scanner/src/cli.ts @@ -5,6 +5,7 @@ import ScanCommand from './cli/scan/command'; import UploadCommand from './cli/upload/command'; import CICommand from './cli/ci/command'; import MergeCommand from './cli/merge/command'; +import TriageCommand from './cli/triage/command'; import { verbose } from './rules/lib/util'; import { AbortError, ValidationError } from './errors'; import { ExitCode } from './cli/exitCode'; @@ -41,6 +42,7 @@ yargs(process.argv.slice(2)) .command(UploadCommand) .command(CICommand) .command(MergeCommand) + .command(TriageCommand) .fail((msg, err, yargs) => { if (msg) { console.warn(yargs.help()); diff --git a/packages/scanner/src/cli/ci/command.ts b/packages/scanner/src/cli/ci/command.ts index 2986274e96..874a9b5cee 100644 --- a/packages/scanner/src/cli/ci/command.ts +++ b/packages/scanner/src/cli/ci/command.ts @@ -3,21 +3,24 @@ import { writeFile } from 'fs/promises'; import { promisify } from 'util'; import { Arguments, Argv } from 'yargs'; -import { FindingStatusListItem } from '@appland/client/dist/src'; +import { + FindingStatusListItem, + loadConfiguration as loadClientConfiguration, +} from '@appland/client/dist/src'; import { parseConfigFile } from '../../configuration/configurationProvider'; import { ValidationError } from '../../errors'; import { ScanResults } from '../../report/scanResults'; import { verbose } from '../../rules/lib/util'; import { appmapDirFromConfig } from '../appmapDirFromConfig'; -import { newFindings } from '../../findings'; +import selectFindings from '../../selectFindings'; import findingsReport from '../../report/findingsReport'; import summaryReport from '../../report/summaryReport'; import resolveAppId from '../resolveAppId'; import validateFile from '../validateFile'; import upload from '../upload'; -import { default as buildScanner } from '../scan/scanner'; +import Scanner, { default as buildScanner } from '../scan/scanner'; import CommandOptions from './options'; import scanArgs from '../scanArgs'; @@ -26,6 +29,8 @@ import reportUploadURL from '../reportUploadURL'; import fail from '../fail'; import codeVersionArgs from '../codeVersionArgs'; import { handleWorkingDirectory } from '../handleWorkingDirectory'; +import { stateFileNameArg } from '../triage/stateFileNameArg'; +import assert from 'assert'; export default { command: 'ci', @@ -33,6 +38,7 @@ export default { builder(args: Argv): Argv { scanArgs(args); codeVersionArgs(args); + stateFileNameArg(args); args.option('fail', { describe: 'exit with non-zero status if there are any new findings', @@ -59,6 +65,7 @@ export default { let { appmapDir } = options as unknown as CommandOptions; const { config, + stateFile: stateFileName, verbose: isVerbose, fail: failOption, app: appIdArg, @@ -76,6 +83,8 @@ export default { verbose(true); } + loadClientConfiguration(); + handleWorkingDirectory(directory); if (!appmapDir) { @@ -87,23 +96,27 @@ export default { ); await validateFile('directory', appmapDir!); - const appId = await resolveAppId(appIdArg, appmapDir); + const appId = await resolveAppId(appIdArg, appmapDir, true); + assert(appId); const glob = promisify(globCallback); const files = await glob(`${appmapDir}/**/*.appmap.json`); const configData = await parseConfigFile(config); - const scanner = await buildScanner(false, configData, files); + const scanner = new Scanner(configData, files); const [rawScanResults, findingStatuses]: [ScanResults, FindingStatusListItem[]] = - await Promise.all([scanner.scan(), scanner.fetchFindingStatus(appIdArg, appmapDir)]); + await Promise.all([ + scanner.scan(), + scanner.fetchFindingStatus(stateFileName, appIdArg, appmapDir), + ]); // Always report the raw data await writeFile(reportFile, JSON.stringify(rawScanResults, null, 2)); const scanResults = rawScanResults.withFindings( - newFindings(rawScanResults.findings, findingStatuses) + selectFindings(rawScanResults.findings, findingStatuses, []) ); findingsReport(scanResults.findings, scanResults.appMapMetadata); diff --git a/packages/scanner/src/cli/directoryArg.ts b/packages/scanner/src/cli/directoryArg.ts new file mode 100644 index 0000000000..587c214446 --- /dev/null +++ b/packages/scanner/src/cli/directoryArg.ts @@ -0,0 +1,9 @@ +import { Argv } from 'yargs'; + +export function directoryArg(args: Argv<{}>): void { + args.option('directory', { + describe: 'program working directory', + type: 'string', + alias: 'd', + }); +} diff --git a/packages/scanner/src/cli/findingsState.ts b/packages/scanner/src/cli/findingsState.ts new file mode 100644 index 0000000000..ec58efe0f8 --- /dev/null +++ b/packages/scanner/src/cli/findingsState.ts @@ -0,0 +1,34 @@ +import { readFile } from 'fs/promises'; +import { load } from 'js-yaml'; +import { exists } from '../../../cli/src/utils'; + +export enum FindingState { + // Finding is valid, accepted and should be fixed. + Active = 'active', + // Finding will not be acted upon at this time. The comment should clarify why this is. + Deferred = 'deferred', + // Finding is a false positive. The code is working as designed. + AsDesigned = 'as-designed', +} + +export type FindingsState = Record; + +export interface FindingStateItem { + hash_v2: string; + comment?: string; + updated_at: Date; +} + +export async function loadFindingsState(stateFileName: string): Promise { + let result: FindingsState; + if (await exists(stateFileName)) { + result = load(await readFile(stateFileName, 'utf8')) as FindingsState; + } else { + result = { + [FindingState.Active]: [], + [FindingState.Deferred]: [], + [FindingState.AsDesigned]: [], + } as FindingsState; + } + return result; +} diff --git a/packages/scanner/src/cli/merge/command.ts b/packages/scanner/src/cli/merge/command.ts index b779b731cf..995199d568 100644 --- a/packages/scanner/src/cli/merge/command.ts +++ b/packages/scanner/src/cli/merge/command.ts @@ -5,6 +5,7 @@ import { merge as mergeScannerJob } from '../../integration/appland/scannerJob/m import resolveAppId from '../resolveAppId'; import updateCommitStatus from '../updateCommitStatus'; import fail from '../fail'; +import assert from 'assert'; export default { command: 'merge ', @@ -45,7 +46,8 @@ export default { verbose(true); } - const appId = await resolveAppId(appIdArg, '.'); + const appId = await resolveAppId(appIdArg, '.', true); + assert(appId); const mergeResults = await mergeScannerJob(appId, mergeKey); console.warn(`Merged results to ${mergeResults.url}`); diff --git a/packages/scanner/src/cli/resolveAppId.ts b/packages/scanner/src/cli/resolveAppId.ts index a6cc5e8894..1423388459 100644 --- a/packages/scanner/src/cli/resolveAppId.ts +++ b/packages/scanner/src/cli/resolveAppId.ts @@ -34,16 +34,23 @@ async function resolveAppId( export default async function ( appIdArg: string | undefined, - appMapDir: string | undefined -): Promise { + appMapDir: string | undefined, + mustExist = false +): Promise { const appId = await resolveAppId(appIdArg, appMapDir); if (!appId) throw new ValidationError('App was not provided and could not be resolved'); const appExists = await new App(appId).exists(); if (!appExists) { - throw new ValidationError( - `App "${appId}" is not valid or does not exist.\nPlease fix the app name in the appmap.yml file, or override it with the --app option.` + if (mustExist) { + throw new ValidationError( + `App "${appId}" is not valid or does not exist. Please fix the app name in the appmap.yml file, or override it with the --app option.` + ); + } + console.warn( + `App "${appId}" does not exist on the AppMap Server. If this is unexpected, provide the correct app name in the appmap.yml file, or override it with the --app option.` ); + return; } return appId; diff --git a/packages/scanner/src/cli/scan/command.ts b/packages/scanner/src/cli/scan/command.ts index 4d9377eb8b..2852e543b5 100644 --- a/packages/scanner/src/cli/scan/command.ts +++ b/packages/scanner/src/cli/scan/command.ts @@ -12,18 +12,18 @@ import singleScan from './singleScan'; import watchScan from './watchScan'; import { parseConfigFile } from '../../configuration/configurationProvider'; import { handleWorkingDirectory } from '../handleWorkingDirectory'; -import interactiveScan from './interactiveScan'; +import { FindingState } from '../findingsState'; +import { loadConfiguration as loadClientConfiguration } from '@appland/client'; +import { stateFileNameArg } from '../triage/stateFileNameArg'; +import assert from 'node:assert'; export default { command: 'scan', describe: 'Scan AppMaps for code behavior findings', builder(args: Argv): Argv { scanArgs(args); + stateFileNameArg(args); - args.option('interactive', { - describe: 'scan in interactive mode', - alias: 'i', - }); args.option('appmap-file', { describe: 'single file to scan, or repeat this option to scan multiple specific files', alias: 'f', @@ -32,10 +32,9 @@ export default { describe: 'choose your IDE protocol to open AppMaps directly in your IDE.', options: ['vscode', 'x-mine', 'idea', 'pycharm'], }); - args.option('all', { - describe: 'report all findings, including duplicates of known findings', - default: false, - type: 'boolean', + args.option('finding-state', { + options: [FindingState.AsDesigned, FindingState.Deferred], + type: 'array', }); args.option('watch', { describe: 'scan code changes and report findings on changed files', @@ -50,11 +49,11 @@ export default { const { appmapFile, directory, - interactive, + stateFile: stateFileName, config: configFile, verbose: isVerbose, - all: reportAllFindings, watch, + findingState: includeFindingStates, app: appIdArg, apiKey, ide, @@ -65,51 +64,45 @@ export default { verbose(true); } - handleWorkingDirectory(directory); - if (apiKey) { process.env.APPLAND_API_KEY = apiKey; } + loadClientConfiguration(); + + handleWorkingDirectory(directory); if (appmapFile && watch) { throw new ValidationError('Use --appmap-file or --watch, but not both'); } - if (reportAllFindings && watch) { - throw new ValidationError( - `Don't use --all with --watch, because in watch mode all findings are reported` - ); - } if (appmapDir) await validateFile('directory', appmapDir); if (!appmapFile && !appmapDir) { appmapDir = (await appmapDirFromConfig()) || '.'; } - let appId = appIdArg; - if (!watch && !reportAllFindings) appId = await resolveAppId(appIdArg, appmapDir); + const appId = await resolveAppId(appIdArg, appmapDir); if (watch) { - const watchAppMapDir = appmapDir!; - return watchScan({ appId, appmapDir: watchAppMapDir, configFile }); + assert(appmapDir); + return watchScan({ + appId, + stateFileName, + appmapDir, + configFile, + includeFindingStates, + }); } else { const configuration = await parseConfigFile(configFile); - if (interactive) { - return interactiveScan({ - appmapFile, - appmapDir, - configuration, - }); - } else { - return singleScan({ - appmapFile, - appmapDir, - configuration, - reportAllFindings, - appId, - ide, - reportFile, - }); - } + return singleScan({ + appmapFile, + appmapDir, + stateFileName, + configuration, + includeFindingStates, + appId, + ide, + reportFile, + }); } }, }; diff --git a/packages/scanner/src/cli/scan/options.ts b/packages/scanner/src/cli/scan/options.ts index b40fb7b3a8..8c74eb7f4a 100644 --- a/packages/scanner/src/cli/scan/options.ts +++ b/packages/scanner/src/cli/scan/options.ts @@ -1,9 +1,9 @@ +import { FindingState } from '../findingsState'; import ScanOptions from '../scanOptions'; export default interface CommandOptions extends ScanOptions { - all: boolean; - interactive: boolean; watch: boolean; appmapFile?: string | string[]; + findingState: [FindingState.AsDesigned | FindingState.Deferred][]; ide?: string; } diff --git a/packages/scanner/src/cli/scan/scanner.ts b/packages/scanner/src/cli/scan/scanner.ts index e8a4564c88..b87358559f 100644 --- a/packages/scanner/src/cli/scan/scanner.ts +++ b/packages/scanner/src/cli/scan/scanner.ts @@ -1,4 +1,8 @@ -import { loadConfiguration, App, FindingStatusListItem } from '@appland/client/dist/src'; +import { + loadConfiguration as loadClientConfiguration, + App, + FindingStatusListItem, +} from '@appland/client/dist/src'; import { loadConfig } from '../../configuration/configurationProvider'; import Configuration from '../../configuration/types/configuration'; @@ -6,53 +10,41 @@ import Configuration from '../../configuration/types/configuration'; import resolveAppId from '../resolveAppId'; import scan from '../scan'; import { ScanResults } from '../../report/scanResults'; +import { FindingState, loadFindingsState } from '../findingsState'; +import { merge } from '../../integration/appland/scannerJob/merge'; -export interface Scanner { - scan(): Promise; - scan(skipErrors: boolean): Promise; - - fetchFindingStatus(appId?: string, appMapDir?: string): Promise; -} - -export default async function scanner( - reportAllFindings: boolean, - configuration: Configuration, - files: string[] -): Promise { - if (reportAllFindings) { - return new StandaloneScanner(configuration, files); - } else { - await loadConfiguration(); - return new ServerIntegratedScanner(configuration, files); - } -} - -abstract class ScannerBase { +export default class Scanner { constructor(public configuration: Configuration, public files: string[]) {} async scan(skipErrors = false): Promise { + loadClientConfiguration(); + const checks = await loadConfig(this.configuration); const { appMapMetadata, findings } = await scan(this.files, checks, skipErrors); return new ScanResults(this.configuration, appMapMetadata, findings, checks); } -} -class ServerIntegratedScanner extends ScannerBase implements Scanner { async fetchFindingStatus( + stateFileName: string, appIdArg?: string, appMapDir?: string ): Promise { - const appId = await resolveAppId(appIdArg, appMapDir); - return await new App(appId).listFindingStatus(); - } -} + let findingStatus: FindingStatusListItem[] = []; -class StandaloneScanner extends ScannerBase implements Scanner { - async verifyServerConfiguration(): Promise { - return true; - } - - async fetchFindingStatus(): Promise { - return []; + const appId = await resolveAppId(appIdArg, appMapDir); + if (appId) findingStatus = await new App(appId).listFindingStatus(); + + const clientFindingsState = await loadFindingsState(stateFileName); + Object.keys(clientFindingsState).forEach((state): void => { + const items = clientFindingsState[state as FindingState]; + items.forEach((item) => { + findingStatus.push({ + app_id: appId, + identity_hash: item.hash_v2, + status: state, + } as FindingStatusListItem); + }); + }); + return findingStatus; } } diff --git a/packages/scanner/src/cli/scan/singleScan.ts b/packages/scanner/src/cli/scan/singleScan.ts index a1f2936658..592f407260 100644 --- a/packages/scanner/src/cli/scan/singleScan.ts +++ b/packages/scanner/src/cli/scan/singleScan.ts @@ -1,9 +1,8 @@ import { writeFile } from 'fs/promises'; -import { default as buildScanner } from './scanner'; +import Scanner, { default as buildScanner } from './scanner'; -import { ValidationError } from '../../errors'; import Configuration from '../../configuration/types/configuration'; -import { newFindings } from '../../findings'; +import selectFindings from '../../selectFindings'; import findingsReport from '../../report/findingsReport'; import summaryReport from '../../report/summaryReport'; import { formatReport } from './formatReport'; @@ -11,20 +10,23 @@ import Telemetry from '../../telemetry'; import { sendScanResultsTelemetry } from '../../report/scanResults'; import { collectAppMapFiles } from '../../rules/lib/util'; import validateFile from '../validateFile'; +import { FindingState } from '../findingsState'; type SingleScanOptions = { appmapFile?: string | string[]; appmapDir?: string; + stateFileName: string; configuration: Configuration; - reportAllFindings: boolean; + includeFindingStates?: [FindingState.AsDesigned | FindingState.Deferred][]; reportFile: string; appId?: string; ide?: string; }; export default async function singleScan(options: SingleScanOptions): Promise { - const { appmapFile, appmapDir, configuration, reportAllFindings, appId, ide, reportFile } = - options; + let { includeFindingStates } = options; + const { appmapFile, appmapDir, configuration, appId, ide, reportFile } = options; + if (!includeFindingStates) includeFindingStates = []; Telemetry.sendEvent({ name: 'scan:started', properties: { @@ -37,30 +39,21 @@ export default async function singleScan(options: SingleScanOptions): Promise validateFile('file', file))); - const scanner = await buildScanner(reportAllFindings, configuration, files).catch( - (error: Error) => { - throw new ValidationError(error.message + '\nUse --all to perform an offline scan.'); - } - ); + const scanner = new Scanner(configuration, files); const startTime = Date.now(); const [rawScanResults, findingStatuses] = await Promise.all([ scanner.scan(skipErrors), - scanner.fetchFindingStatus(appId, appmapDir), + scanner.fetchFindingStatus(options.stateFileName, appId, appmapDir), ]); // Always report the raw data await writeFile(reportFile, formatReport(rawScanResults)); - let scanResults; - if (reportAllFindings) { - scanResults = rawScanResults; - } else { - scanResults = rawScanResults.withFindings( - newFindings(rawScanResults.findings, findingStatuses) - ); - } + const scanResults = rawScanResults.withFindings( + selectFindings(rawScanResults.findings, findingStatuses, includeFindingStates) + ); findingsReport(scanResults.findings, scanResults.appMapMetadata, ide); console.log(); diff --git a/packages/scanner/src/cli/scan/watchScan.ts b/packages/scanner/src/cli/scan/watchScan.ts index 4eeafa06e3..c3ad93d92c 100644 --- a/packages/scanner/src/cli/scan/watchScan.ts +++ b/packages/scanner/src/cli/scan/watchScan.ts @@ -6,7 +6,7 @@ import { queue } from 'async'; import { callbackify } from 'node:util'; import { formatReport } from './formatReport'; -import { default as buildScanner } from './scanner'; +import Scanner, { default as buildScanner } from './scanner'; import { parseConfigFile, TimestampedConfiguration, @@ -15,11 +15,15 @@ import Telemetry from '../../telemetry'; import EventEmitter from 'events'; import { WatchScanTelemetry } from './watchScanTelemetry'; import isAncestorPath from '../../util/isAncestorPath'; +import { FindingState } from '../findingsState'; +import selectFindings from '../../selectFindings'; export type WatchScanOptions = { appId?: string; appmapDir: string; configFile: string; + stateFileName: string; + includeFindingStates?: [FindingState.AsDesigned | FindingState.Deferred][]; }; declare module 'async' { @@ -176,14 +180,28 @@ export class Watcher { return; // report is up to date const startTime = Date.now(); - const scanner = await buildScanner(true, this.config, [appmapFile]); + const scanner = new Scanner(this.config, [appmapFile]); + + const [rawScanResults, findingStatuses] = await Promise.all([ + scanner.scan(), + scanner.fetchFindingStatus( + this.options.stateFileName, + this.options.appId, + this.options.appmapDir + ), + ]); + const scanResults = rawScanResults.withFindings( + selectFindings( + rawScanResults.findings, + findingStatuses, + this.options.includeFindingStates || [] + ) + ); - const rawScanResults = await scanner.scan(); const elapsed = Date.now() - startTime; - this.scanEventEmitter.emit('scan', { scanResults: rawScanResults, elapsed }); + this.scanEventEmitter.emit('scan', { scanResults, elapsed }); - // Always report the raw data - await writeFile(reportFile, formatReport(rawScanResults)); + await writeFile(reportFile, formatReport(scanResults)); } protected async reloadConfig(): Promise { diff --git a/packages/scanner/src/cli/scanArgs.ts b/packages/scanner/src/cli/scanArgs.ts index 8408fc82a2..6dc9b170ec 100644 --- a/packages/scanner/src/cli/scanArgs.ts +++ b/packages/scanner/src/cli/scanArgs.ts @@ -1,20 +1,18 @@ import { Argv } from 'yargs'; +import { directoryArg } from './directoryArg'; export default function (args: Argv): void { - args.option('directory', { - describe: 'program working directory', - type: 'string', - alias: 'd', - }); - args.option('appmap-dir', { - describe: 'directory to recursively inspect for AppMaps', - }); + directoryArg(args); + args.option('config', { describe: 'path to assertions config file (TypeScript or YAML, check docs for configuration format)', default: 'appmap-scanner.yml', alias: 'c', }); + args.option('appmap-dir', { + describe: 'directory to recursively inspect for AppMaps', + }); args.option('report-file', { describe: 'file name for findings report', default: 'appmap-findings.json', diff --git a/packages/scanner/src/cli/scanOptions.ts b/packages/scanner/src/cli/scanOptions.ts index 0ec534b9ec..3199ad388f 100644 --- a/packages/scanner/src/cli/scanOptions.ts +++ b/packages/scanner/src/cli/scanOptions.ts @@ -4,6 +4,7 @@ export default interface ScanOptions { directory?: string; appmapDir?: string; config: string; + stateFile: string; reportFile: string; verbose?: boolean; } diff --git a/packages/scanner/src/cli/triage/command.ts b/packages/scanner/src/cli/triage/command.ts new file mode 100644 index 0000000000..f76374c9a6 --- /dev/null +++ b/packages/scanner/src/cli/triage/command.ts @@ -0,0 +1,115 @@ +import { Arguments, Argv } from 'yargs'; +import readline from 'readline'; + +import { verbose } from '../../rules/lib/util'; + +import CommandOptions from './options'; +import { handleWorkingDirectory } from '../handleWorkingDirectory'; +import { readFile, writeFile } from 'fs/promises'; +import { dump, load } from 'js-yaml'; +import { directoryArg } from '../directoryArg'; +import { exists } from '../../../../cli/src/utils'; +import { FindingsState, FindingState, FindingStateItem, loadFindingsState } from '../findingsState'; +import { stateFileNameArg } from './stateFileNameArg'; + +export default { + command: 'triage ', + describe: 'Triage findings by assigning them to a workflow state', + builder(args: Argv): Argv { + directoryArg(args); + stateFileNameArg(args); + + args.option('comment', { + describe: 'Comment to associate with the triage action', + alias: ['c'], + }); + + args.option('state', { + describe: 'Workflow state to assign to the finding', + type: 'string', + demandOption: true, + options: ['active', 'deferred', 'as-designed'], + alias: ['s'], + }); + + args.positional('finding', { + describe: 'Identifying hash (hash_v2 digest) of the finding', + type: 'string', + array: true, + }); + + return args.strict(); + }, + async handler(options: Arguments): Promise { + let { comment } = options as unknown as CommandOptions; + const { + stateFile: stateFileName, + directory, + verbose: isVerbose, + state: stateStr, + finding: findingHashArray, + } = options as unknown as CommandOptions; + + if (isVerbose) { + verbose(true); + } + + handleWorkingDirectory(directory); + const assignedState = stateStr as FindingState; + const findingHashes = new Set(findingHashArray); + + const triagedFindings = await loadFindingsState(stateFileName); + + if (!comment) { + comment = await promptForComment(); + } + + const existingTriageItems = new Map(); + // Remove any findings that are previously triaged and will now be recategorized. + [FindingState.Active, FindingState.Deferred, FindingState.AsDesigned].forEach((state) => { + triagedFindings[state] = triagedFindings[state].filter((triagedFinding) => { + if (findingHashes.has(triagedFinding.hash_v2)) { + triagedFinding.comment = comment; + triagedFinding.updated_at = new Date(Date.now()); + existingTriageItems.set(triagedFinding.hash_v2, triagedFinding); + return false; + } + return true; + }); + }); + + findingHashes.forEach((hash) => { + let triagedFinding = existingTriageItems.get(hash); + if (!triagedFinding) { + triagedFinding = { + hash_v2: hash, + comment, + updated_at: new Date(Date.now()), + }; + } + + triagedFindings[assignedState].push(triagedFinding); + }); + + triagedFindings[assignedState] = triagedFindings[assignedState].sort((a, b) => { + let diff = b.updated_at.getTime() - a.updated_at.getTime(); + if (diff === 0) diff = a.hash_v2.localeCompare(b.hash_v2); + return diff; + }); + + await writeFile(stateFileName, dump(triagedFindings)); + }, +}; + +async function promptForComment(): Promise { + if (process.stdout.isTTY) { + const ui = readline.createInterface({ + input: process.stdin, + output: process.stdout, + }); + + return new Promise((resolve) => + ui.question('Comment (optional): ', resolve) + ); + } +} diff --git a/packages/scanner/src/cli/triage/options.ts b/packages/scanner/src/cli/triage/options.ts new file mode 100644 index 0000000000..85f9d213c7 --- /dev/null +++ b/packages/scanner/src/cli/triage/options.ts @@ -0,0 +1,8 @@ +export default interface CommandOptions { + directory?: string; + stateFile: string; + verbose?: boolean; + finding: string[]; + state: string; + comment?: string; +} diff --git a/packages/scanner/src/cli/triage/stateFileNameArg.ts b/packages/scanner/src/cli/triage/stateFileNameArg.ts new file mode 100644 index 0000000000..0a078e0da6 --- /dev/null +++ b/packages/scanner/src/cli/triage/stateFileNameArg.ts @@ -0,0 +1,9 @@ +import { Argv } from 'yargs'; + +export function stateFileNameArg(args: Argv<{}>): void { + args.option('state-file', { + describe: 'Name of the file containing the findings state', + type: 'string', + default: 'appmap-findings-state.yml', + }); +} diff --git a/packages/scanner/src/cli/upload/command.ts b/packages/scanner/src/cli/upload/command.ts index d84131db51..49ba37cce1 100644 --- a/packages/scanner/src/cli/upload/command.ts +++ b/packages/scanner/src/cli/upload/command.ts @@ -14,6 +14,7 @@ import upload from '../upload'; import codeVersionArgs from '../codeVersionArgs'; import { ValidationError } from '../../errors'; import { handleWorkingDirectory } from '../handleWorkingDirectory'; +import assert from 'assert'; export default { command: 'upload', @@ -71,6 +72,7 @@ export default { await validateFile('directory', appmapDir!); const appId = await resolveAppId(appIdArg, appmapDir); + assert(appId); const scanResults = JSON.parse((await readFile(reportFile)).toString()) as ScanResults; const uploadResponse = await upload( diff --git a/packages/scanner/src/findings.ts b/packages/scanner/src/findings.ts deleted file mode 100644 index 1038eeaefc..0000000000 --- a/packages/scanner/src/findings.ts +++ /dev/null @@ -1,17 +0,0 @@ -import { FindingStatusListItem } from '@appland/client/dist/src'; -import { Finding } from './types'; - -export function newFindings( - findings: Finding[], - findingStatuses: FindingStatusListItem[] -): Finding[] { - const statusByFindingDigest = findingStatuses.reduce((memo, findingStatus) => { - memo.set(findingStatus.identity_hash, findingStatus.status); - return memo; - }, new Map()); - - return findings.filter((finding) => { - const status = statusByFindingDigest.get(finding.hash); - return !status || status === 'new'; - }); -} diff --git a/packages/scanner/src/selectFindings.ts b/packages/scanner/src/selectFindings.ts new file mode 100644 index 0000000000..b806682343 --- /dev/null +++ b/packages/scanner/src/selectFindings.ts @@ -0,0 +1,33 @@ +import { FindingStatusListItem } from '@appland/client/dist/src'; +import { FindingState } from './cli/findingsState'; +import { Finding } from './types'; + +export default function selectFindings( + findings: Finding[], + knownFindings: FindingStatusListItem[], + includeFindingStates: [FindingState.Deferred | FindingState.AsDesigned][] +): Finding[] { + const statusByFindingDigest = knownFindings.reduce((memo, findingStatus) => { + memo.set( + findingStatus.identity_hash, + // TODO: Would like to clean this up. The client package needs to be updated to know about + // the valid finding states. + findingStatus.status as unknown as FindingState + ); + return memo; + }, new Map()); + + return findings.filter((finding) => { + let status = statusByFindingDigest.get(finding.hash_v2); + // TODO: Currently, hash_v2 and hash are both used to check known statuses. + // Once hash is fully removed, this line can be removed. + if (!status) status = statusByFindingDigest.get(finding.hash); + return ( + status === undefined || + status === FindingState.Active || + includeFindingStates.includes( + status as unknown as [FindingState.Deferred | FindingState.AsDesigned] + ) + ); + }); +} diff --git a/packages/scanner/test/cli/commands.spec.ts b/packages/scanner/test/cli/commands.spec.ts index e2ddaed9e7..9a85b77ea9 100644 --- a/packages/scanner/test/cli/commands.spec.ts +++ b/packages/scanner/test/cli/commands.spec.ts @@ -2,7 +2,7 @@ import { Arguments } from 'yargs'; import * as resolveAppId from '../../src/cli/resolveAppId'; import ScanCommand from '../../src/cli/scan/command'; import CommandOptions from '../../src/cli/scan/options'; -import * as scanner from '../../src/cli/scan/scanner'; +import * as Scanner from '../../src/cli/scan/scanner'; import * as watchScan from '../../src/cli/scan/watchScan'; import tmp from 'tmp-promise'; import { copyFile, mkdir, writeFile } from 'node:fs/promises'; @@ -13,9 +13,9 @@ jest.mock('../../src/telemetry'); const defaultArguments: Arguments = { _: [], $0: 'scan', - all: false, - interactive: false, watch: false, + stateFile: 'appmap-findings-state.yml', + findingState: [], config: 'appmap-scanner.yml', reportFile: 'appmap-findings.json', }; @@ -29,7 +29,17 @@ describe('commands', () => { }); describe('scan --watch', () => { - it('does not attempt to resolve an app ID', async () => { + let watcher: watchScan.Watcher | undefined; + + afterEach(() => { + if (watcher) { + const closer = watcher.close(); + watcher = undefined; + return closer; + } + }); + + it('resolves appId, but its absence is benign', async () => { // Prevent the watcher from running indefinitely jest.spyOn(watchScan, 'default').mockResolvedValue(); @@ -37,22 +47,11 @@ describe('commands', () => { try { await ScanCommand.handler({ ...defaultArguments, watch: true }); - } catch { - // Do nothing. - // We don't want exceptions, we just want to know if our stub was called. + } catch (e) { + expect(e).not.toBeTruthy(); } - expect(spy).not.toBeCalled(); - }); - - let watcher: watchScan.Watcher | undefined; - - afterEach(() => { - if (watcher) { - const closer = watcher.close(); - watcher = undefined; - return closer; - } + expect(spy).toBeCalled(); }); it('work correctly even if the appmap directory does not initially exist', () => @@ -70,11 +69,11 @@ describe('commands', () => { directory: tmpDir, }); - const originalScanner = scanner.default; + const OriginalScanner = Scanner.default; const scanned = new Promise((resolve) => - jest.spyOn(scanner, 'default').mockImplementation((...args) => { + jest.spyOn(Scanner, 'default').mockImplementation((...args) => { resolve(); - return originalScanner(...args); + return new OriginalScanner(...args); }) ); diff --git a/packages/scanner/test/cli/scan.spec.ts b/packages/scanner/test/cli/scan.spec.ts index e4874b538e..bc9fc5093a 100644 --- a/packages/scanner/test/cli/scan.spec.ts +++ b/packages/scanner/test/cli/scan.spec.ts @@ -24,6 +24,7 @@ delete process.env.APPLAND_API_KEY; delete process.env.APPLAND_URL; const ReportFile = 'appmap-findings.json'; +const StateFile = 'appmap-findings-state.yml'; const AppId = test.AppId; const DefaultScanConfigFilePath = join(__dirname, '..', '..', 'src', 'sampleConfig', 'default.yml'); const StandardOneShotScanOptions = { @@ -32,11 +33,11 @@ const StandardOneShotScanOptions = { ), config: DefaultScanConfigFilePath, // need to pass it explicitly reportFile: ReportFile, + stateFile: StateFile, + findingState: [], app: AppId, - all: false, - interactive: false, watch: false, -} as const; +}; function isError(error: unknown, code: string): boolean { const err = error as NodeJS.ErrnoException; @@ -58,54 +59,35 @@ function runCommand(options: CommandOptions): Promise { } describe('scan', () => { - it('errors with default options and without AppMap server API key', async () => { - delete process.env.APPLAND_API_KEY; - try { - await runCommand(StandardOneShotScanOptions); - throw new Error(`Expected this command to fail`); - } catch (err) { - expect((err as any).toString()).toMatch(/No API key available for AppMap server/); - } - }); - - async function checkScan(options: CommandOptions): Promise { - await runCommand(options); - - const scanResults = JSON.parse(readFileSync(ReportFile).toString()) as ScanResults; - expect(scanResults.summary).toBeTruthy(); - const appMapMetadata = scanResults.summary.appMapMetadata; - expect(appMapMetadata.apps).toEqual(['spring-petclinic']); - const checks = scanResults.configuration.checks; - ['http-500', 'n-plus-one-query'].forEach((rule) => - expect(checks.map((check) => check.rule)).toContain(rule) - ); - expect(Object.keys(scanResults).sort()).toEqual([ - 'appMapMetadata', - 'checks', - 'configuration', - 'findings', - 'summary', - ]); - } - - it('runs with server access disabled', async () => { - await checkScan({ ...StandardOneShotScanOptions, all: true }); + describe('without AppMap credentials', () => { + it('reports missing credentials error', async () => { + delete process.env.APPLAND_API_KEY; + delete process.env.APPMAP_API_KEY; + try { + await runCommand(StandardOneShotScanOptions); + throw new Error(`Expected this command to fail`); + } catch (err) { + expect((err as any).toString()).toMatch(/No API key available for AppMap server/); + } + }); }); - it('errors when the provided appId is not valid', async () => { - nock('http://localhost:3000').head(`/api/${AppId}`).reply(404); - - try { - await runCommand(StandardOneShotScanOptions); - throw new Error(`Expected this command to fail`); - } catch (e) { - expect((e as any).message).toMatch( - /App "myorg\/sample_app_6th_ed" is not valid or does not exist./ - ); - } + describe('when the provided appId is not valid', () => { + it('reports missing or unknown app error ', async () => { + nock('http://localhost:3000').head(`/api/${AppId}`).reply(404); + + try { + await runCommand(StandardOneShotScanOptions); + throw new Error(`Expected this command to fail`); + } catch (e) { + expect((e as any).message).toMatch( + /App "myorg\/sample_app_6th_ed" is not valid or does not exist./ + ); + } + }); }); - it('integrates server finding status with local findings', async () => { + it('integrates server finding status', async () => { const localhost = nock('http://localhost:3000'); localhost.head(`/api/${AppId}`).reply(204).persist(); localhost.get(`/api/${AppId}/finding_status`).reply(200, JSON.stringify([])); @@ -113,15 +95,36 @@ describe('scan', () => { await runCommand(StandardOneShotScanOptions); }); + it('integrates local finding status', () => pending()); + it('skips when encountering a bad file in a directory', async () => tmp.withDir( async ({ path }) => { + async function checkScan(options: CommandOptions): Promise { + await runCommand(options); + + const scanResults = JSON.parse(readFileSync(ReportFile).toString()) as ScanResults; + expect(scanResults.summary).toBeTruthy(); + const appMapMetadata = scanResults.summary.appMapMetadata; + expect(appMapMetadata.apps).toEqual(['spring-petclinic']); + const checks = scanResults.configuration.checks; + ['http-500', 'n-plus-one-query'].forEach((rule) => + expect(checks.map((check) => check.rule)).toContain(rule) + ); + expect(Object.keys(scanResults).sort()).toEqual([ + 'appMapMetadata', + 'checks', + 'configuration', + 'findings', + 'summary', + ]); + } + await copyFile(StandardOneShotScanOptions.appmapFile, join(path, 'good.appmap.json')); await writeFile(join(path, 'bad.appmap.json'), 'bad json'); const options: CommandOptions = { ...StandardOneShotScanOptions, - all: true, appmapDir: path, }; delete options.appmapFile; @@ -131,39 +134,42 @@ describe('scan', () => { { unsafeCleanup: true } )); - it('errors when no good files were found', async () => - tmp.withDir( - async ({ path }) => { - await writeFile(join(path, 'bad.appmap.json'), 'bad json'); + describe('when the only existing AppMap(s) in a directory is invalid', () => { + it('reports a processing error', async () => + tmp.withDir( + async ({ path }) => { + await writeFile(join(path, 'bad.appmap.json'), 'bad json'); + + const options: CommandOptions = { + ...StandardOneShotScanOptions, + appmapDir: path, + }; + delete options.appmapFile; + + expect.assertions(1); + return runCommand(options).catch((e: Error) => { + expect(e.message).toMatch(/Error processing/); + }); + }, + { unsafeCleanup: true } + )); + }); - const options: CommandOptions = { + describe('when an invalid AppMap file is explicitly provided', () => { + it('reports a processing error', async () => + tmp.withFile(async ({ path }) => { + await writeFile(path, 'bad json'); + const options = { ...StandardOneShotScanOptions, all: true, - appmapDir: path, + appmapFile: [path, StandardOneShotScanOptions.appmapFile], }; - delete options.appmapFile; - expect.assertions(1); return runCommand(options).catch((e: Error) => { expect(e.message).toMatch(/Error processing/); }); - }, - { unsafeCleanup: true } - )); - - it('errors when a bad file is explicitly provided', async () => - tmp.withFile(async ({ path }) => { - await writeFile(path, 'bad json'); - const options = { - ...StandardOneShotScanOptions, - all: true, - appmapFile: [path, StandardOneShotScanOptions.appmapFile], - }; - expect.assertions(1); - return runCommand(options).catch((e: Error) => { - expect(e.message).toMatch(/Error processing/); - }); - })); + })); + }); describe('watch mode', () => { let watcher: Watcher | undefined; @@ -241,6 +247,7 @@ describe('scan', () => { async function createWatcher(): Promise { watcher = new Watcher({ appId: 'no-such-app', + stateFileName: StateFile, appmapDir: tmpDir, configFile: scanConfigFilePath, });