feat(auth): Add 1Password keyring backend#891
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 30, 2026, 7:11 AM ET / 11:11 UTC. Summary Reproducibility: not applicable. for the feature request itself; the patch bug is source-reproducible because the new cache hit returns before stored-grant validation and cache entries record requested scopes. I did not run a live Google/1Password setup in this read-only review. Review metrics: 2 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Land only after the cache preserves stored-grant semantics, the PR includes redacted real 1Password auth proof, and maintainers explicitly accept the new SDK dependency surface. Do we have a high-confidence way to reproduce the issue? Not applicable for the feature request itself; the patch bug is source-reproducible because the new cache hit returns before stored-grant validation and cache entries record requested scopes. I did not run a live Google/1Password setup in this read-only review. Is this the best way to solve the issue? No; the 1Password backend may be a useful direction, but the cache needs to preserve grant-check semantics and the new SDK dependency needs explicit acceptance before this is the best merge path. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 213ddb60d7d1. Label changesLabel changes:
Label justifications:
Evidence reviewedSecurity concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
|
Could break this up into a smaller one @steipete if that makes it easier, but without the caching it's annoying to have at approve every cli invocation. |
|
Going to break this up into smaller pieces. |
Local and agent workflows currently have to choose between OS keychains, which can prompt repeatedly or behave poorly in headless contexts, and the encrypted file backend, which requires distributing
GOG_KEYRING_PASSWORD. This adds a 1Password-backed option so gog auth material can live in a configured 1Password vault while local desktop workflows can use the 1Password app instead of a service account token.This change adds a
1passwordkeyring backend using the 1Password Go SDK. It supports desktop-app auth throughonepassword_account/GOG_1PASSWORD_ACCOUNT, service-account auth throughOP_SERVICE_ACCOUNT_TOKEN, and vault/item selection through config or environment. 1Password items are API Credential entries titledgogcli-keyringby default, with the keyring key inusernameand the encoded payload in the concealedcredentialfield.Example local setup:
Repeated short-lived subprocesses can also opt in to a state-dir access-token cache:
The cache stores only short-lived Google access tokens, checks it before reopening the refresh-token keyring, and clears it when stored auth is removed. That keeps agent-style Gmail/Calendar loops from repeatedly asking for 1Password approval while preserving 1Password as the source of truth for refresh tokens.
A couple of guardrails are included for review:
gogcli-keyringAPI Credential items as managed, except for an empty starter template that is tagged on first use.OP_SERVICE_ACCOUNT_TOKENremains environment-only; non-secret selectors can live inconfig.json.auth doctorreports the selected 1Password auth mode and missing vault/account/token configuration.