diff --git a/backend/tests/test_delivery.py b/backend/tests/test_delivery.py index 61310d7..187c6ef 100644 --- a/backend/tests/test_delivery.py +++ b/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) diff --git a/scripts/release/broker_health.py b/scripts/release/broker_health.py index becbcaf..d18393c 100644 --- a/scripts/release/broker_health.py +++ b/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)