Private
Public Access
Add URL previews for chat messages (#43)
Slack/Discord-style link unfurling: the first http(s) URL in a message's content gets a small preview card (title/description/image/site name) fetched from the page's Open Graph tags. Backend: - Message.preview_url (extracted at create/edit time, cheap regex, no I/O) points at a link_previews cache row keyed by URL -- the same URL posted in different messages/rooms fetches once, and a failed fetch is cached too so a dead URL isn't retried on every reference. - The actual fetch runs in a background asyncio.create_task from broadcast_new_message/broadcast_message_update, on its own DB session, so a slow third-party site never delays message delivery. A separate "link_preview" WS envelope carries the result once it resolves. - SSRF protection reuses app/services/ssrf.py's validate_target_url (renamed from UnsafeWebhookUrlError to UnsafeUrlError now that it's shared with webhooks), but re-validates before every hop of a redirect chain rather than once up front -- redirects are followed manually so each intermediate URL is checked before it's ever connected to. - Parsed with stdlib html.parser -- no new dependency. Frontend: a LinkPreviewCard rendered under message content when present, patched into state live via the new WS envelope and included in message history for reloads. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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",
|
||||
]
|
||||
|
||||
@@ -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
|
||||
)
|
||||
@@ -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
|
||||
|
||||
@@ -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)"
|
||||
)
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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()}
|
||||
@@ -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(
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user