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