Detect a terminal with an ioctl, not a character-device stat
/dev/null is a character device, and so is the /dev/null the Go runtime reopens onto a closed fd 1, so a bare invocation printed help and exited zero for both -- the two shapes a machine caller is most likely to hand the binary.
This commit is contained in:
@@ -46,7 +46,10 @@ dist/ # build output: binaries, RPM, checksums (not comm
|
||||
wires it to the real streams and `os.Exit`. Cobra runs with `SilenceUsage` and
|
||||
`SilenceErrors` so nothing reaches stderr unless the run failed. `run` derives
|
||||
interactivity from `isTerminal(stdout)` and hands it to `runWith`, which tests
|
||||
call directly to pin either side.
|
||||
call directly to pin either side. `isTerminal` is a terminal ioctl
|
||||
(`x/term.IsTerminal`), not a character-device stat: `/dev/null` and the
|
||||
`/dev/null` the runtime reopens onto a closed fd 1 are character devices, and
|
||||
a stat would hand both of them the help-and-exit-zero path.
|
||||
- **Flat CLI**: the root command takes `common_name` positionally
|
||||
(`cobra.MaximumNArgs(1)`); no args prints help only when stdout is a terminal
|
||||
and otherwise fails, args-less flags fail. `ca`, `completion` and `help` are
|
||||
@@ -107,6 +110,12 @@ four JSON keys, the parsed X.509 CN/SANs/lifetime, and that a refused name
|
||||
exits non-zero with empty stdout. Dev mode is plain HTTP, so it does not
|
||||
exercise the TLS path.
|
||||
|
||||
`TestBareInvocationHelpsOnlyATerminal` drives the built binary once per kind of
|
||||
fd 1 — pipe, regular file, unix socket, `/dev/null`, a closed fd 1 and a pty
|
||||
allocated from `/dev/ptmx` — and only the pty may exit zero with help on
|
||||
stdout. The pty is what pins the true branch: CI containers have no
|
||||
controlling terminal, so a test that reached for `/dev/tty` would skip.
|
||||
|
||||
## Gotchas
|
||||
|
||||
- Changing the JSON key set or key order breaks `parsejson()` in
|
||||
|
||||
@@ -28,10 +28,11 @@ The common name is positional, with no `pki` subcommand in front of it — that
|
||||
is the form Puppet calls, and it is the only issuing form. `ca`, `completion`
|
||||
and `help` are the subcommands; every other first argument is a common name.
|
||||
|
||||
A bare invocation prints help and exits zero only when stdout is a terminal.
|
||||
Puppet's `generate()` reads stdout through a pipe, so there a missing common
|
||||
name exits non-zero with nothing on stdout rather than feeding help text into
|
||||
the catalog.
|
||||
A bare invocation prints help and exits zero only when stdout is a terminal —
|
||||
a pipe, a file, a socket, `/dev/null` and a closed fd 1 all exit non-zero with
|
||||
nothing on stdout. Puppet's `generate()` reads stdout through a pipe, so there
|
||||
a missing common name fails the compile rather than feeding help text into the
|
||||
catalog.
|
||||
|
||||
| Flag | Short | Type | Default | Description |
|
||||
|---|---|---|---|---|
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"strings"
|
||||
|
||||
"github.com/spf13/cobra"
|
||||
"golang.org/x/term"
|
||||
|
||||
"git.unkin.net/unkin/certmanager/internal/config"
|
||||
"git.unkin.net/unkin/certmanager/internal/pki"
|
||||
@@ -43,14 +44,15 @@ func runWith(args []string, stdout, stderr io.Writer, interactive bool) int {
|
||||
}
|
||||
|
||||
// isTerminal reports whether w is a terminal. Puppet's generate() captures
|
||||
// stdout through a pipe, so anything else is a machine caller.
|
||||
// stdout through a pipe, so anything else is a machine caller. The check is a
|
||||
// terminal ioctl, not a character-device stat: /dev/null is a character device,
|
||||
// and so is the /dev/null the Go runtime reopens onto a closed fd 1 at startup.
|
||||
func isTerminal(w io.Writer) bool {
|
||||
f, ok := w.(*os.File)
|
||||
if !ok {
|
||||
return false
|
||||
}
|
||||
info, err := f.Stat()
|
||||
return err == nil && info.Mode()&os.ModeCharDevice != 0
|
||||
return term.IsTerminal(int(f.Fd()))
|
||||
}
|
||||
|
||||
// resolveConfig turns the --config value into a path. An explicitly empty
|
||||
|
||||
@@ -13,6 +13,7 @@ import (
|
||||
"fmt"
|
||||
"io"
|
||||
"math/big"
|
||||
"net"
|
||||
"net/http"
|
||||
"net/http/httptest"
|
||||
"os"
|
||||
@@ -832,7 +833,10 @@ func TestRunNoArgsPipedStdoutFails(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestIsTerminal(t *testing.T) {
|
||||
// Every stdout a machine caller can hand the binary must read as
|
||||
// not-a-terminal. /dev/null is the trap: it is a character device, so a stat of
|
||||
// the file mode calls it a terminal and lets help escape with exit zero.
|
||||
func TestIsTerminalRejectsNonTerminals(t *testing.T) {
|
||||
var buf bytes.Buffer
|
||||
if isTerminal(&buf) {
|
||||
t.Error("a bytes.Buffer is not a terminal")
|
||||
@@ -842,20 +846,65 @@ func TestIsTerminal(t *testing.T) {
|
||||
if err != nil {
|
||||
t.Fatalf("pipe: %v", err)
|
||||
}
|
||||
defer func() { _ = r.Close() }()
|
||||
defer func() { _ = w.Close() }()
|
||||
if isTerminal(w) {
|
||||
t.Error("a pipe is not a terminal")
|
||||
}
|
||||
t.Cleanup(func() { _ = r.Close(); _ = w.Close() })
|
||||
|
||||
tty, err := os.OpenFile("/dev/tty", os.O_WRONLY, 0)
|
||||
regular, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
|
||||
if err != nil {
|
||||
t.Skipf("no controlling terminal: %v", err)
|
||||
t.Fatalf("create: %v", err)
|
||||
}
|
||||
defer func() { _ = tty.Close() }()
|
||||
if !isTerminal(tty) {
|
||||
t.Error("/dev/tty is a terminal")
|
||||
t.Cleanup(func() { _ = regular.Close() })
|
||||
|
||||
devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("open %s: %v", os.DevNull, err)
|
||||
}
|
||||
t.Cleanup(func() { _ = devNull.Close() })
|
||||
|
||||
for name, f := range map[string]*os.File{
|
||||
"pipe": w,
|
||||
"regular file": regular,
|
||||
os.DevNull: devNull,
|
||||
"unix socket": unixSocket(t),
|
||||
} {
|
||||
if isTerminal(f) {
|
||||
t.Errorf("%s is not a terminal", name)
|
||||
}
|
||||
var stderr bytes.Buffer
|
||||
if code := run(nil, f, &stderr); code == 0 {
|
||||
t.Errorf("%s: bare invocation exit code = 0, want non-zero", name)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// unixSocket returns a connected unix socket as an *os.File: the stdout a
|
||||
// caller gets when it wires the binary straight into a service.
|
||||
func unixSocket(t *testing.T) *os.File {
|
||||
t.Helper()
|
||||
path := filepath.Join(t.TempDir(), "s")
|
||||
ln, err := net.Listen("unix", path)
|
||||
if err != nil {
|
||||
t.Fatalf("listen: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = ln.Close() })
|
||||
go func() {
|
||||
conn, err := ln.Accept()
|
||||
if err == nil {
|
||||
_, _ = io.Copy(io.Discard, conn)
|
||||
_ = conn.Close()
|
||||
}
|
||||
}()
|
||||
|
||||
conn, err := net.Dial("unix", path)
|
||||
if err != nil {
|
||||
t.Fatalf("dial: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = conn.Close() })
|
||||
f, err := conn.(*net.UnixConn).File()
|
||||
if err != nil {
|
||||
t.Fatalf("socket file: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = f.Close() })
|
||||
return f
|
||||
}
|
||||
|
||||
// --config with an empty value is a malformed call, not a request to resolve.
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
//go:build linux
|
||||
|
||||
package main
|
||||
|
||||
import (
|
||||
"os"
|
||||
"syscall"
|
||||
"testing"
|
||||
)
|
||||
|
||||
// The help-and-exit-zero branch is the whole point of the gate, and no CI
|
||||
// container has a controlling terminal to reach it through /dev/tty. A pty
|
||||
// allocated from /dev/ptmx is a real terminal, so the true branch is pinned
|
||||
// here instead of skipped.
|
||||
func TestIsTerminalDetectsAPTY(t *testing.T) {
|
||||
pty, err := os.OpenFile("/dev/ptmx", syscall.O_RDWR|syscall.O_NOCTTY, 0)
|
||||
if err != nil {
|
||||
t.Fatalf("open /dev/ptmx: %v", err)
|
||||
}
|
||||
t.Cleanup(func() { _ = pty.Close() })
|
||||
|
||||
if !isTerminal(pty) {
|
||||
t.Error("a pty is a terminal")
|
||||
}
|
||||
}
|
||||
@@ -4,6 +4,8 @@ go 1.25.7
|
||||
|
||||
require (
|
||||
github.com/spf13/cobra v1.10.2
|
||||
golang.org/x/sys v0.46.0
|
||||
golang.org/x/term v0.44.0
|
||||
gopkg.in/yaml.v3 v3.0.1
|
||||
)
|
||||
|
||||
|
||||
@@ -7,6 +7,10 @@ github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiT
|
||||
github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY=
|
||||
github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg=
|
||||
go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg=
|
||||
golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw=
|
||||
golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
|
||||
golang.org/x/term v0.44.0 h1:0rLvDRCtNj0gZkyIXhCyOb2OAzEhLVqc4B+hrsBhrmc=
|
||||
golang.org/x/term v0.44.0/go.mod h1:7ze4MdzUzLXpSAoFP1H0bOI9aXDqveSvatT5vKcFh2Y=
|
||||
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
|
||||
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
|
||||
gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA=
|
||||
|
||||
Reference in New Issue
Block a user