Skip to content

api: Reject packaging errors instead of exiting #1333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Benjamin Christopher Simmonds (benibenj) wants to merge 1 commit into from
Closed

api: Reject packaging errors instead of exiting #1333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 6 additions & 12 deletions src/package.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,8 +1026,7 @@ export class ReadmeProcessor extends MarkdownProcessor {

override async onEnd(): Promise<void> {
if (this.options.readmePath && this.filesProcessed === 0) {
util.log.error(`The provided readme file (${this.options.readmePath}) could not be found.`);
process.exit(1);
throw new Error(`The provided readme file (${this.options.readmePath}) could not be found.`);
}
}
}
Expand All @@ -1050,8 +1049,7 @@ export class ChangelogProcessor extends MarkdownProcessor {

override async onEnd(): Promise<void> {
if (this.options.changelogPath && this.filesProcessed === 0) {
util.log.error(`The provided changelog file (${this.options.changelogPath}) could not be found.`);
process.exit(1);
throw new Error(`The provided changelog file (${this.options.changelogPath}) could not be found.`);
}
}
}
Expand Down Expand Up @@ -2153,8 +2151,7 @@ export async function printAndValidatePackagedFiles(files: IFile[], cwd: string,
message += `Both a ${styleText('bold', '.vscodeignore')} file and a ${styleText('bold', '"files"')} property in package.json were found. `;
message += `VSCE does not support combining both strategies. `;
message += `Either remove the ${styleText('bold', '.vscodeignore')} file or the ${styleText('bold', '"files"')} property in package.json.`;
util.log.error(message);
process.exit(1);
throw new Error(message);
}
// Throw an error if the extension uses the files property in package.json and
// the package does not include at least one file for each include pattern
Expand Down Expand Up @@ -2184,8 +2181,7 @@ export async function printAndValidatePackagedFiles(files: IFile[], cwd: string,
message += '\nRemove any include pattern which is not needed.\n';
message += `\n=> Run ${styleText('bold', 'vsce ls --tree')} to see all included files.\n`;
message += `=> Use ${styleText('bold', '--allow-unused-files-pattern')} to skip this check`;
util.log.error(message);
process.exit(1);
throw new Error(message);
}
}

Expand Down Expand Up @@ -2263,8 +2259,7 @@ export async function scanFilesForSecrets(files: IFile[], fileExclusion: FileExc
hintMessage += secretsFoundRuleNames.map(name => `--allow-package-secrets ${name}`).join(' ');
hintMessage += ` or use --allow-package-all-secrets to skip this check entirely (not recommended).`;

util.log.error(errorMessage + styleText('italic', hintMessage));
process.exit(1);
throw new Error(errorMessage + styleText('italic', hintMessage));
}

// .env file found
Expand All @@ -2283,8 +2278,7 @@ export async function scanFilesForSecrets(files: IFile[], fileExclusion: FileExc

const hintMessage = `\nTo ignore this check, you can use --allow-package-env-file (not recommended).`;

util.log.error(errorMessage + styleText('italic', hintMessage));
process.exit(1);
throw new Error(errorMessage + styleText('italic', hintMessage));
}

}
7 changes: 2 additions & 5 deletions src/secretLint.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import type {
SecretLintRuleCreator,
SecretLintRulePresetCreator
} from "@secretlint/types";
import { log } from "./util";

interface SecretLintFinding {
message: string;
Expand Down Expand Up @@ -127,8 +126,7 @@ export async function lintFiles(
})
);
} catch (error) {
log.error('Error occurred while scanning secrets (files):', error);
process.exit(1);
throw new Error('Error occurred while scanning secrets (files)', { cause: error });
}

return parseResult(results);
Expand Down Expand Up @@ -159,8 +157,7 @@ export async function lintText(
}
});
} catch (error) {
log.error('Error occurred while scanning secrets (content):', error);
process.exit(1);
throw new Error('Error occurred while scanning secrets (content)', { cause: error });
}
return parseResult([result]);
}
Expand Down
51 changes: 23 additions & 28 deletions src/test/package.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
LicenseProcessor,
printAndValidatePackagedFiles, pack
} from '../package';
import { createVSIX } from '../api';
import { ManifestPackage } from '../manifest';
import * as path from 'path';
import * as fs from 'fs';
Expand Down Expand Up @@ -91,38 +92,28 @@ function createManifest(extra: Partial<ManifestPackage> = {}): ManifestPackage {

const PROCESS_ERROR_MESSAGE = 'PROCESS ERROR';
async function testPrintAndValidatePackagedFiles(files: IFile[], cwd: string, manifest: ManifestPackage, options: IPackageOptions, errorExpected: boolean, warningExpected: boolean): Promise<void> {
const originalLogError = log.error;
const originalLogWarn = log.warn;
const originalProcessExit = process.exit;
const warns: string[] = [];
const errors: string[] = [];
let exited = false;
let errorThrown: string | undefined;
log.error = (message: string) => errors.push(message);
let errorThrown: Error | undefined;
log.warn = (message: string) => warns.push(message);
process.exit = (() => { exited = true; throw Error(PROCESS_ERROR_MESSAGE); }) as () => never;

try {
await printAndValidatePackagedFiles(files, cwd, manifest, options);
} catch (e: any) {
if (e instanceof Error && e.message !== PROCESS_ERROR_MESSAGE) {
errorThrown = e.message + '\n' + e.stack;
}
errorThrown = e;
} finally {
process.exit = originalProcessExit;
log.error = originalLogError;
log.warn = originalLogWarn;
}

// Validate that the correct number of errors and warnings were thrown
const messages = [];

if (errorExpected !== !!errors.length) {
if (errors.length) {
messages.push(...errors);
} else {
messages.push('Expected an error');
}
if (errorExpected !== !!errorThrown) {
messages.push(errorThrown ? `Unexpected error: ${errorThrown.stack}` : 'Expected an error');
}

if (warningExpected !== !!warns.length) {
Expand All @@ -133,20 +124,16 @@ async function testPrintAndValidatePackagedFiles(files: IFile[], cwd: string, ma
}
}

if (!errorExpected && exited) {
if (exited) {
messages.push('Process exited');
}

if (!errorExpected && !!errorThrown && !exited) {
messages.push('Error thrown: ' + errorThrown);
}

if (messages.length) {
throw new Error(messages.join('\n'));
}
}

async function processExitExpected(fn: () => Promise<any>, errorMessage: string): Promise<void> {
async function rejectionExpected(fn: () => Promise<any>, errorMessage: string): Promise<void> {
const originalExit = process.exit;
let exitCalled = false;

Expand All @@ -156,10 +143,8 @@ async function processExitExpected(fn: () => Promise<any>, errorMessage: string)
throw new Error('Process exit was called');
}) as any;

await fn();
assert.fail(errorMessage);
} catch (error) {
assert.ok(exitCalled, errorMessage);
await assert.rejects(fn, errorMessage);
assert.strictEqual(exitCalled, false, 'Process exit was called');
} finally {
process.exit = originalExit;
}
Expand Down Expand Up @@ -286,6 +271,16 @@ describe('collect', function () {
await testPrintAndValidatePackagedFiles(files, cwd, manifestCopy, {}, false, false);
});

it('createVSIX rejects instead of exiting the process when package validation fails', async () => {
const cwd = fixture('manifestFiles');
const ignoreFile = path.join(cwd, 'README.md');

await rejectionExpected(
() => createVSIX({ cwd, ignoreFile, packagePath: getVisxOutputPath() }),
'Expected createVSIX to reject when both an ignore file and package.json files are used'
);
});

it('should ignore devDependencies', () => {
const cwd = fixture('devDependencies');
return readManifest(cwd)
Expand Down Expand Up @@ -399,7 +394,7 @@ describe('collect', function () {

it('should not package .env file', async function () {
const cwd = fixture('env');
await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath() }), 'Expected package to throw: .env file should not be packaged');
await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath() }), 'Expected package to throw: .env file should not be packaged');
});

it('allow packaging .env file with --allow-package-env-file', async function () {
Expand All @@ -410,7 +405,7 @@ describe('collect', function () {
it('should not package file which has a private key', async function () {
const cwd = fixture('secrets');
const ignoreFile = path.join(cwd, 'secret1Ignore');
await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: file which has a private key should not be packaged');
await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: file which has a private key should not be packaged');
});

it('should scan files when CPU information is unavailable', async function () {
Expand All @@ -422,7 +417,7 @@ describe('collect', function () {
try {
const cwd = fixture('secrets');
const ignoreFile = path.join(cwd, 'secret1Ignore');
await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected secret scanning to run without CPU information');
await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected secret scanning to run without CPU information');
} finally {
Object.defineProperty(os, 'cpus', cpusDescriptor);
}
Expand Down Expand Up @@ -455,7 +450,7 @@ describe('collect', function () {
it('should not package npm token', async function () {
const cwd = fixture('secrets');
const ignoreFile = path.join(cwd, 'secret2Ignore');
await processExitExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: should not package npm token');
await rejectionExpected(async () => await pack({ cwd, packagePath: getVisxOutputPath(), ignoreFile }), 'Expected package to throw: should not package npm token');
});

it('npm token false positive 1', async function () {
Expand Down
Loading