haskell / haskeline

A Haskell library for line input in command-line programs.
https://hackage.haskell.org/package/haskeline
BSD 3-Clause "New" or "Revised" License
223 stars 75 forks source link

getPassword reveals your password as you type it on MSYS2 (Windows) #50

Closed RyanGlScott closed 7 years ago

RyanGlScott commented 7 years ago

This same issue bit cabal-install (https://github.com/haskell/cabal/issues/4128), and it affects haskeline too. Not only is this Windows-specific, but it's also console-specific, as it requires the MSYS2 (or Cygwin) console (as opposed to cmd.exe or PowerShell, which are exempt from this issue). To reproduce, clone this repo in MSYS2 and run:

$ runghc haskeline/examples/Test.hs
0:password
line 0:password
1:hunter2
line 1:hunter2
2:q

Notice that it echoed my password (hunter2) when I typed it! This is because sadly, the hGetEcho and hSetEcho functions (which getPassword uses internally) don't work properly on MinTTY consoles like MSYS2. To disable echoing on MinTTY, you have to shell out to stty.

I fixed the cabal-install issue by creating my own echo library, which implements all of the functionality needed to have a withoutInputEcho function that works properly on all operating systems and consoles, and made cabal-install depend on it. However, given that haskeline is a GHC dependency, that probably won't work in this case. Would you object if I inlined the code from echo into haskeline to fix getPassword?

judah commented 7 years ago

Thanks for the report. Support for Cygwin is a longstanding open issue in Haskeline: http://trac.haskell.org/haskeline/ticket/1.

In theory adding a special case for this seems reasonable. However, I'm concerned about the analysis from that bug:

Of course, this isn't at all portable, since it won't even work on cmd.exe/PowerShell. Perhaps we could take a cue from terminal-size and default to the strategy above whenever we're not on a Windows console? It's easy enough to detect when you're not on a Windows console (just use System.IO.hIsTerminalDevice System.IO.stdin), but I don't know a reliable way to test if you're on MinTTY (other than check for the presence of the _ environmental variable, as GHC does, but this is obviously fallible). It might be safe to assume that if you're not on a Windows console, then stty is present.

Specifically: if you're running in cmd.exe/PowerShell and pipe command A into command B, and command B uses Haskeline, then it (correctly) won't look like a terminal, but it also won't have stty present. I guess this is more of a concern for a general-purpose library like Haskeline than for thecabal program. Do you have an idea how to make it more robust?

RyanGlScott commented 7 years ago

To be clear, that part was an early proposal, and not what I ended up implementing in echo. What it actually does is use the Win32 API to see if stdin is being piped from from a set of known filenames that MinTTY uses (see here for the implementation, it's directly taken from Win32-2.5.0.0). This avoids ths scenario you're worried about, since the special behavior (stty) only kicks in if it knows for sure that MinTTY is being used.

As evidence, I've tried this out on Cygwin, MSYS2, Git Bash, ConEmu, cmd.exe, and PowerShell, and they all do the right thing.

judah commented 7 years ago

Ah, thanks for the clarification; that approach sounds good.

It sounds like we could eventually go further with your approach to help implement a new MinTTY backend in Haskeline, fixing for example http://trac.haskell.org/haskeline/ticket/114. But, of course, not necessary to go all that way for just this issue.

One approach to patching Haskeline would be to add a new field to FileOps of type IO a -> IO a which runs the given action in no-echoing mode: https://github.com/judah/haskeline/blob/master/System/Console/Haskeline/Term.hs#L69 In fact, that might be able to replace the current field inputHandle, since IIRC we only use it to control echoing.

The actual FileOps for Win32 is constructed here: https://github.com/judah/haskeline/blob/master/System/Console/Haskeline/Backend/Win32.hsc#L411 Let me know if you'd like any other pointers around the codebase.