From f298e0e1ed2d628fc37784a6a8b5e53e68672efa Mon Sep 17 00:00:00 2001 From: msukkari Date: Sat, 18 Apr 2026 17:28:21 -0700 Subject: [PATCH 1/2] fix(web): prevent XSS via OAuth redirect URI scheme injection Block javascript:, data:, and vbscript: URI schemes across the OAuth redirect flow to resolve CodeQL js/xss-through-exception alert. Adds defense-in-depth validation at four layers: client registration, server-side callback resolution, client-side consent screen, and the /oauth/complete handoff page. Fixes SOU-928 Co-Authored-By: Claude Opus 4.6 (1M context) --- .../api/(server)/ee/oauth/register/route.ts | 10 +- .../authorize/components/consentScreen.tsx | 13 +++ packages/web/src/app/oauth/complete/page.tsx | 22 +++- packages/web/src/ee/features/oauth/actions.ts | 5 + .../src/ee/features/oauth/constants.test.ts | 101 ++++++++++++++++++ .../web/src/ee/features/oauth/constants.ts | 22 +++- 6 files changed, 166 insertions(+), 7 deletions(-) create mode 100644 packages/web/src/ee/features/oauth/constants.test.ts diff --git a/packages/web/src/app/api/(server)/ee/oauth/register/route.ts b/packages/web/src/app/api/(server)/ee/oauth/register/route.ts index a8d223f9e..e315b3225 100644 --- a/packages/web/src/app/api/(server)/ee/oauth/register/route.ts +++ b/packages/web/src/app/api/(server)/ee/oauth/register/route.ts @@ -4,7 +4,7 @@ import { __unsafePrisma } from '@/prisma'; import { hasEntitlement } from '@sourcebot/shared'; import { NextRequest } from 'next/server'; import { z } from 'zod'; -import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants'; +import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE, UNPERMITTED_SCHEMES } from '@/ee/features/oauth/constants'; // RFC 7591: OAuth 2.0 Dynamic Client Registration // @see: https://datatracker.ietf.org/doc/html/rfc7591 @@ -39,6 +39,14 @@ export const POST = apiHandler(async (request: NextRequest) => { ); } + // Reject dangerous URI schemes (e.g. javascript:, data:) at registration time + if (redirect_uris.some((uri) => UNPERMITTED_SCHEMES.test(uri))) { + return Response.json( + { error: 'invalid_redirect_uri', error_description: 'Redirect URI uses a scheme that is not permitted.' }, + { status: 400 } + ); + } + const client = await __unsafePrisma.oAuthClient.create({ data: { name: client_name, diff --git a/packages/web/src/app/oauth/authorize/components/consentScreen.tsx b/packages/web/src/app/oauth/authorize/components/consentScreen.tsx index 8d22d126c..94eb27ea8 100644 --- a/packages/web/src/app/oauth/authorize/components/consentScreen.tsx +++ b/packages/web/src/app/oauth/authorize/components/consentScreen.tsx @@ -1,6 +1,7 @@ 'use client'; import { approveAuthorization, denyAuthorization } from '@/ee/features/oauth/actions'; +import { isPermittedRedirectUrl } from '@/ee/features/oauth/constants'; import { LoadingButton } from '@/components/ui/loading-button'; import { isServiceError } from '@/lib/utils'; import { ClientIcon } from './clientIcon'; @@ -44,6 +45,12 @@ export function ConsentScreen({ setPending('approve'); const result = await approveAuthorization({ clientId, redirectUri, codeChallenge, resource, state }); if (!isServiceError(result)) { + if (!isPermittedRedirectUrl(result)) { + toast({ description: `❌ Redirect URL is not permitted.` }); + setPending(null); + return; + } + toast({ description: `✅ Authorization approved successfully. Redirecting...`, }); @@ -64,6 +71,12 @@ export function ConsentScreen({ setPending(null); return; } + + if (!isPermittedRedirectUrl(result)) { + toast({ description: `❌ Redirect URL is not permitted.` }); + setPending(null); + return; + } window.location.href = result; }; diff --git a/packages/web/src/app/oauth/complete/page.tsx b/packages/web/src/app/oauth/complete/page.tsx index c422beba0..df31db65a 100644 --- a/packages/web/src/app/oauth/complete/page.tsx +++ b/packages/web/src/app/oauth/complete/page.tsx @@ -1,21 +1,33 @@ 'use client'; -import { useEffect } from 'react'; +import { useEffect, useState } from 'react'; +import { UNPERMITTED_SCHEMES } from '@/ee/features/oauth/constants'; // Handles the final redirect after OAuth authorization. For http/https redirect URIs, // a server-side redirect is sufficient. For custom protocol URIs (e.g. cursor://, claude://), // browsers won't follow HTTP redirects, so we use window.location.href instead. export default function OAuthCompletePage() { + const [error, setError] = useState(null); + useEffect(() => { - const url = new URLSearchParams(window.location.search).get('url'); - if (url) { - window.location.href = decodeURIComponent(url); + const raw = new URLSearchParams(window.location.search).get('url'); + if (!raw) { + setError('Missing redirect URL. You may close this window.'); + return; + } + const decoded = decodeURIComponent(raw); + if (UNPERMITTED_SCHEMES.test(decoded)) { + setError('Redirect URL is not permitted. You may close this window.'); + return; } + window.location.href = decoded; }, []); return (
-

Redirecting, you may close this window...

+

+ {error ?? 'Redirecting, you may close this window...'} +

); } diff --git a/packages/web/src/ee/features/oauth/actions.ts b/packages/web/src/ee/features/oauth/actions.ts index 351b4b84c..ee3ee9a42 100644 --- a/packages/web/src/ee/features/oauth/actions.ts +++ b/packages/web/src/ee/features/oauth/actions.ts @@ -3,6 +3,7 @@ import { sew } from "@/middleware/sew"; import { generateAndStoreAuthCode } from '@/ee/features/oauth/server'; import { withAuth } from '@/middleware/withAuth'; +import { UNPERMITTED_SCHEMES } from '@/ee/features/oauth/constants'; /** * Resolves the final URL to navigate to after an authorization decision. @@ -10,6 +11,10 @@ import { withAuth } from '@/middleware/withAuth'; * /oauth/complete so the browser can handle the handoff. */ function resolveCallbackUrl(callbackUrl: URL): string { + if (UNPERMITTED_SCHEMES.test(callbackUrl.protocol)) { + throw new Error('Unpermitted redirect URI scheme'); + } + const isWebUrl = callbackUrl.protocol === 'http:' || callbackUrl.protocol === 'https:'; return isWebUrl ? callbackUrl.toString() diff --git a/packages/web/src/ee/features/oauth/constants.test.ts b/packages/web/src/ee/features/oauth/constants.test.ts new file mode 100644 index 000000000..4918651b9 --- /dev/null +++ b/packages/web/src/ee/features/oauth/constants.test.ts @@ -0,0 +1,101 @@ +import { expect, test, describe } from 'vitest'; +import { UNPERMITTED_SCHEMES, isPermittedRedirectUrl } from './constants'; + +describe('UNPERMITTED_SCHEMES', () => { + // Dangerous schemes that must be blocked + test.each([ + 'javascript:', + 'JavaScript:', + 'JAVASCRIPT:', + 'data:', + 'Data:', + 'DATA:', + 'vbscript:', + 'VBScript:', + 'VBSCRIPT:', + ])('blocks %s', (scheme) => { + expect(UNPERMITTED_SCHEMES.test(scheme)).toBe(true); + }); + + // Full URL strings (used in /oauth/complete page) + test.each([ + 'javascript:alert(1)', + 'data:text/html,', + 'vbscript:MsgBox("xss")', + ])('blocks full URL string: %s', (url) => { + expect(UNPERMITTED_SCHEMES.test(url)).toBe(true); + }); + + // Permitted schemes that must not be blocked + test.each([ + 'http:', + 'https:', + 'vscode:', + 'cursor:', + 'claude:', + 'vscode://callback', + 'cursor://callback?code=abc', + 'http://localhost:8080/callback', + 'https://example.com/callback', + ])('permits %s', (url) => { + expect(UNPERMITTED_SCHEMES.test(url)).toBe(false); + }); +}); + +describe('isPermittedRedirectUrl', () => { + // --- Permitted URLs --- + + test('permits https URLs', () => { + expect(isPermittedRedirectUrl('https://example.com/callback')).toBe(true); + }); + + test('permits http URLs', () => { + expect(isPermittedRedirectUrl('http://localhost:8080/callback')).toBe(true); + }); + + test('permits /oauth/complete relative paths', () => { + expect(isPermittedRedirectUrl('/oauth/complete?url=vscode%3A%2F%2Fcallback')).toBe(true); + }); + + test('permits /oauth/complete with encoded custom scheme', () => { + expect(isPermittedRedirectUrl('/oauth/complete?url=cursor%3A%2F%2Fcallback%3Fcode%3Dabc')).toBe(true); + }); + + test('permits https URL with query params and fragment', () => { + expect(isPermittedRedirectUrl('https://example.com/callback?code=abc&state=xyz#fragment')).toBe(true); + }); + + // --- Blocked URLs --- + + test('blocks javascript: URLs', () => { + expect(isPermittedRedirectUrl('javascript:alert(1)')).toBe(false); + }); + + test('blocks data: URLs', () => { + expect(isPermittedRedirectUrl('data:text/html,')).toBe(false); + }); + + test('blocks vbscript: URLs', () => { + expect(isPermittedRedirectUrl('vbscript:MsgBox("xss")')).toBe(false); + }); + + test('blocks javascript: with mixed case', () => { + expect(isPermittedRedirectUrl('JavaScript:alert(1)')).toBe(false); + }); + + test('blocks custom scheme URLs not wrapped in /oauth/complete', () => { + expect(isPermittedRedirectUrl('vscode://callback?code=abc')).toBe(false); + }); + + test('blocks malformed URLs', () => { + expect(isPermittedRedirectUrl('not a url at all')).toBe(false); + }); + + test('blocks empty string', () => { + expect(isPermittedRedirectUrl('')).toBe(false); + }); + + test('blocks relative paths that do not start with /oauth/complete', () => { + expect(isPermittedRedirectUrl('/some/other/path')).toBe(false); + }); +}); diff --git a/packages/web/src/ee/features/oauth/constants.ts b/packages/web/src/ee/features/oauth/constants.ts index cb79b7077..3315d3adf 100644 --- a/packages/web/src/ee/features/oauth/constants.ts +++ b/packages/web/src/ee/features/oauth/constants.ts @@ -1,2 +1,22 @@ -export const OAUTH_NOT_SUPPORTED_ERROR_MESSAGE = 'OAuth is not supported on this instance. Please authenticate using a Sourcebot API key instead. See https://docs.sourcebot.dev/docs/features/mcp-server for more information.'; \ No newline at end of file +export const OAUTH_NOT_SUPPORTED_ERROR_MESSAGE = 'OAuth is not supported on this instance. Please authenticate using a Sourcebot API key instead. See https://docs.sourcebot.dev/docs/features/mcp-server for more information.'; + +export const UNPERMITTED_SCHEMES = /^(javascript|data|vbscript):/i; + +/** + * Returns true if the URL is permitted for use as a redirect target. + * Allows relative paths starting with /oauth/complete and http(s) URLs. + * Returns false for dangerous schemes like javascript:, data:, vbscript:. + */ +export function isPermittedRedirectUrl(url: string): boolean { + if (url.startsWith('/oauth/complete')) { + return true; + } + + try { + const parsed = new URL(url); + return parsed.protocol === 'http:' || parsed.protocol === 'https:'; + } catch { + return false; + } +} \ No newline at end of file From cb00385b4d332e95159daa83bb7cc867819384b9 Mon Sep 17 00:00:00 2001 From: msukkari Date: Sat, 18 Apr 2026 17:29:13 -0700 Subject: [PATCH 2/2] docs: add CHANGELOG entry for OAuth XSS fix Co-Authored-By: Claude Opus 4.6 (1M context) --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4339ccd71..a7f2003e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed path injection vulnerability (CodeQL js/path-injection) in review agent log writing by validating paths stay within the expected log directory. [#1134](https://github.com/sourcebot-dev/sourcebot/pull/1134) - Fixed missing workflow permissions in `docs-broken-links.yml` by adding explicit `permissions: {}` to follow least privilege principle. [#1131](https://github.com/sourcebot-dev/sourcebot/pull/1131) - Fixed CodeQL missing-workflow-permissions alert by adding explicit empty permissions to `deploy-railway.yml`. [#1132](https://github.com/sourcebot-dev/sourcebot/pull/1132) +- [EE] Fixed XSS vulnerability (CodeQL js/xss-through-exception) in OAuth redirect flow by blocking dangerous URI schemes (`javascript:`, `data:`, `vbscript:`) at registration, authorization, and redirect layers. [#1136](https://github.com/sourcebot-dev/sourcebot/pull/1136) ## [4.16.11] - 2026-04-17