diff --git a/cli/contributing/security/runtimes/__main__.py b/cli/contributing/security/runtimes/__main__.py index f73d606881..7495c6b12d 100755 --- a/cli/contributing/security/runtimes/__main__.py +++ b/cli/contributing/security/runtimes/__main__.py @@ -51,7 +51,12 @@ def report(refs: list[ImageRef], probed: list, floors: dict[str, str]) -> int: if runtime is None: continue with_php += 1 - php = runtime.version or f"line {', '.join(runtime.lines)}" + if runtime.version: + php = runtime.version + elif runtime.lines: + php = f"line {', '.join(runtime.lines)}" + else: + php = f"package {', '.join(runtime.packages)}" finding = php_finding(runtime, floors) if finding: findings += 1 diff --git a/roles/sys-ctl-hlth-runtime/files/python/script.py b/roles/sys-ctl-hlth-runtime/files/python/script.py index c371209ea4..0e94150a13 100755 --- a/roles/sys-ctl-hlth-runtime/files/python/script.py +++ b/roles/sys-ctl-hlth-runtime/files/python/script.py @@ -83,6 +83,7 @@ def running_containers() -> list[Container]: capture_output=True, text=True, check=True, + timeout=EXEC_TIMEOUT_SECONDS, ).stdout.split() if not ids: return [] @@ -92,6 +93,7 @@ def running_containers() -> list[Container]: capture_output=True, text=True, check=True, + timeout=EXEC_TIMEOUT_SECONDS, ).stdout ) containers = [] @@ -253,7 +255,12 @@ def main(argv=None) -> int: args = parse_args(argv) php_floors = json.loads(args.floors)["php"] hardened = json.loads(args.hardened) - failures, infos = check(running_containers(), php_floors, hardened, probe_php) + try: + containers = running_containers() + except (subprocess.TimeoutExpired, subprocess.CalledProcessError) as exc: + print(f"❌ listing the running containers failed: {exc}") + return 1 + failures, infos = check(containers, php_floors, hardened, probe_php) for line in infos: print(f"ℹ️ {line}") for line in failures: diff --git a/tests/unit/python/cli/contributing/security/test_runtimes.py b/tests/unit/python/cli/contributing/security/test_runtimes.py index 5a7f913980..8df5ed2f83 100644 --- a/tests/unit/python/cli/contributing/security/test_runtimes.py +++ b/tests/unit/python/cli/contributing/security/test_runtimes.py @@ -58,6 +58,24 @@ class TestReport(unittest.TestCase): self.assertEqual(code, 0) self.assertIn("PHP line 8.4: patch level not in the registry metadata", output) + def test_an_unversioned_php_package_is_listed_with_unknown_patch_level(self): + code, output = self._report( + [_ref("web-app-lam", "lam")], [["RUN apt-get install -y php-cli"]] + ) + self.assertEqual(code, 0) + self.assertIn( + "PHP package php-cli: patch level not in the registry metadata", output + ) + self.assertIn("1 ship PHP", output) + + def test_an_eol_package_line_fails_next_to_a_patched_php_version(self): + code, output = self._report( + [_ref("web-app-lam", "lam")], + [["ENV PHP_VERSION=8.4.27", "RUN apk add php7 php7-fpm"]], + ) + self.assertEqual(code, 1) + self.assertIn("PHP 8.4.27: line 7 is end of life", output) + def test_an_eol_php_base_argument_fails_the_report(self): code, output = self._report( [_ref("web-app-fusiondirectory", "fusiondirectory")], diff --git a/tests/unit/python/roles/sys-ctl-hlth-runtime/files/test_script.py b/tests/unit/python/roles/sys-ctl-hlth-runtime/files/test_script.py index 72782a207f..e6adc4671c 100644 --- a/tests/unit/python/roles/sys-ctl-hlth-runtime/files/test_script.py +++ b/tests/unit/python/roles/sys-ctl-hlth-runtime/files/test_script.py @@ -223,6 +223,24 @@ class TestProbePhp(TestCase): SCRIPT.probe_php("abc") +class TestRunningContainers(TestCase): + def test_every_engine_call_is_bounded_by_a_timeout(self): + timeouts = [] + + def fake_run(args, **kwargs): + timeouts.append(kwargs.get("timeout")) + if args[:2] == ["container", "ps"]: + return SimpleNamespace(returncode=0, stdout="id-app\n", stderr="") + inspected = [{"Id": "id-app", "Name": "/app", "Config": {"Labels": None}}] + return SimpleNamespace( + returncode=0, stdout=json.dumps(inspected), stderr="" + ) + + with mock.patch.object(SCRIPT.subprocess, "run", side_effect=fake_run): + SCRIPT.running_containers() + self.assertEqual(timeouts, [SCRIPT.EXEC_TIMEOUT_SECONDS] * 2) + + class TestMain(TestCase): def _main(self, php_versions): """Run main() against a fake engine. @@ -278,6 +296,31 @@ class TestMain(TestCase): code, _output = self._main({"wordpress": "8.4.27", "postgres": None}) self.assertEqual(code, 0) + def _main_on_engine_error(self, error): + argv = ["--floors", json.dumps({"php": FLOORS}), "--hardened", "{}"] + with ( + mock.patch.object(SCRIPT.subprocess, "run", side_effect=error), + contextlib.redirect_stdout(io.StringIO()) as out, + ): + code = SCRIPT.main(argv) + return code, out.getvalue() + + def test_main_fails_with_one_line_on_a_hung_engine(self): + code, output = self._main_on_engine_error( + subprocess.TimeoutExpired(cmd=["container", "ps"], timeout=60) + ) + self.assertEqual(code, 1) + self.assertEqual(len(output.splitlines()), 1) + self.assertIn("timed out after 60 seconds", output) + + def test_main_fails_with_one_line_on_a_failing_engine(self): + code, output = self._main_on_engine_error( + subprocess.CalledProcessError(returncode=1, cmd=["container", "ps"]) + ) + self.assertEqual(code, 1) + self.assertEqual(len(output.splitlines()), 1) + self.assertIn("returned non-zero exit status 1", output) + if __name__ == "__main__": main() diff --git a/tests/unit/python/utils/docker/test_image_runtime.py b/tests/unit/python/utils/docker/test_image_runtime.py index 9b7c7e7489..150bb040d8 100644 --- a/tests/unit/python/utils/docker/test_image_runtime.py +++ b/tests/unit/python/utils/docker/test_image_runtime.py @@ -11,8 +11,10 @@ from utils.docker.image import runtime FLOORS = {"8.2": "8.2.35", "8.3": "8.3.36", "8.4": "8.4.27", "8.5": "8.5.12"} -def _php(version=None, lines=()): - return runtime.PhpRuntime(version=version, lines=tuple(lines)) +def _php(version=None, lines=(), packages=()): + return runtime.PhpRuntime( + version=version, lines=tuple(lines), packages=tuple(packages) + ) class TestPhpRuntime(TestCase): @@ -75,6 +77,35 @@ class TestPhpRuntime(TestCase): ) ) + def test_an_unversioned_debian_runtime_package_is_php(self): + self.assertEqual( + runtime.php_runtime(["RUN apt-get install -y php-cli"]), + _php(packages=["php-cli"]), + ) + + def test_unversioned_runtime_packages_are_php(self): + found = runtime.php_runtime( + [ + "RUN dnf install -y php php-fpm php-mysqlnd", + "RUN apt-get install -y libapache2-mod-php php-cgi=2:8.2+93 \\", + ] + ) + self.assertEqual( + found, _php(packages=["libapache2-mod-php", "php", "php-cgi", "php-fpm"]) + ) + + def test_php_libraries_extensions_and_paths_are_not_a_runtime(self): + self.assertIsNone( + runtime.php_runtime( + [ + ( + "RUN apt-get install -y php-common php-pear php-mysql phpmyadmin" + " && docker-php-ext-install gd && cp php-fpm.conf /etc/php-fpm.d/" + ) + ] + ) + ) + def test_an_image_without_php_has_no_runtime(self): self.assertIsNone( runtime.php_runtime( @@ -114,6 +145,21 @@ class TestPhpFinding(TestCase): def test_a_supported_distro_line_is_no_finding(self): self.assertIsNone(runtime.php_finding(_php(lines=["8.3"]), FLOORS)) + def test_an_eol_line_fails_next_to_a_patched_php_version(self): + self.assertEqual( + runtime.php_finding(_php("8.4.27", lines=["7"]), FLOORS), + "line 7 is end of life", + ) + + def test_a_php_version_below_its_floor_outranks_an_eol_line(self): + self.assertIn( + "below the security floor 8.4.27", + runtime.php_finding(_php("8.4.15", lines=["7"]), FLOORS), + ) + + def test_an_unversioned_package_alone_is_no_finding(self): + self.assertIsNone(runtime.php_finding(_php(packages=["php-cli"]), FLOORS)) + class TestLoadPhpFloors(TestCase): def test_reads_the_php_floors_from_the_vars_file(self): diff --git a/utils/docker/image/runtime.py b/utils/docker/image/runtime.py index 9b564fe51a..b582f7761e 100644 --- a/utils/docker/image/runtime.py +++ b/utils/docker/image/runtime.py @@ -26,6 +26,9 @@ PACKAGE_INSTALL_PATTERN = re.compile( PHP_PACKAGE_PATTERN = re.compile( r"(? dict[str, str]: @@ -139,11 +147,13 @@ def php_runtime(instructions: list[str]) -> PhpRuntime | None: instructions: see :func:`build_instructions`. Returns: - The last ``PHP_VERSION`` set and the lines of the distro PHP packages - and ``PHP_BASE`` arguments, or ``None`` when none of them appears. + The last ``PHP_VERSION`` set, the lines of the distro PHP packages + and ``PHP_BASE`` arguments, and the unversioned interpreter packages, + or ``None`` when none of them appears. """ version = None lines: set[str] = set() + packages: set[str] = set() for instruction in instructions: for match in PHP_VERSION_PATTERN.finditer(instruction): version = match.group(1) @@ -152,9 +162,14 @@ def php_runtime(instructions: list[str]) -> PhpRuntime | None: continue for major, minor in PHP_PACKAGE_PATTERN.findall(instruction): lines.add(f"{major}.{minor}" if minor else major) - if version is None and not lines: + packages.update(PHP_UNVERSIONED_PACKAGE_PATTERN.findall(instruction)) + if version is None and not lines and not packages: return None - return PhpRuntime(version=version, lines=tuple(sorted(lines))) + return PhpRuntime( + version=version, + lines=tuple(sorted(lines)), + packages=tuple(sorted(packages)), + ) def _numbers(text: str) -> tuple[int, ...]: @@ -169,8 +184,10 @@ def php_finding(runtime: PhpRuntime, floors: dict[str, str]) -> str | None: floors: see :func:`load_php_floors`. Returns: - ``None`` when nothing is below its floor, otherwise the reason. A - version found only as a line is judged by that line. + ``None`` when nothing is below its floor, otherwise the reason. The + lines are judged next to ``PHP_VERSION``, so an end-of-life package + line fails an image whose ``PHP_VERSION`` is patched. Unversioned + packages alone are no finding: their patch level is unknown. """ oldest = min(_numbers(line) for line in floors) newest = max(_numbers(line) for line in floors) @@ -180,13 +197,10 @@ def php_finding(runtime: PhpRuntime, floors: dict[str, str]) -> str | None: return f"unparsable PHP_VERSION {runtime.version!r}" current = tuple(int(part) for part in match.groups()) line = f"{current[0]}.{current[1]}" - if line in floors: - if current < _numbers(floors[line]): - return f"{runtime.version} is below the security floor {floors[line]}" - return None - if current[:2] > newest: - return None - return f"{runtime.version} runs the end-of-life line {line}" + if line in floors and current < _numbers(floors[line]): + return f"{runtime.version} is below the security floor {floors[line]}" + if line not in floors and current[:2] <= newest: + return f"{runtime.version} runs the end-of-life line {line}" eol = [line for line in runtime.lines if _numbers(line) < oldest] if eol: return f"line {', '.join(eol)} is end of life"