diff --git a/.changeset/neat-chairs-complain.md b/.changeset/neat-chairs-complain.md new file mode 100644 index 0000000000..96c0271d81 --- /dev/null +++ b/.changeset/neat-chairs-complain.md @@ -0,0 +1,5 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +--- + +Fixing issues with log redaction in the scaffolder logs diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts index 5d70e5bfba..aece424f5f 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.test.ts @@ -644,6 +644,100 @@ describe('NunjucksWorkflowRunner', () => { }); }); + describe('redactions', () => { + // eslint-disable-next-line jest/expect-expect + it('should redact secrets that are passed with the task', async () => { + actionRegistry.register({ + id: 'log-secret', + description: 'Mock action for testing', + supportsDryRun: true, + handler: async ctx => { + ctx.logger.info(ctx.input.secret); + }, + schema: { + input: { + type: 'object', + required: ['secret'], + properties: { + secret: { + type: 'string', + }, + }, + }, + }, + }); + + const task = createMockTaskWithSpec( + { + apiVersion: 'scaffolder.backstage.io/v1beta3', + parameters: {}, + output: {}, + steps: [ + { + id: 'test', + name: 'name', + action: 'log-secret', + input: { + secret: '${{ secrets.secret }}', + }, + }, + ], + }, + { secret: 'my-secret-value' }, + ); + + await runner.execute(task); + + expectTaskLog('info: ***'); + }); + + // eslint-disable-next-line jest/expect-expect + it('should redact meta fields properly', async () => { + actionRegistry.register({ + id: 'log-secret', + description: 'Mock action for testing', + supportsDryRun: true, + handler: async ctx => { + ctx.logger.child({ thing: ctx.input.secret }).info(ctx.input.secret); + }, + schema: { + input: { + type: 'object', + required: ['secret'], + properties: { + secret: { + type: 'string', + }, + }, + }, + }, + }); + + const task = createMockTaskWithSpec( + { + apiVersion: 'scaffolder.backstage.io/v1beta3', + parameters: {}, + output: {}, + steps: [ + { + id: 'test', + name: 'name', + action: 'log-secret', + input: { + secret: '${{ secrets.secret }}', + }, + }, + ], + }, + { secret: 'my-secret-value' }, + ); + + await runner.execute(task); + + expectTaskLog('info: *** {"thing":"***"}'); + }); + }); + describe('each', () => { it('should run a step repeatedly - flat values', async () => { const colors = ['blue', 'green', 'red']; diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts index 85664e1a6c..e9b1d78e36 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/NunjucksWorkflowRunner.ts @@ -104,24 +104,13 @@ const createStepLogger = ({ step: TaskStep; rootLogger: winston.Logger; }) => { - const stepLogStream = new PassThrough(); - stepLogStream.on('data', async data => { - const message = data.toString().trim(); - if (message?.length > 1) { - await task.emitLog(message, { stepId: step.id }); - } - }); - const taskLogger = WinstonLogger.create({ level: process.env.LOG_LEVEL || 'info', format: winston.format.combine( winston.format.colorize(), winston.format.simple(), ), - transports: [ - new winston.transports.Stream({ stream: stepLogStream }), - new BackstageLoggerTransport(rootLogger), - ], + transports: [new BackstageLoggerTransport(rootLogger, task, step.id)], }); taskLogger.addRedactions(Object.values(task.secrets ?? {})); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts index e85c61d489..38263b73ca 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts @@ -21,7 +21,9 @@ 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'; +import { LEVEL, MESSAGE, SPLAT } from 'triple-beam'; +import { TaskContext } from '@backstage/plugin-scaffolder-node'; +import _ from 'lodash'; /** * Escapes a given string to be used inside a RegExp. @@ -44,33 +46,41 @@ interface WinstonLoggerOptions { export class BackstageLoggerTransport extends Transport { constructor( private readonly backstageLogger: LoggerService, + private readonly taskContext: TaskContext, + private readonly stepId: string, opts?: TransportStreamOptions, ) { super(opts); } - log(info: unknown, callback: VoidFunction) { + log(info: TransformableInfo, callback: VoidFunction) { if (typeof info !== 'object' || info === null) { callback(); return; } - const { level, message, ...meta } = info as JsonObject; + + const message = info[MESSAGE]; + const level = info[LEVEL]; + const splat = info[SPLAT]; + switch (level) { case 'error': - this.backstageLogger.error(String(message), meta); + this.backstageLogger.error(String(message), ...splat); break; case 'warn': - this.backstageLogger.warn(String(message), meta); + this.backstageLogger.warn(String(message), ...splat); break; case 'info': - this.backstageLogger.info(String(message), meta); + this.backstageLogger.info(String(message), ...splat); break; case 'debug': - this.backstageLogger.debug(String(message), meta); + this.backstageLogger.debug(String(message), ...splat); break; default: - this.backstageLogger.info(String(message), meta); + this.backstageLogger.info(String(message), ...splat); } + + this.taskContext.emitLog(message, { stepId: this.stepId }); callback(); } } @@ -87,9 +97,10 @@ export class WinstonLogger implements RootLoggerService { let logger = createLogger({ level: options.level, - format: format.combine(redacter.format, options.format), + format: format.combine(options.format, redacter.format), transports: options.transports ?? new transports.Console(), }); + if (options.meta) { logger = logger.child(options.meta); } @@ -163,7 +174,10 @@ export class WinstonLogger implements RootLoggerService { }, }), format.printf((info: TransformableInfo) => { - const { timestamp, level, message, plugin, service, ...fields } = info; + const { timestamp, plugin, service } = info; + const message = info[MESSAGE]; + const level = info[LEVEL]; + const fields = info[SPLAT]; const prefix = plugin || service; const timestampColor = colorizer.colorize('timestamp', timestamp); const prefixColor = colorizer.colorize('prefix', prefix);