-
Notifications
You must be signed in to change notification settings - Fork 91
Internalize json_rpc_handler along with an XSS vulnerability fix #175
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Internalize json_rpc_handler along with an XSS vulnerability fix #175
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I preserved the name, but this could be moved to a nicer, json-rpc folder to follow suit with mcp.
| PARSE_ERROR = -32700 | ||
| end | ||
|
|
||
| DEFAULT_ALLOWED_ID_CHARACTERS = /\A[a-zA-Z0-9_-]+\z/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a follow up, we need to decide whether we want to keep this as configurable through the mcp gem, or allow developers to punch through layers to be able to directly override it at JSON-RPC module level, or simply make the mcp gem super opinionated about always having strict id validation. Heuristically this pattern should cover any non-pen-tester scenarios.
|
Thank you! |
Motivation and Context
The ruby-sdk gem is the only popular gem that uses the json_rpc_handler gem. For tweaks around transport error handling, there is an ongoing awkwardness of working around the opinions of the json_rpc_handler gem or needing to extend that gem to make its API more flexible.
Just internalizing the JSON-RPC handling will alllow mcp gem development to move faster.
Incidentally, we also pick up an unreleased security fix on json_rpc_handler:
Reviewers: Follow individual commits to see the incremental steps I took in copying things over, so you're not tasked with reviewing the entire internalized "new" code. Code copied from commit Shopify/json-rpc-handler@b41c412
How Has This Been Tested?
Existing unit tests pass.
Breaking Changes
The gem becomes stringent on the format of JSON-RPC call ids by default, to prevent an XSS vulnerability.
Types of changes
Checklist
Additional context