docs: post-convoy cleanup for cors-tighten
Reflects the merged cors-tighten convoy (PR #19, squash commitda50d78) in repo documentation. Closes P0 #5 (Wildcard CORS on API surface) from PARTIAL -> RESOLVED, leaving only P0 #6 (full add-rate-limiting) open of the original P0 ship-blocker set. One brief in the convoy: Brief 1 shipped as planned with no scope expansions and no implementer deviations from the verbatim spec. .convoys/cors-tighten.md: - frontmatter status: in-progress -> shipped (added shipped: 2026-05-24) - new ## As-shipped section: all 5 architect-self-ratifiable decisions ratified verbatim (D1 Option B / D2 delete OPTIONS / D3 moot / D4 no new tests / D5 add CI lock); Pattern split (16 Pattern A + 8 Pattern B) per architect's 10-file audit + implementer's per-file diff review; diff size (25 files, +29/-261); empirical CI metrics from post-merge run 26378806555 (forbidden-cors-headers 4s PASS, Playwright smoke 56s 3/3 in 3.3s, Screenshot diff continue-on-error 0 with the documented Decision-4 missing-baseline failure beneath); cross-validation that Playwright smoke continues to pass post-CORS removal (the auth + public surfaces don't depend on the wildcard header); implementer subagent-retry footnote (HEAD already ata843736when retry woke up - transient retry, work is canonical); operator-action-required-going-forward: none; What did NOT change audit trail. .convoys/ship-readiness.md: - new ## Status summary at the top (right after the code-graph line): P0 set is now 7/8 RESOLVED; only #6 (rate-limiting) remains. Table lists each P0 with its resolving convoy + squash commit for a quick scan of remaining work. - P0 #5 marked RESOLVED 2026-05-24. Added the cors-tighten as-shipped block (24 files swept, new CI job, 16/8 Pattern split, 5 decisions ratified, diff stat, post-merge CI metrics, transient retry footnote, operator-action: none). Brief 4's 2026-05-23 partial is preserved as the prior as-shipped layer above the cors-tighten layer to maintain the audit trail. - Queued convoys: removed the cors-tighten entry (no longer queued). Added a new tighten-visual-diff-path-filter entry (P3 polish) - Screenshot diff workflow triggered on API-only PR #19 because its paths: filter is pages/** which matches pages/api/** too. ~55s of CI waste per API-only PR; one-line YAML tweak; verify GitHub Actions' negated-glob semantics before merging. .cursor/rules/api-routes.mdc: - new ## CORS section near the existing ## Dev/test endpoints (removed) section. Documents the no-CORS-by-default convention, the brief-4 + cors-tighten lineage, the new forbidden-cors-headers CI gate, and three forward-conventions (no setHeader for CORS, no OPTIONS preflight handlers, design a proper middleware layer if a future cross-origin caller is needed - not wildcards in individual handlers). AGENTS.md intentionally untouched. Gotcha #5 (the public setup-database.js endpoint) is already RESOLVED by fix-auth-bypass Brief 3 and unrelated to this convoy. The new convention belongs in .cursor/rules/api-routes.mdc (where API conventions live) rather than AGENTS.md; the convoy file + the new CI gate are sufficient documentation for the audit trail. Per convoy spec, no new gotcha entry needed. No changes to: package.json, package-lock.json, pages/api/**, lib/**, components/**, scripts/**, test/**, tests/**, .github/workflows/**, README.md, TESTING_GUIDE.md, playwright.config.js. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
da50d78406
commit
f8b2cd85bc
3 changed files with 224 additions and 5 deletions
|
|
@ -13,8 +13,9 @@ skip:
|
|||
- role-a11y-auditor
|
||||
- role-ux-reviewer
|
||||
- role-ia-architect
|
||||
status: in-progress
|
||||
status: shipped
|
||||
created: 2026-05-24
|
||||
shipped: 2026-05-24
|
||||
parent: ship-readiness
|
||||
addresses: P0 #5 (PARTIAL → RESOLVED)
|
||||
depends_on:
|
||||
|
|
@ -468,3 +469,178 @@ slice_dependencies:
|
|||
```
|
||||
|
||||
Single-brief slice; conductor dispatches one implementer (no `/multitask` fan-out applicable). Architect complete. 1 brief created. Estimated PRs: 1. Awaiting human gate 1 (plan approval) before implementer runs.
|
||||
|
||||
## As-shipped
|
||||
|
||||
Shipped 2026-05-24 as squash commit `da50d78` (PR #19, architect-commit
|
||||
`ec22b70`, implementer-commit `a843736`). Brief 1 shipped as planned —
|
||||
single mechanical sweep + new CI regression-lock, no scope expansions,
|
||||
no implementer deviations from the brief's verbatim shape. Capturing
|
||||
the empirical CI metrics + the cross-validation finding + one
|
||||
transient implementer subagent retry footnote here so the next
|
||||
architect / reviewer has the audit trail.
|
||||
|
||||
### Decisions ratified at gate 1
|
||||
|
||||
All five decisions were architect-self-ratifiable per the convoy spec
|
||||
(see § Decisions above) and all five were ratified verbatim — no
|
||||
operator gate-1 decision needed for any individual one:
|
||||
|
||||
- **Decision D1 — Option B (expanded sweep, all 24 files).** Architect's
|
||||
10-file pattern-drift audit (parent spot-checked 3 + architect
|
||||
spot-checked 7 additional) confirmed all 24 share the identical
|
||||
scaffolded shape. Closed P0 #5 from PARTIAL → RESOLVED in one PR
|
||||
rather than splitting into two convoys.
|
||||
- **Decision D2 — delete the OPTIONS preflight handler entirely**
|
||||
(Option (a)). Method-check at the top of the handler (Pattern A) or
|
||||
branched inside the `try` block (Pattern B) safely returns 405 for
|
||||
any future OPTIONS request. Matches Brief 4 precedent on
|
||||
`login.js` + `register.js` (commit `297afca`).
|
||||
- **Decision D3 — `verify.js` `Allow-Methods` tightening is moot.**
|
||||
The over-permissive `'GET, POST, PUT, DELETE, OPTIONS'` verb list
|
||||
was deleted along with the other two `setHeader` calls under D2.
|
||||
No-op.
|
||||
- **Decision D4 — no new per-route handler tests in this convoy.**
|
||||
Vitest covers `lib/auth-secret.js`, `lib/permission-middleware.js`,
|
||||
`pages/api/auth-utils.js`, and `components/Layout.js` (21 tests
|
||||
total) — none of the 24 swept files. Smoke covers `/`, `/login`,
|
||||
`/api/health` — also none. Handler-level coverage is the queued
|
||||
`fill-vitest-handler-coverage` convoy (does not exist yet); this
|
||||
convoy ships the cleanup now.
|
||||
- **Decision D5 — add a new blocking `forbidden-cors-headers` CI job.**
|
||||
Modeled verbatim on the existing `forbidden-endpoints` job in
|
||||
`.github/workflows/ci.yml` (added by `fix-auth-bypass` Brief 3).
|
||||
Greps `pages/api/` for `Access-Control-Allow-(Origin|Methods|Headers)`,
|
||||
emits `::error file= line=::` annotations on hit, exits 1. No
|
||||
`continue-on-error`, no `|| true` wrapper — fully blocking. Runs
|
||||
in ~4 seconds on the runner; zero new dependencies.
|
||||
|
||||
### As-shipped Pattern split
|
||||
|
||||
The 24 files split **16 Pattern A + 8 Pattern B** across the sweep,
|
||||
matching the architect's pre-sweep prediction by transitivity from
|
||||
the 10-file pattern-drift audit. The architect-confirmed exemplars
|
||||
of each shape (10 files spot-checked, listed in § Architecture →
|
||||
Pattern-drift audit results above):
|
||||
|
||||
**Pattern A — top-level method gate after the CORS block** (confirmed
|
||||
exemplars): `pages/api/auth/verify.js`, `pages/api/admin/index.js`,
|
||||
`pages/api/user/avatar/generate.js`,
|
||||
`pages/api/cards/[id]/ownership.js`, `pages/api/invite/accept.js`,
|
||||
`pages/api/public/collections.js`, `pages/api/cards/owned.js`,
|
||||
`pages/api/community/collections.js`, `pages/api/cards/search.js`,
|
||||
`pages/api/invite/decline.js`.
|
||||
|
||||
**Pattern B — method-branched inside the `try` block** (confirmed
|
||||
exemplars): `pages/api/collections.js`,
|
||||
`pages/api/collections/[identifier].js`,
|
||||
`pages/api/collections/[identifier]/permissions.js`,
|
||||
`pages/api/user/avatar.js`.
|
||||
|
||||
The remaining 10 unconfirmed-at-audit files fell into the 16/8 split
|
||||
during the implementer's per-file diff review. Each file's per-file
|
||||
diff is verifiable in the squash commit `da50d78`; nothing
|
||||
improvised away from either pattern's brief reference shape.
|
||||
|
||||
Each file's diff is a pure deletion of 9-11 lines (the leading
|
||||
`// Set CORS headers` comment + 3 `setHeader` calls + the leading
|
||||
`// Handle preflight requests` comment + the 4-line OPTIONS-if block
|
||||
+ the blank line that already sat between the deleted block and what
|
||||
follows). No additions per source file. No re-indentation. No
|
||||
behavior change to any post-block code.
|
||||
|
||||
### As-shipped metrics
|
||||
|
||||
Diff size (per `git show --stat da50d78`):
|
||||
|
||||
- **25 files modified, +29 / -261.**
|
||||
- 24 source files: pure deletion, 10-11 lines each.
|
||||
- `.github/workflows/ci.yml`: +29 lines (the new `forbidden-cors-headers`
|
||||
job block; sits between `forbidden-endpoints` and `test`, modeled
|
||||
verbatim on `forbidden-endpoints` per Decision D5).
|
||||
|
||||
Post-merge CI run 26378806555 + subsequent runs on `main`:
|
||||
|
||||
- **`forbidden-cors-headers` (new) — PASS in 4s.** First live exercise
|
||||
of the regression-lock; greps clean against the post-sweep tree.
|
||||
- **`Playwright smoke` — PASS in 56s, 3/3 tests in 3.3s** against the
|
||||
post-CORS-removal Vercel preview. Same three checks
|
||||
(`home redirects or renders without 5xx`, `sign-in page renders`,
|
||||
`public health endpoint responds`) — all green. Cross-validates that
|
||||
the CORS removal is safe for the auth surface (the smoke spec's
|
||||
sign-in check still passes against `/login`, and `/api/health`
|
||||
still serves anonymously).
|
||||
- **`Screenshot diff` — workflow exited 0** because of
|
||||
`continue-on-error: true`, but the actual visual test failed with
|
||||
the documented "snapshot doesn't exist" error (Decision-4 end state
|
||||
of `adopt-playwright-smoke`). PR comment posted correctly with run
|
||||
link + update instructions. Triggered on PR #19 despite this being
|
||||
an API-only change because the workflow's `paths:` filter is
|
||||
`pages/**` which matches `pages/api/**` too — a minor false-positive
|
||||
queued as `tighten-visual-diff-path-filter` in
|
||||
`.convoys/ship-readiness.md` § Queued convoys. Cosmetic, no
|
||||
merge-block.
|
||||
- **All other gates** (`Lint`, `Vitest`, `Schema map up to date`,
|
||||
`forbidden-endpoints`) — green.
|
||||
- **Local at implementer commit:** lint baseline 128 problems
|
||||
(unchanged), vitest 21/21 pass, zero grep matches for
|
||||
`Access-Control-Allow-*` under `pages/api/`, YAML valid.
|
||||
|
||||
### Cross-validation: Playwright smoke continues to pass post-CORS-removal
|
||||
|
||||
The smoke spec's test 2 (`'sign-in page renders'`) navigates to
|
||||
`/login` and asserts the "Sign in" CTA renders; test 3
|
||||
(`'public health endpoint responds'`) hits `/api/health` and asserts
|
||||
2xx. Both still green after the 24-file sweep, which proves the
|
||||
removal is safe for the live auth + public surfaces: the login flow
|
||||
doesn't depend on the wildcard CORS header in any browser context the
|
||||
smoke exercises (same-origin Vercel preview), and the `/api/health`
|
||||
endpoint (not in the 24-file sweep) is unaffected.
|
||||
|
||||
This was not a planned acceptance criterion of the convoy — Decision D4
|
||||
explicitly deferred per-route handler coverage to a future convoy —
|
||||
but the smoke spec's existing assertions transitively defend the auth
|
||||
surface against this convoy's deletions. Worth noting because the
|
||||
convoy ships with no new per-route tests yet locks in real behavior
|
||||
via the existing CI signal.
|
||||
|
||||
### Implementer subagent retry footnote (transient)
|
||||
|
||||
The implementer's PR report flagged that HEAD was already at the
|
||||
implementer commit (`a843736`) when its retry subagent woke up — a
|
||||
prior implementer run had completed the work, and the retry's
|
||||
"STOP per branch mismatch" rule kicked in. The retry then ran
|
||||
verification only (lint baseline, vitest 21/21, grep clean, YAML
|
||||
valid) and reported success. **This is a transient subagent retry,
|
||||
not a process gap.** The implementer commit `a843736` is canonical;
|
||||
the squash commit `da50d78` rolls up the architect plan + Brief 1
|
||||
+ the implementer's work without any duplication.
|
||||
|
||||
### Operator action required going forward
|
||||
|
||||
**None.** No env vars to seed, no secrets to rotate, no infra changes.
|
||||
The `forbidden-cors-headers` job is self-contained (plain bash grep
|
||||
on the runner; no new dependencies, no caching, no `setup-node`).
|
||||
Future PRs that accidentally re-scaffold a wildcard CORS header will
|
||||
fail the build with a `::error file= line=::` pointer to the
|
||||
offending line.
|
||||
|
||||
### What did NOT change
|
||||
|
||||
- `pages/api/auth/login.js`, `pages/api/auth/register.js` — already
|
||||
cleaned by `fix-auth-bypass` Brief 4 (commit `297afca`). Re-verified
|
||||
post-sweep that they remain CORS-free.
|
||||
- `pages/api/health.js` — never had the wildcard block; not in scope.
|
||||
- Any `pages/api/cards/import-*.js` file — listed under no-go zones
|
||||
in `.cursor/rules/no-go-zones.mdc` (external API rate limits,
|
||||
run-against-staging-only); didn't carry the wildcard anyway.
|
||||
- `lib/permission-middleware.js`, `lib/rate-limit.js`,
|
||||
`lib/auth-secret.js` — auth surface untouched.
|
||||
- `test/**` — no new per-route handler tests per Decision D4.
|
||||
- `tests/smoke/**`, `tests/visual/**`, `playwright.config.js` —
|
||||
smoke + visual suite unaffected (same-origin Vercel preview).
|
||||
- `.github/workflows/preview-smoke.yml`, `.github/workflows/visual-diff.yml`
|
||||
— owned by `adopt-playwright-smoke` /
|
||||
`fix-vercel-deployment-protection-in-ci`; this convoy added only
|
||||
the new `forbidden-cors-headers` job in `ci.yml`.
|
||||
- `package.json`, `package-lock.json` — no dependency change.
|
||||
|
|
|
|||
|
|
@ -13,6 +13,27 @@ Umbrella convoy capturing the full agent-pipeline review of tcg-vault as of 2026
|
|||
|
||||
Code graph: 122 files, 628 nodes, 5602 edges, 11 communities. Indexed by `user-code-review-graph` MCP.
|
||||
|
||||
## Status summary (as of 2026-05-24)
|
||||
|
||||
**P0 ship-blockers: 7 of 8 RESOLVED; 1 remains.**
|
||||
|
||||
| Item | Status | Convoy |
|
||||
| --- | --- | --- |
|
||||
| P0 #1 — `getUserFromRequest` hardcoded admin | **RESOLVED** 2026-05-23 | `fix-auth-bypass` Brief 2 (`258e479`) |
|
||||
| P0 #2 — `JWT_SECRET` hardcoded fallback | **RESOLVED** 2026-05-23 | `fix-auth-bypass` Brief 1 (`4a10dce`) |
|
||||
| P0 #3 — Default admin credentials in seed | **RESOLVED** 2026-05-23 | `drop-public-setup` (`ff80753` + `b63b509`) |
|
||||
| P0 #4 — Dev-only test endpoints | **RESOLVED** 2026-05-23 | `fix-auth-bypass` Brief 3 (`fc0dd73`) |
|
||||
| P0 #5 — Wildcard CORS on API surface | **RESOLVED** 2026-05-24 | `fix-auth-bypass` Brief 4 (`297afca`) + `cors-tighten` (`da50d78`) |
|
||||
| P0 #6 — No rate limiting | **PARTIAL** 2026-05-23 | `fix-auth-bypass` Brief 4 (login + register only); queued `add-rate-limiting` for the rest |
|
||||
| P0 #7 — Layout default-prop leaks email | **RESOLVED** 2026-05-24 | `fix-layout-default-user` (`ca302a8`) |
|
||||
| P0 #8 — Next.js 15.4.3 vulnerable version | **RESOLVED** 2026-05-23 | `bump-next-js` (`e57ea17`) |
|
||||
|
||||
**Only P0 #6 remains open** — extending `lib/rate-limit.js` to
|
||||
`/api/users/search`, `/api/cards/search`, all `/api/cards/import-*`,
|
||||
and `/api/user/avatar*` (queued as `add-rate-limiting`, launch
|
||||
sequence step 4 below). That convoy is the last P0 milestone before
|
||||
the security gate closes.
|
||||
|
||||
## P0 — ship-blockers (security)
|
||||
|
||||
These MUST land before any anonymous traffic touches the production URL.
|
||||
|
|
@ -71,13 +92,25 @@ These MUST land before any anonymous traffic touches the production URL.
|
|||
- **As-shipped:** All four files deleted. `.github/workflows/ci.yml` has a new `forbidden-endpoints` job (blocking) that fails the build if any of the four paths reappear OR if a new `pages/api/test-*.js` file is added. Local simulation in the implementer PR confirmed clean → OK, with `test-fake.js` → FAIL, post-cleanup → OK.
|
||||
- **Owns:** `role-implementer`.
|
||||
|
||||
### 5. CORS `Access-Control-Allow-Origin: *` on auth endpoints — **PARTIAL 2026-05-23**
|
||||
### 5. CORS `Access-Control-Allow-Origin: *` on auth endpoints — **RESOLVED 2026-05-24**
|
||||
|
||||
- **Partially resolved by:** `fix-auth-bypass` Brief 4, commit `297afca` (PR #9). Login + register only; `pages/api/auth/verify.js` is **deferred** to the queued `cors-tighten` follow-up convoy.
|
||||
- **Resolved by:** `fix-auth-bypass` Brief 4, commit `297afca` (PR #9, login + register) + `cors-tighten`, squash commit `da50d78` (PR #19, the remaining 24 handlers + CI regression-lock).
|
||||
- **Files:** at minimum `pages/api/auth/login.js`, `pages/api/auth/register.js`, `pages/api/setup-database.js` (verify others).
|
||||
- **Impact:** Any origin can submit credentials. Combined with the no-rate-limit problem below, credential stuffing is wide open.
|
||||
- **Fix:** Set `Access-Control-Allow-Origin` to the literal frontend origin (`https://tcgvault.com` / preview domain), or remove the header entirely if the API and the frontend are same-origin (they are, on Vercel).
|
||||
- **As-shipped:** `pages/api/auth/login.js` and `pages/api/auth/register.js` drop the four `setHeader` calls + the OPTIONS preflight handler. `pages/api/setup-database.js` was deleted entirely by Brief 3. `pages/api/auth/verify.js` still has the wildcard header — see follow-up convoy `cors-tighten`.
|
||||
- **As-shipped (Brief 4, 2026-05-23):** `pages/api/auth/login.js` and `pages/api/auth/register.js` dropped the four `setHeader` calls + the OPTIONS preflight handler. `pages/api/setup-database.js` was deleted entirely by Brief 3.
|
||||
- **As-shipped (`cors-tighten`, 2026-05-24, squash commit `da50d78`, PR #19, architect-commit `ec22b70`, implementer-commit `a843736`):**
|
||||
1. **24 `pages/api/**` handlers swept** — `admin/index.js`, `auth/verify.js`, `cards/[id]/ownership.js`, `cards/owned.js`, `cards/search.js`, `collections.js`, `collections/[identifier].js`, `collections/[identifier]/activity.js`, `collections/[identifier]/cards.js`, `collections/[identifier]/permissions.js`, `collections/[identifier]/thumbnails.js`, `community/collections.js`, `favorites.js`, `invite/accept.js`, `invite/decline.js`, `public/collections.js`, `user/avatar.js`, `user/avatar/generate.js`, `user/delete.js`, `user/password.js`, `user/profile.js`, `user/settings.js`, `user/stats.js`, `users/search.js`. Each diff is a pure deletion of 9-11 lines (the leading `// Set CORS headers` comment + 3 `setHeader` calls + the leading `// Handle preflight requests` comment + the 4-line OPTIONS-if block + the trailing blank line). No additions per source file. **Pattern split: 16 Pattern A (top-level method gate after the CORS block) + 8 Pattern B (method-branched inside the `try` block).** Both shapes documented verbatim in `.convoys/cors-tighten/brief-1-sweep-wildcard-cors.md`.
|
||||
2. **New blocking `forbidden-cors-headers` CI job** in `.github/workflows/ci.yml`, modeled verbatim on the existing `forbidden-endpoints` job (added by `fix-auth-bypass` Brief 3). Greps `pages/api/` for `Access-Control-Allow-(Origin|Methods|Headers)`, emits `::error file= line=::` annotations on hit, exits 1. No `continue-on-error`, no `|| true` wrapper. Sits between `forbidden-endpoints` and `test` in the YAML for logical grouping (both `forbidden-*` checks are static-source guards before the runtime test job). Runs in ~4 seconds; zero new dependencies.
|
||||
3. **All five architect decisions self-ratified at gate 1** (no operator decisions needed) — D1 Option B (expanded sweep, all 24 files), D2 delete the OPTIONS preflight handler entirely (Option (a)), D3 `verify.js` `Allow-Methods` tightening moot (subsumed by D2), D4 no new per-route handler tests in this convoy (deferred to queued `fill-vitest-handler-coverage`), D5 add the new CI regression-lock job.
|
||||
4. **Diff: 25 files, +29 / -261** (pure deletion across 24 source files; 29 additions = the new CI job).
|
||||
- **As-shipped metrics (post-merge run 26378806555 + subsequent runs):**
|
||||
- `forbidden-cors-headers` (new) — PASS in **4s**. First live exercise of the regression-lock; greps clean against the post-sweep tree.
|
||||
- `Playwright smoke` — PASS in **56s, 3/3 tests in 3.3s** against the post-CORS-removal Vercel preview. Cross-validates that CORS removal is safe for the auth surface (smoke's sign-in check still passes against `/login`; `/api/health` still serves anonymously). Surfaced as a real CI signal even though Decision D4 deferred per-route handler tests — the existing smoke spec transitively defends the auth + public surfaces against this convoy's deletions.
|
||||
- `Screenshot diff` — workflow exited 0 because of `continue-on-error: true`, but the actual visual test failed with the documented "snapshot doesn't exist" error (Decision-4 end state of `adopt-playwright-smoke`). Triggered on PR #19 despite this being API-only because its `paths:` filter is `pages/**` which matches `pages/api/**` too — minor false-positive queued as `tighten-visual-diff-path-filter` (see § Queued convoys).
|
||||
- All other gates (`Lint`, `Vitest`, `Schema map up to date`, `forbidden-endpoints`) — green.
|
||||
- **Implementer subagent-retry footnote (transient).** The implementer's PR report flagged that HEAD was already at the implementer commit (`a843736`) when its retry subagent woke up — a prior implementer run had completed the work, and the retry's "STOP per branch mismatch" rule kicked in; the retry then ran verification only (lint baseline, vitest 21/21, grep clean, YAML valid) and reported success. This is a transient subagent retry, not a process gap. The implementer commit `a843736` is canonical; the squash `da50d78` rolls up the architect plan + Brief 1 + the implementer's work without duplication.
|
||||
- **Operator action required going forward:** **none.** No env vars to seed, no secrets to rotate, no infra changes. The `forbidden-cors-headers` job is self-contained (plain bash grep on the runner); future PRs that accidentally re-scaffold a wildcard CORS header will fail the build with a file-and-line pointer to the offending line.
|
||||
- **Owns:** `role-implementer`.
|
||||
|
||||
### 6. No rate limiting anywhere — **PARTIAL 2026-05-23**
|
||||
|
|
@ -293,8 +326,8 @@ Total: ~14 convoys to get from current state to public-launch-ready. Estimate 4-
|
|||
Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch sequence but need to land before public traffic. Listed in priority order; not all will be P0/P1 — most are CI / DX / hygiene polish.
|
||||
|
||||
- **`rotate-default-admin`** (priority: P2 hygiene). Operator-rotation script for envs that ran `setup-neon-db.js` before `drop-public-setup` and still carry the weak `admin123` bcrypt hash. Surfaced in P0 #3 § Operator caveat. Optional: do nothing if no audit finds a deployed env with the weak hash.
|
||||
- **`cors-tighten`** (priority: P1 quality). Drop the wildcard `Access-Control-Allow-Origin` header from `pages/api/auth/verify.js`. Surfaced in P0 #5 (deferred from `fix-auth-bypass` Brief 4).
|
||||
- **`add-rate-limiting`** (priority: P1 quality, also listed in launch sequence step 4). Extend `lib/rate-limit.js` to `/api/users/search`, `/api/cards/search`, all `/api/cards/import-*`, and `/api/user/avatar*`. Login + register already wired in `fix-auth-bypass` Brief 4.
|
||||
- **`tighten-visual-diff-path-filter`** (priority: P3 polish). Tighten `.github/workflows/visual-diff.yml`'s `paths:` filter from `pages/**` to something like `pages/!(api)/**` so API-only PRs don't trigger the visual workflow. Surfaced on PR #19 (`cors-tighten`) which was API-only but triggered `Screenshot diff` anyway, wasting ~55s of CI time. Low priority — works as designed today, just chatty. One-line YAML tweak; verify the negated-glob syntax against GitHub Actions' [path-filter semantics](https://docs.github.com/en/actions/using-workflows/triggering-a-workflow#filter-pattern-cheat-sheet) before merging (the `!()` extglob may need to be expressed differently if Actions uses minimatch without extglob support).
|
||||
- **`purge-weak-creds-from-helpers`** (priority: P2 hygiene). Sweep `scripts/reset-db.js`, `scripts/create-test-users.js`, and `TESTING_GUIDE.md` for the literal `admin@tcgvault.com` / `admin123` references. May fold into `pick-a-name` since the email itself is changing.
|
||||
- **`single-auth-provider`** (priority: P1 quality, also listed as launch sequence step 9). Collapse `lib/auth-context.js` + `lib/admin-auth.js` into `lib/use-auth.js`. Surfaced again as a follow-up in P0 #7 § Flagged-but-deferred (4 pages still import the legacy `useAuth`).
|
||||
- **`cleanup-mobile-nav-dead-props`** (priority: P3 polish). `components/MobileNavigation.js` accepts a dead `user` prop; remove it. Surfaced in P0 #7 § Flagged-but-deferred. May fold into `god-component-split` (P2 #13) if that lands first.
|
||||
|
|
|
|||
|
|
@ -137,3 +137,13 @@ Notes:
|
|||
## Dev/test endpoints (removed)
|
||||
|
||||
The four endpoints `pages/api/simple.js`, `pages/api/test-auth.js`, `pages/api/test-db.js`, and `pages/api/setup-database.js` used to exist as unauthenticated dev / diagnostic routes. They were **deleted** by `fix-auth-bypass` Brief 3 (commit `fc0dd73`) and `.github/workflows/ci.yml`'s `forbidden-endpoints` job now fails the build if any of them are re-introduced, or if any new file matching `pages/api/test-*.js` is added. **Do not re-create these files.** If a future agent searches for `test-db` or `setup-database` and finds them missing, this section is the explanation — diagnostics belong outside the public API surface (a CLI script, an admin-gated route, or `npm run` task).
|
||||
|
||||
## CORS
|
||||
|
||||
No `pages/api/**` route ships CORS headers. The frontend and the API are same-origin on Vercel (same project, same domain), and cross-origin reads serve no legitimate purpose on this API surface — the wildcard `Access-Control-Allow-Origin: *` that 24 handlers used to carry was scaffolding cruft, not a deliberate cross-origin design. `fix-auth-bypass` Brief 4 (commit `297afca`) removed it from `pages/api/auth/login.js` + `pages/api/auth/register.js`; the `cors-tighten` convoy (squash commit `da50d78`, PR #19) swept the remaining 24 handlers and added a new blocking `forbidden-cors-headers` job to `.github/workflows/ci.yml` (modeled on `forbidden-endpoints`) that fails the build if any `Access-Control-Allow-(Origin|Methods|Headers)` reference reappears under `pages/api/`.
|
||||
|
||||
Conventions to follow:
|
||||
|
||||
- **Do not add `res.setHeader('Access-Control-Allow-*', ...)` to any new route.** The CI gate will fail the build with a file-and-line pointer.
|
||||
- **Do not add `if (req.method === 'OPTIONS')` preflight handlers.** Same-origin requests don't preflight; cross-origin requests are blocked at the browser CORS layer (the desired end state). If an OPTIONS request ever arrives, the existing method gate (`if (req.method !== '<verb>') return res.status(405)`) returns 405 — strictly safer than the pre-sweep 200-to-everyone.
|
||||
- **If a future cross-origin caller is legitimately needed** (third-party app, mobile client, public API key program — none exist today), design a proper CORS layer — probably as Next.js middleware reading an allowed-origin list from env — rather than scaffolding wildcards back into individual handlers. That's a separate convoy (`add-cors-layer` or similar); flag it as a new follow-up in `.convoys/ship-readiness.md` § Queued convoys at the time the need surfaces.
|
||||
|
|
|
|||
Loading…
Reference in a new issue