Skip to content

Commit e71e2ba

Browse files
authored
fix: add untrusted data wrapper to the export resource MCP-197 (#550)
1 parent 02fe6a2 commit e71e2ba

File tree

4 files changed

+82
-53
lines changed

4 files changed

+82
-53
lines changed

src/common/exportsManager.ts

Lines changed: 42 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ interface CommonExportData {
2727
interface ReadyExport extends CommonExportData {
2828
exportStatus: "ready";
2929
exportCreatedAt: number;
30+
docsTransformed: number;
3031
}
3132

3233
interface InProgressExport extends CommonExportData {
@@ -124,7 +125,7 @@ export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
124125
}
125126
}
126127

127-
public async readExport(exportName: string): Promise<string> {
128+
public async readExport(exportName: string): Promise<{ content: string; docsTransformed: number }> {
128129
try {
129130
this.assertIsNotShuttingDown();
130131
exportName = decodeAndNormalize(exportName);
@@ -137,9 +138,12 @@ export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
137138
throw new Error("Requested export is still being generated. Try again later.");
138139
}
139140

140-
const { exportPath } = exportHandle;
141+
const { exportPath, docsTransformed } = exportHandle;
141142

142-
return fs.readFile(exportPath, { encoding: "utf8", signal: this.shutdownController.signal });
143+
return {
144+
content: await fs.readFile(exportPath, { encoding: "utf8", signal: this.shutdownController.signal }),
145+
docsTransformed,
146+
};
143147
} catch (error) {
144148
this.logger.error({
145149
id: LogId.exportReadError,
@@ -202,17 +206,15 @@ export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
202206
}): Promise<void> {
203207
try {
204208
let pipeSuccessful = false;
209+
let docsTransformed = 0;
205210
try {
206211
await fs.mkdir(this.exportsDirectoryPath, { recursive: true });
207212
const outputStream = createWriteStream(inProgressExport.exportPath);
208-
await pipeline(
209-
[
210-
input.stream(),
211-
this.docToEJSONStream(this.getEJSONOptionsForFormat(jsonExportFormat)),
212-
outputStream,
213-
],
214-
{ signal: this.shutdownController.signal }
215-
);
213+
const ejsonTransform = this.docToEJSONStream(this.getEJSONOptionsForFormat(jsonExportFormat));
214+
await pipeline([input.stream(), ejsonTransform, outputStream], {
215+
signal: this.shutdownController.signal,
216+
});
217+
docsTransformed = ejsonTransform.docsTransformed;
216218
pipeSuccessful = true;
217219
} catch (error) {
218220
// If the pipeline errors out then we might end up with
@@ -231,6 +233,7 @@ export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
231233
...inProgressExport,
232234
exportCreatedAt: Date.now(),
233235
exportStatus: "ready",
236+
docsTransformed,
234237
};
235238
this.emit("export-available", inProgressExport.exportURI);
236239
}
@@ -256,33 +259,39 @@ export class ExportsManager extends EventEmitter<ExportsManagerEvents> {
256259
}
257260
}
258261

259-
private docToEJSONStream(ejsonOptions: EJSONOptions | undefined): Transform {
262+
private docToEJSONStream(ejsonOptions: EJSONOptions | undefined): Transform & { docsTransformed: number } {
260263
let docsTransformed = 0;
261-
return new Transform({
262-
objectMode: true,
263-
transform(chunk: unknown, encoding, callback): void {
264-
try {
265-
const doc = EJSON.stringify(chunk, undefined, undefined, ejsonOptions);
264+
const result = Object.assign(
265+
new Transform({
266+
objectMode: true,
267+
transform(chunk: unknown, encoding, callback): void {
268+
try {
269+
const doc = EJSON.stringify(chunk, undefined, undefined, ejsonOptions);
270+
if (docsTransformed === 0) {
271+
this.push("[" + doc);
272+
} else {
273+
this.push(",\n" + doc);
274+
}
275+
docsTransformed++;
276+
callback();
277+
} catch (err) {
278+
callback(err as Error);
279+
}
280+
},
281+
flush(callback): void {
266282
if (docsTransformed === 0) {
267-
this.push("[" + doc);
283+
this.push("[]");
268284
} else {
269-
this.push(",\n" + doc);
285+
this.push("]");
270286
}
271-
docsTransformed++;
287+
result.docsTransformed = docsTransformed;
272288
callback();
273-
} catch (err) {
274-
callback(err as Error);
275-
}
276-
},
277-
flush(callback): void {
278-
if (docsTransformed === 0) {
279-
this.push("[]");
280-
} else {
281-
this.push("]");
282-
}
283-
callback();
284-
},
285-
});
289+
},
290+
}),
291+
{ docsTransformed }
292+
);
293+
294+
return result;
286295
}
287296

288297
private async cleanupExpiredExports(): Promise<void> {

src/resources/common/exportedData.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { ResourceTemplate } from "@modelcontextprotocol/sdk/server/mcp.js";
77
import type { Server } from "../../server.js";
88
import { LogId } from "../../common/logger.js";
99
import type { Session } from "../../common/session.js";
10+
import { formatUntrustedData } from "../../tools/tool.js";
1011

1112
export class ExportedData {
1213
private readonly name = "exported-data";
@@ -95,13 +96,17 @@ export class ExportedData {
9596
throw new Error("Cannot retrieve exported data, exportName not provided.");
9697
}
9798

98-
const content = await this.session.exportsManager.readExport(exportName);
99+
const { content, docsTransformed } = await this.session.exportsManager.readExport(exportName);
100+
101+
const text = formatUntrustedData(`The exported data contains ${docsTransformed} documents.`, content)
102+
.map((t) => t.text)
103+
.join("\n");
99104

100105
return {
101106
contents: [
102107
{
103108
uri: url.href,
104-
text: content,
109+
text,
105110
mimeType: "application/json",
106111
},
107112
],

tests/integration/resources/exportedData.test.ts

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import path from "path";
22
import fs from "fs/promises";
3-
import { Long } from "bson";
3+
import { EJSON, Long, ObjectId } from "bson";
44
import { describe, expect, it, beforeEach, afterAll } from "vitest";
55
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
6-
import { defaultTestConfig, resourceChangedNotification, timeout } from "../helpers.js";
6+
import { defaultTestConfig, getDataFromUntrustedContent, resourceChangedNotification, timeout } from "../helpers.js";
77
import { describeWithMongoDB } from "../tools/mongodb/mongodbHelpers.js";
88
import { contentWithResourceURILink } from "../tools/mongodb/read/export.test.js";
99
import type { UserConfig } from "../../../src/lib.js";
@@ -18,15 +18,17 @@ const userConfig: UserConfig = {
1818
describeWithMongoDB(
1919
"exported-data resource",
2020
(integration) => {
21+
let docs: { _id: ObjectId; name: string; longNumber?: Long; bigInt?: Long }[];
22+
let collection: string;
23+
2124
beforeEach(async () => {
2225
const mongoClient = integration.mongoClient();
23-
await mongoClient
24-
.db("db")
25-
.collection("coll")
26-
.insertMany([
27-
{ name: "foo", longNumber: new Long(1234) },
28-
{ name: "bar", bigInt: new Long(123412341234) },
29-
]);
26+
collection = new ObjectId().toString();
27+
docs = [
28+
{ name: "foo", longNumber: new Long(1234), _id: new ObjectId() },
29+
{ name: "bar", bigInt: new Long(123412341234), _id: new ObjectId() },
30+
];
31+
await mongoClient.db("db").collection(collection).insertMany(docs);
3032
});
3133

3234
afterAll(async () => {
@@ -67,7 +69,7 @@ describeWithMongoDB(
6769
name: "export",
6870
arguments: {
6971
database: "db",
70-
collection: "coll",
72+
collection,
7173
exportTitle: "Export for db.coll",
7274
exportTarget: [{ name: "find", arguments: {} }],
7375
},
@@ -106,7 +108,7 @@ describeWithMongoDB(
106108
name: "export",
107109
arguments: {
108110
database: "db",
109-
collection: "coll",
111+
collection,
110112
exportTitle: "Export for db.coll",
111113
exportTarget: [{ name: "find", arguments: {} }],
112114
},
@@ -125,7 +127,16 @@ describeWithMongoDB(
125127
});
126128
expect(response.isError).toBeFalsy();
127129
expect(response.contents[0]?.mimeType).toEqual("application/json");
128-
expect(response.contents[0]?.text).toContain("foo");
130+
131+
expect(response.contents[0]?.text).toContain(`The exported data contains ${docs.length} documents.`);
132+
expect(response.contents[0]?.text).toContain("<untrusted-user-data");
133+
const exportContent = getDataFromUntrustedContent((response.contents[0]?.text as string) || "");
134+
const exportedDocs = EJSON.parse(exportContent) as { name: string; _id: ObjectId }[];
135+
const expectedDocs = docs as unknown as { name: string; _id: ObjectId }[];
136+
expect(exportedDocs[0]?.name).toEqual(expectedDocs[0]?.name);
137+
expect(exportedDocs[0]?._id).toEqual(expectedDocs[0]?._id);
138+
expect(exportedDocs[1]?.name).toEqual(expectedDocs[1]?.name);
139+
expect(exportedDocs[1]?._id).toEqual(expectedDocs[1]?._id);
129140
});
130141

131142
it("should be able to autocomplete the resource", async () => {
@@ -134,7 +145,7 @@ describeWithMongoDB(
134145
name: "export",
135146
arguments: {
136147
database: "big",
137-
collection: "coll",
148+
collection,
138149
exportTitle: "Export for big.coll",
139150
exportTarget: [{ name: "find", arguments: {} }],
140151
},

tests/unit/common/exportsManager.test.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,9 @@ describe("ExportsManager unit test", () => {
235235
jsonExportFormat: "relaxed",
236236
});
237237
await exportAvailableNotifier;
238-
expect(await manager.readExport(exportName)).toEqual("[]");
238+
const { content, docsTransformed } = await manager.readExport(exportName);
239+
expect(content).toEqual("[]");
240+
expect(docsTransformed).toEqual(0);
239241
});
240242

241243
it("should handle encoded name", async () => {
@@ -249,7 +251,9 @@ describe("ExportsManager unit test", () => {
249251
jsonExportFormat: "relaxed",
250252
});
251253
await exportAvailableNotifier;
252-
expect(await manager.readExport(encodeURIComponent(exportName))).toEqual("[]");
254+
const { content, docsTransformed } = await manager.readExport(encodeURIComponent(exportName));
255+
expect(content).toEqual("[]");
256+
expect(docsTransformed).toEqual(0);
253257
});
254258
});
255259

@@ -332,7 +336,7 @@ describe("ExportsManager unit test", () => {
332336
expect(emitSpy).toHaveBeenCalledWith("export-available", exportURI);
333337

334338
// Exports relaxed json
335-
const jsonData = JSON.parse(await manager.readExport(exportName)) as unknown[];
339+
const jsonData = JSON.parse((await manager.readExport(exportName)).content) as unknown[];
336340
expect(jsonData).toEqual([]);
337341
});
338342
});
@@ -366,7 +370,7 @@ describe("ExportsManager unit test", () => {
366370
expect(emitSpy).toHaveBeenCalledWith("export-available", `exported-data://${expectedExportName}`);
367371

368372
// Exports relaxed json
369-
const jsonData = JSON.parse(await manager.readExport(expectedExportName)) as unknown[];
373+
const jsonData = JSON.parse((await manager.readExport(expectedExportName)).content) as unknown[];
370374
expect(jsonData).toContainEqual(expect.objectContaining({ name: "foo", longNumber: 12 }));
371375
expect(jsonData).toContainEqual(expect.objectContaining({ name: "bar", longNumber: 123456 }));
372376
});
@@ -401,7 +405,7 @@ describe("ExportsManager unit test", () => {
401405
expect(emitSpy).toHaveBeenCalledWith("export-available", `exported-data://${expectedExportName}`);
402406

403407
// Exports relaxed json
404-
const jsonData = JSON.parse(await manager.readExport(expectedExportName)) as unknown[];
408+
const jsonData = JSON.parse((await manager.readExport(expectedExportName)).content) as unknown[];
405409
expect(jsonData).toContainEqual(
406410
expect.objectContaining({ name: "foo", longNumber: { $numberLong: "12" } })
407411
);

0 commit comments

Comments
 (0)