Skip to content

Fix ACL inheritance: newly-created files reopenable by creator#10

Merged
hooyao merged 2 commits intomainfrom
fix/acl-inheritance
May 3, 2026
Merged

Fix ACL inheritance: newly-created files reopenable by creator#10
hooyao merged 2 commits intomainfrom
fix/acl-inheritance

Conversation

@hooyao
Copy link
Copy Markdown
Owner

@hooyao hooyao commented May 3, 2026

Summary

  • Root SDDL gets OICI ACE flags so newly-created files and directories inherit a usable DACL
  • Two new test classes lock in the behaviour
  • One-line SDDL change; everything else is tests + docs

Why

Procmon trace from H:\ during the unrelated --remote-debugging-pipe chrome crash investigation revealed: chrome creates Default\Shared Dictionary\cache (SUCCESS) and immediately reopens the same dir → ACCESS_DENIED. Same pattern for Default\Network\<guid>.tmp and many disk_cache temp files.

Root cause: O:BAG:BAD:P(A;;FA;;;SY)(A;;FA;;;BA)(A;;FA;;;WD) had empty ACE flags. WinFsp's kernel-side FspCreateSecurityDescriptor walks the parent's DACL when computing the SD for a newly-created child and copies only ACEs whose flags say "yes, inherit me". With empty flags, no ACE inherits → child gets an empty DACL → kernel interprets as "deny everyone all access".

End-user symptom is Chromium's "Profile error occurred — Something went wrong when opening your profile" dialog and top_sites_backend / login_database / disk_cache failing to initialise.

What changed

  • src/RamDrive.Cli/WinFspRamAdapter.cs line 47: SDDL now (A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;WD) with explanatory XML doc
  • tests/RamDrive.IntegrationTests/RamDriveFixture.cs: same constant updated, kept in sync
  • tests/RamDrive.IntegrationTests/RootSddlTests.cs (new): pins the SDDL string and asserts every ACE carries OI|CI — guards against future typos
  • tests/RamDrive.IntegrationTests/AclInheritanceTests.cs (new): mounted-FS regression — create file/dir, reopen with Read and with raw Win32 DELETE access, assert effective ACL contains inherited FullControl for Everyone
  • CLAUDE.md WinFsp Notes section updated with new SDDL + explanation
  • docs/leveldb-cache-coherency-postmortem.md §9.1 cross-references this fix
  • Diagnostic scripts (debug_batch.*, bisect_chrome.js, repro_chrome_min.js) added to .gitignore — kept locally for the upcoming cache-mode crash investigation

OpenSpec

openspec/changes/fix-acl-inheritance/ — capability default-security-descriptor. Single requirement with 5 scenarios.

Test plan

  • dotnet test tests/RamDrive.IntegrationTests — 33/33 pass (28 existing + 5 new), 56s
  • PowerShell direct verification: Get-Acl on a freshly-created file returns Inherit=ContainerInherit, ObjectInherit FullControl for SYSTEM/Administrators/Everyone
  • OpenSpec strict validation: Change 'fix-acl-inheritance' is valid

Known follow-up out of scope

The chrome --remote-debugging-pipe STATUS_BREAKPOINT bug surfaced during the same investigation is independent of this fix (different root cause, different procmon signature). Tracked separately; will be a future change.

🤖 Generated with Claude Code

hooyao and others added 2 commits May 3, 2026 16:58
Root SDDL `(A;;FA;;;...)` lacked OBJECT_INHERIT_ACE | CONTAINER_INHERIT_ACE
flags, so WinFsp's kernel-side FspCreateSecurityDescriptor dropped every
ACE when computing the SD for newly-created children. Children ended up
with an empty DACL — interpreted as "deny everyone all access".

Symptom captured via procmon while debugging the unrelated `--remote-debugging-pipe`
chrome crash: chrome creates `Default\Shared Dictionary\cache` (SUCCESS) and
immediately reopens the same dir (ACCESS_DENIED). Same pattern on disk_cache
network temp files (`Default\Network\<guid>.tmp`). End-user symptom is
Chromium's "Profile error occurred" dialog and SQLite-backed components
(top_sites, login_database) failing to initialise.

Fix: change SDDL to `(A;OICI;FA;;;SY)(A;OICI;FA;;;BA)(A;OICI;FA;;;WD)` in
both production adapter and integration test fixture so they stay in sync.

Tests:
- RootSddlTests (new): pin the canonical SDDL string and assert every ACE
  carries OI|CI; guards against future SDDL string typos that would silently
  re-introduce the bug.
- AclInheritanceTests (new): mounted-FS regression test — create file/dir,
  reopen with Read and with raw Win32 DELETE access, assert effective ACL
  contains an inherited FullControl Allow ACE for Everyone.
- All 33 integration tests pass (28 existing + 5 new).

Documented in CLAUDE.md (WinFsp Notes) and cross-referenced from the
existing leveldb postmortem §9.1.

Diagnostic scripts (debug_batch.cmd/js, bisect_chrome.js, repro_chrome_min.js)
gitignored — kept locally for the upcoming cache-mode crash investigation.

OpenSpec change: openspec/changes/fix-acl-inheritance/ (capability
default-security-descriptor). Independent of the unrelated cache-mode
STATUS_BREAKPOINT bug, which remains open.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI on PR #10 timed out (15min) on TortureTests.CreateDeleteChurn (locally
~525ms, 5/5 stable). RootSddlTests had no [Collection], so xUnit ran it in
parallel with the main RamDrive collection on the windows-latest runner —
adding contention on a box that's already short on cores. Adding the
collection attribute serializes it; test takes 30ms total so cost is nil.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@hooyao hooyao merged commit e5bc2e1 into main May 3, 2026
2 checks passed
@hooyao hooyao deleted the fix/acl-inheritance branch May 3, 2026 09:31
hooyao added a commit that referenced this pull request May 3, 2026
… spec (#11)

- Move openspec/changes/fix-acl-inheritance/ -> archive/2026-05-03-...
- Sync delta spec into openspec/specs/default-security-descriptor/spec.md

The implementation already shipped in PR #10 (e5bc2e1). This commit only
touches openspec/ bookkeeping.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
hooyao added a commit that referenced this pull request May 3, 2026
…POINT) (#12)

Self-contained context dump so a fresh Claude Code session post-compaction
can pick up the diagnosis without re-deriving anything. Captures:

- Status of bugs #1 (shipped #9), #2 (shipped #10), and #3 (open, this doc)
- Symptoms (STATUS_BREAKPOINT 0x80000003 + early death; degraded "Profile
  error" dialog variant)
- Repro recipe (5-flag minimum, deterministic to ~80% on H:\)
- 8 already-falsified hypotheses (don't redo)
- Procmon evidence captured in F:\procmon_chrome2.csv (gitignored)
- Four ranked working hypotheses with concrete next-test-steps
- Cheat sheet of mount/repro/bisect commands
- Pointers to all relevant code, specs, archived changes, and external refs
  (winfsp source paths)

This file is meant to be read first by any session continuing this work.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant