fix: Block dot-notation updates to authData sub-fields and harden login provider checks#10223
Conversation
…in provider checks
|
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. Tip
Note Please respond to review comments from AI agents just like you would to comments from a human reviewer. Let the reviewer resolve their own comments, unless they have reviewed and accepted your commit, or agreed with your explanation for why the feedback was incorrect. Caution Pull requests must be written using an AI agent with human supervision. Pull requests written entirely by a human will likely be rejected, because of lower code quality, higher review effort and the higher risk of introducing bugs. Please note that AI review comments on this pull request alone do not satisfy this requirement. |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
📝 WalkthroughWalkthroughThis PR adds security tests for authData dot-notation injection attacks and implements input validation to prevent them. It expands the invalid field name check in DatabaseController from just Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
spec/vulnerabilities.spec.js (1)
3014-3032: Strengthen assertion for the rejected dotted-key update.Nice test; consider also asserting
Parse.Error.INVALID_KEY_NAMEso the test fails only for the intended guard, not any generic 400.Suggested test assertion tightening
}).catch(e => e); expect(res.status).toBe(400); + const body = typeof res.data === 'string' ? JSON.parse(res.data) : res.data; + expect(body.code).toBe(Parse.Error.INVALID_KEY_NAME);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/vulnerabilities.spec.js` around lines 3014 - 3032, Update the test "rejects dotted update key that targets authData sub-field" to assert the specific Parse error code for an invalid key name instead of only checking HTTP 400: after receiving the response (res), parse the JSON body and add an assertion that the returned error code equals Parse.Error.INVALID_KEY_NAME (keep the existing expect(res.status).toBe(400) check as well) so the test fails only for the intended invalid-key guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@spec/vulnerabilities.spec.js`:
- Around line 3014-3032: Update the test "rejects dotted update key that targets
authData sub-field" to assert the specific Parse error code for an invalid key
name instead of only checking HTTP 400: after receiving the response (res),
parse the JSON body and add an assertion that the returned error code equals
Parse.Error.INVALID_KEY_NAME (keep the existing expect(res.status).toBe(400)
check as well) so the test fails only for the intended invalid-key guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d750d172-d7dc-48b0-869d-b520e718c39a
📒 Files selected for processing (3)
spec/vulnerabilities.spec.jssrc/Auth.jssrc/Controllers/DatabaseController.js
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10223 +/- ##
==========================================
+ Coverage 92.57% 92.59% +0.01%
==========================================
Files 192 192
Lines 16300 16304 +4
Branches 199 199
==========================================
+ Hits 15090 15096 +6
+ Misses 1193 1191 -2
Partials 17 17 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
# [9.6.0-alpha.30](9.6.0-alpha.29...9.6.0-alpha.30) (2026-03-16) ### Bug Fixes * Block dot-notation updates to authData sub-fields and harden login provider checks ([#10223](#10223)) ([12c24c6](12c24c6))
|
🎉 This change has been released in version 9.6.0-alpha.30 |
Issue
Two related input validation bugs in authData handling:
Dot-notation injection (
src/Controllers/DatabaseController.js): The field name guard only blockedauthData.<alphanumeric>.idvia regex/^authData\.([a-zA-Z0-9_]+)\.id$/. Special characters in provider names (e.g.authData.anonymous".id) bypassed the check. Fix: widen to/^authData\./to block all dot-notation updates to authData sub-fields. This is safe because authData is only updated as a top-level field internally, never via dot-notation.Login crash on unknown provider (
src/Auth.js):checkIfUserHasProvidedConfiguredProvidersForLogincallsgetValidatorForProvider(provider).adapterfor each stored provider. Unknown providers returnundefined, causing a null dereference and 500 on login. Fix: filter out providers where the validator or adapter is missing.Tasks
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests