diff --git a/approval_engine/http_transport.py b/approval_engine/http_transport.py deleted file mode 100644 index 6f47ae5..0000000 --- a/approval_engine/http_transport.py +++ /dev/null @@ -1,71 +0,0 @@ -"""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 158f1f9..5c6ced7 100644 --- a/approval_engine/pep.py +++ b/approval_engine/pep.py @@ -3,18 +3,12 @@ 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, urlsplit -from urllib.request import Request +from urllib.parse import quote +from urllib.request import Request, urlopen from .binding import DIGEST_RE @@ -47,134 +41,28 @@ class ApprovalHTTPClient: token_file: str | Path, *, timeout_seconds: float = 3, - opener: Callable[..., Any] | None = None, + opener: Callable[..., Any] = urlopen, ) -> None: - 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: + if not base_url or not base_url.startswith(("http://", "https://")): 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: - 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) + 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") 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() - 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") + encoded = None if body is None else json.dumps(body).encode("utf-8") request = Request( - url, + self.base_url + path, data=encoded, method=method, headers={ @@ -197,14 +85,17 @@ class ApprovalHTTPClient: except Exception: pass raise ApprovalProtocolError(_status_message(status)) from None - except Exception: + except (URLError, TimeoutError, OSError): 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: - return self._decode(raw.decode("utf-8")) - except UnicodeDecodeError: - raise ApprovalProtocolError("approval endpoint returned invalid JSON") from None + 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 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 deleted file mode 100644 index eb067f6..0000000 --- a/docs/reviews/2026-10-11-confirmation-transport-repair-owner-review.md +++ /dev/null @@ -1,86 +0,0 @@ -# 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 9aef908..4e36bd5 100644 --- a/docs/task-execution-consumption-contract.md +++ b/docs/task-execution-consumption-contract.md @@ -141,51 +141,3 @@ 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 deleted file mode 100644 index c97a520..0000000 --- a/tests/test_confirmation_transport.py +++ /dev/null @@ -1,199 +0,0 @@ -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 fed3f83..63188e6 100644 --- a/tests/test_execution_confirmation.py +++ b/tests/test_execution_confirmation.py @@ -19,7 +19,6 @@ 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 a45b2e0..5a2659c 100644 --- a/tests/test_pep.py +++ b/tests/test_pep.py @@ -58,7 +58,6 @@ 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 2fa4851..7e8ead6 100644 --- a/workplans/ADHOC-2026-10-11.md +++ b/workplans/ADHOC-2026-10-11.md @@ -9,7 +9,6 @@ owner: codex created: "2026-10-11" updated: "2026-10-11" flavor: implementation -state_hub_workstream_id: "77fca3cc-64dc-5c31-915e-999dde18eee5" --- # Strict execution consume-confirmation client source preparation @@ -26,7 +25,6 @@ not promote or implement APPROVAL-WP-0002's residual adoption obligations. id: APPROVAL-WP-ADHOC-2026-10-11-T01 status: done priority: medium -state_hub_task_id: "57b7db4f-87d5-503d-974a-af21c05cde21" ``` Completed source preparation: approval_engine/execution_confirmation.py adds @@ -84,73 +82,3 @@ transport admission. Fresh claim/PDP validation, returned original consumption correspondence and a separate one-shot journal remain caller obligations. No grant, native acceptance, deployment, human approval, residual promotion or full-adoption closure was recorded. - - -## Repair fixed-origin confirmation transport after synthetic credential redirect - -```task -id: APPROVAL-WP-ADHOC-2026-10-11-T02 -status: done -priority: high -``` - -New bounded source demand, distinct from completed T01: the Custodian synthetic -two-origin counterexample proved the inherited ApprovalHTTPClient default could -follow cross-origin redirects, forward Authorization and accept redirected JSON. -Original T01 source review and its outcome are preserved; they did not prove this -transport boundary. The failure receipt is the-custodian/docs/evidence/ -2026-10-10-security-consolidation/approval-confirmation-transport-counterexample.json. - -Completed staged source preparation adds fixed-provider isolated default HTTP -transport with no environment proxies or redirects, exact 200/JSON/content/body -limits, duplicate/nonfinite JSON refusal, closed errors and bounded whole-network -request deadline including exceptional close. Process isolation retains threaded -SDK support without signals or multiprocessing main-guard requirements. Protected -local credential reads refuse symlink components, hardlinks, public/group-writable -files, unknown owners, special files and excessive/invalid contents; token -rotation through reread remains supported. Explicit test-opener injection is -retained as an owner-controlled seam, not an admitted production transport. - -INTENT basis: deterministic authentic approval facts, exact binding and fail-closed -consumption evidence in Purpose, Claim Shape and Consumption Ordering. This -source repair does not add a policy decision, grant, caller attestation, execution -journal or privileged replay service. - -Validation: 270 staged full tests passed, with the existing JWT test-key warning; -51 new transport cases include cross-origin GET/POST redirects (zero second-origin -requests/headers), ignored environment proxies, status/content/size/JSON/error -refusals, total deadline under slow headers/dribbled body/error body from a worker -thread, safe exceptional close and credential-file guards. Existing PEP and exact -consumption tests remain covered after their synthetic token fixture modes become -explicitly private. Native credentials, custody, OpenBao or production APIs were -not accessed. Immutable staged hashes are in normalized-source-manifest.json -and SHA256SUMS; source patch checks against the current owner checkout. - -This completed task records local source preparation only. Formal updated owner -review must follow persistence of T02 in the actual owner checkout. No formal -updated review is written at this preparation stage. Actual mediator read/consume -identity, credential custody, transport/server identity and ingress admission are -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.