Skip to content

Commit a391b3c

Browse files
committed
fix: preserve symlinked destinations in atomic config writes
atomicWriteFile and atomicWriteFileAsync wrote a temp file beside the literal destination path and renamed over it. rename(2) replaces a directory entry, so when the destination was itself a symlink the rename destroyed the link and left a plain file in its place. This breaks dotfiles-managed setups, where ~/.codex/config.toml is a symlink into a tracked repo. The first injected write silently converts it to a real file and the repo stops receiving updates. Nothing surfaces the divergence: the live config keeps working, so the stale repo copy looks current until someone diffs it. Resolve the destination through realpath before choosing the temp path. Both the temp file and the rename target then live inside the link's real directory, so the entry replaced is the real file and the link survives. Same-filesystem atomicity is preserved because the temp stays beside its resolved target, and an unresolvable path falls back to the literal path, which is correct for a first write and for a dangling link.
1 parent 3f1730b commit a391b3c

2 files changed

Lines changed: 90 additions & 6 deletions

File tree

src/config.ts

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { execFileSync } from "node:child_process";
22
import { randomUUID } from "node:crypto";
3-
import { chmodSync, copyFileSync, existsSync, linkSync, mkdirSync, readFileSync, renameSync, truncateSync, unlinkSync, writeFileSync } from "node:fs";
3+
import { chmodSync, copyFileSync, existsSync, linkSync, mkdirSync, readFileSync, realpathSync, renameSync, truncateSync, unlinkSync, writeFileSync } from "node:fs";
44
import { homedir } from "node:os";
55
import { join, resolve } from "node:path";
66
import { Database } from "bun:sqlite";
@@ -104,6 +104,28 @@ function isMissingPathError(error: unknown): boolean {
104104
return (error as NodeJS.ErrnoException | undefined)?.code === "ENOENT";
105105
}
106106

107+
/**
108+
* Resolve a write target through any symlink before the temp+rename dance.
109+
*
110+
* rename(2) replaces a directory ENTRY. When the entry is itself a symlink
111+
* (a dotfiles-managed `~/.codex/config.toml` -> `~/dotfiles/.codex/config.toml`,
112+
* say), renaming a sibling temp file over it destroys the link and leaves a plain
113+
* file behind — the repo silently stops receiving writes. Resolving first puts both
114+
* the temp file and the rename target inside the link's real directory, so the entry
115+
* being replaced is the real file and the symlink survives.
116+
*
117+
* Same-filesystem atomicity is preserved because the temp file stays beside its
118+
* resolved target. An unresolvable path (not yet created) falls back to the literal
119+
* path, which is the correct target for a first write.
120+
*/
121+
function resolveWriteTarget(path: string): string {
122+
try {
123+
return realpathSync(path);
124+
} catch {
125+
return path;
126+
}
127+
}
128+
107129
export function atomicWriteFile(path: string, content: string, io: AtomicWriteIO = {
108130
write: (target, value) => writeFileSync(target, value, { encoding: "utf-8", mode: 0o600 }),
109131
harden: target => {
@@ -115,13 +137,14 @@ export function atomicWriteFile(path: string, content: string, io: AtomicWriteIO
115137
unlink: unlinkSync,
116138
}): void {
117139
recordOwnedConfigPath(resolveConfigDir(), path);
118-
const tmp = `${path}.ocx.${process.pid}.${++_atomicSeq}.tmp`;
140+
const target = resolveWriteTarget(path);
141+
const tmp = `${target}.ocx.${process.pid}.${++_atomicSeq}.tmp`;
119142
let hardened = false;
120143
try {
121144
io.write(tmp, content);
122145
io.harden(tmp);
123146
hardened = true;
124-
io.rename(tmp, path);
147+
io.rename(tmp, target);
125148
forgetHardenedSecretPath(tmp);
126149
} catch (cause) {
127150
let scrubbed = false;
@@ -201,13 +224,14 @@ export async function atomicWriteFileAsync(
201224
truncate: target => truncateSync(target, 0),
202225
unlink: unlinkSync,
203226
};
204-
const tmp = `${path}.ocx.${process.pid}.${++_atomicSeq}.tmp`;
227+
const target = resolveWriteTarget(path);
228+
const tmp = `${target}.ocx.${process.pid}.${++_atomicSeq}.tmp`;
205229
let hardened = false;
206230
try {
207231
await effective.write(tmp, content);
208232
await effective.harden(tmp);
209233
hardened = true;
210-
await effective.rename(tmp, path);
234+
await effective.rename(tmp, target);
211235
forgetHardenedSecretPath(tmp);
212236
} catch (cause) {
213237
let scrubbed = false;

tests/config.test.ts

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test";
2-
import { chmodSync, existsSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, renameSync, rmSync, truncateSync, unlinkSync, writeFileSync } from "node:fs";
2+
import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, readlinkSync, renameSync, rmSync, symlinkSync, truncateSync, unlinkSync, writeFileSync } from "node:fs";
33
import { homedir, tmpdir } from "node:os";
44
import { join, resolve } from "node:path";
55
import {
@@ -1669,3 +1669,63 @@ describe("config.ts – Windows ACL hardening integration", () => {
16691669
spy.mockRestore();
16701670
});
16711671
});
1672+
1673+
describe("config.ts – atomic writes preserve symlinked destinations", () => {
1674+
test("a symlinked destination survives the write and the real file receives it", () => {
1675+
// Dotfiles shape: ~/.codex/config.toml -> ~/dotfiles/.codex/config.toml
1676+
const repoDir = join(testDir, "dotfiles");
1677+
mkdirSync(repoDir, { recursive: true });
1678+
const realFile = join(repoDir, "config.toml");
1679+
writeFileSync(realFile, "original", "utf-8");
1680+
const link = join(testDir, "config.toml");
1681+
symlinkSync(realFile, link);
1682+
1683+
atomicWriteFile(link, "rewritten");
1684+
1685+
expect(lstatSync(link).isSymbolicLink()).toBe(true);
1686+
expect(readlinkSync(link)).toBe(realFile);
1687+
expect(readFileSync(realFile, "utf8")).toBe("rewritten");
1688+
expect(readFileSync(link, "utf8")).toBe("rewritten");
1689+
});
1690+
1691+
test("no temp file is left beside the link or its target", () => {
1692+
const repoDir = join(testDir, "dotfiles-clean");
1693+
mkdirSync(repoDir, { recursive: true });
1694+
const realFile = join(repoDir, "config.toml");
1695+
writeFileSync(realFile, "original", "utf-8");
1696+
const link = join(testDir, "config-clean.toml");
1697+
symlinkSync(realFile, link);
1698+
1699+
atomicWriteFile(link, "rewritten");
1700+
1701+
expect(readdirSync(repoDir).filter(name => name.includes(".ocx."))).toEqual([]);
1702+
expect(readdirSync(testDir).filter(name => name.includes(".ocx."))).toEqual([]);
1703+
});
1704+
1705+
test("a plain destination is unaffected", () => {
1706+
const destination = join(testDir, "plain.toml");
1707+
atomicWriteFile(destination, "first");
1708+
atomicWriteFile(destination, "second");
1709+
1710+
expect(lstatSync(destination).isSymbolicLink()).toBe(false);
1711+
expect(readFileSync(destination, "utf8")).toBe("second");
1712+
});
1713+
1714+
test("a destination that does not exist yet is created at the literal path", () => {
1715+
const destination = join(testDir, "created.toml");
1716+
expect(existsSync(destination)).toBe(false);
1717+
1718+
atomicWriteFile(destination, "fresh");
1719+
1720+
expect(readFileSync(destination, "utf8")).toBe("fresh");
1721+
});
1722+
1723+
test("a dangling symlink is replaced rather than followed into nothing", () => {
1724+
const link = join(testDir, "dangling.toml");
1725+
symlinkSync(join(testDir, "gone", "config.toml"), link);
1726+
1727+
atomicWriteFile(link, "recovered");
1728+
1729+
expect(readFileSync(link, "utf8")).toBe("recovered");
1730+
});
1731+
});

0 commit comments

Comments
 (0)