Skip to content

fix(oauth): persist remote MCP OAuth state in Postgres, hashed and encrypted (#155) - #174

Merged
karlwaldman merged 5 commits into
mainfrom
fix/155-oauth-store
Oct 17, 2026
Merged

karlwaldman merged 5 commits into
mainfrom
fix/155-oauth-store

Conversation

@karlwaldman

@karlwaldman karlwaldman commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Fixes #155. Remote MCP OAuth state (DCR clients, consent forms, codes, link replays, access and refresh tokens) moves from in-process Maps to a durable store, so a deploy or DO restart no longer disconnects every Claude and ChatGPT user.

Do not deploy this image until the prerequisites below are in place. Production now fails closed: without the database URL and encryption key the container exits 1 at boot, the DO deploy fails its health check, and the previous deployment stays active.

What changed

File Change
src/oauthStore.ts (new) OAuthStore interface, MemoryOAuthStore (tests/dev), PostgresOAuthStore (one table mcp_oauth_records, created on boot), Sealer (AES-256-GCM + HMAC), oauthPersistenceFromEnv
src/pluginOAuth.ts Provider reads/writes through the store; registerClient is async; client TTL slides on token issue; consent copy no longer says connections drop on restart
src/remote.ts startRemote is async, builds persistence from env, logs oauth_store_ready {durable}, closes the pool on shutdown; capacity 50,000 rows/kind with a durable store (1,000 in memory)
.github/workflows/live-tests.yml Unit job gets a throwaway postgres:16.4-alpine service; npm test runs the real-Postgres store test; the remote container smoke now supplies a DB URL and key and asserts oauth_store_ready {durable:true}
package.json adds pg@8.23.1 (exact, published 2026-09-30), @types/pg@8.23.1 dev

Store choice: Postgres (opa-2025-nyc-db), dedicated database and user

Read-only inspection on 2026-10-15:

  • Isolated app oilpriceapi-openai-mcp (4a89cb34) has no database bound and is not a trusted source on either cluster (doctl databases firewalls list).
  • opa-2025-nyc-db (PG 15, 2 nodes) already hosts per-app databases (entityapi, verifycarrier), so a mcp_oauth database with its own user follows existing practice and keeps the MCP app out of the API's schema.
  • opa-2025-nyc-redis (Valkey 8, noeviction, RDB persistence) was the alternative. Rejected: it is the API's Sidekiq/cache instance, RDB snapshots can lose the last writes on failover (a lost refresh-token rotation logs a user out), and it gives no per-app isolation comparable to a separate Postgres database and user.

Security notes

  • Only hashes at rest for bearer values. Row id = SHA-256 (base64url) of the flow, code, access token, refresh token, or client_id. Lookups are by hash; raw values are never stored. Test asserts the raw values, the PKCE verifier and the API key are absent from every stored row.
  • Customer API key encrypted at rest. Every row payload (PKCE params, redirect, client metadata incl. client_secret, API key) is sealed with AES-256-GCM, key from OILPRICEAPI_MCP_OAUTH_ENCRYPTION_KEY via HKDF. Associated data binds kind:id, so a payload copied onto another row fails to open. A row that fails to decrypt is treated as absent (reject), never as valid.
  • Client secret. The MCP SDK compares client_secret in plaintext at /token, so the client record is sealed rather than hashed.
  • Linked-redirect replay row contains the live code in its redirect URL; it is sealed too.
  • Revocation deletes every code/access/refresh row with the same client and subject, where subject = HMAC-SHA256(derived key, API key). No unkeyed hash of the API key is stored.
  • Single use across instances. Codes and refresh tokens are consumed with DELETE ... RETURNING; a test runs two concurrent refreshes with one token and exactly one succeeds (memory and Postgres).
  • Fail closed. NODE_ENV=production without the key, or without the database URL, refuses to start. A database URL without a key refuses to start in any environment. Key must decode to 32 bytes (base64 or hex).
  • Key rotation. Not supported in this PR: changing the key invalidates every stored connection (users reconnect once). Tested.
  • TLS. If OILPRICEAPI_MCP_OAUTH_DATABASE_CA_CERT is set, the pool verifies the server against that CA (rejectUnauthorized: true) and drops sslmode from the URL, because pg lets the URL override an explicit ssl object.

Deploy prerequisites (Karl action; nothing here was changed)

  1. Create the database and user on opa-2025-nyc-db:
    doctl databases db create 443d035a-f302-4d01-a3e4-e974d6d70fe9 mcp_oauth
    doctl databases user create 443d035a-f302-4d01-a3e4-e974d6d70fe9 mcp_oauth
    
    Then, as doadmin, give the user ownership of its database only: ALTER DATABASE mcp_oauth OWNER TO mcp_oauth; and in mcp_oauth: GRANT ALL ON SCHEMA public TO mcp_oauth;. The app creates its table on first boot.
  2. Bind the cluster to app 4a89cb34 (this also adds the app as a trusted source). PR against deployment/openai-mcp.yaml, then apply:
    databases:
      - name: mcp-oauth-db
        engine: PG
        production: true
        cluster_name: opa-2025-nyc-db
        db_name: mcp_oauth
        db_user: mcp_oauth
    and under services[energy-mcp].envs:
    - key: OILPRICEAPI_MCP_OAUTH_DATABASE_URL
      scope: RUN_TIME
      value: ${mcp-oauth-db.DATABASE_URL}
    - key: OILPRICEAPI_MCP_OAUTH_DATABASE_CA_CERT
      scope: RUN_TIME
      value: ${mcp-oauth-db.CA_CERT}
    - key: OILPRICEAPI_MCP_OAUTH_ENCRYPTION_KEY
      scope: RUN_TIME
      type: SECRET
      value: <openssl rand -base64 32>
    Note: the live spec also carries MCP_ALLOWED_ORIGINS and the mcp.oilpriceapi.com domain, which deployment/openai-mcp.yaml does not; reconcile the file with doctl apps spec get before applying so they are not dropped.
  3. Store the encryption key in the password manager. Losing it disconnects every user once; it is not recoverable.
  4. Pool usage: 4 connections max per instance (instance_count: 1).

Verification

  • npm test: 562/563 pass. The one failure is energyPlugin > cancels an upstream that stalls after opening its response body, which also fails on origin/main in the same run and is tracked in [P2][test] energyPlugin 'cancels an upstream that stalls' times out intermittently (2/5 local runs on origin/main) #172; it passes in isolation.
  • CI run 38779095061 on the head commit: all jobs green; oauthStore.test.ts ran 16/16 including the Postgres test; container smoke logged {"event":"oauth_store_ready","durable":true}.
  • OAuth suites with MCP_OAUTH_TEST_DATABASE_URL=postgres://localhost/mcp_oauth_test: 60/60 pass, including the real-Postgres restart test (also runs in CI against the service Postgres).
  • New tests: refresh succeeds on a new provider over the same store; DCR client (with secret) survives restart; expired access/refresh rejected after restart; revoked tokens rejected after restart; changed key rejects everything; concurrent refresh is single-use; at-rest rows contain only hashes and v1. ciphertext; payload moved to another row fails; missing key / missing DB in production fail closed, including startRemote().
  • tsc --noEmit, npm run build, prettier on changed files, check:capability-ledger, smoke:package, smoke:public-claims, smoke:product-facts, verify:release-metadata, npm audit (0 vulnerabilities): pass.
  • Dockerfile.remote image, local: production with no key exits 1 with OILPRICEAPI_MCP_OAUTH_ENCRYPTION_KEY is required; key without DB exits 1; key + local Postgres boots, /health 200, oauth_store_ready {"durable":true}, /register row stored as v1. ciphertext and present after docker restart.
  • Unverified: TLS to the DO managed cluster with the bound CA_CERT, and the full Claude reconnect-free refresh across a real deploy. Both need the prerequisites above; check doctl apps logs 4a89cb34-72a3-4daf-a893-0628699ded26 --tail=50 for oauth_store_ready with durable: true after the first deploy.

🤖 Generated with Claude Code

…crypted (#155)

Clients, consent forms, codes, link replays and access/refresh tokens moved
from in-process Maps to an OAuthStore. Rows are keyed by SHA-256 of the
bearer value; every payload (PKCE params, redirect, client secret, customer
API key) is sealed with AES-256-GCM bound to its row. Revocation finds
sibling grants by an HMAC subject, not the key. Codes and refresh tokens are
consumed with an atomic take, so rotation stays single-use across instances.

PostgresOAuthStore is used when OILPRICEAPI_MCP_OAUTH_DATABASE_URL is set;
MemoryOAuthStore remains for tests and local dev. Production refuses to
start without both the database URL and OILPRICEAPI_MCP_OAUTH_ENCRYPTION_KEY.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Oct 15, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: defaults
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b68d63a3-633b-4675-a0ad-39a0fec26597
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@karlwaldman karlwaldman added the risk:elevated Elevated: auto-merges after an adversarial review APPROVE label Oct 15, 2026
…155)

The remote image now refuses to start in production without a durable
store and encryption key, so the container smoke supplies both from a
throwaway service Postgres and asserts the durable store came up.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@karlwaldman karlwaldman added the hold Do not auto-merge; Karl merges label Oct 15, 2026
@karlwaldman

Copy link
Copy Markdown
Member Author

Karl explicitly approved merging and deploying all remaining OPA PRs in this session. Reviewed encrypted storage with kind/id AAD, independently derived sealing/subject keys, hashed bearer lookup, parameterized queries, atomic code/refresh consumption, revocation, TLS CA verification, expiry and fail-closed startup. The refreshed head includes the reviewed futures/marine changes; all CI gates are green. Local full suite against isolated PostgreSQL 15 passed (612 tests before the latest tool merges); current CI repeats the PostgreSQL restart/concurrency tests and remote container smoke.

Production preparation: dedicated mcp_oauth database and non-superuser role created; database ownership and schema rights verified through the trusted backend console, with PUBLIC connection/create revoked on the new database only. A 32-byte encryption key is saved and read-back verified in the Mac login Keychain (service OilPriceAPI MCP production OAuth, account OILPRICEAPI_MCP_OAUTH_ENCRYPTION_KEY). No secret is committed or logged.

Deployment remains gated on the reconciled MCP app spec: bind this database, inject its CA and the backed-up SECRET, preserve the existing domain/origins, and pin the reviewed immutable image. The app is not configured to auto-deploy on merge. Local network access to the managed cluster was unavailable; actual bound-CA and reconnect-free refresh verification will run against the isolated app after deployment. Existing memory-only sessions must reconnect once on this first migration.

@karlwaldman karlwaldman added risk:reviewed Elevated, adversarial review approved this head SHA and removed hold Do not auto-merge; Karl merges risk:elevated Elevated: auto-merges after an adversarial review APPROVE labels Oct 17, 2026
@karlwaldman
karlwaldman merged commit 16c80d8 into main Oct 17, 2026
9 checks passed
@karlwaldman
karlwaldman deleted the fix/155-oauth-store branch October 17, 2026 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk:reviewed Elevated, adversarial review approved this head SHA

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[P2][bug] Remote MCP OAuth state is in memory: every deploy/restart disconnects all Claude and ChatGPT users

1 participant