Skip to content

Commit 24e8e92

Browse files
committed
feat: enforce secret overrides only from headers
1 parent 83af6bc commit 24e8e92

File tree

3 files changed

+46
-9
lines changed

3 files changed

+46
-9
lines changed

src/common/config/configOverrides.ts

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,27 @@ export function applyConfigOverrides({
3434
const overridesFromHeaders = extractConfigOverrides("header", request.headers);
3535
const overridesFromQuery = extractConfigOverrides("query", request.query);
3636

37-
// Merge overrides, with query params taking precedence
38-
const allOverrides = { ...overridesFromHeaders, ...overridesFromQuery };
37+
// Apply header overrides first
38+
for (const [key, overrideValue] of Object.entries(overridesFromHeaders)) {
39+
assertValidConfigKey(key);
40+
const meta = getConfigMeta(key);
41+
const behavior = meta?.overrideBehavior || "not-allowed";
42+
const baseValue = baseConfig[key as keyof UserConfig];
43+
const newValue = applyOverride(key, baseValue, overrideValue, behavior);
44+
(result as Record<keyof UserConfig, unknown>)[key] = newValue;
45+
}
3946

40-
// Apply each override according to its behavior
41-
for (const [key, overrideValue] of Object.entries(allOverrides)) {
47+
// Apply query overrides (with precedence), but block secret fields
48+
for (const [key, overrideValue] of Object.entries(overridesFromQuery)) {
4249
assertValidConfigKey(key);
43-
const behavior = getConfigMeta(key)?.overrideBehavior || "not-allowed";
50+
const meta = getConfigMeta(key);
51+
52+
// Prevent overriding secret fields via query params
53+
if (meta?.isSecret) {
54+
throw new Error(`Config key ${key} can only be overriden with headers.`);
55+
}
56+
57+
const behavior = meta?.overrideBehavior || "not-allowed";
4458
const baseValue = baseConfig[key as keyof UserConfig];
4559
const newValue = applyOverride(key, baseValue, overrideValue, behavior);
4660
(result as Record<keyof UserConfig, unknown>)[key] = newValue;

tests/integration/transports/configOverrides.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ describe("Config Overrides via HTTP", () => {
6969
}
7070
});
7171

72-
it("should override readOnly config via header (false to true)", async () => {
72+
it("should override readOnly config with header (false to true)", async () => {
7373
await startRunner({
7474
...defaultTestConfig,
7575
httpPort: 0,
@@ -95,7 +95,7 @@ describe("Config Overrides via HTTP", () => {
9595
expect(readTools.length).toBe(1);
9696
});
9797

98-
it("should override connectionString via header", async () => {
98+
it("should override connectionString with header", async () => {
9999
await startRunner({
100100
...defaultTestConfig,
101101
httpPort: 0,
@@ -114,7 +114,7 @@ describe("Config Overrides via HTTP", () => {
114114
});
115115

116116
describe("merge behavior", () => {
117-
it("should merge disabledTools via header", async () => {
117+
it("should merge disabledTools with header", async () => {
118118
await startRunner({
119119
...defaultTestConfig,
120120
httpPort: 0,
@@ -178,7 +178,7 @@ describe("Config Overrides via HTTP", () => {
178178
headerName: "x-mongodb-mcp-max-documents-per-query",
179179
headerValue: "1000",
180180
},
181-
])("should reject $configKey override", async ({ configKey, headerName, headerValue }) => {
181+
])("should reject $configKey with header", async ({ configKey, headerName, headerValue }) => {
182182
await startRunner({
183183
...defaultTestConfig,
184184
httpPort: 0,

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,29 @@ describe("configOverrides", () => {
227227
});
228228
});
229229

230+
describe("secret fields", () => {
231+
it("should allow overriding secret fields with headers if they have override behavior", () => {
232+
const request: RequestContext = {
233+
headers: {
234+
"x-mongodb-mcp-connection-string": "mongodb://newhost:27017/",
235+
},
236+
};
237+
const result = applyConfigOverrides({ baseConfig: baseConfig as UserConfig, request });
238+
expect(result.connectionString).toBe("mongodb://newhost:27017/");
239+
});
240+
241+
it("should not allow overriding secret fields via query params", () => {
242+
const request: RequestContext = {
243+
query: {
244+
mongodbMcpConnectionString: "mongodb://malicious.com/",
245+
},
246+
};
247+
expect(() => applyConfigOverrides({ baseConfig: baseConfig as UserConfig, request })).toThrow(
248+
"Config key connectionString can only be overriden with headers"
249+
);
250+
});
251+
});
252+
230253
describe("custom overrides", () => {
231254
it("should have certain config keys to be conditionally overridden", () => {
232255
expect(

0 commit comments

Comments
 (0)