fix(config): check file fallback when keyring has no entry - #9279
Conversation
Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
![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: 4cf53ba36f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| || msg.contains("No matching entry found") => | ||
| { | ||
| HashMap::new() | ||
| self.fallback_to_file_storage()? |
There was a problem hiding this comment.
When get_password() returns a "No entry found" error from an otherwise functional keyring (e.g., first run or after a user deletes the keychain item), this branch now reads secrets.yaml instead of treating the keyring as authoritative-empty. That can silently resurrect stale credentials from prior fallback runs and make get_secret succeed even though the keyring entry was intentionally removed, which is a behavioral and security regression tied to this new call path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
secrets.yaml only gets written when is_keyring_availability_error returns true – meaning the keyring is genuinely unavailable (no dbus, no secret service). on a system with a working keyring, the fallback path never creates that file
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
Nice catch! Clean, minimal fix for a real bug. The logic is sound — fallback_to_file_storage correctly reads secrets.yaml when it exists (written during a prior keyring-unavailable session) and returns an empty HashMap when it doesn't, so this is safe on systems with a working keyring too. Thanks for the fix! 🎉
* origin/main: (160 commits) Add Linux musl CLI builds (#9240) feat(acp): paginate session list (#9199) docs: reorganize (#9310) Structured per-provider config block, non-destructive provider switching (#8977) feat(cli): add `goose review` local code review command (#9114) feat(tui): diff viewer (#9260) fix(otel): emit trace_output as span attribute instead of event (#9255) docs: add guide for connecting goose Desktop to a remote goosed server (#9275) fix(config): check file fallback when keyring has no entry (#9279) fix(desktop): ScheduleModal error message styling (#9278) fix(ui): align sidebar hamburger in macOS fullscreen (#9257) Add documentation for new provider SaladCloud AI Gateway (#9253) fix: use current_exe() instead of PATH lookup when spawning goose (#9236) fix(extension_manager): set TCP_USER_TIMEOUT on streamable HTTP clients (#9207) fix: activate custom provider after adding via configure (#9213) Flush OTLP traces reliably on exit with configurable timeout (#9228) fix: reduce excessive MISSING_TRANSLATION warnings for fallback locales (#9294) feat(acp): pass session cwd param to acp providers (#9229) fix(desktop): eliminate cross-window deep link contamination (#9273) fix: improve Telegram gateway error reporting and connection reliability (#9223) ... Signed-off-by: Michael Neale <michael.neale@gmail.com> # Conflicts: # crates/goose/src/agents/agent.rs # crates/goose/tests/agent.rs
* main: (70 commits) Feat/summon subagent instructions (#9325) feat: open-plugins generalization + skills (#9112) feat(hooks): PreToolUse denial (#9304) Add support for optional api_key configuration for declarative openai-engine providers (#9202) fix(cli): use plain '> ' prompt instead of goose emoji (#9305) flag for login shell PATH (#9313) Remove popular chat topics from new chat screen (#9307) fix: stop killing goosed when a window closes (#9302) Remove vendored Windows binaries (#9318) Add Linux musl CLI builds (#9240) feat(acp): paginate session list (#9199) docs: reorganize (#9310) Structured per-provider config block, non-destructive provider switching (#8977) feat(cli): add `goose review` local code review command (#9114) feat(tui): diff viewer (#9260) fix(otel): emit trace_output as span attribute instead of event (#9255) docs: add guide for connecting goose Desktop to a remote goosed server (#9275) fix(config): check file fallback when keyring has no entry (#9279) fix(desktop): ScheduleModal error message styling (#9278) fix(ui): align sidebar hamburger in macOS fullscreen (#9257) ...
…e#9279) Signed-off-by: sheikhlimon <sheikhlimon404@gmail.com>
2 participants
Summary
Fixes #8902
When keyring is unavailable and secrets were written to the file fallback, reading secrets back returned empty because the "No entry found" handler skipped the file entirely. Now tries
fallback_to_file_storage()before concluding no secrets exist.