diff --git a/tests/test_detector_tools_no_shell.py b/tests/test_detector_tools_no_shell.py new file mode 100644 index 000000000..26006e478 --- /dev/null +++ b/tests/test_detector_tools_no_shell.py @@ -0,0 +1,38 @@ +# SPDX-License-Identifier: MIT +import ast +from pathlib import Path + + +REPO_ROOT = Path(__file__).resolve().parents[1] +DETECTOR_PATHS = [ + REPO_ROOT / "tools" / "gpu_display_detector.py", + REPO_ROOT / "tools" / "os_detector.py", +] + + +def _shell_true_locations(path): + tree = ast.parse(path.read_text(encoding="utf-8")) + locations = [] + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + for keyword in node.keywords: + if ( + keyword.arg == "shell" + and isinstance(keyword.value, ast.Constant) + and keyword.value.value is True + ): + locations.append(node.lineno) + return locations + + +def test_detector_tools_do_not_use_shell_true(): + offenders = { + path.relative_to(REPO_ROOT).as_posix(): _shell_true_locations(path) + for path in DETECTOR_PATHS + } + + assert offenders == { + "tools/gpu_display_detector.py": [], + "tools/os_detector.py": [], + } diff --git a/tools/gpu_display_detector.py b/tools/gpu_display_detector.py index 1fb411437..b1d4622fd 100644 --- a/tools/gpu_display_detector.py +++ b/tools/gpu_display_detector.py @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: MIT import json import platform import subprocess @@ -7,7 +8,7 @@ def detect_gpu_and_display(): badges = [] try: - output = subprocess.check_output("lspci", shell=True).decode().lower() + output = subprocess.check_output(["lspci"], stderr=subprocess.DEVNULL).decode().lower() except Exception: output = "" diff --git a/tools/os_detector.py b/tools/os_detector.py index d1da0286b..a2362387f 100644 --- a/tools/os_detector.py +++ b/tools/os_detector.py @@ -1,5 +1,6 @@ +# SPDX-License-Identifier: MIT +import os import platform -import subprocess import json from datetime import datetime @@ -45,11 +46,11 @@ def detect_legacy_os_badges(): detected_keywords = [] try: - output = subprocess.check_output("dir", shell=True).decode().lower() + output = "\n".join(os.listdir(".")).lower() for system_key, terms in simulated_os_data.items(): if any(term.lower() in output for term in terms): detected_keywords.append(system_key) - except: + except OSError: pass for key in detected_keywords: diff --git a/tools/test_os_detector.py b/tools/test_os_detector.py index 4c8246152..ffe82dbfb 100644 --- a/tools/test_os_detector.py +++ b/tools/test_os_detector.py @@ -1,3 +1,4 @@ +# SPDX-License-Identifier: MIT import importlib.util from pathlib import Path from unittest.mock import patch @@ -21,12 +22,12 @@ def isoformat(): def test_detect_legacy_os_badges_detects_multiple_matching_environments(): - directory_listing = "System Folder\nFinder\nwin.ini\nprogman.exe\n" + directory_listing = ["System Folder", "Finder", "win.ini", "progman.exe"] with patch.object( - os_detector.subprocess, - "check_output", - return_value=directory_listing.encode(), + os_detector.os, + "listdir", + return_value=directory_listing, ), patch.object(os_detector, "datetime", FixedDateTime): result = os_detector.detect_legacy_os_badges() @@ -43,8 +44,8 @@ def test_detect_legacy_os_badges_detects_multiple_matching_environments(): def test_detect_legacy_os_badges_returns_empty_list_when_directory_probe_fails(): with patch.object( - os_detector.subprocess, - "check_output", + os_detector.os, + "listdir", side_effect=OSError("dir unavailable"), ): result = os_detector.detect_legacy_os_badges()