feat(validation): implement validateAnnotationStrokes function for safe annotation data handling

feat(rate-limit): add TRUSTED_PROXY_MODE for configurable proxy header trust
feat(comments): validate annotation data structure in comment routes and components
This commit is contained in:
Yusuf İpek
2026-04-10 20:38:22 +03:00
parent 03bfd565e8
commit 439e74d037
8 changed files with 134 additions and 29 deletions
+8
View File
@@ -74,6 +74,14 @@ NODE_ENV="development"
# Disable all app-level rate limiting for local testing. Leave unset to keep rate limiting enabled. # Disable all app-level rate limiting for local testing. Leave unset to keep rate limiting enabled.
# Accepted truthy values: "true", "1", "yes", "on" # Accepted truthy values: "true", "1", "yes", "on"
# DISABLE_RATE_LIMIT="true" # DISABLE_RATE_LIMIT="true"
# Trusted reverse proxy mode — controls which headers getClientIp() trusts for rate limiting.
# Set this only when you have confirmed that your proxy strips/overwrites client-supplied headers.
# cloudflare — trust cf-connecting-ip (Cloudflare edge in front of the origin)
# nginx — trust x-real-ip / last x-forwarded-for (Nginx real_ip_header with set_real_ip_from)
# Leave unset for local dev or when no trusted proxy is in place.
TRUSTED_PROXY_MODE="cloudflare"
# Admin emails for accessing the /admin panel (comma separated list) # Admin emails for accessing the /admin panel (comma separated list)
# e.g., "[email protected],[email protected]" # e.g., "[email protected],[email protected]"
ADMIN_EMAILS="" ADMIN_EMAILS=""
+12 -1
View File
@@ -9,6 +9,7 @@ import { getShareSessionFromRequest } from '@/lib/share-session';
import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response'; import { apiErrors, successResponse, withCacheControl } from '@/lib/api-response';
import { getGuestIdentityFromRequest } from '@/lib/guest-identity'; import { getGuestIdentityFromRequest } from '@/lib/guest-identity';
import { runWithConcurrency } from '@/lib/async-pool'; import { runWithConcurrency } from '@/lib/async-pool';
import { validateAnnotationStrokes } from '@/lib/validation';
const CLEANUP_DELETE_CONCURRENCY = 5; const CLEANUP_DELETE_CONCURRENCY = 5;
@@ -172,7 +173,17 @@ export async function PATCH(request: NextRequest, { params }: RouteParams) {
const updateData: Record<string, unknown> = {}; const updateData: Record<string, unknown> = {};
if (content !== undefined && typeof content === 'string') updateData.content = content.trim(); if (content !== undefined && typeof content === 'string') updateData.content = content.trim();
if (tagId !== undefined) updateData.tagId = tagId; if (tagId !== undefined) updateData.tagId = tagId;
if (annotationData !== undefined) updateData.annotationData = annotationData; if (annotationData !== undefined) {
if (annotationData === null) {
updateData.annotationData = null;
} else {
const validStrokes = validateAnnotationStrokes(annotationData);
if (validStrokes === null) {
return apiErrors.badRequest('annotationData must be an array of valid stroke objects');
}
updateData.annotationData = JSON.stringify(validStrokes);
}
}
if (isResolved !== undefined) { if (isResolved !== undefined) {
updateData.isResolved = isResolved; updateData.isResolved = isResolved;
updateData.resolvedAt = isResolved ? new Date() : null; updateData.resolvedAt = isResolved ? new Date() : null;
+13 -3
View File
@@ -10,6 +10,7 @@ import { HeadObjectCommand } from '@aws-sdk/client-s3';
import { r2Client, R2_BUCKET_NAME } from '@/lib/r2'; import { r2Client, R2_BUCKET_NAME } from '@/lib/r2';
import { ensureGuestIdentityFromRequest, getGuestIdentityFromRequest, setGuestIdentityCookie } from '@/lib/guest-identity'; import { ensureGuestIdentityFromRequest, getGuestIdentityFromRequest, setGuestIdentityCookie } from '@/lib/guest-identity';
import { extractImageFileNameFromProxyUrl, sanitizeAssetDisplayName } from '@/lib/video-assets'; import { extractImageFileNameFromProxyUrl, sanitizeAssetDisplayName } from '@/lib/video-assets';
import { validateAnnotationStrokes } from '@/lib/validation';
type RouteParams = { params: Promise<{ versionId: string }> }; type RouteParams = { params: Promise<{ versionId: string }> };
const SAFE_IMAGE_PATH = /^\/api\/upload\/image\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i; const SAFE_IMAGE_PATH = /^\/api\/upload\/image\/[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\.[a-z0-9]+$/i;
@@ -259,8 +260,17 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
if (guestName !== undefined && guestName !== null && String(guestName).length > 100) { if (guestName !== undefined && guestName !== null && String(guestName).length > 100) {
return apiErrors.badRequest('Guest name must be 100 characters or fewer'); return apiErrors.badRequest('Guest name must be 100 characters or fewer');
} }
if (annotationData !== undefined && annotationData !== null && JSON.stringify(annotationData).length > 50_000) {
return apiErrors.badRequest('Annotation data is too large'); // Validate annotation data structure to prevent prototype pollution and stored XSS.
// Reject anything that is not a well-formed array of AnnotationStroke objects.
let serializedAnnotationData: string | null = null;
if (annotationData !== undefined && annotationData !== null) {
const validStrokes = validateAnnotationStrokes(annotationData);
if (validStrokes === null) {
return apiErrors.badRequest('annotationData must be an array of valid stroke objects');
}
// Re-serialize to canonical JSON — strips any extra properties from the input.
serializedAnnotationData = JSON.stringify(validStrokes);
} }
// If replying, verify parent exists in same version // If replying, verify parent exists in same version
@@ -309,7 +319,7 @@ export async function POST(request: NextRequest, { params }: RouteParams) {
voiceUrl: voiceUrl || null, voiceUrl: voiceUrl || null,
voiceDuration: voiceDuration || null, voiceDuration: voiceDuration || null,
imageUrl: imageUrl || null, imageUrl: imageUrl || null,
annotationData: annotationData || null, annotationData: serializedAnnotationData,
authorId: session?.user?.id || null, authorId: session?.user?.id || null,
guestName: isGuest ? guestName : null, guestName: isGuest ? guestName : null,
guestEmail: isGuest ? guestEmail : null, guestEmail: isGuest ? guestEmail : null,
+5 -2
View File
@@ -13,6 +13,7 @@ import { VideoPageLoading } from '@/components/video-page/video-page-loading';
import { VideoPageError } from '@/components/video-page/video-page-error'; import { VideoPageError } from '@/components/video-page/video-page-error';
import { GuestNameGate } from '@/components/video-page/guest-name-gate'; import { GuestNameGate } from '@/components/video-page/guest-name-gate';
import { useCommentMedia } from '@/components/video-page/hooks/use-comment-media'; import { useCommentMedia } from '@/components/video-page/hooks/use-comment-media';
import { validateAnnotationStrokes } from '@/lib/validation';
import { useVersionActions } from '@/components/video-page/hooks/use-version-actions'; import { useVersionActions } from '@/components/video-page/hooks/use-version-actions';
import { useWatchProgress } from '@/components/video-page/hooks/use-watch-progress'; import { useWatchProgress } from '@/components/video-page/hooks/use-watch-progress';
import { useVideoPlayer } from '@/components/video-page/hooks/use-video-player'; import { useVideoPlayer } from '@/components/video-page/hooks/use-video-player';
@@ -498,7 +499,8 @@ export function VideoPageContent({
const editAnnotationInitialStrokes = useMemo<AnnotationStroke[] | undefined>(() => { const editAnnotationInitialStrokes = useMemo<AnnotationStroke[] | undefined>(() => {
if (editAnnotationData) { if (editAnnotationData) {
try { try {
return JSON.parse(editAnnotationData) as AnnotationStroke[]; const parsed = JSON.parse(editAnnotationData);
return (validateAnnotationStrokes(parsed) as AnnotationStroke[] | null) ?? undefined;
} catch { } catch {
return undefined; return undefined;
} }
@@ -507,7 +509,8 @@ export function VideoPageContent({
const editingComment = comments.find((comment) => comment.id === editingCommentId); const editingComment = comments.find((comment) => comment.id === editingCommentId);
if (!editingComment?.annotationData) return undefined; if (!editingComment?.annotationData) return undefined;
try { try {
return JSON.parse(editingComment.annotationData) as AnnotationStroke[]; const parsed = JSON.parse(editingComment.annotationData);
return (validateAnnotationStrokes(parsed) as AnnotationStroke[] | null) ?? undefined;
} catch { } catch {
return undefined; return undefined;
} }
@@ -16,6 +16,7 @@ import { toast } from 'sonner';
import type { AnnotationCanvasHandle, AnnotationStroke } from '@/components/annotation-canvas'; import type { AnnotationCanvasHandle, AnnotationStroke } from '@/components/annotation-canvas';
import type { Comment, CommentActionsConfig, CommentTag, Version, VideoData } from '@/components/video-page/types'; import type { Comment, CommentActionsConfig, CommentTag, Version, VideoData } from '@/components/video-page/types';
import { extractPastedImageFile, validateImageFile } from '@/components/video-page/image-upload-utils'; import { extractPastedImageFile, validateImageFile } from '@/components/video-page/image-upload-utils';
import { validateAnnotationStrokes } from '@/lib/validation';
interface UseCommentActionsParams extends CommentActionsConfig { interface UseCommentActionsParams extends CommentActionsConfig {
setVideo: Dispatch<SetStateAction<VideoData | null>>; setVideo: Dispatch<SetStateAction<VideoData | null>>;
@@ -867,7 +868,9 @@ export function useCommentActions({
setIsEditingAnnotation(false); setIsEditingAnnotation(false);
if (finalAnnotationData !== undefined && finalAnnotationData) { if (finalAnnotationData !== undefined && finalAnnotationData) {
try { try {
setViewingAnnotation(JSON.parse(finalAnnotationData)); const parsed = JSON.parse(finalAnnotationData);
const safe = validateAnnotationStrokes(parsed);
if (safe) setViewingAnnotation(safe as AnnotationStroke[]);
} catch { } catch {
// ignore parse errors // ignore parse errors
} }
@@ -11,6 +11,7 @@ import type {
PlayerAdapter, PlayerAdapter,
Version, Version,
} from '@/components/video-page/types'; } from '@/components/video-page/types';
import { validateAnnotationStrokes } from '@/lib/validation';
interface UseVideoPlayerParams { interface UseVideoPlayerParams {
activeVersion: Version | undefined; activeVersion: Version | undefined;
@@ -765,8 +766,9 @@ export function useVideoPlayer({
} }
if (annotation) { if (annotation) {
try { try {
const strokes = JSON.parse(annotation) as AnnotationStroke[]; const parsed = JSON.parse(annotation);
setViewingAnnotation(strokes); const safe = validateAnnotationStrokes(parsed);
setViewingAnnotation(safe as AnnotationStroke[] | null);
} catch { } catch {
setViewingAnnotation(null); setViewingAnnotation(null);
} }
+32 -15
View File
@@ -150,33 +150,50 @@ function isPlausibleIp(value: string): boolean {
/** /**
* Get client IP from request headers. * Get client IP from request headers.
* *
* Header priority: * Trusting proxy-injected headers is only safe when a known trusted proxy sits in front
* 1. cf-connecting-ip — set by Cloudflare (trusted proxy); cannot be spoofed by clients * of this server and strips or overwrites those headers before forwarding requests.
* 2. x-forwarded-for — first entry, trusted only behind a proxy that overwrites it * Set TRUSTED_PROXY_MODE to opt in:
* 3. x-real-ip — set by some reverse proxies (Nginx)
* 4. 127.0.0.1 — local development fallback
* *
* Deployed behind Cloudflare, so cf-connecting-ip is the canonical source. * TRUSTED_PROXY_MODE=cloudflare — trust cf-connecting-ip (Cloudflare edge)
* TRUSTED_PROXY_MODE=nginx — trust x-real-ip / x-forwarded-for (Nginx real_ip_header)
*
* Without TRUSTED_PROXY_MODE set, no proxy headers are trusted: all requests appear
* as 127.0.0.1, which means rate limits apply per-process rather than per-client IP.
* In that configuration, prefer session/user-keyed rate limits for authenticated endpoints.
*
* WARNING: Do not set TRUSTED_PROXY_MODE unless you have confirmed that your proxy
* strips or overwrites the corresponding headers on every inbound request. Failing to
* do so allows clients to spoof their IP and bypass rate limits.
*/ */
export function getClientIp(request: Request): string { export function getClientIp(request: Request): string {
// Cloudflare always sets this to the true client IP const mode = process.env.TRUSTED_PROXY_MODE?.trim().toLowerCase();
if (mode === 'cloudflare') {
// cf-connecting-ip is injected by Cloudflare and cannot be set by clients
// when origin access is restricted to Cloudflare's IP ranges.
const cfIp = request.headers.get('cf-connecting-ip'); const cfIp = request.headers.get('cf-connecting-ip');
if (cfIp && isPlausibleIp(cfIp)) { if (cfIp && isPlausibleIp(cfIp)) {
return cfIp; return cfIp;
} }
}
if (mode === 'nginx') {
// x-real-ip is set by Nginx's real_ip_header directive (connection-level, not spoofable
// by clients when set_real_ip_from is configured for the upstream proxy).
const realIp = request.headers.get('x-real-ip');
if (realIp && isPlausibleIp(realIp)) return realIp;
// x-forwarded-for last entry added by Nginx when proxy_add_x_forwarded_for is used.
const forwardedFor = request.headers.get('x-forwarded-for'); const forwardedFor = request.headers.get('x-forwarded-for');
if (forwardedFor) { if (forwardedFor) {
const first = forwardedFor.split(',')[0].trim(); const entries = forwardedFor.split(',');
if (isPlausibleIp(first)) return first; const last = entries[entries.length - 1].trim();
if (isPlausibleIp(last)) return last;
}
} }
const realIp = request.headers.get('x-real-ip'); // No trusted proxy configured — fall back to a constant value.
if (realIp && isPlausibleIp(realIp)) { // Rate limiting will apply per-process; use userId-keyed limits for authenticated endpoints.
return realIp;
}
// Fallback for local development
return '127.0.0.1'; return '127.0.0.1';
} }
+51
View File
@@ -11,6 +11,57 @@ export function isValidHttpUrl(urlString: string): boolean {
} }
} }
// Matches exactly 6-digit hex colours produced by the annotation canvas (e.g. #FF3B30)
const ANNOTATION_COLOR_RE = /^#[0-9a-fA-F]{6}$/;
const MAX_STROKES = 500;
const MAX_POINTS_PER_STROKE = 2000;
const MIN_STROKE_WIDTH = 1;
const MAX_STROKE_WIDTH = 20;
/**
* Validates and returns a safe copy of annotation stroke data.
*
* Accepts only an array of plain stroke objects with the exact shape created
* by AnnotationCanvas. Rejects anything that could trigger prototype pollution
* or carry unexpected properties into the renderer.
*
* Returns null when the input is absent or structurally invalid.
*/
export function validateAnnotationStrokes(
data: unknown
): { points: { x: number; y: number }[]; color: string; width: number }[] | null {
if (data === null || data === undefined) return null;
if (!Array.isArray(data)) return null;
if (data.length > MAX_STROKES) return null;
const result: { points: { x: number; y: number }[]; color: string; width: number }[] = [];
for (const stroke of data) {
if (stroke === null || typeof stroke !== 'object' || Array.isArray(stroke)) return null;
const { points, color, width } = stroke as Record<string, unknown>;
if (!Array.isArray(points)) return null;
if (points.length > MAX_POINTS_PER_STROKE) return null;
const safePoints: { x: number; y: number }[] = [];
for (const pt of points) {
if (pt === null || typeof pt !== 'object' || Array.isArray(pt)) return null;
const { x, y } = pt as Record<string, unknown>;
if (typeof x !== 'number' || !isFinite(x)) return null;
if (typeof y !== 'number' || !isFinite(y)) return null;
safePoints.push({ x, y });
}
if (typeof color !== 'string' || !ANNOTATION_COLOR_RE.test(color)) return null;
if (typeof width !== 'number' || width < MIN_STROKE_WIDTH || width > MAX_STROKE_WIDTH) return null;
result.push({ points: safePoints, color, width });
}
return result;
}
/** /**
* Validates a URL and returns an error message if invalid * Validates a URL and returns an error message if invalid
*/ */