Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions internal/tmux/attach_utf8_argv_lint_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package tmux

import (
"go/ast"
"go/parser"
"go/token"
"path/filepath"
"strings"
"testing"
)

// TestTmuxAttachSpawnsForceUTF8 guards every production attach path against
// inheriting a non-UTF-8 locale from launchd, systemd, or another supervisor.
func TestTmuxAttachSpawnsForceUTF8(t *testing.T) {
root := moduleRoot(t)

var violations []string
err := walkGoFiles(root, func(path string) error {
if strings.HasSuffix(filepath.Base(path), "_test.go") {
return nil
}
rel, _ := filepath.Rel(root, path)
rel = filepath.ToSlash(rel)
// This package is a test-only multi-client fixture, not an agent-deck
// attach surface. Its explicit -S socket is controlled by each test.
if rel == "internal/testutil/multiclienttmux/multiclienttmux.go" {
return nil
}

fset := token.NewFileSet()
f, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution)
if err != nil {
return nil
}

ast.Inspect(f, func(n ast.Node) bool {
call, ok := n.(*ast.CallExpr)
if !ok {
return true
}

uIdx, attachIdx := -1, -1
for i, arg := range call.Args {
value, ok := stringLiteral(arg)
if !ok {
continue
}
switch value {
case "-u":
uIdx = i
case "attach-session":
attachIdx = i
}
}

if attachIdx == -1 || (uIdx != -1 && uIdx < attachIdx) {
return true
}

violations = append(violations,
rel+":"+itoa(fset.Position(call.Pos()).Line))
return true
})
return nil
})
if err != nil {
t.Fatalf("walk: %v", err)
}

if len(violations) > 0 {
t.Errorf("%d tmux attach spawn(s) do not force UTF-8; pass the global `-u` "+
"flag before `attach-session`:\n %s",
len(violations), strings.Join(violations, "\n "))
}
}
6 changes: 3 additions & 3 deletions internal/tmux/control_mode_argv_lint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ package tmux
// The second failure is the reason this lint scans argv shape rather than
// trusting the reaper to be careful: scripts/reap-stale-tmux.sh now matches by
// socket path only, but nothing this repo spawns should be ambiguous in the
// first place. Pass the command you actually want — `attach-session -t <target>`
// for a client bound to a session.
// first place. Pass `-u attach-session -t <target>` for a UTF-8 client bound
// to the session.

import (
"go/ast"
Expand Down Expand Up @@ -79,7 +79,7 @@ func TestNoBareControlModeTmuxSpawn(t *testing.T) {
t.Errorf("%d bare `tmux -C` spawn site(s) — a control-mode client with no "+
"command implicitly creates a session (leaking a pty), and its argv is "+
"indistinguishable from the server it may auto-start. Pass an explicit "+
"command, e.g. `\"-C\", \"attach-session\", \"-t\", target`:\n %s",
"command, e.g. `\"-C\", \"-u\", \"attach-session\", \"-t\", target`:\n %s",
len(violations), strings.Join(violations, "\n "))
}
}
8 changes: 5 additions & 3 deletions internal/tmux/controlpipe.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,11 @@ const (
controlPipeEOFExitGrace = 200 * time.Millisecond
)

// ControlPipe wraps a persistent `tmux -C attach-session -t <name>` process.
// ControlPipe wraps a persistent `tmux -C -u attach-session -t <name>` process.
// It provides event-driven output detection via %output events and
// zero-subprocess command execution through the stdin/stdout pipe.
// It is deliberately headless: it never opens /dev/tty, so detached callers
// without a controlling terminal (the #1114 failure mode) are supported.
type ControlPipe struct {
sessionName string
socketName string // tmux -L value; "" means user's default server
Expand Down Expand Up @@ -110,7 +112,7 @@ func NewControlPipe(sessionName, socketName string) (*ControlPipe, error) {
}

func newControlPipeOnce(sessionName, socketName string) (*ControlPipe, error) {
cmd := tmuxExec(socketName, "-C", "attach-session", "-t", sessionName)
cmd := tmuxExec(socketName, "-C", "-u", "attach-session", "-t", sessionName)
// Put in own process group so we can kill the entire group on shutdown
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}

Expand Down Expand Up @@ -361,7 +363,7 @@ func (cp *ControlPipe) Done() <-chan struct{} {

// Close shuts down the control mode pipe.
//
// Teardown is staged: (1) close stdin so the `tmux -C attach-session`
// Teardown is staged: (1) close stdin so the `tmux -C -u attach-session`
// child sees EOF and orderly-detaches via the control protocol's %exit
// path; (2) wait up to controlPipeEOFExitGrace (200ms) for that to
// complete — the vast majority of cases settle in 1-4ms; (3) only on
Expand Down
12 changes: 7 additions & 5 deletions internal/tmux/keysender.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ const (
// to #1096). #1096 added 15ms rune batching, but at realistic typing speeds
// (>15ms between keys) every keystroke still triggers its own fork+exec — on
// macOS each one costs 10-50ms, so the user feels per-keystroke lag despite
// the batch window. KeySender opens one `tmux -C attach-session` subprocess at
// the batch window. KeySender opens one `tmux -C -u attach-session` subprocess at
// the start of an insert-mode session and streams send-keys commands over
// stdin for the lifetime of that mode, dropping per-call dispatch to a stdin
// write (<1ms regardless of platform).
Expand All @@ -43,9 +43,11 @@ type KeySender interface {
}

// localKeySender is the in-process KeySender backed by a long-running
// `tmux -L <socket> -C attach-session -t <target>` subprocess. Each Send
// `tmux -L <socket> -C -u attach-session -t <target>` subprocess. Each Send
// writes one command line to its stdin; tmux executes commands in-server
// without spawning new clients.
// It is deliberately headless: its stdin/stdout are pipes and it never opens
// /dev/tty, so detached callers without a controlling terminal (#1114) work.
type localKeySender struct {
target string
cmd *exec.Cmd
Expand All @@ -63,7 +65,7 @@ type localKeySender struct {
//
// The client attaches EXPLICITLY to `target`:
//
// tmux [-L <socket>] -C attach-session -t <target>
// tmux [-L <socket>] -C -u attach-session -t <target>
//
// The explicit `attach-session` is load-bearing twice over, and both reasons
// are incident findings — do not "simplify" it back to a bare `tmux -C`:
Expand Down Expand Up @@ -98,7 +100,7 @@ func OpenKeySender(socket, target string) (KeySender, error) {
// <socket>` selector, and the lint test in tmux_exec_lint_test.go
// enforces this. Plain `exec.Command("tmux", ...)` would silently
// defeat socket isolation when the user has opted in (#687).
cmd := tmuxExec(socket, "-C", "attach-session", "-t", target)
cmd := tmuxExec(socket, "-C", "-u", "attach-session", "-t", target)
// Own process group so Close can take down the whole subtree, matching
// ControlPipe. Without it a wedged child's descendants outlive Close.
cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
Expand All @@ -114,7 +116,7 @@ func OpenKeySender(socket, target string) (KeySender, error) {
if err := cmd.Start(); err != nil {
_ = stdin.Close()
_ = stdout.Close()
return nil, fmt.Errorf("keysender: start tmux -C attach-session: %w", err)
return nil, fmt.Errorf("keysender: start tmux -C -u attach-session: %w", err)
}
k := &localKeySender{target: target, cmd: cmd, stdin: stdin}

Expand Down
10 changes: 8 additions & 2 deletions internal/tmux/pty.go
Original file line number Diff line number Diff line change
Expand Up @@ -728,13 +728,19 @@ func (s *Session) StreamOutput(ctx context.Context, w io.Writer) error {
// when isolation is configured, and byte-identical plain argv when it is
// not. Keeping these as named methods gives the regression lint a stable
// target to assert argv shape against without spawning PTYs.
//
// These are interactive clients: AttachWithOptions and AttachReadOnly require
// stdin to be a real terminal so they can enter raw mode. From a detached
// setsid process like the hook in #1114, they return "failed to set raw mode".
// StartAttachPTY gives attachCmd's tmux child its own controlling PTY, while
// attachReadOnlyCmd uses the caller's terminal directly.

func (s *Session) attachCmd(ctx context.Context) *exec.Cmd {
return s.tmuxCmdContext(ctx, "attach-session", "-t", s.Name)
return s.tmuxCmdContext(ctx, "-u", "attach-session", "-t", s.Name)
}

func (s *Session) attachReadOnlyCmd(ctx context.Context) *exec.Cmd {
return s.tmuxCmdContext(ctx, "attach-session", "-r", "-t", s.Name)
return s.tmuxCmdContext(ctx, "-u", "attach-session", "-r", "-t", s.Name)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

func (s *Session) resizeCmd(cols, rows int) *exec.Cmd {
Expand Down
13 changes: 6 additions & 7 deletions internal/tmux/pty_socket_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,20 @@ import (
func TestSession_AttachCmd_WithSocket_PrependsDashL(t *testing.T) {
s := &Session{Name: "agentdeck_iso_abc", SocketName: "agentdeck"}
cmd := s.attachCmd(context.Background())
wantArgs := []string{"tmux", "-L", "agentdeck", "attach-session", "-t", s.Name}
wantArgs := []string{"tmux", "-L", "agentdeck", "-u", "attach-session", "-t", s.Name}
if !reflect.DeepEqual(cmd.Args, wantArgs) {
t.Fatalf("attach must route through tmuxCmdContext so -L lands before subcommand\n got: %v\n want: %v", cmd.Args, wantArgs)
}
}

// TestSession_AttachCmd_EmptySocket_NoDashL: opt-in contract. No config =
// no -L, byte-identical to pre-v1.7.50. A regression here breaks every user
// who has not enabled socket isolation.
// TestSession_AttachCmd_EmptySocket_NoDashL: opt-in contract. No config means
// no -L; the required UTF-8 flag is independent of socket isolation.
func TestSession_AttachCmd_EmptySocket_NoDashL(t *testing.T) {
s := &Session{Name: "agentdeck_default_abc"}
cmd := s.attachCmd(context.Background())
wantArgs := []string{"tmux", "attach-session", "-t", s.Name}
wantArgs := []string{"tmux", "-u", "attach-session", "-t", s.Name}
if !reflect.DeepEqual(cmd.Args, wantArgs) {
t.Fatalf("empty SocketName must produce plain attach argv\n got: %v\n want: %v", cmd.Args, wantArgs)
t.Fatalf("empty SocketName must produce attach argv without -L\n got: %v\n want: %v", cmd.Args, wantArgs)
}
}

Expand All @@ -51,7 +50,7 @@ func TestSession_AttachCmd_EmptySocket_NoDashL(t *testing.T) {
func TestSession_AttachReadOnlyCmd_WithSocket_PrependsDashL(t *testing.T) {
s := &Session{Name: "agentdeck_ro_abc", SocketName: "agentdeck"}
cmd := s.attachReadOnlyCmd(context.Background())
wantArgs := []string{"tmux", "-L", "agentdeck", "attach-session", "-r", "-t", s.Name}
wantArgs := []string{"tmux", "-L", "agentdeck", "-u", "attach-session", "-r", "-t", s.Name}
if !reflect.DeepEqual(cmd.Args, wantArgs) {
t.Fatalf("read-only attach must include -L <socket>\n got: %v\n want: %v", cmd.Args, wantArgs)
}
Expand Down