Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 21 additions & 19 deletions cmd/kosli/attestGeneric_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,10 +42,24 @@ func (suite *AttestGenericCommandTestSuite) TestAttestGenericCmd() {
golden: "Error: accepts at most 1 arg(s), received 2 [foo bar]\n",
},
{
wantError: true,
name: "fails when artifact-name is provided and there is an --artifact-type flag and --compliant is not set with =",
cmd: fmt.Sprintf("attest generic testdata/file1 %s --artifact-type file --compliant false", suite.defaultKosliArguments),
golden: "Error: accepts at most 1 arg(s), received 2 [testdata/file1 false]\nSee https://docs.kosli.com//faq/#boolean-flags\n",
name: "reports a non-compliant attestation when --compliant is given with a space",
cmd: fmt.Sprintf("attest generic testdata/file1 --artifact-type file --name foo --commit HEAD --origin-url http://example.com --compliant false %s", suite.defaultKosliArguments),
golden: "generic attestation 'foo' is reported to trail: test-123\n",
},
{
name: "passes through a non-boolean flag whose value is the literal false",
cmd: fmt.Sprintf("attest generic testdata/file1 --artifact-type file --name false --commit HEAD --origin-url http://example.com %s", suite.defaultKosliArguments),
golden: "generic attestation 'false' is reported to trail: test-123\n",
},
{
name: "reports a non-compliant attestation when the -C shorthand is given with a space",
cmd: fmt.Sprintf("attest generic testdata/file1 --artifact-type file --name foo --commit HEAD --origin-url http://example.com -C false %s", suite.defaultKosliArguments),
golden: "generic attestation 'foo' is reported to trail: test-123\n",
},
{
name: "reports a non-compliant attestation when --compliant is given with =",
cmd: fmt.Sprintf("attest generic testdata/file1 --artifact-type file --name foo --commit HEAD --origin-url http://example.com --compliant=false %s", suite.defaultKosliArguments),
golden: "generic attestation 'foo' is reported to trail: test-123\n",
},
{
wantError: true,
Expand All @@ -55,28 +69,16 @@ func (suite *AttestGenericCommandTestSuite) TestAttestGenericCmd() {
},
{
wantError: true,
name: "fails when artifact-name is provided (as _unused_ boolean 'space' arg) and there is no --artifact-type and no --fingerprint",
cmd: fmt.Sprintf("attest generic %s --compliant false", suite.defaultKosliArguments),
golden: "Error: --artifact-type or --fingerprint must be specified when artifact name ('false') argument is supplied.\nSee https://docs.kosli.com//faq/#boolean-flags\nUsage: kosli attest generic [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n",
name: "links to the boolean flags FAQ when a stray true|false argument remains",
cmd: fmt.Sprintf("attest generic foo false --artifact-type file --name bar %s", suite.defaultKosliArguments),
golden: "Error: accepts at most 1 arg(s), received 2 [foo false]\nSee https://docs.kosli.com//faq/#boolean-flags\n",
},
{
wantError: true,
name: "fails when artifact-name is provided and there is no --artifact-type",
cmd: fmt.Sprintf("attest generic wibble %s", suite.defaultKosliArguments),
golden: "Error: --artifact-type or --fingerprint must be specified when artifact name ('wibble') argument is supplied.\nUsage: kosli attest generic [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n",
},
{
wantError: true,
name: "fails when there are extra args and gives custom help message when an argument is true|false",
cmd: fmt.Sprintf("attest generic foo -t file %s --compliant false", suite.defaultKosliArguments),
golden: "Error: accepts at most 1 arg(s), received 2 [foo false]\nSee https://docs.kosli.com//faq/#boolean-flags\n",
},
{
wantError: true,
name: "fails when there are extra args and gives custom help message when an argument is true|false",
cmd: fmt.Sprintf("attest generic %s --compliant false", suite.defaultKosliArguments),
golden: "Error: --artifact-type or --fingerprint must be specified when artifact name ('false') argument is supplied.\nSee https://docs.kosli.com//faq/#boolean-flags\nUsage: kosli attest generic [IMAGE-NAME | FILE-PATH | DIR-PATH] [flags]\n",
},
{
wantError: true,
name: "fails when both --fingerprint and --artifact-type",
Expand Down
6 changes: 6 additions & 0 deletions cmd/kosli/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,13 @@ func enrichError(cmd *cobra.Command, err error) error {
return fmt.Errorf("[%s] %w", strings.Join(parts, " "), err)
}

// innerMain runs cmd against args (args[0] being the program name) and turns
// the outcome into the process-level error, printing the update notice on the
// --version path and reporting errors in the friendliest available form.
func innerMain(cmd *cobra.Command, args []string) error {
if len(args) > 1 {
cmd.SetArgs(normalizeBoolFlagArgs(cmd, args[1:]))
}
executedCmd, err := cmd.ExecuteC()
if err == nil {
// Cobra handles --version internally and bypasses all hooks, so we print
Expand Down
7 changes: 7 additions & 0 deletions cmd/kosli/multiHost.go
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,13 @@ func getMultiOpts() MultiOpts {
cmd.SetOut(writer)
cmd.SetErr(writer)

// Parse the same normalized args innerMain will run with, so a boolean flag
// written in the space form (eg --debug false) does not leave a stray
// positional argument that fails arg validation and empties MultiOpts.
if len(os.Args) > 1 {
cmd.SetArgs(normalizeBoolFlagArgs(cmd, os.Args[1:]))
}

fakeError := errors.New("")

cmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error {
Expand Down
24 changes: 24 additions & 0 deletions cmd/kosli/multiHost_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,30 @@ func (suite *MultiHostTestSuite) TestRunDoubledHost() {
}
}

// TestRunDoubledHostAcceptsSpaceSeparatedBoolFlag checks that a boolean flag
// written in the space form is honoured on the multi-host path too. --debug is
// false here, so the output must be the bare fingerprint: no per-host [debug]
// prefix and no secondary-call output. fingerprint is used because it needs no
// server, so the two hosts are never contacted.
func (suite *MultiHostTestSuite) TestRunDoubledHostAcceptsSpaceSeparatedBoolFlag() {
args := []string{
"kosli", "fingerprint", "testdata/person-schema.json",
"--artifact-type", "file",
"--debug", "false",
fmt.Sprintf("--host=%s,%s", localHost, localHost),
fmt.Sprintf("--api-token=%s,%s", apiToken, apiToken),
fmt.Sprintf("--org=%s", orgName),
}
want := []string{"1bef738d0bb1e690500f99a5b57d958caf3a5eb3e00d9012e1f4369fc6812e01", ""}

defer func(original []string) { os.Args = original }(os.Args)
os.Args = args
output, err := runMultiHost(args)

assert.Equal(suite.T(), error(nil), err)
assert.Equal(suite.T(), "", diff(want, strings.Split(output, "\n")))
}

func (suite *MultiHostTestSuite) TestRunTripledHost() {

multiHost := fmt.Sprintf("--host=%s,%s,%s", localHost, localHost, localHost)
Expand Down
91 changes: 91 additions & 0 deletions cmd/kosli/normalizeBoolFlagArgs.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package main

import (
"github.com/spf13/cobra"
"github.com/spf13/pflag"
)

// normalizeBoolFlagArgs rewrites boolean flags written in the space form
// ("--compliant false") into the "=" form ("--compliant=false"), which is the
// only form pflag accepts for an explicitly-valued boolean flag. All other
// tokens are returned unchanged, as is everything following a "--" terminator.
//
// The rewrite is positional: it matches on the tokens themselves and does not
// track which of them pflag will consume as the value of an earlier flag. Two
// pathological inputs are therefore read differently from the way pflag reads
// them, and both are accepted. A positional argument literally named "true" or
// "false" directly after a bare boolean flag becomes that flag's value. And in
// "--name --compliant false", where pflag gives --name the value "--compliant"
// and leaves "false" positional, this rewrite instead produces
// "--compliant=false". Kosli positionals are artifact names, fingerprints and
// file paths, and flag values are not flag tokens, so both inputs are far
// enough outside real usage to be worth the plain forms this rescues.
// The rewrite runs in two passes because the flags in scope depend on where a
// token sits. A flag before the subcommand can only be one of root's own, and
// root.Find cannot resolve the command while such a flag is still in the space
// form: the value is left as a stray positional, which cobra reads as an unknown
// subcommand. The first pass therefore works from root's flags alone, which is
// what lets the second pass resolve the command and reach the flags declared on
// it. Joining is idempotent, since a joined token no longer matches any flag
// token, so the passes cannot rewrite the same token twice.
func normalizeBoolFlagArgs(root *cobra.Command, args []string) []string {
args = joinBoolFlagValues(boolFlagTokens(root.LocalFlags()), args)
cmd, _, err := root.Find(args)
if err != nil {
return args
}
return joinBoolFlagValues(boolFlagTokens(cmd.Flags()), args)
}

// joinBoolFlagValues rewrites every "<flag> <literal>" pair in args, where flag
// is a token in boolFlags and literal is a boolean literal, into the single
// "<flag>=<literal>" token. All other tokens are returned unchanged, as is
// everything following a "--" terminator.
func joinBoolFlagValues(boolFlags map[string]bool, args []string) []string {
normalized := make([]string, 0, len(args))
for i := 0; i < len(args); i++ {
if args[i] == "--" {
// pflag stops parsing flags here, so everything left is a
// positional argument and must be passed through untouched.
normalized = append(normalized, args[i:]...)
break
}
if boolFlags[args[i]] && i+1 < len(args) && isBoolLiteral(args[i+1]) {
Comment thread
JonJagger marked this conversation as resolved.
normalized = append(normalized, args[i]+"="+args[i+1])
i++
continue
}
normalized = append(normalized, args[i])
}
return normalized
}

// boolFlagTokens returns the command-line tokens ("--compliant", "-C") of every
// boolean flag in flags.
//
// Shorthands are listed individually, so a grouped token such as "-qC" is not a
// key here and "-qC false" is left alone: it keeps failing with the arg-count
// error and its link to the boolean flags FAQ. That is a deliberate choice.
// Grouping raises questions this rewrite has no good answer to, such as which
// member of the group the value belongs to and what to do when a group mixes
// boolean and non-boolean shorthands. Rescuing the plain forms is worth a
// low-level rewrite; guessing intent inside a grouped token is not.
func boolFlagTokens(flags *pflag.FlagSet) map[string]bool {
tokens := map[string]bool{}
flags.VisitAll(func(flag *pflag.Flag) {
if flag.Value.Type() != "bool" {
return
}
tokens["--"+flag.Name] = true
if flag.Shorthand != "" {
tokens["-"+flag.Shorthand] = true
}
})
return tokens
}

// isBoolLiteral reports whether token is one of the two values a rewritten
// boolean flag may be given.
func isBoolLiteral(token string) bool {
return token == "true" || token == "false"
}
Loading
Loading