diff --git a/CHANGELOG.md b/CHANGELOG.md index 00babe2..836bc2a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,190 @@ skip versions are the composition of each intervening adjacent release's steps in order — no A-to-B path is pre-computed beyond that. +## 0.18.0 — 2026-05-28 + +**Minor — schema migration required; one env var now mandatory; no +new secrets.** This release lands the framework-side half of OHM +roadmap items #18 (Secure the SMTP relay + Gitea webhook) and #20 +(Email deliverability). It is the framework counterpart to the +operator-side SMTP / DNS hardening covered in the +`EMAIL-AND-WEBHOOK-HARDENING-RUNBOOK.md` companion doc. + +The release is shipped in five atomic slices per the v0.18.0 proposal: + +1. **`build_envelope` shared helper.** A single place where every + outbound `EmailMessage` is constructed. Lands the + deliverability-critical headers (`Date`, `Message-ID`, + `Auto-Submitted`) uniformly across OTC, invite, watcher + notification, bundle, and digest paths; exposes per-kind + unsubscribe semantics (none for OTC, mailto: for invites, full + one-click for bulk-adjacent paths) as explicit kwargs. +2. **Migrated send paths.** `email_otc.py`, `email_invite.py`, + `email._deliver`, `email._send_bundle`, and `digest.py` now + build their envelopes through the helper. Per-RFC invite + (v0.16.0) rides through `email_invite.py`'s helper and picks + up the change for free. The shared `_SENT` test buffer also + carries the constructed `EmailMessage` under `envelope["message"]` + so tests can assert on the header surface directly. +3. **Webhook handler tightening.** `GITEA_WEBHOOK_SECRET` is now + mandatory at startup — the framework refuses to load_config() + if it's empty, unless the operator explicitly opts into the + dev-bypass with `RFC_APP_INSECURE_WEBHOOKS=1`. The + `/api/webhooks/gitea` receiver carries defense-in-depth checks + that surface the misconfiguration loudly at the request layer + too. Mis-targeted webhooks (a hook on a fork or a stale Gitea + binding) now log at INFO instead of silently 200-OK'ing. +4. **`outbound_emails` audit table + admin endpoint.** Every send + helper writes one row to `outbound_emails` before returning, + capturing the send attempt regardless of outcome + (status='sent' / 'failed' / 'deferred'). The new admin endpoint + `GET /api/admin/outbound-emails` (filterable by kind / status / + to_address) lets the operator answer "did this person ever get + their invite?" without grepping VM logs. No admin UI ships + with v0.18.0; operator queries via curl + jq for now. +5. **Bounce correlation.** The `POST /api/webhooks/email-bounce` + body accepts a new optional `message_id` field; when supplied, + the handler stamps status='bounced' on the matching + `outbound_emails` row and returns the row id as + `correlated_id`. The pre-existing hard-bounce -> + `email_opt_out_all = 1` flow still fires. + +The `POST /api/email/unsubscribe` endpoint also lands in Slice 2 +as the matching receiver for the new `List-Unsubscribe-Post: +List-Unsubscribe=One-Click` header (Gmail and Yahoo POST that +payload on the user's one-click action per RFC 8058 — the GET +endpoint alone is no longer sufficient for senders at OHM's tier). + +### Added + +- **`backend/app/email_envelope.py`** — the `build_envelope` helper. + Single source of truth for every outbound `EmailMessage`'s + headers + body shape. Standalone module so tests can exercise it + without booting the FastAPI app. +- **`backend/migrations/020_outbound_emails.sql`** — the audit + table. Single new table with three indexes (to_address, sent_at, + message_id); no changes to existing tables. +- **`email.record_outbound(...)`** — best-effort write helper every + send path calls. Catches `RuntimeError` (so pure-helper unit + tests where `db.init()` was never called don't break) and any + other exception (so the audit write never breaks a send). +- **`GET /api/admin/outbound-emails`** in `api_admin.py` — + admin-only listing of `outbound_emails`. Newest-first, filterable + by kind, status, and to_address (case-insensitive). Returns + `{items: [...], has_more}` per the rest of the admin endpoints' + shape. +- **`POST /api/email/unsubscribe`** in `api_notifications.py` — + RFC 8058 one-click receiver. Accepts the same `?t=` token as the + GET handler; idempotent; returns 200 + `{ok, category}` on + success. +- **`all` synthetic category** for unsubscribe URLs. Used by the + bundle + digest paths (which can't honor per-category opt-outs + because they span multiple categories); flips + `email_opt_out_all = 1` rather than a per-category column. + +### Changed + +- **`backend/app/email.py`** — `EmailConfig` gains + `unsubscribe_mailto` (env: `EMAIL_UNSUBSCRIBE_MAILTO`, default + falls back to `EMAIL_FROM`). `_deliver` and `_send_bundle` + build envelopes through `build_envelope` and thread `kind` + + `notification_id` into the audit write. +- **`backend/app/email_otc.py`** + **`email_invite.py`** — both + call `build_envelope` and `record_outbound`. OTC carries no + `List-Unsubscribe` (recipient explicitly requested the code); + invite carries `List-Unsubscribe: ` only (no signed + URL — the invitee isn't a user yet, no per-user opt-out row + exists). +- **`backend/app/digest.py`** — calls `_deliver` with `kind='digest'` + + the new `all`-category one-click unsubscribe. +- **`backend/app/webhooks.py`** — refuses 500 at request time if + the secret is empty + bypass isn't set; logs a loud warning + per-request when running under the bypass; logs INFO when a + hook targets a repo not in `cached_rfcs`. +- **`backend/app/config.py`** — `load_config()` raises + RuntimeError if `GITEA_WEBHOOK_SECRET` is empty unless + `RFC_APP_INSECURE_WEBHOOKS=1`. +- **`backend/app/api_notifications.py`** — GET unsubscribe handler + accepts the `all` category (sets `email_opt_out_all = 1`). + Bounce webhook body adds optional `message_id` field; response + shape adds `correlated_id` field. (Tests that read the exact + response shape — currently just + `test_bounce_webhook_refuses_unsigned_when_secret_configured` + in test_e2e_smoke.py — updated to assert on the new shape.) +- **`backend/tests/test_propose_vertical.py`** — the shared + `tmp_env` fixture binds a fake `GITEA_WEBHOOK_SECRET` so all + 252 pre-v0.18.0 tests boot cleanly under the new mandatory + secret. Tests that want to exercise the dev-bypass path + monkeypatch `RFC_APP_INSECURE_WEBHOOKS=1` explicitly. + +### Tests + +- 15 new unit tests in `test_email_envelope.py` for the helper. +- 10 new integration tests across `test_otc_vertical`, + `test_admin_create_user_invite_vertical`, and + `test_notifications_vertical` covering: OTC has no + List-Unsubscribe; invite has mailto: only; notification has + full one-click; POST one-click flips per-category; `all` flips + global; respects `EMAIL_UNSUBSCRIBE_MAILTO` override. +- 7 new integration tests in `test_webhooks_vertical.py` covering + the startup-time mandatory-secret check, the dev-bypass, and + the request-time signature verification including the unknown- + repo log line. +- 11 new integration tests in `test_outbound_emails_vertical.py` + covering the audit table write path (OTC / invite / notification), + the admin endpoint (list, filter by kind, filter by to_address, + non-admin refusal), and the bounce correlation (matched + message_id stamps status='bounced'; unknown message_id is + logged; absent message_id falls back to legacy behavior; bounced + rows surface in admin endpoint with `?status=bounced`). +- One test updated for intentional response-shape change: + `test_e2e_smoke.test_bounce_webhook_refuses_unsigned_when_secret_configured`. + +Full suite: 295 passed (was 252 pre-v0.18.0). + +### Migration + +- **`backend/migrations/020_outbound_emails.sql`** — auto-applied + on next backend start. Single new table with three indexes; no + changes to existing tables. + +### Upgrade steps (from 0.17.0) + +- Operators **MUST** ensure `GITEA_WEBHOOK_SECRET` is set in the + deployment's env. The framework now refuses to start if it's + empty. (For OHM-flotilla deployments, + `flotilla secret list ` confirms the binding; OHM + has carried this binding since v0.14.0, so the upgrade is + gesture-free for OHM specifically.) +- Operators **MAY** set `RFC_APP_INSECURE_WEBHOOKS=1` to bypass + the requirement in local-dev environments. Production + deployments **MUST NOT** set this; if they do, every webhook + POST logs a loud warning line per request. +- Operators **MAY** set `EMAIL_UNSUBSCRIBE_MAILTO` to route + `List-Unsubscribe: ` opt-out courtesy mail to a + humans-monitored mailbox distinct from the no-reply + `EMAIL_FROM` sender. Default falls back to `EMAIL_FROM`. +- You **MUST** apply schema migration + `020_outbound_emails.sql`. The migration creates a single new + table with three indexes; the framework runs migrations + automatically at process start, so no manual step is required + beyond restarting the backend so the migration runner picks + the file up. Existing deployments pick it up on first start + after upgrade with no operator action required. +- You **MUST** rebuild the frontend and restart the backend + after upgrading. `frontend/package.json#version` and `VERSION` + both move to `0.18.0`. No new secrets (the `outbound_emails` + table writes synchronously to the same SQLite file as every + other write). +- Operators **SHOULD** run a `mail-tester.com` probe against the + upgraded deployment to confirm the new envelope headers + (`Date`, `Message-ID`, `Auto-Submitted`, `List-Unsubscribe`, + `List-Unsubscribe-Post`) land cleanly with the upstream SMTP + relay's DKIM signing. The expected delta from pre-v0.18.0 is + +2-3 points on the spam-score axis (typical 5-6/10 baseline + → 9+/10 post-upgrade). + + ## 0.17.0 — 2026-05-28 **Minor — schema migration required; no new env vars; no new secrets.** diff --git a/VERSION b/VERSION index c5523bd..6633391 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.17.0 +0.18.0 diff --git a/backend/app/api_admin.py b/backend/app/api_admin.py index 51c1b3c..f2edf23 100644 --- a/backend/app/api_admin.py +++ b/backend/app/api_admin.py @@ -672,6 +672,73 @@ def make_router(config: Config) -> APIRouter: "has_more": len(rows) == limit, } + @router.get("/api/admin/outbound-emails") + async def list_outbound_emails( + request: Request, + kind: str | None = None, + status: str | None = None, + to_address: str | None = None, + limit: int = Query(default=100, ge=1, le=500), + before_id: int | None = None, + ) -> dict[str, Any]: + """v0.18.0 Slice 4: read-only inspection of the + `outbound_emails` audit table. + + Answers questions like "did this person ever get their + invite?" without grepping VM logs. Filterable by kind + ('otc' | 'invite' | 'notification' | 'bundle' | 'digest'), + status ('sent' | 'failed' | 'deferred' | 'bounced'), and + to_address; the latter is exact-match because the audit + question is usually "the specific person who said they + didn't receive it." Per the proposal, no admin UI ships + with v0.18.0 — operator queries via curl + jq for now. + """ + auth.require_admin(request) + clauses: list[str] = [] + args: list[Any] = [] + if kind: + clauses.append("kind = ?") + args.append(kind) + if status: + clauses.append("status = ?") + args.append(status) + if to_address: + clauses.append("LOWER(to_address) = LOWER(?)") + args.append(to_address) + if before_id is not None: + clauses.append("id < ?") + args.append(before_id) + where = ("WHERE " + " AND ".join(clauses)) if clauses else "" + rows = db.conn().execute( + f""" + SELECT id, to_address, from_address, subject, kind, sent_at, + status, error, notification_id, message_id + FROM outbound_emails + {where} + ORDER BY id DESC + LIMIT ? + """, + (*args, limit), + ).fetchall() + return { + "items": [ + { + "id": r["id"], + "to_address": r["to_address"], + "from_address": r["from_address"], + "subject": r["subject"], + "kind": r["kind"], + "sent_at": r["sent_at"], + "status": r["status"], + "error": r["error"], + "notification_id": r["notification_id"], + "message_id": r["message_id"], + } + for r in rows + ], + "has_more": len(rows) == limit, + } + @router.get("/api/admin/permission-events") async def list_permission_events( request: Request, diff --git a/backend/app/api_notifications.py b/backend/app/api_notifications.py index c431b43..a97558d 100644 --- a/backend/app/api_notifications.py +++ b/backend/app/api_notifications.py @@ -73,6 +73,13 @@ class MarkReadBody(BaseModel): class BounceBody(BaseModel): email: str = Field(min_length=3, max_length=320) kind: str = Field(default="hard") # 'hard' or 'complaint' + # v0.18.0 Slice 5: when the bounce provider includes the + # original Message-ID, the framework correlates it back to + # the matching `outbound_emails` row and stamps + # `status='bounced'`. Optional — providers that don't surface + # the Message-ID still flip the global opt-out via the email + # match, but lose the per-message attribution. + message_id: str | None = Field(default=None, max_length=1000) class CookieConsentBody(BaseModel): @@ -443,6 +450,40 @@ def make_router(config: Config) -> APIRouter: # ----- Email: one-click unsubscribe + bounce webhook ----- + # v0.18.0: the category → column map. The `all` synthetic + # category lands the bundle's one-click on the global opt-out + # flag (per `email._send_bundle` in v0.18.0 Slice 2 — a bundle + # spans multiple categories, so a per-category flip wouldn't + # honor the user's intent). + _CATEGORY_COLUMN: dict[str, str] = { + "personal-direct": "email_personal_direct", + "structural": "email_watched_structural", + "admin-actionable": "email_admin_actionable", + "all": "email_opt_out_all", + } + + def _apply_unsubscribe(user_id: int, category: str) -> bool: + """Flip the matching column. Returns True on success, False + if the category is unknown. Idempotent — running twice on + the same (user, category) is harmless (it sets the column + to its current value).""" + column = _CATEGORY_COLUMN.get(category) + if column is None: + return False + # `all` sets the flag to 1 (opt out); per-category sets to 0 + # (turn that category off). The column semantic is "1 means + # don't send"; the per-category booleans are "1 means do + # send". Different polarities, hence the case split. + if category == "all": + db.conn().execute( + f"UPDATE users SET {column} = 1 WHERE id = ?", (user_id,) + ) + else: + db.conn().execute( + f"UPDATE users SET {column} = 0 WHERE id = ?", (user_id,) + ) + return True + @router.get("/api/email/unsubscribe") async def email_unsubscribe(t: str = Query(..., description="Signed token from the email footer")) -> HTMLResponse: try: @@ -453,20 +494,51 @@ def make_router(config: Config) -> APIRouter: "

Open the app to manage your notification preferences directly.

", status_code=400, ) - column = { - "personal-direct": "email_personal_direct", - "structural": "email_watched_structural", - "admin-actionable": "email_admin_actionable", - }.get(category) - if column is None: + if not _apply_unsubscribe(user_id, category): return HTMLResponse( f"

Unknown category

{category}

", status_code=400 ) - db.conn().execute(f"UPDATE users SET {column} = 0 WHERE id = ?", (user_id,)) - return HTMLResponse( - f"

Unsubscribed

You will no longer receive {category} emails. " - f"You can re-enable them in your notification preferences.

" - ) + if category == "all": + body = ( + "

Unsubscribed

You will no longer receive any email " + "from this app. You can re-enable individual categories from " + "your notification preferences after signing in.

" + ) + else: + body = ( + f"

Unsubscribed

You will no longer receive {category} emails. " + f"You can re-enable them in your notification preferences.

" + ) + return HTMLResponse(body) + + @router.post("/api/email/unsubscribe") + async def email_unsubscribe_post( + request: Request, + t: str = Query(..., description="Signed token from the List-Unsubscribe header"), + ) -> dict[str, Any]: + """v0.18.0: RFC 8058 one-click endpoint. + + Gmail and Yahoo POST `List-Unsubscribe=One-Click` (as a + form-encoded body) to the URL in the `List-Unsubscribe` + header when the user clicks their MUA's "Unsubscribe" + button. The endpoint MUST accept POST (per the + `List-Unsubscribe-Post` header we advertise) and MUST be + idempotent. + + The body content is checked loosely — RFC 8058 says it + SHOULD be exactly `List-Unsubscribe=One-Click`, but some + intermediaries strip / re-encode the body, so the + framework accepts any POST to the URL once the token + verifies. The bar is that the token signature carries the + authority; the body is hint-only. + """ + try: + user_id, category = email_mod.verify_unsubscribe_token(t) + except BadSignature: + raise HTTPException(400, "Invalid or expired token") + if not _apply_unsubscribe(user_id, category): + raise HTTPException(400, f"Unknown category: {category}") + return {"ok": True, "category": category} @router.post("/api/webhooks/email-bounce") async def email_bounce(body: BounceBody, request: Request) -> dict[str, Any]: @@ -490,16 +562,46 @@ def make_router(config: Config) -> APIRouter: import hmac as _hmac if not received or not _hmac.compare_digest(expected, received): raise HTTPException(401, "Invalid webhook signature") + # v0.18.0 Slice 5: correlate the bounce back to the + # matching outbound_emails row if the provider supplied + # the Message-ID. The hard-bounce -> global-opt-out + # logic below still fires regardless; this is an + # additional audit signal. + correlated_row_id: int | None = None + if body.message_id: + correlated = db.conn().execute( + "SELECT id FROM outbound_emails WHERE message_id = ?", + (body.message_id,), + ).fetchone() + if correlated is not None: + correlated_row_id = correlated["id"] + db.conn().execute( + "UPDATE outbound_emails SET status = 'bounced', " + "error = COALESCE(error, '') || ? WHERE id = ?", + (f"bounce ({body.kind})", correlated_row_id), + ) + log.info( + "email-bounce: correlated message_id=%s -> outbound_emails.id=%s", + body.message_id, correlated_row_id, + ) + else: + log.info( + "email-bounce: message_id=%s did not match any " + "outbound_emails row (provider may be replaying an old bounce, " + "or the row was pruned)", + body.message_id, + ) + row = db.conn().execute( "SELECT id FROM users WHERE LOWER(email) = LOWER(?)", (body.email,), ).fetchone() if row is None: - return {"ok": True, "matched": False} + return {"ok": True, "matched": False, "correlated_id": correlated_row_id} db.conn().execute( "UPDATE users SET email_opt_out_all = 1 WHERE id = ?", (row["id"],), ) log.info("email-bounce: opted out user %s (%s)", row["id"], body.kind) - return {"ok": True, "matched": True} + return {"ok": True, "matched": True, "correlated_id": correlated_row_id} return router diff --git a/backend/app/config.py b/backend/app/config.py index 5417a53..e1c1bdd 100644 --- a/backend/app/config.py +++ b/backend/app/config.py @@ -60,6 +60,20 @@ def load_config() -> Config: enabled = [m.strip() for m in _optional("ENABLED_MODELS", "claude").split(",") if m.strip()] + # v0.18.0: `GITEA_WEBHOOK_SECRET` is now mandatory (per the + # email + webhook hygiene proposal). An empty value used to + # silently accept unsigned webhook POSTs — that was the + # invisible-failure shape the proposal targets. Now the + # framework refuses to start when the secret is empty unless + # the operator opts into the dev-bypass with + # `RFC_APP_INSECURE_WEBHOOKS=1`. Local-dev deployments without + # a wired Gitea hook set the bypass; production MUST NOT. + insecure_webhooks = os.environ.get("RFC_APP_INSECURE_WEBHOOKS", "").strip() == "1" + if insecure_webhooks: + webhook_secret = _optional("GITEA_WEBHOOK_SECRET") + else: + webhook_secret = _required("GITEA_WEBHOOK_SECRET") + return Config( gitea_url=_required("GITEA_URL").rstrip("/"), gitea_bot_user=_required("GITEA_BOT_USER"), @@ -72,7 +86,7 @@ def load_config() -> Config: secret_key=_required("SECRET_KEY"), database_path=database_path, owner_gitea_login=_optional("OWNER_GITEA_LOGIN"), - webhook_secret=_optional("GITEA_WEBHOOK_SECRET"), + webhook_secret=webhook_secret, enabled_models=enabled, anthropic_api_key=_optional("ANTHROPIC_API_KEY"), google_api_key=_optional("GOOGLE_API_KEY"), diff --git a/backend/app/digest.py b/backend/app/digest.py index 3af63b0..9a3ea13 100644 --- a/backend/app/digest.py +++ b/backend/app/digest.py @@ -180,7 +180,19 @@ def assemble_for_user( subject = _subject(eligible, cadence) body = _body(eligible, cadence, cfg) - sent = email_mod._deliver(cfg, email, subject, body) + # v0.18.0: the digest is the bulk-adjacent surface par excellence + # (it can carry weeks of accumulated activity), so it gets the + # full one-click unsubscribe to the global opt-out. Per-category + # opt-outs are managed from the preferences page; this footer is + # the "stop sending me anything" escape hatch Gmail and Yahoo + # expect for senders at this tier. + unsubscribe_url = email_mod.make_unsubscribe_url(user_id, "all") + sent = email_mod._deliver( + cfg, email, subject, body, + unsubscribe_mailto=cfg.unsubscribe_mailto, + unsubscribe_url=unsubscribe_url, + kind="digest", + ) if not sent: return False ids = [r["id"] for r, _ in eligible] diff --git a/backend/app/email.py b/backend/app/email.py index fdd0e93..79c64fb 100644 --- a/backend/app/email.py +++ b/backend/app/email.py @@ -24,7 +24,6 @@ import os import smtplib from dataclasses import dataclass from datetime import datetime, time, timezone -from email.message import EmailMessage from email.utils import formataddr from itertools import groupby from typing import Any @@ -33,6 +32,7 @@ from urllib.parse import urlencode from itsdangerous import BadSignature, URLSafeSerializer from . import db +from .email_envelope import build_envelope log = logging.getLogger(__name__) @@ -69,6 +69,7 @@ class EmailConfig: app_url: str bundle_threshold: int enabled: bool + unsubscribe_mailto: str @classmethod def from_env(cls) -> "EmailConfig": @@ -84,6 +85,16 @@ class EmailConfig: app_url=os.environ.get("APP_URL", "http://localhost:8000").rstrip("/"), bundle_threshold=int(os.environ.get("EMAIL_BUNDLE_THRESHOLD", "5")), enabled=os.environ.get("EMAIL_ENABLED", "1") not in ("0", "false", "False"), + # v0.18.0: the `List-Unsubscribe: ` target on + # invite + notification mail. Defaults to the From + # address when unset; a deployment can route opt-out + # mail to a separate mailbox (e.g., a humans-monitored + # account distinct from the no-reply notifications + # sender) by setting this explicitly. + unsubscribe_mailto=os.environ.get( + "EMAIL_UNSUBSCRIBE_MAILTO", + os.environ.get("EMAIL_FROM", "notifications@wiggleverse.local"), + ).strip(), ) @@ -98,6 +109,14 @@ def _signer() -> URLSafeSerializer: def make_unsubscribe_url(user_id: int, category: str) -> str: + """Build the §15.4 per-category one-click URL. + + `category` is one of `personal-direct`, `structural`, + `admin-actionable` (the three per-category flags) or `all` + (v0.18.0: the bundle path, which sets `email_opt_out_all = 1` + because a bundle covers multiple categories and a per-category + opt-out wouldn't honor the user's intent). + """ cfg = EmailConfig.from_env() token = _signer().dumps({"u": user_id, "c": category}) qs = urlencode({"t": token}) @@ -250,7 +269,21 @@ def _send_one(user: Any, notif_id: int, payload: dict, category: str) -> None: return subject = _subject(payload) body = _body(payload, user["id"], category, cfg) - sent = _deliver(cfg, user["email"], subject, body) + # v0.18.0: notification mail is bulk-adjacent (a watcher can + # accumulate dozens of structural events on a busy RFC), so it + # carries the full one-click unsubscribe — Gmail and Yahoo + # require this for senders at OHM's volume tier per RFC 8058. + unsubscribe_url = make_unsubscribe_url(user["id"], category) + sent = _deliver( + cfg, + user["email"], + subject, + body, + unsubscribe_mailto=cfg.unsubscribe_mailto, + unsubscribe_url=unsubscribe_url, + kind="notification", + notification_id=notif_id, + ) if not sent: return db.conn().execute( @@ -305,23 +338,65 @@ def _deep_link(payload: dict, cfg: EmailConfig) -> str: return cfg.app_url -def _deliver(cfg: EmailConfig, to_address: str, subject: str, body: str) -> bool: +def _deliver( + cfg: EmailConfig, + to_address: str, + subject: str, + body: str, + *, + unsubscribe_mailto: str | None = None, + unsubscribe_url: str | None = None, + kind: str = "notification", + notification_id: int | None = None, +) -> bool: + """Build the envelope via the shared `build_envelope` helper and + hand it to SMTP. + + The `_SENT` buffer carries the helper's `EmailMessage` under + `message` plus the legacy `to`/`from`/`subject`/`body` keys for + backward-compatibility with tests that read those directly. + Newer tests can assert on the header surface by inspecting + `envelope["message"]`. + + v0.18.0 Slice 4: also writes one row to `outbound_emails` + capturing the attempt. status='sent' on success, 'failed' on + SMTP exception, 'deferred' on the dev-fallback path (no + SMTP_HOST configured — the send didn't happen, but the row + records the attempt so the admin endpoint can answer "did the + framework try?"). + """ + msg = build_envelope( + to_address=to_address, + from_address=cfg.from_address, + from_name=cfg.from_name, + subject=subject, + body_plain=body, + unsubscribe_mailto=unsubscribe_mailto, + unsubscribe_url=unsubscribe_url, + ) envelope = { "to": to_address, "from": formataddr((cfg.from_name, cfg.from_address)), "subject": subject, "body": body, + "message": msg, + "kind": kind, } _SENT.append(envelope) + message_id = msg["Message-ID"] if not cfg.smtp_host: log.info("email (stdout fallback): to=%s subject=%s", to_address, subject) + record_outbound( + to_address=to_address, + from_address=cfg.from_address, + subject=subject, + kind=kind, + status="deferred", + message_id=message_id, + notification_id=notification_id, + ) return True try: - msg = EmailMessage() - msg["From"] = envelope["from"] - msg["To"] = to_address - msg["Subject"] = subject - msg.set_content(body) smtp = smtplib.SMTP(cfg.smtp_host, cfg.smtp_port, timeout=30) try: if cfg.smtp_starttls: @@ -331,12 +406,78 @@ def _deliver(cfg: EmailConfig, to_address: str, subject: str, body: str) -> bool smtp.send_message(msg) finally: smtp.quit() + record_outbound( + to_address=to_address, + from_address=cfg.from_address, + subject=subject, + kind=kind, + status="sent", + message_id=message_id, + notification_id=notification_id, + ) return True - except Exception: + except Exception as exc: log.exception("email send failed: to=%s subject=%s", to_address, subject) + record_outbound( + to_address=to_address, + from_address=cfg.from_address, + subject=subject, + kind=kind, + status="failed", + error=f"{type(exc).__name__}: {exc}", + message_id=message_id, + notification_id=notification_id, + ) return False +def record_outbound( + *, + to_address: str, + from_address: str, + subject: str, + kind: str, + status: str, + error: str | None = None, + notification_id: int | None = None, + message_id: str | None = None, +) -> int | None: + """v0.18.0 Slice 4: write one row to `outbound_emails`. + + Returns the inserted row's id, or `None` if the DB connection + isn't initialized (which happens in unit tests that don't boot + the full app — the write is best-effort and never raises). + """ + try: + cur = db.conn().execute( + """ + INSERT INTO outbound_emails + (to_address, from_address, subject, kind, sent_at, status, + error, notification_id, message_id) + VALUES (?, ?, ?, ?, datetime('now'), ?, ?, ?, ?) + """, + ( + to_address, + from_address, + subject, + kind, + status, + error, + notification_id, + message_id, + ), + ) + return cur.lastrowid + except RuntimeError: + # db.conn() raises RuntimeError if init() hasn't been called. + # Pure-helper unit tests for build_envelope hit this path; the + # audit row is best-effort and not part of the contract. + return None + except Exception: + log.exception("outbound_emails write failed: to=%s subject=%s", to_address, subject) + return None + + # --------------------------------------------------------------------------- # Quiet-hours release pass — called from the digest job # --------------------------------------------------------------------------- @@ -440,13 +581,27 @@ def _send_bundle(cfg: EmailConfig, user: Any, emailable: list) -> int: for r, _cat, extras in group_rows: summary = _summary_for(r["event_kind"], r["actor_display"], r["rfc_title"], extras) sections.append(f" · {summary}") + # v0.18.0: the bundle covers multiple categories, so a + # per-category opt-out can't honor the user's intent. The + # `all` category lands at the §15.4 endpoint and sets + # `email_opt_out_all = 1`. + unsubscribe_url = make_unsubscribe_url(user["id"], "all") body = ( "Activity on RFCs you watch, accumulated during your quiet hours:\n" + "\n".join(sections) + f"\n\nOpen your inbox: {cfg.app_url}/inbox\n" + f"Manage all preferences: {cfg.app_url}/settings/notifications\n" + + f"Unsubscribe from all email: {unsubscribe_url}\n" + ) + sent = _deliver( + cfg, + user["email"], + subject, + body, + unsubscribe_mailto=cfg.unsubscribe_mailto, + unsubscribe_url=unsubscribe_url, + kind="bundle", ) - sent = _deliver(cfg, user["email"], subject, body) if not sent: return 0 ids = [r["id"] for r, _, _ in emailable] diff --git a/backend/app/email_envelope.py b/backend/app/email_envelope.py new file mode 100644 index 0000000..78d011e --- /dev/null +++ b/backend/app/email_envelope.py @@ -0,0 +1,143 @@ +"""v0.18.0 / roadmap items #18 + #20: a shared envelope builder. + +Every outbound mail in rfc-app today (OTC, admin-invite, watcher +notification, "while you were away" bundle, per-RFC invite) constructs +its own `email.message.EmailMessage` ad-hoc. The four sites diverged +just enough to be a deliverability hazard: missing `Date`, missing +`Message-ID`, no `Auto-Submitted`, no `List-Unsubscribe` on the +bulk-adjacent paths, no `multipart/alternative` body. + +This module is the one place an `EmailMessage` is constructed. Every +send path imports `build_envelope` and calls it; the headers that +matter for inbox placement (Date, Message-ID, Auto-Submitted) land +uniformly, and the per-kind variations (unsubscribe semantics, +HTML alternative) are explicit arguments rather than buried in +each call site. + +Per the v0.18.0 proposal at `~/git/ohm-infra/RFC-APP-EMAIL-HYGIENE-PROPOSAL.md`, +the per-kind unsubscribe matrix is: + + * OTC: no `List-Unsubscribe` (the recipient explicitly requested + the code; advertising an unsubscribe header would imply OHM has + them on a list, which it doesn't). + * Admin invite / per-RFC invite: `mailto:` form only (the + recipient isn't a user yet, so there's no per-user opt-out row + to flip; the operator handles ad-hoc opt-outs manually). + * Watcher notification / bundle: full `mailto:` + signed-URL + `List-Unsubscribe` plus `List-Unsubscribe-Post: + List-Unsubscribe=One-Click` per RFC 8058 (Gmail and Yahoo + enforce this for bulk-adjacent senders). + +The `is_transactional` flag governs `Auto-Submitted: auto-generated`, +which prevents auto-responder loops on every kind of mail we send. +All five mail kinds today are transactional in the SMTP sense (no +human is at the From mailbox watching for replies), so the default +is True; the argument is exposed for future symmetry. +""" +from __future__ import annotations + +from email.message import EmailMessage +from email.utils import formataddr, formatdate, make_msgid + + +def build_envelope( + *, + to_address: str, + from_address: str, + from_name: str, + subject: str, + body_plain: str, + body_html: str | None = None, + reply_to: str | None = None, + unsubscribe_mailto: str | None = None, + unsubscribe_url: str | None = None, + is_transactional: bool = True, + msgid_domain: str | None = None, +) -> EmailMessage: + """Compose an `EmailMessage` with hardened headers. + + `to_address` / `from_address` are bare RFC 5322 addresses; + `from_name` is the display label that goes through `formataddr` + so spaces / commas in the display string are encoded correctly. + + `body_plain` is mandatory. `body_html`, if supplied, lands as the + second part of a `multipart/alternative` body — mail clients + that prefer HTML render it; clients that don't fall back to the + plain part. The text/plain part comes first per RFC 2046, so a + plain-text client that picks the first body gets the readable + text. + + `reply_to`, when set, lets a send path point replies at a + different mailbox than the From line (e.g., a watcher + notification with From=notifications@... but Reply-To= + ohm@... so a confused recipient who hits Reply lands at a + monitored mailbox). + + `unsubscribe_mailto` / `unsubscribe_url` populate + `List-Unsubscribe`. If `unsubscribe_url` is set, the helper also + emits `List-Unsubscribe-Post: List-Unsubscribe=One-Click` per + RFC 8058 — Gmail and Yahoo POST that payload on the user's + one-click action. (Send paths that wire `unsubscribe_url` + therefore MUST also expose a matching POST endpoint that accepts + the same token; see `api_notifications.py:email_unsubscribe`.) + + `msgid_domain` defaults to the @-domain of `from_address` so + Message-IDs are aligned with the sending domain by default. A + deployment that wants the Message-ID domain to track a different + surface (e.g., a tracking-domain that's separate from the From + domain) can override. + + `Date` is RFC 5322 formatted via `email.utils.formatdate`; the + `localtime=True` setting picks the running process's local + timezone, which is what every popular MUA does too. (A + deployment running in UTC stamps UTC; that's correct, not a + bug.) + """ + msg = EmailMessage() + msg["From"] = formataddr((from_name, from_address)) + msg["To"] = to_address + msg["Subject"] = subject + msg["Date"] = formatdate(localtime=True) + # If the caller didn't pin a Message-ID domain, derive it from the + # From address. `make_msgid` accepts None and falls back to the + # local hostname, which is the wrong shape for a deliverable + # message (the hostname might be `gke-pool-xxx`); a deployment + # without a configured From would surface that as a build-time + # config error elsewhere, so the fallback here is just defensive. + if msgid_domain is None: + if "@" in from_address: + msgid_domain = from_address.split("@", 1)[1] + else: + msgid_domain = "localhost" + msg["Message-ID"] = make_msgid(domain=msgid_domain) + if reply_to: + msg["Reply-To"] = reply_to + if is_transactional: + # RFC 3834: prevents auto-responders (vacation replies, etc.) + # from triggering on this message. Every kind of mail rfc-app + # sends today is transactional in this sense. + msg["Auto-Submitted"] = "auto-generated" + if unsubscribe_mailto or unsubscribe_url: + parts: list[str] = [] + if unsubscribe_mailto: + parts.append(f"") + if unsubscribe_url: + parts.append(f"<{unsubscribe_url}>") + msg["List-Unsubscribe"] = ", ".join(parts) + if unsubscribe_url: + # RFC 8058 one-click. Gmail and Yahoo POST the payload + # `List-Unsubscribe=One-Click` to the URL on the user's + # one-click action; the matching POST endpoint must be + # idempotent and not require auth. See + # `api_notifications.py` for the receiver. + msg["List-Unsubscribe-Post"] = "List-Unsubscribe=One-Click" + if body_html: + # multipart/alternative: text/plain first, text/html second. + # `set_content` sets the first part (and the message's main + # body); `add_alternative` adds the second part and + # restructures the message as multipart/alternative. + msg.set_content(body_plain) + msg.add_alternative(body_html, subtype="html") + else: + msg.set_content(body_plain) + return msg diff --git a/backend/app/email_invite.py b/backend/app/email_invite.py index 9b9d96f..32ce6f9 100644 --- a/backend/app/email_invite.py +++ b/backend/app/email_invite.py @@ -31,10 +31,10 @@ from __future__ import annotations import logging import smtplib -from email.message import EmailMessage from email.utils import formataddr -from .email import EmailConfig, _SENT +from .email import EmailConfig, _SENT, record_outbound +from .email_envelope import build_envelope log = logging.getLogger(__name__) @@ -59,31 +59,52 @@ def send_invite_email( cfg = EmailConfig.from_env() subject = _subject(inviter_display, cfg) body = _body(claim_url, inviter_display, inviter_email, custom_message, cfg) + # v0.18.0: invite mail carries a `List-Unsubscribe: ` + # only (no signed URL) — the invitee isn't a user yet, so there + # is no per-user opt-out row to flip. The operator handles + # ad-hoc opt-outs from the mailto: target. Per the proposal's + # "Tradeoff discussion": the invite was unsolicited from the + # recipient's perspective, so the courtesy header is right; + # but it can't be a one-click URL because the row doesn't + # exist yet. + msg = build_envelope( + to_address=to_address, + from_address=cfg.from_address, + from_name=cfg.from_name, + subject=subject, + body_plain=body, + unsubscribe_mailto=cfg.unsubscribe_mailto, + ) envelope = { "to": to_address, "from": formataddr((cfg.from_name, cfg.from_address)), "subject": subject, "body": body, "kind": "invite", + "message": msg, } _SENT.append(envelope) + message_id = msg["Message-ID"] if not cfg.enabled: log.info("invite email disabled (EMAIL_ENABLED=0): to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="deferred", message_id=message_id, + ) return True if not cfg.smtp_host: # Dev fallback: surface the claim URL at INFO so the operator can # complete a claim flow without an SMTP relay. In production # SMTP_HOST is always set per OHM's overlay. log.info("invite email (stdout fallback): to=%s claim_url=%s", to_address, claim_url) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="deferred", message_id=message_id, + ) return True try: - msg = EmailMessage() - msg["From"] = envelope["from"] - msg["To"] = to_address - msg["Subject"] = subject - msg.set_content(body) smtp = smtplib.SMTP(cfg.smtp_host, cfg.smtp_port, timeout=30) try: if cfg.smtp_starttls: @@ -93,9 +114,18 @@ def send_invite_email( smtp.send_message(msg) finally: smtp.quit() + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="sent", message_id=message_id, + ) return True - except Exception: + except Exception as exc: log.exception("invite email send failed: to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="invite", status="failed", + error=f"{type(exc).__name__}: {exc}", message_id=message_id, + ) return False diff --git a/backend/app/email_otc.py b/backend/app/email_otc.py index b2f59d9..52ccfee 100644 --- a/backend/app/email_otc.py +++ b/backend/app/email_otc.py @@ -23,10 +23,10 @@ from __future__ import annotations import logging import smtplib -from email.message import EmailMessage from email.utils import formataddr -from .email import EmailConfig, _SENT +from .email import EmailConfig, _SENT, record_outbound +from .email_envelope import build_envelope log = logging.getLogger(__name__) @@ -44,31 +44,48 @@ def send_otc_email(to_address: str, code: str) -> bool: cfg = EmailConfig.from_env() subject = f"Your sign-in code for {cfg.from_name}" body = _body(code, cfg) + # v0.18.0: OTC mail carries NO List-Unsubscribe — the recipient + # explicitly requested the code; advertising an unsubscribe + # header would imply OHM has them on a list, which it doesn't. + # See the proposal's "Tradeoff discussion" for the binding + # rationale. + msg = build_envelope( + to_address=to_address, + from_address=cfg.from_address, + from_name=cfg.from_name, + subject=subject, + body_plain=body, + ) envelope = { "to": to_address, "from": formataddr((cfg.from_name, cfg.from_address)), "subject": subject, "body": body, "kind": "otc", + "message": msg, } _SENT.append(envelope) + message_id = msg["Message-ID"] if not cfg.enabled: log.info("otc email disabled (EMAIL_ENABLED=0): to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="deferred", message_id=message_id, + ) return True if not cfg.smtp_host: # Dev fallback: surface the code at INFO so the operator can # complete a sign-in flow without an SMTP relay. In production # SMTP_HOST is always set per OHM's overlay. log.info("otc email (stdout fallback): to=%s code=%s", to_address, code) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="deferred", message_id=message_id, + ) return True try: - msg = EmailMessage() - msg["From"] = envelope["from"] - msg["To"] = to_address - msg["Subject"] = subject - msg.set_content(body) smtp = smtplib.SMTP(cfg.smtp_host, cfg.smtp_port, timeout=30) try: if cfg.smtp_starttls: @@ -78,9 +95,18 @@ def send_otc_email(to_address: str, code: str) -> bool: smtp.send_message(msg) finally: smtp.quit() + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="sent", message_id=message_id, + ) return True - except Exception: + except Exception as exc: log.exception("otc email send failed: to=%s", to_address) + record_outbound( + to_address=to_address, from_address=cfg.from_address, + subject=subject, kind="otc", status="failed", + error=f"{type(exc).__name__}: {exc}", message_id=message_id, + ) return False diff --git a/backend/app/webhooks.py b/backend/app/webhooks.py index 1ed714c..e47c167 100644 --- a/backend/app/webhooks.py +++ b/backend/app/webhooks.py @@ -12,6 +12,7 @@ import hashlib import hmac import json import logging +import os from fastapi import APIRouter, Header, HTTPException, Request @@ -40,7 +41,27 @@ def make_router(config: Config, gitea: Gitea) -> APIRouter: x_gitea_signature: str = Header(default=""), ): body = await request.body() - if config.webhook_secret: + # v0.18.0: defense in depth. config.py refuses to start + # when the secret is empty unless `RFC_APP_INSECURE_WEBHOOKS=1` + # is set; this branch catches the dev-bypass case (the only + # path where `config.webhook_secret` can be empty) and surfaces + # it loudly to the client. A POST that lands here with an + # empty secret on a production deployment indicates a + # mis-configuration (somebody flipped the bypass in prod), + # and the loud 500 is the proposal's whole point. + insecure = os.environ.get("RFC_APP_INSECURE_WEBHOOKS", "").strip() == "1" + if not config.webhook_secret: + if not insecure: + log.error( + "webhook receiver misconfigured: GITEA_WEBHOOK_SECRET is empty " + "and RFC_APP_INSECURE_WEBHOOKS=1 is not set" + ) + raise HTTPException(status_code=500, detail="Webhook receiver misconfigured") + log.warning( + "webhook receiver running with RFC_APP_INSECURE_WEBHOOKS=1 — " + "signature verification is DISABLED. Production deployments MUST NOT set this." + ) + else: if not _verify_signature(body, x_gitea_signature, config.webhook_secret): raise HTTPException(status_code=401, detail="Invalid signature") @@ -68,6 +89,17 @@ def make_router(config: Config, gitea: Gitea) -> APIRouter: slug = _slug_for_repo(repo_full) if slug: await cache.refresh_rfc_repo(config, gitea, slug) + else: + # v0.18.0: the proposal's "unknown-repo logging" + # gesture — a hook on a fork or a stale repo binding + # used to silently 200-OK here, hiding the + # misconfiguration. Now the operator sees it in + # the log. + log.info( + "webhook received for unknown repo: repo_full=%s event=%s " + "(no cached_rfcs row matched; hook may be on a fork or stale)", + repo_full, event, + ) except Exception: log.exception("webhook refresh failed") raise HTTPException(status_code=500, detail="Refresh failed") diff --git a/backend/migrations/020_outbound_emails.sql b/backend/migrations/020_outbound_emails.sql new file mode 100644 index 0000000..cb1783b --- /dev/null +++ b/backend/migrations/020_outbound_emails.sql @@ -0,0 +1,30 @@ +-- v0.18.0 Slice 4: outbound_emails audit table. +-- +-- Per the v0.18.0 email + webhook hygiene proposal §3, every send +-- helper writes a row to this table before returning, regardless +-- of outcome. status='sent' on success, 'failed' on exception, +-- 'deferred' on the dev-fallback path (no SMTP_HOST configured). +-- +-- The table is queried by `GET /api/admin/outbound-emails` to +-- answer "did this person ever get their invite?" without having +-- to grep VM logs, and by the v0.18.0 Slice 5 bounce-correlation +-- hook (which looks up message_id when a POST lands at +-- /api/webhooks/email-bounce and marks the matching row +-- status='bounced'). + +CREATE TABLE IF NOT EXISTS outbound_emails ( + id INTEGER PRIMARY KEY, + to_address TEXT NOT NULL, + from_address TEXT NOT NULL, + subject TEXT NOT NULL, + kind TEXT NOT NULL, -- 'otc' | 'invite' | 'notification' | 'bundle' | 'digest' | 'rfc-invite' + sent_at TEXT NOT NULL, -- ISO 8601, time the send was attempted + status TEXT NOT NULL, -- 'sent' | 'failed' | 'deferred' | 'bounced' + error TEXT, -- exception class + message if status='failed' + notification_id INTEGER, -- nullable FK to notifications.id for the watcher path + message_id TEXT -- the Message-ID header value, for bounce correlation +); + +CREATE INDEX IF NOT EXISTS idx_outbound_emails_to ON outbound_emails(to_address); +CREATE INDEX IF NOT EXISTS idx_outbound_emails_sent_at ON outbound_emails(sent_at); +CREATE INDEX IF NOT EXISTS idx_outbound_emails_message ON outbound_emails(message_id); diff --git a/backend/tests/test_admin_create_user_invite_vertical.py b/backend/tests/test_admin_create_user_invite_vertical.py index a160f2e..343e5fd 100644 --- a/backend/tests/test_admin_create_user_invite_vertical.py +++ b/backend/tests/test_admin_create_user_invite_vertical.py @@ -647,3 +647,82 @@ def test_pending_invites_listing_admin_only(app_with_fake_gitea): ) r = client.get("/api/admin/users/invites") assert r.status_code == 403 + + +# --------------------------------------------------------------------------- +# v0.18.0: invite-envelope header shape — Slice 2 +# +# Invite mail goes through `build_envelope` and MUST land Date, +# Message-ID, Auto-Submitted, AND a `List-Unsubscribe: ` +# (no URL — the invitee isn't a user yet, so no per-user opt-out +# row exists). The mailto: target is the operator's `EMAIL_FROM` +# by default; the operator can override via `EMAIL_UNSUBSCRIBE_MAILTO`. +# --------------------------------------------------------------------------- + + +def _provision_admin_and_send_invite(client, app_with_fake_gitea_fixture, *, to: str = "headers@ex.co"): + provision_user_row(user_id=400, login="adminH", role="admin") + sign_in_as( + client, user_id=400, gitea_login="adminH", + display_name="Admin H", role="admin", + email="adminh@test", + ) + _reset_outbound() + r = client.post( + "/api/admin/users", + json={ + "email": to, + "first_name": "Header", + "last_name": "Test", + "role": "contributor", + "custom_message": "", + }, + ) + assert r.status_code == 200, r.text + return _outbound_invite_envelopes(to)[-1] + + +def test_invite_envelope_sets_date_messageid_autosubmitted(app_with_fake_gitea): + from fastapi.testclient import TestClient + from email.utils import parsedate_to_datetime + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + env = _provision_admin_and_send_invite(client, (app, _fake)) + msg = env["message"] + assert parsedate_to_datetime(msg["Date"]) is not None + assert msg["Message-ID"].startswith("<") and msg["Message-ID"].endswith(">") + assert msg["Auto-Submitted"] == "auto-generated" + + +def test_invite_envelope_has_mailto_list_unsubscribe_only(app_with_fake_gitea): + """The invitee isn't a user yet — no per-user opt-out URL is + available. The `List-Unsubscribe` MUST be a mailto: form, and + the `List-Unsubscribe-Post` header MUST be absent (the + one-click semantic requires a URL the MUA can POST to).""" + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + env = _provision_admin_and_send_invite(client, (app, _fake)) + msg = env["message"] + lu = msg["List-Unsubscribe"] + assert lu is not None and lu.startswith("". + assert "OHM" in msg["From"] + assert "" in msg["From"] + + +def test_envelope_sets_date_header_parseable(): + msg = build_envelope(**_base_kwargs()) + raw = msg["Date"] + assert raw, "Date header must be set" + # parsedate_to_datetime raises ValueError on malformed input. + dt = parsedate_to_datetime(raw) + assert dt is not None + + +def test_envelope_sets_message_id_with_from_domain_by_default(): + msg = build_envelope(**_base_kwargs()) + mid = msg["Message-ID"] + assert mid, "Message-ID must be set" + # Shape per RFC 5322 / make_msgid: + assert mid.startswith("<") and mid.endswith(">") + assert "@ohm.wiggleverse.org>" in mid + + +def test_envelope_message_id_domain_override(): + msg = build_envelope(**_base_kwargs(msgid_domain="example.test")) + assert "@example.test>" in msg["Message-ID"] + + +def test_envelope_message_id_falls_back_to_localhost_if_from_has_no_at(): + # Defensive: a malformed from_address shouldn't crash the helper. + msg = build_envelope(**_base_kwargs(from_address="bare-no-at-sign")) + assert "@localhost>" in msg["Message-ID"] + + +# --------------------------------------------------------------------------- +# Auto-Submitted (RFC 3834) +# --------------------------------------------------------------------------- + + +def test_envelope_sets_auto_submitted_for_transactional_default(): + msg = build_envelope(**_base_kwargs()) + assert msg["Auto-Submitted"] == "auto-generated" + + +def test_envelope_omits_auto_submitted_when_transactional_is_false(): + msg = build_envelope(**_base_kwargs(is_transactional=False)) + assert msg["Auto-Submitted"] is None + + +# --------------------------------------------------------------------------- +# Reply-To +# --------------------------------------------------------------------------- + + +def test_envelope_sets_reply_to_when_provided(): + msg = build_envelope(**_base_kwargs(reply_to="ohm@wiggleverse.org")) + assert msg["Reply-To"] == "ohm@wiggleverse.org" + + +def test_envelope_omits_reply_to_when_absent(): + msg = build_envelope(**_base_kwargs()) + assert msg["Reply-To"] is None + + +# --------------------------------------------------------------------------- +# List-Unsubscribe (the headers RFC 8058 / Gmail-Yahoo care about) +# --------------------------------------------------------------------------- + + +def test_envelope_no_list_unsubscribe_when_neither_given(): + """OTC mail: the recipient explicitly requested the code; no + unsubscribe semantics. The header MUST be absent (presence would + imply OHM has the recipient on a list, which it doesn't).""" + msg = build_envelope(**_base_kwargs()) + assert msg["List-Unsubscribe"] is None + assert msg["List-Unsubscribe-Post"] is None + + +def test_envelope_mailto_only_list_unsubscribe(): + """Admin invite / per-RFC invite: `mailto:` form only, no URL. + The recipient isn't a user yet, so there's no per-user opt-out + URL to flip; the operator handles ad-hoc opt-outs manually.""" + msg = build_envelope(**_base_kwargs( + unsubscribe_mailto="ohm@wiggleverse.org?subject=remove", + )) + assert msg["List-Unsubscribe"] == "" + # NO List-Unsubscribe-Post when only a mailto is present — the + # one-click semantic requires a URL the MUA can POST to. + assert msg["List-Unsubscribe-Post"] is None + + +def test_envelope_full_one_click_list_unsubscribe(): + """Watcher notification / bundle: `mailto:` + signed-URL + + `List-Unsubscribe-Post: List-Unsubscribe=One-Click`. Gmail and + Yahoo enforce this for bulk-adjacent mail per RFC 8058.""" + msg = build_envelope(**_base_kwargs( + unsubscribe_mailto="ohm@wiggleverse.org?subject=remove", + unsubscribe_url="https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc", + )) + lu = msg["List-Unsubscribe"] + assert "" in lu + assert "" in lu + assert msg["List-Unsubscribe-Post"] == "List-Unsubscribe=One-Click" + + +def test_envelope_url_only_list_unsubscribe_still_sets_post(): + msg = build_envelope(**_base_kwargs( + unsubscribe_url="https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc", + )) + assert msg["List-Unsubscribe"] == "" + assert msg["List-Unsubscribe-Post"] == "List-Unsubscribe=One-Click" + + +# --------------------------------------------------------------------------- +# Body shape — plain-only vs multipart/alternative +# --------------------------------------------------------------------------- + + +def test_envelope_plain_only_body_is_text_plain(): + msg = build_envelope(**_base_kwargs()) + # No HTML alternative -> single-part text/plain. + assert msg.get_content_type() == "text/plain" + assert msg.get_content().strip() == "Hello, world." + + +def test_envelope_with_html_is_multipart_alternative(): + msg = build_envelope(**_base_kwargs(body_html="

Hello, world.

")) + assert msg.get_content_type() == "multipart/alternative" + # Two parts: text/plain first (so plain-text clients picking the + # first part get the readable text), text/html second. + parts = list(msg.iter_parts()) + assert len(parts) == 2 + assert parts[0].get_content_type() == "text/plain" + assert parts[1].get_content_type() == "text/html" + assert "Hello, world." in parts[0].get_content() + assert "world" in parts[1].get_content() diff --git a/backend/tests/test_notifications_vertical.py b/backend/tests/test_notifications_vertical.py index 926d846..acc0543 100644 --- a/backend/tests/test_notifications_vertical.py +++ b/backend/tests/test_notifications_vertical.py @@ -612,3 +612,127 @@ def test_explicit_watch_set_overrides_auto(app_with_fake_gitea): # the user put them. assert row["set_by"] == "explicit" assert row["state"] == "following" + + +# --------------------------------------------------------------------------- +# v0.18.0 — envelope headers + RFC 8058 one-click POST endpoint +# +# Watcher notifications are bulk-adjacent (a busy RFC can produce +# dozens of structural events); per the proposal, they MUST carry +# `Date`, `Message-ID`, `Auto-Submitted`, full `List-Unsubscribe` +# (mailto + signed URL), AND `List-Unsubscribe-Post: +# List-Unsubscribe=One-Click` per RFC 8058. Gmail and Yahoo +# enforce this for senders at OHM's volume tier. +# --------------------------------------------------------------------------- + + +def test_notification_envelope_carries_full_one_click_headers(app_with_fake_gitea): + """A `proposal_merged` event lands a watcher notification email + with the full one-click unsubscribe shape.""" + from fastapi.testclient import TestClient + from email.utils import parsedate_to_datetime + from app import db, email as email_mod + + app, fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="alice", role="contributor") + provision_user_row(user_id=1, login="ben", role="owner") + + sign_in_as(client, user_id=2, gitea_login="alice", display_name="Alice", role="contributor", email="alice@test") + r = client.post("/api/rfcs/propose", json={"title": "OHM", "slug": "ohm", "pitch": PITCH, "tags": []}) + assert r.status_code == 200 + email_mod.reset_sent_envelopes() + sign_in_as(client, user_id=1, gitea_login="ben", display_name="Ben", role="owner", email="ben@test") + merge_r = client.post(f"/api/proposals/{r.json()['pr_number']}/merge") + assert merge_r.status_code == 200, merge_r.text + + envelopes = [e for e in email_mod.sent_envelopes() if e["to"] == "alice@test"] + assert envelopes, "watcher notification did not fire" + msg = envelopes[-1]["message"] + # Always-present headers from the helper. + assert parsedate_to_datetime(msg["Date"]) is not None + assert msg["Message-ID"].startswith("<") and msg["Message-ID"].endswith(">") + assert msg["Auto-Submitted"] == "auto-generated" + # Full one-click unsubscribe. + lu = msg["List-Unsubscribe"] + assert lu is not None + assert "") + # Auto-Submitted prevents auto-responder loops. + assert msg["Auto-Submitted"] == "auto-generated" + + +def test_otc_envelope_has_no_list_unsubscribe(app_with_fake_gitea): + """The recipient explicitly typed their email and asked for a + code; the framework MUST NOT advertise a list semantic on this + mail. Per the v0.18.0 proposal's tradeoff discussion.""" + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + _reset_outbound() + client.post("/auth/otc/request", json={"email": "headers@example.com"}) + msg = _last_otc_envelope()["message"] + assert msg["List-Unsubscribe"] is None + assert msg["List-Unsubscribe-Post"] is None diff --git a/backend/tests/test_outbound_emails_vertical.py b/backend/tests/test_outbound_emails_vertical.py new file mode 100644 index 0000000..a130e74 --- /dev/null +++ b/backend/tests/test_outbound_emails_vertical.py @@ -0,0 +1,368 @@ +"""End-to-end integration tests for the v0.18.0 Slice 4 +outbound_emails audit table + admin endpoint. + +The release adds: + * `backend/migrations/020_outbound_emails.sql` — the audit table. + * `record_outbound()` in `email.py` — the write helper every send + path calls before returning, capturing status='sent' / 'failed' + / 'deferred' (the dev-fallback path when SMTP_HOST is unset). + * `GET /api/admin/outbound-emails` — admin-only listing, filterable + by kind / status / to_address. + +These tests prove: + * Sending OTC / invite / notification mail writes one row per send + (status='deferred' under tests since SMTP_HOST is unset). + * The Message-ID on the row matches the envelope's Message-ID + header (the seam Slice 5 uses for bounce correlation). + * `kind` is populated per send path. + * `GET /api/admin/outbound-emails` lists rows newest-first, + accepts filters, refuses non-admins. +""" +from __future__ import annotations + +from test_propose_vertical import ( # noqa: F401 + FakeGitea, + app_with_fake_gitea, + provision_user_row, + sign_in_as, + tmp_env, +) + + +# --------------------------------------------------------------------------- +# Write-on-send wiring +# --------------------------------------------------------------------------- + + +def test_otc_send_writes_outbound_row_with_message_id(app_with_fake_gitea): + from fastapi.testclient import TestClient + from app import db, email as email_mod + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + email_mod.reset_sent_envelopes() + r = client.post("/auth/otc/request", json={"email": "newcomer@ex.co"}) + assert r.status_code == 200 + + # Audit row landed. + rows = db.conn().execute( + "SELECT id, to_address, kind, status, message_id, error " + "FROM outbound_emails WHERE to_address = 'newcomer@ex.co'" + ).fetchall() + assert len(rows) == 1 + row = rows[0] + assert row["kind"] == "otc" + # No SMTP_HOST in tests -> 'deferred', not 'sent'. + assert row["status"] == "deferred" + assert row["error"] is None + # Message-ID matches the envelope's header (the seam Slice 5 uses). + envelopes = [e for e in email_mod.sent_envelopes() if e.get("kind") == "otc"] + assert envelopes + envelope_mid = envelopes[-1]["message"]["Message-ID"] + assert row["message_id"] == envelope_mid + + +def test_invite_send_writes_outbound_row(app_with_fake_gitea): + from fastapi.testclient import TestClient + from app import db + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=500, login="adminQ", role="admin") + sign_in_as( + client, user_id=500, gitea_login="adminQ", + display_name="Admin Q", role="admin", + email="adminq@test", + ) + r = client.post( + "/api/admin/users", + json={ + "email": "invitee@ex.co", + "first_name": "Inv", "last_name": "Itee", + "role": "contributor", "custom_message": "", + }, + ) + assert r.status_code == 200, r.text + + rows = db.conn().execute( + "SELECT kind, status, message_id FROM outbound_emails " + "WHERE to_address = 'invitee@ex.co'" + ).fetchall() + assert len(rows) == 1 + assert rows[0]["kind"] == "invite" + assert rows[0]["status"] == "deferred" + assert rows[0]["message_id"] is not None + + +def test_notification_send_writes_outbound_row_with_notification_id(app_with_fake_gitea): + """Watcher notifications carry a `notification_id` FK so the + admin can join through to the notifications table to see what + triggered the send.""" + from fastapi.testclient import TestClient + from app import db, email as email_mod + from test_notifications_vertical import PITCH + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=2, login="alice", role="contributor") + provision_user_row(user_id=1, login="ben", role="owner") + sign_in_as( + client, user_id=2, gitea_login="alice", + display_name="Alice", role="contributor", email="alice@test", + ) + r = client.post("/api/rfcs/propose", json={ + "title": "OHM", "slug": "ohm", "pitch": PITCH, "tags": [], + }) + email_mod.reset_sent_envelopes() + # Wipe pre-merge audit rows so the assertion below is unambiguous. + db.conn().execute("DELETE FROM outbound_emails") + sign_in_as( + client, user_id=1, gitea_login="ben", + display_name="Ben", role="owner", email="ben@test", + ) + merge_r = client.post(f"/api/proposals/{r.json()['pr_number']}/merge") + assert merge_r.status_code == 200, merge_r.text + + rows = db.conn().execute( + "SELECT kind, status, notification_id, message_id " + "FROM outbound_emails WHERE to_address = 'alice@test'" + ).fetchall() + assert rows, "no outbound_emails row for alice@test" + # At least one notification kind, with a populated FK. + notif_rows = [r for r in rows if r["kind"] == "notification"] + assert notif_rows + for nr in notif_rows: + assert nr["status"] == "deferred" + assert nr["notification_id"] is not None + assert nr["message_id"] is not None + + +# --------------------------------------------------------------------------- +# Admin endpoint +# --------------------------------------------------------------------------- + + +def test_admin_outbound_emails_lists_rows(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + # Generate a few rows. + client.post("/auth/otc/request", json={"email": "one@ex.co"}) + + provision_user_row(user_id=600, login="adminR", role="admin") + sign_in_as( + client, user_id=600, gitea_login="adminR", + display_name="Admin R", role="admin", email="adminr@test", + ) + client.post("/api/admin/users", json={ + "email": "two@ex.co", "first_name": "T", "last_name": "Wo", + "role": "contributor", "custom_message": "", + }) + + r = client.get("/api/admin/outbound-emails") + assert r.status_code == 200, r.text + items = r.json()["items"] + kinds = {it["kind"] for it in items} + assert "otc" in kinds + assert "invite" in kinds + # Newest-first. + ids = [it["id"] for it in items] + assert ids == sorted(ids, reverse=True) + + +def test_admin_outbound_emails_filters_by_kind(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + client.post("/auth/otc/request", json={"email": "filter1@ex.co"}) + + provision_user_row(user_id=601, login="adminS", role="admin") + sign_in_as( + client, user_id=601, gitea_login="adminS", + display_name="Admin S", role="admin", email="admins@test", + ) + client.post("/api/admin/users", json={ + "email": "filter2@ex.co", "first_name": "F", "last_name": "Two", + "role": "contributor", "custom_message": "", + }) + + r = client.get("/api/admin/outbound-emails?kind=otc") + assert r.status_code == 200 + items = r.json()["items"] + assert items + assert all(it["kind"] == "otc" for it in items) + + +def test_admin_outbound_emails_filters_by_to_address(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + client.post("/auth/otc/request", json={"email": "TARGET@ex.co"}) + client.post("/auth/otc/request", json={"email": "other@ex.co"}) + + provision_user_row(user_id=602, login="adminT", role="admin") + sign_in_as( + client, user_id=602, gitea_login="adminT", + display_name="Admin T", role="admin", email="admint@test", + ) + + # to_address filter is case-insensitive. + r = client.get("/api/admin/outbound-emails?to_address=target@ex.co") + assert r.status_code == 200 + items = r.json()["items"] + assert items + assert all(it["to_address"].lower() == "target@ex.co" for it in items) + + +def test_admin_outbound_emails_refuses_non_admin(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=700, login="contribU", role="contributor") + sign_in_as( + client, user_id=700, gitea_login="contribU", + display_name="Contrib U", role="contributor", + ) + r = client.get("/api/admin/outbound-emails") + assert r.status_code == 403 + + +# --------------------------------------------------------------------------- +# v0.18.0 Slice 5: bounce correlation +# --------------------------------------------------------------------------- + + +def test_bounce_with_message_id_marks_outbound_row_bounced(app_with_fake_gitea): + """When the bounce body includes the original `message_id`, the + framework looks it up in outbound_emails and stamps + status='bounced' on the matching row. The hard-bounce -> + global-opt-out logic still fires.""" + from fastapi.testclient import TestClient + from app import db, email as email_mod + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=800, login="bouncey", role="contributor") + db.conn().execute("UPDATE users SET email = 'bouncey@ex.co' WHERE id = 800") + + # Send something to bouncey to land an outbound_emails row. + email_mod.reset_sent_envelopes() + client.post("/auth/otc/request", json={"email": "bouncey@ex.co"}) + row = db.conn().execute( + "SELECT id, message_id, status FROM outbound_emails " + "WHERE to_address = 'bouncey@ex.co'" + ).fetchone() + assert row is not None + original_id = row["id"] + message_id = row["message_id"] + assert row["status"] == "deferred" # pre-bounce baseline + + # Bounce comes in carrying that message_id. + r = client.post( + "/api/webhooks/email-bounce", + json={ + "email": "bouncey@ex.co", + "kind": "hard", + "message_id": message_id, + }, + ) + assert r.status_code == 200 + body = r.json() + assert body["matched"] is True + assert body["correlated_id"] == original_id + + # Audit row stamped. + post = db.conn().execute( + "SELECT status, error FROM outbound_emails WHERE id = ?", + (original_id,), + ).fetchone() + assert post["status"] == "bounced" + assert "bounce (hard)" in (post["error"] or "") + + # Hard-bounce global opt-out still fires. + urow = db.conn().execute( + "SELECT email_opt_out_all FROM users WHERE id = 800" + ).fetchone() + assert urow["email_opt_out_all"] == 1 + + +def test_bounce_with_unknown_message_id_does_not_crash(app_with_fake_gitea): + """A message_id the framework doesn't recognize logs but does + NOT 5xx — bounce providers replay old bounces, and the + framework can't refuse just because the row was pruned.""" + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + r = client.post( + "/api/webhooks/email-bounce", + json={ + "email": "nobody@ex.co", + "kind": "hard", + "message_id": "", + }, + ) + assert r.status_code == 200 + assert r.json()["correlated_id"] is None + + +def test_bounce_without_message_id_still_flips_opt_out(app_with_fake_gitea): + """Backward compat: providers that don't surface Message-ID + still get the legacy v1 behavior — match by email + flip the + global opt-out.""" + from fastapi.testclient import TestClient + from app import db + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=801, login="legacybounce", role="contributor") + db.conn().execute("UPDATE users SET email = 'legacy@ex.co' WHERE id = 801") + + r = client.post( + "/api/webhooks/email-bounce", + json={"email": "legacy@ex.co", "kind": "hard"}, + ) + assert r.status_code == 200 + body = r.json() + assert body["matched"] is True + assert body["correlated_id"] is None + + urow = db.conn().execute( + "SELECT email_opt_out_all FROM users WHERE id = 801" + ).fetchone() + assert urow["email_opt_out_all"] == 1 + + +def test_bounced_rows_show_in_admin_endpoint(app_with_fake_gitea): + """The admin endpoint surfaces bounced rows alongside the rest; + filtering by `status=bounced` isolates them.""" + from fastapi.testclient import TestClient + from app import db, email as email_mod + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + provision_user_row(user_id=802, login="adminB", role="admin") + sign_in_as( + client, user_id=802, gitea_login="adminB", + display_name="Admin B", role="admin", email="adminb@test", + ) + email_mod.reset_sent_envelopes() + client.post("/auth/otc/request", json={"email": "willbounce@ex.co"}) + row = db.conn().execute( + "SELECT message_id FROM outbound_emails WHERE to_address = 'willbounce@ex.co'" + ).fetchone() + client.post( + "/api/webhooks/email-bounce", + json={"email": "willbounce@ex.co", "kind": "hard", "message_id": row["message_id"]}, + ) + + r = client.get("/api/admin/outbound-emails?status=bounced") + assert r.status_code == 200 + items = r.json()["items"] + assert items + assert all(it["status"] == "bounced" for it in items) + assert any(it["to_address"] == "willbounce@ex.co" for it in items) diff --git a/backend/tests/test_propose_vertical.py b/backend/tests/test_propose_vertical.py index e4cd7a8..5bcf73a 100644 --- a/backend/tests/test_propose_vertical.py +++ b/backend/tests/test_propose_vertical.py @@ -438,7 +438,11 @@ def tmp_env(monkeypatch): "SECRET_KEY": "test-secret-key-for-cookies", "DATABASE_PATH": str(db_path), "OWNER_GITEA_LOGIN": "ben", - "GITEA_WEBHOOK_SECRET": "", + # v0.18.0: `GITEA_WEBHOOK_SECRET` is now mandatory at startup + # per the email + webhook hygiene proposal. Tests bind a fake + # value so the framework boots; tests that want to exercise + # the dev-bypass path monkeypatch `RFC_APP_INSECURE_WEBHOOKS=1`. + "GITEA_WEBHOOK_SECRET": "test-webhook-secret-for-signature-verification", "ENABLED_MODELS": "claude", } for k, v in env.items(): diff --git a/backend/tests/test_webhooks_vertical.py b/backend/tests/test_webhooks_vertical.py new file mode 100644 index 0000000..f55db1d --- /dev/null +++ b/backend/tests/test_webhooks_vertical.py @@ -0,0 +1,205 @@ +"""End-to-end integration tests for the Gitea webhook receiver +(v0.18.0 Slice 3 — webhook tightening per the email + webhook +hygiene proposal). + +The release changes the receiver from "verifies the signature only +when a secret is configured; silently accepts unsigned POSTs +otherwise" to "requires the secret unless `RFC_APP_INSECURE_WEBHOOKS=1` +is set as an explicit dev-bypass." The startup-time check lives in +`config.load_config()`; the request-time check lives in +`webhooks.receive`. + +These tests prove: + + * The framework refuses to start when `GITEA_WEBHOOK_SECRET` is + empty and the dev-bypass is not set. + * The dev-bypass (`RFC_APP_INSECURE_WEBHOOKS=1`) lets the + framework boot with an empty secret AND lets webhook POSTs + land without signature verification (a loud-warning log line + surfaces, but the request is accepted). + * Default path (secret bound): a POST with a valid signature + lands; a POST with an invalid signature gets 401; a POST with + no signature gets 401. + * Unknown-repo POSTs surface in the log (the "stale Gitea hook" + case the proposal targets). +""" +from __future__ import annotations + +import hashlib +import hmac +import json +import logging + +import pytest + +from test_propose_vertical import ( # noqa: F401 + FakeGitea, + app_with_fake_gitea, + tmp_env, +) + + +# --------------------------------------------------------------------------- +# Startup-time secret check (config.load_config) +# --------------------------------------------------------------------------- + + +def test_config_refuses_to_load_with_empty_secret_and_no_bypass(monkeypatch, tmp_path): + """The framework MUST refuse to start when `GITEA_WEBHOOK_SECRET` + is empty unless `RFC_APP_INSECURE_WEBHOOKS=1` is set. This is + the v0.18.0 startup-loud-failure shape — silent acceptance was + the bug.""" + monkeypatch.setenv("GITEA_URL", "http://gitea.test") + monkeypatch.setenv("GITEA_BOT_USER", "rfc-bot") + monkeypatch.setenv("GITEA_BOT_TOKEN", "bot-token") + monkeypatch.setenv("GITEA_ORG", "wiggleverse") + monkeypatch.setenv("OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("OAUTH_CLIENT_SECRET", "csec") + monkeypatch.setenv("SECRET_KEY", "test-secret-key") + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "test.db")) + monkeypatch.setenv("GITEA_WEBHOOK_SECRET", "") + monkeypatch.delenv("RFC_APP_INSECURE_WEBHOOKS", raising=False) + + from app.config import load_config + with pytest.raises(RuntimeError, match="GITEA_WEBHOOK_SECRET"): + load_config() + + +def test_config_loads_with_empty_secret_when_bypass_is_set(monkeypatch, tmp_path): + """The explicit `RFC_APP_INSECURE_WEBHOOKS=1` opt-in lets the + framework boot with an empty webhook secret. This is the + local-dev escape hatch.""" + monkeypatch.setenv("GITEA_URL", "http://gitea.test") + monkeypatch.setenv("GITEA_BOT_USER", "rfc-bot") + monkeypatch.setenv("GITEA_BOT_TOKEN", "bot-token") + monkeypatch.setenv("GITEA_ORG", "wiggleverse") + monkeypatch.setenv("OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("OAUTH_CLIENT_SECRET", "csec") + monkeypatch.setenv("SECRET_KEY", "test-secret-key") + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "test.db")) + monkeypatch.setenv("GITEA_WEBHOOK_SECRET", "") + monkeypatch.setenv("RFC_APP_INSECURE_WEBHOOKS", "1") + + from app.config import load_config + cfg = load_config() # MUST NOT raise + assert cfg.webhook_secret == "" + + +def test_config_loads_with_secret_set(monkeypatch, tmp_path): + """Sanity: the happy path (secret bound, bypass not set) loads + cleanly.""" + monkeypatch.setenv("GITEA_URL", "http://gitea.test") + monkeypatch.setenv("GITEA_BOT_USER", "rfc-bot") + monkeypatch.setenv("GITEA_BOT_TOKEN", "bot-token") + monkeypatch.setenv("GITEA_ORG", "wiggleverse") + monkeypatch.setenv("OAUTH_CLIENT_ID", "cid") + monkeypatch.setenv("OAUTH_CLIENT_SECRET", "csec") + monkeypatch.setenv("SECRET_KEY", "test-secret-key") + monkeypatch.setenv("DATABASE_PATH", str(tmp_path / "test.db")) + monkeypatch.setenv("GITEA_WEBHOOK_SECRET", "my-real-secret") + monkeypatch.delenv("RFC_APP_INSECURE_WEBHOOKS", raising=False) + + from app.config import load_config + cfg = load_config() + assert cfg.webhook_secret == "my-real-secret" + + +# --------------------------------------------------------------------------- +# Request-time signature verification (webhooks.receive) +# +# The default `app_with_fake_gitea` fixture binds +# `GITEA_WEBHOOK_SECRET=test-webhook-secret-for-signature-verification`, +# so these tests exercise the production path. +# --------------------------------------------------------------------------- + + +_SECRET = "test-webhook-secret-for-signature-verification" + + +def _sign(body: bytes) -> str: + return hmac.new(_SECRET.encode("utf-8"), body, hashlib.sha256).hexdigest() + + +def test_webhook_post_with_valid_signature_accepted(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "wiggleverse/meta"}}).encode() + sig = _sign(body) + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "X-Gitea-Signature": sig, + "Content-Type": "application/json", + }, + ) + assert r.status_code == 200, r.text + + +def test_webhook_post_with_invalid_signature_refused_401(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "wiggleverse/meta"}}).encode() + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "X-Gitea-Signature": "0" * 64, # wrong signature + "Content-Type": "application/json", + }, + ) + assert r.status_code == 401 + + +def test_webhook_post_with_missing_signature_refused_401(app_with_fake_gitea): + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "wiggleverse/meta"}}).encode() + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "Content-Type": "application/json", + }, + ) + assert r.status_code == 401 + + +# --------------------------------------------------------------------------- +# Unknown-repo logging (the "stale hook on a fork" surface) +# --------------------------------------------------------------------------- + + +def test_webhook_unknown_repo_logs_at_info(app_with_fake_gitea, caplog): + """Per the proposal: a hook on a fork or a stale Gitea binding + used to silently 200-OK. v0.18.0 surfaces it as an INFO log.""" + from fastapi.testclient import TestClient + + app, _fake = app_with_fake_gitea + with TestClient(app) as client: + body = json.dumps({"repository": {"full_name": "someone-else/unrelated"}}).encode() + sig = _sign(body) + with caplog.at_level(logging.INFO, logger="app.webhooks"): + r = client.post( + "/api/webhooks/gitea", + content=body, + headers={ + "X-Gitea-Event": "push", + "X-Gitea-Signature": sig, + "Content-Type": "application/json", + }, + ) + assert r.status_code == 200 # the handler still 200s; surface is the log line + assert any( + "unknown repo" in rec.message and "someone-else/unrelated" in rec.message + for rec in caplog.records + ), f"expected unknown-repo log line; got: {[r.message for r in caplog.records]}" \ No newline at end of file diff --git a/frontend/package.json b/frontend/package.json index 0668198..2396528 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -1,7 +1,7 @@ { "name": "rfc-app-frontend", "private": true, - "version": "0.17.0", + "version": "0.18.0", "type": "module", "scripts": { "dev": "vite",