Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
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
78 changes: 78 additions & 0 deletions packages/core/src/utils/atomicFileWrite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -269,4 +269,82 @@ describe('atomicWriteFile', () => {
path.normalize('../otherDir/target.txt'),
);
});

it.skipIf(process.platform === 'win32')(
'should use atomic rename when ownership matches (inode changes)',
async () => {
const filePath = path.join(tmpDir, 'mine.txt');
await fs.writeFile(filePath, 'original');
const inoBefore = (await fs.stat(filePath)).ino;

await atomicWriteFile(filePath, 'updated');

const statAfter = await fs.stat(filePath);
// Atomic rename produces a new inode.
expect(statAfter.ino).not.toBe(inoBefore);
expect(await fs.readFile(filePath, 'utf-8')).toBe('updated');
},
);

it.skipIf(
process.platform === 'win32' || typeof process.geteuid !== 'function',
)(
'should fall back to in-place write when atomic rename would change ownership',
async () => {
// Simulate a file owned by a different user by replacing process.geteuid
// so it reports a uid that doesn't match the file's real uid. The code
// should detect rename would strip ownership and fall back to in-place
// writeFile, which preserves the inode — our signal that fallback ran.
const filePath = path.join(tmpDir, 'shared.txt');
await fs.writeFile(filePath, 'original');
await fs.chmod(filePath, 0o664);

const realStat = await fs.stat(filePath);
const inoBefore = realStat.ino;
const realGeteuid = process.geteuid!;
process.geteuid = () => realStat.uid + 1;

try {
await atomicWriteFile(filePath, 'updated');
} finally {
process.geteuid = realGeteuid;
}

expect(await fs.readFile(filePath, 'utf-8')).toBe('updated');

const statAfter = await fs.stat(filePath);
// In-place write preserves the inode — proves rename was skipped.
expect(statAfter.ino).toBe(inoBefore);
// Permissions preserved.
expect(statAfter.mode & 0o777).toBe(0o664);
// No leftover temp file.
expect(await fs.readdir(tmpDir)).toEqual(['shared.txt']);
},
);

it.skipIf(
process.platform === 'win32' || typeof process.getegid !== 'function',
)(
'should fall back to in-place write when group differs from process gid',
async () => {
// Same scenario triggered via gid mismatch.
const filePath = path.join(tmpDir, 'shared-group.txt');
await fs.writeFile(filePath, 'original');

const realStat = await fs.stat(filePath);
const inoBefore = realStat.ino;
const realGetegid = process.getegid!;
process.getegid = () => realStat.gid + 1;

try {
await atomicWriteFile(filePath, 'updated');
} finally {
process.getegid = realGetegid;
}

const statAfter = await fs.stat(filePath);
expect(statAfter.ino).toBe(inoBefore);
Comment thread
doudouOUC marked this conversation as resolved.
Outdated
expect(await fs.readFile(filePath, 'utf-8')).toBe('updated');
},
);
});
62 changes: 55 additions & 7 deletions packages/core/src/utils/atomicFileWrite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/

import * as crypto from 'node:crypto';
import type { Stats } from 'node:fs';
import * as fs from 'node:fs/promises';
import * as path from 'node:path';
import { isNodeError } from './errors.js';
Expand Down Expand Up @@ -106,7 +107,15 @@ async function resolveSymlinkChain(filePath: string): Promise<string> {
* can leave a partially-written file. EXDEV only occurs when the
* resolved target path is on a different filesystem than its parent
* directory, which is rare in practice.
* 6. Always clean up the temp file on failure.
* 6. If the existing file is owned by a different uid/gid than the
* current process (and we are not root), fall back to in-place
* truncate+write instead of rename. POSIX rename creates a new inode
* owned by the process's euid/egid, which would silently strip the
* original ownership and break shared-write setups
* (e.g. group-writable files in a shared workspace). The in-place
* write preserves the inode — and therefore uid/gid — at the cost
* of crash atomicity.
* 7. Always clean up the temp file on failure.
*
* The parent directory of `filePath` must already exist.
*/
Expand All @@ -122,19 +131,19 @@ export async function atomicWriteFile(

const targetPath = await resolveSymlinkChain(filePath);

const tmpPath = `${targetPath}.${crypto.randomBytes(6).toString('hex')}.tmp`;

// Stat the target to preserve existing permissions (mask out file-type bits).
let existingMode: number | undefined;
// Stat the target to preserve existing permissions and detect
// ownership-changing renames (see step 6 in the function doc).
let existingStat: Stats | undefined;
try {
const stat = await fs.stat(targetPath);
existingMode = stat.mode & 0o7777;
existingStat = await fs.stat(targetPath);
} catch (err) {
if (!isNodeError(err) || err.code !== 'ENOENT') {
throw err;
}
}

const existingMode =
existingStat !== undefined ? existingStat.mode & 0o7777 : undefined;
const desiredMode = existingMode ?? options?.mode;

const writeOptions: {
Expand All @@ -156,6 +165,31 @@ export async function atomicWriteFile(
}
};

// Detect when atomic rename would silently change ownership. POSIX
// rename creates a new inode owned by the process's euid/egid; if the
// existing file is owned by someone else (or has a different group),
// shared-write users would lose access. Fall back to in-place write,
// which truncates the existing inode and so preserves uid/gid.
// Skipped on Windows (no POSIX ownership), for new files (no owner to
// preserve), and for root (rename + chown-back works; see below).
const ownershipWouldChange = (): boolean => {
if (existingStat === undefined) return false;
if (process.platform === 'win32') return false;
const euid = process.geteuid?.();
const egid = process.getegid?.();
if (euid === undefined || egid === undefined) return false;
Comment thread
doudouOUC marked this conversation as resolved.
Outdated
if (euid === 0) return false;
return existingStat.uid !== euid || existingStat.gid !== egid;
};

if (ownershipWouldChange()) {
await fs.writeFile(targetPath, data, writeOptions);
Comment thread
doudouOUC marked this conversation as resolved.
Comment thread
doudouOUC marked this conversation as resolved.
await tryChmod(targetPath);
return;
}

const tmpPath = `${targetPath}.${crypto.randomBytes(6).toString('hex')}.tmp`;
Comment thread
doudouOUC marked this conversation as resolved.

try {
await fs.writeFile(tmpPath, data, writeOptions);
await tryChmod(tmpPath);
Expand All @@ -177,6 +211,20 @@ export async function atomicWriteFile(

throw error;
}

// Root: rename produced a root-owned inode. Restore the original
// uid/gid so root invocations don't leak ownership to user files.
if (
existingStat !== undefined &&
process.platform !== 'win32' &&
process.geteuid?.() === 0
) {
try {
await fs.chown(targetPath, existingStat.uid, existingStat.gid);
} catch {
// Best-effort. Not all filesystems support chown.
}
Comment thread
doudouOUC marked this conversation as resolved.
Outdated
}
}

/**
Expand Down
Loading