anti/DECNET
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(security): close INFO ASVS findings — secret echo, TLS floor, mandatory tarball SHA, CORS/Content-Type guards, BUG-17

Browse Source 
- V7.1.3: env known-insecure-default error no longer echoes the rejected secret value.
- V9.1.4: syslog-over-TLS forwarder + listener pin minimum_version=TLSv1_2.
- V12.1.2: updater tarball SHA-256 verification is now mandatory and fail-closed —
  /update and /update-self reject a missing digest (400), the executor rejects
  missing/mismatched digests before extract/apply. Every push path supplies it.
- V13.1.4: reject a wildcard '*' in DECNET_CORS_ORIGINS at startup.
- V13.1.5: enforce application/json on JSON write endpoints (415 otherwise),
  exempting multipart upload routes.
- BUG-17: SSE error log records the user uuid, not the resume cursor.

Also completes V2.1.7 consistently: the attacker-injectable PYTEST* env bypass is
replaced with explicit DECNET_TESTING=1 in the three remaining sites
(env.validate_public_binding, config logging, mysql url builder).

Tests added for every fix; unanimous adversarial review (no update-outage risk —
all push paths verified to send the digest).
This commit is contained in:
anti
2026-06-10 13:50:06 -04:00
parent 245975a6dd
commit 337520c7ad
17 changed files with 520 additions and 73 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
decnet/config.py
View File

@@ -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

14
decnet/env.py
View File

@@ -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

3
decnet/swarm/log_forwarder.py
View File

@@ -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"),

3
decnet/swarm/log_listener.py
View File

@@ -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

4
decnet/updater/app.py
View File

@@ -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(

23
decnet/updater/executor.py
View File

@@ -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.

57
decnet/web/api.py
View File

@@ -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

10
decnet/web/db/mysql/database.py
View File

@@ -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)

2
decnet/web/router/stream/api_stream_events.py
View File

@@ -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(