feat(core): Implement credential resolution service #22799

guillaumejacquart · 2025-12-05T09:36:53Z

Summary

This PR implements credential resolving mechanism, using the resolver id setup in the credential or fallback on the workflow settings.

Service-based proxy pattern: CredentialsHelper delegates to ICredentialResolutionProvider interface
DynamicCredentialService: EE module implementing resolution logic via pluggable resolvers

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/PAY-4218/implement-pass-through-credential-resolution-service

Review / Merge checklist

PR title and summary are descriptive. (conventions)
Docs updated or follow-up ticket created.
Tests included.
PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

codecov · 2025-12-05T10:39:09Z

Codecov Report

❌ Patch coverage is 85.14851% with 15 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
.../1764689448000-AddResolvableFieldsToCredentials.ts	40.00%	6 Missing ⚠️
...namic-credentials.ee/dynamic-credentials.module.ts	0.00%	5 Missing ⚠️
...dentials.ee/services/dynamic-credential.service.ts	94.82%	3 Missing ⚠️
...c/modules/dynamic-credentials.ee/services/index.ts	0.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!

currents-bot · 2025-12-05T10:44:41Z

✅

E2E Tests: n8n tests passed after 6m 57.5s

🟢 57 · 🔴 0 · ⚪️ 12

View Run Details

Run Details

Project: n8n
Groups: 1
Framework: Playwright
Run Status: Passed
Commit: 2f692e9
Spec files: 114
Overall tests: 556
Duration: 6m 57.5s
Parallelization: 1

This message was posted automatically by currents.dev | Integration Settings

…r id and workflow settings resolver id

…solving credential

cubic-dev-ai

4 issues found across 26 files

Prompt for AI agents (all 4 issues)


Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts">

<violation number="1" location="packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts:80">
P1: Decrypt resolver configuration before parsing it; otherwise jsonParse runs on ciphertext and throws before dynamic credentials can resolve.</violation>

<violation number="2" location="packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts:118">
P2: Rule violated: **Prefer Typeguards over Type casting**

The fallback logger in `handleResolutionError` also relies on `error as Error` for type narrowing, conflicting with the guideline to prefer type guards. Use an `instanceof Error` check (or similar) before accessing `.message`.</violation>
</file>

<file name="packages/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts">

<violation number="1" location="packages/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts:235">
P2: This assertion only verifies `executionContext` after `_getCredentials` finishes, so the test would still pass if the helper set the field after calling `getDecrypted`. Capture the argument passed into `getDecrypted` and assert its `executionContext` is already populated so the test truly guards the behavior.</violation>
</file>

<file name="packages/cli/src/modules/dynamic-credentials.ee/database/repositories/__tests__/credential-resolver.repository.integration.test.ts">

<violation number="1" location="packages/cli/src/modules/dynamic-credentials.ee/database/repositories/__tests__/credential-resolver.repository.integration.test.ts:80">
P2: The test assumes a deterministic ordering of `credentialsRepository.find` results, so it will flap whenever the database returns the two credentials in the opposite order. Make the assertion order-independent (e.g., search by property or add an explicit ORDER BY).</violation>
</file>

_{Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR}

cubic-dev-ai · 2025-12-05T16:15:19Z

packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts

+		}
+
+		// Parse resolver configuration
+		const resolverConfig = jsonParse<Record<string, unknown>>(resolverEntity.config);


P1: Decrypt resolver configuration before parsing it; otherwise jsonParse runs on ciphertext and throws before dynamic credentials can resolve.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts, line 80: <comment>Decrypt resolver configuration before parsing it; otherwise jsonParse runs on ciphertext and throws before dynamic credentials can resolve.</comment> <file context> @@ -0,0 +1,203 @@ + } + + // Parse resolver configuration + const resolverConfig = jsonParse<Record<string, unknown>>(resolverEntity.config); + + try { </file context>

cubic-dev-ai · 2025-12-05T16:15:19Z

...es/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts

+
+			await contextWithCredentials['_getCredentials']('testCredential');
+
+			expect(mockAdditionalData.executionContext).toEqual(runtimeData);


P2: This assertion only verifies executionContext after _getCredentials finishes, so the test would still pass if the helper set the field after calling getDecrypted. Capture the argument passed into getDecrypted and assert its executionContext is already populated so the test truly guards the behavior.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/core/src/execution-engine/node-execution-context/__tests__/node-execution-context.test.ts, line 235: <comment>This assertion only verifies `executionContext` after `_getCredentials` finishes, so the test would still pass if the helper set the field after calling `getDecrypted`. Capture the argument passed into `getDecrypted` and assert its `executionContext` is already populated so the test truly guards the behavior. </comment> <file context> @@ -195,6 +195,56 @@ describe('NodeExecutionContext', () => { + + await contextWithCredentials['_getCredentials']('testCredential'); + + expect(mockAdditionalData.executionContext).toEqual(runtimeData); + expect(mockCredentialsHelper.getDecrypted).toHaveBeenCalledWith( + mockAdditionalData, </file context>

Suggested change

expect(mockAdditionalData.executionContext).toEqual(runtimeData);

const additionalDataArg = mockCredentialsHelper.getDecrypted.mock.calls[0][0];

expect(additionalDataArg.executionContext).toEqual(runtimeData);

expect(mockAdditionalData.executionContext).toEqual(runtimeData);

cubic-dev-ai · 2025-12-05T16:15:19Z

...ntials.ee/database/repositories/__tests__/credential-resolver.repository.integration.test.ts

+
+			expect(linked).toHaveLength(2);
+			expect(linked[0].resolverId).toBe(resolver.id);
+			expect(linked[1].resolvableAllowFallback).toBe(true);


P2: The test assumes a deterministic ordering of credentialsRepository.find results, so it will flap whenever the database returns the two credentials in the opposite order. Make the assertion order-independent (e.g., search by property or add an explicit ORDER BY).

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/dynamic-credentials.ee/database/repositories/__tests__/credential-resolver.repository.integration.test.ts, line 80: <comment>The test assumes a deterministic ordering of `credentialsRepository.find` results, so it will flap whenever the database returns the two credentials in the opposite order. Make the assertion order-independent (e.g., search by property or add an explicit ORDER BY).</comment> <file context> @@ -0,0 +1,112 @@ + + expect(linked).toHaveLength(2); + expect(linked[0].resolverId).toBe(resolver.id); + expect(linked[1].resolvableAllowFallback).toBe(true); + }); + </file context>

cubic-dev-ai · 2025-12-05T16:15:19Z

packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts

+			return toCredentialContext(decrypted);
+		} catch (error) {
+			this.logger.error('Failed to decrypt credential context from execution context', {
+				error: (error as Error).message,


P2: Rule violated: Prefer Typeguards over Type casting

The fallback logger in handleResolutionError also relies on error as Error for type narrowing, conflicting with the guideline to prefer type guards. Use an instanceof Error check (or similar) before accessing .message.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At packages/cli/src/modules/dynamic-credentials.ee/services/dynamic-credential.service.ts, line 118: <comment>The fallback logger in `handleResolutionError` also relies on `error as Error` for type narrowing, conflicting with the guideline to prefer type guards. Use an `instanceof Error` check (or similar) before accessing `.message`.</comment> <file context> @@ -0,0 +1,203 @@ + return toCredentialContext(decrypted); + } catch (error) { + this.logger.error('Failed to decrypt credential context from execution context', { + error: (error as Error).message, + }); + return undefined; </file context>

Suggested change

error: (error as Error).message,

error: error instanceof Error ? error.message : String(error),

n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Dec 5, 2025

guillaumejacquart force-pushed the pay-4218-implement-pass-through-credential-resolution-service branch from f63952b to ce26bee Compare December 5, 2025 10:30

This comment has been minimized.

Sign in to view

guillaumejacquart added 4 commits December 5, 2025 13:50

add resolver fields to the credentials

5a2a67a

Fix test for repo

736f0f5

feat(core): Add dynamic credential resolution with credential resolve…

7d0910b

…r id and workflow settings resolver id

fix existing tests

ac3239e

guillaumejacquart force-pushed the pay-4218-implement-pass-through-credential-resolution-service branch from ce26bee to ac3239e Compare December 5, 2025 13:02

add missing executionContext to additionalData before fetching and re…

764886e

…solving credential

guillaumejacquart force-pushed the pay-4218-implement-pass-through-credential-resolution-service branch from 305d84f to dd76799 Compare December 5, 2025 14:03

merge static with dynamic data. Update tests

2f692e9

guillaumejacquart force-pushed the pay-4218-implement-pass-through-credential-resolution-service branch from dd76799 to 2f692e9 Compare December 5, 2025 14:37

This comment has been minimized.

Sign in to view

guillaumejacquart marked this pull request as ready for review December 5, 2025 16:06

guillaumejacquart requested a review from a team as a code owner December 5, 2025 16:06

cubic-dev-ai bot reviewed Dec 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(core): Implement credential resolution service #22799

feat(core): Implement credential resolution service #22799

guillaumejacquart commented Dec 5, 2025

Uh oh!

codecov bot commented Dec 5, 2025 •

edited

Loading

Uh oh!

This comment has been minimized.

currents-bot bot commented Dec 5, 2025 •

edited

Loading

Uh oh!

This comment has been minimized.

cubic-dev-ai bot left a comment

Uh oh!

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading

Uh oh!

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants


		await contextWithCredentials['_getCredentials']('testCredential');

		expect(mockAdditionalData.executionContext).toEqual(runtimeData);

	error: (error as Error).message,
	error: error instanceof Error ? error.message : String(error),

feat(core): Implement credential resolution service #22799

Are you sure you want to change the base?

feat(core): Implement credential resolution service #22799

Conversation

guillaumejacquart commented Dec 5, 2025

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

Uh oh!

codecov bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

This comment has been minimized.

currents-bot bot commented Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cubic-dev-ai bot Dec 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

codecov bot commented Dec 5, 2025 •

edited

Loading

currents-bot bot commented Dec 5, 2025 •

edited

Loading

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading

cubic-dev-ai bot Dec 5, 2025 •

edited

Loading