deckhearth/.convoys/fix-auth-bypass/brief-1-central-jwt-secret-helper.md

---
convoy: fix-auth-bypass
brief_number: 1
depends_on: []
files:
  - lib/auth-secret.js
  - lib/permission-middleware.js
  - pages/api/auth-utils.js
  - pages/api/auth/login.js
  - pages/api/auth/register.js
  - pages/api/auth/verify.js
  - pages/api/favorites.js
  - pages/api/users/search.js
cross_brief_commitments:
  - brief: 2
    description: |
      Brief 2 modifies `lib/permission-middleware.js` (replaces the synthetic-admin
      fallback in `getUserFromRequest`) and `pages/api/auth/verify.js` (removes the
      no-token admin-fetch branch). This brief MUST land first, because Brief 2
      relies on the `JWT_SECRET` import already being in place.
  - brief: 4
    description: |
      Brief 4 modifies `pages/api/auth/login.js` and `pages/api/auth/register.js`
      (drops `Access-Control-Allow-Origin: '*'`, wraps with rate limiter). This
      brief MUST land first, because Brief 4 builds on the post-refactor login /
      register handlers (no `JWT_SECRET` literal, `generateToken` from `auth-utils`).
  - brief: 5
    description: |
      Brief 5 (vitest + tests) imports `JWT_SECRET` and `JWT_TOKEN_TTL` from
      `lib/auth-secret.js` in test setup. This brief MUST land first.
---

# Brief 1: Central JWT secret helper + 24h token TTL

## Goal (1 sentence)

Create `lib/auth-secret.js` as the single source of truth for `JWT_SECRET` (fail-loud at module load if unset) and `JWT_TOKEN_TTL = '24h'`, then refactor the 7 files currently embedding `process.env.JWT_SECRET || '…'` literals to import from it.

## Files in scope (do not edit anything else)

- `lib/auth-secret.js` — **new**
- `lib/permission-middleware.js` — modified (literal → import)
- `pages/api/auth-utils.js` — modified (literal → import; `'7d'` → `JWT_TOKEN_TTL`)
- `pages/api/auth/login.js` — modified (literal → import; inline `jwt.sign(...)` → `generateToken(user)` from `auth-utils`; drop now-unused `jwt` import)
- `pages/api/auth/register.js` — modified (same as login)
- `pages/api/auth/verify.js` — modified (literal → import). **Do NOT remove the no-token admin-fetch branch here** — that's Brief 2's scope. Just swap the secret literal for the import.
- `pages/api/favorites.js` — modified (literal → import)
- `pages/api/users/search.js` — modified (literal → import)

## Conventions to follow

- `.cursor/rules/auth-and-permissions.mdc` § "Token model" — JWT model + signing surface.
- `.cursor/rules/api-routes.mdc` § "Authentication & Authorization" — handler shape stays the same; only the secret source changes.
- `.cursor/rules/no-go-zones.mdc` — do not edit any file outside `files:` above. In particular: no edits to `lib/auth-context.js`, `lib/admin-auth.js`, `lib/use-auth.js`, `lib/database.js`, `pages/_app.js`, or any UI file. Auth-context cleanup is the future `single-auth-provider` convoy.
- `package.json` formatting: 2-space indent, `"type": "module"` is set — use ES module imports throughout.
- Existing `import` style in `pages/api/auth-utils.js`: relative paths, no aliases. Match.
- No `engines` block change.
- No new dependencies in `package.json`. (Brief 4 adds `@upstash/ratelimit`; Brief 5 adds `vitest`. This brief adds nothing.)

## Acceptance criteria

### `lib/auth-secret.js` (new)

- [ ] File contains exactly two named exports: `JWT_SECRET` and `JWT_TOKEN_TTL`.
- [ ] `JWT_SECRET` reads `process.env.JWT_SECRET`. If unset OR empty string, the module **throws at import time** with a clear, actionable message that names the env var and points at `.env.local`. Verbatim shape (or near-verbatim — the message body can be reworded but the shape must be):

```js
const JWT_SECRET = process.env.JWT_SECRET;

if (!JWT_SECRET) {
  throw new Error(
    'JWT_SECRET environment variable is not set. ' +
      'Set it in .env.local for local dev, or in the Vercel project settings for deploys. ' +
      'Generate a strong secret with: openssl rand -hex 32'
  );
}

export { JWT_SECRET };
export const JWT_TOKEN_TTL = '24h';
```

- [ ] **No fallback string literal.** A previous fallback `'your-secret-key-change-in-production'` is what we are explicitly removing — do not reintroduce it under any condition.
- [ ] No length check (a length check is tempting but not required by the convoy and risks breaking existing valid-but-shorter dev secrets in `.env.local`; defer to a future hardening pass).
- [ ] No default export.
- [ ] No top-level side effects beyond the throw on missing env (no `console.log`, no `dotenv.config()` — Next.js loads `.env.local` automatically, and tests load env via `test/setup.js` in Brief 5).

### `pages/api/auth-utils.js`

- [ ] Line 4 (`const JWT_SECRET = process.env.JWT_SECRET || 'your-secret-key';`) **deleted**.
- [ ] Add at top of file: `import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js';`
- [ ] `generateToken(user)` returns `jwt.sign({...}, JWT_SECRET, { expiresIn: JWT_TOKEN_TTL })` — the literal `'7d'` is replaced. **This is the canonical token-minting function.**
- [ ] `verifyToken(token)` continues to call `jwt.verify(token, JWT_SECRET)` (no expiry param needed on verify).
- [ ] No other behavior change. `hashPassword`, `verifyPassword`, `isAdmin`, `getUserById` are untouched.

### `pages/api/auth/login.js`

- [ ] Line 5 (`const JWT_SECRET = process.env.JWT_SECRET || 'your-secret-key-change-in-production';`) **deleted**.
- [ ] Replace `import jwt from 'jsonwebtoken';` (line 2) with `import { generateToken } from '../../auth-utils.js';`. The path is `pages/api/auth/login.js` → `pages/api/auth-utils.js`, so relative import is `../auth-utils.js`. Verify by reading line 4 of `pages/api/auth/register.js` for the existing relative-import pattern (`'../../../lib/slug-utils.js'`).
- [ ] Replace the inline JWT mint:

```js
// before (lines 51-55)
const token = jwt.sign(
  { userId: user.id, email: user.email, role: user.role },
  JWT_SECRET,
  { expiresIn: '24h' }
);

// after
const token = generateToken({ id: user.id, email: user.email, role: user.role });
```

  Note the param shape change: `generateToken` reads `user.id` (not `user.userId`), per the existing implementation in `auth-utils.js`.
- [ ] **No CORS change here.** Brief 4 will tighten `Access-Control-Allow-Origin: '*'`. Leave it alone in this brief.
- [ ] **No rate-limit wiring here.** Brief 4 wraps with `@upstash/ratelimit`. Leave the handler shape alone.

### `pages/api/auth/register.js`

- [ ] Line 6 (`const JWT_SECRET = process.env.JWT_SECRET || 'your-secret-key-change-in-production';`) **deleted**.
- [ ] Replace `import jwt from 'jsonwebtoken';` with `import { generateToken } from '../auth-utils.js';`. Relative path: `pages/api/auth/register.js` → `pages/api/auth-utils.js` is `'../auth-utils.js'`.
- [ ] Replace the inline JWT mint at lines 142-147 with `const token = generateToken({ id: user.id, email: user.email, role: user.role });`
- [ ] Same CORS / rate-limit hands-off rule as login.

### `pages/api/auth/verify.js`

- [ ] Line 4 literal **deleted**.
- [ ] Add `import { JWT_SECRET } from '../../../lib/auth-secret.js';` at top. Path correctness: `pages/api/auth/verify.js` → `lib/auth-secret.js` is `'../../../lib/auth-secret.js'`.
- [ ] Keep `jwt.verify(token, JWT_SECRET)` inline (do not refactor to call `verifyToken` from `auth-utils.js` — that would change error semantics, and Brief 2 is already going to touch this file. Keep this brief mechanical).
- [ ] **Do NOT remove the no-token admin-fetch branch (lines 25-39).** That is Brief 2's job. Touching it here splits the security fix across two PRs unnecessarily.

### `pages/api/favorites.js`

- [ ] Line 4 literal **deleted**.
- [ ] Add `import { JWT_SECRET } from '../../lib/auth-secret.js';` at top. Path: `pages/api/favorites.js` → `lib/auth-secret.js` is `'../../lib/auth-secret.js'`.
- [ ] Keep `jwt.verify(token, JWT_SECRET)` inline. No other change.

### `pages/api/users/search.js`

- [ ] Line 4 literal **deleted**.
- [ ] Add `import { JWT_SECRET } from '../../../lib/auth-secret.js';` at top. Path: `pages/api/users/search.js` → `lib/auth-secret.js` is `'../../../lib/auth-secret.js'`.
- [ ] Keep `jwt.verify(token, JWT_SECRET)` inline. No other change.

### `lib/permission-middleware.js`

- [ ] Line 4 literal **deleted**.
- [ ] Add `import { JWT_SECRET } from './auth-secret.js';` at top.
- [ ] **Keep the rest of `getUserFromRequest` unchanged in this brief.** The synthetic-admin fallback removal is Brief 2's job.
- [ ] `withCollectionPermission`, `checkCollectionPermission`, `logCollectionActivity` are untouched.

### Repo-wide grep verification (run before opening PR)

- [ ] `rg "process\.env\.JWT_SECRET" --type js` returns **zero hits** in `lib/`, `pages/`. (Hits in `.convoys/`, `.cursor/`, `AGENTS.md`, `docs/` are documentation references — leave them alone in this brief.)
- [ ] `rg "your-secret-key" --type js` returns zero hits.
- [ ] `rg "'7d'" --type js pages/api/auth-utils.js` returns zero hits (replaced by `JWT_TOKEN_TTL`).
- [ ] `rg "'24h'" --type js pages/api/auth/` returns zero hits (replaced via `generateToken`).

### Smoke (manual, no test runner yet — Brief 5 adds vitest)

Document that you ran these in the PR description (not enforced in CI):

- [ ] `npm run lint` exits 0 (or matches the existing baseline — pre-existing errors are fine, no new ones).
- [ ] `npm run dev` boots; visit `http://localhost:3000/login`; submit valid credentials; observe that `localStorage.auth_token` is set and decoding the token shows `exp - iat ≈ 86400` (24h, not 7 days).
- [ ] Temporarily unset `JWT_SECRET` in `.env.local` and run `npm run dev`. Confirm the server logs the thrown error and the page returns 500. **Re-set `JWT_SECRET` before opening the PR.**
- [ ] `npm run build` succeeds. Vercel's preview deploy on the PR is green.

### Out of scope (do not do these)

- [ ] No edit to `pages/_app.js`, `lib/auth-context.js`, `lib/admin-auth.js`, `lib/use-auth.js`. Client-side context cleanup is the future `single-auth-provider` convoy.
- [ ] No edit to `AGENTS.md` or `.cursor/rules/auth-and-permissions.mdc`. Doc-writer pass updates these after the convoy lands.
- [ ] No removal of the synthetic-admin fallback in `getUserFromRequest` — Brief 2.
- [ ] No removal of the no-token admin branch in `verify.js` — Brief 2.
- [ ] No CORS changes — Brief 4.
- [ ] No rate-limit wiring — Brief 4.
- [ ] No test files — Brief 5.
- [ ] No deletion of `pages/api/test-*.js`, `pages/api/simple.js`, `pages/api/setup-database.js` — Brief 3.

## Rationale (≤3 sentences)

Centralizing `JWT_SECRET` removes 7 copies of the fallback literal in one PR, making the eventual fail-closed runtime behavior trivial to audit. Co-locating `JWT_TOKEN_TTL` in the same module canonicalizes 24h (matching current `login.js` behavior, which is what existing users have been getting) and resolves the silent inconsistency between `auth-utils.generateToken` (`'7d'`) and `login.js` (`'24h'`). Routing `login.js` and `register.js` through `auth-utils.generateToken` removes a second, drift-prone JWT-mint call site; the alternative — leaving inline `jwt.sign` everywhere — would make the next refactor more painful for no gain.