[BUG] pkgrepo.managed silently change file mode to UMASK

[ymasson]

Description
I use hardened Debian servers. This include a default UMASK enforced to 027 (0640 for files).
This hardening is executed after the Debian installation.
So, /etc/apt/sources.list mode is 0644.

I have a state managing /etc/apt/sources.list.d/salt.list using pkgrepo.managed.
On every state.highstate the mode of /etc/apt/sources.list is modified to 0640 without any change reported.

Setup
use a debian:bullseye Docker container.
install Salt via Salt's repositories and apt-get install salt-minion.
delete all .list files in /etc/apt/sources.list.d/.

create a state like

root@813ebf88c190:/# cat srv/salt/test/init.sls
test:
  pkgrepo.managed:
    - comments: salt
    - name: deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main
    - file: /etc/apt/sources.list.d/salt.list
    - key_url: https://repo.saltproject.io/salt/py3/debian/11/amd64/SALT-PROJECT-GPG-PUBKEY-2023.gpg
    - gpgcheck: 1
    - aptkey: False
    - refresh: True
    - clean_file: True

change the /etc/apt/sources.list mode to 0640 (simulate a different mode than the default UMASK).
run a salt-call --local state.apply test
check the /etc/apt/sources.list mode.

• [ x] on-prem machine
• [x ] VM (Virtualbox, KVM, etc. please specify)
• [x ] VM running on a cloud service, please be explicit and add details
• [x ] container (Kubernetes, Docker, containerd, etc. please specify)
• or a combination, please be explicit
• jails if it is FreeBSD
• classic packaging
• [x ] onedir packaging
• used bootstrap to install

Steps to Reproduce the behavior
I use docker run -it --rm debian:bullseye , install Salt and remove the .list file

mkdir /etc/apt/keyrings
curl -fsSL -o /etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg https://repo.saltproject.io/salt/py3/debian/11/amd64/SALT-PROJECT-GPG-PUBKEY-2023.gpg
echo "deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main" >/etc/apt/sources.list.d/saltstack.list
apt-get update
apt-get install salt-minion
rm /etc/apt/sources.list.d/saltstack.list

Then create the state and

root@813ebf88c190:/# chmod 640 /etc/apt/sources.list
root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r----- 1 root root 430 Mar 11 18:37 /etc/apt/sources.list

root@813ebf88c190:/# salt-call --local state.apply test
/opt/saltstack/salt/lib/python3.10/site-packages/salt/ext/tornado/netutil.py:493: DeprecationWarning: ssl.PROTOCOL_TLS is deprecated
  context = ssl.SSLContext(
local:
----------
          ID: test
    Function: pkgrepo.managed
        Name: deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main
      Result: True
     Comment: Configured package repo 'deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main'
     Started: 18:40:45.495649
    Duration: 1914.183 ms
     Changes:   
              ----------
              repo:
                  deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main

Summary for local
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1
Total run time:   1.914 s

root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r--r-- 1 root root 430 Mar 11 18:40 /etc/apt/sources.list

root@813ebf88c190:/# chmod 640 /etc/apt/sources.list
root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r----- 1 root root 430 Mar 11 18:40 /etc/apt/sources.list

root@813ebf88c190:/# salt-call --local state.apply test
/opt/saltstack/salt/lib/python3.10/site-packages/salt/ext/tornado/netutil.py:493: DeprecationWarning: ssl.PROTOCOL_TLS is deprecated
  context = ssl.SSLContext(
local:
----------
          ID: test
    Function: pkgrepo.managed
        Name: deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main
      Result: True
     Comment: Configured package repo 'deb [signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg arch=amd64] https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7 bullseye main'
     Started: 18:41:14.794378
    Duration: 859.441 ms
     Changes:   

Summary for local
------------
Succeeded: 1
Failed:    0
------------
Total states run:     1
Total run time: 859.441 ms

root@813ebf88c190:/# ls -l /etc/apt/sources.list
-rw-r--r-- 1 root root 430 Mar 11 18:41 /etc/apt/sources.list

Versions Report

Salt Version:
          Salt: 3006.7
 
Python Version:
        Python: 3.10.13 (main, Feb 19 2024, 03:31:20) [GCC 11.2.0]
 
Dependency Versions:
          cffi: 1.14.6
      cherrypy: 18.6.1
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 3.1.3
       libgit2: Not Installed
  looseversion: 1.0.2
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.2
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     packaging: 22.0
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.19.1
        pygit2: Not Installed
  python-gnupg: 0.4.8
        PyYAML: 6.0.1
         PyZMQ: 23.2.0
        relenv: 0.15.1
         smmap: Not Installed
       timelib: 0.2.4
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.10.0-28-amd64
        system: Linux
       version: Debian GNU/Linux 11 bullseye

Additional context
If I add

print(reposplit)
print(sanitizedsplit)

just before the ifstatement here https://github.com/saltstack/salt/blob/master/salt/states/pkgrepo.py#L521
if sanitizedsplit != reposplit:

I have

['deb', '[signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg', 'arch=amd64]', 'bullseye', 'https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7', 'main']
['deb', '[arch=amd64', 'signed-by=/etc/apt/keyrings/SALT-PROJECT-GPG-PUBKEY-2023.gpg]', 'bullseye', 'https://repo.saltproject.io/salt/py3/debian/11/amd64/minor/3006.7', 'main']

[] are not excluded.

So,
1: the file mode of /etc/apt/sources.list is modified even though it is not managed by the state.
2: it seems a change is detected due to the bad .split(), but nothing is reported.

fun fact, if I sort [arch=amd64 signed-by=xxxx], nothing append. no mode change, nothing.

[ymasson]

@whytewolf maybe it is similar to #65703 ?

[whytewolf]

i don't think it is related.

#65703 is caused by indexing issues. that issue is going to be fixed in #66164 by switching to an OrderedDict and just dropping the weird indexing that was implemented before.

[jg-basis]

I note this issue on Debian 3006.7