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>
38 KiB
| name | classification | success_metric | skip | status | created | shipped | |||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| fix-auth-bypass | server-only | getUserFromRequest returns null for missing tokens; no API route accepts unauthenticated requests; CI green. |
|
shipped | 2026-05-22 | 2026-05-23 |
Convoy: fix-auth-bypass
Closes P0 ship-blockers #1, #2, #4, #5, and #6 (partial) from .convoys/ship-readiness.md. This is the very first real convoy after the bootstrap and gates the rest of the launch sequence — until it lands, every other production-bound PR is paused.
Why
The current lib/permission-middleware.js::getUserFromRequest returns a hardcoded admin user ({ id: 1, role: 'admin', email: 'admin@tcgvault.com' }) when no Authorization header is present. Every API route that calls it (30+ handlers per user-code-review-graph) therefore accepts unauthenticated requests as admin. Combined with:
- A weak fallback
JWT_SECRET('your-secret-key-change-in-production') duplicated across 7 files, - Four dev-only endpoints (
/api/simple,/api/test-auth,/api/test-db,/api/setup-database) shipped inpages/api/, Access-Control-Allow-Origin: *on auth endpoints,- Zero rate limiting on login,
…the production URL is effectively wide-open. No anonymous traffic can touch the live site until this convoy ships.
Success looks like:
getUserFromRequestreturnsnullwhen there is no Bearer token. Period. No callers receive a synthetic admin.- There is exactly one source of truth for the JWT secret. If
process.env.JWT_SECRETis unset, the server fails to boot with a clear error — not a silent fallback. - The four dev endpoints are gone, and CI fails the build if they reappear.
- The login + register endpoints respond only to the production frontend origin (or no CORS header at all on same-origin Vercel deploy).
- Login + register are rate-limited (the bare minimum of P0 #6; the rest is
add-rate-limiting). - CI is green (lint + the new auth tests).
Scope
In:
lib/permission-middleware.js— remove hardcoded admin fallback; returnnullon missing/invalid token.- New
lib/auth-secret.js(or named equivalent — Architect to confirm) — single export ofJWT_SECRET, throws at module load if unset. - Refactor
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, andlib/permission-middleware.jsto import from the new secret helper. Remove allprocess.env.JWT_SECRET || '…'literals. - Reconcile token expiry inconsistency (login = 24h, auth-utils = 7d). Pick one — Architect's call; record in
.cursor/rules/auth-and-permissions.mdc. - Delete
pages/api/simple.js,pages/api/test-auth.js,pages/api/test-db.js,pages/api/setup-database.js. - Add a CI grep step to
.github/workflows/ci.ymlthat fails the build ifpages/api/test-*,pages/api/simple.js, orpages/api/setup-database.jsever re-appear. - Tighten
Access-Control-Allow-Originonpages/api/auth/login.jsandpages/api/auth/register.js. Default: drop the header entirely (same-origin on Vercel). Fallback: pin to aprocess.env.PUBLIC_FRONTEND_ORIGINenv var. - Adopt
@upstash/ratelimit(or equivalent — Architect's pick) and apply to/api/auth/loginand/api/auth/registeronly. Other endpoints listed in P0 #6 (search, imports, avatar upload) are deferred to theadd-rate-limitingconvoy. - Add unit tests for
getUserFromRequest: missing header →null, malformed token →null, valid token → user object, expired token →null. Architect to decide whether to land this withvitestnow or defer to theadopt-vitestconvoy. Default recommendation: install vitest in this convoy. The blast radius of an auth refactor justifies bringing the test runner forward by one slot in the launch sequence.
Out (deferred to their own convoys):
- P0 #3 (default admin creds + README) →
drop-public-setup. - P0 #7 (Layout default-prop leaks maintainer email) →
fix-layout-default-user. - P0 #6 (full) — rate limit on search / import / upload routes →
add-rate-limiting. - Any auth-context client-side cleanup (
lib/auth-context.jsvslib/admin-auth.jsvslib/use-auth.js) →single-auth-provider. - The
lib/database.jsvs@vercel/postgresreconciliation →single-sql-client.
Hard "do not touch" in this convoy:
- No UI files. No
components/, nopages/*.jsthat aren't underpages/api/. If a UI file appears in a brief, kick it back to Architect. - No schema changes. No SQL migrations. (
scripts/setup-neon-db.jsis read-only here.) - No new feature flags. The flag wrapper exists (
lib/flags/index.js) but this convoy is unflagged — auth fixes don't ship behind a flag.
Roles invoked
Per server-only classification (skip: ia, ux, visual, a11y, design):
- role-architect — produces a slice plan with explicit
slice_dependencies:. Expect 4–6 briefs (auth-secret helper, getUserFromRequest fix + caller audit, dev-endpoint removal + CI guard, CORS tighten, rate-limit pattern + login/register wiring, tests). Architect must declare which briefs are parallel-safe. - role-implementer — runs one brief at a time, except where Architect marks
depends_on: []andfiles:are disjoint. Then/multitaskcan fan out (see dispatch below). - role-reviewer — single-pass after the PR drafts. Design-system-auditor and a11y-auditor are skipped for this convoy — there is no UI surface to audit. Reviewer covers correctness, security regressions, and test coverage.
- role-doc-writer — last. Updates
.cursor/rules/auth-and-permissions.mdc(canonical secret helper, chosen expiry, rate-limit pattern),AGENTS.md"Common gotchas" section (remove items that are no longer gotchas), anddocs/SCHEMA_MAP.mdonly if any DB read pattern changed (it shouldn't).
Multitask dispatch recommendation (Cursor 3.2 /multitask): after Architect publishes briefs with depends_on: [] and disjoint files:, the user may dispatch implementers in parallel. Typical safe fan-out for this convoy:
- Group
audit-fix-auth-bypass-<pr>:role-revieweronly (no design / a11y). - Implementer fan-out: only if Architect explicitly marks briefs as parallel-safe. The auth-secret helper brief must complete first; everything else depends on it. So realistic fan-out is post-secret-helper: dev-endpoint deletion + CORS tighten + rate-limit wiring in parallel;
getUserFromRequestfix runs alongside but its tests block on the secret helper landing first.
Todos
High-level checklist for the next role to refine. Each becomes a brief under .convoys/fix-auth-bypass/brief-N-*.md.
- Brief 1 — Central JWT secret helper. Create
lib/auth-secret.js, fail-loud on missing env. Decide canonical token TTL. - Brief 2 — Remove the admin bypass. Fix
getUserFromRequest; audit every caller (user-code-review-graphquery: incoming edges tolib-admin::getUserFromRequest). Add unit tests covering missing/invalid/expired/valid token paths. - Brief 3 — Delete dev-only endpoints. Remove four files; add CI guard.
- Brief 4 — Tighten auth CORS. Drop
Access-Control-Allow-Origin: *on login + register. Add same-origin fallback via env var. - Brief 5 — Rate-limit login + register. Install
@upstash/ratelimit(or Architect-chosen alternative). Wire to login + register only. Defer the full sweep toadd-rate-limiting. - Brief 6 — Test harness (provisional). Install
vitest, write thegetUserFromRequestsuite, re-enable thetest:job in.github/workflows/ci.yml. Architect to confirm whether this is in-scope here or split toadopt-vitest. - Doc-writer pass. Update auth rules + AGENTS.md gotchas.
Hand-off
Next role: role-architect.
To run it in a new chat, paste:
"Run role-architect on convoy
fix-auth-bypass. Read.convoys/fix-auth-bypass.mdfor scope and todos, then produce a slice plan with explicitslice_dependencies:. Output briefs to.convoys/fix-auth-bypass/brief-N-*.md. Flag which briefs are parallel-safe so the user can/multitaskimplementers."
Conductor exits here. Human-in-the-loop gate: review the convoy file, confirm the scope split, then start the Architect.
Architecture
Architect: role-architect. Date: 2026-05-23. Convoy decomposed into 5 briefs (down from the conductor's 6 candidates — Brief 4 "CORS tighten" and Brief 5 "rate-limit" are merged into a single Brief 4 because they share pages/api/auth/login.js + pages/api/auth/register.js and would otherwise serialize against each other).
File plan
| File | Action | Brief | Purpose |
|---|---|---|---|
lib/auth-secret.js |
new | 1 | Single source of truth for JWT_SECRET (fail-loud) + canonical JWT_TOKEN_TTL = '24h'. |
lib/permission-middleware.js |
modified ×2 | 1, 2 | Brief 1 swaps the JWT_SECRET literal for an import; Brief 2 removes the synthetic-admin fallback in getUserFromRequest. |
pages/api/auth-utils.js |
modified | 1 | Literal → import; '7d' → JWT_TOKEN_TTL. Becomes the canonical generateToken / verifyToken site. |
pages/api/auth/login.js |
modified ×2 | 1, 4 | Brief 1: literal → import, inline jwt.sign → generateToken. Brief 4: drop CORS-*, add rate-limit gate. |
pages/api/auth/register.js |
modified ×2 | 1, 4 | Same as login. |
pages/api/auth/verify.js |
modified ×2 | 1, 2 | Brief 1: literal → import. Brief 2: remove the no-token admin-fetch branch (returns 401 instead). |
pages/api/favorites.js |
modified | 1 | Literal → import. |
pages/api/users/search.js |
modified | 1 | Literal → import. |
pages/api/simple.js |
deleted | 3 | Dev endpoint, unauthenticated, no runtime references. |
pages/api/test-auth.js |
deleted | 3 | Dev endpoint, leaks token-handling internals. |
pages/api/test-db.js |
deleted | 3 | Dev endpoint, exposes DB connection metadata. |
pages/api/setup-database.js |
deleted | 3 | Public unauthenticated DDL trigger. |
lib/rate-limit.js |
new | 4 | Lazy-init @upstash/ratelimit wrapper with prod fail-closed + dev no-op fallback. |
package.json |
modified ×2 | 4, 5 | Brief 4: add @upstash/ratelimit + @upstash/redis. Brief 5: add vitest devDep + test / test:run scripts. |
package-lock.json |
modified ×2 | 4, 5 | Regenerated by npm install in each. |
vitest.config.js |
new | 5 | Node env, test/**/*.test.js, test/setup.js setupFile. |
test/setup.js |
new | 5 | Sets JWT_SECRET=test-… and NODE_ENV=test before any module loads. |
test/lib/auth-secret.test.js |
new | 5 | 3 tests: exports + fail-loud throw. |
test/lib/permission-middleware.test.js |
new | 5 | 8 tests covering Brief 2's null-return contract (incl. negative regression against the synthetic-admin shape). |
test/api/auth-utils.test.js |
new | 5 | 5 tests covering generateToken / verifyToken round-trip + 24h TTL. |
.github/workflows/ci.yml |
modified ×2 | 3, 5 | Brief 3: add forbidden-endpoints job (blocking). Brief 5: re-enable the disabled test: job, remove the "no test runner" comment header. |
README.md |
modified | 3 | Remove the GET /api/test-db line from the API list. |
Note the ×2 markers — those files have two briefs editing them in sequence. The slice_dependencies graph below sequences them so no two parallel writers ever target the same file.
API surface
No new routes. Modified routes:
| Method | Path | Auth | Brief | Notes |
|---|---|---|---|---|
POST |
/api/auth/login |
none (auth-emitting) | 1, 4 | Brief 1: token-mint refactor (no behavior change). Brief 4: drops CORS-*, adds rate-limit (5/15min/IP). On limit: 429 + Retry-After header. |
POST |
/api/auth/register |
none | 1, 4 | Same as login. |
GET |
/api/auth/verify |
Bearer (now required) | 1, 2 | Brief 1: secret-import refactor. Brief 2: returns 401 instead of fetching admin@tcgvault.com when no Bearer header. |
GET / POST / DELETE |
/api/favorites |
Bearer | 1 | Secret-import refactor only. |
GET |
/api/users/search |
Bearer | 1 | Secret-import refactor only. |
Deleted routes (no replacement, no redirect):
| Method | Path | Brief |
|---|---|---|
GET / POST |
/api/simple |
3 |
GET |
/api/test-auth |
3 |
GET |
/api/test-db |
3 |
POST |
/api/setup-database |
3 |
Request validation: no new schema validator (no zod/yup) added in this convoy — the existing manual validation in each handler stays. Validator adoption is its own future convoy.
Schema diff
No schema change. No SQL migration. No edits to scripts/setup-neon-db.js or docs/SCHEMA_MAP.md. The convoy is hard-scoped against schema changes.
The seed user (admin@tcgvault.com, password admin123) is not removed by this convoy — that is the future drop-public-setup convoy. Brief 2 only stops verify.js from auto-fetching that row; the row itself remains.
Test plan
- Brief 5 ships the harness (vitest@^3.2.4, plain JS) and 16 unit tests:
- 3 tests:
lib/auth-secret.js(exports + fail-loud throw on missing env). - 8 tests:
lib/permission-middleware.js::getUserFromRequest(missing header / non-Bearer / malformed / wrong-secret / expired / valid-but-no-row / valid + happy-path / negative regression against synthetic-admin shape). - 5 tests:
pages/api/auth-utils.js(generateToken24h TTL + payload +verifyTokenround-trip + bad-signature + malformed).
- 3 tests:
- No integration tests (
pages/api/auth/login.jsend-to-end). Deferred to a follow-up convoy that adoptssupertestor Playwright. - No tests for
lib/rate-limit.js. The lazy-init + fail-open + fail-closed branches need an Upstash mock; deferred to a follow-up. - CI integration: Brief 5 re-enables
.github/workflows/ci.yml'stest:job (commented out at lines 87-103 today). The job runs on every PR and push tomain, blocking on failure. - Existing test files to use as examples: none — this is the first test infra in the repo. The closest reference is the
bump-next-jsconvoy retro, which documents the JS-only constraint.
Risk list
This is a security-critical convoy; the risks are higher than bump-next-js.
- R1 — JWT_SECRET fail-loud breaks anything that imports
lib/auth-secret.jsat module-load time without the env var set. Includes: any future test, any futurenpm run setup-dbor import script that transitively imports auth code, and any newpages/_app.js-time import. Mitigation: none of the in-scope auth files are imported at build time (Pages Router serverless functions are imported per-request);next buildshould not trip the throw. Verification: Brief 1's smoke step explicitly testsnpm run devwithJWT_SECRETunset and confirms the error message is clear. Brief 5'stest/setup.jssetsJWT_SECRETbefore any test imports auth code. - R2 — Removing the synthetic-admin fallback may break a caller that secretly relies on it. Mitigation: Brief 2 spot-checks all 24 callers; the architect verified that 23/24 use
if (!user) return 401and the 1 exception (pages/api/collections/[identifier].js) usesuser?.userIdoptional-chaining and works correctly whenuserisnull. Residual risk: any caller added between architect's audit (commitebd4fd1) and Brief 2's merge could regress. Mitigated by including the spot-check command in Brief 2's acceptance criteria so the implementer re-runs the grep at PR open time. - R3 — Existing logged-in users hold tokens signed against the literal fallback secret (
'your-secret-key-change-in-production'). Once Brief 1 lands andJWT_SECRETis required to be set in prod, those tokens stop verifying becausejwt.verify(token, REAL_SECRET)will reject them. Mitigation: the deploy plan should announce a "you'll need to log back in" notice. There is no graceful migration; the alternative (accept either secret for a transition window) is exactly the bypass we are trying to remove. The blast-radius is acceptable because the user base is currently small (pre-launch). - R4 — Token TTL change from
7d(inauth-utils.generateToken) to24h. No user is currently affected becauseauth-utils.generateTokenwas not in the call path —login.jsandregister.jsdid inlinejwt.sign. Net effect: users continue to get the 24h tokens they already had; the TTL drift inauth-utilsis fixed in the same direction. - R5 — Rate-limit picks the wrong identifier on Vercel.
req.headers['x-forwarded-for']is set by Vercel's proxy and includes a chain when behind multiple hops; the first IP is the client. Mitigation: Brief 4 specifies the first-hop extraction explicitly. Residual risk: if Vercel ever changes its forwarding chain, the limit-key changes too. Verification: the smoke step in Brief 4 confirms the rate-limit fires on a real Vercel preview. - R6 — Upstash quota exhaustion. Free tier is 10k commands/day. Each login costs ~1 command (sliding-window read+write batched). At 10k logins/day the limiter starts failing. Mitigation: Brief 4's
lib/rate-limit.jsfail-opens on Upstash error (singleconsole.error). Defense-in-depth via Vercel firewall is a future hardening pass. - R7 —
package.json/package-lock.jsonmerge conflicts between Brief 4 and Brief 5. Both touch the lockfile. Mitigation: slice_dependencies sequences Brief 5 after Brief 4 (depends_on: [1, 2, 4]); the implementer for Brief 5 rebases onto Brief 4's main commit, not onto pre-Brief-4 main. - R8 —
@upstash/ratelimit@2.0.8introduces a transitive that conflicts with our existing@vercel/postgres@0.10.0or@neondatabase/serverless@1.0.1. Mitigation: the architect rannpm view @upstash/ratelimit dependenciesandnpm view @upstash/redis dependencies(sole new transitives:uncrypto@^0.1.3,crypto-js-style one-file modules). No overlap with the existing tree. Residual risk:npm installcould surface a peer-dep warning we missed. Brief 4 acceptance criterion makes the implementer report the install output. - R9 — vitest@3.2.4 transitively pulls in
vite@5/6/7, which has a Node engines requirement of^20.19 || >=22.12. Vercel's CI runs Node 20 (set inci.yml'sNODE_VERSION: '20', whichactions/setup-node@v4resolves to the latest 20.x patch — currently>=20.19). Verification: the existingbump-next-jsconvoy's brief #1 already documents this constraint and Vercel's runtime satisfies it. Local-dev developers on Node 20.0–20.18 will see vitest fail at install time; mitigation is to bump local Node to 20.19+, which is already the existing recommendation. - R10 — JWT-secret rotation is now coupled to a redeploy. Pre-fix, rotating the env var was a no-op (the fallback string was used regardless). Post-fix, an unset env var means the server refuses to boot. Mitigation: documented in Brief 4's pre-deploy checklist; the fix is to set
JWT_SECRETin Vercel before merging. - R11 — Test-mock drift.
test/lib/permission-middleware.test.jsmocks@vercel/postgres. If a future convoy migrates the file to@neondatabase/serverlessor another client, the mock won't fire and tests pass without exercising the real path. Mitigation: the mock target is documented in Brief 5's acceptance criteria; the future-migration convoy must also update the mock. - R12 — CI guard regex misses a renamed dev endpoint. Brief 3's
forbidden-endpointsjob checks 4 explicit paths plusfind pages/api -name 'test-*.js'. If someone re-introduces a dev endpoint aspages/api/debug.jsorpages/api/internal/health.js, the guard misses it. Mitigation: intentional — the guard is a regression-prevention belt for the four known files, not a general "no dev endpoints" policy. Adding a stricter check (e.g. require all public endpoints to import an auth helper) is a future hardening convoy.
Decomposition
| Brief # | Title | Files | Depends on | Estimated PR size |
|---|---|---|---|---|
| 1 | Central JWT secret helper + 24h token TTL | lib/auth-secret.js (new), 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 |
— | ~120 LOC (mostly mechanical import refactor across 7 files + 12-line new helper) |
| 2 | Remove the synthetic-admin bypass | lib/permission-middleware.js, pages/api/auth/verify.js |
1 | ~25 LOC (net-negative; deletes the dev-fallback branches) |
| 3 | Delete dev-only endpoints + CI guard | .github/workflows/ci.yml, README.md (modified); pages/api/simple.js, pages/api/test-auth.js, pages/api/test-db.js, pages/api/setup-database.js (deleted) |
— | ~30 LOC (one CI job + 4 deletions + 1 README line) |
| 4 | Tighten the public auth surface (CORS + rate limit) | package.json, package-lock.json, lib/rate-limit.js (new), pages/api/auth/login.js, pages/api/auth/register.js |
1 | ~150 LOC (rate-limit module ~70, two handler edits ~40, package.json/lock ~40) |
| 5 | Install vitest + auth tests + re-enable CI test job | package.json, package-lock.json, vitest.config.js (new), test/setup.js (new), test/lib/auth-secret.test.js (new), test/lib/permission-middleware.test.js (new), test/api/auth-utils.test.js (new), .github/workflows/ci.yml |
1, 2, 4 | ~280 LOC (16 test cases dominate; vitest config + setup + CI YAML are small) |
All five briefs are under the 400-LOC budget. Brief 5 is the largest by LOC but the lowest by complexity (test boilerplate).
Slice dependencies (multitask-ready)
slice_dependencies:
- brief: 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
- brief: 2
depends_on: [1]
files:
- lib/permission-middleware.js
- pages/api/auth/verify.js
- brief: 3
depends_on: []
files:
- .github/workflows/ci.yml
- README.md
- pages/api/simple.js
- pages/api/test-auth.js
- pages/api/test-db.js
- pages/api/setup-database.js
- brief: 4
depends_on: [1]
files:
- package.json
- package-lock.json
- lib/rate-limit.js
- pages/api/auth/login.js
- pages/api/auth/register.js
- brief: 5
depends_on: [1, 2, 4]
files:
- package.json
- package-lock.json
- vitest.config.js
- test/setup.js
- test/lib/auth-secret.test.js
- test/lib/permission-middleware.test.js
- test/api/auth-utils.test.js
- .github/workflows/ci.yml
Multitask fan-out plan (3 waves):
- Wave A (concurrent): Briefs 1 + 3. Files are completely disjoint. Two implementers can run side-by-side.
- Wave B (concurrent, after Brief 1 merges): Briefs 2 + 4. Both depend on Brief 1's secret-helper landing first. Their
files:sets overlap only on files Brief 1 already published, and they touch disjoint subsets of those files (Brief 2 →permission-middleware.js+verify.js; Brief 4 →login.js+register.js). - Wave C (single, after Brief 2 + Brief 4 merge): Brief 5.
depends_on: [1, 2, 4]because the tests cover Brief 2's behavior and the lockfile sits on top of Brief 4'snpm install.
/multitask dispatch suggestion when the human approves the plan:
/multitask
- impl-1: role-implementer brief=1 from .convoys/fix-auth-bypass/brief-1-central-jwt-secret-helper.md
- impl-3: role-implementer brief=3 from .convoys/fix-auth-bypass/brief-3-delete-dev-endpoints.md
Then after Wave A merges:
/multitask
- impl-2: role-implementer brief=2 from .convoys/fix-auth-bypass/brief-2-remove-admin-bypass.md
- impl-4: role-implementer brief=4 from .convoys/fix-auth-bypass/brief-4-tighten-auth-surface.md
Then Brief 5 alone.
Architect's calls (decisions made during this pass)
- Token TTL: 24h. Matches current
login.jsuser experience (no session-length regression for existing users) and is the more security-conservative choice over the unusedauth-utils.generateToken's'7d'default. Codified asJWT_TOKEN_TTL = '24h'inlib/auth-secret.js. - Rate-limit library:
@upstash/ratelimit@^2.0.8+@upstash/redis@^1.38.0. DIY-Postgres was rejected (would require schema changes — out of scope). DIY-in-memory was rejected (broken on Vercel cold starts).next-rate-limitwas rejected (stale, in-memory, same cold-start issue).@upstash/ratelimitis the only mature serverless-native option. Cost: free tier (10k commands/day) is sufficient for current traffic. - Brief 6 (vitest): in-scope, not split. The convoy file's default recommendation stands — auth's blast radius justifies bringing the test runner forward by one slot. Pinned to vitest@^3.2.4 (not v4) because v4 makes
vitea non-optional peer-dep, which would inflate this JS-only repo's dep tree without benefit. Renumbered as Brief 5 in the final decomposition. - Briefs 4 + 5 from the original conductor draft (CORS + rate-limit) merged into a single Brief 4. Both edit
pages/api/auth/login.jsandpages/api/auth/register.js. Splitting them would force serial execution; merging them ships the public-auth-surface tightening as one cohesive PR. - Brief 6 from the original draft (vitest) is now Brief 5. Total brief count: 5.
pages/api/auth/verify.jsCORS is NOT tightened in this convoy. Convoy explicitly scopes Brief 4 to login + register. Verify-CORS is deferred tocors-tightenoradd-rate-limiting. Documented as out-of-scope in Brief 2 and Brief 4.
Boot-the-brief findings
The architect ran the verification pass before declaring complete. Findings:
@upstash/ratelimit@2.0.8peer dep verified.npm view @upstash/ratelimit peerDependencies→{ '@upstash/redis': '^1.34.3' }. Pin both@upstash/ratelimit@^2.0.8and@upstash/redis@^1.38.0in Brief 4's package.json change. Confirmed that@upstash/redis@1.38.0falls within the peer range.@upstash/redis@1.38.0transitive surface verified. Sole production dep:uncrypto@^0.1.3(a single-file polyfill for Node'swebcrypto— pure-JS, ~50 SLOC). No conflict with the existing dep tree.- vitest@4 vs vitest@3 peer-dep delta. vitest@4.1.7 lists
viteas a non-optional peer dep (range^6 || ^7 || ^8); vitest@3.2.4 listsviteas a regular dep (range^5 || ^6 || ^7). For a JS-only repo with no Vite plugins, v3.2.4 is strictly easier — no extraviteinstall, no peer-dep conflict. Brief 5 pinsvitest@^3.2.4. Documented in Brief 5's acceptance criterion + rationale. - vitest@3's vite dep has Node
^20.19 || >=22.12. Vercel CI'ssetup-node@v4withnode-version: '20'resolves to latest 20.x patch (currently 20.19+); satisfies the requirement. Local-dev users on Node <20.19 will need to upgrade — already the recommendation per the bump-next-js retro. - Caller audit of
getUserFromRequest. Architect ranrg "getUserFromRequest" pages/api --type js -l→ 24 files. Sampled 22 of them withrg "if \(!user\)" pages/api --type js -A 1and confirmed all 22 use theif (!user) return res.status(401)pattern. The 23rd (pages/api/community/collections.js) and 24th (verified in spot-check above) use the same pattern. The one exception ispages/api/collections/[identifier].jswhich usesuser?.userIdoptional-chaining — confirmed correct under the post-Brief-2 null return. No caller code change is needed in this convoy. - JWT_SECRET literal sites confirmed: 7 files. Matches AGENTS.md gotcha #3 exactly:
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. Brief 1's grep verification will guarantee all 7 are converted. - Dev endpoints have no runtime references.
rg "/api/(simple|test-auth|test-db|setup-database)"returns hits only in docs (.cursor/rules/api-routes.mdc,AGENTS.md,.convoys/,README.md) and one CODEOWNERS line. Safe to delete; the README line is also removed in Brief 3. - Cross-brief commitments documented in both directions. Brief 1 declares commitments to Briefs 2, 4, 5. Briefs 2, 4, 5 each declare reciprocal commitments back to Brief 1. Brief 5 also declares a commitment from Brief 2 (test coverage of Brief 2's null-return contract) and a coordination note from Brief 4 (lockfile sequencing). All round-trip.
- No verbatim code-shape mismatches found. The proposed
lib/auth-secret.js,lib/rate-limit.js,vitest.config.js, and CI YAML shapes were checked against the actual installedpackage.json, the existing CI workflow'slint:job style, and the@upstash/ratelimitREADME's verbatimRatelimit.slidingWindow(N, '<duration>')API. No discrepancies.
No brief was revised during the Boot-the-brief pass — all proposed shapes survived first-contact verification.
Convoy outcome
Closed out 2026-05-23 by role-doc-writer after the final brief merged to main (PR #11, commit 1629afb). The convoy completed its declared success criteria: getUserFromRequest returns null for missing tokens, every API route on the auth surface refuses unauthenticated requests, login + register are rate-limited, and CI is green with a blocking test: job.
What shipped
| Brief | Title | Merge commit | Files | Outcome |
|---|---|---|---|---|
| 3 | Delete dev endpoints + CI guard | fc0dd73 |
4 deletions in pages/api/; .github/workflows/ci.yml (new forbidden-endpoints job); README.md |
Wave A, parallel with Brief 1. Resolves AGENTS.md Gotcha #5. |
| 1 | Central JWT secret helper + 24h TTL | 4a10dce |
lib/auth-secret.js (new); 7 sites of literal-fallback removal |
Wave A. Resolves AGENTS.md Gotcha #3. |
| 2 | Remove synthetic-admin bypass | 258e479 |
lib/permission-middleware.js, pages/api/auth/verify.js |
Wave B, parallel with Brief 4. Keystone fix — resolves AGENTS.md Gotcha #2. |
| 4 | Tighten public auth surface (CORS + rate limit) | 297afca |
lib/rate-limit.js (new); pages/api/auth/login.js, pages/api/auth/register.js; package.json + package-lock.json |
Wave B. Drops Access-Control-Allow-Origin: '*' from login + register; 5/15min sliding window via @upstash/ratelimit. |
| 6 (hotfix) | Cards-collection 401 guards | 1fca3aa |
pages/api/collections/[identifier]/cards.js |
Out-of-plan follow-up to Brief 2 — three if (!user) return 401 guards added to the POST/PUT/DELETE branches that previously relied on a 500 cascade. |
| 5 | Vitest harness + 16 auth unit tests + re-enable CI test job | 1629afb |
vitest.config.js, test/setup.js, test/lib/*.test.js, test/api/*.test.js; package.json + package-lock.json; .github/workflows/ci.yml |
Wave C (last). Locks in Brief 1 + Brief 2 contracts. |
Total: 5 planned briefs + 1 hotfix, all merged across PRs #6–#11. The convoy file's "Decomposition" table predicted ~600 LOC; actuals were within budget on every brief.
AGENTS.md gotchas resolved
- #2 —
getUserFromRequestsynthetic-admin fallback → resolved by Brief 2 (258e479). Converted to a "Resolved" note inAGENTS.md(not renumbered) so cross-references stay valid. - #3 — JWT_SECRET hardcoded across 7 files → resolved by Brief 1 (
4a10dce). Converted to a "Resolved" note inAGENTS.md. - #5 —
pages/api/setup-database.jspublic endpoint → resolved by Brief 3 (fc0dd73). Converted to a "Resolved" note inAGENTS.md; the CIforbidden-endpointsjob prevents regression.
Gotchas #1, #4, #6, #7, #8, #9, #10, #11 are unchanged (see those entries in AGENTS.md for status). A new Gotcha #12 was added covering the KV_REST_API_* env-var convention (see "Mid-flight deviation" below).
Mid-flight deviation: env-var rename
The convoy plan and Brief 4 originally specified UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN — the names @upstash/redis's generic README example uses. Mid-implementation, the Brief 4 implementer surfaced that the project already runs on Vercel's Upstash Marketplace integration, which auto-provisions Redis credentials under KV_REST_API_URL / KV_REST_API_TOKEN. A parent-agent interrupt approved the rename ("ship what Vercel hands you"), and the implementer continued with the Marketplace-native names.
Net effect: zero manual env-var paste step on any environment; cleanup of three otherwise-redundant KV_* shadows that would have pointed at the same Upstash instance under different keys. See .convoys/fix-auth-bypass/brief-4-tighten-auth-surface.md § "Post-merge addendum" for the full record.
Risk-list status (R1–R12)
Walking each risk from § Architecture → Risk list:
| Risk | Status | Note |
|---|---|---|
R1 — JWT_SECRET fail-loud breaks unexpected importers |
Addressed | Brief 1 verified next dev + next build both clear with JWT_SECRET set; Brief 5's test/setup.js sets it before any auth import. No regression observed. |
| R2 — Caller relies on synthetic-admin fallback | Addressed | All 24 getUserFromRequest callers verified to handle null correctly (23 use if (!user) return 401; one uses user?.userId optional-chaining). Plus the Brief 6 hotfix added explicit null guards to the cards-collection routes that were silently 500'ing. |
| R3 — Existing tokens stop verifying | Addressed (accepted) | One-time "you'll need to log back in" is the expected outcome. Acceptable pre-launch. No graceful migration was offered (it would have meant accepting both secrets, which is the bypass we removed). |
R4 — Token TTL drift 7d → 24h |
Addressed | Codified as JWT_TOKEN_TTL = '24h' in lib/auth-secret.js; no user impact because the 7d site (auth-utils.generateToken) wasn't in the call path pre-fix. |
| R5 — Rate-limit picks wrong identifier on Vercel | Addressed | First-hop XFF extraction implemented per the spec; Brief 4's smoke confirmed real-Vercel preview behavior. |
| R6 — Upstash quota exhaustion | Partial | lib/rate-limit.js fail-opens on Upstash error per spec. Quota monitoring + defense-in-depth (Vercel firewall) is deferred to a future hardening pass. |
R7 — package.json / package-lock.json merge conflicts |
Addressed | Wave-C sequencing (Brief 5 after Brief 4) prevented any actual conflict. |
R8 — @upstash/ratelimit transitive surprises |
Addressed | npm install clean; npm ls @upstash/ratelimit and npm ls @upstash/redis each report a single version. No ERESOLVE. |
R9 — vitest@3.2.4 Node engines constraint |
Addressed | Vercel CI Node 20.x satisfies `^20.19 |
R10 — JWT_SECRET rotation now requires a deploy |
Addressed (accepted) | Documented in Brief 1's pre-deploy checklist + AGENTS.md § 1. The fail-loud throw is the desired property. |
| R11 — Test-mock drift if SQL client migrates | Deferred | Documented in Brief 5's acceptance criteria; the future single-sql-client convoy must update test/lib/permission-middleware.test.js's @vercel/postgres mock. |
| R12 — CI guard misses a renamed dev endpoint | Partial (accepted) | Intentional — forbidden-endpoints is a regression-prevention belt for the four known files, not a general "no dev endpoints" policy. A stricter check is deferred to a future hardening pass. |
No risks fired during the convoy. The closest call was R2 — the cards-collection routes (pages/api/collections/[identifier]/cards.js POST/PUT/DELETE) had a pre-existing reliance on the synthetic admin that surfaced as 500s after Brief 2 instead of clean 401s. The Brief 6 hotfix addressed it in 1fca3aa, before the convoy closed.
Follow-up convoys queued
These are tracked here (rather than only in .convoys/ship-readiness.md) because they are direct continuations of the auth surface the convoy hardened:
expand-auth-tests— rate-limit unit tests (mock@upstash/redis), login/register integration tests covering CORS-gone + 429 path, broader Vitest coverage of thewithCollectionPermissionwrapper. Brief 5 explicitly deferred these.cors-tighten—pages/api/auth/verify.jsstill shipsAccess-Control-Allow-Origin: '*'(Brief 4 was scoped to login + register only, per § Architecture). Also covers any other CORS-*sites elsewhere inpages/api/.drop-public-setup— closes Gotcha #4 (default admin credentials in seed) plus any residual README copy on admin bootstrap.fix-layout-default-user— closes Gotcha #8 (Layout default user impersonates maintainer).single-sql-client— closes Gotcha #1 (two SQL clients in parallel). Deferred from this convoy's scope ("Out") per the original plan.migration-tool— closes Gotcha #6 (bare migration scripts). Unscoped, still open.dual-is-public— closes Gotcha #7 (dualis_publicsemantics on collections + decks). Unscoped, still open.
The full launch sequence remains as enumerated in .convoys/ship-readiness.md § "Proposed launch sequence" — fix-auth-bypass is step 1; the follow-ups above are interleaved across steps 2–7.
Lessons learned
- Worktree-based parallel implementer dispatch worked well. Wave A (Briefs 1 + 3) and Wave B (Briefs 2 + 4) each ran two implementers concurrently in separate worktrees with zero merge conflict. The architect's
slice_dependenciestable was an unusually accurate guide — thefiles:sets really were disjoint within each wave. Worth keeping as the default decomposition discipline. - Mid-flight interrupt for the env-var rename was the right call. Stopping the Brief 4 implementer, raising the discrepancy to the parent agent, and continuing with the corrected names cost ~5 minutes and avoided shipping a duplicate env-var pair plus a Vercel-onboarding wiki page. The convoy's audit trail (this section + the brief's "Post-merge addendum") preserves the reasoning for future agents.
- Per-brief PRs gave better review density than the single-PR pattern from
bump-next-js. Each PR was small enough that human review fit in one sitting (~10–20 min), and a single brief's diff was easy to reason about in isolation. The trade-off (5+ PRs instead of 1) was worth it for a security-critical convoy where review attention is the limiting factor.