fix(darwin): validate process ids and stabilize tests
- commit
- 05ece642a694242afdcabd249ac6a2b0cfe43718
- author
- brice · · 2026-07-17 06:32 UTC
- parent
- f569d8b
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
3 files · +30 -17
M internal/platform/process_darwin_arm64.go +3 -2
| @@ -70,14 +70,15 @@ func (p *ProcessInspector) processRecords() ([]processRecord, error) { | |||
| 70 | 70 | ||
| 71 | 71 | // ProcessCwd returns the current working directory from proc_pidinfo. | |
| 72 | 72 | func ProcessCwd(pid int) (string, error) { | |
| 73 | - | if pid <= 0 { | |
| 73 | + | pid32, ok := darwinPID(pid) | |
| 74 | + | if !ok { | |
| 74 | 75 | return "", fmt.Errorf("process cwd: invalid pid %d", pid) | |
| 75 | 76 | } | |
| 76 | 77 | var info procVnodePathInfo | |
| 77 | 78 | n, _, errno := syscall.Syscall6( | |
| 78 | 79 | syscall.SYS_PROC_INFO, | |
| 79 | 80 | procInfoCallPIDInfo, | |
| 80 | - | uintptr(pid), | |
| 81 | + | uintptr(pid32), | |
| 81 | 82 | procPIDVnodePathInfo, | |
| 82 | 83 | 0, | |
| 83 | 84 | uintptr(unsafe.Pointer(&info)), | |
| @@ -70,14 +70,15 @@ func (p *ProcessInspector) processRecords() ([]processRecord, error) { | |||||
| 70 | 70 | ||||
| 71 | // ProcessCwd returns the current working directory from proc_pidinfo. | 71 | // ProcessCwd returns the current working directory from proc_pidinfo. | ||
| 72 | func ProcessCwd(pid int) (string, error) { | 72 | func ProcessCwd(pid int) (string, error) { | ||
| 73 | - | if pid <= 0 { | 73 | + | pid32, ok := darwinPID(pid) |
| 74 | + | if !ok { | |||
| 74 | return "", fmt.Errorf("process cwd: invalid pid %d", pid) | 75 | return "", fmt.Errorf("process cwd: invalid pid %d", pid) | ||
| 75 | } | 76 | } | ||
| 76 | var info procVnodePathInfo | 77 | var info procVnodePathInfo | ||
| 77 | n, _, errno := syscall.Syscall6( | 78 | n, _, errno := syscall.Syscall6( | ||
| 78 | syscall.SYS_PROC_INFO, | 79 | syscall.SYS_PROC_INFO, | ||
| 79 | procInfoCallPIDInfo, | 80 | procInfoCallPIDInfo, | ||
| 80 | - | uintptr(pid), | 81 | + | uintptr(pid32), |
| 81 | procPIDVnodePathInfo, | 82 | procPIDVnodePathInfo, | ||
| 82 | 0, | 83 | 0, | ||
| 83 | uintptr(unsafe.Pointer(&info)), | 84 | uintptr(unsafe.Pointer(&info)), | ||
M internal/platform/process_darwin_arm64_test.go +17 -5
| @@ -73,26 +73,38 @@ func TestDarwinProcessInspectorCurrentProcess(t *testing.T) { | |||
| 73 | 73 | t.Fatalf("Argv(%d) = %#v; want argv[0] %q", pid, argv, os.Args[0]) | |
| 74 | 74 | } | |
| 75 | 75 | ||
| 76 | - | cmd := exec.Command("/bin/sleep", "5") | |
| 76 | + | stdinReader, stdinWriter, err := os.Pipe() | |
| 77 | + | if err != nil { | |
| 78 | + | t.Fatal(err) | |
| 79 | + | } | |
| 80 | + | cmd := exec.Command("/bin/cat") | |
| 81 | + | cmd.Stdin = stdinReader | |
| 77 | 82 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | |
| 78 | 83 | if err := cmd.Start(); err != nil { | |
| 84 | + | _ = stdinReader.Close() | |
| 85 | + | _ = stdinWriter.Close() | |
| 79 | 86 | t.Fatal(err) | |
| 80 | 87 | } | |
| 81 | - | defer func() { | |
| 88 | + | t.Cleanup(func() { | |
| 89 | + | _ = stdinReader.Close() | |
| 90 | + | _ = stdinWriter.Close() | |
| 82 | 91 | _ = cmd.Process.Kill() | |
| 83 | 92 | _ = cmd.Wait() | |
| 84 | - | }() | |
| 93 | + | }) | |
| 85 | 94 | groupArgv, err := inspector.GroupArgv(cmd.Process.Pid, -1) | |
| 86 | 95 | if err != nil { | |
| 87 | 96 | t.Fatalf("GroupArgv(child group): %v", err) | |
| 88 | 97 | } | |
| 89 | - | if len(groupArgv) != 2 || groupArgv[0] != "/bin/sleep" || groupArgv[1] != "5" { | |
| 90 | - | t.Fatalf("GroupArgv(child group) = %#v; want [/bin/sleep 5]", groupArgv) | |
| 98 | + | if len(groupArgv) != 1 || groupArgv[0] != "/bin/cat" { | |
| 99 | + | t.Fatalf("GroupArgv(child group) = %#v; want [/bin/cat]", groupArgv) | |
| 91 | 100 | } | |
| 92 | 101 | } | |
| 93 | 102 | ||
| 94 | 103 | func TestDarwinPIDRejectsUnrepresentableValues(t *testing.T) { | |
| 95 | 104 | for _, pid := range []int{0, -1, maxDarwinPID + 1} { | |
| 105 | + | if _, err := ProcessCwd(pid); err == nil { | |
| 106 | + | t.Errorf("ProcessCwd(%d) succeeded; want invalid PID error", pid) | |
| 107 | + | } | |
| 96 | 108 | if _, err := ProcessComm(pid); err == nil { | |
| 97 | 109 | t.Errorf("ProcessComm(%d) succeeded; want invalid PID error", pid) | |
| 98 | 110 | } | |
| @@ -73,26 +73,38 @@ func TestDarwinProcessInspectorCurrentProcess(t *testing.T) { | |||||
| 73 | t.Fatalf("Argv(%d) = %#v; want argv[0] %q", pid, argv, os.Args[0]) | 73 | t.Fatalf("Argv(%d) = %#v; want argv[0] %q", pid, argv, os.Args[0]) | ||
| 74 | } | 74 | } | ||
| 75 | 75 | ||||
| 76 | - | cmd := exec.Command("/bin/sleep", "5") | 76 | + | stdinReader, stdinWriter, err := os.Pipe() |
| 77 | + | if err != nil { | |||
| 78 | + | t.Fatal(err) | |||
| 79 | + | } | |||
| 80 | + | cmd := exec.Command("/bin/cat") | |||
| 81 | + | cmd.Stdin = stdinReader | |||
| 77 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | 82 | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | ||
| 78 | if err := cmd.Start(); err != nil { | 83 | if err := cmd.Start(); err != nil { | ||
| 84 | + | _ = stdinReader.Close() | |||
| 85 | + | _ = stdinWriter.Close() | |||
| 79 | t.Fatal(err) | 86 | t.Fatal(err) | ||
| 80 | } | 87 | } | ||
| 81 | - | defer func() { | 88 | + | t.Cleanup(func() { |
| 89 | + | _ = stdinReader.Close() | |||
| 90 | + | _ = stdinWriter.Close() | |||
| 82 | _ = cmd.Process.Kill() | 91 | _ = cmd.Process.Kill() | ||
| 83 | _ = cmd.Wait() | 92 | _ = cmd.Wait() | ||
| 84 | - | }() | 93 | + | }) |
| 85 | groupArgv, err := inspector.GroupArgv(cmd.Process.Pid, -1) | 94 | groupArgv, err := inspector.GroupArgv(cmd.Process.Pid, -1) | ||
| 86 | if err != nil { | 95 | if err != nil { | ||
| 87 | t.Fatalf("GroupArgv(child group): %v", err) | 96 | t.Fatalf("GroupArgv(child group): %v", err) | ||
| 88 | } | 97 | } | ||
| 89 | - | if len(groupArgv) != 2 || groupArgv[0] != "/bin/sleep" || groupArgv[1] != "5" { | 98 | + | if len(groupArgv) != 1 || groupArgv[0] != "/bin/cat" { |
| 90 | - | t.Fatalf("GroupArgv(child group) = %#v; want [/bin/sleep 5]", groupArgv) | 99 | + | t.Fatalf("GroupArgv(child group) = %#v; want [/bin/cat]", groupArgv) |
| 91 | } | 100 | } | ||
| 92 | } | 101 | } | ||
| 93 | 102 | ||||
| 94 | func TestDarwinPIDRejectsUnrepresentableValues(t *testing.T) { | 103 | func TestDarwinPIDRejectsUnrepresentableValues(t *testing.T) { | ||
| 95 | for _, pid := range []int{0, -1, maxDarwinPID + 1} { | 104 | for _, pid := range []int{0, -1, maxDarwinPID + 1} { | ||
| 105 | + | if _, err := ProcessCwd(pid); err == nil { | |||
| 106 | + | t.Errorf("ProcessCwd(%d) succeeded; want invalid PID error", pid) | |||
| 107 | + | } | |||
| 96 | if _, err := ProcessComm(pid); err == nil { | 108 | if _, err := ProcessComm(pid); err == nil { | ||
| 97 | t.Errorf("ProcessComm(%d) succeeded; want invalid PID error", pid) | 109 | t.Errorf("ProcessComm(%d) succeeded; want invalid PID error", pid) | ||
| 98 | } | 110 | } | ||
M internal/usecase/daemon/picker_test.go +10 -10
| @@ -857,13 +857,9 @@ func TestResumeStoppedAndSwitchInheritsTerminalEnv(t *testing.T) { | |||
| 857 | 857 | }, | |
| 858 | 858 | ).Twice() | |
| 859 | 859 | floating := newQuietPTY() | |
| 860 | - | floatingOpened := make(chan struct{}) | |
| 861 | 860 | f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool { | |
| 862 | 861 | return got != normalSize && got.Valid() | |
| 863 | - | })).RunAndReturn(func(context.Context, string, []string, []string, string, domain.Size) (ports.PTY, error) { | |
| 864 | - | close(floatingOpened) | |
| 865 | - | return floating, nil | |
| 866 | - | }).Once() | |
| 862 | + | })).Return(floating, nil).Once() | |
| 867 | 863 | d := newTestDaemon(t, f, stubClock{}) | |
| 868 | 864 | d.stopped["old"] = stoppedSession{name: "old", cwd: t.TempDir(), createdAt: 1} | |
| 869 | 865 | tr := portsmocks.NewMockTransport(t) | |
| @@ -881,11 +877,15 @@ func TestResumeStoppedAndSwitchInheritsTerminalEnv(t *testing.T) { | |||
| 881 | 877 | require.Contains(t, opens[1], "TERM=xterm-direct") | |
| 882 | 878 | require.Contains(t, opens[1], "COLORTERM=truecolor") | |
| 883 | 879 | require.Contains(t, opens[1], "TERM_PROGRAM=vev") | |
| 884 | - | select { | |
| 885 | - | case <-floatingOpened: | |
| 886 | - | case <-time.After(time.Second): | |
| 887 | - | t.Fatal("floating prewarm PTY was not opened") | |
| 888 | - | } | |
| 880 | + | require.Eventually(t, func() bool { | |
| 881 | + | tb := got.activeTab() | |
| 882 | + | if tb == nil { | |
| 883 | + | return false | |
| 884 | + | } | |
| 885 | + | tb.mu.Lock() | |
| 886 | + | defer tb.mu.Unlock() | |
| 887 | + | return tb.floating.pane != nil && tb.floating.pane.pty == floating | |
| 888 | + | }, time.Second, 5*time.Millisecond) | |
| 889 | 889 | _ = d.killSession(got, ports.ReasonSessionKilled, false) | |
| 890 | 890 | release1() | |
| 891 | 891 | release2() | |
| @@ -857,13 +857,9 @@ func TestResumeStoppedAndSwitchInheritsTerminalEnv(t *testing.T) { | |||||
| 857 | }, | 857 | }, | ||
| 858 | ).Twice() | 858 | ).Twice() | ||
| 859 | floating := newQuietPTY() | 859 | floating := newQuietPTY() | ||
| 860 | - | floatingOpened := make(chan struct{}) | |||
| 861 | f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool { | 860 | f.EXPECT().Open(mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.MatchedBy(func(got domain.Size) bool { | ||
| 862 | return got != normalSize && got.Valid() | 861 | return got != normalSize && got.Valid() | ||
| 863 | - | })).RunAndReturn(func(context.Context, string, []string, []string, string, domain.Size) (ports.PTY, error) { | 862 | + | })).Return(floating, nil).Once() |
| 864 | - | close(floatingOpened) | |||
| 865 | - | return floating, nil | |||
| 866 | - | }).Once() | |||
| 867 | d := newTestDaemon(t, f, stubClock{}) | 863 | d := newTestDaemon(t, f, stubClock{}) | ||
| 868 | d.stopped["old"] = stoppedSession{name: "old", cwd: t.TempDir(), createdAt: 1} | 864 | d.stopped["old"] = stoppedSession{name: "old", cwd: t.TempDir(), createdAt: 1} | ||
| 869 | tr := portsmocks.NewMockTransport(t) | 865 | tr := portsmocks.NewMockTransport(t) | ||
| @@ -881,11 +877,15 @@ func TestResumeStoppedAndSwitchInheritsTerminalEnv(t *testing.T) { | |||||
| 881 | require.Contains(t, opens[1], "TERM=xterm-direct") | 877 | require.Contains(t, opens[1], "TERM=xterm-direct") | ||
| 882 | require.Contains(t, opens[1], "COLORTERM=truecolor") | 878 | require.Contains(t, opens[1], "COLORTERM=truecolor") | ||
| 883 | require.Contains(t, opens[1], "TERM_PROGRAM=vev") | 879 | require.Contains(t, opens[1], "TERM_PROGRAM=vev") | ||
| 884 | - | select { | 880 | + | require.Eventually(t, func() bool { |
| 885 | - | case <-floatingOpened: | 881 | + | tb := got.activeTab() |
| 886 | - | case <-time.After(time.Second): | 882 | + | if tb == nil { |
| 887 | - | t.Fatal("floating prewarm PTY was not opened") | 883 | + | return false |
| 888 | - | } | 884 | + | } |
| 885 | + | tb.mu.Lock() | |||
| 886 | + | defer tb.mu.Unlock() | |||
| 887 | + | return tb.floating.pane != nil && tb.floating.pane.pty == floating | |||
| 888 | + | }, time.Second, 5*time.Millisecond) | |||
| 889 | _ = d.killSession(got, ports.ReasonSessionKilled, false) | 889 | _ = d.killSession(got, ports.ReasonSessionKilled, false) | ||
| 890 | release1() | 890 | release1() | ||
| 891 | release2() | 891 | release2() | ||