pytest-pouch: replace shell=True subprocess calls with argv lists#170
pytest-pouch: replace shell=True subprocess calls with argv lists#170mniestroj wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens pytest-pouch’s credential-generation fixture by removing shell=True subprocess usage and passing explicit argv lists/paths to OpenSSL, reducing command-injection risk from interpolated device.name / project.id values.
Changes:
- Replaced OpenSSL shell command strings with argv lists passed to
subprocess.run(...). - Built certificate/key file paths using
Pathjoins and passed them as explicit OpenSSL arguments. - Updated certificate upload to read from the new
Pathvariable (ca_crt).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "-CA", | ||
| str(ca_crt), | ||
| "-CAkey", | ||
| str(ca_key), | ||
| "-CAcreateserial", | ||
| "-out", |
There was a problem hiding this comment.
openssl x509 -CAcreateserial will create the serial file (default ca.srl) in the process working directory unless -CAserial is provided. Since cwd=creds_dir was removed, this can leak ca.srl into whatever directory pytest is run from and can also break parallel runs. Pass an explicit -CAserial path under creds_dir (or restore cwd=creds_dir) so all generated artifacts stay contained.
| dev_key = creds_dir / f"{device.name}.key.pem" | ||
| dev_csr = creds_dir / f"{device.name}.csr.pem" | ||
| dev_crt = creds_dir / f"{device.name}.crt.pem" |
There was a problem hiding this comment.
device.name is used directly in credential filenames. If it contains path separators (e.g. /, \\) or .., Path joining can write outside creds_dir or create unexpected subdirectories, which is a security and correctness problem even without shell=True. Consider validating/normalizing device.name into a safe filename component (e.g., allowlist characters and reject/replace separators) before building these paths.
This eliminates potential command-injection risk from interpolated device.name and project.id values. File paths are built with Path joins and passed as explicit arguments instead of being embedded in shell command strings. Signed-off-by: Marcin Niestroj <marcin.niestroj@canonical.com>
mniestroj
force-pushed
the
pytest-pouch-use-subprocess-run-without-shell
branch
from
0e2f262 to
2566a4f
Compare
This eliminates potential command-injection risk from interpolated
device.name and project.id values. File paths are built with Path joins and
passed as explicit arguments instead of being embedded in shell command
strings.