UCLH-Foundry / PIXL

PIXL Image eXtraction Laboratory
Apache License 2.0
8 stars 0 forks source link

Don't run docker containers as root #400

Open jeremyestein opened 2 months ago

jeremyestein commented 2 months ago

Fixes #234. Waiting for testing on GAE before merging.

Note the addition of two new variables PIXL_USER_UID and PIXL_USER_GID

Firstly merge all the Dockerfiles for images that we control (imaging, export, hasher) to make this process easier.

Run all our python containers as the user/group pixl, which we create as part of the build process, using the UID/GID as specified in the config.

Export API mounts export dir read-only as it doesn't need to write any more.

Document how the host must be set up for this to work.

Do same for orthanc images.

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.16%. Comparing base (cbf9e3b) to head (242aacf).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #400 +/- ## ======================================= Coverage 81.16% 81.16% ======================================= Files 78 78 Lines 3191 3191 ======================================= Hits 2590 2590 Misses 601 601 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

jeremyestein commented 2 months ago

In writing the docs, I have come up with what I think is a problem, that will likely need a user/group adding to the GAE to fix.


Imagine you are a user on the GAE with username fred, primary group fred, and supplementary group docker.

When you run the CLI, it will run as fred:fred, and normally any files it creates would also have this ownership. (ACLs defined in /gae will affect this however, so the files may have a different group ownership, such as fred:docker)

There is a problem here in that the export API will likely be running as some other user/group, so it will not be able to read the files.

Options for fixing:

jeremyestein commented 2 months ago

This change can't go in until the pixl user and group have been created on the GAE, and the pixl dev users added to the group.

dram1964 commented 2 months ago

This change can't go in until the pixl user and group have been created on the GAE, and the pixl dev users added to the group.

Hi @jeremyestein, I've added PIXL user and group to GAE05 - and added you to the group. Let me know how that works for you.

stefpiatek commented 1 month ago

Had to run using docker group GID because all mounts will preserve the group that wrote them.

Added as a secondary group and as its just a file permission I think that'd be fine.

Example of permission error for a directory:

pixl_dev-orthanc-anon-1  | E0614 14:32:09.175160             MAIN main.cpp:2123] Uncaught exception, stopping now: [boost::filesystem::directory_iterator::construct: Permission denied: "/run/secrets"]