diff --git a/.env.dev b/.env.dev index 7bf3a6a..2b08cb4 100644 --- a/.env.dev +++ b/.env.dev @@ -255,10 +255,22 @@ PLC_DIRECTORY_URL=http://localhost:3002 # ============================================================================= # Image Proxy Configuration # ============================================================================= -# On-the-fly image resizing with disk caching -# Defaults to enabled - falls back to direct PDS URLs if proxy fails +# On-the-fly image resizing with disk caching. Every image URL the AppView +# BUILDS - post/comment embeds, link-card thumbnails, avatars, banners - comes +# from IMAGE_PROXY_BASE_URL (or IMAGE_PROXY_CDN_URL, which wins when set). The +# exception is images inside resolved Bluesky quote embeds, which are passed +# through from bsky's CDN; see docs/PRD_CSAM_SCANNING.md. IMAGE_PROXY_ENABLED=true -IMAGE_PROXY_BASE_URL=http://127.0.0.1:8081 +# Point at the Caddy origin (:8080), not the backend port (:8081), so emitted +# URLs are reachable from mobile devices and emulators: mobile's dev apiUrl is +# also http://localhost:8080 and `make mobile-setup` reverses tcp:8080. The dev +# Caddyfile routes /img/* to the backend, and web is served same-origin at :8080. +# +# This REQUIRES the dev Caddy to be running on :8080 (`make web-proxy`, or +# `make run-web` which expects it). A bare `make run` starts only the backend on +# :8081 and no proxy, so these URLs point at a closed port and every image 404s +# - set IMAGE_PROXY_BASE_URL=http://127.0.0.1:8081 for that workflow. +IMAGE_PROXY_BASE_URL=http://localhost:8080 IMAGE_PROXY_CACHE_PATH=./cache/images IMAGE_PROXY_CACHE_MAX_GB=5 # Optional: CDN URL for production (leave empty for local dev) diff --git a/.env.dev.example b/.env.dev.example index f1e6623..3ff1ead 100644 --- a/.env.dev.example +++ b/.env.dev.example @@ -117,10 +117,22 @@ HS256_ISSUERS=http://localhost:3001 # ============================================================================= # Image Proxy Configuration # ============================================================================= -# On-the-fly image resizing with disk caching -# Defaults to enabled - falls back to direct PDS URLs if proxy fails +# On-the-fly image resizing with disk caching. Every image URL the AppView +# BUILDS - post/comment embeds, link-card thumbnails, avatars, banners - comes +# from IMAGE_PROXY_BASE_URL (or IMAGE_PROXY_CDN_URL, which wins when set). The +# exception is images inside resolved Bluesky quote embeds, which are passed +# through from bsky's CDN; see docs/PRD_CSAM_SCANNING.md. IMAGE_PROXY_ENABLED=true -IMAGE_PROXY_BASE_URL=http://127.0.0.1:8081 +# Point at the Caddy origin (:8080), not the backend port (:8081), so emitted +# URLs are reachable from mobile devices and emulators: mobile's dev apiUrl is +# also http://localhost:8080 and `make mobile-setup` reverses tcp:8080. The dev +# Caddyfile routes /img/* to the backend, and web is served same-origin at :8080. +# +# This REQUIRES the dev Caddy to be running on :8080 (`make web-proxy`, or +# `make run-web` which expects it). A bare `make run` starts only the backend on +# :8081 and no proxy, so these URLs point at a closed port and every image 404s +# - set IMAGE_PROXY_BASE_URL=http://127.0.0.1:8081 for that workflow. +IMAGE_PROXY_BASE_URL=http://localhost:8080 IMAGE_PROXY_CACHE_PATH=./cache/images IMAGE_PROXY_CACHE_MAX_GB=5 # Optional: CDN URL for production (leave empty for local dev) diff --git a/.env.prod.example b/.env.prod.example index 4740c4b..4ae247c 100644 --- a/.env.prod.example +++ b/.env.prod.example @@ -15,8 +15,12 @@ POSTGRES_PASSWORD=CHANGE_ME_SECURE_PASSWORD_HERE # ============================================================================= # PDS (Personal Data Server) # ============================================================================= -# PDS URL for XRPC calls -PDS_URL=https://pds.coves.social +# PDS URL for XRPC calls. This host must also appear in the Caddyfile's CSP +# media-src: social.coves.embed.video#view serves video straight from the +# hosting PDS (the image proxy transcodes stills and cannot stream), so a PDS +# hostname missing from that directive means every video is blocked in the +# browser with no server-side signal. +PDS_URL=https://pds.coves.me # Generate with: openssl rand -hex 32 PDS_JWT_SECRET=CHANGE_ME_64_HEX_CHARS @@ -242,14 +246,43 @@ SKIP_DID_WEB_VERIFICATION=false # ============================================================================= # Image Proxy Configuration # ============================================================================= -# On-the-fly image resizing with disk caching -# Enabled by default - gracefully falls back to direct PDS URLs on failure +# On-the-fly image resizing with disk caching. Every image URL the AppView +# BUILDS - post/comment embeds, link-card thumbnails, avatars, banners - comes +# from IMAGE_PROXY_BASE_URL (or IMAGE_PROXY_CDN_URL, which wins when set), so +# this is the single hostname Coves-built media converges on. The exception is +# images inside resolved Bluesky quote embeds, which are passed through from +# bsky's CDN rather than built here; see the residual-gaps table in +# docs/PRD_CSAM_SCANNING.md. +# +# That convergence is what makes upstream CSAM scanning possible: only img.* is +# intended to be proxied through Cloudflare, and a scanner can only match +# content that crosses its edge. With the proxy disabled the AppView falls back +# to emitting direct com.atproto.sync.getBlob URLs, which skip the CDN entirely +# and are blocked by the site CSP - so the server REFUSES TO START in production +# unless ALLOW_UNPROXIED_MEDIA=true is set. See docs/PRD_CSAM_SCANNING.md for +# which workstreams have actually landed. IMAGE_PROXY_ENABLED=true -IMAGE_PROXY_BASE_URL=https://coves.social +IMAGE_PROXY_BASE_URL=https://img.coves.social IMAGE_PROXY_CACHE_PATH=/var/cache/coves/images IMAGE_PROXY_CACHE_MAX_GB=10 -# Optional: CDN URL for edge caching (recommended for production) -# IMAGE_PROXY_CDN_URL=https://cdn.coves.social +# Optional CDN override; takes precedence over IMAGE_PROXY_BASE_URL when set. +# Not needed when img.coves.social is itself CDN-fronted (the current setup). +# IMAGE_PROXY_CDN_URL= + +# Deliberate opt-out for self-hosted deployments that are NOT fronting media +# with a scanning CDN and want blobs served straight from the PDS. Only has an +# effect with IMAGE_PROXY_ENABLED=false; it exists so that path is a choice +# someone made rather than a variable someone forgot. +# +# false is the default and is what the server already refuses to boot on, so +# uncommenting the line below as written changes nothing - the opt-out is +# ALLOW_UNPROXIED_MEDIA=true specifically. +# +# Taking this opt-out is not complete on its own: the shipped Caddyfile pins +# Content-Security-Policy img-src to the media hostname, so you must also widen +# img-src to cover your PDS hosts or your own web client will block every +# image. The server logs a warning saying so at startup. +# ALLOW_UNPROXIED_MEDIA=true IMAGE_PROXY_FETCH_TIMEOUT_SECONDS=30 IMAGE_PROXY_MAX_SOURCE_SIZE_MB=10 diff --git a/Caddyfile b/Caddyfile index 0f1f567..8af2546 100644 --- a/Caddyfile +++ b/Caddyfile @@ -81,6 +81,23 @@ coves.social { file_server } + # /img/* belongs to the media hostname, and only to the media hostname. + # + # The AppView registers the image-proxy route on its router with no Host + # check, so without this block the catch-all below would serve the exact + # same bytes from coves.social — which is DNS-only, not orange-clouded, and + # therefore never crosses the scanning edge. That would reduce the whole + # choke point to a convention about which URL the AppView happens to emit, + # which is not an enforcement boundary: anyone wanting to pull media past + # the scanner would just swap the hostname. + # + # Redirect rather than 404 so an old or hand-built apex URL still resolves, + # but resolves through the scanned origin. 301 because the mapping is + # permanent and worth caching in the client. + handle /img/* { + redir https://img.coves.social{uri} permanent + } + # Proxy all requests to AppView handle { reverse_proxy appview:8080 { @@ -128,12 +145,83 @@ coves.social { # can't carry nonces) silently downgrades the frontend's script-src # hardening. Flagged in the July 2026 frontend security review. @csp_default not path /m/turnstile.html - header @csp_default Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; img-src 'self' data:; connect-src 'self' https://*.bsky.network wss://*.bsky.network; base-uri 'self'; object-src 'none'; frame-ancestors 'none'" + # img-src covers the media hostname every image URL the AppView emits now + # points at. media-src is separate and must name the PDS hosts directly: + # the image proxy transcodes stills and cannot stream, so + # social.coves.embed.video#view deliberately serves video from the hosting + # PDS (the accepted gap in docs/PRD_CSAM_SCANNING.md workstream 5). Without + # this directive video falls back to default-src 'self' and is blocked. + # + # The listed hosts are our own PDS plus tdpl.io, the bridge PDS that hosts + # blobs for bridged Lemmy communities. Both are reachable; both are needed. + # + # Known limitation, deliberately not papered over: projectVideo builds the + # video URL from whatever PDS owns the record, so video on a community + # hosted by any *other* peer in the network is blocked by this directive. + # A fixed host list cannot express "any PDS in the network", and widening it + # to a wildcard would defeat the point of having one. The failure is + # fail-closed and browser-side only (blocked in console, no server signal). + # It closes when video gets a streaming passthrough on img.coves.social, + # which is also what closes the workstream-5 scanning gap. + header @csp_default Content-Security-Policy "default-src 'self'; script-src 'self' 'unsafe-inline'; style-src 'self' 'unsafe-inline' https://fonts.googleapis.com; font-src 'self' https://fonts.gstatic.com; img-src 'self' data: https://img.coves.social; media-src 'self' https://pds.coves.me https://coves.me https://tdpl.io; connect-src 'self' https://*.bsky.network wss://*.bsky.network; base-uri 'self'; object-src 'none'; frame-ancestors 'none'" # Enable compression encode gzip zstd } +# Media hostname. Every image the AppView hands a client — post and comment +# embeds, link-card thumbnails, avatars, banners — is addressed here, on a +# hostname that carries nothing else. +# +# The isolation is the point: this is the ONLY record in the stack meant to be +# orange-clouded (proxied) at Cloudflare, which is what lets an upstream scanner +# see the media we serve. Everything else must stay DNS-only — the PDS serves +# the atproto sync surface (firehose WebSockets, relay traffic), and tdpl.io +# cannot be proxied at all because its on-demand handle certs need DNS pointing +# straight at this origin. Because only /img/* exists here, the blast radius of +# any CDN feature enabled on this zone is exactly the image proxy. +# +# TLS is issued via the existing Cloudflare DNS-01 token, which works behind the +# orange cloud (no Origin CA cert needed). Set the zone to Full (strict). +img.coves.social { + tls { + dns cloudflare {env.CLOUDFLARE_API_TOKEN} + } + + handle /img/* { + reverse_proxy appview:8080 { + header_up X-Real-IP {remote_host} + header_up X-Forwarded-For {remote_host} + header_up X-Forwarded-Proto {scheme} + } + } + + # Nothing else is served from this hostname. Keeping it a media-only origin + # means a cache rule scoped to img.coves.social/* can never accidentally + # cache an API response. + handle { + respond "Not Found" 404 + } + + header { + Strict-Transport-Security "max-age=31536000" + X-Content-Type-Options "nosniff" + Referrer-Policy "strict-origin-when-cross-origin" + # Images are loaded cross-origin by coves.social and the mobile app. + Access-Control-Allow-Origin "*" + -Server + } + + log { + output stdout + format json + } + + # Images are already compressed; this only benefits the plain-text error + # bodies the proxy returns, which are explicitly no-store. + encode gzip zstd +} + # Tidepool bridge (ActivityPub → atproto). The bridge container lives in the # /opt/tidepool stack and joins coves-prod-network so this Caddy can reach it. # Apex: AP inbox/actor/webfinger/nodeinfo + the com.atproto.sync.* surface diff --git a/cmd/server/wiring.go b/cmd/server/wiring.go index cf1b75f..34b7b96 100644 --- a/cmd/server/wiring.go +++ b/cmd/server/wiring.go @@ -428,22 +428,22 @@ func (a *application) authenticateInstanceWithPDS(ctx context.Context) { slog.Info("instance authenticated with PDS", "instance_did", a.cfg.Instance.DID) } -// buildImageProxy sets up the optional resizing image proxy and publishes the -// URL-generation settings the communities package uses to render avatars. +// buildImageProxy sets up the resizing image proxy and publishes the settings +// every view builder uses to render media URLs. // // The URL config is published on every success path — including when the proxy -// is disabled — because communities needs to know whether to emit proxy URLs or -// direct blob URLs. +// is disabled — because the view builders need to know whether to emit proxy +// URLs or direct blob URLs. In production, config.Validate has already refused +// the disabled path unless the operator opted into it explicitly. func (a *application) buildImageProxy() error { - cfg := imageproxy.ConfigFromEnv() + cfg := a.cfg.Media.ImageProxy - // Published on every path, including the disabled one: communities needs - // to know whether to render proxy URLs or direct blob URLs. Set explicitly - // at each exit rather than via defer — defer is for cleanup, and using it - // for control flow hides that this is the function's main effect when the - // proxy is off. + // Published on every path, including the disabled one. Set explicitly at + // each exit rather than via defer — defer is for cleanup, and using it for + // control flow hides that this is the function's main effect when the proxy + // is off. publishURLConfig := func() { - communities.SetImageProxyConfig(blobs.ImageURLConfig{ + blobs.SetImageURLConfig(blobs.ImageURLConfig{ ProxyEnabled: cfg.Enabled, ProxyBaseURL: cfg.BaseURL, CDNURL: cfg.CDNURL, @@ -452,7 +452,18 @@ func (a *application) buildImageProxy() error { if !cfg.Enabled { publishURLConfig() - slog.Info("image proxy disabled; blob URLs will be served directly") + // Warn, not Info. With the proxy off, every image URL this server hands + // a client addresses a PDS blob endpoint directly — media served around + // whatever CDN is scanning it, and blocked by the shipped CSP. Nothing + // downstream logs that: URL generation succeeds, the embeds projection + // succeeds, and the responses look entirely normal. This line is the + // only signal that a whole deployment is in that state, so it needs to + // be findable in the logs rather than buried at Info alongside routine + // startup chatter. Production additionally refuses to boot here unless + // the operator set ALLOW_UNPROXIED_MEDIA (see config.mediaProblems). + slog.Warn("[IMAGE-PROXY] disabled: image URLs will address PDS blob endpoints directly", + "consequence", "media bypasses any scanning CDN and is blocked by the default Content-Security-Policy", + ) return nil } diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 485aa15..6599d37 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -124,13 +124,24 @@ services: # be accepted. Example: https://tdpl.io TRUSTED_BRIDGE_PDS_HOSTS: ${TRUSTED_BRIDGE_PDS_HOSTS:-} - # Image proxy configuration (on-the-fly resizing with disk cache) + # Image proxy configuration (on-the-fly resizing with disk cache). + # The base URL is the hostname every Coves image URL is built from; it is + # the media hostname, not the AppView apex, because only that hostname is + # CDN-proxied for scanning. See docs/PRD_CSAM_SCANNING.md. IMAGE_PROXY_ENABLED: ${IMAGE_PROXY_ENABLED:-true} - IMAGE_PROXY_BASE_URL: ${IMAGE_PROXY_BASE_URL:-https://coves.social} + IMAGE_PROXY_BASE_URL: ${IMAGE_PROXY_BASE_URL:-https://img.coves.social} + # Optional override that wins over the base URL. Passed through explicitly: + # a variable only present in .env is invisible to the container, so setting + # it there without this line silently has no effect. + IMAGE_PROXY_CDN_URL: ${IMAGE_PROXY_CDN_URL:-} IMAGE_PROXY_CACHE_PATH: /var/cache/coves/images IMAGE_PROXY_CACHE_MAX_GB: ${IMAGE_PROXY_CACHE_MAX_GB:-10} IMAGE_PROXY_FETCH_TIMEOUT_SECONDS: ${IMAGE_PROXY_FETCH_TIMEOUT_SECONDS:-30} IMAGE_PROXY_MAX_SOURCE_SIZE_MB: ${IMAGE_PROXY_MAX_SOURCE_SIZE_MB:-10} + # Acknowledges serving media straight from PDS blob endpoints. Only + # consulted when IMAGE_PROXY_ENABLED=false, where it is required for the + # server to start. + ALLOW_UNPROXIED_MEDIA: ${ALLOW_UNPROXIED_MEDIA:-false} # OpenTelemetry Observability (optional - disabled by default) OTEL_ENABLED: ${OTEL_ENABLED:-false} diff --git a/docs/PRD_CSAM_SCANNING.md b/docs/PRD_CSAM_SCANNING.md new file mode 100644 index 0000000..f6a0001 --- /dev/null +++ b/docs/PRD_CSAM_SCANNING.md @@ -0,0 +1,132 @@ +# PRD: CSAM Scanning via Cloudflare + Media Choke Point + +**Status:** Workstreams 1 and 2 implemented (AppView + Caddy). Workstream 3 (Cloudflare zone config) is manual dashboard/DNS work, not yet done. Workstream 4 (takedown runbook) not started. +**Last updated:** 2026-07-27 + +## Problem + +Coves hosts user media as atproto blobs on PDSes we operate: + +1. **Native uploads** — images/video uploaded by users to `pds.coves.me` (or the community's PDS), and external-link thumbnails that the unfurl pipeline rehosts into the community repo (`internal/core/blobs/service.go` `UploadBlobFromURL`). +2. **Bridged Lemmy media** — the Tidepool bridge materializes federated content as blobs in virtual repos on the bridge PDS (`tdpl.io` stack). Lemmy has a documented history of CSAM-spam waves (2023 lemmy.world attacks); as the blob host, we carry the legal exposure for this content. + +We currently have **no scanning of any kind**. Cloudflare's [CSAM Scanning Tool](https://developers.cloudflare.com/cache/reference/csam-scanning/) is free on all plans, compares content served through the Cloudflare cache against NCMEC (and partner) hash lists, blocks matches at the edge, emails us daily with matched paths, and files a third-party report with NCMEC on our behalf. Since Feb 2025 it requires no NCMEC credentials — just a verified notification email. + +## Key architectural insight + +Cloudflare can only scan what is **proxied through Cloudflare and cached at its edge**. Today, almost none of our media qualifies: + +- Embed thumbnails are hotlinked directly to the origin PDS — `posts.TransformBlobRefsToURLs` emitted `{pds}/xrpc/com.atproto.sync.getBlob?...` (fixed in workstream 1; that code is gone). +- Image-embed blobs (`social.coves.embed.images`) are returned to clients as raw blob refs; clients fetch from wherever they resolve. +- `tdpl.io` **cannot** be CDN-proxied at all: on-demand TLS for bridged-handle certs requires DNS pointing directly at the origin (`Caddyfile` catch-all block), and Cloudflare wildcard proxying doesn't cover `*.*.tdpl.io` anyway. +- `pds.coves.me` serves the atproto sync surface (firehose WebSockets, relay traffic) — proxying it through Cloudflare is possible but risky and unnecessary. + +However, we already have the right choke point built: the **image proxy** (`internal/core/imageproxy/`, route `GET /img/{preset}/plain/{did}/{cid}`). It resolves *any* DID to its PDS (including the bridge PDS), fetches the blob, transforms it, and serves it with `Cache-Control: public, max-age=31536000, immutable` + ETag — ideal for edge caching. The presets registry already includes `content_preview`, `content_full`, and `embed_thumbnail`, not just avatars/banners. + +**Decision:** Do NOT put the whole site (or the PDS, or tdpl.io) behind Cloudflare. Instead: + +> Route **all client-facing media** through the image proxy on a single dedicated hostname (`img.coves.social`), orange-cloud only that hostname, and enable the CSAM Scanning Tool on the `coves.social` zone. + +This leaves DPoP `htu` matching, firehose WebSockets, on-demand TLS, and handle resolution untouched, while giving Cloudflare visibility into 100% of media that Coves clients display. + +## Workstream 1 — Emit proxy URLs for all media (AppView) — ✅ implemented + +Everything the API returns to clients must reference `img.coves.social`, never a PDS. + +All embed projection now lives in one place: **`internal/core/embeds`** (`HydrateView`), called with the DID of the repository that owns the blobs. `posts.TransformBlobRefsToURLs` delegates to it for post embeds (community-owned), and `comments.buildCommentView` calls it for comment embeds (author-owned — comment records live in the user's repo). + +1. **Config**: ✅ `blobs.ImageURLConfig` (`ProxyBaseURL` from `IMAGE_PROXY_BASE_URL`, with `CDNURL` taking precedence). Production defaults now point at `https://img.coves.social` in `.env.prod.example` and `docker-compose.prod.yml`. `IMAGE_PROXY_CDN_URL` was never passed into the container — fixed. + - The URL config moved from a package global in `communities` to `blobs`, where its consumers already live, and its read/write race was replaced with an `RWMutex`. +2. **External embed thumbnails**: ✅ emits `{base}/img/embed_thumbnail/plain/{did}/{cid}` and declares `social.coves.embed.external#view`. The gallery `external.images[]` array is hydrated too; `#viewExternal.images` now refs `#viewImage` rather than the blob-bearing `#image`. + - ⚠️ **Scanning-bypass invariant**: ✅ enforced in `internal/config`. A non-dev config with `IMAGE_PROXY_ENABLED=false` refuses to start unless the operator sets `ALLOW_UNPROXIED_MEDIA=true` — the deliberate opt-out for self-hosters not fronting media with a scanning CDN. An enabled proxy with a missing or localhost base URL is also rejected (relative URLs are unresolvable for the mobile app). +3. **Image embeds** (`social.coves.embed.images`): ✅ each image is projected into `#viewImage` with `thumb` (`content_preview`) and `fullsize` (`content_full`), preserving `alt` and `aspectRatio`. New `#view`/`#viewImage` defs added to the lexicon; both post and comment `embed` unions accept them. +4. **Avatars/banners**: ✅ already converged via `blobs.HydrateImageURL`. One leak fixed: `comments.buildPostView` hand-rolled a `getBlob` URL for community avatars (and its HTTPS-only guard silently dropped the avatar in dev). +5. **Video**: ✅ `social.coves.embed.video#view` — `thumbnail` goes through the proxy (`content_preview`); the `video` blob becomes a direct PDS `getBlob` URL, since the proxy decodes stills and cannot stream. This is the one deliberate, documented gap (Workstream 5). +6. **Mobile app** (`~/Code/coves-mobile`): both clients were already written against this shape (`coves-frontend` types `social.coves.embed.images#view` with `thumb`/`fullsize`; mobile reads `image['thumb'] ?? image['fullsize']`). Two small follow-ups remain — see below. + +Also fixed alongside: `record["embed"]` in `post_repo.go` aliased `postView.Embed`, so hydrating the view silently rewrote the "verbatim record" too. The record now decodes its own copy. + +**Acceptance:** ✅ `grep -rn "sync.getBlob" --include="*.go" internal/ cmd/` returns only the proxy's own fetcher, the `blobs.HydrateBlobURL` helper (used for the video gap and the proxy-disabled fallback), and comments/docs. + +### Client follow-ups (not in this repo) + +- `coves-frontend`: `EmbedImage.image` is typed as required but is no longer emitted — the server sends `thumb`/`fullsize` only (mirroring `app.bsky.embed.images#viewImage`). Make `image` optional, and change `extractEmbedUrl`'s `images#view` branch to return `fullsize` instead of `image`. Rendering paths (`bestImageURL`, `extractEmbedThumbnail`) already go through `imageUrl()` and are unaffected. +- Neither client reads `record.embed`, so the de-aliasing above is invisible to them. + +## Workstream 2 — `img.coves.social` at the origin (Caddy) — ✅ implemented + +The `img.coves.social` site block is in the production `Caddyfile`: `/img/*` reverse-proxies to `appview:8080`, everything else 404s, TLS via the existing Cloudflare DNS-01 token (`CLOUDFLARE_API_TOKEN`), `Access-Control-Allow-Origin: *` so images are embeddable cross-origin from the web app and mobile. + +Also done: +- CSP: the `coves.social` `img-src` is now `'self' data: https://img.coves.social`. +- Error responses from the proxy carry `Cache-Control: no-store` (`writeErrorResponse` in `internal/api/handlers/imageproxy/handler.go`), so a transient PDS timeout or an unpropagated DID can't be pinned at the edge for the year the success path advertises. + +Remaining at deploy time: **the bind-mount trap** — Caddyfile changes require `docker compose up -d --force-recreate caddy`, not just a `git pull` + reload. + +## Workstream 3 — Cloudflare zone configuration + +On the `coves.social` zone (we already own it — DNS-01 tokens exist): + +1. **DNS**: `img.coves.social` A/AAAA → OVH origin IP, **Proxied** (orange cloud). All other records stay DNS-only (grey) — especially anything under `tdpl.io` and `coves.me`. +2. **Cache**: add a Cache Rule for `img.coves.social/*`: *Eligible for cache*, respect origin `Cache-Control`. Blobs are content-addressed (CID in URL) so immutable caching is correct. Optionally enable Tiered Cache. +3. **Enable CSAM Scanning Tool**: Dashboard → Caching → Configuration → CSAM Scanning Tool → Configure. Provide a monitored role address (e.g. `abuse@coves.social`, forwarded to admins) and verify it. Agree to the service-specific terms. +4. **SSL mode**: Full (strict) for the zone (origin has valid certs via Caddy). +5. Do **not** enable Cloudflare features that interfere with API semantics on other hostnames — only `img` is proxied, so blast radius is zero. + +ToS note: since Cloudflare's 2023 self-serve ToS update, serving non-HTML assets like images through the CDN on free plans is explicitly permitted. + +## Workstream 4 — Match response: takedown runbook + tooling + +What Cloudflare does on a match: blocks the URL at the edge and sends a **daily digest email** listing matched file paths, and files a third-party NCMEC report. What it does *not* do: remove the blob from our PDS, purge our origin cache, take down the post, or handle our own reporting/preservation obligations. + +Our matched-path format is self-identifying: `/img/{preset}/plain/{did}/{cid}` gives us the owning repo (DID) and blob (CID) directly. + +Build an admin takedown flow (CLI or admin endpoint), input = DID + CID: + +1. **Locate**: query the AppView index for all posts/records referencing the blob CID; identify the account (native user vs bridged actor via `TRUSTED_BRIDGE_PDS_HOSTS` origin). +2. **Preserve**: before deletion, export the record + blob + account metadata to an encrypted, access-restricted preservation store. US providers must preserve reported content for 90 days (18 U.S.C. §2258A); Cloudflare's docs suggest retaining documentation ~1 year. This store must never be publicly readable. +3. **Remove from serving**: + - Delete the record + blob from the owning PDS (native: PDS admin API; bridged: tidepool bridge admin path). + - Remove/tombstone the post in the AppView index. + - Purge the imageproxy disk cache for **all presets** of that DID+CID (add a purge-by-blob admin method to `imageproxy.DiskCache` — cache keys are preset-scoped). + - Purge the Cloudflare edge cache by URL for each preset variant (single-file purge is available on free plans). Cloudflare's own block covers the exact matched URL; we purge the sibling preset URLs. +4. **Report**: file our own NCMEC CyberTipline report (Cloudflare's third-party report does not replace the provider's own obligation). +5. **Act on the source**: ban the native account, or for bridged content: report to the origin Lemmy instance's admins and, on repeat, drop the instance at the bridge (instance blocklist) — this is where "rely on Lemmy moderation" plugs in. +6. **Log** the entire action (who, what, when) to an audit table. Never log or store the image content outside the preservation store. + +Phase 1 can be a documented manual runbook using existing tools (psql, PDS admin API, `curl` to Cloudflare purge API); the admin tooling hardens it later. + +## Workstream 5 — Residual gaps (known and accepted for phase 1) + +| Gap | Why it remains | Mitigation / future | +|---|---|---| +| Scan-on-serve, not scan-on-ingest | Cloudflare only sees content when a client requests it through the edge | Phase 2: hash-match at ingest (unfurl rehost path + bridge ingest) via PhotoDNA or ROOST hash-matcher | +| Direct PDS `getBlob` remains publicly fetchable | Required by atproto sync (relays, other AppViews) | API no longer emits these URLs; optionally rate-limit `getBlob` at Caddy for non-relay UAs | +| Only known-hash CSAM is detected | Fuzzy hash lists can't catch novel content | Community reporting (`internal/core/adminreports/`) + moderator review remain the backstop | +| Video blobs unscanned | Image proxy is stills-only | Track as separate workstream | +| Bridge PDS stores blobs regardless of scanning | Blobs land before any serve-time scan | Phase 2 ingest scanning; instance allow/blocklist at the bridge is the coarse control | +| `record.embed` still carries blob references | Post and comment responses include the verbatim atproto record, whose embed is unprojected by design (the lexicon calls it verbatim). A client *could* build a `getBlob` URL from it | Neither client reads `record.embed` today. The invariant we actually hold is "the AppView constructs no unproxied URLs", not "no blob reference reaches a client" — the same bytes are public on the PDS regardless. Revisit if a client starts reading it | +| Resolved Bluesky quote-post images | `social.coves.embed.post` resolution returns `cdn.bsky.app` URLs for the quoted author's avatar and embed thumb. Not our blobs, not on our PDS | Bluesky scans its own CDN. These are also currently blocked by our CSP (`img-src` does not include `cdn.bsky.app`) — decide whether to allow-list, proxy, or drop the fields | + +## Rollout order + +WS1 and WS2 are both in the tree, so they ship together. The ordering constraint that remains is **DNS before deploy**: the AppView will start emitting `https://img.coves.social/...` URLs the moment it boots with the new config, so that hostname has to resolve and serve first or every image 404s. + +1. **DNS + Cloudflare (WS3)** — create the `img.coves.social` A/AAAA record pointing at the OVH origin, **Proxied** (orange cloud). Every other record stays DNS-only, especially `tdpl.io` and `coves.me`. Set the zone to Full (strict). Add the cache rule for `img.coves.social/*`. Enable the CSAM Scanning Tool with a verified role address. +2. **Deploy Caddy** — `docker compose up -d --force-recreate caddy` (bind-mount trap). Verify `curl -sD- -o /dev/null https://img.coves.social/img/avatar/plain//` returns 200 with `Cache-Control: public, max-age=31536000, immutable`, and that `https://img.coves.social/` 404s. (Use `-sD- -o /dev/null`, not `-I`: the image route is registered GET-only and chi does not map HEAD to it, so `-I` returns 405 and shows none of the cache headers.) +3. **Deploy the AppView** with `IMAGE_PROXY_BASE_URL=https://img.coves.social`. Startup now fails loudly on a misconfigured proxy rather than silently falling back. Verify feeds render, then watch proxy cache hit rate and origin bandwidth. +4. **Client follow-ups** — ship the two `coves-frontend` type/`extractEmbedUrl` changes noted in WS1. +5. WS4 runbook written and dry-run before announcing Lemmy federation more broadly. +6. Phase 2 (ingest-time hash matching) scheduled after federation traffic is real. + +## Open questions + +- Should the `coves.social` apex also be orange-clouded eventually (DDoS/WAF benefits)? Not required for scanning; revisit separately — DPoP `htu` uses the Host header and should survive proxying, but needs testing. +- Preset URL for full-size originals: do we ever need un-transformed blobs client-side? If yes, add an `original` preset (still scanned/cached) rather than falling back to `getBlob`. +- EU users / DSA reporting equivalents once we have EU presence. + +## References + +- Cloudflare CSAM Scanning Tool docs: https://developers.cloudflare.com/cache/reference/csam-scanning/ +- Feb 2025 onboarding simplification (no NCMEC credentials needed): https://blog.cloudflare.com/a-simpler-path-to-a-safer-internet-an-update-to-our-csam-scanning-tool/ +- Changelog entry: https://developers.cloudflare.com/changelog/post/2025-02-04-easier-onboarding-for-csam-scanning-tool/ diff --git a/internal/api/handlers/imageproxy/handler.go b/internal/api/handlers/imageproxy/handler.go index f2126f1..a56228e 100644 --- a/internal/api/handlers/imageproxy/handler.go +++ b/internal/api/handlers/imageproxy/handler.go @@ -176,8 +176,15 @@ func handleServiceError(w http.ResponseWriter, err error) { // writeErrorResponse writes a plain text error response. // For the image proxy, we use simple text responses rather than JSON // since the expected response is binary image data. +// +// Errors are explicitly uncacheable. Success responses advertise a one-year +// immutable lifetime, which is correct for content-addressed blobs but +// catastrophic for a failure: this route sits behind a CDN, and a transient +// PDS timeout or a DID that had not yet propagated would otherwise be pinned +// at the edge for a year, long after the image became fetchable. func writeErrorResponse(w http.ResponseWriter, status int, message string) { w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.Header().Set("Cache-Control", "no-store") w.WriteHeader(status) if _, err := w.Write([]byte(message)); err != nil { slog.Warn("[IMAGE-PROXY] failed to write error response", diff --git a/internal/api/handlers/imageproxy/handler_test.go b/internal/api/handlers/imageproxy/handler_test.go index 70957a3..52b8be2 100644 --- a/internal/api/handlers/imageproxy/handler_test.go +++ b/internal/api/handlers/imageproxy/handler_test.go @@ -661,3 +661,115 @@ func TestHandler_HandleImage_InvalidCID(t *testing.T) { }) } } + +// This route sits behind a CDN and advertises a one-year immutable lifetime on +// success, which is correct for content-addressed blobs. Inheriting anything +// cacheable on an error would pin a transient failure — a PDS timeout, a DID +// that had not propagated yet — at the edge long after the image became +// fetchable. Every error path must therefore say no-store. +func TestHandler_HandleImage_ErrorsAreNeverCacheable(t *testing.T) { + tests := []struct { + name string + params map[string]string + service *mockService + resolver *mockIdentityResolver + wantStatus int + }{ + { + name: "invalid preset", + params: map[string]string{"preset": "nope", "did": validTestDID, "cid": validTestCID}, + service: &mockService{}, + resolver: &mockIdentityResolver{}, + wantStatus: http.StatusBadRequest, + }, + { + name: "invalid DID", + params: map[string]string{"preset": "avatar", "did": "not-a-did", "cid": validTestCID}, + service: &mockService{}, + resolver: &mockIdentityResolver{}, + wantStatus: http.StatusBadRequest, + }, + { + name: "DID resolution failure", + params: map[string]string{"preset": "avatar", "did": validTestDID, "cid": validTestCID}, + service: &mockService{}, + resolver: &mockIdentityResolver{ + resolveDIDFunc: func(ctx context.Context, did string) (*identity.DIDDocument, error) { + return nil, errors.New("transient PLC failure") + }, + }, + wantStatus: http.StatusBadGateway, + }, + { + name: "blob not found", + params: map[string]string{"preset": "avatar", "did": validTestDID, "cid": validTestCID}, + service: &mockService{ + getImageFunc: func(ctx context.Context, preset, did, cid, pdsURL string) ([]byte, error) { + return nil, imageproxy.ErrPDSNotFound + }, + }, + resolver: resolverForPDS("https://pds.example.com"), + wantStatus: http.StatusNotFound, + }, + { + name: "PDS timeout", + params: map[string]string{"preset": "avatar", "did": validTestDID, "cid": validTestCID}, + service: &mockService{ + getImageFunc: func(ctx context.Context, preset, did, cid, pdsURL string) ([]byte, error) { + return nil, imageproxy.ErrPDSTimeout + }, + }, + resolver: resolverForPDS("https://pds.example.com"), + wantStatus: http.StatusGatewayTimeout, + }, + { + name: "processing failure", + params: map[string]string{"preset": "avatar", "did": validTestDID, "cid": validTestCID}, + service: &mockService{ + getImageFunc: func(ctx context.Context, preset, did, cid, pdsURL string) ([]byte, error) { + return nil, imageproxy.ErrProcessingFailed + }, + }, + resolver: resolverForPDS("https://pds.example.com"), + wantStatus: http.StatusInternalServerError, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + handler := NewHandler(tt.service, tt.resolver) + + path := "/img/" + tt.params["preset"] + "/plain/" + tt.params["did"] + "/" + tt.params["cid"] + w := httptest.NewRecorder() + handler.HandleImage(w, createTestRequest(http.MethodGet, path, tt.params)) + + if w.Code != tt.wantStatus { + t.Fatalf("status = %d, want %d. Body: %s", w.Code, tt.wantStatus, w.Body.String()) + } + if got := w.Header().Get("Cache-Control"); got != "no-store" { + t.Errorf("Cache-Control = %q, want %q so the CDN cannot cache this failure", + got, "no-store") + } + }) + } +} + +// resolverForPDS returns a resolver whose DID document advertises pdsURL as the +// repo's PDS, so a test can reach the service call rather than stopping at +// resolution. +func resolverForPDS(pdsURL string) *mockIdentityResolver { + return &mockIdentityResolver{ + resolveDIDFunc: func(ctx context.Context, did string) (*identity.DIDDocument, error) { + return &identity.DIDDocument{ + DID: did, + Service: []identity.Service{ + { + ID: "#atproto_pds", + Type: "AtprotoPersonalDataServer", + ServiceEndpoint: pdsURL, + }, + }, + }, nil + }, + } +} diff --git a/internal/api/handlers/imageproxy/roundtrip_serving_test.go b/internal/api/handlers/imageproxy/roundtrip_serving_test.go new file mode 100644 index 0000000..559454e --- /dev/null +++ b/internal/api/handlers/imageproxy/roundtrip_serving_test.go @@ -0,0 +1,206 @@ +//go:build integration + +// This file closes the seam between the two halves of the media pipeline, which +// are otherwise only ever tested apart. +// +// One half GENERATES URLs (internal/core/embeds and communities, via +// blobs.HydrateImageURL) and is covered by string comparison. The other half +// SERVES them (/img/{preset}/plain/{did}/{cid}) and is covered — in +// proxy_serving_test.go and avatar_serving_test.go — against paths the tests +// build by hand. Nothing else checks that the first half's output is something +// the second half accepts, so a renamed preset, a changed path shape, or a DID +// that needs escaping would leave both suites green and break every image in +// production. +// +// Here the test takes the URL the AppView actually put in a view — by pointing +// the process-wide image-URL config at the running proxy and letting the real +// projection code emit the URL — and fetches it, against a blob a real PDS +// holds. It is a T1 integration test, not an e2e contract: it exercises the +// generation-plus-serving seam in-process and never touches the firehose or a +// running AppView container. +// +// The file is in the external test package because it imports +// internal/db/postgres and internal/core/communities, which pull in this +// handler package or the domain; in-package that would be an import cycle. +package imageproxy_test + +import ( + "context" + "image/color" + "net/http" + "testing" + + "Coves/internal/atproto/identity" + "Coves/internal/core/blobs" + "Coves/internal/core/communities" + "Coves/internal/core/posts" + "Coves/internal/db/postgres" + "Coves/tests/fixtures" + "Coves/tests/testkit" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestImageProxy_EmittedURLsAreFetchable provisions a community with a real +// avatar blob on the test PDS, points the image-URL config at a running proxy, +// and then fetches the URLs the projection code emits for every embed shape +// that carries an image — plus the community avatar, which shares the generator +// but a different preset and call sites. +// +// It does not call t.Parallel and it must not: it mutates the process-wide +// blobs image-URL config, which the projection code reads, and a parallel +// sibling touching the same global would race. +func TestImageProxy_EmittedURLsAreFetchable(t *testing.T) { + db := testkit.DB(t) + endpoints := testkit.Endpoints() + + identityConfig := identity.DefaultConfig() + identityConfig.PLCURL = endpoints.PLC.BaseURL + // The proxy resolves the community's DID through the stack's PLC directory to + // find the blob, so a fabricated DID would never get past resolution. + resolver := identity.NewResolver(db, identityConfig) + + // A real community on the real test PDS. The blob has to be REFERENCED by the + // community's profile record for the PDS to keep it — an unreferenced upload + // is garbage-collected — which is why it goes through communities.Service + // rather than a row insert. + handleDomain := endpoints.PDS.HandleDomain + communityService := communities.NewCommunityServiceWithPDSFactory( + postgres.NewCommunityRepository(db), + endpoints.PDS.BaseURL, + fixtures.InstanceDID(), + handleDomain, + communities.NewPDSAccountProvisioner(handleDomain, endpoints.PDS.BaseURL), + nil, // no PDS client factory: provisioning uses the password session + blobs.NewBlobService(endpoints.PDS.BaseURL), + ) + + // The provisioner prefixes "c-", and a PDS handle's local label is capped at + // 18 characters, so the name stays short — testkit.UniqueID is built to that + // budget. A 400x300 source sits below the content presets' width caps and is + // large enough to cover the avatar preset. + name := "rt" + testkit.UniqueID(t) + imageData := testkit.TestPNGColor(400, 300, color.RGBA{R: 20, G: 180, B: 90, A: 255}) + community, err := communityService.CreateCommunity(context.Background(), communities.CreateCommunityRequest{ + Name: name, + DisplayName: "Round Trip Test Community", + Description: "Verifies emitted image URLs resolve", + Visibility: "public", + CreatedByDID: fixtures.DID("creator" + name), + HostedByDID: fixtures.InstanceDID(), + AllowExternalDiscovery: true, + AvatarBlob: imageData, + AvatarMimeType: "image/png", + }) + require.NoError(t, err, "provisioning a community with an avatar on the PDS") + require.NotEmpty(t, community.AvatarCID, "the PDS must have assigned the avatar blob a CID") + + // The real route, the real proxy service, the real PDS behind it. + server := newProxyServer(t, resolver, defaultFetchTimeout) + + // Point URL generation at that server, exactly as production points it at the + // media hostname. Everything below flows from this one setting. + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ + ProxyEnabled: true, + ProxyBaseURL: server.URL, + }) + t.Cleanup(blobs.ResetImageURLConfigForTesting) + + blobRef := func(cid string) map[string]interface{} { + return map[string]interface{}{ + "$type": "blob", + "ref": map[string]interface{}{"$link": cid}, + "mimeType": "image/png", + "size": len(imageData), + } + } + + t.Run("image embed URLs resolve", func(t *testing.T) { + postView := &posts.PostView{ + URI: "at://" + community.DID + "/social.coves.community.post/roundtrip", + Community: &posts.CommunityRef{DID: community.DID, PDSURL: community.PDSURL}, + Embed: map[string]interface{}{ + "$type": "social.coves.embed.images", + "images": []interface{}{ + map[string]interface{}{"image": blobRef(community.AvatarCID), "alt": "round trip"}, + }, + }, + } + + posts.TransformBlobRefsToURLs(postView) + + embed := postView.Embed.(map[string]interface{}) + require.Equal(t, "social.coves.embed.images#view", embed["$type"], + "the embed must have been projected, or this test proves nothing") + + img := embed["images"].([]interface{})[0].(map[string]interface{}) + thumb, ok := img["thumb"].(string) + require.True(t, ok, "thumb should be a URL string") + fullsize, ok := img["fullsize"].(string) + require.True(t, ok, "fullsize should be a URL string") + + _, thumbBody := fetch(t, thumb, nil) + _, fullBody := fetch(t, fullsize, nil) + + // content_preview caps width at 800, content_full at 1600, both + // preserving aspect ratio. The source is 400x300, under both caps, so + // neither is upscaled — both decode at the source dimensions. + assertImageSize(t, thumbBody, 400, 300) + assertImageSize(t, fullBody, 400, 300) + }) + + t.Run("external embed thumbnail URL resolves", func(t *testing.T) { + postView := &posts.PostView{ + URI: "at://" + community.DID + "/social.coves.community.post/roundtrip2", + Community: &posts.CommunityRef{DID: community.DID, PDSURL: community.PDSURL}, + Embed: map[string]interface{}{ + "$type": "social.coves.embed.external", + "external": map[string]interface{}{ + "uri": "https://example.com/article", + "thumb": blobRef(community.AvatarCID), + }, + }, + } + + posts.TransformBlobRefsToURLs(postView) + + external := postView.Embed.(map[string]interface{})["external"].(map[string]interface{}) + thumb, ok := external["thumb"].(string) + require.True(t, ok, "thumb should be a URL string") + + resp, body := fetch(t, thumb, nil) + require.Equal(t, http.StatusOK, resp.StatusCode, "body: %s", body) + // embed_thumbnail is 720x360 cover, which crops rather than fitting, so + // the output is exactly the preset size regardless of source aspect. + assertImageSize(t, body, 720, 360) + }) + + t.Run("community avatar URL resolves", func(t *testing.T) { + view := community.ToCommunityViewDetailed() + require.NotEmpty(t, view.Avatar, "detailed community view should carry an avatar URL") + + resp, body := fetch(t, view.Avatar, nil) + require.Equal(t, http.StatusOK, resp.StatusCode, "body: %s", body) + // ToCommunityViewDetailed renders the "avatar" preset (1000x1000 cover), + // so a 400x300 source is upscaled and cropped to the exact preset size. + assertImageSize(t, body, 1000, 1000) + }) + + // Errors must be uncacheable: this route advertises a one-year immutable + // lifetime on success and sits behind a CDN, so a cacheable failure would + // outlive the condition that caused it by a year. + t.Run("an unresolvable blob returns an uncacheable error", func(t *testing.T) { + missing := blobs.HydrateImageURL(blobs.GetImageURLConfig(), + community.PDSURL, community.DID, + "bafkreiabcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopq", "content_preview") + require.NotEmpty(t, missing) + + resp, _ := fetch(t, missing, nil) + + assert.GreaterOrEqual(t, resp.StatusCode, 400, "a missing blob must not return 200") + assert.Equal(t, "no-store", resp.Header.Get("Cache-Control"), + "a CDN must never cache this failure") + }) +} diff --git a/internal/atproto/lexicon/social/coves/community/comment/defs.json b/internal/atproto/lexicon/social/coves/community/comment/defs.json index b4c3f49..a52a01f 100644 --- a/internal/atproto/lexicon/social/coves/community/comment/defs.json +++ b/internal/atproto/lexicon/social/coves/community/comment/defs.json @@ -47,10 +47,12 @@ }, "embed": { "type": "union", - "description": "Embedded content from the comment record (images or quoted post). The AppView may transform blob references into fetchable URLs and enrich quoted posts with a resolved view.", + "description": "Embedded content from the comment record (images or quoted post). The AppView transforms blob references into fetchable image URLs and enriches quoted posts with a resolved view, so a served embed carries a #view type.", "refs": [ "social.coves.embed.images", - "social.coves.embed.post" + "social.coves.embed.images#view", + "social.coves.embed.post", + "social.coves.embed.post#view" ] }, "createdAt": { diff --git a/internal/atproto/lexicon/social/coves/community/post/defs.json b/internal/atproto/lexicon/social/coves/community/post/defs.json index e81e069..5d21814 100644 --- a/internal/atproto/lexicon/social/coves/community/post/defs.json +++ b/internal/atproto/lexicon/social/coves/community/post/defs.json @@ -39,7 +39,9 @@ "description": "Embedded content from the post record. Untransformed embeds keep their record types; when the AppView rewrites blob references to URLs or resolves quoted posts, it serves the corresponding #view type.", "refs": [ "social.coves.embed.images", + "social.coves.embed.images#view", "social.coves.embed.video", + "social.coves.embed.video#view", "social.coves.embed.external", "social.coves.embed.external#view", "social.coves.embed.post", diff --git a/internal/atproto/lexicon/social/coves/embed/external.json b/internal/atproto/lexicon/social/coves/embed/external.json index 0019c0e..3abe517 100644 --- a/internal/atproto/lexicon/social/coves/embed/external.json +++ b/internal/atproto/lexicon/social/coves/embed/external.json @@ -165,7 +165,7 @@ "thumb": { "type": "string", "format": "uri", - "description": "URL of the thumbnail image, served via the hosting PDS blob endpoint" + "description": "URL of the thumbnail image, served via the AppView's image proxy" }, "domain": { "type": "string", @@ -194,7 +194,7 @@ "description": "Preview images for image gallery providers", "items": { "type": "ref", - "ref": "social.coves.embed.images#image" + "ref": "social.coves.embed.images#viewImage" } }, "totalCount": { diff --git a/internal/atproto/lexicon/social/coves/embed/images.json b/internal/atproto/lexicon/social/coves/embed/images.json index 479d96c..45b041e 100644 --- a/internal/atproto/lexicon/social/coves/embed/images.json +++ b/internal/atproto/lexicon/social/coves/embed/images.json @@ -52,6 +52,56 @@ } } }, + "view": { + "type": "object", + "description": "View of an image set embed as served by the AppView; blob references are replaced with fetchable image URLs", + "required": [ + "images" + ], + "properties": { + "images": { + "type": "array", + "minLength": 1, + "maxLength": 8, + "description": "Set of images in the post", + "items": { + "type": "ref", + "ref": "#viewImage" + } + } + } + }, + "viewImage": { + "type": "object", + "description": "Individual image as served by the AppView. Both URLs address the same underlying blob and differ only in rendered size, so clients pick by context instead of resizing themselves.", + "required": [ + "thumb", + "fullsize" + ], + "properties": { + "thumb": { + "type": "string", + "format": "uri", + "description": "URL of the feed-sized rendering of the image" + }, + "fullsize": { + "type": "string", + "format": "uri", + "description": "URL of the full-sized rendering of the image" + }, + "alt": { + "type": "string", + "maxLength": 10000, + "maxGraphemes": 1000, + "description": "Alt text for accessibility" + }, + "aspectRatio": { + "type": "ref", + "ref": "#aspectRatio", + "description": "Aspect ratio for client-side rendering optimization" + } + } + }, "aspectRatio": { "type": "object", "description": "Image aspect ratio for client display", diff --git a/internal/atproto/lexicon/social/coves/embed/video.json b/internal/atproto/lexicon/social/coves/embed/video.json index f6ab08c..76b0cf3 100644 --- a/internal/atproto/lexicon/social/coves/embed/video.json +++ b/internal/atproto/lexicon/social/coves/embed/video.json @@ -40,6 +40,36 @@ "description": "Duration in seconds" } } + }, + "view": { + "type": "object", + "description": "View of a video embed as served by the AppView; blob references are replaced with fetchable URLs", + "required": [ + "video" + ], + "properties": { + "video": { + "type": "string", + "format": "uri", + "description": "URL of the video, served from the hosting PDS blob endpoint. Unlike images, video is not routed through the AppView's image proxy: the proxy decodes and re-encodes stills and cannot stream." + }, + "thumbnail": { + "type": "string", + "format": "uri", + "description": "URL of the video thumbnail image" + }, + "alt": { + "type": "string", + "maxLength": 10000, + "maxGraphemes": 1000, + "description": "Alt text describing video content" + }, + "duration": { + "type": "integer", + "minimum": 0, + "description": "Duration in seconds" + } + } } } } diff --git a/internal/config/config.go b/internal/config/config.go index e646b60..4ea98cc 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -9,6 +9,8 @@ import ( "net/url" "strings" "time" + + "Coves/internal/core/imageproxy" ) // devCursorSecret is the placeholder HMAC key used for pagination cursors when @@ -36,8 +38,17 @@ func isPlaceholder(value string) bool { return strings.HasPrefix(value, placeholderPrefix) } -// requirePublicHost rejects a URL that is empty or points at the loopback -// interface, which in production means the dev default was never replaced. +// requirePublicHost rejects a URL that is empty, not absolute, or points at the +// loopback interface, which in production means the dev default was never +// replaced. +// +// Absoluteness is checked explicitly because url.Parse accepts almost anything: +// "img.coves.social" parses without error as a bare *path* — no scheme, no +// host — and Hostname() then returns "", which is not in the loopback list and +// so used to slip through. Every URL this guards is handed to clients verbatim, +// and a URL with no origin is one the mobile app cannot resolve at all, so an +// absolute http(s) URL is the actual requirement rather than merely a +// well-formed string. func requirePublicHost(name, rawURL string) error { if rawURL == "" { return fmt.Errorf("%s is required in production", name) @@ -46,7 +57,15 @@ func requirePublicHost(name, rawURL string) error { if err != nil { return fmt.Errorf("%s is not a valid URL: %w", name, err) } + if parsed.Scheme != "http" && parsed.Scheme != "https" { + return fmt.Errorf("%s must be an absolute http(s) URL in production (got %q); "+ + "a value with no scheme is parsed as a relative path and reaches clients "+ + "as a URL they cannot resolve", name, rawURL) + } host := parsed.Hostname() + if host == "" { + return fmt.Errorf("%s must include a hostname in production (got %q)", name, rawURL) + } if host == "localhost" || host == "127.0.0.1" || host == "::1" { return fmt.Errorf("%s must not point at localhost in production (got %q); "+ "the loopback default is dev-only and leaves this unreachable to clients", @@ -85,6 +104,7 @@ type Config struct { PDS PDSConfig Jetstream JetstreamConfig Signup SignupConfig + Media MediaConfig // CursorSecret is the HMAC key that signs pagination cursors, preventing // clients from forging or tampering with them. @@ -278,6 +298,34 @@ type SignupConfig struct { TurnstileSiteverifyURL string } +// MediaConfig holds how user media is served to clients. +type MediaConfig struct { + // ImageProxy configures the resizing proxy at /img/{preset}/plain/{did}/{cid}. + // It is parsed here rather than at the point of use so Validate can enforce + // the production invariant below before anything starts. + ImageProxy imageproxy.Config + + // AllowUnproxiedMedia acknowledges, for a production deployment, that + // media will be served straight from PDS blob endpoints. + // + // coves.social routes media through the image proxy on a single CDN-fronted + // hostname because that is the only surface an upstream CSAM scanner can + // see; with the proxy off, the AppView emits com.atproto.sync.getBlob URLs + // that bypass it entirely (and violate the site CSP). Validate therefore + // refuses to start a production server with the proxy disabled, and this + // flag is the deliberate opt-out — a self-hoster who is not fronting media + // with a scanning CDN sets ALLOW_UNPROXIED_MEDIA=true rather than patching + // the source. The point is that no one arrives there by forgetting a + // variable. + // + // Taking the opt-out is not complete on its own: the shipped Caddyfile + // pins img-src to the media hostname, so a deployment serving direct PDS + // URLs must widen that CSP or its own web client will block every image. + // Validate says so in the startup error rather than leaving it to be + // discovered in a browser console. + AllowUnproxiedMedia bool +} + // TokenEndpointEnabled reports whether the signup-token endpoint can operate. // It needs both the captcha secret and (from PDSConfig) an admin password to // mint invite codes, so the caller passes the latter in. @@ -338,6 +386,35 @@ func Load() (*Config, error) { ) } + allowUnproxiedMedia, err := boolVar("ALLOW_UNPROXIED_MEDIA", false) + if err != nil { + return nil, err + } + + imageProxy := imageproxy.ConfigFromEnv() + // IMAGE_PROXY_ENABLED gates a security invariant, so it is re-read through + // boolVar rather than left to ConfigFromEnv's `v == "true" || v == "1"`. + // That comparison is case-sensitive and untrimmed, so left alone + // IMAGE_PROXY_ENABLED=TRUE — or a trailing space in a .env file — would mean + // *disabled*: in production a refused boot whose message says the proxy is + // off while the operator's .env plainly says it is on, and in dev every + // image URL silently dropping back to a direct PDS blob URL. Reading it here + // is what makes those spellings work. + // + // The other eight IMAGE_PROXY_* variables are still parsed by ConfigFromEnv + // on the raw os.Getenv path, so they keep its looser behavior (untrimmed + // strings, unparseable numerics warning and falling back to defaults). + imageProxyEnabled, err := boolVar("IMAGE_PROXY_ENABLED", imageProxy.Enabled) + if err != nil { + return nil, err + } + imageProxy.Enabled = imageProxyEnabled + + cfg.Media = MediaConfig{ + ImageProxy: imageProxy, + AllowUnproxiedMedia: allowUnproxiedMedia, + } + cfg.CursorSecret = stringVar("CURSOR_SECRET", devCursorSecret) if err := cfg.Validate(); err != nil { @@ -660,6 +737,8 @@ func (c *Config) Validate() error { if err := requirePublicHost("PDS_URL", c.PDS.URL); err != nil { problems = append(problems, err.Error()) } + + problems = append(problems, c.mediaProblems()...) } if len(problems) == 0 { @@ -667,3 +746,66 @@ func (c *Config) Validate() error { } return errors.New("invalid configuration:\n - " + strings.Join(problems, "\n - ")) } + +// mediaProblems enforces the production media-serving invariant: every image +// URL the AppView hands a client must point at the image proxy. +// +// The proxy is the choke point that makes upstream CSAM scanning possible — +// one hostname, CDN-fronted, through which all media flows. Disabling it does +// not break anything visibly; URL generation quietly falls back to direct +// com.atproto.sync.getBlob URLs and images keep rendering, so the failure mode +// is a production deployment that looks healthy while serving unscanned media +// past its own CSP. That is worth refusing to start over. +// +// Only called for non-dev configurations. Operators who genuinely intend to +// serve media straight from PDS blob endpoints set ALLOW_UNPROXIED_MEDIA=true. +func (c *Config) mediaProblems() []string { + var problems []string + + if !c.Media.ImageProxy.Enabled { + if !c.Media.AllowUnproxiedMedia { + problems = append(problems, "IMAGE_PROXY_ENABLED must be true in production: "+ + "with the proxy off the AppView emits direct PDS com.atproto.sync.getBlob URLs, "+ + "which bypass the CDN that scans served media and are blocked by the site CSP. "+ + "To serve media directly on purpose (self-hosting without a scanning CDN), "+ + "set ALLOW_UNPROXIED_MEDIA=true — and widen the Caddyfile img-src, which the "+ + "shipped policy pins to the media hostname") + } else { + // Not a problem — the operator asked for this — but the CSP half of + // it is easy to miss, and the symptom (every image blocked, only in + // the browser console) points nowhere near this setting. + slog.Warn("[MEDIA] serving unproxied media: image URLs will address PDS blob endpoints directly", + "reason", "ALLOW_UNPROXIED_MEDIA=true", + "action_required", "widen the Caddyfile Content-Security-Policy img-src to cover your PDS hosts, "+ + "or the web client will block every image", + ) + } + return problems + } + + // With the proxy on, a relative URL ("/img/...") is what an empty base URL + // produces. That resolves correctly for a browser on the AppView origin and + // not at all for the mobile app or any other cross-origin consumer, both of + // which receive these URLs verbatim. + baseURL := c.Media.ImageProxy.CDNURL + name := "IMAGE_PROXY_CDN_URL" + if baseURL == "" { + baseURL = c.Media.ImageProxy.BaseURL + name = "IMAGE_PROXY_BASE_URL" + } + if baseURL == "" { + problems = append(problems, "IMAGE_PROXY_BASE_URL (or IMAGE_PROXY_CDN_URL) is required "+ + "in production: without one the AppView emits relative image URLs, which "+ + "non-browser clients cannot resolve") + return problems + } + if err := requirePublicHost(name, baseURL); err != nil { + problems = append(problems, err.Error()) + } + + if err := c.Media.ImageProxy.Validate(); err != nil { + problems = append(problems, "image proxy configuration: "+err.Error()) + } + + return problems +} diff --git a/internal/config/config_test.go b/internal/config/config_test.go index 4a5f649..693d201 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -33,6 +33,7 @@ func prodEnv(t *testing.T) { t.Setenv("INSTANCE_DID", "did:web:coves.social") t.Setenv("APPVIEW_PUBLIC_URL", "https://coves.social") t.Setenv("PDS_URL", "https://pds.coves.social") + t.Setenv("IMAGE_PROXY_BASE_URL", "https://img.coves.social") } // clearEnv delegates to the exported helper so there is exactly one list of @@ -1009,3 +1010,201 @@ func TestLoad_TrimsWhitespace(t *testing.T) { t.Error("IsDevEnv should parse \" false \" as false") } } + +// The image proxy is the single hostname all Coves media is served from, which +// is what lets an upstream CDN scan it. Disabling it does not break rendering — +// URL generation quietly falls back to direct PDS blob URLs — so a production +// deployment can serve unscanned media past its own CSP while looking healthy. +// Validate refuses to start rather than allow that by omission. +func TestLoad_ProductionRequiresTheImageProxy(t *testing.T) { + clearEnv(t) + prodEnv(t) + t.Setenv("IMAGE_PROXY_ENABLED", "false") + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded in production with the image proxy disabled") + } + if !strings.Contains(err.Error(), "IMAGE_PROXY_ENABLED") { + t.Errorf("error = %q, want it to name IMAGE_PROXY_ENABLED", err.Error()) + } + // The message has to point at the way out, or an operator's only recourse + // is reading the source. + if !strings.Contains(err.Error(), "ALLOW_UNPROXIED_MEDIA") { + t.Errorf("error = %q, want it to name the ALLOW_UNPROXIED_MEDIA opt-out", err.Error()) + } +} + +// Self-hosters who are not fronting media with a scanning CDN opt out +// explicitly instead of patching the source. +func TestLoad_UnproxiedMediaIsAllowedWhenAcknowledged(t *testing.T) { + clearEnv(t) + prodEnv(t) + t.Setenv("IMAGE_PROXY_ENABLED", "false") + t.Setenv("ALLOW_UNPROXIED_MEDIA", "true") + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() returned error for an acknowledged unproxied-media config: %v", err) + } + if cfg.Media.ImageProxy.Enabled { + t.Error("Media.ImageProxy.Enabled should be false") + } + if !cfg.Media.AllowUnproxiedMedia { + t.Error("Media.AllowUnproxiedMedia should be true") + } +} + +// An enabled proxy with no base URL emits relative URLs ("/img/..."). Those +// resolve for a browser on the AppView origin and for nothing else — the mobile +// app receives them verbatim. +func TestLoad_ProductionRejectsUnusableImageProxyBaseURL(t *testing.T) { + tests := []struct { + name string + env map[string]string + wantText string + }{ + { + name: "no base URL at all", + env: map[string]string{"IMAGE_PROXY_BASE_URL": ""}, + wantText: "IMAGE_PROXY_BASE_URL", + }, + { + name: "the dev localhost default left in place", + env: map[string]string{"IMAGE_PROXY_BASE_URL": "http://localhost:8080"}, + wantText: "localhost", + }, + { + name: "a CDN override pointing at loopback", + env: map[string]string{ + "IMAGE_PROXY_BASE_URL": "https://img.coves.social", + "IMAGE_PROXY_CDN_URL": "http://127.0.0.1:8080", + }, + wantText: "IMAGE_PROXY_CDN_URL", + }, + { + // url.Parse accepts this as a bare *path*: no scheme, no host, and + // Hostname() == "", which sits in none of the checks above. It then + // produces "img.coves.social/img/..." — a relative URL, exactly the + // thing mediaProblems exists to reject, since a non-browser client + // has no origin to resolve it against. + name: "a base URL with no scheme", + env: map[string]string{"IMAGE_PROXY_BASE_URL": "img.coves.social"}, + wantText: "absolute http(s) URL", + }, + { + name: "a CDN override with no scheme", + env: map[string]string{ + "IMAGE_PROXY_BASE_URL": "https://img.coves.social", + "IMAGE_PROXY_CDN_URL": "cdn.coves.social", + }, + wantText: "IMAGE_PROXY_CDN_URL", + }, + { + name: "a base URL with a non-http scheme", + env: map[string]string{"IMAGE_PROXY_BASE_URL": "ftp://img.coves.social"}, + wantText: "absolute http(s) URL", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearEnv(t) + prodEnv(t) + for name, value := range tt.env { + t.Setenv(name, value) + } + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded with an unusable image proxy base URL") + } + if !strings.Contains(err.Error(), tt.wantText) { + t.Errorf("error = %q, want it to mention %q", err.Error(), tt.wantText) + } + }) + } +} + +// The CDN override takes precedence over the base URL, so a valid one is +// enough on its own. +func TestLoad_ImageProxyCDNURLSatisfiesTheBaseURLRequirement(t *testing.T) { + clearEnv(t) + prodEnv(t) + t.Setenv("IMAGE_PROXY_BASE_URL", "") + t.Setenv("IMAGE_PROXY_CDN_URL", "https://cdn.coves.social") + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() returned error: %v", err) + } + if cfg.Media.ImageProxy.CDNURL != "https://cdn.coves.social" { + t.Errorf("Media.ImageProxy.CDNURL = %q, want the CDN override", + cfg.Media.ImageProxy.CDNURL) + } +} + +// Dev runs on localhost with relative image URLs and must stay unencumbered by +// the production rule. +func TestLoad_DevIgnoresTheImageProxyInvariant(t *testing.T) { + clearEnv(t) + t.Setenv("IS_DEV_ENV", "true") + t.Setenv("IMAGE_PROXY_ENABLED", "false") + + if _, err := Load(); err != nil { + t.Fatalf("Load() returned error in dev with the image proxy disabled: %v", err) + } +} + +// IMAGE_PROXY_ENABLED gates a security invariant, so it must parse as strictly +// as every other boolean. imageproxy.ConfigFromEnv compares against the literal +// strings "true"/"1", which silently reads TRUE, yes, or a value with a +// trailing space — all common in hand-edited .env files — as *disabled*. +func TestLoad_ImageProxyEnabledParsesStrictly(t *testing.T) { + t.Run("case-insensitive and padded values are honoured, not silently false", func(t *testing.T) { + for _, value := range []string{"TRUE", "True", " true ", "1"} { + t.Run(value, func(t *testing.T) { + clearEnv(t) + prodEnv(t) + t.Setenv("IMAGE_PROXY_ENABLED", value) + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() rejected IMAGE_PROXY_ENABLED=%q: %v", value, err) + } + if !cfg.Media.ImageProxy.Enabled { + t.Errorf("IMAGE_PROXY_ENABLED=%q parsed as disabled", value) + } + }) + } + }) + + t.Run("an unparseable value is an error, not a silent disable", func(t *testing.T) { + clearEnv(t) + prodEnv(t) + t.Setenv("IMAGE_PROXY_ENABLED", "yes") + + _, err := Load() + if err == nil { + t.Fatal("Load() accepted IMAGE_PROXY_ENABLED=yes") + } + if !strings.Contains(err.Error(), "IMAGE_PROXY_ENABLED") { + t.Errorf("error = %q, want it to name IMAGE_PROXY_ENABLED", err.Error()) + } + }) + + t.Run("false still disables", func(t *testing.T) { + clearEnv(t) + prodEnv(t) + t.Setenv("IMAGE_PROXY_ENABLED", "FALSE") + t.Setenv("ALLOW_UNPROXIED_MEDIA", "true") + + cfg, err := Load() + if err != nil { + t.Fatalf("Load() returned error: %v", err) + } + if cfg.Media.ImageProxy.Enabled { + t.Error("IMAGE_PROXY_ENABLED=FALSE should disable the proxy") + } + }) +} diff --git a/internal/config/testing.go b/internal/config/testing.go index 2f4286a..fe91146 100644 --- a/internal/config/testing.go +++ b/internal/config/testing.go @@ -22,6 +22,17 @@ var loadedEnvVars = []string{ "JETSTREAM_FEEDS", "CURSOR_SECRET", "TURNSTILE_SITE_KEY", "TURNSTILE_SECRET_KEY", "TURNSTILE_SITEVERIFY_URL", + // The IMAGE_PROXY_* set is read by imageproxy.ConfigFromEnv rather than by + // this package — except IMAGE_PROXY_ENABLED, which Load re-reads through + // boolVar because it gates a security invariant. Either way this list is a + // hand-maintained mirror with no compile-time link to the parsing. A + // variable added there and not here leaks between tests silently — keep the + // two in sync, or move the parsing here. + "IMAGE_PROXY_ENABLED", "IMAGE_PROXY_BASE_URL", "IMAGE_PROXY_CDN_URL", + "IMAGE_PROXY_CACHE_PATH", "IMAGE_PROXY_CACHE_MAX_GB", "IMAGE_PROXY_CACHE_TTL_DAYS", + "IMAGE_PROXY_CLEANUP_INTERVAL_MINUTES", "IMAGE_PROXY_FETCH_TIMEOUT_SECONDS", + "IMAGE_PROXY_MAX_SOURCE_SIZE_MB", + "ALLOW_UNPROXIED_MEDIA", } // ClearEnvForTest blanks every environment variable Load reads, restoring them diff --git a/internal/core/blobs/blob_upload_integration_test.go b/internal/core/blobs/blob_upload_integration_test.go index 7f79bd4..392109d 100644 --- a/internal/core/blobs/blob_upload_integration_test.go +++ b/internal/core/blobs/blob_upload_integration_test.go @@ -71,6 +71,15 @@ func TestBlobUpload_E2E_PostWithImages(t *testing.T) { // Create test community with PDS credentials community := createTestCommunityWithBlobCredentials(t, communityRepo, "blobtest") + // Serve URLs the way production does: through the image proxy on the media + // hostname. The URL config is process-wide, so it is restored afterwards. + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ + ProxyEnabled: true, + ProxyBaseURL: "https://img.coves.social", + }) + t.Cleanup(blobs.ResetImageURLConfigForTesting) + t.Run("Post with single embedded image", func(t *testing.T) { // STEP 1: Create a test image blob (1x1 PNG) imageData := createTestPNG(t, 1, 1, color.RGBA{R: 255, G: 0, B: 0, A: 255}) @@ -183,12 +192,32 @@ func TestBlobUpload_E2E_PostWithImages(t *testing.T) { // Transform blob refs to URLs (this happens in feed handlers) posts.TransformBlobRefsToURLs(postView) - // NOTE: TransformBlobRefsToURLs only transforms external embed thumbs, - // not image embeds. For image embeds, clients fetch blobs using: - // GET /xrpc/com.atproto.sync.getBlob?did={did}&cid={cid} - // The blob reference is preserved in the embed for clients to construct URLs - - t.Logf("✓ Blob references preserved for client-side URL construction") + // Image embeds are hydrated server-side into fetchable image-proxy URLs. + // Clients no longer build blob URLs themselves: every image Coves serves + // has to travel through the proxy, which is the one hostname a scanning + // CDN can see. + transformedEmbed := postView.Embed.(map[string]interface{}) + assert.Equal(t, "social.coves.embed.images#view", transformedEmbed["$type"], + "served image embeds declare the view type") + + transformedImages := transformedEmbed["images"].([]interface{}) + require.Len(t, transformedImages, 1) + transformedImage := transformedImages[0].(map[string]interface{}) + + thumb, hasThumb := transformedImage["thumb"].(string) + fullsize, hasFullsize := transformedImage["fullsize"].(string) + require.True(t, hasThumb, "image should carry a thumb URL") + require.True(t, hasFullsize, "image should carry a fullsize URL") + + uploadedCID := blobRef.Ref["$link"] + assert.Contains(t, thumb, "/img/content_preview/plain/"+community.DID+"/"+uploadedCID) + assert.Contains(t, fullsize, "/img/content_full/plain/"+community.DID+"/"+uploadedCID) + assert.NotContains(t, thumb, "com.atproto.sync.getBlob", + "a direct PDS blob URL here would bypass the scanning CDN") + assert.Equal(t, "Test image", transformedImage["alt"], "alt text survives hydration") + assert.NotContains(t, transformedImage, "image", "the blob is replaced by the URLs") + + t.Logf("✓ Image embed hydrated to proxy URLs: thumb=%s fullsize=%s", thumb, fullsize) }) t.Run("Post with multiple images", func(t *testing.T) { @@ -330,20 +359,19 @@ func TestBlobUpload_E2E_PostWithImages(t *testing.T) { posts.TransformBlobRefsToURLs(postView) - // After transformation, thumb should be a URL string + // After transformation, thumb should be a proxy URL string transformedEmbed := postView.Embed.(map[string]interface{}) + assert.Equal(t, "social.coves.embed.external#view", transformedEmbed["$type"]) + transformedExternal := transformedEmbed["external"].(map[string]interface{}) thumbURL, isString := transformedExternal["thumb"].(string) + require.True(t, isString, "Thumb should be hydrated into a URL string") - // NOTE: TransformBlobRefsToURLs may keep it as a blob ref if transformation - // conditions aren't met. Check the actual implementation behavior. - if isString { - assert.Contains(t, thumbURL, "/xrpc/com.atproto.sync.getBlob", "Thumb should be blob URL") - assert.Contains(t, thumbURL, fmt.Sprintf("did=%s", community.DID), "URL should contain DID") - t.Logf("✓ Thumbnail transformed to URL: %s", thumbURL) - } else { - t.Logf("✓ Thumbnail preserved as blob ref (transformation skipped)") - } + assert.Contains(t, thumbURL, "/img/embed_thumbnail/plain/"+community.DID+"/", + "Thumb should be an image proxy URL owned by the community repo") + assert.NotContains(t, thumbURL, "com.atproto.sync.getBlob", + "a direct PDS blob URL here would bypass the scanning CDN") + t.Logf("✓ Thumbnail transformed to URL: %s", thumbURL) }) } diff --git a/internal/core/blobs/image_url_config.go b/internal/core/blobs/image_url_config.go new file mode 100644 index 0000000..2f571a7 --- /dev/null +++ b/internal/core/blobs/image_url_config.go @@ -0,0 +1,74 @@ +package blobs + +import ( + "log/slog" + "sync" +) + +// Process-wide image URL configuration. +// +// Every view builder in the AppView — post feeds, comment threads, profiles, +// community headers — needs the same answer to "what URL should a client fetch +// this blob from?", and none of them are constructed with that answer in hand. +// Threading one immutable value through a dozen service constructors buys +// nothing, so it is published once at startup and read wherever a view is +// rendered. +// +// It lives in blobs rather than in any one consumer: posts, comments, users, +// communities and the Postgres repos all read it, and all of them already +// depend on blobs for HydrateImageURL. +var ( + imageURLConfigMu sync.RWMutex + imageURLConfig ImageURLConfig + imageURLConfigSet bool +) + +// SetImageURLConfig publishes the image URL configuration for the process. +// The server wiring calls this once during startup, on every path — including +// the one where the proxy is disabled — because readers must know whether to +// render proxy URLs or direct blob URLs. +// +// The first call wins. A later call carrying a different configuration is +// ignored and logged: the value is read concurrently by request handlers, and +// swapping it mid-flight would hand different clients different URLs for the +// same blob. A repeat call is a wiring bug, not a reconfiguration hook. +func SetImageURLConfig(config ImageURLConfig) { + imageURLConfigMu.Lock() + defer imageURLConfigMu.Unlock() + + if imageURLConfigSet { + if config != imageURLConfig { + slog.Warn("[IMAGE-PROXY] SetImageURLConfig called again with a different configuration; ignoring", + "existing_proxy_enabled", imageURLConfig.ProxyEnabled, + "existing_proxy_base_url", imageURLConfig.ProxyBaseURL, + "existing_cdn_url", imageURLConfig.CDNURL, + "ignored_proxy_enabled", config.ProxyEnabled, + "ignored_proxy_base_url", config.ProxyBaseURL, + "ignored_cdn_url", config.CDNURL, + ) + } + return + } + + imageURLConfig = config + imageURLConfigSet = true +} + +// GetImageURLConfig returns the published image URL configuration. Safe for +// concurrent use. Before SetImageURLConfig runs it reports the proxy as +// disabled, which makes URL generation fall back to direct PDS blob URLs. +func GetImageURLConfig() ImageURLConfig { + imageURLConfigMu.RLock() + defer imageURLConfigMu.RUnlock() + return imageURLConfig +} + +// ResetImageURLConfigForTesting clears the published configuration so a test +// can publish its own. Tests only — production code must treat the +// configuration as write-once. +func ResetImageURLConfigForTesting() { + imageURLConfigMu.Lock() + defer imageURLConfigMu.Unlock() + imageURLConfig = ImageURLConfig{} + imageURLConfigSet = false +} diff --git a/internal/core/blobs/image_url_config_test.go b/internal/core/blobs/image_url_config_test.go new file mode 100644 index 0000000..71caec7 --- /dev/null +++ b/internal/core/blobs/image_url_config_test.go @@ -0,0 +1,124 @@ +package blobs + +import ( + "sync" + "testing" +) + +// The published configuration is process-wide, so every test here resets it +// first and on cleanup, and none of them may call t.Parallel. +func resetConfig(t *testing.T) { + t.Helper() + ResetImageURLConfigForTesting() + t.Cleanup(ResetImageURLConfigForTesting) +} + +// Before startup publishes anything, readers must see the proxy as disabled +// rather than some partially-initialized state. This is the value every view +// builder falls back to, and it is what makes URL generation degrade to direct +// PDS blob URLs — the configuration config.mediaProblems refuses to boot into, +// which is exactly why the default has to be predictable rather than accidental. +func TestGetImageURLConfig_BeforeSetReportsDisabled(t *testing.T) { + resetConfig(t) + + got := GetImageURLConfig() + + if got.ProxyEnabled { + t.Error("ProxyEnabled = true before SetImageURLConfig; the unpublished default must be disabled") + } + if got != (ImageURLConfig{}) { + t.Errorf("GetImageURLConfig() = %+v before publication, want the zero value", got) + } +} + +func TestSetImageURLConfig_PublishesTheValue(t *testing.T) { + resetConfig(t) + + want := ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: "https://img.coves.social"} + SetImageURLConfig(want) + + if got := GetImageURLConfig(); got != want { + t.Errorf("GetImageURLConfig() = %+v, want %+v", got, want) + } +} + +// The configuration is read concurrently by request handlers, so swapping it +// mid-flight would hand different clients different URLs for the same blob. A +// second call carrying a different value is a wiring bug and must be ignored, +// not applied. +func TestSetImageURLConfig_FirstCallWins(t *testing.T) { + resetConfig(t) + + first := ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: "https://img.coves.social"} + SetImageURLConfig(first) + + SetImageURLConfig(ImageURLConfig{ProxyEnabled: false, ProxyBaseURL: "https://other.example"}) + + if got := GetImageURLConfig(); got != first { + t.Errorf("GetImageURLConfig() = %+v after a conflicting second call, want the first value %+v", got, first) + } +} + +// A repeat call carrying the identical value is not a conflict — the latch just +// stays where it is. +func TestSetImageURLConfig_IdenticalRepeatIsHarmless(t *testing.T) { + resetConfig(t) + + want := ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: "https://img.coves.social"} + SetImageURLConfig(want) + SetImageURLConfig(want) + + if got := GetImageURLConfig(); got != want { + t.Errorf("GetImageURLConfig() = %+v, want %+v", got, want) + } +} + +func TestResetImageURLConfigForTesting_ClearsTheLatch(t *testing.T) { + resetConfig(t) + + SetImageURLConfig(ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: "https://img.coves.social"}) + ResetImageURLConfigForTesting() + + if got := GetImageURLConfig(); got != (ImageURLConfig{}) { + t.Errorf("GetImageURLConfig() = %+v after reset, want the zero value", got) + } + + // And the latch is genuinely re-armed, not merely blanked. + second := ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: "https://second.example"} + SetImageURLConfig(second) + if got := GetImageURLConfig(); got != second { + t.Errorf("GetImageURLConfig() = %+v, want %+v; reset must re-arm the write-once latch", got, second) + } +} + +// The RWMutex exists because handlers read this while startup may still be +// writing it. Run under -race, this is the test that justifies it. +func TestImageURLConfig_ConcurrentReadersAndWriters(t *testing.T) { + resetConfig(t) + + const goroutines = 16 + var wg sync.WaitGroup + start := make(chan struct{}) + + for i := 0; i < goroutines; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + <-start + if i%2 == 0 { + SetImageURLConfig(ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: "https://img.coves.social"}) + return + } + _ = GetImageURLConfig() + }(i) + } + + close(start) + wg.Wait() + + // Whichever writer got there first, the latch holds exactly one value and + // every reader saw a coherent one. + if got := GetImageURLConfig(); got.ProxyEnabled && got.ProxyBaseURL != "https://img.coves.social" { + t.Errorf("GetImageURLConfig() = %+v, want a value one writer actually published", got) + } +} diff --git a/internal/core/blobs/types.go b/internal/core/blobs/types.go index edf33cf..5a71ef4 100644 --- a/internal/core/blobs/types.go +++ b/internal/core/blobs/types.go @@ -41,7 +41,7 @@ func HydrateImageProxyURL(proxyBaseURL, preset, did, cid string) string { // ImageURLConfig holds configuration for image URL generation. type ImageURLConfig struct { ProxyEnabled bool // Whether the image proxy is enabled - ProxyBaseURL string // Base URL for the image proxy (e.g., "https://coves.social") + ProxyBaseURL string // Base URL for the image proxy (e.g., "https://img.coves.social") CDNURL string // Optional CDN override URL } @@ -70,10 +70,19 @@ func HydrateImageURL(config ImageURLConfig, pdsURL, did, cid, preset string) str // Generate proxy URL proxyURL := HydrateImageProxyURL(baseURL, preset, did, cid) - // If proxy URL generation failed (e.g., empty preset or base URL), fall back to direct URL - // Log this as it indicates a configuration problem when proxy is enabled + // Proxy URL construction failed. did and cid were already checked above and + // an empty base URL yields a *relative* proxy URL rather than "", so the + // only way to land here is an empty preset — a caller bug, not bad data. + // + // This returns "" rather than falling back to a direct blob URL. While the + // proxy is enabled, emitting a com.atproto.sync.getBlob URL is media routed + // around the CDN that scans it — precisely the state config.mediaProblems + // refuses to boot into — so it must not be reachable by quietly degrading + // at render time. Callers already treat "" as "no URL": embeds keeps the + // record shape, and avatar/banner fields are omitted. The failure surfaces + // as a missing image instead of as a silent hole in the choke point. if proxyURL == "" { - slog.Warn("[IMAGE-PROXY] proxy URL generation failed, falling back to direct PDS URL", + slog.Warn("[IMAGE-PROXY] proxy URL generation failed; omitting the image rather than serving it unproxied", "proxy_enabled", config.ProxyEnabled, "proxy_base_url", config.ProxyBaseURL, "cdn_url", config.CDNURL, @@ -81,7 +90,7 @@ func HydrateImageURL(config ImageURLConfig, pdsURL, did, cid, preset string) str "did", did, "cid", cid, ) - return HydrateBlobURL(pdsURL, did, cid) + return "" } return proxyURL diff --git a/internal/core/blobs/types_test.go b/internal/core/blobs/types_test.go index 3ef298f..f0bf781 100644 --- a/internal/core/blobs/types_test.go +++ b/internal/core/blobs/types_test.go @@ -234,23 +234,25 @@ func TestHydrateImageURL_CDNOverride(t *testing.T) { } } -func TestHydrateImageURL_EmptyPresetUsesDirectURL(t *testing.T) { +func TestHydrateImageURL_EmptyPresetReturnsEmptyRatherThanUnproxiedURL(t *testing.T) { + // An empty preset is the one way proxy URL construction can fail once did + // and cid are known non-empty, and it means a caller bug rather than bad + // data. + // + // The function must NOT degrade to a direct com.atproto.sync.getBlob URL + // here. While the proxy is enabled, that would be media routed around the + // CDN that scans it — the state config.mediaProblems refuses to boot into — + // reintroduced quietly at render time. Callers treat "" as "no URL", so the + // failure shows up as a missing image instead of a hole in the choke point. config := ImageURLConfig{ ProxyEnabled: true, - ProxyBaseURL: "https://coves.social", + ProxyBaseURL: "https://img.coves.social", } - pdsURL := "https://pds.example.com" - did := "did:plc:abc123" - cid := "bafyreiabc123" - preset := "" // empty preset - result := HydrateImageURL(config, pdsURL, did, cid, preset) + result := HydrateImageURL(config, "https://pds.example.com", "did:plc:abc123", "bafyreiabc123", "") - // With empty preset, proxy URL will return empty, so fall back to direct URL - // This tests the behavior when preset is not specified - expected := HydrateBlobURL(pdsURL, did, cid) - if result != expected { - t.Errorf("HydrateImageURL with empty preset = %q, want %q", result, expected) + if result != "" { + t.Errorf("HydrateImageURL with empty preset = %q, want \"\" (must not fall back to an unproxied URL)", result) } } diff --git a/internal/core/comments/comment_service.go b/internal/core/comments/comment_service.go index 3ac0bc7..94aac96 100644 --- a/internal/core/comments/comment_service.go +++ b/internal/core/comments/comment_service.go @@ -3,6 +3,7 @@ package comments import ( "Coves/internal/core/blobs" "Coves/internal/core/communities" + "Coves/internal/core/embeds" "Coves/internal/core/posts" "Coves/internal/core/richtext" "Coves/internal/core/users" @@ -11,7 +12,6 @@ import ( "errors" "fmt" "log/slog" - "net/url" "strings" "time" @@ -433,7 +433,7 @@ func hydrateAuthorProfile(view *posts.AuthorView, user *users.User) { displayName := user.DisplayName view.DisplayName = &displayName } - if avatarURL := blobs.HydrateImageURL(communities.GetImageProxyConfig(), user.PDSURL, user.DID, user.AvatarCID, "avatar_small"); avatarURL != "" { + if avatarURL := blobs.HydrateImageURL(blobs.GetImageURLConfig(), user.PDSURL, user.DID, user.AvatarCID, "avatar_small"); avatarURL != "" { view.Avatar = &avatarURL } } @@ -518,8 +518,23 @@ func (s *commentService) buildCommentView( // The record field is required by social.coves.community.comment.defs#commentView commentRecord := s.buildCommentRecord(comment) - // Deserialize embed from JSONB - // Parse embed from database JSON string to populate embed field + // Deserialize embed from JSONB and project it into its #view shape. + // + // Unlike posts — which the AppView signs into the community's repo — comment + // records live in the author's own repository, so the author's DID and PDS + // own every blob the embed references. + // + // The PDS URL is best-effort: an author who is not indexed yet leaves it + // empty. While the proxy is on that is harmless for the images embeds + // comments can carry, because the proxy resolves the DID itself; under + // ALLOW_UNPROXIED_MEDIA it is what leaves the embed unprojected. Any embed + // that cannot be fully projected keeps its blob references and logs; see + // internal/core/embeds. + // + // HydrateCommentView, not HydrateView: comments declare a narrower embed + // union than posts, and the firehose applies no embed validation, so a + // federated comment carrying a post-only embed type must not be stamped + // with a #view type the comment union does not declare. var embed interface{} if comment.Embed != nil && *comment.Embed != "" { var embedMap map[string]interface{} @@ -527,6 +542,11 @@ func (s *commentService) buildCommentView( // Log error but don't fail request - embed is optional slog.Warn("failed to unmarshal embed for comment", "comment_uri", comment.URI, "error", err) } else { + var authorPDSURL string + if user, found := usersByDID[comment.CommenterDID]; found && user != nil { + authorPDSURL = user.PDSURL + } + embeds.HydrateCommentView(embedMap, comment.CommenterDID, authorPDSURL) embed = embedMap } } @@ -1057,25 +1077,16 @@ func (s *commentService) buildPostView(ctx context.Context, post *posts.Post, vi communityName = community.Handle } - // Build avatar URL from CID if available - // Avatar is stored as blob in community's repository - // Format: https://{pds}/xrpc/com.atproto.sync.getBlob?did={community_did}&cid={avatar_cid} + // Build the community avatar URL through the shared helper, which routes it + // to the image proxy (avatar_small, matching post and feed community + // avatars). Hand-rolling a getBlob URL here served this one avatar around + // the proxy — the one place all media has to converge for scanning — and its + // HTTPS-only guard silently dropped the avatar in dev, where the PDS is + // plain HTTP. var avatarURL *string - if community.AvatarCID != "" && community.PDSURL != "" { - // Validate HTTPS for security (prevent mixed content warnings, MitM attacks) - if !strings.HasPrefix(community.PDSURL, "https://") { - slog.Warn("skipping non-HTTPS PDS URL for community", "community_did", community.DID) - } else if !strings.HasPrefix(community.AvatarCID, "baf") { - // Validate CID format (IPFS CIDs start with "baf" for CIDv1 base32) - slog.Warn("invalid CID format for community avatar", "community_did", community.DID, "avatar_cid", community.AvatarCID) - } else { - // Use proper URL escaping to prevent injection attacks - avatarURLString := fmt.Sprintf("%s/xrpc/com.atproto.sync.getBlob?did=%s&cid=%s", - strings.TrimSuffix(community.PDSURL, "/"), - url.QueryEscape(community.DID), - url.QueryEscape(community.AvatarCID)) - avatarURL = &avatarURLString - } + if hydrated := blobs.HydrateImageURL(blobs.GetImageURLConfig(), + community.PDSURL, community.DID, community.AvatarCID, "avatar_small"); hydrated != "" { + avatarURL = &hydrated } communityRef := &posts.CommunityRef{ diff --git a/internal/core/comments/comment_service_test.go b/internal/core/comments/comment_service_test.go index 06846ec..ffba5d0 100644 --- a/internal/core/comments/comment_service_test.go +++ b/internal/core/comments/comment_service_test.go @@ -1,6 +1,7 @@ package comments import ( + "Coves/internal/core/blobs" "Coves/internal/core/communities" "Coves/internal/core/posts" "Coves/internal/core/users" @@ -2436,3 +2437,93 @@ func TestValidateGetActorCommentsRequest_BoundsEnforcement(t *testing.T) { }) } } + +// Comment records live in the author's own repository — unlike posts, which the +// AppView signs into the community's repo — so the author's DID owns every blob +// in a comment embed. +func TestBuildCommentView_HydratesImageEmbedFromTheAuthorRepo(t *testing.T) { + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ + ProxyEnabled: true, + ProxyBaseURL: "https://img.coves.social", + }) + t.Cleanup(blobs.ResetImageURLConfigForTesting) + + commentRepo := newMockCommentRepo() + userRepo := newMockUserRepo() + postRepo := newMockPostRepo() + communityRepo := newMockCommunityRepo() + + const ( + commenterDID = "did:plc:commenter123" + imageCID = "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm" + ) + + postURI := "at://did:plc:post123/app.bsky.feed.post/test" + embedJSON := `{"$type":"social.coves.embed.images","images":[{"alt":"a cat","image":{"$type":"blob","ref":{"$link":"` + imageCID + `"},"mimeType":"image/jpeg","size":1234}}]}` + + comment := createTestComment("at://"+commenterDID+"/comment/1", commenterDID, "commenter.test", postURI, postURI, 0) + comment.Embed = &embedJSON + + service := NewCommentService(commentRepo, userRepo, postRepo, communityRepo, nil, nil, nil).(*commentService) + + usersByDID := map[string]*users.User{ + commenterDID: {DID: commenterDID, Handle: "commenter.test", PDSURL: "https://pds.example.com"}, + } + result := service.buildCommentView(comment, nil, nil, usersByDID) + + embedMap, ok := result.Embed.(map[string]interface{}) + require.True(t, ok) + assert.Equal(t, "social.coves.embed.images#view", embedMap["$type"]) + + images := embedMap["images"].([]interface{}) + require.Len(t, images, 1) + image := images[0].(map[string]interface{}) + + assert.Equal(t, + "https://img.coves.social/img/content_preview/plain/"+commenterDID+"/"+imageCID, + image["thumb"]) + assert.Equal(t, + "https://img.coves.social/img/content_full/plain/"+commenterDID+"/"+imageCID, + image["fullsize"]) + assert.Equal(t, "a cat", image["alt"]) + assert.NotContains(t, image, "image", "the blob is replaced by the URLs") +} + +// The image proxy resolves the DID to a PDS itself, so an author who is not yet +// indexed still gets proxy URLs rather than an unhydrated blob. +func TestBuildCommentView_HydratesEmbedForAnUnindexedAuthor(t *testing.T) { + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ + ProxyEnabled: true, + ProxyBaseURL: "https://img.coves.social", + }) + t.Cleanup(blobs.ResetImageURLConfigForTesting) + + commentRepo := newMockCommentRepo() + userRepo := newMockUserRepo() + postRepo := newMockPostRepo() + communityRepo := newMockCommunityRepo() + + const ( + commenterDID = "did:plc:commenter123" + imageCID = "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm" + ) + + postURI := "at://did:plc:post123/app.bsky.feed.post/test" + embedJSON := `{"$type":"social.coves.embed.images","images":[{"image":{"$type":"blob","ref":{"$link":"` + imageCID + `"}}}]}` + + comment := createTestComment("at://"+commenterDID+"/comment/1", commenterDID, "commenter.test", postURI, postURI, 0) + comment.Embed = &embedJSON + + service := NewCommentService(commentRepo, userRepo, postRepo, communityRepo, nil, nil, nil).(*commentService) + + // No entry in usersByDID: the author has not been indexed, so no PDS URL. + result := service.buildCommentView(comment, nil, nil, make(map[string]*users.User)) + + embedMap := result.Embed.(map[string]interface{}) + image := embedMap["images"].([]interface{})[0].(map[string]interface{}) + assert.Equal(t, + "https://img.coves.social/img/content_preview/plain/"+commenterDID+"/"+imageCID, + image["thumb"]) +} diff --git a/internal/core/communities/community.go b/internal/core/communities/community.go index dd385f2..798b6cf 100644 --- a/internal/core/communities/community.go +++ b/internal/core/communities/community.go @@ -4,52 +4,16 @@ import ( "fmt" "log" "strings" - "sync" "time" "Coves/internal/core/blobs" ) -// imageProxyConfigOnce ensures thread-safe initialization of the image proxy config. -var imageProxyConfigOnce sync.Once - -// imageProxyConfig holds the immutable configuration after initialization. -// Access only through GetImageProxyConfig(). -var imageProxyConfig = blobs.ImageURLConfig{ - ProxyEnabled: false, // Default to disabled until configured -} - -// imageProxyConfigInitialized tracks whether SetImageProxyConfig has been called. -var imageProxyConfigInitialized bool - -// SetImageProxyConfig initializes the image proxy configuration. -// This should be called once during server startup. Subsequent calls are no-ops -// and will log a warning. This design ensures thread-safety and prevents -// accidental config changes during runtime. -func SetImageProxyConfig(config blobs.ImageURLConfig) { - imageProxyConfigOnce.Do(func() { - imageProxyConfig = config - imageProxyConfigInitialized = true - }) - // Log warning if called multiple times (indicates a programming error) - if imageProxyConfigInitialized && config != imageProxyConfig { - log.Printf("WARN: SetImageProxyConfig called multiple times with different config (ignored)") - } -} - -// GetImageProxyConfig returns the current image proxy configuration. -// Thread-safe for concurrent access. -func GetImageProxyConfig() blobs.ImageURLConfig { - return imageProxyConfig -} - -// ResetImageProxyConfigForTesting resets the config state for testing purposes. -// This should ONLY be used in tests, never in production code. -func ResetImageProxyConfigForTesting() { - imageProxyConfigOnce = sync.Once{} - imageProxyConfig = blobs.ImageURLConfig{ProxyEnabled: false} - imageProxyConfigInitialized = false -} +// communityHandlePrefix namespaces community actor handles apart from user +// actor handles on the PDS: a community named "gardening" on coves.social is +// provisioned as c-gardening.coves.social. Communities bridged in from other +// platforms keep their source handle and carry no prefix. +const communityHandlePrefix = "c-" // Community represents a Coves community indexed from the firehose // Communities are federated, instance-scoped forums built on atProto @@ -298,7 +262,7 @@ func (c *Community) ToCommunityView() *CommunityView { Name: c.Name, DisplayName: c.DisplayName, DisplayHandle: c.GetDisplayHandle(), - Avatar: blobs.HydrateImageURL(GetImageProxyConfig(), c.PDSURL, c.DID, c.AvatarCID, "avatar_small"), + Avatar: blobs.HydrateImageURL(blobs.GetImageURLConfig(), c.PDSURL, c.DID, c.AvatarCID, "avatar_small"), Visibility: c.Visibility, SubscriberCount: c.SubscriberCount, MemberCount: c.MemberCount, @@ -319,8 +283,8 @@ func (c *Community) ToCommunityViewDetailed() *CommunityViewDetailed { DisplayName: c.DisplayName, DisplayHandle: c.GetDisplayHandle(), Description: c.Description, - Avatar: blobs.HydrateImageURL(GetImageProxyConfig(), c.PDSURL, c.DID, c.AvatarCID, "avatar"), - Banner: blobs.HydrateImageURL(GetImageProxyConfig(), c.PDSURL, c.DID, c.BannerCID, "banner"), + Avatar: blobs.HydrateImageURL(blobs.GetImageURLConfig(), c.PDSURL, c.DID, c.AvatarCID, "avatar"), + Banner: blobs.HydrateImageURL(blobs.GetImageURLConfig(), c.PDSURL, c.DID, c.BannerCID, "banner"), CreatedByDID: c.CreatedByDID, HostedByDID: c.HostedByDID, Visibility: c.Visibility, diff --git a/internal/core/communities/community_view_test.go b/internal/core/communities/community_view_test.go index 94741d8..9258f6b 100644 --- a/internal/core/communities/community_view_test.go +++ b/internal/core/communities/community_view_test.go @@ -234,8 +234,9 @@ func TestGetDisplayHandle(t *testing.T) { // The preset each view asks for is unobservable with the image proxy disabled: // blobs.HydrateImageURL ignores the preset entirely and returns the PDS blob URL // for all three. Turning the proxy on is the only way to see the difference, and -// the switch is a package-level variable behind a sync.Once — process-global, -// with no injection seam. So this test owns that global for its duration. +// the switch is a package-level variable in blobs behind a write-once latch — +// process-global, with no injection seam. So this test owns that global for its +// duration. // // It is safe here for a reason that is worth writing down rather than assuming: // Go runs every non-parallel top-level test to completion before resuming any @@ -246,14 +247,14 @@ func TestGetDisplayHandle(t *testing.T) { // hydrating an image URL, this test and that one will race, and the fix is a // config seam on the view functions rather than a mutex here. func TestCommunityViews_ImageProxyPresets(t *testing.T) { - communities.ResetImageProxyConfigForTesting() - t.Cleanup(communities.ResetImageProxyConfigForTesting) + blobs.ResetImageURLConfigForTesting() + t.Cleanup(blobs.ResetImageURLConfigForTesting) - communities.SetImageProxyConfig(blobs.ImageURLConfig{ + blobs.SetImageURLConfig(blobs.ImageURLConfig{ ProxyEnabled: true, ProxyBaseURL: "https://images.invalid", }) - require.True(t, communities.GetImageProxyConfig().ProxyEnabled, + require.True(t, blobs.GetImageURLConfig().ProxyEnabled, "the config did not take; every assertion below would silently fall back to the direct PDS URL "+ "and pass for the wrong reason") @@ -278,26 +279,3 @@ func TestCommunityViews_ImageProxyPresets(t *testing.T) { "URLs directly is what the proxy exists to stop") assert.NotContains(t, list.Avatar, community.PDSURL) } - -// TestSetImageProxyConfig_IsWriteOnce pins the one-shot semantics, which are -// what make GetImageProxyConfig safe to read without a lock from every request. -func TestSetImageProxyConfig_IsWriteOnce(t *testing.T) { - communities.ResetImageProxyConfigForTesting() - t.Cleanup(communities.ResetImageProxyConfigForTesting) - - assert.False(t, communities.GetImageProxyConfig().ProxyEnabled, - "the default must be proxy-disabled: a server that never configures the proxy has to serve "+ - "working PDS URLs rather than URLs pointing at an empty host") - - first := blobs.ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: "https://first.invalid"} - communities.SetImageProxyConfig(first) - assert.Equal(t, first, communities.GetImageProxyConfig()) - - // A second call is a programming error — the value is read without - // synchronisation everywhere — so it is ignored rather than applied. - communities.SetImageProxyConfig(blobs.ImageURLConfig{ - ProxyEnabled: true, ProxyBaseURL: "https://second.invalid"}) - assert.Equal(t, first, communities.GetImageProxyConfig(), - "a second configuration was applied. Every reader holds the value unlocked, so a mutating "+ - "setter is a data race in production, not just a surprise") -} diff --git a/internal/core/communities/identifier_resolution_test.go b/internal/core/communities/identifier_resolution_test.go new file mode 100644 index 0000000..c135513 --- /dev/null +++ b/internal/core/communities/identifier_resolution_test.go @@ -0,0 +1,157 @@ +package communities + +import ( + "context" + "errors" + "testing" +) + +// stubHandleRepo implements only the lookups ResolveCommunityIdentifier needs. +// The embedded nil Repository panics loudly if resolution ever reaches for +// anything else, which keeps the stub honest as the interface grows. +type stubHandleRepo struct { + Repository + byHandle map[string]*Community + byDID map[string]*Community + + handleLookups []string + getByHandleFn func(handle string) (*Community, error) +} + +func (r *stubHandleRepo) GetByHandle(_ context.Context, handle string) (*Community, error) { + r.handleLookups = append(r.handleLookups, handle) + if r.getByHandleFn != nil { + return r.getByHandleFn(handle) + } + if c, ok := r.byHandle[handle]; ok { + return c, nil + } + return nil, ErrCommunityNotFound +} + +func (r *stubHandleRepo) GetByDID(_ context.Context, did string) (*Community, error) { + if c, ok := r.byDID[did]; ok { + return c, nil + } + return nil, ErrCommunityNotFound +} + +func newStubRepo(communities ...*Community) *stubHandleRepo { + repo := &stubHandleRepo{ + byHandle: make(map[string]*Community, len(communities)), + byDID: make(map[string]*Community, len(communities)), + } + for _, c := range communities { + repo.byHandle[c.Handle] = c + repo.byDID[c.DID] = c + } + return repo +} + +func TestResolveCommunityIdentifier_HandleForms(t *testing.T) { + // A community provisioned on this instance: handle carries the c- prefix. + local := &Community{DID: "did:plc:local123", Handle: "c-gardening.coves.social", Name: "gardening"} + // A bridged community: handle is the source platform's, no prefix. + bridged := &Community{DID: "did:plc:bridged456", Handle: "selfhosted.lemmy-world.tdpl.io", Name: "selfhosted"} + + tests := []struct { + name string + identifier string + wantDID string + wantLookups []string + }{ + { + name: "prefixed handle resolves on the first lookup", + identifier: "c-gardening.coves.social", + wantDID: local.DID, + wantLookups: []string{"c-gardening.coves.social"}, + }, + { + name: "bare handle falls back to the prefixed form", + identifier: "gardening.coves.social", + wantDID: local.DID, + wantLookups: []string{"gardening.coves.social", "c-gardening.coves.social"}, + }, + { + name: "bridged handle resolves without a prefixed retry", + identifier: "selfhosted.lemmy-world.tdpl.io", + wantDID: bridged.DID, + wantLookups: []string{"selfhosted.lemmy-world.tdpl.io"}, + }, + { + name: "at-identifier prefix is stripped before lookup", + identifier: "@gardening.coves.social", + wantDID: local.DID, + wantLookups: []string{"gardening.coves.social", "c-gardening.coves.social"}, + }, + { + name: "handle is lowercased before lookup", + identifier: "Gardening.Coves.Social", + wantDID: local.DID, + wantLookups: []string{"gardening.coves.social", "c-gardening.coves.social"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + repo := newStubRepo(local, bridged) + svc := &communityService{repo: repo, instanceDomain: "coves.social"} + + did, err := svc.ResolveCommunityIdentifier(context.Background(), tt.identifier) + if err != nil { + t.Fatalf("ResolveCommunityIdentifier(%q) returned error: %v", tt.identifier, err) + } + if did != tt.wantDID { + t.Errorf("ResolveCommunityIdentifier(%q) = %q, want %q", tt.identifier, did, tt.wantDID) + } + if len(repo.handleLookups) != len(tt.wantLookups) { + t.Fatalf("handle lookups = %v, want %v", repo.handleLookups, tt.wantLookups) + } + for i, want := range tt.wantLookups { + if repo.handleLookups[i] != want { + t.Errorf("handle lookup %d = %q, want %q", i, repo.handleLookups[i], want) + } + } + }) + } +} + +func TestResolveCommunityIdentifier_UnknownHandle(t *testing.T) { + repo := newStubRepo() + svc := &communityService{repo: repo, instanceDomain: "coves.social"} + + _, err := svc.ResolveCommunityIdentifier(context.Background(), "nope.coves.social") + if !errors.Is(err, ErrCommunityNotFound) { + t.Fatalf("error = %v, want it to wrap ErrCommunityNotFound", err) + } + // Both forms must be tried before reporting the miss. + want := []string{"nope.coves.social", "c-nope.coves.social"} + if len(repo.handleLookups) != len(want) { + t.Fatalf("handle lookups = %v, want %v", repo.handleLookups, want) + } + for i, w := range want { + if repo.handleLookups[i] != w { + t.Errorf("handle lookup %d = %q, want %q", i, repo.handleLookups[i], w) + } + } +} + +// A database failure must surface as itself, not be reported as "not found" — +// and must not trigger the prefixed retry, which would mask the outage. +func TestResolveCommunityIdentifier_RepoErrorIsNotSwallowed(t *testing.T) { + dbDown := errors.New("connection refused") + repo := newStubRepo() + repo.getByHandleFn = func(string) (*Community, error) { return nil, dbDown } + svc := &communityService{repo: repo, instanceDomain: "coves.social"} + + _, err := svc.ResolveCommunityIdentifier(context.Background(), "gardening.coves.social") + if !errors.Is(err, dbDown) { + t.Fatalf("error = %v, want it to wrap the repository error", err) + } + if errors.Is(err, ErrCommunityNotFound) { + t.Error("repository failure was misreported as ErrCommunityNotFound") + } + if len(repo.handleLookups) != 1 { + t.Errorf("handle lookups = %v, want a single attempt with no prefixed retry", repo.handleLookups) + } +} diff --git a/internal/core/communities/service.go b/internal/core/communities/service.go index ca5fdf1..33ce400 100644 --- a/internal/core/communities/service.go +++ b/internal/core/communities/service.go @@ -1108,11 +1108,32 @@ func (s *communityService) ResolveCommunityIdentifier(ctx context.Context, ident // 4. Canonical handle: name.community.instance.com (Bluesky standard) if strings.Contains(identifier, ".") { - community, err := s.repo.GetByHandle(ctx, strings.ToLower(identifier)) - if err != nil { - return "", fmt.Errorf("community not found for handle %s: %w", identifier, err) + handle := strings.ToLower(identifier) + + community, err := s.repo.GetByHandle(ctx, handle) + if err == nil { + return community.DID, nil } - return community.DID, nil + if !IsNotFound(err) { + return "", fmt.Errorf("failed to look up community handle %s: %w", identifier, err) + } + + // Communities provisioned on this instance store a "c-" prefixed handle + // (c-gardening.coves.social) that namespaces community actors apart from + // user actors. Clients display and link to the prefix-free form, so retry + // the prefixed handle before giving up. Bridged communities are stored + // without the prefix and resolve on the first lookup above. + if !strings.HasPrefix(handle, communityHandlePrefix) { + community, prefixedErr := s.repo.GetByHandle(ctx, communityHandlePrefix+handle) + if prefixedErr == nil { + return community.DID, nil + } + if !IsNotFound(prefixedErr) { + return "", fmt.Errorf("failed to look up community handle %s: %w", communityHandlePrefix+handle, prefixedErr) + } + } + + return "", fmt.Errorf("community not found for handle %s: %w", identifier, err) } return "", NewValidationError("identifier", "must be a DID, handle, or scoped identifier (!name@instance)") diff --git a/internal/core/embeds/view.go b/internal/core/embeds/view.go new file mode 100644 index 0000000..1d4628e --- /dev/null +++ b/internal/core/embeds/view.go @@ -0,0 +1,348 @@ +// Package embeds projects the atproto embed union from the shape it has on the +// wire between repositories — blob references — into the shape the AppView +// serves to clients: fetchable URLs. +// +// Doing this on the server rather than in each client is a moderation +// requirement, not a convenience. Media is meant to converge on a single +// CDN-fronted hostname, because a scanner can only match content that crosses +// its edge, and a view that hands a client a com.atproto.sync.getBlob URL +// routes around it. Centralizing URL construction here also means changing the +// CDN, adding a preset, or purging a blob needs no client release. +// +// Two limits on that claim, both deliberate and both recorded in the PRD's +// residual-gaps table: +// +// - Video is exempt. The proxy transcodes stills and cannot stream, so +// video#view points at the hosting PDS (see projectVideo). +// - This projects the *view*, not the record. Post and comment responses also +// carry the verbatim atproto record, whose embed keeps its blob references +// — the same bytes any client can already read from the PDS. The invariant +// is that the AppView does not construct unproxied URLs, not that a blob +// reference never reaches a client. +// +// The CDN routing itself is workstream 3 and is configured outside this repo; +// until it is in place this package is emitting the right URLs at a hostname +// that is not yet scanned. See docs/PRD_CSAM_SCANNING.md. +package embeds + +import ( + "log/slog" + + "Coves/internal/core/blobs" +) + +// Embed union member $type discriminators, and the "#view" projections of the +// ones that carry blobs. Keep in sync with the union refs declared in +// social.coves.community.post.defs#postView and +// social.coves.community.comment.defs#commentView. +const ( + TypeImages = "social.coves.embed.images" + TypeVideo = "social.coves.embed.video" + TypeExternal = "social.coves.embed.external" + TypePost = "social.coves.embed.post" + + // viewSuffix marks the served projection of a record type. A view carries + // URL strings where the record carries blobs, so it must not claim to be + // the record type: readers key off $type, and a client that trusts the + // record schema would look for a ref.$link that is no longer there. + viewSuffix = "#view" +) + +// Image proxy presets used for embedded media. The registry that defines their +// dimensions lives in internal/core/imageproxy; these names are the contract +// between it and the URLs we emit. +const ( + presetEmbedThumbnail = "embed_thumbnail" // 720x360 cover — external link cards + presetContentPreview = "content_preview" // 800w contain — in-feed image + presetContentFull = "content_full" // 1600w contain — lightbox / detail +) + +// mutation is a single deferred write into an embed map. +// +// Projection is computed before anything is written, and the writes are applied +// only once the whole embed is known to project. Mutating as we go produced a +// torn embed: a thumbnail already rewritten to a URL under an embed that then +// failed on its gallery and kept its record $type, leaving a URI string where +// the record schema declares a blob. That state was also unrecoverable — the +// rewritten field no longer carries a CID, so a later pass could not finish the +// job. Staging the writes makes the all-or-nothing rule structural rather than +// something each helper has to remember. +type mutation func() + +// HydrateView rewrites embed in place from its record shape into its #view +// shape, replacing every blob reference it understands with a URL. +// +// ownerDID and ownerPDSURL identify the repository holding the embed's blobs. +// That is the community for post embeds — the AppView signs community post +// records into the community's repo and uploads their blobs there — and the +// comment author for comment embeds, whose records live in the user's own repo. +// +// Hydration is all-or-nothing: an embed projects completely or is left exactly +// as it arrived, still carrying its blob references and its record $type. It is +// idempotent because a projected embed's $type is a #view type, which this +// function does not act on. +// +// social.coves.embed.post carries no blobs; it is projected to its own #view by +// posts.TransformPostEmbeds, which resolves the quoted record. +func HydrateView(embed map[string]interface{}, ownerDID, ownerPDSURL string) { + if embed == nil { + return + } + + embedType, _ := embed["$type"].(string) + + // Types this function does not act on are not failures and get no log. That + // includes the #view types of the ones it does act on, which is what makes + // it idempotent, and social.coves.embed.post, which carries no blobs. + switch embedType { + case TypeExternal, TypeImages, TypeVideo: + default: + return + } + + // The #view suffix is a promise about shape — #viewImage requires thumb and + // fullsize, video#view requires a video URI — so it is only stamped once the + // projection has actually delivered that shape. Claiming the view type over + // an embed still carrying blobs would be worse than not projecting at all: a + // client that switched on #view would find none of the fields the schema + // guarantees and render nothing, with no error anywhere. + var commits []mutation + var projected bool + + // Every path that leaves projected false falls through to the warning + // below. These used to return early and silently — an embed whose owning + // repository is unknown, or whose union member arrived in a shape the view + // cannot declare, produced exactly the same missing image as the logged + // cases with nothing anywhere to explain it. + if ownerDID != "" { + switch embedType { + case TypeExternal: + // A record that reached the index without the external object has + // no media to hydrate and no view to declare. + if external, isObject := embed["external"].(map[string]interface{}); isObject { + commits, projected = projectExternal(external, ownerDID, ownerPDSURL) + } + + case TypeImages: + // social.coves.embed.images#view requires a non-empty list (the + // lexicon sets minLength: 1), so an empty one is a malformed record + // rather than an absent gallery. The sibling gallery on + // #viewExternal is optional and treats empty as nothing-to-do; that + // distinction lives here rather than inside projectImages, which + // serves both. + if images, isList := embed["images"].([]interface{}); isList && len(images) > 0 { + commits, projected = projectImages(images, ownerDID, ownerPDSURL) + } + + case TypeVideo: + commits, projected = projectVideo(embed, ownerDID, ownerPDSURL) + } + } + + if !projected { + // Reached whenever any part of the embed could not be turned into a + // URL: an unknown owning repository, a blob whose encoding we do not + // recognize, a union member in a shape the view cannot declare, an + // empty images list, or — the configuration case — the proxy disabled + // with no PDS URL on the owning repo to fall back to. The visible + // symptom is a missing image, which otherwise has no explanation, so it + // is worth a line. + slog.Warn("[EMBED-VIEW] embed could not be projected to its view shape; serving the record shape", + "embed_type", embedType, + "owner_did", ownerDID, + "owner_pds_url", ownerPDSURL, + "proxy_enabled", blobs.GetImageURLConfig().ProxyEnabled, + ) + return + } + + for _, commit := range commits { + commit() + } + embed["$type"] = embedType + viewSuffix +} + +// HydrateCommentView projects a comment's embed, restricted to the union that +// comments actually declare. +// +// Comments carry a narrower union than posts: social.coves.embed.images and +// social.coves.embed.post, on the served view +// (social.coves.community.comment.defs#commentView) and on the create and +// update inputs alike. Posts additionally allow video and external. +// +// The restriction has to be enforced here because nothing upstream enforces it. +// Comment records live in the author's own repository and reach the index +// through the firehose, which applies no embed validation — only the create +// endpoint does, and a federated peer never goes through it. So a comment +// carrying a video or external embed is a shape we can receive, and running the +// full projection over it would stamp a #view type the comment union does not +// declare. A client switching on the union would find a member it has no case +// for; left in record shape it at least names a type the client can recognize +// and skip. +func HydrateCommentView(embed map[string]interface{}, ownerDID, ownerPDSURL string) { + if embed == nil { + return + } + + // TypePost carries no blobs and is projected by posts.TransformPostEmbeds, + // which resolves the quoted record; anything other than images is outside + // the comment union entirely. + if embedType, _ := embed["$type"].(string); embedType != TypeImages { + return + } + + HydrateView(embed, ownerDID, ownerPDSURL) +} + +// projectExternal computes the URL-bearing fields of +// social.coves.embed.external#viewExternal: the link card thumbnail, plus the +// gallery preview images an image-hosting provider can contribute. +// +// Both are optional there, so an external embed carrying no media — including +// one whose gallery is present but empty — projects successfully. Only a blob +// we failed to turn into a URL, or a field whose shape the view cannot declare, +// fails it. +func projectExternal(external map[string]interface{}, ownerDID, ownerPDSURL string) ([]mutation, bool) { + var commits []mutation + + if cid := blobCID(external["thumb"]); cid != "" { + url := imageURL(ownerPDSURL, ownerDID, cid, presetEmbedThumbnail) + if url == "" { + return nil, false + } + commits = append(commits, func() { external["thumb"] = url }) + } else if _, isBlob := external["thumb"].(map[string]interface{}); isBlob { + // A thumb object we could not read a CID from would survive into the + // view as an object where the schema declares a URI string. + return nil, false + } + + raw, present := external["images"] + if !present { + return commits, true + } + images, isList := raw.([]interface{}) + if !isList { + // Present but not a list: it would survive as a shape #viewExternal + // does not declare. + return nil, false + } + + imageCommits, ok := projectImages(images, ownerDID, ownerPDSURL) + if !ok { + return nil, false + } + return append(commits, imageCommits...), true +} + +// projectImages computes the #viewImage projection for every entry in the list: +// the two rendered sizes clients display — thumb for the feed, fullsize for the +// lightbox — leaving alt and aspectRatio untouched. +// +// Every entry projects or none does. A half-hydrated array — some entries with +// URLs, some still carrying blobs — satisfies neither schema and would force a +// client to handle both shapes inside one list. +// +// An empty list yields no work and succeeds; callers that require a non-empty +// list assert that themselves. +func projectImages(images []interface{}, ownerDID, ownerPDSURL string) ([]mutation, bool) { + commits := make([]mutation, 0, len(images)) + + for _, entry := range images { + image, isObject := entry.(map[string]interface{}) + if !isObject { + return nil, false + } + + cid := blobCID(image["image"]) + if cid == "" { + return nil, false + } + + thumb := imageURL(ownerPDSURL, ownerDID, cid, presetContentPreview) + fullsize := imageURL(ownerPDSURL, ownerDID, cid, presetContentFull) + if thumb == "" || fullsize == "" { + return nil, false + } + + commits = append(commits, func() { + delete(image, "image") + image["thumb"] = thumb + image["fullsize"] = fullsize + }) + } + + return commits, true +} + +// projectVideo computes social.coves.embed.video#view. +// +// The still is served through the image proxy like any other image. The video +// blob is not: the proxy decodes and re-encodes images and cannot stream video, +// so its URL points at the hosting PDS directly. That URL is the one piece of +// Coves-served media the scanning CDN never sees — the known, accepted gap +// recorded in docs/PRD_CSAM_SCANNING.md workstream 5, closed later by +// ingest-time hash matching rather than by scan-on-serve. +// +// video is required on the view; thumbnail is optional, so a video with no +// still still projects. +func projectVideo(embed map[string]interface{}, ownerDID, ownerPDSURL string) ([]mutation, bool) { + videoCID := blobCID(embed["video"]) + if videoCID == "" { + return nil, false + } + videoURL := blobs.HydrateBlobURL(ownerPDSURL, ownerDID, videoCID) + if videoURL == "" { + return nil, false + } + commits := []mutation{func() { embed["video"] = videoURL }} + + if cid := blobCID(embed["thumbnail"]); cid != "" { + url := imageURL(ownerPDSURL, ownerDID, cid, presetContentPreview) + if url == "" { + return nil, false + } + commits = append(commits, func() { embed["thumbnail"] = url }) + } else if _, isBlob := embed["thumbnail"].(map[string]interface{}); isBlob { + return nil, false + } + + return commits, true +} + +// imageURL renders a proxy URL for a blob under the given preset, reading the +// process-wide configuration published at startup. +func imageURL(pdsURL, did, cid, preset string) string { + return blobs.HydrateImageURL(blobs.GetImageURLConfig(), pdsURL, did, cid, preset) +} + +// blobCID extracts the CID from an atproto blob reference. +// +// It returns "" for anything that is not a blob carrying a CID, including a ref +// in a shape we do not recognize, which is left as-is rather than guessed at. +// +// Two encodings are accepted. ref.$link is the current form, and is the only +// one consulted when ref is an object — a ref object without a usable $link +// yields "" rather than falling through. A top-level cid is the legacy blob +// encoding that predates the CID-link format, used whenever ref is absent or is +// not an object; records carrying it are still in circulation on the network +// and reach our index through federation. +func blobCID(value interface{}) string { + blob, ok := value.(map[string]interface{}) + if !ok { + return "" + } + + if ref, ok := blob["ref"].(map[string]interface{}); ok { + if cid, ok := ref["$link"].(string); ok { + return cid + } + return "" + } + + if cid, ok := blob["cid"].(string); ok { + return cid + } + + return "" +} diff --git a/internal/core/embeds/view_test.go b/internal/core/embeds/view_test.go new file mode 100644 index 0000000..b6af801 --- /dev/null +++ b/internal/core/embeds/view_test.go @@ -0,0 +1,670 @@ +package embeds + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "Coves/internal/core/blobs" +) + +const ( + testDID = "did:plc:testcommunity" + testPDS = "http://localhost:3001" // coves:allow-host-literal: expected-output fixture for a pure URL projection; never dialled + testCID = "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm" +) + +// withProxy enables the image proxy at the given base URL for the rest of the +// test. The URL configuration is process-wide, so it is restored on cleanup — +// which also means no test in this package may call t.Parallel. +func withProxy(t *testing.T, baseURL string) { + t.Helper() + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ProxyEnabled: true, ProxyBaseURL: baseURL}) + t.Cleanup(blobs.ResetImageURLConfigForTesting) +} + +// withProxyDisabled turns the proxy off for the rest of the test — the +// configuration a self-hosted deployment gets when it opts out of proxied +// media. Process-wide, restored on cleanup; see withProxy. +func withProxyDisabled(t *testing.T) { + t.Helper() + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ProxyEnabled: false}) + t.Cleanup(blobs.ResetImageURLConfigForTesting) +} + +func blobRef(cid string) map[string]interface{} { + return map[string]interface{}{ + "$type": "blob", + "ref": map[string]interface{}{"$link": cid}, + "mimeType": "image/jpeg", + "size": 52813, + } +} + +func TestHydrateView_External(t *testing.T) { + t.Run("thumb becomes an embed_thumbnail proxy URL and the type becomes a view", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://example.com/article", + "title": "An article", + "thumb": blobRef(testCID), + }, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeExternal+"#view", embed["$type"]) + external := embed["external"].(map[string]interface{}) + assert.Equal(t, + "https://img.coves.social/img/embed_thumbnail/plain/"+testDID+"/"+testCID, + external["thumb"]) + assert.Equal(t, "An article", external["title"], "unrelated fields are preserved") + }) + + t.Run("gallery preview images are hydrated too", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://imgur.com/a/abc", + "images": []interface{}{ + map[string]interface{}{ + "image": blobRef(testCID), + "alt": "first", + }, + }, + }, + } + + HydrateView(embed, testDID, testPDS) + + external := embed["external"].(map[string]interface{}) + images := external["images"].([]interface{}) + image := images[0].(map[string]interface{}) + + assert.Equal(t, + "https://img.coves.social/img/content_preview/plain/"+testDID+"/"+testCID, + image["thumb"]) + assert.Equal(t, + "https://img.coves.social/img/content_full/plain/"+testDID+"/"+testCID, + image["fullsize"]) + assert.NotContains(t, image, "image", "the blob is replaced, not left alongside the URLs") + assert.Equal(t, "first", image["alt"]) + }) + + t.Run("an external embed without a thumb still declares the view type", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{"uri": "https://example.com"}, + } + + HydrateView(embed, testDID, testPDS) + + // The served shape is the view shape whether or not media is present; + // a client must not have to guess which schema it received. + assert.Equal(t, TypeExternal+"#view", embed["$type"]) + }) + + // The gallery is optional on #viewExternal and an empty array carries no + // media, so it is nothing to do rather than a failure. The sibling images + // embed treats empty as malformed (lexicon minLength: 1) — the two must not + // share one rule just because they share a helper. + t.Run("an empty gallery is nothing to do, not a failure", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://example.com", + "thumb": blobRef(testCID), + "images": []interface{}{}, + }, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeExternal+viewSuffix, embed["$type"]) + external := embed["external"].(map[string]interface{}) + assert.Equal(t, + "https://img.coves.social/img/embed_thumbnail/plain/"+testDID+"/"+testCID, + external["thumb"]) + }) + + // The regression that made staging necessary: the thumb used to be written + // before the gallery was attempted, so a gallery failure left a URL string + // under a record $type — a shape matching neither schema, and unrecoverable, + // since the rewritten thumb no longer carried a CID for a later pass. + t.Run("a failing gallery leaves the thumb unmutated", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + thumb := blobRef(testCID) + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://example.com", + "thumb": thumb, + "images": []interface{}{ + map[string]interface{}{"image": map[string]interface{}{"$type": "blob"}}, + }, + }, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeExternal, embed["$type"], + "a partial projection must not claim the view type") + external := embed["external"].(map[string]interface{}) + assert.Equal(t, thumb, external["thumb"], + "the thumb must not be committed when the gallery cannot project") + }) + + t.Run("a gallery that is not a list fails rather than being stamped", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://example.com", + "thumb": blobRef(testCID), + "images": map[string]interface{}{"not": "a list"}, + }, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeExternal, embed["$type"], + "a shape #viewExternal cannot declare must not be stamped as a view") + external := embed["external"].(map[string]interface{}) + assert.IsType(t, map[string]interface{}{}, external["thumb"], + "the thumb must not be committed") + }) + + t.Run("an unreadable thumb object fails without touching the gallery", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + gallery := map[string]interface{}{"image": blobRef(testCID)} + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://example.com", + "thumb": map[string]interface{}{"$type": "blob"}, + "images": []interface{}{gallery}, + }, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeExternal, embed["$type"]) + assert.Contains(t, gallery, "image", "no commit runs when any part fails") + assert.NotContains(t, gallery, "thumb") + }) + + t.Run("a malformed external embed is left untouched", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": "not-an-object", + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeExternal, embed["$type"], "no view is claimed for a shape we could not project") + }) +} + +func TestHydrateView_Images(t *testing.T) { + t.Run("each image gets both rendered sizes and keeps its metadata", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + secondCID := "bafyreicaqaqvvlyzhgqmvhkzqvmtcwrgrqzxbxnwqxqvvvxqxqvvxqxqvv" + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{ + map[string]interface{}{ + "image": blobRef(testCID), + "alt": "a cat", + "aspectRatio": map[string]interface{}{"width": 4, "height": 3}, + }, + map[string]interface{}{ + "image": blobRef(secondCID), + }, + }, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeImages+"#view", embed["$type"]) + images := embed["images"].([]interface{}) + require.Len(t, images, 2) + + first := images[0].(map[string]interface{}) + assert.Equal(t, + "https://img.coves.social/img/content_preview/plain/"+testDID+"/"+testCID, + first["thumb"]) + assert.Equal(t, + "https://img.coves.social/img/content_full/plain/"+testDID+"/"+testCID, + first["fullsize"]) + assert.Equal(t, "a cat", first["alt"]) + assert.Equal(t, + map[string]interface{}{"width": 4, "height": 3}, + first["aspectRatio"], + "aspectRatio is a client rendering hint and must survive projection") + + second := images[1].(map[string]interface{}) + assert.Contains(t, second["thumb"], secondCID) + assert.NotContains(t, second, "alt", "absent optional fields are not invented") + }) + + t.Run("the legacy blob encoding is recognized", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + // Federated records predating the CID-link format carry the CID at the + // top level with no ref. They are still on the network, so they still + // reach our index. + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{ + "image": map[string]interface{}{"$type": "blob", "cid": testCID, "mimeType": "image/jpeg"}, + }}, + } + + HydrateView(embed, testDID, testPDS) + + image := embed["images"].([]interface{})[0].(map[string]interface{}) + assert.Contains(t, image["thumb"], testCID) + }) + + t.Run("an unrecognized ref shape is not guessed at", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + malformed := map[string]interface{}{"$type": "blob", "ref": "not-a-ref-object"} + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": malformed}}, + } + + HydrateView(embed, testDID, testPDS) + + image := embed["images"].([]interface{})[0].(map[string]interface{}) + assert.Equal(t, malformed, image["image"], + "treating the ref as a CID would emit a proxy URL that can only 400") + }) + + t.Run("hydration is idempotent", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": blobRef(testCID), "alt": "a cat"}}, + } + + HydrateView(embed, testDID, testPDS) + first := embed["images"].([]interface{})[0].(map[string]interface{}) + thumb, fullsize := first["thumb"], first["fullsize"] + + // A second pass runs on a view whose $type no longer matches a record + // type, so it is a no-op — but even reaching the image list it would + // find strings, not blobs. + HydrateView(embed, testDID, testPDS) + + second := embed["images"].([]interface{})[0].(map[string]interface{}) + assert.Equal(t, thumb, second["thumb"]) + assert.Equal(t, fullsize, second["fullsize"]) + }) + + t.Run("an image with no resolvable CID keeps its blob rather than losing it", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + unresolvable := map[string]interface{}{"$type": "blob", "mimeType": "image/jpeg"} + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": unresolvable}}, + } + + HydrateView(embed, testDID, testPDS) + + image := embed["images"].([]interface{})[0].(map[string]interface{}) + assert.Equal(t, unresolvable, image["image"], + "dropping the blob would make the image unrecoverable for any client") + assert.NotContains(t, image, "thumb") + assert.Equal(t, TypeImages, embed["$type"], + "an embed still carrying a blob must not claim the view type") + }) + + // social.coves.embed.images#view sets minLength: 1, so an empty list is a + // malformed record — the opposite of the optional gallery on #viewExternal. + t.Run("an empty image list is malformed, not empty", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{}, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeImages, embed["$type"], + "the view requires at least one image, so there is no view to declare") + }) + + t.Run("one unprojectable image leaves the whole set unhydrated", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + // A partially hydrated array — some entries with URLs, some with blobs — + // satisfies neither schema, so a client would have to handle both shapes + // inside one list. All or nothing instead. + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{ + map[string]interface{}{"image": blobRef(testCID)}, + map[string]interface{}{"image": map[string]interface{}{"$type": "blob"}}, + }, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeImages, embed["$type"]) + first := embed["images"].([]interface{})[0].(map[string]interface{}) + assert.NotContains(t, first, "thumb", "the projectable entry is not committed either") + assert.Contains(t, first, "image") + }) +} + +// The #view suffix promises a shape: #viewImage requires thumb and fullsize, +// video#view requires a video URI. Stamping it over an embed that still carries +// blobs is worse than leaving the record type alone — a client switching on +// #view finds none of the guaranteed fields and renders nothing, with no error +// raised anywhere. This is reachable in the supported self-hosted configuration +// (proxy disabled) whenever the owning repo has no indexed PDS URL: a community +// row with a null pds_url, or a comment author who is not indexed yet. +func TestHydrateView_NeverClaimsAViewItCouldNotProduce(t *testing.T) { + tests := []struct { + name string + embed map[string]interface{} + }{ + { + name: "images", + embed: map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": blobRef(testCID), "alt": "a cat"}}, + }, + }, + { + name: "video", + embed: map[string]interface{}{ + "$type": TypeVideo, + "video": blobRef(testCID), + }, + }, + { + name: "external with a thumb", + embed: map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://example.com", + "thumb": blobRef(testCID), + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + withProxyDisabled(t) + recordType := tt.embed["$type"] + + // No proxy and no PDS URL: there is no URL to build from anything. + HydrateView(tt.embed, testDID, "") + + assert.Equal(t, recordType, tt.embed["$type"], + "the record type must survive when the view could not be produced") + }) + } +} + +// An external embed carrying no media at all is fully projectable — thumb and +// images are both optional on #viewExternal — so it still declares the view. +func TestHydrateView_ExternalWithNoMediaProjects(t *testing.T) { + withProxyDisabled(t) + + embed := map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{"uri": "https://example.com", "title": "An article"}, + } + + HydrateView(embed, testDID, "") + + assert.Equal(t, TypeExternal+viewSuffix, embed["$type"]) + assert.NotContains(t, embed["external"], "thumb", + "projection must not invent a thumb field on an embed that had none") +} + +// Comments declare a narrower embed union than posts: images and post only, on +// the served view and on the create/update inputs alike. Comment records reach +// the index from the author's own repository over the firehose, which applies no +// embed validation, so a post-only embed type is a shape we can actually +// receive — and stamping it with a #view type the comment union does not declare +// would hand clients a union member they have no case for. +func TestHydrateCommentView_RestrictsToTheCommentUnion(t *testing.T) { + t.Run("images hydrate exactly as they do on posts", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": blobRef(testCID), "alt": "a cat"}}, + } + + HydrateCommentView(embed, testDID, testPDS) + + assert.Equal(t, TypeImages+viewSuffix, embed["$type"]) + image := embed["images"].([]interface{})[0].(map[string]interface{}) + assert.Equal(t, "https://img.coves.social/img/content_preview/plain/"+testDID+"/"+testCID, image["thumb"]) + assert.Equal(t, "https://img.coves.social/img/content_full/plain/"+testDID+"/"+testCID, image["fullsize"]) + assert.Equal(t, "a cat", image["alt"]) + }) + + for _, tc := range []struct { + name string + embed map[string]interface{} + }{ + { + name: "video is post-only and must be left in record shape", + embed: map[string]interface{}{ + "$type": TypeVideo, + "video": blobRef(testCID), + }, + }, + { + name: "external is post-only and must be left in record shape", + embed: map[string]interface{}{ + "$type": TypeExternal, + "external": map[string]interface{}{ + "uri": "https://example.com/article", + "thumb": blobRef(testCID), + }, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + withProxy(t, "https://img.coves.social") + recordType := tc.embed["$type"] + + HydrateCommentView(tc.embed, testDID, testPDS) + + assert.Equal(t, recordType, tc.embed["$type"], + "a type outside the comment union must keep its record $type") + assert.NotContains(t, tc.embed["$type"], viewSuffix) + }) + } + + t.Run("the blob references survive untouched so the record stays readable", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeVideo, + "video": blobRef(testCID), + } + + HydrateCommentView(embed, testDID, testPDS) + + video, isBlob := embed["video"].(map[string]interface{}) + assert.True(t, isBlob, "video must still be a blob reference, not a URL string") + assert.Equal(t, testCID, video["ref"].(map[string]interface{})["$link"]) + }) + + t.Run("a nil embed is a no-op", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + HydrateCommentView(nil, testDID, testPDS) + }) +} + +func TestHydrateView_Video(t *testing.T) { + t.Run("the still goes through the proxy and the video goes to the PDS", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + videoCID := "bafyreivideoqvvlyzhgqmvhkzqvmtcwrgrqzxbxnwqxqvvvxqxqvvxqxqv" + embed := map[string]interface{}{ + "$type": TypeVideo, + "video": blobRef(videoCID), + "thumbnail": blobRef(testCID), + "duration": 42, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypeVideo+"#view", embed["$type"]) + assert.Equal(t, + "https://img.coves.social/img/content_preview/plain/"+testDID+"/"+testCID, + embed["thumbnail"]) + + // The image proxy cannot stream video, so this URL is the accepted + // scanning gap recorded in the CSAM PRD (workstream 5). It must point + // at the hosting PDS, not at the proxy, which would 400 on the blob. + assert.Equal(t, + blobs.HydrateBlobURL(testPDS, testDID, videoCID), + embed["video"]) + assert.Equal(t, 42, embed["duration"]) + }) + + t.Run("a video with no PDS URL keeps its blob", func(t *testing.T) { + withProxy(t, "https://img.coves.social") + + embed := map[string]interface{}{ + "$type": TypeVideo, + "video": blobRef(testCID), + } + + HydrateView(embed, testDID, "") + + assert.IsType(t, map[string]interface{}{}, embed["video"], + "without a PDS URL there is no video URL to emit, so the ref must survive") + }) +} + +func TestHydrateView_ProxyDisabled(t *testing.T) { + // The self-hosted opt-out: no proxy, so URLs address the PDS directly. + // Exercised because it is a supported deployment, not a fallback we hope + // never runs. + t.Run("images fall back to direct PDS blob URLs", func(t *testing.T) { + withProxyDisabled(t) + + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": blobRef(testCID)}}, + } + + HydrateView(embed, testDID, testPDS) + + image := embed["images"].([]interface{})[0].(map[string]interface{}) + direct := blobs.HydrateBlobURL(testPDS, testDID, testCID) + assert.Equal(t, direct, image["thumb"]) + assert.Equal(t, direct, image["fullsize"]) + }) + + t.Run("without a PDS URL there is nothing to fall back to, so blobs survive", func(t *testing.T) { + withProxyDisabled(t) + + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": blobRef(testCID)}}, + } + + HydrateView(embed, testDID, "") + + image := embed["images"].([]interface{})[0].(map[string]interface{}) + assert.NotContains(t, image, "thumb") + assert.Contains(t, image, "image") + }) +} + +func TestHydrateView_LeavesOtherEmbedsAlone(t *testing.T) { + withProxy(t, "https://img.coves.social") + + t.Run("post embeds carry no blobs and are projected elsewhere", func(t *testing.T) { + embed := map[string]interface{}{ + "$type": TypePost, + "post": map[string]interface{}{"uri": "at://did:plc:x/app.bsky.feed.post/abc", "cid": "bafy"}, + } + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, TypePost, embed["$type"]) + }) + + t.Run("an unknown embed type is passed through untouched", func(t *testing.T) { + embed := map[string]interface{}{"$type": "social.coves.embed.future", "data": "x"} + + HydrateView(embed, testDID, testPDS) + + assert.Equal(t, "social.coves.embed.future", embed["$type"]) + assert.Equal(t, "x", embed["data"]) + }) + + t.Run("no owner DID means no URL can be built", func(t *testing.T) { + embed := map[string]interface{}{ + "$type": TypeImages, + "images": []interface{}{map[string]interface{}{"image": blobRef(testCID)}}, + } + + HydrateView(embed, "", testPDS) + + assert.Equal(t, TypeImages, embed["$type"]) + }) + + t.Run("a nil embed does not panic", func(t *testing.T) { + assert.NotPanics(t, func() { HydrateView(nil, testDID, testPDS) }) + }) +} + +func TestBlobCID(t *testing.T) { + tests := []struct { + name string + value interface{} + want string + }{ + {"spec blob ref", blobRef(testCID), testCID}, + {"legacy top-level cid", map[string]interface{}{"$type": "blob", "cid": testCID}, testCID}, + {"unrecognized string ref", map[string]interface{}{"ref": testCID}, ""}, + {"already hydrated URL", "https://img.coves.social/img/x/plain/y/z", ""}, + {"empty link", map[string]interface{}{"ref": map[string]interface{}{"$link": ""}}, ""}, + {"non-string link", map[string]interface{}{"ref": map[string]interface{}{"$link": 42}}, ""}, + {"nil", nil, ""}, + {"number", 7, ""}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, blobCID(tt.value)) + }) + } +} diff --git a/internal/core/posts/blob_transform.go b/internal/core/posts/blob_transform.go index a5b812c..3e8e161 100644 --- a/internal/core/posts/blob_transform.go +++ b/internal/core/posts/blob_transform.go @@ -3,90 +3,37 @@ package posts import ( "context" "errors" - "fmt" "log" "strings" "Coves/internal/core/blueskypost" + "Coves/internal/core/embeds" ) -// TransformBlobRefsToURLs transforms all blob references in a PostView to PDS URLs -// This modifies the Embed field in-place, converting blob refs to direct URLs -// The transformation only affects external embeds with thumbnail blobs +// TransformBlobRefsToURLs projects a post's embed from its record shape into +// the #view shape served to clients, replacing blob references with fetchable +// image-proxy URLs. It modifies the Embed field in place. +// +// Post embeds resolve against the community's repository: the AppView signs +// community post records into the community's PDS and uploads their blobs +// there, so the community DID owns every blob in the embed regardless of who +// authored the post. +// +// Must run before the response is written. A post whose embed still carries +// blob references forces the client to build its own blob URLs, which routes +// media around the image proxy and therefore around CSAM scanning — see +// internal/core/embeds. func TransformBlobRefsToURLs(postView *PostView) { - if postView == nil || postView.Embed == nil { + if postView == nil || postView.Embed == nil || postView.Community == nil { return } - // Get community PDS URL from post view - if postView.Community == nil || postView.Community.PDSURL == "" { - return // Cannot transform without PDS URL - } - - communityDID := postView.Community.DID - pdsURL := postView.Community.PDSURL - - // Check if embed is a map (should be for external embeds) embedMap, ok := postView.Embed.(map[string]interface{}) if !ok { return } - // Check embed type - embedType, ok := embedMap["$type"].(string) - if !ok { - return - } - - // Only transform external embeds - if embedType == "social.coves.embed.external" { - if external, ok := embedMap["external"].(map[string]interface{}); ok { - transformThumbToURL(external, communityDID, pdsURL) - // The served shape no longer matches the record schema (thumb is a - // URL string, not a blob), so declare the view type on the wire - embedMap["$type"] = "social.coves.embed.external#view" - } - } -} - -// transformThumbToURL converts a thumb blob ref to a PDS URL -// This modifies the external map in-place -func transformThumbToURL(external map[string]interface{}, communityDID, pdsURL string) { - // Check if thumb exists - thumb, ok := external["thumb"] - if !ok { - return - } - - // If thumb is already a string (URL), don't transform - if _, isString := thumb.(string); isString { - return - } - - // Try to parse as blob ref - thumbMap, ok := thumb.(map[string]interface{}) - if !ok { - return - } - - // Extract CID from blob ref - ref, ok := thumbMap["ref"].(map[string]interface{}) - if !ok { - return - } - - cid, ok := ref["$link"].(string) - if !ok || cid == "" { - return - } - - // Transform to PDS blob endpoint URL - // Format: {pds_url}/xrpc/com.atproto.sync.getBlob?did={community_did}&cid={cid} - blobURL := fmt.Sprintf("%s/xrpc/com.atproto.sync.getBlob?did=%s&cid=%s", - pdsURL, communityDID, cid) - - // Replace blob ref with URL string - external["thumb"] = blobURL + embeds.HydrateView(embedMap, postView.Community.DID, postView.Community.PDSURL) } // TransformPostEmbeds enriches post embeds with resolved Bluesky post data diff --git a/internal/core/posts/blob_transform_test.go b/internal/core/posts/blob_transform_test.go index 75b5571..b7636fa 100644 --- a/internal/core/posts/blob_transform_test.go +++ b/internal/core/posts/blob_transform_test.go @@ -5,320 +5,200 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" -) - -// testPDSBaseURL is the base URL the transforms under test concatenate onto. -// -// It is data, not an endpoint. TransformBlobRefsToURLs and transformThumbToURL -// take the PDS base off the record (CommunityRef.PDSURL) or as a parameter and -// build a getBlob path from it; there is no HTTP client in this file and -// nothing here is dialled. Reading the base from testkit.Endpoints() — the -// same place the serving code reads it — would make the expected strings -// tautological, so it is written down once here instead, which also keeps the -// assertions focused on the part the function actually builds: the path and -// its query. -const testPDSBaseURL = "http://localhost:3001" // coves:allow-host-literal: expected-output fixture for a pure string transform; never dialled -func TestTransformBlobRefsToURLs(t *testing.T) { - t.Run("transforms external embed thumb from blob to URL", func(t *testing.T) { - post := &PostView{ - Community: &CommunityRef{ - DID: "did:plc:testcommunity", - PDSURL: testPDSBaseURL, - }, - Embed: map[string]interface{}{ - "$type": "social.coves.embed.external", - "external": map[string]interface{}{ - "uri": "https://example.com", - "thumb": map[string]interface{}{ - "$type": "blob", - "ref": map[string]interface{}{ - "$link": "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm", - }, - "mimeType": "image/jpeg", - "size": 52813, - }, - }, - }, - } + "Coves/internal/core/blobs" +) - TransformBlobRefsToURLs(post) +const ( + embedCommunityDID = "did:plc:testcommunity" + embedCommunityPDS = "http://localhost:3001" // coves:allow-host-literal: expected-output fixture for a pure string transform; never dialled + embedBlobCID = "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm" +) - // Verify embed is still a map - embedMap, ok := post.Embed.(map[string]interface{}) - require.True(t, ok, "embed should still be a map") +// withImageProxy enables the process-wide image proxy for one test. The +// projection itself is covered in internal/core/embeds; these cases are about +// what the posts layer contributes — that the community owns the blobs, and +// that a post with nothing to project survives untouched. +func withImageProxy(t *testing.T) { + t.Helper() + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ + ProxyEnabled: true, + ProxyBaseURL: "https://img.coves.social", + }) + t.Cleanup(blobs.ResetImageURLConfigForTesting) +} - // Verify external is still a map - external, ok := embedMap["external"].(map[string]interface{}) - require.True(t, ok, "external should be a map") +func testBlobRef() map[string]interface{} { + return map[string]interface{}{ + "$type": "blob", + "ref": map[string]interface{}{"$link": embedBlobCID}, + "mimeType": "image/jpeg", + "size": 52813, + } +} - // Verify thumb is now a URL string - thumbURL, ok := external["thumb"].(string) - require.True(t, ok, "thumb should be a string URL") - assert.Equal(t, - testPDSBaseURL+"/xrpc/com.atproto.sync.getBlob?did=did:plc:testcommunity&cid=bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm", - thumbURL) - }) +func externalEmbedPost() *PostView { + return &PostView{ + Community: &CommunityRef{DID: embedCommunityDID, PDSURL: embedCommunityPDS}, + Embed: map[string]interface{}{ + "$type": "social.coves.embed.external", + "external": map[string]interface{}{ + "uri": "https://example.com", + "thumb": testBlobRef(), + }, + }, + } +} - t.Run("handles missing thumb gracefully", func(t *testing.T) { - post := &PostView{ - Community: &CommunityRef{ - DID: "did:plc:testcommunity", - PDSURL: testPDSBaseURL, - }, - Embed: map[string]interface{}{ - "$type": "social.coves.embed.external", - "external": map[string]interface{}{ - "uri": "https://example.com", - // No thumb field - }, - }, - } +func TestTransformBlobRefsToURLs(t *testing.T) { + t.Run("an external thumb is served from the image proxy under the community DID", func(t *testing.T) { + withImageProxy(t) - // Should not panic + post := externalEmbedPost() TransformBlobRefsToURLs(post) - // Verify external is unchanged embedMap := post.Embed.(map[string]interface{}) + assert.Equal(t, "social.coves.embed.external#view", embedMap["$type"]) + external := embedMap["external"].(map[string]interface{}) - _, hasThumb := external["thumb"] - assert.False(t, hasThumb, "thumb should not be added") + // The AppView signs community posts into the community's repo and + // uploads their blobs there, so the community DID owns the blob no + // matter who authored the post. + assert.Equal(t, + "https://img.coves.social/img/embed_thumbnail/plain/"+embedCommunityDID+"/"+embedBlobCID, + external["thumb"]) }) - t.Run("handles already-transformed URL thumb", func(t *testing.T) { - expectedURL := testPDSBaseURL + "/xrpc/com.atproto.sync.getBlob?did=did:plc:test&cid=bafytest" + t.Run("image embeds are hydrated, not skipped", func(t *testing.T) { + withImageProxy(t) + post := &PostView{ - Community: &CommunityRef{ - DID: "did:plc:testcommunity", - PDSURL: testPDSBaseURL, - }, + Community: &CommunityRef{DID: embedCommunityDID, PDSURL: embedCommunityPDS}, Embed: map[string]interface{}{ - "$type": "social.coves.embed.external", - "external": map[string]interface{}{ - "uri": "https://example.com", - "thumb": expectedURL, // Already a URL string + "$type": "social.coves.embed.images", + "images": []interface{}{ + map[string]interface{}{"image": testBlobRef(), "alt": "a cat"}, }, }, } - // Should not error or change the URL TransformBlobRefsToURLs(post) - // Verify thumb is unchanged embedMap := post.Embed.(map[string]interface{}) - external := embedMap["external"].(map[string]interface{}) - thumbURL, ok := external["thumb"].(string) - require.True(t, ok, "thumb should still be a string") - assert.Equal(t, expectedURL, thumbURL, "thumb URL should be unchanged") + assert.Equal(t, "social.coves.embed.images#view", embedMap["$type"]) + + image := embedMap["images"].([]interface{})[0].(map[string]interface{}) + assert.Equal(t, + "https://img.coves.social/img/content_preview/plain/"+embedCommunityDID+"/"+embedBlobCID, + image["thumb"]) + assert.Equal(t, + "https://img.coves.social/img/content_full/plain/"+embedCommunityDID+"/"+embedBlobCID, + image["fullsize"]) + assert.Equal(t, "a cat", image["alt"]) }) - t.Run("handles missing embed", func(t *testing.T) { - post := &PostView{ - Community: &CommunityRef{ - DID: "did:plc:testcommunity", - PDSURL: testPDSBaseURL, - }, - Embed: nil, - } + t.Run("no image URL the AppView emits addresses a PDS blob endpoint", func(t *testing.T) { + withImageProxy(t) - // Should not panic + post := externalEmbedPost() TransformBlobRefsToURLs(post) - // Verify embed is still nil - assert.Nil(t, post.Embed, "embed should remain nil") + external := post.Embed.(map[string]interface{})["external"].(map[string]interface{}) + // The whole point of the proxy: a getBlob URL here would be media + // served around the CDN that scans it. + assert.NotContains(t, external["thumb"], "com.atproto.sync.getBlob") }) - t.Run("handles nil post", func(t *testing.T) { - // Should not panic - TransformBlobRefsToURLs(nil) - }) + t.Run("an already-hydrated thumb is left alone", func(t *testing.T) { + withImageProxy(t) - t.Run("handles missing community", func(t *testing.T) { + hydrated := "https://img.coves.social/img/embed_thumbnail/plain/did:plc:test/bafytest" post := &PostView{ - Community: nil, + Community: &CommunityRef{DID: embedCommunityDID, PDSURL: embedCommunityPDS}, Embed: map[string]interface{}{ "$type": "social.coves.embed.external", "external": map[string]interface{}{ - "uri": "https://example.com", - "thumb": map[string]interface{}{ - "$type": "blob", - "ref": map[string]interface{}{ - "$link": "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm", - }, - }, + "uri": "https://example.com", + "thumb": hydrated, }, }, } - // Should not panic or transform TransformBlobRefsToURLs(post) - // Verify thumb is unchanged (still a blob) - embedMap := post.Embed.(map[string]interface{}) - external := embedMap["external"].(map[string]interface{}) - thumb, ok := external["thumb"].(map[string]interface{}) - require.True(t, ok, "thumb should still be a map (blob ref)") - assert.Equal(t, "blob", thumb["$type"], "blob type should be unchanged") + external := post.Embed.(map[string]interface{})["external"].(map[string]interface{}) + assert.Equal(t, hydrated, external["thumb"]) }) - t.Run("handles missing PDS URL", func(t *testing.T) { - post := &PostView{ - Community: &CommunityRef{ - DID: "did:plc:testcommunity", - PDSURL: "", // Empty PDS URL - }, - Embed: map[string]interface{}{ - "$type": "social.coves.embed.external", - "external": map[string]interface{}{ - "uri": "https://example.com", - "thumb": map[string]interface{}{ - "$type": "blob", - "ref": map[string]interface{}{ - "$link": "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm", - }, - }, - }, - }, - } - - // Should not panic or transform - TransformBlobRefsToURLs(post) + t.Run("a post with no community is left untouched", func(t *testing.T) { + withImageProxy(t) - // Verify thumb is unchanged (still a blob) - embedMap := post.Embed.(map[string]interface{}) - external := embedMap["external"].(map[string]interface{}) - thumb, ok := external["thumb"].(map[string]interface{}) - require.True(t, ok, "thumb should still be a map (blob ref)") - assert.Equal(t, "blob", thumb["$type"], "blob type should be unchanged") - }) + post := externalEmbedPost() + post.Community = nil - t.Run("handles malformed blob ref gracefully", func(t *testing.T) { - post := &PostView{ - Community: &CommunityRef{ - DID: "did:plc:testcommunity", - PDSURL: testPDSBaseURL, - }, - Embed: map[string]interface{}{ - "$type": "social.coves.embed.external", - "external": map[string]interface{}{ - "uri": "https://example.com", - "thumb": map[string]interface{}{ - "$type": "blob", - "ref": "invalid-ref-format", // Should be a map with $link - }, - }, - }, - } - - // Should not panic TransformBlobRefsToURLs(post) - // Verify thumb is unchanged (malformed blob) - embedMap := post.Embed.(map[string]interface{}) - external := embedMap["external"].(map[string]interface{}) + external := post.Embed.(map[string]interface{})["external"].(map[string]interface{}) thumb, ok := external["thumb"].(map[string]interface{}) - require.True(t, ok, "thumb should still be a map") - assert.Equal(t, "invalid-ref-format", thumb["ref"], "malformed ref should be unchanged") + require.True(t, ok, "with no owning repo there is no URL to build, so the blob must survive") + assert.Equal(t, "blob", thumb["$type"]) }) - t.Run("ignores non-external embed types", func(t *testing.T) { - post := &PostView{ - Community: &CommunityRef{ - DID: "did:plc:testcommunity", - PDSURL: testPDSBaseURL, - }, - Embed: map[string]interface{}{ - "$type": "social.coves.embed.images", - "images": []interface{}{ - map[string]interface{}{ - "image": map[string]interface{}{ - "$type": "blob", - "ref": map[string]interface{}{ - "$link": "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm", - }, - }, - }, - }, - }, - } + t.Run("the proxy resolves the DID itself, so a missing PDS URL is not fatal", func(t *testing.T) { + withImageProxy(t) - // Should not transform non-external embeds - TransformBlobRefsToURLs(post) + post := externalEmbedPost() + post.Community.PDSURL = "" - // Verify images embed is unchanged - embedMap := post.Embed.(map[string]interface{}) - images := embedMap["images"].([]interface{}) - imageObj := images[0].(map[string]interface{}) - imageBlob := imageObj["image"].(map[string]interface{}) - assert.Equal(t, "blob", imageBlob["$type"], "image blob should be unchanged") - }) -} - -func TestTransformThumbToURL(t *testing.T) { - t.Run("transforms valid blob ref to URL", func(t *testing.T) { - external := map[string]interface{}{ - "uri": "https://example.com", - "thumb": map[string]interface{}{ - "$type": "blob", - "ref": map[string]interface{}{ - "$link": "bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm", - }, - "mimeType": "image/jpeg", - "size": 52813, - }, - } - - transformThumbToURL(external, "did:plc:test", testPDSBaseURL) + TransformBlobRefsToURLs(post) - thumbURL, ok := external["thumb"].(string) - require.True(t, ok, "thumb should be a string URL") + external := post.Embed.(map[string]interface{})["external"].(map[string]interface{}) assert.Equal(t, - testPDSBaseURL+"/xrpc/com.atproto.sync.getBlob?did=did:plc:test&cid=bafyreib6tbnql2ux3whnfysbzabthaj2vvck53nimhbi5g5a7jgvgr5eqm", - thumbURL) + "https://img.coves.social/img/embed_thumbnail/plain/"+embedCommunityDID+"/"+embedBlobCID, + external["thumb"], + "the PDS URL is only needed for the proxy-disabled fallback") }) - t.Run("does not transform if thumb is already string", func(t *testing.T) { - expectedURL := testPDSBaseURL + "/xrpc/com.atproto.sync.getBlob?did=did:plc:test&cid=bafytest" - external := map[string]interface{}{ - "uri": "https://example.com", - "thumb": expectedURL, - } + t.Run("nil inputs do not panic", func(t *testing.T) { + withImageProxy(t) - transformThumbToURL(external, "did:plc:test", testPDSBaseURL) + assert.NotPanics(t, func() { TransformBlobRefsToURLs(nil) }) - thumbURL, ok := external["thumb"].(string) - require.True(t, ok, "thumb should still be a string") - assert.Equal(t, expectedURL, thumbURL, "thumb should be unchanged") + post := &PostView{ + Community: &CommunityRef{DID: embedCommunityDID, PDSURL: embedCommunityPDS}, + Embed: nil, + } + TransformBlobRefsToURLs(post) + assert.Nil(t, post.Embed) }) - t.Run("does not transform if thumb is missing", func(t *testing.T) { - external := map[string]interface{}{ - "uri": "https://example.com", + t.Run("an embed that is not an object is left untouched", func(t *testing.T) { + withImageProxy(t) + + post := &PostView{ + Community: &CommunityRef{DID: embedCommunityDID, PDSURL: embedCommunityPDS}, + Embed: "not-an-object", } - transformThumbToURL(external, "did:plc:test", testPDSBaseURL) + TransformBlobRefsToURLs(post) - _, hasThumb := external["thumb"] - assert.False(t, hasThumb, "thumb should not be added") + assert.Equal(t, "not-an-object", post.Embed) }) +} - t.Run("does not transform if CID is empty", func(t *testing.T) { - external := map[string]interface{}{ - "uri": "https://example.com", - "thumb": map[string]interface{}{ - "$type": "blob", - "ref": map[string]interface{}{ - "$link": "", // Empty CID - }, - }, - } +func TestTransformBlobRefsToURLs_ProxyDisabled(t *testing.T) { + // The self-hosted opt-out (ALLOW_UNPROXIED_MEDIA): URLs address the + // community's PDS directly. + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ProxyEnabled: false}) + t.Cleanup(blobs.ResetImageURLConfigForTesting) - transformThumbToURL(external, "did:plc:test", testPDSBaseURL) + post := externalEmbedPost() + TransformBlobRefsToURLs(post) - // Verify thumb is unchanged - thumb, ok := external["thumb"].(map[string]interface{}) - require.True(t, ok, "thumb should still be a map") - ref := thumb["ref"].(map[string]interface{}) - assert.Equal(t, "", ref["$link"], "empty CID should be unchanged") - }) + external := post.Embed.(map[string]interface{})["external"].(map[string]interface{}) + assert.Equal(t, + blobs.HydrateBlobURL(embedCommunityPDS, embedCommunityDID, embedBlobCID), + external["thumb"]) } diff --git a/internal/core/posts/embed_validation.go b/internal/core/posts/embed_validation.go index 96fd734..342c135 100644 --- a/internal/core/posts/embed_validation.go +++ b/internal/core/posts/embed_validation.go @@ -3,6 +3,7 @@ package posts import ( "fmt" + "Coves/internal/core/embeds" "Coves/internal/validation" ) @@ -11,14 +12,20 @@ import ( // union refs declared in the post.create lexicon — a ref added there must be // added here too, or it will be rejected at the API boundary as unknown. // +// The names come from internal/core/embeds, which is the package that reads +// these same discriminators when projecting a stored embed into its view. Two +// independent spellings of one lexicon contract can drift, and the failure — +// the validator accepting a type the projector then ignores — is silent. +// // The get endpoint projects embeds through a separate, output-only "#view" -// union (e.g. social.coves.embed.record#view); those view types are never -// valid on create input and are correctly rejected here as unknown. +// union (social.coves.embed.images#view and the sibling video, external and +// post projections); those view types are never valid on create input and are +// correctly rejected here as unknown. const ( - embedTypeImages = "social.coves.embed.images" - embedTypeVideo = "social.coves.embed.video" - embedTypeExternal = "social.coves.embed.external" - embedTypePost = "social.coves.embed.post" + embedTypeImages = embeds.TypeImages + embedTypeVideo = embeds.TypeVideo + embedTypeExternal = embeds.TypeExternal + embedTypePost = embeds.TypePost ) // maxEmbedSources mirrors the maxLength on the sources array in diff --git a/internal/core/users/service.go b/internal/core/users/service.go index f1d8542..1dbdebc 100644 --- a/internal/core/users/service.go +++ b/internal/core/users/service.go @@ -3,7 +3,6 @@ package users import ( "Coves/internal/atproto/identity" "Coves/internal/core/blobs" - "Coves/internal/core/communities" "bytes" "context" "encoding/json" @@ -534,9 +533,12 @@ func (s *userService) GetProfile(ctx context.Context, did string) (*ProfileViewD Bio: user.Bio, } - // Transform avatar/banner CIDs to URLs using image proxy config - // Uses 'avatar' preset (160x160) for profile detail view - config := communities.GetImageProxyConfig() + // Transform avatar/banner CIDs to URLs using image proxy config. + // The 'avatar' preset is the full-size rendering used by the profile detail + // view; feeds and comment threads use 'avatar_small'. Dimensions live in + // internal/core/imageproxy's preset registry, not here — they have already + // changed once since this comment was written. + config := blobs.GetImageURLConfig() profile.Avatar = blobs.HydrateImageURL(config, user.PDSURL, user.DID, user.AvatarCID, "avatar") profile.Banner = blobs.HydrateImageURL(config, user.PDSURL, user.DID, user.BannerCID, "banner") diff --git a/internal/db/postgres/community_feed_test.go b/internal/db/postgres/community_feed_test.go index 2f99da0..3dbdefc 100644 --- a/internal/db/postgres/community_feed_test.go +++ b/internal/db/postgres/community_feed_test.go @@ -4,6 +4,7 @@ package postgres_test import ( "Coves/internal/api/handlers/communityFeed" + "Coves/internal/core/blobs" "Coves/internal/core/communities" "Coves/internal/core/communityFeeds" "Coves/internal/core/posts" @@ -595,19 +596,38 @@ func TestGetCommunityFeed_HotCursorTimeDrift(t *testing.T) { // TestGetCommunityFeed_BlobURLTransformation covers the rewrite the AppView // applies on the way out: an embed's thumbnail is stored as the blob ref the -// author's PDS returned, and is served as a getBlob URL a client can fetch -// directly. +// author's PDS returned, and is served — with the image proxy configured, which +// production requires — as a proxy URL on the media hostname rather than a +// direct PDS getBlob URL. // -// Two things are pinned. The URL is built from the COMMUNITY's DID and PDS URL, -// because that is the repository the blob lives in — pointing it at the author -// would 404. And the embed's $type changes to the "#view" variant, because the -// served shape no longer matches the record schema and the postView union in -// social/coves/community/post/defs.json requires the view type on the wire. +// Three things are pinned. The URL is built from the COMMUNITY's DID, because +// that is the repository the blob lives in — pointing it at the author would +// 404. The embed's $type changes to the "#view" variant, because the served +// shape no longer matches the record schema and the postView union in +// social/coves/community/post/defs.json requires the view type on the wire. And +// the verbatim record still carries its blob ref, not the hydrated URL: scanPostView +// decodes the stored embed a second time rather than aliasing the view's map, +// and this is the only test that pins that against the feed path (see +// post_repo.go). +// +// This test is deliberately NOT parallel. It sets the process-wide image-URL +// config that scanPostView reads, so it must run in the serial phase — Go runs +// every non-parallel test to completion before any t.Parallel test resumes, so +// the config is set and restored inside a window no parallel sibling overlaps. +// Do not add t.Parallel here without moving the config off a global. func TestGetCommunityFeed_BlobURLTransformation(t *testing.T) { - t.Parallel() db := testkit.DB(t) handler := newCommunityFeedHandler(db) + // Serve URLs the way production does: through the image proxy on the media + // hostname. The config is process-wide, so it is restored afterwards. + blobs.ResetImageURLConfigForTesting() + blobs.SetImageURLConfig(blobs.ImageURLConfig{ + ProxyEnabled: true, + ProxyBaseURL: "https://img.coves.social", + }) + t.Cleanup(blobs.ResetImageURLConfigForTesting) + ctx := context.Background() testID := testkit.UniqueID(t) communityDID, err := fixtures.Community(ctx, db, @@ -662,7 +682,30 @@ func TestGetCommunityFeed_BlobURLTransformation(t *testing.T) { thumbURL, ok := external["thumb"].(string) require.True(t, ok, "Thumb should be a string URL after transformation") - expectedURL := fmt.Sprintf("%s/xrpc/com.atproto.sync.getBlob?did=%s&cid=%s", - testkit.Endpoints().PDS.BaseURL, communityDID, thumbCID) - assert.Equal(t, expectedURL, thumbURL, "Thumb URL should match expected format") + // With the image proxy configured, the thumb addresses the media hostname, + // never the PDS. A getBlob URL escaping into a feed response is media served + // around the CDN that scans it. + expectedURL := fmt.Sprintf("https://img.coves.social/img/embed_thumbnail/plain/%s/%s", + communityDID, thumbCID) + assert.Equal(t, expectedURL, thumbURL, "Thumb URL should be an image proxy URL") + assert.NotContains(t, thumbURL, "com.atproto.sync.getBlob", + "the AppView must not hand clients a direct PDS blob URL") + + // The lexicon calls `record` the post record verbatim, and hydration mutates + // the embed in place. scanPostView therefore decodes the stored embed a + // second time instead of aliasing the view's map — if it ever goes back to + // sharing one map, the projection above would rewrite the record too and the + // record would claim a record $type while carrying view-shaped fields. + record, ok := feedPost.Post.Record.(map[string]interface{}) + require.True(t, ok, "Record should be a map") + + recordEmbed, ok := record["embed"].(map[string]interface{}) + require.True(t, ok, "the verbatim record must still carry its embed") + assert.Equal(t, "social.coves.embed.external", recordEmbed["$type"], + "the record embed must keep the record $type, not the hydrated #view type") + + recordExternal, ok := recordEmbed["external"].(map[string]interface{}) + require.True(t, ok, "record embed external should be a map") + assert.IsType(t, map[string]interface{}{}, recordExternal["thumb"], + "the record's thumb must still be a blob reference, not the hydrated URL string") } diff --git a/internal/db/postgres/post_repo.go b/internal/db/postgres/post_repo.go index e98a7a2..fd537f8 100644 --- a/internal/db/postgres/post_repo.go +++ b/internal/db/postgres/post_repo.go @@ -12,7 +12,6 @@ import ( "time" "Coves/internal/core/blobs" - "Coves/internal/core/communities" "Coves/internal/core/posts" "github.com/lib/pq" @@ -429,7 +428,7 @@ func scanPostView(rows *sql.Rows, extraDest ...interface{}) (*posts.PostView, er if authorDisplayName.Valid && authorDisplayName.String != "" { authorView.DisplayName = &authorDisplayName.String } - if avatarURL := blobs.HydrateImageURL(communities.GetImageProxyConfig(), authorPDSURL.String, authorView.DID, authorAvatar.String, "avatar_small"); avatarURL != "" { + if avatarURL := blobs.HydrateImageURL(blobs.GetImageURLConfig(), authorPDSURL.String, authorView.DID, authorAvatar.String, "avatar_small"); avatarURL != "" { authorView.Avatar = &avatarURL } postView.Author = &authorView @@ -439,7 +438,7 @@ func scanPostView(rows *sql.Rows, extraDest ...interface{}) (*posts.PostView, er communityRef.Handle = communityHandle.String } // Hydrate avatar CID to URL using image proxy config (avatar_small preset for post views) - if avatarURL := blobs.HydrateImageURL(communities.GetImageProxyConfig(), communityPDSURL.String, communityRef.DID, communityAvatar.String, "avatar_small"); avatarURL != "" { + if avatarURL := blobs.HydrateImageURL(blobs.GetImageURLConfig(), communityPDSURL.String, communityRef.DID, communityAvatar.String, "avatar_small"); avatarURL != "" { communityRef.Avatar = &avatarURL } if communityPDSURL.Valid { @@ -505,8 +504,25 @@ func scanPostView(rows *sql.Rows, extraDest ...interface{}) (*posts.PostView, er if facetArray != nil { record["facets"] = facetArray } - if postView.Embed != nil { - record["embed"] = postView.Embed + // Decode the stored embed a second time rather than aliasing postView.Embed. + // The lexicon calls `record` the post record verbatim, and the handlers + // hydrate postView.Embed in place into its #view shape (blob refs become + // image-proxy URLs) — sharing one map would silently rewrite the record too, + // leaving it claiming a record $type while carrying view-shaped fields. + if embed.Valid { + var recordEmbed interface{} + if err := json.Unmarshal([]byte(embed.String), &recordEmbed); err != nil { + // The same bytes decoded successfully a few lines above, so reaching + // here means something stranger than malformed input. Logged rather + // than dropped silently: the alternative is a post whose record is + // missing its embed with nothing anywhere recording why. + slog.Warn("failed to parse embed JSON for record", + "post_uri", postView.URI, + "error", err, + ) + } else { + record["embed"] = recordEmbed + } } if labelsJSON.Valid { // Labels are stored as JSONB containing full com.atproto.label.defs#selfLabels structure