Skip to content

Commit fe436c7

Browse files
committed
validation: fail closed on unknown declaration manifests (#3390)
1 parent a83b145 commit fe436c7

10 files changed

Lines changed: 726 additions & 152 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { describe, expect, it } from "vitest";
2+
import { validateDeclarationManifest } from "../server/declaration-manifest.js";
3+
4+
const knownDeclarations = {
5+
tools: new Set<string>(["echo", "get-sum"]),
6+
resources: new Set<string>(["resource-templates"]),
7+
prompts: new Set<string>(["simple-prompt"]),
8+
};
9+
10+
describe("declaration manifest validation", () => {
11+
it("accepts valid declaration manifest", () => {
12+
expect(
13+
validateDeclarationManifest(
14+
JSON.stringify({
15+
tools: ["echo"],
16+
resources: ["resource-templates"],
17+
prompts: ["simple-prompt"],
18+
}),
19+
knownDeclarations
20+
)
21+
).toBeTruthy();
22+
});
23+
24+
it("fails on unknown declaration section", () => {
25+
expect(() =>
26+
validateDeclarationManifest(
27+
JSON.stringify({ unknown: ["value"] }),
28+
knownDeclarations
29+
)
30+
).toThrow("manifest.unknown: unknown declaration section");
31+
});
32+
33+
it("fails on unknown tool/resource/prompt declaration names", () => {
34+
expect(() =>
35+
validateDeclarationManifest(
36+
JSON.stringify({
37+
tools: ["unknown-tool"],
38+
resources: ["unknown-resource"],
39+
prompts: ["unknown-prompt"],
40+
}),
41+
knownDeclarations
42+
)
43+
).toThrow("Declaration manifest validation failed");
44+
});
45+
});
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
type DeclarationSection = "tools" | "resources" | "prompts";
2+
3+
export type DeclarationManifest = Partial<Record<DeclarationSection, string[]>>;
4+
5+
const DECLARATION_SECTIONS: DeclarationSection[] = [
6+
"tools",
7+
"resources",
8+
"prompts",
9+
];
10+
11+
export function validateDeclarationManifest(
12+
manifestRaw: string | undefined,
13+
knownDeclarations: Record<DeclarationSection, ReadonlySet<string>>
14+
): DeclarationManifest | null {
15+
if (!manifestRaw) {
16+
return null;
17+
}
18+
19+
let parsed: unknown;
20+
try {
21+
parsed = JSON.parse(manifestRaw);
22+
} catch (error) {
23+
throw new Error(
24+
`Invalid declaration manifest JSON: ${(error as Error).message}`
25+
);
26+
}
27+
28+
if (parsed === null || Array.isArray(parsed) || typeof parsed !== "object") {
29+
throw new Error("Declaration manifest must be a JSON object.");
30+
}
31+
32+
const manifest = parsed as Record<string, unknown>;
33+
const errors: string[] = [];
34+
35+
for (const key of Object.keys(manifest)) {
36+
if (!DECLARATION_SECTIONS.includes(key as DeclarationSection)) {
37+
errors.push(`manifest.${key}: unknown declaration section`);
38+
}
39+
}
40+
41+
for (const section of DECLARATION_SECTIONS) {
42+
const value = manifest[section];
43+
if (value === undefined) {
44+
continue;
45+
}
46+
47+
if (!Array.isArray(value)) {
48+
errors.push(`manifest.${section}: expected an array of strings`);
49+
continue;
50+
}
51+
52+
value.forEach((entry, index) => {
53+
if (typeof entry !== "string") {
54+
errors.push(
55+
`manifest.${section}[${index}]: expected string declaration name`
56+
);
57+
return;
58+
}
59+
if (!knownDeclarations[section].has(entry)) {
60+
errors.push(
61+
`manifest.${section}[${index}]: unknown declaration '${entry}'`
62+
);
63+
}
64+
});
65+
}
66+
67+
if (errors.length > 0) {
68+
throw new Error(
69+
`Declaration manifest validation failed:\n${errors.join("\n")}`
70+
);
71+
}
72+
73+
return manifest as DeclarationManifest;
74+
}

src/everything/server/index.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,37 @@ import { registerResources, readInstructions } from "../resources/index.js";
1212
import { registerPrompts } from "../prompts/index.js";
1313
import { stopSimulatedLogging } from "./logging.js";
1414
import { syncRoots } from "./roots.js";
15+
import { validateDeclarationManifest } from "./declaration-manifest.js";
16+
17+
const KNOWN_EVERYTHING_DECLARATIONS = {
18+
tools: new Set<string>([
19+
"echo",
20+
"get-annotated-message",
21+
"get-env",
22+
"get-resource-links",
23+
"get-resource-reference",
24+
"get-roots-list",
25+
"get-structured-content",
26+
"get-sum",
27+
"get-tiny-image",
28+
"gzip-file-as-resource",
29+
"simulate-research-query",
30+
"toggle-simulated-logging",
31+
"toggle-subscriber-updates",
32+
"trigger-elicitation-request",
33+
"trigger-elicitation-request-async",
34+
"trigger-long-running-operation",
35+
"trigger-sampling-request",
36+
"trigger-sampling-request-async",
37+
]),
38+
resources: new Set<string>(["resource-templates", "file-resources"]),
39+
prompts: new Set<string>([
40+
"simple-prompt",
41+
"args-prompt",
42+
"completable-prompt",
43+
"resource-prompt",
44+
]),
45+
};
1546

1647
// Server Factory response
1748
export type ServerFactoryResponse = {
@@ -33,6 +64,11 @@ export type ServerFactoryResponse = {
3364
* - `cleanup` {Function}: Function to perform cleanup operations for a closing session.
3465
*/
3566
export const createServer: () => ServerFactoryResponse = () => {
67+
validateDeclarationManifest(
68+
process.env.MCP_DECLARATION_MANIFEST,
69+
KNOWN_EVERYTHING_DECLARATIONS
70+
);
71+
3672
// Read the server instructions
3773
const instructions = readInstructions();
3874

src/fetch/src/mcp_server_fetch/server.py

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import json
2+
import os
13
from typing import Annotated, Tuple
24
from urllib.parse import urlparse, urlunparse
35

@@ -24,6 +26,61 @@
2426
DEFAULT_USER_AGENT_MANUAL = "ModelContextProtocol/1.0 (User-Specified; +https://github.com/modelcontextprotocol/servers)"
2527

2628

29+
def validate_declaration_manifest(
30+
manifest_raw: str | None,
31+
*,
32+
known_tools: set[str],
33+
known_resources: set[str],
34+
known_prompts: set[str],
35+
) -> None:
36+
"""Validate declaration manifest and fail closed on unknown entries."""
37+
if not manifest_raw:
38+
return
39+
40+
try:
41+
manifest = json.loads(manifest_raw)
42+
except json.JSONDecodeError as exc:
43+
raise ValueError(f"Invalid declaration manifest JSON: {exc}") from exc
44+
45+
if not isinstance(manifest, dict):
46+
raise ValueError("Declaration manifest must be a JSON object.")
47+
48+
sections = {
49+
"tools": known_tools,
50+
"resources": known_resources,
51+
"prompts": known_prompts,
52+
}
53+
errors: list[str] = []
54+
55+
for key in manifest:
56+
if key not in sections:
57+
errors.append(f"manifest.{key}: unknown declaration section")
58+
59+
for section, known in sections.items():
60+
if section not in manifest:
61+
continue
62+
values = manifest[section]
63+
if not isinstance(values, list):
64+
errors.append(f"manifest.{section}: expected an array of strings")
65+
continue
66+
67+
for index, entry in enumerate(values):
68+
if not isinstance(entry, str):
69+
errors.append(
70+
f"manifest.{section}[{index}]: expected string declaration name"
71+
)
72+
continue
73+
if entry not in known:
74+
errors.append(
75+
f"manifest.{section}[{index}]: unknown declaration '{entry}'"
76+
)
77+
78+
if errors:
79+
raise ValueError(
80+
"Declaration manifest validation failed:\n" + "\n".join(errors)
81+
)
82+
83+
2784
def extract_content_from_html(html: str) -> str:
2885
"""Extract and convert HTML content to Markdown format.
2986
@@ -190,6 +247,13 @@ async def serve(
190247
ignore_robots_txt: Whether to ignore robots.txt restrictions
191248
proxy_url: Optional proxy URL to use for requests
192249
"""
250+
validate_declaration_manifest(
251+
os.getenv("MCP_DECLARATION_MANIFEST"),
252+
known_tools={"fetch"},
253+
known_resources=set(),
254+
known_prompts={"fetch"},
255+
)
256+
193257
server = Server("mcp-fetch")
194258
user_agent_autonomous = custom_user_agent or DEFAULT_USER_AGENT_AUTONOMOUS
195259
user_agent_manual = custom_user_agent or DEFAULT_USER_AGENT_MANUAL

src/fetch/tests/test_server.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
check_may_autonomously_fetch_url,
1111
fetch_url,
1212
DEFAULT_USER_AGENT_AUTONOMOUS,
13+
validate_declaration_manifest,
1314
)
1415

1516

@@ -219,6 +220,34 @@ async def test_fetch_html_page(self):
219220
assert isinstance(content, str)
220221
assert prefix == ""
221222

223+
224+
class TestDeclarationManifestValidation:
225+
def test_accepts_valid_manifest(self):
226+
validate_declaration_manifest(
227+
'{"tools":["fetch"],"prompts":["fetch"],"resources":[]}',
228+
known_tools={"fetch"},
229+
known_resources=set(),
230+
known_prompts={"fetch"},
231+
)
232+
233+
def test_rejects_unknown_declaration_name(self):
234+
with pytest.raises(ValueError, match="manifest.tools\\[0\\]: unknown declaration 'invalid'"):
235+
validate_declaration_manifest(
236+
'{"tools":["invalid"]}',
237+
known_tools={"fetch"},
238+
known_resources=set(),
239+
known_prompts={"fetch"},
240+
)
241+
242+
def test_rejects_unknown_declaration_section(self):
243+
with pytest.raises(ValueError, match="manifest.bad: unknown declaration section"):
244+
validate_declaration_manifest(
245+
'{"bad":["fetch"]}',
246+
known_tools={"fetch"},
247+
known_resources=set(),
248+
known_prompts={"fetch"},
249+
)
250+
222251
@pytest.mark.asyncio
223252
async def test_fetch_html_page_raw(self):
224253
"""Test fetching an HTML page with raw=True returns original HTML."""
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import { describe, expect, it } from "vitest";
2+
import { validateDeclarationManifest } from "../declaration-manifest.js";
3+
4+
const knownDeclarations = {
5+
tools: new Set<string>(["read_text_file", "write_file"]),
6+
resources: new Set<string>(),
7+
prompts: new Set<string>(),
8+
};
9+
10+
describe("filesystem declaration manifest validation", () => {
11+
it("accepts valid tool declarations", () => {
12+
expect(() =>
13+
validateDeclarationManifest(
14+
JSON.stringify({ tools: ["read_text_file"] }),
15+
knownDeclarations
16+
)
17+
).not.toThrow();
18+
});
19+
20+
it("fails on unknown declaration section", () => {
21+
expect(() =>
22+
validateDeclarationManifest(
23+
JSON.stringify({ unknown: ["x"] }),
24+
knownDeclarations
25+
)
26+
).toThrow("manifest.unknown: unknown declaration section");
27+
});
28+
29+
it("fails on unknown declaration name", () => {
30+
expect(() =>
31+
validateDeclarationManifest(
32+
JSON.stringify({ tools: ["not-a-tool"] }),
33+
knownDeclarations
34+
)
35+
).toThrow("manifest.tools[0]: unknown declaration 'not-a-tool'");
36+
});
37+
});

0 commit comments

Comments
 (0)