diff --git a/cmd/mesh-control/secret.go b/cmd/mesh-control/secret.go index 8370f55..a271502 100644 --- a/cmd/mesh-control/secret.go +++ b/cmd/mesh-control/secret.go @@ -31,13 +31,13 @@ func secretCommand(ctx context.Context, args []string) error { if len(args) == 0 || args[0] != "accept" { return errors.New("secret accept [--from ]") } + 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 [--from ]") } @@ -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 diff --git a/cmd/mesh-control/secret_test.go b/cmd/mesh-control/secret_test.go index f6ba77c..7753cf9 100644 --- a/cmd/mesh-control/secret_test.go +++ b/cmd/mesh-control/secret_test.go @@ -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 ") { + 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...) + } +}