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
11 changes: 11 additions & 0 deletions .env.example
Original file line number Diff line number Diff line change
Expand Up @@ -32,3 +32,14 @@ WS_INTERNAL_URL=

# --- App ---
NEXT_PUBLIC_APP_URL=http://localhost:3000

# --- Rate limiting ---
# Per-bucket ceilings per 60s, as RATE_LIMIT_<BUCKET>. Leave unset in
# production: the shipped defaults (boards.create = 10/min) assume a proxy that
# sets x-forwarded-for, so every abuser gets their own bucket.
#
# Locally there is no such header, so ALL callers share one bucket — and
# `pnpm test:e2e` self-seeds a board per spec across 4 workers, which blows a
# 10/min budget and fails with 429. This raises the local ceiling so the suite
# can run; the limiter still applies, just far above one developer's pace.
RATE_LIMIT_BOARDS_CREATE=200
7 changes: 5 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,12 @@ jobs:
steps:
- uses: actions/checkout@v4

# No `version:` on purpose. action-setup@v4 throws "Multiple versions of
# pnpm specified" when its input disagrees with package.json's
# packageManager pin, and "9" !== "9.15.4". Reading the pin is also the
# behaviour we want: CI resolves the exact pnpm the lockfile was written
# with, and there is one place to bump it.
- uses: pnpm/action-setup@v4
with:
version: 9

- uses: actions/setup-node@v4
with:
Expand Down
44 changes: 25 additions & 19 deletions apps/web/app/(marketing)/page.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import Link from "next/link";

import { configuredProviders } from "@/auth";
import { NewBoardButton } from "@/components/marketing/NewBoardButton";
import { resolveViewer } from "@/lib/auth/viewer";

/** Minimal, calm landing: the one-liner, one button, a sign-in affordance. */
export default async function LandingPage() {
const viewer = await resolveViewer();
// No provider means nothing can sign you in, so offering it would send
// people to a page that can only apologise.
const canSignIn = configuredProviders().length > 0;

return (
<main className="flex min-h-screen flex-col items-center justify-center gap-8 p-6">
Expand All @@ -22,25 +26,27 @@ export default async function LandingPage() {

<NewBoardButton />

<p className="text-xs text-faint">
{viewer ? (
<>
You&apos;re in as <span className="text-muted">{viewer.name}</span>
{viewer.kind === "guest" ? (
<>
{" · "}
<Link href="/api/auth/signin" className="text-accent hover:underline">
Sign in to keep your boards
</Link>
</>
) : null}
</>
) : (
<Link href="/api/auth/signin" className="text-accent hover:underline">
Sign in
</Link>
)}
</p>
{viewer || canSignIn ? (
<p className="text-xs text-faint">
{viewer ? (
<>
You&apos;re in as <span className="text-muted">{viewer.name}</span>
{viewer.kind === "guest" && canSignIn ? (
<>
{" · "}
<Link href="/signin" className="text-accent hover:underline">
Sign in to keep your boards
</Link>
</>
) : null}
</>
) : (
<Link href="/signin" className="text-accent hover:underline">
Sign in
</Link>
)}
</p>
) : null}
</main>
);
}
77 changes: 77 additions & 0 deletions apps/web/app/(marketing)/signin/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import Link from "next/link";

import { configuredProviders, signIn } from "@/auth";
import { NewBoardButton } from "@/components/marketing/NewBoardButton";
import { Button } from "@/components/ui/button";
import { Card, CardContent, CardDescription, CardHeader, CardTitle } from "@/components/ui/card";
import { safeCallbackUrl } from "@/lib/auth/safe-callback";

interface SignInPageProps {
searchParams: Promise<{ callbackUrl?: string; error?: string }>;
}

/**
* /signin — Auth.js `pages.signIn`, replacing the built-in page.
*
* Two states, because a bare setup ships with no OAuth keys (zero-setup
* principle) and the built-in page renders an empty card in that case:
* providers configured → a button each; none → say so plainly and offer the
* guest path, which already works and is what most people want anyway.
*/
export default async function SignInPage({ searchParams }: SignInPageProps) {
const { callbackUrl, error } = await searchParams;
const providers = configuredProviders();
const redirectTo = safeCallbackUrl(callbackUrl);

return (
<main className="flex min-h-screen items-center justify-center p-6">
<Card className="w-full max-w-sm">
<CardHeader>
<CardTitle>
{providers.length > 0 ? "Sign in to Quorum" : "Sign-in isn’t set up"}
</CardTitle>
<CardDescription>
{providers.length > 0
? "Signing in keeps your boards across devices. You can also just start a board as a guest."
: "This deployment has no sign-in provider configured, so there’s nothing to sign in with yet. Guest boards work fine without one."}
</CardDescription>
</CardHeader>
<CardContent className="flex flex-col items-center gap-3">
{error ? (
<p role="alert" className="text-danger w-full text-xs">
That sign-in attempt didn’t go through. Try again, or start a board as a guest.
</p>
) : null}

{providers.map((provider) => (
<form
key={provider.id}
className="w-full"
action={async () => {
"use server";
await signIn(provider.id, { redirectTo });
}}
>
<Button type="submit" className="w-full">
Continue with {provider.name}
</Button>
</form>
))}

{providers.length > 0 ? (
<Link href="/" className="text-accent text-xs hover:underline">
Back to Quorum
</Link>
) : (
<>
<NewBoardButton />
<Link href="/" className="text-accent text-xs hover:underline">
Back to Quorum
</Link>
</>
)}
</CardContent>
</Card>
</main>
);
}
25 changes: 25 additions & 0 deletions apps/web/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,27 @@ function buildProviders(): NextAuthConfig["providers"] {
return providers;
}

/** What /signin renders a button for. Empty in a bare setup — see below. */
export interface ConfiguredProvider {
id: string;
name: string;
}

/**
* The providers actually wired up, derived from the SAME list Auth.js gets so
* the two can never disagree.
*
* Callers use this to decide whether signing in is possible at all: with no
* OAuth keys the list is empty, and anything that offers a sign-in affordance
* would otherwise send people to a page that cannot sign them in.
*/
export function configuredProviders(): ConfiguredProvider[] {
return buildProviders().map((provider) => {
const config = typeof provider === "function" ? provider() : provider;
return { id: config.id, name: config.name };
});
}

export const { handlers, auth, signIn, signOut } = NextAuth(() => ({
// Lazy config: getDb() is called per request, not at import, so importing
// this module never opens a database connection by itself.
Expand All @@ -50,6 +71,10 @@ export const { handlers, auth, signIn, signOut } = NextAuth(() => ({
session: { strategy: "database" },
secret: env.AUTH_SECRET,
trustHost: true,
// Auth.js's built-in page renders an empty card when no provider is
// configured, which is the documented default here — a dead end in the
// product's own light theme. Ours explains itself and always offers a way on.
pages: { signIn: "/signin", error: "/signin" },
providers: buildProviders(),
callbacks: {
session({ session, user }) {
Expand Down
41 changes: 41 additions & 0 deletions apps/web/lib/auth/safe-callback.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
/**
* Regression: ISSUE-006 — /api/auth/signin rendered a blank page with no
* provider configured. The replacement /signin page accepts Auth.js's
* attacker-controlled `callbackUrl`, so it must never redirect off-origin.
* Found by /qa on 2026-08-01
* Report: .gstack/qa-reports/qa-report-quorum-2026-08-01-signin.md
*/
import { describe, expect, it } from "vitest";

import { safeCallbackUrl } from "./safe-callback";

describe("safeCallbackUrl", () => {
it("keeps a same-origin path so sign-in returns you where you were", () => {
expect(safeCallbackUrl("/b/brd_abc123")).toBe("/b/brd_abc123");
});

it("keeps the query string and hash on that path", () => {
expect(safeCallbackUrl("/b/brd_abc?tab=chat#msg-3")).toBe("/b/brd_abc?tab=chat#msg-3");
});

it.each([undefined, null, ""])("falls back home for %j", (value) => {
expect(safeCallbackUrl(value)).toBe("/");
});

it.each([
"https://evil.example/steal",
"http://evil.example",
"javascript:alert(1)",
"data:text/html,<script>alert(1)</script>",
"evil.example",
])("refuses the absolute or scheme-bearing target %j", (value) => {
expect(safeCallbackUrl(value)).toBe("/");
});

it.each(["//evil.example", "//evil.example/path", "/\\evil.example"])(
"refuses %j, which a browser reads as protocol-relative despite the leading slash",
(value) => {
expect(safeCallbackUrl(value)).toBe("/");
},
);
});
19 changes: 19 additions & 0 deletions apps/web/lib/auth/safe-callback.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* Sanitiser for the `callbackUrl` Auth.js appends when it bounces someone to
* the sign-in page. The value arrives in the query string, so it is entirely
* attacker-controlled: a link like
* `/signin?callbackUrl=https://evil.example` would otherwise let a phisher use
* our own domain to redirect people after a real, successful sign-in.
*
* Only same-origin paths survive. A leading "/" is NOT enough on its own —
* browsers read "//evil.example" as protocol-relative and "/\evil.example" the
* same way, both absolute despite the leading slash.
*/
export function safeCallbackUrl(raw: string | undefined | null): string {
if (typeof raw !== "string" || raw === "") return "/";
if (!raw.startsWith("/")) return "/";
// Reject every scheme-ish second character; "/" and "\" are the two a
// browser treats as the start of an authority.
if (raw.startsWith("//") || raw.startsWith("/\\")) return "/";
return raw;
}
82 changes: 82 additions & 0 deletions apps/web/lib/server/rate-limit.regression-1.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { afterEach, describe, expect, it } from "vitest";

import { enforceRateLimit, type RateLimitStore } from "./rate-limit";

/**
* Regression: ISSUE-004 — the shipped 10/min boards.create ceiling applies to
* one shared `ip:unknown` bucket when no proxy header is present, so the
* self-seeding e2e suite failed 8/18 specs with 429.
* Found by /qa on 2026-08-01
* Report: .gstack/qa-reports/qa-report-quorum-2026-08-01.md
*/

function fakeStore(): RateLimitStore {
const entries = new Map<string, number>();
return {
async hit(key, windowSec) {
const count = (entries.get(key) ?? 0) + 1;
entries.set(key, count);
return { count, ttlMs: windowSec * 1_000 };
},
};
}

function req(): Request {
return new Request("http://localhost:3000/api/boards", { method: "POST" });
}

const ENV_KEY = "RATE_LIMIT_BOARDS_CREATE";
const original = process.env[ENV_KEY];

afterEach(() => {
if (original === undefined) delete process.env[ENV_KEY];
else process.env[ENV_KEY] = original;
});

/** Requests allowed before the first 429, capped so a broken override can't hang. */
async function allowedBefore429(store: RateLimitStore, ceiling = 40): Promise<number> {
for (let sent = 0; sent < ceiling; sent += 1) {
if (await enforceRateLimit(req(), { bucket: "boards.create", limit: 10, windowSec: 60, store }))
return sent;
}
return ceiling;
}

describe("per-bucket limit override", () => {
it("raises the ceiling so a self-seeding suite is not blocked at 10", async () => {
process.env[ENV_KEY] = "25";
expect(await allowedBefore429(fakeStore())).toBe(25);
});

it("keeps the route default when unset — production is unchanged", async () => {
delete process.env[ENV_KEY];
expect(await allowedBefore429(fakeStore())).toBe(10);
});

it.each(["0", "-5", "abc", "10.5", " "])(
"ignores the unusable override %j rather than disabling the limit",
async (value) => {
process.env[ENV_KEY] = value;
expect(await allowedBefore429(fakeStore())).toBe(10);
},
);

it("scopes the override to its own bucket", async () => {
process.env[ENV_KEY] = "25";
const store = fakeStore();
// A different bucket must not inherit boards.create's raised ceiling.
let allowed = 0;
while (
allowed < 40 &&
!(await enforceRateLimit(req(), {
bucket: "agents.connect",
limit: 10,
windowSec: 60,
store,
}))
) {
allowed += 1;
}
expect(allowed).toBe(10);
});
});
Loading
Loading