search

mohamnag / nginx-file-browser

a simple file browser for nginx autoindex module
MIT License
151 stars 49 forks source link

Run container unprivileged #8

Closed adberger closed 2 years ago

adberger commented 2 years ago

Relates to #7 First tests seem to be working

adberger commented 2 years ago

@mohamnag do you want to take a look at this?

mohamnag commented 2 years ago

Hi, sorry for late response. Is the port change from 80 to 8080 also due to privilege? Asking because that’s a breaking change for many.

Regards

On 07.04.2022, at 08:19, Adrian Berger @.***> wrote:

@mohamnag https://github.com/mohamnag do you want to take a look at this?

— Reply to this email directly, view it on GitHub https://github.com/mohamnag/nginx-file-browser/pull/8#issuecomment-1091122493, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCGDDLY56EKPWIIFIMI35LVDZ47FANCNFSM5SH7BKUQ. You are receiving this because you were mentioned.

adberger commented 2 years ago

Hi, sorry for late response. Is the port change from 80 to 8080 also due to privilege? Asking because that’s a breaking change for many. Regards … On 07.04.2022, at 08:19, Adrian Berger @.***> wrote: @mohamnag https://github.com/mohamnag do you want to take a look at this? — Reply to this email directly, view it on GitHub <#8 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCGDDLY56EKPWIIFIMI35LVDZ47FANCNFSM5SH7BKUQ. You are receiving this because you were mentioned.

Yes, when running as non-root the system ports (0–1023) require root permissions or at least some additional capabilities which is not desired. Maybe you could also provide a root & non-root image of your file browser to avoid breaking changes?

mohamnag commented 2 years ago

Yes it definitely needs to be different tags. This means different docker file. If you would please change the PR to have separate docker file, I will adjust the GitHub workflow to publish two images.

Regards

On 07.04.2022, at 13:26, Adrian Berger @.***> wrote:

Hi, sorry for late response. Is the port change from 80 to 8080 also due to privilege? Asking because that’s a breaking change for many. Regards … <x-msg://4/#> On 07.04.2022, at 08:19, Adrian Berger @.***> wrote: @mohamnag https://github.com/mohamnag https://github.com/mohamnag https://github.com/mohamnag do you want to take a look at this? — Reply to this email directly, view it on GitHub <#8 (comment) https://github.com/mohamnag/nginx-file-browser/pull/8#issuecomment-1091122493>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCGDDLY56EKPWIIFIMI35LVDZ47FANCNFSM5SH7BKUQ https://github.com/notifications/unsubscribe-auth/ABCGDDLY56EKPWIIFIMI35LVDZ47FANCNFSM5SH7BKUQ. You are receiving this because you were mentioned.

Yes, when running as non-root the system ports (0–1023) require root permissions or at least some additional capabilities which is not desired. Maybe you could also provide a root & non-root image of your file browser to avoid breaking changes?

— Reply to this email directly, view it on GitHub https://github.com/mohamnag/nginx-file-browser/pull/8#issuecomment-1091618847, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABCGDDKGQRRV6Y5YBFBZECDVD3A7DANCNFSM5SH7BKUQ. You are receiving this because you were mentioned.

adberger commented 2 years ago

Something like this?

adberger commented 2 years ago

@mohamnag any update on this one?

mohamnag commented 2 years ago

sorry, totally lost track here. thanks for the changes, will merge it.

adberger commented 2 years ago

You probably need to add file in .github/workflows)/docker-image-rootless.yml as the Dockerfile is named different. See: https://github.com/docker/build-push-action#inputs

adberger commented 2 years ago

You probably need to add file in .github/workflows)/docker-image-rootless.yml as the Dockerfile is named different. See: https://github.com/docker/build-push-action#inputs

@mohamnag forgot to tag you :)

mohamnag commented 2 years ago

hi, thanks. just trying to get this through in between other tasks. please check after this action is through if the correct container is published under latest-rootless. The tag might also need to be mentioned on readme

adberger commented 2 years ago

hi, thanks. just trying to get this through in between other tasks. please check after this action is through if the correct container is published under latest-rootless. The tag might also need to be mentioned on readme

@mohamnag

Now it seems to work fine, thanks :) Yeah you could probaly update the README.md from $ docker run -p 8080:8080 -v /path/to/my/files/:/opt/www/files/ mohamnag/nginx-file-browser to $ docker run -p 8080:8080 -v /path/to/my/files/:/opt/www/files/ mohamnag/nginx-file-browser::latest-rootless

The command for the root container can IMO stay as it is, because it defaults to :latest