fix(mcp): harden local file and token access

- Resolve configured root paths through symlinks before file operations

- Compare authentication tokens through fixed-length digests

CHANGELOG.md

@@ -26,6 +26,7 @@
 
 ### Fixed
 
+- Keep MCP file access inside its configured root even when paths contain symlinks, and harden local authentication token checks.
 - Keep desktop text visible across the scene and overlay canvases, refresh it after local fonts load, and preserve rendering when a requested italic face is unavailable (#395).
 - Honor node-scoped variable modes in `.fig` files so light and dark component examples keep their intended colors.
 - Preserve nested instance text, visibility, paint, geometry, and clipping across repeated children and component swaps in `.fig` files.

packages/mcp/src/auth.ts

@@ -1,3 +1,5 @@
+import { createHash, timingSafeEqual } from 'node:crypto'
+
 export function bearerToken(header: string | undefined | null): string | null {
   return header?.startsWith('Bearer ') ? header.slice('Bearer '.length) : null
 }
@@ -10,5 +12,10 @@ export function mcpRequestToken(
 }
 
 export function isAuthorized(provided: string | null, expected: string | null): boolean {
-  return expected === null || provided === expected
+  if (expected === null) return true
+  if (provided === null) return false
+
+  const providedDigest = createHash('sha256').update(provided, 'utf8').digest()
+  const expectedDigest = createHash('sha256').update(expected, 'utf8').digest()
+  return timingSafeEqual(providedDigest, expectedDigest)
 }

packages/mcp/src/tool/output.ts

@@ -1,16 +1,216 @@
-import { mkdir, writeFile } from 'node:fs/promises'
-import { dirname, resolve, sep as osSep } from 'node:path'
+import { lstat, mkdir, readlink, realpath, writeFile } from 'node:fs/promises'
+import { dirname, basename, isAbsolute, join, parse, resolve, sep as osSep } from 'node:path'
 
 import { ok } from '#mcp/result'
 import type { MCPResult } from '#mcp/result'
 
-export function resolveSafePath(filePath: string, root: string): string {
-  const resolved = resolve(filePath)
-  const sep = root.endsWith('/') || root.endsWith('\\') ? '' : osSep
-  if (!resolved.startsWith(root + sep) && resolved !== root) {
+/** Returns true for filesystem errors that indicate a missing or unresolvable path (ENOENT, ENOTDIR, ELOOP). */
+function isMissingPathError(e: unknown): boolean {
+  const code =
+    typeof e === 'object' && e !== null && 'code' in e ? (e as { code?: unknown }).code : undefined
+  return code === 'ENOENT' || code === 'ENOTDIR' || code === 'ELOOP'
+}
+
+/**
+ * Resolve a file path and verify it stays within the allowed root directory.
+ * Uses fs.realpath to resolve symlinks for the security check, preventing
+ * traversal attacks where a symlink inside root points outside root.
+ * Returns the resolved (normalized) path for display and file operations.
+ */
+
+/** Walk up from `p` until we find a path that realpath() can resolve. */
+async function resolveRealAncestor(
+  p: string
+): Promise<{ realAncestor: string; remainder: string }> {
+  let current = resolve(p)
+  let remainder = ''
+  let iterations = 0
+  do {
+    try {
+      const real = await realpath(current)
+      return { realAncestor: real, remainder }
+    } catch (e) {
+      if (!isMissingPathError(e)) throw e
+      const parent = dirname(current)
+      if (parent === current) return { realAncestor: current, remainder }
+      remainder = osSep + basename(current) + remainder
+      current = parent
+    }
+    iterations++
+  } while (iterations < 64) // depth limit to prevent infinite loops
+  // Fail closed: if we exceeded the depth limit, the path may contain
+  // unresolved symlink components (e.g., circular symlinks). Returning a
+  // partially-resolved path would allow it to pass containment checks
+  // even though the fully-resolved path could be outside root.
+  throw new Error(`Path resolution depth limit exceeded (possible circular symlinks): ${p}`)
+}
+
+/**
+ * Walks the non-existent path segments (remainder) from the real ancestor and
+ * rejects any that are symlinks. A symlink in a non-existent path segment
+ * could point outside the allowed root — when the file is eventually written,
+ * the OS follows the symlink chain and the write lands outside root.
+ */
+async function assertNoSymlinksInRemainder(realAncestor: string, remainder: string): Promise<void> {
+  if (!remainder) return
+  const segments = remainder.split(osSep).filter(Boolean)
+  let current = realAncestor
+  for (const seg of segments) {
+    current = join(current, seg)
+    try {
+      const stat = await lstat(current)
+      if (stat.isSymbolicLink()) {
+        throw new Error(`Path is outside the allowed root: symlink at ${current}`)
+      }
+    } catch (e) {
+      if (e instanceof Error && e.message.includes('outside the allowed root')) throw e
+      if (!isMissingPathError(e)) throw e
+      // lstat failed — the component doesn't exist at all, which is expected
+      // since we're creating a new file. No symlink to worry about.
+    }
+  }
+}
+
+/** Throw if `rootPath` is trivially broad (e.g. "/" or "C:\"). */
+function assertNarrowRoot(rootPath: string, original: string): void {
+  const normalized = resolve(rootPath)
+  const parsedRoot = parse(normalized).root
+  if (normalized === '/' || normalized === osSep || normalized === parsedRoot) {
+    throw new Error(
+      `Root path is too broad: "${original}" (resolved to "${normalized}"). ` +
+        'Specify a narrower OPENPENCIL_MCP_ROOT directory.'
+    )
+  }
+}
+
+/** Resolve `p` to its realpath-validated canonical form, handling non-existent paths. */
+async function resolveRealPath(p: string): Promise<string> {
+  try {
+    return await realpath(p)
+  } catch (e) {
+    if (!isMissingPathError(e)) throw e
+    const parentDir = dirname(p)
+    const baseName = basename(p)
+    try {
+      const realParent = await realpath(parentDir)
+      return join(realParent, baseName)
+    } catch (e) {
+      if (!isMissingPathError(e)) throw e
+      const { realAncestor, remainder } = await resolveRealAncestor(parentDir)
+      await assertNoSymlinksInRemainder(realAncestor, remainder)
+      return join(realAncestor, remainder.slice(osSep.length), baseName)
+    }
+  }
+}
+
+export interface SafePathResult {
+  /** The user-provided normalized path (for display/error messages). */
+  resolved: string
+  /** The canonical realpath-validated path (for filesystem operations). */
+  realPath: string
+}
+
+const MAX_SYMLINK_DEPTH = 16
+
+/**
+ * Resolve a dangling symlink's target. When realpath fails on a symlink,
+ * read the link target and recursively validate it is inside root.
+ * Returns the canonical realPath of the target, or undefined if `resolved`
+ * is not a symlink.
+ */
+async function resolveDanglingSymlink(
+  resolved: string,
+  root: string,
+  realRoot: string,
+  symlinkDepth: number
+): Promise<string | undefined> {
+  try {
+    const stat = await lstat(resolved)
+    if (!stat.isSymbolicLink()) return undefined
+    // Dangling symlink — realpath already failed, which means the
+    // target doesn't exist. Read the symlink target and validate it
+    // is inside root. Absolute-path targets pointing outside root are
+    // rejected. Relative targets are resolved relative to the symlink
+    // directory and checked against root.
+    const linkTarget = await readlink(resolved)
+    const linkDir = dirname(resolved)
+    const resolvedTarget = isAbsolute(linkTarget) ? linkTarget : resolve(linkDir, linkTarget)
+    // Recursively validate the target (handles nested symlinks).
+    // Pass the already-canonical realRoot so the recursive call skips
+    // redundant assertNarrowRoot + realpath(root) work.
+    const targetResult = await resolveSafePathInternal(
+      resolvedTarget,
+      root,
+      symlinkDepth + 1,
+      realRoot
+    )
+    return targetResult.realPath
+  } catch (e) {
+    if (e instanceof Error) {
+      // Re-throw security, root-scope, and depth-limit errors — these must
+      // not be swallowed. They indicate genuine validation failures, not
+      // the expected "path doesn't exist" case.
+      if (
+        e.message.includes('outside the allowed root') ||
+        e.message.includes('Root path is too broad') ||
+        e.message.includes('depth limit exceeded')
+      )
+        throw e
+    }
+    if (!isMissingPathError(e)) throw e
+    // lstat failed (file doesn't exist at all) — not a symlink
+    return undefined
+  }
+}
+
+async function resolveSafePathInternal(
+  filePath: string,
+  root: string,
+  symlinkDepth: number,
+  realRoot?: string
+): Promise<SafePathResult> {
+  if (symlinkDepth >= MAX_SYMLINK_DEPTH) {
+    throw new Error(
+      `Symlink resolution depth limit exceeded (possible circular symlinks): ${filePath}`
+    )
+  }
+
+  const normalizedRoot = resolve(root)
+  const resolved = isAbsolute(filePath) ? resolve(filePath) : resolve(normalizedRoot, filePath)
+
+  if (!realRoot) {
+    assertNarrowRoot(root, root)
+
+    try {
+      realRoot = await realpath(root)
+    } catch (e) {
+      if (!isMissingPathError(e)) throw e
+      const { realAncestor, remainder } = await resolveRealAncestor(root)
+      realRoot = remainder ? join(realAncestor, remainder.slice(osSep.length)) : realAncestor
+    }
+
+    assertNarrowRoot(realRoot, root)
+  }
+
+  const realSep = realRoot.endsWith('/') || realRoot.endsWith('\\') ? '' : osSep
+
+  let realPath: string
+  try {
+    realPath = await realpath(resolved)
+  } catch (e) {
+    if (!isMissingPathError(e)) throw e
+    const symlinkRealPath = await resolveDanglingSymlink(resolved, root, realRoot, symlinkDepth)
+    realPath = symlinkRealPath ?? (await resolveRealPath(resolved))
+  }
+
+  if (!realPath.startsWith(realRoot + realSep) && realPath !== realRoot) {
     throw new Error(`Path is outside the allowed root: ${root}`)
   }
-  return resolved
+  return { resolved, realPath }
+}
+
+export async function resolveSafePath(filePath: string, root: string): Promise<SafePathResult> {
+  return resolveSafePathInternal(filePath, root, 0)
 }
 
 export async function writeToolOutput(
@@ -19,18 +219,32 @@ export async function writeToolOutput(
   filePath: string,
   root: string
 ): Promise<MCPResult | null> {
-  const resolved = resolveSafePath(filePath, root)
-  await mkdir(dirname(resolved), { recursive: true })
+  const { resolved, realPath } = await resolveSafePath(filePath, root)
+  // Use the canonical realPath for filesystem operations to prevent TOCTOU:
+  // an attacker could swap a directory component with a symlink between
+  // resolveSafePath's validation and the writeFile call. Writing to the
+  // realpath-validated path ensures the write lands inside root.
+  const parentDir = dirname(realPath)
+  await mkdir(parentDir, { recursive: true })
+  // TOCTOU mitigation: re-verify the parent directory still resolves inside
+  // the root after mkdir. An attacker who replaces an ancestor directory
+  // with a symlink between resolveSafePath and the write would be detected
+  // here — realpath follows the ancestor symlink and resolves outside root.
+  await resolveSafePath(parentDir, root)
   if (toolName === 'export_svg' && typeof result.svg === 'string') {
-    await writeFile(resolved, result.svg, 'utf8')
+    await writeFile(realPath, result.svg, 'utf8')
+    await resolveSafePath(realPath, root)
     return ok({ written: resolved, byteLength: Buffer.byteLength(result.svg, 'utf8') })
   }
   if (toolName === 'export_image' && typeof result.base64 === 'string') {
-    await writeFile(resolved, Buffer.from(result.base64, 'base64'))
-    return ok({ written: resolved, byteLength: result.byteLength ?? null })
+    const buffer = Buffer.from(result.base64, 'base64')
+    await writeFile(realPath, buffer)
+    await resolveSafePath(realPath, root)
+    return ok({ written: resolved, byteLength: buffer.length })
   }
   if (toolName === 'get_jsx' && typeof result.jsx === 'string') {
-    await writeFile(resolved, result.jsx, 'utf8')
+    await writeFile(realPath, result.jsx, 'utf8')
+    await resolveSafePath(realPath, root)
     return ok({ written: resolved, byteLength: Buffer.byteLength(result.jsx, 'utf8') })
   }
   return null

packages/mcp/src/tool/registration.ts

@@ -130,7 +130,11 @@ export function registerTools(mcpServer: McpServer, options: RegisterToolsOption
         : 'Save the current document to disk. Uses the existing file path if available, otherwise prompts for a location.',
       inputSchema: resolvedRoot
         ? z.object({
-            path: z.string().describe('Optional absolute path for the .fig file').optional(),
+            path: z
+              .string()
+              .min(1)
+              .describe('Path for the .fig file, absolute or relative to the MCP root')
+              .optional(),
             ...automationTargetSchema
           })
         : z.object({ ...automationTargetSchema })
@@ -138,14 +142,19 @@ export function registerTools(mcpServer: McpServer, options: RegisterToolsOption
     async (args: { path?: string; document_id?: string; page_id?: string }) => {
       try {
         const safePath =
-          args.path && resolvedRoot ? resolveSafePath(args.path, resolvedRoot) : undefined
+          args.path !== undefined && resolvedRoot
+            ? await resolveSafePath(args.path, resolvedRoot)
+            : undefined
         const { target } = splitAutomationTarget(args)
-        const result = await sendRpc({ command: 'save_file', args: { ...target, path: safePath } })
+        const result = await sendRpc({
+          command: 'save_file',
+          args: { ...target, path: safePath?.realPath }
+        })
         const res = result as { ok?: boolean; result?: unknown; target?: unknown; error?: string }
         if (res.ok === false) return fail(new Error(res.error))
         return ok({
           saved: true,
-          ...(safePath ? { path: safePath } : {}),
+          ...(safePath ? { path: safePath.resolved } : {}),
           ...(res.target ? { target: res.target } : {})
         })
       } catch (e) {
@@ -160,15 +169,21 @@ export function registerTools(mcpServer: McpServer, options: RegisterToolsOption
       {
         description: `Open a .fig or .pen file from disk into a new tab. Path must be inside ${resolvedRoot}.`,
         inputSchema: z.object({
-          path: z.string().describe('Absolute path to the design file'),
+          path: z
+            .string()
+            .min(1)
+            .describe('Path to the design file, absolute or relative to the MCP root'),
           ...automationTargetSchema
         })
       },
       async (args: { path: string; document_id?: string; page_id?: string }) => {
         try {
-          const safe = resolveSafePath(args.path, resolvedRoot)
+          const safe = await resolveSafePath(args.path, resolvedRoot)
           const { target } = splitAutomationTarget(args)
-          const result = await sendRpc({ command: 'open_file', args: { ...target, path: safe } })
+          const result = await sendRpc({
+            command: 'open_file',
+            args: { ...target, path: safe.realPath }
+          })
           const res = result as { ok?: boolean; result?: unknown; target?: unknown; error?: string }
           if (res.ok === false) return fail(new Error(res.error))
           return ok({ opened: true, ...(res.target ? { target: res.target } : {}) })
@@ -183,17 +198,22 @@ export function registerTools(mcpServer: McpServer, options: RegisterToolsOption
       {
         description: `Create a new empty document. Optionally set a save path inside ${resolvedRoot}.`,
         inputSchema: z.object({
-          path: z.string().describe('Optional absolute path for the new file').optional(),
+          path: z
+            .string()
+            .min(1)
+            .describe('Path for the new file, absolute or relative to the MCP root')
+            .optional(),
           ...automationTargetSchema
         })
       },
       async (args: { path?: string; document_id?: string; page_id?: string }) => {
         try {
-          const safePath = args.path ? resolveSafePath(args.path, resolvedRoot) : undefined
+          const safePath =
+            args.path !== undefined ? await resolveSafePath(args.path, resolvedRoot) : undefined
           const { target } = splitAutomationTarget(args)
           const result = await sendRpc({
             command: 'new_document',
-            args: { ...target, path: safePath }
+            args: { ...target, path: safePath?.realPath }
           })
           const res = result as { ok?: boolean; result?: unknown; target?: unknown; error?: string }
           if (res.ok === false) return fail(new Error(res.error))

tests/engine/mcp/auth.test.ts

@@ -0,0 +1,23 @@
+import { describe, expect, test } from 'bun:test'
+
+import { bearerToken, isAuthorized, mcpRequestToken } from '#mcp/auth'
+
+describe('MCP authentication', () => {
+  test('accepts exact tokens and rejects mismatches', () => {
+    expect(isAuthorized('secret-token', 'secret-token')).toBe(true)
+    expect(isAuthorized('secret-token-x', 'secret-token')).toBe(false)
+    expect(isAuthorized(null, 'secret-token')).toBe(false)
+  })
+
+  test('allows requests when authentication is disabled', () => {
+    expect(isAuthorized(null, null)).toBe(true)
+    expect(isAuthorized('unused', null)).toBe(true)
+  })
+
+  test('extracts bearer and fallback MCP tokens', () => {
+    expect(bearerToken('Bearer secret-token')).toBe('secret-token')
+    expect(bearerToken('Basic secret-token')).toBeNull()
+    expect(mcpRequestToken('Bearer bearer-token', 'header-token')).toBe('bearer-token')
+    expect(mcpRequestToken(undefined, 'header-token')).toBe('header-token')
+  })
+})

tests/engine/mcp/path-scoping.test.ts

@@ -1,48 +1,327 @@
 import { describe, test, expect } from 'bun:test'
-import { resolve, sep } from 'node:path'
+import { randomUUID } from 'node:crypto'
+import { mkdir, realpath, rm, symlink, writeFile } from 'node:fs/promises'
+import { tmpdir } from 'node:os'
+import { join, resolve } from 'node:path'
 
-// Test the resolveSafePath logic directly (extracted for testability)
-function resolveSafePath(filePath: string, root: string): string {
-  const resolved = resolve(filePath)
-  const normalizedSep = root.endsWith('/') || root.endsWith('\\') ? '' : sep
-  if (!resolved.startsWith(root + normalizedSep) && resolved !== root) {
-    throw new Error(`Path is outside the allowed root: ${root}`)
-  }
-  return resolved
-}
+import { resolveSafePath } from '#mcp/tool/output'
+
+const isUnix = process.platform !== 'win32'
+const TEST_ID = randomUUID().slice(0, 8)
 
 describe('MCP path scoping', () => {
-  const root = resolve('/tmp/mcp-test-root')
+  const root = resolve(tmpdir(), 'mcp-' + TEST_ID + '-test-root')
 
-  test('allows path inside root', () => {
-    expect(resolveSafePath(`${root}/design.fig`, root)).toBe(`${root}/design.fig`)
+  test('allows path inside root', async () => {
+    try {
+      await mkdir(root, { recursive: true })
+      const result = await resolveSafePath(`${root}/design.fig`, root)
+      expect(result.resolved).toBe(resolve(`${root}/design.fig`))
+      // realPath is the canonical form (realpath-resolved), which may differ
+      // from resolve() on macOS where /var -> /private/var
+      const canonicalRoot = await realpath(root)
+      expect(result.realPath).toBe(join(canonicalRoot, 'design.fig'))
+    } finally {
+      await rm(root, { recursive: true, force: true })
+    }
   })
 
-  test('allows nested path inside root', () => {
-    expect(resolveSafePath(`${root}/sub/dir/file.fig`, root)).toBe(`${root}/sub/dir/file.fig`)
+  test('resolves relative path inside root', async () => {
+    const result = await resolveSafePath('design.fig', root)
+    expect(result.resolved).toBe(resolve(`${root}/design.fig`))
   })
 
-  test('allows root itself', () => {
-    expect(resolveSafePath(root, root)).toBe(root)
+  test('resolves nested relative path inside root', async () => {
+    const result = await resolveSafePath('sub/dir/file.fig', root)
+    expect(result.resolved).toBe(resolve(`${root}/sub/dir/file.fig`))
   })
 
-  test('rejects path outside root', () => {
-    expect(() => resolveSafePath('/etc/passwd', root)).toThrow('outside the allowed root')
+  test('allows nested path inside root', async () => {
+    const result = await resolveSafePath(`${root}/sub/dir/file.fig`, root)
+    expect(result.resolved).toBe(resolve(`${root}/sub/dir/file.fig`))
   })
 
-  test('rejects path traversal', () => {
-    expect(() => resolveSafePath(`${root}/../../../etc/passwd`, root)).toThrow(
+  test('allows root itself', async () => {
+    const result = await resolveSafePath(root, root)
+    expect(result.resolved).toBe(root)
+  })
+
+  test('rejects path outside root', async () => {
+    const outsideRoot = resolve(tmpdir(), 'mcp-' + TEST_ID + '-outside-root')
+    await expect(resolveSafePath(`${outsideRoot}/passwd`, root)).rejects.toThrow(
       'outside the allowed root'
     )
   })
 
-  test('rejects sibling directory', () => {
-    expect(() => resolveSafePath(`${root}/../other-root/file.fig`, root)).toThrow(
+  test('rejects path traversal', async () => {
+    await expect(resolveSafePath(`${root}/../../../etc/passwd`, root)).rejects.toThrow(
       'outside the allowed root'
     )
   })
 
-  test('rejects root prefix trick (root-evil)', () => {
-    expect(() => resolveSafePath(`${root}-evil/file.fig`, root)).toThrow('outside the allowed root')
+  test('rejects sibling directory', async () => {
+    await expect(resolveSafePath(`${root}/../other-root/file.fig`, root)).rejects.toThrow(
+      'outside the allowed root'
+    )
+  })
+
+  test('rejects root prefix trick (root-evil)', async () => {
+    await expect(resolveSafePath(`${root}-evil/file.fig`, root)).rejects.toThrow(
+      'outside the allowed root'
+    )
+  })
+
+  test.skipIf(!isUnix)('rejects non-dangling symlink pointing outside root', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-test')
+    const linkPath = `${testDir}/escape.fig`
+    const outsideTarget = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-outside-target')
+    try {
+      await mkdir(testDir, { recursive: true })
+      // Create the target so the symlink is NOT dangling — this exercises
+      // the realpath → outside-root branch rather than the dangling path.
+      await mkdir(outsideTarget, { recursive: true })
+      await symlink(outsideTarget, linkPath)
+      await expect(resolveSafePath(linkPath, testDir)).rejects.toThrow('outside the allowed root')
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+      await rm(outsideTarget, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('allows dangling symlink pointing inside root', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-dangling-inside-test')
+    const linkPath = `${testDir}/dangling.fig`
+    const insideTarget = `${testDir}/nonexistent-target.fig`
+    try {
+      await mkdir(testDir, { recursive: true })
+      // Dangling symlink: target doesn't exist, but points inside root.
+      // This is a legitimate use case — the target will be created by the
+      // write operation. The target path is validated to be inside root.
+      await symlink(insideTarget, linkPath)
+      const result = await resolveSafePath(linkPath, testDir)
+      expect(result.resolved).toBe(resolve(linkPath))
+      // realPath should resolve to the target's canonical parent + filename
+      const canonicalParent = await realpath(testDir)
+      expect(result.realPath).toBe(join(canonicalParent, 'nonexistent-target.fig'))
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('allows symlink pointing inside root', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-safe-test')
+    const targetDir = `${testDir}/targets`
+    const targetFile = `${targetDir}/real.fig`
+    const linkPath = `${testDir}/link.fig`
+    try {
+      await mkdir(targetDir, { recursive: true })
+      await writeFile(targetFile, 'test')
+      await symlink(targetFile, linkPath)
+      // Returns both resolved (for display) and realPath (canonical, for writes).
+      // The resolved path is the user-provided normalized path for usability.
+      const result = await resolveSafePath(linkPath, testDir)
+      expect(result.resolved).toBe(resolve(linkPath))
+      // realPath must be the canonical target (not the symlink itself)
+      const canonicalTarget = await realpath(targetFile)
+      expect(result.realPath).toBe(canonicalTarget)
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('rejects dangling symlink pointing outside root', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-dangling-test')
+    const linkPath = `${testDir}/dangling.fig`
+    const outsideTarget = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-dangling-outside')
+    try {
+      await mkdir(testDir, { recursive: true })
+      // Dangling symlink: target doesn't exist, points outside root
+      await symlink(outsideTarget, linkPath)
+      await expect(resolveSafePath(linkPath, testDir)).rejects.toThrow('outside the allowed root')
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test('allows nonexistent file inside root', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-nonexistent-test')
+    const filePath = `${testDir}/new.fig`
+    try {
+      await mkdir(testDir, { recursive: true })
+      // File doesn't exist yet (common for save_file / export operations)
+      const result = await resolveSafePath(filePath, testDir)
+      expect(result.resolved).toBe(resolve(filePath))
+      // realPath uses the canonical parent directory + filename
+      const canonicalParent = await realpath(testDir)
+      expect(result.realPath).toBe(join(canonicalParent, 'new.fig'))
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test('rejects trivially broad root "/"', async () => {
+    await expect(resolveSafePath('/some/path', '/')).rejects.toThrow('Root path is too broad')
+  })
+
+  test.skipIf(process.platform !== 'win32')(
+    'rejects drive root as broad root on Windows',
+    async () => {
+      // On Windows, path.parse('C:\\').root === 'C:\\', which resolveSafePath
+      // should reject. This test runs only on Windows where drive roots exist.
+      const { parse, resolve: winResolve } = await import('node:path')
+      const driveRoot = parse(winResolve('C:\\')).root
+      await expect(resolveSafePath('C:\\some\\path', driveRoot)).rejects.toThrow(
+        'Root path is too broad'
+      )
+    }
+  )
+
+  test('rejects path exceeding resolution depth limit', async () => {
+    // resolveRealAncestor has a 64-iteration depth cap. When the entire
+    // ancestor chain doesn't exist (e.g., a deep non-existent root path),
+    // the function must fail closed rather than returning a partially-resolved
+    // path that could bypass containment checks.
+    const deepSegments = Array.from({ length: 70 }, () => 'sub').join('/')
+    const deepRoot = resolve(tmpdir(), deepSegments)
+    await expect(resolveSafePath(`${deepRoot}/file.fig`, deepRoot)).rejects.toThrow(
+      'depth limit exceeded'
+    )
+  })
+
+  test('rejects deep file path exceeding resolution depth limit with existing root', async () => {
+    // When the root exists but the file path has >64 non-existent ancestor
+    // segments, resolveRealAncestor is called for the parent directory and
+    // must fail closed. This exercises the second call site (line 142 in
+    // output.ts: resolveRealAncestor(parentDir)).
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-deep-path-test')
+    try {
+      await mkdir(testDir, { recursive: true })
+      // 70 non-existent subdirectories under an existing root
+      const deepFile = Array.from({ length: 70 }, () => 'sub').join('/') + '/file.fig'
+      await expect(resolveSafePath(deepFile, testDir)).rejects.toThrow('depth limit exceeded')
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('rejects symlinked root that resolves to /', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-root-test')
+    const rootLink = `${testDir}/root-link`
+    try {
+      await mkdir(testDir, { recursive: true })
+      await symlink('/', rootLink)
+      await expect(resolveSafePath(`${rootLink}/file.fig`, rootLink)).rejects.toThrow(
+        'Root path is too broad'
+      )
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('rejects circular symlinks', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-circular-test')
+    const linkA = `${testDir}/a.fig`
+    const linkB = `${testDir}/b.fig`
+    try {
+      await mkdir(testDir, { recursive: true })
+      // Circular symlink chain: a -> b -> a
+      await symlink(linkB, linkA)
+      await symlink(linkA, linkB)
+      await expect(resolveSafePath(linkA, testDir)).rejects.toThrow('depth limit exceeded')
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('allows absolute symlink target inside root', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-abs-inside-test')
+    const targetDir = `${testDir}/targets`
+    const targetFile = `${targetDir}/real.fig`
+    const linkPath = `${testDir}/link.fig`
+    try {
+      await mkdir(targetDir, { recursive: true })
+      await writeFile(targetFile, 'test')
+      // Absolute symlink target pointing inside root
+      await symlink(targetFile, linkPath)
+      const result = await resolveSafePath(linkPath, testDir)
+      expect(result.resolved).toBe(resolve(linkPath))
+      const canonicalTarget = await realpath(targetFile)
+      expect(result.realPath).toBe(canonicalTarget)
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('rejects absolute symlink target outside root', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-abs-outside-test')
+    const outsideDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-abs-outside-target')
+    const outsideFile = `${outsideDir}/secret.fig`
+    const linkPath = `${testDir}/link.fig`
+    try {
+      await mkdir(testDir, { recursive: true })
+      await mkdir(outsideDir, { recursive: true })
+      await writeFile(outsideFile, 'secret')
+      // Absolute symlink target pointing outside root
+      await symlink(outsideFile, linkPath)
+      await expect(resolveSafePath(linkPath, testDir)).rejects.toThrow('outside the allowed root')
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+      await rm(outsideDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)(
+    'allows relative symlink with traversal that stays inside root',
+    async () => {
+      const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-rel-traversal-test')
+      const subDir = `${testDir}/sub`
+      const targetFile = `${testDir}/target.fig`
+      const linkPath = `${subDir}/link.fig`
+      try {
+        await mkdir(subDir, { recursive: true })
+        await writeFile(targetFile, 'test')
+        // Relative symlink: sub/link.fig -> ../target.fig (resolves inside root)
+        await symlink('../target.fig', linkPath)
+        const result = await resolveSafePath(linkPath, testDir)
+        expect(result.resolved).toBe(resolve(linkPath))
+        const canonicalTarget = await realpath(targetFile)
+        expect(result.realPath).toBe(canonicalTarget)
+      } finally {
+        await rm(testDir, { recursive: true, force: true })
+      }
+    }
+  )
+
+  test.skipIf(!isUnix)('rejects self-referencing symlink', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-self-test')
+    const linkPath = `${testDir}/self.fig`
+    try {
+      await mkdir(testDir, { recursive: true })
+      // Self-referencing symlink: link.fig -> link.fig
+      await symlink(linkPath, linkPath)
+      await expect(resolveSafePath(linkPath, testDir)).rejects.toThrow('depth limit exceeded')
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
+  })
+
+  test.skipIf(!isUnix)('allows symlink chain inside root (non-dangling)', async () => {
+    const testDir = resolve(tmpdir(), 'mcp-' + TEST_ID + '-symlink-chain-test')
+    const targetFile = `${testDir}/real.fig`
+    const link1 = `${testDir}/link1.fig`
+    const link2 = `${testDir}/link2.fig`
+    try {
+      await mkdir(testDir, { recursive: true })
+      await writeFile(targetFile, 'test')
+      // Chain: link2 -> link1 -> real.fig
+      await symlink(targetFile, link1)
+      await symlink(link1, link2)
+      const result = await resolveSafePath(link2, testDir)
+      expect(result.resolved).toBe(resolve(link2))
+      const canonicalTarget = await realpath(targetFile)
+      expect(result.realPath).toBe(canonicalTarget)
+    } finally {
+      await rm(testDir, { recursive: true, force: true })
+    }
   })
 })

tests/engine/mcp/server.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, test, beforeEach, afterEach } from 'bun:test'
+import { mkdir, realpath } from 'node:fs/promises'
 import type { AddressInfo } from 'node:net'
 import { tmpdir } from 'node:os'
 import path from 'node:path'
@@ -483,6 +484,7 @@ describe('MCP server with mcpRoot', () => {
     await client.connect(transport)
 
     const savePath = path.join(TEST_MCP_ROOT, 'unicode', 'пример.fig')
+    await mkdir(path.dirname(savePath), { recursive: true })
     const result = await client.callTool({
       name: 'save_file',
       arguments: { path: savePath }
@@ -490,7 +492,8 @@ describe('MCP server with mcpRoot', () => {
 
     expect(result.isError).not.toBe(true)
     const request = browser.requests.find((item) => item.command === 'save_file')
-    expect(request?.args).toEqual({ path: savePath })
+    const canonicalPath = path.join(await realpath(path.dirname(savePath)), path.basename(savePath))
+    expect(request?.args).toEqual({ path: canonicalPath })
 
     await client.close()
     browser.close()