unconditionally creates dirs/files/links as root

[martinpitt]

Spotted in #385:

This is moved code, but I find the unconditional superuser: try here (pretty much all of mkdir, mv, etc.) problematic. Won't that create root-owned directories even if you run it in e.g. your home dir? I feel like this would better work the other way round: first try as your user, and if it fails on permission-denied, retry with root?

This of course becomes even more interesting if it's someone else's home directory. Even a system user, like /var/lib/postgresql/. Perhaps the default should be to run it as the user who owns the containing directory?

Originally posted by @martinpitt in #385 (comment)

[jelly]

Create directory is indeed broken

Rename

Renaming a directory keeps current directory permissions. Same for copy/paste.

[admin@fedora-39-127-0-0-2-2201 ~]$ mkdir test
[admin@fedora-39-127-0-0-2-2201 ~]$ mkdir moveme
[admin@fedora-39-127-0-0-2-2201 ~]$ ls -lh moveme/
total 0
[admin@fedora-39-127-0-0-2-2201 ~]$ ls -lh | grep moveme
drwxr-xr-x. 1 admin admin    0 Apr 23 12:31 moveme

Root:

[root@fedora-39-127-0-0-2-2201 admin]# mv moveme/ foo
[root@fedora-39-127-0-0-2-2201 admin]# ls -lh
total 77M
-rw-r--r--. 1 admin admin 1.7K Apr 22 14:25 0001-WiP-debug-popover.patch
-rw-r--r--. 1 admin admin 1.7K Apr 22 14:25 0001-WiP-debug-popover.txt
-rw-r--r--. 1 admin admin 7.3M Apr 22 14:08 Files.zip
drwxr-xr-x. 1 admin admin    0 Apr 23 12:31 foo
-rw-r--r--. 1 admin admin  14K Apr 22 14:25 gpu-hang.png
drwxr-xr-x. 1 root  root     0 Apr 23 12:28 jij
-rw-r--r--. 1 admin admin 2.1M Apr 22 11:45 KCOnYJg.png
drwxr-xr-x. 1 root  root     0 Apr 23 12:27 lol
-rw-r--r--. 1 admin admin 1.3M Apr 22 11:45 Navigator.penpot
-rw-r--r--. 1 admin admin 1.3M Apr 22 11:45 Navigator.penpot.zip
-rw-r--r--. 1 admin admin  42M Apr 22 11:45 node_modules.tar.zst
-rw-r--r--. 1 admin admin  23M Apr 22 13:59 node-v18.18.0-linux-x64.tar.xz
lrwxrwxrwx. 1 root  root    38 Apr 23 12:30 test -> /home/admin/0001-WiP-debug-popover.txt
drwxr-xr-x. 1 admin admin   56 Apr 23 12:29 testnew

This seems to work?

Symlink

Are created as root not as your current user

[jelly]

This needs some preparation work for what I think is a reasonable fix:

• When creating a directory:
  • I am not a "superuser" but regular issue, allow it permission error will occur as expected when trying to create
  • I am "superuser" and the $CWD is owned by me, should not use superuser: "try" as that will create a dir owned by root
  • I am a "superuser" and the $CWD is not owned by me, should use superuser: try or even require as I cannot make that dir. However that likely does not create the expected result, or it does. Say I have to create a postgresql "state dir" or something, then I probably want it owned by postgresql which might be the $CWD owner.

Option 3 might go to far or too complicated. I say we create it as root and then let the "admin" change it via edit permissions.

This requires us to have information about the $CWD everywhere basically we need to bring back rootInfo from fsinfo this contains all the useful bits except the $CWD path in state. Ideally this would be provided via the exisitng ContextProvider so you don't have to pass it down everywhere and it only changes "once" which requires a full app re-render anyway.

Secondly we need tests which we can make before fixing this bug. The current tests are super naive and just run everything as "superuser" admin while this feature can also be used to give Bob access to the server without them being admin in some shared project.

For us to test a normal user we need to copy the cockpit-podman test code to create an admin session. Also partly copied in cockpit tests and even bots itself. This needs to become something on either testlib or bots.

The authorized_keys copy'ing is assume done for packit as we don't prepare the admin user there like in bots. But hmm maybe this should just be moved to browser.sh instead. That seems a more logical place as we create the user there too.

[jelly]

Tested GNOME files and if I go to admin:/var/lib/redis and create a directory it creates it as root. We don't have to follow that behavior but I did in this draft PR #421

Interesting note, this would behave also different as the current cockpit-navigator plugin.

[allisonkarlitskaya]

This was fixed by #478