fix(worker): addresses PR review findings — security, perf, tests, docs

- adds Turnstile token length guard (>2048) with boundary tests
- adds seal key derivation cache (_sealKeyCache Map, bounded by VALID_PURPOSES)
- passes pre-parsed pathname to validateAndGuardProxyRoute (eliminates redundant URL parse)
- unifies session key cache into single Map (removes duplicate getSessionHmacPrevKey)
- fixes structured error logging in ensureSession catch path
- removes dead options parameter from validateProxyRequest
- corrects HKDF key material descriptions in CryptoEnv and DEPLOY.md
- fixes test key comments to match actual decoded values
- adds 14 new tests covering previously-untested paths

Author: wgordon17

DEPLOY.md

@@ -191,13 +191,13 @@ openssl rand -base64 32  # Run once per key below
 ### Setting secrets
 
 ```bash
-wrangler secret put SESSION_KEY           # HMAC key for session cookies
-wrangler secret put SEAL_KEY              # AES-256-GCM key for sealed tokens
+wrangler secret put SESSION_KEY           # HKDF input key material for session cookies
+wrangler secret put SEAL_KEY              # HKDF input key material for sealed tokens
 wrangler secret put TURNSTILE_SECRET_KEY  # From Cloudflare Turnstile dashboard
 ```
 
-- `SESSION_KEY`: HMAC-SHA256 key used to sign `__Host-session` cookies. Generate with `openssl rand -base64 32`.
-- `SEAL_KEY`: AES-256-GCM key used to encrypt Jira/GitLab API tokens stored client-side as sealed blobs. Generate with `openssl rand -base64 32`.
+- `SESSION_KEY`: HKDF input key material used to derive the HMAC-SHA256 key for signing `__Host-session` cookies. Generate with `openssl rand -base64 32`.
+- `SEAL_KEY`: HKDF input key material used to derive the AES-256-GCM key for encrypting API tokens stored client-side as sealed blobs. Generate with `openssl rand -base64 32`.
 - `TURNSTILE_SECRET_KEY`: From the Cloudflare Turnstile dashboard (Security → Turnstile → your widget → Secret key).
 - `VITE_TURNSTILE_SITE_KEY`: **Build-time env var (public)** — goes in `.env`, not a Worker secret. From the same Turnstile dashboard (Site key).
 

src/worker/crypto.ts

@@ -1,6 +1,6 @@
 export interface CryptoEnv {
-  SEAL_KEY: string; // base64-encoded 32-byte AES-256-GCM key
-  SEAL_KEY_PREV?: string; // previous key for rotation
+  SEAL_KEY: string; // base64-encoded HKDF input key material (32 bytes recommended)
+  SEAL_KEY_PREV?: string; // previous HKDF key material for rotation
 }
 
 // ── Base64url utilities ────────────────────────────────────────────────────

src/worker/index.ts

@@ -152,10 +152,7 @@ function isProxyPath(pathname: string): boolean {
 
 // ── Validation gate for proxy routes ─────────────────────────────────────────
 // Returns a Response if rejected, null if validation passes.
-function validateAndGuardProxyRoute(request: Request, env: Env): Response | null {
-  const url = new URL(request.url);
-  const pathname = url.pathname;
-
+function validateAndGuardProxyRoute(request: Request, env: Env, pathname: string): Response | null {
   if (!isProxyPath(pathname)) return null;
 
   const origin = request.headers.get("Origin");
@@ -187,18 +184,26 @@ function validateAndGuardProxyRoute(request: Request, env: Env): Response | null
 // ── Sealed-token endpoint ────────────────────────────────────────────────────
 const VALID_PURPOSES = new Set(["jira-api-token", "jira-refresh-token", "gitlab-pat"]);
 
+// Module-level cache for derived seal keys, keyed by "<raw>:<purpose>".
+// SEAL_KEY is a deployment constant — safe to cache per-isolate (follows _sessionKeyCache pattern).
+const _sealKeyCache = new Map<string, CryptoKey>();
+
 async function handleProxySeal(request: Request, env: Env, sessionId: string): Promise<Response> {
   if (request.method !== "POST") {
     return errorResponse("method_not_allowed", 405);
   }
 
   // Session + rate limiting (done by caller, sessionId passed in)
-  // Extract Turnstile token and verify (Step 6)
+  // Extract Turnstile token and verify
   const turnstileToken = extractTurnstileToken(request);
   if (!turnstileToken) {
     log("warn", "seal_turnstile_missing", {}, request);
     return errorResponse("turnstile_failed", 403);
   }
+  if (turnstileToken.length > 2048) {
+    log("warn", "seal_turnstile_token_too_long", { token_length: turnstileToken.length }, request);
+    return errorResponse("turnstile_failed", 403);
+  }
 
   const ip = request.headers.get("CF-Connecting-IP");
   const turnstileResult = await verifyTurnstile(turnstileToken, ip, env);
@@ -237,8 +242,13 @@ async function handleProxySeal(request: Request, env: Env, sessionId: string): P
 
   let sealed: string;
   try {
-    // SC-8: derive key with purpose-scoped info string
-    const key = await deriveKey(env.SEAL_KEY, SEAL_SALT, "aes-gcm-key:" + purpose, "encrypt");
+    // SC-8: derive key with purpose-scoped info string (cached per-isolate, bounded by VALID_PURPOSES size)
+    const cacheKey = env.SEAL_KEY + ":" + purpose;
+    let key = _sealKeyCache.get(cacheKey);
+    if (key === undefined) {
+      key = await deriveKey(env.SEAL_KEY, SEAL_SALT, "aes-gcm-key:" + purpose, "encrypt");
+      _sealKeyCache.set(cacheKey, key);
+    }
     sealed = await sealToken(token, key);
   } catch (err) {
     // SC-9: log error server-side but DO NOT include crypto error in response
@@ -696,7 +706,7 @@ export default {
     // ── Proxy routes: validation, session, and rate limiting ─────────────────
     // Applies to /api/proxy/*, /api/jira/*, /api/gitlab/*
     // validateAndGuardProxyRoute handles OPTIONS preflight for proxy routes.
-    const guardResponse = validateAndGuardProxyRoute(request, env);
+    const guardResponse = validateAndGuardProxyRoute(request, env, url.pathname);
     if (guardResponse !== null) return guardResponse;
 
     if (isProxyPath(url.pathname)) {

src/worker/session.ts

@@ -33,20 +33,14 @@ const SESSION_MAX_AGE = 28800; // 8 hours in seconds
 
 // Module-level cache for derived session HMAC keys.
 // SESSION_KEY is a deployment constant — safe to cache per-isolate (follows _dsnCache pattern).
-let _sessionKeyCache: { raw: string; key: CryptoKey } | undefined;
-let _sessionKeyPrevCache: { raw: string; key: CryptoKey } | undefined;
+// Keyed by raw secret string; supports both current and previous key without duplicate logic.
+const _sessionKeyCache = new Map<string, CryptoKey>();
 
 async function getSessionHmacKey(raw: string): Promise<CryptoKey> {
-  if (_sessionKeyCache?.raw === raw) return _sessionKeyCache.key;
+  const cached = _sessionKeyCache.get(raw);
+  if (cached !== undefined) return cached;
   const key = await deriveKey(raw, SESSION_HMAC_SALT, SESSION_HMAC_INFO, "sign");
-  _sessionKeyCache = { raw, key };
-  return key;
-}
-
-async function getSessionHmacPrevKey(raw: string): Promise<CryptoKey> {
-  if (_sessionKeyPrevCache?.raw === raw) return _sessionKeyPrevCache.key;
-  const key = await deriveKey(raw, SESSION_HMAC_SALT, SESSION_HMAC_INFO, "sign");
-  _sessionKeyPrevCache = { raw, key };
+  _sessionKeyCache.set(raw, key);
   return key;
 }
 
@@ -116,7 +110,7 @@ export async function parseSession(
     const currentKey = await getSessionHmacKey(env.SESSION_KEY);
     let valid = await verifySession(json, signature, currentKey);
     if (!valid && env.SESSION_KEY_PREV !== undefined) {
-      const prevKey = await getSessionHmacPrevKey(env.SESSION_KEY_PREV);
+      const prevKey = await getSessionHmacKey(env.SESSION_KEY_PREV);
       valid = await verifySession(json, signature, prevKey);
     }
     if (!valid) return null;
@@ -150,7 +144,11 @@ export async function ensureSession(
     const { cookie, sessionId } = await issueSession(env);
     return { sessionId, setCookie: cookie };
   } catch (error) {
-    console.error("session_issue_failed", error);
+    console.error(JSON.stringify({
+      worker: "github-tracker",
+      event: "session_issue_failed",
+      error: error instanceof Error ? error.message : "unknown",
+    }));
     return { sessionId: crypto.randomUUID() };
   }
 }

src/worker/validation.ts

@@ -65,8 +65,7 @@ const METHODS_REQUIRING_CONTENT_TYPE = new Set(["POST", "PUT", "PATCH"]);
  */
 export function validateProxyRequest(
   request: Request,
-  allowedOrigin: string,
-  options?: { expectedContentType?: string }
+  allowedOrigin: string
 ): ValidationResult {
   const originResult = validateOrigin(request, allowedOrigin);
   if (!originResult.ok) return originResult;
@@ -78,8 +77,7 @@ export function validateProxyRequest(
   if (!customHeaderResult.ok) return customHeaderResult;
 
   if (METHODS_REQUIRING_CONTENT_TYPE.has(request.method)) {
-    const expected = options?.expectedContentType ?? "application/json";
-    const contentTypeResult = validateContentType(request, expected);
+    const contentTypeResult = validateContentType(request, "application/json");
     if (!contentTypeResult.ok) return contentTypeResult;
   }
 

tests/app/lib/proxy.test.ts

@@ -22,16 +22,24 @@ interface MockTurnstile {
   _resolveToken(token: string): void;
   /** Trigger the error callback for the most-recently rendered widget. */
   _rejectWithError(code: string): void;
+  /** Trigger the expired-callback for the most-recently rendered widget. */
+  _triggerExpired(): void;
 }
 
 function makeMockTurnstile(): MockTurnstile {
   let _successCb: ((token: string) => void) | undefined;
   let _errorCb: ((code: string) => void) | undefined;
+  let _expiredCb: (() => void) | undefined;
 
   const mock: MockTurnstile = {
-    render: vi.fn((_container: HTMLElement, options: { callback?: (token: string) => void; "error-callback"?: (code: string) => void }) => {
+    render: vi.fn((_container: HTMLElement, options: {
+      callback?: (token: string) => void;
+      "error-callback"?: (code: string) => void;
+      "expired-callback"?: () => void;
+    }) => {
       _successCb = options.callback;
       _errorCb = options["error-callback"];
+      _expiredCb = options["expired-callback"];
       return "widget-id-1";
     }),
     execute: vi.fn(),
@@ -43,6 +51,9 @@ function makeMockTurnstile(): MockTurnstile {
     _rejectWithError(code: string) {
       _errorCb?.(code);
     },
+    _triggerExpired() {
+      _expiredCb?.();
+    },
   };
 
   return mock;
@@ -115,6 +126,21 @@ describe("proxyFetch", () => {
     expect(headers["cf-turnstile-response"]).toBe("tok123");
   });
 
+  it("merges Headers instance caller headers without dropping defaults", async () => {
+    const mockFetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 }));
+    vi.stubGlobal("fetch", mockFetch);
+
+    await mod.proxyFetch("/api/proxy/seal", {
+      headers: new Headers({ "cf-turnstile-response": "tok" }),
+    });
+
+    const [, init] = mockFetch.mock.calls[0] as [string, RequestInit];
+    const headers = init.headers as Record<string, string>;
+    expect(headers["X-Requested-With"]).toBe("fetch");
+    expect(headers["Content-Type"]).toBe("application/json");
+    expect(headers["cf-turnstile-response"]).toBe("tok");
+  });
+
   it("passes the path to fetch unchanged", async () => {
     const mockFetch = vi.fn().mockResolvedValue(new Response(null, { status: 200 }));
     vi.stubGlobal("fetch", mockFetch);
@@ -229,6 +255,47 @@ describe("acquireTurnstileToken", () => {
 
     await expect(tokenPromise).rejects.toThrow("Turnstile error: invalid-input-response");
   });
+
+  it("rejects when Turnstile fires expired-callback", async () => {
+    const mockTurnstile = makeMockTurnstile();
+    vi.stubGlobal("window", {
+      ...window,
+      turnstile: mockTurnstile,
+    });
+
+    vi.spyOn(document.head, "appendChild").mockImplementation((node) => {
+      const el = node as HTMLScriptElement;
+      if (el.tagName === "SCRIPT") {
+        (el as unknown as { onload: (() => void) | null }).onload?.();
+        return node;
+      }
+      return node;
+    });
+
+    const tokenPromise = mod.acquireTurnstileToken("test-site-key");
+
+    await Promise.resolve();
+    await Promise.resolve();
+
+    mockTurnstile._triggerExpired();
+
+    await expect(tokenPromise).rejects.toThrow("Turnstile token expired before submission");
+  });
+
+  it("rejects when the Turnstile script fails to load (onerror)", async () => {
+    vi.spyOn(document.head, "appendChild").mockImplementation((node) => {
+      const el = node as HTMLScriptElement;
+      if (el.tagName === "SCRIPT") {
+        (el as unknown as { onerror: (() => void) | null }).onerror?.();
+        return node;
+      }
+      return node;
+    });
+
+    await expect(mod.acquireTurnstileToken("test-site-key")).rejects.toThrow(
+      "Failed to load Turnstile script",
+    );
+  });
 });
 
 // ── sealApiToken tests ────────────────────────────────────────────────────────
@@ -306,10 +373,9 @@ describe("sealApiToken", () => {
     );
     vi.stubGlobal("fetch", mockFetch);
 
-    await expect(mod.sealApiToken("my-token", "jira-api-token")).rejects.toMatchObject({
-      status: 429,
-      message: "rate_limited",
-    });
+    const err = await mod.sealApiToken("my-token", "jira-api-token").catch((e: unknown) => e);
+    expect(err).toBeInstanceOf(mod.SealError);
+    expect(err).toMatchObject({ status: 429, message: "rate_limited" });
   });
 
   it("throws SealError on 500 response", async () => {
@@ -320,10 +386,9 @@ describe("sealApiToken", () => {
     );
     vi.stubGlobal("fetch", mockFetch);
 
-    await expect(mod.sealApiToken("my-token", "jira-api-token")).rejects.toMatchObject({
-      status: 500,
-      message: "seal_failed",
-    });
+    const err = await mod.sealApiToken("my-token", "jira-api-token").catch((e: unknown) => e);
+    expect(err).toBeInstanceOf(mod.SealError);
+    expect(err).toMatchObject({ status: 500, message: "seal_failed" });
   });
 
   it("rejects when fetch throws a network error", async () => {
@@ -335,6 +400,19 @@ describe("sealApiToken", () => {
     await expect(mod.sealApiToken("my-token", "jira-api-token")).rejects.toThrow("Failed to fetch");
   });
 
+  it("throws SealError with 'unknown_error' when error response body is non-JSON", async () => {
+    setupMockedTurnstile("turnstile-tok");
+
+    const mockFetch = vi.fn().mockResolvedValue(
+      new Response("Service Unavailable", { status: 503 }),
+    );
+    vi.stubGlobal("fetch", mockFetch);
+
+    const err = await mod.sealApiToken("my-token", "jira-api-token").catch((e: unknown) => e);
+    expect(err).toBeInstanceOf(mod.SealError);
+    expect(err).toMatchObject({ status: 503, message: "unknown_error" });
+  });
+
   it("includes cf-turnstile-response header in POST body", async () => {
     setupMockedTurnstile("expected-turnstile-token");