fix(mcp): pass SSRF-guarded fetch into OAuth start flow, matching probe/revoke by waleedlatif1 · Pull Request #5398 · simstudioai/sim · GitHub
Skip to content

fix(mcp): pass SSRF-guarded fetch into OAuth start flow, matching probe/revoke#5398

Merged
waleedlatif1 merged 1 commit into
stagingfrom
worktree-agent-a0199a36ee807b978
Jul 3, 2026
Merged

fix(mcp): pass SSRF-guarded fetch into OAuth start flow, matching probe/revoke#5398
waleedlatif1 merged 1 commit into
stagingfrom
worktree-agent-a0199a36ee807b978

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • MCP OAuth start route was calling mcpAuth() without a fetchFn, so discovery/registration requests used the default global fetch
  • Sibling files probe.ts and revoke.ts already pass createSsrfGuardedMcpFetch() into their equivalent calls — this brings the start route in line with the same pattern
  • Added a test asserting the route wires the guarded fetch into mcpAuth

Type of Change

  • Bug fix

Testing

  • Added/ran unit tests for apps/sim/app/api/mcp/oauth/start/route.test.ts (all passing)
  • bun run check:api-validation passes
  • Typecheck and biome clean

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel

vercel Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown

@cursor

cursor Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown

PR Summary

Low Risk
Small security-hardening change that closes an SSRF gap in OAuth discovery; behavior otherwise unchanged aside from outbound fetch policy.

Overview
The MCP OAuth start route now passes createSsrfGuardedMcpFetch() as fetchFn into mcpAuth(), so OAuth discovery/registration HTTP uses the same SSRF-guarded transport as probe and revoke instead of the default global fetch.

A unit test mocks pinned-fetch, asserts the factory runs once, and checks mcpAuth receives serverUrl plus the guarded fetchFn.

Reviewed by Cursor Bugbot for commit 079f073. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR closes an SSRF gap in the MCP OAuth start route by passing createSsrfGuardedMcpFetch() as fetchFn to mcpAuth(), ensuring that every outbound request made during OAuth discovery and dynamic client registration is re-validated against the SSRF policy. The fix brings start/route.ts in line with the existing pattern already present in the probe and revoke helpers.

  • apps/sim/app/api/mcp/oauth/start/route.ts: Imports createSsrfGuardedMcpFetch and passes its return value as fetchFn to mcpAuth(), replacing the implicit use of the global fetch.
  • apps/sim/app/api/mcp/oauth/start/route.test.ts: Adds a mock for @/lib/mcp/pinned-fetch and a dedicated test that asserts the guarded fetch factory is called and its result is forwarded into mcpAuth.

Confidence Score: 5/5

Safe to merge — a one-line targeted fix backed by a well-structured new test, with no logic changes to the surrounding OAuth flow.

The change is minimal: one new import and one additional option passed to an existing call. The existing auth, permission, and URL-validation logic is untouched, and the new test correctly verifies that the guarded fetch factory is invoked and forwarded into mcpAuth on every request.

The sibling callback/route.ts makes its own mcpAuth call without fetchFn and may warrant a follow-up fix, but that is unrelated to this change.

Important Files Changed

Filename Overview
apps/sim/app/api/mcp/oauth/start/route.ts Adds createSsrfGuardedMcpFetch() as fetchFn to mcpAuth(), closing the SSRF gap and matching the probe/revoke pattern. Change is minimal and correct.
apps/sim/app/api/mcp/oauth/start/route.test.ts New test correctly mocks createSsrfGuardedMcpFetch, verifies the factory is invoked once, and asserts mcpAuth receives the guarded fetch as fetchFn.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Client
    participant StartRoute as start/route.ts
    participant SSRFGuard as createSsrfGuardedMcpFetch()
    participant mcpAuth as mcpAuth (MCP SDK)
    participant OAuthServer as OAuth / Authorization Server

    Client->>StartRoute: GET /api/mcp/oauth/start
    StartRoute->>StartRoute: Auth + permission check (withMcpAuth)
    StartRoute->>StartRoute: Load server row and OAuth row
    StartRoute->>SSRFGuard: createSsrfGuardedMcpFetch()
    SSRFGuard-->>StartRoute: guardedFetch (validates URL + pins IP per hop)
    StartRoute->>mcpAuth: "mcpAuth(provider, { serverUrl, fetchFn: guardedFetch })"
    mcpAuth->>SSRFGuard: fetch(discovery metadata URL)
    SSRFGuard->>SSRFGuard: validateMcpServerSsrf(url)
    SSRFGuard->>OAuthServer: pinned HTTP request
    OAuthServer-->>SSRFGuard: authorization server metadata
    SSRFGuard-->>mcpAuth: response
    mcpAuth-->>StartRoute: throw McpOauthRedirectRequired(authorizationUrl)
    StartRoute-->>Client: "200 { status: redirect, authorizationUrl }"
Loading 
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Client
    participant StartRoute as start/route.ts
    participant SSRFGuard as createSsrfGuardedMcpFetch()
    participant mcpAuth as mcpAuth (MCP SDK)
    participant OAuthServer as OAuth / Authorization Server

    Client->>StartRoute: GET /api/mcp/oauth/start
    StartRoute->>StartRoute: Auth + permission check (withMcpAuth)
    StartRoute->>StartRoute: Load server row and OAuth row
    StartRoute->>SSRFGuard: createSsrfGuardedMcpFetch()
    SSRFGuard-->>StartRoute: guardedFetch (validates URL + pins IP per hop)
    StartRoute->>mcpAuth: "mcpAuth(provider, { serverUrl, fetchFn: guardedFetch })"
    mcpAuth->>SSRFGuard: fetch(discovery metadata URL)
    SSRFGuard->>SSRFGuard: validateMcpServerSsrf(url)
    SSRFGuard->>OAuthServer: pinned HTTP request
    OAuthServer-->>SSRFGuard: authorization server metadata
    SSRFGuard-->>mcpAuth: response
    mcpAuth-->>StartRoute: throw McpOauthRedirectRequired(authorizationUrl)
    StartRoute-->>Client: "200 { status: redirect, authorizationUrl }"
Loading

Reviews (2): Last reviewed commit: "fix(mcp): pass SSRF-guarded fetch into O..." | Re-trigger Greptile

@waleedlatif1

waleedlatif1 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

Good catch on the callback route — that's being addressed in a separate PR against apps/sim/app/api/mcp/oauth/callback/route.ts (same pattern, different file). Keeping this PR scoped to the start route only.

@waleedlatif1
fix(mcp): pass SSRF-guarded fetch into OAuth start flow, matching pro…
079f073 
…be/revoke

Discovery and registration during the MCP OAuth start flow were using the
default global fetch. probe.ts and revoke.ts already route these calls
through createSsrfGuardedMcpFetch(); this brings the start route in line
with the same pattern.
@waleedlatif1 waleedlatif1 force-pushed the worktree-agent-a0199a36ee807b978 branch from 73be918 to 079f073 Compare July 3, 2026 22:24
@waleedlatif1

waleedlatif1 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile review

@waleedlatif1

waleedlatif1 commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator Author

cursor[bot]
cursor Bot reviewed Jul 3, 2026
View reviewed changes

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 079f073. Configure here.

@waleedlatif1 waleedlatif1 merged commit 1574c20 into staging Jul 3, 2026
18 checks passed
@waleedlatif1 waleedlatif1 deleted the worktree-agent-a0199a36ee807b978 branch July 3, 2026 22:31
@waleedlatif1 waleedlatif1 mentioned this pull request Jul 3, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cursor cursor[bot] cursor[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1