Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
…-consumer # Conflicts: # src/app/[transport]/route.ts
Exchange MCP-audience tokens with the issuer's RFC 8693 profile, accept its five-minute exchanged tokens while keeping a one-minute request lease, check exchange lineage, require explicit configuration when enabled, and leave legacy authorize/token/register and auth-context behavior unchanged.
rgarcia
changed the title
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 36f6300. Configure here.
| credentialType: "oauth", | ||
| transportSessionId, | ||
| observeConnection, | ||
| }); |
There was a problem hiding this comment.
Native entitlements cache never hits
Medium Severity
Native requests pass the one-time exchanged token into handleMcpRequestWithIdentity, whose entitlements cacheIdentity includes that token. Each native tools/list or gated call therefore misses the 30-minute snapshot and refetches GET /org/entitlements, and unique keys can churn the bounded map and evict other callers. Auth still rechecks via exchange; the entitlements cache is only for tool exposure and is keyed by credential plus connection.
Additional Locations (1)
Triggered by learned rule: MCP entitlements: snapshot cache, list+gated calls only
Reviewed by Cursor Bugbot for commit 36f6300. Configure here.
masnwilliams
left a comment
masnwilliams
left a comment
There was a problem hiding this comment.
Requesting changes for one reproduced bug: native callers get an identity the rest of the server doesn't recognize. The verification and exchange contract itself looks careful and well tested. Everything outside must fix is optional or can wait until before staging.
tsc is clean and the native, route and auth-login-app suites pass on 36f6300.
must fix
-
Native callers get an inconsistent identity (inline on
route.ts). The route records a native caller asnative:<sub>withuserId: nulland forwards the exchanged token, which changes on every request. Code downstream rebuilds the caller's identity fromauthInfo.extra.userIdandauthInfo.token, so it reaches a different answer:- MCP Apps gate (reproduced): on a legacy-protocol client that declared MCP Apps,
begin_auth_login/open_auth_loginshow up intools/list, but every call returns the gate-denied message. The route saved the marker undernative:…, andauth-login-app.tslooks it up underapikey:<hmac(exchanged token)>. - Entitlements cache (reproduced): the cache key includes
token, so two nativetools/listcalls make two/org/entitlementscalls. An API key makes one. Each miss also adds a fresh 30-minute entry to the shared 1,000-entry cache, which pushes out other callers' entries. - Analytics (same cause, not run):
analyticsSessionKeyfalls back totoken:<token>whenuserIdis null, so every modern native request starts a new session.
Fix: build
authSubjectwithmcpAppsAuthSubject, setuserId: native.subject(the only readers ofextra.userIdare the analytics session key and the auth-login gate), and keep the rotating token out of the entitlements cache key. That last part sits outside this diff, inhandleMcpRequestWithIdentity: - MCP Apps gate (reproduced): on a legacy-protocol client that declared MCP Apps,
@@ -214,6 +214,7 @@ async function handleMcpRequestWithIdentity({
connectionContextCacheIdentity?: string;
+ entitlementsCacheToken?: string;
observeConnection: boolean;
}) {
@@ -248,7 +249,7 @@ async function handleMcpRequestWithIdentity({
connection.context.authContext.organization.id,
transportSessionId ?? "stateless",
- token,
+ entitlementsCacheToken ?? token,
].join("\0"),Then pass entitlementsCacheToken: "native" from the native branch. That's safe because the exchange has already authenticated this request. With these changes the gate passes (user:… on both sides), native makes one entitlements call, and the existing suites still pass.
- Make the fixture catch this (inline on the fixture). Every exchanged token gets
jti: "jti_derived"with timestamps to the second, so two requests in the same second produce byte-identical tokens and the cache looks like it works. Give each one a uniquejti, as the real issuer does, and add route tests for the MCP Apps gate and the entitlements call count. A condensed version of the repro I used is below.
suggestions (not blocking)
leaseNativeResponseis probably redundant. The request already carriesAbortSignal.timeout(deadline), andcreateMcpHandlertears down the transport whenrequest.signalaborts (per-request transport, legacy SSE andsubscriptions/listen). With the wrapper, the extraAbortControllerand its try/catch removed, a nativesubscriptions/listenstream with a 1s lease ended cleanly after about 1s throughPOST. That's −105/+39 lines including a route-level lease test. Keeping a second layer for a security cutoff is defensible, but then add a route-level test so the lease is tested end to end.- Native error mapping duplicates
connectionScopeFailureResponseand has already drifted (inline). A result type likeMcpConnectionContextFailure(rejected401/403 orunavailable) would remove both exception classes, theinstanceofchain and the throw that exists only to reach the route's owncatch. - Log which lineage check failed before it becomes a 503 (inline). As written, a staging failure only says "invalid native exchange".
- Parse config and keys once. Every native request runs
JSON.parse+ zod on the JWKS and callscreateLocalJWKSettwice. Caching the config, or the error, keeps the "misconfigured means 503 for native only" behavior.
docs
- This repo is public, and
docs/native-oauth.mdplus two code comments reference internal API source paths, internal function names and a PR number in a private repo (inline). I'd keep the behavior and configuration tables here and track the activation prerequisites and staging checklist internally. - Prerequisite 2 presents the scoped-operation coverage gap as fixed, but that coverage is being expanded on the API side. It'll go stale quickly, which is another reason to keep the checklist out of this doc.
nits
jwtVerifyacceptstyp: "at+jwt"and normalizes theapplication/prefix and case, so the manualtypcheck atnative-oauth.ts:155can go. Both test files also build theOAUTH_NATIVE_*env separately, with values that have already diverged. Anissuer.env()on the fixture would cover both.
repro: MCP Apps gate + entitlements cache through POST
// Uses an in-memory stand-in for @/lib/redis that keys markers with the real mcpAppsMarkerKey,
// and gives each exchanged token a unique jti, as the deployed issuer does.
const fresh = async () => {
issuer.deriveWith({ jti: crypto.randomUUID() });
return issuer.parent();
};
// legacy flow: initialize (declares io.modelcontextprotocol/ui) -> tools/list -> tools/call
// native: listed=true, call -> "this tool is only available to the secure KERNEL login app…"
// marker written under native:…, gate looked up apikey:…
// api key: listed=true, call passes the gate
// fixed: native passes the gate (user:… on both sides)
// two stateless tools/list calls each
// /org/entitlements calls: native 2, api key 1; fixed: native 1| authSubject: `native:${native.subject}`, | ||
| scopes: native.scopes, | ||
| authInfoExtra: { userId: null, clerkToken: null }, |
There was a problem hiding this comment.
must fix: this identity disagrees with how the rest of the server identifies the caller. auth-login-app.ts rebuilds it as mcpAppsAuthSubject({ token, userId }) from authInfo. With userId: null and a token that rotates every request, that gives apikey:<hmac(exchanged token)>, not native:<sub>, so the MCP Apps marker written at initialize is never found and begin_auth_login / open_auth_login are listed but always denied. I reproduced this through POST. Analytics has the same fallback (token:<token> when userId is null).
| authSubject: `native:${native.subject}`, | |
| scopes: native.scopes, | |
| authInfoExtra: { userId: null, clerkToken: null }, | |
| authSubject: mcpAppsAuthSubject({ | |
| token: native.token, | |
| userId: native.subject, | |
| }), | |
| scopes: native.scopes, | |
| authInfoExtra: { userId: native.subject, clerkToken: null }, |
The entitlements cache key also needs to stop including the rotating token. See the review body for that part.
| aud: [config.apiAudience], | ||
| iat: now, | ||
| exp: now + lifetime, | ||
| jti: "jti_derived", |
There was a problem hiding this comment.
must fix: a constant jti with second-resolution iat/exp makes two exchanges in the same second byte-identical, which hides the per-request token rotation that the real issuer does. The entitlements cache miss only showed up once each token was unique.
| jti: "jti_derived", | |
| jti: crypto.randomUUID(), |
| 403, | ||
| "insufficient_scope", | ||
| "This credential cannot access the requested resource", | ||
| { "WWW-Authenticate": 'Bearer error="insufficient_scope"' }, |
There was a problem hiding this comment.
nit: the existing 403 in connectionScopeFailureResponse sends Bearer realm="OAuth", error="insufficient_scope", but this one omits realm. Reusing that function, or returning a rejected/unavailable result instead of throwing, would keep the 401/403/503 mapping in one place.
| if ( | ||
| derived.sub !== parent.sub || | ||
| derived.grant_id !== parent.grant_id || | ||
| derived.org_id !== parent.org_id || | ||
| (derived.project_id ?? "") !== (parent.project_id ?? "") || | ||
| derived.client_id !== config.clientId || | ||
| derived.act?.sub !== `oauth_client:${config.clientId}` || | ||
| derived.parent_jti !== parent.jti || | ||
| derived.exp > parent.exp || | ||
| derived.scope !== exchanged.scope || | ||
| exchanged.scope.split(" ").some((scope) => !parentScopes.includes(scope)) | ||
| ) | ||
| throw new Error("invalid native exchange"); |
There was a problem hiding this comment.
suggestion: when any of these ten conditions fails, the client gets an opaque retryable 503 and nothing is logged. Naming the checks and logging the ones that failed would make staging failures debuggable:
const lineage = {
sub: derived.sub === parent.sub,
grant_id: derived.grant_id === parent.grant_id,
// ...
};
const failed = Object.entries(lineage).filter(([, ok]) => !ok).map(([k]) => k);
if (failed.length) throw new Error(`invalid native exchange: ${failed.join(",")}`);|
|
||
| // Ends a native response when its authorization lease expires; reconnecting | ||
| // requires a fresh exchange. | ||
| export function leaseNativeResponse( |
There was a problem hiding this comment.
suggestion: I think this is redundant with the AbortSignal.timeout(deadline) already on the request. The MCP handler tears down the transport when request.signal aborts. With this function, the route's extra AbortController and its try/catch removed, a native subscriptions/listen stream with a 1s lease still ended cleanly after about 1s through POST. If you keep it as a second layer, a route-level lease test would cover the actual cutoff rather than the wrapper in isolation.
| # Kernel-issued (native) OAuth consumer | ||
|
|
||
| The MCP server can accept access tokens issued by the Kernel API's native | ||
| issuer (`packages/api/lib/oauthnative` in kernel/kernel). It is off by default. |
There was a problem hiding this comment.
This repo is public. This line, prerequisites 1, 2 and 5, and the comments at the top of native-oauth.ts and the fixture point at internal API source paths, function names and a private PR number. I'd keep the behavior and config tables here and track the activation and staging checklist internally, where it can stay current.
Summary
Adds a default-off consumer for Kernel-issued (native) OAuth access tokens. It matches the native issuer contract now on kernel/kernel main. With
OAUTH_NATIVE_ENABLEDunset, existing Clerk OAuth and API-key callers, discovery and the MCP transport behave as on main, and no new secret is required.typat+jwt, the same selector the Kernel API uses. Clerk session tokens (typ: JWT) andsk_API keys keep their paths. While disabled, a native token gets 401 without any issuer or API call.<issuer>/token(RFC 8693,client_secret_basic) on every request. The exchange re-checks grant and membership status, so revocation applies on the next request.client_idandact.sub,parent_jti, and scopes no wider than the parent's. Only this token reaches the API.invalid_grantgives 401,invalid_scopegives 403, and any other issuer failure or invalid configuration gives a retryable 503. A native token never falls back to Clerk or API-key handling.Changes since the previous revision of this PR:
main.oauth_compatibilitylogging, thekrn_prefix and theserviceauth-context principal. The deployed API emits none of them, and they changed legacy behavior.Activation prerequisites (nothing is enabled here)
docs/native-oauth.mdhas the exact staging values (https://auth.dev.onkernel.com,https://mcp.dev.onkernel.com,https://api.dev.onkernel.com, pinned JWKS, epoch, exchange client). Before any staging opt-in:GET /auth/context) is deployed. Without it, every native request ends in 403.https://mcp.dev.onkernel.com/mcp, but the API accepts only the originhttps://mcp.dev.onkernel.com. This server does not rewrite audiences.NATIVE_OAUTH_DEFAULT_NEW_FLOWS), after this consumer is enabled there.NATIVE_OAUTH_MCP_EXCHANGE_CLIENT_ID. The secret is stored only in the staging MCP environment.Production stays off.
Validation
bunx tsc --noEmitpasses.bun test: 1172 pass, 0 fail. The focused native, transport and legacy OAuth suites pass 116 tests.Service.Exchange. They cover detection, the exchange request contract, the 5-minute token with a 1-minute lease, skewed and short parents, expiry, revocation, scope and outage classification, lineage tampering, configuration validation and stream lease close.AGENTS.mdone onmain.docs/native-oauth.md.Reviewed with sol-loop: 1 rounds with openai-codex/gpt-6.1-sol on extra-high reasoning. Sol verdict: approved.