Skip to content

Security audit — 2026-06-24 #73

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
versila22 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Security audit — 2026-06-24 #73

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 90 additions & 0 deletions security/pentest-2026-06-24.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
# Pentest mini-report — lima-app — 2026-06-24

**Probed URL:** https://limaimpro.duckdns.org/
**Stack:** React 18 + Vite 7 / npm / PWA=yes (vite-plugin-pwa, autoUpdate, SW workbox)
**Backend:** FastAPI (Python) on Railway — https://api-production-e15b.up.railway.app
**Counts:** Critical=1 High=3 Medium=4 Low=3 Info=2

## Findings

| Sev | Cat | Title | Location |
|---|---|---|---|
| Critical | SAST | Hardcoded admin credentials in production source | `backend/app/main.py:54` |
| High | SAST | Stale debug files tracked in git (`.env.production`, `api.ts.orig`, `api_upload.patch`) | `src/lib/api.ts.orig`, `.env.production` |
| High | SAST | JWT token stored in `sessionStorage` (XSS-accessible) | `src/lib/api.ts:45,53` |
| High | SAST | Unauthenticated `/health/db` leaks DB URL prefix and full table list | `backend/app/main.py:202-222` |
| Medium | SAST | `FRONTEND_URL` default is stale third-party domain auto-injected into CORS_ORIGINS | `backend/app/config.py:37,85-87` |
| Medium | Infra | TLS 1.2 enabled — TLS 1.3 preferred, 1.2 still accepted | `limaimpro.duckdns.org:443` |
| Medium | Deps | `vitest <3.2.6` — arbitrary file read/exec via UI server (GHSA-5xrq-8626-4rwp, CVSS 9.8) | `package.json` devDependency |
| Medium | Deps | `react-router-dom <6.30.4` — open redirect via `//` prefix paths (GHSA-2j2x-hqr9-3h42) | `package.json` |
| Low | SAST | `/health/migrations` leaks full Alembic traceback on error (unauthenticated) | `backend/app/main.py:225-240` |
| Low | Deps | `serialize-javascript <=7.0.2` — RCE via RegExp/Date during build (GHSA-5c6j-r48x-rmvq, High) | `package.json` devDependency |
| Low | Infra | `robots.txt` allows all crawlers on a members-only app | `public/robots.txt` |
| Info | DAST | Security headers confirmed present in nginx.conf (CSP, HSTS, nosniff, X-Frame-Options) | `nginx.conf` |
| Info | PWA | No VAPID private key in client code; SW uses NetworkFirst for API, no auth-route caching risk | `vite.config.ts` workbox config |

## Top 3 fixes

1. **Hardcoded admin credentials** — Remove `_SEED_MEMBERS` passwords from source; load seed credentials from environment variables or a secrets manager, and enforce a must-change-on-first-login flow.
2. **Unauthenticated `/health/db` and `/health/migrations`** — Add `Depends(require_admin)` to both endpoints, or restrict to internal network only via Traefik middleware.
3. **Stale debug files in git** — `git rm src/lib/api.ts.orig src/lib/api_upload.patch`; evaluate whether `.env.production` (contains Railway API URL only, no secrets) should remain tracked or be gitignored.

## Evidence (Critical/High only)

### Critical — Hardcoded admin credentials in source
- **Location:** `backend/app/main.py:54`
- **Snippet:** `{"email": "admin@lima-impro.fr", ..., "password": "Admin1234!", "app_role": "admin", ...}`
- **Impact:** Anyone with read access to the repository knows the admin account password. If the seed ran against production DB and the password was never changed, the admin account is fully compromised.
- **Fix:** Move seed credentials to environment variables (`SEED_ADMIN_PASSWORD`), add a `must_change_password` flag, or remove the seed entirely from production startup.

### High — Stale debug files tracked in git
- **Location:** `src/lib/api.ts.orig`, `src/lib/api_upload.patch`, `.env.production`, `.env.development`
- **Snippet:** `git ls-files` returns all four files as tracked.
- **Impact:** `api.ts.orig` contains the old `localStorage`-based token implementation (line 25: `localStorage.getItem(TOKEN_KEY)`) — a weaker auth pattern. `api_upload.patch` is a stray diff. Both increase attack surface understanding for an adversary who clones the repo. `.env.production` contains the Railway API base URL (no secrets), but establishing the precedent of committing `.env.*` files is dangerous.
- **Fix:** `git rm src/lib/api.ts.orig src/lib/api_upload.patch`; add `*.orig` and `*.patch` to `.gitignore`; confirm `.env.production` and `.env.development` are intended to be tracked (they currently contain no secrets but the pattern is risky).

### High — JWT token in sessionStorage (XSS-accessible)
- **Location:** `src/lib/api.ts:42-53`, `src/contexts/AuthContext.tsx:74`
- **Snippet:** `sessionStorage.setItem(_SESSION_KEY, token)` — fallback for Safari cross-origin cookie ITP.
- **Impact:** Any XSS vulnerability (e.g., in a third-party dependency) can extract the access token from sessionStorage, bypassing the httpOnly cookie protection entirely. The fallback is active whenever Safari's ITP blocks cookies.
- **Fix:** Consider using a same-site backend reverse proxy to serve both front and API from the same origin, eliminating the need for the sessionStorage fallback; or document and accept the risk with a strict CSP that prevents inline scripts.

### High — Unauthenticated `/health/db` info disclosure
- **Location:** `backend/app/main.py:202-222`
- **Snippet:** `return {"status": "ok", "async_url_prefix": settings.async_database_url[:60], "tables": tables}`
- **Impact:** Returns the first 60 chars of the database connection URL (may include hostname, port, db name, and partial credentials) plus a full list of table names to any unauthenticated caller.
- **Fix:** Add `_: Member = Depends(require_admin)` to `health_check_db` and `health_check_migrations`, or gate behind an internal-only Traefik middleware.

## Verified safe

- **CORS configuration:** `allow_credentials=True` paired with an explicit allowlist (`settings.CORS_ORIGINS`), not a wildcard — FastAPI correctly refuses to reflect arbitrary origins with credentials.
- **Login rate limiting:** `@limiter.limit("5/minute")` on `/auth/login` and `/auth/activate`.
- **httpOnly auth cookies:** Primary token delivery is via httpOnly, Secure, SameSite=None cookies (for cross-origin); sessionStorage is a documented fallback.
- **CSP (nginx.conf):** No `unsafe-eval`; `script-src 'self'` only; `frame-ancestors 'none'`; `object-src 'none'`.
- **HSTS:** `max-age=31536000; includeSubDomains` set in nginx.
- **X-Content-Type-Options:** `nosniff` present.
- **X-Frame-Options:** `DENY` present.
- **No VAPID private key in client code.**
- **No `eval()` or `Function()` in source.**
- **No postMessage usage in source.**
- **dangerouslySetInnerHTML in chart.tsx:** Content is generated entirely from static developer-controlled color config strings (CSS custom property values) — not from user input. Risk is negligible.
- **ReactMarkdown in PlanPreview.tsx:** Uses `remark-gfm` only, no `rehype-raw` — HTML tags in markdown are escaped, not rendered.
- **TLS:** TLS 1.0 and 1.1 disabled (openssl reports "no protocols available"); TLS 1.3 negotiated by default.
- **docs/redoc:** Disabled in production (`docs_url=None` when `APP_ENV != "development"`).
- **Password reset:** Always returns 200 (no email enumeration).
- **No open redirect exploitable in app code:** `navigate()` calls use hardcoded paths, not user-controlled URL parameters.

## Needs server-side verification

- **Actual HTTP response headers** from `https://limaimpro.duckdns.org/` — egress proxy policy denied direct HTTP access; headers were inferred from `nginx.conf`. Verify with `curl -sIL https://limaimpro.duckdns.org/` from a non-sandboxed environment.
- **CORS reflection test** on `https://api-production-e15b.up.railway.app` with `Origin: https://attacker.example` — confirm no reflection + credentials.
- **`/health/db` and `/health/migrations` on Railway API** — confirm they are reachable without auth from the public internet.
- **Seed admin password rotation** — confirm `admin@lima-impro.fr` password has been changed from `Admin1234!` in production DB.
- **`FRONTEND_URL` in Railway env** — confirm it is overridden from `https://improv-cabaret-planner.lovable.app` to `https://limaimpro.duckdns.org` in Railway environment variables to prevent the stale Lovable domain from appearing in CORS_ORIGINS.
- **Cookie flags in live traffic** — verify `Secure` and `SameSite` flags are set correctly on `access_token` and `refresh_token` cookies in production responses.
- **Sensitive file exposure** (`.env`, `.git/config`, `backup.zip`) — network-level check blocked by egress policy; run manually.
- **TLS cipher suite audit** — only TLSv1.3 + `TLS_AES_256_GCM_SHA384` verified; full cipher list (especially for TLS 1.2 fallback) needs `nmap --script ssl-enum-ciphers` from outside the sandbox.

## Tools

ran=npm-audit, openssl-s_client, git-ls-files, grep-secret-scan; skipped=curl-http-headers (egress proxy policy denial — host not in allowlist), curl-cors-test (same), curl-sensitive-files (same), nmap (not installed)
Loading