fix: weiger opstart bij onveilige SECRET_KEY in productie

[anneschuth]

Kwetsbaarheid

opi/core/config.py definieerde SECRET_KEY met een vaste default die in de
publieke repository staat: default-secret-key-for-development-change-in-production.

Die sleutel ondertekent twee dingen:

• de sessiecookie via Starlette's SessionMiddleware (opi/server.py, SessionMiddleware(secret_key=settings.SECRET_KEY))
• de websocket log-stream sessie via itsdangerous.TimestampSigner (opi/api/logs_websocket_router.py)

De enige aanwezige controle was if not secret_key (lege string). De bekende
default is geen lege string en kwam er dus ongehinderd doorheen. Niets faalde
gesloten wanneer SECRET_KEY ongezet of de default was terwijl de applicatie
productie-achtig draaide.

Gevolg: een aanvaller die de publieke constante kent ondertekent zelf een
session-cookie met een willekeurige user en omzeilt daarmee zowel de
HTTP-authenticatie als de websocket log-stream-authenticatie. Volledige
authenticatiebypass.

Bevestigd dat het productie-secret (bootstrap/.../odcn-production/operations-manager-env-secrets.yaml)
momenteel geen SECRET_KEY zet. Productie draait dus feitelijk op de publieke
default totdat operators dit instellen (zie onder).

Fix: fail-closed

• Nieuwe module opi/core/secret_key.py met de pure functie validate_secret_key(secret_key, debug),
  zonder pydantic-afhankelijkheid zodat ze los te testen is.
• validate_secret_key weigert (raises InsecureSecretKeyError) wanneer
  SECRET_KEY ongezet/leeg is, gelijk is aan de bekende dev-default, of korter
  is dan 32 tekens, en DEBUG=False.
• DEBUG is het bestaande dev/prod-signaal in de codebase (server.py:265,
  web/router.py:1980); er is geen nieuwe env var geintroduceerd. .env levert
  DEBUG=false, productie zet DEBUG=False.
• In DEBUG-modus blijft de default toegestaan, maar met een luide
  WARNING-logregel zodat lokale ontwikkeling blijft werken.
• Aangeroepen vanuit een model_validator(mode="after") op Settings, dus de
  applicatie weigert te booten voordat er verkeer wordt bediend.
• De default wordt niet stil door een willekeurige waarde vervangen: dat zou
  sessies onvoorspelbaar invalideren bij herstart en de misconfiguratie
  maskeren. Het doel is luid falen.

Actie voor operators

Operators moeten een sterke, unieke SECRET_KEY (>= 32 tekens) meegeven via het
productie-secret (rig-system operations-manager env secret). Zonder die
sleutel weigert OPI in productie (DEBUG=False) bewust op te starten.

Test plan

• uv run pytest --noconftest tests/test_secret_key_failclosed.py (11 passed)
• Rood-groen op de pure functie: raised voor ongezet / dev-default / te kort
  in productiemodus; slaagt voor een sterke sleutel of in DEBUG-modus;
  controleert de WARNING-logregel in DEBUG
• uv run ruff check + uv run ruff format schoon op gewijzigde bestanden
• uv run pyright schoon op gewijzigde bestanden (0 errors)

Opmerking: uv run pytest tests/ faalt al bij collectie voor de hele suite door
een pre-existing omgevingsincompatibiliteit (Python 3.14.0b4 + gepinde pydantic
2.12.5, import fastapi raised). Dat staat los van deze wijziging en is hier
niet aangeraakt. De nieuwe test mijdt die importketen via --noconftest en de
losse secret_key-module.

[uittenbroekrobbert]

LGTM. Failure-semantics zijn correct (validator op Settings()-instantiatie, op module-import, voordat uvicorn bind) en de allowlist-keuze op CLUSTER_MANAGER is beter dan een DEBUG-gate (geverifieerd: prod-configmap zet alleen DEBUG_MODE als string, geen boolean DEBUG, dus DEBUG-gate zou prod nooit raken).

Drie niet-blokkerende observaties

1. Lengte-drempel zonder entropie-check

Bewuste vereenvoudiging die ik onderschrijf (entropie-meting in een opstart-validator wordt brittle, en de echte aanval is de gepubliceerde constante zelf). "x"*32 passeert wel — voor de duidelijkheid: graag een korte rationale-comment bij MIN_SECRET_KEY_LENGTH in opi/core/secret_key.py zodat de discussie niet steeds terugkomt.

2. tests/e2e/conftest.py:34 dupliceert de dev-default als literal

Liever:

from opi.core.secret_key import DEV_DEFAULT_SECRET_KEY

zodat als de default ooit wordt geroteerd, die regel meekomt. Nu staat dezelfde string op twee plekken.

3. WARNING bij elke Settings() is luidruchtig

Bij her-imports in test-runs vuurt de log-WARNING herhaald. Niet functioneel een probleem, wel ruis in logs. Eenmalige lru_cache of een module-level _warned flag zou volstaan. Niet-blokkerend.

Belangrijk vóór merge

De fix zal in productie crash-loopen tenzij bootstrap/rig-system/kustomize/operations-manager/overlays/odcn-production/operations-manager-env-secrets.yaml een sterke SECRET_KEY (≥ 32 random) bevat. PR-beschrijving stelt dat 'ie momenteel ontbreekt — dat is precies wat de fix bedoelt te blokkeren. Graag voor merge expliciet bevestigen dat de SOPS-encrypted secret is bijgewerkt.

Andere SECRET_KEY-consumers (geverifieerd, geen issue)

opi/server.py:340 (Starlette SessionMiddleware) en opi/api/logs_websocket_router.py:146 (itsdangerous.TimestampSigner voor dezelfde session). Eén doel — sessions — geen OWASP-overtreding van "separate keys per purpose". CSRF (opi/utils/csrf.py) gebruikt secrets.token_urlsafe, niet gekoppeld aan SECRET_KEY. Geen JWT in OPI zelf. Sluit aan.

[uittenbroekrobbert]

Eén commit toegevoegd (70063f1c) + acute productie-bevinding hieronder.

🚨 Kritieke voorgrondsfeit — productie draait NU op de publieke dev-default key

# opi/core/config.py:
SECRET_KEY: str = DEV_DEFAULT_SECRET_KEY   # = "default-secret-key-for-development-change-in-production"

In bootstrap/rig-system/kustomize/operations-manager/overlays/odcn-production/operations-manager-env-secrets.yaml zit géén SECRET_KEY entry. Geverifieerd: 19 keys aanwezig (OIDC_CLIENT_SECRET, DATABASE_ADMIN_PASSWORD, MINIO_, KEYCLOAK_, REDIS_PASSWORD, BACKUP_S3_*, REGISTRY_PASSWORD, ...), maar geen SECRET_KEY.

Gevolg vandaag: productie OPI signeert sessie-cookies met de publieke gepubliceerde string. Iedereen die de repo kan lezen kan een Starlette SessionMiddleware-cookie forgen voor elke gebruiker — bypass van zowel HTTP- als websocket-auth.

Dit is geen kwetsbaarheid die deze PR introduceert. Deze PR maakt 'm zichtbaar door fail-closed te worden. Ook zonder deze PR mergen is dit een acute incident-class bug.

Verplicht vóór merge

Toevoegen aan operations-manager/python/.env.odcn-production.secrets (lokaal, gitignored, jij doet 't):

SECRET_KEY=<output van: python3 -c "import secrets; print(secrets.token_urlsafe(48))">

Daarna regenereren van de SOPS-encrypted yaml via de Taskfile:

task generate-env-secrets-for-operations-manager CLUSTER_TYPE=odcn-production

Dat updatet bootstrap/.../odcn-production/operations-manager-env-secrets.yaml met de SOPS-encrypted SECRET_KEY-entry. Commit dat resultaat.

Side-effect waar je je gebruikers moet voorbereiden: bestaande session-cookies werken NIET meer na rotatie. Alle ingelogde gebruikers worden geforceerd opnieuw in te loggen. Bewust acceptabel? Mocht dat reden zijn voor een geplande deploy-window, dan moet dat met communicatie.

Onze commit (70063f1c)

tests/e2e/conftest.py:34 had de dev-default hardcoded als string-literal in de os.environ.get() fallback — gedupliceerd t.o.v. opi/core/secret_key.py:DEV_DEFAULT_SECRET_KEY. Vervangen door import van de constante zodat één bron de waarde bezit.

-E2E_SECRET_KEY = os.environ.get("E2E_SECRET_KEY", "default-secret-key-for-development-change-in-production")
+from opi.core.secret_key import DEV_DEFAULT_SECRET_KEY
+E2E_SECRET_KEY = os.environ.get("E2E_SECRET_KEY", DEV_DEFAULT_SECRET_KEY)

24/24 secret-key tests groen na de wijziging.

Audit van de PR-implementatie zelf

• ✅ validate_secret_key (pure functie in opi/core/secret_key.py) — fail-closed bij unset/empty/dev-default/<32 chars + niet-local cluster
• ✅ Gewired via @model_validator(mode="after") op Settings in config.py — raise bij module-import, vóór uvicorn bindt
• ✅ Gate op CLUSTER_MANAGER (allowlist {"local", "sandboxed-local"}) — terecht; DEBUG-gate zou productie nooit raken (productie zet alleen DEBUG_MODE als string, niet boolean DEBUG)
• ✅ from __future__ import annotations toegevoegd zodat -> Settings geen NameError gooit
• ✅ Tests dekken pure-function paths én daadwerkelijke Settings-validator wiring (TestSettingsModelValidatorWiring)

Klein, bewust niet gefixt

• Geen entropie-check: "x"*32 zou passeren. Bewuste vereenvoudiging — de echte aanval is de publieke string, lengte ≥ 32 sluit triviale gevallen uit, entropie-meting in een opstart-validator is brittle. Eens met de keuze.
• WARNING-log per Settings()-instantiatie niet rate-limited. Bij her-imports in tests luidruchtig maar functioneel geen probleem. Optionele cleanup, geen blocker.

Andere SECRET_KEY-consumers geverifieerd

Locatie	Gebruik
opi/server.py:340	Starlette SessionMiddleware — sessie-cookie signing
opi/api/logs_websocket_router.py:146	itsdangerous.TimestampSigner voor websocket log-stream session

Eén doel (sessions), gedeeld over twee endpoints. Geen JWT in OPI zelf; CSRF gebruikt secrets.token_urlsafe (niet SECRET_KEY). Geen OWASP-overtreding van "separate keys per purpose".

---

Audit: één commit gepushed, pre-push pytest groen. Belangrijkste actiepunt voor jou: lokaal SECRET_KEY toevoegen + regenereren + committen vóór merge.

[uittenbroekrobbert]

Update — rotating SECRET_KEY default i.p.v. gecommitte dev-default (commit 5ccebbb)

Tijdens review kwam de vraag op: waarom überhaupt een dev-default in source? Het cluster-gate patroon vermeed wel een crash voor lokale ontwikkelaars, maar zette een publiek bekende literal in de repo en introduceerde een tweede code-pad ("tolerated on local cluster") dat eigen edge-cases (typo'd CLUSTER_MANAGER → fail-closed) en test-matrix vereiste.

Cleaner: rotating per-process random key als default, geen literal. Operators die stabiele sessies over reboots willen zetten SECRET_KEY expliciet in env (zoals productie al doet via het SOPS-encrypted secret).

Wat is veranderd

• opi/core/secret_key.py — kleiner en simpeler:
  • Geen DEV_DEFAULT_SECRET_KEY constant meer (literal weg uit source)
  • Geen LOCAL_CLUSTER_MANAGERS / is_local_cluster — geen cluster-gate
  • Nieuwe generate_secret_key() factory: secrets.token_urlsafe(32) + één WARNING-log
  • validate_secret_key(secret_key): alleen lengte-check (≥32 chars). Vuurt alleen als operator zelf een te korte key heeft gezet.
• opi/core/config.py — SECRET_KEY: str = Field(default_factory=generate_secret_key). Validator-call vereenvoudigd (geen CLUSTER_MANAGER argument).
• tests/test_secret_key_failclosed.py — herschreven:
  • Weg: TestIsLocalCluster matrix, dev-default tests, local-cluster-tolerance tests
  • Op: factory-uniqueness (twee opeenvolgende calls verschillen), factory-output-passes-validator, factory-emits-warning, short-env-raises, strong-env-boots
  • 12 tests, allemaal groen
• tests/e2e/conftest.py — DEV_DEFAULT_SECRET_KEY import weg + sandbox-fallback string vervangen (was hardcoded literal-fallback)
• tests/e2e/testserver.py — SECRET_KEY was 19 chars ("e2e-test-secret-key") → verlengd naar ≥32 zodat de strict-length validator hem accepteert in test-context

Design-eigenschappen

Scenario	Gedrag
Geen SECRET_KEY in env (dev/sandbox default)	Fresh random per process, WARNING gelogd, sessies invalideren bij restart
SECRET_KEY=<<32 chars>	InsecureSecretKeyError → refuse to boot
SECRET_KEY=<≥32 chars> (prod-secret of dev-override)	Werkt, sessies stabiel over restarts
Multi-pod prod zonder expliciete SECRET_KEY	Elke pod eigen random key → sessies werken niet cross-pod. Geen lek, wel UX-degradatie → operators zien de WARNING en zetten een echte key

Pre-push pytest equivalent

3403 passed, 31 deselected, 68 warnings in 35.47s

Specifiek test_secret_key_failclosed.py: 12/12 passed.

Net diff

5 files, +94/-176 — netto kleiner dan de oorspronkelijke fix omdat cluster-gate logic + bijbehorende tests verdwijnen.