feat(core): Emit audit event when execution data is revealed#26425
feat(core): Emit audit event when execution data is revealed#26425
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
No issues found across 8 files
Architecture diagram
sequenceDiagram
participant Client as Client (UI/API)
participant ExecSvc as ExecutionService
participant RedactSvc as ExecutionRedactionService
participant EventSvc as EventService (Internal)
participant Relay as LogStreamingEventRelay
participant AuditBus as EventBus (Audit Layer)
Note over Client,AuditBus: Request to retrieve execution data
Client->>ExecSvc: findOne(id, redactExecutionData)
ExecSvc->>ExecSvc: NEW: Extract req.ip & req.headers['user-agent']
ExecSvc->>RedactSvc: processExecution(execution, options)
RedactSvc->>RedactSvc: Check permissions & Redaction Policy
alt User is Forbidden
RedactSvc-->>ExecSvc: throw ForbiddenError
ExecSvc-->>Client: 403 Forbidden
else User Allowed AND data revealed (redactExecutionData === false)
RedactSvc->>EventSvc: NEW: emit('execution-data-revealed', metadata)
activate EventSvc
EventSvc->>Relay: Trigger Relay Handler
Relay->>AuditBus: NEW: sendAuditEvent('n8n.audit.execution.data.revealed', payload)
deactivate EventSvc
RedactSvc-->>ExecSvc: Return unredacted execution data
ExecSvc-->>Client: 200 OK (Full Data)
else User Allowed BUT data remains redacted
RedactSvc-->>ExecSvc: Return redacted execution data
Note right of RedactSvc: No audit event emitted
ExecSvc-->>Client: 200 OK (Redacted)
end
| } | ||
|
|
||
| private executionDataRevealed({ | ||
| userId, |
There was a problem hiding this comment.
in other events we've used user: UserLike instead of just id which is more useful for debugging purposes (not even sure you can get the user id from the n8n FE)
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/cli/src/events/relays/log-streaming.event-relay.ts">
<violation number="1" location="packages/cli/src/events/relays/log-streaming.event-relay.ts:757">
P2: The execution-data-revealed audit payload now includes the full `user` object (email/firstName/lastName), but this handler is missing `@Redactable()`. This can leak PII into audit/log streaming sinks. Add redaction to align with other audit handlers that include user data.</violation>
</file>
<file name="packages/cli/src/modules/redaction/executions/__tests__/execution-redaction.service.test.ts">
<violation number="1" location="packages/cli/src/modules/redaction/executions/__tests__/execution-redaction.service.test.ts:313">
P2: The new expectation asserts a `user` object instead of the required `userId` field. According to linked Linear issue IAM-188, the audit payload must include `userId`, so this test should enforce that field and the emit payload should carry `userId` rather than a full user object.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| }: RelayEventMap['execution-data-revealed']) { | ||
| void this.eventBus.sendAuditEvent({ | ||
| eventName: 'n8n.audit.execution.data.revealed', | ||
| payload: { ...user, executionId, workflowId, ipAddress, userAgent, redactionPolicy }, |
There was a problem hiding this comment.
P2: The execution-data-revealed audit payload now includes the full user object (email/firstName/lastName), but this handler is missing @Redactable(). This can leak PII into audit/log streaming sinks. Add redaction to align with other audit handlers that include user data.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/events/relays/log-streaming.event-relay.ts, line 757:
<comment>The execution-data-revealed audit payload now includes the full `user` object (email/firstName/lastName), but this handler is missing `@Redactable()`. This can leak PII into audit/log streaming sinks. Add redaction to align with other audit handlers that include user data.</comment>
<file context>
@@ -754,7 +754,7 @@ export class LogStreamingEventRelay extends EventRelay {
void this.eventBus.sendAuditEvent({
eventName: 'n8n.audit.execution.data.revealed',
- payload: { userId, executionId, workflowId, ipAddress, userAgent, redactionPolicy },
+ payload: { ...user, executionId, workflowId, ipAddress, userAgent, redactionPolicy },
});
}
</file context>
| await service.processExecution(execution, options); | ||
|
|
||
| expect(eventService.emit).toHaveBeenCalledWith('execution-data-revealed', { | ||
| user: mockUser, |
There was a problem hiding this comment.
P2: The new expectation asserts a user object instead of the required userId field. According to linked Linear issue IAM-188, the audit payload must include userId, so this test should enforce that field and the emit payload should carry userId rather than a full user object.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/redaction/executions/__tests__/execution-redaction.service.test.ts, line 313:
<comment>The new expectation asserts a `user` object instead of the required `userId` field. According to linked Linear issue IAM-188, the audit payload must include `userId`, so this test should enforce that field and the emit payload should carry `userId` rather than a full user object.</comment>
<file context>
@@ -310,7 +310,7 @@ describe('ExecutionRedactionService', () => {
expect(eventService.emit).toHaveBeenCalledWith('execution-data-revealed', {
- userId: mockUser.id,
+ user: mockUser,
executionId: execution.id,
workflowId: execution.workflowId,
</file context>
There was a problem hiding this comment.
don't reference ticket criteria in PRs
There was a problem hiding this comment.
Thanks for the feedback! I've saved this as a new learning to improve future reviews.
Summary
Implements audit event emission for the
n8n.audit.execution.data.revealedevent, completing the audit trail for the execution data reveal feature.What changed:
'n8n.audit.execution.data.revealed'to theeventNamesAuditarray and four new optional fields (executionId,ipAddress,userAgent,redactionPolicy) toEventPayloadAudit— the eventbus audit layer'execution-data-revealed'entry toRelayEventMapwith fully typed, required fields:userId,executionId,workflowId,ipAddress,userAgent,redactionPolicyEventServiceintoExecutionRedactionServiceand emits'execution-data-revealed'in theredactExecutionData === false && allowedbranch — the exact point where unredacted data is returned to the caller. The event is not emitted when the request is denied (ForbiddenError)ExecutionRedactionOptionswith optionalipAddressanduserAgentfields; updatedExecutionService.findOne()to forwardreq.ipandreq.headers['user-agent']into the optionsexecutionDataRevealedrelay handler inLogStreamingEventRelayto bridge the relay event tosendAuditEvent('n8n.audit.execution.data.revealed', ...)Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/IAM-189
https://linear.app/n8n/issue/IAM-188
Review / Merge checklist
release/backport(if the PR is an urgent fix that needs to be backported)