Merge feature/v0.18.0-email-webhook-hygiene
v0.18.0 — email + webhook hygiene per the proposal at ~/git/ohm-infra/RFC-APP-EMAIL-HYGIENE-PROPOSAL.md. All five slices ship; 295 tests passing.
This commit is contained in:
+184
@@ -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
|
release's steps in order — no A-to-B path is pre-computed beyond
|
||||||
that.
|
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: <mailto:…>` 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 <deployment>` 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: <mailto:…>` 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
|
## 0.17.0 — 2026-05-28
|
||||||
|
|
||||||
**Minor — schema migration required; no new env vars; no new secrets.**
|
**Minor — schema migration required; no new env vars; no new secrets.**
|
||||||
|
|||||||
@@ -672,6 +672,73 @@ def make_router(config: Config) -> APIRouter:
|
|||||||
"has_more": len(rows) == limit,
|
"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")
|
@router.get("/api/admin/permission-events")
|
||||||
async def list_permission_events(
|
async def list_permission_events(
|
||||||
request: Request,
|
request: Request,
|
||||||
|
|||||||
@@ -73,6 +73,13 @@ class MarkReadBody(BaseModel):
|
|||||||
class BounceBody(BaseModel):
|
class BounceBody(BaseModel):
|
||||||
email: str = Field(min_length=3, max_length=320)
|
email: str = Field(min_length=3, max_length=320)
|
||||||
kind: str = Field(default="hard") # 'hard' or 'complaint'
|
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):
|
class CookieConsentBody(BaseModel):
|
||||||
@@ -443,6 +450,40 @@ def make_router(config: Config) -> APIRouter:
|
|||||||
|
|
||||||
# ----- Email: one-click unsubscribe + bounce webhook -----
|
# ----- 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")
|
@router.get("/api/email/unsubscribe")
|
||||||
async def email_unsubscribe(t: str = Query(..., description="Signed token from the email footer")) -> HTMLResponse:
|
async def email_unsubscribe(t: str = Query(..., description="Signed token from the email footer")) -> HTMLResponse:
|
||||||
try:
|
try:
|
||||||
@@ -453,20 +494,51 @@ def make_router(config: Config) -> APIRouter:
|
|||||||
"<p>Open the app to manage your notification preferences directly.</p>",
|
"<p>Open the app to manage your notification preferences directly.</p>",
|
||||||
status_code=400,
|
status_code=400,
|
||||||
)
|
)
|
||||||
column = {
|
if not _apply_unsubscribe(user_id, category):
|
||||||
"personal-direct": "email_personal_direct",
|
|
||||||
"structural": "email_watched_structural",
|
|
||||||
"admin-actionable": "email_admin_actionable",
|
|
||||||
}.get(category)
|
|
||||||
if column is None:
|
|
||||||
return HTMLResponse(
|
return HTMLResponse(
|
||||||
f"<h1>Unknown category</h1><p>{category}</p>", status_code=400
|
f"<h1>Unknown category</h1><p>{category}</p>", status_code=400
|
||||||
)
|
)
|
||||||
db.conn().execute(f"UPDATE users SET {column} = 0 WHERE id = ?", (user_id,))
|
if category == "all":
|
||||||
return HTMLResponse(
|
body = (
|
||||||
|
"<h1>Unsubscribed</h1><p>You will no longer receive any email "
|
||||||
|
"from this app. You can re-enable individual categories from "
|
||||||
|
"your notification preferences after signing in.</p>"
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
body = (
|
||||||
f"<h1>Unsubscribed</h1><p>You will no longer receive {category} emails. "
|
f"<h1>Unsubscribed</h1><p>You will no longer receive {category} emails. "
|
||||||
f"You can re-enable them in your notification preferences.</p>"
|
f"You can re-enable them in your notification preferences.</p>"
|
||||||
)
|
)
|
||||||
|
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")
|
@router.post("/api/webhooks/email-bounce")
|
||||||
async def email_bounce(body: BounceBody, request: Request) -> dict[str, Any]:
|
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
|
import hmac as _hmac
|
||||||
if not received or not _hmac.compare_digest(expected, received):
|
if not received or not _hmac.compare_digest(expected, received):
|
||||||
raise HTTPException(401, "Invalid webhook signature")
|
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(
|
row = db.conn().execute(
|
||||||
"SELECT id FROM users WHERE LOWER(email) = LOWER(?)", (body.email,),
|
"SELECT id FROM users WHERE LOWER(email) = LOWER(?)", (body.email,),
|
||||||
).fetchone()
|
).fetchone()
|
||||||
if row is None:
|
if row is None:
|
||||||
return {"ok": True, "matched": False}
|
return {"ok": True, "matched": False, "correlated_id": correlated_row_id}
|
||||||
db.conn().execute(
|
db.conn().execute(
|
||||||
"UPDATE users SET email_opt_out_all = 1 WHERE id = ?", (row["id"],),
|
"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)
|
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
|
return router
|
||||||
|
|
||||||
|
|||||||
+15
-1
@@ -60,6 +60,20 @@ def load_config() -> Config:
|
|||||||
|
|
||||||
enabled = [m.strip() for m in _optional("ENABLED_MODELS", "claude").split(",") if m.strip()]
|
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(
|
return Config(
|
||||||
gitea_url=_required("GITEA_URL").rstrip("/"),
|
gitea_url=_required("GITEA_URL").rstrip("/"),
|
||||||
gitea_bot_user=_required("GITEA_BOT_USER"),
|
gitea_bot_user=_required("GITEA_BOT_USER"),
|
||||||
@@ -72,7 +86,7 @@ def load_config() -> Config:
|
|||||||
secret_key=_required("SECRET_KEY"),
|
secret_key=_required("SECRET_KEY"),
|
||||||
database_path=database_path,
|
database_path=database_path,
|
||||||
owner_gitea_login=_optional("OWNER_GITEA_LOGIN"),
|
owner_gitea_login=_optional("OWNER_GITEA_LOGIN"),
|
||||||
webhook_secret=_optional("GITEA_WEBHOOK_SECRET"),
|
webhook_secret=webhook_secret,
|
||||||
enabled_models=enabled,
|
enabled_models=enabled,
|
||||||
anthropic_api_key=_optional("ANTHROPIC_API_KEY"),
|
anthropic_api_key=_optional("ANTHROPIC_API_KEY"),
|
||||||
google_api_key=_optional("GOOGLE_API_KEY"),
|
google_api_key=_optional("GOOGLE_API_KEY"),
|
||||||
|
|||||||
+13
-1
@@ -180,7 +180,19 @@ def assemble_for_user(
|
|||||||
|
|
||||||
subject = _subject(eligible, cadence)
|
subject = _subject(eligible, cadence)
|
||||||
body = _body(eligible, cadence, cfg)
|
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:
|
if not sent:
|
||||||
return False
|
return False
|
||||||
ids = [r["id"] for r, _ in eligible]
|
ids = [r["id"] for r, _ in eligible]
|
||||||
|
|||||||
+165
-10
@@ -24,7 +24,6 @@ import os
|
|||||||
import smtplib
|
import smtplib
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from datetime import datetime, time, timezone
|
from datetime import datetime, time, timezone
|
||||||
from email.message import EmailMessage
|
|
||||||
from email.utils import formataddr
|
from email.utils import formataddr
|
||||||
from itertools import groupby
|
from itertools import groupby
|
||||||
from typing import Any
|
from typing import Any
|
||||||
@@ -33,6 +32,7 @@ from urllib.parse import urlencode
|
|||||||
from itsdangerous import BadSignature, URLSafeSerializer
|
from itsdangerous import BadSignature, URLSafeSerializer
|
||||||
|
|
||||||
from . import db
|
from . import db
|
||||||
|
from .email_envelope import build_envelope
|
||||||
|
|
||||||
log = logging.getLogger(__name__)
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -69,6 +69,7 @@ class EmailConfig:
|
|||||||
app_url: str
|
app_url: str
|
||||||
bundle_threshold: int
|
bundle_threshold: int
|
||||||
enabled: bool
|
enabled: bool
|
||||||
|
unsubscribe_mailto: str
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def from_env(cls) -> "EmailConfig":
|
def from_env(cls) -> "EmailConfig":
|
||||||
@@ -84,6 +85,16 @@ class EmailConfig:
|
|||||||
app_url=os.environ.get("APP_URL", "http://localhost:8000").rstrip("/"),
|
app_url=os.environ.get("APP_URL", "http://localhost:8000").rstrip("/"),
|
||||||
bundle_threshold=int(os.environ.get("EMAIL_BUNDLE_THRESHOLD", "5")),
|
bundle_threshold=int(os.environ.get("EMAIL_BUNDLE_THRESHOLD", "5")),
|
||||||
enabled=os.environ.get("EMAIL_ENABLED", "1") not in ("0", "false", "False"),
|
enabled=os.environ.get("EMAIL_ENABLED", "1") not in ("0", "false", "False"),
|
||||||
|
# v0.18.0: the `List-Unsubscribe: <mailto:…>` 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:
|
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()
|
cfg = EmailConfig.from_env()
|
||||||
token = _signer().dumps({"u": user_id, "c": category})
|
token = _signer().dumps({"u": user_id, "c": category})
|
||||||
qs = urlencode({"t": token})
|
qs = urlencode({"t": token})
|
||||||
@@ -250,7 +269,21 @@ def _send_one(user: Any, notif_id: int, payload: dict, category: str) -> None:
|
|||||||
return
|
return
|
||||||
subject = _subject(payload)
|
subject = _subject(payload)
|
||||||
body = _body(payload, user["id"], category, cfg)
|
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:
|
if not sent:
|
||||||
return
|
return
|
||||||
db.conn().execute(
|
db.conn().execute(
|
||||||
@@ -305,23 +338,65 @@ def _deep_link(payload: dict, cfg: EmailConfig) -> str:
|
|||||||
return cfg.app_url
|
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 = {
|
envelope = {
|
||||||
"to": to_address,
|
"to": to_address,
|
||||||
"from": formataddr((cfg.from_name, cfg.from_address)),
|
"from": formataddr((cfg.from_name, cfg.from_address)),
|
||||||
"subject": subject,
|
"subject": subject,
|
||||||
"body": body,
|
"body": body,
|
||||||
|
"message": msg,
|
||||||
|
"kind": kind,
|
||||||
}
|
}
|
||||||
_SENT.append(envelope)
|
_SENT.append(envelope)
|
||||||
|
message_id = msg["Message-ID"]
|
||||||
if not cfg.smtp_host:
|
if not cfg.smtp_host:
|
||||||
log.info("email (stdout fallback): to=%s subject=%s", to_address, subject)
|
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
|
return True
|
||||||
try:
|
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)
|
smtp = smtplib.SMTP(cfg.smtp_host, cfg.smtp_port, timeout=30)
|
||||||
try:
|
try:
|
||||||
if cfg.smtp_starttls:
|
if cfg.smtp_starttls:
|
||||||
@@ -331,12 +406,78 @@ def _deliver(cfg: EmailConfig, to_address: str, subject: str, body: str) -> bool
|
|||||||
smtp.send_message(msg)
|
smtp.send_message(msg)
|
||||||
finally:
|
finally:
|
||||||
smtp.quit()
|
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
|
return True
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
log.exception("email send failed: to=%s subject=%s", to_address, subject)
|
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
|
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
|
# 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:
|
for r, _cat, extras in group_rows:
|
||||||
summary = _summary_for(r["event_kind"], r["actor_display"], r["rfc_title"], extras)
|
summary = _summary_for(r["event_kind"], r["actor_display"], r["rfc_title"], extras)
|
||||||
sections.append(f" · {summary}")
|
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 = (
|
body = (
|
||||||
"Activity on RFCs you watch, accumulated during your quiet hours:\n"
|
"Activity on RFCs you watch, accumulated during your quiet hours:\n"
|
||||||
+ "\n".join(sections)
|
+ "\n".join(sections)
|
||||||
+ f"\n\nOpen your inbox: {cfg.app_url}/inbox\n"
|
+ f"\n\nOpen your inbox: {cfg.app_url}/inbox\n"
|
||||||
+ f"Manage all preferences: {cfg.app_url}/settings/notifications\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:
|
if not sent:
|
||||||
return 0
|
return 0
|
||||||
ids = [r["id"] for r, _, _ in emailable]
|
ids = [r["id"] for r, _, _ in emailable]
|
||||||
|
|||||||
@@ -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"<mailto:{unsubscribe_mailto}>")
|
||||||
|
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
|
||||||
@@ -31,10 +31,10 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
import smtplib
|
import smtplib
|
||||||
from email.message import EmailMessage
|
|
||||||
from email.utils import formataddr
|
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__)
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -59,31 +59,52 @@ def send_invite_email(
|
|||||||
cfg = EmailConfig.from_env()
|
cfg = EmailConfig.from_env()
|
||||||
subject = _subject(inviter_display, cfg)
|
subject = _subject(inviter_display, cfg)
|
||||||
body = _body(claim_url, inviter_display, inviter_email, custom_message, cfg)
|
body = _body(claim_url, inviter_display, inviter_email, custom_message, cfg)
|
||||||
|
# v0.18.0: invite mail carries a `List-Unsubscribe: <mailto:…>`
|
||||||
|
# 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 = {
|
envelope = {
|
||||||
"to": to_address,
|
"to": to_address,
|
||||||
"from": formataddr((cfg.from_name, cfg.from_address)),
|
"from": formataddr((cfg.from_name, cfg.from_address)),
|
||||||
"subject": subject,
|
"subject": subject,
|
||||||
"body": body,
|
"body": body,
|
||||||
"kind": "invite",
|
"kind": "invite",
|
||||||
|
"message": msg,
|
||||||
}
|
}
|
||||||
_SENT.append(envelope)
|
_SENT.append(envelope)
|
||||||
|
|
||||||
|
message_id = msg["Message-ID"]
|
||||||
if not cfg.enabled:
|
if not cfg.enabled:
|
||||||
log.info("invite email disabled (EMAIL_ENABLED=0): to=%s", to_address)
|
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
|
return True
|
||||||
if not cfg.smtp_host:
|
if not cfg.smtp_host:
|
||||||
# Dev fallback: surface the claim URL at INFO so the operator can
|
# Dev fallback: surface the claim URL at INFO so the operator can
|
||||||
# complete a claim flow without an SMTP relay. In production
|
# complete a claim flow without an SMTP relay. In production
|
||||||
# SMTP_HOST is always set per OHM's overlay.
|
# SMTP_HOST is always set per OHM's overlay.
|
||||||
log.info("invite email (stdout fallback): to=%s claim_url=%s", to_address, claim_url)
|
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
|
return True
|
||||||
|
|
||||||
try:
|
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)
|
smtp = smtplib.SMTP(cfg.smtp_host, cfg.smtp_port, timeout=30)
|
||||||
try:
|
try:
|
||||||
if cfg.smtp_starttls:
|
if cfg.smtp_starttls:
|
||||||
@@ -93,9 +114,18 @@ def send_invite_email(
|
|||||||
smtp.send_message(msg)
|
smtp.send_message(msg)
|
||||||
finally:
|
finally:
|
||||||
smtp.quit()
|
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
|
return True
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
log.exception("invite email send failed: to=%s", to_address)
|
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
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -23,10 +23,10 @@ from __future__ import annotations
|
|||||||
|
|
||||||
import logging
|
import logging
|
||||||
import smtplib
|
import smtplib
|
||||||
from email.message import EmailMessage
|
|
||||||
from email.utils import formataddr
|
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__)
|
log = logging.getLogger(__name__)
|
||||||
|
|
||||||
@@ -44,31 +44,48 @@ def send_otc_email(to_address: str, code: str) -> bool:
|
|||||||
cfg = EmailConfig.from_env()
|
cfg = EmailConfig.from_env()
|
||||||
subject = f"Your sign-in code for {cfg.from_name}"
|
subject = f"Your sign-in code for {cfg.from_name}"
|
||||||
body = _body(code, cfg)
|
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 = {
|
envelope = {
|
||||||
"to": to_address,
|
"to": to_address,
|
||||||
"from": formataddr((cfg.from_name, cfg.from_address)),
|
"from": formataddr((cfg.from_name, cfg.from_address)),
|
||||||
"subject": subject,
|
"subject": subject,
|
||||||
"body": body,
|
"body": body,
|
||||||
"kind": "otc",
|
"kind": "otc",
|
||||||
|
"message": msg,
|
||||||
}
|
}
|
||||||
_SENT.append(envelope)
|
_SENT.append(envelope)
|
||||||
|
|
||||||
|
message_id = msg["Message-ID"]
|
||||||
if not cfg.enabled:
|
if not cfg.enabled:
|
||||||
log.info("otc email disabled (EMAIL_ENABLED=0): to=%s", to_address)
|
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
|
return True
|
||||||
if not cfg.smtp_host:
|
if not cfg.smtp_host:
|
||||||
# Dev fallback: surface the code at INFO so the operator can
|
# Dev fallback: surface the code at INFO so the operator can
|
||||||
# complete a sign-in flow without an SMTP relay. In production
|
# complete a sign-in flow without an SMTP relay. In production
|
||||||
# SMTP_HOST is always set per OHM's overlay.
|
# SMTP_HOST is always set per OHM's overlay.
|
||||||
log.info("otc email (stdout fallback): to=%s code=%s", to_address, code)
|
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
|
return True
|
||||||
|
|
||||||
try:
|
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)
|
smtp = smtplib.SMTP(cfg.smtp_host, cfg.smtp_port, timeout=30)
|
||||||
try:
|
try:
|
||||||
if cfg.smtp_starttls:
|
if cfg.smtp_starttls:
|
||||||
@@ -78,9 +95,18 @@ def send_otc_email(to_address: str, code: str) -> bool:
|
|||||||
smtp.send_message(msg)
|
smtp.send_message(msg)
|
||||||
finally:
|
finally:
|
||||||
smtp.quit()
|
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
|
return True
|
||||||
except Exception:
|
except Exception as exc:
|
||||||
log.exception("otc email send failed: to=%s", to_address)
|
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
|
return False
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
+33
-1
@@ -12,6 +12,7 @@ import hashlib
|
|||||||
import hmac
|
import hmac
|
||||||
import json
|
import json
|
||||||
import logging
|
import logging
|
||||||
|
import os
|
||||||
|
|
||||||
from fastapi import APIRouter, Header, HTTPException, Request
|
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=""),
|
x_gitea_signature: str = Header(default=""),
|
||||||
):
|
):
|
||||||
body = await request.body()
|
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):
|
if not _verify_signature(body, x_gitea_signature, config.webhook_secret):
|
||||||
raise HTTPException(status_code=401, detail="Invalid signature")
|
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)
|
slug = _slug_for_repo(repo_full)
|
||||||
if slug:
|
if slug:
|
||||||
await cache.refresh_rfc_repo(config, gitea, 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:
|
except Exception:
|
||||||
log.exception("webhook refresh failed")
|
log.exception("webhook refresh failed")
|
||||||
raise HTTPException(status_code=500, detail="Refresh failed")
|
raise HTTPException(status_code=500, detail="Refresh failed")
|
||||||
|
|||||||
@@ -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);
|
||||||
@@ -647,3 +647,82 @@ def test_pending_invites_listing_admin_only(app_with_fake_gitea):
|
|||||||
)
|
)
|
||||||
r = client.get("/api/admin/users/invites")
|
r = client.get("/api/admin/users/invites")
|
||||||
assert r.status_code == 403
|
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: <mailto:…>`
|
||||||
|
# (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("<mailto:")
|
||||||
|
# No URL part — invite is mailto-only.
|
||||||
|
assert "https://" not in lu and "http://" not in lu
|
||||||
|
assert msg["List-Unsubscribe-Post"] is None
|
||||||
|
|
||||||
|
|
||||||
|
def test_invite_envelope_respects_email_unsubscribe_mailto_override(app_with_fake_gitea, monkeypatch):
|
||||||
|
"""When `EMAIL_UNSUBSCRIBE_MAILTO` is set, the mailto: target on
|
||||||
|
`List-Unsubscribe` honors it (lets a deployment route opt-outs
|
||||||
|
to a humans-monitored mailbox distinct from the no-reply
|
||||||
|
sender)."""
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
monkeypatch.setenv("EMAIL_UNSUBSCRIBE_MAILTO", "ohm@wiggleverse.org?subject=remove")
|
||||||
|
app, _fake = app_with_fake_gitea
|
||||||
|
with TestClient(app) as client:
|
||||||
|
env = _provision_admin_and_send_invite(client, (app, _fake))
|
||||||
|
msg = env["message"]
|
||||||
|
assert "ohm@wiggleverse.org?subject=remove" in msg["List-Unsubscribe"]
|
||||||
|
|||||||
@@ -231,13 +231,17 @@ def test_bounce_webhook_refuses_unsigned_when_secret_configured(app_with_fake_gi
|
|||||||
|
|
||||||
# With the right header, the call passes the guard. (No matching
|
# With the right header, the call passes the guard. (No matching
|
||||||
# user exists, so we get {matched: False} — that's the v1 contract.)
|
# user exists, so we get {matched: False} — that's the v1 contract.)
|
||||||
|
# v0.18.0 Slice 5: the response now includes `correlated_id`
|
||||||
|
# (the outbound_emails row id that matched the bounce's
|
||||||
|
# `message_id`, if one was supplied). The body didn't pass a
|
||||||
|
# message_id, so correlated_id is None.
|
||||||
r = client.post(
|
r = client.post(
|
||||||
"/api/webhooks/email-bounce",
|
"/api/webhooks/email-bounce",
|
||||||
json={"email": "stranger@example.com", "kind": "hard"},
|
json={"email": "stranger@example.com", "kind": "hard"},
|
||||||
headers={"X-Webhook-Secret": "shhh"},
|
headers={"X-Webhook-Secret": "shhh"},
|
||||||
)
|
)
|
||||||
assert r.status_code == 200, r.text
|
assert r.status_code == 200, r.text
|
||||||
assert r.json() == {"ok": True, "matched": False}
|
assert r.json() == {"ok": True, "matched": False, "correlated_id": None}
|
||||||
|
|
||||||
|
|
||||||
def test_bounce_webhook_open_when_secret_unset(app_with_fake_gitea):
|
def test_bounce_webhook_open_when_secret_unset(app_with_fake_gitea):
|
||||||
|
|||||||
@@ -0,0 +1,173 @@
|
|||||||
|
"""Unit tests for `app.email_envelope.build_envelope` (v0.18.0 Slice 1).
|
||||||
|
|
||||||
|
These tests don't spin up the FastAPI app or touch the DB — they
|
||||||
|
exercise the helper directly. The integration tests in
|
||||||
|
test_otc_vertical / test_admin_create_user_invite_vertical /
|
||||||
|
test_notifications_vertical exercise the helper's *use* via the
|
||||||
|
shared `_SENT` buffer (the send path appends the envelope dict
|
||||||
|
before invoking the helper).
|
||||||
|
"""
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from email.utils import parsedate_to_datetime
|
||||||
|
|
||||||
|
from app.email_envelope import build_envelope
|
||||||
|
|
||||||
|
|
||||||
|
def _base_kwargs(**overrides):
|
||||||
|
base = dict(
|
||||||
|
to_address="recipient@example.com",
|
||||||
|
from_address="notifications@ohm.wiggleverse.org",
|
||||||
|
from_name="OHM",
|
||||||
|
subject="A test subject",
|
||||||
|
body_plain="Hello, world.\n",
|
||||||
|
)
|
||||||
|
base.update(overrides)
|
||||||
|
return base
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# Always-present headers
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def test_envelope_sets_from_to_subject():
|
||||||
|
msg = build_envelope(**_base_kwargs())
|
||||||
|
assert msg["To"] == "recipient@example.com"
|
||||||
|
assert msg["Subject"] == "A test subject"
|
||||||
|
# `From` is the display-form: "OHM <notifications@ohm.wiggleverse.org>".
|
||||||
|
assert "OHM" in msg["From"]
|
||||||
|
assert "<notifications@ohm.wiggleverse.org>" 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: <random@domain>
|
||||||
|
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"] == "<mailto:ohm@wiggleverse.org?subject=remove>"
|
||||||
|
# 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 "<mailto:ohm@wiggleverse.org?subject=remove>" in lu
|
||||||
|
assert "<https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc>" 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"] == "<https://ohm.wiggleverse.org/api/email/unsubscribe?t=abc>"
|
||||||
|
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="<p>Hello, <b>world</b>.</p>"))
|
||||||
|
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 "<b>world</b>" in parts[1].get_content()
|
||||||
@@ -612,3 +612,127 @@ def test_explicit_watch_set_overrides_auto(app_with_fake_gitea):
|
|||||||
# the user put them.
|
# the user put them.
|
||||||
assert row["set_by"] == "explicit"
|
assert row["set_by"] == "explicit"
|
||||||
assert row["state"] == "following"
|
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 "<mailto:" in lu
|
||||||
|
# URL part carries the signed token per make_unsubscribe_url.
|
||||||
|
assert "/api/email/unsubscribe?t=" in lu
|
||||||
|
assert msg["List-Unsubscribe-Post"] == "List-Unsubscribe=One-Click"
|
||||||
|
|
||||||
|
|
||||||
|
def test_email_unsubscribe_post_one_click_flips_category_off(app_with_fake_gitea):
|
||||||
|
"""RFC 8058: Gmail/Yahoo POST `List-Unsubscribe=One-Click` to the
|
||||||
|
URL in the List-Unsubscribe header. The endpoint MUST accept POST
|
||||||
|
+ the same token shape as the GET handler + return 200 + flip the
|
||||||
|
flag."""
|
||||||
|
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=2, login="alice", role="contributor")
|
||||||
|
token = email_mod.make_unsubscribe_url(2, "personal-direct").split("t=", 1)[1]
|
||||||
|
|
||||||
|
r = client.post(
|
||||||
|
f"/api/email/unsubscribe?t={token}",
|
||||||
|
data={"List-Unsubscribe": "One-Click"},
|
||||||
|
)
|
||||||
|
assert r.status_code == 200, r.text
|
||||||
|
body = r.json()
|
||||||
|
assert body["ok"] is True
|
||||||
|
assert body["category"] == "personal-direct"
|
||||||
|
|
||||||
|
row = db.conn().execute(
|
||||||
|
"SELECT email_personal_direct FROM users WHERE id = 2"
|
||||||
|
).fetchone()
|
||||||
|
assert row["email_personal_direct"] == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_email_unsubscribe_post_all_sets_global_opt_out(app_with_fake_gitea):
|
||||||
|
"""The v0.18.0 `all` synthetic category (used by the bundle +
|
||||||
|
digest paths) MUST set `email_opt_out_all = 1`."""
|
||||||
|
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=2, login="alice", role="contributor")
|
||||||
|
token = email_mod.make_unsubscribe_url(2, "all").split("t=", 1)[1]
|
||||||
|
r = client.post(f"/api/email/unsubscribe?t={token}")
|
||||||
|
assert r.status_code == 200
|
||||||
|
assert r.json() == {"ok": True, "category": "all"}
|
||||||
|
row = db.conn().execute(
|
||||||
|
"SELECT email_opt_out_all FROM users WHERE id = 2"
|
||||||
|
).fetchone()
|
||||||
|
assert row["email_opt_out_all"] == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_email_unsubscribe_get_all_sets_global_opt_out(app_with_fake_gitea):
|
||||||
|
"""GET handler also accepts the `all` category and lands the
|
||||||
|
global opt-out (so an MUA that doesn't honor RFC 8058 POST and
|
||||||
|
just opens the URL in a browser still works)."""
|
||||||
|
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=2, login="alice", role="contributor")
|
||||||
|
token = email_mod.make_unsubscribe_url(2, "all").split("t=", 1)[1]
|
||||||
|
r = client.get(f"/api/email/unsubscribe?t={token}")
|
||||||
|
assert r.status_code == 200
|
||||||
|
assert "Unsubscribed" in r.text
|
||||||
|
row = db.conn().execute(
|
||||||
|
"SELECT email_opt_out_all FROM users WHERE id = 2"
|
||||||
|
).fetchone()
|
||||||
|
assert row["email_opt_out_all"] == 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_email_unsubscribe_post_refuses_invalid_token(app_with_fake_gitea):
|
||||||
|
from fastapi.testclient import TestClient
|
||||||
|
|
||||||
|
app, _fake = app_with_fake_gitea
|
||||||
|
with TestClient(app) as client:
|
||||||
|
r = client.post("/api/email/unsubscribe?t=not-a-valid-token")
|
||||||
|
assert r.status_code == 400
|
||||||
|
|||||||
@@ -347,3 +347,53 @@ def test_otc_re_request_invalidates_prior_unused_code(app_with_fake_gitea, monke
|
|||||||
# The new code still works.
|
# The new code still works.
|
||||||
r = client.post("/auth/otc/verify", json={"email": "alice@example.com", "code": second})
|
r = client.post("/auth/otc/verify", json={"email": "alice@example.com", "code": second})
|
||||||
assert r.status_code == 200
|
assert r.status_code == 200
|
||||||
|
|
||||||
|
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
# v0.18.0: envelope headers — Slice 2
|
||||||
|
#
|
||||||
|
# OTC mail goes through `build_envelope` and MUST land Date,
|
||||||
|
# Message-ID, and Auto-Submitted but MUST NOT carry a
|
||||||
|
# List-Unsubscribe header (the recipient explicitly requested the
|
||||||
|
# code; advertising a list semantic would be wrong).
|
||||||
|
# ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
|
||||||
|
def _last_otc_envelope():
|
||||||
|
from app import email as email_mod
|
||||||
|
otc = [e for e in email_mod.sent_envelopes() if e.get("kind") == "otc"]
|
||||||
|
assert otc, "no OTC envelope in the buffer"
|
||||||
|
return otc[-1]
|
||||||
|
|
||||||
|
|
||||||
|
def test_otc_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:
|
||||||
|
_reset_outbound()
|
||||||
|
client.post("/auth/otc/request", json={"email": "headers@example.com"})
|
||||||
|
msg = _last_otc_envelope()["message"]
|
||||||
|
# Date is RFC 5322 parseable.
|
||||||
|
assert parsedate_to_datetime(msg["Date"]) is not None
|
||||||
|
# Message-ID is bracketed and carries the From-address @-domain.
|
||||||
|
mid = msg["Message-ID"]
|
||||||
|
assert mid.startswith("<") and mid.endswith(">")
|
||||||
|
# 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
|
||||||
|
|||||||
@@ -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": "<not-in-our-db@ex.co>",
|
||||||
|
},
|
||||||
|
)
|
||||||
|
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)
|
||||||
@@ -438,7 +438,11 @@ def tmp_env(monkeypatch):
|
|||||||
"SECRET_KEY": "test-secret-key-for-cookies",
|
"SECRET_KEY": "test-secret-key-for-cookies",
|
||||||
"DATABASE_PATH": str(db_path),
|
"DATABASE_PATH": str(db_path),
|
||||||
"OWNER_GITEA_LOGIN": "ben",
|
"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",
|
"ENABLED_MODELS": "claude",
|
||||||
}
|
}
|
||||||
for k, v in env.items():
|
for k, v in env.items():
|
||||||
|
|||||||
@@ -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]}"
|
||||||
@@ -1,7 +1,7 @@
|
|||||||
{
|
{
|
||||||
"name": "rfc-app-frontend",
|
"name": "rfc-app-frontend",
|
||||||
"private": true,
|
"private": true,
|
||||||
"version": "0.17.0",
|
"version": "0.18.0",
|
||||||
"type": "module",
|
"type": "module",
|
||||||
"scripts": {
|
"scripts": {
|
||||||
"dev": "vite",
|
"dev": "vite",
|
||||||
|
|||||||
Reference in New Issue
Block a user