Skip to content

Conversation

@ahmedbilal9
Copy link
Contributor

@ahmedbilal9 ahmedbilal9 commented Nov 24, 2025

Changes or fixes:

Replace hardcoded 0644 file permissions with 0666 to respect system umask.

This change allows the system umask to control file permissions instead of hardcoding them to 0644. Files will now be created with permissions determined by (0666 & ~umask), which is the standard Unix behavior.

Files modified:

  • io/io/src/TFile.cxx
  • io/io/src/TMapFile.cxx
  • io/io/src/TMemFile.cxx
  • io/dcache/src/TDCacheFile.cxx
  • net/net/src/TFTP.cxx
  • net/net/src/TApplicationServer.cxx

Checklist:

  • tested changes locally
  • updated the docs (if necessary)

This PR fixes #20470

This change allows the system umask to control file permissions
instead of hardcoding them to 0644. Files will now be created with
permissions determined by (0666 & ~umask), which is the standard
Unix behavior.

Files modified:
- io/io/src/TFile.cxx
- io/io/src/TMapFile.cxx
- io/io/src/TMemFile.cxx
- io/dcache/src/TDCacheFile.cxx
- net/net/src/TFTP.cxx
- net/net/src/TApplicationServer.cxx

Fixes root-project#17095
@ahmedbilal9 ahmedbilal9 requested a review from pcanal as a code owner November 24, 2025 18:55
@ferdymercury
Copy link
Collaborator

fixes #17095

You mean #20470 ?

@ahmedbilal9
Copy link
Contributor Author

fixes #17095

You mean #20470 ?

pardon me, yes!

@github-actions
Copy link

Test Results

    22 files      22 suites   3d 22h 5m 4s ⏱️
 3 781 tests  3 781 ✅ 0 💤 0 ❌
81 148 runs  81 148 ✅ 0 💤 0 ❌

Results for commit fed7ac7.

@dpiparo dpiparo self-assigned this Nov 25, 2025
@dpiparo dpiparo requested review from hahnjo and silverweed November 25, 2025 05:37
@dpiparo
Copy link
Member

dpiparo commented Nov 25, 2025

Added two Linux and experts as reviewers.

@dpiparo dpiparo assigned pcanal and unassigned pcanal Nov 25, 2025
@ahmedbilal9
Copy link
Contributor Author

Hi @dpiparo, @hahnjo, @silverweed - just wanted to gently ping on this PR.
All tests are passing and I'm happy to address any feedback.
Thanks for your time reviewing!

@silverweed
Copy link
Contributor

silverweed commented Dec 1, 2025

Judging from the forum posts referenced by the linked issue, this is more of a policy decision than a technical one as far as I understand, and I think @pcanal is the person who should have the final word on it.

Me personally, I'd apply this change, but my decision is likely less informed than Philippe's.

Edit: actually I think I misinterpreted the whole thing. As far as I understand we don't simply want to change the file mask to another hardcoded value, but actually not hardcode it anymore and rather respect the ACL (which I'm not sure how is done - my knowledge of ACL is very limited). So I doubt this PR does what the issue wants...in fact it makes things worse by making the file writable by anyone by default which is really not a good default.

Edit2: I guess we can probably ignore ACL-related stuff and just use the umask correctly, but see my later post about this.

@ahmedbilal9
Copy link
Contributor Author

Judging from the forum posts referenced by the linked issue, this is more of a policy decision than a technical one as far as I understand, and I think @pcanal is the person who should have the final word on it.

Me personally, I'd apply this change, but my decision is likely less informed than Philippe's.

Thanks for reviewing! I understand this is a policy decision.

For context, the current hardcoded 0644 creates files that ignore the user's umask settings, which goes against standard Unix conventions. Most file-creation functions (fopen, open, etc.) use 0666 & ~umask to respect the user's security preferences.

This particularly affects users who set restrictive umasks (like 0077) for security/privacy in shared environments - their ROOT files are still created as 0644 (world-readable) instead of 0600 (user-only).

Happy to wait for @pcanal's input on whether ROOT should respect system umask or maintain the current behavior.

@silverweed
Copy link
Contributor

silverweed commented Dec 1, 2025

the current hardcoded 0644 creates files that ignore the user's umask settings, which goes against standard Unix conventions.

I don't understand how this PR fixes this though. Aren't we now simply hardcoding it to a different value, thus ignoring the umask anyway?

In your description you say "by setting it to 066 & ~umask", but that's not what the code is doing...
In my understanding, that would actually work as intended to fix the issue, but you're not using the umask at all at the moment.

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.

ROOT still uses hard-coded open mode flags, overriding umask and ACL settings

5 participants