diff --git a/CHANGELOG.md b/CHANGELOG.md index e02b926..6b4c37d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,13 @@ ## Unreleased +- Capability tokens are no longer minted from a silently-failed CSPRNG read. If `/dev/urandom` could not be opened or read the error was discarded and the buffer kept its zero initializer, so every token became 32 zeros — a predictable credential for reattach and for TCP agents. `mint_token` now propagates the failure, which surfaces as a session-spawn error rather than a weak token. Most likely to have bitten agents run inside a container or bubblewrap sandbox with no `/dev` bound. +- A panic no longer leaves the terminal unusable. The relay client installs a panic hook that leaves the alternate screen, disables raw mode and mouse reporting, and pops kitty flags before the panic report prints, so the message lands on the normal screen instead of a shell with no echo. `SIGTERM`/`SIGHUP` are now handled the same way as a detach rather than killing the client mid-alternate-screen. +- Fixed a crash when the command bar's slash-command popup was open on a short terminal: the popup claimed one row per match (up to 8) without checking how many rows were left above the bar, underflowing the row calculation and panicking inside ratatui's buffer indexing. The popup now yields rows to the bar, and terminals below 20x12 render a "terminal too small" placeholder instead of attempting a layout the solver can't satisfy. +- Fixed the client hanging after detach until an extra keypress, which was then swallowed. The stdin reader runs as a blocking task that can't be cancelled, and dropping the tokio runtime waits for it; the client now exits directly once the terminal is restored. +- Fixed `ctrl`/`alt` chords being typed as literal characters in the chat pane, command bar, new-session dialog, and settings editor — crossterm reports `ctrl-c` as `Char('c')` with a modifier set, so it inserted a `c`. Shifted characters are still text. Search mode already had this guard. +- Tokenless IPC clients are now rejected on platforms without `SO_PEERCRED` (macOS, BSD) instead of being granted operator capabilities. The "same-uid peer is the operator" shortcut is only sound where the kernel can attest the peer's uid; elsewhere a connection is anonymous and must present a token, as TCP already did. Relatedly, the default socket path now falls back to `TMPDIR` off Linux rather than `/run/user/`, which doesn't exist there. +- A pipe's `Summarize` relay no longer stalls indefinitely against an unresponsive endpoint; the request now carries a 60s timeout, matching the bounds the orchestrator paths already set. - Fixed Escape keypresses being swallowed when followed quickly by another key (crossterm merges them into Alt+char): the ESC prefix is now forwarded to the PTY. Most visible in vim, where Esc then `:wq` typed fast left the session in insert mode with `:wq` inserted into the buffer. - `PageUp`/`PageDown` now scroll linkshell's captured scrollback in claude/codex panes (matching the mouse wheel) instead of being sent to the TUI, which ignored them. - Fixed the whole UI shaking when a codex session flapped in and out of WAITING: the status panel's waiting-preview row now shrinks with a few seconds of hysteresis, breaking the resize→repaint→state-flap feedback loop. diff --git a/src/app.rs b/src/app.rs index 6712e96..1842543 100644 --- a/src/app.rs +++ b/src/app.rs @@ -738,6 +738,10 @@ impl App { return Err(anyhow::anyhow!("Maximum {} sessions reached", MAX_SESSIONS)); } + // Mint before touching any state: a CSPRNG failure must not leave a + // half-registered session behind (next_id bumped, pane slot claimed). + let token = crate::auth::mint_token()?; + let id = self.next_id; self.next_id += 1; @@ -785,7 +789,6 @@ impl App { // to the human and keep full operator rights (so orchestrator scripts // run inside a linkshell shell can manage pipes / create sessions); // AI agent sessions are confined to worker capabilities. - let token = crate::auth::mint_token(); self.tokens.insert(token.clone(), id); let caps = if matches!(kind, SessionKind::Shell) { crate::auth::operator_caps() @@ -2650,7 +2653,10 @@ impl App { } else { None } - } else if transport == Transport::Unix { + } else if transport == Transport::Unix && crate::ipc::PEER_UID_VERIFIED { + // Tokenless peers are trusted only because SO_PEERCRED already + // confirmed they run as us. Where that check is unavailable the + // connection is anonymous, so it falls through to the reject below. if name.as_ref().map(|n| !n.is_empty()).unwrap_or(false) { match self.spawn_headless_session(name.unwrap_or_default(), group) { Ok(id) => { @@ -2664,7 +2670,7 @@ impl App { Some((None, crate::auth::operator_caps())) } } else { - // TCP with no token → reject + // TCP with no token, or a Unix peer we cannot attribute → reject. None }; @@ -4394,7 +4400,7 @@ impl App { } pub fn chat_key(&mut self, key: crossterm::event::KeyEvent) { - use crossterm::event::KeyCode; + use crossterm::event::{KeyCode, KeyModifiers}; match key.code { KeyCode::Esc => { self.chat_selection = None; @@ -4458,7 +4464,14 @@ impl App { } KeyCode::PageUp => self.chat_scroll_up(10), KeyCode::PageDown => self.chat_scroll_down(10), - KeyCode::Char(c) => { + // Only bare/shifted characters are text. crossterm reports Ctrl+C as + // Char('c') with CONTROL set, so an unguarded arm typed a literal + // "c" into the message — likewise "u" for Ctrl+U, "w" for Ctrl+W. + // Matching the guard already used by search mode in main.rs. + KeyCode::Char(c) + if !key.modifiers.contains(KeyModifiers::CONTROL) + && !key.modifiers.contains(KeyModifiers::ALT) => + { self.chat.input.insert(self.chat.cursor, c); self.chat.cursor += c.len_utf8(); self.chat.history_pos = None; @@ -6680,6 +6693,28 @@ mod tests { assert_eq!(app.chat.history, vec!["first", "second"]); } + #[test] + fn chat_ignores_control_and_alt_chords_instead_of_typing_them() { + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + let mut app = make_app(); + for c in "hi".chars() { + chat_press(&mut app, KeyCode::Char(c)); + } + + // crossterm reports these as Char(_) with a modifier set. Inserting them + // put a literal "c"/"u"/"b" into the message. + app.chat_key(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)); + app.chat_key(KeyEvent::new(KeyCode::Char('u'), KeyModifiers::CONTROL)); + app.chat_key(KeyEvent::new(KeyCode::Char('b'), KeyModifiers::ALT)); + assert_eq!(app.chat.input, "hi"); + assert_eq!(app.chat.cursor, 2); + + // Shifted characters are still ordinary text. + app.chat_key(KeyEvent::new(KeyCode::Char('X'), KeyModifiers::SHIFT)); + assert_eq!(app.chat.input, "hiX"); + assert_eq!(app.chat.cursor, app.chat.input.len()); + } + #[test] fn chat_slash_opens_a_filtering_palette_and_tab_completes() { use crossterm::event::KeyCode; diff --git a/src/auth.rs b/src/auth.rs index de62b5f..dc63dc4 100644 --- a/src/auth.rs +++ b/src/auth.rs @@ -59,12 +59,26 @@ pub fn council_caps() -> CapSet { [Capability::SignalState].into_iter().collect() } -pub fn mint_token() -> String { +/// Mint a 128-bit capability token, hex-encoded. +/// +/// Fails closed: if the system CSPRNG is unavailable the error is propagated +/// rather than swallowed. Swallowing it (the previous `.ok()`) left `buf` at +/// its zero initializer, so every token became 32 zeros — a fully predictable +/// credential. That is reachable in practice: a container or bubblewrap +/// sandbox with no `/dev` bound, a seccomp filter, or fd exhaustion all make +/// the open/read fail while the process otherwise runs fine. +pub fn mint_token() -> std::io::Result { let mut buf = [0u8; 16]; // /dev/urandom keeps the dep surface at zero; swap for `getrandom` if preferred. use std::io::Read; - std::fs::File::open("/dev/urandom") - .and_then(|mut f| f.read_exact(&mut buf)) - .ok(); - buf.iter().map(|b| format!("{:02x}", b)).collect() + std::fs::File::open("/dev/urandom")?.read_exact(&mut buf)?; + // Defence in depth: a short read can't get here (read_exact errors), but an + // all-zero buffer would be indistinguishable from the old bug, so reject it. + if buf.iter().all(|&b| b == 0) { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "CSPRNG returned all zero bytes", + )); + } + Ok(buf.iter().map(|b| format!("{:02x}", b)).collect()) } diff --git a/src/ipc.rs b/src/ipc.rs index 4ce7ef3..c281cc3 100644 --- a/src/ipc.rs +++ b/src/ipc.rs @@ -20,11 +20,27 @@ pub enum Transport { Tcp, } +/// Whether this platform lets us verify a Unix peer's uid (SO_PEERCRED). +/// +/// The tokenless "same-uid peer is the operator" shortcut is only sound when +/// this holds. On platforms where it doesn't, a connecting process is anonymous +/// and must present a token like any TCP client — otherwise anything that can +/// reach the socket is handed `operator_caps()`, which includes `InjectInput` +/// and `CreateSession`, i.e. arbitrary command execution as the user. +pub const PEER_UID_VERIFIED: bool = cfg!(target_os = "linux"); + fn runtime_dir() -> std::path::PathBuf { let base = std::env::var_os("XDG_RUNTIME_DIR") .map(std::path::PathBuf::from) .unwrap_or_else(|| { - std::path::PathBuf::from(format!("/run/user/{}", unsafe { libc::getuid() })) + // /run/user/ is a Linux (systemd) convention. Falling back to + // it unconditionally meant the default socket path could not be + // created at all on macOS/BSD, where TMPDIR is the equivalent. + if cfg!(target_os = "linux") { + std::path::PathBuf::from(format!("/run/user/{}", unsafe { libc::getuid() })) + } else { + std::env::temp_dir() + } }); let dir = base.join("linkshell"); let _ = std::fs::create_dir_all(&dir); @@ -60,17 +76,17 @@ pub fn spawn_listener(tx: mpsc::Sender, config: Arc) { let _ = std::fs::set_permissions(&path, std::fs::Permissions::from_mode(0o600)); eprintln!("[linkshell] IPC socket: {}", path); write_last_socket(&path); + if !PEER_UID_VERIFIED { + eprintln!( + "[ipc] warning: SO_PEERCRED unavailable on this platform; \ + tokenless clients will be rejected" + ); + } while let Ok((stream, _)) = listener.accept().await { #[cfg(target_os = "linux")] if peer_uid(&stream).ok() != Some(unsafe { libc::getuid() }) { continue; } - #[cfg(not(target_os = "linux"))] - { - eprintln!( - "[ipc] warning: SO_PEERCRED unavailable on this platform, skipping uid check" - ); - } let tx = tx.clone(); let (r, w) = stream.into_split(); tokio::spawn(handle_stream( diff --git a/src/main.rs b/src/main.rs index b032199..bc94475 100644 --- a/src/main.rs +++ b/src/main.rs @@ -122,7 +122,7 @@ async fn attach_existing(requested: Option) -> anyhow::Result<()> { "[linkshell] detached — sessions keep running; run `linkshell -r {id}` to reattach" ); } - result + reattach::exit_after_detach(result) } /// Spawn a fresh detached server with a new session id, then attach the relay @@ -161,7 +161,7 @@ async fn launch_and_attach(name: Option) -> anyhow::Result<()> { "[linkshell] detached — sessions keep running; run `linkshell -r {id}` to reattach" ); } - result + reattach::exit_after_detach(result) } /// Spawn `linkshell --server` as a daemon: new session (setsid) so it has no @@ -310,7 +310,7 @@ async fn run_server() -> anyhow::Result<()> { std::env::var("LINKSHELL_SESSION_ID").unwrap_or_else(|_| reattach::new_session_id()); let session_name = std::env::var("LINKSHELL_SESSION_NAME").unwrap_or_default(); let ipc_socket_path = ipc::socket_path(&config); - let reattach_token = auth::mint_token(); + let reattach_token = auth::mint_token()?; let reattach_socket_path = reattach::reattach_socket_from_ipc(&ipc_socket_path); reattach::write_session_entry(&reattach::SessionEntry { id: session_id.clone(), @@ -958,7 +958,10 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { KeyCode::Backspace => { app.new_session_backspace(); } - KeyCode::Char(c) => { + KeyCode::Char(c) + if !key.modifiers.contains(KeyModifiers::CONTROL) + && !key.modifiers.contains(KeyModifiers::ALT) => + { app.new_session_input(c); } _ => {} @@ -1010,7 +1013,10 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { KeyCode::End => { app.command_cursor_end(); } - KeyCode::Char(c) => { + KeyCode::Char(c) + if !key.modifiers.contains(KeyModifiers::CONTROL) + && !key.modifiers.contains(KeyModifiers::ALT) => + { app.command_input_char(c); } _ => {} @@ -1214,7 +1220,11 @@ fn handle_key(app: &mut App, key: crossterm::event::KeyEvent) { } app.mode = AppMode::CommandResult; } - KeyCode::Char(c) if app.settings_state.editing => { + KeyCode::Char(c) + if app.settings_state.editing + && !key.modifiers.contains(KeyModifiers::CONTROL) + && !key.modifiers.contains(KeyModifiers::ALT) => + { let cursor = app.settings_state.edit_cursor; app.settings_state.edit_buf.insert(cursor, c); app.settings_state.edit_cursor += c.len_utf8(); diff --git a/src/pipe.rs b/src/pipe.rs index 22ae18a..656227a 100644 --- a/src/pipe.rs +++ b/src/pipe.rs @@ -1,5 +1,5 @@ use std::sync::Arc; -use std::time::Instant; +use std::time::{Duration, Instant}; use tokio::sync::mpsc; use tokio::task::JoinHandle; @@ -8,6 +8,9 @@ use crate::config::Config; use crate::events::AppEvent; use crate::session::Session; +/// Wall-clock bound on a pipe's summarize request, covering connect + response. +const SUMMARIZE_TIMEOUT: Duration = Duration::from_secs(60); + #[derive(Debug, Clone)] pub enum ExtractMode { LastBlock, @@ -146,7 +149,13 @@ async fn summarize_for_relay( max_tokens: u32, config: &Config, ) -> anyhow::Result { - let client = reqwest::Client::new(); + // reqwest has no default timeout, so an unresponsive endpoint would wedge + // this relay indefinitely. The orchestrator paths already set one + // per-request; this is a short summarize of a bounded amount of text, so it + // gets a tighter bound. + let client = reqwest::Client::builder() + .timeout(SUMMARIZE_TIMEOUT) + .build()?; let auth = crate::config::AnthropicAuth::from_env().ok_or_else(|| { anyhow::anyhow!("no Anthropic credentials (set ANTHROPIC_AUTH_TOKEN or ANTHROPIC_API_KEY)") })?; diff --git a/src/protocol.rs b/src/protocol.rs index 6638640..d217d3a 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -297,8 +297,8 @@ mod tests { #[test] fn minted_tokens_are_32_hex_chars_and_unique() { - let a = crate::auth::mint_token(); - let b = crate::auth::mint_token(); + let a = crate::auth::mint_token().unwrap(); + let b = crate::auth::mint_token().unwrap(); assert_eq!(a.len(), 32); assert!(a.chars().all(|c| c.is_ascii_hexdigit())); assert_ne!(a, b); diff --git a/src/reattach.rs b/src/reattach.rs index cd48e56..979cfa8 100644 --- a/src/reattach.rs +++ b/src/reattach.rs @@ -279,11 +279,62 @@ pub fn open_server_log() -> Option { // ── Relay client: `linkshell -r` / `linkshell --reattach` ───────────────── +/// Put the terminal back the way we found it. Idempotent and infallible by +/// design — every step is best-effort because the callers that need it most +/// (panic hook, signal handler) cannot propagate an error anywhere useful. +fn restore_terminal(kitty: bool) { + use crossterm::{event::DisableMouseCapture, execute, terminal::disable_raw_mode}; + + let _ = disable_raw_mode(); + let mut stdout = std::io::stdout(); + if kitty { + // The server pushes kitty flags on our terminal; pop them in case the + // server-side restore didn't reach us (crash, abort). + use crossterm::event::PopKeyboardEnhancementFlags; + let _ = execute!(stdout, PopKeyboardEnhancementFlags); + } + let _ = execute!( + stdout, + crossterm::terminal::LeaveAlternateScreen, + DisableMouseCapture, + crossterm::cursor::Show + ); +} + +/// Await a signal, or never resolve if the stream failed to register. Keeps the +/// `select!` arms below uniform. `Signal::recv` is cancel-safe, so rebuilding +/// this future each loop iteration loses nothing. +async fn wait_signal(sig: Option<&mut tokio::signal::unix::Signal>) { + match sig { + Some(s) => { + s.recv().await; + } + None => std::future::pending::<()>().await, + } +} + +/// Install a panic hook that restores the terminal before the panic report is +/// printed. +/// +/// Without this, a panic anywhere in the client — or in the server-side render +/// path, which surfaces here as a dropped relay — leaves the user's shell in +/// raw mode inside the alternate screen with mouse reporting on: no echo, no +/// visible prompt, and escape garbage on every keystroke, recoverable only by +/// a blind `reset`. Restoring first also means the panic message lands on the +/// normal screen where it can actually be read and reported. +fn install_terminal_panic_hook(kitty: bool) { + let previous = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + restore_terminal(kitty); + previous(info); + })); +} + pub async fn run_relay_client(id: &str) -> anyhow::Result<()> { use crossterm::{ - event::{self, DisableMouseCapture, EnableMouseCapture}, + event::{self, EnableMouseCapture}, execute, - terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen, LeaveAlternateScreen}, + terminal::{disable_raw_mode, enable_raw_mode, EnterAlternateScreen}, }; // ── Find the detached session ───────────────────────────────────────── @@ -357,24 +408,15 @@ pub async fn run_relay_client(id: &str) -> anyhow::Result<()> { // screen / mouse capture / kitty-push sequences over the relay on attach, // but we enter the alternate screen locally too so there is no flash of // shell content between connect and the server's first frame. - execute!(std::io::stdout(), EnterAlternateScreen, EnableMouseCapture)?; - - let restore = move || { - let _ = disable_raw_mode(); - let mut stdout = std::io::stdout(); - if kitty { - // The server pushes kitty flags on our terminal; pop them in case - // the server-side restore didn't reach us (crash, abort). - use crossterm::event::PopKeyboardEnhancementFlags; - let _ = execute!(stdout, PopKeyboardEnhancementFlags); - } - let _ = execute!( - stdout, - LeaveAlternateScreen, - DisableMouseCapture, - crossterm::cursor::Show - ); - }; + // Arm the hook before we take over the screen, so every path from here on + // is covered. + install_terminal_panic_hook(kitty); + if let Err(e) = execute!(std::io::stdout(), EnterAlternateScreen, EnableMouseCapture) { + // Raw mode is already on from the kitty probe; don't hand the user back + // a half-configured terminal. + restore_terminal(kitty); + return Err(e.into()); + } // ── Task A: server terminal bytes → our stdout ──────────────────────── let (done_tx, mut done_rx) = mpsc::channel::<()>(1); @@ -415,6 +457,13 @@ pub async fn run_relay_client(id: &str) -> anyhow::Result<()> { }); // ── Main relay pump ─────────────────────────────────────────────────── + // SIGTERM/SIGHUP would otherwise kill us mid-alternate-screen and leave the + // terminal wrecked; treat them as an ordinary detach so `restore` runs. + // Registration failure is not worth aborting an otherwise healthy attach, + // and bailing here with `?` would skip the restore. + use tokio::signal::unix::{signal, SignalKind}; + let mut sigterm = signal(SignalKind::terminate()).ok(); + let mut sighup = signal(SignalKind::hangup()).ok(); loop { tokio::select! { data = ev_rx.recv() => { @@ -428,13 +477,42 @@ pub async fn run_relay_client(id: &str) -> anyhow::Result<()> { } } _ = done_rx.recv() => break, + _ = wait_signal(sigterm.as_mut()) => break, + _ = wait_signal(sighup.as_mut()) => break, } } - restore(); + restore_terminal(kitty); + + // NOTE: the event-reader task is still parked in a blocking `event::read()` + // on stdin and cannot be cancelled. Dropping the tokio runtime — which is + // what returning from `main` does — waits for blocking tasks that have + // already started, so the caller must `exit_after_detach` rather than fall + // off the end of `main`, or the process hangs here until the user presses + // one more key (which the dying reader then swallows). Ok(()) } +/// Terminate the client process, bypassing the tokio runtime shutdown that +/// would otherwise block on the parked stdin reader described above. +/// +/// Nothing in the client owns unflushed state — stdout is flushed on every +/// relay write and the terminal restore executes synchronously — so this is +/// only skipping a wait we don't want. +pub fn exit_after_detach(result: anyhow::Result<()>) -> ! { + use std::io::Write; + let code = match result { + Ok(()) => 0, + Err(e) => { + eprintln!("linkshell: {:#}", e); + 1 + } + }; + let _ = std::io::stdout().flush(); + let _ = std::io::stderr().flush(); + std::process::exit(code) +} + /// Encode a crossterm Event as a compact JSON line for the relay protocol. /// Returns empty Vec for events we don't forward. fn encode_relay_event(event: &crossterm::event::Event) -> Vec { diff --git a/src/ui.rs b/src/ui.rs index fff291e..03d280f 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -2,7 +2,7 @@ use ratatui::{ layout::{Alignment, Constraint, Direction, Layout, Rect}, style::{Color, Modifier, Style}, text::{Line, Span}, - widgets::{Block, Borders, Clear, List, ListItem, Paragraph}, + widgets::{Block, Borders, Clear, List, ListItem, Paragraph, Wrap}, Frame, }; @@ -138,8 +138,23 @@ fn state_border_style(state: &SessionState, active: bool) -> Style { } } +/// Smallest terminal the main layout can be solved for: the vertical split +/// below asks for a 5-row main pane, a 3-row session bar and a status panel of +/// at least 4 rows. Under that, ratatui's solver starts handing back +/// zero-height rects and the geometry arithmetic downstream has nothing valid +/// to work from. +const MIN_ROWS: u16 = 12; +const MIN_COLS: u16 = 20; + pub fn draw(f: &mut Frame<'_>, app: &App) -> LayoutInfo { let size = f.size(); + if size.height < MIN_ROWS || size.width < MIN_COLS { + draw_too_small(f, size); + // An empty LayoutInfo is safe: every consumer either iterates these + // vectors or length-checks before indexing, so hit-testing simply finds + // nothing until the terminal is large enough to lay out again. + return LayoutInfo::default(); + } let menu_open = matches!(app.mode, AppMode::Menu { .. }); let mut menu_bar_area = Rect::default(); let mut menu_item_areas = Vec::new(); @@ -1651,8 +1666,23 @@ fn draw_chat_in(f: &mut Frame<'_>, app: &App, popup: Rect, focused: bool) -> Cha // ── Command bar ──────────────────────────────────────────────────────────── +/// How many rows the command palette popup may occupy: at most 8 entries, and +/// never more than the rows left above the one-row command bar. +/// +/// The clamp against `area_height` is the load-bearing part. Without it the +/// caller's `area.y + area.height - 1 - match_count` wrapped the u16 on any +/// terminal shorter than the match list (8 matches needed 9 rows), producing a +/// y of ~65530 and a panic inside ratatui's buffer indexing. +fn palette_popup_rows(match_len: usize, area_height: u16) -> u16 { + let rows_above_bar = area_height.saturating_sub(1); + (match_len.min(8) as u16).min(rows_above_bar) +} + fn draw_command_bar(f: &mut Frame<'_>, app: &App, area: Rect) -> Rect { - let match_count = app.palette.matches.len().min(8) as u16; + if area.height == 0 || area.width == 0 { + return Rect::default(); + } + let match_count = palette_popup_rows(app.palette.matches.len(), area.height); if match_count > 0 { let popup = Rect { x: area.x, @@ -1665,7 +1695,7 @@ fn draw_command_bar(f: &mut Frame<'_>, app: &App, area: Rect) -> Rect { .palette .matches .iter() - .take(8) + .take(match_count as usize) .enumerate() .map(|(index, entry)| { let style = if index == app.palette.selected { @@ -2164,8 +2194,37 @@ fn build_row_line( Line::from(spans) } +/// Fallback frame for terminals below the minimum layout size. Deliberately +/// uses no arithmetic on the area beyond ratatui's own clipping. +fn draw_too_small(f: &mut Frame<'_>, size: Rect) { + f.render_widget(Clear, size); + let text = vec![ + Line::from(Span::styled( + "terminal too small", + Style::default() + .fg(Color::Yellow) + .add_modifier(Modifier::BOLD), + )), + Line::from(Span::styled( + format!( + "{}x{} — need {}x{}", + size.width, size.height, MIN_COLS, MIN_ROWS + ), + Style::default().fg(Color::DarkGray), + )), + ]; + f.render_widget( + Paragraph::new(text) + .alignment(Alignment::Center) + .wrap(Wrap { trim: true }), + size, + ); +} + fn centered_rect(percent_x: u16, height: u16, r: Rect) -> Rect { - let w = r.width * percent_x / 100; + // Widen to u32 for the percentage: `r.width * percent_x` overflows u16 past + // 655 columns, which is reachable on a wide display at a small font size. + let w = ((r.width as u32 * percent_x as u32) / 100).min(r.width as u32) as u16; let x = r.x + (r.width - w) / 2; let y = r.y + (r.height.saturating_sub(height)) / 2; Rect { @@ -2180,6 +2239,46 @@ fn centered_rect(percent_x: u16, height: u16, r: Rect) -> Rect { mod tests { use super::*; + #[test] + fn palette_popup_never_claims_rows_it_does_not_have() { + // Normal case: one row per match, bar keeps its own row. + assert_eq!(palette_popup_rows(5, 40), 5); + // Capped at 8 entries regardless of match count. + assert_eq!(palette_popup_rows(200, 40), 8); + // Exactly enough room for 8 matches plus the bar. + assert_eq!(palette_popup_rows(8, 9), 8); + // One row short: the bar wins, the popup gives one up. + assert_eq!(palette_popup_rows(8, 8), 7); + // Degenerate heights must not wrap or panic. + assert_eq!(palette_popup_rows(8, 1), 0); + assert_eq!(palette_popup_rows(8, 0), 0); + } + + #[test] + fn palette_popup_y_offset_stays_in_range_at_every_height() { + // Guards the exact expression in draw_command_bar: + // y = area.y + area.height - 1 - match_count + for height in 1..=64u16 { + let count = palette_popup_rows(64, height); + assert!( + height > count, + "height {} would underflow with {} popup rows", + height, + count + ); + } + } + + #[test] + fn centered_rect_survives_terminals_wider_than_655_columns() { + // r.width * percent_x overflowed u16 above 655 columns. + let r = Rect::new(0, 0, 1200, 40); + let full = centered_rect(100, 10, r); + assert_eq!((full.x, full.width), (0, 1200)); + let half = centered_rect(50, 10, r); + assert_eq!((half.x, half.width), (300, 600)); + } + fn line_text(line: &Line<'_>) -> String { line.spans.iter().map(|s| s.content.as_ref()).collect() }