Skip to content

Add reusable OAuth lifecycle custody - #2602

Draft
3mdistal wants to merge 4 commits into
mainfrom
codex/oauth-lifecycle-foundation
Draft

Add reusable OAuth lifecycle custody#2602
3mdistal wants to merge 4 commits into
mainfrom
codex/oauth-lifecycle-foundation

Conversation

@3mdistal

@3mdistal 3mdistal commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Problem

Agent Native integrations each need the same sensitive OAuth lifecycle behavior: credentials must stay bound to the intended provider, resource, and owner; rotating refresh tokens must have one redeemer across concurrent callers; expired or malformed grants must fail into explicit reconnect states; and disconnect must attempt provider revocation without deleting a newer authorization.

Without a shared foundation, later managed AI and Fusion integrations would either duplicate this security logic or inherit the obsolete installation-key design from closed PR #2515.

Approach

Add a provider-agnostic lifecycle layer over the existing encrypted OAuth token store. The public product direction can still present one Builder sign-in, while each future capability keeps a distinct resource-bound grant underneath.

This PR does not add Builder connection UI, managed AI or Fusion consumers, BuilderSync migration, feature enablement, credentials, or deployment changes.

What changed

  • Added resource-derived, owner-scoped credential identity plus explicit missing, malformed, connected, expired, and reconnect_required states.
  • Added revision compare-and-swap writes and atomically increasing row revisions so stale refresh and revocation work cannot overwrite or delete a newer authorization.
  • Added a database-backed refresh lease with heartbeat renewal. One caller redeems a rotating refresh token; concurrent callers wait and reload the winning credential.
  • Fenced both successful and failed refresh completion with renewed lease ownership. A stale successful redeemer cannot write after a successor has acquired the lease, and a stale failure cannot mark the shared credential for reconnect.
  • Made owner binding atomic at the upsert itself. Concurrent first saves by different owners cannot silently rebind custody; the loser receives the existing 409 conflict.
  • Added provider revocation hooks that report remote and local outcomes separately and always remove the inspected local credential unless a newer revision replaced it.
  • Adapted existing remote MCP OAuth to the shared lifecycle through an explicit legacy storage-key bridge, preserving exported helper compatibility and validating the requested MCP resource on every token path.
  • Routed MCP OAuth discovery, registration, token exchange, refresh, and every redirect hop through the DNS-aware SSRF guard. Cross-origin redirects strip credential headers, and 307/308 redirects cannot forward an OAuth request body to another origin. Private origins are allowed only through deployment-owned AGENT_NATIVE_MCP_OAUTH_PRIVATE_ORIGINS configuration.
  • Hardened MCP revocation with the same DNS-aware SSRF guard, HTTPS-only requests, no redirects, and local deletion even when remote revocation fails.

Safety and operations

  • The generic foundation is dormant until a later capability lane calls it; this PR adds no Builder UI or default-on rollout. Existing remote MCP OAuth uses the shared lifecycle behind its unchanged UI.
  • Tokens remain AES-256-GCM encrypted in oauth_tokens. Lease keys contain only a SHA-256 digest of credential identity.
  • Existing MCP rows remain readable through the legacy key bridge. No credential rotation or legacy BuilderSync removal occurs here.
  • Rollback is code-only. Credentials written through the generic API use resource-derived account keys and remain encrypted rows; no Builder consumer writes them in this PR.
  • Provider-controlled OAuth URLs are DNS-checked again at connection time. Request data cannot opt a private origin into the allowlist.
  • Remote revocation never follows redirects or allows discovery-derived private origins. A failed or unsupported provider revocation is reported separately from local cleanup.

Verification

  • Refreshed onto main at 5007e0713 without conflicts or overlap in the 11-file OAuth diff.
  • Pinned Node 24 affected-spec run passed 29 tests covering MCP OAuth redirects and rotating-refresh lease loss.
  • The sequential full Core suite passed 10,927 tests across 809 files, with one unrelated test skipped.
  • All 45 repository guards passed, including credential scoping, secret literals, one-sign-in, and SSR/cache boundaries.
  • Core typecheck and build passed, including CLI TypeScript and distribution import checks; changed-file formatting and git diff --check passed.
  • Independent exact-head security review found and then re-reviewed two blocking boundaries: cross-origin 307/308 body forwarding and stale successful refresh writes after lease loss. Both repairs are covered by regressions, and the follow-up found no remaining blocker.
  • The remaining non-blocking evidence gap is unchanged: lease concurrency is exercised deterministically in-process rather than through truly independent processes and database connections.
  • All current GitHub CI and preview checks passed or were skipped by design against the repaired exact head.

Human acceptance boundary

  • There is no UI diff and therefore no changed visual state or honest screenshot to review.
  • The existing MCP OAuth journey remains the only current user-facing adapter: consent and callback, persisted connection and authenticated token use, refresh/concurrency, disconnect/reconnect, cancellation/failure handling, and replay rejection are covered as far as the fixture-backed test surface permits.
  • Independent browser execution is not claimed. No real provider was authorized for this foundation refresh.
  • Builder managed-AI consent, workspace membership, reconnect, and capability-specific disconnect UI belong to a later draft consumer lane.

Review focus

  • Does the default resource-derived storage identity provide the right isolation while keeping the MCP legacy bridge narrow enough?
  • Are lease renewal, revision CAS, lease-owned reconnect transitions, and waiter reload semantics sufficient for rotating refresh tokens across server processes?
  • Does the DNS-aware MCP OAuth fetch path preserve required same-origin redirect behavior while preventing OAuth secrets from crossing origins?
  • Does remote revocation balance provider cleanup, SSRF safety, and preservation of a concurrently reauthorized credential?

Follow-ups

  • Add a true multi-process/database-connection concurrency test when a suitable fixture exists.
  • Shape and implement a managed AI capability lane with its own resource/scopes and default-off acceptance story.
  • Shape and implement the Fusion remote MCP lane separately against the landed Builder and AI Services contracts.
  • Keep BuilderSync legacy authentication in place until each replacement lane is proven in production.

@github-actions

github-actions Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Visual recap — skipped

The visual recap job did not run for this pull request. This is informational only and does not block the PR.

Recap skipped for 4ab202a: draft PR.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

@netlify

This comment has been minimized.

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