fix: set initiatedBy for app credential auth to resolve user-scoped MCP credentials#2912
fix: set initiatedBy for app credential auth to resolve user-scoped MCP credentials#2912amikofalvy merged 2 commits intomainfrom
Conversation
…CP credentials The app credential auth paths (both authenticated and anonymous web_client) were setting endUserId but not initiatedBy in the execution context metadata. getUserIdFromContext checks initiatedBy first, so user-scoped MCP tools (like Linear OAuth) couldn't resolve credentials — the userId was undefined, causing the user-scoped credential lookup to be skipped entirely. This is a follow-up to #2899 which fixed the playground auth path but missed the app credential paths. Made-with: Cursor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: 25d1a56 The changes in this PR will be included in the next version bump. This PR includes changesets to release 10 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
TL;DR — App credential auth paths ( Key changes
Summary | 2 files | 2 commits | base:
|
Made-with: Cursor
There was a problem hiding this comment.
Clean, correct fix. Both change sites are well-guarded: the authenticated path (line 765) correctly sets initiatedBy unconditionally since endUserId is guaranteed non-empty from tryAsymmetricJwtVerification, and the anonymous fallback path (line 834) correctly uses a conditional spread since payload.sub may be undefined.
Claude Opus | 𝕏
There was a problem hiding this comment.
PR Review Summary
(0) Total Issues | Risk: Low
Analysis
This PR adds initiatedBy metadata to the two app credential auth paths (web_client authenticated and anonymous) in tryAppCredentialAuth. This is a follow-up to #2899, which added a fallback in getUserIdFromContext() to check endUserId when initiatedBy is missing.
What the fix does:
- Authenticated path (line 765): Sets
initiatedBy: { type: 'user', id: endUserId }unconditionally — safe becauseendUserIdis guaranteed non-empty from the verified JWT'ssubclaim. - Anonymous path (line 834): Sets
initiatedByconditionally via spread — correctly handles the case wherepayload.submay beundefined.
Why it's correct:
- Normalizes auth metadata shape across all strategies (temp-JWT, Slack, bypass, team-agent all set
initiatedBy) - No security implications — the
endUserIdvalues were already being set and trusted; this change simply propagates them into the canonical field - The credential lookup in
AgentMcpManager.getToolSet()is already tenant/project-scoped, so no cross-tenant access is possible
Changeset: Properly formatted and accurately describes the change.
✅ APPROVE
Summary: Clean, minimal fix that normalizes auth metadata across strategies. The implementation is correct, type-safe, and consistent with existing patterns. Ship it! 🚀
Note: Unable to submit formal approval due to GitHub App permissions — this is an approval recommendation.
Reviewers (3)
| Reviewer | Returned | Main Findings | Consider | While You're Here | Inline Comments | Pending Recs | Discarded |
|---|---|---|---|---|---|---|---|
pr-review-standards |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-security-iam |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
pr-review-consistency |
0 | 0 | 0 | 0 | 0 | 0 | 0 |
| Total | 0 | 0 | 0 | 0 | 0 | 0 | 0 |
Ito Test Report ❌10 test cases ran. 3 failed, 7 passed. Overall, the unified run executed 10 test cases with 7 passes and 3 failures, indicating generally solid auth boundary behavior but with meaningful gaps that should block a full pass. The key confirmed defects were: anonymous global web_client sessions can lose tenant/project context and fail chat (High), invalid/expired asymmetric JWTs do not actually fall back to anonymous mode when allowed (Medium), and disallowed-origin web_client requests can still proceed in development/test due to default-context fallback (Medium), while other checks passed including no-endUserId handling, strict token/scope/app-claim rejection paths, prompt-injection non-disclosure, and refresh/rapid-submit stability. ❌ Failed (3)🟠 Disallowed Origin requests bypass rejection in development/test auth fallback
Relevant code:
if (app.type === 'web_client') {
const config = app.config as WebClientConfig;
if (!validateOrigin(origin, config.webClient.allowedDomains)) {
logger.warn(
{ origin, allowedDomains: config.webClient.allowedDomains, appId: app.id },
'App credential auth: origin not allowed'
);
return { authResult: null, failureMessage: 'Origin not allowed for this app' };
}
// Development/test environment handling
if (isDev) {
logger.info({}, 'development environment');
const attempt = await authenticateRequest(reqData);
if (attempt.authResult) {
c.set('executionContext', buildExecutionContext(attempt.authResult, reqData));
} else {
logger.info(
{},
reqData.apiKey
? 'Development/test environment - fallback to default context due to invalid API key'
: 'Development/test environment - no API key provided, using default context'
);
c.set('executionContext', buildExecutionContext(createDevContext(reqData), reqData));
}
}🟠 Allow-anonymous asymmetric auth path still hard-fails invalid tokens
Relevant code:
if (!asymResult.ok) {
const allowAnonymous = config.webClient.auth?.allowAnonymous !== false;
if (!allowAnonymous) {
logger.debug(
{ appId: app.id, reason: asymResult.failureMessage },
'Asymmetric JWT verification failed, anonymous not allowed'
);
throw createApiError({ code: 'unauthorized', message: asymResult.failureMessage });
}
logger.debug(
{ appId: app.id, reason: asymResult.failureMessage },
'Asymmetric JWT verification failed, falling back to anonymous'
);
} catch (err) {
const errorType =
err instanceof errors.JWTExpired
? 'expired'
: err instanceof errors.JWSSignatureVerificationFailed
? 'signature_invalid'
: 'unknown';
logger.debug({ errorType, appId: appIdHeader }, 'Anonymous JWT verification failed');
if (hasAuthConfigured) {
throw createApiError({ code: 'unauthorized', message: 'Invalid or expired token' });
}
|
…CP credentials (#2912) * fix: set initiatedBy for app credential auth to resolve user-scoped MCP credentials The app credential auth paths (both authenticated and anonymous web_client) were setting endUserId but not initiatedBy in the execution context metadata. getUserIdFromContext checks initiatedBy first, so user-scoped MCP tools (like Linear OAuth) couldn't resolve credentials — the userId was undefined, causing the user-scoped credential lookup to be skipped entirely. This is a follow-up to #2899 which fixed the playground auth path but missed the app credential paths. Made-with: Cursor * chore: add changeset Made-with: Cursor
Summary
web_client) setendUserIdbut notinitiatedByin the execution context metadatagetUserIdFromContextchecksinitiatedBy.type === 'user'first — when it's missing,userIdisundefinedisUserScoped && userIdcheck inAgentMcpManager.getToolSet()to evaluate tofalse, skipping the user-scoped credential lookup entirely and resulting inno-cred/ 401 from the MCP serverTest plan
no-credcache key segment is replaced with the actual credential referenceMade with Cursor