Merge pull request #1145 from microsoft/benibenj/cultural-elephant

Allow packaging .env and secrets using command line flags

Author: benibenj

src/main.ts

@@ -124,6 +124,9 @@ module.exports = function (argv: string[]): void {
 		.option('--allow-star-activation', 'Allow using * in activation events')
 		.option('--allow-missing-repository', 'Allow missing a repository URL in package.json')
 		.option('--allow-unused-files-pattern', 'Allow include patterns for the files field in package.json that does not match any file')
+		.option('--allow-package-secrets <secrets...>', 'Allow packaging specific secrets. The names of the secrets can be found in the error message ([SECRET_NAME]).')
+		.option('--allow-package-all-secrets', 'Allow to package all kinds of secrets')
+		.option('--allow-package-env-file', 'Allow packaging .env files')
 		.option('--skip-license', 'Allow packaging without license file')
 		.option('--sign-tool <path>', 'Path to the VSIX signing tool. Will be invoked with two arguments: `SIGNTOOL <path/to/extension.signature.manifest> <path/to/extension.signature.p7s>`.')
 		.option('--follow-symlinks', 'Recurse into symlinked directories instead of treating them as files')
@@ -153,6 +156,9 @@ module.exports = function (argv: string[]): void {
 					allowStarActivation,
 					allowMissingRepository,
 					allowUnusedFilesPattern,
+					allowPackageSecrets,
+					allowPackageAllSecrets,
+					allowPackageEnvFile,
 					skipLicense,
 					signTool,
 					followSymlinks,
@@ -183,6 +189,9 @@ module.exports = function (argv: string[]): void {
 						allowStarActivation,
 						allowMissingRepository,
 						allowUnusedFilesPattern,
+						allowPackageSecrets,
+						allowPackageAllSecrets,
+						allowPackageEnvFile,
 						skipLicense,
 						signTool,
 						followSymlinks,
@@ -230,6 +239,9 @@ module.exports = function (argv: string[]): void {
 		.addOption(new Option('--noVerify', 'Allow all proposed APIs (deprecated: use --allow-all-proposed-apis instead)').hideHelp(true))
 		.option('--allow-proposed-apis <apis...>', 'Allow specific proposed APIs')
 		.option('--allow-all-proposed-apis', 'Allow all proposed APIs')
+		.option('--allow-package-secrets <secrets...>', 'Allow packaging specific secrets. The names of the secrets can be found in the error message ([SECRET_NAME]).')
+		.option('--allow-package-all-secrets', 'Allow to package all kinds of secrets')
+		.option('--allow-package-env-file', 'Allow packaging .env files')
 		.option('--ignoreFile <path>', 'Indicate alternative .vscodeignore')
 		// default must remain undefined for dependencies or we will fail to load defaults from package.json
 		.option('--dependencies', 'Enable dependency detection via npm or yarn', undefined)
@@ -267,6 +279,9 @@ module.exports = function (argv: string[]): void {
 					noVerify,
 					allowProposedApis,
 					allowAllProposedApis,
+					allowPackageSecrets,
+					allowPackageAllSecrets,
+					allowPackageEnvFile,
 					ignoreFile,
 					dependencies,
 					preRelease,
@@ -303,6 +318,9 @@ module.exports = function (argv: string[]): void {
 						noVerify: noVerify || !verify,
 						allowProposedApis,
 						allowAllProposedApis,
+						allowPackageSecrets,
+						allowPackageAllSecrets,
+						allowPackageEnvFile,
 						ignoreFile,
 						dependencies,
 						preRelease,

src/package.ts

@@ -27,7 +27,7 @@ import * as GitHost from 'hosted-git-info';
 import parseSemver from 'parse-semver';
 import * as jsonc from 'jsonc-parser';
 import * as vsceSign from '@vscode/vsce-sign';
-import { lintFiles, lintText, prettyPrintLintResult } from './secretLint';
+import { getRuleNameFromRuleId, lintFiles, lintText, prettyPrintLintResult } from './secretLint';
 
 const MinimatchOptions: minimatch.IOptions = { dot: true };
 
@@ -162,6 +162,10 @@ export interface IPackageOptions {
 	readonly allowStarActivation?: boolean;
 	readonly allowMissingRepository?: boolean;
 	readonly allowUnusedFilesPattern?: boolean;
+	readonly allowPackageSecrets?: string[];
+	readonly allowPackageAllSecrets?: boolean;
+	readonly allowPackageEnvFile?: boolean;
+
 	readonly skipLicense?: boolean;
 
 	readonly signTool?: string;
@@ -2108,10 +2112,17 @@ export async function printAndValidatePackagedFiles(files: IFile[], cwd: string,
 	message += '\n';
 	util.log.info(message);
 
-	await scanFilesForSecrets(files);
+	const fileExclusion = hasIgnoreFile ? FileExclusionType.VSCodeIgnore : manifest.files ? FileExclusionType.PackageFiles : FileExclusionType.None;
+	await scanFilesForSecrets(files, fileExclusion, options);
+}
+
+enum FileExclusionType {
+	None = 'none',
+	VSCodeIgnore = 'vscodeIgnore',
+	PackageFiles = 'package.files'
 }
 
-export async function scanFilesForSecrets(files: IFile[]): Promise<void> {
+export async function scanFilesForSecrets(files: IFile[], fileExclusion: FileExclusionType, options: IPackageOptions): Promise<void> {
 	const onDiskFiles: ILocalFile[] = files.filter(file => !isInMemoryFile(file)) as ILocalFile[];
 	const inMemoryFiles: IInMemoryFile[] = files.filter(file => isInMemoryFile(file)) as IInMemoryFile[];
 
@@ -2121,25 +2132,49 @@ export async function scanFilesForSecrets(files: IFile[]): Promise<void> {
 	);
 
 	const secretsFound = [...inMemoryResults, onDiskResult].filter(result => !result.ok).flatMap(result => result.results);
-	if (secretsFound.length === 0) {
-		return;
-	}
 
 	// secrets found
-	const noneDotEnvSecretsFound = secretsFound.filter(result => result.ruleId !== '@secretlint/secretlint-rule-no-dotenv');
+	const noneDotEnvSecretsFound = secretsFound.filter(result =>
+		result.ruleId &&
+		result.ruleId !== '@secretlint/secretlint-rule-no-dotenv' &&
+		!options.allowPackageAllSecrets &&
+		!options.allowPackageSecrets?.includes(getRuleNameFromRuleId(result.ruleId))
+	);
 	if (noneDotEnvSecretsFound.length > 0) {
-		let errorOutput = '';
-		for (const secret of noneDotEnvSecretsFound) {
-			errorOutput += '\n' + prettyPrintLintResult(secret);
-		}
-		util.log.error(`Secrets have been detected in the files which are being packaged:\n\n${errorOutput}`);
+		const uniqueSecretIds = new Set<string>(noneDotEnvSecretsFound.map(result => result.ruleId!));
+		const secretsFoundRuleNames = Array.from(uniqueSecretIds).map(getRuleNameFromRuleId);
+
+		let errorMessage = `${chalk.bold('Potential security issue detected:')}`;
+		errorMessage += ` Your extension package contains sensitive information that should not be published.`
+		errorMessage += ` Please remove these secrets before packaging.`;
+		errorMessage += `\n` + noneDotEnvSecretsFound.map(prettyPrintLintResult).join('\n');
+
+		let hintMessage = `\nIn case of a false positives, you can allowlist secrets with `;
+		hintMessage += secretsFoundRuleNames.map(name => `--allow-package-secrets ${name}`).join(' ');
+		hintMessage += ` or use --allow-package-all-secrets to skip this check (not recommended).`;
+
+		util.log.error(errorMessage + chalk.italic(hintMessage));
+		process.exit(1);
 	}
 
 	// .env file found
 	const allRuleIds = new Set(secretsFound.map(result => result.ruleId).filter(Boolean));
-	if (allRuleIds.has('@secretlint/secretlint-rule-no-dotenv')) {
-		util.log.error(`${chalk.bold.red('.env')} files should not be packaged. Ignore them in your ${chalk.bold('.vscodeignore')} file or exclude them from the package.json ${chalk.bold('files')} property.`);
+	if (!options.allowPackageEnvFile && allRuleIds.has('@secretlint/secretlint-rule-no-dotenv')) {
+		let errorMessage = `${chalk.bold.red('.env')} files should not be packaged.`;
+
+		switch (fileExclusion) {
+			case FileExclusionType.None:
+				errorMessage += ` Ignore the file in your ${chalk.bold('.vscodeignore')} or exclude it from the package.json ${chalk.bold('files')} property.`; break;
+			case FileExclusionType.VSCodeIgnore:
+				errorMessage += ` Ignore the file in your ${chalk.bold('.vscodeignore')}.`; break;
+			case FileExclusionType.PackageFiles:
+				errorMessage += ` Do not include the file in your package.json ${chalk.bold('files')} property.`; break;
+		}
+
+		const hintMessage = `\nTo ignore this check, you can use --allow-package-env-file (not recommended).`;
+
+		util.log.error(errorMessage + chalk.italic(hintMessage));
+		process.exit(1);
 	}
 
-	process.exit(1);
 }

src/publish.ts

@@ -74,6 +74,9 @@ export interface IPublishOptions {
 	readonly noVerify?: boolean;
 	readonly allowProposedApis?: string[];
 	readonly allowAllProposedApis?: boolean;
+	readonly allowPackageSecrets?: string[];
+	readonly allowPackageAllSecrets?: boolean;
+	readonly allowPackageEnvFile?: boolean;
 	readonly dependencies?: boolean;
 	readonly preRelease?: boolean;
 	readonly allowStarActivation?: boolean;

src/secretLint.ts

@@ -74,6 +74,11 @@ function parseResult(result: SecretLintEngineResult): SecretLintResult {
 	return { ok: result.ok, results };
 }
 
+export function getRuleNameFromRuleId(ruleId: string): string {
+	const parts = ruleId.split('-rule-');
+	return parts[parts.length - 1];
+}
+
 export function prettyPrintLintResult(result: Result): string {
 	if (!result.message.text) {
 		return JSON.stringify(result);
@@ -82,7 +87,9 @@ export function prettyPrintLintResult(result: Result): string {
 	const text = result.message.text;
 	const titleColor = result.level === undefined || result.level === Level.Error ? chalk.bold.red : chalk.bold.yellow;
 	const title = text.length > 54 ? text.slice(0, 50) + '...' : text;
-	let output = `\t${titleColor(title)}\n`;
+	const ruleName = result.ruleId ? getRuleNameFromRuleId(result.ruleId) : 'unknown';
+
+	let output = `\t${titleColor(title)} [${ruleName}]\n`;
 
 	if (result.locations) {
 		result.locations.forEach(location => {

src/test/package.test.ts

@@ -168,6 +168,19 @@ async function processExitExpected(fn: () => Promise<any>, errorMessage: string)
 describe('collect', function () {
 	this.timeout(60000);
 
+	let testDir: tmp.DirResult;
+	before(() => {
+		testDir = tmp.dirSync({ unsafeCleanup: true, tmpdir: fixture('') });
+	});
+	after(() => {
+		testDir.removeCallback();
+	});
+
+	function getVisxOutputPath() {
+		const randomValue = Math.random().toString(36).substring(2, 15);
+		return path.join(testDir.name, `extension-${randomValue}.vsix`);
+	}
+
 	it('should catch all files', () => {
 		const cwd = fixture('uuid');
 
@@ -373,12 +386,27 @@ describe('collect', function () {
 
 	it('should not package .env file', async function () {
 		const cwd = fixture('env');
-		await processExitExpected(() => pack({ cwd }), 'Expected package to throw: .env file should not be packaged');
+		await processExitExpected(() => 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 () {
+		const cwd = fixture('env');
+		await pack({ cwd, allowPackageEnvFile: true, packagePath: getVisxOutputPath() });
 	});
 
 	it('should not package file which has a private key', async function () {
 		const cwd = fixture('secret');
-		await processExitExpected(() => pack({ cwd }), 'Expected package to throw: file which has a private key should not be packaged');
+		await processExitExpected(() => pack({ cwd, packagePath: getVisxOutputPath() }), 'Expected package to throw: file which has a private key should not be packaged');
+	});
+
+	it('allow packaging file which has a private key with --allow-package-secrets', async function () {
+		const cwd = fixture('secret');
+		await pack({ cwd, allowPackageSecrets: ['privatekey'], packagePath: getVisxOutputPath() });
+	});
+
+	it('allow packaging file which has a private key with --allow-package-all-secrets', async function () {
+		const cwd = fixture('secret');
+		await pack({ cwd, allowPackageAllSecrets: true, packagePath: getVisxOutputPath() });
 	});
 });