fix(daemon): make private directory DACL inheritable

[Kiborgik]

Fixes #1351

Problem

win_security_init builds the owner-only ACL with AddAccessAllowedAce (src/daemon/ipc.c:3664). That API has no AceFlags parameter, so the ACE it produces always has flags set to zero. Applied to a directory together with PROTECTED_DACL_SECURITY_INFORMATION, the result is:

D:PAI(A;;FA;;;<user>)

The protection severs the inherited ACEs and the new ACE propagates nothing, so every child created afterwards is born with an empty DACL and is unreadable even by its owner. That is the RuleCount=0 state on _config.db, logs, and project databases reported in #1351.

win_runtime_directory_secure is also a repair path: line 4051 continues on ERROR_ALREADY_EXISTS and line 4093 re-stamps the existing directory, so a tree repaired by hand with icacls is destroyed again on the next daemon or client start.

The three other owner-only ACL builders in the tree avoid this by using SetEntriesInAclW with an explicit grfInheritance (compat.c:88, compat_fs.c:454, and activation_transaction.c:368, the last correctly using NO_INHERITANCE because it is applied only to a CREATE_NEW file).

Change

Build a second ACL for containers using AddAccessAllowedAceEx with CONTAINER_INHERIT_ACE | OBJECT_INHERIT_ACE, and use it at the three directory sites (ipc.c:4050, 4097, 4184). Files and kernel objects are leaves and keep the existing flagless ACL (3819, 3823, 4236, 4260).

The container ACE carries FILE_ALL_ACCESS rather than GENERIC_ALL. Windows splits an inheritable generic-rights ACE into an effective mapped ACE plus an INHERIT_ONLY one holding the generic bits, and private_win_owner_only_dacl requires exactly one ACE (private_file_lock.c:910). Using generic rights here fails every lock directory with secure CLI coordination could not be created (project-locks). I hit that while developing this patch, which is why the test asserts the ACE count.

Verification

Built from this branch and compared against the stock 0.10.0 release binary on the same cache path (Windows 11 26200, MinGW gcc, non-elevated standard token):

Path	stock 0.10.0	this branch
create, fresh directory	<user>:(F)	<user>:(OI)(CI)(F)
repair, existing directory	strips to (F)	(OI)(CI)(F)
cli list_projects	works	works

Test

daemon_ipc_windows_private_directory_ace_is_inheritable asserts that the secured directory has exactly one ACE, that it carries both inherit flags, that a child created inside it has a non-empty DACL, and that cbm_private_lock_directory_adopt_windows still accepts the directory. The inherit-flag and child-DACL assertions fail on current main.

What I could not verify locally

MinGW ships no libsanitizer, so scripts/test.sh could not run in its ASan and UBSan configuration; my run was unsanitized. Several pre-existing -Werror diagnostics in tests/test_daemon_ipc.c and tests/test_daemon_application.c (stringop-overflow, free-nonheap-object) also had to be downgraded before the runner would build under this gcc. None are in code this patch touches, but CI is the authority on sanitizer and warning cleanliness here.

Separately, scripts/run-tests-parallel.sh stamps the build directory using a bare username, which misresolves on a machine whose hostname equals the username and locks the harness out of its own log directory. That is unrelated to this change and I will open it separately.

[github-actions]

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

• It will not be closed for inactivity. No stale bot touches pull requests here.
• It may still sit a while before a human reads it. That is on us, not on you.
• Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

• Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
• Get CI green, or say which failures you believe are pre-existing.
• Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
• Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

[Kiborgik]

On the test-windows-guards red (section_cold_storm, client 3): I could not reproduce it, and the ordering in the captured output argues against this branch.

The failing client's last line was version_cohort.claimed_unheld, which cbm_version_cohort_acquire emits at src/main.c:2438. That runs only after the coordination_failure gate at :2418-2429 has passed, so the client had already completed endpoint creation, cbm_project_lock_manager_new, the cohort manager, executable resolution and build identity. Those are the stages this branch touches. It changes the DACL applied to the runtime directory and nothing after it.

Local runs, MinGW gcc on Windows 11 26200:

Build	TEST_SEAMS	Runs	section_cold_storm
this branch	off	2	green
this branch	on	1	green
main (control, same toolchain)	on	1	green

All four runs were green across all seven sections. The control and the patched binary differ only in this change.

One thing may be worth fixing in the harness regardless. section_cold_storm prints out_text(result)[:300], and the two startup lines (version_cohort.claimed_unheld and the mem.allocator.preloading_completed warning) already exceed that, so the actual failure reason is truncated away. Every failure path after the cohort claim does print a reason; main_local_transition_acquire returning 0 gives "CLI startup coordination remained busy", for example. Raising that slice, or printing the tail instead of the head, would make the next occurrence diagnosable. section_churn_stability already prints stdout[-300:] and stderr[-400:], which is the shape that would have shown it.

My guess is startup-transition contention among six racing cold clients on a shared runner, which a 28-core machine resolves and a 2-core one may not. That is a guess. I can rebuild under mingw-w64-clang to match the runner more closely if another data point would help.

[DeusData]

Merged — thank you @Kiborgik. This is precise Windows work and the diagnosis is the valuable part: a flagless ACE combined with PROTECTED_DACL_SECURITY_INFORMATION severs inherited ACEs while propagating nothing, so every child object is created with an empty DACL. That is exactly the shape of #1351 (cache files left unreadable), and it is the kind of bug that looks like a permissions mystery from the outside.

The FILE_ALL_ACCESS vs GENERIC_ALL note deserves highlighting for anyone who touches this later: Windows splits an inheritable generic-rights ACE into a mapped ACE plus an INHERIT_ONLY one carrying the generic bits, which would produce two ACEs — and our owner-only DACL validators require exactly one. Getting that wrong would have traded an empty-DACL bug for a validation failure.

Worth noting this does not relax the security posture: children now inherit the same owner-only ACE the parent already enforced. Shipping in v0.10.3.