Unknown subcommands of vault, unlocker, version and generate fail (closes #183)

Given a subcommand they do not have, `secret vault`, `secret unlocker`,
`secret version` and `secret generate` printed their help and exited 0,
so a mistyped command looked like a successful one. Each now exits 1
with `unknown command "<name>" for "secret <parent>"`, and with no
arguments still prints its help.

Cobra checks a command's Args only when it has a RunE, so each parent
gets `Args: cobra.NoArgs` and a RunE that prints its help. Args alone
changes nothing. TestNoVaultImportCommand required the old answer to
`secret vault import` and now requires the error.

Judgement call: their help now shows a `secret <parent> [flags]` usage
line, which cobra adds for any command that runs.
Judgement call: `secret vault help` now fails as an unknown subcommand;
`secret help vault` and `secret vault --help` still work.

Model: opus-5-5

internal/cli/create_vault_test.go

@@ -144,25 +144,20 @@ func TestVaultCreationLeavesNoSecretInEnvironment(t *testing.T) {
 // TestNoVaultImportCommand is a regression test for
 // https://git.eeqj.de/sneak/secret/issues/139: `secret vault import`
 // refused every vault that `secret vault create` makes, because each has a
-// long-term key. The command is gone, so cobra answers it with the help of
-// `secret vault`, as it does any subcommand of `secret vault` it does not
-// know.
+// long-term key. The command is gone, so it fails as any subcommand of
+// `secret vault` that does not exist does.
 func TestNoVaultImportCommand(t *testing.T) {
 	t.Setenv(secret.EnvStateDir, t.TempDir())
+	t.Setenv(secret.EnvMnemonic, testMnemonic)
+	t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
 
-	run := func(args ...string) string {
-		t.Setenv(secret.EnvMnemonic, testMnemonic)
-		t.Setenv(secret.EnvUnlockPassphrase, testPassphrase)
+	_, err := cli.ExecuteCommandInProcess(
+		[]string{"vault", "create", "work"}, "", nil)
+	require.NoError(t, err)
 
-		output, err := cli.ExecuteCommandInProcess(args, "", nil)
-		require.NoError(t, err, "%v", args)
-
-		return output
-	}
-
-	run("vault", "create", "work")
-
-	require.Equal(t, run("vault"), run("vault", "import", "work"))
+	_, err = cli.ExecuteCommandInProcess(
+		[]string{"vault", "import", "work"}, "", nil)
+	require.EqualError(t, err, `unknown command "import" for "secret vault"`)
 }
 
 // TestStopAtPassphrasePromptLeavesNothing is a regression test for the

internal/cli/generate.go

@@ -29,6 +29,12 @@ func newGenerateCmd() *cobra.Command {
 		Use:   "generate",
 		Short: "Generate random data",
 		Long:  `Generate various types of random data including mnemonics and secrets.`,
+		// Cobra checks Args only on a command that has a RunE; without
+		// one, an unknown subcommand prints this help and exits 0.
+		Args: cobra.NoArgs,
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			return cmd.Help()
+		},
 	}
 
 	cmd.AddCommand(newGenerateMnemonicCmd())

internal/cli/unknown_subcommand_test.go

@@ -0,0 +1,64 @@
+package cli_test
+
+import (
+	"fmt"
+	"os/exec"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+	"sneak.berlin/go/secret/internal/secret"
+)
+
+// TestUnknownSubcommandFails is a regression test for
+// https://git.eeqj.de/sneak/secret/issues/183: given a subcommand it does not
+// have, `secret vault`, `secret unlocker`, `secret version` and `secret
+// generate` printed their help and exited 0, so a script could not tell a
+// mistyped command from a successful one. Each runs here as the built binary
+// and must exit 1 naming the subcommand, as an unknown command does at the
+// top level, and must still print its help and exit 0 when given nothing.
+func TestUnknownSubcommandFails(t *testing.T) {
+	t.Parallel()
+
+	stateDir := t.TempDir()
+
+	// run runs the built binary with args and returns what it printed and
+	// its exit code
+	run := func(args ...string) (string, int) {
+		//nolint:gosec // G204: test executes the freshly built secret binary
+		cmd := exec.CommandContext(t.Context(), secretBinaryPath(t), args...)
+		cmd.Env = []string{secret.EnvStateDir + "=" + stateDir}
+
+		output, err := cmd.CombinedOutput()
+		if err != nil {
+			var exitErr *exec.ExitError
+			require.ErrorAs(t, err, &exitErr, "secret %v did not run", args)
+		}
+
+		return string(output), cmd.ProcessState.ExitCode()
+	}
+
+	tests := []struct {
+		parent  string
+		unknown string
+	}{
+		{parent: "vault", unknown: "improt"},
+		{parent: "unlocker", unknown: "foo"},
+		{parent: "version", unknown: "foo"},
+		{parent: "generate", unknown: "foo"},
+	}
+
+	for _, tt := range tests {
+		output, exitCode := run(tt.parent, tt.unknown)
+		assert.Equal(t, 1, exitCode,
+			"secret %s %s should fail:\n%s", tt.parent, tt.unknown, output)
+		assert.Contains(t, output, fmt.Sprintf(
+			"unknown command %q for \"secret %s\"", tt.unknown, tt.parent))
+
+		output, exitCode = run(tt.parent)
+		assert.Equal(t, 0, exitCode,
+			"secret %s should succeed:\n%s", tt.parent, output)
+		assert.Contains(t, output, usageHeading,
+			"secret %s should print its help", tt.parent)
+	}
+}

internal/cli/unlockers.go

@@ -126,6 +126,12 @@ func newUnlockerCmd() *cobra.Command {
 		Use:   "unlocker",
 		Short: "Manage unlockers",
 		Long:  `Create, list, and remove unlockers for the current vault.`,
+		// Cobra checks Args only on a command that has a RunE; without
+		// one, an unknown subcommand prints this help and exits 0.
+		Args: cobra.NoArgs,
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			return cmd.Help()
+		},
 	}
 
 	cmd.AddCommand(newUnlockerListCmd())

internal/cli/vault.go

@@ -29,6 +29,12 @@ func newVaultCmd() *cobra.Command {
 		Use:   "vault",
 		Short: "Manage vaults",
 		Long:  `Create, list, and select vaults for organizing secrets.`,
+		// Cobra checks Args only on a command that has a RunE; without
+		// one, an unknown subcommand prints this help and exits 0.
+		Args: cobra.NoArgs,
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			return cmd.Help()
+		},
 	}
 
 	cmd.AddCommand(newVaultListCmd())

internal/cli/version.go

@@ -43,6 +43,12 @@ func VersionCommands(cli *Instance) *cobra.Command {
 		Short: "Manage secret versions",
 		Long: "Commands for managing secret versions including listing, " +
 			"promoting, and retrieving specific versions",
+		// Cobra checks Args only on a command that has a RunE; without
+		// one, an unknown subcommand prints this help and exits 0.
+		Args: cobra.NoArgs,
+		RunE: func(cmd *cobra.Command, _ []string) error {
+			return cmd.Help()
+		},
 	}
 
 	// List versions command