From 089276191ba2c9cabb4c948209bf436a34a97da2 Mon Sep 17 00:00:00 2001
From: galuis116 <galuis116@users.noreply.github.com>
Date: Thu, 11 Jun 2026 02:51:24 -0700
Subject: [PATCH] fix(ui): strip cookies from the analytics proxy request path

handleAnalyticsProxy stripped response set-cookie but forwarded the browser's first-party request cookies to the Umami upstream, breaking its documented cookieless/strips-cookies guarantee and leaking session/OAuth cookies to the analytics host. Add cookie and x-forwarded-for to the request strip-list and set x-forwarded-for to the trusted cf-connecting-ip only (no client spoofing). Add the first handleAnalyticsProxy unit test.
---
 apps/gittensory-ui/src/lib/analytics-proxy.ts | 15 +++--
 test/unit/analytics-proxy.test.ts             | 60 +++++++++++++++++++
 2 files changed, 69 insertions(+), 6 deletions(-)
 create mode 100644 test/unit/analytics-proxy.test.ts

diff --git a/apps/gittensory-ui/src/lib/analytics-proxy.ts b/apps/gittensory-ui/src/lib/analytics-proxy.ts
index f8e88f91e4..2fcb71b506 100644
--- a/apps/gittensory-ui/src/lib/analytics-proxy.ts
+++ b/apps/gittensory-ui/src/lib/analytics-proxy.ts
@@ -29,12 +29,16 @@ const STRIP_REQUEST_HEADERS = new Set([
   "keep-alive",
   "transfer-encoding",
   "upgrade",
+  // Cookieless analytics: never forward the visitor's first-party cookies upstream.
+  "cookie",
   "cf-connecting-ip",
   "cf-ipcountry",
   "cf-ray",
   "cf-visitor",
   "x-forwarded-host",
   "x-forwarded-proto",
+  // Re-derived below from the trusted cf-connecting-ip; never trust a client-supplied value.
+  "x-forwarded-for",
 ]);
 
 // Response headers we never relay back to the browser. content-encoding/-length
@@ -72,12 +76,11 @@ export async function handleAnalyticsProxy(request: Request): Promise<Response |
   request.headers.forEach((value, key) => {
     if (!STRIP_REQUEST_HEADERS.has(key.toLowerCase())) headers.set(key, value);
   });
-  // Preserve the real client IP so Umami geolocates the visitor, not the Worker.
+  // Preserve the real client IP so Umami geolocates the visitor, not the Worker. Set it to the
+  // trusted cf-connecting-ip only -- the client-supplied x-forwarded-for is stripped above so a
+  // visitor cannot spoof their geolocation.
   const clientIp = request.headers.get("cf-connecting-ip");
-  if (clientIp) {
-    const existing = request.headers.get("x-forwarded-for");
-    headers.set("x-forwarded-for", existing ? `${existing}, ${clientIp}` : clientIp);
-  }
+  if (clientIp) headers.set("x-forwarded-for", clientIp);
 
   const hasBody = request.method !== "GET" && request.method !== "HEAD";
 
@@ -87,7 +90,7 @@ export async function handleAnalyticsProxy(request: Request): Promise<Response |
       method: request.method,
       headers,
       // Buffer the (tiny) collect payload so we don't need a streaming/duplex body.
-      body: hasBody ? await request.arrayBuffer() : undefined,
+      body: hasBody ? await request.arrayBuffer() : null,
     });
   } catch {
     // Analytics must never take the page down — fail quietly.
diff --git a/test/unit/analytics-proxy.test.ts b/test/unit/analytics-proxy.test.ts
new file mode 100644
index 0000000000..d2d41ad85e
--- /dev/null
+++ b/test/unit/analytics-proxy.test.ts
@@ -0,0 +1,60 @@
+import { afterEach, describe, expect, it, vi } from "vitest";
+import { handleAnalyticsProxy } from "../../apps/gittensory-ui/src/lib/analytics-proxy";
+
+const UPSTREAM = "https://tasty.aethereal.dev";
+
+function captureUpstream() {
+  const calls: Array<{ url: string; init: RequestInit; headers: Headers }> = [];
+  vi.stubGlobal("fetch", async (url: RequestInfo | URL, init: RequestInit = {}) => {
+    calls.push({ url: url.toString(), init, headers: new Headers(init.headers) });
+    return new Response("ok", { status: 200, headers: { "set-cookie": "umami=1", "content-type": "application/javascript" } });
+  });
+  return calls;
+}
+
+describe("handleAnalyticsProxy", () => {
+  afterEach(() => {
+    vi.unstubAllGlobals();
+  });
+
+  it("does not forward the visitor's cookies to the analytics upstream", async () => {
+    const calls = captureUpstream();
+    const response = await handleAnalyticsProxy(
+      new Request("https://gittensory.aethereal.dev/stats/script.js", {
+        method: "GET",
+        headers: { cookie: "gittensory_session=secret; gh_oauth_state=abc", "cf-connecting-ip": "203.0.113.7" },
+      }),
+    );
+
+    expect(response?.status).toBe(200);
+    expect(calls).toHaveLength(1);
+    expect(calls[0]?.url).toBe(`${UPSTREAM}/script.js`);
+    // The first-party cookie must never reach the analytics host.
+    expect(calls[0]?.headers.has("cookie")).toBe(false);
+    // The upstream set-cookie must never be relayed back to the browser.
+    expect(response?.headers.has("set-cookie")).toBe(false);
+  });
+
+  it("forwards only the trusted client IP, ignoring a spoofed x-forwarded-for", async () => {
+    const calls = captureUpstream();
+    await handleAnalyticsProxy(
+      new Request("https://gittensory.aethereal.dev/stats/api/send", {
+        method: "POST",
+        headers: { "x-forwarded-for": "1.2.3.4", "cf-connecting-ip": "203.0.113.7", "content-type": "application/json" },
+        body: "{}",
+      }),
+    );
+
+    expect(calls).toHaveLength(1);
+    expect(calls[0]?.url).toBe(`${UPSTREAM}/api/send`);
+    expect(calls[0]?.headers.get("x-forwarded-for")).toBe("203.0.113.7");
+  });
+
+  it("returns undefined for non-allowlisted paths and 405 for disallowed methods", async () => {
+    captureUpstream();
+    expect(await handleAnalyticsProxy(new Request("https://gittensory.aethereal.dev/stats/api/admin"))).toBeUndefined();
+    expect(await handleAnalyticsProxy(new Request("https://gittensory.aethereal.dev/about"))).toBeUndefined();
+    const notAllowed = await handleAnalyticsProxy(new Request("https://gittensory.aethereal.dev/stats/script.js", { method: "POST" }));
+    expect(notAllowed?.status).toBe(405);
+  });
+});