Skip to content

feat(ssh): nested SSH detection, host dossier system, and host_note AI tool#157

Merged
jexShain merged 6 commits into
AI-Shell-Team:rustfrom
jexShain:feat/nested-ssh-detection
May 12, 2026
Merged

feat(ssh): nested SSH detection, host dossier system, and host_note AI tool#157
jexShain merged 6 commits into
AI-Shell-Team:rustfrom
jexShain:feat/nested-ssh-detection

Conversation

@jexShain
Copy link
Copy Markdown
Collaborator

@jexShain jexShain commented May 11, 2026

Summary

  • Nested SSH detection: Automatically detects SSH commands in PTY output (handles history, paste, reverse-i-search), probes new hosts, and creates YAML profiles
  • Host dossier system: Per-host profile storage with auto-detected system info (OS, kernel, shell, user, locale) and user-authored notes that persist across sessions
  • host_note AI tool: Lets the AI save/list/forget per-host notes during SSH sessions via Arc<Mutex<Option<String>>> shared host state
  • Dossier commands: ;remember, ;notes, ;forget, ;refresh handled directly without AI dependency, with Chinese command support (no space required after 记住/忘记)

Key technical decisions

  • Output-based SSH scanning (not keystroke) for reliability with history/paste/reverse-i-search
  • Only scans complete PTY output lines to avoid false positives during typing
  • strip_ansi_escapes preserves UTF-8 multi-byte chars and handles BS/CSI/OSC sequences
  • Forward-parsing extract_remote_host_from_cmd skips SSH options with arguments
  • Printf-constructed probe markers avoid echo contamination
  • Package manager inferred from /etc/os-release ID instead of slow which check
  • Host stack with MAX_NESTING=8 limit, disconnect detection with host verification
  • Line-boundary truncation for output_ssh_scan buffer

Files changed

  • crates/aish-hosts/ (new crate): profile storage, probe, system info parsing
  • crates/aish-pty/src/persistent.rs: nested SSH detection, probe injection, dossier commands, shared_host integration
  • crates/aish-shell/src/app.rs: shared_host wiring, host_note tool registration, dynamic dossier loading
  • crates/aish-tools/src/host_note.rs (new): HostNoteTool implementation
  • crates/aish-i18n/locales/: i18n strings for host_note tool and probing status

Test plan

  • cargo test -p aish-hosts -p aish-pty — 113 tests pass
  • Connect to host A, verify probe creates YAML profile
  • SSH from A to B, verify nested detection creates B's profile
  • ;记住... on nested host saves to correct profile
  • Disconnect from B restores to A's host tracking
  • strip_ansi_escapes handles BS, ANSI sequences, UTF-8 correctly

Summary by CodeRabbit

  • New Features
    • Per-host note management in SSH sessions (store, list, forget) and a host-note tool available to follow-ups
    • Automatic remote environment detection with cached system info injected into AI prompts
    • Secure preflight for shell commands with optional confirmation dialog
  • Behavioral Improvements
    • Smarter non-blocking session probing, timeouts, and probe capture
    • Improved AI-cancellation, follow-up handling, and consistent remote-context across multi-round sessions
  • Localization
    • Added English and Chinese messages for session states and host-note interactions

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 11, 2026

Warning

Rate limit exceeded

@jexShain has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 9 minutes and 53 seconds before requesting another review.

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 72442ae0-1254-4762-aad6-547141c11871

📥 Commits

Reviewing files that changed from the base of the PR and between 27485bf and 8a25b76.

📒 Files selected for processing (9)
  • crates/aish-hosts/src/lib.rs
  • crates/aish-hosts/src/probe.rs
  • crates/aish-hosts/src/profile.rs
  • crates/aish-hosts/src/store.rs
  • crates/aish-pty/src/persistent.rs
  • crates/aish-shell/src/app.rs
  • crates/aish-shell/src/security_panel.rs
  • crates/aish-tools/src/host_note.rs
  • crates/aish-tools/src/secure_bash.rs
📝 Walkthrough

Walkthrough

Adds a new aish-hosts crate (probe, profile, store), a host_note tool and SecureBash, integrates idle-triggered remote probing and dossier commands into PTY/session flows, and shares remote-host state across multi-round shell/AI interactions.

Changes

Host Profile System Foundation

Layer / File(s) Summary
Crate + workspace entries
Cargo.toml, crates/aish-hosts/Cargo.toml
Register aish-hosts as a workspace member and dependency; new crate manifest declares package and workspace-shared deps.
Public API surface
crates/aish-hosts/src/lib.rs
Declare probe, profile, and store modules and re-export key types/functions.
Shell Probe Generation & Parsing
crates/aish-hosts/src/probe.rs
probe_command() emits a runtime marker and five probe sections; parse_probe_output() builds SystemInfo; helpers parse os-release and infer package manager; unit tests included.
Host Data Models
crates/aish-hosts/src/profile.rs
SystemInfo, HostNote, and HostProfile serializable structs with methods: new, probe_is_stale, add_note (1000-byte truncation, 100-note pruning), remove_notes, format_display, format_for_prompt; unit tests included.
Profile Persistence & Storage
crates/aish-hosts/src/store.rs
Compute config-based hosts dir, sanitize host keys, profile_path; implement load_profile, save_profile, get_or_create_profile with YAML serialization; tests included.

Host Note Tool Implementation

Layer / File(s) Summary
HostNoteTool Definition
crates/aish-tools/src/host_note.rs
Define HostNoteTool with injected store/list/forget callbacks, JSON param schema (action: store/list/forget), localized description, and execute dispatch with validation and formatted results.
Tool Module Export
crates/aish-tools/src/lib.rs
Expose host_note module and re-export HostNoteEntry and HostNoteTool.
Secure Bash
crates/aish-tools/src/secure_bash.rs
Add SecureBashTool wrapping BashTool with preflight security check that strips leading sudo (via strip_sudo) before evaluating security callback; unit tests included.

PTY Interactive Session Integration

Layer / File(s) Summary
PTY dependency & signature
crates/aish-pty/Cargo.toml, crates/aish-pty/src/persistent.rs
Add aish-hosts dependency and update send_command_interactive signature to accept shared host state; minor re-exports formatting.
Execute command buffer
crates/aish-pty/src/persistent.rs
Refactor PTY read loop: append to exec_buffer only when exec_mode is enabled.
Probe state & injection
crates/aish-pty/src/persistent.rs
Add probe capture state, idle-triggered probe injection when profile is stale, marker-count and timeout handling, and timeout safety-net.
Probe capture & parsing
crates/aish-pty/src/persistent.rs
While probe active, collect marker-delimited sections, suppress display, parse into SystemInfo, and save profile with updated timestamp.
Dossier command interception
crates/aish-pty/src/persistent.rs
Intercept remember/notes/forget/refresh dossier commands before AI invocation and perform profile reads/writes, printing results directly.
AI cancellation & followup gating
crates/aish-pty/src/persistent.rs
Add ai_cancelled handling to suppress pending followups and print cancellation output; add hard-abort handling for BashExec follow-ups.
SSH detection & reverse-i-search
crates/aish-pty/src/persistent.rs
Enhance SSH host detection by parsing reverse-i-search lines and add unit tests.
String/ANSI handling
crates/aish-pty/src/persistent.rs
Rewrite strip_ansi_escapes with byte scanning, add pop_last_utf8_from_string, and adjust drain/poll helpers.

Shell Session & Remote Host State Management

Layer / File(s) Summary
Shell wiring & security
crates/aish-shell/Cargo.toml, crates/aish-shell/src/app.rs
Add security_manager to AishShell; initialize it before tool registration; replace prior security-panel UI with inline ANSI confirmation dialog and new text-wrapping helpers.
Shared host state & host_note tool
crates/aish-shell/src/app.rs
Store remote-host in Arc<Mutex<Option<String>>>; add make_host_note_tool that reads shared host; register host_note in remote sessions.
Session callback & dynamic dossier injection
crates/aish-shell/src/app.rs
Change session callback signature to accept shared_host; build remote prompt/dossier dynamically per invocation using current shared host value; include host_note in available tools.
Followup chaining & multi-round flows
crates/aish-shell/src/app.rs
Refactor followup closure wiring to propagate shared host and to coordinate AskUser/BashExec multi-round event chaining and forwarding.
Text helpers & XML stripping
crates/aish-shell/src/app.rs
Introduce pad_to_width, change wrap_text to return a single wrapped string; consolidate XML stripping and remove strip_return_code_block.

Localization & Message Strings

Layer / File(s) Summary
English & Chinese i18n
crates/aish-i18n/locales/en-US.yaml, crates/aish-i18n/locales/zh-CN.yaml
Add session state messages (running, ai_cancelled, probing) and tools.host_note localization (description, params, and result/error strings) in both languages.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Suggested labels

tests

Poem

🐰 I dug a tiny crate beneath the shell,
Markers hum and dossiers softly tell.
Notes tucked in YAML, probes sent on the breeze,
Shared hosts remembered across multi-round seas,
A rabbit hops — profiles snug as can be!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary feature additions: nested SSH detection, host dossier system, and host_note AI tool, which align with the main changes across the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

Thanks for the pull request. A maintainer will review it when available.

Please keep the PR focused, explain the why in the description, and make sure local checks pass before requesting review.

Contribution guide: https://github.com/AI-Shell-Team/aish/blob/main/CONTRIBUTING.md

@github-actions
Copy link
Copy Markdown
Contributor

This pull request description looks incomplete. Please update the missing sections below before review.

Missing items:

  • User-visible Changes
  • Compatibility
  • Testing
  • Change Type
  • Scope

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/aish-hosts/src/store.rs`:
- Around line 5-10: The hosts directory and dossier files are currently created
with system-default permissions; modify the creation code (where hosts_dir() is
used together with std::fs::create_dir_all and std::fs::write) to enforce
private permissions: ensure directories are created with 0o700 and files with
0o600. On Unix, use std::fs::DirBuilder or std::fs::create_dir_all then
std::fs::set_permissions (or use std::os::unix::fs::DirBuilderExt/
OpenOptionsExt::mode) to set 0o700 for the host dossier directory and use
OpenOptions with mode(0o600) (or set_permissions after write) for dossier files;
keep behavior portable by gating Unix-specific APIs under cfg(unix) and falling
back to strict defaults on other platforms.
- Around line 20-26: load_profile currently collapses "file missing" and "YAML
parse failed" into None, causing get_or_create_profile to silently overwrite
corrupted dossiers; change load_profile to return a Result<Option<HostProfile>,
E> (e.g. Result<Option<HostProfile>, anyhow::Error> or
Result<Option<HostProfile>, Box<dyn std::error::Error>>) so it returns Ok(None)
only when the file is not found, Err when read_to_string or serde_yaml::from_str
fail, and Ok(Some(profile)) on success; update the callers (notably
get_or_create_profile and the code referenced at the other occurrence around
lines 38-40) to handle Err by surfacing/logging the parse/read error instead of
creating a fresh profile.

In `@crates/aish-pty/src/persistent.rs`:
- Around line 2458-2473: The extractor extract_remote_host_from_cmd currently
scans tokens in reverse which returns ports or trailing commands; change it to
scan forward over parts.iter().skip(1), skipping tokens that start with '-' and
skipping tokens that are purely numeric (ports), and when you find a candidate
token: if it contains '@' return the substring after the '@', else if it
contains '.' or matches a hostname pattern (not empty and not all digits) return
the token; handle ssh/mosh/telnet/sftp/nc/netcat the same way (first non-option
non-numeric host token). Replace the reverse loop in
extract_remote_host_from_cmd with this forward-scan logic and the numeric check
(token.chars().all(|c| c.is_ascii_digit())) to avoid returning ports or
subsequent commands.
- Around line 893-916: The dossier branch currently returns None for resp which
is indistinguishable from an AI-cancel/error later; update the logic around
handle_dossier_command and interceptor.call_ai so the two states are distinct —
e.g., introduce a boolean flag like dossier_handled or change resp to an enum
(Handled | AiResponse(Option<String>)) so that later code only prints the AI
cancellation banner when dossier_handled is false and the AI call returned
None/error; apply the same change to the analogous block handling lines around
interceptor.call_ai (the second occurrence referenced in the review).
- Around line 2481-2487: The prefix checks only match when a space follows
("remember " / "记住 "), so commands like ";记住foo" or ";forgetbar" slip through;
update the conditional and slicing logic to accept both spaced and unspaced
prefixes: change the starts_with tests to "remember" and "记住" (and likewise for
the forget/忘记 block at the other spot), then compute content by slicing using
the shorter prefix length when no space is present (e.g., if
lower.starts_with("remember ") use &question["remember ".len()..] else use
&question["remember".len()..]), and finally call .trim().to_string() as before;
apply the same change to the other block referenced (lines 2502-2507) so both
spaced and unspaced Chinese prefixes are handled.
- Around line 456-463: The probe never reaches completion because
probe_command() emits 5 markers while the loop expects PROBE_MARKER_COUNT = 6
and never appends probe_current_section before parsing; update the marker
accounting by setting PROBE_MARKER_COUNT to 5 (to match probe_command()) and/or
ensure that when the final marker sequence is detected the code appends
probe_current_section into probe_sections before calling the parser; adjust the
logic around probe_current_section, probe_sections and the loop that checks
PROBE_MARKER_COUNT so captured data is pushed and parsed (affects symbols:
probe_command, PROBE_MARKER_COUNT, probe_current_section, probe_sections,
probe_injected).

In `@crates/aish-shell/src/app.rs`:
- Around line 1857-1863: The closure currently calls profile.add_note(...) and
then ignores the Result from aish_hosts::save_profile(&profile), always
returning t_with_args("tools.host_note.stored", ...); change it to propagate or
handle save errors: call aish_hosts::save_profile(&profile) and if it returns
Err, return or propagate that error (or an appropriate failure message) instead
of the success string; only call t_with_args("tools.host_note.stored", &args)
when save_profile returns Ok. Apply the same fix for the similar block around
profile.forget_note(...) / aish_hosts::save_profile(...) so failures aren’t
swallowed.
- Around line 1543-1547: shared_host is created from
extract_remote_host(command) and only passed into build_session_ai_callback, so
the PTY layer cannot update it when the active SSH hop changes; this pins host
lookups to the first hop. Fix by creating the shared Arc/Mutex (shared_host)
once and passing a clone of that same Arc into pty.send_command_interactive as
well as into Self::build_session_ai_callback so the PTY can update the
shared_host when hops change (ensure the PTY code that detects hop changes
writes the current host into that Arc so host_note/dossier lookups use the live
host).
- Around line 2373-2383: The code appends untrusted host data directly into the
system prompt (see system_msg_with_dossier, dossier_host,
aish_hosts::load_profile and profile.format_for_prompt), which permits
remote-controlled fields to inject instructions; instead, treat the dossier as
data-only by wrapping it with an explicit non-instruction delimiter (e.g.,
prepend a clear label like "HOST DATA (do not interpret as instructions):" and
enclose the dossier in fenced markers such as triple backticks) before appending
to system_msg_with_dossier, or move the dossier into a non-system message
channel (assistant/user/extra) so it is never placed in the highest-priority
system context. Ensure the wrapper is applied every time
profile.format_for_prompt() is added and that empty dossiers remain unchanged.

In `@crates/aish-tools/src/host_note.rs`:
- Around line 71-123: The file fails rustfmt; run the formatter and commit the
changes so the code in the execute method compiles with canonical formatting.
Specifically, run cargo fmt (or rustfmt) on the source that defines the
execute(&self, args: serde_json::Value) -> ToolResult implementation and
reformat the match arms for "store", "list", "forget", and the default branch,
then add the formatted file to the commit.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c2384955-bc40-43e4-8d1f-498fc0689f4c

📥 Commits

Reviewing files that changed from the base of the PR and between 2f6d426 and bc245f1.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (14)
  • Cargo.toml
  • crates/aish-hosts/Cargo.toml
  • crates/aish-hosts/src/lib.rs
  • crates/aish-hosts/src/probe.rs
  • crates/aish-hosts/src/profile.rs
  • crates/aish-hosts/src/store.rs
  • crates/aish-i18n/locales/en-US.yaml
  • crates/aish-i18n/locales/zh-CN.yaml
  • crates/aish-pty/Cargo.toml
  • crates/aish-pty/src/persistent.rs
  • crates/aish-shell/Cargo.toml
  • crates/aish-shell/src/app.rs
  • crates/aish-tools/src/host_note.rs
  • crates/aish-tools/src/lib.rs

Comment thread crates/aish-hosts/src/store.rs
Comment thread crates/aish-hosts/src/store.rs
Comment thread crates/aish-pty/src/persistent.rs Outdated
Comment thread crates/aish-pty/src/persistent.rs
Comment thread crates/aish-pty/src/persistent.rs
Comment thread crates/aish-pty/src/persistent.rs Outdated
Comment thread crates/aish-shell/src/app.rs Outdated
Comment thread crates/aish-shell/src/app.rs
Comment thread crates/aish-shell/src/app.rs Outdated
Comment thread crates/aish-tools/src/host_note.rs
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
crates/aish-pty/src/persistent.rs (1)

2547-2705: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

BashExec cannot actually be hard-aborted with Ctrl+C in this loop.

After approval, the loop only polls master_fd. stdin_fd is never watched, so a quiet long-running command cannot see the user’s Ctrl+C even though the comments say it can be interrupted “at any time”. With BASH_EXEC_IDLE_THRESHOLD = u32::MAX, this can wedge the interactive flow until the command produces output or exits on its own.

Poll stdin alongside the PTY and forward Ctrl+C
                     loop {
                         let mut rfds: libc::fd_set = unsafe { std::mem::zeroed() };
                         unsafe {
                             libc::FD_ZERO(&mut rfds);
+                            libc::FD_SET(stdin_fd, &mut rfds);
                             libc::FD_SET(master_fd, &mut rfds);
                         }
                         let mut tv = libc::timeval {
                             tv_sec: 0,
                             tv_usec: 50_000,
@@
-                                master_fd + 1,
+                                master_fd.max(stdin_fd) + 1,
                                 &mut rfds,
                                 std::ptr::null_mut(),
                                 std::ptr::null_mut(),
                                 &mut tv,
                             )
                         };
+                        if sel > 0 && unsafe { libc::FD_ISSET(stdin_fd, &rfds) } {
+                            let mut byte = [0u8; 1];
+                            if unsafe {
+                                libc::read(
+                                    stdin_fd,
+                                    byte.as_mut_ptr() as *mut libc::c_void,
+                                    1,
+                                )
+                            } == 1 && byte[0] == 0x03
+                            {
+                                unsafe {
+                                    libc::write(
+                                        libc::STDOUT_FILENO,
+                                        b"^C\r\n".as_ptr() as *const libc::c_void,
+                                        4,
+                                    );
+                                    libc::write(
+                                        master_fd,
+                                        b"\x03".as_ptr() as *const libc::c_void,
+                                        1,
+                                    );
+                                }
+                                drain_stdin_trailing(stdin_fd);
+                                let _ = output_sender.send(format!("(cancelled: {})", command));
+                                return true;
+                            }
+                        }
                         if sel > 0
                             && unsafe { libc::FD_ISSET(master_fd, &rfds) }
                         {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/aish-pty/src/persistent.rs` around lines 2547 - 2705, The BashExec
loop only polls master_fd so Ctrl+C on stdin is ignored; update the select loop
inside the AiEvent::BashExec match arm to FD_SET and poll stdin_fd as well (use
nfds = max(master_fd, stdin_fd) + 1), read from stdin_fd when ready, detect 0x03
(Ctrl+C), echo "^C\r\n" to STDOUT, drain_stdin_trailing(stdin_fd), and forward
the interrupt to the running shell by writing the byte (or appropriate signal
byte) into master_fd (or sending SIGINT to the pty process), set hard_abort as
needed and break/stop waiting so the handler sends the appropriate response via
output_sender; keep references to master_fd, stdin_fd, BASH_EXEC_IDLE_THRESHOLD,
captured, and output_sender when implementing this behavior.
🧹 Nitpick comments (1)
crates/aish-tools/src/secure_bash.rs (1)

170-222: ⚡ Quick win

Add bypass-regression tests.

These cases are absent and would have caught the issues in strip_sudo:

#[test]
fn test_strip_sudo_askpass_no_arg() {
    // -A does NOT take an argument; must not swallow the command.
    assert_eq!(strip_sudo("sudo -A rm -rf /"), "rm -rf /");
}

#[test]
fn test_strip_sudo_long_inline_value() {
    assert_eq!(strip_sudo("sudo --user=root rm -rf /"), "rm -rf /");
}

#[test]
fn test_strip_sudo_end_of_options() {
    assert_eq!(strip_sudo("sudo -- rm -rf /"), "rm -rf /");
}

#[test]
fn test_preflight_askpass_does_not_bypass() {
    let tool = SecureBashTool::with_security_check(test_security_check);
    let args = serde_json::json!({"command": "sudo -A rm -rf /"});
    assert!(matches!(tool.preflight(&args), aish_llm::PreflightResult::Block { .. }));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/aish-tools/src/secure_bash.rs` around lines 170 - 222, The tests are
missing regressions for sudo option parsing which caused bypasses; add tests
that ensure strip_sudo correctly handles the -A askpass flag (which takes no
argument), long-form options with inline values like --user=root, and the
end-of-options marker --, and confirm SecureBashTool::preflight (using
test_security_check) still blocks commands when -A is present. Update or add
unit tests named test_strip_sudo_askpass_no_arg,
test_strip_sudo_long_inline_value, test_strip_sudo_end_of_options, and
test_preflight_askpass_does_not_bypass to call strip_sudo and
SecureBashTool::preflight with the example commands from the review comment and
assert the expected outputs/Block results.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/aish-pty/src/persistent.rs`:
- Around line 2972-2984: The English branch currently calls strip_prefix(&lower,
"remember ", "remember") which uses the lowercased input and causes saved notes
to be permanently lowercased; change the matching so only the command detection
is case-insensitive but the saved content comes from the original input
(question) — e.g., perform case-insensitive prefix check on lower but extract
and trim the remainder from question (not lower) before calling profile.add_note
and aish_hosts::save_profile; keep references to strip_prefix, lower, question,
profile.add_note, aish_hosts::save_profile and host_key when making the change.
- Around line 2911-2944: The extracted destination token in
extract_remote_host_from_cmd is returned verbatim which breaks later comparisons
and also fails to match commands invoked via a full path; update
extract_remote_host_from_cmd to first match the command by its basename (e.g.,
strip path from parts.first() before comparing to "ssh|telnet|...") and then
normalize the returned destination by removing any user@ prefix (keep only the
hostname), strip optional port suffixes like :2222 or [host]:port, and trim
surrounding brackets so the stored host is the bare hostname; ensure this
normalization happens before returning from extract_remote_host_from_cmd so that
comparisons in scan_output_for_disconnect() and behavior vs is_session_command()
are consistent.
- Around line 1249-1329: The scanner is mistaking the outer session's echoed
"ssh host" for a nested hop; when scan_output_for_ssh_host returns a host that
equals the already-set remote_host_for_probe, skip treating it as a nested
session. Concretely, inside the block handling the Some(host) from
scan_output_for_ssh_host, add a guard that compares host to
remote_host_for_probe.as_deref() (or shared_host) and if equal, clear or drain
the echoed chunk (e.g. output_ssh_scan) and continue without pushing onto
nested_host_stack or toggling probe_active/nested_probe_pending; only run the
existing nested-host push/assignment logic when the discovered host differs from
remote_host_for_probe.

In `@crates/aish-tools/src/secure_bash.rs`:
- Around line 13-53: strip_sudo currently unsafely parses sudo flags (e.g.,
treating -A and long flags with 'u' as having arguments) so it can strip real
commands and allow bypasses; update strip_sudo to use a proper tokenizer (e.g.,
shell-words) to split into tokens, implement explicit handling for "--"
end-of-options, handle long flags and "--long=value" vs "--long value"
correctly, and replace the heuristic flag.contains(...) logic with a closed
allowlist of single-char flags that take an argument (e.g., u g U p r t T C D h)
while treating -A as no-arg; ensure tokens are processed left-to-right and
return the first non-flag token as the command (or unchanged trimmed input if
none), referencing the strip_sudo function and the existing flag-parsing loop to
locate where to replace logic.
- Around line 146-149: SecureBashTool.execute currently returns
self.inner.execute(...) and relies on preflight being called elsewhere, but
Execute can be reached via execute_tool_by_name / execute_async (used in
ParsedReact::Actions) which bypasses preflight; fix by enforcing preflight at
the call site or in the tool: either (A) update execute_tool_by_name (or
ParsedReact::Actions call path) to call preflight() and abort on failure before
calling execute_async()/execute(), or (B) change SecureBashTool.execute to call
self.preflight() (or a shared validation method) and return an error if
preflight fails, ensuring any direct execute_async()/execute() invocation gets
the security check; reference SecureBashTool.execute, SecureBashTool.preflight,
execute_tool_by_name, execute_tool_external, execute_async, and
ParsedReact::Actions to find the relevant spots to implement the change.

In `@crates/aish-tools/src/system_diagnose.rs`:
- Line 183: The diagnose flow currently constructs SecureBashTool via
SecureBashTool::new(), which sets security_check to None and effectively behaves
like BashTool; update SystemDiagnoseTool to carry an optional security_check
callback (add a security_check field and a builder method on
SystemDiagnoseTool), then instantiate the tool with
SecureBashTool::with_security_check(self.security_check.clone()) instead of
SecureBashTool::new() when building the tools list; alternatively, if diagnose
should remain trusted, revert the change and use BashTool::new() and remove the
misleading security wording from the commit/message.

---

Outside diff comments:
In `@crates/aish-pty/src/persistent.rs`:
- Around line 2547-2705: The BashExec loop only polls master_fd so Ctrl+C on
stdin is ignored; update the select loop inside the AiEvent::BashExec match arm
to FD_SET and poll stdin_fd as well (use nfds = max(master_fd, stdin_fd) + 1),
read from stdin_fd when ready, detect 0x03 (Ctrl+C), echo "^C\r\n" to STDOUT,
drain_stdin_trailing(stdin_fd), and forward the interrupt to the running shell
by writing the byte (or appropriate signal byte) into master_fd (or sending
SIGINT to the pty process), set hard_abort as needed and break/stop waiting so
the handler sends the appropriate response via output_sender; keep references to
master_fd, stdin_fd, BASH_EXEC_IDLE_THRESHOLD, captured, and output_sender when
implementing this behavior.

---

Nitpick comments:
In `@crates/aish-tools/src/secure_bash.rs`:
- Around line 170-222: The tests are missing regressions for sudo option parsing
which caused bypasses; add tests that ensure strip_sudo correctly handles the -A
askpass flag (which takes no argument), long-form options with inline values
like --user=root, and the end-of-options marker --, and confirm
SecureBashTool::preflight (using test_security_check) still blocks commands when
-A is present. Update or add unit tests named test_strip_sudo_askpass_no_arg,
test_strip_sudo_long_inline_value, test_strip_sudo_end_of_options, and
test_preflight_askpass_does_not_bypass to call strip_sudo and
SecureBashTool::preflight with the example commands from the review comment and
assert the expected outputs/Block results.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ffccb1fb-d987-471e-810f-a92cc3419195

📥 Commits

Reviewing files that changed from the base of the PR and between bc245f1 and 5b23374.

📒 Files selected for processing (5)
  • crates/aish-pty/src/persistent.rs
  • crates/aish-shell/src/app.rs
  • crates/aish-tools/src/lib.rs
  • crates/aish-tools/src/secure_bash.rs
  • crates/aish-tools/src/system_diagnose.rs
🚧 Files skipped from review as they are similar to previous changes (2)
  • crates/aish-tools/src/lib.rs
  • crates/aish-shell/src/app.rs

Comment thread crates/aish-pty/src/persistent.rs
Comment thread crates/aish-pty/src/persistent.rs
Comment thread crates/aish-pty/src/persistent.rs
Comment thread crates/aish-tools/src/secure_bash.rs
Comment thread crates/aish-tools/src/secure_bash.rs
Comment thread crates/aish-tools/src/system_diagnose.rs
…I tool

Add nested SSH session detection with prompt pattern matching, Ctrl+C
hard abort support, and interrupt grace handling. Introduce host dossier
system (probe/profile/store) and host_note AI tool for persisting
per-host context across sessions.
@jexShain jexShain force-pushed the feat/nested-ssh-detection branch from 5b23374 to 27485bf Compare May 12, 2026 05:18
@github-actions github-actions Bot added ci-cd docs packaging Packaging or installation issue labels May 12, 2026
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@crates/aish-hosts/src/probe.rs`:
- Around line 94-113: The infer_package_manager function currently only
exact-matches ID and misses IDs like "opensuse-leap"/"opensuse-tumbleweed" and
ignores ID_LIKE; update infer_package_manager to (1) normalize the parsed ID by
trimming quotes, lowercasing and treating IDs with hyphenated suffixes as their
base (e.g., split on '-' or use starts_with checks so "opensuse-*" maps to
"zypper"), and (2) if no package manager is found from ID, parse an ID_LIKE line
from os_release and try each space/comma-separated token the same way to map to
apt/yum/dnf/pacman/zypper/apk; ensure the function still returns String and uses
the same matching arms (e.g., the match for "opensuse"/"suse"/"sles" should
accept the normalized form or ID_LIKE tokens).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: f173324a-5f99-4007-9a68-d86cec1c1546

📥 Commits

Reviewing files that changed from the base of the PR and between 5b23374 and 27485bf.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (18)
  • Cargo.toml
  • crates/aish-hosts/Cargo.toml
  • crates/aish-hosts/src/lib.rs
  • crates/aish-hosts/src/probe.rs
  • crates/aish-hosts/src/profile.rs
  • crates/aish-hosts/src/store.rs
  • crates/aish-i18n/locales/en-US.yaml
  • crates/aish-i18n/locales/zh-CN.yaml
  • crates/aish-pty/Cargo.toml
  • crates/aish-pty/src/lib.rs
  • crates/aish-pty/src/persistent.rs
  • crates/aish-pty/src/session_interceptor.rs
  • crates/aish-shell/Cargo.toml
  • crates/aish-shell/src/app.rs
  • crates/aish-tools/src/host_note.rs
  • crates/aish-tools/src/lib.rs
  • crates/aish-tools/src/secure_bash.rs
  • crates/aish-tools/src/system_diagnose.rs
✅ Files skipped from review due to trivial changes (7)
  • crates/aish-pty/src/lib.rs
  • crates/aish-shell/Cargo.toml
  • Cargo.toml
  • crates/aish-pty/Cargo.toml
  • crates/aish-i18n/locales/zh-CN.yaml
  • crates/aish-i18n/locales/en-US.yaml
  • crates/aish-hosts/Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (8)
  • crates/aish-hosts/src/lib.rs
  • crates/aish-tools/src/system_diagnose.rs
  • crates/aish-hosts/src/store.rs
  • crates/aish-tools/src/host_note.rs
  • crates/aish-tools/src/lib.rs
  • crates/aish-hosts/src/profile.rs
  • crates/aish-tools/src/secure_bash.rs
  • crates/aish-shell/src/app.rs

Comment thread crates/aish-hosts/src/probe.rs
jexShain added 5 commits May 12, 2026 13:38
The store and forget closures were discarding save_profile errors
with `let _ =`, reporting success even when the write failed.
- Replace useless format!() with .to_string() in probe_command
- Use is_some_and() instead of map_or(false, ...)
- Fix u32::MAX comparison in BASH_EXEC_IDLE_THRESHOLD check
- Collapse nested if into single condition in extract_remote_host
- Remove unused extract_command_from_isearch_line function
- Allow dead_code for security_panel utilities (not yet wired up)
@jexShain jexShain merged commit f82bb4e into AI-Shell-Team:rust May 12, 2026
7 checks passed
jexShain added a commit to jexShain/aish that referenced this pull request May 14, 2026
…I tool (AI-Shell-Team#157)

* feat(ssh): nested SSH detection, host dossier system, and host_note AI tool

Add nested SSH session detection with prompt pattern matching, Ctrl+C
hard abort support, and interrupt grace handling. Introduce host dossier
system (probe/profile/store) and host_note AI tool for persisting
per-host context across sessions.

* style: apply rustfmt formatting

* fix: propagate save_profile errors in host_note tool closures

The store and forget closures were discarding save_profile errors
with `let _ =`, reporting success even when the write failed.

* fix: resolve clippy warnings for CI

- Replace useless format!() with .to_string() in probe_command
- Use is_some_and() instead of map_or(false, ...)
- Fix u32::MAX comparison in BASH_EXEC_IDLE_THRESHOLD check
- Collapse nested if into single condition in extract_remote_host
- Remove unused extract_command_from_isearch_line function
- Allow dead_code for security_panel utilities (not yet wired up)

* fix: collapse nested if into match guards for clippy 1.95

* fix: remove orphaned test for deleted extract_command_from_isearch_line
@jexShain jexShain deleted the feat/nested-ssh-detection branch May 14, 2026 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant