Skip to content

Commit 17bc495

Browse files
committed
codex: address PR review feedback (#14853)
1 parent 6796e59 commit 17bc495

File tree

2 files changed

+69
-6
lines changed

2 files changed

+69
-6
lines changed

codex-rs/tui_app_server/src/lib.rs

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -280,7 +280,10 @@ async fn start_embedded_app_server(
280280
#[derive(Clone, Debug, PartialEq, Eq)]
281281
pub(crate) enum AppServerTarget {
282282
Embedded,
283-
Remote(String),
283+
Remote {
284+
websocket_url: String,
285+
auth_token: Option<String>,
286+
},
284287
}
285288

286289
fn remote_addr_has_explicit_port(addr: &str, parsed: &Url) -> bool {
@@ -312,6 +315,16 @@ fn remote_addr_has_explicit_port(addr: &str, parsed: &Url) -> bool {
312315
host_and_port == format!("{expected_host}:{explicit_default_port}")
313316
}
314317

318+
fn websocket_url_supports_auth_token(parsed: &Url) -> bool {
319+
match (parsed.scheme(), parsed.host()) {
320+
("wss", Some(_)) => true,
321+
("ws", Some(url::Host::Domain(domain))) => domain.eq_ignore_ascii_case("localhost"),
322+
("ws", Some(url::Host::Ipv4(addr))) => addr.is_loopback(),
323+
("ws", Some(url::Host::Ipv6(addr))) => addr.is_loopback(),
324+
_ => false,
325+
}
326+
}
327+
315328
pub fn normalize_remote_addr(addr: &str) -> color_eyre::Result<String> {
316329
let parsed = match Url::parse(addr) {
317330
Ok(parsed) => parsed,
@@ -336,9 +349,24 @@ pub fn normalize_remote_addr(addr: &str) -> color_eyre::Result<String> {
336349
);
337350
}
338351

339-
async fn connect_remote_app_server(websocket_url: String) -> color_eyre::Result<AppServerClient> {
352+
fn validate_remote_auth_token_transport(websocket_url: &str) -> color_eyre::Result<()> {
353+
let parsed = Url::parse(websocket_url).map_err(color_eyre::Report::new)?;
354+
if websocket_url_supports_auth_token(&parsed) {
355+
return Ok(());
356+
}
357+
358+
color_eyre::eyre::bail!(
359+
"remote auth tokens require `wss://` or loopback `ws://` URLs; got `{websocket_url}`"
360+
)
361+
}
362+
363+
async fn connect_remote_app_server(
364+
websocket_url: String,
365+
auth_token: Option<String>,
366+
) -> color_eyre::Result<AppServerClient> {
340367
let app_server = RemoteAppServerClient::connect(RemoteAppServerConnectArgs {
341368
websocket_url,
369+
auth_token,
342370
client_name: "codex-tui".to_string(),
343371
client_version: env!("CARGO_PKG_VERSION").to_string(),
344372
experimental_api: true,
@@ -370,9 +398,10 @@ async fn start_app_server(
370398
)
371399
.await
372400
.map(AppServerClient::InProcess),
373-
AppServerTarget::Remote(websocket_url) => {
374-
connect_remote_app_server(websocket_url.clone()).await
375-
}
401+
AppServerTarget::Remote {
402+
websocket_url,
403+
auth_token,
404+
} => connect_remote_app_server(websocket_url.clone(), auth_token.clone()).await,
376405
}
377406
}
378407

@@ -586,11 +615,18 @@ pub async fn run_main(
586615
arg0_paths: Arg0DispatchPaths,
587616
loader_overrides: LoaderOverrides,
588617
remote: Option<String>,
618+
remote_auth_token: Option<String>,
589619
) -> std::io::Result<AppExitInfo> {
590620
let remote_url = remote;
621+
if let (Some(websocket_url), Some(_)) = (remote_url.as_deref(), remote_auth_token.as_ref()) {
622+
validate_remote_auth_token_transport(websocket_url).map_err(std::io::Error::other)?;
623+
}
591624
let app_server_target = remote_url
592625
.clone()
593-
.map(AppServerTarget::Remote)
626+
.map(|websocket_url| AppServerTarget::Remote {
627+
websocket_url,
628+
auth_token: remote_auth_token.clone(),
629+
})
594630
.unwrap_or(AppServerTarget::Embedded);
595631
let (sandbox_mode, approval_policy) = if cli.full_auto {
596632
(
@@ -1682,6 +1718,32 @@ mod tests {
16821718
);
16831719
}
16841720

1721+
#[test]
1722+
fn remote_auth_token_transport_accepts_loopback_ws() {
1723+
validate_remote_auth_token_transport("ws://127.0.0.1:4500/")
1724+
.expect("loopback ws should be allowed for auth tokens");
1725+
validate_remote_auth_token_transport("ws://localhost:4500/")
1726+
.expect("localhost ws should be allowed for auth tokens");
1727+
validate_remote_auth_token_transport("ws://[::1]:4500/")
1728+
.expect("ipv6 loopback ws should be allowed for auth tokens");
1729+
}
1730+
1731+
#[test]
1732+
fn remote_auth_token_transport_accepts_secure_wss() {
1733+
validate_remote_auth_token_transport("wss://example.com:443/")
1734+
.expect("wss should be allowed for auth tokens");
1735+
}
1736+
1737+
#[test]
1738+
fn remote_auth_token_transport_rejects_non_loopback_ws() {
1739+
let err = validate_remote_auth_token_transport("ws://example.com:4500/")
1740+
.expect_err("non-loopback ws should be rejected for auth tokens");
1741+
assert!(
1742+
err.to_string()
1743+
.contains("remote auth tokens require `wss://` or loopback `ws://` URLs")
1744+
);
1745+
}
1746+
16851747
#[tokio::test]
16861748
async fn latest_session_lookup_params_keep_local_filters_for_embedded_sessions()
16871749
-> std::io::Result<()> {

codex-rs/tui_app_server/src/main.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ fn main() -> anyhow::Result<()> {
2727
arg0_paths,
2828
codex_core::config_loader::LoaderOverrides::default(),
2929
/*remote*/ None,
30+
/*remote_auth_token*/ None,
3031
)
3132
.await?;
3233
let token_usage = exit_info.token_usage;

0 commit comments

Comments
 (0)