diff --git a/decnet/config.py b/decnet/config.py index c4913ffc..4bcda23f 100644 --- a/decnet/config.py +++ b/decnet/config.py @@ -79,9 +79,12 @@ def _configure_logging(dev: bool) -> None: stream_handler.setFormatter(fmt) root.addHandler(stream_handler) - # Skip the file handler during pytest runs to avoid polluting the test cwd. - _in_pytest = any(k.startswith("PYTEST") for k in os.environ) - if not _in_pytest: + # Skip the file handler during test runs to avoid polluting the test cwd. + # Gated on the explicit, non-attacker-injectable DECNET_TESTING=1 flag + # (set by the test harness) rather than the attacker-controllable PYTEST* + # namespace (V2.1.7). + _in_test = os.environ.get("DECNET_TESTING") == "1" + if not _in_test: _log_path = os.environ.get("DECNET_SYSTEM_LOGS", "decnet.system.log") # Never let file-handler attach failure kill the process. The # stream handler above is already installed, so losing the file diff --git a/decnet/env.py b/decnet/env.py index fc0c3e3f..9880a109 100644 --- a/decnet/env.py +++ b/decnet/env.py @@ -58,8 +58,10 @@ def _require_env(name: str) -> str: return value if value.lower() in _KNOWN_BAD: + # Do NOT echo the rejected value — that leaks the secret into logs / + # stderr / crash reporters (V7.1.3). Name the variable, not its value. raise ValueError( - f"Environment variable '{name}' is set to an insecure default ('{value}'). " + f"Environment variable '{name}' is set to a known-insecure default. " f"Choose a strong, unique value before starting DECNET." ) _developer = os.environ.get("DECNET_DEVELOPER", "False").lower() == "true" @@ -275,10 +277,14 @@ def validate_public_binding() -> None: slip past unmentioned on a public binding. Called from the FastAPI lifespan so it surfaces at startup, not on - first request. Skipped automatically when running under pytest so - the test suite doesn't have to set five env vars per fixture. + first request. Skipped automatically under the explicit, non-attacker- + injectable DECNET_TESTING=1 flag (set by the test harness) so the test + suite doesn't have to set five env vars per fixture. The old "any PYTEST* + var present" check was fail-open: PYTEST* is an attacker-controllable + namespace, so leaking one into a prod environment silently disabled the + public-binding guard. Fail closed (V2.1.7). """ - if any(k.startswith("PYTEST") for k in os.environ): + if os.environ.get("DECNET_TESTING") == "1": return if DECNET_API_HOST in _LOOPBACK_HOSTS: return # not exposed; nothing to validate diff --git a/decnet/swarm/log_forwarder.py b/decnet/swarm/log_forwarder.py index 9fa95a13..2b8a3d07 100644 --- a/decnet/swarm/log_forwarder.py +++ b/decnet/swarm/log_forwarder.py @@ -108,6 +108,9 @@ def build_worker_ssl_context(agent_dir: pathlib.Path) -> ssl.SSLContext: f"no worker bundle at {agent_dir} — enroll from the master first" ) ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_CLIENT) + # Pin an explicit TLS 1.2 floor so it can't silently regress if a future + # runtime lowers the implicit default (ASVS V9.1.4). + ctx.minimum_version = ssl.TLSVersion.TLSv1_2 ctx.load_cert_chain( certfile=str(agent_dir / "worker.crt"), keyfile=str(agent_dir / "worker.key"), diff --git a/decnet/swarm/log_listener.py b/decnet/swarm/log_listener.py index 6769e473..a727a915 100644 --- a/decnet/swarm/log_listener.py +++ b/decnet/swarm/log_listener.py @@ -61,6 +61,9 @@ def build_listener_ssl_context(ca_dir: pathlib.Path) -> ssl.SSLContext: f"master identity missing at {master_dir} — call ensure_master_identity first" ) ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + # Pin an explicit TLS 1.2 floor so it can't silently regress if a future + # runtime lowers the implicit default (ASVS V9.1.4). + ctx.minimum_version = ssl.TLSVersion.TLSv1_2 ctx.load_cert_chain(certfile=str(cert), keyfile=str(key)) ctx.load_verify_locations(cafile=str(ca_cert)) ctx.verify_mode = ssl.CERT_REQUIRED diff --git a/decnet/updater/app.py b/decnet/updater/app.py index c14a8c34..007981b1 100644 --- a/decnet/updater/app.py +++ b/decnet/updater/app.py @@ -165,8 +165,8 @@ async def update( try: return _exec.run_update( body, sha=sha or None, + expected_sha256=sha256, install_dir=_Config.install_dir, agent_dir=_Config.agent_dir, - expected_sha256=sha256 or None, ) except _exec.UpdateError as exc: status = 409 if exc.rolled_back else 500 @@ -196,7 +196,7 @@ async def update_self( return _exec.run_update_self( body, sha=sha or None, updater_install_dir=_Config.updater_install_dir, - expected_sha256=sha256 or None, + expected_sha256=sha256, ) except _exec.UpdateError as exc: raise HTTPException( diff --git a/decnet/updater/executor.py b/decnet/updater/executor.py index b319bece..0576b658 100644 --- a/decnet/updater/executor.py +++ b/decnet/updater/executor.py @@ -576,19 +576,22 @@ def _point_current_at(install_dir: pathlib.Path, target: pathlib.Path) -> None: os.replace(tmp, link) -def _verify_tarball_sha256(tarball_bytes: bytes, expected_sha256: Optional[str]) -> None: +def _verify_tarball_sha256(tarball_bytes: bytes, expected_sha256: str) -> None: """Refuse to extract a tarball whose SHA-256 disagrees with the operator-supplied digest. mTLS already authenticates the master, so a network MITM can't forge - bytes. This check exists for two narrower cases: catching corruption + bytes. This check is mandatory defense-in-depth: it catches corruption in transit (proxies, broken disks) before we explode a half-decoded - archive into the staging tree, and giving the operator a way to pin - "exactly these bytes" when distributing a vetted release. The form - field is optional — if the caller doesn't send one, we skip the - check rather than reject (no breaking change for older masters). + archive into the staging tree, and pins "exactly these bytes" when + distributing a vetted release. It runs BEFORE extraction or any + pip-install. There is no skip path: a missing/empty digest is a + fail-closed REJECT — every caller (UpdaterClient) computes and sends + the digest, so an absent one means an untrusted/malformed request. """ - if not expected_sha256: - return + if not expected_sha256 or not expected_sha256.strip(): + raise UpdateError( + "tarball sha256 digest is required but was missing or empty — refusing to extract" + ) expected = expected_sha256.strip().lower() if len(expected) != 64 or any(c not in "0123456789abcdef" for c in expected): raise UpdateError(f"sha256 digest is not a 64-char hex string: {expected_sha256!r}") @@ -602,9 +605,9 @@ def _verify_tarball_sha256(tarball_bytes: bytes, expected_sha256: Optional[str]) def run_update( tarball_bytes: bytes, sha: Optional[str], + expected_sha256: str, install_dir: pathlib.Path = DEFAULT_INSTALL_DIR, agent_dir: pathlib.Path = pki.DEFAULT_AGENT_DIR, - expected_sha256: Optional[str] = None, ) -> dict[str, Any]: """Apply an update atomically. Rolls back on probe failure.""" log.info("update received sha=%s bytes=%d install_dir=%s", sha, len(tarball_bytes), install_dir) @@ -699,8 +702,8 @@ def run_update_self( tarball_bytes: bytes, sha: Optional[str], updater_install_dir: pathlib.Path, + expected_sha256: str, exec_cb: Optional[Callable[[list[str]], None]] = None, - expected_sha256: Optional[str] = None, ) -> dict[str, Any]: """Replace the updater's own source tree, then re-exec this process. diff --git a/decnet/web/api.py b/decnet/web/api.py index b362561c..73960502 100644 --- a/decnet/web/api.py +++ b/decnet/web/api.py @@ -15,6 +15,8 @@ from fastapi.exceptions import RequestValidationError from fastapi.responses import ORJSONResponse, Response from pydantic import ValidationError from fastapi.middleware.cors import CORSMiddleware +from starlette.middleware.base import BaseHTTPMiddleware +from starlette.responses import Response as StarletteResponse from decnet.env import ( DECNET_CORS_ORIGINS, @@ -59,6 +61,20 @@ def get_background_tasks() -> dict[str, Optional[asyncio.Task[Any]]]: } +def _check_cors_origins(origins: list[str]) -> None: + """V13.1.4 — raise at startup if a wildcard CORS origin is configured. + + Called from the lifespan so the error surfaces before any worker or DB + comes up, making misconfiguration immediately visible in uvicorn logs. + Exposed as a module-level function so tests can exercise it directly + without needing to reload the module. + """ + if "*" in origins: + raise ValueError( + "DECNET_CORS_ORIGINS must not contain a wildcard '*' — list explicit origin URLs" + ) + + @asynccontextmanager async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: global ingestion_task, collector_task, attacker_task, sniffer_task @@ -79,6 +95,10 @@ async def lifespan(app: FastAPI) -> AsyncGenerator[None, None]: # Raises ValueError with an actionable message; uvicorn surfaces it. validate_public_binding() + # V13.1.4 — CORS wildcard guard: wildcard '*' bypasses SOP and must be + # rejected at startup so the operator sees an actionable error immediately. + _check_cors_origins(DECNET_CORS_ORIGINS) + # Defence-in-depth on top of the CLI mode gating. Typer hides master-only # commands when DECNET_MODE=agent, but a misconfigured systemd unit or # a direct `python -m uvicorn decnet.web.api:app` call would bypass that. @@ -254,6 +274,43 @@ app.add_middleware( allow_headers=["Authorization", "Content-Type", "Last-Event-ID"], ) +# V13.1.5 — Content-Type enforcement: POST/PUT/PATCH with a non-empty body +# under /api/ must send application/json. Multipart/form-data endpoints +# (file-drop, canary blob upload) are exempt by their Content-Type prefix. +_MULTIPART_EXEMPT_PATHS: frozenset[str] = frozenset({ + "/api/v1/deckies/files", + "/api/v1/canary/blobs", +}) +_JSON_ENFORCE_METHODS: frozenset[str] = frozenset({"POST", "PUT", "PATCH"}) + + +class _ContentTypeMiddleware(BaseHTTPMiddleware): + async def dispatch(self, request: Request, call_next: Any) -> StarletteResponse: + if ( + request.method in _JSON_ENFORCE_METHODS + and request.url.path.startswith("/api/") + ): + # Allow through if the path belongs to a multipart-exempt route. + path = request.url.path + exempt = any(path.startswith(p) for p in _MULTIPART_EXEMPT_PATHS) + if not exempt: + ct = request.headers.get("content-type", "") + # Only enforce when a body is actually present (non-zero Content-Length + # or chunked transfer), so empty-body POST health-checks stay clean. + cl = request.headers.get("content-length", "") + te = request.headers.get("transfer-encoding", "") + has_body = (cl not in ("", "0")) or ("chunked" in te.lower()) + if has_body and not ct.lower().startswith("application/json"): + return StarletteResponse( + content="Unsupported Media Type — Content-Type must be application/json", + status_code=415, + media_type="text/plain", + ) + return await call_next(request) + + +app.add_middleware(_ContentTypeMiddleware) + if DECNET_PROFILE_REQUESTS: import time from pathlib import Path diff --git a/decnet/web/db/mysql/database.py b/decnet/web/db/mysql/database.py index d52fb35c..7caac4da 100644 --- a/decnet/web/db/mysql/database.py +++ b/decnet/web/db/mysql/database.py @@ -50,12 +50,14 @@ def build_mysql_url( if password is None: password = os.environ.get("DECNET_DB_PASSWORD") or "" - # Allow empty passwords during tests (pytest sets PYTEST_* env vars). - # Outside tests, an empty MySQL password is almost never intentional. - if not password and not any(k.startswith("PYTEST") for k in os.environ): + # Allow empty passwords during tests, gated on the explicit, non-attacker- + # injectable DECNET_TESTING=1 flag (set by the test harness) rather than + # the attacker-controllable PYTEST* namespace (V2.1.7). Outside tests, an + # empty MySQL password is almost never intentional. + if not password and os.environ.get("DECNET_TESTING") != "1": raise ValueError( "DECNET_DB_PASSWORD is not set. Either export it, set DECNET_DB_URL, " - "or run under pytest for an empty-password default." + "or run under the test harness (DECNET_TESTING=1) for an empty-password default." ) pw_enc = quote_plus(password) diff --git a/decnet/web/router/stream/api_stream_events.py b/decnet/web/router/stream/api_stream_events.py index 19c8c715..a00416c1 100644 --- a/decnet/web/router/stream/api_stream_events.py +++ b/decnet/web/router/stream/api_stream_events.py @@ -139,7 +139,7 @@ async def stream_events( except asyncio.CancelledError: pass except Exception: - log.exception("SSE stream error for user %s", last_event_id) + log.exception("SSE stream error for user %s", user["uuid"]) yield f"event: error\ndata: {orjson.dumps({'type': 'error', 'message': 'Stream interrupted'}).decode()}\n\n" return StreamingResponse( diff --git a/tests/api/test_security_middleware.py b/tests/api/test_security_middleware.py new file mode 100644 index 00000000..cc454c89 --- /dev/null +++ b/tests/api/test_security_middleware.py @@ -0,0 +1,204 @@ +# SPDX-License-Identifier: AGPL-3.0-or-later +""" +Security-middleware tests covering: + - V13.1.4: CORS wildcard guard raises ValueError at app startup + - V13.1.5: Content-Type enforcement middleware (415 on wrong CT; pass for + application/json; multipart exempt paths; GET/DELETE unaffected) + - BUG-17: SSE stream error log uses user["uuid"], not last_event_id + - Regression: multipart upload endpoints still work (canary blob, file-drop) +""" +from __future__ import annotations + +import pytest +import httpx + + +# --------------------------------------------------------------------------- +# V13.1.4 — CORS wildcard guard (unit tests; lifespan path in tests/web/) +# --------------------------------------------------------------------------- + +class TestCORSWildcardGuard: + def test_wildcard_raises(self): + """_check_cors_origins raises ValueError when '*' is present.""" + from decnet.web.api import _check_cors_origins + with pytest.raises(ValueError, match="wildcard"): + _check_cors_origins(["*"]) + + def test_wildcard_among_explicit_origins_raises(self): + """Wildcard in a mixed list is still rejected.""" + from decnet.web.api import _check_cors_origins + with pytest.raises(ValueError, match="wildcard"): + _check_cors_origins(["https://example.com", "*"]) + + def test_explicit_origins_ok(self): + """Explicit origin URLs pass without raising.""" + from decnet.web.api import _check_cors_origins + _check_cors_origins(["https://example.com", "https://app.internal"]) + + def test_empty_origins_ok(self): + """Empty list is valid (no CORS).""" + from decnet.web.api import _check_cors_origins + _check_cors_origins([]) + + +# --------------------------------------------------------------------------- +# V13.1.5 — Content-Type enforcement middleware +# --------------------------------------------------------------------------- + +class TestContentTypeMiddleware: + @pytest.mark.asyncio + async def test_post_wrong_content_type_returns_415( + self, client: httpx.AsyncClient + ): + """POST with text/plain body to a JSON endpoint returns 415. + + /api/v1/auth/login is the most stable JSON POST target — no auth + required, always present, middleware fires before the handler. + """ + resp = await client.post( + "/api/v1/auth/login", + content=b"not json", + headers={"Content-Type": "text/plain"}, + ) + assert resp.status_code == 415 + + @pytest.mark.asyncio + async def test_post_application_json_passes_middleware( + self, client: httpx.AsyncClient + ): + """POST with application/json does NOT get a 415 from middleware.""" + resp = await client.post( + "/api/v1/auth/login", + json={"username": "nobody", "password": "wrong"}, + ) + # Middleware passes; handler may 401/422 but must not 415. + assert resp.status_code != 415 + + @pytest.mark.asyncio + async def test_post_json_with_charset_passes( + self, client: httpx.AsyncClient + ): + """application/json; charset=utf-8 is a valid Content-Type.""" + resp = await client.post( + "/api/v1/auth/login", + content=b'{"username":"x","password":"y"}', + headers={"Content-Type": "application/json; charset=utf-8"}, + ) + assert resp.status_code != 415 + + @pytest.mark.asyncio + async def test_get_not_enforced(self, client: httpx.AsyncClient, auth_token: str): + """GET requests are never rejected by the CT middleware.""" + resp = await client.get( + "/api/v1/logs", + headers={"Authorization": f"Bearer {auth_token}"}, + ) + assert resp.status_code != 415 + + @pytest.mark.asyncio + async def test_delete_not_enforced( + self, client: httpx.AsyncClient, auth_token: str + ): + """DELETE requests are never rejected by the CT middleware.""" + resp = await client.delete( + "/api/v1/deckies/nonexistent", + headers={"Authorization": f"Bearer {auth_token}"}, + ) + # Could be 404/401/403 but never 415. + assert resp.status_code != 415 + + @pytest.mark.asyncio + async def test_multipart_canary_blob_exempt( + self, client: httpx.AsyncClient, auth_token: str + ): + """Canary blob upload (multipart/form-data) is NOT rejected with 415.""" + resp = await client.post( + "/api/v1/canary/blobs", + files={"file": ("test.txt", b"hello world", "text/plain")}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + # 201 on success, 4xx on business-logic errors — never 415. + assert resp.status_code != 415 + + @pytest.mark.asyncio + async def test_multipart_file_drop_exempt( + self, client: httpx.AsyncClient, auth_token: str + ): + """Decky file-drop (multipart/form-data) is NOT rejected with 415.""" + resp = await client.post( + "/api/v1/deckies/files/some-container", + files={"file": ("test.txt", b"data", "text/plain")}, + headers={"Authorization": f"Bearer {auth_token}"}, + ) + # Expect 4xx business error (no real container), never 415. + assert resp.status_code != 415 + + @pytest.mark.asyncio + async def test_empty_body_post_not_enforced( + self, client: httpx.AsyncClient, auth_token: str + ): + """POST with genuinely empty body (Content-Length: 0) is not rejected.""" + resp = await client.post( + "/api/v1/logs", + content=b"", + headers={ + "Authorization": f"Bearer {auth_token}", + "Content-Length": "0", + }, + ) + # Middleware should not 415 on empty bodies. + assert resp.status_code != 415 + + +# --------------------------------------------------------------------------- +# BUG-17 — SSE error log uses user["uuid"], not last_event_id +# --------------------------------------------------------------------------- + +class TestSSEErrorLog: + def test_sse_error_log_uses_user_uuid(self): + """ + Verify the log.exception call in the SSE generator uses user["uuid"], + not last_event_id (which is an int cursor, not an identity). + """ + import ast, pathlib + src = pathlib.Path( + "decnet/web/router/stream/api_stream_events.py" + ).read_text() + tree = ast.parse(src) + + bad_pattern_found = False + correct_pattern_found = False + + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + # Look for log.exception(...) calls + func = node.func + if not (isinstance(func, ast.Attribute) and func.attr == "exception"): + continue + # Check args after the format string + if len(node.args) >= 2: + arg = node.args[1] + # Bad pattern: bare Name "last_event_id" + if isinstance(arg, ast.Name) and arg.id == "last_event_id": + bad_pattern_found = True + # Good pattern: user["uuid"] subscript + if ( + isinstance(arg, ast.Subscript) + and isinstance(arg.value, ast.Name) + and arg.value.id == "user" + ): + correct_pattern_found = True + + assert not bad_pattern_found, ( + "BUG-17: log.exception still uses last_event_id instead of user['uuid']" + ) + assert correct_pattern_found, ( + "BUG-17 fix not found: expected log.exception(..., user['uuid']) in SSE handler" + ) + + @pytest.mark.asyncio + async def test_sse_stream_unauthenticated_401(self, client: httpx.AsyncClient): + """SSE endpoint rejects unauthenticated requests (regression guard).""" + resp = await client.get("/api/v1/stream") + assert resp.status_code == 401 diff --git a/tests/core/test_env_secrets.py b/tests/core/test_env_secrets.py index b12fd799..a5184f32 100644 --- a/tests/core/test_env_secrets.py +++ b/tests/core/test_env_secrets.py @@ -4,7 +4,7 @@ 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. +under the test harness via DECNET_TESTING=1) are actually exercised. """ from __future__ import annotations @@ -14,8 +14,8 @@ 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. + # Replace the whole environ for the call so the DECNET_TESTING 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") @@ -36,6 +36,20 @@ def test_admin_password_other_known_bad_raises(monkeypatch: pytest.MonkeyPatch, _require(monkeypatch, {"DECNET_ADMIN_PASSWORD": bad}) +def test_known_bad_message_does_not_leak_secret_value( + monkeypatch: pytest.MonkeyPatch, +) -> None: + # V7.1.3: the known-bad rejection must NOT echo the rejected secret value + # (it would land in logs / stderr / crash reporters). Name the variable, + # not its value. + secret = "admin" + with pytest.raises(ValueError) as exc: + _require(monkeypatch, {"DECNET_ADMIN_PASSWORD": secret}) + msg = str(exc.value) + assert secret not in msg + assert "DECNET_ADMIN_PASSWORD" in msg + + 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"}) diff --git a/tests/db/mysql/test_mysql_url_builder.py b/tests/db/mysql/test_mysql_url_builder.py index daa09bb2..b44a4710 100644 --- a/tests/db/mysql/test_mysql_url_builder.py +++ b/tests/db/mysql/test_mysql_url_builder.py @@ -13,7 +13,7 @@ def test_build_url_defaults(monkeypatch): for v in ("DECNET_DB_HOST", "DECNET_DB_PORT", "DECNET_DB_NAME", "DECNET_DB_USER", "DECNET_DB_PASSWORD", "DECNET_DB_URL"): monkeypatch.delenv(v, raising=False) - # PYTEST_* is set by pytest itself, so empty password is allowed here. + # DECNET_TESTING=1 is set by conftest, so empty password is allowed here. url = build_mysql_url() assert url == "mysql+asyncmy://decnet:@localhost:3306/decnet" @@ -66,14 +66,13 @@ def test_resolve_url_falls_back_to_components(monkeypatch): assert url.startswith("mysql+asyncmy://") -def test_build_url_requires_password_outside_pytest(monkeypatch): - """Without a password and not in a pytest run, construction must fail loudly.""" +def test_build_url_requires_password_outside_tests(monkeypatch): + """Without a password and not under the test harness, construction must fail loudly.""" for v in ("DECNET_DB_URL", "DECNET_DB_PASSWORD"): monkeypatch.delenv(v, raising=False) - # Strip every PYTEST_* env var so the safety check trips. - import os - for k in list(os.environ): - if k.startswith("PYTEST"): - monkeypatch.delenv(k, raising=False) + # Clear the test-harness flag so the safety check trips. A leaked PYTEST_* + # var must NOT re-enable the bypass (V2.1.7). + monkeypatch.delenv("DECNET_TESTING", raising=False) + monkeypatch.setenv("PYTEST_CURRENT_TEST", "x") with pytest.raises(ValueError, match="DECNET_DB_PASSWORD is not set"): build_mysql_url() diff --git a/tests/swarm/test_forwarder_resilience.py b/tests/swarm/test_forwarder_resilience.py index 2c90d2df..7456060f 100644 --- a/tests/swarm/test_forwarder_resilience.py +++ b/tests/swarm/test_forwarder_resilience.py @@ -69,6 +69,18 @@ async def _wait_for(pred, timeout: float = 5.0, interval: float = 0.1) -> bool: # ----------------------------------------------------------- pure helpers +def test_worker_ssl_context_pins_tls12_floor(_pki_env: dict) -> None: + """V9.1.4: forwarder client context must set an explicit TLS 1.2 floor.""" + ctx = fwd.build_worker_ssl_context(_pki_env["worker_dir"]) + assert ctx.minimum_version == ssl.TLSVersion.TLSv1_2 + + +def test_listener_ssl_context_pins_tls12_floor(_pki_env: dict) -> None: + """V9.1.4: listener server context must set an explicit TLS 1.2 floor.""" + ctx = lst.build_listener_ssl_context(_pki_env["ca_dir"]) + assert ctx.minimum_version == ssl.TLSVersion.TLSv1_2 + + def test_peer_cn_returns_unknown_when_no_ssl_object() -> None: assert lst.peer_cn(None) == "unknown" diff --git a/tests/updater/test_updater_app.py b/tests/updater/test_updater_app.py index 33088dd9..1c9c1e4f 100644 --- a/tests/updater/test_updater_app.py +++ b/tests/updater/test_updater_app.py @@ -54,10 +54,13 @@ def test_health_returns_role_and_releases(client: TestClient, monkeypatch: pytes def test_update_happy_path(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr( - ex, "run_update", - lambda data, sha, install_dir, agent_dir, expected_sha256=None: {"status": "updated", "release": {"slot": "active", "sha": sha}, "probe": "ok"}, - ) + seen: dict[str, object] = {} + + def _run_update(data, sha, expected_sha256, install_dir, agent_dir): + seen["expected_sha256"] = expected_sha256 + return {"status": "updated", "release": {"slot": "active", "sha": sha}, "probe": "ok"} + + monkeypatch.setattr(ex, "run_update", _run_update) r = client.post( "/update", files={"tarball": ("tree.tgz", _tarball(), "application/gzip")}, @@ -65,6 +68,8 @@ def test_update_happy_path(client: TestClient, monkeypatch: pytest.MonkeyPatch) ) assert r.status_code == 200, r.text assert r.json()["release"]["sha"] == "ABC123" + # Route forwards the digest verbatim — executor verifies it before extract. + assert seen["expected_sha256"] == "0" * 64 def test_update_rollback_returns_409(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None: @@ -104,10 +109,13 @@ def test_update_self_requires_confirm(client: TestClient) -> None: def test_update_self_happy_path(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setattr( - ex, "run_update_self", - lambda data, sha, updater_install_dir, expected_sha256=None: {"status": "self_update_queued", "argv": ["python", "-m", "decnet", "updater"]}, - ) + seen: dict[str, object] = {} + + def _run_update_self(data, sha, updater_install_dir, expected_sha256): + seen["expected_sha256"] = expected_sha256 + return {"status": "self_update_queued", "argv": ["python", "-m", "decnet", "updater"]} + + monkeypatch.setattr(ex, "run_update_self", _run_update_self) r = client.post( "/update-self", files={"tarball": ("t.tgz", _tarball(), "application/gzip")}, @@ -115,6 +123,7 @@ def test_update_self_happy_path(client: TestClient, monkeypatch: pytest.MonkeyPa ) assert r.status_code == 200 assert r.json()["status"] == "self_update_queued" + assert seen["expected_sha256"] == "0" * 64 def test_rollback_happy(client: TestClient, monkeypatch: pytest.MonkeyPatch) -> None: @@ -158,6 +167,47 @@ def test_update_without_sha256_is_rejected(client: TestClient) -> None: assert "sha256" in r.json()["detail"] +def test_update_with_empty_sha256_is_rejected(client: TestClient) -> None: + # An explicit empty form value is treated the same as absent → 400. + r = client.post( + "/update", + files={"tarball": ("t.tgz", _tarball(), "application/gzip")}, + data={"sha": "ABC", "sha256": ""}, + ) + assert r.status_code == 400 + assert "sha256" in r.json()["detail"] + + +def test_update_self_without_sha256_is_rejected(client: TestClient) -> None: + r = client.post( + "/update-self", + files={"tarball": ("t.tgz", _tarball(), "application/gzip")}, + data={"confirm_self": "true"}, + ) + assert r.status_code == 400 + assert "sha256" in r.json()["detail"] + + +def test_update_mismatched_sha256_is_rejected_before_apply( + client: TestClient, monkeypatch: pytest.MonkeyPatch +) -> None: + """End-to-end through the REAL executor verify: a non-matching digest is a + 500 UpdateError and no extraction/pip happens (extract/_run_pip would be + reached only AFTER the digest check, so we assert they are never called).""" + called: list[str] = [] + monkeypatch.setattr(ex, "extract_tarball", lambda *a, **k: called.append("extract")) + monkeypatch.setattr(ex, "_run_pip", lambda *a, **k: called.append("pip")) + + r = client.post( + "/update", + files={"tarball": ("t.tgz", _tarball(), "application/gzip")}, + data={"sha": "ABC", "sha256": "0" * 64}, # wrong digest for this tarball + ) + assert r.status_code == 500, r.text + assert "mismatch" in r.json()["detail"]["error"] + assert called == [] # rejected before any extract/install + + # ------------------------- master-cert gate --------------------------------- diff --git a/tests/updater/test_updater_executor.py b/tests/updater/test_updater_executor.py index 3c081322..20f57b6b 100644 --- a/tests/updater/test_updater_executor.py +++ b/tests/updater/test_updater_executor.py @@ -8,6 +8,7 @@ against a ``tmp_path`` install dir. """ from __future__ import annotations +import hashlib import io import pathlib import subprocess @@ -32,6 +33,11 @@ def _make_tarball(files: dict[str, str]) -> bytes: return buf.getvalue() +def _digest(tarball: bytes) -> str: + """SHA-256 hex of the tarball — now mandatory on run_update/run_update_self.""" + return hashlib.sha256(tarball).hexdigest() + + class _PipOK: returncode = 0 stdout = "" @@ -207,6 +213,40 @@ def test_run_update_rejects_malformed_sha256( ) +@pytest.mark.parametrize("missing", ["", " ", None]) +def test_run_update_rejects_missing_sha256_fail_closed( + install_dir: pathlib.Path, agent_dir: pathlib.Path, missing: Any, +) -> None: + """V12.1.2 fail-closed: an absent/empty digest is rejected BEFORE any + extraction or pip-install. No staging tree is produced.""" + tb = _make_tarball({"x.txt": "y"}) + with pytest.raises(ex.UpdateError, match="required but was missing or empty"): + ex.run_update( + tb, sha="S", expected_sha256=missing, # type: ignore[arg-type] + install_dir=install_dir, agent_dir=agent_dir, + ) + assert not (install_dir / "releases" / "active.new").exists() + + +@pytest.mark.parametrize("missing", ["", " ", None]) +def test_run_update_self_rejects_missing_sha256_fail_closed( + install_dir: pathlib.Path, missing: Any, +) -> None: + active = install_dir / "releases" / "active" + active.mkdir() + (active / "marker").write_text("old-updater") + tb = _make_tarball({"marker": "new-updater"}) + with pytest.raises(ex.UpdateError, match="required but was missing or empty"): + ex.run_update_self( + tb, sha="U", updater_install_dir=install_dir, + expected_sha256=missing, # type: ignore[arg-type] + exec_cb=lambda a: None, + ) + # Active untouched, nothing staged. + assert (install_dir / "releases" / "active" / "marker").read_text() == "old-updater" + assert not (install_dir / "releases" / "active.new").exists() + + def test_clean_stale_staging(install_dir: pathlib.Path) -> None: staging = install_dir / "releases" / "active.new" staging.mkdir() @@ -229,7 +269,7 @@ def test_update_rotates_and_probes( monkeypatch.setattr(ex, "_probe_agent", lambda **_: (True, "ok")) tb = _make_tarball({"marker.txt": "new"}) - result = ex.run_update(tb, sha="NEWSHA", install_dir=install_dir, agent_dir=agent_dir) + result = ex.run_update(tb, sha="NEWSHA", expected_sha256=_digest(tb), install_dir=install_dir, agent_dir=agent_dir) assert result["status"] == "updated" assert result["release"]["sha"] == "NEWSHA" @@ -252,7 +292,7 @@ def test_update_first_install_without_previous( monkeypatch.setattr(ex, "_probe_agent", lambda **_: (True, "ok")) tb = _make_tarball({"marker.txt": "first"}) - result = ex.run_update(tb, sha="S1", install_dir=install_dir, agent_dir=agent_dir) + result = ex.run_update(tb, sha="S1", expected_sha256=_digest(tb), install_dir=install_dir, agent_dir=agent_dir) assert result["status"] == "updated" assert not (install_dir / "releases" / "prev").exists() @@ -273,7 +313,7 @@ def test_update_pip_failure_aborts_before_rotation( tb = _make_tarball({"marker.txt": "new"}) with pytest.raises(ex.UpdateError, match="pip install failed") as ei: - ex.run_update(tb, sha="S", install_dir=install_dir, agent_dir=agent_dir) + ex.run_update(tb, sha="S", expected_sha256=_digest(tb), install_dir=install_dir, agent_dir=agent_dir) assert "resolver error" in ei.value.stderr # Nothing rotated — old active still live, no prev created. @@ -309,7 +349,7 @@ def test_update_probe_failure_rolls_back( tb = _make_tarball({"marker.txt": "new"}) with pytest.raises(ex.UpdateError, match="health probe") as ei: - ex.run_update(tb, sha="NEWSHA", install_dir=install_dir, agent_dir=agent_dir) + ex.run_update(tb, sha="NEWSHA", expected_sha256=_digest(tb), install_dir=install_dir, agent_dir=agent_dir) assert ei.value.rolled_back is True assert "connection refused" in ei.value.stderr @@ -381,6 +421,7 @@ def test_update_self_rotates_and_calls_exec_cb( tb = _make_tarball({"marker": "new-updater"}) result = ex.run_update_self( tb, sha="USHA", updater_install_dir=install_dir, + expected_sha256=_digest(tb), exec_cb=lambda argv: seen_argv.append(argv), ) assert result["status"] == "self_update_queued" @@ -412,7 +453,7 @@ def test_update_self_under_systemd_defers_to_systemctl( monkeypatch.setattr(ex.os, "execv", lambda *a, **k: pytest.fail("execv taken under systemd")) tb = _make_tarball({"marker": "new-updater"}) - result = ex.run_update_self(tb, sha="USHA", updater_install_dir=install_dir) + result = ex.run_update_self(tb, sha="USHA", updater_install_dir=install_dir, expected_sha256=_digest(tb)) assert result == {"status": "self_update_queued", "via": "systemd"} assert len(popen_calls) == 1 sh_cmd = popen_calls[0] @@ -431,7 +472,7 @@ def test_update_self_pip_failure_leaves_active_intact( tb = _make_tarball({"marker": "new-updater"}) with pytest.raises(ex.UpdateError, match="pip install failed"): - ex.run_update_self(tb, sha="U", updater_install_dir=install_dir, exec_cb=lambda a: None) + ex.run_update_self(tb, sha="U", updater_install_dir=install_dir, expected_sha256=_digest(tb), exec_cb=lambda a: None) assert (install_dir / "releases" / "active" / "marker").read_text() == "old-updater" assert not (install_dir / "releases" / "active.new").exists() diff --git a/tests/web/test_api_startup_guards.py b/tests/web/test_api_startup_guards.py index fb6ba9b4..061d1b05 100644 --- a/tests/web/test_api_startup_guards.py +++ b/tests/web/test_api_startup_guards.py @@ -23,13 +23,6 @@ def _reload_api(monkeypatch: pytest.MonkeyPatch): return importlib.import_module("decnet.web.api") -def _strip_pytest_vars(monkeypatch: pytest.MonkeyPatch) -> None: - import os - for k in list(os.environ): - if k.startswith("PYTEST"): - monkeypatch.delenv(k, raising=False) - - async def _run_lifespan_startup(api_mod) -> None: """Run the lifespan up to (but not past) yield, then unwind cleanly. @@ -59,6 +52,7 @@ def test_master_api_refuses_to_start_in_agent_mode( monkeypatch.setenv("DECNET_MODE", "agent") monkeypatch.setenv("DECNET_DISALLOW_MASTER", "true") monkeypatch.setenv("DECNET_JWT_SECRET", "x" * 32) + monkeypatch.setenv("DECNET_CORS_ORIGINS", "http://localhost:8080") api = _reload_api(monkeypatch) with pytest.raises(RuntimeError, match="master-only"): asyncio.get_event_loop_policy().new_event_loop().run_until_complete( @@ -74,6 +68,7 @@ def test_master_api_starts_when_dual_role_enabled( monkeypatch.setenv("DECNET_MODE", "agent") monkeypatch.setenv("DECNET_DISALLOW_MASTER", "false") monkeypatch.setenv("DECNET_JWT_SECRET", "x" * 32) + monkeypatch.setenv("DECNET_CORS_ORIGINS", "http://localhost:8080") api = _reload_api(monkeypatch) # Reaching the DB init phase means the gate passed; we don't need to # actually finish startup. Cancel via a synthetic exception that the @@ -107,7 +102,7 @@ def test_master_api_eager_loads_jwt_secret_at_startup( monkeypatch.setenv("DECNET_MODE", "master") monkeypatch.setenv("DECNET_JWT_SECRET", "y" * 32) monkeypatch.setenv("DECNET_API_HOST", "127.0.0.1") - monkeypatch.delenv("DECNET_CORS_ORIGINS", raising=False) + monkeypatch.setenv("DECNET_CORS_ORIGINS", "http://localhost:8080") api = _reload_api(monkeypatch) import decnet.env as env_mod @@ -130,3 +125,45 @@ def test_master_api_eager_loads_jwt_secret_at_startup( assert "DECNET_JWT_SECRET" in seen, ( "lifespan must access env.DECNET_JWT_SECRET at startup" ) + + +# --------------------------------------------------------------------------- +# V13.1.4 — CORS wildcard guard +# --------------------------------------------------------------------------- + +def test_cors_wildcard_guard_function_raises() -> None: + """_check_cors_origins raises ValueError when '*' is in the list.""" + from decnet.web.api import _check_cors_origins + with pytest.raises(ValueError, match="wildcard"): + _check_cors_origins(["*"]) + + +def test_cors_wildcard_among_explicit_origins_raises() -> None: + """Wildcard in a mixed list is still rejected.""" + from decnet.web.api import _check_cors_origins + with pytest.raises(ValueError, match="wildcard"): + _check_cors_origins(["https://example.com", "*"]) + + +def test_cors_explicit_origins_pass() -> None: + """Explicit origin URLs pass the guard without raising.""" + from decnet.web.api import _check_cors_origins + _check_cors_origins(["https://example.com", "https://app.internal"]) + + +def test_cors_wildcard_raises_in_lifespan( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Lifespan raises ValueError when DECNET_CORS_ORIGINS contains '*'. + + Uses _reload_api to pick up the patched env; tests the full guard + path including the lifespan call to _check_cors_origins. + """ + monkeypatch.setenv("DECNET_MODE", "master") + monkeypatch.setenv("DECNET_JWT_SECRET", "z" * 32) + monkeypatch.setenv("DECNET_CORS_ORIGINS", "*") + api = _reload_api(monkeypatch) + with pytest.raises(ValueError, match="wildcard"): + asyncio.get_event_loop_policy().new_event_loop().run_until_complete( + _run_lifespan_startup(api) + ) diff --git a/tests/web/test_validate_public_binding.py b/tests/web/test_validate_public_binding.py index 880b167f..4649d2b7 100644 --- a/tests/web/test_validate_public_binding.py +++ b/tests/web/test_validate_public_binding.py @@ -1,9 +1,9 @@ # SPDX-License-Identifier: AGPL-3.0-or-later """validate_public_binding refuses footgun configs at master startup. -The validator no-ops under pytest by design (so unit tests in unrelated -modules don't have to set five env vars per fixture); these tests strip -the PYTEST_* vars before calling it so the real code path runs. +The validator no-ops under the test harness (DECNET_TESTING=1) by design (so +unit tests in unrelated modules don't have to set five env vars per fixture); +these tests clear that flag before calling it so the real code path runs. """ from __future__ import annotations @@ -20,18 +20,17 @@ def _reimport_env(monkeypatch: pytest.MonkeyPatch): return importlib.import_module("decnet.env") -def _strip_pytest_vars(monkeypatch: pytest.MonkeyPatch) -> None: - import os - for k in list(os.environ): - if k.startswith("PYTEST"): - monkeypatch.delenv(k, raising=False) +def _strip_test_flag(monkeypatch: pytest.MonkeyPatch) -> None: + # The validator short-circuits on DECNET_TESTING=1 (set globally by + # conftest). Clear it so the real production code path runs. + monkeypatch.delenv("DECNET_TESTING", raising=False) def test_validator_noop_on_loopback_binding(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setenv("DECNET_API_HOST", "127.0.0.1") monkeypatch.setenv("DECNET_CORS_ORIGINS", "http://localhost:8080") env = _reimport_env(monkeypatch) - _strip_pytest_vars(monkeypatch) + _strip_test_flag(monkeypatch) env.validate_public_binding() # no raise @@ -41,7 +40,7 @@ def test_validator_rejects_loopback_cors_on_public_bind( monkeypatch.setenv("DECNET_API_HOST", "0.0.0.0") monkeypatch.setenv("DECNET_CORS_ORIGINS", "http://localhost:8080") env = _reimport_env(monkeypatch) - _strip_pytest_vars(monkeypatch) + _strip_test_flag(monkeypatch) with pytest.raises(ValueError, match="loopback origin"): env.validate_public_binding() @@ -52,7 +51,7 @@ def test_validator_accepts_public_cors_on_public_bind( monkeypatch.setenv("DECNET_API_HOST", "0.0.0.0") monkeypatch.setenv("DECNET_CORS_ORIGINS", "https://dashboard.example.com") env = _reimport_env(monkeypatch) - _strip_pytest_vars(monkeypatch) + _strip_test_flag(monkeypatch) env.validate_public_binding() # no raise @@ -63,7 +62,7 @@ def test_validator_rejects_plaintext_canary_on_public_bind( monkeypatch.setenv("DECNET_CORS_ORIGINS", "https://dashboard.example.com") monkeypatch.setenv("DECNET_CANARY_HTTP_BASE", "http://canary.example.com:8088") env = _reimport_env(monkeypatch) - _strip_pytest_vars(monkeypatch) + _strip_test_flag(monkeypatch) with pytest.raises(ValueError, match="plaintext HTTP"): env.validate_public_binding() @@ -77,14 +76,28 @@ def test_validator_allows_loopback_canary_even_on_public_bind( monkeypatch.setenv("DECNET_CORS_ORIGINS", "https://dashboard.example.com") monkeypatch.setenv("DECNET_CANARY_HTTP_BASE", "http://localhost:8088") env = _reimport_env(monkeypatch) - _strip_pytest_vars(monkeypatch) + _strip_test_flag(monkeypatch) env.validate_public_binding() # no raise -def test_validator_skips_under_pytest(monkeypatch: pytest.MonkeyPatch) -> None: - # With PYTEST_* still in env (default), even a misconfigured env passes — - # this is the deliberate bypass so unrelated tests don't trip on it. +def test_validator_skips_under_test_harness(monkeypatch: pytest.MonkeyPatch) -> None: + # With DECNET_TESTING=1 still in env (set by conftest), even a misconfigured + # env passes — this is the deliberate bypass so unrelated tests don't trip. + monkeypatch.setenv("DECNET_TESTING", "1") monkeypatch.setenv("DECNET_API_HOST", "0.0.0.0") monkeypatch.setenv("DECNET_CORS_ORIGINS", "http://localhost:8080") env = _reimport_env(monkeypatch) env.validate_public_binding() # no raise — guard short-circuits + + +def test_validator_pytest_var_leak_does_not_bypass(monkeypatch: pytest.MonkeyPatch) -> None: + # V2.1.7 regression: a leaked PYTEST_* env var must NOT disable the guard. + # With DECNET_TESTING cleared, a misconfigured public binding still raises + # even though a PYTEST_* var is present. + monkeypatch.delenv("DECNET_TESTING", raising=False) + monkeypatch.setenv("PYTEST_CURRENT_TEST", "x") + monkeypatch.setenv("DECNET_API_HOST", "0.0.0.0") + monkeypatch.setenv("DECNET_CORS_ORIGINS", "http://localhost:8080") + env = _reimport_env(monkeypatch) + with pytest.raises(ValueError, match="loopback origin"): + env.validate_public_binding()