cosign icon indicating copy to clipboard operation
cosign copied to clipboard

-f or --no-input option does not work with piv-tool set-pin

Open trondat opened this issue 3 years ago • 2 comments

Description

When setting new pin on the PIV device the command cosign piv-tool set-pin --no-input --new-pin <new pin> --old-pin <old pin> will prompt for confirmation even though --no-input is specfied ? Setting new pin. This will overwrite the previous pin.? [y/N] █

Version

GitVersion: devel GitCommit: unknown GitTreeState: unknown BuildDate: unknown GoVersion: go1.19.1 Compiler: gc Platform: linux/amd64

trondat avatar Sep 30 '22 13:09 trondat

thanks, i will try to reproduce and if confirms will see how to fix it

cpanato avatar Sep 30 '22 14:09 cpanato

Problem is Confirm in pivcli/commands.go: https://github.com/sigstore/cosign/blob/cb4898b327f195b2059db7288f433e668e33c575/cmd/cosign/cli/pivcli/commands.go#L301

The solution would be to use ConfirmPrompt instead. This would be a fast fix.

It has an added benefit of getting rid of a dependency on promptui!

While we're in there, we should replace any term.ReadPassword calls with calls to GetPassFromTerm.

Long term fix

Nail down semantics

In past discussions (see https://github.com/sigstore/cosign/issues/844 https://github.com/sigstore/cosign/pull/1909 https://github.com/sigstore/cosign/pull/2039) it's come up that there are two distinct use cases for a --no-input-like flag:

  1. "I'm running in a script, fail fast instead of waiting for input" ("non-interactive")
  2. "shut up and do what I say" ("force")

There are also two kinds of types of prompt:

  1. simple confirmation ("FYI this information is going into a transparency log")
  2. destructive operations

The following is a proposed matrix of how they'd interact:

|              | interactive | non-interactive | force |
|--------------+-------------+-----------------+-------|
| notification | `[Yn]`      | print, proceed  | do it |
| destructive  | `[yN]`      | fail fast       | do it |

We should codify this in some documentation.

Enforce those semantics

Ban reader.ReadString and term.ReadPassword (using forbidigo in golanglint-ci) so that we always go through the Cosign libraries.

znewman01 avatar Oct 17 '22 13:10 znewman01