fix(auth): remove synthetic-admin bypass (Brief 2 of fix-auth-bypass) #8

Merged

varutasu merged 1 commit from brief/fix-auth-bypass/2-remove-admin-bypass into main

2026-05-23 11:57:24 -04:00

varutasu commented

2026-05-23 11:47:50 -04:00

(Migrated from github.com)

Summary

Convoy fix-auth-bypass / Brief 2 of 5. This is the keystone PR of the convoy — it closes AGENTS.md gotcha #2 (the actual auth bypass). PR #6 (Brief 1) only renamed the secret; this PR removes the bypass behavior.

lib/permission-middleware.js::getUserFromRequest no longer returns a hardcoded { userId: 1, email: 'admin@tcgvault.com', role: 'admin' } when the Authorization header is missing or malformed. It now returns null, and the 24 callers across pages/api/ correctly handle the null per the if (!user) return res.status(401) pattern documented in .cursor/rules/auth-and-permissions.mdc.
pages/api/auth/verify.js no-token branch returns 401 instead of fetching the seed admin row via WHERE email = 'admin@tcgvault.com'. The admin-record leak side of the same bypass is closed.
No NODE_ENV === 'development' env-gate. No console.warn. No commented-out fallback "for later." The bypass is gone, period. If a developer needs an authenticated session locally, they log in.

Files changed

lib/permission-middleware.js — 4-line synthetic admin block → 1-line return null;
pages/api/auth/verify.js — 12-line no-token admin-fetch branch → 3-line 401 response

Net diff: 2 files, +2 / −16 LOC.

Test plan

npm run build exits 0
npm run lint matches baseline (128 problems / 81 errors / 47 warnings); zero new lint issues
git grep -E \"userId: 1|admin@tcgvault\.com\" against lib/ + pages/ returns ZERO hits (only scripts/setup-neon-db.js still seeds that email, which is out of scope — gotcha #4's drop-public-setup convoy)
git grep \"Development mode\" -- 'lib/' 'pages/' returns ZERO hits
All 24 callers of getUserFromRequest verified to handle null correctly with the if (!user) return res.status(401) shape — except the one in Known follow-up below
Reviewer to verify on preview deployment: curl -X GET https://<preview>/api/users/search?q=foo (no Authorization header) returns 401, not admin data. curl -X GET https://<preview>/api/auth/verify returns 401, not the admin record.

⚠️ Known follow-up (NOT addressed in this PR, intentional)

pages/api/collections/[identifier]/cards.js POST/PUT/DELETE handlers dereference user.userId without a null guard. Previously masked by the synthetic admin (anonymous-write-as-admin on collections owned by userId: 1 was the security hole). After this PR, those handlers degrade to NPE → 500 instead of a clean 401.

Security: improved either way. No more anonymous writes.
UX/error format: slightly worse until we add the guard.
Fix: one-line follow-up PR (add if (!user) return res.status(401)... to each of the three handlers). I'll dispatch this as a separate small patch after this PR merges. It does NOT block this PR — the bypass is closed; the cosmetic 500-vs-401 is a strict improvement over the previous status quo (anonymous-write-as-admin).

Pre-merge requirements

None directly. (Brief 1's JWT_SECRET requirement already satisfied — Brief 1 has merged.)
After this merges, the admin must log in via /api/auth/login to use admin features. The fallback that previously let unauthenticated admin actions through is gone.

Out of scope (intentionally — see convoy plan)

The cosmetic 500-vs-401 in cards.js → small follow-up PR after this merges
pages/api/admin/index.js uses verifyToken directly (not getUserFromRequest) — unaffected by this brief
CORS tightening + rate limiting → Brief 4 (in progress)
Vitest + auth tests that prove getUserFromRequest returns null for the 4 unauthenticated shapes → Brief 5
Removing admin@tcgvault.com / admin123 default seed → gotcha #4, drop-public-setup convoy

Audit cohort

Reviewer dispatch follows post-push. No UI surface → design-system + a11y + browser-smoke auditors do not run.

🤖 Pipeline metadata: convoy=fix-auth-bypass, brief=2, depends_on=[1], audit=reviewer-only

Made with Cursor

## Summary Convoy `fix-auth-bypass` / Brief 2 of 5. **This is the keystone PR of the convoy** — it closes AGENTS.md gotcha #2 (the actual auth bypass). PR #6 (Brief 1) only renamed the secret; this PR removes the bypass behavior. - `lib/permission-middleware.js::getUserFromRequest` no longer returns a hardcoded `{ userId: 1, email: 'admin@tcgvault.com', role: 'admin' }` when the `Authorization` header is missing or malformed. It now returns `null`, and the 24 callers across `pages/api/` correctly handle the null per the `if (!user) return res.status(401)` pattern documented in `.cursor/rules/auth-and-permissions.mdc`. - `pages/api/auth/verify.js` no-token branch returns 401 instead of fetching the seed admin row via `WHERE email = 'admin@tcgvault.com'`. The admin-record leak side of the same bypass is closed. - No `NODE_ENV === 'development'` env-gate. No `console.warn`. No commented-out fallback "for later." The bypass is gone, period. If a developer needs an authenticated session locally, they log in. ## Files changed - `lib/permission-middleware.js` — 4-line synthetic admin block → 1-line `return null;` - `pages/api/auth/verify.js` — 12-line no-token admin-fetch branch → 3-line 401 response Net diff: 2 files, +2 / −16 LOC. ## Test plan - [x] `npm run build` exits 0 - [x] `npm run lint` matches baseline (128 problems / 81 errors / 47 warnings); zero new lint issues - [x] `git grep -E \"userId: 1|admin@tcgvault\.com\"` against `lib/` + `pages/` returns ZERO hits (only `scripts/setup-neon-db.js` still seeds that email, which is out of scope — gotcha #4's `drop-public-setup` convoy) - [x] `git grep \"Development mode\" -- 'lib/' 'pages/'` returns ZERO hits - [x] All 24 callers of `getUserFromRequest` verified to handle `null` correctly with the `if (!user) return res.status(401)` shape — except the one in **Known follow-up** below - [ ] **Reviewer to verify on preview deployment:** `curl -X GET https://<preview>/api/users/search?q=foo` (no Authorization header) returns 401, not admin data. `curl -X GET https://<preview>/api/auth/verify` returns 401, not the admin record. ## ⚠️ Known follow-up (NOT addressed in this PR, intentional) `pages/api/collections/[identifier]/cards.js` POST/PUT/DELETE handlers dereference `user.userId` without a null guard. **Previously masked by the synthetic admin** (anonymous-write-as-admin on collections owned by `userId: 1` was the security hole). **After this PR**, those handlers degrade to NPE → 500 instead of a clean 401. - **Security:** improved either way. No more anonymous writes. - **UX/error format:** slightly worse until we add the guard. - **Fix:** one-line follow-up PR (add `if (!user) return res.status(401)...` to each of the three handlers). I'll dispatch this as a separate small patch after this PR merges. It does NOT block this PR — the bypass is closed; the cosmetic 500-vs-401 is a strict improvement over the previous status quo (anonymous-write-as-admin). ## Pre-merge requirements - None directly. (Brief 1's `JWT_SECRET` requirement already satisfied — Brief 1 has merged.) - After this merges, the admin must log in via `/api/auth/login` to use admin features. The fallback that previously let unauthenticated admin actions through is gone. ## Out of scope (intentionally — see convoy plan) - The cosmetic 500-vs-401 in `cards.js` → small follow-up PR after this merges - `pages/api/admin/index.js` uses `verifyToken` directly (not `getUserFromRequest`) — unaffected by this brief - CORS tightening + rate limiting → **Brief 4** (in progress) - Vitest + auth tests that prove `getUserFromRequest` returns `null` for the 4 unauthenticated shapes → **Brief 5** - Removing `admin@tcgvault.com` / `admin123` default seed → gotcha #4, `drop-public-setup` convoy ## Audit cohort Reviewer dispatch follows post-push. No UI surface → design-system + a11y + browser-smoke auditors do not run. --- 🤖 Pipeline metadata: convoy=fix-auth-bypass, brief=2, depends_on=[1], audit=reviewer-only Made with [Cursor](https://cursor.com)

vercel[bot] commented

2026-05-23 11:47:55 -04:00

(Migrated from github.com)

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
tcg-vault	Ready	Preview, Comment	May 23, 2026 3:47pm

[vc]: #ss1Zxe2L15Hnzfj9PgzAyXPvwZhv7mo4bXYPFr4TCfQ=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWJyaWVmLWZpeC1hdXRoLTYzYTA4My1yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMudmVyY2VsLmFwcCJ9LCJpbnNwZWN0b3JVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzL3RjZy12YXVsdC9IMWh1cm54c044RlplTTRGNFZqdEJzRHpuOXU2IiwicHJldmlld1VybCI6InRjZy12YXVsdC1naXQtYnJpZWYtZml4LWF1dGgtNjNhMDgzLXJhbmRhbGwtc3RpbGx3ZWxscy1wcm9qZWN0cy52ZXJjZWwuYXBwIiwibmV4dENvbW1pdFN0YXR1cyI6IkRFUExPWUVEIn1dLCJyZXF1ZXN0UmV2aWV3VXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3ZlcmNlbC1hZ2VudC9yZXF1ZXN0LXJldmlldz9vd25lcj12YXJ1dGFzdSZyZXBvPXRjZy12YXVsdCZwcj04In0= The latest updates on your projects. Learn more about [Vercel for GitHub](https://vercel.link/github-learn-more). | Project | Deployment | Actions | Updated (UTC) | | :--- | :----- | :------ | :------ | | [tcg-vault](https://vercel.com/randall-stillwells-projects/tcg-vault) | ![Ready](https://vercel.com/static/status/ready.svg) [Ready](https://vercel.com/randall-stillwells-projects/tcg-vault/H1hurnxsN8FZeM4F4VjtBsDzn9u6) | [Preview](https://tcg-vault-git-brief-fix-auth-63a083-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-brief-fix-auth-63a083-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 23, 2026 3:47pm | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=8" rel="noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://agents-vade-review.vercel.sh/request-review-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://agents-vade-review.vercel.sh/request-review-light.svg"><img src="https://agents-vade-review.vercel.sh/request-review-light.svg" alt="Request Review"></picture></a>

github-actions[bot] commented

2026-05-23 11:47:59 -04:00

(Migrated from github.com)

Pipeline Health

Build + CI gates

Gate	Status
Vercel build (Preview)	✅ pass
CI: Lint	✅ pass
CI: Schema map fresh	❌ skipped
Preview smoke	❌ failure
Visual diff	❌ failure

Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build).

Role reports

Role	Status
Reviewer report	⏳ pending
A11y audit	⏳ pending
Design system audit	⏳ pending

See individual comments above for details. This rollup updates automatically.

## Pipeline Health ### Build + CI gates | Gate | Status | | --- | --- | | Vercel build (Preview) | ✅ pass | | CI: Lint | ✅ pass | | CI: Schema map fresh | ❌ skipped | | Preview smoke | ❌ failure | | Visual diff | ❌ failure | _Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build)._ ### Role reports | Role | Status | | --- | --- | | Reviewer report | ⏳ pending | | A11y audit | ⏳ pending | | Design system audit | ⏳ pending | See individual comments above for details. This rollup updates automatically.