From 2515a89dabf88bd6d4cf06386053352ea4107f7e Mon Sep 17 00:00:00 2001
From: Michael Kriese <michael.kriese@visualon.de>
Date: Sat, 16 May 2020 12:35:41 +0200
Subject: [PATCH] fix(logging): sanitize known token (#5917)
---
lib/config/decrypt.ts | 3 +
lib/datasource/npm/npmrc.spec.ts | 56 +++++++++++++++++++
lib/datasource/npm/npmrc.ts | 23 +++++++-
.../config-serializer.spec.ts.snap | 8 ---
lib/logger/config-serializer.spec.ts | 8 ---
lib/logger/config-serializer.ts | 15 -----
lib/logger/index.spec.ts | 8 +++
lib/logger/utils.ts | 28 ++++++++--
lib/util/sanitize.ts | 13 +++++
lib/workers/repository/init/index.spec.ts | 3 +
10 files changed, 129 insertions(+), 36 deletions(-)
create mode 100644 lib/datasource/npm/npmrc.spec.ts
diff --git a/lib/config/decrypt.ts b/lib/config/decrypt.ts
index 682cd5a179..7fa3a9c860 100644
--- a/lib/config/decrypt.ts
+++ b/lib/config/decrypt.ts
@@ -2,6 +2,7 @@ import crypto from 'crypto';
import is from '@sindresorhus/is';
import { logger } from '../logger';
import { maskToken } from '../util/mask';
+import { add } from '../util/sanitize';
import { RenovateConfig } from './common';
export function decryptConfig(
@@ -43,6 +44,7 @@ export function decryptConfig(
logger.debug(`Decrypted ${eKey}`);
if (eKey === 'npmToken') {
const token = decryptedStr.replace(/\n$/, '');
+ add(token);
logger.debug(
{ decryptedToken: maskToken(token) },
'Migrating npmToken to npmrc'
@@ -71,6 +73,7 @@ export function decryptConfig(
}
} else {
decryptedConfig[eKey] = decryptedStr;
+ add(decryptedStr);
}
} catch (err) {
const error = new Error('config-validation');
diff --git a/lib/datasource/npm/npmrc.spec.ts b/lib/datasource/npm/npmrc.spec.ts
new file mode 100644
index 0000000000..ee4f5e63ec
--- /dev/null
+++ b/lib/datasource/npm/npmrc.spec.ts
@@ -0,0 +1,56 @@
+import { getName, mocked } from '../../../test/util';
+import * as _sanitize from '../../util/sanitize';
+import { getNpmrc, setNpmrc } from './npmrc';
+
+jest.mock('../../util/sanitize');
+
+const sanitize = mocked(_sanitize);
+
+describe(getName(__filename), () => {
+ beforeEach(() => {
+ delete process.env.NPM_TOKEN;
+ delete global.trustLevel;
+ setNpmrc('');
+ jest.resetAllMocks();
+ });
+
+ it('sanitize _auth', () => {
+ setNpmrc('_auth=test');
+ expect(sanitize.add).toBeCalledWith('test');
+ expect(sanitize.add).toBeCalledTimes(1);
+ });
+
+ it('sanitize _authtoken', () => {
+ // eslint-disable-next-line no-template-curly-in-string
+ setNpmrc('//registry.test.com:_authToken=test\n_authToken=${NPM_TOKEN}');
+ expect(sanitize.add).toBeCalledWith('test');
+ expect(sanitize.add).toBeCalledTimes(1);
+ });
+
+ it('sanitize _password', () => {
+ setNpmrc(
+ `registry=https://test.org\n//test.org/:username=test\n//test.org/:_password=dGVzdA==`
+ );
+ expect(sanitize.add).toHaveBeenNthCalledWith(1, 'dGVzdA==');
+ expect(sanitize.add).toHaveBeenNthCalledWith(2, 'test');
+ expect(sanitize.add).toHaveBeenNthCalledWith(3, 'dGVzdDp0ZXN0');
+ expect(sanitize.add).toBeCalledTimes(3);
+ });
+
+ it('sanitize _authtoken with high trust', () => {
+ global.trustLevel = 'high';
+ process.env.TEST_TOKEN = 'test';
+ setNpmrc(
+ // eslint-disable-next-line no-template-curly-in-string
+ '//registry.test.com:_authToken=${TEST_TOKEN}\n_authToken=\nregistry=http://localhost'
+ );
+ expect(sanitize.add).toBeCalledWith('test');
+ expect(sanitize.add).toBeCalledTimes(1);
+ });
+
+ it('ignores localhost', () => {
+ setNpmrc(`registry=http://localhost`);
+ expect(sanitize.add).toHaveBeenCalledTimes(0);
+ expect(getNpmrc()).toBeNull();
+ });
+});
diff --git a/lib/datasource/npm/npmrc.ts b/lib/datasource/npm/npmrc.ts
index 2c78b868bf..c383519ae4 100644
--- a/lib/datasource/npm/npmrc.ts
+++ b/lib/datasource/npm/npmrc.ts
@@ -1,6 +1,7 @@
import is from '@sindresorhus/is';
import ini from 'ini';
import { logger } from '../../logger';
+import { add } from '../../util/sanitize';
let npmrc: Record<string, any> | null = null;
let npmrcRaw: string;
@@ -26,6 +27,23 @@ function envReplace(value: any, env = process.env): any {
});
}
+const envRe = /(\\*)\$\{([^}]+)\}/;
+// TODO: better add to host rules
+function sanitize(key: string, val: string): void {
+ if (!val || envRe.test(val)) {
+ return;
+ }
+ if (key.endsWith('_authToken') || key.endsWith('_auth')) {
+ add(val);
+ } else if (key.endsWith(':_password')) {
+ add(val);
+ const password = Buffer.from(val, 'base64').toString();
+ add(password);
+ const username = npmrc[key.replace(':_password', ':username')];
+ add(Buffer.from(`${username}:${password}`).toString('base64'));
+ }
+}
+
export function setNpmrc(input?: string): void {
if (input) {
if (input === npmrcRaw) {
@@ -36,7 +54,9 @@ export function setNpmrc(input?: string): void {
logger.debug('Setting npmrc');
npmrc = ini.parse(input.replace(/\\n/g, '\n'));
for (const [key, val] of Object.entries(npmrc)) {
- // istanbul ignore if
+ if (global.trustLevel !== 'high') {
+ sanitize(key, val);
+ }
if (
global.trustLevel !== 'high' &&
key.endsWith('registry') &&
@@ -56,6 +76,7 @@ export function setNpmrc(input?: string): void {
}
for (const key of Object.keys(npmrc)) {
npmrc[key] = envReplace(npmrc[key]);
+ sanitize(key, npmrc[key]);
}
} else if (npmrc) {
logger.debug('Resetting npmrc');
diff --git a/lib/logger/__snapshots__/config-serializer.spec.ts.snap b/lib/logger/__snapshots__/config-serializer.spec.ts.snap
index 75f1a41fae..7d087484e2 100644
--- a/lib/logger/__snapshots__/config-serializer.spec.ts.snap
+++ b/lib/logger/__snapshots__/config-serializer.spec.ts.snap
@@ -1,13 +1,5 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
-exports[`logger/config-serializer redacts sensitive fields 1`] = `
-Object {
- "githubAppKey": "***********",
- "nottoken": "b",
- "token": "***********",
-}
-`;
-
exports[`logger/config-serializer squashes templates 1`] = `
Object {
"nottoken": "b",
diff --git a/lib/logger/config-serializer.spec.ts b/lib/logger/config-serializer.spec.ts
index 95cbfa9081..d3079b2b5d 100644
--- a/lib/logger/config-serializer.spec.ts
+++ b/lib/logger/config-serializer.spec.ts
@@ -1,14 +1,6 @@
import configSerializer from './config-serializer';
describe('logger/config-serializer', () => {
- it('redacts sensitive fields', () => {
- const config = {
- token: 'a',
- nottoken: 'b',
- githubAppKey: 'c',
- };
- expect(configSerializer(config)).toMatchSnapshot();
- });
it('squashes templates', () => {
const config = {
nottoken: 'b',
diff --git a/lib/logger/config-serializer.ts b/lib/logger/config-serializer.ts
index 6167662bba..856531945e 100644
--- a/lib/logger/config-serializer.ts
+++ b/lib/logger/config-serializer.ts
@@ -4,18 +4,6 @@ import { RenovateConfig } from '../config/common';
export default function configSerializer(
config: RenovateConfig
): RenovateConfig {
- const redactedFields = [
- 'authorization',
- 'token',
- 'githubAppKey',
- 'npmToken',
- 'npmrc',
- 'yarnrc',
- 'privateKey',
- 'gitPrivateKey',
- 'forkToken',
- 'password',
- ];
const templateFields = ['prBody'];
const contentFields = [
'content',
@@ -28,9 +16,6 @@ export default function configSerializer(
return traverse(config).map(
// eslint-disable-next-line array-callback-return
function scrub(val: string) {
- if (val && redactedFields.includes(this.key)) {
- this.update('***********');
- }
if (val && templateFields.includes(this.key)) {
this.update('[Template]');
}
diff --git a/lib/logger/index.spec.ts b/lib/logger/index.spec.ts
index 37decab0d4..892cbafb4a 100644
--- a/lib/logger/index.spec.ts
+++ b/lib/logger/index.spec.ts
@@ -143,11 +143,19 @@ describe('logger', () => {
logger.error({
foo: 'secret"password',
bar: ['somethingelse', 'secret"password'],
+ npmToken: 'token',
+ buffer: Buffer.from('test'),
+ content: 'test',
+ prBody: 'test',
});
expect(logged.foo).not.toEqual('secret"password');
expect(logged.bar[0]).toEqual('somethingelse');
expect(logged.foo).toContain('redacted');
expect(logged.bar[1]).toContain('redacted');
+ expect(logged.npmToken).not.toEqual('token');
+ expect(logged.buffer).toEqual('[content]');
+ expect(logged.content).toEqual('[content]');
+ expect(logged.prBody).toEqual('[Template]');
});
});
diff --git a/lib/logger/utils.ts b/lib/logger/utils.ts
index 258edcc8d4..c21302907f 100644
--- a/lib/logger/utils.ts
+++ b/lib/logger/utils.ts
@@ -1,7 +1,7 @@
import { Stream } from 'stream';
import bunyan from 'bunyan';
import fs from 'fs-extra';
-import { sanitize } from '../util/sanitize';
+import { redactedFields, sanitize } from '../util/sanitize';
export interface BunyanRecord extends Record<string, any> {
level: number;
@@ -37,6 +37,13 @@ export class ErrorStream extends Stream {
return this._errors;
}
}
+const templateFields = ['prBody'];
+const contentFields = [
+ 'content',
+ 'contents',
+ 'packageLockParsed',
+ 'yarnLockParsed',
+];
function sanitizeValue(value: any, seen = new WeakMap()): any {
if (Array.isArray(value)) {
@@ -52,6 +59,10 @@ function sanitizeValue(value: any, seen = new WeakMap()): any {
return arrayResult;
}
+ if (value instanceof Buffer) {
+ return '[content]';
+ }
+
const valueType = typeof value;
if (value != null && valueType !== 'function' && valueType === 'object') {
@@ -62,9 +73,18 @@ function sanitizeValue(value: any, seen = new WeakMap()): any {
const objectResult: Record<string, any> = {};
seen.set(value, objectResult);
for (const [key, val] of Object.entries<any>(value)) {
- objectResult[key] = seen.has(val)
- ? seen.get(val)
- : sanitizeValue(val, seen);
+ let curValue: any;
+ if (redactedFields.includes(key)) {
+ curValue = '***********';
+ } else if (contentFields.includes(key)) {
+ curValue = '[content]';
+ } else if (templateFields.includes(key)) {
+ curValue = '[Template]';
+ } else {
+ curValue = seen.has(val) ? seen.get(val) : sanitizeValue(val, seen);
+ }
+
+ objectResult[key] = curValue;
}
return objectResult;
}
diff --git a/lib/util/sanitize.ts b/lib/util/sanitize.ts
index e2e8e90bb4..0ff2667d29 100644
--- a/lib/util/sanitize.ts
+++ b/lib/util/sanitize.ts
@@ -1,5 +1,18 @@
const secrets = new Set<string>();
+export const redactedFields = [
+ 'authorization',
+ 'token',
+ 'githubAppKey',
+ 'npmToken',
+ 'npmrc',
+ 'yarnrc',
+ 'privateKey',
+ 'gitPrivateKey',
+ 'forkToken',
+ 'password',
+];
+
export function sanitize(input: string): string {
if (!input) {
return input;
diff --git a/lib/workers/repository/init/index.spec.ts b/lib/workers/repository/init/index.spec.ts
index 0a3e6c7627..204803170b 100644
--- a/lib/workers/repository/init/index.spec.ts
+++ b/lib/workers/repository/init/index.spec.ts
@@ -1,6 +1,7 @@
import { mocked } from '../../../../test/util';
import * as _apis from './apis';
import * as _base from './base';
+import * as _config from './config';
import { initRepo } from '.';
jest.mock('../../../workers/repository/onboarding/branch');
@@ -12,12 +13,14 @@ jest.mock('../../../workers/repository/init/semantic');
const base = mocked(_base);
const apis = mocked(_apis);
+const config = mocked(_config);
describe('workers/repository/init', () => {
describe('initRepo', () => {
it('runs', async () => {
base.checkBaseBranch.mockResolvedValue({});
apis.initApis.mockResolvedValue({} as never);
+ config.mergeRenovateConfig.mockResolvedValueOnce({});
await initRepo({});
});
});
--
GitLab