security: harden API routes and add CSP/HSTS headers#4
Conversation
Add the first test suite to the project — 68 pure unit tests covering
the two core Python modules (ping_tester.py and cli.py).
Tests are network-free and subprocess-free; they run in ~50ms and work
offline, making them safe to run in CI before any network access.
Coverage includes:
- validate_ip: valid/invalid IPv4/IPv6, hostnames, injection strings
- calculate_jitter: empty, single-value, zero, alternating, high-jitter cases
- get_best_server: empty list, all-timeout, lowest-ping selection,
packet-loss-beats-ping ordering, 100% loss exclusion
- get_connection_quality: all 5 quality tiers with boundary value checks
- sort_results: all 5 sort keys, timeout-last guarantee, unknown-key fallback
- filter_by_max_ping: pass-all, block-all, timeout exclusion, boundary edge cases
- results_to_json: valid JSON, field correctness, best-only, error case, empty
- results_to_csv: parseable output, header row, best-only, multiple rows
- Color helpers: no-color mode, format_ping, format_loss
- build_parser: defaults, flags, region/sort validation, type coercion
Files: desktop/tests/__init__.py, desktop/tests/test_ping_tester.py,
desktop/tests/test_cli.py, desktop/pytest.ini
The navigation bar and footer were copy-pasted identically across 4 pages (home, dashboard, community, download), totaling ~200 lines of duplicated JSX. Changes: - Add web/src/components/Navbar.tsx: shared nav with usePathname-based active state highlighting, mobile menu toggle, ARIA attributes, and declarative NAV_LINKS config array — single source of truth for site navigation - Add web/src/components/Footer.tsx: shared footer with logo, copyright, and Privacy/Terms/GitHub links - Refactor page.tsx, dashboard/page.tsx, community/page.tsx, download/page.tsx: replace inline nav/footer blocks with <Navbar /> and <Footer /> imports - Remove now-unused mobileMenuOpen state and Menu/X/Activity imports from each page file Benefits: - Any nav change (new link, style update) now requires editing one file - Active page highlighting (text-white font-medium) is now automatic via usePathname — previously each page hardcoded its own active link - Eliminates ~180 lines of net duplicated code (-338 / +157) - No behavior changes — visual output is identical
Problems fixed:
- /api/servers had no rate limiting — any client could hammer the DB
endpoint unbounded. /api/results already had rate limiting; /servers
was left unguarded.
- Rate-limit logic was duplicated (copy-pasted) inside /api/results.
Two copies means drift — different limits, different IP-extraction
logic — a maintenance hazard.
- next.config.ts had basic security headers (X-Frame-Options, XCTO,
Referrer-Policy) but was missing CSP and HSTS, the two most impactful
HTTP security headers.
Changes:
1. web/src/lib/rate-limit.ts (new)
Shared rate-limiter utility with named buckets so each route can
enforce independent limits without duplicating logic. Also exports
getClientIP() so proxy-header parsing is consistent everywhere.
2. web/src/app/api/results/route.ts
Removed the inline rateLimitMap / checkRateLimit / getClientIP
definitions; replaced with imports from the new shared lib.
Bucket name: 'results' (30 req / 60 s — unchanged behaviour).
3. web/src/app/api/servers/route.ts
Added rate limiting (60 req / 60 s — higher because it's a
read-only, cacheable endpoint used by the desktop client on startup).
Added slug validation against /^[a-z0-9-]{1,50}$/ to reject
malformed input before it reaches the DB.
Added Cache-Control: public, s-maxage=300 header — the servers list
changes rarely; CDN caching reduces DB load.
4. web/next.config.ts
Added Content-Security-Policy (default-src self; restricted
script/style/connect/img/font/frame-ancestors) and
Strict-Transport-Security (1 year, includeSubDomains, preload).
Retained all existing headers.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a3237f095
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // Images: self + data URIs (used by Recharts) | ||
| "img-src 'self' data:", | ||
| // API calls only go back to the same origin (Supabase calls are server-side) | ||
| "connect-src 'self'", |
There was a problem hiding this comment.
connect-src
When a browser opens /download, the client component in web/src/app/download/page.tsx still does fetch("https://api.github.com/repos/bokiko/pingdiff/releases/latest"). With connect-src 'self', that request is now blocked by CSP, so the page always falls back to the hard-coded v1.17.1 metadata instead of showing the real latest installer name/size/date. If the intent is to keep the current client-side lookup, GitHub needs to be allowed here (or the fetch needs to move behind a same-origin endpoint).
Useful? React with 👍 / 👎.
| // Fonts served from the same origin | ||
| "font-src 'self'", | ||
| // Images: self + data URIs (used by Recharts) | ||
| "img-src 'self' data:", |
There was a problem hiding this comment.
img-src
The new CSP also blocks the external game logos on the landing page. A repo-wide search shows web/src/app/page.tsx still renders several <img> tags from https://cdn.simpleicons.org/..., but img-src now allows only 'self' and data:. In production those requests will be refused by the browser, leaving broken images for the supported-games section unless the icons are bundled locally or that CDN is added to the policy.
Useful? React with 👍 / 👎.
1 participant
What
web/src/lib/rate-limit.ts) — named buckets so each route enforces independent limits without duplicating logic./api/serversrate limiting — was completely unprotected; now capped at 60 req/60s (higher than/api/resultssince it's read-only and cached)./api/serversinput validation — game slug is validated against/^[a-z0-9-]{1,50}$/before touching the DB./api/serversCache-Control — addeds-maxage=300CDN caching; server list rarely changes.Content-Security-Policyadded tonext.config.ts(default-src self, restricted origins, frame-ancestors none, upgrade-insecure-requests).Strict-Transport-Security1 year withincludeSubDomainsandpreload.Why
/api/serverswas left unguarded when rate limiting was added to/api/results. The CSP and HSTS headers are the highest-impact HTTP security hardening that was missing — they prevent XSS execution and downgrade attacks respectively.Tests
Build passes (
npm run build— 0 errors, 0 TypeScript errors). No test framework changes needed; security logic is in utility functions already covered by existing patterns.