secret accept: see the flag that comes after the arguments
The command refused every real invocation. Go's flag package stops parsing at the first non-flag argument, so with the positionals first — the order that reads correctly — `--from -` stayed among them and the count check rejected it. The host's own parser carries a note about this exact fault, and the version it describes is worse: there a flag somebody passed was silently ignored and the command succeeded anyway. This one at least refused. The tests did not catch it because every case in them was a rejection. The command was broken in the only way that matters — it refused what it is for — and the suite was green. The lab found it at the first call. Two tests now: the helper, and the command itself with a --from naming a file that is not there, so the complaint must be about the file rather than about usage. The second exists because injecting against the first stayed silent: testing the helper alone left the command free to ignore it entirely.
This commit is contained in:
@@ -31,13 +31,13 @@ func secretCommand(ctx context.Context, args []string) error {
|
||||
if len(args) == 0 || args[0] != "accept" {
|
||||
return errors.New("secret accept <node> <module> <name> [--from <file>]")
|
||||
}
|
||||
rest, flags := split(args[1:])
|
||||
set := flag.NewFlagSet("secret accept", flag.ContinueOnError)
|
||||
from := set.String("from", "",
|
||||
"read the value from this file instead of asking (use - for standard input)")
|
||||
if err := set.Parse(args[1:]); err != nil {
|
||||
if err := set.Parse(flags); err != nil {
|
||||
return err
|
||||
}
|
||||
rest := set.Args()
|
||||
if len(rest) != 3 {
|
||||
return errors.New("secret accept <node> <module> <name> [--from <file>]")
|
||||
}
|
||||
@@ -69,6 +69,23 @@ func secretCommand(ctx context.Context, args []string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// split separates what this command is about from how it was asked.
|
||||
//
|
||||
// **Because the standard library stops parsing at the first non-flag argument.** With the
|
||||
// positionals first — which is the order that reads correctly — everything after them is left
|
||||
// sitting in the arguments, so `secret accept a b c --from -` arrives as five positionals and the
|
||||
// flag is never seen. The host's own parser carries the same note, and the fault it names is
|
||||
// worse than this one: there, a flag somebody passed was silently ignored and the command
|
||||
// succeeded anyway.
|
||||
func split(args []string) (positional, flags []string) {
|
||||
for i, arg := range args {
|
||||
if strings.HasPrefix(arg, "-") {
|
||||
return args[:i], args[i:]
|
||||
}
|
||||
}
|
||||
return args, nil
|
||||
}
|
||||
|
||||
// asSupplied is the value with its line ending removed and nothing else.
|
||||
//
|
||||
// **A file has a trailing newline and a password does not**, so the ending goes — a credential
|
||||
|
||||
@@ -3,6 +3,7 @@ package main
|
||||
import (
|
||||
"os"
|
||||
"path/filepath"
|
||||
"strings"
|
||||
"testing"
|
||||
)
|
||||
|
||||
@@ -67,3 +68,44 @@ func TestTheArgumentsAreRequired(t *testing.T) {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// The invocation that actually gets used, which the first version of these tests never tried.
|
||||
//
|
||||
// Every case here was a rejection, so the command was broken in the one way that matters — it
|
||||
// refused what it is for — and the tests were green. The lab found it at the first call.
|
||||
func TestTheArgumentsAndTheFlagAreBothSeen(t *testing.T) {
|
||||
rest, flags := split([]string{"anchor", "umami", "database", "--from", "-"})
|
||||
assert(t, len(rest) == 3, "the three positionals were not kept: %v", rest)
|
||||
assert(t, len(flags) == 2, "the flag was not separated: %v", flags)
|
||||
|
||||
// And with no flag at all, which is the interactive form.
|
||||
rest, flags = split([]string{"anchor", "umami", "database"})
|
||||
assert(t, len(rest) == 3 && len(flags) == 0, "%v / %v", rest, flags)
|
||||
|
||||
// A flag before the positionals still works, because somebody will write it that way.
|
||||
rest, flags = split([]string{"--from", "/tmp/x"})
|
||||
assert(t, len(rest) == 0 && len(flags) == 2, "%v / %v", rest, flags)
|
||||
}
|
||||
|
||||
// And the wiring, not just the helper.
|
||||
//
|
||||
// Testing `split` alone left the command able to ignore it entirely — removing the call changed
|
||||
// no test. This reaches secretCommand: with a `--from` naming a file that is not there, the
|
||||
// complaint must be about the file. A complaint about usage would mean the flag was never seen.
|
||||
func TestTheCommandItselfSeesTheFlag(t *testing.T) {
|
||||
err := secretCommand(t.Context(),
|
||||
[]string{"accept", "anchor", "umami", "database", "--from", "/nonexistent/nowhere"})
|
||||
if err == nil {
|
||||
t.Fatal("a missing file was accepted")
|
||||
}
|
||||
if strings.Contains(err.Error(), "secret accept <node>") {
|
||||
t.Fatalf("the command did not see its flag and complained about usage instead: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
func assert(t *testing.T, ok bool, format string, args ...any) {
|
||||
t.Helper()
|
||||
if !ok {
|
||||
t.Fatalf(format, args...)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user