feat: Add server option readOnlyMasterKeyIps to restrict readOnlyMasterKey by IP#10115
Merged
mtrezza merged 5 commits intoparse-community:alphafrom Mar 6, 2026
Merged
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. |
Contributor
✅ 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. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a new public option Changes
Sequence DiagramsequenceDiagram
participant Client
participant Middleware
participant ConfigStore as readOnlyMasterKeyIpsStore
Client->>Middleware: HTTP request with read-only master key
Middleware->>Middleware: detect read-only master key usage
alt read-only master key used
Middleware->>ConfigStore: check client IP against allowed list
alt IP allowed (explicit or wildcard)
ConfigStore-->>Middleware: allowed
Middleware->>Middleware: set req.auth.isReadOnly = true
Middleware-->>Client: continue normal request processing
else IP not allowed
ConfigStore-->>Middleware: not allowed
Middleware->>Client: respond 403 Forbidden (throw)
end
else not using read-only master key
Middleware-->>Client: continue normal flow
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 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)
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 |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10115 +/- ##
=======================================
Coverage 92.69% 92.69%
=======================================
Files 191 191
Lines 15883 15898 +15
Branches 180 180
=======================================
+ Hits 14722 14737 +15
Misses 1149 1149
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a8f9508c-83d8-4be0-8878-e42be2b8f0f3
📒 Files selected for processing (7)
spec/Middlewares.spec.jssrc/Config.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.jssrc/ParseServer.tssrc/middlewares.js
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/middlewares.js (1)
273-281: Extract the privileged-key IP rejection path into a shared helper.This new branch now duplicates the
masterKeyIpsdeny/log/throw flow almost verbatim. Keeping these separate makes it easy for one key type to drift from the others the next time auth handling changes.♻️ Possible refactor
- if (!checkIp(clientIp, req.config.readOnlyMasterKeyIps || [], req.config.readOnlyMasterKeyIpsStore)) { - const log = req.config?.loggerController || defaultLogger; - log.error( - `Request using read-only master key rejected as the request IP address '${clientIp}' is not set in Parse Server option 'readOnlyMasterKeyIps'.` - ); - const error = new Error(); - error.status = 403; - error.message = 'unauthorized'; - throw error; - } + assertPrivilegedKeyIpAllowed({ + clientIp, + ipRangeList: req.config.readOnlyMasterKeyIps || [], + store: req.config.readOnlyMasterKeyIpsStore, + keyLabel: 'read-only master key', + optionName: 'readOnlyMasterKeyIps', + config: req.config, + });function assertPrivilegedKeyIpAllowed({ clientIp, ipRangeList, store, keyLabel, optionName, config }) { if (checkIp(clientIp, ipRangeList, store)) { return; } const log = config?.loggerController || defaultLogger; log.error( `Request using ${keyLabel} rejected as the request IP address '${clientIp}' is not set in Parse Server option '${optionName}'.` ); throw createSanitizedHttpError(403, 'unauthorized', config); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middlewares.js` around lines 273 - 281, The read-only master key IP rejection logic duplicates the masterKeyIps deny/log/throw flow; extract this into a shared helper (e.g., assertPrivilegedKeyIpAllowed) and replace the duplicated branches to call it. The helper should accept clientIp, ipRangeList (readOnlyMasterKeyIps or masterKeyIps), store (readOnlyMasterKeyIpsStore), keyLabel and optionName, use checkIp to validate, log via config?.loggerController || defaultLogger on failure, and throw a sanitized 403 'unauthorized' error (use createSanitizedHttpError or the existing error creation pattern) so both the read-only and master key checks share the same deny/log/throw behavior.
🤖 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/middlewares.js`:
- Around line 273-281: The read-only master key IP rejection logic duplicates
the masterKeyIps deny/log/throw flow; extract this into a shared helper (e.g.,
assertPrivilegedKeyIpAllowed) and replace the duplicated branches to call it.
The helper should accept clientIp, ipRangeList (readOnlyMasterKeyIps or
masterKeyIps), store (readOnlyMasterKeyIpsStore), keyLabel and optionName, use
checkIp to validate, log via config?.loggerController || defaultLogger on
failure, and throw a sanitized 403 'unauthorized' error (use
createSanitizedHttpError or the existing error creation pattern) so both the
read-only and master key checks share the same deny/log/throw behavior.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
spec/SecurityCheckGroups.spec.js (1)
50-50: Avoid coupling these assertions to check index8.This PR had to update the expected position as soon as one more check was inserted. Looking up the check by its title/identity instead of array index would make the spec resilient to future additions or reordering.
Also applies to: 72-72
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/SecurityCheckGroups.spec.js` at line 50, The test is brittle because it asserts on group.checks()[8] by index; instead locate the check by a stable identifier (e.g., title or id) and assert its state. Update the assertions that reference group.checks()[8] (and the similar one around line 72) to find the check via group.checks().find(c => c.title === 'EXPECTED_TITLE' || c.id === 'EXPECTED_ID') and then call .checkState() on that found check and compare to CheckState.success; ensure you handle the case where find returns undefined with a clear test failure message.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/Security/CheckGroups/CheckGroupServerConfig.js`:
- Around line 130-132: The current check in CheckGroupServerConfig.js uses
wildcards = ['0.0.0.0/0','::/0','::','::0'] and then tests ips.some(ip =>
wildcards.includes(ip)), which misses the plain '0.0.0.0' sentinel; update the
wildcard detection to reject plain IPv4 allow-all too by adding '0.0.0.0' to the
wildcards array (or normalize entries to strip CIDR suffixes and compare
canonical forms) so the ips.some(...) check in the readOnlyMasterKeyIps
validation correctly flags both '0.0.0.0' and '0.0.0.0/0'.
---
Nitpick comments:
In `@spec/SecurityCheckGroups.spec.js`:
- Line 50: The test is brittle because it asserts on group.checks()[8] by index;
instead locate the check by a stable identifier (e.g., title or id) and assert
its state. Update the assertions that reference group.checks()[8] (and the
similar one around line 72) to find the check via group.checks().find(c =>
c.title === 'EXPECTED_TITLE' || c.id === 'EXPECTED_ID') and then call
.checkState() on that found check and compare to CheckState.success; ensure you
handle the case where find returns undefined with a clear test failure message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 294f6bae-fed6-4ffb-bb1d-249cfc4ff7c1
📒 Files selected for processing (2)
spec/SecurityCheckGroups.spec.jssrc/Security/CheckGroups/CheckGroupServerConfig.js
parseplatformorg
pushed a commit
that referenced
this pull request
Mar 6, 2026
# [9.5.0-alpha.12](9.5.0-alpha.11...9.5.0-alpha.12) (2026-03-06) ### Features * Add server option `readOnlyMasterKeyIps` to restrict `readOnlyMasterKey` by IP ([#10115](#10115)) ([cbff6b4](cbff6b4))
Contributor
|
🎉 This change has been released in version 9.5.0-alpha.12 |
parseplatformorg
pushed a commit
that referenced
this pull request
Mar 7, 2026
# [9.5.0](9.4.1...9.5.0) (2026-03-07) ### Bug Fixes * `PagesRouter` path traversal allows reading files outside configured pages directory ([GHSA-hm3f-q6rw-m6wh](GHSA-hm3f-q6rw-m6wh)) ([#10104](#10104)) ([e772543](e772543)) * Endpoint `/loginAs` allows `readOnlyMasterKey` to gain full read and write access as any user ([GHSA-79wj-8rqv-jvp5](GHSA-79wj-8rqv-jvp5)) ([#10098](#10098)) ([bc20945](bc20945)) * File creation and deletion bypasses `readOnlyMasterKey` write restriction ([GHSA-xfh7-phr7-gr2x](GHSA-xfh7-phr7-gr2x)) ([#10095](#10095)) ([036365a](036365a)) * File metadata endpoint bypasses `beforeFind` / `afterFind` trigger authorization ([GHSA-hwx8-q9cg-mqmc](GHSA-hwx8-q9cg-mqmc)) ([#10106](#10106)) ([72e7707](72e7707)) * GraphQL `__type` introspection bypass via inline fragments when public introspection is disabled ([GHSA-q5q9-2rhp-33qw](GHSA-q5q9-2rhp-33qw)) ([#10111](#10111)) ([61261a5](61261a5)) * JWT audience validation bypass in Google, Apple, and Facebook authentication adapters ([GHSA-x6fw-778m-wr9v](GHSA-x6fw-778m-wr9v)) ([#10113](#10113)) ([9f8d3f3](9f8d3f3)) * Malformed `$regex` query leaks database error details in API response ([GHSA-9cp7-3q5w-j92g](GHSA-9cp7-3q5w-j92g)) ([#10101](#10101)) ([9792d24](9792d24)) * Regular Expression Denial of Service (ReDoS) via `$regex` query in LiveQuery ([GHSA-mf3j-86qx-cq5j](https://github.com/parse-community/parse-server/security/advisories/GHSA-mf3j-86qx-cq5j)) ([#10118](#10118)) ([5e113c2](5e113c2)) ### Features * Add `Parse.File` option `maxUploadSize` to override the Parse Server option `maxUploadSize` per file upload ([#10093](#10093)) ([3d8807b](3d8807b)) * Add security check for server option `mountPlayground` for GraphQL development ([#10103](#10103)) ([2ae5db1](2ae5db1)) * Add server option `readOnlyMasterKeyIps` to restrict `readOnlyMasterKey` by IP ([#10115](#10115)) ([cbff6b4](cbff6b4)) * Add support for `Parse.File.setDirectory`, `setMetadata`, `setTags` with stream-based file upload ([#10092](#10092)) ([ca666b0](ca666b0)) * Allow to identify `readOnlyMasterKey` invocation of Cloud Function via `request.isReadOnly` ([#10100](#10100)) ([2c48751](2c48751)) * Deprecate GraphQL Playground that exposes master key in HTTP response ([#10112](#10112)) ([d54d800](d54d800))
Contributor
|
🎉 This change has been released in version 9.5.0 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Issue
Add server option
readOnlyMasterKeyIpsto restrictreadOnlyMasterKeyby IP.Security Note
This is not a vulnerability fix, as the existing
masterKeyIpsoption only restricts themasterKeyand was never designed to apply toreadOnlyMasterKey. Each master-level key has its own independent IP restriction option:masterKey→masterKeyIpsmaintenanceKey→maintenanceKeyIpsreadOnlyMasterKey→ (no IP restriction option existed)Tasks
Summary by CodeRabbit
New Features
Documentation
Bug Fixes
Tests
Chores / Deprecations