diff --git a/docs/rfc/INDEX.md b/docs/rfc/INDEX.md index ceaf70cd87..6ca005be8d 100644 --- a/docs/rfc/INDEX.md +++ b/docs/rfc/INDEX.md @@ -89,6 +89,12 @@ Generated by `pnpm run gen-rfc-index` from the RFC tree — never edit by hand; | [Optional time-context plugin](implemented/feature/2026-07-14-time-context-plugin.md) | 2026-07-14 | | [Durable per-step time context](implemented/feature/2026-07-16-durable-per-step-time-context.md) | 2026-07-16 | +### Bug-fix + +| Title | First proposed | +|---|---| +| [Preserve Windows DACLs during atomic file replacement](implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.md) | 2026-07-19 | + ### Simplification | Title | First proposed | diff --git a/docs/rfc/implemented/architecture/2026-07-05-windows-fs-permissions.md b/docs/rfc/implemented/architecture/2026-07-05-windows-fs-permissions.md index 0001e8223c..4d477990cd 100644 --- a/docs/rfc/implemented/architecture/2026-07-05-windows-fs-permissions.md +++ b/docs/rfc/implemented/architecture/2026-07-05-windows-fs-permissions.md @@ -2,21 +2,23 @@ Status: implemented +The replacement-file decision in this record is superseded by [Windows DACL preservation](../bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.md). + ## Problem `writeFileAtomic` in `@deepseek-ai/dsh-fs-local` protects write-in-progress content with POSIX mode bits: the staging directory is created `0o700`, the temp file is opened `0o600`, and new files default to `0o600`. On POSIX this keeps temporary content owner-only regardless of the parent directory's permissions. -Windows has no working equivalent behind the same API. Node's `chmod` there drives only the read-only attribute (every mode this package passes carries owner-write, so the calls are benign no-ops), and `stat().mode` reports synthetic `0o666`/`0o444` bits. The real security state is the file's DACL, which this code never sets; a newly created file or directory inherits its DACL from its parent directory. +Windows has no working equivalent behind the same API. Node's `chmod` there drives only the read-only attribute (every mode this package passes carries owner-write, so the calls are benign no-ops), and `stat().mode` reports synthetic `0o666`/`0o444` bits. The real security state is the file's DACL: a newly created file or directory inherits from its parent, while replacement needs the explicit handling owned by the superseding RFC. ## Decision -Production code is unchanged: no platform fork, no DACL management. The Windows privacy invariant is structural rather than mode-driven — the staging directory is created inside the target's parent directory (`dirname(absolutePath)`), so it and the temp file inherit exactly the destination directory's DACL, and write-in-progress content is never exposed more widely than the destination itself. In the typical deployment (a coding agent writing the user's own project tree under `C:\Users\\`) the inherited DACL is owner + SYSTEM + Administrators, matching the POSIX intent. +New Windows files use directory inheritance rather than synthetic mode bits: the staging directory is created inside the target's parent directory (`dirname(absolutePath)`), so it and the temp file inherit the destination directory's DACL. Replacement files follow the stricter [DACL preservation contract](../bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.md). -Tests assert mode bits on POSIX only. There is no Windows-side ACL assertion because there is no Windows-side code behavior to pin: an ACL check on a `mkdtemp(tmpdir())` fixture would verify Windows DACL inheritance plus the machine's `%TEMP%` ACL — the operating system, not this package — and no change to this package could turn it red. +Tests assert mode bits on POSIX only. Native Windows coverage pins the package-owned replacement behavior; new-file inheritance remains an operating-system contract rather than a machine-specific ACL allowlist. ## Alternatives considered -**Explicit protected DACLs.** Granting owner-only access would require per-write FFI or a subprocess, break inheritance, and surprise users whose project directories are deliberately shared. This becomes appropriate only if the threat model includes hostile local readers of broadly accessible target directories. +**Explicit owner-only DACLs for new files.** Rejected because they would break inheritance and surprise users whose project directories are deliberately shared. Replacement writes copy the target's existing DACL rather than inventing an owner-only policy. **Test-side ACL verification.** A `Get-Acl` SID allowlist or `icacls` would verify Windows inheritance and the machine's `%TEMP%` ACL rather than package behavior; `icacls` also localizes well-known account names, making parsing locale-fragile. @@ -24,6 +26,6 @@ Tests assert mode bits on POSIX only. There is no Windows-side ACL assertion bec ## Consequences -POSIX keeps the stronger guarantee: owner-only temp content regardless of the parent directory. Windows guarantees only "no wider than the destination": a target inside a broadly accessible directory (a share, a permissive `D:\` root) gets equally accessible write-in-progress content. The gap is deliberate and documented, not an oversight. +POSIX keeps owner-only temp content regardless of the parent directory. A new Windows target inside a broadly accessible directory inherits that accessibility by design; a replacement retains the target's narrower DACL when one exists. -Mode preservation across a replace degenerates to a no-op on Windows: a writable file probes as `0o666`, and replaying that through `chmod` leaves the read-only attribute clear. A read-only target cannot be replaced at all there — `rename` over it fails before the preserved mode would matter. +Mode preservation across a replace degenerates to a no-op on Windows: a writable file probes as `0o666`, and replaying that through `chmod` leaves the read-only attribute clear. A read-only target cannot be replaced there because publication fails before the synthetic mode would matter. diff --git a/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.i18n.yaml b/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.i18n.yaml new file mode 100644 index 0000000000..5f37bf3ca3 --- /dev/null +++ b/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write +2026-07-19-windows-atomic-write-dacl-preservation.md: 393ce8a992b8c0b7b580f2c794e098d66e14258e +2026-07-19-windows-atomic-write-dacl-preservation.zh.md: c7a0b6278cf739cc5ef4432d679e48b88b61d198 diff --git a/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.md b/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.md new file mode 100644 index 0000000000..393ce8a992 --- /dev/null +++ b/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.md @@ -0,0 +1,27 @@ +# RFC: Preserve Windows DACLs during atomic file replacement + +Status: implemented + +English | [中文](2026-07-19-windows-atomic-write-dacl-preservation.zh.md) + +## Problem + +On Windows, creating the staging directory and temp file under the target's parent and relying only on inherited DACLs is sufficient for a new file, but not for replacing an existing file whose explicit or protected DACL is narrower than its parent: content is written under the broader parent DACL, and rename carries that staging descriptor onto the replacement. + +## Decision + +`dsh-fs-local` reads an existing target's DACL with `GetFileSecurityW`, applies it to the empty temp file with inheritance protected before writing content, and publishes the closed temp with `ReplaceFileW`. The protected staging descriptor prevents the temp directory's inherited entries from broadening access; `ReplaceFileW` preserves the original target security descriptor and other replacement metadata. New files have no prior descriptor to preserve and continue to inherit the destination directory's DACL. + +Native Windows coverage protects a target DACL, inspects the written staging file, and compares the final replacement descriptor. Host-independent binding tests cover Win32 error translation and every native call boundary. + +## Alternatives considered + +**Rely on directory inheritance for replacements.** Rejected because a target may carry a narrower explicit or protected DACL than its parent, so inheritance neither protects staged content nor preserves the target access policy. + +**Use `ReplaceFileW` without protecting the temp.** Rejected because it repairs the final descriptor only after the content has already been written under the staging file's inherited DACL. + +**Install an owner-only DACL for every write.** Rejected because it would discard deliberate project sharing. Copying the target DACL preserves the deployment's existing access policy instead of inventing one. + +## Consequences + +Replacing a Windows file now requires permission to read the target DACL and set the temp DACL; failure is loud before content is written. The package carries Koffi for the narrow Win32 calls, loaded only on Windows replacement paths. New-file behavior remains directory-inherited, and POSIX mode behavior is unchanged. diff --git a/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.zh.md b/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.zh.md new file mode 100644 index 0000000000..c7a0b6278c --- /dev/null +++ b/docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.zh.md @@ -0,0 +1,27 @@ +# RFC: Windows 原子文件替换期间保留 DACL + +Status: implemented + +[English](2026-07-19-windows-atomic-write-dacl-preservation.md) | 中文 + +## 问题 + +在 Windows 上，在目标文件的父目录下创建暂存目录和临时文件，并且只依赖继承的 DACL，足以满足新建文件的需要，但无法安全替换显式或受保护 DACL 比父目录更严格的现有文件：内容会在权限更宽松的父目录 DACL 下写入，而重命名又会把这个暂存安全描述符带到替换后的文件上。 + +## 决策 + +`dsh-fs-local` 通过 `GetFileSecurityW` 读取现有目标文件的 DACL，在写入内容前将其以禁止继承的形式应用到空临时文件，并通过 `ReplaceFileW` 发布已关闭的临时文件。受保护的暂存安全描述符可防止暂存目录中的继承条目扩大访问权限；`ReplaceFileW` 会保留原目标文件的安全描述符及其他替换元数据。新建文件没有既有描述符需要保留，因此仍继承目标目录的 DACL。 + +Windows 原生覆盖率测试会保护目标文件的 DACL、检查写入完成的暂存文件，并对比最终替换文件的描述符。与宿主平台无关的绑定测试覆盖 Win32 错误转换以及每个原生调用边界。 + +## 备选方案 + +**替换文件时依赖目录继承。** 不予采用，因为目标文件可能带有比父目录更严格的显式或受保护 DACL；目录继承既无法保护暂存内容，也无法保留目标文件的访问策略。 + +**使用 `ReplaceFileW`，但不保护临时文件。** 不予采用，因为这只能在内容已经按暂存文件继承的 DACL 写入之后修复最终描述符。 + +**每次写入都设置仅所有者可访问的 DACL。** 不予采用，因为这会破坏项目有意设置的共享权限。复制目标文件的 DACL 可以保留部署中已有的访问策略，无需另行创设策略。 + +## 影响 + +替换 Windows 文件现在要求调用方有权读取目标 DACL 并设置临时文件 DACL；如果权限不足，系统会在写入内容前明确失败。该包（package）引入 Koffi 以执行少量 Win32 调用，并且只在 Windows 替换路径上加载。新建文件仍按目录继承，POSIX mode 行为保持不变。 diff --git a/packages/fs/fs-local/README.md b/packages/fs/fs-local/README.md index 7c3899e5cb..551492db67 100644 --- a/packages/fs/fs-local/README.md +++ b/packages/fs/fs-local/README.md @@ -16,7 +16,7 @@ await ctx.plugin(LocalFileSystem, { cwd: process.cwd() }) - **`stat` / `lstat`** — return target metadata or `undefined` when absent. `stat` reports `FsInfo` for an already resolved target (`version` = an opaque token derived from bigint `dev:ino:size:mtimeNs:ctimeNs`, `type` of `file`/`directory`/`other`, byte `size`); path-shaped `lstat` reports `FsPathInfo` without following the final symlink and can therefore return `symlink`. Both check cancellation before and after their asynchronous metadata probe, so an abort that lands in flight reports `FS_ABORTED` rather than stale absence. - **`readText` / `streamText`** — UTF-8 only. `readText` reads the whole file; `streamText` streams it in chunks (cross-chunk decoding) so a huge file never has to be held whole in memory. Both reject invalid UTF-8 and NUL-byte binary samples (`FS_NOT_TEXT`) and non-regular targets. The `read` tool (`@deepseek-ai/dsh-tool-fs`) decides which to call by size and owns the line windowing. - **`listDir`** — lists one directory level in stable `name.localeCompare()` order. Each entry carries the child basename, type, resolved child target (`displayPath` under the listed directory, `targetKey` as the realpath identity), and cheap stat metadata (`version`, plus `size` for regular files). It never opens or decodes file contents. Missing targets report `FS_NOT_FOUND`, file/special-file targets report `FS_NOT_DIRECTORY`, aborted calls report `FS_ABORTED`, permission failures report `FS_PERMISSION_DENIED`, and other listing or child metadata I/O failures report `FS_IO_ERROR`. Broken/disappeared children are returned as `other` without metadata, but permission/IO failures while resolving a child fail the whole listing with a structured `FsError`. -- **`writeText`** — atomic: writes to a temp file opened exclusively (`wx`, `0o600`) inside a randomly-named private staging dir (`0o700`) next to the target, fsyncs, then renames over the target. An existing file's mode is preserved, while new files default to `0o600`; on Windows the mode bits drive only the read-only attribute, and write-in-progress privacy comes instead from the staging dir inheriting the destination directory's DACL ([Windows write-permission RFC](../../../docs/rfc/implemented/architecture/2026-07-05-windows-fs-permissions.md)). The `expected` guard is OPTIONAL: omitting it unconditionally creates-or-overwrites; `createIfAbsent` creates a missing target and rejects an existing one (`FS_NOT_OBSERVED`); `replaceIfVersion` replaces only at the observed version (a missing target or mismatch is `FS_STALE_VERSION`). +- **`writeText`** — atomic: writes to a temp file opened exclusively (`wx`, `0o600`) inside a randomly-named private staging dir (`0o700`) next to the target, fsyncs, then renames over the target. An existing file's mode is preserved, while new files default to `0o600`; on Windows a new file inherits the destination directory's DACL, while replacement copies the target DACL onto the empty temp before writing and publishes through `ReplaceFileW` so the original descriptor survives ([Windows DACL preservation RFC](../../../docs/rfc/implemented/bug-fix/2026-07-19-windows-atomic-write-dacl-preservation.md)). The `expected` guard is OPTIONAL: omitting it unconditionally creates-or-overwrites; `createIfAbsent` creates a missing target and rejects an existing one (`FS_NOT_OBSERVED`); `replaceIfVersion` replaces only at the observed version (a missing target or mismatch is `FS_STALE_VERSION`). - **`editText`** — atomic literal read-modify-write over the same primitive, serialized per target by a mutation lock. The `expected` guard is OPTIONAL: when supplied it verifies the version BEFORE literal matching (a stale edit reports `FS_STALE_VERSION`, never `FS_EDIT_NOT_FOUND`/`FS_AMBIGUOUS_EDIT` against newer content); omitting it edits the current content unconditionally. A missing target reports `FS_STALE_VERSION` either way. LF-normalizes for matching, restores the file's dominant CRLF/LF style, and rejects empty `oldString` / zero matches (`FS_EDIT_NOT_FOUND`) or ambiguous multi-matches without `replace_all` (`FS_AMBIGUOUS_EDIT`). The package-root SDK surface is the default/named `LocalFileSystem` class plus `Config`. Raw I/O lives in `src/fsio.ts` (Cordis-free, independently unit-tested); `src/index.ts` is the thin service wiring. diff --git a/packages/fs/fs-local/package.json b/packages/fs/fs-local/package.json index dd80cb4d9c..ded2a9d55f 100644 --- a/packages/fs/fs-local/package.json +++ b/packages/fs/fs-local/package.json @@ -26,6 +26,7 @@ "cordis": "^4.0.0-rc.7" }, "dependencies": { + "koffi": "^3.1.0", "schemastery": "^3.18.0" }, "devDependencies": { diff --git a/packages/fs/fs-local/src/fsio.ts b/packages/fs/fs-local/src/fsio.ts index 05d957105a..549554043f 100644 --- a/packages/fs/fs-local/src/fsio.ts +++ b/packages/fs/fs-local/src/fsio.ts @@ -12,6 +12,7 @@ import type { BigIntStats, Dirent, Stats } from 'node:fs' import { basename, dirname, join, resolve } from 'node:path' import { TextDecoder } from 'node:util' import { FsError, FsTargetKey, FsVersion } from '@deepseek-ai/dsh-fs' +import { copyFileDaclWin32, replaceFileWin32 } from './win32.ts' const BINARY_SAMPLE_BYTES = 8192 @@ -74,10 +75,16 @@ function versionOf(info: BigIntStats): FsVersion { * file before it is renamed over the target. */ export interface FsIoInternals { + /** Override the host platform for native-publication unit coverage. */ + platform?: NodeJS.Platform /** Override the generated private staging-dir name (relative to the target dir). */ tempDirName?: (writePath: string) => string /** Override the generated temp-file name (relative to the private staging dir). */ tempName?: (writePath: string) => string + /** Override the Win32 DACL copy boundary. */ + copyFileDacl?: (source: string, destination: string) => Promise + /** Override the Win32 security-preserving replacement boundary. */ + replaceFile?: (replaced: string, replacement: string) => Promise /** Test hook after the temp file is written/synced but before final chmod+rename. */ inspectTemp?: (paths: { stagingDir: string; tempPath: string }) => void | Promise } @@ -412,11 +419,13 @@ async function removeStagingDirOrThrow(stagingDir: string, originalError: unknow /** * Atomically replace a file through a private, synced staging file in the same directory. - * POSIX protects the staging directory and file with `0o700` and `0o600`; Windows - * inherits the destination directory's DACL because Node mode bits are synthetic there. + * POSIX protects the staging directory and file with `0o700` and `0o600`. A new Windows file + * inherits the destination directory's DACL; a replacement copies the existing target's DACL + * onto the empty temp before writing and preserves the target descriptor at publication. * @param absolutePath - destination; missing parent directories are created. * @param content - the full UTF-8 text to write. - * @param mode - final POSIX mode, or `0o600` when omitted; inert on Windows. + * @param mode - existing destination's POSIX mode to preserve, or `undefined` for a new file; + * inert as a mode on Windows but identifies replacement security semantics. * @param signal - cancellation checked before the final rename. * @param internals - test seam for pinning temp names and observing the staged file. */ @@ -436,6 +445,9 @@ export async function writeFileAtomic( const stagingDir = join(directory, stagingDirName) const tempName = internals.tempName?.(absolutePath) ?? `${basename(absolutePath)}.tmp` const tempPath = join(stagingDir, tempName) + const platform = internals.platform ?? process.platform + const copyFileDacl = internals.copyFileDacl ?? copyFileDaclWin32 + const replaceFile = internals.replaceFile ?? replaceFileWin32 let handle: Awaited> | undefined let stagingCreated = false try { @@ -445,6 +457,9 @@ export async function writeFileAtomic( handle = await open(tempPath, 'wx', 0o600) await handle.chmod(0o600) + if (platform === 'win32' && mode !== undefined) { + await copyFileDacl(absolutePath, tempPath) + } await handle.writeFile(content, { encoding: 'utf8', ...signal ? { signal } : {} }) await handle.sync() await internals.inspectTemp?.({ stagingDir, tempPath }) @@ -453,7 +468,18 @@ export async function writeFileAtomic( handle = undefined throwIfAborted(signal, 'write') - await rename(tempPath, absolutePath) + if (platform === 'win32' && mode !== undefined) { + try { + await replaceFile(absolutePath, tempPath) + } catch (error: unknown) { + // Preserve the old behavior when an external actor removes the observed target during + // staging: the temp already carries that target's protected DACL, so rename recreates it. + if (!isENOENT(error)) throw error + await rename(tempPath, absolutePath) + } + } else { + await rename(tempPath, absolutePath) + } await rm(stagingDir, { recursive: true, force: true }) } catch (error: unknown) { /* v8 ignore next -- abort-mid-write needs a writeFile/signal race; the non-abort (rename/open) side is tested. */ diff --git a/packages/fs/fs-local/src/win32.ts b/packages/fs/fs-local/src/win32.ts new file mode 100644 index 0000000000..6f459898a9 --- /dev/null +++ b/packages/fs/fs-local/src/win32.ts @@ -0,0 +1,134 @@ +/** + * Windows security-descriptor helpers for atomic local-file replacement. Koffi loads lazily so + * non-Windows processes never open Win32 libraries. + * @module @deepseek-ai/dsh-fs-local/win32 + */ + +import { toNamespacedPath } from 'node:path' + +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 +type GetLastError = () => number + +interface Win32Bindings { + getFileSecurityW: GetFileSecurityW + setFileSecurityW: SetFileSecurityW + replaceFileW: ReplaceFileW + getLastError: GetLastError +} + +interface Win32ErrnoException extends NodeJS.ErrnoException { + win32Code: number +} + +const DACL_SECURITY_INFORMATION = 0x00000004 +const PROTECTED_DACL_SECURITY_INFORMATION = 0x80000000 +const ERROR_FILE_NOT_FOUND = 2 +const ERROR_PATH_NOT_FOUND = 3 +const ERROR_ACCESS_DENIED = 5 + +let bindings: Win32Bindings | undefined + +async function win32(): Promise { + if (bindings !== undefined) return bindings + const koffi = (await import('koffi')).default + const advapi32 = koffi.load('advapi32.dll') + const kernel32 = koffi.load('kernel32.dll') + bindings = { + getFileSecurityW: advapi32.func('int __stdcall GetFileSecurityW(const char16_t *path, uint32_t requested, void *descriptor, uint32_t length, _Out_ uint32_t *needed)') as GetFileSecurityW, + setFileSecurityW: advapi32.func('int __stdcall SetFileSecurityW(const char16_t *path, uint32_t information, const void *descriptor)') as SetFileSecurityW, + replaceFileW: kernel32.func('int __stdcall ReplaceFileW(const char16_t *replaced, const char16_t *replacement, const char16_t *backup, uint32_t flags, void *exclude, void *reserved)') as ReplaceFileW, + getLastError: kernel32.func('uint32_t __stdcall GetLastError()') as GetLastError, + } + return bindings +} + +function errnoCode(win32Code: number): string { + switch (win32Code) { + case ERROR_FILE_NOT_FOUND: + case ERROR_PATH_NOT_FOUND: + return 'ENOENT' + case ERROR_ACCESS_DENIED: + return 'EACCES' + default: + return 'EIO' + } +} + +function win32Error(syscall: string, win32Code: number, path: string): Win32ErrnoException { + const code = errnoCode(win32Code) + const error = new Error(`${syscall} ${code} (Win32 ${win32Code}): ${path}`) as Win32ErrnoException + error.code = code + error.errno = win32Code + error.syscall = syscall + error.path = path + error.win32Code = win32Code + return error +} + +/** + * Read a file's self-relative DACL security descriptor. + * @param path - existing file whose DACL is read. + * @returns a descriptor buffer accepted by `SetFileSecurityW`. + */ +export async function readFileDaclWin32(path: string): Promise { + const api = await win32() + const nativePath = toNamespacedPath(path) + const needed: [number] = [0] + api.getFileSecurityW(nativePath, DACL_SECURITY_INFORMATION, null, 0, needed) + if (needed[0] === 0) throw win32Error('GetFileSecurityW', api.getLastError(), path) + + const descriptor = Buffer.alloc(needed[0]) + if (api.getFileSecurityW(nativePath, DACL_SECURITY_INFORMATION, descriptor, descriptor.length, needed) === 0) { + throw win32Error('GetFileSecurityW', api.getLastError(), path) + } + return descriptor.subarray(0, needed[0]) +} + +/** + * Copy an existing file's DACL onto another file and protect it from staging-parent inheritance. + * The destination must still be empty when confidentiality depends on this call. + * @param source - existing file whose DACL is copied. + * @param destination - existing file that receives the protected DACL. + */ +export async function copyFileDaclWin32(source: string, destination: string): Promise { + const descriptor = await readFileDaclWin32(source) + const api = await win32() + const information = (DACL_SECURITY_INFORMATION | PROTECTED_DACL_SECURITY_INFORMATION) >>> 0 + if (api.setFileSecurityW(toNamespacedPath(destination), information, descriptor) === 0) { + throw win32Error('SetFileSecurityW', api.getLastError(), destination) + } +} + +/** + * Replace a Windows file while preserving the replaced file's ACL and other replace metadata. + * @param replaced - existing destination file. + * @param replacement - closed staging file on the same volume. + */ +export async function replaceFileWin32(replaced: string, replacement: string): Promise { + const api = await win32() + if (api.replaceFileW( + toNamespacedPath(replaced), + toNamespacedPath(replacement), + null, + 0, + null, + null, + ) === 0) { + throw win32Error('ReplaceFileW', api.getLastError(), replaced) + } +} diff --git a/packages/fs/fs-local/tests/fsio.spec.ts b/packages/fs/fs-local/tests/fsio.spec.ts index 559ef86563..d292f0bfb7 100644 --- a/packages/fs/fs-local/tests/fsio.spec.ts +++ b/packages/fs/fs-local/tests/fsio.spec.ts @@ -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 => { 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) diff --git a/packages/fs/fs-local/tests/win32.spec.ts b/packages/fs/fs-local/tests/win32.spec.ts new file mode 100644 index 0000000000..dc2b69ea82 --- /dev/null +++ b/packages/fs/fs-local/tests/win32.spec.ts @@ -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 { + 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', + }) + }) +}) diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5c591034cc..0ed8c8449c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -819,6 +819,9 @@ importers: packages/fs/fs-local: dependencies: + koffi: + specifier: ^3.1.0 + version: 3.1.1 schemastery: specifier: ^3.18.0 version: 3.18.0