fix(api): return 401 (not 500) on unauthenticated cards-collection writes #10

Merged

varutasu merged 1 commit from brief/fix-auth-bypass/6-cards-null-guard into main

2026-05-23 12:04:55 -04:00

varutasu commented

2026-05-23 12:03:12 -04:00

(Migrated from github.com)

Summary

Follow-up hotfix to PR #8 / commit 258e479 (fix-auth-bypass Brief 2). The Brief 2 reviewer flagged that pages/api/collections/[identifier]/cards.js POST/PUT/DELETE branches dereference user.userId without a null guard. Previously masked by the synthetic admin (anonymous-write-as-admin was the security hole); now degrades to NPE → HTTP 500 instead of a clean HTTP 401.

Security side was already fixed by Brief 2. This PR is the cosmetic 500 → 401 cleanup.

Files changed

pages/api/collections/[identifier]/cards.js (+12 / -0)

Three identical if (!user) return 401 guards added, one at the top of each of the POST, PUT, and DELETE branches. GET branch untouched (already guarded via ${user ? sql\...` : sql``}` ternary pattern for anonymous public-collection reads).

Test plan

npm run build exits 0
npm run lint adds zero new errors to cards.js (pre-existing repo lint debt unchanged at 128 problems)
git grep \"if (!user)\" cards.js returns 3 hits (was 0 pre-PR), one per write branch
Sibling endpoints under pages/api/collections/** re-audited: thumbnails.js, permissions.js, activity.js all have early null checks; [identifier].js uses optional chaining. No further hotfixes needed for that route group.
Reviewer to verify on preview: curl -X POST https://<preview>/api/collections/<public-slug>/cards -d '{\"cardId\":1}' -H \"Content-Type: application/json\" (no Authorization header) returns 401. Same for PUT and DELETE.

Notes

Error message is intentionally \"Authentication required to modify this collection\" (distinct from generic \"Authentication required\" elsewhere) to match the user-visible 403 messaging just below in the same handler.
No withAuth(handler) wrapper introduced — that's a future refactor convoy. Minimal-invasive fix only.

Convoy context

This closes the last open thread from fix-auth-bypass. After this merges, the convoy's only remaining work is Brief 5 (vitest + auth tests, in flight as PR-pending) and the doc-writer pass.

🤖 Pipeline metadata: convoy=fix-auth-bypass, brief=6 (post-architect hotfix), depends_on=[2 merged], audit=reviewer-only

Made with Cursor

## Summary Follow-up hotfix to [PR #8 / commit 258e479](https://github.com/varutasu/tcg-vault/commit/258e479) (fix-auth-bypass Brief 2). The Brief 2 reviewer flagged that `pages/api/collections/[identifier]/cards.js` POST/PUT/DELETE branches dereference `user.userId` without a null guard. Previously masked by the synthetic admin (anonymous-write-as-admin was the security hole); now degrades to NPE → HTTP 500 instead of a clean HTTP 401. **Security side was already fixed by Brief 2.** This PR is the cosmetic 500 → 401 cleanup. ## Files changed - `pages/api/collections/[identifier]/cards.js` (+12 / -0) Three identical `if (!user) return 401` guards added, one at the top of each of the POST, PUT, and DELETE branches. GET branch untouched (already guarded via `${user ? sql\`...\` : sql\`\`}` ternary pattern for anonymous public-collection reads). ## Test plan - [x] `npm run build` exits 0 - [x] `npm run lint` adds zero new errors to `cards.js` (pre-existing repo lint debt unchanged at 128 problems) - [x] `git grep \"if (!user)\" cards.js` returns 3 hits (was 0 pre-PR), one per write branch - [x] Sibling endpoints under `pages/api/collections/**` re-audited: `thumbnails.js`, `permissions.js`, `activity.js` all have early null checks; `[identifier].js` uses optional chaining. **No further hotfixes needed for that route group.** - [ ] **Reviewer to verify on preview:** `curl -X POST https://<preview>/api/collections/<public-slug>/cards -d '{\"cardId\":1}' -H \"Content-Type: application/json\"` (no Authorization header) returns 401. Same for PUT and DELETE. ## Notes - Error message is intentionally `\"Authentication required to modify this collection\"` (distinct from generic `\"Authentication required\"` elsewhere) to match the user-visible 403 messaging just below in the same handler. - No `withAuth(handler)` wrapper introduced — that's a future refactor convoy. Minimal-invasive fix only. ## Convoy context This closes the last open thread from `fix-auth-bypass`. After this merges, the convoy's only remaining work is Brief 5 (vitest + auth tests, in flight as PR-pending) and the doc-writer pass. --- 🤖 Pipeline metadata: convoy=fix-auth-bypass, brief=6 (post-architect hotfix), depends_on=[2 merged], audit=reviewer-only Made with [Cursor](https://cursor.com)

vercel[bot] commented

2026-05-23 12:03:17 -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	Building	Preview, Comment	May 23, 2026 4:03pm

[vc]: #fQ/mwcuborqGi6yF/LcLmSr1tljAEmepDLtyVAKbMbY=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImluc3BlY3RvclVybCI6Imh0dHBzOi8vdmVyY2VsLmNvbS9yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMvdGNnLXZhdWx0L0E4ZXB1TUw1eFZCRjlIY0p2eVFhdW1UOVJ0QXIiLCJwcmV2aWV3VXJsIjoidGNnLXZhdWx0LWdpdC1icmllZi1maXgtYXV0aC1lMDlhZjgtcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzLnZlcmNlbC5hcHAiLCJuZXh0Q29tbWl0U3RhdHVzIjoiUEVORElORyIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWJyaWVmLWZpeC1hdXRoLWUwOWFmOC1yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMudmVyY2VsLmFwcCJ9fV0sInJlcXVlc3RSZXZpZXdVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vdmVyY2VsLWFnZW50L3JlcXVlc3QtcmV2aWV3P293bmVyPXZhcnV0YXN1JnJlcG89dGNnLXZhdWx0JnByPTEwIn0= 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) | ![Building](https://vercel.com/static/status/building.svg) [Building](https://vercel.com/randall-stillwells-projects/tcg-vault/A8epuML5xVBF9HcJvyQaumT9RtAr) | [Preview](https://tcg-vault-git-brief-fix-auth-e09af8-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-brief-fix-auth-e09af8-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 23, 2026 4:03pm | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=10" 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 12:03:23 -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.