feat: Add route block with new server option routeAllowList#10389
Merged
mtrezza merged 13 commits intoparse-community:alphafrom Apr 3, 2026
Merged
feat: Add route block with new server option routeAllowList#10389mtrezza merged 13 commits intoparse-community:alphafrom
routeAllowList#10389mtrezza merged 13 commits intoparse-community:alphafrom
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. 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. |
|
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds a new Changes
Sequence DiagramsequenceDiagram
participant Client as External Client
participant AuthMW as handleParseAuth
participant AllowMW as enforceRouteAllowList
participant Config as Server Config
participant Handler as Route Handler
Client->>AuthMW: HTTP request (headers, path)
AuthMW->>Config: load per-app config & keys
AuthMW->>AuthMW: resolve master/maintenance auth (sets req.auth)
AuthMW->>AllowMW: next() with req.auth
AllowMW->>Config: check _routeAllowListRegex for normalized route
alt routeAllowList not set
AllowMW->>Handler: next()
else routeAllowList set
alt req.auth is master/maintenance/internal
AllowMW->>Handler: next()
else pattern matches normalized route
AllowMW->>Handler: next()
else no match
AllowMW-->>Client: OPERATION_FORBIDDEN (sanitized)
end
end
Handler->>Client: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 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 |
mtrezza
changed the title
routeAllowListrouteAllowList
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/ParseServer.ts (1)
332-343:/healthis now gated byrouteAllowList; ensure probe routes are explicitly allowlisted.With
enforceRouteAllowListbefore the health handler, liveness/readiness checks will be denied unlesshealthmatches the configured patterns. Consider adding a startup warning whenrouteAllowListis set buthealthis not matched.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseServer.ts` around lines 332 - 343, The health endpoint is registered after api.use(middlewares.enforceRouteAllowList) so liveness/readiness probes will be blocked unless the configured routeAllowList includes "health"; update startup logic to either (A) ensure the configured routeAllowList always matches the "/health" probe route or (B) move the api.use('/health', ...) registration to run before enforceRouteAllowList, and add a startup warning when a non-empty routeAllowList is set but does not match the health path; check symbols api.use(middlewares.enforceRouteAllowList), api.use('/health', ...) and the routeAllowList configuration to implement the chosen fix.
🤖 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/Config.js`:
- Around line 749-753: The catch currently declares an unused parameter (catch
(e)) when validating regex patterns for routeAllowList; replace it with an empty
catch block (catch) to satisfy static analysis: update the try/catch around new
RegExp('^' + pattern + '$') so the catch has no parameter and still throws the
existing error message about an invalid regex pattern for routeAllowList.
In `@src/Options/Definitions.js`:
- Around line 563-567: The env parsing for routeAllowList currently uses
parsers.arrayParser which naively splits on commas and breaks regexes
(routeAllowList / parsers.arrayParser); update the source option in
src/Options/index.js to use a parser that preserves regex commas (e.g., swap to
parsers.moduleOrObjectParser or implement a new regex-aware parser such as
parsers.regexArrayParser) and ensure the option still maps to routeAllowList in
Definitions.js, then regenerate the auto-generated file by running npm run
definitions.
---
Nitpick comments:
In `@src/ParseServer.ts`:
- Around line 332-343: The health endpoint is registered after
api.use(middlewares.enforceRouteAllowList) so liveness/readiness probes will be
blocked unless the configured routeAllowList includes "health"; update startup
logic to either (A) ensure the configured routeAllowList always matches the
"/health" probe route or (B) move the api.use('/health', ...) registration to
run before enforceRouteAllowList, and add a startup warning when a non-empty
routeAllowList is set but does not match the health path; check symbols
api.use(middlewares.enforceRouteAllowList), api.use('/health', ...) and the
routeAllowList configuration to implement the chosen fix.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a6844f27-be24-4b83-a20d-566f5545d589
📒 Files selected for processing (8)
README.mdspec/RouteAllowList.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/Config.js (1)
103-107: Consider compilingrouteAllowListregexes once and reusing the result.Patterns are compiled in
validateRouteAllowListfor validation and then compiled again inConfig.put. You can compile once in validation and assign the compiled array.♻️ Proposed refactor
static put(serverConfiguration) { - Config.validateOptions(serverConfiguration); + const routeAllowListRegex = Config.validateOptions(serverConfiguration); Config.validateControllers(serverConfiguration); - if (serverConfiguration.routeAllowList) { - serverConfiguration._routeAllowListRegex = serverConfiguration.routeAllowList.map( - pattern => new RegExp('^' + pattern + '$') - ); - } + if (routeAllowListRegex) { + serverConfiguration._routeAllowListRegex = routeAllowListRegex; + } Config.transformConfiguration(serverConfiguration); AppCache.put(serverConfiguration.appId, serverConfiguration); Config.setupPasswordValidator(serverConfiguration.passwordPolicy); return serverConfiguration; } static validateOptions({ ... routeAllowList, }) { ... - this.validateRouteAllowList(routeAllowList); + return this.validateRouteAllowList(routeAllowList); } static validateRouteAllowList(routeAllowList) { if (routeAllowList === undefined || routeAllowList === null) { - return; + return null; } if (!Array.isArray(routeAllowList)) { throw 'Parse Server option routeAllowList must be an array of strings.'; } + const compiled = []; for (const pattern of routeAllowList) { if (typeof pattern !== 'string') { throw `Parse Server option routeAllowList contains a non-string value.`; } try { - new RegExp('^' + pattern + '$'); + compiled.push(new RegExp('^' + pattern + '$')); } catch { throw `Parse Server option routeAllowList contains an invalid regex pattern: "${pattern}".`; } } + return compiled; }Also applies to: 738-755
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Config.js` around lines 103 - 107, The code currently compiles routeAllowList patterns twice (once in validateRouteAllowList and again in Config.put); change validation to compile the patterns once and store the resulting RegExp array on the config object (serverConfiguration._routeAllowListRegex) so Config.put (and other callers) reuse that precompiled array instead of recompiling; update validateRouteAllowList to return/assign the compiled array and remove the second new RegExp map in the block that currently sets _routeAllowListRegex to avoid duplicate compilation (references: validateRouteAllowList, serverConfiguration._routeAllowListRegex, Config.put).
🤖 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/Config.js`:
- Around line 103-107: The code currently compiles routeAllowList patterns twice
(once in validateRouteAllowList and again in Config.put); change validation to
compile the patterns once and store the resulting RegExp array on the config
object (serverConfiguration._routeAllowListRegex) so Config.put (and other
callers) reuse that precompiled array instead of recompiling; update
validateRouteAllowList to return/assign the compiled array and remove the second
new RegExp map in the block that currently sets _routeAllowListRegex to avoid
duplicate compilation (references: validateRouteAllowList,
serverConfiguration._routeAllowListRegex, Config.put).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c1a22320-10f6-412b-a629-80ebe8d897d2
📒 Files selected for processing (4)
src/Config.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.js
🚧 Files skipped from review as they are similar to previous changes (1)
- src/Options/docs.js
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10389 +/- ##
==========================================
- Coverage 92.52% 92.50% -0.02%
==========================================
Files 192 192
Lines 16568 16620 +52
Branches 231 229 -2
==========================================
+ Hits 15330 15375 +45
- Misses 1216 1223 +7
Partials 22 22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/middlewares.js`:
- Around line 493-516: The middleware handleParseAuth does not catch errors from
async calls (notably resolveKeyAuth and config.loadKeys), which can cause
unhandled promise rejections; wrap the async section in a try-catch, call
next(error) on catch, and only continue to set req.auth and call next() when no
error occurred. Specifically, in handleParseAuth, surround the await
config.loadKeys() and await resolveKeyAuth({...}) calls with try { ... } catch
(error) { return next(error); } so thrown errors (e.g., from resolveKeyAuth) are
forwarded to Express error handlers.
- Around line 530-557: The allow-list check uses mount-relative req.url which
becomes '' for the health mount, causing documented pattern 'health' to never
match; update enforceRouteAllowList to derive the matchable path from
req.originalUrl (falling back to req.url) before trimming leading '/' and query
string so patterns like 'health' match as documented (adjust the existing path
normalization logic around req.url to use originalUrl when present and preserve
current fallbacks and regex matching against config._routeAllowListRegex).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Member
Author
|
@coderabbitai resolve conversations where we found a consensus |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me fetch the open review conversations on this PR to identify which ones have reached consensus. 🏁 Script executed: Length of output: 45846 Let me fetch the review thread IDs so I can resolve the right ones. 🏁 Script executed: Length of output: 1722 🏁 Script executed: Length of output: 184
🧠 Learnings used |
Member
Author
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/middlewares.js`:
- Around line 449-457: The maintenance-key branch currently logs an error and
falls through when checkIp fails, allowing the request to continue as a regular
user; change it to reject the request with a 403 like the master/read-only
master key branches: when maintenanceKeyValue === config.maintenanceKey and
checkIp(...) returns false, use the same error-handling path used for
master/read-only keys (log via config.loggerController || defaultLogger and
return/throw an HTTP 403 response or auth rejection) instead of returning new
auth.Auth({ ..., isMaintenance: true }) or falling through, ensuring the
invalid-IP maintenance key holder is blocked.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c59859a8-4cd9-4caa-8ce8-f5b1c80034d1
📒 Files selected for processing (2)
spec/RouteAllowList.spec.jssrc/middlewares.js
✅ Files skipped from review due to trivial changes (1)
- spec/RouteAllowList.spec.js
Member
Author
|
@coderabbitai resolve conversations where we found a consensus |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 865 🏁 Script executed: Length of output: 92
All open threads where consensus was reached are now resolved. 🧠 Learnings used |
parseplatformorg
pushed a commit
that referenced
this pull request
Apr 3, 2026
# [9.8.0-alpha.1](9.7.1-alpha.4...9.8.0-alpha.1) (2026-04-03) ### Features * Add route block with new server option `routeAllowList` ([#10389](#10389)) ([f2d06e7](f2d06e7))
Contributor
|
🎉 This change has been released in version 9.8.0-alpha.1 |
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
Apps that run all logic in Cloud Code have no need for clients to access API routes directly. While CLP can restrict per-class access, it operates deep in the request pipeline and leaks information that enables class enumeration attacks. There is no way to categorically block external network access at the middleware level.
A core principle of defense-in-depth is to reject unwanted requests as early as possible in the processing chain — before authentication, authorization, parsing, or business logic can be reached. The earlier a request is blocked, the smaller the attack surface and the less code an attacker can interact with.
Approach
Adds a new opt-in server option
routeAllowList(array of regex patterns) that blocks all external non-master-key requests unless the route matches a configured pattern. When not set, behavior is unchanged.ParseServerRESTController)OPERATION_FORBIDDENwith sanitized "Permission denied" message, indistinguishable from CLP denials^pattern$) and compiled once at startupfileUploadoptions)Tasks
Summary by CodeRabbit
New Features
[]) blocks all external non-master-key requests. Master/maintenance keys and internal Cloud Code/Jobs/triggers bypass the restriction. Health endpoint now enforces the allow list.Documentation
Tests