fix: CLI --password-stdin + honor handler exit codes by erikdarlingdata · Pull Request #253 · erikdarlingdata/PerformanceStudio

erikdarlingdata · 2026-04-21T22:55:17Z

Summary

CLI passwords passed on the command line (--password hunter2) are visible in process listings (tasklist /v, ps), shell history, and OS audit logs. This PR adds --password-stdin so the password can be piped in instead, and warns when the inline flag is used.

While investigating, noticed a pre-existing bug: handlers that set Environment.ExitCode = 1 on validation failure were exiting 0 because return await root.InvokeAsync(args) clobbered the exit code in the top-level program. Fixed that in the same PR since --password-stdin depends on it (the mutual-exclusion and stdin-not-redirected checks would otherwise be advisory).

Changes

New: src/PlanViewer.Cli/PasswordResolver.cs — shared resolver for the CLI password input chain.
AnalyzeCommand, QueryStoreCommand: add --password-stdin option, route through the resolver.
CredentialCommand: add the same inline-password warning for symmetry (existing stdin-when-redirected behavior is already correct).
Program.cs: propagate Environment.ExitCode through the top-level return, so handlers that signal failure actually exit non-zero.

Resolver rules

--password and --password-stdin are mutually exclusive → exit 1.
--password-stdin + --stdin (plan XML) → exit 1 (both claim stdin).
--password-stdin without stdin redirected → exit 1.
--password inline → emit stderr warning pointing at safer alternatives.
Otherwise fall back to PLANVIEW_PASSWORD env var (unchanged).

Test plan

Exercised against the built planview.exe:

Scenario	Expected	Got
`--password X --password-stdin`	exit 1, "mutually exclusive"	✅ exit 1
`--stdin --password-stdin` (piped)	exit 1, "can't be combined with --stdin"	✅ exit 1
`--password X file.sqlplan` (inline)	stderr warning + usual processing	✅ warning printed
`echo X \| planview analyze --password-stdin ...`	password read, no warning	✅
`planview analyze /nonexistent.sqlplan` (pre-existing path)	exit 1 (regression: was 0)	✅ exit 1
`planview --help`	exit 0	✅

Also verified the --password-stdin option appears in --help for both analyze and query-store.

🤖 Generated with Claude Code

Passwords given on the command line are visible in process listings, shell history, and audit logs. Add --password-stdin to analyze and query-store so scripts and humans can feed the password through a pipe instead. When --password is used inline, emit a stderr warning pointing at the safer alternatives (stdin, env var, credential store). Helper `PasswordResolver.TryResolve` centralizes: - Mutual exclusion between --password and --password-stdin. - Conflict detection between --stdin (plan XML) and --password-stdin. - Validation that stdin is actually redirected when --password-stdin is set. - Fallback to PLANVIEW_PASSWORD env var. - The inline warning for --password. CredentialCommand already reads from redirected stdin when --password is omitted; kept that behavior and added the inline warning for symmetry. Also fixed a pre-existing bug where `Environment.ExitCode = 1` set by handlers was clobbered by `return await root.InvokeAsync(args)`. Program.cs now returns `code != 0 ? code : Environment.ExitCode` so scripts can tell a validation failure from a successful run. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

erikdarlingdata

Review summary

What it does: Adds --password-stdin to analyze and query-store so CLI passwords don't have to appear in ps/tasklist/shell history, routes the resolution through a new PasswordResolver, emits a stderr warning when --password inline is used, and fixes a pre-existing bug in Program.cs where Environment.ExitCode = 1 from a handler was clobbered by InvokeAsync returning 0.

What's good:

Base branch is dev. ✓
Mutual-exclusion (--password + --password-stdin) and stdin-conflict (--stdin + --password-stdin) checks are both in place and fail loud with exit 1.
Warning for inline --password added symmetrically to CredentialCommand as well, so the advice is consistent across the three commands that accept passwords.
Exit-code fix is a real bug fix — planview analyze /nonexistent.sqlplan previously exited 0 despite the handler setting ExitCode = 1. Worth calling out separately in a release note.

What needs attention (see inline):

PasswordResolver.cs:47 — empty/EOF stdin silently becomes password = "" and succeeds; should fail explicitly since --password-stdin implies "I'm sending one".
PasswordResolver.cs:51 — string.IsNullOrEmpty(inlinePassword) changes the fall-through semantics for --password ""; previously empty literal was used as-is, now it falls back to env var. Minor behavior delta.
Program.cs:30 — code != 0 ? code : Environment.ExitCode technically doesn't "honor either signal" as the comment claims; the framework's non-zero code wins over the handler's. Fine today, noting for later.
CredentialCommand.cs:39 — credential add doesn't get the explicit --password-stdin flag (only the implicit redirect path); three password-input commands now have three slightly different models.
PasswordResolver.cs — no unit tests. Five branches, no coverage in tests/. End-to-end table in the PR description is nice but doesn't pin the behavior.

Not applicable this PR: Avalonia/XAML concerns (CLI only), PlanAnalyzer rule sync with PerformanceMonitor Dashboard/Lite (no analyzer changes), text-contrast brushes (no UI).

Comments only — maintainer's call.

Generated by Claude Code

erikdarlingdata · 2026-04-21T22:56:46Z

+                Console.Error.WriteLine("--password-stdin requires stdin to be redirected (pipe the password into the command).");
+                return false;
+            }
+            password = Console.In.ReadLine()?.TrimEnd('\r', '\n') ?? "";


Console.In.ReadLine() returns null on EOF and "" on an empty line; both collapse to password = "" here and TryResolve returns true. That empty password flows into the connection string and produces a generic SQL auth failure rather than a clear "no password read from stdin" error.

Repro: planview analyze --password-stdin ... < /dev/null (or piping an empty file) silently attempts to connect with an empty password instead of failing at the CLI boundary.

Consider failing explicitly when the read yields null-or-empty, since --password-stdin means "I'm providing a password via stdin" — a zero-byte read is almost certainly a scripting mistake.

Generated by Claude Code

erikdarlingdata · 2026-04-21T22:56:51Z

+            return true;
+        }
+
+        if (!string.IsNullOrEmpty(inlinePassword))


Subtle behavior change from string.IsNullOrEmpty here: the old code was password ??= env.GetValueOrDefault("PLANVIEW_PASSWORD"), which only fell back to the env var when --password was unset (null). Now --password "" (empty string) falls through to envPassword as well.

Pre-change: --password "" with PLANVIEW_PASSWORD=foo → connects with "".
Post-change: same invocation → connects with foo.

Almost certainly nobody depends on passing an empty literal, but worth being aware it's a behavior delta, not just a refactor.

Generated by Claude Code

erikdarlingdata · 2026-04-21T22:56:56Z

+// handler set Environment.ExitCode = 1 to signal a validation error. Honor either
+// signal so scripts can tell success from failure.
+var code = await root.InvokeAsync(args);
+return code != 0 ? code : Environment.ExitCode;


The expression code != 0 ? code : Environment.ExitCode is asymmetric: if a handler sets Environment.ExitCode = 2 and InvokeAsync returns non-zero for an unrelated reason (e.g. parser error), the handler's signal is dropped. Today no handler uses a code other than 1 so it's fine, but the comment says "honor either signal" — it actually prefers the framework's signal. Minor; leaving a note in case this grows teeth later.

Generated by Claude Code

erikdarlingdata · 2026-04-21T22:56:59Z

            string password;
            if (!string.IsNullOrEmpty(passwordArg))
            {
+                Console.Error.WriteLine(


credential add doesn't expose an explicit --password-stdin flag (it just has the implicit "read from stdin if redirected" path below). For symmetry with analyze / query-store, consider routing this through PasswordResolver or adding the matching flag — otherwise the three commands end up with three slightly different password-input models (explicit flag, implicit redirect, or env). Not a blocker; just inconsistency surface area.

Generated by Claude Code

erikdarlingdata · 2026-04-21T22:57:04Z

+    /// redirected when --password-stdin was requested). The caller is responsible
+    /// for setting Environment.ExitCode on failure.
+    /// </summary>
+    public static bool TryResolve(


New file with five distinct decision branches (mutual-exclusion, stdin-claimed, stdin-not-redirected, stdin-success, inline-with-warning, env-fallback) and no unit tests. The table in the PR description was exercised end-to-end against the built planview.exe, but there's nothing in tests/ pinning this behavior. A small PasswordResolverTests class (injecting a TextReader + bool inputRedirected instead of reading Console.In directly) would catch the kinds of regressions this kind of branchy validation tends to collect.

Generated by Claude Code

erikdarlingdata merged commit b945eba into dev Apr 21, 2026
2 checks passed

erikdarlingdata deleted the fix/cli-password-stdin branch April 21, 2026 22:55

erikdarlingdata commented Apr 21, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: CLI --password-stdin + honor handler exit codes#253

fix: CLI --password-stdin + honor handler exit codes#253
erikdarlingdata merged 1 commit into
devfrom
fix/cli-password-stdin

erikdarlingdata commented Apr 21, 2026

Uh oh!

Uh oh!

erikdarlingdata left a comment

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

erikdarlingdata Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

erikdarlingdata commented Apr 21, 2026

Summary

Changes

Resolver rules

Test plan

Uh oh!

Uh oh!

erikdarlingdata left a comment

Choose a reason for hiding this comment

Review summary

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

erikdarlingdata Apr 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant