fix: re-enable uname support; fix build chain vulnerability issue; ...#56
fix: re-enable uname support; fix build chain vulnerability issue; ...#56Gunzinger wants to merge 1 commit into
Conversation
…h su-exec, reduce variable duplication in Dockerfile; add switch to disable mumble ICE integration without using a config file; address shellcheck warnings; fix some spelling errors
|
PUID / PGID could be removed entirely as it is a redundant with the usage of |
Krzmbrzl
left a comment
Krzmbrzl
left a comment
There was a problem hiding this comment.
If possible, I would prefer having individual commits for independent changesets. Atm, there are at least the following changesets in this PR
- Add possibility to inject additional build flags (cmake arguments & build number)
- Fix supply-chain vulnerability
- Fix uname support
- Misc fixes in README (typos, HTML attributes)
- Option to disable Ice
- Various shell refactorings (though for some of them I am not sure if they are objectively better or mainly changed due to subjective preference)
Also, I think for the time being, we don't have to think about Alpine support as it doesn't provide a package for zeroc-ice, which we need. Thus, I think it would make a lot of sense to switch to using setpriv instead of su-exec.
| readonly CONFIG_REGEX="^(\;|\#)?\ *([a-zA-Z_0-9]+)=.*" | ||
| CONFIG_FILE="${DATA_DIR}/mumble_server_config.ini" | ||
|
|
||
| # shellcheck disable=SC2034 |
| set_config "ice" "\"tcp -h 127.0.0.1 -p 6502\"" true | ||
| { # Add ICE section | ||
| echo -e "\n[Ice]" | ||
| echo "Ice.Warn.UnknownProperties=1" | ||
| echo "Ice.MessageSizeMax=65536" | ||
| } >> "$CONFIG_FILE" |
| if [[ "$(id -u)" = 0 && ( "${PUID}" != 0 || "${MUMBLE_UNAME_ENABLE}" == true ) && "${MUMBLE_CHOWN_DATA}" == true ]]; then | ||
| if [[ "${MUMBLE_UNAME_ENABLE}" == true ]]; then | ||
| # if we are here, root with uname enabled | ||
| if [[ ( "$(id -u mumble)" -ne ${MUMBLE_UNAME_UID} ) || ( "$(getent group mumble | cut -d: -f3 )" -ne ${MUMBLE_UNAME_GID} ) ]]; then | ||
| # MUMBLE_UNAME_UID or MUMBLE_UNAME_GID have changed -> adjust mumble/uname uid+gid | ||
| groupmod -og "${MUMBLE_UNAME_GID}" mumble | ||
| usermod -ou "${MUMBLE_UNAME_UID}" -g "${MUMBLE_UNAME_GID}" mumble | ||
| fi | ||
| set_config "uname" "mumble" | ||
| chown -R "${MUMBLE_UNAME_UID}":"${MUMBLE_UNAME_GID}" /data | ||
| echo "Running Mumble server as uid=${MUMBLE_UNAME_UID} gid=${MUMBLE_UNAME_GID}" | ||
| else | ||
| # if we are here, root with puid | ||
| chown -R "${PUID}":"${PGID}" /data | ||
| echo "Running Mumble server as uid=${PUID} gid=${PGID}" | ||
| fi |
There was a problem hiding this comment.
Inconsistent indentation (we use tabs)
| if [[ "$(id -u)" = 0 && "${PUID}" != "$(id -u)" && ! "${MUMBLE_UNAME_ENABLE}" == true ]]; then | ||
| su-exec "${PUID}":"${PGID}" "${server_invocation[@]}" | ||
| else | ||
| # start up with root (or we are in a rootless context/container started with --user parameter) | ||
| exec "${server_invocation[@]}" | ||
| fi | ||
| fi No newline at end of file |
There was a problem hiding this comment.
Please don't change indentation or trailing newlines
Removing these optin would likely break existing workflows that set these variables instead of making use of the As for your TODO list: I think we should put most usage docs and examples in particular into the wiki instead of the README. |
2 participants
learnings:
shadowpackage for usermod and groupmodsu-execentirely and work withsetpriv; e.g.:setpriv --euid="${PUID}" --egid="${PGID}" --inh-caps=-all -- "${server_invocation[@]}"for the server invocationtodo: