From b3f50da36cb228757479f6a115a9d831bdfc5082 Mon Sep 17 00:00:00 2001 From: Sebastian Danielsson Date: Thu, 20 Aug 2026 12:50:06 +0200 Subject: [PATCH] fix: write the generated secret key where it survives, and is writable Closes #26662. `start.sh` cd's to its own directory before generating `.webui_secret_key`, so in a container the key lands in `/app/backend` -- an image layer -- rather than on the volume mounted at `/app/backend/data`. Two consequences: 1. The key does not survive a container recreate. Three recreates against a named volume produce three different keys, so every issued JWT stops validating and everyone is signed out. 2. `/app/backend` is not writable when the container runs as a non-root or arbitrary UID -- OpenShift's restricted SCC, `docker run --user`, a read-only rootfs. `set -euo pipefail` turns the failed redirect into an aborted boot: No WEBUI_SECRET_KEY environment variable set, loading from file. Generating new WEBUI_SECRET_KEY... start.sh: line 46: .webui_secret_key: Permission denied The path is now resolved in order: `WEBUI_SECRET_KEY_FILE`, then an existing non-empty `./.webui_secret_key` so installs that already have one keep it, then `DATA_DIR`. Existing keys are preserved. Two smaller behaviour changes ride along: `WEBUI_SECRET_KEY_FILE` may point at a nested path, whose parents are now created, and the `.dockerignore` entry means a custom image that had baked in a stray key stops shipping it -- that deployment generates a fresh one once and signs its users out. Regeneration triggers on missing-or-empty (`! -s`) rather than absent (`! -f`). An empty key is reachable -- an interrupted write leaves one -- and used to be self-correcting only because the file was ephemeral; on a volume it persists and is loaded as an empty string on every later boot, failing at `env.py` with "WEBUI_SECRET_KEY is not set" and pointing the operator at a variable they never set. Deliberately not `! -r`: a key we cannot read may be a good one owned by another UID or group, and `WEBUI_SECRET_KEY` is the default for `OAUTH_CLIENT_INFO_ENCRYPTION_KEY`, `OAUTH_SESSION_TOKEN_ENCRYPTION_KEY` and valve encryption, so replacing it would make data already at rest undecryptable rather than merely signing people out. `-s` needs no read permission, so that case falls through to the `cat` and aborts loudly, as it does today. For the same reason the legacy arm tests `-e`, not `-r`. The write goes to a temp file and is renamed -- only ever when the target is missing or empty, so it cannot land on a key worth keeping -- with a trap so an interrupted boot leaves no stray key material, and a symlink is resolved first so an operator's indirection is written through rather than replaced. Mode is 0640 rather than the inherited umask: the key now lives on a volume that may be remounted under a different arbitrary UID -- a PVC restored into another namespace gets a new one from `sa.scc.uid-range` -- and group 0 is the part OpenShift keeps stable. 0600 would tie the key to a UID and break exactly the recovery this is meant to enable. This does require a writable `DATA_DIR` earlier than before, at the point the key is generated. In practice the app already needs one: with `DATA_DIR` read-only, stock dies creating `cache/audio/speech`, and with every cache subdirectory pre-created it dies in chromadb, since `VECTOR_DB` defaults to `chroma` and persists there. I could not construct a stock boot that survived a read-only `DATA_DIR`, but I have not proved none exists -- a deployment that externalises the database, the storage provider and the vector DB might, and would now fail earlier and more clearly. Verified in-container as `--user 1002720000:0` against stock v0.11.0; the full matrix is in the PR body. Two limits I have not addressed: `DATA_DIR` is read from the environment only, so a value set solely in `backend/.env` is invisible here, and two replicas racing on a shared volume at first boot can still settle on different keys -- the rename makes each write atomic but elects no winner. Co-Authored-By: Claude Opus 5 (1M context) --- .dockerignore | 3 +++ backend/start.sh | 59 +++++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 59 insertions(+), 3 deletions(-) diff --git a/.dockerignore b/.dockerignore index 2b4f7b5fcf..6d87589f6c 100644 --- a/.dockerignore +++ b/.dockerignore @@ -18,3 +18,6 @@ uploads **/*.db _test backend/data/* +# a dev key from `bash backend/start.sh` would otherwise be COPYd into the +# image and then outrank the deployment's own persisted key +backend/.webui_secret_key diff --git a/backend/start.sh b/backend/start.sh index 0846273b89..849a6c7c24 100755 --- a/backend/start.sh +++ b/backend/start.sh @@ -29,7 +29,28 @@ fi # ── Secret key setup ───────────────────────────────────────────────────────── -KEY_FILE="${WEBUI_SECRET_KEY_FILE:-.webui_secret_key}" +# Where the generated key lives, in order of preference: +# 1. WEBUI_SECRET_KEY_FILE, if the operator set one +# 2. an existing, non-empty ./.webui_secret_key, so installs that already have +# one keep it -- tested with -e, not -r, so that a key we cannot read is +# still selected and fails loudly below rather than being quietly bypassed +# in favour of a freshly generated one +# 3. DATA_DIR, which is the mounted volume +# This script cd's to its own directory, so for a container (2) resolves inside +# the image rather than on the volume: the key is lost whenever the container is +# recreated, which silently invalidates every session. That directory is also +# not writable when the container runs as a non-root or arbitrary UID +# (OpenShift's restricted SCC), and `set -e` then aborts the boot outright. +# DATA_DIR is read from the environment only. A value set solely in +# backend/.env is not visible here, and those deployments keep the old +# behaviour; this does not make them worse, it just does not fix them. +if [[ -n "${WEBUI_SECRET_KEY_FILE:-}" ]]; then + KEY_FILE="$WEBUI_SECRET_KEY_FILE" +elif [[ -e .webui_secret_key && -s .webui_secret_key ]]; then + KEY_FILE=".webui_secret_key" +else + KEY_FILE="${DATA_DIR:-./data}/.webui_secret_key" +fi WEBUI_SECRET_KEY_LENGTH="${WEBUI_SECRET_KEY_LENGTH:-24}" PORT="${PORT:-8080}" HOST="${HOST:-0.0.0.0}" @@ -37,13 +58,45 @@ HOST="${HOST:-0.0.0.0}" if [[ -z "${WEBUI_SECRET_KEY:-}" && -z "${WEBUI_JWT_SECRET_KEY:-}" ]]; then echo "No WEBUI_SECRET_KEY environment variable set, loading from file." - if [[ ! -f "$KEY_FILE" ]]; then + # Regenerate when the key is missing or empty, but deliberately NOT when it is + # merely unreadable. An empty key is a reachable state -- an interrupted write + # leaves one -- and on a volume it persists, so it would otherwise be loaded + # as "" on every later boot and fail with a misleading "WEBUI_SECRET_KEY is + # not set". A key we cannot read, on the other hand, may well be a perfectly + # good one belonging to another UID or group, and it is also the default + # encryption key for OAuth client info, OAuth session tokens and valves, so + # replacing it would destroy data at rest rather than merely sign people out. + # `-s` needs no read permission, so that case falls through to the `cat` below + # and fails loudly, which is what stock does today. + if [[ ! -s "$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 - head -c "$WEBUI_SECRET_KEY_LENGTH" /dev/random | base64 > "$KEY_FILE" + # Resolve a symlink first so we replace its target, as the plain redirect + # used to, rather than swapping out the operator's link for a regular file. + if [[ -L "$KEY_FILE" ]]; then + # readlink -f exits non-zero and silent when a parent component is missing + # or the link is circular; say so rather than dying with no output. + key_link="$KEY_FILE" + KEY_FILE=$(readlink -f -- "$key_link") || { + echo "Cannot resolve the symlink at $key_link." >&2 + exit 1 + } + fi + mkdir -p -- "$(dirname -- "$KEY_FILE")" + # Write to a temporary file and rename so an interrupted write cannot leave + # a half-written key behind. Only reached when the target is missing or + # empty, so the rename never lands on a key worth keeping. + # 0640, not 0600: the key lives on a volume that may be remounted under a + # different arbitrary UID, and group 0 is the part OpenShift keeps stable. + key_tmp=$(mktemp -- "$KEY_FILE.XXXXXX") + trap 'rm -f -- "${key_tmp:-}"' EXIT + head -c "$WEBUI_SECRET_KEY_LENGTH" /dev/random | base64 > "$key_tmp" + chmod 640 -- "$key_tmp" + mv -f -- "$key_tmp" "$KEY_FILE" + trap - EXIT fi echo "Loading WEBUI_SECRET_KEY from ${KEY_FILE}"