Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 60 additions & 0 deletions .agentsmesh/lessons/lessons.json
Original file line number Diff line number Diff line change
Expand Up @@ -4836,6 +4836,36 @@
"t-glob-4c8d45b0"
]
},
"mcp-path-containment-mcp-path-containment-canonicalize-recons": {
"createdAt": "2026-07-08",
"evidence": [
"commit:54b4f38f"
],
"rule": "MCP path containment: canonicalize() reconstructs a DANGLING symlink leaf (a link whose target does not exist) as if it were an in-root file — realpath throws ENOENT, the tail is re-appended, and assertContainedPath then ALLOWS it. Any writer relying on the guard MUST create via temp-file + rename (rename replaces the link) and must NEVER writeFile directly on the target path (that follows the dangling link and writes outside the project). safeWrite/safeConfigWrite/atomicWrite are all rename-based; keep every new MCP writer rename-based.",
"status": "active",
"topics": [
"mcp-path-containment"
],
"triggers": [
"t-glob-2f2abaf1",
"t-glob-13a07989"
]
},
"mcp-path-containment-mcp-path-containment-must-anchor": {
"createdAt": "2026-07-08",
"evidence": [
"commit:54b4f38f"
],
"rule": "MCP path containment must anchor the boundary at the PROJECT ROOT, never at `.agentsmesh`: a `.agentsmesh` boundary canonicalizes THROUGH a symlinked `.agentsmesh` and cancels (root and boundary both resolve outside, so the escape passes) — this lets a symlinked `.agentsmesh` parent leak reads and land destructive out-of-tree writes. Guard EVERY user/config read, write, AND delete at choke points so no handler can forget: canonical `list` (not just get/update/delete), settings config WRITES (atomicWrite + safeConfigWrite), and read-before-write pre-reads (updatePermissions/updateIgnore append). Every symlink test must `it.skipIf(platform()==='win32')` — unprivileged Windows `symlink()` throws EPERM and reddens the windows-latest CI matrix.",
"status": "active",
"topics": [
"mcp-path-containment"
],
"triggers": [
"t-glob-13a07989",
"t-glob-2f2abaf1"
]
},
"pnpm-lockfile-in-this-repo-run-dev": {
"createdAt": "2026-06-23",
"evidence": [],
Expand Down Expand Up @@ -6514,6 +6544,21 @@
"t-cmd-7c1c43ea"
]
},
"verification-workflow-to-fetch-a-pr-head": {
"createdAt": "2026-07-07",
"evidence": [
"commit:d4e496a5"
],
"rationale": "SSH keyless environment blocks the default origin fetch when reviewing external PRs.",
"rule": "To fetch a PR head in this repo, `origin` is SSH-only (git@github.com) with no local key so `git fetch origin pull/<n>/head` fails with Permission denied (publickey); instead fetch over HTTPS with the gh token: `git fetch \"https://x-access-token:$(gh auth token)@github.com/<owner>/<repo>.git\" pull/<n>/head:pr-<n>`.",
"status": "active",
"topics": [
"verification-workflow"
],
"triggers": [
"t-cmd-671640f4"
]
},
"verification-workflow-verify-a-referenced-source-path": {
"createdAt": "2026-06-06",
"evidence": [
Expand Down Expand Up @@ -7413,6 +7458,9 @@
"mcp-generate-lock": {
"summary": "MCP generate must mirror CLI generate (lockfile/stale-cleanup/process-lock)"
},
"mcp-path-containment": {
"summary": "MCP path containment implementation guardrails (project-root anchor, choke points, Windows symlink tests)"
},
"pnpm-lockfile": {
"summary": "pnpm lockfile overrides ↔ package.json consistency and pnpm-version parity with CI (CI uses pnpm 10)."
},
Expand Down Expand Up @@ -7582,6 +7630,10 @@
"kind": "command_pattern",
"pattern": "pnpm exec"
},
"t-cmd-671640f4": {
"kind": "command_pattern",
"pattern": "git fetch.*pull/\\d+/head"
},
"t-cmd-69dc7d4e": {
"kind": "command_pattern",
"pattern": "<<-"
Expand Down Expand Up @@ -7858,6 +7910,10 @@
"kind": "file_glob",
"pattern": "src/targets/catalog/registry.ts"
},
"t-glob-13a07989": {
"kind": "file_glob",
"pattern": "src/mcp/handlers/*.ts"
},
"t-glob-151a6581": {
"kind": "file_glob",
"pattern": "tests/e2e/helpers/**"
Expand Down Expand Up @@ -7930,6 +7986,10 @@
"kind": "file_glob",
"pattern": "src/lessons/ranking.ts"
},
"t-glob-2f2abaf1": {
"kind": "file_glob",
"pattern": "src/mcp/writers/*.ts"
},
"t-glob-300bbdba": {
"kind": "file_glob",
"pattern": "src/lessons/mutate.ts"
Expand Down
5 changes: 5 additions & 0 deletions .changeset/mcp-symlink-containment.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"agentsmesh": patch
---

Reject MCP read, write, and delete paths that escape the project directory through symlinks. Containment is anchored at the project root (not `.agentsmesh`), so a symlinked config file **or** a symlinked `.agentsmesh` parent directory can no longer leak or overwrite files outside the project. Covers skill and canonical (rules/commands/agents) list/get/create/update/delete, plus all config reads and writes (`get_config`/`update_config`, permissions, hooks, ignore, and MCP-server tools). Note: config reads that escape through a symlink now raise `PATH_TRAVERSAL` instead of returning `null`.
35 changes: 34 additions & 1 deletion src/mcp/handlers/canonical-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { McpContext } from '../context.js';
import { McpError } from '../errors.js';
import { MAX_DIR_ENTRIES } from '../limits.js';
import { safeWrite } from '../writers/safe-write.js';
import { assertContainedPath } from '../writers/path-containment.js';
import { parseMd, serializeMd } from '../writers/md-frontmatter.js';

// Flat identifier only — `/` is intentionally excluded so canonical names
Expand Down Expand Up @@ -82,6 +83,12 @@ export function createCanonicalHandlers<TSummary>(

return {
async list(ctx) {
// Reject a symlinked feature dir before reading any file through it.
await assertContainedPath({
root: ctx.projectRoot,
target: featureDir(ctx.projectRoot),
message: `path escapes ${feature} directory`,
});
const files = await listFiles(ctx.projectRoot);
const out: TSummary[] = [];
for (const f of files) {
Expand All @@ -96,6 +103,12 @@ export function createCanonicalHandlers<TSummary>(
async get(ctx, { name }) {
checkName(name);
const file = pathFor(ctx.projectRoot, feature, name);
await assertContainedPath({
root: featureDir(ctx.projectRoot),
target: file,
boundaryRoot: ctx.projectRoot,
message: `path escapes ${feature} directory`,
});
try {
const src = await readFile(file, 'utf8');
const { frontmatter, body } = parseMd(src);
Expand All @@ -111,11 +124,19 @@ export function createCanonicalHandlers<TSummary>(

async create(ctx, { name, frontmatter, body, dry_run }) {
checkName(name);
const file = pathFor(ctx.projectRoot, feature, name);
// Assert containment BEFORE the existence probe so a symlinked feature dir
// cannot leak an out-of-project filename-existence oracle via ALREADY_EXISTS.
await assertContainedPath({
root: featureDir(ctx.projectRoot),
target: file,
boundaryRoot: ctx.projectRoot,
message: `path escapes ${feature} directory`,
});
const parsed = frontmatterSchema.safeParse(frontmatter);
if (!parsed.success) {
throw new McpError('VALIDATION_FAILED', 'invalid frontmatter', parsed.error.issues);
}
const file = pathFor(ctx.projectRoot, feature, name);
if (await exists(file)) throw new McpError('ALREADY_EXISTS', `${feature} "${name}" exists`);
const all = await listFiles(ctx.projectRoot);
if (all.length >= MAX_DIR_ENTRIES) {
Expand All @@ -136,6 +157,12 @@ export function createCanonicalHandlers<TSummary>(
checkName(name);
const file = pathFor(ctx.projectRoot, feature, name);
let current: { frontmatter: Record<string, unknown>; body: string };
await assertContainedPath({
root: featureDir(ctx.projectRoot),
target: file,
boundaryRoot: ctx.projectRoot,
message: `path escapes ${feature} directory`,
});
try {
const src = await readFile(file, 'utf8');
current = parseMd(src);
Expand Down Expand Up @@ -174,6 +201,12 @@ export function createCanonicalHandlers<TSummary>(
throw new McpError('PROTECTED_FILE', `${feature} "${name}" requires force: true`);
}
const file = pathFor(ctx.projectRoot, feature, name);
await assertContainedPath({
root: featureDir(ctx.projectRoot),
target: file,
boundaryRoot: ctx.projectRoot,
message: `path escapes ${feature} directory`,
});
if (!(await exists(file))) throw new McpError('NOT_FOUND', `${feature} "${name}" not found`);
if (dry_run === true) return { path: file, deleted: false };
await rm(file);
Expand Down
61 changes: 48 additions & 13 deletions src/mcp/handlers/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,27 @@ import type { McpContext } from '../context.js';
import { McpError } from '../errors.js';
import { MAX_FILE_SIZE_BYTES } from '../limits.js';
import { safeConfigWrite } from '../writers/safe-config-write.js';
import { assertContainedPath } from '../writers/path-containment.js';
import { normalizeHooksRecord } from '../writers/normalize-hooks.js';
import { configSchema } from '../../config/core/schema.js';
import { parseMcp } from '../../canonical/features/mcp.js';
import type { McpConfig } from '../../core/mcp-types.js';

async function readYaml<T>(path: string): Promise<T | null> {
// Reject any config read/write whose resolved (symlink-followed) path escapes
// the project root — a symlinked config file OR a symlinked `.agentsmesh` parent
// dir must not leak or overwrite files outside the project. Anchoring at the
// project root (not `.agentsmesh`) is deliberate: a boundary of `.agentsmesh`
// would canonicalize through a symlinked `.agentsmesh` and cancel.
async function assertWithinProject(projectRoot: string, target: string): Promise<void> {
await assertContainedPath({
root: projectRoot,
target,
message: 'config path escapes project directory',
});
}

async function readYaml<T>(projectRoot: string, path: string): Promise<T | null> {
await assertWithinProject(projectRoot, path);
try {
return parseYaml(await readFile(path, 'utf8')) as T;
} catch (e: unknown) {
Expand All @@ -19,7 +34,8 @@ async function readYaml<T>(path: string): Promise<T | null> {
}
}

async function atomicWrite(path: string, content: string): Promise<void> {
async function atomicWrite(projectRoot: string, path: string, content: string): Promise<void> {
await assertWithinProject(projectRoot, path);
if (Buffer.byteLength(content, 'utf8') > MAX_FILE_SIZE_BYTES) {
throw new McpError('LIMIT_EXCEEDED', 'file exceeds 1 MiB cap');
}
Expand All @@ -32,13 +48,17 @@ export const settingsHandlers = {
// ─── reads ───

async getConfig(ctx: McpContext): Promise<unknown> {
const cfg = await readYaml<unknown>(resolve(ctx.projectRoot, 'agentsmesh.yaml'));
const cfg = await readYaml<unknown>(
ctx.projectRoot,
resolve(ctx.projectRoot, 'agentsmesh.yaml'),
);
if (cfg === null) throw new McpError('NO_PROJECT', 'agentsmesh.yaml missing');
return cfg;
},

async listMcpServers(ctx: McpContext): Promise<{ servers: McpConfig['mcpServers'] | null }> {
const path = resolve(ctx.projectRoot, '.agentsmesh/mcp.json');
await assertWithinProject(ctx.projectRoot, path);
try {
const cfg = await parseMcp(path);
return { servers: cfg?.mcpServers ?? null };
Expand All @@ -48,16 +68,23 @@ export const settingsHandlers = {
},

async getPermissions(ctx: McpContext): Promise<unknown> {
return (await readYaml(resolve(ctx.projectRoot, '.agentsmesh/permissions.yaml'))) ?? null;
return (
(await readYaml(ctx.projectRoot, resolve(ctx.projectRoot, '.agentsmesh/permissions.yaml'))) ??
null
);
},

async getHooks(ctx: McpContext): Promise<unknown> {
return (await readYaml(resolve(ctx.projectRoot, '.agentsmesh/hooks.yaml'))) ?? null;
return (
(await readYaml(ctx.projectRoot, resolve(ctx.projectRoot, '.agentsmesh/hooks.yaml'))) ?? null
);
},

async getIgnore(ctx: McpContext): Promise<{ patterns: string[] | null }> {
const file = resolve(ctx.projectRoot, '.agentsmesh/ignore');
await assertWithinProject(ctx.projectRoot, file);
try {
const src = await readFile(resolve(ctx.projectRoot, '.agentsmesh/ignore'), 'utf8');
const src = await readFile(file, 'utf8');
return { patterns: src.split(/\r?\n/).filter((l) => l !== '' && !l.startsWith('#')) };
} catch {
return { patterns: null };
Expand All @@ -78,7 +105,10 @@ export const settingsHandlers = {
},
): Promise<{ path: string; written: boolean }> {
const current =
(await readYaml<Record<string, unknown>>(resolve(ctx.projectRoot, 'agentsmesh.yaml'))) ?? {};
(await readYaml<Record<string, unknown>>(
ctx.projectRoot,
resolve(ctx.projectRoot, 'agentsmesh.yaml'),
)) ?? {};
const next: Record<string, unknown> = { ...current };
const apply = (k: 'targets' | 'features', v: string[] | undefined): void => {
if (v === undefined) return;
Expand Down Expand Up @@ -116,13 +146,14 @@ export const settingsHandlers = {
input: { name: string; server: Record<string, unknown>; dry_run?: boolean },
): Promise<{ path: string; written: boolean }> {
const path = resolve(ctx.projectRoot, '.agentsmesh/mcp.json');
await assertWithinProject(ctx.projectRoot, path);
const cfg = (await parseMcp(path).catch(() => null)) ?? { mcpServers: {} };
if (cfg.mcpServers[input.name] !== undefined) {
throw new McpError('ALREADY_EXISTS', `server "${input.name}" exists`);
}
cfg.mcpServers[input.name] = input.server as never;
if (input.dry_run === true) return { path, written: false };
await atomicWrite(path, JSON.stringify(cfg, null, 2) + '\n');
await atomicWrite(ctx.projectRoot, path, JSON.stringify(cfg, null, 2) + '\n');
return { path, written: true };
},

Expand All @@ -131,6 +162,7 @@ export const settingsHandlers = {
input: { name: string; server: Record<string, unknown>; merge?: boolean; dry_run?: boolean },
): Promise<{ path: string; written: boolean }> {
const path = resolve(ctx.projectRoot, '.agentsmesh/mcp.json');
await assertWithinProject(ctx.projectRoot, path);
const cfg = await parseMcp(path).catch(() => null);
if (cfg === null || cfg.mcpServers[input.name] === undefined) {
throw new McpError('NOT_FOUND', `server "${input.name}" not found`);
Expand All @@ -140,7 +172,7 @@ export const settingsHandlers = {
? ({ ...cfg.mcpServers[input.name], ...input.server } as never)
: (input.server as never);
if (input.dry_run === true) return { path, written: false };
await atomicWrite(path, JSON.stringify(cfg, null, 2) + '\n');
await atomicWrite(ctx.projectRoot, path, JSON.stringify(cfg, null, 2) + '\n');
return { path, written: true };
},

Expand All @@ -149,13 +181,14 @@ export const settingsHandlers = {
input: { name: string; dry_run?: boolean },
): Promise<{ path: string; removed: boolean }> {
const path = resolve(ctx.projectRoot, '.agentsmesh/mcp.json');
await assertWithinProject(ctx.projectRoot, path);
const cfg = await parseMcp(path).catch(() => null);
if (cfg === null || cfg.mcpServers[input.name] === undefined) {
throw new McpError('NOT_FOUND', `server "${input.name}" not found`);
}
delete cfg.mcpServers[input.name];
if (input.dry_run === true) return { path, removed: false };
await atomicWrite(path, JSON.stringify(cfg, null, 2) + '\n');
await atomicWrite(ctx.projectRoot, path, JSON.stringify(cfg, null, 2) + '\n');
return { path, removed: true };
},

Expand All @@ -171,6 +204,7 @@ export const settingsHandlers = {
): Promise<{ path: string; written: boolean }> {
const path = resolve(ctx.projectRoot, '.agentsmesh/permissions.yaml');
const current = (await readYaml<{ allow?: string[]; deny?: string[]; ask?: string[] }>(
ctx.projectRoot,
path,
)) ?? {
allow: [],
Expand All @@ -186,7 +220,7 @@ export const settingsHandlers = {
apply('deny', input.deny);
apply('ask', input.ask);
if (input.dry_run === true) return { path, written: false };
await atomicWrite(path, stringifyYaml(next));
await atomicWrite(ctx.projectRoot, path, stringifyYaml(next));
return { path, written: true };
},

Expand All @@ -198,7 +232,7 @@ export const settingsHandlers = {
if (input.dry_run === true) return { path, written: false };
// Flatten the nested native form to the flat canonical shape so parseHooks
// can recover it; a verbatim nested write is silently dropped on generate.
await atomicWrite(path, stringifyYaml(normalizeHooksRecord(input.hooks)));
await atomicWrite(ctx.projectRoot, path, stringifyYaml(normalizeHooksRecord(input.hooks)));
return { path, written: true };
},

Expand All @@ -209,13 +243,14 @@ export const settingsHandlers = {
const path = resolve(ctx.projectRoot, '.agentsmesh/ignore');
let next: string[];
if (input.mode === 'append') {
await assertWithinProject(ctx.projectRoot, path);
const cur = (await readFile(path, 'utf8').catch(() => '')).split(/\r?\n/).filter(Boolean);
next = Array.from(new Set([...cur, ...input.patterns]));
} else {
next = input.patterns;
}
if (input.dry_run === true) return { path, written: false };
await atomicWrite(path, next.join('\n') + '\n');
await atomicWrite(ctx.projectRoot, path, next.join('\n') + '\n');
return { path, written: true };
},
};
Loading
Loading