fix: preserve MCP passthrough account context - #366
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| apiToken: callerToken, | ||
| apiToken: | ||
| credentialType === 'bearer' ? `Bearer ${callerToken}` : callerToken, | ||
| accountId: requestedAccountId, |
There was a problem hiding this comment.
requestedAccountId is assigned for every Token request, even when the request is not marked as an account-scoped bearer credential. As a result, an ordinary API-key client that sends X-Account-ID will forward that caller-controlled value on every Terminal49 API request, changing its tenant context despite the requirement that ordinary Token clients remain unchanged. Only propagate the account ID when credentialType === 'bearer'.
Greptile automatically discovered a related ticket stating that account authorization must not fall back to browser-supplied account mapping, which informed this comment.
| accountId: requestedAccountId, | |
| accountId: credentialType === 'bearer' ? requestedAccountId : undefined, |
Knowledge Base Used: MCP protocol, transports, and authentication
Source Used: Linear — Resolve and verify per-user MCP OAuth for AskT49/Eve
Prompt To Fix With AI
This is a comment left during a code review.
Path: api/mcp.ts
Line: 527
Comment:
**Unmarked Tokens Gain Account Context**
`requestedAccountId` is assigned for every `Token` request, even when the request is not marked as an account-scoped bearer credential. As a result, an ordinary API-key client that sends `X-Account-ID` will forward that caller-controlled value on every Terminal49 API request, changing its tenant context despite the requirement that ordinary `Token` clients remain unchanged. Only propagate the account ID when `credentialType === 'bearer'`.
Greptile automatically discovered a related ticket stating that account authorization must not fall back to browser-supplied account mapping, which informed this comment.
```suggestion
accountId: credentialType === 'bearer' ? requestedAccountId : undefined,
```
**Knowledge Base Used:** [MCP protocol, transports, and authentication](https://app.greptile.com/terminal49/-/custom-context/knowledge-base/terminal49/api/-/docs/mcp-protocol-and-authentication.md)
**Source Used:** Linear — [Resolve and verify per-user MCP OAuth for AskT49/Eve](https://linear.app/terminal49/issue/DEV-12439/resolve-and-verify-per-user-mcp-oauth-for-askt49eve)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 891d04fc00
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| apiToken: callerToken, | ||
| apiToken: | ||
| credentialType === 'bearer' ? `Bearer ${callerToken}` : callerToken, | ||
| accountId: requestedAccountId, |
There was a problem hiding this comment.
When a normal Token API-key caller supplies X-Account-ID without X-T49-Credential-Type, this assignment still accepts and forwards the caller-controlled UUID. Even if the upstream API ignores account context for API keys, createTerminal49McpServer uses this value directly as the PostHog distinctId (packages/mcp/src/server.ts:917-919), so any API-key holder can attribute MCP activity to an arbitrary account and corrupt tenant analytics. Only retain the account ID when the credential type is the account-scoped bearer type.
Useful? React with 👍 / 👎.
1 participant
Summary
X-Account-IDthrough the existing MCPTokenpassthrough pathBearerresolver path and ordinary API-keyTokenclients unchangedVerification
list_containerscall for a signed-in dashboard user without an authorization promptNo production deployment is included. This PR remains draft.
Related: Terminal49/t49#3543, DEV-12439.
Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.Greptile Summary
This PR adds an MCP passthrough mode that preserves a dashboard bearer credential and account context while leaving the WorkOS resolver path intact. It also permits the associated headers during CORS preflight and adds handler tests for successful and malformed passthrough requests.
Tokencredentials as upstream bearer credentials.Confidence Score: 4/5
The PR should not merge until account context forwarding is restricted to explicitly marked bearer passthrough requests.
Ordinary API-key Token requests can now influence the upstream account header without opting into the new credential mode, changing their established behavior and potentially selecting or rejecting a different account context.
Files Needing Attention: api/mcp.ts
Important Files Changed
Sequence Diagram
sequenceDiagram participant D as Dashboard participant M as MCP gateway participant S as MCP server participant A as Terminal49 API D->>M: Token credential + credential type + account ID M->>M: Validate passthrough headers M->>S: Bearer credential and account context S->>A: Authorization: Bearer ... + x-account-id A-->>S: Account-scoped result S-->>D: MCP responsePrompt To Fix All With AI
Reviews (1): Last reviewed commit: "fix: preserve passthrough bearer type" | Re-trigger Greptile
Context used: