procurement-ingest/tests/test_reprocess_contract.py

127 lines
5.3 KiB
Python
Raw Normal View History

Ops/recovery tooling + dependency hygiene (refactor phase 7) (#110) * feat: ops/recovery tooling + dependency hygiene (refactor phase 7) Generalize scripts/reprocess.py from a PO-only full-sweep script into a pipeline-general recovery tool. Targeted replay (--key/--prefix/--since) is now the default, and the full inbound/ sweep is demoted behind an explicit --all that documents its five hazards (async concurrency does not serialize, use RequestResponse if order matters, metric double-count, Bedrock re-bill, out-of-order field regression). --pipeline po|wo resolves the correct function + bucket; dry-run-by-default / --execute is preserved. A new tests/test_reprocess_contract.py pins the synthetic S3 event shape and asserts the raw list_objects_v2 key is emitted untransformed (the handler is the single decode point; a pre-decoded key would corrupt keys containing spaces or '+'). Add docs/runbook-dlq-recovery.md: the async on-failure DLQ has no console redrive-to-source, so it documents the receive -> extract key -> targeted reprocess --key -> verify -> purge procedure, the real recovery windows (14-day DLQ breadcrumb, 90-day raw-email S3 that overrides the table RETAIN policy and is the true replay floor), and that sender-auth and ai_fallback_rejected drops are fail-closed skips that never reach the DLQ. Linked from the README alarms and scripts sections. Drop the vendored boto3 floor pin from both email-processor requirements (the Lambda runtime provides boto3; lambda-template.md empty-with-comment form). With nothing left to install, the email-processor bundling becomes cp-only -- the whole pip step is removed, which is the only acceptable way the manylinux2014_aarch64 pin disappears (removing the pin while keeping a pip install caused the PR #34 x86-wheel outage). Exact-pin moto==5.2.2 and add pinned po/web_ui + po/site_extractor manifests (excluded from their bundles, so hash-neutral) so their new Dependabot entries have something to act on; add Dependabot entries for /tests, /lambdas/po/web_ui, and /lambdas/po/site_extractor. cdk diff is confined to exactly the two email processors' asset hashes on both stacks. The wo/web_ui dead-manifest reduction was deliberately left out: that manifest already ships inside the plain (non-bundled) WebUI asset on main, so reducing or excluding it would redeploy workorder-web-ui for no functional change -- deferred to keep the blast radius to the two intended targets. The untracked 44 MB lambdas/po/email_processor/package/ dir was removed from the filesystem (asset-hash-neutral given Phase 2's package/ exclude); it is untracked, so there is nothing to commit for it. * Reject --all combined with --prefix/--since in reprocess.py --all is a distinct mode (the demoted full-prefix sweep), but the args.all branch unconditionally set prefix=inbound/ and since=None, so passing it alongside a narrower selector silently discarded that selector. `--all --since 2026-07-01` swept the entire corpus instead of the bounded window, triggering every documented --all hazard (Bedrock re-bill, metric double- count, merged-field regression) on objects the operator never targeted -- contradicting the tool's safety goal. Add the missing mutual-exclusion guard alongside the existing --key one, and pin --all+--prefix, --all+--since, and all three together as argparse rejections.
2026-07-20 12:53:34 -04:00
"""Synthetic-event-shape contract test for scripts/reprocess.py (refactor §4.7).
Pins the exact S3 event shape reprocess emits and asserts the object key is
the RAW list_objects_v2 key — reprocess applies NO URL-encoding or decoding.
Why this matters: a real S3 event notification URL-encodes the object key, and
the handler is where any decode would live. reprocess builds its synthetic
event from the RAW list_objects_v2 obj["Key"] (or the raw --key arg), so it
must emit that key byte-for-byte. Replaying a pre-decoded (or pre-encoded) key
would not match what s3.get_object(Bucket, Key) expects downstream.
Recon note (honored narrowly): neither the PO nor the WO handler imports
urllib.parse or calls unquote/unquote_plus today — the raw event key goes
straight into s3.get_object. So the "handler decodes -> replaying a decoded
key double-decodes" failure mode is NOT reproducible against today's handlers
(they never decode once). This test therefore guards ONLY reprocess's own
contract: emit the raw list_objects_v2 key with no transformation. That keeps
replay single-decode-correct if a future handler ever adds unquote_plus.
"""
import importlib.util
import os
import pathlib
import sys
import pytest
# reprocess.py builds its boto3 clients inside main()/functions, so importing
# it is side-effect-free (no AWS calls, no clients at module scope). Set dummy
# region/creds defensively anyway so import stays offline-safe even if that
# ever changes. build_s3_event itself makes no AWS calls.
os.environ.setdefault("AWS_DEFAULT_REGION", "us-east-1")
os.environ.setdefault("AWS_ACCESS_KEY_ID", "testing")
os.environ.setdefault("AWS_SECRET_ACCESS_KEY", "testing")
os.environ.setdefault("AWS_SESSION_TOKEN", "testing")
# scripts/ is not a package (no __init__.py) and is not on sys.path, and
# tests/conftest.py only wires up per-handler dirs — so load reprocess.py by
# file path with importlib rather than `import scripts.reprocess`.
_p = pathlib.Path(__file__).resolve().parents[1] / "scripts" / "reprocess.py"
_spec = importlib.util.spec_from_file_location("reprocess", _p)
reprocess = importlib.util.module_from_spec(_spec)
_spec.loader.exec_module(reprocess)
def test_build_s3_event_shape_and_raw_key():
Migrate to seahaven-prod: deploy role, backfill tooling, account-portability fixes (#125) * feat(migration): prepare stacks and tooling for the seahaven-prod account move Phase 1 of the mgmt (328440206208) -> seahaven-prod (011934824531) migration. No behavior change in-account; everything here is additive or account-portability hygiene: - infra/deploy-role/: reviewed OIDC deploy-role artifacts for prod (trust main-only, cdk-hnb659fds-* AssumeRole, smoke-invoke-lambda scoped to exactly the two email-processor fn ARNs). Codifies the previously out-of-band smoke-invoke grant. - Table resource policies: make_slack_bot_read_policy in cdk/common.py, applied to purchase-orders, verified-sites, WorkOrders, WorkOrderComments (NOT pending-site-review; no bot consumer). Grants the mgmt-resident seahaven-slack-bot roles read-only cross-account access post-move (bot-side identity grants land in the slack-bot repo). - scripts/migrate_tables.py: dry-run-default backfill tool implementing the plan's per-table semantics (superset overwrite, ingested_at cutoff for WorkOrderComments, backup-gated truncate-and-load for the two site tables) plus a verify subcommand (count parity, spot checks, sticky-Cancelled drift check). - tests/test_resource_policy_helper.py: statement-shape unit tests + static pins that exactly the four bot-read tables carry the policy. - Account-literal fixes: account-agnostic fixture bucket in test_reprocess_contract; runbook/README/po-template-parser account references updated to prod with historical mgmt notes; README gains the account-prerequisites list (imported-by-name dependencies). deploy.yaml is deliberately unchanged (push-to-main auto-deploy kept). Merge is held until migration Phase 0 completes; flipping the AWS_DEPLOY_ROLE_ARN repo secret and merging this PR IS the first prod deploy. * fix(migration): verify backup AVAILABLE pre-truncate; document wildcard risk acceptance (cross-review FIX/NIT) * refactor(migration): drop cross-account read grants (slack-bot decommissioned); harden backfill + deploy role seahaven-slack-bot was decommissioned 2026-07-23 (stack DELETE_IN_PROGRESS, consumer Lambdas gone); its successor sh-mcp is undeployed and uses same-account DynamoDB access. So no live consumer reads these tables cross-account. Per Adam's call, drop the cross-account grants entirely and re-add correctly-scoped ones if/when sh-mcp deploys to a different account. - Remove the four table resource policies + make_slack_bot_read_policy helper + its constants (cdk/common.py, po_stack.py, wo_stack.py) and the helper's unit test. Both stacks synth with zero table ResourcePolicy. - scripts/migrate_tables.py hardening (fixes from the sh-security-review fan-out on the destructive backfill tool): * validate --cutoff strictly (parse ISO-8601, require aware UTC, re-emit canonical second-precision form) so a malformed cutoff can't silently copy dual-window rows or drop history; * reject `copy --all` up front (must run tables individually, in order, with the stream-drain wait) instead of writing three tables then erroring; * truncate backup gate now also checks recency (<1h) and TableId, not just status+name; * verify requires --cutoff whenever a cutoff table is in scope (else it false-flags dual-window rows as MISSING); * sticky-cancel is now PREVENTED copy-side (a non-Cancelled source item never overwrites a dest-Cancelled PO), and the verify comment no longer overstates what its source-side scan covers; * spot-check all modes (truncate_load keys are verbatim, so key-existence is sound there too). - Deploy role: scope cloudformation:DescribeStacks to this repo's stacks + CDKToolkit (was Resource:*, disclosed all tenant stacks in the shared prod account); add a drift check warning on unexpected role policies and drop the dead SMOKE_POLICY_NAME var; document the shared-account bootstrap-role accepted risk in the deploy-role README. * docs(deploy-role): fold in cross-review NITs (DescribeStacks maintenance note, warn-only drift rationale) * ci: update workflow to use new workflow tag (ruff versioning fix) * fix(migration): address Open SWE review findings on migrate_tables.py - Validate the truncate backup on dry-run as well as --execute so a missing/stale/wrong-incarnation --backup-arn surfaces on the rehearsal run (finding f_24a48b8900). - Assert configured keys match the live key schema of both tables before any key projection, turning config/schema drift into a descriptive abort instead of a mid-backfill KeyError (finding f_cb6b5a6c59). - Clarify why key-existence spot-checks are sound for WorkOrderComments: the copy Puts source items verbatim and the sample uses the same cutoff filter, so per-account comment_id divergence never enters the check (finding f_390b7d6c3b is a false positive; comment hardened).
2026-07-23 17:08:47 -04:00
# Account-agnostic fixture (the suffix is never parsed); the real bucket
# name is account-derived at deploy time.
bucket = "po-ingest-emails-000000000000"
Ops/recovery tooling + dependency hygiene (refactor phase 7) (#110) * feat: ops/recovery tooling + dependency hygiene (refactor phase 7) Generalize scripts/reprocess.py from a PO-only full-sweep script into a pipeline-general recovery tool. Targeted replay (--key/--prefix/--since) is now the default, and the full inbound/ sweep is demoted behind an explicit --all that documents its five hazards (async concurrency does not serialize, use RequestResponse if order matters, metric double-count, Bedrock re-bill, out-of-order field regression). --pipeline po|wo resolves the correct function + bucket; dry-run-by-default / --execute is preserved. A new tests/test_reprocess_contract.py pins the synthetic S3 event shape and asserts the raw list_objects_v2 key is emitted untransformed (the handler is the single decode point; a pre-decoded key would corrupt keys containing spaces or '+'). Add docs/runbook-dlq-recovery.md: the async on-failure DLQ has no console redrive-to-source, so it documents the receive -> extract key -> targeted reprocess --key -> verify -> purge procedure, the real recovery windows (14-day DLQ breadcrumb, 90-day raw-email S3 that overrides the table RETAIN policy and is the true replay floor), and that sender-auth and ai_fallback_rejected drops are fail-closed skips that never reach the DLQ. Linked from the README alarms and scripts sections. Drop the vendored boto3 floor pin from both email-processor requirements (the Lambda runtime provides boto3; lambda-template.md empty-with-comment form). With nothing left to install, the email-processor bundling becomes cp-only -- the whole pip step is removed, which is the only acceptable way the manylinux2014_aarch64 pin disappears (removing the pin while keeping a pip install caused the PR #34 x86-wheel outage). Exact-pin moto==5.2.2 and add pinned po/web_ui + po/site_extractor manifests (excluded from their bundles, so hash-neutral) so their new Dependabot entries have something to act on; add Dependabot entries for /tests, /lambdas/po/web_ui, and /lambdas/po/site_extractor. cdk diff is confined to exactly the two email processors' asset hashes on both stacks. The wo/web_ui dead-manifest reduction was deliberately left out: that manifest already ships inside the plain (non-bundled) WebUI asset on main, so reducing or excluding it would redeploy workorder-web-ui for no functional change -- deferred to keep the blast radius to the two intended targets. The untracked 44 MB lambdas/po/email_processor/package/ dir was removed from the filesystem (asset-hash-neutral given Phase 2's package/ exclude); it is untracked, so there is nothing to commit for it. * Reject --all combined with --prefix/--since in reprocess.py --all is a distinct mode (the demoted full-prefix sweep), but the args.all branch unconditionally set prefix=inbound/ and since=None, so passing it alongside a narrower selector silently discarded that selector. `--all --since 2026-07-01` swept the entire corpus instead of the bounded window, triggering every documented --all hazard (Bedrock re-bill, metric double- count, merged-field regression) on objects the operator never targeted -- contradicting the tool's safety goal. Add the missing mutual-exclusion guard alongside the existing --key one, and pin --all+--prefix, --all+--since, and all three together as argparse rejections.
2026-07-20 12:53:34 -04:00
# Deliberately contains a SPACE, a literal '+', and a literal '%41'.
# A real S3 event notification would deliver this key encoded as
# "inbound/2026/AB+12%2B34+%2541.eml"
# (space->'+', '+'->'%2B', '%'->'%25'); the handler is where any decode
# would live. reprocess builds from the RAW list_objects_v2 key, so it
# must emit "inbound/2026/AB 12+34 %41.eml" unchanged.
raw_key = "inbound/2026/AB 12+34 %41.eml"
event = reprocess.build_s3_event(bucket, raw_key)
# 1. Exact top-level shape: a single Records entry.
assert list(event.keys()) == ["Records"]
assert len(event["Records"]) == 1
# 2. Bucket name is nested at Records[0].s3.bucket.name.
assert event["Records"][0]["s3"]["bucket"]["name"] == bucket
# 3. Object key is nested at Records[0].s3.object.key, byte-for-byte.
assert event["Records"][0]["s3"]["object"]["key"] == raw_key
# 4. RAW / no-URL-encoding, no-URL-decoding on the emitted key.
emitted = event["Records"][0]["s3"]["object"]["key"]
assert emitted == raw_key # reprocess applies NO transformation
# space NOT percent-encoded (a real S3 notification would send %20):
assert " " in emitted and "%20" not in emitted
# literal '+' preserved, not turned into a space:
assert "+" in emitted
# '%41' NOT decoded to 'A':
assert "%41" in emitted and "A .eml" not in emitted
# --- Selector mutual-exclusion: --all is a MODE, not a modifier ---------------
# reprocess.main() reaches its first AWS call (sts get_caller_identity) only
# AFTER the argparse mutual-exclusion gate, so an invalid selector combination
# raises SystemExit(2) with no AWS access needed. This pins that `--all` cannot
# be silently combined with a narrower --prefix/--since (which the args.all
# clobber branch would otherwise discard -> unintended whole-corpus sweep).
def _expect_argparse_reject(monkeypatch, capsys, argv):
monkeypatch.setattr(sys, "argv", ["reprocess.py", *argv])
with pytest.raises(SystemExit) as exc:
reprocess.main()
assert exc.value.code == 2 # argparse error exit code
return capsys.readouterr().err
def test_all_rejects_prefix(monkeypatch, capsys):
err = _expect_argparse_reject(
monkeypatch,
capsys,
["--pipeline", "po", "--all", "--prefix", "inbound/2026/07/"],
)
assert "--all cannot be combined with --prefix or --since" in err
def test_all_rejects_since(monkeypatch, capsys):
err = _expect_argparse_reject(
monkeypatch,
capsys,
["--pipeline", "po", "--all", "--since", "2026-07-01T00:00:00Z"],
)
assert "--all cannot be combined with --prefix or --since" in err
def test_all_rejects_prefix_and_since_together(monkeypatch, capsys):
_expect_argparse_reject(
monkeypatch,
capsys,
[
"--pipeline",
"po",
"--all",
"--prefix",
"inbound/2026/",
"--since",
"2026-07-01T00:00:00Z",
],
)