[PR #19] [MERGED] fix: prioritize keychain over env vars for credentials #231

New issue

Closed

opened 2026-02-26 21:34:14 +03:00 by kerem · 0 comments

kerem commented

2026-02-26 21:34:14 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/19
Author: @rudrankriyam
Created: 1/20/2026
Status: ✅ Merged
Merged: 1/20/2026
Merged by: @rudrankriyam

Base: main ← Head: fix/keychain-priority-over-env

📝 Commits (1)

4501c2d fix: prioritize keychain over env vars for credentials

📊 Changes

1 file changed (+18 additions, -19 deletions)

View changed files

📝 cmd/commands.go (+18 -19)

📄 Description

Summary

The credential resolution in getASCClient() was incorrectly checking environment variables first and only falling back to keychain when env vars were missing. This violated the documented behavior in AGENTS.md where credentials set via asc auth login should take precedence.

Problem: Users who authenticated with asc auth login (storing credentials in keychain) were unexpectedly using credentials from stale ASC_* env vars in their .zshrc/.bashrc.

Fix: Swap the credential resolution order:

Priority 1: Keychain credentials (explicit user setup via asc auth login)
Priority 2: Environment variables (fallback for CI/CD or when keychain unavailable)

Test Plan

make test passes (all 58 tests)
go fmt ./... clean
Manual testing: verified keychain credentials now take precedence over env vars

Breaking Change?

No. Users relying on env var overrides can still use them by clearing their keychain credentials (asc auth logout). The new behavior matches the documented intent in AGENTS.md.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/19 **Author:** [@rudrankriyam](https://github.com/rudrankriyam) **Created:** 1/20/2026 **Status:** ✅ Merged **Merged:** 1/20/2026 **Merged by:** [@rudrankriyam](https://github.com/rudrankriyam) **Base:** `main` ← **Head:** `fix/keychain-priority-over-env` --- ### 📝 Commits (1) - [`4501c2d`](https://github.com/rudrankriyam/App-Store-Connect-CLI/commit/4501c2d159a36450200ce1078be560c22553dfb5) fix: prioritize keychain over env vars for credentials ### 📊 Changes **1 file changed** (+18 additions, -19 deletions) <details> <summary>View changed files</summary> 📝 `cmd/commands.go` (+18 -19) </details> ### 📄 Description ## Summary The credential resolution in `getASCClient()` was incorrectly checking environment variables first and only falling back to keychain when env vars were missing. This violated the documented behavior in AGENTS.md where credentials set via `asc auth login` should take precedence. **Problem**: Users who authenticated with `asc auth login` (storing credentials in keychain) were unexpectedly using credentials from stale `ASC_*` env vars in their `.zshrc`/`.bashrc`. **Fix**: Swap the credential resolution order: 1. **Priority 1**: Keychain credentials (explicit user setup via `asc auth login`) 2. **Priority 2**: Environment variables (fallback for CI/CD or when keychain unavailable) ## Test Plan - [x] `make test` passes (all 58 tests) - [x] `go fmt ./...` clean - [x] Manual testing: verified keychain credentials now take precedence over env vars ## Breaking Change? No. Users relying on env var overrides can still use them by clearing their keychain credentials (`asc auth logout`). The new behavior matches the documented intent in AGENTS.md. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>