Skip to content

v2: 1Password credential provider (org + personal service accounts, reference credentials) - #163

Open
yourbuddyconner wants to merge 14 commits into
dev-v2from
feat/onepassword-credentials
Open

v2: 1Password credential provider (org + personal service accounts, reference credentials)#163
yourbuddyconner wants to merge 14 commits into
dev-v2from
feat/onepassword-credentials

Conversation

@yourbuddyconner

@yourbuddyconner yourbuddyconner commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Credentials can now be stored as 1Password secret references (op://vault/item/field) instead of raw secrets. The reference rides an ordinary credential row's metadata; the secret is resolved at read time via the 1Password SDK using an org-level or personal service-account token, and is never persisted.

Design: docs/specs/2026-07-21-onepassword-credentials-design.md (incl. Deviations) · Plan: docs/plans/2026-07-21-onepassword-credentials.md

What's in here

  • packages/api/src/services/onepassword.ts — the only @1password/sdk (^0.4.0) import point: client cache, 5-min resolve cache, token lookup (reserved service onepassword, org- or user-owned), typed OnePasswordAuthError.
  • Owner-precedence contract (services/credential-resolution.ts): credential reads follow user row → org row precedence for ALL credential kinds, with 1Password reference resolution built into the read — one shared helper consumed by the session resolver, the workflow action invoker, and ChannelHost. Reserved onepassword rows (the tokens themselves) are excluded from the read path entirely.
  • Resolver wiring in EngineHost.buildCredentialResolver — zero packages/engine changes; errors surface as tool errors, never session failures.
  • /api/onepassword/* — vault/item/field picker backend + org settings (personal-token toggle allowPersonalOnePassword, default on for single-user mode).
  • /api/credentials extension — reference creation with save-time resolve validation; metadata.onepassword is a reserved key (smuggle-guarded); github references are rejected (github keeps its own user→org token-service tiering).
  • Web: org settings 1Password page (token card, toggle, org credential creation), personal token card + picker on connected accounts, reference badges.

Trust model & deliberate behavior changes (documented in-code and in-spec)

  • Connecting the org service-account token grants every org member browse + resolve access to whatever that service account can read (design decision 2). Scope the service account to a dedicated vault in 1Password. Org token CRUD, the toggle, and org-scoped credential rows remain admin-only.
  • Member sessions now read plain org-owned credential rows (e.g. an admin-pasted org-wide Linear key, Slack/Telegram bot tokens) on a user-row miss — not just 1Password references. An admin opts a credential into org-wide sharing by creating the org-owned row at all; mirrors github's existing user→org tiering. Pinned by explicit tests.

Disclosures

  • Owed before merge: the live SDK e2e has not been executed (no real token in the dev environment): OP_SERVICE_ACCOUNT_TOKEN=… [OP_TEST_REFERENCE=…] pnpm --filter @valet/api test -- src/integration/onepassword.live. The suite is verified skip-clean for CI.
  • Resolved values live in an in-memory 5-min TTL cache; token rotation takes up to 5 min to fully bite (deletion and toggle-off bite immediately — the token/toggle are re-checked before every cache read).
  • Pre-existing unrelated failure: llm-providers.e2e (expired real OpenAI key fixture in the local env).

Tests

Every task passed an adversarial per-task review, a whole-branch final review, and a post-PR adversarial pass that caught and fixed a real gap (org-scoped rows were dead on the session path — no owner read-union existed; now resolved by the owner-precedence contract). Battery at HEAD: pnpm typecheck clean · api 1460+ passed · web 447 · engine 430 (zero engine diff vs merge-base).

…dential resolver

Wires the Task 1 OnePasswordService into EngineHost.buildCredentialResolver:
rows carrying metadata.onepassword now resolve through the service instead
of a raw store read, while every other row stays byte-identical. Threads
onePassword through providers/node.ts and the integration test harness.
Add live-gated SDK e2e test (OP_SERVICE_ACCOUNT_TOKEN, skip-clean without
it) and a Deviations section documenting SDK version, the
metadata.onepassword smuggle-bypass fix, and web helper/hook changes found
during the implementation pass.
…erences

Align scope=org browsing with the org token's intended trust model: any
authed org member can browse once the token is connected (org-owned
credential rows and settings stay admin-only). Also reject onepassword
references on the github service, since the session resolver's github
branch ignores them silently.
…ion path

Session.credentialProvider() reads user-owned rows only — the spec's
assumed owner read-union never existed, so admin-created org-wide
reference credentials were dead rows. buildCredentialResolver now falls
back to the org row on a user-owner miss, reference rows only; plain
org rows stay session-invisible. Pinned by 3 tests.
@yourbuddyconner

Copy link
Copy Markdown
Collaborator Author

Post-PR adversarial review found a gap all prior review layers missed: org-scoped reference credentials were dead rows on the session path. The engine's Session.credentialProvider() reads user-owned rows only (no owner read-union exists, contrary to the design doc's original premise), so admin-created org-wide 1Password credentials were created, listed, and badged as connected — but never read by any session.

Fixed in ef154cc: buildCredentialResolver now falls back to the org-owned row on a user-owner miss, reference rows only — a user's own credential always shadows the org one, and plain org-owned rows (ChannelHost bot tokens, workflow credentials) remain session-invisible exactly as before. Three new pinning tests (org-fallback resolve, user-shadows-org, plain-org-invisible); spec corrected in the Data model section + a Deviations entry documenting the false premise.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant