mirror of
https://github.com/open-webui/open-webui.git
synced 2026-10-10 03:27:57 +00:00
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) <noreply@anthropic.com>
This commit is contained in:
parent
3e3281faa6
commit
b3f50da36c
2 changed files with 59 additions and 3 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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}"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue