feat: enhanced structural validation for mcpb validate (#57) · modelcontextprotocol/mcpb@f14863b · GitHub
Skip to content

Commit f14863b

Browse files
bryan-anthropicclaude
bryan-anthropic
and
claude
committed
feat: enhanced structural validation for mcpb validate (#57)
Add three pre-pack validators to catch common runtime failures early: - validateEntryPoint: checks file exists, extension matches server type, binary has executable bit (Unix) - validateCommandVariables: scans mcp_config for unrecognized ${...} variables against the runtime allowlist - validateSensitiveFiles: warns about credentials.json, *.pem, *.key, .ssh/, etc. that would be bundled validateManifest() now accepts optional { projectDir } to separate the manifest directory from the source file directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 1dca91e commit f14863b

6 files changed

Lines changed: 629 additions & 8 deletions

File tree

‎src/cli/pack.ts‎

Lines changed: 5 additions & 1 deletion

‎src/node/validate.ts‎

Lines changed: 239 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { existsSync, readFileSync, statSync } from "fs";
22
import * as fs from "fs/promises";
33
import { DestroyerOfModules } from "galactus";
44
import * as os from "os";
5-
import { dirname, isAbsolute, join, resolve } from "path";
5+
import { dirname, extname, isAbsolute, join, resolve } from "path";
66
import prettyBytes from "pretty-bytes";
77

88
import { unpackExtension } from "../cli/unpack.js";
@@ -11,6 +11,7 @@ import {
1111
MANIFEST_SCHEMAS_LOOSE,
1212
} from "../shared/constants.js";
1313
import { getManifestVersionFromRawData } from "../shared/manifestVersionResolve.js";
14+
import { getAllFilesWithCount, readMcpbIgnorePatterns } from "./files.js";
1415

1516
/**
1617
* Check if a buffer contains a valid PNG file signature
@@ -108,7 +109,191 @@ function validateIcon(
108109
};
109110
}
110111

111-
export function validateManifest(inputPath: string): boolean {
112+
interface ValidationResult {
113+
valid: boolean;
114+
errors: string[];
115+
warnings: string[];
116+
}
117+
118+
// Expected file extensions by server type
119+
const NODE_EXTENSIONS = new Set([".js", ".mjs", ".cjs"]);
120+
const PYTHON_EXTENSIONS = new Set([".py"]);
121+
const SCRIPT_EXTENSIONS = new Set([".js", ".mjs", ".cjs", ".py"]);
122+
123+
/**
124+
* Validate that the server entry_point file exists and matches the server type
125+
*/
126+
function validateEntryPoint(
127+
manifest: { server: { type: string; entry_point: string } },
128+
baseDir: string,
129+
): ValidationResult {
130+
const errors: string[] = [];
131+
const warnings: string[] = [];
132+
const { type, entry_point } = manifest.server;
133+
const entryPath = join(baseDir, entry_point);
134+
135+
if (!existsSync(entryPath)) {
136+
errors.push(`Entry point file not found: ${entry_point}`);
137+
return { valid: false, errors, warnings };
138+
}
139+
140+
const ext = extname(entry_point).toLowerCase();
141+
142+
if (type === "node" && !NODE_EXTENSIONS.has(ext)) {
143+
warnings.push(
144+
`Unusual entry point extension "${ext}" for server type "node". Expected: .js, .mjs, or .cjs`,
145+
);
146+
} else if (
147+
(type === "python" || type === "uv") &&
148+
!PYTHON_EXTENSIONS.has(ext)
149+
) {
150+
warnings.push(
151+
`Unusual entry point extension "${ext}" for server type "${type}". Expected: .py`,
152+
);
153+
} else if (type === "binary" && SCRIPT_EXTENSIONS.has(ext)) {
154+
warnings.push(
155+
`Entry point has script extension "${ext}" but server type is "binary". Did you mean type "node" or "python"?`,
156+
);
157+
}
158+
159+
// For binary type on Unix, check executable bit
160+
if (type === "binary" && process.platform !== "win32") {
161+
const stat = statSync(entryPath);
162+
if (!(stat.mode & 0o111)) {
163+
errors.push(
164+
`Binary entry point is not executable: ${entry_point}. Run: chmod +x ${entry_point}`,
165+
);
166+
}
167+
}
168+
169+
return { valid: errors.length === 0, errors, warnings };
170+
}
171+
172+
// Valid variable patterns from src/shared/config.ts replaceVariables()
173+
const VALID_VARIABLE_PATTERN =
174+
/^\$\{(__dirname|pathSeparator|\/|user_config\..+)\}$/;
175+
176+
/**
177+
* Validate that ${...} variables in mcp_config are recognized
178+
*/
179+
function validateCommandVariables(manifest: {
180+
server: {
181+
mcp_config: {
182+
command?: string;
183+
args?: string[];
184+
env?: Record<string, string>;
185+
platform_overrides?: Record<
186+
string,
187+
{
188+
command?: string;
189+
args?: string[];
190+
env?: Record<string, string>;
191+
}
192+
>;
193+
};
194+
};
195+
}): ValidationResult {
196+
const errors: string[] = [];
197+
const warnings: string[] = [];
198+
199+
function checkString(value: string, context: string): void {
200+
const variablePattern = /\$\{([^}]+)\}/g;
201+
let match;
202+
while ((match = variablePattern.exec(value)) !== null) {
203+
const fullVar = match[0];
204+
if (!VALID_VARIABLE_PATTERN.test(fullVar)) {
205+
errors.push(
206+
`Invalid variable "${fullVar}" in ${context}. Valid variables: \${__dirname}, \${pathSeparator}, \${/}, \${user_config.<key>}`,
207+
);
208+
}
209+
}
210+
}
211+
212+
function checkConfig(
213+
config: { command?: string; args?: string[]; env?: Record<string, string> },
214+
prefix: string,
215+
): void {
216+
if (config.command) checkString(config.command, `${prefix}command`);
217+
if (config.args) {
218+
config.args.forEach((arg, i) => checkString(arg, `${prefix}args[${i}]`));
219+
}
220+
if (config.env) {
221+
for (const [key, val] of Object.entries(config.env)) {
222+
checkString(val, `${prefix}env.${key}`);
223+
}
224+
}
225+
}
226+
227+
const { mcp_config } = manifest.server;
228+
checkConfig(mcp_config, "mcp_config.");
229+
230+
if (mcp_config.platform_overrides) {
231+
for (const [platform, override] of Object.entries(
232+
mcp_config.platform_overrides,
233+
)) {
234+
checkConfig(override, `mcp_config.platform_overrides.${platform}.`);
235+
}
236+
}
237+
238+
return { valid: errors.length === 0, errors, warnings };
239+
}
240+
241+
// Sensitive file patterns not already covered by EXCLUDE_PATTERNS in files.ts
242+
const SENSITIVE_PATTERNS = [
243+
/(^|\/)credentials\.json$/i,
244+
/(^|\/)secrets\./i,
245+
/\.pem$/i,
246+
/\.key$/i,
247+
/\.p12$/i,
248+
/\.pfx$/i,
249+
/\.jks$/i,
250+
/(^|\/)\.aws\//,
251+
/(^|\/)\.ssh\//,
252+
/(^|\/)id_rsa/,
253+
/(^|\/)id_ed25519/,
254+
/(^|\/)id_ecdsa/,
255+
/\.keystore$/i,
256+
/(^|\/)token\.json$/i,
257+
];
258+
259+
/**
260+
* Check if the file list that would be bundled contains sensitive files
261+
*/
262+
function validateSensitiveFiles(baseDir: string): ValidationResult {
263+
const warnings: string[] = [];
264+
265+
try {
266+
const mcpbIgnorePatterns = readMcpbIgnorePatterns(baseDir);
267+
const { files } = getAllFilesWithCount(
268+
baseDir,
269+
baseDir,
270+
{},
271+
mcpbIgnorePatterns,
272+
);
273+
274+
for (const filePath of Object.keys(files)) {
275+
for (const pattern of SENSITIVE_PATTERNS) {
276+
if (pattern.test(filePath)) {
277+
warnings.push(
278+
`Potentially sensitive file will be included in bundle: ${filePath}`,
279+
);
280+
break;
281+
}
282+
}
283+
}
284+
} catch {
285+
// If we can't read the directory, skip this check silently —
286+
// pack will fail with a clearer error later
287+
}
288+
289+
// Sensitive files are always warnings, never errors — a .pem might be a legitimate TLS cert
290+
return { valid: true, errors: [], warnings };
291+
}
292+
293+
export function validateManifest(
294+
inputPath: string,
295+
options?: { projectDir?: string },
296+
): boolean {
112297
try {
113298
const resolvedPath = resolve(inputPath);
114299
let manifestPath = resolvedPath;
@@ -131,17 +316,23 @@ export function validateManifest(inputPath: string): boolean {
131316
if (result.success) {
132317
console.log("Manifest schema validation passes!");
133318

134-
// Validate icon if present
319+
const manifestDir = dirname(manifestPath);
320+
// projectDir is where source files live — defaults to the manifest's directory
321+
const projectDir = options?.projectDir
322+
? resolve(options.projectDir)
323+
: manifestDir;
324+
let hasErrors = false;
325+
326+
// Validate icon if present (always relative to manifest directory)
135327
if (manifestData.icon) {
136-
const baseDir = dirname(manifestPath);
137-
const iconValidation = validateIcon(manifestData.icon, baseDir);
328+
const iconValidation = validateIcon(manifestData.icon, manifestDir);
138329

139330
if (iconValidation.errors.length > 0) {
140331
console.log("\nERROR: Icon validation failed:\n");
141332
iconValidation.errors.forEach((error) => {
142333
console.log(` - ${error}`);
143334
});
144-
return false;
335+
hasErrors = true;
145336
}
146337

147338
if (iconValidation.warnings.length > 0) {
@@ -152,7 +343,48 @@ export function validateManifest(inputPath: string): boolean {
152343
}
153344
}
154345

155-
return true;
346+
// Validate entry point (relative to project directory)
347+
const entryPointValidation = validateEntryPoint(manifestData, projectDir);
348+
if (entryPointValidation.errors.length > 0) {
349+
console.log("\nERROR: Entry point validation failed:\n");
350+
entryPointValidation.errors.forEach((error) => {
351+
console.log(` - ${error}`);
352+
});
353+
hasErrors = true;
354+
}
355+
if (entryPointValidation.warnings.length > 0) {
356+
console.log("\nEntry point warnings:\n");
357+
entryPointValidation.warnings.forEach((warning) => {
358+
console.log(` - ${warning}`);
359+
});
360+
}
361+
362+
// Validate command variables
363+
const variableValidation = validateCommandVariables(manifestData);
364+
if (variableValidation.errors.length > 0) {
365+
console.log("\nERROR: Command variable validation failed:\n");
366+
variableValidation.errors.forEach((error) => {
367+
console.log(` - ${error}`);
368+
});
369+
hasErrors = true;
370+
}
371+
if (variableValidation.warnings.length > 0) {
372+
console.log("\nCommand variable warnings:\n");
373+
variableValidation.warnings.forEach((warning) => {
374+
console.log(` - ${warning}`);
375+
});
376+
}
377+
378+
// Check for sensitive files (relative to project directory)
379+
const sensitiveValidation = validateSensitiveFiles(projectDir);
380+
if (sensitiveValidation.warnings.length > 0) {
381+
console.log("\nSensitive file warnings:\n");
382+
sensitiveValidation.warnings.forEach((warning) => {
383+
console.log(` - ${warning}`);
384+
});
385+
}
386+
387+
return !hasErrors;
156388
} else {
157389
console.log("ERROR: Manifest validation failed:\n");
158390
result.error.issues.forEach((issue) => {

‎test/cli.test.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,10 @@ describe("DXT CLI", () => {
9999
0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
100100
]);
101101
fs.writeFileSync(join(testDir, "icon.png"), validPngBuffer);
102+
103+
// Create entry point file referenced by test manifests
104+
fs.mkdirSync(join(testDir, "server"), { recursive: true });
105+
fs.writeFileSync(join(testDir, "server", "index.js"), "// fixture");
102106
});
103107

104108
afterAll(() => {
@@ -196,6 +200,9 @@ describe("DXT CLI", () => {
196200
fs.writeFileSync(join(tempDir, "file1.txt"), "hello");
197201
fs.mkdirSync(join(tempDir, "subdir"));
198202
fs.writeFileSync(join(tempDir, "subdir", "file2.txt"), "world");
203+
// Create entry point file referenced by manifest
204+
fs.mkdirSync(join(tempDir, "server"), { recursive: true });
205+
fs.writeFileSync(join(tempDir, "server", "index.js"), "// fixture");
199206
});
200207

201208
afterAll(() => {
@@ -291,6 +298,10 @@ describe("DXT CLI", () => {
291298
}),
292299
);
293300

301+
// Create entry point file referenced by manifest
302+
fs.mkdirSync(join(tempExecDir, "server"), { recursive: true });
303+
fs.writeFileSync(join(tempExecDir, "server", "index.js"), "// fixture");
304+
294305
// Create an executable script
295306
const executableScript = join(tempExecDir, "run-script.sh");
296307
fs.writeFileSync(

‎test/icon-validation.test.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,10 @@ describe("Icon Validation", () => {
9393
"Not a PNG file",
9494
);
9595

96+
// Create entry point file referenced by test manifests
97+
fs.mkdirSync(join(testFixturesDir, "server"), { recursive: true });
98+
fs.writeFileSync(join(testFixturesDir, "server", "index.js"), "// fixture");
99+
96100
// Create test manifests
97101
createTestManifest("valid-local-icon.json", {
98102
icon: "valid-icon.png",

‎test/server/index.js‎

Lines changed: 1 addition & 0 deletions

0 commit comments

Comments
 (0)