fix: Endpoint /upgradeToRevocableSession ignores _Session protectedFields

[mtrezza]

Pull Request

• Report security issues confidentially.
• Any contribution is under this license.

Issue

Endpoint /upgradeToRevocableSession ignores _Session protectedFields

Tasks

• Add tests
• Add changes to documentation (guides, repository pages, code comments)
• Add security check
• Add new Parse Error codes to Parse JS SDK

[parse-github-assistant]

🚀 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

• Keep pull requests small. Large PRs will be rejected. Break complex features into smaller, incremental PRs.
• Use Test Driven Development. Write failing tests before implementing functionality. Ensure tests pass.
• Group code into logical blocks. Add a short comment before each block to explain its purpose.
• We offer conceptual guidance. Coding is up to you. PRs must be merge-ready for human review.
• Our review focuses on concept, not quality. PRs with code issues will be rejected. Use an AI agent.
• Human review time is precious. Avoid review ping-pong. Inspect and test your AI-generated code.

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 and design rather than using review cycles to fix low-quality code.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7ebe43df-906d-4caa-a5c6-85aeff863a4d

📥 Commits

Reviewing files that changed from the base of the PR and between 87b8729 and 87ae27e.

📒 Files selected for processing (1)

• src/Routers/SessionsRouter.js

🚧 Files skipped from review as they are similar to previous changes (1)

• src/Routers/SessionsRouter.js

---

📝 Walkthrough

Walkthrough

Adds a test for upgrading a legacy session to a revocable session with protected fields configured as non-exempt, and refactors the revocable-session upgrade handler to async/await and to re-fetch the created _Session using non-master auth so protected-field filtering is applied.

Changes

Cohort / File(s)	Summary
Revocable Session Upgrade Test spec/RevocableSessionsUpgrade.spec.js	New test that configures protectedFieldsSaveResponseExempt: false, marks _Session fields createdWith and installationId as protected, upgrades a legacy user session via POST /upgradeToRevocableSession (using session token and installation id), and asserts returned sessionToken begins with r: and that protected fields are omitted.
Session Handler Refactor src/Routers/SessionsRouter.js	Converted handleUpdateToRevocableSession to async/await. After creating the new session and clearing the old _User sessionToken, it re-fetches the created _Session using rest.find with a non-master Auth derived from the caller; returns the first result or throws an INTERNAL_SERVER_ERROR if none found.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant SessionsRouter
    participant Database
    participant Auth
    participant RestFind

    Client->>SessionsRouter: POST /upgradeToRevocableSession (sessionToken, installationId)
    SessionsRouter->>Database: createSession() (master auth)
    Database-->>SessionsRouter: new _Session record
    SessionsRouter->>Database: clear legacy _User.sessionToken
    Database-->>SessionsRouter: deletion ack
    SessionsRouter->>Auth: build non-master Auth from caller context
    Auth-->>SessionsRouter: non-master Auth instance
    SessionsRouter->>RestFind: rest.find(_Session, query, non-master Auth)
    RestFind->>Database: query _Session
    Database-->>RestFind: results (filtered by protectedFields/CLP)
    RestFind-->>SessionsRouter: filtered session result
    SessionsRouter-->>Client: { response: filtered session }

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• fix: Auth data exposed via /users/me endpoint (GHSA-37mj-c2wf-cx96) #10278: Also re-fetches resources with caller non-master auth after master-level operations to enforce protected-field filtering.
• fix: Endpoint /sessions/me bypasses _Session protectedFields (GHSA-g4v2-qx3q-4p64) #10406: Makes SessionsRouter handler async and re-fetches _Session records with non-master Auth to apply protectedFields and CLP.

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Description check	⚠️ Warning	No pull request description was provided by the author; the PR is missing required sections from the template including Issue, Approach, and Tasks checklist.	Add a comprehensive PR description following the template: include a link to the related issue, describe the changes made to fix the problem, and complete the relevant tasks checklist.
Engage In Review Feedback	❓ Inconclusive	Cannot verify engagement with PR review feedback comments due to lack of GitHub API access in the sandbox environment.	Provide direct access to PR review comments or grant GitHub API access to query PR #10408 review data.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title begins with the required 'fix:' prefix and clearly describes the main issue being addressed: the /upgradeToRevocableSession endpoint not respecting _Session protectedFields configuration.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Security Check	✅ Passed	The pull request implements a security fix that properly addresses the protected fields vulnerability in the /upgradeToRevocableSession endpoint by using a non-master Auth instance with re-fetching via rest.find() to ensure protectedFields filtering and ACL checks are applied.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

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/Routers/SessionsRouter.js`:
- Around line 92-101: Guard against empty or missing results from the rest.find
call before indexing into response.results[0]; in the SessionsRouter.js flow
where you call rest.find with config, userAuth, '_Session', and
sessionData.sessionToken, check that response and response.results exist and
have at least one element, and return a safe value or throw a controlled error
(e.g., return null or an explicit error/401) instead of directly returning
response.results[0].

🪄 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: 92d0984d-c755-44ed-9986-fa56ef0d52c5

📥 Commits

Reviewing files that changed from the base of the PR and between 8b19852 and 87b8729.

📒 Files selected for processing (2)

• spec/RevocableSessionsUpgrade.spec.js
• src/Routers/SessionsRouter.js

[mtrezza]

@coderabbitai review your feedback conversations and resolve where you found a consensus

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Get all review comments on PR `#10408`
gh api repos/parse-community/parse-server/pulls/10408/comments --paginate \
  | jq '[.[] | {id, body: .body[:200], user: .user.login, path: .path, line: .line, in_reply_to_id}]'

Length of output: 1114

---

🏁 Script executed:

#!/bin/bash
# Get review threads via GraphQL to find thread IDs
gh api graphql -f query='
{
  repository(owner: "parse-community", name: "parse-server") {
    pullRequest(number: 10408) {
      reviewThreads(first: 50) {
        nodes {
          id
          isResolved
          comments(first: 5) {
            nodes {
              databaseId
              author { login }
              body
            }
          }
        }
      }
    }
  }
}' | jq '.data.repository.pullRequest.reviewThreads.nodes[] | {id, isResolved, firstComment: .comments.nodes[0] | {databaseId, author: .author.login, body: .body[:100]}}'

Length of output: 260

---

@mtrezza: I reviewed all feedback conversations on this PR. There is only one review thread — my concern about guarding empty rest.find results before returning results[0] — and it is already resolved. The fix was confirmed in the thread (you added a guard that throws INTERNAL_SERVER_ERROR if the re-fetch returns empty results), and I acknowledged it at that time. No further action is needed.

---

🧠 Learnings used

Learnt from: CR
Repo: parse-community/parse-server PR: 0
File: coderabbit-custom-pre-merge-checks-unique-id-file-non-traceable-F7F2B60C-1728-4C9A-8889-4F2235E186CA.txt:0-0
Timestamp: 2026-04-01T13:35:45.477Z
Learning: Engage in review feedback: The user must engage in review of feedback comments, and not ignore or resolve a review feedback without prior engagement in a discussion and either implementing the feedback via a commit, or convincing the reviewer of their standpoint to retract the feedback.

---

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: 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: 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: 10175
File: src/LiveQuery/ParseLiveQueryServer.ts:919-952
Timestamp: 2026-03-10T18:04:39.708Z
Learning: In Parse Server LiveQuery (`src/LiveQuery/ParseLiveQueryServer.ts`), the `addProtectedFields` method in `DatabaseController` uses **intersection** logic: it computes a protected set for each applicable ACL group (`*`, `userId`, `role:X`) and intersects them. Role entries serve as **exemptions** (they narrow/reduce the protected set for that role), not additions. Consequently, passing `userRoles: []` when building the `auth` object for the protected-fields WHERE-clause check is deliberately more restrictive (not less): without role sets, only the `'*'` set applies, which is the maximum protection. The only correctness gap is a rare role-only config (`{ 'role:X': ['field'] }` with no `'*'` entry), where omitting roles yields an empty intersection and the field is unprotected for the role member — but unprivileged users are unaffected, so this is not a security escalation. The same pattern exists pre-existing in `_filterSensitiveData`. This is intentional and documented with tests.

---

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 10406
File: src/Routers/SessionsRouter.js:35-46
Timestamp: 2026-04-06T12:23:06.465Z
Learning: In parse-community/parse-server (`src/Routers/SessionsRouter.js`), the middleware at `src/middlewares.js:335` deliberately skips session resolution for `GET /sessions/me` (to support expired session token lookups, introduced in PR `#8722`). As a result, `req.auth` is `undefined` for non-master requests on this route. `handleMe` must construct a user-level `Auth.Auth` instance from the validated session result (master-key lookup first, then build Auth from `sessionResponse.results[0].user.objectId`). Do not suggest using `req.auth` directly in `handleMe` — it follows the same "master lookup → caller-auth get" pattern as `UsersRouter.handleMe` (PR `#10278`).

---

Learnt from: Moumouls
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2026-03-28T19:24:55.383Z
Learning: In parse-community/parse-server (PR `#10341`), `requestComplexity.batchRequestLimit` intentionally defaults to `-1` (unlimited) in PS9. This is by design: batch request limiting is opt-in in the current major version and will become a hard default (safe non-unlimited value) in PS10, consistent with the same opt-in policy already established for other `requestComplexity` sub-options (e.g., `includeLimit`). Do not flag the `-1` default as a security gap for PS9.

---

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T08:00:20.138Z
Learning: For Parse Server 9 release (PR `#9938` and related), the parse/push-adapter dependency must be upgraded to version >= 8.0.0, not 7.0.0. Version 8.x drops support for Node 18.

[codecov]

Codecov Report

❌ Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 92.49%. Comparing base (d507575) to head (87ae27e).
⚠️ Report is 3 commits behind head on alpha.

Files with missing lines	Patch %	Lines
src/Routers/SessionsRouter.js	85.71%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha   #10408      +/-   ##
==========================================
- Coverage   92.49%   92.49%   -0.01%     
==========================================
  Files         192      192              
  Lines       16732    16736       +4     
  Branches      229      229              
==========================================
+ Hits        15477    15480       +3     
- Misses       1233     1234       +1     
  Partials       22       22              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[parseplatformorg]

🎉 This change has been released in version 9.8.0-alpha.8