mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Restore app-server websocket listener with auth guard (#22404)
## Why PR #21843 removed the TCP websocket app-server listener, but that also removed functionality that still needs to exist. Restoring it as-is would reopen the old remote exposure problem, so this keeps the restored listener while making remote and non-loopback usage require explicit auth. ## What Changed - Mostly reverts #21843 and reapplies the small merge-conflict resolutions needed on top of current main. - Restores ws://IP:PORT parsing, the app-server TCP websocket acceptor, websocket auth CLI flags, and the associated tests. - The only intentional behavior change from the restored code is that non-loopback websocket listeners now fail startup unless --ws-auth capability-token or --ws-auth signed-bearer-token is configured. Loopback listeners remain available for local and SSH-forwarding workflows. ## Reviewer Focus Please focus review on the small auth-enforcement delta layered on top of the revert: - codex-rs/app-server-transport/src/transport/websocket.rs: start_websocket_acceptor now rejects unauthenticated non-loopback websocket binds before accepting connections. - codex-rs/app-server-transport/src/transport/auth.rs: helper logic classifies unauthenticated non-loopback listeners. - codex-rs/app-server/tests/suite/v2/connection_handling_websocket.rs: tests cover unauthenticated ws://0.0.0.0 startup rejection and authenticated non-loopback capability-token startup. Everything else is intended to be revert/merge-conflict restoration rather than new product behavior. ## Verification - Manually verified that TUI remoting is restored and that auth is enforced for non-localhost urls.
This commit is contained in:
@@ -1,7 +1,7 @@
|
||||
//! Tracing helpers shared by socket and in-process app-server entry points.
|
||||
//!
|
||||
//! The in-process path intentionally reuses the same span shape as JSON-RPC
|
||||
//! transports so request telemetry stays comparable across stdio, unix socket,
|
||||
//! transports so request telemetry stays comparable across stdio, websocket,
|
||||
//! and embedded callers. [`typed_request_span`] is the in-process counterpart
|
||||
//! of [`request_span`] and stamps `rpc.transport` as `"in-process"` while
|
||||
//! deriving client identity from the typed [`ClientRequest`] rather than
|
||||
@@ -86,6 +86,7 @@ fn transport_name(transport: &AppServerTransport) -> &'static str {
|
||||
match transport {
|
||||
AppServerTransport::Stdio => "stdio",
|
||||
AppServerTransport::UnixSocket { .. } => "unix_socket",
|
||||
AppServerTransport::WebSocket { .. } => "websocket",
|
||||
AppServerTransport::Off => "off",
|
||||
}
|
||||
}
|
||||
|
||||
@@ -31,10 +31,12 @@ use crate::transport::ConnectionState;
|
||||
use crate::transport::OutboundConnectionState;
|
||||
use crate::transport::RemoteControlStartConfig;
|
||||
use crate::transport::TransportEvent;
|
||||
use crate::transport::auth::policy_from_settings;
|
||||
use crate::transport::route_outgoing_envelope;
|
||||
use crate::transport::start_control_socket_acceptor;
|
||||
use crate::transport::start_remote_control;
|
||||
use crate::transport::start_stdio_connection;
|
||||
use crate::transport::start_websocket_acceptor;
|
||||
use codex_analytics::AppServerRpcTransport;
|
||||
use codex_app_server_protocol::ConfigLayerSource;
|
||||
use codex_app_server_protocol::ConfigWarningNotification;
|
||||
@@ -101,6 +103,9 @@ pub use crate::error_code::INPUT_TOO_LARGE_ERROR_CODE;
|
||||
pub use crate::error_code::INVALID_PARAMS_ERROR_CODE;
|
||||
pub use crate::transport::AppServerTransport;
|
||||
pub use crate::transport::app_server_control_socket_path;
|
||||
pub use crate::transport::auth::AppServerWebsocketAuthArgs;
|
||||
pub use crate::transport::auth::AppServerWebsocketAuthSettings;
|
||||
pub use crate::transport::auth::WebsocketAuthCliMode;
|
||||
|
||||
const LOG_FORMAT_ENV_VAR: &str = "LOG_FORMAT";
|
||||
const OTEL_SERVICE_NAME: &str = "codex-app-server";
|
||||
@@ -378,6 +383,7 @@ pub async fn run_main(
|
||||
default_analytics_enabled,
|
||||
AppServerTransport::Stdio,
|
||||
SessionSource::VSCode,
|
||||
AppServerWebsocketAuthSettings::default(),
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -410,6 +416,7 @@ pub async fn run_main_with_transport(
|
||||
default_analytics_enabled: bool,
|
||||
transport: AppServerTransport,
|
||||
session_source: SessionSource,
|
||||
auth: AppServerWebsocketAuthSettings,
|
||||
) -> IoResult<()> {
|
||||
run_main_with_transport_options(
|
||||
arg0_paths,
|
||||
@@ -418,6 +425,7 @@ pub async fn run_main_with_transport(
|
||||
default_analytics_enabled,
|
||||
transport,
|
||||
session_source,
|
||||
auth,
|
||||
AppServerRuntimeOptions::default(),
|
||||
)
|
||||
.await
|
||||
@@ -431,6 +439,7 @@ pub async fn run_main_with_transport_options(
|
||||
default_analytics_enabled: bool,
|
||||
transport: AppServerTransport,
|
||||
session_source: SessionSource,
|
||||
auth: AppServerWebsocketAuthSettings,
|
||||
runtime_options: AppServerRuntimeOptions,
|
||||
) -> IoResult<()> {
|
||||
let (transport_event_tx, mut transport_event_rx) =
|
||||
@@ -670,6 +679,16 @@ pub async fn run_main_with_transport_options(
|
||||
.await?;
|
||||
transport_accept_handles.push(accept_handle);
|
||||
}
|
||||
AppServerTransport::WebSocket { bind_address } => {
|
||||
let accept_handle = start_websocket_acceptor(
|
||||
*bind_address,
|
||||
transport_event_tx.clone(),
|
||||
transport_shutdown_token.clone(),
|
||||
policy_from_settings(&auth)?,
|
||||
)
|
||||
.await?;
|
||||
transport_accept_handles.push(accept_handle);
|
||||
}
|
||||
AppServerTransport::Off => {}
|
||||
}
|
||||
|
||||
@@ -741,7 +760,7 @@ pub async fn run_main_with_transport_options(
|
||||
}
|
||||
OutboundControlEvent::DisconnectAll => {
|
||||
info!(
|
||||
"disconnecting {} outbound connection(s) for graceful restart",
|
||||
"disconnecting {} outbound websocket connection(s) for graceful restart",
|
||||
outbound_connections.len()
|
||||
);
|
||||
for connection_state in outbound_connections.values() {
|
||||
@@ -1068,9 +1087,9 @@ pub async fn run_main_with_transport_options(
|
||||
fn analytics_rpc_transport(transport: &AppServerTransport) -> AppServerRpcTransport {
|
||||
match transport {
|
||||
AppServerTransport::Stdio => AppServerRpcTransport::Stdio,
|
||||
AppServerTransport::UnixSocket { .. } | AppServerTransport::Off => {
|
||||
AppServerRpcTransport::Websocket
|
||||
}
|
||||
AppServerTransport::UnixSocket { .. }
|
||||
| AppServerTransport::WebSocket { .. }
|
||||
| AppServerTransport::Off => AppServerRpcTransport::Websocket,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use clap::Parser;
|
||||
use codex_app_server::AppServerRuntimeOptions;
|
||||
use codex_app_server::AppServerTransport;
|
||||
use codex_app_server::AppServerWebsocketAuthArgs;
|
||||
use codex_app_server::PluginStartupTasks;
|
||||
use codex_app_server::run_main_with_transport_options;
|
||||
use codex_arg0::Arg0DispatchPaths;
|
||||
@@ -18,7 +19,7 @@ const DISABLE_MANAGED_CONFIG_ENV_VAR: &str = "CODEX_APP_SERVER_DISABLE_MANAGED_C
|
||||
#[derive(Debug, Parser)]
|
||||
struct AppServerArgs {
|
||||
/// Transport endpoint URL. Supported values: `stdio://` (default),
|
||||
/// `unix://`, `unix://PATH`, `off`.
|
||||
/// `unix://`, `unix://PATH`, `ws://IP:PORT`, `off`.
|
||||
#[arg(
|
||||
long = "listen",
|
||||
value_name = "URL",
|
||||
@@ -35,6 +36,9 @@ struct AppServerArgs {
|
||||
)]
|
||||
session_source: SessionSource,
|
||||
|
||||
#[command(flatten)]
|
||||
auth: AppServerWebsocketAuthArgs,
|
||||
|
||||
/// Hidden debug-only test hook used by integration tests that spawn the
|
||||
/// production app-server binary.
|
||||
#[cfg(debug_assertions)]
|
||||
@@ -58,6 +62,7 @@ fn main() -> anyhow::Result<()> {
|
||||
};
|
||||
let transport = args.listen;
|
||||
let session_source = args.session_source;
|
||||
let auth = args.auth.try_into_settings()?;
|
||||
let mut runtime_options = AppServerRuntimeOptions::default();
|
||||
#[cfg(debug_assertions)]
|
||||
if args.disable_plugin_startup_tasks_for_tests {
|
||||
@@ -72,6 +77,7 @@ fn main() -> anyhow::Result<()> {
|
||||
/*default_analytics_enabled*/ false,
|
||||
transport,
|
||||
session_source,
|
||||
auth,
|
||||
runtime_options,
|
||||
)
|
||||
.await?;
|
||||
|
||||
@@ -21,9 +21,11 @@ pub(crate) use codex_app_server_transport::QueuedOutgoingMessage;
|
||||
pub(crate) use codex_app_server_transport::RemoteControlStartConfig;
|
||||
pub(crate) use codex_app_server_transport::TransportEvent;
|
||||
pub use codex_app_server_transport::app_server_control_socket_path;
|
||||
pub use codex_app_server_transport::auth;
|
||||
pub(crate) use codex_app_server_transport::start_control_socket_acceptor;
|
||||
pub(crate) use codex_app_server_transport::start_remote_control;
|
||||
pub(crate) use codex_app_server_transport::start_stdio_connection;
|
||||
pub(crate) use codex_app_server_transport::start_websocket_acceptor;
|
||||
|
||||
pub(crate) struct ConnectionState {
|
||||
pub(crate) outbound_initialized: Arc<AtomicBool>,
|
||||
|
||||
Reference in New Issue
Block a user