From fed51035b51e3dce0e3226095b1062facad3d271 Mon Sep 17 00:00:00 2001 From: Utkarsh Date: Tue, 13 Oct 2026 13:54:17 +0530 Subject: [PATCH] Fix code scanning alerts #1, #12 and #278 - serve: stop printing a prefix of the agent secret in the startup banner (#12) - ptyctl: bound scrollback pre-allocation by a constant; output unchanged (#1) - pager: drop config_key interpolation from a debug_assert message (#278) --- crates/codegen/ainxt-pager-bin/src/main.rs | 35 ++++---- .../ainxt-pager/src/app/effects/mod.rs | 3 +- crates/codegen/ptyctl/src/term.rs | 81 ++++++++++++++++++- 3 files changed, 97 insertions(+), 22 deletions(-) diff --git a/crates/codegen/ainxt-pager-bin/src/main.rs b/crates/codegen/ainxt-pager-bin/src/main.rs index 037f8b1..b70e3e6 100644 --- a/crates/codegen/ainxt-pager-bin/src/main.rs +++ b/crates/codegen/ainxt-pager-bin/src/main.rs @@ -87,33 +87,28 @@ fn resolve_agent_profile_path(path: &std::path::Path) -> std::path::PathBuf { } /// Print startup information for the serve command. /// -/// The secret itself is never printed to the terminal (which can be +/// No part of the secret is ever printed to the terminal (which can be /// captured by session recorders, terminal multiplexer scrollback, or -/// screen-sharing); only a short prefix is shown so an operator who already -/// knows the secret can sanity-check they're looking at the right value. -fn print_serve_startup_info(bind_addr: SocketAddr, secret: &str) { - let masked = mask_secret_for_display(secret); +/// screen-sharing). Even a short prefix materially reduces the search space +/// of an auto-generated secret, so only its *source* is reported. The +/// function deliberately does not take the secret as a parameter so that no +/// secret-derived data can reach this output: it only receives whether the +/// user supplied the value (`user_supplied`) and prints a fixed label. +fn print_serve_startup_info(bind_addr: SocketAddr, user_supplied: bool) { + let origin_label = if user_supplied { + "provided via --secret / AINXT_AGENT_SECRET (not shown)" + } else { + "auto-generated (not shown; pass --secret / AINXT_AGENT_SECRET to choose one)" + }; eprintln!(); eprintln!(" ainxt agent server starting..."); eprintln!(); eprintln!(" Address: {}:{}", bind_addr.ip(), bind_addr.port()); - eprintln!(" Secret: {masked} (see --secret / AINXT_AGENT_SECRET to recover the full value)"); + eprintln!(" Secret: {origin_label}"); eprintln!(); eprintln!(" WebSocket URL: ws://{bind_addr}/ws?server-key="); eprintln!(); } - -/// Render a secret as a short, non-reversible display form: the first 4 -/// characters followed by an ellipsis, or fully masked when too short for a -/// partial prefix to be safe. -fn mask_secret_for_display(secret: &str) -> String { - if secret.chars().count() <= 4 { - "****".to_string() - } else { - let prefix: String = secret.chars().take(4).collect(); - format!("{prefix}…") - } -} /// Entrypoint tag for `ainxt -p`; keys the quiet stderr default in `init_tracing_simple`. const HEADLESS_ENTRYPOINT: &str = "headless"; /// Initialize simple tracing for non-TUI agent modes. @@ -1338,9 +1333,9 @@ async fn run_agent_command( let secret = a.get_secret(); let server_config = ainxt_shell::agent::ServerConfig { bind_addr: a.bind, - secret: secret.clone(), + secret, }; - print_serve_startup_info(a.bind, &secret); + print_serve_startup_info(a.bind, a.secret.is_some()); ainxt_shell::agent::run_agent_server(server_config, agent_config).await } Some(AgentCmd::Leader(a)) => { diff --git a/crates/codegen/ainxt-pager/src/app/effects/mod.rs b/crates/codegen/ainxt-pager/src/app/effects/mod.rs index 264bd55..8d887a4 100644 --- a/crates/codegen/ainxt-pager/src/app/effects/mod.rs +++ b/crates/codegen/ainxt-pager/src/app/effects/mod.rs @@ -1957,7 +1957,8 @@ pub(crate) fn execute( debug_assert!( config_key == "fork_worktree_mode" || config_key == "new_session_worktree_mode", - "unexpected worktree config_key: {config_key}" + "unexpected worktree config_key (expected fork_worktree_mode or \ + new_session_worktree_mode)" ); persist_hint(tasks, config_key, mode.as_config_str(), "worktree mode"); } diff --git a/crates/codegen/ptyctl/src/term.rs b/crates/codegen/ptyctl/src/term.rs index a2ad038..8867dc3 100644 --- a/crates/codegen/ptyctl/src/term.rs +++ b/crates/codegen/ptyctl/src/term.rs @@ -139,6 +139,12 @@ impl Dimensions for TermDimensions { } } +/// Upper bound on the number of scrollback entries reserved up front by +/// [`Terminal::scrollback_lines`]. Matches alacritty's default +/// `scrolling_history` (10,000), which is the history limit this terminal is +/// created with. +const MAX_SCROLLBACK_PREALLOC: usize = 10_000; + /// Headless terminal emulator wrapping `alacritty_terminal`. pub struct Terminal { term: Term, @@ -217,7 +223,17 @@ impl Terminal { let n = count.min(history); let num_cols = grid.columns(); - let mut lines = Vec::with_capacity(n); + // `count` arrives from an HTTP query parameter. `n` is already bounded + // by the history size, but bound the up-front reservation by a + // constant as well so the allocation size never depends on + // caller-controlled input. This only limits pre-allocation: all `n` + // lines are still returned (the Vec grows if ever needed). + let prealloc = if n > MAX_SCROLLBACK_PREALLOC { + MAX_SCROLLBACK_PREALLOC + } else { + n + }; + let mut lines = Vec::with_capacity(prealloc); // history lines are at negative indices: Line(-1) is most recent for offset in (1..=n).rev() { let row = &grid[Line(-(offset as i32))]; @@ -356,3 +372,66 @@ fn resolve_range(range: &Option>, max: usize) -> (usize, usize) { None => (0, max), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn terminal(cols: u16, rows: u16) -> Terminal { + let (tx, _rx) = tokio::sync::mpsc::unbounded_channel(); + Terminal::new(cols, rows, SessionListener::new(tx)) + } + + fn feed_lines(term: &mut Terminal, count: usize) { + for i in 0..count { + term.feed(format!("line {i}\r\n").as_bytes()); + } + } + + #[test] + fn scrollback_lines_returns_requested_tail_in_order() { + let mut term = terminal(40, 5); + feed_lines(&mut term, 30); + let history = term.scrollback_count(); + assert!(history > 3); + + let lines = term.scrollback_lines(3); + assert_eq!(lines.len(), 3); + // Oldest first, offsets counting down to 1 (most recent). + assert_eq!( + lines.iter().map(|l| l.offset).collect::>(), + vec![3, 2, 1] + ); + } + + #[test] + fn scrollback_lines_huge_count_returns_full_history() { + let mut term = terminal(40, 5); + feed_lines(&mut term, 30); + let history = term.scrollback_count(); + + // A caller-supplied count far beyond the history (or the prealloc + // bound) must still return exactly the full history, oldest first. + let lines = term.scrollback_lines(usize::MAX); + assert_eq!(lines.len(), history); + assert_eq!(lines.first().map(|l| l.text.as_str()), Some("line 0")); + assert_eq!(lines.last().map(|l| l.offset), Some(1)); + // Requesting exactly `history` yields the identical result. + assert_eq!(texts(&term.scrollback_lines(history)), texts(&lines)); + } + + #[test] + fn scrollback_lines_beyond_prealloc_bound_returns_all() { + // History is capped at alacritty's default (10,000). Fill past it and + // check every history line is still returned despite the prealloc cap. + let mut term = terminal(20, 2); + feed_lines(&mut term, MAX_SCROLLBACK_PREALLOC + 500); + let history = term.scrollback_count(); + assert_eq!(history, MAX_SCROLLBACK_PREALLOC); + assert_eq!(term.scrollback_lines(usize::MAX).len(), history); + } + + fn texts(lines: &[ScrollbackLine]) -> Vec<(usize, String)> { + lines.iter().map(|l| (l.offset, l.text.clone())).collect() + } +}