From b5886c02cebcaab78bd027d980e4a21e0e5c8d9a Mon Sep 17 00:00:00 2001 From: sneak Date: Thu, 15 Oct 2026 01:51:40 +0000 Subject: [PATCH] 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 "" for "secret "`, 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 [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 | 25 ++++------ internal/cli/generate.go | 6 +++ internal/cli/unknown_subcommand_test.go | 64 +++++++++++++++++++++++++ internal/cli/unlockers.go | 6 +++ internal/cli/vault.go | 6 +++ internal/cli/version.go | 6 +++ 6 files changed, 98 insertions(+), 15 deletions(-) create mode 100644 internal/cli/unknown_subcommand_test.go diff --git a/internal/cli/create_vault_test.go b/internal/cli/create_vault_test.go index 33f5f6d..b633b54 100644 --- a/internal/cli/create_vault_test.go +++ b/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 diff --git a/internal/cli/generate.go b/internal/cli/generate.go index 8c8c645..eef79c6 100644 --- a/internal/cli/generate.go +++ b/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()) diff --git a/internal/cli/unknown_subcommand_test.go b/internal/cli/unknown_subcommand_test.go new file mode 100644 index 0000000..fc3e0b3 --- /dev/null +++ b/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) + } +} diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index 87b3d7e..5179521 100644 --- a/internal/cli/unlockers.go +++ b/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()) diff --git a/internal/cli/vault.go b/internal/cli/vault.go index 97a1a3a..0e2cd22 100644 --- a/internal/cli/vault.go +++ b/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()) diff --git a/internal/cli/version.go b/internal/cli/version.go index a4b8a8c..bf9a44c 100644 --- a/internal/cli/version.go +++ b/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