[PR #786] [MERGED] fix keychain auth when original .p8 file is removed #786

New issue

Closed

opened 2026-02-26 22:32:33 +03:00 by kerem · 0 comments

kerem commented

2026-02-26 22:32:33 +03:00

Owner

📋 Pull Request Information

Original PR: https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/786
Author: @rudrankriyam
Created: 2/26/2026
Status: ✅ Merged
Merged: 2/26/2026
Merged by: @rudrankriyam

Base: main ← Head: fix/keychain-review-details-create-auth

📝 Commits (4)

13abd9f fix keychain auth when original .p8 file is deleted
a0ffcf4 refactor keychain auth to avoid runtime temp key files
a6c15c1 fix auth diagnostics for keychain PEM-only credentials
38aa534 remove unused exported PEM http-client constructor

📊 Changes

11 files changed (+459 additions, -51 deletions)

View changed files

📝 internal/asc/client_core.go (+20 -2)
➕ internal/asc/client_core_auth_test.go (+55 -0)
📝 internal/auth/doctor.go (+15 -0)
📝 internal/auth/doctor_test.go (+39 -0)
📝 internal/auth/keychain.go (+50 -14)
📝 internal/auth/keychain_test.go (+107 -0)
📝 internal/cli/auth/auth.go (+29 -11)
📝 internal/cli/auth/auth_test.go (+28 -0)
📝 internal/cli/shared/shared.go (+23 -15)
📝 internal/cli/shared/shared_test.go (+92 -9)
📝 internal/config/config.go (+1 -0)

📄 Description

Summary

store encrypted private key material in keychain credential payloads (keychain mode only) while keeping private_key_path metadata for compatibility
use an in-memory auth path for keychain-backed credentials by adding asc.NewClientFromPEM(...), so runtime auth no longer materializes temporary key files from keychain data
align auth diagnostics with runtime behavior: auth status --validate and auth doctor now validate keychain PEM credentials without requiring the original key file path
backfill legacy keychain entries that only have private_key_path when the source file still exists, and add regression tests for PEM persistence, legacy backfill, deleted-file resolution, and shared auth resolution when only PEM is present

Test plan

go test ./internal/cli/auth ./internal/auth ./internal/cli/shared ./internal/asc
make format
make lint
ASC_BYPASS_KEYCHAIN=1 make test
Live repro on local keychain profile:
- temporarily move original .p8
- verify localizations list succeeds (auth works)
- verify review details-create reaches API validation (no private key path error)
- verify auth status --validate succeeds from keychain PEM with missing original file
- verify no /tmp/asc-keychain-key-*.p8 files are created
- restore original .p8

_{🔄 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/786 **Author:** [@rudrankriyam](https://github.com/rudrankriyam) **Created:** 2/26/2026 **Status:** ✅ Merged **Merged:** 2/26/2026 **Merged by:** [@rudrankriyam](https://github.com/rudrankriyam) **Base:** `main` ← **Head:** `fix/keychain-review-details-create-auth` --- ### 📝 Commits (4) - [`13abd9f`](https://github.com/rudrankriyam/App-Store-Connect-CLI/commit/13abd9ffad1f1c0270440e1e57e47a334d29b983) fix keychain auth when original .p8 file is deleted - [`a0ffcf4`](https://github.com/rudrankriyam/App-Store-Connect-CLI/commit/a0ffcf443e2a3d4e56960f48ecf9b9dedaef76df) refactor keychain auth to avoid runtime temp key files - [`a6c15c1`](https://github.com/rudrankriyam/App-Store-Connect-CLI/commit/a6c15c1240212623559c517a0665a9cd03488414) fix auth diagnostics for keychain PEM-only credentials - [`38aa534`](https://github.com/rudrankriyam/App-Store-Connect-CLI/commit/38aa534ebe35a2c1cf3922245088b01e3c0179a8) remove unused exported PEM http-client constructor ### 📊 Changes **11 files changed** (+459 additions, -51 deletions) <details> <summary>View changed files</summary> 📝 `internal/asc/client_core.go` (+20 -2) ➕ `internal/asc/client_core_auth_test.go` (+55 -0) 📝 `internal/auth/doctor.go` (+15 -0) 📝 `internal/auth/doctor_test.go` (+39 -0) 📝 `internal/auth/keychain.go` (+50 -14) 📝 `internal/auth/keychain_test.go` (+107 -0) 📝 `internal/cli/auth/auth.go` (+29 -11) 📝 `internal/cli/auth/auth_test.go` (+28 -0) 📝 `internal/cli/shared/shared.go` (+23 -15) 📝 `internal/cli/shared/shared_test.go` (+92 -9) 📝 `internal/config/config.go` (+1 -0) </details> ### 📄 Description ## Summary - store encrypted private key material in keychain credential payloads (keychain mode only) while keeping `private_key_path` metadata for compatibility - use an in-memory auth path for keychain-backed credentials by adding `asc.NewClientFromPEM(...)`, so runtime auth no longer materializes temporary key files from keychain data - align auth diagnostics with runtime behavior: `auth status --validate` and `auth doctor` now validate keychain PEM credentials without requiring the original key file path - backfill legacy keychain entries that only have `private_key_path` when the source file still exists, and add regression tests for PEM persistence, legacy backfill, deleted-file resolution, and shared auth resolution when only PEM is present ## Test plan - [x] `go test ./internal/cli/auth ./internal/auth ./internal/cli/shared ./internal/asc` - [x] `make format` - [x] `make lint` - [x] `ASC_BYPASS_KEYCHAIN=1 make test` - [x] Live repro on local keychain profile: - temporarily move original `.p8` - verify `localizations list` succeeds (auth works) - verify `review details-create` reaches API validation (no private key path error) - verify `auth status --validate` succeeds from keychain PEM with missing original file - verify no `/tmp/asc-keychain-key-*.p8` files are created - restore original `.p8` --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>