Add privacy statement for PII storage

[haydentherapper]

This statement has been added just before the OIDC code/normal flow,
since only in this flow we expect a user to be present. For the device
flow or when a token is provided, it's likely that Cosign is being run
in an automated environment.

This requires the user either type Y or provide a global flag for
confirmation. This should not break any existing flows in automated
environments because it's only the flow when a browser is needed, not
for the device flow or when a token is provided.

Signed-off-by: Hayden Blauzvern [email protected]

Summary

Ticket Link

Fixes

Release Note

Added a consent statement before uploading to the log for the browser-based OIDC login flow. This confirmation requires user input. If you want to bypass this, add the '-y' or '--yes' flag.

[haydentherapper]

@bobcallaway @dlorenc @lukehinds - Please take a look and let me know if this seems satisfactory.

[codecov-commenter]

Codecov Report

Merging #1909 (10e050d) into main (b73da70) will decrease coverage by 0.06%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1909      +/-   ##
==========================================
- Coverage   34.14%   34.07%   -0.07%     
==========================================
  Files         153      153              
  Lines        9949     9969      +20     
==========================================
  Hits         3397     3397              
- Misses       6172     6192      +20     
  Partials      380      380              

Impacted Files	Coverage Δ
cmd/cosign/cli/clean.go	0.00% <0.00%> (ø)
cmd/cosign/cli/commands.go	0.00% <0.00%> (ø)
cmd/cosign/cli/fulcio/fulcio.go	19.62% <0.00%> (-1.81%)	⬇️
cmd/cosign/cli/options/root.go	0.00% <0.00%> (ø)
pkg/cosign/common.go	0.00% <0.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b73da70...10e050d. Read the comment docs.

[dlorenc]

cc @znewman01 as well

Looks great to me, thanks!

[lukehinds]

LGTM, let's just let it sit for a business day before merging.

[znewman01]

I worry some folks will get in the habit of passing -y always to skip this prompt, but then they'll also skip some other prompt that doesn't show up every time.

Can we add a way to skip just this prompt? Maybe an environment variable COSIGN_ACCEPT_PII_NOTICE=1 or something?

[haydentherapper]

How do other CLIs handle this when they add a new feature that requires confirmation, but also support a flag that skips confirmation? Is an environment variable per confirmation a standard practice? To me, a “y” flag seemed more in line with the typical CLI interface.

FWIW, the other feature using a confirmation is a destructive operation, with its own flag. I’d say that would be a good practice to follow - a unique flag for destructive, but a single flag for skipping simple acknowledgements. Thoughts?

[znewman01]

I tend to like a lot of the guidelines at clig.dev for this.

I’d say that would be a good practice to follow - a unique flag for destructive, but a single flag for skipping simple acknowledgements. Thoughts?

That seems smart to me. In general, here's what I'd like to see (but I'm okay with variations here!):

• in an interactive environment
  • require confirmation for simple notifications (i.e., [Y]/n)
  • require explicit confirmation for destructive actions (i.e., y/[N]), unless a flag specific to that action is passed
• in a non-interactive environment (e.g., no TTY, or some explicit -y flag passed), accept defaults
  • print simple notifications, bypassing confirmation
  • error on destructive actions, unless a flag specific to that action is passed

In this case, I think I'd rather make the difference between displaying this message or not "am I interactive?" rather than "what flow am I using?"

[haydentherapper]

I can move the check up to what calls GetCert, but it'll be the same effectively. For this case, even if we're in a terminal, it's not a sufficient check because a Cosign provider may have already fetched the OIDC token, so we need to be aware of state.
I'll clean up the printing of the statement so we still print in non-interactive modes.

[haydentherapper]

Updated the code based on the comments - The message is now always printed, and only in an interactive session the user is prompted for input.

[znewman01]

LGTM, my other comments in this review are just nits

[haydentherapper]

Thanks for the reviews! This should address all of the comments - Added a second function, and changed to y/N (also handles both Y and y)