Print usage only for a command called wrongly (closes #41)
check / check (push) Failing after 1s
check / check (push) Failing after 1s
A failed command printed the whole usage text after its error, burying it. The root command's PersistentPreRunE now turns usage off, so an error from running the command is printed once on its own. Wrong arity, an unknown flag, a bad flag value, a missing required flag and broken flag groups still get usage: cobra checks arguments and flag values before that hook but required flags and flag groups only after it, so the hook checks those two first. Root SilenceUsage was not used: in this cobra version it hides usage for argument and flag errors too. Cobra still prints the error; Entry is unchanged. Model: opus-5-5
This commit is contained in:
@@ -25,6 +25,15 @@ 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, missing required flag, or flags that
|
||||
break a flag group (mutually exclusive, required together, one
|
||||
required). The root command's `PersistentPreRunE` turns usage off.
|
||||
Cobra checks arguments and flag values before that hook but required
|
||||
flags and flag groups only after it, so the hook checks those two
|
||||
first. Root `SilenceUsage` would have hidden usage for all of these.
|
||||
- 2026-10-04: `secret get` keeps the secret in locked memory until it
|
||||
writes it out (https://git.eeqj.de/sneak/secret/issues/37):
|
||||
`Vault.GetSecret` and `Vault.GetSecretVersion` return a
|
||||
@@ -224,8 +233,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).
|
||||
|
||||
+23
-2
@@ -46,9 +46,30 @@ 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 flag values before this runs, but
|
||||
// checks required flags (ValidateRequiredFlags) and flag groups
|
||||
// (ValidateFlagGroups) only after it, so both are checked here
|
||||
// to keep usage for them. 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
|
||||
}
|
||||
|
||||
err = cmd.ValidateFlagGroups()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
cmd.SilenceUsage = true
|
||||
|
||||
return nil
|
||||
},
|
||||
}
|
||||
|
||||
secret.Debug("Adding subcommands to root command")
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user