Skip to content

Quality: Insecure subprocess usage in get_system_info.py #1977

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.

Sign up for GitHub

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

Open
tomaioo wants to merge 3 commits into
base: main
Choose a base branch
from
+12 −14
Open

Quality: Insecure subprocess usage in get_system_info.py #1977

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b932d60
refactor: insecure subprocess usage in get_system_info.py
tomaioo Jul 15, 2026
6451f58
fix: address review feedback — can you address this as well?
tomaioo Jul 15, 2026
1c00576
fix: address review feedback — can you address this as well?
tomaioo Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 12 additions & 14 deletions .github/ISSUE_TEMPLATE/get_system_info.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ def get_nvidia_gpu_info():
try:
nvidia_smi = (
subprocess.check_output(
"nvidia-smi --query-gpu=name,memory.total,count --format=csv,noheader,nounits",
shell=True,
["nvidia-smi", "--query-gpu=name,memory.total,count", "--format=csv,noheader,nounits"],
)
.decode("utf-8")
.strip()
Expand All @@ -39,18 +38,20 @@ def get_nvidia_gpu_info():
gpu_count = len(nvidia_smi)
return gpu_name, f"{gpu_memory} GB", gpu_count
except Exception:
return "?", "?", "?"
pass
return "?", "?", "?"


def get_cuda_version():
"""Get CUDA Version."""
try:
nvcc_output = subprocess.check_output("nvcc --version", shell=True).decode("utf-8")
nvcc_output = subprocess.check_output(["nvcc", "--version"]).decode("utf-8")
match = re.search(r"release (\d+\.\d+)", nvcc_output)
if match:
return match.group(1)
except Exception:
return "?"
pass
return "?"


def get_package_version(package):
Expand All @@ -65,14 +66,11 @@ def get_package_version(package):
os_info = f"{platform.system()} {platform.release()}"
if platform.system() == "Linux":
with contextlib.suppress(Exception):
os_info = (
subprocess.check_output(
"cat /etc/os-release | grep PRETTY_NAME | cut -d= -f2", shell=True
)
.decode("utf-8")
.strip()
.strip('"')
)
with open("/etc/os-release", "r") as f:
for line in f:
if line.startswith("PRETTY_NAME="):
os_info = line.split("=", 1)[1].strip().strip('"')
break
Comment on lines +69 to +73

@coderabbitai coderabbitai Bot Jul 15, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,140p' .github/ISSUE_TEMPLATE/get_system_info.py

Repository: NVIDIA/Model-Optimizer

Length of output: 3724

🌐 Web query:

os-release PRETTY_NAME single quotes double quotes systemd man page

💡 Result:

According to the os-release manual page, variable assignment values must be enclosed in either double or single quotes if they contain spaces, semicolons, or any special characters outside of the range A–Z, a–z, and 0–9 [1][2]. If the value does not include these special characters, the use of quotes is optional but still permitted [1][3]. When using quotes, shell special characters (such as $, quotes, backslashes, or backticks) must be escaped with backslashes following standard shell style [1][2]. Concatenation of multiple individually quoted strings is not supported [1][2]. Because PRETTY_NAME typically includes spaces (e.g., "Fedora 32 (Workstation Edition)"), it is essentially always quoted in practice [1][4]. The specification ensures the file remains compatible with Bourne shell scripts by using environment-like variable assignments, although full shell evaluation is not supported (e.g., variable expansion is explicitly disallowed) [1][5].

Citations:

Parse PRETTY_NAME using os-release quoting rules.

/etc/os-release allows single- or double-quoted values, but this only strips double quotes. A single-quoted PRETTY_NAME will show up in the issue output with literal quotes. Use an os-release-aware parser instead of trimming one quote character.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/ISSUE_TEMPLATE/get_system_info.py around lines 67 - 71, Update the
PRETTY_NAME parsing in the system information collection flow to use an
os-release-aware parser that correctly handles both single- and double-quoted
values, rather than manually stripping only double quotes. Preserve the existing
behavior of selecting PRETTY_NAME and stopping after it is found.

Source: MCP tools

@kevalmorabia97 kevalmorabia97 Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you address this as well?

@coderabbitai coderabbitai Bot Jul 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevalmorabia97 Sure, here's the fix to handle both single- and double-quoted PRETTY_NAME values per the os-release spec:

  • Update the PRETTY_NAME parsing logic in .github/ISSUE_TEMPLATE/get_system_info.py to strip matching single or double quotes instead of only double quotes.

</plan_code_changes]

Failed to handle agent chat message. Please try again.

elif platform.system() == "Windows":
print("Please add the `windows` label to the issue.")

Expand All @@ -98,4 +96,4 @@ def get_package_version(package):
print(" - ONNXRuntime: " + get_package_version("onnxruntime"))
print(" - TensorRT: " + get_package_version("tensorrt"))
print("- Any other details that may help: " + "?")
print("=" * 70)
print("=" * 70)