Fixed a bug with shared sockets.

internal/control/coexistence_linux.go

@@ -5,9 +5,13 @@ package control
 
 import (
 	"fmt"
+	"io"
 	"os"
 	"path/filepath"
 	"slices"
+	"strconv"
+	"strings"
+	"syscall"
 )
 
 func (s *Server) endpoint_snapshot() []endpoint {
@@ -53,9 +57,9 @@ func (s *Server) private_path(data instance) string {
 	return ""
 }
 
-// Select a terminal only when it is an ancestor of the caller. Outside a
-// terminal instance, prefer TTY; a single same-user terminal is unambiguous.
-// Never silently send a command to an arbitrary terminal in a multi-reader session.
+// Ancestry identifies ordinary PTY children. Reattached Screen sessions and
+// detached helpers can lose that ancestry, so also use the caller's own X11
+// identity. Neither a single terminal nor a console is a safe default.
 func (s *Server) route_target(pid, uid int) (string, error) {
 	instances := s.instances()
 	var terminals, consoles []instance
@@ -83,17 +87,81 @@ func (s *Server) route_target(pid, uid int) (string, error) {
 			}
 		}
 	}
-	if len(consoles) == 1 {
-		return s.destination(consoles[0])
+	identity := process_identity(pid, uid)
+	return s.route_identity(identity, uid, terminals, consoles)
+}
+
+type processIdentity struct {
+	window, display string
+	console         bool
+}
+
+// Read only identity fields from the credentialed process, never a supplied PID
+// or environment in the request text. Environment is a routing hint, not an
+// authorization grant; the destination still checks the original SO_PEERCRED.
+func process_identity(pid, uid int) processIdentity {
+	var identity processIdentity
+	info, err := os.Stat(fmt.Sprintf("/proc/%d", pid))
+	if err != nil {
+		return identity
 	}
-	if len(consoles) == 0 && len(terminals) == 1 {
-		return s.destination(terminals[0])
+	if stat, ok := info.Sys().(*syscall.Stat_t); !ok || int(stat.Uid) != uid {
+		return identity
 	}
-	if len(consoles) == 0 && len(terminals) == 0 {
-		// The application applies its normal authorization to the shared owner.
-		return "", nil
+	if data, err := os.ReadFile(fmt.Sprintf("/proc/%d/stat", pid)); err == nil {
+		end := strings.LastIndexByte(string(data), ')')
+		if end >= 0 {
+			fields := strings.Fields(string(data[end+1:]))
+			if len(fields) > 4 {
+				tty, _ := strconv.ParseInt(fields[4], 10, 64)
+				device := uint32(tty)
+				major := (device >> 8) & 0xfff
+				minor := (device & 0xff) | ((device >> 12) & 0xfff00)
+				identity.console = major == 4 && minor >= 1 && minor <= 63
+			}
+		}
 	}
-	return "", fmt.Errorf("caller does not identify a unique reader; use its private socket")
+	file, err := os.Open(fmt.Sprintf("/proc/%d/environ", pid))
+	if err != nil {
+		return identity
+	}
+	defer file.Close()
+	data, err := io.ReadAll(io.LimitReader(file, 1<<20))
+	if err != nil {
+		return identity
+	}
+	for _, entry := range strings.Split(string(data), "\x00") {
+		key, value, _ := strings.Cut(entry, "=")
+		switch key {
+		case "WINDOWID":
+			identity.window = value
+		case "DISPLAY":
+			identity.display = value
+		}
+	}
+	return identity
+}
+
+func (s *Server) route_identity(identity processIdentity, uid int, terminals, consoles []instance) (string, error) {
+	var matches []instance
+	if identity.console {
+		// A real controlling VT takes precedence over inherited X11 variables.
+		matches = consoles
+	} else if identity.window != "" && identity.display != "" {
+		for _, data := range terminals {
+			if data.uid != uid || data.WindowID != identity.window {
+				continue
+			}
+			target := process_identity(data.PID, data.uid)
+			if target.display == identity.display {
+				matches = append(matches, data)
+			}
+		}
+	}
+	if len(matches) == 1 {
+		return s.destination(matches[0])
+	}
+	return "", fmt.Errorf("caller does not identify a unique reader; run inside its managed terminal or use its private socket")
 }
 
 func (s *Server) destination(data instance) (string, error) {

internal/control/coexistence_test.go

@@ -72,7 +72,7 @@ func child_command(mode string) *exec.Cmd {
 func run_reader_process(mode string) {
 	encoder := json.NewEncoder(os.Stdout)
 	directory := os.Getenv("FENRIR_CONTROL_DIRECTORY")
-	server, err := Open(context.Background(), Options{Directory: directory, RegistryDirectory: filepath.Join(directory, "registry"), TerminalMode: mode == "terminal"})
+	server, err := Open(context.Background(), Options{Directory: directory, RegistryDirectory: filepath.Join(directory, "registry"), TerminalMode: mode == "terminal", WindowID: os.Getenv("WINDOWID")})
 	if err != nil {
 		_ = encoder.Encode(processReply{Error: err.Error()})
 		return
@@ -239,7 +239,7 @@ func TestReadersCoexistInBothStartupOrdersAndSurviveOwnerExit(t *testing.T) {
 				privatePath := filepath.Join(options.Directory, fmt.Sprintf("fenrirscreenreader-%d.sock", reader.pid))
 				check_receipt(t, second.send(t, privatePath, "COMMAND SAY private"), reader.pid, "COMMAND SAY private")
 			}
-			// No descendant terminal: a legacy caller belongs to the TTY reader.
+			// An unrelated caller must not fall through to the console reader.
 			client := child_command("client")
 			client.Env = append(client.Env, "FENRIR_CONTROL_PATH="+mainPath, "FENRIR_CONTROL_TEXT=COMMAND SAY legacy")
 			data, err := client.Output()
@@ -250,7 +250,9 @@ func TestReadersCoexistInBothStartupOrdersAndSurviveOwnerExit(t *testing.T) {
 			if err := json.Unmarshal(data, &reply); err != nil {
 				t.Fatal(err)
 			}
-			check_receipt(t, reply, tty.pid, "COMMAND SAY legacy")
+			if !strings.Contains(reply.Response, "unique reader") {
+				t.Fatal("unidentified caller reached a reader", reply)
+			}
 			owner.stop(t)
 			wait_main_owner(t, observer, 2, owner.pid)
 			for _, reader := range []*readerProcess{tty, first, second} {
@@ -328,3 +330,89 @@ func TestTerminalReadersRejectAmbiguityAndRecoverAfterOwnerFailure(t *testing.T)
 		check_receipt(t, reader.send(t, mainPath, "COMMAND SAY recovered"), reader.pid, "COMMAND SAY recovered")
 	}
 }
+
+// Screen reattachment and Codex's app-server can leave the caller outside the
+// reader's descendant tree. The connecting process retains its X11 identity.
+func TestWindowRoutingWithoutReaderAncestor(t *testing.T) {
+	for _, consoleFirst := range []bool{true, false} {
+		name := "terminal-first"
+		if consoleFirst {
+			name = "console-first"
+		}
+		t.Run(name, func(t *testing.T) {
+			options := private_options(t)
+			observer := &Server{ctx: context.Background(), options: options}
+			mainPath := observer.main_path()
+			t.Setenv("DISPLAY", ":fixture")
+			t.Setenv("WINDOWID", "42")
+			var terminal *readerProcess
+			if consoleFirst {
+				start_reader_process(t, options.Directory, "tty")
+			}
+			terminal = start_reader_process(t, options.Directory, "terminal")
+			if !consoleFirst {
+				start_reader_process(t, options.Directory, "tty")
+			}
+			// This client is a sibling, not a child, of the terminal reader.
+			text := "command application codex"
+			check_receipt(t, external_request(t, mainPath, text), terminal.pid, text)
+			t.Setenv("WINDOWID", "43")
+			second := start_reader_process(t, options.Directory, "terminal")
+			check_receipt(t, external_request(t, mainPath, text), second.pid, text)
+			t.Setenv("WINDOWID", "42")
+			check_receipt(t, external_request(t, mainPath, text), terminal.pid, text)
+			// Even a sole terminal on a different display is not a match.
+			t.Setenv("DISPLAY", ":other")
+			if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") {
+				t.Fatal("cross-display routing", reply)
+			}
+			t.Setenv("DISPLAY", ":fixture")
+			t.Setenv("WINDOWID", "unknown")
+			if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") {
+				t.Fatal("unknown window routed", reply)
+			}
+			t.Setenv("WINDOWID", "")
+			if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") {
+				t.Fatal("unidentified background caller routed", reply)
+			}
+			t.Setenv("WINDOWID", "42")
+			start_reader_process(t, options.Directory, "terminal")
+			if reply := external_request(t, mainPath, text); !strings.Contains(reply.Response, "unique reader") {
+				t.Fatal("duplicate window routed arbitrarily", reply)
+			}
+			// Discovery remains usable when routing is ambiguous.
+			if reply := external_request(t, mainPath, "ls"); !strings.Contains(reply.Response, "pid=") {
+				t.Fatal("discovery failed", reply)
+			}
+		})
+	}
+}
+
+func TestConsoleIdentityAndUnknownCallers(t *testing.T) {
+	options := private_options(t)
+	console := instance{PID: os.Getpid(), uid: os.Geteuid(), ScreenDriver: "vcsaDriver"}
+	server := &Server{ctx: context.Background(), options: options}
+	for _, trial := range []struct {
+		name     string
+		identity processIdentity
+		consoles []instance
+		valid    bool
+	}{
+		{"console", processIdentity{console: true}, []instance{console}, true},
+		{"console ignores stale window", processIdentity{console: true, window: "42", display: ":fixture"}, []instance{console}, true},
+		{"multiple consoles", processIdentity{console: true}, []instance{console, console}, false},
+		{"no console", processIdentity{console: true}, nil, false},
+		{"background", processIdentity{}, []instance{console}, false},
+		{"unknown window", processIdentity{window: "42", display: ":fixture"}, []instance{console}, false},
+	} {
+		t.Run(trial.name, func(t *testing.T) {
+			_, err := server.route_identity(trial.identity, os.Geteuid(), nil, trial.consoles)
+			if (err == nil) != trial.valid {
+				t.Fatal("unexpected routing", err)
+			}
+		})
+	}
+	if got := process_identity(os.Getpid(), os.Geteuid()+1); got != (processIdentity{}) {
+		t.Fatal("read identity of a different UID", got)
+	}
+}

internal/control/server_test.go

@@ -262,7 +262,7 @@ func TestStaleSocketReplacementAndPrivateInstanceSelection(t *testing.T) {
 		t.Fatal("registered ancestor instance not selected", target, err)
 	}
 	target, err = server.route_target(pid, os.Geteuid()+1)
-	if err != nil || target != "" {
+	if err == nil || target != "" {
 		t.Fatal("forwarded to another user's instance", target, err)
 	}
 }