diff --git a/devlog/_plan/260928_remote_thread_provider_policy/000_plan.md b/devlog/_plan/260928_remote_thread_provider_policy/000_plan.md new file mode 100644 index 00000000000..530fa2bd582 --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/000_plan.md @@ -0,0 +1,32 @@ +# 5848: native remote-list provider policy before a backend relay + +Status: **proposal and executable specifications only; no production fix**. +Date: 2026-09-28. + +Related: [#5848](https://github.com/lidge-jun/opencodex/issues/5848), +[duplicate #5906](https://github.com/lidge-jun/opencodex/issues/5906), +[upstream #48358](https://github.com/openai/codex/issues/48358). + +## Proposed decision + +Prefer a narrowly scoped, operator-opt-in policy in **native Codex app-server** +for remote `thread/list` requests that do not supply a provider array. Keep the +normal default-provider behavior, all existing authorization, and history intact. +The mobile client's explicit all-provider request remains the simplest upstream +client correction. Compare both with the previously tested local backend relay; +do not turn that experimental relay into an installed feature in this PR. + +The current OpenCodex runtime and [ADR-5848](../../../structure/decisions/ADR-5848-provider-table-remote-history-visibility.md) +remain unchanged. This proposal does **not** close #5848. + +## Work remaining + +- [x] Identify the native remote-connection boundary and existing provider predicate. +- [x] Specify precedence and test the native-policy proposal offline. +- [x] Retain and rerun the isolated loopback relay alternative. +- [ ] Obtain upstream agreement on the native configuration/API contract. +- [ ] Implement config/schema, trusted-origin plumbing, and Rust regressions upstream. +- [ ] Verify native pagination, managed policy, account changes, and actual mobile resume. +- [ ] Establish released-version/capability detection before adding any OpenCodex toggle. + +See [design](010_design.md) and [verification](020_verification.md). diff --git a/devlog/_plan/260928_remote_thread_provider_policy/010_design.md b/devlog/_plan/260928_remote_thread_provider_policy/010_design.md new file mode 100644 index 00000000000..a0f8ca22474 --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/010_design.md @@ -0,0 +1,134 @@ +# Design comparison and proposed native contract + +This is an RFC, not an accepted architecture change or a released setting. +`probes/native_policy.py` is an executable specification, **not native Rust code**. +No probe is imported by OpenCodex or registered in its runtime/CLI/test suite. + +## Evidence and scope + +The source baseline is OpenAI Codex +`1cc7e2361237ce7244430ee1d581c77f95c57ac8` and OpenCodex `dev` +`eb7f0f0970c2298f8b2d66d170c4d4be869f301b`. + +- [Native list predicate](https://github.com/openai/codex/blob/1cc7e2361237ce7244430ee1d581c77f95c57ac8/codex-rs/app-server/src/request_processors/thread_processor.rs#L5456-L5530): explicit nonempty arrays filter to those ids; `[]` removes the provider predicate; omission defaults to the configured provider, except for related-thread queries. +- [Remote connection origin](https://github.com/openai/codex/blob/1cc7e2361237ce7244430ee1d581c77f95c57ac8/codex-rs/app-server-transport/src/transport/remote_control/client_tracker.rs): native remote sessions open with `ConnectionOrigin::RemoteControl`; messages become native transport events. +- [Original rationale, #5658](https://github.com/openai/codex/pull/5658): provider annotations/filtering address cross-provider resume/decryption failures. Visibility is not proof that a conversation is safe to resume under a different provider. +- [OpenCodex mitigation, #6007](https://github.com/lidge-jun/opencodex/pull/6007): warnings and documentation without rewriting native history or intercepting native RPC. + +## Alternatives + +| Approach | Can leave official mobile app unchanged? | Additional ownership | Recommendation | +| --- | --- | --- | --- | +| Mobile explicitly sends `modelProviders: []` (or a chosen list) | No | Client's list behavior and resume UX | Simplest client correction; continue upstream tracking. | +| Native, opt-in, remote-only default list policy | Yes, once a compatible native build ships | Native config and request policy, no extra service | **Preferred implementation proposal** when the host needs explicit control. | +| Local relay selected through `chatgpt_base_url` | Potentially; live service not verified | Shared backend traffic, token forwarding, connection lifecycle | Research fallback, not a default or currently supported remedy. | +| Globally change omitted filter to all providers | Yes | Changes behavior for every caller | Do not use: loses the existing default isolation behavior. | +| Rewrite `openai` history tags to `opencodex` | Yes in some observations | Native SQLite/rollout state | Do not use: violates the retained history-writer boundary. | + +The native proposal minimizes new connection and credential handling; this is an +engineering recommendation, not evidence of upstream acceptance or deployment. +Neither upstream route can be delivered by modifying OpenCodex's `/v1` inference +proxy alone. This PR publishes the comparison rather than disguising a probe as a fix. + +## Proposed native setting (name subject to upstream review) + +**Illustrative only; do not add this to current Codex/OpenCodex configuration.** + +```toml +[remote_control] +thread_list_model_providers = ["openai", "opencodex"] +``` + +Absent setting means opt-out. An explicitly empty array means all providers; +a nonempty array means exactly those ids. Do not automatically infer equivalence +from provider names, model names, or a shared URL. The setting changes listing, +not authentication, permission checks, routing, compaction, or resume semantics. +An operator who only needs two provider ids need not opt into every provider. + +### Precedence + +1. Existing authentication and managed remote-control policy still run first. +2. A request's explicit provider **array**, including `[]`, always wins. +3. With no array, parent/ancestor queries retain their current no-default-filter behavior. +4. Only a server-identified remote connection may use the operator's configured policy. +5. With no policy, and for every non-remote connection, preserve the existing default. + +Typed Rust `Option>` treats omission and JSON `null` alike. This +proposal deliberately applies its default to both. The raw relay probe preserves +all present JSON keys, including null; that difference is documented and must not +be mistaken for identical behavior between the two alternatives. + +Use the native `ConnectionOrigin` attached to the connection. Never infer remote +origin from clientInfo.name, a user-agent, JSON fields, or a caller-supplied header. +The Python enum tests only model this trusted input; they do not prove authentication. + +### Native implementation boundary + +The upstream change must add and validate the config contract, snapshot it with +the native app-server's effective configuration, carry the trusted connection +origin to the `thread/list` handler, and resolve the provider predicate before +calling the existing store pagination. A new RPC layer is unnecessary. + +Conceptual selection, **not a drop-in patch**: + +```text +if request supplies provider array: use that array +else if parent/ancestor query: use existing related-thread behavior +else if trusted origin is RemoteControl and operator policy is configured: + use the configured array +else: use existing configured-default behavior +``` + +Do not add a global exception to `list_threads_common` for all callers. Do not +change `thread/start`, `thread/resume`, `turn/start`, stored provider metadata, +or request error handling. Keep source/cwd/archive/project filters and ordering. +The new configuration's trust/precedence should prevent project content from +silently opting an operator into a broader default; upstream must choose and test +the appropriate configuration layers and any managed restrictions. + +Hold one effective policy throughout a listing/pagination sequence. A changed +policy requires a fresh listing cursor; do not combine pages obtained under +incompatible filters. Native tests must pin behavior with actual cursor semantics. +The fixture tests here use integer pagination, not native opaque cursors. + +## Local relay alternative: what was and was not shown + +[Native startup](https://github.com/openai/codex/blob/1cc7e2361237ce7244430ee1d581c77f95c57ac8/codex-rs/app-server/src/lib.rs) +uses `config.chatgpt_base_url` for remote control. The +[URL protocol](https://github.com/openai/codex/blob/1cc7e2361237ce7244430ee1d581c77f95c57ac8/codex-rs/app-server-transport/src/transport/remote_control/protocol.rs) +accepts loopback URLs. A host-side relay is therefore a concrete design alternative: + +```text +unchanged mobile <-> existing ChatGPT backend <-> local relay <-> native app-server +``` + +Connection establishment starts on the host. Enroll/pair/refresh can be forwarded +rather than reimplemented. However, `chatgpt_base_url` is shared with +[authentication configuration](https://github.com/openai/codex/blob/1cc7e2361237ce7244430ee1d581c77f95c57ac8/codex-rs/core/src/config/auth_keyring.rs) +and other backend consumers. A WebSocket-only implementation is insufficient. +[Enrollment persistence](https://github.com/openai/codex/blob/1cc7e2361237ce7244430ee1d581c77f95c57ac8/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs) +also keys state by URL/account/client, so changing the URL can affect enrollment +selection; whether live re-pairing is required remains unverified. + +The retained relay fixture accepts only literal `127.0.0.1` HTTP upstreams and +synthetic credentials. It is not a production service and cannot be configured +for ChatGPT. It handles regular and single-chunk frames; multi-chunk messages are +passed unchanged. Actual protocol-v3 segmentation, native reconnect/ACK/cursors, +account changes, managed network policy, and full backend compatibility remain +release gates, not claims supported by these tests. See native +[WebSocket handling](https://github.com/openai/codex/blob/1cc7e2361237ce7244430ee1d581c77f95c57ac8/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs). + +No global base-URL mutation, token-pool integration, production listener, +configuration migration, history write, or authentication bypass is proposed here. + +## Upstream and downstream follow-through + +An upstream implementation needs Rust config/schema, origin-scoping, explicit/null/ +related-query, pagination, and managed-policy tests. Keep existing defaults intact. +Document the omitted/default behavior and regenerate affected schema/TS fixtures. + +After an accepted implementation is released, OpenCodex may consider an opt-in +integration with positive capability/version evidence and precise configuration +ownership/restoration. Merely writing an unknown TOML key is not a feature test. +Until then, retain #6007's warnings and keep #5848 open. Do not suppress the warning +because an experimental setting was written or a fixture passed. diff --git a/devlog/_plan/260928_remote_thread_provider_policy/020_verification.md b/devlog/_plan/260928_remote_thread_provider_policy/020_verification.md new file mode 100644 index 00000000000..520f602b369 --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/020_verification.md @@ -0,0 +1,65 @@ +# Verification + +Status: local evidence for an RFC. The probes are executable specifications that +run on demand; no CI workflow runs them, and nothing in the OpenCodex build, test, +or runtime path imports them. All inputs, accounts, tokens, thread ids, and +database rows are synthetic. No native Codex process, user home, installed +config, or real service is accessed. + +## How to run + +From the repository root: + +```sh +cd devlog/_plan/260928_remote_thread_provider_policy/probes +python3 -m unittest -v test_native_policy test_probe +``` + +The socket suite needs aiohttp. Install it into a scratch directory, not the +repository or a system site-packages: + +```sh +python3 -m pip install --target "$SCRATCH_DIR" aiohttp +PYTHONPATH="$SCRATCH_DIR" python3 -m unittest -v test_native_policy test_probe test_loopback_bridge +``` + +## Inventory + +63 tests across three suites: + +- 23 tests (`test_native_policy`) specify the proposed native policy: opt-out, + trusted-origin scoping, explicit arrays, null semantics, parent/ancestor + exceptions, immutable policy, exact ids, UUID thread-id validation, and + synthetic paginated fixture reads. +- 29 raw-frame tests (`test_probe`) cover the relay alternative's + ordinary/single-chunk rewrites, byte-identical pass-through, limits, and + unsupported inputs. +- 11 localhost HTTP/WebSocket tests (`test_loopback_bridge`) use a mock host and + mock backend, checking two-way traffic, fixture auth/headers, explicit filters, + enrollment, unrelated HTTP, fixture endpoint restrictions, and both directions + of close-code forwarding. + +The first 52 use the Python standard library only. + +The synthetic database has 5,200 `openai` rows, one `opencodex` row, and one +`other` row. Default filtering yields one; the two-id opt-in yields 5,201; all +providers yields 5,202. Page sizes 1, 37, and 1,000 are exercised. The fixture +dump hash stays unchanged after reads. This demonstrates the specification, not +native SQLite schema compatibility or actual mobile results. + +## Recorded runs + +| Date | Environment | Suites | Result | +| --- | --- | --- | --- | +| 2026-09-28 | author, local | native policy + raw frame | 52 passed | +| 2026-10-13 | Linux, Python 3.12.14, aiohttp 3.14.4 | all three | 63 passed | +| 2026-10-15 | macOS, Python 3.9.6, aiohttp 3.13.5 | all three | 63 passed | + +Each run reflects the probe source at that time; a later edit to a probe needs a +fresh local run. + +## Not executed / not established + +- Native Rust implementation, compilation, config/schema generation, or native tests. +- Actual ChatGPT mobile pairing, full pagination, resume, token renewal, or reconnect. +- Production multi-segment relay support or management/security review of a relay. diff --git a/devlog/_plan/260928_remote_thread_provider_policy/probes/native_policy.py b/devlog/_plan/260928_remote_thread_provider_policy/probes/native_policy.py new file mode 100644 index 00000000000..80859ac8395 --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/probes/native_policy.py @@ -0,0 +1,103 @@ +"""Executable specification for a PROPOSED native Codex policy, not an integration. + +No socket, authentication, config, history, or rollout access. The real Rust +implementation must obtain origin from its authenticated server-side connection, +not from JSON, clientInfo.name, or an HTTP header. This enum only models that input. +None as the result means no provider predicate, not no results. +""" +from __future__ import annotations + +from dataclasses import dataclass +from enum import Enum +from uuid import UUID +import re + +# Rust's Uuid::parse_str accepts exactly four textual shapes - hyphenated, simple, +# braced-hyphenated and the urn:uuid prefix. Python's UUID() is looser: it tolerates +# arbitrary hyphen placement inside a 32-hex payload, so wrapper/hyphen variants +# must be rejected by shape before parsing instead of leaning on the constructor. +_UUID_ACCEPTED_SHAPES = re.compile( + r"(?:" + r"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" + r"|[0-9a-fA-F]{32}" + r"|\{[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}\}" + r"|urn:uuid:[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}" + r")\Z" +) + + +class ConnectionOrigin(Enum): + STDIO = "stdio" + WEBSOCKET = "websocket" + IN_PROCESS = "in_process" + REMOTE_CONTROL = "remote_control" + + +def _validate_ids(value: tuple[str, ...], label: str) -> None: + if not isinstance(value, tuple): + raise ValueError(f"{label} must be a tuple") + if any(not isinstance(item, str) or not item.strip() for item in value): + raise ValueError(f"{label} must contain non-empty strings") + if len(set(value)) != len(value): + raise ValueError(f"{label} must not contain duplicate identifiers") + + +def _validate_thread_id(value: str, label: str) -> None: + """Upstream parses ThreadId as a UUID; a non-UUID string must not widen the list.""" + if not isinstance(value, str) or not _UUID_ACCEPTED_SHAPES.match(value): + raise ValueError(f"{label} must be a UUID thread id") + try: + UUID(value) + except (ValueError, AttributeError, TypeError): + raise ValueError(f"{label} must be a UUID thread id") + + +@dataclass(frozen=True) +class RemoteListPolicy: + # None: opt-out; (): explicitly all; non-empty: exactly these provider ids. + providers: tuple[str, ...] | None = None + + def __post_init__(self) -> None: + if self.providers is not None: + _validate_ids(self.providers, "providers") + + +def resolve_provider_filter( + *, + origin: ConnectionOrigin, + default_provider: str, + requested: tuple[str, ...] | None = None, + parent_thread_id: str | None = None, + ancestor_thread_id: str | None = None, + policy: RemoteListPolicy = RemoteListPolicy(), +) -> tuple[str, ...] | None: + """Model precedence after typed request decoding and existing authorization. + + Rust Option> decodes both omission and JSON null as None. They + intentionally have the same semantics in this native-policy proposal. The + separate raw-frame relay probe instead preserves every present JSON key. + Empty requested tuple is an explicit client request for all providers. + The policy applies only to thread/list; callers must not reuse it for resume. + """ + if not isinstance(origin, ConnectionOrigin): + raise ValueError("origin must be supplied by the trusted connection context") + _validate_ids((default_provider,), "default_provider") + for label, tid in ( + ("parent_thread_id", parent_thread_id), + ("ancestor_thread_id", ancestor_thread_id), + ): + if tid is not None: + _validate_thread_id(tid, label) + if parent_thread_id is not None and ancestor_thread_id is not None: + raise ValueError("parent_thread_id and ancestor_thread_id are mutually exclusive") + if requested is not None: + # Preserve every typed client array, including duplicate/empty ids. + # Native Vec accepts them; this proposal must not add rejection. + if not isinstance(requested, tuple) or any(not isinstance(item, str) for item in requested): + raise ValueError("requested must model a typed string array") + return requested or None + if parent_thread_id is not None or ancestor_thread_id is not None: + return None + if origin is ConnectionOrigin.REMOTE_CONTROL and policy.providers is not None: + return policy.providers or None + return (default_provider,) diff --git a/devlog/_plan/260928_remote_thread_provider_policy/probes/remote_list_probe.py b/devlog/_plan/260928_remote_thread_provider_policy/probes/remote_list_probe.py new file mode 100644 index 00000000000..a2622cdf18e --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/probes/remote_list_probe.py @@ -0,0 +1,160 @@ +"""Offline research probe; NOT an installed OpenCodex feature or production proxy. + +Based on openai/codex commit 1cc7e2361237ce7244430ee1d581c77f95c57ac8. +Only changes omitted thread/list.modelProviders in explicit opt-in mode. +No network, authentication, filesystem, database, or rollout writes occur here. +Complete multi-segment messages need a separate bounded streaming reassembler; +this probe deliberately passes those messages through unchanged. +""" +from __future__ import annotations + +import base64 +import binascii +import json +from dataclasses import dataclass +from typing import Any + + +@dataclass(frozen=True) +class Policy: + enabled: bool = False + # An empty tuple explicitly requests all providers, matching modelProviders: []. + providers: tuple[str, ...] = ("openai", "opencodex") + max_wire_bytes: int = 150 * 1024 + + def __post_init__(self) -> None: + if not isinstance(self.providers, tuple) or any( + not isinstance(p, str) or not p.strip() for p in self.providers + ): + raise ValueError("providers must be a tuple of non-empty strings") + if len(set(self.providers)) != len(self.providers): + raise ValueError("duplicate provider identifiers") + if self.max_wire_bytes < 64: + raise ValueError("max_wire_bytes must be at least 64") + + +@dataclass(frozen=True) +class Outcome: + text: str + changed: bool + reason: str + + +def _unique_object(pairs: list[tuple[str, Any]]) -> dict[str, Any]: + result: dict[str, Any] = {} + for key, value in pairs: + if key in result: + raise ValueError("duplicate JSON member") + result[key] = value + return result + + +def _reject_constant(value: str) -> None: + raise ValueError("non-standard JSON constant") + + +def decode(text: str | bytes) -> Any: + return json.loads(text, object_pairs_hook=_unique_object, + parse_constant=_reject_constant) + + +def encode(value: Any) -> str: + return json.dumps(value, ensure_ascii=False, separators=(",", ":"), + allow_nan=False) + + +def _patch_message(message: Any, policy: Policy) -> tuple[bool, str]: + if not isinstance(message, dict) or message.get("method") != "thread/list": + return False, "not_thread_list" + request_id = message.get("id") + if isinstance(request_id, bool) or not isinstance(request_id, (int, str)): + return False, "not_request" + if "result" in message or "error" in message: + return False, "ambiguous_request" + params = message.get("params", {}) + if not isinstance(params, dict): + return False, "non_object_params" + # Preserve explicit client choices, including an explicit null. + if "modelProviders" in params: + return False, "explicit_filter_preserved" + # Native related-thread queries deliberately bypass the omitted-provider default. + if params.get("parentThreadId") is not None or params.get("ancestorThreadId") is not None: + return False, "relation_query_preserved" + params["modelProviders"] = list(policy.providers) + message["params"] = params + return True, "omitted_filter_patched" + + +def rewrite_backend_frame(text: str, policy: Policy) -> Outcome: + """Process ONE backend-to-host text frame. No authentication decision is made. + + The caller must already enforce the authorized connection and user opt-in. + Unmodified and unsupported messages are returned byte-for-byte unchanged. + """ + if not isinstance(text, str): + raise TypeError("text frame must be str") + if not policy.enabled: + return Outcome(text, False, "disabled") + try: + if len(text.encode("utf-8")) > policy.max_wire_bytes: + return Outcome(text, False, "input_size_limit") + envelope = decode(text) + if not isinstance(envelope, dict): + return Outcome(text, False, "not_envelope") + if not isinstance(envelope.get("client_id"), str) or not envelope["client_id"]: + return Outcome(text, False, "missing_client_id") + kind = envelope.get("type") + if kind == "client_message": + changed, reason = _patch_message(envelope.get("message"), policy) + elif kind == "client_message_chunk": + if type(envelope.get("segment_count")) is not int or type(envelope.get("segment_id")) is not int: + return Outcome(text, False, "invalid_segment_index") + if envelope["segment_count"] != 1 or envelope["segment_id"] != 0: + return Outcome(text, False, "multi_segment_not_implemented") + size = envelope.get("message_size_bytes") + if type(size) is not int or size < 0 or size > policy.max_wire_bytes: + return Outcome(text, False, "invalid_segment_size") + payload_b64 = envelope.get("message_chunk_base64") + if not isinstance(payload_b64, str): + return Outcome(text, False, "invalid_segment_payload") + payload = base64.b64decode(payload_b64, validate=True) + if len(payload) != size: + return Outcome(text, False, "segment_length_mismatch") + message = decode(payload) + changed, reason = _patch_message(message, policy) + if changed: + encoded_payload = encode(message).encode("utf-8") + envelope["message_size_bytes"] = len(encoded_payload) + envelope["message_chunk_base64"] = base64.b64encode(encoded_payload).decode("ascii") + else: + return Outcome(text, False, "unrelated_envelope") + if not changed: + return Outcome(text, False, reason) + output = encode(envelope) + if len(output.encode("utf-8")) > policy.max_wire_bytes: + return Outcome(text, False, "output_size_limit") + return Outcome(output, True, reason) + except (ValueError, TypeError, UnicodeError, binascii.Error, RecursionError): + return Outcome(text, False, "invalid_frame_preserved") + + +def make_envelope(message: dict[str, Any], *, chunk: bool = False) -> dict[str, Any]: + """Synthetic fixture builder; all identifiers below are test-only.""" + envelope: dict[str, Any] = { + "client_id": "mock-client", "stream_id": "mock-stream", + "seq_id": 7, "cursor": "mock-backend-cursor", + } + if chunk: + payload = encode(message).encode("utf-8") + envelope.update(type="client_message_chunk", segment_id=0, segment_count=1, + message_size_bytes=len(payload), + message_chunk_base64=base64.b64encode(payload).decode("ascii")) + else: + envelope.update(type="client_message", message=message) + return envelope + + +def extract_message(envelope: dict[str, Any]) -> dict[str, Any]: + if envelope["type"] == "client_message": + return envelope["message"] + return decode(base64.b64decode(envelope["message_chunk_base64"], validate=True)) diff --git a/devlog/_plan/260928_remote_thread_provider_policy/probes/test_loopback_bridge.py b/devlog/_plan/260928_remote_thread_provider_policy/probes/test_loopback_bridge.py new file mode 100644 index 00000000000..2438a4334a6 --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/probes/test_loopback_bridge.py @@ -0,0 +1,259 @@ +"""Loopback-only mock transport proof. NEVER connects to ChatGPT/OpenAI. + +This is a test fixture, not a production proxy. Native Codex is not executed. +The mock backend validates a hard-coded, non-secret fixture token. +""" +from __future__ import annotations +import asyncio +import contextlib +import unittest +from urllib.parse import urlsplit + +import aiohttp +from aiohttp import web +from remote_list_probe import Policy, decode, encode, make_envelope, extract_message, rewrite_backend_frame + +BASE = "/backend-api" +WS_PATH = BASE + "/wham/remote/control/server" +MOCK_AUTH = "Bearer NOT-A-REAL-TOKEN-MOCK-ONLY" + + +def assert_loopback_only(base: str) -> None: + url = urlsplit(base) + if url.scheme != "http" or url.hostname != "127.0.0.1" or not url.port or url.username or url.password: + raise ValueError("this research fixture permits only HTTP at literal 127.0.0.1") + if url.query or url.fragment or url.path: + raise ValueError("mock base must contain only origin") + + +def selected_headers(headers) -> dict[str, str]: + return {key: value for key, value in headers.items() + if key.lower() in ("authorization", "chatgpt-account-id", "content-type") + or key.lower().startswith("x-codex-")} + + +class LoopbackFixtureRelay: + def __init__(self, mock_backend: str, session: aiohttp.ClientSession, policy: Policy): + assert_loopback_only(mock_backend) + self.backend = mock_backend + self.session = session + self.policy = policy + + async def handle(self, request: web.Request) -> web.StreamResponse: + # Test-only origin/host guard. No remote bind and no arbitrary upstream selection. + if request.headers.get("Origin"): + return web.Response(status=403, text="browser-origin-blocked") + if not request.host.startswith("127.0.0.1:"): + return web.Response(status=403, text="unexpected-host") + if request.path == WS_PATH and request.headers.get("Upgrade", "").lower() == "websocket": + return await self.handle_websocket(request) + target = self.backend + request.raw_path + async with self.session.request(request.method, target, + data=await request.read(), + headers=selected_headers(request.headers), + allow_redirects=False) as response: + body = await response.read() + headers = {key: value for key, value in response.headers.items() + if key.lower() in ("content-type", "x-request-id", "retry-after")} + return web.Response(status=response.status, body=body, headers=headers) + + async def handle_websocket(self, request: web.Request) -> web.StreamResponse: + target = self.backend + request.raw_path + try: + upstream = await self.session.ws_connect(target, + headers=selected_headers(request.headers), autoping=False) + except aiohttp.WSServerHandshakeError as exc: + return web.Response(status=exc.status, text="mock-upstream-rejected") + downstream = web.WebSocketResponse(autoping=False) + await downstream.prepare(request) + + async def pump(source, destination, transform: bool): + async for message in source: + if message.type == aiohttp.WSMsgType.TEXT: + text = rewrite_backend_frame(message.data, self.policy).text if transform else message.data + await destination.send_str(text) + elif message.type == aiohttp.WSMsgType.BINARY: + await destination.send_bytes(message.data) + elif message.type == aiohttp.WSMsgType.PING: + await destination.ping(message.data) + elif message.type == aiohttp.WSMsgType.PONG: + await destination.pong(message.data) + else: + break + # Forward the peer's close so the other side sees the same code + # instead of hanging on a connection that already ended. + close_code = getattr(source, "close_code", None) or 1000 + if not destination.closed: + await destination.close(code=close_code) + + tasks = [asyncio.create_task(pump(upstream, downstream, True)), + asyncio.create_task(pump(downstream, upstream, False))] + try: + done, _ = await asyncio.wait(tasks, return_when=asyncio.FIRST_COMPLETED) + for task in done: + task.result() + finally: + for task in tasks: + task.cancel() + await asyncio.gather(*tasks, return_exceptions=True) + await upstream.close() + await downstream.close() + return downstream + + +async def start_loopback_app(app: web.Application): + runner = web.AppRunner(app, access_log=None) + await runner.setup() + site = web.TCPSite(runner, "127.0.0.1", 0) + await site.start() + port = runner.addresses[0][1] + return runner, f"http://127.0.0.1:{port}" + + +class LoopbackBridgeTests(unittest.IsolatedAsyncioTestCase): + async def asyncSetUp(self): + self.frames = [] + self.received = asyncio.Queue() + self.handshake_headers = {} + self.http_received = [] + self.backend_closed = asyncio.Event() + self.backend_close_code = None + self.backend_close_on_connect = None + self.backend_app = web.Application() + self.backend_app.router.add_route("*", "/backend-api/{tail:.*}", self.mock_backend) + self.backend_runner, self.backend_base = await start_loopback_app(self.backend_app) + self.relay_session = aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=5)) + self.relay = LoopbackFixtureRelay(self.backend_base, self.relay_session, Policy(enabled=True)) + relay_app = web.Application() + relay_app.router.add_route("*", "/backend-api/{tail:.*}", self.relay.handle) + self.relay_runner, self.relay_base = await start_loopback_app(relay_app) + self.host = aiohttp.ClientSession(timeout=aiohttp.ClientTimeout(total=5)) + + async def asyncTearDown(self): + await self.host.close() + await self.relay_runner.cleanup() + await self.relay_session.close() + await self.backend_runner.cleanup() + + async def mock_backend(self, request: web.Request): + if request.headers.get("Authorization") != MOCK_AUTH: + return web.Response(status=401, text="mock-auth-required") + if request.path == WS_PATH and request.headers.get("Upgrade", "").lower() == "websocket": + self.handshake_headers = dict(request.headers) + websocket = web.WebSocketResponse(autoping=False) + await websocket.prepare(request) + for frame in self.frames: + await websocket.send_str(frame) + if self.backend_close_on_connect is not None: + await websocket.close(code=self.backend_close_on_connect) + return websocket + async for message in websocket: + if message.type == aiohttp.WSMsgType.TEXT: + await self.received.put(message.data) + elif message.type == aiohttp.WSMsgType.PING: + await websocket.pong(message.data) + self.backend_close_code = websocket.close_code + self.backend_closed.set() + return websocket + body = await request.read() + self.http_received.append((request.path, body, selected_headers(request.headers))) + if request.path.endswith("/enroll"): + return web.json_response({"server_id": "mock-server", "environment_id": "mock-env", + "remote_control_token": "MOCK-ONLY", "expires_at": "2099-01-01T00:00:00Z"}, + status=201, headers={"x-request-id": "mock-http-request"}) + return web.Response(status=200, body=b"mock-unrelated-backend-endpoint") + + def add_request(self, params=None, *, chunk=False): + request = {"id": 42, "method": "thread/list", "params": params or {"limit": 20}} + self.frames.append(encode(make_envelope(request, chunk=chunk))) + + async def receive_host_frame(self, headers=None): + async with self.host.ws_connect(self.relay_base + WS_PATH, + headers=headers or {"Authorization": MOCK_AUTH}) as host: + message = await asyncio.wait_for(host.receive(), 2) + self.assertEqual(message.type, aiohttp.WSMsgType.TEXT) + return message.data + + async def test_backend_to_host_rewrite_and_host_reply_passthrough(self): + self.add_request() + async with self.host.ws_connect(self.relay_base + WS_PATH, + headers={"Authorization": MOCK_AUTH}) as host: + incoming = await asyncio.wait_for(host.receive(), 2) + envelope = decode(incoming.data) + self.assertEqual(extract_message(envelope)["params"]["modelProviders"], ["openai", "opencodex"]) + response = '{ "type":"server_message", "client_id":"mock-client", "stream_id":"mock-stream", "seq_id":9, "message":{"id":42,"result":{"data":[]}} }' + await host.send_str(response) + self.assertEqual(await asyncio.wait_for(self.received.get(), 2), response) + + async def test_auth_and_protocol_handshake_headers_preserved(self): + self.add_request() + headers = {"Authorization": MOCK_AUTH, "chatgpt-account-id": "mock-account", + "x-codex-protocol-version": "3", "x-codex-server-id": "mock-server", + "x-codex-installation-id": "mock-installation", "x-codex-subscribe-cursor": "mock-resume-cursor"} + await self.receive_host_frame(headers) + actual = {k.lower(): v for k, v in self.handshake_headers.items()} + for name, value in headers.items(): + self.assertEqual(actual[name.lower()], value) + + async def test_invalid_auth_stays_denied(self): + with self.assertRaises(aiohttp.WSServerHandshakeError) as ctx: + await self.host.ws_connect(self.relay_base + WS_PATH, + headers={"Authorization": "Bearer WRONG-MOCK"}) + self.assertEqual(ctx.exception.status, 401) + + async def test_explicit_filter_frame_is_byte_identical(self): + self.add_request({"modelProviders": ["other"], "limit": 20}) + self.assertEqual(await self.receive_host_frame(), self.frames[0]) + + async def test_single_chunk_transport(self): + self.add_request(chunk=True) + incoming = decode(await self.receive_host_frame()) + self.assertEqual(extract_message(incoming)["params"]["modelProviders"], ["openai", "opencodex"]) + self.assertEqual(incoming["seq_id"], 7) + self.assertEqual(incoming["cursor"], "mock-backend-cursor") + + async def test_host_close_code_reaches_backend(self): + async with self.host.ws_connect(self.relay_base + WS_PATH, + headers={"Authorization": MOCK_AUTH}) as host: + await host.close(code=1001) + await asyncio.wait_for(self.backend_closed.wait(), 5) + self.assertEqual(self.backend_close_code, 1001) + + async def test_backend_close_code_reaches_host(self): + self.backend_close_on_connect = 1011 + async with self.host.ws_connect(self.relay_base + WS_PATH, + headers={"Authorization": MOCK_AUTH}) as host: + async for message in host: + if message.type in (aiohttp.WSMsgType.CLOSE, + aiohttp.WSMsgType.CLOSED, + aiohttp.WSMsgType.CLOSING): + break + self.assertEqual(host.close_code, 1011) + + async def test_enrollment_http_forwarding_with_fake_credentials(self): + body = b'{"name":"mock-host","installation_id":"mock-installation"}' + async with self.host.post(self.relay_base + WS_PATH + "/enroll", data=body, + headers={"Authorization": MOCK_AUTH, "chatgpt-account-id": "mock-account"}) as response: + self.assertEqual(response.status, 201) + self.assertEqual(response.headers["x-request-id"], "mock-http-request") + self.assertEqual((await response.json())["server_id"], "mock-server") + self.assertEqual(self.http_received[0][1], body) + + async def test_unrelated_http_backend_endpoint_forwarded(self): + async with self.host.get(self.relay_base + BASE + "/mock/account", headers={"Authorization": MOCK_AUTH}) as response: + self.assertEqual(response.status, 200) + self.assertEqual(await response.read(), b"mock-unrelated-backend-endpoint") + + async def test_cross_origin_browser_request_denied(self): + async with self.host.get(self.relay_base + BASE + "/mock/account", headers={"Authorization": MOCK_AUTH, "Origin": "https://example.invalid"}) as response: + self.assertEqual(response.status, 403) + self.assertFalse(self.http_received) + + async def test_fixture_rejects_real_service_upstream(self): + for base in ("https://chatgpt.com", "http://example.invalid", "http://0.0.0.0:1234"): + with self.subTest(base=base): + with self.assertRaises(ValueError): + LoopbackFixtureRelay(base, self.relay_session, Policy(enabled=True)) + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/devlog/_plan/260928_remote_thread_provider_policy/probes/test_native_policy.py b/devlog/_plan/260928_remote_thread_provider_policy/probes/test_native_policy.py new file mode 100644 index 00000000000..58d07be97f2 --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/probes/test_native_policy.py @@ -0,0 +1,198 @@ +"""Offline contract tests. This does NOT run Rust, native Codex, or a mobile app.""" +from __future__ import annotations + +import hashlib +import json +import sqlite3 +import unittest +from dataclasses import FrozenInstanceError +from native_policy import ConnectionOrigin as Origin, RemoteListPolicy, resolve_provider_filter + + +class NativePolicyTests(unittest.TestCase): + def resolve(self, **kwargs): + defaults = {"origin": Origin.REMOTE_CONTROL, "default_provider": "opencodex"} + defaults.update(kwargs) + return resolve_provider_filter(**defaults) + + def test_opt_out_keeps_existing_default_for_every_origin(self): + for origin in Origin: + with self.subTest(origin=origin): + self.assertEqual(self.resolve(origin=origin), ("opencodex",)) + + def test_only_native_remote_origin_uses_opt_in(self): + policy = RemoteListPolicy(("openai", "opencodex")) + for origin in Origin: + with self.subTest(origin=origin): + expected = policy.providers if origin is Origin.REMOTE_CONTROL else ("opencodex",) + self.assertEqual(self.resolve(origin=origin, policy=policy), expected) + + def test_explicit_nonempty_filter_wins_over_every_policy(self): + for policy in (RemoteListPolicy(), RemoteListPolicy(()), RemoteListPolicy(("openai",))): + for origin in Origin: + with self.subTest(policy=policy, origin=origin): + for requested in (("other",), ("other", "other"), ("",), (" other ",)): + self.assertEqual(self.resolve(origin=origin, requested=requested, policy=policy), requested) + + def test_explicit_empty_filter_is_all_for_every_origin(self): + for origin in Origin: + with self.subTest(origin=origin): + self.assertIsNone(self.resolve(origin=origin, requested=(), policy=RemoteListPolicy(("openai",)))) + + def test_opt_in_all_is_not_the_default(self): + self.assertIsNone(self.resolve(policy=RemoteListPolicy(()))) + self.assertEqual(self.resolve(), ("opencodex",)) + + def test_parent_query_keeps_native_no_default_filter(self): + for policy in (RemoteListPolicy(), RemoteListPolicy(("opencodex",))): + self.assertIsNone(self.resolve(parent_thread_id="00000000-0000-4000-8000-0000000000aa", policy=policy)) + + def test_ancestor_query_keeps_native_no_default_filter(self): + for policy in (RemoteListPolicy(), RemoteListPolicy(("opencodex",))): + self.assertIsNone(self.resolve(ancestor_thread_id="00000000-0000-4000-8000-0000000000bb", policy=policy)) + + def test_related_thread_ids_follow_native_validation(self): + # Upstream thread_list_response_inner rejects malformed ids and rejects + # parent+ancestor together; a malformed id must not silently widen the list. + for relation in ( + {"parent_thread_id": ""}, + {"ancestor_thread_id": " "}, + {"parent_thread_id": "fixture-parent"}, + {"ancestor_thread_id": "fixture-ancestor"}, + {"parent_thread_id": "p"}, + ): + with self.subTest(relation=relation), self.assertRaises(ValueError): + self.resolve(**relation) + # Python's UUID() tolerates hyphen shapes Rust Uuid::parse_str rejects: + # shifted hyphens, a bare 32-hex blob inside braces, an uppercased URN, and + # a braced simple string all stay invalid no matter the provider shape. + malformed = ( + "000-00000-0000-4000-8000-0000000000aa", + "{000000000000400080000000000000aa}", + "URN:UUID:00000000-0000-4000-8000-0000000000aa", + "{000000000000-4000-8000-0000000000aa}", + ) + for key in ("parent_thread_id", "ancestor_thread_id"): + for bad in malformed: + for requested in (None, ("openai",)): + with self.subTest(key=key, bad=bad, requested=requested), self.assertRaises(ValueError): + self.resolve(**{key: bad}, requested=requested) + # The four shapes upstream accepts stay valid for both relations. + for key in ("parent_thread_id", "ancestor_thread_id"): + for good in ( + "00000000-0000-4000-8000-0000000000aa", + "000000000000400080000000000000aa", + "{00000000-0000-4000-8000-0000000000aa}", + "urn:uuid:00000000-0000-4000-8000-0000000000aa", + ): + with self.subTest(key=key, good=good): + self.assertIsNone(self.resolve(**{key: good})) + with self.assertRaises(ValueError): + self.resolve( + parent_thread_id="00000000-0000-4000-8000-0000000000aa", + ancestor_thread_id="00000000-0000-4000-8000-0000000000bb", + ) + + def test_explicit_filter_still_wins_for_related_queries(self): + for relation in ( + {"parent_thread_id": "00000000-0000-4000-8000-0000000000aa"}, + {"ancestor_thread_id": "00000000-0000-4000-8000-0000000000bb"}, + ): + self.assertEqual(self.resolve(requested=("other",), policy=RemoteListPolicy(()), **relation), ("other",)) + + def test_null_and_omitted_match_typed_native_option_semantics(self): + for wire in ('{}', '{"modelProviders": null}'): + requested = json.loads(wire).get("modelProviders") + self.assertIsNone(requested) + self.assertEqual(self.resolve(requested=requested, policy=RemoteListPolicy(("openai",))), ("openai",)) + + def test_provider_names_are_not_hardcoded(self): + self.assertEqual(self.resolve(default_provider="custom"), ("custom",)) + self.assertEqual(self.resolve(policy=RemoteListPolicy(("azure-team", "local"))), ("azure-team", "local")) + + def test_provider_ids_are_exact_not_aliases(self): + self.assertEqual(self.resolve(policy=RemoteListPolicy(("OpenAI", " openai "))), ("OpenAI", " openai ")) + + def test_policy_cannot_be_mutated_during_pagination(self): + policy = RemoteListPolicy(("openai",)) + with self.assertRaises(FrozenInstanceError): + policy.providers = () + + def test_rejects_invalid_policy(self): + for value in (["openai"], "openai", ("",), (" ",), (1,), ("openai", "openai")): + with self.subTest(value=value), self.assertRaises(ValueError): + RemoteListPolicy(value) + + def test_rejects_untrusted_origin_string(self): + with self.assertRaises(ValueError): + self.resolve(origin="remote_control") + + def test_rejects_invalid_default_provider(self): + for value in ("", " ", None, 1): + with self.subTest(value=value), self.assertRaises(ValueError): + self.resolve(default_provider=value) + + def test_rejects_invalid_requested_filter(self): + for value in (["other"], "other", (1,), (None,)): + with self.subTest(value=value), self.assertRaises(ValueError): + self.resolve(requested=value) + + +class SyntheticPaginationTests(unittest.TestCase): + """Fixture SQL illustrates the contract, not the native Codex store schema.""" + def setUp(self): + self.db = sqlite3.connect(":memory:") + self.addCleanup(self.db.close) + self.db.execute("CREATE TABLE threads (id INTEGER PRIMARY KEY, provider TEXT NOT NULL)") + self.db.executemany("INSERT INTO threads VALUES (?, ?)", + [(i, "openai") for i in range(1, 5201)] + [(5201, "opencodex"), (5202, "other")]) + self.before = self.digest() + + def digest(self): + return hashlib.sha256("\n".join(self.db.iterdump()).encode()).hexdigest() + + def list_all(self, policy, limit=37, requested=None): + providers = resolve_provider_filter(origin=Origin.REMOTE_CONTROL, + default_provider="opencodex", requested=requested, policy=policy) + rows, last_id = [], 0 + while True: + values = [last_id] + sql = "SELECT id, provider FROM threads WHERE id > ?" + if providers is not None: + sql += " AND provider IN (" + ",".join("?" for _ in providers) + ")" + values.extend(providers) + sql += " ORDER BY id LIMIT ?" + values.append(limit) + page = self.db.execute(sql, values).fetchall() + if not page: + break + rows.extend(page) + last_id = page[-1][0] + self.assertEqual(self.digest(), self.before, "read must not retag fixture rows") + self.assertEqual(len({row[0] for row in rows}), len(rows)) + return rows + + def test_default_remains_one_thread(self): + self.assertEqual(self.list_all(RemoteListPolicy()), [(5201, "opencodex")]) + + def test_opt_in_two_provider_list_returns_5201_rows(self): + rows = self.list_all(RemoteListPolicy(("openai", "opencodex"))) + self.assertEqual(len(rows), 5201) + self.assertNotIn("other", {row[1] for row in rows}) + + def test_explicit_all_returns_5202_rows(self): + self.assertEqual(len(self.list_all(RemoteListPolicy(()))), 5202) + + def test_explicit_client_filter_not_broadened(self): + self.assertEqual(self.list_all(RemoteListPolicy(()), requested=("other",)), [(5202, "other")]) + + def test_unlisted_provider_returns_empty_without_fallback(self): + self.assertEqual(self.list_all(RemoteListPolicy(("not-in-fixture",))), []) + + def test_every_page_uses_same_policy_without_duplicates(self): + policy = RemoteListPolicy(("openai", "opencodex")) + self.assertEqual(self.list_all(policy, limit=1), self.list_all(policy, limit=1000)) + + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/devlog/_plan/260928_remote_thread_provider_policy/probes/test_probe.py b/devlog/_plan/260928_remote_thread_provider_policy/probes/test_probe.py new file mode 100644 index 00000000000..ffdb0c6e24d --- /dev/null +++ b/devlog/_plan/260928_remote_thread_provider_policy/probes/test_probe.py @@ -0,0 +1,196 @@ +from __future__ import annotations +import base64 +import hashlib +import json +import sqlite3 +import unittest +from copy import deepcopy +from remote_list_probe import Policy, decode, encode, make_envelope, extract_message, rewrite_backend_frame + + +class RewriteTests(unittest.TestCase): + def setUp(self): + self.policy = Policy(enabled=True) + self.request = {"id": 27, "method": "thread/list", "params": {"limit": 20, "cursor": "opaque"}} + + def frame(self, request=None, chunk=False): + return encode(make_envelope(deepcopy(self.request if request is None else request), chunk=chunk)) + + def result(self, request=None, chunk=False, policy=None): + return rewrite_backend_frame(self.frame(request, chunk), policy or self.policy) + + def assert_preserved(self, raw): + outcome = rewrite_backend_frame(raw, self.policy) + self.assertFalse(outcome.changed) + self.assertEqual(raw, outcome.text) + + def test_opt_in_required(self): + raw = self.frame() + self.assertEqual(rewrite_backend_frame(raw, Policy()).text, raw) + self.assertFalse(rewrite_backend_frame(raw, Policy()).changed) + + def test_plain_omitted_filter(self): + out = self.result() + self.assertTrue(out.changed) + self.assertEqual(extract_message(decode(out.text))["params"]["modelProviders"], ["openai", "opencodex"]) + + def test_explicit_all_provider_mode(self): + out = self.result(policy=Policy(enabled=True, providers=())) + self.assertEqual(extract_message(decode(out.text))["params"]["modelProviders"], []) + + def test_preserves_envelope_metadata(self): + before = decode(self.frame()) + after = decode(self.result().text) + for key in ("client_id", "stream_id", "seq_id", "cursor"): + self.assertEqual(before[key], after[key]) + + def test_preserves_all_other_parameters(self): + self.request["params"].update(archived=True, cwd="C:/mock", sourceKinds=["cli"], + sortDirection="asc", searchTerm="mock", isPinned=False) + after = extract_message(decode(self.result().text)) + del after["params"]["modelProviders"] + self.assertEqual(after, self.request) + + def test_explicit_filter_preserved(self): + self.request["params"]["modelProviders"] = ["other"] + self.assert_preserved(self.frame()) + + def test_explicit_empty_filter_preserved(self): + self.request["params"]["modelProviders"] = [] + self.assert_preserved(self.frame()) + + def test_explicit_null_filter_preserved(self): + self.request["params"]["modelProviders"] = None + self.assert_preserved(self.frame()) + + def test_missing_params_created(self): + del self.request["params"] + self.assertTrue(self.result().changed) + + def test_non_object_params_preserved(self): + for value in (None, [], "not-an-object", 1): + with self.subTest(value=value): + self.request["params"] = value + self.assert_preserved(self.frame()) + + def test_resume_and_other_methods_unchanged(self): + for method in ("thread/resume", "thread/start", "turn/start", "turn/interrupt", "initialize", "thread/search"): + with self.subTest(method=method): + self.request["method"] = method + self.assert_preserved(self.frame()) + + def test_notification_without_id_unchanged(self): + del self.request["id"] + self.assert_preserved(self.frame()) + + def test_request_id_preserved_exactly(self): + for request_id in ("request-string", 9007199254740993, 0, -1): + with self.subTest(request_id=request_id): + self.request["id"] = request_id + self.assertEqual(extract_message(decode(self.result().text))["id"], request_id) + + def test_response_like_object_unchanged(self): + self.request["result"] = {"data": []} + self.assert_preserved(self.frame()) + + def test_relation_query_bypass_preserved(self): + for key in ("parentThreadId", "ancestorThreadId"): + with self.subTest(key=key): + self.request["params"] = {key: "00000000-0000-4000-8000-0000000000aa"} + self.assert_preserved(self.frame()) + + def test_null_relation_does_not_block(self): + self.request["params"]["parentThreadId"] = None + self.assertTrue(self.result().changed) + + def test_ping_ack_close_unchanged(self): + for kind in ("ping", "ack", "client_closed", "unknown_future_event"): + with self.subTest(kind=kind): + self.assert_preserved(encode({"type": kind, "client_id": "mock-client", "seq_id": 3})) + + def test_invalid_json_preserved(self): + for raw in ("{broken", "[]", "null", '{"type":"client_message","client_id":"x","message":NaN}'): + with self.subTest(raw=raw): + self.assert_preserved(raw) + + def test_duplicate_members_preserved(self): + raw = self.frame().replace('"method":"thread/list"', '"method":"thread/list","method":"thread/resume"') + self.assert_preserved(raw) + + def test_unknown_envelope_fields_preserved(self): + envelope = decode(self.frame()) + envelope["future_field"] = {"tag": [1, 2, 3]} + out = rewrite_backend_frame(encode(envelope), self.policy) + self.assertEqual(decode(out.text)["future_field"], envelope["future_field"]) + + def test_single_chunk_transformed_with_correct_byte_size(self): + self.request["params"]["searchTerm"] = "ν•œκΈ€πŸ™‚" + out = self.result(chunk=True) + self.assertTrue(out.changed) + after = decode(out.text) + payload = base64.b64decode(after["message_chunk_base64"], validate=True) + self.assertEqual(len(payload), after["message_size_bytes"]) + self.assertEqual(decode(payload)["params"]["modelProviders"], ["openai", "opencodex"]) + self.assertEqual(after["seq_id"], 7) + + def test_single_chunk_existing_filter_unchanged(self): + self.request["params"]["modelProviders"] = ["other"] + self.assert_preserved(self.frame(chunk=True)) + + def test_multi_segment_deliberately_not_implemented(self): + envelope = decode(self.frame(chunk=True)) + envelope["segment_count"] = 2 + out = rewrite_backend_frame(encode(envelope), self.policy) + self.assertEqual(out.reason, "multi_segment_not_implemented") + self.assertFalse(out.changed) + + def test_bad_chunk_length_unchanged(self): + envelope = decode(self.frame(chunk=True)) + envelope["message_size_bytes"] += 1 + self.assert_preserved(encode(envelope)) + + def test_bad_chunk_base64_unchanged(self): + envelope = decode(self.frame(chunk=True)) + envelope["message_chunk_base64"] = "not@@base64" + self.assert_preserved(encode(envelope)) + + def test_oversized_input_unchanged(self): + raw = self.frame() + out = rewrite_backend_frame(raw, Policy(enabled=True, max_wire_bytes=64)) + self.assertFalse(out.changed) + self.assertEqual(out.text, raw) + + def test_oversized_output_unchanged(self): + raw = self.frame() + out = rewrite_backend_frame(raw, Policy(enabled=True, max_wire_bytes=len(raw.encode()))) + self.assertFalse(out.changed) + self.assertEqual(out.reason, "output_size_limit") + + def test_invalid_policy_rejected(self): + for providers in (("",), ("a", "a"), (1,), ["a"]): + with self.subTest(providers=providers): + with self.assertRaises(ValueError): + Policy(enabled=True, providers=providers) + + def test_synthetic_sqlite_visibility_and_no_mutation(self): + # Synthetic fixture, NOT the user's database and NOT native Codex. + db = sqlite3.connect(":memory:") + try: + db.execute("CREATE TABLE threads (id INTEGER, model_provider TEXT)") + db.executemany("INSERT INTO threads VALUES (?, ?)", [(i, "openai") for i in range(5200)]) + db.execute("INSERT INTO threads VALUES (5200, 'opencodex')") + db.execute("INSERT INTO threads VALUES (5201, 'unrelated')") + before = hashlib.sha256("\n".join(db.iterdump()).encode()).hexdigest() + self.assertEqual(db.execute("SELECT count(*) FROM threads WHERE model_provider = 'opencodex'").fetchone()[0], 1) + providers = extract_message(decode(self.result().text))["params"]["modelProviders"] + placeholders = ",".join("?" for _ in providers) + scoped = db.execute(f"SELECT count(*) FROM threads WHERE model_provider IN ({placeholders})", providers).fetchone()[0] + self.assertEqual(scoped, 5201) + self.assertEqual(db.execute("SELECT count(*) FROM threads").fetchone()[0], 5202) + after = hashlib.sha256("\n".join(db.iterdump()).encode()).hexdigest() + self.assertEqual(before, after) + finally: + db.close() + +if __name__ == "__main__": + unittest.main(verbosity=2) diff --git a/src/codex/user-identity.ts b/src/codex/user-identity.ts index c70b8ee31c0..15fe18c7e3a 100644 --- a/src/codex/user-identity.ts +++ b/src/codex/user-identity.ts @@ -19,6 +19,7 @@ import { isAbsolute, join, resolve } from "node:path"; import { resolveTrustedWindowsPowerShellExe } from "../lib/windows-elevation"; import { WINDOWS_PRINCIPAL_LOOKUP_TIMEOUT_MS } from "../lib/windows-user-principal"; +import { resolveWindowsLocalAppData } from "./windows-local-appdata"; import type { ResolveCodexCoordinatorDatabasePath, @@ -209,10 +210,11 @@ export function decodeWindowsIdentityPowerShellOutputForTests(output: Uint8Array /** * Per-process memo for the Windows lookups. * - * Both values -- the effective token's SID and its known-folder local AppData -- + * Both helper values -- the effective token's SID and its known-folder local AppData -- * are fixed for the lifetime of a process: neither can change without a new logon * token, and the lookups deliberately ignore the environment, so nothing a caller - * does between two calls can alter the answer. Each call otherwise spawns a fresh + * does between two calls can alter the answer. The native x64 known-folder path + * avoids this helper entirely. Each fallback call otherwise spawns a fresh * PowerShell, roughly 150ms for the SID and 310ms for the folder, and the * coordinator asks for both on every config write and lock acquisition. That cost * pushed real multi-process injection tests past their budget while proving @@ -265,6 +267,15 @@ function resolveWindowsSid(): string { return sid.toUpperCase(); } +function windowsLocalAppData(): string { + try { + return resolveWindowsLocalAppData(() => powershellValue(windowsLocalAppDataExpression())); + } catch (cause) { + if (cause instanceof CodexUserIdentityRefusal) throw cause; + refuse("Windows LocalAppData resolution failed.", cause); + } +} + export const resolveEffectiveUserIdentity: ResolveEffectiveUserIdentity = () => { if (process.platform === "win32") { return { platform: "win32", sid: resolveWindowsSid() }; @@ -374,7 +385,7 @@ export function probeCodexCoordinatorNamespace(identity: UserIdentity): Coordina } if (!SID_PATTERN.test(identity.sid)) refuse("The coordinator identity contains an invalid SID."); - const localAppData = powershellValue(windowsLocalAppDataExpression()); + const localAppData = windowsLocalAppData(); if (!isAbsolute(localAppData)) refuse("Windows LocalAppData resolution returned a relative path."); const root = resolve(localAppData, "OpenCodex", "Runtime", "v1", identity.sid.toUpperCase()); let entry; @@ -404,10 +415,10 @@ export function probeCodexCoordinatorNamespace(identity: UserIdentity): Coordina function resolveWindowsRuntimeRoot(identity: Extract): string { if (!SID_PATTERN.test(identity.sid)) refuse("The coordinator identity contains an invalid SID."); - const localAppData = powershellValue(windowsLocalAppDataExpression()); + const localAppData = windowsLocalAppData(); if (!isAbsolute(localAppData)) refuse("Windows LocalAppData resolution returned a relative path."); - // The SID and known-folder values come from the effective token/.NET OS APIs, + // The SID and known-folder values come from effective-token OS APIs, // never USERPROFILE or LOCALAPPDATA. WP11 adds descriptor/reparse/ACL checks at // the stable-database open boundary where those checks can cover SQLite too. const root = resolve(localAppData, "OpenCodex", "Runtime", "v1", identity.sid.toUpperCase()); diff --git a/src/codex/windows-local-appdata.ts b/src/codex/windows-local-appdata.ts new file mode 100644 index 00000000000..552562d1605 --- /dev/null +++ b/src/codex/windows-local-appdata.ts @@ -0,0 +1,84 @@ +import { dlopen, ptr, read, type Pointer } from "bun:ffi"; +import { win32 } from "node:path"; +import { resolveTrustedWindowsSystemDirectory } from "../lib/windows-elevation"; + +// FOLDERID_LocalAppData: GUID fields Data1/Data2/Data3 are little-endian. +const LOCAL_APPDATA_ID = Buffer.from("8527b3f1ba6fcf4f9d557b8e7f157091", "hex"); +const KF_FLAG_DEFAULT_PATH = 0x400; +const MAX_PATH_CODE_UNITS = 32_768; + +export interface WindowsKnownFolderApi { + getPath(id: Uint8Array, flags: number, token: null, output: BigUint64Array): number; + readCodeUnit(address: number, byteOffset: number): number; + free(address: number): void; +} + +/** Same current-token/default-path API as the PowerShell fallback, without Add-Type. */ +export function readWindowsLocalAppData(api: WindowsKnownFolderApi): string { + const output = new BigUint64Array(1); + let address = 0; + try { + const hr = api.getPath(LOCAL_APPDATA_ID, KF_FLAG_DEFAULT_PATH, null, output); + address = Number(output[0]); + if (hr !== 0 || !Number.isSafeInteger(address) || address <= 0) { + throw new Error("Windows LocalAppData known-folder lookup failed."); + } + const units: number[] = []; + for (let index = 0; index < MAX_PATH_CODE_UNITS; index += 1) { + const unit = api.readCodeUnit(address, index * 2); + if (unit === 0) { + const path = String.fromCharCode(...units); + if (!win32.isAbsolute(path) || !/^[a-z]:\\|^\\\\/i.test(path)) { + throw new Error("Windows LocalAppData known-folder lookup returned an invalid path."); + } + return path; + } + units.push(unit); + } + throw new Error("Windows LocalAppData known-folder lookup returned an unterminated path."); + } finally { + // SHGetKnownFolderPath requires CoTaskMemFree even on a failed HRESULT. + if (address > 0 && Number.isSafeInteger(address)) api.free(address); + } +} + +let nativeLookup: (() => string) | null | undefined; + +function loadNativeLookup(): (() => string) | null { + if (nativeLookup !== undefined) return nativeLookup; + if (process.platform !== "win32" || process.arch !== "x64") return null; + try { + const systemDirectory = resolveTrustedWindowsSystemDirectory(); + const shell = dlopen(win32.join(systemDirectory, "shell32.dll"), { + SHGetKnownFolderPath: { args: ["ptr", "u32", "ptr", "ptr"], returns: "i32" }, + }); + let ole; + try { + ole = dlopen(win32.join(systemDirectory, "ole32.dll"), { + CoTaskMemFree: { args: ["ptr"], returns: "void" }, + }); + } catch (error) { + shell.close(); + throw error; + } + const api: WindowsKnownFolderApi = { + getPath: (id, flags, token, output) => shell.symbols.SHGetKnownFolderPath( + ptr(id), flags, token, ptr(output), + ), + readCodeUnit: (address, offset) => read.u16(address as Pointer, offset), + free: address => { ole.symbols.CoTaskMemFree(address as Pointer); }, + }; + nativeLookup = () => readWindowsLocalAppData(api); + } catch { + nativeLookup = null; + } + return nativeLookup; +} + +/** Unsupported FFI keeps the bounded trusted helper; an OS API failure still refuses. */ +export function resolveWindowsLocalAppData( + powershellLookup: () => string, + lookup: (() => string) | null = loadNativeLookup(), +): string { + return lookup ? lookup() : powershellLookup(); +} diff --git a/src/service/logging.ts b/src/service/logging.ts index ce91623717d..31eff046bf6 100644 --- a/src/service/logging.ts +++ b/src/service/logging.ts @@ -1,24 +1,81 @@ -import { closeSync, fstatSync, ftruncateSync, openSync, readSync, renameSync, writeSync } from "node:fs"; +import { closeSync, constants, fstatSync, fsyncSync, ftruncateSync, lstatSync, openSync, readSync, realpathSync, renameSync, statSync, writeSync, type BigIntStats } from "node:fs"; import { formatWithOptions } from "node:util"; -import { dirname } from "node:path"; +import { basename, dirname, join } from "node:path"; import { recordOwnedConfigPath } from "../lib/config-ownership"; export const SERVICE_LOG_MAX_BYTES = 20 * 1024 * 1024; export const SERVICE_LOG_CHECK_INTERVAL_MS = 60_000; +function regularLeaf(path: string): BigIntStats | undefined { + try { + const info = lstatSync(path, { bigint: true }); + if (!info.isFile() || info.nlink !== 1n || info.ino === 0n) throw new Error("Unsafe service log leaf"); + return info; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === "ENOENT") return undefined; + throw error; + } +} + +function sameFile(left: BigIntStats | undefined, right: BigIntStats | undefined): boolean { + return left !== undefined && right !== undefined && left.dev === right.dev && left.ino === right.ino; +} + +function matchesDescriptor(path: string, fd: number, expected: BigIntStats): boolean { + const info = fstatSync(fd, { bigint: true }); + return info.isFile() && info.nlink === 1n && sameFile(expected, info) + && sameFile(info, regularLeaf(path)); +} + /** Preserve the manager's open append descriptor; retain at most two bounded archives. */ -export function rotateServiceLog(path: string, maxBytes = SERVICE_LOG_MAX_BYTES): boolean { +export function rotateServiceLog(requestedPath: string, maxBytes = SERVICE_LOG_MAX_BYTES): boolean { let source: number | undefined; let archive: number | undefined; try { if (!Number.isSafeInteger(maxBytes) || maxBytes <= 0) return false; - source = openSync(path, "r+"); - const size = fstatSync(source).size; - if (size <= maxBytes) return false; - try { renameSync(`${path}.1`, `${path}.2`); } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") return false; + // Resolve only the parent: exclusive creation through a Windows junction + // can report EEXIST for a missing leaf. Never resolve away a leaf symlink. + const parent = dirname(requestedPath); + const directory = statSync(parent, { bigint: true }); + if (!directory.isDirectory() || directory.ino === 0n) return false; + const canonicalParent = realpathSync(parent); + const path = join(canonicalParent, basename(requestedPath)); + const sameDirectory = () => { + const alias = statSync(parent, { bigint: true }); + const canonical = statSync(canonicalParent, { bigint: true }); + return alias.isDirectory() && canonical.isDirectory() + && sameFile(directory, alias) && sameFile(directory, canonical); + }; + const matches = (suffix: string, fd: number, expected: BigIntStats) => sameDirectory() + && matchesDescriptor(`${path}${suffix}`, fd, expected) + && matchesDescriptor(`${requestedPath}${suffix}`, fd, expected); + if (!sameDirectory()) return false; + const original = regularLeaf(requestedPath); + if (!original) return false; + // Windows does not expose O_NOFOLLOW: descriptor/leaf identity checks still + // apply there. Do not reject legitimate junctions in the home path. + const guardedFlags = process.platform === "win32" ? 0 : constants.O_NOFOLLOW | constants.O_NONBLOCK; + source = openSync(path, constants.O_RDWR | guardedFlags); + if (!matches("", source, original)) return false; + const size = Number(fstatSync(source, { bigint: true }).size); + if (!Number.isSafeInteger(size) || size <= maxBytes) return false; + const previous = regularLeaf(`${path}.1`); + const oldest = regularLeaf(`${path}.2`); + if (!matches("", source, original)) return false; + if (previous) { + if (!sameFile(previous, regularLeaf(`${path}.1`))) return false; + const currentOldest = regularLeaf(`${path}.2`); + if (oldest ? !sameFile(oldest, currentOldest) : currentOldest !== undefined) return false; + if (!sameDirectory()) return false; + renameSync(`${path}.1`, `${path}.2`); + if (!sameFile(previous, regularLeaf(`${path}.2`))) return false; } - archive = openSync(`${path}.1`, "w", 0o600); + // Never truncate an existing archive, even if another rotation or a path + // replacement wins the gap after rename. Failure leaves the source intact. + if (!sameDirectory()) return false; + archive = openSync(`${path}.1`, constants.O_WRONLY | constants.O_CREAT | constants.O_EXCL | guardedFlags, 0o600); + const created = fstatSync(archive, { bigint: true }); + if (!matches(".1", archive, created)) return false; // A legacy unbounded file may be huge. Copy only its newest cap-sized tail, // using fixed memory, before truncating the original inode. const buffer = Buffer.alloc(Math.min(64 * 1024, maxBytes)); @@ -28,12 +85,18 @@ export function rotateServiceLog(path: string, maxBytes = SERVICE_LOG_MAX_BYTES) if (read === 0) return false; let written = 0; while (written < read) { + if (!matches(".1", archive, created)) return false; const count = writeSync(archive, buffer, written, read - written); if (count === 0) return false; written += count; } position += read; } + fsyncSync(archive); + // Revalidate both names and open objects at the destructive boundary. These + // checks detect observed replacement; they are not an atomic namespace lock. + if (!matches(".1", archive, created) + || !matches("", source, original)) return false; ftruncateSync(source, 0); return true; } catch { diff --git a/structure/codex-home.md b/structure/codex-home.md index d637ff05b6b..724cde6a422 100644 --- a/structure/codex-home.md +++ b/structure/codex-home.md @@ -377,7 +377,7 @@ separate role. It is not a kernel lock, a cross-host lock, or protection against writer between byte verification and rename. A rolling mix of old writers that do not reserve takeover has the same cooperation limit. -Windows secret-file hardening resolves the effective token SID through an absolute, trusted +Effective-user roots use `src/codex/windows-local-appdata.ts` ([known-folder contract](ops/test-sandbox-cleanup.md#windows-runtime-namespace)). Windows secret-file hardening resolves the effective token SID through an absolute, trusted PowerShell path before granting the owner and removing inherited broad ACL entries. The normal path obtains System32 from `GetSystemDirectoryW`. Windows ARM64 Bun builds that cannot execute `bun:ffi` use a narrower ACL-only fallback to the fixed protected default installation path diff --git a/structure/ops/service-logging.md b/structure/ops/service-logging.md index f7701e07c4d..dba9fc700d2 100644 --- a/structure/ops/service-logging.md +++ b/structure/ops/service-logging.md @@ -16,6 +16,19 @@ place, preserving the service manager's existing append descriptors. Both archives stay bounded even when a legacy active log is already very large. Archive paths join the config cleanup manifest. +Rotation accepts only regular, singly linked active and archive leaves. The +active descriptor must match the inspected leaf's device and inode; POSIX opens +also use no-follow and nonblocking flags. A new archive is created exclusively, +not opened with truncation. Descriptor/leaf identities are checked while copying +and again before truncating the active inode, after flushing the new archive. +Unsafe leaves, observed replacements, and copy or flush failures skip truncation. +Parent directory aliases remain supported: rotation resolves only the parent, +uses its canonical path for exclusive archive creation, and rechecks the original +and canonical directory identities and both leaf paths against the descriptors. +It never resolves the leaf itself to bypass symlink rejection. +These checks are not an atomic +namespace lock: concurrent directory or hardlink changes can race a final check. + The timer is unreferenced and filesystem failures are swallowed. The active log may exceed the threshold between checks. Copy-truncate can lose concurrent writes, and the retained tail can begin partway through a line. There is no @@ -23,4 +36,6 @@ per-write filesystem check and no timer when service logging is inactive. `tests/service/service-log.test.ts` covers service-only timestamps, multiline formatting, startup and periodic checks, bounded generations, failure handling, -and continued writes through an append descriptor opened before rotation. +continued writes through an append descriptor opened before rotation, unsafe +leaves, controlled identity replacements, exclusive archive creation, parent +aliases, and partial-copy/flush failures. diff --git a/structure/ops/test-sandbox-cleanup.md b/structure/ops/test-sandbox-cleanup.md index f855bbde96c..42c7025507d 100644 --- a/structure/ops/test-sandbox-cleanup.md +++ b/structure/ops/test-sandbox-cleanup.md @@ -1,5 +1,17 @@ # Test Sandbox Cleanup +## Windows runtime namespace + +`src/codex/user-identity.ts` keys runtime namespaces on the effective SID and environment-independent +LocalAppData. `src/codex/windows-local-appdata.ts` calls `SHGetKnownFolderPath` in-process on Windows +x64, loading Shell32/Ole32 from the OS-resolved system directory. It uses `KF_FLAG_DEFAULT_PATH` +and a null token, bounds UTF-16 decoding, and frees the allocation even when the API fails. +Unsupported FFI retains the bounded trusted PowerShell helper; native API failures refuse. +`USERPROFILE` and `LOCALAPPDATA` never choose this root. Runtime canonicalization/reparse checks +and the Bun test lock's required ACL hardening remain separate admission checks. + +## Sandbox lifecycle + The batch runner's per-process home isolation and live-home/service-manager guards remain active because the preload installs them before the lock boundary. `tests/preload.ts` resolves cleanup dependencies after home/lock admission and before test cases. Teardown awaits native-main startup diff --git a/tests/codex-integration/codex-user-identity.test.ts b/tests/codex-integration/codex-user-identity.test.ts index 9635bdb5262..9128fb69af5 100644 --- a/tests/codex-integration/codex-user-identity.test.ts +++ b/tests/codex-integration/codex-user-identity.test.ts @@ -15,10 +15,104 @@ import { } from "../../src/codex/user-identity"; import { removeTreeWithRetry } from "../helpers/remove-tree"; import { repoPath } from "../helpers/repo-root"; +import { + readWindowsLocalAppData, + resolveWindowsLocalAppData, + type WindowsKnownFolderApi, +} from "../../src/codex/windows-local-appdata"; let codexHome = ""; let previousHome: string | undefined; +function knownFolderFixture(path = "C:\\Users\\ν•œκΈ€πŸ˜€\\AppData\\Local", hr = 0) { + const freed: number[] = []; + const api: WindowsKnownFolderApi = { + getPath(id, flags, token, output) { + expect(Buffer.from(id).toString("hex")).toBe("8527b3f1ba6fcf4f9d557b8e7f157091"); + expect(flags).toBe(0x400); + expect(token).toBeNull(); + expect(output[0]).toBe(0n); + output[0] = 4096n; + return hr; + }, + readCodeUnit(address, offset) { + expect(address).toBe(4096); + return path.charCodeAt(offset / 2) || 0; + }, + free(address) { freed.push(address); }, + }; + return { api, freed, path }; +} + +test("native known-folder resolution avoids a timed-out PowerShell identity probe", () => { + const fixture = knownFolderFixture(); + let helperCalls = 0; + const timedOutHelper = (): never => { + helperCalls += 1; + throw new Error("Windows effective-account lookup timed out."); + }; + // Toggle only native availability: the failing helper models the 30s Add-Type + // timeout without a wall-clock sleep, on every platform. + expect(() => resolveWindowsLocalAppData(timedOutHelper, null)).toThrow("timed out"); + expect(helperCalls).toBe(1); + expect(resolveWindowsLocalAppData(timedOutHelper, () => readWindowsLocalAppData(fixture.api))) + .toBe(fixture.path); + expect(helperCalls).toBe(1); + expect(fixture.freed).toEqual([4096]); +}); + +test("native known-folder failures refuse and free the OS allocation without helper fallback", () => { + for (const fixture of [knownFolderFixture(undefined, -2147467259), knownFolderFixture("relative")]) { + let fallbackCalls = 0; + expect(() => resolveWindowsLocalAppData( + () => { fallbackCalls += 1; return "C:\\forged-environment"; }, + () => readWindowsLocalAppData(fixture.api), + )).toThrow("known-folder lookup"); + expect(fixture.freed).toEqual([4096]); + expect(fallbackCalls).toBe(0); + } +}); + +test("native known-folder resolution refuses a null result and frees on decoding failure", () => { + const fixture = knownFolderFixture(); + fixture.api.getPath = () => 0; + expect(() => readWindowsLocalAppData(fixture.api)).toThrow("lookup failed"); + expect(fixture.freed).toEqual([]); + + const broken = knownFolderFixture(); + broken.api.readCodeUnit = () => { throw new Error("read failed"); }; + expect(() => readWindowsLocalAppData(broken.api)).toThrow("read failed"); + expect(broken.freed).toEqual([4096]); +}); + +test("native known-folder resolution bounds an unterminated response and frees it", () => { + const fixture = knownFolderFixture(); + let reads = 0; + fixture.api.readCodeUnit = () => { reads += 1; return 65; }; + expect(() => readWindowsLocalAppData(fixture.api)).toThrow("unterminated path"); + expect(reads).toBe(32_768); + expect(fixture.freed).toEqual([4096]); +}); + +test.if(process.platform === "win32" && process.arch === "x64")( + "Windows x64 obtains the real known folder in-process even when the helper cannot start", + () => { + const previous = { USERPROFILE: process.env.USERPROFILE, LOCALAPPDATA: process.env.LOCALAPPDATA }; + try { + process.env.USERPROFILE = "C:\\forged-known-folder-profile"; + process.env.LOCALAPPDATA = "C:\\forged-known-folder-appdata"; + const path = resolveWindowsLocalAppData(() => { throw new Error("PowerShell must not start"); }); + expect(isAbsolute(path)).toBe(true); + expect(path).not.toContain("forged-known-folder"); + } finally { + for (const [key, value] of Object.entries(previous)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + } + }, +); + const CHILD_TIMEOUT_MS = 10_000; const userIdentityModuleUrl = pathToFileURL( repoPath("src", "codex", "user-identity.ts"), diff --git a/tests/service/service-log.test.ts b/tests/service/service-log.test.ts index fe253c29dab..1a6272a7bf1 100644 --- a/tests/service/service-log.test.ts +++ b/tests/service/service-log.test.ts @@ -1,5 +1,6 @@ -import { afterEach, describe, expect, test } from "bun:test"; -import { closeSync, existsSync, mkdirSync, mkdtempSync, openSync, readFileSync, rmSync, statSync, writeFileSync, writeSync } from "node:fs"; +import { afterEach, describe, expect, spyOn, test } from "bun:test"; +import { closeSync, existsSync, linkSync, mkdirSync, mkdtempSync, openSync, readFileSync, realpathSync, renameSync, rmSync, statSync, symlinkSync, writeFileSync, writeSync } from "node:fs"; +import * as fs from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { repoPath, repoRoot } from "../helpers/repo-root"; @@ -7,7 +8,7 @@ import { rotateServiceLog } from "../../src/service/logging"; const roots: string[] = []; function sandbox(): string { - const root = mkdtempSync(join(tmpdir(), "ocx-service-log-")); + const root = realpathSync(mkdtempSync(join(tmpdir(), "ocx-service-log-"))); roots.push(root); return root; } @@ -106,6 +107,219 @@ describe("managed service log", () => { expect(rotateServiceLog(path, 0)).toBe(false); }); + for (const suffix of ["", ".1", ".2"]) { + test(`hardlinked ${suffix || "active"} leaf is rejected without changing any file`, () => { + const root = sandbox(); + const path = join(root, "service.log"); + const target = join(root, "unrelated.txt"); + writeFileSync(target, "unrelated contents"); + for (const leaf of ["", ".1", ".2"]) { + if (leaf === suffix) linkSync(target, `${path}${leaf}`); + else writeFileSync(`${path}${leaf}`, `original ${leaf || "active"}`); + } + const before = ["", ".1", ".2"].map(leaf => readFileSync(`${path}${leaf}`, "utf8")); + expect(rotateServiceLog(path, 4)).toBe(false); + expect(["", ".1", ".2"].map(leaf => readFileSync(`${path}${leaf}`, "utf8"))).toEqual(before); + expect(readFileSync(target, "utf8")).toBe("unrelated contents"); + }); + + test.skipIf(process.platform === "win32")(`symlinked ${suffix || "active"} leaf is rejected`, () => { + const root = sandbox(); + const path = join(root, "service.log"); + const target = join(root, "unrelated.txt"); + writeFileSync(target, "unrelated contents"); + if (suffix) writeFileSync(path, "active contents"); + symlinkSync(target, `${path}${suffix}`); + expect(rotateServiceLog(path, 4)).toBe(false); + expect(readFileSync(target, "utf8")).toBe("unrelated contents"); + if (suffix) expect(readFileSync(path, "utf8")).toBe("active contents"); + }); + } + + test("a non-regular active leaf is rejected", () => { + const path = join(sandbox(), "service.log"); + mkdirSync(path); + expect(rotateServiceLog(path, 4)).toBe(false); + expect(existsSync(`${path}.1`)).toBe(false); + }); + + test("an allowed parent directory alias still rotates", () => { + const root = sandbox(); + const actual = join(root, "actual"); + const alias = join(root, "alias"); + mkdirSync(actual); + symlinkSync(actual, alias, process.platform === "win32" ? "junction" : "dir"); + const path = join(alias, "service.log"); + writeFileSync(path, "oldest latest"); + expect(rotateServiceLog(path, 6)).toBe(true); + expect(readFileSync(join(actual, "service.log.1"), "utf8")).toBe("latest"); + }); + + test("parent aliases use canonical exclusive creation without weakening flags", () => { + const root = sandbox(); + const actual = join(root, "actual"); + const alias = join(root, "alias"); + mkdirSync(actual); + symlinkSync(actual, alias, process.platform === "win32" ? "junction" : "dir"); + const path = join(alias, "service.log"); + writeFileSync(path, "oldest latest"); + const open = fs.openSync; + let exclusiveOpens = 0; + const hook = spyOn(fs, "openSync").mockImplementation(((file: fs.PathLike, flags: number, ...args: unknown[]) => { + if (String(file).endsWith(".1")) { + exclusiveOpens++; + expect(file).toBe(join(realpathSync(actual), "service.log.1")); + expect(flags & fs.constants.O_EXCL).toBe(fs.constants.O_EXCL); + expect(flags & fs.constants.O_CREAT).toBe(fs.constants.O_CREAT); + expect(flags & fs.constants.O_TRUNC).toBe(0); + } + return Reflect.apply(open, fs, [file, flags, ...args]); + }) as typeof fs.openSync); + try { expect(rotateServiceLog(path, 6)).toBe(true); } + finally { hook.mockRestore(); } + expect(exclusiveOpens).toBe(1); + expect(readFileSync(join(actual, "service.log.1"), "utf8")).toBe("latest"); + }); + + for (const boundary of ["resolve", "copy"] as const) { + test(`parent alias retargeted during ${boundary} preserves both source files`, () => { + const root = sandbox(); + const actual = join(root, "actual"); + const other = join(root, "other"); + const alias = join(root, "alias"); + mkdirSync(actual); mkdirSync(other); + writeFileSync(join(actual, "service.log"), "original contents"); + writeFileSync(join(other, "service.log"), "replacement contents"); + symlinkSync(actual, alias, process.platform === "win32" ? "junction" : "dir"); + const method = boundary === "resolve" ? "realpathSync" : "readSync"; + const original = fs[method]; + let changed = false; + const hook = spyOn(fs, method).mockImplementation(((...args: unknown[]) => { + const result = Reflect.apply(original, fs, args); + if (!changed) { + changed = true; + renameSync(alias, `${alias}.saved`); + symlinkSync(other, alias, process.platform === "win32" ? "junction" : "dir"); + } + return result; + }) as typeof original); + try { expect(rotateServiceLog(join(alias, "service.log"), 4)).toBe(false); } + finally { hook.mockRestore(); } + expect(changed).toBe(true); + expect(readFileSync(join(actual, "service.log"), "utf8")).toBe("original contents"); + expect(readFileSync(join(other, "service.log"), "utf8")).toBe("replacement contents"); + expect(existsSync(join(other, "service.log.1"))).toBe(false); + }); + } + + test("an archive inserted through a parent alias still fails exclusive creation", () => { + const root = sandbox(); + const actual = join(root, "actual"); + const alias = join(root, "alias"); + mkdirSync(actual); + symlinkSync(actual, alias, process.platform === "win32" ? "junction" : "dir"); + const path = join(alias, "service.log"); + writeFileSync(path, "original contents"); + const open = fs.openSync; + const hook = spyOn(fs, "openSync").mockImplementation(((file: fs.PathLike, ...args: unknown[]) => { + if (file === join(actual, "service.log.1")) writeFileSync(`${path}.1`, "inserted contents"); + return Reflect.apply(open, fs, [file, ...args]); + }) as typeof fs.openSync); + try { expect(rotateServiceLog(path, 4)).toBe(false); } + finally { hook.mockRestore(); } + expect(readFileSync(path, "utf8")).toBe("original contents"); + expect(readFileSync(`${path}.1`, "utf8")).toBe("inserted contents"); + }); + + test("replacement between leaf inspection and source open is not truncated", () => { + const path = join(sandbox(), "service.log"); + writeFileSync(path, "original contents"); + const open = fs.openSync; + const hook = spyOn(fs, "openSync").mockImplementation(((file: fs.PathLike, ...args: unknown[]) => { + if (file === path) { + renameSync(path, `${path}.saved`); + writeFileSync(path, "replacement contents"); + } + return Reflect.apply(open, fs, [file, ...args]); + }) as typeof fs.openSync); + try { expect(rotateServiceLog(path, 4)).toBe(false); } + finally { hook.mockRestore(); } + expect(readFileSync(path, "utf8")).toBe("replacement contents"); + expect(readFileSync(`${path}.saved`, "utf8")).toBe("original contents"); + expect(existsSync(`${path}.1`)).toBe(false); + }); + + test("an archive inserted after rename is not opened destructively", () => { + const path = join(sandbox(), "service.log"); + writeFileSync(path, "active contents"); + writeFileSync(`${path}.1`, "previous archive"); + const open = fs.openSync; + const hook = spyOn(fs, "openSync").mockImplementation(((file: fs.PathLike, ...args: unknown[]) => { + if (file === `${path}.1`) writeFileSync(`${path}.1`, "inserted contents"); + return Reflect.apply(open, fs, [file, ...args]); + }) as typeof fs.openSync); + try { expect(rotateServiceLog(path, 4)).toBe(false); } + finally { hook.mockRestore(); } + expect(readFileSync(path, "utf8")).toBe("active contents"); + expect(readFileSync(`${path}.1`, "utf8")).toBe("inserted contents"); + expect(readFileSync(`${path}.2`, "utf8")).toBe("previous archive"); + }); + + for (const change of ["replace-source", "link-source", "replace-archive", "link-archive", "read-failure"] as const) { + test(`copy boundary refuses ${change} and preserves the source`, () => { + const path = join(sandbox(), "service.log"); + writeFileSync(path, "original contents"); + const read = fs.readSync; + let changed = false; + const hook = spyOn(fs, "readSync").mockImplementation(((...args: unknown[]) => { + const count = Reflect.apply(read, fs, args); + if (!changed) { + changed = true; + const leaf = change.endsWith("archive") ? `${path}.1` : path; + if (change.startsWith("replace")) { + renameSync(leaf, `${leaf}.saved`); + writeFileSync(leaf, "replacement contents"); + } else if (change.startsWith("link")) linkSync(leaf, `${leaf}.linked`); + else throw new Error("synthetic copy failure"); + } + return count; + }) as typeof fs.readSync); + try { expect(rotateServiceLog(path, 4)).toBe(false); } + finally { hook.mockRestore(); } + expect(readFileSync(change === "replace-source" ? `${path}.saved` : path, "utf8")).toBe("original contents"); + if (change.startsWith("replace")) { + expect(readFileSync(change.endsWith("archive") ? `${path}.1` : path, "utf8")).toBe("replacement contents"); + } + if (change === "link-archive") expect(readFileSync(`${path}.1.linked`, "utf8")).toBe(""); + }); + } + + test("a write failure after a partial archive leaves the active log intact", () => { + const path = join(sandbox(), "service.log"); + const contents = "x".repeat(128 * 1024 + 1); + writeFileSync(path, contents); + const write = fs.writeSync; + let writes = 0; + const hook = spyOn(fs, "writeSync").mockImplementation(((...args: unknown[]) => { + if (++writes === 2) throw new Error("synthetic disk failure"); + return Reflect.apply(write, fs, args); + }) as typeof fs.writeSync); + try { expect(rotateServiceLog(path, 128 * 1024)).toBe(false); } + finally { hook.mockRestore(); } + expect(writes).toBe(2); + expect(readFileSync(path, "utf8")).toBe(contents); + expect(statSync(`${path}.1`).size).toBe(64 * 1024); + }); + + test("archive flush failure preserves the active source", () => { + const path = join(sandbox(), "service.log"); + writeFileSync(path, "original contents"); + const hook = spyOn(fs, "fsyncSync").mockImplementation(() => { throw new Error("synthetic flush failure"); }); + try { expect(rotateServiceLog(path, 4)).toBe(false); } + finally { hook.mockRestore(); } + expect(readFileSync(path, "utf8")).toBe("original contents"); + }); + test("rotation checks at startup and periodically only for service processes", async () => { const path = join(sandbox(), "service.log"); const child = Bun.spawn([process.execPath, "--eval", `