fix(fs): preserve Windows DACLs across atomic replacement
Copy an existing target's DACL onto the empty staging file before any content is written, then publish with ReplaceFileW so Windows replacement keeps the target security descriptor instead of inheriting the broader parent policy. Keep new-file inheritance and POSIX mode behavior unchanged, retain the already-protected temp when a concurrently removed target requires rename fallback, and translate native errors into Node-style codes for the filesystem error boundary. Add host-independent Win32 binding coverage, native Windows descriptor assertions, package documentation, and a bilingual implemented RFC that supersedes the earlier inheritance-only replacement claim.
This commit is contained in:
Tianyi Cui
@@ -6,7 +6,7 @@
|
||||
*/
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
|
||||
import { chmod, mkdtemp, readFile, rm, stat, symlink, unlink, writeFile, mkdir, readdir, realpath } from 'node:fs/promises'
|
||||
import { chmod, mkdtemp, readFile, rename, rm, stat, symlink, unlink, writeFile, mkdir, readdir, realpath } from 'node:fs/promises'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { createServer } from 'node:net'
|
||||
@@ -23,6 +23,7 @@ import {
|
||||
writeFileAtomic,
|
||||
} from '../src/fsio.ts'
|
||||
import type { LocalTarget } from '../src/fsio.ts'
|
||||
import { copyFileDaclWin32, readFileDaclWin32 } from '../src/win32.ts'
|
||||
import { FsError, FsTargetKey } from '@deepseek-ai/dsh-fs'
|
||||
|
||||
let dir: string
|
||||
@@ -367,15 +368,15 @@ describe('streamWholeText', () => {
|
||||
})
|
||||
})
|
||||
|
||||
// Windows drives only the read-only attribute through `chmod` and reports
|
||||
// synthetic `stat` mode bits, so mode assertions are POSIX-only; on Windows
|
||||
// write-in-progress privacy comes from the destination directory's inherited
|
||||
// DACL (docs/rfc/implemented/architecture/2026-07-05-windows-fs-permissions.md).
|
||||
// Windows drives only the read-only attribute through `chmod` and reports synthetic `stat` mode
|
||||
// bits, so mode assertions are POSIX-only; native DACL preservation is asserted separately.
|
||||
const posixModes = process.platform !== 'win32'
|
||||
|
||||
describe('writeFileAtomic — temp-file safety', () => {
|
||||
it('writes through a private staging dir and owner-only temp file', async () => {
|
||||
const file = join(dir, 'a.txt')
|
||||
await writeFile(file, 'old')
|
||||
if (posixModes) await chmod(file, 0o640)
|
||||
let inspected = false
|
||||
await writeFileAtomic(file, 'hello', 0o640, undefined, {
|
||||
inspectTemp: async ({ stagingDir, tempPath }) => {
|
||||
@@ -395,6 +396,84 @@ describe('writeFileAtomic — temp-file safety', () => {
|
||||
expect((await readdir(dir)).filter(n => n.includes('.tmp'))).toEqual([])
|
||||
})
|
||||
|
||||
it.skipIf(process.platform !== 'win32')('protects staged content with the existing target DACL and preserves it after replacement', async () => {
|
||||
const file = join(dir, 'protected.txt')
|
||||
await writeFile(file, 'old')
|
||||
await copyFileDaclWin32(file, file)
|
||||
const expectedDacl = await readFileDaclWin32(file)
|
||||
|
||||
await writeFileAtomic(file, 'new', (await stat(file)).mode, undefined, {
|
||||
inspectTemp: async ({ tempPath }) => {
|
||||
expect(await readFileDaclWin32(tempPath)).toEqual(expectedDacl)
|
||||
},
|
||||
})
|
||||
|
||||
expect(await readFile(file, 'utf8')).toBe('new')
|
||||
expect(await readFileDaclWin32(file)).toEqual(expectedDacl)
|
||||
})
|
||||
|
||||
it('copies a Windows target DACL before content and publishes through secure replacement', async () => {
|
||||
const file = join(dir, 'a.txt')
|
||||
await writeFile(file, 'old')
|
||||
const calls: string[] = []
|
||||
|
||||
await writeFileAtomic(file, 'new', 0o666, undefined, {
|
||||
platform: 'win32',
|
||||
copyFileDacl: async (source, temp) => {
|
||||
calls.push(`copy:${source}`)
|
||||
expect(await readFile(temp, 'utf8')).toBe('')
|
||||
},
|
||||
replaceFile: async (target, temp) => {
|
||||
calls.push(`replace:${target}`)
|
||||
await rename(temp, target)
|
||||
},
|
||||
})
|
||||
|
||||
expect(calls).toEqual([`copy:${file}`, `replace:${file}`])
|
||||
expect(await readFile(file, 'utf8')).toBe('new')
|
||||
})
|
||||
|
||||
it('creates a new Windows file through directory inheritance without replacement calls', async () => {
|
||||
const file = join(dir, 'new.txt')
|
||||
const unexpected = async (): Promise<void> => { throw new Error('unexpected native replacement call') }
|
||||
|
||||
await writeFileAtomic(file, 'new', undefined, undefined, {
|
||||
platform: 'win32',
|
||||
copyFileDacl: unexpected,
|
||||
replaceFile: unexpected,
|
||||
})
|
||||
|
||||
expect(await readFile(file, 'utf8')).toBe('new')
|
||||
})
|
||||
|
||||
it('recreates a vanished Windows target with the already-protected temp', async () => {
|
||||
const file = join(dir, 'a.txt')
|
||||
await writeFile(file, 'old')
|
||||
const missing = Object.assign(new Error('target vanished'), { code: 'ENOENT' })
|
||||
|
||||
await writeFileAtomic(file, 'new', 0o666, undefined, {
|
||||
platform: 'win32',
|
||||
copyFileDacl: () => Promise.resolve(),
|
||||
replaceFile: async () => { throw missing },
|
||||
})
|
||||
|
||||
expect(await readFile(file, 'utf8')).toBe('new')
|
||||
})
|
||||
|
||||
it('surfaces a Windows secure-replacement failure and cleans the staging directory', async () => {
|
||||
const file = join(dir, 'a.txt')
|
||||
await writeFile(file, 'old')
|
||||
const denied = Object.assign(new Error('replace denied'), { code: 'EACCES' })
|
||||
|
||||
await expect(writeFileAtomic(file, 'new', 0o666, undefined, {
|
||||
platform: 'win32',
|
||||
copyFileDacl: () => Promise.resolve(),
|
||||
replaceFile: async () => { throw denied },
|
||||
})).rejects.toBe(denied)
|
||||
expect(await readFile(file, 'utf8')).toBe('old')
|
||||
expect((await readdir(dir)).filter(name => name.includes('.tmp'))).toEqual([])
|
||||
})
|
||||
|
||||
it.skipIf(!posixModes)('creates new files owner-only by default', async () => {
|
||||
const file = join(dir, 'a.txt')
|
||||
await writeFileAtomic(file, 'hello', undefined, undefined)
|
||||
|
||||
145
packages/fs/fs-local/tests/win32.spec.ts
Normal file
145
packages/fs/fs-local/tests/win32.spec.ts
Normal file
@@ -0,0 +1,145 @@
|
||||
/** Host-independent binding tests for the Win32 DACL and replacement helpers. */
|
||||
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
type GetFileSecurityW = (
|
||||
path: string,
|
||||
requestedInformation: number,
|
||||
descriptor: Buffer | null,
|
||||
length: number,
|
||||
needed: [number],
|
||||
) => number
|
||||
type SetFileSecurityW = (path: string, securityInformation: number, descriptor: Buffer) => number
|
||||
type ReplaceFileW = (
|
||||
replaced: string,
|
||||
replacement: string,
|
||||
backup: null,
|
||||
flags: number,
|
||||
exclude: null,
|
||||
reserved: null,
|
||||
) => number
|
||||
|
||||
interface NativeMock {
|
||||
getFileSecurityW: GetFileSecurityW
|
||||
setFileSecurityW: SetFileSecurityW
|
||||
replaceFileW: ReplaceFileW
|
||||
getLastError: () => number
|
||||
}
|
||||
|
||||
async function importWithNative(native: NativeMock): Promise<typeof import('../src/win32.ts')> {
|
||||
vi.resetModules()
|
||||
vi.doMock('koffi', () => ({
|
||||
default: {
|
||||
load: () => ({
|
||||
func: (definition: string) => {
|
||||
if (definition.includes('GetFileSecurityW')) return native.getFileSecurityW
|
||||
if (definition.includes('SetFileSecurityW')) return native.setFileSecurityW
|
||||
if (definition.includes('ReplaceFileW')) return native.replaceFileW
|
||||
if (definition.includes('GetLastError')) return native.getLastError
|
||||
throw new Error(`unexpected native function: ${definition}`)
|
||||
},
|
||||
}),
|
||||
},
|
||||
}))
|
||||
return import('../src/win32.ts')
|
||||
}
|
||||
|
||||
function successfulNative(descriptor: Buffer): NativeMock & { installed: Buffer[]; replacements: string[][] } {
|
||||
let lastError = 0
|
||||
const installed: Buffer[] = []
|
||||
const replacements: string[][] = []
|
||||
return {
|
||||
installed,
|
||||
replacements,
|
||||
getLastError: () => lastError,
|
||||
getFileSecurityW: (_path, _requested, output, _length, needed) => {
|
||||
needed[0] = descriptor.length
|
||||
if (output === null) {
|
||||
lastError = 122
|
||||
return 0
|
||||
}
|
||||
descriptor.copy(output)
|
||||
lastError = 0
|
||||
return 1
|
||||
},
|
||||
setFileSecurityW: (_path, information, value) => {
|
||||
expect(information).toBe(0x80000004)
|
||||
installed.push(Buffer.from(value))
|
||||
lastError = 0
|
||||
return 1
|
||||
},
|
||||
replaceFileW: (replaced, replacement, backup, flags, exclude, reserved) => {
|
||||
expect([backup, flags, exclude, reserved]).toEqual([null, 0, null, null])
|
||||
replacements.push([replaced, replacement])
|
||||
lastError = 0
|
||||
return 1
|
||||
},
|
||||
}
|
||||
}
|
||||
|
||||
afterEach(() => {
|
||||
vi.doUnmock('koffi')
|
||||
vi.resetModules()
|
||||
})
|
||||
|
||||
describe('Windows file-security helpers', () => {
|
||||
it('reads and installs a protected DACL before replacing the destination', async () => {
|
||||
const descriptor = Buffer.from([1, 2, 3, 4])
|
||||
const native = successfulNative(descriptor)
|
||||
const { copyFileDaclWin32, readFileDaclWin32, replaceFileWin32 } = await importWithNative(native)
|
||||
|
||||
expect(await readFileDaclWin32('source')).toEqual(descriptor)
|
||||
await copyFileDaclWin32('source', 'temp')
|
||||
expect(native.installed).toEqual([descriptor])
|
||||
await replaceFileWin32('target', 'temp')
|
||||
expect(native.replacements).toEqual([['target', 'temp']])
|
||||
})
|
||||
|
||||
it('maps descriptor-size probe failures to Node-style codes', async () => {
|
||||
const cases = [[2, 'ENOENT'], [3, 'ENOENT'], [5, 'EACCES'], [9999, 'EIO']] as const
|
||||
for (const [win32Code, code] of cases) {
|
||||
const native = successfulNative(Buffer.from([1]))
|
||||
native.getFileSecurityW = (_path, _requested, _output, _length, needed) => {
|
||||
needed[0] = 0
|
||||
return 0
|
||||
}
|
||||
native.getLastError = () => win32Code
|
||||
const { readFileDaclWin32 } = await importWithNative(native)
|
||||
await expect(readFileDaclWin32('source')).rejects.toMatchObject({ code, win32Code, path: 'source' })
|
||||
}
|
||||
})
|
||||
|
||||
it('surfaces a descriptor read failure after the size probe', async () => {
|
||||
const native = successfulNative(Buffer.from([1, 2]))
|
||||
native.getFileSecurityW = (_path, _requested, _output, _length, needed) => {
|
||||
needed[0] = 2
|
||||
return 0
|
||||
}
|
||||
native.getLastError = () => 5
|
||||
const { readFileDaclWin32 } = await importWithNative(native)
|
||||
|
||||
await expect(readFileDaclWin32('source')).rejects.toMatchObject({ code: 'EACCES', syscall: 'GetFileSecurityW' })
|
||||
})
|
||||
|
||||
it('surfaces DACL installation and replacement failures', async () => {
|
||||
const setFailure = successfulNative(Buffer.from([1]))
|
||||
setFailure.setFileSecurityW = () => 0
|
||||
setFailure.getLastError = () => 5
|
||||
const setModule = await importWithNative(setFailure)
|
||||
await expect(setModule.copyFileDaclWin32('source', 'temp')).rejects.toMatchObject({
|
||||
code: 'EACCES',
|
||||
syscall: 'SetFileSecurityW',
|
||||
path: 'temp',
|
||||
})
|
||||
|
||||
const replaceFailure = successfulNative(Buffer.from([1]))
|
||||
replaceFailure.replaceFileW = () => 0
|
||||
replaceFailure.getLastError = () => 2
|
||||
const replaceModule = await importWithNative(replaceFailure)
|
||||
await expect(replaceModule.replaceFileWin32('target', 'temp')).rejects.toMatchObject({
|
||||
code: 'ENOENT',
|
||||
syscall: 'ReplaceFileW',
|
||||
path: 'target',
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user