diff --git a/internal/session/ssh.go b/internal/session/ssh.go index 15411d02..add21242 100644 --- a/internal/session/ssh.go +++ b/internal/session/ssh.go @@ -14,11 +14,14 @@ import ( "syscall" "time" + "github.com/asheshgoplani/agent-deck/internal/termreply" "github.com/asheshgoplani/agent-deck/internal/tmux" "github.com/creack/pty" "golang.org/x/term" ) +const sshAttachReplyQuarantine = 2 * time.Second + // sshControlDir is the directory for SSH ControlMaster sockets. const sshControlDir = "/tmp/agent-deck-ssh" @@ -206,6 +209,7 @@ func (r *SSHRunner) Attach(sessionID string) error { case <-outputDone: case <-time.After(50 * time.Millisecond): } + termreply.QuarantineFor(sshAttachReplyQuarantine) // Reset terminal styles that may have leaked from the remote session. _, _ = os.Stdout.WriteString("\x1b]8;;\x1b\\\x1b[0m\x1b[24m\x1b[39m\x1b[49m") diff --git a/internal/termreply/filter.go b/internal/termreply/filter.go new file mode 100644 index 00000000..3fbc26e0 --- /dev/null +++ b/internal/termreply/filter.go @@ -0,0 +1,211 @@ +package termreply + +const ( + escapeByte = 0x1b + bellByte = 0x07 + + controlSequenceIntroducerByte = '[' + singleShiftThreeByte = 'O' + + operatingSystemCommandByte = ']' + deviceControlStringByte = 'P' + applicationProgramCommandByte = '_' + privacyMessageByte = '^' + startOfStringByte = 'X' + + stringTerminatorByte = '\\' + + csiFinalArrowUpByte = 'A' + csiFinalArrowDownByte = 'B' + csiFinalArrowRightByte = 'C' + csiFinalArrowLeftByte = 'D' + csiFinalEndByte = 'F' + csiFinalHomeByte = 'H' + csiFinalBacktabByte = 'Z' + csiFinalTildeByte = '~' + csiFinalKittyKeyByte = 'u' +) + +type filterMode uint8 + +const ( + filterModeIdle filterMode = iota + filterModeDiscardEscapeString + filterModeCollectCSI + filterModeCollectSS3 +) + +// Filter strips terminal-generated control replies from a byte stream while +// preserving ordinary keyboard input. It is stateful so replies split across +// reads are discarded without relying on terminal-specific payload strings. +type Filter struct { + mode filterMode + pendingEsc bool + escapeSeenInDiscard bool + sequenceBuf []byte +} + +// Active reports whether the filter is carrying parser state across read boundaries. +func (f *Filter) Active() bool { + return f.pendingEsc || f.mode != filterModeIdle || len(f.sequenceBuf) > 0 +} + +func isEscapeStringIntroducer(b byte) bool { + switch b { + case operatingSystemCommandByte, + deviceControlStringByte, + applicationProgramCommandByte, + privacyMessageByte, + startOfStringByte: + return true + default: + return false + } +} + +func isSequenceFinalByte(b byte) bool { + return b >= 0x40 && b <= 0x7e +} + +func isKeyboardCSIFinalByte(b byte) bool { + switch b { + case csiFinalArrowUpByte, + csiFinalArrowDownByte, + csiFinalArrowRightByte, + csiFinalArrowLeftByte, + csiFinalEndByte, + csiFinalHomeByte, + csiFinalBacktabByte, + csiFinalTildeByte, + csiFinalKittyKeyByte: + return true + default: + return false + } +} + +func flushSequence(out []byte, seq []byte) []byte { + return append(out, seq...) +} + +func (f *Filter) beginSequence(mode filterMode, prefix ...byte) { + f.mode = mode + f.sequenceBuf = append(f.sequenceBuf[:0], prefix...) +} + +func (f *Filter) resetSequenceState() { + f.mode = filterModeIdle + f.sequenceBuf = f.sequenceBuf[:0] + f.escapeSeenInDiscard = false +} + +// Consume filters a chunk of bytes. When armed is true, terminal-generated +// control replies are discarded. If a reply started in a previous chunk, it +// continues to be discarded until it terminates even if armed is now false. +// +// Terminal replies covered here: +// - escape-string families: OSC, DCS, APC, PM, SOS +// - CSI replies during the quarantine window, except for a small whitelist of +// keyboard-related CSI finals (arrows/home/end/backtab/~ keys/kitty CSI u) +// +// If final is true, any incomplete pending escape/CSI/SS3 sequence is flushed as +// literal input, while an incomplete discarded escape-string reply is dropped. +func (f *Filter) Consume(src []byte, armed bool, final bool) []byte { + out := make([]byte, 0, len(src)) + + for _, b := range src { + switch f.mode { + case filterModeDiscardEscapeString: + if f.escapeSeenInDiscard { + f.escapeSeenInDiscard = false + if b == stringTerminatorByte { + f.resetSequenceState() + continue + } + if b == escapeByte { + f.escapeSeenInDiscard = true + } + continue + } + + if b == bellByte { + f.resetSequenceState() + continue + } + if b == escapeByte { + f.escapeSeenInDiscard = true + } + continue + + case filterModeCollectCSI: + f.sequenceBuf = append(f.sequenceBuf, b) + if !isSequenceFinalByte(b) { + continue + } + + if armed && !isKeyboardCSIFinalByte(b) { + f.resetSequenceState() + continue + } + + out = flushSequence(out, f.sequenceBuf) + f.resetSequenceState() + continue + + case filterModeCollectSS3: + f.sequenceBuf = append(f.sequenceBuf, b) + if !isSequenceFinalByte(b) { + continue + } + + out = flushSequence(out, f.sequenceBuf) + f.resetSequenceState() + continue + } + + if f.pendingEsc { + f.pendingEsc = false + switch { + case armed && isEscapeStringIntroducer(b): + f.mode = filterModeDiscardEscapeString + continue + case b == controlSequenceIntroducerByte: + f.beginSequence(filterModeCollectCSI, escapeByte, controlSequenceIntroducerByte) + continue + case b == singleShiftThreeByte: + f.beginSequence(filterModeCollectSS3, escapeByte, singleShiftThreeByte) + continue + case b == escapeByte: + out = append(out, escapeByte) + f.pendingEsc = true + continue + default: + out = append(out, escapeByte, b) + continue + } + } + + if b == escapeByte { + f.pendingEsc = true + continue + } + + out = append(out, b) + } + + if final { + if f.pendingEsc { + out = append(out, escapeByte) + } + switch f.mode { + case filterModeCollectCSI, filterModeCollectSS3: + out = flushSequence(out, f.sequenceBuf) + case filterModeDiscardEscapeString: + // Drop incomplete escape-string replies on EOF. + } + f.pendingEsc = false + f.resetSequenceState() + } + + return out +} diff --git a/internal/termreply/filter_test.go b/internal/termreply/filter_test.go new file mode 100644 index 00000000..c5af0552 --- /dev/null +++ b/internal/termreply/filter_test.go @@ -0,0 +1,37 @@ +package termreply + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFilterDiscardsStringRepliesAcrossChunks(t *testing.T) { + var f Filter + + got := f.Consume([]byte("\x1b]11;rgb:d3d3/f5f5/f5f5"), true, false) + require.Empty(t, got) + require.True(t, f.Active()) + + got = f.Consume([]byte("\x07j"), true, false) + require.Equal(t, []byte("j"), got) + require.False(t, f.Active()) +} + +func TestFilterDiscardsGenericCSIReplies(t *testing.T) { + var f Filter + + got := f.Consume([]byte("\x1b[?1;2c"), true, false) + require.Empty(t, got) + require.False(t, f.Active()) +} + +func TestFilterPreservesKeyboardCSIAndSS3Input(t *testing.T) { + var f Filter + + require.Equal(t, []byte("\x1b[A"), f.Consume([]byte("\x1b[A"), true, false)) + require.False(t, f.Active()) + + require.Equal(t, []byte("\x1bOA"), f.Consume([]byte("\x1bOA"), true, false)) + require.False(t, f.Active()) +} diff --git a/internal/termreply/guard.go b/internal/termreply/guard.go new file mode 100644 index 00000000..05e1bbe4 --- /dev/null +++ b/internal/termreply/guard.go @@ -0,0 +1,36 @@ +package termreply + +import ( + "sync/atomic" + "time" +) + +var quarantineUntilUnixNano atomic.Int64 + +// QuarantineFor drops terminal reply traffic until the later of the existing +// deadline or now+duration. +func QuarantineFor(duration time.Duration) { + if duration <= 0 { + return + } + target := time.Now().Add(duration).UnixNano() + for { + current := quarantineUntilUnixNano.Load() + if current >= target { + return + } + if quarantineUntilUnixNano.CompareAndSwap(current, target) { + return + } + } +} + +// Active reports whether terminal replies should currently be discarded. +func Active() bool { + return time.Now().UnixNano() < quarantineUntilUnixNano.Load() +} + +// Clear removes any active quarantine window. Intended for tests. +func Clear() { + quarantineUntilUnixNano.Store(0) +} diff --git a/internal/tmux/pty.go b/internal/tmux/pty.go index 06e2c6f7..bbdd7d5a 100644 --- a/internal/tmux/pty.go +++ b/internal/tmux/pty.go @@ -15,10 +15,15 @@ import ( "syscall" "time" + "github.com/asheshgoplani/agent-deck/internal/termreply" "github.com/creack/pty" + "golang.org/x/sys/unix" "golang.org/x/term" ) +const attachOutputDrainTimeout = 250 * time.Millisecond +const attachReplyQuarantine = 2 * time.Second + // IndexDetachKey returns the index of a control-key sequence in data, or -1 if // not found. detachByte is the raw ASCII byte (e.g. 0x11 for Ctrl+Q). // Handles three encodings: @@ -55,6 +60,23 @@ func IndexCtrlQ(data []byte) int { return IndexDetachKey(data, 17) } +func waitForAttachOutputDrain(outputDone <-chan struct{}, timeout time.Duration) (bool, time.Duration) { + start := time.Now() + timer := time.NewTimer(timeout) + defer timer.Stop() + + select { + case <-outputDone: + return true, time.Since(start) + case <-timer.C: + return false, time.Since(start) + } +} + +func flushDetachInput(fd int) error { + return unix.IoctlSetInt(fd, unix.TCFLSH, unix.TCIFLUSH) +} + // Attach attaches to the tmux session with full PTY support. // The configured detach key (default Ctrl+Q) will detach and return to the caller. // Pass an optional detachByte to override the default (0x11 / Ctrl+Q). @@ -156,9 +178,7 @@ func (s *Session) Attach(ctx context.Context, detachByte ...byte) error { // Channel for I/O errors (buffered to prevent goroutine leaks) ioErrors := make(chan error, 2) - // Timeout to ignore initial terminal control sequences (50ms) startTime := time.Now() - const controlSeqTimeout = 50 * time.Millisecond const terminalStyleReset = "\x1b]8;;\x1b\\\x1b[0m\x1b[24m\x1b[39m\x1b[49m" const clearScrollback = "\033[3J" outputDone := make(chan struct{}) @@ -184,6 +204,7 @@ func (s *Session) Attach(ctx context.Context, detachByte ...byte) error { go func() { defer wg.Done() buf := make([]byte, 32) + var replyFilter termreply.Filter for { n, err := os.Stdin.Read(buf) if err != nil { @@ -198,21 +219,21 @@ func (s *Session) Attach(ctx context.Context, detachByte ...byte) error { return } - // Discard initial terminal ESC sequences (within first 50ms). - // These are things like terminal capability queries sent on attach. - // Only drop bytes starting with ESC (0x1b). Non-ESC bytes - // (including Ctrl+C / 0x03, Ctrl+Z / 0x1a) are forwarded immediately. - if time.Since(startTime) < controlSeqTimeout && n > 0 && buf[0] == 0x1b { - continue + chunk := buf[:n] + if time.Since(startTime) < attachReplyQuarantine || replyFilter.Active() { + chunk = replyFilter.Consume(chunk, time.Since(startTime) < attachReplyQuarantine, false) + if len(chunk) == 0 { + continue + } } // Check for the detach key anywhere in the input chunk. // Some terminals coalesce reads, so detach must not require a single-byte read. // Handles raw byte, xterm modifyOtherKeys, and kitty CSI u encodings. - if idx := IndexDetachKey(buf[:n], detach); idx >= 0 { + if idx := IndexDetachKey(chunk, detach); idx >= 0 { // Forward any bytes before the detach key, then detach. if idx > 0 { - if _, err := ptmx.Write(buf[:idx]); err != nil { + if _, err := ptmx.Write(chunk[:idx]); err != nil { select { case ioErrors <- fmt.Errorf("PTY write error: %w", err): default: @@ -226,7 +247,7 @@ func (s *Session) Attach(ctx context.Context, detachByte ...byte) error { } // Forward other input to tmux PTY - if _, err := ptmx.Write(buf[:n]); err != nil { + if _, err := ptmx.Write(chunk); err != nil { // Report PTY write error select { case ioErrors <- fmt.Errorf("PTY write error: %w", err): @@ -245,6 +266,8 @@ func (s *Session) Attach(ctx context.Context, detachByte ...byte) error { cmdDone <- cmd.Wait() }() + didDetach := false + // Ensures we don't return to Bubble Tea while PTY output is still being written. // This avoids terminal style leakage (for example underline/hyperlink state) // from the attached client into the Agent Deck UI. @@ -255,9 +278,14 @@ func (s *Session) Attach(ctx context.Context, detachByte ...byte) error { signal.Reset(syscall.SIGINT) cancel() _ = ptmx.Close() - select { - case <-outputDone: - case <-time.After(20 * time.Millisecond): + _, _ = waitForAttachOutputDrain(outputDone, attachOutputDrainTimeout) + // Prompts can issue terminal capability/color queries as they redraw during + // detach. Kitty replies on stdin; if those queued bytes survive until Bubble Tea + // resumes, they can leak as literal fragments like terminal version strings or + // rgb payloads in the TUI. + if didDetach { + _ = flushDetachInput(int(os.Stdin.Fd())) + termreply.QuarantineFor(attachReplyQuarantine) } // Clear host terminal scrollback before returning to TUI. // The on-attach clear at the top of Attach() covers the "next attach" direction; @@ -272,6 +300,7 @@ func (s *Session) Attach(ctx context.Context, detachByte ...byte) error { select { case <-detachCh: // User pressed the detach key, detach gracefully + didDetach = true attachErr = nil case err := <-cmdDone: if err != nil { diff --git a/internal/ui/keyboard_compat.go b/internal/ui/keyboard_compat.go index 9953029d..45952028 100644 --- a/internal/ui/keyboard_compat.go +++ b/internal/ui/keyboard_compat.go @@ -23,6 +23,7 @@ import ( "io" "os" + "github.com/asheshgoplani/agent-deck/internal/termreply" tea "github.com/charmbracelet/bubbletea" ) @@ -245,10 +246,11 @@ func parseDecimalBytes(b []byte) int { // sequences in the byte stream and translates them into legacy byte sequences // that Bubble Tea can parse. All other bytes pass through unchanged. type csiuReader struct { - src io.Reader - outBuf []byte // pending translated bytes to emit - inBuf []byte // buffered input bytes not yet processed - err error // pending source error to return after draining buffers + src io.Reader + outBuf []byte // pending translated bytes to emit + inBuf []byte // buffered input bytes not yet processed + err error // pending source error to return after draining buffers + replyFilter termreply.Filter } // csiuFileReader wraps a *os.File and overrides Read with CSI u translation. @@ -306,7 +308,14 @@ func (c *csiuReader) Read(p []byte) (int, error) { tmp := make([]byte, len(p)) n, err := c.src.Read(tmp) if n > 0 { - c.inBuf = append(c.inBuf, tmp[:n]...) + chunk := tmp[:n] + if termreply.Active() || c.replyFilter.Active() { + chunk = c.replyFilter.Consume(chunk, termreply.Active(), false) + } + c.inBuf = append(c.inBuf, chunk...) + } + if err == io.EOF && (termreply.Active() || c.replyFilter.Active()) { + c.inBuf = append(c.inBuf, c.replyFilter.Consume(nil, termreply.Active(), true)...) } processed := c.translate(err == io.EOF) diff --git a/internal/ui/keyboard_compat_test.go b/internal/ui/keyboard_compat_test.go index d1fa82a0..2c343bb4 100644 --- a/internal/ui/keyboard_compat_test.go +++ b/internal/ui/keyboard_compat_test.go @@ -5,6 +5,9 @@ import ( "fmt" "io" "testing" + "time" + + "github.com/asheshgoplani/agent-deck/internal/termreply" ) type chunkedReader struct { @@ -519,6 +522,75 @@ func TestCSIuReaderBuffersSplitModifyOtherKeysSequence(t *testing.T) { } } +func TestCSIuReaderDropsTerminalRepliesDuringQuarantine(t *testing.T) { + t.Cleanup(termreply.Clear) + termreply.QuarantineFor(time.Second) + + tests := []struct { + name string + input string + }{ + { + name: "drops OSC color reply", + input: "\x1b]11;rgb:d3d3/f5f5/f5f5\x07", + }, + { + name: "drops DCS kitty version reply", + input: "\x1bP>|kitty(0.44.0)\x1b\\", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := NewCSIuReader(bytes.NewReader([]byte(tt.input))) + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("ReadAll error: %v", err) + } + if string(out) != "" { + t.Fatalf("expected terminal reply to be discarded, got %q", string(out)) + } + }) + } +} + +func TestCSIuReaderDropsSplitTerminalRepliesDuringQuarantine(t *testing.T) { + t.Cleanup(termreply.Clear) + termreply.QuarantineFor(time.Second) + + r := NewCSIuReader(&chunkedReader{ + chunks: [][]byte{ + []byte("\x1bP>|kitty"), + []byte("(0.44.0)\x1b\\"), + []byte("\x1b]11;rgb:d3d3/f5f5"), + []byte("/d3d3/f5f5\x07"), + []byte("j"), + }, + }) + + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("ReadAll error: %v", err) + } + if string(out) != "j" { + t.Fatalf("expected split terminal replies to be discarded, got %q", string(out)) + } +} + +func TestCSIuReaderPreservesNormalInputDuringQuarantine(t *testing.T) { + t.Cleanup(termreply.Clear) + termreply.QuarantineFor(time.Second) + + r := NewCSIuReader(bytes.NewReader([]byte("j\r"))) + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("ReadAll error: %v", err) + } + if string(out) != "j\r" { + t.Fatalf("expected normal input to survive quarantine, got %q", string(out)) + } +} + func TestCSIuReaderBuffersSplitStandardCSISequence(t *testing.T) { r := NewCSIuReader(&chunkedReader{ chunks: [][]byte{