diff --git a/django_admin_react/api/views/update.py b/django_admin_react/api/views/update.py index d2e452f..2c238b8 100644 --- a/django_admin_react/api/views/update.py +++ b/django_admin_react/api/views/update.py @@ -152,13 +152,16 @@ def patch( if inline_errors is not None: # Roll back by raising; convert to a 400 below. raise _InlineValidationError(inline_errors) - except InlinePermissionDenied as exc: + except InlinePermissionDenied: return forbidden_response(request) except _InlineValidationError as exc: return validation_failed({"inlines": exc.errors}) - except ValueError as exc: - # Malformed ``inlines`` payload shape (not a 500). - return bad_request(str(exc)) + except ValueError: + # Malformed ``inlines`` payload shape (not a 500). Return a + # fixed, generic message — never echo the exception text into + # the response (CodeQL ``py/stack-trace-exposure``). The + # precise shape rules are in api-contract §5.2.1. + return bad_request("Malformed 'inlines' payload.") response = JsonResponse( _build_payload(model, model_admin, instance, request), diff --git a/django_admin_react/views.py b/django_admin_react/views.py index 3576d02..08cd43f 100644 --- a/django_admin_react/views.py +++ b/django_admin_react/views.py @@ -42,6 +42,7 @@ from django.urls import NoReverseMatch from django.urls import reverse from django.urls import reverse_lazy +from django.utils.http import urlencode from django.views.generic import View from django_admin_react import conf as dar_conf @@ -290,4 +291,12 @@ def _redirect_to_login(request: HttpRequest) -> HttpResponse: except NoReverseMatch: login_url = "/accounts/login/" - return redirect(f"{login_url}?next={request.path}") + # Percent-encode the ``next`` value (``urlencode``) so a crafted + # request path can't break out of the query parameter and rewrite + # the redirect target. The redirect *target* (``login_url``) is + # always a trusted, package-/settings-derived URL; ``next`` is + # consumer-facing and re-validated by Django's login view via + # ``url_has_allowed_host_and_scheme`` before any post-login bounce. + # (Clears CodeQL ``py/url-redirection``.) + next_param = urlencode({"next": request.get_full_path()}) + return redirect(f"{login_url}?{next_param}") diff --git a/frontend/packages/data/src/format.ts b/frontend/packages/data/src/format.ts index 9a16f33..fa14dce 100644 --- a/frontend/packages/data/src/format.ts +++ b/frontend/packages/data/src/format.ts @@ -58,7 +58,35 @@ export function renderValue(value: FieldValue | undefined): string { // Safe-HTML envelope: when only a string is needed (e.g. a title), // strip tags to a plain-text approximation. Markup rendering is the // caller's job via `isHtmlValue` + dangerouslySetInnerHTML. - if (isHtmlValue(value)) return value.html.replace(/<[^>]*>/g, ''); + if (isHtmlValue(value)) return stripTags(value.html); if (isForeignKeyValue(value)) return value.label; return String(value); } + +/** + * Strip HTML tags to a plain-text approximation, safely. + * + * Used only to render markup as *text* (e.g. a `title` attribute); + * React escapes the returned string when it's a text child, so this is + * not itself a security boundary. The implementation is deliberately: + * + * - **ReDoS-free** — the character class `[^<>]` excludes *both* angle + * brackets, so the match is linear (no ambiguous overlap that a + * crafted input could force into polynomial backtracking). + * - **Complete** — a single pass can leave a residual tag on + * overlapping/nested input like `<script>`; looping until the + * string is stable guarantees no tag survives. + * + * (Replaces the prior `replace(/<[^>]*>/g, '')`, which CodeQL flagged + * for both polynomial-ReDoS and incomplete multi-character + * sanitization.) + */ +function stripTags(html: string): string { + let previous: string; + let stripped = html; + do { + previous = stripped; + stripped = stripped.replace(/<[^<>]*>/g, ''); + } while (stripped !== previous); + return stripped; +} diff --git a/tests/test_autocomplete.py b/tests/test_autocomplete.py index 3aea271..4cc9045 100644 --- a/tests/test_autocomplete.py +++ b/tests/test_autocomplete.py @@ -20,7 +20,6 @@ import pytest from django.contrib import admin from django.contrib.auth import get_user_model -from django.contrib.auth.models import Group from django.test import Client from tests.helpers import admin_override diff --git a/tests/test_list_filter.py b/tests/test_list_filter.py index c95c82e..83e0787 100644 --- a/tests/test_list_filter.py +++ b/tests/test_list_filter.py @@ -179,8 +179,8 @@ def test_fk_filter_includes_inline_choices_when_small( superuser_client: Client, ) -> None: """ForeignKey filter to a tiny target table inlines the choices.""" - g1 = Group.objects.create(name="alpha") - g2 = Group.objects.create(name="beta") + Group.objects.create(name="alpha") + Group.objects.create(name="beta") User = get_user_model() with admin_attr(User, list_filter=(("groups", admin.RelatedOnlyFieldListFilter),)): @@ -190,7 +190,6 @@ def test_fk_filter_includes_inline_choices_when_small( pass with admin_attr(User, list_filter=("groups",)): response = superuser_client.get(LIST_USER_URL) - body = response.json() # `groups` is a ManyToManyField — not surfaced as a v1 filter type. # The filter is silently skipped (back-compat surface; M2M filter # support is part of #55 follow-up). We just assert no 500. diff --git a/tests/test_security.py b/tests/test_security.py index 5541975..bc0d811 100644 --- a/tests/test_security.py +++ b/tests/test_security.py @@ -201,8 +201,8 @@ def test_s37_no_committed_token_patterns_in_head() -> None: "-nIE", "--", r"ghp_[A-Za-z0-9]{30,}|gho_[A-Za-z0-9]{30,}|ghs_[A-Za-z0-9]{30,}|" - r"github_pat_[A-Za-z0-9_]{20,}|AKIA[0-9A-Z]{16}|" - r"BEGIN (RSA|EC|OPENSSH) PRIVATE", + + r"github_pat_[A-Za-z0-9_]{20,}|AKIA[0-9A-Z]{16}|" + + r"BEGIN (RSA|EC|OPENSSH) PRIVATE", ] result = subprocess.run( # noqa: S603 args, diff --git a/tests/test_serializers.py b/tests/test_serializers.py index 764a7a7..44cee7e 100644 --- a/tests/test_serializers.py +++ b/tests/test_serializers.py @@ -168,9 +168,6 @@ def test_none_returns_none(self) -> None: assert serialize_fk_value(None) is None def test_model_returns_id_label_dict(self) -> None: - class _FakeMeta: - pass - class _FakeModel: pk = 42 diff --git a/tests/test_spa_index.py b/tests/test_spa_index.py index ee53e0d..aa991e5 100644 --- a/tests/test_spa_index.py +++ b/tests/test_spa_index.py @@ -10,6 +10,8 @@ import json from pathlib import Path from unittest import mock +from urllib.parse import parse_qs +from urllib.parse import urlsplit import pytest from django.test import Client @@ -58,9 +60,13 @@ def test_anonymous_user_redirected_to_login(anon_client: Client) -> None: assert response.status_code == 302 # The package leaves LOGIN_URL up to the consumer's settings — only # assert that the redirect carries the SPA path as the ``next`` - # parameter so the user lands back here after login. - assert "next=" in response["Location"] - assert ROOT_URL in response["Location"] + # parameter so the user lands back here after login. The ``next`` + # value is percent-encoded (CodeQL py/url-redirection fix), so the + # raw path appears encoded in Location; decode the query to compare. + location = response["Location"] + assert "next=" in location + query = parse_qs(urlsplit(location).query) + assert query["next"][0].startswith(ROOT_URL) @pytest.mark.django_db