fix: Bypass of class-level permissions in LiveQuery (GHSA-7ch5-98q2-7289)#10133
Conversation
|
🚀 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. 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. |
📝 WalkthroughWalkthroughThe changes add class-level permissions (CLP) validation to LiveQuery subscriptions. New test cases verify that subscriptions are rejected or allowed based on CLP rules. The server implementation is modified to validate CLP at subscription time before proceeding with event delivery. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Server as LiveQuery Server
participant SchemaController
participant Config
Client->>Server: Subscribe to class with credentials
Server->>Config: Load class schema
Config-->>Server: Return schema with CLP
Server->>SchemaController: validatePermission(CLP, client, find)
alt CLP allows find
SchemaController-->>Server: Permission granted
Server->>Server: Create subscription
Server-->>Client: Subscription established
Note over Client,Server: Events delivered on mutations
else CLP denies find
SchemaController-->>Server: Permission denied
Server-->>Client: Subscription rejected
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
✅ 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/LiveQuery/ParseLiveQueryServer.ts (1)
892-917: Good security fix: subscribe-time CLP validation.This correctly validates class-level permissions before establishing a subscription, preventing unauthorized clients from subscribing to classes they cannot query.
One defensive consideration:
Config.get(this.config.appId)returnsundefinedif the appId is not registered in AppCache (persrc/Config.js:43-46). While this scenario is unlikely in normal operation, accessingappConfig.databasewould throw a confusing error.🛡️ Optional: Add a guard for missing config
// Check CLP for subscribe operation const appConfig = Config.get(this.config.appId); + if (!appConfig) { + throw new Parse.Error(Parse.Error.INTERNAL_SERVER_ERROR, 'Server configuration not found'); + } const schemaController = await appConfig.database.loadSchema();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/LiveQuery/ParseLiveQueryServer.ts` around lines 892 - 917, The code assumes Config.get(this.config.appId) always returns an appConfig; add a guard after calling Config.get(this.config.appId) to handle undefined (e.g., log an error and abort the subscribe flow or throw) before accessing appConfig.database and calling appConfig.database.loadSchema(); ensure you only call loadSchema(), schemaController.getClassLevelPermissions(className) and SchemaController.validatePermission when appConfig is present to avoid confusing runtime errors from dereferencing undefined.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/LiveQuery/ParseLiveQueryServer.ts`:
- Around line 892-917: The code assumes Config.get(this.config.appId) always
returns an appConfig; add a guard after calling Config.get(this.config.appId) to
handle undefined (e.g., log an error and abort the subscribe flow or throw)
before accessing appConfig.database and calling appConfig.database.loadSchema();
ensure you only call loadSchema(),
schemaController.getClassLevelPermissions(className) and
SchemaController.validatePermission when appConfig is present to avoid confusing
runtime errors from dereferencing undefined.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 976e767f-740d-437e-8d30-ea41d53be9f4
📒 Files selected for processing (3)
spec/ParseLiveQuery.spec.jsspec/ParseLiveQueryServer.spec.jssrc/LiveQuery/ParseLiveQueryServer.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10133 +/- ##
=======================================
Coverage 92.71% 92.71%
=======================================
Files 192 192
Lines 16051 16062 +11
Branches 180 183 +3
=======================================
+ Hits 14881 14892 +11
Misses 1158 1158
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
mtrezza
changed the title
## [9.5.2-alpha.3](9.5.2-alpha.2...9.5.2-alpha.3) (2026-03-08) ### Bug Fixes * Bypass of class-level permissions in LiveQuery ([GHSA-7ch5-98q2-7289](GHSA-7ch5-98q2-7289)) ([#10133](#10133)) ([98188d9](98188d9))
|
🎉 This change has been released in version 9.5.2-alpha.3 |
2 participants
Pull Request
Issue
Bypass of class-level permissions in LiveQuery (GHSA-7ch5-98q2-7289)
Tasks
Summary by CodeRabbit
Bug Fixes
Tests