Skip to content

Commit 1919237

Browse files
fmontesclaude
andauthored
fix(agentic-tools): return binary HTTP responses as base64 envelope (#36242)
### Proposed Changes Fixes the MCP `agentic-tools` adapter corrupting binary HTTP responses (`libs/agentic-tools/src/lib/http-client.ts`). `response.text()` UTF-8-decoded every non-JSON body, replacing invalid bytes with `U+FFFD` — so any binary file asset (images, fonts, `.ico`) pulled via MCP `api.request` came back irreversibly corrupted. The dotCMS endpoints were already correct; the bug was entirely in the adapter. * **Response parsing now branches by content-type:** * `application/json` → `.json()` * textual types (`text/*`, `application/xml`, `application/javascript`, `+json`/`+xml`, `application/x-www-form-urlencoded`) → `.text()` * everything else → `response.arrayBuffer()` returned as a base64 envelope `{ __dotcmsBinary: true, contentType, base64, byteLength }`, so the raw bytes survive `JSON.stringify` across the sandbox boundary in `execute.ts`. * **Error responses are read as text** regardless of content-type, so dotCMS HTML/text error messages are preserved (the previous error path assumed string/JSON). * **25MB size cap** on the binary path (`MAX_BINARY_RESPONSE_BYTES`), matching the existing `MAX_REMOTE_FILE_BYTES` on the upload side. * **New `responseType?: 'auto' | 'base64'`** option on `RequestOptions` to force the binary path regardless of declared content-type. * **Exported `BinaryResponseEnvelope` type + `isBinaryResponseEnvelope()` guard** so consumers can detect and base64-decode the body. ### Checklist - [x] Tests — new `http-client.spec.ts` with a real 1×1 PNG fixture asserting the round-tripped bytes are byte-exact to the source (the regression guard), plus JSON / textual / `+json` / forced-base64 / error-as-text cases. All 6 pass; lint clean. - [ ] Translations — N/A - [x] Security Implications Contemplated — size cap guards against memory exhaustion; auth/SSRF handling unchanged. ### Additional Info The fix mirrors the existing upload path (`Buffer.from(arrayBuffer).toString('base64')`). `execute.ts` was not changed — its `JSON.stringify` boundary now serializes the envelope correctly; consumers can import `isBinaryResponseEnvelope` to decode when ready. Fixes #36241 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 16b248d commit 1919237

5 files changed

Lines changed: 320 additions & 15 deletions

File tree

core-web/apps/mcp-server/src/tools/execute.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ Tips:
4343
- Use \`pick(arr, fields)\` to return only the fields you need — responses can be very large
4444
- For file uploads use \`formData\` with \`{ name, type, data }\` (base64) or \`{ name, type, url }\` (remote URL)
4545
46+
Binary responses (file assets — images, fonts, PDFs, etc.):
47+
- Endpoints that return non-text bodies (e.g. GET \`/api/v2/assets/{identifier}\` and \`/dA/{id}\`, content-type \`application/octet-stream\` or \`image/*\`) come back as an envelope: \`{ __dotcmsBinary: true, contentType, base64, byteLength }\`.
48+
- The \`base64\` field IS the raw file bytes — base64-decode it to recover the exact file. Do NOT treat it as text; the bytes are intact (not UTF-8-mangled).
49+
- JSON and textual responses (\`text/*\`, xml, js, \`+json\`/\`+xml\`) are returned as parsed objects / strings as before — only binary bodies use the envelope.
50+
4651
Block Editor (Story Block) fields:
4752
- A Story Block field stores a string. When creating or updating content via a fire endpoint, send the field value as an **HTML or Markdown string** — do NOT hand-author the ProseMirror/JSON document. dotCMS stores it as-is and converts it to the Block Editor structure when the contentlet is opened in the editor.
4853
- Example: \`{ "contentType": "Blog", "title": "My Post", "body": "<h2>Intro</h2><p>Hello <strong>world</strong>.</p>" }\` — where \`body\` is the Story Block field.

core-web/libs/agentic-tools/src/index.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
export { Executor, createExecutor } from './lib/executor';
22
export type { ExecutorOptions } from './lib/executor';
33

4-
export { createApiAdapter } from './lib/http-client';
5-
export type { ApiAdapterConfig } from './lib/http-client';
4+
export { createApiAdapter, isBinaryResponseEnvelope } from './lib/http-client';
5+
export type { ApiAdapterConfig, BinaryResponseEnvelope } from './lib/http-client';
66

77
export { createSandbox } from './lib/sandbox';
88
export type { ISandbox, SandboxFactory } from './lib/sandbox/interface';
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
import { createApiAdapter, isBinaryResponseEnvelope } from './http-client';
2+
3+
import type { Adapter, AdapterMethod } from './types';
4+
5+
/**
6+
* A real 1x1 red PNG. Its first byte is 0x89, which is not valid UTF-8 — the
7+
* exact kind of byte that `response.text()` corrupts into U+FFFD. This is the
8+
* regression fixture for the binary-response corruption bug.
9+
*/
10+
const PNG_BASE64 =
11+
'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8z8BQDwAEhQGAhKmMIQAAAABJRU5ErkJggg==';
12+
const PNG_BYTES = Buffer.from(PNG_BASE64, 'base64');
13+
14+
const CONFIG = { dotcmsUrl: 'https://example.dotcms.com', authToken: 'test-token' };
15+
16+
function getRequestMethod(adapter: Adapter): AdapterMethod {
17+
const method = adapter.methods.get('request');
18+
if (!method) {
19+
throw new Error('request method not registered');
20+
}
21+
return method;
22+
}
23+
24+
/** Build a Response-like stub backed by a fixed body buffer. */
25+
function makeResponse(
26+
body: Buffer | string,
27+
{
28+
contentType,
29+
ok = true,
30+
status = 200,
31+
statusText = 'OK',
32+
contentLength
33+
}: {
34+
contentType: string;
35+
ok?: boolean;
36+
status?: number;
37+
statusText?: string;
38+
// Override the Content-Length header independently of the actual body —
39+
// lets us simulate a server that advertises an oversized response.
40+
contentLength?: string;
41+
}
42+
): Response {
43+
const buffer = typeof body === 'string' ? Buffer.from(body, 'utf-8') : body;
44+
const headerValues: Record<string, string | null> = {
45+
'content-type': contentType,
46+
'content-length': contentLength ?? String(buffer.byteLength)
47+
};
48+
return {
49+
ok,
50+
status,
51+
statusText,
52+
headers: { get: (name: string) => headerValues[name.toLowerCase()] ?? null },
53+
json: async () => JSON.parse(buffer.toString('utf-8')),
54+
text: async () => buffer.toString('utf-8'),
55+
arrayBuffer: async () =>
56+
buffer.buffer.slice(buffer.byteOffset, buffer.byteOffset + buffer.byteLength)
57+
} as unknown as Response;
58+
}
59+
60+
describe('createApiAdapter response parsing', () => {
61+
const fetchMock = jest.fn();
62+
63+
beforeEach(() => {
64+
fetchMock.mockReset();
65+
global.fetch = fetchMock as unknown as typeof fetch;
66+
});
67+
68+
it('round-trips a binary (PNG) body as a base64 envelope without corrupting bytes', async () => {
69+
fetchMock.mockResolvedValue(makeResponse(PNG_BYTES, { contentType: 'image/png' }));
70+
71+
const adapter = createApiAdapter(CONFIG);
72+
const result = await getRequestMethod(adapter).execute({ path: '/dA/abc123' });
73+
74+
expect(isBinaryResponseEnvelope(result)).toBe(true);
75+
const envelope = result as {
76+
__dotcmsBinary: true;
77+
contentType: string;
78+
base64: string;
79+
byteLength: number;
80+
};
81+
expect(envelope.contentType).toBe('image/png');
82+
expect(envelope.byteLength).toBe(PNG_BYTES.byteLength);
83+
// The decoded bytes must be byte-exact to the source — the actual regression guard.
84+
expect(Buffer.from(envelope.base64, 'base64').equals(PNG_BYTES)).toBe(true);
85+
});
86+
87+
it('parses JSON content as an object', async () => {
88+
fetchMock.mockResolvedValue(
89+
makeResponse(JSON.stringify({ hello: 'world' }), { contentType: 'application/json' })
90+
);
91+
92+
const adapter = createApiAdapter(CONFIG);
93+
const result = await getRequestMethod(adapter).execute({ path: '/api/v1/x' });
94+
95+
expect(result).toEqual({ hello: 'world' });
96+
});
97+
98+
it('returns textual content types as strings', async () => {
99+
fetchMock.mockResolvedValue(
100+
makeResponse('<root/>', { contentType: 'application/xml; charset=utf-8' })
101+
);
102+
103+
const adapter = createApiAdapter(CONFIG);
104+
const result = await getRequestMethod(adapter).execute({ path: '/api/x.xml' });
105+
106+
expect(result).toBe('<root/>');
107+
});
108+
109+
it('treats +json content types as textual strings', async () => {
110+
fetchMock.mockResolvedValue(
111+
makeResponse('{"a":1}', { contentType: 'application/vnd.api+json' })
112+
);
113+
114+
const adapter = createApiAdapter(CONFIG);
115+
const result = await getRequestMethod(adapter).execute({ path: '/api/x' });
116+
117+
expect(result).toBe('{"a":1}');
118+
});
119+
120+
it('forces the binary path when responseType is "base64", even for JSON', async () => {
121+
fetchMock.mockResolvedValue(
122+
makeResponse(JSON.stringify({ hello: 'world' }), { contentType: 'application/json' })
123+
);
124+
125+
const adapter = createApiAdapter(CONFIG);
126+
const result = await getRequestMethod(adapter).execute({
127+
path: '/api/v1/x',
128+
responseType: 'base64'
129+
});
130+
131+
expect(isBinaryResponseEnvelope(result)).toBe(true);
132+
});
133+
134+
it('reads the error body as text regardless of content-type', async () => {
135+
fetchMock.mockResolvedValue(
136+
makeResponse('<html>Not Found</html>', {
137+
contentType: 'text/html',
138+
ok: false,
139+
status: 404,
140+
statusText: 'Not Found'
141+
})
142+
);
143+
144+
const adapter = createApiAdapter(CONFIG);
145+
await expect(getRequestMethod(adapter).execute({ path: '/dA/missing' })).rejects.toThrow(
146+
'HTTP 404 Not Found: <html>Not Found</html>'
147+
);
148+
});
149+
150+
it('rejects an oversized binary response via Content-Length before buffering', async () => {
151+
const oversized = String(26 * 1024 * 1024); // 26MB > 25MB cap
152+
const arrayBuffer = jest.fn();
153+
fetchMock.mockResolvedValue({
154+
ok: true,
155+
status: 200,
156+
statusText: 'OK',
157+
headers: {
158+
get: (name: string) =>
159+
name.toLowerCase() === 'content-type'
160+
? 'application/octet-stream'
161+
: name.toLowerCase() === 'content-length'
162+
? oversized
163+
: null
164+
},
165+
arrayBuffer
166+
} as unknown as Response);
167+
168+
const adapter = createApiAdapter(CONFIG);
169+
await expect(getRequestMethod(adapter).execute({ path: '/dA/huge' })).rejects.toThrow(
170+
'exceeds the'
171+
);
172+
// The body must never be buffered when Content-Length already exceeds the cap.
173+
expect(arrayBuffer).not.toHaveBeenCalled();
174+
});
175+
176+
describe('isBinaryResponseEnvelope', () => {
177+
it('accepts a fully-formed envelope', () => {
178+
expect(
179+
isBinaryResponseEnvelope({
180+
__dotcmsBinary: true,
181+
contentType: 'image/png',
182+
base64: 'AA==',
183+
byteLength: 1
184+
})
185+
).toBe(true);
186+
});
187+
188+
it('rejects an envelope missing contentType or byteLength', () => {
189+
expect(isBinaryResponseEnvelope({ __dotcmsBinary: true, base64: 'AA==' })).toBe(false);
190+
expect(
191+
isBinaryResponseEnvelope({
192+
__dotcmsBinary: true,
193+
base64: 'AA==',
194+
contentType: 'image/png'
195+
})
196+
).toBe(false);
197+
});
198+
199+
it('rejects non-envelope values', () => {
200+
expect(isBinaryResponseEnvelope(null)).toBe(false);
201+
expect(isBinaryResponseEnvelope('string')).toBe(false);
202+
expect(isBinaryResponseEnvelope({ hello: 'world' })).toBe(false);
203+
});
204+
});
205+
});

core-web/libs/agentic-tools/src/lib/http-client.ts

Lines changed: 106 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,12 @@ interface RequestOptions {
1616
body?: unknown;
1717
formData?: Record<string, FormDataFieldValue>;
1818
headers?: Record<string, string>;
19+
// How to decode the response body. Defaults to content-type auto-detection:
20+
// JSON content types are parsed; textual types come back as strings; everything
21+
// else (images, fonts, etc.) comes back as a base64 binary envelope so the bytes
22+
// survive the JSON.stringify boundary in the consuming sandbox. Set 'base64' to
23+
// force the binary path regardless of the declared content-type.
24+
responseType?: 'auto' | 'base64';
1925
}
2026

2127
function isFileDescriptor(value: unknown): value is FileFieldDescriptor {
@@ -35,6 +41,89 @@ const MAX_REMOTE_FILE_BYTES = 25 * 1024 * 1024; // 25 MB
3541
// Timeout (ms) for the remote fetch, so a slow/hanging URL cannot stall the host.
3642
const REMOTE_FILE_FETCH_TIMEOUT_MS = 15000;
3743

44+
// Max size (bytes) for a binary response body returned as a base64 envelope.
45+
// base64 inflates the payload ~33% and the whole thing flows through
46+
// JSON.stringify in the consuming sandbox, so large assets can blow up memory
47+
// and model context — cap it like the upload side already does.
48+
const MAX_BINARY_RESPONSE_BYTES = 25 * 1024 * 1024; // 25 MB
49+
50+
/**
51+
* Tagged envelope returned for non-textual response bodies. The raw bytes are
52+
* base64-encoded so they survive the `JSON.stringify` serialization boundary in
53+
* `execute.ts` intact — `response.text()` would corrupt any non-UTF-8 byte into
54+
* the U+FFFD replacement char. Consumers detect `__dotcmsBinary` and decode.
55+
*/
56+
export interface BinaryResponseEnvelope {
57+
__dotcmsBinary: true;
58+
contentType: string;
59+
base64: string;
60+
byteLength: number;
61+
}
62+
63+
/**
64+
* Type guard for the binary response envelope. Consumers can use this to detect
65+
* a binary body and `Buffer.from(envelope.base64, 'base64')` to recover the bytes.
66+
*/
67+
export function isBinaryResponseEnvelope(value: unknown): value is BinaryResponseEnvelope {
68+
if (typeof value !== 'object' || value === null) {
69+
return false;
70+
}
71+
const obj = value as Record<string, unknown>;
72+
return (
73+
obj.__dotcmsBinary === true &&
74+
typeof obj.base64 === 'string' &&
75+
typeof obj.contentType === 'string' &&
76+
typeof obj.byteLength === 'number'
77+
);
78+
}
79+
80+
/**
81+
* Decide whether a content-type should be decoded as text. Everything that is
82+
* not JSON (handled separately) and not in this textual set is treated as
83+
* binary and returned as a base64 envelope.
84+
*/
85+
function isTextualContentType(contentType: string): boolean {
86+
const ct = contentType.toLowerCase();
87+
return (
88+
ct.startsWith('text/') ||
89+
ct.includes('application/xml') ||
90+
ct.includes('application/javascript') ||
91+
ct.includes('application/x-www-form-urlencoded') ||
92+
ct.includes('+json') ||
93+
ct.includes('+xml')
94+
);
95+
}
96+
97+
/**
98+
* Read a response body as a base64 binary envelope, enforcing the size cap.
99+
*/
100+
async function readBinaryResponse(
101+
response: Response,
102+
contentType: string
103+
): Promise<BinaryResponseEnvelope> {
104+
// Reject early via Content-Length so we never buffer an oversized body into
105+
// memory. The header can be absent or lie, so the post-read check below stays
106+
// as the authoritative backstop.
107+
const declaredLength = Number(response.headers.get('content-length'));
108+
if (Number.isFinite(declaredLength) && declaredLength > MAX_BINARY_RESPONSE_BYTES) {
109+
throw new Error(
110+
`Binary response (${declaredLength} bytes) exceeds the ${MAX_BINARY_RESPONSE_BYTES}-byte limit`
111+
);
112+
}
113+
const buffer = await response.arrayBuffer();
114+
if (buffer.byteLength > MAX_BINARY_RESPONSE_BYTES) {
115+
throw new Error(
116+
`Binary response (${buffer.byteLength} bytes) exceeds the ${MAX_BINARY_RESPONSE_BYTES}-byte limit`
117+
);
118+
}
119+
return {
120+
__dotcmsBinary: true,
121+
contentType,
122+
base64: Buffer.from(buffer).toString('base64'),
123+
byteLength: buffer.byteLength
124+
};
125+
}
126+
38127
/**
39128
* Validates a user-supplied file URL before fetching it, to mitigate SSRF.
40129
* Sandbox code can put any string in `desc.url`, and the fetch runs on the
@@ -216,23 +305,28 @@ export function createApiAdapter(config: ApiAdapterConfig): Adapter {
216305

217306
const response = await fetch(url.toString(), fetchOptions);
218307

219-
// Parse response
308+
// Parse response.
220309
const contentType = response.headers.get('content-type') || '';
221-
let data: unknown;
222-
223-
if (contentType.includes('application/json')) {
224-
data = await response.json();
225-
} else {
226-
data = await response.text();
227-
}
228310

311+
// On error, always read the body as text regardless of the declared
312+
// content-type — dotCMS errors come back as HTML/text and we want a
313+
// readable message, not a base64 envelope of the error page.
229314
if (!response.ok) {
230-
throw new Error(
231-
`HTTP ${response.status} ${response.statusText}: ${typeof data === 'string' ? data : JSON.stringify(data)}`
232-
);
315+
const errorBody = await response.text();
316+
throw new Error(`HTTP ${response.status} ${response.statusText}: ${errorBody}`);
233317
}
234318

235-
return data;
319+
const forceBinary = options.responseType === 'base64';
320+
321+
if (!forceBinary && contentType.includes('application/json')) {
322+
return await response.json();
323+
}
324+
if (!forceBinary && isTextualContentType(contentType)) {
325+
return await response.text();
326+
}
327+
// Non-JSON, non-textual (or explicitly requested): return a base64
328+
// envelope so the raw bytes survive JSON.stringify intact.
329+
return await readBinaryResponse(response, contentType);
236330
}
237331
};
238332

core-web/libs/agentic-tools/tsconfig.lib.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,6 @@
66
"types": ["node"],
77
"resolveJsonModule": true
88
},
9-
"include": ["src/**/*.ts", "src/**/*.json"]
9+
"include": ["src/**/*.ts", "src/**/*.json"],
10+
"exclude": ["src/**/*.spec.ts", "src/**/*.test.ts"]
1011
}

0 commit comments

Comments
 (0)