diff --git a/.claude/workflows/phase-3-shared-extraction.js b/.claude/workflows/phase-3-shared-extraction.js new file mode 100644 index 0000000..a0a2a7c --- /dev/null +++ b/.claude/workflows/phase-3-shared-extraction.js @@ -0,0 +1,679 @@ +export const meta = { + name: 'phase-3-shared-extraction', + description: 'Phase 3 of the procurement-ingest refactor (docs/refactor-evaluation.md): extract lambdas/shared/ — ses_auth.py (byte-identical move, zero handler diff), web_ui_auth.py (byte-identical auth block from both web_ui handlers), email_parsing.py (parse_raw_email superset, cc unconditional), emf.py (parameterized emitter preserving every envelope byte-for-byte). Bundling gains cp shared/*.py in both email-processor commands + the same staging mechanism for both web_ui functions. Requires Phases 0-2 on the base branch. Auth code moves, so push is gated on /sh-security-review in the main loop. Committed locally, never pushed.', + phases: [ + { title: 'Setup', detail: 'verify Phases 0+1+2 on base, branch feature/phase-3-shared-extraction', model: 'haiku' }, + { title: 'Recon', detail: '4 mappers: the four duplicated modules, post-Phase-2 bundling sites, test-loader plumbing, deployed-zip baseline (read-only AWS)' }, + { title: 'Spec', detail: 'serial fable spec: pin shared-module contents, handler edit lists, web_ui staging, bundling strings, EMF design, test plumbing' }, + { title: 'Implement', detail: 'opus: shared/ + four handlers; sonnet: both CDK stacks; opus: all test plumbing + README — disjoint files', model: 'opus' }, + { title: 'Verify', detail: 'mechanical gates + bundle-parity verifier + 3 fable lenses (auth integrity, EMF/telemetry, loader integrity)' }, + { title: 'Fix', detail: 'opus fixer, full re-verify, max 3 rounds', model: 'opus' }, + { title: 'Package', detail: 'single commit via -F (no push)', model: 'sonnet' }, + ], +} + +// ---------------------------------------------------------------- constants + +const REPO = '/Users/adammoussa/Documents/repositories/seahaven/procurement-ingest' +const BRANCH = 'feature/phase-3-shared-extraction' +let _args = args +if (typeof _args === 'string') { + try { _args = JSON.parse(_args) } catch (e) { _args = null } +} +const BASE = (_args && _args.base) || 'main' + +const CONSTRAINTS = ` +PINNED BEHAVIORAL CONSTRAINTS (docs/refactor-evaluation.md Phase 3 — violating any is a build failure): +1. THE MOVE SET IS EXACTLY FOUR MODULES, in this order of dependency risk: + lambdas/shared/ses_auth.py, lambdas/shared/web_ui_auth.py, + lambdas/shared/email_parsing.py, lambdas/shared/emf.py. Nothing else + moves into shared/. lambdas/shared/ is the handbook location + (cdk-project-layout.md); modules land FLAT in every bundle so bare-name + imports keep working. +2. ses_auth: FIRST verify the two current copies are still byte-identical + (sha256 both against the ${BASE} versions — any drift since the audit is + a blocker, not something to silently reconcile). shared/ses_auth.py is + the EXACT bytes of that single copy; both originals are git rm'd. The + email-processor handlers keep 'from ses_auth import + authenticate_inbound_email' UNCHANGED — flat landing in /asset-output + means ZERO handler diff for this move, which is what keeps fail-closed + auth byte-identical through the change. +3. web_ui_auth: extract exactly the byte-identical block — _get_auth_token / + _header / is_authenticated + the four cache globals — from BOTH web_ui + handlers. The per-stack INFRA-74 comments STAY in each handler (their + wording has drifted deliberately; they are stack-specific — do NOT unify + or move them into the shared module). Fail-closed semantics (unset ARN, + Secrets Manager exception -> deny) must be unchanged; the token-cache + globals move with the functions that read them. +4. email_parsing.py: parse_raw_email as the SUPERSET version returning cc + unconditionally. WO's output is bit-identical to today; PO simply + ignores cc — do NOT "clean up" PO to consume it, and do NOT preserve two + variants. +5. emf.py: a generic emitter parameterized by namespace / dimension-sets / + properties. Every call site's emitted EMF envelope must be EXACTLY what + it emits today — the dimension-set list + [["ParseMethod"],["ParseMethod","TemplateId"]] is load-bearing for the + alarms and metric filters; making one-sided dimension fixes impossible + is the point of this move. Emission ORDERING is untouchable: PO emits + ai_fallback BEFORE the Bedrock call, WO after its gate with mutually- + exclusive ai_fallback/ai_fallback_rejected — these two deliberate + per-pipeline differences are pinned by tests; converting a call site + must not move it. The deliberate-double-count comments survive. +6. DERIVED-FIELDS EXCEPTION: if recon finds _emit_derived_agreement_metric + lives INSIDE derived_fields.py, do NOT convert it — derived_fields.py + and the shadow DerivedFieldAgreement telemetry are UNTOUCHABLE while the + bake runs (this outranks the emf consolidation). Leave it as a third + copy with a code comment pointing at shared/emf.py and report it in + notes/blockers. Only convert it if it lives outside derived_fields.py. +7. UNTOUCHABLE FILES (git diff ${BASE}...HEAD must be empty for each): + both template_parser.py (990 vs 508 lines, genuinely divergent — stays + per-pipeline), derived_fields.py, both validate_ai_fallback gate + modules, extract_with_claude's Bedrock invocation/prompt logic. Handler + diffs are LIMITED to: deleting moved code, import changes, and + emitter-call swaps per the binding spec. No opportunistic refactors. +8. Bundling (cdk): append 'cp shared/*.py /asset-output/' to BOTH + email-processor bundling commands (Phase 2 made the bundling cwd the + ../lambdas asset root, so the path resolves as written). + BASE-AWARENESS — READ THE ACTUAL cdk FILES ON THE BASE, DO NOT ASSUME: + this phase is stacked on the Phase 7 branch, which ALREADY removed the + pip install step from both email-processor commands (they are now + CP-ONLY: no pip line, no manylinux pin, because nothing third-party is + installed). Phase 3 moves only pure first-party modules (no new deps), + so cp-only STAYS cp-only — you simply add another 'cp shared/*.py + /asset-output/' line. DO NOT re-introduce a pip install step and DO NOT + re-add the manylinux pin: there is nothing to pin when nothing installs, + and adding a pip step back would be the regression here. (The PR #34 + lesson — never drop the manylinux pin while a pip install runs — still + holds ONLY if recon finds a pip step actually present on the base; on + the cp-only Phase 7 base there is none.) PRESERVE the Phase 2 exclude + lists exactly. Both web_ui functions gain the SAME staging mechanism + (widened-root bundled from_asset) so web_ui_auth.py ships beside their + handler — the spec agent pins the exact form. site_extractor's + from_asset is UNTOUCHED (Phase 6 territory). +9. tests/test_bundle_consistency.py is updated IN THE SAME CHANGE without + losing teeth: the PO_EXPECTED_TOP_LEVEL_MODULES exact-set pin gains the + shared modules that now ship; the AST sibling-import check must resolve + imports whose source file now lives under shared/; the new shared cp + line gets its own revert/mutation detection (a commented-out + 'cp shared/*.py' must fail the test). +10. Test plumbing in the same PR: update _SIBLING_MODULES resolution and + _po_parser_support.py (~line 71 — verify the current line) so tests + load ses_auth/email_parsing/emf from shared/; retire or repoint the + fixture-hygiene test that polices the two ses_auth copies for + byte-identity (obsolete once there is one copy — do not leave it + failing); drop the ses_auth fixture params from test_ses_auth + (parameterizing over two identical copies is dead weight — halves the + run). The sys.modules save/restore dance SURVIVES for template_parser + (still a duplicated bare name) — do not delete it. Preserve the + load-bearing moto-before-handler import ordering. +11. STALE-SHADOW HAZARD: after the move, no stale ses_auth.py / .pyc / + __pycache__ copy may remain anywhere it could shadow the shared copy — + in the repo (git rm, don't empty), in staged assets, or on any test + sys.path. Verify explicitly. +12. cdk diff on BOTH stacks: the only resource deltas allowed are Code/ + S3Key (asset) changes on the two email processors and the two web_ui + functions (+ CDK metadata). No logical-ID changes, no alarm, IAM, + table, env, runtime, or handler-property deltas of any kind. +13. AWS access is READ-ONLY (get-function, downloading deployed zips via + presigned URLs). NEVER cdk deploy, never invoke, never mutate. +` + +const PREAMBLE = ` +You are one of several agents building refactor Phase 3 in the git repo at +${REPO} on branch ${BRANCH} (already checked out — do NOT switch branches, +do NOT create branches, do NOT commit, NEVER push, do NOT run cdk deploy or +touch AWS resources beyond read-only calls). +Authoritative spec: docs/refactor-evaluation.md, section "Phase 3". +Work ONLY in the files you are told you own; other agents are concurrently +editing other files in this same working tree. +${CONSTRAINTS} +Your final message is consumed by an orchestrator script, not a human — +return only the structured data requested. +` + +// ------------------------------------------------------------------ schemas + +const RECON = { + type: 'object', + required: ['summary', 'facts'], + properties: { + summary: { type: 'string' }, + facts: { type: 'array', items: { type: 'string' } }, + blockers: { type: 'array', items: { type: 'string' } }, + }, +} + +const SPEC = { + type: 'object', + required: ['sharedModules', 'handlerEdits', 'webUiStaging', 'bundlingEdits', 'emfDesign', 'testPlumbing', 'parityRules', 'notes'], + properties: { + sharedModules: { type: 'string', description: 'per shared module: exact provenance (which copy is the source, sha256), full contents decision (verbatim move vs superset vs parameterized), and the public surface each importer uses' }, + handlerEdits: { type: 'string', description: 'per handler (po/wo email_processor, po/wo web_ui): the exact deletions, import lines, and emitter-call swaps — file:line, nothing else may change' }, + webUiStaging: { type: 'string', description: 'the complete new from_asset blocks for both web_ui functions: asset path, command or cp form, exclude list — exact code' }, + bundlingEdits: { type: 'string', description: 'the two email-processor bundling command strings with cp shared/*.py appended, verbatim, plus the expected top-level module set of each resulting bundle' }, + emfDesign: { type: 'string', description: 'shared/emf.py signature + per-call-site mapping proving each emitted envelope (namespace, dimension-set list, properties) is byte-equivalent to today; the derived-agreement emitter decision per constraint 6' }, + testPlumbing: { type: 'string', description: 'every test/support file change: loader resolution, _SIBLING_MODULES, fixture-hygiene retirement, test_ses_auth de-parameterization, test_bundle_consistency edits with their mutation-detection shapes' }, + parityRules: { type: 'string', description: 'how Verify judges staged bundles vs the deployed-zip baseline: expected first-party set per function (= deployed set + the new shared modules), ses_auth byte-identity requirement, expected removals (none beyond moved-file provenance), web_ui bundle expectations' }, + notes: { type: 'string' }, + }, +} + +const IMPL = { + type: 'object', + required: ['filesChanged', 'summary', 'checksRun'], + properties: { + filesChanged: { type: 'array', items: { type: 'string' } }, + summary: { type: 'string' }, + checksRun: { type: 'string' }, + blockers: { type: 'array', items: { type: 'string' } }, + }, +} + +const CHECKS = { + type: 'object', + required: ['passed', 'details'], + properties: { + passed: { type: 'boolean' }, + details: { type: 'string' }, + scopeViolations: { type: 'array', items: { type: 'string' } }, + }, +} + +const PARITY = { + type: 'object', + required: ['passed', 'poVerdict', 'woVerdict', 'webUiVerdict', 'details'], + properties: { + passed: { type: 'boolean' }, + poVerdict: { type: 'string', description: 'PO email-processor staged vs deployed: module-set delta exactly as spec, ses_auth bytes identical — full evidence' }, + woVerdict: { type: 'string', description: 'same for WO email-processor' }, + webUiVerdict: { type: 'string', description: 'both web_ui staged bundles: own *.py + web_ui_auth.py present, nothing stray, hashes deterministic' }, + details: { type: 'string' }, + }, +} + +const FINDINGS = { + type: 'object', + required: ['findings'], + properties: { + findings: { + type: 'array', + items: { + type: 'object', + required: ['title', 'severity', 'confirmed', 'evidence', 'fix'], + properties: { + title: { type: 'string' }, + severity: { enum: ['critical', 'high', 'medium', 'low'] }, + confirmed: { type: 'boolean' }, + evidence: { type: 'string' }, + fix: { type: 'string' }, + }, + }, + }, + }, +} + +// ------------------------------------------------------------------- setup + +phase('Setup') +const setup = await agent(` +In ${REPO}: +1. SEQUENCING GATE — Phases 0, 1 AND 2 must all be on ${BASE} (Phase 3 + edits the same stack files as Phase 2 and depends on its widened + ../lambdas asset roots to make shared/ reachable). git fetch origin, + then pick the base ref: origin/${BASE} if that remote ref exists, + otherwise the local branch ${BASE} (a stacked local-only base is + expected and fine). Verify on the base ref: + (a) Phase 0: tests/test_bundle_consistency.py exists + (git show :tests/test_bundle_consistency.py | head -3); + (b) Phase 1: validate_ai_fallback exists in the PO pipeline + (git grep validate_ai_fallback -- lambdas/po); + (c) Phase 2: BOTH cdk/po_stack.py and cdk/wo_stack.py on the base ref + contain Code.from_asset("../lambdas") for the email processors + (git show :cdk/po_stack.py | grep -n '\\.\\./lambdas', same + for wo_stack.py). + If any is missing, STOP with a blocker naming the unmet phase and do + nothing else. +2. Verify clean working tree (untracked .coverage / .claude/ / the local + 44 MB lambdas/po/email_processor/package/ dir are fine; any OTHER dirt = + blocker, never stash or discard). +3. git checkout ${BASE}; then git pull --ff-only ONLY if the branch has an + upstream (a local-only base skips the pull — not a blocker); then + git checkout -b ${BRANCH} +4. gh pr list --state open --json number,title,headRefName (overlap check). +Return facts: HEAD sha, per-phase gate evidence, open PRs, blockers. +`, { label: 'setup:branch', model: 'haiku', schema: RECON }) + +if (!setup || (setup.blockers && setup.blockers.length)) { + return { aborted: 'setup blockers', blockers: setup ? setup.blockers : ['setup agent died'], facts: setup ? setup.facts : [] } +} +log(`Branch ${BRANCH} ready off ${BASE}. ${setup.summary}`) + +// ------------------------------------------------------------------- recon + +phase('Recon') +const recon = await parallel([ + () => agent(`${PREAMBLE} +Read-only recon of the FOUR duplicated surfaces being extracted: +1. ses_auth.py both copies — sha256 of each (MUST match; drift = blocker), + line count, the exact import line each email-processor handler uses, + any other importer (git grep 'import ses_auth\\|from ses_auth'). +2. web_ui auth block — in BOTH web_ui handlers quote with file:line the + exact boundaries of the byte-identical block (_get_auth_token, _header, + is_authenticated, the four cache globals), diff the two blocks to prove + byte-identity, quote each INFRA-74 comment verbatim (they differ — + that is expected), and note everything else in each handler that CALLS + the block. +3. parse_raw_email both copies — where each lives (own sibling file vs + inside handler.py), the exact cc delta between them, every call site. +4. The THREE EMF emitters — quote each verbatim with file:line (PO + ParseMethod emitter, WO ParseMethod emitter, + _emit_derived_agreement_metric), namespace + dimension-set list + + properties of each, and CRITICALLY: which FILE _emit_derived_agreement_metric + lives in (constraint 6 hinges on whether it is inside derived_fields.py). + Also pin current line numbers of PO's pre-Bedrock ai_fallback emit and + WO's post-gate emit. +25-35 precise facts.`, + { label: 'recon:duplicated-modules', model: 'sonnet', phase: 'Recon', schema: RECON }), + + () => agent(`${PREAMBLE} +Read-only recon of the post-Phase-2 CDK bundling state: for ALL FIVE +Code.from_asset sites in cdk/po_stack.py and cdk/wo_stack.py quote verbatim +with current file:line — asset path, full bundling command (or plain form), +exclude list. For the two web_ui functions additionally record: runtime, +architecture, handler property, memory/timeout, whether any requirements.txt +exists for them, and every function property that must NOT change when +bundling is added. Confirm lambdas/shared/ does not exist yet and list +anything at lambdas/ top level. 15-25 facts.`, + { label: 'recon:cdk-bundling', model: 'haiku', phase: 'Recon', schema: RECON }), + + () => agent(`${PREAMBLE} +Read-only recon of the test plumbing this phase must rewire: +- tests/conftest.py importlib loader: how it resolves module paths, the + sys.modules save/restore, _SIBLING_MODULES (exact current contents). +- _po_parser_support.py: the independent importlib reimplementation, the + load-bearing moto-before-handler import order (~lines 26-33), and what + is at line ~71 (the doc cites it — quote the current code). +- _wo_parser_support.py bare sys.path 'import handler' strategy. +- test_ses_auth.py at root: the fixture parameterization over both copies + (quote it), total line count, the fixture-hygiene test that polices + byte-identity between the two copies (name + assertion). +- test_parse_raw_email.py: how it loads both handlers' parse_raw_email. +- tests/test_bundle_consistency.py: every assertion that will fail red + against the Phase 3 shapes (shared cp line, PO_EXPECTED_TOP_LEVEL_MODULES, + AST sibling-import resolution for moved modules). +20-30 facts.`, + { label: 'recon:test-plumbing', model: 'sonnet', phase: 'Recon', schema: RECON }), + + () => agent(`${PREAMBLE} +Read-only AWS recon (region us-east-1, READ-ONLY): for po-email-processor, +workorder-email-processor, AND both web_ui functions (find their exact +function names via the stacks/aws lambda list-functions) run aws lambda +get-function; for the two email processors download each Code.Location +presigned zip to a scratch dir and produce the COMPLETE file list +(unzip -l) separated into (a) first-party top-level .py, (b) dependency +dirs, (c) anything else; record every function's CodeSha256 and the +sha256 of ses_auth.py inside each deployed zip (the byte-identity baseline +the moved copy is judged against). For the web_ui functions the current +zip file list is the baseline their new bundled asset must cover. +Return the categorized lists as facts.`, + { label: 'recon:deployed-baseline', model: 'sonnet', phase: 'Recon', schema: RECON }), +]) + +const reconOk = recon.filter(Boolean) +const pack = reconOk.map(r => `## ${r.summary}\n${r.facts.join('\n')}`).join('\n\n') +const reconBlockers = reconOk.flatMap(r => r.blockers || []) + .filter(b => b && !/^\s*(none|n\/a)\b/i.test(b)) +log(`Recon complete: ${reconOk.length}/4 mappers, ${reconBlockers.length} blockers`) +if (reconBlockers.length) { + return { aborted: 'recon blockers (likely copy drift — resolve before extracting)', blockers: reconBlockers, reconPack: pack } +} + +// -------------------------------------------------------------------- spec + +phase('Spec') +const spec = await agent(`${PREAMBLE} +You are the SPEC agent — the single authority that pins every contested +decision BEFORE parallel implementation (parallel leaves cannot see each +other's choices). Using the recon pack below plus your own reads of the +actual files, produce the binding implementation spec: +- sharedModules: per constraint 1's four modules — provenance, contents + decision, public surface. ses_auth is a verbatim byte-move; web_ui_auth + is the exact block; email_parsing is the WO superset (cc unconditional); + emf is the parameterized emitter. +- handlerEdits: for each of the four handlers, the exact minimal edit list + (constraint 7 — deletions, imports, emitter-call swaps ONLY). State + explicitly that the two email-processor ses_auth import lines are + UNCHANGED. +- webUiStaging: complete replacement from_asset blocks for both web_ui + functions — widened root, cp command staging the function's own *.py + plus web_ui_auth.py (pin whether to cp shared/*.py or just + shared/web_ui_auth.py — pick ONE rule, state why), Phase-2-style + excludes. Mind bundling cwd semantics: the container mounts the asset + root (../lambdas) as the working dir. If a no-Docker staging form is + viable and simpler, you may pin that instead — but ONE mechanism for + both, exact code. +- bundlingEdits: both email-processor command strings verbatim with + 'cp shared/*.py /asset-output/' appended, plus the exact expected + top-level module set of each resulting bundle (this feeds + PO_EXPECTED_TOP_LEVEL_MODULES and the parity gate). +- emfDesign: the shared emitter signature and a per-call-site table + proving envelope byte-equivalence; resolve the derived-agreement emitter + per constraint 6 based on where recon found it. +- testPlumbing: every file, every edit — loader resolution for shared/, + _SIBLING_MODULES, _po_parser_support.py, _wo_parser_support.py if it + needs the shared path, fixture-hygiene retirement, test_ses_auth + de-parameterization, test_parse_raw_email (single implementation now — + what does it exercise?), test_bundle_consistency edits including the new + mutation shapes (commented-out shared cp must fail). +- parityRules: exactly how Verify judges staged bundles vs recon's + deployed baseline — per-function expected first-party set (deployed set + minus nothing, plus the shared modules per your cp rule), ses_auth + sha256 must equal the deployed zips' copy, web_ui bundles must cover + their deployed file list plus web_ui_auth.py, nothing else may appear + or vanish. +Recon pack:\n${pack}`, + { label: 'spec:pin-extraction', phase: 'Spec', schema: SPEC }) + +if (!spec) return { aborted: 'spec agent died — rerun workflow', reconBlockers } +const specBlock = `BINDING SPEC (from the spec agent — implement EXACTLY this):\n${JSON.stringify(spec, null, 2)}` +log('Spec pinned: shared modules, handler edits, web_ui staging, bundling strings, EMF design, test plumbing') + +// --------------------------------------------------------------- implement + +phase('Implement') +const impl = await parallel([ + () => agent(`${PREAMBLE} +YOU OWN: everything under lambdas/ EXCEPT the tests/ directories (another +agent owns all test and support files). Do not touch cdk/ or README. +Task: execute the four moves per spec.sharedModules + spec.handlerEdits + +spec.emfDesign, in the spec's order: +1. git mv (or create+git rm preserving exact bytes) ses_auth.py -> + lambdas/shared/ses_auth.py; delete both originals; prove sha256 + equality in your summary. Handler import lines untouched. +2. Create lambdas/shared/web_ui_auth.py from the byte-identical block; + replace the block in BOTH web_ui handlers with the import; keep each + INFRA-74 comment in place. +3. Create lambdas/shared/email_parsing.py (superset); rewire both + email-processor call sites. +4. Create lambdas/shared/emf.py; swap the emitter call sites per + spec.emfDesign — honoring constraint 6 (derived_fields.py stays + untouched if the third emitter lives there) and constraint 5 (emission + ordering does not move). +Delete every now-empty moved-out file with git rm (constraint 11). +Run before returning: ruff check lambdas && ruff format lambdas --check, +plus an import smoke: python3 -c with sys.path prepended for +lambdas/shared + each function dir, importing every touched module (tests +are NOT yours to run — the plumbing agent lands them in parallel). +${specBlock}`, + { label: 'impl:shared-lambdas', model: 'opus', phase: 'Implement', schema: IMPL }), + + () => agent(`${PREAMBLE} +YOU OWN: cdk/po_stack.py and cdk/wo_stack.py ONLY (and only the from_asset +regions — alarms, IAM, tables, env are all off-limits). +Task: apply spec.bundlingEdits (append the shared cp to both +email-processor commands — the base is the Phase 7 CP-ONLY bundling, so +just add another 'cp shared/*.py /asset-output/' line; DO NOT re-add a pip +install step or a manylinux pin, there is nothing third-party to install +(constraint 8); preserve the Phase 2 excludes verbatim) and +spec.webUiStaging (both web_ui functions). Touch nothing else; +site_extractor's from_asset stays byte-identical. +Run before returning: ruff check cdk && cd cdk && +npx cdk synth po-ingest -q -o /tmp/phase3-synth && +npx cdk synth workorder-ingest -q -o /tmp/phase3-synth (artifact-id +selectors, NOT stack_name). Docker bundling runs — confirm each staged +email-processor asset contains the shared modules and each staged web_ui +asset contains web_ui_auth.py; note asset hashes in your summary. If the +lambdas/ moves have not landed yet the synth will fail on missing files — +poll by re-running up to ~10 min before reporting a blocker. +${specBlock}`, + { label: 'impl:cdk-stacks', model: 'sonnet', phase: 'Implement', schema: IMPL }), + + () => agent(`${PREAMBLE} +YOU OWN: all test and support files (tests/ at repo root including +tests/test_bundle_consistency.py and tests/conftest.py, +lambdas/po/email_processor/tests/, lambdas/wo/email_processor/tests/) +and README.md ONLY. +Task A: apply spec.testPlumbing in full — loader/_SIBLING_MODULES +resolution for shared/, _po_parser_support.py edit (preserving the +moto-before-handler ordering comment), fixture-hygiene retirement, +test_ses_auth de-parameterization, test_parse_raw_email update, +test_bundle_consistency updates WITHOUT losing teeth (constraint 9 — the +new shared cp pin must reject a commented-out or narrowed variant; keep +the existing mutation tests green). +Task B: README — document lambdas/shared/ in the repo-layout section +(which modules live there, the flat-landing import rule, the bundling cp +that ships them), update the bundling/deploy-guards paragraphs for the +shared cp + web_ui staging, and note that ses_auth is now single-sourced +(one hardening fix lands once). Match existing README style. +Run before returning: pytest -q --no-cov at repo root (must be green +against the OTHER agents' edits — they land in parallel; poll by +re-running up to ~10 min before reporting a blocker) and ruff check on +every file you touched. +${specBlock}`, + { label: 'impl:test-plumbing-readme', model: 'opus', phase: 'Implement', schema: IMPL }), +]) + +const implOk = impl.filter(Boolean) +const implBlockers = implOk.flatMap(r => r.blockers || []) +log(`Implement complete: ${implOk.length}/3 agents, blockers: ${implBlockers.length}`) + +// ---------------------------------------------------- verify + fix loop + +const EXPECTED_SCOPE = [ + 'lambdas/shared/', + 'lambdas/po/email_processor/', + 'lambdas/wo/email_processor/', + 'lambdas/po/web_ui/', + 'lambdas/wo/web_ui/', + 'cdk/po_stack.py', + 'cdk/wo_stack.py', + 'tests/', + 'README.md', +] + +const mechanicalPrompt = `${PREAMBLE} +Independent re-verification — trust nothing self-reported. Run ALL gates, +quoting failures verbatim: +1. pytest -q --no-cov (repo root, all three roots green) +2. ruff check . && ruff format --check . +3. cd cdk && npx cdk synth po-ingest -q && npx cdk synth workorder-ingest -q +4. cd cdk && npx cdk diff po-ingest ; npx cdk diff workorder-ingest — + constraint 12: only the four functions' Code/S3Key (+ metadata) deltas. + Any alarm/IAM/env/runtime/handler-prop/logical-ID delta = FAIL. Paste + the diff summaries. +5. UNTOUCHABLES (each must output NOTHING): + git diff ${BASE}...HEAD -- lambdas/po/email_processor/derived_fields.py + git diff ${BASE}...HEAD -- lambdas/po/email_processor/template_parser.py + git diff ${BASE}...HEAD -- lambdas/wo/email_processor/template_parser.py + plus both validate_ai_fallback gate modules (resolve their filenames + first) and lambdas/po/site_extractor/. +6. BYTE-IDENTITY: sha256 of lambdas/shared/ses_auth.py equals sha256 of + git show ${BASE}:lambdas/po/email_processor/ses_auth.py (and the wo + copy). Both originals gone from the tree (git ls-files check) and no + stray ses_auth.py/__pycache__ anywhere under lambdas/ outside shared/ + (constraint 11). +7. ORDERING PINS: grep line numbers proving PO's ai_fallback emit still + precedes the Bedrock invoke and WO's emits are untouched relative to + ${BASE} (the wo handler diff must contain ONLY the spec's edit classes). +8. INFRA-74: both web_ui handlers still contain their own INFRA-74 + comment verbatim per ${BASE}. +9. git status --porcelain scope check: every modified/added/deleted path + under ${EXPECTED_SCOPE.join(', ')} (untracked .coverage/.claude/ + package/ tolerated). +passed=true only if all green. YOU MAY NOT edit files.` + +const parityPrompt = `${PREAMBLE} +You are the BUNDLE-PARITY verifier — the load-bearing gate of this phase. +Everything is local synth + read-only AWS. +1. cd cdk && npx cdk synth po-ingest -q -o /tmp/phase3-parity && + npx cdk synth workorder-ingest -q -o /tmp/phase3-parity. Locate all + four staged assets (two email processors, two web_ui). +2. Re-download both email-processor deployed zips fresh (aws lambda + get-function Code.Location, us-east-1) — do not trust a recon cache — + and fetch both web_ui functions' deployed file lists. +3. Judge per spec.parityRules: per email processor, staged first-party + top-level .py set == deployed set PLUS exactly the shared modules the + spec's cp rule ships (list both sets; nothing else may appear or + vanish); sha256 of staged ses_auth.py == sha256 of the ses_auth.py + inside each deployed zip (fail-closed auth byte-identical through the + move); dependency packages compared by name, version drift noted not + failed. Per web_ui function: staged bundle covers the deployed file + list plus web_ui_auth.py, nothing stray (no tests/, no other + pipeline's sources, no .eml, no package/). +4. DETERMINISM: synth po-ingest twice into fresh -o dirs — identical + asset hashes, including the NEW web_ui assets. Then drop a throwaway + __pycache__/junk.pyc under lambdas/shared/ (delete it afterwards), + re-synth, and confirm the excludes keep every hash unchanged. +5. IMPORT-RESOLUTION sanity: inside each staged email-processor asset + dir run python3 -c "import handler" with that dir alone on sys.path + (env-var stubs as needed) — proves the flat landing satisfies every + import including 'from ses_auth import ...' with the moved copy. +passed=true only if every check holds.` + +const lenses = [ + { key: 'auth-integrity', prompt: `${PREAMBLE} +ADVERSARIAL REVIEW — auth-integrity lens. Auth code moved; try to prove +the move WEAKENED it. (1) ses_auth: byte-compare the shared copy against +${BASE}'s copies yourself; then attack import resolution — in the BUNDLE +and in TESTS, which ses_auth wins if anything shadows (stale .pyc, a +same-named module on sys.path, the tests loader resolving the old path +silently to a stub)? Could a test now pass against a MOCK of ses_auth +where it previously exercised the real module? (2) web_ui_auth: diff the +extracted block against both originals — any dropped line, changed +global, or reordered check? Is the fail-closed path (unset ARN, Secrets +exception, wrong token -> 401 before any table access) provably +unchanged? Do the four cache globals still behave per-function (module +now shared — could cross-importer state ever leak)? (3) The gate call +sites: could any handler path now reach S3-fetch/Bedrock/save before +authenticate_inbound_email or is_authenticated, where it could not +before? confirmed=true only with a concrete exploit sketch or +file:line proof.` }, + { key: 'telemetry-emf', prompt: `${PREAMBLE} +ADVERSARIAL REVIEW — EMF/telemetry lens. The alarms and metric filters +consume exact EMF shapes; a silent envelope change breaks paging without +failing any test. Read shared/emf.py + every converted call site +(git diff ${BASE}) and the synthesized templates' metric filters/alarms. +Verify per call site: namespace exact, dimension-set list EXACT +([["ParseMethod"],["ParseMethod","TemplateId"]] where applicable, order +included), property keys/types identical, timestamp/CloudWatchMetrics +envelope structure identical — construct a sample emission per call site +and diff it against ${BASE}'s hand-built _aws dict output. Then the +ordering pins: PO ai_fallback still pre-Bedrock, WO still post-gate +mutually-exclusive, the deliberate double-count comment intact. Finally +constraint 6: where does _emit_derived_agreement_metric live and was the +decision honored (derived_fields.py diff empty)? confirmed=true only +with evidence.` }, + { key: 'loader-integrity', prompt: `${PREAMBLE} +ADVERSARIAL REVIEW — test/loader-integrity lens. The three loading idioms +were rewired; attack them. (1) Does test_ses_auth now exercise the REAL +lambdas/shared/ses_auth.py (trace the loader path), and did +de-parameterization silently drop any assertion that only ran under one +param? (2) Fixture-hygiene: the byte-identity police is retired — does +anything still guard against a future stray ses_auth.py copy reappearing +in a pipeline dir (should the bundle-consistency or a hygiene test)? If +nothing does, that is a finding. (3) sys.modules save/restore: prove it +still isolates template_parser between pipelines (run two cross-pipeline +tests back to back); prove moto-before-handler ordering survived in +_po_parser_support.py. (4) test_bundle_consistency: hand-mutate command +strings on scratch copies — a commented-out 'cp shared/*.py', a shared +glob narrowed to one file, and a PO_EXPECTED_TOP_LEVEL_MODULES missing a +shared module must each FAIL. (5) Run pytest twice in one session and in +file-shuffled order (-p no:randomly not installed? then two explicit +orderings) to smoke out import-order coupling introduced by the shared +path. confirmed=true only with file:line or reproduced-failure +evidence.` }, +] + +let round = 0 +let checks = null +let parity = null +let confirmed = [] +while (round < 3) { + phase('Verify') + const results = await parallel([ + () => agent(mechanicalPrompt, { label: `verify:mechanical-r${round}`, model: 'sonnet', phase: 'Verify', schema: CHECKS }), + () => agent(parityPrompt, { label: `verify:parity-r${round}`, model: 'opus', phase: 'Verify', schema: PARITY }), + ...lenses.map(l => () => + agent(l.prompt, { label: `verify:${l.key}-r${round}`, phase: 'Verify', schema: FINDINGS })), + ]) + checks = results[0] + parity = results[1] + confirmed = results.slice(2).filter(Boolean) + .flatMap(r => r.findings || []) + .filter(f => f.confirmed && f.severity !== 'low') + const green = checks && checks.passed && parity && parity.passed + log(`Verify round ${round}: mechanical ${checks && checks.passed ? 'GREEN' : 'RED'}, parity ${parity && parity.passed ? 'GREEN' : 'RED'}, confirmed findings: ${confirmed.length}`) + if (green && confirmed.length === 0) break + + round += 1 + if (round >= 3) break + phase('Fix') + await agent(`${PREAMBLE} +You are the fix agent — you may edit files under: ${EXPECTED_SCOPE.join(', ')}. +Fix EVERY item below minimally; the binding spec and 13 pinned constraints +still hold (a finding that conflicts with a constraint is reported, not +"fixed" — the constraint wins, esp. constraint 6's derived_fields +untouchability and constraint 5's emission ordering). Re-run the specific +failing gate/test per fix. +MECHANICAL:\n${checks ? checks.details : '(agent died — rerun all gates)'} +PARITY:\n${parity ? parity.details : '(agent died — rerun all)'} +CONFIRMED FINDINGS:\n${JSON.stringify(confirmed, null, 2)} +${specBlock}`, + { label: `fix:round-${round}`, model: 'opus', phase: 'Fix', schema: IMPL }) +} + +const verifyClean = checks && checks.passed && parity && parity.passed && confirmed.length === 0 +if (!verifyClean) { + return { + status: 'NEEDS ATTENTION — verify not clean after 3 rounds; branch left uncommitted', + branch: BRANCH, + mechanical: checks, + parity, + unresolvedFindings: confirmed, + implBlockers, + reconBlockers, + spec, + } +} + +// ----------------------------------------------------------------- package + +phase('Package') +const commit = await agent(`${PREAMBLE.replace('do NOT commit, ', '')} +YOU are the commit agent: +1. Read ~/Documents/repositories/seahaven/engineering-handbook/commit-messages.md + and follow it exactly. +2. git add only paths under: ${EXPECTED_SCOPE.join(', ')} and + .claude/workflows/phase-3-shared-extraction.js. NOT .coverage, NOT + package/. Verify the staged set with git status — the deletions of the + moved originals MUST be staged too. +3. ONE commit; write the message to /tmp/phase3-commit-msg.txt and use + git commit -F /tmp/phase3-commit-msg.txt (backticks in -m get eaten by + zsh). Suggested subject: + "feat: extract lambdas/shared/ — single-source ses_auth, web_ui auth, email parsing, EMF emitter (refactor phase 3)" + Body: the four moves with the byte-identity evidence one-liner for + ses_auth, the flat-landing import rule, the shared cp bundling change + + web_ui staging, the derived-agreement emitter decision (constraint 6), + and the test-plumbing summary. NO AI attribution / Co-Authored-By + lines. +4. Do NOT push. Return commit sha + shortstat in summary.`, + { label: 'package:commit', model: 'sonnet', phase: 'Package', schema: IMPL }) + +return { + status: 'BUILT — committed locally, NOT pushed', + branch: BRANCH, + base: BASE, + commit: commit ? commit.summary : 'commit agent died — commit manually', + spec: { sharedModules: spec.sharedModules, webUiStaging: spec.webUiStaging, emfDesign: spec.emfDesign, derivedAgreementNote: spec.notes }, + parityEvidence: parity ? { po: parity.poVerdict, wo: parity.woVerdict, webUi: parity.webUiVerdict } : null, + implementation: implOk.map(r => r.summary), + filesChanged: implOk.flatMap(r => r.filesChanged), + verifyRounds: round + 1, + blockers: implBlockers.concat(reconBlockers), + outstandingGates: [ + '/sh-security-review (MANDATORY before push — auth code moved: ses_auth + the web_ui auth gate are exactly the authentication surface; run on the committed diff, and re-run after any post-review fix to this code)', + 'cross-family cross_review.py NOT mandatory (no IAM change; handler event/return contracts unchanged — internal module moves only). Opt-in if judgment says so', + 'push + PR + gh pr checks green', + 'deploy-then-merge: deploy from branch, smoke green, one real PO + WO email each, watch BOTH sender-auth-rejected alarms through live mail (the auth move must not change accept/reject behavior), verify web_ui login still works, THEN merge — and note the post-merge CI redeploy being a no-op (unchanged asset hashes) is itself a verification signal', + ], +} diff --git a/README.md b/README.md index 3d25999..61ba9ab 100644 --- a/README.md +++ b/README.md @@ -100,7 +100,7 @@ All Lambdas: Python 3.12, ARM64, 60-day log retention. ### Sender authentication (INFRA-107) -The `From` header and any `Authentication-Results` header inside the raw MIME are attacker-forgeable, so neither is trusted. Instead, both email processors (`lambdas/*/email_processor/ses_auth.py`) authenticate the sender against the verdicts SES itself stamps at delivery time, failing closed: +The `From` header and any `Authentication-Results` header inside the raw MIME are attacker-forgeable, so neither is trusted. Instead, both email processors authenticate the sender against the verdicts SES itself stamps at delivery time, failing closed. The authenticator is **single-sourced** at `lambdas/shared/ses_auth.py` (Phase 3) — previously duplicated byte-for-byte in each pipeline's `email_processor/` dir and kept in sync by a fixture-hygiene test; now one copy, so a future hardening fix to the fail-closed logic lands **once** instead of needing two identical edits. The CDK bundling `cp`s it flat beside each handler so the handlers' unchanged `from ses_auth import authenticate_inbound_email` still resolves at runtime (see [`lambdas/shared/`](#deploy-pipeline-guards-phase-0)). The gate: 1. Take **only the topmost** `Authentication-Results` header (SES prepends its trace headers; any lower copies arrived inside the message and are ignored). 2. Require its authserv-id to be `amazonses.com`. @@ -189,6 +189,25 @@ This is cleanup, not a regression: none of those file classes are imported at ru `tests/test_bundle_consistency.py` guards all of the above with a pure-AST check (no synth, no boto3, no handler import): it parses each handler.py's top-level first-party sibling imports, extracts the bundling `command=[...]` string from the corresponding CDK stack file, and asserts every required sibling module is guaranteed to ship. It recognizes both the scoped glob (`cp po/email_processor/*.py` / `cp wo/email_processor/*.py`, with or without a path prefix) and a whole-dir recursive copy (`cp -r . /asset-output/`) as unconditionally-safe shapes, and falls back to literal filename matching for any other (allowlist-style) shape. It pins each stack's command to the scoped-glob form specifically — a future revert to a narrowed single-file copy, a commented-out glob, or a filename allowlist missing a sibling all fail CI loudly instead of silently shipping a broken bundle. Runs in the existing pytest step, before synth. +### Phase 3: shared module extraction (`lambdas/shared/`) + +Four first-party modules that were previously duplicated per pipeline (or inlined in each handler) are now **single-sourced** under `lambdas/shared/`, following the handbook's `lambdas/shared/` convention: + +| Module | What it is | Imported by | +|---|---|---| +| `ses_auth.py` | fail-closed SES sender-authentication (INFRA-107) | both email processors | +| `web_ui_auth.py` | fail-closed `X-Auth-Token` gate + token cache (INFRA-74) | both web_ui handlers | +| `email_parsing.py` | `parse_raw_email` (the WO superset that returns `cc` unconditionally; PO simply ignores `cc`) | both email processors | +| `emf.py` | generic CloudWatch EMF emitter (`emit_metric`, `emit_parse_outcome`) parameterized by namespace / dimension-sets / properties | both email processors (PO also uses `emit_metric` for `DerivedFieldAgreement`) | + +**Flat-landing import rule.** The shared dir has **no `__init__.py`** — the modules are consumed by bare name (`from ses_auth import ...`, `from emf import emit_parse_outcome`), exactly as when they were siblings. This works because the bundling `cp` lands them **flat in `/asset-output/`** beside `handler.py`, so at runtime each shared module sits on the function's own `sys.path` under its bare name — the handler import lines are unchanged, which is what keeps the byte-identical fail-closed `ses_auth` behavior through the move. The load-bearing `emf` dimension-set list `[["ParseMethod"], ["ParseMethod", "TemplateId"]]` is now pinned **once** in `emf.py` (one-sided dimension drift between the two pipelines becomes structurally impossible), while the deliberate per-pipeline **emission-ordering** differences stay in the handlers (PO emits `ai_fallback` *before* the Bedrock call with an intentional double-count; WO emits mutually-exclusive `ai_fallback`/`ai_fallback_rejected` after its gate). + +**Bundling — email processors.** Both email-processor commands append a second glob, `cp shared/*.py /asset-output/`, after their own `cp /email_processor/*.py`. This ships all four shared modules flat into each email-processor zip. `web_ui_auth.py` therefore rides along into both email-processor bundles even though the email handlers never import it — a harmless, deliberate consequence of the all-of-`shared/` glob (`PO_EXPECTED_TOP_LEVEL_MODULES` and the bundle-parity expectations account for it). The base is cp-only (Phase 7 removed the pip install / `manylinux` pin from both email-processor commands, and this phase moves only pure first-party modules with no new dependencies, so it **stays** cp-only — no pip step is reintroduced). + +**Bundling — web UIs.** Both `po-web-ui` and `workorder-web-ui` gain the same widened-root Docker bundling mechanism: their `Code.from_asset` root widens to `../lambdas` and their command copies the function's own dir contents plus **only** `shared/web_ui_auth.py` (`cp shared/web_ui_auth.py`, *not* `cp shared/*.py`) — the web UIs need only the auth module, and shipping the email-processor-only modules would break the "deployed set + `web_ui_auth`, nothing else" parity. The per-stack INFRA-74 comments stay in each handler (their wording is deliberately pipeline-specific and is not unified). `site_extractor`'s asset is untouched. + +`tests/test_bundle_consistency.py` is updated in lockstep without losing teeth: `_first_party_sibling_imports` resolves shared-sourced imports under `lambdas/shared/`; the command extractor selects the email-processor command now that each stack has two bundled functions; `_bundling_ships_all` accumulates shipped module stems across **both** globs; `PO_EXPECTED_TOP_LEVEL_MODULES` gains the four shared modules; and a new pin + mutation test require the `cp shared/*.py` line to be actually executed (a commented-out or removed shared `cp` fails CI). + ## Security **Web UI auth (defense-in-depth).** The `po-web-ui` / `workorder-web-ui` handlers refuse unauthenticated requests even though their public Function URLs were removed (INFRA-74). Each requires a shared secret in the `X-Auth-Token` header (or `Authorization: Bearer `), compared in constant time against the configured token. The handler **fails closed** if the token is unset or unreadable (denies all). The token lives in the Secrets Manager secret `procurement-ingest/web-ui-auth-token`; only its ARN is passed to the Lambda (`WEB_UI_AUTH_TOKEN_SECRET_ARN`), and the value is fetched at runtime — never embedded in the CloudFormation template or Lambda env vars. The fetched value is cached in the warm container with a short TTL (5 min) so a rotated secret propagates without waiting for the execution environment to recycle. This is a defense-in-depth floor for a detached URL, not primary auth. @@ -325,23 +344,32 @@ cdk/ po_stack.py # Purchase order pipeline resources wo_stack.py # Work order pipeline resources lambdas/ # Phase 2: shared Code.from_asset("../lambdas") bundling root for - # BOTH po-email-processor and workorder-email-processor -- each - # bundling command `cp`s only its own po/email_processor/*.py or - # wo/email_processor/*.py subset out; site_extractor and both - # web_ui assets keep their own narrower, non-bundled asset root + # BOTH po-email-processor and workorder-email-processor (and, since + # Phase 3, both web_ui functions) -- each email-processor command + # `cp`s its own po/email_processor/*.py (or wo/...) subset PLUS + # `cp shared/*.py`; each web_ui command `cp`s its own dir PLUS only + # `shared/web_ui_auth.py`; site_extractor keeps its narrower, non- + # bundled asset root + shared/ # Phase 3: single-sourced first-party modules, landed FLAT (no + # __init__.py -- bare-name imports) into each bundle by the cp above + ses_auth.py # fail-closed SES sender-auth (INFRA-107) -- one copy, one fix + web_ui_auth.py # fail-closed X-Auth-Token gate + token cache (INFRA-74) + email_parsing.py # parse_raw_email (WO superset; returns cc unconditionally) + emf.py # generic CloudWatch EMF emitter (dimension-sets pinned once) po/ # PO pipeline Lambdas email_processor/ handler.py # template-first + Bedrock fallback, EMF metric, merge writes, {"healthcheck": true} early-return template_parser.py # pure deterministic Coupa parser + fail-closed validation gate + derived_fields.py # deterministic site_code/trade/fiscal_year classifier (untouched by Phase 3) tests/ # golden-file + validation-gate + fallback-dispatch + healthcheck tests + fixtures site_extractor/ - web_ui/ + web_ui/ # handler.py imports `from web_ui_auth import is_authenticated` (shared) wo/ # WO pipeline Lambdas email_processor/ handler.py # template-first + Bedrock fallback, EMF metric, #23 comment_id, {"healthcheck": true} early-return template_parser.py # pure deterministic parser + fail-closed validation gate tests/ # golden-file + validation-gate + comment_id + fallback + healthcheck tests - web_ui/ + web_ui/ # handler.py imports `from web_ui_auth import is_authenticated` (shared) scripts/ reprocess.py backfill_sites.py @@ -349,7 +377,7 @@ scripts/ test_local.py # Parse sample emails through Bedrock locally (no AWS mutation) tests/ requirements.txt # Test-only deps (moto) - conftest.py # AWS env stubs + per-pipeline module loader + conftest.py # AWS env stubs + module loader (per-pipeline siblings + shared/ fallback) test_pad_zip.py # PO zip-code padding tests test_parse_raw_email.py # MIME parsing tests (PO + WO handlers) test_po_merge.py # PO merge-write semantics tests (#97) diff --git a/cdk/po_stack.py b/cdk/po_stack.py index 4cd0614..a63a4ba 100644 --- a/cdk/po_stack.py +++ b/cdk/po_stack.py @@ -262,7 +262,13 @@ class PoIngestStack(Stack): # pip step removed in Phase 7: requirements.txt is now # empty (boto3 comes from the Lambda runtime), so nothing # is installed and the manylinux pin has nothing to pin. - "cp po/email_processor/*.py /asset-output/", + # shared/*.py ships the four modules extracted to + # lambdas/shared/ (Phase 3): ses_auth, web_ui_auth, + # email_parsing, emf. Flat cp keeps the bare-name + # imports (e.g. `from ses_auth import ...`) resolving + # unchanged in /asset-output. + "cp po/email_processor/*.py /asset-output/ && " + "cp shared/*.py /asset-output/", ], ), ), @@ -602,14 +608,32 @@ class PoIngestStack(Stack): architecture=lambda_.Architecture.ARM_64, handler="handler.handler", code=lambda_.Code.from_asset( - # requirements.txt is excluded from the bundle: it exists only - # as a Dependabot anchor (git-based scan sees it), never pip- - # installed (this is a plain non-bundled asset) and never needed - # at runtime (boto3 comes from the Lambda runtime). Excluding it - # keeps the deployed asset hash neutral vs base while the manifest - # still lands in git for Dependabot. - "../lambdas/po/web_ui", + "../lambdas", exclude=["**/__pycache__/**", "requirements.txt"], + bundling=cdk.BundlingOptions( + image=lambda_.Runtime.PYTHON_3_12.bundling_image, + command=[ + "bash", + "-c", + # web_ui_auth.py is shared (lambdas/shared) and must land + # FLAT beside handler.py so the bare + # `from web_ui_auth import is_authenticated` resolves at + # runtime. Only web_ui_auth is copied from shared/ -- the + # other shared modules (ses_auth/email_parsing/emf) are + # email-processor-only and must not bloat the web UI zip. + # requirements.txt stays excluded (Dependabot anchor only, + # never runtime): keeps the deployed file list = {handler, + # web_ui_auth}. NOTE: the top-level `exclude=` on + # from_asset only filters the asset-hash fingerprint, NOT + # the directory Docker bundling actually mounts, so a + # local __pycache__/requirements.txt on disk at synth + # time WOULD otherwise leak into the bundled zip -- strip + # them explicitly post-cp instead of relying on exclude. + "cp -r po/web_ui/. /asset-output/ && " + "cp shared/web_ui_auth.py /asset-output/ && " + "rm -rf /asset-output/__pycache__ /asset-output/requirements.txt", + ], + ), ), timeout=Duration.seconds(60), memory_size=256, diff --git a/cdk/wo_stack.py b/cdk/wo_stack.py index a99be30..e5b37e7 100644 --- a/cdk/wo_stack.py +++ b/cdk/wo_stack.py @@ -248,7 +248,13 @@ class WorkorderIngestStack(Stack): # pip step removed in Phase 7: requirements.txt is now empty # (boto3 comes from the Lambda runtime), so nothing is installed # and the manylinux pin has nothing to pin. cp-only is safe. - "cp wo/email_processor/*.py /asset-output/", + # shared/*.py ships the four modules extracted to + # lambdas/shared/ (Phase 3): ses_auth, web_ui_auth, + # email_parsing, emf. Flat cp keeps the bare-name + # imports (e.g. `from ses_auth import ...`) resolving + # unchanged in /asset-output. + "cp wo/email_processor/*.py /asset-output/ && " + "cp shared/*.py /asset-output/", ], ), ), @@ -548,7 +554,31 @@ class WorkorderIngestStack(Stack): architecture=lambda_.Architecture.ARM_64, handler="handler.handler", code=lambda_.Code.from_asset( - "../lambdas/wo/web_ui", exclude=["**/__pycache__/**"] + "../lambdas", + exclude=["**/__pycache__/**"], + bundling=cdk.BundlingOptions( + image=lambda_.Runtime.PYTHON_3_12.bundling_image, + command=[ + "bash", + "-c", + # web_ui_auth.py is shared (lambdas/shared) and must land + # FLAT beside handler.py so + # `from web_ui_auth import is_authenticated` resolves at + # runtime. Only web_ui_auth is copied from shared/. WO + # baseline keeps __init__.py and requirements.txt, so the + # whole web_ui dir is copied; deployed file list becomes + # {__init__, handler, requirements.txt, web_ui_auth}. + # NOTE: the top-level `exclude=` on from_asset only + # filters the asset-hash fingerprint, NOT the directory + # Docker bundling actually mounts, so a local + # __pycache__ on disk at synth time WOULD otherwise leak + # into the bundled zip -- strip it explicitly post-cp + # instead of relying on exclude. + "cp -r wo/web_ui/. /asset-output/ && " + "cp shared/web_ui_auth.py /asset-output/ && " + "rm -rf /asset-output/__pycache__", + ], + ), ), timeout=Duration.seconds(15), memory_size=128, diff --git a/lambdas/po/email_processor/handler.py b/lambdas/po/email_processor/handler.py index 16bdaa1..36a15e8 100644 --- a/lambdas/po/email_processor/handler.py +++ b/lambdas/po/email_processor/handler.py @@ -7,17 +7,17 @@ back to Claude on Bedrock for structured extraction on a miss/invalid result, then writes the result to the purchase-orders DynamoDB table. """ -import email import json import logging import os import re from datetime import datetime, timezone from decimal import Decimal, InvalidOperation -from email import policy import boto3 from derived_fields import derive_all +from email_parsing import parse_raw_email +from emf import emit_metric, emit_parse_outcome from ses_auth import authenticate_inbound_email from template_parser import try_deterministic_parse, validate_ai_fallback @@ -36,7 +36,6 @@ BEDROCK_MODEL_ID = os.environ.get( # CloudWatch EMF namespace/metric for the parse-outcome metric (the PO # fallback-rate alarm in cdk/po_stack.py reads the ["ParseMethod"] series). METRIC_NAMESPACE = "Seahaven/PoIngest" -METRIC_NAME = "ParseOutcome" # Shadow telemetry for the Python-derived classifier bake. One EMF record per # derived field per email, emitted ONLY on the ai_fallback path (the template @@ -249,36 +248,6 @@ The Coupa commodity/category field if present in the email (e.g., "Maintenance - """ -def parse_raw_email(raw_bytes: bytes) -> dict: - """Parse a raw MIME email into subject, sender, and body text.""" - msg = email.message_from_bytes(raw_bytes, policy=policy.default) - - subject = msg.get("Subject", "") - sender = msg.get("From", "") - to = msg.get("To", "") - date = msg.get("Date", "") - - body = "" - if msg.is_multipart(): - for part in msg.walk(): - content_type = part.get_content_type() - if content_type == "text/plain": - body = part.get_content() - break - elif content_type == "text/html" and not body: - body = part.get_content() - else: - body = msg.get_content() - - return { - "subject": subject, - "sender": sender, - "to": to, - "date": date, - "body": body, - } - - def extract_with_claude(email_data: dict) -> dict: """Send parsed email to Claude on Bedrock for structured extraction. @@ -347,26 +316,9 @@ def _emit_parse_method_metric(method, template_id, reason_code, po_number): dashboards). CloudWatch materializes only the exact dimension sets listed here and does NOT auto-aggregate, so the alarm's single-dimension query would receive no data unless ["ParseMethod"] is emitted explicitly.""" - emf = { - "_aws": { - # EMF requires Timestamp (epoch ms); without it CloudWatch may not - # extract the metric datapoint from the log event. - "Timestamp": int(datetime.now(timezone.utc).timestamp() * 1000), - "CloudWatchMetrics": [ - { - "Namespace": METRIC_NAMESPACE, - "Dimensions": [["ParseMethod"], ["ParseMethod", "TemplateId"]], - "Metrics": [{"Name": METRIC_NAME, "Unit": "Count"}], - } - ], - }, - "ParseMethod": method, - "TemplateId": template_id or "unknown", - "ReasonCode": reason_code or "ok", - "po_number": po_number or "", - METRIC_NAME: 1, - } - print(json.dumps(emf)) + emit_parse_outcome( + METRIC_NAMESPACE, method, template_id, reason_code, "po_number", po_number + ) def _derived_agreement(llm_value, python_value) -> str | None: @@ -403,29 +355,22 @@ def _emit_derived_agreement_metric(field, llm_value, python_value, po_number): agreement = _derived_agreement(llm_value, python_value) if agreement is None: return - emf = { - "_aws": { - "Timestamp": int(datetime.now(timezone.utc).timestamp() * 1000), - "CloudWatchMetrics": [ - { - "Namespace": METRIC_NAMESPACE, - "Dimensions": [["Field", "Agreement"]], - "Metrics": [{"Name": DERIVED_METRIC_NAME, "Unit": "Count"}], - } - ], + emit_metric( + METRIC_NAMESPACE, + DERIVED_METRIC_NAME, + [["Field", "Agreement"]], + { + "Field": field, + "Agreement": agreement, + "po_number": po_number or "", + # Length-clamped: Python values are regex/enum-bounded by construction, + # but the LLM value is schema-unvalidated model output -- a hallucinated + # free-text field must not land unbounded in a 2-month log line + # (sh-security-review PO-DC-02, confirmed low). + "PythonValue": "" if python_value is None else str(python_value)[:64], + "LlmValue": "" if llm_value is None else str(llm_value)[:64], }, - "Field": field, - "Agreement": agreement, - "po_number": po_number or "", - # Length-clamped: Python values are regex/enum-bounded by construction, - # but the LLM value is schema-unvalidated model output -- a hallucinated - # free-text field must not land unbounded in a 2-month log line - # (sh-security-review PO-DC-02, confirmed low). - "PythonValue": "" if python_value is None else str(python_value)[:64], - "LlmValue": "" if llm_value is None else str(llm_value)[:64], - DERIVED_METRIC_NAME: 1, - } - print(json.dumps(emf)) + ) def pad_zip(zip_code: str | None) -> str | None: diff --git a/lambdas/po/email_processor/tests/_po_parser_support.py b/lambdas/po/email_processor/tests/_po_parser_support.py index 36587e7..bcebbf4 100644 --- a/lambdas/po/email_processor/tests/_po_parser_support.py +++ b/lambdas/po/email_processor/tests/_po_parser_support.py @@ -34,6 +34,9 @@ import moto # noqa: F401 _HERE = os.path.dirname(__file__) _MODULE_DIR = os.path.abspath(os.path.join(_HERE, "..")) +# Phase 3 single-sourced ses_auth/email_parsing/emf under lambdas/shared/. +# _HERE = lambdas/po/email_processor/tests -> ../../.. = lambdas. +_SHARED_DIR = os.path.abspath(os.path.join(_HERE, "..", "..", "..", "shared")) # The handler creates boto3 clients at import time; give it a region and dummy # creds so import works offline/under CI. @@ -59,6 +62,24 @@ def _load_module(filename, module_name): return module +def _load_shared_module(filename, module_name): + """Load a Phase 3 shared sibling (ses_auth/email_parsing/emf) from + lambdas/shared/. Mirrors ``_load_module`` but resolves against + ``_SHARED_DIR`` -- the handler ``from email_parsing import ...`` / + ``from emf import ...`` / ``from ses_auth import ...`` bare imports must be + pre-bound in sys.modules because this support module does NOT put shared/ + on sys.path.""" + if module_name in sys.modules: + return sys.modules[module_name] + spec = importlib.util.spec_from_file_location( + module_name, os.path.join(_SHARED_DIR, filename) + ) + module = importlib.util.module_from_spec(spec) + sys.modules[module_name] = module + spec.loader.exec_module(module) + return module + + def load_template_parser(): return _load_module("template_parser.py", f"{_HANDLER_NAME}__template_parser") @@ -68,10 +89,14 @@ def load_po_handler(): return sys.modules[_HANDLER_NAME] siblings = { "template_parser": load_template_parser(), - "ses_auth": _load_module("ses_auth.py", f"{_HANDLER_NAME}__ses_auth"), + "ses_auth": _load_shared_module("ses_auth.py", f"{_HANDLER_NAME}__ses_auth"), "derived_fields": _load_module( "derived_fields.py", f"{_HANDLER_NAME}__derived_fields" ), + "email_parsing": _load_shared_module( + "email_parsing.py", f"{_HANDLER_NAME}__email_parsing" + ), + "emf": _load_shared_module("emf.py", f"{_HANDLER_NAME}__emf"), } saved = {name: sys.modules.get(name) for name in siblings} sys.modules.update(siblings) diff --git a/lambdas/po/email_processor/tests/test_po_bedrock_fallback.py b/lambdas/po/email_processor/tests/test_po_bedrock_fallback.py index fb3a018..810fa3e 100644 --- a/lambdas/po/email_processor/tests/test_po_bedrock_fallback.py +++ b/lambdas/po/email_processor/tests/test_po_bedrock_fallback.py @@ -633,10 +633,12 @@ def test_fixture_hygiene_ses_auth_and_scrub_markers(): from _po_parser_support import ( ADVERSARIAL_STEMS, AI_FALLBACK_STEMS, - _load_module, + _load_shared_module, ) - ses_auth = _load_module("ses_auth.py", "po_email_processor_handler__ses_auth") + ses_auth = _load_shared_module( + "ses_auth.py", "po_email_processor_handler__ses_auth" + ) coupa = ( [("new-po", s) for s in NEW_PO_STEMS] + [("cancellation", s) for s in CANCELLATION_STEMS] diff --git a/lambdas/po/web_ui/handler.py b/lambdas/po/web_ui/handler.py index e229a0d..8d79513 100644 --- a/lambdas/po/web_ui/handler.py +++ b/lambdas/po/web_ui/handler.py @@ -5,15 +5,14 @@ Serves a simple HTML dashboard for viewing purchase orders. Accessed via Lambda Function URL. """ -import hmac import json import logging import os -import time from decimal import Decimal from html import escape as esc import boto3 +from web_ui_auth import is_authenticated logger = logging.getLogger() logger.setLevel(logging.INFO) @@ -30,73 +29,6 @@ PO_TABLE = os.environ.get("PO_TABLE", "purchase-orders") # Manager to keep it out of CloudFormation templates and Lambda env vars. If the # ARN is unset or the secret is missing, the handler fails closed and denies every # request. -_WEB_UI_AUTH_TOKEN_SECRET_ARN = os.environ.get("WEB_UI_AUTH_TOKEN_SECRET_ARN") -# Refresh the cached token this often so a rotated secret propagates without -# waiting for the execution environment to recycle (emergency-rotation path). -_AUTH_TOKEN_CACHE_TTL_SECONDS = 300 -_auth_token_cache = None -_auth_token_cached_at = 0.0 - - -def _get_auth_token() -> str | None: - """Fetch the shared web UI auth token from Secrets Manager. - - Cached in the warm container for a short TTL so we don't hit Secrets Manager - on every request, while still picking up a rotated secret within the TTL - rather than only when the execution environment recycles. Returns None when - not configured or unreadable (the caller then fails closed). - """ - global _auth_token_cache, _auth_token_cached_at - now = time.monotonic() - if ( - _auth_token_cache is not None - and now - _auth_token_cached_at < _AUTH_TOKEN_CACHE_TTL_SECONDS - ): - return _auth_token_cache - if not _WEB_UI_AUTH_TOKEN_SECRET_ARN: - return None - secrets = boto3.client("secretsmanager") - try: - secret = secrets.get_secret_value(SecretId=_WEB_UI_AUTH_TOKEN_SECRET_ARN) - _auth_token_cache = secret["SecretString"] - _auth_token_cached_at = now - return _auth_token_cache - except Exception: - # Fail closed (return None -> caller 401s) but surface the failure: a - # Secrets Manager permission/config error would otherwise make every - # request 401 with no operational signal. The secret value is never - # logged. - logger.exception( - "Failed to fetch web UI auth token from Secrets Manager; " - "denying request (failing closed)" - ) - return None - - -def _header(event: dict, name: str) -> str: - """Case-insensitive header lookup from a Lambda Function URL / APIGW event.""" - headers = event.get("headers") or {} - name_lower = name.lower() - for key, value in headers.items(): - if key.lower() == name_lower: - return value or "" - return "" - - -def is_authenticated(event: dict) -> bool: - """Constant-time check of the request's shared secret against the configured - token. Fails closed when no token is configured.""" - token = _get_auth_token() - if not token: - return False - presented = _header(event, "x-auth-token") - if not presented: - auth = _header(event, "authorization") - if auth.lower().startswith("bearer "): - presented = auth[7:].strip() - if not presented: - return False - return hmac.compare_digest(presented, token) def get_purchase_orders(limit=500): diff --git a/lambdas/shared/email_parsing.py b/lambdas/shared/email_parsing.py new file mode 100644 index 0000000..538f0ac --- /dev/null +++ b/lambdas/shared/email_parsing.py @@ -0,0 +1,34 @@ +import email +from email import policy + + +def parse_raw_email(raw_bytes: bytes) -> dict: + """Parse a raw email into subject, sender, body text.""" + msg = email.message_from_bytes(raw_bytes, policy=policy.default) + + subject = msg.get("Subject", "") + sender = msg.get("From", "") + to = msg.get("To", "") + cc = msg.get("Cc", "") + date = msg.get("Date", "") + + body = "" + if msg.is_multipart(): + for part in msg.walk(): + content_type = part.get_content_type() + if content_type == "text/plain": + body = part.get_content() + break + elif content_type == "text/html" and not body: + body = part.get_content() + else: + body = msg.get_content() + + return { + "subject": subject, + "sender": sender, + "to": to, + "cc": cc, + "date": date, + "body": body, + } diff --git a/lambdas/shared/emf.py b/lambdas/shared/emf.py new file mode 100644 index 0000000..7b22ca8 --- /dev/null +++ b/lambdas/shared/emf.py @@ -0,0 +1,82 @@ +"""Shared CloudWatch EMF emitter for the procurement-ingest pipelines. + +A single generic ``emit_metric`` builds the Embedded Metric Format envelope +(``_aws`` block + promoted dimension properties + the metric-value key) and +prints it to stdout, where the Lambda log subscription materializes the metric. +Both the PO and WO email processors emit their parse-outcome metric through +``emit_parse_outcome`` so the load-bearing dimension-set list +``[["ParseMethod"], ["ParseMethod", "TemplateId"]]`` is pinned in exactly ONE +place -- a one-sided dimension change to one pipeline is structurally +impossible. + +Envelope byte-equivalence rests on CPython dict insertion order (preserved by +json.dumps with default separators): ``{**properties, metric_name: value}`` +yields ``_aws`` first, then the caller's properties in order, then the metric +value last -- matching every inline emitter this module replaced. +""" + +import json +from datetime import datetime, timezone + +PARSE_METRIC_NAME = "ParseOutcome" + +# Two dimension sets are published for every parse-outcome metric: ["ParseMethod"] +# (aggregated across all template ids -- the series the fallback-rate alarms +# query) AND ["ParseMethod", "TemplateId"] (per-template breakdown for Logs +# Insights / dashboards). CloudWatch materializes only the exact dimension sets +# listed here and does NOT auto-aggregate, so an alarm's single-dimension query +# would receive no data unless ["ParseMethod"] is emitted explicitly. Pinned +# ONCE here; both pipelines share it. +_PARSE_DIMENSION_SETS = [["ParseMethod"], ["ParseMethod", "TemplateId"]] + + +def emit_metric( + namespace, metric_name, dimension_sets, properties, *, value=1, unit="Count" +): + """Print one CloudWatch EMF log line for ``metric_name`` in ``namespace``. + + Zero-latency (no PutMetricData API call): the extraction path is async and + the role already has logs:PutLogEvents. ``properties`` are emitted in caller + order between the ``_aws`` block and the trailing metric-value key; the keys + named in ``dimension_sets`` are the promoted (dimensioned) fields, the rest + ride along as Logs-Insights-queryable properties. + """ + emf = { + "_aws": { + # EMF requires Timestamp (epoch ms); without it CloudWatch may not + # extract the metric datapoint from the log event. + "Timestamp": int(datetime.now(timezone.utc).timestamp() * 1000), + "CloudWatchMetrics": [ + { + "Namespace": namespace, + "Dimensions": dimension_sets, + "Metrics": [{"Name": metric_name, "Unit": unit}], + } + ], + }, + **properties, + metric_name: value, + } + print(json.dumps(emf)) + + +def emit_parse_outcome(namespace, method, template_id, reason_code, id_key, id_value): + """Emit one parse-outcome EMF line shared by both email processors. + + ParseMethod/TemplateId are the only promoted (dimensioned) fields to keep + cardinality low; ReasonCode and the pipeline id (``id_key``: ``po_number`` + or ``work_order_id``) ride along as Logs-Insights-queryable properties. See + ``_PARSE_DIMENSION_SETS`` for why both the single- and two-dimension sets + are published. + """ + emit_metric( + namespace, + PARSE_METRIC_NAME, + _PARSE_DIMENSION_SETS, + { + "ParseMethod": method, + "TemplateId": template_id or "unknown", + "ReasonCode": reason_code or "ok", + id_key: id_value or "", + }, + ) diff --git a/lambdas/po/email_processor/ses_auth.py b/lambdas/shared/ses_auth.py similarity index 100% rename from lambdas/po/email_processor/ses_auth.py rename to lambdas/shared/ses_auth.py diff --git a/lambdas/shared/web_ui_auth.py b/lambdas/shared/web_ui_auth.py new file mode 100644 index 0000000..4b2d245 --- /dev/null +++ b/lambdas/shared/web_ui_auth.py @@ -0,0 +1,77 @@ +import hmac +import logging +import os +import time + +import boto3 + +logger = logging.getLogger() + + +_WEB_UI_AUTH_TOKEN_SECRET_ARN = os.environ.get("WEB_UI_AUTH_TOKEN_SECRET_ARN") +# Refresh the cached token this often so a rotated secret propagates without +# waiting for the execution environment to recycle (emergency-rotation path). +_AUTH_TOKEN_CACHE_TTL_SECONDS = 300 +_auth_token_cache = None +_auth_token_cached_at = 0.0 + + +def _get_auth_token() -> str | None: + """Fetch the shared web UI auth token from Secrets Manager. + + Cached in the warm container for a short TTL so we don't hit Secrets Manager + on every request, while still picking up a rotated secret within the TTL + rather than only when the execution environment recycles. Returns None when + not configured or unreadable (the caller then fails closed). + """ + global _auth_token_cache, _auth_token_cached_at + now = time.monotonic() + if ( + _auth_token_cache is not None + and now - _auth_token_cached_at < _AUTH_TOKEN_CACHE_TTL_SECONDS + ): + return _auth_token_cache + if not _WEB_UI_AUTH_TOKEN_SECRET_ARN: + return None + secrets = boto3.client("secretsmanager") + try: + secret = secrets.get_secret_value(SecretId=_WEB_UI_AUTH_TOKEN_SECRET_ARN) + _auth_token_cache = secret["SecretString"] + _auth_token_cached_at = now + return _auth_token_cache + except Exception: + # Fail closed (return None -> caller 401s) but surface the failure: a + # Secrets Manager permission/config error would otherwise make every + # request 401 with no operational signal. The secret value is never + # logged. + logger.exception( + "Failed to fetch web UI auth token from Secrets Manager; " + "denying request (failing closed)" + ) + return None + + +def _header(event: dict, name: str) -> str: + """Case-insensitive header lookup from a Lambda Function URL / APIGW event.""" + headers = event.get("headers") or {} + name_lower = name.lower() + for key, value in headers.items(): + if key.lower() == name_lower: + return value or "" + return "" + + +def is_authenticated(event: dict) -> bool: + """Constant-time check of the request's shared secret against the configured + token. Fails closed when no token is configured.""" + token = _get_auth_token() + if not token: + return False + presented = _header(event, "x-auth-token") + if not presented: + auth = _header(event, "authorization") + if auth.lower().startswith("bearer "): + presented = auth[7:].strip() + if not presented: + return False + return hmac.compare_digest(presented, token) diff --git a/lambdas/wo/email_processor/handler.py b/lambdas/wo/email_processor/handler.py index 4ae6773..7b274b5 100644 --- a/lambdas/wo/email_processor/handler.py +++ b/lambdas/wo/email_processor/handler.py @@ -6,19 +6,18 @@ Parses the raw email, sends it to Claude for structured extraction, then writes the result to DynamoDB. """ -import email import hashlib import json import logging import os import re from datetime import datetime, timezone -from email import policy from email.utils import parsedate_to_datetime import boto3 +from email_parsing import parse_raw_email +from emf import emit_parse_outcome from ses_auth import authenticate_inbound_email - from template_parser import try_deterministic_parse, validate_ai_fallback logger = logging.getLogger() @@ -36,7 +35,6 @@ BEDROCK_MODEL_ID = os.environ.get( # CloudWatch EMF namespace + metric for parse-outcome observability. METRIC_NAMESPACE = "Seahaven/WorkorderIngest" -METRIC_NAME = "ParseOutcome" EXTRACTION_PROMPT = """\ You are an email parser for a facilities maintenance work order system. @@ -79,38 +77,6 @@ Rules: """ -def parse_raw_email(raw_bytes: bytes) -> dict: - """Parse a raw email into subject, sender, body text.""" - msg = email.message_from_bytes(raw_bytes, policy=policy.default) - - subject = msg.get("Subject", "") - sender = msg.get("From", "") - to = msg.get("To", "") - cc = msg.get("Cc", "") - date = msg.get("Date", "") - - body = "" - if msg.is_multipart(): - for part in msg.walk(): - content_type = part.get_content_type() - if content_type == "text/plain": - body = part.get_content() - break - elif content_type == "text/html" and not body: - body = part.get_content() - else: - body = msg.get_content() - - return { - "subject": subject, - "sender": sender, - "to": to, - "cc": cc, - "date": date, - "body": body, - } - - # An / (or whitespace-padded variant) appearing INSIDE the # untrusted email text could forge the data-block boundary, so any such # sequence is neutralized before wrapping. A single [\s/]* class (not two @@ -185,26 +151,14 @@ def emit_parse_metric(method, template_id, reason_code, work_order_id): dashboards). CloudWatch materializes only the exact dimension sets listed here and does NOT auto-aggregate, so the alarm's single-dimension query would receive no data unless ["ParseMethod"] is emitted explicitly.""" - emf = { - "_aws": { - # EMF requires Timestamp (epoch ms); without it CloudWatch may not - # extract the metric datapoint from the log event (advisory A2). - "Timestamp": int(datetime.now(timezone.utc).timestamp() * 1000), - "CloudWatchMetrics": [ - { - "Namespace": METRIC_NAMESPACE, - "Dimensions": [["ParseMethod"], ["ParseMethod", "TemplateId"]], - "Metrics": [{"Name": METRIC_NAME, "Unit": "Count"}], - } - ], - }, - "ParseMethod": method, - "TemplateId": template_id or "unknown", - "ReasonCode": reason_code or "ok", - "work_order_id": work_order_id or "", - METRIC_NAME: 1, - } - print(json.dumps(emf)) + emit_parse_outcome( + METRIC_NAMESPACE, + method, + template_id, + reason_code, + "work_order_id", + work_order_id, + ) def save_work_order(parsed: dict, s3_key: str): diff --git a/lambdas/wo/email_processor/ses_auth.py b/lambdas/wo/email_processor/ses_auth.py deleted file mode 100644 index db58ce6..0000000 --- a/lambdas/wo/email_processor/ses_auth.py +++ /dev/null @@ -1,452 +0,0 @@ -"""Fail-closed SES sender authentication (INFRA-107). - -SES Email Receiving *prepends* its own trace headers -- including an -``Authentication-Results`` header whose authserv-id is ``amazonses.com`` -- -to the top of the raw MIME it writes to S3. Everything below those -prepended headers (the From header, any additional Authentication-Results -copies) is attacker-controlled, so ONLY the topmost Authentication-Results -header is trusted, and only when its authserv-id is ``amazonses.com``. - -An email is accepted only when that header carries ``dkim=pass`` for a -domain in the ``ALLOWED_DKIM_DOMAINS`` allowlist (a comma-separated Lambda -environment variable set by the CDK stack). Every other outcome fails -closed and the email is rejected: - -- ``ALLOWED_DKIM_DOMAINS`` unset or empty -- no Authentication-Results header at all -- topmost header unparseable or from an authserv-id other than SES -- no ``dkim=pass`` clause -- ``dkim=pass`` only for domains outside the allowlist - -Observed SES format (2026-07-15, both ingest buckets):: - - Authentication-Results: amazonses.com; - spf=pass (spfCheck: ...) client-ip=...; envelope-from=...; helo=...; - dkim=pass header.i=@seahaven.com; - dmarc=none header.from=hxgnsmartcloud.com; - -Note SES reports the passing DKIM identity as ``header.i=@`` -(RFC 6376 AUID), not ``header.d=``; the parser accepts both, and treats -``header.d`` (the plain signing domain) as authoritative over ``header.i`` -when both are present. The ``header.i`` domain is derived per RFC 6376 as -the part after the AUID's *last* ``@`` -- a ``@`` inside a quoted -local-part (e.g. ``i="@seahaven.com"@attacker.com``) is signer-controlled -label text, never the identity domain, and yields the true signer -(``attacker.com``). - -**Clause injection defence (INFRA-107 hardening).** SES echoes several -attacker-controlled SMTP-session tokens into its own Authentication-Results -value as their own semicolon-delimited property clauses -- notably -``envelope-from=``, ``helo=`` and ``header.from=``. An RFC 5321 quoted-local-part MAIL FROM may legally contain -spaces and semicolons, e.g.:: - - MAIL FROM:<"x; dkim=pass header.i=@amazon.coupahost.com"@attacker.com> - -which SES renders verbatim as ``envelope-from="x; dkim=pass -header.i=@amazon.coupahost.com"@attacker.com;``. A naive ``split(";")`` -would tear the quoted string apart and manufacture a synthetic -``dkim=pass header.i=@amazon.coupahost.com`` clause out of attacker input. -The parser therefore tokenises per RFC 8601 / RFC 5322 structure: CFWS -comments ``(...)`` are stripped first, and the value is split into clauses -only on semicolons that sit *outside* a quoted-string. A ``;`` inside a -quoted ``pvalue`` stays part of that one property clause and can never be -read as the start of a ``dkim=`` methodspec. -""" - -import email.parser -import email.policy -import json -import logging -import os -import re - -logger = logging.getLogger() - -ALLOWED_DKIM_DOMAINS_ENV = "ALLOWED_DKIM_DOMAINS" -SES_AUTHSERV_ID = "amazonses.com" - -# One resinfo clause of an Authentication-Results value, e.g. -# "dkim=pass header.i=@seahaven.com". The clause must START with the -# method=result pair; header.d= / header.i= may appear anywhere after it. -# The result token must be terminated by end-of-clause or whitespace so -# "dkim=pass-anything" can never be read as "pass". (Comments are stripped -# before matching, so the pre-strip "dkim=pass(comment)" form arrives here -# as "dkim=pass ..." and still terminates on whitespace.) -_DKIM_RESULT_RE = re.compile(r"^dkim\s*=\s*([a-z0-9]+)(?=$|\s)", re.IGNORECASE) - - -def get_allowed_dkim_domains() -> frozenset: - """Read the DKIM-domain allowlist from the environment (may be empty).""" - raw = os.environ.get(ALLOWED_DKIM_DOMAINS_ENV, "") - return frozenset( - d.strip().lower().lstrip("@").rstrip(".") for d in raw.split(",") if d.strip() - ) - - -def _unfold(value: str) -> str: - """Collapse RFC 5322 folding whitespace into single spaces.""" - return re.sub(r"[\r\n\t ]+", " ", value).strip() - - -def _strip_comments(value: str) -> tuple: - """Remove RFC 5322 CFWS comments ``(...)`` from an unfolded header value. - - Comments may nest and may contain quoted pairs (``\\)``). A ``(`` that - appears *inside* a quoted-string is literal text, not a comment start, - so quoted-strings (which carry attacker-controlled ``pvalue`` content - such as a quoted MAIL FROM local part) are passed through untouched. - Dropping comments first means comment-embedded ``header.i=`` / ``;`` - fakes can never influence clause splitting or domain extraction. - - Every removed comment is replaced by a single space so comments act as - folding whitespace (RFC 5322 CFWS semantics). A ``)`` that appears - outside any comment (depth 0) is an unmatched close and makes the value - not well-formed, closing the ``)(`` clause-injection gap. - - CFWS also means a comment abutting a token *ends* that token: - ``header.d=seahaven.com(note)`` reads as domain ``seahaven.com``, where - the pre-CFWS gluing behaviour read ``seahaven.comnote`` (rejected). - This is deliberate -- ``header.d`` is written verbatim by SES from a - *verified* signature's ``d=`` tag, so a comment in that position is - never attacker-controlled. - - Returns ``(stripped_text, well_formed)``. ``well_formed`` is False when - the value ends inside an unterminated comment or quoted-string, or when - an unmatched ``)`` appears at the top level. Callers reject on - ``not well_formed`` so a malformed header (which could otherwise be - mis-tokenised) fails closed rather than being partially parsed. - """ - out = [] - depth = 0 # comment nesting depth - in_quote = False # inside a quoted-string (only tracked at depth 0) - extra_close = False # SES-AR-01: ")" at depth 0 (no matching open) - i = 0 - n = len(value) - while i < n: - c = value[i] - if depth > 0: - # Inside a comment: only quoted-pairs and nested parens matter. - if c == "\\": - i += 2 - continue - if c == "(": - depth += 1 - elif c == ")": - depth -= 1 - if depth == 0: - # SES-AR-02: comment acts as folding whitespace (RFC 5322). - out.append(" ") - i += 1 - continue - if in_quote: - out.append(c) - if c == "\\" and i + 1 < n: - out.append(value[i + 1]) - i += 2 - continue - if c == '"': - in_quote = False - i += 1 - continue - # Normal context (outside any comment or quoted-string). - if c == "(": - depth += 1 - i += 1 - continue - if c == ")": - # SES-AR-01: ")" at depth 0 is an unmatched close -- refuse to - # tokenise rather than letting a ")(...)" pair manufacture a - # gap where attacker text leaks out at depth 0. - extra_close = True - i += 1 - continue - if c == '"': - in_quote = True - out.append(c) - i += 1 - well_formed = depth == 0 and not in_quote and not extra_close - return "".join(out), well_formed - - -def _split_clauses(value: str) -> list: - """Split a comment-free header value into clauses on top-level ``;``. - - A semicolon inside a quoted-string is preserved as part of the clause, - so an attacker-controlled quoted ``pvalue`` (e.g. a quoted MAIL FROM - echoed into ``envelope-from=``) cannot smuggle in a fake ``dkim=pass`` - clause. Callers must run :func:`_strip_comments` first. - """ - clauses = [] - buf = [] - in_quote = False - i = 0 - n = len(value) - while i < n: - c = value[i] - if in_quote: - buf.append(c) - if c == "\\" and i + 1 < n: - buf.append(value[i + 1]) - i += 2 - continue - if c == '"': - in_quote = False - i += 1 - continue - if c == '"': - in_quote = True - buf.append(c) - i += 1 - continue - if c == ";": - clauses.append("".join(buf)) - buf = [] - i += 1 - continue - buf.append(c) - i += 1 - clauses.append("".join(buf)) - return clauses - - -def _split_properties(clause: str) -> list: - """Split a clause into whitespace-separated ``name=pvalue`` tokens. - - Whitespace *inside* a quoted-string does not split, so an RFC 8601 pvalue - that embeds a quoted-string (e.g. a DKIM AUID with a quoted local-part that - legally contains spaces) survives as a single token. This is the same - quoted-string discipline :func:`_split_clauses` applies at the ``;`` level, - reused here at the token level so ``header.d=`` / ``header.i=`` extraction - is quoted-string-aware rather than a naive regex grab. - """ - tokens = [] - buf = [] - in_quote = False - i = 0 - n = len(clause) - while i < n: - c = clause[i] - if in_quote: - buf.append(c) - if c == "\\" and i + 1 < n: - buf.append(clause[i + 1]) - i += 2 - continue - if c == '"': - in_quote = False - i += 1 - continue - if c == '"': - in_quote = True - buf.append(c) - i += 1 - continue - if c.isspace(): - if buf: - tokens.append("".join(buf)) - buf = [] - i += 1 - continue - buf.append(c) - i += 1 - if buf: - tokens.append("".join(buf)) - return tokens - - -def _auid_domain(pvalue: str) -> str: - """Domain of a DKIM AUID (``header.i``) per RFC 6376. - - The identity domain is the part after the *last* ``@`` of the AUID -- but a - ``@`` inside a quoted local-part is NOT the identity separator. So - ``"@seahaven.com"@attacker.com`` yields ``attacker.com`` (the real signer), - not ``seahaven.com``: the ``@seahaven.com`` sits inside the quoted - local-part and is signer-controlled label text, never the domain. A bare - unquoted ``@seahaven.com`` still yields ``seahaven.com``. Returns "" when - there is no top-level ``@`` (no valid domain) or the domain looks malformed. - """ - last_at = -1 - in_quote = False - i = 0 - n = len(pvalue) - while i < n: - c = pvalue[i] - if in_quote: - if c == "\\": - i += 2 - continue - if c == '"': - in_quote = False - i += 1 - continue - if c == '"': - in_quote = True - elif c == "@": - last_at = i - i += 1 - if last_at < 0: - return "" - domain = pvalue[last_at + 1 :].lower().rstrip(".") - # A DKIM domain-name is a plain dot-atom; anything with a residual quote is - # malformed (or a smuggling attempt) and must not be trusted. - if not domain or '"' in domain: - return "" - return domain - - -def _plain_domain(pvalue: str) -> str: - """Domain of ``header.d`` -- the DKIM signing domain, a plain dot-atom. - - SES writes ``header.d`` verbatim from the signature's ``d=`` tag, which is - never a quoted-string. A residual quote means malformed/smuggled input and - fails closed. - """ - domain = pvalue.lower().rstrip(".") - if not domain or '"' in domain: - return "" - return domain - - -def _clause_signer_domain(clause: str) -> str: - """Authoritative DKIM signer domain of a ``dkim=pass`` clause, or "". - - ``header.d`` (the signing domain) is authoritative and is preferred when - present; only when it is absent does this fall back to ``header.i`` and - derive the domain from the AUID's post-final-``@`` part. Both lookups run - over :func:`_split_properties` tokens, so a ``header.d=``/``header.i=`` - literal smuggled *inside* another property's quoted pvalue is confined to - that one token and can never be read as a top-level property. - """ - header_d = None - header_i = None - for token in _split_properties(clause): - name, sep, val = token.partition("=") - if not sep: - continue - key = name.strip().lower() - if key == "header.d" and header_d is None: - header_d = val - elif key == "header.i" and header_i is None: - header_i = val - if header_d is not None: - return _plain_domain(header_d) - if header_i is not None: - return _auid_domain(header_i) - return "" - - -def parse_authentication_results(value: str) -> tuple: - """Parse one Authentication-Results header value. - - Returns ``(authserv_id, passing_dkim_domains)`` where the domains are the - authoritative signer domains of every ``dkim=pass`` clause -- ``header.d`` - when present, else the ``header.i`` AUID's post-final-``@`` domain (see - :func:`_clause_signer_domain`). Malformed input yields ``("", frozenset())``, - which callers treat as a rejection. - - Tokenisation is comment- and quoted-string-aware (RFC 8601 / RFC 5322) at - the ``;`` (clause), whitespace (property) and ``@`` (AUID domain) levels, so - attacker-controlled tokens SES echoes into its header (envelope-from, helo, - header.from) cannot be split into a forged ``dkim=pass`` clause and a quoted - AUID local-part cannot masquerade as the identity domain. - """ - text, well_formed = _strip_comments(_unfold(value)) - if not well_formed: - # Unbalanced quotes/comments: refuse to guess how to tokenise it. - return "", frozenset() - clauses = [c.strip() for c in _split_clauses(text)] - if not clauses or not clauses[0]: - return "", frozenset() - - # First clause is the authserv-id, optionally followed by a version - # token ("amazonses.com 1"); take only the first token. - authserv_id = clauses[0].split()[0].strip('"').lower() - - passing = set() - for clause in clauses[1:]: - match = _DKIM_RESULT_RE.match(clause) - if not match or match.group(1).lower() != "pass": - continue - domain = _clause_signer_domain(clause) - if domain: - passing.add(domain) - return authserv_id, frozenset(passing) - - -def evaluate_sender_authentication(raw_email: bytes, allowed_domains) -> tuple: - """Evaluate the SES-stamped verdicts in a raw MIME message. - - Returns ``(accepted, reason, detail)``. Pure function of its inputs so - it can be unit-tested without touching the environment. - """ - if not allowed_domains: - return False, "allowlist_not_configured", {} - - try: - # compat32 keeps header values as raw strings (we unfold ourselves) - # and never raises on structurally odd headers; headersonly avoids - # parsing the body at all. - msg = email.parser.BytesParser(policy=email.policy.compat32).parsebytes( - raw_email, headersonly=True - ) - except Exception: - return False, "unparseable_message", {} - - ar_headers = msg.get_all("Authentication-Results") or [] - if not ar_headers: - return False, "authentication_results_missing", {} - - # SES prepends its trace headers, so index 0 is the SES-stamped copy. - # Any Authentication-Results header further down arrived inside the - # message (attacker-suppliable) and is deliberately ignored. - # - # The parse is wrapped so an unexpected parser exception fails CLOSED - # (rejected, structured reason) instead of propagating out of the handler - # into Lambda's async retries / DLQ on attacker-crafted input. - try: - authserv_id, passing = parse_authentication_results(str(ar_headers[0])) - except Exception: - return False, "authentication_results_unparseable", {} - detail = { - "authserv_id": authserv_id, - "passing_dkim_domains": sorted(passing), - } - - if authserv_id != SES_AUTHSERV_ID: - return False, "untrusted_authserv_id", detail - if not passing: - return False, "no_passing_dkim_signature", detail - - matched = passing & set(allowed_domains) - if not matched: - return False, "dkim_domain_not_allowlisted", detail - - detail["matched_domains"] = sorted(matched) - return True, "authenticated", detail - - -def authenticate_inbound_email(raw_email: bytes, s3_key: str) -> bool: - """Fail-closed gate used by the S3-triggered handlers. - - On rejection: logs a structured warning with the reason and S3 key and - returns False. Callers skip the message and return normally, so - rejected mail never errors the invocation (no retries, no DLQ spam). - """ - allowed = get_allowed_dkim_domains() - accepted, reason, detail = evaluate_sender_authentication(raw_email, allowed) - if accepted: - logger.info(json.dumps({"event": "sender_auth_ok", "s3_key": s3_key, **detail})) - return True - logger.warning( - json.dumps( - { - "event": "sender_auth_rejected", - "reason": reason, - "s3_key": s3_key, - "allowed_dkim_domains": sorted(allowed), - **detail, - } - ) - ) - return False diff --git a/lambdas/wo/email_processor/tests/_wo_parser_support.py b/lambdas/wo/email_processor/tests/_wo_parser_support.py index 2aa22f5..9c00bbf 100644 --- a/lambdas/wo/email_processor/tests/_wo_parser_support.py +++ b/lambdas/wo/email_processor/tests/_wo_parser_support.py @@ -16,6 +16,14 @@ _HERE = os.path.dirname(__file__) _MODULE_DIR = os.path.abspath(os.path.join(_HERE, "..")) if _MODULE_DIR not in sys.path: sys.path.insert(0, _MODULE_DIR) +# Phase 3: handler.py now imports ses_auth/email_parsing/emf, single-sourced +# under lambdas/shared/ (no longer in _MODULE_DIR). Put shared/ on sys.path so +# the handler's bare `from email_parsing import ...` etc. resolve. These modules +# are single-copy, so bare-name caching is correct for both pipelines -- no +# collision guard needed (that only matters for the duplicated template_parser). +_SHARED_DIR = os.path.abspath(os.path.join(_HERE, "..", "..", "..", "shared")) +if _SHARED_DIR not in sys.path: + sys.path.insert(0, _SHARED_DIR) os.environ.setdefault("AWS_DEFAULT_REGION", "us-east-1") FIXTURES = os.path.join(_HERE, "fixtures") diff --git a/lambdas/wo/web_ui/handler.py b/lambdas/wo/web_ui/handler.py index 3e3c849..8ab9943 100644 --- a/lambdas/wo/web_ui/handler.py +++ b/lambdas/wo/web_ui/handler.py @@ -5,14 +5,13 @@ Serves a simple HTML dashboard for viewing work orders and comments. Accessed via Lambda Function URL. """ -import hmac import json import logging import os -import time from html import escape as esc import boto3 +from web_ui_auth import is_authenticated logger = logging.getLogger() logger.setLevel(logging.INFO) @@ -29,73 +28,6 @@ COMMENTS_TABLE = os.environ.get("COMMENTS_TABLE", "WorkOrderComments") # fetched at runtime from Secrets Manager to keep it out of CloudFormation # templates and Lambda env vars. If the ARN is unset or the secret is missing, the # handler fails closed and denies every request. -_WEB_UI_AUTH_TOKEN_SECRET_ARN = os.environ.get("WEB_UI_AUTH_TOKEN_SECRET_ARN") -# Refresh the cached token this often so a rotated secret propagates without -# waiting for the execution environment to recycle (emergency-rotation path). -_AUTH_TOKEN_CACHE_TTL_SECONDS = 300 -_auth_token_cache = None -_auth_token_cached_at = 0.0 - - -def _get_auth_token() -> str | None: - """Fetch the shared web UI auth token from Secrets Manager. - - Cached in the warm container for a short TTL so we don't hit Secrets Manager - on every request, while still picking up a rotated secret within the TTL - rather than only when the execution environment recycles. Returns None when - not configured or unreadable (the caller then fails closed). - """ - global _auth_token_cache, _auth_token_cached_at - now = time.monotonic() - if ( - _auth_token_cache is not None - and now - _auth_token_cached_at < _AUTH_TOKEN_CACHE_TTL_SECONDS - ): - return _auth_token_cache - if not _WEB_UI_AUTH_TOKEN_SECRET_ARN: - return None - secrets = boto3.client("secretsmanager") - try: - secret = secrets.get_secret_value(SecretId=_WEB_UI_AUTH_TOKEN_SECRET_ARN) - _auth_token_cache = secret["SecretString"] - _auth_token_cached_at = now - return _auth_token_cache - except Exception: - # Fail closed (return None -> caller 401s) but surface the failure: a - # Secrets Manager permission/config error would otherwise make every - # request 401 with no operational signal. The secret value is never - # logged. - logger.exception( - "Failed to fetch web UI auth token from Secrets Manager; " - "denying request (failing closed)" - ) - return None - - -def _header(event: dict, name: str) -> str: - """Case-insensitive header lookup from a Lambda Function URL / APIGW event.""" - headers = event.get("headers") or {} - name_lower = name.lower() - for key, value in headers.items(): - if key.lower() == name_lower: - return value or "" - return "" - - -def is_authenticated(event: dict) -> bool: - """Constant-time check of the request's shared secret against the configured - token. Fails closed when no token is configured.""" - token = _get_auth_token() - if not token: - return False - presented = _header(event, "x-auth-token") - if not presented: - auth = _header(event, "authorization") - if auth.lower().startswith("bearer "): - presented = auth[7:].strip() - if not presented: - return False - return hmac.compare_digest(presented, token) def get_work_orders(limit=500): diff --git a/tests/conftest.py b/tests/conftest.py index 48407f9..7568298 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -19,15 +19,25 @@ os.environ.setdefault("AWS_SECRET_ACCESS_KEY", "testing") os.environ.setdefault("AWS_SESSION_TOKEN", "testing") REPO_ROOT = Path(__file__).resolve().parents[1] +_SHARED_DIR = REPO_ROOT / "lambdas" / "shared" -# Sibling modules that exist per-pipeline and are imported by bare name from -# the handlers (the Lambda runtime puts each function's own directory on -# sys.path). Both pipelines duplicate these filenames, so the bare names MUST -# be bound to the right pipeline's file around each handler exec -- relying on -# sys.path ordering (or on whatever a previously collected suite left in -# sys.modules) silently binds a handler to the OTHER pipeline's sibling. -_SIBLING_MODULES = ("ses_auth", "template_parser", "derived_fields") +# Sibling modules imported by bare name from the handlers (the Lambda runtime +# puts each function's own directory on sys.path; the CDK bundling then cp's the +# shared modules in flat beside handler.py so those bare imports resolve too). +# template_parser/derived_fields are duplicated PER PIPELINE, so their bare +# names MUST be bound to the right pipeline's file around each handler exec -- +# relying on sys.path ordering (or on whatever a previously collected suite left +# in sys.modules) silently binds a handler to the OTHER pipeline's sibling. +# ses_auth/email_parsing/emf are now single-sourced under lambdas/shared/ (Phase +# 3); the loop below resolves them from there via a shared-dir fallback. +_SIBLING_MODULES = ( + "ses_auth", + "template_parser", + "derived_fields", + "email_parsing", + "emf", +) def _load_module(path, module_name): @@ -69,7 +79,14 @@ def load_handler(relative_path, module_name): return _load_module(path, module_name) saved = {} for sibling in _SIBLING_MODULES: + # Per-pipeline siblings (template_parser/derived_fields) resolve next to + # the handler; the shared, single-sourced siblings (ses_auth/ + # email_parsing/emf) fall back to lambdas/shared/. No ambiguity: post + # Phase 3 the shared names exist ONLY under shared/, the per-pipeline + # names ONLY next to the handler. sibling_path = path.parent / f"{sibling}.py" + if not sibling_path.exists(): + sibling_path = _SHARED_DIR / f"{sibling}.py" if not sibling_path.exists(): continue saved[sibling] = sys.modules.get(sibling) @@ -109,11 +126,12 @@ def email_handler(request): return request.getfixturevalue(request.param) -@pytest.fixture(params=["wo", "po"]) -def ses_auth(request): - """The ses_auth module of each pipeline (duplicated file, kept in sync).""" - pipeline = request.param - return load_handler( - f"lambdas/{pipeline}/email_processor/ses_auth.py", - f"{pipeline}_ses_auth", - ) +@pytest.fixture +def ses_auth(): + """The single-sourced ses_auth module (lambdas/shared/, Phase 3). + + Previously parameterized over the two per-pipeline copies to prove they + stayed byte-identical; now there is exactly one copy, so this loads it + once -- halving the test_ses_auth run. + """ + return load_handler("lambdas/shared/ses_auth.py", "shared_ses_auth") diff --git a/tests/test_bundle_consistency.py b/tests/test_bundle_consistency.py index b76d042..2f1e3a7 100644 --- a/tests/test_bundle_consistency.py +++ b/tests/test_bundle_consistency.py @@ -23,16 +23,45 @@ PO_HANDLER = REPO_ROOT / "lambdas" / "po" / "email_processor" / "handler.py" WO_HANDLER = REPO_ROOT / "lambdas" / "wo" / "email_processor" / "handler.py" PO_STACK = REPO_ROOT / "cdk" / "po_stack.py" WO_STACK = REPO_ROOT / "cdk" / "wo_stack.py" +# Phase 3: ses_auth/web_ui_auth/email_parsing/emf are single-sourced here and +# shipped into the email-processor bundles via `cp shared/*.py`. +SHARED_DIR = REPO_ROOT / "lambdas" / "shared" -# The complete set of top-level modules the PO email_processor ships via the -# non-recursive `cp ./*.py` glob. Pinned as an upper bound: a new top-level -# .py in that dir must be added here deliberately, which is the moment to -# decide whether it SHOULD ship (a real module) or must be excluded (a scratch -# or secrets file that from_asset would otherwise stage into the bundle on a -# local deploy). See test_po_bundle_ships_no_unexpected_top_level_modules. -PO_EXPECTED_TOP_LEVEL_MODULES = frozenset( - {"handler", "ses_auth", "template_parser", "derived_fields"} -) +# The names Phase 3 single-sourced under lambdas/shared/. After the move NONE +# of these may reappear as a top-level .py in either email-processor pipeline +# dir: every handler loader resolves a pipeline-local copy FIRST +# (tests/conftest.py load_handler checks path.parent before _SHARED_DIR; +# lambdas/wo/email_processor/tests/_wo_parser_support.py inserts the pipeline +# dir ahead of shared/ on sys.path), so a stray reappearance would silently +# SHADOW the single shared source in every handler-loaded test while +# test_ses_auth / test_parse_raw_email keep exercising shared/ -- divergence +# undetected. This is exactly the future-drift invariant the retired +# byte-identity ses_auth fixture used to guard; it is now enforced by policing +# the pipeline dirs and shared/ SEPARATELY (never as a union, which would let +# the same name already expected in shared/ mask a pipeline-dir stray) -- +# see test_no_shared_module_shadow_in_pipeline_dirs and the per-dir exact-set +# pins below. +SHARED_MODULES = frozenset({"ses_auth", "email_parsing", "emf", "web_ui_auth"}) + +# Exact top-level .py stems each dir must hold. Pinned as exact sets (both +# bounds): the bundling globs (`cp /email_processor/*.py` + +# `cp shared/*.py`) ship every top-level .py in these dirs, and +# `Code.from_asset` stages untracked files too (it does not honor .gitignore), +# so a stray scratch/secrets .py -- or a shadow copy of a shared module -- would +# silently ship into the production zip on a local deploy. Any new top-level +# module must be added here deliberately, the moment to decide whether it SHOULD +# ship (a real module) or must be excluded. +PO_PIPELINE_MODULES = frozenset({"handler", "template_parser", "derived_fields"}) +WO_PIPELINE_MODULES = frozenset({"__init__", "handler", "template_parser"}) + +# Full shipped set of each email-processor bundle (pipeline glob + shared glob). +# Derived from the per-dir pins above so it stays consistent with them; policed +# per-dir (NOT via this union) so a shared-name shadow in a pipeline dir cannot +# be masked. web_ui_auth is a deliberate, harmless ride-along of the pinned +# `cp shared/*.py` (constraint #8) -- never imported by the email handlers, but +# it ships, so it is part of the shipped set. +PO_EXPECTED_TOP_LEVEL_MODULES = PO_PIPELINE_MODULES | SHARED_MODULES +WO_EXPECTED_TOP_LEVEL_MODULES = WO_PIPELINE_MODULES | SHARED_MODULES # Pipeline-scoped glob shapes the per-stack ships-all pins accept. Phase 2 # widened the bundling cwd to the shared ../lambdas asset root, so the executed @@ -47,6 +76,22 @@ PO_EXPECTED_TOP_LEVEL_MODULES = frozenset( PO_SCOPED_GLOB_RE = r"(?:^|\s)(?:\./|po/email_processor/)?\*\.py(?:\s|$)" WO_SCOPED_GLOB_RE = r"(?:^|\s)(?:\./|wo/email_processor/)?\*\.py(?:\s|$)" +# Phase 3: both email-processor bundles gain a second glob copying the +# single-sourced shared modules flat into the zip. This must be the EXECUTED +# form (`_executed_cp_commands` strips comments), so a commented-out shared cp +# cannot false-pass the pin. +SHARED_CP_RE = r"(?:^|\s)shared/\*\.py(?:\s|$)" + +# Phase 3: each stack's web_ui function stages ONLY shared/web_ui_auth.py flat +# beside its handler (NOT all of shared/ -- the email-only modules must not +# bloat the web UI zip). The auth-gated web_ui handlers do a module-top-level +# `from web_ui_auth import is_authenticated`, so dropping/commenting this cp +# ImportErrors the Lambda at cold start with green CI -- the PR #105 ImportError +# class the AST test exists to prevent, but for a surface (web_ui) the +# email-processor selector deliberately excludes. Checked as the EXECUTED form +# so a commented-out cp cannot false-pass. +WEB_UI_AUTH_CP_RE = r"(?:^|\s)shared/web_ui_auth\.py(?:\s|$)" + def _first_party_sibling_imports(handler_path: Path) -> set[str]: """Top-level module names handler.py imports that are first-party siblings. @@ -67,7 +112,16 @@ def _first_party_sibling_imports(handler_path: Path) -> set[str]: if node.module: names.add(node.module.split(".")[0]) sibling_dir = handler_path.parent - return {name for name in names if (sibling_dir / f"{name}.py").exists()} + # A first-party sibling resolves either next to the handler (per-pipeline: + # template_parser/derived_fields) or under lambdas/shared/ (single-sourced: + # ses_auth/email_parsing/emf, Phase 3). Both must count, or the ships-all + # pin would silently drop the shared siblings (ses_auth was silently + # dropped, email_parsing/emf never seen, before this resolved shared/). + return { + name + for name in names + if (sibling_dir / f"{name}.py").exists() or (SHARED_DIR / f"{name}.py").exists() + } def _extract_bundling_command(stack_path: Path) -> str: @@ -79,11 +133,13 @@ def _extract_bundling_command(stack_path: Path) -> str: lines, with `#` comments interspersed) is folded by the parser itself, so this returns the exact single command string CDK will hand to bash. - Each stack currently has exactly one bundled function; if a second one - is ever added, "the" bundling command becomes ambiguous and every - assertion in this file needs to pick its target explicitly — so demand - exactly one match rather than silently returning whichever ast.walk - happens to visit first. + Since Phase 3 each stack has TWO bundled functions -- the email processor + and the web_ui function (which now also stages a shared module). "The" + bundling command this test cares about is the EMAIL-processor one, so we + select the command whose text mentions ``email_processor`` (its first cp + is ``cp /email_processor/*.py``) and demand exactly one match -- + the web_ui command (``cp -r /web_ui/.``) and site_extractor do + not match. """ tree = ast.parse(stack_path.read_text()) commands: list[str] = [] @@ -94,13 +150,47 @@ def _extract_bundling_command(stack_path: Path) -> str: last = list_node.elts[-1] if isinstance(last, ast.Constant) and isinstance(last.value, str): commands.append(last.value) - if len(commands) != 1: + email_cmds = [c for c in commands if "email_processor" in c] + if len(email_cmds) != 1: raise AssertionError( - f"expected exactly one bundling command=[...] list in {stack_path}, " - f"found {len(commands)} — update this test to select the intended " - "bundling command explicitly" + f"expected exactly one email-processor bundling command=[...] list in " + f"{stack_path}, found {len(email_cmds)} (of {len(commands)} total " + "command lists) — update this test to select the intended bundling " + "command explicitly" ) - return commands[0] + return email_cmds[0] + + +def _extract_web_ui_command(stack_path: Path) -> str: + """Extract the web_ui function's bash -c bundling command string. + + Mirrors _extract_bundling_command but selects the command whose text + mentions ``web_ui`` (its first cp is ``cp -r /web_ui/.``). The + email-processor command (``cp /email_processor/*.py``, plus + ``cp shared/*.py``) and site_extractor do not contain ``web_ui`` in the + folded command STRING (the explanatory ``# ... web_ui_auth ...`` comments + around the email command are Python comments, not string content, so they + are not part of the folded literal). Demands exactly one match so an + ambiguity surfaces loudly instead of silently checking the wrong command. + """ + tree = ast.parse(stack_path.read_text()) + commands: list[str] = [] + for node in ast.walk(tree): + if isinstance(node, ast.keyword) and node.arg == "command": + list_node = node.value + if isinstance(list_node, ast.List) and list_node.elts: + last = list_node.elts[-1] + if isinstance(last, ast.Constant) and isinstance(last.value, str): + commands.append(last.value) + web_ui_cmds = [c for c in commands if "web_ui" in c] + if len(web_ui_cmds) != 1: + raise AssertionError( + f"expected exactly one web_ui bundling command=[...] list in " + f"{stack_path}, found {len(web_ui_cmds)} (of {len(commands)} total " + "command lists) — update this test to select the intended bundling " + "command explicitly" + ) + return web_ui_cmds[0] def _executed_cp_commands(command: str) -> list[str]: @@ -127,39 +217,38 @@ def _bundling_ships_all(command: str, sibling_names: set[str]) -> bool: """True if `command` is guaranteed to ship every name in `sibling_names`. Inspects only the `cp` commands bash actually executes (comments stripped - via `_executed_cp_commands`). An executed copy ships every top-level .py - sibling unconditionally in two shapes: - - a non-recursive glob copy whose (optional) path prefix resolves, under - the ../lambdas bundling cwd, to a directory that ACTUALLY contains - every required sibling, e.g. `cp po/email_processor/*.py /asset-output/` - (PO, Phase 2). The directory is filesystem-checked, not merely - pattern-matched: a wrong-pipeline or arbitrary-directory glob - (`cp wo/email_processor/*.py` in po_stack, `cp venv/lib/*.py`) does NOT - qualify, because that directory does not hold every required sibling -- - so it can never short-circuit to True while dropping a module; + via `_executed_cp_commands`) and ACCUMULATES the set of shipped module + stems across ALL of them -- because since Phase 3 the required siblings + ship via TWO globs, not one: `cp /email_processor/*.py` covers + the per-pipeline siblings and `cp shared/*.py` covers the single-sourced + ones (ses_auth/email_parsing/emf). A single-cp "one glob ships everything" + check would wrongly report False now that coverage is split. + + Each executed cp contributes to the shipped set as follows: + - a non-recursive `*.py` glob ships every top-level .py in the globbed + directory -- but ONLY that directory. Its (optional) path prefix is + resolved against the ../lambdas asset root (the Phase 2 bundling cwd) + and every `.py` stem actually present there is added. A wrong-pipeline + or arbitrary-directory glob (`cp wo/email_processor/*.py` in po_stack, + `cp venv/lib/*.py`) therefore contributes only what that dir really + holds (or nothing, if it does not exist) and can never cover a sibling + that lives elsewhere; - a recursive copy of the WHOLE source dir, e.g. `cp -r . /asset-output/` - -- the source operand must be `.`/`./`, so a narrowed recursive - copy like `cp -r ./package /asset-output/` does NOT qualify. - Any other executed `cp` is treated as an explicit filename allowlist (the - legacy PO form) and is safe only if every required `.py` literally - appears among the copied filenames -- the branch that must reject a - reverted allowlist missing a sibling. + (`.`/`./` source only), ships everything -> short-circuit True; + - any other executed `cp` is an explicit filename allowlist (the legacy + PO form): each copied `.py` stem is added, so a reverted + allowlist missing a sibling leaves that sibling out of the set. + Returns True only if every required sibling ended up in the accumulated set. """ cp_cmds = _executed_cp_commands(command) if not cp_cmds: return False - copied_files: set[str] = set() + shipped: set[str] = set() for cp in cp_cmds: glob_match = re.search(r"(?:^|\s)((?:[\w./-]+/)?)\*\.py(?:\s|$)", cp) if glob_match: - # A `*.py` glob ships every top-level .py in the globbed directory - # -- but ONLY that directory. Resolve its prefix against the - # ../lambdas asset root (the Phase 2 bundling cwd) and confirm it - # actually holds every required sibling, so a wrong-pipeline glob - # cannot pass the ships-all check on the mere presence of a `*.py`. glob_dir = (REPO_ROOT / "lambdas" / glob_match.group(1)).resolve() - if all((glob_dir / f"{name}.py").exists() for name in sibling_names): - return True + shipped.update(p.stem for p in glob_dir.glob("*.py")) continue if re.search(r"cp\s+-r\s+\.\/?\s+/asset-output", cp): return True @@ -169,8 +258,8 @@ def _bundling_ships_all(command: str, sibling_names: set[str]) -> bool: r"cp\s+(?:-\S+\s+)*(.*?)\s*/asset-output/?\"?\s*$", cp.strip() ) if match: - copied_files.update(match.group(1).split()) - return all(f"{name}.py" in copied_files for name in sibling_names) + shipped.update(Path(f).stem for f in match.group(1).split()) + return all(name in shipped for name in sibling_names) def test_po_bundling_ships_all_first_party_siblings(): @@ -194,6 +283,16 @@ def test_po_bundling_ships_all_first_party_siblings(): "arbitrary-directory glob is rejected here on purpose. " f"Executed cp commands: {executed_cps}" ) + # Phase 3: the shared siblings (ses_auth/email_parsing/emf) ship via a + # SECOND executed glob, `cp shared/*.py /asset-output/`. Require it to be + # actually executed (comment-stripped), so commenting it out fails here; + # combined with ships-all below (those siblings resolve only under shared/) + # a removed/commented shared cp fails BOTH assertions. + assert any(re.search(SHARED_CP_RE, cp) for cp in executed_cps), ( + "cdk/po_stack.py email-processor bundling command must EXECUTE " + "'cp shared/*.py /asset-output/' so the single-sourced shared modules " + f"ship flat beside handler.py. Executed cp commands: {executed_cps}" + ) assert _bundling_ships_all(command, siblings), ( f"cdk/po_stack.py bundling command does not ship all of {sorted(siblings)}: " f"{command!r}" @@ -221,36 +320,115 @@ def test_wo_bundling_ships_all_first_party_siblings(): "wrong-pipeline or arbitrary-directory glob is rejected here on purpose. " f"Executed cp commands: {executed_cps}" ) + # Phase 3: shared siblings ship via the second executed glob `cp shared/*.py`. + assert any(re.search(SHARED_CP_RE, cp) for cp in executed_cps), ( + "cdk/wo_stack.py email-processor bundling command must EXECUTE " + "'cp shared/*.py /asset-output/' so the single-sourced shared modules " + f"ship flat beside handler.py. Executed cp commands: {executed_cps}" + ) assert _bundling_ships_all(command, siblings), ( f"cdk/wo_stack.py bundling command does not ship all of {sorted(siblings)}: " f"{command!r}" ) -def test_po_bundle_ships_no_unexpected_top_level_modules(): - """Upper bound: the PO glob ships EXACTLY the expected top-level modules. +def test_po_email_processor_dir_ships_no_unexpected_top_level_modules(): + """Upper bound on the PO pipeline dir alone (NOT unioned with shared/). The sibling-import tests above prove the glob ships every module the - handler needs (shipped >= required). This proves the other direction - (shipped <= expected): because `cp ./*.py` copies every top-level .py in - the dir -- and `Code.from_asset` stages untracked files too (it does not - honor .gitignore) -- a stray scratch/secrets .py left in this dir would - silently ship into the production zip on a local deploy. Pinning the set - forces any new top-level module to be added to - PO_EXPECTED_TOP_LEVEL_MODULES deliberately, at which point the author - decides whether it should ship or be excluded from bundling. + handler needs (shipped >= required). This proves the other direction for + the pipeline dir (shipped <= expected): because `cp po/email_processor/*.py` + copies every top-level .py in that dir -- and `Code.from_asset` stages + untracked files too (it does not honor .gitignore) -- a stray scratch or + secrets .py, OR a shadow copy of a moved shared module, would silently ship + into the zip on a local deploy. Policing this dir SEPARATELY from shared/ + (rather than as a union with it) is what makes a strayed-back ses_auth.py / + email_parsing.py / emf.py FAIL here instead of being masked by the same name + already being expected in shared/. """ top_level = {p.stem for p in PO_HANDLER.parent.glob("*.py")} - assert top_level == set(PO_EXPECTED_TOP_LEVEL_MODULES), ( + assert top_level == set(PO_PIPELINE_MODULES), ( "unexpected top-level .py set in lambdas/po/email_processor -- the " - "'cp ./*.py' glob would ship exactly these into the Lambda zip. " - f"Found {sorted(top_level)}, expected " - f"{sorted(PO_EXPECTED_TOP_LEVEL_MODULES)}. If a new module is " - "intended, add it to PO_EXPECTED_TOP_LEVEL_MODULES; if it is a scratch " - "or secrets file, remove it (or exclude it from bundling) before deploy." + "'cp po/email_processor/*.py' glob would ship exactly these into the " + f"Lambda zip. Found {sorted(top_level)}, expected " + f"{sorted(PO_PIPELINE_MODULES)}. A moved shared module " + f"({sorted(SHARED_MODULES)}) reappearing here would SHADOW the single " + "shared source in every handler-loaded test -- remove it. If a new " + "per-pipeline module is intended, add it to PO_PIPELINE_MODULES." ) +def test_wo_email_processor_dir_ships_no_unexpected_top_level_modules(): + """Upper bound on the WO pipeline dir alone (NOT unioned with shared/). + + The WO equivalent of the PO exact-set pin -- previously MISSING entirely, + so a stray .py (or a shadow copy of a moved shared module) in + lambdas/wo/email_processor was policed by nothing. Same rationale as the PO + pin: `cp wo/email_processor/*.py` ships every top-level .py in this dir, and + a strayed-back ses_auth/email_parsing/emf would shadow the shared source in + the WO handler-loaded tests. + """ + top_level = {p.stem for p in WO_HANDLER.parent.glob("*.py")} + assert top_level == set(WO_PIPELINE_MODULES), ( + "unexpected top-level .py set in lambdas/wo/email_processor -- the " + "'cp wo/email_processor/*.py' glob would ship exactly these into the " + f"Lambda zip. Found {sorted(top_level)}, expected " + f"{sorted(WO_PIPELINE_MODULES)}. A moved shared module " + f"({sorted(SHARED_MODULES)}) reappearing here would SHADOW the single " + "shared source in every handler-loaded test -- remove it. If a new " + "per-pipeline module is intended, add it to WO_PIPELINE_MODULES." + ) + + +def test_shared_dir_ships_no_unexpected_top_level_modules(): + """Upper bound on lambdas/shared/ alone (NOT unioned with a pipeline dir). + + `cp shared/*.py` ships every top-level .py under lambdas/shared/ into BOTH + email-processor bundles, so a stray scratch/secrets .py here would leak into + both production zips. Pinned to exactly the four single-sourced modules. + """ + top_level = {p.stem for p in SHARED_DIR.glob("*.py")} + assert top_level == set(SHARED_MODULES), ( + "unexpected top-level .py set in lambdas/shared -- the 'cp shared/*.py' " + "glob would ship exactly these into BOTH email-processor Lambda zips. " + f"Found {sorted(top_level)}, expected {sorted(SHARED_MODULES)}. If a new " + "shared module is intended, add it to SHARED_MODULES; if it is a scratch " + "or secrets file, remove it before deploy." + ) + + +def test_no_shared_module_shadow_in_pipeline_dirs(): + """The moved shared names must live ONLY under lambdas/shared/. + + Replaces the future-drift guard the retired byte-identity ses_auth fixture + used to provide. A stray reappearance of a moved shared module in either + email-processor pipeline dir would be resolved FIRST by every handler loader + -- tests/conftest.py load_handler checks `path.parent / f"{sibling}.py"` + before the _SHARED_DIR fallback, and + lambdas/wo/email_processor/tests/_wo_parser_support.py inserts the pipeline + dir ahead of shared/ on sys.path -- silently SHADOWING the single shared + source in every handler-loaded test, while test_ses_auth / test_parse_raw_email + keep exercising shared/. Divergence would go undetected. Police the pipeline + dirs and shared/ SEPARATELY so no union can mask the shadow. + """ + for name in sorted(SHARED_MODULES): + assert (SHARED_DIR / f"{name}.py").exists(), ( + f"{name}.py must exist under lambdas/shared/ (single source of truth)" + ) + assert not (PO_HANDLER.parent / f"{name}.py").exists(), ( + f"{name}.py reappeared in lambdas/po/email_processor -- it would " + "SHADOW lambdas/shared/{name}.py in every PO handler-loaded test " + "(the loader resolves the pipeline-local copy first). Delete it; " + "the single source lives under lambdas/shared/." + ) + assert not (WO_HANDLER.parent / f"{name}.py").exists(), ( + f"{name}.py reappeared in lambdas/wo/email_processor -- it would " + "SHADOW lambdas/shared/{name}.py in every WO handler-loaded test " + "(_wo_parser_support inserts the pipeline dir ahead of shared/ on " + "sys.path). Delete it; the single source lives under lambdas/shared/." + ) + + def test_detection_logic_catches_allowlist_missing_a_sibling(): """Unit-level check on `_bundling_ships_all` itself. @@ -272,13 +450,15 @@ def test_detection_logic_catches_allowlist_missing_a_sibling(): assert not _bundling_ships_all(reverted_allowlist_command, siblings) - # Control: the same allowlist shape WITH derived_fields.py added back in - # is correctly recognized as complete, proving the failure above is - # about the missing file and not a false-positive-prone regex. + # Control: the same allowlist shape listing ALL required siblings (including + # the Phase 3 shared ones -- derived_fields, email_parsing, emf added back) + # is correctly recognized as complete under the accumulate model, proving + # the failure above is about the missing files and not a regex artifact. complete_allowlist_command = ( "pip install --platform manylinux2014_aarch64 --only-binary=:all: " "-r requirements.txt -t /asset-output && " - "cp handler.py ses_auth.py template_parser.py derived_fields.py /asset-output/" + "cp handler.py ses_auth.py template_parser.py derived_fields.py " + "email_parsing.py emf.py /asset-output/" ) assert _bundling_ships_all(complete_allowlist_command, siblings) @@ -325,6 +505,26 @@ def test_detection_logic_rejects_commented_out_scoped_glob(): assert not _bundling_ships_all(commented_out_command, siblings) +def test_detection_logic_rejects_commented_out_shared_cp(): + """A `cp shared/*.py` surviving only in a `#` comment must not count as run. + + Simulates a half-finished revert that drops the executed shared cp but + leaves its text trailing as a comment. `_executed_cp_commands` strips `#` + comments before any regex sees the segment, so the shared-cp text alone -- + never executed by bash -- is not among the executed cp's. Combined with the + fixed ships-all (ses_auth/email_parsing/emf resolve ONLY under shared/), a + removed or commented shared cp fails both the SHARED_CP_RE pin and ships-all. + """ + commented_out_command = ( + "cp po/email_processor/*.py /asset-output/ # cp shared/*.py /asset-output/" + ) + executed = _executed_cp_commands(commented_out_command) + assert not any(re.search(SHARED_CP_RE, cp) for cp in executed) + # And ships-all is False: the shared siblings never got shipped. + po_siblings = _first_party_sibling_imports(PO_HANDLER) + assert not _bundling_ships_all(commented_out_command, po_siblings) + + def test_detection_logic_rejects_wrong_pipeline_glob(): """A glob pointing at the WRONG pipeline (or any other dir) must not pass. @@ -361,3 +561,58 @@ def test_detection_logic_rejects_wrong_pipeline_glob(): assert not re.search(PO_SCOPED_GLOB_RE, "cp wo/email_processor/*.py /asset-output/") assert re.search(WO_SCOPED_GLOB_RE, "cp wo/email_processor/*.py /asset-output/") assert not re.search(WO_SCOPED_GLOB_RE, "cp po/email_processor/*.py /asset-output/") + + +def test_po_web_ui_bundle_stages_shared_auth_module(): + """The PO web_ui bundle must EXECUTE `cp shared/web_ui_auth.py`. + + Phase 3 removed the inline auth block from lambdas/po/web_ui/handler.py, + which now does a module-top-level `from web_ui_auth import is_authenticated`; + web_ui_auth.py lives ONLY under lambdas/shared/. No test imports the web_ui + handler, and the email-processor AST pins deliberately exclude the web_ui + command -- so without this pin, dropping/commenting the web_ui cp would + ImportError the auth-gated Lambda at cold start with green CI. Checked + against the comment-stripped executed cp so the old text surviving only in a + comment cannot satisfy it. + """ + command = _extract_web_ui_command(PO_STACK) + executed_cps = _executed_cp_commands(command) + assert any(re.search(WEB_UI_AUTH_CP_RE, cp) for cp in executed_cps), ( + "cdk/po_stack.py web_ui bundling command must EXECUTE " + "'cp shared/web_ui_auth.py /asset-output/' so the shared auth module " + "ships flat beside handler.py and `from web_ui_auth import " + f"is_authenticated` resolves at cold start. Executed cp commands: " + f"{executed_cps}" + ) + + +def test_wo_web_ui_bundle_stages_shared_auth_module(): + """The WO web_ui bundle must EXECUTE `cp shared/web_ui_auth.py`. + + WO equivalent of test_po_web_ui_bundle_stages_shared_auth_module -- same + ImportError-at-cold-start hazard for lambdas/wo/web_ui/handler.py. + """ + command = _extract_web_ui_command(WO_STACK) + executed_cps = _executed_cp_commands(command) + assert any(re.search(WEB_UI_AUTH_CP_RE, cp) for cp in executed_cps), ( + "cdk/wo_stack.py web_ui bundling command must EXECUTE " + "'cp shared/web_ui_auth.py /asset-output/' so the shared auth module " + "ships flat beside handler.py and `from web_ui_auth import " + f"is_authenticated` resolves at cold start. Executed cp commands: " + f"{executed_cps}" + ) + + +def test_detection_logic_rejects_commented_out_web_ui_auth_cp(): + """A `cp shared/web_ui_auth.py` surviving only in a `#` comment must not pass. + + Mirror of test_detection_logic_rejects_commented_out_shared_cp for the + web_ui staging: a half-finished revert that drops the executed cp but leaves + its text trailing as a comment must NOT register as executed, since bash + would never run it. + """ + commented_out_command = ( + "cp -r po/web_ui/. /asset-output/ # cp shared/web_ui_auth.py /asset-output/" + ) + executed = _executed_cp_commands(commented_out_command) + assert not any(re.search(WEB_UI_AUTH_CP_RE, cp) for cp in executed)