fix(env): fail closed on weak DECNET_ADMIN_PASSWORD
DECNET_ADMIN_PASSWORD defaulted to the literal "admin" with no guard, so a master that never set it seeded an admin/admin account. Resolve it lazily via __getattr__ -> _require_env (the same pattern as DECNET_JWT_SECRET): unset or a known-bad default (admin/secret/...) is rejected, and <12 chars is rejected outside DECNET_DEVELOPER. Only the master web/api processes that import the DB layer resolve it; workers never do, and the pytest short-circuit keeps the dev loop unaffected. The module attribute stays addressable for the admin-seed monkeypatch.
This commit is contained in:
@@ -46,13 +46,19 @@ def _require_env(name: str) -> str:
|
||||
f"Environment variable '{name}' is set to an insecure default ('{value}'). "
|
||||
f"Choose a strong, unique value before starting DECNET."
|
||||
)
|
||||
_developer = os.environ.get("DECNET_DEVELOPER", "False").lower() == "true"
|
||||
if name == "DECNET_JWT_SECRET" and len(value) < 32:
|
||||
_developer = os.environ.get("DECNET_DEVELOPER", "False").lower() == "true"
|
||||
if not _developer:
|
||||
raise ValueError(
|
||||
f"DECNET_JWT_SECRET is too short ({len(value)} bytes). "
|
||||
f"Use at least 32 characters to satisfy HS256 requirements (RFC 7518 §3.2)."
|
||||
)
|
||||
if name == "DECNET_ADMIN_PASSWORD" and len(value) < 12:
|
||||
if not _developer:
|
||||
raise ValueError(
|
||||
f"DECNET_ADMIN_PASSWORD is too short ({len(value)} chars). "
|
||||
f"Use at least 12 characters, or set DECNET_DEVELOPER=true for local runs."
|
||||
)
|
||||
return value
|
||||
|
||||
|
||||
@@ -132,7 +138,9 @@ DECNET_BATCH_MAX_WAIT_MS: int = int(os.environ.get("DECNET_BATCH_MAX_WAIT_MS", "
|
||||
DECNET_WEB_HOST: str = os.environ.get("DECNET_WEB_HOST", "127.0.0.1")
|
||||
DECNET_WEB_PORT: int = _port("DECNET_WEB_PORT", 8080)
|
||||
DECNET_ADMIN_USER: str = os.environ.get("DECNET_ADMIN_USER", "admin")
|
||||
DECNET_ADMIN_PASSWORD: str = os.environ.get("DECNET_ADMIN_PASSWORD", "admin")
|
||||
# DECNET_ADMIN_PASSWORD is resolved lazily via __getattr__ (like DECNET_JWT_SECRET)
|
||||
# so it is validated only on the master processes that seed the admin user, and
|
||||
# never silently defaults to "admin". See _require_env + __getattr__ below.
|
||||
DECNET_DEVELOPER: bool = os.environ.get("DECNET_DEVELOPER", "False").lower() == "true"
|
||||
|
||||
# Host role — seeded by /etc/decnet/decnet.ini or exported directly.
|
||||
@@ -277,6 +285,6 @@ def validate_public_binding() -> None:
|
||||
|
||||
def __getattr__(name: str) -> str:
|
||||
"""Lazy resolution for secrets only the master web/api process needs."""
|
||||
if name == "DECNET_JWT_SECRET":
|
||||
return _require_env("DECNET_JWT_SECRET")
|
||||
if name in ("DECNET_JWT_SECRET", "DECNET_ADMIN_PASSWORD"):
|
||||
return _require_env(name)
|
||||
raise AttributeError(f"module 'decnet.env' has no attribute {name!r}")
|
||||
|
||||
66
tests/core/test_env_secrets.py
Normal file
66
tests/core/test_env_secrets.py
Normal file
@@ -0,0 +1,66 @@
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
"""Fail-closed validation of security-sensitive env secrets (env._require_env).
|
||||
|
||||
DECNET_ADMIN_PASSWORD must never silently default to "admin": it is resolved
|
||||
lazily and validated like DECNET_JWT_SECRET. These tests drive _require_env
|
||||
against a controlled environ so the production raise paths (which are bypassed
|
||||
under live pytest) are actually exercised.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
import decnet.env as envmod
|
||||
|
||||
|
||||
def _require(monkeypatch: pytest.MonkeyPatch, environ: dict[str, str]) -> str:
|
||||
# Replace the whole environ for the call so the PYTEST_* short-circuit in
|
||||
# _require_env doesn't fire — we want the real production behaviour.
|
||||
monkeypatch.setattr(envmod.os, "environ", dict(environ))
|
||||
return envmod._require_env("DECNET_ADMIN_PASSWORD")
|
||||
|
||||
|
||||
def test_admin_password_unset_raises(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
with pytest.raises(ValueError, match="not set"):
|
||||
_require(monkeypatch, {})
|
||||
|
||||
|
||||
def test_admin_password_known_bad_default_raises(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
with pytest.raises(ValueError, match="insecure default"):
|
||||
_require(monkeypatch, {"DECNET_ADMIN_PASSWORD": "admin"})
|
||||
|
||||
|
||||
@pytest.mark.parametrize("bad", ["secret", "password", "changeme", "ADMIN"])
|
||||
def test_admin_password_other_known_bad_raises(monkeypatch: pytest.MonkeyPatch, bad: str) -> None:
|
||||
with pytest.raises(ValueError, match="insecure default"):
|
||||
_require(monkeypatch, {"DECNET_ADMIN_PASSWORD": bad})
|
||||
|
||||
|
||||
def test_admin_password_too_short_raises_in_production(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
with pytest.raises(ValueError, match="too short"):
|
||||
_require(monkeypatch, {"DECNET_ADMIN_PASSWORD": "short1"})
|
||||
|
||||
|
||||
def test_admin_password_short_allowed_in_developer_mode(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
val = _require(monkeypatch, {"DECNET_ADMIN_PASSWORD": "short1", "DECNET_DEVELOPER": "true"})
|
||||
assert val == "short1"
|
||||
|
||||
|
||||
def test_admin_password_strong_value_passes(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
val = _require(monkeypatch, {"DECNET_ADMIN_PASSWORD": "a-strong-unique-password"})
|
||||
assert val == "a-strong-unique-password"
|
||||
|
||||
|
||||
def test_pytest_short_circuit_returns_value_unchecked() -> None:
|
||||
# Under live pytest (PYTEST_* present), _require_env returns the configured
|
||||
# value without the production checks — documents why the dev loop is safe.
|
||||
# conftest sets a strong value, so this also proves lazy resolution works.
|
||||
assert envmod._require_env("DECNET_ADMIN_PASSWORD") == "test-password-123"
|
||||
|
||||
|
||||
def test_lazy_getattr_resolves_admin_password() -> None:
|
||||
# Accessing the attribute (not a module global anymore) routes through
|
||||
# __getattr__ -> _require_env.
|
||||
assert envmod.DECNET_ADMIN_PASSWORD == "test-password-123"
|
||||
with pytest.raises(AttributeError):
|
||||
envmod.NOT_A_REAL_SECRET # noqa: B018
|
||||
Reference in New Issue
Block a user