Skip to content

feat(server): provider credentials can live in 1Password - #27

Merged
yordis merged 6 commits into
mainfrom
yordis/feat-provider-secret-references
Aug 20, 2026
Merged

feat(server): provider credentials can live in 1Password#27
yordis merged 6 commits into
mainfrom
yordis/feat-provider-secret-references

Conversation

@yordis

@yordis yordis commented Aug 20, 2026

Copy link
Copy Markdown
Member
  • Running several accounts of one provider means several long-lived tokens, and each one had to be copied out of the password manager into a second store before it was usable. That second copy is the one nobody rotates: it outlives the original, has no expiry anyone tracks, and sits in a file that gets synced and backed up.
  • Pointing at the secret instead of duplicating it keeps a single copy under the policy that was chosen for it, and turns rotation into a vault edit plus a refresh rather than a hunt for everywhere the value was pasted.
  • Holding the resolved value in memory is what makes it usable rather than merely correct. Reading the vault on every thread start turns an occasional biometric prompt into a constant interruption, and that friction ends with the token pasted back in plaintext to make it stop.
  • A reference that cannot be read leaves the variable unset rather than blank, so the provider reports as not authenticated instead of starting with an empty credential and failing on the first message.

yordis added 2 commits August 20, 2026 05:17
Running several accounts of one provider means several long-lived tokens, and
each had to be copied out of the password manager into a second store to be
usable. That copy is the one nobody rotates. Pointing at the secret keeps a
single copy under the policy chosen for it, and makes rotation a vault edit
rather than a hunt for everywhere the value was pasted.

Signed-off-by: Yordis Prieto <[email protected]>
@cursor

cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

PR Summary

High Risk
Touches credential handling and provider instance lifecycle: secrets are read via the 1Password CLI, cached in memory, and instances are torn down/rebuilt on refresh. Bugs here can leak inherited credentials, drop providers, or race settings reconcile.

Overview
Lets users paste 1Password op:// references as provider environment values instead of storing the secret. Drivers resolve those references at create time via op read; only the reference is persisted.

Resolved values are cached in process until Settings refresh. Refresh invalidates the cache and rebuilds only instances that use secret references (including currently unavailable ones), so a rotated vault item reaches a new provider process without restarting every agent. Failed reads unset the variable rather than substituting empty or inheriting the server’s env, so the provider shows unauthenticated.

The instance registry gains rebuildInstanceWhen: in-place replace, mutex with reconcile, park the last snapshot while the secret store is slow, and keep failed/interrupted rebuilds retryable without shuffling list order.

Reviewed by Cursor Bugbot for commit b5d7b9b. Bugbot is set up for automated code reviews on this repo. Configure here.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:XL labels Aug 20, 2026
@coderabbitai

coderabbitai Bot commented Aug 20, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@yordis, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 21 minutes

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

Wait for the limit to reset, then comment @coderabbitai review or push new commits to the PR.

An organization admin can change what happens after included review limits in Billing.

How do review limits work?

CodeRabbit enforces per-developer PR review limits within each organization.

For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bfcd070c-75c3-43a2-86da-180f605c4b96

📥 Commits

Reviewing files that changed from the base of the PR and between 84e3dad and b5d7b9b.

📒 Files selected for processing (26)
  • apps/server/src/provider/Drivers/ClaudeDriver.ts
  • apps/server/src/provider/Drivers/CodexDriver.ts
  • apps/server/src/provider/Drivers/CursorDriver.ts
  • apps/server/src/provider/Drivers/GrokDriver.ts
  • apps/server/src/provider/Drivers/OpenCodeDriver.ts
  • apps/server/src/provider/Layers/ProviderAdapterRegistry.test.ts
  • apps/server/src/provider/Layers/ProviderInstanceRegistryLive.test.ts
  • apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts
  • apps/server/src/provider/Layers/ProviderRegistry.test.ts
  • apps/server/src/provider/Layers/ProviderRegistry.ts
  • apps/server/src/provider/Layers/ProviderSecretResolverLive.test.ts
  • apps/server/src/provider/Layers/ProviderSecretResolverLive.ts
  • apps/server/src/provider/ProviderInstanceEnvironment.test.ts
  • apps/server/src/provider/ProviderInstanceEnvironment.ts
  • apps/server/src/provider/ProviderSecretReference.test.ts
  • apps/server/src/provider/ProviderSecretReference.ts
  • apps/server/src/provider/Services/ProviderInstanceRegistry.ts
  • apps/server/src/provider/Services/ProviderSecretResolver.ts
  • apps/server/src/server.ts
  • apps/server/src/textGeneration/TextGeneration.test.ts
  • docs/README.md
  • docs/fork/0016-provider-secrets-live-in-1password.md
  • docs/fork/README.md
  • docs/internals/glossary.md
  • docs/internals/providers.md
  • docs/user/provider-secrets.md
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch yordis/feat-provider-secret-references

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

Comment thread apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts Outdated
Comment thread apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts Outdated
Comment thread apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts
Resolving a secret can park on a person at a biometric prompt, and for that
whole window a rebuild was observable: lookups handed back a torn-down
instance, a settings change landing mid-rebuild was silently reverted, and an
instance a locked vault had failed to rebuild stayed gone until settings
changed.

Signed-off-by: Yordis Prieto <[email protected]>
Comment thread apps/server/src/provider/Layers/ProviderRegistry.ts
Comment thread apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts
An instance being rebuilt is briefly neither live nor unavailable, and the
aggregator treats an id it finds in neither list as gone, so a provider could
vanish from Settings for as long as the secret store took to answer. A refresh
with no explicit target also only reached live instances, which left the one
instance that most needed retrying, the one a locked vault had just failed to
rebuild, waiting on a settings edit instead.

Signed-off-by: Yordis Prieto <[email protected]>
Comment thread apps/server/src/provider/Layers/ProviderSecretResolverLive.ts
…n credential

A provider process starts from the server's environment, so leaving an
unreadable reference out of the resolved variables was not the same as leaving
it unset: a machine that already exported the same name handed the agent that
credential instead, under the account the instance did not name, and reported
it as authenticated. This is the behavior the secrets guide already describes.

Signed-off-by: Yordis Prieto <[email protected]>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 1f94869. Configure here.

Comment thread apps/server/src/provider/Layers/ProviderInstanceRegistryLive.ts
… rebuilding

The request that asks for a refresh can go away while 1Password is still
waiting on a fingerprint. The registry only recorded what it needed to retry
after the build returned, so an interrupt in that window left the instance with
no live entry and no envelope, and the refresh button, its only recovery path,
had nothing to act on.

Signed-off-by: Yordis Prieto <[email protected]>
@yordis
yordis merged commit 4ecc703 into main Aug 20, 2026
10 of 14 checks passed
@yordis
yordis deleted the yordis/feat-provider-secret-references branch August 20, 2026 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XL vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant