From 4cae85ea5b36b69961e3d349b6c14b424b0233c1 Mon Sep 17 00:00:00 2001 From: Konrad Kleczkowski Date: Thu, 2 Jul 2026 13:22:46 +0200 Subject: [PATCH 1/4] Add daily registration digest email to the team MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An in-app scheduler emails the registered-contact list to a fixed recipient set once per UTC day, so registration stats can be handed off without the operator acting each time. - reporting/registrations.py: read-only projection of the agents table (modeled on persistence/leaderboard.py) with an explicit column allowlist, so the agent id (the secret API key) is never exported. CSV cells are sanitized against spreadsheet formula injection. - orchestration/digest_scheduler.py: hourly loop + testable run_once; idempotent via a one-row digest_state table. Fail-closed — inert unless both DIGEST_RECIPIENTS and SMTP are set, so the contact list never reaches the logs. - email/sender.py: send_digest on the Protocol; SMTP attaches the CSV, Console/Fake redact. - cli.py: `qtw send-digest [--dry-run]` for manual send / verification. - config: DIGEST_RECIPIENTS, DIGEST_HOUR_UTC (+ docker-compose, AGENTS.md). Adds no HTTP route and no MCP tool. Tests cover the key-leak guard, column allowlist, CSV injection, stats, and idempotency. Refs QUI-785 --- AGENTS.md | 20 +- backend/src/backend/api/app.py | 3 + backend/src/backend/cli.py | 55 +++++ backend/src/backend/config.py | 14 ++ backend/src/backend/db/models.py | 14 ++ backend/src/backend/email/sender.py | 76 +++++++ .../backend/orchestration/digest_scheduler.py | 137 ++++++++++++ backend/src/backend/reporting/__init__.py | 6 + .../src/backend/reporting/registrations.py | 151 +++++++++++++ backend/tests/conftest.py | 4 +- backend/tests/test_digest.py | 207 ++++++++++++++++++ docker-compose.yml | 4 + 12 files changed, 689 insertions(+), 2 deletions(-) create mode 100644 backend/src/backend/orchestration/digest_scheduler.py create mode 100644 backend/src/backend/reporting/__init__.py create mode 100644 backend/src/backend/reporting/registrations.py create mode 100644 backend/tests/test_digest.py diff --git a/AGENTS.md b/AGENTS.md index ed892de..a8ca77d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -50,4 +50,22 @@ Returns per-asset expected return (μ) and current holdings for the authenticate } ``` -Pre-solve: falls back to the agent's configured basket with `units=0`. Post-solve: reflects actual holdings. μ is the annualised hourly expected return over the last 7 days (`MU_WINDOW_HOURS=168`). \ No newline at end of file +Pre-solve: falls back to the agent's configured basket with `units=0`. Post-solve: reflects actual holdings. μ is the annualised hourly expected return over the last 7 days (`MU_WINDOW_HOURS=168`). + +## Registration digest + +A daily email of the registered-contact list to the team, so registration stats can be handed off +without the operator acting each time. An in-app scheduler (`orchestration/digest_scheduler.py`, +started in the app lifespan) sends once per UTC day at/after `DIGEST_HOUR_UTC`; the projection lives +in `reporting/registrations.py` (modeled on `persistence/leaderboard.py`). + +- **Enable:** set `DIGEST_RECIPIENTS` (comma-separated) and a real `SMTP_PASSWORD`. Empty recipients + or no SMTP ⇒ the scheduler is inert (**fail-closed**) — the contact list never reaches the logs. +- **Config:** `DIGEST_RECIPIENTS`, `DIGEST_HOUR_UTC` (default `7`). Recipients: Konrad, Brent, Paula. +- **Manual send / verify:** `qtw send-digest` (real send) or `qtw send-digest --dry-run` (render the + summary + CSV to stdout without sending). +- **Safety:** the CSV uses an explicit column allowlist (`reporting.registrations.CSV_COLUMNS`) — the + agent `id` (the secret API key) is never selected or emitted — and cells are sanitized against + spreadsheet formula injection. The digest adds no HTTP route and no MCP tool. +- **Accepted risk:** exported PII then lives in recipients' inboxes; treat those addresses and any + saved CSVs accordingly. \ No newline at end of file diff --git a/backend/src/backend/api/app.py b/backend/src/backend/api/app.py index 604fa20..e29a6f4 100644 --- a/backend/src/backend/api/app.py +++ b/backend/src/backend/api/app.py @@ -19,6 +19,7 @@ from .. import config from ..db.engine import get_engine, init_engine from ..db.models import Base +from ..orchestration import digest_scheduler from . import routes from .auth import APIKeyMiddleware @@ -62,7 +63,9 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: init_engine() async with get_engine().begin() as conn: await conn.run_sync(Base.metadata.create_all) + digest_scheduler.start(app) yield + await digest_scheduler.stop(app) # Note: the engine is process-wide and intentionally NOT disposed here — # tests share it across many app instances and own its lifecycle. diff --git a/backend/src/backend/cli.py b/backend/src/backend/cli.py index a39b340..f1ea75d 100644 --- a/backend/src/backend/cli.py +++ b/backend/src/backend/cli.py @@ -279,6 +279,57 @@ def cmd_verify_dwave(args: argparse.Namespace) -> None: ) +def cmd_send_digest(args: argparse.Namespace) -> None: + """Build and send the registration digest now (bypasses the daily schedule). + + ``--dry-run`` renders the summary + CSV to stdout without sending — use it to + verify the payload when SMTP is not configured. A real send requires both + ``DIGEST_RECIPIENTS`` and ``SMTP_PASSWORD``. + """ + import asyncio + + asyncio.run(_send_digest_async(args.dry_run)) + + +async def _send_digest_async(dry_run: bool) -> None: + from datetime import UTC, datetime + + from .db.engine import get_engine, init_engine, session_scope + from .db.models import Base, DigestState + from .email.sender import get_email_sender + from .orchestration import digest_scheduler + from .reporting import registrations + + init_engine() + async with get_engine().begin() as conn: + await conn.run_sync(Base.metadata.create_all) + + now = datetime.now(UTC) + today = now.date().isoformat() + async with session_scope() as session: + rows = await registrations.fetch_rows(session) + state = await session.get(DigestState, 1) + since = state.last_sent_date if state else None + stats = registrations.compute_stats(rows, since) + subject, text, _ = registrations.render(stats, today) + + if dry_run: + print(f"[dry-run] would send to: {config.DIGEST_RECIPIENTS or '(none set)'}") + print(f"[dry-run] subject: {subject}\n") + print(text) + print(f"\n--- registrations-{today}.csv ---") + print(registrations.rows_to_csv(rows), end="") + return + + if not config.DIGEST_RECIPIENTS: + raise SystemExit("DIGEST_RECIPIENTS is empty — set it (or use --dry-run)") + if not config.SMTP_PASSWORD: + raise SystemExit("SMTP is not configured — set SMTP_PASSWORD (or use --dry-run)") + + await digest_scheduler.send_now(session, get_email_sender(), now) + print(f"sent registration digest ({stats.total} registrants) to {config.DIGEST_RECIPIENTS}") + + def _print_speedup(winner, results, winner_label) -> None: if winner is None or winner.solve_time_s <= 0: return @@ -320,6 +371,10 @@ def main(argv: list[str] | None = None) -> None: _add_common_args(p_verify) p_verify.set_defaults(func=cmd_verify_dwave) + p_digest = sub.add_parser("send-digest", help="send the registration digest now") + p_digest.add_argument("--dry-run", action="store_true", help="render without sending") + p_digest.set_defaults(func=cmd_send_digest) + args = parser.parse_args(argv) try: args.func(args) diff --git a/backend/src/backend/config.py b/backend/src/backend/config.py index 1ced8b6..c384043 100644 --- a/backend/src/backend/config.py +++ b/backend/src/backend/config.py @@ -152,3 +152,17 @@ SMTP_PASSWORD: str = os.environ.get("SMTP_PASSWORD", "") SMTP_STARTTLS: bool = os.environ.get("SMTP_STARTTLS", "true").lower() != "false" EMAIL_FROM: str = os.environ.get("EMAIL_FROM", "Qubitrefill ") + +# ----------------------------------------------------------------------------- +# Registration digest — a daily email of the registered-contact list to the team +# ----------------------------------------------------------------------------- + +# Comma-separated recipient addresses. EMPTY ⇒ the digest is disabled (fail-closed): +# the scheduler never runs and no PII leaves the DB. The digest also stays inert +# unless SMTP_PASSWORD is set, so it can never fall back to logging the list. +DIGEST_RECIPIENTS: list[str] = [ + addr.strip() for addr in os.environ.get("DIGEST_RECIPIENTS", "").split(",") if addr.strip() +] +# Hour of day (UTC) at/after which the daily digest may send. The scheduler wakes +# hourly and sends the first time it is past this hour with no send recorded today. +DIGEST_HOUR_UTC: int = int(os.environ.get("DIGEST_HOUR_UTC", "7")) diff --git a/backend/src/backend/db/models.py b/backend/src/backend/db/models.py index 4069223..adf626e 100644 --- a/backend/src/backend/db/models.py +++ b/backend/src/backend/db/models.py @@ -37,6 +37,20 @@ class Agent(Base): created_at: Mapped[str] = mapped_column(String(64)) +class DigestState(Base): + """Single-row marker for the registration digest scheduler. + + Holds the calendar date (UTC, ``YYYY-MM-DD``) of the last successful send so + the daily digest is idempotent across restarts — the row is created on first + send and its ``id`` is always ``1``. + """ + + __tablename__ = "digest_state" + + id: Mapped[int] = mapped_column(Integer, primary_key=True) # always 1 + last_sent_date: Mapped[str | None] = mapped_column(String(16), nullable=True) + + class Job(Base): __tablename__ = "jobs" diff --git a/backend/src/backend/email/sender.py b/backend/src/backend/email/sender.py index 228d9a4..4d79b08 100644 --- a/backend/src/backend/email/sender.py +++ b/backend/src/backend/email/sender.py @@ -40,6 +40,16 @@ def _render(name: str, api_key: str) -> tuple[str, str, str]: class EmailSender(Protocol): async def send_api_key(self, to_email: str, name: str, api_key: str) -> None: ... + async def send_digest( + self, + recipients: list[str], + subject: str, + text: str, + html: str, + csv_bytes: bytes, + csv_filename: str, + ) -> None: ... + class SmtpEmailSender: """Sends via an SMTP relay (Resend: smtp.resend.com:587, STARTTLS).""" @@ -62,6 +72,34 @@ async def send_api_key(self, to_email: str, name: str, api_key: str) -> None: timeout=10.0, ) + async def send_digest( + self, + recipients: list[str], + subject: str, + text: str, + html: str, + csv_bytes: bytes, + csv_filename: str, + ) -> None: + msg = EmailMessage() + msg["From"] = config.EMAIL_FROM + msg["To"] = ", ".join(recipients) + msg["Subject"] = subject + msg.set_content(text) + msg.add_alternative(html, subtype="html") + msg.add_attachment( + csv_bytes, maintype="text", subtype="csv", filename=csv_filename + ) + await aiosmtplib.send( + msg, + hostname=config.SMTP_HOST, + port=config.SMTP_PORT, + username=config.SMTP_USERNAME, + password=config.SMTP_PASSWORD, + start_tls=config.SMTP_STARTTLS, + timeout=30.0, + ) + class ConsoleEmailSender: """Dev fallback — logs the key instead of sending it.""" @@ -69,16 +107,54 @@ class ConsoleEmailSender: async def send_api_key(self, to_email: str, name: str, api_key: str) -> None: log.warning("[email:console] API key for %s <%s>: %s", name, to_email, api_key) + async def send_digest( + self, + recipients: list[str], + subject: str, + text: str, + html: str, + csv_bytes: bytes, + csv_filename: str, + ) -> None: + # Redacted: never log the contact list. The scheduler already refuses to + # run without real SMTP; this is a second line of defense. + log.warning( + "[email:console] digest suppressed (no SMTP): %d recipients, %d CSV bytes", + len(recipients), + len(csv_bytes), + ) + class FakeEmailSender: """Test double — records every send for assertion.""" def __init__(self) -> None: self.sent: list[dict[str, str]] = [] + self.digests: list[dict] = [] async def send_api_key(self, to_email: str, name: str, api_key: str) -> None: self.sent.append({"email": to_email, "name": name, "api_key": api_key}) + async def send_digest( + self, + recipients: list[str], + subject: str, + text: str, + html: str, + csv_bytes: bytes, + csv_filename: str, + ) -> None: + self.digests.append( + { + "recipients": recipients, + "subject": subject, + "text": text, + "html": html, + "csv": csv_bytes.decode("utf-8"), + "filename": csv_filename, + } + ) + def get_email_sender() -> EmailSender: if config.SMTP_PASSWORD: diff --git a/backend/src/backend/orchestration/digest_scheduler.py b/backend/src/backend/orchestration/digest_scheduler.py new file mode 100644 index 0000000..6ca5a16 --- /dev/null +++ b/backend/src/backend/orchestration/digest_scheduler.py @@ -0,0 +1,137 @@ +"""Registration-digest scheduler — a single in-app daily email to the team. + +A background task started in the app lifespan wakes hourly and sends the digest +at most once per UTC calendar day, at/after ``DIGEST_HOUR_UTC``. Idempotency is a +one-row ``digest_state`` marker, so a restart cannot double-send (and, with a +single web worker, neither can concurrency). The task is **fail-closed**: it does +not even start unless both ``DIGEST_RECIPIENTS`` and real SMTP are configured, so +the contact list can never fall through to the console logger. + +``run_once`` (scheduled, guarded) and ``send_now`` (forced, for the CLI) share +one build path and are pure enough to unit-test with a fake sender + session. +""" + +from __future__ import annotations + +import asyncio +import logging +from datetime import UTC, datetime + +from sqlalchemy.ext.asyncio import AsyncSession + +from .. import config +from ..db.engine import session_scope +from ..db.models import DigestState +from ..email.sender import EmailSender, get_email_sender +from ..reporting import registrations + +log = logging.getLogger(__name__) + +_POLL_INTERVAL_S = 3600 # wake hourly; the date/hour guards decide when to send + + +async def _get_state(session: AsyncSession) -> DigestState: + state = await session.get(DigestState, 1) + if state is None: + state = DigestState(id=1, last_sent_date=None) + session.add(state) + return state + + +async def _build_and_send( + session: AsyncSession, sender: EmailSender, today: str, since_date: str | None +) -> registrations.RegistrationStats: + """Fetch → stats → CSV → send → advance the marker (committed).""" + rows = await registrations.fetch_rows(session) + stats = registrations.compute_stats(rows, since_date) + csv_bytes = registrations.rows_to_csv(rows).encode("utf-8") + subject, text, html = registrations.render(stats, today) + await sender.send_digest( + recipients=config.DIGEST_RECIPIENTS, + subject=subject, + text=text, + html=html, + csv_bytes=csv_bytes, + csv_filename=f"registrations-{today}.csv", + ) + state = await _get_state(session) + state.last_sent_date = today + await session.commit() + log.info( + "registration digest sent: %d recipients, %d registrants (%d new)", + len(config.DIGEST_RECIPIENTS), + stats.total, + stats.new_since, + ) + return stats + + +async def run_once(session: AsyncSession, sender: EmailSender, now: datetime) -> bool: + """Send the digest iff it is due (past DIGEST_HOUR_UTC, not yet sent today). + + Returns True when a digest was sent. The marker only advances on success, so a + send failure is retried on the next wake. + """ + today = now.date().isoformat() + state = await _get_state(session) + if state.last_sent_date == today: + return False + if now.hour < config.DIGEST_HOUR_UTC: + return False + await _build_and_send(session, sender, today, state.last_sent_date) + return True + + +async def send_now( + session: AsyncSession, sender: EmailSender, now: datetime +) -> registrations.RegistrationStats: + """Force a send regardless of the schedule (CLI/ops). Still advances the marker.""" + state = await _get_state(session) + today = now.date().isoformat() + return await _build_and_send(session, sender, today, state.last_sent_date) + + +def _enabled() -> tuple[bool, str]: + if not config.DIGEST_RECIPIENTS: + return False, "no DIGEST_RECIPIENTS set" + if not config.SMTP_PASSWORD: + return False, "no SMTP configured (SMTP_PASSWORD unset)" + return True, "" + + +async def _loop() -> None: + while True: + try: + async with session_scope() as session: + await run_once(session, get_email_sender(), datetime.now(UTC)) + except asyncio.CancelledError: + raise + except Exception: + log.exception("registration digest tick failed; will retry next wake") + await asyncio.sleep(_POLL_INTERVAL_S) + + +def start(app) -> None: + """Start the digest loop as a lifespan-scoped background task (if enabled).""" + enabled, why = _enabled() + if not enabled: + log.info("registration digest disabled: %s", why) + app.state.digest_task = None + return + log.info( + "registration digest enabled: %d recipients, daily at %02d:00 UTC", + len(config.DIGEST_RECIPIENTS), + config.DIGEST_HOUR_UTC, + ) + app.state.digest_task = asyncio.create_task(_loop()) + + +async def stop(app) -> None: + task = getattr(app.state, "digest_task", None) + if task is None: + return + task.cancel() + try: + await task + except asyncio.CancelledError: + pass diff --git a/backend/src/backend/reporting/__init__.py b/backend/src/backend/reporting/__init__.py new file mode 100644 index 0000000..2dfd6a4 --- /dev/null +++ b/backend/src/backend/reporting/__init__.py @@ -0,0 +1,6 @@ +"""Read-only reporting projections over the persistence tables. + +Like ``persistence.leaderboard``, these derive team-facing views from the +``agents`` table on read — there is no separate metrics store. Every projection +uses an explicit column allowlist because ``Agent.id`` is the secret API key. +""" diff --git a/backend/src/backend/reporting/registrations.py b/backend/src/backend/reporting/registrations.py new file mode 100644 index 0000000..128ccff --- /dev/null +++ b/backend/src/backend/reporting/registrations.py @@ -0,0 +1,151 @@ +"""Registration digest projection — the registered-contact list for the team. + +Modeled on ``persistence.leaderboard``: a typed, read-only projection of the +``agents`` table. The agent id is the secret API key, so — exactly as in the +leaderboard — it is **never** selected or emitted here; the query names its +columns explicitly rather than loading whole ``Agent`` rows. + +Pure functions (no I/O except the passed session) so the scheduler and the CLI +share one code path and the whole thing is unit-testable without a mail server. +""" + +from __future__ import annotations + +import csv +import io +from dataclasses import dataclass + +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from ..db.models import Agent + +# The exact, ordered set of columns that leave the DB. Adding a column to Agent +# does not silently widen the export — it must be added here on purpose. Note the +# absence of ``id`` (the API key). +CSV_COLUMNS: tuple[str, ...] = ( + "name", + "handle", + "email", + "updates_opt_in", + "reach_out", + "created_at", + "jobs_solved", +) + +# Leading characters a spreadsheet may interpret as a formula. User-controlled +# fields (name, reach_out) are prefixed with a quote so Excel/Sheets treat the +# cell as text — CSV/formula-injection defense. +_FORMULA_TRIGGERS: tuple[str, ...] = ("=", "+", "-", "@", "\t", "\r") + + +@dataclass(frozen=True) +class RegistrationRow: + """One registrant, allowlisted to non-secret fields.""" + + name: str + handle: str | None + email: str + updates_opt_in: bool | None + reach_out: list[str] | None + created_at: str + jobs_solved: int + + +@dataclass(frozen=True) +class RegistrationStats: + total: int + opted_in: int + opt_in_rate: float # 0..1 + new_since: int + + +async def fetch_rows(session: AsyncSession) -> list[RegistrationRow]: + """Load all registrants as allowlisted rows, oldest first. + + Selects named columns — never ``Agent.id`` — so the secret API key cannot + reach a report even by accident. + """ + result = await session.execute( + select( + Agent.name, + Agent.handle, + Agent.email, + Agent.updates_opt_in, + Agent.reach_out, + Agent.created_at, + Agent.jobs_solved, + ).order_by(Agent.created_at) + ) + return [RegistrationRow(*row) for row in result.all()] + + +def compute_stats(rows: list[RegistrationRow], since_date: str | None) -> RegistrationStats: + """Aggregate counts. ``since_date`` is a ``YYYY-MM-DD`` UTC date (last digest). + + ``new_since`` counts registrants created after ``since_date`` (all of them + when ``since_date`` is None — the first digest). ISO dates sort lexically, so + a string comparison on the date prefix is correct. + """ + total = len(rows) + opted_in = sum(1 for r in rows if r.updates_opt_in) + new_since = ( + total + if since_date is None + else sum(1 for r in rows if r.created_at[:10] > since_date) + ) + rate = opted_in / total if total else 0.0 + return RegistrationStats(total=total, opted_in=opted_in, opt_in_rate=rate, new_since=new_since) + + +def _sanitize_cell(value: object) -> str: + """Render a cell as text, neutralizing spreadsheet formula injection.""" + if value is None: + text = "" + elif isinstance(value, bool): + text = "yes" if value else "no" + elif isinstance(value, (list, tuple)): + text = "; ".join(str(v) for v in value) + else: + text = str(value) + if text.startswith(_FORMULA_TRIGGERS): + return "'" + text + return text + + +def rows_to_csv(rows: list[RegistrationRow]) -> str: + """Serialize rows to CSV text with the fixed header and injection guard.""" + buffer = io.StringIO() + writer = csv.writer(buffer) + writer.writerow(CSV_COLUMNS) + for r in rows: + writer.writerow([_sanitize_cell(getattr(r, col)) for col in CSV_COLUMNS]) + return buffer.getvalue() + + +def render(stats: RegistrationStats, today: str) -> tuple[str, str, str]: + """Build (subject, text, html) for the digest email. The CSV is attached.""" + subject = f"Qubitrefill registrations — {today} ({stats.total} total)" + pct = f"{stats.opt_in_rate * 100:.0f}%" + lines = ( + f"Registration digest for {today}", + "", + f" Total registered: {stats.total}", + f" Opted in to updates: {stats.opted_in} ({pct})", + f" New since last digest: {stats.new_since}", + "", + "The full contact list is attached as a CSV. The 'updates_opt_in' column " + "shows who consented to updates — check it before contacting anyone.", + ) + text = "\n".join(lines) + html = ( + f"

Registration digest for {today}

" + "" + "

The full contact list is attached as a CSV. The updates_opt_in " + "column shows who consented to updates — check it before contacting anyone.

" + ) + return subject, text, html diff --git a/backend/tests/conftest.py b/backend/tests/conftest.py index 33c0ade..c7ba837 100644 --- a/backend/tests/conftest.py +++ b/backend/tests/conftest.py @@ -42,7 +42,9 @@ async def _init_db(): async def isolated_state(): """Truncate tables and pin a fixed-clock market so pipeline math is reproducible.""" async with session_scope() as session: - await session.execute(text("TRUNCATE jobs, agents RESTART IDENTITY CASCADE")) + await session.execute( + text("TRUNCATE jobs, agents, digest_state RESTART IDENTITY CASCADE") + ) await session.commit() set_source(SyntheticMarketSource(clock=lambda: 1000.0)) yield diff --git a/backend/tests/test_digest.py b/backend/tests/test_digest.py new file mode 100644 index 0000000..f2939fe --- /dev/null +++ b/backend/tests/test_digest.py @@ -0,0 +1,207 @@ +"""Registration digest: key-leak guard, allowlist, CSV injection, stats, idempotency.""" + +from __future__ import annotations + +import csv +import io +from datetime import UTC, datetime + +import pytest + +from backend import config +from backend.api.schemas import SliderValues +from backend.db.engine import session_scope +from backend.db.models import DigestState +from backend.email.sender import ConsoleEmailSender, FakeEmailSender +from backend.orchestration import digest_scheduler +from backend.persistence.agents import AgentRepo +from backend.reporting import registrations +from backend.reporting.registrations import ( + CSV_COLUMNS, + RegistrationRow, + compute_stats, + rows_to_csv, +) + +_SLIDERS = SliderValues(rebalanceFrequency=50, riskPreference=50, maxPositionSize=50) + +# A recognizable secret standing in for the uuid API key (Agent.id). +_SECRET_KEY = "deadbeefdeadbeefdeadbeefdeadbeef" + + +def _row(name="Ada", *, email=None, opt_in=None, reach_out=None, created_at="2026-07-01T10:00:00+00:00"): + return RegistrationRow( + name=name, + handle=name.lower(), + email=email or f"{name.lower()}@example.com", + updates_opt_in=opt_in, + reach_out=reach_out, + created_at=created_at, + jobs_solved=0, + ) + + +async def _create(repo: AgentRepo, name: str, *, agent_id: str, opt_in=None): + return await repo.create( + agent_id=agent_id, + name=name, + email=f"{name.lower()}@example.com", + handle=name.lower(), + reach_out=None, + updates_opt_in=opt_in, + sliders=_SLIDERS, + assets=["BTC", "ETH"], + bankroll=10_000.0, + ) + + +# --- Pure projection tests (no DB) -------------------------------------------- + + +def test_csv_header_is_the_allowlist(): + text = rows_to_csv([_row()]) + header = next(csv.reader(io.StringIO(text))) + assert tuple(header) == CSV_COLUMNS + assert "id" not in header # the API key column is never present + + +def test_registration_row_has_no_id_field(): + assert not hasattr(_row(), "id") + + +def test_csv_injection_is_neutralized(): + rows = [_row(name="=cmd()", reach_out=["+telegram", "safe"])] + parsed = list(csv.reader(io.StringIO(rows_to_csv(rows)))) + data = parsed[1] + name_cell = data[CSV_COLUMNS.index("name")] + reach_cell = data[CSV_COLUMNS.index("reach_out")] + assert name_cell == "'=cmd()" + assert reach_cell.startswith("'+telegram") + + +def test_bool_and_none_rendering(): + rows = [_row(opt_in=True), _row(opt_in=False), _row(opt_in=None)] + data = list(csv.reader(io.StringIO(rows_to_csv(rows))))[1:] + col = CSV_COLUMNS.index("updates_opt_in") + assert [r[col] for r in data] == ["yes", "no", ""] + + +def test_compute_stats_counts_and_rate(): + rows = [ + _row(name="A", opt_in=True, created_at="2026-07-01T09:00:00+00:00"), + _row(name="B", opt_in=False, created_at="2026-07-02T09:00:00+00:00"), + _row(name="C", opt_in=True, created_at="2026-07-02T11:00:00+00:00"), + ] + stats = compute_stats(rows, since_date="2026-07-01") + assert stats.total == 3 + assert stats.opted_in == 2 + assert stats.opt_in_rate == pytest.approx(2 / 3) + assert stats.new_since == 2 # the two created on 07-02 + + +def test_compute_stats_first_digest_counts_all(): + rows = [_row(name="A"), _row(name="B")] + assert compute_stats(rows, since_date=None).new_since == 2 + + +def test_compute_stats_empty(): + stats = compute_stats([], since_date=None) + assert stats == registrations.RegistrationStats(0, 0, 0.0, 0) + + +# --- DB-backed tests ---------------------------------------------------------- + + +async def test_fetch_rows_never_exposes_api_key(): + async with session_scope() as session: + repo = AgentRepo(session) + await _create(repo, "Ada", agent_id=_SECRET_KEY, opt_in=True) + await session.commit() + + rows = await registrations.fetch_rows(session) + csv_text = rows_to_csv(rows) + + assert len(rows) == 1 + assert rows[0].email == "ada@example.com" + # The secret key must not leak into any row or the serialized CSV. + assert _SECRET_KEY not in csv_text + assert all(_SECRET_KEY not in str(getattr(r, c)) for r in rows for c in CSV_COLUMNS) + + +async def test_run_once_is_idempotent_within_a_day(monkeypatch): + monkeypatch.setattr(config, "DIGEST_RECIPIENTS", ["team@example.com"]) + monkeypatch.setattr(config, "DIGEST_HOUR_UTC", 7) + sender = FakeEmailSender() + + async with session_scope() as session: + repo = AgentRepo(session) + await _create(repo, "Ada", agent_id=_SECRET_KEY, opt_in=True) + await session.commit() + + day1_morning = datetime(2026, 7, 2, 9, 0, tzinfo=UTC) + assert await digest_scheduler.run_once(session, sender, day1_morning) is True + # Second tick same day: already sent → no-op. + assert await digest_scheduler.run_once(session, sender, day1_morning) is False + # Next day → sends again. + assert await digest_scheduler.run_once(session, sender, datetime(2026, 7, 3, 9, 0, tzinfo=UTC)) is True + + assert len(sender.digests) == 2 + assert sender.digests[0]["recipients"] == ["team@example.com"] + assert _SECRET_KEY not in sender.digests[0]["csv"] + + +async def test_run_once_waits_until_digest_hour(monkeypatch): + monkeypatch.setattr(config, "DIGEST_RECIPIENTS", ["team@example.com"]) + monkeypatch.setattr(config, "DIGEST_HOUR_UTC", 7) + sender = FakeEmailSender() + + async with session_scope() as session: + before = datetime(2026, 7, 2, 5, 0, tzinfo=UTC) # before the hour + assert await digest_scheduler.run_once(session, sender, before) is False + assert sender.digests == [] + + +async def test_send_now_forces_regardless_of_schedule(monkeypatch): + monkeypatch.setattr(config, "DIGEST_RECIPIENTS", ["team@example.com"]) + monkeypatch.setattr(config, "DIGEST_HOUR_UTC", 23) + sender = FakeEmailSender() + + async with session_scope() as session: + await _create(AgentRepo(session), "Ada", agent_id=_SECRET_KEY) + await session.commit() + # 03:00, well before DIGEST_HOUR_UTC=23, but send_now ignores the guard. + stats = await digest_scheduler.send_now(session, sender, datetime(2026, 7, 2, 3, 0, tzinfo=UTC)) + state = await session.get(DigestState, 1) + + assert stats.total == 1 + assert len(sender.digests) == 1 + assert state.last_sent_date == "2026-07-02" + + +async def test_console_sender_does_not_log_pii(caplog): + sender = ConsoleEmailSender() + with caplog.at_level("WARNING"): + await sender.send_digest( + recipients=["team@example.com"], + subject="s", + text="t", + html="h", + csv_bytes=b"name,email\nAda,ada@secret.example\n", + csv_filename="registrations.csv", + ) + joined = " ".join(r.getMessage() for r in caplog.records) + assert "ada@secret.example" not in joined + assert "suppressed" in joined + + +def test_enabled_requires_recipients_and_smtp(monkeypatch): + monkeypatch.setattr(config, "DIGEST_RECIPIENTS", []) + monkeypatch.setattr(config, "SMTP_PASSWORD", "x") + assert digest_scheduler._enabled()[0] is False + + monkeypatch.setattr(config, "DIGEST_RECIPIENTS", ["a@b.com"]) + monkeypatch.setattr(config, "SMTP_PASSWORD", "") + assert digest_scheduler._enabled()[0] is False + + monkeypatch.setattr(config, "SMTP_PASSWORD", "resend-key") + assert digest_scheduler._enabled()[0] is True diff --git a/docker-compose.yml b/docker-compose.yml index 7d0dbc7..34c3f3e 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -35,6 +35,10 @@ services: SMTP_USERNAME: ${SMTP_USERNAME:-resend} SMTP_PASSWORD: ${SMTP_PASSWORD:-} EMAIL_FROM: ${EMAIL_FROM:-Qubitrefill } + # Registration digest — daily contact-list email to the team. Empty + # DIGEST_RECIPIENTS ⇒ disabled (fail-closed); also inert without SMTP_PASSWORD. + DIGEST_RECIPIENTS: ${DIGEST_RECIPIENTS:-} + DIGEST_HOUR_UTC: ${DIGEST_HOUR_UTC:-7} ports: - "8000:8000" healthcheck: From bd8322ec3e95c508f96121c0a700819f39ed44d5 Mon Sep 17 00:00:00 2001 From: Konrad Kleczkowski Date: Thu, 2 Jul 2026 20:52:17 +0200 Subject: [PATCH 2/4] Set registration digest recipient to the BD alias MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Point the daily digest at bd@postquant.xyz instead of a list of personal inboxes, centralizing the PII handoff to one BD address. - docker-compose.yml: default DIGEST_RECIPIENTS to bd@postquant.xyz. Use ${VAR-default} (no colon) so an explicit empty value still disables the digest — the fail-closed escape hatch is preserved. - config.py: note that the library default stays empty and the deployment default lives in docker-compose.yml. - AGENTS.md: document the default recipient and override path. Refs QUI-785 --- AGENTS.md | 8 +++++--- backend/src/backend/config.py | 2 ++ docker-compose.yml | 7 ++++--- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a8ca77d..1446144 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -59,9 +59,11 @@ without the operator acting each time. An in-app scheduler (`orchestration/diges started in the app lifespan) sends once per UTC day at/after `DIGEST_HOUR_UTC`; the projection lives in `reporting/registrations.py` (modeled on `persistence/leaderboard.py`). -- **Enable:** set `DIGEST_RECIPIENTS` (comma-separated) and a real `SMTP_PASSWORD`. Empty recipients - or no SMTP ⇒ the scheduler is inert (**fail-closed**) — the contact list never reaches the logs. -- **Config:** `DIGEST_RECIPIENTS`, `DIGEST_HOUR_UTC` (default `7`). Recipients: Konrad, Brent, Paula. +- **Enable:** set a real `SMTP_PASSWORD` (the recipient defaults to `bd@postquant.xyz`). Empty + recipients or no SMTP ⇒ the scheduler is inert (**fail-closed**) — the contact list never reaches + the logs. +- **Config:** `DIGEST_RECIPIENTS`, `DIGEST_HOUR_UTC` (default `7`). Deployment defaults the recipient to + the BD alias `bd@postquant.xyz` (see `docker-compose.yml`); override with a comma-separated list. - **Manual send / verify:** `qtw send-digest` (real send) or `qtw send-digest --dry-run` (render the summary + CSV to stdout without sending). - **Safety:** the CSV uses an explicit column allowlist (`reporting.registrations.CSV_COLUMNS`) — the diff --git a/backend/src/backend/config.py b/backend/src/backend/config.py index c384043..d20417e 100644 --- a/backend/src/backend/config.py +++ b/backend/src/backend/config.py @@ -160,6 +160,8 @@ # Comma-separated recipient addresses. EMPTY ⇒ the digest is disabled (fail-closed): # the scheduler never runs and no PII leaves the DB. The digest also stays inert # unless SMTP_PASSWORD is set, so it can never fall back to logging the list. +# The library default is empty; the deployment default (bd@postquant.xyz) is set +# in docker-compose.yml. DIGEST_RECIPIENTS: list[str] = [ addr.strip() for addr in os.environ.get("DIGEST_RECIPIENTS", "").split(",") if addr.strip() ] diff --git a/docker-compose.yml b/docker-compose.yml index 34c3f3e..e20b668 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -35,9 +35,10 @@ services: SMTP_USERNAME: ${SMTP_USERNAME:-resend} SMTP_PASSWORD: ${SMTP_PASSWORD:-} EMAIL_FROM: ${EMAIL_FROM:-Qubitrefill } - # Registration digest — daily contact-list email to the team. Empty - # DIGEST_RECIPIENTS ⇒ disabled (fail-closed); also inert without SMTP_PASSWORD. - DIGEST_RECIPIENTS: ${DIGEST_RECIPIENTS:-} + # Registration digest — daily contact-list email to the team. Defaults to + # the BD alias; set DIGEST_RECIPIENTS="" to disable (fail-closed). Also + # inert without SMTP_PASSWORD, so it can never fall back to logging the list. + DIGEST_RECIPIENTS: ${DIGEST_RECIPIENTS-bd@postquant.xyz} DIGEST_HOUR_UTC: ${DIGEST_HOUR_UTC:-7} ports: - "8000:8000" From b8f7a9140a712c61b98616da8844f852622530cb Mon Sep 17 00:00:00 2001 From: Konrad Kleczkowski Date: Thu, 2 Jul 2026 21:20:57 +0200 Subject: [PATCH 3/4] Harden digest against double-send and same-day undercount MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the two PR #10 review findings. Finding 1 (high) — concurrent workers could double-send. `start()` runs in every uvicorn worker's lifespan, so WEB_CONCURRENCY>1 spawns one scheduler per process; the in-memory `last_sent_date` guard and the marker commit weren't atomic, so two workers could both pass the guard and send duplicate PII digests. Serialize the check→send→mark with a transaction-scoped Postgres advisory lock (cluster-global, auto-released on commit/rollback), which preserves the "marker advances only on success" retry semantics. Regression test races two run_once calls and asserts exactly one send. Finding 2 (medium) — `new_since` undercounted same-day registrations. The marker stored only a date, so anyone registering later on a send's own UTC day was permanently excluded from "New since last digest" even though the CSV listed them. Store the full send timestamp (DigestState.last_sent_date → last_sent_at) and compare full ISO timestamps; the date prefix still drives once-per-day idempotency. Note: `create_all` does not alter existing tables, so the digest_state table must be dropped once for the column change to take effect. Refs QUI-785 --- backend/src/backend/cli.py | 2 +- backend/src/backend/db/models.py | 8 ++-- .../backend/orchestration/digest_scheduler.py | 40 +++++++++++----- .../src/backend/reporting/registrations.py | 20 ++++---- backend/tests/test_digest.py | 47 +++++++++++++++++-- 5 files changed, 86 insertions(+), 31 deletions(-) diff --git a/backend/src/backend/cli.py b/backend/src/backend/cli.py index f1ea75d..6d8e96f 100644 --- a/backend/src/backend/cli.py +++ b/backend/src/backend/cli.py @@ -309,7 +309,7 @@ async def _send_digest_async(dry_run: bool) -> None: async with session_scope() as session: rows = await registrations.fetch_rows(session) state = await session.get(DigestState, 1) - since = state.last_sent_date if state else None + since = state.last_sent_at if state else None stats = registrations.compute_stats(rows, since) subject, text, _ = registrations.render(stats, today) diff --git a/backend/src/backend/db/models.py b/backend/src/backend/db/models.py index adf626e..e65f64e 100644 --- a/backend/src/backend/db/models.py +++ b/backend/src/backend/db/models.py @@ -40,15 +40,17 @@ class Agent(Base): class DigestState(Base): """Single-row marker for the registration digest scheduler. - Holds the calendar date (UTC, ``YYYY-MM-DD``) of the last successful send so - the daily digest is idempotent across restarts — the row is created on first + Holds the ISO-8601 UTC timestamp (``…+00:00``) of the last successful send. + The date prefix drives once-per-day idempotency across restarts; the full + timestamp is the ``new_since`` boundary, so a registrant who signs up later on + the send's own day is still counted next time. The row is created on first send and its ``id`` is always ``1``. """ __tablename__ = "digest_state" id: Mapped[int] = mapped_column(Integer, primary_key=True) # always 1 - last_sent_date: Mapped[str | None] = mapped_column(String(16), nullable=True) + last_sent_at: Mapped[str | None] = mapped_column(String(32), nullable=True) class Job(Base): diff --git a/backend/src/backend/orchestration/digest_scheduler.py b/backend/src/backend/orchestration/digest_scheduler.py index 6ca5a16..aebaddb 100644 --- a/backend/src/backend/orchestration/digest_scheduler.py +++ b/backend/src/backend/orchestration/digest_scheduler.py @@ -2,8 +2,9 @@ A background task started in the app lifespan wakes hourly and sends the digest at most once per UTC calendar day, at/after ``DIGEST_HOUR_UTC``. Idempotency is a -one-row ``digest_state`` marker, so a restart cannot double-send (and, with a -single web worker, neither can concurrency). The task is **fail-closed**: it does +one-row ``digest_state`` marker guarded by a Postgres advisory lock, so neither a +restart nor concurrent workers (``WEB_CONCURRENCY>1``) can double-send. The task +is **fail-closed**: it does not even start unless both ``DIGEST_RECIPIENTS`` and real SMTP are configured, so the contact list can never fall through to the console logger. @@ -17,6 +18,7 @@ import logging from datetime import UTC, datetime +from sqlalchemy import text from sqlalchemy.ext.asyncio import AsyncSession from .. import config @@ -29,33 +31,46 @@ _POLL_INTERVAL_S = 3600 # wake hourly; the date/hour guards decide when to send +# Cross-process/instance mutex for the send. A Postgres advisory lock is +# cluster-global and transaction-scoped (auto-released on commit OR rollback), so +# acquiring it before the guard makes the whole check→send→mark sequence atomic +# even when WEB_CONCURRENCY>1 runs one scheduler per worker. Key is arbitrary but +# fixed — the ASCII bytes of "DIGEST". +_SEND_LOCK_KEY = 0x444947455354 # b"DIGEST" + + +async def _acquire_send_lock(session: AsyncSession) -> None: + """Serialize the digest send across workers/instances until this txn ends.""" + await session.execute(text("SELECT pg_advisory_xact_lock(:key)"), {"key": _SEND_LOCK_KEY}) + async def _get_state(session: AsyncSession) -> DigestState: state = await session.get(DigestState, 1) if state is None: - state = DigestState(id=1, last_sent_date=None) + state = DigestState(id=1, last_sent_at=None) session.add(state) return state async def _build_and_send( - session: AsyncSession, sender: EmailSender, today: str, since_date: str | None + session: AsyncSession, sender: EmailSender, now: datetime, since: str | None ) -> registrations.RegistrationStats: """Fetch → stats → CSV → send → advance the marker (committed).""" + today = now.date().isoformat() rows = await registrations.fetch_rows(session) - stats = registrations.compute_stats(rows, since_date) + stats = registrations.compute_stats(rows, since) csv_bytes = registrations.rows_to_csv(rows).encode("utf-8") - subject, text, html = registrations.render(stats, today) + subject, body, html = registrations.render(stats, today) await sender.send_digest( recipients=config.DIGEST_RECIPIENTS, subject=subject, - text=text, + text=body, html=html, csv_bytes=csv_bytes, csv_filename=f"registrations-{today}.csv", ) state = await _get_state(session) - state.last_sent_date = today + state.last_sent_at = now.isoformat() await session.commit() log.info( "registration digest sent: %d recipients, %d registrants (%d new)", @@ -73,12 +88,13 @@ async def run_once(session: AsyncSession, sender: EmailSender, now: datetime) -> send failure is retried on the next wake. """ today = now.date().isoformat() + await _acquire_send_lock(session) state = await _get_state(session) - if state.last_sent_date == today: + if state.last_sent_at is not None and state.last_sent_at[:10] == today: return False if now.hour < config.DIGEST_HOUR_UTC: return False - await _build_and_send(session, sender, today, state.last_sent_date) + await _build_and_send(session, sender, now, state.last_sent_at) return True @@ -86,9 +102,9 @@ async def send_now( session: AsyncSession, sender: EmailSender, now: datetime ) -> registrations.RegistrationStats: """Force a send regardless of the schedule (CLI/ops). Still advances the marker.""" + await _acquire_send_lock(session) state = await _get_state(session) - today = now.date().isoformat() - return await _build_and_send(session, sender, today, state.last_sent_date) + return await _build_and_send(session, sender, now, state.last_sent_at) def _enabled() -> tuple[bool, str]: diff --git a/backend/src/backend/reporting/registrations.py b/backend/src/backend/reporting/registrations.py index 128ccff..ad6fd2c 100644 --- a/backend/src/backend/reporting/registrations.py +++ b/backend/src/backend/reporting/registrations.py @@ -80,20 +80,18 @@ async def fetch_rows(session: AsyncSession) -> list[RegistrationRow]: return [RegistrationRow(*row) for row in result.all()] -def compute_stats(rows: list[RegistrationRow], since_date: str | None) -> RegistrationStats: - """Aggregate counts. ``since_date`` is a ``YYYY-MM-DD`` UTC date (last digest). - - ``new_since`` counts registrants created after ``since_date`` (all of them - when ``since_date`` is None — the first digest). ISO dates sort lexically, so - a string comparison on the date prefix is correct. +def compute_stats(rows: list[RegistrationRow], since: str | None) -> RegistrationStats: + """Aggregate counts. ``since`` is the ISO-8601 UTC timestamp of the last send. + + ``new_since`` counts registrants created after ``since`` (all of them when + ``since`` is None — the first digest). Comparing full timestamps (not just the + date prefix) means someone who registers later on the send's own day is still + counted; ISO-8601 with a fixed offset sorts lexically, so a string compare is + correct. """ total = len(rows) opted_in = sum(1 for r in rows if r.updates_opt_in) - new_since = ( - total - if since_date is None - else sum(1 for r in rows if r.created_at[:10] > since_date) - ) + new_since = total if since is None else sum(1 for r in rows if r.created_at > since) rate = opted_in / total if total else 0.0 return RegistrationStats(total=total, opted_in=opted_in, opt_in_rate=rate, new_since=new_since) diff --git a/backend/tests/test_digest.py b/backend/tests/test_digest.py index f2939fe..0d19b9b 100644 --- a/backend/tests/test_digest.py +++ b/backend/tests/test_digest.py @@ -2,6 +2,7 @@ from __future__ import annotations +import asyncio import csv import io from datetime import UTC, datetime @@ -92,20 +93,31 @@ def test_compute_stats_counts_and_rate(): _row(name="B", opt_in=False, created_at="2026-07-02T09:00:00+00:00"), _row(name="C", opt_in=True, created_at="2026-07-02T11:00:00+00:00"), ] - stats = compute_stats(rows, since_date="2026-07-01") + stats = compute_stats(rows, since="2026-07-01T12:00:00+00:00") assert stats.total == 3 assert stats.opted_in == 2 assert stats.opt_in_rate == pytest.approx(2 / 3) assert stats.new_since == 2 # the two created on 07-02 +def test_compute_stats_counts_same_day_after_send(): + """Finding 2: a registrant created after the send, on the send's own day, is + still 'new since' — the boundary is the full send timestamp, not the date.""" + rows = [ + _row(name="Before", created_at="2026-07-02T06:00:00+00:00"), + _row(name="After", created_at="2026-07-02T09:30:00+00:00"), + ] + # Previous digest sent at 07:00 the same day. + assert compute_stats(rows, since="2026-07-02T07:00:00+00:00").new_since == 1 + + def test_compute_stats_first_digest_counts_all(): rows = [_row(name="A"), _row(name="B")] - assert compute_stats(rows, since_date=None).new_since == 2 + assert compute_stats(rows, since=None).new_since == 2 def test_compute_stats_empty(): - stats = compute_stats([], since_date=None) + stats = compute_stats([], since=None) assert stats == registrations.RegistrationStats(0, 0, 0.0, 0) @@ -150,6 +162,33 @@ async def test_run_once_is_idempotent_within_a_day(monkeypatch): assert _SECRET_KEY not in sender.digests[0]["csv"] +async def test_run_once_no_duplicate_send_under_concurrency(monkeypatch): + """Two schedulers racing on the same UTC day send exactly once. + + With WEB_CONCURRENCY>1 each uvicorn worker runs its own ``_loop`` in a + separate process, so the in-process ``last_sent_at`` guard cannot prevent a + double send — only the DB advisory lock serializing check→send→mark can. + """ + monkeypatch.setattr(config, "DIGEST_RECIPIENTS", ["team@example.com"]) + monkeypatch.setattr(config, "DIGEST_HOUR_UTC", 7) + sender = FakeEmailSender() + + async with session_scope() as setup: + await _create(AgentRepo(setup), "Ada", agent_id=_SECRET_KEY) + await setup.commit() + + now = datetime(2026, 7, 2, 9, 0, tzinfo=UTC) + + async def tick() -> bool: + async with session_scope() as session: + return await digest_scheduler.run_once(session, sender, now) + + results = await asyncio.gather(tick(), tick()) + + assert sum(1 for sent in results if sent) == 1 # exactly one worker sent + assert len(sender.digests) == 1 + + async def test_run_once_waits_until_digest_hour(monkeypatch): monkeypatch.setattr(config, "DIGEST_RECIPIENTS", ["team@example.com"]) monkeypatch.setattr(config, "DIGEST_HOUR_UTC", 7) @@ -175,7 +214,7 @@ async def test_send_now_forces_regardless_of_schedule(monkeypatch): assert stats.total == 1 assert len(sender.digests) == 1 - assert state.last_sent_date == "2026-07-02" + assert state.last_sent_at == "2026-07-02T03:00:00+00:00" async def test_console_sender_does_not_log_pii(caplog): From 5ca21842981d0a5f1027f8a3f38a71b6e4ad5e62 Mon Sep 17 00:00:00 2001 From: Konrad Kleczkowski Date: Thu, 2 Jul 2026 21:39:36 +0200 Subject: [PATCH 4/4] Harden digest CSV guard, CLI output, and hour config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-round PR #10 review fixes (substantive only). - registrations: quote a formula trigger hidden behind leading whitespace (" =HYPERLINK(...)") — spreadsheets strip leading spaces before evaluating, so the CSV-injection guard must too. - cli: print the RegistrationStats returned by send_now (what was actually emailed) instead of a pre-send snapshot. - config: reject DIGEST_HOUR_UTC outside 0–23 at startup; the schedule guard is `now.hour < DIGEST_HOUR_UTC`, so a bad value silently never fires. Left DigestState.last_sent_at at String(32): the stored value is always datetime.now(UTC).isoformat(), which is exactly <=32 chars, so it fits by construction. Refs QUI-785 --- backend/src/backend/cli.py | 4 ++-- backend/src/backend/config.py | 4 ++++ backend/src/backend/reporting/registrations.py | 5 ++++- backend/tests/test_digest.py | 9 +++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/backend/src/backend/cli.py b/backend/src/backend/cli.py index 6d8e96f..e6e7b6e 100644 --- a/backend/src/backend/cli.py +++ b/backend/src/backend/cli.py @@ -326,8 +326,8 @@ async def _send_digest_async(dry_run: bool) -> None: if not config.SMTP_PASSWORD: raise SystemExit("SMTP is not configured — set SMTP_PASSWORD (or use --dry-run)") - await digest_scheduler.send_now(session, get_email_sender(), now) - print(f"sent registration digest ({stats.total} registrants) to {config.DIGEST_RECIPIENTS}") + sent = await digest_scheduler.send_now(session, get_email_sender(), now) + print(f"sent registration digest ({sent.total} registrants) to {config.DIGEST_RECIPIENTS}") def _print_speedup(winner, results, winner_label) -> None: diff --git a/backend/src/backend/config.py b/backend/src/backend/config.py index d20417e..64c3a4d 100644 --- a/backend/src/backend/config.py +++ b/backend/src/backend/config.py @@ -167,4 +167,8 @@ ] # Hour of day (UTC) at/after which the daily digest may send. The scheduler wakes # hourly and sends the first time it is past this hour with no send recorded today. +# Validated fail-fast: an out-of-range value would silently never fire (the guard +# is ``now.hour < DIGEST_HOUR_UTC``), so reject it at startup instead. DIGEST_HOUR_UTC: int = int(os.environ.get("DIGEST_HOUR_UTC", "7")) +if not 0 <= DIGEST_HOUR_UTC <= 23: + raise ValueError(f"DIGEST_HOUR_UTC must be 0–23, got {DIGEST_HOUR_UTC}") diff --git a/backend/src/backend/reporting/registrations.py b/backend/src/backend/reporting/registrations.py index ad6fd2c..a1208e7 100644 --- a/backend/src/backend/reporting/registrations.py +++ b/backend/src/backend/reporting/registrations.py @@ -106,7 +106,10 @@ def _sanitize_cell(value: object) -> str: text = "; ".join(str(v) for v in value) else: text = str(value) - if text.startswith(_FORMULA_TRIGGERS): + # Spreadsheets strip leading whitespace before evaluating a cell, so a trigger + # hidden behind spaces/tabs (" =HYPERLINK(...)") must be quoted too — check the + # raw string (catches a literal leading control char) and the lstripped one. + if text.startswith(_FORMULA_TRIGGERS) or text.lstrip().startswith(_FORMULA_TRIGGERS): return "'" + text return text diff --git a/backend/tests/test_digest.py b/backend/tests/test_digest.py index 0d19b9b..9010d4a 100644 --- a/backend/tests/test_digest.py +++ b/backend/tests/test_digest.py @@ -80,6 +80,15 @@ def test_csv_injection_is_neutralized(): assert reach_cell.startswith("'+telegram") +def test_csv_injection_behind_leading_whitespace_is_neutralized(): + """Spreadsheets strip leading spaces/tabs before evaluating, so a formula + hidden behind whitespace must still be quoted.""" + rows = [_row(name=" =HYPERLINK(1)", reach_out=["\t+evil"])] + data = list(csv.reader(io.StringIO(rows_to_csv(rows))))[1] + assert data[CSV_COLUMNS.index("name")].startswith("'") + assert data[CSV_COLUMNS.index("reach_out")].startswith("'") + + def test_bool_and_none_rendering(): rows = [_row(opt_in=True), _row(opt_in=False), _row(opt_in=None)] data = list(csv.reader(io.StringIO(rows_to_csv(rows))))[1:]