fix(nextjs-backend): patch secret leak and invalid CORS header; harden users API

[frankstupak]

nextjs-backend uplift — secret leak, invalid CORS header, hardened users API, O(1) dedupe

The nextjs-backend reference had two shipped-config bugs that would bite in production, an unguarded users endpoint, and a benchmarkable perf trap waiting for anyone who added dedupe the obvious way. Fixed all four, backward-compatibly, with tests to keep them fixed.

🔒 Security — two real config bugs

1. Secrets were being inlined into the client bundle. next.config.js exposed JWT_SECRET and DATABASE_URL through the env key. Per the Next.js docs, values in env are always included in the JavaScript bundle at build time — so the JWT signing secret shipped to the browser. Removed them; server code already reads process.env.JWT_SECRET / process.env.DATABASE_URL directly in route handlers and server components.

2. The CORS header was invalid. The static header block set Access-Control-Allow-Origin to process.env.ALLOWED_ORIGINS, which env.example documents as a comma-separated list (http://localhost:3000,http://localhost:3034). Access-Control-Allow-Origin is not list-valued — the spec allows a single origin, *, or null. A comma/space list is rejected by every browser, so cross-origin requests from the "allowed" origins would fail outright.

The correct pattern for a multi-origin allow-list is per-request origin reflection, which a static header block can't do. Moved CORS into src/middleware.ts:

• match the request Origin against the allow-list and reflect back a single valid origin,
• add Vary: Origin so shared caches don't cross-serve,
• answer OPTIONS preflight with 204,
• never emit * together with credentials (also spec-illegal).

All the origin-decision logic lives in src/lib/cors.ts as pure functions with exhaustive unit tests. Also set X-XSS-Protection: 0 (current OWASP guidance — the legacy auditor can introduce vulnerabilities) and added Referrer-Policy: no-referrer.

✅ Correctness — POST /api/users

• Duplicate emails now return 409 Conflict instead of silently creating duplicate records (the Prisma schema marks email @unique; the in-memory store ignored it).
• Emails are normalized (trim + lowercase), so Person@Example.com and person@example.com are one identity.
• Whitespace-only names are rejected — plain z.string().min(1) accepted " "; now .trim().min(1).

⚡ Performance — O(1) uniqueness, no O(n²) trap

Email uniqueness is backed by a Map index (O(1) per insert) rather than the obvious users.find(u => u.email === email) scan, which is O(n) per insert → O(n²) across N inserts. Benchmark (bench/dedupe-bench.ts, npx tsx):

N inserts	naive Array.find	indexed Map	speedup
10,000	559.2 ms	4.4 ms	127x
50,000	30,901.3 ms	25.6 ms	1209x

➕ Extras

• Optional, backward-compatible pagination on GET /api/users (?limit&offset). No params → the full list, unchanged shape; total always reflects the full count. (The subproject even defined PaginatedResponse<T> but never used pagination.)
• Added CONFLICT: 409 to the HttpStatus constants (the file's stated purpose is replacing magic numbers).

🧪 Verification

• Tests: 9 → 32 (+23). New: CORS unit tests, users dedupe/normalization/trim/pagination, and a next.config regression guard (asserts no secrets in env, no static ACAO, X-XSS-Protection: 0).
• nextjs-backend jest project (run via the repo's root jest config): 32/32 green.
• tsc --noEmit clean; eslint --max-warnings 0 clean.
• All changes are confined to src/api/nextjs-backend/; the root default jest project excludes that path by config, so nothing else is affected. Public API/response shapes unchanged.

— Lumen Industries

[SkinnnyJay]

CI has now run on this branch for the first time — the earlier runs sat unapproved (fork PR, first-time-contributor policy) and expired, so nothing was ever reported here.

Result: failure on Node 22, in this PR's own route. Node 18 and 20 were cancelled once 22 failed.

next build rejects the handler signature:

src/app/api/users/route.ts
Type error: Route "src/app/api/users/route.ts" has an invalid "GET" export:
  Type "NextRequest | undefined" is not a valid type for the function's first argument.
    Expected "NextRequest | Request", got "NextRequest | undefined".

The optional parameter is the problem:

export async function GET(request?: NextRequest): Promise<NextResponse> {
  const params = request?.nextUrl?.searchParams;

Next validates route exports structurally, and an optional first parameter widens the type to include undefined, which the framework never passes. I take it the ? is there so the existing tests can keep calling GET() with no argument — but that convenience is what breaks the build.

Making the parameter required is the fix on the route side:

export async function GET(request: NextRequest): Promise<NextResponse> {
  const params = request.nextUrl.searchParams;

and the callers construct one, e.g. GET(new NextRequest("http://localhost/api/users")) — with the query string appended for the paginated cases, which reads better in the tests than the current implicit "no request means no pagination" path anyway.

Worth knowing before you re-push: main is currently red on Node 18 for an unrelated reason, and #14 fixes that. Until it lands, the other two legs here will still come back red.

[SkinnnyJay]

Status update. main is green again and the other ten PRs in this batch have landed — #1, #2, #4, #5, #6, #7, #8, #9, #11, #12, all with CI passing on Node 20 and 22.

Two things had been masking the real verdicts:

1. fix(ci): drop Node 18 from the matrix #14 dropped Node 18 from the matrix. Its websocket teardown failure was hitting every PR in the batch, including ones that touched nothing but src/algorithms.
2. The merge refs were stale — closing and reopening a PR does not recompute refs/pull/N/merge against a moved base, so the first re-runs still executed the old three-version matrix. update-branch on each PR fixed that.

This one is now the only thing standing between the branch and a merge — the failure is in your own change, detailed in the comment above. Once it is fixed, rebase or use "Update branch" so CI runs against current main, and it should go green.

Also filed #15 for a flake you may hit on a re-run: cache.test.ts › should handle TTL correctly fails intermittently on Node 22 regardless of branch. If you see that one, it is not yours.

[RealLumenHere]

Rebased onto main and both checks are green now. test 20 and test 22 were failing on a Next.js route-handler type error — the exported GET had an optional NextRequest param, which the App Router type checker rejects (requires NextRequest | Request, not | undefined). Made the param required and updated the direct-invocation test calls to pass a request instead of calling GET() bare. No change to the CORS/secret-leak fix itself.

Ready for review whenever you get a chance.

[SkinnnyJay]

Sorry for the delay — you cleared this on Aug 30 and it sat. Confirmed green: test (20) and test (22) both pass, mergeable: CLEAN.

Verified the fix matches the description:

-export async function GET(): Promise<NextResponse> {
+export async function GET(request: NextRequest): Promise<NextResponse> {

Your diagnosis is right, and it's worth stating why for anyone reading this later: App Router type-checks each route.ts export against (request: NextRequest) => ..., and a parameter TypeScript widens to NextRequest | undefined isn't assignable to that — a zero-arg GET() fails for the same reason. Making it required and passing a request in the direct-invocation tests is the correct fix, not a workaround, and it doesn't touch the CORS or secret-leak changes. Agreed on all of it.

Heads up on a possible collision with #10. That PR adds a brand-new src/api/nextjs-backend/package-lock.json (11,560 lines) even though it's a pagination PR — a first-ever lockfile for this package. This PR adds no lockfile, so both are CLEAN against main right now and nothing is broken. But if #10 lands first, your nextjs-backend changes end up resolving against a dependency tree that arrived through someone else's pagination PR. I've asked over there to drop the unrelated lockfiles, so this should resolve itself — flagging it so it isn't a surprise if you see main move under you.

Nothing needed from you here. This one's ready and I'll review the CORS and users-API hardening properly next — 9 files with real tests (cors.test.ts, next-config.test.ts, the middleware) is the shape I want these in, so thanks for that.