mirror of
https://github.com/openai/codex.git
synced 2026-05-29 23:40:29 +00:00
fix(exec-server): reject websocket requests with Origin headers (#24947)
## Why `codex exec-server` has a local WebSocket listener, but it did not apply the same browser-origin request handling as the `app-server` WebSocket transport. Requests that carry an `Origin` header should not be upgraded by this local transport, keeping both local WebSocket servers consistent and avoiding unexpected browser-initiated connections. ## What changed - Added an Axum middleware guard in `codex-rs/exec-server/src/server/transport.rs` that returns `403 Forbidden` for requests carrying an `Origin` header. - Added an integration test in `codex-rs/exec-server/tests/websocket.rs` that covers rejection of an `Origin`-bearing WebSocket handshake. - Kept ordinary WebSocket clients unchanged: existing no-`Origin` initialization and process behavior remains covered by the crate tests. ## Validation - `just test -p codex-exec-server` test phase (`186 passed`; run outside the parent macOS sandbox so nested sandbox tests can execute) - `just clippy -p codex-exec-server`
This commit is contained in:
@@ -1,9 +1,15 @@
|
||||
use axum::Router;
|
||||
use axum::body::Body;
|
||||
use axum::extract::ConnectInfo;
|
||||
use axum::extract::State;
|
||||
use axum::extract::ws::WebSocketUpgrade;
|
||||
use axum::http::Request;
|
||||
use axum::http::StatusCode;
|
||||
use axum::http::header::ORIGIN;
|
||||
use axum::middleware;
|
||||
use axum::middleware::Next;
|
||||
use axum::response::IntoResponse;
|
||||
use axum::response::Response;
|
||||
use axum::routing::any;
|
||||
use axum::routing::get;
|
||||
use std::io::Write as _;
|
||||
@@ -13,6 +19,7 @@ use tokio::io::AsyncRead;
|
||||
use tokio::io::AsyncWrite;
|
||||
use tokio::net::TcpListener;
|
||||
use tracing::info;
|
||||
use tracing::warn;
|
||||
|
||||
use crate::ExecServerRuntimePaths;
|
||||
use crate::connection::JsonRpcConnection;
|
||||
@@ -123,6 +130,7 @@ async fn run_websocket_listener(
|
||||
let router = Router::new()
|
||||
.route("/", any(websocket_upgrade_handler))
|
||||
.route("/readyz", get(readiness_handler))
|
||||
.layer(middleware::from_fn(reject_requests_with_origin_header))
|
||||
.with_state(ExecServerWebSocketState { processor });
|
||||
axum::serve(
|
||||
listener,
|
||||
@@ -141,6 +149,22 @@ async fn readiness_handler() -> StatusCode {
|
||||
StatusCode::OK
|
||||
}
|
||||
|
||||
async fn reject_requests_with_origin_header(
|
||||
request: Request<Body>,
|
||||
next: Next,
|
||||
) -> Result<Response, StatusCode> {
|
||||
if request.headers().contains_key(ORIGIN) {
|
||||
warn!(
|
||||
method = %request.method(),
|
||||
uri = %request.uri(),
|
||||
"rejecting exec-server websocket listener request with Origin header"
|
||||
);
|
||||
Err(StatusCode::FORBIDDEN)
|
||||
} else {
|
||||
Ok(next.run(request).await)
|
||||
}
|
||||
}
|
||||
|
||||
async fn websocket_upgrade_handler(
|
||||
websocket: WebSocketUpgrade,
|
||||
ConnectInfo(peer_addr): ConnectInfo<SocketAddr>,
|
||||
|
||||
@@ -9,6 +9,12 @@ use codex_exec_server::InitializeParams;
|
||||
use codex_exec_server::InitializeResponse;
|
||||
use common::exec_server::exec_server;
|
||||
use pretty_assertions::assert_eq;
|
||||
use tokio_tungstenite::connect_async;
|
||||
use tokio_tungstenite::tungstenite::Error as WebSocketError;
|
||||
use tokio_tungstenite::tungstenite::client::IntoClientRequest;
|
||||
use tokio_tungstenite::tungstenite::http::HeaderValue;
|
||||
use tokio_tungstenite::tungstenite::http::StatusCode;
|
||||
use tokio_tungstenite::tungstenite::http::header::ORIGIN;
|
||||
use uuid::Uuid;
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
@@ -96,3 +102,24 @@ async fn exec_server_accepts_binary_websocket_json() -> anyhow::Result<()> {
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
|
||||
async fn exec_server_rejects_browser_origin_websocket_handshake() -> anyhow::Result<()> {
|
||||
let mut server = exec_server().await?;
|
||||
let mut request = server.websocket_url().into_client_request()?;
|
||||
request
|
||||
.headers_mut()
|
||||
.insert(ORIGIN, HeaderValue::from_static("https://evil.example"));
|
||||
|
||||
let error = match connect_async(request).await {
|
||||
Ok(_) => anyhow::bail!("browser-origin websocket handshake should be rejected"),
|
||||
Err(error) => error,
|
||||
};
|
||||
let WebSocketError::Http(response) = error else {
|
||||
anyhow::bail!("browser-origin websocket handshake failed unexpectedly: {error}");
|
||||
};
|
||||
assert_eq!(response.status(), StatusCode::FORBIDDEN);
|
||||
|
||||
server.shutdown().await?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user