diff --git a/src/resources/AGENTS.md b/src/resources/AGENTS.md index 661ab7586..c1cb7214f 100644 --- a/src/resources/AGENTS.md +++ b/src/resources/AGENTS.md @@ -30,8 +30,10 @@ to two origins — the one clients are told to use (`getApifyAPIPublicBaseUrl()` and the egress one (`getApifyAPIBaseUrl()`), which differ on the hosted server — and rejects userinfo-bearing URLs (axios drops the `Authorization` header for those, silently degrading to unauthenticated). An accepted URI is sent on the egress origin and returned under the URI as given; -templates, instructions, refusals and link-out URLs name only the client-facing origin. The API tools' `callApi` -(`../tools/api/apify_api_request.ts`) reuses `isApifyApiUri()`, `isMaxContentLengthAbort()`, +templates, instructions, refusals and link-out URLs name only the client-facing origin. `readApiResource()` +then refuses, without a request, what the API tools refuse (`validateApiBlocklist` in +`../tools/api/apify_api_blocklist.ts`): the `method` query parameter and a GET to the synchronous run +endpoints. The API tools' `callApi` (`../tools/api/apify_api_request.ts`) reuses `isApifyApiUri()`, `isMaxContentLengthAbort()`, `maskSessionToken()`, `redactJsonSecrets()` and `REDACTED`, so a change to any changes the tools too. `get-key-value-store-record` builds its inlined-record URI with `buildUrlOnOrigin()`. diff --git a/src/resources/api_resources.ts b/src/resources/api_resources.ts index b889b4161..b827dcaa6 100644 --- a/src/resources/api_resources.ts +++ b/src/resources/api_resources.ts @@ -9,6 +9,8 @@ import type { ApifyClient } from '../apify_client.js'; import { getApifyAPIBaseUrl, getApifyAPIPublicBaseUrl, sendApifyApiRequest } from '../apify_client.js'; import { MAX_INLINE_BYTES } from '../const.js'; import { InternalError, InvalidParamsError } from '../mcp/errors.js'; +import { validateApiBlocklist } from '../tools/api/apify_api_blocklist.js'; +import { normalizeApiPath } from '../tools/api/apify_api_path.js'; import { parseBaseMimeType } from '../tools/storage/storage_helpers.js'; import { getHttpStatusCode, logHttpError } from '../utils/logging.js'; import { getHttpErrorHint } from '../utils/mcp.js'; @@ -208,8 +210,8 @@ export function redactJsonSecrets(json: string): string { * JSON primitives, formatting, and bytes round-trip exactly, except that the session token and * `urlSigningSecretKey` and `password` values in JSON become `[REDACTED]`. * - * Genuine failures (no token, bad origin, a missing resource, a bad token, a 5xx, a network error) - * throw a domain error (`InvalidParamsError`/`InternalError`) that the protocol adapters + * Genuine failures (no token, bad origin, a request the API tools refuse, a missing resource, a bad token, + * a 5xx, a network error) throw a domain error (`InvalidParamsError`/`InternalError`) that the protocol adapters * (`legacy_server.ts`, `stateless_server.ts`) map to a JSON-RPC error, so the SDK returns an error * rather than success-shaped content for an unreadable resource (see SEP-2164). A body over * `MAX_INLINE_BYTES` is NOT a failure — it is a successful read returning a download pointer. @@ -229,6 +231,15 @@ export async function readApiResource(uri: string, apifyClient?: ApifyClient): P uri, }); } + // A GET with the method query parameter can delete, and one to a synchronous run endpoint starts a paid run; + // see apify/apify-mcp-server#1524. + const { pathname, search } = new URL(uri); + const refusal = validateApiBlocklist({ + method: 'GET', + normalizedPath: normalizeApiPath(`${pathname}${search}`), + loadedToolNames: [], + }); + if (refusal) throw new InvalidParamsError(`Failed to read ${uri}: ${refusal}`, { uri }); let response: { data: unknown; headers: Record; status: number; statusText: string }; try { diff --git a/src/tools/api/apify_api_blocklist.ts b/src/tools/api/apify_api_blocklist.ts index d67d35ea1..ed3cd4c5a 100644 --- a/src/tools/api/apify_api_blocklist.ts +++ b/src/tools/api/apify_api_blocklist.ts @@ -26,9 +26,7 @@ export const API_BLOCK_RULES: readonly ApiBlockRule[] = [ { // A GET with method=DELETE deletes, so the read tool could write; see apify/apify-mcp-server#1501. match: { queryParam: 'method' }, - reason: - 'The API tools do not send the method query parameter: the API would take it as the HTTP method ' + - 'of the request.', + reason: 'The method query parameter is not allowed because it can turn a read request into a write request.', suggestedToolNames: [HELPER_TOOLS.API_WRITE], }, // A GET here starts a paid run, so the read tool would not be read-only; see apify/apify-mcp-server#1502. diff --git a/tests/unit/resources.api.test.ts b/tests/unit/resources.api.test.ts index db5063ea0..a7ea940ee 100644 --- a/tests/unit/resources.api.test.ts +++ b/tests/unit/resources.api.test.ts @@ -12,6 +12,12 @@ import { mockApifyClient } from './helpers/tool_context.js'; const API = 'https://api.apify.com'; +const METHOD_PARAM_REFUSAL = + 'The method query parameter is not allowed because it can turn a read request into a write request.'; + +const SYNC_RUN_REFUSAL = + 'A GET to the synchronous run endpoints starts a paid run, as a POST does, so the API tools do not send it.'; + // `contents[0]` is a text|blob union; narrow it in tests that read one shape. function firstContent(result: ReadResourceResult): { mimeType?: string; text?: string; blob?: string } { return result.contents[0] as { mimeType?: string; text?: string; blob?: string }; @@ -205,6 +211,23 @@ describe('readApiResource()', () => { expect(error.data).toEqual({ uri: 'https://example.com/steal-my-token' }); }); + it.each([ + [`${API}/v2/datasets/ds-1?method=DELETE`, METHOD_PARAM_REFUSAL], + [`${API}/v2/datasets/ds-1?%6Dethod=DELETE`, METHOD_PARAM_REFUSAL], + [`${API}/v2/acts/apify~hello-world/run-sync`, SYNC_RUN_REFUSAL], + [`${API}/v2/actor-tasks/abc/run-sync-get-dataset-items`, SYNC_RUN_REFUSAL], + ])('throws InvalidParams without a request for %s, as the API tools refuse it', async (uri, refusal) => { + const { request, captured } = requestReturning(streamOf('{}'), 'application/json'); + + const error = await expectReadError(readApiResource(uri, stubApifyClient({ request }))); + + expect(error).toBeInstanceOf(InvalidParamsError); + // readApiResource does not know the session's tools, so the refusal names none. + expect(error.message).toBe(`Failed to read ${uri}: ${refusal}`); + expect(error.data).toEqual({ uri }); + expect(captured.url).toBe(''); + }); + it('requests the full URL as a stream capped at MAX_INLINE_BYTES', async () => { // The size cap is enforced by axios itself mid-consumption (streamed responses are wrapped in a // byte-counting generator since axios 1.16), so the contract under test is the request config. diff --git a/tests/unit/tools.apify_api.test.ts b/tests/unit/tools.apify_api.test.ts index 25fa2d2e8..49779fc97 100644 --- a/tests/unit/tools.apify_api.test.ts +++ b/tests/unit/tools.apify_api.test.ts @@ -89,7 +89,7 @@ const SYNC_RUN_REFUSAL = 'A GET to the synchronous run endpoints starts a paid run, as a POST does, so the API tools do not send it.'; const METHOD_PARAM_REFUSAL = - 'The API tools do not send the method query parameter: the API would take it as the HTTP method of the request.'; + 'The method query parameter is not allowed because it can turn a read request into a write request.'; /** The URL axios sends for the last request, with the query parameters added. */ function readSentUrl(): string { diff --git a/tests/unit/tools.apify_api_blocklist.test.ts b/tests/unit/tools.apify_api_blocklist.test.ts index 4dc365b5a..9a1e3b696 100644 --- a/tests/unit/tools.apify_api_blocklist.test.ts +++ b/tests/unit/tools.apify_api_blocklist.test.ts @@ -7,7 +7,7 @@ import { normalizeApiPath } from '../../src/tools/api/apify_api_path.js'; import { API_METHODS } from '../../src/tools/api/apify_api_spec.js'; const METHOD_PARAM_REFUSAL = - 'The API tools do not send the method query parameter: the API would take it as the HTTP method of the request.'; + 'The method query parameter is not allowed because it can turn a read request into a write request.'; const SYNC_RUN_REFUSAL = 'A GET to the synchronous run endpoints starts a paid run, as a POST does, so the API tools do not send it.';