Skip to content

cli: argument validation gaps remain after #334 (plugin, plugin install, convert) #345

Description

@jbonofre

Description

#334 / #335 fixed ossie plugin list, but the same "silently ignore positional arguments and exit 0" defect remains in three other places, including one level up on the plugin parent command itself.

The parent case is the most likely to be hit, since a typo'd subcommand is more common than a stray argument to a valid one, and it is easy to assume it was covered by #334.

Important: the one-line fix from #335 does not work on the parent command. See "Cause" below before attempting it.

Steps to reproduce

Against main with #335 merged:

ossie plugin bogus;               echo $?   # unknown subcommand
ossie plugin help list;           echo $?   # wrong help, still succeeds
ossie plugin install a b c;       echo $?   # extra positional args ignored
ossie plugin install;             echo $?   # no name and no --all
ossie convert --from x --input y extra; echo $?   # trailing arg ignored

Actual behavior

All five exit 0. ossie plugin bogus prints the plugin help to STDOUT and reports success; the rest print not yet implemented and report success, discarding the arguments.

Expected behavior

Each should reject the invocation with a non-zero exit status and show usage, consistent with plugin list after #335 and with plugin remove.

Cause

Two different causes, which is why one fix does not cover all of them:

plugin install (cli/cmd/plugin/install.go:25) and convert (cli/cmd/convert.go:25) simply declare no Args validator. Cobra treats a nil validator as accepting arbitrary arguments — the same root cause as #334.

plugin (cli/cmd/plugin/plugin.go:23) is different. Adding Args: cobra.NoArgs to Cmd has no effect, verified by building it: because the parent has no Run/RunE, cobra's execute() returns flag.ErrHelp at the if !c.Runnable() check (command.go:954) and never reaches c.ValidateArgs (command.go:968). The Args field is dead code on a non-runnable command.

Suggested fix

  • plugin install: Args: cobra.MaximumNArgs(1), plus a check that exactly one of the plugin name or --all is supplied.
  • convert: Args: cobra.NoArgs (it is flag-only).
  • plugin: give it a RunE that prints help and returns an error, so an unknown or missing subcommand exits non-zero. Args: cobra.NoArgs alone will not do it.

To keep this from recurring as commands are added, consider one table-driven test that walks rootCmd.Commands() recursively and asserts every command either declares a non-nil Args or is a parent with an explicit help-and-error RunE. That would have caught all three of these in a single pass.

Note that a test asserting this by calling cmd.ValidateArgs(...) directly will pass on the plugin parent while the real CLI still exits 0. These need to be exercised through rootCmd.SetArgs(...) / Execute() with captured output.

Related, likely a separate issue

The unimplemented stubs print not yet implemented and return nil, so they exit 0 while doing nothing: convert (convert.go:46), validate (validate.go:38), plugin install (install.go:36), plugin remove (remove.go:33). A pipeline running ossie validate model.yaml && deploy would deploy an unvalidated model. These should write to cmd.ErrOrStderr() and return an error until implemented. Happy to split this out if preferred.

Environment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions