From 95d54baf83cb1a97b229026f5b89f4812493e715 Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Sat, 7 Sep 2019 14:51:00 +0200
Subject: [PATCH] feat: centralized sanitation (#4446)
Adds a sanitize function to host-rules that redacts tokens and passwords from issues and PRs.
Closes #4444
---
lib/platform/azure/index.ts | 8 +++---
lib/platform/bitbucket-server/index.ts | 10 ++++---
lib/platform/bitbucket/index.ts | 15 +++++++----
lib/platform/github/index.ts | 16 +++++++-----
lib/platform/gitlab/index.ts | 10 ++++---
lib/util/host-rules.ts | 26 +++++++++++++++++++
lib/workers/branch/index.js | 5 ++++
test/platform/azure/index.spec.ts | 1 +
test/platform/bitbucket-server/index.spec.ts | 1 +
test/platform/bitbucket/index.spec.ts | 1 +
test/platform/github/index.spec.ts | 1 +
test/platform/gitlab/index.spec.ts | 1 +
.../__snapshots__/host-rules.spec.ts.snap | 2 ++
test/util/host-rules.spec.ts | 23 +++++++++++++++-
14 files changed, 97 insertions(+), 23 deletions(-)
diff --git a/lib/platform/azure/index.ts b/lib/platform/azure/index.ts
index ceb2704263..94a7bc142c 100644
--- a/lib/platform/azure/index.ts
+++ b/lib/platform/azure/index.ts
@@ -353,7 +353,7 @@ export async function createPr(
const targetRefName = azureHelper.getNewBranchName(
useDefaultBranch ? config.defaultBranch : config.baseBranch
);
- const description = azureHelper.max4000Chars(body);
+ const description = azureHelper.max4000Chars(hostRules.sanitize(body));
const azureApiGit = await azureApi.gitApi();
const workItemRefs = [
{
@@ -405,7 +405,9 @@ export async function updatePr(prNo: number, title: string, body?: string) {
title,
};
if (body) {
- objToUpdate.description = azureHelper.max4000Chars(body);
+ objToUpdate.description = azureHelper.max4000Chars(
+ hostRules.sanitize(body)
+ );
}
await azureApiGit.updatePullRequest(objToUpdate, config.repoId, prNo);
}
@@ -416,7 +418,7 @@ export async function ensureComment(
content: string
) {
logger.debug(`ensureComment(${issueNo}, ${topic}, content)`);
- const body = `### ${topic}\n\n${content}`;
+ const body = `### ${topic}\n\n${hostRules.sanitize(content)}`;
const azureApiGit = await azureApi.gitApi();
await azureApiGit.createThread(
{
diff --git a/lib/platform/bitbucket-server/index.ts b/lib/platform/bitbucket-server/index.ts
index 8f15c9cca2..b0b5aa73cc 100644
--- a/lib/platform/bitbucket-server/index.ts
+++ b/lib/platform/bitbucket-server/index.ts
@@ -459,7 +459,6 @@ export /* istanbul ignore next */ function ensureIssue(
title: string,
body: string
) {
- logger.debug(`ensureIssue(${title}, body={${body}})`);
logger.warn({ title }, 'Cannot ensure issue');
// TODO: Needs implementation
// This is used by Renovate when creating its own issues, e.g. for deprecated package warnings, config error notifications, or "masterIssue"
@@ -591,8 +590,9 @@ async function deleteComment(prNo: number, commentId: number) {
export async function ensureComment(
prNo: number,
topic: string | null,
- content: string
+ rawContent: string
) {
+ const content = hostRules.sanitize(rawContent);
try {
const comments = await getComments(prNo);
let body: string;
@@ -724,10 +724,11 @@ export async function findPr(
export async function createPr(
branchName: string,
title: string,
- description: string,
+ rawDescription: string,
_labels?: string[] | null,
useDefaultBranch?: boolean
) {
+ const description = hostRules.sanitize(rawDescription);
logger.debug(`createPr(${branchName}, title=${title})`);
const base = useDefaultBranch ? config.defaultBranch : config.baseBranch;
let reviewers = [];
@@ -880,8 +881,9 @@ export async function getPrFiles(prNo: number) {
export async function updatePr(
prNo: number,
title: string,
- description: string
+ rawDescription: string
) {
+ const description = hostRules.sanitize(rawDescription);
logger.debug(`updatePr(${prNo}, title=${title})`);
try {
diff --git a/lib/platform/bitbucket/index.ts b/lib/platform/bitbucket/index.ts
index fff83a9339..7835ad4ed3 100644
--- a/lib/platform/bitbucket/index.ts
+++ b/lib/platform/bitbucket/index.ts
@@ -353,12 +353,12 @@ async function closeIssue(issueNumber: number) {
export async function ensureIssue(title: string, body: string) {
logger.debug(`ensureIssue()`);
- const description = getPrBody(body);
+ const description = getPrBody(hostRules.sanitize(body));
/* istanbul ignore if */
if (!config.has_issues) {
logger.warn('Issues are disabled - cannot ensureIssue');
- logger.info({ title, body }, 'Failed to ensure Issue');
+ logger.info({ title }, 'Failed to ensure Issue');
return null;
}
try {
@@ -476,7 +476,12 @@ export function ensureComment(
content: string
) {
// https://developer.atlassian.com/bitbucket/api/2/reference/search?q=pullrequest+comment
- return comments.ensureComment(config, prNo, topic, content);
+ return comments.ensureComment(
+ config,
+ prNo,
+ topic,
+ hostRules.sanitize(content)
+ );
}
export function ensureCommentRemoval(prNo: number, topic: string) {
@@ -531,7 +536,7 @@ export async function createPr(
const body = {
title,
- description,
+ description: hostRules.sanitize(description),
source: {
branch: {
name: branchName,
@@ -647,7 +652,7 @@ export async function updatePr(
) {
logger.debug(`updatePr(${prNo}, ${title}, body)`);
await api.put(`/2.0/repositories/${config.repository}/pullrequests/${prNo}`, {
- body: { title, description },
+ body: { title, description: hostRules.sanitize(description) },
});
}
diff --git a/lib/platform/github/index.ts b/lib/platform/github/index.ts
index f4ef149696..7e85b45ef0 100644
--- a/lib/platform/github/index.ts
+++ b/lib/platform/github/index.ts
@@ -830,11 +830,12 @@ export async function findIssue(title: string) {
export async function ensureIssue(
title: string,
- body: string,
+ rawbody: string,
once = false,
reopen = true
) {
- logger.debug(`ensureIssue()`);
+ logger.debug(`ensureIssue(${title})`);
+ const body = hostRules.sanitize(rawbody);
try {
const issueList = await getIssueList();
const issues = issueList.filter(i => i.title === title);
@@ -1032,8 +1033,9 @@ async function deleteComment(commentId: number) {
export async function ensureComment(
issueNo: number,
topic: string | null,
- content: string
+ rawContent: string
) {
+ const content = hostRules.sanitize(rawContent);
try {
const comments = await getComments(issueNo);
let body: string;
@@ -1041,7 +1043,7 @@ export async function ensureComment(
let commentNeedsUpdating = false;
if (topic) {
logger.debug(`Ensuring comment "${topic}" in #${issueNo}`);
- body = `### ${topic}\n\n${content}`;
+ body = hostRules.sanitize(`### ${topic}\n\n${content}`);
comments.forEach(comment => {
if (comment.body.startsWith(`### ${topic}\n\n`)) {
commentId = comment.id;
@@ -1180,11 +1182,12 @@ export async function findPr(
export async function createPr(
branchName: string,
title: string,
- body: string,
+ rawBody: string,
labels: string[] | null,
useDefaultBranch: boolean,
platformOptions: { statusCheckVerify?: boolean } = {}
) {
+ const body = hostRules.sanitize(rawBody);
const base = useDefaultBranch ? config.defaultBranch : config.baseBranch;
// Include the repository owner to handle forkMode and regular mode
const head = `${config.repository!.split('/')[0]}:${branchName}`;
@@ -1583,8 +1586,9 @@ export async function getPrFiles(prNo: number) {
return files.map((f: { filename: string }) => f.filename);
}
-export async function updatePr(prNo: number, title: string, body?: string) {
+export async function updatePr(prNo: number, title: string, rawBody?: string) {
logger.debug(`updatePr(${prNo}, ${title}, body)`);
+ const body = rawBody ? hostRules.sanitize(rawBody) : null;
const patchBody: any = { title };
if (body) {
patchBody.body = body;
diff --git a/lib/platform/gitlab/index.ts b/lib/platform/gitlab/index.ts
index 906043aa8d..e34ea3b7c0 100644
--- a/lib/platform/gitlab/index.ts
+++ b/lib/platform/gitlab/index.ts
@@ -453,7 +453,7 @@ export async function findIssue(title: string) {
export async function ensureIssue(title: string, body: string) {
logger.debug(`ensureIssue()`);
- const description = getPrBody(body);
+ const description = getPrBody(hostRules.sanitize(body));
try {
const issueList = await getIssueList();
const issue = issueList.find((i: { title: string }) => i.title === title);
@@ -572,8 +572,9 @@ async function deleteComment(issueNo: number, commentId: number) {
export async function ensureComment(
issueNo: number,
topic: string | null | undefined,
- content: string
+ rawContent: string
) {
+ const content = hostRules.sanitize(rawContent);
const massagedTopic = topic
? topic.replace(/Pull Request/g, 'Merge Request').replace(/PR/g, 'MR')
: topic;
@@ -687,10 +688,11 @@ export async function findPr(
export async function createPr(
branchName: string,
title: string,
- description: string,
+ rawDescription: string,
labels?: string[] | null,
useDefaultBranch?: boolean
) {
+ const description = hostRules.sanitize(rawDescription);
const targetBranch = useDefaultBranch
? config.defaultBranch
: config.baseBranch;
@@ -798,7 +800,7 @@ export async function updatePr(
await api.put(`projects/${config.repository}/merge_requests/${iid}`, {
body: {
title,
- description,
+ description: hostRules.sanitize(description),
},
});
}
diff --git a/lib/util/host-rules.ts b/lib/util/host-rules.ts
index beb26180d7..1f045a24e1 100644
--- a/lib/util/host-rules.ts
+++ b/lib/util/host-rules.ts
@@ -15,6 +15,8 @@ export interface HostRule {
timeout?: number;
}
+let secrets: string[] = [];
+
let hostRules: HostRule[] = [];
export function add(params: HostRule) {
@@ -28,6 +30,18 @@ export function add(params: HostRule) {
throw new Error('hostRules cannot contain both a hostName and baseUrl');
}
hostRules.push(params);
+ const confidentialFields = ['password', 'token'];
+ confidentialFields.forEach(field => {
+ const secret = params[field];
+ if (secret && secret.length > 3 && !secrets.includes(secret))
+ secrets.push(secret);
+ });
+ if (params.username && params.password) {
+ const secret = Buffer.from(
+ `${params.username}:${params.password}`
+ ).toString('base64');
+ if (!secrets.includes(secret)) secrets.push(secret);
+ }
}
export interface HostRuleSearch {
@@ -152,6 +166,18 @@ export function hosts({ hostType }: { hostType: string }) {
.filter(Boolean);
}
+export function sanitize(input: string) {
+ if (!input) return input;
+ let output: string = input;
+ secrets.forEach(secret => {
+ while (output.includes(secret)) {
+ output = output.replace(secret, '**redacted**');
+ }
+ });
+ return output;
+}
+
export function clear() {
hostRules = [];
+ secrets = [];
}
diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js
index 8dc43b5220..fcbfd0b491 100644
--- a/lib/workers/branch/index.js
+++ b/lib/workers/branch/index.js
@@ -437,6 +437,11 @@ async function processBranch(branchConfig, prHourlyLimitReached, packageFiles) {
' - you check the rebase/retry checkbox if found above, or\n';
content +=
' - you rename this PR\'s title to start with "rebase!" to trigger it manually';
+ content += '\n\nThe artifact failure details are included below:\n\n';
+ config.artifactErrors.forEach(error => {
+ content += `##### File name: ${error.lockFile}\n\n`;
+ content += `\`\`\`\n${error.stderr}\n\`\`\`\n\n`;
+ });
if (
!(
config.suppressNotifications.includes('artifactErrors') ||
diff --git a/test/platform/azure/index.spec.ts b/test/platform/azure/index.spec.ts
index 7c7ad7c8f1..ce772c70ac 100644
--- a/test/platform/azure/index.spec.ts
+++ b/test/platform/azure/index.spec.ts
@@ -20,6 +20,7 @@ describe('platform/azure', () => {
jest.mock('../../../lib/platform/git/storage');
jest.mock('../../../lib/util/host-rules');
hostRules = require('../../../lib/util/host-rules');
+ hostRules.sanitize = jest.fn(input => input);
azure = require('../../../lib/platform/azure');
azureApi = require('../../../lib/platform/azure/azure-got-wrapper');
azureHelper = require('../../../lib/platform/azure/azure-helper');
diff --git a/test/platform/bitbucket-server/index.spec.ts b/test/platform/bitbucket-server/index.spec.ts
index 063944356a..abdeb19342 100644
--- a/test/platform/bitbucket-server/index.spec.ts
+++ b/test/platform/bitbucket-server/index.spec.ts
@@ -34,6 +34,7 @@ describe('platform/bitbucket-server', () => {
jest.mock('../../../lib/platform/git/storage');
jest.mock('../../../lib/util/host-rules');
hostRules = require('../../../lib/util/host-rules');
+ hostRules.sanitize = jest.fn(input => input);
api = require('../../../lib/platform/bitbucket-server/bb-got-wrapper')
.api;
jest.spyOn(api, 'get');
diff --git a/test/platform/bitbucket/index.spec.ts b/test/platform/bitbucket/index.spec.ts
index a097eeaf56..ce2bfbe8cb 100644
--- a/test/platform/bitbucket/index.spec.ts
+++ b/test/platform/bitbucket/index.spec.ts
@@ -17,6 +17,7 @@ describe('platform/bitbucket', () => {
jest.mock('../../../lib/platform/git/storage');
jest.mock('../../../lib/util/host-rules');
hostRules = require('../../../lib/util/host-rules');
+ hostRules.sanitize = jest.fn(input => input);
api = require('../../../lib/platform/bitbucket/bb-got-wrapper').api;
bitbucket = require('../../../lib/platform/bitbucket');
GitStorage = require('../../../lib/platform/git/storage').Storage;
diff --git a/test/platform/github/index.spec.ts b/test/platform/github/index.spec.ts
index 7d63b3dbf2..14b9891cbd 100644
--- a/test/platform/github/index.spec.ts
+++ b/test/platform/github/index.spec.ts
@@ -17,6 +17,7 @@ describe('platform/github', () => {
.api as any;
github = await import('../../../lib/platform/github');
hostRules = (await import('../../../lib/util/host-rules')) as any;
+ hostRules.sanitize = jest.fn(input => input);
jest.mock('../../../lib/platform/git/storage');
GitStorage = (await import('../../../lib/platform/git/storage'))
.Storage as any;
diff --git a/test/platform/gitlab/index.spec.ts b/test/platform/gitlab/index.spec.ts
index 106d2f0bb4..252daf41b2 100644
--- a/test/platform/gitlab/index.spec.ts
+++ b/test/platform/gitlab/index.spec.ts
@@ -19,6 +19,7 @@ describe('platform/gitlab', () => {
api = require('../../../lib/platform/gitlab/gl-got-wrapper').api;
jest.mock('../../../lib/util/host-rules');
hostRules = require('../../../lib/util/host-rules');
+ hostRules.sanitize = jest.fn(input => input);
jest.mock('../../../lib/platform/git/storage');
GitStorage = require('../../../lib/platform/git/storage').Storage;
GitStorage.mockImplementation(() => ({
diff --git a/test/util/__snapshots__/host-rules.spec.ts.snap b/test/util/__snapshots__/host-rules.spec.ts.snap
index 2eb17cea81..4ed77aa00e 100644
--- a/test/util/__snapshots__/host-rules.spec.ts.snap
+++ b/test/util/__snapshots__/host-rules.spec.ts.snap
@@ -33,3 +33,5 @@ Array [
"my.local.registry",
]
`;
+
+exports[`util/host-rules find() sanitizes secrets from strings 1`] = `"My token is **redacted**, username is \\"userabc\\" and password is \\"**redacted**\\" (hashed: **redacted**)"`;
diff --git a/test/util/host-rules.spec.ts b/test/util/host-rules.spec.ts
index c3c47ffbe1..3304b60026 100644
--- a/test/util/host-rules.spec.ts
+++ b/test/util/host-rules.spec.ts
@@ -1,4 +1,4 @@
-import { add, find, clear, hosts } from '../../lib/util/host-rules';
+import { add, find, clear, hosts, sanitize } from '../../lib/util/host-rules';
describe('util/host-rules', () => {
beforeEach(() => {
@@ -143,5 +143,26 @@ describe('util/host-rules', () => {
expect(res).toMatchSnapshot();
expect(res).toHaveLength(2);
});
+ it('sanitizes empty string', () => {
+ expect(sanitize(null)).toEqual(null);
+ });
+ it('sanitizes secrets from strings', () => {
+ const token = 'abc123token';
+ const username = 'userabc';
+ const password = 'password123';
+ add({
+ token,
+ });
+ add({
+ username,
+ password,
+ });
+ const hashed = Buffer.from(`${username}:${password}`).toString('base64');
+ expect(
+ sanitize(
+ `My token is ${token}, username is "${username}" and password is "${password}" (hashed: ${hashed})`
+ )
+ ).toMatchSnapshot();
+ });
});
});
--
GitLab