Skip to content

1.2.0 Cache permission issues #6400

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

Closed
santiagobiali opened this issue Mar 4, 2022 · 10 comments
Closed

1.2.0 Cache permission issues #6400

santiagobiali opened this issue Mar 4, 2022 · 10 comments
Assignees
Labels
Milestone

Comments

@santiagobiali
Copy link

Have you checked borgbackup docs, FAQ, and open Github issues?

Yes

Is this a BUG / ISSUE report or a QUESTION?

BUG/ISSUE

System information. For client/server mode post info for both machines.

Debian 11 as a Gen2 Hyper-V VM.

Your borg version (borg -V).

1.2.0 (does not work as intended) & 1.117 (works as intended)

Operating system (distribution) and version.

Debian 11 as a Gen2 Hyper-V VM. - 5.10.0-11-amd64

Hardware / network configuration, and filesystems used.

Filesystem: ext4

Describe the problem you're observing.

Borg 1.2.0 is setting permission of cache files to 700, no matter what --umask or "umask" in script is set to.
This causes problems when running borg on different computers using a single shared cache (network share), as only the owner will have permission, and not the group.

Can you reproduce the problem? If so, describe how. If not, describe troubleshooting steps you took before opening the issue.

Yes, script:

#!/bin/bash

export LC_ALL=C
export BORG_CONFIG_DIR="Borg_config_test"
export BORG_CACHE_DIR="Borg_cache_test"
export BORG_SECURITY_DIR="${BORG_CONFIG_DIR}/security"
export BORG_KEYS_DIR="${BORG_CONFIG_DIR}/keys"
export BORG_PASSPHRASE="SOMEPASSWORD"

repo=repoTest

umask 0007

borg init --encryption=keyfile-blake2 $repo &> /dev/null

mkdir folderToBackup &> /dev/null
echo textToBackup > folderToBackup/file.txt

chown root $repo -R
chgrp root $repo -R
chmod 770 $repo -R
chmod 770 $BORG_CONFIG_DIR -R
chmod 770 $BORG_CACHE_DIR -R

echo "Permissions before backup:"
ls -lha $BORG_CACHE_DIR/*/config

borg create \
        --umask 0007 \
        --show-version \
        "${repo}::archive1_$(date +%Y%m%d-%H%M%S)" "folderToBackup" &> /dev/null

echo "Permissions after borg create 1.2.0:"
ls -lha $BORG_CACHE_DIR/*/config


echo -----------------------------------------------------------------------------

#Resetting permissions
chown root $repo -R
chgrp root $repo -R
chmod 770 $repo -R
chmod 770 $BORG_CONFIG_DIR -R
chmod 770 $BORG_CACHE_DIR -R

echo "Permissions before backup:"
ls -lha $BORG_CACHE_DIR/*/config

borg1.1.17 create \
        --umask 0007 \
        --show-version \
        "${repo}::archive1_$(date +%Y%m%d-%H%M%S)" "folderToBackup" &> /dev/null

echo "Permissions after borg create 1.1.17:"
ls -lha $BORG_CACHE_DIR/*/config

echo -----------------------------------------------------------------------------
echo "Permissions of file created using the 'touch' at the beggining of script:"
ls -lha folderToBackup/file.txt

Output:

Permissions before backup:
-rwxrwx--- 1 root root 590 Mar  4 14:34 Borg_cache_test/7c07863b42def5d41bf0e64e057e1c774414d071ed743738cc925c806eec30dc/config
Permissions after borg create 1.2.0:
-rw------- 1 root root 590 Mar  4 14:36 Borg_cache_test/7c07863b42def5d41bf0e64e057e1c774414d071ed743738cc925c806eec30dc/config
-----------------------------------------------------------------------------
Permissions before backup:
-rwxrwx--- 1 root root 590 Mar  4 14:36 Borg_cache_test/7c07863b42def5d41bf0e64e057e1c774414d071ed743738cc925c806eec30dc/config
Permissions after borg create 1.1.17:
-rw-rw---- 1 root root 590 Mar  4 14:36 Borg_cache_test/7c07863b42def5d41bf0e64e057e1c774414d071ed743738cc925c806eec30dc/config
-----------------------------------------------------------------------------
Permissions of file created using the 'touch' at the beggining of script:
-rw-rw---- 1 root root 13 Mar  4 14:36 folderToBackup/file.txt
@ThomasWaldmann
Copy link
Member

Are you using the borg "pyinstaller-made fat binary" from the github releases page?

@santiagobiali
Copy link
Author

Are you using the borg "pyinstaller-made fat binary" from the github releases page?
Yes, tried using "borg-linux64" downloaded from github AND pip install "borgbackup==1.2.0".
The result is the same.

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Mar 4, 2022

BTW, it is a bit unusual what you are doing.

The cache is usually a local cache used by one machine and one user.
Its storage location is on same machine, same user, because borg assumes that location to be fully trusted.
The cache contents are not encrypted and some stuff (e.g. in the security dir) is relevant for security.

So, if you move that to a network share on a server and use it by multiple machines, you must trust them all.

So, there are potential problems lurking in case something goes wrong and you trusted too much.
And the only advantage I see is saving some storage space for chunks.archives.d and avoiding a chunks cache sync IF you use the same repo for all clients.

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Mar 4, 2022

Your reproducer script is doing way too much and requires root, how about this:

# reproduce issue 6400

export BORG_BASE_DIR=borg_base_dir
export BORG_REPO=repo

umask
umask 0007
umask
mkdir $BORG_BASE_DIR

borg init --umask 0007 -e none
borg create --umask 0007 ::{now} src
ls -ld $BORG_BASE_DIR
ls -laR $BORG_BASE_DIR
ls -ld $BORG_REPO
ls -laR $BORG_REPO

borg delete --umask 0007 --force
rm -rf $BORG_BASE_DIR

Updated: ls needs -a to show .stuff

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Mar 4, 2022

One potential issue in your usage is inconsistent usage of --umask.

If you use that borg option, you always need to use it in a consistent way.
If you do not give that option, it will use the default of --umask 0077.

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Mar 4, 2022

Output of the above script:

0022
0007
drwxrwx---  4 tw  staff  128 Mar  4 19:47 borg_base_dir
total 0
drwxrwx---   4 tw  staff   128 Mar  4 19:47 .
drwxr-xr-x  54 tw  staff  1728 Mar  4 19:47 ..
drwxrwx---   3 tw  staff    96 Mar  4 19:47 .cache
drwxrwx---   3 tw  staff    96 Mar  4 19:47 .config

borg_base_dir/.cache:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 19:47 .
drwxrwx---  4 tw  staff  128 Mar  4 19:47 ..
drwx------  4 tw  staff  128 Mar  4 19:47 borg          <---- BUG

borg_base_dir/.cache/borg:
total 8
drwx------  4 tw  staff  128 Mar  4 19:47 .
drwxrwx---  3 tw  staff   96 Mar  4 19:47 ..
drwxrwx---  8 tw  staff  256 Mar  4 19:47 56b65780e9f22493f528f0498e3e6b000f68e7819cf7373dfd041cdced723101
-rw-------  1 tw  staff  198 Mar  4 19:47 CACHEDIR.TAG          <---- BUG

borg_base_dir/.cache/borg/56b65780e9f22493f528f0498e3e6b000f68e7819cf7373dfd041cdced723101:
total 176
drwxrwx---  8 tw  staff    256 Mar  4 19:47 .
drwx------  4 tw  staff    128 Mar  4 19:47 ..
-rw-rw----  1 tw  staff     68 Mar  4 19:47 README
-rw-rw----  1 tw  staff  45382 Mar  4 19:47 chunks
drwxrwx---  2 tw  staff     64 Mar  4 19:47 chunks.archive.d
-rw-------  1 tw  staff    581 Mar  4 19:47 config          <---- BUG
-rw-------  1 tw  staff  27065 Mar  4 19:47 files          <---- BUG
-rw-------  1 tw  staff      2 Mar  4 19:47 pre12-meta          <---- BUG

borg_base_dir/.cache/borg/56b65780e9f22493f528f0498e3e6b000f68e7819cf7373dfd041cdced723101/chunks.archive.d:
total 0
drwxrwx---  2 tw  staff   64 Mar  4 19:47 .
drwxrwx---  8 tw  staff  256 Mar  4 19:47 ..

borg_base_dir/.config:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 19:47 .
drwxrwx---  4 tw  staff  128 Mar  4 19:47 ..
drwx------  3 tw  staff   96 Mar  4 19:47 borg          <---- BUG

borg_base_dir/.config/borg:
total 0
drwx------  3 tw  staff  96 Mar  4 19:47 .
drwxrwx---  3 tw  staff  96 Mar  4 19:47 ..
drwxrwx---  3 tw  staff  96 Mar  4 19:47 security

borg_base_dir/.config/borg/security:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 19:47 .
drwx------  3 tw  staff   96 Mar  4 19:47 ..
drwx------  5 tw  staff  160 Mar  4 19:47 56b65780e9f22493f528f0498e3e6b000f68e7819cf7373dfd041cdced723101          <---- BUG

borg_base_dir/.config/borg/security/56b65780e9f22493f528f0498e3e6b000f68e7819cf7373dfd041cdced723101:
total 24
drwx------  5 tw  staff  160 Mar  4 19:47 .
drwxrwx---  3 tw  staff   96 Mar  4 19:47 ..
-rw-------  1 tw  staff    1 Mar  4 19:47 key-type          <---- BUG
-rw-------  1 tw  staff   21 Mar  4 19:47 location          <---- BUG
-rw-------  1 tw  staff   26 Mar  4 19:47 manifest-timestamp          <---- BUG
drwxrwx---  8 tw  staff  256 Mar  4 19:47 repo
total 120
drwxrwx---   8 tw  staff    256 Mar  4 19:47 .
drwxr-xr-x  54 tw  staff   1728 Mar  4 19:47 ..
-rw-rw----   1 tw  staff     73 Mar  4 19:47 README
-rw-------   1 tw  staff    209 Mar  4 19:47 config          <---- BUG
drwxrwx---   3 tw  staff     96 Mar  4 19:47 data
-rw-rw----   1 tw  staff     90 Mar  4 19:47 hints.5
-rw-rw----   1 tw  staff  41258 Mar  4 19:47 index.5
-rw-rw----   1 tw  staff    190 Mar  4 19:47 integrity.5

repo/data:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 19:47 .
drwxrwx---  8 tw  staff  256 Mar  4 19:47 ..
drwxrwx---  8 tw  staff  256 Mar  4 19:47 0

repo/data/0:
total 9800
drwxrwx---  8 tw  staff      256 Mar  4 19:47 .
drwxrwx---  3 tw  staff       96 Mar  4 19:47 ..
-rw-rw----  1 tw  staff      476 Mar  4 19:47 0
-rw-rw----  1 tw  staff       17 Mar  4 19:47 1
-rw-rw----  1 tw  staff  4993652 Mar  4 19:47 2
-rw-rw----  1 tw  staff       49 Mar  4 19:47 3
-rw-rw----  1 tw  staff      539 Mar  4 19:47 4
-rw-rw----  1 tw  staff       17 Mar  4 19:47 5

@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Mar 4, 2022

Hmm, guess the unexpected modes of config (and some other files) is due to security reasons:
Python's tempfile code likely creates the temp file with access for the user only (intentionally ignoring the umask).

So guess we need a chmod after renaming the temp file to its final destination.

Not sure why some of the directory modes are also wrong.

@borgbackup borgbackup deleted a comment from santiagobiali Mar 4, 2022
@ThomasWaldmann
Copy link
Member

ThomasWaldmann commented Mar 4, 2022

The file mode issue was introduced by the race-condition fix that introduced tempfile.mkstemp usage, so it affects 1.1-maint, 1.2-maint and master. (see #6325 #6306)

The directory mode issue is likely present since longer, but most users did not notice, because they didn't need group or other permissions.

@ThomasWaldmann ThomasWaldmann self-assigned this Mar 4, 2022
@ThomasWaldmann ThomasWaldmann modified the milestones: 1.1.18, 1.2.1 Mar 4, 2022
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Mar 4, 2022
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Mar 4, 2022
…gbackup#6400

we tried to be very private / secure here, but that created the issue
that a less secure umask (like e.g. 0o007) just did not work.

to make the umask work, we must start from 0o777 mode and let the
umask do its work, like e.g. 0o777 & ~0o007 --> 0o770.

with borg's default umask of 0o077, it usually ends up being 0o700,
so only permissions for the user (not group, not others).
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Mar 4, 2022
…up#6400

we tried to be very private / secure here, but that created the issue
that a less secure umask (like e.g. 0o007) just did not work.

to make the umask work, we must start from 0o777 mode and let the
umask do its work, like e.g. 0o777 & ~0o007 --> 0o770.

with borg's default umask of 0o077, it usually ends up being 0o700,
so only permissions for the user (not group, not others).
@ThomasWaldmann
Copy link
Member

After applying PR #6403 it looks like this:

(borg-env) tw@mba2020 borg % ./reproduce-6400.sh
0022
0007
drwxrwx---  4 tw  staff  128 Mar  4 21:35 borg_base_dir
total 0
drwxrwx---   4 tw  staff   128 Mar  4 21:35 .
drwxr-xr-x  54 tw  staff  1728 Mar  4 21:35 ..
drwxrwx---   3 tw  staff    96 Mar  4 21:35 .cache
drwxrwx---   3 tw  staff    96 Mar  4 21:35 .config

borg_base_dir/.cache:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 21:35 .
drwxrwx---  4 tw  staff  128 Mar  4 21:35 ..
drwxrwx---  4 tw  staff  128 Mar  4 21:35 borg

borg_base_dir/.cache/borg:
total 8
drwxrwx---  4 tw  staff  128 Mar  4 21:35 .
drwxrwx---  3 tw  staff   96 Mar  4 21:35 ..
drwxrwx---  8 tw  staff  256 Mar  4 21:35 8dbe69469c0c6f589f1f6ba1628a66f7eac38ec3699f0bb7634136c9a3c27bc5
-rw-rw----  1 tw  staff  198 Mar  4 21:35 CACHEDIR.TAG

borg_base_dir/.cache/borg/8dbe69469c0c6f589f1f6ba1628a66f7eac38ec3699f0bb7634136c9a3c27bc5:
total 176
drwxrwx---  8 tw  staff    256 Mar  4 21:35 .
drwxrwx---  4 tw  staff    128 Mar  4 21:35 ..
-rw-rw----  1 tw  staff     68 Mar  4 21:35 README
-rw-rw----  1 tw  staff  45382 Mar  4 21:35 chunks
drwxrwx---  2 tw  staff     64 Mar  4 21:35 chunks.archive.d
-rw-rw----  1 tw  staff    581 Mar  4 21:35 config
-rw-rw----  1 tw  staff  27066 Mar  4 21:35 files
-rw-rw----  1 tw  staff      2 Mar  4 21:35 pre12-meta

borg_base_dir/.cache/borg/8dbe69469c0c6f589f1f6ba1628a66f7eac38ec3699f0bb7634136c9a3c27bc5/chunks.archive.d:
total 0
drwxrwx---  2 tw  staff   64 Mar  4 21:35 .
drwxrwx---  8 tw  staff  256 Mar  4 21:35 ..

borg_base_dir/.config:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 21:35 .
drwxrwx---  4 tw  staff  128 Mar  4 21:35 ..
drwxrwx---  3 tw  staff   96 Mar  4 21:35 borg

borg_base_dir/.config/borg:
total 0
drwxrwx---  3 tw  staff  96 Mar  4 21:35 .
drwxrwx---  3 tw  staff  96 Mar  4 21:35 ..
drwxrwx---  3 tw  staff  96 Mar  4 21:35 security

borg_base_dir/.config/borg/security:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 21:35 .
drwxrwx---  3 tw  staff   96 Mar  4 21:35 ..
drwxrwx---  5 tw  staff  160 Mar  4 21:35 8dbe69469c0c6f589f1f6ba1628a66f7eac38ec3699f0bb7634136c9a3c27bc5

borg_base_dir/.config/borg/security/8dbe69469c0c6f589f1f6ba1628a66f7eac38ec3699f0bb7634136c9a3c27bc5:
total 24
drwxrwx---  5 tw  staff  160 Mar  4 21:35 .
drwxrwx---  3 tw  staff   96 Mar  4 21:35 ..
-rw-rw----  1 tw  staff    1 Mar  4 21:35 key-type
-rw-rw----  1 tw  staff   21 Mar  4 21:35 location
-rw-rw----  1 tw  staff   26 Mar  4 21:35 manifest-timestamp
drwxrwx---  8 tw  staff  256 Mar  4 21:35 repo
total 120
drwxrwx---   8 tw  staff    256 Mar  4 21:35 .
drwxr-xr-x  54 tw  staff   1728 Mar  4 21:35 ..
-rw-rw----   1 tw  staff     73 Mar  4 21:35 README
-rw-rw----   1 tw  staff    209 Mar  4 21:35 config
drwxrwx---   3 tw  staff     96 Mar  4 21:35 data
-rw-rw----   1 tw  staff     90 Mar  4 21:35 hints.5
-rw-rw----   1 tw  staff  41258 Mar  4 21:35 index.5
-rw-rw----   1 tw  staff    190 Mar  4 21:35 integrity.5

repo/data:
total 0
drwxrwx---  3 tw  staff   96 Mar  4 21:35 .
drwxrwx---  8 tw  staff  256 Mar  4 21:35 ..
drwxrwx---  8 tw  staff  256 Mar  4 21:35 0

repo/data/0:
total 9800
drwxrwx---  8 tw  staff      256 Mar  4 21:35 .
drwxrwx---  3 tw  staff       96 Mar  4 21:35 ..
-rw-rw----  1 tw  staff      476 Mar  4 21:35 0
-rw-rw----  1 tw  staff       17 Mar  4 21:35 1
-rw-rw----  1 tw  staff  4994118 Mar  4 21:35 2
-rw-rw----  1 tw  staff       49 Mar  4 21:35 3
-rw-rw----  1 tw  staff      539 Mar  4 21:35 4
-rw-rw----  1 tw  staff       17 Mar  4 21:35 5

@ThomasWaldmann
Copy link
Member

Also, for the usual 0o077 umask, it still looks good (everything only accessible for the user).

ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Mar 9, 2022
…up#6400

we tried to be very private / secure here, but that created the issue
that a less secure umask (like e.g. 0o007) just did not work.

to make the umask work, we must start from 0o777 mode and let the
umask do its work, like e.g. 0o777 & ~0o007 --> 0o770.

with borg's default umask of 0o077, it usually ends up being 0o700,
so only permissions for the user (not group, not others).
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Mar 9, 2022
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Mar 9, 2022
…up#6400

we tried to be very private / secure here, but that created the issue
that a less secure umask (like e.g. 0o007) just did not work.

to make the umask work, we must start from 0o777 mode and let the
umask do its work, like e.g. 0o777 & ~0o007 --> 0o770.

with borg's default umask of 0o077, it usually ends up being 0o700,
so only permissions for the user (not group, not others).
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Aug 4, 2022
…orgbackup#6400

hopefully this is the final fix.

after first fixing of borgbackup#6400 (by using os.umask after mkstemp), there
was a new problem that chmod was not supported on some fs.

even after fixing that, there were other issues, see the ACLs issue
documented in borgbackup#6933.

the root cause of all this is tempfile.mkstemp internally using a
very secure, but hardcoded and for our use case problematic mode
of 0o600.

mkstemp_mode (mosty copy&paste from python stdlib tempfile module +
"black" formatting applied) supports giving the mode via the api,
that is the only change needed.

slightly dirty due to the _xxx imports from tempfile, but hopefully
this will be supported in some future python version.
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Aug 4, 2022
…orgbackup#6400

hopefully this is the final fix.

after first fixing of borgbackup#6400 (by using os.umask after mkstemp), there
was a new problem that chmod was not supported on some fs.

even after fixing that, there were other issues, see the ACLs issue
documented in borgbackup#6933.

the root cause of all this is tempfile.mkstemp internally using a
very secure, but hardcoded and for our use case problematic mode
of 0o600.

mkstemp_mode (mosty copy&paste from python stdlib tempfile module +
"black" formatting applied) supports giving the mode via the api,
that is the only change needed.

slightly dirty due to the _xxx imports from tempfile, but hopefully
this will be supported in some future python version.
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Aug 4, 2022
…orgbackup#6400

hopefully this is the final fix.

after first fixing of borgbackup#6400 (by using os.umask after mkstemp), there
was a new problem that chmod was not supported on some fs.

even after fixing that, there were other issues, see the ACLs issue
documented in borgbackup#6933.

the root cause of all this is tempfile.mkstemp internally using a
very secure, but hardcoded and for our use case problematic mode
of 0o600.

mkstemp_mode (mosty copy&paste from python stdlib tempfile module +
"black" formatting applied) supports giving the mode via the api,
that is the only change needed.

slightly dirty due to the _xxx imports from tempfile, but hopefully
this will be supported in some future python version.
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Aug 4, 2022
…orgbackup#6400

hopefully this is the final fix.

after first fixing of borgbackup#6400 (by using os.umask after mkstemp), there
was a new problem that chmod was not supported on some fs.

even after fixing that, there were other issues, see the ACLs issue
documented in borgbackup#6933.

the root cause of all this is tempfile.mkstemp internally using a
very secure, but hardcoded and for our use case problematic mode
of 0o600.

mkstemp_mode (mosty copy&paste from python stdlib tempfile module +
"black" formatting applied) supports giving the mode via the api,
that is the only change needed.

slightly dirty due to the _xxx imports from tempfile, but hopefully
this will be supported in some future python version.
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Aug 4, 2022
…orgbackup#6400

hopefully this is the final fix.

after first fixing of borgbackup#6400 (by using os.umask after mkstemp), there
was a new problem that chmod was not supported on some fs.

even after fixing that, there were other issues, see the ACLs issue
documented in borgbackup#6933.

the root cause of all this is tempfile.mkstemp internally using a
very secure, but hardcoded and for our use case problematic mode
of 0o600.

mkstemp_mode (mosty copy&paste from python stdlib tempfile module +
"black" formatting applied) supports giving the mode via the api,
that is the only change needed.

slightly dirty due to the _xxx imports from tempfile, but hopefully
this will be supported in some future python version.
ThomasWaldmann added a commit that referenced this issue Aug 5, 2022
use a custom mkstemp with mode support, fixes #6933, fixes #6400
ThomasWaldmann added a commit that referenced this issue Aug 5, 2022
use a custom mkstemp with mode support, fixes #6933, fixes #6400
ThomasWaldmann added a commit to ThomasWaldmann/borg that referenced this issue Aug 5, 2022
…orgbackup#6400

hopefully this is the final fix.

after first fixing of borgbackup#6400 (by using os.umask after mkstemp), there
was a new problem that chmod was not supported on some fs.

even after fixing that, there were other issues, see the ACLs issue
documented in borgbackup#6933.

the root cause of all this is tempfile.mkstemp internally using a
very secure, but hardcoded and for our use case problematic mode
of 0o600.

mkstemp_mode (mosty copy&paste from python stdlib tempfile module +
"black" formatting applied) supports giving the mode via the api,
that is the only change needed.

slightly dirty due to the _xxx imports from tempfile, but hopefully
this will be supported in some future python version.
ThomasWaldmann added a commit that referenced this issue Aug 5, 2022
use a custom mkstemp with mode support, fixes #6933, fixes #6400
pytorchmergebot pushed a commit to pytorch/pytorch that referenced this issue Aug 24, 2023
### Description
The `download_url_to_file` function in torch.hub uses a temporary file to prevent overriding a local working checkpoint with a broken download.This temporary file is created using `NamedTemporaryFile`. However, since `NamedTemporaryFile` creates files with overly restrictive permissions (0600), the resulting download will not have default permissions and will not respect umask on Linux (since moving the file will retain the restrictive permissions of the temporary file). This is especially problematic when trying to share model checkpoints between multiple users as other users will not even have read access to the file.

The change in this PR fixes the issue by using custom code to create the temporary file without changing the permissions to 0600 (unfortunately there is no way to override the permissions behaviour of existing Python standard library code). This ensures that the downloaded checkpoint file correctly have the default permissions applied. If a user wants to apply more restrictive permissions, they can do so via usual means (i.e. by setting umask).

See these similar issues in other projects for even more context:
* borgbackup/borg#6400
* borgbackup/borg#6933
* zarr-developers/zarr-python#325

### Issue
#81297

### Testing
Extended the unit test `test_download_url_to_file` to also check permissions.

Pull Request resolved: #82869
Approved by: https://github.com/vmoens
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants