From a038e144249d6770af6ae0a540c2edab739274ab Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Tue, 28 Jul 2026 20:40:33 +0800 Subject: [PATCH 1/9] fix(server): require explicit deployment settings --- CHANGELOG.md | 8 ++ docs/dev-guide/server.md | 7 +- server/.env.example | 12 +-- server/README.md | 18 ++-- server/docker-compose.yml | 4 +- server/pssm_gremlin_server/auth.py | 3 +- server/pssm_gremlin_server/config.py | 22 +++-- .../maintenance/manager.py | 5 +- server/pssm_gremlin_server/pssm_gremlin.py | 75 +++++++++------- server/run/restart_pssm_flask.sh | 89 ++++++++++++++++--- server/tests/conftest.py | 4 + server/tests/test_admin.py | 28 ++++-- server/tests/test_config.py | 51 ++++++----- server/tests/test_database_backup.py | 2 + server/tests/test_process_isolation.py | 76 +++++++++++++--- 15 files changed, 288 insertions(+), 116 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7aac6bf9..4ca65b80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ``` ## [Unreleased] +### Changed +- **GREMLIN server: explicit deployment configuration**: removed implicit + server/database path and bootstrap-admin defaults. `SERVER_DIR`, + `DB_UNIREF30`, `DB_UNIREF90`, and `ADMIN_USERS` are now mandatory; both + the restart script and direct server startup fail before touching the + database when any required setting is absent. The initial administrator + passwords are generated one-by-one and supplied transiently by the restart + script instead of being stored in the env file. ## [1.9.1] - 2026-07-28 ### Added diff --git a/docs/dev-guide/server.md b/docs/dev-guide/server.md index b4a15f8e..fe165f9a 100644 --- a/docs/dev-guide/server.md +++ b/docs/dev-guide/server.md @@ -305,10 +305,11 @@ Important environment variables (see the organized sections in | Variable | Description | |----------|-------------| | `SERVER_IMAGE` / `RUNNER_IMAGE` | Built locally in dev mode or pulled from their configured references in prod mode | -| `SERVER_DIR` | Host root shared by web and worker for uploads, task SQLite, and results; never contains the user DB | +| `SERVER_DIR` | Required host root shared by web and worker for uploads, task SQLite, and results; never contains the user DB | | `LOG_DIR` | Host directory for Gunicorn, Celery, and `maintenance.log` | -| `DB_UNIREF30` | UniRef30 HHsuite database prefix path | -| `DB_UNIREF90` | UniRef90 BLAST database prefix path | +| `DB_UNIREF30` | Required UniRef30 HHsuite database prefix path | +| `DB_UNIREF90` | Required UniRef90 BLAST database prefix path | +| `ADMIN_USERS` | Required comma-separated bootstrap-administrator usernames; the restart script generates and transiently supplies one password per account | | `AUTH_SECRET_KEY` | Fixed secret for signing auth tokens (set in production) | | `AUTH_TOKEN_MAX_AGE` | Token lifetime in seconds (default: 604800 = 7 days) | | `AUTH_DIR` | Host directory containing `users.sqlite3`; mounted only into web and maintenance and required to be outside `SERVER_DIR` | diff --git a/server/.env.example b/server/.env.example index 3b2e924c..6c3b0e16 100644 --- a/server/.env.example +++ b/server/.env.example @@ -17,7 +17,7 @@ RUNNER_IMAGE=revodesign-pssm-gremlin-non-root # ============================================================================ # Storage paths and databases — must exist on the Docker host # ============================================================================ -# Root directory for uploads, results, and the SQLite task database. +# #REQ Root directory for uploads, results, and the SQLite task database. SERVER_DIR=/srv/revodesign/server # Host directory that the Docker runner may bind from. Defaults to the parent # of SERVER_DIR; set this when SERVER_DIR is under a symlink or alternate mount. @@ -43,10 +43,10 @@ AUTH_DIR=/srv/revodesign/auth USER_DB_PATH=/var/lib/revodesign-auth/users.sqlite3 # Large read-only MSA databases mounted into the runner container. -# Prefix for the UniRef30 HHblits database (the directory containing the .cs219 +# #REQ Prefix for the UniRef30 HHblits database (the directory containing the .cs219 # and .a3m files; omit extensions). DB_UNIREF30=/srv/revodesign/databases/uniref30/UniRef30_2023_02 -# Prefix for the UniRef90 BLAST database (the directory containing the .pin/.phr +# #REQ Prefix for the UniRef90 BLAST database (the directory containing the .pin/.phr # etc. files; omit extensions). DB_UNIREF90=/srv/revodesign/databases/uniref90/uniref90 @@ -60,9 +60,11 @@ DB_UNIREF90=/srv/revodesign/databases/uniref90/uniref90 # AUTH_SECRET_KEY= # Token lifetime in seconds (default: 604800 = 7 days). # AUTH_TOKEN_MAX_AGE=604800 -# Comma-separated usernames granted access to all tasks, user management, and -# admin API routes. +# #REQ Comma-separated usernames granted access to all tasks, user management, +# and admin API routes. On an empty user database, the restart script creates +# each listed administrator and prints a distinct generated password for each. ADMIN_USERS=admin +# Do not store administrator bootstrap passwords in this file. # ============================================================================ # Registration settings — self-service registration requires email service diff --git a/server/README.md b/server/README.md index 7ad203fa..8bbfb77c 100644 --- a/server/README.md +++ b/server/README.md @@ -152,11 +152,12 @@ Fallback when `REVODESIGN_SERVER_ENV` is unset: | Variable | Purpose | | --- | --- | | `SERVER_IMAGE`, `RUNNER_IMAGE` | Image names built locally in dev mode or pulled in prod mode. Production must use full published Docker Hub references. | -| `SERVER_DIR` | Host root shared by web and worker for uploads, task SQLite, and result folders (default: `./pssm_gremlin_data`). Never store the user database here. | +| `SERVER_DIR` | Required host root shared by web and worker for uploads, task SQLite, and result folders. Never store the user database here. | | `RUNNER_HOST_ROOT` | Host root allowed for Docker runner bind mounts (default: parent of `SERVER_DIR`). | | `LOG_DIR` | Host directory for Gunicorn, Celery, and `maintenance.log`. | -| `DB_UNIREF30` | UniRef30 prefix path (default: `{SERVER_DIR}/db/uniref30/UniRef30_2022_02`). | -| `DB_UNIREF90` | UniRef90 BLAST prefix path (default: `{SERVER_DIR}/db/uniref90/uniref90`). | +| `DB_UNIREF30` | Required UniRef30 prefix path. | +| `DB_UNIREF90` | Required UniRef90 BLAST prefix path. | +| `ADMIN_USERS` | Required comma-separated administrator usernames. On an empty user database, the restart script creates each account and prints a distinct generated password. | | `AUTH_SECRET_KEY` | Fixed secret for signing auth tokens. Set in production so tokens survive restarts. | | `AUTH_TOKEN_MAX_AGE` | Token lifetime in seconds (default: 604800 = 7 days). | | `AUTH_DIR` | Host-side directory containing `users.sqlite3`; Compose mounts it only into web and maintenance. It must be outside `SERVER_DIR`. | @@ -287,11 +288,14 @@ fail validation on another. ### First run -If the user database is empty, a default admin account is created automatically: +If the user database is empty, every username in the required `ADMIN_USERS` +list is created automatically: -- Username: `admin` (customize with `DEFAULT_ADMIN_USERNAME`) -- Password: auto-generated and displayed in the restart script output. - Change immediately after first login. +- Passwords: generated separately and printed once by + `restart_pssm_flask.sh`. Change each after first login. + +Bootstrap passwords must not be stored in the env file. They are transient +first-boot values supplied by the restart script only. Set `ENABLE_REGISTER=true` and configure either SMTP or Resend to allow self-registration. Registration requires full name, affiliation, academic diff --git a/server/docker-compose.yml b/server/docker-compose.yml index 87b10142..9206bc14 100644 --- a/server/docker-compose.yml +++ b/server/docker-compose.yml @@ -30,7 +30,7 @@ x-task-env: &task-env x-web-auth-env: &web-auth-env PORT: ${PORT:-8080} GUNICORN_WORKERS: ${GUNICORN_WORKERS:-2} - ADMIN_USERS: ${ADMIN_USERS:-admin} + ADMIN_USERS: ${ADMIN_USERS} AUTH_SECRET_KEY: ${AUTH_SECRET_KEY:-} AUTH_TOKEN_MAX_AGE: ${AUTH_TOKEN_MAX_AGE:-604800} USER_DB_PATH: ${USER_DB_PATH:-/var/lib/revodesign-auth/users.sqlite3} @@ -48,7 +48,6 @@ x-web-auth-env: &web-auth-env RESEND_FROM_NAME: ${RESEND_FROM_NAME:-REvoDesign GREMLIN Server} SERVER_BASE_URL: ${SERVER_BASE_URL:-http://localhost:8080} ALLOWED_EMAIL_DOMAINS: ${ALLOWED_EMAIL_DOMAINS:-} - DEFAULT_ADMIN_PASSWORD: ${DEFAULT_ADMIN_PASSWORD:-} CLIENT_IP_HEADERS: ${CLIENT_IP_HEADERS:-X-Forwarded-For,X-Real-IP} CLIENT_COUNTRY_HEADER: ${CLIENT_COUNTRY_HEADER:-} @@ -93,6 +92,7 @@ services: PORT: ${PORT:-8080} environment: <<: [*task-env, *web-auth-env] + ADMIN_BOOTSTRAP_CREDENTIALS: ${ADMIN_BOOTSTRAP_CREDENTIALS:-} restart: unless-stopped user: ${RUNNER_UID}:${RUNNER_GID} depends_on: diff --git a/server/pssm_gremlin_server/auth.py b/server/pssm_gremlin_server/auth.py index f7d19b92..fdf5281d 100644 --- a/server/pssm_gremlin_server/auth.py +++ b/server/pssm_gremlin_server/auth.py @@ -92,7 +92,8 @@ def _get_user_db_path() -> str: """Resolve the user database path. - Uses ``USER_DB_PATH`` env var, falling back to ``{SERVER_DIR}/users.sqlite3``. + Uses ``USER_DB_PATH`` env var, falling back to the required + ``{SERVER_DIR}/users.sqlite3``. """ from_server_dir = os.environ.get("SERVER_DIR", "") default = ( diff --git a/server/pssm_gremlin_server/config.py b/server/pssm_gremlin_server/config.py index fcf6eddd..5b78a176 100644 --- a/server/pssm_gremlin_server/config.py +++ b/server/pssm_gremlin_server/config.py @@ -63,6 +63,17 @@ def env_path(var: str, default: str) -> str: return os.path.abspath(default) +def env_required_path(var: str) -> str: + return os.path.abspath(os.path.expanduser(env_required(var))) + + +def env_required(var: str) -> str: + value = os.environ.get(var, "").strip() + if not value: + raise RuntimeError(f"Required environment variable {var} is not set") + return value + + def env_csv(var: str, default: str) -> list[str]: source = os.environ.get(var) or default return [value for raw in source.split(",") if (value := raw.strip())] @@ -111,11 +122,6 @@ def ensure_directories(*paths: str) -> None: os.makedirs(path, exist_ok=True) -DEFAULT_SERVER_DIR = os.path.abspath(os.path.join(os.getcwd(), "pssm_gremlin_data")) -DEFAULT_UNIREF30_DB = os.path.join(DEFAULT_SERVER_DIR, "db", "uniref30", "UniRef30_2022_02") -DEFAULT_UNIREF90_DB = os.path.join(DEFAULT_SERVER_DIR, "db", "uniref90", "uniref90") - - @dataclass(frozen=True, slots=True) class GremlinConfig: """Centralized configuration for GREMLIN paths and runtime settings.""" @@ -134,7 +140,7 @@ class GremlinConfig: @classmethod def from_env(cls) -> GremlinConfig: - server_dir = env_path("SERVER_DIR", DEFAULT_SERVER_DIR) + server_dir = env_required_path("SERVER_DIR") return cls( server_dir=server_dir, upload_folder=os.path.join(server_dir, "upload"), @@ -142,8 +148,8 @@ def from_env(cls) -> GremlinConfig: db_path=env_path("DB_PATH", os.path.join(server_dir, "pssm_gremlin.sqlite3")), docker_image=os.environ.get("RUNNER_IMAGE", "revodesign-pssm-gremlin"), docker_user=resolve_docker_user(), - uniref30_db=env_path("DB_UNIREF30", DEFAULT_UNIREF30_DB), - uniref90_db=env_path("DB_UNIREF90", DEFAULT_UNIREF90_DB), + uniref30_db=env_required_path("DB_UNIREF30"), + uniref90_db=env_required_path("DB_UNIREF90"), nproc=env_int("NPROC", 16), maxmem=env_int("MAXMEM", 64), port=env_int("PORT", 8080), diff --git a/server/pssm_gremlin_server/maintenance/manager.py b/server/pssm_gremlin_server/maintenance/manager.py index adb98646..21bded5a 100644 --- a/server/pssm_gremlin_server/maintenance/manager.py +++ b/server/pssm_gremlin_server/maintenance/manager.py @@ -11,7 +11,7 @@ from collections.abc import Iterable from apscheduler.schedulers.blocking import BlockingScheduler -from pssm_gremlin_server.config import DEFAULT_SERVER_DIR, env_path +from pssm_gremlin_server.config import GremlinConfig, env_path from pssm_gremlin_server.maintenance.model import PeriodicTask from pssm_gremlin_server.maintenance.tasks.admin_digest import admin_digest_task from pssm_gremlin_server.maintenance.tasks.database_backup import database_backup_task @@ -29,7 +29,7 @@ def configure_logging( """Write maintenance logs to ``LOG_DIR`` while retaining console output.""" resolved_log_dir = log_dir or env_path( "LOG_DIR", - os.path.join(DEFAULT_SERVER_DIR, "logs"), + os.path.join(os.getcwd(), "pssm_gremlin_data", "logs"), ) os.makedirs(resolved_log_dir, exist_ok=True) log_path = os.path.join(resolved_log_dir, LOG_FILENAME) @@ -66,6 +66,7 @@ def build_scheduler() -> BlockingScheduler: def main() -> int: + GremlinConfig.from_env() configure_logging() build_scheduler().start() return 0 diff --git a/server/pssm_gremlin_server/pssm_gremlin.py b/server/pssm_gremlin_server/pssm_gremlin.py index 86e4ee03..fcff4e14 100644 --- a/server/pssm_gremlin_server/pssm_gremlin.py +++ b/server/pssm_gremlin_server/pssm_gremlin.py @@ -17,13 +17,11 @@ from celery.result import AsyncResult from flask import Flask, g, jsonify, request from pssm_gremlin_server.config import ( - DEFAULT_SERVER_DIR, - DEFAULT_UNIREF30_DB, - DEFAULT_UNIREF90_DB, GremlinConfig, ensure_directories as _ensure_directories, env_csv as _env_csv, env_path as _env_path, + env_required as _env_required, format_runner_identity as _format_runner_identity, resolve_docker_user as _resolve_docker_user, ) @@ -45,6 +43,15 @@ from pssm_gremlin_server.auth import _env_bool # noqa: E402 from pssm_gremlin_server.auth import _env_str # noqa: E402 +CONFIG = GremlinConfig.from_env() +_env_required("ADMIN_USERS") +_ADMIN_USERNAMES = tuple(_env_csv("ADMIN_USERS", "")) +if not _ADMIN_USERNAMES: + raise RuntimeError("Required environment variable ADMIN_USERS must contain a username") +if len(_ADMIN_USERNAMES) != len(set(_ADMIN_USERNAMES)): + raise RuntimeError("Environment variable ADMIN_USERS must not contain duplicate usernames") +ADMIN_USERS = set(_ADMIN_USERNAMES) + THIS_FILE = os.path.abspath(__file__) THIS_DIR = os.path.dirname(THIS_FILE) TEMPLATE_IMAGE_DIR = os.path.join(THIS_DIR, "templates", "images") @@ -89,38 +96,42 @@ def _add_security_headers(response): app.secret_key = app.secret_key or _token_key -CONFIG = GremlinConfig.from_env() - - -ADMIN_USERS = set(_env_csv("ADMIN_USERS", "admin")) - -# Bootstrap: if the user database is empty (first run), create a default -# admin account so the server isn't locked out. +# Bootstrap every configured admin if the user database is empty. if _user_db.user_count() == 0: - _default_admin = _env_str("DEFAULT_ADMIN_USERNAME", "admin") - _default_pass = _env_str("DEFAULT_ADMIN_PASSWORD", os.urandom(16).hex()) - try: - _created_admin = _user_db.create_user( - username=_default_admin, - email=f"{_default_admin}@revodesign.local", - password=_default_pass, - is_admin=True, - registration_status="approved", - user_status="active", - ) - _user_db.verify_email(_created_admin["id"]) - logging.warning( - "No users found — created default admin user %r with an auto-generated password. " - "Log in and change it immediately.", - _default_admin, + _credential_lines = os.environ.get("ADMIN_BOOTSTRAP_CREDENTIALS", "").splitlines() + _bootstrap_passwords = dict( + line.split("\t", 1) for line in _credential_lines if "\t" in line + ) + if set(_bootstrap_passwords) != ADMIN_USERS: + raise RuntimeError( + "Bootstrap credentials for every ADMIN_USERS entry are required for an empty " + "user database; start the deployment with restart_pssm_flask.sh" ) - except IntegrityError: - # Web and Celery can import the app concurrently on first boot. If - # another process won the bootstrap insert race, continue with it. - _created_admin = _user_db.get_user_by_username(_default_admin) - if _created_admin and not _created_admin.get("email_verified"): + for _admin_username in _ADMIN_USERNAMES: + try: + _created_admin = _user_db.create_user( + username=_admin_username, + email=f"{_admin_username}@revodesign.local", + password=_bootstrap_passwords[_admin_username], + is_admin=True, + registration_status="approved", + user_status="active", + ) _user_db.verify_email(_created_admin["id"]) - logging.info("Default admin user %r already exists after bootstrap race.", _default_admin) + logging.warning( + "No users found — created configured admin user %r. " + "Log in and change its password immediately.", + _admin_username, + ) + except IntegrityError: + # Concurrent import may win an individual bootstrap insert race. + _created_admin = _user_db.get_user_by_username(_admin_username) + if _created_admin and not _created_admin.get("email_verified"): + _user_db.verify_email(_created_admin["id"]) + logging.info( + "Configured admin user %r already exists after bootstrap race.", + _admin_username, + ) # Worker-safe task runtime. This module has no Flask/auth dependency, so the diff --git a/server/run/restart_pssm_flask.sh b/server/run/restart_pssm_flask.sh index 72138934..b24abefe 100644 --- a/server/run/restart_pssm_flask.sh +++ b/server/run/restart_pssm_flask.sh @@ -3,11 +3,11 @@ set -euo pipefail SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" -SERVER_DIR="$(cd "${SCRIPT_DIR}/.." && pwd)" -COMPOSE_FILE="${SERVER_DIR}/docker-compose.yml" -ENV_EXAMPLE_FILE="${SERVER_DIR}/.env.example" -PRIMARY_ENV_FILE="${SERVER_DIR}/.env.production" -FALLBACK_ENV_FILE="${SERVER_DIR}/.env" +SERVER_ROOT="$(cd "${SCRIPT_DIR}/.." && pwd)" +COMPOSE_FILE="${SERVER_ROOT}/docker-compose.yml" +ENV_EXAMPLE_FILE="${SERVER_ROOT}/.env.example" +PRIMARY_ENV_FILE="${SERVER_ROOT}/.env.production" +FALLBACK_ENV_FILE="${SERVER_ROOT}/.env" CALLER_DIR="$(pwd)" resolve_env_file() { @@ -66,6 +66,28 @@ require_env_file() { fi } +validate_required_settings() ( + set +u + set -a + source "${ENV_FILE}" + set +a + set -u + + local missing=() + local name="" + local value="" + for name in SERVER_DIR DB_UNIREF30 DB_UNIREF90 ADMIN_USERS; do + value="${!name:-}" + if [[ -z "${value//[[:space:]]/}" ]]; then + missing+=("${name}") + fi + done + if [[ ${#missing[@]} -gt 0 ]]; then + echo "Missing required setting(s) in ${ENV_FILE}: ${missing[*]}" >&2 + exit 1 + fi +) + if docker compose version >/dev/null 2>&1; then COMPOSE_CMD=(docker compose) elif docker-compose --version >/dev/null 2>&1; then @@ -229,6 +251,7 @@ cmd_setup() { cmd_build() { require_env_file + validate_required_settings ensure_docker_gid resolve_runner_identity @@ -241,6 +264,7 @@ cmd_build() { cmd_up() { require_env_file + validate_required_settings validate_auth_storage ensure_docker_gid resolve_runner_identity @@ -286,7 +310,9 @@ cmd_migrate_auth_db() { } cmd_restart() { - # Source env early — first boot may need a generated admin password. + require_env_file + validate_required_settings + # Source the validated deployment settings. set +u set -a source "${ENV_FILE}" @@ -299,18 +325,53 @@ cmd_restart() { _auth_dir="${AUTH_DIR:-${SCRIPT_DIR}/../auth-data}" _user_db="${_auth_dir}/users.sqlite3" + _admin_login_lines=() _legacy_user_db="${SERVER_DIR}/users.sqlite3" if [[ -f "${_legacy_user_db}" && ! -f "${_user_db}" ]]; then echo "Legacy user DB detected at ${_legacy_user_db}." >&2 echo "Run the migrate-auth-db subcommand before restarting this release." >&2 exit 1 fi - if [[ ! -f "${_user_db}" ]]; then - # First boot — generate and export the admin password. - _admin_pw="$(openssl rand -hex 16 2>/dev/null || python3 -c 'import secrets; print(secrets.token_hex(16))')" - export DEFAULT_ADMIN_PASSWORD="${_admin_pw}" + _needs_admin_bootstrap="$( + python3 - "${_user_db}" <<'PY' +import sqlite3 +import sys +from pathlib import Path + +path = Path(sys.argv[1]) +if not path.is_file(): + print("yes") +else: + try: + with sqlite3.connect(f"file:{path}?mode=ro", uri=True) as conn: + has_users = conn.execute( + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'users'" + ).fetchone() + count = conn.execute("SELECT COUNT(*) FROM users").fetchone()[0] if has_users else 0 + print("yes" if count == 0 else "no") + except sqlite3.Error: + print("no") +PY + )" + if [[ "${_needs_admin_bootstrap}" == "yes" ]]; then + _admin_bootstrap_credentials="" + IFS=',' read -r -a _configured_admins <<< "${ADMIN_USERS}" + for _admin_username in "${_configured_admins[@]}"; do + _admin_username="${_admin_username#"${_admin_username%%[![:space:]]*}"}" + _admin_username="${_admin_username%"${_admin_username##*[![:space:]]}"}" + if [[ -z "${_admin_username}" ]]; then + continue + fi + _admin_pw="$(openssl rand -hex 16 2>/dev/null || python3 -c 'import secrets; print(secrets.token_hex(16))')" + _admin_bootstrap_credentials+="${_admin_username}"$'\t'"${_admin_pw}"$'\n' + _admin_login_lines+=("Admin login — username: ${_admin_username} password: ${_admin_pw}") + done + if [[ ${#_admin_login_lines[@]} -eq 0 ]]; then + echo "ADMIN_USERS must contain at least one username." >&2 + exit 1 + fi + export ADMIN_BOOTSTRAP_CREDENTIALS="${_admin_bootstrap_credentials}" fi - cmd_down if [[ -f "${_user_db}" ]]; then @@ -345,8 +406,8 @@ PY PORT="${PORT:-8080}" echo "Deployment completed." echo "Flask app is now running at http://${DOMAIN}:${PORT}/PSSM_GREMLIN/dashboard" - if [[ -n "${_admin_pw:-}" ]]; then - echo "Admin login — username: admin password: ${_admin_pw}" + if [[ ${#_admin_login_lines[@]} -gt 0 ]]; then + printf '%s\n' "${_admin_login_lines[@]}" fi } @@ -386,7 +447,7 @@ fi echo "Using env file: ${ENV_FILE}" -pushd "${SERVER_DIR}" >/dev/null +pushd "${SERVER_ROOT}" >/dev/null case "${SUBCOMMAND}" in setup) diff --git a/server/tests/conftest.py b/server/tests/conftest.py index c2ae485e..4d203944 100644 --- a/server/tests/conftest.py +++ b/server/tests/conftest.py @@ -119,6 +119,8 @@ def _load_pssm_module(monkeypatch, tmp_path, extra_env: dict | None = None): "DB_UNIREF30": str(env_root / "uniref30"), "DB_UNIREF90": str(env_root / "uniref90"), "LOG_DIR": str(log_dir), + "ADMIN_USERS": "admin", + "ADMIN_BOOTSTRAP_CREDENTIALS": "admin\ttest-admin-password", } for key, value in base_env.items(): monkeypatch.setenv(key, value) @@ -539,6 +541,8 @@ def __post_init__(self): "DB_UNIREF30": self.miniuc["uniref30_prefix"], "DB_UNIREF90": self.miniuc["uniref90_prefix"], "LOG_DIR": str(self.log_dir), + "ADMIN_USERS": self.username, + "ADMIN_BOOTSTRAP_CREDENTIALS": f"{self.username}\t{self.password}", "NPROC": "4", "GUNICORN_WORKERS": "2", "WORKER_CONCURRENCY": "2", diff --git a/server/tests/test_admin.py b/server/tests/test_admin.py index 185f3458..8d1f221b 100644 --- a/server/tests/test_admin.py +++ b/server/tests/test_admin.py @@ -496,15 +496,27 @@ def test_admin_batch_operations_skip_self_lockout(monkeypatch, tmp_path): def test_bootstrap_admin_has_correct_statuses(monkeypatch, tmp_path): - """First-run bootstrap admin gets approved+active statuses.""" - module = _load_pssm_module(monkeypatch, tmp_path, extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}) + """Every first-run bootstrap admin gets approved+active statuses.""" + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={ + "RUNNER_UID": "1234", + "RUNNER_GID": "5678", + "ADMIN_USERS": "admin,group_admin", + "ADMIN_BOOTSTRAP_CREDENTIALS": ( + "admin\ttest-admin-password\n" + "group_admin\ttest-group-admin-password" + ), + }, + ) db = module.app.config["user_db"] - # The module's bootstrap code should have created 'admin' already - admin = db.get_user_by_username("admin") - assert admin is not None - assert admin["registration_status"] == "approved" - assert admin["user_status"] == "active" - assert admin["is_admin"] is True + for username in ("admin", "group_admin"): + admin = db.get_user_by_username(username) + assert admin is not None + assert admin["registration_status"] == "approved" + assert admin["user_status"] == "active" + assert admin["is_admin"] is True # ================================================================== diff --git a/server/tests/test_config.py b/server/tests/test_config.py index acd46272..7a0044b9 100644 --- a/server/tests/test_config.py +++ b/server/tests/test_config.py @@ -62,27 +62,38 @@ def test_pssm_config_uses_named_runner_identity(monkeypatch, tmp_path): assert module.CONFIG.docker_user == "revodesign:revodesign_appgroup" -def test_pssm_config_defaults_are_not_cluster_paths(monkeypatch, tmp_path): - monkeypatch.chdir(tmp_path) - module = _load_pssm_module( - monkeypatch, - tmp_path, - extra_env={ - "RUNNER_UID": "1234", - "RUNNER_GID": "5678", - "SERVER_DIR": None, - "DB_PATH": None, - "DB_UNIREF30": None, - "DB_UNIREF90": None, - "RUNNER_HOST_ROOT": None, - }, - ) +@pytest.mark.parametrize( + "name", + ["SERVER_DIR", "DB_UNIREF30", "DB_UNIREF90"], +) +def test_pssm_config_requires_deployment_settings_before_database_setup(monkeypatch, tmp_path, name): + with pytest.raises(RuntimeError, match=f"Required environment variable {name} is not set"): + _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={ + "RUNNER_UID": "1234", + "RUNNER_GID": "5678", + name: None, + }, + ) + + assert not (tmp_path / "pssm_env" / "users.sqlite3").exists() + + +def test_pssm_app_requires_admin_users_before_database_setup(monkeypatch, tmp_path): + with pytest.raises(RuntimeError, match="ADMIN_USERS is not set"): + _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={ + "RUNNER_UID": "1234", + "RUNNER_GID": "5678", + "ADMIN_USERS": None, + }, + ) - assert module.CONFIG.server_dir.endswith("pssm_gremlin_data") - assert not module.CONFIG.server_dir.startswith("/mnt/") - assert not module.CONFIG.uniref30_db.startswith("/mnt/") - assert not module.CONFIG.uniref90_db.startswith("/mnt/") - assert module._ROOT_MOUNT_DIRECTORY == str(module.Path(module.CONFIG.server_dir).parent) + assert not (tmp_path / "pssm_env" / "users.sqlite3").exists() def test_pssm_config_uses_runner_host_root_override(monkeypatch, tmp_path): diff --git a/server/tests/test_database_backup.py b/server/tests/test_database_backup.py index eef4e5f0..fb27afb7 100644 --- a/server/tests/test_database_backup.py +++ b/server/tests/test_database_backup.py @@ -27,6 +27,8 @@ def _configure_sources(monkeypatch, tmp_path): monkeypatch.setenv("SERVER_DIR", str(tmp_path)) monkeypatch.setenv("DB_PATH", str(task_db)) monkeypatch.setenv("USER_DB_PATH", str(user_db)) + monkeypatch.setenv("DB_UNIREF30", str(tmp_path / "uniref30")) + monkeypatch.setenv("DB_UNIREF90", str(tmp_path / "uniref90")) monkeypatch.setenv("RUNNER_UID", "1234") monkeypatch.setenv("RUNNER_GID", "5678") return task_db, user_db diff --git a/server/tests/test_process_isolation.py b/server/tests/test_process_isolation.py index c303271e..5859ba5a 100644 --- a/server/tests/test_process_isolation.py +++ b/server/tests/test_process_isolation.py @@ -15,7 +15,14 @@ from conftest import REPO_DIR -def _run_restart_script(tmp_path, *arguments, uid="1000", gid="1000"): +def _run_restart_script( + tmp_path, + *arguments, + uid="1000", + gid="1000", + admins="admin", + omit_settings=(), +): fake_bin = tmp_path / "bin" fake_bin.mkdir(parents=True) docker_log = tmp_path / "docker.log" @@ -32,21 +39,25 @@ def _run_restart_script(tmp_path, *arguments, uid="1000", gid="1000"): for path in (task_dir, auth_dir, log_dir): path.mkdir(exist_ok=True) env_file = tmp_path / "server.env" + settings = { + "SERVER_DIR": str(task_dir), + "AUTH_DIR": str(auth_dir), + "LOG_DIR": str(log_dir), + "DB_UNIREF30": str(tmp_path / "uniref30"), + "DB_UNIREF90": str(tmp_path / "uniref90"), + "ADMIN_USERS": admins, + "RUNNER_UID": uid, + "RUNNER_GID": gid, + "RUNNER_USERNAME": "revodesign", + "RUNNER_GROUP": "revodesign", + "SERVER_IMAGE": "example/revodesign-server:latest", + "RUNNER_IMAGE": "example/revodesign-runner:latest", + } env_file.write_text( "\n".join( - ( - f"SERVER_DIR={task_dir}", - f"AUTH_DIR={auth_dir}", - f"LOG_DIR={log_dir}", - f"DB_UNIREF30={tmp_path / 'uniref30'}", - f"DB_UNIREF90={tmp_path / 'uniref90'}", - f"RUNNER_UID={uid}", - f"RUNNER_GID={gid}", - "RUNNER_USERNAME=revodesign", - "RUNNER_GROUP=revodesign", - "SERVER_IMAGE=example/revodesign-server:latest", - "RUNNER_IMAGE=example/revodesign-runner:latest", - ) + f"{name}={value}" + for name, value in settings.items() + if name not in omit_settings ), encoding="utf-8", ) @@ -76,6 +87,7 @@ def _run_restart_script(tmp_path, *arguments, uid="1000", gid="1000"): def test_restart_modes_choose_build_or_pull(tmp_path): dev_result, dev_commands = _run_restart_script(tmp_path / "dev", "restart") assert dev_result.returncode == 0, dev_result.stderr + assert "Admin login — username: admin password:" in dev_result.stdout assert any("--profile runner build runner" in command for command in dev_commands) assert any("build web worker" in command for command in dev_commands) assert not any(" pull " in command for command in dev_commands) @@ -89,6 +101,23 @@ def test_restart_modes_choose_build_or_pull(tmp_path): assert pull_index < up_index +def test_restart_generates_distinct_password_for_each_configured_admin(tmp_path): + result, _commands = _run_restart_script( + tmp_path, + "restart", + admins="admin,group_admin", + ) + + assert result.returncode == 0, result.stderr + login_lines = [ + line for line in result.stdout.splitlines() if line.startswith("Admin login — ") + ] + assert [line.split()[4] for line in login_lines] == ["admin", "group_admin"] + passwords = [line.rsplit("password: ", 1)[1] for line in login_lines] + assert len(set(passwords)) == 2 + assert all(len(password) == 32 for password in passwords) + + def test_restart_mode_validation(tmp_path): identity_result, identity_commands = _run_restart_script( tmp_path / "identity", @@ -105,6 +134,23 @@ def test_restart_mode_validation(tmp_path): assert "Too many arguments" in spelling_result.stderr +@pytest.mark.parametrize( + "name", + ["SERVER_DIR", "DB_UNIREF30", "DB_UNIREF90", "ADMIN_USERS"], +) +def test_restart_rejects_missing_required_settings_before_shutdown(tmp_path, name): + result, commands = _run_restart_script( + tmp_path / name.lower(), + "restart", + omit_settings=(name,), + ) + + assert result.returncode != 0 + assert f"Missing required setting(s)" in result.stderr + assert name in result.stderr + assert not any(" down" in command or " build " in command or " pull " in command or " up " in command for command in commands) + + def test_restart_backup_includes_uncheckpointed_user_db_wal(tmp_path): auth_dir = tmp_path / "auth" auth_dir.mkdir(parents=True) @@ -165,6 +211,8 @@ def test_worker_runtime_import_has_no_auth_or_flask_side_effects(tmp_path): "PYTHONPATH": str(server_dir), "SERVER_DIR": str(task_dir), "DB_PATH": str(task_dir / "tasks.sqlite3"), + "DB_UNIREF30": str(tmp_path / "uniref30"), + "DB_UNIREF90": str(tmp_path / "uniref90"), "USER_DB_PATH": str(user_db), "RUNNER_UID": "1234", "RUNNER_GID": "5678", From 7afdcb7321866fcc4d401aa12bb3f094d70dd890 Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:00:21 +0800 Subject: [PATCH 2/9] fix(build): handle initial gist manifest upload --- Makefile | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/Makefile b/Makefile index 84d03380..daba5140 100644 --- a/Makefile +++ b/Makefile @@ -80,18 +80,14 @@ upload-gists: # JSONs for installer gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 -f REvoDesignExtrasTableRich.json jsons/REvoDesignExtrasTableRich.json # HMAC manifest — key is extracted from the uploaded source file - @python -c '\n\ -import hmac, hashlib, json, re;\n\ -src = open("src/REvoDesign/tools/package_manager.py").read();\n\ -m = re.search(r"_MANAGER_HMAC_KEY\s*=\s*bytes\.fromhex\(\"([a-f0-9]+)\"\)", src);\n\ -key = bytes.fromhex(m.group(1));\n\ -files = {"REvoDesign_PyMOL.py": "src/REvoDesign/tools/package_manager.py", "REvoDesign-PyMOL-entry.ui": "src/REvoDesign/UI/REvoDesign-PyMOL-entry.ui", "REvoDesignExtrasTableRich.json": "jsons/REvoDesignExtrasTableRich.json"};\n\ -manifest = {name: hmac.new(key, open(path, "rb").read(), "sha256").hexdigest() for name, path in files.items()};\n\ -json.dump(manifest, open("/tmp/revodesign-manifest.json", "w"), indent=2);\n\ -print("Manifest:", json.dumps(manifest, indent=2))\n\ -' - gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 -f manifest.json /tmp/revodesign-manifest.json - rm /tmp/revodesign-manifest.json + @python -c 'import hmac, json, re; from pathlib import Path; src = Path("src/REvoDesign/tools/package_manager.py").read_text(); m = re.search(r"_MANAGER_HMAC_KEY\s*=\s*bytes\.fromhex\(\"([a-f0-9]+)\"\)", src); assert m, "_MANAGER_HMAC_KEY not found"; key = bytes.fromhex(m.group(1)); files = {"REvoDesign_PyMOL.py": "src/REvoDesign/tools/package_manager.py", "REvoDesign-PyMOL-entry.ui": "src/REvoDesign/UI/REvoDesign-PyMOL-entry.ui", "REvoDesignExtrasTableRich.json": "jsons/REvoDesignExtrasTableRich.json"}; manifest = {name: hmac.new(key, Path(path).read_bytes(), "sha256").hexdigest() for name, path in files.items()}; Path("/tmp/manifest.json").write_text(json.dumps(manifest, indent=2) + "\n"); print("Manifest:", json.dumps(manifest, indent=2))' + @trap 'rm -f /tmp/manifest.json' EXIT; \ + files="$$(gh gist view c1e8bfe0fc0b9c60bf49ea04a550a044 --files)" || exit; \ + if printf '%s\n' "$$files" | grep -Fxq manifest.json; then \ + gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 -f manifest.json /tmp/manifest.json; \ + else \ + gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 --add /tmp/manifest.json; \ + fi install-pymol-plugin: cp ./src/REvoDesign/tools/package_manager.py ~/.pymol/startup/REvoDesign_PyMOL.py From 43a4c7c9778646aac024faec5f2b36b094395de2 Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Tue, 28 Jul 2026 21:54:15 +0800 Subject: [PATCH 3/9] refactor(server): use ephemeral auth signing key --- CHANGELOG.md | 4 ++++ docs/dev-guide/server.md | 5 ++--- server/.env.example | 8 +++----- server/README.md | 7 +++++-- server/docker-compose.yml | 1 - server/pssm_gremlin_server/auth.py | 6 ++---- server/pssm_gremlin_server/pssm_gremlin.py | 15 +++------------ server/tests/test_process_isolation.py | 1 + 8 files changed, 20 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4ca65b80..b2b022b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 database when any required setting is absent. The initial administrator passwords are generated one-by-one and supplied transiently by the restart script instead of being stored in the env file. +- **GREMLIN server: ephemeral signing key**: removed the configurable + `AUTH_SECRET_KEY`. Gunicorn generates one in memory per preloaded web launch; + restarting web intentionally invalidates active login, verification, and + password-reset tokens. ## [1.9.1] - 2026-07-28 ### Added diff --git a/docs/dev-guide/server.md b/docs/dev-guide/server.md index fe165f9a..ccbff572 100644 --- a/docs/dev-guide/server.md +++ b/docs/dev-guide/server.md @@ -166,8 +166,8 @@ its ``args`` to ``scheduler.add_job`` only when ``is_enabled`` is true. dispatched via Celery so the HTTP request returns immediately with a task ID for polling. - **Gunicorn `--preload`**: The WSGI application is loaded in the arbiter - before workers are forked, ensuring shared module state (especially the - `AUTH_SECRET_KEY` used for token signing) is consistent across workers. + before workers are forked, ensuring the ephemeral token-signing key is + consistent across workers for the lifetime of the web service. - **Pydantic at the API boundary**: All inbound request payloads are validated through typed Pydantic models (``schemas.py``) before reaching business logic. Response serialisation uses ``UserResponse`` to guarantee @@ -310,7 +310,6 @@ Important environment variables (see the organized sections in | `DB_UNIREF30` | Required UniRef30 HHsuite database prefix path | | `DB_UNIREF90` | Required UniRef90 BLAST database prefix path | | `ADMIN_USERS` | Required comma-separated bootstrap-administrator usernames; the restart script generates and transiently supplies one password per account | -| `AUTH_SECRET_KEY` | Fixed secret for signing auth tokens (set in production) | | `AUTH_TOKEN_MAX_AGE` | Token lifetime in seconds (default: 604800 = 7 days) | | `AUTH_DIR` | Host directory containing `users.sqlite3`; mounted only into web and maintenance and required to be outside `SERVER_DIR` | | `USER_DB_PATH` | Path through which web and maintenance see that database inside their containers (default: `/var/lib/revodesign-auth/users.sqlite3`) | diff --git a/server/.env.example b/server/.env.example index 6c3b0e16..5f6648e9 100644 --- a/server/.env.example +++ b/server/.env.example @@ -53,11 +53,9 @@ DB_UNIREF90=/srv/revodesign/databases/uniref90/uniref90 # ============================================================================ # Authentication and access control # ============================================================================ -# Secret key for signing auth tokens. #REQ in production: set a fixed, -# high-entropy value (e.g. `openssl rand -hex 32`) so tokens survive -# gunicorn/celery restarts. If left empty a random key is generated per -# process, which breaks tokens after every restart. -# AUTH_SECRET_KEY= +# The server generates an in-memory signing key on each launch. Restarting the +# web service logs users out and invalidates outstanding verification/reset +# links; no signing secret is stored in this file. # Token lifetime in seconds (default: 604800 = 7 days). # AUTH_TOKEN_MAX_AGE=604800 # #REQ Comma-separated usernames granted access to all tasks, user management, diff --git a/server/README.md b/server/README.md index 8bbfb77c..4cc19984 100644 --- a/server/README.md +++ b/server/README.md @@ -158,7 +158,6 @@ Fallback when `REVODESIGN_SERVER_ENV` is unset: | `DB_UNIREF30` | Required UniRef30 prefix path. | | `DB_UNIREF90` | Required UniRef90 BLAST prefix path. | | `ADMIN_USERS` | Required comma-separated administrator usernames. On an empty user database, the restart script creates each account and prints a distinct generated password. | -| `AUTH_SECRET_KEY` | Fixed secret for signing auth tokens. Set in production so tokens survive restarts. | | `AUTH_TOKEN_MAX_AGE` | Token lifetime in seconds (default: 604800 = 7 days). | | `AUTH_DIR` | Host-side directory containing `users.sqlite3`; Compose mounts it only into web and maintenance. It must be outside `SERVER_DIR`. | | `USER_DB_PATH` | Container-side path used by web and maintenance to open the user DB. Keep the default `/var/lib/revodesign-auth/users.sqlite3` unless the Compose mount target also changes. | @@ -286,6 +285,9 @@ generated once in the arbiter before forking. Without this, each worker independently generates its own signing key, making tokens from one worker fail validation on another. +The key is intentionally ephemeral. Restarting the web service logs users out +and invalidates outstanding verification and password-reset links. + ### First run If the user database is empty, every username in the required `ADMIN_USERS` @@ -576,7 +578,8 @@ banned users, and login throttling are maintained in ### Authentication -- Set `AUTH_SECRET_KEY` to a fixed, high-entropy value in production; otherwise tokens are lost on restart. +- Authentication signing keys are ephemeral; restarting web invalidates + existing login, verification, and password-reset tokens. - Browser page navigations use an `HttpOnly`/`SameSite=Lax` cookie; JavaScript cannot read it, so logout requires the server endpoint (`POST /api/auth/logout`). - Rate limiting: 5 login attempts/minute/IP, 3 registrations/hour/IP. diff --git a/server/docker-compose.yml b/server/docker-compose.yml index 9206bc14..6ba143a3 100644 --- a/server/docker-compose.yml +++ b/server/docker-compose.yml @@ -31,7 +31,6 @@ x-web-auth-env: &web-auth-env PORT: ${PORT:-8080} GUNICORN_WORKERS: ${GUNICORN_WORKERS:-2} ADMIN_USERS: ${ADMIN_USERS} - AUTH_SECRET_KEY: ${AUTH_SECRET_KEY:-} AUTH_TOKEN_MAX_AGE: ${AUTH_TOKEN_MAX_AGE:-604800} USER_DB_PATH: ${USER_DB_PATH:-/var/lib/revodesign-auth/users.sqlite3} ENABLE_REGISTER: ${ENABLE_REGISTER:-false} diff --git a/server/pssm_gremlin_server/auth.py b/server/pssm_gremlin_server/auth.py index fdf5281d..81e1b1d0 100644 --- a/server/pssm_gremlin_server/auth.py +++ b/server/pssm_gremlin_server/auth.py @@ -13,6 +13,7 @@ import logging import os +import secrets import smtplib import time from collections.abc import Callable @@ -392,10 +393,7 @@ def unmark_users_notified(self, user_ids: list[int]) -> None: # Token serialiser # --------------------------------------------------------------------------- -_SECRET_KEY = _env_str( - "AUTH_SECRET_KEY", - os.environ.get("SECRET_KEY", os.urandom(32).hex()), -) +_SECRET_KEY = secrets.token_hex(32) _TOKEN_MAX_AGE = _env_int("AUTH_TOKEN_MAX_AGE", 7 * 24 * 3600) # 7 days diff --git a/server/pssm_gremlin_server/pssm_gremlin.py b/server/pssm_gremlin_server/pssm_gremlin.py index fcff4e14..a080babd 100644 --- a/server/pssm_gremlin_server/pssm_gremlin.py +++ b/server/pssm_gremlin_server/pssm_gremlin.py @@ -33,15 +33,9 @@ deleted_status_from_task as _result_deleted_status, ) -# Ensure AUTH_SECRET_KEY is set *before* auth.py initialises its token -# serializer, otherwise multi-worker gunicorn generates independent signing -# keys per worker and tokens from one worker fail validation on another. -if not os.environ.get("AUTH_SECRET_KEY"): - os.environ["AUTH_SECRET_KEY"] = os.urandom(32).hex() - from pssm_gremlin_server.auth import UserDatabase # noqa: E402 +from pssm_gremlin_server.auth import _SECRET_KEY as _TOKEN_SIGNING_KEY # noqa: E402 from pssm_gremlin_server.auth import _env_bool # noqa: E402 -from pssm_gremlin_server.auth import _env_str # noqa: E402 CONFIG = GremlinConfig.from_env() _env_required("ADMIN_USERS") @@ -89,11 +83,8 @@ def _add_security_headers(response): app.config["user_db"] = _user_db ENABLE_REGISTER = _env_bool("ENABLE_REGISTER", False) -# Secrets for token signing — reuse a shared secret or generate a random one. -# In production set AUTH_SECRET_KEY to a fixed, high-entropy value so tokens -# survive process restarts. -_token_key = _env_str("AUTH_SECRET_KEY", os.urandom(32).hex()) -app.secret_key = app.secret_key or _token_key +# Gunicorn preloads this once, then forks workers with the same ephemeral key. +app.secret_key = app.secret_key or _TOKEN_SIGNING_KEY # Bootstrap every configured admin if the user database is empty. diff --git a/server/tests/test_process_isolation.py b/server/tests/test_process_isolation.py index 5859ba5a..913fcfdc 100644 --- a/server/tests/test_process_isolation.py +++ b/server/tests/test_process_isolation.py @@ -237,6 +237,7 @@ def test_worker_runtime_import_has_no_auth_or_flask_side_effects(tmp_path): def test_compose_isolates_worker_auth_and_web_docker_socket(): compose = (Path(REPO_DIR) / "server" / "docker-compose.yml").read_text(encoding="utf-8") + assert "AUTH_SECRET_KEY" not in compose task_env = compose.split("x-task-env:", 1)[1].split("x-web-auth-env:", 1)[0] web_auth_env = compose.split("x-web-auth-env:", 1)[1].split("x-maintenance-env:", 1)[0] maintenance_env = compose.split("x-maintenance-env:", 1)[1].split("x-docker-socket-access:", 1)[0] From d57417fcc0f374a2c55cbcd940e5834ff1397163 Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Wed, 29 Jul 2026 09:56:53 +0800 Subject: [PATCH 4/9] refactor(build): extract gist manifest generator --- Makefile | 2 +- tools/generate_gist_manifest.py | 52 +++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) create mode 100644 tools/generate_gist_manifest.py diff --git a/Makefile b/Makefile index daba5140..3f52fd2d 100644 --- a/Makefile +++ b/Makefile @@ -80,7 +80,7 @@ upload-gists: # JSONs for installer gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 -f REvoDesignExtrasTableRich.json jsons/REvoDesignExtrasTableRich.json # HMAC manifest — key is extracted from the uploaded source file - @python -c 'import hmac, json, re; from pathlib import Path; src = Path("src/REvoDesign/tools/package_manager.py").read_text(); m = re.search(r"_MANAGER_HMAC_KEY\s*=\s*bytes\.fromhex\(\"([a-f0-9]+)\"\)", src); assert m, "_MANAGER_HMAC_KEY not found"; key = bytes.fromhex(m.group(1)); files = {"REvoDesign_PyMOL.py": "src/REvoDesign/tools/package_manager.py", "REvoDesign-PyMOL-entry.ui": "src/REvoDesign/UI/REvoDesign-PyMOL-entry.ui", "REvoDesignExtrasTableRich.json": "jsons/REvoDesignExtrasTableRich.json"}; manifest = {name: hmac.new(key, Path(path).read_bytes(), "sha256").hexdigest() for name, path in files.items()}; Path("/tmp/manifest.json").write_text(json.dumps(manifest, indent=2) + "\n"); print("Manifest:", json.dumps(manifest, indent=2))' + @python tools/generate_gist_manifest.py /tmp/manifest.json @trap 'rm -f /tmp/manifest.json' EXIT; \ files="$$(gh gist view c1e8bfe0fc0b9c60bf49ea04a550a044 --files)" || exit; \ if printf '%s\n' "$$files" | grep -Fxq manifest.json; then \ diff --git a/tools/generate_gist_manifest.py b/tools/generate_gist_manifest.py new file mode 100644 index 00000000..643e7a08 --- /dev/null +++ b/tools/generate_gist_manifest.py @@ -0,0 +1,52 @@ +# Copyright (c) 2026 The REvoDesign Developers. +# Distributed under the terms of the GNU General Public License v3.0. +# SPDX-License-Identifier: GPL-3.0-only + +"""Generate the HMAC manifest for the PyMOL installer Gist.""" + +from __future__ import annotations + +import argparse +import hmac +import json +import re +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parents[1] +MANAGER_PATH = REPO_ROOT / "src" / "REvoDesign" / "tools" / "package_manager.py" +ASSET_PATHS = { + "REvoDesign_PyMOL.py": MANAGER_PATH, + "REvoDesign-PyMOL-entry.ui": REPO_ROOT / "src" / "REvoDesign" / "UI" / "REvoDesign-PyMOL-entry.ui", + "REvoDesignExtrasTableRich.json": REPO_ROOT / "jsons" / "REvoDesignExtrasTableRich.json", +} +HMAC_KEY_PATTERN = re.compile(r"_MANAGER_HMAC_KEY\s*=\s*bytes\.fromhex\(\s*['\"]([a-fA-F0-9]+)['\"]\s*\)") + + +def extract_hmac_key(manager_path: Path = MANAGER_PATH) -> bytes: + """Extract the public installer HMAC key without importing PyMOL dependencies.""" + match = HMAC_KEY_PATTERN.search(manager_path.read_text()) + if match is None: + raise ValueError(f"_MANAGER_HMAC_KEY not found in {manager_path}") + return bytes.fromhex(match.group(1)) + + +def generate_manifest() -> dict[str, str]: + """Return installer asset names mapped to their HMAC-SHA256 digests.""" + key = extract_hmac_key() + return {name: hmac.new(key, path.read_bytes(), "sha256").hexdigest() for name, path in ASSET_PATHS.items()} + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("output", type=Path, help="Path to the generated JSON manifest") + args = parser.parse_args(argv) + + manifest = generate_manifest() + rendered_manifest = json.dumps(manifest, indent=2) + args.output.write_text(rendered_manifest + "\n") + print(f"Manifest: {rendered_manifest}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main()) From fd905d813026ee9aad143a7393d97f01fc795722 Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:09:40 +0800 Subject: [PATCH 5/9] feat(server): rotate and cap maintenance logs --- CHANGELOG.md | 6 + docs/dev-guide/server.md | 5 +- server/.env.example | 11 ++ server/README.md | 7 +- server/docker-compose.yml | 3 + .../maintenance/manager.py | 8 +- .../maintenance/tasks/log_rotation.py | 169 ++++++++++++++++++ server/tests/test_log_rotation.py | 107 +++++++++++ server/tests/test_maintenance_manager.py | 21 +++ server/tests/test_process_isolation.py | 8 + 10 files changed, 342 insertions(+), 3 deletions(-) create mode 100644 server/pssm_gremlin_server/maintenance/tasks/log_rotation.py create mode 100644 server/tests/test_log_rotation.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b2b022b5..aabfeec4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 `AUTH_SECRET_KEY`. Gunicorn generates one in memory per preloaded web launch; restarting web intentionally invalidates active login, verification, and password-reset tokens. +- **GREMLIN server: automatic log rotation**: the maintenance scheduler can + ZIP and copy-truncate `LOG_DIR/*.log` after `ROTATE_LOG_MAX_LINENO` is + exceeded or every `ROTATE_LOG_PERIOD` days. Either unset trigger is disabled; + `MAX_LOG_SIZE` optionally caps active logs plus ZIP archives using bytes or + K/M/G/T suffixes and removes the oldest archives first. Leaving all three + settings unset disables rotation. ## [1.9.1] - 2026-07-28 ### Added diff --git a/docs/dev-guide/server.md b/docs/dev-guide/server.md index ccbff572..4ae64fd7 100644 --- a/docs/dev-guide/server.md +++ b/docs/dev-guide/server.md @@ -136,7 +136,7 @@ The server is a pip-installable package at ``server/pssm_gremlin_server/`` | ``config.py`` | Side-effect-free environment parsing and ``GremlinConfig`` | | ``maintenance/model.py`` | ``PeriodicTask`` contract for task configuration and APScheduler registration | | ``maintenance/manager.py`` | Standalone APScheduler entrypoint that imports task objects and calls their common ``register()`` interface | -| ``maintenance/tasks/`` | One self-configuring task object per module: registration digest, result cleanup, and consistent task/user SQLite backups | +| ``maintenance/tasks/`` | One self-configuring task object per module: registration digest, result cleanup, log rotation, and consistent task/user SQLite backups | | ``task_runtime.py`` | Celery instance, task DB, Docker runner, archives, and ``run_gremlin_task`` | | ``routes.py`` | All ``@app.route`` HTTP handlers — page routes, task API, auth API, admin API | | ``auth.py`` | Token serialisation, ``UserDatabase`` (SQLite/SQLAlchemy), ``login_required`` decorator, email verification, password reset | @@ -336,6 +336,9 @@ Important environment variables (see the organized sections in | `BACKUP_DB_CRON` | Five-field crontab schedule for database snapshots; unset disables the task. Recommended daily value: `0 0 * * *` | | `BACKUP_DB_PATH` | Snapshot directory inside maintenance; `/var/lib/revodesign-auth/backups` maps to `${AUTH_DIR}/backups` on the host | | `MAX_DB_BACKUP` | Maximum complete snapshot sets to retain; unset is unlimited, recommended value is `30` | +| `ROTATE_LOG_MAX_LINENO` | Optional positive line-count threshold for ZIP log rotation; unset disables this trigger | +| `ROTATE_LOG_PERIOD` | Optional positive rotation period in days (`1` daily, `7` weekly, `30` monthly); unset disables this trigger | +| `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest archives first | | `ADMIN_USERS` | Comma-separated admin usernames | | `ALLOWED_EMAIL_DOMAINS` | Comma-separated allowed email domains for self-registration (empty = all allowed). Plus-aliased addresses normalised. | | `ADMIN_NOTIFY_EMAIL` | Comma-separated recipients for new-registration digests | diff --git a/server/.env.example b/server/.env.example index 5f6648e9..402f5bce 100644 --- a/server/.env.example +++ b/server/.env.example @@ -160,6 +160,17 @@ WORKER_CONCURRENCY=2 # BACKUP_DB_PATH=/var/lib/revodesign-auth/backups # Retain the newest 30 complete snapshot sets. Leave unset for unlimited history. # MAX_DB_BACKUP=30 +# +# ZIP and copy-truncate every *.log file in LOG_DIR. Any setting below enables +# the task; leave all three unset to disable it. Line rotation occurs after the +# configured count is exceeded. Period is expressed in days: 1=daily, 7=weekly, +# 30=monthly. The maintenance process checks once per hour. +# ROTATE_LOG_MAX_LINENO=100000 +# ROTATE_LOG_PERIOD=7 +# Cap the combined size of active logs and their ZIP archives. Oldest ZIPs are +# removed first; active logs are rotated only when that is insufficient. +# Accepts bytes or K/M/G/T suffixes. Leave unset to disable the size cap. +# MAX_LOG_SIZE=1G # ============================================================================ # Web settings diff --git a/server/README.md b/server/README.md index 4cc19984..be0c3f79 100644 --- a/server/README.md +++ b/server/README.md @@ -14,7 +14,8 @@ is intentionally excluded. The server stack contains: - `web`: Flask + Gunicorn API/UI service -- `maintenance`: APScheduler process for registration digests, optional result cleanup, and database backups +- `maintenance`: APScheduler process for registration digests, result cleanup, + database backups, and log rotation - `worker`: Celery worker for background jobs - `redis`: Celery broker/backend - `runner` image: GREMLIN/PSSM execution container launched by `worker` @@ -28,6 +29,7 @@ pssm_gremlin_server/maintenance/ └── tasks/ ├── admin_digest.py # self-configuring admin_digest_task ├── database_backup.py # consistent task/user SQLite snapshots + ├── log_rotation.py # ZIP rotation and total-size pruning └── result_cleanup.py # self-configuring result_cleanup_task ``` @@ -175,6 +177,9 @@ Fallback when `REVODESIGN_SERVER_ENV` is unset: | `BACKUP_DB_CRON` | Five-field crontab schedule for database snapshots. Leave unset to disable; recommended daily schedule: `0 0 * * *`. | | `BACKUP_DB_PATH` | Snapshot directory inside the maintenance container. `/var/lib/revodesign-auth/backups` persists at `${AUTH_DIR}/backups` on the host. | | `MAX_DB_BACKUP` | Maximum complete snapshot sets to retain. Leave unset for unlimited history; recommended value: `30`. | +| `ROTATE_LOG_MAX_LINENO` | Optional line-count rotation threshold; unset disables this trigger. | +| `ROTATE_LOG_PERIOD` | Optional period in days (`1` daily, `7` weekly, `30` monthly); unset disables this trigger. | +| `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest ZIPs first. | | `ADMIN_USERS` | Comma-separated admin usernames for cross-user management. | | `ADMIN_NOTIFY_EMAIL` | Comma-separated admin email addresses for new-user registration digests (default: empty = no notification). | | `ADMIN_NEW_USER_INFORM` | Interval in minutes between new-user digest emails (default: `0` = disabled). | diff --git a/server/docker-compose.yml b/server/docker-compose.yml index 6ba143a3..4c3993e3 100644 --- a/server/docker-compose.yml +++ b/server/docker-compose.yml @@ -57,6 +57,9 @@ x-maintenance-env: &maintenance-env BACKUP_DB_CRON: ${BACKUP_DB_CRON:-} BACKUP_DB_PATH: ${BACKUP_DB_PATH:-} MAX_DB_BACKUP: ${MAX_DB_BACKUP:-} + ROTATE_LOG_MAX_LINENO: ${ROTATE_LOG_MAX_LINENO:-} + ROTATE_LOG_PERIOD: ${ROTATE_LOG_PERIOD:-} + MAX_LOG_SIZE: ${MAX_LOG_SIZE:-} x-docker-socket-access: &docker-socket-access # Docker socket group access is host-root-equivalent — the Docker API diff --git a/server/pssm_gremlin_server/maintenance/manager.py b/server/pssm_gremlin_server/maintenance/manager.py index 21bded5a..1e2a2229 100644 --- a/server/pssm_gremlin_server/maintenance/manager.py +++ b/server/pssm_gremlin_server/maintenance/manager.py @@ -15,9 +15,15 @@ from pssm_gremlin_server.maintenance.model import PeriodicTask from pssm_gremlin_server.maintenance.tasks.admin_digest import admin_digest_task from pssm_gremlin_server.maintenance.tasks.database_backup import database_backup_task +from pssm_gremlin_server.maintenance.tasks.log_rotation import log_rotation_task from pssm_gremlin_server.maintenance.tasks.result_cleanup import result_cleanup_task -PERIODIC_TASKS = (admin_digest_task, result_cleanup_task, database_backup_task) +PERIODIC_TASKS = ( + admin_digest_task, + result_cleanup_task, + database_backup_task, + log_rotation_task, +) LOG_FILENAME = "maintenance.log" diff --git a/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py b/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py new file mode 100644 index 00000000..535e65df --- /dev/null +++ b/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py @@ -0,0 +1,169 @@ +# Copyright (c) 2026 The REvoDesign Developers. +# Distributed under the terms of the GNU General Public License v3.0. +# SPDX-License-Identifier: GPL-3.0-only + +"""ZIP and copy-truncate server logs on line-count or age thresholds.""" + +from __future__ import annotations + +import logging +import os +import re +import time +import zipfile +from collections.abc import Callable +from datetime import datetime, timezone +from pathlib import Path +from typing import Any + +from pssm_gremlin_server.config import env_float, env_int, env_path +from pssm_gremlin_server.maintenance.model import PeriodicTask + +_SIZE_PATTERN = re.compile(r"(\d+(?:\.\d+)?)\s*([KMGT]?B?)?", re.IGNORECASE) +_SIZE_MULTIPLIERS = { + "": 1, + "B": 1, + "K": 1024, + "KB": 1024, + "M": 1024**2, + "MB": 1024**2, + "G": 1024**3, + "GB": 1024**3, + "T": 1024**4, + "TB": 1024**4, +} + + +def parse_log_size(value: str) -> int: + """Parse a byte count with an optional binary K/M/G/T suffix.""" + match = _SIZE_PATTERN.fullmatch(value.strip()) + if not match: + raise ValueError("MAX_LOG_SIZE must be bytes or use a K, M, G, or T suffix") + size = int(float(match.group(1)) * _SIZE_MULTIPLIERS[match.group(2).upper()]) + if size <= 0: + raise ValueError("MAX_LOG_SIZE must be positive when set") + return size + + +def _line_count(path: Path) -> int: + with path.open("rb") as handle: + return sum(1 for _line in handle) + + +def _managed_log_size(log_dir: Path) -> int: + paths = [*log_dir.glob("*.log"), *log_dir.glob("*.log.*.zip")] + return sum(path.stat().st_size for path in paths if path.is_file()) + + +def _prune_oldest_archives(log_dir: Path, max_size: int) -> None: + total = _managed_log_size(log_dir) + archives = sorted( + log_dir.glob("*.log.*.zip"), + key=lambda path: (path.stat().st_mtime, path.name), + ) + for archive in archives: + if total <= max_size: + break + size = archive.stat().st_size + archive.unlink() + total -= size + logging.info("Removed oldest rotated log %s", archive) + + +def rotate_logs( + log_dir: str, + max_lines: int | None, + period_days: float | None, + max_size: int | None, + *, + now: float | None = None, +) -> int: + """Rotate matching logs, enforce the total size cap, and return the count.""" + current_time = time.time() if now is None else now + directory = Path(log_dir) + rotated = 0 + + if max_size is not None: + _prune_oldest_archives(directory, max_size) + rotate_for_size = max_size is not None and _managed_log_size(directory) > max_size + + for log_path in sorted(directory.glob("*.log")): + marker = log_path.with_name(f".{log_path.name}.rotation") + if not marker.exists(): + marker.touch() + os.utime(marker, (current_time, current_time)) + + by_lines = max_lines is not None and _line_count(log_path) > max_lines + by_period = ( + period_days is not None + and current_time - marker.stat().st_mtime >= period_days * 86400 + ) + if log_path.stat().st_size == 0 or not (by_lines or by_period or rotate_for_size): + continue + + timestamp = datetime.fromtimestamp(current_time, timezone.utc).strftime( + "%Y%m%dT%H%M%S%fZ" + ) + archive = log_path.with_name(f"{log_path.name}.{timestamp}.zip") + with zipfile.ZipFile(archive, "x", compression=zipfile.ZIP_DEFLATED) as bundle: + bundle.write(log_path, arcname=log_path.name) + # ponytail: copy-truncate keeps existing process file descriptors valid; + # use service-specific reopen signals only if the tiny write race matters. + log_path.open("w", encoding="utf-8").close() + os.utime(marker, (current_time, current_time)) + rotated += 1 + logging.info("Rotated log %s to %s", log_path, archive) + + if max_size is not None: + _prune_oldest_archives(directory, max_size) + return rotated + + +class LogRotationTask(PeriodicTask): + """Environment-configured log rotation.""" + + id = "log-rotation" + + @property + def task_method(self) -> Callable[..., Any]: + return rotate_logs + + def configure(self) -> None: + max_lines_raw = os.environ.get("ROTATE_LOG_MAX_LINENO", "").strip() + period_raw = os.environ.get("ROTATE_LOG_PERIOD", "").strip() + max_size_raw = os.environ.get("MAX_LOG_SIZE", "").strip() + max_lines = env_int("ROTATE_LOG_MAX_LINENO", 0) if max_lines_raw else None + period_days = env_float("ROTATE_LOG_PERIOD", 0.0) if period_raw else None + max_size = parse_log_size(max_size_raw) if max_size_raw else None + log_dir = env_path( + "LOG_DIR", + os.path.join(os.getcwd(), "pssm_gremlin_data", "logs"), + ) + + self.env = { + "ROTATE_LOG_MAX_LINENO": max_lines, + "ROTATE_LOG_PERIOD": period_days, + "MAX_LOG_SIZE": max_size, + "LOG_DIR": log_dir, + } + self._args = {} + self._is_enabled = False + + if max_lines is not None and max_lines <= 0: + raise ValueError("ROTATE_LOG_MAX_LINENO must be a positive integer when set") + if period_days is not None and period_days <= 0: + raise ValueError("ROTATE_LOG_PERIOD must be a positive number when set") + if max_lines is None and period_days is None and max_size is None: + return + + self._is_enabled = True + self._args = { + "trigger": "interval", + "hours": 1, + "args": (log_dir, max_lines, period_days, max_size), + "misfire_grace_time": 3600, + "next_run_time": datetime.now(timezone.utc), + } + + +log_rotation_task = LogRotationTask() diff --git a/server/tests/test_log_rotation.py b/server/tests/test_log_rotation.py new file mode 100644 index 00000000..808f2259 --- /dev/null +++ b/server/tests/test_log_rotation.py @@ -0,0 +1,107 @@ +# Copyright (c) 2026 The REvoDesign Developers. +# Distributed under the terms of the GNU General Public License v3.0. +# SPDX-License-Identifier: GPL-3.0-only + +from __future__ import annotations + +import os +import zipfile + +import pytest + +from pssm_gremlin_server.maintenance.tasks.log_rotation import ( + log_rotation_task, + rotate_logs, +) + + +def test_line_threshold_rotates_to_zip_and_truncates_live_log(tmp_path): + log = tmp_path / "worker.log" + content = "one\ntwo\nthree\n" + log.write_text(content, encoding="utf-8") + + assert rotate_logs(str(tmp_path), 2, None, None, now=1_000_000) == 1 + + archives = list(tmp_path.glob("worker.log.*.zip")) + assert len(archives) == 1 + with zipfile.ZipFile(archives[0]) as bundle: + assert bundle.read("worker.log").decode() == content + assert log.read_text(encoding="utf-8") == "" + + +def test_period_uses_persistent_marker(tmp_path): + log = tmp_path / "maintenance.log" + log.write_text("entry\n", encoding="utf-8") + + assert rotate_logs(str(tmp_path), None, 1, None, now=1_000_000) == 0 + assert rotate_logs(str(tmp_path), None, 1, None, now=1_086_400) == 1 + + +def test_size_cap_removes_oldest_archive_before_touching_live_log(tmp_path): + log = tmp_path / "web.log" + log.write_bytes(b"x" * 300_000) + oldest = tmp_path / "web.log.old.zip" + newest = tmp_path / "web.log.new.zip" + oldest.write_bytes(b"a" * 400_000) + newest.write_bytes(b"b" * 400_000) + os.utime(oldest, (1, 1)) + os.utime(newest, (2, 2)) + + assert rotate_logs(str(tmp_path), None, None, 800_000, now=3) == 0 + + assert not oldest.exists() + assert newest.exists() + assert log.stat().st_size == 300_000 + + +def test_size_cap_rotates_live_log_when_archives_cannot_reduce_total(tmp_path): + log = tmp_path / "web.log" + log.write_bytes(os.urandom(2 * 1024**2)) + + assert rotate_logs(str(tmp_path), None, None, 1024**2, now=3) == 1 + + total = sum(path.stat().st_size for path in tmp_path.glob("web.log*")) + assert log.stat().st_size == 0 + assert total <= 1024**2 + + +def test_log_rotation_task_configures_all_triggers(monkeypatch, tmp_path): + monkeypatch.setenv("LOG_DIR", str(tmp_path)) + monkeypatch.setenv("ROTATE_LOG_MAX_LINENO", "1000") + monkeypatch.setenv("ROTATE_LOG_PERIOD", "7") + monkeypatch.setenv("MAX_LOG_SIZE", "512.5M") + + log_rotation_task.configure() + + assert log_rotation_task.is_enabled is True + assert log_rotation_task.env == { + "ROTATE_LOG_MAX_LINENO": 1000, + "ROTATE_LOG_PERIOD": 7.0, + "MAX_LOG_SIZE": int(512.5 * 1024**2), + "LOG_DIR": str(tmp_path), + } + assert log_rotation_task.args["trigger"] == "interval" + assert log_rotation_task.args["hours"] == 1 + assert log_rotation_task.args["args"] == ( + str(tmp_path), + 1000, + 7.0, + int(512.5 * 1024**2), + ) + + +@pytest.mark.parametrize( + ("name", "value", "message"), + [ + ("ROTATE_LOG_MAX_LINENO", "0", "must be a positive integer"), + ("ROTATE_LOG_PERIOD", "-1", "must be a positive number"), + ("MAX_LOG_SIZE", "0", "must be positive"), + ], +) +def test_log_rotation_rejects_non_positive_settings(monkeypatch, name, value, message): + for setting in ("ROTATE_LOG_MAX_LINENO", "ROTATE_LOG_PERIOD", "MAX_LOG_SIZE"): + monkeypatch.delenv(setting, raising=False) + monkeypatch.setenv(name, value) + + with pytest.raises(ValueError, match=message): + log_rotation_task.configure() diff --git a/server/tests/test_maintenance_manager.py b/server/tests/test_maintenance_manager.py index d10b6150..57d940a8 100644 --- a/server/tests/test_maintenance_manager.py +++ b/server/tests/test_maintenance_manager.py @@ -13,6 +13,7 @@ from pssm_gremlin_server.maintenance.tasks import admin_digest from pssm_gremlin_server.maintenance.tasks.admin_digest import admin_digest_task from pssm_gremlin_server.maintenance.tasks.database_backup import database_backup_task +from pssm_gremlin_server.maintenance.tasks.log_rotation import log_rotation_task from pssm_gremlin_server.maintenance.tasks.result_cleanup import result_cleanup_task @@ -24,6 +25,12 @@ def add_job(self, func, trigger, **kwargs): self.jobs.append((func, trigger, kwargs)) +@pytest.fixture(autouse=True) +def _clear_log_rotation_settings(monkeypatch): + for name in ("ROTATE_LOG_MAX_LINENO", "ROTATE_LOG_PERIOD", "MAX_LOG_SIZE"): + monkeypatch.delenv(name, raising=False) + + def test_configure_logging_writes_maintenance_log(monkeypatch, tmp_path): monkeypatch.setenv("LOG_DIR", str(tmp_path)) logger = logging.getLogger(f"maintenance-test-{id(tmp_path)}") @@ -139,6 +146,20 @@ def test_configure_jobs_registers_database_backup_cron(monkeypatch, tmp_path): } +def test_configure_jobs_registers_log_rotation(monkeypatch, tmp_path): + monkeypatch.setenv("LOG_DIR", str(tmp_path)) + monkeypatch.setenv("ROTATE_LOG_PERIOD", "1") + scheduler = RecordingScheduler() + + assert manager.configure_jobs(scheduler) == ["log-rotation"] + + task_func, trigger, options = scheduler.jobs[0] + assert task_func is log_rotation_task.task_method + assert trigger == "interval" + assert options["hours"] == 1 + assert options["args"] == (str(tmp_path), None, 1.0, None) + + def test_database_backup_retention_is_unlimited_when_unset(monkeypatch, tmp_path): monkeypatch.setenv("BACKUP_DB_CRON", "0 0 * * *") monkeypatch.setenv("BACKUP_DB_PATH", str(tmp_path / "backups")) diff --git a/server/tests/test_process_isolation.py b/server/tests/test_process_isolation.py index 913fcfdc..01c20e9e 100644 --- a/server/tests/test_process_isolation.py +++ b/server/tests/test_process_isolation.py @@ -250,6 +250,14 @@ def test_compose_isolates_worker_auth_and_web_docker_socket(): assert "RUNNER_HOST_ROOT" in task_env assert "RESULT_RETENTION_DAYS" not in task_env assert "RESULT_RETENTION_DAYS" not in web_auth_env + for rotation_setting in ( + "ROTATE_LOG_MAX_LINENO", + "ROTATE_LOG_PERIOD", + "MAX_LOG_SIZE", + ): + assert rotation_setting not in task_env + assert rotation_setting not in web_auth_env + assert rotation_setting in maintenance_env assert "PUBLIC_DASHBOARD" not in web_auth_env assert "RESULT_RETENTION_DAYS" in maintenance_env for backup_setting in ("BACKUP_DB_CRON", "BACKUP_DB_PATH", "MAX_DB_BACKUP"): From 8b2e7c07297a3a65a28e7db970542c9efc8155b4 Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:21:55 +0800 Subject: [PATCH 6/9] feat(server): add admin log viewer --- CHANGELOG.md | 12 ++- docs/dev-guide/server.md | 6 +- server/.env.example | 6 +- server/README.md | 7 +- .../maintenance/tasks/log_rotation.py | 81 ++++++++++++------ server/pssm_gremlin_server/routes.py | 57 ++++++++++++- .../static/css/log-viewer.css | 63 ++++++++++++++ .../static/js/log-viewer.js | 79 +++++++++++++++++ .../templates/log_viewer.html | 47 ++++++++++ .../templates/pssm_gremlin_dashboard.html | 1 + server/tests/test_admin.py | 85 +++++++++++++++++++ server/tests/test_log_rotation.py | 24 +++--- server/tests/test_maintenance_manager.py | 19 +++-- 13 files changed, 432 insertions(+), 55 deletions(-) create mode 100644 server/pssm_gremlin_server/static/css/log-viewer.css create mode 100644 server/pssm_gremlin_server/static/js/log-viewer.js create mode 100644 server/pssm_gremlin_server/templates/log_viewer.html diff --git a/CHANGELOG.md b/CHANGELOG.md index aabfeec4..7d79cf64 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,10 +32,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 password-reset tokens. - **GREMLIN server: automatic log rotation**: the maintenance scheduler can ZIP and copy-truncate `LOG_DIR/*.log` after `ROTATE_LOG_MAX_LINENO` is - exceeded or every `ROTATE_LOG_PERIOD` days. Either unset trigger is disabled; - `MAX_LOG_SIZE` optionally caps active logs plus ZIP archives using bytes or - K/M/G/T suffixes and removes the oldest archives first. Leaving all three - settings unset disables rotation. + exceeded or on the `ROTATE_LOG_PERIOD` crontab schedule. Either unset trigger + is disabled; `MAX_LOG_SIZE` optionally caps active logs plus ZIP archives + using bytes or K/M/G/T suffixes and removes the oldest archives first. + Leaving all three settings unset disables rotation. +- **GREMLIN server: admin log viewer**: a standalone page linked from the + administrator dashboard lazily streams the four active Gunicorn access, + Gunicorn error, Celery worker, and maintenance logs through fixed-name + admin-only endpoints. ## [1.9.1] - 2026-07-28 ### Added diff --git a/docs/dev-guide/server.md b/docs/dev-guide/server.md index 4ae64fd7..3b25d1c6 100644 --- a/docs/dev-guide/server.md +++ b/docs/dev-guide/server.md @@ -119,7 +119,7 @@ the admin user-control system. | Service | Base Image | Role | |---------|-----------|------| | **web** | `python:3.12-slim` | Flask + Gunicorn HTTP server. Serves the web UI and REST API. | -| **maintenance** | Same as `web` | Single APScheduler process for registration digests, optional result retention, and database backups. No HTTP port or Docker socket. | +| **maintenance** | Same as `web` | Single APScheduler process for registration digests, optional result retention, database backups, and log rotation. No HTTP port or Docker socket. | | **worker** | Same as `web` | Celery worker that receives `run_gremlin_task` jobs from Redis. | | **redis** | `redis:7.2-alpine` | Celery message broker and result backend. | | **runner** | `condaforge/mambaforge` | On-demand container that runs the PSSM/GREMLIN computation. Launched dynamically by `worker`. | @@ -232,6 +232,8 @@ cookie-only writes are rejected to avoid CSRF on browser sessions. | Method | Endpoint | Description | |--------|----------|-------------| | `GET` | `/PSSM_GREMLIN/user_control` | Admin-only user management page (web UI) | +| `GET` | `/PSSM_GREMLIN/logs` | Admin-only viewer for the four active service logs | +| `GET` | `/PSSM_GREMLIN/api/auth/admin/logs/` | Stream one fixed active service log; rotated archives are not exposed | | `GET` | `/PSSM_GREMLIN/api/auth/admin/users` | List all users (safe fields, excludes soft-deleted) | | `POST` | `/PSSM_GREMLIN/api/auth/admin/users` | Create user (pre-verified, immediately active) | | `PUT` | `/PSSM_GREMLIN/api/auth/admin/users/` | Update profile fields, email, password, role, and account statuses | @@ -337,7 +339,7 @@ Important environment variables (see the organized sections in | `BACKUP_DB_PATH` | Snapshot directory inside maintenance; `/var/lib/revodesign-auth/backups` maps to `${AUTH_DIR}/backups` on the host | | `MAX_DB_BACKUP` | Maximum complete snapshot sets to retain; unset is unlimited, recommended value is `30` | | `ROTATE_LOG_MAX_LINENO` | Optional positive line-count threshold for ZIP log rotation; unset disables this trigger | -| `ROTATE_LOG_PERIOD` | Optional positive rotation period in days (`1` daily, `7` weekly, `30` monthly); unset disables this trigger | +| `ROTATE_LOG_PERIOD` | Optional quoted five-field crontab expression for scheduled rotation (for example, `"0 0 * * *"` for daily at midnight); unset disables this trigger | | `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest archives first | | `ADMIN_USERS` | Comma-separated admin usernames | | `ALLOWED_EMAIL_DOMAINS` | Comma-separated allowed email domains for self-registration (empty = all allowed). Plus-aliased addresses normalised. | diff --git a/server/.env.example b/server/.env.example index 402f5bce..8d295b99 100644 --- a/server/.env.example +++ b/server/.env.example @@ -163,10 +163,10 @@ WORKER_CONCURRENCY=2 # # ZIP and copy-truncate every *.log file in LOG_DIR. Any setting below enables # the task; leave all three unset to disable it. Line rotation occurs after the -# configured count is exceeded. Period is expressed in days: 1=daily, 7=weekly, -# 30=monthly. The maintenance process checks once per hour. +# configured count is exceeded and is checked hourly. Scheduled rotation uses a +# quoted five-field crontab expression in TZ. # ROTATE_LOG_MAX_LINENO=100000 -# ROTATE_LOG_PERIOD=7 +# ROTATE_LOG_PERIOD="0 0 * * *" # Cap the combined size of active logs and their ZIP archives. Oldest ZIPs are # removed first; active logs are rotated only when that is insufficient. # Accepts bytes or K/M/G/T suffixes. Leave unset to disable the size cap. diff --git a/server/README.md b/server/README.md index be0c3f79..9e7b3e83 100644 --- a/server/README.md +++ b/server/README.md @@ -178,7 +178,7 @@ Fallback when `REVODESIGN_SERVER_ENV` is unset: | `BACKUP_DB_PATH` | Snapshot directory inside the maintenance container. `/var/lib/revodesign-auth/backups` persists at `${AUTH_DIR}/backups` on the host. | | `MAX_DB_BACKUP` | Maximum complete snapshot sets to retain. Leave unset for unlimited history; recommended value: `30`. | | `ROTATE_LOG_MAX_LINENO` | Optional line-count rotation threshold; unset disables this trigger. | -| `ROTATE_LOG_PERIOD` | Optional period in days (`1` daily, `7` weekly, `30` monthly); unset disables this trigger. | +| `ROTATE_LOG_PERIOD` | Optional quoted five-field crontab expression for scheduled rotation (for example, `"0 0 * * *"` for daily at midnight); unset disables this trigger. | | `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest ZIPs first. | | `ADMIN_USERS` | Comma-separated admin usernames for cross-user management. | | `ADMIN_NOTIFY_EMAIL` | Comma-separated admin email addresses for new-user registration digests (default: empty = no notification). | @@ -344,6 +344,11 @@ Admins cannot ban or delete their own account. Direct self-ban/self-delete requests return HTTP 400, and batch Disable/Delete skips the acting admin while still applying the requested action to other selected users. +The dashboard header also links administrators to `/PSSM_GREMLIN/logs`. That +standalone page loads only the selected active Gunicorn access, Gunicorn error, +Celery worker, or maintenance log and streams it incrementally. Rotated ZIP +archives and arbitrary filesystem paths are not exposed. + ### API keys (programmatic access) Long-lived API keys are available for scripted/programmatic access. Generate and revoke diff --git a/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py b/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py index 535e65df..13bba3fc 100644 --- a/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py +++ b/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py @@ -2,7 +2,7 @@ # Distributed under the terms of the GNU General Public License v3.0. # SPDX-License-Identifier: GPL-3.0-only -"""ZIP and copy-truncate server logs on line-count or age thresholds.""" +"""ZIP and copy-truncate server logs on configured triggers.""" from __future__ import annotations @@ -16,7 +16,9 @@ from pathlib import Path from typing import Any -from pssm_gremlin_server.config import env_float, env_int, env_path +from apscheduler.schedulers.base import BaseScheduler +from apscheduler.triggers.cron import CronTrigger +from pssm_gremlin_server.config import env_int, env_path from pssm_gremlin_server.maintenance.model import PeriodicTask _SIZE_PATTERN = re.compile(r"(\d+(?:\.\d+)?)\s*([KMGT]?B?)?", re.IGNORECASE) @@ -73,7 +75,7 @@ def _prune_oldest_archives(log_dir: Path, max_size: int) -> None: def rotate_logs( log_dir: str, max_lines: int | None, - period_days: float | None, + rotate_for_period: bool, max_size: int | None, *, now: float | None = None, @@ -88,17 +90,10 @@ def rotate_logs( rotate_for_size = max_size is not None and _managed_log_size(directory) > max_size for log_path in sorted(directory.glob("*.log")): - marker = log_path.with_name(f".{log_path.name}.rotation") - if not marker.exists(): - marker.touch() - os.utime(marker, (current_time, current_time)) - by_lines = max_lines is not None and _line_count(log_path) > max_lines - by_period = ( - period_days is not None - and current_time - marker.stat().st_mtime >= period_days * 86400 - ) - if log_path.stat().st_size == 0 or not (by_lines or by_period or rotate_for_size): + if log_path.stat().st_size == 0 or not ( + by_lines or rotate_for_period or rotate_for_size + ): continue timestamp = datetime.fromtimestamp(current_time, timezone.utc).strftime( @@ -110,7 +105,6 @@ def rotate_logs( # ponytail: copy-truncate keeps existing process file descriptors valid; # use service-specific reopen signals only if the tiny write race matters. log_path.open("w", encoding="utf-8").close() - os.utime(marker, (current_time, current_time)) rotated += 1 logging.info("Rotated log %s to %s", log_path, archive) @@ -133,7 +127,6 @@ def configure(self) -> None: period_raw = os.environ.get("ROTATE_LOG_PERIOD", "").strip() max_size_raw = os.environ.get("MAX_LOG_SIZE", "").strip() max_lines = env_int("ROTATE_LOG_MAX_LINENO", 0) if max_lines_raw else None - period_days = env_float("ROTATE_LOG_PERIOD", 0.0) if period_raw else None max_size = parse_log_size(max_size_raw) if max_size_raw else None log_dir = env_path( "LOG_DIR", @@ -142,28 +135,64 @@ def configure(self) -> None: self.env = { "ROTATE_LOG_MAX_LINENO": max_lines, - "ROTATE_LOG_PERIOD": period_days, + "ROTATE_LOG_PERIOD": period_raw, "MAX_LOG_SIZE": max_size, "LOG_DIR": log_dir, } self._args = {} + self._threshold_args: dict[str, Any] = {} self._is_enabled = False if max_lines is not None and max_lines <= 0: raise ValueError("ROTATE_LOG_MAX_LINENO must be a positive integer when set") - if period_days is not None and period_days <= 0: - raise ValueError("ROTATE_LOG_PERIOD must be a positive number when set") - if max_lines is None and period_days is None and max_size is None: + if max_lines is None and not period_raw and max_size is None: return + if period_raw: + self._args = { + "trigger": CronTrigger.from_crontab( + period_raw, + timezone=os.environ.get("TZ", "UTC"), + ), + "args": (log_dir, None, True, max_size), + "misfire_grace_time": 3600, + } + if max_lines is not None or max_size is not None: + self._threshold_args = { + "trigger": "interval", + "hours": 1, + "args": (log_dir, max_lines, False, max_size), + "misfire_grace_time": 3600, + "next_run_time": datetime.now(timezone.utc), + } + if not period_raw: + self._args = self._threshold_args + self._is_enabled = True - self._args = { - "trigger": "interval", - "hours": 1, - "args": (log_dir, max_lines, period_days, max_size), - "misfire_grace_time": 3600, - "next_run_time": datetime.now(timezone.utc), - } + + def register(self, scheduler: BaseScheduler) -> bool: + """Register cron rotation and hourly thresholds independently.""" + self.configure() + if not self.is_enabled: + return False + if self._threshold_args and self.env["ROTATE_LOG_PERIOD"]: + scheduler.add_job( + self.task_method, + id=f"{self.id}-thresholds", + replace_existing=True, + coalesce=True, + max_instances=self.max_instances, + **self._threshold_args, + ) + scheduler.add_job( + self.task_method, + id=self.id, + replace_existing=True, + coalesce=True, + max_instances=self.max_instances, + **self.args, + ) + return True log_rotation_task = LogRotationTask() diff --git a/server/pssm_gremlin_server/routes.py b/server/pssm_gremlin_server/routes.py index 27b0062b..323ec77f 100644 --- a/server/pssm_gremlin_server/routes.py +++ b/server/pssm_gremlin_server/routes.py @@ -17,10 +17,11 @@ import os import shutil import time +from pathlib import Path from typing import Any from celery.result import AsyncResult -from flask import current_app, g, jsonify, redirect, render_template, request, send_from_directory, url_for +from flask import Response, current_app, g, jsonify, redirect, render_template, request, send_from_directory, url_for from pssm_gremlin_server.auth import ( _DUMMY_PASSWORD_HASH, UserDatabase, @@ -144,6 +145,15 @@ def user_control_page(): return render_template("user_control.html", is_admin_user=True) +@app.route("/PSSM_GREMLIN/logs", methods=["GET"]) +@login_required +def log_viewer_page(): + """Admin-only active-log viewer.""" + if not g.current_user.get("is_admin"): + return render_template("error.html", code=403, message="Admin access required"), 403 + return render_template("log_viewer.html") + + @app.route("/favicon.ico", methods=["GET"]) def favicon(): return send_from_directory(TEMPLATE_IMAGE_DIR, "logo.ico", mimetype="image/vnd.microsoft.icon") @@ -675,6 +685,51 @@ def require_admin(): return None +_ADMIN_LOG_FILES = { + "gunicorn-access": "gunicorn-access.log", + "gunicorn-error": "gunicorn-error.log", + "celery-worker": "celery-worker.log", + "maintenance": "maintenance.log", +} + + +@app.route("/PSSM_GREMLIN/api/auth/admin/logs/", methods=["GET"]) +@login_required +def admin_stream_log(log_name: str): + """Stream one fixed, unrotated server log to an administrator.""" + if _blocked := require_admin(): + return _blocked + filename = _ADMIN_LOG_FILES.get(log_name) + if filename is None: + return jsonify({"error": "Unknown log"}), 404 + + log_dir = os.environ.get("LOG_DIR", "").strip() + if not log_dir: + return jsonify({"error": "LOG_DIR is not configured"}), 503 + log_path = Path(log_dir).resolve() / filename + if log_path.is_symlink() or not log_path.is_file(): + return jsonify({"error": "Log is not available"}), 404 + try: + handle = log_path.open("rb") + except OSError: + return jsonify({"error": "Log is not available"}), 404 + + def stream(): + with handle: + while chunk := handle.read(64 * 1024): + yield chunk + + return Response( + stream(), + mimetype="text/plain", + headers={ + "Cache-Control": "no-store", + "Content-Disposition": f'inline; filename="{filename}"', + "X-Accel-Buffering": "no", + }, + ) + + def _reject_guest(): """Return 403 if the current user is a guest account.""" if g.current_user.get("role") == "guest": diff --git a/server/pssm_gremlin_server/static/css/log-viewer.css b/server/pssm_gremlin_server/static/css/log-viewer.css new file mode 100644 index 00000000..bc90e3a3 --- /dev/null +++ b/server/pssm_gremlin_server/static/css/log-viewer.css @@ -0,0 +1,63 @@ +/* REvoDesign GREMLIN Server — active-log viewer */ + +.hero { + margin-bottom: 1rem; +} + +.log-panel { + padding: 1rem; +} + +.log-toolbar { + display: flex; + flex-wrap: wrap; + gap: 0.45rem; + align-items: center; +} + +.log-select { + border: 1px solid var(--line); + border-radius: 999px; + padding: 0.4rem 0.75rem; + background: var(--paper); + color: var(--muted); + cursor: pointer; + font: inherit; + font-size: 0.8rem; + font-weight: 600; +} + +.log-select.active { + border-color: var(--accent); + background: var(--accent); + color: #f6fbff; +} + +.log-status { + min-height: 1.2rem; + margin: 0.75rem 0 0.5rem; + font-size: 0.78rem; +} + +.log-output { + height: min(70vh, 48rem); + margin: 0; + padding: 1rem; + overflow: auto; + border: 1px solid var(--line); + border-radius: 10px; + background: #101820; + color: #d8e4ea; + font: 0.76rem/1.5 ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; + white-space: pre; +} + +html[data-theme="dark"] .log-select { + background: #17252c; + border-color: #324751; +} + +html[data-theme="dark"] .log-select.active { + background: var(--accent); + color: #f0f9ff; +} diff --git a/server/pssm_gremlin_server/static/js/log-viewer.js b/server/pssm_gremlin_server/static/js/log-viewer.js new file mode 100644 index 00000000..09f322b2 --- /dev/null +++ b/server/pssm_gremlin_server/static/js/log-viewer.js @@ -0,0 +1,79 @@ +/* REvoDesign GREMLIN Server — lazy active-log viewer */ +(function () { + "use strict"; + + var A = window.REvoDesignAuth; + var T = window.REvoDesignTheme; + var selectedLog = "gunicorn-access"; + var activeController = null; + var output = document.getElementById("logOutput"); + var status = document.getElementById("logStatus"); + var buttons = document.querySelectorAll(".log-select"); + + T.initToggle(document.getElementById("themeToggle")); + + function stopLoad() { + if (activeController) { + activeController.abort(); + activeController = null; + } + } + + async function loadSelectedLog() { + stopLoad(); + var controller = new AbortController(); + activeController = controller; + output.textContent = ""; + status.textContent = "Loading " + selectedLog + "…"; + + try { + var response = await A.authFetch( + "/PSSM_GREMLIN/api/auth/admin/logs/" + selectedLog, + { signal: controller.signal } + ); + if (!response.ok) { + var error = await response.json(); + throw new Error(error.error || "Failed to load log"); + } + if (!response.body) { + output.textContent = await response.text(); + } else { + var reader = response.body.getReader(); + var decoder = new TextDecoder(); + var byteCount = 0; + while (true) { + var result = await reader.read(); + if (result.done) break; + byteCount += result.value.byteLength; + output.appendChild(document.createTextNode(decoder.decode(result.value, { stream: true }))); + output.scrollTop = output.scrollHeight; + status.textContent = "Streaming " + selectedLog + " — " + byteCount + " bytes"; + } + output.appendChild(document.createTextNode(decoder.decode())); + } + status.textContent = "Loaded " + selectedLog; + } catch (error) { + if (error.name !== "AbortError") { + status.textContent = error.message || "Failed to load log."; + } + } finally { + if (activeController === controller) activeController = null; + } + } + + buttons.forEach(function (button) { + button.addEventListener("click", function () { + buttons.forEach(function (item) { item.classList.remove("active"); }); + button.classList.add("active"); + selectedLog = button.dataset.log; + loadSelectedLog(); + }); + }); + document.getElementById("refreshLog").addEventListener("click", loadSelectedLog); + document.getElementById("logoutBtn").addEventListener("click", function () { + A.authFetch("/PSSM_GREMLIN/api/auth/logout", { method: "POST" }) + .then(function () { A.clearToken(); window.location.href = "/PSSM_GREMLIN/login"; }) + .catch(function () { A.clearToken(); window.location.href = "/PSSM_GREMLIN/login"; }); + }); + loadSelectedLog(); +}()); diff --git a/server/pssm_gremlin_server/templates/log_viewer.html b/server/pssm_gremlin_server/templates/log_viewer.html new file mode 100644 index 00000000..8f81ed95 --- /dev/null +++ b/server/pssm_gremlin_server/templates/log_viewer.html @@ -0,0 +1,47 @@ + + + + + +REvoDesign | Server Logs + + + + + + + + + +
+
+
+
+

Server Logs

+

Read the four active, unrotated service logs.

+
+
+ + ← Dashboard + +
+
+
+ +
+
+ + + + + +
+

Loading Gunicorn access…

+

+  
+
+ + + + + diff --git a/server/pssm_gremlin_server/templates/pssm_gremlin_dashboard.html b/server/pssm_gremlin_server/templates/pssm_gremlin_dashboard.html index f8d49711..fc00213d 100644 --- a/server/pssm_gremlin_server/templates/pssm_gremlin_dashboard.html +++ b/server/pssm_gremlin_server/templates/pssm_gremlin_dashboard.html @@ -27,6 +27,7 @@

PSSM GREMLIN Task Dashboard

GitHub {% if is_admin_user %} User Control + Server Logs {% endif %} Profile diff --git a/server/tests/test_admin.py b/server/tests/test_admin.py index 8d1f221b..443a1332 100644 --- a/server/tests/test_admin.py +++ b/server/tests/test_admin.py @@ -5,6 +5,7 @@ from __future__ import annotations import json +import os import pytest from conftest import ( @@ -402,6 +403,90 @@ def test_user_control_page_requires_admin(monkeypatch, tmp_path): assert resp.status_code == 403 +def test_log_viewer_page_requires_admin(monkeypatch, tmp_path): + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + client = module.app.test_client() + + response = client.get( + "/PSSM_GREMLIN/logs", + headers=_admin_client_auth(module), + ) + assert response.status_code == 200 + assert b"Gunicorn access" in response.data + assert b"Maintenance" in response.data + assert b"/static/js/log-viewer.js" in response.data + + response = client.get( + "/PSSM_GREMLIN/dashboard", + headers=_admin_client_auth(module), + ) + assert response.status_code == 200 + assert b'href="/PSSM_GREMLIN/logs"' in response.data + + response = client.get( + "/PSSM_GREMLIN/logs", + headers=_test_client_auth(module), + ) + assert response.status_code == 403 + + +@pytest.mark.parametrize( + ("log_name", "filename"), + [ + ("gunicorn-access", "gunicorn-access.log"), + ("gunicorn-error", "gunicorn-error.log"), + ("celery-worker", "celery-worker.log"), + ("maintenance", "maintenance.log"), + ], +) +def test_admin_can_stream_fixed_server_logs(monkeypatch, tmp_path, log_name, filename): + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + content = f"{filename}: first\n{filename}: second\n".encode() + log_dir = os.environ["LOG_DIR"] + with open(os.path.join(log_dir, filename), "wb") as handle: + handle.write(content) + + response = module.app.test_client().get( + f"/PSSM_GREMLIN/api/auth/admin/logs/{log_name}", + headers=_admin_client_auth(module), + buffered=False, + ) + + assert response.status_code == 200 + assert response.is_streamed + assert b"".join(response.response) == content + assert response.headers["Cache-Control"] == "no-store" + + +def test_server_log_stream_rejects_non_admin_and_unknown_names(monkeypatch, tmp_path): + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + client = module.app.test_client() + + response = client.get( + "/PSSM_GREMLIN/api/auth/admin/logs/maintenance", + headers=_test_client_auth(module), + ) + assert response.status_code == 403 + + response = client.get( + "/PSSM_GREMLIN/api/auth/admin/logs/not-a-log", + headers=_admin_client_auth(module), + ) + assert response.status_code == 404 + + def test_user_verify_endpoint(monkeypatch, tmp_path): """GET /PSSM_GREMLIN/user_verify validates token and sets verified status.""" module = _load_pssm_module(monkeypatch, tmp_path, extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}) diff --git a/server/tests/test_log_rotation.py b/server/tests/test_log_rotation.py index 808f2259..5004b789 100644 --- a/server/tests/test_log_rotation.py +++ b/server/tests/test_log_rotation.py @@ -20,7 +20,7 @@ def test_line_threshold_rotates_to_zip_and_truncates_live_log(tmp_path): content = "one\ntwo\nthree\n" log.write_text(content, encoding="utf-8") - assert rotate_logs(str(tmp_path), 2, None, None, now=1_000_000) == 1 + assert rotate_logs(str(tmp_path), 2, False, None, now=1_000_000) == 1 archives = list(tmp_path.glob("worker.log.*.zip")) assert len(archives) == 1 @@ -29,12 +29,11 @@ def test_line_threshold_rotates_to_zip_and_truncates_live_log(tmp_path): assert log.read_text(encoding="utf-8") == "" -def test_period_uses_persistent_marker(tmp_path): +def test_scheduled_period_rotates_nonempty_logs(tmp_path): log = tmp_path / "maintenance.log" log.write_text("entry\n", encoding="utf-8") - assert rotate_logs(str(tmp_path), None, 1, None, now=1_000_000) == 0 - assert rotate_logs(str(tmp_path), None, 1, None, now=1_086_400) == 1 + assert rotate_logs(str(tmp_path), None, True, None, now=1_000_000) == 1 def test_size_cap_removes_oldest_archive_before_touching_live_log(tmp_path): @@ -47,7 +46,7 @@ def test_size_cap_removes_oldest_archive_before_touching_live_log(tmp_path): os.utime(oldest, (1, 1)) os.utime(newest, (2, 2)) - assert rotate_logs(str(tmp_path), None, None, 800_000, now=3) == 0 + assert rotate_logs(str(tmp_path), None, False, 800_000, now=3) == 0 assert not oldest.exists() assert newest.exists() @@ -58,7 +57,7 @@ def test_size_cap_rotates_live_log_when_archives_cannot_reduce_total(tmp_path): log = tmp_path / "web.log" log.write_bytes(os.urandom(2 * 1024**2)) - assert rotate_logs(str(tmp_path), None, None, 1024**2, now=3) == 1 + assert rotate_logs(str(tmp_path), None, False, 1024**2, now=3) == 1 total = sum(path.stat().st_size for path in tmp_path.glob("web.log*")) assert log.stat().st_size == 0 @@ -68,7 +67,7 @@ def test_size_cap_rotates_live_log_when_archives_cannot_reduce_total(tmp_path): def test_log_rotation_task_configures_all_triggers(monkeypatch, tmp_path): monkeypatch.setenv("LOG_DIR", str(tmp_path)) monkeypatch.setenv("ROTATE_LOG_MAX_LINENO", "1000") - monkeypatch.setenv("ROTATE_LOG_PERIOD", "7") + monkeypatch.setenv("ROTATE_LOG_PERIOD", "0 0 * * *") monkeypatch.setenv("MAX_LOG_SIZE", "512.5M") log_rotation_task.configure() @@ -76,16 +75,15 @@ def test_log_rotation_task_configures_all_triggers(monkeypatch, tmp_path): assert log_rotation_task.is_enabled is True assert log_rotation_task.env == { "ROTATE_LOG_MAX_LINENO": 1000, - "ROTATE_LOG_PERIOD": 7.0, + "ROTATE_LOG_PERIOD": "0 0 * * *", "MAX_LOG_SIZE": int(512.5 * 1024**2), "LOG_DIR": str(tmp_path), } - assert log_rotation_task.args["trigger"] == "interval" - assert log_rotation_task.args["hours"] == 1 + assert log_rotation_task.args["trigger"].timezone is not None assert log_rotation_task.args["args"] == ( str(tmp_path), - 1000, - 7.0, + None, + True, int(512.5 * 1024**2), ) @@ -94,7 +92,7 @@ def test_log_rotation_task_configures_all_triggers(monkeypatch, tmp_path): ("name", "value", "message"), [ ("ROTATE_LOG_MAX_LINENO", "0", "must be a positive integer"), - ("ROTATE_LOG_PERIOD", "-1", "must be a positive number"), + ("ROTATE_LOG_PERIOD", "not a cron", "Wrong number of fields"), ("MAX_LOG_SIZE", "0", "must be positive"), ], ) diff --git a/server/tests/test_maintenance_manager.py b/server/tests/test_maintenance_manager.py index 57d940a8..f798e139 100644 --- a/server/tests/test_maintenance_manager.py +++ b/server/tests/test_maintenance_manager.py @@ -148,16 +148,25 @@ def test_configure_jobs_registers_database_backup_cron(monkeypatch, tmp_path): def test_configure_jobs_registers_log_rotation(monkeypatch, tmp_path): monkeypatch.setenv("LOG_DIR", str(tmp_path)) - monkeypatch.setenv("ROTATE_LOG_PERIOD", "1") + monkeypatch.setenv("ROTATE_LOG_MAX_LINENO", "1000") + monkeypatch.setenv("ROTATE_LOG_PERIOD", "0 0 * * *") scheduler = RecordingScheduler() assert manager.configure_jobs(scheduler) == ["log-rotation"] - task_func, trigger, options = scheduler.jobs[0] + assert len(scheduler.jobs) == 2 + threshold_func, threshold_trigger, threshold_options = scheduler.jobs[0] + assert threshold_func is log_rotation_task.task_method + assert threshold_trigger == "interval" + assert threshold_options["hours"] == 1 + assert threshold_options["args"] == (str(tmp_path), 1000, False, None) + assert threshold_options["id"] == "log-rotation-thresholds" + + task_func, trigger, options = scheduler.jobs[1] assert task_func is log_rotation_task.task_method - assert trigger == "interval" - assert options["hours"] == 1 - assert options["args"] == (str(tmp_path), None, 1.0, None) + assert trigger is log_rotation_task.args["trigger"] + assert options["args"] == (str(tmp_path), None, True, None) + assert options["id"] == log_rotation_task.id def test_database_backup_retention_is_unlimited_when_unset(monkeypatch, tmp_path): From 85ecfd93c1b6313ba744b4bd56d83ecf6bed584c Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:28:54 +0800 Subject: [PATCH 7/9] feat(server): expose rotated logs to admins --- CHANGELOG.md | 3 +- docs/dev-guide/server.md | 4 +- server/README.md | 5 +- server/pssm_gremlin_server/routes.py | 79 ++++++++++++++++++- .../static/css/log-viewer.css | 52 ++++++++++++ .../static/js/log-viewer.js | 75 ++++++++++++++++++ .../templates/log_viewer.html | 9 +++ server/tests/test_admin.py | 64 +++++++++++++++ 8 files changed, 286 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d79cf64..dc8fad5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,7 +39,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **GREMLIN server: admin log viewer**: a standalone page linked from the administrator dashboard lazily streams the four active Gunicorn access, Gunicorn error, Celery worker, and maintenance logs through fixed-name - admin-only endpoints. + admin-only endpoints. Rotated ZIPs are grouped in a lazy file tree and can + be downloaded individually. ## [1.9.1] - 2026-07-28 ### Added diff --git a/docs/dev-guide/server.md b/docs/dev-guide/server.md index 3b25d1c6..2a04cc4b 100644 --- a/docs/dev-guide/server.md +++ b/docs/dev-guide/server.md @@ -233,7 +233,9 @@ cookie-only writes are rejected to avoid CSRF on browser sessions. |--------|----------|-------------| | `GET` | `/PSSM_GREMLIN/user_control` | Admin-only user management page (web UI) | | `GET` | `/PSSM_GREMLIN/logs` | Admin-only viewer for the four active service logs | -| `GET` | `/PSSM_GREMLIN/api/auth/admin/logs/` | Stream one fixed active service log; rotated archives are not exposed | +| `GET` | `/PSSM_GREMLIN/api/auth/admin/logs/` | Stream one fixed active service log | +| `GET` | `/PSSM_GREMLIN/api/auth/admin/logs/archives` | List rotated ZIP archives grouped under the four fixed service logs | +| `GET` | `/PSSM_GREMLIN/api/auth/admin/logs/archives/` | Download one managed rotated-log ZIP | | `GET` | `/PSSM_GREMLIN/api/auth/admin/users` | List all users (safe fields, excludes soft-deleted) | | `POST` | `/PSSM_GREMLIN/api/auth/admin/users` | Create user (pre-verified, immediately active) | | `PUT` | `/PSSM_GREMLIN/api/auth/admin/users/` | Update profile fields, email, password, role, and account statuses | diff --git a/server/README.md b/server/README.md index 9e7b3e83..b6ebedc7 100644 --- a/server/README.md +++ b/server/README.md @@ -346,8 +346,9 @@ still applying the requested action to other selected users. The dashboard header also links administrators to `/PSSM_GREMLIN/logs`. That standalone page loads only the selected active Gunicorn access, Gunicorn error, -Celery worker, or maintenance log and streams it incrementally. Rotated ZIP -archives and arbitrary filesystem paths are not exposed. +Celery worker, or maintenance log and streams it incrementally. Its lazy +file tree lists rotated ZIP archives under those same four logs and permits +individual downloads; arbitrary filesystem paths are not exposed. ### API keys (programmatic access) diff --git a/server/pssm_gremlin_server/routes.py b/server/pssm_gremlin_server/routes.py index 323ec77f..bb1d8fe4 100644 --- a/server/pssm_gremlin_server/routes.py +++ b/server/pssm_gremlin_server/routes.py @@ -21,7 +21,7 @@ from typing import Any from celery.result import AsyncResult -from flask import Response, current_app, g, jsonify, redirect, render_template, request, send_from_directory, url_for +from flask import Response, current_app, g, jsonify, redirect, render_template, request, send_file, send_from_directory, url_for from pssm_gremlin_server.auth import ( _DUMMY_PASSWORD_HASH, UserDatabase, @@ -693,6 +693,83 @@ def require_admin(): } +def _admin_log_archive_path(archive_name: str) -> Path | None: + """Resolve one managed rotated-log ZIP without allowing arbitrary paths.""" + if not any( + archive_name.startswith(f"{filename}.") and archive_name.endswith(".zip") + for filename in _ADMIN_LOG_FILES.values() + ): + return None + log_dir = os.environ.get("LOG_DIR", "").strip() + if not log_dir: + return None + archive_path = Path(log_dir).resolve() / archive_name + if archive_path.is_symlink() or not archive_path.is_file(): + return None + return archive_path + + +@app.route("/PSSM_GREMLIN/api/auth/admin/logs/archives", methods=["GET"]) +@login_required +def admin_log_archives(): + """List managed rotated-log ZIPs grouped by active log.""" + if _blocked := require_admin(): + return _blocked + log_dir = os.environ.get("LOG_DIR", "").strip() + if not log_dir: + return jsonify({"error": "LOG_DIR is not configured"}), 503 + + directory = Path(log_dir).resolve() + groups = [] + for log_name, filename in _ADMIN_LOG_FILES.items(): + archives = [] + for archive in directory.glob(f"{filename}.*.zip"): + if archive.is_symlink() or not archive.is_file(): + continue + try: + stat = archive.stat() + except OSError: + continue + archives.append( + { + "filename": archive.name, + "size": stat.st_size, + "modified_at": stat.st_mtime, + } + ) + archives.sort(key=lambda item: (item["modified_at"], item["filename"]), reverse=True) + groups.append( + { + "id": log_name, + "filename": filename, + "archives": archives, + } + ) + return jsonify({"logs": groups}) + + +@app.route( + "/PSSM_GREMLIN/api/auth/admin/logs/archives/", + methods=["GET"], +) +@login_required +def admin_download_log_archive(archive_name: str): + """Download one managed rotated-log ZIP.""" + if _blocked := require_admin(): + return _blocked + archive_path = _admin_log_archive_path(archive_name) + if archive_path is None: + return jsonify({"error": "Log archive is not available"}), 404 + response = send_file( + archive_path, + as_attachment=True, + download_name=archive_path.name, + mimetype="application/zip", + ) + response.headers["Cache-Control"] = "no-store" + return response + + @app.route("/PSSM_GREMLIN/api/auth/admin/logs/", methods=["GET"]) @login_required def admin_stream_log(log_name: str): diff --git a/server/pssm_gremlin_server/static/css/log-viewer.css b/server/pssm_gremlin_server/static/css/log-viewer.css index bc90e3a3..6b3ddaaf 100644 --- a/server/pssm_gremlin_server/static/css/log-viewer.css +++ b/server/pssm_gremlin_server/static/css/log-viewer.css @@ -61,3 +61,55 @@ html[data-theme="dark"] .log-select.active { background: var(--accent); color: #f0f9ff; } + +.archive-panel { + margin-top: 1rem; + padding: 1rem; +} + +.archive-panel > summary, +.archive-branch > summary { + cursor: pointer; + font-weight: 600; +} + +.archive-actions { + display: flex; + align-items: center; + justify-content: space-between; + gap: 1rem; + margin: 0.75rem 0; +} + +.archive-actions p { + margin: 0; +} + +.archive-tree { + display: grid; + gap: 0.5rem; +} + +.archive-branch { + border-left: 2px solid var(--line); + padding: 0.4rem 0 0.4rem 0.8rem; +} + +.archive-branch ul { + margin: 0.5rem 0 0; + padding-left: 1.25rem; +} + +.archive-branch li { + margin: 0.35rem 0; +} + +.archive-branch a { + overflow-wrap: anywhere; +} + +.archive-meta { + display: block; + color: var(--muted); + font-size: 0.75rem; +} diff --git a/server/pssm_gremlin_server/static/js/log-viewer.js b/server/pssm_gremlin_server/static/js/log-viewer.js index 09f322b2..f8b0ca2d 100644 --- a/server/pssm_gremlin_server/static/js/log-viewer.js +++ b/server/pssm_gremlin_server/static/js/log-viewer.js @@ -9,6 +9,10 @@ var output = document.getElementById("logOutput"); var status = document.getElementById("logStatus"); var buttons = document.querySelectorAll(".log-select"); + var archivePanel = document.getElementById("archivePanel"); + var archiveTree = document.getElementById("archiveTree"); + var archiveStatus = document.getElementById("archiveStatus"); + var archivesLoaded = false; T.initToggle(document.getElementById("themeToggle")); @@ -61,6 +65,73 @@ } } + function formatSize(bytes) { + if (bytes < 1024) return bytes + " B"; + var units = ["KiB", "MiB", "GiB", "TiB"]; + var value = bytes; + var unit = "B"; + for (var i = 0; i < units.length && value >= 1024; i += 1) { + value /= 1024; + unit = units[i]; + } + return value.toFixed(value >= 10 ? 0 : 1) + " " + unit; + } + + function renderArchives(groups) { + archiveTree.textContent = ""; + groups.forEach(function (group) { + var branch = document.createElement("details"); + branch.className = "archive-branch"; + branch.open = group.archives.length > 0; + + var summary = document.createElement("summary"); + summary.textContent = group.filename + " (" + group.archives.length + ")"; + branch.appendChild(summary); + + var list = document.createElement("ul"); + group.archives.forEach(function (archive) { + var item = document.createElement("li"); + var link = document.createElement("a"); + link.href = "/PSSM_GREMLIN/api/auth/admin/logs/archives/" + + encodeURIComponent(archive.filename); + link.download = archive.filename; + link.textContent = archive.filename; + item.appendChild(link); + + var metadata = document.createElement("span"); + metadata.className = "archive-meta"; + metadata.textContent = formatSize(archive.size) + " · " + + new Date(archive.modified_at * 1000).toLocaleString(); + item.appendChild(metadata); + list.appendChild(item); + }); + if (!group.archives.length) { + var empty = document.createElement("li"); + empty.className = "muted"; + empty.textContent = "No rotated files"; + list.appendChild(empty); + } + branch.appendChild(list); + archiveTree.appendChild(branch); + }); + } + + async function loadArchives() { + archiveStatus.textContent = "Loading rotated logs…"; + try { + var response = await A.authFetch( + "/PSSM_GREMLIN/api/auth/admin/logs/archives" + ); + var result = await response.json(); + if (!response.ok) throw new Error(result.error || "Failed to load rotated logs"); + renderArchives(result.logs || []); + archivesLoaded = true; + archiveStatus.textContent = "Rotated logs loaded."; + } catch (error) { + archiveStatus.textContent = error.message || "Failed to load rotated logs."; + } + } + buttons.forEach(function (button) { button.addEventListener("click", function () { buttons.forEach(function (item) { item.classList.remove("active"); }); @@ -70,6 +141,10 @@ }); }); document.getElementById("refreshLog").addEventListener("click", loadSelectedLog); + archivePanel.addEventListener("toggle", function () { + if (archivePanel.open && !archivesLoaded) loadArchives(); + }); + document.getElementById("refreshArchives").addEventListener("click", loadArchives); document.getElementById("logoutBtn").addEventListener("click", function () { A.authFetch("/PSSM_GREMLIN/api/auth/logout", { method: "POST" }) .then(function () { A.clearToken(); window.location.href = "/PSSM_GREMLIN/login"; }) diff --git a/server/pssm_gremlin_server/templates/log_viewer.html b/server/pssm_gremlin_server/templates/log_viewer.html index 8f81ed95..65646c3a 100644 --- a/server/pssm_gremlin_server/templates/log_viewer.html +++ b/server/pssm_gremlin_server/templates/log_viewer.html @@ -39,6 +39,15 @@

Server Logs

Loading Gunicorn access…


   
+
+  
+ Rotated log files +
+

Expand to load available ZIP archives.

+ +
+
+
diff --git a/server/tests/test_admin.py b/server/tests/test_admin.py index 443a1332..b8518313 100644 --- a/server/tests/test_admin.py +++ b/server/tests/test_admin.py @@ -6,6 +6,7 @@ import json import os +import zipfile import pytest from conftest import ( @@ -487,6 +488,69 @@ def test_server_log_stream_rejects_non_admin_and_unknown_names(monkeypatch, tmp_ assert response.status_code == 404 +def test_admin_can_list_and_download_rotated_logs(monkeypatch, tmp_path): + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + log_dir = os.environ["LOG_DIR"] + archive_name = "maintenance.log.20260729T000000000000Z.zip" + archive_path = os.path.join(log_dir, archive_name) + with zipfile.ZipFile(archive_path, "w") as archive: + archive.writestr("maintenance.log", "rotated entry\n") + with open(os.path.join(log_dir, "unrelated.zip"), "wb") as handle: + handle.write(b"not managed") + + client = module.app.test_client() + admin_header = _admin_client_auth(module) + response = client.get( + "/PSSM_GREMLIN/api/auth/admin/logs/archives", + headers=admin_header, + ) + + assert response.status_code == 200 + groups = {group["id"]: group for group in response.get_json()["logs"]} + assert [item["filename"] for item in groups["maintenance"]["archives"]] == [ + archive_name + ] + assert all( + item["filename"] != "unrelated.zip" + for group in groups.values() + for item in group["archives"] + ) + + response = client.get( + f"/PSSM_GREMLIN/api/auth/admin/logs/archives/{archive_name}", + headers=admin_header, + ) + assert response.status_code == 200 + assert response.data.startswith(b"PK") + assert "attachment" in response.headers["Content-Disposition"] + assert response.headers["Cache-Control"] == "no-store" + + +def test_rotated_log_endpoints_reject_non_admin_and_unmanaged_files(monkeypatch, tmp_path): + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + client = module.app.test_client() + + response = client.get( + "/PSSM_GREMLIN/api/auth/admin/logs/archives", + headers=_test_client_auth(module), + ) + assert response.status_code == 403 + + response = client.get( + "/PSSM_GREMLIN/api/auth/admin/logs/archives/unrelated.zip", + headers=_admin_client_auth(module), + ) + assert response.status_code == 404 + + def test_user_verify_endpoint(monkeypatch, tmp_path): """GET /PSSM_GREMLIN/user_verify validates token and sets verified status.""" module = _load_pssm_module(monkeypatch, tmp_path, extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}) From f8da9f61b1ca3724dd7ed5d1d5fd5b7862928a58 Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Wed, 29 Jul 2026 10:34:11 +0800 Subject: [PATCH 8/9] test(server): harden log path traversal coverage --- server/pssm_gremlin_server/routes.py | 23 +++-- server/tests/test_security_advanced.py | 124 +++++++++++++++++++++++++ 2 files changed, 139 insertions(+), 8 deletions(-) diff --git a/server/pssm_gremlin_server/routes.py b/server/pssm_gremlin_server/routes.py index bb1d8fe4..57bda1d9 100644 --- a/server/pssm_gremlin_server/routes.py +++ b/server/pssm_gremlin_server/routes.py @@ -15,13 +15,14 @@ import hashlib import logging import os +import re import shutil import time from pathlib import Path from typing import Any from celery.result import AsyncResult -from flask import Response, current_app, g, jsonify, redirect, render_template, request, send_file, send_from_directory, url_for +from flask import Response, current_app, g, jsonify, redirect, render_template, request, send_from_directory, url_for from pssm_gremlin_server.auth import ( _DUMMY_PASSWORD_HASH, UserDatabase, @@ -691,14 +692,15 @@ def require_admin(): "celery-worker": "celery-worker.log", "maintenance": "maintenance.log", } +_ADMIN_LOG_ARCHIVE_PATTERN = re.compile( + rf"(?:{'|'.join(re.escape(name) for name in _ADMIN_LOG_FILES.values())})" + r"\.\d{8}T\d{12}Z\.zip" +) def _admin_log_archive_path(archive_name: str) -> Path | None: """Resolve one managed rotated-log ZIP without allowing arbitrary paths.""" - if not any( - archive_name.startswith(f"{filename}.") and archive_name.endswith(".zip") - for filename in _ADMIN_LOG_FILES.values() - ): + if _ADMIN_LOG_ARCHIVE_PATTERN.fullmatch(archive_name) is None: return None log_dir = os.environ.get("LOG_DIR", "").strip() if not log_dir: @@ -724,7 +726,11 @@ def admin_log_archives(): for log_name, filename in _ADMIN_LOG_FILES.items(): archives = [] for archive in directory.glob(f"{filename}.*.zip"): - if archive.is_symlink() or not archive.is_file(): + if ( + _ADMIN_LOG_ARCHIVE_PATTERN.fullmatch(archive.name) is None + or archive.is_symlink() + or not archive.is_file() + ): continue try: stat = archive.stat() @@ -760,8 +766,9 @@ def admin_download_log_archive(archive_name: str): archive_path = _admin_log_archive_path(archive_name) if archive_path is None: return jsonify({"error": "Log archive is not available"}), 404 - response = send_file( - archive_path, + response = send_from_directory( + archive_path.parent, + archive_path.name, as_attachment=True, download_name=archive_path.name, mimetype="application/zip", diff --git a/server/tests/test_security_advanced.py b/server/tests/test_security_advanced.py index c91e7bf7..d76af0ba 100644 --- a/server/tests/test_security_advanced.py +++ b/server/tests/test_security_advanced.py @@ -6,6 +6,7 @@ import io import json +import os import pytest from conftest import ( @@ -74,6 +75,129 @@ def test_attack_download_path_traversal_in_task_id(monkeypatch, tmp_path): assert resp.status_code in {400, 404}, f"{route} {tid!r}: got {resp.status_code}" +@pytest.mark.parametrize( + "archive_name", + [ + "../outside.zip", + "..%2Foutside.zip", + "%2e%2e%2foutside.zip", + "..%5Coutside.zip", + "%2Fetc%2Fpasswd", + "%5C%5Cserver%5Cshare%5Csecret.zip", + "C:%5CWindows%5Cwin.ini", + "maintenance.log.C:%5CWindows%5Csecret.zip", + "maintenance.log.%2e%2e%2foutside.zip", + "maintenance.log.%252e%252e%252foutside.zip", + "maintenance.log.%00.zip", + "maintenance.log.%E2%88%95..%E2%88%95outside.zip", + "maintenance.log.evil.zip", + "maintenance.log.20260729T010203123456Z.zip.bak", + "gunicorn-access.log....%2Foutside.zip", + ], +) +def test_attack_log_archive_download_rejects_traversal_and_prefix_spoofing( + monkeypatch, + tmp_path, + archive_name, +): + """Encoded separators, dot segments, and prefix spoofing never select a file.""" + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + log_dir = os.environ["LOG_DIR"] + sentinel = b"must-not-be-downloaded" + with open(os.path.join(log_dir, "maintenance.log.evil.zip"), "wb") as handle: + handle.write(sentinel) + + client = module.app.test_client() + admin_header = _admin_client_auth(module) + listing = client.get( + "/PSSM_GREMLIN/api/auth/admin/logs/archives", + headers=admin_header, + ) + response = client.get( + f"/PSSM_GREMLIN/api/auth/admin/logs/archives/{archive_name}", + headers=admin_header, + ) + + assert listing.status_code == 200 + assert b"maintenance.log.evil.zip" not in listing.data + assert response.status_code in {400, 404} + assert sentinel not in response.data + + +def test_attack_log_archive_listing_and_download_reject_symlink_escape( + monkeypatch, + tmp_path, +): + """A valid-looking archive symlink cannot escape LOG_DIR.""" + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + log_dir = os.environ["LOG_DIR"] + outside = tmp_path / "outside-secret.zip" + sentinel = b"outside-log-directory" + outside.write_bytes(sentinel) + archive_name = "maintenance.log.20260729T010203123456Z.zip" + os.symlink(outside, os.path.join(log_dir, archive_name)) + + client = module.app.test_client() + admin_header = _admin_client_auth(module) + listing = client.get( + "/PSSM_GREMLIN/api/auth/admin/logs/archives", + headers=admin_header, + ) + download = client.get( + f"/PSSM_GREMLIN/api/auth/admin/logs/archives/{archive_name}", + headers=admin_header, + ) + + assert listing.status_code == 200 + assert archive_name.encode() not in listing.data + assert download.status_code == 404 + assert sentinel not in download.data + + +@pytest.mark.parametrize( + "log_name", + [ + "../maintenance", + "..%2Fmaintenance", + "%2e%2e%2fmaintenance", + "maintenance%2F..%2Fgunicorn-error", + "..%5Cmaintenance", + "%5C%5Cserver%5Cshare", + "maintenance.log", + ], +) +def test_attack_active_log_stream_rejects_traversal_names( + monkeypatch, + tmp_path, + log_name, +): + """The active-log endpoint accepts only its four opaque identifiers.""" + module = _load_pssm_module( + monkeypatch, + tmp_path, + extra_env={"RUNNER_UID": "1234", "RUNNER_GID": "5678"}, + ) + sentinel = b"active-log-secret" + outside = tmp_path / "outside.log" + outside.write_bytes(sentinel) + + response = module.app.test_client().get( + f"/PSSM_GREMLIN/api/auth/admin/logs/{log_name}", + headers=_admin_client_auth(module), + ) + + assert response.status_code in {400, 404} + assert sentinel not in response.data + + def test_attack_register_with_path_traversal_email(monkeypatch, tmp_path): """Registration with path-traversal in email is rejected by validation.""" module = _load_pssm_module( From 53c46d99651c1ceeccdbab2e0055982145dd07b1 Mon Sep 17 00:00:00 2001 From: YaoYinYing <33014714+YaoYinYing@users.noreply.github.com> Date: Wed, 29 Jul 2026 11:00:52 +0800 Subject: [PATCH 9/9] fix: address PR security and reliability reviews --- CHANGELOG.md | 7 +- Makefile | 10 +- docs/dev-guide/server.md | 5 +- server/.env.example | 3 +- server/README.md | 7 +- .../maintenance/tasks/log_rotation.py | 44 +++++- .../static/js/log-viewer.js | 34 +++-- server/run/restart_pssm_flask.sh | 138 +++++++++++------- server/tests/test_log_rotation.py | 58 +++++++- server/tests/test_process_isolation.py | 23 ++- 10 files changed, 251 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dc8fad5b..b71cbbf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 the restart script and direct server startup fail before touching the database when any required setting is absent. The initial administrator passwords are generated one-by-one and supplied transiently by the restart - script instead of being stored in the env file. + script instead of being stored in the env file. Both `up` and `restart` + support first-start bootstrap; direct Compose startup with an empty user + database remains rejected. - **GREMLIN server: ephemeral signing key**: removed the configurable `AUTH_SECRET_KEY`. Gunicorn generates one in memory per preloaded web launch; restarting web intentionally invalidates active login, verification, and @@ -35,6 +37,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 exceeded or on the `ROTATE_LOG_PERIOD` crontab schedule. Either unset trigger is disabled; `MAX_LOG_SIZE` optionally caps active logs plus ZIP archives using bytes or K/M/G/T suffixes and removes the oldest archives first. + Cron and threshold executions are serialized to protect ZIP/truncate + operations from overlap. Archives created in the current pass are protected + from immediate pruning so the only surviving copy is never discarded. Leaving all three settings unset disables rotation. - **GREMLIN server: admin log viewer**: a standalone page linked from the administrator dashboard lazily streams the four active Gunicorn access, diff --git a/Makefile b/Makefile index 3f52fd2d..35b30ee0 100644 --- a/Makefile +++ b/Makefile @@ -80,13 +80,15 @@ upload-gists: # JSONs for installer gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 -f REvoDesignExtrasTableRich.json jsons/REvoDesignExtrasTableRich.json # HMAC manifest — key is extracted from the uploaded source file - @python tools/generate_gist_manifest.py /tmp/manifest.json - @trap 'rm -f /tmp/manifest.json' EXIT; \ + @tmp_dir="$$(mktemp -d)" || exit; \ + trap 'rm -rf "$$tmp_dir"' 0; \ + manifest="$$tmp_dir/manifest.json"; \ + python tools/generate_gist_manifest.py "$$manifest" || exit; \ files="$$(gh gist view c1e8bfe0fc0b9c60bf49ea04a550a044 --files)" || exit; \ if printf '%s\n' "$$files" | grep -Fxq manifest.json; then \ - gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 -f manifest.json /tmp/manifest.json; \ + gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 -f manifest.json "$$manifest"; \ else \ - gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 --add /tmp/manifest.json; \ + gh gist edit c1e8bfe0fc0b9c60bf49ea04a550a044 --add "$$manifest"; \ fi install-pymol-plugin: diff --git a/docs/dev-guide/server.md b/docs/dev-guide/server.md index 2a04cc4b..2653c732 100644 --- a/docs/dev-guide/server.md +++ b/docs/dev-guide/server.md @@ -342,7 +342,7 @@ Important environment variables (see the organized sections in | `MAX_DB_BACKUP` | Maximum complete snapshot sets to retain; unset is unlimited, recommended value is `30` | | `ROTATE_LOG_MAX_LINENO` | Optional positive line-count threshold for ZIP log rotation; unset disables this trigger | | `ROTATE_LOG_PERIOD` | Optional quoted five-field crontab expression for scheduled rotation (for example, `"0 0 * * *"` for daily at midnight); unset disables this trigger | -| `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest archives first | +| `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest archives first. A newly created archive is retained even when it temporarily exceeds the cap | | `ADMIN_USERS` | Comma-separated admin usernames | | `ALLOWED_EMAIL_DOMAINS` | Comma-separated allowed email domains for self-registration (empty = all allowed). Plus-aliased addresses normalised. | | `ADMIN_NOTIFY_EMAIL` | Comma-separated recipients for new-registration digests | @@ -399,6 +399,9 @@ successful run creates `${AUTH_DIR}/backups//tasks.sqlite3` and REVODESIGN_SERVER_ENV=server/.env.production \ bash server/run/restart_pssm_flask.sh restart --mode=prod ``` + Use the helper script for the first start; direct Docker Compose startup is + rejected while the user database is empty because bootstrap passwords are + generated and supplied transiently by the script. 5. **Access** the web UI at `http://:/PSSM_GREMLIN/dashboard` diff --git a/server/.env.example b/server/.env.example index 8d295b99..6741644f 100644 --- a/server/.env.example +++ b/server/.env.example @@ -168,7 +168,8 @@ WORKER_CONCURRENCY=2 # ROTATE_LOG_MAX_LINENO=100000 # ROTATE_LOG_PERIOD="0 0 * * *" # Cap the combined size of active logs and their ZIP archives. Oldest ZIPs are -# removed first; active logs are rotated only when that is insufficient. +# removed first; active logs are rotated only when that is insufficient. A ZIP +# created in the current pass is retained even when it temporarily exceeds cap. # Accepts bytes or K/M/G/T suffixes. Leave unset to disable the size cap. # MAX_LOG_SIZE=1G diff --git a/server/README.md b/server/README.md index b6ebedc7..2d643a18 100644 --- a/server/README.md +++ b/server/README.md @@ -179,7 +179,7 @@ Fallback when `REVODESIGN_SERVER_ENV` is unset: | `MAX_DB_BACKUP` | Maximum complete snapshot sets to retain. Leave unset for unlimited history; recommended value: `30`. | | `ROTATE_LOG_MAX_LINENO` | Optional line-count rotation threshold; unset disables this trigger. | | `ROTATE_LOG_PERIOD` | Optional quoted five-field crontab expression for scheduled rotation (for example, `"0 0 * * *"` for daily at midnight); unset disables this trigger. | -| `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest ZIPs first. | +| `MAX_LOG_SIZE` | Optional total cap for active logs plus ZIP archives; accepts bytes or K/M/G/T suffixes and removes oldest ZIPs first. A newly created archive is retained even when it temporarily exceeds the cap. | | `ADMIN_USERS` | Comma-separated admin usernames for cross-user management. | | `ADMIN_NOTIFY_EMAIL` | Comma-separated admin email addresses for new-user registration digests (default: empty = no notification). | | `ADMIN_NEW_USER_INFORM` | Interval in minutes between new-user digest emails (default: `0` = disabled). | @@ -448,6 +448,11 @@ start normally. The web process creates `${AUTH_DIR}/users.sqlite3`. ### Equivalent Docker Compose commands +These commands are equivalent only after `users.sqlite3` contains an account. +On a fresh installation, use the helper script's `up` or `restart` command so +it can generate and pass transient bootstrap credentials. A direct Compose +startup with an empty user database is rejected. + Development mode: ```bash diff --git a/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py b/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py index 13bba3fc..7f8b9529 100644 --- a/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py +++ b/server/pssm_gremlin_server/maintenance/tasks/log_rotation.py @@ -9,6 +9,7 @@ import logging import os import re +import threading import time import zipfile from collections.abc import Callable @@ -34,6 +35,7 @@ "T": 1024**4, "TB": 1024**4, } +_ROTATION_LOCK = threading.Lock() def parse_log_size(value: str) -> int: @@ -57,8 +59,13 @@ def _managed_log_size(log_dir: Path) -> int: return sum(path.stat().st_size for path in paths if path.is_file()) -def _prune_oldest_archives(log_dir: Path, max_size: int) -> None: +def _prune_oldest_archives( + log_dir: Path, + max_size: int, + protected: set[Path] | None = None, +) -> None: total = _managed_log_size(log_dir) + protected = protected or set() archives = sorted( log_dir.glob("*.log.*.zip"), key=lambda path: (path.stat().st_mtime, path.name), @@ -66,13 +73,15 @@ def _prune_oldest_archives(log_dir: Path, max_size: int) -> None: for archive in archives: if total <= max_size: break + if archive in protected: + continue size = archive.stat().st_size archive.unlink() total -= size logging.info("Removed oldest rotated log %s", archive) -def rotate_logs( +def _rotate_logs( log_dir: str, max_lines: int | None, rotate_for_period: bool, @@ -80,16 +89,21 @@ def rotate_logs( *, now: float | None = None, ) -> int: - """Rotate matching logs, enforce the total size cap, and return the count.""" + """Perform one log-rotation pass without concurrency control.""" current_time = time.time() if now is None else now directory = Path(log_dir) rotated = 0 + created_archives: set[Path] = set() if max_size is not None: _prune_oldest_archives(directory, max_size) - rotate_for_size = max_size is not None and _managed_log_size(directory) > max_size for log_path in sorted(directory.glob("*.log")): + if max_size is not None: + _prune_oldest_archives(directory, max_size, created_archives) + rotate_for_size = ( + max_size is not None and _managed_log_size(directory) > max_size + ) by_lines = max_lines is not None and _line_count(log_path) > max_lines if log_path.stat().st_size == 0 or not ( by_lines or rotate_for_period or rotate_for_size @@ -102,6 +116,7 @@ def rotate_logs( archive = log_path.with_name(f"{log_path.name}.{timestamp}.zip") with zipfile.ZipFile(archive, "x", compression=zipfile.ZIP_DEFLATED) as bundle: bundle.write(log_path, arcname=log_path.name) + created_archives.add(archive) # ponytail: copy-truncate keeps existing process file descriptors valid; # use service-specific reopen signals only if the tiny write race matters. log_path.open("w", encoding="utf-8").close() @@ -109,10 +124,29 @@ def rotate_logs( logging.info("Rotated log %s to %s", log_path, archive) if max_size is not None: - _prune_oldest_archives(directory, max_size) + _prune_oldest_archives(directory, max_size, created_archives) return rotated +def rotate_logs( + log_dir: str, + max_lines: int | None, + rotate_for_period: bool, + max_size: int | None, + *, + now: float | None = None, +) -> int: + """Serialize and run one log-rotation pass.""" + with _ROTATION_LOCK: + return _rotate_logs( + log_dir, + max_lines, + rotate_for_period, + max_size, + now=now, + ) + + class LogRotationTask(PeriodicTask): """Environment-configured log rotation.""" diff --git a/server/pssm_gremlin_server/static/js/log-viewer.js b/server/pssm_gremlin_server/static/js/log-viewer.js index f8b0ca2d..30e25b45 100644 --- a/server/pssm_gremlin_server/static/js/log-viewer.js +++ b/server/pssm_gremlin_server/static/js/log-viewer.js @@ -45,13 +45,13 @@ var reader = response.body.getReader(); var decoder = new TextDecoder(); var byteCount = 0; - while (true) { - var result = await reader.read(); - if (result.done) break; + var result = await reader.read(); + while (!result.done) { byteCount += result.value.byteLength; output.appendChild(document.createTextNode(decoder.decode(result.value, { stream: true }))); output.scrollTop = output.scrollHeight; status.textContent = "Streaming " + selectedLog + " — " + byteCount + " bytes"; + result = await reader.read(); } output.appendChild(document.createTextNode(decoder.decode())); } @@ -67,12 +67,20 @@ function formatSize(bytes) { if (bytes < 1024) return bytes + " B"; - var units = ["KiB", "MiB", "GiB", "TiB"]; - var value = bytes; - var unit = "B"; - for (var i = 0; i < units.length && value >= 1024; i += 1) { - value /= 1024; - unit = units[i]; + var value; + var unit; + if (bytes < 1024 * 1024) { + value = bytes / 1024; + unit = "KiB"; + } else if (bytes < 1024 * 1024 * 1024) { + value = bytes / (1024 * 1024); + unit = "MiB"; + } else if (bytes < 1024 * 1024 * 1024 * 1024) { + value = bytes / (1024 * 1024 * 1024); + unit = "GiB"; + } else { + value = bytes / (1024 * 1024 * 1024 * 1024); + unit = "TiB"; } return value.toFixed(value >= 10 ? 0 : 1) + " " + unit; } @@ -146,9 +154,13 @@ }); document.getElementById("refreshArchives").addEventListener("click", loadArchives); document.getElementById("logoutBtn").addEventListener("click", function () { + function finishLogout() { + A.clearToken(); + window.location.replace("/PSSM_GREMLIN/login"); + } A.authFetch("/PSSM_GREMLIN/api/auth/logout", { method: "POST" }) - .then(function () { A.clearToken(); window.location.href = "/PSSM_GREMLIN/login"; }) - .catch(function () { A.clearToken(); window.location.href = "/PSSM_GREMLIN/login"; }); + .then(finishLogout) + .catch(finishLogout); }); loadSelectedLog(); }()); diff --git a/server/run/restart_pssm_flask.sh b/server/run/restart_pssm_flask.sh index b24abefe..5a423ce3 100644 --- a/server/run/restart_pssm_flask.sh +++ b/server/run/restart_pssm_flask.sh @@ -227,6 +227,91 @@ if os.path.commonpath([server_dir, auth_dir]) == server_dir: ' "${SERVER_DIR}" "${AUTH_DIR}" ) +ADMIN_LOGIN_LINES=() + +prepare_admin_bootstrap() { + set +u + set -a + source "${ENV_FILE}" + set +a + set -u + + local auth_dir="${AUTH_DIR:-${SCRIPT_DIR}/../auth-data}" + local user_db="${auth_dir}/users.sqlite3" + local legacy_user_db="${SERVER_DIR}/users.sqlite3" + local needs_admin_bootstrap="" + local admin_bootstrap_credentials="" + local admin_username="" + local admin_pw="" + local seen_admin="" + local -a configured_admins=() + local -a seen_admins=() + + if [[ -f "${legacy_user_db}" && ! -f "${user_db}" ]]; then + echo "Legacy user DB detected at ${legacy_user_db}." >&2 + echo "Run the migrate-auth-db subcommand before starting this release." >&2 + exit 1 + fi + if [[ -n "${ADMIN_BOOTSTRAP_CREDENTIALS:-}" ]]; then + return + fi + + needs_admin_bootstrap="$( + python3 - "${user_db}" <<'PY' +import sqlite3 +import sys +from pathlib import Path + +path = Path(sys.argv[1]) +if not path.is_file(): + print("yes") +else: + try: + with sqlite3.connect(f"file:{path}?mode=ro", uri=True) as conn: + has_users = conn.execute( + "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'users'" + ).fetchone() + count = conn.execute("SELECT COUNT(*) FROM users").fetchone()[0] if has_users else 0 + print("yes" if count == 0 else "no") + except sqlite3.Error: + print("no") +PY + )" + if [[ "${needs_admin_bootstrap}" != "yes" ]]; then + return + fi + + IFS=',' read -r -a configured_admins <<< "${ADMIN_USERS}" + for admin_username in "${configured_admins[@]}"; do + admin_username="${admin_username#"${admin_username%%[![:space:]]*}"}" + admin_username="${admin_username%"${admin_username##*[![:space:]]}"}" + if [[ -z "${admin_username}" ]]; then + continue + fi + for seen_admin in "${seen_admins[@]}"; do + if [[ "${seen_admin}" == "${admin_username}" ]]; then + echo "ADMIN_USERS must not contain duplicate usernames: ${admin_username}" >&2 + exit 1 + fi + done + seen_admins+=("${admin_username}") + admin_pw="$(openssl rand -hex 16 2>/dev/null || python3 -c 'import secrets; print(secrets.token_hex(16))')" + admin_bootstrap_credentials+="${admin_username}"$'\t'"${admin_pw}"$'\n' + ADMIN_LOGIN_LINES+=("Admin login — username: ${admin_username} password: ${admin_pw}") + done + if [[ ${#ADMIN_LOGIN_LINES[@]} -eq 0 ]]; then + echo "ADMIN_USERS must contain at least one username." >&2 + exit 1 + fi + export ADMIN_BOOTSTRAP_CREDENTIALS="${admin_bootstrap_credentials}" +} + +print_admin_logins() { + if [[ ${#ADMIN_LOGIN_LINES[@]} -gt 0 ]]; then + printf '%s\n' "${ADMIN_LOGIN_LINES[@]}" + fi +} + cmd_setup() { local _detected_docker_gid="" @@ -266,10 +351,12 @@ cmd_up() { require_env_file validate_required_settings validate_auth_storage + prepare_admin_bootstrap ensure_docker_gid resolve_runner_identity echo "Starting services via docker compose..." "${COMPOSE_CMD[@]}" -f "${COMPOSE_FILE}" --env-file "${ENV_FILE}" up "$@" -d redis web maintenance worker + print_admin_logins } cmd_down() { @@ -323,55 +410,9 @@ cmd_restart() { require_production_identity fi + prepare_admin_bootstrap _auth_dir="${AUTH_DIR:-${SCRIPT_DIR}/../auth-data}" _user_db="${_auth_dir}/users.sqlite3" - _admin_login_lines=() - _legacy_user_db="${SERVER_DIR}/users.sqlite3" - if [[ -f "${_legacy_user_db}" && ! -f "${_user_db}" ]]; then - echo "Legacy user DB detected at ${_legacy_user_db}." >&2 - echo "Run the migrate-auth-db subcommand before restarting this release." >&2 - exit 1 - fi - _needs_admin_bootstrap="$( - python3 - "${_user_db}" <<'PY' -import sqlite3 -import sys -from pathlib import Path - -path = Path(sys.argv[1]) -if not path.is_file(): - print("yes") -else: - try: - with sqlite3.connect(f"file:{path}?mode=ro", uri=True) as conn: - has_users = conn.execute( - "SELECT 1 FROM sqlite_master WHERE type = 'table' AND name = 'users'" - ).fetchone() - count = conn.execute("SELECT COUNT(*) FROM users").fetchone()[0] if has_users else 0 - print("yes" if count == 0 else "no") - except sqlite3.Error: - print("no") -PY - )" - if [[ "${_needs_admin_bootstrap}" == "yes" ]]; then - _admin_bootstrap_credentials="" - IFS=',' read -r -a _configured_admins <<< "${ADMIN_USERS}" - for _admin_username in "${_configured_admins[@]}"; do - _admin_username="${_admin_username#"${_admin_username%%[![:space:]]*}"}" - _admin_username="${_admin_username%"${_admin_username##*[![:space:]]}"}" - if [[ -z "${_admin_username}" ]]; then - continue - fi - _admin_pw="$(openssl rand -hex 16 2>/dev/null || python3 -c 'import secrets; print(secrets.token_hex(16))')" - _admin_bootstrap_credentials+="${_admin_username}"$'\t'"${_admin_pw}"$'\n' - _admin_login_lines+=("Admin login — username: ${_admin_username} password: ${_admin_pw}") - done - if [[ ${#_admin_login_lines[@]} -eq 0 ]]; then - echo "ADMIN_USERS must contain at least one username." >&2 - exit 1 - fi - export ADMIN_BOOTSTRAP_CREDENTIALS="${_admin_bootstrap_credentials}" - fi cmd_down if [[ -f "${_user_db}" ]]; then @@ -406,9 +447,6 @@ PY PORT="${PORT:-8080}" echo "Deployment completed." echo "Flask app is now running at http://${DOMAIN}:${PORT}/PSSM_GREMLIN/dashboard" - if [[ ${#_admin_login_lines[@]} -gt 0 ]]; then - printf '%s\n' "${_admin_login_lines[@]}" - fi } SUBCOMMAND="${1:-restart}" diff --git a/server/tests/test_log_rotation.py b/server/tests/test_log_rotation.py index 5004b789..9e0a9c6e 100644 --- a/server/tests/test_log_rotation.py +++ b/server/tests/test_log_rotation.py @@ -5,9 +5,12 @@ from __future__ import annotations import os +import threading +import time import zipfile import pytest +from pssm_gremlin_server.maintenance.tasks import log_rotation as log_rotation_module from pssm_gremlin_server.maintenance.tasks.log_rotation import ( log_rotation_task, @@ -36,6 +39,39 @@ def test_scheduled_period_rotates_nonempty_logs(tmp_path): assert rotate_logs(str(tmp_path), None, True, None, now=1_000_000) == 1 +def test_rotation_passes_are_serialized_across_scheduler_jobs(monkeypatch, tmp_path): + active = 0 + max_active = 0 + state_lock = threading.Lock() + start = threading.Barrier(3) + + def recording_rotation(*_args, **_kwargs): + nonlocal active, max_active + with state_lock: + active += 1 + max_active = max(max_active, active) + time.sleep(0.05) + with state_lock: + active -= 1 + return 0 + + monkeypatch.setattr(log_rotation_module, "_rotate_logs", recording_rotation) + + def run_rotation(): + start.wait() + rotate_logs(str(tmp_path), 1, False, None) + + threads = [threading.Thread(target=run_rotation) for _ in range(2)] + for thread in threads: + thread.start() + start.wait() + for thread in threads: + thread.join(timeout=2) + + assert all(not thread.is_alive() for thread in threads) + assert max_active == 1 + + def test_size_cap_removes_oldest_archive_before_touching_live_log(tmp_path): log = tmp_path / "web.log" log.write_bytes(b"x" * 300_000) @@ -55,13 +91,29 @@ def test_size_cap_removes_oldest_archive_before_touching_live_log(tmp_path): def test_size_cap_rotates_live_log_when_archives_cannot_reduce_total(tmp_path): log = tmp_path / "web.log" - log.write_bytes(os.urandom(2 * 1024**2)) + content = os.urandom(2 * 1024**2) + log.write_bytes(content) assert rotate_logs(str(tmp_path), None, False, 1024**2, now=3) == 1 - total = sum(path.stat().st_size for path in tmp_path.glob("web.log*")) + archives = list(tmp_path.glob("web.log.*.zip")) + assert len(archives) == 1 + with zipfile.ZipFile(archives[0]) as bundle: + assert bundle.read("web.log") == content assert log.stat().st_size == 0 - assert total <= 1024**2 + + +def test_size_rotation_stops_after_total_falls_below_cap(tmp_path): + large = tmp_path / "a.log" + untouched = tmp_path / "b.log" + large.write_bytes(b"x" * (2 * 1024**2)) + untouched.write_text("keep me\n", encoding="utf-8") + + assert rotate_logs(str(tmp_path), None, False, 1024**2, now=3) == 1 + + assert large.stat().st_size == 0 + assert untouched.read_text(encoding="utf-8") == "keep me\n" + assert not list(tmp_path.glob("b.log.*.zip")) def test_log_rotation_task_configures_all_triggers(monkeypatch, tmp_path): diff --git a/server/tests/test_process_isolation.py b/server/tests/test_process_isolation.py index 01c20e9e..4c0870ca 100644 --- a/server/tests/test_process_isolation.py +++ b/server/tests/test_process_isolation.py @@ -118,6 +118,27 @@ def test_restart_generates_distinct_password_for_each_configured_admin(tmp_path) assert all(len(password) == 32 for password in passwords) +def test_up_generates_bootstrap_password_for_empty_user_database(tmp_path): + result, commands = _run_restart_script(tmp_path, "up") + + assert result.returncode == 0, result.stderr + assert "Admin login — username: admin password:" in result.stdout + assert any("up -d redis web maintenance worker" in command for command in commands) + + +def test_up_rejects_duplicate_admin_usernames_before_start(tmp_path): + result, commands = _run_restart_script( + tmp_path, + "up", + admins="admin,admin", + ) + + assert result.returncode != 0 + assert "ADMIN_USERS must not contain duplicate usernames: admin" in result.stderr + assert "Admin login" not in result.stdout + assert not any("up -d redis web maintenance worker" in command for command in commands) + + def test_restart_mode_validation(tmp_path): identity_result, identity_commands = _run_restart_script( tmp_path / "identity", @@ -146,7 +167,7 @@ def test_restart_rejects_missing_required_settings_before_shutdown(tmp_path, nam ) assert result.returncode != 0 - assert f"Missing required setting(s)" in result.stderr + assert "Missing required setting(s)" in result.stderr assert name in result.stderr assert not any(" down" in command or " build " in command or " pull " in command or " up " in command for command in commands)