Skip to content

Commit 7b0b2de

Browse files
dcramerComposercodex
authored
feat(mcp): Carry regionUrl on constraints and unify regional routing (#907)
Resolve and carry `regionUrl` on MCP session constraints so it is filtered from tool schemas and auto-injected like `organizationSlug` when it is a non-empty string. Hosted verification already loads `links.regionUrl` from the org API; OAuth can also persist `constraintRegionUrl` when the grant resource scopes `/mcp/:org`, and the worker merges that into URL constraints when the request path org matches the token scope (defense-in-depth when cache or verification paths differ). Stdio now performs a one-time `getOrganization` when both an org slug and access token are configured, so CLI org-scoped sessions populate `constraints.regionUrl` the same way. Trace, profile, and replay detail tools share `resolveRegionUrlForOrganization`; `get_sentry_resource` forwards `constraints.regionUrl` into composed handlers because those calls bypass MCP parameter merge. Constraint helpers gain documentation and tests for `regionUrl` as a string constraint. OAuth `resource` URL parsing is extracted to `resource-scope.ts` for reuse in authorize and callback. Cloudflare constraint verification caching is adjusted for org-only keys alongside existing project-scoped entries. Reviewers should sanity-check the OAuth merge conditions in `mcp-handler.ts` and KV cache semantics in `constraint-utils.ts` for multi-tenant isolation. --------- Co-authored-by: Composer <composer@cursor.com> Co-authored-by: Codex CLI Agent <noreply@openai.com>
1 parent dcce393 commit 7b0b2de

19 files changed

Lines changed: 510 additions & 104 deletions

‎packages/mcp-cloudflare/src/server/lib/constraint-utils.test.ts‎

Lines changed: 83 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,7 @@ describe("verifyConstraintsAccess", () => {
177177
// Verify cache was checked
178178
expect(mockKV.get).toHaveBeenCalledOnce();
179179
expect(mockKV.get).toHaveBeenCalledWith(
180-
"caps:v1:user-123:sentry.io:sentry-mcp-evals:cloudflare-mcp",
180+
"caps:v1:user-123:sentry.io:sentry-mcp-evals:project:cloudflare-mcp",
181181
"json",
182182
);
183183

@@ -222,7 +222,7 @@ describe("verifyConstraintsAccess", () => {
222222
// Verify cache was written with correct key and TTL
223223
expect(mockKV.put).toHaveBeenCalledOnce();
224224
expect(mockKV.put).toHaveBeenCalledWith(
225-
"caps:v1:user-456:sentry.io:sentry-mcp-evals:cloudflare-mcp",
225+
"caps:v1:user-456:sentry.io:sentry-mcp-evals:project:cloudflare-mcp",
226226
expect.any(String),
227227
{ expirationTtl: 900 },
228228
);
@@ -294,8 +294,13 @@ describe("verifyConstraintsAccess", () => {
294294
expect(mockKV.put).toHaveBeenCalledOnce();
295295
});
296296

297-
it("does not use cache for org-only verification", async () => {
298-
const mockKV = createMockKV({ getResult: cachedData });
297+
it("uses cache for org-only verification", async () => {
298+
const orgOnlyCached: CachedConstraints = {
299+
regionUrl: "https://us.sentry.io",
300+
projectCapabilities: null,
301+
cachedAt: Date.now(),
302+
};
303+
const mockKV = createMockKV({ getResult: orgOnlyCached });
299304
const cache: CacheOptions = {
300305
kv: mockKV,
301306
userId: "user-org-only",
@@ -307,10 +312,82 @@ describe("verifyConstraintsAccess", () => {
307312
);
308313

309314
expect(result.ok).toBe(true);
315+
if (result.ok) {
316+
expect(result.constraints).toEqual({
317+
organizationSlug: "sentry-mcp-evals",
318+
projectSlug: null,
319+
regionUrl: "https://us.sentry.io",
320+
projectCapabilities: null,
321+
});
322+
}
310323

311-
// Cache should not be checked for org-only verification
312-
expect(mockKV.get).not.toHaveBeenCalled();
324+
expect(mockKV.get).toHaveBeenCalledOnce();
325+
expect(mockKV.get).toHaveBeenCalledWith(
326+
"caps:v1:user-org-only:sentry.io:sentry-mcp-evals:org",
327+
"json",
328+
);
313329
expect(mockKV.put).not.toHaveBeenCalled();
314330
});
331+
332+
it("uses distinct cache keys for org-only and '__org__' project constraints", async () => {
333+
const mockKV = createMockKV({ getResult: cachedData });
334+
const cache: CacheOptions = {
335+
kv: mockKV,
336+
userId: "user-sentinel",
337+
};
338+
339+
const orgOnlyResult = await verifyConstraintsAccess(
340+
{ organizationSlug: "sentry-mcp-evals", projectSlug: null },
341+
{ accessToken: token, sentryHost: host, cache },
342+
);
343+
const projectResult = await verifyConstraintsAccess(
344+
{ organizationSlug: "sentry-mcp-evals", projectSlug: "__org__" },
345+
{ accessToken: token, sentryHost: host, cache },
346+
);
347+
348+
expect(orgOnlyResult.ok).toBe(true);
349+
expect(projectResult.ok).toBe(true);
350+
expect(mockKV.get).toHaveBeenNthCalledWith(
351+
1,
352+
"caps:v1:user-sentinel:sentry.io:sentry-mcp-evals:org",
353+
"json",
354+
);
355+
expect(mockKV.get).toHaveBeenNthCalledWith(
356+
2,
357+
"caps:v1:user-sentinel:sentry.io:sentry-mcp-evals:project:__org__",
358+
"json",
359+
);
360+
});
361+
362+
it("writes KV cache after org-only verification on miss", async () => {
363+
const mockKV = createMockKV({ getResult: null });
364+
const cache: CacheOptions = {
365+
kv: mockKV,
366+
userId: "user-org-cache-write",
367+
};
368+
369+
const result = await verifyConstraintsAccess(
370+
{ organizationSlug: "sentry-mcp-evals", projectSlug: null },
371+
{ accessToken: token, sentryHost: host, cache },
372+
);
373+
374+
expect(result.ok).toBe(true);
375+
expect(mockKV.get).toHaveBeenCalledOnce();
376+
await new Promise((resolve) => setTimeout(resolve, 10));
377+
expect(mockKV.put).toHaveBeenCalledOnce();
378+
expect(mockKV.put).toHaveBeenCalledWith(
379+
"caps:v1:user-org-cache-write:sentry.io:sentry-mcp-evals:org",
380+
expect.any(String),
381+
{ expirationTtl: 900 },
382+
);
383+
const parsed = JSON.parse(
384+
vi.mocked(mockKV.put).mock.calls[0][1] as string,
385+
);
386+
expect(parsed).toMatchObject({
387+
regionUrl: "https://us.sentry.io",
388+
projectCapabilities: null,
389+
cachedAt: expect.any(Number),
390+
});
391+
});
315392
});
316393
});

‎packages/mcp-cloudflare/src/server/lib/constraint-utils.ts‎

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ const CACHE_KEY_VERSION = "v1";
1414
*/
1515
export type CachedConstraints = {
1616
regionUrl: string | null;
17-
projectCapabilities: ProjectCapabilities;
17+
/** Populated when a project constraint was verified; null for org-only cache entries. */
18+
projectCapabilities: ProjectCapabilities | null;
1819
cachedAt: number;
1920
};
2021

@@ -28,15 +29,20 @@ export type CacheOptions = {
2829

2930
/**
3031
* Build a cache key for constraints verification.
31-
* Format: caps:v1:{userId}:{sentryHost}:{organizationSlug}:{projectSlug}
32+
* Format: caps:v1:{userId}:{sentryHost}:{organizationSlug}:{scopeKey}
33+
* scopeKey is "org" for org-only entries or "project:{slug}" for project-scoped entries.
3234
*/
3335
function buildCacheKey(
3436
userId: string,
3537
sentryHost: string,
3638
organizationSlug: string,
37-
projectSlug: string,
39+
projectSlug: string | null | undefined,
3840
): string {
39-
return `caps:${CACHE_KEY_VERSION}:${userId}:${sentryHost}:${organizationSlug}:${projectSlug}`;
41+
const normalizedProjectSlug = projectSlug?.trim();
42+
const scopeKey = normalizedProjectSlug
43+
? `project:${normalizedProjectSlug}`
44+
: "org";
45+
return `caps:${CACHE_KEY_VERSION}:${userId}:${sentryHost}:${organizationSlug}:${scopeKey}`;
4046
}
4147

4248
/**
@@ -149,10 +155,10 @@ export async function verifyConstraintsAccess(
149155
};
150156
}
151157

152-
// Check cache if project constraints are requested and cache is available
153-
// Cache key includes userId to ensure per-user isolation
158+
// Check KV cache when available (org-only and org+project keys).
159+
// Cache key includes userId to ensure per-user isolation.
154160
let cacheKey: string | null = null;
155-
if (projectSlug && cache) {
161+
if (cache) {
156162
cacheKey = buildCacheKey(
157163
cache.userId,
158164
sentryHost,
@@ -161,12 +167,11 @@ export async function verifyConstraintsAccess(
161167
);
162168
const cached = await getCachedConstraints(cache.kv, cacheKey);
163169
if (cached) {
164-
// Cache hit - return cached constraints without API calls
165170
return {
166171
ok: true,
167172
constraints: {
168173
organizationSlug,
169-
projectSlug,
174+
projectSlug: projectSlug || null,
170175
regionUrl: cached.regionUrl,
171176
projectCapabilities: cached.projectCapabilities,
172177
},
@@ -254,12 +259,12 @@ export async function verifyConstraintsAccess(
254259
}
255260
}
256261

257-
// Cache successful verification results for project constraints
258-
// Fire-and-forget write - don't block response on cache update
259-
if (cacheKey && projectCapabilities) {
262+
// Cache: org-only after org fetch; org+project only when project verification
263+
// succeeded (skip caching on project timeout so the next request can retry).
264+
if (cacheKey && (!projectSlug || projectCapabilities !== null)) {
260265
void setCachedConstraints(cache!.kv, cacheKey, {
261266
regionUrl: regionUrl || null,
262-
projectCapabilities,
267+
projectCapabilities: projectSlug ? projectCapabilities : null,
263268
cachedAt: Date.now(),
264269
});
265270
}

‎packages/mcp-cloudflare/src/server/lib/mcp-handler.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ interface OAuthProps {
99
accessToken: string;
1010
refreshToken: string;
1111
grantedSkills: string[];
12+
constraintOrganizationSlug?: string | null;
13+
constraintProjectSlug?: string | null;
1214
}
1315

1416
const DEFAULT_OAUTH_PROPS: OAuthProps = {
@@ -248,6 +250,68 @@ describe("MCP Handler", () => {
248250
expect(response.status).toBe(404);
249251
expect(await response.text()).toContain("not found");
250252
});
253+
254+
it("returns 403 when the token is org-scoped but the MCP URL uses a different organization", async () => {
255+
const request = createMcpRequest(
256+
"initialize",
257+
{
258+
protocolVersion: "2024-11-05",
259+
capabilities: {},
260+
clientInfo: { name: "test-client", version: "1.0.0" },
261+
},
262+
{ path: "/mcp/other-org" },
263+
);
264+
const ctx = createMcpContext({
265+
constraintOrganizationSlug: "my-org",
266+
});
267+
268+
const response = await mcpHandler.fetch!(request, createTestEnv(), ctx);
269+
270+
expect(response.status).toBe(403);
271+
expect(await response.text()).toContain("scoped to an organization");
272+
});
273+
274+
it("returns 403 when the token is project-scoped but the MCP URL uses a different project", async () => {
275+
const request = createMcpRequest(
276+
"initialize",
277+
{
278+
protocolVersion: "2024-11-05",
279+
capabilities: {},
280+
clientInfo: { name: "test-client", version: "1.0.0" },
281+
},
282+
{ path: "/mcp/my-org/wrong-project" },
283+
);
284+
const ctx = createMcpContext({
285+
constraintOrganizationSlug: "my-org",
286+
constraintProjectSlug: "expected-project",
287+
});
288+
289+
const response = await mcpHandler.fetch!(request, createTestEnv(), ctx);
290+
291+
expect(response.status).toBe(403);
292+
expect(await response.text()).toContain("scoped to a project");
293+
});
294+
295+
it("returns 403 when the token is project-scoped but the MCP URL omits the project segment", async () => {
296+
const request = createMcpRequest(
297+
"initialize",
298+
{
299+
protocolVersion: "2024-11-05",
300+
capabilities: {},
301+
clientInfo: { name: "test-client", version: "1.0.0" },
302+
},
303+
{ path: "/mcp/my-org" },
304+
);
305+
const ctx = createMcpContext({
306+
constraintOrganizationSlug: "my-org",
307+
constraintProjectSlug: "my-project",
308+
});
309+
310+
const response = await mcpHandler.fetch!(request, createTestEnv(), ctx);
311+
312+
expect(response.status).toBe(403);
313+
expect(await response.text()).toContain("scoped to a project");
314+
});
251315
});
252316

253317
describe("MCP protocol", () => {

‎packages/mcp-cloudflare/src/server/lib/mcp-handler.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,6 +220,23 @@ const mcpHandler: ExportedHandler<Env> = {
220220
);
221221
}
222222

223+
const tokenOrg = rawProps.constraintOrganizationSlug?.trim() || null;
224+
const tokenProject = rawProps.constraintProjectSlug?.trim() || null;
225+
if (tokenOrg && organizationSlug !== tokenOrg) {
226+
return new Response(
227+
"This token is scoped to an organization. Use the MCP URL for the organization you authorized.",
228+
{ status: 403 },
229+
);
230+
}
231+
if (tokenProject) {
232+
if (!projectSlug || projectSlug !== tokenProject) {
233+
return new Response(
234+
"This token is scoped to a project. Use the MCP URL that includes that project (for example /mcp/<org>/<project>).",
235+
{ status: 403 },
236+
);
237+
}
238+
}
239+
223240
// Verify user has access to the requested org/project
224241
// Cache verification results in KV to avoid repeated API calls
225242
const verification = await verifyConstraintsAccess(
@@ -240,13 +257,15 @@ const mcpHandler: ExportedHandler<Env> = {
240257
});
241258
}
242259

260+
const constraints = verification.constraints;
261+
243262
// Build complete ServerContext from OAuth props + verified constraints
244263
const serverContext: ServerContext = {
245264
userId,
246265
clientId,
247266
accessToken,
248267
grantedSkills: validSkills,
249-
constraints: verification.constraints,
268+
constraints,
250269
sentryHost,
251270
mcpUrl: env.MCP_URL,
252271
agentMode: isAgentMode,
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
/**
2+
* Parse RFC 8707 `resource` URLs that scope this MCP deployment to an org/project.
3+
* Example: https://example.com/mcp/my-org/backend
4+
*/
5+
export function parseResourceMcpConstraints(
6+
resource: string | null | undefined,
7+
): { organizationSlug: string; projectSlug: string | null } | null {
8+
if (!resource) {
9+
return null;
10+
}
11+
12+
try {
13+
const { pathname } = new URL(resource);
14+
const pathSegments = pathname.split("/").filter(Boolean);
15+
16+
if (pathSegments[0] !== "mcp") {
17+
return null;
18+
}
19+
20+
if (pathSegments.length === 2) {
21+
return {
22+
organizationSlug: pathSegments[1],
23+
projectSlug: null,
24+
};
25+
}
26+
27+
if (pathSegments.length === 3) {
28+
return {
29+
organizationSlug: pathSegments[1],
30+
projectSlug: pathSegments[2],
31+
};
32+
}
33+
34+
return null;
35+
} catch {
36+
return null;
37+
}
38+
}

0 commit comments

Comments
 (0)