-
-
Notifications
You must be signed in to change notification settings - Fork 360
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
Fix RAW file support for docker non-root users #829
Fix RAW file support for docker non-root users #829
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #829 +/- ##
=======================================
Coverage 65.53% 65.53%
=======================================
Files 138 138
Lines 12895 12895
Branches 533 533
=======================================
Hits 8451 8451
Misses 4288 4288
Partials 156 156
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
I have this same issue. Glad to see it was a simple fix. I would love if this could get merged |
I had the same issue, but this did not fix it for me without modifying the paths. For some reason mine have to be created at |
RUN if [ "${TARGETPLATFORM}" = "linux/amd64" ] || [ "${TARGETPLATFORM}" = "linux/arm64" ]; then \ | ||
apt install -y darktable; fi | ||
apt install -y darktable && \ | ||
mkdir -p /.cache/darktable /.config/darktable; fi |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The .cache
and .config
folders are created in the user's home folder: in case, the image is running under the root, they are created in the /root
folder. In case, it is running under a non-root user, it will be in the /home/<user>
folder. There should be permission to create files and folders in user's home folder, so I don't see a need to do it at the image creation time (which will require also chown
and chmod
commands to be run, as Dockerfile is executed by root and folders, created here, are owned by root as well). It makes sense only if you also add the USER
declaration at the end of the Dockerfile, which will result in the creation of the image, always running under specified non-root user
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but if for some reason the darktable
, executed under non-root user, wants to create these folders in the Dockerfile's WORKDIR location, then from my point of view it makes more sense to set the RWX permissions to the folder and all its content for all users by the command chmod -R 777 /app
- this is the application's working folder and the application has to have permissions to do anything there, even if run in non-root mode.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated my PR #863 with the changes, proposed in the previous comment. Please try to build and run the image from that PR and test whether the problem is fixed for you
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change in #863 is extremely yikes. The application's folder should really only be relevant for the execution of the application, not for anything else. The current state where a manual volume mount is required is objectively better than 777-ing the whole application. A root owned FS with 755 for folders/executables only and 644 otherwise should be the most open it should ever need to be.
Strictly speaking, from a security perspective, an image should even be able to function if the whole image is set completely immutable/read-only and this application easily can run in those conditions (I force the image at deployment into read-only) with the proper volumes mounted.
It also does not account for the paths mentioned in this PR so neither PR actually deals with the problem properly as far as I can tell; either PR assumes the directory exists on a given location but there is no such guarantee with the way darktable is executed. That is the issue to resolve first.
Since the folder/volume locations are really the only problem, the proper and more secure way to do this is configure/run darktable with the config/cache/tmp in a fixed/known location within the image instead of relying on the environment.
The clean way to do this would be to run darktable with the flags --cachedir
and --configdir
(and optionally --tmpdir
) set to fixed/known directories. This could, for instance, be /darktable/{cache,config,tmp}
with broader permissions on specifically only those sub-directories (probably 1777
) and marked VOLUME
in the Dockerfile. The location is then constant regardless of the user running things and any files created there-in are owned and writable by the user running darktable only (whichever that one ends up being).
Additionally, it'd be good to run with a non-root user and you could then be more specific with the permissions for those 3 sub-directories but that is, after the above, basically just a cherry-on-top and doesn't really do anything for the core issue at hand here (OP already runs non-root and so do I).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for your feedback. I'll do a review of #863 and try to include the fix for this issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Today I did a code review of my PR #863 and rewrote it to build an image, configured to run under a non-root user.
As part of this change, I investigated the issue that Darktable fails to run because of missing 2 mentioned folders and with inability to create them. As I see, there is no need to create them in the /
folder, as Darktable wants them to be in the user's home folder. So, once the /home/user
folder exists and is owned by the user, under which Photoview is running, Darktable creates those folders inside it successfully and runs fine.
@jordy2254, as the fix, proposed by this PR, is implemented in my #863 and @subtlepseudonym didn't respond to my comments, I propose to decline this PR. |
Supporting RAW file formats requires running
darktable-cli
, which fails in docker for non-root users with the following error:Both
/.config/darktable
and/.cache/darktable
must be present to rundarktable-cli
. Normally, these directories would be silently created on first run, but non-root users don't have permission to create directories.This change creates those directories in the docker image.