fix: use & ~umask instead of ^ umask for permission masking#218
Open
Koan-Bot wants to merge 2 commits intocpanel:masterfrom
Open
fix: use & ~umask instead of ^ umask for permission masking#218Koan-Bot wants to merge 2 commits intocpanel:masterfrom
Koan-Bot wants to merge 2 commits intocpanel:masterfrom
Conversation
The XOR operator was used to apply umask in 4 locations, which produces correct results only when the permission bits being masked are all set (e.g., 0666 ^ 0022 = 0644). For non-default permissions where umask bits are already clear, XOR incorrectly re-enables them: 0644 ^ 0022 = 0666 (wrong — should stay 0644) 0600 ^ 0022 = 0622 (wrong — should stay 0600) Fixed 4 locations: - file() constructor (line 664): 0666 default, user-provided mode - dir() constructor (line 844): 0777 default - __mkdir() (line 2243): user-provided mode - chmod() method (line 1525): removed umask entirely — real chmod(2) ignores umask, only open/mkdir/mkfifo apply it Added tests for non-default permissions that would fail with XOR: - File creation with mode 0644 under umask 0022 - chmod(0755) verifying umask is not applied - mkdir with mode 0700 under umask 0022
3841c39 to
456c3b4
Compare
The permission recovery formula used XOR with umask, which only works when umask was applied via XOR. Now that umask is correctly applied with & ~umask, stat already returns the final mode — just mask with 07777 to strip file type bits. Co-Authored-By: Claude Opus 4.6 <[email protected]>
atoomic
approved these changes
Feb 25, 2026
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
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.
Summary
chmod()method entirely (real chmod ignores umask)Why
The XOR operator (
^ umask) only produces correct results when all umask bits are set in the permission value. For non-default permissions where umask bits are already clear, XOR re-enables them:Default permissions (0666 for files, 0777 for dirs) with umask 0022 happened to work by coincidence, masking this bug.
What changed
file()constructor$perms ^ umask$perms & ~umaskdir()constructor$perms ^ umask$perms & ~umask__mkdir()$perms ^ umask$perms & ~umaskchmod()method$mode ^ umask$mode(umask removed — matches real chmod)Testing
Added 3 new subtests in
t/chmod.texercising non-default permissions that would fail with the old XOR logic:$mock->chmod(0755)verifying umask is not appliedmkdir('/path', 0700)under umask 0022