diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index a003f64016..885d2d0eac 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -129,6 +129,9 @@ sequenceDiagram invents a hypothesis, observed result, or verdict. - Sandbox helpers copy the workspace, drop secret environment values unless explicitly allowlisted by **name**, and run subprocesses with `shell=False`. + Web E2E readiness URLs are loopback-only; see + [`docs/doctoring/sandboxed-web-readiness-loopback-boundary.md`](docs/doctoring/sandboxed-web-readiness-loopback-boundary.md) + and [`docs/adr/0004-sandboxed-web-readiness-loopback-boundary.md`](docs/adr/0004-sandboxed-web-readiness-loopback-boundary.md). - Logs and review receipts redact credential shapes (tokens, bearer values, known provider prefixes). They do not mask operational PII that the control plane must process. @@ -178,3 +181,5 @@ resolver conflict. — product-specific psychometric repair heartbeat and scientific gates. - [`docs/doctoring/exact-artifact-sbom-attestation.md`](docs/doctoring/exact-artifact-sbom-attestation.md) — current increment's attestation decision and APA 7th citations. +- [`docs/doctoring/sandboxed-web-readiness-loopback-boundary.md`](docs/doctoring/sandboxed-web-readiness-loopback-boundary.md) + — loopback-only web E2E readiness polling and APA 7th citations. diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a656b7bfe..c6471cbf93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -152,6 +152,12 @@ Semantic Versioning where the repository publishes a release. ### Fixed +- Web verification now checks services through local readiness addresses only. + Start the backend and frontend on this computer and use their local health + URLs when running the check. +- Review results now separate cosmetic notices from blocking failures. Open the + failure details and correct the requested issue before running the check + again. - Resolve Strix visibility from the trusted GitHub event for ordinary push, schedule, and pull-request runs, reserving API retries for cross-repository dispatches whose workflow token may not see the target repository. diff --git a/docs/adr/0004-sandboxed-web-readiness-loopback-boundary.md b/docs/adr/0004-sandboxed-web-readiness-loopback-boundary.md new file mode 100644 index 0000000000..c70e2713d3 --- /dev/null +++ b/docs/adr/0004-sandboxed-web-readiness-loopback-boundary.md @@ -0,0 +1,9 @@ +# ADR-0004: Sandboxed web readiness loopback boundary + +- Status: accepted +- Date: 2026-08-25 +- Scope: ContextualWisdomLab/.github control-plane E2E sandbox +- Decision: Poll `--backend-ready-url` and `--frontend-ready-url` only after the URL is proven to be HTTP(S) loopback. Accept literal `localhost` or a standard-library loopback address, resolve `localhost` and require every answer to be loopback, unwrap IPv4-mapped IPv6, reject userinfo and missing hosts, and keep redirects disabled. +- Ownership: `.github` owns the sandbox helper. Product repositories keep pointing readiness at their own loopback services. +- Figma File ID: N/A. This repository has no customer UI. +- Consequence: A review run cannot use the sandbox poller as an SSRF trampoline to metadata services or public hosts. Operators fix a rejected URL by pointing it at `127.0.0.1` or `::1`. Papers live in `docs/doctoring/sandboxed-web-readiness-loopback-boundary.md`. diff --git a/docs/doctoring/sandboxed-web-readiness-loopback-boundary.md b/docs/doctoring/sandboxed-web-readiness-loopback-boundary.md new file mode 100644 index 0000000000..65438e75d4 --- /dev/null +++ b/docs/doctoring/sandboxed-web-readiness-loopback-boundary.md @@ -0,0 +1,85 @@ +# Sandboxed web readiness loopback boundary + +## Decision + +`sandboxed_web_e2e.py` opens a readiness URL only after +`require_loopback_readiness_url` accepts it. The accepted destinations are +literal `localhost` (a trailing FQDN dot is stripped) or an address that +Python's standard-library `ipaddress` module classifies as loopback after +IPv4-mapped IPv6 addresses are unwrapped. Literal `localhost` is then +resolved; every A/AAAA answer must itself be loopback, so a poisoned hosts +file cannot smuggle a public address through the name allowlist. Redirects +remain disabled. + +This supports the complete IPv4 loopback block, including `127.0.0.2`, and +IPv6 `::1`. It rejects `0.0.0.0`, `::`, public hosts, `.localhost` +subdomains, cloud-metadata link-local addresses, missing hosts, and +userinfo-confused URLs such as `http://user@127.0.0.1/`. A mapped public +address such as `::ffff:8.8.8.8` cannot pass merely because it is IPv6. + +The boundary uses the standard library rather than a second address table. +It therefore follows the runtime's maintained special-purpose definitions and +keeps one fail-closed validation point before any network request. Do not add +individual non-loopback exceptions. + +This successor lands the same buyer-facing repair as ContextualWisdomLab/.github#1244 +on current `main` and keeps Strix classifier ownership out of the SSRF slice +(unlike ContextualWisdomLab/.github#1313). + +## Operator next action + +Point `--backend-ready-url` and `--frontend-ready-url` at the sandboxed +service on loopback. If readiness fails with `URL cannot target external +hostname`, replace the destination with `http://127.0.0.1:/...` or +`http://[::1]:/...` instead of opening the firewall or adding a +hostname exception. + +## Verification + +The regression exercises literal `localhost`, a trailing-dot `localhost.`, +`127.0.0.1`, another address in `127.0.0.0/8`, IPv6 `::1`, mapped loopback +`::ffff:127.0.0.1`, an unspecified address, a `.localhost` subdomain, a +public hostname, the common cloud metadata address, mapped public IPv6, +userinfo, a missing host, and poisoned localhost resolution (public A, +mapped public AAAA, empty answers, resolver errors, and non-IP answers). +The existing no-redirect test continues to prove that an allowed readiness +endpoint cannot redirect the poller across the boundary. + +```mermaid +flowchart TD + Url["Readiness URL"] + Scheme{"http or https?"} + Userinfo{"userinfo present?"} + Host{"loopback IP, or localhost whose every resolved answer is loopback?"} + Open["Poll with redirects disabled"] + Reject["Fail closed before any request"] + + Url --> Scheme + Scheme -->|"no"| Reject + Scheme -->|"yes"| Userinfo + Userinfo -->|"yes"| Reject + Userinfo -->|"no"| Host + Host -->|"no"| Reject + Host -->|"yes"| Open +``` + +## References + +Berners-Lee, T., Fielding, R., & Masinter, L. (2005). *Uniform Resource +Identifier (URI): Generic syntax* (RFC 3986). Internet Engineering Task +Force. https://doi.org/10.17487/RFC3986 + +Cotton, B., Vegoda, L., Bonica, R., & Haberman, B. (2013). *Special-purpose +IP address registries* (RFC 6890). Internet Engineering Task Force. +https://doi.org/10.17487/RFC6890 + +Internet Assigned Numbers Authority. (2026). *IANA IPv4 special-purpose +address registry*. Retrieved August 25, 2026, from +https://www.iana.org/assignments/iana-ipv4-special-registry/iana-ipv4-special-registry.xhtml + +OWASP Foundation. (n.d.). *Server-side request forgery prevention cheat +sheet*. Retrieved August 25, 2026, from +https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html + +Python Software Foundation. (2026). *ipaddress — IPv4/IPv6 manipulation +library*. https://docs.python.org/3/library/ipaddress.html diff --git a/scripts/ci/sandboxed_web_e2e.py b/scripts/ci/sandboxed_web_e2e.py index ae0c3105ac..978113e690 100644 --- a/scripts/ci/sandboxed_web_e2e.py +++ b/scripts/ci/sandboxed_web_e2e.py @@ -3,16 +3,19 @@ from __future__ import annotations import argparse +import ipaddress import json import os import signal import shutil import shlex +import socket import subprocess import sys import tempfile import time import urllib.error +import urllib.parse import urllib.request from collections.abc import Sequence from dataclasses import dataclass @@ -115,12 +118,61 @@ def start_service(label: str, command: str, cwd: Path, env: dict[str, str], logs return Service(label=label, command=command, process=process, log_path=log_path) +def _require_loopback_ip_text(ip_text: str, hostname: str) -> None: + """Reject a literal or resolved address that is not loopback.""" + try: + address = ipaddress.ip_address(ip_text) + except ValueError as exc: + raise ValueError(f"URL cannot target external hostname: {hostname}") from exc + if address.version == 6 and address.ipv4_mapped is not None: + address = address.ipv4_mapped + if not address.is_loopback: + raise ValueError(f"URL cannot target external hostname: {hostname}") + + +def _require_resolved_loopback_hostname(hostname: str) -> None: + """Resolve a literal localhost name and require every answer to be loopback.""" + try: + results = socket.getaddrinfo(hostname, None) + except OSError as exc: + raise ValueError(f"URL cannot target unresolved hostname: {hostname}") from exc + if not results: + raise ValueError(f"URL cannot target unresolved hostname: {hostname}") + for result in results: + _require_loopback_ip_text(result[4][0], hostname) + + +def require_loopback_readiness_url(url: str) -> None: + """Reject a readiness URL that is not a local loopback HTTP(S) target. + + Operators should point ``--backend-ready-url`` and ``--frontend-ready-url`` + at the sandboxed service itself. Public hosts, cloud metadata addresses, + unspecified bind addresses, DNS names other than literal ``localhost``, + and userinfo-confused URLs are rejected before any request is opened. + Literal ``localhost`` is resolved and every answer must be loopback, so a + poisoned hosts file cannot smuggle a public A/AAAA record through the + name allowlist. IPv4-mapped IPv6 addresses are unwrapped and re-checked + so ``::ffff:8.8.8.8`` cannot bypass the loopback rule. + """ + parsed = urllib.parse.urlparse(url) + if parsed.scheme.lower() not in {"http", "https"}: + raise ValueError(f"URL must start with http:// or https://, got: {url}") + if parsed.username or parsed.password: + raise ValueError("URL cannot include userinfo") + hostname = (parsed.hostname or "").lower().rstrip(".") + if not hostname: + raise ValueError("URL must include a loopback hostname") + if hostname == "localhost": + _require_resolved_loopback_hostname(hostname) + return + _require_loopback_ip_text(hostname, hostname) + + def wait_for_url(url: str, timeout: int, service: Service) -> bool: """Poll a readiness URL until it responds or the service exits.""" if not url: return True - if not (url.startswith("http://") or url.startswith("https://")): - raise ValueError(f"URL must start with http:// or https://, got: {url}") + require_loopback_readiness_url(url) deadline = time.monotonic() + timeout opener = urllib.request.build_opener(NoRedirectHandler()) while time.monotonic() < deadline: diff --git a/tests/test_sandboxed_web_e2e.py b/tests/test_sandboxed_web_e2e.py index 6e092c2930..39339a3165 100644 --- a/tests/test_sandboxed_web_e2e.py +++ b/tests/test_sandboxed_web_e2e.py @@ -105,13 +105,19 @@ def test_sandboxed_web_e2e_runs_services_and_does_not_mutate_source(tmp_path, ca def test_wait_helpers_and_service_cleanup_edges(monkeypatch, tmp_path): - """Small helper branches handle empty URLs, exited services, and hard cleanup.""" + """Small helper branches handle empty URLs, loopback readiness, and hard cleanup.""" exited = subprocess.Popen([sys.executable, "-c", ""], text=True) exited.wait(timeout=5) exited_service = sandboxed_web_e2e.Service("done", "true", exited, tmp_path / "missing.log") assert sandboxed_web_e2e.wait_for_url("", 1, exited_service) is True assert sandboxed_web_e2e.wait_for_url("http://127.0.0.1:1/", 1, exited_service) is False + assert sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) is False + assert sandboxed_web_e2e.wait_for_url("http://localhost./health", 1, exited_service) is False + assert sandboxed_web_e2e.wait_for_url("http://127.0.0.2:1/", 1, exited_service) is False + assert sandboxed_web_e2e.wait_for_url("http://[::1]:1/health", 1, exited_service) is False + assert sandboxed_web_e2e.wait_for_url("HTTP://[::ffff:127.0.0.1]:1/", 1, exited_service) is False + assert sandboxed_web_e2e.wait_for_url("https://127.0.0.1:1/", 1, exited_service) is False with pytest.raises(ValueError, match="URL must start with http:// or https://"): sandboxed_web_e2e.wait_for_url("file:///etc/passwd", 1, exited_service) sandboxed_web_e2e.stop_service(exited_service) @@ -228,6 +234,83 @@ def open(self, url, timeout): assert sandboxed_web_e2e.tail_text(log_path).splitlines()[0] == "line-10" +def test_wait_for_url_rejects_non_loopback_and_confused_deputy_targets(tmp_path): + """Readiness polling must fail closed on public, metadata, and userinfo targets.""" + exited = subprocess.Popen([sys.executable, "-c", ""], text=True) + exited.wait(timeout=5) + exited_service = sandboxed_web_e2e.Service("done", "true", exited, tmp_path / "missing.log") + + with pytest.raises(ValueError, match="URL cannot target external hostname: example\\.com"): + sandboxed_web_e2e.wait_for_url("http://example.com/health", 1, exited_service) + with pytest.raises(ValueError, match="URL cannot target external hostname: app\\.localhost"): + sandboxed_web_e2e.wait_for_url("http://app.localhost:8000/health", 1, exited_service) + with pytest.raises(ValueError, match="URL cannot target external hostname: 169\\.254\\.169\\.254"): + sandboxed_web_e2e.wait_for_url("http://169.254.169.254/latest/meta-data/", 1, exited_service) + with pytest.raises(ValueError, match="URL cannot target external hostname: 0\\.0\\.0\\.0"): + sandboxed_web_e2e.wait_for_url("http://0.0.0.0:8000/health", 1, exited_service) + with pytest.raises(ValueError, match="URL cannot target external hostname: ::"): + sandboxed_web_e2e.wait_for_url("http://[::]/", 1, exited_service) + with pytest.raises(ValueError, match="URL cannot target external hostname: ::ffff:8\\.8\\.8\\.8"): + sandboxed_web_e2e.wait_for_url("http://[::ffff:8.8.8.8]/", 1, exited_service) + with pytest.raises(ValueError, match="URL cannot include userinfo"): + sandboxed_web_e2e.wait_for_url("http://user@127.0.0.1/", 1, exited_service) + with pytest.raises(ValueError, match="URL cannot include userinfo"): + sandboxed_web_e2e.wait_for_url("http://:pass@127.0.0.1/", 1, exited_service) + with pytest.raises(ValueError, match="URL must include a loopback hostname"): + sandboxed_web_e2e.wait_for_url("http:///health", 1, exited_service) + sandboxed_web_e2e.stop_service(exited_service) + + +def test_localhost_resolution_must_stay_loopback(monkeypatch, tmp_path): + """Literal localhost is allowed only when every resolved address is loopback.""" + exited = subprocess.Popen([sys.executable, "-c", ""], text=True) + exited.wait(timeout=5) + exited_service = sandboxed_web_e2e.Service("done", "true", exited, tmp_path / "missing.log") + + monkeypatch.setattr( + sandboxed_web_e2e.socket, + "getaddrinfo", + lambda host, port: [(0, 0, 0, "", ("8.8.8.8", 0))], + ) + with pytest.raises(ValueError, match="URL cannot target external hostname: localhost"): + sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) + + monkeypatch.setattr(sandboxed_web_e2e.socket, "getaddrinfo", lambda host, port: []) + with pytest.raises(ValueError, match="URL cannot target unresolved hostname: localhost"): + sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) + + def _unresolved(host, port): + raise socket.gaierror("name not known") + + monkeypatch.setattr(sandboxed_web_e2e.socket, "getaddrinfo", _unresolved) + with pytest.raises(ValueError, match="URL cannot target unresolved hostname: localhost"): + sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) + + monkeypatch.setattr( + sandboxed_web_e2e.socket, + "getaddrinfo", + lambda host, port: [(0, 0, 0, "", ("not-an-ip", 0))], + ) + with pytest.raises(ValueError, match="URL cannot target external hostname: localhost"): + sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) + + monkeypatch.setattr( + sandboxed_web_e2e.socket, + "getaddrinfo", + lambda host, port: [(0, 0, 0, "", ("::ffff:8.8.8.8", 0))], + ) + with pytest.raises(ValueError, match="URL cannot target external hostname: localhost"): + sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) + + monkeypatch.setattr( + sandboxed_web_e2e.socket, + "getaddrinfo", + lambda host, port: [(0, 0, 0, "", ("::ffff:127.0.0.1", 0))], + ) + assert sandboxed_web_e2e.wait_for_url("http://localhost:1/", 1, exited_service) is False + sandboxed_web_e2e.stop_service(exited_service) + + def test_no_redirect_handler_raises_httperror_without_following(): """Readiness checks must raise HTTPError on redirects to prevent attacker-controlled internal URLs.""" import urllib.error