fix(6244): the instance keeps its own SECRET_KEY in /data, and docker-compose.yml is the compose users deploy from
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 1m9s
CI & Build / Python tests (push) Successful in 2m7s
CI & Build / Build & push image (push) Successful in 30s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 55s
CI & Build / integration (push) Successful in 1m9s
CI & Build / Python tests (push) Successful in 2m7s
CI & Build / Build & push image (push) Successful in 30s
With no SECRET_KEY set, Grimoire fell back to a fixed placeholder. The quickstart compose substituted change-me-in-production, so the README's own update step (pull, up -d) recreated the container on that placeholder, and every credential sealed under the key the user started with stopped opening. - config: with neither SECRET_KEY nor SECRET_KEY_FILE, the first start writes a random key to DATA_DIR/secret_key (mode 0600, published with a hard link so concurrent starts agree), and later starts read it back. An explicit key wins and leaves the file alone. A missing SECRET_KEY_FILE or an empty key file stops the start instead of switching keys. With no data directory (a run outside the container) the process gets a temporary key and says so. - app: warns on a public placeholder key, on a temporary key, and when /data is not a mounted volume. - Dockerfile creates /data. - docker-compose.yml is now the deploy compose (published image, both volumes, nothing to set). The build-from-source compose is docker-compose.dev.yml, and docker-compose.quickstart.yml is gone. The Swarm stack file gains the /data volume and keeps its explicit key, because a Swarm can move the app away from a node-local volume. - README: a Deploy section with the compose inline, SECRET_KEY optional, "keep both volumes", and updates that keep the key. A test holds the README's copy equal to the file. - Installs that ran on the change-me-in-production placeholder get a key of their own on the new compose; Settings then asks once for saved forge tokens and the backup secret. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+18
-4
@@ -1,11 +1,12 @@
|
||||
import logging
|
||||
import os
|
||||
import time
|
||||
import traceback as tb_module
|
||||
from pathlib import Path
|
||||
|
||||
from quart import Quart, g, jsonify, make_response, request, send_from_directory
|
||||
|
||||
from grimoire.config import Config
|
||||
from grimoire.config import PLACEHOLDER_SECRET_KEYS, SECRET_KEY_FILENAME, Config
|
||||
from grimoire.routes.admin import admin_bp
|
||||
from grimoire.routes.api import api
|
||||
from grimoire.routes.auth import auth_bp
|
||||
@@ -62,10 +63,23 @@ def create_app() -> Quart:
|
||||
app.config["SESSION_COOKIE_SAMESITE"] = "Lax"
|
||||
app.config["SESSION_COOKIE_SECURE"] = Config.SECURE_COOKIES
|
||||
|
||||
if Config.SECRET_KEY == "dev-secret-change-me":
|
||||
if Config.SECRET_KEY in PLACEHOLDER_SECRET_KEYS:
|
||||
logger.warning(
|
||||
"SECRET_KEY is set to the default value — session cookies are insecure. "
|
||||
"Set SECRET_KEY or SECRET_KEY_FILE for production use."
|
||||
"SECRET_KEY is set to a public placeholder, so session cookies are insecure. "
|
||||
"Remove it to let the instance keep its own key in %s, or set a real one.",
|
||||
Config.DATA_DIR,
|
||||
)
|
||||
if Config.SECRET_KEY_SOURCE == "ephemeral":
|
||||
logger.warning(
|
||||
"No SECRET_KEY is set and %s does not exist, so this process is using a "
|
||||
"temporary key. Sessions and saved credentials will not survive a restart.",
|
||||
Config.DATA_DIR,
|
||||
)
|
||||
elif Config.SECRET_KEY_SOURCE == "data" and not os.path.ismount(Config.DATA_DIR):
|
||||
logger.warning(
|
||||
"%s is not a mounted volume. The instance's key (%s) and the model cache "
|
||||
"are lost when the container is recreated; mount a volume there.",
|
||||
Config.DATA_DIR, SECRET_KEY_FILENAME,
|
||||
)
|
||||
|
||||
if not Config.TRUST_PROXY_HEADERS:
|
||||
|
||||
+76
-3
@@ -1,4 +1,13 @@
|
||||
import os
|
||||
import secrets
|
||||
from pathlib import Path
|
||||
|
||||
# The file in the data directory that holds the instance's own SECRET_KEY.
|
||||
SECRET_KEY_FILENAME = "secret_key"
|
||||
|
||||
# Placeholder keys that sample files and older compose files have shipped. A
|
||||
# key anyone can read in this repository signs nothing safely.
|
||||
PLACEHOLDER_SECRET_KEYS = frozenset({"dev-secret-change-me", "change-me-in-production"})
|
||||
|
||||
|
||||
def _read_secret(env_var: str, secret_file_var: str, default: str) -> str:
|
||||
@@ -16,13 +25,76 @@ def _read_secret(env_var: str, secret_file_var: str, default: str) -> str:
|
||||
return default
|
||||
|
||||
|
||||
def load_or_create_secret_key(data_dir: Path) -> str | None:
|
||||
"""The instance's own SECRET_KEY, kept in `data_dir` and made on first use.
|
||||
|
||||
The first call writes a random key to `<data_dir>/secret_key`, readable by
|
||||
its owner only; every later call, from this process or another, reads that
|
||||
same key back. The file is published with a hard link, so two processes
|
||||
starting together agree on one key and a crash never leaves an empty file.
|
||||
Answers None when `data_dir` doesn't exist, which is a run outside the
|
||||
container. An empty key file raises rather than being replaced, because a
|
||||
new key would orphan everything sealed under the old one.
|
||||
"""
|
||||
if not data_dir.is_dir():
|
||||
return None
|
||||
path = data_dir / SECRET_KEY_FILENAME
|
||||
if not path.exists():
|
||||
tmp = data_dir / f".{SECRET_KEY_FILENAME}.{os.getpid()}"
|
||||
fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600)
|
||||
try:
|
||||
with os.fdopen(fd, "w") as f:
|
||||
f.write(secrets.token_hex(32) + "\n")
|
||||
f.flush()
|
||||
os.fsync(f.fileno())
|
||||
try:
|
||||
os.link(tmp, path)
|
||||
except FileExistsError:
|
||||
pass
|
||||
finally:
|
||||
tmp.unlink(missing_ok=True)
|
||||
key = path.read_text().strip()
|
||||
if not key:
|
||||
raise RuntimeError(
|
||||
f"{path} is empty. Restore it from a copy, or delete it to start "
|
||||
"with a new key (saved forge tokens and the backup secret are then "
|
||||
"entered again)."
|
||||
)
|
||||
return key
|
||||
|
||||
|
||||
def _secret_key() -> tuple[str, str]:
|
||||
"""SECRET_KEY and where it came from: "env", "data" or "ephemeral".
|
||||
|
||||
An explicit key (SECRET_KEY or SECRET_KEY_FILE) wins and leaves the data
|
||||
directory's key file alone, so unsetting it returns to that key. With no
|
||||
explicit key the instance keeps its own in DATA_DIR. With neither, the
|
||||
process gets a random key of its own that dies with it.
|
||||
"""
|
||||
explicit = _read_secret("SECRET_KEY", "SECRET_KEY_FILE", "")
|
||||
if explicit:
|
||||
return explicit, "env"
|
||||
if os.environ.get("SECRET_KEY_FILE"):
|
||||
# Falling through to another key would orphan what the named one sealed.
|
||||
raise RuntimeError(
|
||||
f"SECRET_KEY_FILE={os.environ['SECRET_KEY_FILE']} is missing or empty."
|
||||
)
|
||||
kept = load_or_create_secret_key(Path(os.environ.get("DATA_DIR", "/data")))
|
||||
if kept:
|
||||
return kept, "data"
|
||||
return secrets.token_hex(32), "ephemeral"
|
||||
|
||||
|
||||
class Config:
|
||||
DATABASE_URL: str = _read_secret(
|
||||
"DATABASE_URL",
|
||||
"DATABASE_URL_FILE",
|
||||
"postgresql+asyncpg://grimoire:grimoire@localhost:5432/grimoire",
|
||||
)
|
||||
SECRET_KEY: str = _read_secret("SECRET_KEY", "SECRET_KEY_FILE", "dev-secret-change-me")
|
||||
# Where the instance keeps its own SECRET_KEY. /data is a volume in every
|
||||
# shipped compose file, beside the model cache and diagnostics.
|
||||
DATA_DIR: str = os.environ.get("DATA_DIR", "/data")
|
||||
SECRET_KEY, SECRET_KEY_SOURCE = _secret_key()
|
||||
SECURE_COOKIES: bool = os.environ.get("SECURE_COOKIES", "").lower() in ("1", "true", "yes")
|
||||
LOG_LEVEL: str = os.environ.get("LOG_LEVEL", "INFO")
|
||||
LOG_RETENTION_DAYS: int = int(os.environ.get("LOG_RETENTION_DAYS", "90"))
|
||||
@@ -93,8 +165,9 @@ class Config:
|
||||
errors.append(f"BASE_URL='{cls.BASE_URL}' must start with http:// or https:// when OIDC is enabled")
|
||||
if cls.SECRET_KEY == "dev-secret-change-me" and cls.SECURE_COOKIES:
|
||||
errors.append(
|
||||
"SECRET_KEY is set to the insecure default but SECURE_COOKIES=true indicates "
|
||||
"a production deployment. Set SECRET_KEY or SECRET_KEY_FILE before starting."
|
||||
"SECRET_KEY is set to the public placeholder 'dev-secret-change-me' but "
|
||||
"SECURE_COOKIES=true indicates a production deployment. Remove SECRET_KEY "
|
||||
"to let the instance keep its own key in DATA_DIR, or set a real one."
|
||||
)
|
||||
if errors:
|
||||
raise ValueError("Configuration errors:\n" + "\n".join(f" - {e}" for e in errors))
|
||||
|
||||
Reference in New Issue
Block a user