Merge pull request #3054 from code-yeongyu/fix/p0-6-mcp-local-scope-subdirectory
Fix local MCP scope loading in subdirectories
This commit is contained in:
@@ -1,17 +1,6 @@
|
|||||||
import { existsSync, realpathSync } from "fs"
|
import { containsPath } from "../../shared/contains-path"
|
||||||
import { resolve } from "path"
|
|
||||||
import type { ClaudeCodeMcpServer } from "./types"
|
import type { ClaudeCodeMcpServer } from "./types"
|
||||||
|
|
||||||
function normalizePath(path: string): string {
|
|
||||||
const resolvedPath = resolve(path)
|
|
||||||
|
|
||||||
if (!existsSync(resolvedPath)) {
|
|
||||||
return resolvedPath
|
|
||||||
}
|
|
||||||
|
|
||||||
return realpathSync(resolvedPath)
|
|
||||||
}
|
|
||||||
|
|
||||||
export function shouldLoadMcpServer(
|
export function shouldLoadMcpServer(
|
||||||
server: Pick<ClaudeCodeMcpServer, "scope" | "projectPath">,
|
server: Pick<ClaudeCodeMcpServer, "scope" | "projectPath">,
|
||||||
cwd = process.cwd()
|
cwd = process.cwd()
|
||||||
@@ -24,5 +13,5 @@ export function shouldLoadMcpServer(
|
|||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|
||||||
return normalizePath(server.projectPath) === normalizePath(cwd)
|
return containsPath(server.projectPath, cwd)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, it, mock } from "bun:test"
|
|||||||
import { mkdirSync, rmSync, writeFileSync } from "fs"
|
import { mkdirSync, rmSync, writeFileSync } from "fs"
|
||||||
import { tmpdir } from "os"
|
import { tmpdir } from "os"
|
||||||
import { join } from "path"
|
import { join } from "path"
|
||||||
|
import { shouldLoadMcpServer } from "./scope-filter"
|
||||||
|
|
||||||
const TEST_DIR = join(tmpdir(), `mcp-scope-filtering-test-${Date.now()}`)
|
const TEST_DIR = join(tmpdir(), `mcp-scope-filtering-test-${Date.now()}`)
|
||||||
const TEST_HOME = join(TEST_DIR, "home")
|
const TEST_HOME = join(TEST_DIR, "home")
|
||||||
@@ -27,6 +28,56 @@ describe("loadMcpConfigs", () => {
|
|||||||
rmSync(TEST_DIR, { recursive: true, force: true })
|
rmSync(TEST_DIR, { recursive: true, force: true })
|
||||||
})
|
})
|
||||||
|
|
||||||
|
describe("#given local MCP scope checks", () => {
|
||||||
|
it("#when cwd exactly matches project path #then the server is loaded", () => {
|
||||||
|
const result = shouldLoadMcpServer(
|
||||||
|
{
|
||||||
|
scope: "local",
|
||||||
|
projectPath: "/tmp/repo",
|
||||||
|
},
|
||||||
|
"/tmp/repo"
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when cwd is a subdirectory of project path #then the server is loaded", () => {
|
||||||
|
const result = shouldLoadMcpServer(
|
||||||
|
{
|
||||||
|
scope: "local",
|
||||||
|
projectPath: "/tmp/repo",
|
||||||
|
},
|
||||||
|
"/tmp/repo/packages/app"
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when cwd does not overlap project path #then the server is not loaded", () => {
|
||||||
|
const result = shouldLoadMcpServer(
|
||||||
|
{
|
||||||
|
scope: "local",
|
||||||
|
projectPath: "/tmp/repo",
|
||||||
|
},
|
||||||
|
"/tmp/other"
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result).toBe(false)
|
||||||
|
})
|
||||||
|
|
||||||
|
it("#when cwd is the parent of project path #then the server is not loaded", () => {
|
||||||
|
const result = shouldLoadMcpServer(
|
||||||
|
{
|
||||||
|
scope: "local",
|
||||||
|
projectPath: "/tmp/repo",
|
||||||
|
},
|
||||||
|
"/tmp"
|
||||||
|
)
|
||||||
|
|
||||||
|
expect(result).toBe(false)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
describe("#given user-scoped MCP entries with local scope metadata", () => {
|
describe("#given user-scoped MCP entries with local scope metadata", () => {
|
||||||
it("#when loading configs #then only servers matching the current project path are loaded", async () => {
|
it("#when loading configs #then only servers matching the current project path are loaded", async () => {
|
||||||
writeFileSync(
|
writeFileSync(
|
||||||
|
|||||||
@@ -6,12 +6,14 @@ import type { LoadedPlugin } from "./types"
|
|||||||
|
|
||||||
const TEST_DIR = join(tmpdir(), `plugin-mcp-loader-test-${Date.now()}`)
|
const TEST_DIR = join(tmpdir(), `plugin-mcp-loader-test-${Date.now()}`)
|
||||||
const PROJECT_DIR = join(TEST_DIR, "project")
|
const PROJECT_DIR = join(TEST_DIR, "project")
|
||||||
|
const PROJECT_SUBDIRECTORY = join(PROJECT_DIR, "packages", "app")
|
||||||
const PLUGIN_DIR = join(TEST_DIR, "plugin")
|
const PLUGIN_DIR = join(TEST_DIR, "plugin")
|
||||||
const MCP_CONFIG_PATH = join(PLUGIN_DIR, "mcp.json")
|
const MCP_CONFIG_PATH = join(PLUGIN_DIR, "mcp.json")
|
||||||
|
|
||||||
describe("loadPluginMcpServers", () => {
|
describe("loadPluginMcpServers", () => {
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
mkdirSync(PROJECT_DIR, { recursive: true })
|
mkdirSync(PROJECT_DIR, { recursive: true })
|
||||||
|
mkdirSync(PROJECT_SUBDIRECTORY, { recursive: true })
|
||||||
mkdirSync(PLUGIN_DIR, { recursive: true })
|
mkdirSync(PLUGIN_DIR, { recursive: true })
|
||||||
mock.module("../../shared/logger", () => ({
|
mock.module("../../shared/logger", () => ({
|
||||||
log: () => {},
|
log: () => {},
|
||||||
@@ -24,7 +26,7 @@ describe("loadPluginMcpServers", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
describe("#given plugin MCP entries with local scope metadata", () => {
|
describe("#given plugin MCP entries with local scope metadata", () => {
|
||||||
it("#when loading plugin MCP servers #then only entries matching the current cwd are included", async () => {
|
it("#when loading plugin MCP servers from a project subdirectory #then only entries within the same project are included", async () => {
|
||||||
writeFileSync(
|
writeFileSync(
|
||||||
MCP_CONFIG_PATH,
|
MCP_CONFIG_PATH,
|
||||||
JSON.stringify({
|
JSON.stringify({
|
||||||
@@ -45,6 +47,12 @@ describe("loadPluginMcpServers", () => {
|
|||||||
scope: "local",
|
scope: "local",
|
||||||
projectPath: join(PROJECT_DIR, "other-project"),
|
projectPath: join(PROJECT_DIR, "other-project"),
|
||||||
},
|
},
|
||||||
|
parentLocal: {
|
||||||
|
command: "npx",
|
||||||
|
args: ["parent-plugin-local"],
|
||||||
|
scope: "local",
|
||||||
|
projectPath: join(PROJECT_SUBDIRECTORY, "nested-project"),
|
||||||
|
},
|
||||||
},
|
},
|
||||||
})
|
})
|
||||||
)
|
)
|
||||||
@@ -59,7 +67,7 @@ describe("loadPluginMcpServers", () => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const originalCwd = process.cwd()
|
const originalCwd = process.cwd()
|
||||||
process.chdir(PROJECT_DIR)
|
process.chdir(PROJECT_SUBDIRECTORY)
|
||||||
|
|
||||||
try {
|
try {
|
||||||
const { loadPluginMcpServers } = await import("./mcp-server-loader")
|
const { loadPluginMcpServers } = await import("./mcp-server-loader")
|
||||||
@@ -68,6 +76,7 @@ describe("loadPluginMcpServers", () => {
|
|||||||
expect(servers).toHaveProperty("demo-plugin:globalServer")
|
expect(servers).toHaveProperty("demo-plugin:globalServer")
|
||||||
expect(servers).toHaveProperty("demo-plugin:matchingLocal")
|
expect(servers).toHaveProperty("demo-plugin:matchingLocal")
|
||||||
expect(servers).not.toHaveProperty("demo-plugin:nonMatchingLocal")
|
expect(servers).not.toHaveProperty("demo-plugin:nonMatchingLocal")
|
||||||
|
expect(servers).not.toHaveProperty("demo-plugin:parentLocal")
|
||||||
} finally {
|
} finally {
|
||||||
process.chdir(originalCwd)
|
process.chdir(originalCwd)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,10 @@ import { spawn } from "bun";
|
|||||||
import { validateArchiveEntries, type ArchiveEntry } from "./archive-entry-validator";
|
import { validateArchiveEntries, type ArchiveEntry } from "./archive-entry-validator";
|
||||||
import { extractZip } from "./zip-extractor";
|
import { extractZip } from "./zip-extractor";
|
||||||
|
|
||||||
|
function isTarTraversalErrorOutput(output: string): boolean {
|
||||||
|
return /path contains '\.\.'|member name contains '\.\.'|removing leading [`'\"]?\.\.\//i.test(output)
|
||||||
|
}
|
||||||
|
|
||||||
export function getCachedBinaryPath(cacheDir: string, binaryName: string): string | null {
|
export function getCachedBinaryPath(cacheDir: string, binaryName: string): string | null {
|
||||||
const binaryPath = path.join(cacheDir, binaryName);
|
const binaryPath = path.join(cacheDir, binaryName);
|
||||||
return existsSync(binaryPath) ? binaryPath : null;
|
return existsSync(binaryPath) ? binaryPath : null;
|
||||||
@@ -43,6 +47,11 @@ export async function extractTarGz(
|
|||||||
const exitCode = await proc.exited;
|
const exitCode = await proc.exited;
|
||||||
if (exitCode !== 0) {
|
if (exitCode !== 0) {
|
||||||
const stderr = await new Response(proc.stderr).text();
|
const stderr = await new Response(proc.stderr).text();
|
||||||
|
|
||||||
|
if (isTarTraversalErrorOutput(stderr)) {
|
||||||
|
throw new Error(`Unsafe archive entry: path contains path traversal (${archivePath})`)
|
||||||
|
}
|
||||||
|
|
||||||
throw new Error(`tar extraction failed (exit ${exitCode}): ${stderr}`);
|
throw new Error(`tar extraction failed (exit ${exitCode}): ${stderr}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -102,6 +111,10 @@ async function listTarEntries(archivePath: string, cwd?: string): Promise<Archiv
|
|||||||
new Response(proc.stderr).text(),
|
new Response(proc.stderr).text(),
|
||||||
])
|
])
|
||||||
|
|
||||||
|
if (isTarTraversalErrorOutput(stderr)) {
|
||||||
|
throw new Error(`Unsafe archive entry: path contains path traversal (${archivePath})`)
|
||||||
|
}
|
||||||
|
|
||||||
if (exitCode !== 0) {
|
if (exitCode !== 0) {
|
||||||
throw new Error(`tar entry listing failed (exit ${exitCode}): ${stderr}`)
|
throw new Error(`tar entry listing failed (exit ${exitCode}): ${stderr}`)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,6 +1,22 @@
|
|||||||
import { existsSync, realpathSync } from "fs"
|
import { existsSync, realpathSync } from "fs"
|
||||||
import { basename, dirname, isAbsolute, join, normalize, relative, resolve } from "path"
|
import { basename, dirname, isAbsolute, join, normalize, relative, resolve } from "path"
|
||||||
|
|
||||||
|
function findNearestExistingAncestor(resolvedPath: string): string {
|
||||||
|
let candidatePath = resolvedPath
|
||||||
|
|
||||||
|
while (!existsSync(candidatePath)) {
|
||||||
|
const parentPath = dirname(candidatePath)
|
||||||
|
|
||||||
|
if (parentPath === candidatePath) {
|
||||||
|
return candidatePath
|
||||||
|
}
|
||||||
|
|
||||||
|
candidatePath = parentPath
|
||||||
|
}
|
||||||
|
|
||||||
|
return candidatePath
|
||||||
|
}
|
||||||
|
|
||||||
function toCanonicalPath(pathToNormalize: string): string {
|
function toCanonicalPath(pathToNormalize: string): string {
|
||||||
const resolvedPath = resolve(pathToNormalize)
|
const resolvedPath = resolve(pathToNormalize)
|
||||||
|
|
||||||
@@ -12,12 +28,13 @@ function toCanonicalPath(pathToNormalize: string): string {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
const parentDirectory = dirname(resolvedPath)
|
const nearestExistingAncestor = findNearestExistingAncestor(resolvedPath)
|
||||||
const canonicalParentDirectory = existsSync(parentDirectory)
|
const canonicalAncestor = existsSync(nearestExistingAncestor)
|
||||||
? realpathSync.native(parentDirectory)
|
? realpathSync.native(nearestExistingAncestor)
|
||||||
: parentDirectory
|
: nearestExistingAncestor
|
||||||
|
const relativePathFromAncestor = relative(nearestExistingAncestor, resolvedPath)
|
||||||
|
|
||||||
return normalize(join(canonicalParentDirectory, basename(resolvedPath)))
|
return normalize(join(canonicalAncestor, relativePathFromAncestor || basename(resolvedPath)))
|
||||||
}
|
}
|
||||||
|
|
||||||
export function containsPath(rootPath: string, candidatePath: string): boolean {
|
export function containsPath(rootPath: string, candidatePath: string): boolean {
|
||||||
|
|||||||
@@ -1,4 +1,4 @@
|
|||||||
import { spawn } from "bun"
|
import { spawn, spawnSync } from "bun"
|
||||||
|
|
||||||
import type { ArchiveEntry } from "./archive-entry-validator"
|
import type { ArchiveEntry } from "./archive-entry-validator"
|
||||||
|
|
||||||
@@ -48,6 +48,56 @@ export async function listZipEntriesWithTar(archivePath: string): Promise<Archiv
|
|||||||
.filter((entry): entry is ArchiveEntry => entry !== null)
|
.filter((entry): entry is ArchiveEntry => entry !== null)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export function isPythonZipListingAvailable(): boolean {
|
||||||
|
const proc = spawnSync(["python3", "--version"], {
|
||||||
|
stdout: "ignore",
|
||||||
|
stderr: "ignore",
|
||||||
|
})
|
||||||
|
|
||||||
|
return proc.exitCode === 0
|
||||||
|
}
|
||||||
|
|
||||||
|
export async function listZipEntriesWithPython(archivePath: string): Promise<ArchiveEntry[]> {
|
||||||
|
const script = [
|
||||||
|
"import json, stat, sys, zipfile",
|
||||||
|
"entries = []",
|
||||||
|
"with zipfile.ZipFile(sys.argv[1], 'r') as archive:",
|
||||||
|
" for info in archive.infolist():",
|
||||||
|
" mode = (info.external_attr >> 16) & 0xFFFF",
|
||||||
|
" if stat.S_ISLNK(mode):",
|
||||||
|
" entry_type = 'symlink'",
|
||||||
|
" link_path = archive.read(info).decode('utf-8', 'surrogateescape')",
|
||||||
|
" elif info.filename.endswith('/'):",
|
||||||
|
" entry_type = 'directory'",
|
||||||
|
" link_path = None",
|
||||||
|
" else:",
|
||||||
|
" entry_type = 'file'",
|
||||||
|
" link_path = None",
|
||||||
|
" entry = {'path': info.filename, 'type': entry_type}",
|
||||||
|
" if link_path is not None:",
|
||||||
|
" entry['linkPath'] = link_path",
|
||||||
|
" entries.append(entry)",
|
||||||
|
"print(json.dumps(entries))",
|
||||||
|
].join("\n")
|
||||||
|
|
||||||
|
const proc = spawn(["python3", "-c", script, archivePath], {
|
||||||
|
stdout: "pipe",
|
||||||
|
stderr: "pipe",
|
||||||
|
})
|
||||||
|
|
||||||
|
const [exitCode, stdout, stderr] = await Promise.all([
|
||||||
|
proc.exited,
|
||||||
|
new Response(proc.stdout).text(),
|
||||||
|
new Response(proc.stderr).text(),
|
||||||
|
])
|
||||||
|
|
||||||
|
if (exitCode !== 0) {
|
||||||
|
throw new Error(`zip entry listing failed (exit ${exitCode}): ${stderr}`)
|
||||||
|
}
|
||||||
|
|
||||||
|
return JSON.parse(stdout) as ArchiveEntry[]
|
||||||
|
}
|
||||||
|
|
||||||
export async function listZipEntriesWithPowerShell(
|
export async function listZipEntriesWithPowerShell(
|
||||||
archivePath: string,
|
archivePath: string,
|
||||||
escapePowerShellPath: (path: string) => string,
|
escapePowerShellPath: (path: string) => string,
|
||||||
|
|||||||
@@ -2,7 +2,12 @@ import { spawn, spawnSync } from "bun"
|
|||||||
import { release } from "os"
|
import { release } from "os"
|
||||||
|
|
||||||
import { validateArchiveEntries } from "./archive-entry-validator"
|
import { validateArchiveEntries } from "./archive-entry-validator"
|
||||||
import { listZipEntriesWithPowerShell, listZipEntriesWithTar } from "./zip-entry-listing"
|
import {
|
||||||
|
isPythonZipListingAvailable,
|
||||||
|
listZipEntriesWithPowerShell,
|
||||||
|
listZipEntriesWithPython,
|
||||||
|
listZipEntriesWithTar,
|
||||||
|
} from "./zip-entry-listing"
|
||||||
|
|
||||||
const WINDOWS_BUILD_WITH_TAR = 17134
|
const WINDOWS_BUILD_WITH_TAR = 17134
|
||||||
|
|
||||||
@@ -98,5 +103,9 @@ async function listZipEntries(archivePath: string) {
|
|||||||
return listZipEntriesWithPowerShell(archivePath, escapePowerShellPath, extractor)
|
return listZipEntriesWithPowerShell(archivePath, escapePowerShellPath, extractor)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (isPythonZipListingAvailable()) {
|
||||||
|
return listZipEntriesWithPython(archivePath)
|
||||||
|
}
|
||||||
|
|
||||||
return listZipEntriesWithTar(archivePath)
|
return listZipEntriesWithTar(archivePath)
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user