Skip to content

Conversation

@guillaumejacquart
Copy link
Contributor

Summary

This PR adds logic to prevent saving credentials which lacks mandatory parameters

Related Linear tickets, Github issues, and Community forum posts

https://linear.app/n8n/issue/PAY-4098/credentials-saved-successfully-without-mandatory-fields

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)

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team labels Nov 24, 2025
@bundlemon
Copy link

bundlemon bot commented Nov 24, 2025

BundleMon

Unchanged files (2)
Status Path Size Limits
WASM Dependencies
tree-sitter-bash.wasm
181.26KB -
WASM Dependencies
tree-sitter.wasm
74.47KB -

No change in files bundle size

Groups updated (2)
Status Path Size Limits
**/*.js
11.38MB (+94.3KB +0.82%) -
**/*.css
231.53KB (+10.76KB +4.87%) -

Final result: ✅

View report in BundleMon website ➡️


Current branch size history | Target branch size history

@codecov
Copy link

codecov bot commented Nov 24, 2025

Codecov Report

❌ Patch coverage is 81.81818% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
...ackages/cli/src/credentials/credentials.service.ts 80.00% 1 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@blacksmith-sh
Copy link

blacksmith-sh bot commented Nov 24, 2025

Found 6 test failures on Blacksmith runners:

Failures

Test View Logs
tests/ui/2-credentials.spec.ts/
Credentials › should create credentials from NDV for node with multiple auth options
View Logs
tests/ui/26-resource-locator.spec.ts/
Resource Locator › should show appropriate error when credentials are not valid
View Logs
tests/ui/34-template-credentials-setup.spec.ts/
Template credentials setup @db:reset › can create credentials and workflow from the tem
plate
View Logs
tests/ui/34-template-credentials-setup.spec.ts/
Template credentials setup @db:reset › Credential setup from workflow editor › should a
llow credential setup from workflow editor if user fills in credentials partially durin
g template setup
View Logs
tests/ui/34-template-credentials-setup.spec.ts/
Template credentials setup @db:reset › Credential setup from workflow editor › should f
ill credentials from workflow editor
View Logs
tests/ui/43-oauth-flow.spec.ts/
OAuth Credentials › should create and connect with Google OAuth2
View Logs


Fix in Cursor

@guillaumejacquart guillaumejacquart force-pushed the pay-4098-credentials-saved-successfully-without-mandatory-fields branch from 4ffdf45 to 37ab3eb Compare November 24, 2025 15:39
@guillaumejacquart guillaumejacquart marked this pull request as ready for review November 24, 2025 15:55
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 12 files

Prompt for AI agents (all 1 issues)

Understand the root cause of the following 1 issues and fix them.


<file name="packages/cli/src/credentials/credentials.service.ts">

<violation number="1" location="packages/cli/src/credentials/credentials.service.ts:893">
Rule violated: **Prefer Typeguards over Type casting**

`opts.data` is narrowed to `ICredentialDataDecryptedObject` with an `as` assertion, violating the “Prefer Typeguards over Type casting” rule. Propagate the proper credential-data type through `CreateCredentialOptions`/DTO typings (or introduce a type guard) so this invocation doesn’t rely on `as`.</violation>
</file>

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

}

private async createCredential(opts: CreateCredentialOptions, user: User) {
this.checkCredentialData(opts.type, opts.data as ICredentialDataDecryptedObject);
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Nov 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rule violated: Prefer Typeguards over Type casting

opts.data is narrowed to ICredentialDataDecryptedObject with an as assertion, violating the “Prefer Typeguards over Type casting” rule. Propagate the proper credential-data type through CreateCredentialOptions/DTO typings (or introduce a type guard) so this invocation doesn’t rely on as.

Prompt for AI agents
Address the following comment on packages/cli/src/credentials/credentials.service.ts at line 893:

<comment>`opts.data` is narrowed to `ICredentialDataDecryptedObject` with an `as` assertion, violating the “Prefer Typeguards over Type casting” rule. Propagate the proper credential-data type through `CreateCredentialOptions`/DTO typings (or introduce a type guard) so this invocation doesn’t rely on `as`.</comment>

<file context>
@@ -872,6 +890,7 @@ export class CredentialsService {
 	}
 
 	private async createCredential(opts: CreateCredentialOptions, user: User) {
+		this.checkCredentialData(opts.type, opts.data as ICredentialDataDecryptedObject);
 		const encryptedCredential = this.createEncryptedData({
 			id: null,
</file context>
Fix with Cubic

@currents-bot
Copy link

currents-bot bot commented Nov 24, 2025

E2E Tests: n8n tests failed after 14m 50.5s

🟢 582 · 🔴 6 · ⚪️ 12 · 🟣 6

View Run Details

Run Details

  • Project: n8n

  • Groups: 2

  • Framework: Playwright

  • Run Status: Failed

  • Commit: 079ba5e

  • Spec files: 96

  • Overall tests: 600

  • Duration: 14m 50.5s

  • Parallelization: 9

Failed Spec Files

Spec File Failures
tests/ui/34-template-credentials-setup.spec.ts 3
tests/ui/2-credentials.spec.ts 1
tests/ui/26-resource-locator.spec.ts 1
tests/ui/43-oauth-flow.spec.ts 1

Groups

GroupId Results Spec Files Progress
ui 🟢 536 · 🔴 3 · ⚪️ 12 · 🟣 5 90 / 90
ui:isolated 🟢 46 · 🔴 3 · ⚪️ 0 · 🟣 1 6 / 6


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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants