Skip to content

fix(windows): distinguish read-only ACLs from executable mutation rights - #273

Open
seradotcom wants to merge 2 commits into
mainfrom
codex/windows-acl-read-execute
Open

seradotcom wants to merge 2 commits into
mainfrom
codex/windows-acl-read-execute

Conversation

@seradotcom

@seradotcom seradotcom commented Oct 17, 2026 •

Copy link
Copy Markdown
Owner

Change and reason

A native Windows executable with an explicit or inherited read/execute ACL for another principal was rejected as writable. The admission check intersected each ACE with FILE_GENERIC_WRITE, whose expanded mask also contains READ_CONTROL and SYNCHRONIZE.

Use concrete mutation rights plus GENERIC_WRITE/GENERIC_ALL instead. Apply the same correction to private-file read checks while explicitly retaining FILE_READ_DATA, FILE_READ_EA and FILE_READ_ATTRIBUTES, so private data and attribute reads remain denied.

Verification actually executed

  • Regression-first commit: 385d4ed. Windows run 39004484744 failed on x64 and ARM64 at the new ACL regression step. Both architecture logs record 4 passing rejection tests and 4 failing acceptance tests with the exact DACL rejection, including explicit/inherited executable R and private-file RC grants.
  • Fix commit: cf321a5. Windows run 39004850922 passed both complete native noninteractive jobs (x64 and ARM64) and both sealed-tool compatibility jobs. On each native architecture, all 8 new ACL tests and 3 existing private-data tests passed with no ignored/filtered cases, covering 66 explicit/inherited ACL combinations in the new suite. Runtime check, Clippy, broader runtime/Host tests, documentation and fixture compilation also passed.
  • Runtime tools portability run 39004850882 passed the Windows/Linux Host jobs and Windows/Linux/macOS SDK jobs. Quality gates passed; unrelated application-engine/fuzz lanes were correctly skipped by path scope, not counted as native tests.
  • Local cargo fmt --all -- --check, git diff --check and 4 Windows Python contract tests passed. Heavy native compilation/tests run on disposable CI in accordance with repository policy.
  • A separate agent reviewed the final sources and found no actionable issues, conditional on green Windows CI. This is not an external security audit or maintainer approval.

Permissions / protocols / compatibility affected

Windows file admission now permits harmless shared standard rights while retaining all existing concrete write/delete/owner/DACL restrictions and private data/EA/attribute read restrictions. Broker grants, approval policy and confirm_mutations are unchanged. Installed binaries and running application bridges were not modified or restarted. No new dependency or protocol change. Reverting the mask change restores the previous false-positive behavior.

Regression fixture

Eight tests exercise real temporary-file ACLs via the public executable/private-file verifiers, using an untrusted Everyone principal with explicit grants or grants inherited from a protected temporary parent. They cover executable R/RX/RA/REA/RC/S acceptance, individual and aggregate mutation denial, private-file RC/S acceptance, and private data/attribute/EA read and mutation denial. The Windows workflow runs these fixtures before the broader runtime checks on x64 and ARM64.

@seradotcom
seradotcom marked this pull request as ready for review October 17, 2026 05:21
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