From 26b5da1c1aadabe65c741c0d6ee0a2610f9acae4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 16 Nov 2021 11:42:10 +0100 Subject: [PATCH 1/3] Do not redact the empty string MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/spicy-rice-build.md | 5 +++++ packages/backend-common/src/logging/rootLogger.ts | 9 ++++++--- 2 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 .changeset/spicy-rice-build.md diff --git a/.changeset/spicy-rice-build.md b/.changeset/spicy-rice-build.md new file mode 100644 index 0000000000..d8da66f66c --- /dev/null +++ b/.changeset/spicy-rice-build.md @@ -0,0 +1,5 @@ +--- +'@backstage/backend-common': patch +--- + +Do not redact the empty string, destroying all logs diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index b05763a42c..2a6e92ac87 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -21,7 +21,7 @@ import { coloredFormat } from './formats'; import { escapeRegExp } from '../util/escapeRegExp'; let rootLogger: winston.Logger; -let redactionRegExp: RegExp; +let redactionRegExp: RegExp | undefined; /** @public */ export function getRootLogger(): winston.Logger { @@ -34,11 +34,14 @@ export function setRootLogger(newLogger: winston.Logger) { } export function setRootLoggerRedactionList(redactionList: string[]) { - if (redactionList.length) { + const filtered = redactionList.filter(Boolean); + if (filtered.length) { redactionRegExp = new RegExp( - `(${redactionList.map(escapeRegExp).join('|')})`, + `(${filtered.map(escapeRegExp).join('|')})`, 'g', ); + } else { + redactionRegExp = undefined; } } From f9daf056e52cf9632b250b87a08f1493eaa8dab3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 16 Nov 2021 11:47:51 +0100 Subject: [PATCH 2/3] add test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .../backend-common/src/logging/rootLogger.test.ts | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/backend-common/src/logging/rootLogger.test.ts b/packages/backend-common/src/logging/rootLogger.test.ts index 192decb653..67353a2f2d 100644 --- a/packages/backend-common/src/logging/rootLogger.test.ts +++ b/packages/backend-common/src/logging/rootLogger.test.ts @@ -48,6 +48,19 @@ describe('rootLogger', () => { ); }); + it('redacts but ignores empty secrets', () => { + const logger = createRootLogger(); + jest.spyOn(logger, 'write'); + setRootLoggerRedactionList(['SECRET-1', 'SECRET_2', '']); + logger.info('Logging SECRET-1 and SECRET_2'); + + expect(logger.write).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Logging [REDACTED] and [REDACTED]', + }), + ); + }); + describe('createRootLogger', () => { it('creates a new logger', () => { const oldLogger = getRootLogger(); From 50154fb792222551a0530846505fd10d810f04d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fredrik=20Adel=C3=B6w?= Date: Tue, 16 Nov 2021 13:37:28 +0100 Subject: [PATCH 3/3] exclude one-character secrets as well MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Fredrik Adelöw --- .changeset/spicy-rice-build.md | 2 +- packages/backend-common/src/logging/rootLogger.test.ts | 8 ++++---- packages/backend-common/src/logging/rootLogger.ts | 6 +++++- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/.changeset/spicy-rice-build.md b/.changeset/spicy-rice-build.md index d8da66f66c..c0322d778a 100644 --- a/.changeset/spicy-rice-build.md +++ b/.changeset/spicy-rice-build.md @@ -2,4 +2,4 @@ '@backstage/backend-common': patch --- -Do not redact the empty string, destroying all logs +Do not redact empty or one-character strings. These imply that it's just a test or local dev, and unnecessarily ruin the log output. diff --git a/packages/backend-common/src/logging/rootLogger.test.ts b/packages/backend-common/src/logging/rootLogger.test.ts index 67353a2f2d..0a664f9656 100644 --- a/packages/backend-common/src/logging/rootLogger.test.ts +++ b/packages/backend-common/src/logging/rootLogger.test.ts @@ -48,15 +48,15 @@ describe('rootLogger', () => { ); }); - it('redacts but ignores empty secrets', () => { + it('redacts but ignores empty and one-character secrets', () => { const logger = createRootLogger(); jest.spyOn(logger, 'write'); - setRootLoggerRedactionList(['SECRET-1', 'SECRET_2', '']); - logger.info('Logging SECRET-1 and SECRET_2'); + setRootLoggerRedactionList(['SECRET-1', 'SECRET_2', 'Q', '']); + logger.info('Logging SECRET-1 and SECRET_2 and Q'); expect(logger.write).toHaveBeenCalledWith( expect.objectContaining({ - message: 'Logging [REDACTED] and [REDACTED]', + message: 'Logging [REDACTED] and [REDACTED] and Q', }), ); }); diff --git a/packages/backend-common/src/logging/rootLogger.ts b/packages/backend-common/src/logging/rootLogger.ts index 2a6e92ac87..2a4226f88f 100644 --- a/packages/backend-common/src/logging/rootLogger.ts +++ b/packages/backend-common/src/logging/rootLogger.ts @@ -34,7 +34,11 @@ export function setRootLogger(newLogger: winston.Logger) { } export function setRootLoggerRedactionList(redactionList: string[]) { - const filtered = redactionList.filter(Boolean); + // Exclude secrets that are empty or just one character in length. These + // typically mean that you are running local dev or tests, or using the + // --lax flag which sets things to just 'x'. So exclude those. + const filtered = redactionList.filter(r => r.length > 1); + if (filtered.length) { redactionRegExp = new RegExp( `(${filtered.map(escapeRegExp).join('|')})`,