feat: add support to display mnemonic discreetly for algokey generate

[Halimao]

Summary

This PR add support for displaying mnemonic discreetly.

• New Features
  • Addition of a new bool flag generateIndiscreet, used to disable new feature and printing mnemonic as old way.
  • Addition of discreet mnemonic display for algokey generate in cmd/algokey.
  • Added functionality to print sensitive information discreetly to an alternate screen.

Test Plan

• Add test Test_printDiscreetly for new function printDiscreetly
• Build new algokey binary and test in local machine

./algokey generate --indiscreet

./algokey generate

[CLAassistant]

All committers have signed the CLA.

[Halimao]

If this pr LGTY, I will draft another prs for algokey import and algokey export

[codecov]

Codecov Report

Attention: 10 lines in your changes are missing coverage. Please review.

Comparison is base (b5927a6) 56.00% compared to head (f7d8ed0) 55.66%.
Report is 7 commits behind head on master.

Files	Patch %	Lines
cmd/algokey/generate.go	14.28%	6 Missing ⚠️
cmd/algokey/common.go	55.55%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5886      +/-   ##
==========================================
- Coverage   56.00%   55.66%   -0.34%     
==========================================
  Files         478      487       +9     
  Lines       67561    68040     +479     
==========================================
+ Hits        37835    37875      +40     
- Misses      27174    27592     +418     
- Partials     2552     2573      +21     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Halimao]

@jasonpaulos @onetechnical Hi sir, could you please help me review this pr, thanks🙏

[gmalouf]

User deleted their account.

[Halimao]

User deleted their account.

Are you kidding？

I'm just take this "ghost" nickname for funny.

Github deleted user's profile is github.com/ghost

[gmalouf]

@Halimao our mistake, we looked quick and thought you had deleted the account.

If we understand this PR correctly, the mnemonic is still displayed when you first run it - so people watching over you could see it. An alternative to this code change to could be to hit ^L on your keyboard in the terminal to hide the mnemonic right?

[Halimao]

hit ^L on your keyboard

I'm afraid I can't agree with this. In some cases, Ctrl+L doesn't clear the output on the terminal, it could be easily recovered just by sliding up the mouse scroll wheel.

This pr is indeed not suitable in the case that someone is watching over you. If we wanna prevent displaying private information when someone is watching over us, there are still more things that need to be done first IMO.

This pr aims to split the public info and private info displayed to the user automatically(without another user operation). It improves security somehow, shows the mnemonic to the user, and then closes the alternate screen. No one can recover from this PC anymore.

[jannotti]

I don't like this change in behaviour. But if we we did want it, is there something wrong with this:

package main

import (
    "github.com/inancgumus/screen"
)

func main() {
    // Clears the screen
    screen.Clear()
}

Do we have any way to know which terminals this solution works in vs another solution? Do we have any way to know if it stops working in some subset of the million terminal emulators on million OSes?

[Halimao]

// Clears the screen

Why should we clear the whole screen? I just wanna clear the secret information on the terminal.

Do we have any way to know which terminals this solution works in vs another solution? Do we have any way to know if it stops working in some subset of the million terminal emulators on million OSes?

If we are concerned about the compatibility with all Oses, we could make --indiscreet default to true, so that this new feature won't affect current behavior. For someone using common OSes, and wanna to hide secret pieces of information, this pr will provide them with a convenient way.

[gmalouf]

If we are concerned about the compatibility with all Oses, we could make --indiscreet default to true, so that this new feature won't affect current behavior. For someone using common OSes, and wanna to hide secret pieces of information, this pr will provide them with a convenient way.

This was in my initial comment, I would make the setting --discrete defaulting to false (so that true is enabling the hiding functionality). Definitely did not want to change default behavior for folks under them.

[Halimao]

This was in my initial comment

Aha, sorry for missing this comment.

I just updated --indiscreet option's default value to true. Thanks for your review

[gmalouf]

Hi @Halimao, do you mind rebasing off of master? Our license check is failing I think because we updated them all to 2024 after January 1.

[gmalouf]

There are also some linter warnings from reviewdog related to expected test structure:

lint: Test_printDiscreetly: Add missing partition call to top of test

[gmalouf]

@Halimao can you update the license header in common_test.go to match the others? The license check is failing based on that one file it seems.

[gmalouf]

@Halimao I just checked this out manually and tested it:

The default algokey generate is indiscreet, to get the discreetness you are looking for you have to do algokey generate --indiscreet=false. I think the default behavior is correct (do not want to change the default for existing users), but I would rename your flag to discreet with it defaulted to false, then update your logic for when it is true. Alternatively, allowing one to simply have the discreet flag present to enable (without specifying a boolean) would be even better if the framework supports it.

Side note: had to refresh myself on discrete vs discreet usage :)

[Halimao]

@Halimao I just checked this out manually and tested it:

The default algokey generate is indiscreet, to get the discreetness you are looking for you have to do algokey generate --indiscreet=false. I think the default behavior is correct (do not want to change the default for existing users), but I would rename your flag to discreet with it defaulted to false, then update your logic for when it is true. Alternatively, allowing one to simply have the discreet flag present to enable (without specifying a boolean) would be even better if the framework supports it.

Side note: had to refresh myself on discrete vs discreet usage :)

Addressed~

[gmalouf]

Thanks for making the changes, looks good.

[jasonpaulos]

Seems useful, and I confirmed locally that it works as intended