diff --git a/TODO.md b/TODO.md index 99d609e..0fb5321 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,13 @@ Bring the repo into policy compliance in one commit: # Completed Steps +- 2026-10-04: A failed command prints its error once, without the usage + text after it (https://git.eeqj.de/sneak/secret/issues/41). Usage is + still printed for a command called wrongly: wrong number of arguments, + unknown flag, bad flag value or missing required flag. The root + command's `PersistentPreRunE` turns usage off once cobra has checked + the arguments and flags; root `SilenceUsage` would have hidden usage + for those too. - 2026-10-04: `.gitignore` is the org's standard file, which ignores `.env`, `.env.*`, `*.pem` and `*.key` and editor and OS files, plus this repo's `/secret`, `*.log`, `*.test` and `settings.local.json` @@ -203,8 +210,6 @@ Bring the repo into policy compliance in one commit: 209-216); non-constant-time public key compare (vault.go:95-100). - High priority: - Secure temporary file handling and cleanup. - - Print cobra usage only for argument errors, not internal - failures. - Initialize a default unlock key at vault creation. - Confirmation prompts for destructive operations (keys rm, vault deletion). diff --git a/internal/cli/root.go b/internal/cli/root.go index c911991..6088dc4 100644 --- a/internal/cli/root.go +++ b/internal/cli/root.go @@ -46,9 +46,24 @@ func newRootCmd() *cobra.Command { Short: "A simple secrets manager", Long: `A simple secrets manager to store and retrieve sensitive ` + `information securely.`, - // Ensure usage is shown after errors - SilenceUsage: false, + // Cobra prints the error a command returns; Entry does not. SilenceErrors: false, + // Usage belongs only to a command called wrongly. Cobra has + // checked its arguments and flags before this runs, except for + // required flags, which are checked here so they still get usage. + // An error after that comes from running the command, and usage + // would only bury it. A subcommand that sets its own + // PersistentPreRun replaces this one. + PersistentPreRunE: func(cmd *cobra.Command, _ []string) error { + err := cmd.ValidateRequiredFlags() + if err != nil { + return err + } + + cmd.SilenceUsage = true + + return nil + }, } secret.Debug("Adding subcommands to root command") diff --git a/internal/cli/usage_test.go b/internal/cli/usage_test.go new file mode 100644 index 0000000..185cd9d --- /dev/null +++ b/internal/cli/usage_test.go @@ -0,0 +1,46 @@ +package cli_test + +import ( + "strings" + "testing" + + "git.eeqj.de/sneak/secret/internal/cli" + "git.eeqj.de/sneak/secret/internal/secret" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// usageHeading starts the usage text cobra prints after an error. +const usageHeading = "Usage:" + +// A command called wrongly gets usage after its error; a command that +// fails while running gets its error alone. Either way the command fails +// and its error is shown exactly once. +// +//nolint:paralleltest // executes the CLI in-process and sets the environment +func TestUsageOnlyForCallErrors(t *testing.T) { + // No vault in the state directory, so `get x` fails while running. + env := map[string]string{secret.EnvStateDir: t.TempDir()} + + tests := []struct { + call string + wantUsage bool + }{ + {call: "get", wantUsage: true}, + {call: "get x y", wantUsage: true}, + {call: "get --no-such-flag x", wantUsage: true}, + {call: "generate secret x --length abc", wantUsage: true}, + {call: "import x", wantUsage: true}, + {call: "get x", wantUsage: false}, + } + + for _, tt := range tests { + output, err := cli.ExecuteCommandInProcess(strings.Fields(tt.call), "", env) + require.Error(t, err, "%q should fail", tt.call) + + assert.Equal(t, 1, strings.Count(output, err.Error()), + "%q should show its error once:\n%s", tt.call, output) + assert.Equal(t, tt.wantUsage, strings.Contains(output, usageHeading), + "usage shown for %q:\n%s", tt.call, output) + } +}