Skip to content

Commit d2d538f

Browse files
wesmclaude
andauthored
fix: masked password prompt with asterisk echo for IMAP accounts (#216)
## Summary - Replace `term.ReadPassword` (no visual feedback) with `charmbracelet/huh` masked input that shows `*` per keystroke during IMAP password entry - Add pipe detection: when stdin is not a terminal, read password from piped stdin for scripted usage (`echo "pass" | msgvault add-imap ...`) - Use `go-isatty` for TTY detection (including Cygwin/MSYS), matching existing codebase patterns - Extract `choosePasswordStrategy()` with table-driven tests covering all stdin/stdout/stderr TTY combinations - Remove `golang.org/x/term` direct dependency (no longer needed) - Update nix flake vendorHash for new dependency The strategy selection is a pure function of four booleans, tested across 10 cases: | stdin | stderr | stdout | method | |-------|--------|--------|--------| | any TTY | TTY | any | huh → stderr | | any TTY | redirected | TTY | huh → stdout | | any TTY | redirected | redirected | clear error | | not TTY | any | any | pipe read | ## Test plan - [x] Table-driven tests for `choosePasswordStrategy`: 10 cases covering native/Cygwin/piped stdin × stderr/stdout TTY combinations - [x] Table-driven tests for `readPasswordFromPipe`: newline trimming, CRLF, empty input, whitespace-only, EOF, multi-line - [ ] Manual test: `msgvault add-imap --host ... --username ...` shows asterisks during password entry - [ ] Manual test: `echo "pass" | msgvault add-imap --host ... --username ...` reads from pipe Closes #209 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 754f459 commit d2d538f

File tree

8 files changed

+360
-16
lines changed

8 files changed

+360
-16
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
# Go binaries
22
/msgvault
3+
/msgvault.exe
34
/mimeshootout
45

56
# Database files

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ install:
4242

4343
# Clean build artifacts
4444
clean:
45-
rm -f msgvault mimeshootout
45+
rm -f msgvault msgvault.exe mimeshootout
4646
rm -rf bin/
4747

4848
# Run tests

cmd/msgvault/cmd/addimap.go

Lines changed: 101 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,53 @@
11
package cmd
22

33
import (
4+
"bufio"
45
"fmt"
6+
"io"
57
"os"
8+
"strings"
69

10+
"github.com/charmbracelet/huh"
11+
"github.com/mattn/go-isatty"
712
"github.com/spf13/cobra"
813
imapclient "github.com/wesm/msgvault/internal/imap"
914
"github.com/wesm/msgvault/internal/store"
10-
"golang.org/x/term"
1115
)
1216

17+
// passwordMethod describes how to read the password.
18+
type passwordMethod int
19+
20+
const (
21+
// passwordInteractive uses huh masked input with asterisk echo.
22+
passwordInteractive passwordMethod = iota
23+
// passwordNoPrompt means stdin is a TTY but no output TTY is
24+
// available for the prompt. Fail with a clear error.
25+
passwordNoPrompt
26+
// passwordPipe reads from piped (non-terminal) stdin.
27+
passwordPipe
28+
)
29+
30+
// choosePasswordStrategy selects the password input method based on
31+
// which file descriptors are terminals. Returns the method and, for
32+
// passwordInteractive, the output file to render the TUI to.
33+
func choosePasswordStrategy(
34+
stdinNative, stdinCygwin, stderrTTY, stdoutTTY bool,
35+
) (passwordMethod, *os.File) {
36+
stdinTTY := stdinNative || stdinCygwin
37+
if !stdinTTY {
38+
return passwordPipe, nil
39+
}
40+
// Prefer stderr (keeps stdout clean); fall back to stdout.
41+
switch {
42+
case stderrTTY:
43+
return passwordInteractive, os.Stderr
44+
case stdoutTTY:
45+
return passwordInteractive, os.Stdout
46+
default:
47+
return passwordNoPrompt, nil
48+
}
49+
}
50+
1351
var (
1452
imapHost string
1553
imapPort int
@@ -28,6 +66,9 @@ Use --starttls for STARTTLS upgrade on port 143.
2866
Use --no-tls for a plain unencrypted connection (not recommended).
2967
3068
You will be prompted to enter your password interactively.
69+
For scripting, pipe the password via stdin to avoid exposing it in
70+
shell history or process listings:
71+
read -s PASS && echo "$PASS" | msgvault add-imap --host ... --username ...
3172
3273
Security note: Your password is stored on disk with restricted file
3374
permissions (0600). For stronger security, use an app-specific password
@@ -58,17 +99,28 @@ Examples:
5899
Username: imapUsername,
59100
}
60101

61-
// Get password via secure interactive prompt only (never via flag to
62-
// avoid exposure in shell history and process listings).
63-
fmt.Printf("Password for %s@%s: ", imapUsername, imapHost)
64-
raw, err := term.ReadPassword(int(os.Stdin.Fd()))
65-
fmt.Println()
66-
if err != nil {
67-
return fmt.Errorf("read password: %w", err)
102+
prompt := fmt.Sprintf("Password for %s@%s:", imapUsername, imapHost)
103+
method, promptOut := choosePasswordStrategy(
104+
isatty.IsTerminal(os.Stdin.Fd()),
105+
isatty.IsCygwinTerminal(os.Stdin.Fd()),
106+
isatty.IsTerminal(os.Stderr.Fd()) || isatty.IsCygwinTerminal(os.Stderr.Fd()),
107+
isatty.IsTerminal(os.Stdout.Fd()) || isatty.IsCygwinTerminal(os.Stdout.Fd()),
108+
)
109+
110+
var (
111+
password string
112+
err error
113+
)
114+
switch method {
115+
case passwordInteractive:
116+
password, err = readPasswordInteractive(prompt, promptOut)
117+
case passwordNoPrompt:
118+
return fmt.Errorf("cannot read password: no terminal available for prompt (try piping the password via stdin)")
119+
case passwordPipe:
120+
password, err = readPasswordFromPipe(os.Stdin)
68121
}
69-
password := string(raw)
70-
if password == "" {
71-
return fmt.Errorf("password is required")
122+
if err != nil {
123+
return err
72124
}
73125

74126
// Test connection
@@ -131,6 +183,44 @@ Examples:
131183
},
132184
}
133185

186+
// readPasswordFromPipe reads a password from a non-terminal reader
187+
// (e.g. piped stdin). Uses only the first line.
188+
func readPasswordFromPipe(r io.Reader) (string, error) {
189+
scanner := bufio.NewScanner(r)
190+
if !scanner.Scan() {
191+
if err := scanner.Err(); err != nil {
192+
return "", fmt.Errorf("read password: %w", err)
193+
}
194+
return "", fmt.Errorf("password is required")
195+
}
196+
password := strings.TrimRight(scanner.Text(), "\r\n")
197+
if strings.TrimSpace(password) == "" {
198+
return "", fmt.Errorf("password is required")
199+
}
200+
return password, nil
201+
}
202+
203+
// readPasswordInteractive prompts for a password using a masked
204+
// input field with asterisk echo. The output writer controls where
205+
// the TUI renders (typically stderr to avoid polluting stdout).
206+
func readPasswordInteractive(prompt string, output io.Writer) (string, error) {
207+
var password string
208+
input := huh.NewInput().
209+
Title(prompt).
210+
EchoMode(huh.EchoModePassword).
211+
Value(&password)
212+
err := huh.NewForm(huh.NewGroup(input)).
213+
WithOutput(output).
214+
Run()
215+
if err != nil {
216+
return "", fmt.Errorf("read password: %w", err)
217+
}
218+
if strings.TrimSpace(password) == "" {
219+
return "", fmt.Errorf("password is required")
220+
}
221+
return password, nil
222+
}
223+
134224
func init() {
135225
addIMAPCmd.Flags().StringVar(&imapHost, "host", "", "IMAP server hostname (required)")
136226
addIMAPCmd.Flags().IntVar(&imapPort, "port", 0, "IMAP server port (default: 993 for TLS, 143 otherwise)")

cmd/msgvault/cmd/addimap_test.go

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,196 @@
1+
package cmd
2+
3+
import (
4+
"io"
5+
"os"
6+
"strings"
7+
"testing"
8+
)
9+
10+
func TestPasswordPromptStrategy(t *testing.T) {
11+
tests := []struct {
12+
name string
13+
stdinNat bool // stdin is a native terminal
14+
stdinCyg bool // stdin is a Cygwin/MSYS PTY
15+
stderrTTY bool
16+
stdoutTTY bool
17+
wantMethod passwordMethod
18+
wantOutput *os.File // nil for pipe/error methods
19+
}{
20+
{
21+
name: "normal interactive terminal",
22+
stdinNat: true,
23+
stderrTTY: true,
24+
stdoutTTY: true,
25+
wantMethod: passwordInteractive,
26+
wantOutput: os.Stderr,
27+
},
28+
{
29+
name: "stdout redirected",
30+
stdinNat: true,
31+
stderrTTY: true,
32+
stdoutTTY: false,
33+
wantMethod: passwordInteractive,
34+
wantOutput: os.Stderr,
35+
},
36+
{
37+
name: "stderr redirected",
38+
stdinNat: true,
39+
stderrTTY: false,
40+
stdoutTTY: true,
41+
wantMethod: passwordInteractive,
42+
wantOutput: os.Stdout,
43+
},
44+
{
45+
name: "both outputs redirected, native stdin",
46+
stdinNat: true,
47+
stderrTTY: false,
48+
stdoutTTY: false,
49+
wantMethod: passwordNoPrompt,
50+
},
51+
{
52+
name: "cygwin normal terminal",
53+
stdinCyg: true,
54+
stderrTTY: true,
55+
stdoutTTY: true,
56+
wantMethod: passwordInteractive,
57+
wantOutput: os.Stderr,
58+
},
59+
{
60+
name: "cygwin stdout redirected",
61+
stdinCyg: true,
62+
stderrTTY: true,
63+
stdoutTTY: false,
64+
wantMethod: passwordInteractive,
65+
wantOutput: os.Stderr,
66+
},
67+
{
68+
name: "cygwin stderr redirected",
69+
stdinCyg: true,
70+
stderrTTY: false,
71+
stdoutTTY: true,
72+
wantMethod: passwordInteractive,
73+
wantOutput: os.Stdout,
74+
},
75+
{
76+
name: "cygwin both outputs redirected",
77+
stdinCyg: true,
78+
stderrTTY: false,
79+
stdoutTTY: false,
80+
wantMethod: passwordNoPrompt,
81+
},
82+
{
83+
name: "piped stdin",
84+
stdinNat: false,
85+
stdinCyg: false,
86+
stderrTTY: true,
87+
stdoutTTY: true,
88+
wantMethod: passwordPipe,
89+
},
90+
{
91+
name: "piped stdin, all redirected",
92+
stdinNat: false,
93+
stdinCyg: false,
94+
stderrTTY: false,
95+
stdoutTTY: false,
96+
wantMethod: passwordPipe,
97+
},
98+
}
99+
100+
for _, tt := range tests {
101+
t.Run(tt.name, func(t *testing.T) {
102+
method, output := choosePasswordStrategy(
103+
tt.stdinNat, tt.stdinCyg, tt.stderrTTY, tt.stdoutTTY,
104+
)
105+
if method != tt.wantMethod {
106+
t.Errorf("method = %v, want %v", method, tt.wantMethod)
107+
}
108+
if output != tt.wantOutput {
109+
t.Errorf("output = %v, want %v", output, tt.wantOutput)
110+
}
111+
})
112+
}
113+
}
114+
115+
func TestReadPasswordFromPipe(t *testing.T) {
116+
tests := []struct {
117+
name string
118+
input string
119+
want string
120+
wantErr string
121+
}{
122+
{
123+
name: "reads password from pipe",
124+
input: "secret123\n",
125+
want: "secret123",
126+
},
127+
{
128+
name: "trims trailing newline",
129+
input: "mypassword\n",
130+
want: "mypassword",
131+
},
132+
{
133+
name: "trims trailing CRLF",
134+
input: "mypassword\r\n",
135+
want: "mypassword",
136+
},
137+
{
138+
name: "handles no trailing newline",
139+
input: "mypassword",
140+
want: "mypassword",
141+
},
142+
{
143+
name: "rejects empty input",
144+
input: "\n",
145+
wantErr: "password is required",
146+
},
147+
{
148+
name: "rejects whitespace-only input",
149+
input: " \n",
150+
wantErr: "password is required",
151+
},
152+
{
153+
name: "rejects EOF with no data",
154+
input: "",
155+
wantErr: "password is required",
156+
},
157+
}
158+
159+
for _, tt := range tests {
160+
t.Run(tt.name, func(t *testing.T) {
161+
r := strings.NewReader(tt.input)
162+
got, err := readPasswordFromPipe(r)
163+
if tt.wantErr != "" {
164+
if err == nil {
165+
t.Fatalf("expected error containing %q, got nil", tt.wantErr)
166+
}
167+
if !strings.Contains(err.Error(), tt.wantErr) {
168+
t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErr)
169+
}
170+
return
171+
}
172+
if err != nil {
173+
t.Fatalf("unexpected error: %v", err)
174+
}
175+
if got != tt.want {
176+
t.Errorf("got %q, want %q", got, tt.want)
177+
}
178+
})
179+
}
180+
}
181+
182+
func TestReadPasswordFromPipeLargeInput(t *testing.T) {
183+
// Only first line should be used as the password.
184+
input := "firstline\nsecondline\n"
185+
r := strings.NewReader(input)
186+
got, err := readPasswordFromPipe(r)
187+
if err != nil {
188+
t.Fatalf("unexpected error: %v", err)
189+
}
190+
if got != "firstline" {
191+
t.Errorf("got %q, want %q", got, "firstline")
192+
}
193+
}
194+
195+
// Verify the function signature accepts io.Reader.
196+
var _ func(io.Reader) (string, error) = readPasswordFromPipe

flake.nix

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
pname = "msgvault";
3030
version = "0.0.0-dev";
3131
src = ./.;
32-
vendorHash = "sha256-EyVSGMpQIVo3zn+2BJ/KaMT4HdkdZpe2OQtveoj7oAQ=";
32+
vendorHash = "sha256-o7yjPy1pDkD6Ia1H/4Ny/GYqfwv4Vbsd86bQJY6IiVo=";
3333
proxyVendor = true;
3434
subPackages = [ "cmd/msgvault" ];
3535
tags = [ "fts5" ];

0 commit comments

Comments
 (0)