fix(sys-ctl-hlth-runtime): bound the engine calls and catch hidden PHP runtimes

Three CodeRabbit findings on PR #1265:

- script.py listed the running containers with `container ps` and
  `container inspect` without a timeout, so a hung engine kept the daily
  unit active forever and blocked the next timer run. Both calls now use
  EXEC_TIMEOUT_SECONDS; main turns a timeout or engine error into a
  one-line failure and exit 1 instead of a traceback.

- runtime.py's PHP_PACKAGE_PATTERN requires a digit after "php", so an
  image installing an unversioned interpreter package (`php-cli`) and
  setting no PHP_VERSION was reported as PHP-free. The interpreter
  packages php, php-cli, php-fpm, php-cgi, php-phpdbg and
  libapache2-mod-php are now recorded as PhpRuntime.packages and reported
  with an unknown patch level; extension and library packages
  (php-mysql, php-pear, php-common, phpmyadmin) and paths do not count.

- php_finding returned early for a patched PHP_VERSION, so an end-of-life
  package line installed next to it (e.g. php7) was never reported. The
  version only returns a finding now; otherwise the lines are judged too.

Every finding has unit tests that fail on the previous code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

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

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:

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")],

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()

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):

utils/docker/image/runtime.py

@@ -26,6 +26,9 @@ PACKAGE_INSTALL_PATTERN = re.compile(
 PHP_PACKAGE_PATTERN = re.compile(
     r"(?<!\S)(?:[\w.+]+-)*php(\d)(?:\.?(\d))?(?:-[\w.+-]+)?(?:=\S*?)?(?=[;&|\\]*(?:\s|$))"
 )
+PHP_UNVERSIONED_PACKAGE_PATTERN = re.compile(
+    r"(?<!\S)(php|php-(?:cli|fpm|cgi|phpdbg)|libapache2-mod-php)(?:=\S*?)?(?=[;&|\\]*(?:\s|$))"
+)
 VERSION_PATTERN = re.compile(r"^(\d+)\.(\d+)\.(\d+)")
 
 
@@ -37,10 +40,15 @@ class PhpRuntime:
         version: ``PHP_VERSION`` the image sets, ``None`` when it sets none.
         lines: ``major[.minor]`` lines named by distro PHP packages or a
             ``PHP_BASE`` build argument.
+        packages: unversioned distro packages that are the interpreter itself
+            (``php``, ``php-cli``, ``php-fpm``, ``php-cgi``, ``php-phpdbg``,
+            ``libapache2-mod-php``); they name no line. Extension and library
+            packages such as ``php-mysql`` or ``php-pear`` do not count.
     """
 
     version: str | None
     lines: tuple[str, ...]
+    packages: tuple[str, ...]
 
 
 def load_php_floors() -> 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"