Merge pull request #136 from bthompson-sys/feat/icon-validation
feat: add comprehensive icon validation to mcpb validate command
This commit is contained in:
joan-anthropic
GitHub
commit
ba74af5217
4 changed files with 532 additions and 2 deletions
|
|
@ -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",
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
|
|
@ -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");
|
||||
|
|
|
|||
328
test/icon-validation.test.ts
Normal file
328
test/icon-validation.test.ts
Normal file
|
|
@ -0,0 +1,328 @@
|
|||
import { execSync } from "node:child_process";
|
||||
import fs from "node:fs";
|
||||
import { join } from "node:path";
|
||||
|
||||
describe("Icon Validation", () => {
|
||||
const cliPath = join(__dirname, "../dist/cli/cli.js");
|
||||
const testFixturesDir = join(__dirname, "fixtures", "icon-validation");
|
||||
|
||||
beforeAll(() => {
|
||||
// Ensure the CLI is built
|
||||
execSync("yarn build", { cwd: join(__dirname, "..") });
|
||||
|
||||
// Create test fixtures directory
|
||||
if (!fs.existsSync(testFixturesDir)) {
|
||||
fs.mkdirSync(testFixturesDir, { recursive: true });
|
||||
}
|
||||
|
||||
// Create a valid PNG file (1x1 transparent pixel)
|
||||
const validPngBuffer = Buffer.from([
|
||||
0x89,
|
||||
0x50,
|
||||
0x4e,
|
||||
0x47,
|
||||
0x0d,
|
||||
0x0a,
|
||||
0x1a,
|
||||
0x0a, // PNG signature
|
||||
0x00,
|
||||
0x00,
|
||||
0x00,
|
||||
0x0d,
|
||||
0x49,
|
||||
0x48,
|
||||
0x44,
|
||||
0x52, // IHDR chunk
|
||||
0x00,
|
||||
0x00,
|
||||
0x00,
|
||||
0x01,
|
||||
0x00,
|
||||
0x00,
|
||||
0x00,
|
||||
0x01, // 1x1 dimensions
|
||||
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(testFixturesDir, "valid-icon.png"), validPngBuffer);
|
||||
|
||||
// Create an invalid (non-PNG) file
|
||||
fs.writeFileSync(
|
||||
join(testFixturesDir, "invalid-icon.jpg"),
|
||||
"Not a PNG file",
|
||||
);
|
||||
|
||||
// Create test manifests
|
||||
createTestManifest("valid-local-icon.json", {
|
||||
icon: "valid-icon.png",
|
||||
});
|
||||
|
||||
createTestManifest("invalid-remote-url.json", {
|
||||
icon: "https://example.com/icon.png",
|
||||
});
|
||||
|
||||
createTestManifest("invalid-dirname-variable.json", {
|
||||
icon: "${__dirname}/icon.png",
|
||||
});
|
||||
|
||||
createTestManifest("invalid-absolute-path.json", {
|
||||
icon: "/absolute/path/to/icon.png",
|
||||
});
|
||||
|
||||
createTestManifest("invalid-missing-file.json", {
|
||||
icon: "missing-icon.png",
|
||||
});
|
||||
|
||||
createTestManifest("invalid-non-png.json", {
|
||||
icon: "invalid-icon.jpg",
|
||||
});
|
||||
|
||||
createTestManifest("no-icon.json", {
|
||||
// No icon field
|
||||
});
|
||||
});
|
||||
|
||||
afterAll(() => {
|
||||
// Clean up test fixtures
|
||||
if (fs.existsSync(testFixturesDir)) {
|
||||
fs.rmSync(testFixturesDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
function createTestManifest(filename: string, iconConfig: { icon?: string }) {
|
||||
const manifest = {
|
||||
manifest_version: "0.3",
|
||||
name: "test-extension",
|
||||
version: "1.0.0",
|
||||
description: "Test extension for icon validation",
|
||||
author: {
|
||||
name: "Test Author",
|
||||
},
|
||||
server: {
|
||||
type: "node",
|
||||
entry_point: "server/index.js",
|
||||
mcp_config: {
|
||||
command: "node",
|
||||
args: ["${__dirname}/server/index.js"],
|
||||
},
|
||||
},
|
||||
...iconConfig,
|
||||
};
|
||||
|
||||
fs.writeFileSync(
|
||||
join(testFixturesDir, filename),
|
||||
JSON.stringify(manifest, null, 2),
|
||||
);
|
||||
}
|
||||
|
||||
describe("Valid icon configurations", () => {
|
||||
it("should pass validation with a valid local PNG icon", () => {
|
||||
const manifestPath = join(testFixturesDir, "valid-local-icon.json");
|
||||
const result = execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
});
|
||||
|
||||
expect(result).toContain("Manifest schema validation passes!");
|
||||
expect(result).toContain("Icon validation passed");
|
||||
});
|
||||
|
||||
it("should pass validation when no icon is specified", () => {
|
||||
const manifestPath = join(testFixturesDir, "no-icon.json");
|
||||
const result = execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
});
|
||||
|
||||
expect(result).toContain("Manifest schema validation passes!");
|
||||
expect(result).not.toContain("Icon validation");
|
||||
});
|
||||
});
|
||||
|
||||
describe("Invalid icon configurations", () => {
|
||||
it("should warn about remote URL icons but not fail", () => {
|
||||
const manifestPath = join(testFixturesDir, "invalid-remote-url.json");
|
||||
|
||||
const result = execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
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");
|
||||
expect(result).toContain("Best practice for local MCP servers");
|
||||
expect(result).toContain("Claude Desktop currently only supports local");
|
||||
expect(result).not.toContain("ERROR");
|
||||
});
|
||||
|
||||
it("should reject icons with ${__dirname} variable", () => {
|
||||
const manifestPath = join(
|
||||
testFixturesDir,
|
||||
"invalid-dirname-variable.json",
|
||||
);
|
||||
|
||||
expect(() => {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
}).toThrow();
|
||||
|
||||
try {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch (error: unknown) {
|
||||
const execError = error as { stdout?: Buffer; stderr?: Buffer };
|
||||
const output = execError.stdout?.toString() || "";
|
||||
expect(output).toContain("Icon validation failed");
|
||||
expect(output).toContain("${__dirname}");
|
||||
expect(output).toContain("simple relative path");
|
||||
}
|
||||
});
|
||||
|
||||
it("should reject absolute paths", () => {
|
||||
const manifestPath = join(testFixturesDir, "invalid-absolute-path.json");
|
||||
|
||||
expect(() => {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
}).toThrow();
|
||||
|
||||
try {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch (error: unknown) {
|
||||
const execError = error as { stdout?: Buffer; stderr?: Buffer };
|
||||
const output = execError.stdout?.toString() || "";
|
||||
expect(output).toContain("Icon validation failed");
|
||||
expect(output).toContain("relative to the bundle root");
|
||||
}
|
||||
});
|
||||
|
||||
it("should reject missing icon files", () => {
|
||||
const manifestPath = join(testFixturesDir, "invalid-missing-file.json");
|
||||
|
||||
expect(() => {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
}).toThrow();
|
||||
|
||||
try {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch (error: unknown) {
|
||||
const execError = error as { stdout?: Buffer; stderr?: Buffer };
|
||||
const output = execError.stdout?.toString() || "";
|
||||
expect(output).toContain("Icon validation failed");
|
||||
expect(output).toContain("not found");
|
||||
}
|
||||
});
|
||||
|
||||
it("should reject non-PNG files", () => {
|
||||
const manifestPath = join(testFixturesDir, "invalid-non-png.json");
|
||||
|
||||
expect(() => {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
}).toThrow();
|
||||
|
||||
try {
|
||||
execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
stdio: "pipe",
|
||||
});
|
||||
} catch (error: unknown) {
|
||||
const execError = error as { stdout?: Buffer; stderr?: Buffer };
|
||||
const output = execError.stdout?.toString() || "";
|
||||
expect(output).toContain("Icon validation failed");
|
||||
expect(output).toContain("PNG format");
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("Edge cases", () => {
|
||||
it("should handle icons in subdirectories", () => {
|
||||
// Create subdirectory and icon
|
||||
const assetsDir = join(testFixturesDir, "assets");
|
||||
if (!fs.existsSync(assetsDir)) {
|
||||
fs.mkdirSync(assetsDir);
|
||||
}
|
||||
|
||||
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(assetsDir, "icon.png"), validPngBuffer);
|
||||
|
||||
createTestManifest("valid-subdirectory-icon.json", {
|
||||
icon: "assets/icon.png",
|
||||
});
|
||||
|
||||
const manifestPath = join(
|
||||
testFixturesDir,
|
||||
"valid-subdirectory-icon.json",
|
||||
);
|
||||
const result = execSync(`node ${cliPath} validate ${manifestPath}`, {
|
||||
encoding: "utf-8",
|
||||
});
|
||||
|
||||
expect(result).toContain("Manifest schema validation passes!");
|
||||
expect(result).toContain("Icon validation passed");
|
||||
});
|
||||
});
|
||||
});
|
||||
Loading…
Reference in a new issue