From ff368beaa4c73ad2f1ce3ab3307bd355a86a0179 Mon Sep 17 00:00:00 2001
From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com>
Date: Mon, 20 Jul 2026 16:19:15 -0400
Subject: [PATCH] =?UTF-8?q?test:=20consolidate=20test=20roots=20=E2=80=94?=
=?UTF-8?q?=20one=20loader,=20shared=20support,=20enforced=20CI=20floor=20?=
=?UTF-8?q?(phase=208)=20(#118)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* test: consolidate test roots — one repo-root loader, shared support package, missing-scenario suites, enforced ruff/coverage floor (refactor phase 8)
tests/conftest.py only loads for the tests/ root, not a standalone
`pytest lambdas/po/email_processor/tests` run, so it could never carry
session invariants like the dummy AWS env or the moto stubber
registration. Add a single repo-root conftest.py (pytest.ini pins
rootdir there, so it loads for every invocation) that sets the dummy
AWS credentials/region, imports moto BEFORE any handler module so
boto3 sessions pick up its stubber hook (carrying the explanatory
comment verbatim from the old _po_parser_support.py), and exposes one
load_lambda_module(pipeline, name) — the sys.modules save/restore
dance stays, since template_parser is still a duplicated bare name
across pipelines needing per-exec sibling binding.
Add tests/support/ as the shared package both pipelines' local
_*_parser_support.py modules delegate to: a superset FakeTable (PO's
update_item recording + WO's put_item and keyed single-row store),
FakeDynamoResource, load_email, and load_golden with parse_float=Decimal
kept (load-bearing for exact money comparison at PO magnitudes — WO's
prior load_golden had no parse_float and must not regress PO by losing
it). Rewrite _wo_parser_support.py off the bare `import handler` /
`from handler import parse_raw_email` strategy that was the source of
the bare-name sys.modules collision the other two loaders defend
against.
Move test_po_merge.py and test_pad_zip.py into
lambdas/po/email_processor/tests/ (PO-specific, belongs beside the
code) via git mv so history follows; test_parse_raw_email.py and
test_ses_auth.py stay at the repo root since they're genuinely
cross-pipeline, parameterized over both handlers. Delete
tests/test_local.py: it globs a nonexistent samples/ dir, is WO-only,
and imports a handler at collection time, bypassing the loader gate
entirely — the golden suites already cover its role. Its pytest.ini
exclusion comment goes with it.
New scenario coverage, all built on the single loader + support
package:
- PO+WO Bedrock transport errors (ThrottlingException, missing
'content' key, empty content list, non-JSON model text), asserting
PO's pre-call ai_fallback metric survives with no partial write and
the exception propagates; WO's no-datapoint-on-throttle behavior is
pinned with a documenting test rather than "fixed" by reordering.
- Handler-level SES-auth reject seam per pipeline: no auth
monkeypatch + empty ALLOWED_DKIM_DOMAINS asserts zero Bedrock calls,
zero writes, no raise — closing the hole where deleting the gate
line today still passes every test.
- web_ui coverage for both PO and WO (0% before this): fail-closed on
unset ARN and on a Secrets Manager exception, TTL cache refresh,
Bearer/X-Auth-Token/header-case-insensitivity, wrong-token 401 with
no table scan, non-ASCII token, and a hostile-field-escaping
regression lock. PO web_ui has no __init__.py, so these go through
the loader rather than package imports.
- A moto-backed mirror of test_po_merge for WO merge semantics
(table 'WorkOrders'): null-status never clobbers wo_status,
created_at immutable via if_not_exists, status->wo_status mapping,
None fields absent from SET, record_type only-when-present.
- Small pins: the PO-DC-02 64-char EMF clamp regression and
per-pipeline multi-record failure-isolation (all-or-retry contract).
The reprocess.py synthetic-event-shape contract test already landed
in Phase 7, so it isn't duplicated here.
Two WO product-code fixes ride along, since this is the phase that
exercises them: (a) the invalid_status reason-code fix in
template_parser.py's status check, which previously returned
malformed_site_code for the same failure validate_ai_fallback already
labels invalid_status, making one failure surface two codes depending
on path (grepped the dashboards/metric filters for
malformed_site_code first — no external references found, safe to
diverge the two codes); (b) wrapping the WO Bedrock call in
handler.py so a transport failure emits ai_fallback/bedrock_error in
an except-and-reraise. This is deliberately not a naive reorder: the
emit sits in the except block, not pre-call, so a gate-rejected email
still emits only ai_fallback_rejected and wo_stack's "a rejected
email emits nothing else" alarm contract doesn't double-count. A test
computes the emitted series by hand to pin the no-double-count
behavior. Neither change touches the handler event/return contract.
_validate_new_po_values in the PO template_parser.py is split into
per-rule helpers, and the V4 anchor-frame dataclass now carries
summary_matches/price so V13 can consume them; extract_new_po
(C901=35) is included in the split. Add ruff.toml enabling C901/PLR
so the mccabe/complexity suppressions scattered through the tree stop
being decorative; derived_fields.py is under the shadow-bake freeze
so its violations are silenced via a per-file ignore with a
justification comment instead of an in-file edit, and the handful of
other pre-existing violations surfaced by turning the config on get
the same per-file-ignore treatment with a reason, or a fix where the
file isn't frozen. scripts/ is added to the CI lint scope.
CI gains an explicit --cov module list (lambdas/po and wo
email_processor + web_ui, po/site_extractor, lambdas/shared) plus
--cov-fail-under=80, since web_ui and site_extractor lack __init__.py
markers and a bare --cov=lambdas silently skips them for the missing
package marker; .coveragerc omits the test dirs themselves from the
count. The Phase 0 AST bundle-consistency test stays in the standard
pytest run. .gitignore picks up the resulting .coverage data file.
docs/po-template-parser.md gets a small correction: the EXTRACTION_PROMPT
declares quantity/price as "number or null", not JSON strings, so
parse_float=Decimal already handles a conforming Bedrock response —
the doc previously implied the coercion path was the primary
mechanism rather than a defensive net for non-conforming responses.
* test: lock attribute-context quote escaping in web_ui hostile-field test
The escaping regression lock asserted only the element-context vector
(raw &\"'"
+
+
+class _WebFakeTable:
+ def __init__(self, items):
+ self._items = items
+
+ def scan(self, **kwargs):
+ return {"Items": list(self._items)}
+
+ def get_item(self, Key): # noqa: N803 (boto3 kwarg name)
+ pk, val = next(iter(Key.items()))
+ for item in self._items:
+ if item.get(pk) == val:
+ return {"Item": item}
+ return {}
+
+ def query(self, **kwargs):
+ return {"Items": []}
+
+
+class _WebFakeDynamo:
+ def __init__(self, tables):
+ self._tables = tables
+
+ def Table(self, name): # noqa: N802 (boto3 method name)
+ return _WebFakeTable(self._tables.get(name, []))
+
+
+class _ExplodingDynamo:
+ """Any table access is a contract violation: auth must run first."""
+
+ def Table(self, name): # noqa: N802
+ raise AssertionError(f"auth must precede any table access ({name})")
+
+
+@pytest.fixture(params=["po", "wo"])
+def web_ui(request):
+ mod = load_lambda_module(request.param, "web_ui/handler")
+ if request.param == "po":
+ table = mod.PO_TABLE
+ hostile_item = {
+ "po_number": _HOSTILE,
+ "po_status": _HOSTILE,
+ "email_type": _HOSTILE,
+ "supplier": {"name": _HOSTILE},
+ "site_code": _HOSTILE,
+ "trade": _HOSTILE,
+ "total_amount": _HOSTILE,
+ "processed_at": _HOSTILE,
+ }
+ else:
+ table = mod.WORK_ORDERS_TABLE
+ hostile_item = {
+ "work_order_id": _HOSTILE,
+ "description": _HOSTILE,
+ "site_code": _HOSTILE,
+ "wo_status": _HOSTILE,
+ "record_type": _HOSTILE,
+ "due_date": _HOSTILE,
+ "updated_at": _HOSTILE,
+ }
+ return SimpleNamespace(mod=mod, table=table, hostile_item=hostile_item)
+
+
+def test_wrong_token_returns_401(web_ui, monkeypatch):
+ monkeypatch.setattr(web_ui.mod, "is_authenticated", lambda event: False)
+ result = web_ui.mod.handler({"headers": {"x-auth-token": "wrong"}}, None)
+ assert result["statusCode"] == 401
+
+
+def test_absent_token_returns_401(web_ui, monkeypatch):
+ monkeypatch.setattr(web_ui.mod, "is_authenticated", lambda event: False)
+ result = web_ui.mod.handler({}, None)
+ assert result["statusCode"] == 401
+
+
+def test_401_does_not_touch_the_table(web_ui, monkeypatch):
+ """Auth is the first line of handler(): a rejected request must return 401
+ WITHOUT scanning (or otherwise touching) the DynamoDB table."""
+ monkeypatch.setattr(web_ui.mod, "is_authenticated", lambda event: False)
+ monkeypatch.setattr(web_ui.mod, "dynamodb", _ExplodingDynamo())
+ result = web_ui.mod.handler({}, None) # _ExplodingDynamo raises if touched
+ assert result["statusCode"] == 401
+
+
+def test_authenticated_render_path(web_ui, monkeypatch):
+ monkeypatch.setattr(web_ui.mod, "is_authenticated", lambda event: True)
+ monkeypatch.setattr(
+ web_ui.mod, "dynamodb", _WebFakeDynamo({web_ui.table: [web_ui.hostile_item]})
+ )
+ result = web_ui.mod.handler({}, None)
+ assert result["statusCode"] == 200
+ assert result["headers"]["Content-Type"] == "text/html"
+ assert " in element context, and the payload's quotes
+ must be entity-escaped in attribute context (the onclick row-link sink), or
+ a " breaks out of the attribute value and injects an event handler."""
+ monkeypatch.setattr(web_ui.mod, "is_authenticated", lambda event: True)
+ monkeypatch.setattr(
+ web_ui.mod, "dynamodb", _WebFakeDynamo({web_ui.table: [web_ui.hostile_item]})
+ )
+ body = web_ui.mod.handler({}, None)["body"]
+
+ # element context: angle brackets escaped
+ assert "" not in body # never reflected raw
+ assert "<script>alert(1)</script>" in body # escaped form present
+
+ # attribute context: the id flows through json.dumps into
+ # onclick="window.location={esc(..., quote=True)}", so the JSON string's
+ # opening quote must render as " -- a raw " right after the = means
+ # quote-escaping regressed and the attribute is breakable
+ assert "window.location="" in body
+ assert 'window.location="' not in body
+
+ # the payload's own quote characters appear only entity-escaped
+ assert """ in body
+ assert "'" in body
+ assert _HOSTILE not in body