diff --git a/.changeset/nine-ties-type.md b/.changeset/nine-ties-type.md new file mode 100644 index 0000000000..abb0661724 --- /dev/null +++ b/.changeset/nine-ties-type.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +'@backstage/backend-app-api': patch +--- + +Fixing issue with log meta fields possibly being circular refs diff --git a/packages/backend-app-api/package.json b/packages/backend-app-api/package.json index 0634c30de1..414a4333e7 100644 --- a/packages/backend-app-api/package.json +++ b/packages/backend-app-api/package.json @@ -83,6 +83,7 @@ "path-to-regexp": "^6.2.1", "selfsigned": "^2.0.0", "stoppable": "^1.1.0", + "triple-beam": "^1.4.1", "uuid": "^9.0.0", "winston": "^3.2.1", "winston-transport": "^4.5.0" diff --git a/packages/backend-app-api/src/logging/WinstonLogger.test.ts b/packages/backend-app-api/src/logging/WinstonLogger.test.ts index 06f9cdd4c3..4b608f8e57 100644 --- a/packages/backend-app-api/src/logging/WinstonLogger.test.ts +++ b/packages/backend-app-api/src/logging/WinstonLogger.test.ts @@ -14,12 +14,10 @@ * limitations under the License. */ -import { TransformableInfo } from 'logform'; +import { format } from 'logform'; import { WinstonLogger } from './WinstonLogger'; - -function msg(info: TransformableInfo): TransformableInfo { - return { message: info.message, level: info.level, stack: info.stack }; -} +import Transport from 'winston-transport'; +import { MESSAGE } from 'triple-beam'; describe('WinstonLogger', () => { it('creates a winston logger instance with default options', () => { @@ -33,57 +31,66 @@ describe('WinstonLogger', () => { expect(childLogger).toBeInstanceOf(WinstonLogger); }); - it('redacter should redact and escape regex', () => { - const redacter = WinstonLogger.redacter(); - const log = { - level: 'error', - message: 'hello (world)', - stack: 'hello (world) from this file', - }; - expect(redacter.format.transform(msg(log))).toEqual(msg(log)); - redacter.add(['hello\n']); - expect(redacter.format.transform(msg(log))).toEqual( - msg({ - ...log, - message: '[REDACTED] (world)', - stack: '[REDACTED] (world) from this file', - }), - ); - redacter.add(['(world']); - expect(redacter.format.transform(msg(log))).toEqual( - msg({ - ...log, - message: '[REDACTED] [REDACTED])', - stack: '[REDACTED] [REDACTED]) from this file', + it('should redact and escape regex', () => { + const mockTransport = new Transport({ + log: jest.fn(), + logv: jest.fn(), + }); + + const logger = WinstonLogger.create({ + format: format.json(), + transports: [mockTransport], + }); + + logger.addRedactions(['hello (world']); + + logger.error('hello (world) from this file'); + + expect(mockTransport.log).toHaveBeenCalledWith( + expect.objectContaining({ + [MESSAGE]: JSON.stringify({ + level: 'error', + message: '[REDACTED]) from this file', + }), }), + expect.any(Function), ); }); - it('redacter should redact nested object', () => { - const redacter = WinstonLogger.redacter(); - const log = { - level: 'error', - message: { - nested: 'hello (world) from nested object', - null: null, - nullProto: Object.create(null, { - foo: { value: 'hello foo', enumerable: true }, - }), - }, - }; + it('should redact nested object', () => { + const mockTransport = new Transport({ + log: jest.fn(), + logv: jest.fn(), + }); - redacter.add(['hello']); - expect(redacter.format.transform(msg(log))).toEqual( - msg({ - ...log, - message: { + const logger = WinstonLogger.create({ + format: format.json(), + transports: [mockTransport], + }); + + logger.addRedactions(['hello']); + + logger.error('something went wrong', { + null: null, + nested: 'hello (world) from nested object', + nullProto: Object.create(null, { + foo: { value: 'hello foo', enumerable: true }, + }), + }); + + expect(mockTransport.log).toHaveBeenCalledWith( + expect.objectContaining({ + [MESSAGE]: JSON.stringify({ + level: 'error', + message: 'something went wrong', nested: '[REDACTED] (world) from nested object', null: null, nullProto: { - foo: 'hello foo', // read only prop is not redacted + foo: '[REDACTED] foo', }, - }, + }), }), + expect.any(Function), ); }); }); diff --git a/packages/backend-app-api/src/logging/WinstonLogger.ts b/packages/backend-app-api/src/logging/WinstonLogger.ts index 3fcb4142b6..8ee86d247f 100644 --- a/packages/backend-app-api/src/logging/WinstonLogger.ts +++ b/packages/backend-app-api/src/logging/WinstonLogger.ts @@ -27,6 +27,7 @@ import { transports, transport as Transport, } from 'winston'; +import { MESSAGE } from 'triple-beam'; import { escapeRegExp } from '../lib/escapeRegExp'; /** @@ -61,8 +62,8 @@ export class WinstonLogger implements RootLoggerService { let logger = createLogger({ level: process.env.LOG_LEVEL || options.level || 'info', format: format.combine( - redacter.format, options.format ?? defaultFormatter, + redacter.format, ), transports: options.transports ?? new transports.Console(), }); @@ -85,24 +86,16 @@ export class WinstonLogger implements RootLoggerService { let redactionPattern: RegExp | undefined = undefined; - const replace = (obj: TransformableInfo) => { - for (const key in obj) { - if (Object.hasOwn(obj, key)) { - if (typeof obj[key] === 'object') { - obj[key] = replace(obj[key] as TransformableInfo); - } else if (typeof obj[key] === 'string') { - try { - obj[key] = obj[key]?.replace(redactionPattern, '[REDACTED]'); - } catch { - /* ignore read only properties */ - } - } - } - } - return obj; - }; return { - format: format(replace)(), + format: format((obj: TransformableInfo) => { + if (!redactionPattern || !obj) { + return obj; + } + + obj[MESSAGE] = obj[MESSAGE]?.replace?.(redactionPattern, '[REDACTED]'); + + return obj; + })(), add(newRedactions) { let added = 0; for (const redactionToTrim of newRedactions) { diff --git a/plugins/scaffolder-backend/package.json b/plugins/scaffolder-backend/package.json index f2f4da6a84..f748ccb92d 100644 --- a/plugins/scaffolder-backend/package.json +++ b/plugins/scaffolder-backend/package.json @@ -95,6 +95,7 @@ "p-queue": "^6.6.2", "prom-client": "^15.0.0", "tar": "^6.1.12", + "triple-beam": "^1.4.1", "uuid": "^9.0.0", "winston": "^3.2.1", "winston-transport": "^4.7.0", diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts index 064eb69349..3824a32a6f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts @@ -21,6 +21,7 @@ import { JsonObject } from '@backstage/types'; import { Format, TransformableInfo } from 'logform'; import Transport, { TransportStreamOptions } from 'winston-transport'; import { Logger, format, createLogger, transports } from 'winston'; +import { MESSAGE } from 'triple-beam'; /** * Escapes a given string to be used inside a RegExp. @@ -107,20 +108,16 @@ export class WinstonLogger implements RootLoggerService { let redactionPattern: RegExp | undefined = undefined; - const replace = (obj: TransformableInfo) => { - for (const key in obj) { - if (obj.hasOwnProperty(key)) { - if (typeof obj[key] === 'object') { - obj[key] = replace(obj[key] as TransformableInfo); - } else if (typeof obj[key] === 'string') { - obj[key] = obj[key]?.replace(redactionPattern, '[REDACTED]'); - } - } - } - return obj; - }; return { - format: format(replace)(), + format: format((obj: TransformableInfo) => { + if (!redactionPattern || !obj) { + return obj; + } + + obj[MESSAGE] = obj[MESSAGE]?.replace?.(redactionPattern, '[REDACTED]'); + + return obj; + })(), add(newRedactions) { let added = 0; for (const redactionToTrim of newRedactions) { diff --git a/yarn.lock b/yarn.lock index f10de89d74..728bac0c68 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3330,6 +3330,7 @@ __metadata: selfsigned: ^2.0.0 stoppable: ^1.1.0 supertest: ^6.1.3 + triple-beam: ^1.4.1 uuid: ^9.0.0 winston: ^3.2.1 winston-transport: ^4.5.0 @@ -6685,6 +6686,7 @@ __metadata: strip-ansi: ^7.1.0 supertest: ^6.1.3 tar: ^6.1.12 + triple-beam: ^1.4.1 uuid: ^9.0.0 wait-for-expect: ^3.0.2 winston: ^3.2.1 @@ -41480,10 +41482,10 @@ __metadata: languageName: node linkType: hard -"triple-beam@npm:^1.3.0": - version: 1.3.0 - resolution: "triple-beam@npm:1.3.0" - checksum: 7d7b77d8625fb252c126c24984a68de462b538a8fcd1de2abd0a26421629cf3527d48e23b3c2264f08f4a6c3bc40a478a722176f4d7b6a1acc154cb70c359f2b +"triple-beam@npm:^1.3.0, triple-beam@npm:^1.4.1": + version: 1.4.1 + resolution: "triple-beam@npm:1.4.1" + checksum: 2e881a3e8e076b6f2b85b9ec9dd4a900d3f5016e6d21183ed98e78f9abcc0149e7d54d79a3f432b23afde46b0885bdcdcbff789f39bc75de796316961ec07f61 languageName: node linkType: hard