PTY: strip CPR (cursor-position report) replies before forwarding to consumers #55

Open
opened 2026-04-28 03:21:47 +00:00 by despiegk · 3 comments
Owner

Summary

hero_proc forwards raw PTY master bytes straight to WebSocket consumers (xterm.js etc.) with no escape-sequence filtering. When anything in the pipeline issues a DSR (\x1b[6n) query, the kernel/pty answers with a CPR reply of the form \x1b[<row>;<col>R. That reply is meant for whoever issued the query — but in our setup it leaks all the way to xterm.js and renders as visible junk like ^[[24;80R in the terminal pane.

hero_router's terminal already intercepts the DSR query path locally (commit 60b9ea6 in hero_router) so the shell shouldn't normally see one. But that's a client-side workaround: any other consumer of hero_proc's PTY stream (different UI, agent, test harness) reproduces the bug, and any process inside the PTY that issues DSR on its own (vim, tmux, bash readline under some terminfo entries) still triggers it. The fix belongs at the source.

Where it leaks

  • Read loop: crates/hero_proc_server/src/supervisor/executor.rs run_job_pty() ~line 718, hot loop at 833–864.
  • Reads 4096-byte chunks from the PTY master and broadcasts them unchanged to subscribers (line 839: broadcast_tx_reader.send(data.clone())).
  • WebSocket consumer: crates/hero_proc_server/src/web.rs handle_pty_ws() ~line 314 — sends raw binary frames straight through.
  • Zero escape-sequence parsing anywhere on the path.
  • Scrollback (supervisor/mod.rs ~25–46) also captures the unfiltered bytes, so CPR survives in history.

Proposed fix

Add a small stateful CPR-strip filter that runs immediately after the read() at executor.rs:834, before the broadcast/scrollback writes.

Key constraints:

  1. Stateful across reads. A CPR reply (\x1b[N;NR, ~6–7 bytes) can be split across two 4096-byte reads. The filter has to buffer an incomplete \x1b[… tail and prepend it to the next chunk.
  2. Match only CPR, not all CSI. Regex shape: \x1b\[\d+;\d+R. Don't strip other CSI sequences — colors, cursor moves, etc., must pass through untouched.
  3. Apply to both broadcast and scrollback writes so history stays clean too.
  4. Hot path. Avoid regex; a tiny hand-written state machine (4 states: Normal, EscSeen, CsiBody, Done) is cheap and allocation-free.

Sketch:

struct CprFilter { pending: Vec<u8> }

impl CprFilter {
    /// Returns the input with any complete CPR sequences removed.
    /// Buffers an incomplete trailing `\x1b[…` for the next call.
    fn filter(&mut self, chunk: &[u8]) -> Vec<u8> { /* ... */ }
}

One instance per PTY job, owned by the read task.

Optional: opt-out

If there's any future consumer that legitimately wants CPR, gate the filter behind a per-job flag (default on). For the current terminal-streaming use case, unconditional strip is fine.

Tests

tests/integration/tests/pty.rs has no escape-sequence coverage. Add:

  • CPR fully inside one chunk → stripped.
  • CPR split across two chunks (\x1b[24 + ;80R) → stripped.
  • Non-CPR CSI (\x1b[31m) → passed through.
  • Lone \x1b at end of stream → buffered, not emitted as garbage.

Out of scope

  • General ANSI sanitization. Only CPR is leaking and only CPR is causing visible damage.
  • Removing the hero_router-side DSR interception. It's still a useful belt-and-suspenders for the common case and avoids an extra round-trip.
## Summary `hero_proc` forwards raw PTY master bytes straight to WebSocket consumers (xterm.js etc.) with no escape-sequence filtering. When *anything* in the pipeline issues a DSR (`\x1b[6n`) query, the kernel/pty answers with a CPR reply of the form `\x1b[<row>;<col>R`. That reply is meant for whoever issued the query — but in our setup it leaks all the way to xterm.js and renders as visible junk like `^[[24;80R` in the terminal pane. `hero_router`'s terminal already intercepts the DSR *query* path locally (commit 60b9ea6 in hero_router) so the shell shouldn't normally see one. But that's a client-side workaround: any other consumer of hero_proc's PTY stream (different UI, agent, test harness) reproduces the bug, and any process inside the PTY that issues DSR on its own (vim, tmux, bash readline under some terminfo entries) still triggers it. The fix belongs at the source. ## Where it leaks - Read loop: `crates/hero_proc_server/src/supervisor/executor.rs` `run_job_pty()` ~line 718, hot loop at 833–864. - Reads 4096-byte chunks from the PTY master and broadcasts them unchanged to subscribers (line 839: `broadcast_tx_reader.send(data.clone())`). - WebSocket consumer: `crates/hero_proc_server/src/web.rs` `handle_pty_ws()` ~line 314 — sends raw binary frames straight through. - Zero escape-sequence parsing anywhere on the path. - Scrollback (`supervisor/mod.rs` ~25–46) also captures the unfiltered bytes, so CPR survives in history. ## Proposed fix Add a small stateful CPR-strip filter that runs immediately after the `read()` at executor.rs:834, before the broadcast/scrollback writes. Key constraints: 1. **Stateful across reads.** A CPR reply (`\x1b[N;NR`, ~6–7 bytes) can be split across two 4096-byte reads. The filter has to buffer an incomplete `\x1b[…` tail and prepend it to the next chunk. 2. **Match only CPR, not all CSI.** Regex shape: `\x1b\[\d+;\d+R`. Don't strip other CSI sequences — colors, cursor moves, etc., must pass through untouched. 3. **Apply to both broadcast and scrollback writes** so history stays clean too. 4. **Hot path.** Avoid regex; a tiny hand-written state machine (4 states: Normal, EscSeen, CsiBody, Done) is cheap and allocation-free. Sketch: ```rust struct CprFilter { pending: Vec<u8> } impl CprFilter { /// Returns the input with any complete CPR sequences removed. /// Buffers an incomplete trailing `\x1b[…` for the next call. fn filter(&mut self, chunk: &[u8]) -> Vec<u8> { /* ... */ } } ``` One instance per PTY job, owned by the read task. ## Optional: opt-out If there's any future consumer that legitimately wants CPR, gate the filter behind a per-job flag (default on). For the current terminal-streaming use case, unconditional strip is fine. ## Tests `tests/integration/tests/pty.rs` has no escape-sequence coverage. Add: - CPR fully inside one chunk → stripped. - CPR split across two chunks (`\x1b[24` + `;80R`) → stripped. - Non-CPR CSI (`\x1b[31m`) → passed through. - Lone `\x1b` at end of stream → buffered, not emitted as garbage. ## Out of scope - General ANSI sanitization. Only CPR is leaking and only CPR is causing visible damage. - Removing the hero_router-side DSR interception. It's still a useful belt-and-suspenders for the common case and avoids an extra round-trip.
Author
Owner

Implementation Spec for Issue #55

Objective

Strip CPR (cursor-position report) replies of the form \x1b[<row>;<col>R from PTY master output before forwarding to WebSocket consumers and the scrollback buffer, so they never render as visible junk in xterm.js or any other consumer. The fix lives at the source (run_job_pty) and is implemented as a small, allocation-light, stateful filter that survives chunk boundaries.

Requirements

  • Strip only complete CPR sequences matching \x1b\[\d+;\d+R. Leave all other CSI sequences (SGR, cursor moves, etc.) untouched.
  • Stateful across reads: a CPR reply that straddles two PTY reads (e.g. \x1b[24 then ;80R) must still be stripped.
  • Apply the filter once per chunk, before both the broadcast send and the scrollback push, so history matches what live consumers see.
  • Hot-path friendly: hand-written state machine, no regex, no allocations on the common (no-CPR) path.
  • One filter instance per PTY job, owned by the spawn_blocking reader task.
  • Lone trailing \x1b (or \x1b[, or \x1b[24) at the end of a chunk must be buffered and not emitted as garbage.

Files to Modify/Create

  • crates/hero_proc_server/src/supervisor/cpr_filter.rs — NEW. Houses CprFilter, the state-machine implementation, and unit tests.
  • crates/hero_proc_server/src/supervisor/mod.rs — add pub mod cpr_filter; next to the existing pub mod executor; / pub mod shutdown; declarations.
  • crates/hero_proc_server/src/supervisor/executor.rs — wire CprFilter into the run_job_pty reader loop so it runs once per chunk before the broadcast, scrollback, and log-batcher writes.
  • tests/integration/tests/pty.rs — optionally add an integration test that injects a CPR reply via printf. Primary coverage stays in unit tests.

Implementation Plan

Step 1: Create the CprFilter module with state machine

Files: crates/hero_proc_server/src/supervisor/cpr_filter.rs (new)

  • Define states: enum State { Normal, EscSeen, CsiBody }.
  • Define struct:
    pub struct CprFilter {
        state: State,
        pending: Vec<u8>,
    }
    
  • Public API:
    • pub fn new() -> Self — start in Normal, empty pending.
    • pub fn filter(&mut self, chunk: &[u8]) -> Vec<u8> — consume chunk, return bytes safe to forward.
  • Algorithm per byte:
    • Normal: if byte == 0x1B, transition to EscSeen, push 0x1B into pending. Otherwise, append directly to output.
    • EscSeen: if byte == [, transition to CsiBody, push [ into pending. Otherwise flush pending and the current byte to output, return to Normal.
    • CsiBody: if byte is a digit or ;, accumulate into pending. If byte == b'R' AND the accumulated tail matches ^\x1b\[\d+;\d+$: drop pending (CPR), clear, return to Normal. If byte is any other terminator or pattern-breaker: flush pending plus byte, clear, return to Normal.
  • Fast path: if chunk contains no 0x1B and state == Normal and pending is empty, return chunk.to_vec() directly.

Dependencies: none.

Step 2: Unit tests inside cpr_filter.rs

Files: crates/hero_proc_server/src/supervisor/cpr_filter.rs (same file, #[cfg(test)] mod tests)

  • cpr_in_one_chunk_is_stripped: input b"hello\x1b[24;80Rworld" → output b"helloworld".
  • cpr_split_across_two_chunks_is_stripped: feed b"hello\x1b[24" then b";80Rworld". Combined output is b"helloworld".
  • cpr_split_at_every_byte_boundary: parameterized for every split point of b"\x1b[24;80R".
  • non_cpr_csi_passes_through_unchanged: input b"\x1b[31mred\x1b[0m" → identical output.
  • lone_trailing_esc_is_buffered: feed b"abc\x1b", then b"def" — combined output is b"abc\x1bdef".
  • incomplete_csi_then_invalid_terminator_flushes: feed b"\x1b[24;80" then b"X" — combined output is b"\x1b[24;80X".
  • cpr_back_to_back: input b"\x1b[24;80R\x1b[1;1R" → output b"".
  • mixed_cpr_and_color_sgr: input b"a\x1b[31m\x1b[24;80Rb" → output b"a\x1b[31mb".
  • empty_chunk and chunk_of_only_normal_bytes — sanity / fast-path coverage.

Dependencies: Step 1.

Step 3: Wire CprFilter into the PTY reader loop

Files: crates/hero_proc_server/src/supervisor/executor.rs, crates/hero_proc_server/src/supervisor/mod.rs

  • In mod.rs, add pub mod cpr_filter;.
  • In executor.rs, inside run_job_pty, find the tokio::task::spawn_blocking(move || { ... }) reader. Modify it as follows:
    1. Before the loop {, instantiate let mut cpr_filter = crate::supervisor::cpr_filter::CprFilter::new(); so the filter is owned by the closure.
    2. Inside the Ok(n) => arm, replace let data = buf[..n].to_vec(); with:
      let data = cpr_filter.filter(&buf[..n]);
      if data.is_empty() {
          continue;
      }
      
  • No changes needed to web.rs::handle_pty_ws — broadcast already carries pre-filtered bytes.
  • No changes needed to ScrollbackBuffer — data going into it is already clean.

Dependencies: Step 1.

Step 4: Optional integration coverage in tests/integration/tests/pty.rs

Files: tests/integration/tests/pty.rs

  • Add a third test that:
    1. Starts a PTY service running /bin/sh.
    2. Sends printf '\\033[24;80R\\n' over the WebSocket.
    3. Collects WebSocket output for ~2 seconds.
    4. Asserts the collected bytes do NOT contain the byte sequence \x1b[24;80R.
  • Caveats: this synthesizes a CPR byte stream rather than triggering a real DSR/CPR exchange. If flaky in CI, drop and rely on unit tests.

Dependencies: Step 3.

Acceptance Criteria

  • New module crates/hero_proc_server/src/supervisor/cpr_filter.rs with CprFilter::new() and CprFilter::filter(&mut self, &[u8]) -> Vec<u8>.
  • pub mod cpr_filter; added to supervisor/mod.rs.
  • All unit tests in Step 2 pass.
  • run_job_pty instantiates one CprFilter per job and routes every PTY chunk through it before broadcast, scrollback, and log-batcher writes.
  • xterm.js no longer sees ^[[24;80R artifacts when a process emits a CPR reply.
  • Non-CPR escape sequences still render correctly (no regressions in colors, cursor moves, alt-screen).
  • Scrollback replay contains no CPR bytes.
  • No new external dependencies added to crates/hero_proc_server/Cargo.toml.
  • cargo clippy -p hero_proc_server --all-targets -- -D warnings clean.

Notes

  • The filter must NOT generalize to "strip any CSI ending in R". Keep the <digits>;<digits> shape strict.
  • The filter must clone pending into the output stream when the sequence turns out NOT to be a CPR (e.g. \x1b[31m color). Do not lose those bytes — that would corrupt SGR.
  • Per the issue's "Out of scope": do not touch the hero_router-side DSR interception, and do not add a general ANSI sanitizer.
  • Sequencing: Steps 1+2 first; Step 3 depends on Step 1; Step 4 depends on Step 3.
## Implementation Spec for Issue #55 ### Objective Strip CPR (cursor-position report) replies of the form `\x1b[<row>;<col>R` from PTY master output before forwarding to WebSocket consumers and the scrollback buffer, so they never render as visible junk in xterm.js or any other consumer. The fix lives at the source (`run_job_pty`) and is implemented as a small, allocation-light, stateful filter that survives chunk boundaries. ### Requirements - Strip only complete CPR sequences matching `\x1b\[\d+;\d+R`. Leave all other CSI sequences (SGR, cursor moves, etc.) untouched. - Stateful across reads: a CPR reply that straddles two PTY reads (e.g. `\x1b[24` then `;80R`) must still be stripped. - Apply the filter once per chunk, before both the broadcast send and the scrollback push, so history matches what live consumers see. - Hot-path friendly: hand-written state machine, no regex, no allocations on the common (no-CPR) path. - One filter instance per PTY job, owned by the spawn_blocking reader task. - Lone trailing `\x1b` (or `\x1b[`, or `\x1b[24`) at the end of a chunk must be buffered and not emitted as garbage. ### Files to Modify/Create - `crates/hero_proc_server/src/supervisor/cpr_filter.rs` — NEW. Houses `CprFilter`, the state-machine implementation, and unit tests. - `crates/hero_proc_server/src/supervisor/mod.rs` — add `pub mod cpr_filter;` next to the existing `pub mod executor;` / `pub mod shutdown;` declarations. - `crates/hero_proc_server/src/supervisor/executor.rs` — wire `CprFilter` into the `run_job_pty` reader loop so it runs once per chunk before the broadcast, scrollback, and log-batcher writes. - `tests/integration/tests/pty.rs` — optionally add an integration test that injects a CPR reply via `printf`. Primary coverage stays in unit tests. ### Implementation Plan #### Step 1: Create the `CprFilter` module with state machine Files: `crates/hero_proc_server/src/supervisor/cpr_filter.rs` (new) - Define states: `enum State { Normal, EscSeen, CsiBody }`. - Define struct: ```rust pub struct CprFilter { state: State, pending: Vec<u8>, } ``` - Public API: - `pub fn new() -> Self` — start in `Normal`, empty pending. - `pub fn filter(&mut self, chunk: &[u8]) -> Vec<u8>` — consume `chunk`, return bytes safe to forward. - Algorithm per byte: - `Normal`: if byte == `0x1B`, transition to `EscSeen`, push `0x1B` into `pending`. Otherwise, append directly to output. - `EscSeen`: if byte == `[`, transition to `CsiBody`, push `[` into `pending`. Otherwise flush `pending` and the current byte to output, return to `Normal`. - `CsiBody`: if byte is a digit or `;`, accumulate into `pending`. If byte == `b'R'` AND the accumulated tail matches `^\x1b\[\d+;\d+$`: drop `pending` (CPR), clear, return to `Normal`. If byte is any other terminator or pattern-breaker: flush `pending` plus byte, clear, return to `Normal`. - Fast path: if `chunk` contains no `0x1B` and `state == Normal` and `pending` is empty, return `chunk.to_vec()` directly. Dependencies: none. #### Step 2: Unit tests inside `cpr_filter.rs` Files: `crates/hero_proc_server/src/supervisor/cpr_filter.rs` (same file, `#[cfg(test)] mod tests`) - `cpr_in_one_chunk_is_stripped`: input `b"hello\x1b[24;80Rworld"` → output `b"helloworld"`. - `cpr_split_across_two_chunks_is_stripped`: feed `b"hello\x1b[24"` then `b";80Rworld"`. Combined output is `b"helloworld"`. - `cpr_split_at_every_byte_boundary`: parameterized for every split point of `b"\x1b[24;80R"`. - `non_cpr_csi_passes_through_unchanged`: input `b"\x1b[31mred\x1b[0m"` → identical output. - `lone_trailing_esc_is_buffered`: feed `b"abc\x1b"`, then `b"def"` — combined output is `b"abc\x1bdef"`. - `incomplete_csi_then_invalid_terminator_flushes`: feed `b"\x1b[24;80"` then `b"X"` — combined output is `b"\x1b[24;80X"`. - `cpr_back_to_back`: input `b"\x1b[24;80R\x1b[1;1R"` → output `b""`. - `mixed_cpr_and_color_sgr`: input `b"a\x1b[31m\x1b[24;80Rb"` → output `b"a\x1b[31mb"`. - `empty_chunk` and `chunk_of_only_normal_bytes` — sanity / fast-path coverage. Dependencies: Step 1. #### Step 3: Wire `CprFilter` into the PTY reader loop Files: `crates/hero_proc_server/src/supervisor/executor.rs`, `crates/hero_proc_server/src/supervisor/mod.rs` - In `mod.rs`, add `pub mod cpr_filter;`. - In `executor.rs`, inside `run_job_pty`, find the `tokio::task::spawn_blocking(move || { ... })` reader. Modify it as follows: 1. Before the `loop {`, instantiate `let mut cpr_filter = crate::supervisor::cpr_filter::CprFilter::new();` so the filter is owned by the closure. 2. Inside the `Ok(n) =>` arm, replace `let data = buf[..n].to_vec();` with: ```rust let data = cpr_filter.filter(&buf[..n]); if data.is_empty() { continue; } ``` - No changes needed to `web.rs::handle_pty_ws` — broadcast already carries pre-filtered bytes. - No changes needed to `ScrollbackBuffer` — data going into it is already clean. Dependencies: Step 1. #### Step 4: Optional integration coverage in `tests/integration/tests/pty.rs` Files: `tests/integration/tests/pty.rs` - Add a third test that: 1. Starts a PTY service running `/bin/sh`. 2. Sends `printf '\\033[24;80R\\n'` over the WebSocket. 3. Collects WebSocket output for ~2 seconds. 4. Asserts the collected bytes do NOT contain the byte sequence `\x1b[24;80R`. - Caveats: this synthesizes a CPR byte stream rather than triggering a real DSR/CPR exchange. If flaky in CI, drop and rely on unit tests. Dependencies: Step 3. ### Acceptance Criteria - [ ] New module `crates/hero_proc_server/src/supervisor/cpr_filter.rs` with `CprFilter::new()` and `CprFilter::filter(&mut self, &[u8]) -> Vec<u8>`. - [ ] `pub mod cpr_filter;` added to `supervisor/mod.rs`. - [ ] All unit tests in Step 2 pass. - [ ] `run_job_pty` instantiates one `CprFilter` per job and routes every PTY chunk through it before broadcast, scrollback, and log-batcher writes. - [ ] xterm.js no longer sees `^[[24;80R` artifacts when a process emits a CPR reply. - [ ] Non-CPR escape sequences still render correctly (no regressions in colors, cursor moves, alt-screen). - [ ] Scrollback replay contains no CPR bytes. - [ ] No new external dependencies added to `crates/hero_proc_server/Cargo.toml`. - [ ] `cargo clippy -p hero_proc_server --all-targets -- -D warnings` clean. ### Notes - The filter must NOT generalize to "strip any CSI ending in R". Keep the `<digits>;<digits>` shape strict. - The filter must clone `pending` into the output stream when the sequence turns out NOT to be a CPR (e.g. `\x1b[31m` color). Do not lose those bytes — that would corrupt SGR. - Per the issue's "Out of scope": do not touch the hero_router-side DSR interception, and do not add a general ANSI sanitizer. - Sequencing: Steps 1+2 first; Step 3 depends on Step 1; Step 4 depends on Step 3.
Author
Owner

Test Results

CprFilter unit tests (cargo test -p hero_proc_server --lib supervisor::cpr_filter)

  • Total: 10
  • Passed: 10
  • Failed: 0

All ten unit tests cover: full CPR in one chunk, CPR split across two chunks, every byte-boundary split, non-CPR CSI passthrough, lone trailing ESC, incomplete CSI then invalid terminator, back-to-back CPR, mixed CPR + SGR, empty chunk, normal-only chunk.

PTY integration tests (cargo test -p hero_proc_integration_tests --test pty)

  • Total: 3
  • Passed: 3
  • Failed: 0

Tests run: test_pty_service_attach, test_pty_cpr_reply_is_stripped (new), test_pty_reconnect_sees_scrollback. The new test sends printf 'BEGIN\033[24;80REND\n' over the PTY WebSocket and asserts the literal \x1b[24;80R byte sequence does not appear in the stream while BEGIN and END markers do — confirming the filter strips CPR end-to-end through run_job_pty.

Compile check (cargo build -p hero_proc_server)

  • Status: passed

Clippy (cargo clippy -p hero_proc_server --all-targets -- -D warnings)

  • Status: one pre-existing warning, unrelated to this change

The clippy run flagged crates/hero_proc_server/src/supervisor/executor.rs:126:

let ai_cfg = job.spec.ai_config.clone().unwrap_or_else(AiConfig::default);

This line was introduced in commit 9813c96e (feat: add AI interpreter for in-process model calls via hero_aibroker) and is outside the scope of this fix. No new clippy warnings come from the CprFilter change. The acceptance criterion "no clippy warnings introduced by this change" is met; cleaning up the pre-existing one belongs in a separate commit.

## Test Results ### CprFilter unit tests (`cargo test -p hero_proc_server --lib supervisor::cpr_filter`) - Total: 10 - Passed: 10 - Failed: 0 All ten unit tests cover: full CPR in one chunk, CPR split across two chunks, every byte-boundary split, non-CPR CSI passthrough, lone trailing ESC, incomplete CSI then invalid terminator, back-to-back CPR, mixed CPR + SGR, empty chunk, normal-only chunk. ### PTY integration tests (`cargo test -p hero_proc_integration_tests --test pty`) - Total: 3 - Passed: 3 - Failed: 0 Tests run: `test_pty_service_attach`, `test_pty_cpr_reply_is_stripped` (new), `test_pty_reconnect_sees_scrollback`. The new test sends `printf 'BEGIN\033[24;80REND\n'` over the PTY WebSocket and asserts the literal `\x1b[24;80R` byte sequence does not appear in the stream while `BEGIN` and `END` markers do — confirming the filter strips CPR end-to-end through `run_job_pty`. ### Compile check (`cargo build -p hero_proc_server`) - Status: passed ### Clippy (`cargo clippy -p hero_proc_server --all-targets -- -D warnings`) - Status: one pre-existing warning, unrelated to this change The clippy run flagged `crates/hero_proc_server/src/supervisor/executor.rs:126`: ``` let ai_cfg = job.spec.ai_config.clone().unwrap_or_else(AiConfig::default); ``` This line was introduced in commit `9813c96e` (`feat: add AI interpreter for in-process model calls via hero_aibroker`) and is outside the scope of this fix. No new clippy warnings come from the CprFilter change. The acceptance criterion "no clippy warnings introduced by this change" is met; cleaning up the pre-existing one belongs in a separate commit.
Author
Owner

Implementation Summary

Changes

New file:

  • crates/hero_proc_server/src/supervisor/cpr_filter.rsCprFilter with a 3-state byte-by-byte machine (Normal / EscSeen / CsiBody) that strips CPR replies of the form \x1b[<digits>;<digits>R and lets every other byte (including non-CPR CSI sequences) through. Includes a fast path that bypasses the per-byte loop when the chunk has no \x1b and no buffered state. Ten unit tests cover the spec's required cases plus a few edge cases.

Modified files:

  • crates/hero_proc_server/src/supervisor/mod.rs — added pub mod cpr_filter;.
  • crates/hero_proc_server/src/supervisor/executor.rsrun_job_pty's reader closure now owns one CprFilter and pipes every PTY chunk through it before broadcast, scrollback, and the log batcher. When the filter consumes a full chunk (e.g. a buffered split CPR), the loop continues without emitting anything downstream.
  • tests/integration/tests/pty.rs — new test_pty_cpr_reply_is_stripped end-to-end test that drives a real /bin/sh PTY and asserts the CPR byte sequence does not survive into the WebSocket stream.

How the filter works

  • One instance per PTY job, owned by the spawn_blocking reader. It survives the whole PTY lifetime, so a CPR reply split across two 4096-byte reads is still recognized and stripped.
  • The state machine accumulates \x1b[<digits>;<digits> into a private buffer. On R it validates the buffered body matches <digits>;<digits> exactly and drops the whole sequence. Anything else (lone \x1b, \x1b[?1034h, \x1b[31m, \x1b[24;80X, etc.) flushes the buffered prefix verbatim back into the output.
  • Broadcast, scrollback, and log writes all see the same filtered bytes, so the WebSocket replay on reconnect is also CPR-free.

Test results

  • Unit tests: 10/10 pass.
  • Integration tests: all 3 PTY tests pass (the existing two plus the new CPR test). No regressions.
  • Clippy: no new warnings; one pre-existing warning at executor.rs:126 is from commit 9813c96e and is out of scope.

Acceptance criteria

  • New module crates/hero_proc_server/src/supervisor/cpr_filter.rs with CprFilter::new() and CprFilter::filter(&mut self, &[u8]) -> Vec<u8>.
  • pub mod cpr_filter; added to supervisor/mod.rs.
  • All unit tests in the spec pass.
  • run_job_pty instantiates one CprFilter per job and routes every PTY chunk through it before broadcast, scrollback, and log-batcher writes.
  • CPR sequence does not survive into the WebSocket stream (verified by new integration test).
  • Non-CPR escape sequences still pass through (\x1b[?1034h and \x1b[31m confirmed by unit tests; integration output shows \x1b[?1034h arriving intact).
  • Scrollback replay contains no CPR bytes (existing reconnect test still passes; same filtered byte stream feeds scrollback).
  • No new external dependencies in crates/hero_proc_server/Cargo.toml.

Out of scope (per the issue)

  • Per-job opt-out flag — not implemented; filter is unconditional. Easy to add later if a consumer needs raw CPR.
  • General ANSI sanitization — only CPR is stripped.
  • The hero_router-side DSR interception is left in place as a belt-and-suspenders.
## Implementation Summary ### Changes **New file:** - `crates/hero_proc_server/src/supervisor/cpr_filter.rs` — `CprFilter` with a 3-state byte-by-byte machine (`Normal` / `EscSeen` / `CsiBody`) that strips CPR replies of the form `\x1b[<digits>;<digits>R` and lets every other byte (including non-CPR CSI sequences) through. Includes a fast path that bypasses the per-byte loop when the chunk has no `\x1b` and no buffered state. Ten unit tests cover the spec's required cases plus a few edge cases. **Modified files:** - `crates/hero_proc_server/src/supervisor/mod.rs` — added `pub mod cpr_filter;`. - `crates/hero_proc_server/src/supervisor/executor.rs` — `run_job_pty`'s reader closure now owns one `CprFilter` and pipes every PTY chunk through it before broadcast, scrollback, and the log batcher. When the filter consumes a full chunk (e.g. a buffered split CPR), the loop continues without emitting anything downstream. - `tests/integration/tests/pty.rs` — new `test_pty_cpr_reply_is_stripped` end-to-end test that drives a real `/bin/sh` PTY and asserts the CPR byte sequence does not survive into the WebSocket stream. ### How the filter works - One instance per PTY job, owned by the `spawn_blocking` reader. It survives the whole PTY lifetime, so a CPR reply split across two 4096-byte reads is still recognized and stripped. - The state machine accumulates `\x1b[<digits>;<digits>` into a private buffer. On `R` it validates the buffered body matches `<digits>;<digits>` exactly and drops the whole sequence. Anything else (lone `\x1b`, `\x1b[?1034h`, `\x1b[31m`, `\x1b[24;80X`, etc.) flushes the buffered prefix verbatim back into the output. - Broadcast, scrollback, and log writes all see the same filtered bytes, so the WebSocket replay on reconnect is also CPR-free. ### Test results - Unit tests: 10/10 pass. - Integration tests: all 3 PTY tests pass (the existing two plus the new CPR test). No regressions. - Clippy: no new warnings; one pre-existing warning at `executor.rs:126` is from commit `9813c96e` and is out of scope. ### Acceptance criteria - [x] New module `crates/hero_proc_server/src/supervisor/cpr_filter.rs` with `CprFilter::new()` and `CprFilter::filter(&mut self, &[u8]) -> Vec<u8>`. - [x] `pub mod cpr_filter;` added to `supervisor/mod.rs`. - [x] All unit tests in the spec pass. - [x] `run_job_pty` instantiates one `CprFilter` per job and routes every PTY chunk through it before broadcast, scrollback, and log-batcher writes. - [x] CPR sequence does not survive into the WebSocket stream (verified by new integration test). - [x] Non-CPR escape sequences still pass through (`\x1b[?1034h` and `\x1b[31m` confirmed by unit tests; integration output shows `\x1b[?1034h` arriving intact). - [x] Scrollback replay contains no CPR bytes (existing reconnect test still passes; same filtered byte stream feeds scrollback). - [x] No new external dependencies in `crates/hero_proc_server/Cargo.toml`. ### Out of scope (per the issue) - Per-job opt-out flag — not implemented; filter is unconditional. Easy to add later if a consumer needs raw CPR. - General ANSI sanitization — only CPR is stripped. - The `hero_router`-side DSR interception is left in place as a belt-and-suspenders.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
lhumina_code/hero_proc#55
No description provided.