fix: handle drive root paths in image directory security check (#301)

* fix: handle drive root paths in image directory security check

When --image-dir is a drive root (e.g., E:\), the path already ends
with a separator. Appending another separator created a double-separator
prefix that never matched resolved paths, rejecting all valid localPath
values.

Closes #300

* test: fix path-validation stubs so happy-path tests assert success

The stub FigmaService was an empty object, so tests that passed path
validation immediately threw on downloadImages, fell into the catch
block, and only "passed" via a weak negative assertion. Give the stub a
downloadImages that returns [] so the handler completes normally, and
assert isError is undefined.

Author: GLips

src/mcp/tools/download-figma-images-tool.ts

@@ -95,7 +95,9 @@ async function downloadFigmaImages(
     // LLMs frequently produce paths like "/public/images" when they mean "public/images".
     const baseDir = imageDir ?? process.cwd();
     const resolvedPath = path.resolve(path.join(baseDir, localPath));
-    if (resolvedPath !== baseDir && !resolvedPath.startsWith(baseDir + path.sep)) {
+    // Drive roots (e.g. E:\) already end with a separator — avoid doubling it
+    const baseDirPrefix = baseDir.endsWith(path.sep) ? baseDir : baseDir + path.sep;
+    if (resolvedPath !== baseDir && !resolvedPath.startsWith(baseDirPrefix)) {
       return {
         isError: true,
         content: [

src/tests/path-validation.test.ts

@@ -3,7 +3,9 @@ import { describe, expect, it } from "vitest";
 import { downloadFigmaImagesTool } from "~/mcp/tools/download-figma-images-tool.js";
 import { downloadFigmaImage } from "~/utils/common.js";
 
-const stubFigmaService = {} as Parameters<typeof downloadFigmaImagesTool.handler>[1];
+const stubFigmaService = {
+  downloadImages: () => Promise.resolve([]),
+} as unknown as Parameters<typeof downloadFigmaImagesTool.handler>[1];
 
 const validParams = {
   fileKey: "abc123",
@@ -38,30 +40,37 @@ describe("download path validation", () => {
   });
 
   it("accepts valid relative path within imageDir", async () => {
-    // Will fail on the Figma API call — we only care that it doesn't
-    // return the path validation error.
     const result = await downloadFigmaImagesTool.handler(
       { ...validParams, localPath: "public/images" },
       stubFigmaService,
       imageDir,
     );
 
-    if (result.isError) {
-      expect(result.content[0].text).not.toContain("resolves outside the allowed image directory");
-    }
+    expect(result.isError).toBeUndefined();
+  });
+
+  it("accepts valid path when imageDir is a drive root", async () => {
+    // Windows drive roots (E:\, D:\) already end with a separator.
+    // path.resolve on posix treats this as a regular directory, which is fine
+    // — the logic under test is the prefix check, not actual Windows I/O.
+    const driveRoot = path.resolve("/");
+    const result = await downloadFigmaImagesTool.handler(
+      { ...validParams, localPath: "project/src/static/images/test" },
+      stubFigmaService,
+      driveRoot,
+    );
+
+    expect(result.isError).toBeUndefined();
   });
 
   it("accepts path with leading slash as relative", async () => {
-    // LLMs frequently produce paths like "/public/images" when they mean "public/images"
     const result = await downloadFigmaImagesTool.handler(
       { ...validParams, localPath: "/public/images" },
       stubFigmaService,
       imageDir,
     );
 
-    if (result.isError) {
-      expect(result.content[0].text).not.toContain("resolves outside the allowed image directory");
-    }
+    expect(result.isError).toBeUndefined();
   });
 });