From d1aeb1114d40613bade64de7d276c4d90f2c2b66 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 16:29:01 -0400 Subject: [PATCH 1/4] auth/token: emit trailing newline (C4) --- pkg/cmd/auth/token/token.go | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/auth/token/token.go b/pkg/cmd/auth/token/token.go index 1c3cd13..da79af5 100644 --- a/pkg/cmd/auth/token/token.go +++ b/pkg/cmd/auth/token/token.go @@ -1,8 +1,9 @@ // SPDX-License-Identifier: AGPL-3.0-or-later // Package token implements `shithub auth token`. Prints the stored token -// for a host to stdout (no trailing newline). Used as a building block by -// git credential helpers and by shell pipelines that need the raw bearer. +// for a host to stdout, terminated with a newline (gh-compat C4). Used +// as a building block by git credential helpers and by shell pipelines +// that need the raw bearer. package token import ( @@ -35,7 +36,7 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command { cmd := &cobra.Command{ Use: "token", Short: "Print the authenticated token for a host", - Long: `Print the stored token for a host (no trailing newline). + Long: `Print the stored token for a host, terminated with a newline. Useful when shell-scripting against the shithub REST API or when configuring a git credential helper manually: @@ -68,7 +69,7 @@ func Run(ctx context.Context, opts *Options) error { } return err } - if _, err := opts.IO.Out.Write([]byte(token)); err != nil { + if _, err := fmt.Fprintln(opts.IO.Out, token); err != nil { return err } _ = ctx From 7a3fb9bb72354b960aab97e4c1393fdfb6cccf80 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 16:29:01 -0400 Subject: [PATCH 2/4] auth/token: assert trailing newline in test --- pkg/cmd/auth/token/token_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/pkg/cmd/auth/token/token_test.go b/pkg/cmd/auth/token/token_test.go index 28ccbec..f20b9fd 100644 --- a/pkg/cmd/auth/token/token_test.go +++ b/pkg/cmd/auth/token/token_test.go @@ -30,11 +30,15 @@ func TestTokenPrintsStoredValue(t *testing.T) { t.Fatalf("Run: %v", err) } got := tf.Out.String() - if got != "shithub_pat_xyz" { - t.Errorf("token output: want exact bytes, got %q", got) + // C4: token must end with a single newline. gh emits one; shell + // `read TOKEN` and line-oriented pipes assume one; the no-newline + // behavior was visibly bad ("shithub_pat_...$EXIT_CODE" butted + // against the next prompt char). + if got != "shithub_pat_xyz\n" { + t.Errorf("token output: want exact bytes %q, got %q", "shithub_pat_xyz\n", got) } - if strings.HasSuffix(got, "\n") { - t.Errorf("token should have no trailing newline") + if !strings.HasSuffix(got, "\n") { + t.Errorf("token must end with a newline") } } From 2c0fe87ee82ed82cd848f9e70fe81bd972cccd46 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 16:29:01 -0400 Subject: [PATCH 3/4] auth/status: error on no-hosts and unknown-hostname (C2/C3) --- pkg/cmd/auth/status/status.go | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/auth/status/status.go b/pkg/cmd/auth/status/status.go index 4969f3d..b04e59f 100644 --- a/pkg/cmd/auth/status/status.go +++ b/pkg/cmd/auth/status/status.go @@ -94,6 +94,20 @@ func Run(ctx context.Context, opts *Options) error { opts.Hostname = h } + // C2/C3 (D-audit): both "no hosts configured" and "asked for a + // host that isn't configured" previously printed a message and + // returned nil — script-friendly false-pass risk (CI gating on + // `shithub auth status >/dev/null 2>&1` silently succeeded). + // Make both error so the non-zero exit matches the stderr line. + if len(hosts) == 0 { + return errors.New("auth: no hosts configured. Run `shithub auth login` to add one") + } + if opts.Hostname != "" { + if _, ok := hosts[opts.Hostname]; !ok { + return fmt.Errorf("auth: not logged into %s", opts.Hostname) + } + } + rows := collect(ctx, opts, hosts) if opts.Active { @@ -162,12 +176,10 @@ func collect(ctx context.Context, opts *Options, hosts config.Hosts) []HostStatu return out } -// writeHuman renders the table-of-hosts shape gh users expect. +// writeHuman renders the table-of-hosts shape gh users expect. The +// empty-rows case is now caught earlier in Run and returned as an +// error — see the C2/C3 guard there. func writeHuman(ios *iostreams.IOStreams, rows []HostStatus, showToken bool, ks config.KeyringStore, hosts config.Hosts) error { - if len(rows) == 0 { - fmt.Fprintln(ios.ErrOut, "auth: no hosts configured. Run `shithub auth login` to add one.") - return nil - } for i, r := range rows { if i > 0 { fmt.Fprintln(ios.Out) From 078a10b98d57b0296ea3fa41e0bb0a48f2ab4d27 Mon Sep 17 00:00:00 2001 From: espadonne Date: Sun, 17 May 2026 16:29:01 -0400 Subject: [PATCH 4/4] auth/status: regression tests for C2 + C3 exit codes --- pkg/cmd/auth/status/status_test.go | 34 +++++++++++++++++++++++++----- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/pkg/cmd/auth/status/status_test.go b/pkg/cmd/auth/status/status_test.go index 02fe2da..d7384da 100644 --- a/pkg/cmd/auth/status/status_test.go +++ b/pkg/cmd/auth/status/status_test.go @@ -23,13 +23,37 @@ func newOpts(t *testing.T) (*Options, *cmdutiltest.Factory) { }, tf } -func TestStatusEmptyHostsExitsZero(t *testing.T) { +// TestStatusEmptyHostsExitsNonZero is the C2 regression: previously +// `auth status` with no hosts configured exited 0 with a stdout/stderr +// message — CI gating on `shithub auth status >/dev/null` silently +// succeeded. Now it errors so the exit code matches the message. +func TestStatusEmptyHostsExitsNonZero(t *testing.T) { + opts, _ := newOpts(t) + err := Run(context.Background(), opts) + if err == nil { + t.Fatal("expected error when no hosts configured") + } + if !strings.Contains(err.Error(), "no hosts configured") { + t.Errorf("error should mention no hosts configured; got: %v", err) + } +} + +// TestStatusUnknownHostnameExitsNonZero is the C3 regression: passing +// --hostname for a host that isn't in hosts.yml said "no hosts +// configured" (misleading: there ARE hosts, just not that one) and +// exited 0. Now it errors with a hostname-specific message. +func TestStatusUnknownHostnameExitsNonZero(t *testing.T) { opts, tf := newOpts(t) - if err := Run(context.Background(), opts); err != nil { - t.Fatalf("Run: %v", err) + tf.Config.Hosts.Get("shithub.sh").User = "mf" + _ = tf.Config.Hosts.Save() + opts.Hostname = "example.com" + + err := Run(context.Background(), opts) + if err == nil { + t.Fatal("expected error when --hostname is not configured") } - if !strings.Contains(tf.ErrOut.String(), "no hosts configured") { - t.Errorf("expected empty-state hint, got %q", tf.ErrOut.String()) + if !strings.Contains(err.Error(), "not logged into example.com") { + t.Errorf("error should name the missing host; got: %v", err) } }