Cobra prints full usage on internal failures, burying the actual error #41

New Issue

clawbot · 2026-08-09T03:41:34+02:00

clawbot commented

2026-08-09 03:41:34 +02:00

From the 1.0 security survey.

Problem

internal/cli/root.go:36-38 sets both flags off explicitly:

SilenceUsage:  false,
SilenceErrors: false,
// Ensure usage is shown after errors

No subcommand overrides this. So every RunE that returns an internal error — a failed decrypt, a missing unlocker, an I/O error, a wrong passphrase — dumps the entire usage block after the error message.

Two consequences. The mild one is noise. The one that matters for a secrets tool: the actual error is pushed far up the scrollback behind a wall of flag documentation, so a user who mistypes a passphrase and one who has a corrupted or unreadable vault get visually near-identical output, and the second is easy to miss. Usage text is the correct response to "you called this wrong" and actively unhelpful for "your key material could not be read".

Definition of done

Usage is printed only for argument and flag errors — wrong arity, unknown flag, bad flag value.
Usage is not printed for errors returned from RunE bodies.
The error message itself is still shown exactly once, and the exit code stays non-zero. A common mistake when setting SilenceErrors: true is that nothing prints the error afterward, or that it prints twice.
Tests cover both sides: a wrong-arity invocation still shows usage, and a RunE failure shows the error without usage.
make check green. TODO.md updated in the same commit.

Implementation requirements

The standard approach is SilenceUsage: true on the root command, which suppresses usage for RunE errors while cobra still prints usage for argument and flag parsing errors it raises itself. Verify that is actually the observed behavior for this cobra version rather than assuming it — test both paths.
Decide deliberately about SilenceErrors. Leaving it false lets cobra print the error, which is usually what you want; setting it true means Entry() must print the error itself. Do not set it true without adding that print, or failures will exit non-zero silently — which in a secrets tool is a genuinely dangerous outcome, since a script would see a failure with no indication of what went wrong.
Coordinate with the memguard purge issue (#35), which also restructures Entry(). If both land, keep the error-printing and the purge ordering coherent: print the error, then purge, then exit.
Remove the now-incorrect // Ensure usage is shown after errors comment.

From the 1.0 security survey. ## Problem `internal/cli/root.go:36-38` sets both flags off explicitly: ```go SilenceUsage: false, SilenceErrors: false, // Ensure usage is shown after errors ``` No subcommand overrides this. So every `RunE` that returns an internal error — a failed decrypt, a missing unlocker, an I/O error, a wrong passphrase — dumps the entire usage block after the error message. Two consequences. The mild one is noise. The one that matters for a secrets tool: the actual error is pushed far up the scrollback behind a wall of flag documentation, so a user who mistypes a passphrase and one who has a **corrupted or unreadable vault** get visually near-identical output, and the second is easy to miss. Usage text is the correct response to "you called this wrong" and actively unhelpful for "your key material could not be read". ## Definition of done - Usage is printed **only** for argument and flag errors — wrong arity, unknown flag, bad flag value. - Usage is **not** printed for errors returned from `RunE` bodies. - The error message itself is still shown exactly once, and the exit code stays non-zero. A common mistake when setting `SilenceErrors: true` is that nothing prints the error afterward, or that it prints twice. - Tests cover both sides: a wrong-arity invocation still shows usage, and a `RunE` failure shows the error without usage. - `make check` green. `TODO.md` updated in the same commit. ## Implementation requirements - The standard approach is `SilenceUsage: true` on the root command, which suppresses usage for `RunE` errors while cobra still prints usage for argument and flag parsing errors it raises itself. Verify that is actually the observed behavior for this cobra version rather than assuming it — test both paths. - Decide deliberately about `SilenceErrors`. Leaving it `false` lets cobra print the error, which is usually what you want; setting it `true` means `Entry()` must print the error itself. Do not set it `true` without adding that print, or failures will exit non-zero silently — which in a secrets tool is a genuinely dangerous outcome, since a script would see a failure with no indication of what went wrong. - Coordinate with the memguard purge issue (#35), which also restructures `Entry()`. If both land, keep the error-printing and the purge ordering coherent: print the error, then purge, then exit. - Remove the now-incorrect `// Ensure usage is shown after errors` comment.

clawbot added this to the 1.0.0 milestone 2026-08-09 03:41:34 +02:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: sneak/secret#41