From 99065cb8ed7e31a41882c5691e4a09c6c248c103 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 1 Nov 2026 07:06:28 +0000 Subject: [PATCH 1/6] Bind repository agent tools to the authenticated chat session Agent-Session: https://cmux.com/deeplink/agent?id=aid-483ddc0ed5b690116566 --- cmux-tui/crates/acpmux/src/agent_tools.rs | 3 + .../crates/acpmux/src/agent_tools_tests.rs | 26 +++++ cmux-tui/crates/acpmux/src/client.rs | 13 +++ cmux-tui/crates/acpmux/src/cloud_repo_mcp.rs | 67 +++++++++++- cmux-tui/crates/acpmux/src/server/requests.rs | 2 + .../crates/acpmux/src/server/team_tests.rs | 23 ++++ .../cmux-tui-core/src/server/cloud_repos.rs | 101 +++++++++++++++++- .../crates/cmux-tui/src/cli/cloud_repo.rs | 11 +- 8 files changed, 232 insertions(+), 14 deletions(-) diff --git a/cmux-tui/crates/acpmux/src/agent_tools.rs b/cmux-tui/crates/acpmux/src/agent_tools.rs index 9c2d0f7383ff..e260bdb81f2f 100644 --- a/cmux-tui/crates/acpmux/src/agent_tools.rs +++ b/cmux-tui/crates/acpmux/src/agent_tools.rs @@ -781,6 +781,9 @@ impl AgentTools { .map(|(k, v)| (k.to_owned(), v)), ); } + if server.name == crate::cloud_repo_mcp::SERVER_NAME { + server.env.push(("ACPMUX_SESSION_ID".to_owned(), session_id.to_owned())); + } server.env.extend(repo_route); } self diff --git a/cmux-tui/crates/acpmux/src/agent_tools_tests.rs b/cmux-tui/crates/acpmux/src/agent_tools_tests.rs index 07ebe7333d1a..2b374ca06626 100644 --- a/cmux-tui/crates/acpmux/src/agent_tools_tests.rs +++ b/cmux-tui/crates/acpmux/src/agent_tools_tests.rs @@ -197,3 +197,29 @@ fn a_launch_without_servers_removes_the_last_mcp_config() { assert!(!file.exists(), "the last launch's config stayed for the hook to read"); let _ = std::fs::remove_dir_all(&root); } + +#[test] +fn cloud_repo_tools_receive_complete_protected_caller_identity() { + let tools = super::AgentTools { + servers: vec![super::McpServer { + name: crate::cloud_repo_mcp::SERVER_NAME.to_owned(), + command: "/bin/acpmux".into(), + args: vec!["repo-mcp".into()], + env: vec![ + ("ACPMUX_SESSION_ID".into(), "foreign".into()), + ("CMUX_BIN".into(), "/bin/cmux".into()), + ], + }], + ..Default::default() + } + .for_session("own-chat"); + let env: std::collections::HashMap<_, _> = tools.servers[0].env.iter().cloned().collect(); + assert_eq!(env["ACPMUX_SESSION_ID"], "own-chat"); + assert_eq!(env[crate::cloud_repo_mcp::SESSION_ENV], "own-chat"); + assert_eq!( + env[crate::hub::person::CALLER_TOKEN_ENV], + crate::hub::person::caller_token("own-chat") + ); + assert_eq!(env[crate::hub::person::CALLER_SOCKET_ENV], env["ACPMUX_SOCKET"]); + assert_eq!(env["CMUX_BIN"], "/bin/cmux"); +} diff --git a/cmux-tui/crates/acpmux/src/client.rs b/cmux-tui/crates/acpmux/src/client.rs index 3dbeb394ae86..86c8d402f5c3 100644 --- a/cmux-tui/crates/acpmux/src/client.rs +++ b/cmux-tui/crates/acpmux/src/client.rs @@ -16,6 +16,8 @@ use tokio::sync::{Mutex, mpsc, oneshot}; pub struct Client { /// Build id the daemon reported at initialize, for mismatch hints. daemon_build: std::sync::Mutex>, + /// The session this connection proved at initialize, never the caller's claim. + proven_caller_session: std::sync::OnceLock, /// Absolute path to the CLI bundled with the app that started the daemon, /// when the daemon was launched by an app. A standalone acpmux CLI does /// not inherit the app's environment, so this is how `--group` finds the @@ -106,6 +108,7 @@ impl Client { } let client = Arc::new(Self { daemon_build: std::sync::Mutex::new(None), + proven_caller_session: std::sync::OnceLock::new(), daemon_cli_path: std::sync::Mutex::new(None), daemon_features: std::sync::Mutex::new(Vec::new()), person_challenge: std::sync::Mutex::new(None), @@ -138,6 +141,10 @@ impl Client { hello.set("_meta", json!({"acpmux": meta})); } let init = client.request(method::INITIALIZE, hello).await?; + if let Some(session) = init.pointer("/_meta/acpmux/callerSessionId").and_then(Value::as_str) + { + let _ = client.proven_caller_session.set(session.to_owned()); + } *client.daemon_build.lock_or_recover() = init.pointer("/_meta/acpmux/build").and_then(Value::as_str).map(str::to_owned); *client.daemon_cli_path.lock_or_recover() = init @@ -159,6 +166,12 @@ impl Client { Ok(client) } + /// The session the daemon authenticated on this connection. Older daemons + /// report none, so consumers requiring a caller binding fail closed. + pub fn proven_caller_session(&self) -> Option<&str> { + self.proven_caller_session.get().map(String::as_str) + } + /// Proves this connection is the person (`_acpmux/person_prove`) with the /// key the daemon was started with; must be the request right after /// initialize. True when the daemon accepted the proof. diff --git a/cmux-tui/crates/acpmux/src/cloud_repo_mcp.rs b/cmux-tui/crates/acpmux/src/cloud_repo_mcp.rs index bf2df41012b6..ade97fa754ba 100644 --- a/cmux-tui/crates/acpmux/src/cloud_repo_mcp.rs +++ b/cmux-tui/crates/acpmux/src/cloud_repo_mcp.rs @@ -2,7 +2,8 @@ //! //! The agent and the web UI use the same daemon actions. This stdio server //! forwards the chat ID supplied by acpmux; it never trusts a session id -//! supplied by tool arguments. +//! supplied by tool arguments. The acpmux connection must also prove the +//! same session with its caller token before any CLI operation runs. use crate::client::Client; use crate::secret_mcp::error_result; @@ -115,7 +116,7 @@ fn result_text(result: &Value) -> String { /// it without a shell keeps the chat ID in an argv value only /// (never interpolated into a command string), and makes this bridge work with /// the same socket as `cmux cloud repo`. -async fn run_cli(action: &str, request: Value) -> Result { +async fn run_cli(action: &'static str, request: Value) -> Result { let binary = std::env::var_os("CMUX_BIN").unwrap_or_else(|| "cmux".into()); run_cli_with(&binary, std::env::var_os("CMUX_SOCKET_PATH").as_deref(), action, request).await } @@ -197,7 +198,25 @@ fn args_object(params: &Value) -> Result { } } -pub async fn call(_daemon: &Arc, session: &str, params: &Value) -> Value { +pub async fn call(daemon: &Arc, session: &str, params: &Value) -> Value { + call_for_caller(daemon.proven_caller_session(), session, params, run_cli).await +} + +async fn call_for_caller( + caller: Option<&str>, + session: &str, + params: &Value, + run: F, +) -> Value +where + F: Fn(&'static str, Value) -> Fut, + Fut: std::future::Future>, +{ + if caller != Some(session) { + return error_result( + "Repository tools could not verify this chat. Restart the chat session, then retry Add repository.".to_owned(), + ); + } let args = match args_object(params) { Ok(v) => v, Err(v) => return v, @@ -211,7 +230,7 @@ pub async fn call(_daemon: &Arc, session: &str, params: &Value) -> Value { req.set("query", json!(query)); } - match run_cli(REPO_LIST_ACTION, req).await { + match run(REPO_LIST_ACTION, req).await { Ok(result) => ok(result), Err(e) => error_result(e), } @@ -228,7 +247,7 @@ pub async fn call(_daemon: &Arc, session: &str, params: &Value) -> Value { req.set("ref", json!(reference)); } - match run_cli(REPO_ADD_ACTION, req).await { + match run(REPO_ADD_ACTION, req).await { Ok(mut result) => { enrich_retry(&mut result, session, repo, args.get("ref")); ok(result) @@ -268,6 +287,44 @@ fn enrich_retry(result: &mut Value, session: &str, repo: &str, reference: Option #[cfg(test)] mod tests { use super::*; + #[tokio::test] + async fn cloud_repo_missing_or_foreign_caller_never_invokes_cli() { + for caller in [None, Some("other-chat")] { + for tool in ["list_repos", "add_repo"] { + let result = call_for_caller( + caller, + "own-chat", + &json!({"name":tool,"arguments":{"repo":"org/repo"}}), + |_, _| async { panic!("unauthenticated caller reached the CLI") }, + ) + .await; + assert_eq!(result["isError"], true); + assert!( + result["content"][0]["text"] + .as_str() + .unwrap() + .contains("Restart the chat session") + ); + } + } + } + + #[tokio::test] + async fn cloud_repo_proven_caller_uses_protected_session() { + let result = call_for_caller( + Some("own-chat"), + "own-chat", + &json!({"name":"add_repo","arguments":{"repo":"org/repo","session":"other-chat"}}), + |action, request| async move { + assert_eq!(action, REPO_ADD_ACTION); + assert_eq!(request["session"], "own-chat"); + Ok(json!({"state":"done","repo":"org/repo","folder":"/home/chats/own-chat/repo"})) + }, + ) + .await; + assert_eq!(result["isError"], false); + } + #[test] fn retry_shape_is_stable() { let mut result = json!({"code":"repo.needs_github","message":"Connect GitHub."}); diff --git a/cmux-tui/crates/acpmux/src/server/requests.rs b/cmux-tui/crates/acpmux/src/server/requests.rs index b9790b2d4710..6682d3c0cc24 100644 --- a/cmux-tui/crates/acpmux/src/server/requests.rs +++ b/cmux-tui/crates/acpmux/src/server/requests.rs @@ -910,6 +910,8 @@ async fn dispatch_request( // This connection is the person (`hub/person.rs`): never yet, // as the proof comes after this reply. "person": conn.is_person(), + // Only a verified first-initialize caller token supplies this ID. + "callerSessionId": conn.caller(), // The challenge a local connection proves the person key against. "personChallenge": if conn.challengeable() { json!({"nonce": conn.nonce, "connection": conn.id}) diff --git a/cmux-tui/crates/acpmux/src/server/team_tests.rs b/cmux-tui/crates/acpmux/src/server/team_tests.rs index fb7cf7850ced..8000633df2bf 100644 --- a/cmux-tui/crates/acpmux/src/server/team_tests.rs +++ b/cmux-tui/crates/acpmux/src/server/team_tests.rs @@ -143,6 +143,10 @@ impl Rpc { } let result = client.call("initialize", hello).await; assert!(result.get("error").is_none(), "{result}"); + let expected = + caller.filter(|(_, valid)| *valid && origin == Origin::Local).map(|(id, _)| id); + assert_eq!(result["result"]["_meta"]["acpmux"]["callerSessionId"].as_str(), expected); + client } @@ -494,3 +498,22 @@ async fn team_receipt_origin_survives_restart_and_gates_owner_sends_without_wire assert_eq!(reply["error"]["data"]["reason"], "team_messaging_disabled", "{reply}"); std::fs::remove_dir_all(dir).unwrap(); } + +#[tokio::test] +async fn cloud_repo_initialize_reports_only_the_verified_local_caller() { + let (hub, _) = setup(false); + for caller in [None, Some((OWN, true)), Some((OWN, false))] { + let _client = Rpc::connect(&hub, Origin::Local, caller).await; + } + // A later initialize cannot replace the first connection binding. + let mut client = Rpc::connect(&hub, Origin::Local, Some((OWN, true))).await; + let reply = client + .call( + "initialize", + json!({"_meta":{"acpmux":{"callerSession":{ + "sessionId":OTHER,"token":crate::hub::person::caller_token(OTHER) + }}}}), + ) + .await; + assert_eq!(reply["result"]["_meta"]["acpmux"]["callerSessionId"], OWN); +} diff --git a/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs b/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs index 10b6a8bd76d8..488947f658d2 100644 --- a/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs +++ b/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs @@ -17,11 +17,7 @@ pub(super) fn try_handle( let cmd = crate::cloud_repos::frame_command(line)?; let request: Value = serde_json::from_str(line).ok()?; let id = request.get("id").cloned().unwrap_or(Value::Null); - let transport = mux.control_clients.transport_of(client); - let permitted = matches!(transport, Some(ClientTransport::Unix | ClientTransport::Remote)) - && mux.frame_principal(client).is_some() - && (!matches!(cmd.as_str(), "cloud.repo.local" | "cloud.repo.remember") - || matches!(transport, Some(ClientTransport::Unix))); + let permitted = permitted(mux, client, &cmd, &request); if !permitted { return Some(writer.send_control(&json!({"id":id,"ok":false,"error_code":"remote_denied","error":"permission denied"})).is_ok()); } @@ -53,3 +49,98 @@ pub(super) fn try_handle( } } } + +/// Local user and linked Mac requests retain their existing access. A +/// presented agent identity, however, may address only its own chat. +fn permitted(mux: &Mux, client: u64, cmd: &str, request: &Value) -> bool { + let transport = mux.control_clients.transport_of(client); + if !matches!(transport, Some(ClientTransport::Unix | ClientTransport::Remote)) + || mux.frame_principal(client).is_none() + || (matches!(cmd, "cloud.repo.local" | "cloud.repo.remember") + && !matches!(transport, Some(ClientTransport::Unix))) + { + return false; + } + let credential = match request.get("credential") { + None => None, + Some(Value::String(value)) if !value.is_empty() => Some(value.as_str()), + Some(_) => return false, + }; + match mux.request_actor(super::origin_gate::connection_actor(mux, client), credential) { + Ok(crate::Actor::AcpSession { id, .. }) => request["session"].as_str() == Some(id.as_str()), + Ok(_) => true, + Err(_) => false, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::server::{BoundedOutbound, QueuedSink}; + + fn client(mux: &Mux, transport: ClientTransport) -> (u64, MessageWriter, Arc) { + let outbound = Arc::new(BoundedOutbound::default()); + let writer = MessageWriter::new(QueuedSink { outbound: outbound.clone(), control: None }); + (mux.control_clients.register(transport, writer.clone()), writer, outbound) + } + + #[test] + fn cloud_repo_agent_credential_cannot_target_another_chat() { + let mux = Mux::new_for_test("repo-auth", crate::SurfaceOptions::default()); + let (client, writer, outbound) = client(&mux, ClientTransport::Unix); + let minted = mux.mint_acp_session_credential("own-chat", None).unwrap(); + let request = json!({"id":1,"cmd":"cloud.repo.add","session":"other-chat", + "repo":"org/repo","credential":minted["credential"]}); + assert_eq!(try_handle(&mux, client, &request.to_string(), &writer), Some(true)); + let reply: Value = serde_json::from_str(&outbound.try_pop().unwrap()).unwrap(); + assert_eq!(reply["error_code"], "remote_denied"); + let own = json!({"session":"own-chat","credential":minted["credential"]}); + assert!(permitted(&mux, client, "cloud.repo.add", &own)); + for credential in [Value::Null, json!(12), json!("forged"), json!("")] { + assert!(!permitted( + &mux, + client, + "cloud.repo.add", + &json!({"session":"own-chat","credential":credential}) + )); + } + assert!(!permitted( + &mux, + client, + "cloud.repo.sync", + &json!({"credential":minted["credential"]}) + )); + } + + #[test] + fn cloud_repo_local_user_and_authenticated_mac_keep_access() { + let mux = Mux::new_for_test("repo-auth-routes", crate::SurfaceOptions::default()); + let (local, _, _) = client(&mux, ClientTransport::Unix); + let request = json!({"session":"any-chat"}); + assert!(permitted(&mux, local, "cloud.repo.add", &request)); + assert!(permitted(&mux, local, "cloud.repo.local", &json!({}))); + let (remote, _, _) = client(&mux, ClientTransport::Remote); + assert!(!permitted(&mux, remote, "cloud.repo.add", &request)); + let peer = crate::remote_relay_state::LinkPeer { + install: "repo-mac".into(), + user: "user".into(), + team: "team".into(), + }; + mux.record_remote_check(&peer.install).unwrap(); + mux.bind_remote_peer(remote, &peer).unwrap(); + assert!(permitted(&mux, remote, "cloud.repo.add", &request)); + assert!(permitted(&mux, remote, "cloud.repo.sync", &json!({}))); + assert!(!permitted(&mux, remote, "cloud.repo.local", &json!({}))); + let minted = mux.mint_acp_session_credential("any-chat", None).unwrap(); + assert!(!permitted( + &mux, + remote, + "cloud.repo.add", + &json!({"session":"any-chat","credential":minted["credential"]}) + )); + mux.revoke_remote_install(&peer.install).unwrap(); + assert!(!permitted(&mux, remote, "cloud.repo.add", &request)); + let (web, _, _) = client(&mux, ClientTransport::WebSocket); + assert!(!permitted(&mux, web, "cloud.repo.add", &request)); + } +} diff --git a/cmux-tui/crates/cmux-tui/src/cli/cloud_repo.rs b/cmux-tui/crates/cmux-tui/src/cli/cloud_repo.rs index 717a2ac17d13..459ee3277855 100644 --- a/cmux-tui/crates/cmux-tui/src/cli/cloud_repo.rs +++ b/cmux-tui/crates/cmux-tui/src/cli/cloud_repo.rs @@ -162,7 +162,7 @@ pub(super) fn call(global: &GlobalArgs, action: &str, args: Value) -> Result Result Result Date: Sun, 1 Nov 2026 07:28:57 +0000 Subject: [PATCH 2/6] Validate every repository installation link before rendering recovery Agent-Session: https://cmux.com/deeplink/agent?id=aid-483ddc0ed5b690116566 --- .../cmux-tui-core/src/cloud_repos/engine.rs | 39 +++- .../cmux-tui-core/src/cloud_repos/runtime.rs | 58 ++--- .../src/cloud_repos/runtime_tests.rs | 199 +++++++++++++++++- .../cmux-tui-core/src/cloud_repos/store.rs | 19 +- .../src/cloud_repos/store_tests.rs | 23 ++ .../cmux-tui-core/src/cloud_repos/tests.rs | 45 ++++ plans/cmux-next/cloud-repo-actions.md | 7 + 7 files changed, 348 insertions(+), 42 deletions(-) diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs index 4f6916d5d358..f72a87cd5b9f 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs @@ -24,12 +24,39 @@ impl Failure { Self(json!({"code":code,"message":message})) } pub fn needs_github(url: &str) -> Self { + if !valid_install_url(url) { + return install_link_required(); + } Self( json!({"code":"repo.needs_github", "message":"Connect GitHub in the card. This add will retry automatically when connected; no new message is needed.", "install_url":url}), ) } } +/// All installation-link sources share this origin check. URL parsing strips +/// some controls, so reject them before parsing rather than trusting its output. +pub(super) fn valid_install_url(value: &str) -> bool { + value.trim() == value + && !value.chars().any(char::is_control) + && value.split_once("://").is_some_and(|(_, rest)| { + !rest.split(['/', '?', '#']).next().unwrap_or_default().contains('@') + }) + && url::Url::parse(value).is_ok_and(|url| { + url.scheme() == "https" + && url.host_str() == Some("github.com") + && url.username().is_empty() + && url.password().is_none() + && url.port().is_none() + }) +} + +pub(super) fn install_link_required() -> Failure { + Failure::new( + "seed.bad_request", + "Open Add repository on your Mac to refresh the GitHub installation link, then retry.", + ) +} + pub(crate) trait Backend { fn sources(&self) -> Result, Failure>; fn connector(&self, query: Option<&str>) -> Result; @@ -133,6 +160,9 @@ pub(crate) fn list(backend: &impl Backend, query: Option<&str>) -> Result Failure { "repo.no_access", "Install GitHub access for this organization, or request access from its owner, then retry Add repository.", ); - for key in ["org", "install_url"] { - if let Some(v) = value.get(key) { - result.0.set(key, v.clone()); - } + if let Some(org) = value["org"].as_str() { + result.0.set("org", json!(org)); + } + if let Some(url) = value["install_url"].as_str().filter(|url| valid_install_url(url)) { + result.0.set("install_url", json!(url)); } return result; } diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs index c26ba6bb72b7..63dd813da181 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs @@ -73,19 +73,20 @@ impl<'a> Runtime<'a> { fn seed_failure(&self, value: &Value, repo: &str) -> Result { let mut failure = super::engine::seed_failure(value); - if failure.0.get("code").and_then(Value::as_str) == Some("repo.needs_github") { + // Read the raw seed code: the engine may reject its unsafe/missing + // URL, but the connector and the Mac can still supply a trusted one. + let code = value + .get("error_code") + .and_then(Value::as_str) + .or_else(|| value.get("code").and_then(Value::as_str)); + if matches!(code, Some("seed.needs_github" | "repo.needs_github")) { // A linked installation can still exclude this organization or // repo. Its recovery is access, not reconnecting the same account. let catalog = self.connector(Some(repo))?; if let Some(url) = catalog.install_url { failure = Failure::needs_github(&url); } else { - let url = value - .get("install_url") - .and_then(Value::as_str) - .filter(|s| !s.is_empty()) - .map(str::to_owned) - .or(store::install_url(self.mux)?); + let url = self.optional_install_url(value)?; failure = Failure::new( "repo.no_access", "Install GitHub access for this organization, or request access from its owner, then retry Add repository.", @@ -97,6 +98,10 @@ impl<'a> Runtime<'a> { failure.0.set("install_url", json!(url)); } } + } else if failure.0["code"] == "repo.no_access" { + if let Some(url) = self.optional_install_url(value)? { + failure.0.set("install_url", json!(url)); + } } Ok(failure) } @@ -114,19 +119,20 @@ impl<'a> Runtime<'a> { } } + fn optional_install_url(&self, response: &Value) -> Result, Failure> { + // The Mac obtained this link as the signed-in user; prefer it to + // links carried by a connector or seed error. Both paths are checked. + Ok(store::install_url(self.mux)?.or_else(|| { + response + .get("install_url") + .and_then(Value::as_str) + .filter(|url| super::engine::valid_install_url(url)) + .map(str::to_owned) + })) + } + fn install_url(&self, response: &Value) -> Result { - response - .get("install_url") - .and_then(Value::as_str) - .filter(|url| !url.is_empty()) - .map(str::to_owned) - .or(store::install_url(self.mux)?) - .ok_or_else(|| { - Failure::new( - "seed.bad_request", - "Open Add repository on your Mac to connect GitHub, then retry.", - ) - }) + self.optional_install_url(response)?.ok_or_else(super::engine::install_link_required) } } impl Backend for Runtime<'_> { @@ -173,14 +179,10 @@ impl Backend for Runtime<'_> { "Retry repository discovery. If it still fails, reconnect GitHub from Add repository." }, ); - if code == "repo.no_access" - && let Some(url) = error - .get("install_url") - .and_then(Value::as_str) - .map(str::to_owned) - .or(store::install_url(self.mux)?) - { - failure.0.set("install_url", json!(url)); + if code == "repo.no_access" { + if let Some(url) = self.optional_install_url(error)? { + failure.0.set("install_url", json!(url)); + } } return Err(failure); } @@ -320,7 +322,7 @@ impl Backend for Runtime<'_> { return Err(self.seed_failure(&status, repo)?); } Some("running") if Instant::now() < deadline => { - std::thread::sleep(Duration::from_millis(250)); + std::thread::sleep(Duration::from_millis(250)) } Some("running") => { return Err(Failure::new( diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime_tests.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime_tests.rs index f1b2b01d59d3..04af61e09205 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime_tests.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime_tests.rs @@ -77,7 +77,10 @@ fn cloud_repo_runtime_connector_auth_error_keeps_connect_and_automatic_retry() { store::sync(&fixture.mux, &json!({"repos":[],"install_url":url})).unwrap(); let mut error = json!({"code":"repo.needs_github"}); if supplied_url { - error.set("install_url", json!(url)); + error.set( + "install_url", + json!("https://github.com/apps/cmux/installations/new?state=connector"), + ); } let unlinked = json!({"ok":false,"error":error}); let agent = fixture.server( @@ -430,3 +433,197 @@ fn cloud_repo_runtime_connector_pages_and_rejects_repeated_cursor() { assert_eq!(asks[1], json!({"github_repos":{"query":"tool","cursor":"page-2"}})); assert_eq!(asks[3], json!({"github_repos":{"cursor":"again"}})); } + +#[test] +fn cloud_repo_runtime_connector_links_prefer_mac_and_reject_unsafe_fallbacks() { + let trusted = "https://github.com/apps/cmux/installations/new?state=mac"; + let response_url = "https://github.com/apps/cmux/installations/new?state=connector"; + for source in ["connect", "repo.needs_github", "repo.no_access"] { + for cached in [false, true] { + for url in [response_url, "https://github.com@evil.example/install"] { + let fixture = Fixture::new(); + if cached { + store::sync(&fixture.mux, &json!({"repos":[],"install_url":trusted})).unwrap(); + } + let reply = if source == "connect" { + json!({"ok":true,"repos":[],"connect":{"install_url":url}}) + } else { + json!({"ok":false,"error":{"code":source,"install_url":url}}) + }; + let agent = fixture.server("agent.sock", vec![reply]); + let runtime = fixture.runtime(); + let expected = if cached { + Some(trusted) + } else if url == response_url { + Some(response_url) + } else { + None + }; + match runtime.connector(None) { + Ok(catalog) => { + assert_ne!(source, "repo.no_access"); + assert!(expected.is_some()); + assert_eq!(catalog.install_url.as_deref(), expected); + } + Err(failure) => { + if source == "repo.no_access" { + assert_eq!(failure.0["code"], "repo.no_access"); + assert_eq!(failure.0["install_url"].as_str(), expected); + } else { + assert!(expected.is_none()); + assert_eq!(failure.0["code"], "seed.bad_request"); + assert!(failure.0.get("install_url").is_none()); + assert!(failure.0["message"].as_str().unwrap().contains("Mac")); + } + } + } + assert_eq!(agent.join().unwrap().len(), 1); + // Connector replies never populate the Mac-owned cache. + assert_eq!( + store::install_url(&fixture.mux).unwrap().as_deref(), + cached.then_some(trusted) + ); + } + } + } +} + +#[test] +fn cloud_repo_runtime_unsafe_connector_link_never_enters_list_wait_or_retry_card() { + let fixture = Fixture::new(); + let unsafe_url = "https://user:secret@github.com:444/install"; + let reply = json!({"ok":true,"repos":[],"connect":{"install_url":unsafe_url}}); + let agent = fixture.server("agent.sock", vec![reply.clone(), reply.clone(), reply]); + let runtime = fixture.runtime(); + for failure in [ + list(&runtime, None).unwrap_err().0, + runtime.wait_github(0).unwrap_err().0, + add(&runtime, &json!({"session":fixture.session,"repo":"tool"})).unwrap_err().0, + ] { + assert_eq!(failure["code"], "seed.bad_request"); + assert!(!failure.to_string().contains(unsafe_url)); + assert!(failure.get("retry").is_none()); + } + assert_eq!(agent.join().unwrap().len(), 3); +} + +#[test] +fn cloud_repo_runtime_seed_links_are_checked_without_losing_connect_recovery() { + let mac_url = "https://github.com/apps/cmux/installations/new?state=mac"; + let seed_url = "https://github.com/apps/cmux/installations/new?state=seed"; + let connector_url = "https://github.com/apps/cmux/installations/new?state=connector"; + for details in [false, true] { + for cached in [false, true] { + for code in + ["seed.needs_github", "repo.needs_github", "seed.no_access", "repo.no_access"] + { + for url in [seed_url, "javascript:alert(1)"] { + let fixture = Fixture::new(); + if cached { + store::sync(&fixture.mux, &json!({"repos":[],"install_url":mac_url})) + .unwrap(); + } + let needs_github = code.ends_with("needs_github"); + let agent = needs_github.then(|| { + fixture.server( + "agent.sock", + vec![json!({ + "ok":true,"repos":[],"connect":{"install_url":connector_url} + })], + ) + }); + let replies = if details { + vec![ + json!({"ok":false,"error_code":code,"error_details":{"install_url":url}}), + ] + } else { + vec![ + seed_reply(json!({"job":"seed-link"})), + seed_reply(json!({ + "state":"failed","error_code":code,"install_url":url + })), + ] + }; + let seed = fixture.server("seed.sock", replies); + let args = json!({"session":fixture.session,"repo":"owner/tool"}); + let failure = add(&fixture.runtime(), &args).unwrap_err().0; + let expected = if cached { + Some(mac_url) + } else if needs_github { + Some(connector_url) + } else if url == seed_url { + Some(seed_url) + } else { + None + }; + assert_eq!(failure["install_url"].as_str(), expected, "{failure}"); + assert_eq!( + failure["code"], + if needs_github { "repo.needs_github" } else { "repo.no_access" } + ); + if needs_github { + assert_eq!(failure["retry"]["args"], args); + assert_eq!(failure["retry"]["wait_action"], "cloud.repo.wait_github"); + } + assert_eq!(seed.join().unwrap().len(), if details { 1 } else { 2 }); + if let Some(agent) = agent { + assert_eq!(agent.join().unwrap().len(), 1); + } + } + } + } + } +} + +#[test] +fn cloud_repo_runtime_stale_unsafe_cache_falls_back_to_validated_connector_link() { + let fixture = Fixture::new(); + let mutation = + crate::WorkspaceMutation::new("old-link", "cloud.repo", crate::Actor::local_user()) + .unwrap(); + fixture.mux.put_frontend_projection(&mutation, "cloud-repos", "session", "catalog", 1, Some(0), + &json!({"local":[],"recent":[],"chats":{},"jobs":{},"install_url":"https://evil.example/install"})).unwrap(); + let valid = "https://github.com/apps/cmux/installations/new?state=connector"; + let agent = fixture + .server("agent.sock", vec![json!({"ok":true,"repos":[],"connect":{"install_url":valid}})]); + assert_eq!(list(&fixture.runtime(), None).unwrap()["connect"]["install_url"], valid); + assert!(store::install_url(&fixture.mux).unwrap().is_none()); + assert_eq!(agent.join().unwrap().len(), 1); +} + +#[test] +fn cloud_repo_runtime_linked_seed_failure_uses_only_safe_org_install_link() { + let mac_url = "https://github.com/apps/cmux/installations/new?state=mac"; + let seed_url = "https://github.com/apps/cmux/installations/new?state=seed"; + for cached in [false, true] { + for url in [seed_url, "https://github.com.evil.example/install"] { + let fixture = Fixture::new(); + if cached { + store::sync(&fixture.mux, &json!({"repos":[],"install_url":mac_url})).unwrap(); + } + let agent = fixture.server("agent.sock", vec![json!({"ok":true,"repos":[]})]); + let seed = fixture.server( + "seed.sock", + vec![json!({ + "ok":false,"error_code":"seed.needs_github","error_details":{"install_url":url} + })], + ); + let failure = + add(&fixture.runtime(), &json!({"session":fixture.session,"repo":"owner/tool"})) + .unwrap_err() + .0; + assert_eq!(failure["code"], "repo.no_access"); + assert_eq!(failure["org"], "owner"); + let expected = if cached { + Some(mac_url) + } else if url == seed_url { + Some(seed_url) + } else { + None + }; + assert_eq!(failure["install_url"].as_str(), expected); + assert_eq!(agent.join().unwrap().len(), 1); + assert_eq!(seed.join().unwrap().len(), 1); + } + } +} diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store.rs index 6610593d6204..e70b7c4ec47f 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store.rs @@ -142,14 +142,10 @@ pub(super) fn sync(mux: &Mux, args: &Value) -> Result { for repo in &mut recent { repo.source = "recent".into(); } - let install_url = args.get("install_url").and_then(Value::as_str).filter(|s| { - url::Url::parse(s).is_ok_and(|u| { - u.scheme() == "https" - && u.host_str() == Some("github.com") - && u.username().is_empty() - && u.password().is_none() - }) - }); + let install_url = args + .get("install_url") + .and_then(Value::as_str) + .filter(|url| super::engine::valid_install_url(url)); if args.get("install_url").is_some() && install_url.is_none() { return Err(Failure::new( "seed.bad_request", @@ -272,7 +268,12 @@ pub(super) fn set_job( } pub(super) fn install_url(mux: &Mux) -> Result, Failure> { - Ok(load(mux)?.0.get("install_url").and_then(Value::as_str).map(str::to_owned)) + Ok(load(mux)? + .0 + .get("install_url") + .and_then(Value::as_str) + .filter(|url| super::engine::valid_install_url(url)) + .map(str::to_owned)) } #[cfg(test)] diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs index 283cad98b45b..752cb29b34d1 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs @@ -112,3 +112,26 @@ fn cloud_repo_remember_rejects_invalid_attachment_without_mutating_catalog() { ); assert!(sources(&mux).unwrap().is_empty()); } + +#[test] +fn cloud_repo_install_links_are_checked_on_sync_and_when_reading_old_records() { + let mux = mux("cloud-repo-install-link-validation"); + let trusted = "https://github.com/apps/cmux/installations/new?state=mac"; + for url in crate::cloud_repos::tests::UNSAFE_INSTALL_URLS { + let failure = sync(&mux, &json!({"repos":[],"install_url":url})).unwrap_err().0; + assert_eq!(failure["code"], "seed.bad_request"); + assert!(install_url(&mux).unwrap().is_none()); + assert!(load(&mux).unwrap().0.get("install_url").is_none()); + } + sync(&mux, &json!({"repos":[],"install_url":trusted})).unwrap(); + for url in crate::cloud_repos::tests::UNSAFE_INSTALL_URLS { + assert!(sync(&mux, &json!({"repos":[],"install_url":url})).is_err()); + assert_eq!(install_url(&mux).unwrap().as_deref(), Some(trusted)); + } + // Old versions could persist links that the new writer rejects. Reading + // one must not expose it or prevent a validated connector fallback. + for url in crate::cloud_repos::tests::UNSAFE_INSTALL_URLS { + change(&mux, |doc| doc.set("install_url", json!(url))).unwrap(); + assert!(install_url(&mux).unwrap().is_none()); + } +} diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/tests.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/tests.rs index 87191908394d..f71b0dea6f4c 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/tests.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/tests.rs @@ -228,3 +228,48 @@ fn cloud_repo_remote_gate_never_exports_mac_projects_or_accepts_lookalike_comman assert!(!crate::server::FsGate.admit(&peer, &value.to_string())); } } + +// Shared hostile-link corpus exercises the same validator at every boundary. +pub(super) const UNSAFE_INSTALL_URLS: &[&str] = &[ + "", + "http://github.com/apps/cmux", + "javascript:alert(1)", + "https://evil.example/apps/cmux", + "https://github.com.evil.example/apps/cmux", + "https://github.com@evil.example/apps/cmux", + "https://user@github.com/apps/cmux", + "https://@github.com/apps/cmux", + "https://user:secret@github.com/apps/cmux", + "https://github.com:444/apps/cmux", + "https://github.com/apps/cmux\n", + "https://git\thub.com/apps/cmux", + "https://github.com/apps/\rcmux", + " https://github.com/apps/cmux", +]; + +#[test] +fn cloud_repo_install_link_validation_and_engine_outputs_share_one_policy() { + for url in + ["https://github.com/apps/cmux/installations/new", "https://github.com:443/apps/cmux"] + { + assert!(engine::valid_install_url(url)); + assert_eq!(Failure::needs_github(url).0["install_url"], url); + } + for url in UNSAFE_INSTALL_URLS { + assert!(!engine::valid_install_url(url), "{url:?}"); + let failure = Failure::needs_github(url).0; + assert_eq!(failure["code"], "seed.bad_request"); + assert!(failure.get("install_url").is_none()); + assert!(failure["message"].as_str().unwrap().contains("Mac")); + let fake = Fake { install_url: RefCell::new(Some((*url).into())), ..Default::default() }; + assert_eq!(list(&fake, None).unwrap_err().0["code"], "seed.bad_request"); + let failure = add(&fake, &args("project")).unwrap_err().0; + assert_eq!(failure["code"], "seed.bad_request"); + assert!(failure.get("install_url").is_none()); + for code in ["seed.needs_github", "repo.needs_github", "seed.no_access", "repo.no_access"] { + let failure = engine::seed_failure(&json!({"error_code":code,"install_url":url})).0; + assert!(failure.get("install_url").is_none(), "{code}: {failure}"); + assert!(!failure["message"].as_str().unwrap().is_empty()); + } + } +} diff --git a/plans/cmux-next/cloud-repo-actions.md b/plans/cmux-next/cloud-repo-actions.md index d6c5cb196a9a..d7bf36397ede 100644 --- a/plans/cmux-next/cloud-repo-actions.md +++ b/plans/cmux-next/cloud-repo-actions.md @@ -17,6 +17,10 @@ failures retain the normal `ok:false` envelope. | `cloud.repo.remember` | `{session,attachment:{state:"done",repo,folder,head}}` | `{remembered:true}` | `session` is the chat ID, a single safe path component. It is not the daemon session selector. +Agent tools verify this ID against the caller authenticated by acpmux's initialize handshake. +A presented ACP launch credential also restricts daemon actions to that same chat. Existing +local-user CLI access and authenticated Mac links remain authorized; same-user raw sockets +are not a process isolation boundary. A bare repo name is resolved in source order: local Mac remotes, previous Cloud chat repos, then all pages of the GitHub installation catalog. One match attaches; multiple matches within one source return `repo.ambiguous` with `choices` in the same repo-row shape. An explicit @@ -39,6 +43,9 @@ it with the catalog. Machine VM bearers can enumerate repositories, but cannot e agent accepts `{github_repos:{query?,cursor?}}`, fixes team/environment from its binding, and uses `GET /v1/connectors/github/repos`. A proven unlinked account returns `connect:{}` when VM install-link issuance is refused; the daemon fills the URL from the synchronized catalog. +Every installation link is checked for HTTPS, the exact `github.com` host, and absence of +credentials or a nondefault port. A valid synchronized Mac link takes precedence over +connector or seed responses. Unsafe links never become card destinations. ## Clone, recovery and persistence From 627d999774e3619d137d2f9c51b0a464a2d99b6a Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 1 Nov 2026 07:37:37 +0000 Subject: [PATCH 3/6] Fix unit return type in installation-link cache regression Agent-Session: https://cmux.com/deeplink/agent?id=aid-483ddc0ed5b690116566 --- cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs index 752cb29b34d1..db9799cff42a 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/store_tests.rs @@ -131,7 +131,10 @@ fn cloud_repo_install_links_are_checked_on_sync_and_when_reading_old_records() { // Old versions could persist links that the new writer rejects. Reading // one must not expose it or prevent a validated connector fallback. for url in crate::cloud_repos::tests::UNSAFE_INSTALL_URLS { - change(&mux, |doc| doc.set("install_url", json!(url))).unwrap(); + change(&mux, |doc| { + doc.set("install_url", json!(url)); + }) + .unwrap(); assert!(install_url(&mux).unwrap().is_none()); } } From f3ae23c6fbbccefb180fe157cb943f45163a802b Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 1 Nov 2026 07:50:42 +0000 Subject: [PATCH 4/6] Keep repository adapter clippy clean after stack rebase Agent-Session: https://cmux.com/deeplink/agent?id=aid-483ddc0ed5b690116566 --- cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs index 63dd813da181..f0bdbbea9a14 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs @@ -322,7 +322,7 @@ impl Backend for Runtime<'_> { return Err(self.seed_failure(&status, repo)?); } Some("running") if Instant::now() < deadline => { - std::thread::sleep(Duration::from_millis(250)) + std::thread::sleep(Duration::from_millis(250)); } Some("running") => { return Err(Failure::new( From d28d2c2e00aa6e547b3e1cd8ae7dfe871d3978c4 Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 1 Nov 2026 08:00:28 +0000 Subject: [PATCH 5/6] Collapse connector install link checks for clippy Agent-Session: https://cmux.com/deeplink/agent?id=aid-483ddc0ed5b690116566 --- .../cmux-tui-core/src/cloud_repos/runtime.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs index f0bdbbea9a14..4e8c89962a8c 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs @@ -98,10 +98,10 @@ impl<'a> Runtime<'a> { failure.0.set("install_url", json!(url)); } } - } else if failure.0["code"] == "repo.no_access" { - if let Some(url) = self.optional_install_url(value)? { - failure.0.set("install_url", json!(url)); - } + } else if failure.0["code"] == "repo.no_access" + && let Some(url) = self.optional_install_url(value)? + { + failure.0.set("install_url", json!(url)); } Ok(failure) } @@ -179,10 +179,10 @@ impl Backend for Runtime<'_> { "Retry repository discovery. If it still fails, reconnect GitHub from Add repository." }, ); - if code == "repo.no_access" { - if let Some(url) = self.optional_install_url(error)? { - failure.0.set("install_url", json!(url)); - } + if code == "repo.no_access" + && let Some(url) = self.optional_install_url(error)? + { + failure.0.set("install_url", json!(url)); } return Err(failure); } From 964ceb9a3e81d26c95a91db39ebd387277eaf15a Mon Sep 17 00:00:00 2001 From: Leo Li Date: Sun, 1 Nov 2026 11:52:55 +0000 Subject: [PATCH 6/6] Preserve panic-free repository reads on the current base Agent-Session: https://cmux.com/deeplink/agent?id=aid-483ddc0ed5b690116566 --- cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs | 6 ++++-- cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs | 2 +- cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs | 4 +++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs index f72a87cd5b9f..02782fab5e7b 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/engine.rs @@ -294,10 +294,12 @@ pub(crate) fn seed_failure(value: &Value) -> Failure { "repo.no_access", "Install GitHub access for this organization, or request access from its owner, then retry Add repository.", ); - if let Some(org) = value["org"].as_str() { + if let Some(org) = value.get("org").and_then(Value::as_str) { result.0.set("org", json!(org)); } - if let Some(url) = value["install_url"].as_str().filter(|url| valid_install_url(url)) { + if let Some(url) = + value.get("install_url").and_then(Value::as_str).filter(|url| valid_install_url(url)) + { result.0.set("install_url", json!(url)); } return result; diff --git a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs index 4e8c89962a8c..c178c02647e5 100644 --- a/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs +++ b/cmux-tui/crates/cmux-tui-core/src/cloud_repos/runtime.rs @@ -98,7 +98,7 @@ impl<'a> Runtime<'a> { failure.0.set("install_url", json!(url)); } } - } else if failure.0["code"] == "repo.no_access" + } else if failure.0.get("code").and_then(Value::as_str) == Some("repo.no_access") && let Some(url) = self.optional_install_url(value)? { failure.0.set("install_url", json!(url)); diff --git a/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs b/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs index 488947f658d2..5f311d685f2d 100644 --- a/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs +++ b/cmux-tui/crates/cmux-tui-core/src/server/cloud_repos.rs @@ -67,7 +67,9 @@ fn permitted(mux: &Mux, client: u64, cmd: &str, request: &Value) -> bool { Some(_) => return false, }; match mux.request_actor(super::origin_gate::connection_actor(mux, client), credential) { - Ok(crate::Actor::AcpSession { id, .. }) => request["session"].as_str() == Some(id.as_str()), + Ok(crate::Actor::AcpSession { id, .. }) => { + request.get("session").and_then(Value::as_str) == Some(id.as_str()) + } Ok(_) => true, Err(_) => false, }