Repository navigation
fix(file): let a created file inherit its directory DACL - #571
tietoumiao wants to merge 1 commit into
Conversation
A file created by ops_write_file kept only the caller's explicit ACE. Staging builds a protected D:P(A;;FA;;;<sid>) descriptor before any byte exists and publication hard-links that inode, so the directory's inheritable grants - including the sandbox write capability the host granted the workspace - never reached the published file. The same session could not delete what it had just created, and other tools or accounts the directory grants lost access (T-Auto#420). The staged object stays private until its bytes are complete; ops_write_file then asks the operating system to recompute that DACL from the directory with UNPROTECTED semantics, which keeps the caller's explicit full-control entry, adds every object-inheritable directory entry as an inherited ACE, and leaves directory deny entries in force because the kernel merges them instead of this crate rebuilding the set. Only creation adopts: an overwritten file keeps its own descriptor, a copy keeps its source descriptor, and cache objects stay private. The fail-closed verification compares generic rights as the specific rights Windows propagates: a directory entry spelled GA reaches a file as FA, SDGXGWGR as 0x1301bf and GXGR as 0x1200a9, so MapGenericMask reduces both sides before the exact match. Without that reduction a correctly inherited generic entry reads as missing and every workspace whose ACL carries inherit-only generic entries refuses the create. Asserted twice: native/windows-metadata creates a child under a directory with a distinctive inheritable grant plus an inherit-only generic grant and requires both plus the caller's explicit ACE, and test/file-inherited-dacl drives the real worker to create a file, compares its SDDL against the directory's inheritable entries with the same mapping, and deletes it in the same session. Removing the DACL write turns the native test red with 'Adopted DACL is still protected'; dropping the mask mapping turns it red with 'missing a directory grant'. Existing files do not self-heal: a file published by an older version keeps its descriptor until it is recreated. Documented in the Windows security regression map.
|
This overlaps #547's create-only DACL work. The two implementations should be reviewed as alternatives, with ordinary
Also keep the desired semantics explicit: preserving an extra caller full-control ACE differs from the ordinary file's inherited permission set, especially with inherited denies. This source review does not claim to have run this branch; the original work remains available for comparison. |
Nicholas023
left a comment
There was a problem hiding this comment.
审查结论:REQUEST_CHANGES — 建议关闭
方向(新建后让 OS 重算 DACL)是对的,但本 PR 引入了一个 0.5.0 基线没有的崩溃路径。
1. NULL DACL 父目录会让 worker panic(阻断,新缺陷)
父目录为 NULL DACL 时,本 PR 的 worker:
text exit: 101 stderr: thread 'main' panicked at windows-permissions-0.2.4/.../get_security_descriptor_dacl_sacl.rs:53:1: $f indicated success but returned NULL dir now: [".c.txt.dsh-ops-31cf596-…"] ← staging 泄漏
控制变量对照(唯一变量 = worker):
text worker NULL DACL 父目录 基线 8934fd33 exit 0,completed,文件正常创建 本 PR fb4421a4 exit 101,panic,staging 泄漏
let Some(dacl) = sd.dacl() else … 的防御是死代码 —— 该库对 present-but-NULL DACL 直接 panic。这违反「工具失败必须是返回值,不得 panic」。
2. 新 e2e 在非 Windows 静默通过(阻断)
test/file-inherited-dacl.test.mjs 的 report() / process.exit() 都在 if (process.platform === 'win32') 内部。模拟非 Windows 后 runner 报 1 suite(s) passed —— 零检查算通过,与「skip 不算 pass」相反,且 verify-gate 的 POSIX allow-skip 白名单里也没有它。
3. 与 #547 重叠且更弱
verify_adopted() 按「不变 SID」逐条比对,会被 Windows 的 CREATOR OWNER/CREATOR GROUP 替换规则误拒(父目录 (A;OIIO;FA;;;CO) 传播后变成 owner SID);staging 句柄仍用 FILE_SHARE_READ|WRITE|DELETE,adopt 扩权后到硬链接发布前存在写共享窗口。#547 用 CreatePrivateObjectSecurityEx + held parent descriptor,并在整个发布生命周期排除 FILE_SHARE_WRITE。
4. e2e 对环境假设脆弱
默认环境下直接红:ACCESS_DENIED(受限令牌写 %TEMP%),且 whoami 解析到 Git coreutils 报 extra operand '/user'。
已核实为真的部分
- 0.5.0 基线上真 worker 创建的文件 DACL 只有 1 条非继承 ACE;同目录普通创建有 5 条继承 ACE —— 问题本身真实存在
- Rust 单测 3 passed(需正确设置 TEMP/TMPDIR)
- revert-red(去掉
UnprotectedDacl)→Adopted DACL is still protected变红,还原后 3 passed
处置建议
关闭。 采纳 #547。若要救本 PR,需要:与普通创建的文件逐字比较 DACL、NULL DACL 不 panic、补非 Windows skip 与 gate 白名单、修 whoami 绝对路径、排除 FILE_SHARE_WRITE —— 做完基本就收敛成 #547 了。
|
Closing — #547 owns the DACL adoption and is in |
Closes #420
What was wrong
A file created by
ops_write_filekept only the caller's explicit ACE:native/windows-metadata/src/lib.rsbuilt a protected descriptorD:P(A;;FA;;<sid>), so theparent directory's inheritable ACEs were dropped. The symptom in the issue is immediate: the
freshly created file cannot be deleted in the same session, and other tools/users lose access to it.
The change
native/windows-metadata/src/lib.rs— after the bytes exist, ask the OS to recompute the stagedDACL with inheritance (
SetSecurityInfo(… DACL_SECURITY_INFORMATION | UNPROTECTED_DACL_SECURITY_INFORMATION …)),so the child carries the directory's inheritable ACEs as inherited entries plus one explicit
full-control ACE for the caller.
verify_adoptedstill fails closed before publication: everyobject-inheritable directory entry must be present, the caller's explicit ACE must be there, and
the DACL must not be protected. Both sides are reduced through
MapGenericMask(file mapping)before the exact match, because Windows maps
GA/SDGXGWGR/GXGRto specific rights while itpropagates them — the earlier code compared the stored generic spelling against the propagated
mask and read a correctly inherited
GAas missing.native/crates/ops-files/src/{file_publication.rs,file_single.rs}— plumb the adoption into thecreate path; staging stays private until the bytes exist.
test/file-inherited-dacl.test.mjs(new) + awindows-metadataunit test,docs/CHANGELOG.md,docs/windows-security-regressions.md+docs/Windows安全回归.md(the W1 row, including "existingfiles do not self-heal").
Deliberately not done: overwrite, copy/move and cache objects keep their current descriptors; no
retroactive repair of files created by older versions.
Validation
docs/CHANGELOG.md## [Unreleased]:已加(EN+ZH)pull_request事件触发,届时回填CI required的 run URLEnvironment
cargo build --release --locked)065cd7fnpm ci --ignore-scripts未验证 / 风险披露
DACL 挡住)。这是 issue 想要的方向,但若目录上有针对创建者的 deny,行为会变化。
verify_adopted仍要求调用者显式 ACE 且失败即中止,不发布半成品。cargo test --workspace的ops-results4 项失败是预存的(受限令牌 +%TEMP%),在pristine main 上复现,与本片无关。
CI