From bbd55907422b1ebf25e9f04616b394e8da24179b Mon Sep 17 00:00:00 2001 From: Paul Payne Date: Mon, 6 Jul 2026 16:57:29 -0700 Subject: [PATCH] fix(core): audit matches split *_HOST/*_PORT env pairs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The consumption audit only matched host:port inside a single value (a URL, DATABASE_URL), missing the very common split-var shape (X_HOST=localhost + X_PORT=1883, e.g. castle-api's mqtt config). Resolve those pairs too — factored into a shared _resolve helper. A bare *_PORT with no *_HOST is ignored so a deployment's own listen port isn't mistaken for a dependency. Adds test_audit.py. --- core/src/castle_core/audit.py | 79 ++++++++++++++++++++++------- core/tests/test_audit.py | 94 +++++++++++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 18 deletions(-) create mode 100644 core/tests/test_audit.py diff --git a/core/src/castle_core/audit.py b/core/src/castle_core/audit.py index bfd0c89..29e6dcb 100644 --- a/core/src/castle_core/audit.py +++ b/core/src/castle_core/audit.py @@ -35,12 +35,43 @@ class Suggestion: protocol: str # the provider's socket protocol +def _resolve( + host: str, + port: int, + *, + consumer: str, + port_provider: dict[int, tuple[str, str]], + declared: set[str], + proposed: set[str], + env_var: str, + out: list[Suggestion], +) -> None: + """Resolve a (host, port) to a provider and record a suggestion if it's a new, + undeclared, local match. Only resolve local hosts (this node's providers) or a + host that names the provider — avoids matching a coincidental external host that + happens to share a port number.""" + prov = port_provider.get(port) + if not prov: + return + pname, proto = prov + if host not in _LOCAL_HOSTS and host != pname: + return + if pname == consumer or pname in declared or pname in proposed: + return + proposed.add(pname) + out.append(Suggestion(consumer, pname, env_var, f"{host}:{port}", proto)) + + def suggest_consumption(config: CastleConfig) -> list[Suggestion]: """Undeclared consumption suggestions, derived from env endpoint values. - For each deployment, scan its ``defaults.env`` values for ``host:port`` (or a - URL). When the port resolves to a *local* provider's socket and the consumer - doesn't already declare it, propose the edge. Deduped per (consumer, provider).""" + Two shapes are recognized in a deployment's ``defaults.env``: + (a) ``host:port`` inside a single value (a URL, a ``DATABASE_URL``); and + (b) a split ``X_HOST`` + ``X_PORT`` pair (e.g. ``CASTLE_API_MQTT_HOST`` + + ``CASTLE_API_MQTT_PORT``). When the port resolves to a *local* provider's socket + and the consumer doesn't already declare it, propose the edge — deduped per + (consumer, provider). A bare ``*_PORT`` with no ``*_HOST`` is ignored: without an + explicit host it can't be told apart from the deployment's own listen port.""" model = build_model(config, check=False) # port -> (provider name, protocol); ports are unique per host, so a port match # against a local host is a confident resolution. @@ -54,21 +85,33 @@ def suggest_consumption(config: CastleConfig) -> list[Suggestion]: env = dict(dep.defaults.env) if (dep.defaults and dep.defaults.env) else {} declared = {r.ref for r in getattr(dep, "requires", [])} proposed: set[str] = set() + + def resolve(host: str, port: int, env_var: str) -> None: + _resolve( + host, + port, + consumer=name, + port_provider=port_provider, + declared=declared, + proposed=proposed, + env_var=env_var, + out=out, + ) + + # (a) host:port inside a single value. for var, val in env.items(): for m in _HOSTPORT.finditer(str(val)): - host = m.group("host") - port = int(m.group("port")) - prov = port_provider.get(port) - if not prov: - continue - pname, proto = prov - # Only resolve when the value points at a local host (this node's - # providers) or names the provider directly — avoids matching a - # coincidental external host that happens to share a port number. - if host not in _LOCAL_HOSTS and host != pname: - continue - if pname == name or pname in declared or pname in proposed: - continue - proposed.add(pname) - out.append(Suggestion(name, pname, var, f"{host}:{port}", proto)) + resolve(m.group("host"), int(m.group("port")), var) + # (b) split X_HOST + X_PORT pair. + for var, val in env.items(): + if not var.endswith("_HOST"): + continue + pvar = var[:-5] + "_PORT" # X_HOST -> X_PORT + if pvar not in env: + continue + try: + port = int(str(env[pvar]).strip()) + except ValueError: + continue + resolve(str(val).strip(), port, f"{var}+{pvar}") return out diff --git a/core/tests/test_audit.py b/core/tests/test_audit.py new file mode 100644 index 0000000..16b638b --- /dev/null +++ b/core/tests/test_audit.py @@ -0,0 +1,94 @@ +"""Tests for the consumption audit (core/src/castle_core/audit.py).""" + +from __future__ import annotations + +import castle_core.config as C +from castle_core import audit +from castle_core.manifest import SystemdDeployment + + +def _svc( + program: str, + *, + tcp: int | None = None, + http: int | None = None, + env: dict | None = None, + requires: list | None = None, +) -> SystemdDeployment: + spec: dict = { + "manager": "systemd", + "program": program, + "run": {"launcher": "command", "argv": [program]}, + } + if tcp is not None: + spec["expose"] = {"tcp": {"port": tcp}} + spec["reach"] = "internal" + elif http is not None: + spec["expose"] = {"http": {"internal": {"port": http}}} + spec["reach"] = "internal" + if env is not None: + spec["defaults"] = {"env": env} + if requires is not None: + spec["requires"] = requires + return SystemdDeployment.model_validate(spec) + + +def _cfg(deployments: dict) -> C.CastleConfig: + return C.CastleConfig( + root=None, + gateway=C.GatewayConfig(port=9000), + repo=None, + programs={}, + deployments=deployments, + ) + + +def _pairs(cfg: C.CastleConfig) -> set[tuple[str, str]]: + return {(s.consumer, s.provider) for s in audit.suggest_consumption(cfg)} + + +def test_split_host_port_pair_is_suggested() -> None: + """A split X_HOST + X_PORT pair resolves like a single host:port value.""" + cfg = _cfg( + { + "broker": _svc("broker", tcp=1883), + "api": _svc( + "api", + http=9020, + env={"CASTLE_API_MQTT_HOST": "localhost", "CASTLE_API_MQTT_PORT": "1883"}, + ), + } + ) + assert ("api", "broker") in _pairs(cfg) + + +def test_single_value_url_is_suggested() -> None: + cfg = _cfg( + { + "db": _svc("db", tcp=5432), + "app": _svc("app", http=9001, env={"DATABASE_URL": "postgresql://u@localhost:5432/x"}), + } + ) + assert ("app", "db") in _pairs(cfg) + + +def test_bare_port_without_host_is_not_matched() -> None: + """A deployment's own listen port (no host) must not become a dependency.""" + cfg = _cfg({"app": _svc("app", http=9001, env={"APP_PORT": "9001"})}) + assert _pairs(cfg) == set() + + +def test_declared_pair_is_not_suggested() -> None: + """Already-declared consumption is not re-suggested.""" + cfg = _cfg( + { + "broker": _svc("broker", tcp=1883), + "api": _svc( + "api", + http=9020, + env={"X_HOST": "localhost", "X_PORT": "1883"}, + requires=[{"ref": "broker"}], + ), + } + ) + assert ("api", "broker") not in _pairs(cfg)