fix(core): Stop applying node-defined sensitive output fields to runtime data (#30198)

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Guillaume Jacquart 2026-05-11 10:57:42 +02:00 committed by GitHub
parent 174f0f805e
commit f4e8088cb8
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 7 additions and 72 deletions

View File

@ -15,7 +15,6 @@ import { WorkflowFinderService } from '@/workflows/workflow-finder.service';
import { ExecutionRedactionService } from '../execution-redaction.service';
import { FullItemRedactionStrategy } from '../strategies/full-item-redaction.strategy';
import { NodeDefinedFieldRedactionStrategy } from '../strategies/node-defined-field-redaction.strategy';
describe('ExecutionRedactionService', () => {
const logger = mockInstance(Logger);
@ -23,7 +22,6 @@ describe('ExecutionRedactionService', () => {
const workflowFinderService = mockInstance(WorkflowFinderService);
const eventService = mock<EventService>();
const fullItemRedactionStrategy = mockInstance(FullItemRedactionStrategy);
const nodeDefinedFieldRedactionStrategy = mockInstance(NodeDefinedFieldRedactionStrategy);
let service: ExecutionRedactionService;
@ -44,12 +42,10 @@ describe('ExecutionRedactionService', () => {
workflowFinderService,
eventService,
fullItemRedactionStrategy,
nodeDefinedFieldRedactionStrategy,
);
// Default: user lacks execution:reveal scope
workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue(new Set());
fullItemRedactionStrategy.apply.mockResolvedValue(undefined);
nodeDefinedFieldRedactionStrategy.apply.mockResolvedValue(undefined);
});
const makeExecution = (
@ -195,8 +191,6 @@ describe('ExecutionRedactionService', () => {
// FullItemRedactionStrategy called only for allExecution and nonManualTrigger
expect(fullItemRedactionStrategy.apply).toHaveBeenCalledTimes(2);
// NodeDefinedFieldRedactionStrategy called for all 4
expect(nodeDefinedFieldRedactionStrategy.apply).toHaveBeenCalledTimes(4);
});
it('uses a single DB query for N executions when redactExecutionData === true', async () => {
@ -233,8 +227,6 @@ describe('ExecutionRedactionService', () => {
// FullItemRedactionStrategy.requiresRedaction always returns true when in the pipeline
fullItemRedactionStrategy.requiresRedaction.mockReturnValue(true);
// NodeDefinedFieldRedactionStrategy.requiresRedaction returns false (no sensitive fields)
nodeDefinedFieldRedactionStrategy.requiresRedaction.mockReturnValue(false);
const executions = [noneExecution, allExecution, nonManualManual];
const options: ExecutionRedactionOptions = { user: mockUser, keepOriginal: true };
@ -305,46 +297,6 @@ describe('ExecutionRedactionService', () => {
});
});
describe('NodeDefinedFieldRedactionStrategy inclusion', () => {
it('is always included when redacting', async () => {
const execution = makeExecution({ policy: 'all', mode: 'trigger' });
await service.processExecution(execution, { user: mockUser });
expect(nodeDefinedFieldRedactionStrategy.apply).toHaveBeenCalledTimes(1);
});
it('is included even when policy is "none" (no item clearing)', async () => {
const execution = makeExecution({ policy: 'none', mode: 'trigger' });
await service.processExecution(execution, { user: mockUser });
expect(nodeDefinedFieldRedactionStrategy.apply).toHaveBeenCalledTimes(1);
});
it('is included on reveal path (redactExecutionData === false)', async () => {
workflowFinderService.findWorkflowIdsWithScopeForUser.mockResolvedValue(
new Set(['workflow-123']),
);
const execution = makeExecution({ policy: 'all', mode: 'trigger' });
await service.processExecution(execution, { user: mockUser, redactExecutionData: false });
expect(nodeDefinedFieldRedactionStrategy.apply).toHaveBeenCalledTimes(1);
});
});
describe('strategy ordering', () => {
it('runs FullItemRedactionStrategy before NodeDefinedFieldRedactionStrategy', async () => {
const callOrder: string[] = [];
fullItemRedactionStrategy.apply.mockImplementation(async () => {
callOrder.push('full');
});
nodeDefinedFieldRedactionStrategy.apply.mockImplementation(async () => {
callOrder.push('node-defined');
});
const execution = makeExecution({ policy: 'all', mode: 'trigger' });
await service.processExecution(execution, { user: mockUser });
expect(callOrder).toEqual(['full', 'node-defined']);
});
});
describe('context passed to strategies', () => {
it('passes redactExecutionData from options', async () => {
const execution = makeExecution({ policy: 'all', mode: 'trigger' });
@ -372,22 +324,6 @@ describe('ExecutionRedactionService', () => {
const [, context] = fullItemRedactionStrategy.apply.mock.calls[0];
expect(context.userCanReveal).toBe(false);
});
it('passes userCanReveal: true when policyAllowsReveal (policy=none)', async () => {
const execution = makeExecution({ policy: 'none', mode: 'trigger' });
await service.processExecution(execution, { user: mockUser });
const [, context] = nodeDefinedFieldRedactionStrategy.apply.mock.calls[0];
expect(context.userCanReveal).toBe(true);
});
it('passes userCanReveal: true when policyAllowsReveal (policy=non-manual, mode=manual)', async () => {
const execution = makeExecution({ policy: 'non-manual', mode: 'manual' });
await service.processExecution(execution, { user: mockUser });
const [, context] = nodeDefinedFieldRedactionStrategy.apply.mock.calls[0];
expect(context.userCanReveal).toBe(true);
});
});
describe('reveal path (redactExecutionData === false)', () => {

View File

@ -17,7 +17,6 @@ import type {
RedactionContext,
} from './execution-redaction.interfaces';
import { FullItemRedactionStrategy } from './strategies/full-item-redaction.strategy';
import { NodeDefinedFieldRedactionStrategy } from './strategies/node-defined-field-redaction.strategy';
const MANUAL_MODES: ReadonlySet<WorkflowExecuteMode> = new Set(['manual']);
@ -41,7 +40,6 @@ export class ExecutionRedactionService implements ExecutionRedaction {
private readonly workflowFinderService: WorkflowFinderService,
private readonly eventService: EventService,
private readonly fullItemRedactionStrategy: FullItemRedactionStrategy,
private readonly nodeDefinedFieldRedactionStrategy: NodeDefinedFieldRedactionStrategy,
) {}
async init(): Promise<void> {
@ -123,8 +121,7 @@ export class ExecutionRedactionService implements ExecutionRedaction {
}
// Unified pipeline execution. buildPipeline excludes FullItemRedactionStrategy on the
// reveal path (redactExecutionData === false). NodeDefinedFieldRedactionStrategy
// always runs — node-declared sensitive fields are never revealable.
// reveal path (redactExecutionData === false).
for (let i = 0; i < executions.length; i++) {
const execution = executions[i];
@ -184,8 +181,12 @@ export class ExecutionRedactionService implements ExecutionRedaction {
* explicit redact (`redactExecutionData === true`), policy=all, or
* policy=non-manual on a non-manual execution mode, or dynamic credentials.
* It is never included on the reveal path (`redactExecutionData === false`).
* - `NodeDefinedFieldRedactionStrategy` is always appended last node-declared
* sensitive fields are never revealable.
*
* Note: `NodeDefinedFieldRedactionStrategy` (node-declared `sensitiveOutputFields`)
* is intentionally not wired in here. The previous always-on behaviour broke
* partial/single-step execution because the FE replays the redacted push payload
* back to the server, and is being redesigned. Re-introduce only after the
* product approach (per-workflow gating + partial-run rehydration) is settled.
*/
private buildPipeline(
execution: RedactableExecution,
@ -209,8 +210,6 @@ export class ExecutionRedactionService implements ExecutionRedaction {
pipeline.push(this.fullItemRedactionStrategy);
}
pipeline.push(this.nodeDefinedFieldRedactionStrategy);
return pipeline;
}