Closes out the fix-auth-bypass convoy (PRs #6–#11, merged through
1629afb) on the docs side. Code already on main; this PR is docs only.
Updates:
AGENTS.md
- §1 auth bullet refreshed (auth-secret SoT, 24h TTL, no synthetic
admin, login/register rate limit)
- §3 conventions point at lib/auth-secret.js + lib/rate-limit.js
- §4 gotchas #2/#3/#5 converted to "Resolved" notes in place
(NOT renumbered, to preserve cross-references)
- new #12 documents the KV_REST_API_* env-var convention
- §5 setup list adds the rate-limit env vars
- §6 testing rewritten for Vitest (16 unit tests, blocking CI gate)
.cursor/rules/auth-and-permissions.mdc
- canonical-surface table gains lib/auth-secret.js + lib/rate-limit.js
- token model now 24h (was 7d) with fail-loud explanation
- server-side authorization patterns lead with null → 401 contract
.cursor/rules/api-routes.mdc
- removes the "CRITICAL — known bug" callout (resolved by Brief 2)
- adds a "Rate limiting" section with verbatim shape + env-var notes
- "Dev/test endpoints" → "Removed" historical note so future agents
searching for test-db understand why it's gone
.convoys/fix-auth-bypass.md (restored — was on convoy branch only)
- frontmatter → status: shipped
- new "Convoy outcome" section: briefs + commits + resolved gotchas,
R1-R12 risk walk, env-var-rename deviation record, queued follow-up
convoys, lessons learned
.convoys/fix-auth-bypass/brief-{1..5}-*.md (restored from convoy branch)
- audit-trail completeness; convoy plan references them by name
- brief 4 additionally updated: UPSTASH_REDIS_REST_* → KV_REST_API_*
across init rules, smoke, pre-deploy checklist
- brief 4 has a new "Post-merge addendum" explaining the rename
.convoys/ship-readiness.md
- P0 #1, #2, #4 → RESOLVED with merge-commit citations
- P0 #5 (CORS), #6 (rate limit) → PARTIAL with deferral pointers
(cors-tighten and add-rate-limiting convoys)
- each item gains an "As-shipped" line for self-containment
README.md
- Next.js 15 → 16, TypeScript claim corrected to JS-with-devDep
- auth + rate-limit + testing bullets updated
- env-var template extended with KV_REST_API_*
- deleted dev-endpoints note added to the API list
- "Default Admin Account" section LEFT ALONE — drop-public-setup territory
Verified: build exit 0 (with JWT_SECRET set), 16/16 vitest tests pass,
lint baseline unchanged (128/81/47).
Convoy: fix-auth-bypass / role-doc-writer (closeout)
Co-authored-by: Cursor <cursoragent@cursor.com>
11 KiB
| convoy | brief_number | depends_on | files | cross_brief_commitments | |||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| fix-auth-bypass | 1 |
|
|
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— newlib/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; inlinejwt.sign(...)→generateToken(user)fromauth-utils; drop now-unusedjwtimport)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 outsidefiles:above. In particular: no edits tolib/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 futuresingle-auth-providerconvoy.package.jsonformatting: 2-space indent,"type": "module"is set — use ES module imports throughout.- Existing
importstyle inpages/api/auth-utils.js: relative paths, no aliases. Match. - No
enginesblock change. - No new dependencies in
package.json. (Brief 4 adds@upstash/ratelimit; Brief 5 addsvitest. This brief adds nothing.)
Acceptance criteria
lib/auth-secret.js (new)
- File contains exactly two named exports:
JWT_SECRETandJWT_TOKEN_TTL. JWT_SECRETreadsprocess.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):
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, nodotenv.config()— Next.js loads.env.localautomatically, and tests load env viatest/setup.jsin 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)returnsjwt.sign({...}, JWT_SECRET, { expiresIn: JWT_TOKEN_TTL })— the literal'7d'is replaced. This is the canonical token-minting function.verifyToken(token)continues to calljwt.verify(token, JWT_SECRET)(no expiry param needed on verify).- No other behavior change.
hashPassword,verifyPassword,isAdmin,getUserByIdare 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) withimport { generateToken } from '../../auth-utils.js';. The path ispages/api/auth/login.js→pages/api/auth-utils.js, so relative import is../auth-utils.js. Verify by reading line 4 ofpages/api/auth/register.jsfor the existing relative-import pattern ('../../../lib/slug-utils.js'). - Replace the inline JWT mint:
// 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';withimport { generateToken } from '../auth-utils.js';. Relative path:pages/api/auth/register.js→pages/api/auth-utils.jsis'../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.jsis'../../../lib/auth-secret.js'. - Keep
jwt.verify(token, JWT_SECRET)inline (do not refactor to callverifyTokenfromauth-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.jsis'../../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.jsis'../../../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
getUserFromRequestunchanged in this brief. The synthetic-admin fallback removal is Brief 2's job. withCollectionPermission,checkCollectionPermission,logCollectionActivityare untouched.
Repo-wide grep verification (run before opening PR)
rg "process\.env\.JWT_SECRET" --type jsreturns zero hits inlib/,pages/. (Hits in.convoys/,.cursor/,AGENTS.md,docs/are documentation references — leave them alone in this brief.)rg "your-secret-key" --type jsreturns zero hits.rg "'7d'" --type js pages/api/auth-utils.jsreturns zero hits (replaced byJWT_TOKEN_TTL).rg "'24h'" --type js pages/api/auth/returns zero hits (replaced viagenerateToken).
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 lintexits 0 (or matches the existing baseline — pre-existing errors are fine, no new ones).npm run devboots; visithttp://localhost:3000/login; submit valid credentials; observe thatlocalStorage.auth_tokenis set and decoding the token showsexp - iat ≈ 86400(24h, not 7 days).- Temporarily unset
JWT_SECRETin.env.localand runnpm run dev. Confirm the server logs the thrown error and the page returns 500. Re-setJWT_SECRETbefore opening the PR. npm run buildsucceeds. 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 futuresingle-auth-providerconvoy. - No edit to
AGENTS.mdor.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.