Skip to content

fix(discover): register ssh in the rewrite rule table so hook routes it#2004

Open
YOMXXX wants to merge 1 commit into
rtk-ai:developfrom
YOMXXX:fix/ssh-rewrite-no-match
Open

fix(discover): register ssh in the rewrite rule table so hook routes it#2004
YOMXXX wants to merge 1 commit into
rtk-ai:developfrom
YOMXXX:fix/ssh-rewrite-no-match

Conversation

@YOMXXX
Copy link
Copy Markdown
Contributor

@YOMXXX YOMXXX commented May 21, 2026

Summary

`rtk verify --filter ssh` reports 3/3 passing because the TOML filter at `src/filters/ssh.toml` is valid, but `rtk rewrite "ssh …"` exits 1 for every shape — the Claude Code hook's rewrite path therefore never produces `rtk ssh …` and the ssh filter is effectively dark in production, even though verify passes.

Reporter (#1654) saw ssh as the #1 unhandled command in 30 days of `rtk discover`, with ~50K–100K tokens/month of savings missed on a single user.

Root cause

`registry::rewrite_command` → `rewrite_segment_inner` looks the base command up in `RULES` (`src/discover/rules.rs`). `RULES` is the source of truth for the hook rewrite path, but it did not list ssh. `rtk verify` consults the TOML filter registry; `rtk rewrite` consults the hard-coded RULES table — so a TOML-only filter with no RULES entry passes verify yet is invisible to the hook.

This split is the same shape as the historical `ps` / `ansible-playbook` setup that the reporter mentions still working — those have RULES entries; ssh did not.

Fix

Add one `RtkRule` for ssh next to curl/wget (same `Network` category, same `^cmd\s+` pattern shape). Once `rtk rewrite` produces `rtk ssh …`, main.rs's TOML-filter dispatch picks up `src/filters/ssh.toml` unchanged — no other code paths needed.

```rust
RtkRule {
pattern: r"^ssh\s+",
rtk_cmd: "rtk ssh",
rewrite_prefixes: &["ssh"],
category: "Network",
savings_pct: 65.0,
...
}
```

Behaviour

Input After
`ssh user@host uptime` `rtk ssh user@host uptime` (Allow + filter)
`ssh -o ConnectTimeout=5 host echo OK` `rtk ssh -o ConnectTimeout=5 host echo OK`
`ssh host` (login only) `rtk ssh host` (banners still filtered by `strip_lines_matching`)
bare `ssh` (no args) unchanged (`\s+` guard, nothing to filter)

`GIT_SSH_COMMAND="ssh -o …" git push` continues to env-strip → `rtk git push` exactly as today; covered by existing `test_classify_env_prefix_stripped`.

Test plan

  • `test_classify_ssh_with_remote_command` — Supported / Network classification, locks the rule-order invariant.
  • `test_rewrite_ssh_shapes` — three reproduction shapes from the issue verbatim.
  • `test_rewrite_ssh_with_host_only` — the no-remote-command shape from the repro.
  • `test_rewrite_bare_ssh_not_rewritten` — guards the `\s+` boundary so a future loosened pattern doesn't start rewriting argv-less `ssh`.
  • `cargo fmt --all` clean.
  • `cargo clippy --all-targets` zero warnings.
  • `cargo test --bin rtk -- --test-threads=8` → 1913 passed (was 1909).

Notes

  • `src/filters/ssh.toml` itself is unchanged; this PR only fills the RULES gap that kept the hook from ever reaching it.
  • Token savings for an ssh command depend heavily on remote output, but `ssh.toml` strips connection banners + caps at `max_lines = 200`; `savings_pct: 65` follows wget's neighbour entry as a conservative estimate.
  • `rtk verify --filter ssh` continues to pass (filter logic untouched).

Fixes #1654

`rtk verify --filter ssh` reports 3/3 tests passing because the TOML
filter at `src/filters/ssh.toml` is valid, but `rtk rewrite "ssh ..."`
returned exit 1 for every shape ‑- the Claude Code hook's rewrite path
therefore never produced `rtk ssh ...` and the ssh filter was effectively
dark in production, despite passing verify.

Root cause: `rewrite_command` ➜ `rewrite_segment_inner` looks up the base
command in `RULES` (`src/discover/rules.rs`). `RULES` is the source of
truth for the hook rewrite path, but it didn't list ssh. Verify and
rewrite consult two different tables, so a TOML-only filter with no
RULES entry passes verify yet is invisible to the hook.

Fix: add a single `RtkRule` for ssh next to the existing curl/wget
entries (same `Network` category, same `\s+` pattern shape). Once
rewritten, main.rs's TOML-filter dispatch picks up `src/filters/ssh.toml`
unchanged ‑- no other code paths needed.

Behaviour:
- `ssh user@host uptime`           → `rtk ssh user@host uptime`        (Allow + filter)
- `ssh -o ConnectTimeout=5 h cmd`  → `rtk ssh -o ConnectTimeout=5 h cmd`
- `ssh host`                       → `rtk ssh host`                    (login banners still benefit from strip_lines_matching)
- bare `ssh` (no args)             → unchanged                          (`\s+` guard, nothing to filter)

Tests:
- test_classify_ssh_with_remote_command pins the Supported / Network
  classification so future tweaks to RULES order can't silently undo
  this.
- test_rewrite_ssh_shapes locks in the three reproduction shapes from
  the issue verbatim.
- test_rewrite_ssh_with_host_only covers the no-remote-command shape
  that the issue's repro script also exits 1 for today.
- test_rewrite_bare_ssh_not_rewritten guards the `\s+` boundary so a
  future loosened pattern doesn't start rewriting argv-less `ssh`.

cargo fmt / clippy --all-targets / test --bin rtk: 1913 passed,
0 warnings.

Fixes rtk-ai#1654
@YOMXXX
Copy link
Copy Markdown
Contributor Author

YOMXXX commented May 24, 2026

Review note: I would not merge this as-is because it rewrites interactive SSH shapes.

The tests intentionally rewrite ssh host to rtk ssh host, but that can be an interactive login session. Running it through RTK's TOML filter means stdout is captured/filtered, which can change or break interactive SSH, port forwarding, and no-command sessions like ssh -N, ssh -L, or ssh -D.

Please narrow the rewrite rule to remote-command forms only, or explicitly exclude interactive/forwarding shapes. Also, the new comment in rules.rs says interactive ssh host is intentionally not rewritten, while the test asserts that it is rewritten. That contradiction should be resolved with the behavior change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSH filter passes verify (3/3) but rtk rewrite returns exit 1 — ssh commands unintercepted on Claude Code hook path

1 participant