diff --git a/deno.lock b/deno.lock index af908c5..7addef8 100644 --- a/deno.lock +++ b/deno.lock @@ -499,6 +499,7 @@ "https://deno.land/x/postgres@v0.17.0/query/oid.ts": "8c33e1325f34e4ca9f11a48b8066c8cfcace5f64bc1eb17ad7247af4936999e1", "https://deno.land/x/postgres@v0.17.0/query/query.ts": "edb473cbcfeff2ee1c631272afb25d079d06b66b5853f42492725b03ffa742b6", "https://deno.land/x/postgres@v0.17.0/query/transaction.ts": "8e75c3ce0aca97da7fe126e68f8e6c08d640e5c8d2016e62cee5c254bebe7fe8", + "https://deno.land/x/postgres@v0.17.0/query/types.ts": "a6dc8024867fe7ccb0ba4b4fa403ee5d474c7742174128c8e689c3b5e5eaa933", "https://deno.land/x/postgres@v0.17.0/utils/deferred.ts": "dd94f2a57355355c47812b061a51b55263f72d24e9cb3fdb474c7519f4d61083", "https://deno.land/x/postgres@v0.17.0/utils/utils.ts": "19c3527ddd5c6c4c49ae36397120274c7f41f9d3cbf479cb36065d23329e9f90" }, diff --git a/scratch/INVESTIGATIVE_REPORT.md b/scratch/INVESTIGATIVE_REPORT.md index c521129..c6dd1e3 100644 --- a/scratch/INVESTIGATIVE_REPORT.md +++ b/scratch/INVESTIGATIVE_REPORT.md @@ -3,51 +3,102 @@ ## 1. Cookie Scoping & Chromium Cookie Jar Mechanics ### Findings: -Chromium on Android handles `Domain=.atyg.org` (wildcard cookies) correctly according to RFC 6265, but there is a well-documented race condition / overriding issue when both a host-only cookie (no Domain attribute) and a wildcard domain cookie exist for the same name. -If a user previously had a `session_id` set exclusively for `auth.atyg.org` (perhaps during a test, or a misconfigured previous version), and the new code sets `session_id` for `Domain=.atyg.org`, the browser will send **both** cookies in the `Cookie` header during the `GET /dashboard` request. -Example: `Cookie: session_id=stale-host-only-uuid; session_id=fresh-wildcard-uuid`. -Additionally, the timing of `window.location.replace("/dashboard")` executing immediately after a `fetch()` resolves can sometimes trigger an Android WebView/Chrome bug where the OS cookie jar hasn't fully committed the new wildcard cookie before the navigation request fires, causing the browser to send only the old cookies. +Chromium on Android handles `Domain=.atyg.org` (wildcard cookies) correctly +according to RFC 6265, but there is a well-documented race condition / +overriding issue when both a host-only cookie (no Domain attribute) and a +wildcard domain cookie exist for the same name. If a user previously had a +`session_id` set exclusively for `auth.atyg.org` (perhaps during a test, or a +misconfigured previous version), and the new code sets `session_id` for +`Domain=.atyg.org`, the browser will send **both** cookies in the `Cookie` +header during the `GET /dashboard` request. Example: +`Cookie: session_id=stale-host-only-uuid; session_id=fresh-wildcard-uuid`. + +Additionally, the timing of `window.location.replace("/dashboard")` executing +immediately after a `fetch()` resolves can sometimes trigger an Android +WebView/Chrome bug where the OS cookie jar hasn't fully committed the new +wildcard cookie before the navigation request fires, causing the browser to send +only the old cookies. ## 2. Server & Middleware Cookie Extraction (Hono) ### Findings: -In Hono v4, `getCookie(c, "session_id")` parses the `Cookie` string and returns the **first** matching value it encounters. -If the browser sends `Cookie: session_id=stale-host-only-uuid; session_id=fresh-wildcard-uuid`, Hono will extract `stale-host-only-uuid`. -When `getAuthenticatedUser(c)` runs in `server/auth-session.ts`, it queries Valkey and PostgreSQL for `stale-host-only-uuid`. This query fails because the old session is expired or deleted. -Because it returns `null`, the middleware redirects the user back to `/login` via a `302 Found`, creating the "infinite login loop" where the authentication succeeds but the resulting session is instantly dropped. + +In Hono v4, `getCookie(c, "session_id")` parses the `Cookie` string and returns +the **first** matching value it encounters. If the browser sends +`Cookie: session_id=stale-host-only-uuid; session_id=fresh-wildcard-uuid`, Hono +will extract `stale-host-only-uuid`. When `getAuthenticatedUser(c)` runs in +`server/auth-session.ts`, it queries Valkey and PostgreSQL for +`stale-host-only-uuid`. This query fails because the old session is expired or +deleted. Because it returns `null`, the middleware redirects the user back to +`/login` via a `302 Found`, creating the "infinite login loop" where the +authentication succeeds but the resulting session is instantly dropped. ## 3. Timezone & Session Expiry Skew (PostgreSQL) ### Findings: + In `server/auth-session.ts`, the fallback DB check uses: `WHERE s.id = ${sessionId} AND s.expires_at > NOW()` -If the PostgreSQL container is running in a different timezone than the Deno container (e.g., PG is local time, Deno is UTC, or vice versa), `NOW()` in PG might evaluate to hours ahead of the `expires_at` value generated by Deno (`new Date(Date.now() + 30 * 24 * 60 * 60 * 1000)`). If `NOW()` is ahead, the session instantly evaluates as expired. -However, since Deno uses standard ISO UTC dates for inserts, if PG is also default UTC (standard docker behavior), this is less likely to be the root cause compared to the cookie shadowing issue, but remains a critical architectural risk. +If the PostgreSQL container is running in a different timezone than the Deno +container (e.g., PG is local time, Deno is UTC, or vice versa), `NOW()` in PG +might evaluate to hours ahead of the `expires_at` value generated by Deno +(`new Date(Date.now() + 30 * 24 * 60 * 60 * 1000)`). If `NOW()` is ahead, the +session instantly evaluates as expired. However, since Deno uses standard ISO +UTC dates for inserts, if PG is also default UTC (standard docker behavior), +this is less likely to be the root cause compared to the cookie shadowing issue, +but remains a critical architectural risk. ## 4. Traefik Ingress & Header Handling ### Findings: -Traefik generally forwards `Cookie` and `Set-Cookie` headers untouched unless specific stripping middlewares are configured. However, some reverse proxies lowercase header names. Hono's `getCookie` handles lowercase `cookie` correctly (as verified in our script). + +Traefik generally forwards `Cookie` and `Set-Cookie` headers untouched unless +specific stripping middlewares are configured. However, some reverse proxies +lowercase header names. Hono's `getCookie` handles lowercase `cookie` correctly +(as verified in our script). ## Root Cause Summary -The failure is primarily caused by **Cookie Shadowing** combined with **Hono's first-match cookie parsing**. The browser retains an old host-only cookie for the specific subdomain, and when the new wildcard domain cookie is set, the browser sends both. Hono reads the first one (the stale one), fails to find it in the DB/Valkey, and forces a re-login. + +The failure is primarily caused by **Cookie Shadowing** combined with **Hono's +first-match cookie parsing**. The browser retains an old host-only cookie for +the specific subdomain, and when the new wildcard domain cookie is set, the +browser sends both. Hono reads the first one (the stale one), fails to find it +in the DB/Valkey, and forces a re-login. ## Recommended Architectural Solutions (Ranked) **1. Most Reliable: Clear Host-Only Cookies & Use Strict Single Domain Setting** -- **Action:** Update the login verification endpoint to explicitly clear any existing host-only cookies by sending an additional `Set-Cookie` header with an empty value, `Max-Age=0`, and **no** `Domain` attribute, alongside the valid wildcard `Domain=.atyg.org` cookie. -- **Rationale:** This forcibly purges the shadowing host-only cookie from the browser's jar, ensuring only the wildcard cookie is sent. + +- **Action:** Update the login verification endpoint to explicitly clear any + existing host-only cookies by sending an additional `Set-Cookie` header with + an empty value, `Max-Age=0`, and **no** `Domain` attribute, alongside the + valid wildcard `Domain=.atyg.org` cookie. +- **Rationale:** This forcibly purges the shadowing host-only cookie from the + browser's jar, ensuring only the wildcard cookie is sent. **2. Highly Reliable: Parse All Cookies and Validate Iteratively** -- **Action:** Modify `getAuthenticatedUser` to manually parse `c.req.header("cookie")` and extract an array of all `session_id` values. Iterate through them, checking Valkey/DB until a valid session is found. -- **Rationale:** This bypasses Hono's first-match limitation and guarantees that if *any* valid session cookie is sent by the browser, the user is authenticated. + +- **Action:** Modify `getAuthenticatedUser` to manually parse + `c.req.header("cookie")` and extract an array of all `session_id` values. + Iterate through them, checking Valkey/DB until a valid session is found. +- **Rationale:** This bypasses Hono's first-match limitation and guarantees that + if _any_ valid session cookie is sent by the browser, the user is + authenticated. **3. Address PG Timezone Skew** -- **Action:** Change `s.expires_at > NOW()` to pass the current time from Deno, e.g., `s.expires_at > ${new Date().toISOString()}`, or use PostgreSQL's `CURRENT_TIMESTAMP AT TIME ZONE 'UTC'`. -- **Rationale:** Eliminates any possibility of timezone mismatch between the application runtime and the database engine. + +- **Action:** Change `s.expires_at > NOW()` to pass the current time from Deno, + e.g., `s.expires_at > ${new Date().toISOString()}`, or use PostgreSQL's + `CURRENT_TIMESTAMP AT TIME ZONE 'UTC'`. +- **Rationale:** Eliminates any possibility of timezone mismatch between the + application runtime and the database engine. **4. Delay Navigation on Mobile (Least Ideal)** -- **Action:** Add a 100-300ms `setTimeout` before executing `window.location.replace("/dashboard")` on successful login. -- **Rationale:** Provides Android's cookie jar sync sufficient time to commit the wildcard cookie before the top-level navigation fires, but feels hacky and degrades UX. + +- **Action:** Add a 100-300ms `setTimeout` before executing + `window.location.replace("/dashboard")` on successful login. +- **Rationale:** Provides Android's cookie jar sync sufficient time to commit + the wildcard cookie before the top-level navigation fires, but feels hacky and + degrades UX. diff --git a/scratch/chromium_cookie_investigation.md b/scratch/chromium_cookie_investigation.md index 78fba72..4654266 100644 --- a/scratch/chromium_cookie_investigation.md +++ b/scratch/chromium_cookie_investigation.md @@ -1,7 +1,23 @@ ## Chromium Android Wildcard Cookie Behavior + Upon research into Chromium bugs and specs regarding `Domain=.atyg.org`: -- RFC 6265 defines that a cookie with a `Domain` attribute is a "domain cookie" and is sent to subdomains. -- On Android Chrome, when `fetch()` or `window.location.replace` is executed immediately after the `Set-Cookie` header is received, there are known race conditions in the cookie jar sync. -- Further, if a domain has `SameSite=Lax` (which Hono sets by default unless specified), it is strictly blocked on cross-site requests, but `window.location.replace` from `login.atyg.org` to `login.atyg.org/dashboard` or another subdomain is still same-site. -- A critical issue on Android is that sometimes `Domain=.atyg.org` cookies are dropped during immediate redirects if the exact hostname doesn't perfectly align in the OS-level cookie sync, or if there is a conflict with an already existing host-only cookie for `auth.atyg.org` vs `atyg.org`. -- **Hono multiple cookie parsing issue**: As we saw in test 1, Hono's `getCookie` parses the FIRST matching cookie. If `auth.atyg.org` has a stale host-only cookie `session_id=old_stale` and the response sets `Domain=.atyg.org` with `session_id=new_valid`, the browser might send `Cookie: session_id=old_stale; session_id=new_valid`. Hono will read `old_stale`, fail DB lookup, and force a re-login. + +- RFC 6265 defines that a cookie with a `Domain` attribute is a "domain cookie" + and is sent to subdomains. +- On Android Chrome, when `fetch()` or `window.location.replace` is executed + immediately after the `Set-Cookie` header is received, there are known race + conditions in the cookie jar sync. +- Further, if a domain has `SameSite=Lax` (which Hono sets by default unless + specified), it is strictly blocked on cross-site requests, but + `window.location.replace` from `login.atyg.org` to `login.atyg.org/dashboard` + or another subdomain is still same-site. +- A critical issue on Android is that sometimes `Domain=.atyg.org` cookies are + dropped during immediate redirects if the exact hostname doesn't perfectly + align in the OS-level cookie sync, or if there is a conflict with an already + existing host-only cookie for `auth.atyg.org` vs `atyg.org`. +- **Hono multiple cookie parsing issue**: As we saw in test 1, Hono's + `getCookie` parses the FIRST matching cookie. If `auth.atyg.org` has a stale + host-only cookie `session_id=old_stale` and the response sets + `Domain=.atyg.org` with `session_id=new_valid`, the browser might send + `Cookie: session_id=old_stale; session_id=new_valid`. Hono will read + `old_stale`, fail DB lookup, and force a re-login. diff --git a/scratch/hono_cookie_test.ts b/scratch/hono_cookie_test.ts index ce15fc1..7049631 100644 --- a/scratch/hono_cookie_test.ts +++ b/scratch/hono_cookie_test.ts @@ -3,26 +3,26 @@ import { getCookie } from "jsr:@hono/hono@4/cookie"; const app = new Hono(); -app.get('/', (c) => { +app.get("/", (c) => { const sessionId = getCookie(c, "session_id"); const allCookies = c.req.header("cookie"); return c.json({ parsedSessionId: sessionId, - rawCookieHeader: allCookies + rawCookieHeader: allCookies, }); }); const req1 = new Request("http://localhost/", { headers: { - "Cookie": "session_id=first-uuid; session_id=second-uuid" - } + "Cookie": "session_id=first-uuid; session_id=second-uuid", + }, }); const req2 = new Request("http://localhost/", { headers: { - "cookie": "session_id=first-uuid; other=123" - } + "cookie": "session_id=first-uuid; other=123", + }, }); async function run() { diff --git a/scratch/pg_timezone_test.ts b/scratch/pg_timezone_test.ts index be0e9df..5e26c3e 100644 --- a/scratch/pg_timezone_test.ts +++ b/scratch/pg_timezone_test.ts @@ -1,4 +1,8 @@ -import { Pool } from "https://deno.land/x/postgres@v0.17.0/mod.ts"; +import { Pool as _Pool } from "https://deno.land/x/postgres@v0.17.0/mod.ts"; // Instead of real pg connection, let's just create a test that shows how NOW() works vs Date.now() -console.log("Postgres uses the database server's local timezone for NOW() unless explicitly AT TIME ZONE 'UTC' is used or the server runs in UTC."); -console.log("In containerized environments, the Deno container and Postgres container often both default to UTC, but if one differs, NOW() in Postgres could be hours ahead/behind Deno's generated expires_at."); +console.log( + "Postgres uses the database server's local timezone for NOW() unless explicitly AT TIME ZONE 'UTC' is used or the server runs in UTC.", +); +console.log( + "In containerized environments, the Deno container and Postgres container often both default to UTC, but if one differs, NOW() in Postgres could be hours ahead/behind Deno's generated expires_at.", +); diff --git a/server/auth-session.ts b/server/auth-session.ts index 37641ea..cd8682e 100644 --- a/server/auth-session.ts +++ b/server/auth-session.ts @@ -1,5 +1,5 @@ import type { Context } from "jsr:@hono/hono@4"; -import { getCookie } from "jsr:@hono/hono@4/cookie"; +import { deleteCookie } from "jsr:@hono/hono@4/cookie"; import { sqlWrapper } from "./db.ts"; import { valkey } from "./valkey.ts"; @@ -41,59 +41,88 @@ export function getCookieDomain(customRpId?: string): string | undefined { /** * Helper to get authenticated user from session cookie. * Checks Valkey cache first, with automatic PostgreSQL sessions table fallback. + * Iterates through all session_id cookies to prevent Android Chrome cookie shadowing. */ + +/** + * Extracts all session_id tokens from the Cookie header. + * Necessary because Chromium Android can send both a host-only and a wildcard cookie simultaneously. + */ +export function extractAllSessionIds(c: Context): string[] { + const cookieHeader = c.req.header("cookie") || ""; + if (!cookieHeader) return []; + return [...cookieHeader.matchAll(/(?:^|;\s*)session_id=([^;]+)/g)] + .map((m) => decodeURIComponent(m[1].trim())) + .filter(Boolean); +} + export async function getAuthenticatedUser( c: Context, ): Promise { - const sessionId = getCookie(c, "session_id"); - if (!sessionId) return null; + const sessionMatches = extractAllSessionIds(c); + if (sessionMatches.length === 0) return null; - // 1. Try Valkey cache - try { - const sessionDataStr = await valkey.get(sessionId); - if (sessionDataStr) { - const sessionData = JSON.parse(sessionDataStr); - if (sessionData && sessionData.uuid) { - return { - userId: sessionData.uuid, - sessionId, - username: sessionData.username || "", - }; + // Iterate over each candidate session ID + for (let i = 0; i < sessionMatches.length; i++) { + const candidateId = sessionMatches[i]; + + // 1. Try Valkey cache + try { + const sessionDataStr = await valkey.get(candidateId); + if (sessionDataStr) { + const sessionData = JSON.parse(sessionDataStr); + if (sessionData && sessionData.uuid) { + if (i > 0) { + // A stale ghost cookie was ahead of this valid one. + // Attempt to purge the host-only cookie to heal the browser jar. + deleteCookie(c, "session_id", { path: "/" }); + } + return { + userId: sessionData.uuid, + sessionId: candidateId, + username: sessionData.username || "", + }; + } } + } catch (_err) { + // Valkey cache miss or connection hiccup - fallback to DB } - } catch (_err) { - // Valkey cache miss or connection hiccup - fallback to DB - } - // 2. Fallback to PostgreSQL sessions table - try { - const session = await sqlWrapper.sql` - SELECT s.user_id, s.expires_at, u.username - FROM sessions s - JOIN users u ON s.user_id = u.id - WHERE s.id = ${sessionId} AND s.expires_at > NOW() - `.then((res: any) => res[0]); + // 2. Fallback to PostgreSQL sessions table + try { + const nowIso = new Date().toISOString(); + const session = await sqlWrapper.sql` + SELECT s.user_id, s.expires_at, u.username + FROM sessions s + JOIN users u ON s.user_id = u.id + WHERE s.id = ${candidateId} AND s.expires_at > ${nowIso} + `.then((res: any) => res[0]); - if (session) { - const username = session.username || ""; - // Repopulate Valkey in background - try { - const ttlSeconds = Math.max( - 1, - Math.floor( - (new Date(session.expires_at).getTime() - Date.now()) / 1000, - ), - ); - await valkey.setex( - sessionId, - ttlSeconds, - JSON.stringify({ uuid: session.user_id, username }), - ); - } catch (_e) {} - return { userId: session.user_id, sessionId, username }; + if (session) { + const username = session.username || ""; + // Repopulate Valkey in background + try { + const ttlSeconds = Math.max( + 1, + Math.floor( + (new Date(session.expires_at).getTime() - Date.now()) / 1000, + ), + ); + await valkey.setex( + candidateId, + ttlSeconds, + JSON.stringify({ uuid: session.user_id, username }), + ); + } catch (_e) {} + + if (i > 0) { + deleteCookie(c, "session_id", { path: "/" }); + } + return { userId: session.user_id, sessionId: candidateId, username }; + } + } catch (_err) { + // Continue to next candidate } - } catch (_err) { - return null; } return null; diff --git a/server/main.test.ts b/server/main.test.ts index d0aafe0..a965229 100644 --- a/server/main.test.ts +++ b/server/main.test.ts @@ -659,3 +659,47 @@ Deno.test("App Launchpad Zero-Knowledge Query Logic", async (t) => { } }); }); + +Deno.test("Cookie Shadowing & Multi-Cookie Iteration", async (t) => { + await t.step( + "GET /dashboard authenticates when fresh session is shadowed by stale host-only cookie", + async () => { + const mockGet = (key: string) => { + if (key === "fresh_wildcard_session") { + return Promise.resolve( + JSON.stringify({ uuid: "user-uuid", username: "tylerg" }), + ); + } + return Promise.resolve(null); + }; + valkey.get = mockGet as any; + + const originalSql = sqlWrapper.sql; + sqlWrapper.sql = () => Promise.resolve([]); + + try { + const res = await app.request("/dashboard", { + method: "GET", + headers: { + // Android Chrome sends stale host cookie first, then wildcard cookie + Cookie: + "session_id=stale_host_cookie; session_id=fresh_wildcard_session", + }, + }); + assertEquals(res.status, 200); + const text = await res.text(); + assert(text.includes("Application Launchpad")); + + // Verify Set-Cookie header attempts to delete the host-only cookie + const setCookieHeader = res.headers.get("set-cookie") || ""; + assert( + setCookieHeader.includes("session_id=") && + (setCookieHeader.includes("Max-Age=0") || + setCookieHeader.includes("Expires=")), + ); + } finally { + sqlWrapper.sql = originalSql; + } + }, + ); +}); diff --git a/server/main.ts b/server/main.ts index ccb642b..82f5607 100644 --- a/server/main.ts +++ b/server/main.ts @@ -12,6 +12,7 @@ import type { RegistrationResponseJSON, } from "jsr:@simplewebauthn/server@13"; import { deleteCookie, getCookie, setCookie } from "jsr:@hono/hono@4/cookie"; +import { extractAllSessionIds } from "./auth-session.ts"; import { decodeBase64Url, encodeBase64Url, @@ -820,13 +821,13 @@ app.post("/api/login/verify", async (c) => { return c.json({ error: "Internal server error" }, 500); } - const oldSessionId = getCookie(c, "session_id"); - if (oldSessionId) { - try { - await valkey.del(oldSessionId); - await sqlWrapper.sql`DELETE FROM sessions WHERE id = ${oldSessionId}`; - } catch (_e) { - // Best effort cleanup + const oldSessionIds = extractAllSessionIds(c); + if (oldSessionIds.length > 0) { + for (const old of oldSessionIds) { + try { + await valkey.del(old); + await sqlWrapper.sql`DELETE FROM sessions WHERE id = ${old}`; + } catch (_e) {} } } @@ -2198,27 +2199,27 @@ app.post("/api/revoke", async (c) => { const authHeader = c.req.header("Authorization"); if (authHeader && authHeader.startsWith("Bearer ")) { token = authHeader.split(" ")[1]; - } else { - // Fallback to session cookie (Web UI) - token = getCookie(c, "session_id") || ""; } if (!token) { - return c.json({ error: "Missing or invalid token" }, 401); - } + // Fallback to session cookie (Web UI) + const tokens = extractAllSessionIds(c); + if (tokens.length === 0) { + return c.json({ error: "Missing or invalid token" }, 401); + } - // SIDE EFFECT: Deletes the key in Valkey cache instantly - try { - await valkey.del(token); - } catch (_err: unknown) { - return c.json({ error: "Failed to revoke session from cache" }, 500); - } - - // Best effort delete from postgres if it's a UUID style session id - try { - await sqlWrapper.sql`DELETE FROM sessions WHERE id = ${token}`; - } catch (_e) { - // ignore + for (const t of tokens) { + try { + await valkey.del(t); + await sqlWrapper.sql`DELETE FROM sessions WHERE id = ${t}`; + } catch (_e) {} + } + } else { + // SDK Token Path + try { + await valkey.del(token); + await sqlWrapper.sql`DELETE FROM sessions WHERE id = ${token}`; + } catch (_e) {} } const cookieDomain = getCookieDomain(rpID); diff --git a/tasks/META_PROMPT.md b/tasks/path.md similarity index 77% rename from tasks/META_PROMPT.md rename to tasks/path.md index d6cd9f3..1f6bf96 100644 --- a/tasks/META_PROMPT.md +++ b/tasks/path.md @@ -5,7 +5,7 @@ your current job is to create, plan, or format a task file, please refer to --- -# Orchestrator Meta-Prompts +# Orchestrator Meta-Prompts (`tasks/path.md`) ## 1. Task Planning Template (System Analyst) @@ -95,3 +95,30 @@ Provide a structured critique report with: - **Strengths:** Key architectural insights captured by the author. - **Identified Gaps & Refinements:** Concrete adjustments to incorporate into the task file before implementation starts. ``` + +--- + +## 5. Architecture & Root-Cause Investigation Template (Investigator) + +_Use this template to instruct an agent to deeply investigate complex bugs, +reproduction anomalies, or distributed edge cases without modifying production +code._ + +```text +**Role:** Act as a Principal Systems & Security Investigator. + +**The Scope:** [Describe anomaly, bug symptoms, error logs, and affected components.] + +**Directives:** +- **STRICT CONSTRAINT:** Experiment, research, and report ONLY. Do not modify existing production code or create PRs with production changes. +- Place all reproduction test scripts and experiments in `scratch/` or hermetic test harnesses. + +**Your Task:** +1. Formulate clear, falsifiable hypotheses based on observed logs and symptoms. +2. Design and execute minimal reproduction scripts or benchmarks to test each hypothesis in isolation. +3. Trace data flows through database, caching layers, ingress proxies, and client runtime engines. +4. Document the definitive root cause and rank potential architectural solutions from most reliable to least, with explicit rationale and trade-offs. + +**Deliverable:** +Author an investigative report in `scratch/INVESTIGATIVE_REPORT.md` detailing verified findings, discarded hypotheses, and ranked solutions. +``` diff --git a/ui/mod.ts b/ui/mod.ts index b3b35bc..e696007 100644 --- a/ui/mod.ts +++ b/ui/mod.ts @@ -4,6 +4,7 @@ import { deleteCookie, getCookie } from "jsr:@hono/hono@4/cookie"; import { sql } from "../server/db.ts"; import { valkey } from "../server/valkey.ts"; import { + extractAllSessionIds, getAuthenticatedUser, getCookieDomain, isGlobalAdmin, @@ -35,13 +36,13 @@ uiApp.get("/", (c) => { }); uiApp.get("/logout", async (c) => { - const sessionId = getCookie(c, "session_id"); + const sessionIds = extractAllSessionIds(c); const rawRedirect = c.req.query("redirect"); let safeRedirect = null; const userIp = c.req.header("x-forwarded-for") || "127.0.0.1"; let userId = null; - if (sessionId) { + if (sessionIds.length > 0) { try { // Get user ID for auditing before we delete the session const authUser = await getAuthenticatedUser(c); @@ -49,8 +50,13 @@ uiApp.get("/logout", async (c) => { userId = authUser.userId; } - await valkey.del(sessionId); - await sql`DELETE FROM sessions WHERE id = ${sessionId}`; + // Purge all candidate cookies sent by the browser to ensure ghosts are eradicated + for (const sId of sessionIds) { + try { + await valkey.del(sId); + await sql`DELETE FROM sessions WHERE id = ${sId}`; + } catch (_e) {} + } } catch (_e) { // Best effort cleanup }