vikunja
/
vikunja
6
43
Fork 72
Code Issues 150 Pull Requests 19 Packages Releases 32 Activity

Skip modify of user/group when PUID/GUID not changed #1541

Closed
rocktop wants to merge 2 commits from rocktop/api:no_root into main
pull from: rocktop/api:no_root
merge into: vikunja:main
Conversation 5 Commits 2 Files Changed 3 +18 -3
rocktop commented 2023-06-05 19:20:23 +00:00
First-time contributor
Copy Link

With this change, the API container will now run as the non-root user vikunja.

If you need to change the PUID and/or PGID of the user, you must restart the container and run it as root, e.g.:
docker run -e PGID=1012 -e PUID=1012 --user 0 -v /tmp/vikunja-api-dev-files:/app/vikunja/files vikunja/api:dev

With this change, the API container will now run as the non-root user vikunja. If you need to change the PUID and/or PGID of the user, you must restart the container and run it as root, e.g.: `docker run -e PGID=1012 -e PUID=1012 --user 0 -v /tmp/vikunja-api-dev-files:/app/vikunja/files vikunja/api:dev`
rocktop added 1 commit 2023-06-05 19:20:24 +00:00
continuous-integration/drone/pr Build is passing Details
52009b3327 Skip modify of user/group when PUID/GUID not changed 
With this change, you can run the API image as non-root.
To run as non-root you must use 1000:1000.
konrad requested changes 2023-06-06 09:38:43 +00:00
konrad left a comment
Owner
Copy Link

Looks like this does not work.

Running the container with

docker run -p 3456:3456 -e PGID=1000 -e PUID=1000 -v /tmp/vikunja-api-dev-files:/app/vikunja/files vikunja/api:dev

gives me

info: creation of non-root user is skipped
2023/06/06 09:34:49 No config file found, using default or config from environment variables.
2023-06-06T09:34:49.678737621Z: INFO	▶ migration/Migrate 0ac Ran all migrations successfully.
2023-06-06T09:34:49.678857752Z: INFO	▶ models/RegisterReminderCron 0ad Mailer is disabled, not sending reminders per mail
2023-06-06T09:34:49.678904722Z: INFO	▶ models/RegisterOverdueReminderCron 0ae Mailer is disabled, not sending overdue per mail
2023-06-06T09:34:49.678997723Z: INFO	▶ cmd/func25 0af Vikunja version v0.20.4+207-52009b3327
⇨ http server started on [::]:3456

and Vikunja still runs as root.

Looks like this does not work. Running the container with ``` docker run -p 3456:3456 -e PGID=1000 -e PUID=1000 -v /tmp/vikunja-api-dev-files:/app/vikunja/files vikunja/api:dev ``` gives me ``` info: creation of non-root user is skipped 2023/06/06 09:34:49 No config file found, using default or config from environment variables. 2023-06-06T09:34:49.678737621Z: INFO ▶ migration/Migrate 0ac Ran all migrations successfully. 2023-06-06T09:34:49.678857752Z: INFO ▶ models/RegisterReminderCron 0ad Mailer is disabled, not sending reminders per mail 2023-06-06T09:34:49.678904722Z: INFO ▶ models/RegisterOverdueReminderCron 0ae Mailer is disabled, not sending overdue per mail 2023-06-06T09:34:49.678997723Z: INFO ▶ cmd/func25 0af Vikunja version v0.20.4+207-52009b3327 ⇨ http server started on [::]:3456 ``` and Vikunja still runs as root.
rocktop commented 2023-06-06 13:09:11 +00:00
Author
First-time contributor
Copy Link

You need to set the user when you run the command.

docker run -e PGID=1000 -e PUID=1000 --user vikunja ...
or
docker run -e PGID=1000 -e PUID=1000 --user 1000 ...

You need to set the user when you run the command. `docker run -e PGID=1000 -e PUID=1000 --user vikunja ...` or `docker run -e PGID=1000 -e PUID=1000 --user 1000 ...`
rocktop requested review from konrad 2023-06-06 13:09:25 +00:00
konrad commented 2023-06-06 14:59:10 +00:00
Owner
Copy Link

Can you change it so that passing the user won't be required?

Can you change it so that passing the user won't be required?
rocktop commented 2023-06-06 15:45:33 +00:00
Author
First-time contributor
Copy Link

Added.

Added.
konrad requested changes 2023-06-06 15:48:28 +00:00
Dockerfile Outdated
@ -42,2 +42,4 @@
COPY --from=builder /build/vikunja-* vikunja
USER vikunja
konrad commented 2023-06-06 15:48:26 +00:00
Owner
Copy Link

That will break the dynamic uid switching because the vikunja user does not have permission to do that. You need to change this in the entrypoint script.

That will break the dynamic uid switching because the `vikunja` user does not have permission to do that. You need to change this in the entrypoint script.
rocktop commented 2023-06-06 15:58:32 +00:00
Author
First-time contributor
Copy Link

Dynamic uid switching is only possible as root. If you want to keep it, the default user has to remain root (this change gets reverted). Therefore, the opt-in of --user vikunja will be required if you want to run as non-root.

Dynamic uid switching is only possible as root. If you want to keep it, the default user has to remain root (this change gets reverted). Therefore, the opt-in of `--user vikunja` will be required if you want to run as non-root.
konrad commented 2023-06-06 16:12:39 +00:00
Owner
Copy Link

Dynamic UID switching should still be possible.

Please revert this and add a comment to the docs about how to run the api rootless.

Dynamic UID switching should still be possible. Please revert this and add a comment to the docs about how to run the api rootless.
rocktop marked this conversation as resolved
rocktop force-pushed no_root from 40ed6bfba7 to 51b73e0463 2023-06-07 19:10:08 +00:00 Compare
rocktop requested review from konrad 2023-06-07 19:10:40 +00:00
konrad reviewed 2023-06-07 19:38:06 +00:00
docs/content/doc/setup/docker-start-to-finish.md
@ -108,2 +108,4 @@
</div>
<div class="notification is-info">
<b>NOTE:</b> The API image supports running as the non-root user vikunja as long as PUID/PGUID are static. A modification to docker-compose is required.
konrad commented 2023-06-07 19:38:06 +00:00
Owner
Copy Link

Nice!

Can you add the same note to these two other places as well? https://kolaente.dev/vikunja/api/src/branch/main/docs/content/doc/setup/install-backend.md and https://kolaente.dev/vikunja/api/src/branch/main/docs/content/doc/setup/full-docker-example.md

Nice! Can you add the same note to these two other places as well? https://kolaente.dev/vikunja/api/src/branch/main/docs/content/doc/setup/install-backend.md and https://kolaente.dev/vikunja/api/src/branch/main/docs/content/doc/setup/full-docker-example.md
konrad commented 2023-10-11 08:59:42 +00:00
Owner
Copy Link

hey @rocktop are you still interested in this?

hey @rocktop are you still interested in this?
konrad commented 2024-01-16 12:17:22 +00:00
Owner
Copy Link

Closing this due to inactivity. Please ping if you want to pick it up again.

Closing this due to inactivity. Please ping if you want to pick it up again.
konrad closed this pull request 2024-01-16 12:17:22 +00:00
All checks were successful
continuous-integration/drone/pr Build is passing
Required
Details

Pull request closed

Please reopen this pull request to perform a merge.
Sign in to join this conversation.
Reviewers
No reviewers
konrad
Labels
Clear labels   
dependencies

   
duplicate

This issue or pull request already exists

  
help wanted

Need some help

  
invalid

Something is wrong

  
kind/bug

Something is not working

  
kind/feature

New feature

  
needs reproduction

   
question

More information is needed

  
security

   
wontfix

This won't be fixed

No Label
dependencies
duplicate
help wanted
invalid
kind/bug
kind/feature
needs reproduction
question
security
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: vikunja/vikunja#1541
No description provided.
Delete Branch "rocktop/api:no_root"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?