diff --git a/.changeset/chatty-dodos-design.md b/.changeset/chatty-dodos-design.md new file mode 100644 index 0000000000..d998f09250 --- /dev/null +++ b/.changeset/chatty-dodos-design.md @@ -0,0 +1,6 @@ +--- +'@backstage/plugin-scaffolder-backend': patch +'@backstage/backend-defaults': patch +--- + +Fixed WinstonLogger throwing when redactions were null or undefined diff --git a/.github/vale/config/vocabularies/Backstage/accept.txt b/.github/vale/config/vocabularies/Backstage/accept.txt index 430e76a2ce..7b7bf79dbc 100644 --- a/.github/vale/config/vocabularies/Backstage/accept.txt +++ b/.github/vale/config/vocabularies/Backstage/accept.txt @@ -367,6 +367,7 @@ readonly rebase rebasing Recharts +redactions Redash redis reimplement diff --git a/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.test.ts b/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.test.ts index 39293dcc55..7208c22d39 100644 --- a/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.test.ts +++ b/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.test.ts @@ -110,4 +110,12 @@ describe('WinstonLogger', () => { `={"foo":{"bar":{"baz":"qux"}}}`, ); }); + + it('should handle null and undefined values in redactions without crashing', () => { + const { add } = WinstonLogger.redacter(); + + expect(() => { + add([null as any, undefined as any, 'valid-secret']); + }).not.toThrow(); + }); }); diff --git a/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts b/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts index 545c9377b2..1b6b9db900 100644 --- a/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts +++ b/packages/backend-defaults/src/entrypoints/rootLogger/WinstonLogger.ts @@ -99,6 +99,10 @@ export class WinstonLogger implements RootLoggerService { add(newRedactions) { let added = 0; for (const redactionToTrim of newRedactions) { + // Skip null or undefined values + if (redactionToTrim === null || redactionToTrim === undefined) { + continue; + } // Trimming the string ensures that we don't accdentally get extra // newlines or other whitespace interfering with the redaction; this // can happen for example when using string literals in yaml diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.test.ts new file mode 100644 index 0000000000..ae83c1d719 --- /dev/null +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.test.ts @@ -0,0 +1,75 @@ +/* + * Copyright 2025 The Backstage Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { WinstonLogger } from './logger'; +import { MESSAGE } from 'triple-beam'; + +describe('WinstonLogger', () => { + describe('redacter', () => { + describe('add method', () => { + it('should handle null and undefined values in newRedactions without crashing', () => { + const { add, format } = WinstonLogger.redacter(); + + expect(() => { + add([null as any, undefined as any, 'valid-secret']); + }).not.toThrow(); + + const testObj = { + level: 'info', + message: 'This contains valid-secret and should be redacted', + [MESSAGE]: 'This contains valid-secret and should be redacted', + }; + const result = format.transform(testObj); + expect((result as any)?.[MESSAGE]).toBe( + 'This contains *** and should be redacted', + ); + }); + + it('should skip empty and single character redactions', () => { + const { add, format } = WinstonLogger.redacter(); + + add(['', 'x', 'valid-secret-123']); + + // MESSAGE symbol is where Winston stores the formatted message for redaction + const testObj = { + level: 'info', + message: 'This contains valid-secret-123 and should be redacted', + [MESSAGE]: 'This contains valid-secret-123 and should be redacted', + }; + + const result = format.transform(testObj); + expect((result as any)?.[MESSAGE]).toBe( + 'This contains *** and should be redacted', + ); + }); + + it('should trim whitespace from redactions', () => { + const { add, format } = WinstonLogger.redacter(); + + add([' secret-with-spaces \n', ' another-secret\t']); + + const testObj = { + level: 'info', + message: 'This contains secret-with-spaces and another-secret', + [MESSAGE]: 'This contains secret-with-spaces and another-secret', + }; + + const result = format.transform(testObj); + expect((result as any)?.[MESSAGE]).toBe('This contains *** and ***'); + }); + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts index 38263b73ca..a8e844c827 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/logger.ts @@ -132,6 +132,10 @@ export class WinstonLogger implements RootLoggerService { add(newRedactions) { let added = 0; for (const redactionToTrim of newRedactions) { + // Skip null or undefined values + if (redactionToTrim === null || redactionToTrim === undefined) { + continue; + } // Trimming the string ensures that we don't accdentally get extra // newlines or other whitespace interfering with the redaction; this // can happen for example when using string literals in yaml