Merge pull request #136 from bthompson-sys/feat/icon-validation

feat: add comprehensive icon validation to mcpb validate command

Author: joan-anthropic

examples/hello-world-node/manifest.json

@@ -1,6 +1,6 @@
 {
   "$schema": "../../dist/mcpb-manifest.schema.json",
-  "manifest_version": "0.1",
+  "manifest_version": "0.3",
   "name": "hello-world-node",
   "display_name": "Hello World MCP Server (Reference Extension)",
   "version": "0.1.0",

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 { join, resolve } from "path";
+import { dirname, isAbsolute, join, resolve } from "path";
 import prettyBytes from "pretty-bytes";
 
 import { unpackExtension } from "../cli/unpack.js";
@@ -11,6 +11,102 @@ import {
   LATEST_MANIFEST_SCHEMA_LOOSE,
 } from "../shared/constants.js";
 
+/**
+ * Check if a buffer contains a valid PNG file signature
+ */
+function isPNG(buffer: Buffer): boolean {
+  // PNG signature: 89 50 4E 47 0D 0A 1A 0A
+  return (
+    buffer.length >= 8 &&
+    buffer[0] === 0x89 &&
+    buffer[1] === 0x50 &&
+    buffer[2] === 0x4e &&
+    buffer[3] === 0x47 &&
+    buffer[4] === 0x0d &&
+    buffer[5] === 0x0a &&
+    buffer[6] === 0x1a &&
+    buffer[7] === 0x0a
+  );
+}
+
+/**
+ * Validate icon field in manifest
+ * @param iconPath - The icon path from manifest.json
+ * @param baseDir - The base directory containing the manifest
+ * @returns Validation result with errors and warnings
+ */
+function validateIcon(
+  iconPath: string,
+  baseDir: string,
+): { valid: boolean; errors: string[]; warnings: string[] } {
+  const errors: string[] = [];
+  const warnings: string[] = [];
+
+  const isRemoteUrl =
+    iconPath.startsWith("http://") || iconPath.startsWith("https://");
+  const hasVariableSubstitution = iconPath.includes("${__dirname}");
+  const isAbsolutePath = isAbsolute(iconPath);
+
+  // Warn about remote URLs (best practice: use local files)
+  if (isRemoteUrl) {
+    warnings.push(
+      "Icon path uses a remote URL. " +
+        'Best practice for local MCP servers: Use local files like "icon": "icon.png" for maximum compatibility. ' +
+        "Claude Desktop currently only supports local icon files in bundles.",
+    );
+  }
+
+  // Check for ${__dirname} variable (error - doesn't work)
+  if (hasVariableSubstitution) {
+    errors.push(
+      "Icon path should not use ${__dirname} variable substitution. " +
+        'Use a simple relative path like "icon.png" instead of "${__dirname}/icon.png".',
+    );
+  }
+
+  // Check for absolute path (error - not portable)
+  if (isAbsolutePath) {
+    errors.push(
+      "Icon path must be relative to the bundle root, not an absolute path. " +
+        `Found: "${iconPath}"`,
+    );
+  }
+
+  // Only proceed with file checks if the path looks like a local file
+  if (!isRemoteUrl && !isAbsolutePath && !hasVariableSubstitution) {
+    // Check file existence
+    const fullIconPath = join(baseDir, iconPath);
+    if (!existsSync(fullIconPath)) {
+      errors.push(`Icon file not found at path: ${iconPath}`);
+    } else {
+      try {
+        // Check PNG format
+        const buffer = readFileSync(fullIconPath);
+        if (!isPNG(buffer)) {
+          errors.push(
+            `Icon file must be PNG format. The file at "${iconPath}" does not appear to be a valid PNG file.`,
+          );
+        } else {
+          // File exists and is a valid PNG - add recommendation
+          warnings.push(
+            "Icon validation passed. Recommended size is 512×512 pixels for best display in Claude Desktop.",
+          );
+        }
+      } catch (error) {
+        errors.push(
+          `Unable to read icon file at "${iconPath}": ${error instanceof Error ? error.message : "Unknown error"}`,
+        );
+      }
+    }
+  }
+
+  return {
+    valid: errors.length === 0,
+    errors,
+    warnings,
+  };
+}
+
 export function validateManifest(inputPath: string): boolean {
   try {
     const resolvedPath = resolve(inputPath);
@@ -28,6 +124,28 @@ export function validateManifest(inputPath: string): boolean {
 
     if (result.success) {
       console.log("Manifest schema validation passes!");
+
+      // Validate icon if present
+      if (manifestData.icon) {
+        const baseDir = dirname(manifestPath);
+        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;
+        }
+
+        if (iconValidation.warnings.length > 0) {
+          console.log("\nIcon validation warnings:\n");
+          iconValidation.warnings.forEach((warning) => {
+            console.log(`  - ${warning}`);
+          });
+        }
+      }
+
       return true;
     } else {
       console.log("ERROR: Manifest validation failed:\n");

test/cli.test.ts

@@ -80,6 +80,90 @@ describe("DXT CLI", () => {
     fs.unlinkSync(invalidJsonPath);
   });
 
+  describe("icon validation integration", () => {
+    const testDir = join(__dirname, "icon-test");
+
+    beforeAll(() => {
+      // Create test directory
+      if (!fs.existsSync(testDir)) {
+        fs.mkdirSync(testDir, { recursive: true });
+      }
+
+      // Create a valid PNG file (1x1 transparent pixel)
+      const validPngBuffer = Buffer.from([
+        0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x0d,
+        0x49, 0x48, 0x44, 0x52, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,
+        0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4, 0x89, 0x00, 0x00, 0x00,
+        0x0a, 0x49, 0x44, 0x41, 0x54, 0x78, 0x9c, 0x63, 0x00, 0x01, 0x00, 0x00,
+        0x05, 0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00, 0x00, 0x00, 0x49,
+        0x45, 0x4e, 0x44, 0xae, 0x42, 0x60, 0x82,
+      ]);
+      fs.writeFileSync(join(testDir, "icon.png"), validPngBuffer);
+    });
+
+    afterAll(() => {
+      // Clean up test directory
+      if (fs.existsSync(testDir)) {
+        fs.rmSync(testDir, { recursive: true, force: true });
+      }
+    });
+
+    it("should pass validation with valid local icon", () => {
+      const manifestWithIcon = join(testDir, "manifest-with-icon.json");
+      fs.writeFileSync(
+        manifestWithIcon,
+        JSON.stringify({
+          manifest_version: LATEST_MANIFEST_VERSION,
+          name: "test-with-icon",
+          version: "1.0.0",
+          description: "Test with icon",
+          author: { name: "Test" },
+          icon: "icon.png",
+          server: {
+            type: "node",
+            entry_point: "server/index.js",
+            mcp_config: { command: "node" },
+          },
+        }),
+      );
+
+      const result = execSync(`node ${cliPath} validate ${manifestWithIcon}`, {
+        encoding: "utf-8",
+      });
+
+      expect(result).toContain("Manifest schema validation passes!");
+      expect(result).toContain("Icon validation");
+    });
+
+    it("should warn about manifest with remote icon URL but not fail", () => {
+      const manifestWithUrl = join(testDir, "manifest-with-url.json");
+      fs.writeFileSync(
+        manifestWithUrl,
+        JSON.stringify({
+          manifest_version: LATEST_MANIFEST_VERSION,
+          name: "test-with-url",
+          version: "1.0.0",
+          description: "Test with URL",
+          author: { name: "Test" },
+          icon: "https://example.com/icon.png",
+          server: {
+            type: "node",
+            entry_point: "server/index.js",
+            mcp_config: { command: "node" },
+          },
+        }),
+      );
+
+      const result = execSync(`node ${cliPath} validate ${manifestWithUrl}`, {
+        encoding: "utf-8",
+      });
+
+      expect(result).toContain("Manifest schema validation passes!");
+      expect(result).toContain("Icon validation warnings");
+      expect(result).toContain("Icon path uses a remote URL");
+    });
+  });
+
   describe("pack and unpack", () => {
     const tempDir = join(__dirname, "temp-pack-test");
     const packedFilePath = join(__dirname, "test-extension.dxt");