fix(logging): redact email verification tokens from access logs

RM
2d ago
+223-46

Reviewed CI corrections — November 7

Current head: a56a386e50dbda7b8cc5e21528cd17b963c0a88d. This follow-up repairs the inherited resolver setup, missing mock-plugin registration and stale import inventory. It also fixes two reproduced AuthNZ pool lifecycles: bootstrap test readback and startup override refresh now use the registered pool after seeding replaces it. Both startup acquisitions share the original sanitized, fail-closed guard. The access-log callback annotation now matches Starlette; its runtime behavior is unchanged.

Local validation passed: 12 SQLite/PostgreSQL resolver cases; 73 MCP/inventory checks; 13 access-log tests; 44 startup/runtime tests; and four official PostgreSQL bootstrap tests with clean teardown and process exit. PID-only diagnostics show zero detached pool reopens after both corrections. Seven replacement-acquisition failure cases failed before the guard correction and pass afterward. Independent review also ran 11 startup cases and confirmed that a replacement failure remains fatal through the real runtime wrapper. All seven changed Python files pass mypy 2.4.1 and Ruff. Bandit has no runtime findings; existing test findings remain, with only four added regression assertions.

The exact historical ObjectInUseError and native pytest GC-abort interleaving remains unproven; these are concrete lifecycle repairs, not a claim that the new hosted run already passed. Fresh required and advisory CI is awaiting verification. Earlier CI reports below refer to the superseded head. The requester Change summary waiver remains in force. No merge, auto-merge, restore or recovery operation was performed.

Required E2E fixture correction — November 7

The superseded dd2f8da4 E2E Critical run passed 45 tests and failed the Rowan citation-membership journey. Its trace shows that the randomly generated fixture UUID contained phone-shaped digits (3374-4043-8077), so the existing privacy filter redacted the synthetic namespace in both the source title and excerpt. The literal-source assertion correctly detected the difference.

The fixture now uses underscore-separated UUIDs, preserving all UUID entropy while avoiding phone-number matches. Product behavior, privacy controls, citation assertions, timeouts and retries are unchanged. The exact failing UUID reproduces the assertion failure through the real content-policy function and unchanged grounding oracle before the correction, then passes afterward; a fully numeric UUID edge case also passes, while a phone-number control still redacts. All 50 existing grounding tests, standard full WebUI TypeScript (8192 MiB), scoped ESLint and task/diff checks pass. Independent review found no issue and checked 10,004 UUIDs. ADR required: no; this restores the intended synthetic fixture contract.

The complete browser journey was not rerun locally. Fresh required and advisory CI must validate this new head; previous check successes do not transfer. Temporary dependency links were removed, and unrelated work was preserved.

Summary

Drop the query string for GET /api/v1/auth/verify-email from formatted Uvicorn access records. The endpoint takes a verification token in the query. The existing redactor now covers that route, including the trailing-slash form, while preserving the method, path, HTTP version and response status.

Validation

  • Before the fix, both verification-route tests exposed the synthetic token. The neighboring-route control passed.
  • All 8 tests in tldw_Server_API/tests/Logging/test_access_log_redaction.py passed after the change.
  • Ruff and git diff --check passed. Bandit reported zero findings in the changed runtime module.
  • ADR assessment: this extends the existing request-log redaction contract.

CI follow-up (2026-11-07 UTC)

All eight protected checks passed on 395288a47bc184529d24ef552db7f52a11832214. The advisory AuthNZ integration shard failed six storage-resolver cases. The earlier baseline shard reports the identical six failures and 534 passes. Its head ddd8731096074f17b4d32aca57696f1e18d2dad4 and this PR's base 2f402bd7e34015ca6823793fd05cee68a16092b2 have the identical complete Git tree. Narrow exact-base SQLite reproductions identify an omitted mandatory context argument in those tests.

The current advisory run additionally encountered a database teardown error and a pytest shutdown abort. Those occurrences remain unclassified; they are not claimed as proven baseline failures. No redaction regression has been demonstrated. At 13:07 UTC on November 7, all current-head checks have finished and all eight protected checks remain successful. The advisory full suite failed in the three shards documented here; its summary job only aggregates those shard results and reports no additional test failure. This is not a claim that all CI passed.

Inherited advisory MCP setup errors (November 7)

The current-head platform-mcp-inapp shard reports 4,303 passed, three skipped and 36 setup errors. Every error is a parameter variant of test_invalid_handle_insertion_owner_has_no_repository_or_governor_effects in test_model_completion_accounting.py: the mocker fixture is unavailable, so these cases do not run.

All 36 setup errors are inherited. The matching baseline shard reports the identical 36 cases and missing-fixture errors, with the same 4,303 passed and three skipped. Its revision ddd87310 has the identical entire Git tree as this run's exact base 2f402bd7. The complete MCP_unified source/test tree, root and shared conftest, shared test plugins, pyproject, GitHub workflow/action tree and Helper_Scripts are unchanged between base and PR head.

No redaction regression is demonstrated. Classification used existing CI logs and source comparison; no deterministic baseline tests were rerun or unrelated MCP/dependency changes made. All eight protected checks still report SUCCESS as of 13:07 UTC. The earlier AuthNZ teardown/shutdown limitations remain unclassified.

Inherited advisory import-inventory drift (November 7)

The current-head integrations shard reports 3,471 passed, 46 skipped and one failure: test_import_inventory_artifact_matches_current_import_surface. The checked-in WebScraping inventory retains old line numbers for three imports in RAG/rag_service/unified_pipeline.py: 3244 instead of 3387, and 3273 instead of 3416 for the other two.

This failure is inherited. The matching baseline shard reports the same failure and pass/skip counts, with identical assertion details. Its revision ddd87310 has the identical entire Git tree as the exact run base 2f402bd7. The WebScraping source/tests, inventory generator and artifacts, affected RAG file, shared configuration and CI workflow are unchanged between base and PR head. No redaction regression is demonstrated; no inventory regeneration, unrelated code change or deterministic test rerun was made.

Requester Change summary exception

The requester explicitly waived the human-written Change summary requirement for PRs #3428, #3429, #3430 and #3431 in the CI follow-up chat, answering “waived” on November 6, 2026 (Pacific). Required checks and review still apply.

Summary by cubic

Redacts email verification tokens from Uvicorn access logs so the token in the query string no longer appears in log output, and repairs inherited CI failures found while validating this change.

  • Routes matching /api/v1/auth/verify-email (with or without trailing slash) now log ?[REDACTED]; unrelated routes with similar names keep their query string, and existing redaction for email search queries and ticket tokens is unchanged.
  • AuthNZ startup and the PostgreSQL bootstrap test reacquire the registered pool after seeding instead of reusing a retired reference; replacement-pool failures abort startup without exposing exception text.
  • Registers pytest_mock explicitly, passes explicit trusted membership context to resolver setup, and regenerates stale WebScraping import inventory line numbers.
  • E2E journey fixture UUIDs now use underscore separators so hyphenated numeric groups do not trigger the real phone-number redactor; assertions are unchanged.