[io] Replace hardcoded 0644 permissions with 0666 to respect umask

[ahmedbilal9]

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:

• [x] tested changes locally
• [ ] updated the docs (if necessary)

This PR fixes #20470

[ferdymercury]

fixes #17095

You mean https://github.com/root-project/root/issues/20470 ?

[ahmedbilal9]

fixes #17095

You mean #20470 ?

pardon me, yes!

[github-actions[bot]]

Test Results

    21 files      21 suites   3d 17h 39m 57s ⏱️  3 787 tests  3 787 ✅ 0 💤 0 ❌ 77 608 runs  77 608 ✅ 0 💤 0 ❌

Results for commit fdb07f25.

:recycle: This comment has been updated with latest results.

[dpiparo]

Added two Linux and experts as reviewers.

[ahmedbilal9]

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]

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]

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]

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.

[hahnjo]

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.

Application of umask is automatic in POSIX. From man 2 open:

The effective mode is modified by the process's umask in the usual way: in the absence of a default ACL, the mode of the created file is (mode & ~umask).

[silverweed]

Application of umask is automatic in POSIX. From man 2 open:

The effective mode is modified by the process's umask in the usual way: in the absence of a default ACL, the mode of the created file is (mode & ~umask).

TIL, thanks for the clarification. Then I suppose this PR is good to merge, if @pcanal approves it.

[pcanal]

I am not sure why it was that way but it has been since at least the transition to svn (circa 2000). It is also a noticeable change for those that inadvertently or not relied on it. i.e. their files will now possibly be more public that before but indeed based on their umask so it is consistent. In order to merge this change we must clearly indicate in the release note the net effect. Thanks.

[ferdymercury]

Hi, thanks, but I think you need to do a git rebase instead of the last merge commit solving conflicts

[ferdymercury]

Potential (but not really necessary) improvements could be:

• to create a Test in roottest/root/io subfolder actually checking the reproducer in https://github.com/root-project/root/issues/20470#issue-3649268094 is solved
• to disclose to the reviewers the use of AI, if any, to improve their own review.

[ahmedbilal9]

Potential (but not really necessary) improvements could be:

* to create a Test in roottest/root/io subfolder actually checking the reproducer in [ROOT still uses hard-coded open mode flags, overriding umask and ACL settings #20470 (comment)](https://github.com/root-project/root/issues/20470#issue-3649268094) is solved

* to disclose to the reviewers the use of AI, if any, to improve their own review.

Thank you for the feedback!

Regarding the suggestions:

1. Test in roottest: I'm working on adding a test case in roottest/root/io/umask that verifies the umask behavior described in the original issue reproducer. I'll submit that as a separate PR to the roottest repository once complete.

2. AI disclosure: I used AI assistance (GitHub Copilot) to help with:

   • Understanding the rebase workflow and resolving merge conflicts
   • Formatting commit messages and release notes
   • Code review suggestions and best practices

   The actual code changes and logic for the umask fix were implemented by me based on the issue requirements (#20470) and reviewer feedback.

I appreciate the transparency guidelines and will keep this in mind for future contributions!