fix: harden install and credential cleanup#14
Merged
Conversation
Store API keys under a stable keyring entry instead of deriving the entry name from base URL and account ID. The old scheme could leave credentials behind if config.yaml was edited manually before logout. On first read, migrate an existing URL/account-scoped key to the stable api-key entry and delete the legacy entry. Keep a TODO to remove the migration in v1 after users have had a release cycle to move forward. Logout now clears the whole chatwoot-cli keyring service even when config.yaml is missing, so stale entries are removed independently of the current config state.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cc2d68eba
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sanitize server-provided strings before writing auth login output, raw API responses, verbose non-JSON bodies, and API error bodies to the terminal. Expand verbose JSON redaction to cover hmac_identifier and common secret-like field names such as *_token, *_secret, *_key, and hmac_* values. Tests cover terminal escape stripping for the affected output paths and redaction of HMAC/secret-like response fields.
Store stable keyring credentials as scoped JSON containing the normalized base URL, account ID, and API key instead of a raw token. ResolveAPIKey now rejects saved credentials when the current config points at a different instance or account, preventing a token from being sent to the wrong host after config.yaml is edited or restored. Legacy scoped entries are migrated into the new scoped JSON format and then deleted.
Merging this branch will increase overall coverage
Coverage by fileChanged files (no unit tests)
Please note that the "Total", "Covered", and "Missed" counts above refer to code statements instead of lines of code. The value in brackets refers to the test coverage of that file in the old version of the code. Changed unit test files
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR has the following changes
chatwoot-clikeyring entries, even without confighmac_identifierin verbose outputTests
go test ./...sh -n install.sh