diff --git a/CHANGELOG.md b/CHANGELOG.md index 5a26cae..95477c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ ## Unreleased +### Changed + +- **Temporary resources** (`mapbox://temp/...`) are now scoped to the account that created them: a read by a different account returns the standard not-found response. Token resolution mirrors the tools (request auth, then the env token for stdio/single-user), so local reads are unaffected. Adds regression tests. + ## 0.12.2-dev - 2026-06-10 ### Security diff --git a/src/resources/temporary/TemporaryDataResource.ts b/src/resources/temporary/TemporaryDataResource.ts index 3c4ad88..d32cc2d 100644 --- a/src/resources/temporary/TemporaryDataResource.ts +++ b/src/resources/temporary/TemporaryDataResource.ts @@ -2,8 +2,14 @@ // Licensed under the MIT License. import { BaseResource } from '../BaseResource.js'; -import type { ReadResourceResult } from '@modelcontextprotocol/sdk/types.js'; +import type { RequestHandlerExtra } from '@modelcontextprotocol/sdk/shared/protocol.js'; +import type { + ReadResourceResult, + ServerNotification, + ServerRequest +} from '@modelcontextprotocol/sdk/types.js'; import { temporaryResourceManager } from '../../utils/temporaryResourceManager.js'; +import { getUserNameFromToken } from '../../utils/jwtUtils.js'; /** * Resource for temporary data storage (large tool responses). @@ -16,19 +22,46 @@ export class TemporaryDataResource extends BaseResource { 'Temporary storage for large tool responses. Data expires after TTL.'; readonly mimeType = 'application/json'; - async read(uri: string): Promise { + async read( + uri: string, + extra?: RequestHandlerExtra + ): Promise { + const notFound: ReadResourceResult = { + contents: [ + { + uri: uri, + mimeType: 'text/plain', + text: 'Resource not found or expired. Temporary resources have a 30-minute TTL.' + } + ] + }; + const resource = temporaryResourceManager.get(uri); if (!resource) { - return { - contents: [ - { - uri: uri, - mimeType: 'text/plain', - text: 'Resource not found or expired. Temporary resources have a 30-minute TTL.' - } - ] - }; + return notFound; + } + + // Enforce per-account ownership: only the account that created the resource + // may read it. Resolve the requester the same way tools resolve their token + // (request auth first, then the env token). A mismatch returns the SAME + // "not found" response as a missing resource, so a caller cannot probe + // whether someone else's resource exists. + // + // The env-token fallback is for stdio/single-user mode only. In hosted + // multi-tenant deployments MAPBOX_ACCESS_TOKEN is NOT set and every request + // carries its own bearer token via authInfo; the request token therefore + // always wins and the env fallback never applies cross-tenant. Do not set a + // shared MAPBOX_ACCESS_TOKEN in a multi-tenant deployment or all env-less + // reads would resolve to that one account. + const requesterToken = + (extra?.authInfo?.token as string | undefined) ?? + process.env.MAPBOX_ACCESS_TOKEN; + const requester = requesterToken + ? getUserNameFromToken(requesterToken) + : undefined; + if (!resource.owner || !requester || resource.owner !== requester) { + return notFound; } // For image data, return as blob diff --git a/src/tools/directions-tool/DirectionsTool.ts b/src/tools/directions-tool/DirectionsTool.ts index e749070..c48ab14 100644 --- a/src/tools/directions-tool/DirectionsTool.ts +++ b/src/tools/directions-tool/DirectionsTool.ts @@ -16,6 +16,7 @@ import { } from './DirectionsTool.output.schema.js'; import type { HttpRequest } from '../..//utils/types.js'; import { temporaryResourceManager } from '../../utils/temporaryResourceManager.js'; +import { getUserNameFromToken } from '../../utils/jwtUtils.js'; import { isMcpUiEnabled } from '../../config/toolConfig.js'; import { resolveMapboxPublicToken } from '../../utils/mapboxPublicToken.js'; import { renderDirectionsAppHtml } from '../../resources/ui-apps/directionsAppHtml.js'; @@ -297,9 +298,12 @@ export class DirectionsTool extends MapboxApiBasedTool< const resourceId = randomBytes(16).toString('hex'); const resourceUri = `mapbox://temp/directions-${resourceId}`; - temporaryResourceManager.create(resourceId, resourceUri, validatedData, { - toolName: this.name, - size: responseSize + temporaryResourceManager.create({ + id: resourceId, + uri: resourceUri, + data: validatedData, + metadata: { toolName: this.name, size: responseSize }, + owner: getUserNameFromToken(accessToken) }); // Extract summary information diff --git a/src/tools/isochrone-tool/IsochroneTool.ts b/src/tools/isochrone-tool/IsochroneTool.ts index 3348eda..ac74bda 100644 --- a/src/tools/isochrone-tool/IsochroneTool.ts +++ b/src/tools/isochrone-tool/IsochroneTool.ts @@ -12,6 +12,7 @@ import { type IsochroneResponse } from './IsochroneTool.output.schema.js'; import { temporaryResourceManager } from '../../utils/temporaryResourceManager.js'; +import { getUserNameFromToken } from '../../utils/jwtUtils.js'; export class IsochroneTool extends MapboxApiBasedTool< typeof IsochroneInputSchema, @@ -161,9 +162,12 @@ export class IsochroneTool extends MapboxApiBasedTool< const resourceId = randomBytes(16).toString('hex'); const resourceUri = `mapbox://temp/isochrone-${resourceId}`; - temporaryResourceManager.create(resourceId, resourceUri, data, { - toolName: this.name, - size: responseSize + temporaryResourceManager.create({ + id: resourceId, + uri: resourceUri, + data, + metadata: { toolName: this.name, size: responseSize }, + owner: getUserNameFromToken(accessToken) }); const contourCount = diff --git a/src/tools/static-map-image-tool/StaticMapImageTool.ts b/src/tools/static-map-image-tool/StaticMapImageTool.ts index 28d48d8..d619936 100644 --- a/src/tools/static-map-image-tool/StaticMapImageTool.ts +++ b/src/tools/static-map-image-tool/StaticMapImageTool.ts @@ -9,6 +9,7 @@ import { StaticMapImageInputSchema } from './StaticMapImageTool.input.schema.js' import type { OverlaySchema } from './StaticMapImageTool.input.schema.js'; import type { CallToolResult } from '@modelcontextprotocol/sdk/types.js'; import { temporaryResourceManager } from '../../utils/temporaryResourceManager.js'; +import { getUserNameFromToken } from '../../utils/jwtUtils.js'; // Images larger than this threshold are stored as temporary resources instead // of being inlined as base64, to avoid exceeding Claude Desktop's 1MB tool @@ -134,14 +135,14 @@ export class StaticMapImageTool extends MapboxApiBasedTool< const resourceId = randomBytes(16).toString('hex'); const resourceUri = `mapbox://temp/static-map-${resourceId}`; const base64Data = Buffer.from(buffer).toString('base64'); - temporaryResourceManager.create( - resourceId, - resourceUri, - base64Data, - { toolName: this.name, size: buffer.byteLength }, - undefined, - mimeType - ); + temporaryResourceManager.create({ + id: resourceId, + uri: resourceUri, + data: base64Data, + metadata: { toolName: this.name, size: buffer.byteLength }, + mimeType, + owner: getUserNameFromToken(accessToken) + }); content.push({ type: 'text', text: `⚠️ Image (${Math.round(buffer.byteLength / 1024)}KB) stored as temporary resource.\nResource URI: ${resourceUri}\nTTL: 30 minutes` diff --git a/src/utils/temporaryResourceManager.ts b/src/utils/temporaryResourceManager.ts index fb9f7b9..9ae5f7d 100644 --- a/src/utils/temporaryResourceManager.ts +++ b/src/utils/temporaryResourceManager.ts @@ -16,6 +16,13 @@ export interface TemporaryResource { byteSize: number; created: number; ttl: number; + /** + * Account (Mapbox username) that created this resource. Reads are restricted + * to the same account so one account cannot read another account's temporary + * data. Undefined means no owner could be determined, in which case reads + * fail closed. + */ + owner?: string; metadata?: { toolName?: string; size?: number; @@ -39,14 +46,16 @@ export class TemporaryResourceManager { * Create a temporary resource. Evicts oldest entries if the byte cap would * be exceeded. */ - create( - id: string, - uri: string, - data: unknown, - metadata?: TemporaryResource['metadata'], - ttl?: number, - mimeType?: string - ): TemporaryResource { + create(params: { + id: string; + uri: string; + data: unknown; + owner?: string; + metadata?: TemporaryResource['metadata']; + ttl?: number; + mimeType?: string; + }): TemporaryResource { + const { id, uri, data, owner, metadata, ttl, mimeType } = params; const dataStr = typeof data === 'string' ? data : JSON.stringify(data); const byteSize = Buffer.byteLength(dataStr, 'utf8'); @@ -69,6 +78,7 @@ export class TemporaryResourceManager { byteSize, created: Date.now(), ttl: ttl ?? this.defaultTTL, + owner, metadata }; diff --git a/test/resources/temporary/TemporaryDataResource.test.ts b/test/resources/temporary/TemporaryDataResource.test.ts new file mode 100644 index 0000000..ebc7a5a --- /dev/null +++ b/test/resources/temporary/TemporaryDataResource.test.ts @@ -0,0 +1,149 @@ +// Copyright (c) Mapbox, Inc. +// Licensed under the MIT License. + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { TemporaryDataResource } from '../../../src/resources/temporary/TemporaryDataResource.js'; +import { temporaryResourceManager } from '../../../src/utils/temporaryResourceManager.js'; + +// Build a Mapbox-style 3-part JWT whose payload carries the username (`u`). +function tokenFor(username: string): string { + const payload = Buffer.from(JSON.stringify({ u: username })).toString( + 'base64' + ); + return `pk.${payload}.sig`; +} + +// eslint-disable-next-line @typescript-eslint/no-explicit-any +function extraFor(token?: string): any { + return token ? { authInfo: { token } } : {}; +} + +const NOT_FOUND = + 'Resource not found or expired. Temporary resources have a 30-minute TTL.'; + +describe('TemporaryDataResource — AGI-890 cross-account access control', () => { + let resource: TemporaryDataResource; + let savedEnvToken: string | undefined; + + beforeEach(() => { + temporaryResourceManager.clear(); + resource = new TemporaryDataResource(); + savedEnvToken = process.env.MAPBOX_ACCESS_TOKEN; + delete process.env.MAPBOX_ACCESS_TOKEN; + }); + + afterEach(() => { + temporaryResourceManager.clear(); + if (savedEnvToken !== undefined) { + process.env.MAPBOX_ACCESS_TOKEN = savedEnvToken; + } else { + delete process.env.MAPBOX_ACCESS_TOKEN; + } + }); + + function seedTextResource(uri: string, owner: string, data: unknown) { + temporaryResourceManager.create({ + id: 'id', + uri, + data, + metadata: { toolName: 'directions_tool' }, + owner + }); + } + + it('lets the creating account read its own resource', async () => { + const uri = 'mapbox://temp/directions-aaa'; + seedTextResource(uri, 'accountA', { route: 'A-secret-geometry' }); + + const result = await resource.read(uri, extraFor(tokenFor('accountA'))); + const text = result.contents[0].text as string; + + expect(text).toContain('A-secret-geometry'); + }); + + it('does NOT return another account’s resource body (regression)', async () => { + const uri = 'mapbox://temp/directions-bbb'; + seedTextResource(uri, 'accountA', { route: 'A-secret-geometry' }); + + const result = await resource.read(uri, extraFor(tokenFor('accountB'))); + const text = result.contents[0].text as string; + + expect(text).toBe(NOT_FOUND); + expect(text).not.toContain('A-secret-geometry'); + }); + + it('returns an identical response for "not yours" and "does not exist" (no existence oracle)', async () => { + const ownedUri = 'mapbox://temp/directions-ccc'; + seedTextResource(ownedUri, 'accountA', { route: 'A-secret-geometry' }); + + const crossAccount = await resource.read( + ownedUri, + extraFor(tokenFor('accountB')) + ); + const missing = await resource.read( + 'mapbox://temp/directions-does-not-exist', + extraFor(tokenFor('accountB')) + ); + + // The only field that differs is the echoed-back request URI (caller's own + // input, not an existence signal). The mimeType and message are identical, + // so a caller cannot distinguish "not yours" from "does not exist". + expect(crossAccount.contents[0].mimeType).toBe( + missing.contents[0].mimeType + ); + expect(crossAccount.contents[0].text).toBe(missing.contents[0].text); + expect(crossAccount.contents[0].text).toBe(NOT_FOUND); + }); + + it('fails closed when the reader has no token', async () => { + const uri = 'mapbox://temp/directions-ddd'; + seedTextResource(uri, 'accountA', { route: 'A-secret-geometry' }); + + const result = await resource.read(uri, extraFor(undefined)); + expect(result.contents[0].text).toBe(NOT_FOUND); + }); + + it('fails closed when the resource has no owner recorded', async () => { + const uri = 'mapbox://temp/directions-eee'; + // No owner -> owner undefined + temporaryResourceManager.create({ + id: uri, + uri, + data: { route: 'legacy' } + }); + + const result = await resource.read(uri, extraFor(tokenFor('accountA'))); + expect(result.contents[0].text).toBe(NOT_FOUND); + }); + + it('falls back to the env token so stdio/single-user reads still work', async () => { + const envToken = tokenFor('localuser'); + process.env.MAPBOX_ACCESS_TOKEN = envToken; + const uri = 'mapbox://temp/directions-fff'; + seedTextResource(uri, 'localuser', { route: 'local-data' }); + + // No authInfo on the request (stdio) -> requester resolved from env token. + const result = await resource.read(uri, extraFor(undefined)); + expect(result.contents[0].text as string).toContain('local-data'); + }); + + it('returns image blobs to the owner and not-found to others', async () => { + const uri = 'mapbox://temp/static-map-ggg'; + temporaryResourceManager.create({ + id: 'imgid', + uri, + data: 'BASE64IMAGEDATA', + metadata: { toolName: 'static_map_image_tool' }, + mimeType: 'image/png', + owner: 'accountA' + }); + + const owner = await resource.read(uri, extraFor(tokenFor('accountA'))); + expect(owner.contents[0].blob).toBe('BASE64IMAGEDATA'); + expect(owner.contents[0].mimeType).toBe('image/png'); + + const other = await resource.read(uri, extraFor(tokenFor('accountB'))); + expect(other.contents[0].blob).toBeUndefined(); + expect(other.contents[0].text).toBe(NOT_FOUND); + }); +}); diff --git a/test/tools/directions-tool/DirectionsTool.test.ts b/test/tools/directions-tool/DirectionsTool.test.ts index 027de31..72cfbbc 100644 --- a/test/tools/directions-tool/DirectionsTool.test.ts +++ b/test/tools/directions-tool/DirectionsTool.test.ts @@ -10,6 +10,7 @@ import { } from '../../utils/httpPipelineUtils.js'; import { DirectionsTool } from '../../../src/tools/directions-tool/DirectionsTool.js'; import * as cleanResponseModule from '../../../src/tools/directions-tool/cleanResponseData.js'; +import { temporaryResourceManager } from '../../../src/utils/temporaryResourceManager.js'; describe('DirectionsTool', () => { beforeEach(() => { @@ -1159,3 +1160,62 @@ describe('DirectionsTool', () => { }); }); }); + +describe('DirectionsTool — temporary resource ownership', () => { + afterEach(() => { + vi.restoreAllMocks(); + temporaryResourceManager.clear(); + }); + + // Build a Mapbox-style JWT whose payload carries the username (`u`). + function tokenFor(username: string): string { + const payload = Buffer.from(JSON.stringify({ u: username })).toString( + 'base64' + ); + return `pk.${payload}.sig`; + } + + it('stores the temp resource with owner = the calling account (real token round-trip)', async () => { + // A geojson response large enough (>50KB) to be stored as a temp resource. + const bigGeometry = { + type: 'LineString', + coordinates: Array.from({ length: 4000 }, (_, i) => [ + i * 0.001, + i * 0.001 + ]) + }; + const largeResponse = { + code: 'Ok', + routes: [{ distance: 1, duration: 1, geometry: bigGeometry }], + waypoints: [] + }; + const { httpRequest } = setupHttpRequest({ + json: async () => largeResponse + }); + + const token = tokenFor('account-zhuwenlong'); + const result = await new DirectionsTool({ httpRequest }).run( + { + coordinates: [ + { longitude: -73.989, latitude: 40.733 }, + { longitude: -73.979, latitude: 40.743 } + ], + geometries: 'geojson' + }, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + { authInfo: { token } } as any + ); + + // The transcript should point to a temp resource... + const text = (result.content ?? []) + .map((c) => (c as { text?: string }).text ?? '') + .join('\n'); + const uri = text.match(/mapbox:\/\/temp\/[^\s`]+/)?.[0]; + expect(uri).toBeTruthy(); + + // ...and that resource must be owned by the account from the token, not + // undefined — proving the tool wires `owner` from the real accessToken. + const stored = temporaryResourceManager.get(uri as string); + expect(stored?.owner).toBe('account-zhuwenlong'); + }); +});