From b428ffecb1e26fededc4b29e84aff3824fd19a47 Mon Sep 17 00:00:00 2001 From: tegwick Date: Sun, 11 Oct 2026 16:45:18 +0200 Subject: [PATCH] Bound approval transport and refuse redirects and proxies Assistant: codex Assistant-Model: gpt-6.1-sol Assistant-Session: 01a126e1-76f8-7643-8cf3-e93336539ade --- approval_engine/http_transport.py | 71 +++++++ approval_engine/pep.py | 141 +++++++++++-- ...firmation-transport-repair-owner-review.md | 86 ++++++++ docs/task-execution-consumption-contract.md | 48 +++++ tests/test_confirmation_transport.py | 199 ++++++++++++++++++ tests/test_execution_confirmation.py | 1 + tests/test_pep.py | 1 + workplans/ADHOC-2026-10-11.md | 21 ++ 8 files changed, 552 insertions(+), 16 deletions(-) create mode 100644 approval_engine/http_transport.py create mode 100644 docs/reviews/2026-10-11-confirmation-transport-repair-owner-review.md create mode 100644 tests/test_confirmation_transport.py diff --git a/approval_engine/http_transport.py b/approval_engine/http_transport.py new file mode 100644 index 0000000..6f47ae5 --- /dev/null +++ b/approval_engine/http_transport.py @@ -0,0 +1,71 @@ +"""Isolated fixed-origin HTTP transport. No credentials in argv or diagnostics. + +Invoked by the SDK through a bounded subprocess, not as an application command. +Isolation allows the parent to terminate DNS, reads and exceptional close without +signals in an application's main thread or mandatory multiprocessing main guards. +""" + +import json +import sys +from urllib.error import HTTPError +from urllib.request import HTTPRedirectHandler, ProxyHandler, Request, build_opener + +MAX_BODY = 256 * 1024 + + +class NoRedirect(HTTPRedirectHandler): + def redirect_request(self, req, fp, code, msg, headers, newurl): + raise HTTPError(req.full_url, code, "redirect refused", headers, fp) + + +def fetch(data): + request = Request(data["url"], data=data["body"].encode("utf-8") if data["body"] is not None else None, + method=data["method"], headers={"Authorization": "Bearer " + data["token"], + "Content-Type": "application/json", "Accept": "application/json"}) + opener = build_opener(ProxyHandler({}), NoRedirect()) + response = None + try: + response = opener.open(request, timeout=data["timeout"]) + if response.geturl() != data["url"] or response.getcode() != 200: + return {"error": "status", "status": response.getcode()} + types = response.headers.get_all("Content-Type", []) + if len(types) != 1 or response.headers.get_content_type() != "application/json": + return {"error": "content"} + charset = response.headers.get_content_charset() + if charset not in (None, "utf-8") or response.headers.get("Content-Encoding", "identity").lower() != "identity": + return {"error": "content"} + lengths = response.headers.get_all("Content-Length", []) + if len(lengths) > 1 or (lengths and (not lengths[0].isdigit() or int(lengths[0]) > MAX_BODY)): + return {"error": "content"} + raw = response.read(MAX_BODY + 1) + if len(raw) > MAX_BODY or (lengths and len(raw) != int(lengths[0])): + return {"error": "content"} + return {"body": raw.decode("utf-8")} + except HTTPError as exc: + # Never read a refused/redirect/error body; even close is parent-bounded. + try: + exc.close() + except Exception: + pass + return {"error": "status", "status": int(exc.code)} + except Exception: + return {"error": "unavailable"} + finally: + if response is not None: + try: + response.close() + except Exception: + pass + + +def main(): + try: + data = json.loads(sys.stdin.buffer.read(MAX_BODY + 1)) + result = fetch(data) + except Exception: + result = {"error": "unavailable"} + sys.stdout.write(json.dumps(result, allow_nan=False)) + + +if __name__ == "__main__": + main() diff --git a/approval_engine/pep.py b/approval_engine/pep.py index 5c6ced7..158f1f9 100644 --- a/approval_engine/pep.py +++ b/approval_engine/pep.py @@ -3,12 +3,18 @@ from __future__ import annotations import json +import math +import os import re +import stat +import subprocess +import sys +import time from pathlib import Path from typing import Any, Callable from urllib.error import HTTPError, URLError -from urllib.parse import quote -from urllib.request import Request, urlopen +from urllib.parse import quote, urlsplit +from urllib.request import Request from .binding import DIGEST_RE @@ -41,28 +47,134 @@ class ApprovalHTTPClient: token_file: str | Path, *, timeout_seconds: float = 3, - opener: Callable[..., Any] = urlopen, + opener: Callable[..., Any] | None = None, ) -> None: - if not base_url or not base_url.startswith(("http://", "https://")): + try: + parsed = urlsplit(base_url) + valid = (parsed.scheme in {"http", "https"} and parsed.hostname and + parsed.port != 0 and not parsed.username and not parsed.password and + not parsed.query and not parsed.fragment and + not any(ch.isspace() or ord(ch) < 32 for ch in base_url)) + except (TypeError, ValueError): + valid = False + if not valid: raise ApprovalProtocolError("approval-engine URL is missing or invalid") + if isinstance(timeout_seconds, bool) or not isinstance(timeout_seconds, (int, float)) or not math.isfinite(timeout_seconds) or not 0 < timeout_seconds <= 60: + raise ApprovalProtocolError("approval timeout must be finite and bounded") self.base_url = base_url.rstrip("/") self.token_file = Path(token_file) self.timeout_seconds = timeout_seconds self.opener = opener def _token(self) -> str: - token = self.token_file.read_text(encoding="utf-8").strip() - if not token or any(ch.isspace() for ch in token): - raise ApprovalProtocolError("approval credential is unavailable") + fd = None + directory = None + try: + parts = self.token_file.absolute().parts + if ".." in parts: + raise ValueError("unsafe credential path") + directory = os.open(parts[0], os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW) + for part in parts[1:-1]: + child = os.open(part, os.O_RDONLY | os.O_DIRECTORY | os.O_NOFOLLOW, dir_fd=directory) + os.close(directory) + directory = child + fd = os.open(parts[-1], os.O_RDONLY | os.O_NOFOLLOW | os.O_NONBLOCK, dir_fd=directory) + info = os.fstat(fd) + if (not stat.S_ISREG(info.st_mode) or info.st_uid not in {0, os.geteuid()} + or info.st_mode & 0o037 or info.st_size > 16384 or info.st_nlink != 1): + raise ValueError("unsafe credential file") + raw = os.read(fd, 16385) + token = raw.decode("utf-8").strip() + if len(raw) > 16384 or not token or any(ch.isspace() or ord(ch) < 33 or ord(ch) > 126 for ch in token): + raise ValueError("invalid credential") + except (OSError, UnicodeError, ValueError): + raise ApprovalProtocolError("approval credential is unavailable") from None + finally: + if fd is not None: + os.close(fd) + if directory is not None: + os.close(directory) return token + @staticmethod + def _decode(raw: str) -> dict[str, Any]: + def pairs(items): + result = {} + for key, value in items: + if key in result: + raise ValueError("duplicate JSON key") + result[key] = value + return result + def constant(_value): + raise ValueError("non-finite JSON value") + def number(value): + parsed = float(value) + if not math.isfinite(parsed): + raise ValueError("non-finite JSON number") + return parsed + try: + result = json.loads(raw, object_pairs_hook=pairs, parse_constant=constant, parse_float=number) + except (ValueError, RecursionError): + raise ApprovalProtocolError("approval endpoint returned invalid JSON") from None + if not isinstance(result, dict): + raise ApprovalProtocolError("approval endpoint returned invalid payload") + return result + + def _isolated_request(self, method, url, body, token, deadline): + payload = json.dumps({"method": method, "url": url, "body": body, + "token": token, "timeout": self.timeout_seconds}, allow_nan=False).encode() + if len(payload) > _MAX_BODY: + raise ApprovalProtocolError("approval request is too large") + remaining = deadline - time.monotonic() + if remaining <= 0: + raise ApprovalProtocolError("approval transport deadline exceeded") + try: + completed = subprocess.run( + [sys.executable, "-I", str(Path(__file__).with_name("http_transport.py"))], + input=payload, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, + timeout=remaining, check=False, env={}, + ) + except subprocess.TimeoutExpired: + raise ApprovalProtocolError("approval transport deadline exceeded") from None + except OSError: + raise ApprovalProtocolError("approval-engine unreachable") from None + if completed.returncode != 0 or len(completed.stdout) > 2 * _MAX_BODY: + raise ApprovalProtocolError("approval transport failed") + try: + result = json.loads(completed.stdout) + except (ValueError, UnicodeError): + raise ApprovalProtocolError("approval transport failed") from None + if not isinstance(result, dict): + raise ApprovalProtocolError("approval transport failed") + if result.get("error") == "status" and type(result.get("status")) is int: + raise ApprovalProtocolError(_status_message(result["status"])) + if result.get("error") == "content": + raise ApprovalProtocolError("approval endpoint returned invalid content") + if not isinstance(result.get("body"), str): + raise ApprovalProtocolError("approval-engine unreachable") + return self._decode(result["body"]) + def _request( self, method: str, path: str, body: dict[str, Any] | None = None ) -> dict[str, Any]: + deadline = time.monotonic() + self.timeout_seconds + if not isinstance(path, str) or not path.startswith("/") or path.startswith("//") or any(ch.isspace() or ord(ch) < 32 for ch in path): + raise ApprovalProtocolError("approval request path is invalid") + url = self.base_url + path + if urlsplit(url)[:2] != urlsplit(self.base_url)[:2] or urlsplit(url).fragment: + raise ApprovalProtocolError("approval request origin is invalid") token = self._token() - encoded = None if body is None else json.dumps(body).encode("utf-8") + try: + text_body = None if body is None else json.dumps(body, allow_nan=False) + except (ValueError, TypeError): + raise ApprovalProtocolError("approval request is invalid") from None + if self.opener is None: + return self._isolated_request(method, url, text_body, token, deadline) + # Explicit owner-controlled injection for test adapters. It does not + # establish production transport admission or an isolated deadline. + encoded = None if text_body is None else text_body.encode("utf-8") request = Request( - self.base_url + path, + url, data=encoded, method=method, headers={ @@ -85,17 +197,14 @@ class ApprovalHTTPClient: except Exception: pass raise ApprovalProtocolError(_status_message(status)) from None - except (URLError, TimeoutError, OSError): + except Exception: raise ApprovalProtocolError("approval-engine unreachable") from None if status != 200 or len(raw) > _MAX_BODY: raise ApprovalProtocolError(_status_message(status if status != 200 else 502)) try: - result = json.loads(raw.decode("utf-8")) - except (UnicodeDecodeError, json.JSONDecodeError) as exc: - raise ApprovalProtocolError("approval endpoint returned invalid JSON") from exc - if not isinstance(result, dict): - raise ApprovalProtocolError("approval endpoint returned invalid payload") - return result + return self._decode(raw.decode("utf-8")) + except UnicodeDecodeError: + raise ApprovalProtocolError("approval endpoint returned invalid JSON") from None def claim(self, approval_id: str) -> dict[str, Any]: if not _ID_RE.fullmatch(approval_id): diff --git a/docs/reviews/2026-10-11-confirmation-transport-repair-owner-review.md b/docs/reviews/2026-10-11-confirmation-transport-repair-owner-review.md new file mode 100644 index 0000000..eb067f6 --- /dev/null +++ b/docs/reviews/2026-10-11-confirmation-transport-repair-owner-review.md @@ -0,0 +1,86 @@ +# Approval Engine confirmation transport repair owner review + +```yaml +reviewer: approval-engine-owner (agent approval-engine, codex) +decision: approved +comment: >- + Approved for bounded provider source repair only, under the persisted + APPROVAL-WP-ADHOC-2026-10-11-T02 completed preparation record in + 2eb045bfbfab08ce3a642d6f0ee3800af1151873, work record SHA256 + 3e09a565dca3b7ffcff8bed209e39f56a158dcaa63945007a096f244b9d86088. + Review performed from approval-engine's actual checkout with its AGENTS.md + and INTENT.md loaded. Purpose, Claim Shape and Consumption Ordering require + authenticated exact approval facts and fail-closed use evidence. The unchanged + source revisions listed below fix default redirect/proxy/response/timeout and + credential-file boundaries without adding PDP semantics or execution authority. + No other owner role is reviewed. Actual admitted mediator approval:read/consume + identity, custody, server/transport origin, ingress or native request acceptance + remains unproved. This review approves no credential, grant, privileged replay, + deployment, production execution, T-06 proof or adoption closure. +``` + +Exact reviewed source revisions (SHA256): + +- approval_engine/pep.py: + 4f82931accc27cf6eb03e5266f644d117bb1bd954c4683145a630878b3c33e24 +- approval_engine/http_transport.py: + ffd948f71fe12873bfc321ad3ea363065146c0cb1607b1d93e74e03875b9bfdb +- tests/test_pep.py: + ef063f10d76c79413e2e3ee932759d0364a6b31e9bae32a0aaa3e951e5e90f88 +- tests/test_execution_confirmation.py: + c8c7565e52af40fc4f49a1c29705dc19be1c1d4924ca92cf72fcd884beba09e9 +- tests/test_confirmation_transport.py: + b59e5f8014d89523675734072fbc3344cb135fc1d8febc57023631733aeefd02 +- docs/task-execution-consumption-contract.md: + f70f764fc67d1f16e87a59ec73de8f4b4adac3985f68a1234cfd0e609d94823b + +The original T01 acknowledgement and Custodian redirect counterexample are +preserved as history. The original inherited urllib transport did forward bearer +authentication across an origin change and accept redirected JSON; this repair +does not erase that failed outcome or imply an earlier native route was safe. + +The default SDK transport now invokes a fixed provider source child with -I and +an empty environment. Bearer material is confined to private process pipes, +never argv or diagnostics. ProxyHandler({}) rejects environment proxy routing; +NoRedirect refuses every redirect without a second request. Returned origin and +status must match the original request and 200; content/encoding/length limits +and strict duplicate-key/nonfinite/UTF-8/object JSON parsing refuse ambiguity. +No error body is needed or exposed. The parent's bounded subprocess call can +terminate DNS/header/read/error-close hangs without signals in an application's +main thread. The fixed child does not fork descendants or retry. Threaded SDK +invocation remains supported. This is not a general sandbox or a custody-isolated +execution process: it still runs as its owner and creates no permission. + +Credential reread uses non-following descriptor traversal and a bounded regular +local file read. Symlink components/final hardlinks, special files, public or +group-writable modes, unrelated ownership, oversized and malformed tokens refuse. +Current/root-owned protected group-readable files remain supported. A projected +symlink layout must be explicitly corrected by its custody owner, never followed +or replaced by an inferred provider. Source validation does not admit any real +credential path, and the subprocess timeout is the network-child bound, not a +claim that arbitrary remote filesystems or operating-system scheduling cannot +block outside that child. + +Explicit opener injection remains a trusted owner test-adapter seam. It does not +provide the default child's no-redirect/no-proxy/content/deadline guarantees. +An injected urllib.urlopen must not be admitted for production authoritative +confirmation; caller-selected transport is unacceptable. HTTP support remains +for separately reviewed private estate origins and does not attest their actual +identity or substitute for a TLS/ingress/native reachability receipt. + +Validation evidence for the exact hashes above: 51 new synthetic transport cases +and 270 staged full tests passed; only the existing JWT test-key warning remains. +Cross-origin GET/POST 301/302/303/307/308 received zero requests or bearer headers +at the second server; environment proxies received none; slow headers, dribbled +bodies and error bodies stopped within the local worker-thread test budget; +status/content/size/JSON/error and credential guards refused. Exceptional adapter +close raised only a closed provider error. These tests establish local source +behavior, not deployment or an actual admitted mediator runtime. + +Before any protected effect, the independently admitted mediator must validate +a fresh exact approval claim and Access decision, match the returned original +consumption tuple, and persist confirmation in a separate live one-shot journal. +Timeouts remain unknown and never authorize retry. Idempotent historical use +never permits a second execution; original approval/PDP/operation deadlines +remain effective. No consumer flag, source hash, local receipt or this review +attests authority. Existing residual/native/adoption task statuses are unchanged. diff --git a/docs/task-execution-consumption-contract.md b/docs/task-execution-consumption-contract.md index 4e36bd5..9aef908 100644 --- a/docs/task-execution-consumption-contract.md +++ b/docs/task-execution-consumption-contract.md @@ -141,3 +141,51 @@ change, wrong act/target/actor/principal/purpose/digest/decision refusal, strict booleans/time, denied read, filtered evidence and idempotency after expiry with terminal revoke/supersede refusal. Staged full suite: 219 passed, one existing JWT short-test-key warning. No production HTTP/DB/OpenBao call or native action. + + +## Transport repair proposal after local redirect counterexample + +The original source review above does not attest the inherited transport's +authoritative reachability. A later synthetic two-origin counterexample proved +that urllib's previous default followed a redirect, forwarded Authorization and +accepted its JSON. This is retained as a failed transport outcome, not rewritten +into a success. Bounded source repair is a new demand under +APPROVAL-WP-ADHOC-2026-10-11-T02, without residual promotion or native admission. + +The staged default ApprovalHTTPClient transport uses the fixed provider-owned +http_transport.py child via the current Python executable with -I. Request data, +including bearer material, travels only through private process pipes, never +argv, logs or diagnostic output. The child receives an empty environment; environment proxy settings are ignored, every +redirect refuses without issuing another request, status must be exactly 200, +Content-Type must be a single application/json with UTF-8 (or no charset), +compressed/oversized/inconsistent responses refuse, and duplicate-key/non-finite/ +malformed/non-object JSON refuses. Backend/error bodies are not read or exposed. +Public errors contain closed provider messages and numeric status only. + +The parent's subprocess timeout covers DNS/connect/TLS/header/body/error-close +and process cleanup for the fixed child, which has no retry or subprocess +descendants. It avoids SIGALRM and multiprocessing-spawn main-guard requirements +so default SDK calls remain usable in worker threads and ordinary scripts. +The token is read from a bounded local regular file using nonblocking/no-follow +descriptor traversal: no symlink component, final hardlink, other-readable or +group-writable mode, non-current/non-root owner, FIFO/directory or oversized +credential is accepted. Restrictive current/root-owned group-readable files +(such as 0440) are permitted; unknown projected/symlink credential layouts must +be corrected by the custody owner, never silently followed or substituted. +Only ASCII non-whitespace bearer content is admitted; credential errors do not +republish paths or values. The file is reopened every request, retaining rotation. + +The explicit opener injection is retained solely as a trusted owner test-adapter +seam; it does not inherit isolated/default transport admission or total deadline +guarantees. Supplying urllib.urlopen as an injected opener recreates its own +redirect/proxy behavior and must not be admitted as production execution +confirmation. Native mediator implementation must use this fixed default or a +separately reviewed equivalent; caller-selected transport code is unacceptable. +The default code still supports HTTP for reviewed private estate routes; source +validation does not prove TLS, the actual server origin, deployed custody/read +scope or ingress admission. No native identity/profile is widened. + +After a timeout/refusal, consume remains potentially unknown and cannot be +retried to infer absence. Fresh exact claim/PDP validation, returned original +consumption correspondence and separate one-shot journal remain required. +This is a transport repair only, never an execution authority or replay service. diff --git a/tests/test_confirmation_transport.py b/tests/test_confirmation_transport.py new file mode 100644 index 0000000..c97a520 --- /dev/null +++ b/tests/test_confirmation_transport.py @@ -0,0 +1,199 @@ +from contextlib import contextmanager +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +import os +from pathlib import Path +import threading +import time + +import pytest + +from approval_engine.execution_confirmation import ExecutionConfirmationClient +from approval_engine.pep import ApprovalHTTPClient, ApprovalProtocolError + + +@contextmanager +def server(handle): + class Handler(BaseHTTPRequestHandler): + def do_GET(self): + handle(self) + def do_POST(self): + handle(self) + def log_message(self, *args): + pass + http = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + worker = threading.Thread(target=http.serve_forever, daemon=True) + worker.start() + try: + yield f"http://127.0.0.1:{http.server_port}" + finally: + http.shutdown() + http.server_close() + worker.join() + + +def credential(tmp_path): + path = tmp_path / "token" + path.write_text("synthetic-test-only") + path.chmod(0o600) + return path + + +def reply(req, body=b'{"ok":true}', content="application/json", status=200): + req.send_response(status) + req.send_header("Content-Type", content) + req.send_header("Content-Length", str(len(body))) + req.end_headers() + try: + req.wfile.write(body) + except (BrokenPipeError, ConnectionResetError): + pass + + +@pytest.mark.parametrize("status", [301, 302, 303, 307, 308]) +@pytest.mark.parametrize("method", ["GET", "POST"]) +def test_default_transport_never_forwards_redirect_credentials(tmp_path, status, method): + received = [] + def second(req): + received.append(req.headers.get("Authorization")) + reply(req) + with server(second) as foreign: + def first(req): + req.send_response(status) + req.send_header("Location", foreign + "/stolen") + req.end_headers() + with server(first) as origin: + c = ExecutionConfirmationClient(origin, credential(tmp_path)) + with pytest.raises(ApprovalProtocolError): + c._request(method, "/v1/approvals/synthetic", {} if method == "POST" else None) + assert received == [] + + +def test_no_environment_proxy_receives_bearer(tmp_path, monkeypatch): + received = [] + with server(lambda req: (received.append(req.headers.get("Authorization")), reply(req))) as proxy: + for name in ("HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", "http_proxy", "https_proxy", "all_proxy"): + monkeypatch.setenv(name, proxy) + monkeypatch.setenv("NO_PROXY", "") + monkeypatch.setenv("no_proxy", "") + with server(reply) as origin: + assert ApprovalHTTPClient(origin, credential(tmp_path)).claim("synthetic") == {"ok": True} + assert received == [] + + +@pytest.mark.parametrize("body,content", [ + (b'{"ok":true,"ok":false}', "application/json"), + (b'{"nested":{"ok":true,"ok":false}}', "application/json"), + (b'{"value":NaN}', "application/json"), (b'{"value":Infinity}', "application/json"), + (b'{"value":1e9999}', "application/json"), + (b'{"ok":true}', "text/html"), (b'{"ok":true}', "application/json; charset=latin1"), + (b'\xff', "application/json"), (b'{', "application/json"), + (b'[]', "application/json"), (b' ' * (256 * 1024 + 1), "application/json"), +]) +def test_oversized_or_malformed_content_refuses(tmp_path, body, content): + with server(lambda req: reply(req, body, content)) as origin: + with pytest.raises(ApprovalProtocolError) as error: + ApprovalHTTPClient(origin, credential(tmp_path)).claim("synthetic") + assert "synthetic-test-only" not in str(error.value) + assert repr(body) not in str(error.value) + + +@pytest.mark.parametrize("status", [201, 204, 401, 403, 404, 409, 500, 503]) +def test_non_200_has_safe_closed_error_without_error_body(tmp_path, status): + with server(lambda req: reply(req, b'private error details', status=status)) as origin: + with pytest.raises(ApprovalProtocolError) as error: + ApprovalHTTPClient(origin, credential(tmp_path)).claim("synthetic") + assert "private" not in str(error.value) + + +@pytest.mark.parametrize("phase", ["headers", "body", "errorbody"]) +def test_entire_transport_deadline_in_worker_thread(tmp_path, phase): + arrived = threading.Event() + def slow(req): + arrived.set() + if phase == "headers": + time.sleep(1.5) + req.send_response(500 if phase == "errorbody" else 200) + req.send_header("Content-Type", "application/json") + req.send_header("Content-Length", "100") + req.end_headers() + if phase != "errorbody": + for _ in range(30): + try: + req.wfile.write(b" ") + req.wfile.flush() + except (BrokenPipeError, ConnectionResetError): + break + time.sleep(0.1) + with server(slow) as origin: + elapsed = [] + errors = [] + def caller(): + start = time.monotonic() + try: + ApprovalHTTPClient(origin, credential(tmp_path), timeout_seconds=0.4).claim("synthetic") + except ApprovalProtocolError as exc: + errors.append(str(exc)) + elapsed.append(time.monotonic() - start) + worker = threading.Thread(target=caller) + worker.start() + worker.join(timeout=1.2) + assert not worker.is_alive() + assert arrived.is_set() + assert len(errors) == 1 + assert elapsed[0] < 1.2 + + +@pytest.mark.parametrize("kind", ["symlink", "fifo", "directory", "public", "group_write", "oversized", "malformed", "hardlink", "missing"]) +def test_credential_file_guards_refuse_before_network(tmp_path, kind): + path = credential(tmp_path) + if kind in {"symlink", "fifo", "directory", "missing"}: + path.unlink() + if kind == "symlink": + other = tmp_path / "other" + other.write_text("synthetic-test-only") + path.symlink_to(other) + elif kind == "fifo": + os.mkfifo(path, 0o600) + elif kind == "directory": + path.mkdir() + elif kind == "public": + path.chmod(0o644) + elif kind == "group_write": + path.chmod(0o660) + elif kind == "oversized": + path.write_text("x" * 16385) + elif kind == "malformed": + path.write_bytes(b'\xff') + elif kind == "hardlink": + os.link(path, tmp_path / "linked") + with pytest.raises(ApprovalProtocolError, match="credential is unavailable"): + ApprovalHTTPClient("http://127.0.0.1:1", path).claim("synthetic") + + +@pytest.mark.parametrize("url", ["http://user:password@host", "http://host?target=x", "http://host#target", " http://host", "http://host:0", "http://host:wrong", "ftp://host"]) +def test_invalid_origin_refuses(tmp_path, url): + with pytest.raises(ApprovalProtocolError): + ApprovalHTTPClient(url, credential(tmp_path)) + + +def test_safe_injected_opener_close_failure_is_closed(tmp_path): + class Response: + def getcode(self): + return 200 + def read(self, size): + return b'{"ok":true}' + def close(self): + raise RuntimeError("synthetic credential-bearing exception") + with pytest.raises(ApprovalProtocolError) as error: + ApprovalHTTPClient("http://fixed-origin", credential(tmp_path), opener=lambda *_a, **_k: Response()).claim("synthetic") + assert "credential-bearing" not in str(error.value) + + +def test_symlinked_credential_parent_refuses(tmp_path): + real = tmp_path / "real" + real.mkdir() + token = credential(real) + link = tmp_path / "link" + link.symlink_to(real, target_is_directory=True) + with pytest.raises(ApprovalProtocolError, match="credential is unavailable"): + ApprovalHTTPClient("http://127.0.0.1:1", link / token.name).claim("synthetic") diff --git a/tests/test_execution_confirmation.py b/tests/test_execution_confirmation.py index 63188e6..fed3f83 100644 --- a/tests/test_execution_confirmation.py +++ b/tests/test_execution_confirmation.py @@ -19,6 +19,7 @@ FREE = "sha256:" + "cd" * 32 def client(tmp_path, opener): token = tmp_path / "test-token" token.write_text("test-token") + token.chmod(0o600) return ExecutionConfirmationClient("http://approval-engine", token, opener=opener) diff --git a/tests/test_pep.py b/tests/test_pep.py index 5a2659c..a45b2e0 100644 --- a/tests/test_pep.py +++ b/tests/test_pep.py @@ -58,6 +58,7 @@ class Response: def _token(tmp_path, value="test-token"): path = tmp_path / "token" path.write_text(value) + path.chmod(0o600) return path diff --git a/workplans/ADHOC-2026-10-11.md b/workplans/ADHOC-2026-10-11.md index 305e835..2fa4851 100644 --- a/workplans/ADHOC-2026-10-11.md +++ b/workplans/ADHOC-2026-10-11.md @@ -133,3 +133,24 @@ identity, credential custody, transport/server identity and ingress admission ar unproved. Fresh claim/PDP validation, returned original consumption tuple and a separate live one-shot execution journal remain mandatory. Existing adoption/ native obligations retain their owners/statuses; no residual is promoted. + + +## Formal T02 transport source review after persisted preparation + +The approval-engine owner worker reviewed the exact unchanged T02 staged sources +from the actual owner checkout after commit +2eb045bfbfab08ce3a642d6f0ee3800af1151873 persisted its completed source-preparation +anchor (SHA256 3e09a565dca3b7ffcff8bed209e39f56a158dcaa63945007a096f244b9d86088). +Decision: approved for bounded source repair only. Review entry: +`docs/reviews/2026-10-11-confirmation-transport-repair-owner-review.md`. +The original T01 review, redirect failure, T02 preparation outcome, task statuses +and generated identifiers remain unchanged; this is a refinement of the same +bounded repair, not another claimed success. + +Exact reviewed sources/tests/contract and limitations are pinned in that entry. +Default transport is the reviewed fixed child; injected opener remains solely +an owner-controlled test seam and is not admitted authoritative transport. +No review establishes actual credential path, mediator scope/custody, server +identity/ingress, deployment, native effect, new grant, residual promotion or +adoption closure. Fresh claim/PDP plus original exact consumption and a separate +one-shot journal remain mandatory.