-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
JS: React-relay support #18858
base: main
Are you sure you want to change the base?
JS: React-relay support #18858
Conversation
a107b89
to
aea5fed
Compare
… as a source but isn't
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.
PR Overview
This pull request adds support for the react-relay library by introducing a new model that classifies specific react-relay hook return values as a "response" threat and extends test coverage for DOM-based XSS scenarios.
- Introduces a YAML file to model response threats for various react-relay hooks
- Adds corresponding test cases in a React component to validate DOM-based XSS alerts
- Updates change notes to document the new support for react-relay
Reviewed Changes
File | Description |
---|---|
javascript/ql/lib/ext/react-relay-threat.model.yml | Adds model definitions for marking react-relay hook return values as "response" threats |
javascript/ql/test/query-tests/Security/CWE-079/DomBasedXssWithResponseThreat/testReactRelay.tsx | Provides test cases for new response threat scenarios using react-relay hooks |
javascript/ql/lib/change-notes/2025-02-25-react-relay.md | Notes the addition of react-relay support |
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Tip: Leave feedback on Copilot's review comments with the 👎 and 👍 buttons to help improve review quality. Learn more
- ["react-relay", "Member[useLazyLoadQuery].ReturnValue", "response"] | ||
- ["react-relay", "Member[usePreloadedQuery].ReturnValue", "response"] | ||
- ["react-relay", "Member[useClientQuery].ReturnValue", "response"] | ||
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"] |
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.
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"] | |
- ["react-relay", "Member[useRefetchableFragment].ReturnValue.Member[0]", "response"] |
Based on the code sample it seems the data is returned in the first array element:
const [data, refetch] = useRefetchableFragment(
- ["react-relay", "Member[usePreloadedQuery].ReturnValue", "response"] | ||
- ["react-relay", "Member[useClientQuery].ReturnValue", "response"] | ||
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"] | ||
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"] |
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.
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"] | |
- ["react-relay", "Member[usePaginationFragment].ReturnValue.Member[data]", "response"] |
- ["react-relay", "Member[useClientQuery].ReturnValue", "response"] | ||
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"] | ||
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"] | ||
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"] |
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.
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"] | |
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Parameter[0]", "response"] |
For complicated reasons our implementation considers Argument
and Parameter
to be interchangeable in some places, but it's more appropriate to use Parameter
here.
- ["react-relay", "Member[useRefetchableFragment].ReturnValue", "response"] | ||
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"] | ||
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"] | ||
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Argument[0]", "response"] |
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.
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Argument[0]", "response"] | |
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Parameter[0]", "response"] |
- ["react-relay", "Member[usePaginationFragment].ReturnValue", "response"] | ||
- ["react-relay", "Member[useMutation].ReturnValue.Member[0].Argument[0].Member[onCompleted].Argument[0]", "response"] | ||
- ["react-relay", "Member[useSubscription].Argument[0].Member[onNext].Argument[0]", "response"] | ||
- ["react-relay", "Member[fetchQuery].ReturnValue.Member[subscribe].Argument[0].Member[next].Argument[0]", "response"] |
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.
- ["react-relay", "Member[fetchQuery].ReturnValue.Member[subscribe].Argument[0].Member[next].Argument[0]", "response"] | |
- ["react-relay", "Member[fetchQuery].ReturnValue.Member[subscribe].Argument[0].Member[next].Parameter[0]", "response"] |
const [data, refetch] = useRefetchableFragment(query, props.comment); // $ Source | ||
return ( | ||
<> | ||
<h1 dangerouslySetInnerHTML={{ __html: data }} /> // $ Alert |
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.
Maybe add a line here to ensure refetch
isn't seen as a source.
<h1 dangerouslySetInnerHTML={{ __html: data }} /> // $ Alert | |
<h1 dangerouslySetInnerHTML={{ __html: data }} /> // $ Alert | |
<h1 dangerouslySetInnerHTML={{ __html: refetch }} /> // OK |
This pull request introduces a library model for
react-relay
's, categorizing it's new sources as aresponse
threat.Closes #465