Keep the keychain password off the command line on macOS

[erikdarlingdata]

What does this PR do?

On macOS, saving a credential keeps the password off the command line. KeychainCredentialService.SaveCredential used to run security add-generic-password ... -w <password>, which put the password in the argument list of the security process while it ran. It now sends that command to security -i on stdin. The only argument security gets is -i. The password goes as hex (-X), so no character in it needs quoting.

Why security -i and not the Security framework

An item saved by the current release has an access list that lets only /usr/bin/security read its password. On the test Mac, security dump-keychain -a shows applications (1): /usr/bin/security. Reading these items through the Security framework shows a keychain prompt for every saved credential. Items the app saved itself trust the app's own binary instead. The macOS build is an unsigned zip, so each update brings the prompts back. Staying with /usr/bin/security keeps old and new items readable without prompts.

Reading back returns exactly what was saved

The non-ASCII and space tests needed these changes to pass:

• GetCredential read the password with find-generic-password -w. That prints hex for any password with a byte outside printable ASCII. The hex output looks the same as a password made of hex digits. It also trimmed leading and trailing spaces. It now reads the -g output, which marks hex values with 0x, and decodes them. One security call now reads both the username and the password, instead of two calls.
• The same decoding fixes usernames with a backslash (CORP\sa), which GetCredential failed to read. It also fixes server names with a backslash (SQL01\PROD), which planview credential list left out.

The service name (PlanViewer:<server>) and account are unchanged. Items saved by the current release are found, updated in place (still one item), and deleted.

The one-line limit

A command sent to security -i must be one line of at most 4094 bytes. security reads each line into a 4096-byte buffer and runs whatever does not fit as a separate command. A save returns false and sends nothing when its line is too long, or when the server name or username has a newline. A password hits the limit at about 1,900 bytes.

Which component(s) does this affect?

• Desktop App (PlanViewer.App)
• Core Library (PlanViewer.Core): used by the desktop app and the CLI on macOS
• CLI Tool (PlanViewer.Cli)
• SSMS Extension (PlanViewer.Ssms)
• Tests
• Documentation

How was this tested?

Platform: macOS 27.0 on Apple silicon, .NET SDK 10.0.401. The Windows and Linux credential services are not changed.

New tests

KeychainCredentialServiceTests has 28 cases. They run on macOS only and are skipped elsewhere. Each test makes its own throwaway keychain file, so the login keychain is never touched. Every call goes through a small script that logs its arguments and then runs /usr/bin/security.

• Save, read, update, and delete keep the password exactly. The passwords tested have spaces, spaces at both ends, double and single quotes, or backslashes. Others are héllo wörld, 密码🔑, a tab, a newline, an empty password, 68656c6c6f (hex-looking), and -U (option-looking).
• An item saved with the current release's exact command reads, updates in place (one item), and deletes. That command has -w on the command line. The same passwords are tested.
• The logged arguments of every security run never contain the password or its hex. With the save switched back to -w on the command line, this test fails.
• SQL01\PROD / CORP\svc_sql, names with spaces and quotes, and a non-ASCII server name read back and are listed.
• A password too long for one line is refused and nothing is saved. A server name with a newline is refused and nothing is saved.

Manual checks against the real login keychain

The test items were removed afterwards.

• Saved an item with the current release's command, then read, updated, and deleted it with the new code. The current release's read (-w) sees the new code's update.
• The passwords listed above round-trip.
• During 40 saves, no running security process had the password or its hex in its arguments.
• planview credential add 'SQL01\PROD_…' -u 'CORP\sa' with a non-ASCII password piped in, then credential list (shows the server) and credential remove.

Build and suite

dotnet build PlanViewer.sln -c Release --no-incremental and -c Debug both have 0 warnings. Full suite on macOS at b579e45: 1,082 passed, 5 skipped, 1 failed.

The failure was SingleInstanceTests.ALineSentByTheClientReachesTheServer, which has nothing to do with the keychain. Its pipe name gave a socket path of 131 bytes under the macOS temp folder, and macOS allows 104. The app's own pipe path is 89 bytes, so only the test was affected. Commit 44dd389 shortens the test's pipe name, which makes the path 98 bytes. It passes on Windows and on the Linux CI runner, and has not been run on macOS.

Checklist

• I have read the contributing guide
• My code builds with zero warnings (dotnet build -c Debug)
• All tests pass (dotnet test): all passed on macOS except the pipe test that 44dd389 fixes, which has not been re-run there
• I have not introduced any hardcoded credentials or server names

🤖 Generated with Claude Code

[claude]

Reviewed KeychainCredentialService and found no blocking issues. The password is sent as hex on stdin, the quoting and newline/NUL/length guards look right, and the read path decodes -g output correctly.

One thing to confirm (Low): SaveCredential treats security -i exit code 0 as success. As I remember it, interactive mode can exit 0 even when an individual command fails, for example on a locked keychain or a rejected argument. I couldn't check that here because this runner isn't macOS. If it does exit 0 on failure, SaveCredential would report success for a save that didn't happen. You could check stderr for security: error text, or read the item back after the save. A test that runs -i against a locked or nonexistent keychain and asserts SaveCredential returns false would settle it.

The macOS-only tests are skipped on CI, so the Linux runner doesn't cover any of this.

[erikdarlingdata]

On the exit-code question: I checked Apple's source for security (SecurityTool/macOS/security.c in apple-oss-distributions/Security). In interactive mode, main stores each command's status in result and returns it when stdin ends. Nothing after the loop resets it. The only reset is for a command that returns -1, which is the quit command. SaveCredential sends exactly one line, so a failed add exits nonzero and SaveCredential returns false. No change needed.