You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/03/18 05:21:09 UTC
[GitHub] [skywalking] kezhenxu94 opened a new pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
kezhenxu94 opened a new pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703
<!--
⚠️ Please make sure to read this template first, pull requests that don't accord with this template
maybe closed without notice.
Texts surrounded by `<` and `>` are meant to be replaced by you, e.g. <framework name>, <issue number>.
Put an `x` in the `[ ]` to mark the item as CHECKED. `[x]`
-->
<!-- ==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👇 ====
### Fix <bug description or the bug issue number or bug issue link>
- [ ] Add a unit test to verify that the fix works.
- [ ] Explain briefly why the bug exists and how to fix it.
==== 🐛 Remove this line WHEN AND ONLY WHEN you're fixing a bug, follow the checklist 👆 ==== -->
<!-- ==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👇 ====
### Improve the performance of <class or module or ...>
- [ ] Add a benchmark for the improvement, refer to [the existing ones](https://github.com/apache/skywalking/blob/master/apm-commons/apm-datacarrier/src/test/java/org/apache/skywalking/apm/commons/datacarrier/LinkedArrayBenchmark.java)
- [ ] The benchmark result.
```text
<Paste the benchmark results here>
```
- [ ] Links/URLs to the theory proof or discussion articles/blogs. <links/URLs here>
==== 📈 Remove this line WHEN AND ONLY WHEN you're improving the performance, follow the checklist 👆 ==== -->
<!-- ==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👇 ====
### <Feature description>
- [ ] If this is non-trivial feature, paste the links/URLs to the design doc.
- [ ] Update the documentation to include this new feature.
- [ ] Tests(including UT, IT, E2E) are added to verify the new feature.
- [ ] If it's UI related, attach the screenshots below.
==== 🆕 Remove this line WHEN AND ONLY WHEN you're adding a new feature, follow the checklist 👆 ==== -->
- [x] If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes #8694 .
- [x] Update the [`CHANGES` log](https://github.com/apache/skywalking/blob/master/CHANGES.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703#discussion_r829731099
##########
File path: .github/workflows/skywalking.yaml
##########
@@ -18,17 +18,18 @@ name: CI
on:
pull_request:
- paths-ignore:
- - "*.md"
- - "*.txt"
- - "skywalking-ui"
- - ".asf.yaml"
- - ".dlc.yaml"
- - ".licenserc.yaml"
- - "docs/menu.yml"
- - ".github/workflows/codeql.yaml"
- - "dist-material/release-docs"
- - "*/component-libraries.yml"
+ paths:
Review comment:
There are some issues in the conditions to run / not to run the workflows, will take another look, just reverted these
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703#issuecomment-1072043935
Drafting to see whether the default value is suitable for all tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703#discussion_r829703035
##########
File path: docs/en/setup/backend/configuration-vocabulary.md
##########
@@ -230,6 +230,7 @@ core|default|role|Option values: `Mixed/Receiver/Aggregator`. **Receiver** mode
| - | - | sampleRate | Sampling rate for receiving trace. Precise to 1/10000. 10000 means sampling rate of 100% by default. | SW_RECEIVER_BROWSER_SAMPLE_RATE | 10000 |
| query | graphql | - | GraphQL query implementation. | - |
| - | - | enableLogTestTool | Enable the log testing API to test the LAL. **NOTE**: This API evaluates untrusted code on the OAP server. A malicious script can do significant damage (steal keys and secrets, remove files and directories, install malware, etc). As such, please enable this API only when you completely trust your users. | SW_QUERY_GRAPHQL_ENABLE_LOG_TEST_TOOL | false |
+| - | - | maxQueryComplexity | Maximum complexity allowed for the GraphQL query that can be used to abort a query if the total number of data fields queried exceeds the defined threshold. | SW_QUERY_MAX_QUERY_COMPLEXITY | 30 |
Review comment:
Let's make it 50 or 100. Today new dashboard could set up a complex page.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703#issuecomment-1072043935
Drafting to see whether the default value is suitable for all tests
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 commented on a change in pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
kezhenxu94 commented on a change in pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703#discussion_r829731099
##########
File path: .github/workflows/skywalking.yaml
##########
@@ -18,17 +18,18 @@ name: CI
on:
pull_request:
- paths-ignore:
- - "*.md"
- - "*.txt"
- - "skywalking-ui"
- - ".asf.yaml"
- - ".dlc.yaml"
- - ".licenserc.yaml"
- - "docs/menu.yml"
- - ".github/workflows/codeql.yaml"
- - "dist-material/release-docs"
- - "*/component-libraries.yml"
+ paths:
Review comment:
There are some issues in the conditions to run / not to run the workflows, will take another look, just reverted these
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 merged pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703#discussion_r829703035
##########
File path: docs/en/setup/backend/configuration-vocabulary.md
##########
@@ -230,6 +230,7 @@ core|default|role|Option values: `Mixed/Receiver/Aggregator`. **Receiver** mode
| - | - | sampleRate | Sampling rate for receiving trace. Precise to 1/10000. 10000 means sampling rate of 100% by default. | SW_RECEIVER_BROWSER_SAMPLE_RATE | 10000 |
| query | graphql | - | GraphQL query implementation. | - |
| - | - | enableLogTestTool | Enable the log testing API to test the LAL. **NOTE**: This API evaluates untrusted code on the OAP server. A malicious script can do significant damage (steal keys and secrets, remove files and directories, install malware, etc). As such, please enable this API only when you completely trust your users. | SW_QUERY_GRAPHQL_ENABLE_LOG_TEST_TOOL | false |
+| - | - | maxQueryComplexity | Maximum complexity allowed for the GraphQL query that can be used to abort a query if the total number of data fields queried exceeds the defined threshold. | SW_QUERY_MAX_QUERY_COMPLEXITY | 30 |
Review comment:
Let's make it 50 or 100. Today new dashboard could set up a complex page.
##########
File path: .github/workflows/skywalking.yaml
##########
@@ -18,17 +18,18 @@ name: CI
on:
pull_request:
- paths-ignore:
- - "*.md"
- - "*.txt"
- - "skywalking-ui"
- - ".asf.yaml"
- - ".dlc.yaml"
- - ".licenserc.yaml"
- - "docs/menu.yml"
- - ".github/workflows/codeql.yaml"
- - "dist-material/release-docs"
- - "*/component-libraries.yml"
+ paths:
Review comment:
What does this `paths-ignore` to `path` mean? The following patterns should be CI irrelative?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] wu-sheng commented on a change in pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
wu-sheng commented on a change in pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703#discussion_r829717067
##########
File path: .github/workflows/skywalking.yaml
##########
@@ -18,17 +18,18 @@ name: CI
on:
pull_request:
- paths-ignore:
- - "*.md"
- - "*.txt"
- - "skywalking-ui"
- - ".asf.yaml"
- - ".dlc.yaml"
- - ".licenserc.yaml"
- - "docs/menu.yml"
- - ".github/workflows/codeql.yaml"
- - "dist-material/release-docs"
- - "*/component-libraries.yml"
+ paths:
Review comment:
What does this `paths-ignore` to `path` mean? The following patterns should be CI irrelative?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [skywalking] kezhenxu94 merged pull request #8703: Add complexity limitation for GraphQL query to avoid malicious query
Posted by GitBox <gi...@apache.org>.
kezhenxu94 merged pull request #8703:
URL: https://github.com/apache/skywalking/pull/8703
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org