Skip to content

fix(analytics): make Umami opt-in, drop visitor-IP event (OWASP A06) - #50

Merged
carbon-evolution merged 1 commit into
mainfrom
fix/analytics-opt-in
Oct 18, 2026
Merged

carbon-evolution merged 1 commit into
mainfrom
fix/analytics-opt-in

Conversation

@carbon-evolution

@carbon-evolution carbon-evolution commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

Fixes the OWASP A06 finding in SECURITY.md: src/middleware.ts sent every page view, plus a custom "Network Log" event containing the visitor's IP, to a hardcoded http://umami-umami-1:3000 (upstream's Umami container) with upstream's website ID as the default.

What changed

  • Off by default. Nothing is sent unless the operator sets both UMAMI_URL (base URL, posts to <UMAMI_URL>/api/send) and UMAMI_WEBSITE_ID. If either is missing, or UMAMI_URL isn't a valid http(s) URL, the middleware returns NextResponse.next() right away and makes no outbound request. There is no hardcoded host and no default site ID. UMAMI_WEBSITE_ID was already the name in use; no other Umami variables existed in the README, DOCKER.md or .env.example.
  • I removed the visitor-IP "Network Log" event instead of putting it behind an opt-in flag. It stored the raw IP as plaintext custom event data. Umami doesn't need that: it works out geo and unique visitors from the request headers on its side. An opt-in switch for raw-IP logging would just be a way to turn the privacy problem back on, and no OSIRIS feature uses the event.
  • Header forwarding. The client IP (first entry of cf-connecting-ip / x-forwarded-for) is still forwarded, but only as the X-Forwarded-For header and never in the payload. Without it, Umami would record every visitor as the OSIRIS server, so geo stats and visitor counts would be meaningless. User-Agent is forwarded for the same reason.
  • Non-blocking, fail-silent. Still uses event.waitUntil(...). The fetch now has a 2s AbortSignal.timeout. Rejections are swallowed, and a synchronous throw is caught too, so analytics can't break a page load.
  • docker-compose.yml: removed the external: true umami_default network. With it, docker compose up fails on any host that doesn't already have upstream's Umami stack. Operators who run Umami in another Compose project can attach its network with an override (noted in .env.example).
  • .env.example: documents both variables, plus a privacy note on what is sent when analytics is enabled. SECURITY.md: A06 analytics bullet updated.

Tests

New file src/middleware.test.ts (7 tests). It stubs global fetch, uses a real NextRequest and a fake NextFetchEvent, and covers:

  • no env set: no fetch and no waitUntil
  • only one of the two variables set: no fetch
  • invalid URL: no fetch
  • configured: exactly one page view goes to the configured URL with the configured site ID; no IP in the payload; IP sent as an XFF header; abort signal present
  • no "Network Log" / IP event is sent, even with UMAMI_TRACK_IP=true
  • fetch rejection doesn't throw
  • a synchronous fetch throw doesn't throw

All 7 fail against the old middleware. On the rejection test, the old code also swallowed the error; that test fails there only because the old code made 2 calls instead of 1.

Verification

  • tsc --noEmit 0 · eslint . 0 errors · vitest 73/73 · npm run build OK
  • Smoke test on a production build (next start -p 3020, a listener on :3029 logging requests):
    • analytics unset: GET / returned 200 and the listener got 0 requests
    • UMAMI_URL=http://127.0.0.1:3029 UMAMI_WEBSITE_ID=smoke-site-1: GET / returned 200 and the listener got exactly 1 POST /api/send with payload {hostname,language,referrer,screen,title,url:"/",website:"smoke-site-1"} and header x-forwarded-for: 203.0.113.7
    • analytics configured but the endpoint down: GET / still returned 200

🤖 Generated with Claude Code

The middleware posted every page view, plus a custom "Network Log" event
carrying the visitor's IP, to a hardcoded http://umami-umami-1:3000 with
the upstream author's website id as a default.

- Off by default: nothing is sent unless UMAMI_URL and UMAMI_WEBSITE_ID
  are both set; otherwise return NextResponse.next() with no fetch.
- No hardcoded host or default website id; invalid/non-http URL disables.
- Remove the "Network Log" IP event. The client IP is forwarded only as
  X-Forwarded-For so Umami can do its own geo lookup.
- 2s timeout on the analytics fetch; failures are swallowed, still via
  event.waitUntil so page loads never wait on analytics.
- docker-compose: drop the external umami_default network, which made
  `docker compose up` fail on hosts without upstream's Umami stack.
- Document the variables and what is sent in .env.example; update
  SECURITY.md A06. Add middleware unit tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@carbon-evolution
carbon-evolution merged commit 0385471 into main Oct 18, 2026
8 checks passed
@carbon-evolution
carbon-evolution deleted the fix/analytics-opt-in branch October 18, 2026 04:34
carbon-evolution added a commit that referenced this pull request Oct 18, 2026
- A01 Broken Access Control: Medium -> Low. Recon scanner closed by
  default (token / explicit opt-in), client IP from forwarding headers
  only with TRUST_PROXY_HEADERS (#51).
- A06 Insecure Design: Medium -> Low. Analytics opt-in, visitor-IP
  event removed (#50); recon no longer anonymous by default.
- A02 Security Misconfiguration: stays Medium (improved). Hardening
  headers and a tighter CSP without unsafe-eval (#52); ZAP on main now
  0 High / 3 Medium / 1 Low / 4 Info (was 0/5/5/5). Remaining Medium
  items are script/style 'unsafe-inline' and wildcard media sources.
- A07: scanner access token compared in constant time.
- Overall: 2 Medium, 8 Low (was 4 Medium, 6 Low); Scorecard 8.0.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant