diff --git a/packages/@n8n/backend-services/eslint.config.mjs b/packages/@n8n/backend-services/eslint.config.mjs index c9c5ad26bf3..766d97365e7 100644 --- a/packages/@n8n/backend-services/eslint.config.mjs +++ b/packages/@n8n/backend-services/eslint.config.mjs @@ -21,6 +21,10 @@ export default defineConfig( rule: 'no-unsealed-workflow-entity-write', message: 'Route the write through a token-gated `WorkflowRepository` method.', }, + { + rule: 'no-unsealed-credentials-entity-write', + message: 'Route the write through a token-gated `CredentialsRepository` method.', + }, ], }, ], diff --git a/packages/@n8n/backend-services/oxlint.config.mts b/packages/@n8n/backend-services/oxlint.config.mts index c9ade75b8f7..a6060889413 100644 --- a/packages/@n8n/backend-services/oxlint.config.mts +++ b/packages/@n8n/backend-services/oxlint.config.mts @@ -21,6 +21,10 @@ export default defineConfig({ rule: 'no-unsealed-workflow-entity-write', message: 'Route the write through a token-gated `WorkflowRepository` method.', }, + { + rule: 'no-unsealed-credentials-entity-write', + message: 'Route the write through a token-gated `CredentialsRepository` method.', + }, ], }, ], diff --git a/packages/@n8n/eslint-config/src/plugin.ts b/packages/@n8n/eslint-config/src/plugin.ts index 0bb5f8b8c58..785572340b8 100644 --- a/packages/@n8n/eslint-config/src/plugin.ts +++ b/packages/@n8n/eslint-config/src/plugin.ts @@ -18,8 +18,9 @@ export const localRulesPlugin = { 'n8n-local-rules': plugin, }, rules: { - // Seals WorkflowEntity node writes to the token-gated repository methods. + // Seal entity content writes to the token-gated repository methods. 'n8n-local-rules/no-unsealed-workflow-entity-write': 'error', + 'n8n-local-rules/no-unsealed-credentials-entity-write': 'error', 'n8n-local-rules/no-uncaught-json-parse': 'error', 'n8n-local-rules/no-json-parse-json-stringify': 'error', 'n8n-local-rules/no-unneeded-backticks': 'error', diff --git a/packages/@n8n/eslint-config/src/rules/index.ts b/packages/@n8n/eslint-config/src/rules/index.ts index 9c61a2eaeda..923b82624ed 100644 --- a/packages/@n8n/eslint-config/src/rules/index.ts +++ b/packages/@n8n/eslint-config/src/rules/index.ts @@ -28,6 +28,7 @@ import { NoRestrictedSleepImportRule } from './no-restricted-sleep-import.js'; import { NoRepositoryInPublicApiHandlerRule } from './no-repository-in-public-api-handler.js'; import { RequirePublicApiControllerRule } from './require-public-api-controller.js'; import { NoUnsealedWorkflowEntityWriteRule } from './no-unsealed-workflow-entity-write.js'; +import { NoUnsealedCredentialsEntityWriteRule } from './no-unsealed-credentials-entity-write.js'; import { RequireEscapedQueryValuesRule } from './require-escaped-query-values.js'; import { NoOnLeaderTakeoverRule } from './no-on-leader-takeover.js'; import { NoEncryptionGuardrailDisableRule } from './no-encryption-guardrail-disable.js'; @@ -62,6 +63,7 @@ export const rules = { 'no-repository-in-public-api-handler': NoRepositoryInPublicApiHandlerRule, 'require-public-api-controller': RequirePublicApiControllerRule, 'no-unsealed-workflow-entity-write': NoUnsealedWorkflowEntityWriteRule, + 'no-unsealed-credentials-entity-write': NoUnsealedCredentialsEntityWriteRule, 'require-escaped-query-values': RequireEscapedQueryValuesRule, 'no-on-leader-takeover': NoOnLeaderTakeoverRule, 'no-encryption-guardrail-disable': NoEncryptionGuardrailDisableRule, diff --git a/packages/@n8n/eslint-config/src/rules/no-unsealed-credentials-entity-write.test.ts b/packages/@n8n/eslint-config/src/rules/no-unsealed-credentials-entity-write.test.ts new file mode 100644 index 00000000000..d619cdd0cce --- /dev/null +++ b/packages/@n8n/eslint-config/src/rules/no-unsealed-credentials-entity-write.test.ts @@ -0,0 +1,95 @@ +import { RuleTester } from '@typescript-eslint/rule-tester'; + +import { NoUnsealedCredentialsEntityWriteRule } from './no-unsealed-credentials-entity-write.js'; + +const ruleTester = new RuleTester(); +const errors = [{ messageId: 'unsealedWrite' as const }]; + +ruleTester.run('no-unsealed-credentials-entity-write', NoUnsealedCredentialsEntityWriteRule, { + valid: [ + // An OAuth token refresh writes ciphertext, never the type. + 'credentialsRepository.update(id, { data });', + 'credentialsRepository.update(id, { data, updatedAt });', + 'credentialsRepository.update(id);', + // The sanctioned, token-gated write methods. + 'credentialsRepository.updateContent(id, { type }, ctx);', + 'credentialsRepository.createContent(credential, ctx);', + 'credentialsRepository.saveInstanceCredential(credential, ctx);', + // Reads and existence changes are out of the type seal's scope. + 'credentialsRepository.delete(id);', + 'credentialsRepository.existsBy({ id });', + "manager.query('DELETE FROM credentials_entity WHERE id = ?');", + "manager.query('SELECT data FROM credentials_entity WHERE id = ?');", + // Anchored, so a sibling repository stays out. + 'sharedCredentialsRepository.save(sharedCredential);', + 'credentialDependencyRepository.save(dependency);', + // A different entity. + 'workflowRepository.save(workflow);', + 'manager.save(WorkflowEntity, workflow);', + "manager.query('UPDATE workflow_entity SET nodes = ?');", + { code: 'credentialsRepository.save(credential);', filename: '/src/example.test.ts' }, + { + code: 'credentialsRepository.save(credential);', + filename: '/repositories/credentials.repository.ts', + }, + { + code: 'credentialsRepository.save(credential);', + filename: '/packages/@n8n/backend-test-utils/src/db/credentials.ts', + }, + // A database migration owns its table. + { + code: 'credentialsRepository.save(credential);', + filename: '/packages/@n8n/db/src/migrations/common/1700000000000-Foo.ts', + }, + { + code: 'credentialsRepository.save(credential);', + filename: '/packages/@n8n/engine/src/database/migrations/1700000000000-Foo.ts', + }, + ], + invalid: [ + { code: 'credentialsRepository.save(credential);', errors }, + { code: 'this.credentialsRepository.save(credential);', errors }, + { code: "this.credentialsRepo.update(id, { type: 'slackApi' });", errors }, + { code: "credentialsRepository['save'](credential);", errors }, + { code: 'credentialRepo.insert(credential);', errors }, + { code: 'Container.get(CredentialsRepository).save(credential);', errors }, + { code: 'dataSource.getRepository(CredentialsEntity).save(credential);', errors }, + { code: 'manager.getRepository(CredentialsEntity).insert(credential);', errors }, + { code: "manager.getRepository(CredentialsEntity).upsert(credential, ['id']);", errors }, + { code: "manager.getRepository(CredentialsEntity).update(id, { type: 'slackApi' });", errors }, + { code: "manager.getRepository('credentials_entity').save(credential);", errors }, + { code: 'credentialsRepository?.save(credential);', errors }, + { code: "credentialsRepository.update(id, { type: 'slackApi' });", errors }, + { code: "credentialsRepository.update(id, { 'type': 'slackApi' });", errors }, + { code: 'manager.save(CredentialsEntity, credential);', errors }, + { code: "manager.upsert(CredentialsEntity, credential, ['id']);", errors }, + { code: "manager.update(CredentialsEntity, id, { type: 'slackApi' });", errors }, + { code: "manager.update(id, { type: 'slackApi' });", errors }, + // A call in the receiver must not hide the type argument. + { code: "dataSource.manager.update(id, { type: 'slackApi' });", errors }, + { + code: "container.get(DataSource).manager.update(id, { type: 'slackApi' });", + errors, + }, + { code: 'queryBuilder.insert().into(CredentialsEntity);', errors }, + { + code: "manager.createQueryBuilder().update(CredentialsEntity).set({ type: 'slackApi' });", + errors, + }, + { + code: "credentialsRepository.createQueryBuilder().update().set({ type: 'slackApi' });", + errors, + }, + { code: "manager.query('UPDATE credentials_entity SET type = ?');", errors }, + { code: 'manager.query(`INSERT INTO credentials_entity (type) VALUES (?)`);', errors }, + { code: 'manager.query(`UPDATE ${prefix}credentials_entity SET type = ?`);', errors }, + { code: "manager.query('UPDATE public.credentials_entity SET type = ?');", errors }, + // A folder merely named `migrations` is not a database migration. + { + code: 'credentialsRepository.save(credential);', + filename: '/packages/cli/src/modules/breaking-changes/migrations/foo.migration.ts', + errors, + }, + { code: 'manager.query(`UPDATE "public"."credentials_entity" SET type = ?`);', errors }, + ], +}); diff --git a/packages/@n8n/eslint-config/src/rules/no-unsealed-credentials-entity-write.ts b/packages/@n8n/eslint-config/src/rules/no-unsealed-credentials-entity-write.ts new file mode 100644 index 00000000000..29f5aeda152 --- /dev/null +++ b/packages/@n8n/eslint-config/src/rules/no-unsealed-credentials-entity-write.ts @@ -0,0 +1,15 @@ +import { createUnsealedEntityWriteRule } from './unsealed-entity-write.factory.js'; + +export const NoUnsealedCredentialsEntityWriteRule = createUnsealedEntityWriteRule({ + entityName: 'CredentialsEntity', + tableName: 'credentials_entity', + repositoryName: 'CredentialsRepository', + // A sibling like `sharedCredentialsRepository` stays out because the match is anchored. + repositoryReceiver: /^(?:credential|credentials)Repo(?:sitory)?$/i, + repositoryFile: /[\\/]repositories[\\/]credentials\.repository\.ts$/, + // `type` is the only field the credential policy check reads, so a data-only write + // (an OAuth token refresh) stays clean. + policedKey: 'type', + description: 'Route credential type writes through policy-cleared repository methods.', + message: 'Route CredentialsEntity type writes through a policy-cleared repository method.', +}); diff --git a/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.test.ts b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.test.ts index d3015285282..c3560a9d42b 100644 --- a/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.test.ts +++ b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.test.ts @@ -21,6 +21,11 @@ ruleTester.run('no-unsealed-workflow-entity-write', NoUnsealedWorkflowEntityWrit code: 'workflowRepository.save(workflow);', filename: '/packages/@n8n/backend-test-utils/src/db/workflows.ts', }, + // A database migration owns its table. + { + code: 'workflowRepository.save(workflow);', + filename: '/packages/@n8n/db/src/migrations/common/1700000000000-Foo.ts', + }, ], invalid: [ { code: 'workflowRepository.save(workflow);', errors }, @@ -41,6 +46,11 @@ ruleTester.run('no-unsealed-workflow-entity-write', NoUnsealedWorkflowEntityWrit { code: "manager.upsert(WorkflowEntity, workflow, ['id']);", errors }, { code: 'manager.update(WorkflowEntity, id, { nodes: [] });', errors }, { code: 'manager.update(id, { nodes: [] });', errors }, + // A call in the receiver must not hide the type argument. + { + code: 'container.get(DataSource).manager.update(id, { nodes: [] });', + errors, + }, { code: 'queryBuilder.insert().into(WorkflowEntity);', errors }, { code: 'manager.createQueryBuilder().update(WorkflowEntity).set({ nodes: [] });', @@ -56,5 +66,12 @@ ruleTester.run('no-unsealed-workflow-entity-write', NoUnsealedWorkflowEntityWrit { code: 'manager.query(`UPDATE\\x20workflow_entity SET nodes = ?`);', errors }, { code: "manager.query('UPDATE public.workflow_entity SET nodes = ?');", errors }, { code: 'manager.query(`UPDATE "public"."workflow_entity" SET nodes = ?`);', errors }, + // A folder merely named `migrations` is not a database migration: the node + // migrations under breaking-changes rewrite workflow content at runtime. + { + code: 'workflowRepository.save(workflow);', + filename: '/packages/cli/src/modules/breaking-changes/migrations/foo.migration.ts', + errors, + }, ], }); diff --git a/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.ts b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.ts index 8a34c3600b0..5b8557aeb66 100644 --- a/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.ts +++ b/packages/@n8n/eslint-config/src/rules/no-unsealed-workflow-entity-write.ts @@ -1,204 +1,12 @@ -import { ESLintUtils } from '@typescript-eslint/utils'; +import { createUnsealedEntityWriteRule } from './unsealed-entity-write.factory.js'; -const NON_RUNTIME_FILE = - /(\.(test|spec)\.ts$)|([\\/]__tests__[\\/])|([\\/]test[\\/])|([\\/]migrations[\\/])|([\\/]backend-test-utils[\\/])/; -const WORKFLOW_REPOSITORY_FILE = /[\\/]repositories[\\/]workflow\.repository\.ts$/; -const WRITE_METHODS = new Set(['save', 'insert', 'upsert']); -const RAW_WORKFLOW_WRITE = - /\b(?:UPDATE|INSERT\s+INTO)\s+(?:(?:"?\w+"?)\s*\.\s*)?["'`]?\w*workflow_entity["'`]?(?=\s|\(|$)/i; -const WORKFLOW_REPOSITORY_NAME = /^(?:workflow|workflows)Repo(?:sitory)?$/i; - -type SyntaxNode = { - type: string; - name?: string; - value?: unknown; - computed?: boolean; - property?: SyntaxNode; - object?: SyntaxNode; - expression?: SyntaxNode; - callee?: SyntaxNode; - arguments?: SyntaxNode[]; - properties?: Array<{ - type: string; - computed?: boolean; - key?: SyntaxNode; - }>; -}; - -const unwrapChain = (node: SyntaxNode): SyntaxNode => - node.type === 'ChainExpression' && node.expression ? node.expression : node; - -const memberName = (node: SyntaxNode) => { - if (!node.property) return undefined; - if (!node.computed && node.property.type === 'Identifier') return node.property.name; - if ( - node.computed && - node.property.type === 'Literal' && - typeof node.property.value === 'string' - ) { - return node.property.value; - } - return undefined; -}; - -const targetsWorkflowEntity = (node: SyntaxNode | undefined) => - node !== undefined && - ((node.type === 'Identifier' && node.name === 'WorkflowEntity') || - (node.type === 'Literal' && - (node.value === 'WorkflowEntity' || node.value === 'workflow_entity'))); - -const targetsWorkflowRepositoryClass = (node: SyntaxNode | undefined) => - node?.type === 'Identifier' && node.name === 'WorkflowRepository'; - -const isWorkflowRepository = (node: SyntaxNode): boolean => { - const target = unwrapChain(node); - if (target.type === 'Identifier') return WORKFLOW_REPOSITORY_NAME.test(target.name ?? ''); - if (target.type === 'MemberExpression') { - return WORKFLOW_REPOSITORY_NAME.test(memberName(target) ?? ''); - } - if (target.type !== 'CallExpression' || !target.callee) return false; - const callee = unwrapChain(target.callee); - return ( - callee.type === 'MemberExpression' && - ((memberName(callee) === 'get' && targetsWorkflowRepositoryClass(target.arguments?.[0])) || - (memberName(callee) === 'getRepository' && targetsWorkflowEntity(target.arguments?.[0]))) - ); -}; - -const hasNodes = (node: SyntaxNode) => - node.type === 'ObjectExpression' && - (node.properties ?? []).some((property) => { - if (property.type === 'SpreadElement' || !property.key) return false; - if (!property.computed && property.key.type === 'Identifier') { - return property.key.name === 'nodes'; - } - return property.key.type === 'Literal' && property.key.value === 'nodes'; - }); - -const isWorkflowQueryBuilder = (node: SyntaxNode): boolean => { - const call = unwrapChain(node); - if (call.type !== 'CallExpression' || !call.callee) return false; - const callee = unwrapChain(call.callee); - if (callee.type !== 'MemberExpression' || !callee.object) return false; - const method = memberName(callee); - if (method === 'createQueryBuilder') return isWorkflowRepository(callee.object); - if (method !== 'update') return false; - return ( - targetsWorkflowEntity(call.arguments?.[0]) || - (call.arguments?.length === 0 && isWorkflowQueryBuilder(callee.object)) - ); -}; - -export const NoUnsealedWorkflowEntityWriteRule = ESLintUtils.RuleCreator.withoutDocs({ - meta: { - type: 'problem', - docs: { - description: 'Route workflow content writes through policy-cleared repository methods.', - }, - messages: { - unsealedWrite: - 'Route WorkflowEntity content writes through a policy-cleared repository method.', - }, - schema: [], - }, - defaultOptions: [], - create(context) { - if ( - NON_RUNTIME_FILE.test(context.filename) || - WORKFLOW_REPOSITORY_FILE.test(context.filename) - ) { - return {}; - } - - return { - CallExpression(node) { - const callee = unwrapChain(node.callee as never); - if (callee.type !== 'MemberExpression' || !callee.object) return; - const method = memberName(callee); - if (!method) return; - - if (WRITE_METHODS.has(method) && isWorkflowRepository(callee.object)) { - context.report({ node, messageId: 'unsealedWrite' }); - return; - } - - const secondArgument = node.arguments[1]; - if ( - method === 'update' && - isWorkflowRepository(callee.object) && - secondArgument !== undefined && - secondArgument.type !== 'SpreadElement' && - hasNodes(secondArgument as never) - ) { - context.report({ node, messageId: 'unsealedWrite' }); - return; - } - - const callPrefix = context.sourceCode.getText(node).split('(', 1)[0]; - const hasWorkflowTypeArgument = /<[^>]*\bWorkflowEntity\b[^>]*>/.test(callPrefix); - const explicitWorkflowTarget = targetsWorkflowEntity(node.arguments[0] as never); - const updatePayload = explicitWorkflowTarget ? node.arguments[2] : node.arguments[1]; - if ( - method === 'update' && - (explicitWorkflowTarget || hasWorkflowTypeArgument) && - updatePayload !== undefined && - updatePayload.type !== 'SpreadElement' && - hasNodes(updatePayload as never) - ) { - context.report({ node, messageId: 'unsealedWrite' }); - return; - } - - const queryBuilderText = callee.object - ? context.sourceCode.getText(callee.object as never) - : ''; - if ( - method === 'set' && - node.arguments[0]?.type !== 'SpreadElement' && - hasNodes(node.arguments[0] as never) && - (isWorkflowQueryBuilder(callee.object) || - /update\s*<[^>]*\bWorkflowEntity\b[^>]*>/.test(queryBuilderText)) - ) { - context.report({ node, messageId: 'unsealedWrite' }); - return; - } - - if ( - ['save', 'insert', 'upsert'].includes(method) && - node.arguments[0]?.type !== 'SpreadElement' && - targetsWorkflowEntity(node.arguments[0] as never) - ) { - context.report({ node, messageId: 'unsealedWrite' }); - return; - } - - if ( - method === 'into' && - node.arguments[0]?.type !== 'SpreadElement' && - targetsWorkflowEntity(node.arguments[0] as never) - ) { - context.report({ node, messageId: 'unsealedWrite' }); - return; - } - - for (const argument of node.arguments) { - if ( - argument.type === 'Literal' && - typeof argument.value === 'string' && - RAW_WORKFLOW_WRITE.test(argument.value) - ) { - context.report({ node: argument, messageId: 'unsealedWrite' }); - } - if ( - argument.type === 'TemplateLiteral' && - RAW_WORKFLOW_WRITE.test( - argument.quasis.map((quasi) => quasi.value.cooked ?? quasi.value.raw).join(''), - ) - ) { - context.report({ node: argument, messageId: 'unsealedWrite' }); - } - } - }, - }; - }, +export const NoUnsealedWorkflowEntityWriteRule = createUnsealedEntityWriteRule({ + entityName: 'WorkflowEntity', + tableName: 'workflow_entity', + repositoryName: 'WorkflowRepository', + repositoryReceiver: /^(?:workflow|workflows)Repo(?:sitory)?$/i, + repositoryFile: /[\\/]repositories[\\/]workflow\.repository\.ts$/, + policedKey: 'nodes', + description: 'Route workflow content writes through policy-cleared repository methods.', + message: 'Route WorkflowEntity content writes through a policy-cleared repository method.', }); diff --git a/packages/@n8n/eslint-config/src/rules/unsealed-entity-write.factory.ts b/packages/@n8n/eslint-config/src/rules/unsealed-entity-write.factory.ts new file mode 100644 index 00000000000..1b36a9b33d1 --- /dev/null +++ b/packages/@n8n/eslint-config/src/rules/unsealed-entity-write.factory.ts @@ -0,0 +1,252 @@ +import { ESLintUtils } from '@typescript-eslint/utils'; + +const NON_RUNTIME_FILE = + /(\.(test|spec)\.ts$)|([\\/]__tests__[\\/])|([\\/]test[\\/])|([\\/]backend-test-utils[\\/])/; + +// A database migration owns its table, so it writes entities directly. Only those +// directories are exempt: `cli/src/modules/breaking-changes/migrations` holds node +// migrations, which are runtime code that rewrites workflow content. +const DATABASE_MIGRATION_FILE = /[\\/](?:db[\\/]src|database)[\\/]migrations[\\/]/; +const WRITE_METHODS = new Set(['save', 'insert', 'upsert']); + +export type SealedEntityWriteConfig = { + /** The entity class the seal governs, e.g. `WorkflowEntity`. */ + entityName: string; + /** The table behind it, to catch a write built as raw SQL. */ + tableName: string; + /** The repository that owns the token-gated write methods, e.g. `WorkflowRepository`. */ + repositoryName: string; + /** The repository under any injected name. Anchor it so siblings stay out. */ + repositoryReceiver: RegExp; + /** The one file allowed to write the table directly. */ + repositoryFile: RegExp; + /** The one payload key the policy check reads, e.g. `nodes`. */ + policedKey: string; + /** `meta.docs.description`. */ + description: string; + /** The report message. */ + message: string; +}; + +type SyntaxNode = { + type: string; + name?: string; + value?: unknown; + computed?: boolean; + property?: SyntaxNode; + object?: SyntaxNode; + expression?: SyntaxNode; + callee?: SyntaxNode; + arguments?: SyntaxNode[]; + properties?: Array<{ + type: string; + computed?: boolean; + key?: SyntaxNode; + }>; +}; + +const unwrapChain = (node: SyntaxNode): SyntaxNode => + node.type === 'ChainExpression' && node.expression ? node.expression : node; + +const memberName = (node: SyntaxNode) => { + if (!node.property) return undefined; + if (!node.computed && node.property.type === 'Identifier') return node.property.name; + if ( + node.computed && + node.property.type === 'Literal' && + typeof node.property.value === 'string' + ) { + return node.property.value; + } + return undefined; +}; + +/** + * Builds a rule that routes one entity's content writes through its policy-cleared + * repository methods. Purely syntactic, so Oxlint can execute it as a jsPlugin rule: + * a hoisted payload, an aliased receiver and a target built at runtime all stay out of + * reach. The `assertClearedFor` gate in the repository is the enforcing half. + */ +export const createUnsealedEntityWriteRule = (config: SealedEntityWriteConfig) => { + const { + entityName, + tableName, + repositoryName, + repositoryReceiver, + repositoryFile, + policedKey, + description, + message, + } = config; + + const rawWrite = new RegExp( + `\\b(?:UPDATE|INSERT\\s+INTO)\\s+(?:(?:"?\\w+"?)\\s*\\.\\s*)?["'\`]?\\w*${tableName}["'\`]?(?=\\s|\\(|$)`, + 'i', + ); + + const targetsEntity = (node: SyntaxNode | undefined) => + node !== undefined && + ((node.type === 'Identifier' && node.name === entityName) || + (node.type === 'Literal' && (node.value === entityName || node.value === tableName))); + + const targetsRepositoryClass = (node: SyntaxNode | undefined) => + node?.type === 'Identifier' && node.name === repositoryName; + + const isRepository = (node: SyntaxNode): boolean => { + const target = unwrapChain(node); + if (target.type === 'Identifier') return repositoryReceiver.test(target.name ?? ''); + if (target.type === 'MemberExpression') { + return repositoryReceiver.test(memberName(target) ?? ''); + } + if (target.type !== 'CallExpression' || !target.callee) return false; + const callee = unwrapChain(target.callee); + return ( + callee.type === 'MemberExpression' && + ((memberName(callee) === 'get' && targetsRepositoryClass(target.arguments?.[0])) || + (memberName(callee) === 'getRepository' && targetsEntity(target.arguments?.[0]))) + ); + }; + + const hasPolicedKey = (node: SyntaxNode) => + node.type === 'ObjectExpression' && + (node.properties ?? []).some((property) => { + if (property.type === 'SpreadElement' || !property.key) return false; + if (!property.computed && property.key.type === 'Identifier') { + return property.key.name === policedKey; + } + return property.key.type === 'Literal' && property.key.value === policedKey; + }); + + const isEntityQueryBuilder = (node: SyntaxNode): boolean => { + const call = unwrapChain(node); + if (call.type !== 'CallExpression' || !call.callee) return false; + const callee = unwrapChain(call.callee); + if (callee.type !== 'MemberExpression' || !callee.object) return false; + const method = memberName(callee); + if (method === 'createQueryBuilder') return isRepository(callee.object); + if (method !== 'update') return false; + return ( + targetsEntity(call.arguments?.[0]) || + (call.arguments?.length === 0 && isEntityQueryBuilder(callee.object)) + ); + }; + + const entityTypeArgument = new RegExp(`<[^>]*\\b${entityName}\\b[^>]*>`); + const updateTypeArgument = new RegExp(`update\\s*<[^>]*\\b${entityName}\\b[^>]*>`); + + return ESLintUtils.RuleCreator.withoutDocs({ + meta: { + type: 'problem', + docs: { description }, + messages: { unsealedWrite: message }, + schema: [], + }, + defaultOptions: [], + create(context) { + if ( + NON_RUNTIME_FILE.test(context.filename) || + DATABASE_MIGRATION_FILE.test(context.filename) || + repositoryFile.test(context.filename) + ) { + return {}; + } + + return { + CallExpression(node) { + const callee = unwrapChain(node.callee as never); + if (callee.type !== 'MemberExpression' || !callee.object) return; + const method = memberName(callee); + if (!method) return; + + if (WRITE_METHODS.has(method) && isRepository(callee.object)) { + context.report({ node, messageId: 'unsealedWrite' }); + return; + } + + const secondArgument = node.arguments[1]; + if ( + method === 'update' && + isRepository(callee.object) && + secondArgument !== undefined && + secondArgument.type !== 'SpreadElement' && + hasPolicedKey(secondArgument as never) + ) { + context.report({ node, messageId: 'unsealedWrite' }); + return; + } + + // Only the text between the callee and its arguments is the type argument list. + // Cutting the whole call at the first `(` stops inside a receiver such as + // `container.get(dataSource)`, which hides the type argument. + const callText = context.sourceCode.getText(node); + const calleeText = context.sourceCode.getText(callee as never); + const afterCallee = callText.startsWith(calleeText) + ? callText.slice(calleeText.length) + : callText; + const hasEntityTypeArgument = entityTypeArgument.test(afterCallee.split('(', 1)[0]); + const explicitEntityTarget = targetsEntity(node.arguments[0] as never); + const updatePayload = explicitEntityTarget ? node.arguments[2] : node.arguments[1]; + if ( + method === 'update' && + (explicitEntityTarget || hasEntityTypeArgument) && + updatePayload !== undefined && + updatePayload.type !== 'SpreadElement' && + hasPolicedKey(updatePayload as never) + ) { + context.report({ node, messageId: 'unsealedWrite' }); + return; + } + + const queryBuilderText = callee.object + ? context.sourceCode.getText(callee.object as never) + : ''; + if ( + method === 'set' && + node.arguments[0]?.type !== 'SpreadElement' && + hasPolicedKey(node.arguments[0] as never) && + (isEntityQueryBuilder(callee.object) || updateTypeArgument.test(queryBuilderText)) + ) { + context.report({ node, messageId: 'unsealedWrite' }); + return; + } + + if ( + WRITE_METHODS.has(method) && + node.arguments[0]?.type !== 'SpreadElement' && + targetsEntity(node.arguments[0] as never) + ) { + context.report({ node, messageId: 'unsealedWrite' }); + return; + } + + if ( + method === 'into' && + node.arguments[0]?.type !== 'SpreadElement' && + targetsEntity(node.arguments[0] as never) + ) { + context.report({ node, messageId: 'unsealedWrite' }); + return; + } + + for (const argument of node.arguments) { + if ( + argument.type === 'Literal' && + typeof argument.value === 'string' && + rawWrite.test(argument.value) + ) { + context.report({ node: argument, messageId: 'unsealedWrite' }); + } + if ( + argument.type === 'TemplateLiteral' && + rawWrite.test( + argument.quasis.map((quasi) => quasi.value.cooked ?? quasi.value.raw).join(''), + ) + ) { + context.report({ node: argument, messageId: 'unsealedWrite' }); + } + } + }, + }; + }, + }); +}; diff --git a/packages/@n8n/oxlint-config/src/configs/base.ts b/packages/@n8n/oxlint-config/src/configs/base.ts index c4bb482b5d3..87d45f17ae3 100644 --- a/packages/@n8n/oxlint-config/src/configs/base.ts +++ b/packages/@n8n/oxlint-config/src/configs/base.ts @@ -224,6 +224,7 @@ export const baseConfig = defineConfig({ 'n8n-local-rules/no-restricted-sleep-definition': 'error', 'n8n-local-rules/no-restricted-sleep-import': 'error', 'n8n-local-rules/no-type-only-import-in-di': 'error', + 'n8n-local-rules/no-unsealed-credentials-entity-write': 'error', 'n8n-local-rules/no-unsealed-workflow-entity-write': 'error', 'n8n-local-rules/no-unneeded-backticks': 'error', 'n8n-local-rules/no-unused-param-in-catch-clause': 'error', diff --git a/packages/cli/eslint.config.mjs b/packages/cli/eslint.config.mjs index 67c57a45f19..fba891470fa 100644 --- a/packages/cli/eslint.config.mjs +++ b/packages/cli/eslint.config.mjs @@ -67,6 +67,10 @@ export default defineConfig( rule: 'no-unsealed-workflow-entity-write', message: 'Route the write through a token-gated `WorkflowRepository` method.', }, + { + rule: 'no-unsealed-credentials-entity-write', + message: 'Route the write through a token-gated `CredentialsRepository` method.', + }, ], }, ], diff --git a/packages/cli/oxlint.config.mts b/packages/cli/oxlint.config.mts index eaba2e4b5b5..434380ca284 100644 --- a/packages/cli/oxlint.config.mts +++ b/packages/cli/oxlint.config.mts @@ -66,6 +66,10 @@ export default defineConfig({ rule: 'no-unsealed-workflow-entity-write', message: 'Route the write through a token-gated `WorkflowRepository` method.', }, + { + rule: 'no-unsealed-credentials-entity-write', + message: 'Route the write through a token-gated `CredentialsRepository` method.', + }, ], }, ], @@ -103,6 +107,17 @@ export default defineConfig({ 'typescript/no-duplicate-type-constituents': 'warn', }, overrides: [ + { + // Shrink-only ratchet: the two import paths that upsert a whole credential row, + // including `type`. There is no sealed credential import method yet. + // NEVER add to this list — the override is per file, so a second write here + // goes unreported. + files: [ + './src/commands/import/credentials.ts', + './src/modules/source-control.ee/source-control-import.service.ee.ts', + ], + rules: { 'n8n-local-rules/no-unsealed-credentials-entity-write': 'off' }, + }, { // Public API guardrail: handlers/controllers must go through a service, never a repository. files: ['./src/public-api/v1/handlers/**/*.ts', './src/public-api/v1/controllers/**/*.ts'], diff --git a/packages/cli/src/__tests__/credentials-helper.test.ts b/packages/cli/src/__tests__/credentials-helper.test.ts index 563df922186..46da2ed569c 100644 --- a/packages/cli/src/__tests__/credentials-helper.test.ts +++ b/packages/cli/src/__tests__/credentials-helper.test.ts @@ -1047,13 +1047,10 @@ describe('CredentialsHelper', () => { expect(credentialsRepository.update).toHaveBeenCalledWith( { id: 'cred-123', type: 'oAuth2Api' }, - expect.objectContaining({ - id: 'cred-123', - name: 'Test OAuth2 Credential', - type: 'oAuth2Api', + { data: expect.any(String), updatedAt: expect.any(Date), - }), + }, ); const updateCall = credentialsRepository.update.mock.calls[0]; @@ -1277,11 +1274,10 @@ describe('CredentialsHelper', () => { expect(storeOAuthTokenDataSpy).not.toHaveBeenCalled(); expect(credentialsRepository.update).toHaveBeenCalledWith( { id: 'cred-789', type: 'oAuth2Api' }, - expect.objectContaining({ - id: 'cred-789', + { data: expect.any(String), updatedAt: expect.any(Date), - }), + }, ); // Verify OAuth token was updated in database @@ -1324,11 +1320,10 @@ describe('CredentialsHelper', () => { expect(storeOAuthTokenDataSpy).not.toHaveBeenCalled(); expect(credentialsRepository.update).toHaveBeenCalledWith( { id: 'cred-789', type: 'oAuth2Api' }, - expect.objectContaining({ - id: 'cred-789', + { data: expect.any(String), updatedAt: expect.any(Date), - }), + }, ); // Verify OAuth token was updated in database diff --git a/packages/cli/src/credentials-helper.ts b/packages/cli/src/credentials-helper.ts index c89e4b34176..b673a0753e5 100644 --- a/packages/cli/src/credentials-helper.ts +++ b/packages/cli/src/credentials-helper.ts @@ -808,10 +808,12 @@ export class CredentialsHelper extends ICredentialsHelper { const credentials = await this.getCredentials(nodeCredentials, type); await credentials.setData(data); - const newCredentialsData = credentials.getDataToSave() as ICredentialsDb; - - // Add special database related data - newCredentialsData.updatedAt = new Date(); + // Ciphertext only. `name` and `type` would be written back unchanged, and a payload + // that cannot carry `type` keeps this off the sealed `credentialSave` path. + const newCredentialsData: Pick = { + data: credentials.getDataToSave().data, + updatedAt: new Date(), + }; // Save the credentials in DB const findQuery = { @@ -865,10 +867,12 @@ export class CredentialsHelper extends ICredentialsHelper { const credentials = await this.getCredentials(nodeCredentials, type); await credentials.updateData({ oauthTokenData: data.oauthTokenData }); - const newCredentialsData = credentials.getDataToSave() as ICredentialsDb; - - // Add special database related data - newCredentialsData.updatedAt = new Date(); + // Ciphertext only. `name` and `type` would be written back unchanged, and a payload + // that cannot carry `type` keeps this off the sealed `credentialSave` path. + const newCredentialsData: Pick = { + data: credentials.getDataToSave().data, + updatedAt: new Date(), + }; // Save the credentials in DB const findQuery = { diff --git a/packages/cli/src/oauth/__tests__/oauth.service.test.ts b/packages/cli/src/oauth/__tests__/oauth.service.test.ts index 5420b1c751b..89feea5b77c 100644 --- a/packages/cli/src/oauth/__tests__/oauth.service.test.ts +++ b/packages/cli/src/oauth/__tests__/oauth.service.test.ts @@ -451,9 +451,6 @@ describe('OauthService', () => { await service.encryptAndSaveData(credential, toUpdate, toDelete); expect(credentialsRepository.update).toHaveBeenCalledWith('1', { - id: '1', - name: expect.anything(), - type: 'test', data: expect.any(String), updatedAt: expect.any(Date), }); @@ -472,9 +469,6 @@ describe('OauthService', () => { await service.encryptAndSaveData(credential, toUpdate); expect(credentialsRepository.update).toHaveBeenCalledWith('1', { - id: '1', - name: expect.anything(), - type: 'test', data: expect.any(String), updatedAt: expect.any(Date), }); diff --git a/packages/cli/src/oauth/oauth.service.ts b/packages/cli/src/oauth/oauth.service.ts index cd0ff3d2692..bf23692438d 100644 --- a/packages/cli/src/oauth/oauth.service.ts +++ b/packages/cli/src/oauth/oauth.service.ts @@ -415,10 +415,13 @@ export class OauthService { const credentials = new Credentials(credential, credential.type, credential.data); await credentials.updateData(toUpdate, toDelete); - await this.credentialsRepository.update(credential.id, { - ...credentials.getDataToSave(), + // Ciphertext only. `name` and `type` would be written back unchanged, and a payload + // that cannot carry `type` keeps this off the sealed `credentialSave` path. + const update: Pick = { + data: credentials.getDataToSave().data, updatedAt: new Date(), - }); + }; + await this.credentialsRepository.update(credential.id, update); } /** Get a credential without user check */