paulononato/backstage
1
0
Fork 0
Code Issues Pull Requests Actions 31 Packages Projects Releases Wiki Activity

scaffolder-backend: refactor parameter truncation

Browse Source 
Signed-off-by: Patrik Oldsberg <poldsberg@gmail.com>
This commit is contained in:
Patrik Oldsberg
2025-08-12 17:04:19 +02:00
parent 3af9eacf0b
commit 3aeaf2555f
2 changed files with 143 additions and 75 deletions
Download Patch File Download Diff File Expand all files Collapse all files
plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.test.ts
+81 -30
View File
@@ -16,10 +16,14 @@
import os from 'os';
import { DatabaseManager } from '@backstage/backend-defaults/database';
import { Config, ConfigReader } from '@backstage/config';
import { ConfigReader } from '@backstage/config';
import { DatabaseTaskStore } from './DatabaseTaskStore';
import { StorageTaskBroker } from './StorageTaskBroker';
import { TaskWorker, TaskWorkerOptions } from './TaskWorker';
import {
createParameterTruncator,
TaskWorker,
TaskWorkerOptions,
} from './TaskWorker';
import { ScmIntegrations } from '@backstage/integration';
import { TemplateActionRegistry } from '../actions';
import { NunjucksWorkflowRunner } from './NunjucksWorkflowRunner';
@@ -140,6 +144,66 @@ describe('TaskWorker', () => {
const event = events.find(e => e.type === 'completion');
expect(event?.body.output).toEqual({ testOutput: 'testmockoutput' });
});
it('should log an audit event with task parameters when running a task', async () => {
(workflowRunner.execute as jest.Mock).mockResolvedValue({
output: {},
});
const auditor = mockServices.auditor.mock();
const auditEvent = {
success: jest.fn(),
fail: jest.fn(),
};
auditor.createEvent.mockResolvedValue(auditEvent);
const broker = new StorageTaskBroker(storage, logger);
const taskWorker = await TaskWorker.create({
logger,
workingDirectory,
integrations,
taskBroker: broker,
actionRegistry,
auditor,
config: mockServices.rootConfig({
data: {
scaffolder: {
auditor: {
taskParameterMaxLength: 5,
},
},
},
}),
});
await taskWorker.runOneTask({
spec: {
apiVersion: 'scaffolder.backstage.io/v1beta3',
parameters: {
test: 'thisisaverylongstring',
},
steps: [],
output: {},
},
complete: jest.fn(),
createdBy: 'test-creator',
taskId: 'test-id',
} as unknown as TaskContext);
expect(auditor.createEvent).toHaveBeenCalledWith({
eventId: 'task',
severityLevel: 'medium',
meta: {
actionType: 'execution',
createdBy: 'test-creator',
taskId: 'test-id',
taskParameters: {
test: 'thisi...<truncated>',
},
},
});
expect(auditEvent.success).toHaveBeenCalled();
});
});
describe('Concurrent TaskWorker', () => {
@@ -344,33 +408,11 @@ describe('TaskWorker internals', () => {
});
});
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,
});
});
describe('createParameterTruncator', () => {
it('successfully does nothing', async () => {
const testParams = {};
// @ts-expect-error (truncateParameters is private, but for test we can access)
const result = worker.truncateParameters(testParams);
const result = createParameterTruncator()(testParams);
expect(result).toEqual({});
});
@@ -381,19 +423,28 @@ describe('TaskWorker.truncateParameters', () => {
test2: 'thisisaverylongstring',
nested: {
test3: 'anotherlongstringhere',
test4: ['ok', 'toolongstring'],
test4: ['ok', 'toolongstring', { prop: 'thisisaverylongstring' }],
},
};
// @ts-expect-error (truncateParameters is private, but for test we can access)
const result = worker.truncateParameters(params);
const result = createParameterTruncator(
mockServices.rootConfig({
data: {
scaffolder: {
auditor: {
taskParameterMaxLength: 5,
},
},
},
}),
)(params);
expect(result).toEqual({
test: 'short',
test2: 'thisi...<truncated>',
nested: {
test3: 'anoth...<truncated>',
test4: ['ok', 'toolo...<truncated>'],
test4: ['ok', 'toolo...<truncated>', { prop: 'thisi...<truncated>' }],
},
});
});
plugins/scaffolder-backend/src/scaffolder/tasks/TaskWorker.ts
+62 -45
View File
@@ -15,7 +15,7 @@
*/
import { AuditorService, LoggerService } from '@backstage/backend-plugin-api';
import { assertError, stringifyError } from '@backstage/errors';
import { assertError, InputError, stringifyError } from '@backstage/errors';
import { ScmIntegrations } from '@backstage/integration';
import { PermissionEvaluator } from '@backstage/plugin-permission-common';
import {
@@ -32,6 +32,8 @@ import { setTimeout } from 'timers/promises';
import { JsonObject } from '@backstage/types';
import { Config } from '@backstage/config';
const DEFAULT_TASK_PARAMETER_MAX_LENGTH = 256;
/**
* TaskWorkerOptions
* @deprecated this type is deprecated, and there will be a new way to create Workers in the next major version.
@@ -91,17 +93,21 @@ export class TaskWorker {
private taskQueue: PQueue;
private logger: LoggerService | undefined;
private auditor: AuditorService | undefined;
private config: Config | undefined;
private parameterAuditTransform: ParameterAuditTransform;
private stopWorkers: boolean;
private constructor(private readonly options: TaskWorkerOptions) {
private constructor(
private readonly options: TaskWorkerOptions & {
parameterAuditTransform: ParameterAuditTransform;
},
) {
this.stopWorkers = false;
this.logger = options.logger;
this.auditor = options.auditor;
this.config = options.config;
this.taskQueue = new PQueue({
concurrency: options.concurrentTasksLimit,
});
this.parameterAuditTransform = options.parameterAuditTransform;
}
static async create(options: CreateWorkerOptions): Promise<TaskWorker> {
@@ -139,6 +145,7 @@ export class TaskWorker {
auditor,
config,
gracefulShutdown,
parameterAuditTransform: createParameterTruncator(config),
});
}
@@ -190,46 +197,6 @@ export class TaskWorker {
});
}
private truncateParameters(parameters: JsonObject) {
const taskParameterMaxLength =
this.config?.getOptionalNumber(
'scaffolder.auditor.taskParameterMaxLength',
) ?? 256;
if (taskParameterMaxLength === -1) {
this.logger?.debug(
`scaffolder.auditor.taskParameterMaxLength manually disabled via configuration, no task parameter length limit set.`,
);
return parameters;
}
function truncate(value: unknown): unknown {
if (typeof value === 'string') {
if (value.length > taskParameterMaxLength) {
return value
.slice(0, taskParameterMaxLength)
.concat('...<truncated>');
}
return value;
}
if (Array.isArray(value)) {
return value.map(truncate);
}
if (value && typeof value === 'object') {
const result: Record<string, unknown> = {};
for (const k in value as object) {
if (Object.hasOwn(value, k)) {
result[k] = truncate((value as any)[k]);
}
}
return result;
}
return value;
}
return truncate(parameters) as JsonObject;
}
async runOneTask(task: TaskContext) {
const auditorEvent = await this.auditor?.createEvent({
eventId: 'task',
@@ -238,7 +205,7 @@ export class TaskWorker {
actionType: 'execution',
createdBy: task.createdBy,
taskId: task.taskId,
taskParameters: this.truncateParameters(task.spec.parameters),
taskParameters: this.parameterAuditTransform(task.spec.parameters),
templateRef: task.spec.templateInfo?.entityRef,
},
});
@@ -267,3 +234,53 @@ export class TaskWorker {
}
}
}
type ParameterAuditTransform = (parameters: JsonObject) => JsonObject;
/**
* Truncates task parameters for audit logging using the configured max length.
* @internal
*/
export function createParameterTruncator(
config?: Config,
): ParameterAuditTransform {
const maxLength =
config?.getOptionalNumber('scaffolder.auditor.taskParameterMaxLength') ??
DEFAULT_TASK_PARAMETER_MAX_LENGTH;
if (!Number.isSafeInteger(maxLength) || maxLength < -1) {
throw new InputError(
`Invalid configuration for 'scaffolder.auditor.taskParameterMaxLength', got ${maxLength}. Must be a positive integer or -1 to disable truncation.`,
);
}
if (maxLength === -1) {
return (parameters: JsonObject) => parameters;
}
return (parameters: JsonObject) => {
function truncate(value: unknown): unknown {
if (typeof value === 'string') {
if (value.length > maxLength) {
return value.slice(0, maxLength).concat('...<truncated>');
}
return value;
}
if (Array.isArray(value)) {
return value.map(truncate);
}
if (value && typeof value === 'object') {
const result: Record<string, unknown> = {};
for (const k in value as object) {
if (Object.hasOwn(value, k)) {
result[k] = truncate((value as any)[k]);
}
}
return result;
}
return value;
}
return truncate(parameters) as JsonObject;
};
}