Skip to content

Commit 72ea09f

Browse files
Berik AshimovBerik Ashimov
authored andcommitted
Security hardening (v0.3.0)
- Breaking: admin panel is fail-closed; requests denied with 401 when no auth callable is configured (CWE-306) - Detail view renders detail_fields() so hidden columns no longer leak (CWE-200) - Extended sensitive/authorization field detection; matched fields forced read-only to prevent mass-assignment (CWE-915) - CSRF cookie sets HttpOnly and Max-Age (CWE-1004) - Security headers + CSP on admin HTML responses (CWE-1021) - Non-integer page no longer 500s; page clamped (CWE-20) - Security events logged (CWE-778)
1 parent df0ad15 commit 72ea09f

10 files changed

Lines changed: 178 additions & 19 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,17 @@
11
# Changelog
22

3+
## 0.3.0 — 2026-06-10
4+
5+
Security hardening — **breaking**.
6+
7+
- **Breaking:** the admin panel is now fail-closed — when no `auth` callable is configured, all requests are denied with HTTP 401 (previously they were silently allowed) (CWE-306). Pass an `auth` callable to enable access.
8+
- Detail views render only `detail_fields()` (derived from `list_display` when configured), so columns hidden from the list no longer leak on the detail page (CWE-200).
9+
- Sensitive/authorization field detection extended to `role`/`admin`/`superuser`/`permission`/`privilege`/`staff`/`scope`; matched fields are forced read-only unless explicitly opted into `form_fields`, preventing mass-assignment privilege escalation (CWE-915).
10+
- CSRF cookie now sets `HttpOnly` and `Max-Age` (CWE-1004).
11+
- Admin HTML responses carry `X-Frame-Options`, `X-Content-Type-Options`, `Referrer-Policy`, and a restrictive `Content-Security-Policy` (CWE-1021).
12+
- Non-integer `?page=` values no longer cause a 500; page is clamped to a maximum (CWE-20).
13+
- Security events (auth/CSRF failures, blocked mutations) are now logged (CWE-778).
14+
315
## 0.2.0 — 2026-05-16
416

517
Security hardening — **breaking**.

‎pyproject.toml‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ build-backend = "hatchling.build"
44

55
[project]
66
name = "hawkapi-admin"
7-
version = "0.2.0"
7+
version = "0.3.0"
88
description = "Auto-generated admin UI for HawkAPI + SQLAlchemy — list, detail, create, edit, delete with no boilerplate"
99
readme = "README.md"
1010
license = { file = "LICENSE" }

‎src/hawkapi_admin/_admin.py‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,13 @@
1515
AuthCallable = Callable[[Any], Awaitable[None]]
1616

1717

18+
async def _deny_all_auth(_request: Any) -> None:
19+
"""Default fail-closed auth: deny every request when no auth is configured."""
20+
from hawkapi import HTTPException
21+
22+
raise HTTPException(401, detail="Admin authentication is not configured")
23+
24+
1825
class Admin:
1926
"""Top-level admin object. Build one per HawkAPI app and register resources."""
2027

@@ -29,14 +36,17 @@ def __init__(
2936
self.title = title
3037
self.url_prefix = url_prefix.rstrip("/") or "/admin"
3138
self.resources: dict[str, ModelResource] = {}
32-
self.auth: AuthCallable | None = auth
3339
self.csrf_enabled: bool = csrf_enabled
3440
if auth is None:
3541
warnings.warn(
36-
"hawkapi-admin: no auth configured; all admin endpoints are publicly accessible",
42+
"hawkapi-admin: no auth configured; all admin endpoints will deny "
43+
"access (fail-closed). Pass an `auth` callable to enable the panel.",
3744
UserWarning,
3845
stacklevel=2,
3946
)
47+
self.auth: AuthCallable = _deny_all_auth
48+
else:
49+
self.auth = auth
4050

4151
def register(self, resource: ModelResource | type[Any], **kwargs: Any) -> ModelResource:
4252
"""Register ``resource`` with the admin. Accepts a SQLAlchemy model class
@@ -57,16 +67,15 @@ def register(self, resource: ModelResource | type[Any], **kwargs: Any) -> ModelR
5767
return resource
5868

5969
async def _check_auth(self, request: Any) -> None:
60-
if self.auth is not None:
61-
await self.auth(request)
70+
await self.auth(request)
6271

6372
def attach(self, app: Any) -> None:
6473
"""Wire the admin routes onto ``app``."""
6574
prefix = self.url_prefix
6675
admin = self
67-
if self.auth is None:
76+
if self.auth is _deny_all_auth:
6877
_logger.warning(
69-
"hawkapi-admin: no auth configured; all admin endpoints are publicly accessible"
78+
"hawkapi-admin: no auth configured; all admin endpoints deny access (fail-closed)"
7079
)
7180

7281
async def _index(request: Any) -> Any:

‎src/hawkapi_admin/_csrf.py‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,10 @@
66
* Templates render the same token as a hidden ``<input name="_csrf">``.
77
* POST handlers compare cookie and form value via ``hmac.compare_digest``.
88
9-
The cookie is intentionally NOT ``HttpOnly`` so Jinja-rendered server-side
10-
templates can echo it back without needing JavaScript — but the template
11-
reads it from the request scope, not the cookie itself, so the lack of
12-
HttpOnly is purely a defense-in-depth concern (an attacker who can run JS
13-
on the admin origin has already lost).
9+
The cookie is ``HttpOnly``: templates echo the token from the request scope
10+
(server-side), not from the cookie via JavaScript, so there is no reason to
11+
expose it to client scripts. The cookie is also short-lived (``Max-Age``) so
12+
tokens rotate rather than living indefinitely.
1413
"""
1514

1615
from __future__ import annotations
@@ -51,7 +50,7 @@ def ensure_csrf_token(request: Any) -> str:
5150

5251
def build_set_cookie_header(token: str, *, path: str) -> str:
5352
"""Build a Set-Cookie header value for the CSRF cookie."""
54-
return f"{COOKIE_NAME}={token}; Path={path}; Secure; SameSite=Lax"
53+
return f"{COOKIE_NAME}={token}; Path={path}; Max-Age=86400; Secure; HttpOnly; SameSite=Lax"
5554

5655

5756
def validate_csrf(request: Any, form: Any) -> None:

‎src/hawkapi_admin/_resource.py‎

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,11 @@
1111

1212
from ._inspect import FieldSpec, list_fields, primary_key_column
1313

14-
_SENSITIVE_NAME_RE = re.compile(r"(password|secret|token|key|hash)$", re.IGNORECASE)
14+
_SENSITIVE_NAME_RE = re.compile(
15+
r"(password|secret|token|key|hash"
16+
r"|role|admin|superuser|permission|privilege|staff|scope)$",
17+
re.IGNORECASE,
18+
)
1519

1620

1721
@dataclass
@@ -54,6 +58,7 @@ class ModelResource:
5458

5559
_fields: list[FieldSpec] = field(default_factory=list, init=False)
5660
_pk_name: str = field(default="", init=False)
61+
_list_display_explicit: bool = field(default=False, init=False)
5762

5863
def __post_init__(self) -> None:
5964
if not self.name:
@@ -64,20 +69,35 @@ def __post_init__(self) -> None:
6469
self.label_plural = self.label + "s"
6570
self._fields = list_fields(self.model)
6671
self._pk_name = primary_key_column(self.model)
72+
# Remember whether the developer explicitly opted a field into the form,
73+
# so enforcement below doesn't override a deliberate choice.
74+
explicit_form_fields = set(self.form_fields)
75+
self._list_display_explicit = bool(self.list_display)
6776
if not self.list_display:
6877
self.list_display = tuple(f.name for f in self._fields)
6978
if not self.form_fields:
7079
self.form_fields = tuple(f.name for f in self._fields if not f.primary_key)
80+
# Enforce: sensitive / authorization-bearing fields are made read-only
81+
# (and thus excluded from editable_fields) unless the developer explicitly
82+
# listed them in form_fields. This prevents mass-assignment of fields like
83+
# is_admin / role / password_hash via the edit form.
84+
readonly = list(self.readonly_fields)
7185
for name in self.form_fields:
7286
if name in self.readonly_fields:
7387
continue
7488
if _SENSITIVE_NAME_RE.search(name):
7589
warnings.warn(
7690
f"hawkapi-admin: field {self.model.__name__}.{name} looks "
91+
f"sensitive; forcing it read-only to prevent mass-assignment"
92+
if name not in explicit_form_fields
93+
else f"hawkapi-admin: field {self.model.__name__}.{name} looks "
7794
f"sensitive but is not in readonly_fields",
7895
UserWarning,
7996
stacklevel=2,
8097
)
98+
if name not in explicit_form_fields and name not in readonly:
99+
readonly.append(name)
100+
self.readonly_fields = tuple(readonly)
81101

82102
@property
83103
def fields(self) -> list[FieldSpec]:
@@ -96,6 +116,18 @@ def primary_key(self) -> str:
96116
def display_fields(self) -> list[FieldSpec]:
97117
return [self.field(n) for n in self.list_display]
98118

119+
def detail_fields(self) -> list[FieldSpec]:
120+
"""Fields shown on the detail page.
121+
122+
Mirrors the configured ``list_display`` so that columns deliberately
123+
hidden from the list (e.g. ``password_hash``) are not exposed on the
124+
detail view either. Falls back to every mapped column only when no
125+
``list_display`` was configured.
126+
"""
127+
if self._list_display_explicit:
128+
return self.display_fields()
129+
return list(self._fields)
130+
99131
def editable_fields(self) -> list[FieldSpec]:
100132
return [self.field(n) for n in self.form_fields if n not in self.readonly_fields]
101133

‎src/hawkapi_admin/_routes.py‎

Lines changed: 48 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import logging
56
from pathlib import Path
67
from typing import Any
78

@@ -24,6 +25,8 @@
2425
from ._resource import ModelResource
2526

2627
_TEMPLATES_DIR = Path(__file__).parent / "templates"
28+
_MAX_PAGE = 10_000
29+
_logger = logging.getLogger("hawkapi_admin")
2730

2831

2932
def _build_env() -> Environment:
@@ -87,9 +90,25 @@ async def _render(
8790
tpl = _env.get_template(template)
8891
body = await tpl.render_async(csrf_token=csrf_token, **context)
8992
resp = HTMLResponse(body, status_code=status_code)
93+
_apply_security_headers(resp)
9094
return _attach_csrf_cookie(request, resp, _admin)
9195

9296

97+
# Inline styles (a <style> block plus a handful of style="" attrs) and one
98+
# inline onclick confirm dialog require 'unsafe-inline'; everything else is
99+
# locked down to the admin origin.
100+
_CSP = "default-src 'self'; style-src 'self' 'unsafe-inline'; script-src 'unsafe-inline'"
101+
102+
103+
def _apply_security_headers(response: Response) -> None:
104+
"""Stamp hardening headers on every admin HTML response."""
105+
headers = response.headers
106+
headers["X-Frame-Options"] = "DENY"
107+
headers["X-Content-Type-Options"] = "nosniff"
108+
headers["Referrer-Policy"] = "same-origin"
109+
headers["Content-Security-Policy"] = _CSP
110+
111+
93112
async def index(request: Request, *, admin: Any) -> Response:
94113
return await _render(
95114
request,
@@ -104,7 +123,11 @@ async def list_resource(request: Request, *, admin: Any) -> Response:
104123
name = str(request.path_params["resource"])
105124
res = _resource(admin, name)
106125
q = request.query_params.get("q", "")
107-
page = max(1, int(request.query_params.get("page", "1") or 1))
126+
try:
127+
page = max(1, int(request.query_params.get("page", "1") or 1))
128+
except (ValueError, TypeError):
129+
page = 1
130+
page = min(page, _MAX_PAGE)
108131
sessions = _session_factory(request)
109132

110133
stmt = select(res.model)
@@ -156,13 +179,17 @@ async def edit_form(request: Request, *, admin: Any) -> Response:
156179
obj = None
157180
if pk is not None:
158181
if not res.can_update:
182+
_logger.warning(
183+
"admin blocked edit form on resource %r pk=%r (can_update=False)", name, pk
184+
)
159185
raise HTTPException(403)
160186
async with sessions(commit=False) as sess:
161187
obj = await sess.get(res.model, pk)
162188
if obj is None:
163189
raise HTTPException(404)
164190
else:
165191
if not res.can_create:
192+
_logger.warning("admin blocked new form on resource %r (can_create=False)", name)
166193
raise HTTPException(403)
167194
values: dict[str, Any] = (
168195
{f.name: getattr(obj, f.name, "") for f in res.editable_fields()} if obj else {}
@@ -212,12 +239,18 @@ async def save(request: Request, *, admin: Any) -> Response:
212239
sessions = _session_factory(request)
213240
form = await request.form()
214241
if getattr(admin, "csrf_enabled", True):
215-
validate_csrf(request, form)
242+
try:
243+
validate_csrf(request, form)
244+
except HTTPException:
245+
_logger.warning("admin CSRF validation failed for save on resource %r", name)
246+
raise
216247

217248
# Authorization first — fail fast.
218249
if pk is None and not res.can_create:
250+
_logger.warning("admin blocked create on resource %r (can_create=False)", name)
219251
raise HTTPException(403)
220252
if pk is not None and not res.can_update:
253+
_logger.warning("admin blocked update on resource %r pk=%r (can_update=False)", name, pk)
221254
raise HTTPException(403)
222255

223256
creating = pk is None
@@ -266,6 +299,12 @@ async def save(request: Request, *, admin: Any) -> Response:
266299
values=values,
267300
errors={"_form": detail_msg},
268301
)
302+
_logger.info(
303+
"admin %s on resource %r pk=%r succeeded",
304+
"create" if creating else "update",
305+
name,
306+
new_pk,
307+
)
269308
return RedirectResponse(f"{admin.url_prefix}/{name}/{new_pk}", status_code=303)
270309

271310

@@ -283,17 +322,23 @@ async def delete(request: Request, *, admin: Any) -> Response:
283322
name = str(request.path_params["resource"])
284323
res = _resource(admin, name)
285324
if not res.can_delete:
325+
_logger.warning("admin blocked delete on resource %r (can_delete=False)", name)
286326
raise HTTPException(403)
287327
if getattr(admin, "csrf_enabled", True):
288328
form = await request.form()
289-
validate_csrf(request, form)
329+
try:
330+
validate_csrf(request, form)
331+
except HTTPException:
332+
_logger.warning("admin CSRF validation failed for delete on resource %r", name)
333+
raise
290334
pk = request.path_params["pk"]
291335
sessions = _session_factory(request)
292336
async with sessions() as sess:
293337
obj = await sess.get(res.model, pk)
294338
if obj is None:
295339
raise HTTPException(404)
296340
await sess.delete(obj)
341+
_logger.info("admin delete on resource %r pk=%r succeeded", name, pk)
297342
return RedirectResponse(f"{admin.url_prefix}/{name}", status_code=303)
298343

299344

‎src/hawkapi_admin/templates/detail.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
<h2>{{ resource.label }} #{{ obj | attr(resource.primary_key) }}</h2>
1010

1111
<div>
12-
{% for fs in resource.fields %}
12+
{% for fs in resource.detail_fields() %}
1313
<div class="field-row">
1414
<div class="muted">{{ fs.name }}</div>
1515
<div>{{ resource.render_value(obj, fs.name) }}</div>

‎tests/test_resource.py‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,37 @@ def test_sensitive_field_in_readonly_does_not_warn() -> None:
6565
ModelResource(model=Secret, readonly_fields=("password_hash",))
6666

6767

68+
def test_sensitive_field_forced_readonly() -> None:
69+
# password_hash is not explicitly in form_fields, so it must be auto-added
70+
# to readonly_fields and excluded from the editable form (mass-assignment guard).
71+
with pytest.warns(UserWarning, match="looks sensitive"):
72+
r = ModelResource(model=Secret)
73+
assert "password_hash" in r.readonly_fields
74+
assert "password_hash" not in [f.name for f in r.editable_fields()]
75+
76+
77+
def test_sensitive_field_explicit_form_field_stays_editable() -> None:
78+
# If the developer explicitly opts the field into the form, honor it (still warns).
79+
with pytest.warns(UserWarning, match="looks sensitive"):
80+
r = ModelResource(model=Secret, form_fields=("password_hash",))
81+
assert "password_hash" not in r.readonly_fields
82+
assert "password_hash" in [f.name for f in r.editable_fields()]
83+
84+
85+
def test_detail_fields_hides_columns_not_in_list_display() -> None:
86+
# Detail view must respect list_display so sensitive columns stay hidden.
87+
r = ModelResource(model=User, list_display=("id", "email"))
88+
names = [f.name for f in r.detail_fields()]
89+
assert names == ["id", "email"]
90+
assert "name" not in names
91+
92+
93+
def test_detail_fields_defaults_to_all_when_unconfigured() -> None:
94+
r = ModelResource(model=User)
95+
names = [f.name for f in r.detail_fields()]
96+
assert "email" in names and "name" in names
97+
98+
6899
def test_duplicate_register_raises() -> None:
69100
a = Admin(title="t", auth=_noop_auth, csrf_enabled=False)
70101
a.register(ModelResource(model=User))

0 commit comments

Comments
 (0)