Skip to content

Tighten up some logic around file permissions #662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jakepetroules
Copy link
Collaborator

@jakepetroules jakepetroules commented Jul 21, 2025

Fix a regression (from #565) in PbxCp which caused files to inappropriately gain the execute bit. isExecutable is conceptually a higher level operation which (per POSIX) can return true even if the file has no execute permission bits set:

"If execute permission is requested, access shall be granted if execute permission is granted to at least one user by the file permission bits or by an alternate access control mechanism"

No test because it's not clear what the concrete conditions are under which this occurs. That said, copying now checks against the owner permission bit specifically, as that's the actual intent of what the code is meaning to do and matches the original behavior.

Additionally, fix some missing permission information in getFileInfo for the PseudoFS implementation and increase test coverage a bit. Avoids it coming back to bite later.

rdar://154663442

Fix a regression in PbxCp which caused files to inappropriately gain the execute bit. `isExecutable` is conceptually a higher level operation which (per POSIX) can return true even if the file has no execute permission bits set:

"If execute permission is requested, access shall be granted if execute permission is granted to at least one user by the file permission bits or by an alternate access control mechanism"

No test because it's not clear what the concrete conditions are under which this occurs. That said, copying now checks against the owner permission bit specifically, as that's the actual intent of what the code is meaning to do and matches the original behavior.

Additionally, fix some missing permission information in getFileInfo for the PseudoFS implementation and increase test coverage a bit. Avoids it coming back to bite later.

rdar://154663442
@jakepetroules
Copy link
Collaborator Author

@swift-ci test

@jakepetroules
Copy link
Collaborator Author

@swift-ci test linux

@jakepetroules jakepetroules enabled auto-merge (rebase) July 21, 2025 21:01
@jakepetroules
Copy link
Collaborator Author

@swift-ci test

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.

3 participants