From d4bb69aee26a9d596a17754b23a768680d6a7b1e Mon Sep 17 00:00:00 2001 From: Stephen Wright Date: Wed, 3 Jun 2026 13:29:42 +0100 Subject: [PATCH] refactor(core): Store redaction enforcement as a floor enum natively (#31629) --- .../api-types/src/redaction-enforcement.ts | 16 +-- ...0025-MigrateRedactionEnforcementToFloor.ts | 99 +++++++++++++++ .../db/src/migrations/postgresdb/index.ts | 2 + .../@n8n/db/src/migrations/sqlite/index.ts | 2 + .../security-settings.controller.ts | 17 +-- .../log-streaming-event-relay.test.ts | 26 ++-- .../cli/src/events/maps/relay.event-map.ts | 10 +- ...ance-redaction-enforcement.service.test.ts | 54 ++++----- ...redaction-context-hook.integration.test.ts | 2 +- .../__tests__/redaction-context-hook.test.ts | 2 +- .../redaction-enforcement-mapper.test.ts | 61 ---------- .../redaction-enforcement.service.test.ts | 6 +- .../instance-redaction-enforcement.service.ts | 48 +++----- .../redaction/redaction-context-hook.ts | 2 +- .../redaction/redaction-enforcement-mapper.ts | 27 ----- .../redaction-enforcement.service.ts | 9 +- .../security-settings.controller.test.ts | 78 +++--------- ...ate-redaction-enforcement-to-floor.test.ts | 113 ++++++++++++++++++ 18 files changed, 300 insertions(+), 274 deletions(-) create mode 100644 packages/@n8n/db/src/migrations/common/1784000000025-MigrateRedactionEnforcementToFloor.ts delete mode 100644 packages/cli/src/modules/redaction/__tests__/redaction-enforcement-mapper.test.ts delete mode 100644 packages/cli/src/modules/redaction/redaction-enforcement-mapper.ts create mode 100644 packages/cli/test/migration/1784000000025-migrate-redaction-enforcement-to-floor.test.ts diff --git a/packages/@n8n/api-types/src/redaction-enforcement.ts b/packages/@n8n/api-types/src/redaction-enforcement.ts index 1bff272c5b4..1bbbe872cc2 100644 --- a/packages/@n8n/api-types/src/redaction-enforcement.ts +++ b/packages/@n8n/api-types/src/redaction-enforcement.ts @@ -1,15 +1,3 @@ -import { z } from 'zod'; +import type { RedactionFloor } from './redaction-enforcement-floor'; -export const redactionEnforcementSettingsSchema = z.object({ - enforced: z.boolean(), - manual: z.boolean(), - production: z.boolean(), -}); - -export type RedactionEnforcementSettings = z.infer; - -export const REDACTION_ENFORCEMENT_DEFAULTS: RedactionEnforcementSettings = { - enforced: false, - manual: false, - production: false, -}; +export const REDACTION_FLOOR_DEFAULT: RedactionFloor = 'off'; diff --git a/packages/@n8n/db/src/migrations/common/1784000000025-MigrateRedactionEnforcementToFloor.ts b/packages/@n8n/db/src/migrations/common/1784000000025-MigrateRedactionEnforcementToFloor.ts new file mode 100644 index 00000000000..5558ba75083 --- /dev/null +++ b/packages/@n8n/db/src/migrations/common/1784000000025-MigrateRedactionEnforcementToFloor.ts @@ -0,0 +1,99 @@ +import { jsonParse } from 'n8n-workflow'; + +import type { IrreversibleMigration, MigrationContext } from '../migration-types'; + +const REDACTION_ENFORCEMENT_KEY = 'redaction.enforcement'; + +type RedactionFloor = 'off' | 'production' | 'all'; + +const FLOORS: readonly RedactionFloor[] = ['off', 'production', 'all']; + +interface BooleanSettings { + enforced: boolean; + manual: boolean; + production: boolean; +} + +/** + * Migrates the stored redaction enforcement setting from the boolean triple + * `{ enforced, manual, production }` to the floor enum `'off' | 'production' | 'all'`. + * + * Only the three combinations the API could produce map to a floor; every other + * (impossible) combination normalises up to the strictest `'all'` — never weaker + * than what was stored: + * - `enforced: false` -> 'off' + * - `{ enforced: true, manual: false, production: true }` -> 'production' + * - `{ enforced: true, manual: true, production: true }` -> 'all' + * - anything else (impossible tuple, unparseable, malformed) -> 'all' + * + * The value is stored as a JSON-encoded string (e.g. the literal `"off"`), matching + * how the service serialises it, so the read path stays unchanged. + * + * Irreversible: collapsing three booleans into one enum is lossy, so there is no + * faithful `down()`. + * + * Compatible with SQLite and PostgreSQL. + */ +export class MigrateRedactionEnforcementToFloor1784000000025 implements IrreversibleMigration { + async up({ escape, runQuery, logger, migrationName }: MigrationContext) { + const settingsTable = escape.tableName('settings'); + const keyCol = escape.columnName('key'); + const valueCol = escape.columnName('value'); + + const rows: Array<{ value: string }> = await runQuery( + `SELECT ${valueCol} AS value FROM ${settingsTable} WHERE ${keyCol} = :key;`, + { key: REDACTION_ENFORCEMENT_KEY }, + ); + + if (rows.length === 0) { + logger.info( + `[${migrationName}] No stored redaction enforcement setting found, nothing to migrate`, + ); + return; + } + + const floor = this.toFloor(rows[0].value); + const serialized = JSON.stringify(floor); + + await runQuery(`UPDATE ${settingsTable} SET ${valueCol} = :value WHERE ${keyCol} = :key;`, { + value: serialized, + key: REDACTION_ENFORCEMENT_KEY, + }); + + logger.info(`[${migrationName}] Migrated redaction enforcement setting to floor='${floor}'`); + } + + private toFloor(raw: string): RedactionFloor { + let parsed: unknown; + try { + parsed = jsonParse(raw); + } catch { + // Unparseable value — fall back to the strictest floor. + return 'all'; + } + + // Already migrated (e.g. a re-run): leave the existing floor untouched. + if (typeof parsed === 'string' && FLOORS.includes(parsed as RedactionFloor)) { + return parsed as RedactionFloor; + } + + if (!this.isBooleanSettings(parsed)) return 'all'; + + // Map only the API-producible combinations; every other (impossible) + // enforced combination — including `{ manual: false, production: false }` + // and `{ manual: true, production: false }` — normalises up to 'all'. + if (!parsed.enforced) return 'off'; + if (!parsed.manual && parsed.production) return 'production'; + return 'all'; + } + + private isBooleanSettings(value: unknown): value is BooleanSettings { + if (value === null || typeof value !== 'object') return false; + const candidate = value as Record; + return ( + typeof candidate.enforced === 'boolean' && + typeof candidate.manual === 'boolean' && + typeof candidate.production === 'boolean' + ); + } +} diff --git a/packages/@n8n/db/src/migrations/postgresdb/index.ts b/packages/@n8n/db/src/migrations/postgresdb/index.ts index 64adec81ac8..d40d108ac75 100644 --- a/packages/@n8n/db/src/migrations/postgresdb/index.ts +++ b/packages/@n8n/db/src/migrations/postgresdb/index.ts @@ -200,6 +200,7 @@ import { CreateAgentTaskDefinitionTable1784000000021 } from '../common/178400000 import { AddSubAgentLinkageToAgentExecutionThreads1784000000022 } from '../common/1784000000022-AddSubAgentLinkageToAgentExecutionThreads'; import { CreateInstanceAiMcpRegistryConnectionTable1784000000023 } from '../common/1784000000023-CreateInstanceAiMcpRegistryConnectionTable'; import { AddResourceToOAuthAuthorizationCodes1784000000024 } from '../common/1784000000024-AddResourceToOAuthAuthorizationCodes'; +import { MigrateRedactionEnforcementToFloor1784000000025 } from '../common/1784000000025-MigrateRedactionEnforcementToFloor'; import type { Migration } from '../migration-types'; export const postgresMigrations: Migration[] = [ @@ -405,4 +406,5 @@ export const postgresMigrations: Migration[] = [ AddSubAgentLinkageToAgentExecutionThreads1784000000022, CreateInstanceAiMcpRegistryConnectionTable1784000000023, AddResourceToOAuthAuthorizationCodes1784000000024, + MigrateRedactionEnforcementToFloor1784000000025, ]; diff --git a/packages/@n8n/db/src/migrations/sqlite/index.ts b/packages/@n8n/db/src/migrations/sqlite/index.ts index 66ca5209005..34a132ed0d9 100644 --- a/packages/@n8n/db/src/migrations/sqlite/index.ts +++ b/packages/@n8n/db/src/migrations/sqlite/index.ts @@ -194,6 +194,7 @@ import { CreateInstanceAiMcpRegistryConnectionTable1784000000023 } from '../comm import { AddResourceToOAuthAuthorizationCodes1784000000024 } from '../common/1784000000024-AddResourceToOAuthAuthorizationCodes'; import type { Migration } from '../migration-types'; import { CreateAgentTaskDefinitionTable1784000000021 } from './1784000000021-CreateAgentTaskDefinitionTable'; +import { MigrateRedactionEnforcementToFloor1784000000025 } from '../common/1784000000025-MigrateRedactionEnforcementToFloor'; const sqliteMigrations: Migration[] = [ InitialMigration1588102412422, @@ -391,6 +392,7 @@ const sqliteMigrations: Migration[] = [ AddSubAgentLinkageToAgentExecutionThreads1784000000022, CreateInstanceAiMcpRegistryConnectionTable1784000000023, AddResourceToOAuthAuthorizationCodes1784000000024, + MigrateRedactionEnforcementToFloor1784000000025, ]; export { sqliteMigrations }; diff --git a/packages/cli/src/controllers/security-settings.controller.ts b/packages/cli/src/controllers/security-settings.controller.ts index e8502548ec6..d7b33dafce3 100644 --- a/packages/cli/src/controllers/security-settings.controller.ts +++ b/packages/cli/src/controllers/security-settings.controller.ts @@ -11,7 +11,6 @@ import type { Response } from 'express'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { EventService } from '@/events/event.service'; import { InstanceRedactionEnforcementService } from '@/modules/redaction/instance-redaction-enforcement.service'; -import { floorToSettings, settingsToFloor } from '@/modules/redaction/redaction-enforcement-mapper'; import { isRedactionEnforcementEnabled } from '@/modules/redaction/redaction-enforcement.feature-flag'; import { SecuritySettingsService } from '@/services/security-settings.service'; @@ -46,11 +45,7 @@ export class SecuritySettingsController { : Promise.resolve(undefined), ]); - // API surface uses a single `floor` enum, while the service stores the - // three booleans the cache layer was built around. Translate at the boundary. - const redactionEnforcement = redactionSettings - ? { floor: settingsToFloor(redactionSettings) } - : undefined; + const redactionEnforcement = redactionSettings ? { floor: redactionSettings } : undefined; return { ...settings, @@ -96,13 +91,9 @@ export class SecuritySettingsController { if (dto.redactionEnforcement !== undefined && isRedactionEnforcementEnabled()) { const before = await this.instanceRedactionEnforcementService.get(); - const after = floorToSettings(dto.redactionEnforcement.floor); - updatedSettings.redactionEnforcement = { floor: dto.redactionEnforcement.floor }; - if ( - before.enforced !== after.enforced || - before.manual !== after.manual || - before.production !== after.production - ) { + const after = dto.redactionEnforcement.floor; + updatedSettings.redactionEnforcement = { floor: after }; + if (before !== after) { await this.instanceRedactionEnforcementService.set(after); this.eventService.emit('redaction-enforcement-updated', { user: { diff --git a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts index aeb670fc4c9..34af36798cf 100644 --- a/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts +++ b/packages/cli/src/events/__tests__/log-streaming-event-relay.test.ts @@ -2668,8 +2668,8 @@ describe('LogStreamingEventRelay', () => { lastName: 'Admin', role: { slug: 'global:owner' }, }, - before: { enforced: false, manual: false, production: false }, - after: { enforced: true, manual: false, production: true }, + before: 'off', + after: 'production', }; eventService.emit('redaction-enforcement-updated', event); @@ -2682,8 +2682,8 @@ describe('LogStreamingEventRelay', () => { _firstName: 'Seventh', _lastName: 'Admin', globalRole: 'global:owner', - before: { enforced: false, manual: false, production: false }, - after: { enforced: true, manual: false, production: true }, + before: 'off', + after: 'production', }, }); }); @@ -2697,8 +2697,8 @@ describe('LogStreamingEventRelay', () => { lastName: 'Admin', role: { slug: 'global:owner' }, }, - before: { enforced: true, manual: true, production: true }, - after: { enforced: true, manual: false, production: true }, + before: 'all', + after: 'production', }; eventService.emit('redaction-enforcement-updated', event); @@ -2711,13 +2711,13 @@ describe('LogStreamingEventRelay', () => { _firstName: 'Seventh', _lastName: 'Admin', globalRole: 'global:owner', - before: { enforced: true, manual: true, production: true }, - after: { enforced: true, manual: false, production: true }, + before: 'all', + after: 'production', }, }); }); - it('should log `redaction-enforcement.updated` for upgrade to `all` (full-tuple passthrough)', () => { + it('should log `redaction-enforcement.updated` for upgrade to `all`', () => { const event: RelayEventMap['redaction-enforcement-updated'] = { user: { id: 'user404', @@ -2726,8 +2726,8 @@ describe('LogStreamingEventRelay', () => { lastName: 'Admin', role: { slug: 'global:owner' }, }, - before: { enforced: true, manual: false, production: true }, - after: { enforced: true, manual: true, production: true }, + before: 'production', + after: 'all', }; eventService.emit('redaction-enforcement-updated', event); @@ -2740,8 +2740,8 @@ describe('LogStreamingEventRelay', () => { _firstName: 'Seventh', _lastName: 'Admin', globalRole: 'global:owner', - before: { enforced: true, manual: false, production: true }, - after: { enforced: true, manual: true, production: true }, + before: 'production', + after: 'all', }, }); }); diff --git a/packages/cli/src/events/maps/relay.event-map.ts b/packages/cli/src/events/maps/relay.event-map.ts index f7b690d04a6..27bb98bea3a 100644 --- a/packages/cli/src/events/maps/relay.event-map.ts +++ b/packages/cli/src/events/maps/relay.event-map.ts @@ -1,8 +1,4 @@ -import type { - AuthenticationMethod, - ProjectRelation, - RedactionEnforcementSettings, -} from '@n8n/api-types'; +import type { AuthenticationMethod, ProjectRelation, RedactionFloor } from '@n8n/api-types'; import type { AuthProviderType, User, IWorkflowDb } from '@n8n/db'; import type { CancellationReason, @@ -990,8 +986,8 @@ export type RelayEventMap = { 'redaction-enforcement-updated': { user: UserLike; - before: RedactionEnforcementSettings; - after: RedactionEnforcementSettings; + before: RedactionFloor; + after: RedactionFloor; }; // #endregion diff --git a/packages/cli/src/modules/redaction/__tests__/instance-redaction-enforcement.service.test.ts b/packages/cli/src/modules/redaction/__tests__/instance-redaction-enforcement.service.test.ts index 8545ef741f2..346974bc00e 100644 --- a/packages/cli/src/modules/redaction/__tests__/instance-redaction-enforcement.service.test.ts +++ b/packages/cli/src/modules/redaction/__tests__/instance-redaction-enforcement.service.test.ts @@ -1,4 +1,4 @@ -import { REDACTION_ENFORCEMENT_DEFAULTS } from '@n8n/api-types'; +import { REDACTION_FLOOR_DEFAULT, type RedactionFloor } from '@n8n/api-types'; import type { Logger } from '@n8n/backend-common'; import type { Settings, SettingsRepository } from '@n8n/db'; import { mock } from 'jest-mock-extended'; @@ -64,8 +64,8 @@ describe('InstanceRedactionEnforcementService', () => { describe('with feature flag off', () => { beforeEach(() => disableFlag()); - it('returns defaults without touching cache or repository', async () => { - await expect(service.get()).resolves.toEqual(REDACTION_ENFORCEMENT_DEFAULTS); + it('returns the default floor without touching cache or repository', async () => { + await expect(service.get()).resolves.toBe(REDACTION_FLOOR_DEFAULT); expect(cacheService.get).not.toHaveBeenCalled(); expect(findByKey).not.toHaveBeenCalled(); expect(cacheService.set).not.toHaveBeenCalled(); @@ -75,17 +75,17 @@ describe('InstanceRedactionEnforcementService', () => { describe('with feature flag on', () => { beforeEach(() => enableFlag()); - it('returns parsed value from cache on hit without reading DB', async () => { - const cached = { enforced: true, manual: true, production: true }; + it('returns parsed floor from cache on hit without reading DB', async () => { + const cached: RedactionFloor = 'all'; simulateCacheHit(JSON.stringify(cached)); - await expect(service.get()).resolves.toEqual(cached); + await expect(service.get()).resolves.toBe(cached); expect(findByKey).not.toHaveBeenCalled(); expect(cacheService.set).not.toHaveBeenCalled(); }); it('falls back to DB, returns row, and backfills cache on cache miss', async () => { - const stored = { enforced: true, manual: false, production: true }; + const stored: RedactionFloor = 'production'; simulateCacheMiss(); findByKey.mockResolvedValueOnce( mock({ @@ -95,26 +95,23 @@ describe('InstanceRedactionEnforcementService', () => { }), ); - await expect(service.get()).resolves.toEqual(stored); + await expect(service.get()).resolves.toBe(stored); expect(findByKey).toHaveBeenCalledWith(KEY); expect(cacheService.set).toHaveBeenCalledWith(KEY, JSON.stringify(stored)); }); - it('returns defaults and backfills cache when no DB row exists', async () => { + it('returns the default floor and backfills cache when no DB row exists', async () => { simulateCacheMiss(); findByKey.mockResolvedValueOnce(null); - await expect(service.get()).resolves.toEqual(REDACTION_ENFORCEMENT_DEFAULTS); - expect(cacheService.set).toHaveBeenCalledWith( - KEY, - JSON.stringify(REDACTION_ENFORCEMENT_DEFAULTS), - ); + await expect(service.get()).resolves.toBe(REDACTION_FLOOR_DEFAULT); + expect(cacheService.set).toHaveBeenCalledWith(KEY, JSON.stringify(REDACTION_FLOOR_DEFAULT)); }); - it('returns defaults when cache has invalid JSON, and logs a warning', async () => { + it('returns the default floor when cache has invalid JSON, and logs a warning', async () => { simulateCacheHit('not-json'); - await expect(service.get()).resolves.toEqual(REDACTION_ENFORCEMENT_DEFAULTS); + await expect(service.get()).resolves.toBe(REDACTION_FLOOR_DEFAULT); expect(logger.warn).toHaveBeenCalledWith( 'Failed to parse redaction enforcement setting JSON', expect.objectContaining({ source: 'cache' }), @@ -122,25 +119,22 @@ describe('InstanceRedactionEnforcementService', () => { expect(findByKey).not.toHaveBeenCalled(); }); - it('returns defaults and backfills cache when DB value has an invalid shape, and logs a warning', async () => { + it('returns the default floor and backfills cache when DB value has an invalid shape, and logs a warning', async () => { simulateCacheMiss(); findByKey.mockResolvedValueOnce( mock({ key: KEY, - value: JSON.stringify({ enforced: 'yes', manual: false, production: true }), + value: JSON.stringify('bogus'), loadOnStartup: true, }), ); - await expect(service.get()).resolves.toEqual(REDACTION_ENFORCEMENT_DEFAULTS); + await expect(service.get()).resolves.toBe(REDACTION_FLOOR_DEFAULT); expect(logger.warn).toHaveBeenCalledWith( 'Redaction enforcement setting has an invalid shape', expect.objectContaining({ source: 'database' }), ); - expect(cacheService.set).toHaveBeenCalledWith( - KEY, - JSON.stringify(REDACTION_ENFORCEMENT_DEFAULTS), - ); + expect(cacheService.set).toHaveBeenCalledWith(KEY, JSON.stringify(REDACTION_FLOOR_DEFAULT)); }); }); }); @@ -150,9 +144,7 @@ describe('InstanceRedactionEnforcementService', () => { beforeEach(() => disableFlag()); it('throws OperationalError without touching cache or repository', async () => { - await expect( - service.set({ enforced: true, manual: true, production: true }), - ).rejects.toThrow(OperationalError); + await expect(service.set('all')).rejects.toThrow(OperationalError); expect(upsert).not.toHaveBeenCalled(); expect(cacheService.set).not.toHaveBeenCalled(); }); @@ -161,8 +153,8 @@ describe('InstanceRedactionEnforcementService', () => { describe('with feature flag on', () => { beforeEach(() => enableFlag()); - it('upserts the setting and writes the same serialized value to cache', async () => { - const next = { enforced: true, manual: false, production: true }; + it('upserts the floor and writes the same serialized value to cache', async () => { + const next: RedactionFloor = 'production'; await service.set(next); @@ -174,11 +166,7 @@ describe('InstanceRedactionEnforcementService', () => { }); it('rejects invalid input with a UserError and logs validation issues', async () => { - const invalid = { enforced: true, manual: 'yes', production: true } as unknown as { - enforced: boolean; - manual: boolean; - production: boolean; - }; + const invalid = 'bogus' as unknown as RedactionFloor; await expect(service.set(invalid)).rejects.toThrow(UserError); expect(logger.warn).toHaveBeenCalledWith( diff --git a/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.integration.test.ts b/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.integration.test.ts index 44acda29688..742378b448e 100644 --- a/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.integration.test.ts +++ b/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.integration.test.ts @@ -82,7 +82,7 @@ describe('RedactionContextHook integration with establishExecutionContext', () = floor: 'off' | 'production' | 'all', workflowPolicy?: WorkflowSettings.RedactionPolicy, ) => { - enforcementService.getFloor.mockResolvedValue(floor); + enforcementService.get.mockResolvedValue(floor); const workflow = buildWorkflow(workflowPolicy); const runExecutionData = buildRunExecutionData(); diff --git a/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.test.ts b/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.test.ts index b4a3343a3a7..98860902c05 100644 --- a/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.test.ts +++ b/packages/cli/src/modules/redaction/__tests__/redaction-context-hook.test.ts @@ -21,7 +21,7 @@ describe('RedactionContextHook', () => { }); const setFloor = (floor: RedactionFloor) => { - service.getFloor.mockResolvedValue(floor); + service.get.mockResolvedValue(floor); }; const expectChannels = (production: boolean, manual: boolean) => ({ diff --git a/packages/cli/src/modules/redaction/__tests__/redaction-enforcement-mapper.test.ts b/packages/cli/src/modules/redaction/__tests__/redaction-enforcement-mapper.test.ts deleted file mode 100644 index aeca7325a05..00000000000 --- a/packages/cli/src/modules/redaction/__tests__/redaction-enforcement-mapper.test.ts +++ /dev/null @@ -1,61 +0,0 @@ -import { floorToSettings, settingsToFloor } from '../redaction-enforcement-mapper'; - -describe('redaction-enforcement-mapper', () => { - describe('floorToSettings', () => { - it('maps off → all booleans false', () => { - expect(floorToSettings('off')).toEqual({ - enforced: false, - manual: false, - production: false, - }); - }); - - it('maps production → enforced + production only', () => { - expect(floorToSettings('production')).toEqual({ - enforced: true, - manual: false, - production: true, - }); - }); - - it('maps all → enforced + manual + production', () => { - expect(floorToSettings('all')).toEqual({ - enforced: true, - manual: true, - production: true, - }); - }); - }); - - describe('settingsToFloor', () => { - it('maps {enforced: false} → off regardless of other flags', () => { - expect(settingsToFloor({ enforced: false, manual: false, production: false })).toBe('off'); - expect(settingsToFloor({ enforced: false, manual: true, production: true })).toBe('off'); - }); - - it('maps {enforced, production} → production', () => { - expect(settingsToFloor({ enforced: true, manual: false, production: true })).toBe( - 'production', - ); - }); - - it('maps {enforced, manual, production} → all', () => { - expect(settingsToFloor({ enforced: true, manual: true, production: true })).toBe('all'); - }); - - it('normalizes upward when manual is true but production is false', () => { - // Combination unreachable via the API; defensive fallback returns the - // stricter floor so callers never see a weaker floor than what is stored. - expect(settingsToFloor({ enforced: true, manual: true, production: false })).toBe('all'); - }); - }); - - describe('round-trip', () => { - it.each(['off', 'production', 'all'] as const)( - 'floor → settings → floor preserves %s', - (floor) => { - expect(settingsToFloor(floorToSettings(floor))).toBe(floor); - }, - ); - }); -}); diff --git a/packages/cli/src/modules/redaction/__tests__/redaction-enforcement.service.test.ts b/packages/cli/src/modules/redaction/__tests__/redaction-enforcement.service.test.ts index 5e59c3b2f47..ec594ceb83a 100644 --- a/packages/cli/src/modules/redaction/__tests__/redaction-enforcement.service.test.ts +++ b/packages/cli/src/modules/redaction/__tests__/redaction-enforcement.service.test.ts @@ -1,8 +1,7 @@ -import type { RedactionEnforcementSettings, RedactionFloor } from '@n8n/api-types'; +import type { RedactionFloor } from '@n8n/api-types'; import { mock } from 'jest-mock-extended'; import type { WorkflowSettings } from 'n8n-workflow'; -import { floorToSettings } from '../redaction-enforcement-mapper'; import { UnprocessableRequestError } from '@/errors/response-errors/unprocessable.error'; import type { InstanceRedactionEnforcementService } from '../instance-redaction-enforcement.service'; @@ -11,8 +10,7 @@ import { RedactionEnforcementService } from '../redaction-enforcement.service'; describe('RedactionEnforcementService', () => { function createService(floor: RedactionFloor) { const instanceRedactionEnforcementService = mock(); - const settings: RedactionEnforcementSettings = floorToSettings(floor); - instanceRedactionEnforcementService.get.mockResolvedValue(settings); + instanceRedactionEnforcementService.get.mockResolvedValue(floor); return new RedactionEnforcementService(instanceRedactionEnforcementService); } diff --git a/packages/cli/src/modules/redaction/instance-redaction-enforcement.service.ts b/packages/cli/src/modules/redaction/instance-redaction-enforcement.service.ts index 207093bf081..10a9c65edf2 100644 --- a/packages/cli/src/modules/redaction/instance-redaction-enforcement.service.ts +++ b/packages/cli/src/modules/redaction/instance-redaction-enforcement.service.ts @@ -1,9 +1,4 @@ -import { - REDACTION_ENFORCEMENT_DEFAULTS, - redactionEnforcementSettingsSchema, - type RedactionEnforcementSettings, - type RedactionFloor, -} from '@n8n/api-types'; +import { REDACTION_FLOOR_DEFAULT, redactionFloorSchema, type RedactionFloor } from '@n8n/api-types'; import { Logger } from '@n8n/backend-common'; import { SettingsRepository } from '@n8n/db'; import { Service } from '@n8n/di'; @@ -11,7 +6,6 @@ import { OperationalError, UserError } from 'n8n-workflow'; import { CacheService } from '@/services/cache/cache.service'; -import { settingsToFloor } from './redaction-enforcement-mapper'; import { isRedactionEnforcementEnabled } from './redaction-enforcement.feature-flag'; const KEY = 'redaction.enforcement'; @@ -24,36 +18,31 @@ export class InstanceRedactionEnforcementService { private readonly logger: Logger, ) {} - async get(): Promise { - if (!isRedactionEnforcementEnabled()) return REDACTION_ENFORCEMENT_DEFAULTS; + /** + * Resolves the instance redaction floor. Returns `'off'` when enforcement is + * disabled or no value is stored. The floor is stored as the enum directly, + * so no translation is needed. + */ + async get(): Promise { + if (!isRedactionEnforcementEnabled()) return REDACTION_FLOOR_DEFAULT; return await this.load(); } - /** - * Resolves the instance redaction floor as a `RedactionFloor` enum. - * Returns `'off'` when enforcement is disabled. Normalizes any stored - * setting upward via `settingsToFloor`, so the floor never reports a - * weaker level than what is stored. - */ - async getFloor(): Promise { - return settingsToFloor(await this.get()); - } - - private async load(): Promise { + private async load(): Promise { const raw = await this.cacheService.get(KEY, { refreshFn: async () => await this.loadFromDatabase(), }); - if (raw === undefined) return REDACTION_ENFORCEMENT_DEFAULTS; - return this.parseStoredValue(raw, 'cache') ?? REDACTION_ENFORCEMENT_DEFAULTS; + if (raw === undefined) return REDACTION_FLOOR_DEFAULT; + return this.parseStoredValue(raw, 'cache') ?? REDACTION_FLOOR_DEFAULT; } - async set(next: RedactionEnforcementSettings): Promise { + async set(next: RedactionFloor): Promise { if (!isRedactionEnforcementEnabled()) { throw new OperationalError('Redaction enforcement is not enabled on this instance'); } - const result = redactionEnforcementSettingsSchema.safeParse(next); + const result = redactionFloorSchema.safeParse(next); if (!result.success) { this.logger.warn('Invalid redaction enforcement settings payload', { issues: result.error.issues, @@ -74,15 +63,12 @@ export class InstanceRedactionEnforcementService { const row = await this.settingsRepository.findByKey(KEY); const value = row?.value !== undefined - ? (this.parseStoredValue(row.value, 'database') ?? REDACTION_ENFORCEMENT_DEFAULTS) - : REDACTION_ENFORCEMENT_DEFAULTS; + ? (this.parseStoredValue(row.value, 'database') ?? REDACTION_FLOOR_DEFAULT) + : REDACTION_FLOOR_DEFAULT; return JSON.stringify(value); } - private parseStoredValue( - raw: string, - source: 'cache' | 'database', - ): RedactionEnforcementSettings | undefined { + private parseStoredValue(raw: string, source: 'cache' | 'database'): RedactionFloor | undefined { let parsedJson: unknown; try { parsedJson = JSON.parse(raw); @@ -94,7 +80,7 @@ export class InstanceRedactionEnforcementService { return undefined; } - const result = redactionEnforcementSettingsSchema.safeParse(parsedJson); + const result = redactionFloorSchema.safeParse(parsedJson); if (!result.success) { this.logger.warn('Redaction enforcement setting has an invalid shape', { source, diff --git a/packages/cli/src/modules/redaction/redaction-context-hook.ts b/packages/cli/src/modules/redaction/redaction-context-hook.ts index 8435a25af87..ab0ab2036fd 100644 --- a/packages/cli/src/modules/redaction/redaction-context-hook.ts +++ b/packages/cli/src/modules/redaction/redaction-context-hook.ts @@ -34,7 +34,7 @@ export class RedactionContextHook implements IContextEstablishmentHook { * A workflow can be equal to or stricter than the floor, never weaker. */ async execute(options: ContextEstablishmentOptions): Promise { - const floor = await this.instanceRedactionEnforcementService.getFloor(); + const floor = await this.instanceRedactionEnforcementService.get(); const workflow = policyToChannels(options.workflow.settings?.redactionPolicy ?? 'none'); const floorEnforcesProduction = floor !== 'off'; diff --git a/packages/cli/src/modules/redaction/redaction-enforcement-mapper.ts b/packages/cli/src/modules/redaction/redaction-enforcement-mapper.ts deleted file mode 100644 index a8d87f2847a..00000000000 --- a/packages/cli/src/modules/redaction/redaction-enforcement-mapper.ts +++ /dev/null @@ -1,27 +0,0 @@ -import type { RedactionEnforcementSettings, RedactionFloor } from '@n8n/api-types'; -import { UnexpectedError } from 'n8n-workflow'; - -import { assertNever } from '@/utils'; - -export function floorToSettings(floor: RedactionFloor): RedactionEnforcementSettings { - switch (floor) { - case 'off': - return { enforced: false, manual: false, production: false }; - case 'production': - return { enforced: true, manual: false, production: true }; - case 'all': - return { enforced: true, manual: true, production: true }; - default: - assertNever(floor); - throw new UnexpectedError(`Unknown redaction floor: ${String(floor)}`); - } -} - -export function settingsToFloor(settings: RedactionEnforcementSettings): RedactionFloor { - if (!settings.enforced) return 'off'; - // `manual` implies the strictest stored level reachable via this API. - // Combinations the API cannot produce (e.g. `{manual: true, production: false}`) - // normalize upward — never report a weaker floor than what is stored. - if (settings.manual) return 'all'; - return 'production'; -} diff --git a/packages/cli/src/modules/redaction/redaction-enforcement.service.ts b/packages/cli/src/modules/redaction/redaction-enforcement.service.ts index a39e0d5087d..b79210f8908 100644 --- a/packages/cli/src/modules/redaction/redaction-enforcement.service.ts +++ b/packages/cli/src/modules/redaction/redaction-enforcement.service.ts @@ -1,7 +1,6 @@ import { Service } from '@n8n/di'; import type { WorkflowSettings } from 'n8n-workflow'; -import { settingsToFloor } from './redaction-enforcement-mapper'; import { UnprocessableRequestError } from '@/errors/response-errors/unprocessable.error'; import { InstanceRedactionEnforcementService } from './instance-redaction-enforcement.service'; @@ -22,11 +21,6 @@ export class RedactionEnforcementService { private readonly instanceRedactionEnforcementService: InstanceRedactionEnforcementService, ) {} - private async getFloor() { - const settings = await this.instanceRedactionEnforcementService.get(); - return settingsToFloor(settings); - } - async assertPolicyChangeAllowed( currentPolicy: WorkflowSettings.RedactionPolicy | undefined, incomingPolicy: WorkflowSettings.RedactionPolicy | undefined, @@ -36,7 +30,8 @@ export class RedactionEnforcementService { // Unchanged: preserve legacy below-floor state (no retroactive application). if (incomingPolicy === currentPolicy) return; - const floor = await this.getFloor(); + // The floor is stored as the enum directly, so no translation is needed. + const floor = await this.instanceRedactionEnforcementService.get(); if (!policyMeetsFloor(incomingPolicy, floor)) { throw new UnprocessableRequestError(REDACTION_FLOOR_VIOLATION_MESSAGE); } diff --git a/packages/cli/test/integration/controllers/security-settings.controller.test.ts b/packages/cli/test/integration/controllers/security-settings.controller.test.ts index 90056c6209f..b82c4e9c695 100644 --- a/packages/cli/test/integration/controllers/security-settings.controller.test.ts +++ b/packages/cli/test/integration/controllers/security-settings.controller.test.ts @@ -256,11 +256,7 @@ describe('SecuritySettingsController', () => { it('should return redactionEnforcement.floor = "off" by default when flag is on', async () => { enableRedactionFlag(); - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: false, - manual: false, - production: false, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('off'); const response = await ownerAgent.get('/settings/security').expect(200); @@ -268,26 +264,18 @@ describe('SecuritySettingsController', () => { expect(instanceRedactionEnforcementService.get).toHaveBeenCalledTimes(1); }); - it('should translate stored {enforced, production} to floor = "production"', async () => { + it('should return stored floor = "production"', async () => { enableRedactionFlag(); - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: true, - manual: false, - production: true, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('production'); const response = await ownerAgent.get('/settings/security').expect(200); expect(response.body.data.redactionEnforcement).toEqual({ floor: 'production' }); }); - it('should translate stored {enforced, manual, production} to floor = "all"', async () => { + it('should return stored floor = "all"', async () => { enableRedactionFlag(); - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: true, - manual: true, - production: true, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('all'); const response = await ownerAgent.get('/settings/security').expect(200); @@ -297,11 +285,7 @@ describe('SecuritySettingsController', () => { describe('POST /settings/security', () => { beforeEach(() => { - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: false, - manual: false, - production: false, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('off'); }); it('should ignore redactionEnforcement when feature flag is off', async () => { @@ -331,11 +315,7 @@ describe('SecuritySettingsController', () => { data: { redactionEnforcement: { floor: 'production' } }, }); expect(instanceRedactionEnforcementService.set).toHaveBeenCalledTimes(1); - expect(instanceRedactionEnforcementService.set).toHaveBeenCalledWith({ - enforced: true, - manual: false, - production: true, - }); + expect(instanceRedactionEnforcementService.set).toHaveBeenCalledWith('production'); }); it('should persist redactionEnforcement.floor = "all" when flag is on', async () => { @@ -347,20 +327,12 @@ describe('SecuritySettingsController', () => { .send({ redactionEnforcement: { floor: 'all' } }) .expect(200); - expect(instanceRedactionEnforcementService.set).toHaveBeenCalledWith({ - enforced: true, - manual: true, - production: true, - }); + expect(instanceRedactionEnforcementService.set).toHaveBeenCalledWith('all'); }); it('should persist redactionEnforcement.floor = "off" when flag is on', async () => { enableRedactionFlag(); - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: true, - manual: false, - production: true, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('production'); instanceRedactionEnforcementService.set.mockResolvedValue(undefined); await ownerAgent @@ -368,11 +340,7 @@ describe('SecuritySettingsController', () => { .send({ redactionEnforcement: { floor: 'off' } }) .expect(200); - expect(instanceRedactionEnforcementService.set).toHaveBeenCalledWith({ - enforced: false, - manual: false, - production: false, - }); + expect(instanceRedactionEnforcementService.set).toHaveBeenCalledWith('off'); }); it('should update personalSpace and redactionEnforcement together', async () => { @@ -412,11 +380,7 @@ describe('SecuritySettingsController', () => { describe('audit event emission', () => { it('should emit `redaction-enforcement-updated` with before/after when settings change', async () => { enableRedactionFlag(); - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: false, - manual: false, - production: false, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('off'); instanceRedactionEnforcementService.set.mockResolvedValue(undefined); await ownerAgent @@ -428,19 +392,15 @@ describe('SecuritySettingsController', () => { 'redaction-enforcement-updated', expect.objectContaining({ user: expect.objectContaining({ id: expect.any(String) }), - before: { enforced: false, manual: false, production: false }, - after: { enforced: true, manual: false, production: true }, + before: 'off', + after: 'production', }), ); }); it('should emit `redaction-enforcement-updated` with before/after when settings are disabled', async () => { enableRedactionFlag(); - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: true, - manual: false, - production: true, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('production'); instanceRedactionEnforcementService.set.mockResolvedValue(undefined); await ownerAgent @@ -452,19 +412,15 @@ describe('SecuritySettingsController', () => { 'redaction-enforcement-updated', expect.objectContaining({ user: expect.objectContaining({ id: expect.any(String) }), - before: { enforced: true, manual: false, production: true }, - after: { enforced: false, manual: false, production: false }, + before: 'production', + after: 'off', }), ); }); it('should not emit when save is idempotent', async () => { enableRedactionFlag(); - instanceRedactionEnforcementService.get.mockResolvedValue({ - enforced: true, - manual: false, - production: true, - }); + instanceRedactionEnforcementService.get.mockResolvedValue('production'); instanceRedactionEnforcementService.set.mockResolvedValue(undefined); await ownerAgent diff --git a/packages/cli/test/migration/1784000000025-migrate-redaction-enforcement-to-floor.test.ts b/packages/cli/test/migration/1784000000025-migrate-redaction-enforcement-to-floor.test.ts new file mode 100644 index 00000000000..331992102ff --- /dev/null +++ b/packages/cli/test/migration/1784000000025-migrate-redaction-enforcement-to-floor.test.ts @@ -0,0 +1,113 @@ +import { + createTestMigrationContext, + initDbUpToMigration, + runSingleMigration, + type TestMigrationContext, +} from '@n8n/backend-test-utils'; +import { DbConnection } from '@n8n/db'; +import { Container } from '@n8n/di'; +import { DataSource } from '@n8n/typeorm'; + +const MIGRATION_NAME = 'MigrateRedactionEnforcementToFloor1784000000025'; +const REDACTION_ENFORCEMENT_KEY = 'redaction.enforcement'; + +describe('MigrateRedactionEnforcementToFloor Migration', () => { + let dataSource: DataSource; + + beforeAll(async () => { + const dbConnection = Container.get(DbConnection); + await dbConnection.init(); + dataSource = Container.get(DataSource); + }); + + beforeEach(async () => { + const context = createTestMigrationContext(dataSource); + await context.queryRunner.clearDatabase(); + await context.queryRunner.release(); + await initDbUpToMigration(MIGRATION_NAME); + }); + + afterAll(async () => { + const dbConnection = Container.get(DbConnection); + await dbConnection.close(); + }); + + async function insertSetting(context: TestMigrationContext, value: string): Promise { + const tableName = context.escape.tableName('settings'); + const keyCol = context.escape.columnName('key'); + const valueCol = context.escape.columnName('value'); + const loadOnStartupCol = context.escape.columnName('loadOnStartup'); + + await context.runQuery( + `INSERT INTO ${tableName} (${keyCol}, ${valueCol}, ${loadOnStartupCol}) VALUES (:key, :value, :loadOnStartup)`, + { key: REDACTION_ENFORCEMENT_KEY, value, loadOnStartup: true }, + ); + } + + async function getStoredValue(context: TestMigrationContext): Promise { + const tableName = context.escape.tableName('settings'); + const keyCol = context.escape.columnName('key'); + const valueCol = context.escape.columnName('value'); + + const rows: Array<{ value: string }> = await context.runQuery( + `SELECT ${valueCol} AS value FROM ${tableName} WHERE ${keyCol} = :key`, + { key: REDACTION_ENFORCEMENT_KEY }, + ); + + return rows[0]?.value; + } + + async function migrateValue(stored: string): Promise { + const context = createTestMigrationContext(dataSource); + await insertSetting(context, stored); + await context.queryRunner.release(); + + await runSingleMigration(MIGRATION_NAME); + dataSource = Container.get(DataSource); + + const postContext = createTestMigrationContext(dataSource); + const value = await getStoredValue(postContext); + await postContext.queryRunner.release(); + return value; + } + + it.each([ + ['{"enforced":false,"manual":false,"production":false}', 'off'], + ['{"enforced":true,"manual":false,"production":true}', 'production'], + ['{"enforced":true,"manual":true,"production":true}', 'all'], + // Impossible-but-storable combinations normalise up to the strictest 'all'. + ['{"enforced":true,"manual":true,"production":false}', 'all'], + ['{"enforced":true,"manual":false,"production":false}', 'all'], + ])('maps stored boolean shape %s to floor "%s"', async (stored, expected) => { + const value = await migrateValue(stored); + expect(value).toBe(JSON.stringify(expected)); + }); + + it('falls back to "all" for unparseable JSON', async () => { + const value = await migrateValue('not-json'); + expect(value).toBe(JSON.stringify('all')); + }); + + it('falls back to "all" for valid JSON with the wrong shape', async () => { + const value = await migrateValue('{"unexpected":true}'); + expect(value).toBe(JSON.stringify('all')); + }); + + it('leaves an already-migrated floor value untouched', async () => { + const value = await migrateValue(JSON.stringify('production')); + expect(value).toBe(JSON.stringify('production')); + }); + + it('does not create a row when none exists', async () => { + const context = createTestMigrationContext(dataSource); + await context.queryRunner.release(); + + await runSingleMigration(MIGRATION_NAME); + dataSource = Container.get(DataSource); + + const postContext = createTestMigrationContext(dataSource); + const value = await getStoredValue(postContext); + expect(value).toBeUndefined(); + await postContext.queryRunner.release(); + }); +});