From 5bd9c2f66d2ec4ef9b094f77ccfbc3f83ef75939 Mon Sep 17 00:00:00 2001 From: lekss361 Date: Sun, 24 May 2026 14:38:56 +0300 Subject: [PATCH] fix(tradein-frontend): safeUrl validator, apiFetch dedup, ListingsCard a11y, error boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four security/UX fixes from 2026-05-24 audit: * lib/safeUrl.ts (new): validate external URLs, reject non-http(s) schemes (closes finding #3 — backend supplies lot.source_url verbatim from scraped sites; javascript:/data:/file: would have executed on click). * lib/api.ts: refactor apiFetch to delegate to apiFetchWithStatus — single read-body-once implementation, drops the body-stream-twice anti-pattern (finding #8). * components/trade-in/ListingsCard.tsx: drop tr.onClick (not keyboard accessible), keep the inline as the only interactive element, gate it through safeUrl() (findings #7 + #3). * app/error.tsx (new): Next 15 App Router route-segment error boundary — prevents white-screen on component crash (finding #16). Scope-cut from original audit: * #17 (sessionId crypto.randomUUID) — already implemented in sessionId.ts; no change needed. * safeUrl wiring in DealsCard / IMVBenchmark — neither renders user-supplied URLs (DealsCard has no links, IMVBenchmark links a hardcoded domain). --- tradein-mvp/frontend/src/app/error.tsx | 55 +++++++++++++++++++ .../src/components/trade-in/ListingsCard.tsx | 18 +++--- tradein-mvp/frontend/src/lib/api.ts | 26 +++------ tradein-mvp/frontend/src/lib/safeUrl.ts | 23 ++++++++ 4 files changed, 96 insertions(+), 26 deletions(-) create mode 100644 tradein-mvp/frontend/src/app/error.tsx create mode 100644 tradein-mvp/frontend/src/lib/safeUrl.ts diff --git a/tradein-mvp/frontend/src/app/error.tsx b/tradein-mvp/frontend/src/app/error.tsx new file mode 100644 index 00000000..9c4caa65 --- /dev/null +++ b/tradein-mvp/frontend/src/app/error.tsx @@ -0,0 +1,55 @@ +"use client"; + +/** + * App Router error boundary for the trade-in page (Next 15 idiom). + * + * Triggers when a component below `app/page.tsx` throws during render or + * inside an Effect. Without this, the page would white-screen. Closes + * finding #16 from 2026-05-24 trade-in audit. + */ +import { useEffect } from "react"; + +interface Props { + error: Error & { digest?: string }; + reset: () => void; +} + +export default function Error({ error, reset }: Props) { + useEffect(() => { + // Surfaces to browser devtools + (if wired) Sentry. + console.error("trade-in page error:", error); + }, [error]); + + return ( +
+

+ Что-то пошло не так +

+

+ Произошла ошибка при отображении страницы. Можно попробовать ещё раз — + если повторяется, перезагрузите страницу. +

+ {error.digest && ( +

+ ID ошибки: {error.digest} +

+ )} + +
+ ); +} diff --git a/tradein-mvp/frontend/src/components/trade-in/ListingsCard.tsx b/tradein-mvp/frontend/src/components/trade-in/ListingsCard.tsx index 78acd6ae..441b6b0b 100644 --- a/tradein-mvp/frontend/src/components/trade-in/ListingsCard.tsx +++ b/tradein-mvp/frontend/src/components/trade-in/ListingsCard.tsx @@ -5,6 +5,7 @@ * Counts strip + filters + price bar + table. */ import type { AggregatedEstimate, AnalogLot } from "@/types/trade-in"; +import { safeUrl } from "@/lib/safeUrl"; interface Props { estimate: AggregatedEstimate; @@ -243,13 +244,15 @@ function AnalogRow({ lot }: { lot: AnalogLot }) { const dot = SOURCE_DOTS[src] ?? "dom"; const label = SOURCE_LABELS[src] ?? src; const distance = - lot.distance_m === null ? "—" : lot.distance_m < 1000 ? `${lot.distance_m} м` : `${(lot.distance_m / 1000).toFixed(1)} км`; - const onClick = lot.source_url - ? () => window.open(lot.source_url!, "_blank", "noopener,noreferrer") - : undefined; + lot.distance_m === null + ? "—" + : lot.distance_m < 1000 + ? `${lot.distance_m} м` + : `${(lot.distance_m / 1000).toFixed(1)} км`; + const externalUrl = safeUrl(lot.source_url); return ( - +
{lot.address} @@ -270,13 +273,12 @@ function AnalogRow({ lot }: { lot: AnalogLot }) { {fmtRub(lot.price_rub)} {distance} - {lot.source_url && ( + {externalUrl && ( e.stopPropagation()} aria-label="Открыть оригинал" > diff --git a/tradein-mvp/frontend/src/lib/api.ts b/tradein-mvp/frontend/src/lib/api.ts index 89793996..743c1e5e 100644 --- a/tradein-mvp/frontend/src/lib/api.ts +++ b/tradein-mvp/frontend/src/lib/api.ts @@ -26,24 +26,6 @@ function withSessionHeader(init?: RequestInit): RequestInit { }; } -export async function apiFetch( - path: string, - init?: RequestInit, -): Promise { - const merged = withSessionHeader(init); - const response = await fetch(`${API_BASE_URL}${path}`, { - ...merged, - headers: { - "Content-Type": "application/json", - ...(merged.headers ?? {}), - }, - }); - if (!response.ok) { - throw new Error(`API error ${response.status}: ${await response.text()}`); - } - return response.json() as Promise; -} - /** * Status-aware variant of apiFetch — возвращает status + body для случаев * когда логика зависит от HTTP кода (например 202 Accepted для async jobs). @@ -93,3 +75,11 @@ export async function apiFetchWithStatus( } return { status: response.status, body: body as T }; } + +export async function apiFetch( + path: string, + init?: RequestInit, +): Promise { + const { body } = await apiFetchWithStatus(path, init); + return body; +} diff --git a/tradein-mvp/frontend/src/lib/safeUrl.ts b/tradein-mvp/frontend/src/lib/safeUrl.ts new file mode 100644 index 00000000..1cfcf826 --- /dev/null +++ b/tradein-mvp/frontend/src/lib/safeUrl.ts @@ -0,0 +1,23 @@ +/** + * Validate an external URL string. Returns the URL only if it parses and uses + * an http/https scheme — otherwise null. + * + * Defends against: + * - javascript: URLs (XSS via href clicks) + * - data: URLs (data exfiltration, weird content) + * - file:, ftp:, custom-scheme abuse + * - malformed strings (null reference protection) + * + * Closes finding #3 from 2026-05-24 trade-in audit. Backend supplies + * lot.source_url verbatim from scraped sites — must not trust the string. + */ +export function safeUrl(raw: string | null | undefined): string | null { + if (!raw || typeof raw !== "string") return null; + try { + const u = new URL(raw); + if (u.protocol !== "http:" && u.protocol !== "https:") return null; + return u.toString(); + } catch { + return null; + } +} -- 2.45.3