-
-
Notifications
You must be signed in to change notification settings - Fork 676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(python): allowing PIN/passphrase input for Git Bash #1959
Conversation
It seems to me that According to the documentation |
It was not the case for me, neither for the reporter of the issue - I have replicated the same behavior. There was no warning, and the script literally froze and had to be killed. |
Sorry, I now see the upstream Python bug mentioned earlier: https://bugs.python.org/issue44762 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
additional comments:
Consider looking at CAN_HANDLE_HIDDEN_INPUT
in get_passphrase
and in that case, only ask once. No point in retyping to confirm if the user can see typos on screen.
This solves the issue for Git bash, but I'm not sure that it solves it for usage in scripts. I was considering a separate ScriptUI
implementation:
- uses stdin instead of stderr (so
click.echo
andclick.prompt
without theerr
override) - never uses hidden inputs
- prints prompts on single line, so instead of:
it would show
prompt("Enter PIN")
click.echo("Enter PIN") pin = click.prompt()
- you can select
ScriptUI
by something liketrezorctl --script
- it does not show PIN help text
- it prints out button request code (and in the future also name/index), PIN request type
- instead of auto-returning PASSPHRASE_ON_DEVICE, it asks you
- (and of course only asks for passphrase once)
- does not use the envvar - the script is responsible for providing it.
- shows you something like "enter pin: code=None, max_length=50"
- and then returns whatever you entered directly, the script is responsible for validating
In essence, this mode would turn trezorctl
into an interactive tool that allows you to integrate Trezor into, say, Java, without writing a Java library.
Thanks for the suggestions. After c2e5955 it looks like: The creation of |
I consider #1737 to be it -- if you read the issue description, it concerns script use, the git bash thing is a sub-issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, please squash&rebase
also please add a changelog entry: "Fix PIN and passphrase entry in certain terminals on Windows" |
0927626
to
027f875
Compare
027f875
to
67bd642
Compare
Sorry for a lot of force-pushes, I had issues with Windows line-endings and formatting |
Trying to solve #1737
Allowing the input for PIN/passphrase on Windows in Git Bash, which has issues with hidden input.
Recognizing the situation (through
sys.stdin.isatty()
) and in negative case disallowing the hidden feature and reporting this to the user.Screenshot from Git Bash on Windows: