fix: identify failed broker metadata queries

backend/tests/test_delivery.py

@@ -2231,3 +2231,45 @@ def test_proxy_render_is_fixed_to_a_selected_snapshot_without_business_credentia
     assert "User=kf-proxy" in text and "CapabilityBoundingSet=CAP_NET_BIND_SERVICE" in text
     assert "PrivatePIDs=yes" in text and "LogNamespace=kf-onlinejudge" in text
     assert "EnvironmentFile=" not in text and "LoadCredential=" not in text
+
+
+@pytest.mark.parametrize("name", ["queues", "vhost_limits", "users", "permissions"])
+@pytest.mark.parametrize("failure", ["deadline", "exit", "output"])
+def test_broker_query_failure_identifies_fixed_operation_without_leaking_process_data(
+    monkeypatch, name, failure
+):
+    import subprocess
+    from types import SimpleNamespace
+
+    from scripts.release import broker_health
+
+    secret = "synthetic-private-process-detail"
+
+    def failed(command, *, stdout, stderr, timeout, env):
+        assert stderr == subprocess.DEVNULL
+        assert timeout == 8
+        assert command[-1] == "--formatter=json"
+        if failure == "deadline":
+            raise subprocess.TimeoutExpired([secret], timeout, stderr=secret.encode())
+        if failure == "output":
+            stdout.write(b"x" * 32769)
+        return SimpleNamespace(returncode=1 if failure == "exit" else 0)
+
+    monkeypatch.setattr(broker_health.subprocess, "run", failed)
+    suffix = {"deadline": "exceeded deadline", "exit": "failed", "output": "output exceeds bound"}
+    with pytest.raises(policy.DeliveryError) as raised:
+        broker_health.command(name)
+    assert str(raised.value) == "broker " + name + " query " + suffix[failure]
+    assert secret not in str(raised.value)
+
+
+@pytest.mark.parametrize("name", ["delete_queue", "users --delete", None])
+def test_broker_query_selector_never_accepts_extra_commands_or_flags(monkeypatch, name):
+    from scripts.release import broker_health
+
+    def unexpected(*args, **kwargs):
+        pytest.fail("unknown selector launched a process")
+
+    monkeypatch.setattr(broker_health.subprocess, "run", unexpected)
+    with pytest.raises(policy.DeliveryError, match="unknown broker metadata query"):
+        broker_health.command(name)

scripts/release/broker_health.py

@@ -11,6 +11,30 @@ from . import dependencies, operator_units, preflight
 from .policy import DeliveryError, canonical, fields, integer
 
 CTL = Path("/usr/lib/rabbitmq/sbin/rabbitmqctl")
+QUERIES = {
+    "queues": [
+        "list_queues",
+        "-p",
+        "kf",
+        "name",
+        "durable",
+        "auto_delete",
+        "exclusive",
+        "type",
+        "state",
+        "arguments",
+        "messages_ready",
+        "messages_unacknowledged",
+        "message_bytes",
+        "consumers",
+        "policy",
+        "operator_policy",
+        "effective_policy_definition",
+    ],
+    "vhost_limits": ["list_vhost_limits", "--global"],
+    "users": ["list_users"],
+    "permissions": ["list_permissions", "-p", "kf"],
+}
 
 
 def queue_arguments(value) -> dict:
@@ -123,7 +147,9 @@ def validate_snapshot(value: dict) -> dict:
     }
 
 
-def command(arguments: list[str]):
+def command(name: str):
+    if name not in QUERIES:
+        raise DeliveryError("unknown broker metadata query")
     # A file-backed capped stream prevents malicious metadata/log output from
     # allocating an unbounded capture buffer. The unit adds a hard file limit.
     env = {
@@ -137,24 +163,29 @@ def command(arguments: list[str]):
         ),
     }
     with tempfile.TemporaryFile() as output:
-        result = subprocess.run(
-            [
-                str(CTL),
-                "-q",
-                "-n",
-                "kf@localhost",
-                "--timeout",
-                "5",
-                *arguments,
-                "--formatter=json",
-            ],
-            stdout=output,
-            stderr=subprocess.DEVNULL,
-            timeout=8,
-            env=env,
-        )
-        if result.returncode or output.tell() > 32768:
-            raise DeliveryError("bounded broker metadata query unavailable")
+        try:
+            result = subprocess.run(
+                [
+                    str(CTL),
+                    "-q",
+                    "-n",
+                    "kf@localhost",
+                    "--timeout",
+                    "5",
+                    *QUERIES[name],
+                    "--formatter=json",
+                ],
+                stdout=output,
+                stderr=subprocess.DEVNULL,
+                timeout=8,
+                env=env,
+            )
+        except subprocess.TimeoutExpired as error:
+            raise DeliveryError("broker " + name + " query exceeded deadline") from error
+        if result.returncode:
+            raise DeliveryError("broker " + name + " query failed")
+        if output.tell() > 32768:
+            raise DeliveryError("broker " + name + " query output exceeds bound")
         output.seek(0)
         return operator_units.decode(output.read(32769))
 
@@ -170,32 +201,7 @@ def observe() -> dict:
         or (info.st_uid, info.st_gid, stat.S_IMODE(info.st_mode)) != (1204, 1204, 0o400)
     ):
         raise DeliveryError("ordinary private broker cookie required")
-    data = {
-        "queues": command(
-            [
-                "list_queues",
-                "-p",
-                "kf",
-                "name",
-                "durable",
-                "auto_delete",
-                "exclusive",
-                "type",
-                "state",
-                "arguments",
-                "messages_ready",
-                "messages_unacknowledged",
-                "message_bytes",
-                "consumers",
-                "policy",
-                "operator_policy",
-                "effective_policy_definition",
-            ]
-        ),
-        "vhost_limits": command(["list_vhost_limits", "--global"]),
-        "users": command(["list_users"]),
-        "permissions": command(["list_permissions", "-p", "kf"]),
-    }
+    data = {name: command(name) for name in QUERIES}
     return validate_snapshot(data)