added auth to mcp server, uses OAuth bridge to privy.io

[Jahnik]

Summary by CodeRabbit

Release Notes

• New Features

  • Implemented OAuth 2.0 authorization server with client registration, token exchange, and token refresh capabilities
  • Added OAuth consent page UI for user authentication workflows
  • Integrated Privy authentication service for token validation
  • Added token validation, revocation, and management endpoints

• Documentation

  • Updated README with OAuth and Privy authentication setup information

• Chores

  • Added Privy authentication dependencies

[coderabbitai]

Walkthrough

This pull request introduces OAuth 2.0 authorization server capabilities with PKCE support, Privy authentication integration, and a locally-bundled OAuth consent UI. Changes span configuration, backend authentication middleware, OAuth token/flow management, frontend consent UI components, and build configuration updates.

Changes

Cohort / File(s)	Summary
OAuth Configuration & Data .data/oauth-clients.json	Static OAuth client configurations with clientId, clientName, redirectUris, scopes, and metadata.
Environment Configuration env.example	Added OAuth-related env vars (issuer URL, token TTLs, client ID/URI allowlists), Privy API keys, and development auth override flag.
Backend Dependencies package.json, widgets/package.json	Added @privy-io/js-sdk-core and @privy-io/node SDKs to project and widget dependencies.
Privy Integration src/privy.ts	New module for PrivyClient initialization, token verification, and environment validation.
OAuth Server Implementation src/oauth.ts	Complete OAuth 2.0 authorization server with PKCE support, authorization code flow, token management, client registration, and metadata endpoints.
Authentication Middleware src/auth.ts	Express middleware for Bearer token validation, token extraction, and OAuth session context attachment.
Server Integration src/server.ts	New OAuth endpoints (/oauth/authorize, /oauth/token, /oauth/revoke, etc.), consent page rendering, asset handling refactoring, and authentication middleware integration.
OAuth Consent UI widgets/src/oauth-consent/App.tsx, widgets/src/oauth-consent/index.tsx, widgets/src/oauth-consent/index.html, widgets/src/oauth-consent/oauth-consent.css	React-based consent UI with email/OTP verification, Privy client initialization, and token exchange integration.
Build Configuration widgets/vite.config.ts	Updated Vite multi-entry build to route widget and OAuth consent assets into separate output directories with cache-busting hashes.
Documentation README.md, spec.md	Updated README with OAuth/Privy sections and bundled consent UI notes; added comprehensive implementation spec for OAuth consent page rebuild.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant UI as OAuth Consent UI
    participant Server as Express Server
    participant Privy as Privy Auth
    participant OAuth as OAuth Server

    User->>UI: Opens /oauth/authorize
    Server->>UI: Renders consent page + context injection
    UI->>UI: Parses AuthorizationPageContext from DOM
    UI->>Privy: Initialize Privy client
    User->>UI: Enters email address
    UI->>Privy: Send OTP via email
    Privy-->>User: Email with verification code
    User->>UI: Enters verification code
    UI->>Privy: Verify code & authenticate
    Privy-->>UI: Access token (privyToken)
    UI->>Server: POST /oauth/authorize/complete<br/>(state, privyToken)
    Server->>Privy: Validate token
    Privy-->>Server: Verified claims
    Server->>OAuth: Complete authorization<br/>(issue code/token)
    OAuth-->>Server: Authorization code
    Server-->>UI: Redirect URI + code
    UI->>Server: POST /oauth/token (code + PKCE verifier)
    Server->>OAuth: Exchange code for tokens
    OAuth-->>Server: Access + refresh tokens
    Server-->>UI: Access token response
    UI->>Server: Call protected /mcp endpoint<br/>(Bearer token)
    Server->>Server: Validate token via authenticatePrivy
    Server-->>UI: Protected resource

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Areas requiring extra attention:

• src/oauth.ts: Dense OAuth 2.0 logic with PKCE validation, token lifecycle management, in-memory storage patterns, and error handling edge cases across multiple flows
• src/server.ts: Substantial endpoint additions, asset rendering/injection patterns, and integration points between OAuth and Privy
• widgets/src/oauth-consent/App.tsx: Complex React state management with multi-stage form, error handling, and token exchange logic
• Token validation flow: Verify token TTL/expiration logic, scope validation, and cascading cleanup across authorization codes, access tokens, and refresh tokens
• PKCE implementation: Code challenge generation/verification logic in authorization and token exchange flows

Possibly related PRs

• Dev #2: Related through shared modifications to Vite build configuration (widgets/vite.config.ts) and build output structure.

Poem

🐰 Hops of auth through Privy's door,
OAuth flows we've not seen before,
PKCE checks and tokens bright,
Consent pages shining in the light,
Bundled joy, no CDN required,
Authorization dreams, finally wired! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 13.33% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately captures the main objective: adding authentication to the MCP server through an OAuth bridge to Privy.io, which aligns with the comprehensive changes across auth middleware, OAuth implementation, Privy integration, and consent UI.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 10

🧹 Nitpick comments (6)

src/server.ts (2)

189-189: Verify trust proxy configuration.

Setting trust proxy to true makes Express trust all proxies. This is necessary for getRequestOrigin to correctly read x-forwarded-proto and x-forwarded-host headers (lines 241-244). However, in production, you should configure this to trust only specific proxies to prevent header spoofing attacks.

Consider using a more restrictive configuration in production:

-app.set("trust proxy", true);
+app.set("trust proxy", process.env.TRUST_PROXY || "loopback");

Then set TRUST_PROXY appropriately in your deployment environment (e.g., "loopback, linklocal, uniquelocal" or a specific IP/CIDR range).

---

356-372: Missing CSRF protection on authorization completion endpoint.

The /oauth/authorize/complete endpoint accepts state and tokens via POST body without additional CSRF protection. While the state parameter provides some binding to the authorization request, an attacker with access to a valid state value could potentially forge completion requests.

Consider adding one of the following protections:

1. Verify the request origin matches the expected domain
2. Add a short-lived CSRF token to the authorization page context
3. Ensure the consent UI submits via form POST from the same domain (already appears to be the case based on the architecture)

Since the consent UI is served from the same origin and the state is cryptographically random, the current implementation may be acceptable, but document this security assumption.

src/oauth.ts (4)

201-227: Expired record cleanup lacks concurrency protection.

The cleanupExpiredRecords function iterates over maps and deletes entries while iterating. While JavaScript Map iteration is safe for deletions during iteration, there's no protection against concurrent access if multiple requests trigger cleanup simultaneously.

Consider one of the following:

1. Add a simple flag to prevent concurrent cleanup runs
2. Use a scheduled background task instead of inline cleanup
3. Accept that concurrent cleanup is safe for the in-memory implementation

For production, document that this implementation is not thread-safe and is intended for single-process deployment only.

---

397-406: Default client_id logic may cause confusion.

Lines 397-406 automatically default to the first static client if only one exists and no client_id is provided. This behavior is logged as a warning but might surprise developers.

Consider making this behavior explicit via an environment variable like OAUTH_ALLOW_DEFAULT_CLIENT_ID=true to make the implicit behavior opt-in.

---

418-431: PKCE validation warnings are appropriate but permissive.

The code logs warnings when PKCE parameters are missing (lines 418-420, 422-424) but proceeds without PKCE. This maintains backward compatibility but weakens security.

Consider adding a strict mode via environment variable:

const requirePKCE = process.env.OAUTH_REQUIRE_PKCE === "true";

if (!hasChallenge) {
  if (requirePKCE) {
    throw createOAuthError("invalid_request", "code_challenge is required when PKCE enforcement is enabled.");
  }
  console.warn("[oauth] Missing code_challenge; proceeding without PKCE validation.");
}

---

550-554: Consider using stronger token generation.

While randomBytes(32).toString("base64url") provides 256 bits of entropy (lines 550, 553), which is cryptographically strong, tokens are currently stateful (stored in maps). For a stateless token approach, consider JWT tokens signed with a secret key.

The current approach is secure for the in-memory implementation. If scaling to multiple servers, consider:

1. Using Redis or a database for shared token storage
2. Switching to signed JWT tokens for stateless validation

This is documented as a single-process limitation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a9cf74f and c5f81e9.

⛔ Files ignored due to path filters (2)

• bun.lock is excluded by !**/*.lock
• widgets/bun.lock is excluded by !**/*.lock

📒 Files selected for processing (15)

• .data/oauth-clients.json (1 hunks)
• README.md (6 hunks)
• env.example (1 hunks)
• package.json (1 hunks)
• spec.md (1 hunks)
• src/auth.ts (1 hunks)
• src/oauth.ts (1 hunks)
• src/privy.ts (1 hunks)
• src/server.ts (2 hunks)
• widgets/package.json (1 hunks)
• widgets/src/oauth-consent/App.tsx (1 hunks)
• widgets/src/oauth-consent/index.html (1 hunks)
• widgets/src/oauth-consent/index.tsx (1 hunks)
• widgets/src/oauth-consent/oauth-consent.css (1 hunks)
• widgets/vite.config.ts (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (5)

widgets/src/oauth-consent/App.tsx (1)

src/oauth.ts (1)

• AuthorizationPageContext (67-79)

widgets/src/oauth-consent/index.tsx (1)

widgets/src/oauth-consent/App.tsx (1)

• App (41-238)

src/oauth.ts (1)

src/privy.ts (2)

• VerifyAuthTokenResponse (57-57)
• verifyPrivyToken (27-55)

src/auth.ts (2)

src/privy.ts (1)

• VerifyAuthTokenResponse (57-57)

src/oauth.ts (1)

• validateAccessToken (764-798)

src/server.ts (2)

src/oauth.ts (12)

• AuthorizationPageContext (67-79)
• serializeAuthorizationContext (844-846)
• authorizationServerMetadata (815-830)
• protectedResourceMetadata (832-838)
• listSupportedScopes (840-842)
• registerClient (306-358)
• prepareAuthorization (360-487)
• completeAuthorization (489-546)
• TokenEndpointResponse (81-87)
• exchangeCodeForTokens (594-657)
• refreshAccessToken (659-721)
• revokeToken (723-762)

src/auth.ts (2)

• authenticatePrivy (31-75)
• AuthenticatedRequest (13-23)

🔇 Additional comments (29)

env.example (1)

9-40: Well-documented environment variables for OAuth/Privy integration.

The environment variables are clearly documented with inline comments explaining their purpose. The naming is consistent and follows best practices. The DANGEROUSLY_OMIT_AUTH flag is appropriately named to warn developers about the security implications.

README.md (2)

40-40: Good feature documentation.

The addition of the bundled consent UI feature to the features list clearly communicates a key capability of the system.

---

174-188: Comprehensive security documentation.

The new Privy Authentication and OAuth Facade sections provide clear guidance on authentication flows, token handling, and security configuration. The warning about DANGEROUSLY_OMIT_AUTH is appropriately emphasized.

widgets/src/oauth-consent/index.tsx (1)

1-12: Clean React 18 entry point implementation.

The implementation correctly uses React 18's createRoot API and includes appropriate error handling for missing DOM containers. The code is minimal and focused on its single responsibility.

widgets/src/oauth-consent/index.html (1)

1-13: Appropriate HTML scaffold for OAuth consent UI.

The HTML structure is minimal and appropriate for a Vite-based React application. The __OAUTH_CONTEXT__ placeholder indicates server-side context injection, which is a standard pattern for embedding runtime configuration.

package.json (1)

22-23: Privy SDK dependencies added correctly.

The Privy SDK dependencies are properly placed alphabetically and use appropriate version ranges. Note that there is a version mismatch with widgets/package.json (0.56.1 vs 0.57.0) that should be reconciled.

widgets/vite.config.ts (2)

3-3: Import added for multi-entry build support.

The resolve import from path is correctly added to support the multi-entry build configuration.

---

15-16: Multi-entry build configuration.

The build now supports both widget and OAuth consent UI entries, with appropriate output directory separation. This aligns with the PR's goal of bundling the OAuth consent UI locally.

widgets/src/oauth-consent/oauth-consent.css (1)

1-143: Polished consent page styling.

Layout, focus states, and dark-mode handling look solid here.

src/privy.ts (1)

1-55: Privy client bootstrap looks sound.

Env validation, client singleton, and defensive claim logging all check out.

spec.md (1)

1-73: Spec is comprehensive.

The plan covers build outputs, server wiring, and risk mitigations clearly.

src/server.ts (8)

37-45: LGTM! Asset fallback handling is robust.

The fallback logic correctly handles multiple asset paths for both build and development scenarios. The error message clearly identifies which paths were checked.

---

199-204: LGTM! OAuth asset caching is properly disabled.

The explicit Cache-Control: no-store, no-cache, must-revalidate headers prevent browsers from caching OAuth assets, which is essential for ensuring users see the latest consent UI.

---

324-354: Handle empty query parameters more defensively.

Line 325 returns a 204 response when no query parameters are present, which is a clean way to handle direct navigation. However, ensure this doesn't conflict with OAuth clients that might send empty-string parameters.

The implementation looks correct, but verify that OAuth clients (e.g., ChatGPT connector) don't send requests with empty query parameter values that would bypass this check.

---

374-405: LGTM! Token endpoint properly handles multiple grant types.

The implementation correctly branches on grant_type and calls the appropriate OAuth functions. The Cache-Control headers are properly set to prevent token caching.

---

415-434: Userinfo endpoint implementation follows OpenID Connect standards.

The endpoint correctly requires authentication and returns standard OIDC claims. The Cache-Control headers prevent caching of user information.

---

440-440: Authentication requirement on MCP endpoint is correctly enforced.

The /mcp endpoint now requires authentication via authenticatePrivy middleware, which validates the OAuth access token. This properly secures the MCP protocol endpoint.

---

50-59: Regex pattern is safe; HTML attributes use double quotes consistently.

The codebase's HTML templates (widgets/src/echo/index.html and widgets/src/oauth-consent/index.html) use exclusively double-quoted attributes (e.g., src="./index.tsx", type="module"). No single-quoted or unquoted attributes are present. The regex pattern /(src|href)="\/([^"]+)"/g correctly targets double-quoted attributes, which is the format used in the actual source HTML. The bundler-generated HTML processed by rewriteAssetUrls will maintain this well-formed structure.

---

1-1: dotenv/config is correctly positioned as first import.

Verified that src/server.ts is the sole entry point (compiled to dist/server.js and executed via start script), with import "dotenv/config" at line 1 before any other imports. Module-level environment variable reads in dependent modules (src/auth.ts, src/privy.ts) occur after the dotenv side-effect loads the .env file synchronously. No alternative entry points exist that could bypass this initialization.

src/oauth.ts (10)

193-195: LGTM! Token generation uses cryptographically secure randomness.

The generateId function correctly uses randomBytes from the crypto module, which provides cryptographically secure random values. The prefix helps with token identification and debugging.

---

197-199: LGTM! PKCE code challenge implementation is correct.

The SHA-256 hashing with base64url encoding matches the PKCE specification (RFC 7636).

---

267-304: Redirect URI validation correctly enforces security constraints.

The validation properly:

• Requires absolute URIs
• Restricts to HTTP/HTTPS protocols
• Allows HTTP only for localhost (lines 285-294)
• Requires exact match against registered URIs (line 296)

---

306-358: Client registration validates redirect URIs during registration.

The function correctly validates all redirect URIs before creating the client and persists the registered client immediately.

---

489-546: Authorization completion includes fallback token mechanism.

The completeAuthorization function accepts both a primary token and a fallback token (lines 491-492), attempting verification with fallback if primary fails (lines 506-518). This is a good resilience pattern.

Ensure that the fallback mechanism is documented and that the consent UI understands when to provide a fallback token.

---

548-592: Token issuance correctly handles offline_access scope.

The issueTokens function only issues refresh tokens when offline_access is included in the scope (line 551), which follows OAuth best practices. Token expiration is correctly calculated.

---

594-657: Code exchange correctly implements PKCE verification.

The exchangeCodeForTokens function properly validates:

• All required parameters
• Client ID match
• Redirect URI match
• Code expiration
• PKCE code_verifier (lines 638-646)

The authorization code is deleted after use (line 648), preventing replay attacks.

---

659-721: Refresh token implementation follows OAuth security best practices.

The refreshAccessToken function:

• Validates the refresh token exists and hasn't expired
• Enforces that requested scopes don't exceed original scopes (lines 700-707)
• Rotates both access and refresh tokens (lines 709-712)

Token rotation is an excellent security practice that limits the window of exposure if tokens are compromised.

---

723-762: Token revocation performs cascading cleanup.

The revokeToken function correctly revokes both the specified token and its associated counterpart (access↔refresh). The logic handles both with and without token_type_hint.

---

764-798: Access token validation includes automatic cleanup.

The validateAccessToken function not only validates tokens but also removes expired tokens (lines 775-782) and their associated refresh tokens. This helps maintain the integrity of the token stores.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Excessive duplicate client entries and hardcoded OAuth configuration.

This file contains 46 OAuth client entries, where 45 of them are nearly identical (all named "ChatGPT" with the same redirect URIs and scopes). This appears to be accumulated test/development data.

Concerns:

• Production readiness: Having dozens of duplicate client configurations suggests this is test data that shouldn't be committed.
• Security: Hardcoding OAuth client configurations in version control makes it difficult to rotate clients or manage them dynamically.
• Maintainability: Static JSON files are difficult to manage for OAuth clients, which typically need dynamic registration and rotation.

Consider:

1. Reducing this to only the essential production client(s) needed (likely just "chatgpt-connector").
2. Moving OAuth client management to a database or external configuration service.
3. Implementing dynamic client registration rather than hardcoded configurations.
4. Adding a .gitignore entry for this file if it will contain environment-specific clients.

Example minimal configuration:

[
  {
    "clientId": "chatgpt-connector",
    "clientName": "ChatGPT Connector",
    "redirectUris": [
      "https://chat.openai.com/connector_platform_oauth_redirect",
      "https://chatgpt.com/connector_platform_oauth_redirect"
    ],
    "scopes": [
      "openid",
      "profile",
      "email",
      "offline_access"
    ],
    "clientIdIssuedAt": 1762366322
  }
]

🤖 Prompt for AI Agents

In .data/oauth-clients.json spanning lines 1 to 633, the file contains 46 OAuth
client entries where 45 are nearly identical duplicate "ChatGPT" clients with
identical configurations, representing accumulated test data that should not be
committed to version control. Remove all duplicate ChatGPT client entries (from
the second entry onwards) and keep only the "chatgpt-connector" entry which is
the essential production client needed. This reduces the file to a minimal
configuration containing only the legitimate production OAuth client.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Use the correct WWW-Authenticate error code.

For resource servers, RFC 6750 requires error="invalid_token" when the bearer token is unknown or expired. Returning invalid_grant here can confuse standards-compliant clients. Please normalize the error code to invalid_token.

-    res.setHeader(
-      "WWW-Authenticate",
-      `Bearer realm="index-mcp", error="${
-        validation.error === "expired" ? "invalid_token" : "invalid_grant"
-      }", error_description="${validation.message}"`
-    );
+    res.setHeader(
+      "WWW-Authenticate",
+      `Bearer realm="index-mcp", error="invalid_token", error_description="${validation.message}"`
+    );

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents

In src/auth.ts around lines 57 to 63, the WWW-Authenticate header currently sets
error to "invalid_grant" for non-expired failures; RFC 6750 requires resource
servers to use error="invalid_token" for unknown or expired bearer tokens.
Change the header construction so the error value is always "invalid_token"
(remove the conditional), keep the rest of the header the same (realm and
error_description), and return the 401 response as before.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Configuration lacks validation and has hardcoded fallbacks.

The configuration reads from environment variables with fallback defaults, but several issues exist:

1. Hardcoded client ID and redirect URIs (lines 127-138): The static client "chatgpt-connector" with specific redirect URIs is hardcoded. This reduces flexibility and makes testing harder.

2. No validation of critical config: TTL values are parsed as numbers without validation (lines 107-115). Invalid values could cause runtime errors or security issues.

3. Issuer URL normalization inconsistent: Some places use issuer.replace(/\/$/, "") while the issuer itself may or may not have a trailing slash.

Add configuration validation at startup:

// After line 125
if (accessTokenTtlSeconds <= 0 || refreshTokenTtlSeconds <= 0 || authorizationCodeTtlSeconds <= 0) {
  throw new Error("OAuth TTL values must be positive integers");
}

if (!issuer || !resourceIndicator) {
  throw new Error("OAUTH_ISSUER_URL and OAUTH_RESOURCE_INDICATOR must be configured");
}

// Normalize issuer consistently
const issuer = (process.env.OAUTH_ISSUER_URL || process.env.MCP_SERVER_URL || "http://localhost:3002").replace(/\/$/, "");

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Client bootstrap creates pre-registered clients without versioning.

The bootstrapClients function reads persisted clients and ensures static clients exist. However, if the static client configuration changes (e.g., redirect URIs are updated), existing persisted clients won't be updated.

Consider adding a version check or update mechanism:

 for (const id of staticClientIds) {
   if (!clients.has(id)) {
     clients.set(id, {
       clientId: id,
       clientName: "ChatGPT Connector",
       redirectUris: [...staticRedirectUris],
       scopes: [...defaultScopes],
       clientIdIssuedAt: Math.floor(Date.now() / 1000),
     });
+  } else {
+    // Update redirect URIs for static clients to match current config
+    const existing = clients.get(id)!;
+    existing.redirectUris = [...staticRedirectUris];
+    existing.scopes = [...defaultScopes];
   }
 }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	function bootstrapClients() {
	if (!existsSync(dataDir)) {
	mkdirSync(dataDir, { recursive: true });
	}
	if (existsSync(clientsFile)) {
	try {
	const parsed: OAuthClient[] = JSON.parse(
	readFileSync(clientsFile, "utf-8")
	);
	for (const client of parsed) {
	clients.set(client.clientId, client);
	}
	} catch (error) {
	console.warn("Failed to read persisted OAuth clients:", error);
	}
	}
	for (const id of staticClientIds) {
	if (!clients.has(id)) {
	clients.set(id, {
	clientId: id,
	clientName: "ChatGPT Connector",
	redirectUris: [...staticRedirectUris],
	scopes: [...defaultScopes],
	clientIdIssuedAt: Math.floor(Date.now() / 1000),
	});
	}
	}
	persistClients();
	}
	function bootstrapClients() {
	if (!existsSync(dataDir)) {
	mkdirSync(dataDir, { recursive: true });
	}
	if (existsSync(clientsFile)) {
	try {
	const parsed: OAuthClient[] = JSON.parse(
	readFileSync(clientsFile, "utf-8")
	);
	for (const client of parsed) {
	clients.set(client.clientId, client);
	}
	} catch (error) {
	console.warn("Failed to read persisted OAuth clients:", error);
	}
	}
	for (const id of staticClientIds) {
	if (!clients.has(id)) {
	clients.set(id, {
	clientId: id,
	clientName: "ChatGPT Connector",
	redirectUris: [...staticRedirectUris],
	scopes: [...defaultScopes],
	clientIdIssuedAt: Math.floor(Date.now() / 1000),
	});
	} else {
	// Update redirect URIs for static clients to match current config
	const existing = clients.get(id)!;
	existing.redirectUris = [...staticRedirectUris];
	existing.scopes = [...defaultScopes];
	}
	}
	persistClients();
	}

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

XSS prevention in context serialization.

The serializeAuthorizationContext function escapes < characters to prevent XSS when embedding JSON in HTML. However, this only handles one case.

For complete XSS prevention when embedding in HTML, escape additional characters:

 export function serializeAuthorizationContext(context: AuthorizationPageContext) {
-  return JSON.stringify(context).replace(/</g, "\u003c");
+  return JSON.stringify(context)
+    .replace(/</g, "\u003c")
+    .replace(/>/g, "\u003e")
+    .replace(/&/g, "\u0026");
 }

This prevents various HTML and script injection patterns when the JSON is embedded in script tags or HTML attributes.

🤖 Prompt for AI Agents

In src/oauth.ts around lines 844 to 846, the serializeAuthorizationContext only
escapes "<" which is insufficient for safe HTML embedding; update the function
to escape additional characters before returning the stringified context — at
minimum replace &, >, <, /, single quote (') and the Unicode line/separator
characters U+2028 and U+2029 with their corresponding \\uXXXX escapes (e.g.
\\u0026, \\u003e, \\u003c, \\u002f, \\u0027, \\u2028, \\u2029) or use a small
helper that maps and replaces these characters to their unicode-escaped forms so
the JSON can be safely embedded in script tags or attributes.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Origin detection relies on proxy headers without validation.

The getRequestOrigin function trusts x-forwarded-proto and x-forwarded-host headers when present. Since trust proxy is set to true (line 189), these headers are accepted from any source, creating a potential security risk if attackers can inject these headers.

Consider validating the proxy headers or restricting which proxies are trusted. If an attacker controls these headers, they could manipulate the origin used in OAuth redirects, authorization endpoints, and MCP manifests. As mentioned in the earlier comment on line 189, configure trust proxy to trust only specific proxies in production.

🤖 Prompt for AI Agents

In src/server.ts around lines 240 to 249, the getRequestOrigin function blindly
trusts the x-forwarded-proto and x-forwarded-host headers without validating
they come from authorized sources, creating a security risk for OAuth redirects
and authorization endpoints. Add validation to check the request's origin IP
address against a whitelist of trusted proxies before accepting these headers.
If the request does not come from a trusted proxy IP, fall back to using the
request's own protocol and host properties instead of the forwarded headers.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Version mismatch with root package.json.

The widgets package specifies @privy-io/js-sdk-core version ^0.56.1, while the root package.json (line 22) specifies ^0.57.0. This version mismatch may cause type incompatibilities or runtime issues if the packages evolve differently.

Apply this diff to align versions:

-    "@privy-io/js-sdk-core": "^0.56.1",
+    "@privy-io/js-sdk-core": "^0.57.0",

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	"@privy-io/js-sdk-core": "^0.56.1",
	"@privy-io/js-sdk-core": "^0.57.0",

🤖 Prompt for AI Agents

In widgets/package.json around line 13, the @privy-io/js-sdk-core dependency is
set to "^0.56.1" which mismatches the root package.json specifying "^0.57.0";
update the widgets package dependency to the same version range as the root
(change to "^0.57.0") so both manifests align and avoid type/runtime
incompatibilities, then run a fresh install (npm/yarn) and verify lockfile
consistency.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Trim and persist the email before verifying.

handleSendCode trims the input before sending it to Privy but leaves the original (possibly whitespace-padded) value in state. Later, loginWithCode reuses the untrimmed email, so a trailing space causes Privy to reject the verification. Please sanitize the email you store and reuse the trimmed value on verification.

-    const trimmed = email.trim();
-    if (!trimmed) {
+    const trimmedEmail = email.trim();
+    if (!trimmedEmail) {
       setStatusMessage('Enter a valid email address.', 'error');
       return;
     }
 
     try {
       setPending(true);
       setStatusMessage('Sending verification code…');
-      await privyClient.auth.email.sendCode(trimmed);
+      await privyClient.auth.email.sendCode(trimmedEmail);
+      setEmail(trimmedEmail);
@@
-      const session = await privyClient.auth.email.loginWithCode(email, trimmedCode);
+      const sanitizedEmail = email.trim();
+      const session = await privyClient.auth.email.loginWithCode(sanitizedEmail, trimmedCode);

Also applies to: 110-114

🤖 Prompt for AI Agents

In widgets/src/oauth-consent/App.tsx around lines 71 to 83 (and similarly at
lines 110 to 114), the handler trims the email for the API call but doesn't
persist the trimmed value in component state, so later handlers use the original
untrimmed email and cause Privy to reject it; update the flow to store the
sanitized trimmed email in state before calling privyClient (e.g., call
setEmail(trimmed) or otherwise assign the trimmed value to the state variable
used by loginWithCode) and ensure any other send/verify paths also use the
trimmed/stored value.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Remove sensitive token logging.

console.log statements here dump session objects and token presence flags to the browser console. That exposes bearer tokens to anyone with console access and violates basic token-handling hygiene. Please drop these logs (or guard them behind an explicit debug flag that never runs in production).

-      console.log('Privy session', session);
-
       const appAccessToken = (session as any)?.token;
       const privyAccessToken = (session as any)?.privy_access_token;
-      console.log('Selected Privy token source', {
-        hasAppAccessToken: Boolean(appAccessToken),
-        hasPrivyAccessToken: Boolean(privyAccessToken),
-        hasAccessTokenField: Boolean(session?.accessToken?.token)
-      });

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	console.log('Privy session', session);
	const appAccessToken = (session as any)?.token;
	const privyAccessToken = (session as any)?.privy_access_token;
	console.log('Selected Privy token source', {
	hasAppAccessToken: Boolean(appAccessToken),
	hasPrivyAccessToken: Boolean(privyAccessToken),
	hasAccessTokenField: Boolean(session?.accessToken?.token)
	});
	const appAccessToken = (session as any)?.token;
	const privyAccessToken = (session as any)?.privy_access_token;

🤖 Prompt for AI Agents

In widgets/src/oauth-consent/App.tsx around lines 114 to 122, remove the
console.log statements that print the full session object and token presence
flags (they expose bearer tokens to the browser console); either delete those
logs entirely or replace them with a guarded debug-only logger that checks a
dedicated debug flag (e.g., process.env.REACT_APP_DEBUG === 'true') before
logging, and ensure the default production build never enables this flag.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Inconsistent path detection logic for assets.

The file naming functions use inconsistent logic to determine which folder assets should be placed in:

• entryFileNames and chunkFileNames use name.startsWith('oauth/') (lines 21, 26)
• assetFileNames uses name.includes('oauth') (line 31)

This inconsistency could cause assets to be misplaced. For example, a file like widgets/oauth-helper.css would be placed in the oauth folder due to the includes check, even though it belongs to the widgets entry.

Apply this diff to use consistent logic:

         assetFileNames: (assetInfo) => {
           const name = assetInfo.name ?? '';
-          const folder = name.includes('oauth') ? 'oauth' : 'widgets';
+          const folder = name.startsWith('oauth/') || name.includes('src/oauth-consent/') ? 'oauth' : 'widgets';
           return `${folder}/[name]-[hash:8][extname]`;
         }

Alternatively, if asset placement should be based on the entry point that generated them, consider using Rollup's asset source information to determine the correct folder more reliably.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	entryFileNames: (chunk) => {
	const name = chunk.name ?? '';
	const folder = name.startsWith('oauth/') ? 'oauth' : 'widgets';
	return `${folder}/[name]-[hash:8].js`;
	},
	chunkFileNames: (chunk) => {
	const name = chunk.name ?? '';
	const folder = name.startsWith('oauth/') ? 'oauth' : 'widgets';
	return `${folder}/[name]-[hash:8].js`;
	},
	assetFileNames: (assetInfo) => {
	const name = assetInfo.name ?? '';
	const folder = name.includes('oauth') ? 'oauth' : 'widgets';
	return `${folder}/[name]-[hash:8][extname]`;
	}
	entryFileNames: (chunk) => {
	const name = chunk.name ?? '';
	const folder = name.startsWith('oauth/') ? 'oauth' : 'widgets';
	return `${folder}/[name]-[hash:8].js`;
	},
	chunkFileNames: (chunk) => {
	const name = chunk.name ?? '';
	const folder = name.startsWith('oauth/') ? 'oauth' : 'widgets';
	return `${folder}/[name]-[hash:8].js`;
	},
	assetFileNames: (assetInfo) => {
	const name = assetInfo.name ?? '';
	const folder = name.startsWith('oauth/') || name.includes('src/oauth-consent/') ? 'oauth' : 'widgets';
	return `${folder}/[name]-[hash:8][extname]`;
	}