From ffbb23650cf42a5aa67dc08dc34486ba3871ba14 Mon Sep 17 00:00:00 2001 From: James Spoor Date: Thu, 16 Sep 2021 14:44:34 +0100 Subject: [PATCH] fix: dot-snyk files respected --- src/bundles.ts | 24 ++++++------------------ src/constants.ts | 2 +- src/files.ts | 33 +++++++++++++++------------------ tests/analysis.spec.ts | 4 ++-- tests/constants/sample.ts | 7 ++----- tests/files.spec.ts | 26 ++++++++------------------ 6 files changed, 34 insertions(+), 62 deletions(-) diff --git a/src/bundles.ts b/src/bundles.ts index 4888f606..c5233f20 100644 --- a/src/bundles.ts +++ b/src/bundles.ts @@ -12,7 +12,6 @@ import { collectIgnoreRules, determineBaseDir, collectBundleFiles, - parseDotSnykExcludes, } from './files'; import { @@ -28,12 +27,7 @@ import { getFilters, } from './http'; -import { - DOTSNYK_FILENAME, - MAX_PAYLOAD, - MAX_UPLOAD_ATTEMPTS, - UPLOAD_CONCURRENCY -} from './constants'; +import { MAX_PAYLOAD, MAX_UPLOAD_ATTEMPTS, UPLOAD_CONCURRENCY } from './constants'; import emitter from './emitter'; type BundleErrorCodes = CreateBundleErrorCodes | CheckBundleErrorCodes | ExtendBundleErrorCodes; @@ -122,11 +116,9 @@ export async function uploadRemoteBundle({ for (const bucketFiles of composeFilePayloads(options.files, maxPayload)) { tasks.push(bucketFiles); } - const results = await pMap( - tasks, - async (task: FileInfo[]) => await uploadFileChunks(task), - { concurrency: UPLOAD_CONCURRENCY } - ); + const results = await pMap(tasks, async (task: FileInfo[]) => await uploadFileChunks(task), { + concurrency: UPLOAD_CONCURRENCY, + }); // Returning false if at least one result is false return results.every(r => !!r); } @@ -179,7 +171,7 @@ export async function remoteBundleFactory(options: RemoteBundleFactoryOptions): if (response.type === 'error') { throw response.error; } - + remoteBundle = await fullfillRemoteBundle({ ...baseOptions, remoteBundle: response.value }); if (remoteBundle.missingFiles.length) { throw new Error(`Failed to upload # files: ${remoteBundle.missingFiles.length}`); @@ -226,17 +218,13 @@ export interface FileBundle extends RemoteBundle { export async function createBundleFromFolders(options: CreateBundleFromFoldersOptions): Promise { const baseDir = determineBaseDir(options.paths); - const [supportedFiles, excludesFromIgnoreFiles, excludesFromDotSnyk] = await Promise.all([ + const [supportedFiles, fileIgnores] = await Promise.all([ // Fetch supporte files to save network traffic getSupportedFiles(options.baseURL, options.source), // Scan for custom ignore rules collectIgnoreRules(options.paths, options.symlinksEnabled, options.defaultFileIgnores), - // Get exclusions from .snyk file - parseDotSnykExcludes(`${baseDir}/${DOTSNYK_FILENAME}`), ]); - const fileIgnores = [...excludesFromIgnoreFiles, ...excludesFromDotSnyk]; - emitter.scanFilesProgress(0); const bundleFiles = []; let totalFiles = 0; diff --git a/src/constants.ts b/src/constants.ts index cce6ddc6..81c0965a 100644 --- a/src/constants.ts +++ b/src/constants.ts @@ -17,7 +17,7 @@ export const REQUEST_RETRY_DELAY = 30 * 1000; // 30 seconds delay between retrie export const IGNORES_DEFAULT = [`**/${GIT_FILENAME}/**`]; -export const IGNORE_FILES_NAMES = [GITIGNORE_FILENAME, DCIGNORE_FILENAME]; +export const IGNORE_FILES_NAMES = [GITIGNORE_FILENAME, DCIGNORE_FILENAME, DOTSNYK_FILENAME]; export const DCIGNORE_DRAFTS = { custom: CustomDCIgnore, diff --git a/src/files.ts b/src/files.ts index b64f7d37..b76d6905 100644 --- a/src/files.ts +++ b/src/files.ts @@ -7,7 +7,15 @@ import { parse as parseYaml } from 'yaml'; import union from 'lodash.union'; import util from 'util'; import { Cache } from './cache'; -import { HASH_ALGORITHM, ENCODE_TYPE, MAX_PAYLOAD, IGNORES_DEFAULT, IGNORE_FILES_NAMES, CACHE_KEY } from './constants'; +import { + HASH_ALGORITHM, + ENCODE_TYPE, + MAX_PAYLOAD, + IGNORES_DEFAULT, + IGNORE_FILES_NAMES, + CACHE_KEY, + DOTSNYK_FILENAME, +} from './constants'; import { SupportedFiles, FileInfo } from './interfaces/files.interface'; @@ -98,7 +106,13 @@ export function parseFileIgnores(path: string): string[] { const dirname = nodePath.dirname(path); try { const f = fs.readFileSync(path, { encoding: 'utf8' }); + if (path.includes(DOTSNYK_FILENAME)) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + const parsed: { exclude: { code: string[]; global: string[] } } = parseYaml(f); + const concatIgnorePath = (p: string) => `${nodePath.dirname(path)}/${p}`; + return [...parsed.exclude.code.map(concatIgnorePath), ...parsed.exclude.global.map(concatIgnorePath)]; + } rules = f .split('\n') .map(l => l.trim()) @@ -461,20 +475,3 @@ export function* composeFilePayloads(files: FileInfo[], bucketSize = MAX_PAYLOAD export function isMatch(filePath: string, rules: string[]): boolean { return !!multimatch([filePath], rules, { ...multiMatchOptions, matchBase: false }).length; } - -export function parseDotSnykExcludes(pathToDotSnykFile: string): string[] { - try { - const dotSnykFile = fs.readFileSync(pathToDotSnykFile, 'utf-8'); - // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment - const parsed: { exclude: { code: string[]; global: string[] } } = parseYaml(dotSnykFile); - - const concatIgnorePath = (path: string) => `${nodePath.dirname(pathToDotSnykFile)}/${path}`; - return [...parsed.exclude.code.map(concatIgnorePath), ...parsed.exclude.global.map(concatIgnorePath)]; - } catch (err) { - // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access - if (err.code === 'EACCES' || err.code === 'EPERM') { - console.info(`${pathToDotSnykFile} is not accessible.`); - } - return []; - } -} diff --git a/tests/analysis.spec.ts b/tests/analysis.spec.ts index 27146932..673045b8 100644 --- a/tests/analysis.spec.ts +++ b/tests/analysis.spec.ts @@ -135,7 +135,7 @@ describe('Functional test of analysis', () => { const shouldNotBeInBundle = [ '/.eslintrc.json', // <= no linters on backend 'main.js', // <= over maxPayload (23098 > 1000) - ] + ]; // Force uploading files one more time uploaded = await uploadRemoteBundle({ baseURL, @@ -222,7 +222,7 @@ describe('Functional test of analysis', () => { expect(extendedBundle).toBeTruthy(); if (!extendedBundle) return; // TS trick - expect(extendedBundle.analysisResults.sarif.runs[0].tool.driver.rules?.length).toEqual(4); + expect(extendedBundle.analysisResults.sarif.runs[0].tool.driver.rules?.length).toEqual(5); expect(extendedBundle.analysisResults.sarif.runs[0].results?.length).toEqual(10); const getRes = (path: string) => extendedBundle!.analysisResults.sarif.runs[0].results!.find( diff --git a/tests/constants/sample.ts b/tests/constants/sample.ts index a58e30c5..4045f395 100644 --- a/tests/constants/sample.ts +++ b/tests/constants/sample.ts @@ -10,11 +10,6 @@ export const supportedFiles = { configFiles: ['.eslintrc.json', '.dcignore'], // <= we are not running linters in the backend anymore }; -export const bundlefileExcludes = [ - `${sampleProjectPath}/exclude/excluded-file.js`, - `${sampleProjectPath}/exclude/excluded-folder/**`, -]; - export const bundleFileIgnores = [ '**/.git/**', `${sampleProjectPath}/**/mode_nodules/**`, @@ -26,6 +21,8 @@ export const bundleFileIgnores = [ `!${sampleProjectPath}/**/not/ignored`, `${sampleProjectPath}/**/*.jsx/**`, `${sampleProjectPath}/**/*.jsx`, + `${sampleProjectPath}/exclude/excluded-file.js`, + `${sampleProjectPath}/exclude/excluded-folder/**`, ]; export const bundleFilePaths = [ diff --git a/tests/files.spec.ts b/tests/files.spec.ts index 0ccb1c2f..605e5768 100644 --- a/tests/files.spec.ts +++ b/tests/files.spec.ts @@ -1,6 +1,5 @@ import * as fs from 'fs'; import * as nodePath from 'path'; -import { DOTSNYK_FILENAME } from '../src/constants'; import { collectIgnoreRules, @@ -9,22 +8,18 @@ import { composeFilePayloads, parseFileIgnores, getFileInfo, - parseDotSnykExcludes, } from '../src/files'; -import { - sampleProjectPath, - supportedFiles, - bundleFiles, - bundleFilesFull, - bundleFileIgnores, - bundlefileExcludes, -} from './constants/sample'; +import { sampleProjectPath, supportedFiles, bundleFiles, bundleFilesFull, bundleFileIgnores } from './constants/sample'; describe('files', () => { it('parse dc ignore file', () => { const patterns = parseFileIgnores(`${sampleProjectPath}/.dcignore`); - expect(patterns).toEqual(bundleFileIgnores.slice(1)); + expect(patterns).toEqual(bundleFileIgnores.slice(1, 10)); + }); + it('parse dot snyk file', () => { + const patterns = parseFileIgnores(`${sampleProjectPath}/.snyk`); + expect(patterns).toEqual(bundleFileIgnores.slice(10)); }); it('collect ignore rules', async () => { @@ -32,18 +27,13 @@ describe('files', () => { expect(ignoreRules).toEqual(bundleFileIgnores); }); - it('should parse .snyk file', async () => { - const patterns = await parseDotSnykExcludes(`${sampleProjectPath}/${DOTSNYK_FILENAME}`); - expect(patterns).toEqual(bundlefileExcludes); - }); - it('collect bundle files', async () => { // TODO: We should introduce some performance test using a big enough repo to avoid flaky results const collector = collectBundleFiles({ baseDir: sampleProjectPath, paths: [sampleProjectPath], supportedFiles, - fileIgnores: [...bundleFileIgnores, ...bundlefileExcludes], + fileIgnores: bundleFileIgnores, }); const files = []; for await (const f of collector) { @@ -79,7 +69,7 @@ describe('files', () => { baseDir: sampleProjectPath, paths: [sampleProjectPath], supportedFiles, - fileIgnores: [...bundleFileIgnores, ...bundlefileExcludes], + fileIgnores: bundleFileIgnores, maxPayload: 500, }); const smallFiles = [];