From 1fec7cbccbceb2d6129f73d78bb4ac0c9db7a4ed Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 14 Aug 2026 15:35:53 +0200 Subject: [PATCH 1/3] fix(windows): re-stamp the runtime DACL only when it is actually wrong 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 --- src/daemon/ipc.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index 5fdb7efe1..a755bbb40 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -4204,8 +4204,31 @@ static bool win_runtime_directory_secure(const wchar_t *runtime_dir) { * refused, and the final validation below still demands the exact user. */ bool owner_ok = owner_exact || (valid_handle && can_write_owner && win_file_owner_secure(&security, directory, false)); + /* Re-stamp only when the directory is not ALREADY correct. + * + * This used to fire on every process start, whether or not anything was + * wrong. Two costs, both observed in the field: + * + * - It rewrites the security descriptor of a directory that already has + * the right one, and Windows propagates that to children. #1601 counted + * ELEVEN "Security change" USN records against a single _config.db in + * one day, none of which changed anything. + * - Every rewrite is a window. #1620 loses an atomic publish to exactly + * this: MoveFileEx needs DELETE on the destination, and a concurrent + * re-protect of the parent is a chance to be refused for a state that is + * about to be correct again anyway. + * + * The repair is what matters, not the ritual. If the owner is already the + * exact current user and the DACL already passes the private-directory + * check, there is nothing to fix and the correct action is to leave it + * alone. When it IS wrong we still repair exactly as before. */ DWORD secure_result = ERROR_ACCESS_DENIED; - if (valid_handle && owner_ok) { + bool already_correct = + valid_handle && owner_exact && + win_file_security_secure(&security, directory, true, win_private_mutation_rights(), false); + if (already_correct) { + secure_result = ERROR_SUCCESS; + } else if (valid_handle && owner_ok) { secure_result = security.set_security_info( directory, SE_FILE_OBJECT, (owner_exact ? 0U : (DWORD)OWNER_SECURITY_INFORMATION) | DACL_SECURITY_INFORMATION | From 60b390bea44b2ce9d70d94ac9e8673baef4717e0 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 14 Aug 2026 15:59:26 +0200 Subject: [PATCH 2/3] fix(windows): correct the argument count in the conditional re-stamp 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 --- src/daemon/ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index a755bbb40..f34e637dd 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -4225,7 +4225,7 @@ static bool win_runtime_directory_secure(const wchar_t *runtime_dir) { DWORD secure_result = ERROR_ACCESS_DENIED; bool already_correct = valid_handle && owner_exact && - win_file_security_secure(&security, directory, true, win_private_mutation_rights(), false); + win_file_security_secure(&security, directory, true, win_private_mutation_rights()); if (already_correct) { secure_result = ERROR_SUCCESS; } else if (valid_handle && owner_ok) { From dfd9f7e122f758d7c272b03715680390b3d50554 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 14 Aug 2026 16:24:00 +0200 Subject: [PATCH 3/3] fix(windows): align the conditional re-stamp with the ancestor-flag signature #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 --- src/daemon/ipc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/daemon/ipc.c b/src/daemon/ipc.c index f34e637dd..a755bbb40 100644 --- a/src/daemon/ipc.c +++ b/src/daemon/ipc.c @@ -4225,7 +4225,7 @@ static bool win_runtime_directory_secure(const wchar_t *runtime_dir) { DWORD secure_result = ERROR_ACCESS_DENIED; bool already_correct = valid_handle && owner_exact && - win_file_security_secure(&security, directory, true, win_private_mutation_rights()); + win_file_security_secure(&security, directory, true, win_private_mutation_rights(), false); if (already_correct) { secure_result = ERROR_SUCCESS; } else if (valid_handle && owner_ok) {