starred/App-Store-Connect-CLI

Fork 0

mirror of https://github.com/rudrankriyam/App-Store-Connect-CLI.git synced 2026-04-25 07:35:48 +03:00

[GH-ISSUE #522] Keychain migration cleanup should remove stale global config credentials when local config is active #146

New issue

Closed

opened 2026-02-26 21:33:46 +03:00 by kerem · 3 comments

kerem commented

2026-02-26 21:33:46 +03:00

Owner

Originally created by @rudrankriyam on GitHub (Feb 14, 2026).
Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/522

Summary

After successful keychain storage, config cleanup can miss credentials stored in global config when a repo-local config is active.

Why this matters

Users can end up with stale credentials left in ~/.asc/config.json even after migration, causing confusion and potential fallback use when keychain is bypassed/unavailable.

Current behavior

removeFromConfigIfPresent -> removeFromConfig uses config.Load() / config.Path(), which prefer local config if present.
This can skip cleanup in global config.

Expected behavior

Cleanup should be applied to all relevant config scopes (active + global), similar to clearConfigCredentials() behavior.

Reproduction

Store credentials in global config.
Enter repo with local .asc/config.json.
Run keychain-backed login/migration flow.
Observe global config credentials may remain.

Detailed implementation plan

Refactor single-profile removal helper to support path-scoped removal.
Apply removal to both active path and global path (when different).
Preserve non-target credentials and file integrity.
Add tests for mixed local/global scenarios.
Ensure no regressions for ASC_CONFIG_PATH overrides.

Acceptance criteria

Successful keychain storage removes matching stale config credentials from both active and global locations where applicable.
Local-only and global-only setups continue to behave correctly.
Multi-profile config behavior remains intact.

cc @cursor please implement this end-to-end.

Originally created by @rudrankriyam on GitHub (Feb 14, 2026). Original GitHub issue: https://github.com/rudrankriyam/App-Store-Connect-CLI/issues/522 ## Summary After successful keychain storage, config cleanup can miss credentials stored in global config when a repo-local config is active. ## Why this matters Users can end up with stale credentials left in `~/.asc/config.json` even after migration, causing confusion and potential fallback use when keychain is bypassed/unavailable. ## Current behavior `removeFromConfigIfPresent` -> `removeFromConfig` uses `config.Load()` / `config.Path()`, which prefer local config if present. This can skip cleanup in global config. ## Expected behavior Cleanup should be applied to all relevant config scopes (active + global), similar to `clearConfigCredentials()` behavior. ## Reproduction 1. Store credentials in global config. 2. Enter repo with local `.asc/config.json`. 3. Run keychain-backed login/migration flow. 4. Observe global config credentials may remain. ## Detailed implementation plan - [ ] Refactor single-profile removal helper to support path-scoped removal. - [ ] Apply removal to both active path and global path (when different). - [ ] Preserve non-target credentials and file integrity. - [ ] Add tests for mixed local/global scenarios. - [ ] Ensure no regressions for `ASC_CONFIG_PATH` overrides. ## Acceptance criteria - [ ] Successful keychain storage removes matching stale config credentials from both active and global locations where applicable. - [ ] Local-only and global-only setups continue to behave correctly. - [ ] Multi-profile config behavior remains intact. cc @cursor please implement this end-to-end.

kerem

2026-02-26 21:33:46 +03:00

closed this issue
added the
bug
label

kerem commented

2026-02-26 21:33:47 +03:00

Author

Owner

@rudrankriyam commented on GitHub (Feb 14, 2026):

@cursor please implement this issue end-to-end using the checklist and acceptance criteria.

@rudrankriyam commented on GitHub (Feb 14, 2026): @cursor please implement this issue end-to-end using the checklist and acceptance criteria.

kerem commented

2026-02-26 21:33:47 +03:00

Author

Owner

@cursor[bot] commented on GitHub (Feb 14, 2026):

Implemented end-to-end on cursor/keychain-migration-cleanup-scope-50c1 and pushed in two commits:

b6034f4 — Fix keychain migration cleanup across config scopes
dcc4e25 — Remove unused config cleanup wrapper

What I changed:

Refactored credential cleanup to be path-scoped and reusable:
- Added removeFromConfigAt(name, path) so removal can target specific config files.
- Added configCleanupPaths() to resolve active + global config paths (deduped when same).
Updated removeFromConfigIfPresent to apply cleanup across all relevant scopes:
- Removes matching stale credentials from active and global configs where applicable.
- Preserves existing behavior for not-found/missing-file cases.
Updated clearConfigCredentials() to use the same active+global path resolution helper.
Added regression tests for mixed-scope and override behavior:
- TestStoreCredentials_RemovesStaleGlobalCredentialWhenLocalConfigActive
- TestStoreCredentials_RemovesStaleCredentialFromOverrideAndGlobalConfigs
- Both assert non-target credentials are preserved and config files remain valid.

Validation performed:

Reproduced bug with new tests (RED), then verified fix (GREEN).
Ran focused regression suite:
- go test ./internal/auth -run 'TestStoreCredentials(FallbackToConfig|_RemovesStaleGlobalCredentialWhenLocalConfigActive|_RemovesStaleCredentialFromOverrideAndGlobalConfigs)$' -count=1 ✅
Ran required repo checks:
- make format ✅
- make lint ✅
- make test ✅

@cursor[bot] commented on GitHub (Feb 14, 2026): Implemented end-to-end on `cursor/keychain-migration-cleanup-scope-50c1` and pushed in two commits: - `b6034f4` — **Fix keychain migration cleanup across config scopes** - `dcc4e25` — **Remove unused config cleanup wrapper** What I changed: - Refactored credential cleanup to be **path-scoped** and reusable: - Added `removeFromConfigAt(name, path)` so removal can target specific config files. - Added `configCleanupPaths()` to resolve **active + global** config paths (deduped when same). - Updated `removeFromConfigIfPresent` to apply cleanup across all relevant scopes: - Removes matching stale credentials from active and global configs where applicable. - Preserves existing behavior for not-found/missing-file cases. - Updated `clearConfigCredentials()` to use the same active+global path resolution helper. - Added regression tests for mixed-scope and override behavior: - `TestStoreCredentials_RemovesStaleGlobalCredentialWhenLocalConfigActive` - `TestStoreCredentials_RemovesStaleCredentialFromOverrideAndGlobalConfigs` - Both assert non-target credentials are preserved and config files remain valid. Validation performed: - Reproduced bug with new tests (RED), then verified fix (GREEN). - Ran focused regression suite: - `go test ./internal/auth -run 'TestStoreCredentials(FallbackToConfig|_RemovesStaleGlobalCredentialWhenLocalConfigActive|_RemovesStaleCredentialFromOverrideAndGlobalConfigs)$' -count=1` ✅ - Ran required repo checks: - `make format` ✅ - `make lint` ✅ - `make test` ✅ <p><a href="https://github.com/rudrankriyam/App-Store-Connect-CLI/pull/530"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/view-pr-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/view-pr-light.png"><img alt="View PR" width="86" height="28" src="https://cursor.com/assets/images/view-pr-light.png"></picture></a></p> <p><a href="https://cursor.com/background-agent?bcId=bc-6575c373-004e-4fab-b340-34cc4de2d208"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a> <a href="https://cursor.com/agents?id=bc-6575c373-004e-4fab-b340-34cc4de2d208"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a></p>

kerem commented

2026-02-26 21:33:47 +03:00

Author

Owner

@rudrankriyam commented on GitHub (Feb 14, 2026):

Fixed in #530 (merged to main).

@rudrankriyam commented on GitHub (Feb 14, 2026): Fixed in #530 (merged to main).

kerem referenced this issue

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

[PR #146] [MERGED] Signing certificate handling #327