chore(ci_visibility): support xdist via distributed event hub (POC) #13504
Conversation
|
|
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
| def remote_dispatch_with_results(*args): | ||
| print("ꙮ Remote dispatch_with_results: {args}", flush=True) | ||
| response = urlopen(f"http://{DD_CORE_HOST}:{DD_CORE_PORT}/dispatch_with_results", data=pickle.dumps(args)) | ||
| result = EventResultDict(pickle.loads(response.read())) |
There was a problem hiding this comment.
⚪ Code Vulnerability
pickle.loads is not safe for deserializing unstrusted data (...read more)
Do not deserialize untrusted data. Make sure you use alternatives to check that the data can be deserialized safely. There is no workaround around this: unless you really trust the data source, it's better to use another way to exchange data, such as an API or other protocols such as protobuf or thrift.
Learn More
| return | ||
| print("ꙮ Remote dispatch: {args}", flush=True) | ||
| response = urlopen(f"http://{DD_CORE_HOST}:{DD_CORE_PORT}/dispatch", data=pickle.dumps(args)) | ||
| result = pickle.loads(response.read()) |
There was a problem hiding this comment.
⚪ Code Vulnerability
pickle.loads is not safe for deserializing unstrusted data (...read more)
Do not deserialize untrusted data. Make sure you use alternatives to check that the data can be deserialized safely. There is no workaround around this: unless you really trust the data source, it's better to use another way to exchange data, such as an API or other protocols such as protobuf or thrift.
Learn More
| def do_POST(self): | ||
| length = int(self.headers.get('content-length')) | ||
| body = self.rfile.read(length) | ||
| args = pickle.loads(body) |
There was a problem hiding this comment.
⚪ Code Vulnerability
pickle.loads is not safe for deserializing unstrusted data (...read more)
Do not deserialize untrusted data. Make sure you use alternatives to check that the data can be deserialized safely. There is no workaround around this: unless you really trust the data source, it's better to use another way to exchange data, such as an API or other protocols such as protobuf or thrift.
Learn More
| elif self.path == "/dispatch_with_results": | ||
| method = core.dispatch_with_results | ||
| else: | ||
| raise Exception("unknown core method") |
There was a problem hiding this comment.
⚪ Code Quality Violation
Exception is too generic (...read more)
Do not raise Exception and BaseException. These are too generic. Having generic exceptions makes it difficult to differentiate errors in a program. Use a specific exception, for example, ValueError, or create your own instead of using generic ones.
Learn More
| body = pickle.dumps(result) | ||
|
|
||
| self.wfile.write(b"HTTP/1.0 200 OK\r\n") | ||
| self.wfile.write(b"content-length: " + (str(len(body)).encode('utf-8')) + b"\r\n\r\n") |
There was a problem hiding this comment.
🟠 Code Quality Violation
use f-string or .format to format strings (...read more)
Concatenation of multiple strings is not efficient and make the code hard to read and understand.
Instead of concatenating multiple strings, use an f-string or a format string.
Learn More
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| def do_POST(self): | ||
| length = int(self.headers.get('content-length')) | ||
| body = self.rfile.read(length) | ||
| args = pickle.loads(body) |
Check failure
Code scanning / CodeQL
Deserialization of user-controlled data Critical
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 19 days ago
To fix the issue, we should replace the use of pickle.loads with a safer alternative for deserialization. The recommended approach is to use json.loads, which does not allow arbitrary code execution during deserialization. This requires ensuring that the data being sent to the server is serialized as JSON instead of using pickle.dumps.
The following changes are needed:
- Replace
pickle.loadswithjson.loadson line 166. - Replace
pickle.dumpswithjson.dumpson line 180 and in theremote_dispatchandremote_dispatch_with_resultsfunctions. - Update the
EventResultDicthandling to ensure compatibility with JSON serialization/deserialization.
| @@ -10,3 +10,3 @@ | ||
| from typing import Tuple | ||
|
|
||
| import json | ||
| from ddtrace.settings._config import config | ||
| @@ -165,3 +165,3 @@ | ||
| body = self.rfile.read(length) | ||
| args = pickle.loads(body) | ||
| args = json.loads(body) | ||
|
|
||
| @@ -179,3 +179,3 @@ | ||
| #print("RESULT:", result, flush=True) | ||
| body = pickle.dumps(result) | ||
| body = json.dumps(result).encode('utf-8') | ||
|
|
||
| @@ -211,4 +211,4 @@ | ||
| print("ꙮ Remote dispatch: {args}", flush=True) | ||
| response = urlopen(f"http://{DD_CORE_HOST}:{DD_CORE_PORT}/dispatch", data=pickle.dumps(args)) | ||
| result = pickle.loads(response.read()) | ||
| response = urlopen(f"http://{DD_CORE_HOST}:{DD_CORE_PORT}/dispatch", data=json.dumps(args).encode('utf-8')) | ||
| result = json.loads(response.read().decode('utf-8')) | ||
| print("ꙮ Remote dispatch result: {result}") | ||
| @@ -218,4 +218,4 @@ | ||
| print("ꙮ Remote dispatch_with_results: {args}", flush=True) | ||
| response = urlopen(f"http://{DD_CORE_HOST}:{DD_CORE_PORT}/dispatch_with_results", data=pickle.dumps(args)) | ||
| result = EventResultDict(pickle.loads(response.read())) | ||
| response = urlopen(f"http://{DD_CORE_HOST}:{DD_CORE_PORT}/dispatch_with_results", data=json.dumps(args).encode('utf-8')) | ||
| result = EventResultDict(json.loads(response.read().decode('utf-8'))) | ||
| print("ꙮ Remote dispatch_with_results result: {result}") |
Bootstrap import analysisComparison of import times between this PR and base. SummaryThe average import time from this PR is: 238 ± 2 ms. The average import time from base is: 234 ± 2 ms. The import time difference between this PR and base is: 4.06 ± 0.08 ms. Import time breakdownThe following import paths have appeared:
|
Checklist
Reviewer Checklist