From 9dd67e11b32547c0e9e9bdb9c593ca008b9177c1 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 19 May 2025 13:56:34 +1000 Subject: [PATCH 1/3] gha: bump containerd/project-checks to v1.2.2 v1.1.0 was released in 2022, and since then there have been some project renames that cause the old version to no longer work, breaking CI: go: github.com/kunalkushwaha/ltag@latest: github.com/kunalkushwaha/ltag@v0.3.0: parsing go.mod: module declares its path as: github.com/containerd/ltag but was required as: github.com/kunalkushwaha/ltag Signed-off-by: Aleksa Sarai --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 12798a2..0661b93 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -37,7 +37,7 @@ jobs: go install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.50.1 - name: Project Checks - uses: containerd/project-checks@v1.1.0 + uses: containerd/project-checks@v1.2.2 with: working-directory: src/github.com/containerd/console From 12ba7453ffca933d433261ec89cbb9b97974567a Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Mon, 19 May 2025 13:16:23 +1000 Subject: [PATCH 2/3] tc: make internal handlers take File interface In a future patch we will split GetPty() so that users can pass a File as the already-opened pty master, but in order for us to be able to use the same File interface as our other external APIs we need to migrate our internal handlers to use File rather than *os.File. This change has no user-visible impact, as the underlying types used are identical. Signed-off-by: Aleksa Sarai --- tc_darwin.go | 5 ++--- tc_freebsd_cgo.go | 5 ++--- tc_freebsd_nocgo.go | 5 ++--- tc_linux.go | 5 ++--- tc_netbsd.go | 5 ++--- tc_openbsd_cgo.go | 6 ++---- tc_openbsd_nocgo.go | 6 ++---- tc_zos.go | 5 ++--- 8 files changed, 16 insertions(+), 26 deletions(-) diff --git a/tc_darwin.go b/tc_darwin.go index 7871545..77c695a 100644 --- a/tc_darwin.go +++ b/tc_darwin.go @@ -18,7 +18,6 @@ package console import ( "fmt" - "os" "golang.org/x/sys/unix" ) @@ -30,12 +29,12 @@ const ( // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f. // unlockpt should be called before opening the slave side of a pty. -func unlockpt(f *os.File) error { +func unlockpt(f File) error { return unix.IoctlSetPointerInt(int(f.Fd()), unix.TIOCPTYUNLK, 0) } // ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { n, err := unix.IoctlGetInt(int(f.Fd()), unix.TIOCPTYGNAME) if err != nil { return "", err diff --git a/tc_freebsd_cgo.go b/tc_freebsd_cgo.go index 3328257..627f7d5 100644 --- a/tc_freebsd_cgo.go +++ b/tc_freebsd_cgo.go @@ -21,7 +21,6 @@ package console import ( "fmt" - "os" "golang.org/x/sys/unix" ) @@ -39,7 +38,7 @@ const ( // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f. // unlockpt should be called before opening the slave side of a pty. -func unlockpt(f *os.File) error { +func unlockpt(f File) error { fd := C.int(f.Fd()) if _, err := C.unlockpt(fd); err != nil { C.close(fd) @@ -49,7 +48,7 @@ func unlockpt(f *os.File) error { } // ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { n, err := unix.IoctlGetInt(int(f.Fd()), unix.TIOCGPTN) if err != nil { return "", err diff --git a/tc_freebsd_nocgo.go b/tc_freebsd_nocgo.go index 18a9b9c..434ba46 100644 --- a/tc_freebsd_nocgo.go +++ b/tc_freebsd_nocgo.go @@ -21,7 +21,6 @@ package console import ( "fmt" - "os" "golang.org/x/sys/unix" ) @@ -42,12 +41,12 @@ const ( // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f. // unlockpt should be called before opening the slave side of a pty. -func unlockpt(f *os.File) error { +func unlockpt(f File) error { panic("unlockpt() support requires cgo.") } // ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { n, err := unix.IoctlGetInt(int(f.Fd()), unix.TIOCGPTN) if err != nil { return "", err diff --git a/tc_linux.go b/tc_linux.go index 7d552ea..e98dc02 100644 --- a/tc_linux.go +++ b/tc_linux.go @@ -18,7 +18,6 @@ package console import ( "fmt" - "os" "unsafe" "golang.org/x/sys/unix" @@ -31,7 +30,7 @@ const ( // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f. // unlockpt should be called before opening the slave side of a pty. -func unlockpt(f *os.File) error { +func unlockpt(f File) error { var u int32 // XXX do not use unix.IoctlSetPointerInt here, see commit dbd69c59b81. if _, _, err := unix.Syscall(unix.SYS_IOCTL, f.Fd(), unix.TIOCSPTLCK, uintptr(unsafe.Pointer(&u))); err != 0 { @@ -41,7 +40,7 @@ func unlockpt(f *os.File) error { } // ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { var u uint32 // XXX do not use unix.IoctlGetInt here, see commit dbd69c59b81. if _, _, err := unix.Syscall(unix.SYS_IOCTL, f.Fd(), unix.TIOCGPTN, uintptr(unsafe.Pointer(&u))); err != 0 { diff --git a/tc_netbsd.go b/tc_netbsd.go index 71227ae..73cf439 100644 --- a/tc_netbsd.go +++ b/tc_netbsd.go @@ -18,7 +18,6 @@ package console import ( "bytes" - "os" "golang.org/x/sys/unix" ) @@ -31,12 +30,12 @@ const ( // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f. // unlockpt should be called before opening the slave side of a pty. // This does not exist on NetBSD, it does not allocate controlling terminals on open -func unlockpt(f *os.File) error { +func unlockpt(f File) error { return nil } // ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { ptm, err := unix.IoctlGetPtmget(int(f.Fd()), unix.TIOCPTSNAME) if err != nil { return "", err diff --git a/tc_openbsd_cgo.go b/tc_openbsd_cgo.go index 0e76f6c..46f4250 100644 --- a/tc_openbsd_cgo.go +++ b/tc_openbsd_cgo.go @@ -20,8 +20,6 @@ package console import ( - "os" - "golang.org/x/sys/unix" ) @@ -34,7 +32,7 @@ const ( ) // ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { ptspath, err := C.ptsname(C.int(f.Fd())) if err != nil { return "", err @@ -44,7 +42,7 @@ func ptsname(f *os.File) (string, error) { // unlockpt unlocks the slave pseudoterminal device corresponding to the master pseudoterminal referred to by f. // unlockpt should be called before opening the slave side of a pty. -func unlockpt(f *os.File) error { +func unlockpt(f File) error { if _, err := C.grantpt(C.int(f.Fd())); err != nil { return err } diff --git a/tc_openbsd_nocgo.go b/tc_openbsd_nocgo.go index dca9241..a8f9f6c 100644 --- a/tc_openbsd_nocgo.go +++ b/tc_openbsd_nocgo.go @@ -29,8 +29,6 @@ package console import ( - "os" - "golang.org/x/sys/unix" ) @@ -39,10 +37,10 @@ const ( cmdTcSet = unix.TIOCSETA ) -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { panic("ptsname() support requires cgo.") } -func unlockpt(f *os.File) error { +func unlockpt(f File) error { panic("unlockpt() support requires cgo.") } diff --git a/tc_zos.go b/tc_zos.go index fc90ba5..23b0bd2 100644 --- a/tc_zos.go +++ b/tc_zos.go @@ -17,7 +17,6 @@ package console import ( - "os" "strings" "golang.org/x/sys/unix" @@ -29,11 +28,11 @@ const ( ) // unlockpt is a no-op on zos. -func unlockpt(_ *os.File) error { +func unlockpt(File) error { return nil } // ptsname retrieves the name of the first available pts for the given master. -func ptsname(f *os.File) (string, error) { +func ptsname(f File) (string, error) { return "/dev/ttyp" + strings.TrimPrefix(f.Name(), "/dev/ptyp"), nil } From c79e45e6b8addceef7d8bb3c96809bd7f0ed4433 Mon Sep 17 00:00:00 2001 From: Aleksa Sarai Date: Thu, 15 May 2025 21:54:15 +1000 Subject: [PATCH 3/3] pty: add GetPtyFromFile as safer GetPty Security-conscious callers may wish to have more control over opening /dev/ptmx, and so GetPty is not suitable for them. Previously it was not possible for such users to open /dev/ptmx and then use this package to create new ptys and manage the console. On Linux, the intended usage would be for a daemon to get an O_PATH handle to /dev/ptmx that can then be re-opened (through procfs) to get a new handle to /dev/ptmx. I suspect on FreeBSD you could use O_EMPTY_PATH to accomplish something similar. Signed-off-by: Aleksa Sarai --- console_test.go | 19 +++++++++++++++++-- console_unix.go | 9 +++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/console_test.go b/console_test.go index 74695ab..c7d9ae3 100644 --- a/console_test.go +++ b/console_test.go @@ -54,8 +54,8 @@ func TestWinSize(t *testing.T) { } } -func TestConsolePty(t *testing.T) { - console, slavePath, err := NewPty() +func testConsolePty(t *testing.T, newPty func() (Console, string, error)) { + console, slavePath, err := newPty() if err != nil { t.Fatal(err) } @@ -100,3 +100,18 @@ func TestConsolePty(t *testing.T) { t.Errorf("unexpected output %q", out) } } + +func TestConsolePty_NewPty(t *testing.T) { + testConsolePty(t, NewPty) +} + +func TestConsolePty_NewPtyFromFile(t *testing.T) { + testConsolePty(t, func() (Console, string, error) { + // Equivalent to NewPty(). + f, err := openpt() + if err != nil { + return nil, "", err + } + return NewPtyFromFile(f) + }) +} diff --git a/console_unix.go b/console_unix.go index 161f5d1..aa4c696 100644 --- a/console_unix.go +++ b/console_unix.go @@ -31,6 +31,15 @@ func NewPty() (Console, string, error) { if err != nil { return nil, "", err } + return NewPtyFromFile(f) +} + +// NewPtyFromFile creates a new pty pair, just like [NewPty] except that the +// provided [os.File] is used as the master rather than automatically creating +// a new master from /dev/ptmx. The ownership of [os.File] is passed to the +// returned [Console], so the caller must be careful to not call Close on the +// underlying file. +func NewPtyFromFile(f File) (Console, string, error) { slave, err := ptsname(f) if err != nil { return nil, "", err