From 98a62a62ce7e1e6e6100c6e888e9d2bd794378e2 Mon Sep 17 00:00:00 2001 From: Anton Panasenko Date: Tue, 2 Jun 2026 17:01:02 -0700 Subject: [PATCH] feat(app-server): add remote control client management RPCs (#25785) ## Why Remote-control clients need to list and revoke controller-device grants without enabling or enrolling the local relay. These are signed-in account-management operations, so coupling them to websocket, pairing, enrollment, or persisted relay state would prevent clients from managing stale grants from the picker. Related enhancement request: N/A. This adds the Codex app-server surface for the planned upstream environment-scoped revoke endpoint. ## What Changed - Added experimental app-server v2 RPCs: - `remoteControl/client/list` - `remoteControl/client/revoke` - Added picker-oriented protocol types and standard generated schema fixtures. The list response intentionally omits backend account id, enrollment status, and location fields. - Added `app-server-transport/src/transport/remote_control/clients.rs` for environment-scoped GET and DELETE requests. It builds escaped URL path segments, forwards optional pagination query fields, sends ChatGPT auth plus `chatgpt-account-id`, converts RFC3339 `last_seen_at` values to Unix seconds, accepts `204 No Content` revoke responses, and retries once after a `401`. - Extracted shared ChatGPT auth loading and recovery into `app-server-transport/src/transport/remote_control/auth.rs` so websocket, pairing, and client management use the same account-auth boundary. - Retained the configured remote-control base URL on `RemoteControlHandle` and resolve management URLs lazily, preserving deferred validation while relay startup is disabled. - Registered list as `global_shared_read("remote-control-clients")` and revoke as `global("remote-control-clients")`. ## Verification - Added transport coverage proving list and revoke work while relay state is disabled, IDs are escaped, picker-only fields are returned, timestamps are converted, revoke accepts `204`, auth headers are forwarded, `401` retries exactly once, `403` is not retried, and malformed list payloads retain decode context. - Added an app-server integration test proving both JSON-RPC methods work before relay enablement and successful revoke returns `{}`. - Regenerated and validated experimental and standard app-server schema fixtures. --- codex-rs/app-server-protocol/src/export.rs | 38 +- .../src/protocol/common.rs | 35 ++ .../src/protocol/v2/remote_control.rs | 60 +++ .../src/protocol/v2/remote_control_tests.rs | 50 +++ .../src/transport/remote_control/auth.rs | 105 +++++ .../src/transport/remote_control/clients.rs | 307 +++++++++++++++ .../src/transport/remote_control/enroll.rs | 7 +- .../src/transport/remote_control/mod.rs | 37 +- .../src/transport/remote_control/protocol.rs | 73 ++-- .../src/transport/remote_control/tests.rs | 2 + .../remote_control/tests/clients_tests.rs | 359 ++++++++++++++++++ .../src/transport/remote_control/websocket.rs | 96 +---- codex-rs/app-server/README.md | 2 + codex-rs/app-server/src/message_processor.rs | 10 + .../remote_control_processor.rs | 34 ++ .../remote_control_processor_tests.rs | 31 ++ .../tests/common/test_app_server.rs | 21 + .../tests/suite/v2/remote_control.rs | 144 +++++++ 18 files changed, 1281 insertions(+), 130 deletions(-) create mode 100644 codex-rs/app-server-protocol/src/protocol/v2/remote_control_tests.rs create mode 100644 codex-rs/app-server-transport/src/transport/remote_control/auth.rs create mode 100644 codex-rs/app-server-transport/src/transport/remote_control/clients.rs create mode 100644 codex-rs/app-server-transport/src/transport/remote_control/tests/clients_tests.rs diff --git a/codex-rs/app-server-protocol/src/export.rs b/codex-rs/app-server-protocol/src/export.rs index b9c44fe9a..20f923f32 100644 --- a/codex-rs/app-server-protocol/src/export.rs +++ b/codex-rs/app-server-protocol/src/export.rs @@ -39,6 +39,8 @@ use ts_rs::TS; pub(crate) const GENERATED_TS_HEADER: &str = "// GENERATED CODE! DO NOT MODIFY BY HAND!\n\n"; const IGNORED_DEFINITIONS: &[&str] = &["Option<()>"]; const JSON_V1_ALLOWLIST: &[&str] = &["InitializeParams", "InitializeResponse"]; +const EXPERIMENTAL_CLIENT_METHOD_DEPENDENCY_TYPES: &[&str] = + &["RemoteControlClient", "RemoteControlClientsListOrder"]; const SPECIAL_DEFINITIONS: &[&str] = &[ "ClientNotification", "ClientRequest", @@ -554,6 +556,7 @@ fn experimental_method_types() -> HashSet { let mut type_names = HashSet::new(); collect_experimental_type_names(EXPERIMENTAL_CLIENT_METHOD_PARAM_TYPES, &mut type_names); collect_experimental_type_names(EXPERIMENTAL_CLIENT_METHOD_RESPONSE_TYPES, &mut type_names); + collect_experimental_type_names(EXPERIMENTAL_CLIENT_METHOD_DEPENDENCY_TYPES, &mut type_names); type_names } @@ -2132,6 +2135,14 @@ mod tests { fixture_tree.contains_key(Path::new("v2/MockExperimentalMethodResponse.ts")), false ); + assert_eq!( + fixture_tree.contains_key(Path::new("v2/RemoteControlClient.ts")), + false + ); + assert_eq!( + fixture_tree.contains_key(Path::new("v2/RemoteControlClientsListOrder.ts")), + false + ); let mut undefined_offenders = Vec::new(); let mut optional_nullable_offenders = BTreeSet::new(); @@ -2847,6 +2858,11 @@ permissionProfile?: string | null}; flat_v2_bundle_json.contains("MockExperimentalMethodResponse"), false ); + assert_eq!(flat_v2_bundle_json.contains("RemoteControlClient"), false); + assert_eq!( + flat_v2_bundle_json.contains("RemoteControlClientsListOrder"), + false + ); assert_eq!(flat_v2_bundle_json.contains("#/definitions/v2/"), false); assert_eq!( flat_v2_bundle_json.contains("\"title\": \"CodexAppServerProtocolV2\""), @@ -2920,22 +2936,42 @@ permissionProfile?: string | null}; .exists(), false ); + assert_eq!( + output_dir + .join("v2") + .join("RemoteControlClient.json") + .exists(), + false + ); + assert_eq!( + output_dir + .join("v2") + .join("RemoteControlClientsListOrder.json") + .exists(), + false + ); let _cleanup = fs::remove_dir_all(&output_dir); Ok(()) } #[test] - fn generate_json_includes_remote_control_pairing_start_with_experimental_api() -> Result<()> { + fn generate_json_includes_remote_control_methods_with_experimental_api() -> Result<()> { let output_dir = std::env::temp_dir().join(format!("codex_schema_{}", Uuid::now_v7())); fs::create_dir(&output_dir)?; generate_json_with_experimental(&output_dir, /*experimental_api*/ true)?; let client_request_json = fs::read_to_string(output_dir.join("ClientRequest.json"))?; assert!(client_request_json.contains("remoteControl/pairing/start")); + assert!(client_request_json.contains("remoteControl/client/list")); + assert!(client_request_json.contains("remoteControl/client/revoke")); for schema in [ "RemoteControlPairingStartParams.json", "RemoteControlPairingStartResponse.json", + "RemoteControlClientsListParams.json", + "RemoteControlClientsListResponse.json", + "RemoteControlClientsRevokeParams.json", + "RemoteControlClientsRevokeResponse.json", ] { assert!(output_dir.join("v2").join(schema).exists()); } diff --git a/codex-rs/app-server-protocol/src/protocol/common.rs b/codex-rs/app-server-protocol/src/protocol/common.rs index 961a4a227..5a60d8d00 100644 --- a/codex-rs/app-server-protocol/src/protocol/common.rs +++ b/codex-rs/app-server-protocol/src/protocol/common.rs @@ -849,6 +849,18 @@ client_request_definitions! { serialization: global("remote-control-pairing"), response: v2::RemoteControlPairingStartResponse, }, + #[experimental("remoteControl/client/list")] + RemoteControlClientsList => "remoteControl/client/list" { + params: v2::RemoteControlClientsListParams, + serialization: global_shared_read("remote-control-clients"), + response: v2::RemoteControlClientsListResponse, + }, + #[experimental("remoteControl/client/revoke")] + RemoteControlClientsRevoke => "remoteControl/client/revoke" { + params: v2::RemoteControlClientsRevokeParams, + serialization: global("remote-control-clients"), + response: v2::RemoteControlClientsRevokeResponse, + }, #[experimental("collaborationMode/list")] /// Lists collaboration mode presets. CollaborationModeList => "collaborationMode/list" { @@ -1994,6 +2006,29 @@ mod tests { "remote-control-pairing" )) ); + let remote_control_clients_list = ClientRequest::RemoteControlClientsList { + request_id: request_id(), + params: v2::RemoteControlClientsListParams::default(), + }; + assert_eq!( + remote_control_clients_list.serialization_scope(), + Some(ClientRequestSerializationScope::GlobalSharedRead( + "remote-control-clients" + )) + ); + let remote_control_clients_revoke = ClientRequest::RemoteControlClientsRevoke { + request_id: request_id(), + params: v2::RemoteControlClientsRevokeParams { + environment_id: "environment-id".to_string(), + client_id: "client-id".to_string(), + }, + }; + assert_eq!( + remote_control_clients_revoke.serialization_scope(), + Some(ClientRequestSerializationScope::Global( + "remote-control-clients" + )) + ); } #[test] diff --git a/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs b/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs index e5c39a777..f5f004f78 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2/remote_control.rs @@ -62,6 +62,62 @@ pub struct RemoteControlPairingStartResponse { pub expires_at: i64, } +#[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct RemoteControlClientsListParams { + pub environment_id: String, + #[ts(optional = nullable)] + pub cursor: Option, + #[ts(optional = nullable)] + pub limit: Option, + #[ts(optional = nullable)] + pub order: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(rename_all = "camelCase", export_to = "v2/")] +pub enum RemoteControlClientsListOrder { + Asc, + Desc, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct RemoteControlClientsListResponse { + pub data: Vec, + pub next_cursor: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct RemoteControlClient { + pub client_id: String, + pub display_name: Option, + pub device_type: Option, + pub platform: Option, + pub os_version: Option, + pub device_model: Option, + pub app_version: Option, + pub last_seen_at: Option, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct RemoteControlClientsRevokeParams { + pub environment_id: String, + pub client_id: String, +} + +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema, TS)] +#[serde(rename_all = "camelCase")] +#[ts(export_to = "v2/")] +pub struct RemoteControlClientsRevokeResponse {} + #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema, TS)] #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase", export_to = "v2/")] @@ -105,3 +161,7 @@ impl From for RemoteControlDisableRespon } } } + +#[cfg(test)] +#[path = "remote_control_tests.rs"] +mod tests; diff --git a/codex-rs/app-server-protocol/src/protocol/v2/remote_control_tests.rs b/codex-rs/app-server-protocol/src/protocol/v2/remote_control_tests.rs new file mode 100644 index 000000000..ff6b26d23 --- /dev/null +++ b/codex-rs/app-server-protocol/src/protocol/v2/remote_control_tests.rs @@ -0,0 +1,50 @@ +use super::*; +use pretty_assertions::assert_eq; +use serde_json::json; + +#[test] +fn remote_control_clients_list_params_serialize_nullable_optional_fields() { + assert_eq!( + serde_json::to_value(RemoteControlClientsListParams { + environment_id: "env-123".to_string(), + cursor: None, + limit: None, + order: None, + }) + .expect("params should serialize"), + json!({ + "environmentId": "env-123", + "cursor": null, + "limit": null, + "order": null, + }) + ); +} + +#[test] +fn remote_control_clients_list_params_deserialize_camel_case_fields() { + assert_eq!( + serde_json::from_value::(json!({ + "environmentId": "env-123", + "cursor": "cursor-123", + "limit": 10, + "order": "asc", + })) + .expect("params should deserialize"), + RemoteControlClientsListParams { + environment_id: "env-123".to_string(), + cursor: Some("cursor-123".to_string()), + limit: Some(10), + order: Some(RemoteControlClientsListOrder::Asc), + } + ); +} + +#[test] +fn remote_control_clients_revoke_response_serializes_as_empty_object() { + assert_eq!( + serde_json::to_value(RemoteControlClientsRevokeResponse {}) + .expect("response should serialize"), + json!({}) + ); +} diff --git a/codex-rs/app-server-transport/src/transport/remote_control/auth.rs b/codex-rs/app-server-transport/src/transport/remote_control/auth.rs new file mode 100644 index 000000000..dbd231027 --- /dev/null +++ b/codex-rs/app-server-transport/src/transport/remote_control/auth.rs @@ -0,0 +1,105 @@ +use codex_api::SharedAuthProvider; +use codex_login::AuthManager; +use codex_login::UnauthorizedRecovery; +use std::io; +use std::io::ErrorKind; +use std::sync::Arc; +use tokio::sync::watch; +use tracing::info; +use tracing::warn; + +pub(super) struct RemoteControlConnectionAuth { + pub(super) auth_provider: SharedAuthProvider, + pub(super) account_id: String, +} + +pub(super) async fn load_remote_control_auth( + auth_manager: &Arc, +) -> io::Result { + let mut reloaded = false; + let auth = loop { + let Some(auth) = auth_manager.auth().await else { + if reloaded { + return Err(io::Error::new( + ErrorKind::PermissionDenied, + "remote control requires ChatGPT authentication", + )); + } + auth_manager.reload().await; + reloaded = true; + continue; + }; + if !auth.uses_codex_backend() { + break auth; + } + if auth.get_account_id().is_none() && !reloaded { + auth_manager.reload().await; + reloaded = true; + continue; + } + break auth; + }; + + if !auth.uses_codex_backend() { + return Err(io::Error::new( + ErrorKind::PermissionDenied, + "remote control requires ChatGPT authentication; API key auth is not supported", + )); + } + + Ok(RemoteControlConnectionAuth { + auth_provider: codex_model_provider::auth_provider_from_auth(&auth), + account_id: auth.get_account_id().ok_or_else(|| { + io::Error::new( + ErrorKind::WouldBlock, + "remote control enrollment is waiting for a ChatGPT account id", + ) + })?, + }) +} + +pub(super) async fn recover_remote_control_auth( + auth_recovery: &mut UnauthorizedRecovery, + auth_change_rx: &mut watch::Receiver, +) -> bool { + if !auth_recovery.has_next() { + return false; + } + + let mode = auth_recovery.mode_name(); + let step = auth_recovery.step_name(); + let auth_change_revision_before_recovery = *auth_change_rx.borrow(); + match auth_recovery.next().await { + Ok(step_result) => { + if step_result.auth_state_changed() == Some(true) { + mark_recovery_auth_change_seen( + auth_change_rx, + auth_change_revision_before_recovery, + ); + } + info!( + "remote control auth recovery succeeded: mode={mode}, step={step}, auth_state_changed={:?}", + step_result.auth_state_changed() + ); + true + } + Err(err) => { + warn!("remote control auth recovery failed: mode={mode}, step={step}: {err}"); + false + } + } +} + +pub(super) fn mark_recovery_auth_change_seen( + auth_change_rx: &mut watch::Receiver, + auth_change_revision_before_recovery: u64, +) { + let auth_change_revision_after_recovery = *auth_change_rx.borrow(); + if auth_change_revision_after_recovery == auth_change_revision_before_recovery.wrapping_add(1) { + // Recovery updated the same watch that wakes the outer reconnect + // loop. Mark only that single revision seen; if more revisions + // arrived while recovery was in flight, leave them pending so the + // reconnect loop still reacts to the later external auth change. + auth_change_rx.borrow_and_update(); + } +} diff --git a/codex-rs/app-server-transport/src/transport/remote_control/clients.rs b/codex-rs/app-server-transport/src/transport/remote_control/clients.rs new file mode 100644 index 000000000..25316c609 --- /dev/null +++ b/codex-rs/app-server-transport/src/transport/remote_control/clients.rs @@ -0,0 +1,307 @@ +use super::auth::RemoteControlConnectionAuth; +use super::auth::load_remote_control_auth; +use super::auth::recover_remote_control_auth; +use super::enroll::REMOTE_CONTROL_ACCOUNT_ID_HEADER; +use super::enroll::format_headers; +use super::enroll::preview_remote_control_response_body; +use super::protocol::normalize_remote_control_base_url; +use axum::http::HeaderMap; +use codex_app_server_protocol::RemoteControlClient; +use codex_app_server_protocol::RemoteControlClientsListOrder; +use codex_app_server_protocol::RemoteControlClientsListParams; +use codex_app_server_protocol::RemoteControlClientsListResponse; +use codex_app_server_protocol::RemoteControlClientsRevokeParams; +use codex_app_server_protocol::RemoteControlClientsRevokeResponse; +use codex_login::AuthManager; +use codex_login::default_client::build_reqwest_client; +use serde::Deserialize; +use std::io; +use std::io::ErrorKind; +use std::sync::Arc; +use time::OffsetDateTime; +use time::format_description::well_known::Rfc3339; +use url::Url; + +const REMOTE_CONTROL_CLIENT_MANAGEMENT_TIMEOUT: std::time::Duration = + std::time::Duration::from_secs(30); + +#[derive(Debug, Deserialize)] +struct ListRemoteControlClientsResponse { + items: Vec, + #[serde(default)] + cursor: Option, +} + +#[derive(Debug, Deserialize)] +struct RemoteControlClientResponse { + client_id: String, + #[serde(default)] + display_name: Option, + #[serde(default)] + device_type: Option, + #[serde(default)] + platform: Option, + #[serde(default)] + os_version: Option, + #[serde(default)] + device_model: Option, + #[serde(default)] + app_version: Option, + #[serde(default)] + last_seen_at: Option, +} + +enum ClientManagementRequest<'a> { + List { + url: &'a Url, + params: &'a RemoteControlClientsListParams, + }, + Revoke { + url: &'a Url, + }, +} + +struct ClientManagementResponse { + status: axum::http::StatusCode, + headers: HeaderMap, + body: Vec, +} + +pub(super) async fn list_remote_control_clients( + remote_control_url: &str, + auth_manager: &Arc, + params: RemoteControlClientsListParams, +) -> io::Result { + if params.environment_id.is_empty() { + return Err(io::Error::new( + ErrorKind::InvalidInput, + "remote control client list requires environmentId", + )); + } + if params + .limit + .is_some_and(|limit| !(1..=100).contains(&limit)) + { + return Err(io::Error::new( + ErrorKind::InvalidInput, + "remote control client list limit must be between 1 and 100", + )); + } + let url = environment_clients_url(remote_control_url, ¶ms.environment_id)?; + let response = send_client_management_request( + auth_manager, + ClientManagementRequest::List { + url: &url, + params: ¶ms, + }, + "list remote control clients", + ) + .await?; + let ClientManagementResponse { + status, + headers, + body, + } = response; + let body_preview = preview_remote_control_response_body(&body); + ensure_success_response(status, &headers, &url, &body_preview, "client list")?; + let response = serde_json::from_slice::(&body).map_err( + |err| { + io::Error::other(format!( + "failed to parse remote control client list response from `{url}`: HTTP {status}, {}, body: {body_preview}, decode error: {err}", + format_headers(&headers) + )) + }, + )?; + Ok(RemoteControlClientsListResponse { + data: response + .items + .into_iter() + .map(RemoteControlClient::try_from) + .collect::>()?, + next_cursor: response.cursor, + }) +} + +pub(super) async fn revoke_remote_control_client( + remote_control_url: &str, + auth_manager: &Arc, + params: RemoteControlClientsRevokeParams, +) -> io::Result { + if params.environment_id.is_empty() { + return Err(io::Error::new( + ErrorKind::InvalidInput, + "remote control client revoke requires environmentId", + )); + } + if params.client_id.is_empty() { + return Err(io::Error::new( + ErrorKind::InvalidInput, + "remote control client revoke requires clientId", + )); + } + let mut url = environment_clients_url(remote_control_url, ¶ms.environment_id)?; + url.path_segments_mut() + .map_err(|()| { + io::Error::new( + ErrorKind::InvalidInput, + "remote control URL cannot be a base", + ) + })? + .push(¶ms.client_id); + let response = send_client_management_request( + auth_manager, + ClientManagementRequest::Revoke { url: &url }, + "revoke remote control client", + ) + .await?; + let ClientManagementResponse { + status, + headers, + body, + } = response; + let body_preview = preview_remote_control_response_body(&body); + ensure_success_response(status, &headers, &url, &body_preview, "client revoke")?; + Ok(RemoteControlClientsRevokeResponse {}) +} + +async fn send_client_management_request( + auth_manager: &Arc, + request: ClientManagementRequest<'_>, + action: &str, +) -> io::Result { + let mut auth_recovery = auth_manager.unauthorized_recovery(); + let mut auth_change_rx = auth_manager.auth_change_receiver(); + let auth = load_remote_control_auth(auth_manager).await?; + let response = send_client_management_request_once(&auth, &request, action).await?; + if response.status.as_u16() != 401 + || !recover_remote_control_auth(&mut auth_recovery, &mut auth_change_rx).await + { + return Ok(response); + } + let auth = load_remote_control_auth(auth_manager).await?; + send_client_management_request_once(&auth, &request, action).await +} + +async fn send_client_management_request_once( + auth: &RemoteControlConnectionAuth, + request: &ClientManagementRequest<'_>, + action: &str, +) -> io::Result { + let client = build_reqwest_client(); + let mut auth_headers = HeaderMap::new(); + auth.auth_provider.add_auth_headers(&mut auth_headers); + let request = match request { + ClientManagementRequest::List { url, params } => { + let mut query = Vec::new(); + if let Some(cursor) = ¶ms.cursor { + query.push(("cursor", cursor.clone())); + } + if let Some(limit) = params.limit { + query.push(("limit", limit.to_string())); + } + if let Some(order) = params.order { + query.push(( + "order", + match order { + RemoteControlClientsListOrder::Asc => "asc", + RemoteControlClientsListOrder::Desc => "desc", + } + .to_string(), + )); + } + client.get((*url).clone()).query(&query) + } + ClientManagementRequest::Revoke { url } => client.delete((*url).clone()), + }; + let response = request + .timeout(REMOTE_CONTROL_CLIENT_MANAGEMENT_TIMEOUT) + .headers(auth_headers) + .header(REMOTE_CONTROL_ACCOUNT_ID_HEADER, &auth.account_id) + .send() + .await + .map_err(|err| io::Error::other(format!("failed to {action}: {err}")))?; + let headers = response.headers().clone(); + let status = response.status(); + let body = response + .bytes() + .await + .map_err(|err| io::Error::other(format!("failed to read {action} response: {err}")))? + .to_vec(); + Ok(ClientManagementResponse { + status, + headers, + body, + }) +} + +fn ensure_success_response( + status: axum::http::StatusCode, + headers: &HeaderMap, + url: &Url, + body_preview: &str, + response_kind: &str, +) -> io::Result<()> { + if status.is_success() { + return Ok(()); + } + let error_kind = match status.as_u16() { + 400 => ErrorKind::InvalidInput, + 401 | 403 => ErrorKind::PermissionDenied, + 404 => ErrorKind::NotFound, + _ => ErrorKind::Other, + }; + Err(io::Error::new( + error_kind, + format!( + "remote control {response_kind} failed at `{url}`: HTTP {status}, {}, body: {body_preview}", + format_headers(headers) + ), + )) +} + +fn environment_clients_url(remote_control_url: &str, environment_id: &str) -> io::Result { + let mut url = normalize_remote_control_base_url(remote_control_url)? + .join("wham/remote/control/environments") + .map_err(io::Error::other)?; + url.path_segments_mut() + .map_err(|()| { + io::Error::new( + ErrorKind::InvalidInput, + "remote control URL cannot be a base", + ) + })? + .push(environment_id) + .push("clients"); + Ok(url) +} + +impl TryFrom for RemoteControlClient { + type Error = io::Error; + + fn try_from(client: RemoteControlClientResponse) -> Result { + Ok(Self { + client_id: client.client_id, + display_name: client.display_name, + device_type: client.device_type, + platform: client.platform, + os_version: client.os_version, + device_model: client.device_model, + app_version: client.app_version, + last_seen_at: client + .last_seen_at + .map(|last_seen_at| { + OffsetDateTime::parse(&last_seen_at, &Rfc3339) + .map(OffsetDateTime::unix_timestamp) + .map_err(|err| { + io::Error::new( + ErrorKind::InvalidData, + format!( + "failed to parse remote control client last_seen_at `{last_seen_at}`: {err}" + ), + ) + }) + }) + .transpose()?, + }) + } +} diff --git a/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs b/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs index ac6772637..c491596e6 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/enroll.rs @@ -1,3 +1,4 @@ +use super::auth::RemoteControlConnectionAuth; use super::pairing_unavailable_error; use super::protocol::EnrollRemoteServerRequest; use super::protocol::EnrollRemoteServerResponse; @@ -6,7 +7,6 @@ use super::protocol::RemoteControlTarget; use super::protocol::StartRemoteControlPairingRequest; use super::protocol::StartRemoteControlPairingResponse; use axum::http::HeaderMap; -use codex_api::SharedAuthProvider; use codex_app_server_protocol::RemoteControlPairingStartResponse; use codex_login::default_client::build_reqwest_client; use codex_state::RemoteControlEnrollmentRecord; @@ -151,11 +151,6 @@ impl RemoteControlEnrollment { } } -pub(super) struct RemoteControlConnectionAuth { - pub(super) auth_provider: SharedAuthProvider, - pub(super) account_id: String, -} - pub(super) async fn load_persisted_remote_control_enrollment( state_db: Option<&StateRuntime>, remote_control_target: &RemoteControlTarget, diff --git a/codex-rs/app-server-transport/src/transport/remote_control/mod.rs b/codex-rs/app-server-transport/src/transport/remote_control/mod.rs index a55667886..38e34e6d4 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/mod.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/mod.rs @@ -1,9 +1,13 @@ +mod auth; mod client_tracker; +mod clients; mod enroll; mod protocol; mod segment; mod websocket; +use self::auth::load_remote_control_auth; +use self::auth::recover_remote_control_auth; use self::enroll::RemoteControlEnrollment; use self::enroll::refresh_remote_control_server; use crate::transport::remote_control::websocket::RemoteControlChannels; @@ -17,6 +21,10 @@ use self::protocol::normalize_remote_control_url; use super::CHANNEL_CAPACITY; use super::TransportEvent; use super::next_connection_id; +use codex_app_server_protocol::RemoteControlClientsListParams; +use codex_app_server_protocol::RemoteControlClientsListResponse; +use codex_app_server_protocol::RemoteControlClientsRevokeParams; +use codex_app_server_protocol::RemoteControlClientsRevokeResponse; use codex_app_server_protocol::RemoteControlConnectionStatus; use codex_app_server_protocol::RemoteControlPairingStartParams; use codex_app_server_protocol::RemoteControlPairingStartResponse; @@ -57,6 +65,7 @@ pub struct RemoteControlHandle { enabled_tx: Arc>, status_tx: Arc>, state_db_available: bool, + remote_control_url: String, current_enrollment: CurrentRemoteControlEnrollment, auth_manager: Arc, } @@ -146,7 +155,7 @@ impl RemoteControlHandle { if !*self.enabled_tx.borrow() { return Err(Self::pairing_disabled_error()); } - let mut auth = websocket::load_remote_control_auth(&self.auth_manager) + let mut auth = load_remote_control_auth(&self.auth_manager) .await .map_err(|_| pairing_unavailable_error())?; let mut enrollment = { @@ -205,7 +214,7 @@ impl RemoteControlHandle { if !*self.enabled_tx.borrow() { return Err(Self::pairing_disabled_error()); } - let current_auth = websocket::load_remote_control_auth(&self.auth_manager) + let current_auth = load_remote_control_auth(&self.auth_manager) .await .map_err(|_| pairing_unavailable_error())?; if current_auth.account_id != auth.account_id { @@ -214,6 +223,22 @@ impl RemoteControlHandle { pairing_response } + pub async fn list_clients( + &self, + params: RemoteControlClientsListParams, + ) -> io::Result { + clients::list_remote_control_clients(&self.remote_control_url, &self.auth_manager, params) + .await + } + + pub async fn revoke_client( + &self, + params: RemoteControlClientsRevokeParams, + ) -> io::Result { + clients::revoke_remote_control_client(&self.remote_control_url, &self.auth_manager, params) + .await + } + fn pairing_disabled_error() -> io::Error { io::Error::new( io::ErrorKind::InvalidInput, @@ -255,7 +280,7 @@ impl RemoteControlHandle { async fn refresh_pairing_enrollment( current_enrollment: &CurrentRemoteControlEnrollment, auth_manager: &Arc, - auth: &mut enroll::RemoteControlConnectionAuth, + auth: &mut auth::RemoteControlConnectionAuth, installation_id: &str, enrollment: &mut RemoteControlEnrollment, ) -> io::Result<()> { @@ -265,10 +290,10 @@ async fn refresh_pairing_enrollment( } let mut auth_recovery = auth_manager.unauthorized_recovery(); let mut auth_change_rx = auth_manager.auth_change_receiver(); - if !websocket::recover_remote_control_auth(&mut auth_recovery, &mut auth_change_rx).await { + if !recover_remote_control_auth(&mut auth_recovery, &mut auth_change_rx).await { return Err(err); } - *auth = websocket::load_remote_control_auth(auth_manager) + *auth = load_remote_control_auth(auth_manager) .await .map_err(|_| pairing_unavailable_error())?; if auth.account_id != enrollment.account_id { @@ -440,6 +465,7 @@ pub async fn start_remote_control( "starting app-server remote control websocket task" ); let remote_control_url_for_log = remote_control_url.clone(); + let handle_remote_control_url = remote_control_url.clone(); let installation_id_for_log = installation_id.clone(); let server_name_for_log = server_name.clone(); let shutdown_token_for_log = shutdown_token.clone(); @@ -508,6 +534,7 @@ pub async fn start_remote_control( enabled_tx: Arc::new(enabled_tx), status_tx: Arc::new(status_tx), state_db_available, + remote_control_url: handle_remote_control_url, current_enrollment, auth_manager: handle_auth_manager, }, diff --git a/codex-rs/app-server-transport/src/transport/remote_control/protocol.rs b/codex-rs/app-server-transport/src/transport/remote_control/protocol.rs index 8eeb28e26..630fd862c 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/protocol.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/protocol.rs @@ -177,6 +177,48 @@ fn is_localhost(host: &Option>) -> bool { pub(super) fn normalize_remote_control_url( remote_control_url: &str, ) -> io::Result { + let remote_control_url = normalize_remote_control_base_url(remote_control_url)?; + let map_url_parse_error = |err: url::ParseError| -> io::Error { + io::Error::new( + ErrorKind::InvalidInput, + format!("invalid remote control URL `{remote_control_url}`: {err}"), + ) + }; + + let enroll_url = remote_control_url + .join("wham/remote/control/server/enroll") + .map_err(map_url_parse_error)?; + let refresh_url = remote_control_url + .join("wham/remote/control/server/refresh") + .map_err(map_url_parse_error)?; + let pair_url = remote_control_url + .join("wham/remote/control/server/pair") + .map_err(map_url_parse_error)?; + let mut websocket_url = remote_control_url + .join("wham/remote/control/server") + .map_err(map_url_parse_error)?; + websocket_url + .set_scheme(if enroll_url.scheme() == "https" { + "wss" + } else { + "ws" + }) + .map_err(|()| { + io::Error::new( + ErrorKind::InvalidInput, + format!("invalid remote control URL `{remote_control_url}`"), + ) + })?; + + Ok(RemoteControlTarget { + websocket_url: websocket_url.to_string(), + enroll_url: enroll_url.to_string(), + refresh_url: refresh_url.to_string(), + pair_url: pair_url.to_string(), + }) +} + +pub(super) fn normalize_remote_control_base_url(remote_control_url: &str) -> io::Result { let map_url_parse_error = |err: url::ParseError| -> io::Error { io::Error::new( ErrorKind::InvalidInput, @@ -198,35 +240,14 @@ pub(super) fn normalize_remote_control_url( remote_control_url.set_path(&normalized_path); } - let enroll_url = remote_control_url - .join("wham/remote/control/server/enroll") - .map_err(map_url_parse_error)?; - let refresh_url = remote_control_url - .join("wham/remote/control/server/refresh") - .map_err(map_url_parse_error)?; - let pair_url = remote_control_url - .join("wham/remote/control/server/pair") - .map_err(map_url_parse_error)?; - let mut websocket_url = remote_control_url - .join("wham/remote/control/server") - .map_err(map_url_parse_error)?; - let host = enroll_url.host(); - match enroll_url.scheme() { - "https" if is_localhost(&host) || is_allowed_remote_control_chatgpt_host(&host) => { - websocket_url.set_scheme("wss").map_err(map_scheme_error)?; - } - "http" if is_localhost(&host) => { - websocket_url.set_scheme("ws").map_err(map_scheme_error)?; - } + let host = remote_control_url.host(); + match remote_control_url.scheme() { + "https" if is_localhost(&host) || is_allowed_remote_control_chatgpt_host(&host) => {} + "http" if is_localhost(&host) => {} _ => return Err(map_scheme_error(())), } - Ok(RemoteControlTarget { - websocket_url: websocket_url.to_string(), - enroll_url: enroll_url.to_string(), - refresh_url: refresh_url.to_string(), - pair_url: pair_url.to_string(), - }) + Ok(remote_control_url) } #[cfg(test)] diff --git a/codex-rs/app-server-transport/src/transport/remote_control/tests.rs b/codex-rs/app-server-transport/src/transport/remote_control/tests.rs index 5d96f89d8..a1a284da9 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/tests.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/tests.rs @@ -60,6 +60,7 @@ use tokio_tungstenite::accept_hdr_async; use tokio_tungstenite::tungstenite; use tokio_util::sync::CancellationToken; +mod clients_tests; mod pairing_tests; const TEST_INSTALLATION_ID: &str = "11111111-1111-4111-8111-111111111111"; @@ -163,6 +164,7 @@ fn remote_control_handle_with_current_enrollment( enabled_tx: Arc::new(enabled_tx), status_tx: Arc::new(status_tx), state_db_available: true, + remote_control_url: remote_control_url.to_string(), current_enrollment, auth_manager, } diff --git a/codex-rs/app-server-transport/src/transport/remote_control/tests/clients_tests.rs b/codex-rs/app-server-transport/src/transport/remote_control/tests/clients_tests.rs new file mode 100644 index 000000000..22aa29076 --- /dev/null +++ b/codex-rs/app-server-transport/src/transport/remote_control/tests/clients_tests.rs @@ -0,0 +1,359 @@ +use super::super::clients::list_remote_control_clients; +use super::super::clients::revoke_remote_control_client; +use super::*; +use codex_app_server_protocol::RemoteControlClient; +use codex_app_server_protocol::RemoteControlClientsListOrder; +use codex_app_server_protocol::RemoteControlClientsListParams; +use codex_app_server_protocol::RemoteControlClientsListResponse; +use codex_app_server_protocol::RemoteControlClientsRevokeParams; +use codex_app_server_protocol::RemoteControlClientsRevokeResponse; +use pretty_assertions::assert_eq; + +fn client_management_handle( + remote_control_url: String, + auth_manager: Arc, +) -> RemoteControlHandle { + let (enabled_tx, _enabled_rx) = watch::channel(/*init*/ false); + let (status_tx, _status_rx) = watch::channel(RemoteControlStatusChangedNotification { + status: RemoteControlConnectionStatus::Disabled, + server_name: test_server_name(), + installation_id: TEST_INSTALLATION_ID.to_string(), + environment_id: None, + }); + RemoteControlHandle { + enabled_tx: Arc::new(enabled_tx), + status_tx: Arc::new(status_tx), + state_db_available: false, + remote_control_url, + current_enrollment: Arc::new(StdMutex::new(None)), + auth_manager, + } +} + +fn empty_client_list() -> serde_json::Value { + json!({ + "items": [], + "cursor": null, + }) +} + +#[tokio::test] +async fn remote_control_handle_lists_clients_while_disabled() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let remote_control_url = remote_control_url_for_listener(&listener); + let server_task = tokio::spawn(async move { + let request = accept_http_request(&listener).await; + assert_eq!( + request.request_line, + "GET /backend-api/wham/remote/control/environments/env%20%2F%3F/clients?cursor=cursor+%2F%3F&limit=10&order=asc HTTP/1.1" + ); + assert_eq!( + request.headers.get("authorization"), + Some(&"Bearer Access Token".to_string()) + ); + assert_eq!( + request.headers.get(REMOTE_CONTROL_ACCOUNT_ID_HEADER), + Some(&"account_id".to_string()) + ); + respond_with_json( + request.stream, + json!({ + "items": [{ + "client_id": "client-123", + "account_user_id": "user-123", + "enrollment_status": "enrolled_device_key", + "display_name": "Anton Phone", + "device_type": "phone", + "platform": "ios", + "os_version": "19.0", + "device_model": "iPhone", + "app_version": "1.2.3", + "last_seen_at": "2026-03-05T07:00:00Z", + "last_seen_city": "San Francisco", + }], + "cursor": "next-cursor", + }), + ) + .await; + }); + let handle = client_management_handle(remote_control_url, remote_control_auth_manager()); + + let response = handle + .list_clients(RemoteControlClientsListParams { + environment_id: "env /?".to_string(), + cursor: Some("cursor /?".to_string()), + limit: Some(10), + order: Some(RemoteControlClientsListOrder::Asc), + }) + .await + .expect("client list should succeed while remote control is disabled"); + server_task.await.expect("server task should finish"); + + assert_eq!( + response, + RemoteControlClientsListResponse { + data: vec![RemoteControlClient { + client_id: "client-123".to_string(), + display_name: Some("Anton Phone".to_string()), + device_type: Some("phone".to_string()), + platform: Some("ios".to_string()), + os_version: Some("19.0".to_string()), + device_model: Some("iPhone".to_string()), + app_version: Some("1.2.3".to_string()), + last_seen_at: Some(1_772_694_000), + }], + next_cursor: Some("next-cursor".to_string()), + } + ); +} + +#[tokio::test] +async fn remote_control_handle_revokes_client_while_disabled() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let remote_control_url = remote_control_url_for_listener(&listener); + let server_task = tokio::spawn(async move { + let request = accept_http_request(&listener).await; + assert_eq!( + request.request_line, + "DELETE /backend-api/wham/remote/control/environments/env%20%2F%3F/clients/client%20%2F%3F HTTP/1.1" + ); + respond_with_status(request.stream, "204 No Content", "").await; + }); + let handle = client_management_handle(remote_control_url, remote_control_auth_manager()); + + let response = handle + .revoke_client(RemoteControlClientsRevokeParams { + environment_id: "env /?".to_string(), + client_id: "client /?".to_string(), + }) + .await + .expect("client revoke should succeed while remote control is disabled"); + server_task.await.expect("server task should finish"); + + assert_eq!(response, RemoteControlClientsRevokeResponse {}); +} + +#[tokio::test] +async fn list_remote_control_clients_recovers_auth_after_unauthorized() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let remote_control_url = remote_control_url_for_listener(&listener); + let server_task = tokio::spawn(async move { + let stale_request = accept_http_request(&listener).await; + assert_eq!( + stale_request.headers.get("authorization"), + Some(&"Bearer stale-token".to_string()) + ); + respond_with_status(stale_request.stream, "401 Unauthorized", "").await; + + let recovered_request = accept_http_request(&listener).await; + assert_eq!( + recovered_request.headers.get("authorization"), + Some(&"Bearer fresh-token".to_string()) + ); + respond_with_json(recovered_request.stream, empty_client_list()).await; + }); + let codex_home = TempDir::new().expect("temp dir should create"); + let mut stale_auth = remote_control_auth_dot_json(Some("account_id")); + stale_auth + .tokens + .as_mut() + .expect("stale auth should include tokens") + .access_token = "stale-token".to_string(); + save_auth( + codex_home.path(), + &stale_auth, + AuthCredentialsStoreMode::File, + ) + .expect("stale auth should save"); + let auth_manager = AuthManager::shared( + codex_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await; + let mut fresh_auth = remote_control_auth_dot_json(Some("account_id")); + fresh_auth + .tokens + .as_mut() + .expect("fresh auth should include tokens") + .access_token = "fresh-token".to_string(); + save_auth( + codex_home.path(), + &fresh_auth, + AuthCredentialsStoreMode::File, + ) + .expect("fresh auth should save"); + + let response = list_remote_control_clients( + &remote_control_url, + &auth_manager, + RemoteControlClientsListParams { + environment_id: "env-123".to_string(), + ..Default::default() + }, + ) + .await + .expect("client list should recover auth"); + server_task.await.expect("server task should finish"); + + assert_eq!( + response, + RemoteControlClientsListResponse { + data: Vec::new(), + next_cursor: None, + } + ); +} + +#[tokio::test] +async fn list_remote_control_clients_retries_unauthorized_only_once() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let remote_control_url = remote_control_url_for_listener(&listener); + let server_task = tokio::spawn(async move { + let stale_request = accept_http_request(&listener).await; + assert_eq!( + stale_request.headers.get("authorization"), + Some(&"Bearer stale-token".to_string()) + ); + respond_with_status(stale_request.stream, "401 Unauthorized", "").await; + + let recovered_request = accept_http_request(&listener).await; + assert_eq!( + recovered_request.headers.get("authorization"), + Some(&"Bearer fresh-token".to_string()) + ); + respond_with_status(recovered_request.stream, "401 Unauthorized", "").await; + + assert!( + timeout(Duration::from_millis(100), accept_http_request(&listener)) + .await + .is_err() + ); + }); + let codex_home = TempDir::new().expect("temp dir should create"); + let mut stale_auth = remote_control_auth_dot_json(Some("account_id")); + stale_auth + .tokens + .as_mut() + .expect("stale auth should include tokens") + .access_token = "stale-token".to_string(); + save_auth( + codex_home.path(), + &stale_auth, + AuthCredentialsStoreMode::File, + ) + .expect("stale auth should save"); + let auth_manager = AuthManager::shared( + codex_home.path().to_path_buf(), + /*enable_codex_api_key_env*/ false, + AuthCredentialsStoreMode::File, + /*chatgpt_base_url*/ None, + ) + .await; + let mut fresh_auth = remote_control_auth_dot_json(Some("account_id")); + fresh_auth + .tokens + .as_mut() + .expect("fresh auth should include tokens") + .access_token = "fresh-token".to_string(); + save_auth( + codex_home.path(), + &fresh_auth, + AuthCredentialsStoreMode::File, + ) + .expect("fresh auth should save"); + + let err = list_remote_control_clients( + &remote_control_url, + &auth_manager, + RemoteControlClientsListParams { + environment_id: "env-123".to_string(), + ..Default::default() + }, + ) + .await + .expect_err("second unauthorized response should fail"); + server_task.await.expect("server task should finish"); + + assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); +} + +#[tokio::test] +async fn revoke_remote_control_client_does_not_retry_forbidden() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let remote_control_url = remote_control_url_for_listener(&listener); + let server_task = tokio::spawn(async move { + let request = accept_http_request(&listener).await; + respond_with_status_and_headers( + request.stream, + "403 Forbidden", + &[("x-request-id", "request-123"), ("cf-ray", "ray-123")], + "forbidden", + ) + .await; + }); + + let err = revoke_remote_control_client( + &remote_control_url, + &remote_control_auth_manager(), + RemoteControlClientsRevokeParams { + environment_id: "env-123".to_string(), + client_id: "client-123".to_string(), + }, + ) + .await + .expect_err("forbidden revoke should fail"); + server_task.await.expect("server task should finish"); + + assert_eq!(err.kind(), std::io::ErrorKind::PermissionDenied); + assert_eq!( + err.to_string(), + format!( + "remote control client revoke failed at `{remote_control_url}wham/remote/control/environments/env-123/clients/client-123`: HTTP 403 Forbidden, request-id: request-123, cf-ray: ray-123, body: forbidden" + ) + ); +} + +#[tokio::test] +async fn list_remote_control_clients_preserves_decode_error_context() { + let listener = TcpListener::bind("127.0.0.1:0") + .await + .expect("listener should bind"); + let remote_control_url = remote_control_url_for_listener(&listener); + let server_task = tokio::spawn(async move { + let request = accept_http_request(&listener).await; + respond_with_status(request.stream, "200 OK", "{").await; + }); + + let err = list_remote_control_clients( + &remote_control_url, + &remote_control_auth_manager(), + RemoteControlClientsListParams { + environment_id: "env-123".to_string(), + ..Default::default() + }, + ) + .await + .expect_err("malformed client list should fail"); + server_task.await.expect("server task should finish"); + + assert!( + err.to_string().contains( + "failed to parse remote control client list response from `http://127.0.0.1:" + ) + ); + assert!(err.to_string().contains("HTTP 200 OK")); + assert!(err.to_string().contains("body: {")); + assert!(err.to_string().contains("decode error:")); +} diff --git a/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs b/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs index 5ec4f3d04..60792df54 100644 --- a/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs +++ b/codex-rs/app-server-transport/src/transport/remote_control/websocket.rs @@ -13,9 +13,11 @@ use super::segment::ClientSegmentReassembler; use super::segment::REMOTE_CONTROL_SEGMENT_MAX_BYTES; use super::segment::split_server_envelope_for_transport; use crate::transport::TransportEvent; +use crate::transport::remote_control::auth::RemoteControlConnectionAuth; +use crate::transport::remote_control::auth::load_remote_control_auth; +use crate::transport::remote_control::auth::recover_remote_control_auth; use crate::transport::remote_control::client_tracker::ClientTracker; use crate::transport::remote_control::client_tracker::REMOTE_CONTROL_IDLE_SWEEP_INTERVAL; -use crate::transport::remote_control::enroll::RemoteControlConnectionAuth; use crate::transport::remote_control::enroll::RemoteControlEnrollment; use crate::transport::remote_control::enroll::enroll_remote_control_server; use crate::transport::remote_control::enroll::format_headers; @@ -1172,51 +1174,6 @@ fn build_remote_control_websocket_request( Ok(request) } -pub(crate) async fn load_remote_control_auth( - auth_manager: &Arc, -) -> io::Result { - let mut reloaded = false; - let auth = loop { - let Some(auth) = auth_manager.auth().await else { - if reloaded { - return Err(io::Error::new( - ErrorKind::PermissionDenied, - "remote control requires ChatGPT authentication", - )); - } - auth_manager.reload().await; - reloaded = true; - continue; - }; - if !auth.uses_codex_backend() { - break auth; - } - if auth.get_account_id().is_none() && !reloaded { - auth_manager.reload().await; - reloaded = true; - continue; - } - break auth; - }; - - if !auth.uses_codex_backend() { - return Err(io::Error::new( - ErrorKind::PermissionDenied, - "remote control requires ChatGPT authentication; API key auth is not supported", - )); - } - - Ok(RemoteControlConnectionAuth { - auth_provider: codex_model_provider::auth_provider_from_auth(&auth), - account_id: auth.get_account_id().ok_or_else(|| { - io::Error::new( - ErrorKind::WouldBlock, - "remote control enrollment is waiting for a ChatGPT account id", - ) - })?, - }) -} - fn next_reconnect_delay(reconnect_attempt: &mut u64) -> (std::time::Duration, bool) { let reconnect_delay = backoff(*reconnect_attempt).min(REMOTE_CONTROL_RECONNECT_BACKOFF_CAP); let reconnect_backoff_reset = reconnect_delay == REMOTE_CONTROL_RECONNECT_BACKOFF_CAP; @@ -1548,52 +1505,6 @@ async fn enroll_remote_control_server_if_missing( Ok(()) } -pub(super) async fn recover_remote_control_auth( - auth_recovery: &mut UnauthorizedRecovery, - auth_change_rx: &mut watch::Receiver, -) -> bool { - if !auth_recovery.has_next() { - return false; - } - - let mode = auth_recovery.mode_name(); - let step = auth_recovery.step_name(); - let auth_change_revision_before_recovery = *auth_change_rx.borrow(); - match auth_recovery.next().await { - Ok(step_result) => { - if step_result.auth_state_changed() == Some(true) { - mark_recovery_auth_change_seen( - auth_change_rx, - auth_change_revision_before_recovery, - ); - } - info!( - "remote control websocket auth recovery succeeded: mode={mode}, step={step}, auth_state_changed={:?}", - step_result.auth_state_changed() - ); - true - } - Err(err) => { - warn!("remote control websocket auth recovery failed: mode={mode}, step={step}: {err}"); - false - } - } -} - -fn mark_recovery_auth_change_seen( - auth_change_rx: &mut watch::Receiver, - auth_change_revision_before_recovery: u64, -) { - let auth_change_revision_after_recovery = *auth_change_rx.borrow(); - if auth_change_revision_after_recovery == auth_change_revision_before_recovery.wrapping_add(1) { - // Recovery updated the same watch that wakes the outer reconnect - // loop. Mark only that single revision seen; if more revisions - // arrived while recovery was in flight, leave them pending so the - // reconnect loop still reacts to the later external auth change. - auth_change_rx.borrow_and_update(); - } -} - fn format_remote_control_websocket_connect_error( websocket_url: &str, err: &tungstenite::Error, @@ -1620,6 +1531,7 @@ mod tests { use super::*; use crate::outgoing_message::OutgoingMessage; use crate::transport::remote_control::ServerEvent; + use crate::transport::remote_control::auth::mark_recovery_auth_change_seen; use crate::transport::remote_control::protocol::StreamId; use crate::transport::remote_control::protocol::normalize_remote_control_url; use chrono::Utc; diff --git a/codex-rs/app-server/README.md b/codex-rs/app-server/README.md index fdc34e524..dc02ff8c0 100644 --- a/codex-rs/app-server/README.md +++ b/codex-rs/app-server/README.md @@ -211,6 +211,8 @@ Example with notification opt-out: - `remoteControl/disable` — experimental; disable remote control for the current app-server process and return the current remote-control status snapshot. This does not revoke already enrolled controller devices. - `remoteControl/status/read` — experimental; read the current remote-control status snapshot. `status` is one of `disabled`, `connecting`, `connected`, or `errored`; `serverName` is the local machine name used by this app-server process; `environmentId` is a string when the app-server has a current enrollment and `null` when that enrollment is cleared, invalidated, or remote control is disabled. - `remoteControl/pairing/start` — experimental; start a short-lived remote-control pairing artifact for the current app-server process. Pass `manualCode: true` to also request a manual pairing code. Returns `pairingCode`, `manualPairingCode`, `environmentId`, and Unix-seconds `expiresAt`; app-server intentionally does not expose the backend `serverId`. +- `remoteControl/client/list` — experimental; list controller devices granted access to an environment. Pass `environmentId` and optional `cursor`, `limit`, and `order`; returns picker-oriented client metadata plus `nextCursor`. This signed-in account-management operation works while the local relay is disabled or unenrolled. +- `remoteControl/client/revoke` — experimental; revoke one controller device's grant for an environment. Pass `environmentId` and `clientId`; returns an empty object. This signed-in account-management operation works while the local relay is disabled or unenrolled. - `remoteControl/status/changed` — notification emitted when the remote-control status or client-visible environment id changes. `status` is one of `disabled`, `connecting`, `connected`, or `errored`; `serverName` is the local machine name used by this app-server process; `environmentId` is a string when the app-server has a current enrollment and `null` when that enrollment is cleared, invalidated, or remote control is disabled. Newly initialized app-server clients always receive the current status snapshot. - `skills/config/write` — write user-level skill config by name or absolute path. - `plugin/install` — install a plugin from a discovered marketplace entry, rejecting marketplace entries marked unavailable for install, install MCPs if any, and return the effective plugin auth policy plus any apps that still need auth (**under development; do not call from production clients yet**). diff --git a/codex-rs/app-server/src/message_processor.rs b/codex-rs/app-server/src/message_processor.rs index 7aa93ec86..84f22809e 100644 --- a/codex-rs/app-server/src/message_processor.rs +++ b/codex-rs/app-server/src/message_processor.rs @@ -922,6 +922,16 @@ impl MessageProcessor { .pairing_start(params) .await .map(|response| Some(response.into())), + ClientRequest::RemoteControlClientsList { params, .. } => self + .remote_control_processor + .clients_list(params) + .await + .map(|response| Some(response.into())), + ClientRequest::RemoteControlClientsRevoke { params, .. } => self + .remote_control_processor + .clients_revoke(params) + .await + .map(|response| Some(response.into())), ClientRequest::ConfigRequirementsRead { params: _, .. } => self .config_processor .config_requirements_read() diff --git a/codex-rs/app-server/src/request_processors/remote_control_processor.rs b/codex-rs/app-server/src/request_processors/remote_control_processor.rs index 7c279585f..804339c17 100644 --- a/codex-rs/app-server/src/request_processors/remote_control_processor.rs +++ b/codex-rs/app-server/src/request_processors/remote_control_processor.rs @@ -3,6 +3,10 @@ use crate::error_code::invalid_request; use crate::transport::RemoteControlHandle; use crate::transport::RemoteControlUnavailable; use codex_app_server_protocol::JSONRPCErrorError; +use codex_app_server_protocol::RemoteControlClientsListParams; +use codex_app_server_protocol::RemoteControlClientsListResponse; +use codex_app_server_protocol::RemoteControlClientsRevokeParams; +use codex_app_server_protocol::RemoteControlClientsRevokeResponse; use codex_app_server_protocol::RemoteControlDisableResponse; use codex_app_server_protocol::RemoteControlEnableResponse; use codex_app_server_protocol::RemoteControlPairingStartParams; @@ -55,6 +59,26 @@ impl RemoteControlRequestProcessor { .map_err(map_pairing_start_error) } + pub(crate) async fn clients_list( + &self, + params: RemoteControlClientsListParams, + ) -> Result { + self.handle()? + .list_clients(params) + .await + .map_err(map_client_management_error) + } + + pub(crate) async fn clients_revoke( + &self, + params: RemoteControlClientsRevokeParams, + ) -> Result { + self.handle()? + .revoke_client(params) + .await + .map_err(map_client_management_error) + } + fn handle(&self) -> Result<&RemoteControlHandle, JSONRPCErrorError> { self.remote_control_handle .as_ref() @@ -74,5 +98,15 @@ fn map_pairing_start_error(err: io::Error) -> JSONRPCErrorError { } } +fn map_client_management_error(err: io::Error) -> JSONRPCErrorError { + match err.kind() { + io::ErrorKind::InvalidInput + | io::ErrorKind::NotFound + | io::ErrorKind::PermissionDenied + | io::ErrorKind::WouldBlock => invalid_request(err.to_string()), + _ => internal_error(err.to_string()), + } +} + #[cfg(test)] mod remote_control_processor_tests; diff --git a/codex-rs/app-server/src/request_processors/remote_control_processor/remote_control_processor_tests.rs b/codex-rs/app-server/src/request_processors/remote_control_processor/remote_control_processor_tests.rs index 8a1db50a3..4381e4895 100644 --- a/codex-rs/app-server/src/request_processors/remote_control_processor/remote_control_processor_tests.rs +++ b/codex-rs/app-server/src/request_processors/remote_control_processor/remote_control_processor_tests.rs @@ -46,3 +46,34 @@ fn pairing_start_maps_backend_failures_to_internal_error() { } ); } + +#[test] +fn client_management_maps_user_actionable_errors_to_invalid_request() { + for kind in [ + io::ErrorKind::InvalidInput, + io::ErrorKind::NotFound, + io::ErrorKind::PermissionDenied, + io::ErrorKind::WouldBlock, + ] { + assert_eq!( + map_client_management_error(io::Error::new(kind, "client management unavailable")), + JSONRPCErrorError { + code: INVALID_REQUEST_ERROR_CODE, + data: None, + message: "client management unavailable".to_string(), + } + ); + } +} + +#[test] +fn client_management_maps_backend_failures_to_internal_error() { + assert_eq!( + map_client_management_error(io::Error::other("client management failed")), + JSONRPCErrorError { + code: INTERNAL_ERROR_CODE, + data: None, + message: "client management failed".to_string(), + } + ); +} diff --git a/codex-rs/app-server/tests/common/test_app_server.rs b/codex-rs/app-server/tests/common/test_app_server.rs index 88378322a..ec4b30d92 100644 --- a/codex-rs/app-server/tests/common/test_app_server.rs +++ b/codex-rs/app-server/tests/common/test_app_server.rs @@ -67,6 +67,8 @@ use codex_app_server_protocol::ProcessKillParams; use codex_app_server_protocol::ProcessResizePtyParams; use codex_app_server_protocol::ProcessSpawnParams; use codex_app_server_protocol::ProcessWriteStdinParams; +use codex_app_server_protocol::RemoteControlClientsListParams; +use codex_app_server_protocol::RemoteControlClientsRevokeParams; use codex_app_server_protocol::RemoteControlPairingStartParams; use codex_app_server_protocol::RequestId; use codex_app_server_protocol::ReviewStartParams; @@ -654,6 +656,25 @@ impl TestAppServer { .await } + /// Send a `remoteControl/client/list` JSON-RPC request. + pub async fn send_remote_control_clients_list_request( + &mut self, + params: RemoteControlClientsListParams, + ) -> anyhow::Result { + let params = Some(serde_json::to_value(params)?); + self.send_request("remoteControl/client/list", params).await + } + + /// Send a `remoteControl/client/revoke` JSON-RPC request. + pub async fn send_remote_control_clients_revoke_request( + &mut self, + params: RemoteControlClientsRevokeParams, + ) -> anyhow::Result { + let params = Some(serde_json::to_value(params)?); + self.send_request("remoteControl/client/revoke", params) + .await + } + /// Send an `app/list` JSON-RPC request. pub async fn send_apps_list_request(&mut self, params: AppsListParams) -> anyhow::Result { let params = Some(serde_json::to_value(params)?); diff --git a/codex-rs/app-server/tests/suite/v2/remote_control.rs b/codex-rs/app-server/tests/suite/v2/remote_control.rs index 86187cfc4..b5384d3ba 100644 --- a/codex-rs/app-server/tests/suite/v2/remote_control.rs +++ b/codex-rs/app-server/tests/suite/v2/remote_control.rs @@ -8,6 +8,12 @@ use app_test_support::to_response; use app_test_support::write_chatgpt_auth; use app_test_support::write_mock_responses_config_toml_with_chatgpt_base_url; use codex_app_server_protocol::JSONRPCResponse; +use codex_app_server_protocol::RemoteControlClient; +use codex_app_server_protocol::RemoteControlClientsListOrder; +use codex_app_server_protocol::RemoteControlClientsListParams; +use codex_app_server_protocol::RemoteControlClientsListResponse; +use codex_app_server_protocol::RemoteControlClientsRevokeParams; +use codex_app_server_protocol::RemoteControlClientsRevokeResponse; use codex_app_server_protocol::RemoteControlConnectionStatus; use codex_app_server_protocol::RemoteControlDisableResponse; use codex_app_server_protocol::RemoteControlEnableResponse; @@ -187,11 +193,130 @@ async fn remote_control_pairing_start_returns_pairing_artifacts() -> Result<()> Ok(()) } +#[tokio::test] +async fn remote_control_client_management_works_while_disabled() -> Result<()> { + let codex_home = TempDir::new()?; + let mut backend = ClientManagementRemoteControlBackend::start(codex_home.path()).await?; + let mut mcp = TestAppServer::new(codex_home.path()).await?; + timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; + + let request_id = mcp + .send_remote_control_clients_list_request(RemoteControlClientsListParams { + environment_id: "environment-id".to_string(), + cursor: Some("cursor-id".to_string()), + limit: Some(10), + order: Some(RemoteControlClientsListOrder::Desc), + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let received: RemoteControlClientsListResponse = to_response(response)?; + assert_eq!( + received, + RemoteControlClientsListResponse { + data: vec![RemoteControlClient { + client_id: "client-id".to_string(), + display_name: Some("Anton Phone".to_string()), + device_type: Some("phone".to_string()), + platform: Some("ios".to_string()), + os_version: Some("19.0".to_string()), + device_model: Some("iPhone".to_string()), + app_version: Some("1.2.3".to_string()), + last_seen_at: Some(1_772_694_000), + }], + next_cursor: Some("next-cursor".to_string()), + } + ); + + let request_id = mcp + .send_remote_control_clients_revoke_request(RemoteControlClientsRevokeParams { + environment_id: "environment-id".to_string(), + client_id: "client-id".to_string(), + }) + .await?; + let response: JSONRPCResponse = timeout( + DEFAULT_TIMEOUT, + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), + ) + .await??; + let received: RemoteControlClientsRevokeResponse = to_response(response)?; + assert_eq!(received, RemoteControlClientsRevokeResponse {}); + assert_eq!( + timeout(DEFAULT_TIMEOUT, backend.wait_for_requests()).await??, + vec![ + "GET /backend-api/wham/remote/control/environments/environment-id/clients?cursor=cursor-id&limit=10&order=desc HTTP/1.1".to_string(), + "DELETE /backend-api/wham/remote/control/environments/environment-id/clients/client-id HTTP/1.1".to_string(), + ] + ); + Ok(()) +} + struct BlockingRemoteControlBackend { enroll_request_rx: Option>>, server_task: JoinHandle<()>, } +struct ClientManagementRemoteControlBackend { + requests_rx: Option>>>, + server_task: JoinHandle<()>, +} + +impl ClientManagementRemoteControlBackend { + async fn start(codex_home: &std::path::Path) -> Result { + let listener = configured_remote_control_listener(codex_home).await?; + let (requests_tx, requests_rx) = oneshot::channel(); + let server_task = tokio::spawn(async move { + let result = async { + let list_request = read_http_request(&listener).await?; + let list_request_line = list_request.request_line; + respond_with_json( + list_request.reader.into_inner(), + serde_json::json!({ + "items": [{ + "client_id": "client-id", + "account_user_id": "user-id", + "enrollment_status": "enrolled_device_key", + "display_name": "Anton Phone", + "device_type": "phone", + "platform": "ios", + "os_version": "19.0", + "device_model": "iPhone", + "app_version": "1.2.3", + "last_seen_at": "2026-03-05T07:00:00Z", + "last_seen_city": "San Francisco", + }], + "cursor": "next-cursor", + }), + ) + .await?; + + let revoke_request = read_http_request(&listener).await?; + let revoke_request_line = revoke_request.request_line; + respond_with_status(revoke_request.reader.into_inner(), "204 No Content", "") + .await?; + + Ok(vec![list_request_line, revoke_request_line]) + } + .await; + let _ = requests_tx.send(result); + }); + Ok(Self { + requests_rx: Some(requests_rx), + server_task, + }) + } + + async fn wait_for_requests(&mut self) -> Result> { + self.requests_rx + .take() + .context("requests should only be awaited once")? + .await? + } +} + impl BlockingRemoteControlBackend { async fn start(codex_home: &std::path::Path) -> Result { let listener = configured_remote_control_listener(codex_home).await?; @@ -303,6 +428,12 @@ impl Drop for BlockingRemoteControlBackend { } } +impl Drop for ClientManagementRemoteControlBackend { + fn drop(&mut self) { + self.server_task.abort(); + } +} + struct HttpRequest { request_line: String, reader: BufReader, @@ -365,3 +496,16 @@ async fn respond_with_json(stream: TcpStream, body: serde_json::Value) -> Result .await?; Ok(()) } + +async fn respond_with_status(mut stream: TcpStream, status: &str, body: &str) -> Result<()> { + stream + .write_all( + format!( + "HTTP/1.1 {status}\r\ncontent-type: text/plain\r\ncontent-length: {}\r\nconnection: close\r\n\r\n{body}", + body.len() + ) + .as_bytes(), + ) + .await?; + Ok(()) +}