From b4d6d4bb203570661ccd11700f6e3735a76336bd Mon Sep 17 00:00:00 2001 From: ixgadmin Date: Thu, 17 Sep 2026 14:48:16 +0000 Subject: [PATCH] Harden the markup hygiene and seed idempotency tests Replace the Bootstrap-prefix allowlist with a default-to-leak rule for bare hyphenated tokens, parse tags quote-aware so a '>' inside an earlier attribute cannot truncate the tag, render the batch edit and lineage screens in the sweep with a seeded batch, and cover stale evidence in the seed re-run test. --- apps/core/tests.py | 313 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 301 insertions(+), 12 deletions(-) diff --git a/apps/core/tests.py b/apps/core/tests.py index 7175173..ebca469 100644 --- a/apps/core/tests.py +++ b/apps/core/tests.py @@ -14,17 +14,25 @@ import re import shutil import tempfile import urllib.error +from decimal import Decimal +from html.parser import HTMLParser from pathlib import Path from unittest.mock import MagicMock, patch from django.conf import settings from django.contrib.auth import get_user_model from django.core.management import call_command -from django.test import Client, TestCase, override_settings +from django.test import Client, SimpleTestCase, TestCase, override_settings from apps.batches.models import ProcessingBatch from apps.capa.models import CAPARecord -from apps.compliance.models import ProductSpecification +from apps.compliance.models import ( + DueDiligenceCase, + EUDREvidence, + HarvestIntake, + IntakeBatchLineage, + ProductSpecification, +) from apps.qc.models import QCResult from .tiles import browser_tile_url, upstream_url @@ -36,6 +44,7 @@ OPS_ROUTES = [ "/ops/intakes/", "/ops/intakes/new/", "/ops/batches/", + "/ops/batches/new/", "/ops/qc/", "/ops/qc/verify/", "/ops/coa/", @@ -57,6 +66,31 @@ PORTAL_ROUTES = [ ] +def make_sweep_batch(): + """A batch with one lineage row, so the edit and lineage screens render. + + The route sweeps otherwise run against an empty database and never render + ``batch_form.html`` in edit mode or ``batch_lineage.html`` at all — only the + source guard covers them, and that is the weaker guard. + """ + batch = ProcessingBatch.objects.create(batch_reference="IXG-SWEEP-001") + intake = HarvestIntake.objects.create( + intake_reference="IXG-SWEEP-INTAKE-001", weight_kg=Decimal("100.00") + ) + IntakeBatchLineage.objects.create( + batch=batch, harvest_intake=intake, weight_allocated_kg=Decimal("40.00") + ) + return batch + + +def batch_screens(batch): + return [ + f"/ops/batches/{batch.pk}/", + f"/ops/batches/{batch.pk}/edit/", + f"/ops/batches/{batch.pk}/lineage/", + ] + + class PortalPagesRenderTests(TestCase): def make_user(self, email, role): return User.objects.create_user( @@ -76,7 +110,9 @@ class PortalPagesRenderTests(TestCase): self.assertNotIn(marker, body, f"{route} rendered an error page") def test_ops_pages_render(self): - self.sweep(self.make_user("ops-sweep@ixg.local", "ops"), OPS_ROUTES, "ops.ixgltd.com") + batch = make_sweep_batch() + routes = OPS_ROUTES + batch_screens(batch) + self.sweep(self.make_user("ops-sweep@ixg.local", "ops"), routes, "ops.ixgltd.com") def test_buyer_pages_render(self): self.sweep(self.make_user("buyer-sweep@ixg.local", "buyer"), PORTAL_ROUTES, "portal.ixgltd.com") @@ -236,6 +272,92 @@ class SeedWalkthroughTests(TestCase): self.assertEqual(CAPARecord.objects.count(), 1) self.assertEqual(ProcessingBatch.objects.filter(batch_reference="IXG-TEST-001").count(), 1) + def test_every_intake_keeps_within_what_it_received(self): + """The rule the ops lineage screen enforces, applied to the training data. + + It was not applied here, and nobody noticed: both demo batches were traced + back to the same three intakes, so the 820 kg intake 0141 was allocated + 800 kg twice over. The screen now refuses that, so the data must not need it. + """ + for intake in HarvestIntake.objects.all(): + allocated = sum(row.weight_allocated_kg or 0 for row in intake.batch_lineage.all()) + with self.subTest(intake=intake.intake_reference): + self.assertLessEqual( + allocated, intake.weight_kg, + f"{intake.intake_reference} received {intake.weight_kg} kg " + f"and is allocated {allocated} kg", + ) + + def test_the_two_passing_batches_have_their_own_intakes(self): + completed = set( + ProcessingBatch.objects.get(batch_reference="IXG-BATCH-2026-014") + .intake_lineage.values_list("harvest_intake__intake_reference", flat=True) + ) + inflight = set( + ProcessingBatch.objects.get(batch_reference="IXG-BATCH-2026-005") + .intake_lineage.values_list("harvest_intake__intake_reference", flat=True) + ) + self.assertTrue(completed, "the completed batch needs a lineage to show") + self.assertTrue(inflight, "the in-flight batch needs a lineage to show") + self.assertEqual( + completed & inflight, set(), + "an intake cannot supply two batches' worth of shea", + ) + + def test_re_running_the_seed_removes_records_it_no_longer_makes(self): + batch = ProcessingBatch.objects.get(batch_reference="IXG-BATCH-2026-005") + stray = HarvestIntake.objects.get(intake_reference="IXG-INTAKE-2026-0141") + IntakeBatchLineage.objects.create( + batch=batch, harvest_intake=stray, weight_allocated_kg=Decimal("10.00") + ) + # Evidence is keyed on the filename, so reassigning the in-flight batch's + # intakes used to leave the old harvest_document row in place. + case = DueDiligenceCase.objects.get(batch=batch) + stale = EUDREvidence.objects.create( + case=case, + evidence_type=EUDREvidence.EvidenceType.HARVEST_DOCUMENT, + file="IXG-INTAKE-2026-0141-weighbridge-ticket.pdf", + ) + call_command("seed_walkthrough", verbosity=0) + self.assertFalse( + batch.intake_lineage.filter(harvest_intake=stray).exists(), + "a link the seed no longer makes must not survive a re-run", + ) + self.assertEqual(batch.intake_lineage.count(), 3) + self.assertFalse( + EUDREvidence.objects.filter(pk=stale.pk).exists(), + "stale evidence the seed no longer references must not survive a re-run", + ) + self.assertEqual(case.evidence.count(), 4) + + +class RawTagScanner(HTMLParser): + """Collect each start tag's raw source text, quote-aware. + + A regex such as ``<[^>]*>`` ends the tag at the first ``>``, which a quoted + attribute value may contain: ```` + then arrives truncated, and a real leak after the quote is never seen. + ``html.parser`` knows where the tag actually ends. + """ + + def __init__(self): + super().__init__(convert_charrefs=False) + self.tags = [] + + def handle_starttag(self, tag, attrs): + self.tags.append((self.getpos()[0], self.get_starttag_text())) + + def handle_startendtag(self, tag, attrs): + self.tags.append((self.getpos()[0], self.get_starttag_text())) + + +def scan_tags(html): + """Return ``(line, raw_tag)`` for every start tag in ``html``.""" + scanner = RawTagScanner() + scanner.feed(html) + scanner.close() + return scanner.tags + class TemplateAttributeHygieneTests(TestCase): """No ops or buyer page may render an unquoted attribute that holds more than @@ -251,43 +373,178 @@ class TemplateAttributeHygieneTests(TestCase): """ DETECTOR = re.compile(r'\s([\w-]+)=([^\s"\'=<>`]+(?:\s+[^\s"\'=<>`]+)+)(?=[\s>])') - TAG = re.compile(r"<[a-zA-Z/][^>]*>") @classmethod def offenders(cls, html): found = [] - for tag in cls.TAG.findall(html): + for _, tag in scan_tags(html): for match in cls.DETECTOR.finditer(tag): found.append(match.group(0).strip()) return found def test_the_detector_catches_the_real_defect(self): bad = 'Fail' - self.assertTrue(self.DETECTOR.search(self.TAG.findall(bad)[0])) + self.assertTrue(self.DETECTOR.search(scan_tags(bad)[0][1])) good = 'Fail' - self.assertFalse(self.DETECTOR.search(self.TAG.findall(good)[0])) + self.assertFalse(self.DETECTOR.search(scan_tags(good)[0][1])) + + def test_a_quoted_angle_bracket_does_not_hide_a_defect(self): + # The old ``<[^>]*>`` tag regex stopped at the ``>`` inside the title, so + # the unquoted attribute after it was never examined. + unquoted = 'Fail' + self.assertTrue(self.offenders(unquoted)) + leaked = 'Approved' + self.assertEqual(self.leaked_classes(leaked), ["bg-success"]) + + # Two more defects from the same family, both of which shipped and neither of + # which any status code, test or screenshot review caught. + # + # 1. The *inverse* of the mistake above, made when the unquoted attributes were + # fixed in bulk: the quotes went around two attributes instead of around one + # value, so `method=post novalidate` became `method="post novalidate"`. A + # form's method is then an unrecognised keyword, the browser falls back to + # GET, and the form silently saves nothing. + # 2. A conditional class list written *outside* the quotes — + # `` + # — leaves the colour as a bare attribute. Bootstrap's `.badge` on its own + # is white text on a transparent background, so the status is invisible. + FORM_METHOD = re.compile(r']*\bmethod="([^"]*)"') + + ATTRIBUTE_NAMES = { + "post", "get", "dialog", "novalidate", "async", "defer", "multiple", + "required", "checked", "selected", "disabled", "readonly", "autofocus", + "hidden", "target", "method", "action", "enctype", "accept", + } + + # Any bare hyphenated token is a class that has escaped its quotes unless it + # is an attribute that legitimately carries no value. An allowlist of known + # Bootstrap prefixes missed the rest of the library (``form-control``, + # ``alert-danger``, ``is-invalid``, ``rounded-*``, ``shadow-*``); the default + # has to be "leak". Class lists inside quotes may contain spaces + # legitimately, so only value-less tokens are considered. + VALUELESS_ATTRIBUTES = ATTRIBUTE_NAMES | { + "allowfullscreen", "autoplay", "controls", "default", "inert", "ismap", + "itemscope", "loop", "muted", "nomodule", "open", "playsinline", + "reversed", "truespeed", + # Hyphenated boolean attributes from common front-end libraries. + "x-cloak", "x-ignore", "x-transition", + } + + QUOTED_VALUE = re.compile(r'\s([\w-]+)="([^"]*)"') + ANY_QUOTED = re.compile(r'"[^"]*"|\'[^\']*\'') + + @classmethod + def is_leaked_class(cls, token): + return ( + "-" in token + and "=" not in token + and token.lower() not in cls.VALUELESS_ATTRIBUTES + ) + + @classmethod + def folded_attributes(cls, html): + """Quoted values that are really two attributes run together.""" + found = [] + for _, tag in scan_tags(html): + for name, value in cls.QUOTED_VALUE.findall(tag): + tokens = value.split() + if len(tokens) > 1 and all(t in cls.ATTRIBUTE_NAMES for t in tokens): + found.append(f"{name}={value!r}") + return found + + @classmethod + def leaked_classes(cls, html): + """Classes sitting outside their class attribute.""" + found = [] + for _, tag in scan_tags(html): + # Drop the angle brackets first: with them left on, the last token in + # the tag arrives as "bg-success>" and matches nothing. + without_values = cls.ANY_QUOTED.sub(" ", tag[1:-1]) + for token in without_values.split(): + if cls.is_leaked_class(token): + found.append(token) + return found + + def test_the_folded_attribute_detector_catches_the_real_defect(self): + self.assertTrue(self.folded_attributes('
')) + for good in ( + '', + '
', + '', + '', + ): + with self.subTest(good=good): + self.assertEqual(self.folded_attributes(good), []) + + def test_the_leaked_class_detector_catches_the_real_defect(self): + # The first is the defect that shipped; the rest are from the same family + # and were missed by the old Bootstrap-prefix allowlist. + cases = { + 'Approved': "bg-success", + '
': "form-control", + '
': "alert-danger", + '': "is-invalid", + '
': "rounded-3", + '
': "shadow-sm", + } + for bad, expected in cases.items(): + with self.subTest(bad=bad): + self.assertIn(expected, self.leaked_classes(bad)) + + def test_the_leaked_class_detector_leaves_legitimate_markup_alone(self): + for good in ( + 'Approved', + '
', + '', + '', + '', + '', + '
hidden until ready
', + ): + with self.subTest(good=good): + self.assertEqual(self.leaked_classes(good), []) def check_pages(self, user, routes, host): + """Every rendered page, held to all four markup rules at once.""" client = Client(SERVER_NAME=host) client.force_login(user) for route in routes: - with self.subTest(route=route): - html = client.get(route).content.decode() + html = client.get(route).content.decode() + with self.subTest(route=route, rule="unquoted multi-token attribute"): self.assertEqual( self.offenders(html), [], f"{route} renders unquoted multi-token attributes", ) + with self.subTest(route=route, rule="form method"): + for value in self.FORM_METHOD.findall(html): + self.assertIn( + value, ("post", "get"), + f"{route} renders a form whose method is {value!r}", + ) + with self.subTest(route=route, rule="folded attribute"): + self.assertEqual( + self.folded_attributes(html), [], + f"{route} folds two attributes into one quoted value", + ) + with self.subTest(route=route, rule="leaked class"): + self.assertEqual( + self.leaked_classes(html), [], + f"{route} renders a class outside its class attribute", + ) - def test_ops_pages_have_no_unquoted_multi_token_attributes(self): + def test_ops_pages_render_clean_markup(self): + batch = make_sweep_batch() self.check_pages( User.objects.create_user( email="attr-ops@ixg.local", password="pass1234", role="ops", full_name="Attr Ops", is_staff=True, ), - OPS_ROUTES, "ops.ixgltd.com", + OPS_ROUTES + batch_screens(batch), "ops.ixgltd.com", ) - def test_portal_pages_have_no_unquoted_multi_token_attributes(self): + def test_portal_pages_render_clean_markup(self): self.check_pages( User.objects.create_user( email="attr-buyer@ixg.local", password="pass1234", role="buyer", @@ -295,3 +552,35 @@ class TemplateAttributeHygieneTests(TestCase): ), PORTAL_ROUTES, "portal.ixgltd.com", ) + + +class TemplateSourceHygieneTests(SimpleTestCase): + """The same class-leak check, on the template source instead of a response. + + The route sweep above does not render every *detail* page — and the detail + pages are exactly where the status badges are. Seven templates kept their + badge colour outside the class attribute, which showed up only as a blank + Status column in a screenshot. + + Scanning the source is sound for this particular question, and only this one: + once `{% %}` tags are stripped, a class list split across `{% if %}` branches + is unambiguous. The *unquoted attribute* question stays on rendered pages, + because there a template can be ambiguous and a response cannot. + """ + + QUOTED = re.compile(r'"[^"]*"|\'[^\']*\'') + TEMPLATE_TAG = re.compile(r"\{%[^%]*%\}", re.S) + + def test_no_template_puts_a_class_outside_its_class_attribute(self): + root = Path(settings.BASE_DIR) / "templates" + offenders = [] + for path in sorted(root.rglob("*.html")): + text = path.read_text() + for line, tag in scan_tags(text): + stripped = self.QUOTED.sub(" ", self.TEMPLATE_TAG.sub(" ", tag[1:-1])) + for token in stripped.split(): + if TemplateAttributeHygieneTests.is_leaked_class(token): + offenders.append(f"{path.relative_to(root)}:{line} {token}") + self.assertEqual( + offenders, [], "a class list is sitting outside its class attribute" + )
total