feat(cli): promptSecret()

[lambdalisue]

The function is similar to prompt, but it prints the user's input as * to prevent the password from being shown.

Sometimes, I find the need for this type of function when creating CLI applications, so I decided to implement it. I'm not certain whether it should be included in the standard library, as it's somewhat challenging to test since it hooks into stdin. Therefore, I couldn't provide tests (or please let me know if there is a way).

Please inform me if this function shouldn't be included in the standard library, and I'll develop it as a third-party module. 👍

[iuioiua]

Hi @lambdalisue, thanks for the contribution! This is related to #3526. Can you please move the implementation to a new std/cli top-level folder? We haven't confirmed the new sub-module, but I feel like it's likely to happen.

[iuioiua]

Just a little update - we're going ahead with the std/cli sub-module.

[lambdalisue]

I'll apply that change on this weekend 👍

[lambdalisue]

@iuioiua Done. Please re-review. Note that I rebased and force pushed.

[iuioiua]

Nice work so far. Few things:

1. I'm getting the following artefact on my macOS machine:

   Password **********% 

2. Backspace isn't being correctly processed.
3. Perhaps, we can open a new process using and simulate typing with Deno.Command().

[lambdalisue]

Perhaps, we can open a new process using and simulate typing with Deno.Command().

Ah, sounds nice. I'll try.

[lambdalisue]

@iuioiua

I thought that the following implementation would handle the Ctrl sequence on the side of the process invoked by Deno.Command to handle backspaces properly, and replied that it looked good, but it did not work. I think I misunderstood something, so could you please tell me what kind of implementation you were expecting?

test.ts

const cmd = new Deno.Command(Deno.execPath(), {
  args: ["run", "./cli/prompt_secret_script.ts"],
  stdin: "inherit",
  stdout: "piped",
});
try {
  Deno.stdin.setRaw(true, { cbreak: true });
  const proc = cmd.spawn();
  const { stdout } = await proc.output();
  const result = (new TextDecoder()).decode(stdout);
  console.log(`OUTPUT: ${result}`);
} finally {
  Deno.stdin.setRaw(false);
}

prompt_secret_script.ts

const input = Deno.stdin;
const output = Deno.stdout;
const LF = "\n".charCodeAt(0);
const CR = "\r".charCodeAt(0);

const c = new Uint8Array(1);
const buf = [];

while (true) {
  const n = input.readSync(c);
  if (n === null || n === 0) {
    break;
  }
  if (c[0] === CR || c[0] === LF) {
    break;
  }
  buf.push(c[0]);
}
output.writeSync(new Uint8Array(buf));

[iuioiua]

I tried playing around with this and I couldn't get a test work. I would've thought that this could be accomplished by piping both input and output. Using child.output() is not ideal because you want to capture output bit-by-bit. Not all in one hit (most likely, IIUC).

[lambdalisue]

I see. We want a stdin that doesn't display the input string, not a stdin that doesn't process it... I think it would make more sense to add a conceal mode or whatever to deno's stdin instead of using setRaw to force deno_std to do it. What do you think?

[lambdalisue]

@iuioiua, I'm not certain whether my implementation works on Windows, but I've added control sequence handling to properly manage the backspace. I'm unsure if this approach is a good idea compared to my previous comment, but in case you find it preferable.

CleanShot.2023-11-20.at.04.33.26.mp4

Above video, I use Backspace (DEL in macOS) and Ctrl-H.

[iuioiua]

Nice. Backspaces are working now. Frankly, I'm not sure which direction to go with this. @kt3k, do you have any advice on the implementation of this?

Another issue I found. Once I hit enter upon entering my secret, the Secret ********* line disappears from the console. The line remains in the console when I do the same for prompt(). Also, check this out. It may be of value to you.

[lambdalisue]

Once I hit enter upon entering my secret, the Secret ********* line disappears from the console.

Actually, that's on purpose. I can make the behavior similar to prompt() but Secret ***** shows the number of characters so I thought it should be removed.

[iuioiua]

I think it's ok to have the *** characters shown and be as close to prompt() as possible.

[lambdalisue]

I made some adjustments to the interface to enable developers to customize the behavior.

CleanShot.2023-11-21.at.21.25.02.mp4

[iuioiua]

This is looking good! Just one final nit. Also, @kt3k, any idea how we can test this?

[iuioiua]

This LGTM for first pass! I think we can improve in following PRs by adding tests (however that's done) and perhaps simplifying logic. Thank you, @lambdalisue!

[iuioiua]

@ayame113, are you able to confirm whether this works correctly on Windows? IIRC, you've got a Windows machine.

[ayame113]

Hi @iuioiua !

I tried running the snippet below on windows.

import { promptSecret } from "./cli/prompt_secret.ts"

const password = promptSecret("please input password")
console.log(password)

Then the following error message was output.

please input passworderror: Uncaught NotSupported: The operation is not supported
  Deno.stdin.setRaw(true, { cbreak: true });
             ^
    at Stdin.setRaw (ext:deno_io/12_io.js:256:9)
    at promptSecret (file:///C:/Users/ayame/work/pr-check/deno_std/cli/prompt_secret.ts:39:14)
    at file:///C:/Users/ayame/work/pr-check/deno_std/aaaaaaaa.ts:3:18

When I looked at the Deno.stdin.setRaw documentation, it seems that the cbreak option is not supported on Windows.

The cbreak option can be used to indicate that characters that correspond to a signal should still be generated. When disabling raw mode, this option is ignored. This functionality currently only works on Linux and Mac OS.
https://deno.land/api@v1.38.3?s=Deno.SetRawOptions

[lambdalisue]

@ayame113 I see. I'll remove that option on Windows later but could you remove it from code and try if that works properly?

[ayame113]

@ayame113 I see. I'll remove that option on Windows later but could you remove it from code and try if that works properly?

@lambdalisue
I tried removing the cbreak option and it worked fine on windows. 🎉🎉🎉

[lambdalisue]

@ayame113 Thanks a lot. I removed the option on Windows 👍

[iuioiua]

LGTM again. Nice. @kt3k, are you happy to have this even though there's no test method yet?

[kt3k]

are you happy to have this even though there's no test method yet?

Not an ideal situation, but let's land this one for now as this seems manually tested enough.

Another note: std/cli is not considered stable yet

[kt3k]

LGTM