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>

src/cli/pack.ts

@@ -83,7 +83,11 @@ export async function packExtension({
 
   // Validate manifest first
   logger.log("Validating manifest...");
-  if (!validateManifest(manifestPath)) {
+  if (
+    !validateManifest(manifestPath, {
+      projectDir: resolvedPath,
+    })
+  ) {
     logger.error("ERROR: Cannot pack extension with invalid manifest");
     return false;
   }

src/node/validate.ts

@@ -2,7 +2,7 @@ import { existsSync, readFileSync, statSync } from "fs";
 import * as fs from "fs/promises";
 import { DestroyerOfModules } from "galactus";
 import * as os from "os";
-import { dirname, isAbsolute, join, resolve } from "path";
+import { dirname, extname, isAbsolute, join, resolve } from "path";
 import prettyBytes from "pretty-bytes";
 
 import { unpackExtension } from "../cli/unpack.js";
@@ -11,6 +11,7 @@ import {
   MANIFEST_SCHEMAS_LOOSE,
 } from "../shared/constants.js";
 import { getManifestVersionFromRawData } from "../shared/manifestVersionResolve.js";
+import { getAllFilesWithCount, readMcpbIgnorePatterns } from "./files.js";
 
 /**
  * Check if a buffer contains a valid PNG file signature
@@ -108,7 +109,191 @@ function validateIcon(
   };
 }
 
-export function validateManifest(inputPath: string): boolean {
+interface ValidationResult {
+  valid: boolean;
+  errors: string[];
+  warnings: string[];
+}
+
+// Expected file extensions by server type
+const NODE_EXTENSIONS = new Set([".js", ".mjs", ".cjs"]);
+const PYTHON_EXTENSIONS = new Set([".py"]);
+const SCRIPT_EXTENSIONS = new Set([".js", ".mjs", ".cjs", ".py"]);
+
+/**
+ * Validate that the server entry_point file exists and matches the server type
+ */
+function validateEntryPoint(
+  manifest: { server: { type: string; entry_point: string } },
+  baseDir: string,
+): ValidationResult {
+  const errors: string[] = [];
+  const warnings: string[] = [];
+  const { type, entry_point } = manifest.server;
+  const entryPath = join(baseDir, entry_point);
+
+  if (!existsSync(entryPath)) {
+    errors.push(`Entry point file not found: ${entry_point}`);
+    return { valid: false, errors, warnings };
+  }
+
+  const ext = extname(entry_point).toLowerCase();
+
+  if (type === "node" && !NODE_EXTENSIONS.has(ext)) {
+    warnings.push(
+      `Unusual entry point extension "${ext}" for server type "node". Expected: .js, .mjs, or .cjs`,
+    );
+  } else if (
+    (type === "python" || type === "uv") &&
+    !PYTHON_EXTENSIONS.has(ext)
+  ) {
+    warnings.push(
+      `Unusual entry point extension "${ext}" for server type "${type}". Expected: .py`,
+    );
+  } else if (type === "binary" && SCRIPT_EXTENSIONS.has(ext)) {
+    warnings.push(
+      `Entry point has script extension "${ext}" but server type is "binary". Did you mean type "node" or "python"?`,
+    );
+  }
+
+  // For binary type on Unix, check executable bit
+  if (type === "binary" && process.platform !== "win32") {
+    const stat = statSync(entryPath);
+    if (!(stat.mode & 0o111)) {
+      errors.push(
+        `Binary entry point is not executable: ${entry_point}. Run: chmod +x ${entry_point}`,
+      );
+    }
+  }
+
+  return { valid: errors.length === 0, errors, warnings };
+}
+
+// Valid variable patterns from src/shared/config.ts replaceVariables()
+const VALID_VARIABLE_PATTERN =
+  /^\$\{(__dirname|pathSeparator|\/|user_config\..+)\}$/;
+
+/**
+ * Validate that ${...} variables in mcp_config are recognized
+ */
+function validateCommandVariables(manifest: {
+  server: {
+    mcp_config: {
+      command?: string;
+      args?: string[];
+      env?: Record<string, string>;
+      platform_overrides?: Record<
+        string,
+        {
+          command?: string;
+          args?: string[];
+          env?: Record<string, string>;
+        }
+      >;
+    };
+  };
+}): ValidationResult {
+  const errors: string[] = [];
+  const warnings: string[] = [];
+
+  function checkString(value: string, context: string): void {
+    const variablePattern = /\$\{([^}]+)\}/g;
+    let match;
+    while ((match = variablePattern.exec(value)) !== null) {
+      const fullVar = match[0];
+      if (!VALID_VARIABLE_PATTERN.test(fullVar)) {
+        errors.push(
+          `Invalid variable "${fullVar}" in ${context}. Valid variables: \${__dirname}, \${pathSeparator}, \${/}, \${user_config.<key>}`,
+        );
+      }
+    }
+  }
+
+  function checkConfig(
+    config: { command?: string; args?: string[]; env?: Record<string, string> },
+    prefix: string,
+  ): void {
+    if (config.command) checkString(config.command, `${prefix}command`);
+    if (config.args) {
+      config.args.forEach((arg, i) => checkString(arg, `${prefix}args[${i}]`));
+    }
+    if (config.env) {
+      for (const [key, val] of Object.entries(config.env)) {
+        checkString(val, `${prefix}env.${key}`);
+      }
+    }
+  }
+
+  const { mcp_config } = manifest.server;
+  checkConfig(mcp_config, "mcp_config.");
+
+  if (mcp_config.platform_overrides) {
+    for (const [platform, override] of Object.entries(
+      mcp_config.platform_overrides,
+    )) {
+      checkConfig(override, `mcp_config.platform_overrides.${platform}.`);
+    }
+  }
+
+  return { valid: errors.length === 0, errors, warnings };
+}
+
+// Sensitive file patterns not already covered by EXCLUDE_PATTERNS in files.ts
+const SENSITIVE_PATTERNS = [
+  /(^|\/)credentials\.json$/i,
+  /(^|\/)secrets\./i,
+  /\.pem$/i,
+  /\.key$/i,
+  /\.p12$/i,
+  /\.pfx$/i,
+  /\.jks$/i,
+  /(^|\/)\.aws\//,
+  /(^|\/)\.ssh\//,
+  /(^|\/)id_rsa/,
+  /(^|\/)id_ed25519/,
+  /(^|\/)id_ecdsa/,
+  /\.keystore$/i,
+  /(^|\/)token\.json$/i,
+];
+
+/**
+ * Check if the file list that would be bundled contains sensitive files
+ */
+function validateSensitiveFiles(baseDir: string): ValidationResult {
+  const warnings: string[] = [];
+
+  try {
+    const mcpbIgnorePatterns = readMcpbIgnorePatterns(baseDir);
+    const { files } = getAllFilesWithCount(
+      baseDir,
+      baseDir,
+      {},
+      mcpbIgnorePatterns,
+    );
+
+    for (const filePath of Object.keys(files)) {
+      for (const pattern of SENSITIVE_PATTERNS) {
+        if (pattern.test(filePath)) {
+          warnings.push(
+            `Potentially sensitive file will be included in bundle: ${filePath}`,
+          );
+          break;
+        }
+      }
+    }
+  } catch {
+    // If we can't read the directory, skip this check silently —
+    // pack will fail with a clearer error later
+  }
+
+  // Sensitive files are always warnings, never errors — a .pem might be a legitimate TLS cert
+  return { valid: true, errors: [], warnings };
+}
+
+export function validateManifest(
+  inputPath: string,
+  options?: { projectDir?: string },
+): boolean {
   try {
     const resolvedPath = resolve(inputPath);
     let manifestPath = resolvedPath;
@@ -131,17 +316,23 @@ export function validateManifest(inputPath: string): boolean {
     if (result.success) {
       console.log("Manifest schema validation passes!");
 
-      // Validate icon if present
+      const manifestDir = dirname(manifestPath);
+      // projectDir is where source files live — defaults to the manifest's directory
+      const projectDir = options?.projectDir
+        ? resolve(options.projectDir)
+        : manifestDir;
+      let hasErrors = false;
+
+      // Validate icon if present (always relative to manifest directory)
       if (manifestData.icon) {
-        const baseDir = dirname(manifestPath);
+        const iconValidation = validateIcon(manifestData.icon, manifestDir);
-        const iconValidation = validateIcon(manifestData.icon, baseDir);
 
         if (iconValidation.errors.length > 0) {
           console.log("\nERROR: Icon validation failed:\n");
           iconValidation.errors.forEach((error) => {
             console.log(`  - ${error}`);
           });
-          return false;
+          hasErrors = true;
         }
 
         if (iconValidation.warnings.length > 0) {
@@ -152,7 +343,48 @@ export function validateManifest(inputPath: string): boolean {
         }
       }
 
-      return true;
+      // Validate entry point (relative to project directory)
+      const entryPointValidation = validateEntryPoint(manifestData, projectDir);
+      if (entryPointValidation.errors.length > 0) {
+        console.log("\nERROR: Entry point validation failed:\n");
+        entryPointValidation.errors.forEach((error) => {
+          console.log(`  - ${error}`);
+        });
+        hasErrors = true;
+      }
+      if (entryPointValidation.warnings.length > 0) {
+        console.log("\nEntry point warnings:\n");
+        entryPointValidation.warnings.forEach((warning) => {
+          console.log(`  - ${warning}`);
+        });
+      }
+
+      // Validate command variables
+      const variableValidation = validateCommandVariables(manifestData);
+      if (variableValidation.errors.length > 0) {
+        console.log("\nERROR: Command variable validation failed:\n");
+        variableValidation.errors.forEach((error) => {
+          console.log(`  - ${error}`);
+        });
+        hasErrors = true;
+      }
+      if (variableValidation.warnings.length > 0) {
+        console.log("\nCommand variable warnings:\n");
+        variableValidation.warnings.forEach((warning) => {
+          console.log(`  - ${warning}`);
+        });
+      }
+
+      // Check for sensitive files (relative to project directory)
+      const sensitiveValidation = validateSensitiveFiles(projectDir);
+      if (sensitiveValidation.warnings.length > 0) {
+        console.log("\nSensitive file warnings:\n");
+        sensitiveValidation.warnings.forEach((warning) => {
+          console.log(`  - ${warning}`);
+        });
+      }
+
+      return !hasErrors;
     } else {
       console.log("ERROR: Manifest validation failed:\n");
       result.error.issues.forEach((issue) => {

test/cli.test.ts

@@ -99,6 +99,10 @@ describe("DXT CLI", () => {
         0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
       ]);
       fs.writeFileSync(join(testDir, "icon.png"), validPngBuffer);
+
+      // Create entry point file referenced by test manifests
+      fs.mkdirSync(join(testDir, "server"), { recursive: true });
+      fs.writeFileSync(join(testDir, "server", "index.js"), "// fixture");
     });
 
     afterAll(() => {
@@ -196,6 +200,9 @@ describe("DXT CLI", () => {
       fs.writeFileSync(join(tempDir, "file1.txt"), "hello");
       fs.mkdirSync(join(tempDir, "subdir"));
       fs.writeFileSync(join(tempDir, "subdir", "file2.txt"), "world");
+      // Create entry point file referenced by manifest
+      fs.mkdirSync(join(tempDir, "server"), { recursive: true });
+      fs.writeFileSync(join(tempDir, "server", "index.js"), "// fixture");
     });
 
     afterAll(() => {
@@ -291,6 +298,10 @@ describe("DXT CLI", () => {
           }),
         );
 
+        // Create entry point file referenced by manifest
+        fs.mkdirSync(join(tempExecDir, "server"), { recursive: true });
+        fs.writeFileSync(join(tempExecDir, "server", "index.js"), "// fixture");
+
         // Create an executable script
         const executableScript = join(tempExecDir, "run-script.sh");
         fs.writeFileSync(

test/icon-validation.test.ts

@@ -93,6 +93,10 @@ describe("Icon Validation", () => {
       "Not a PNG file",
     );
 
+    // Create entry point file referenced by test manifests
+    fs.mkdirSync(join(testFixturesDir, "server"), { recursive: true });
+    fs.writeFileSync(join(testFixturesDir, "server", "index.js"), "// fixture");
+
     // Create test manifests
     createTestManifest("valid-local-icon.json", {
       icon: "valid-icon.png",

test/server/index.js

@@ -0,0 +1 @@
+// test fixture

test/validate.test.ts

@@ -0,0 +1,369 @@
+import { execSync } from "node:child_process";
+import fs from "node:fs";
+import { join } from "node:path";
+
+import { DEFAULT_MANIFEST_VERSION } from "../src/shared/constants";
+
+interface ExecSyncError extends Error {
+  stdout: Buffer;
+  stderr: Buffer;
+  status: number | null;
+}
+
+describe("Enhanced Validation", () => {
+  const cliPath = join(__dirname, "../dist/cli/cli.js");
+  const fixturesDir = join(__dirname, "fixtures", "validate");
+
+  function createManifest(
+    dir: string,
+    overrides: Record<string, unknown> = {},
+  ) {
+    const manifest = {
+      manifest_version: DEFAULT_MANIFEST_VERSION,
+      name: "test-extension",
+      version: "1.0.0",
+      description: "Test extension for validation",
+      author: { name: "Test Author" },
+      server: {
+        type: "node",
+        entry_point: "server/index.js",
+        mcp_config: {
+          command: "node",
+          args: ["${__dirname}/server/index.js"],
+        },
+      },
+      ...overrides,
+    };
+
+    fs.writeFileSync(
+      join(dir, "manifest.json"),
+      JSON.stringify(manifest, null, 2),
+    );
+  }
+
+  beforeAll(() => {
+    execSync("yarn build", { cwd: join(__dirname, "..") });
+
+    if (fs.existsSync(fixturesDir)) {
+      fs.rmSync(fixturesDir, { recursive: true, force: true });
+    }
+    fs.mkdirSync(fixturesDir, { recursive: true });
+  });
+
+  afterAll(() => {
+    if (fs.existsSync(fixturesDir)) {
+      fs.rmSync(fixturesDir, { recursive: true, force: true });
+    }
+  });
+
+  describe("entry point validation", () => {
+    it("should fail when entry_point file is missing", () => {
+      const dir = join(fixturesDir, "missing-entry");
+      fs.mkdirSync(dir, { recursive: true });
+      createManifest(dir);
+      // Deliberately NOT creating server/index.js
+
+      expect(() => {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      }).toThrow();
+
+      try {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      } catch (error: unknown) {
+        const execError = error as ExecSyncError;
+        const output = execError.stdout?.toString() || "";
+        expect(output).toContain("Entry point file not found");
+      }
+    });
+
+    it("should warn about extension mismatch (node type with .py file)", () => {
+      const dir = join(fixturesDir, "ext-mismatch");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "main.py"), "print('hello')");
+      createManifest(dir, {
+        server: {
+          type: "node",
+          entry_point: "server/main.py",
+          mcp_config: { command: "node" },
+        },
+      });
+
+      const result = execSync(`node ${cliPath} validate ${dir}`, {
+        encoding: "utf-8",
+      });
+
+      expect(result).toContain("Manifest schema validation passes!");
+      expect(result).toContain("Unusual entry point extension");
+    });
+
+    it("should fail when binary entry_point is not executable (Unix only)", () => {
+      if (process.platform === "win32") {
+        return;
+      }
+
+      const dir = join(fixturesDir, "binary-not-exec");
+      fs.mkdirSync(dir, { recursive: true });
+      fs.writeFileSync(join(dir, "myserver"), "#!/bin/sh\necho hi");
+      fs.chmodSync(join(dir, "myserver"), 0o644);
+      createManifest(dir, {
+        server: {
+          type: "binary",
+          entry_point: "myserver",
+          mcp_config: { command: "./myserver" },
+        },
+      });
+
+      expect(() => {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      }).toThrow();
+
+      try {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      } catch (error: unknown) {
+        const execError = error as ExecSyncError;
+        const output = execError.stdout?.toString() || "";
+        expect(output).toContain("not executable");
+      }
+    });
+
+    it("should pass when binary entry_point is executable (Unix only)", () => {
+      if (process.platform === "win32") {
+        return;
+      }
+
+      const dir = join(fixturesDir, "binary-exec");
+      fs.mkdirSync(dir, { recursive: true });
+      fs.writeFileSync(join(dir, "myserver"), "#!/bin/sh\necho hi");
+      fs.chmodSync(join(dir, "myserver"), 0o755);
+      createManifest(dir, {
+        server: {
+          type: "binary",
+          entry_point: "myserver",
+          mcp_config: { command: "./myserver" },
+        },
+      });
+
+      const result = execSync(`node ${cliPath} validate ${dir}`, {
+        encoding: "utf-8",
+      });
+
+      expect(result).toContain("Manifest schema validation passes!");
+    });
+  });
+
+  describe("command variable validation", () => {
+    it("should fail on invalid variables like ${BUNDLE_ROOT}", () => {
+      const dir = join(fixturesDir, "invalid-var");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "index.js"), "// fixture");
+      createManifest(dir, {
+        server: {
+          type: "node",
+          entry_point: "server/index.js",
+          mcp_config: {
+            command: "node",
+            args: ["${BUNDLE_ROOT}/server/index.js"],
+          },
+        },
+      });
+
+      expect(() => {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      }).toThrow();
+
+      try {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      } catch (error: unknown) {
+        const execError = error as ExecSyncError;
+        const output = execError.stdout?.toString() || "";
+        expect(output).toContain("Invalid variable");
+        expect(output).toContain("${BUNDLE_ROOT}");
+      }
+    });
+
+    it("should fail on invalid variables in env values", () => {
+      const dir = join(fixturesDir, "invalid-env-var");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "index.js"), "// fixture");
+      createManifest(dir, {
+        server: {
+          type: "node",
+          entry_point: "server/index.js",
+          mcp_config: {
+            command: "node",
+            args: ["${__dirname}/server/index.js"],
+            env: { PATH: "${HOME}/bin" },
+          },
+        },
+      });
+
+      expect(() => {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      }).toThrow();
+
+      try {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      } catch (error: unknown) {
+        const execError = error as ExecSyncError;
+        const output = execError.stdout?.toString() || "";
+        expect(output).toContain("Invalid variable");
+        expect(output).toContain("${HOME}");
+      }
+    });
+
+    it("should pass with valid variables: ${__dirname} and ${user_config.*}", () => {
+      const dir = join(fixturesDir, "valid-vars");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "index.js"), "// fixture");
+      createManifest(dir, {
+        server: {
+          type: "node",
+          entry_point: "server/index.js",
+          mcp_config: {
+            command: "node",
+            args: [
+              "${__dirname}/server/index.js",
+              "--key",
+              "${user_config.api_key}",
+            ],
+            env: { SEP: "${pathSeparator}" },
+          },
+        },
+      });
+
+      const result = execSync(`node ${cliPath} validate ${dir}`, {
+        encoding: "utf-8",
+      });
+
+      expect(result).toContain("Manifest schema validation passes!");
+      expect(result).not.toContain("Invalid variable");
+    });
+
+    it("should fail on invalid variables in platform_overrides", () => {
+      const dir = join(fixturesDir, "invalid-platform-var");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "index.js"), "// fixture");
+
+      const manifest = {
+        manifest_version: "0.3",
+        name: "test-extension",
+        version: "1.0.0",
+        description: "Test",
+        author: { name: "Test" },
+        server: {
+          type: "node",
+          entry_point: "server/index.js",
+          mcp_config: {
+            command: "node",
+            args: ["${__dirname}/server/index.js"],
+            platform_overrides: {
+              win32: {
+                args: ["${APPDATA}/server/index.js"],
+              },
+            },
+          },
+        },
+      };
+
+      fs.writeFileSync(
+        join(dir, "manifest.json"),
+        JSON.stringify(manifest, null, 2),
+      );
+
+      expect(() => {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      }).toThrow();
+
+      try {
+        execSync(`node ${cliPath} validate ${dir}`, {
+          encoding: "utf-8",
+          stdio: "pipe",
+        });
+      } catch (error: unknown) {
+        const execError = error as ExecSyncError;
+        const output = execError.stdout?.toString() || "";
+        expect(output).toContain("Invalid variable");
+        expect(output).toContain("${APPDATA}");
+      }
+    });
+  });
+
+  describe("sensitive file detection", () => {
+    it("should warn about credentials.json in project directory", () => {
+      const dir = join(fixturesDir, "sensitive-files");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "index.js"), "// fixture");
+      fs.writeFileSync(join(dir, "credentials.json"), '{"secret": "value"}');
+      createManifest(dir);
+
+      const result = execSync(`node ${cliPath} validate ${dir}`, {
+        encoding: "utf-8",
+      });
+
+      // Should pass (warnings only) but show the warning
+      expect(result).toContain("Manifest schema validation passes!");
+      expect(result).toContain("Potentially sensitive file");
+      expect(result).toContain("credentials.json");
+    });
+
+    it("should warn about .pem files in project directory", () => {
+      const dir = join(fixturesDir, "sensitive-pem");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "index.js"), "// fixture");
+      fs.writeFileSync(join(dir, "server.pem"), "--- CERT ---");
+      createManifest(dir);
+
+      const result = execSync(`node ${cliPath} validate ${dir}`, {
+        encoding: "utf-8",
+      });
+
+      expect(result).toContain("Manifest schema validation passes!");
+      expect(result).toContain("Potentially sensitive file");
+      expect(result).toContain("server.pem");
+    });
+  });
+
+  describe("happy path", () => {
+    it("should pass with all files present and correct types", () => {
+      const dir = join(fixturesDir, "happy-path");
+      fs.mkdirSync(join(dir, "server"), { recursive: true });
+      fs.writeFileSync(join(dir, "server", "index.js"), "// server code");
+      createManifest(dir);
+
+      const result = execSync(`node ${cliPath} validate ${dir}`, {
+        encoding: "utf-8",
+      });
+
+      expect(result).toContain("Manifest schema validation passes!");
+      expect(result).not.toContain("ERROR");
+      expect(result).not.toContain("sensitive");
+    });
+  });
+});