diff --git a/backend/README.md b/backend/README.md index 062e661..1e0d1ab 100644 --- a/backend/README.md +++ b/backend/README.md @@ -572,6 +572,52 @@ unguessable expiring token is the actual protection once a request is made), no cleanup job for expired/used `password_resets` rows (same as `site_invites`, which has never had one either). +## Link previews + +Slack/Discord-style unfurling: the first `http(s)://` URL found in a +message's `content` (`link_preview_service.extract_first_url`) gets a small +preview card fetched from that page's Open Graph tags (`og:title`, +`og:description`, `og:image`, `og:site_name`, falling back to ``). + +- **Never blocks the send.** `create_message` extracts and stores the URL + on `Message.preview_url` synchronously (cheap, no I/O), but the actual + fetch runs in a background `asyncio.create_task` from + `message_events.broadcast_new_message`/`broadcast_message_update`, on its + own DB session (`async_session_factory()`, never the caller's session — + see `push_service.send_push_to_user`'s docstring for why sharing a + session across a fire-and-forget task is unsafe). Once it resolves, a + separate `"link_preview"` WS envelope carries the result to the room; + the initial `"message"`/`"message_update"` broadcast always has + `link_preview: null`. +- **SSRF protection is the real security boundary here**, more so than for + outgoing webhooks — a webhook's `target_url` is admin-configured, but a + link-preview URL comes from *any* room member's message content. Reuses + `app/services/ssrf.py`'s `validate_target_url` (originally webhook-only, + the exception renamed from `UnsafeWebhookUrlError` to `UnsafeUrlError` + now that it's shared), but re-validates before **every hop** of a + redirect chain instead of once up front — redirects are followed + manually (`httpx.AsyncClient(follow_redirects=False)`) specifically so + each intermediate URL is checked before it's ever connected to, not + after. Same accepted DNS-rebinding gap as the webhook case (see that + module's docstring); response body capped at 512 KB and only fetched if + `Content-Type` is `text/html`. +- **Cached by URL, not by message** (`link_previews` table, unique on + `url`) — a URL posted by five different people in five different rooms + fetches once. A row also gets written on a *failed* fetch + (`fetch_failed=True`) so a URL that genuinely doesn't unfurl (SSRF + rejection, timeout, no usable title) isn't re-attempted on every message + that references it; both kinds expire after 7 days (`_CACHE_TTL`). +- Parsed with stdlib `html.parser.HTMLParser`, not a new dependency — only + meta-tag scraping is needed, not general HTML parsing. +- Editing a message re-extracts the URL; if it changed or was removed, the + frontend clears the now-stale preview immediately (`preview_url` on the + `message_update` envelope) rather than leaving the old one showing while + a new fetch (if any) is in flight. +- Scope cuts: one preview per message (the first URL only, matching the + issue's "a small preview" framing), no way to dismiss/suppress a preview + before sending, no re-fetch-on-demand if a cached preview goes stale + mid-TTL. + ## Notes / scope decisions - Invite-only site registration: no `POST /api/auth/register`. Accounts are diff --git a/backend/alembic/versions/d00f93766fa5_link_previews_for_url_unfurling_in_chat_.py b/backend/alembic/versions/d00f93766fa5_link_previews_for_url_unfurling_in_chat_.py new file mode 100644 index 0000000..d6ad448 --- /dev/null +++ b/backend/alembic/versions/d00f93766fa5_link_previews_for_url_unfurling_in_chat_.py @@ -0,0 +1,53 @@ +"""link previews for URL unfurling in chat messages + +Revision ID: d00f93766fa5 +Revises: ffdd409227bb +Create Date: 2026-08-17 17:31:13.598917 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'd00f93766fa5' +down_revision: Union[str, Sequence[str], None] = 'ffdd409227bb' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + """Upgrade schema.""" + op.create_table( + "link_previews", + sa.Column("id", sa.Uuid(), nullable=False), + sa.Column("url", sa.String(length=2048), nullable=False), + sa.Column("title", sa.String(length=500), nullable=True), + sa.Column("description", sa.String(length=1000), nullable=True), + sa.Column("image_url", sa.String(length=2048), nullable=True), + sa.Column("site_name", sa.String(length=200), nullable=True), + sa.Column("fetch_failed", sa.Boolean(), nullable=False), + sa.Column( + "fetched_at", + sa.DateTime(timezone=True), + server_default=sa.text("clock_timestamp()"), + nullable=False, + ), + sa.PrimaryKeyConstraint("id"), + ) + op.create_index( + op.f("ix_link_previews_url"), "link_previews", ["url"], unique=True + ) + # Nullable, no default -- just adds a column to the catalog, no table + # rewrite, no lock beyond the instant one ALTER TABLE ADD COLUMN always + # takes for a nullable column with no default. + op.add_column("messages", sa.Column("preview_url", sa.String(length=2048), nullable=True)) + + +def downgrade() -> None: + """Downgrade schema.""" + op.drop_column("messages", "preview_url") + op.drop_index(op.f("ix_link_previews_url"), table_name="link_previews") + op.drop_table("link_previews") diff --git a/backend/app/models/__init__.py b/backend/app/models/__init__.py index 1e30e09..253de1c 100644 --- a/backend/app/models/__init__.py +++ b/backend/app/models/__init__.py @@ -4,6 +4,7 @@ from app.models.base import Base from app.models.custom_theme import CustomTheme from app.models.event_subscription import EventSubscription from app.models.invite import InviteStatus +from app.models.link_preview import LinkPreview from app.models.membership import RoomMembership, RoomRole from app.models.message import Message from app.models.message_file import MessageFile @@ -41,4 +42,5 @@ __all__ = [ "WebhookIncoming", "EventSubscription", "CustomTheme", + "LinkPreview", ] diff --git a/backend/app/models/link_preview.py b/backend/app/models/link_preview.py new file mode 100644 index 0000000..6fa7a5f --- /dev/null +++ b/backend/app/models/link_preview.py @@ -0,0 +1,32 @@ +import uuid +from datetime import datetime + +from sqlalchemy import Boolean, DateTime, String, func +from sqlalchemy.orm import Mapped, mapped_column + +from app.models.base import Base + + +class LinkPreview(Base): + """A cache keyed by URL, not by message -- multiple messages (in the + same room or different ones) linking the same URL share one fetch + instead of each triggering their own. link_preview_service is the only + writer; Message.preview_url (the first URL found in a message's + content, set at creation time) is the join key a caller looks this up + by.""" + + __tablename__ = "link_previews" + + id: Mapped[uuid.UUID] = mapped_column(primary_key=True, default=uuid.uuid4) + url: Mapped[str] = mapped_column(String(2048), unique=True, index=True, nullable=False) + title: Mapped[str | None] = mapped_column(String(500)) + description: Mapped[str | None] = mapped_column(String(1000)) + image_url: Mapped[str | None] = mapped_column(String(2048)) + site_name: Mapped[str | None] = mapped_column(String(200)) + # Cached separately from a "no row yet" state so a URL that genuinely + # doesn't unfurl (no title, fetch error, blocked by SSRF checks) isn't + # re-fetched on every message that references it within the TTL. + fetch_failed: Mapped[bool] = mapped_column(Boolean, default=False, nullable=False) + fetched_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), server_default=func.clock_timestamp(), nullable=False + ) diff --git a/backend/app/models/message.py b/backend/app/models/message.py index 0b9676d..2503f49 100644 --- a/backend/app/models/message.py +++ b/backend/app/models/message.py @@ -1,7 +1,7 @@ import uuid from datetime import datetime -from sqlalchemy import CheckConstraint, DateTime, ForeignKey, Text, func +from sqlalchemy import CheckConstraint, DateTime, ForeignKey, String, Text, func from sqlalchemy.orm import Mapped, mapped_column, relationship from app.models.base import Base @@ -25,6 +25,12 @@ class Message(Base): content: Mapped[str | None] = mapped_column(Text) image_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("message_images.id")) file_id: Mapped[uuid.UUID | None] = mapped_column(ForeignKey("message_files.id")) + # First http(s) URL found in content at creation time (see + # link_preview_service.extract_first_url), if any -- a plain string, not + # a FK, since the actual preview data lives in link_previews cached by + # URL and is looked up separately (see message_service.list_recent_ + # messages), not eager-loaded as an ORM relationship. + preview_url: Mapped[str | None] = mapped_column(String(2048)) # clock_timestamp(), not now()/func.now() -- the WS handler (ws/chat.py) # shares one AsyncSession for a whole connection's lifetime, and a # read-only op (e.g. a "join" frame's membership check) can leave a diff --git a/backend/app/routers/rooms.py b/backend/app/routers/rooms.py index 0626b7c..d32b3ec 100644 --- a/backend/app/routers/rooms.py +++ b/backend/app/routers/rooms.py @@ -13,7 +13,7 @@ from app.dependencies import ( require_scope, ) from app.models import MessageFile, MessageImage, RoomRole, User -from app.schemas.message import MessageFileInfo, MessageRead +from app.schemas.message import LinkPreviewInfo, MessageFileInfo, MessageRead from app.schemas.message_file import MessageFileCreated from app.schemas.message_image import MessageImageCreated from app.schemas.room import ( @@ -35,6 +35,7 @@ from app.schemas.webhook import ( WebhookIncomingCreate, WebhookIncomingRead, ) +from app.services.link_preview_service import get_link_previews_for_urls from app.services.message_events import broadcast_room_added from app.services.message_service import ( get_reactions_for_messages, @@ -78,7 +79,7 @@ from app.services.webhook_service import ( revoke_event_subscription, revoke_incoming_webhook, ) -from app.services.ssrf import UnsafeWebhookUrlError +from app.services.ssrf import UnsafeUrlError from app.storage import ( ALLOWED_IMAGE_CONTENT_TYPES, UPLOADS_DIR, @@ -341,6 +342,8 @@ async def get_room_messages_endpoint( await require_room_member(room_id, current_user, db) messages = await list_recent_messages(db, room_id, limit) reactions_by_message = await get_reactions_for_messages(db, [m.id for m in messages]) + preview_urls = {m.preview_url for m in messages if m.preview_url} + previews_by_url = await get_link_previews_for_urls(db, list(preview_urls)) return [ MessageRead( id=m.id, @@ -350,6 +353,11 @@ async def get_room_messages_endpoint( content=m.content, image_id=m.image_id, file=_to_message_file_info(m.file) if m.file else None, + link_preview=( + LinkPreviewInfo.model_validate(previews_by_url[m.preview_url]) + if m.preview_url and m.preview_url in previews_by_url + else None + ), reactions=reactions_by_message.get(m.id, []), created_at=m.created_at, edited_at=m.edited_at, @@ -607,7 +615,7 @@ async def create_event_subscription_endpoint( ) except InvalidEventTypeError: raise HTTPException(status_code=400, detail="Unrecognized event type") - except UnsafeWebhookUrlError: + except UnsafeUrlError: raise HTTPException( status_code=400, detail="target_url is not allowed (internal/private address)" ) diff --git a/backend/app/schemas/message.py b/backend/app/schemas/message.py index 7914d58..97c5a49 100644 --- a/backend/app/schemas/message.py +++ b/backend/app/schemas/message.py @@ -19,6 +19,16 @@ class MessageFileInfo(BaseModel): content_type: str +class LinkPreviewInfo(BaseModel): + model_config = ConfigDict(from_attributes=True) + + url: str + title: str | None + description: str | None + image_url: str | None + site_name: str | None + + class MessageRead(BaseModel): model_config = ConfigDict(from_attributes=True) @@ -29,6 +39,7 @@ class MessageRead(BaseModel): content: str | None image_id: uuid.UUID | None file: MessageFileInfo | None + link_preview: LinkPreviewInfo | None reactions: list[ReactionSummary] created_at: datetime edited_at: datetime | None diff --git a/backend/app/services/link_preview_service.py b/backend/app/services/link_preview_service.py new file mode 100644 index 0000000..4dded5c --- /dev/null +++ b/backend/app/services/link_preview_service.py @@ -0,0 +1,207 @@ +import logging +import re +import uuid +from datetime import datetime, timedelta, timezone +from html.parser import HTMLParser + +import httpx +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.database import async_session_factory +from app.models import LinkPreview +from app.services.ssrf import UnsafeUrlError, validate_target_url +from app.ws.broadcaster import Broadcaster + +logger = logging.getLogger(__name__) + +# Deliberately not a full URL regex (e.g. no IPv6-literal-host support) -- +# just enough to find "a URL-shaped thing" in a chat message the same way +# the frontend's Markdown renderer autolinks bare URLs. Trailing punctuation +# a sentence would naturally have after a URL (a period, closing paren from +# "(see https://example.com)", etc.) is trimmed off separately below. +_URL_RE = re.compile(r"https?://[^\s<>\"]+") +_TRAILING_PUNCTUATION = ".,;:!?)'\">" + +_FETCH_TIMEOUT_SECONDS = 5.0 +_MAX_BYTES = 512 * 1024 +_MAX_REDIRECTS = 3 +_USER_AGENT = "ds-chat-link-preview/1.0" +_CACHE_TTL = timedelta(days=7) + + +def extract_first_url(content: str | None) -> str | None: + if not content: + return None + match = _URL_RE.search(content) + if not match: + return None + return match.group(0).rstrip(_TRAILING_PUNCTUATION) or None + + +class _OpenGraphParser(HTMLParser): + """Pulls og:title/og:description/og:image/og:site_name meta tags, + falling back to <title> -- stdlib html.parser is enough for meta-tag + scraping, no reason to add a full HTML parsing dependency (e.g. + BeautifulSoup) just for this.""" + + def __init__(self) -> None: + super().__init__(convert_charrefs=True) + self.og: dict[str, str] = {} + self.title: str | None = None + self._in_title = False + + def handle_starttag(self, tag: str, attrs: list[tuple[str, str | None]]) -> None: + if tag == "meta": + attr_dict = dict(attrs) + prop = attr_dict.get("property") or attr_dict.get("name") + content = attr_dict.get("content") + if prop and content and prop.startswith("og:"): + self.og.setdefault(prop, content) + elif tag == "title": + self._in_title = True + + def handle_endtag(self, tag: str) -> None: + if tag == "title": + self._in_title = False + + def handle_data(self, data: str) -> None: + if self._in_title and self.title is None: + self.title = data.strip() + + +async def _fetch_preview_data(url: str) -> dict | None: + """SSRF-safe fetch: validates (scheme + resolved-IP allowlist check, + see ssrf.py) before *every* hop, following redirects manually rather + than via httpx's own follow_redirects -- that would connect to each + intermediate hop before any of them got validated, defeating the point. + Still subject to the DNS-rebinding gap documented in ssrf.py (a window + between validating a hostname and httpx independently resolving it to + connect); accepted for the same reason it's accepted there. + """ + current_url = url + async with httpx.AsyncClient(timeout=_FETCH_TIMEOUT_SECONDS, follow_redirects=False) as client: + for _ in range(_MAX_REDIRECTS + 1): + try: + validate_target_url(current_url) + except UnsafeUrlError: + return None + + try: + async with client.stream( + "GET", current_url, headers={"User-Agent": _USER_AGENT} + ) as response: + if response.is_redirect: + location = response.headers.get("location") + if not location: + return None + current_url = str(httpx.URL(current_url).join(location)) + continue + if response.status_code >= 400: + return None + content_type = response.headers.get("content-type", "") + if "text/html" not in content_type: + return None + body = b"" + async for chunk in response.aiter_bytes(): + body += chunk + if len(body) >= _MAX_BYTES: + break + break + except httpx.HTTPError: + return None + else: + return None + + parser = _OpenGraphParser() + try: + parser.feed(body.decode("utf-8", errors="replace")) + except Exception: + return None + + title = parser.og.get("og:title") or parser.title + if not title: + return None + + return { + "title": title.strip()[:500], + "description": (parser.og.get("og:description") or "").strip()[:1000] or None, + "image_url": parser.og.get("og:image") or None, + "site_name": (parser.og.get("og:site_name") or "").strip()[:200] or None, + } + + +async def _get_or_fetch(db: AsyncSession, url: str) -> LinkPreview | None: + """Returns None only when there's genuinely nothing to show (a fresh + fetch failed, or a cached row says an earlier one did) -- callers don't + need to distinguish "still fetching" from "never going to have one".""" + existing = (await db.execute(select(LinkPreview).where(LinkPreview.url == url))).scalar_one_or_none() + + if existing is not None and datetime.now(timezone.utc) - existing.fetched_at < _CACHE_TTL: + return None if existing.fetch_failed else existing + + data = await _fetch_preview_data(url) + if existing is not None: + existing.fetch_failed = data is None + if data: + existing.title = data["title"] + existing.description = data["description"] + existing.image_url = data["image_url"] + existing.site_name = data["site_name"] + existing.fetched_at = datetime.now(timezone.utc) + await db.commit() + return None if data is None else existing + + row = LinkPreview( + url=url, + fetch_failed=data is None, + **(data or {"title": None, "description": None, "image_url": None, "site_name": None}), + ) + db.add(row) + await db.commit() + return None if data is None else row + + +async def fetch_and_broadcast_link_preview( + broadcaster: Broadcaster, room_id: uuid.UUID, message_id: uuid.UUID, url: str +) -> None: + """Entry point for a fire-and-forget asyncio.create_task from + broadcast_new_message -- runs on its own DB session (see push_service.py's + send_push_to_user docstring for why a background task must never share + the caller's session) so a slow/hanging fetch can never delay message + delivery to anyone actually online. + """ + try: + async with async_session_factory() as db: + preview = await _get_or_fetch(db, url) + except Exception: + logger.warning("Link preview fetch failed for %s", url, exc_info=True) + return + + if preview is None: + return + + await broadcaster.publish( + room_id, + { + "type": "link_preview", + "room_id": str(room_id), + "id": str(message_id), + "url": preview.url, + "title": preview.title, + "description": preview.description, + "image_url": preview.image_url, + "site_name": preview.site_name, + }, + ) + + +async def get_link_previews_for_urls(db: AsyncSession, urls: list[str]) -> dict[str, LinkPreview]: + """Batch lookup for message history -- mirrors message_service. + get_reactions_for_messages's shape (one query, zipped back onto results + by the caller) rather than an ORM relationship, since the join key is a + plain string column, not a FK.""" + if not urls: + return {} + result = await db.execute(select(LinkPreview).where(LinkPreview.url.in_(urls), LinkPreview.fetch_failed.is_(False))) + return {row.url: row for row in result.scalars().all()} diff --git a/backend/app/services/message_events.py b/backend/app/services/message_events.py index 3d22e40..18d4b36 100644 --- a/backend/app/services/message_events.py +++ b/backend/app/services/message_events.py @@ -1,3 +1,4 @@ +import asyncio import uuid from sqlalchemy import select @@ -5,6 +6,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.models import Message, MessageFile, MessageMention, Room, RoomMembership, User from app.schemas.message import ReactionSummary +from app.services.link_preview_service import fetch_and_broadcast_link_preview from app.services.push_service import send_push_to_user from app.services.webhook_service import dispatch_event from app.ws.broadcaster import Broadcaster @@ -90,12 +92,24 @@ async def _message_payload(db: AsyncSession, message: Message, username: str) -> "content": message.content, "image_id": str(message.image_id) if message.image_id else None, "file": file_payload, + # Never populated here -- fetching it is a network call to a + # third-party URL, which has no business delaying message delivery. + # A separate "link_preview" envelope arrives shortly after (see + # _maybe_fetch_link_preview) once/if the fetch succeeds. + "link_preview": None, "reactions": [], "created_at": message.created_at.isoformat(), "edited_at": message.edited_at.isoformat() if message.edited_at else None, } +def _maybe_fetch_link_preview(broadcaster: Broadcaster, room_id: uuid.UUID, message: Message) -> None: + if message.preview_url: + asyncio.create_task( + fetch_and_broadcast_link_preview(broadcaster, room_id, message.id, message.preview_url) + ) + + async def broadcast_new_message( db: AsyncSession, broadcaster: Broadcaster, @@ -111,6 +125,7 @@ async def broadcast_new_message( await broadcaster.publish(room_id, payload) await _notify_offline_members(db, broadcaster, presence, room_id, sender, message) await dispatch_event(db, "message.created", room_id, payload) + _maybe_fetch_link_preview(broadcaster, room_id, message) async def broadcast_message_update( @@ -122,9 +137,15 @@ async def broadcast_message_update( "room_id": str(room_id), "content": message.content, "edited_at": message.edited_at.isoformat() if message.edited_at else None, + # Lets the frontend clear a stale preview when an edit changes or + # removes the URL it came from -- it compares this against the + # link_preview it already has for the message rather than blindly + # keeping whatever was there before the edit. + "preview_url": message.preview_url, } await broadcaster.publish(room_id, payload) await dispatch_event(db, "message.updated", room_id, payload) + _maybe_fetch_link_preview(broadcaster, room_id, message) async def broadcast_reaction_update( diff --git a/backend/app/services/message_service.py b/backend/app/services/message_service.py index 638a4bc..d2c5f1b 100644 --- a/backend/app/services/message_service.py +++ b/backend/app/services/message_service.py @@ -8,6 +8,7 @@ from sqlalchemy.orm import selectinload from app.models import Message, MessageMention, MessageReaction from app.schemas.message import ReactionSummary +from app.services.link_preview_service import extract_first_url from app.services.mention_service import extract_mentioned_user_ids @@ -28,7 +29,12 @@ async def create_message( file_id: uuid.UUID | None = None, ) -> Message: message = Message( - room_id=room_id, user_id=user_id, content=content, image_id=image_id, file_id=file_id + room_id=room_id, + user_id=user_id, + content=content, + image_id=image_id, + file_id=file_id, + preview_url=extract_first_url(content), ) db.add(message) # message.id is available immediately (a Python-side uuid4 default, not @@ -52,6 +58,7 @@ async def edit_message( raise NotMessageAuthorError() message.content = content + message.preview_url = extract_first_url(content) message.edited_at = datetime.now(timezone.utc) await db.commit() await db.refresh(message) diff --git a/backend/app/services/ssrf.py b/backend/app/services/ssrf.py index 6b2eead..585b473 100644 --- a/backend/app/services/ssrf.py +++ b/backend/app/services/ssrf.py @@ -3,27 +3,30 @@ import socket from urllib.parse import urlparse -class UnsafeWebhookUrlError(Exception): +class UnsafeUrlError(Exception): pass def validate_target_url(url: str) -> None: - """Creation-time-only SSRF check: rejects non-http(s) schemes and any - target whose hostname resolves to a private/loopback/link-local/ - reserved/multicast address. Not re-checked per delivery, so this doesn't - defend against DNS rebinding between creation and a later send -- a - documented known limitation, not an oversight. + """One-shot SSRF check: rejects non-http(s) schemes and any target whose + hostname resolves to a private/loopback/link-local/reserved/multicast + address. Shared by two callers with different re-check needs: webhook + subscriptions validate once at creation time and reuse the URL for many + future deliveries (a real but accepted DNS-rebinding gap, documented + here), while link_preview_service calls this fresh before *every* hop of + a redirect chain for a one-shot fetch, which closes that gap for its own + use case. """ parsed = urlparse(url) if parsed.scheme not in ("http", "https"): - raise UnsafeWebhookUrlError() + raise UnsafeUrlError() if not parsed.hostname: - raise UnsafeWebhookUrlError() + raise UnsafeUrlError() try: addrinfo = socket.getaddrinfo(parsed.hostname, None) except socket.gaierror as exc: - raise UnsafeWebhookUrlError() from exc + raise UnsafeUrlError() from exc for *_rest, sockaddr in addrinfo: ip = ipaddress.ip_address(sockaddr[0]) @@ -35,4 +38,4 @@ def validate_target_url(url: str) -> None: or ip.is_multicast or ip.is_unspecified ): - raise UnsafeWebhookUrlError() + raise UnsafeUrlError() diff --git a/backend/tests/test_link_previews.py b/backend/tests/test_link_previews.py new file mode 100644 index 0000000..64d7bb0 --- /dev/null +++ b/backend/tests/test_link_previews.py @@ -0,0 +1,266 @@ +import asyncio +import uuid + +import pytest_asyncio + +from app.database import engine as _link_preview_engine +from app.schemas.user import UserCreate +from app.services.auth_service import register_user +from app.services.link_preview_service import extract_first_url +from tests.conftest import register_and_login + + +@pytest_asyncio.fixture(autouse=True) +async def _dispose_engine_between_tests(): + # link_preview_service's background task uses app.database's module- + # level engine directly (see test_cli.py's identical fixture for why: + # pytest-asyncio's per-test event loops make a stale pooled connection + # from an earlier test's loop fail with asyncpg "another operation is + # in progress" -- silently, in this file's case, since + # fetch_and_broadcast_link_preview catches and logs rather than raises). + yield + await _link_preview_engine.dispose() + + +def _unique(prefix: str) -> str: + return f"{prefix}-{uuid.uuid4().hex[:8]}" + + +def _unique_url() -> str: + # link_previews rows are written via the background task's own, + # separately-committed DB session (see link_preview_service.py), not + # the db_session fixture's rollback-at-teardown transaction -- they're + # real, permanent rows that persist across test runs. A literal URL + # shared between tests (or repeated suite runs) would silently hit an + # earlier run's cached row instead of exercising a fresh fetch. + return f"http://8.8.8.8/{uuid.uuid4().hex}" + + +def _recv(ws) -> dict: + """Reads the next frame, discarding member_updated presence-change + broadcasts -- same convention as test_message_edit.py.""" + while True: + msg = ws.receive_json() + if msg.get("type") != "member_updated": + return msg + + +def _register_ws(ws_client, username: str) -> dict: + async def _seed(): + async with ws_client.session_factory() as session: + await register_user( + session, + UserCreate(username=username, email=f"{username}@example.com", password="password123"), + ) + + ws_client.portal.call(_seed) + resp = ws_client.post( + "/api/auth/login", json={"username_or_email": username, "password": "password123"} + ) + assert resp.status_code == 200, resp.text + return resp.json() + + +class _FakeResponse: + def __init__(self, status_code=200, headers=None, body=b""): + self.status_code = status_code + self.headers = headers or {} + self.is_redirect = status_code in (301, 302, 303, 307, 308) + self._body = body + + async def aiter_bytes(self): + yield self._body + + +class _FakeStreamCtx: + def __init__(self, response): + self._response = response + + async def __aenter__(self): + return self._response + + async def __aexit__(self, *args): + return False + + +_OG_HTML = ( + b"<html><head>" + b'<meta property="og:title" content="Example Article">' + b'<meta property="og:description" content="A description of the article.">' + b'<meta property="og:image" content="https://8.8.8.8/image.png">' + b'<meta property="og:site_name" content="Example">' + b"</head></html>" +) + + +def _fake_client_factory(html=_OG_HTML, status_code=200, call_log=None): + class _FakeAsyncClient: + def __init__(self, *args, **kwargs): + pass + + async def __aenter__(self): + return self + + async def __aexit__(self, *args): + return False + + def stream(self, method, url, headers=None): + if call_log is not None: + call_log.append(url) + return _FakeStreamCtx(_FakeResponse(status_code, {"content-type": "text/html"}, html)) + + return _FakeAsyncClient + + +def test_extract_first_url_finds_url_in_content(): + assert extract_first_url("check out https://example.com for more") == "https://example.com" + + +def test_extract_first_url_strips_trailing_punctuation(): + assert extract_first_url("see (https://example.com/page).") == "https://example.com/page" + + +def test_extract_first_url_returns_none_without_url(): + assert extract_first_url("no links here") is None + + +def test_extract_first_url_returns_none_for_empty_content(): + assert extract_first_url(None) is None + assert extract_first_url("") is None + + +def test_ws_message_with_url_triggers_link_preview_broadcast(ws_client, monkeypatch): + monkeypatch.setattr("app.services.link_preview_service.httpx.AsyncClient", _fake_client_factory()) + url = _unique_url() + + username = _unique("alice") + _register_ws(ws_client, username=username) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert _recv(ws)["type"] == "joined" + + ws.send_json({"type": "message", "room_id": room["id"], "content": f"check this out {url}"}) + message = _recv(ws) + assert message["type"] == "message" + # Never populated on the initial broadcast -- fetching is async. + assert message["link_preview"] is None + + preview = _recv(ws) + assert preview["type"] == "link_preview" + assert preview["id"] == message["id"] + assert preview["url"] == url + assert preview["title"] == "Example Article" + assert preview["description"] == "A description of the article." + assert preview["image_url"] == "https://8.8.8.8/image.png" + assert preview["site_name"] == "Example" + + +def test_ws_message_with_private_url_gets_no_preview(ws_client, monkeypatch): + calls: list[str] = [] + monkeypatch.setattr( + "app.services.link_preview_service.httpx.AsyncClient", + _fake_client_factory(call_log=calls), + ) + + username = _unique("alice") + _register_ws(ws_client, username=username) + room = ws_client.post("/api/rooms", json={"name": _unique("general")}).json() + + with ws_client.websocket_connect("/ws/chat") as ws: + ws.send_json({"type": "join", "room_id": room["id"]}) + assert _recv(ws)["type"] == "joined" + + ws.send_json( + { + "type": "message", + "room_id": room["id"], + "content": "internal link http://127.0.0.1/secret", + } + ) + message = _recv(ws) + assert message["link_preview"] is None + + # Sync barrier: an idempotent second frame's ack proves the first + # message's entire async handling (including the spawned link- + # preview task, which is rejected before any real I/O) has settled + # without ever publishing a link_preview envelope. + ws.send_json({"type": "join", "room_id": room["id"]}) + assert _recv(ws)["type"] == "joined" + + assert calls == [] + + +async def test_message_history_includes_cached_link_preview(client, db_session, monkeypatch): + captured_tasks: list[asyncio.Task] = [] + real_create_task = asyncio.create_task + + def fake_create_task(coro): + task = real_create_task(coro) + captured_tasks.append(task) + return task + + monkeypatch.setattr("app.services.message_events.asyncio.create_task", fake_create_task) + monkeypatch.setattr("app.services.link_preview_service.httpx.AsyncClient", _fake_client_factory()) + url = _unique_url() + + await register_and_login(client, db_session, username="alice") + room = (await client.post("/api/rooms", json={"name": "general"})).json() + webhook = (await client.post(f"/api/rooms/{room['id']}/webhooks/incoming", json={})).json() + + resp = await client.post( + f"/api/webhooks/incoming/{webhook['token']}", + json={"content": f"see {url}"}, + ) + assert resp.status_code == 204 + await asyncio.gather(*captured_tasks) + + history = (await client.get(f"/api/rooms/{room['id']}/messages")).json() + assert len(history) == 1 + assert history[0]["link_preview"]["title"] == "Example Article" + assert history[0]["link_preview"]["url"] == url + + +async def test_link_preview_reused_across_messages_with_same_url(client, db_session, monkeypatch): + captured_tasks: list[asyncio.Task] = [] + real_create_task = asyncio.create_task + + def fake_create_task(coro): + task = real_create_task(coro) + captured_tasks.append(task) + return task + + monkeypatch.setattr("app.services.message_events.asyncio.create_task", fake_create_task) + calls: list[str] = [] + monkeypatch.setattr( + "app.services.link_preview_service.httpx.AsyncClient", + _fake_client_factory(call_log=calls), + ) + url = _unique_url() + + await register_and_login(client, db_session, username="alice") + room = (await client.post("/api/rooms", json={"name": "general"})).json() + webhook = (await client.post(f"/api/rooms/{room['id']}/webhooks/incoming", json={})).json() + + # Awaited one at a time so the second send's cache check happens after + # the first's fetch has actually committed -- otherwise both could race + # past the "not cached yet" check concurrently and this would flake. + resp1 = await client.post( + f"/api/webhooks/incoming/{webhook['token']}", json={"content": f"see {url}"} + ) + assert resp1.status_code == 204 + await asyncio.gather(*captured_tasks) + captured_tasks.clear() + + resp2 = await client.post( + f"/api/webhooks/incoming/{webhook['token']}", json={"content": f"again: {url}"} + ) + assert resp2.status_code == 204 + await asyncio.gather(*captured_tasks) + + assert len(calls) == 1 + + history = (await client.get(f"/api/rooms/{room['id']}/messages")).json() + assert len(history) == 2 + assert all(m["link_preview"]["title"] == "Example Article" for m in history) diff --git a/backend/tests/test_webhooks.py b/backend/tests/test_webhooks.py index a487c96..a4cd7b3 100644 --- a/backend/tests/test_webhooks.py +++ b/backend/tests/test_webhooks.py @@ -10,22 +10,22 @@ from app.models import PushSubscription from app.schemas.user import UserCreate from app.services.auth_service import register_user from app.services.room_service import join_room -from app.services.ssrf import UnsafeWebhookUrlError, validate_target_url +from app.services.ssrf import UnsafeUrlError, validate_target_url from tests.conftest import register_and_login def test_validate_target_url_rejects_loopback(): - with pytest.raises(UnsafeWebhookUrlError): + with pytest.raises(UnsafeUrlError): validate_target_url("http://127.0.0.1/hook") def test_validate_target_url_rejects_private_range(): - with pytest.raises(UnsafeWebhookUrlError): + with pytest.raises(UnsafeUrlError): validate_target_url("http://10.0.0.5/hook") def test_validate_target_url_rejects_non_http_scheme(): - with pytest.raises(UnsafeWebhookUrlError): + with pytest.raises(UnsafeUrlError): validate_target_url("ftp://8.8.8.8/hook") diff --git a/frontend/src/components/ChatPane.tsx b/frontend/src/components/ChatPane.tsx index 0c531d6..ed1f731 100644 --- a/frontend/src/components/ChatPane.tsx +++ b/frontend/src/components/ChatPane.tsx @@ -122,16 +122,45 @@ export function ChatPane({ setLive((prev) => [...prev, envelope]) markRead() } else if (envelope.type === 'message_update' && envelope.room_id === room.id) { + // link_preview only survives the edit if the URL it came from is + // still there -- an edit that changed or removed it clears the + // stale preview instead of leaving the old one showing. A new one + // (if the new URL has any) arrives via its own 'link_preview' + // envelope shortly after, same as a fresh send. setHistory((prev) => prev.map((m) => - m.id === envelope.id ? { ...m, content: envelope.content, edited_at: envelope.edited_at } : m, + m.id === envelope.id + ? { + ...m, + content: envelope.content, + edited_at: envelope.edited_at, + link_preview: m.link_preview?.url === envelope.preview_url ? m.link_preview : null, + } + : m, ), ) setLive((prev) => prev.map((m) => - m.id === envelope.id ? { ...m, content: envelope.content, edited_at: envelope.edited_at } : m, + m.id === envelope.id + ? { + ...m, + content: envelope.content, + edited_at: envelope.edited_at, + link_preview: m.link_preview?.url === envelope.preview_url ? m.link_preview : null, + } + : m, ), ) + } else if (envelope.type === 'link_preview' && envelope.room_id === room.id) { + const linkPreview = { + url: envelope.url, + title: envelope.title, + description: envelope.description, + image_url: envelope.image_url, + site_name: envelope.site_name, + } + setHistory((prev) => prev.map((m) => (m.id === envelope.id ? { ...m, link_preview: linkPreview } : m))) + setLive((prev) => prev.map((m) => (m.id === envelope.id ? { ...m, link_preview: linkPreview } : m))) } else if (envelope.type === 'reaction_update' && envelope.room_id === room.id) { setHistory((prev) => prev.map((m) => (m.id === envelope.id ? { ...m, reactions: envelope.reactions } : m)), diff --git a/frontend/src/components/LinkPreviewCard.css b/frontend/src/components/LinkPreviewCard.css new file mode 100644 index 0000000..a57c3e1 --- /dev/null +++ b/frontend/src/components/LinkPreviewCard.css @@ -0,0 +1,65 @@ +.link-preview-card { + display: flex; + gap: 10px; + align-items: stretch; + background: var(--ds-surface-2); + border: 1px solid var(--ds-border); + border-left: 3px solid var(--ds-accent); + border-radius: var(--radius); + padding: 10px 12px; + margin-bottom: 4px; + max-width: min(420px, 100%); + text-decoration: none; + color: var(--ds-text); +} + +.link-preview-card:hover { + border-color: var(--ds-accent); + border-left-color: var(--ds-accent); +} + +.link-preview-image { + flex: none; + width: 64px; + height: 64px; + object-fit: cover; + border-radius: 6px; + background: var(--ds-void); +} + +.link-preview-body { + display: flex; + flex-direction: column; + gap: 2px; + min-width: 0; + justify-content: center; +} + +.link-preview-site { + font-size: 0.72rem; + font-weight: 700; + text-transform: uppercase; + letter-spacing: 0.03em; + color: var(--ds-muted); +} + +.link-preview-title { + font-size: 0.86rem; + font-weight: 700; + color: var(--ds-accent-2, var(--ds-accent)); + overflow: hidden; + text-overflow: ellipsis; + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; +} + +.link-preview-description { + font-size: 0.8rem; + color: var(--ds-muted); + overflow: hidden; + text-overflow: ellipsis; + display: -webkit-box; + -webkit-line-clamp: 2; + -webkit-box-orient: vertical; +} diff --git a/frontend/src/components/LinkPreviewCard.tsx b/frontend/src/components/LinkPreviewCard.tsx new file mode 100644 index 0000000..2d60d70 --- /dev/null +++ b/frontend/src/components/LinkPreviewCard.tsx @@ -0,0 +1,30 @@ +import type { LinkPreviewInfo } from '../types' +import './LinkPreviewCard.css' + +interface LinkPreviewCardProps { + preview: LinkPreviewInfo +} + +// Slack/Discord-style unfurl card, rendered under a message's text when the +// backend found a URL in it and successfully fetched Open Graph data for it +// (see link_preview_service.py -- title/description/image_url/site_name are +// all independently optional, since not every page sets every og: tag). +export function LinkPreviewCard({ preview }: LinkPreviewCardProps) { + return ( + <a + href={preview.url} + target="_blank" + rel="noopener noreferrer" + className="link-preview-card" + > + {preview.image_url && ( + <img src={preview.image_url} alt="" className="link-preview-image" loading="lazy" /> + )} + <div className="link-preview-body"> + {preview.site_name && <span className="link-preview-site">{preview.site_name}</span>} + {preview.title && <span className="link-preview-title">{preview.title}</span>} + {preview.description && <span className="link-preview-description">{preview.description}</span>} + </div> + </a> + ) +} diff --git a/frontend/src/components/MessageList.tsx b/frontend/src/components/MessageList.tsx index b7aa1d0..2d554ad 100644 --- a/frontend/src/components/MessageList.tsx +++ b/frontend/src/components/MessageList.tsx @@ -7,6 +7,7 @@ import type { ChatMessageEnvelope, Message, MessageFileInfo, RoomMember } from ' import { EMOJI_PICKER_MAX_HEIGHT, EmojiPicker } from './EmojiPicker' import { FilePreviewModal, getPreviewKind } from './FilePreviewModal' import { ImageLightbox } from './ImageLightbox' +import { LinkPreviewCard } from './LinkPreviewCard' import { MessageContent } from './MessageContent' import { UserAvatar } from './UserAvatar' import './MessageList.css' @@ -171,6 +172,7 @@ export function MessageList({ roomId, messages, members, onEdit, onReact }: Mess {msg.edited_at && <span className="message-edited"> (edited)</span>} </div> )} + {msg.link_preview && <LinkPreviewCard preview={msg.link_preview} />} {msg.reactions.length > 0 && ( <div className="message-reaction-pills"> {msg.reactions.map((r) => { diff --git a/frontend/src/types.ts b/frontend/src/types.ts index 9cf0ed4..e3edb51 100644 --- a/frontend/src/types.ts +++ b/frontend/src/types.ts @@ -108,6 +108,14 @@ export interface MessageFileInfo { content_type: string } +export interface LinkPreviewInfo { + url: string + title: string | null + description: string | null + image_url: string | null + site_name: string | null +} + export interface Message { id: string room_id: string @@ -116,6 +124,7 @@ export interface Message { content: string | null image_id: string | null file: MessageFileInfo | null + link_preview: LinkPreviewInfo | null reactions: ReactionSummary[] created_at: string edited_at: string | null @@ -130,6 +139,7 @@ export interface ChatMessageEnvelope { content: string | null image_id: string | null file: MessageFileInfo | null + link_preview: LinkPreviewInfo | null reactions: ReactionSummary[] created_at: string edited_at: string | null @@ -141,6 +151,21 @@ export interface ChatMessageUpdateEnvelope { room_id: string content: string edited_at: string | null + // Lets the frontend clear a stale preview when an edit changes/removes + // the URL it came from -- compare against whatever link_preview.url the + // message currently has rather than assuming it's still valid. + preview_url: string | null +} + +export interface ChatLinkPreviewEnvelope { + type: 'link_preview' + id: string + room_id: string + url: string + title: string | null + description: string | null + image_url: string | null + site_name: string | null } export interface ChatReactionUpdateEnvelope { @@ -181,6 +206,7 @@ export type ServerEnvelope = | ChatMessageEnvelope | ChatMessageUpdateEnvelope | ChatReactionUpdateEnvelope + | ChatLinkPreviewEnvelope | ChatJoinedEnvelope | ChatErrorEnvelope | ChatRoomAddedEnvelope