feat: Deprecate GraphQL Playground that exposes master key in HTTP response #10112
feat: Deprecate GraphQL Playground that exposes master key in HTTP response #10112mtrezza wants to merge 2 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. 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. |
✅ 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. |
📝 WalkthroughWalkthroughAdds deprecations for GraphQL Playground options (mountPlayground, playgroundPath), updates deprecation handling and messages, and adds tests covering the new deprecation logging behavior. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![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
🧹 Nitpick comments (2)
spec/Deprecator.spec.js (1)
130-151: Add a regression test for the shippedplaygroundPathdeprecation entry.The production change deprecates both
mountPlaygroundandplaygroundPath, but this block only asserts themountPlaygroundentry. A typo or accidental removal ofplaygroundPathfromsrc/Deprecator/Deprecations.jswould still pass this suite.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@spec/Deprecator.spec.js` around lines 130 - 151, Add an assertion to the test block that also verifies Deprecator._logOption is called for the 'playgroundPath' deprecation when playground-related options are set: in the 'logs deprecation for mountPlayground when set' test (using reconfigureServer and the logSpy on Deprecator._logOption) add an expect that toHaveBeenCalledWith jasmine.objectContaining({ optionKey: 'playgroundPath', changeNewKey: '' }) so the suite fails if the playgroundPath deprecation entry is removed or renamed.src/ParseServer.ts (1)
459-463: Avoid double-warning formountPlayground.Line 69 already routes this option through
Deprecator.scanParseServerOptions(options), so enabling the playground now emits the generic deprecation warning plus this second warning. Consider keeping a single deprecation warning here, or making this log security-specific only.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ParseServer.ts` around lines 459 - 463, The code emits a second generic deprecation warning when options.mountPlayground is true even though Deprecator.scanParseServerOptions(options) already logged a deprecation; update the logic around parseGraphQLServer.applyPlayground to avoid duplicate warnings by either removing the generic deprecation log or making it security-specific. Specifically, in the block that checks options.mountPlayground and calls parseGraphQLServer.applyPlayground(app), remove or replace the logging.getLogger().warn(...) call so it no longer duplicates Deprecator.scanParseServerOptions(options); if you choose to preserve a log, change it to a focused security warning about master-key exposure (mentioning master key) rather than a generic deprecation 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/Deprecator/Deprecations.js`:
- Around line 29-38: The options metadata still lists mountPlayground and
playgroundPath as supported; mark both option entries as deprecated in the
options metadata (add deprecated: true and a deprecation message for optionKey
'mountPlayground' and 'playgroundPath' in the Options index so runtime and
generated docs reflect deprecation), then regenerate the Options definitions and
docs so Definitions.js and docs.js include the deprecation flags and updated
solutions; ensure the option entries in the generated output include the same
deprecation message text as in the Deprecator entries to keep runtime warnings
and canonical metadata consistent.
---
Nitpick comments:
In `@spec/Deprecator.spec.js`:
- Around line 130-151: Add an assertion to the test block that also verifies
Deprecator._logOption is called for the 'playgroundPath' deprecation when
playground-related options are set: in the 'logs deprecation for mountPlayground
when set' test (using reconfigureServer and the logSpy on Deprecator._logOption)
add an expect that toHaveBeenCalledWith jasmine.objectContaining({ optionKey:
'playgroundPath', changeNewKey: '' }) so the suite fails if the playgroundPath
deprecation entry is removed or renamed.
In `@src/ParseServer.ts`:
- Around line 459-463: The code emits a second generic deprecation warning when
options.mountPlayground is true even though
Deprecator.scanParseServerOptions(options) already logged a deprecation; update
the logic around parseGraphQLServer.applyPlayground to avoid duplicate warnings
by either removing the generic deprecation log or making it security-specific.
Specifically, in the block that checks options.mountPlayground and calls
parseGraphQLServer.applyPlayground(app), remove or replace the
logging.getLogger().warn(...) call so it no longer duplicates
Deprecator.scanParseServerOptions(options); if you choose to preserve a log,
change it to a focused security warning about master-key exposure (mentioning
master key) rather than a generic deprecation message.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 37818ee6-2ff2-415c-bd43-17d86e21a79b
📒 Files selected for processing (5)
DEPRECATIONS.mdspec/Deprecator.spec.jssrc/Deprecator/Deprecations.jssrc/Deprecator/Deprecator.jssrc/ParseServer.ts
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## alpha #10112 +/- ##
=======================================
Coverage 92.68% 92.68%
=======================================
Files 191 191
Lines 15879 15882 +3
Branches 180 180
=======================================
+ Hits 14717 14720 +3
Misses 1150 1150
Partials 12 12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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/Options/index.js`:
- Around line 342-347: Update the deprecation text to reference the announced
removal timeline by replacing vague "future version" wording with "next major
release" in the user-facing comments; specifically edit the comment string for
the mountPlayground option (symbol: mountPlayground) and the playground mount
path comment (environment symbol: PARSE_SERVER_PLAYGROUND_PATH) so both
explicitly say the playground will be removed in the "next major release".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aba02a0d-b600-451c-b377-7a2c93ccf998
📒 Files selected for processing (3)
src/Options/Definitions.jssrc/Options/docs.jssrc/Options/index.js
✅ Files skipped from review due to trivial changes (1)
- src/Options/Definitions.js
2 participants
Pull Request
Issue
Closes: #10110
Tasks
Summary by CodeRabbit
Documentation
Tests