feat(windows): detect and repair cache children left unusable by the old DACL regime - #1636
Draft
DeusData wants to merge 4 commits into
Draft
feat(windows): detect and repair cache children left unusable by the old DACL regime#1636DeusData wants to merge 4 commits into
DeusData wants to merge 4 commits into
Conversation
win_runtime_directory_secure called set_security_info with PROTECTED_DACL_SECURITY_INFORMATION on every process start, whether or not anything needed repairing. Line 4126 computes `created`, but it only guards the ERROR_ALREADY_EXISTS check - the re-stamp itself ran unconditionally. Two costs, both observed rather than theorised: #1601 counted ELEVEN "Security change" USN records against a single _config.db in one day. Windows propagates a directory's security descriptor to its children, so a rewrite that changes nothing still churns every file underneath. #1620 loses its atomic publish to this. MoveFileEx needs DELETE on the destination, and a concurrent re-protect of the parent is a window in which it can be refused - for a state that was about to be correct anyway. That reporter proved the interaction by running a background icacls loop during indexing and watching the identical index succeed. The repair is what matters, not the ritual. When the owner is already the exact current user AND the DACL already passes the private-directory check, there is nothing to fix and the right action is to leave it alone. When it IS wrong, the repair is byte-for-byte what it was before. This does not fix the underlying ACL damage on already-broken installs - files created under the pre-v0.10.3 flagless regime still carry empty DACLs and need a child repair, which is a separate change. It stops us from making it worse and from creating a failure window on every start. Costs one extra GetSecurityInfo to avoid a SetSecurityInfo, which is the cheap direction. Builds clean; daemon_ipc 48 passed. The changed region is Windows-only, so the real verification is the Windows CI leg. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
The previous commit called win_file_security_secure with five arguments; on this branch it takes four. That is a compile error on Windows. It passed the local build because the entire win_* region is Windows-only and guarded out on macOS, so a clean macOS build says nothing about it. Worth recording as the trap it is: for Windows-only code, a green host build is not evidence, and the Windows CI leg is the first thing that can actually reject it. (The five-argument form belongs to #1623, which threads an ancestor flag through this function. When that lands, this call becomes the five-argument form again on rebase.) Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
…ignature #1623 landed on main and threads an ancestor flag through win_file_security_secure. The conditional check added here predates it and still passed four arguments; both calls now pass ancestor=false, which is correct - the private runtime directory is never an ancestor and must keep full strictness. This is the second arity correction on this branch, in opposite directions, and the reason is worth stating: the win_* region is Windows-only, so a local macOS build compiles none of it and reports success regardless. Nothing on this machine can catch a mismatched call here. The Windows CI leg is the only venue that can. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
force-pushed
the
fix/repair-damaged-cache-acls
branch
from
August 14, 2026 14:24
9644eed to
d8655bf
Compare
…old DACL regime Between v0.9.1-rc and v0.10.2 the runtime directory carried a PROTECTED DACL whose ACE was not inheritable, so Windows gave every file created inside it either an empty DACL or the token default (SYSTEM + TokenOwner + logon SID). Under an elevated token TokenOwner is BUILTIN\Administrators, so the interactive user is left with no durable grant and the file becomes unreadable after the next logon. #1601: takeown and icacls both fail non-elevated, and the daemon can no longer open _config.db. now, so new children are fine. Nothing repaired the children already damaged, which is why upgrading rescued nobody whose cache was written under the old regime. Their only route back was a rename-the-parent dance the reporter worked out themselves. The repair runs after the directory is confirmed good, and is deliberately narrow: immediate children only, no recursion, capped at 4096, regular files only - directories, reparse points and symlinks are skipped rather than followed. A child is touched ONLY when demonstrably damaged: an empty DACL, or an owner that is not the current user. Failures are counted and reported, never fatal, because this runs inside daemon startup. Empty-DACL detection needs its own test and cannot reuse the validator: win_file_acl_secure scans ACEs for untrusted mutation grants, and a DACL with zero ACEs trivially has none - so the damage reads as compliance. It only ever touches cbm's own runtime/cache directory, which we created and own; it does not reach into user directories. Windows-only code. A clean macOS build says nothing about it - the previous commit on this branch shipped a five-argument call to a four-argument function and built green here - so the Windows CI leg is the first venue that can actually reject this. Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
DeusData
force-pushed
the
fix/repair-damaged-cache-acls
branch
from
August 14, 2026 14:25
d8655bf to
4623477
Compare
2 tasks
DeusData
marked this pull request as draft
August 14, 2026 18:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1634 — review that one first; this branch contains it.
The gap this closes
Between v0.9.1-rc and v0.10.2 the runtime directory carried a
PROTECTEDDACL whose ACE was not inheritable. Windows therefore gave every file created inside it either an empty DACL or the token default (SYSTEM +TokenOwner+ logon SID). Under an elevated tokenTokenOwnerisBUILTIN\Administrators, so the interactive user ends up with no durable grant at all and the file is unreadable after the next logon.That is #1601:
takeownandicaclsboth fail non-elevated, and the daemon can no longer open_config.db.#1531 fixed the cause forward-only in v0.10.3 — the directory ACE is inheritable now, so newly created children are fine. But nothing repaired the children already damaged, which is why upgrading rescued nobody whose cache was written under the old regime. Their only route back was the rename-the-parent workaround #1601's reporter worked out for themselves.
This is that repair.
Why empty-DACL detection needs its own test
win_file_acl_securescans ACEs looking for untrusted mutation grants. A DACL with zero ACEs trivially has none — so the damage reads as compliance. Reusing the validator would have silently skipped exactly the files that need fixing. Hencewin_file_dacl_is_empty.Deliberately narrow
daemon.runtime_child_acl_repaired), never fatal — this runs inside daemon startupScope: it only ever touches cbm's own runtime/cache directory, which we created and own. It does not reach into user directories.
Verification caveat, stated plainly
This is Windows-only code and a clean macOS build proves nothing about it — the whole
win_*region is guarded out. The previous commit on this branch shipped a five-argument call to a four-argument function and still built green locally; I caught that by reading, not by building. The Windows CI leg is the first venue that can actually reject this, and the honest end-to-end test is a machine with a cache damaged under the old regime.