fix(proxy/interceptors): constrain disk-verify to a trusted, confined workspace root

JerrettDavis flagged the opt-in disk-verify fallback as a local-file
oracle: x-headroom-cwd and the Read tool's file_path are HTTP-caller
input, absolute paths bypassed cwd, symlinks were followed unchecked,
and the read had no size bound or off-loop guarantee.

Gate verification on is_loopback_host(request.client.host) -- a fact
the server observes from the TCP peer, not something a header can
assert -- before cwd or file_path are even inspected. Once trusted,
confine every read strictly under the canonicalized cwd via
resolve() + relative_to() (rejects `..` traversal, absolute escapes,
and post-resolution symlink escapes), require a regular file, and
enforce a byte cap checked via fstat on the open fd before any read;
O_NONBLOCK keeps a special file with no writer from parking a worker
thread. Untrusted requests return UNKNOWN without touching cwd,
file_path resolution, or disk at all.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
slxng1758 2026-08-25 18:20:35 -04:00
parent b356d3a88d
commit b6fcbc6317
6 changed files with 330 additions and 26 deletions

View file

@ -15,6 +15,7 @@ import logging
import os
import re
import shutil
import stat
import subprocess
import tempfile
from enum import Enum
@ -24,7 +25,7 @@ from typing import Any
from headroom import binaries
from headroom._subprocess import run
from headroom.proxy import runtime_env
from headroom.proxy.project_context import get_current_cwd
from headroom.proxy.project_context import get_current_cwd, is_current_request_trusted
from . import base
@ -151,32 +152,108 @@ def _verify_truncation_on_disk_enabled() -> bool:
)
def _max_disk_verify_bytes() -> int:
# Live read, same hot-reload pattern as _min_chars_to_rewrite(). 5 MB
# comfortably covers real source files while bounding worst-case read
# time/memory for the disk-verify fallback.
try:
return int(runtime_env.getenv("HEADROOM_VERIFY_TRUNCATION_MAX_BYTES", "5000000"))
except (TypeError, ValueError):
return 5_000_000
def _resolve_read_path_in_workspace(file_path: str, resolved_root: Path) -> Path | None:
"""Confine `file_path` (relative or absolute) strictly under `resolved_root`.
Mirrors memory_handler._resolve_native_path's join/resolve/relative_to
pattern. `resolved_root` must already be canonicalized by the caller (a
single source of truth for "inside the workspace" across a call) --
this does not re-resolve it. `.resolve()` collapses symlinks, including
in intermediate path components, before the containment check runs, so
a symlink that points outside the workspace is rejected the same way a
`..` traversal is -- this is a path-segment containment check via
`relative_to()`, not a string-prefix check, so a sibling directory that
merely shares a string prefix with the root is correctly rejected too.
"""
candidate = Path(file_path) if Path(file_path).is_absolute() else resolved_root / file_path
try:
resolved = candidate.resolve(strict=True)
except OSError:
return None
try:
resolved.relative_to(resolved_root)
except ValueError:
return None
return resolved
def _read_disk_content_bounded(resolved: Path, max_bytes: int) -> str | None:
"""Read `resolved` iff it's a regular file no larger than `max_bytes`.
Opens with O_NONBLOCK so a FIFO/special file with no writer returns
immediately instead of blocking the calling thread indefinitely --
O_NONBLOCK has no effect on reads once fstat confirms a regular file.
fstat runs on the already-open fd (not the path) so the type/size
check and the read happen on the same underlying file object, closing
the TOCTOU gap a separate path-based stat-then-open would leave.
"""
try:
fd = os.open(resolved, os.O_RDONLY | os.O_NONBLOCK)
except OSError:
return None
try:
st = os.fstat(fd)
if not stat.S_ISREG(st.st_mode) or st.st_size > max_bytes:
return None
with os.fdopen(fd, "r", encoding="utf-8") as f:
fd = -1 # ownership transferred to the file object
return f.read()
except (OSError, UnicodeDecodeError):
return None
finally:
if fd >= 0:
os.close(fd)
def _verify_read_against_disk(
file_path: str | None,
received_content: str,
cwd: str | None,
*,
trusted: bool,
) -> tuple[ReadVerificationResult, tuple[int, int] | None]:
"""Compare `received_content` against the real file at `file_path`.
`cwd` (the `x-headroom-cwd` header) resolves a relative `file_path`.
`cwd` (the `x-headroom-cwd` header) is never authority on its own --
`trusted` (whether the request's peer is loopback, a server-observed
fact, not a header) must hold before `cwd` or `file_path` are even
inspected, let alone touch disk. Once trusted, every read must be a
regular file whose fully resolved path is beneath the canonically
resolved `cwd`; anything else is UNKNOWN, never a guess.
TRUNCATED requires an exact-prefix match with strictly more on disk
a weaker match means the file diverged since the client read it, not
a provable truncation, so it's UNKNOWN. Returns `(visible_lines,
total_lines)` alongside TRUNCATED so the header can cite real numbers
without a second, potentially racy, read.
"""
if not trusted:
return ReadVerificationResult.UNKNOWN, None
if not file_path:
return ReadVerificationResult.UNKNOWN, None
if os.path.isabs(file_path):
resolved = file_path
elif cwd:
resolved = os.path.join(cwd, file_path)
else:
if not cwd or not os.path.isabs(cwd):
return ReadVerificationResult.UNKNOWN, None
try:
disk_content = Path(resolved).read_text(encoding="utf-8")
except (OSError, UnicodeDecodeError) as e:
logger.debug("disk verification: cannot read %s: %s", resolved, e)
resolved_root = Path(cwd).resolve(strict=True)
except OSError:
return ReadVerificationResult.UNKNOWN, None
if not resolved_root.is_dir():
return ReadVerificationResult.UNKNOWN, None
resolved = _resolve_read_path_in_workspace(file_path, resolved_root)
if resolved is None:
return ReadVerificationResult.UNKNOWN, None
disk_content = _read_disk_content_bounded(resolved, _max_disk_verify_bytes())
if disk_content is None:
return ReadVerificationResult.UNKNOWN, None
if disk_content == received_content:
return ReadVerificationResult.COMPLETE, None
@ -233,7 +310,10 @@ class AstGrepReadOutline:
truncation = _detect_truncation(tool_output)
if truncation is None and _verify_truncation_on_disk_enabled():
verdict, disk_truncation = _verify_read_against_disk(
_path_from_input(tool_input), tool_output, get_current_cwd()
_path_from_input(tool_input),
tool_output,
get_current_cwd(),
trusted=is_current_request_trusted(),
)
if verdict is ReadVerificationResult.TRUNCATED:
truncation = disk_truncation

View file

@ -17,6 +17,16 @@ second, unsanitized contextvar for consumers that need the literal
filesystem path (e.g. verifying a Read tool_result against disk). Not
(yet) bound at the WebSocket accept paths an absent cwd there is already
treated as "can't resolve, don't guess."
A third contextvar, ``_current_request_trusted``, records whether the
active request's peer is loopback — a fact the HTTP middleware observes
from the TCP connection itself, not something a caller can assert via a
header. Consumers that turn ``_current_cwd`` into a filesystem read (again,
disk verification) must gate on this first: the cwd header alone is never
sufficient authority for touching disk, only a signal to interpret once the
peer is already known to be trusted. Defaults to ``False`` so any request
path that never calls the setter (e.g. the WebSocket accept path) is safe
by construction.
"""
from __future__ import annotations
@ -41,6 +51,11 @@ _current_project: ContextVar[str | None] = ContextVar("headroom_current_project"
# tool's file_path and read from disk, so it must stay the literal path.
_current_cwd: ContextVar[str | None] = ContextVar("headroom_current_cwd", default=None)
# Server-observed (not header-derived) loopback signal — see module docstring.
_current_request_trusted: ContextVar[bool] = ContextVar(
"headroom_current_request_trusted", default=False
)
def set_current_project(project: str | None) -> None:
"""Bind the active request's project for downstream outcome recording."""
@ -62,6 +77,16 @@ def get_current_cwd() -> str | None:
return _current_cwd.get()
def set_current_request_trusted(trusted: bool) -> None:
"""Bind whether the active request's peer is loopback."""
_current_request_trusted.set(trusted)
def is_current_request_trusted() -> bool:
"""Whether the active request's peer is loopback, or ``False`` if unset."""
return _current_request_trusted.get()
def strip_project_path_prefix(scope: MutableMapping[str, Any]) -> str | None:
"""Strip a ``/p/<name>`` prefix from an ASGI scope, returning the name.
@ -80,8 +105,10 @@ __all__ = [
"classify_project",
"get_current_cwd",
"get_current_project",
"is_current_request_trusted",
"set_current_cwd",
"set_current_project",
"set_current_request_trusted",
"split_project_path",
"strip_project_path_prefix",
"with_project_prefix",

View file

@ -80,6 +80,11 @@ RUNTIME_ENV_KNOBS: tuple[Knob, ...] = (
"bool",
"Verify ast-grep Read truncation against disk when no client banner is found.",
),
Knob(
"HEADROOM_VERIFY_TRUNCATION_MAX_BYTES",
"int",
"Max on-disk file size (bytes) the disk-verify fallback will read.",
),
)
_KNOBS_BY_ENV: dict[str, Knob] = {k.env: k for k in RUNTIME_ENV_KNOBS}

View file

@ -162,6 +162,7 @@ from headroom.proxy.project_context import (
classify_project,
set_current_cwd,
set_current_project,
set_current_request_trusted,
strip_project_path_prefix,
)
from headroom.proxy.prometheus_metrics import PrometheusMetrics # noqa: F401
@ -3263,8 +3264,19 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI:
method = request.method
query = request.url.query
headers = dict(request.headers.items())
client = getattr(request, "client", None)
client_addr = ""
client_host = None
if client is not None:
client_host = getattr(client, "host", None)
client_port = getattr(client, "port", None)
client_addr = f"{client_host}:{client_port}" if client_port else str(client_host)
set_current_project(classify_project(headers) or prefix_project)
set_current_cwd(headers.get("x-headroom-cwd"))
# Server-observed (not header-derived) trust signal for consumers that
# turn x-headroom-cwd into a filesystem read (e.g. astgrep disk
# verification) -- the header alone is never sufficient authority.
set_current_request_trusted(is_loopback_host(client_host))
# Path-based Codex identification: stamp X-Client: codex on the
# Responses endpoint for callers that don't otherwise classify (e.g.
# Codex Desktop, whose User-Agent isn't a known codex UA). Without it
@ -3274,12 +3286,6 @@ def create_app(config: ProxyConfig | None = None) -> FastAPI:
# makes every downstream classify_client(headers) read "codex".
if should_stamp_codex_client(path, headers):
request.scope["headers"].append((b"x-client", b"codex"))
client = getattr(request, "client", None)
client_addr = ""
if client is not None:
client_host = getattr(client, "host", None)
client_port = getattr(client, "port", None)
client_addr = f"{client_host}:{client_port}" if client_port else str(client_host)
try:
proxy.metrics.record_inbound_request(method=method, path=path)
except Exception:

View file

@ -745,3 +745,44 @@ def test_dashboard_client_cidr_does_not_expand_other_management_endpoints(
assert client.get("/admin/upstream").status_code == 404
assert client.get("/debug/tasks").status_code == 404
assert client.post("/stats/reset").status_code == 404
def _app_with_trust_probe() -> FastAPI:
"""`_record_headroom_stack` binds `is_current_request_trusted()` from the
peer's loopback-ness before any route runs; this test-only route reads
it back so the binding itself (not just a downstream consumer like
astgrep's disk-verify fallback) can be asserted directly."""
from headroom.proxy.project_context import is_current_request_trusted
app = _make_app()
@app.get("/__test/trusted")
def _trusted_probe() -> dict[str, bool]:
return {"trusted": is_current_request_trusted()}
# A catch-all passthrough route registered by create_app() would
# otherwise shadow this path -- Starlette matches routes in
# registration order, not by specificity.
app.router.routes.insert(0, app.router.routes.pop())
return app
def test_record_headroom_stack_binds_trusted_true_for_loopback_client() -> None:
client = TestClient(
_app_with_trust_probe(), base_url="http://127.0.0.1", client=("127.0.0.1", 12345)
)
resp = client.get("/__test/trusted", headers={"x-headroom-cwd": "/some/spoofed/path"})
assert resp.json() == {"trusted": True}
def test_record_headroom_stack_binds_trusted_false_for_non_loopback_client() -> None:
"""A spoofed x-headroom-cwd header from a non-loopback peer must not
make the request trusted -- the header carries no authority on its
own. Direct regression test for the disk-verify oracle finding."""
client = TestClient(
_app_with_trust_probe(),
base_url="http://100.82.0.2:8787",
client=("100.90.0.5", 12345),
)
resp = client.get("/__test/trusted", headers={"x-headroom-cwd": "/some/spoofed/path"})
assert resp.json() == {"trusted": False}

View file

@ -2,6 +2,8 @@
from __future__ import annotations
import os
import sys
import textwrap
import pytest
@ -20,7 +22,11 @@ from headroom.proxy.interceptors.astgrep import (
_verify_read_against_disk,
)
from headroom.proxy.interceptors.base import reset_interceptor_failure_counts
from headroom.proxy.project_context import get_current_cwd, set_current_cwd
from headroom.proxy.project_context import (
get_current_cwd,
set_current_cwd,
set_current_request_trusted,
)
from headroom.tokenizer import Tokenizer
@ -370,24 +376,47 @@ def test_set_current_cwd_none_and_blank_both_clear():
class TestVerifyReadAgainstDisk:
def test_missing_file_path_is_unknown(self):
verdict, info = _verify_read_against_disk(None, "abc", "/repo")
verdict, info = _verify_read_against_disk(None, "abc", "/repo", trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_relative_path_without_cwd_is_unknown(self):
verdict, info = _verify_read_against_disk("payments.py", "abc", None)
verdict, info = _verify_read_against_disk("payments.py", "abc", None, trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_unreadable_file_is_unknown(self, tmp_path):
verdict, info = _verify_read_against_disk(str(tmp_path / "missing.py"), "abc", None)
def test_untrusted_request_never_touches_disk(self, tmp_path, monkeypatch):
"""A fully valid cwd/file_path/content combo that would resolve
COMPLETE if trusted must short-circuit to UNKNOWN -- and never call
os.open -- when the request isn't trusted. The regression guarantee
is "untrusted -> UNKNOWN without any filesystem operation," not just
"untrusted -> UNKNOWN.\""""
f = tmp_path / "payments.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
opened: list[object] = []
real_open = os.open
def _tracking_open(*args, **kwargs):
opened.append(args)
return real_open(*args, **kwargs)
monkeypatch.setattr(os, "open", _tracking_open)
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, str(tmp_path), trusted=False)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
assert opened == []
def test_missing_file_under_workspace_root_is_unknown(self, tmp_path):
verdict, info = _verify_read_against_disk(
str(tmp_path / "missing.py"), "abc", str(tmp_path), trusted=True
)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_exact_match_is_complete(self, tmp_path):
f = tmp_path / "payments.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, None)
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, str(tmp_path), trusted=True)
assert verdict is ReadVerificationResult.COMPLETE
assert info is None
@ -395,7 +424,7 @@ class TestVerifyReadAgainstDisk:
f = tmp_path / "payments.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
partial = _PY_FIXTURE[:200]
verdict, info = _verify_read_against_disk(str(f), partial, None)
verdict, info = _verify_read_against_disk(str(f), partial, str(tmp_path), trusted=True)
assert verdict is ReadVerificationResult.TRUNCATED
assert info == (len(partial.splitlines()), len(_PY_FIXTURE.splitlines()))
@ -403,7 +432,9 @@ class TestVerifyReadAgainstDisk:
f = tmp_path / "payments.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
partial = _PY_FIXTURE[:200]
verdict, info = _verify_read_against_disk("payments.py", partial, str(tmp_path))
verdict, info = _verify_read_against_disk(
"payments.py", partial, str(tmp_path), trusted=True
)
assert verdict is ReadVerificationResult.TRUNCATED
assert info is not None
@ -411,7 +442,78 @@ class TestVerifyReadAgainstDisk:
# File diverged since the client read it -- not a clean prefix.
f = tmp_path / "payments.py"
f.write_text(_PY_FIXTURE.replace("compute_subtotal", "compute_total"), encoding="utf-8")
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, None)
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, str(tmp_path), trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_absolute_path_outside_workspace_root_is_unknown(self, tmp_path):
root = tmp_path / "project"
root.mkdir()
sibling = tmp_path / "other"
sibling.mkdir()
f = sibling / "secret.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, str(root), trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_relative_traversal_escapes_workspace_is_unknown(self, tmp_path):
root = tmp_path / "project"
root.mkdir()
f = tmp_path / "secret.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
verdict, info = _verify_read_against_disk(
"../secret.py", _PY_FIXTURE, str(root), trusted=True
)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_sibling_directory_sharing_string_prefix_is_not_inside_workspace(self, tmp_path):
"""`relative_to()` is a path-segment containment check, not a string
prefix check -- a target that merely starts with the root's string
must not be treated as inside it."""
root = tmp_path / "project"
root.mkdir()
other = tmp_path / "project-other"
other.mkdir()
f = other / "secret.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, str(root), trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
@pytest.mark.skipif(
sys.platform == "win32", reason="symlinks need elevated privilege on Windows"
)
def test_symlinked_file_escapes_workspace_is_unknown(self, tmp_path):
root = tmp_path / "project"
root.mkdir()
outside = tmp_path / "outside.py"
outside.write_text(_PY_FIXTURE, encoding="utf-8")
link = root / "link.py"
link.symlink_to(outside)
verdict, info = _verify_read_against_disk("link.py", _PY_FIXTURE, str(root), trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_directory_passed_as_file_path_is_unknown(self, tmp_path):
# Also proves O_NONBLOCK doesn't need a FIFO fixture to matter: this
# exercises the same os.open(..., O_NONBLOCK) path, just rejected by
# the S_ISREG check rather than by not blocking on a missing writer.
root = tmp_path / "project"
subdir = root / "subdir"
subdir.mkdir(parents=True)
verdict, info = _verify_read_against_disk("subdir", "abc", str(root), trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
def test_oversized_file_exceeding_byte_cap_is_unknown(self, tmp_path, monkeypatch):
monkeypatch.setenv("HEADROOM_VERIFY_TRUNCATION_MAX_BYTES", "10")
f = tmp_path / "payments.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
assert len(_PY_FIXTURE.encode("utf-8")) > 10
# Content matches exactly -- would be COMPLETE without the cap.
verdict, info = _verify_read_against_disk(str(f), _PY_FIXTURE, str(tmp_path), trusted=True)
assert verdict is ReadVerificationResult.UNKNOWN
assert info is None
@ -432,6 +534,7 @@ def test_astgrep_disk_verification_flags_truncation_when_no_banner(
assert "truncated" not in partial.lower()
set_current_cwd(str(tmp_path))
set_current_request_trusted(True)
try:
messages = [
{
@ -453,6 +556,7 @@ def test_astgrep_disk_verification_flags_truncation_when_no_banner(
result = apply_to_messages(messages, tokenizer)
finally:
set_current_cwd(None)
set_current_request_trusted(False)
assert len(result.spans) == 1
new_content = result.messages[1]["content"][0]["content"]
@ -477,6 +581,47 @@ def test_astgrep_disk_verification_disabled_by_default(tokenizer, tmp_path, monk
partial = _PY_FIXTURE[: _PY_FIXTURE.index(marker)]
set_current_cwd(str(tmp_path))
set_current_request_trusted(True)
try:
messages = [
{
"role": "assistant",
"content": [
{
"type": "tool_use",
"id": "abc",
"name": "Read",
"input": {"file_path": "payments.py"},
}
],
},
{
"role": "user",
"content": [{"type": "tool_result", "tool_use_id": "abc", "content": partial}],
},
]
result = apply_to_messages(messages, tokenizer)
finally:
set_current_cwd(None)
set_current_request_trusted(False)
new_content = result.messages[1]["content"][0]["content"]
assert "truncated upstream" not in new_content
def test_astgrep_disk_verification_skips_when_request_untrusted(tokenizer, tmp_path, monkeypatch):
"""Same truncated-on-disk scenario, opted in and cwd bound, but the
request's peer was never marked loopback-trusted (the contextvar's
False default) -- must behave exactly like the no-signal case."""
monkeypatch.setenv("HEADROOM_VERIFY_TRUNCATION_ON_DISK", "1")
monkeypatch.setenv("HEADROOM_INTERCEPT_READ_MIN_CHARS", "50")
f = tmp_path / "payments.py"
f.write_text(_PY_FIXTURE, encoding="utf-8")
marker = "\n\ndef format_receipt"
partial = _PY_FIXTURE[: _PY_FIXTURE.index(marker)]
set_current_cwd(str(tmp_path))
# Deliberately not calling set_current_request_trusted(True).
try:
messages = [
{