Repository navigation
cmux-tui: reject unsafe session names before socket resolution #10239
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
5c68a63
03c47e6
83ad35b
5ad19db
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -918,6 +918,7 @@ const fn decimal_width(mut value: u16) -> usize { | |
| #[derive(Debug, PartialEq, Eq)] | ||
| pub(crate) struct StartupMessages { | ||
| schema_too_new: &'static str, | ||
| pub invalid_session: &'static str, | ||
| pub session_socket: &'static str, | ||
| pub stop_newer_server: &'static str, | ||
| pub no_server_listening: &'static str, | ||
|
|
@@ -1032,6 +1033,7 @@ static ENGLISH: Catalog = Catalog { | |
| japanese: false, | ||
| startup: StartupMessages { | ||
| schema_too_new: "cannot open session \"{session}\" with cmux {version}: its saved state is incompatible with this build", | ||
| invalid_session: "the session name is invalid; use a non-empty name without path separators or control characters", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Describe the complete session-name contract. The validator also rejects non-ASCII names, leading Update all four messages to describe Also applies to: 1195-1195, 1627-1627, 1786-1786 🤖 Prompt for AI Agents |
||
| session_socket: "session socket", | ||
| stop_newer_server: "a newer cmux server owns this saved session; stop it before retrying:", | ||
| no_server_listening: "no server is listening on this socket", | ||
|
|
@@ -1190,7 +1192,7 @@ edits shell files. Authenticate with the configured host before retrying. | |
| migration_failed: "Could not reconnect the machine; please try again", | ||
| pairing_code_unavailable: "Pairing code could not be displayed securely. Run this command from an interactive terminal and retry", | ||
| runtime_failed: "The machine agent could not start or continue; check its configuration", | ||
| invalid_session: "The session name is invalid; use a short name without spaces or control characters", | ||
| invalid_session: "The session name is invalid; use an ASCII name up to 128 bytes with letters, digits, dots, hyphens, underscores, or colons", | ||
| identity_unavailable: "The private machine identity is unavailable; check that --state points to a private writable file", | ||
| registration_already_running: "A machine agent is already sharing this session; stop it before starting another", | ||
| cloud_configuration_invalid: "The cloud connection settings are invalid; check the host, user, port, and identity file", | ||
|
|
@@ -1622,6 +1624,7 @@ static JAPANESE: Catalog = Catalog { | |
| japanese: true, | ||
| startup: StartupMessages { | ||
| schema_too_new: "cmux {version} ではセッション \"{session}\" を開けません。保存状態はこのビルドと互換性がありません", | ||
| invalid_session: "セッション名が無効です。英数字、ドット、ハイフン、アンダースコア、コロンを使った 128 バイト以内の ASCII 名を使用してください", | ||
| session_socket: "セッションソケット", | ||
| stop_newer_server: "新しい cmux サーバーがこの保存済みセッションを所有しています。再試行する前に停止:", | ||
| no_server_listening: "このソケットを待ち受けているサーバーはありません", | ||
|
|
@@ -1780,7 +1783,7 @@ cmux machine-agent - ローカルの cmux セッションをリモートサー | |
| migration_failed: "マシンを再接続できませんでした。もう一度お試しください", | ||
| pairing_code_unavailable: "ペアリングコードを安全に表示できませんでした。対話型端末でこのコマンドを実行して再試行してください", | ||
| runtime_failed: "machine-agent を開始または続行できませんでした。設定を確認してください", | ||
| invalid_session: "セッション名が無効です。空白や制御文字を含まない短い名前を使用してください", | ||
| invalid_session: "セッション名が無効です。パス区切り文字や制御文字を含まない空でない名前を使用してください", | ||
| identity_unavailable: "非公開のマシン ID を使用できません。--state が非公開で書き込み可能なファイルを指していることを確認してください", | ||
| registration_already_running: "このセッションは別の machine-agent が共有中です。停止してからもう一度開始してください", | ||
| cloud_configuration_invalid: "クラウド接続設定が無効です。ホスト、ユーザー、ポート、ID ファイルを確認してください", | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -98,8 +98,13 @@ fn run_agent(args: Args) -> anyhow::Result<()> { | |
| StderrReporter::new().map_err(|_| anyhow::Error::msg(messages.runtime_failed))?; | ||
| let session = SessionName::new(args.session.clone()) | ||
| .map_err(|_| anyhow::Error::msg(messages.invalid_session))?; | ||
| let socket = | ||
| args.socket.unwrap_or_else(|| cmux_tui_core::server::default_socket_path(&args.session)); | ||
| cmux_tui_core::server::validate_session_name(&args.session) | ||
| .map_err(|_| anyhow::Error::msg(messages.invalid_session))?; | ||
| let socket = match args.socket { | ||
| Some(path) => path, | ||
| None => cmux_tui_core::server::default_socket_path(&args.session) | ||
| .map_err(|_| anyhow::Error::msg(messages.invalid_session))?, | ||
| }; | ||
|
Comment on lines
99
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Inspect SessionName::new's validation rule to compare it with validate_session_name.
set -euo pipefail
rg -n -A 15 'impl SessionName' cmux-tui || true
rg -n -A 15 'fn new' cmux-tui/crates/cmux-tui/src/machine_agent 2>/dev/null || trueRepository: manaflow-ai/cmux Length of output: 16468 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- AGENTS.md files ---'
find cmux-tui -name AGENTS.md -print
echo '--- SessionName and shared validation ---'
rg -n -A 12 -B 8 'SessionName|fn valid_identifier|valid_identifier|validate_session_name' \
cmux-tui/crates/cmux-tui-machine-agent-protocol/src/lib.rs \
cmux-tui/crates/cmux-tui/src \
cmux-tui/AGENTS.md 2>/dev/null
echo '--- relevant source sections ---'
sed -n '1,90p' cmux-tui/crates/cmux-tui-machine-agent-protocol/src/lib.rs
sed -n '330,375p' cmux-tui/crates/cmux-tui-machine-agent-protocol/src/lib.rs
rg -n -A 20 -B 8 'pub fn validate_session_name|fn validate_session_name' cmux-tuiRepository: manaflow-ai/cmux Length of output: 50372 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- cmux-tui/AGENTS.md ---'
cat cmux-tui/AGENTS.md
echo '--- validate_session_name definition and constants ---'
rg -n -A 25 -B 10 'validate_session_name|MAX_SESSION|session name' cmux-tui/crates \
-g '*.rs' | head -240
echo '--- localization message ---'
rg -n -A 4 -B 4 'invalid_session' cmux-tui/crates/cmux-tui/src/localization.rsRepository: manaflow-ai/cmux Length of output: 25000 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '--- repository-wide validator definition ---'
rg -n -A 30 -B 10 'fn validate_session_name|validate_session_name\s*\(' . \
-g '*.rs' -g '!target' | head -300
echo '--- core crate locations ---'
git ls-files | rg '(^|/)(server|core|session|validate).*\.rs$|cmux.*core.*\.rs$' | head -200Repository: manaflow-ai/cmux Length of output: 28018 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
def session_name_new(value: str) -> bool:
data = value.encode()
return (
bool(value)
and len(data) <= 128
and all(
byte.isascii() and (byte.isalnum() or byte in "-_.:")
for byte in value
)
)
def validate_session_name(value: str) -> bool:
data = value.encode()
return (
bool(data)
and len(data) <= 64
and data[0] < 128
and chr(data[0]).isalnum()
and all(
byte < 128
and (chr(byte).isalnum() or byte in (ord("."), ord("_"), ord("-")))
for byte in data[1:]
)
)
cases = ["a", "1", "a:b", ":a", "_a", ".a", "-a", "a" * 64, "a" * 65, "a" * 128, "a b", "é"]
for value in cases:
print(f"{value!r}: SessionName::new={session_name_new(value)}, "
f"validate_session_name={validate_session_name(value)}")
# Check the relation over a finite representative corpus.
alphabet = "a1:._- `@é`"
corpus = [""]
for length in range(1, 5):
if length <= 2:
import itertools
corpus.extend("".join(chars) for chars in itertools.product(alphabet, repeat=length))
else:
corpus.extend(["a" * length, ":" * length, "é" * length])
narrowed = [value for value in corpus if session_name_new(value) and not validate_session_name(value)]
widened = [value for value in corpus if validate_session_name(value) and not session_name_new(value)]
print(f"representative SessionName-only values: {len(narrowed)}")
print(f"examples: {narrowed[:12]}")
print(f"validate_session_name-only values: {len(widened)}")
PYRepository: manaflow-ai/cmux Length of output: 1246 Align the session-name validators and update the error message. 🤖 Prompt for AI Agents |
||
| let state = args | ||
| .state | ||
| .map_or_else(default_state_path, Ok) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -762,6 +762,8 @@ fn parse_args_result(args: impl IntoIterator<Item = String>) -> Result<Args, Str | |
| if out.agent_browser_provider { | ||
| return Err(format!("--agent-browser-provider is unsupported on {}", std::env::consts::OS)); | ||
| } | ||
| cmux_tui_core::server::validate_session_name(&out.session) | ||
| .map_err(|_| localization::catalog().startup.invalid_session.to_string())?; | ||
| Ok(out) | ||
| } | ||
|
|
||
|
|
@@ -1503,8 +1505,10 @@ fn run_terminal_host_process(args: &[String]) -> anyhow::Result<()> { | |
| } | ||
|
|
||
| fn run_attach(args: Args) -> anyhow::Result<()> { | ||
| let socket_path = | ||
| args.socket.unwrap_or_else(|| cmux_tui_core::server::default_socket_path(&args.session)); | ||
| let socket_path = match args.socket { | ||
| Some(path) => path, | ||
| None => cmux_tui_core::server::default_socket_path(&args.session)?, | ||
| }; | ||
| let config = config::load(); | ||
| let messages = &localization::catalog().attach; | ||
| let terminal = args | ||
|
|
@@ -1594,8 +1598,10 @@ fn run_relay(args: Args) -> anyhow::Result<()> { | |
| if args.provider_cli_requested() { | ||
| anyhow::bail!("relay cannot also select a machine provider"); | ||
| } | ||
| let socket_path = | ||
| args.socket.unwrap_or_else(|| cmux_tui_core::server::default_socket_path(&args.session)); | ||
| let socket_path = match args.socket { | ||
| Some(path) => path, | ||
| None => cmux_tui_core::server::default_socket_path(&args.session)?, | ||
| }; | ||
| let stream = cmux_tui_core::platform::transport::connect(&socket_path).map_err(|error| { | ||
| anyhow::anyhow!("cannot connect relay to session socket {}: {error}", socket_path.display()) | ||
| })?; | ||
|
|
@@ -1776,10 +1782,10 @@ fn run_server( | |
| let ws_token = args.ws_token.clone().or(config.server.ws_token.clone()); | ||
| // Compute the socket path up front so a normal interactive launch can | ||
| // reuse an existing local session and surface children inherit it. | ||
| let socket_path = args | ||
| .socket | ||
| .clone() | ||
| .unwrap_or_else(|| cmux_tui_core::server::default_socket_path(&args.session)); | ||
| let socket_path = match args.socket.clone() { | ||
| Some(path) => path, | ||
| None => cmux_tui_core::server::default_socket_path(&args.session)?, | ||
| }; | ||
| if args.should_attach_existing(&ws_addr, &ws_token) | ||
| && socket_path.exists() | ||
| && let Ok(remote) = RemoteSession::connect(&socket_path) | ||
|
|
@@ -2574,6 +2580,32 @@ mod remote_args_tests { | |
| assert_eq!(args.remote_http.as_deref(), Some("127.0.0.1:8765")); | ||
| } | ||
|
|
||
| #[test] | ||
| fn session_names_reject_path_components_and_control_characters() { | ||
| for session in [ | ||
| "", | ||
| ".", | ||
| "..", | ||
| "../escape", | ||
| "nested/session", | ||
| "nested\\session", | ||
| "bad\0name", | ||
| "bad\nname", | ||
| ] { | ||
| let arguments = ["--session", session].map(str::to_string); | ||
| let error = parse_args_result(arguments) | ||
| .expect_err("unsafe session name was accepted before socket resolution"); | ||
| assert!(error.contains("session"), "unexpected error for {session:?}: {error}"); | ||
| } | ||
|
|
||
| for session in ["legacy name", "名前", "_legacy", "-legacy", &"x".repeat(200)] { | ||
| let arguments = ["--session", session].map(str::to_string); | ||
| parse_args_result(arguments).unwrap_or_else(|error| { | ||
| panic!("legacy-safe session name {session:?} was rejected: {error}") | ||
| }); | ||
| } | ||
|
Comment on lines
+2601
to
+2606
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win Make the accepted-session cases match the validation contract.
Keep these values in the rejection cases. Add valid examples such as 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| #[test] | ||
| fn malformed_relay_endpoint_errors_do_not_echo_credentials() { | ||
| let error = relay_daemon_options( | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.