diff --git a/src/cli/pack.ts b/src/cli/pack.ts index e2411bf..d2aba54 100644 --- a/src/cli/pack.ts +++ b/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; } diff --git a/src/node/validate.ts b/src/node/validate.ts index d1ee8fd..b958798 100644 --- a/src/node/validate.ts +++ b/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; + platform_overrides?: Record< + string, + { + command?: string; + args?: string[]; + env?: Record; + } + >; + }; + }; +}): 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.}`, + ); + } + } + } + + function checkConfig( + config: { command?: string; args?: string[]; env?: Record }, + 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, baseDir); + const iconValidation = validateIcon(manifestData.icon, manifestDir); 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) => { diff --git a/test/cli.test.ts b/test/cli.test.ts index e7757ac..7ecc402 100644 --- a/test/cli.test.ts +++ b/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( diff --git a/test/icon-validation.test.ts b/test/icon-validation.test.ts index b55abd4..bdc7bbe 100644 --- a/test/icon-validation.test.ts +++ b/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", diff --git a/test/server/index.js b/test/server/index.js new file mode 100644 index 0000000..c8a782c --- /dev/null +++ b/test/server/index.js @@ -0,0 +1 @@ +// test fixture diff --git a/test/validate.test.ts b/test/validate.test.ts new file mode 100644 index 0000000..e998fb2 --- /dev/null +++ b/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 = {}, + ) { + 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"); + }); + }); +});