Skip to content

Commit acb1eba

Browse files
committed
chore: remove override value print, add erroring when disabled, cleanup
1 parent b094b83 commit acb1eba

File tree

3 files changed

+26
-36
lines changed

3 files changed

+26
-36
lines changed

src/common/config/configOverrides.ts

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export function applyConfigOverrides({
2727

2828
// Only apply overrides if allowRequestOverrides is enabled
2929
if (!baseConfig.allowRequestOverrides) {
30-
return baseConfig;
30+
throw new Error("Request overrides are not enabled");
3131
}
3232

3333
const result: UserConfig = { ...baseConfig };
@@ -69,11 +69,6 @@ function extractConfigOverrides(
6969
}
7070
assertValidConfigKey(configKey);
7171

72-
const behavior = getConfigMeta(configKey)?.overrideBehavior || "not-allowed";
73-
if (behavior === "not-allowed") {
74-
throw new Error(`Config key ${configKey} is not allowed to be overridden`);
75-
}
76-
7772
const parsedValue = parseConfigValue(configKey, value);
7873
if (parsedValue !== undefined) {
7974
overrides[configKey] = parsedValue;
@@ -143,7 +138,7 @@ function applyOverride(
143138
return behavior(baseValue, overrideValue);
144139
} catch (error) {
145140
throw new Error(
146-
`Cannot apply override for ${key} from ${JSON.stringify(baseValue)} to ${JSON.stringify(overrideValue)}: ${error instanceof Error ? error.message : String(error)}`
141+
`Cannot apply override for ${key}: ${error instanceof Error ? error.message : String(error)}`
147142
);
148143
}
149144
}
@@ -155,9 +150,10 @@ function applyOverride(
155150
if (Array.isArray(baseValue) && Array.isArray(overrideValue)) {
156151
return [...(baseValue as unknown[]), ...(overrideValue as unknown[])];
157152
}
158-
throw new Error("Cannot merge non-array values, did you mean to use the 'override' behavior?");
153+
throw new Error(`Cannot merge non-array values for ${key}`);
159154

160155
case "not-allowed":
156+
throw new Error(`Config key ${key} is not allowed to be overridden`);
161157
default:
162158
return baseValue;
163159
}

tests/integration/transports/configOverrides.test.ts

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,26 +48,25 @@ describe("Config Overrides via HTTP", () => {
4848
});
4949

5050
describe("override behavior", () => {
51-
it("should not apply overrides when allowRequestOverrides is false", async () => {
51+
it("should error when allowRequestOverrides is false", async () => {
5252
await startRunner({
5353
...defaultTestConfig,
5454
httpPort: 0,
5555
readOnly: false,
5656
allowRequestOverrides: false,
5757
});
5858

59-
await connectClient({
60-
["x-mongodb-mcp-read-only"]: "true",
61-
});
62-
63-
const response = await client.listTools();
64-
65-
expect(response).toBeDefined();
66-
expect(response.tools).toBeDefined();
67-
68-
// Verify read-only mode is NOT applied - insert-many should still be available
69-
const writeTools = response.tools.filter((tool) => tool.name === "insert-many");
70-
expect(writeTools.length).toBe(1);
59+
try {
60+
await connectClient({
61+
["x-mongodb-mcp-read-only"]: "true",
62+
});
63+
expect.fail("Expected an error to be thrown");
64+
} catch (error) {
65+
if (!(error instanceof Error)) {
66+
throw new Error("Expected an error to be thrown");
67+
}
68+
expect(error.message).toContain("Request overrides are not enabled");
69+
}
7170
});
7271

7372
it("should override readOnly config via header (false to true)", async () => {
@@ -376,9 +375,7 @@ describe("Config Overrides via HTTP", () => {
376375
throw new Error("Expected an error to be thrown");
377376
}
378377
expect(error.message).toContain("Error POSTing to endpoint (HTTP 400)");
379-
expect(error.message).toContain(
380-
`Cannot apply override for readOnly from true to false: Can only set to true`
381-
);
378+
expect(error.message).toContain(`Cannot apply override for readOnly: Can only set to true`);
382379
}
383380
});
384381
});

tests/unit/common/config/configOverrides.test.ts

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -80,10 +80,9 @@ describe("configOverrides", () => {
8080
...baseConfig,
8181
allowRequestOverrides: false,
8282
} as UserConfig;
83-
const result = applyConfigOverrides({ baseConfig: configWithOverridesDisabled, request });
84-
// Config should remain unchanged
85-
expect(result.readOnly).toBe(false);
86-
expect(result.idleTimeoutMs).toBe(600_000);
83+
expect(() => applyConfigOverrides({ baseConfig: configWithOverridesDisabled, request })).to.throw(
84+
"Request overrides are not enabled"
85+
);
8786
});
8887

8988
it("should apply overrides when allowRequestOverrides is true", () => {
@@ -107,9 +106,9 @@ describe("configOverrides", () => {
107106
};
108107
// eslint-disable-next-line @typescript-eslint/no-unused-vars
109108
const { allowRequestOverrides, ...configWithoutOverridesFlag } = baseConfig;
110-
const result = applyConfigOverrides({ baseConfig: configWithoutOverridesFlag as UserConfig, request });
111-
// Should not apply overrides since the default is false
112-
expect(result.readOnly).toBe(false);
109+
expect(() =>
110+
applyConfigOverrides({ baseConfig: configWithoutOverridesFlag as UserConfig, request })
111+
).to.throw("Request overrides are not enabled");
113112
});
114113
});
115114

@@ -254,7 +253,7 @@ describe("configOverrides", () => {
254253
const request: RequestContext = { headers: { "x-mongodb-mcp-read-only": "false" } };
255254
expect(() =>
256255
applyConfigOverrides({ baseConfig: { ...baseConfig, readOnly: true } as UserConfig, request })
257-
).toThrow("Cannot apply override for readOnly from true to false: Can only set to true");
256+
).toThrow("Cannot apply override for readOnly: Can only set to true");
258257
});
259258

260259
it("should allow indexCheck override from false to true", () => {
@@ -270,7 +269,7 @@ describe("configOverrides", () => {
270269
const request: RequestContext = { headers: { "x-mongodb-mcp-index-check": "false" } };
271270
expect(() =>
272271
applyConfigOverrides({ baseConfig: { ...baseConfig, indexCheck: true } as UserConfig, request })
273-
).toThrow("Cannot apply override for indexCheck from true to false: Can only set to true");
272+
).toThrow("Cannot apply override for indexCheck: Can only set to true");
274273
});
275274

276275
it("should allow disableEmbeddingsValidation override from true to false", () => {
@@ -289,9 +288,7 @@ describe("configOverrides", () => {
289288
baseConfig: { ...baseConfig, disableEmbeddingsValidation: false } as UserConfig,
290289
request,
291290
})
292-
).toThrow(
293-
"Cannot apply override for disableEmbeddingsValidation from false to true: Can only set to false"
294-
);
291+
).toThrow("Cannot apply override for disableEmbeddingsValidation: Can only set to false");
295292
});
296293
});
297294

0 commit comments

Comments
 (0)