parse-server icon indicating copy to clipboard operation
parse-server copied to clipboard

Add support for global and per-route security headers in PagesRouter

Open J1vvoo opened this issue 2 weeks ago • 6 comments

Pull Request

Issue

Closes: #9951

Approach

Implemented global and per-route security header support in PagesRouter.

  • Added pages.headers for global HTML-page headers
  • Added headers option for each custom route
  • Updated pageResponse, fileResponse, redirectResponse to merge headers properly
  • Ensures backward compatibility and applies headers only to HTML page responses

Tasks

  • [ ] Add tests
  • [ ] Add changes to documentation (guides, repository pages, code comments)

Summary by CodeRabbit

  • New Features
    • Added configurable global headers to apply default headers across all page responses.
    • Added per-route headers to apply custom headers for specific page routes.
    • Improved header handling so global and route-specific headers are merged and consistently propagated through all response paths.

✏️ Tip: You can customize this high-level summary in your review settings.

J1vvoo avatar Dec 02 '25 06:12 J1vvoo

🚀 Thanks for opening this pull request!

:white_check_mark: Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
:white_check_mark: Open Source Security 0 0 0 0 0 issues

:computer: Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

parseplatformorg avatar Dec 02 '25 06:12 parseplatformorg

📝 Walkthrough

Walkthrough

Adds a new headers option to PagesOptions and PagesRoute, and updates PagesRouter to accept, propagate, and merge per-route headers with global and per-call headers; expressRouter now applies configured security and custom headers before delegating to downstream handlers.

Changes

Cohort / File(s) Summary
Configuration Definitions
src/Options/Definitions.js
Added headers property to PagesOptions (global, env: PARSE_SERVER_PAGES_HEADERS) and to PagesRoute (per-route, env: PARSE_SERVER_PAGES_ROUTE_HEADERS), both using objectParser with default {} and exposed in public declarations.
Pages router & response plumbing
src/Routers/PagesRouter.js
Extended method signatures: goToPage(req, page, params = {}, responseType, routeHeaders = {}), pageResponse(path, params = {}, placeholders = {}, routeHeaders = {}), fileResponse(path, routeHeaders = {}), redirectResponse(url, params, routeHeaders = {}). Threaded routeHeaders through mountCustomRoutes, redirect/page/file responses, and unified header merging (global → route → per-call). expressRouter() applies security and custom headers from _pagesOptions before delegating; custom route handlers forward routeHeaders into goToPage calls.

Sequence Diagram(s)

mermaid sequenceDiagram participant Client participant ExpressMiddleware as Express (PagesRouter.expressRouter) participant CustomRouteHandler as Route Handler participant PagesRouter as PagesRouter.goToPage participant ResponseBuilder as pageResponse/fileResponse/redirectResponse Client->>ExpressMiddleware: HTTP request ExpressMiddleware->>ExpressMiddleware: apply global security & custom headers ExpressMiddleware->>CustomRouteHandler: dispatch to mounted route CustomRouteHandler->>PagesRouter: call goToPage(..., routeHeaders) PagesRouter->>ResponseBuilder: build response with global + route + per-call headers ResponseBuilder-->>Client: HTTP response (merged headers)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

  • Verify header precedence and merging in pageResponse(), fileResponse(), and redirectResponse() (global → route → per-call).
  • Confirm expressRouter() applies security headers correctly and allows overrides from custom headers or per-route headers.
  • Ensure mountCustomRoutes() and custom route handlers consistently pass routeHeaders to goToPage().
  • Check backward compatibility for callers of modified methods and that default {} prevents breaking behavior.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description provides good approach details but fails to address critical implementation gaps noted by reviewers: headers properties are missing from src/Options/index.js and npm run definitions was not executed. Add headers properties to PagesOptions and PagesRoute in src/Options/index.js, then run npm run definitions to properly generate Definitions.js and docs.js changes.
Out of Scope Changes check ⚠️ Warning Changes were added directly to src/Options/Definitions.js without following the required workflow (modifying src/Options/index.js first and running npm run definitions), which is outside the proper development process for Parse Server options. Revert manual Definitions.js changes and instead add properties to src/Options/index.js, then execute npm run definitions to auto-generate all required file updates.
Linked Issues check ❓ Inconclusive PR addresses #9951 partially by implementing headers for PagesRouter only, but scope appears narrower than the linked issue's requirement for global application across entire ParseServer.app request pipeline. Clarify whether the PR is a partial implementation (PagesRouter-only) of #9951 or if scope differs; ensure full pipeline coverage as requested in the issue.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically summarizes the main change: adding support for global and per-route security headers in PagesRouter.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60011079e551fd6d4c9ee14806082968be19f5a5 and 81614be1b7b701311f1d7551f0665b9d7e7fa529.

📒 Files selected for processing (1)
  • src/Options/Definitions.js (2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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-12-02T06:55:53.788Z
Learning: When reviewing Parse Server PRs that add or modify Parse Server options, always verify that changes are properly reflected in three files: src/Options/index.js (where changes originate), src/Options/Definitions.js, and src/Options/docs.js. The correct workflow is: make changes in index.js first, then run `npm run definitions` to automatically replicate the changes to Definitions.js and docs.js.
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.
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.
📚 Learning: 2025-11-08T13:46:04.940Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

Applied to files:

  • src/Options/Definitions.js
📚 Learning: 2025-12-02T06:55:53.788Z
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-12-02T06:55:53.788Z
Learning: When reviewing Parse Server PRs that add or modify Parse Server options, always verify that changes are properly reflected in three files: src/Options/index.js (where changes originate), src/Options/Definitions.js, and src/Options/docs.js. The correct workflow is: make changes in index.js first, then run `npm run definitions` to automatically replicate the changes to Definitions.js and docs.js.

Applied to files:

  • src/Options/Definitions.js
🧬 Code graph analysis (1)
src/Options/Definitions.js (1)
resources/buildConfigDefinitions.js (1)
  • parsers (12-12)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Benchmarks
🔇 Additional comments (1)
src/Options/Definitions.js (1)

812-817: PagesRoute.headers shape is consistent and aligns with PagesOptions.headers.

Per‑route headers are defined analogously to the global option:

  • pages‑route‑scoped env name PARSE_SERVER_PAGES_ROUTE_HEADERS.
  • parsers.objectParser with default: {}.
  • Help text clearly indicates route‑specific application.

This is a good, minimal public surface for route‑level customization and matches the global headers option semantics.


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

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

coderabbitai[bot] avatar Dec 02 '25 06:12 coderabbitai[bot]

@coderabbitai When reviewing a PR that contains changes to the Parse Server options, always make sure the changes are reflected in src/Options/Definitions.js, src/Options/docs.js and src/Options/index.js. The changes are made originally in index.js and then via npm run definitions replicated in the other 2 files.

mtrezza avatar Dec 02 '25 06:12 mtrezza

[!TIP] For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the new headers options are present in all three options files

echo "=== Checking src/Options/index.js ==="
rg -n -A5 -B2 'headers.*:.*{' src/Options/index.js | head -50

echo -e "\n=== Checking src/Options/Definitions.js ==="
rg -n -A5 -B2 'headers.*:.*{' src/Options/Definitions.js | head -50

echo -e "\n=== Checking src/Options/docs.js ==="
rg -n -A5 -B2 'headers.*:.*{' src/Options/docs.js | head -50

echo -e "\n=== Checking if PagesOptions headers is defined in index.js ==="
rg -n -C10 'PagesOptions' src/Options/index.js | rg -C5 'headers'

echo -e "\n=== Checking if PagesRoute headers is defined in index.js ==="
rg -n -C10 'PagesRoute.*{' src/Options/index.js | rg -C5 'headers'

Length of output: 608


🏁 Script executed:

#!/bin/bash
# Check if the new headers options are present in all three options files

echo "=== Checking src/Options/index.js ==="
rg -n 'headers' src/Options/index.js

echo -e "\n=== Checking src/Options/Definitions.js ==="
rg -n 'headers' src/Options/Definitions.js

echo -e "\n=== Checking src/Options/docs.js ==="
rg -n 'headers' src/Options/docs.js

echo -e "\n=== Detailed look at PagesOptions in index.js ==="
ast-grep --pattern 'PagesOptions = {
  $$$
}'

echo -e "\n=== Detailed look at PagesRoute in index.js ==="
ast-grep --pattern 'PagesRoute = {
  $$$
}'

Length of output: 521


🏁 Script executed:

#!/bin/bash
# Get detailed sections from all three files

echo "=== PagesOptions section in src/Options/index.js ==="
rg -n -A50 'export class PagesOptions' src/Options/index.js

echo -e "\n=== PagesRoute section in src/Options/index.js ==="
rg -n -A30 'export class PagesRoute' src/Options/index.js

echo -e "\n=== Lines around headers in Definitions.js ==="
rg -n -B10 -A10 'headers.*{' src/Options/Definitions.js | head -60

echo -e "\n=== Check docs.js for PagesOptions ==="
rg -n -A50 '@property.*pages' src/Options/docs.js | head -80

Length of output: 19811


🏁 Script executed:

#!/bin/bash
# Get detailed view of how options are structured in these files

echo "=== Full structure of PagesOptions in index.js ==="
rg -n -A100 'PagesOptions' src/Options/index.js | head -150

echo -e "\n=== Full structure of PagesOptions in Definitions.js ==="
rg -n -B5 -A20 'PagesOptions' src/Options/Definitions.js | head -100

echo -e "\n=== Check around line 782 in Definitions.js (where headers appears) ==="
sed -n '770,800p' src/Options/Definitions.js

echo -e "\n=== Full structure check in docs.js for PagesOptions details ==="
rg -n -A100 'PagesOptions.*=|class PagesOptions' src/Options/docs.js | head -150

Length of output: 12941


🏁 Script executed:

#!/bin/bash
# Verify PagesRoute headers and check docs.js

echo "=== PagesRoute section in Definitions.js around line 805 ==="
sed -n '795,820p' src/Options/Definitions.js

echo -e "\n=== Check if docs.js has PagesOptions type definition ==="
rg -n -A50 '@typedef.*PagesOptions' src/Options/docs.js

echo -e "\n=== Check if docs.js has PagesRoute type definition ==="
rg -n -A50 '@typedef.*PagesRoute' src/Options/docs.js

Length of output: 1014


@mtrezza, I've verified the Parse Server options files and found an issue with the workflow in this PR:

Current state:

  • src/Options/Definitions.js - Contains the new headers properties for both PagesOptions (line 782) and PagesRoute (line 805)
  • src/Options/index.js - Missing the headers properties in both PagesOptions interface (lines 396-426) and PagesRoute interface (lines 428-435)
  • src/Options/docs.js - No typedef documentation for these properties

The issue: The changes were added directly to Definitions.js instead of following the correct workflow. According to your guidance, changes should originate in index.js and then be replicated to the other files via npm run definitions.

Required fix: The PR author (@J1vvoo) needs to:

  1. Add the headers property to the PagesOptions interface in src/Options/index.js:
export interface PagesOptions {
  // ... existing properties ...
  /* Global headers applied to all PagesRouter responses.
  :DEFAULT: {} */
  headers: ?Object;
}
  1. Add the headers property to the PagesRoute interface in src/Options/index.js:
export interface PagesRoute {
  // ... existing properties ...
  /* Headers applied only to this specific page route.
  :DEFAULT: {} */
  headers: ?Object;
}
  1. Run npm run definitions to replicate changes to Definitions.js and docs.js

I'll remember this verification requirement for future Parse Server PRs. Thank you for the guidance!


✏️ Learnings added
Learnt from: mtrezza
URL: https://github.com/parse-community/parse-server/pull/9958

Timestamp: 2025-12-02T06:55:53.788Z
Learning: When reviewing Parse Server PRs that add or modify Parse Server options, always verify that changes are properly reflected in three files: src/Options/index.js (where changes originate), src/Options/Definitions.js, and src/Options/docs.js. The correct workflow is: make changes in index.js first, then run `npm run definitions` to automatically replicate the changes to Definitions.js and docs.js.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: mtrezza
Repo: parse-community/parse-server PR: 0
File: :0-0
Timestamp: 2025-11-08T13:46:04.940Z
Learning: For new Parse Server options, verify that the option is documented in src/Options/index.js and that npm run definitions has been executed to reflect changes in src/Options/docs.js and src/Options/Definitions.js. README.md documentation is a bonus but not required for new options.

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: 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: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: src/triggers.js:446-454
Timestamp: 2025-08-27T09:08:34.252Z
Learning: When analyzing function signature changes in Parse Server codebase, verify that call sites are actually incorrect before flagging them. Passing tests are a strong indicator that function calls are already properly aligned with new signatures.

Learnt from: EmpiDev
Repo: parse-community/parse-server PR: 9770
File: spec/CloudCode.spec.js:446-469
Timestamp: 2025-08-26T14:06:31.853Z
Learning: In the Parse Server codebase, when handling query objects in maybeRunAfterFindTrigger, objects without a where property that contain options like limit/skip should be treated as query JSON with an empty where clause using the spread pattern { where: {}, ...query }, not nested as { where: query }.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: Tests in the parse-server repository should use promise-based approaches rather than callback patterns with `done()`. Use a pattern where a Promise is created that resolves when the event occurs, then await that promise.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1312-1338
Timestamp: 2025-05-04T20:41:05.147Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`.

Learnt from: mtrezza
Repo: parse-community/parse-server PR: 9445
File: spec/ParseLiveQuery.spec.js:1340-1375
Timestamp: 2025-05-09T09:59:06.289Z
Learning: New tests in the parse-server repository should use async/await with promise-based patterns rather than callback patterns with `done()`. The preferred pattern is to create a Promise that resolves when an expected event occurs, then await that Promise.

Learnt from: RahulLanjewar93
Repo: parse-community/parse-server PR: 9744
File: spec/ParseLiveQuery.spec.js:0-0
Timestamp: 2025-04-30T19:31:35.344Z
Learning: In the Parse Server codebase, the functions in QueryTools.js are typically tested through end-to-end behavior tests rather than direct unit tests, even though the functions are exported from the module.

coderabbitai[bot] avatar Dec 02 '25 06:12 coderabbitai[bot]

Codecov Report

:x: Patch coverage is 84.00000% with 4 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 93.71%. Comparing base (d4c6de0) to head (6001107). :warning: Report is 11 commits behind head on alpha.

Files with missing lines Patch % Lines
src/Routers/PagesRouter.js 84.00% 4 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##            alpha    #9958      +/-   ##
==========================================
+ Coverage   93.08%   93.71%   +0.63%     
==========================================
  Files         188      188              
  Lines       15298    15314      +16     
  Branches      177      177              
==========================================
+ Hits        14240    14352     +112     
+ Misses       1046      950      -96     
  Partials       12       12              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Dec 02 '25 06:12 codecov[bot]