fix: Make subprocess shell usage explicit in fm11rf08s_recovery.py#3345
Conversation
The fm11rf08s_recovery
|
You are welcome to add an entry to the CHANGELOG.md as well |
|
@doegox this one is for you. I mean we do already allow for cmd prompts in the client with |
orbisai0security
changed the title
|
Thanks for taking a look. I re-reviewed the changed call sites and agree the original risk is lower than my PR description suggested: these calls already pass argv lists to subprocess.run(), and shell=False is the Python default. So this is better framed as an explicit hardening/clarity change rather than a confirmed high-severity command-injection fix. I’m happy to update the PR description accordingly, and I can also remove the regression test if it does not fit the project’s test structure. |
|
ok please rephrase and remove the test |
…ions The test targeted a non-existent function (recover_card vs recovery), used pytest while the project uses unittest, and was placed in a new top-level tests/ directory not wired into pm3_tests.sh. The shell=False change is hardening-only; no regression test is needed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
done. Pls review. |
|
merged, thank you |
3 participants
Hardening: explicitly set `shell=False` on all `subprocess.run()` calls in `fm11rf08s_recovery.py`.
All four call sites already pass `cmd` as a Python list, so Python's default of `shell=False` means there is no active command-injection risk. This change makes the security intent explicit, prevents accidental future regression if a call site is later changed to pass a string, and improves code clarity.
No functional behaviour is changed.