fix: preserve symlinks in atomicWrite to prevent config.toml symlink breakage

.changeset/fix-atomic-write-symlink.md

@@ -0,0 +1,7 @@
+---
+"@moonshot-ai/agent-core": patch
+---
+
+fix: preserve symlinks in atomicWrite to prevent config.toml symlink breakage
+
+When config.toml is a symlink (e.g., pointing to iCloud), atomicWrite's rename() was replacing the symlink itself instead of writing through it. This fix resolves symlinks before rename, preserving the symlink.

packages/agent-core/src/config/toml.ts

@@ -450,7 +450,7 @@ export async function writeConfigFile(filePath: string, config: KimiConfig): Pro
   // stripEnvModelConfig / the getConfig -> setConfig round-trip).
   const validated = validateConfig(stripEnvModelConfig(config));
   await mkdir(dirname(filePath), { recursive: true, mode: 0o700 });
-  await atomicWrite(filePath, `${stringifyToml(configToTomlData(validated))}\n`);
+  await atomicWrite(filePath, `${stringifyToml(configToTomlData(validated))}\n`, { followSymlinks: true });
 }
 
 export function configToTomlData(config: KimiConfig): Record<string, unknown> {

packages/agent-core/src/utils/fs.ts

@@ -17,7 +17,7 @@
 import { randomBytes } from 'node:crypto';
 import { closeSync, fsyncSync, openSync } from 'node:fs';
 import * as nodeFs from 'node:fs';
-import { open, rename, unlink } from 'node:fs/promises';
+import { open, rename, unlink, lstat, realpath } from 'node:fs/promises';
 import { dirname } from 'pathe';
 
 /**
@@ -62,12 +62,30 @@ export function syncDirSync(dirPath: string): void {
  * failure *after* the rename (i.e. in the parent-directory fsync) is
  * surfaced to the caller — the content is already in place, but
  * durability is not guaranteed.
+ *
+ * @param options.followSymlinks — if true and `filePath` is a symlink,
+ *   resolve to the real path before rename, preserving the symlink.
+ *   Default false (replaces the symlink itself). Opt-in because following
+ *   symlinks can break path-traversal boundaries in guarded stores.
  */
 export async function writeFileAtomicDurable(
   filePath: string,
   content: string | Uint8Array,
+  options?: { followSymlinks?: boolean },
 ): Promise<void> {
-  const tmpPath = filePath + '.tmp';
+  let writeTarget = filePath;
+  if (options?.followSymlinks) {
+    try {
+      const stat = await lstat(filePath);
+      if (stat.isSymbolicLink()) {
+        writeTarget = await realpath(filePath);
+      }
+    } catch {
+      // lstat/realpath failure — fall back to original path.
+    }
+  }
+
+  const tmpPath = writeTarget + '.tmp';
   let renamed = false;
   try {
     const fh = await open(tmpPath, 'w');
@@ -80,15 +98,15 @@ export async function writeFileAtomicDurable(
     // Windows pre-unlink for MoveFileEx parity.
     if (process.platform === 'win32') {
       try {
-        await unlink(filePath);
+        await unlink(writeTarget);
       } catch (error) {
         const code = (error as NodeJS.ErrnoException).code;
         if (code !== 'ENOENT') throw error;
       }
     }
-    await rename(tmpPath, filePath);
+    await rename(tmpPath, writeTarget);
     renamed = true;
-    await syncDir(dirname(filePath));
+    await syncDir(dirname(writeTarget));
   } finally {
     if (!renamed) {
       // Best-effort cleanup of the `.tmp` file if we never got to the
@@ -143,16 +161,33 @@ function syncFd(fd: number): Promise<void> {
  *
  * @param filePath — absolute or relative path to the target file.
  * @param content  — string or binary payload to write.
+ * @param options.followSymlinks — if true and `filePath` is a symlink,
+ *   resolve to the real path before rename, preserving the symlink.
+ *   Default false (replaces the symlink itself). Opt-in because following
+ *   symlinks can break path-traversal boundaries in guarded stores.
  * @param _syncOverride — test seam: override the fsync implementation for
  *   fault injection. Production callers must never supply this.
  */
 export async function atomicWrite(
   filePath: string,
   content: string | Uint8Array,
+  options?: { followSymlinks?: boolean },
   _syncOverride?: (fd: number) => Promise<void>,
 ): Promise<void> {
+  let writeTarget = filePath;
+  if (options?.followSymlinks) {
+    try {
+      const stat = await lstat(filePath);
+      if (stat.isSymbolicLink()) {
+        writeTarget = await realpath(filePath);
+      }
+    } catch {
+      // lstat/realpath failure — fall back to original path.
+    }
+  }
+
   const hex = randomBytes(4).toString('hex');
-  const tmpPath = `${filePath}.tmp.${process.pid}.${hex}`;
+  const tmpPath = `${writeTarget}.tmp.${process.pid}.${hex}`;
   let renamed = false;
   try {
     const fh = await open(tmpPath, 'w');
@@ -167,13 +202,13 @@ export async function atomicWrite(
     // before the rename turns this into the POSIX-style "replace" case.
     if (process.platform === 'win32') {
       try {
-        await unlink(filePath);
+        await unlink(writeTarget);
       } catch (error) {
         const code = (error as NodeJS.ErrnoException).code;
         if (code !== 'ENOENT') throw error;
       }
     }
-    await rename(tmpPath, filePath);
+    await rename(tmpPath, writeTarget);
     renamed = true;
   } finally {
     if (!renamed) {

packages/agent-core/test/utils/atomic-write.test.ts

@@ -0,0 +1,159 @@
+/**
+ * Tests for atomicWrite — specifically symlink preservation.
+ *
+ * The core invariant: when followSymlinks is true and `filePath` is a
+ * symlink, atomicWrite should preserve the symlink (write to the resolved
+ * target), not replace it. Default behavior (followSymlinks false) replaces
+ * the symlink itself.
+ */
+
+import { lstat, mkdir, readFile, rm, symlink, writeFile } from 'node:fs/promises';
+import { tmpdir } from 'node:os';
+import { join } from 'pathe';
+
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+
+import { atomicWrite } from '../../src/utils/fs';
+
+let rootDir: string;
+
+beforeEach(async () => {
+  rootDir = join(
+    tmpdir(),
+    `kimi-atomic-write-${Date.now()}-${Math.random().toString(36).slice(2)}`,
+  );
+  await mkdir(rootDir, { recursive: true });
+});
+
+afterEach(async () => {
+  await rm(rootDir, { recursive: true, force: true });
+});
+
+describe('atomicWrite', () => {
+  it('writes content to a regular file', async () => {
+    const filePath = join(rootDir, 'regular.txt');
+    await atomicWrite(filePath, 'hello world');
+
+    const content = await readFile(filePath, 'utf-8');
+    expect(content).toBe('hello world');
+  });
+
+  it('overwrites existing regular file', async () => {
+    const filePath = join(rootDir, 'existing.txt');
+    await writeFile(filePath, 'old content');
+
+    await atomicWrite(filePath, 'new content');
+
+    const content = await readFile(filePath, 'utf-8');
+    expect(content).toBe('new content');
+  });
+
+  it('default behavior: replaces symlink with regular file', async () => {
+    const targetDir = join(rootDir, 'target');
+    await mkdir(targetDir);
+    const realFile = join(targetDir, 'config.toml');
+    await writeFile(realFile, 'original');
+
+    const symlinkPath = join(rootDir, 'config.toml');
+    await symlink(realFile, symlinkPath);
+
+    // Without followSymlinks, symlink is replaced
+    await atomicWrite(symlinkPath, 'new content');
+
+    const stat = await lstat(symlinkPath);
+    expect(stat.isSymbolicLink()).toBe(false);
+
+    const content = await readFile(symlinkPath, 'utf-8');
+    expect(content).toBe('new content');
+
+    // Original target unchanged
+    const originalContent = await readFile(realFile, 'utf-8');
+    expect(originalContent).toBe('original');
+  });
+
+  it('followSymlinks: preserves symlink and writes to target', async () => {
+    const targetDir = join(rootDir, 'real-target');
+    await mkdir(targetDir);
+    const realFile = join(targetDir, 'config.toml');
+    await writeFile(realFile, 'original content');
+
+    const symlinkPath = join(rootDir, 'config.toml');
+    await symlink(realFile, symlinkPath);
+
+    // Verify symlink exists
+    expect((await lstat(symlinkPath)).isSymbolicLink()).toBe(true);
+
+    // Write with followSymlinks
+    await atomicWrite(symlinkPath, 'updated content', { followSymlinks: true });
+
+    // Symlink should still exist
+    expect((await lstat(symlinkPath)).isSymbolicLink()).toBe(true);
+
+    // Real file should have new content
+    expect(await readFile(realFile, 'utf-8')).toBe('updated content');
+
+    // Reading via symlink should also show new content
+    expect(await readFile(symlinkPath, 'utf-8')).toBe('updated content');
+  });
+
+  it('followSymlinks: replaces broken symlink when target does not exist', async () => {
+    const targetDir = join(rootDir, 'target-dir');
+    await mkdir(targetDir);
+    const realFile = join(targetDir, 'new-file.toml');
+    const symlinkPath = join(rootDir, 'link.toml');
+    await symlink(realFile, symlinkPath);
+
+    // Verify it's a broken symlink
+    expect((await lstat(symlinkPath)).isSymbolicLink()).toBe(true);
+
+    // realpath fails on broken symlink, falls back to original path
+    await atomicWrite(symlinkPath, 'new content', { followSymlinks: true });
+
+    // Symlink is replaced by regular file (fallback behavior)
+    expect((await lstat(symlinkPath)).isSymbolicLink()).toBe(false);
+
+    expect(await readFile(symlinkPath, 'utf-8')).toBe('new content');
+  });
+
+  it('handles non-existent file (no symlink)', async () => {
+    const filePath = join(rootDir, 'new-file.txt');
+    await atomicWrite(filePath, 'brand new');
+
+    const content = await readFile(filePath, 'utf-8');
+    expect(content).toBe('brand new');
+  });
+
+  it('followSymlinks: preserves symlink through multiple writes', async () => {
+    const targetDir = join(rootDir, 'target');
+    await mkdir(targetDir);
+    const realFile = join(targetDir, 'config.toml');
+    await writeFile(realFile, 'v1');
+
+    const symlinkPath = join(rootDir, 'config.toml');
+    await symlink(realFile, symlinkPath);
+
+    await atomicWrite(symlinkPath, 'v2', { followSymlinks: true });
+    await atomicWrite(symlinkPath, 'v3', { followSymlinks: true });
+    await atomicWrite(symlinkPath, 'v4', { followSymlinks: true });
+
+    expect((await lstat(symlinkPath)).isSymbolicLink()).toBe(true);
+    expect(await readFile(realFile, 'utf-8')).toBe('v4');
+  });
+
+  it('followSymlinks: handles circular symlink gracefully', async () => {
+    // Create circular symlinks: a -> b -> a
+    const linkA = join(rootDir, 'link-a.toml');
+    const linkB = join(rootDir, 'link-b.toml');
+    await symlink(linkB, linkA);
+    await symlink(linkA, linkB);
+
+    // realpath should fail on circular symlink, fall back to original path
+    await expect(
+      atomicWrite(linkA, 'content', { followSymlinks: true }),
+    ).resolves.not.toThrow();
+
+    // Fallback: symlink is replaced by regular file
+    expect((await lstat(linkA)).isSymbolicLink()).toBe(false);
+    expect(await readFile(linkA, 'utf-8')).toBe('content');
+  });
+});