Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/server-rs/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1243,6 +1243,10 @@ impl ReadOnlyMuxStateSource {
return;
};
provider.switch_session(&name, client_tty);
// Move the sidebar highlight to the switched-to session, mirroring the
// move_focus path. Without this, Alt+digit switches the tmux session but
// the highlighted selection stays on the previously focused session.
*self.focused_session.lock().unwrap() = Some(name);
}

fn move_focus(&self, delta: i64, current_session: Option<&str>) -> Option<String> {
Expand Down
7 changes: 6 additions & 1 deletion apps/server-rs/tests/protocol_shell.rs
Original file line number Diff line number Diff line change
Expand Up @@ -551,11 +551,16 @@ async fn websocket_switch_index_switches_to_visible_session() {
.await
.expect("refresh command should send");

let _ = timeout(Duration::from_secs(1), receiver.next())
let state = timeout(Duration::from_secs(1), receiver.next())
.await
.expect("refresh state should arrive before timeout")
.expect("refresh state should arrive")
.expect("refresh state should be valid");
let state_text = state.as_text().expect("state should be text");
assert!(
state_text.contains(r#""focusedSession":"worker""#),
"switch-index should move the sidebar highlight (focusedSession) to the switched-to session; got: {state_text}"
);
assert_eq!(
*mux.switch_calls.lock().unwrap(),
vec![("worker".to_string(), None)]
Expand Down
59 changes: 54 additions & 5 deletions packages/runtime-rs/src/tmux_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -605,6 +605,8 @@ impl MuxProvider for TmuxProvider {
let panes = self.client.list_panes(PaneScope::All);
let mut window_pane_counts: HashMap<String, u32> = HashMap::new();
let mut sidebars_by_window: HashMap<String, Vec<String>> = HashMap::new();
let mut window_session: HashMap<String, String> = HashMap::new();
let mut windows_by_session: HashMap<String, HashSet<String>> = HashMap::new();
let mut seen_pane_ids = HashSet::new();

for pane in panes {
Expand All @@ -614,6 +616,13 @@ impl MuxProvider for TmuxProvider {
*window_pane_counts
.entry(pane.window_id.clone())
.or_insert(0) += 1;
window_session
.entry(pane.window_id.clone())
.or_insert_with(|| pane.session_name.clone());
windows_by_session
.entry(pane.session_name.clone())
.or_default()
.insert(pane.window_id.clone());
if pane.title == "opensessions-sidebar" {
sidebars_by_window
.entry(pane.window_id)
Expand All @@ -624,8 +633,19 @@ impl MuxProvider for TmuxProvider {

for (window_id, sidebars) in sidebars_by_window {
if window_pane_counts.get(&window_id) == Some(&1) {
for pane_id in sidebars {
self.client.kill_pane(&pane_id);
// The window holds only the sidebar. Killing it leaves the
// window with zero panes, destroying the window — and the
// session too if this is its last window. Only reclaim it when
// the session has other windows; otherwise keep the sidebar so
// closing the last work pane doesn't close the whole session.
let session_has_other_windows = window_session
.get(&window_id)
.and_then(|session| windows_by_session.get(session))
.is_some_and(|windows| windows.len() > 1);
if session_has_other_windows {
for pane_id in sidebars {
self.client.kill_pane(&pane_id);
}
}
continue;
}
Expand All @@ -652,10 +672,27 @@ impl MuxProvider for TmuxProvider {
// pane works even when the parent pane's cwd is unrelated to the
// workspace (e.g. tmux sessions whose default cwd is `$HOME`). Falls
// back to the literal path if the env is unset.
let command = format!(
"OPENSESSIONS_SESSION_NAME={} OPENSESSIONS_WINDOW_ID={window_id} REFOCUS_WINDOW={window_id} exec \"${{OPENSESSIONS_DIR:-.}}\"/{scripts_dir}/start.sh",
target.session_name,
//
// Wrap in `sh -c '...'`: tmux runs pane commands via the user's
// `default-command`/`default-shell`, which may be a non-POSIX shell
// (e.g. fish) that cannot parse `FOO=bar exec` or `${VAR:-default}`.
// Forcing `sh` keeps the launcher portable regardless of the user's
// interactive shell.
//
// Quoting is two-layered: the session name / window id sit inside
// double quotes (so `$`, backtick, `"` and `\` are still live to the
// inner `sh`), so escape those metacharacters first; then the whole
// `inner` is wrapped in single quotes for `sh -c`, with embedded single
// quotes escaped via the standard `'\''` dance. Without the inner
// escape a session name like `x"; rm -rf ~ #` would break out of the
// double quotes and inject commands.
let inner = format!(
"OPENSESSIONS_SESSION_NAME=\"{}\" OPENSESSIONS_WINDOW_ID=\"{}\" REFOCUS_WINDOW=\"{}\" exec \"${{OPENSESSIONS_DIR:-.}}\"/{scripts_dir}/start.sh",
sh_double_quote_escape(&target.session_name),
sh_double_quote_escape(window_id),
sh_double_quote_escape(window_id),
);
let command = format!("sh -c '{}'", inner.replace('\'', r"'\''"));
let new_pane = self.client.split_sidebar_pane(
&target.id,
position == SidebarPosition::Left,
Expand All @@ -672,6 +709,18 @@ impl MuxProvider for TmuxProvider {
}
}

/// Escape the characters that stay special inside a POSIX double-quoted
/// string (`\`, `"`, `$`, backtick) so an untrusted value (e.g. a tmux session
/// name) can be interpolated into `FOO="..."` without breaking out of the
/// quotes or triggering command/parameter substitution. Backslash is escaped
/// first so the backslashes added for the others are not doubled.
fn sh_double_quote_escape(s: &str) -> String {
s.replace('\\', "\\\\")
.replace('"', "\\\"")
.replace('$', "\\$")
.replace('`', "\\`")
}

fn session_format() -> &'static str {
"#{session_id}\t#{session_name}\t#{session_created}\t#{session_attached}\t#{session_windows}\t#{session_path}"
}
Expand Down
102 changes: 100 additions & 2 deletions packages/runtime-rs/tests/tmux_provider.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ fn tmux_provider_resolves_focuses_and_kills_agent_panes() {
fn tmux_provider_kills_orphaned_and_duplicate_sidebar_panes() {
let runner = Arc::new(RecordingRunner::new(HashMap::from([(
"list-panes".to_string(),
"%1\talpha\t@1\t0\t0\t1\t/dev/ttys1\t123\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%1\talpha-2\t@1\t0\t0\t1\t/dev/ttys1\t123\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%2\tbeta\t@2\t0\t0\t1\t/dev/ttys2\t124\t/repo\tbash\tmain\t94\t24\t26\t119\n%3\tbeta\t@2\t0\t1\t0\t/dev/ttys3\t125\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%4\tbeta\t@2\t0\t2\t0\t/dev/ttys4\t126\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%5\t_os_stash\t@3\t0\t0\t1\t/dev/ttys5\t127\t/tmp\tzsh\topensessions-sidebar\t26\t24\t0\t25"
"%1\talpha\t@1\t0\t0\t1\t/dev/ttys1\t123\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%1\talpha-2\t@1\t0\t0\t1\t/dev/ttys1\t123\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%2\tbeta\t@2\t0\t0\t1\t/dev/ttys2\t124\t/repo\tbash\tmain\t94\t24\t26\t119\n%3\tbeta\t@2\t0\t1\t0\t/dev/ttys3\t125\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%4\tbeta\t@2\t0\t2\t0\t/dev/ttys4\t126\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%5\t_os_stash\t@3\t0\t0\t1\t/dev/ttys5\t127\t/tmp\tzsh\topensessions-sidebar\t26\t24\t0\t25\n%6\talpha\t@4\t0\t0\t1\t/dev/ttys6\t128\t/repo\tbash\tmain\t94\t24\t26\t119"
.to_string(),
)])));
let provider = TmuxProvider::new(runner.clone());
Expand Down Expand Up @@ -280,6 +280,30 @@ fn tmux_provider_kills_orphaned_and_duplicate_sidebar_panes() {
);
}

#[test]
fn tmux_provider_keeps_lone_sidebar_when_it_is_the_sessions_only_window() {
// A window containing only the sidebar, in a session that has no other
// window, must NOT be killed: doing so would leave the window with zero
// panes, destroying the session (closing the last work pane should not
// close the whole session).
let runner = Arc::new(RecordingRunner::new(HashMap::from([(
"list-panes".to_string(),
"%9\tsolo\t@9\t0\t0\t1\t/dev/ttys9\t130\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25"
.to_string(),
)])));
let provider = TmuxProvider::new(runner.clone());

provider.kill_orphaned_sidebar_panes();

let calls = runner.calls.lock().unwrap().clone();
assert!(
!calls
.iter()
.any(|call| call == &vec!["kill-pane", "-t", "%9"]),
"a lone sidebar that is the session's only window must be kept (killing it would destroy the session)"
);
}

#[test]
fn tmux_provider_spawns_sidebar_against_edge_pane_and_titles_it() {
let runner = Arc::new(RecordingRunner::new(HashMap::from([
Expand Down Expand Up @@ -316,7 +340,7 @@ fn tmux_provider_spawns_sidebar_against_edge_pane_and_titles_it() {
assert_eq!(
split_call.last().map(String::as_str),
Some(
"OPENSESSIONS_SESSION_NAME=alpha OPENSESSIONS_WINDOW_ID=@1 REFOCUS_WINDOW=@1 exec \"${OPENSESSIONS_DIR:-.}\"//scripts/start.sh"
"sh -c 'OPENSESSIONS_SESSION_NAME=\"alpha\" OPENSESSIONS_WINDOW_ID=\"@1\" REFOCUS_WINDOW=\"@1\" exec \"${OPENSESSIONS_DIR:-.}\"//scripts/start.sh'"
)
);
assert!(
Expand All @@ -326,6 +350,80 @@ fn tmux_provider_spawns_sidebar_against_edge_pane_and_titles_it() {
);
}

#[test]
fn tmux_provider_escapes_shell_metacharacters_in_session_name() {
// spawn_sidebar takes the session name from the target pane's listing, so
// the untrusted value lives in the list-panes mock (field 2). A name
// carrying double quotes, `$` and a backtick must not break out of the
// `OPENSESSIONS_SESSION_NAME="..."` double quotes and inject commands.
let evil = r#"x"; touch /tmp/pwned; $(id)`whoami`"#;
let runner = Arc::new(RecordingRunner::new(HashMap::from([
(
"list-panes".to_string(),
format!("%1\t{evil}\t@1\t0\t0\t1\t/dev/ttys1\t123\t/repo\tbash\tmain\t80\t24\t0\t79"),
),
(
"split-window".to_string(),
format!("%9\t{evil}\t@1\t0\t1\t0\t/dev/ttys9\t999\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25"),
),
])));
let provider = TmuxProvider::new(runner.clone());

provider.spawn_sidebar("ignored", "@1", 26, SidebarPosition::Left, "/scripts");

let calls = runner.calls.lock().unwrap().clone();
let split_call = calls
.iter()
.find(|call| call.first().map(String::as_str) == Some("split-window"))
.expect("split-window should be called");
let command = split_call.last().map(String::as_str).unwrap();

// Every metacharacter is backslash-escaped inside the double quotes.
assert!(
command.contains(r#"OPENSESSIONS_SESSION_NAME="x\"; touch /tmp/pwned; \$(id)\`whoami\`""#),
"session name not safely escaped: {command}"
);
// The raw, unescaped breakout sequence must never appear.
assert!(
!command.contains(r#"="x"; touch"#),
"unescaped double quote broke out of the assignment: {command}"
);
}

#[test]
fn tmux_provider_escapes_single_quotes_in_session_name() {
// Single quotes are inert inside the inner double quotes but must survive
// the outer `sh -c '...'` wrap via the `'\''` dance.
let runner = Arc::new(RecordingRunner::new(HashMap::from([
(
"list-panes".to_string(),
"%1\to'brien\t@1\t0\t0\t1\t/dev/ttys1\t123\t/repo\tbash\tmain\t80\t24\t0\t79"
.to_string(),
),
(
"split-window".to_string(),
"%9\to'brien\t@1\t0\t1\t0\t/dev/ttys9\t999\t/repo\tzsh\topensessions-sidebar\t26\t24\t0\t25"
.to_string(),
),
])));
let provider = TmuxProvider::new(runner.clone());

provider.spawn_sidebar("ignored", "@1", 26, SidebarPosition::Left, "/scripts");

let calls = runner.calls.lock().unwrap().clone();
let command = calls
.iter()
.find(|call| call.first().map(String::as_str) == Some("split-window"))
.and_then(|call| call.last())
.cloned()
.expect("split-window should be called");

assert_eq!(
command,
r#"sh -c 'OPENSESSIONS_SESSION_NAME="o'\''brien" OPENSESSIONS_WINDOW_ID="@1" REFOCUS_WINDOW="@1" exec "${OPENSESSIONS_DIR:-.}"//scripts/start.sh'"#
);
}

#[test]
fn std_command_runner_executes_tmux_binary_and_captures_output() {
let runner = StdCommandRunner::new("printf");
Expand Down