From 4ff55f052acebb4231463d379e95b5f43ea687e1 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 22:50:05 +1000 Subject: [PATCH 01/10] Fail a bare invocation when stdout is not a terminal Puppet's generate() reads stdout through a pipe with combine: true, so help text on a zero exit lands in the catalog. Help now requires an interactive stdout; a piped one exits non-zero with nothing on stdout. - gate the no-args help path on an isTerminal(stdout) check - reject an explicitly empty --config instead of resolving it - ignore a relative XDG_CONFIG_HOME, per the XDG spec - give the integration subprocess a hermetic HOME/XDG_CONFIG_HOME - pass []string{} to run(), so cobra cannot fall back to os.Args --- AGENTS.md | 7 +- README.md | 13 +++- cmd/certmanager/main.go | 46 +++++++++++-- cmd/certmanager/main_test.go | 99 ++++++++++++++++++++++++++-- internal/config/config.go | 6 +- internal/config/config_test.go | 20 +++++- test/integration/certmanager_test.go | 50 ++++++++++++++ 7 files changed, 219 insertions(+), 22 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 079f3a8..b6fbe32 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,9 +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. - **Flat CLI**: the root command takes `common_name` positionally - (`cobra.MaximumNArgs(1)`); no args prints help, args-less flags fail. `ca`, - `completion` and `help` are the only subcommands; `--config` is persistent so - `ca` shares it. + (`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 + the only subcommands; `--config` is persistent so `ca` shares it, and an + explicitly empty `--config` is rejected. - **`ca` is unauthenticated**: `GET {addr}/v1/{mount}/ca_chain` sends no `X-Vault-Token` and loads the config through `config.LoadCA`, which requires only `vault.addr`. The body is parsed as PEM, never sniffed — Vault labels it diff --git a/README.md b/README.md index b9118fa..1e85850 100644 --- a/README.md +++ b/README.md @@ -21,13 +21,18 @@ failure path exits non-zero. ``` certmanager [flags] # issue a certificate certmanager ca [flags] # print the CA chain -certmanager # print help +certmanager # print help (interactive terminals only) ``` 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. + | Flag | Short | Type | Default | Description | |---|---|---|---|---| | `--alt-names` | `-a` | comma list | `""` | DNS subject alternative names | @@ -110,8 +115,10 @@ YAML, everything nested under `vault:`. ### Config resolution order -1. `--config `, which always wins; -2. `$XDG_CONFIG_HOME/certmanager/config.yaml`, if the file exists; +1. `--config `, which always wins; an empty `--config ''` is an error, + not a request to resolve; +2. `$XDG_CONFIG_HOME/certmanager/config.yaml`, if `$XDG_CONFIG_HOME` is + absolute and the file exists; 3. `$HOME/.config/certmanager/config.yaml`, if the file exists; 4. `/opt/certmanager/config.yaml`, the path baked in at build time (`make build CONFIG_PATH=...`). diff --git a/cmd/certmanager/main.go b/cmd/certmanager/main.go index ad0a241..c3fb629 100644 --- a/cmd/certmanager/main.go +++ b/cmd/certmanager/main.go @@ -26,7 +26,11 @@ func main() { // it writes nothing to stderr: Puppet's generate() runs this binary with // combine: true, which splices stderr into the stdout that parsejson() reads. func run(args []string, stdout, stderr io.Writer) int { - cmd := rootCmd() + return runWith(args, stdout, stderr, isTerminal(stdout)) +} + +func runWith(args []string, stdout, stderr io.Writer, interactive bool) int { + cmd := rootCmd(interactive) cmd.SetArgs(args) cmd.SetOut(stdout) cmd.SetErr(stderr) @@ -38,7 +42,27 @@ func run(args []string, stdout, stderr io.Writer) int { return 0 } -func rootCmd() *cobra.Command { +// isTerminal reports whether w is a terminal. Puppet's generate() captures +// stdout through a pipe, so anything else is a machine caller. +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 +} + +// resolveConfig turns the --config value into a path. An explicitly empty +// --config is a malformed call, not a request to resolve. +func resolveConfig(cmd *cobra.Command, explicit string) (string, error) { + if explicit == "" && cmd.Flags().Changed("config") { + return "", fmt.Errorf("--config requires a path") + } + return config.Resolve(explicit), nil +} + +func rootCmd(interactive bool) *cobra.Command { var ( cfgFile string altNames string @@ -57,15 +81,19 @@ func rootCmd() *cobra.Command { SilenceErrors: true, RunE: func(cmd *cobra.Command, args []string) error { if len(args) == 0 { - // A bare invocation is someone asking what this is; flags - // without a common name is a malformed call and must fail. - if cmd.Flags().NFlag() == 0 { + // Help only for a human at a terminal: under generate() an + // argv that lost its common name must fail the compile. + if interactive && cmd.Flags().NFlag() == 0 { return cmd.Help() } return fmt.Errorf("a common name is required") } - cfg, err := config.Load(config.Resolve(cfgFile)) + path, err := resolveConfig(cmd, cfgFile) + if err != nil { + return err + } + cfg, err := config.Load(path) if err != nil { return err } @@ -116,7 +144,11 @@ func caCmd(cfgFile *string) *cobra.Command { SilenceUsage: true, SilenceErrors: true, RunE: func(cmd *cobra.Command, _ []string) error { - cfg, err := config.LoadCA(config.Resolve(*cfgFile)) + path, err := resolveConfig(cmd, *cfgFile) + if err != nil { + return err + } + cfg, err := config.LoadCA(path) if err != nil { return err } diff --git a/cmd/certmanager/main_test.go b/cmd/certmanager/main_test.go index 4df7d93..e8e8934 100644 --- a/cmd/certmanager/main_test.go +++ b/cmd/certmanager/main_test.go @@ -772,11 +772,11 @@ func TestRunCAFailures(t *testing.T) { } } -// A bare invocation is a human at a terminal, not Puppet: print help and exit -// zero rather than failing on the missing positional. -func TestRunNoArgsPrintsHelp(t *testing.T) { +// A bare invocation at a terminal is a human asking what this is: print help +// and exit zero rather than failing on the missing positional. +func TestRunNoArgsOnATerminalPrintsHelp(t *testing.T) { var stdout, stderr bytes.Buffer - if code := run(nil, &stdout, &stderr); code != 0 { + if code := runWith([]string{}, &stdout, &stderr, true); code != 0 { t.Fatalf("exit code = %d, stderr = %q", code, stderr.String()) } if !strings.Contains(stdout.String(), "Usage:") { @@ -790,6 +790,97 @@ func TestRunNoArgsPrintsHelp(t *testing.T) { } } +// generate() splices stderr into stdout and reads the result as JSON, so an +// argv that lost its common name must fail loudly with nothing on stdout. +func TestRunNoArgsWithoutATerminalFails(t *testing.T) { + var stdout, stderr bytes.Buffer + code := runWith([]string{}, &stdout, &stderr, false) + if code == 0 { + t.Fatalf("exit code = 0, want non-zero (stdout %q)", stdout.String()) + } + if stdout.Len() != 0 { + t.Errorf("stdout = %q, want empty", stdout.String()) + } + if !strings.Contains(stderr.String(), "a common name is required") { + t.Errorf("stderr = %q, want it to name the missing common name", stderr.String()) + } +} + +// run() decides interactivity from the real stdout, so a piped stdout must not +// take the help path. +func TestRunNoArgsPipedStdoutFails(t *testing.T) { + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + defer func() { _ = r.Close() }() + + var stderr bytes.Buffer + code := run([]string{}, w, &stderr) + if err := w.Close(); err != nil { + t.Fatalf("close: %v", err) + } + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read: %v", err) + } + if code == 0 { + t.Fatalf("exit code = 0, want non-zero (stdout %q)", out) + } + if len(out) != 0 { + t.Errorf("stdout = %q, want empty", out) + } +} + +func TestIsTerminal(t *testing.T) { + var buf bytes.Buffer + if isTerminal(&buf) { + t.Error("a bytes.Buffer is not a terminal") + } + + r, w, err := os.Pipe() + 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") + } + + tty, err := os.OpenFile("/dev/tty", os.O_WRONLY, 0) + if err != nil { + t.Skipf("no controlling terminal: %v", err) + } + defer func() { _ = tty.Close() }() + if !isTerminal(tty) { + t.Error("/dev/tty is a terminal") + } +} + +// --config with an empty value is a malformed call, not a request to resolve. +func TestRunRejectsEmptyConfigFlag(t *testing.T) { + srv, _ := newVaultStub(t, stubOptions{}) + seedXDGConfig(t, srv.URL) + + for _, args := range [][]string{ + {"host.example.com", "--config", "", "--json"}, + {"ca", "--config", ""}, + } { + var stdout, stderr bytes.Buffer + code := run(args, &stdout, &stderr) + if code == 0 { + t.Fatalf("%v: exit code = 0, want non-zero (stdout %q)", args, stdout.String()) + } + if stdout.Len() != 0 { + t.Errorf("%v: stdout = %q, want empty", args, stdout.String()) + } + if !strings.Contains(stderr.String(), "--config requires a path") { + t.Errorf("%v: stderr = %q", args, stderr.String()) + } + } +} + // seedXDGConfig writes a config where DefaultPath() looks first. func seedXDGConfig(t *testing.T, addr string) { t.Helper() diff --git a/internal/config/config.go b/internal/config/config.go index 2648b83..d341629 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -40,7 +40,8 @@ func FallbackPath() string { return defaultPath } // are only taken when the file is actually there, so an unset or unusable // HOME lands on the Puppet-managed path instead of a path that cannot exist. func DefaultPath() string { - if base := os.Getenv("XDG_CONFIG_HOME"); base != "" { + // The XDG spec requires a relative XDG_CONFIG_HOME to be ignored. + if base := os.Getenv("XDG_CONFIG_HOME"); filepath.IsAbs(base) { if p := filepath.Join(base, "certmanager", "config.yaml"); isFile(p) { return p } @@ -53,7 +54,8 @@ func DefaultPath() string { return defaultPath } -// Resolve picks the config path for a run: an explicit --config always wins. +// Resolve picks the config path for a run: a non-empty explicit --config +// always wins, an empty one means "resolve". func Resolve(explicit string) string { if explicit != "" { return explicit diff --git a/internal/config/config_test.go b/internal/config/config_test.go index e4687fe..3ad41c1 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -204,7 +204,7 @@ func TestDefaultPathIgnoresADirectory(t *testing.T) { func TestResolvePrefersExplicitPath(t *testing.T) { xdg := t.TempDir() - seedConfig(t, xdg) + resolved := seedConfig(t, xdg) t.Setenv("XDG_CONFIG_HOME", xdg) t.Setenv("HOME", t.TempDir()) @@ -212,8 +212,22 @@ func TestResolvePrefersExplicitPath(t *testing.T) { if got := Resolve(explicit); got != explicit { t.Errorf("Resolve(%q) = %q, want the explicit path", explicit, got) } - if got := Resolve(""); got == explicit { - t.Error("Resolve(\"\") returned the explicit path") + if got := Resolve(""); got != resolved { + t.Errorf("Resolve(\"\") = %q, want the resolved path %q", got, resolved) + } +} + +// The XDG spec says a non-absolute XDG_CONFIG_HOME must be ignored, so a +// config sitting under the working directory cannot be picked up. +func TestDefaultPathIgnoresRelativeXDG(t *testing.T) { + wd := t.TempDir() + seedConfig(t, filepath.Join(wd, "xdg")) + t.Chdir(wd) + t.Setenv("XDG_CONFIG_HOME", "xdg") + t.Setenv("HOME", t.TempDir()) + + if got := DefaultPath(); got != FallbackPath() { + t.Errorf("DefaultPath() = %q, want %q", got, FallbackPath()) } } diff --git a/test/integration/certmanager_test.go b/test/integration/certmanager_test.go index b3a0799..5981d1a 100644 --- a/test/integration/certmanager_test.go +++ b/test/integration/certmanager_test.go @@ -272,9 +272,24 @@ func writeConfig(t *testing.T, dir, roleID string) string { return path } +// hermeticEnv strips the developer's environment from the subprocess. The +// tests claim to exercise the baked-in config path, so a real +// ~/.config/certmanager/config.yaml must not resolve ahead of it and point the +// binary at a production Vault. +func hermeticEnv(t *testing.T) []string { + t.Helper() + empty := t.TempDir() + return []string{ + "PATH=/usr/bin:/bin", + "HOME=" + empty, + "XDG_CONFIG_HOME=" + empty, + } +} + func runCertmanager(t *testing.T, bin string, args ...string) (int, string, string) { t.Helper() cmd := exec.Command(bin, args...) + cmd.Env = hermeticEnv(t) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout cmd.Stderr = &stderr @@ -474,6 +489,41 @@ func TestCAChainNeedsNoCredentials(t *testing.T) { } } +// A developer with a real ~/.config/certmanager/config.yaml must not have it +// leak into the subprocess, which would run the suite against their Vault. +func TestIgnoresTheDevelopersConfig(t *testing.T) { + waitForVault(t) + roleID := setupVault(t) + + dir := t.TempDir() + bin := buildBinary(t, dir, writeConfig(t, dir, roleID)) + + poisoned := t.TempDir() + cfgDir := filepath.Join(poisoned, "certmanager") + if err := os.MkdirAll(cfgDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + body := "vault:\n addr: http://127.0.0.1:1\n mount_point: nope\n role_name: nope\n" + + " approle_path: approle\n role_id: nope\n" + if err := os.WriteFile(filepath.Join(cfgDir, "config.yaml"), []byte(body), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } + t.Setenv("XDG_CONFIG_HOME", poisoned) + t.Setenv("HOME", poisoned) + + code, stdout, stderr := runCertmanager(t, bin, commonName, "--json") + if code != 0 { + t.Fatalf("exit code %d, stderr: %s", code, stderr) + } + var doc map[string]string + if err := json.Unmarshal([]byte(stdout), &doc); err != nil { + t.Fatalf("stdout is not JSON: %v\n%s", err, stdout) + } + if doc["certificate"] == "" { + t.Error("no certificate issued; the baked-in config path was not used") + } +} + func TestCAChainFailsCleanlyForAnUnknownMount(t *testing.T) { waitForVault(t) roleID := setupVault(t) -- 2.47.3 From 5a7fea56e223c9db7eb2cdb4c0507f650011b131 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 22:58:52 +1000 Subject: [PATCH 02/10] Document the interactive seam in AGENTS.md --- AGENTS.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index b6fbe32..1b2bfcb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -44,7 +44,9 @@ dist/ # build output: binaries, RPM, checksums (not comm - **`run(args, stdout, stderr) int`** is the testable entry point; `main()` only wires it to the real streams and `os.Exit`. Cobra runs with `SilenceUsage` and - `SilenceErrors` so nothing reaches stderr unless the run failed. + `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. - **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 -- 2.47.3 From 30eee88d95983cf341838a7beebc135528470970 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:29:32 +1000 Subject: [PATCH 03/10] 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. --- AGENTS.md | 11 +++- README.md | 9 ++-- cmd/certmanager/main.go | 8 +-- cmd/certmanager/main_test.go | 71 ++++++++++++++++++++++---- cmd/certmanager/terminal_linux_test.go | 25 +++++++++ go.mod | 2 + go.sum | 4 ++ 7 files changed, 111 insertions(+), 19 deletions(-) create mode 100644 cmd/certmanager/terminal_linux_test.go diff --git a/AGENTS.md b/AGENTS.md index 1b2bfcb..30f5f72 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -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 diff --git a/README.md b/README.md index 1e85850..3862e0b 100644 --- a/README.md +++ b/README.md @@ -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 | |---|---|---|---|---| diff --git a/cmd/certmanager/main.go b/cmd/certmanager/main.go index c3fb629..bec4cc1 100644 --- a/cmd/certmanager/main.go +++ b/cmd/certmanager/main.go @@ -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 diff --git a/cmd/certmanager/main_test.go b/cmd/certmanager/main_test.go index e8e8934..4dfd50f 100644 --- a/cmd/certmanager/main_test.go +++ b/cmd/certmanager/main_test.go @@ -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. diff --git a/cmd/certmanager/terminal_linux_test.go b/cmd/certmanager/terminal_linux_test.go new file mode 100644 index 0000000..0ed8782 --- /dev/null +++ b/cmd/certmanager/terminal_linux_test.go @@ -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") + } +} diff --git a/go.mod b/go.mod index 7bcb130..0b1aa05 100644 --- a/go.mod +++ b/go.mod @@ -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 ) diff --git a/go.sum b/go.sum index 47edb24..499532b 100644 --- a/go.sum +++ b/go.sum @@ -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= -- 2.47.3 From c6c3f6a09201aca0c1986f9308a1b686c7a7f5c9 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:29:32 +1000 Subject: [PATCH 04/10] Pin the invocation gate against every kind of stdout --- test/integration/invocation_linux_test.go | 180 ++++++++++++++++++++++ 1 file changed, 180 insertions(+) create mode 100644 test/integration/invocation_linux_test.go diff --git a/test/integration/invocation_linux_test.go b/test/integration/invocation_linux_test.go new file mode 100644 index 0000000..b88ca22 --- /dev/null +++ b/test/integration/invocation_linux_test.go @@ -0,0 +1,180 @@ +//go:build integration && linux + +package integration + +import ( + "fmt" + "io" + "os" + "path/filepath" + "strings" + "syscall" + "testing" + + "golang.org/x/sys/unix" +) + +// Under generate() the exit code is the only failure signal and every byte of +// stdout is fed to parsejson(). A bare invocation may therefore print help and +// exit zero on a terminal and nowhere else. /dev/null and a closed fd 1 are the +// traps: both leave the process holding a character device, so a stat of the +// file mode calls them terminals. +func TestBareInvocationHelpsOnlyATerminal(t *testing.T) { + dir := t.TempDir() + bin := buildBinary(t, dir, filepath.Join(dir, "absent.yaml")) + + t.Run("pipe", func(t *testing.T) { + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + defer func() { _ = r.Close() }() + + code := runBare(t, bin, w) + _ = w.Close() + out, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read: %v", err) + } + assertMachineCaller(t, code, out) + }) + + t.Run("regular file", func(t *testing.T) { + path := filepath.Join(t.TempDir(), "stdout") + f, err := os.Create(path) + if err != nil { + t.Fatalf("create: %v", err) + } + defer func() { _ = f.Close() }() + + code := runBare(t, bin, f) + out, err := os.ReadFile(path) + if err != nil { + t.Fatalf("read: %v", err) + } + assertMachineCaller(t, code, out) + }) + + t.Run("unix socket", func(t *testing.T) { + sock, peer := socketPair(t) + code := runBare(t, bin, sock) + // The child's copy went with it; dropping ours ends the peer's read. + _ = sock.Close() + out, err := io.ReadAll(peer) + if err != nil { + t.Fatalf("read: %v", err) + } + assertMachineCaller(t, code, out) + }) + + // Nothing written to /dev/null is observable, so the exit code carries the + // whole contract here -- which is exactly the situation under generate(). + t.Run("dev null", func(t *testing.T) { + devNull, err := os.OpenFile(os.DevNull, os.O_WRONLY, 0) + if err != nil { + t.Fatalf("open %s: %v", os.DevNull, err) + } + defer func() { _ = devNull.Close() }() + + if code := runBare(t, bin, devNull); code == 0 { + t.Errorf("exit code = 0, want non-zero") + } + }) + + // A nil file closes fd 1 across the exec. The Go runtime reopens it onto + // /dev/null before main() runs, so the process sees a character device it + // never chose. + t.Run("closed fd 1", func(t *testing.T) { + if code := runBare(t, bin, nil); code == 0 { + t.Errorf("exit code = 0, want non-zero") + } + }) + + t.Run("pty", func(t *testing.T) { + master, slave := openPTY(t) + drained := make(chan []byte, 1) + go func() { + out, _ := io.ReadAll(master) + drained <- out + }() + + code := runBare(t, bin, slave) + // The child's copy is gone; dropping ours makes the master read EIO. + _ = slave.Close() + out := <-drained + + if code != 0 { + t.Errorf("exit code = %d, want 0", code) + } + if !strings.Contains(string(out), "Usage:") { + t.Errorf("stdout = %q, want the help text", out) + } + }) +} + +func assertMachineCaller(t *testing.T, code int, stdout []byte) { + t.Helper() + if code == 0 { + t.Errorf("exit code = 0, want non-zero (stdout %q)", stdout) + } + if len(stdout) != 0 { + t.Errorf("stdout = %q, want empty", stdout) + } +} + +// runBare runs the binary with no arguments and stdout as fd 1. A nil stdout +// leaves fd 1 closed at exec. +func runBare(t *testing.T, bin string, stdout *os.File) int { + t.Helper() + proc, err := os.StartProcess(bin, []string{bin}, &os.ProcAttr{ + Env: hermeticEnv(t), + Files: []*os.File{nil, stdout, nil}, + }) + if err != nil { + t.Fatalf("start %s: %v", bin, err) + } + state, err := proc.Wait() + if err != nil { + t.Fatalf("wait: %v", err) + } + return state.ExitCode() +} + +// socketPair returns a connected unix socket to hand the child as stdout, and +// the peer end to read back whatever reached it. +func socketPair(t *testing.T) (sock, peer *os.File) { + t.Helper() + fds, err := syscall.Socketpair(syscall.AF_UNIX, syscall.SOCK_STREAM, 0) + if err != nil { + t.Fatalf("socketpair: %v", err) + } + sock = os.NewFile(uintptr(fds[0]), "stdout-socket") + peer = os.NewFile(uintptr(fds[1]), "stdout-socket-peer") + t.Cleanup(func() { _ = sock.Close(); _ = peer.Close() }) + return sock, peer +} + +// openPTY allocates a pty pair. CI containers have no controlling terminal, so +// this is the only way to exercise the branch that prints help and exits zero. +func openPTY(t *testing.T) (master, slave *os.File) { + t.Helper() + m, 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() { _ = m.Close() }) + + if err := unix.IoctlSetPointerInt(int(m.Fd()), unix.TIOCSPTLCK, 0); err != nil { + t.Fatalf("unlock pty: %v", err) + } + n, err := unix.IoctlGetInt(int(m.Fd()), unix.TIOCGPTN) + if err != nil { + t.Fatalf("pty number: %v", err) + } + s, err := os.OpenFile(fmt.Sprintf("/dev/pts/%d", n), syscall.O_RDWR|syscall.O_NOCTTY, 0) + if err != nil { + t.Fatalf("open pty slave: %v", err) + } + t.Cleanup(func() { _ = s.Close() }) + return m, s +} -- 2.47.3 From b0099991c0406ea867e03aec277c789ee9810a89 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:29:32 +1000 Subject: [PATCH 05/10] Poison both config legs the hermetic env has to strip --- test/integration/certmanager_test.go | 40 +++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/test/integration/certmanager_test.go b/test/integration/certmanager_test.go index 5981d1a..568fc1a 100644 --- a/test/integration/certmanager_test.go +++ b/test/integration/certmanager_test.go @@ -286,6 +286,28 @@ func hermeticEnv(t *testing.T) []string { } } +// Neither leg of the resolution order announces itself when it is missing: an +// unset HOME or XDG_CONFIG_HOME silently falls through to whatever the +// developer has. Both must therefore be set to somewhere empty, not dropped. +func TestHermeticEnvSetsBothConfigLegs(t *testing.T) { + env := hermeticEnv(t) + for _, key := range []string{"HOME=", "XDG_CONFIG_HOME="} { + idx := slices.IndexFunc(env, func(e string) bool { return strings.HasPrefix(e, key) }) + if idx < 0 { + t.Fatalf("%s is unset in the subprocess env: %v", key, env) + } + dir := strings.TrimPrefix(env[idx], key) + if !filepath.IsAbs(dir) { + t.Errorf("%s%s is not an absolute path", key, dir) + } + for _, rel := range []string{"certmanager/config.yaml", ".config/certmanager/config.yaml"} { + if _, err := os.Stat(filepath.Join(dir, rel)); err == nil { + t.Errorf("%s%s already holds %s", key, dir, rel) + } + } + } +} + func runCertmanager(t *testing.T, bin string, args ...string) (int, string, string) { t.Helper() cmd := exec.Command(bin, args...) @@ -498,15 +520,21 @@ func TestIgnoresTheDevelopersConfig(t *testing.T) { dir := t.TempDir() bin := buildBinary(t, dir, writeConfig(t, dir, roleID)) + // Both legs of the resolution order get a config, so either one leaking + // into the subprocess points it at an unreachable Vault and fails the run. poisoned := t.TempDir() - cfgDir := filepath.Join(poisoned, "certmanager") - if err := os.MkdirAll(cfgDir, 0o755); err != nil { - t.Fatalf("mkdir: %v", err) - } body := "vault:\n addr: http://127.0.0.1:1\n mount_point: nope\n role_name: nope\n" + " approle_path: approle\n role_id: nope\n" - if err := os.WriteFile(filepath.Join(cfgDir, "config.yaml"), []byte(body), 0o600); err != nil { - t.Fatalf("write config: %v", err) + for _, cfgDir := range []string{ + filepath.Join(poisoned, "certmanager"), + filepath.Join(poisoned, ".config", "certmanager"), + } { + if err := os.MkdirAll(cfgDir, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + if err := os.WriteFile(filepath.Join(cfgDir, "config.yaml"), []byte(body), 0o600); err != nil { + t.Fatalf("write config: %v", err) + } } t.Setenv("XDG_CONFIG_HOME", poisoned) t.Setenv("HOME", poisoned) -- 2.47.3 From 21990d9297dd85f9b7f1fb7a59fdb2dfd17e23f6 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:48:59 +1000 Subject: [PATCH 06/10] Pass an explicit empty argv so cobra cannot reach os.Args A nil argv lets cobra fall back to os.Args[1:], which under `go test -args foo` turns the bare-invocation cases into `certmanager foo` and resolves /opt/certmanager/config.yaml. Assert the bare-invocation error on stderr so the fallback cannot hide. Puppet gives the child a temp file as fd 1, not a pipe; say so. --- README.md | 6 +++--- cmd/certmanager/main.go | 4 ++-- cmd/certmanager/main_test.go | 10 +++++++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 3862e0b..8b3de95 100644 --- a/README.md +++ b/README.md @@ -30,9 +30,9 @@ 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 — 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. +nothing on stdout. Puppet's `generate()` collects stdout through a temp file, +so there a missing common name fails the compile rather than feeding help text +into the catalog. | Flag | Short | Type | Default | Description | |---|---|---|---|---| diff --git a/cmd/certmanager/main.go b/cmd/certmanager/main.go index bec4cc1..f196a22 100644 --- a/cmd/certmanager/main.go +++ b/cmd/certmanager/main.go @@ -43,8 +43,8 @@ func runWith(args []string, stdout, stderr io.Writer, interactive bool) int { return 0 } -// isTerminal reports whether w is a terminal. Puppet's generate() captures -// stdout through a pipe, so anything else is a machine caller. The check is a +// isTerminal reports whether w is a terminal. Puppet hands the child a temp +// file as fd 1, never a terminal, 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 { diff --git a/cmd/certmanager/main_test.go b/cmd/certmanager/main_test.go index 4dfd50f..d76d58f 100644 --- a/cmd/certmanager/main_test.go +++ b/cmd/certmanager/main_test.go @@ -869,10 +869,18 @@ func TestIsTerminalRejectsNonTerminals(t *testing.T) { if isTerminal(f) { t.Errorf("%s is not a terminal", name) } + // An explicit empty argv, never nil: cobra falls back to os.Args[1:] + // for a nil one, which under `go test -args foo` would turn this into + // `certmanager foo` and issue against whatever Vault the host's + // config resolves to. The stderr assertion is what catches that. var stderr bytes.Buffer - if code := run(nil, f, &stderr); code == 0 { + code := run([]string{}, f, &stderr) + if code == 0 { t.Errorf("%s: bare invocation exit code = 0, want non-zero", name) } + if want := "a common name is required"; !strings.Contains(stderr.String(), want) { + t.Errorf("%s: stderr = %q, want it to contain %q", name, stderr.String(), want) + } } } -- 2.47.3 From ddd93a8b71abba5f3e534e4197bc14e8d60295c6 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:48:59 +1000 Subject: [PATCH 07/10] Assert the hermetic env replaces the ambient one, not extends it The shape test passed on `return os.Environ()`; only a Vault-backed test caught it. Move the helper and its test out from behind the integration tag and reject any entry outside PATH/HOME/XDG_CONFIG_HOME. --- test/integration/certmanager_test.go | 36 --------------- test/integration/doc.go | 6 +-- test/integration/hermetic_env_test.go | 66 +++++++++++++++++++++++++++ 3 files changed, 69 insertions(+), 39 deletions(-) create mode 100644 test/integration/hermetic_env_test.go diff --git a/test/integration/certmanager_test.go b/test/integration/certmanager_test.go index 568fc1a..e837d49 100644 --- a/test/integration/certmanager_test.go +++ b/test/integration/certmanager_test.go @@ -272,42 +272,6 @@ func writeConfig(t *testing.T, dir, roleID string) string { return path } -// hermeticEnv strips the developer's environment from the subprocess. The -// tests claim to exercise the baked-in config path, so a real -// ~/.config/certmanager/config.yaml must not resolve ahead of it and point the -// binary at a production Vault. -func hermeticEnv(t *testing.T) []string { - t.Helper() - empty := t.TempDir() - return []string{ - "PATH=/usr/bin:/bin", - "HOME=" + empty, - "XDG_CONFIG_HOME=" + empty, - } -} - -// Neither leg of the resolution order announces itself when it is missing: an -// unset HOME or XDG_CONFIG_HOME silently falls through to whatever the -// developer has. Both must therefore be set to somewhere empty, not dropped. -func TestHermeticEnvSetsBothConfigLegs(t *testing.T) { - env := hermeticEnv(t) - for _, key := range []string{"HOME=", "XDG_CONFIG_HOME="} { - idx := slices.IndexFunc(env, func(e string) bool { return strings.HasPrefix(e, key) }) - if idx < 0 { - t.Fatalf("%s is unset in the subprocess env: %v", key, env) - } - dir := strings.TrimPrefix(env[idx], key) - if !filepath.IsAbs(dir) { - t.Errorf("%s%s is not an absolute path", key, dir) - } - for _, rel := range []string{"certmanager/config.yaml", ".config/certmanager/config.yaml"} { - if _, err := os.Stat(filepath.Join(dir, rel)); err == nil { - t.Errorf("%s%s already holds %s", key, dir, rel) - } - } - } -} - func runCertmanager(t *testing.T, bin string, args ...string) (int, string, string) { t.Helper() cmd := exec.Command(bin, args...) diff --git a/test/integration/doc.go b/test/integration/doc.go index dde368b..a7d8776 100644 --- a/test/integration/doc.go +++ b/test/integration/doc.go @@ -1,5 +1,5 @@ // Package integration holds the end-to-end tests that run against a real Vault -// dev server. The tests are behind the `integration` build tag; this file -// keeps the package buildable for a plain `go test ./...`, which otherwise -// fails on a package whose files are all excluded by build constraints. +// dev server. Those tests are behind the `integration` build tag; the +// subprocess environment they share, and its own test, are not, so a plain +// `go test ./...` still guards the helper the tagged tests depend on. package integration diff --git a/test/integration/hermetic_env_test.go b/test/integration/hermetic_env_test.go new file mode 100644 index 0000000..3bbc3bb --- /dev/null +++ b/test/integration/hermetic_env_test.go @@ -0,0 +1,66 @@ +package integration + +import ( + "os" + "path/filepath" + "slices" + "strings" + "testing" +) + +// hermeticEnv strips the developer's environment from the subprocess. The +// tests claim to exercise the baked-in config path, so a real +// ~/.config/certmanager/config.yaml must not resolve ahead of it and point the +// binary at a production Vault. +func hermeticEnv(t *testing.T) []string { + t.Helper() + empty := t.TempDir() + return []string{ + "PATH=/usr/bin:/bin", + "HOME=" + empty, + "XDG_CONFIG_HOME=" + empty, + } +} + +// Neither leg of the resolution order announces itself when it is missing: an +// unset HOME or XDG_CONFIG_HOME silently falls through to whatever the +// developer has. Both must therefore be set to somewhere empty, not dropped. +// Setting them is not enough either: the env has to replace the ambient one +// rather than extend it, so anything the developer exported is asserted gone. +func TestHermeticEnvSetsBothConfigLegs(t *testing.T) { + t.Setenv("CERTMANAGER_AMBIENT_MARKER", "leaked") + t.Setenv("HOME", filepath.Join(t.TempDir(), "developer")) + + env := hermeticEnv(t) + + allowed := []string{"PATH", "HOME", "XDG_CONFIG_HOME"} + for _, entry := range env { + key, _, ok := strings.Cut(entry, "=") + if !ok { + t.Errorf("%q is not a KEY=VALUE entry", entry) + continue + } + if !slices.Contains(allowed, key) { + t.Errorf("%s is carried over from the ambient environment; want only %v", key, allowed) + } + } + + for _, key := range []string{"HOME=", "XDG_CONFIG_HOME="} { + idx := slices.IndexFunc(env, func(e string) bool { return strings.HasPrefix(e, key) }) + if idx < 0 { + t.Fatalf("%s is unset in the subprocess env: %v", key, env) + } + dir := strings.TrimPrefix(env[idx], key) + if !filepath.IsAbs(dir) { + t.Errorf("%s%s is not an absolute path", key, dir) + } + if ambient := os.Getenv(strings.TrimSuffix(key, "=")); ambient != "" && dir == ambient { + t.Errorf("%s%s is the ambient value, not a scratch directory", key, dir) + } + for _, rel := range []string{"certmanager/config.yaml", ".config/certmanager/config.yaml"} { + if _, err := os.Stat(filepath.Join(dir, rel)); err == nil { + t.Errorf("%s%s already holds %s", key, dir, rel) + } + } + } +} -- 2.47.3 From 173a82f29ee1590fff96c5c0db7585d052fc4924 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:48:59 +1000 Subject: [PATCH 08/10] Pin x/term v0.45.0 and x/sys v0.47.0 Newest pair still declaring go 1.25.0; v0.46.0/v0.48.0 declare go 1.26.0 and break the golang:1.25 CI images. --- go.mod | 4 ++-- go.sum | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 0b1aa05..608ac3b 100644 --- a/go.mod +++ b/go.mod @@ -4,8 +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 + golang.org/x/sys v0.47.0 + golang.org/x/term v0.45.0 gopkg.in/yaml.v3 v3.0.1 ) diff --git a/go.sum b/go.sum index 499532b..37f8f68 100644 --- a/go.sum +++ b/go.sum @@ -7,10 +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= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= 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= -- 2.47.3 From 0860da03c43ef678738f530196484fc8f56125d5 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:49:06 +1000 Subject: [PATCH 09/10] Document the untagged hermetic-env helper --- AGENTS.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/AGENTS.md b/AGENTS.md index 30f5f72..b6540e3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -110,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. +`hermeticEnv`, the subprocess environment those tests share, sits outside the +tag in `hermetic_env_test.go` so a plain `go test ./...` guards it. It replaces +the ambient environment rather than extending it; its test asserts exactly +that, because a helper that leaked `HOME` would point the binary at a real +Vault and only the tagged tests would notice. + `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 -- 2.47.3 From d39ca1e3a6c861c063c014d93c3bbe5927032e68 Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Tue, 22 Sep 2026 23:49:34 +1000 Subject: [PATCH 10/10] Rewrap the isTerminal comment --- cmd/certmanager/main.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/certmanager/main.go b/cmd/certmanager/main.go index f196a22..999ce2e 100644 --- a/cmd/certmanager/main.go +++ b/cmd/certmanager/main.go @@ -44,9 +44,10 @@ func runWith(args []string, stdout, stderr io.Writer, interactive bool) int { } // isTerminal reports whether w is a terminal. Puppet hands the child a temp -// file as fd 1, never a terminal, 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. +// file as fd 1, never a terminal, 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 { -- 2.47.3