-
Notifications
You must be signed in to change notification settings - Fork 65
apollo_network: broadcast network stress test draft #8303
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
base: 25-sep-29-bnst-metrics
Are you sure you want to change the base?
apollo_network: broadcast network stress test draft #8303
Conversation
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
| try: | ||
| result = subprocess.run( | ||
| cmd, | ||
| shell=True, |
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.
Semgrep identified an issue in your code:
Found 'subprocess' function 'run' with 'shell=True'. This is dangerous because this call will spawn the command using a shell process. Doing so propagates current shell settings and variables, which makes it much easier for a malicious actor to execute commands. Use 'shell=False' instead.
To resolve this comment:
✨ Commit Assistant fix suggestion
| shell=True, | |
| shell=False, |
View step-by-step instructions
- Change the value of the
shellparameter fromTruetoFalsein thesubprocess.runcall. - Change the
cmdargument from a single string to a list of command arguments. Break up the string at spaces and make each part an element of the list. For example,cmd = ["cargo", "run", "--bin", "get_peer_id_from_secret_key", secret_key]. - Pass this list directly to
subprocess.run()as the first argument.
This prevents the shell from interpreting the command and reduces the risk of shell injection attacks.
💬 Ignore this finding
Reply with Semgrep commands to ignore this finding.
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by subprocess-shell-true.
You can view more details about this finding in the Semgrep AppSec Platform.
|
Benchmark movements: No major performance changes detected. |
d04380c to
61e6485
Compare
b24e12f to
30f4a37
Compare
30f4a37 to
a604cea
Compare
61e6485 to
a52944b
Compare
a604cea to
f9b196a
Compare
a52944b to
8f65443
Compare
1dd11f1 to
5d2213f
Compare
8a0c365 to
ed57747
Compare
ed57747 to
5ee188a
Compare
5d2213f to
1677cf0
Compare
5ee188a to
a1645a3
Compare
a1645a3 to
d97f628
Compare
00dcd1d to
d6d6346
Compare
735f358 to
9024760
Compare
d6d6346 to
4f96406
Compare
4f96406 to
5827ce7
Compare
9024760 to
92c87f1
Compare

No description provided.