From df849819542e092043923d8aac6c3e2e1e3824d0 Mon Sep 17 00:00:00 2001 From: anti Date: Fri, 24 Apr 2026 14:27:58 -0400 Subject: [PATCH] feat(api): pin response_model on dict-returning mutation routes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every mutation route that returned an untyped dict now declares response_model at the decorator. MessageResponse covers the eight {"message": ...} envelopes (change-password, mutate-decky, mutate- interval, update-deployment-limit, update-global-mutation-interval, delete-user, update-user-role, reset-user-password). Purpose-built models cover the richer shapes (DeployResponse for /deckies/deploy, PurgeResponse for /config/reinit, ReapReportResponse for /reap-orphans, UserResponse for /config/users). 204-No-Content and Response/ ORJSONResponse routes stay as-is. The wire shape for clients is unchanged — the envelopes already only shipped a message field. What changes is that a handler which accidentally returns a richer dict (e.g. a full user row including password_hash) would be silently stripped to the declared fields at serialization time. Also flips F4/D "expensive LIKE" to accepted (new DA-09) — the /logs and /attackers search routes LIKE-scan unbounded columns, but both are admin-gated, limit-capped, and operator rate-limit scope per DA-04. FTS5 stays a performance TODO, not a security blocker. --- decnet/web/db/models/__init__.py | 11 +++++++++++ decnet/web/db/models/common.py | 15 +++++++++++++++ decnet/web/db/models/deploy.py | 10 ++++++++++ decnet/web/db/models/topology.py | 8 ++++++++ decnet/web/router/auth/api_change_pass.py | 3 ++- decnet/web/router/config/api_manage_users.py | 7 ++++++- decnet/web/router/config/api_reinit.py | 2 ++ decnet/web/router/config/api_update_config.py | 4 +++- decnet/web/router/fleet/api_deploy_deckies.py | 3 ++- decnet/web/router/fleet/api_mutate_decky.py | 2 ++ decnet/web/router/fleet/api_mutate_interval.py | 3 ++- decnet/web/router/topology/api_reap_orphans.py | 2 ++ development/THREAT_MODEL.md | 12 +++++++----- 13 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 decnet/web/db/models/common.py diff --git a/decnet/web/db/models/__init__.py b/decnet/web/db/models/__init__.py index 69295d07..88e8e6c8 100644 --- a/decnet/web/db/models/__init__.py +++ b/decnet/web/db/models/__init__.py @@ -11,6 +11,9 @@ from ._base import ( _BIG_TEXT, _normalize_null, ) +from .common import ( + MessageResponse, +) from .auth import ( AdminConfigResponse, ChangePasswordRequest, @@ -34,7 +37,9 @@ from .attackers import ( ) from .deploy import ( DeployIniRequest, + DeployResponse, MutateIntervalRequest, + PurgeResponse, ) from .health import ( ComponentHealth, @@ -82,6 +87,7 @@ from .topology import ( NextIPResponse, NextSubnetResponse, NotEditableResponse, + ReapReportResponse, ServiceCatalogResponse, Topology, TopologyDecky, @@ -120,6 +126,8 @@ __all__ = [ "NullableString", "_BIG_TEXT", "_normalize_null", + # common + "MessageResponse", # auth "AdminConfigResponse", "ChangePasswordRequest", @@ -141,7 +149,9 @@ __all__ = [ "SmtpTarget", # deploy "DeployIniRequest", + "DeployResponse", "MutateIntervalRequest", + "PurgeResponse", # health "ComponentHealth", "HealthResponse", @@ -185,6 +195,7 @@ __all__ = [ "NextIPResponse", "NextSubnetResponse", "NotEditableResponse", + "ReapReportResponse", "ServiceCatalogResponse", "Topology", "TopologyDecky", diff --git a/decnet/web/db/models/common.py b/decnet/web/db/models/common.py new file mode 100644 index 00000000..feb41187 --- /dev/null +++ b/decnet/web/db/models/common.py @@ -0,0 +1,15 @@ +"""Generic response shapes used across multiple router domains.""" +from __future__ import annotations + +from pydantic import BaseModel + + +class MessageResponse(BaseModel): + """Standard envelope for mutations whose only payload is a status message. + + Pinning the wire shape at the decorator (``response_model=MessageResponse``) + prevents a handler that accidentally returns a richer dict — e.g. a user + row with ``password_hash`` — from leaking extra fields to the client. + """ + + message: str diff --git a/decnet/web/db/models/deploy.py b/decnet/web/db/models/deploy.py index a0990e48..59580850 100644 --- a/decnet/web/db/models/deploy.py +++ b/decnet/web/db/models/deploy.py @@ -17,3 +17,13 @@ class DeployIniRequest(BaseModel): # This field now enforces strict INI structure during Pydantic initialization. # The OpenAPI schema correctly shows it as a required string. ini_content: IniContent = PydanticField(..., description="A valid INI formatted string") + + +class DeployResponse(BaseModel): + message: str + mode: str + + +class PurgeResponse(BaseModel): + message: str + deleted: dict[str, int] diff --git a/decnet/web/db/models/topology.py b/decnet/web/db/models/topology.py index 29b9ac8b..20e8423d 100644 --- a/decnet/web/db/models/topology.py +++ b/decnet/web/db/models/topology.py @@ -420,3 +420,11 @@ class DeployAcceptedResponse(BaseModel): topology_id: str status: str dry_run: bool = False + + +class ReapReportResponse(BaseModel): + live_prefixes: list[str] + orphan_prefixes: list[str] + containers_removed: list[str] + networks_removed: list[str] + errors: list[str] diff --git a/decnet/web/router/auth/api_change_pass.py b/decnet/web/router/auth/api_change_pass.py index 592b11e1..4ed27a57 100644 --- a/decnet/web/router/auth/api_change_pass.py +++ b/decnet/web/router/auth/api_change_pass.py @@ -5,7 +5,7 @@ from fastapi import APIRouter, Depends, HTTPException, status from decnet.telemetry import traced as _traced from decnet.web.auth import ahash_password, averify_password from decnet.web.dependencies import get_current_user_unchecked, invalidate_user_cache, repo -from decnet.web.db.models import ChangePasswordRequest +from decnet.web.db.models import ChangePasswordRequest, MessageResponse router = APIRouter() @@ -13,6 +13,7 @@ router = APIRouter() @router.post( "/auth/change-password", tags=["Authentication"], + response_model=MessageResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, diff --git a/decnet/web/router/config/api_manage_users.py b/decnet/web/router/config/api_manage_users.py index 70e0fe99..ff13e44b 100644 --- a/decnet/web/router/config/api_manage_users.py +++ b/decnet/web/router/config/api_manage_users.py @@ -8,8 +8,9 @@ from decnet.web.dependencies import require_admin, invalidate_user_cache, repo from decnet.web.router.config.api_get_config import invalidate_list_users_cache from decnet.web.db.models import ( CreateUserRequest, - UpdateUserRoleRequest, + MessageResponse, ResetUserPasswordRequest, + UpdateUserRoleRequest, UserResponse, ) @@ -19,6 +20,7 @@ router = APIRouter() @router.post( "/config/users", tags=["Configuration"], + response_model=UserResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, @@ -56,6 +58,7 @@ async def api_create_user( @router.delete( "/config/users/{user_uuid}", tags=["Configuration"], + response_model=MessageResponse, responses={ 401: {"description": "Could not validate credentials"}, 403: {"description": "Admin access required / cannot delete self"}, @@ -81,6 +84,7 @@ async def api_delete_user( @router.put( "/config/users/{user_uuid}/role", tags=["Configuration"], + response_model=MessageResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, @@ -111,6 +115,7 @@ async def api_update_user_role( @router.put( "/config/users/{user_uuid}/reset-password", tags=["Configuration"], + response_model=MessageResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, diff --git a/decnet/web/router/config/api_reinit.py b/decnet/web/router/config/api_reinit.py index ebdd1c74..bab17f8d 100644 --- a/decnet/web/router/config/api_reinit.py +++ b/decnet/web/router/config/api_reinit.py @@ -2,6 +2,7 @@ from fastapi import APIRouter, Depends, HTTPException from decnet.env import DECNET_DEVELOPER from decnet.telemetry import traced as _traced +from decnet.web.db.models import PurgeResponse from decnet.web.dependencies import require_admin, repo router = APIRouter() @@ -10,6 +11,7 @@ router = APIRouter() @router.delete( "/config/reinit", tags=["Configuration"], + response_model=PurgeResponse, responses={ 401: {"description": "Could not validate credentials"}, 403: {"description": "Admin access required or developer mode not enabled"}, diff --git a/decnet/web/router/config/api_update_config.py b/decnet/web/router/config/api_update_config.py index a7feee3b..94800102 100644 --- a/decnet/web/router/config/api_update_config.py +++ b/decnet/web/router/config/api_update_config.py @@ -2,7 +2,7 @@ from fastapi import APIRouter, Depends from decnet.telemetry import traced as _traced from decnet.web.dependencies import require_admin, repo -from decnet.web.db.models import DeploymentLimitRequest, GlobalMutationIntervalRequest +from decnet.web.db.models import DeploymentLimitRequest, GlobalMutationIntervalRequest, MessageResponse router = APIRouter() @@ -10,6 +10,7 @@ router = APIRouter() @router.put( "/config/deployment-limit", tags=["Configuration"], + response_model=MessageResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, @@ -29,6 +30,7 @@ async def api_update_deployment_limit( @router.put( "/config/global-mutation-interval", tags=["Configuration"], + response_model=MessageResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, diff --git a/decnet/web/router/fleet/api_deploy_deckies.py b/decnet/web/router/fleet/api_deploy_deckies.py index 5371ef77..9994981d 100644 --- a/decnet/web/router/fleet/api_deploy_deckies.py +++ b/decnet/web/router/fleet/api_deploy_deckies.py @@ -9,7 +9,7 @@ from decnet.engine import deploy as _deploy from decnet.ini_loader import load_ini_from_string from decnet.network import detect_interface, detect_subnet, get_host_ip from decnet.web.dependencies import require_admin, repo -from decnet.web.db.models import DeployIniRequest +from decnet.web.db.models import DeployIniRequest, DeployResponse from decnet.web.router.swarm.api_deploy_swarm import dispatch_decnet_config log = get_logger("api") @@ -20,6 +20,7 @@ router = APIRouter() @router.post( "/deckies/deploy", tags=["Fleet Management"], + response_model=DeployResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, diff --git a/decnet/web/router/fleet/api_mutate_decky.py b/decnet/web/router/fleet/api_mutate_decky.py index 7f2e0950..14795036 100644 --- a/decnet/web/router/fleet/api_mutate_decky.py +++ b/decnet/web/router/fleet/api_mutate_decky.py @@ -3,6 +3,7 @@ from fastapi import APIRouter, Depends, HTTPException, Path from decnet.telemetry import traced as _traced from decnet.mutator import mutate_decky +from decnet.web.db.models import MessageResponse from decnet.web.dependencies import require_admin, repo router = APIRouter() @@ -11,6 +12,7 @@ router = APIRouter() @router.post( "/deckies/{decky_name}/mutate", tags=["Fleet Management"], + response_model=MessageResponse, responses={ 401: {"description": "Could not validate credentials"}, 403: {"description": "Insufficient permissions"}, diff --git a/decnet/web/router/fleet/api_mutate_interval.py b/decnet/web/router/fleet/api_mutate_interval.py index 10afba97..d788ff3b 100644 --- a/decnet/web/router/fleet/api_mutate_interval.py +++ b/decnet/web/router/fleet/api_mutate_interval.py @@ -3,7 +3,7 @@ from fastapi import APIRouter, Depends, HTTPException from decnet.telemetry import traced as _traced from decnet.config import DecnetConfig from decnet.web.dependencies import require_admin, repo -from decnet.web.db.models import MutateIntervalRequest +from decnet.web.db.models import MessageResponse, MutateIntervalRequest router = APIRouter() @@ -17,6 +17,7 @@ def _parse_duration(s: str) -> int: @router.put("/deckies/{decky_name}/mutate-interval", tags=["Fleet Management"], + response_model=MessageResponse, responses={ 400: {"description": "Bad Request (e.g. malformed JSON)"}, 401: {"description": "Could not validate credentials"}, diff --git a/decnet/web/router/topology/api_reap_orphans.py b/decnet/web/router/topology/api_reap_orphans.py index b9201213..98335203 100644 --- a/decnet/web/router/topology/api_reap_orphans.py +++ b/decnet/web/router/topology/api_reap_orphans.py @@ -18,6 +18,7 @@ from fastapi import APIRouter, Depends from decnet.engine.reaper import reap_orphan_topology_resources from decnet.telemetry import traced as _traced +from decnet.web.db.models import ReapReportResponse from decnet.web.dependencies import repo, require_admin router = APIRouter() @@ -26,6 +27,7 @@ router = APIRouter() @router.post( "/reap-orphans", tags=["MazeNET Topologies"], + response_model=ReapReportResponse, responses={ 401: {"description": "Missing or invalid credentials"}, 403: {"description": "Insufficient permissions"}, diff --git a/development/THREAT_MODEL.md b/development/THREAT_MODEL.md index 26b7854c..672f5526 100644 --- a/development/THREAT_MODEL.md +++ b/development/THREAT_MODEL.md @@ -227,7 +227,7 @@ Each sub-flow below gets its own table. Status codes: | I | Schema enumeration via schemathesis-style fuzzing | A | Schemathesis contract tests document 400/422 shape; an attacker learning the schema gains nothing beyond the public OpenAPI spec. See `feedback_schemathesis_400.md`. | | D | Unbounded result set via missing `limit` | M | Every query endpoint declares `limit: int = Query(..., le=N)` at the FastAPI layer — `/logs`, `/attackers`, `/bounties`, `/attacker-commands`, `/topologies/{id}` (`le=1000`), `/topologies` (`le=500`), `/transcripts` (`le=5000`). Cap enforced at pydantic validation, before the handler runs. | | D | Deep-pagination scan via large `offset` | M | Every `offset` param is `Query(0, ge=0, le=2147483647)` (INT32 max). At that scale SQLite returns empty immediately once rows exhaust; the point is to keep callers within a range the indexer can skip cheaply. `api_get_transcript.py:147` and `api_list_topologies.py:29` brought in line 2026-04-24. | -| D | Expensive `LIKE '%foo%'` on non-indexed column | **?** | Verify: free-text `q` params hit an FTS5 virtual table or indexed column, not `LIKE`-scan of a large table. | +| D | Expensive `LIKE '%foo%'` on non-indexed column | A | See DA-09. `/logs?search=` LIKE-scans four columns on the unbounded logs table; `/attackers?search=` LIKE-scans `attacker.ip`. Both routes are admin-gated. Cost-to-caller is bounded by `limit ≤ 1000` and by operator-level reverse-proxy rate limiting (see DA-04). Performance upgrade to FTS5 is tracked separately; within the current admin-trust model the cost is acceptable. | | D | Repeated expensive queries from single user | A | Per-user rate limiting is out of scope pre-v1. Operator-deployment mitigation: reverse-proxy rate limit. | | E | Filter params allow reading across tenants (future multi-tenant) | X | Multi-tenant is not in the v1 model; revisit when tenants exist. | @@ -239,7 +239,7 @@ Each sub-flow below gets its own table. Status codes: | T | Replay of a captured mutation request | A | No nonce/idempotency-key pre-v1. Accepted: admin role already has full mutation power; replay gains nothing a fresh request couldn't. Revisit if multi-admin audit becomes a requirement. | | T | Concurrent-write race corrupting state | **?** | Verify: SQLModel session scoping + DB-level constraints cover the likely races (user creation, topology CRUD). | | R | Admin denies having mutated | M | Actor UUID + timestamp logged on every mutation. | -| I | Mutation response returns internal state not meant for client | **?** | Verify per-route response_model shape. | +| I | Mutation response returns internal state not meant for client | M | Every mutation route that returns a dict-shaped body now pins `response_model=...` at the decorator: `MessageResponse` for `{"message": ...}` envelopes, purpose-built models (`DeployResponse`, `PurgeResponse`, `ReapReportResponse`, `UserResponse`) for richer shapes. FastAPI strips undocumented extra fields at serialization time, so a handler that accidentally returned a full user row (including `password_hash`) would only ship declared fields. `Response`/`ORJSONResponse` routes bypass response_model intentionally and are audited individually. | | D | Malformed body triggers expensive validation / oversized payload | M | FastAPI enforces content-length at ASGI layer; Pydantic short-circuits on type mismatch. | | D | Destructive mutation storm (e.g. delete-all-users) | A | Admin role is trusted; protecting admins from themselves is out of scope. | | E | Mutation bypasses role check via missing `require_admin` | M | `tests/api/test_rbac_contract.py::test_admin_route_rejects_viewer` parametrizes every route classified admin by FastAPI-dependency introspection (identity-match on the `require_admin` closure) and asserts viewer JWT → 403. A missing `require_admin` would reclassify the route away from "admin" and break the viewer route's non-403 assertion, so the check is bidirectional. | @@ -281,6 +281,7 @@ Consolidated for easy reference: | DA-06 | Destructive admin mutations not protected against the admin | Trusted-admin assumption; protecting root from root is out of scope | Multi-admin RBAC with mutual-approval workflows | | DA-07 | SSE token in query string | No alternative in the SSE spec; operator must control access-log handling | Move to WebSocket with in-band auth | | DA-08 | Reverse-proxy deployments collapse per-IP rate-limit bucket to one shared bucket | `X-Forwarded-For` is spoofable by any client; trusting it defeats the rate limit. Operators behind a proxy get coarser granularity but no spoofing lane. | Verified-proxy config lands (allow-list of proxy IPs whose `X-Forwarded-For` we trust) | +| DA-09 | Admin-initiated `LIKE '%q%'` scan on `/logs` or `/attackers` ties up a worker for the duration of the scan on a large dataset | Both routes are admin-gated; the admin role already carries DA-06 (protecting admins from themselves is out of scope). `limit ≤ 1000` caps the result page size, and per-user rate-limiting is operator-scope per DA-04. FTS5 is a performance upgrade, not a security change, under the current trust model. | Logs table growth causes operator-observable latency on the LIKE path, OR trust model changes (multi-tenant / SaaS / untrusted-admin delegation) | ### Needs-verification checklist (Dashboard ↔ API) @@ -298,8 +299,8 @@ code" or "accepted, add to table above." - [ ] Role-scoped repo methods OR per-route pre-filter for viewer queries (pick one, document it). - [x] ~~Every query endpoint has a server-side hard cap independent of `limit`.~~ All 7 query endpoints declare `Query(..., le=N)` at the FastAPI layer; enforced pre-handler. - [x] ~~`offset` is capped OR pagination is cursor-based OR deep-offset is cheap.~~ Every `offset` now uses `le=2147483647` (`api_list_topologies.py:29` and `api_get_transcript.py:147` brought in line 2026-04-24; others already capped). -- [ ] Free-text `q` parameters hit an indexed/FTS5 column, never a full-table `LIKE` scan. -- [ ] Per-route response_model shape audit on mutations. +- [x] ~~Free-text `q` parameters hit an indexed/FTS5 column, never a full-table `LIKE` scan.~~ Moved to accepted risk **DA-09** — admin-only surface, `limit` capped, operator rate-limit applies. Revisit if logs-table LIKE latency becomes operator-observable OR if the trust model changes (multi-tenant / SaaS). +- [x] ~~Per-route response_model shape audit on mutations.~~ Every dict-returning mutation now declares `response_model=...`. `MessageResponse` covers the 8 `{"message": ...}` envelopes; `DeployResponse`/`PurgeResponse`/`ReapReportResponse`/`UserResponse` cover the richer shapes. 204-No-Content routes and manual `Response`/`ORJSONResponse` routes are explicitly scoped out (no body to validate). - [x] ~~Contract test asserting every mutation route returns 403 for viewer.~~ Covered by `test_rbac_contract.py` (same test also covers read routes — classification is by dependency, not HTTP verb). - [ ] SSE handler applies per-connection role filter before forwarding events. - [ ] Per-user concurrent SSE connection cap. @@ -337,7 +338,7 @@ regardless of component: | Component | ID | Summary | |-----------|----|---------| -| Dashboard↔API | DA-01..DA-07 | See component section. | +| Dashboard↔API | DA-01..DA-09 | See component section. | ## Components not yet modeled @@ -362,3 +363,4 @@ In priority order: | 2026-04-24 | F7: "MIME sniffing" moved from **?** to **M** (explicit `Content-Disposition`/`nosniff` headers + test). F7: "path-traversal" row reworded to point at the existing `_resolve_artifact_path` containment check. Fleet-deploy `detail=str(e)` audit resolved — all four sites documented as deliberate, admin-gated, no sanitization needed. | ANTI | | 2026-04-24 | F2/I + F5/E moved from **?** to **M** via new `tests/api/test_rbac_contract.py` — classifies every APIRoute by FastAPI-dependency introspection and asserts viewer JWT → 403 on admin routes, non-401/403 on viewer routes. Role hints deliberately omitted from OpenAPI spec. SSE routes skipped (F6 scope). | ANTI | | 2026-04-24 | F4/T (ORM sort injection), F4/D (unbounded `limit`), F4/D (deep `offset`) all moved from **?** to **M**. Limit caps were already universal; sort is pattern-validated on the only surface that exposes it; added `le=2147483647` to the two offset params that were unbounded (`api_list_topologies.py`, `api_get_transcript.py`). | ANTI | +| 2026-04-24 | F5/I moved from **?** to **M** via `response_model=...` on every dict-returning mutation (`MessageResponse` + purpose-built models). F4/D "expensive `LIKE`" moved from **?** to **A** under new accepted risk DA-09 — admin-only surface, operator-scope rate limiting, `limit` cap. FTS5 kept as a performance TODO, not a security blocker. | ANTI |