diff --git a/.github/groom/README.md b/.github/groom/README.md index 22de4ee..4ddff2b 100644 --- a/.github/groom/README.md +++ b/.github/groom/README.md @@ -166,8 +166,58 @@ Single-signature probe (exit 0 = should file, 1 = suppressed): python3 .github/groom/ledger.py --repo owner/name --check "" ``` +## `key-broker.mjs` — the localhost API-key proxy (BE-4419) + +A tiny, dependency-free localhost HTTP proxy that **holds the real Anthropic API +key and injects it into forwarded requests**, so the groom agent steps (BE-4311) +can run the Claude CLI with only a **dummy** key and `ANTHROPIC_BASE_URL` pointed +at the broker — the real key never enters the agent step's environment. + +**Why not just put the key in the agent step's env?** The groom agent has an +unscoped `Read` tool and `Bash(cat:*)`, and everything on the runner is the same +user, so `/proc//environ` — and, crucially, `/proc//environ` +and `/proc//cmdline` — are all agent-readable. The broker's design +follows from that: + +1. **The real key arrives on stdin (first line), never via env or argv.** A key in + the process environment or command line would be recoverable straight out of + `/proc`. Stdin is not. The broker reads exactly the first line at startup and + exits non-zero if stdin closes without one. +2. **It never logs request/response headers or bodies** — the log file is + agent-readable too. At most it logs `method path -> status`. +3. **It listens on `127.0.0.1` only.** + +What it does per request: + +- `HEAD` / `GET` on `/` → answered locally with `200` (the CLI's connectivity + probe; never forwarded). +- Path starting with `/v1/` → forwarded to the upstream preserving method and + body (streamed both ways, so SSE works), **stripping** any inbound `x-api-key`, + `authorization`, and `host`, then **injecting** the real `x-api-key` and the + upstream `host`. Upstream status + response headers are copied back verbatim. +- Anything else → `404` locally. Upstream connection error → `502` with a static + body (no detail echoed). + +Env knobs (config only — **never** the key): + +| Env var | Default | Meaning | +|---|---|---| +| `GROOM_BROKER_PORT` | `8199` | port to listen on (`127.0.0.1:`) | +| `GROOM_BROKER_UPSTREAM` | `https://api.anthropic.com` | where `/v1/*` is forwarded (overridable so tests can point it at a local fake; `http://` is accepted **only** for loopback hosts, so a plaintext non-loopback upstream can't leak the injected key) | + +On listen it prints one readiness line (`groom-key-broker listening on +127.0.0.1:`); a consumer's wait-loop should key off **that line** (proof +the broker itself bound the port), not merely the port being connectable — a +foreign process already holding the port would pass a bare connect check while +the broker exits with `EADDRINUSE`, and the consumer would then stream prompts +and repo data (plus the dummy key) to an unrelated listener. + +> **groom.yml wiring lands in the sibling ticket (BE-4311)** — this file adds the +> broker script + its unit tests only; nothing in `groom.yml` calls it yet. + - **`tests/`** — `unittest` suite, run by - [`test-groom-scripts.yml`](../workflows/test-groom-scripts.yml). + [`test-groom-scripts.yml`](../workflows/test-groom-scripts.yml). The key-broker + tests boot the real script under `node` (skipped when `node` is absent). ```bash python3 -m unittest discover -s .github/groom/tests -p 'test_*.py' -v diff --git a/.github/groom/key-broker.mjs b/.github/groom/key-broker.mjs new file mode 100644 index 0000000..4ca73cc --- /dev/null +++ b/.github/groom/key-broker.mjs @@ -0,0 +1,214 @@ +#!/usr/bin/env node +// groom key-broker (BE-4419) — a tiny localhost HTTP proxy that HOLDS the real +// API key and injects it into forwarded requests, so the groom agent steps +// (BE-4311) can run the Claude CLI with only a DUMMY key and the CLI's base-URL +// override pointed at this broker (see .github/groom/README.md for the env +// wiring the caller sets). +// +// Why a broker at all: the groom agent has an unscoped Read tool and +// `Bash(cat:*)`, and everything on the runner is the same user — so +// `/proc//environ` and `/proc//cmdline` are +// agent-readable. Therefore the real key MUST arrive on **stdin** (never env, +// never argv), this process must never log request/response headers or bodies +// (the log file is agent-readable too — we log at most `method path -> status`), +// and it listens on 127.0.0.1 only. This file deliberately never names or reads +// the provider key's env var (a grep for the provider name over this file finds +// nothing) — the only env it reads is GROOM_BROKER_PORT / GROOM_BROKER_UPSTREAM. + +import http from "node:http"; +import https from "node:https"; +import readline from "node:readline"; +import { pipeline } from "node:stream"; + +const DEFAULT_PORT = 8199; +const DEFAULT_UPSTREAM = "https://api.anthropic.com"; +// Socket-inactivity timeout for a forwarded upstream request: a stalled or dead +// upstream is torn down instead of pinning the forwarded request (and the real +// key it carries) plus its downstream socket open forever, which would let +// repeated stalls exhaust the broker's sockets. This is an INACTIVITY timeout — +// it does not fire while data (e.g. an SSE token stream) keeps flowing — so it +// is set generously to never clip a legitimately slow/long streaming reply. +const UPSTREAM_TIMEOUT_MS = 600_000; + +function die(message) { + console.error(`groom-key-broker: ${message}`); + process.exit(1); +} + +// --- config (env only — NOT the key) ------------------------------------- +const rawPort = process.env.GROOM_BROKER_PORT || String(DEFAULT_PORT); +// Reject the whole string, not just parseInt's leading digits — otherwise +// "8199x" (→ 8199) or "1e3" (→ 1) would be silently coerced to a valid port. +const PORT = /^\d+$/.test(rawPort) ? Number.parseInt(rawPort, 10) : NaN; +if (!Number.isInteger(PORT) || PORT < 1 || PORT > 65535) { + die(`invalid GROOM_BROKER_PORT: ${JSON.stringify(rawPort)}`); +} + +function isLoopbackHost(host) { + const h = host.replace(/^\[|\]$/g, ""); // strip IPv6 brackets, e.g. [::1] + return h === "localhost" || h === "::1" || /^127\./.test(h); +} + +let upstream; +try { + upstream = new URL(process.env.GROOM_BROKER_UPSTREAM || DEFAULT_UPSTREAM); +} catch { + die(`invalid GROOM_BROKER_UPSTREAM: ${JSON.stringify(process.env.GROOM_BROKER_UPSTREAM)}`); +} +if (upstream.protocol !== "http:" && upstream.protocol !== "https:") { + die(`GROOM_BROKER_UPSTREAM must be http/https, got ${upstream.protocol}`); +} +// Cleartext http:// is allowed ONLY for loopback (the fake upstream used by the +// tests). Injecting the real key into a plaintext request to a remote host would +// transmit the secret in the clear — defeating the broker's whole purpose. +if (upstream.protocol === "http:" && !isLoopbackHost(upstream.hostname)) { + die(`GROOM_BROKER_UPSTREAM http:// is allowed only for loopback hosts (got ${upstream.hostname}); use https:// for remote upstreams`); +} +const client = upstream.protocol === "http:" ? http : https; +// Prefix from the upstream URL (e.g. https://host/prefix → "/prefix"), stripped +// of a trailing slash; "" for a root upstream. Prepended to the forwarded path +// so a non-root upstream isn't silently dropped. +const upstreamPrefix = upstream.pathname.replace(/\/+$/, ""); + +// --- the real key: first line of stdin, nothing else --------------------- +async function readKeyFromStdin() { + const rl = readline.createInterface({ input: process.stdin }); + for await (const line of rl) { + rl.close(); + return line; + } + return null; // stdin closed without a single line +} + +const firstLine = await readKeyFromStdin(); +// readline strips the trailing newline; trim() drops a lone CR from CRLF input +// plus any leading/trailing whitespace common in copy-pasted secrets, which +// would otherwise be an illegal header character that throws on client.request. +const realKey = firstLine == null ? "" : firstLine.trim(); +// We no longer need stdin — release the pipe so nothing lingers holding the key. +process.stdin.destroy(); +if (!realKey) { + die("no API key on stdin (first line was empty or stdin closed before a line); refusing to start"); +} + +// --- request logging: method + path + status ONLY, never headers/body ---- +function logReq(method, path, status) { + process.stderr.write(`${method} ${path} -> ${status}\n`); +} + +// --- forward a /v1/* request to the upstream, injecting the real key ----- +function forward(req, res, method, path) { + const headers = { ...req.headers }; + // Strip any inbound credentials the CLI sent (the dummy x-api-key, and an + // authorization: Bearer if an auth-token env was set) and the inbound host. + delete headers["x-api-key"]; + delete headers["authorization"]; + delete headers["host"]; + headers["x-api-key"] = realKey; + headers["host"] = upstream.host; + + // `settled` guards the 502 path: once the upstream's headers are on their way + // back (or we've already emitted a 502), a later error must NOT try to write a + // second response. + let settled = false; + const fail502 = () => { + if (settled) return; + settled = true; + if (!res.headersSent) { + res.writeHead(502, { "content-type": "text/plain" }); + res.end("upstream unavailable"); + } else { + // Headers already flushed — destroy so the caller sees a broken + // connection, not a silently truncated "success". + res.destroy(); + } + logReq(method, path, 502); + }; + + let upstreamReq; + try { + upstreamReq = client.request({ + protocol: upstream.protocol, + hostname: upstream.hostname, + port: upstream.port || undefined, + method, + // Prepend any upstream path prefix; req.url is already validated /v1/*. + path: upstreamPrefix + req.url, + headers, + timeout: UPSTREAM_TIMEOUT_MS, + }); + } catch { + // client.request can throw synchronously (an illegal char in the request + // target or an injected key). Fail this one request; never crash the broker. + fail502(); + req.resume(); // drain the inbound body so its socket can close cleanly + return; + } + + upstreamReq.on("response", (upstreamRes) => { + // Past the 502 window: copy upstream status + headers back verbatim, then + // stream the body. pipeline tears down BOTH sides on any error/disconnect, + // so an upstream reset mid-stream destroys `res` (the caller sees a broken + // connection, not a truncated 200) and a client disconnect destroys the + // upstream response. + settled = true; + res.writeHead(upstreamRes.statusCode || 502, upstreamRes.headers); + logReq(method, path, upstreamRes.statusCode || 502); + pipeline(upstreamRes, res, () => {}); + }); + + // A stalled upstream trips the inactivity timeout; destroy() surfaces as an + // 'error' handled below (local 502 if nothing sent yet, else a torn stream). + upstreamReq.on("timeout", () => upstreamReq.destroy(new Error("upstream timeout"))); + // Connection-level errors (refused/reset/DNS) before any response → local 502. + upstreamReq.on("error", fail502); + + // Client disconnect (or normal completion) → tear down the outbound + // real-key-bearing request so it can't linger and leak its socket. + res.on("close", () => { + if (!upstreamReq.destroyed) upstreamReq.destroy(); + }); + + // Stream the inbound body up to the upstream (so large / streaming bodies + // work). pipeline aborts the upstream on a client abort rather than letting an + // unhandled 'error' on `req` crash the broker. + pipeline(req, upstreamReq, (err) => { + if (err && !upstreamReq.destroyed) upstreamReq.destroy(); + }); +} + +const server = http.createServer((req, res) => { + const method = req.method || "GET"; + const rawUrl = req.url || "/"; + const path = rawUrl.split("?", 1)[0]; + + // The CLI's connectivity probe — answer locally, never forward. + if ((method === "HEAD" || method === "GET") && path === "/") { + res.writeHead(200); + res.end(); + logReq(method, path, 200); + return; + } + + if (path.startsWith("/v1/")) { + forward(req, res, method, path); + return; + } + + // Anything else is not part of the CLI's surface — refuse locally. + res.writeHead(404); + res.end(); + logReq(method, path, 404); +}); + +server.on("error", (err) => { + die(`failed to listen on 127.0.0.1:${PORT}: ${err.message}`); +}); + +server.listen(PORT, "127.0.0.1", () => { + // Exactly one readiness line. A consumer wait-loop should match THIS line + // (proof the broker itself bound the port) rather than a bare port-connect + // check, which a foreign process already holding the port would pass while the + // broker exits with EADDRINUSE. + console.log(`groom-key-broker listening on 127.0.0.1:${PORT}`); +}); diff --git a/.github/groom/tests/test_key_broker.py b/.github/groom/tests/test_key_broker.py new file mode 100644 index 0000000..e691fef --- /dev/null +++ b/.github/groom/tests/test_key_broker.py @@ -0,0 +1,374 @@ +#!/usr/bin/env python3 +"""Tests for the groom key-broker (BE-4419). + +`key-broker.mjs` is a localhost HTTP proxy that HOLDS the real API key and +injects it into forwarded requests, so the groom agent steps (BE-4311) can run +the Claude CLI with only a dummy key + a base-URL override pointed at the broker. +The properties under test are the ones the design leans on: + + * inbound credentials (a dummy `x-api-key`, an `authorization: Bearer`) are + stripped and the REAL key is injected before forwarding to the upstream; + * request bodies reach the upstream byte-for-byte (streaming, so SSE works); + * the connectivity probe (`HEAD /`) and non-`/v1/` paths are answered locally + and never forwarded; + * the real key is NOT visible in the broker process's env or argv + (`/proc//{environ,cmdline}` are agent-readable on the shared runner); + * an upstream connection error yields a local 502 and the broker stays alive. + +These are integration tests: they boot the real `key-broker.mjs` under `node` +against a local fake upstream. The class is skipped when `node` is absent. + +Run: python3 -m unittest discover -s .github/groom/tests -p 'test_*.py' -v +""" + +import http.client +import os +import shutil +import socket +import socketserver +import subprocess +import sys +import time +import unittest +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from threading import Thread + +# A distinctive sentinel so the /proc assertions can't false-negative against a +# real key that happens to live in this machine's environment. +REAL_KEY = "sk-broker-test-REALKEY-9f2c1a7b4e" + +NODE = shutil.which("node") +SCRIPT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "key-broker.mjs")) + + +def _free_port(): + """Bind :0, read the assigned port, release it — good enough for a test.""" + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + try: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + finally: + s.close() + + +def _wait_port(port, timeout=15): + deadline = time.time() + timeout + while time.time() < deadline: + try: + with socket.create_connection(("127.0.0.1", port), timeout=0.2): + return True + except OSError: + time.sleep(0.05) + return False + + +class _RecordingHandler(BaseHTTPRequestHandler): + """Fake upstream: records each request and returns a canned JSON response.""" + + def _handle(self): + length = int(self.headers.get("Content-Length") or 0) + body = self.rfile.read(length) if length else b"" + self.server.records.append({ + "method": self.command, + "path": self.path, + "headers": {k.lower(): v for k, v in self.headers.items()}, + "body": body, + }) + payload = self.server.resp_body + self.send_response(self.server.resp_status) + self.send_header("Content-Type", "application/json") + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + + do_GET = _handle + do_POST = _handle + do_PUT = _handle + + def log_message(self, *args): # silence the default stderr access log + pass + + +class _Upstream(ThreadingHTTPServer): + def server_bind(self): + # Skip HTTPServer.server_bind's socket.getfqdn() reverse-DNS lookup, which + # can hang for tens of seconds on hosts with slow PTR resolution (seen on + # macOS). TCPServer.server_bind just binds; we set the name/port ourselves. + socketserver.TCPServer.server_bind(self) + host, port = self.server_address[:2] + self.server_name = host + self.server_port = port + + +def _start_upstream(): + server = _Upstream(("127.0.0.1", 0), _RecordingHandler) + server.records = [] + server.resp_status = 200 + server.resp_body = b'{"upstream":"pong","n":42}' + thread = Thread(target=server.serve_forever, daemon=True) + thread.start() + return server + + +def _start_broker(upstream_url, port, stdin_payload=None): + env = {**os.environ, "GROOM_BROKER_UPSTREAM": upstream_url, "GROOM_BROKER_PORT": str(port)} + proc = subprocess.Popen( + [NODE, SCRIPT], + stdin=subprocess.PIPE, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + env=env, + ) + # The real key arrives ONLY on stdin's first line — never env, never argv. + # stdin_payload lets a test feed a raw (e.g. padded/CRLF) line; default is + # the clean key followed by the newline the broker's readline consumes. + payload = (REAL_KEY + "\n") if stdin_payload is None else stdin_payload + proc.stdin.write(payload.encode()) + proc.stdin.flush() + return proc + + +def _stop_broker(proc): + proc.terminate() + try: + proc.wait(timeout=5) + except subprocess.TimeoutExpired: + proc.kill() + proc.wait() + if proc.stdin: + proc.stdin.close() + + +@unittest.skipUnless(NODE, "node is required to run the key-broker") +class KeyBrokerTest(unittest.TestCase): + def setUp(self): + self.upstream = _start_upstream() + # Cleanups run LIFO: shutdown() the serve loop first, then close the socket. + self.addCleanup(self.upstream.server_close) + self.addCleanup(self.upstream.shutdown) + upstream_url = "http://127.0.0.1:%d" % self.upstream.server_address[1] + + self.port = _free_port() + self.broker = _start_broker(upstream_url, self.port) + self.addCleanup(_stop_broker, self.broker) + if not _wait_port(self.port): + raise self.failureException("broker did not become connectable") + + def _conn(self, port=None): + return http.client.HTTPConnection("127.0.0.1", port or self.port, timeout=5) + + @property + def records(self): + return self.upstream.records + + # --- header injection + status/body round-trip ------------------------- + + def test_injects_real_key_and_strips_inbound_credentials(self): + self.upstream.resp_status = 202 # prove a non-200 status round-trips + conn = self._conn() + conn.request( + "POST", + "/v1/messages?beta=true", + body=b'{"model":"x"}', + headers={ + "x-api-key": "dummy", + "authorization": "Bearer nope", + "content-type": "application/json", + }, + ) + resp = conn.getresponse() + got_body = resp.read() + conn.close() + + self.assertEqual(resp.status, 202) + self.assertEqual(got_body, self.upstream.resp_body) + + self.assertEqual(len(self.records), 1) + rec = self.records[0] + self.assertEqual(rec["method"], "POST") + self.assertEqual(rec["path"], "/v1/messages?beta=true") + # The upstream saw the REAL key, not the dummy the CLI sent... + self.assertEqual(rec["headers"].get("x-api-key"), REAL_KEY) + self.assertNotEqual(rec["headers"].get("x-api-key"), "dummy") + # ...and the inbound authorization header was stripped entirely. + self.assertNotIn("authorization", rec["headers"]) + + def test_stdin_key_is_trimmed(self): + # A padded/copy-pasted secret (leading+trailing spaces + CRLF) must reach + # the upstream TRIMMED: the raw value carries illegal header chars that + # would otherwise throw on client.request. Guards key-broker.mjs `.trim()`. + padded = " " + REAL_KEY + " \r\n" + port = _free_port() + upstream_url = "http://127.0.0.1:%d" % self.upstream.server_address[1] + proc = _start_broker(upstream_url, port, stdin_payload=padded) + self.addCleanup(_stop_broker, proc) + if not _wait_port(port): + raise self.failureException("padded-key broker did not become connectable") + + conn = self._conn(port) + conn.request("POST", "/v1/messages", body=b"{}", headers={"content-type": "application/json"}) + resp = conn.getresponse() + resp.read() + conn.close() + + # The trimmed key — not the padded input — reaches x-api-key upstream. + self.assertEqual(self.records[-1]["headers"].get("x-api-key"), REAL_KEY) + + def test_request_body_reaches_upstream_intact(self): + body = b'{"prompt":"unicode \xe2\x9c\x93 check","pad":"' + b"x" * 2048 + b'"}' + conn = self._conn() + conn.request( + "POST", + "/v1/messages", + body=body, + headers={"x-api-key": "dummy", "content-type": "application/json"}, + ) + resp = conn.getresponse() + resp.read() + conn.close() + + self.assertEqual(len(self.records), 1) + self.assertEqual(self.records[0]["body"], body) + + # --- local-only responses (never forwarded) --------------------------- + + def test_head_root_is_answered_locally(self): + conn = self._conn() + conn.request("HEAD", "/") + resp = conn.getresponse() + resp.read() + conn.close() + + self.assertEqual(resp.status, 200) + self.assertEqual(self.records, []) # the probe was NOT forwarded + + def test_non_v1_path_is_404_and_not_forwarded(self): + conn = self._conn() + conn.request("GET", "/anything-else") + resp = conn.getresponse() + resp.read() + conn.close() + + self.assertEqual(resp.status, 404) + self.assertEqual(self.records, []) + + # --- the key must not leak via the process table ---------------------- + + @unittest.skipUnless(sys.platform.startswith("linux"), "/proc is Linux-only") + def test_key_absent_from_process_env_and_argv(self): + pid = self.broker.pid + with open("/proc/%d/environ" % pid, "rb") as f: + environ = f.read() + with open("/proc/%d/cmdline" % pid, "rb") as f: + cmdline = f.read() + self.assertNotIn(REAL_KEY.encode(), environ) + self.assertNotIn(REAL_KEY.encode(), cmdline) + + # --- upstream failure is a local 502 and non-fatal -------------------- + + def test_upstream_down_returns_502_and_broker_survives(self): + dead_port = _free_port() # nothing is listening here + broker_port = _free_port() + proc = _start_broker("http://127.0.0.1:%d" % dead_port, broker_port) + self.addCleanup(_stop_broker, proc) + if not _wait_port(broker_port): + raise self.failureException("broker did not become connectable") + + for _ in range(2): # a second request proves the broker stayed alive + conn = self._conn(broker_port) + conn.request("POST", "/v1/messages", body=b"{}", + headers={"content-type": "application/json"}) + resp = conn.getresponse() + resp.read() + conn.close() + self.assertEqual(resp.status, 502) + + self.assertIsNone(proc.poll(), "broker should stay alive after an upstream error") + + # --- a client abort mid-request must not crash the broker ------------- + + def test_client_abort_does_not_kill_broker(self): + # Send a Content-Length that promises more body than we deliver, then + # slam the socket shut — the classic "unhandled 'error' on req.pipe" + # crash. The broker must survive and still serve the next request. + raw = socket.create_connection(("127.0.0.1", self.port), timeout=5) + raw.sendall( + b"POST /v1/messages HTTP/1.1\r\n" + b"Host: 127.0.0.1\r\n" + b"Content-Type: application/json\r\n" + b"Content-Length: 4096\r\n\r\n" + b"{}" # far short of the promised 4096 bytes + ) + raw.close() # abort mid-body + time.sleep(0.2) + + self.assertIsNone(self.broker.poll(), "broker must survive a client abort") + conn = self._conn() # a fresh request proves it's still serving + conn.request("HEAD", "/") + resp = conn.getresponse() + resp.read() + conn.close() + self.assertEqual(resp.status, 200) + + # --- a non-root upstream keeps its path prefix ------------------------ + + def test_upstream_path_prefix_is_preserved(self): + prefixed = "http://127.0.0.1:%d/prefix" % self.upstream.server_address[1] + port = _free_port() + proc = _start_broker(prefixed, port) + self.addCleanup(_stop_broker, proc) + if not _wait_port(port): + raise self.failureException("broker did not become connectable") + + conn = self._conn(port) + conn.request("POST", "/v1/messages", body=b"{}", + headers={"content-type": "application/json"}) + resp = conn.getresponse() + resp.read() + conn.close() + + self.assertEqual(resp.status, 200) + self.assertEqual(self.records[-1]["path"], "/prefix/v1/messages") + + +class BrokerStartupTest(unittest.TestCase): + """Startup guards that don't need a running upstream.""" + + @unittest.skipUnless(NODE, "node is required to run the key-broker") + def test_exits_nonzero_when_stdin_has_no_key(self): + # No key on stdin (immediate EOF) -> refuse to start, non-zero exit. + proc = subprocess.Popen( + [NODE, SCRIPT], + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + env={**os.environ, "GROOM_BROKER_PORT": str(_free_port())}, + ) + self.assertNotEqual(proc.wait(timeout=10), 0) + + def _startup_exit_code(self, env_overrides): + env = {**os.environ, "GROOM_BROKER_PORT": str(_free_port())} + env.update(env_overrides) + proc = subprocess.Popen( + [NODE, SCRIPT], + stdin=subprocess.DEVNULL, + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + env=env, + ) + return proc.wait(timeout=10) + + @unittest.skipUnless(NODE, "node is required to run the key-broker") + def test_rejects_port_with_trailing_garbage(self): + # "8199x" must be rejected, not silently coerced to 8199. + self.assertNotEqual(self._startup_exit_code({"GROOM_BROKER_PORT": "8199x"}), 0) + + @unittest.skipUnless(NODE, "node is required to run the key-broker") + def test_rejects_plaintext_non_loopback_upstream(self): + # http:// to a non-loopback host would leak the key in cleartext. + code = self._startup_exit_code({"GROOM_BROKER_UPSTREAM": "http://example.com"}) + self.assertNotEqual(code, 0) + + +if __name__ == "__main__": + unittest.main()