From 9c3af5793b1cf62879fe329c4a85ff3c188eaa30 Mon Sep 17 00:00:00 2001 From: Alex <25013571+alexhb1@users.noreply.github.com> Date: Tue, 12 May 2026 08:47:42 +0100 Subject: [PATCH] Fail closed on unwritable config (#985) --- entrypoint.sh | 49 ++++++------ tests/config/test_entrypoint_permissions.py | 89 +++++++++++++++++++++ 2 files changed, 113 insertions(+), 25 deletions(-) diff --git a/entrypoint.sh b/entrypoint.sh index 0e8e2cb9..bfd4d792 100644 --- a/entrypoint.sh +++ b/entrypoint.sh @@ -322,6 +322,27 @@ require_writable_dir() { fi } +fail_unwritable_config_dir() { + local folder="$1" + local owner + + owner=$(stat -c '%u:%g' "$folder" 2>/dev/null || echo "unknown") + + echo "" + echo "========================================================" + echo "ERROR: Config directory is not writable!" + echo "" + echo "Config directory: $folder" + echo "Current owner: $owner" + echo "Configured runtime identity: ${RUN_UID}:${RUN_GID}" + echo "" + echo "To fix this permanently, run on your HOST machine:" + echo " chown -R $RUN_UID:$RUN_GID /path/to/config" + echo "========================================================" + echo "" + exit 1 +} + resolve_runtime_home() { local runtime_home @@ -417,37 +438,15 @@ else # Config is Shelfmark-owned state, so it keeps the thorough repair path. make_writable "${CONFIG_DIR:-/config}" tree - # Fallback to root if config dir is still not writable (common on NAS/Unraid after upgrade from v0.4.0) + # Refuse to continue if the config directory is still not writable after repair. CONFIG_PATH=${CONFIG_DIR:-/config} set +e test_write "$CONFIG_PATH" >/dev/null 2>&1 config_ok=$? set -e - if [ $config_ok -ne 0 ] && [ "$RUN_UID" != "0" ]; then - config_owner=$(stat -c '%u' "$CONFIG_PATH" 2>/dev/null || echo "unknown") - if [ "$config_owner" = "0" ]; then - echo "" - echo "========================================================" - echo "WARNING: Permission issue detected!" - echo "" - echo "Config directory is owned by root but PUID=$RUN_UID." - echo "This typically happens after upgrading from v0.4.0 where" - echo "PUID/PGID settings were not respected." - echo "" - echo "Falling back to running as root to prevent data loss." - echo "" - echo "To fix this permanently, run on your HOST machine:" - echo " chown -R $RUN_UID:$RUN_GID /path/to/config" - echo "" - echo "Then restart the container." - echo "========================================================" - echo "" - RUN_UID=0 - RUN_GID=0 - USERNAME=root - TARGET_USER_SPEC="0:0" - fi + if [ $config_ok -ne 0 ]; then + fail_unwritable_config_dir "$CONFIG_PATH" fi fi diff --git a/tests/config/test_entrypoint_permissions.py b/tests/config/test_entrypoint_permissions.py index 0f5df186..cc9512a2 100644 --- a/tests/config/test_entrypoint_permissions.py +++ b/tests/config/test_entrypoint_permissions.py @@ -10,6 +10,9 @@ from pathlib import Path ENTRYPOINT_PATH = Path(__file__).resolve().parents[2] / "entrypoint.sh" ENTRYPOINT_LOCK_PATH = Path("/tmp/shelfmark_entrypoint_test.lock") BASH_PATH = shutil.which("bash") or "/bin/bash" +ID_PATH = shutil.which("id") or "/usr/bin/id" +MKDIR_PATH = shutil.which("mkdir") or "/bin/mkdir" +STAT_PATH = shutil.which("stat") or "/usr/bin/stat" @contextlib.contextmanager @@ -55,6 +58,61 @@ exit 2 printf '%s' "$HOME" > "$ENTRYPOINT_GUNICORN_HOME_FILE" printf '%s' "$*" > "$ENTRYPOINT_GUNICORN_ARGS_FILE" exit 0 +""", + ) + _write_executable( + bin_dir / "id", + """#!/bin/sh +if [ -n "${ENTRYPOINT_STUB_CURRENT_UID:-}" ]; then + if [ "$1" = "-u" ]; then + printf '%s\\n' "$ENTRYPOINT_STUB_CURRENT_UID" + exit 0 + fi + if [ "$1" = "-g" ]; then + printf '%s\\n' "$ENTRYPOINT_STUB_CURRENT_GID" + exit 0 + fi +fi +exec "$ENTRYPOINT_REAL_ID" "$@" +""", + ) + _write_executable( + bin_dir / "gosu", + """#!/bin/sh +shift +if [ "${ENTRYPOINT_STUB_GOSU_FAIL_WRITES:-false}" = "true" ] && [ "$1" = "sh" ] && [ "$2" = "-c" ]; then + exit 1 +fi +exec "$@" +""", + ) + _write_executable( + bin_dir / "mkdir", + """#!/bin/sh +for arg in "$@"; do + if [ "$arg" = "/var/log/shelfmark" ]; then + exit 0 + fi +done +exec "$ENTRYPOINT_REAL_MKDIR" "$@" +""", + ) + _write_executable( + bin_dir / "stat", + """#!/bin/sh +if [ "$1" = "-c" ]; then + if [ "$2" = "%u:%g" ]; then + printf '%s\\n' "${ENTRYPOINT_STUB_STAT_OWNER:-0:0}" + exit 0 + fi +fi +exec "$ENTRYPOINT_REAL_STAT" "$@" +""", + ) + _write_executable( + bin_dir / "chown", + """#!/bin/sh +exit 0 """, ) @@ -65,6 +123,8 @@ def _run_entrypoint( tmp_path: Path, *, extra_env: dict[str, str] | None = None, + simulate_root_startup: bool = False, + fail_gosu_writes: bool = False, stub_home: Path | str | None = None, ) -> tuple[subprocess.CompletedProcess[str], Path, Path, Path]: runtime_home = tmp_path / "runtime-home" @@ -86,6 +146,9 @@ def _run_entrypoint( "ENABLE_LOGGING": "false", "ENTRYPOINT_GUNICORN_ARGS_FILE": str(runtime_args_file), "ENTRYPOINT_GUNICORN_HOME_FILE": str(runtime_home_file), + "ENTRYPOINT_REAL_ID": ID_PATH, + "ENTRYPOINT_REAL_MKDIR": MKDIR_PATH, + "ENTRYPOINT_REAL_STAT": STAT_PATH, "ENTRYPOINT_STUB_GID": str(os.getgid()), "ENTRYPOINT_STUB_HOME": str(stub_home), "ENTRYPOINT_STUB_UID": str(os.getuid()), @@ -99,6 +162,18 @@ def _run_entrypoint( "USING_EXTERNAL_BYPASSER": "true", } ) + if simulate_root_startup: + env.update( + { + "ENTRYPOINT_STUB_CURRENT_GID": "0", + "ENTRYPOINT_STUB_CURRENT_UID": "0", + "ENTRYPOINT_STUB_STAT_OWNER": "0:0", + "PGID": str(os.getgid()), + "PUID": str(os.getuid()), + } + ) + if fail_gosu_writes: + env["ENTRYPOINT_STUB_GOSU_FAIL_WRITES"] = "true" if extra_env: env.update(extra_env) @@ -168,3 +243,17 @@ def test_entrypoint_non_root_mode_requires_writable_config_dir(tmp_path): f"Config directory is not writable in non-root mode: {readonly_config_dir}" in result.stdout ) assert "Prepare ownership outside the container" in result.stdout + + +def test_entrypoint_root_bootstrap_fails_closed_when_config_repair_fails(tmp_path): + result, _, _, _ = _run_entrypoint( + tmp_path, + simulate_root_startup=True, + fail_gosu_writes=True, + ) + + assert result.returncode == 1 + assert "ERROR: Config directory is not writable!" in result.stdout + assert f"Configured runtime identity: {os.getuid()}:{os.getgid()}" in result.stdout + assert f"chown -R {os.getuid()}:{os.getgid()} /path/to/config" in result.stdout + assert "Startup mode: root" not in result.stdout