OSC / ondemand

Supercomputing. Seamlessly. Open, Interactive HPC Via the Web
https://openondemand.org/
MIT License
294 stars 107 forks source link

Staging folder permissions #2798

Closed jrlagrone closed 1 year ago

jrlagrone commented 1 year ago

We're seeing the permissions of APP directories are being reflected in staging directory incorrectly (or at least not in the way we expect).

In particular, we remove have removed all write permissions on the app files, e.g.

ls -lu jupyterlab/
total 68
-r--r--r-- 1 appmgr appmgr   950 Apr 17 10:49 CHANGELOG.md
-r--r--r-- 1 appmgr appmgr   529 May  5 12:36 form.js
-r--r--r-- 1 appmgr appmgr 13072 May  5 12:36 form.yml.erb
-r--r--r-- 1 appmgr appmgr 12760 May  5 12:36 icon.png
-r--r--r-- 1 appmgr appmgr   707 May  5 13:07 info.md.erb
-r--r--r-- 1 appmgr appmgr  1087 Apr 17 10:49 LICENSE.txt
-r--r--r-- 1 appmgr appmgr   575 May  5 12:36 manifest.yml
-r--r--r-- 1 appmgr appmgr  2291 Apr 17 10:49 README.md
-r--r--r-- 1 appmgr appmgr  1372 May  5 12:39 submit.yml.erb
dr-xr-xr-x 2 appmgr appmgr  4096 May  5 13:09 template
-r--r--r-- 1 appmgr appmgr   274 May  5 12:39 view.html.erb

The folder has permissions like dr-xr-xr-x 3 appmgr appmgr 4096 May 5 13:00 jupyterlab

If we try to run the app with these permissions, we get a permission error like: Permission denied @ rb_sysopen - $HOME/ondemand/data/sys/dashboard/batch_connect/sys/jupyterlab/output/66670874-0d74-494e-82e5-f3a4b3ac44ab/user_defined_context.json

In particular, the staging directory gets created with similar permissions (e.g. no write permissions)

~/ondemand/data/sys/dashboard/batch_connect/sys/jupyterlab/output$ ls -lu
total 4
dr-xr-xr-x 3 username appmgr 4096 May  5 12:44 5ecf84e7-926f-4a32-b048-c1ce9c447ba5

Which causes the app to fail since the user can't write to the staging directory.

Note, if we give the template directory write permissions (e.g. chmod -R u+w jupyterlab/template) everything seems fine and we get a staging directory with permissions like

~/ondemand/data/sys/dashboard/batch_connect/sys/jupyterlab/output$ ls -lu
total 16
drwxr-xr-x 3 username appmgr 4096 May  5 13:57 6b56b26a-8757-45f3-bb58-5d51819cbe4f

I'm not sure this is a bug or not. We're doing this for an extra layer of protection (e.g. making it more difficult to directly edit files we want updated only by pull request.) We weren't able to locate where these staging folders are being created, so we are hoping we could modify that process so it uses reasonable permissions instead of inheriting them from the app/template folder permissions.

┆Issue is synchronized with this Asana task by Unito

johrstrom commented 1 year ago

Here's how the staging directory is made and how it get's copied over. I'll look into how/if we can adapt to your use case.

https://github.com/OSC/ondemand/blob/f9a02cfca01d54ef7b6f700060dc964f0eae7826/apps/dashboard/app/models/batch_connect/session.rb#L260-L263

jrlagrone commented 1 year ago

Thanks for the staging location.

At least for now, we're just setting the permissions to be writeable. We're reticent to change the internal behavior to avoid potential upgrade conflicts in the future.

rsync accepts chown and chmod options, so that's certainly a possibility (e.g. add --chown=username:usergroup --chmod=u+rwx). Though I'm not sure that's generally applicable or what associated settings would need to be added. At least the username seems straightforward, but determining the appropriate group is more problematic / system dependent. At least for us, these staging folders are in user home directories so just setting the user / user permissions seems reasonable and group permissions could all just be disabled (I think.) That being said, I'm not sure the "preserve group" implied by -a makes sense either

jrlagrone commented 1 year ago

Just wanted to note, that this appears to be intended behavior. See https://osc.github.io/ood-documentation/latest/how-tos/app-development/app-sharing.html. So apps are relying on file permissions for access controls.

I guess what we'd like is some sort of integrated access control instead of relying on underlying system settings. One of the related issues we've encountered is undefined behavior (at a system level) from users being in too many groups for reasons described in the link above (limiting access to licensed products, etc.). Some of out file systems do not support ACLs (or similar) so we're limited to vanilla unix groups. We're trying to move away from that, e.g. by using Slurm accounts as access controls when possible.

johrstrom commented 1 year ago

Hi, I'm just circling back to this now as we prep for 3.1.

I see 2 issues - the first is the creation of the staged root.

~/ondemand/data/sys/dashboard/batch_connect/sys/jupyterlab/output$ ls -lu
total 4
dr-xr-xr-x 3 username appmgr 4096 May  5 12:44 5ecf84e7-926f-4a32-b048c1ce9c447ba5

I can force this to be 700 - though I believe this is coming from your own personal umask. My directories get created as 755. Here's the relevant code that makes that directory.

https://github.com/OSC/ondemand/blob/ed3512e9ee3535ae06f56d8d7c7198667bf732af/apps/dashboard/app/models/batch_connect/session.rb#L259

The second issue is the file ownership/permissions after it's been copied. I think adding --chown=username:usergroup is fine (though I'll have to look up if the primary group will always be valid).

Changing the permissions after copy could be trickier - the copy has preserved permissions for forever, so to break that interface may cause issues with folks... But it's something I'll think on.

johrstrom commented 1 year ago

@jrlagrone I'll submit a patch for the staging folders now, but I believe we've fixed the other issue - preserving the mode bits copied files - in this pull request below (that was backported to 3.0, so it's in 3.0.3 right now.)

https://github.com/OSC/ondemand/pull/3045

johrstrom commented 1 year ago

I believe this is fixed in #3202 and #3045, both will be in the next version 3.1, one is already in 3.0.

johrstrom commented 1 year ago

Thanks for the ticket, and just re-open or ping me on this ticket to get it re-opened if you wish/need.