Skip to content

AUDIT-2026-05-15.md

Latest commit

 

History

History
209 lines (162 loc) · 15 KB

AUDIT-2026-05-15.md

File metadata and controls

209 lines (162 loc) · 15 KB
Raw

L99 Audit Sprint - 2026-05-15

Consolidated findings from 4 audit lanes against commit efeb574 running live on elfixmobile-next.deni-4b0.workers.dev:

  1. audit-context-building - line-by-line review of new files (Lead pipeline, Reviews API, Telegram helper, CF Analytics block)
  2. differential-review - last 5 commits, risk rated
  3. live design - Playwright snapshot of /de, /de/buchen, /de/reparatur/apple/iphone-13, /de/preisrechner at desktop 1440 + mobile 390
  4. simplify - dead code and over-flexibility

P0 - Ship-blockers (data loss, security, broken pipeline)

P0-1. /api/lead rate-limit is a KV race - throttle bypassable

File: src/app/api/lead/route.ts:313-324 Classic read-modify-write on KV. Two concurrent requests both read cur=4, both write 5, attacker rides past RATE_LIMIT_MAX=5. KV has no atomic increment and global eventual-consistency makes it worse across edge regions. Fix: Use a Durable Object counter or the new cf.rateLimit binding. KV is best-effort - either move the counter or document "this is soft throttling, abuse via burst is possible." Cost to fix: ~1h (small DO).

P0-2. AGB checkbox not required at HTML/server layer

File: src/components/book/SimpleBookingForm.tsx (visible at runtime) DOM check: required: false on the AGB checkbox. Submit button is disabled via React state, but on JS-disabled browsers OR an attacker bypassing the client, the POST goes through without consent. The Zod schema on the server doesn't enforce AGB either. Fix: Add required attribute + add agb: z.literal(true) to the Zod schema in src/app/api/lead/route.ts. Return 422 if missing. Cost to fix: 5 min.

P1 - Broken UX or silent prod failure

P1-1. Telegram MarkdownV2 URL-escape breaks the status link

File: src/lib/telegram.ts:75 escapeMarkdownV2(statusUrl) escapes . and - inside the URL. The rendered link points to https://bersaev\.com/de/status/\<uuid\>?t=\<token\>, which Telegram delivers but most clients follow as text - and . inside the URL path is escaped wrong for Telegram's URL-position parser. Result: the status link inside the lead notification is broken. Fix: For URL position, escape only ) and \:

const escapedUrl = statusUrl.replace(/([)\\])/g, "\\$1");

Cost to fix: 2 min.

P1-2. Telegram code-block can fail entire send on backtick in summary

File: src/lib/telegram.ts:73 Triple-backtick code block contains args.summary.slice(0, 800). Customer message with a backtick (Display ist tot`` etc.) breaks the fence and Telegram returns 400 Bad Request: can't parse entities - no notification reaches Natalja. Fix: Inside the code-fence, escape both backticks and backslashes:

const safeSummary = args.summary.replace(/\\/g, "\\\\").replace(/`/g, "\\`").slice(0, 800);

Cost to fix: 2 min.

P1-3. Zod error tree leaked to public POST callers

File: src/app/api/lead/route.ts:300 parsed.error.toString() returns the full validator tree including field paths, regex names, custom messages. An attacker can fingerprint the schema (which fields are required, which regex names map to which inputs) by POSTing intentionally bad payloads. Fix: Return { ok: false, error: "invalid_payload" } in production; gate detail behind if (process.env.NODE_ENV === 'development'). Cost to fix: 5 min.

P1-4. /api/reviews cache thundering-herd on cold start

File: src/app/api/reviews/route.ts:120-135 No lock on the cache-miss path. On cold start (or every 24h after TTL expires), every concurrent request fires its own Google Places call before the first writer fills KV. Under traffic this bursts the quota by N requests in a few hundred ms. Fix: Stale-while-revalidate: keep stale_until separate from expires_at, return stale immediately while issuing one fetch via ctx.waitUntil. Or coalesce via a DO. Cost to fix: ~30 min.

P1-5. Google Places legacy API is on a deprecation sunset

File: src/app/api/reviews/route.ts:88-94 /maps/api/place/details/json is the legacy v0 endpoint. Google announced deprecation in March 2025; the new endpoint is places.googleapis.com/v1/places/{id} with X-Goog-Api-Key header and X-Goog-FieldMask. Legacy currently works but will break at some point. Fix: Migrate to the new API now while we have time - the field mask is a one-liner. Cost to fix: ~30 min.

P1-6. Title duplicated: "EL Fix Mobile | EL Fix Mobile"

Live observation: /de/buchen and /de/reparatur/apple/iphone-13 both render <title>… | EL Fix Mobile | EL Fix Mobile</title>. Damages Google SERP snippet. /de/preisrechner is correct. Root cause: Those pages set metadata.title = "X | EL Fix Mobile" AND the layout's metadata.title.template = "%s | EL Fix Mobile" adds another suffix. Fix: Strip " | EL Fix Mobile" from the page-level metadata.title on those two pages. Trust the template. Cost to fix: 5 min.

P1-7. Damage textarea prefills literal slug

Live observation: /de/buchen?repairs=display renders the word "display" in the damage textarea verbatim. Customer sees a placeholder-y "display" they have to delete to write their actual symptom. Fix: Map the slug through repair-labels.ts before prefill, e.g. "Display defekt - " as a starter sentence, or leave the field blank and just preselect the service tag. Cost to fix: 10 min.

P1-8. H1 spacing bug on model pages

Live observation: /de/reparatur/apple/iphone-13 renders Express-Reparatur: iPhone 13in Wien Aspern. - missing space between model name and "in". Fix: Find the template literal in the model page (likely ${model}in Wien → ${model} in Wien). Cost to fix: 2 min.

P1-9. Mobile menu hamburger has no aria-label

Live observation: The 59x36 icon-only button in Nav.tsx has no aria-label, no visible text, no aria-expanded. Screen-reader users can't open the mobile nav. Fix: Add aria-label="Menü öffnen" / aria-label="Menü schließen" + aria-expanded={isOpen} + aria-controls={menuId}. Cost to fix: 5 min.

P1-10. "Termin buchen" CTA is 131x23px (sub-44px tap target)

Live observation: Hero secondary CTA and "Reparatur starten" service-tile CTAs are 23px tall. WCAG 2.5.5 recommends 44x44; Apple HIG and Material both require 44+. Below this size on mobile causes mistaps. Fix: Wrap these in inline-flex items-center min-h-[44px] or convert the text-link styling into a pill button. Cost to fix: 10 min - find all Reparatur starten → / Termin buchen → link components and add min-height.

P1-11. /api/lead returns 502 when only Resend failed but KV+Telegram succeeded

File: src/app/api/lead/route.ts:430-436 On !ownerSend.ok the response is 502, but customer confirmation has already gone, Telegram has fired, and the lead is in KV. Customer thinks it failed and retries; dedup eats the retry but the customer is left with a poor signal. Fix: Return 200 { ok: true, id, token, warning: "owner_email_delayed" }. The lead is durable - 502 misrepresents the state. Cost to fix: 5 min.

P2 - Correctness papercuts

P2-1. Reviews fetched client-side per visit per locale

File: src/components/sections/Reviews.tsx:32-43 On every mount, the home page fires /api/reviews. No AbortController cleanup, no SSR. SSR would put the static fallback in HTML and only swap on the client when live data differs - the current pattern always shows static first, flashes on swap. Fix: Move the fetch to a server component (or getStaticProps-equivalent in App Router) and pass live data as a prop. Bonus: removes the client-side waterfall entirely.

P2-2. x-forwarded-for trusted on Worker route

File: src/app/api/lead/route.ts:309-312 Worker only ever sees cf-connecting-ip; the XFF fallback is for non-CF runtimes. Direct Worker hits with spoofed XFF bypass rate-limit by rotating fake IPs. Fix: Drop the XFF fallback. Use cf-connecting-ip or unknown constant.

P2-3. Resend SENDER hardcoded as sandbox address

File: src/app/api/lead/route.ts:78 "EL Fix Mobile <onboarding@resend.dev>" is Resend's sandbox - works in dev but Resend rejects production delivery to addresses outside your team. Once we set the prod RESEND_API_KEY, this needs to flip to a verified domain sender. Fix: const SENDER = process.env.RESEND_FROM ?? "EL Fix Mobile <onboarding@resend.dev>";.

P2-4. CF Web Analytics fires before cookie consent

File: src/app/[locale]/layout.tsx (analytics block) CF Web Analytics is cookie-free, but the Austrian DSB's TTDSG-equivalent reading still requires consent for any third-country fetch that could identify a user. Most sites ship CF WA without consent gating; legal exposure is real but minor. Fix: Either gate behind the CookieBanner accept state, or accept the risk and document.

P2-5. Reviews API browser-cache header misaligned with KV TTL

File: src/app/api/reviews/route.ts:128-133 Cached response sends cache-control: public, max-age=3600 while KV TTL is 24h. After a KV refresh, downstream browsers/CDN keep the stale snapshot for an hour, and fetched_at reads as fresh when it's actually a stale browser cache. Fix: Drop the public cache-control header (let KV be the cache) or align both windows.

P2-6. Review card aria-label hardcoded "5 von 5 Sternen"

File: src/components/sections/Reviews.tsx:146 Every card announces "5 von 5 Sternen" to screen readers regardless of actual rating. Static reviews are all 5-star; live Google reviews can be 4 or 3. Fix: aria-label={${review.rating} von 5 Sternen} when rating is available.

P2-7. HTML email line-splitter is fragile

File: src/app/api/lead/route.ts:101-135 (renderBookingEmail) The HTML rendering splits each line on ": " to derive label/value. Customer typing "Display: schwarzer Punkt" breaks the parser. The plain-text version works fine - layouts diverge. Fix: Pass [label, value] tuples explicitly instead of re-splitting joined strings.

P2-8. BreadcrumbList JSON-LD present, visible breadcrumb absent

Live observation: Model page has BreadcrumbList in JSON-LD but no <nav aria-label="Breadcrumb"> in DOM. Inconsistent for Google's enhanced SERP rules - they require visible AND structured breadcrumbs to qualify. Fix: Add a visible breadcrumb component above the H1 on brand/model pages.

P2-9. PII retention in KV: 90 days, plaintext

File: src/app/api/lead/route.ts:343-355 Full sanitized payload (name, phone, email, free-text damage description) stored 90 days. IP hashed but the body isn't. DSGVO Art. 5(1)(c) data-minimisation expects shorter retention or pseudonymisation of the message body. Fix: Either document the lawful basis (Vertragsanbahnung) or add a 30-day rotation that nulls the free-text message.

P2-10. No timeout on Telegram fetch

File: src/lib/telegram.ts:81-90 workerd default fetch has no client-side timeout. Telegram slow → entire /api/lead POST hangs to Worker CPU limit. The .catch() upstream swallows errors but not slow responses. Fix: fetch(url, { signal: AbortSignal.timeout(5000), ... }).

P3 - Nits, simplify, dead flexibility

  • P3-1: KvLike interface and getKv() helper duplicated in src/app/api/lead/route.ts and src/app/api/reviews/route.ts. Move to src/lib/kv.ts.
  • P3-2: CachedPayload / ReviewPublic in route + LiveReviewsPayload / LiveReview in component are near-duplicates. Export from one place.
  • P3-3: Reviews.tsx:120 - [...items, ...items, ...items, ...items] is 4x for an animation that needs 2x.
  • P3-4: Reviews.tsx:132 - whileHover={{ scale: 1 }} is a no-op.
  • P3-5: Reviews.tsx:75,84 - initial={{ opacity: 0.999 }} force-paint hack combined with amount: 0, margin: "200px" viewport is two redundant fire-immediately tricks layered.
  • P3-6: telegram.ts - siteUrl arg is always SITE.url; leadToken is always concatenated into a URL the caller could build once. Replace 3 args with a single statusUrl: string.
  • P3-7: lead/route.ts:199-256 - renderCustomerConfirmation takes isContactForm: boolean next to payload.type === "contact" discriminator. Drop the boolean.

Cross-cutting concerns

CC-1. No automated tests anywhere in the 5-commit range

Every commit relies on manual curl + "loaded the page." /api/lead Telegram extension, /api/reviews cache logic, and SimpleBookingForm URL-param parsing all have branches that only fire on rare states. No regression net. Recommendation: Add a tests/ harness with one happy-path test per route handler + one prefill test for the booking form. Vitest + miniflare is the workerd-friendly stack.

CC-2. Dead-code lag

BookingFlow.tsx lived for one commit after /buchen stopped pointing at it (commit 20aafd1 swapped the route, a901635 deleted the file). Practice: delete same-commit. Risk: a stash or rebase reintroduces the dead component.

CC-3. Body-text contrast inherited from semi-transparent token

The first <p> on the home page evaluates to oklab(0.999994 ... / 0.55) - white at 55% opacity. On dark hero it's intentional; on the light section below it would be near-invisible. Verify the same paragraph styling isn't reused on the light "Wir reparieren" cards (Section 2). If it is, that's a P1 contrast failure.

CC-4. i18n discipline solid, but ru/tr glosses unverified

The display/display_orig rename hit all four locales correctly. Worth a native-speaker pass on the new strings - "Стекло дисплея" / "Ekran camı" look right but only a translator confirms.

Recommended fix order (impact / effort)

  1. P0-2 AGB required (5 min, security + legal) - server schema + HTML attr
  2. P1-1, P1-2 Telegram escaping (~5 min, broken alerts)
  3. P1-6 Title de-dup (5 min, SEO bleed)
  4. P1-8 "iPhone 13in Wien" typo (2 min, embarrassment)
  5. P1-9 Mobile hamburger aria-label (5 min, a11y)
  6. P1-3 Zod leak (5 min, schema fingerprinting)
  7. P1-11 Lead 502 → 200 with warning (5 min, customer signal)
  8. P1-7 Damage prefill mapping (10 min, conversion)
  9. P1-10 Sub-44px tap targets (10 min, mobile UX)
  10. P0-1 Rate-limit DO (1h, security depth) - or accept and document
  11. P1-4 Reviews SWR (30 min, quota safety)
  12. P1-5 Places API migration (30 min, future-proof)

Total for items 1-9: about 50 minutes. Items 10-12: about 2 hours. Whole sprint fixable in a half-day.

What looks healthy

  • Schema.org markup is rich: Product / Service / FAQPage / BreadcrumbList on model pages, MobilePhoneStore in layout
  • Honeypot + dedup + rate-limit + Telegram fan-out + KV durability is a well-thought lead pipeline (mostly correct, even with the race-condition notes above)
  • Mobile bottom dock (Anrufen / WhatsApp / Reparatur buchen) is the right pattern for a phone-repair shop where calls convert
  • Phone CTA discoverability: 4 tel: links, 2 wa.me links, 1 mailto - good coverage
  • Hero parallax was correctly halved + useReducedMotion respected
  • i18n consistency across de/en/ru/tr is exemplary for the display/display_orig rename
  • BookingFlow → SimpleBookingForm was the right radical simplification