Skip to content

security(hardening): pentest follow-ups — webhook registration DNS validation, forecast storage-key disclosure, VITE_ secret guard (P3) #5213

Description

@koala73

Summary

Low-priority security hardening follow-ups from a full pentest pass (static + deep/SAST + live authenticated DAST + free-tier session sweep) against origin/main. None of these are exploitable vulnerabilities — the real finding from that pass (entitlement bypass) was #5208, fixed in #5210. These are defense-in-depth / hygiene improvements, filed together to keep them tracked without cluttering the board.

Priority: P3.


1. Webhook URL validation is hostname-only at registration (fail-fast + defense-in-depth gap)

The registration-time validators check the hostname string only; they do not resolve DNS:

  • api/_notification-webhook-ssrf.tsisBlockedNotificationResolvedAddress matches IP literals only; a DNS name that resolves to a private/metadata IP passes registration.
  • server/worldmonitor/shipping/v2/webhook-shared.tsisBlockedCallbackUrl, same shape (POST /api/v2/shipping/webhooks).

Not exploitable — the delivery path already closes it correctly: scripts/lib/notification-webhook-ssrf.cjs::assertNotificationWebhookDeliveryUrlSafe (DNS-resolve + reject private/reserved) + postJsonWithPinnedAddress (socket-pin), and server/worldmonitor/shipping/v2/deliver-webhook.ts:152,167 (same). A hostname rebinding to 169.254.169.254 is rejected before any request goes out.

Why hardening is still worth it: (a) a bad webhook is accepted at set-time and only fails silently at delivery — poor UX; (b) all protection rests on the single delivery control. Mirror the delivery-time DNS-resolution + private-IP block into the registration validators for fail-fast + a second layer.

2. Internal storage object keys returned in forecast API responses

GET /api/forecast/v1/get-simulation-package and .../get-simulation-outcome include the raw storage key in the JSON, e.g. "pkgKey":"seed-data/forecast-traces/2026/07/11/<runId>/simulation-package.json" (handlers under server/worldmonitor/forecast/v1/).

Minor information disclosure of the internal R2 path/naming scheme (global forecast data, not per-user; only exploitable if the bucket is publicly readable — it isn't). Consider omitting pkgKey/outcomeKey from the public response shape (they're internal pointers).

3. Preventive: CI guard against VITE_-prefixed secrets

Three VITE_-prefixed secret vars existed in a local .env.local (VITE_AISSTREAM_API_KEY, VITE_CLOUDFLARE_API_TOKEN, and an empty VITE_ACLED_ACCESS_TOKEN). Nothing was shipping — the current prod bundle is clean and none are referenced via import.meta.env in client code — but Vite inlines any VITE_-prefixed var into the client bundle, so this is a standing footgun. A small CI lint that flags secret-looking VITE_* names (in .env.example and any committed env, and warns in local dev) would prevent a future npm run build from ever bundling one. See the existing envPrefix behavior notes.


Found via Strix (usestrix/strix) + manual verification. Each item verified against current origin/main; the "not exploitable" notes above were confirmed by reading the delivery paths / response shapes, not assumed.

Metadata

Metadata

Assignees

No one assigned

    Labels

    securitySecurity-related

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions