Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ GitHub profile to request a private channel before sharing exploit details.
- Never requests repository, organization, code, issue, workflow, administration, or write permissions.
- Stores expiring user and refresh tokens only on the server, keyed by an opaque session id. Tokens never reach the browser, never enter `localStorage`, and are never logged.
- Requires user-to-server token expiration and rotates a token if it approaches expiry.
- Protects the authorization-code flow with both a one-time `state` value and PKCE (`S256`). OAuth state expires after 10 minutes.
- Protects the authorization-code flow with a short-lived signed `state` cookie and PKCE (`S256`). The verifier stays in the HttpOnly cookie so an authorization in progress survives a server restart; it expires after 10 minutes.
- Uses the configured `APP_ORIGIN` for the callback URL instead of trusting the incoming request host in production.
- Keeps sessions in memory for one hour and caches fetched stars for 60 seconds. Both disappear when the server restarts.

Expand Down
15 changes: 15 additions & 0 deletions scripts/session.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ const {
destroySession,
getCachedStars,
getSession,
newOAuthRequest,
reserveStarsFetch,
signSessionId,
consumeOAuthState,
verifySignedSessionId,
} = await import("../src/server/session.ts");

Expand All @@ -36,6 +38,19 @@ test("signed session ids reject tampering", () => {
destroySession(sessionId);
});

test("OAuth request cookies carry a signed PKCE verifier", () => {
const request = newOAuthRequest();
const verifier = consumeOAuthState(request.state, request.cookieValue);

assert.ok(verifier);
assert.ok(verifier.length >= 43);
assert.equal(consumeOAuthState(`${request.state}x`, request.cookieValue), null);

const last = request.cookieValue.at(-1);
const tampered = `${request.cookieValue.slice(0, -1)}${last === "0" ? "1" : "0"}`;
assert.equal(consumeOAuthState(request.state, tampered), null);
});

test("star caches remain isolated between user sessions", () => {
const aliceId = createSession(token("alice"), "alice");
const bobId = createSession(token("bob"), "bob");
Expand Down
6 changes: 3 additions & 3 deletions src/pages/api/auth/callback.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,11 @@ export const GET: APIRoute = async ({ request, url, cookies, redirect }) => {
return new Response("Missing code or state", { status: 400 });
}

const cookieState = cookies.get(OAUTH_STATE_COOKIE)?.value;
const oauthRequest = cookies.get(OAUTH_STATE_COOKIE)?.value;
cookies.delete(OAUTH_STATE_COOKIE, { path: "/" });

const codeVerifier = cookieState === state ? consumeOAuthState(state) : null;
if (!cookieState || cookieState !== state || !codeVerifier) {
const codeVerifier = oauthRequest ? consumeOAuthState(state, oauthRequest) : null;
if (!codeVerifier) {
return new Response("Invalid OAuth state. Please try signing in again.", {
status: 400,
});
Expand Down
4 changes: 2 additions & 2 deletions src/pages/api/auth/login.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ export const GET: APIRoute = async ({ request, cookies, redirect }) => {
{ status: 503, headers: { "content-type": "text/plain" } }
);
}
const { state, codeChallenge } = newOAuthRequest();
cookies.set(OAUTH_STATE_COOKIE, state, {
const { state, codeChallenge, cookieValue } = newOAuthRequest();
cookies.set(OAUTH_STATE_COOKIE, cookieValue, {
httpOnly: true,
secure: useSecureCookies(request),
sameSite: "lax",
Expand Down
43 changes: 28 additions & 15 deletions src/server/session.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
/**
* In-memory session store. GitHub App user and refresh tokens never leave the server.
*
* v1 limitation: this is a single-process Map. Fine for self-hosting and
* local dev. For multi-instance production deploys, swap this file's body
* for a Redis/KV client. The exported interface must stay the same.
* User sessions remain single-process in v1. OAuth requests are instead kept
* in a signed, short-lived cookie so the GitHub callback survives a process
* restart or a different instance.
*/
import { randomBytes, createHash, createHmac, timingSafeEqual } from "node:crypto";
import type { StarredData } from "../scripts/treemap";
Expand All @@ -26,7 +26,6 @@ const COOKIE_NAME = "gitvis_session";
const STATE_COOKIE = "gitvis_oauth_state";

const sessions = new Map<string, Session>();
const pendingStates = new Map<string, { createdAt: number; codeVerifier: string }>();

export function signSessionId(sessionId: string): string {
const h = createHmac("sha256", getSessionSecret()).update(sessionId).digest("hex");
Expand Down Expand Up @@ -103,23 +102,37 @@ function pruneExpired(): void {
export function newOAuthRequest(): {
state: string;
codeChallenge: string;
cookieValue: string;
} {
const state = randomBytes(16).toString("hex");
const codeVerifier = randomBytes(32).toString("base64url");
const codeChallenge = createHash("sha256").update(codeVerifier).digest("base64url");
pendingStates.set(state, { createdAt: Date.now(), codeVerifier });
for (const [key, pending] of pendingStates) {
if (Date.now() - pending.createdAt > OAUTH_STATE_TTL_MS) pendingStates.delete(key);
}
return { state, codeChallenge };
const payload = `${state}.${Date.now()}.${codeVerifier}`;
const signature = createHmac("sha256", getSessionSecret()).update(payload).digest("hex");
return { state, codeChallenge, cookieValue: `${payload}.${signature}` };
}

export function consumeOAuthState(state: string): string | null {
const pending = pendingStates.get(state);
if (!pending) return null;
pendingStates.delete(state);
if (Date.now() - pending.createdAt > OAUTH_STATE_TTL_MS) return null;
return pending.codeVerifier;
export function consumeOAuthState(state: string, cookieValue: string): string | null {
const parts = cookieValue.split(".");
if (parts.length !== 4) return null;
const [cookieState, createdAtValue, codeVerifier, signature] = parts;
if (cookieState !== state) return null;

const createdAt = Number(createdAtValue);
const age = Date.now() - createdAt;
if (!Number.isFinite(createdAt) || age < 0 || age > OAUTH_STATE_TTL_MS) return null;

const payload = `${cookieState}.${createdAtValue}.${codeVerifier}`;
const expected = createHmac("sha256", getSessionSecret()).update(payload).digest("hex");
try {
const actualBytes = Buffer.from(signature, "hex");
const expectedBytes = Buffer.from(expected, "hex");
if (actualBytes.length !== expectedBytes.length) return null;
if (!timingSafeEqual(actualBytes, expectedBytes)) return null;
} catch {
return null;
}
return codeVerifier;
}

export const SESSION_COOKIE_NAME = COOKIE_NAME;
Expand Down
Loading