Commit `47b439b55a`

47b439b55af1ce1f8dd2756339743945ad2bdaf6

parent: f70349ad3c

Verified · cmc ci/build: success ci/sonar: success ci/test: success ci/vuln: success

cmc <hello@cleberg.net> · 2026-09-04 19:40 UTC

cli: a command reading stdin on a terminal says so

`repo secret set` blocked with nothing printed, which is
indistinguishable from a hung connection — and pressing Enter did not end
it, because the server reads to EOF. It looked identical before and after
the user had done the right thing, and the token echoed into the
scrollback on the way.

The fix is client-side: whether stdin is a terminal is a fact about the
client, and the daemon cannot see it.

A terminal now gets told what is wanted and how to end it. A secret is
read without echo and takes a single line, so Enter is enough and the
value never reaches the scrollback — which is the point of a rule that
keeps it out of argv and logs.

Piped input is passed through untouched: no prompt, no byte added or
removed, so `printf %s "$TOKEN" | gitbay repo secret set ...` sends
exactly the token. That is the half that can silently corrupt a
credential, so it is the half with a test.

Public keys keep echoing. They are public, and hiding them would be
theatre.

The payload's name rides the command tree as an annotation, so
TestStdinCommandsNameTheirPayload fails when a new stdin command forgets
one and would have prompted with the word "input".

Closes #150

Layout: unified · split

cmd/gitbay/main.go +42 −10

 const serverPath = "gitbay.server_path"
 const stdinMode = "gitbay.stdin_mode"
+// stdinWhat carries the payload's name onto the command tree so the
+// coverage test can assert every stdin-payload command has one; without
+// it the terminal prompt says the unhelpful word "input" (#150).
+const stdinWhat = "gitbay.stdin_what"
 // passOpts describes how one CLI command maps onto the server command.
 type passOpts struct {
 	server      []string // server-side command path
 	needsRepo   bool     // prepend inferred owner/name unless given
 	stdinOK     bool     // wire local stdin through when --file - asks for it
 	alwaysStdin bool     // stdin is the payload, named by no flag: a bare redirect
-	editor      string   // open $EDITOR for a body when none given
+	// stdinWhat names the payload for the prompt shown when stdin is a
-	inferSource bool     // --source defaults to the checked-out branch inside a clone
+	// terminal; stdinSecret hides the input and takes one line, for a
+	// value that should not reach the scrollback.
+	stdinWhat   string
+	stdinSecret bool
+	editor      string // open $EDITOR for a body when none given
+	inferSource bool   // --source defaults to the checked-out branch inside a clone
 }
 // pass builds a passthrough command. Flags are parsed by the server, which
 		Annotations: map[string]string{
 			serverPath: strings.Join(o.server, " "),
 			stdinMode:  o.stdinModeName(),
+			stdinWhat:  o.stdinWhat,
 		},
 		DisableFlagParsing: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
 	}
 	if stdin == nil || isEmptyReader(stdin) {
 		if o.alwaysStdin || (o.stdinOK && usesStdin(args)) {
-			stdin = os.Stdin
+			what := o.stdinWhat
+			if what == "" {
+				what = "input"
+			}
+			r, err := stdinPayload(os.Stdin, what, o.stdinSecret)
+			if err != nil {
+				fmt.Fprintln(os.Stderr, "gitbay:", err)
+				return protocol.ExitFailure
+			}
+			stdin = r
 		}
 	}
 	return runSSH(t, append(o.server, args...), stdin)
 func authCmd() *cobra.Command {
 	keysAdd := pass("add", "register an SSH public key (reads the key from stdin or --file -)",
-		passOpts{server: []string{"keys", "add"}, alwaysStdin: true})
+		passOpts{server: []string{"keys", "add"}, alwaysStdin: true, stdinWhat: "an SSH public key"})
 	// keys add always reads stdin on the server; wire it through directly.
 	keysAdd.RunE = func(cmd *cobra.Command, args []string) error {
 		t, err := resolveTarget()
 		if err != nil {
 			return err
 		}
-		os.Exit(runSSH(t, append([]string{"keys", "add"}, args...), os.Stdin))
+		in, err := stdinPayload(os.Stdin, "an SSH public key", false)
+		if err != nil {
+			return err
+		}
+		os.Exit(runSSH(t, append([]string{"keys", "add"}, args...), in))
 		return nil
 	}
 	pgpAdd := &cobra.Command{
 		Use: "add", Short: "register an OpenPGP public key (armored, on stdin)",
-		Annotations:        map[string]string{serverPath: "pgp add"},
+		Annotations: map[string]string{
+			serverPath: "pgp add",
+			stdinMode:  "always",
+			stdinWhat:  "an armored OpenPGP public key",
+		},
 		DisableFlagParsing: true,
 		RunE: func(cmd *cobra.Command, args []string) error {
 			t, err := resolveTarget()
 			if err != nil {
 				return err
 			}
-			os.Exit(runSSH(t, append([]string{"pgp", "add"}, args...), os.Stdin))
+			in, err := stdinPayload(os.Stdin, "an armored OpenPGP public key", false)
+			if err != nil {
+				return err
+			}
+			os.Exit(runSSH(t, append([]string{"pgp", "add"}, args...), in))
 			return nil
 		},
 	}
 		pass("import-issues", "import GitHub issue/PR history: --from <ghowner/ghrepo> [--token-stdin]",
 			passOpts{server: []string{"repo", "import-issues"}, needsRepo: true, stdinOK: true}),
 		group("deploy-key", "repository-bound CI keys",
-			pass("add", "bind a key: [--rw] < key.pub", passOpts{server: []string{"repo", "deploy-key", "add"}, needsRepo: true, alwaysStdin: true}),
+			pass("add", "bind a key: [--rw] < key.pub", passOpts{server: []string{"repo", "deploy-key", "add"}, needsRepo: true, alwaysStdin: true, stdinWhat: "an SSH public key"}),
 			pass("list", "list deploy keys", passOpts{server: []string{"repo", "deploy-key", "list"}, needsRepo: true}),
 			pass("remove", "remove a deploy key: <fingerprint>", passOpts{server: []string{"repo", "deploy-key", "remove"}, needsRepo: true}),
 		),
 			pass("status", "show check state and what is behind", passOpts{server: []string{"repo", "deps", "status"}, needsRepo: true}),
 		),
 		group("secret", "build secrets (values on stdin, injected into build env)",
-			pass("set", "set a secret: <NAME> (value on stdin)", passOpts{server: []string{"repo", "secret", "set"}, needsRepo: true, alwaysStdin: true}),
+			pass("set", "set a secret: <NAME> (value on stdin)", passOpts{server: []string{"repo", "secret", "set"}, needsRepo: true, alwaysStdin: true, stdinWhat: "the secret value", stdinSecret: true}),
 			pass("list", "list secret names", passOpts{server: []string{"repo", "secret", "list"}, needsRepo: true}),
 			pass("remove", "remove a secret: <NAME>", passOpts{server: []string{"repo", "secret", "remove"}, needsRepo: true}),
 		),
 		pass("show", "show a release with assets: <tag>", passOpts{server: []string{"release", "show"}, needsRepo: true}),
 		pass("delete", "delete a release and its assets: <tag> --yes", passOpts{server: []string{"release", "delete"}, needsRepo: true}),
 		group("asset", "binary assets on a release",
-			pass("add", "upload from stdin: <tag> <filename> < file", passOpts{server: []string{"release", "asset", "add"}, needsRepo: true, alwaysStdin: true}),
+			pass("add", "upload from stdin: <tag> <filename> < file", passOpts{server: []string{"release", "asset", "add"}, needsRepo: true, alwaysStdin: true, stdinWhat: "the asset's bytes"}),
 			pass("get", "download to stdout: <tag> <filename> > file", passOpts{server: []string{"release", "asset", "get"}, needsRepo: true}),
 			pass("remove", "remove an asset: <tag> <filename>", passOpts{server: []string{"release", "asset", "remove"}, needsRepo: true}),
 		),

cmd/gitbay/stdinpayload.go added +47

@@ -0,0 +1,47 @@
	1	package main
	2
	3	import (
	4	"bytes"
	5	"fmt"
	6	"io"
	7	"os"
	8
	9	"golang.org/x/term"
	10	)
	11
	12	// A command whose payload is stdin blocks on a terminal with nothing
	13	// printed, which is indistinguishable from a hung connection — and
	14	// pressing Enter does not end it, because the server reads to EOF. So it
	15	// looks the same before and after you have done the right thing (#150).
	16	//
	17	// The fix belongs here rather than in the daemon: whether stdin is a
	18	// terminal is a fact about the client, and the server cannot see it.
	19
	20	// isTerminal is a variable so tests can drive both paths; a test process
	21	// has no terminal, which is the case that must stay byte-identical.
	22	var isTerminal = func(f *os.File) bool { return term.IsTerminal(int(f.Fd())) }
	23
	24	// stdinPayload returns the reader for a command that takes its payload on
	25	// stdin. Piped or redirected input is passed through untouched — no
	26	// prompt, no added or removed bytes — because a script's `printf %s
	27	// "$TOKEN" \| gitbay ...` must send exactly the token.
	28	//
	29	// what names the thing being read, for the prompt. secret hides the input
	30	// and takes a single line, so a credential never lands in the terminal's
	31	// scrollback and Enter is enough to finish.
	32	func stdinPayload(in *os.File, what string, secret bool) (io.Reader, error) {
	33	if !isTerminal(in) {
	34	return in, nil
	35	}
	36	if secret {
	37	fmt.Fprintf(os.Stderr, "%s (input hidden, Enter when done): ", what)
	38	raw, err := term.ReadPassword(int(in.Fd()))
	39	fmt.Fprintln(os.Stderr)
	40	if err != nil {
	41	return nil, fmt.Errorf("reading %s: %w", what, err)
	42	}
	43	return bytes.NewReader(raw), nil
	44	}
	45	fmt.Fprintf(os.Stderr, "reading %s from stdin — paste it, then press Ctrl-D. (Or pipe it in.)\n", what)
	46	return in, nil
	47	}

cmd/gitbay/stdinpayload_test.go added +116

@@ -0,0 +1,116 @@
	1	package main
	2
	3	import (
	4	"io"
	5	"os"
	6	"sort"
	7	"strings"
	8	"testing"
	9
	10	"github.com/spf13/cobra"
	11	)
	12
	13	// The piped path must stay byte-identical: a script doing
	14	// `printf %s "$TOKEN" \| gitbay repo secret set ...` has to send exactly
	15	// the token, with no prompt written and no byte added or removed. This is
	16	// the half that can silently corrupt a credential, so it is the half with
	17	// a test.
	18	func TestStdinPayloadPipedIsUntouched(t *testing.T) {
	19	defer swapTerminal(func(*os.File) bool { return false })()
	20
	21	r, w, err := os.Pipe()
	22	if err != nil {
	23	t.Fatal(err)
	24	}
	25	const token = "sqp_0123456789abcdef"
	26	go func() { io.WriteString(w, token); w.Close() }()
	27
	28	got, err := stdinPayload(r, "the secret value", true)
	29	if err != nil {
	30	t.Fatal(err)
	31	}
	32	if got != io.Reader(r) {
	33	t.Fatal("piped stdin was replaced rather than passed through")
	34	}
	35	b, _ := io.ReadAll(got)
	36	if string(b) != token {
	37	t.Fatalf("piped payload = %q, want %q", b, token)
	38	}
	39	}
	40
	41	// A non-terminal stdin must not be prompted at, in either mode. The
	42	// prompt goes to stderr, so a test watching stdout would miss this.
	43	func TestStdinPayloadPipedIsSilent(t *testing.T) {
	44	defer swapTerminal(func(*os.File) bool { return false })()
	45
	46	prev := os.Stderr
	47	r, w, err := os.Pipe()
	48	if err != nil {
	49	t.Fatal(err)
	50	}
	51	os.Stderr = w
	52	for _, secret := range []bool{false, true} {
	53	if _, err := stdinPayload(os.Stdin, "the secret value", secret); err != nil {
	54	os.Stderr = prev
	55	t.Fatalf("secret=%v: %v", secret, err)
	56	}
	57	}
	58	w.Close()
	59	os.Stderr = prev
	60	out, _ := io.ReadAll(r)
	61	if len(out) != 0 {
	62	t.Fatalf("piped stdin was prompted at: %q", out)
	63	}
	64	}
	65
	66	// On a terminal the non-secret path says what it wants and how to end it.
	67	// Someone who does not know Ctrl-D is the person this exists for.
	68	func TestStdinPayloadTerminalExplainsItself(t *testing.T) {
	69	defer swapTerminal(func(*os.File) bool { return true })()
	70
	71	prev := os.Stderr
	72	r, w, err := os.Pipe()
	73	if err != nil {
	74	t.Fatal(err)
	75	}
	76	os.Stderr = w
	77	_, err = stdinPayload(os.Stdin, "an SSH public key", false)
	78	w.Close()
	79	os.Stderr = prev
	80	if err != nil {
	81	t.Fatal(err)
	82	}
	83	out, _ := io.ReadAll(r)
	84	got := string(out)
	85	for _, want := range []string{"an SSH public key", "Ctrl-D"} {
	86	if !strings.Contains(got, want) {
	87	t.Errorf("terminal prompt %q lacks %q", got, want)
	88	}
	89	}
	90	}
	91
	92	// Every command whose payload is stdin names it, or the prompt above says
	93	// the unhelpful word "input".
	94	func TestStdinCommandsNameTheirPayload(t *testing.T) {
	95	var missing []string
	96	var walk func(*cobra.Command, string)
	97	walk = func(c *cobra.Command, path string) {
	98	if c.Annotations[stdinMode] == "always" && c.Annotations[stdinWhat] == "" {
	99	missing = append(missing, strings.TrimSpace(path))
	100	}
	101	for _, sub := range c.Commands() {
	102	walk(sub, path+" "+sub.Name())
	103	}
	104	}
	105	walk(newRoot(), "")
	106	sort.Strings(missing)
	107	if len(missing) > 0 {
	108	t.Errorf("stdin-payload commands with no stdinWhat: %s", strings.Join(missing, ", "))
	109	}
	110	}
	111
	112	func swapTerminal(f func(*os.File) bool) func() {
	113	prev := isTerminal
	114	isTerminal = f
	115	return func() { isTerminal = prev }
	116	}

Commit 47b439b55a

Commit `47b439b55a`