chore(hardening): re-check file-tool paths against their symlink target

.changeset/hardening-symlink-containment.md

@@ -0,0 +1,12 @@
+---
+'@moonshot-ai/agent-core-v2': minor
+---
+
+Re-check file-tool paths against their symlink-resolved target before reading,
+writing or editing. Path canonicalization is lexical, so a symlink sitting
+inside the workspace reads as inside it while the OS follows the link at open
+time. Read, Write and Edit now resolve the longest existing prefix of the path
+and require that a path which looked in-workspace is still in-workspace once
+symlinks are resolved, and that the resolved target is not a sensitive file even
+when the link itself is innocuously named. Paths the caller already gave as
+outside the workspace are unaffected — those remain the approval layer's call.

packages/agent-core-v2/src/agent/tools/edit/editTool.ts

@@ -21,13 +21,15 @@
 
 import {
   extendWorkspaceWithSkillRoots,
+  assertRealPathAccess,
   resolvePathAccessPath,
   type WorkspaceConfig,
 } from '#/tool/path-access';
 import { toInputJsonSchema } from '#/tool/input-schema';
 import { literalRulePattern, matchesPathRuleSubject } from '#/tool/rule-match';
 import { IFileEditService } from '#/app/edit/fileEdit';
 import { IHostEnvironment } from '#/os/interface/hostEnvironment';
+import { IHostFileSystem } from '#/os/interface/hostFileSystem';
 import { ISessionSkillCatalog } from '#/session/sessionSkillCatalog/skillCatalog';
 import { ISessionWorkspaceContext } from '#/session/workspaceContext/workspaceContext';
 import {
@@ -48,6 +50,7 @@ export class EditTool implements IEditTool {
 
   constructor(
     @IFileEditService private readonly editor: IFileEditService,
+    @IHostFileSystem private readonly fs: IHostFileSystem,
     @IHostEnvironment private readonly env: IHostEnvironment,
     @ISessionWorkspaceContext private readonly workspaceCtx: ISessionWorkspaceContext,
     @ISessionSkillCatalog private readonly skillCatalog?: ISessionSkillCatalog,
@@ -92,6 +95,11 @@ export class EditTool implements IEditTool {
   }
 
   private async execution(args: EditInput, safePath: string): Promise<ExecutableToolResult> {
+    // The path was canonicalized lexically; re-check it against what the
+    // symlinks actually resolve to before editing through them.
+    await assertRealPathAccess(safePath, args.path, this.workspaceConfig, this.fs, {
+      pathClass: this.env.pathClass,
+    });
     if (args.old_string === args.new_string) {
       return {
         isError: true,

packages/agent-core-v2/src/agent/tools/os/read/readTool.ts

@@ -35,6 +35,7 @@ import {
 import { registerAgentToolService } from '#/agent/toolRegistry/toolContribution';
 import {
   extendWorkspaceWithSkillRoots,
+  assertRealPathAccess,
   resolvePathAccessPath,
   type WorkspaceConfig,
 } from '#/tool/path-access';
@@ -271,6 +272,11 @@ export class ReadTool implements IReadTool {
   }
 
   private async execution(args: ReadInput, safePath: string): Promise<ExecutableToolResult> {
+    // The path was canonicalized lexically; re-check it against what the
+    // symlinks actually resolve to before reading through them.
+    await assertRealPathAccess(safePath, args.path, this.workspaceConfig, this.fs, {
+      pathClass: this.env.pathClass,
+    });
     try {
       let stat: Awaited<ReturnType<IHostFileSystem['stat']>>;
       try {

packages/agent-core-v2/src/agent/tools/os/write/writeTool.ts

@@ -32,6 +32,7 @@ import {
 } from '#/tool/toolContract';
 import { registerAgentToolService } from '#/agent/toolRegistry/toolContribution';
 import {
+  assertRealPathAccess,
   extendWorkspaceWithSkillRoots,
   resolvePathAccessPath,
   type WorkspaceConfig,
@@ -87,6 +88,11 @@ export class WriteTool implements IWriteTool {
   }
 
   private async execution(args: WriteInput, safePath: string): Promise<ExecutableToolResult> {
+    // The path was canonicalized lexically; re-check it against what the
+    // symlinks actually resolve to before writing through them.
+    await assertRealPathAccess(safePath, args.path, this.workspaceConfig, this.fs, {
+      pathClass: this.env.pathClass,
+    });
     const parentError = await this.ensureParentDirectory(safePath);
     if (parentError !== undefined) {
       return { isError: true, output: parentError };

packages/agent-core-v2/src/tool/path-access.ts

@@ -333,6 +333,103 @@ export function resolvePathAccessPath(
   }).path;
 }
 
+export interface PathRealpathResolver {
+  realpath(path: string): Promise<string>;
+}
+
+export interface AssertRealPathOptions {
+  readonly pathClass?: PathClass | undefined;
+  readonly checkSensitive?: boolean | undefined;
+}
+
+/**
+ * Resolve the longest existing prefix of `abs` through symlinks and re-attach
+ * the not-yet-existing tail. A write to a new file still gets its parent
+ * directory resolved, which is where a redirect would sit.
+ */
+async function realpathExistingPrefix(abs: string, fs: PathRealpathResolver): Promise<string> {
+  const tail: string[] = [];
+  let current = abs;
+  for (let i = 0; i < 256; i++) {
+    try {
+      const real = await fs.realpath(current);
+      return tail.length === 0 ? real : pathe.join(real, ...tail.toReversed());
+    } catch {
+      const parent = pathe.dirname(current);
+      if (parent === current) return abs;
+      tail.push(pathe.basename(current));
+      current = parent;
+    }
+  }
+  return abs;
+}
+
+async function realWorkspaceRoots(
+  config: WorkspaceConfig,
+  fs: PathRealpathResolver,
+): Promise<readonly string[]> {
+  const roots: string[] = [];
+  for (const dir of [config.workspaceDir, ...config.additionalDirs]) {
+    try {
+      roots.push(await fs.realpath(dir));
+    } catch {
+      roots.push(dir);
+    }
+  }
+  return roots;
+}
+
+/**
+ * Symlink-aware re-check, run at execution time.
+ *
+ * `resolvePathAccess` canonicalizes lexically, so a symlink that sits inside
+ * the workspace still reads as inside it — while the OS follows the link at
+ * open time. This re-runs the two checks against the resolved target:
+ *
+ *   - a path that looked inside the workspace must still be inside it once
+ *     symlinks are resolved (a path the caller already gave as outside is
+ *     governed by the approval layer, so it is left alone here);
+ *   - the resolved target must not be a sensitive file, even when the link
+ *     itself has an innocuous name.
+ *
+ * Costs nothing on the common path: when nothing along the path is a symlink
+ * the resolved path equals the canonical one and this returns immediately.
+ */
+export async function assertRealPathAccess(
+  canonicalPath: string,
+  rawPath: string,
+  config: WorkspaceConfig,
+  fs: PathRealpathResolver,
+  options: AssertRealPathOptions = {},
+): Promise<void> {
+  const pathClass = options.pathClass ?? DEFAULT_PATH_CLASS;
+  const checkSensitive = options.checkSensitive ?? DEFAULT_WORKSPACE_ACCESS_POLICY.checkSensitive;
+  const realPath = await realpathExistingPrefix(canonicalPath, fs);
+  if (realPath === canonicalPath) return;
+
+  if (checkSensitive && isSensitiveFile(realPath)) {
+    throw new PathSecurityError(
+      'PATH_SENSITIVE',
+      rawPath,
+      realPath,
+      `"${rawPath}" resolves through a symlink to a sensitive file ` +
+        `(env / credential / SSH key). Access is blocked to protect secrets.`,
+    );
+  }
+
+  if (!isWithinWorkspace(canonicalPath, config, pathClass)) return;
+
+  const roots = await realWorkspaceRoots(config, fs);
+  if (roots.some((root) => isWithinDirectory(realPath, root, pathClass))) return;
+
+  throw new PathSecurityError(
+    'PATH_OUTSIDE_WORKSPACE',
+    rawPath,
+    realPath,
+    `"${rawPath}" resolves through a symlink to "${realPath}", which is outside the workspace.`,
+  );
+}
+
 export function assertPathAllowed(
   path: string,
   cwd: string,

packages/agent-core-v2/test/app/edit/tools/edit.test.ts

@@ -75,7 +75,7 @@ function buildTool(
       reg.define(IFileEditService, FileEditService);
     },
   });
-  return new EditTool(ix.get(IFileEditService), env, workspace);
+  return new EditTool(ix.get(IFileEditService), fs, env, workspace);
 }
 
 function isPromiseLike(

packages/agent-core-v2/test/tool/path-access.test.ts

@@ -1,6 +1,14 @@
+import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import nodePath from 'node:path';
+
 import { describe, expect, it } from 'vitest';
 
-import { extendWorkspaceWithSkillRoots, isSensitiveFile } from '#/tool/path-access';
+import {
+  assertRealPathAccess,
+  extendWorkspaceWithSkillRoots,
+  isSensitiveFile,
+} from '#/tool/path-access';
 
 describe('isSensitiveFile', () => {
   it('flags base .env files in any directory', () => {
@@ -102,3 +110,141 @@ describe('extendWorkspaceWithSkillRoots', () => {
     ).toEqual([]);
   });
 });
+
+describe('assertRealPathAccess', () => {
+  const workspace = { workspaceDir: '/ws', additionalDirs: [] as string[] };
+
+  /**
+   * Resolver where `links` maps a path to what it really resolves to, and
+   * `missing` paths reject the way `realpath` does for a path that does not
+   * exist yet (which is what makes the guard walk up to the parent).
+   */
+  function resolver(
+    links: Record<string, string>,
+    missing: readonly string[] = [],
+  ): { realpath: (p: string) => Promise<string> } {
+    return {
+      realpath: (p: string) => {
+        if (missing.includes(p)) return Promise.reject(new Error(`ENOENT: ${p}`));
+        for (const [from, to] of Object.entries(links)) {
+          if (p === from) return Promise.resolve(to);
+        }
+        return Promise.resolve(p);
+      },
+    };
+  }
+
+  it('allows a path that resolves to itself', async () => {
+    await expect(
+      assertRealPathAccess('/ws/src/a.ts', 'src/a.ts', workspace, resolver({}), {
+        pathClass: 'posix',
+      }),
+    ).resolves.toBeUndefined();
+  });
+
+  it('rejects an in-workspace path that resolves outside the workspace', async () => {
+    // scripts/deploy.sh -> /home/u/.zshrc
+    await expect(
+      assertRealPathAccess(
+        '/ws/scripts/deploy.sh',
+        'scripts/deploy.sh',
+        workspace,
+        resolver({ '/ws/scripts/deploy.sh': '/home/u/.zshrc' }),
+        { pathClass: 'posix' },
+      ),
+    ).rejects.toThrow(/outside the workspace/);
+  });
+
+  it('rejects a link whose target is sensitive even when the link name is innocuous', async () => {
+    // notes.md -> /home/u/.aws/credentials
+    await expect(
+      assertRealPathAccess(
+        '/ws/notes.md',
+        'notes.md',
+        workspace,
+        resolver({ '/ws/notes.md': '/home/u/.aws/credentials' }),
+        { pathClass: 'posix' },
+      ),
+    ).rejects.toThrow(/sensitive file/);
+  });
+
+  it('resolves the parent directory for a file that does not exist yet', async () => {
+    // scripts -> /etc, so a new file under it lands outside the workspace.
+    await expect(
+      assertRealPathAccess(
+        '/ws/scripts/new.sh',
+        'scripts/new.sh',
+        workspace,
+        resolver({ '/ws/scripts': '/etc' }, ['/ws/scripts/new.sh']),
+        { pathClass: 'posix' },
+      ),
+    ).rejects.toThrow(/outside the workspace/);
+  });
+
+  it('allows a symlink that stays inside the workspace', async () => {
+    await expect(
+      assertRealPathAccess(
+        '/ws/link.ts',
+        'link.ts',
+        workspace,
+        resolver({ '/ws/link.ts': '/ws/real.ts' }),
+        { pathClass: 'posix' },
+      ),
+    ).resolves.toBeUndefined();
+  });
+
+  it('honours additionalDirs as legitimate roots', async () => {
+    await expect(
+      assertRealPathAccess(
+        '/ws/skill',
+        'skill',
+        { workspaceDir: '/ws', additionalDirs: ['/opt/skills'] },
+        resolver({ '/ws/skill': '/opt/skills/a' }),
+        { pathClass: 'posix' },
+      ),
+    ).resolves.toBeUndefined();
+  });
+
+  it('leaves an explicitly-outside path to the approval layer', async () => {
+    // Already outside the workspace lexically: not this guard's call.
+    await expect(
+      assertRealPathAccess(
+        '/tmp/scratch',
+        '/tmp/scratch',
+        workspace,
+        resolver({ '/tmp/scratch': '/tmp/elsewhere' }),
+        { pathClass: 'posix' },
+      ),
+    ).resolves.toBeUndefined();
+  });
+});
+
+describe('assertRealPathAccess against a real filesystem', () => {
+  it('blocks a real in-workspace symlink that points outside the workspace', async () => {
+    const hostFs = { realpath: (p: string) => realpath(p) };
+    const root = await realpath(await mkdtemp(nodePath.join(tmpdir(), 'kimi-symlink-')));
+    try {
+      const ws = nodePath.join(root, 'repo');
+      const outside = nodePath.join(root, 'home');
+      await mkdir(nodePath.join(ws, 'scripts'), { recursive: true });
+      await mkdir(outside, { recursive: true });
+      const target = nodePath.join(outside, '.zshrc');
+      await writeFile(target, 'echo hi\n');
+      const link = nodePath.join(ws, 'scripts', 'deploy.sh');
+      await symlink(target, link);
+      const workspace = { workspaceDir: ws, additionalDirs: [] as string[] };
+
+      await expect(
+        assertRealPathAccess(link, 'scripts/deploy.sh', workspace, hostFs, { pathClass: 'posix' }),
+      ).rejects.toThrow(/outside the workspace/);
+
+      const real = nodePath.join(ws, 'scripts', 'ok.sh');
+      await writeFile(real, '#!/bin/sh\n');
+      await expect(
+        assertRealPathAccess(real, 'scripts/ok.sh', workspace, hostFs, { pathClass: 'posix' }),
+      ).resolves.toBeUndefined();
+    } finally {
+      await rm(root, { recursive: true, force: true });
+    }
+  });
+});