From d9537e9d84ecfea096c8522e41ff7e30a5d3ad52 Mon Sep 17 00:00:00 2001 From: itswl Date: Mon, 19 Oct 2026 09:55:58 +0800 Subject: [PATCH 1/4] =?UTF-8?q?locks:=20mcp=202.0.0=20=E2=86=92=202.3.0=20?= =?UTF-8?q?for=20CVE-2026-59951?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pip-audit turned the probe's gate red on a new advisory against the SDK's transitive mcp pin (fix versions 1.30.0 / 2.2.0). Upgraded in the probe's venv, relocked with scripts/relock.sh (only this pin and mcp-types moved), audit clean. Unrelated to the rest of this branch and kept apart so it cherry-picks alone. --- hookprobe/requirements.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hookprobe/requirements.lock b/hookprobe/requirements.lock index a9525a5f..0c4d4e1f 100644 --- a/hookprobe/requirements.lock +++ b/hookprobe/requirements.lock @@ -30,8 +30,8 @@ jsonschema-specifications==2025.9.1 librt==0.16.0 license-expression==30.4.4 markdown-it-py==4.2.0 -mcp==2.0.0 -mcp-types==2.0.0 +mcp==2.3.0 +mcp-types==2.3.0 mdurl==0.1.2 msgpack==1.2.1 mypy==2.4.0 From 564f708a2a24ff3eccb6bb8f2a2c9d935b986781 Mon Sep 17 00:00:00 2001 From: itswl Date: Mon, 19 Oct 2026 09:55:58 +0800 Subject: [PATCH 2/4] hookprobe: a chat bot's incoming-webhook URL is a credential The output guard names the kind (chat webhook url) for Feishu/Lark, Slack, DingTalk, WeCom and Discord incoming hooks, and the redactor masks the token half while keeping the host, so a case file still says which chat was posted to. A second deployment running copies of these files read a function's environment on 2026-10-12 and such a URL went into its transcript, two logs and the model call; nothing here named the shape, and this family posts its own cards through exactly such URLs. Fixtures are visibly fake on purpose: the estate guard refuses anything shaped like a real token. --- hookprobe/hookprobe/gate.py | 15 +++++++++++++++ hookprobe/hookprobe/redact.py | 9 +++++++++ hookprobe/tests/test_output_guard.py | 16 ++++++++++++++++ hookprobe/tests/test_redact.py | 17 +++++++++++++++++ 4 files changed, 57 insertions(+) diff --git a/hookprobe/hookprobe/gate.py b/hookprobe/hookprobe/gate.py index 97ad5be6..57fe71ab 100644 --- a/hookprobe/hookprobe/gate.py +++ b/hookprobe/hookprobe/gate.py @@ -277,6 +277,21 @@ def deny_reason( ("openai-style api key", re.compile(r"\bsk-[A-Za-z0-9_-]{20,}\b")), ("slack token", re.compile(r"\bxox[abprs]-[A-Za-z0-9-]{10,}\b")), ("json web token", re.compile(r"\beyJ[A-Za-z0-9_-]{10,}\.eyJ[A-Za-z0-9_-]{10,}\.[A-Za-z0-9_-]{10,}\b")), + # A chat bot's incoming-webhook URL IS the credential: whoever holds it posts + # to that chat as the bot, and this family posts its own cards through + # exactly such URLs. A sibling deployment read a function's environment on + # 2026-10-12 and its chat webhook went into the transcript, two logs and the + # model call; nothing here named the shape until now. + ( + "chat webhook url", + re.compile( + r"open-apis/bot/v2/hook/[A-Za-z0-9-]{8,}" + r"|hooks\.slack\.com/services/[A-Za-z0-9/]{20,}" + r"|oapi\.dingtalk\.com/robot/send\?access_token=[A-Za-z0-9]{20,}" + r"|qyapi\.weixin\.qq\.com/cgi-bin/webhook/send\?key=[A-Za-z0-9-]{20,}" + r"|discord(?:app)?\.com/api/webhooks/[0-9]{10,}/[A-Za-z0-9_-]{20,}" + ), + ), ) # How much of an answer is examined. Tool output can be a megabyte of logs and diff --git a/hookprobe/hookprobe/redact.py b/hookprobe/hookprobe/redact.py index 81f6a1e7..b52f71ac 100644 --- a/hookprobe/hookprobe/redact.py +++ b/hookprobe/hookprobe/redact.py @@ -114,6 +114,15 @@ re.compile(r"(? None: @@ -54,6 +68,8 @@ def test_a_credential_in_tool_output_is_named_by_kind(kind: str, output: str) -> # guard people learn to ignore — every Kubernetes manifest has them. "spec:\n containers:\n - env:\n - name: DB_PASSWORD\n valueFrom:\n secretKeyRef:", "error: secret 'db-creds' not found", + # A webhook HOST without its token is a destination, not a credential. + "channel ops-feishu posts to open.larksuite.com (bridge mode)", "", ], ) diff --git a/hookprobe/tests/test_redact.py b/hookprobe/tests/test_redact.py index 1265b6de..31b4232b 100644 --- a/hookprobe/tests/test_redact.py +++ b/hookprobe/tests/test_redact.py @@ -26,6 +26,23 @@ def test_the_shapes_that_carry_a_secret_are_masked() -> None: ("kubectl get pods --token=eyJhbGciOiJSUzI1NiJ9.abc", "eyJhbGciOiJSUzI1NiJ9.abc"), ("redis-cli -a hunter2 --scan", "hunter2"), ("aws s3 ls --secret-access-key wJalrXUtnFEMI0K7MDENGbPxRfiCY", "wJalrXUtnFEMI0K7MDENGbPxRfiCY"), + # A chat bot's incoming webhook: the host survives, the token does not. + ( + "curl -X POST https://open.larksuite.com/open-apis/bot/v2/hook/FAKE-HOOK-TOKEN-FOR-A-TEST -d '{}'", + "FAKE-HOOK-TOKEN-FOR-A-TEST", + ), + ( + "curl https://hooks.slack.com/services/FAKE/FAKE/FAKESLACKTOKENFORATESTONLY", + "FAKESLACKTOKENFORATESTONLY", + ), + ( + "curl 'https://qyapi.weixin.qq.com/cgi-bin/webhook/send?key=FAKE-WECOM-KEY-FOR-A-TEST-ONLY'", + "FAKE-WECOM-KEY", + ), + ( + "curl https://discord.com/api/webhooks/1234567890123/FAKE-DISCORD-TOKEN-FOR-A-TEST", + "FAKE-DISCORD-TOKEN-FOR-A-TEST", + ), ] for command, secret in cases: masked = _tool_detail({"command": command}) From bee3db7024330f2329e556c47b5ab13354788076 Mon Sep 17 00:00:00 2001 From: itswl Date: Mon, 19 Oct 2026 09:55:58 +0800 Subject: [PATCH 3/4] hookprobe: scratch and the CLI's own logs get a retention window of their own HOOKPROBE_SCRATCH_DAYS (default 7): the agent subprocess gets TMPDIR={workdir}/scratch, and that directory plus $HOME/.cache/claude-cli-nodejs are swept daily on their own window, whatever HOOKPROBE_RETENTION_DAYS says. A scratch folder is old only when nothing under it was written inside the window, so a job still writing into its own folder is left alone. Measured on two probe volumes: the CLI's MCP transport logs had grown since 2026-09-04 (113 and 364 files) carrying whatever tools returned, with retention.prune never naming them and the case-file window defaulting to keep-forever because case files are recall. A second deployment found a day-old scratch holding other teams' configs. Passed through every compose that passes the retention knob; reference regenerated. --- deploy/docker-compose.work.yml | 1 + deploy/docker-compose.yml | 1 + docker-compose.yml | 1 + hookprobe/deploy/docker-compose.prod.yml | 1 + hookprobe/deploy/docker-compose.yml | 1 + hookprobe/docs/reference.md | 3 +- hookprobe/hookprobe/app.py | 6 +- hookprobe/hookprobe/engine.py | 6 ++ hookprobe/hookprobe/retention.py | 78 ++++++++++++++++++++++++ hookprobe/hookprobe/settings.py | 6 ++ hookprobe/tests/helpers.py | 2 + hookprobe/tests/test_hygiene.py | 2 + hookprobe/tests/test_ops.py | 28 +++++++++ 13 files changed, 133 insertions(+), 3 deletions(-) diff --git a/deploy/docker-compose.work.yml b/deploy/docker-compose.work.yml index e9859a0e..e282148b 100644 --- a/deploy/docker-compose.work.yml +++ b/deploy/docker-compose.work.yml @@ -402,6 +402,7 @@ services: # Reachable HERE, not only on production. `assert_knobs_are_reachable.py` # asks whether SOME compose passes a knob, and both of these were # satisfied by the production compose alone while being unsettable on the + HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} # node that actually runs them — measured on this stack 2026-09-12. # Empty keeps the documented default either way. HOOKPROBE_SELFTEST_EVERY_SECONDS: ${HOOKPROBE_SELFTEST_EVERY_SECONDS:-} diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index b9c97cb7..179819db 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -96,6 +96,7 @@ services: # Case files and transcripts older than this are pruned daily. 0 keeps # everything — the case files are the agent's episodic memory. HOOKPROBE_RETENTION_DAYS: ${HOOKPROBE_RETENTION_DAYS:-} + HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} # Skill layers: "project" (distilled, on the volume) is always on; # add "user" to also load a host library mounted at # /data/home/.claude/skills. HOOKPROBE_SKILLS pins the session's diff --git a/docker-compose.yml b/docker-compose.yml index a1af38b3..25bf2d8c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -185,6 +185,7 @@ services: # Case files and transcripts older than this are pruned daily. 0 keeps # everything — the case files are the agent's episodic memory. HOOKPROBE_RETENTION_DAYS: ${HOOKPROBE_RETENTION_DAYS:-} + HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} # Skill layers: "project" (distilled, on the volume) is always on; # add "user" to also load a host library mounted at # /data/home/.claude/skills. HOOKPROBE_SKILLS pins the session's diff --git a/hookprobe/deploy/docker-compose.prod.yml b/hookprobe/deploy/docker-compose.prod.yml index 93eca401..e69798e6 100644 --- a/hookprobe/deploy/docker-compose.prod.yml +++ b/hookprobe/deploy/docker-compose.prod.yml @@ -208,6 +208,7 @@ services: # add "user" to also load a host library mounted at # /data/home/.claude/skills. HOOKPROBE_SKILLS pins the session's # skill list (comma names or "all") so a big library cannot flood it. + HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} HOOKPROBE_SETTING_SOURCES: ${HOOKPROBE_SETTING_SOURCES:-} HOOKPROBE_SKILLS: ${HOOKPROBE_SKILLS:-} # Operator methodology appended to the system prompt (a file path; diff --git a/hookprobe/deploy/docker-compose.yml b/hookprobe/deploy/docker-compose.yml index 46570a09..c8b4d9bb 100644 --- a/hookprobe/deploy/docker-compose.yml +++ b/hookprobe/deploy/docker-compose.yml @@ -62,6 +62,7 @@ services: # Case files and transcripts older than this are pruned daily. 0 keeps # everything — the case files are the agent's episodic memory. HOOKPROBE_RETENTION_DAYS: ${HOOKPROBE_RETENTION_DAYS:-} + HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} # Skill layers: "project" (distilled, on the volume) is always on; # add "user" to also load a host library mounted at # /data/home/.claude/skills. HOOKPROBE_SKILLS pins the session's diff --git a/hookprobe/docs/reference.md b/hookprobe/docs/reference.md index 2037ac3c..5aa13fb1 100644 --- a/hookprobe/docs/reference.md +++ b/hookprobe/docs/reference.md @@ -7,7 +7,7 @@ routes and their first sentence from the handlers. Correct it by editing the comment beside the field or the handler's docstring — an edit here is lost on the next run, and `scripts/gen_reference.py --check` will say so. -## Environment (79) +## Environment (80) | variable | default | what it is | | --- | --- | --- | @@ -60,6 +60,7 @@ the next run, and `scripts/gen_reference.py --check` will say so. | `HOOKPROBE_BUDGET_USD` | `0.0` | The budget breaker, guarding the only path that spends money without a human asking: once the window's recorded spend reaches budget_usd, the event door refuses NEW investigations… | | `HOOKPROBE_BUDGET_WINDOW_HOURS` | `24.0` | Window the spend ceiling is measured over | | `HOOKPROBE_RETENTION_DAYS` | `0` | Volume retention (days): case files and engine transcripts older than this are pruned daily | +| `HOOKPROBE_SCRATCH_DAYS` | `7` | Scratch retention (days): the agent subprocess's TMPDIR ({workdir}/scratch) and the CLI's own transport logs under $HOME/.cache are pruned daily when older than this, whatever rete… | | `HOOKPROBE_SELFTEST_EVERY_SECONDS` | `3600` | How often the node demonstrates its own boundaries (/v1/selftest) and tells the alarm channel when one does not hold | | `HOOKPROBE_PI_PROVIDER` | *(empty)* | Which provider in that configuration serves this node's model | | `HOOKPROBE_SKILLS` | *(empty)* | Directory of skills the engine may load | diff --git a/hookprobe/hookprobe/app.py b/hookprobe/hookprobe/app.py index 4dab819d..3375cb62 100644 --- a/hookprobe/hookprobe/app.py +++ b/hookprobe/hookprobe/app.py @@ -64,7 +64,7 @@ from hookprobe.engine import file_fact from hookprobe.files import system_prompt_path from hookprobe.live import Live -from hookprobe.retention import prune +from hookprobe.retention import prune, prune_scratch from hookprobe.runs import INFERRED_BY_PREFIX, RUNNING, Run, turn_cost from hookprobe.service import NotResumableError, NoTurnRunningError, RunBusyError, RunService from hookprobe.settings import Settings @@ -240,6 +240,7 @@ async def lifespan(_: FastAPI): async def retention_loop() -> None: while True: await asyncio.to_thread(prune, settings.workdir, Path.home(), settings.retention_days) + await asyncio.to_thread(prune_scratch, settings.workdir, Path.home(), settings.scratch_days) await asyncio.sleep(86400) async def outcome_loop() -> None: @@ -247,7 +248,8 @@ async def outcome_loop() -> None: await asyncio.sleep(OUTCOME_SWEEP_SECONDS) service.sweep_outcomes() - pruner = asyncio.create_task(retention_loop()) if settings.retention_days > 0 else None + sweeps = settings.retention_days > 0 or settings.scratch_days > 0 + pruner = asyncio.create_task(retention_loop()) if sweeps else None outcomes = asyncio.create_task(outcome_loop()) # And the boundaries, on a clock. `/v1/selftest` shipped run by nothing, # which makes it a claim rather than a check; this is what turns it into diff --git a/hookprobe/hookprobe/engine.py b/hookprobe/hookprobe/engine.py index 0b108329..12be1f3f 100644 --- a/hookprobe/hookprobe/engine.py +++ b/hookprobe/hookprobe/engine.py @@ -878,6 +878,12 @@ def _subprocess_env(self, session_key: str = "") -> dict[str, str]: # and all three derive it the same way or the derivation is worthless. env.update(agent_credentials(self._settings)) env.update(trace_environment(session_key)) + # Where a step's temporary files go: on the volume, inside the scratch + # window (retention.prune_scratch), rather than the container's /tmp + # where nothing prunes them and nothing can see what a run downloaded. + scratch = self._workdir / "scratch" + scratch.mkdir(parents=True, exist_ok=True) + env["TMPDIR"] = str(scratch) return env def describe_inputs(self, *, resume: str | None = None) -> dict[str, Any]: diff --git a/hookprobe/hookprobe/retention.py b/hookprobe/hookprobe/retention.py index 59bc462f..992df428 100644 --- a/hookprobe/hookprobe/retention.py +++ b/hookprobe/hookprobe/retention.py @@ -11,11 +11,18 @@ from __future__ import annotations import logging +import os +import shutil import time from pathlib import Path logger = logging.getLogger("hookprobe.retention") +# Where the agent's subprocesses put temporary files (engine._subprocess_env +# sets TMPDIR to it), and where the CLI writes its own MCP transport logs. +SCRATCH = "scratch" +CLI_CACHE = Path(".cache") / "claude-cli-nodejs" + def prune(workdir: Path, home: Path, days: int) -> int: """Remove case files and transcripts older than `days`. Returns the count.""" @@ -47,3 +54,74 @@ def prune(workdir: Path, home: Path, days: int) -> int: if removed: logger.info("retention removed %s file(s) older than %s days", removed, days) return removed + + +def _newest_write(path: Path) -> float | None: + """When anything under `path` was last written; None if any of it is unreadable.""" + try: + newest = path.lstat().st_mtime + except OSError: + return None + if path.is_dir() and not path.is_symlink(): + failed: list[OSError] = [] + for root, dirs, files in os.walk(path, onerror=failed.append): + for name in dirs + files: + try: + newest = max(newest, os.lstat(os.path.join(root, name)).st_mtime) + except OSError: + return None + if failed: + return None + return newest + + +def prune_scratch(workdir: Path, home: Path, days: int) -> int: + """Remove scratch entries and CLI transport logs older than `days`. + + Two things that are not the agent's memory and never were: `{workdir}/scratch`, + the TMPDIR every agent subprocess is given, where a download an investigation + made lands; and `$HOME/.cache/claude-cli-nodejs`, the CLI's own MCP transport + logs, which carry whatever a tool returned — colleagues' messages included — + and which nothing reads back. Measured 2026-10-19 on two nodes: those logs + had grown since 2026-09-04 with `prune` above never naming them, and a + sibling deployment had found a day-old scratch holding other teams' configs. + A window of their own because the default differs: case files are recall and + default to kept; these default to a week. + + A scratch entry is a top-level file or directory. A directory is old only + when NOTHING under it was written inside the window, so a job still writing + into its own folder is left alone; anything unreadable is left alone too. + """ + if days <= 0: + return 0 + cutoff = time.time() - days * 86400 + removed = 0 + scratch = workdir / SCRATCH + try: + entries = sorted(scratch.iterdir()) if scratch.is_dir() else [] + except OSError: + entries = [] + for entry in entries: + newest = _newest_write(entry) + if newest is None or newest >= cutoff: + continue + try: + if entry.is_dir() and not entry.is_symlink(): + shutil.rmtree(entry) + else: + entry.unlink() + removed += 1 + except OSError: + continue + cache = home / CLI_CACHE + if cache.is_dir(): + for path in cache.glob("**/*"): + try: + if path.is_file() and path.stat().st_mtime < cutoff: + path.unlink() + removed += 1 + except OSError: + continue + if removed: + logger.info("scratch retention removed %s entr(ies) older than %s days", removed, days) + return removed diff --git a/hookprobe/hookprobe/settings.py b/hookprobe/hookprobe/settings.py index 21110a2a..347b8697 100644 --- a/hookprobe/hookprobe/settings.py +++ b/hookprobe/hookprobe/settings.py @@ -441,6 +441,11 @@ class Settings: # this are pruned daily. 0 keeps everything — the case files are the # agent's episodic memory, so deletion is a choice, never a surprise. retention_days: int + # Scratch retention (days): the agent subprocess's TMPDIR ({workdir}/scratch) + # and the CLI's own transport logs under $HOME/.cache are pruned daily when + # older than this, whatever `retention_days` says — downloads and debug + # traffic, not the agent's memory. 0 keeps them. + scratch_days: int # Direct self-alarm for report returns that exhaust their retries: the # pipe is the broken link at that moment, so this posts straight to a @@ -552,6 +557,7 @@ def load(cls) -> Settings: budget_window_hours=max(0.1, _float("HOOKPROBE_BUDGET_WINDOW_HOURS", 24.0)), budget_gates_agent_door=_flag("HOOKPROBE_BUDGET_GATES_AGENT_DOOR", False), retention_days=max(0, _int("HOOKPROBE_RETENTION_DAYS", 0)), + scratch_days=max(0, _int("HOOKPROBE_SCRATCH_DAYS", 7)), alarm_url=os.environ.get("HOOKPROBE_ALARM_URL", "").strip(), alarm_min_interval_seconds=_int("HOOKPROBE_ALARM_MIN_INTERVAL_SECONDS", 600), selftest_every_seconds=max(0, _int("HOOKPROBE_SELFTEST_EVERY_SECONDS", 3600)), diff --git a/hookprobe/tests/helpers.py b/hookprobe/tests/helpers.py index b399a7ee..d4f302bd 100644 --- a/hookprobe/tests/helpers.py +++ b/hookprobe/tests/helpers.py @@ -38,6 +38,8 @@ def make_settings(tmp_path: Path, **overrides: object) -> Settings: "default_timeout_seconds": 5, "max_timeout_seconds": 10, "workdir": tmp_path, + # Off in the fixture: a test that wants the scratch sweep says so. + "scratch_days": 0, "mcp_config": None, "setting_sources": ("project",), "skills": "", diff --git a/hookprobe/tests/test_hygiene.py b/hookprobe/tests/test_hygiene.py index 125fa9d4..592cf4dd 100644 --- a/hookprobe/tests/test_hygiene.py +++ b/hookprobe/tests/test_hygiene.py @@ -157,6 +157,8 @@ def test_service_secrets_are_withheld_from_the_agent_subprocess(tmp_path: Path) from tests.helpers import make_settings env = ClaudeAgentEngine(make_settings(tmp_path, bash_timeout_ms=30000, bash_max_timeout_ms=90000))._subprocess_env() + # Temporary files land on the volume, under the scratch window, not in /tmp. + assert env["TMPDIR"] == str(tmp_path / "scratch") and (tmp_path / "scratch").is_dir() for name in ( "HOOKPROBE_EVENT_SECRET", "HOOKPROBE_RETURN_SECRET", diff --git a/hookprobe/tests/test_ops.py b/hookprobe/tests/test_ops.py index 20cbb6de..64d71b30 100644 --- a/hookprobe/tests/test_ops.py +++ b/hookprobe/tests/test_ops.py @@ -44,6 +44,34 @@ def test_prune_removes_only_old_files(tmp_path) -> None: # Skills and memory live outside both roots — pruning never sees them. +def test_scratch_and_the_cli_logs_have_a_window_of_their_own(tmp_path) -> None: + """Downloads and the CLI's transport logs were never the agent's memory, so + they are swept on their own window whatever `retention_days` says. A folder + something wrote into this week stays whole, however old the folder itself.""" + from hookprobe.retention import prune_scratch + + workdir, home = tmp_path / "wd", tmp_path / "home" + old_job = workdir / "scratch" / "job-old" + old_job.mkdir(parents=True) + (old_job / "function-config.json").write_text("{}") + live_job = workdir / "scratch" / "job-live" + live_job.mkdir() + (live_job / "partial.bin").write_text("x") + old_dump = workdir / "scratch" / "dump.bin" + old_dump.write_text("x") + log = home / ".cache" / "claude-cli-nodejs" / "-data" / "mcp-logs-chat" / "2026-09-04.jsonl" + log.parent.mkdir(parents=True) + log.write_text("") + stale = time.time() - 10 * 86400 + for path in (old_job, old_job / "function-config.json", old_dump, log, live_job): + os.utime(path, (stale, stale)) # live_job's own mtime is old; its file is fresh + + assert prune_scratch(workdir, home, 0) == 0, "0 keeps everything" + assert prune_scratch(workdir, home, 7) == 3 + assert not old_job.exists() and not old_dump.exists() and not log.exists() + assert (live_job / "partial.bin").exists(), "a folder written into this week is left whole" + + def test_healthz_slot_arithmetic(tmp_path) -> None: engine = GatedEngine() settings = make_settings(tmp_path, token=TOKEN, max_concurrent=2) From 0353c45e2181a614a229197f8986b058c32b420a Mon Sep 17 00:00:00 2001 From: itswl Date: Mon, 19 Oct 2026 09:55:58 +0800 Subject: [PATCH 4/4] egress: the allowlist names shapes, and a listed name must resolve where its kind says EGRESS_ALLOW takes @aws-api ([.].amazonaws.com, any service, any region) and @aws-s3 (a bucket in every spelling), and the work compose's default uses them instead of .amazonaws.com, which also admitted every hostname anyone can get under that suffix: an instance's public name, a load balancer, an API Gateway stage, somebody else's EKS cluster. A cluster endpoint the planner is given goes on the operator's list by name; the shapes do not match it on purpose. An unknown @ rule stops the container at start. A listed name is vetted at the address: the proxy resolves it, refuses any answer that is loopback, link-local, private, reserved or the host machine's mapped address (IPv4-in-IPv6 included), and connects to the address it vetted, never a second lookup. Compose peers go in EGRESS_INTERNAL and must resolve inside the compose network; both deploy composes name theirs. The name is checked before any lookup. A name that does not resolve is a 502, not a 403. Decision note beside it. --- ...a-name-must-resolve-where-its-kind-says.md | 68 +++++++ deploy/docker-compose.work.yml | 17 +- deploy/sidecars/proxy.py | 192 ++++++++++++++++-- deploy/sidecars/tests/test_allowlist.py | 149 +++++++++++++- hookprobe/deploy/docker-compose.prod.yml | 6 +- 5 files changed, 401 insertions(+), 31 deletions(-) create mode 100644 .agents/notes/implemented/2026-10-19-the-allowlist-names-shapes-and-a-name-must-resolve-where-its-kind-says.md diff --git a/.agents/notes/implemented/2026-10-19-the-allowlist-names-shapes-and-a-name-must-resolve-where-its-kind-says.md b/.agents/notes/implemented/2026-10-19-the-allowlist-names-shapes-and-a-name-must-resolve-where-its-kind-says.md new file mode 100644 index 00000000..59e2e5ce --- /dev/null +++ b/.agents/notes/implemented/2026-10-19-the-allowlist-names-shapes-and-a-name-must-resolve-where-its-kind-says.md @@ -0,0 +1,68 @@ +--- +title: The allowlist names shapes, and a name must resolve where its kind says +status: implemented +date: 2026-10-19 +scope: stack +--- + +## Decision + +Three changes to what leaves a probe and what a probe keeps, each one a gap a +second deployment running copies of these files found in use and closed: + +1. **`EGRESS_ALLOW` admits two named shapes, `@aws-api` and `@aws-s3`, and the + work compose's default uses them instead of `.amazonaws.com`.** `@aws-api` + is `[.].amazonaws.com`, any service, any region; `@aws-s3` + is a bucket in every spelling. Neither matches an EKS cluster endpoint, an + EC2 instance's public name, a load balancer or an API Gateway stage: the + first label of those is anyone's. A cluster the operator means is one bare + name in the operator's own list. An unknown `@` rule stops the container at + start. +2. **A listed name is vetted at the address.** The proxy resolves the name, + refuses any answer that is loopback, link-local, private, reserved or the + address a desktop container runtime maps the host machine to, and connects to + the address it vetted — never a second lookup. Compose peers are named in + `EGRESS_INTERNAL` and held to the inverse: they must resolve inside the + compose network. The name is checked before any lookup. A name that does not + resolve is a 502, not a 403. +3. **A chat bot's incoming-webhook URL is a credential.** The investigator's + output guard names the kind (`chat webhook url`) and the redactor masks the + token half of Feishu/Lark, Slack, DingTalk, WeCom and Discord hooks while + keeping the host. +4. **Scratch and the CLI's own logs have a retention window of their own** + (`HOOKPROBE_SCRATCH_DAYS`, default 7): the agent subprocess gets + `TMPDIR={workdir}/scratch`, and that directory plus `$HOME/.cache/claude-cli-nodejs` + are pruned daily, whatever `HOOKPROBE_RETENTION_DAYS` says. + +## Why + +- `.amazonaws.com` was written for the planner's service calls and admitted + every hostname anyone can obtain under that suffix. The sibling verified it + with the matcher itself; the operator's list carried the same rule. +- The list was on names and the connection on addresses, with nothing between + them. In this family's topology the probes have no route, so the risk sat in + the doors container, which does — and that container is the one with the + allowlist in it. +- On 2026-10-12 the sibling's action job read a function's environment and a + chat webhook URL went into the transcript, two logs and the model call. + Nothing in `redact.py` or `_SECRET_SHAPES` named the shape, and this family + posts its own cards through exactly such URLs. +- The sibling found a day-old scratch holding other teams' configs with on-call + numbers. Here the measured accumulation was different and older: the CLI's + MCP transport logs under `home/.cache` had grown since 2026-09-04 on two + nodes, carrying whatever tools returned, with nothing reading them and + `retention.prune` not naming them. Case files stay kept-forever by default + because they are recall; these never were. + +## Consequences + +- Operators whose `.env` pins `EGRESS_ALLOW` keep what they pinned until they + change it; the work stack's line now carries the two shapes and its cluster + endpoints by name. +- A compose peer reached through the proxy has to be in `EGRESS_INTERNAL`; the + two deploy composes list theirs. A peer name that resolves to the host is + refused, which is the point. +- The redaction keeps the platform host, so a case file still says which chat + was posted to. +- `HOOKPROBE_SCRATCH_DAYS=0` keeps scratch and the logs; the window is per node + like every other knob of its kind. diff --git a/deploy/docker-compose.work.yml b/deploy/docker-compose.work.yml index e282148b..c931a365 100644 --- a/deploy/docker-compose.work.yml +++ b/deploy/docker-compose.work.yml @@ -208,8 +208,21 @@ services: # its old name so the ledger's history stays one file. - ../work-data/mcp-gate:/data environment: + # `@aws-api` is any AWS service API endpoint in any region and `@aws-s3` + # is a bucket (deploy/sidecars/proxy.py, SHAPES). They replaced + # `.amazonaws.com` on 2026-10-19: that suffix also admitted every hostname + # anyone can get under it — an instance's public name, a load balancer, an + # API Gateway stage, somebody else's EKS cluster — a destination an + # attacker controls, inside the allowlist. A cluster endpoint the planner + # is given has an id for its first label, so the shapes do not match it on + # purpose: add it to EGRESS_ALLOW in .env by name, next to these. EGRESS_ALLOW: >- - ${EGRESS_ALLOW:-open.larksuite.com,api.anthropic.com,.amazonaws.com,docs.aws.amazon.com,grafana.com,raw.githubusercontent.com,hookrelay,sink} + ${EGRESS_ALLOW:-open.larksuite.com,api.anthropic.com,@aws-api,@aws-s3,docs.aws.amazon.com,grafana.com,raw.githubusercontent.com,hookrelay,sink} + # This compose's own peers reached through the proxy. A listed external + # name must resolve to a public address; a peer named here must resolve + # inside the compose network — a peer name that has fallen through to the + # host's resolver is not the peer, and is refused. + EGRESS_INTERNAL: ${EGRESS_INTERNAL:-hookrelay,sink} MCPGATE_UPSTREAM: ${WATCH_CHAT_MCP_URL:-http://host.docker.internal:52222/mcp/} # A second MCP server shares this port as a route: replace the line above # with one MCPGATE_ROUTE_= per server, write each node's list @@ -389,6 +402,7 @@ services: HOOKPROBE_PRICE_CACHE_READ_PER_1M: ${HOOKPROBE_PRICE_CACHE_READ_PER_1M:-} HOOKPROBE_PRICE_CACHE_WRITE_PER_1M: ${HOOKPROBE_PRICE_CACHE_WRITE_PER_1M:-} HOOKPROBE_RETENTION_DAYS: ${HOOKPROBE_RETENTION_DAYS:-} + HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} # Operator methodology appended to the system prompt, re-read every run — # edit it and the next run uses it, no restart. That file is how this # instance becomes a PLANNER with opinions rather than a generic @@ -402,7 +416,6 @@ services: # Reachable HERE, not only on production. `assert_knobs_are_reachable.py` # asks whether SOME compose passes a knob, and both of these were # satisfied by the production compose alone while being unsettable on the - HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} # node that actually runs them — measured on this stack 2026-09-12. # Empty keeps the documented default either way. HOOKPROBE_SELFTEST_EVERY_SECONDS: ${HOOKPROBE_SELFTEST_EVERY_SECONDS:-} diff --git a/deploy/sidecars/proxy.py b/deploy/sidecars/proxy.py index ad0435ba..867a3c60 100644 --- a/deploy/sidecars/proxy.py +++ b/deploy/sidecars/proxy.py @@ -18,17 +18,47 @@ that nobody in this repository can audit would be a worse trade than the hole it closes. +WHAT THE LIST IS MADE OF. Three kinds of rule, comma-separated in EGRESS_ALLOW: +a bare name matches exactly; a leading dot matches the tree under it and the +apex; and two named shapes, `@aws-api` and `@aws-s3`, stand for "an AWS service +API endpoint, any service, any region" and "an S3 bucket" — +`.amazonaws.com`, `..amazonaws.com`, +`.s3[.].amazonaws.com`. The shapes replaced `.amazonaws.com` on +2026-10-19: that suffix also admitted every hostname anyone can get under +amazonaws.com — an EC2 instance's public name, a load balancer, an API Gateway +stage, somebody else's EKS cluster — which is a destination an attacker controls +sitting inside the allowlist. A read-only investigation needs the service APIs +and the buckets; a cluster endpoint it is given is one more bare name in the +operator's list, and the shapes do not match it on purpose (the id in front is +anyone's). Found on a second deployment running a copy of this file, and +verified against this file's own matcher. A shape the proxy does not know stops +the container at start rather than admitting nothing in silence. + +WHERE A NAME IS ALLOWED TO RESOLVE. The list is on names; the connection is to +an address, and those are two claims. A listed name that resolves to a loopback, +link-local, private or reserved address — or to the address a desktop container +runtime maps the host machine to — is refused, and the proxy connects to the +address it vetted rather than looking the name up a second time. The names of +this compose's own peers (EGRESS_INTERNAL: the pipe, the sink) are the exception +and the inverse: they MUST resolve to a private or loopback address, because a +peer whose name has fallen through to the host's resolver is not the peer. The +name is checked before any lookup, so a name is not a DNS exfiltration channel. +A name that does not resolve is a 502, not a 403: the agent is told to read 403 +as "refused, do not retry", and a resolver hiccup is not that. + WHAT THIS IS NOT. It is not a defence against an adversary with code execution -and patience: the allowlist is on HOSTNAMES, and a name on it that serves -attacker-controlled content is a way out (the model gateway itself takes POST -bodies). It bounds the accident and the opportunist, it makes the attempt +and patience: a name on the list that serves attacker-controlled content is a +way out (the model gateway itself takes POST bodies, and anyone can create an +S3 bucket). It bounds the accident and the opportunist, it makes the attempt visible, and it turns "anywhere" into "somewhere somebody chose". """ from __future__ import annotations +import ipaddress import logging import os +import re import socket import socketserver import sys @@ -39,23 +69,54 @@ logging.basicConfig(level=logging.INFO, format="%(asctime)s %(levelname)s %(name)s %(message)s") logger = logging.getLogger("egress") -# Comma-separated. A bare name matches exactly; a leading dot matches any -# subdomain (".amazonaws.com" admits "ec2.ap-southeast-1.amazonaws.com"). -# Empty means allow NOTHING, which is the safe direction for a variable that -# failed to interpolate — an empty allowlist is loud within seconds, where an -# empty list meaning "allow all" would be silent forever. -ALLOW = tuple(h.strip().lower() for h in os.environ.get("EGRESS_ALLOW", "").split(",") if h.strip()) +_AWS_REGION = r"[a-z]{2}(?:-gov)?-[a-z]+-[0-9]" +# The two shapes an `@` rule may name. Full matches, lower-case hosts. +SHAPES: dict[str, re.Pattern[str]] = { + "@aws-api": re.compile(rf"[a-z0-9][a-z0-9-]*(?:\.{_AWS_REGION})?\.amazonaws\.com"), + "@aws-s3": re.compile(rf"[a-z0-9][a-z0-9.-]*\.s3(?:\.dualstack)?(?:[.-]{_AWS_REGION})?\.amazonaws\.com"), +} + + +def _rules(text: str) -> tuple[str, ...]: + return tuple(h.strip().lower().rstrip(".") for h in text.split(",") if h.strip()) + + +# Comma-separated. Empty means allow NOTHING, which is the safe direction for a +# variable that failed to interpolate — an empty allowlist is loud within +# seconds, where an empty list meaning "allow all" would be silent forever. +ALLOW = _rules(os.environ.get("EGRESS_ALLOW", "")) +# This compose's own peers, by name: admitted without being on ALLOW, and held +# to resolving inside the compose network. +INTERNAL = frozenset(_rules(os.environ.get("EGRESS_INTERNAL", ""))) PORT = int(os.environ.get("EGRESS_PORT", "8888")) # Ports a client may CONNECT to. Everything the family speaks is HTTP or HTTPS; # a proxy that will open 22 or 5432 for you is a tunnel, not a boundary. PORTS = frozenset(int(p) for p in os.environ.get("EGRESS_PORTS", "80,443,8088,8100,8200,4318").split(",") if p.strip()) IDLE_SECONDS = 300 +CONNECT_TIMEOUT = 20 +# Where a desktop container runtime maps the host machine, inside a range that +# is otherwise "this network" and that the stdlib does not call private. +HOST_MAPPED = ipaddress.ip_network("0.250.250.0/24") +# Where a compose peer may resolve: the bridge networks' private ranges, and +# loopback for the one case of a peer that is this container itself. +INTERNAL_NETS = tuple( + ipaddress.ip_network(n) for n in ("10.0.0.0/8", "172.16.0.0/12", "192.168.0.0/16", "127.0.0.0/8", "::1/128") +) + +for _rule in ALLOW: + if _rule.startswith("@") and _rule not in SHAPES: + sys.exit(f"egress: EGRESS_ALLOW names a shape this proxy does not know: {_rule} (known: {', '.join(SHAPES)})") def permitted(host: str) -> bool: host = host.lower().strip(".") + if not host: + return False for rule in ALLOW: - if rule.startswith("."): + if rule.startswith("@"): + if SHAPES[rule].fullmatch(host): + return True + elif rule.startswith("."): if host == rule[1:] or host.endswith(rule): return True elif host == rule: @@ -63,6 +124,84 @@ def permitted(host: str) -> bool: return False +def internal(host: str) -> bool: + return host.lower().strip(".") in INTERNAL + + +class Refused(Exception): + """A destination this proxy will not connect to, with the status the client gets.""" + + def __init__(self, code: int, why: str) -> None: + super().__init__(why) + self.code, self.why = code, why + + +def _unwrapped(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> ipaddress.IPv4Address | ipaddress.IPv6Address: + """An IPv4 address carried inside IPv6 is the IPv4 address.""" + return ip.ipv4_mapped if ip.version == 6 and ip.ipv4_mapped is not None else ip + + +def public(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: + candidate = _unwrapped(ip) + if candidate.version == 4 and candidate in HOST_MAPPED: + return False + return candidate.is_global + + +def inside_compose(ip: ipaddress.IPv4Address | ipaddress.IPv6Address) -> bool: + candidate = _unwrapped(ip) + return any(candidate in net for net in INTERNAL_NETS) + + +def resolve(host: str, port: int, kind: str) -> list[tuple]: + """Every address the name gives, each vetted for where that kind of name may + point: `external` must be public, `internal` must be inside the compose.""" + try: + infos = socket.getaddrinfo(host, port, type=socket.SOCK_STREAM) + except socket.gaierror as exc: + raise Refused(502, f"did not resolve: {exc}") from None + vetted: list[tuple] = [] + for family, type_, proto, _canon, sockaddr in infos: + ip = ipaddress.ip_address(sockaddr[0].split("%", 1)[0]) + if kind == "internal" and not inside_compose(ip): + raise Refused(403, f"a compose peer resolved outside the compose network ({ip})") + if kind == "external" and not public(ip): + raise Refused(403, f"resolves to a non-public address ({ip})") + vetted.append((family, type_, proto, sockaddr)) + if not vetted: + raise Refused(502, "did not resolve") + return vetted + + +def decide(host: str, port: int) -> list[tuple]: + """Name first, address second. The list is consulted before any lookup, then + every address the name resolves to is vetted; what comes back is what + `connect` dials, with no second lookup in between.""" + host = host.lower().strip(".") + if port not in PORTS: + raise Refused(403, f"port {port} is not one this proxy opens") + if internal(host): + return resolve(host, port, "internal") + if not permitted(host): + raise Refused(403, "not on the egress allowlist") + return resolve(host, port, "external") + + +def connect(infos: list[tuple]) -> socket.socket: + """A socket to the first vetted address that answers.""" + error: OSError | None = None + for family, type_, proto, sockaddr in infos: + sock = socket.socket(family, type_, proto) + sock.settimeout(CONNECT_TIMEOUT) + try: + sock.connect(sockaddr) + return sock + except OSError as exc: + error = exc + sock.close() + raise error or OSError("no address to connect to") + + class Handler(socketserver.StreamRequestHandler): timeout = 30 # Unbuffered reads. The default `rfile` is a buffered reader, and its first @@ -102,14 +241,13 @@ def handle(self) -> None: self._refuse(400, "no port in the CONNECT target", target[:80]) return host = host.strip("[]") - if port not in PORTS: - self._refuse(403, f"port {port} is not one this proxy opens", target[:80]) - return - if not permitted(host): - self._refuse(403, "not on the egress allowlist", target[:80]) + try: + infos = decide(host, port) + except Refused as refused: + self._refuse(refused.code, refused.why, target[:80]) return try: - upstream = socket.create_connection((host, port), timeout=20) + upstream = connect(infos) except OSError as exc: self._refuse(502, f"upstream refused: {exc}", target[:80], level=logging.WARNING) return @@ -141,11 +279,14 @@ def _plain(self, parts: list[str]) -> None: except ValueError: self._refuse(400, "unreadable port", authority[:80]) return - if port not in PORTS or not permitted(host): - self._refuse(403, "not on the egress allowlist", f"{host}:{port}") + host = host.strip("[]") + try: + infos = decide(host, port) + except Refused as refused: + self._refuse(refused.code, refused.why, f"{host}:{port}") return try: - upstream = socket.create_connection((host, port), timeout=20) + upstream = connect(infos) except OSError as exc: self._refuse(502, f"upstream refused: {exc}", f"{host}:{port}") return @@ -173,8 +314,9 @@ def _refuse(self, code: int, why: str, target: str, level: int = logging.WARNING # whether it was a mistake, a dependency nobody declared, or worse. logger.log(level, "REFUSED %s — %s", target, why) body = f"{why}\n".encode() + reason = {400: "Bad Request", 403: "Forbidden", 502: "Bad Gateway"}.get(code, "Forbidden") try: - self.wfile.write(f"HTTP/1.1 {code} Forbidden\r\nContent-Length: {len(body)}\r\n\r\n".encode() + body) + self.wfile.write(f"HTTP/1.1 {code} {reason}\r\nContent-Length: {len(body)}\r\n\r\n".encode() + body) self.wfile.flush() except OSError: pass @@ -185,9 +327,17 @@ def build() -> Server: and the ingress in one process; main() runs it alone.""" if not ALLOW: logger.warning("EGRESS_ALLOW is empty — every destination will be refused") - logger.info("egress proxy on :%s, %s host rule(s), ports %s", PORT, len(ALLOW), sorted(PORTS)) + logger.info( + "egress proxy on :%s, %s host rule(s), %s compose peer(s), ports %s", + PORT, + len(ALLOW), + len(INTERNAL), + sorted(PORTS), + ) for rule in ALLOW: logger.info(" allow %s", rule) + for name in sorted(INTERNAL): + logger.info(" peer %s (must resolve inside the compose network)", name) return Server(("0.0.0.0", PORT), Handler) # nosec B104 — internal network only diff --git a/deploy/sidecars/tests/test_allowlist.py b/deploy/sidecars/tests/test_allowlist.py index fb0f7e8e..a104c7bd 100644 --- a/deploy/sidecars/tests/test_allowlist.py +++ b/deploy/sidecars/tests/test_allowlist.py @@ -17,14 +17,27 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1])) -def _with(allow: str, ports: str = "80,443"): +def _with(allow: str, ports: str = "80,443", internal: str = ""): os.environ["EGRESS_ALLOW"] = allow os.environ["EGRESS_PORTS"] = ports + os.environ["EGRESS_INTERNAL"] = internal import proxy return importlib.reload(proxy) +def _resolving_to(monkeypatch, module, *addresses: str): + """A resolver that answers every name with these addresses, and counts.""" + calls: list[str] = [] + + def fake(host, port, *args, **kwargs): + calls.append(host) + return [(2, 1, 6, "", (a, port)) for a in addresses] + + monkeypatch.setattr(module.socket, "getaddrinfo", fake) + return calls + + def test_a_bare_name_is_exact_and_a_lookalike_is_refused() -> None: """`endswith("example.com")` would have admitted `notexample.com`, and that is the domain somebody registers on purpose.""" @@ -36,11 +49,131 @@ def test_a_bare_name_is_exact_and_a_lookalike_is_refused() -> None: def test_a_leading_dot_admits_the_tree_and_the_apex() -> None: - p = _with(".amazonaws.com") - assert p.permitted("ec2.ap-southeast-1.amazonaws.com") - assert p.permitted("amazonaws.com") - assert not p.permitted("notamazonaws.com") - assert not p.permitted("amazonaws.com.evil.net") + p = _with(".example.net") + assert p.permitted("api.eu.example.net") + assert p.permitted("example.net") + assert not p.permitted("notexample.net") + assert not p.permitted("example.net.evil.net") + + +def test_the_aws_api_shape_admits_service_endpoints_and_nothing_anyone_can_register() -> None: + """`.amazonaws.com` admitted every hostname anyone can get under it. The shape + admits `[.].amazonaws.com` and refuses the forms whose first + label is somebody's: an instance, a load balancer, an API Gateway stage, an + EKS cluster — a cluster the operator means goes on the list by name.""" + p = _with("@aws-api") + for host in ( + "sts.amazonaws.com", + "iam.amazonaws.com", + "ec2.ap-southeast-1.amazonaws.com", + "logs.us-gov-west-1.amazonaws.com", + "ssmmessages.eu-central-1.amazonaws.com", + ): + assert p.permitted(host), host + for host in ( + "amazonaws.com", + "notamazonaws.com", + "sts.amazonaws.com.evil.net", + "ec2-1-2-3-4.ap-southeast-1.compute.amazonaws.com", + "my-lb-123456.ap-southeast-1.elb.amazonaws.com", + "abc123.execute-api.ap-southeast-1.amazonaws.com", + "0123456789abcdef0123456789abcdef.gr7.ap-southeast-1.eks.amazonaws.com", + "my-bucket.s3.ap-southeast-1.amazonaws.com", + ): + assert not p.permitted(host), host + + +def test_the_s3_shape_admits_buckets_in_every_spelling() -> None: + p = _with("@aws-s3") + for host in ( + "my-bucket.s3.amazonaws.com", + "my.dotted.bucket.s3.ap-southeast-1.amazonaws.com", + "my-bucket.s3-ap-southeast-1.amazonaws.com", + "my-bucket.s3.dualstack.ap-southeast-1.amazonaws.com", + ): + assert p.permitted(host), host + for host in ( + "s3.amazonaws.com", + "evil.com", + "my-bucket.s3.amazonaws.com.evil.net", + "ec2.ap-southeast-1.amazonaws.com", + ): + assert not p.permitted(host), host + + +def test_a_shape_the_proxy_does_not_know_stops_it_at_start() -> None: + """A typo in a rule must not become a rule that admits nothing in silence.""" + with pytest.raises(SystemExit, match="shape"): + _with("@aws-apii") + _with("example.com") # and the module imports again afterwards + + +def test_a_listed_name_may_not_resolve_to_a_private_or_host_address(monkeypatch) -> None: + """The list is on names and the connection is to an address. A listed name + whose answer is loopback, private, link-local or the host machine's mapped + address is refused, including an IPv4 answer wrapped in IPv6.""" + p = _with("example.com") + for address in ( + "127.0.0.1", + "10.0.0.5", + "172.18.0.9", + "192.168.1.2", + "169.254.169.254", + "0.250.250.254", + "::ffff:127.0.0.1", + "::1", + ): + _resolving_to(monkeypatch, p, address) + with pytest.raises(p.Refused) as refused: + p.decide("example.com", 443) + assert refused.value.code == 403 and "non-public" in refused.value.why, address + _resolving_to(monkeypatch, p, "93.184.216.34") + assert p.decide("example.com", 443)[0][3] == ("93.184.216.34", 443) + + +def test_one_bad_answer_refuses_the_whole_name(monkeypatch) -> None: + p = _with("example.com") + _resolving_to(monkeypatch, p, "93.184.216.34", "127.0.0.1") + with pytest.raises(p.Refused): + p.decide("example.com", 443) + + +def test_a_compose_peer_must_resolve_inside_the_compose(monkeypatch) -> None: + """A peer name that has fallen through to the host's resolver is not the peer.""" + p = _with("", internal="hookrelay") + _resolving_to(monkeypatch, p, "172.18.0.5") + assert p.decide("hookrelay", 443)[0][3] == ("172.18.0.5", 443) + _resolving_to(monkeypatch, p, "127.0.0.1") + assert p.decide("hookrelay", 443), "loopback is this container itself" + for address in ("93.184.216.34", "0.250.250.254"): + _resolving_to(monkeypatch, p, address) + with pytest.raises(p.Refused) as refused: + p.decide("hookrelay", 443) + assert refused.value.code == 403 and "compose" in refused.value.why, address + + +def test_the_name_is_checked_before_any_lookup(monkeypatch) -> None: + """Otherwise a name is a DNS exfiltration channel: the lookup itself leaves.""" + p = _with("example.com") + calls = _resolving_to(monkeypatch, p, "93.184.216.34") + with pytest.raises(p.Refused) as refused: + p.decide("evil.example.org", 443) + assert refused.value.code == 403 and calls == [] + + +def test_a_name_that_does_not_resolve_is_a_502_not_a_refusal(monkeypatch) -> None: + """The agent is told 403 means do not retry; a resolver hiccup must not read as that.""" + import socket as socket_module + + p = _with("example.com") + + def failing(*args, **kwargs): + raise socket_module.gaierror(-2, "Name or service not known") + + monkeypatch.setattr(p.socket, "getaddrinfo", failing) + with pytest.raises(p.Refused) as refused: + p.decide("example.com", 443) + assert refused.value.code == 502 def test_case_and_a_trailing_dot_do_not_get_around_it() -> None: @@ -97,7 +230,9 @@ def handle(self) -> None: upstream = socketserver.TCPServer(("127.0.0.1", 0), Echo) port = upstream.server_address[1] - proxy = _with("127.0.0.1", str(port)) + # Loopback is where a compose peer may live (this container itself), never + # where a listed external name may point. + proxy = _with("", str(port), internal="127.0.0.1") gateway = proxy.Server(("127.0.0.1", 0), proxy.Handler) for server in (upstream, gateway): threading.Thread(target=server.serve_forever, kwargs={"poll_interval": 0.02}, daemon=True).start() diff --git a/hookprobe/deploy/docker-compose.prod.yml b/hookprobe/deploy/docker-compose.prod.yml index e69798e6..8d9f880d 100644 --- a/hookprobe/deploy/docker-compose.prod.yml +++ b/hookprobe/deploy/docker-compose.prod.yml @@ -77,6 +77,10 @@ services: # degrades to "carried and allowed" rather than "refused". EGRESS_ALLOW: >- ${EGRESS_ALLOW:-open.larksuite.com,api.anthropic.com,docs.aws.amazon.com,grafana.com,raw.githubusercontent.com,hookrelay,hookjudge,hookprobe,webhook-receiver,webhookwise-observability-alloy-1} + # The in-network names above, named as peers: each must resolve inside the + # compose network, where an external name must resolve to a public address + # (deploy/sidecars/proxy.py says why the two are held apart). + EGRESS_INTERNAL: ${EGRESS_INTERNAL:-hookrelay,hookjudge,hookprobe,webhook-receiver,webhookwise-observability-alloy-1} hookprobe: build: @@ -204,11 +208,11 @@ services: # Case files and transcripts older than this are pruned daily. 0 keeps # everything — the case files are the agent's episodic memory. HOOKPROBE_RETENTION_DAYS: ${HOOKPROBE_RETENTION_DAYS:-} + HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} # Skill layers: "project" (distilled, on the volume) is always on; # add "user" to also load a host library mounted at # /data/home/.claude/skills. HOOKPROBE_SKILLS pins the session's # skill list (comma names or "all") so a big library cannot flood it. - HOOKPROBE_SCRATCH_DAYS: ${HOOKPROBE_SCRATCH_DAYS:-} HOOKPROBE_SETTING_SOURCES: ${HOOKPROBE_SETTING_SOURCES:-} HOOKPROBE_SKILLS: ${HOOKPROBE_SKILLS:-} # Operator methodology appended to the system prompt (a file path;