fix(web): stop advertising MCP OAuth without entitlement#985
Merged
brendan-kellam merged 5 commits intosourcebot-dev:mainfrom Mar 6, 2026
Merged
fix(web): stop advertising MCP OAuth without entitlement#985brendan-kellam merged 5 commits intosourcebot-dev:mainfrom
brendan-kellam merged 5 commits intosourcebot-dev:mainfrom
Conversation
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds entitlement checks and a shared error message constant so OAuth discovery and token/revoke/register endpoints only advertise or return OAuth metadata when the instance has the OAuth entitlement; also removes an older protected-resource route, adds a generic 404 catch-all API route, updates proxy/rewrite for Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Server as Web Server (API routes)
participant Ent as Entitlement Checker (hasEntitlement)
Client->>Server: GET /.well-known/oauth-authorization-server
Server->>Ent: hasEntitlement('oauth')?
Ent-->>Server: yes / no
alt entitlement present
Server-->>Client: 200 JSON (OAuth metadata)
else entitlement missing
Server-->>Client: 404 JSON (error_description = OAUTH_NOT_SUPPORTED_ERROR_MESSAGE)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/web/src/app/api/(server)/mcp/route.ts (1)
21-21: Add regression coverage for the entitlement-gated auth hint.This branch changes public MCP auth discovery behavior, but the PR doesn't add coverage for it. Please add a small test matrix that asserts the
WWW-Authenticateheader is present on 401s whenoauthis entitled and absent when it is not.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/api/`(server)/mcp/route.ts at line 21, Add regression tests that cover the entitlement-gated auth hint: create a small test matrix that sends a request triggering the same 401 branch (StatusCodes.UNAUTHORIZED) in the MCP route handler and toggles hasEntitlement('oauth') true/false; when hasEntitlement('oauth') is true assert the response includes a WWW-Authenticate header with the expected auth hint, and when false assert the WWW-Authenticate header is absent. Stub/mocking: mock hasEntitlement to return true/false for each case and invoke the route handler (the exported handler in route.ts) or perform an integration request against it; ensure the assertions check response.status === 401 and presence/absence of response.headers['www-authenticate'] accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CHANGELOG.md`:
- Around line 10-11: Prefix the existing changelog entry "Avoid advertising
OAuth support on MCP endpoints if that entitlement is not actually configured.
[`#985`](https://github.com/sourcebot-dev/sourcebot/pull/985)" with "[EE]" to mark
it as enterprise-only; update the line to begin with "### Fixed" followed by "-
[EE] Avoid advertising OAuth support on MCP endpoints if that entitlement is not
actually configured. [`#985`](...)" (keep the PR link unchanged) so the entry
follows the enterprise-only convention.
---
Nitpick comments:
In `@packages/web/src/app/api/`(server)/mcp/route.ts:
- Line 21: Add regression tests that cover the entitlement-gated auth hint:
create a small test matrix that sends a request triggering the same 401 branch
(StatusCodes.UNAUTHORIZED) in the MCP route handler and toggles
hasEntitlement('oauth') true/false; when hasEntitlement('oauth') is true assert
the response includes a WWW-Authenticate header with the expected auth hint, and
when false assert the WWW-Authenticate header is absent. Stub/mocking: mock
hasEntitlement to return true/false for each case and invoke the route handler
(the exported handler in route.ts) or perform an integration request against it;
ensure the assertions check response.status === 401 and presence/absence of
response.headers['www-authenticate'] accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f59e785f-7324-4e4b-af62-386c8d9cb598
📒 Files selected for processing (5)
CHANGELOG.mdpackages/web/src/app/api/(server)/ee/.well-known/oauth-authorization-server/route.tspackages/web/src/app/api/(server)/ee/.well-known/oauth-protected-resource/[...path]/route.tspackages/web/src/app/api/(server)/ee/.well-known/oauth-protected-resource/route.tspackages/web/src/app/api/(server)/mcp/route.ts
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
docs/docs/features/mcp-server.mdx (1)
31-33: Use a relative path for internal documentation links.Line 32 uses a full URL while line 280 uses a relative path for the same link. For consistency and maintainability, prefer relative paths for internal documentation links.
📝 Suggested fix
<Note> - If [anonymous access](https://docs.sourcebot.dev/docs/configuration/auth/access-settings#anonymous-access) is enabled on your Sourcebot instance, no OAuth token or API key is required. You can connect directly to the MCP endpoint without any authorization. + If [anonymous access](/docs/configuration/auth/access-settings#anonymous-access) is enabled on your Sourcebot instance, no OAuth token or API key is required. You can connect directly to the MCP endpoint without any authorization. </Note>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/docs/features/mcp-server.mdx` around lines 31 - 33, Replace the absolute internal docs URL in the Note block with the relative path used elsewhere: change the full link "https://docs.sourcebot.dev/docs/configuration/auth/access-settings#anonymous-access" to the relative route "/docs/configuration/auth/access-settings#anonymous-access" so the Note in docs/features/mcp-server.mdx uses the same relative internal link format as line ~280.packages/web/src/app/api/(server)/[...slug]/route.ts (1)
1-13: PreferserviceErrorResponseover open-coding the JSON error payload.This duplicates the standard API error serialization logic that already exists in
@/lib/serviceError. Reusing the helper keeps the 404 shape/status handling consistent with the rest of the API and avoids another spot to update ifServiceErrorevolves.♻️ Suggested cleanup
-import { ErrorCode } from "@/lib/errorCodes" -import { ServiceError } from "@/lib/serviceError" +import { ErrorCode } from "@/lib/errorCodes" +import { serviceErrorResponse } from "@/lib/serviceError" import { StatusCodes } from "http-status-codes" -import { NextResponse } from "next/server" -// Repeat for other methods or use a handler: -const handler = () => NextResponse.json( - { +const handler = () => + serviceErrorResponse({ statusCode: StatusCodes.NOT_FOUND, errorCode: ErrorCode.NOT_FOUND, message: "This API endpoint does not exist", - } satisfies ServiceError, - { status: 404 } - ) + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/web/src/app/api/`(server)/[...slug]/route.ts around lines 1 - 13, Replace the open-coded JSON error response in the fallback handler with the shared helper: call serviceErrorResponse(...) instead of NextResponse.json(...) so the 404 payload uses the canonical ServiceError shape; locate the handler function in this file and replace its body to invoke serviceErrorResponse with StatusCodes.NOT_FOUND, ErrorCode.NOT_FOUND and the same message so serialization and status are consistent with the rest of the API.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/docs/features/mcp-server.mdx`:
- Around line 31-33: Replace the absolute internal docs URL in the Note block
with the relative path used elsewhere: change the full link
"https://docs.sourcebot.dev/docs/configuration/auth/access-settings#anonymous-access"
to the relative route
"/docs/configuration/auth/access-settings#anonymous-access" so the Note in
docs/features/mcp-server.mdx uses the same relative internal link format as line
~280.
In `@packages/web/src/app/api/`(server)/[...slug]/route.ts:
- Around line 1-13: Replace the open-coded JSON error response in the fallback
handler with the shared helper: call serviceErrorResponse(...) instead of
NextResponse.json(...) so the 404 payload uses the canonical ServiceError shape;
locate the handler function in this file and replace its body to invoke
serviceErrorResponse with StatusCodes.NOT_FOUND, ErrorCode.NOT_FOUND and the
same message so serialization and status are consistent with the rest of the
API.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54b877d9-0ef0-4006-9b3c-3e12825da95d
📒 Files selected for processing (12)
CHANGELOG.mddocs/docs/features/mcp-server.mdxpackages/web/next.config.mjspackages/web/src/app/api/(server)/[...slug]/route.tspackages/web/src/app/api/(server)/ee/.well-known/oauth-authorization-server/route.tspackages/web/src/app/api/(server)/ee/.well-known/oauth-protected-resource/[...path]/route.tspackages/web/src/app/api/(server)/ee/.well-known/oauth-protected-resource/route.tspackages/web/src/app/api/(server)/ee/oauth/register/route.tspackages/web/src/app/api/(server)/ee/oauth/revoke/route.tspackages/web/src/app/api/(server)/ee/oauth/token/route.tspackages/web/src/ee/features/oauth/constants.tspackages/web/src/proxy.ts
💤 Files with no reviewable changes (1)
- packages/web/src/app/api/(server)/ee/.well-known/oauth-protected-resource/route.ts
✅ Files skipped from review due to trivial changes (1)
- packages/web/src/ee/features/oauth/constants.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- CHANGELOG.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fix MCP OAuth discovery so instances without the
oauthentitlement do not advertise OAuth support to clients.WWW-AuthenticateOAuth hint behindhasEntitlement('oauth')404from the OAuth well-known metadata endpoints when OAuth is not licensedProblem
Non-licensed instances were still exposing OAuth discovery metadata and OAuth-specific authentication hints to MCP clients. That let clients infer OAuth support even though the OAuth flow was not actually available on that plan.
Solution
Gate the OAuth advertisement paths instead of only gating the token flow.
/api/mcponly includes the OAuthWWW-Authenticatehint when theoauthentitlement is active/.well-known/oauth-protected-resourcereturns404without the entitlement/.well-known/oauth-authorization-serverreturns404without the entitlement/registerto/api/ee/oauth/registerto handle non-compliant mcp clientsTesting
yarn workspace @sourcebot/web test✅ (258tests passed)Checklist
🤖 Agent Plan
Expand full agent plan
WWW-Authenticatehint and OAuth well-known metadata routes onhasEntitlement('oauth').@sourcebot/webtest suite to validate the change before opening the PR.Claude Code:
Codex:
Cursor:
Summary by CodeRabbit
Bug Fixes
Documentation
Chore