From 926a553a760ed40bdfa277fa7c75639088591ad8 Mon Sep 17 00:00:00 2001 From: Rhys Arkins Date: Fri, 3 Jul 2020 16:52:16 +0200 Subject: [PATCH] fix: sanitize commitMessage, prTitle, branchName In preparation for secrets PR --- lib/constants/error-messages.ts | 1 + lib/workers/branch/commit.ts | 9 +++++++++ lib/workers/repository/error.spec.ts | 2 ++ lib/workers/repository/error.ts | 9 +++++++++ lib/workers/repository/updates/generate.ts | 10 ++++++++++ 5 files changed, 31 insertions(+) diff --git a/lib/constants/error-messages.ts b/lib/constants/error-messages.ts index fa4f6e4e307b9b..3064bab6d2204a 100644 --- a/lib/constants/error-messages.ts +++ b/lib/constants/error-messages.ts @@ -12,6 +12,7 @@ export const PLATFORM_RATE_LIMIT_EXCEEDED = 'rate-limit-exceeded'; // Config Error export const CONFIG_VALIDATION = 'config-validation'; +export const CONFIG_SECRETS_EXPOSED = 'config-secrets-exposed'; // Repository Error export const REPOSITORY_ACCESS_FORBIDDEN = 'forbidden'; diff --git a/lib/workers/branch/commit.ts b/lib/workers/branch/commit.ts index af166f625e0269..4bf55c818406d6 100644 --- a/lib/workers/branch/commit.ts +++ b/lib/workers/branch/commit.ts @@ -1,7 +1,9 @@ import is from '@sindresorhus/is'; import minimatch from 'minimatch'; +import { CONFIG_SECRETS_EXPOSED } from '../../constants/error-messages'; import { logger } from '../../logger'; import { platform } from '../../platform'; +import { sanitize } from '../../util/sanitize'; import { BranchConfig } from '../common'; export async function commitFilesToBranch( @@ -33,6 +35,13 @@ export async function commitFilesToBranch( logger.info('DRY-RUN: Would commit files to branch ' + config.branchName); return null; } + // istanbul ignore if + if ( + config.branchName !== sanitize(config.branchName) || + config.commitMessage !== sanitize(config.commitMessage) + ) { + throw new Error(CONFIG_SECRETS_EXPOSED); + } // API will know whether to create new branch or not return platform.commitFiles({ branchName: config.branchName, diff --git a/lib/workers/repository/error.spec.ts b/lib/workers/repository/error.spec.ts index 29be585ad698ae..29008a48b71e1f 100644 --- a/lib/workers/repository/error.spec.ts +++ b/lib/workers/repository/error.spec.ts @@ -1,5 +1,6 @@ import { RenovateConfig, getConfig } from '../../../test/util'; import { + CONFIG_SECRETS_EXPOSED, CONFIG_VALIDATION, EXTERNAL_HOST_ERROR, MANAGER_LOCKFILE_ERROR, @@ -46,6 +47,7 @@ describe('workers/repository/error', () => { REPOSITORY_CHANGED, REPOSITORY_FORKED, MANAGER_NO_PACKAGE_FILES, + CONFIG_SECRETS_EXPOSED, CONFIG_VALIDATION, REPOSITORY_ARCHIVED, REPOSITORY_MIRRORED, diff --git a/lib/workers/repository/error.ts b/lib/workers/repository/error.ts index 3014762e6f9140..469f4248facfee 100644 --- a/lib/workers/repository/error.ts +++ b/lib/workers/repository/error.ts @@ -1,6 +1,7 @@ import { RenovateConfig } from '../../config'; import { + CONFIG_SECRETS_EXPOSED, CONFIG_VALIDATION, EXTERNAL_HOST_ERROR, MANAGER_LOCKFILE_ERROR, @@ -106,6 +107,14 @@ export default async function handleError( await raiseConfigWarningIssue(config, err); return err.message; } + if (err.message === CONFIG_SECRETS_EXPOSED) { + delete config.branchList; // eslint-disable-line no-param-reassign + logger.warn( + { error: err }, + 'Repository aborted due to potential secrets exposure' + ); + return err.message; + } if (err instanceof ExternalHostError) { logger.warn( { hostType: err.hostType, lookupName: err.lookupName, err: err.err }, diff --git a/lib/workers/repository/updates/generate.ts b/lib/workers/repository/updates/generate.ts index c37f56c146b69f..134141e67b8f90 100644 --- a/lib/workers/repository/updates/generate.ts +++ b/lib/workers/repository/updates/generate.ts @@ -2,7 +2,9 @@ import { DateTime } from 'luxon'; import mdTable from 'markdown-table'; import semver from 'semver'; import { mergeChildConfig } from '../../../config'; +import { CONFIG_SECRETS_EXPOSED } from '../../../constants/error-messages'; import { logger } from '../../../logger'; +import { sanitize } from '../../../util/sanitize'; import * as template from '../../../util/template'; import { BranchConfig, BranchUpgradeConfig } from '../../common'; @@ -194,6 +196,10 @@ export function generateBranchConfig( ); upgrade.commitMessage = template.compile(upgrade.commitMessage, upgrade); upgrade.commitMessage = template.compile(upgrade.commitMessage, upgrade); + // istanbul ignore if + if (upgrade.commitMessage !== sanitize(upgrade.commitMessage)) { + throw new Error(CONFIG_SECRETS_EXPOSED); + } upgrade.commitMessage = upgrade.commitMessage.trim(); // Trim exterior whitespace upgrade.commitMessage = upgrade.commitMessage.replace(/\s+/g, ' '); // Trim extra whitespace inside string upgrade.commitMessage = upgrade.commitMessage.replace( @@ -220,6 +226,10 @@ export function generateBranchConfig( .compile(upgrade.prTitle, upgrade) .trim() .replace(/\s+/g, ' '); + // istanbul ignore if + if (upgrade.prTitle !== sanitize(upgrade.prTitle)) { + throw new Error(CONFIG_SECRETS_EXPOSED); + } if (upgrade.toLowerCase) { upgrade.prTitle = upgrade.prTitle.toLowerCase(); }