Skip to content

Commit b2941c4

Browse files
msukkariclaude
andauthored
fix(web): prevent XSS via OAuth redirect URI scheme injection (#1136)
* 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) <noreply@anthropic.com> * docs: add CHANGELOG entry for OAuth XSS fix Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent beff3b7 commit b2941c4

7 files changed

Lines changed: 167 additions & 7 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1313
- 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)
1414
- 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)
1515
- Fixed CodeQL missing-workflow-permissions alert by adding explicit empty permissions to `deploy-railway.yml`. [#1132](https://github.com/sourcebot-dev/sourcebot/pull/1132)
16+
- [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)
1617

1718
## [4.16.11] - 2026-04-17
1819

packages/web/src/app/api/(server)/ee/oauth/register/route.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { __unsafePrisma } from '@/prisma';
44
import { hasEntitlement } from '@sourcebot/shared';
55
import { NextRequest } from 'next/server';
66
import { z } from 'zod';
7-
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE } from '@/ee/features/oauth/constants';
7+
import { OAUTH_NOT_SUPPORTED_ERROR_MESSAGE, UNPERMITTED_SCHEMES } from '@/ee/features/oauth/constants';
88

99
// RFC 7591: OAuth 2.0 Dynamic Client Registration
1010
// @see: https://datatracker.ietf.org/doc/html/rfc7591
@@ -39,6 +39,14 @@ export const POST = apiHandler(async (request: NextRequest) => {
3939
);
4040
}
4141

42+
// Reject dangerous URI schemes (e.g. javascript:, data:) at registration time
43+
if (redirect_uris.some((uri) => UNPERMITTED_SCHEMES.test(uri))) {
44+
return Response.json(
45+
{ error: 'invalid_redirect_uri', error_description: 'Redirect URI uses a scheme that is not permitted.' },
46+
{ status: 400 }
47+
);
48+
}
49+
4250
const client = await __unsafePrisma.oAuthClient.create({
4351
data: {
4452
name: client_name,

packages/web/src/app/oauth/authorize/components/consentScreen.tsx

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use client';
22

33
import { approveAuthorization, denyAuthorization } from '@/ee/features/oauth/actions';
4+
import { isPermittedRedirectUrl } from '@/ee/features/oauth/constants';
45
import { LoadingButton } from '@/components/ui/loading-button';
56
import { isServiceError } from '@/lib/utils';
67
import { ClientIcon } from './clientIcon';
@@ -44,6 +45,12 @@ export function ConsentScreen({
4445
setPending('approve');
4546
const result = await approveAuthorization({ clientId, redirectUri, codeChallenge, resource, state });
4647
if (!isServiceError(result)) {
48+
if (!isPermittedRedirectUrl(result)) {
49+
toast({ description: `❌ Redirect URL is not permitted.` });
50+
setPending(null);
51+
return;
52+
}
53+
4754
toast({
4855
description: `✅ Authorization approved successfully. Redirecting...`,
4956
});
@@ -64,6 +71,12 @@ export function ConsentScreen({
6471
setPending(null);
6572
return;
6673
}
74+
75+
if (!isPermittedRedirectUrl(result)) {
76+
toast({ description: `❌ Redirect URL is not permitted.` });
77+
setPending(null);
78+
return;
79+
}
6780
window.location.href = result;
6881
};
6982

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,33 @@
11
'use client';
22

3-
import { useEffect } from 'react';
3+
import { useEffect, useState } from 'react';
4+
import { UNPERMITTED_SCHEMES } from '@/ee/features/oauth/constants';
45

56
// Handles the final redirect after OAuth authorization. For http/https redirect URIs,
67
// a server-side redirect is sufficient. For custom protocol URIs (e.g. cursor://, claude://),
78
// browsers won't follow HTTP redirects, so we use window.location.href instead.
89
export default function OAuthCompletePage() {
10+
const [error, setError] = useState<string | null>(null);
11+
912
useEffect(() => {
10-
const url = new URLSearchParams(window.location.search).get('url');
11-
if (url) {
12-
window.location.href = decodeURIComponent(url);
13+
const raw = new URLSearchParams(window.location.search).get('url');
14+
if (!raw) {
15+
setError('Missing redirect URL. You may close this window.');
16+
return;
17+
}
18+
const decoded = decodeURIComponent(raw);
19+
if (UNPERMITTED_SCHEMES.test(decoded)) {
20+
setError('Redirect URL is not permitted. You may close this window.');
21+
return;
1322
}
23+
window.location.href = decoded;
1424
}, []);
1525

1626
return (
1727
<div className="min-h-screen flex items-center justify-center bg-background">
18-
<p className="text-sm text-muted-foreground">Redirecting, you may close this window...</p>
28+
<p className="text-sm text-muted-foreground">
29+
{error ?? 'Redirecting, you may close this window...'}
30+
</p>
1931
</div>
2032
);
2133
}

packages/web/src/ee/features/oauth/actions.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,18 @@
33
import { sew } from "@/middleware/sew";
44
import { generateAndStoreAuthCode } from '@/ee/features/oauth/server';
55
import { withAuth } from '@/middleware/withAuth';
6+
import { UNPERMITTED_SCHEMES } from '@/ee/features/oauth/constants';
67

78
/**
89
* Resolves the final URL to navigate to after an authorization decision.
910
* Non-web redirect URIs (e.g. custom schemes like vscode://) are wrapped in
1011
* /oauth/complete so the browser can handle the handoff.
1112
*/
1213
function resolveCallbackUrl(callbackUrl: URL): string {
14+
if (UNPERMITTED_SCHEMES.test(callbackUrl.protocol)) {
15+
throw new Error('Unpermitted redirect URI scheme');
16+
}
17+
1318
const isWebUrl = callbackUrl.protocol === 'http:' || callbackUrl.protocol === 'https:';
1419
return isWebUrl
1520
? callbackUrl.toString()
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
import { expect, test, describe } from 'vitest';
2+
import { UNPERMITTED_SCHEMES, isPermittedRedirectUrl } from './constants';
3+
4+
describe('UNPERMITTED_SCHEMES', () => {
5+
// Dangerous schemes that must be blocked
6+
test.each([
7+
'javascript:',
8+
'JavaScript:',
9+
'JAVASCRIPT:',
10+
'data:',
11+
'Data:',
12+
'DATA:',
13+
'vbscript:',
14+
'VBScript:',
15+
'VBSCRIPT:',
16+
])('blocks %s', (scheme) => {
17+
expect(UNPERMITTED_SCHEMES.test(scheme)).toBe(true);
18+
});
19+
20+
// Full URL strings (used in /oauth/complete page)
21+
test.each([
22+
'javascript:alert(1)',
23+
'data:text/html,<script>alert(1)</script>',
24+
'vbscript:MsgBox("xss")',
25+
])('blocks full URL string: %s', (url) => {
26+
expect(UNPERMITTED_SCHEMES.test(url)).toBe(true);
27+
});
28+
29+
// Permitted schemes that must not be blocked
30+
test.each([
31+
'http:',
32+
'https:',
33+
'vscode:',
34+
'cursor:',
35+
'claude:',
36+
'vscode://callback',
37+
'cursor://callback?code=abc',
38+
'http://localhost:8080/callback',
39+
'https://example.com/callback',
40+
])('permits %s', (url) => {
41+
expect(UNPERMITTED_SCHEMES.test(url)).toBe(false);
42+
});
43+
});
44+
45+
describe('isPermittedRedirectUrl', () => {
46+
// --- Permitted URLs ---
47+
48+
test('permits https URLs', () => {
49+
expect(isPermittedRedirectUrl('https://example.com/callback')).toBe(true);
50+
});
51+
52+
test('permits http URLs', () => {
53+
expect(isPermittedRedirectUrl('http://localhost:8080/callback')).toBe(true);
54+
});
55+
56+
test('permits /oauth/complete relative paths', () => {
57+
expect(isPermittedRedirectUrl('/oauth/complete?url=vscode%3A%2F%2Fcallback')).toBe(true);
58+
});
59+
60+
test('permits /oauth/complete with encoded custom scheme', () => {
61+
expect(isPermittedRedirectUrl('/oauth/complete?url=cursor%3A%2F%2Fcallback%3Fcode%3Dabc')).toBe(true);
62+
});
63+
64+
test('permits https URL with query params and fragment', () => {
65+
expect(isPermittedRedirectUrl('https://example.com/callback?code=abc&state=xyz#fragment')).toBe(true);
66+
});
67+
68+
// --- Blocked URLs ---
69+
70+
test('blocks javascript: URLs', () => {
71+
expect(isPermittedRedirectUrl('javascript:alert(1)')).toBe(false);
72+
});
73+
74+
test('blocks data: URLs', () => {
75+
expect(isPermittedRedirectUrl('data:text/html,<script>alert(1)</script>')).toBe(false);
76+
});
77+
78+
test('blocks vbscript: URLs', () => {
79+
expect(isPermittedRedirectUrl('vbscript:MsgBox("xss")')).toBe(false);
80+
});
81+
82+
test('blocks javascript: with mixed case', () => {
83+
expect(isPermittedRedirectUrl('JavaScript:alert(1)')).toBe(false);
84+
});
85+
86+
test('blocks custom scheme URLs not wrapped in /oauth/complete', () => {
87+
expect(isPermittedRedirectUrl('vscode://callback?code=abc')).toBe(false);
88+
});
89+
90+
test('blocks malformed URLs', () => {
91+
expect(isPermittedRedirectUrl('not a url at all')).toBe(false);
92+
});
93+
94+
test('blocks empty string', () => {
95+
expect(isPermittedRedirectUrl('')).toBe(false);
96+
});
97+
98+
test('blocks relative paths that do not start with /oauth/complete', () => {
99+
expect(isPermittedRedirectUrl('/some/other/path')).toBe(false);
100+
});
101+
});
Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,22 @@
11

2-
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.';
2+
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.';
3+
4+
export const UNPERMITTED_SCHEMES = /^(javascript|data|vbscript):/i;
5+
6+
/**
7+
* Returns true if the URL is permitted for use as a redirect target.
8+
* Allows relative paths starting with /oauth/complete and http(s) URLs.
9+
* Returns false for dangerous schemes like javascript:, data:, vbscript:.
10+
*/
11+
export function isPermittedRedirectUrl(url: string): boolean {
12+
if (url.startsWith('/oauth/complete')) {
13+
return true;
14+
}
15+
16+
try {
17+
const parsed = new URL(url);
18+
return parsed.protocol === 'http:' || parsed.protocol === 'https:';
19+
} catch {
20+
return false;
21+
}
22+
}

0 commit comments

Comments
 (0)