/sh-security-review of PR #105 (5 fresh-context detectors + proof-or-kill
verifier) confirmed two diff-introduced findings; both fixed here.
F3 (medium, real PII in new fixtures): the harvest scrub replaced header
tokens but left real third-party PII in message BODIES -- an Amazon
contact's name/phone/personal email in non-coupa-02.eml and an internal
t.corp.amazon.com ticket URL in comment-02.eml, plus real submitter/attn
names recurring across the new_po corpus. Replaced every personal name,
phone, personal email, and internal URL with synthetic placeholders
(QP-soft-wrap aware) across both .eml bodies and expected goldens.
Extended test_fixture_hygiene to scan BODIES (phone shapes, corp URLs,
the leaked tokens), closing the header-only gap that let this through.
F1 (medium, cancellation gate): _CANCELLATION_SUBJECT was unanchored and
matched with .search(), unlike the anchored new_po pattern -- a subject
merely ending with the cancellation phrase could be routed to the sticky-
Cancelled write. Fully anchored it and switched to .match, and added a
body-corroboration gate (the real Coupa body independently restates
'Purchase Order #<po> ... has been cancelled'); a near-miss/misrouted
subject whose body does not corroborate now fails closed to the LLM
(new reason code cancellation_body_unconfirmed).
Pre-existing (advisory, not this PR): the LLM-fallback else->save_new_po
dispatch and undelimited extraction prompt (issue #101 family) are
byte-identical to main and unchanged here.
401 tests pass; ruff/format clean; cdk synth po-ingest clean.
The first-pass harvest scrub sanitized only the primary SES/DKIM
header blocks, leaving the real SES Feedback-ID sender-identity hash
in 49 committed fixtures and, on the two non-Coupa fixtures, an
embedded second SES block's X-Ses-Receipt, the Exchange cross-tenant
UPN ciphertext, and Gmail ARC fh= / X-Gm-* tokens -- exactly the
token classes the PR #99 fixture lesson requires placeholdered.
Replace each with a same-shape ScrubbedFixture value (byte-safe,
CRLF and folding preserved) so header structure and ses_auth
behavior are unchanged.
The non_usd rule only checked the Total-block top-level currency, so a
new_po whose line item read 'for 55,206.00 CAD' under a USD Total block
still template-parsed as ok -- a fail-open hole in the fail-closed
gate. Every line item's captured currency and its re-derived body token
must now byte-equal the proven-USD top-level currency; covered by a
line-level CAD adversarial fixture (the existing adv-non-usd only
exercised the Total-block variant) and a candidate-mutation unit test.
132 tests: golden-file comparison for all 25 positive fixtures
(17 single-line new-PO + 8 cancellations, Decimal-exact via
parse_float=Decimal), every fail-closed gate reason code covered
(body-level triggers via 17 synthetic adversarial .eml mutations,
candidate-level via direct validate() unit tests), real multi-line
and comment/non-Coupa fallback fixtures, dual line-ending parse
identity, two-path enrich/save parity (site-extractor stream
guard), V10 URL-id corpus sweep, fixture hygiene (ses_auth pass +
scrub-marker leak sweep), and Bedrock dispatch/EMF assertions.
The suite loads handler/template_parser via importlib under
unique module names and binds the handler's bare sibling imports
around exec (tests/conftest.py load_handler gets the same
treatment) -- the WO suite caches bare 'handler'/'template_parser'
names in sys.modules, and bare imports here would silently bind
to the wrong pipeline. moto is imported before the handler so its
botocore stubber hook precedes boto3 session creation (the PO
conftest chain now loads at pytest session start).
Fixtures are scrubbed real S3 samples: transport/auth header
values replaced with same-shape placeholders (structure kept so
ses_auth still passes), per-file digit ciphers, amounts remapped
with sum==total re-established. The .gitignore exception is
scoped to the PO fixtures path only.
Refs: #99
