From f0bf3b24a67eba641e6eea310adecd55e2e49b3b Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Thu, 15 Oct 2026 06:02:45 +0200 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 | 58 +++++++++++++++++++++++++ internal/cli/unlockers.go | 6 +++ internal/cli/vault.go | 6 +++ internal/cli/version.go | 6 +++ 6 files changed, 92 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..176fb80 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( + strings.Fields("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( + strings.Fields("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..aa48d74 --- /dev/null +++ b/internal/cli/unknown_subcommand_test.go @@ -0,0 +1,58 @@ +package cli_test + +import ( + "fmt" + "os/exec" + "strings" + "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() + } + + for _, call := range []string{ + "vault improt", "unlocker foo", "version foo", "generate foo", + } { + parent, unknown, _ := strings.Cut(call, " ") + + output, exitCode := run(parent, unknown) + assert.Equal(t, 1, exitCode, "secret %s should fail:\n%s", call, output) + assert.Contains(t, output, fmt.Sprintf( + "unknown command %q for \"secret %s\"", unknown, parent)) + + output, exitCode = run(parent) + assert.Equal(t, 0, exitCode, + "secret %s should succeed:\n%s", parent, output) + assert.Contains(t, output, usageHeading, + "secret %s should print its help", parent) + } +} diff --git a/internal/cli/unlockers.go b/internal/cli/unlockers.go index 0d3078b..c64a8e8 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 6d292e3..dd22b93 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