feat: Add header aliasing system for Parse headers#10401
Open
Moumouls wants to merge 6 commits intoparse-community:alphafrom
Open
feat: Add header aliasing system for Parse headers#10401Moumouls wants to merge 6 commits intoparse-community:alphafrom
Moumouls wants to merge 6 commits intoparse-community:alphafrom
Conversation
|
I will reformat the title to use the proper commit message syntax. |
parse-github-assistant
bot
changed the title
|
🚀 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. Our CI and AI review are safeguards, not development tools. If many issues are flagged, rethink your development approach. Invest more effort in planning rather than using review cycles to iteratively fix low-quality code. |
📝 WalkthroughWalkthroughAdds support for configurable header aliases: validates config, rewrites incoming request headers to canonical Parse names via new middleware, includes aliases in CORS and GraphQL CSRF headers, and adds tests covering middleware, REST, and GraphQL flows. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Proxy as Parse Server (Express)
participant AliasMW as handleHeaderAliases
participant HeaderMW as handleParseHeaders
participant AuthMW as handleParseAuth
participant GraphQL as Apollo Server
Client->>Proxy: HTTP request with alias header (e.g., X-App-Id)
Proxy->>AliasMW: invoke middleware (load app config)
AliasMW->>AliasMW: map alias → canonical header, rewrite req.headers
AliasMW-->>Proxy: next()
Proxy->>HeaderMW: handleParseHeaders reads canonical headers
HeaderMW-->>AuthMW: parsed header info (appId, sessionToken)
AuthMW->>AuthMW: validate token / master key
Proxy->>GraphQL: request reaches GraphQL server with canonical headers
GraphQL->>GraphQL: CSRF header whitelist includes aliases
GraphQL-->>Client: response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (1 warning, 2 inconclusive)
✅ 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 |
Member
Author
|
@coderabbitai do you see security issues ? Or strong implementation gaps ? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: Length of output: 24429
🔴 Security Issues & Implementation Gaps1. Canonical Header Overwrite — Behaviour Contradicts Documentation (Critical Bug)The documented behaviour states: "If the canonical header is not present in a request, Parse Server uses the first matching alias." However, // src/middlewares.js
Object.entries(req.headers).forEach(([header, value]) => {
const targetHeader = indexHeaderByAlias[header.toLowerCase()];
if (targetHeader) {
req.headers[targetHeader] = value; // ← always overwrites, no guard
}
});If a client sends both if (targetHeader && !req.headers[targetHeader]) {
req.headers[targetHeader] = value;
}2. Null Safety Crash in
|
| # | Severity | Type | Description |
|---|---|---|---|
| 1 | 🔴 High | Bug + Security | Alias overwrites canonical header even when already present |
| 2 | 🔴 High | Bug | Object.entries(undefined) crash when config is null |
| 3 | 🟠 Medium | Security | No CRLF sanitization on alias names → header injection |
| 4 | 🟠 Medium | Security | No allowlist for canonical header keys |
| 5 | 🟡 Low | Bug | Duplicate alias across canonical headers silently ignored |
| 6 | 🟡 Low | Security | No warning for master key aliasing |
| 7 | 🟡 Low | Process | Security check PR task is unchecked |
🧠 Learnings used
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10158
File: spec/CloudCode.spec.js:4473-4487
Timestamp: 2026-03-09T19:00:02.915Z
Learning: In parse-community/parse-server, the `responseHeaders` object exposed to `afterFind` file hooks (Parse.Cloud.afterFind(Parse.File, ...)) is intentionally fully mutable by the server administrator. Admins can add, modify, or delete headers — including default security headers like `X-Content-Type-Options: nosniff` — via `req.responseHeaders`. Removing a security header is treated as an explicit, conscious decision by the person who also holds the master key. Parse Server provides secure defaults but does not enforce them as non-overridable. This is consistent with the `forceDownload` pattern that also gives Cloud Code full control over file response behavior.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10394
File: src/Routers/FilesRouter.js:96-105
Timestamp: 2026-04-03T18:40:52.899Z
Learning: In parse-community/parse-server `src/Routers/FilesRouter.js` (PR `#10394`), the `initInfo` middleware uses `!sessionToken` (not `== null`) to gate the public-access fast path. `req.get('X-Parse-Session-Token')` returns `undefined` when the header is absent and `''` when the header is present but empty. Both values are trivially invalid session tokens; treating them identically (skipping `handleParseSession`) is intentional and correct. Using `== null` would send empty strings to `handleParseSession` for a guaranteed-failing DB lookup, which is strictly worse behavior.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10237
File: src/Routers/PagesRouter.js:552-573
Timestamp: 2026-03-18T17:53:35.669Z
Learning: In parse-community/parse-server, `src/Routers/PagesRouter.js`: the `encodeURIComponent` call in `composePageParamHeaders` is pre-existing behavior extracted from inline code in `pageResponse` and `redirectResponse`. The potential URIError on malformed Unicode (lone surrogates) when `encodePageParamHeaders` is enabled is a known pre-existing issue, not introduced by the CRLF sanitization refactor. Do not flag it as a new issue in PRs that only refactor or consolidate this call.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## alpha #10401 +/- ##
=======================================
Coverage 92.49% 92.50%
=======================================
Files 192 192
Lines 16732 16806 +74
Branches 229 229
=======================================
+ Hits 15477 15547 +70
- Misses 1233 1237 +4
Partials 22 22 ☔ 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: 5
🧹 Nitpick comments (2)
spec/rest.spec.js (1)
1741-1788: Add one precedence test for canonical + alias sent together.These tests validate alias support well; please add a case that sends both canonical and alias headers and asserts canonical wins (or the intended policy), so this behavior is locked down.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/rest.spec.js` around lines 1741 - 1788, Add a new test in the same "rest header aliases" describe block that calls reconfigureServer with headerAliases (same shape used in existing tests), then sends a request via the existing request helper to an endpoint (e.g., GET /users/me or GET /schemas) with both the canonical header (e.g., 'X-Parse-Application-Id' or 'X-Parse-Session-Token') and its alias (e.g., 'X-App-Id' or 'X-Session-Token-Alias') present but containing different values, and assert that the server uses the canonical value (for example, create a user via Parse.User.signUp, send both 'X-Parse-Session-Token' with the real token and 'X-Session-Token-Alias' with a bogus token, call request({ url: `${Parse.serverURL}/users/me`, ... }) and expect response.data.objectId to equal the signed-up user's id), then finally call reconfigureServer() in a finally block to restore defaults. Ensure the test name clearly states it checks canonical-over-alias precedence and reuse the helpers reconfigureServer, request, and Parse from the existing tests.spec/Middlewares.spec.js (1)
431-481: Add a canonical-wins regression for aliased headers.These tests cover the alias-only path, but they do not pin the more security-sensitive case where both the canonical
X-Parse-*header and an alias are present. Please add one regression that proves the canonical header wins; that will catch accidental overwrites inhandleHeaderAliases().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/Middlewares.spec.js` around lines 431 - 481, Add a regression test that ensures canonical X-Parse-* headers take precedence over configured aliases by creating a test (alongside existing ones) that sets both the canonical header (e.g., 'x-parse-application-id' or 'x-parse-session-token' or 'x-parse-master-key') and its alias (e.g., 'x-app-id' / 'x-session-token-alias' / 'x-master-key-alias') to different values, invoke middlewares.handleHeaderAliases(...) then invoke middlewares.handleParseHeaders(...) or middlewares.handleParseAuth(...) as appropriate, and assert that fakeReq.headers['x-parse-application-id'] (or fakeReq.info.appId / fakeReq.info.sessionToken / fakeReq.auth.isMaster) reflects the canonical header value (not the alias); use the same AppCachePut pattern and header alias config as existing tests and mirror their async/callback invocation style so the regression is enforced.
🤖 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 727-748: In validateHeaderAliases, after the existing type checks
in the static method validateHeaderAliases, normalize every canonicalHeader and
each alias by trimming and lowercasing, then detect and reject any collisions:
(1) an alias that normalizes to the same string as any other alias or
canonicalHeader (including its own canonicalHeader) should cause a thrown error,
and (2) any duplicate normalized alias within a single aliases array should be
rejected; use the identifiers canonicalHeader and aliases to build clear error
messages that indicate which canonical header and alias collided and throw (not
silently accept) to prevent multiple Parse headers mapping to the same effective
header.
In `@src/GraphQL/ParseGraphQLServer.js`:
- Around line 158-163: The Apollo CSRF whitelist currently includes header
aliases from parseServer.config.headerAliases via getCSRFRequestHeaders(), which
can allow CORS-safelisted names (e.g., Accept) and weaken protection; update
getCSRFRequestHeaders() to either (a) only return canonical header names (no
aliases) or (b) filter out any alias that matches a predefined set of
CORS-safelisted header names before returning (include names like Accept,
Accept-Language, Content-Language, Content-Type and other safelisted request
headers), and ensure the ApolloServer config uses that filtered list for
csrfPrevention.requestHeaders; additionally move or delay the
handleHeaderAliases middleware so it runs after Apollo's CSRF check (or remove
alias handling from the pre-CSRF path) to avoid aliasing before validation.
In `@src/middlewares.js`:
- Around line 86-116: getHeaderAliases and applyHeaderAliases assume
headerAliases is an object and will throw if undefined; in handleHeaderAliases,
ensure the config?.headerAliases value is null-safe before passing it (e.g.,
default to {} or guard and skip application). Update handleHeaderAliases to call
applyHeaderAliases(req, config?.headerAliases || {}) or guard in
applyHeaderAliases/getHeaderAliases to handle non-object inputs, referencing
functions getHeaderAliases, applyHeaderAliases and the call site in
handleHeaderAliases that uses Config.get(appId, getMountForRequest(req)).
- Around line 95-110: In applyHeaderAliases, avoid letting alias headers clobber
an existing canonical header: for each canonical name in headerAliases, first
check (case-insensitively) if req.headers already contains the canonical name
and if so do nothing; otherwise iterate the configured aliases in their defined
order and, for the first alias present in req.headers (case-insensitive), copy
that value into the canonical header slot. Use lowercased lookups for
comparisons but do not overwrite an existing canonical header; ensure alias
precedence comes from headerAliases' alias array order rather than request
header iteration order.
In `@src/ParseServer.ts`:
- Line 314: The middleware currently overwrites canonical headers with aliases;
update the implementation in handleHeaderAliases/applyHeaderAliases so that for
each header alias mapping it only sets the canonical header value when the
canonical header is missing or empty (do not overwrite when request.headers has
the canonical key present), and ensure you preserve original header casing/keys
used by Parse logic; also add a unit/integration test that sends both canonical
and alias headers and asserts the canonical header value is preserved and used
for authentication/application-id resolution.
---
Nitpick comments:
In `@spec/Middlewares.spec.js`:
- Around line 431-481: Add a regression test that ensures canonical X-Parse-*
headers take precedence over configured aliases by creating a test (alongside
existing ones) that sets both the canonical header (e.g.,
'x-parse-application-id' or 'x-parse-session-token' or 'x-parse-master-key') and
its alias (e.g., 'x-app-id' / 'x-session-token-alias' / 'x-master-key-alias') to
different values, invoke middlewares.handleHeaderAliases(...) then invoke
middlewares.handleParseHeaders(...) or middlewares.handleParseAuth(...) as
appropriate, and assert that fakeReq.headers['x-parse-application-id'] (or
fakeReq.info.appId / fakeReq.info.sessionToken / fakeReq.auth.isMaster) reflects
the canonical header value (not the alias); use the same AppCachePut pattern and
header alias config as existing tests and mirror their async/callback invocation
style so the regression is enforced.
In `@spec/rest.spec.js`:
- Around line 1741-1788: Add a new test in the same "rest header aliases"
describe block that calls reconfigureServer with headerAliases (same shape used
in existing tests), then sends a request via the existing request helper to an
endpoint (e.g., GET /users/me or GET /schemas) with both the canonical header
(e.g., 'X-Parse-Application-Id' or 'X-Parse-Session-Token') and its alias (e.g.,
'X-App-Id' or 'X-Session-Token-Alias') present but containing different values,
and assert that the server uses the canonical value (for example, create a user
via Parse.User.signUp, send both 'X-Parse-Session-Token' with the real token and
'X-Session-Token-Alias' with a bogus token, call request({ url:
`${Parse.serverURL}/users/me`, ... }) and expect response.data.objectId to equal
the signed-up user's id), then finally call reconfigureServer() in a finally
block to restore defaults. Ensure the test name clearly states it checks
canonical-over-alias precedence and reuse the helpers reconfigureServer,
request, and Parse from the existing tests.
🪄 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: 985cb1fa-03eb-47f1-beed-cd7632aa1b45
📒 Files selected for processing (12)
spec/Middlewares.spec.jsspec/ParseGraphQLServer.spec.jsspec/rest.spec.jssrc/Config.jssrc/GraphQL/ParseGraphQLServer.jssrc/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.jssrc/ParseServer.tssrc/defaults.jssrc/middlewares.jstypes/Options/index.d.ts
Comment on lines
+95
to
+110
| function applyHeaderAliases(req, headerAliases) { | ||
| req.headers = req.headers || {}; | ||
| const indexHeaderByAlias = Object.fromEntries( | ||
| Object.entries(headerAliases) | ||
| .map(([source, aliases]) => | ||
| aliases | ||
| .map(alias => [alias.toLowerCase(), source.toLowerCase()]) | ||
| ) | ||
| .flat() | ||
| ); | ||
| Object.entries(req.headers).forEach(([header, value]) => { | ||
| const targetHeader = indexHeaderByAlias[header.toLowerCase()]; | ||
| if (targetHeader) { | ||
| req.headers[targetHeader] = value; | ||
| } | ||
| }); |
There was a problem hiding this comment.
Do not let aliases overwrite canonical headers.
Line 108 always copies the alias value into the canonical slot. If both headers are present, a client-controlled alias can clobber an existing canonical header, and if multiple aliases are present the winner depends on request header order instead of the configured alias order. For auth headers, that can swap the app ID or session token unexpectedly.
Proposed fix
function applyHeaderAliases(req, headerAliases) {
req.headers = req.headers || {};
- const indexHeaderByAlias = Object.fromEntries(
- Object.entries(headerAliases)
- .map(([source, aliases]) =>
- aliases
- .map(alias => [alias.toLowerCase(), source.toLowerCase()])
- )
- .flat()
- );
- Object.entries(req.headers).forEach(([header, value]) => {
- const targetHeader = indexHeaderByAlias[header.toLowerCase()];
- if (targetHeader) {
- req.headers[targetHeader] = value;
- }
- });
+ for (const [canonicalHeader, aliases] of Object.entries(headerAliases)) {
+ const canonicalKey = canonicalHeader.toLowerCase();
+ if (req.headers[canonicalKey] !== undefined) {
+ continue;
+ }
+ const matchedAlias = aliases.find(alias => req.headers[alias.toLowerCase()] !== undefined);
+ if (matchedAlias) {
+ req.headers[canonicalKey] = req.headers[matchedAlias.toLowerCase()];
+ }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| function applyHeaderAliases(req, headerAliases) { | |
| req.headers = req.headers || {}; | |
| const indexHeaderByAlias = Object.fromEntries( | |
| Object.entries(headerAliases) | |
| .map(([source, aliases]) => | |
| aliases | |
| .map(alias => [alias.toLowerCase(), source.toLowerCase()]) | |
| ) | |
| .flat() | |
| ); | |
| Object.entries(req.headers).forEach(([header, value]) => { | |
| const targetHeader = indexHeaderByAlias[header.toLowerCase()]; | |
| if (targetHeader) { | |
| req.headers[targetHeader] = value; | |
| } | |
| }); | |
| function applyHeaderAliases(req, headerAliases) { | |
| req.headers = req.headers || {}; | |
| for (const [canonicalHeader, aliases] of Object.entries(headerAliases)) { | |
| const canonicalKey = canonicalHeader.toLowerCase(); | |
| if (req.headers[canonicalKey] !== undefined) { | |
| continue; | |
| } | |
| const matchedAlias = aliases.find(alias => req.headers[alias.toLowerCase()] !== undefined); | |
| if (matchedAlias) { | |
| req.headers[canonicalKey] = req.headers[matchedAlias.toLowerCase()]; | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/middlewares.js` around lines 95 - 110, In applyHeaderAliases, avoid
letting alias headers clobber an existing canonical header: for each canonical
name in headerAliases, first check (case-insensitively) if req.headers already
contains the canonical name and if so do nothing; otherwise iterate the
configured aliases in their defined order and, for the first alias present in
req.headers (case-insensitive), copy that value into the canonical header slot.
Use lowercased lookups for comparisons but do not overwrite an existing
canonical header; ensure alias precedence comes from headerAliases' alias array
order rather than request header iteration order.
Member
There was a problem hiding this comment.
Check to see whether the issue is addressed.
There was a problem hiding this comment.
@mtrezza, the primary concern (aliases unconditionally overwriting canonical headers) is addressed in the current file. The guard !req.headers[targetHeader] was added, so a canonical header already present in the request is never overwritten by an alias.
However, one sub-point from the original comment is not yet resolved: alias precedence still depends on req.headers iteration order rather than the configured alias array order. The current implementation iterates over request headers and stops at the first alias hit in that order:
// Current: iterates req.headers — winner depends on request header iteration order
Object.entries(req.headers).forEach(([header, value]) => {
const targetHeader = indexHeaderByAlias[header.toLowerCase()];
if (targetHeader && !req.headers[targetHeader]) {
req.headers[targetHeader] = value;
}
});If a client sends two alias headers for the same canonical target (e.g., both X-App-Id-1 and X-App-Id-2), the winner is whichever appears first in req.headers — which is undefined/engine-dependent — rather than the first alias listed in the headerAliases config. The proposed fix addressed this by iterating the config instead:
// Proposed: iterates config — winner is first alias in the configured array
for (const [canonicalHeader, aliases] of Object.entries(headerAliases)) {
const canonicalKey = canonicalHeader.toLowerCase();
if (req.headers[canonicalKey] !== undefined) continue;
const matchedAlias = aliases.find(alias => req.headers[alias.toLowerCase()] !== undefined);
if (matchedAlias) {
req.headers[canonicalKey] = req.headers[matchedAlias.toLowerCase()];
}
}Summary:
| Concern | Status |
|---|---|
| Alias overwrites canonical header | ✅ Fixed (!req.headers[targetHeader] guard) |
| Alias precedence follows configured array order | ❌ Still uses req.headers iteration order |
Would you like to keep the remaining sub-issue for a follow-up, or apply the full proposed fix now?
🧠 Learnings used
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10237
File: src/Routers/PagesRouter.js:552-573
Timestamp: 2026-03-18T17:53:35.669Z
Learning: In parse-community/parse-server, `src/Routers/PagesRouter.js`: the `encodeURIComponent` call in `composePageParamHeaders` is pre-existing behavior extracted from inline code in `pageResponse` and `redirectResponse`. The potential URIError on malformed Unicode (lone surrogates) when `encodePageParamHeaders` is enabled is a known pre-existing issue, not introduced by the CRLF sanitization refactor. Do not flag it as a new issue in PRs that only refactor or consolidate this call.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10394
File: src/Routers/FilesRouter.js:96-105
Timestamp: 2026-04-03T18:40:52.899Z
Learning: In parse-community/parse-server `src/Routers/FilesRouter.js` (PR `#10394`), the `initInfo` middleware uses `!sessionToken` (not `== null`) to gate the public-access fast path. `req.get('X-Parse-Session-Token')` returns `undefined` when the header is absent and `''` when the header is present but empty. Both values are trivially invalid session tokens; treating them identically (skipping `handleParseSession`) is intentional and correct. Using `== null` would send empty strings to `handleParseSession` for a guaranteed-failing DB lookup, which is strictly worse behavior.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: When reviewing Parse Server PRs that add new features, always check whether the feature is documented in the README.md file, though for new Parse Server options this is optional rather than required.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:48.786Z
Learning: For Parse Server PRs, always suggest an Angular commit convention PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion on every commit. The format should be: type(scope): description. Common types include feat, fix, perf, refactor, docs, test, chore. The scope should identify the subsystem (e.g., graphql, rest, push, security). The description should be action-oriented and clearly convey the change's impact to developers.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2026-03-23T22:11:01.903Z
Learning: In Parse Server PRs (parse-community/parse-server), when the commit/PR type prefix is `fix:`, the PR title must describe the bug (the broken behavior), not the fix (the expected or corrected behavior). For example, instead of "fix: Strip undefined values in ParseServerRESTController to match HTTP mode" (describes the fix), prefer "fix: directAccess update with undefined field values persists them as null in MongoDB" (describes the broken behavior).
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-17T15:02:24.824Z
Learning: For Parse Server PRs, always suggest an Angular-style PR title that would make a meaningful changelog entry for developers. Update the PR title suggestion with every new commit to the PR.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10394
File: src/Routers/FilesRouter.js:94-111
Timestamp: 2026-04-03T18:23:49.632Z
Learning: In parse-community/parse-server `src/Routers/FilesRouter.js`, the old `_resolveAuth` helper that silently caught all `auth.getAuthForSessionToken` errors and fell back to anonymous/public access has been intentionally removed in PR `#10394`. File download auth now uses `initInfo` + `Middlewares.handleParseSession`, matching file upload behavior: invalid/stale session tokens on GET (download) routes are now rejected rather than silently falling back to public access. This alignment is a deliberate design decision — the silent fallback was considered the anomaly.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10158
File: spec/CloudCode.spec.js:4473-4487
Timestamp: 2026-03-09T19:00:02.915Z
Learning: In parse-community/parse-server, the `responseHeaders` object exposed to `afterFind` file hooks (Parse.Cloud.afterFind(Parse.File, ...)) is intentionally fully mutable by the server administrator. Admins can add, modify, or delete headers — including default security headers like `X-Content-Type-Options: nosniff` — via `req.responseHeaders`. Removing a security header is treated as an explicit, conscious decision by the person who also holds the master key. Parse Server provides secure defaults but does not enforce them as non-overridable. This is consistent with the `forceDownload` pattern that also gives Cloud Code full control over file response behavior.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10117
File: benchmark/performance.js:713-737
Timestamp: 2026-03-06T21:30:03.253Z
Learning: In Parse Server LiveQuery, use liveQuery: { classNames: [...] } in the server config to specify which classes emit pub/sub events (server-side producers). Use ParseServer.createLiveQueryServer(httpServer, { appId, masterKey, serverURL }) to start the WebSocket server that forwards events to subscribed clients (event consumers); this function does not require a classNames parameter, as it simply matches incoming events against active client subscriptions.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10117
File: benchmark/performance.js:52-52
Timestamp: 2026-03-06T21:30:10.326Z
Learning: In reviews of JavaScript files, ensure separation of concerns for LiveQuery in Parse Server: do not rely on the LiveQuery server to perform className filtering. The filtering and class event emission should occur in the ParseServer configuration (classNames within liveQuery) or related server logic, while ParseServer.createLiveQueryServer(httpServer, { appId, masterKey, serverURL }) should only establish the WebSocket-based delivery of already-filtered events. Treat classNames filtering as the responsibility of ParseServer, not the LiveQuery server, and verify that LiveQuery code receives pre-filtered events and does not duplicate filtering logic.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10225
File: spec/Utils.spec.js:399-419
Timestamp: 2026-03-16T21:02:42.989Z
Learning: In the parse-community/parse-server repository, Biome is not used. Do not flag Biome lint issues (e.g., lint/suspicious/noThenProperty) or suggest biome-ignore comments in any JavaScript files. Treat Biome-related checks as exempt for all JS sources under this repo; if CI enforces Biome, ensure this guidance is reflected in CI configuration accordingly.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10389
File: src/middlewares.js:493-516
Timestamp: 2026-04-03T02:00:09.295Z
Learning: In this codebase (Parse Server using Express 5), do not require explicit try/catch + next(error) wrappers for async Express middleware. Express 5 automatically forwards errors from async middleware (including unhandled promise rejections) to the next error handler. Therefore, in src/middlewares.js, it is expected that async functions like handleParseHeaders and handleParseAuth can intentionally throw without try/catch as part of normal Express 5 behavior.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/middlewares.js (1)
86-93: Add null guard togetHeaderAliasesfor defensive programming.
getHeaderAliasesis exported and called fromgetCSRFRequestHeadersinParseGraphQLServer.jswiththis.parseServer.config.headerAliases, which could theoretically beundefinedbefore defaults are applied or if explicitly set tonull. Whilesrc/defaults.jsprovidesheaderAliases: {}as default, adding a guard improves robustness.♻️ Proposed fix
export function getHeaderAliases(headerAliases, canonicalHeader) { + if (!headerAliases || typeof headerAliases !== 'object') { + return []; + } const aliases = headerAliases[canonicalHeader]; if (!Array.isArray(aliases)) { return []; } // Clean up the aliases and remove any empty strings return aliases.map(alias => alias.trim()).filter(Boolean); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/middlewares.js` around lines 86 - 93, The getHeaderAliases function should defensively handle a null/undefined headerAliases argument: check that headerAliases is an object (and not null) before accessing headerAliases[canonicalHeader], and return an empty array if headerAliases is missing or not an object; then proceed to validate that the retrieved aliases is an array and perform the existing trim/filter logic. Update getHeaderAliases to early-return [] when headerAliases is falsy or not an object to avoid runtime errors when called (e.g., from getCSRFRequestHeaders in ParseGraphQLServer.js) and keep the rest of the function (aliases.map(...).filter(Boolean)) unchanged.
🤖 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 86-93: The getHeaderAliases function should defensively handle a
null/undefined headerAliases argument: check that headerAliases is an object
(and not null) before accessing headerAliases[canonicalHeader], and return an
empty array if headerAliases is missing or not an object; then proceed to
validate that the retrieved aliases is an array and perform the existing
trim/filter logic. Update getHeaderAliases to early-return [] when headerAliases
is falsy or not an object to avoid runtime errors when called (e.g., from
getCSRFRequestHeaders in ParseGraphQLServer.js) and keep the rest of the
function (aliases.map(...).filter(Boolean)) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23d169ab-15ff-4a69-afe5-7aed7b82a141
📒 Files selected for processing (6)
spec/HeaderAliasesValidation.spec.jsspec/Middlewares.spec.jsspec/ParseGraphQLServer.spec.jssrc/Config.jssrc/GraphQL/ParseGraphQLServer.jssrc/middlewares.js
🚧 Files skipped from review as they are similar to previous changes (2)
- src/Config.js
- spec/ParseGraphQLServer.spec.js
Member
|
@Moumouls Is this ready for review? I see an open CR feedback... |
Member
|
Rebased... |
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
Security hardening through header masking:
According to OWASP recommendations, it is best practice to mask or obfuscate technology-specific details whenever possible. For example, headers such as X-Parse-Session-Token and X-Parse-Application-Id clearly reveal the backend technology stack and should be renamed or aliased to reduce exposure.
Session management in micro frontend architectures or multi parse-server under one domain:
In a micro frontend setup with multiple Parse Server instances under the same domain, developers may not be able to use HttpOnly cookies. As a result, they might be forced to rely on alternative methods, often less secure, such as localStorage or sessionStorage to manage session tokens across instances.
This feature allows developers to configure alias names for session headers and application IDs, helping maintain flexibility while improving security.
Approach
Alias system: redirects the value of a custom (aliased) header to the corresponding internally recognized Parse header.
Made with Composer 2 and Codex 5.3 High + manually reduced implementation complexity
Tasks
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation