feat(auth): Add 1Password keyring backend#897
Conversation
Co-authored-by: Lachlan Donald <lachlan+codex@buildkite.com>
|
Codex review: needs real behavior proof before merge. Reviewed July 4, 2026, 5:49 AM ET / 09:49 UTC. Summary Reproducibility: not applicable. this is a feature PR rather than a reported bug. The relevant check is after-fix runtime proof for the new native-SDK backend, and that proof is missing from the current PR. Review metrics: 3 noteworthy metrics.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. 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: Decide whether core should accept this native-SDK backend, ask for an Do we have a high-confidence way to reproduce the issue? Not applicable; this is a feature PR rather than a reported bug. The relevant check is after-fix runtime proof for the new native-SDK backend, and that proof is missing from the current PR. Is this the best way to solve the issue? Unclear; the implementation is focused and current main has no equivalent backend, but the recorded maintainer direction on the predecessor favored shelling out to Full review comments:
Overall correctness: patch is correct AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against c0aac99f6be6. 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
|
|
@steipete totally happy to provide an option for just using The other angle I am working on is I've forked the dead https://github.com/99designs/keyring that this project depends on (I was the original author at my first startup) into https://github.com/lox/keyring. It would provide a unified interface to the various keyring providers (including 1password) that this cli uses. |
The previous 1Password backend PR was closed because the official SDK added too much binary and dependency weight to the CLI. This keeps the same opt-in keyring backend behavior, but switches the implementation to
github.com/lox/onepassword-sdk-native-goso gog does not embed the official SDK's WASM runtime stack.Measured on this branch against the commit immediately before the SDK swap:
go.modnow has one new direct dependency on the native SDK and no longer needsgithub.com/1password/onepassword-sdk-go,github.com/tetratelabs/wazero,github.com/extism/go-sdk,github.com/tetratelabs/wabin, or the related WASM/Extism support packages.The feature remains opt-in: users configure
GOG_KEYRING_BACKEND=1passwordorgog auth keyring 1password, then set the 1Password vault/account/auth-mode config for either desktop app integration or service-account auth. Static/no-CGO builds still fail closed for desktop-app mode, while service-account mode remains available.