-
Notifications
You must be signed in to change notification settings - Fork 0
secure remote app server connection
Issue #149 / Issue #151 / Issue #150 / Issue #152 / Issue #153
Complete the secure remote-connection behavior tracked by Issue #151. The Extension continues to communicate with its local Worker. The Worker selects local stdio or an explicitly applied WebSocket profile. This design completes endpoint policy, connection and token ownership, health diagnostics, bounded read-only overload retry, profile freshness, and local restart versus remote reconnect behavior. The remote server remains externally managed. Remote mode remains Preview until the separate account and principal state isolation in Issue #152 is complete.
Issue #150 supplies the pinned Codex CLI 0.159.1 contract (0.155.1 regression baseline), shared JSON-RPC dispatch, and connection-generation behavior. Issue #152 owns complete path mapping and account/authentication-principal/root state partitioning. Issue #153 owns automatic reconnect and history recovery, retained drafts, and uncertain mutation handling. This design does not claim those later guarantees.
| Component | Existing surface | Design change |
|---|---|---|
Contracts (netstandard2.0) |
Shared Extension/Worker contract types | Add a pure RemoteEndpointPolicy and typed validation result. It may use System.Uri and System.Net.IPAddress; it has no Protocol dependency. |
| Protocol |
TransportPolicies, JsonRpcRetryPolicy, SendIdempotentRequestAsync, WebSocketTransportSecurityPolicy
|
Make WebSocket security policy call the shared endpoint and token-format policies. Replace the caller-supplied retry boolean with the allowlist-enforcing SendReadOnlyRequestAsync. Protocol references Contracts; Contracts never references Protocol. |
| Worker |
WorkerRpcService, File.ReadAllText, transport construction, Worker diagnostics |
Add the connection gate checks, bounded token-file reader, worker-lifetime networking factory, typed connection diagnostics, and idle-generation watchdog. Replace unbounded File.ReadAllText token loading. |
| Extension |
RemoteProfilePresentation, bridge operations and connection-target flyout |
Remove duplicate URI validation and consume the shared policy. Keep unsaved editor state separate from the exact profile snapshot currently applied to the Worker. |
| Secret redaction and sinks |
ISecretRedactor, queued diagnostics, Worker/Extension output |
Add reference-counted secret leases and redact at production before any callback queue or I/O. Retire callbacks before releasing a token lease. |
No new runtime package is required. Networking stays in the Worker. There is no new telemetry sink.
The shared endpoint policy is pure validation and performs no DNS, file access, network request, or mutation. It accepts an absolute wss URI for a remote host and accepts ws only for syntactically eligible loopback forms. The policy does not claim DNS verification: the Worker resolves exact localhost before reading the token and requires every answer to be loopback. Reject other schemes, relative URIs, user information, query strings, and fragments. A routing path is allowed. Reject unspecified literal destinations for every scheme, including wss; they are not valid remote destinations. No policy or UI option can disable TLS validation or follow redirects for a credential-bearing WebSocket handshake.
For ws, accept only these loopback forms:
- The exact hostname
localhost, compared ordinally without regard to case. Reject a terminal-dot spelling and every subdomain oflocalhost, even if DNS resolves it to loopback. - A canonical IPv4 literal whose first octet is 127.
- The IPv6 loopback literal.
- An IPv4-mapped IPv6 literal only when its embedded IPv4 address is loopback.
Reject unspecified addresses, DNS aliases, and every other hostname or address for ws. Canonical literal parsing must reject alternate or ambiguous textual encodings. For the exact localhost name, the Worker resolves it before reading the token file and requires every resolved address to be loopback. Only for that verified exact-localhost authority, on its direct connection attempt, pin the connection to the verified loopback address set with ConnectCallback, trying each verified address in resolver order until one connects (a server may listen on only ::1 or only 127.0.0.1), while preserving the URI authority for TLS/Host processing. Do not apply that callback to remote wss IP literals or to a proxy target. This prevents a second DNS resolution from changing the destination. No DNS lookup is used to authorize a DNS alias. Only verified-loopback literal endpoints and exact-localhost endpoints bypass proxies; a remote wss endpoint whose host is an IP literal still follows the configured proxy. Health probes derived from those verified loopback endpoints also bypass proxies; all other probes use the captured proxy policy.
A Worker-lifetime networking factory supplies a shared HttpMessageInvoker to WebSocket connect and HTTP diagnostics. It uses the same captured HttpClient.DefaultProxy resolver for both paths, honoring HTTP_PROXY, HTTPS_PROXY, ALL_PROXY, NO_PROXY, then Windows user proxy settings; only Worker-verified loopback literals and exact-localhost endpoints bypass it. A remote wss endpoint is subject to the configured proxy even when its host is an IP literal. Map ws to HTTP and wss to HTTPS for proxy resolution. Both paths disable cookies, default credentials, redirects, and any forwarding of the App Server bearer token to a proxy; they retain platform TLS defaults. Proxy authentication is permitted only when explicitly configured by the user's existing proxy environment or system settings. Add no proxy credential UI or extension-owned proxy settings. Do not fall back to a direct connection after proxy failure, and do not log proxy configuration. A new Worker captures the effective proxy settings and inherits the Visual Studio process environment. Restart Visual Studio when changes to the operating-system environment must be inherited; restart the Worker to refresh system proxy settings. When the invoker is supplied, WebSocket options contain only transport-specific settings.
Save validates a nonempty unique display name, endpoint policy, both roots, and—when enabled—a nonempty syntactically valid absolute local token-file path. Save never opens or reads the token file. File existence, readability, and token contents are checked by the Worker during explicit connection. The Worker independently rejects missing local/server roots before connecting; existing attachment checks reject a local path outside the configured local root before turn/start. Health diagnosis requires a saved enabled profile without pending editor changes and valid endpoint metadata, but does not read its token file or need a token: it sends unauthenticated requests only to fixed health routes. This keeps health checks an explicit opt-in for a known profile while avoiding token access and arbitrary endpoint input.
The Worker and Extension retain the active applied profile's display name, canonical non-secret metadata fingerprint (endpoint, token-file path, roots, name, enabled state), and connection generation in memory. Saving, renaming, disabling, or deleting a profile does not change the live socket. It only changes future eligibility. Token-file content rotation does not change the metadata fingerprint.
Before an explicit remote reconnect, the Extension reloads current settings by the applied profile name and requires an enabled profile whose complete metadata fingerprint matches the applied snapshot. It also rejects a reconnect targeting a profile with unsaved editor changes. On mismatch or removal, report ProfileChanged or ProfileUnavailable and require an explicit apply of a currently saved profile or an explicit switch to local. Never reconnect with stale options or silently fall back to local.
Serialize all same-instance profile/settings mutations—selection persistence, Save, rename, enable/disable, delete, and explicit switch to local—together with snapshot validation and Extension RPC dispatch through the same operation gate. Carry an immutable RemoteReconnectRequest containing the profile snapshot and expected generation. Before any token read, socket stop, or send, the Worker connection gate revalidates the enabled metadata against its bound options and expected generation. The active generation alone may publish state or diagnostic results. This is an in-process freshness guarantee; it does not claim the cross-process registry, cache, account, or principal isolation assigned to Issue #152.
The token path must be an absolute local path. Reject UNC paths, network drives, device namespaces, and other non-local path forms. Resolve symlinks and junctions and require the final target to remain local. Apply the size bound to the opened stream. Do not inspect, change, or warn about file ACLs; the operator is responsible for keeping the local token file private.
Immediately before each explicit handshake, the Worker reads at most 16 KiB asynchronously using strict UTF-8, accepts an optional UTF-8 BOM, strips it, and trims only outer whitespace. Require a single ASCII RFC 6750 bearer-token value of at least 32 characters. Reject empty, short, oversized, malformed-encoding, non-ASCII, embedded-whitespace, or control-character values, plus missing files, directories, and read failures. Do not cache contents or install a watcher. Rotation is observed on the next explicit reconnect; it never starts a connection or replays an operation on its own.
Send the value only as the WebSocket handshake bearer Authorization header. The Extension, settings, contract DTOs, Remote UI, transcript, and diagnostics receive only the token-file path, never token contents. ISecretRedactor supports RegisterSecret(string) -> IDisposable; each Worker uses reference-counted leases so duplicate registration cannot remove another active lease. Register immediately after reading and before any handshake work. Redact diagnostic text at production before enqueueing or writing it. Queues and callbacks receive only already-redacted strings or categorical errors. While holding the transition gate during cleanup, unsubscribe or suppress callbacks and dispose the candidate connection. Never await a callback that may need the same gate while the gate is held. Track queued close callbacks and drain them after releasing the gate, then release the lease; callback-owned leases are an equivalent option if they preserve the same secret lifetime. Do not retain a raw exception or header string beyond the lease lifetime.
The current output sinks are Worker/Extension stderr, the shared diagnostics.log, and the Extension's Codex Diagnostics Output Channel. Both the Worker and Extension writers append to the same diagnostics.log, and each writer redacts immediately before appending. Each stderr and Output Channel writer also redacts before writing. There is no telemetry sink and no new diagnostic sink in this design. Never emit raw headers, token values, health bodies, proxy configuration, or unsanitized exception chains.
One monotonic 45-second budget covers explicit remote connection startup. Its bounded stages are token read (at most 5 seconds), WebSocket handshake (at most 15 seconds), initialize (at most 15 seconds), and the startup account/read (at most 15 seconds); each stage receives no more than the remaining overall time. Preserve cancellation as cancellation. A failure after candidate socket creation retires that candidate, completes its pending work, and cannot publish over a newer generation. Account-read failure is distinguishable from initialize failure. A successfully returned SignedOut account state remains connected and permits the existing sign-in action. An account-read error classified as Unavailable after initialization does not by itself degrade the RPC connection; preserve the Worker's existing Ready state. A transport close, caller cancellation, or overall startup deadline fails the startup attempt and retires the candidate connection after cleaning up pending work.
A Worker-owned RemoteConnectionDiagnostics service performs health diagnosis independently of the App Server connection. It uses the networking factory, derives HTTP from ws and HTTPS from wss, preserves authority and port, and requests only the fixed /healthz and /readyz routes. Run both unauthenticated GETs concurrently under one five-second monotonic budget. Send no Authorization or Origin header; do not inherit authentication headers. Use ResponseHeadersRead, classify the status, then dispose the response without reading its body. Redirects and cookies are disabled. A health failure never prevents a valid RPC connection.
For a WebSocket URI with a non-root routing path, probes still go to the authority root. Return HealthScope = AuthorityRoot and RouteCoverage = Unverified. The UI says the diagnostic describes the root listener; it does not claim that the routed application path is healthy. A successful health response never proves JSON-RPC availability, authentication, or feature support.
Typed results contain probe state, optional numeric HTTP status, duration, observation time, and fixed or redacted reason text only. A health check never starts, stops, reconnects, initializes, refreshes account credentials, or sends a mutation; it cannot change Worker connection state. It is available for the selected saved enabled profile independently of whether that profile is active. An inactive profile displays RPC Not connected, not an inference from health.
The UI keeps the selected diagnostic profile distinct from the active target. Clear or cancel the result when selection or saved endpoint metadata changes, and discard stale completion after another check, a connection change, generation change, or disposal. Disable duplicate checks while one is running and expose status through the existing polite live region. Sanitize all dynamic presentation through SafeMarkdownService.
The transport reports every valid parsed inbound JSON-RPC response, notification, and server request to the watchdog as activity, using a monotonic inbound-activity sequence. After 30 seconds without activity on an active remote connection, capture the sequence and issue one account/read with refreshToken: false and a 10-second deadline, without overload retry. Any valid parsed inbound message resets the silence count. At timeout, if the sequence advanced since the probe started, do not issue a second probe; reset the count and return to the normal 30-second idle wait. Only when no inbound activity occurred during the first probe, issue a second read immediately with a 10-second deadline. Close only the captured socket and publish Degraded for that still-current generation only if the second probe also times out with no activity since it started. Do not auto-reconnect or replay a mutation. A remote close superseded by a newer generation cannot overwrite its state. A successfully returned SignedOut account state is not a dead-peer result.
Classify failures into bounded actionable categories: invalid endpoint, profile changed/unavailable, token file missing/unreadable/invalid, authentication rejected, certificate rejected, DNS/network failure, timeout, RPC initialization failure, account-read failure, and health-route outcome. Preserve the underlying sensitive details only long enough to redact them; the user-visible reason is fixed categorical text.
Set the .NET 8 WebSocket KeepAliveInterval to 30 seconds. This sends unsolicited PONG frames only; it is not proof that the peer is alive, and .NET 8 does not provide KeepAliveTimeout. The idle-generation watchdog detects peer silence.
Replace any API that accepts a caller-supplied idempotency boolean with SendReadOnlyRequestAsync. The method itself validates the exact method allowlist. Beyond allowlist membership, it validates only the refreshToken predicate for account/read and the forceReload predicate for skills/list; it does not require new full-shape validation for the other six known read-only methods. All other methods—including mutations and unknown methods—are sent at most once.
| Allowed method | Retry condition |
|---|---|
account/read |
refreshToken is explicitly boolean false; missing, invalid, or true is not retryable. |
account/rateLimits/read |
Known read-only method; no caller override. |
thread/list |
Known read-only method; no caller override. |
thread/goal/get |
Known read-only method; no caller override. |
model/list |
Known read-only method; no caller override. |
permissionProfile/list |
Known read-only method; no caller override. |
mcpServerStatus/list |
Known read-only method; no caller override. |
skills/list |
Only when forceReload is explicitly boolean false. Missing, invalid, or true is sent once without retry because force reload can clear cache and rescan discovery; do not describe that behavior as durable mutation. |
Verify the skills/list parameter semantics against the pinned upstream codex-rs/app-server/src/request_processors/catalog_processor.rs (skills_list_response), schema codex-rs/app-server-protocol/schema/json/v2/SkillsListParams.json, and test codex-rs/app-server/tests/suite/v2/skills_list.rs before enabling the false-only retry condition. Future history or attachment reads (including thread/read, item/turn listing, and attachment listing) are not implicitly retryable; extend the allowlist only with explicit contract evidence and tests.
Retry only a completed JSON-RPC overload response with code -32001. Do not retry transport errors, authentication/TLS failures, disconnections, timeouts, malformed responses, -32601, other RPC errors, mutations, or unknown methods. Allow at most three retries (four sends total), with base waits of 250, 500, and 1000 ms and uniform ±20% jitter. Validate the policy values and keep them finite and bounded. One monotonic deadline covers all sends and waits, based on the existing per-call timeout; each attempt gets only the remaining time. Cancellation, connection closure, and generation retirement cancel the outstanding request or delay promptly. A retry always uses the original captured generation. Preserve the final original RPC error after exhaustion. Inject time, delay, and jitter sources for deterministic boundary tests.
The current base contract is v16. This change advances the bundled Extension/Worker contract atomically to the next available version (v17 if no intervening change lands). Before merge, rebase this choice on the latest contract and use its next version. A mismatched Extension and Worker fail closed; no compatibility negotiation is added.
Add typed connection target and diagnostic snapshots to Worker status. A snapshot carries local/remote kind, configured display name, metadata fingerprint, and attempt/connection generation, but no token value. Ready, Busy, or WaitingForApproval confirms the active target for that generation. Disconnected, Connecting, or Degraded reports the intended target. Add worker/connection/diagnose and connection-only worker/reconnect operations. Remote reconnect uses the last applied snapshot and reads the current token file. worker/restart is restricted to a Worker-owned local process; it must reject remote state before stopping or sending anything.
Use a typed -32051 WorkerErrorCodes.ConnectionOperationRejected response with one reason: LocalProcessRequired, RemoteConnectionRequired, ProfileUnavailable, ProfileChanged, or StaleGeneration. Do not return free-form paths, endpoints, token-file content, or raw transport exceptions. The local restart action remains unchanged. In degraded state, label the local action Restart local app-server and the remote action Reconnect remote app-server, with matching tooltip, automation name, help text, and command-state notifications. Show a PID only for an actually owned local process.
The connection-target flyout shows the health and ready results separately from the actual RPC state, distinguishes the checked profile from the active target, and explains authority-root coverage for pathful endpoints. It retains mutual exclusion with Usage and History, keyboard access, Escape/Tab behavior, Visual Studio theme resources, accessible names, and live status. A health result does not affect whether Connect or a feature is available.
Update Preview guidance to say that health diagnostics and bounded retry of allowlisted read-only RPCs are available. Keep the outstanding account/principal state-isolation limitation and the distinction between external remote-server ownership and the local Worker-owned socket.
- Confirm the revised Issue #151 design and proposed ADR-012 amendment. Keep this Phase 2 section,
doc/design.mdsection 12, the English/Japanese detailed design, and Wiki plan/index pairs synchronized. Implementation begins after design confirmation. - Extend shared policy/retry components, replace unbounded token I/O, add secret leases and redacted diagnostic production, and make failed-start cleanup deterministic. Keep the existing runtime/SDK/package versions.
- Add typed diagnostics, reconnect refusal reasons, target/generation snapshots, and the .NET 8 liveness watchdog. Allocate the next Worker contract version from the actual merge base; update and package all producers/consumers atomically.
- Wire profile freshness validation, independent health/RPC rows, and local Restart/remote Reconnect labels. Add policy/token/TLS/proxy/read-only retry/lifecycle/serialization/command-state tests, including all excluded argument variants.
- Use the pinned CLI 0.159.1 for a zero-warning Release build, Core/UI tests, schema/contract gates, VSIX manifest/assembly/XAML inspection, and Experimental Instance screenshots. Record observed evidence in
doc/implementation.mdanddoc/task.mdwith Issue #151 tracking; leave completion unchecked until every acceptance criterion has evidence.
| Area | Required evidence |
|---|---|
| Shared endpoint policy | Pure Contracts tests for absolute schemes, syntactic loopback eligibility, exact-localhost matching, terminal-dot and *.localhost rejection, canonical loopback IPv4/IPv6, mapped IPv6 cases, unspecified destinations for all schemes, DNS aliases, user information, query, fragment, routing paths, and non-loopback ws. Protocol, Worker, and Extension consume the same result; DNS verification belongs to the Worker. |
| DNS and proxy safety | Resolve localhost before token read; reject if any answer is non-loopback; pin only the verified exact-localhost authority and connection attempt to the verified address set, trying each address until one connects (IPv6-only and IPv4-only listeners); preserve authority; no DNS-alias authorization or proxy/direct fallback. Verify proxy resolver consistency, bypass only for verified loopback literals/exact localhost (never remote wss literals), health probes bypass only for those same verified local endpoints, no cookies/default credentials/redirects, and no bearer forwarding to proxy. |
| Save and freshness | Save performs metadata-only checks and never reads a token. Selection persistence, Save, rename, enable/disable, delete, and explicit local switch are serialized with snapshot validation and RPC dispatch. Unsaved edits, changed fingerprint, stale generation, and removed profile block reconnect before token read or socket action; explicit apply/local switch recovers. Token-content rotation is read on explicit reconnect. |
| Token and redaction | Local-path and final-link checks; missing/unreadable/directory and 16 KiB boundaries; strict UTF-8 with/without BOM; ASCII token grammar, length, whitespace/control rejection; header only on handshake. Concurrent equal-secret leases, callback queues, exception text, both writers appending to shared diagnostics.log and redacting before append, all existing output sinks, cleanup ordering, and no token value in DTOs/settings/UI/logs. Exercise failed initialize plus close callback with a secret echo; prove gate release avoids deadlock and all output remains redacted. |
| TLS and authentication | Valid trusted TLS, hostname/chain rejection, auth rejection, and cross-origin redirect refusal. Tests use ephemeral certificates without changing machine trust. |
| Health versus RPC | Concurrent probes, one five-second limit, 200 health plus failed upgrade/initialize, health failure plus valid RPC, redirect, timeout, no auth/origin/body, pathful route reported as authority-root/unverified, inactive profile Not connected. |
| Idle watchdog | Transport reports every valid parsed response/notification/server request; 30-second idle, 30-second KeepAliveInterval with unsolicited PONG not treated as liveness, 10-second probes, inbound sequence change during first timeout suppressing the second probe and restarting the idle period, only two silent probes closing the captured socket, late generation ignored, SignedOut remaining connected, and no automatic reconnect/mutation. |
| Retry allowlist | Every listed method; false/missing/invalid/true parameters for account/read and skills/list; -32001 only; four-send maximum; jitter bounds; shared monotonic deadline; cancellation, close, generation retirement; every mutation and unknown method sent once. |
| Connection lifetime and ownership | Startup 45-second bound and each stage cap; cancellation and failure after socket creation; initialize/account-read distinction; Unavailable account result preserving RPC Ready; transport close/caller cancellation/overall deadline cleanup; one active generation; pending client/server requests completed; remote reconnect/dispose leaves external server running; local restart replaces only its owned process; concurrent Workers do not share sockets or leases. |
| Contract and UI | Next contract version applied atomically and mismatch rejected; status DTO serialization; typed error reason tests; local/remote action binding and notifications; sanitized text; health and RPC state independently rendered. |
| Actual display | Experimental Instance screenshots for local/remote states, auth/RPC failure, health results, Dark/Light/High Contrast, narrow width, keyboard/focus, and disabled/running/completed health check. |
| Release | Pinned CLI 0.159.1; zero-warning Release solution build; Core and UI tests; schema cache and method-surface checks; VSIX manifest/assembly/embedded-XAML inspection; git diff --check. |
The tests own their loopback listeners and ephemeral certificates. They do not change user proxy settings, file ACLs, user profiles, normal Visual Studio instances, or system certificate trust. Existing pinned package/build workflows remain in force; no new runtime dependency is introduced.
- Repository source:
src/Codex.AppServer.Protocol/TransportPolicies.cs(containingJsonRpcRetryPolicyandWebSocketTransportSecurityPolicy);src/Codex.VisualStudio.Worker/WorkerRpcService.cs;ISecretRedactorand the existing diagnostics sinks. - Pinned upstream source:
codex-rs/app-server-transport/src/transport/websocket.rsfor health and ready routes;codex-rs/app-server/src/request_processors/catalog_processor.rs(skills_list_response),codex-rs/app-server-protocol/schema/json/v2/SkillsListParams.json, andcodex-rs/app-server/tests/suite/v2/skills_list.rsfor force-reload behavior. - Microsoft Learn: .NET
ClientWebSocket.ConnectAsyncwithHttpMessageInvoker;HttpClient.DefaultProxy;SocketsHttpHandler.ConnectCallback; WebSocket keep-alive and unsolicited PONG behavior.