From 84b1dadea712e7babccfd45f6075c24ef3782e9b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 7 Sep 2026 18:59:19 +0000 Subject: [PATCH] fix: make the non-root container boot without writing into the image The secret key fix leaves one image path still written on every start, and DATA_DIR is now shared state the old in-place write was not safe for. Static assets. config.py deletes and re-copies every file from the frontend build into open_webui/static at import. That directory lives in the image, so a UID that does not own it fails each copy and fills the boot log: ERROR [root] An error occurred: [Errno 13] Permission denied: '/app/backend/open_webui/static/favicon.png' The Dockerfile answers this with chgrp 0 + chmod g=u, which covers OpenShift because its random UID is always in group 0. Rootless Podman's keep-id maps to 1000:1000, which is not, so it falls through to the "other" bits. Widening those is the wrong trade: static holds loader.js and custom.css, served to every browser, so a world-writable copy lets anything else in the container inject script into the UI. The writes are unnecessary instead. Every asset the build ships is already byte-identical in the image, so compare before copying and the shipped image needs no write at all, under any UID. What remains is a real customised build, reported once with the files that differ rather than one error per file, and never fatal. Copies stage beside the target and rename into place, so a link anywhere in the destination cannot carry an asset out of the directory, a target this process cannot write in place is still replaced, and a copy that fails leaves the asset already being served intact. Two ways this could take the app down with it are closed as well, since it all runs in a module body: resolve() reports a symlink loop as RuntimeError rather than OSError, and iterdir() needs only read permission where lstat needs search, so probing an entry can fail on its own. Also stop the cleanup pass from emptying STATIC_DIR when FRONTEND_BUILD_DIR is missing or misconfigured. It unlinked everything before copying, so a build directory that resolved to nothing deleted the packaged assets outright: 17 of 20 files in a checkout that has not run a frontend build, and in a pip install they are gone from site-packages until reinstall. Cleanup now runs only against a build that has assets. STATIC_DIR='' resolved to the working directory and deleted files there too, so an empty value now falls back like an unset one. Secret key. DATA_DIR can be a volume several instances mount, and the write was a plain truncate: two starting together both passed the emptiness check and both wrote. A barrier-synced test reproduced divergent keys and empty reads, and an empty key reaches env.py as WEBUI_SECRET_KEY='' and exits. Build the key in a temporary file and claim the name with a link, so an instance that loses adopts the winner's key instead of signing tokens the winner rejects; rename only where the filesystem has no links, and never onto a directory, which would leave live key material inside it. A write that dies partway no longer leaves a truncated key the next boot accepts, and a stop during generation no longer leaves one behind at all. Set 0640 with chmod rather than a umask, which had applied to mkdir -p and left an existing empty file its own mode. Group 0 read is what lets an OpenShift arbitrary UID reuse the key; 0644 would not rescue a volume left owned by an earlier root-owned run, since the database and DATA_DIR itself are unwritable until they are chowned. Say that, rather than dying on a bare "Permission denied", and say it for an unwritable key directory too. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_016q1DB9aa3ydNK4qJ67UMGj --- backend/open_webui/config.py | 152 ++++++++++++++++++++++++++--------- backend/start.sh | 81 +++++++++++++++++-- 2 files changed, 190 insertions(+), 43 deletions(-) diff --git a/backend/open_webui/config.py b/backend/open_webui/config.py index ee540f5598..6688e7192c 100644 --- a/backend/open_webui/config.py +++ b/backend/open_webui/config.py @@ -1,6 +1,7 @@ from __future__ import annotations import base64 +import filecmp import logging import os import shutil @@ -95,54 +96,131 @@ async def import_legacy_config_json(): # Static DIR #################################### -STATIC_DIR = Path(os.getenv('STATIC_DIR', OPEN_WEBUI_DIR / 'static')).resolve() - +_static_dir = Path(os.getenv('STATIC_DIR') or OPEN_WEBUI_DIR / 'static') try: - if STATIC_DIR.exists(): - for item in STATIC_DIR.iterdir(): - if item.is_file() or item.is_symlink(): - try: - item.unlink() - except Exception as e: - pass -except Exception as e: - pass + # resolve() reports a symlink loop as RuntimeError rather than OSError, and this + # runs at import: an exception here would stop the app from starting at all. An + # unresolved path still serves, and the containment check below simply refuses to + # write through whatever made it unresolvable. + STATIC_DIR = _static_dir.resolve() +except (OSError, RuntimeError): + STATIC_DIR = _static_dir +FRONTEND_STATIC_DIR = FRONTEND_BUILD_DIR / 'static' -for file_path in (FRONTEND_BUILD_DIR / 'static').glob('**/*'): - if file_path.is_file(): - target_path = STATIC_DIR / file_path.relative_to((FRONTEND_BUILD_DIR / 'static')) - target_path.parent.mkdir(parents=True, exist_ok=True) - try: - shutil.copyfile(file_path, target_path) - except Exception as e: - logging.error(f'An error occurred: {e}') - -# LICENSE covers copied Open WebUI logo/favicon assets. +# The frontend build is the source of truth for the bundled static assets, so they are +# refreshed here on every start. STATIC_DIR lives inside the package, which the process +# does not own whenever the container runs under a UID the image was not built for +# (OpenShift's random UID, rootless Podman's keep-id mapping), so only write what +# actually differs: an unmodified image then needs no write at all. A refresh that +# genuinely cannot be applied is reported once and never aborts the boot. +# +# LICENSE covers the Open WebUI logo/favicon assets copied below. # Do not alter, remove, obscure, or replace them except as LICENSE permits: # https://docs.openwebui.com/license. -frontend_favicon = FRONTEND_BUILD_DIR / 'static' / 'favicon.png' -if frontend_favicon.exists(): +_static_sync_errors = [] + +# Staging files for the copies below. Named with a shared prefix so the cleanup pass can +# tell one apart from a stale asset: with several uvicorn workers importing this module +# at once, deleting a peer's in-flight staging file would fail its rename. +_STATIC_TEMP_PREFIX = '.owui-static-tmp.' + +try: + _frontend_static_files = { + path.relative_to(FRONTEND_STATIC_DIR) for path in FRONTEND_STATIC_DIR.glob('**/*') if path.is_file() + } +except OSError as e: + _frontend_static_files = set() + _static_sync_errors.append(f'{FRONTEND_STATIC_DIR}: {e}') + +if not _frontend_static_files and not _static_sync_errors: + # glob swallows a permission error internally, so an unreadable build directory + # yields nothing rather than raising. A build directory that is simply absent is + # normal, though: a checkout that has not run a frontend build yet, for instance. try: - shutil.copyfile(frontend_favicon, STATIC_DIR / 'favicon.png') - except Exception as e: - logging.error(f'An error occurred: {e}') + if FRONTEND_STATIC_DIR.is_dir(): + _static_sync_errors.append(f'{FRONTEND_STATIC_DIR}: no readable files') + except OSError as e: + _static_sync_errors.append(f'{FRONTEND_STATIC_DIR}: {e}') -frontend_splash = FRONTEND_BUILD_DIR / 'static' / 'splash.png' - -if frontend_splash.exists(): +if _frontend_static_files: + # Drop what an earlier frontend build left behind, but only against a build that + # actually has assets: a missing or misconfigured FRONTEND_BUILD_DIR must not empty + # the directory the packaged assets are served from. A file the frontend still ships + # stays put so the copy below can compare against it, while a symlink standing in for + # one always goes, for the reason the copy loop gives. try: - shutil.copyfile(frontend_splash, STATIC_DIR / 'splash.png') - except Exception as e: - logging.error(f'An error occurred: {e}') + static_dir_entries = list(STATIC_DIR.iterdir()) + except FileNotFoundError: + # Nothing to clean; the copy loop creates the directory. Not worth reporting, + # since the sync that follows goes on to succeed. + static_dir_entries = [] + except OSError as e: + static_dir_entries = [] + _static_sync_errors.append(f'{STATIC_DIR}: {e}') -frontend_loader = FRONTEND_BUILD_DIR / 'static' / 'loader.js' + for item in static_dir_entries: + # Every stat here stays inside the try: iterdir() needs only read permission on + # the directory while lstat needs search, so probing an entry can fail on its own + # and must not escape a module body that is running at import. + if item.name.startswith(_STATIC_TEMP_PREFIX): + continue + try: + if item.is_symlink() or (item.is_file() and item.relative_to(STATIC_DIR) not in _frontend_static_files): + item.unlink() + except OSError as e: + _static_sync_errors.append(f'{item}: {e}') + +for relative_path in sorted(_frontend_static_files): + source_path = FRONTEND_STATIC_DIR / relative_path + target_path = STATIC_DIR / relative_path -if frontend_loader.exists(): try: - shutil.copyfile(frontend_loader, STATIC_DIR / 'loader.js') - except Exception as e: - logging.error(f'An error occurred: {e}') + if target_path.is_file() and not target_path.is_symlink(): + if filecmp.cmp(source_path, target_path, shallow=False): + continue + except OSError: + # An unreadable target is worth replacing rather than skipping. + pass + + # copyfile follows symlinks in every component of the destination, so a link + # anywhere along the way puts the asset outside STATIC_DIR. Check the directory + # this resolves into, then write beside the target and rename into place: the + # swap replaces a symlink or a file this process cannot write in place, and a + # copy that fails leaves the asset already being served untouched. + temp_path = target_path.with_name(f'{_STATIC_TEMP_PREFIX}{target_path.name}.{os.getpid()}') + try: + parent_path = target_path.parent + # resolve() reports a symlink loop as RuntimeError rather than OSError, and an + # exception loose in a module body would stop the app from importing at all. + if not parent_path.resolve().is_relative_to(STATIC_DIR): + raise OSError(f'{parent_path} resolves outside {STATIC_DIR}') + parent_path.mkdir(parents=True, exist_ok=True) + shutil.copyfile(source_path, temp_path) + os.replace(temp_path, target_path) + except (OSError, RuntimeError) as e: + _static_sync_errors.append(f'{target_path}: {e}') + try: + temp_path.unlink(missing_ok=True) + except OSError: + pass + +if _static_sync_errors: + # One line, so cap it: a STATIC_DIR that fails for every asset would otherwise + # report every one of them. + _static_sync_reported = list(dict.fromkeys(_static_sync_errors)) + _static_sync_summary = '; '.join(_static_sync_reported[:5]) + if len(_static_sync_reported) > 5: + _static_sync_summary += f'; and {len(_static_sync_reported) - 5} more' + log.warning( + 'Could not refresh the bundled static assets in %s, serving the ones already there (%s). ' + 'This only matters for a customised frontend build; to apply one, check that %s is ' + 'readable and that the running UID/GID can write %s.', + STATIC_DIR, + _static_sync_summary, + FRONTEND_STATIC_DIR, + STATIC_DIR, + ) # --- Storage Provider --- diff --git a/backend/start.sh b/backend/start.sh index 8a5ed12060..60efc8885e 100755 --- a/backend/start.sh +++ b/backend/start.sh @@ -47,22 +47,91 @@ if [[ -z "${WEBUI_SECRET_KEY:-}" && -z "${WEBUI_JWT_SECRET_KEY:-}" ]]; then # Regenerate when missing or empty, never when merely unreadable: such a key may # belong to another UID, and it also encrypts OAuth sessions and valves. - if [[ ! -s "$KEY_FILE" ]]; then + # A directory at the key path never generates: some filesystems report one as zero + # bytes, and both ln and mv would then put the key inside it, leaving live key + # material under a name nothing reads. The ladder below reports it instead. + if [[ ! -s "$KEY_FILE" && ! -d "$KEY_FILE" ]]; then echo "Generating new WEBUI_SECRET_KEY..." if ! [[ "$WEBUI_SECRET_KEY_LENGTH" =~ ^[1-9][0-9]*$ ]]; then echo "WEBUI_SECRET_KEY_LENGTH must be a positive integer." >&2 exit 1 fi - # 0640/0750: a volume remounted under a different arbitrary UID keeps group 0. + + # Report an unwritable directory here rather than letting mktemp below fail with + # a bare "Permission denied", which is what a volume left owned by an earlier + # root-owned run looks like to the non-root uid this whole change is aimed at. + KEY_DIR=$(dirname -- "$KEY_FILE") + if ! mkdir -p -- "$KEY_DIR" 2>/dev/null || [[ ! -w "$KEY_DIR" ]]; then + echo "Cannot write a WEBUI_SECRET_KEY into ${KEY_DIR} as uid $(id -u):$(id -g)." >&2 + echo "Give that uid write access to it, or set WEBUI_SECRET_KEY or" >&2 + echo "WEBUI_SECRET_KEY_FILE to somewhere writable instead." >&2 + exit 1 + fi + + # DATA_DIR can be a volume shared by several instances, so build the key in a + # temporary file next to it and put it in place as a single rename. Without + # that, two instances starting together both truncate the same path and one + # can read back an empty or half-written key; a crash mid-write leaves a + # truncated key that the next boot silently accepts. A symlink sitting on the + # path is replaced rather than written through, so a key never lands wherever + # it points. ( - umask 027 - mkdir -p -- "$(dirname -- "$KEY_FILE")" - head -c "$WEBUI_SECRET_KEY_LENGTH" /dev/random | base64 > "$KEY_FILE" + tmp_key=$(mktemp -- "${KEY_FILE}.XXXXXX") + # Drop the staging key on the way out, so a kill partway through generation does + # not leave live key material in the volume. A container stop signals PID 1 only, + # which leaves this subshell to finish and clean up through the EXIT trap; the + # signal trap covers a stop that reaches the whole process group instead. + trap 'rm -f -- "$tmp_key"' EXIT + trap 'rm -f -- "$tmp_key"; exit 143' INT TERM HUP + head -c "$WEBUI_SECRET_KEY_LENGTH" /dev/random | base64 >"$tmp_key" + # 0640: an OpenShift-style arbitrary UID keeps group 0 and can still read the + # key. Set here rather than through the umask, so neither the caller's umask + # nor the mode of an empty leftover carries over. The terminator goes before + # the mode: BSD chmod stops parsing options at the first operand, so a + # trailing "--" would be read as a file name. + chmod -- 640 "$tmp_key" + # Claim the name with a hard link, so an instance that loses the race adopts + # the winner's key instead of replacing it and signing tokens the winner + # rejects. An empty file from an earlier crash cannot be linked over, so drop + # it and retry; rename only where the filesystem has no hard links. + for _ in 1 2 3; do + # Linked it, or a peer got there first and its key is the one to use. + if ln -- "$tmp_key" "$KEY_FILE" 2>/dev/null || [[ -s "$KEY_FILE" ]]; then + break + fi + # Nothing in the way, so the link itself is unsupported here (some network + # volumes): rename below rather than retrying, which would only spin. + if [[ ! -e "$KEY_FILE" ]]; then + break + fi + rm -f -- "$KEY_FILE" 2>/dev/null || true + done + if [[ ! -s "$KEY_FILE" ]]; then + mv -- "$tmp_key" "$KEY_FILE" + fi ) fi echo "Loading WEBUI_SECRET_KEY from ${KEY_FILE}" - WEBUI_SECRET_KEY=$(cat "$KEY_FILE") + + # Read first and explain afterwards. Testing the path and then reading it leaves a + # gap another instance can act in, and the bare "cat: Permission denied" this + # replaces said nothing about what to do next. + WEBUI_SECRET_KEY=$(cat -- "$KEY_FILE" 2>/dev/null) || true + if [[ -z "$WEBUI_SECRET_KEY" ]]; then + if [[ ! -e "$KEY_FILE" && ! -L "$KEY_FILE" ]]; then + echo "WEBUI_SECRET_KEY file ${KEY_FILE} vanished while starting; retry." >&2 + elif [[ ! -f "$KEY_FILE" ]]; then + echo "WEBUI_SECRET_KEY file ${KEY_FILE} is not a regular file." >&2 + elif [[ ! -r "$KEY_FILE" ]]; then + echo "WEBUI_SECRET_KEY file ${KEY_FILE} is not readable as uid $(id -u):$(id -g)." >&2 + echo "Give that uid access to it (a volume written by an earlier root-owned run" >&2 + echo "needs chown), or set WEBUI_SECRET_KEY or WEBUI_SECRET_KEY_FILE instead." >&2 + else + echo "WEBUI_SECRET_KEY file ${KEY_FILE} holds no key." >&2 + fi + exit 1 + fi fi # ── Ollama (bundled Docker image) ────────────────────────────────────────────