Skip to content

fix(cli): store the macOS keychain secret through security -i, not stdin to -w - #74

Closed
Zachajones wants to merge 1 commit into
plannotator:mainfrom
Zachajones:fix/macos-keychain-write-stdin
Closed

Zachajones wants to merge 1 commit into
plannotator:mainfrom
Zachajones:fix/macos-keychain-write-stdin

Conversation

@Zachajones

@Zachajones Zachajones commented Sep 23, 2026 •

Copy link
Copy Markdown

Symptom

On macOS, artifactserver auth login <origin> --name x --api-key-stdin prints "status": "authenticated", then artifactserver auth status x immediately prints "status": "invalid". The keychain item exists (account = the profile's credentialId, service artifactserver.com/cli) but its password is empty:

/usr/bin/security find-generic-password -a <credentialId> -s artifactserver.com/cli -w | wc -c   # 1

Seen with the v0.1.1 portable Node package on macOS 14.

Cause

macOsCredentialStore.write spawns security add-generic-password -a … -s … -U -w and writes the secret to the child's stdin. security takes the -w value only from argv or from a terminal prompt; with stdin piped and no terminal it stores an empty password and exits 0. Reproduce without the CLI:

printf 'secret\n' | /usr/bin/security add-generic-password -a t -s t -U -w
/usr/bin/security find-generic-password -a t -s t -w | wc -c    # 1, empty
/usr/bin/security delete-generic-password -a t -s t

Fix

security -i reads whole commands from stdin, so the add-generic-password line goes over the pipe with the secret single-quoted, and the secret still never appears in argv. Verified on macOS 14 that a JSON value with double quotes and a space round-trips exactly. Stored values are JSON of base64url or hex tokens and UUID accounts, so a single quote or newline never occurs; quoteForSecurity refuses one rather than guessing at the interactive tokenizer's escaping.

Notes for review

  • Read and delete paths are unchanged.
  • In interactive mode security may exit 0 even when the inner command fails; a read-back after write would make the failure visible. Happy to add that if you want it in this PR.
  • The throw in quoteForSecurity surfaces through runCredentialProcess as backend_unavailable; a dedicated reason may read better.
  • Verified on macOS 14 from this branch with pnpm artifactserver against a real remote server: auth login … --api-key-stdin then auth status reads authenticated with no manual keychain repair (the item holds the full 140-byte envelope), and auth logout removes it. tsc -p tsconfig.json --noEmit and oxlint --type-aware --type-check --deny-warnings on the file both pass. The full pnpm check (site, conformance, cloudflare) was not run.

🤖 Generated with Claude Code

…stdin to `-w`

`security add-generic-password … -w` with no value reads the password from
the controlling terminal, never from stdin. With stdin piped and no terminal
it stores an empty password and exits 0, so `auth login` reported
"authenticated" while the saved profile could never be read back and
`auth status` reported it invalid.

`security -i` reads whole commands from stdin, so the add-generic-password
line goes over the pipe with the secret single-quoted, and the secret never
appears in argv. Stored values are JSON of base64url/hex tokens and UUID
accounts, so a quote or newline never occurs; the helper refuses one rather
than guessing at the interactive tokenizer's escaping.

Verified on macOS 14 (v0.1.1 portable package): the previous item read back
empty; a JSON value with double quotes and a space round-trips exactly
through `security -i`.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Zachajones

Copy link
Copy Markdown
Author

Closing for now; I want to test this more broadly before asking for review. Will reopen.

@Zachajones Zachajones closed this Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant