From 8083fc16ebda71f2d012d67d616742224eeb571d Mon Sep 17 00:00:00 2001 From: Konrad Kasperkiewicz <119067436+bihius@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:21:00 +0200 Subject: [PATCH 1/2] fix(backend): refuse stale seed config and unreported version skew A tag-only upgrade of the release kit runs new images against the old bind-mounted haproxy/ files. The old coraza.cfg sends app=str(default), so every request is inspected by the default Coraza application instead of its vhost's policy_: traffic is still filtered, but with the wrong policy and no error anywhere. Stamp haproxy.cfg and coraza.cfg with `# guard-proxy-config-version: N` and bake the expected version into the backend. The seed coraza.cfg is checked at startup, in GET /health (degraded), in GET /ready (fails, so the release kit backend healthcheck fails) and in POST /config/apply (409), naming the stale file. Absent outside a release kit, so local runs are unaffected. Also stop hardcoding the CRS version: the template now takes it from one constant that a test pins to the configs/coraza/crs submodule, and GET /runtime/status reports app_version, the configured image tag and the CRS version, warning when the tag and the backend version disagree. Closes #312 Closes #316 --- CHANGELOG.md | 25 +++ configs/haproxy/coraza.cfg | 1 + configs/haproxy/haproxy.cfg | 1 + docs/architecture.md | 17 +- release/README.md | 20 +- release/haproxy/coraza.cfg | 1 + release/haproxy/haproxy.cfg | 1 + src/backend/app/config.py | 6 + src/backend/app/constants/versions.py | 18 ++ src/backend/app/main.py | 68 +++---- src/backend/app/routers/config.py | 15 ++ src/backend/app/schemas/runtime_status.py | 16 ++ src/backend/app/services/config_renderer.py | 20 +- src/backend/app/services/config_version.py | 111 +++++++++++ .../app/services/runtime_status_service.py | 28 ++- src/backend/app/templates/crs-setup.conf.j2 | 10 +- src/backend/app/templates/haproxy.cfg.j2 | 1 + src/backend/app/version.py | 34 ++++ .../tests/integration/test_config_router.py | 45 +++++ .../tests/integration/test_health_router.py | 61 ++++++ .../integration/test_runtime_status_router.py | 57 ++++++ src/backend/tests/unit/test_app_version.py | 16 +- .../unit/test_config_renderer_crs_setup.py | 12 ++ src/backend/tests/unit/test_config_version.py | 173 ++++++++++++++++++ 24 files changed, 705 insertions(+), 52 deletions(-) create mode 100644 src/backend/app/constants/versions.py create mode 100644 src/backend/app/services/config_version.py create mode 100644 src/backend/app/version.py create mode 100644 src/backend/tests/unit/test_config_version.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ded1e02e..7cbc9f2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,31 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 Python distributions use the [PEP 440](https://peps.python.org/pep-0440/) spelling of the same version (e.g. `0.1.0b2` for `0.1.0-beta.2`). +## [Unreleased] + +### Added + +- `GET /runtime/status` reports `version_agreement` (`app_version`, + `configured_image_tag`, `crs_version`) and warns when the tag in + `GUARD_PROXY_IMAGE_TAG` and the backend's own version disagree, so a stack + mixing images from two releases is visible instead of assumed absent. +- The generated CRS metadata (`ver:'OWASP_CRS/...'` and + `tx.crs_setup_version`) comes from one constant that a test pins to the + `configs/coraza/crs` submodule, replacing the hardcoded `4.25.0` / `4250` in + the template. Bumping the submodule without the constant now fails a test + instead of silently mislabelling the rules that load. + +### Fixed + +- A tag-only upgrade (new images, old bind-mounted `haproxy/` files) no longer + runs silently with the `default` Coraza application instead of each vhost's + `policy_`. The shipped `haproxy/haproxy.cfg` and `haproxy/coraza.cfg` + carry a `# guard-proxy-config-version: N` stamp; the backend compares the + mounted `coraza.cfg` with the version its image expects at startup, in + `GET /health` (`degraded`), in `GET /ready` (fails, so the release kit's + backend healthcheck fails too) and in `POST /config/apply` (`409`). Every + report names the stale file. See `release/README.md`. + ## [0.1.0-beta.10] - 2026-10-15 ### Fixed diff --git a/configs/haproxy/coraza.cfg b/configs/haproxy/coraza.cfg index e21557e3..285b0491 100644 --- a/configs/haproxy/coraza.cfg +++ b/configs/haproxy/coraza.cfg @@ -1,3 +1,4 @@ +# guard-proxy-config-version: 1 # SPOE engine configuration for the Coraza SPOA. # # This file is referenced from haproxy.cfg via: diff --git a/configs/haproxy/haproxy.cfg b/configs/haproxy/haproxy.cfg index aa821c16..1861edfe 100644 --- a/configs/haproxy/haproxy.cfg +++ b/configs/haproxy/haproxy.cfg @@ -1,3 +1,4 @@ +# guard-proxy-config-version: 1 # Guard Proxy HAProxy configuration. # Generated from the policy database; manual edits are overwritten on the # next config apply. diff --git a/docs/architecture.md b/docs/architecture.md index 7f5fa44d..6f0bbd90 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -369,19 +369,28 @@ The backend exposes two probe endpoints, both unauthenticated: | Endpoint | Type | Returns | Checks | |----------|------|---------|--------| -| `GET /health` | Liveness | 200 always | None — proves the process is alive | -| `GET /ready` | Readiness | 200 / 503 | DB connectivity (`SELECT 1`), runtime config volume writable | +| `GET /health` | Liveness | 200 always | Process aliveness, plus the seed config version stamp (issue #312) | +| `GET /ready` | Readiness | 200 / 503 | DB connectivity (`SELECT 1`), runtime config volume writable, seed config version stamp | `/ready` returns a JSON body with per-check status so operators can tell which dependency is down: ```json // 200 — all clear -{"status": "ready", "checks": {"database": {"status": "ok"}, "runtime_config": {"status": "ok"}}} +{"status": "ready", "checks": {"database": {"status": "ok"}, "runtime_config": {"status": "ok"}, "seed_config": {"status": "ok"}}} // 503 — one or more dependencies unavailable -{"status": "not ready", "checks": {"database": {"status": "ok"}, "runtime_config": {"status": "error", "detail": "/var/lib/guard-proxy/generated is not a writable directory"}}} +{"status": "not ready", "checks": {"database": {"status": "ok"}, "runtime_config": {"status": "error", "detail": "/var/lib/guard-proxy/generated is not a writable directory"}, "seed_config": {"status": "ok"}}} ``` +Both probes also compare the bind-mounted seed `coraza.cfg` with the +`guard-proxy-config-version` stamp this image expects. A missing or older +stamp — the tag-only upgrade of issue #312, where HAProxy would silently +inspect every request with the `default` Coraza application instead of the +vhost's `policy_` — makes `/health` report `degraded` and `/ready` fail +with the stale file named. `POST /config/apply` refuses with `409` for the +same reason. Outside a release kit the file is absent and the check is +skipped. + The Docker Compose `healthcheck` for the `backend` service targets `/ready`. Almost every other service in the stack (`frontend`, `haproxy`, `coraza`, `log-shipper`) depends on `backend` with `condition: service_healthy`, so `/ready` must pass before dependents start. This prevents dependents from launching while the database is still initialising or the config volume is unavailable. `/health` is retained as a lightweight liveness probe for orchestrators that separately track process aliveness (e.g. a future Kubernetes `livenessProbe`). diff --git a/release/README.md b/release/README.md index d87a564a..9d9751ba 100644 --- a/release/README.md +++ b/release/README.md @@ -119,10 +119,28 @@ it — the static `haproxy.cfg` seed is replaced at that point. breaking-change callouts. 2. Back up the `pgdata` volume (`docker compose exec postgres pg_dump ...` or a volume snapshot) before upgrading, out of caution during beta. -3. Update `GUARD_PROXY_IMAGE_TAG` in `.env` to the new version. +3. Update `GUARD_PROXY_IMAGE_TAG` in `.env` to the new version **and** replace + the `haproxy/` directory (`haproxy.cfg` and `coraza.cfg`) with the one from + that release. The tag and the files are one unit: the config files are + bind-mounted into the containers, not shipped in the images. 4. `docker compose pull && docker compose up -d`. The backend runs `alembic upgrade head` automatically on startup. +### A tag-only upgrade is refused + +The shipped `haproxy/haproxy.cfg` and `haproxy/coraza.cfg` carry a +`# guard-proxy-config-version: N` stamp, and the backend knows the version its +image expects. If you bump the tag but keep the old files, `GET /health` +reports `degraded`, `GET /ready` — which the backend container's healthcheck +calls — fails, and `POST /config/apply` answers `409`; each names the stale +file. Replace the `haproxy/` files and restart the backend. + +The check exists because the failure it catches is otherwise invisible: an old +`coraza.cfg` tells HAProxy to inspect every request with the `default` Coraza +application instead of the vhost's `policy_`. Traffic is still inspected +and attacks are still blocked, so nothing looks broken — the vhost's own +paranoia level, thresholds and rule overrides are simply not applied. + ### What an upgrade does not carry over Everything you configure in the admin UI survives an upgrade: it lives in diff --git a/release/haproxy/coraza.cfg b/release/haproxy/coraza.cfg index e21557e3..285b0491 100644 --- a/release/haproxy/coraza.cfg +++ b/release/haproxy/coraza.cfg @@ -1,3 +1,4 @@ +# guard-proxy-config-version: 1 # SPOE engine configuration for the Coraza SPOA. # # This file is referenced from haproxy.cfg via: diff --git a/release/haproxy/haproxy.cfg b/release/haproxy/haproxy.cfg index aa821c16..1861edfe 100644 --- a/release/haproxy/haproxy.cfg +++ b/release/haproxy/haproxy.cfg @@ -1,3 +1,4 @@ +# guard-proxy-config-version: 1 # Guard Proxy HAProxy configuration. # Generated from the policy database; manual edits are overwritten on the # next config apply. diff --git a/src/backend/app/config.py b/src/backend/app/config.py index 4b507d5b..e9a4a158 100644 --- a/src/backend/app/config.py +++ b/src/backend/app/config.py @@ -107,6 +107,12 @@ class Settings(EnvFileSettings): haproxy_reload_timeout_seconds: int = 10 haproxy_stats_socket_path: str = "/var/run/haproxy/admin.sock" haproxy_stats_timeout_seconds: int = 10 + # Bind-mounted seed SPOE config from the release kit, checked against + # EXPECTED_CONFIG_VERSION (issue #312). Absent outside a container. + seed_coraza_config_path: str = "/usr/local/etc/haproxy/coraza.cfg" + # Image tag the operator pulled, compared with this backend's own version + # in /runtime/status. Set by the release kit's .env; unset in dev. + guard_proxy_image_tag: str | None = None log_retention_days: int = 30 # Log level written into the generated coraza-spoa.yaml (debug, info, # warn, error). The debug compose overlay sets CORAZA_LOG_LEVEL=debug. diff --git a/src/backend/app/constants/versions.py b/src/backend/app/constants/versions.py new file mode 100644 index 00000000..49db5d6a --- /dev/null +++ b/src/backend/app/constants/versions.py @@ -0,0 +1,18 @@ +"""Versions this backend expects the rest of the stack to agree with. + +`EXPECTED_CONFIG_VERSION` is the stamp every shipped `haproxy.cfg` / +`coraza.cfg` carries; bump it together with those files (see +`configs/haproxy/coraza.cfg`) when the contract they implement changes. The +backend refuses to report healthy for a bind-mounted seed config that does not +carry the expected stamp (issue #312). + +`CRS_VERSION` must match the `configs/coraza/crs` submodule; bump both +together, because the generated CRS metadata is derived from it and the +submodule supplies the actual rules (issue #316). +""" + +from __future__ import annotations + +EXPECTED_CONFIG_VERSION = 1 + +CRS_VERSION = "4.25.0" diff --git a/src/backend/app/main.py b/src/backend/app/main.py index 787b5999..bf069d36 100644 --- a/src/backend/app/main.py +++ b/src/backend/app/main.py @@ -1,10 +1,7 @@ import logging import os -import tomllib from collections.abc import AsyncIterator from contextlib import asynccontextmanager -from importlib.metadata import PackageNotFoundError -from importlib.metadata import version as _package_version from pathlib import Path from fastapi import Depends, FastAPI @@ -47,37 +44,13 @@ ) from app.services.config_apply import seed_runtime_config from app.services.config_generator import generate +from app.services.config_version import check_seed_config from app.services.scheduler import start_scheduler, stop_scheduler +from app.version import APP_VERSION logger = logging.getLogger(__name__) -def _resolve_app_version() -> str: - """Return the app version, reading from a single source of truth. - - Prefers installed package metadata (present in dev venvs where the - project is installed editable). The production Docker image installs - only dependencies as a distinct layer and copies app source on top - without installing the project itself, so no dist-info exists there — - fall back to parsing `pyproject.toml` directly in that case. - """ - try: - return _package_version("guard-proxy-backend") - except PackageNotFoundError: - pyproject_path = Path(__file__).resolve().parent.parent / "pyproject.toml" - try: - with pyproject_path.open("rb") as f: - version = tomllib.load(f)["project"]["version"] - if not isinstance(version, str): - raise KeyError("project.version is not a string") - return version - except (OSError, KeyError): # pragma: no cover - defensive fallback - return "0.0.0+unknown" - - -APP_VERSION = _resolve_app_version() - - class _HealthcheckAccessFilter(logging.Filter): def filter(self, record: logging.LogRecord) -> bool: args = record.args @@ -105,6 +78,11 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: "sent over plain HTTP. Enable it once the admin UI is served over " "HTTPS." ) + seed_config = check_seed_config() + if not seed_config.ok: + # Log once at startup; the probes re-read the file, so replacing it + # clears the degraded state without a restart. + logger.error("Refusing to report healthy: %s", seed_config.detail) _seed_runtime_config() start_scheduler() yield @@ -183,6 +161,7 @@ def _reconcile_runtime_operation_checksum( + app = FastAPI( title=settings.app_name, version=APP_VERSION, @@ -219,10 +198,22 @@ def _reconcile_runtime_operation_checksum( @app.get("/health") def health_check() -> dict[str, str]: - """Liveness probe — always returns 200 if the process is running.""" + """Liveness probe — always returns 200 if the process is running. + + Reports `degraded` when the bind-mounted seed config does not match this + image (issue #312): the process is alive, but HAProxy may be enforcing a + different policy set than the one this release expects. + """ + seed_config = check_seed_config() + if seed_config.ok: + return { + "status": "healthy", + "version": APP_VERSION, + } return { - "status": "healthy", + "status": "degraded", "version": APP_VERSION, + "detail": seed_config.detail or "", } @@ -259,6 +250,21 @@ def readiness_check(db: Session = Depends(get_db)) -> JSONResponse: } all_ok = False + # Check 3: the bind-mounted seed config matches this image (issue #312). + # The release kit's backend healthcheck calls this endpoint, so a half-done + # upgrade leaves the stack unhealthy instead of silently enforcing the + # wrong Coraza application. + seed_config = check_seed_config() + if seed_config.ok: + checks["seed_config"] = {"status": "ok"} + else: + logger.error("Readiness check failed: %s", seed_config.detail) + checks["seed_config"] = { + "status": "error", + "detail": seed_config.detail or "seed config version mismatch", + } + all_ok = False + return JSONResponse( content={ "status": "ready" if all_ok else "not ready", diff --git a/src/backend/app/routers/config.py b/src/backend/app/routers/config.py index d5dc71fb..a5a5dcc0 100644 --- a/src/backend/app/routers/config.py +++ b/src/backend/app/routers/config.py @@ -1,5 +1,6 @@ """Config apply router — triggers generate + apply pipeline.""" +import logging import threading from fastapi import APIRouter, Depends, HTTPException, status @@ -24,6 +25,9 @@ from app.services.config_apply import ApplyResult, ApplyStatus from app.services.config_apply import apply as _apply from app.services.config_generator import generate +from app.services.config_version import check_seed_config + +logger = logging.getLogger(__name__) router = APIRouter(prefix="/config", tags=["config"]) @@ -92,6 +96,17 @@ def apply_config( detail="Another config apply is already in progress", ) try: + seed_config = check_seed_config() + if not seed_config.ok: + # HAProxy reads the bind-mounted seed config at runtime, so a + # stale one applies the wrong Coraza application no matter what + # this release generates (issue #312). + logger.error("Refusing config apply: %s", seed_config.detail) + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail=f"Refusing to apply: {seed_config.detail}", + ) + vhosts = db.query(VHost).options(selectinload(VHost.backends)).all() policies = db.query(Policy).all() rule_overrides = db.query(RuleOverride).all() diff --git a/src/backend/app/schemas/runtime_status.py b/src/backend/app/schemas/runtime_status.py index d21ac9cc..e85965cb 100644 --- a/src/backend/app/schemas/runtime_status.py +++ b/src/backend/app/schemas/runtime_status.py @@ -43,6 +43,21 @@ class ActiveRuntimeConfig(BaseModel): warning: str | None = None +class VersionAgreement(BaseModel): + """Versions the stack pieces must agree on (issue #316). + + `configured_image_tag` is the tag the operator wrote into `.env`; it is + None when the backend is not run from the release kit. `warning` is set + when that tag and this backend's own version disagree, which means the + stack mixes images from different releases. + """ + + app_version: str + configured_image_tag: str | None + crs_version: str + warning: str | None = None + + class RuntimeOperationSnapshot(BaseModel): """Latest persisted status for a specific runtime operation type.""" @@ -62,5 +77,6 @@ class RuntimeStatusResponse(BaseModel): deployment_state: DeploymentState generated_config: RuntimeGeneratedConfigStatus active_config: ActiveRuntimeConfig + version_agreement: VersionAgreement latest_validation: RuntimeOperationSnapshot | None = None latest_reload: RuntimeOperationSnapshot | None = None diff --git a/src/backend/app/services/config_renderer.py b/src/backend/app/services/config_renderer.py index 90eb3ff1..d97e833f 100644 --- a/src/backend/app/services/config_renderer.py +++ b/src/backend/app/services/config_renderer.py @@ -8,6 +8,7 @@ from jinja2 import Environment, PackageLoader, StrictUndefined +from app.constants.versions import CRS_VERSION, EXPECTED_CONFIG_VERSION from app.coraza_syntax import VARIABLES_PATTERN, quoted_value_error from app.haproxy_syntax import host_error from app.models.custom_rule import RuleOperator, RulePhase @@ -483,7 +484,17 @@ def render_haproxy_cfg_multi(vhost_contexts: list[HaproxyRenderContext]) -> str: def _render_haproxy_routes(routes: tuple[HaproxyRoute, ...]) -> str: template = _ENVIRONMENT.get_template("haproxy.cfg.j2") - return template.render(routes=routes, default_waf_app=CORAZA_DEFAULT_APP) + return template.render( + routes=routes, + default_waf_app=CORAZA_DEFAULT_APP, + config_version=EXPECTED_CONFIG_VERSION, + ) + + +def _crs_setup_version(version: str) -> int: + """CRS setup version integer derived from a release version (`4.25.0` -> 4250).""" + major, minor, patch = (int(part) for part in version.split(".")) + return major * 1000 + minor * 10 + patch def render_crs_setup(policy: CrsPolicyRenderContext) -> str: @@ -494,7 +505,12 @@ def render_crs_setup(policy: CrsPolicyRenderContext) -> str: if policy.enforcement_mode == PolicyEnforcementMode.detect_only else "On" ) - return template.render(policy=policy, sec_rule_engine=sec_rule_engine) + return template.render( + policy=policy, + sec_rule_engine=sec_rule_engine, + crs_version=CRS_VERSION, + crs_setup_version=_crs_setup_version(CRS_VERSION), + ) def render_coraza_spoa_yaml(app_names: tuple[str, ...], log_level: str) -> str: diff --git a/src/backend/app/services/config_version.py b/src/backend/app/services/config_version.py new file mode 100644 index 00000000..30ab2ef4 --- /dev/null +++ b/src/backend/app/services/config_version.py @@ -0,0 +1,111 @@ +"""Version agreement between this image and the config/CRS it runs with. + +Issue #312: `haproxy/coraza.cfg` is bind-mounted from the operator's extracted +release kit, so a tag-only upgrade runs a new image against an old file. A +pre-beta.5 `coraza.cfg` sends `app=str(default)`, so every request is inspected +by the default application instead of its vhost's `policy_` — no error, +just the wrong policy. The stamp check below turns that into a loud, nameable +failure. +""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from pathlib import Path + +from app.config import settings +from app.constants.versions import EXPECTED_CONFIG_VERSION +from app.version import APP_VERSION + +CONFIG_VERSION_STAMP = "# guard-proxy-config-version:" + +_STAMP_RE = re.compile(rf"^{re.escape(CONFIG_VERSION_STAMP)}\s*(\d+)\s*$", re.MULTILINE) +# Release tags use the GitHub spelling `v0.1.0-beta.10`; the backend reports +# the PEP 440 spelling `0.1.0b10`. +_TAG_RE = re.compile(r"^v?(\d+)\.(\d+)\.(\d+)(?:-(alpha|beta|rc)\.?(\d+))?$") +_TAG_SUFFIX = {"alpha": "a", "beta": "b", "rc": "rc"} + + +@dataclass(frozen=True) +class ConfigVersionCheck: + """Result of comparing the mounted seed config with this image.""" + + path: Path + present: bool + version: int | None + detail: str | None + + @property + def ok(self) -> bool: + return self.detail is None + + +def check_seed_config(path: Path | None = None) -> ConfigVersionCheck: + """Compare the bind-mounted seed `coraza.cfg` with what this image expects. + + A missing file is not a failure: local runs (and `pytest`) have no release + kit mounted, and a wrong mount path fails loudly in HAProxy instead. A + present file without a current stamp is the half-done upgrade of #312. + """ + target = path or Path(settings.seed_coraza_config_path) + try: + text = target.read_text(encoding="utf-8", errors="replace") + except OSError: + return ConfigVersionCheck(path=target, present=False, version=None, detail=None) + + match = _STAMP_RE.search(text) + version = int(match.group(1)) if match else None + if version == EXPECTED_CONFIG_VERSION: + return ConfigVersionCheck( + path=target, present=True, version=version, detail=None + ) + + if version is None: + detail = ( + f"{target} has no '{CONFIG_VERSION_STAMP} N' stamp, so it predates " + "this image. Replace the haproxy/ files with the ones from the " + "release named by GUARD_PROXY_IMAGE_TAG." + ) + elif version < EXPECTED_CONFIG_VERSION: + detail = ( + f"{target} is config version {version}, but this image expects " + f"{EXPECTED_CONFIG_VERSION}: it predates this image. Replace the " + "haproxy/ files with the ones from the release named by " + "GUARD_PROXY_IMAGE_TAG." + ) + else: + detail = ( + f"{target} is config version {version}, newer than the " + f"{EXPECTED_CONFIG_VERSION} this image understands: the haproxy/ " + "files belong to a newer release than GUARD_PROXY_IMAGE_TAG." + ) + return ConfigVersionCheck(path=target, present=True, version=version, detail=detail) + + +def tag_to_pep440(tag: str) -> str | None: + """Map a release tag (`v0.1.0-beta.10`) to PEP 440 (`0.1.0b10`).""" + match = _TAG_RE.match(tag.strip()) + if match is None: + return None + major, minor, patch, stage, number = match.groups() + suffix = "" if stage is None else f"{_TAG_SUFFIX[stage]}{int(number)}" + return f"{major}.{minor}.{patch}{suffix}" + + +def configured_image_tag() -> str | None: + """The image tag the operator selected, or None when it is not configured.""" + tag = (settings.guard_proxy_image_tag or "").strip() + return tag or None + + +def image_tag_warning() -> str | None: + """Warn when the configured image tag and this backend's version disagree.""" + tag = configured_image_tag() + if tag is None or tag_to_pep440(tag) == APP_VERSION: + return None + return ( + f"GUARD_PROXY_IMAGE_TAG is {tag!r} but this backend is version " + f"{APP_VERSION}: the stack mixes releases. Set the tag to the release " + "the images were pulled from." + ) diff --git a/src/backend/app/services/runtime_status_service.py b/src/backend/app/services/runtime_status_service.py index 2c0b4720..6f47113e 100644 --- a/src/backend/app/services/runtime_status_service.py +++ b/src/backend/app/services/runtime_status_service.py @@ -8,6 +8,7 @@ from sqlalchemy.orm import Session, selectinload from app.config import settings +from app.constants.versions import CRS_VERSION from app.models.custom_rule import CustomRule from app.models.policy import Policy from app.models.policy_binding import PolicyBinding @@ -27,6 +28,7 @@ RuntimeGeneratedConfigStatus, RuntimeOperationSnapshot, RuntimeStatusResponse, + VersionAgreement, ) from app.services.config_apply import ( CORAZA_SPOA_YAML, @@ -34,6 +36,12 @@ calculate_checksum, ) from app.services.config_generator import generate +from app.services.config_version import ( + check_seed_config, + configured_image_tag, + image_tag_warning, +) +from app.version import APP_VERSION _FRONTEND_CONTRACT_VERSION = "1" @@ -68,6 +76,7 @@ def classify_active_release( def _active_config_warning( haproxy_source: HaproxyConfigSource, coraza_source: CorazaConfigSource, + seed_config_detail: str | None = None, ) -> str | None: parts: list[str] = [] if haproxy_source == "seed": @@ -85,6 +94,8 @@ def _active_config_warning( "request is inspected at paranoia level 1 in detect-only mode and " "per-policy settings are ignored." ) + if seed_config_detail is not None: + parts.append(seed_config_detail) return " ".join(parts) or None @@ -106,6 +117,12 @@ def get_runtime_status(self) -> RuntimeStatusResponse: deployment_state=deployment_state, generated_config=generated_config, active_config=self._build_active_config_status(), + version_agreement=VersionAgreement( + app_version=APP_VERSION, + configured_image_tag=configured_image_tag(), + crs_version=CRS_VERSION, + warning=image_tag_warning(), + ), latest_validation=latest_validation, latest_reload=latest_reload, ) @@ -120,12 +137,19 @@ def _build_active_config_status(self) -> ActiveRuntimeConfig: """ current_link = Path(settings.runtime_generated_config_root) / "current" haproxy_source, coraza_source = classify_active_release(current_link) - degraded = haproxy_source != "generated" or coraza_source != "generated" + seed_config_detail = check_seed_config().detail + degraded = ( + haproxy_source != "generated" + or coraza_source != "generated" + or seed_config_detail is not None + ) return ActiveRuntimeConfig( haproxy_source=haproxy_source, coraza_source=coraza_source, degraded=degraded, - warning=_active_config_warning(haproxy_source, coraza_source), + warning=_active_config_warning( + haproxy_source, coraza_source, seed_config_detail + ), ) def _build_generated_config_status(self) -> RuntimeGeneratedConfigStatus: diff --git a/src/backend/app/templates/crs-setup.conf.j2 b/src/backend/app/templates/crs-setup.conf.j2 index c0e0b832..90d81a10 100644 --- a/src/backend/app/templates/crs-setup.conf.j2 +++ b/src/backend/app/templates/crs-setup.conf.j2 @@ -5,12 +5,12 @@ SecRuleEngine {{ sec_rule_engine }} -SecAction "id:900000,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/4.25.0',setvar:tx.blocking_paranoia_level={{ policy.paranoia_level }}" +SecAction "id:900000,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/{{ crs_version }}',setvar:tx.blocking_paranoia_level={{ policy.paranoia_level }}" -SecAction "id:900005,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/4.25.0',setvar:tx.detection_paranoia_level={{ policy.paranoia_level }}" +SecAction "id:900005,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/{{ crs_version }}',setvar:tx.detection_paranoia_level={{ policy.paranoia_level }}" -SecAction "id:900110,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/4.25.0',setvar:tx.inbound_anomaly_score_threshold={{ policy.inbound_anomaly_threshold }},setvar:tx.outbound_anomaly_score_threshold={{ policy.outbound_anomaly_threshold }}" +SecAction "id:900110,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/{{ crs_version }}',setvar:tx.inbound_anomaly_score_threshold={{ policy.inbound_anomaly_threshold }},setvar:tx.outbound_anomaly_score_threshold={{ policy.outbound_anomaly_threshold }}" -SecAction "id:900200,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/4.25.0',setvar:'tx.allowed_methods=GET HEAD POST PUT DELETE PATCH OPTIONS'" +SecAction "id:900200,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/{{ crs_version }}',setvar:'tx.allowed_methods=GET HEAD POST PUT DELETE PATCH OPTIONS'" -SecAction "id:900990,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/4.25.0',setvar:tx.crs_setup_version=4250" +SecAction "id:900990,phase:1,pass,nolog,tag:'OWASP_CRS',ver:'OWASP_CRS/{{ crs_version }}',setvar:tx.crs_setup_version={{ crs_setup_version }}" diff --git a/src/backend/app/templates/haproxy.cfg.j2 b/src/backend/app/templates/haproxy.cfg.j2 index 1736ae94..344beb5b 100644 --- a/src/backend/app/templates/haproxy.cfg.j2 +++ b/src/backend/app/templates/haproxy.cfg.j2 @@ -1,3 +1,4 @@ +# guard-proxy-config-version: {{ config_version }} # Guard Proxy HAProxy configuration. # Generated from the policy database; manual edits are overwritten on the # next config apply. diff --git a/src/backend/app/version.py b/src/backend/app/version.py new file mode 100644 index 00000000..8d9cb114 --- /dev/null +++ b/src/backend/app/version.py @@ -0,0 +1,34 @@ +"""The backend's own version, resolved once from a single source of truth.""" + +from __future__ import annotations + +import tomllib +from importlib.metadata import PackageNotFoundError +from importlib.metadata import version as _package_version +from pathlib import Path + + +def resolve_app_version() -> str: + """Return the app version, reading from a single source of truth. + + Prefers installed package metadata (present in dev venvs where the + project is installed editable). The production Docker image installs + only dependencies as a distinct layer and copies app source on top + without installing the project itself, so no dist-info exists there — + fall back to parsing `pyproject.toml` directly in that case. + """ + try: + return _package_version("guard-proxy-backend") + except PackageNotFoundError: + pyproject_path = Path(__file__).resolve().parent.parent / "pyproject.toml" + try: + with pyproject_path.open("rb") as f: + version = tomllib.load(f)["project"]["version"] + if not isinstance(version, str): + raise KeyError("project.version is not a string") + return version + except (OSError, KeyError): # pragma: no cover - defensive fallback + return "0.0.0+unknown" + + +APP_VERSION = resolve_app_version() diff --git a/src/backend/tests/integration/test_config_router.py b/src/backend/tests/integration/test_config_router.py index bc8390b0..bfccae49 100644 --- a/src/backend/tests/integration/test_config_router.py +++ b/src/backend/tests/integration/test_config_router.py @@ -288,3 +288,48 @@ def test_apply_validation_failure_records_failed_validation_only( assert body["deployment_state"] == "never_deployed" assert body["latest_validation"]["status"] == "failed" assert body["latest_reload"] is None + + +# --------------------------------------------------------------------------- +# Seed config version stamp (issue #312) +# --------------------------------------------------------------------------- + + +def test_apply_refuses_when_the_seed_config_is_stale( + client: TestClient, + admin_token: dict[str, str], + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + stale = tmp_path / "coraza.cfg" + stale.write_text("[coraza]\nspoe-message coraza-req\n args app=str(default)\n") + monkeypatch.setattr(settings, "seed_coraza_config_path", str(stale)) + + resp = client.post("/config/apply", headers=admin_token) + + assert resp.status_code == 409 + assert str(stale) in resp.json()["detail"] + + +def test_apply_succeeds_with_the_current_seed_config( + client: TestClient, + admin_token: dict[str, str], + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + settings, "runtime_generated_config_root", str(tmp_path / "generated") + ) + monkeypatch.setattr( + "app.services.config_apply._validate_haproxy", _mock_validate_ok + ) + monkeypatch.setattr("app.services.config_apply._reload_haproxy", _mock_reload_ok) + monkeypatch.setattr( + settings, + "seed_coraza_config_path", + str(Path(__file__).resolve().parents[3] / "configs/haproxy/coraza.cfg"), + ) + + resp = client.post("/config/apply", headers=admin_token) + + assert resp.status_code == 200 diff --git a/src/backend/tests/integration/test_health_router.py b/src/backend/tests/integration/test_health_router.py index a82b8e50..ee1f3c9d 100644 --- a/src/backend/tests/integration/test_health_router.py +++ b/src/backend/tests/integration/test_health_router.py @@ -157,3 +157,64 @@ def broken_db() -> Iterator[Session]: data = response.json() assert data["checks"]["database"]["status"] == "error" assert data["checks"]["runtime_config"]["status"] == "error" + + +# --------------------------------------------------------------------------- +# Seed config version stamp (issue #312) +# --------------------------------------------------------------------------- + + +def test_health_reports_degraded_for_a_stale_seed_config( + client: TestClient, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + stale = tmp_path / "coraza.cfg" + stale.write_text("[coraza]\nspoe-message coraza-req\n args app=str(default)\n") + monkeypatch.setattr(main_module.settings, "seed_coraza_config_path", str(stale)) + + response = client.get("/health") + + assert response.status_code == 200 + data = response.json() + assert data["status"] == "degraded" + assert str(stale) in data["detail"] + + +def test_health_is_healthy_for_the_current_seed_config( + client: TestClient, + monkeypatch: pytest.MonkeyPatch, +) -> None: + repo_config = ( + Path(main_module.__file__).resolve().parents[3] / "configs/haproxy/coraza.cfg" + ) + monkeypatch.setattr( + main_module.settings, "seed_coraza_config_path", str(repo_config) + ) + + response = client.get("/health") + + assert response.status_code == 200 + assert response.json()["status"] == "healthy" + + +def test_ready_returns_503_for_a_stale_seed_config( + client: TestClient, + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """The release kit healthcheck calls /ready, so a tag-only upgrade fails it.""" + stale = tmp_path / "coraza.cfg" + stale.write_text("[coraza]\n") + monkeypatch.setattr( + main_module.settings, "runtime_generated_config_root", str(tmp_path) + ) + monkeypatch.setattr(main_module.settings, "seed_coraza_config_path", str(stale)) + + response = client.get("/ready") + + assert response.status_code == 503 + data = response.json() + assert data["checks"]["seed_config"]["status"] == "error" + assert str(stale) in data["checks"]["seed_config"]["detail"] + assert data["checks"]["database"]["status"] == "ok" diff --git a/src/backend/tests/integration/test_runtime_status_router.py b/src/backend/tests/integration/test_runtime_status_router.py index fafcbb87..365ba006 100644 --- a/src/backend/tests/integration/test_runtime_status_router.py +++ b/src/backend/tests/integration/test_runtime_status_router.py @@ -4,12 +4,14 @@ from sqlalchemy.orm import Session from app.config import settings +from app.constants.versions import CRS_VERSION from app.models.runtime_operation import ( RuntimeOperation, RuntimeOperationStatus, RuntimeOperationType, ) from app.models.vhost import VHost +from app.version import APP_VERSION def test_runtime_status_requires_auth(client: TestClient) -> None: @@ -231,3 +233,58 @@ def test_active_config_reports_coraza_fallback_warning( assert body["active_config"]["coraza_source"] == "fallback" assert body["active_config"]["degraded"] is True assert "detect-only" in body["active_config"]["warning"] + + +def test_runtime_status_reports_version_agreement( + client: TestClient, + admin_token: dict[str, str], + tmp_path: Path, + monkeypatch, +) -> None: + _point_runtime_root_at(monkeypatch, tmp_path / "generated") + monkeypatch.setattr(settings, "guard_proxy_image_tag", None) + + body = client.get("/runtime/status", headers=admin_token).json() + + agreement = body["version_agreement"] + assert agreement["configured_image_tag"] is None + assert agreement["crs_version"] == CRS_VERSION + assert agreement["app_version"] == APP_VERSION + assert agreement["warning"] is None + + +def test_runtime_status_warns_when_the_image_tag_and_backend_version_skew( + client: TestClient, + admin_token: dict[str, str], + tmp_path: Path, + monkeypatch, +) -> None: + _point_runtime_root_at(monkeypatch, tmp_path / "generated") + monkeypatch.setattr(settings, "guard_proxy_image_tag", "v0.0.1") + + body = client.get("/runtime/status", headers=admin_token).json() + + assert "v0.0.1" in body["version_agreement"]["warning"] + + +def test_active_config_warns_about_a_stale_seed_config( + client: TestClient, + admin_token: dict[str, str], + tmp_path: Path, + monkeypatch, +) -> None: + runtime_root = tmp_path / "generated" + release = runtime_root / "releases" / "abc" + release.mkdir(parents=True) + (release / "haproxy.cfg").write_text("global\n", encoding="utf-8") + (release / "coraza-spoa.yaml").write_text("applications: []\n") + current = _point_runtime_root_at(monkeypatch, runtime_root) + current.symlink_to("releases/abc") + stale = tmp_path / "coraza.cfg" + stale.write_text("[coraza]\n") + monkeypatch.setattr(settings, "seed_coraza_config_path", str(stale)) + + body = client.get("/runtime/status", headers=admin_token).json() + + assert body["active_config"]["degraded"] is True + assert str(stale) in body["active_config"]["warning"] diff --git a/src/backend/tests/unit/test_app_version.py b/src/backend/tests/unit/test_app_version.py index 9b042c9e..c328b3d8 100644 --- a/src/backend/tests/unit/test_app_version.py +++ b/src/backend/tests/unit/test_app_version.py @@ -5,10 +5,12 @@ import pytest -import app.main as main_module +import app.version as version_module _PYPROJECT_VERSION = tomllib.loads( - (Path(main_module.__file__).resolve().parent.parent / "pyproject.toml").read_text() + ( + Path(version_module.__file__).resolve().parent.parent / "pyproject.toml" + ).read_text() )["project"]["version"] @@ -22,16 +24,16 @@ def package_metadata_missing() -> Iterator[None]: def _raise(name: str) -> str: raise PackageNotFoundError(name) - original = main_module._package_version - main_module._package_version = _raise + original = version_module._package_version + version_module._package_version = _raise try: yield finally: - main_module._package_version = original + version_module._package_version = original def test_resolves_from_installed_package_metadata_when_present() -> None: - assert main_module._resolve_app_version() == main_module._package_version( + assert version_module.resolve_app_version() == version_module._package_version( "guard-proxy-backend" ) @@ -39,4 +41,4 @@ def test_resolves_from_installed_package_metadata_when_present() -> None: def test_falls_back_to_pyproject_toml_when_package_metadata_missing( package_metadata_missing: None, ) -> None: - assert main_module._resolve_app_version() == _PYPROJECT_VERSION + assert version_module.resolve_app_version() == _PYPROJECT_VERSION diff --git a/src/backend/tests/unit/test_config_renderer_crs_setup.py b/src/backend/tests/unit/test_config_renderer_crs_setup.py index 08790561..e101eb1b 100644 --- a/src/backend/tests/unit/test_config_renderer_crs_setup.py +++ b/src/backend/tests/unit/test_config_renderer_crs_setup.py @@ -2,6 +2,7 @@ import pytest +from app.constants.versions import CRS_VERSION from app.models.policy import PolicyEnforcementMode from app.services.config_renderer import CrsPolicyRenderContext, render_crs_setup @@ -75,3 +76,14 @@ def test_crs_setup_pl1_baseline_matches_reference_modulo_engine_line() -> None: rendered_without_engine = rendered.replace("SecRuleEngine On\n\n", "") assert _normalise_config(rendered_without_engine) == _normalise_config(reference) + + +def test_crs_setup_takes_the_crs_version_from_the_constants() -> None: + """No CRS version literal in the template: the constant supplies it (#316).""" + rendered = render_crs_setup(_policy()) + + assert f"ver:'OWASP_CRS/{CRS_VERSION}'" in rendered + assert "setvar:tx.crs_setup_version=4250" in rendered + assert "{{" not in rendered + template = (REPO_ROOT / "src/backend/app/templates/crs-setup.conf.j2").read_text() + assert "OWASP_CRS/4." not in template diff --git a/src/backend/tests/unit/test_config_version.py b/src/backend/tests/unit/test_config_version.py new file mode 100644 index 00000000..f62ba6ff --- /dev/null +++ b/src/backend/tests/unit/test_config_version.py @@ -0,0 +1,173 @@ +from __future__ import annotations + +import re +from pathlib import Path + +import pytest + +from app.config import settings +from app.constants.versions import CRS_VERSION, EXPECTED_CONFIG_VERSION +from app.services.config_version import ( + CONFIG_VERSION_STAMP, + check_seed_config, + image_tag_warning, + tag_to_pep440, +) +from app.version import APP_VERSION + + +def _repo_root() -> Path: + for candidate in Path(__file__).resolve().parents: + if (candidate / ".git").exists(): + return candidate + raise FileNotFoundError("repository root not found") + + +REPO_ROOT = _repo_root() + +# What the release kit shipped before the stamp existed: no version header, +# hardcoded `app=str(default)` so per-vhost policies are never selected. +STALE_CORAZA_CFG = """\ +[coraza] + +spoe-agent coraza-agent + groups coraza-req + option var-prefix coraza + option set-on-error error + +spoe-message coraza-req + args app=str(default) id=unique-id method=method path=path +""" + + +@pytest.mark.parametrize( + "relative_path", + [ + "configs/haproxy/coraza.cfg", + "configs/haproxy/haproxy.cfg", + "release/haproxy/coraza.cfg", + "release/haproxy/haproxy.cfg", + ], +) +def test_shipped_configs_carry_the_expected_stamp(relative_path: str) -> None: + check = check_seed_config(REPO_ROOT / relative_path) + + assert check.present + assert check.ok + assert check.version == EXPECTED_CONFIG_VERSION + + +def test_config_without_stamp_is_stale(tmp_path: Path) -> None: + stale = tmp_path / "coraza.cfg" + stale.write_text(STALE_CORAZA_CFG) + + check = check_seed_config(stale) + + assert check.present + assert not check.ok + assert check.version is None + assert str(stale) in check.detail + assert CONFIG_VERSION_STAMP in check.detail + assert "Replace the haproxy/ files" in check.detail + + +def test_older_stamp_is_stale(tmp_path: Path) -> None: + stale = tmp_path / "coraza.cfg" + stale.write_text(f"{CONFIG_VERSION_STAMP} 0\n\n[coraza]\n") + + check = check_seed_config(stale) + + assert not check.ok + assert check.version == 0 + assert "config version 0" in check.detail + assert str(EXPECTED_CONFIG_VERSION) in check.detail + + +def test_newer_stamp_is_stale(tmp_path: Path) -> None: + """Config files from a newer release are as wrong as older ones.""" + future = tmp_path / "coraza.cfg" + future.write_text(f"{CONFIG_VERSION_STAMP} {EXPECTED_CONFIG_VERSION + 1}\n") + + check = check_seed_config(future) + + assert not check.ok + assert "newer than" in check.detail + + +def test_missing_seed_config_is_not_an_error(tmp_path: Path) -> None: + """No release kit mounted (local runs, pytest) must not degrade health.""" + check = check_seed_config(tmp_path / "absent.cfg") + + assert not check.present + assert check.ok + assert check.detail is None + + +def test_check_reads_the_configured_seed_path( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + seed = tmp_path / "coraza.cfg" + seed.write_text(STALE_CORAZA_CFG) + monkeypatch.setattr(settings, "seed_coraza_config_path", str(seed)) + + check = check_seed_config() + + assert check.path == seed + assert not check.ok + + +@pytest.mark.parametrize( + ("tag", "expected"), + [ + ("v0.1.0-beta.10", "0.1.0b10"), + ("0.1.0-beta.10", "0.1.0b10"), + ("v0.1.0-rc.1", "0.1.0rc1"), + ("v0.1.0-alpha.2", "0.1.0a2"), + ("v0.1.0", "0.1.0"), + ("nightly", None), + ("v0.1", None), + ], +) +def test_tag_to_pep440(tag: str, expected: str | None) -> None: + assert tag_to_pep440(tag) == expected + + +def test_image_tag_warning_is_silent_when_unset( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "guard_proxy_image_tag", None) + + assert image_tag_warning() is None + + +def test_image_tag_warning_is_silent_when_it_matches( + monkeypatch: pytest.MonkeyPatch, +) -> None: + tag = "v" + re.sub(r"b(\d+)$", r"-beta.\1", APP_VERSION) + monkeypatch.setattr(settings, "guard_proxy_image_tag", tag) + + assert tag_to_pep440(tag) == APP_VERSION + assert image_tag_warning() is None + + +def test_image_tag_warning_names_a_skewed_tag( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr(settings, "guard_proxy_image_tag", "v0.0.1") + + warning = image_tag_warning() + + assert warning is not None + assert "v0.0.1" in warning + assert APP_VERSION in warning + + +def test_crs_version_matches_the_submodule() -> None: + """The generated CRS metadata must describe the rules the submodule ships.""" + example = REPO_ROOT / "configs/coraza/crs/crs-setup.conf.example" + if not example.exists(): + pytest.skip("configs/coraza/crs submodule is not initialized") + + header = "\n".join(example.read_text(encoding="utf-8").splitlines()[:4]) + + assert f"# OWASP CRS ver.{CRS_VERSION}" in header From 4e2f31365f83dfca32a1e0aa9ffea835dc0462c2 Mon Sep 17 00:00:00 2001 From: Konrad Kasperkiewicz <119067436+bihius@users.noreply.github.com> Date: Wed, 21 Oct 2026 22:08:56 +0200 Subject: [PATCH 2/2] fix(backend): reject wildcard CORS origins and hide readiness path Reject CORS_ORIGINS='*' at startup since credentialed requests are always enabled. Return a generic /ready error detail and log the runtime config path server-side. Document global /logs access and the X-Forwarded-For trust dependency on HAProxy. --- CHANGELOG.md | 14 ++++++++++ docs/architecture.md | 19 ++++++++++--- src/backend/app/config.py | 11 ++++++++ src/backend/app/main.py | 5 +++- .../tests/integration/test_health_router.py | 15 ++++++++--- src/backend/tests/unit/test_config.py | 27 +++++++++++++++++++ 6 files changed, 82 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 15157944..b47283b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,20 @@ spelling of the same version (e.g. `0.1.0b2` for `0.1.0-beta.2`). this release keep a NULL contact and renew with the previous `admin@` fallback. Documented in `docs/architecture.md`. +### Changed + +- `CORS_ORIGINS` now rejects `*` at startup: the API always sends credentialed + requests, so a wildcard origin is invalid (and version-dependent in browsers). +- `GET /ready` no longer returns the runtime config path in its error detail; + the path is logged server-side instead. + +### Documented + +- `GET /logs` is global to all authenticated users, including `viewer`, by + design; per-vhost log isolation is not implemented. +- The rate-limit key trusts `X-Forwarded-For` only because HAProxy overwrites + it and the backend is not published on a host port. + ## [0.1.0-beta.10] - 2026-10-15 ### Fixed diff --git a/docs/architecture.md b/docs/architecture.md index a5ac8fc2..85fe34d8 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -151,7 +151,11 @@ events are attributed to the vhost's policy. nullable `vhost_id` and `policy_id` snapshots on the log row. `GET /logs` policy filtering uses the stored `logs.policy_id`, so results reflect the policy assignment captured at event-ingest time, not a later vhost reassignment. -6. `GET /logs` exposes stored events for the admin panel log viewer. +6. `GET /logs` exposes stored events for the admin panel log viewer. The view + is global: every authenticated user, `viewer` included, sees events from + all vhosts (including `raw_context`). This matches the read-only role + definition and is intentional for the single-tenant MVP — per-vhost log + isolation is not implemented. ### Log Retention `LOG_RETENTION_DAYS` (default `30`) controls how long `Log` rows are kept. @@ -339,9 +343,15 @@ http-request set-header X-Forwarded-For %[src] This overwrites any client-supplied `X-Forwarded-For` value with the real source IP before the request reaches the backend, preventing header-spoofing bypass of -the rate limit. The backend's key function reads the first (and only) XFF entry -and falls back to the socket peer when the header is absent (direct connections in -development and test). +the rate limit. The backend's key function reads the right-most XFF entry (the +address the nearest proxy observed) and falls back to the socket peer when the +header is absent (direct connections in development and test). Client-supplied +entries to the left of that one cannot shift the bucket. + +**Security dependency:** with HAProxy overwriting the header and the backend +not published on a host port (it is only reachable on `gp_internal`), the XFF +value can be trusted. If anything else can reach the backend directly, it can +forge the header and bypass or poison the per-IP rate limit. ### Timing-attack mitigation @@ -360,6 +370,7 @@ CORS is restricted to the methods and headers the panel actually uses: array); the built-in default lists only local dev origins (`localhost`/`127.0.0.1` on ports 3000, 5173, 5174) and must be overridden with the production panel origin(s) via environment configuration before deployment. + `*` is rejected at startup because credentials are always enabled. ## Deployment diff --git a/src/backend/app/config.py b/src/backend/app/config.py index 4b507d5b..3d6fedf0 100644 --- a/src/backend/app/config.py +++ b/src/backend/app/config.py @@ -250,6 +250,17 @@ def parse_cors_origins(cls, value: str | list[str]) -> list[str]: return value + @model_validator(mode="after") + def cors_origins_must_not_be_wildcard(self) -> "Settings": + if "*" in self.cors_origins: + # The API always sends credentials (HTTP-only refresh cookie), which + # browsers reject together with a "*" origin. + raise ValueError( + "CORS_ORIGINS must list explicit origins: '*' cannot be combined " + "with credentialed requests." + ) + return self + @model_validator(mode="after") def validate_cookie_settings(self) -> "Settings": if ( diff --git a/src/backend/app/main.py b/src/backend/app/main.py index 787b5999..a4a9b500 100644 --- a/src/backend/app/main.py +++ b/src/backend/app/main.py @@ -253,9 +253,12 @@ def readiness_check(db: Session = Depends(get_db)) -> JSONResponse: if config_root.is_dir() and os.access(config_root, os.W_OK | os.X_OK): checks["runtime_config"] = {"status": "ok"} else: + logger.error( + "Readiness check failed: %s is not a writable directory", config_root + ) checks["runtime_config"] = { "status": "error", - "detail": f"{config_root} is not a writable directory", + "detail": "runtime config directory is not writable", } all_ok = False diff --git a/src/backend/tests/integration/test_health_router.py b/src/backend/tests/integration/test_health_router.py index a82b8e50..b9b643db 100644 --- a/src/backend/tests/integration/test_health_router.py +++ b/src/backend/tests/integration/test_health_router.py @@ -111,21 +111,28 @@ def broken_db() -> Iterator[Session]: def test_ready_returns_503_when_config_dir_missing( client: TestClient, monkeypatch: pytest.MonkeyPatch, + caplog: pytest.LogCaptureFixture, ) -> None: - """Missing config dir → 503 with runtime_config check reporting error.""" + """Missing config dir → 503 with a generic detail; the path stays in logs.""" + config_root = "/nonexistent/guard-proxy-test-path" monkeypatch.setattr( main_module.settings, "runtime_generated_config_root", - "/nonexistent/guard-proxy-test-path", + config_root, ) - response = client.get("/ready") + with caplog.at_level("ERROR"): + response = client.get("/ready") assert response.status_code == 503 data = response.json() assert data["status"] == "not ready" assert data["checks"]["runtime_config"]["status"] == "error" - assert "detail" in data["checks"]["runtime_config"] + assert data["checks"]["runtime_config"]["detail"] == ( + "runtime config directory is not writable" + ) + assert config_root not in response.text + assert config_root in caplog.text # DB check should still have passed assert data["checks"]["database"]["status"] == "ok" diff --git a/src/backend/tests/unit/test_config.py b/src/backend/tests/unit/test_config.py index 13e13219..8ffcb7c9 100644 --- a/src/backend/tests/unit/test_config.py +++ b/src/backend/tests/unit/test_config.py @@ -361,6 +361,33 @@ def test_geoip_database_url_empty_raises() -> None: ) +# --------------------------------------------------------------------------- +# cors_origins +# --------------------------------------------------------------------------- + + +def test_cors_origins_wildcard_is_rejected() -> None: + """`*` cannot be combined with the credentialed requests the API sends.""" + with pytest.raises( + ValidationError, + match=r"CORS_ORIGINS must list explicit origins", + ): + _make_settings( + JWT_SECRET_KEY=_VALID_JWT_SECRET, + LOG_INGEST_SHARED_SECRET=_VALID_INGEST_SECRET, + CORS_ORIGINS='["https://panel.example.com", "*"]', + ) + + +def test_cors_origins_explicit_values_are_accepted() -> None: + s = _make_settings( + JWT_SECRET_KEY=_VALID_JWT_SECRET, + LOG_INGEST_SHARED_SECRET=_VALID_INGEST_SECRET, + CORS_ORIGINS='["https://panel.example.com"]', + ) + assert getattr(s, "cors_origins") == ["https://panel.example.com"] + + # --------------------------------------------------------------------------- # acme_server_url # ---------------------------------------------------------------------------