feat: add authenticated remote MCP tools#681
Conversation
|
Controller intake: blocked before QA/merge. Current blocker:
Required fix:
|
![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: c38322df21
ℹ️ 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".
| } | ||
|
|
||
| return { response: null, session: apiKeySession }; | ||
| return { response: null, session: oauthSession }; |
There was a problem hiding this comment.
When any non-MCP API receives Authorization: Bearer lin_oauth_at_..., this shared helper now returns an OAuth session just like a browser/API-key session, but those routes do not enforce OAuth scopes or the token workspace. For example, src/app/api/issues/route.ts calls requireApiSession() at the POST entry and then creates an issue without checking scopes, so a third-party app authorized with only read can mutate data outside MCP; keep OAuth bearer handling scoped to /api/mcp or add per-route scope/workspace enforcement before returning it here.
Useful? React with 👍 / 👎.
| candidate.tokenHash === tokenHash && | ||
| candidate.revokedAt === null && | ||
| new Date(candidate.expiresAt).getTime() > now, |
There was a problem hiding this comment.
When a user or workspace admin revokes an authorized application, the existing revoke flows delete authorizedApplicationGrant, but this validator only checks the stored token row's hash, revokedAt, and expiry. In that scenario an already-issued access token continues to authenticate to MCP until expiry even though the UI says the application loses access, so the live revocation check should also confirm the grant still exists or revoke matching tokens when the grant is deleted.
Useful? React with 👍 / 👎.
| if (stateId !== undefined) { | ||
| if (!stateId) throw new Error("stateId must not be empty"); | ||
| updates.stateId = await resolveDefaultStateId(found.teamId, stateId); |
There was a problem hiding this comment.
When exponential_update_issue receives a stateId for a completed or canceled workflow state, this path only writes issue.stateId. The normal issue update route also sets completedAt/canceledAt from workflowState.category, so MCP moves into or out of terminal states leave those timestamps null or stale, which breaks completion/cancellation reporting and filters.
Useful? React with 👍 / 👎.
| .insert(comment) | ||
| .values({ | ||
| body, | ||
| issueId: found.id, | ||
| userId: context.userId, |
There was a problem hiding this comment.
When MCP creates a comment, it inserts the row directly and returns; unlike src/app/api/issues/[id]/comments/route.ts, it does not record issue history, mark the discussion summary stale, or notify assignees/subscribers/mentioned users. For comments created through exponential_create_comment, users will not get notifications and issue activity/summaries can omit the new discussion.
Useful? React with 👍 / 👎.
|
Closing in favor of regeneration on the current architecture. This PR was authored against the pre-monorepo-split layout ( Review also flagged blocking security/correctness issues (varies per PR: SSRF, OAuth CSRF nonce never verified, full-table workspace scans, issue-number races, Re-generate from the spec in #590 on current |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72d5ac311b
ℹ️ 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".
| const scopes = | ||
| body.scopes === undefined | ||
| ? normalizeOAuthScopes(["read", "write"]) | ||
| : normalizeOAuthScopes(body.scopes); |
There was a problem hiding this comment.
When this endpoint is authenticated with one of the new scoped API keys, requireApiSession() returns the key's scopes but this create path never compares them to the requested scopes. A key created with only read can therefore POST action: "createApiKey" and either omit scopes to get the default read,write key or request write explicitly, as long as the owner’s member role may create keys, bypassing the MCP write-scope checks.
Useful? React with 👍 / 👎.
| const responses = ( | ||
| await Promise.all( | ||
| body.map((entry) => | ||
| handleSingleJsonRpcRequest(entry, context, repository), | ||
| ), |
There was a problem hiding this comment.
For JSON-RPC batches with multiple tools/call entries, this runs each call concurrently while auditToolCall does a read-modify-write of workspace.settings.api.mcpAuditLog. Two calls can read the same existing log and then overwrite each other’s updates, so the MCP audit log can miss tool calls from a single batch; make the audit append atomic or process audited calls serially.
Useful? React with 👍 / 👎.
jaeyunha
force-pushed
the
issue-590-p2-mcp-server-authenticated-remote
branch
from
72d5ac3 to
1ada0aa
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ada0aa458
ℹ️ 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".
| if id == "" || body == "" { | ||
| return nil, errors.New("comment_id and body are required") | ||
| } | ||
| tag, err := h.DB.Exec(ctx, `update comment set body=$1, updated_at=now() where id=$2::uuid and user_id=$3`, body, id, p.UserID) |
There was a problem hiding this comment.
When the same user belongs to multiple workspaces, a PAT issued for workspace A can update one of that user's comments in workspace B if the caller knows the comment UUID, because this predicate only checks id and user_id. Since PATs are workspace-scoped, join through issue/team (and apply the same constraint to the delete query below) so MCP comment mutations cannot cross the credential's workspace boundary.
Useful? React with 👍 / 👎.
| title=coalesce(nullif($2,''), title), | ||
| description=case when $3 then $4 else description end, | ||
| priority=coalesce(nullif($5,'')::issue_priority, priority), | ||
| state_id=coalesce(nullif($6,'')::uuid, state_id), |
There was a problem hiding this comment.
When a caller supplies state_id, this writes the UUID directly without checking that it exists on the issue's team; issue.state_id is not protected by a foreign key in the schema, and the read paths inner-join workflow_state, so a typo or a state from another team/workspace can corrupt the issue or make it disappear from joined reads. Please validate the state against the target team before both updating and creating issues.
Useful? React with 👍 / 👎.
| defer rows.Close() | ||
| items := []map[string]any{} | ||
| for rows.Next() { | ||
| var id, name string |
There was a problem hiding this comment.
When a team has a cycle created without the optional name, cycle.name is NULL (the existing cycles API allows this), and scanning it into a plain string makes exponential_list_team_cycles fail the whole tool call with a scan error. Use a nullable string/pointer here and return null like the regular cycles API does.
Useful? React with 👍 / 👎.
| priority := enumOrDefault(firstString(args, "priority"), "none", "urgent", "high", "medium", "low") | ||
| status := enumOrDefault(firstString(args, "status"), "planned", "started", "paused", "completed", "canceled") | ||
| var id string | ||
| err := h.DB.QueryRow(ctx, `insert into project (name, slug, description, workspace_id, priority, status) values ($1,$2,nullif($3,''),$4::uuid,$5,$6) returning id::text`, name, slug, firstString(args, "description"), p.WorkspaceID, priority, status).Scan(&id) |
There was a problem hiding this comment.
When MCP clients pass the advertised team_ids/teamIds arguments, this create path only inserts the project row and never writes project_team associations (the update path also ignores them), so projects created through MCP are left unassigned and won't show up in team-scoped project lists/pickers as requested. Please resolve and persist the team links alongside the project.
Useful? React with 👍 / 👎.
|
Controller disposition: accepted for staging at current head |
1 participant
Closes #590.
Summary
origin/stagingand ported the remote MCP work into split-monorepo paths only.GET|POST|OPTIONS /v1/mcpinapps/api/internal/mcp, authenticated by bearer personal access tokens only.personal_access_token_audit_logentries for MCP tool calls.apps/websettings and updates remote/local MCP docs.Verification
pnpm lintpasses.docker run --rm -v "$PWD":/repo -w /repo/apps/api golang:1.25 go build ./...passes.docker run --rm -v "$PWD":/repo -w /repo/apps/api golang:1.25 go test ./...passes.pnpm --filter @exponential/web exec vitest run tests/settings-api.test.tsxpasses.make checkattempted; blocked by existing web typecheck failures on current staging paths such asapps/web/src/app/(app)/layout.tsxand settings pages, before Go/API gates.pnpm --filter @exponential/mcp-server testattempted; blocked by existing local package resolution for@namuh-eng/expn-sdkentry when the SDK package is not built.