From b90047cfec1a7b205a361862cfe44077ac4982c0 Mon Sep 17 00:00:00 2001 From: John Redwood Date: Wed, 16 Jul 2025 21:05:03 +1000 Subject: [PATCH] chore: resolve raised changes Signed-off-by: John Redwood --- app-config.yaml | 2 +- plugins/scaffolder-backend/config.d.ts | 15 +++++ .../src/scaffolder/tasks/TaskWorker.test.ts | 57 ++++++++++++++++++- .../src/scaffolder/tasks/TaskWorker.ts | 44 ++++++++------ 4 files changed, 99 insertions(+), 19 deletions(-) diff --git a/app-config.yaml b/app-config.yaml index d0189595e4..59a790d406 100644 --- a/app-config.yaml +++ b/app-config.yaml @@ -201,7 +201,7 @@ catalog: scaffolder: auditor: - maxLength: 256 + taskParameterMaxLength: 256 # Use to customize default commit author info used when new components are created defaultAuthor: name: Scaffolder diff --git a/plugins/scaffolder-backend/config.d.ts b/plugins/scaffolder-backend/config.d.ts index ae91c905b3..b4fb532901 100644 --- a/plugins/scaffolder-backend/config.d.ts +++ b/plugins/scaffolder-backend/config.d.ts @@ -93,5 +93,20 @@ export interface Config { * Default value is 24 hours. */ taskTimeout?: HumanDuration | string; + + /** + * Sets the maximum length for task parameters recorded by the auditor. + * + * If set to -1, the limit is disabled and parameters are not truncated. + * Defaults to 256 character length. + * + * @example + * scaffolder: + * auditor: + * taskParameterMaxLength: 512 + */ + auditor?: { + taskParameterMaxLength?: number; + }; }; } diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts index 6fedc4df40..c66b5dbae5 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts @@ -16,7 +16,7 @@ import os from 'os'; import { DatabaseManager } from '@backstage/backend-defaults/database'; -import { ConfigReader } from '@backstage/config'; +import { Config, ConfigReader } from '@backstage/config'; import { DatabaseTaskStore } from './DatabaseTaskStore'; import { StorageTaskBroker } from './StorageTaskBroker'; import { TaskWorker, TaskWorkerOptions } from './TaskWorker'; @@ -343,3 +343,58 @@ describe('TaskWorker internals', () => { expect(inflightTasks.length).toBe(2); }); }); + +describe('TaskWorker.truncateParameters', () => { + let worker: TaskWorker; + + beforeEach(async () => { + jest.resetAllMocks(); + + const logger = { debug: jest.fn() } as any; + + const config = { + getOptionalNumber: jest.fn().mockReturnValue(5), + } as unknown as Config; + + worker = await TaskWorker.create({ + logger, + workingDirectory: '/tmp', + integrations: {} as ScmIntegrations, + taskBroker: {} as TaskBroker, + actionRegistry: {} as TemplateActionRegistry, + config, + }); + }); + + it('successfully does nothing', async () => { + const testParams = {}; + + // @ts-expect-error (truncateParameters is private, but for test we can access) + const result = worker.truncateParameters(testParams); + + expect(result).toEqual({}); + }); + + it('truncates long strings in nested objects and arrays', async () => { + const params = { + test: 'short', + test2: 'thisisaverylongstring', + nested: { + test3: 'anotherlongstringhere', + test4: ['ok', 'toolongstring'], + }, + }; + + // @ts-expect-error (truncateParameters is private, but for test we can access) + const result = worker.truncateParameters(params); + + expect(result).toEqual({ + test: 'short', + test2: 'thisi...', + nested: { + test3: 'anoth...', + test4: ['ok', 'toolo...'], + }, + }); + }); +}); diff --git a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts index 40a56e4368..119cfaf180 100644 --- a/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts +++ b/plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts @@ -190,34 +190,44 @@ export class TaskWorker { }); } - protected truncateParameters(parameters: JsonObject) { - const auditMaxLength = - this.config?.getOptionalNumber('scaffolder.auditor.maxLength') ?? 256; + private truncateParameters(parameters: JsonObject) { + const taskParameterMaxLength = + this.config?.getOptionalNumber( + 'scaffolder.auditor.taskParameterMaxLength', + ) ?? 256; - if (auditMaxLength === -1) { + if (taskParameterMaxLength === -1) { this.logger?.debug( - `scaffolder.auditor.maxLength manually disabled via configuration, no task parameter length limit set.`, + `scaffolder.auditor.taskParameterMaxLength manually disabled via configuration, no task parameter length limit set.`, ); return parameters; } - const truncatedParameters: JsonObject = {}; - - for (const key in parameters) { - if (Object.prototype.hasOwnProperty.call(parameters, key)) { - const rawValue = parameters[key]; - const value = rawValue?.toString(); - if (value && value.length > auditMaxLength) { - truncatedParameters[key] = value - .slice(0, auditMaxLength) + function truncate(value: unknown): unknown { + if (typeof value === 'string') { + if (value.length > taskParameterMaxLength) { + return value + .slice(0, taskParameterMaxLength) .concat('...'); - } else { - truncatedParameters[key] = rawValue; } + return value; } + if (Array.isArray(value)) { + return value.map(truncate); + } + if (value && typeof value === 'object') { + const result: Record = {}; + for (const k in value as object) { + if (Object.hasOwn(value, k)) { + result[k] = truncate((value as any)[k]); + } + } + return result; + } + return value; } - return truncatedParameters; + return truncate(parameters) as JsonObject; } async runOneTask(task: TaskContext) {