Skip to content

Commit 126852a

Browse files
authored
Merge branch 'main' into MCP-188
2 parents e24937a + ce02189 commit 126852a

File tree

4 files changed

+98
-29
lines changed

4 files changed

+98
-29
lines changed

src/tools/mongodb/metadata/logs.ts

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { CallToolResult } from "@modelcontextprotocol/sdk/types.js";
22
import { MongoDBToolBase } from "../mongodbTool.js";
3-
import type { ToolArgs, OperationType } from "../../tool.js";
3+
import { type ToolArgs, type OperationType, formatUntrustedData } from "../../tool.js";
44
import { z } from "zod";
55

66
export class LogsTool extends MongoDBToolBase {
@@ -33,23 +33,16 @@ export class LogsTool extends MongoDBToolBase {
3333
getLog: type,
3434
});
3535

36-
const logs = (result.log as string[]).slice(0, limit);
36+
// Trim ending newlines so that when we join the logs we don't insert empty lines
37+
// between messages.
38+
const logs = (result.log as string[]).slice(0, limit).map((l) => l.trimEnd());
3739

40+
let message = `Found: ${result.totalLinesWritten} messages`;
41+
if (result.totalLinesWritten > limit) {
42+
message += ` (showing only the first ${limit})`;
43+
}
3844
return {
39-
content: [
40-
{
41-
text: `Found: ${result.totalLinesWritten} messages`,
42-
type: "text",
43-
},
44-
45-
...logs.map(
46-
(log) =>
47-
({
48-
text: log,
49-
type: "text",
50-
}) as const
51-
),
52-
],
45+
content: formatUntrustedData(message, logs.join("\n")),
5346
};
5447
}
5548
}

src/tools/mongodb/read/aggregate.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -49,16 +49,23 @@ export class AggregateTool extends MongoDBToolBase {
4949
}
5050

5151
private assertOnlyUsesPermittedStages(pipeline: Record<string, unknown>[]): void {
52-
if (!this.config.readOnly) {
52+
const writeOperations: OperationType[] = ["update", "create", "delete"];
53+
let writeStageForbiddenError = "";
54+
55+
if (this.config.readOnly) {
56+
writeStageForbiddenError = "In readOnly mode you can not run pipelines with $out or $merge stages.";
57+
} else if (this.config.disabledTools.some((t) => writeOperations.includes(t as OperationType))) {
58+
writeStageForbiddenError =
59+
"When 'create', 'update', or 'delete' operations are disabled, you can not run pipelines with $out or $merge stages.";
60+
}
61+
62+
if (!writeStageForbiddenError) {
5363
return;
5464
}
5565

5666
for (const stage of pipeline) {
5767
if (stage.$out || stage.$merge) {
58-
throw new MongoDBError(
59-
ErrorCodes.ForbiddenWriteOperation,
60-
"In readOnly mode you can not run pipelines with $out or $merge stages."
61-
);
68+
throw new MongoDBError(ErrorCodes.ForbiddenWriteOperation, writeStageForbiddenError);
6269
}
6370
}
6471
}

tests/integration/tools/mongodb/metadata/logs.test.ts

Lines changed: 35 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { expect, it } from "vitest";
2-
import { validateToolMetadata, validateThrowsForInvalidArguments, getResponseElements } from "../../../helpers.js";
2+
import {
3+
validateToolMetadata,
4+
validateThrowsForInvalidArguments,
5+
getResponseElements,
6+
getDataFromUntrustedContent,
7+
} from "../../../helpers.js";
38
import { describeWithMongoDB, validateAutoConnectBehavior } from "../mongodbHelpers.js";
49

510
describeWithMongoDB("logs tool", (integration) => {
@@ -36,13 +41,27 @@ describeWithMongoDB("logs tool", (integration) => {
3641

3742
const elements = getResponseElements(response);
3843

44+
expect(elements).toHaveLength(2);
45+
expect(elements[1]?.text).toContain("<untrusted-user-data-");
46+
47+
const logs = getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n");
3948
// Default limit is 50
40-
expect(elements.length).toBeLessThanOrEqual(51);
49+
expect(logs.length).toBeLessThanOrEqual(50);
50+
51+
// Expect at least one log entry
52+
expect(logs.length).toBeGreaterThan(1);
53+
4154
expect(elements[0]?.text).toMatch(/Found: \d+ messages/);
55+
const totalMessages = parseInt(elements[0]?.text.match(/Found: (\d+) messages/)?.[1] ?? "0", 10);
56+
expect(totalMessages).toBeGreaterThanOrEqual(logs.length);
57+
58+
if (totalMessages > logs.length) {
59+
expect(elements[0]?.text).toContain(`(showing only the first ${logs.length})`);
60+
}
4261

43-
for (let i = 1; i < elements.length; i++) {
62+
for (const message of logs) {
4463
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
45-
const log = JSON.parse(elements[i]?.text ?? "{}");
64+
const log = JSON.parse(message ?? "{}");
4665
expect(log).toHaveProperty("t");
4766
expect(log).toHaveProperty("msg");
4867
}
@@ -58,9 +77,18 @@ describeWithMongoDB("logs tool", (integration) => {
5877
});
5978

6079
const elements = getResponseElements(response);
61-
expect(elements.length).toBeLessThanOrEqual(51);
62-
for (let i = 1; i < elements.length; i++) {
63-
const log = JSON.parse(elements[i]?.text ?? "{}") as { tags: string[] };
80+
expect(elements).toHaveLength(2);
81+
expect(elements[1]?.text).toContain("<untrusted-user-data-");
82+
83+
const logs = getDataFromUntrustedContent(elements[1]?.text ?? "").split("\n");
84+
// Default limit is 50
85+
expect(logs.length).toBeLessThanOrEqual(50);
86+
87+
// Expect at least one log entry
88+
expect(logs.length).toBeGreaterThan(1);
89+
90+
for (const message of logs) {
91+
const log = JSON.parse(message ?? "{}") as { tags: string[] };
6492
expect(log).toHaveProperty("t");
6593
expect(log).toHaveProperty("msg");
6694
expect(log).toHaveProperty("tags");

tests/integration/tools/mongodb/read/aggregate.test.ts

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,15 @@ import {
44
validateThrowsForInvalidArguments,
55
getResponseContent,
66
} from "../../../helpers.js";
7-
import { expect, it } from "vitest";
7+
import { expect, it, afterEach } from "vitest";
88
import { describeWithMongoDB, getDocsFromUntrustedContent, validateAutoConnectBehavior } from "../mongodbHelpers.js";
99

1010
describeWithMongoDB("aggregate tool", (integration) => {
11+
afterEach(() => {
12+
integration.mcpServer().userConfig.readOnly = false;
13+
integration.mcpServer().userConfig.disabledTools = [];
14+
});
15+
1116
validateToolMetadata(integration, "aggregate", "Run an aggregation against a MongoDB collection", [
1217
...databaseCollectionParameters,
1318
{
@@ -129,6 +134,42 @@ describeWithMongoDB("aggregate tool", (integration) => {
129134
);
130135
});
131136

137+
for (const disabledOpType of ["create", "update", "delete"] as const) {
138+
it(`can not run $out stages when ${disabledOpType} operation is disabled`, async () => {
139+
await integration.connectMcpClient();
140+
integration.mcpServer().userConfig.disabledTools = [disabledOpType];
141+
const response = await integration.mcpClient().callTool({
142+
name: "aggregate",
143+
arguments: {
144+
database: integration.randomDbName(),
145+
collection: "people",
146+
pipeline: [{ $out: "outpeople" }],
147+
},
148+
});
149+
const content = getResponseContent(response);
150+
expect(content).toEqual(
151+
"Error running aggregate: When 'create', 'update', or 'delete' operations are disabled, you can not run pipelines with $out or $merge stages."
152+
);
153+
});
154+
155+
it(`can not run $merge stages when ${disabledOpType} operation is disabled`, async () => {
156+
await integration.connectMcpClient();
157+
integration.mcpServer().userConfig.disabledTools = [disabledOpType];
158+
const response = await integration.mcpClient().callTool({
159+
name: "aggregate",
160+
arguments: {
161+
database: integration.randomDbName(),
162+
collection: "people",
163+
pipeline: [{ $merge: "outpeople" }],
164+
},
165+
});
166+
const content = getResponseContent(response);
167+
expect(content).toEqual(
168+
"Error running aggregate: When 'create', 'update', or 'delete' operations are disabled, you can not run pipelines with $out or $merge stages."
169+
);
170+
});
171+
}
172+
132173
validateAutoConnectBehavior(integration, "aggregate", () => {
133174
return {
134175
args: {

0 commit comments

Comments
 (0)