Merge pull request 'Fail a bare invocation when stdout is not a terminal' (#4) from benvin/fix-invocation-guards into master

Reviewed-on: #4

AGENTS.md

@@ -44,10 +44,17 @@ 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. `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.ExactArgs(1)`). `ca`, `completion` and `help` are the only
+  (`cobra.MaximumNArgs(1)`); no args prints help only when stdout is a terminal
-  subcommands; `--config` is persistent so `ca` shares it.
+  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
@@ -59,8 +66,13 @@ dist/                         # build output: binaries, RPM, checksums (not comm
 - **`vault.New`** authenticates once and never falls back between auth methods.
   Every request is bounded by `vault.timeout` (default 30s) — the Python set no
   timeout, so a black-holed Vault hung the whole catalog compile.
-- **`config.defaultPath`** is baked in at build time via
+- **`config.Resolve`** picks the config: `--config`, then
-  `-X ...internal/config.defaultPath=` (`make build CONFIG_PATH=...`).
+  `$XDG_CONFIG_HOME/certmanager/config.yaml`, then
+  `$HOME/.config/certmanager/config.yaml` (both only if the file exists), then
+  `config.FallbackPath()` — baked in at build time via
+  `-X ...internal/config.defaultPath=` (`make build CONFIG_PATH=...`). Under
+  `generate()` the puppet user has no usable home, so the baked path is what
+  resolves.
 
 ## Deliberate deviations from the Python script
 
@@ -98,6 +110,18 @@ 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
+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

README.md

@@ -21,8 +21,19 @@ failure path exits non-zero.
 ```
 certmanager <common_name> [flags]   # issue a certificate
 certmanager ca [flags]              # print the CA chain
+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 —
+a pipe, a file, a socket, `/dev/null` and a closed fd 1 all exit non-zero with
+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 |
 |---|---|---|---|---|
 | `--alt-names` | `-a` | comma list | `""` | DNS subject alternative names |
@@ -30,7 +41,7 @@ certmanager ca [flags]              # print the CA chain
 | `--expiry-days` | `-e` | int | `365` | Certificate validity in days (`ttl: <n>d`) |
 | `--compress` | `-c` | bool | `false` | Write a zip instead of a directory |
 | `--json` | | bool | `false` | Print the certificate data as JSON |
-| `--config` | | path | `/opt/certmanager/config.yaml` | Config file location |
+| `--config` | | path | resolved (see below) | Config file location |
 
 `ca` takes `--config` plus:
 
@@ -101,8 +112,23 @@ stdout.
 
 ## Configuration
 
-YAML, everything nested under `vault:`. The path is baked in at build time
+YAML, everything nested under `vault:`.
-(`make build CONFIG_PATH=...`) and overridable per-invocation with `--config`.
+
+### Config resolution order
+
+1. `--config <path>`, 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=...`).
+
+Steps 2 and 3 are skipped unless the file is really there, so the last step is
+what Puppet gets: `generate()` runs the binary as the `puppet` user with no
+usable home directory, and `profiles::helpers::certmanager` writes the config
+to `/opt/certmanager/config.yaml` on managed nodes. The k8s puppetserver
+compilers mount it at the same path.
 
 | Key | Required | Default | Notes |
 |---|---|---|---|

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"
@@ -26,7 +27,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 +43,29 @@ func run(args []string, stdout, stderr io.Writer) int {
 	return 0
 }
 
-func rootCmd() *cobra.Command {
+// 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 {
+	f, ok := w.(*os.File)
+	if !ok {
+		return false
+	}
+	return term.IsTerminal(int(f.Fd()))
+}
+
+// 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
@@ -52,11 +79,24 @@ func rootCmd() *cobra.Command {
 		Use:           "certmanager <common_name>",
 		Short:         "Issue a PKI certificate from HashiCorp Vault",
 		Version:       version,
-		Args:          cobra.ExactArgs(1),
+		Args:          cobra.MaximumNArgs(1),
 		SilenceUsage:  true,
 		SilenceErrors: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
-			cfg, err := config.Load(cfgFile)
+			if len(args) == 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")
+			}
+
+			path, err := resolveConfig(cmd, cfgFile)
+			if err != nil {
+				return err
+			}
+			cfg, err := config.Load(path)
 			if err != nil {
 				return err
 			}
@@ -79,9 +119,10 @@ func rootCmd() *cobra.Command {
 		},
 	}
 
-	// Persistent so the ca subcommand shares it; the positional form keeps
+	// Persistent so the ca subcommand shares it; empty means "resolve", which
-	// parsing --config exactly as before.
+	// keeps the resolution order out of the usage text and out of --help.
-	cmd.PersistentFlags().StringVar(&cfgFile, "config", config.DefaultPath(), "path to config.yaml")
+	cmd.PersistentFlags().StringVar(&cfgFile, "config", "",
+		"path to config.yaml (default: $XDG_CONFIG_HOME or ~/.config, else "+config.FallbackPath()+")")
 	cmd.Flags().StringVarP(&altNames, "alt-names", "a", "", "comma-separated DNS subject alternative names")
 	cmd.Flags().StringVarP(&ipSans, "ip-sans", "i", "", "comma-separated IP subject alternative names")
 	cmd.Flags().IntVarP(&expiryDays, "expiry-days", "e", 365, "certificate validity in days")
@@ -106,7 +147,11 @@ func caCmd(cfgFile *string) *cobra.Command {
 		SilenceUsage:  true,
 		SilenceErrors: true,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			cfg, err := config.LoadCA(*cfgFile)
+			path, err := resolveConfig(cmd, *cfgFile)
+			if err != nil {
+				return err
+			}
+			cfg, err := config.LoadCA(path)
 			if err != nil {
 				return err
 			}

cmd/certmanager/main_test.go

@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"io"
 	"math/big"
+	"net"
 	"net/http"
 	"net/http/httptest"
 	"os"
@@ -21,6 +22,8 @@ import (
 	"strings"
 	"testing"
 	"time"
+
+	"git.unkin.net/unkin/certmanager/internal/config"
 )
 
 const (
@@ -769,3 +772,236 @@ func TestRunCAFailures(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 := runWith([]string{}, &stdout, &stderr, true); code != 0 {
+		t.Fatalf("exit code = %d, stderr = %q", code, stderr.String())
+	}
+	if !strings.Contains(stdout.String(), "Usage:") {
+		t.Errorf("stdout = %q, want the help text", stdout.String())
+	}
+	if !strings.Contains(stdout.String(), "certmanager <common_name>") {
+		t.Errorf("help does not document the positional form: %q", stdout.String())
+	}
+	if stderr.Len() != 0 {
+		t.Errorf("stderr = %q, want empty", stderr.String())
+	}
+}
+
+// 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)
+	}
+}
+
+// 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")
+	}
+
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("pipe: %v", err)
+	}
+	t.Cleanup(func() { _ = r.Close(); _ = w.Close() })
+
+	regular, err := os.Create(filepath.Join(t.TempDir(), "stdout"))
+	if err != nil {
+		t.Fatalf("create: %v", err)
+	}
+	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)
+		}
+		// 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
+		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)
+		}
+	}
+}
+
+// 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.
+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()
+	xdg := t.TempDir()
+	dir := filepath.Join(xdg, "certmanager")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	body := "vault:\n  addr: " + addr + "\n  auth_method: approle\n" +
+		"  approle_path: approle\n  role_id: test-role-id\n" +
+		"  mount_point: pki_int\n  role_name: servers_default\n"
+	if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(body), 0o600); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", t.TempDir())
+}
+
+// Puppet passes no --config, so the resolved path has to be used.
+func TestRunResolvesConfigWithoutFlag(t *testing.T) {
+	srv, got := newVaultStub(t, stubOptions{})
+	seedXDGConfig(t, srv.URL)
+
+	var stdout, stderr bytes.Buffer
+	code := run([]string{"host.example.com", "--alt-names", "host", "--expiry-days", "90", "--json"}, &stdout, &stderr)
+	if code != 0 {
+		t.Fatalf("exit code = %d, stderr = %q", code, stderr.String())
+	}
+	if got.issueBody["common_name"] != "host.example.com" {
+		t.Errorf("common_name = %v", got.issueBody["common_name"])
+	}
+	if stderr.Len() != 0 {
+		t.Errorf("stderr = %q, want empty", stderr.String())
+	}
+}
+
+// The ca subcommand shares the same resolution.
+func TestRunCAResolvesConfigWithoutFlag(t *testing.T) {
+	srv, got := newVaultStub(t, stubOptions{})
+	seedXDGConfig(t, srv.URL)
+
+	var stdout, stderr bytes.Buffer
+	if code := run([]string{"ca"}, &stdout, &stderr); code != 0 {
+		t.Fatalf("exit code = %d, stderr = %q", code, stderr.String())
+	}
+	if !got.caRequested {
+		t.Error("the CA chain endpoint was never called")
+	}
+}
+
+// With no config anywhere the run fails naming the baked-in path, not a
+// home-relative one that could never exist under generate().
+func TestRunWithoutAnyConfigNamesTheBakedPath(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	t.Setenv("HOME", t.TempDir())
+
+	var stdout, stderr bytes.Buffer
+	if code := run([]string{"host.example.com", "--json"}, &stdout, &stderr); code == 0 {
+		t.Fatal("exit code = 0, want non-zero")
+	}
+	if stdout.Len() != 0 {
+		t.Errorf("stdout = %q, want empty", stdout.String())
+	}
+	if !strings.Contains(stderr.String(), config.FallbackPath()) {
+		t.Errorf("stderr = %q, want it to name %q", stderr.String(), config.FallbackPath())
+	}
+}

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")
+	}
+}

go.mod

@@ -4,6 +4,8 @@ go 1.25.7
 
 require (
 	github.com/spf13/cobra v1.10.2
+	golang.org/x/sys v0.47.0
+	golang.org/x/term v0.45.0
 	gopkg.in/yaml.v3 v3.0.1
 )
 

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.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=

internal/config/config.go

@@ -3,6 +3,7 @@ package config
 import (
 	"fmt"
 	"os"
+	"path/filepath"
 	"time"
 
 	"gopkg.in/yaml.v3"
@@ -28,8 +29,44 @@ const DefaultTimeout = 30 * time.Second
 // -X git.unkin.net/unkin/certmanager/internal/config.defaultPath=/some/path
 var defaultPath = "/opt/certmanager/config.yaml"
 
-// DefaultPath returns the config path baked in at build time.
+// FallbackPath returns the config path baked in at build time. Puppet writes
-func DefaultPath() string { return defaultPath }
+// the config there, and it is the only path that resolves under generate(),
+// which runs as the puppet user with no usable home directory.
+func FallbackPath() string { return defaultPath }
+
+// DefaultPath resolves the config to use when --config was not given:
+// $XDG_CONFIG_HOME/certmanager/config.yaml, then
+// $HOME/.config/certmanager/config.yaml, then FallbackPath(). The first two
+// 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 {
+	// 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
+		}
+	}
+	if home, err := os.UserHomeDir(); err == nil && home != "" {
+		if p := filepath.Join(home, ".config", "certmanager", "config.yaml"); isFile(p) {
+			return p
+		}
+	}
+	return defaultPath
+}
+
+// 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
+	}
+	return DefaultPath()
+}
+
+func isFile(path string) bool {
+	info, err := os.Stat(path)
+	return err == nil && info.Mode().IsRegular()
+}
 
 // Config is the top-level configuration structure.
 type Config struct {

internal/config/config_test.go

@@ -120,9 +120,114 @@ func TestLoadMissingFile(t *testing.T) {
 	}
 }
 
-func TestDefaultPath(t *testing.T) {
+func TestFallbackPath(t *testing.T) {
-	if got := DefaultPath(); got != "/opt/certmanager/config.yaml" {
+	if got := FallbackPath(); got != "/opt/certmanager/config.yaml" {
-		t.Errorf("DefaultPath() = %q, want the Puppet-managed path", got)
+		t.Errorf("FallbackPath() = %q, want the Puppet-managed path", got)
+	}
+}
+
+// seedConfig drops a config file at dir/certmanager/config.yaml and returns it.
+func seedConfig(t *testing.T, dir string) string {
+	t.Helper()
+	path := filepath.Join(dir, "certmanager", "config.yaml")
+	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	if err := os.WriteFile(path, []byte(approleBody), 0o600); err != nil {
+		t.Fatalf("write: %v", err)
+	}
+	return path
+}
+
+func TestDefaultPathPrefersXDG(t *testing.T) {
+	xdg := t.TempDir()
+	home := t.TempDir()
+	want := seedConfig(t, xdg)
+	seedConfig(t, filepath.Join(home, ".config"))
+
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", home)
+
+	if got := DefaultPath(); got != want {
+		t.Errorf("DefaultPath() = %q, want the XDG config %q", got, want)
+	}
+}
+
+func TestDefaultPathFallsBackToHome(t *testing.T) {
+	home := t.TempDir()
+	want := seedConfig(t, filepath.Join(home, ".config"))
+
+	t.Setenv("XDG_CONFIG_HOME", "")
+	t.Setenv("HOME", home)
+
+	if got := DefaultPath(); got != want {
+		t.Errorf("DefaultPath() = %q, want the home config %q", got, want)
+	}
+}
+
+// XDG pointing somewhere without a config must not shadow a usable home one.
+func TestDefaultPathSkipsXDGWithoutAConfig(t *testing.T) {
+	home := t.TempDir()
+	want := seedConfig(t, filepath.Join(home, ".config"))
+
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	t.Setenv("HOME", home)
+
+	if got := DefaultPath(); got != want {
+		t.Errorf("DefaultPath() = %q, want the home config %q", got, want)
+	}
+}
+
+// What generate() sees: the puppet user has no usable home, so neither lookup
+// yields a file and the baked-in path has to win.
+func TestDefaultPathFallsBackToBakedPath(t *testing.T) {
+	t.Setenv("XDG_CONFIG_HOME", "")
+	t.Setenv("HOME", "")
+
+	if got := DefaultPath(); got != FallbackPath() {
+		t.Errorf("DefaultPath() = %q, want %q", got, FallbackPath())
+	}
+}
+
+func TestDefaultPathIgnoresADirectory(t *testing.T) {
+	xdg := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(xdg, "certmanager", "config.yaml"), 0o755); err != nil {
+		t.Fatalf("mkdir: %v", err)
+	}
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", t.TempDir())
+
+	if got := DefaultPath(); got != FallbackPath() {
+		t.Errorf("DefaultPath() = %q, want %q", got, FallbackPath())
+	}
+}
+
+func TestResolvePrefersExplicitPath(t *testing.T) {
+	xdg := t.TempDir()
+	resolved := seedConfig(t, xdg)
+	t.Setenv("XDG_CONFIG_HOME", xdg)
+	t.Setenv("HOME", t.TempDir())
+
+	explicit := writeConfig(t, approleBody)
+	if got := Resolve(explicit); got != explicit {
+		t.Errorf("Resolve(%q) = %q, want the explicit path", explicit, got)
+	}
+	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())
 	}
 }
 

test/integration/certmanager_test.go

@@ -275,6 +275,7 @@ func writeConfig(t *testing.T, dir, roleID string) string {
 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 +475,47 @@ 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))
+
+	// 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()
+	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"
+	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)
+
+	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)

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
+// dev server. Those tests are behind the `integration` build tag; the
-// keeps the package buildable for a plain `go test ./...`, which otherwise
+// subprocess environment they share, and its own test, are not, so a plain
-// fails on a package whose files are all excluded by build constraints.
+// `go test ./...` still guards the helper the tagged tests depend on.
 package integration

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)
+			}
+		}
+	}
+}

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
+}