Public-release prep: ELv2 license, README/CLA/NOTICE, SSRF safeFetch, identifier genericization, export tooling
This commit is contained in:
@@ -5,7 +5,7 @@ import { getDb, schema } from '../db/index.js';
|
||||
import { generateSnowflake } from './snowflake.js';
|
||||
import { classifyUrl } from './embedClassifier.js';
|
||||
import { fetchUrlMetadata } from './metadataFetcher.js';
|
||||
import { validateExternalUrl } from './ssrf.js';
|
||||
import { safeFetch } from './ssrf.js';
|
||||
import { connectionManager } from '../ws/handler.js';
|
||||
|
||||
const MAX_EMBEDS_PER_MESSAGE = 5;
|
||||
@@ -18,27 +18,20 @@ const PROBE_TIMEOUT_MS = 3_000;
|
||||
* Uses Range request to avoid downloading the entire file.
|
||||
* Returns null on any failure (timeout, network, unrecognized format, SSRF block).
|
||||
*/
|
||||
async function probeRemoteImageDimensions(
|
||||
export async function probeRemoteImageDimensions(
|
||||
url: string,
|
||||
): Promise<{ width: number; height: number } | null> {
|
||||
try {
|
||||
await validateExternalUrl(url);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), PROBE_TIMEOUT_MS);
|
||||
|
||||
try {
|
||||
const response = await fetch(url, {
|
||||
const response = await safeFetch(url, {
|
||||
headers: {
|
||||
'User-Agent': 'BackspaceBot/1.0',
|
||||
Accept: 'image/*',
|
||||
Range: `bytes=0-${PROBE_BYTES - 1}`,
|
||||
},
|
||||
signal: controller.signal,
|
||||
redirect: 'follow',
|
||||
});
|
||||
// NOTE: Do NOT clearTimeout here — keep the abort active during body read.
|
||||
// The finally block handles cleanup after all reads complete.
|
||||
|
||||
@@ -73,7 +73,7 @@ beforeEach(() => {
|
||||
}).run();
|
||||
// Online native FRIENDED with the stub — should be snapshotted
|
||||
testDb.insert(schema.users).values({
|
||||
id: 'native-friend', username: 'youruser', passwordHash: 'x',
|
||||
id: 'native-friend', username: 'erin', passwordHash: 'x',
|
||||
status: 'online', isAdmin: 0, homeUserId: 'native-friend', createdAt: Date.now(),
|
||||
}).run();
|
||||
testDb.insert(schema.friends).values({
|
||||
|
||||
@@ -62,7 +62,7 @@ beforeEach(() => {
|
||||
// Native local user
|
||||
testDb.insert(schema.users).values({
|
||||
id: 'native-1',
|
||||
username: 'youruser',
|
||||
username: 'erin',
|
||||
passwordHash: 'x',
|
||||
status: 'online',
|
||||
isAdmin: 0,
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import * as cheerio from 'cheerio';
|
||||
import { validateExternalUrl } from './ssrf.js';
|
||||
import { safeFetch } from './ssrf.js';
|
||||
|
||||
export interface UrlMetadata {
|
||||
title: string | null;
|
||||
@@ -16,21 +16,14 @@ export interface UrlMetadata {
|
||||
}
|
||||
|
||||
export async function fetchUrlMetadata(url: string): Promise<UrlMetadata | null> {
|
||||
try {
|
||||
await validateExternalUrl(url);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
|
||||
const controller = new AbortController();
|
||||
const timeout = setTimeout(() => controller.abort(), 5000);
|
||||
try {
|
||||
const response = await fetch(url, {
|
||||
const response = await safeFetch(url, {
|
||||
headers: {
|
||||
'User-Agent': 'BackspaceBot/1.0',
|
||||
},
|
||||
signal: controller.signal,
|
||||
redirect: 'follow',
|
||||
});
|
||||
clearTimeout(timeout);
|
||||
|
||||
|
||||
@@ -22,11 +22,18 @@ vi.mock('../db/index.js', () => ({
|
||||
}));
|
||||
|
||||
// Mock the ssrf module so tests don't need real DNS resolution.
|
||||
// Default: validateExternalUrl resolves (allow). Individual tests override as needed.
|
||||
vi.mock('./ssrf.js', () => ({
|
||||
validateExternalUrl: vi.fn().mockResolvedValue(undefined),
|
||||
isPrivateIp: vi.fn().mockReturnValue(false),
|
||||
}));
|
||||
// safeFetch mirrors the real contract: validate the URL first, then fetch — so a
|
||||
// rejecting validator must prevent the fetch. Default: validateExternalUrl
|
||||
// resolves (allow). Individual tests override via mockRejectedValueOnce.
|
||||
vi.mock('./ssrf.js', () => {
|
||||
const validateExternalUrl = vi.fn().mockResolvedValue(undefined);
|
||||
const isPrivateIp = vi.fn().mockReturnValue(false);
|
||||
const safeFetch = vi.fn(async (url: string, init?: RequestInit) => {
|
||||
await validateExternalUrl(url);
|
||||
return (global.fetch as unknown as typeof fetch)(url, init);
|
||||
});
|
||||
return { validateExternalUrl, isPrivateIp, safeFetch };
|
||||
});
|
||||
|
||||
function applyMigrations(db: Database.Database): void {
|
||||
const migrationsDir = path.resolve(__dirname, '../../drizzle');
|
||||
@@ -95,16 +102,14 @@ describe('fetchSpaceInviteSnapshot', () => {
|
||||
});
|
||||
|
||||
it('returns null when SSRF validator rejects the origin', async () => {
|
||||
// Override the module-level mock to reject for this test only.
|
||||
// Reject validation for this one call; safeFetch must bail before fetching.
|
||||
const ssrf = await import('./ssrf.js');
|
||||
const spy = vi.spyOn(ssrf, 'validateExternalUrl').mockRejectedValueOnce(new Error('blocked'));
|
||||
(ssrf.validateExternalUrl as unknown as ReturnType<typeof vi.fn>).mockRejectedValueOnce(new Error('blocked'));
|
||||
const fetchSpy = global.fetch as any;
|
||||
|
||||
const snap = await fetchSpaceInviteSnapshot('http://127.0.0.1:9200', 'abc');
|
||||
expect(snap).toBeNull();
|
||||
expect(fetchSpy).not.toHaveBeenCalled(); // CRITICAL — the fetch must NOT happen
|
||||
|
||||
spy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import type { AvatarColor } from '@backspace/shared';
|
||||
import { eq } from 'drizzle-orm';
|
||||
import { getDb, schema } from '../db/index.js';
|
||||
import { validateExternalUrl } from './ssrf.js';
|
||||
import { safeFetch } from './ssrf.js';
|
||||
|
||||
export interface SpaceInviteSnapshot {
|
||||
spaceId: string;
|
||||
@@ -59,15 +59,10 @@ export async function fetchSpaceInviteSnapshot(
|
||||
timeoutMs = 5000,
|
||||
): Promise<SpaceInviteSnapshot | null> {
|
||||
const url = `${spaceInstanceOrigin}/api/spaces/invite/${encodeURIComponent(inviteCode)}/preview`;
|
||||
try {
|
||||
await validateExternalUrl(url);
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
const controller = new AbortController();
|
||||
const timer = setTimeout(() => controller.abort(), timeoutMs);
|
||||
try {
|
||||
const res = await fetch(url, { signal: controller.signal });
|
||||
const res = await safeFetch(url, { signal: controller.signal });
|
||||
if (!res.ok) return null;
|
||||
const data = await res.json() as Partial<SpaceInviteSnapshot>;
|
||||
if (typeof data?.spaceId !== 'string' || typeof data?.spaceName !== 'string') return null;
|
||||
|
||||
@@ -43,3 +43,38 @@ export async function validateExternalUrl(url: string): Promise<void> {
|
||||
throw new Error('Private IP not allowed');
|
||||
}
|
||||
}
|
||||
|
||||
const MAX_REDIRECTS = 5;
|
||||
|
||||
/**
|
||||
* SSRF-safe fetch. Validates the target URL and re-validates the destination of
|
||||
* every redirect hop before following it, so a hostile server cannot 30x-redirect
|
||||
* an outbound request to an internal address (loopback, link-local, RFC1918).
|
||||
*
|
||||
* Use this instead of bare `fetch()` for any request to a user- or peer-supplied
|
||||
* URL. Redirects are followed manually (Node/undici exposes the 3xx + Location
|
||||
* with `redirect: 'manual'`), capped at MAX_REDIRECTS.
|
||||
*
|
||||
* Residual: validateExternalUrl resolves DNS, then fetch resolves again — a
|
||||
* narrow DNS-rebinding TOCTOU window remains. Pinning the resolved IP at connect
|
||||
* time would close it but requires a custom dispatcher; the redirect re-check
|
||||
* here closes the practical, attacker-controlled bypass.
|
||||
*/
|
||||
export async function safeFetch(url: string, init: RequestInit = {}): Promise<Response> {
|
||||
let currentUrl = url;
|
||||
for (let hop = 0; hop <= MAX_REDIRECTS; hop++) {
|
||||
await validateExternalUrl(currentUrl);
|
||||
const response = await fetch(currentUrl, { ...init, redirect: 'manual' });
|
||||
|
||||
if (response.status >= 300 && response.status < 400) {
|
||||
const location = response.headers.get('location');
|
||||
if (!location) return response; // 3xx without a target — hand back as-is
|
||||
// Resolve relative redirects against the current URL, then loop to re-validate.
|
||||
currentUrl = new URL(location, currentUrl).toString();
|
||||
continue;
|
||||
}
|
||||
|
||||
return response;
|
||||
}
|
||||
throw new Error('Too many redirects');
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user