D3d: CLI MIN cleanups (C18+C22+C23+C25+C26)

internal/secrets/client.go

@@ -9,6 +9,7 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
+	"strings"
 
 	"github.com/tenseleyFlow/shithub-cli/internal/api"
 )
@@ -225,22 +226,27 @@ func (c *Client) DeleteOrgVariable(ctx context.Context, org, name string) error
 	return c.api.REST(ctx, http.MethodDelete, path, nil, nil)
 }
 
-// validateName rejects names containing path separators / wildcards so
-// a malformed name can't escape the secret namespace via traversal.
-// Match GitHub's spec: alphanumeric + underscore, can't start with a
-// digit, can't start with GITHUB_.
+// validateName enforces GitHub Actions secret-name rules. Match gh /
+// GitHub: ^[A-Z_][A-Z0-9_]*$, can't start with GITHUB_, no
+// lowercase (C-audit C25). The pre-D3d implementation accepted
+// lowercase silently which then mismatched gh-compat scripts.
 func validateName(name string) error {
 	if name == "" {
 		return errors.New("secrets: name is required")
 	}
-	for _, r := range name {
+	if strings.HasPrefix(name, "GITHUB_") {
+		return fmt.Errorf("secrets: name %q is reserved (cannot start with GITHUB_)", name)
+	}
+	for i, r := range name {
 		switch {
 		case r >= 'A' && r <= 'Z':
-		case r >= 'a' && r <= 'z':
-		case r >= '0' && r <= '9':
 		case r == '_':
+		case r >= '0' && r <= '9':
+			if i == 0 {
+				return fmt.Errorf("secrets: name %q cannot start with a digit", name)
+			}
 		default:
-			return fmt.Errorf("secrets: invalid character %q in name %q (allowed: A-Z, a-z, 0-9, _)", r, name)
+			return fmt.Errorf("secrets: invalid character %q in name %q (allowed: A-Z, 0-9, _)", r, name)
 		}
 	}
 	return nil

internal/secrets/client_test.go

@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: AGPL-3.0-or-later
+
+package secrets
+
+import (
+	"strings"
+	"testing"
+)
+
+// TestValidateNameRejectsLowercase is the C-audit C25 regression:
+// gh-compat secret-name rules forbid lowercase. The pre-D3d code
+// accepted them silently.
+func TestValidateNameRejectsLowercase(t *testing.T) {
+	for _, bad := range []string{"lowercase", "Mixed_Case", "mostlyUpper"} {
+		err := validateName(bad)
+		if err == nil {
+			t.Errorf("validateName(%q) should reject lowercase", bad)
+		}
+	}
+}
+
+// TestValidateNameRejectsLeadingDigit catches the "name can't start
+// with a digit" rule documented in gh.
+func TestValidateNameRejectsLeadingDigit(t *testing.T) {
+	for _, bad := range []string{"1FOO", "9", "0_X"} {
+		err := validateName(bad)
+		if err == nil {
+			t.Errorf("validateName(%q) should reject leading digit", bad)
+		}
+	}
+}
+
+// TestValidateNameRejectsGithubPrefix: GITHUB_* names are reserved.
+func TestValidateNameRejectsGithubPrefix(t *testing.T) {
+	err := validateName("GITHUB_TOKEN")
+	if err == nil {
+		t.Fatal("validateName should reject GITHUB_ prefix")
+	}
+	if !strings.Contains(err.Error(), "reserved") {
+		t.Errorf("error should mention reserved; got: %v", err)
+	}
+}
+
+// TestValidateNameAccepts: confirm the legal shapes still pass.
+func TestValidateNameAccepts(t *testing.T) {
+	for _, good := range []string{"FOO", "A", "_PRIVATE", "DEPLOY_KEY_42", "X1"} {
+		if err := validateName(good); err != nil {
+			t.Errorf("validateName(%q) should accept; got: %v", good, err)
+		}
+	}
+}

pkg/cmd/alias/set.go

@@ -63,6 +63,11 @@ func setRun(_ context.Context, opts *setOptions) error {
 	if cfg.Aliases == nil {
 		cfg.Aliases = map[string]string{}
 	}
+	// C26: warn on overwrite so users notice when they typo over an
+	// existing alias. gh prints `! Changing alias X from Y to Z`.
+	if prev, ok := cfg.Aliases[opts.Name]; ok && prev != expansion {
+		fmt.Fprintf(opts.IO.ErrOut, "! Changing alias %s from %s to %s\n", opts.Name, prev, expansion)
+	}
 	cfg.Aliases[opts.Name] = expansion
 	if err := cfg.Save(); err != nil {
 		return fmt.Errorf("alias: save: %w", err)

pkg/cmd/cache/cache.go

@@ -27,6 +27,20 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command {
 	cmd := &cobra.Command{
 		Use:   "cache <command>",
 		Short: "Manage GitHub-Actions-compatible cache entries",
+		// C-audit C18: without an explicit Args/RunE on the parent,
+		// `shithub cache totally-not-a-real-subcmd` printed the
+		// parent's help and exited 0. CI scripts running an unknown
+		// subcommand against an older binary silently no-op'd. Reject
+		// any positional that didn't match a registered subcommand.
+		Args: func(c *cobra.Command, args []string) error {
+			if len(args) > 0 {
+				return fmt.Errorf("unknown subcommand %q for %q", args[0], c.CommandPath())
+			}
+			return nil
+		},
+		RunE: func(c *cobra.Command, _ []string) error {
+			return c.Help()
+		},
 	}
 	cmd.AddCommand(cachelist.NewCmd(f))
 	cmd.AddCommand(newDeferredStub("delete", "Delete a cache entry (deferred until shithub-S41g)"))

pkg/cmd/completion/completion.go

@@ -70,7 +70,11 @@ The shell may be passed positionally (gh-style) or via -s/--shell.
 // Run emits the completion script for opts.Shell to opts.IO.Out.
 func Run(_ context.Context, root *cobra.Command, opts *options) error {
 	if opts.Shell == "" {
-		return errors.New("completion: --shell is required (one of: " + listShells() + ")")
+		// C-audit C22: message text used to say "--shell is required",
+		// which conflicts with the help shape (`completion <shell>`).
+		// Match the help: positional is preferred, --shell is a script
+		// alias that lands at the same destination.
+		return errors.New("completion: shell is required (one of: " + listShells() + ")")
 	}
 	return generate(root, opts.IO.Out, opts.Shell, opts.NoDesc)
 }

pkg/cmd/repo/list/list.go

@@ -47,6 +47,10 @@ type options struct {
 	Sort      string
 	Direction string
 	Limit     int
+	Web       bool
+
+	// Opener is overridden in tests.
+	Opener func(url string) error
 
 	Exporter output.Options
 }
@@ -58,6 +62,7 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command {
 		HTTPClient:  f.HTTPClient,
 		DefaultHost: f.DefaultHost,
 		Limit:       DefaultLimit,
+		Opener:      defaultOpener,
 	}
 	cmd := &cobra.Command{
 		Use:   "list [<owner>]",
@@ -81,7 +86,10 @@ func NewCmd(f *cmdutil.Factory) *cobra.Command {
 	cmd.Flags().StringVar(&opts.Sort, "sort", "updated", "sort field: {created|updated|pushed|name}")
 	cmd.Flags().StringVar(&opts.Direction, "order", "desc", "sort order: {asc|desc}")
 	cmd.Flags().IntVarP(&opts.Limit, "limit", "L", DefaultLimit, "maximum number of repositories to list (max 1000)")
+	cmd.Flags().BoolVarP(&opts.Web, "web", "w", false, "open the repository list in a browser")
 	output.AddFlags(cmd, &opts.Exporter)
+	// TODO(D3a-followup): add output.MarkWebMutuallyExclusive(cmd)
+	// once D3a (output-web-mutex helper) lands on trunk.
 	return cmd
 }
 
@@ -104,6 +112,33 @@ func Run(ctx context.Context, opts *options) error {
 	if host == "" && opts.DefaultHost != nil {
 		host = opts.DefaultHost()
 	}
+
+	// C23: --web sends the user to the host's repo-list page for the
+	// supplied (or authenticated) user/org. gh has this flag; we
+	// were the odd ones out.
+	if opts.Web {
+		who := opts.Owner
+		if who == "" {
+			// Best-effort: defer to the host's "your repos" page;
+			// the server's UI handles the auth redirect.
+			who = "?tab=repositories"
+		}
+		base := "https://"
+		if host != "" {
+			base += host
+		}
+		var url string
+		if strings.HasPrefix(who, "?") {
+			url = base + "/" + who
+		} else {
+			url = base + "/" + who + "?tab=repositories"
+		}
+		fmt.Fprintf(opts.IO.ErrOut, "Opening %s in your browser.\n", url)
+		if opts.Opener == nil {
+			return nil
+		}
+		return opts.Opener(url)
+	}
 	client, err := opts.HTTPClient(host)
 	if err != nil {
 		return err
@@ -248,3 +283,7 @@ func truncate(s string, n int) string {
 	}
 	return string(runes[:n-1]) + "…"
 }
+
+// defaultOpener is the prod-time no-op; iostreams will plumb a real
+// browser launcher in a later sprint. Tests override.
+var defaultOpener = func(_ string) error { return nil }