From 521e52c853fef7f93d6e01c28077a149bcc12154 Mon Sep 17 00:00:00 2001
From: Maurice <61554723+mauriceboe@users.noreply.github.com>
Date: Thu, 27 Aug 2026 12:03:04 +0200
Subject: [PATCH] fix(collab): harden the link preview fetcher (#2058)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The preview endpoint takes a URL from whoever is typing in a trip chat and
fetches it from the server. Trip access was added to it earlier; this is the
rest of what that route needed.
The SSRF guard had a hole under it. `isAlwaysBlocked` knew `::1` and `0.0.0.0`
but not the IPv6 unspecified address: `::` starts with a colon, so the `0.`
check never saw it, and connecting to it lands on loopback. `http://[::]:9200/`
therefore passed the guard and came back with the target's title and og: tags
scraped out of the body — an exfiltrating read of anything listening locally,
from any account. `[::0]`, `[0:0:0:0:0:0:0:0]` and `[::ffff:0:0]` are the same
address in other spellings and behaved the same way. The two `::ffff:`-prefix
regexes had the same shape of gap: they matched the dotted spelling of loopback
and link-local, so `::ffff:7f00:1` walked past them. Both checks now decide on
the expanded hextets, which covers every spelling at once, and fe80::/10 is
read as ten bits rather than as the four characters 'fe80'. This is in
ssrfGuard, so every caller gets it, not just collab.
The scrape could be turned into a denial of service. Each og: regex had two
unbounded `[^>]*` runs separated by a literal, so a document with no `>` in it
made the engine rescan the remainder from every `.
Deliberately no `@RequirePermission` on the route: the client asks for a
preview while rendering a message or a note, never while composing one, so it
belongs with the GET siblings. Requiring collab_edit would blank every preview
for members of a trip whose owner narrowed that right while leaving them able
to read the chat, and it would not stop the abuse it looks like it addresses —
the permission check passes for a trip's owner at every level, and anyone can
create a trip.
---
server/src/nest/collab/collab.controller.ts | 17 +-
server/src/nest/collab/collab.module.ts | 3 +-
server/src/nest/collab/collab.service.ts | 200 ++++++++++++++----
server/src/utils/ssrfGuard.ts | 44 +++-
server/tests/e2e/collab.e2e.test.ts | 50 +++++
server/tests/helpers/mcp-test-controllers.ts | 3 +-
server/tests/helpers/plugin-host.ts | 3 +-
server/tests/integration/collab.test.ts | 28 +++
.../tests/unit/nest/collab.controller.test.ts | 39 ++++
server/tests/unit/nest/collab.service.test.ts | 176 ++++++++++++++-
.../unit/nest/trip-read-model.service.test.ts | 3 +-
server/tests/unit/nest/trips.service.test.ts | 3 +-
server/tests/unit/utils/ssrfGuard.test.ts | 70 ++++++
wiki/Collab-Chat.md | 2 +
14 files changed, 580 insertions(+), 61 deletions(-)
diff --git a/server/src/nest/collab/collab.controller.ts b/server/src/nest/collab/collab.controller.ts
index 4f35faf4b..6e2d7ea74 100644
--- a/server/src/nest/collab/collab.controller.ts
+++ b/server/src/nest/collab/collab.controller.ts
@@ -299,6 +299,13 @@ export class CollabController {
}
// ── Link preview ──────────────────────────────────────────────────────────
+ // Deliberately no @RequirePermission: this is a read path. The client asks for
+ // a preview while *rendering* a message or a note (CollabChatMessages,
+ // CollabNotesCard), never while composing one, so it belongs with the GET
+ // siblings above — requiring 'collab_edit' would strip every preview from a
+ // trip whose owner has narrowed that right, for people who may still read the
+ // chat. What keeps the fetcher from being a free outbound proxy is the budget
+ // inside the service, not a write permission.
@UseGuards(TripAccessGuard)
@Get('link-preview')
async linkPreview(@CurrentUser() user: User, @Param('tripId') tripId: string, @Query('url') url?: string) {
@@ -308,10 +315,12 @@ export class CollabController {
throw new HttpException({ error: 'URL is required' }, 400);
}
try {
- const preview = await this.collab.linkPreview(url);
- const asRecord = preview as { error?: string };
- if (asRecord.error) {
- throw new HttpException({ error: asRecord.error }, 400);
+ const preview = await this.collab.linkPreview(url, user.id);
+ if (preview.rateLimited) {
+ throw new HttpException({ error: 'Too many requests' }, 429);
+ }
+ if (preview.error) {
+ throw new HttpException({ error: preview.error }, 400);
}
return preview;
} catch (err) {
diff --git a/server/src/nest/collab/collab.module.ts b/server/src/nest/collab/collab.module.ts
index dcb148297..e876e5262 100644
--- a/server/src/nest/collab/collab.module.ts
+++ b/server/src/nest/collab/collab.module.ts
@@ -14,6 +14,7 @@ import { McpSharedModule } from '../mcp-shared/mcp-shared.module';
import { StorageModule } from '../storage/storage.module';
import { StorageService } from '../storage/storage.service';
import { buildStorageUploadOptions } from '../storage/storage-upload.factory';
+import { RateLimitModule } from '../common/rate-limit.module';
@Module({
imports: [
@@ -29,7 +30,7 @@ import { buildStorageUploadOptions } from '../storage/storage-upload.factory';
}),
}),
StorageModule,
- McpSharedModule, NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule],
+ McpSharedModule, NotificationsModule, PermissionsModule, AuthModule, RealtimeModule, PluginGuardsModule, AddonsModule, RateLimitModule],
controllers: [CollabController],
providers: [CollabService, CollabMcp, CollabRpc],
// For in-container consumers (CollabRpc).
diff --git a/server/src/nest/collab/collab.service.ts b/server/src/nest/collab/collab.service.ts
index 3208a7aff..1812a4f07 100644
--- a/server/src/nest/collab/collab.service.ts
+++ b/server/src/nest/collab/collab.service.ts
@@ -6,10 +6,11 @@ import { RealtimeService } from '../realtime/realtime.service';
import { PermissionsService } from '../permissions/permissions.service';
import { avatarUrl } from '../common/avatarUrl';
import { checkSsrf, createPinnedDispatcher } from '../../utils/ssrfGuard';
-import { exceedsDeclaredLength, readCappedText } from '../../utils/cappedFetch';
+import { discardBody, exceedsDeclaredLength, readCappedText } from '../../utils/cappedFetch';
import type { CollabNote, CollabPoll, CollabMessage, TripFile, User } from '../../types';
import { NotificationsService } from '../notifications/notifications.service';
import { StorageService } from '../storage/storage.service';
+import { RateLimitService } from '../common/rate-limit.service';
type Trip = TripAccess;
@@ -51,6 +52,56 @@ export interface LinkPreviewResult {
image: string | null;
site_name?: string | null;
url: string;
+ /** Set when the URL was refused; the controller turns it into a 400. */
+ error?: string;
+ /** Set when the caller is out of preview fetches; the controller turns it into a 429. */
+ rateLimited?: boolean;
+}
+
+/**
+ * Outbound fetches one user may trigger per minute. Deliberately generous: the
+ * client asks for a preview per rendered message (`LIMIT 100`) and per note with
+ * a website, both without debounce, so opening a link-heavy trip is a burst of
+ * dozens. Cache hits are not charged, so this only ever meters *new* URLs.
+ */
+const PREVIEW_FETCHES_PER_MINUTE = 60;
+
+/** How long a scraped preview stays good. og: tags are near-static. */
+const PREVIEW_CACHE_TTL_MS = 10 * 60 * 1000;
+
+/** Entries kept before the least recently used one is dropped. */
+const PREVIEW_CACHE_MAX = 500;
+
+/**
+ * Pulls the og:/
/description fields out of a document.
+ *
+ * Every `[^>]` run is bounded. Unbounded, two of them separated by a literal make
+ * the engine rescan the rest of the document from every `` in it took ~58s at 240KB on the
+ * measured build, and the cap admits half a megabyte. Node runs one thread, so
+ * that is the whole server — WebSocket, auth and health included — for one
+ * request. No real attribute list comes close to 512 characters.
+ */
+function scrapeOpenGraph(html: string): Omit {
+ const og = (prop: string) => {
+ const m = html.match(new RegExp(`]{0,512}property=["']og:${prop}["'][^>]{0,512}content=["']([^"']*)["']`, 'i'))
+ || html.match(new RegExp(`]{0,512}content=["']([^"']*)["'][^>]{0,512}property=["']og:${prop}["']`, 'i'));
+ return m ? m[1] : null;
+ };
+ const titleTag = html.match(/]{0,512}>([^<]*)<\/title>/i);
+ const descMeta = html.match(/]{0,512}name=["']description["'][^>]{0,512}content=["']([^"']*)["']/i)
+ || html.match(/]{0,512}content=["']([^"']*)["'][^>]{0,512}name=["']description["']/i);
+ const image = og('image');
+
+ return {
+ title: og('title') || (titleTag ? titleTag[1].trim() : null),
+ description: og('description') || (descMeta ? descMeta[1].trim() : null),
+ // The client renders this straight into an
, so the page being
+ // previewed must not be able to point that at anything but a web address —
+ // the same scheme pin placeImageUrlSchema applies to a stored place picture.
+ image: image && /^https?:\/\//i.test(image) ? image : null,
+ site_name: og('site_name') || null,
+ };
}
/**
@@ -75,8 +126,23 @@ export class CollabService {
private readonly realtime: RealtimeService,
private readonly notifications: NotificationsService,
private readonly storage: StorageService,
+ private readonly rateLimit: RateLimitService,
) {}
+ /**
+ * Scraped previews, keyed by URL and ordered least-recently-used first.
+ *
+ * On the service rather than at module scope so a test gets a fresh one per
+ * container. What it buys: the client re-requests every preview it renders on
+ * each mount, so without it a reload of a busy trip is another hundred outbound
+ * fetches — the exact traffic the budget above is meant to stop the server from
+ * emitting on someone else's behalf.
+ */
+ private readonly previewCache = new Map();
+
+ /** Preview fetches currently in flight, so simultaneous askers share one request. */
+ private readonly inFlight = new Map>();
+
verifyTripAccess(tripId: string | number, userId: number) {
return this.db.canAccessTrip(tripId, userId);
}
@@ -461,60 +527,114 @@ export class CollabService {
/* Link preview */
/* ------------------------------------------------------------------ */
- async linkPreview(url: string): Promise {
+ async linkPreview(url: string, userId?: number): Promise {
const fallback: LinkPreviewResult = { title: null, description: null, image: null, url };
// A malformed URL returns the fallback directly (the legacy code let
// `new URL` throw and relied on the controller's catch for the same 200).
try { new URL(url); } catch { return fallback; }
+
+ // Served before the budget is charged: opening a chat re-requests every
+ // preview it renders, so a reload must not cost the caller its allowance.
+ const cached = this.readPreviewCache(url);
+ if (cached) return cached;
+
+ // A fetch for this URL is already on its way. The client renders one preview
+ // per message and does not deduplicate, so the same link posted twenty times
+ // arrives as twenty simultaneous requests — none of which would find a cache
+ // entry yet, since the first has not answered. Joining the running fetch keeps
+ // that a single outbound request instead of twenty.
+ const running = this.inFlight.get(url);
+ if (running) return { ...(await running), url };
+
+ // Charged per outbound fetch rather than per request, which is what the
+ // budget is actually protecting. Without a user there is no one to charge —
+ // no caller passes that today, and the fetch stays behind the SSRF guard.
+ if (userId !== undefined && !this.rateLimit.check('collab_link_preview', String(userId), PREVIEW_FETCHES_PER_MINUTE, 60_000, Date.now())) {
+ return { ...fallback, rateLimited: true };
+ }
+
+ const task = this.fetchPreview(url, fallback);
+ this.inFlight.set(url, task);
+ try {
+ return await task;
+ } finally {
+ this.inFlight.delete(url);
+ }
+ }
+
+ /** The outbound half of linkPreview, past the cache and the budget. */
+ private async fetchPreview(url: string, fallback: LinkPreviewResult): Promise {
const ssrf = await checkSsrf(url, true);
if (!ssrf.allowed) {
- return { ...fallback, error: ssrf.error } as LinkPreviewResult & { error?: string };
+ // The caller learns that the URL was refused, never why: the three distinct
+ // reasons ("could not resolve", "private address", "loopback") would together
+ // map out the server's internal DNS for anyone willing to guess hostnames.
+ return { ...fallback, error: 'URL not allowed' };
}
+ const dispatcher = createPinnedDispatcher(ssrf.resolvedIp!);
try {
- const controller = new AbortController();
- const timeout = setTimeout(() => controller.abort(), 5000);
-
- try {
- const r = await fetch(url, {
- redirect: 'error',
- signal: controller.signal,
- dispatcher: createPinnedDispatcher(ssrf.resolvedIp!),
- headers: { 'User-Agent': 'Mozilla/5.0 (compatible; NOMAD/1.0; +https://github.com/mauriceboe/NOMAD)' },
- } as any);
- clearTimeout(timeout);
- if (!r.ok) throw new Error('Fetch failed');
- if (exceedsDeclaredLength(r, MAX_PREVIEW_BYTES)) return fallback;
-
- // A truncated head still carries the tags we scrape, so a page over the
- // budget degrades to fewer fields rather than to an error.
- const { text: html } = await readCappedText(r, MAX_PREVIEW_BYTES);
- const get = (prop: string) => {
- const m = html.match(new RegExp(`]*property=["']og:${prop}["'][^>]*content=["']([^"']*)["']`, 'i'))
- || html.match(new RegExp(`]*content=["']([^"']*)["'][^>]*property=["']og:${prop}["']`, 'i'));
- return m ? m[1] : null;
- };
- const titleTag = html.match(/]*>([^<]*)<\/title>/i);
- const descMeta = html.match(/]*name=["']description["'][^>]*content=["']([^"']*)["']/i)
- || html.match(/]*content=["']([^"']*)["'][^>]*name=["']description["']/i);
-
- return {
- title: get('title') || (titleTag ? titleTag[1].trim() : null),
- description: get('description') || (descMeta ? descMeta[1].trim() : null),
- image: get('image') || null,
- site_name: get('site_name') || null,
- url,
- };
- } catch {
- clearTimeout(timeout);
- return fallback;
+ // AbortSignal.timeout covers the body as well. The hand-rolled controller
+ // this replaces was cleared as soon as the headers arrived, so a server that
+ // answered fast and then dripped the body one byte at a time held the handler,
+ // the socket and this dispatcher open indefinitely — the byte cap counts
+ // bytes, and at that rate it would never reach one.
+ const r = await fetch(url, {
+ redirect: 'error',
+ signal: AbortSignal.timeout(5000),
+ dispatcher,
+ headers: { 'User-Agent': 'Mozilla/5.0 (compatible; NOMAD/1.0; +https://github.com/mauriceboe/NOMAD)' },
+ } as any);
+ if (!r.ok) { discardBody(r); return this.cachePreview(url, fallback); }
+ // Only markup is worth scraping. A declared type that is not HTML means the
+ // regexes below would comb a video or an archive for og: tags and find nothing.
+ const type = r.headers?.get('content-type') ?? '';
+ if (type && !/^\s*(text\/html|application\/xhtml\+xml|text\/plain)\b/i.test(type)) {
+ discardBody(r);
+ return this.cachePreview(url, fallback);
}
+ // An unread body keeps its socket reserved until the garbage collector runs,
+ // which is the one thing a size cap is there to prevent.
+ if (exceedsDeclaredLength(r, MAX_PREVIEW_BYTES)) { discardBody(r); return this.cachePreview(url, fallback); }
+
+ // A truncated head still carries the tags we scrape, so a page over the
+ // budget degrades to fewer fields rather than to an error.
+ const { text: html } = await readCappedText(r, MAX_PREVIEW_BYTES);
+ return this.cachePreview(url, { ...scrapeOpenGraph(html), url });
} catch {
- return fallback;
+ return this.cachePreview(url, fallback);
+ } finally {
+ // Closed rather than left to the garbage collector: one Agent is built per
+ // preview, and each keeps its sockets until something releases them.
+ void (dispatcher as { close?: () => Promise } | undefined)?.close?.()?.catch(() => {});
}
}
+ /** A cached preview, or undefined once its entry has expired or was never there. */
+ private readPreviewCache(url: string): LinkPreviewResult | undefined {
+ const hit = this.previewCache.get(url);
+ if (!hit) return undefined;
+ if (Date.now() - hit.at > PREVIEW_CACHE_TTL_MS) {
+ this.previewCache.delete(url);
+ return undefined;
+ }
+ // Re-insert so the eviction below drops the least recently used entry.
+ this.previewCache.delete(url);
+ this.previewCache.set(url, hit);
+ return { ...hit.result, url };
+ }
+
+ /** Stores a preview and returns it, so call sites can `return this.cachePreview(...)`. */
+ private cachePreview(url: string, result: LinkPreviewResult): LinkPreviewResult {
+ if (this.previewCache.size >= PREVIEW_CACHE_MAX) {
+ const oldest = this.previewCache.keys().next().value;
+ if (oldest !== undefined) this.previewCache.delete(oldest);
+ }
+ this.previewCache.set(url, { at: Date.now(), result });
+ return result;
+ }
+
/** Fire-and-forget collab notification (mirrors the legacy route's dynamic import). */
notifyCollab(tripId: string, actor: User, preview?: string): void {
// Injected, not a lazy import of the old notifications bridge. The laziness bought
diff --git a/server/src/utils/ssrfGuard.ts b/server/src/utils/ssrfGuard.ts
index b52e00adc..10393cfaa 100644
--- a/server/src/utils/ssrfGuard.ts
+++ b/server/src/utils/ssrfGuard.ts
@@ -1,7 +1,7 @@
import dns from 'node:dns/promises';
import { Agent } from 'undici';
import { readEnv } from '../app-config';
-import { embeddedTransitionIpv4 } from './ipv6';
+import { embeddedTransitionIpv4, expandIpv6 } from './ipv6';
// Frozen at import on purpose (legacy timing; tests reload the module to change it).
const ALLOW_INTERNAL_NETWORK = readEnv().net.allowInternalNetwork;
@@ -13,6 +13,20 @@ export interface SsrfResult {
error?: string;
}
+/**
+ * The IPv4 an IPv4-mapped address (`::ffff:a.b.c.d`) stands for, or null.
+ *
+ * Taken from the hextets rather than from the text, because `::ffff:127.0.0.1`
+ * and `::ffff:7f00:1` are one address in two spellings and `dns.lookup` picks
+ * which one comes back. The prefix regexes this replaces covered the dotted
+ * spelling of two ranges; every other mapped address walked past them.
+ */
+function mappedIpv4(hextets: number[]): string | null {
+ const g = hextets;
+ if (g[0] || g[1] || g[2] || g[3] || g[4] || g[5] !== 0xffff) return null;
+ return `${g[6] >> 8}.${g[6] & 0xff}.${g[7] >> 8}.${g[7] & 0xff}`;
+}
+
// Always blocked — no override possible
function isAlwaysBlocked(ip: string): boolean {
// Strip IPv6 brackets
@@ -23,9 +37,21 @@ function isAlwaysBlocked(ip: string): boolean {
// Unspecified
if (addr.startsWith('0.')) return true;
// Link-local / cloud metadata
- if (addr.startsWith('169.254.') || /^fe80:/i.test(addr)) return true;
- // IPv4-mapped loopback / link-local: ::ffff:127.x.x.x, ::ffff:169.254.x.x
- if (/^::ffff:127\./i.test(addr) || /^::ffff:169\.254\./i.test(addr)) return true;
+ if (addr.startsWith('169.254.')) return true;
+
+ const hextets = expandIpv6(addr);
+ if (hextets) {
+ // The IPv6 unspecified address. Connecting to it lands on loopback, so it
+ // reaches a local service without naming one, and it has enough spellings
+ // (`::`, `::0`, `0:0:0:0:0:0:0:0`) that only the expanded hextets settle it.
+ // The `0.` check above never saw any of them: they begin with a colon.
+ if (hextets.every(h => h === 0)) return true;
+ // fe80::/10 spans fe80: through febf:, not just the four characters 'fe80'.
+ if ((hextets[0] & 0xffc0) === 0xfe80) return true;
+ // A mapped address inherits the verdict of the IPv4 it carries.
+ const mapped = mappedIpv4(hextets);
+ if (mapped) return isAlwaysBlocked(mapped);
+ }
// IPv6 transition addresses (NAT64/6to4/Teredo) embedding a hard-blocked IPv4.
const embedded = embeddedTransitionIpv4(addr);
if (embedded) return isAlwaysBlocked(embedded);
@@ -45,10 +71,12 @@ function isPrivateNetwork(ip: string): boolean {
if (/^100\.(6[4-9]|[7-9]\d|1[01]\d|12[0-7])\./.test(addr)) return true;
// IPv6 ULA (fc00::/7)
if (/^f[cd]/i.test(addr)) return true;
- // IPv4-mapped RFC-1918
- if (/^::ffff:10\./i.test(addr)) return true;
- if (/^::ffff:172\.(1[6-9]|2\d|3[01])\./i.test(addr)) return true;
- if (/^::ffff:192\.168\./i.test(addr)) return true;
+ // A mapped address inherits the verdict of the IPv4 it carries. Deciding on the
+ // hextets rather than on three `::ffff:`-prefix regexes also covers the hex
+ // spelling and the CGNAT range those regexes never listed.
+ const hextets = expandIpv6(addr);
+ const mapped = hextets && mappedIpv4(hextets);
+ if (mapped) return isPrivateNetwork(mapped);
// IPv6 transition addresses (NAT64/6to4/Teredo) embedding a private IPv4.
const embedded = embeddedTransitionIpv4(addr);
if (embedded) return isPrivateNetwork(embedded);
diff --git a/server/tests/e2e/collab.e2e.test.ts b/server/tests/e2e/collab.e2e.test.ts
index 067cb5130..5e592253d 100644
--- a/server/tests/e2e/collab.e2e.test.ts
+++ b/server/tests/e2e/collab.e2e.test.ts
@@ -70,6 +70,7 @@ let checkPermission: MockInstance;
import { CollabModule } from '../../src/nest/collab/collab.module';
import { TrekExceptionFilter } from '../../src/nest/common/trek-exception.filter';
+import { RateLimitService } from '../../src/nest/common/rate-limit.service';
describe('Collab e2e (real auth guard + temp SQLite)', () => {
let server: Server;
@@ -168,6 +169,55 @@ describe('Collab e2e (real auth guard + temp SQLite)', () => {
expect(db.prepare('SELECT COUNT(*) as c FROM collab_message_reactions WHERE message_id = 3').get()).toEqual({ c: 1 });
});
+ // The advisory this route was reported under: it answered anyone with a session,
+ // for any trip id, and drove an outbound fetch from that.
+ it('404 on link-preview for a trip the caller cannot reach', async () => {
+ canAccessTrip.mockReturnValue(undefined);
+ const res = await request(server)
+ .get('/api/trips/5/collab/link-preview?url=https://example.com/')
+ .set('Cookie', sessionCookie(1));
+ expect(res.status).toBe(404);
+ expect(res.body).toEqual({ error: 'Trip not found' });
+ });
+
+ it('401 on link-preview without a session cookie', async () => {
+ const res = await request(server).get('/api/trips/5/collab/link-preview?url=https://example.com/');
+ expect(res.status).toBe(401);
+ });
+
+ // A read-only member still gets previews: the route is requested while rendering
+ // the chat, so gating it on the write permission would blank the chat for them.
+ it('200 on link-preview for a member without collab_edit', async () => {
+ checkPermission.mockReturnValue(false);
+ vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
+ ok: true, headers: { get: () => null }, text: async () => 'Lesbar',
+ }));
+ const res = await request(server)
+ .get('/api/trips/5/collab/link-preview?url=https://example.com/reader')
+ .set('Cookie', sessionCookie(1));
+ vi.unstubAllGlobals();
+ expect(res.status).toBe(200);
+ expect(res.body.title).toBe('Lesbar');
+ });
+
+ it('429 once the caller has spent a minute of preview fetches', async () => {
+ vi.stubGlobal('fetch', vi.fn().mockResolvedValue({
+ ok: true, headers: { get: () => null }, text: async () => 'T',
+ }));
+ // Distinct URLs, because a repeat is served from the cache and costs nothing.
+ let last = 200;
+ for (let i = 0; i < 61 && last === 200; i++) {
+ last = (await request(server)
+ .get(`/api/trips/5/collab/link-preview?url=${encodeURIComponent(`https://example.com/e2e-${i}`)}`)
+ .set('Cookie', sessionCookie(1))).status;
+ }
+ vi.unstubAllGlobals();
+ expect(last).toBe(429);
+ // The counters live on the container singleton, so a spent budget would
+ // follow this user into every test declared after it.
+ app.get(RateLimitService).reset('collab_link_preview');
+ });
+
it('400 on link-preview without a url', async () => {
const res = await request(server).get('/api/trips/5/collab/link-preview').set('Cookie', sessionCookie(1));
expect(res.status).toBe(400);
diff --git a/server/tests/helpers/mcp-test-controllers.ts b/server/tests/helpers/mcp-test-controllers.ts
index 1d27a5f4a..2d61e04f3 100644
--- a/server/tests/helpers/mcp-test-controllers.ts
+++ b/server/tests/helpers/mcp-test-controllers.ts
@@ -13,6 +13,7 @@ import { CategoriesMcp } from '../../src/nest/categories/categories.mcp';
import { CategoriesService } from '../../src/nest/categories/categories.service';
import { CollabMcp } from '../../src/nest/collab/collab.mcp';
import { CollabService } from '../../src/nest/collab/collab.service';
+import { RateLimitService } from '../../src/nest/common/rate-limit.service';
import { CollectionsMcp } from '../../src/nest/collections/collections.mcp';
import { CollectionsService } from '../../src/nest/collections/collections.service';
import { DatabaseService } from '../../src/nest/database/database.service';
@@ -111,7 +112,7 @@ export function createMcpTestRegistry(): McpRegistry {
const daysService = new DaysService(dbService, permissionsService, realtimeService, queryHelpersService);
const todoService = new TodoService(dbService, permissionsService, realtimeService);
const packingService = new PackingService(dbService, permissionsService, realtimeService, notificationsStub());
- const collabService = new CollabService(dbService, permissionsService, realtimeService, notificationsStub(), generalStorage);
+ const collabService = new CollabService(dbService, permissionsService, realtimeService, notificationsStub(), generalStorage, new RateLimitService());
// Exactly one instance, shared by maps, places and share: its stampede guard
// and its on-disk set only work if all three readers see the same maps.
const placePhotoCache = new PlacePhotoCacheService(dbService, makeStorageFixture('photos/google/').storage);
diff --git a/server/tests/helpers/plugin-host.ts b/server/tests/helpers/plugin-host.ts
index 600a1da2e..0d046501c 100644
--- a/server/tests/helpers/plugin-host.ts
+++ b/server/tests/helpers/plugin-host.ts
@@ -26,6 +26,7 @@ import { LlmConfigResolver } from '../../src/nest/llm-parse/llm-config.resolver'
import { SettingsService } from '../../src/nest/settings/settings.service';
import { FilesService } from '../../src/nest/files/files.service';
import { CollabService } from '../../src/nest/collab/collab.service';
+import { RateLimitService } from '../../src/nest/common/rate-limit.service';
import { VacayService } from '../../src/nest/vacay/vacay.service';
import { TripsService } from '../../src/nest/trips/trips.service';
import { PlacesService } from '../../src/nest/places/places.service';
@@ -94,7 +95,7 @@ export function createPluginRpcHostFactory(dbs: DatabaseService): PluginRpcHostF
const packing = new PackingService(dbs, permissions, realtime, notificationsStub());
const files = new FilesService(dbs, permissions, realtime, new EphemeralTokenService(), generalStorage);
const reservations = new ReservationsService(dbs, permissions, budget, realtime, notificationsStub(), new ReservationsReadRepository(dbs));
- const collab = new CollabService(dbs, permissions, realtime, notificationsStub(), generalStorage);
+ const collab = new CollabService(dbs, permissions, realtime, notificationsStub(), generalStorage, new RateLimitService());
const vacay = new VacayService(dbs, realtime, notificationsStub());
const days = new DaysService(dbs, permissions, realtime, queryHelpers);
const photoCache = new PlacePhotoCacheService(dbs, makeStorageFixture('photos/google/').storage);
diff --git a/server/tests/integration/collab.test.ts b/server/tests/integration/collab.test.ts
index 44a72066f..83b72eef2 100644
--- a/server/tests/integration/collab.test.ts
+++ b/server/tests/integration/collab.test.ts
@@ -58,6 +58,9 @@ import { CollabService } from '../../src/nest/collab/collab.service';
// Spy on the DI-native service's linkPreview so the SSRF-guarded fetch never
// runs; the rest of CollabService exercises its real SQL through the container.
+// The original is kept so the guard cases below can put it back for one call —
+// mockRestore would drop the spy for every test declared after them.
+const realLinkPreview = CollabService.prototype.linkPreview;
const linkPreviewSpy = vi.spyOn(CollabService.prototype, 'linkPreview')
.mockResolvedValue({ title: null, description: null, image: null, url: '' });
@@ -808,6 +811,31 @@ describe('Link preview', () => {
expect(res.body.error).toBeDefined();
});
+ // Every case above mocks linkPreview out, so the SSRF guard, the redirect
+ // refusal and the pinned dispatcher are unproven over HTTP: swapping
+ // checkSsrf(url, true) for checkSsrf(url) would leave this file green. These
+ // two put the real implementation back for one call each. No traffic leaves
+ // the machine — the guard refuses both targets before any fetch.
+ it.each([
+ ['loopback', 'http://127.0.0.1:9/'],
+ // The unspecified address routes to loopback when connected to, and used to
+ // pass the guard: it is not '::1', and it does not start with '0.'.
+ ['the unspecified address', 'http://[::]:9/'],
+ ])('COLLAB-032 — GET /collab/link-preview refuses %s (%s) through the real guard', async (_label, url) => {
+ const { user } = createUser(testDb);
+ const trip = createTrip(testDb, user.id);
+ linkPreviewSpy.mockImplementationOnce(realLinkPreview);
+
+ const res = await request(app)
+ .get(`/api/trips/${trip.id}/collab/link-preview?url=${encodeURIComponent(url)}`)
+ .set('Cookie', authCookie(user.id));
+
+ expect(res.status).toBe(400);
+ // One constant reason. The guard knows three, and telling them apart would
+ // map out the internal DNS of the server one guessed hostname at a time.
+ expect(res.body).toEqual({ error: 'URL not allowed' });
+ });
+
it('COLLAB-027 — GET /collab/link-preview catches thrown errors and returns fallback', async () => {
const { user } = createUser(testDb);
const trip = createTrip(testDb, user.id);
diff --git a/server/tests/unit/nest/collab.controller.test.ts b/server/tests/unit/nest/collab.controller.test.ts
index 08da31f29..89f7693bc 100644
--- a/server/tests/unit/nest/collab.controller.test.ts
+++ b/server/tests/unit/nest/collab.controller.test.ts
@@ -5,6 +5,8 @@ import os from 'os';
import path from 'path';
import { CollabController } from '../../../src/nest/collab/collab.controller';
+import { TripAccessGuard, TRIP_PERMISSION_KEY } from '../../../src/nest/permissions/trip-access.guard';
+import { JwtAuthGuard } from '../../../src/nest/auth/jwt-auth.guard';
import type { CollabService } from '../../../src/nest/collab/collab.service';
import type { StorageService } from '../../../src/nest/storage/storage.service';
import type { User } from '../../../src/types';
@@ -181,6 +183,32 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou
});
});
+ // The decorators, not the handler body. Constructing the controller directly,
+ // which every test above does, runs no guard at all — so removing the trip check
+ // again would leave this file green. It shipped without one once.
+ describe('link preview guard chain', () => {
+ const guardsOn = (target: object): unknown[] => (Reflect.getMetadata('__guards__', target) as unknown[]) ?? [];
+
+ it('resolves the trip and refuses a caller who cannot reach it', () => {
+ expect(guardsOn(CollabController.prototype.linkPreview)).toContain(TripAccessGuard);
+ });
+
+ it('sits behind the same authentication as the rest of the controller', () => {
+ expect(guardsOn(CollabController)).toContain(JwtAuthGuard);
+ });
+
+ it('demands no write permission, matching the other read routes', () => {
+ // It is requested while rendering a message or a note, never while writing
+ // one. Requiring collab_edit would strip previews from a trip whose owner
+ // narrowed that right, for people who may still read the chat.
+ for (const handler of [CollabController.prototype.linkPreview, CollabController.prototype.listMessages, CollabController.prototype.listNotes]) {
+ expect(Reflect.getMetadata(TRIP_PERMISSION_KEY, handler)).toBeUndefined();
+ }
+ // The write siblings do demand it, so this is a deliberate split, not an omission.
+ expect(Reflect.getMetadata(TRIP_PERMISSION_KEY, CollabController.prototype.createMessage)).toBe('collab_edit');
+ });
+ });
+
describe('link preview', () => {
it('400 without url, maps an error result to 400, else returns the preview', async () => {
expect(await thrownAsync(() => new CollabController(svc(), storageStub).linkPreview(user, '5', undefined))).toEqual({ status: 400, body: { error: 'URL is required' } });
@@ -189,6 +217,17 @@ describe('CollabController (parity with the legacy /api/trips/:tripId/collab rou
expect(await new CollabController(s, storageStub).linkPreview(user, '5', 'http://x')).toEqual({ title: 'T', description: null, image: null, url: 'http://x' });
});
+ it('maps an exhausted preview budget to 429, not to the 400 a refused URL gets', async () => {
+ const s = svc({ linkPreview: vi.fn().mockResolvedValue({ title: null, description: null, image: null, url: 'http://x', rateLimited: true }) } as Partial);
+ expect(await thrownAsync(() => new CollabController(s, storageStub).linkPreview(user, '5', 'http://x'))).toEqual({ status: 429, body: { error: 'Too many requests' } });
+ });
+
+ it('passes the caller through, so the budget is charged per user and not per instance', async () => {
+ const linkPreview = vi.fn().mockResolvedValue({ title: 'T', description: null, image: null, url: 'http://x' });
+ await new CollabController(svc({ linkPreview } as Partial), storageStub).linkPreview(user, '5', 'http://x');
+ expect(linkPreview).toHaveBeenCalledWith('http://x', user.id);
+ });
+
it('falls back to a null preview when the service throws', async () => {
const s = svc({ linkPreview: vi.fn().mockRejectedValue(new Error('network')) } as Partial);
expect(await new CollabController(s, storageStub).linkPreview(user, '5', 'http://x')).toEqual({ title: null, description: null, image: null, url: 'http://x' });
diff --git a/server/tests/unit/nest/collab.service.test.ts b/server/tests/unit/nest/collab.service.test.ts
index 03187b0b9..e36a51a6c 100644
--- a/server/tests/unit/nest/collab.service.test.ts
+++ b/server/tests/unit/nest/collab.service.test.ts
@@ -68,9 +68,14 @@ import { CollabService } from '../../../src/nest/collab/collab.service';
import { RealtimeService } from '../../../src/nest/realtime/realtime.service';
import { notificationsStub } from '../../helpers/notifications';
import { makeStorageFixture } from '../../helpers/storage-fixture';
+import { RateLimitService } from '../../../src/nest/common/rate-limit.service';
const collabFx = makeStorageFixture('files/');
-const svc = new CollabService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), notificationsStub(), collabFx.storage);
+const rateLimit = new RateLimitService();
+const svc = new CollabService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), notificationsStub(), collabFx.storage, rateLimit);
+
+/** A CollabService with its own preview cache and budget, for the tests that fill either. */
+const freshSvc = () => new CollabService(new DatabaseService(testDb), new PermissionsService(new DatabaseService(testDb)), new RealtimeService(), notificationsStub(), collabFx.storage, new RateLimitService());
beforeAll(() => {
createTables(testDb);
@@ -488,6 +493,169 @@ describe('linkPreview', () => {
});
});
+// ── linkPreview hardening ───────────────────────────────────────────────
+
+describe('linkPreview hardening', () => {
+ afterEach(() => {
+ vi.unstubAllGlobals();
+ });
+
+ /** A fetch stub that records what it was called with. */
+ const stubFetch = (response: Record) => {
+ const fetchMock = vi.fn().mockResolvedValue({ headers: { get: () => null }, ...response });
+ vi.stubGlobal('fetch', fetchMock);
+ return fetchMock;
+ };
+
+ it('COLLAB-SVC-039: asks the SSRF guard to refuse internal targets even where the instance allows them', async () => {
+ stubFetch({ ok: true, text: async () => '' });
+ await freshSvc().linkPreview('https://example.com/guard-args');
+ // The second argument is what keeps ALLOW_INTERNAL_NETWORK from widening a
+ // route whose URL comes from whoever is typing in the chat.
+ expect(mockCheckSsrf).toHaveBeenCalledWith('https://example.com/guard-args', true);
+ });
+
+ it('COLLAB-SVC-040: pins the connection to the checked IP and refuses to follow redirects', async () => {
+ const dispatcher = { close: vi.fn().mockResolvedValue(undefined) };
+ mockCreatePinnedDispatcher.mockReturnValueOnce(dispatcher);
+ const fetchMock = stubFetch({ ok: true, text: async () => '' });
+
+ await freshSvc().linkPreview('https://example.com/init');
+
+ const init = fetchMock.mock.calls[0][1] as Record;
+ // Without redirect:'error' a public URL could 302 onto an internal one, and
+ // the pin does not cover that hop: Node skips the pinned lookup for a literal IP.
+ expect(init.redirect).toBe('error');
+ expect(init.signal).toBeInstanceOf(AbortSignal);
+ expect(mockCreatePinnedDispatcher).toHaveBeenCalledWith('93.184.216.34');
+ expect(init.dispatcher).toBe(dispatcher);
+ // One Agent is built per preview; leaving it open leaks its sockets.
+ expect(dispatcher.close).toHaveBeenCalled();
+ });
+
+ it('COLLAB-SVC-041: a refused URL comes back with one constant reason, never the one the guard gave', async () => {
+ // The guard distinguishes "could not resolve", "private address" and
+ // "loopback". Relaying that verbatim turns the route into a probe for the
+ // internal DNS of the server, one guessed hostname at a time.
+ mockCheckSsrf.mockResolvedValue({ allowed: false, isPrivate: true, resolvedIp: '10.0.0.5', error: 'Requests to private/internal network addresses are not allowed. Set ALLOW_INTERNAL_NETWORK=true to permit this for self-hosted setups.' });
+ const result = await freshSvc().linkPreview('http://nas.internal/');
+ expect(result.error).toBe('URL not allowed');
+ expect(JSON.stringify(result)).not.toContain('ALLOW_INTERNAL_NETWORK');
+ expect(JSON.stringify(result)).not.toContain('private');
+ });
+
+ it('COLLAB-SVC-042: a document built to make the scrape backtrack still returns promptly', async () => {
+ // '' anywhere used to make each unbounded [^>]* rescan the
+ // rest of the document from every ' ' {
+ const cancel = vi.fn().mockResolvedValue(undefined);
+ stubFetch({
+ ok: true,
+ headers: { get: (h: string) => (h === 'content-type' ? 'video/mp4' : null) },
+ body: { getReader: () => ({ read: async () => ({ done: true }), cancel }), cancel },
+ text: async () => 'Nicht gelesen',
+ });
+ const result = await freshSvc().linkPreview('https://example.com/video');
+ expect(result.title).toBeNull();
+ // An unread body keeps its socket reserved until the collector runs.
+ expect(cancel).toHaveBeenCalled();
+ });
+
+ it('COLLAB-SVC-044: an og:image the client could not load safely is dropped', async () => {
+ // The client puts this straight into an
. A previewed page must not
+ // be able to aim that at a scheme, or at a host, of its choosing.
+ for (const [image, expected] of [
+ ['javascript:alert(1)', null],
+ ['data:image/png;base64,AAAA', null],
+ ['file:///etc/passwd', null],
+ ['//evil.example/x.png', null],
+ ['https://cdn.example/ok.png', 'https://cdn.example/ok.png'],
+ ['http://cdn.example/ok.png', 'http://cdn.example/ok.png'],
+ ] as const) {
+ stubFetch({ ok: true, text: async () => `` });
+ const result = await freshSvc().linkPreview(`https://example.com/img-${encodeURIComponent(image)}`);
+ expect(result.image).toBe(expected);
+ }
+ });
+
+ it('COLLAB-SVC-045: a preview already fetched is served again without a second request', async () => {
+ const fetchMock = stubFetch({ ok: true, text: async () => 'Einmal geholt' });
+ const service = freshSvc();
+ const first = await service.linkPreview('https://example.com/cached', 7);
+ const second = await service.linkPreview('https://example.com/cached', 7);
+ expect(first.title).toBe('Einmal geholt');
+ expect(second.title).toBe('Einmal geholt');
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ });
+
+ it('COLLAB-SVC-046: the budget runs out per user, and cached previews do not charge it', async () => {
+ const fetchMock = stubFetch({ ok: true, text: async () => 'T' });
+ const service = freshSvc();
+
+ // 60 distinct URLs is the whole allowance for a minute.
+ for (let i = 0; i < 60; i++) {
+ expect((await service.linkPreview(`https://example.com/p${i}`, 42)).rateLimited).toBeUndefined();
+ }
+ expect((await service.linkPreview('https://example.com/p60', 42)).rateLimited).toBe(true);
+ expect(fetchMock).toHaveBeenCalledTimes(60);
+
+ // The client re-requests every preview it renders on each mount, so a reload
+ // has to stay free: otherwise the fix is the regression.
+ expect((await service.linkPreview('https://example.com/p0', 42)).rateLimited).toBeUndefined();
+ expect(fetchMock).toHaveBeenCalledTimes(60);
+
+ // Another user has their own allowance.
+ expect((await service.linkPreview('https://example.com/p60', 43)).rateLimited).toBeUndefined();
+ });
+
+ it('COLLAB-SVC-048: simultaneous askers for one URL share a single outbound fetch', async () => {
+ // The client renders a preview per message and deduplicates nothing, so the
+ // same link posted twenty times arrives as twenty requests at once — none of
+ // which finds a cache entry, because the first has not answered yet.
+ let release: (() => void) | undefined;
+ const gate = new Promise(resolve => { release = resolve; });
+ const fetchMock = vi.fn().mockImplementation(async () => {
+ await gate;
+ return { ok: true, headers: { get: () => null }, text: async () => 'Geteilt' };
+ });
+ vi.stubGlobal('fetch', fetchMock);
+
+ const service = freshSvc();
+ const all = Promise.all(Array.from({ length: 20 }, () => service.linkPreview('https://example.com/same', 9)));
+ release!();
+ const results = await all;
+
+ expect(fetchMock).toHaveBeenCalledTimes(1);
+ expect(results.every(r => r.title === 'Geteilt')).toBe(true);
+ // And the nineteen that joined were not charged for a fetch they did not make.
+ expect(results.some(r => r.rateLimited)).toBe(false);
+ });
+
+ it('COLLAB-SVC-047: a body over the cap, and one behind a failed response, are both released', async () => {
+ for (const response of [
+ { ok: true, headers: { get: (h: string) => (h === 'content-length' ? String(10 * 1024 * 1024) : null) } },
+ { ok: false },
+ ]) {
+ const cancel = vi.fn().mockResolvedValue(undefined);
+ stubFetch({ ...response, body: { getReader: () => ({ read: async () => ({ done: true }), cancel }), cancel }, text: async () => '' });
+ const result = await freshSvc().linkPreview(`https://example.com/drop-${String(response.ok)}`);
+ expect(result.title).toBeNull();
+ expect(cancel).toHaveBeenCalled();
+ }
+ });
+});
+
// COLLAB-SVC-031..033 (collab.bridge delegation) were deleted with the bridge —
// its last consumers (the legacy tripService and the legacy get_trip_summary
// registrar) migrated into the DI-native TripsService/TripsMcp.
@@ -498,7 +666,7 @@ describe('hardening', () => {
it('COLLAB-SVC-034: votePoll switch is atomic — prior vote survives a failed INSERT', () => {
const { user1, trip } = setup();
const dbs = new DatabaseService(testDb);
- const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), collabFx.storage);
+ const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), collabFx.storage, new RateLimitService());
const poll = failing.createPoll(trip.id, user1.id, { question: 'Q?', options: ['A', 'B'] });
failing.votePoll(trip.id, poll!.id, user1.id, 0);
@@ -519,7 +687,7 @@ describe('hardening', () => {
it('COLLAB-SVC-035: deleteNote is atomic — trip_files rows survive a failed note DELETE', async () => {
const { user1, trip } = setup();
const dbs = new DatabaseService(testDb);
- const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), collabFx.storage);
+ const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), collabFx.storage, new RateLimitService());
const note = failing.createNote(trip.id, user1.id, { title: 'With file' });
testDb.prepare('INSERT INTO trip_files (trip_id, note_id, filename, original_name) VALUES (?, ?, ?, ?)')
.run(trip.id, note.id, 'files/a.pdf', 'a.pdf');
@@ -540,7 +708,7 @@ describe('hardening', () => {
const { user1, trip } = setup();
const dbs = new DatabaseService(testDb);
const failingStorage = { delete: vi.fn().mockRejectedValue(new Error('EACCES')) };
- const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), failingStorage as unknown as import('../../../src/nest/storage/storage.service').StorageService);
+ const failing = new CollabService(dbs, new PermissionsService(dbs), new RealtimeService(), notificationsStub(), failingStorage as unknown as import('../../../src/nest/storage/storage.service').StorageService, new RateLimitService());
const note = failing.createNote(trip.id, user1.id, { title: 'Sticky file' });
testDb.prepare('INSERT INTO trip_files (trip_id, note_id, filename, original_name) VALUES (?, ?, ?, ?)')
.run(trip.id, note.id, 'stuck.pdf', 'stuck.pdf');
diff --git a/server/tests/unit/nest/trip-read-model.service.test.ts b/server/tests/unit/nest/trip-read-model.service.test.ts
index 262652900..feb0d046a 100644
--- a/server/tests/unit/nest/trip-read-model.service.test.ts
+++ b/server/tests/unit/nest/trip-read-model.service.test.ts
@@ -60,6 +60,7 @@ import { ReservationsReadRepository } from '../../../src/nest/reservations/reser
import { BudgetService } from '../../../src/nest/budget/budget.service';
import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service';
import { CollabService } from '../../../src/nest/collab/collab.service';
+import { RateLimitService } from '../../../src/nest/common/rate-limit.service';
import { PlacesService } from '../../../src/nest/places/places.service';
import { UserCleanupService } from '../../../src/nest/auth/user-cleanup.service';
import { TripMembersService } from '../../../src/nest/trip-members/trip-members.service';
@@ -100,7 +101,7 @@ const buildReadModel = (database: DatabaseService, roster: TripMembersService =
database, roster, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
- new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), makeStorageFixture('').storage),
+ new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), makeStorageFixture('').storage, new RateLimitService()),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new EphemeralTokenService(), makeStorageFixture('').storage),
diff --git a/server/tests/unit/nest/trips.service.test.ts b/server/tests/unit/nest/trips.service.test.ts
index 61a5a8bb6..b03919a43 100644
--- a/server/tests/unit/nest/trips.service.test.ts
+++ b/server/tests/unit/nest/trips.service.test.ts
@@ -66,6 +66,7 @@ import { ReservationsReadRepository } from '../../../src/nest/reservations/reser
import { BudgetService } from '../../../src/nest/budget/budget.service';
import { ExchangeRatesService } from '../../../src/nest/budget/exchange-rates.service';
import { CollabService } from '../../../src/nest/collab/collab.service';
+import { RateLimitService } from '../../../src/nest/common/rate-limit.service';
import { VacayService } from '../../../src/nest/vacay/vacay.service';
import { TripsService } from '../../../src/nest/trips/trips.service';
import { PlacesService } from '../../../src/nest/places/places.service';
@@ -130,7 +131,7 @@ const readModelSvc = new TripReadModelService(
dbs(), membersSvc, daysSvc, accommodationsSvc, budgetSvc,
new PackingService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub()),
new ReservationsService(dbs(), new PermissionsService(dbs()), budgetSvc, new RealtimeService(), notificationsStub(), new ReservationsReadRepository(dbs())),
- new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), coversFx.storage),
+ new CollabService(dbs(), new PermissionsService(dbs()), new RealtimeService(), notificationsStub(), coversFx.storage, new RateLimitService()),
placesSvc,
new TodoService(dbs(), new PermissionsService(dbs()), new RealtimeService()),
new FilesService(dbs(), new PermissionsService(dbs()), new RealtimeService(), new EphemeralTokenService(), coversFx.storage),
diff --git a/server/tests/unit/utils/ssrfGuard.test.ts b/server/tests/unit/utils/ssrfGuard.test.ts
index af572ee1f..054740cb4 100644
--- a/server/tests/unit/utils/ssrfGuard.test.ts
+++ b/server/tests/unit/utils/ssrfGuard.test.ts
@@ -179,6 +179,76 @@ describe('checkSsrf', () => {
});
});
+ // SEC-013 — the unspecified address and IPv4-mapped spellings
+ describe('unspecified address and IPv4-mapped spellings (always blocked)', () => {
+ // Connecting to the unspecified address lands on loopback, so `::` reaches a
+ // local service without ever naming one. It has several spellings and none of
+ // them start with '0.', which is all the IPv4 check ever looked for.
+ it.each([
+ ['bare', '::'],
+ ['single zero', '::0'],
+ ['fully written out', '0:0:0:0:0:0:0:0'],
+ ['zero-padded', '0000:0000:0000:0000:0000:0000:0000:0000'],
+ ])('SEC-013: blocks the unspecified address, %s (%s)', async (_label, ip) => {
+ mockIp(ip);
+ const result = await checkSsrf('http://attacker.example', true);
+ expect(result.allowed).toBe(false);
+ });
+
+ // An IPv4-mapped address is the IPv4 it carries. Matching '::ffff:127.' and
+ // '::ffff:169.254.' as text missed both the hex spelling of those same
+ // addresses and every other blocked range.
+ it.each([
+ ['mapped loopback, dotted', '::ffff:127.0.0.1'],
+ ['mapped loopback, hex', '::ffff:7f00:1'],
+ ['mapped metadata, dotted', '::ffff:169.254.169.254'],
+ ['mapped metadata, hex', '::ffff:a9fe:a9fe'],
+ ['mapped unspecified', '::ffff:0:0'],
+ ['mapped 0.0.0.0, dotted', '::ffff:0.0.0.0'],
+ ])('SEC-013: blocks %s (%s)', async (_label, ip) => {
+ mockIp(ip);
+ const result = await checkSsrf('http://attacker.example', true);
+ expect(result.allowed).toBe(false);
+ });
+
+ it.each([
+ ['mapped RFC-1918, hex', '::ffff:c0a8:1'],
+ ['mapped CGNAT', '::ffff:100.64.0.1'],
+ ])('SEC-013: treats %s (%s) as private', async (_label, ip) => {
+ mockIp(ip);
+ const result = await checkSsrf('http://attacker.example');
+ expect(result.allowed).toBe(false);
+ expect(result.isPrivate).toBe(true);
+ });
+
+ it('SEC-013: still allows a mapped public address', async () => {
+ mockIp('::ffff:8.8.8.8');
+ const result = await checkSsrf('http://cdn.example');
+ expect(result.allowed).toBe(true);
+ expect(result.isPrivate).toBe(false);
+ });
+ });
+
+ // SEC-014 — fe80::/10 is ten bits, not the four characters 'fe80'
+ describe('the whole IPv6 link-local range', () => {
+ it.each([['fe80::1'], ['fe90::1'], ['fea0::1'], ['febf::1']])(
+ 'SEC-014: blocks %s',
+ async (ip) => {
+ mockIp(ip);
+ const result = await checkSsrf('http://attacker.example', true);
+ expect(result.allowed).toBe(false);
+ },
+ );
+
+ // The bounds, so widening fe80: to the full /10 cannot creep further: fe7f
+ // sits just below the range and fec0 just above it, and neither is link-local.
+ it.each([['fe7f::1'], ['fec0::1']])('SEC-014: %s is outside fe80::/10', async (ip) => {
+ mockIp(ip);
+ const result = await checkSsrf('http://cdn.example', true);
+ expect(result.allowed).toBe(true);
+ });
+ });
+
describe('internal hostname suffixes', () => {
it('blocks .local domains', async () => {
const result = await checkSsrf('http://myserver.local');
diff --git a/wiki/Collab-Chat.md b/wiki/Collab-Chat.md
index 17462e8c9..6468aba9e 100644
--- a/wiki/Collab-Chat.md
+++ b/wiki/Collab-Chat.md
@@ -38,6 +38,8 @@ Hover a message to reveal the action buttons. Click **Reply** to quote that mess
When a message contains a URL, TREK automatically fetches an Open Graph preview (title, description, and thumbnail image) and displays it below the message text. Only the first URL in a message generates a preview.
+The fetch runs on the server, so it is kept on a short leash: only trip members can request one, the target must be a public HTTP(S) address (addresses on your own network are refused), a preview is remembered for ten minutes, and a member can trigger sixty new fetches a minute. A preview that is refused or runs past the budget is simply not shown — the message itself is unaffected.
+
## Message styling
Your own messages appear **right-aligned** with a blue bubble. Other members' messages appear **left-aligned** with a gray bubble. The username is shown above the first message in a group of consecutive messages from the same person; the avatar is shown beside the **last** message in that group. Timestamps appear below the last message in each group.