fix(security): stop leaking Gemini API key to browsers (#34)
Delete the public /api/config/gemini endpoint and remove client auto-load paths so GEMINI_AI_API_KEY stays server-side only. Add a scan rate-limit class for the upcoming server-side identify route and a CI gate that blocks reintroducing config key leaks or new browser LLM URLs. Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
7efb6efe69
commit
8c58990fd9
13 changed files with 1047 additions and 115 deletions
146
.convoys/add-real-ocr-layer.md
Normal file
146
.convoys/add-real-ocr-layer.md
Normal file
|
|
@ -0,0 +1,146 @@
|
|||
---
|
||||
name: add-real-ocr-layer
|
||||
classification: feature
|
||||
success_metric: |
|
||||
≥70% of legitimate scans resolve at Layer-1 (Tesseract + pg_trgm) with
|
||||
zero Gemini calls; scan_attempts.layer distribution proves it.
|
||||
skip:
|
||||
- ia
|
||||
status: open
|
||||
created: 2026-05-27
|
||||
depends_on:
|
||||
- server-side-scan-pipeline
|
||||
---
|
||||
|
||||
# Convoy: add-real-ocr-layer
|
||||
|
||||
Add a cheap local OCR + fuzzy DB match layer so most scans never hit Gemini.
|
||||
|
||||
## Why
|
||||
|
||||
Every scan today (post convoy #2) calls Gemini Flash server-side. That is
|
||||
slow, costs money, and hits rate limits under bulk scanning. The audit
|
||||
proposed a two-layer pipeline: Layer-1 runs Tesseract on the card name strip
|
||||
+ `pg_trgm` similarity against `cards.name`; only escalates to
|
||||
`/api/scan/identify` (Gemini) when confidence is low. Target: **≥70%
|
||||
Layer-1 hit rate** measured via `scan_attempts.layer`.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- **Migration** — enable `pg_trgm` extension; GIN index on
|
||||
`cards (name gin_trgm_ops)`.
|
||||
- **`pages/api/cards/identify-by-text.js`** (new):
|
||||
- similarity > 0.85 → single match
|
||||
- 0.6–0.85 → disambiguation list
|
||||
- < 0.6 → `{ escalate: true }` (client falls back to `/api/scan/identify`)
|
||||
- **`tesseract.js`** dependency + **`lib/ocr-worker.js`** (new) — browser
|
||||
Worker wrapping Tesseract; OCR name strip before API calls.
|
||||
- **`components/CameraScanner.js`** — integrate Worker: try Layer-1 path
|
||||
first, escalate on low confidence.
|
||||
- **`docs/SCHEMA_MAP.md`** — index + extension documented.
|
||||
- **`.github/workflows/ci.yml`** — piggyback fix: extend `schema-map-fresh`
|
||||
job `if:` condition to include `migrations/**` paths (currently only
|
||||
watches `scripts/add-*`, `scripts/fix-*`, `setup-neon-db.js`, and
|
||||
`docs/SCHEMA_MAP.md` — misses post-`migration-tool` migrations).
|
||||
|
||||
### Out of scope
|
||||
|
||||
- **OpenCV perspective transform / card boundary detection** — queue
|
||||
`improve-scan-card-detection` if Layer-1 hit rate stays below 70% after
|
||||
this lands.
|
||||
- **Retraining or custom ML models** — Tesseract + trigram is sufficient
|
||||
for v1.
|
||||
- **Scanner UX redesign** — convoy `redesign-scanner-flow` (#4).
|
||||
|
||||
## Roles invoked
|
||||
|
||||
1. `role-ux-reviewer` — Layer-1 vs escalation feedback (scanning status).
|
||||
2. `role-architect` — similarity thresholds, Worker bundling, 2 briefs.
|
||||
3. `role-implementer` — 2 briefs (Brief 2 depends on Brief 1).
|
||||
4. `role-reviewer` + `role-design-system-auditor` + `role-a11y-auditor`.
|
||||
|
||||
## Todos
|
||||
|
||||
- [ ] Architect: ratify similarity thresholds + Worker load strategy
|
||||
- [ ] Brief 1 — migration (pg_trgm) + identify-by-text route + SCHEMA_MAP + CI schema-map-fresh fix
|
||||
- [ ] Brief 2 — Tesseract Worker + CameraScanner integration
|
||||
- [ ] Post-ship: measure `scan_attempts.layer` distribution for 70% target
|
||||
|
||||
## Operator action required
|
||||
|
||||
**None.** No new secrets. Tesseract runs client-side; pg_trgm is a Postgres
|
||||
extension enabled via migration.
|
||||
|
||||
## Multitask dispatch
|
||||
|
||||
### Slice dependencies
|
||||
|
||||
```yaml
|
||||
slice_dependencies:
|
||||
- brief: 1
|
||||
depends_on: []
|
||||
files:
|
||||
- migrations/*
|
||||
- pages/api/cards/identify-by-text.js
|
||||
- docs/SCHEMA_MAP.md
|
||||
- .github/workflows/ci.yml
|
||||
- brief: 2
|
||||
depends_on: [1]
|
||||
files:
|
||||
- components/CameraScanner.js
|
||||
- lib/ocr-worker.js
|
||||
- package.json
|
||||
```
|
||||
|
||||
Serial dispatch: Brief 2 after Brief 1 (Worker calls identify-by-text route).
|
||||
|
||||
Post-PR audit:
|
||||
|
||||
```
|
||||
/multitask role-reviewer + role-design-system-auditor + role-a11y-auditor
|
||||
```
|
||||
|
||||
Group id: `audit-add-real-ocr-layer-<pr>`.
|
||||
|
||||
## CI impact
|
||||
|
||||
| Workflow / job | Behavior |
|
||||
| --- | --- |
|
||||
| `schema-map-fresh` | **Modified** — `if:` paths include `migrations/**`. |
|
||||
| `forbidden-client-side-llm-keys` | Unchanged (no new client LLM URLs). |
|
||||
| `preview-smoke.yml` | Fires. |
|
||||
| `visual-diff.yml` | **Fires** — `components/CameraScanner.js` in paths. |
|
||||
|
||||
No new grep gate. Total added CI time: ~0 beyond existing workflows.
|
||||
|
||||
## Decisions to ratify (architect)
|
||||
|
||||
1. **Similarity thresholds** — 0.85 / 0.6 defaults from audit; tune with
|
||||
sample set.
|
||||
2. **Tesseract language data** — bundled vs CDN fetch; impact on first-load
|
||||
latency.
|
||||
3. **`scan_attempts.layer` values** — recommend `1 | 2` (trgm vs gemini).
|
||||
|
||||
## Known constraints
|
||||
|
||||
- **`pg_trgm` on Neon** — verify extension availability on prod tier.
|
||||
- **Worker + Turbopack** — confirm `tesseract.js` Worker path works under
|
||||
Next.js 16 default bundler; fallback `--webpack` only if architect
|
||||
documents regression.
|
||||
- **Layer-1 must not block camera** — Worker runs off main thread.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
1. Migration applies cleanly; GIN index exists on `cards.name`.
|
||||
2. `identify-by-text` returns match / disambig / escalate per thresholds.
|
||||
3. CameraScanner tries Layer-1 before `/api/scan/identify`.
|
||||
4. `scan_attempts.layer` populated for analytics.
|
||||
5. `schema-map-fresh` fires when only `migrations/` changes.
|
||||
6. Lint + vitest baseline preserved.
|
||||
|
||||
## Out of scope follow-ups
|
||||
|
||||
- **`improve-scan-card-detection`** — if Layer-1 hit rate < 70%.
|
||||
- **`god-component-split`** — CameraScanner remains large; split is P2.
|
||||
160
.convoys/redesign-scanner-flow.md
Normal file
160
.convoys/redesign-scanner-flow.md
Normal file
|
|
@ -0,0 +1,160 @@
|
|||
---
|
||||
name: redesign-scanner-flow
|
||||
classification: feature
|
||||
success_metric: |
|
||||
Scanner page uses a stack-destination model (one target at top, every scan
|
||||
pours into it); per-card condition + foil + quantity captured and propagate
|
||||
to destinations; "you already own N" ownership badge via /api/cards/[id]/ownership;
|
||||
captured frame persisted to Vercel Blob and attached to user_cards.
|
||||
skip: []
|
||||
status: open
|
||||
created: 2026-05-27
|
||||
depends_on:
|
||||
- server-side-scan-pipeline
|
||||
---
|
||||
|
||||
# Convoy: redesign-scanner-flow
|
||||
|
||||
Redesign the scanner session around a single destination stack, rich card
|
||||
metadata, ownership feedback, and persisted scan images.
|
||||
|
||||
## Why
|
||||
|
||||
The current scanner treats each card as an isolated add with no session
|
||||
context: users re-select destination every scan, cannot set condition/foil/
|
||||
quantity in bulk, get no feedback when they already own a card, and lose
|
||||
the captured frame after confirm. The audit flagged these as core UX gaps
|
||||
that block efficient bulk scanning at the table.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- **Brief 1 — stack-destination UX + game pre-select**
|
||||
- `pages/scanner.js` — session model: one chosen target at top; every
|
||||
confirmed scan routes there.
|
||||
- `components/ScannerDestinationPicker.js` (new) — collection / deck /
|
||||
default collection picker with game filter.
|
||||
|
||||
- **Brief 2 — condition / foil / quantity + ownership badge**
|
||||
- `pages/scanner.js`, `components/ScannedCardItem.js` (extract from
|
||||
scanner page).
|
||||
- `pages/api/user-cards.js`, `pages/api/decks/[id]/cards.js`,
|
||||
`pages/api/collections/[identifier]/cards.js` — accept `condition`,
|
||||
`is_foil`, `quantity` body params on POST.
|
||||
- Ownership badge via existing `pages/api/cards/[id]/ownership.js`.
|
||||
|
||||
- **Brief 3 — captured image persistence**
|
||||
- `pages/api/scan/upload-image.js` (new) — auth + Blob upload.
|
||||
- `components/CameraScanner.js` — upload frame on confirm.
|
||||
- `pages/api/user-cards.js` — accept `scan_image_url`.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- **OCR / identify pipeline changes** — convoys #2 and #3.
|
||||
- **Vocabulary rename** ("My Collection" / "Lists") — convoy
|
||||
`rename-collections-vocabulary` (#5).
|
||||
- **Schema cleanup** — global `cards.quantity` removal is
|
||||
`schema-cleanup-from-scanner-audit`, not here.
|
||||
|
||||
## Roles invoked
|
||||
|
||||
1. `role-ia-architect` — destination stack model, nav labels.
|
||||
2. `role-ux-reviewer` — bulk-scan flow, ownership badge placement.
|
||||
3. `role-architect` — API param contract, 3 briefs.
|
||||
4. `role-implementer` — 3 briefs (Briefs 2+3 parallel after Brief 1).
|
||||
5. `role-reviewer` + `role-design-system-auditor` + `role-a11y-auditor`.
|
||||
|
||||
## Todos
|
||||
|
||||
- [ ] IA: stack-destination information architecture
|
||||
- [ ] UX: condition/foil/quantity controls + ownership badge
|
||||
- [ ] Architect: brief decomposition + API body-param contract
|
||||
- [ ] Brief 1 — destination picker + session state
|
||||
- [ ] Brief 2 — metadata + ownership (after Brief 1)
|
||||
- [ ] Brief 3 — Blob persistence (after Brief 1)
|
||||
- [ ] Design-system: verify theme tokens on new components
|
||||
|
||||
## Operator action required
|
||||
|
||||
**None.** Assumes `BLOB_READ_WRITE_TOKEN` is already provisioned (avatar
|
||||
upload path uses Blob today).
|
||||
|
||||
## Multitask dispatch
|
||||
|
||||
### Slice dependencies
|
||||
|
||||
```yaml
|
||||
slice_dependencies:
|
||||
- brief: 1
|
||||
depends_on: []
|
||||
files:
|
||||
- pages/scanner.js
|
||||
- components/ScannerDestinationPicker.js
|
||||
- brief: 2
|
||||
depends_on: [1]
|
||||
files:
|
||||
- pages/scanner.js
|
||||
- components/ScannedCardItem.js
|
||||
- pages/api/user-cards.js
|
||||
- pages/api/decks/[id]/cards.js
|
||||
- pages/api/collections/[identifier]/cards.js
|
||||
- brief: 3
|
||||
depends_on: [1]
|
||||
files:
|
||||
- pages/api/scan/upload-image.js
|
||||
- components/CameraScanner.js
|
||||
- pages/api/user-cards.js
|
||||
```
|
||||
|
||||
**After Brief 1 merges:** `/multitask role-implementer briefs 2, 3`
|
||||
(disjoint file sets except shared `pages/scanner.js` / `user-cards.js` —
|
||||
architect must resolve: likely Brief 2 owns `scanner.js` queue UI, Brief 3
|
||||
owns CameraScanner + upload route only; adjust `files:` if conflict).
|
||||
|
||||
Post-PR audit:
|
||||
|
||||
```
|
||||
/multitask role-reviewer + role-design-system-auditor + role-a11y-auditor
|
||||
```
|
||||
|
||||
Group id: `audit-redesign-scanner-flow-<pr>`.
|
||||
|
||||
**Cross-convoy:** can run **parallel with #3** after #2 Brief 1+2 merge
|
||||
(disjoint primary surfaces).
|
||||
|
||||
## CI impact
|
||||
|
||||
| Workflow / job | Behavior |
|
||||
| --- | --- |
|
||||
| `preview-smoke.yml` | Fires. |
|
||||
| `visual-diff.yml` | **Fires** — `pages/scanner.js`, `components/**` match paths; `!pages/api/**` still allows page changes through. |
|
||||
| New grep gates | None. |
|
||||
|
||||
## Decisions to ratify (architect)
|
||||
|
||||
1. **Default destination** — last-used vs explicit pick-required on session start.
|
||||
2. **Condition enum** — align with existing `user_cards.condition` VARCHAR values.
|
||||
3. **Blob path convention** — e.g. `scans/{userId}/{uuid}.jpg`.
|
||||
4. **Brief 2 vs 3 file overlap** — split `pages/scanner.js` ownership to avoid multitask conflict.
|
||||
|
||||
## Known constraints
|
||||
|
||||
- **Theme tokens** — no hardcoded hex; use `var(--*)` per ui-and-theming rule.
|
||||
- **Rate limits** — upload-image may need `checkUploadRateLimit` if architect
|
||||
classifies scan images as upload class (Decision pending).
|
||||
- **Mobile scanner** — destination picker must work on narrow viewports.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
1. User selects one destination; all scans in session target it until changed.
|
||||
2. Condition, foil, quantity propagate on add-to-collection/deck/user-cards.
|
||||
3. Ownership badge shows when user already holds the card.
|
||||
4. Confirmed scan image URL stored on `user_cards` row.
|
||||
5. Lint + vitest baseline preserved.
|
||||
6. A11y: destination picker keyboard-operable; badge has accessible text.
|
||||
|
||||
## Out of scope follow-ups
|
||||
|
||||
- **`god-component-split`** — further split `pages/scanner.js` if still > 500 lines.
|
||||
- **`harden-multipart-parser`** — if upload-image uses multipart.
|
||||
142
.convoys/rename-collections-vocabulary.md
Normal file
142
.convoys/rename-collections-vocabulary.md
Normal file
|
|
@ -0,0 +1,142 @@
|
|||
---
|
||||
name: rename-collections-vocabulary
|
||||
classification: feature
|
||||
success_metric: |
|
||||
UI copy reads "My Collection" (ownership) and "Lists" / "Binders" (curated
|
||||
lists); no rendered string "Owned Cards" / "Mark Owned" / "All My Cards"
|
||||
remains in pages/ or components/; schema unchanged; AGENTS.md extended
|
||||
with vocabulary table.
|
||||
skip:
|
||||
- arch
|
||||
status: open
|
||||
created: 2026-05-27
|
||||
---
|
||||
|
||||
# Convoy: rename-collections-vocabulary
|
||||
|
||||
Align user-facing copy with the product taxonomy: owned cards vs curated lists.
|
||||
|
||||
## Why
|
||||
|
||||
The scanner audit and IA review found inconsistent vocabulary: "Owned Cards",
|
||||
"Mark Owned", and "All My Cards" imply a different mental model than the
|
||||
schema (global `cards` catalog vs per-user ownership via `user_cards` vs
|
||||
curated `collections`). Users confuse "my collection" (everything I own) with
|
||||
"lists/binders" (curated subsets). This convoy is copy + docs only — no
|
||||
schema migration.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- **Brief 1 — UI copy sweep** (~20 files in `pages/` + `components/`):
|
||||
- Replace stale strings per vocabulary table (IA architect supplies full
|
||||
inventory).
|
||||
- Target removals: `"Owned Cards"`, `"Mark Owned"`, `"All My Cards"`.
|
||||
- Canonical replacements: **"My Collection"** (ownership), **"Lists"** /
|
||||
**"Binders"** (curated lists).
|
||||
- **Brief 2 — docs + rules**
|
||||
- `AGENTS.md` § Branding — vocabulary table.
|
||||
- `.cursor/rules/ui-and-theming.mdc` — copy conventions.
|
||||
- `docs/SCHEMA_MAP.md` — clarify naming vs UI labels (no DDL change).
|
||||
- **`.github/workflows/ci.yml`** — new `forbidden-stale-strings` grep gate
|
||||
(~30s): fail if `"Mark Owned"`, `"Owned Cards"`, or `"All My Cards"`
|
||||
appear in `pages/` or `components/`.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- **Schema renames** — table/column names stay; UI copy only.
|
||||
- **URL slug changes** — `/collections` path unchanged in v1.
|
||||
- **Architecture decisions** — `skip: arch`; IA + UX run explicitly.
|
||||
|
||||
## Roles invoked
|
||||
|
||||
1. `role-ia-architect` — vocabulary table + file inventory (**primary owner**).
|
||||
2. `role-ux-reviewer` — scan flow + nav label consistency.
|
||||
3. `role-implementer` — 2 briefs (serial: Brief 2 after Brief 1).
|
||||
4. `role-reviewer` + `role-design-system-auditor` + `role-a11y-auditor` —
|
||||
copy changes affect screen reader strings.
|
||||
|
||||
Note: **`role-architect` skipped** per `skip: arch`. IA architect owns
|
||||
taxonomy; implementer briefs written by IA + conductor handoff or parent
|
||||
agent.
|
||||
|
||||
## Todos
|
||||
|
||||
- [ ] IA: publish vocabulary table + grep inventory of stale strings
|
||||
- [ ] UX: review scanner + nav + collection views for consistency
|
||||
- [ ] Brief 1 — pages/ + components/ copy sweep
|
||||
- [ ] Brief 2 — AGENTS.md + rules + SCHEMA_MAP glossary
|
||||
- [ ] Add `forbidden-stale-strings` CI job
|
||||
|
||||
## Operator action required
|
||||
|
||||
**None.**
|
||||
|
||||
## Multitask dispatch
|
||||
|
||||
### Slice dependencies
|
||||
|
||||
```yaml
|
||||
slice_dependencies:
|
||||
- brief: 1
|
||||
depends_on: []
|
||||
files:
|
||||
- pages/**/*.js
|
||||
- components/*.js
|
||||
notes: exclude pages/api/**
|
||||
- brief: 2
|
||||
depends_on: [1]
|
||||
files:
|
||||
- AGENTS.md
|
||||
- .cursor/rules/ui-and-theming.mdc
|
||||
- docs/SCHEMA_MAP.md
|
||||
- .github/workflows/ci.yml
|
||||
```
|
||||
|
||||
Serial: Brief 2 after Brief 1 (docs reference final copy).
|
||||
|
||||
**Cross-convoy:** parallel with #1+#2 after `secure-scanner-gemini-key`
|
||||
merges — `/multitask role-implementer` **#5 Brief 1 + #6 + #2 Brief 1**
|
||||
(disjoint files).
|
||||
|
||||
Post-PR audit:
|
||||
|
||||
```
|
||||
/multitask role-reviewer + role-design-system-auditor + role-a11y-auditor
|
||||
```
|
||||
|
||||
Group id: `audit-rename-collections-vocabulary-<pr>`.
|
||||
|
||||
## CI impact
|
||||
|
||||
| Workflow / job | Behavior |
|
||||
| --- | --- |
|
||||
| `forbidden-stale-strings` | **New blocking job** — grep `pages/` + `components/`. |
|
||||
| `visual-diff.yml` | **Likely fires** — widespread UI string changes in pages/components. |
|
||||
| `preview-smoke.yml` | Fires; sign-in CTA wording must stay smoke-compatible. |
|
||||
|
||||
**Smoke caveat:** smoke test 2 asserts `/sign in/i` on login page — do not
|
||||
rename that CTA in this convoy.
|
||||
|
||||
## Decisions to ratify (IA architect)
|
||||
|
||||
1. **"Lists" vs "Binders"** — when to use each term in nav vs empty states.
|
||||
2. **Scanner button label** — replacement for "Mark Owned" (e.g. "Add to
|
||||
session" vs "Confirm card").
|
||||
3. **Admin UI** — out of copy sweep or separate pass?
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
1. Zero rendered occurrences of the three forbidden strings in `pages/` +
|
||||
`components/`.
|
||||
2. Vocabulary table committed in `AGENTS.md`.
|
||||
3. `forbidden-stale-strings` CI green.
|
||||
4. Schema DDL unchanged (grep `migrations/` — no new files).
|
||||
5. Vitest 21/21; smoke 3/3 (sign-in CTA intact).
|
||||
|
||||
## Out of scope follow-ups
|
||||
|
||||
- **`schema-cleanup-from-scanner-audit`** — `is_system_collection` vs
|
||||
`user_cards` unification (separate convoy).
|
||||
- **`rename-repo-and-vercel-project`** — infra naming, not UI copy.
|
||||
109
.convoys/scanner-correctness-polish.md
Normal file
109
.convoys/scanner-correctness-polish.md
Normal file
|
|
@ -0,0 +1,109 @@
|
|||
---
|
||||
name: scanner-correctness-polish
|
||||
classification: infra-only
|
||||
success_metric: |
|
||||
Mark-Owned is idempotent (in-flight lock per queue row); bulk toolbar no
|
||||
longer uses setTimeout(..., 100); pages/scanner.js imports lib/use-auth
|
||||
not legacy auth-context; POST /api/collections/[identifier]/cards calls
|
||||
logCollectionActivity('card_added', …).
|
||||
skip:
|
||||
- ia
|
||||
- ux
|
||||
- visual
|
||||
- a11y
|
||||
- design
|
||||
status: open
|
||||
created: 2026-05-27
|
||||
---
|
||||
|
||||
# Convoy: scanner-correctness-polish
|
||||
|
||||
Fix scanner-page correctness bugs without changing UX or visual design.
|
||||
|
||||
## Why
|
||||
|
||||
The scanner audit surfaced non-security bugs that cause duplicate adds,
|
||||
racey bulk actions, a stale auth import, and missing activity logs. These
|
||||
are small fixes with high reliability impact during bulk scanning sessions.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- **`pages/scanner.js`**
|
||||
- Line 8: change import from legacy `lib/auth-context` to `lib/use-auth`
|
||||
(should have been swept by `single-auth-provider`; scanner was missed or
|
||||
regressed).
|
||||
- **Mark-Owned idempotency** — in-flight lock per queue row so double-tap
|
||||
/ double-click cannot duplicate POSTs.
|
||||
- **Bulk toolbar** — remove `setTimeout(..., 100)` race; await or use
|
||||
proper batch completion signal.
|
||||
- **`pages/api/collections/[identifier]/cards.js`** — POST handler calls
|
||||
`logCollectionActivity(collectionId, userId, 'card_added', details)` per
|
||||
AGENTS.md convention.
|
||||
|
||||
### Out of scope
|
||||
|
||||
- **Scanner UX redesign** — convoy `redesign-scanner-flow` (#4).
|
||||
- **Copy / vocabulary** — convoy `rename-collections-vocabulary` (#5).
|
||||
- **OCR / identify pipeline** — convoys #2–#3.
|
||||
- **Visual or a11y changes** — none intended; diff should be behavior-only.
|
||||
|
||||
## Roles invoked
|
||||
|
||||
1. `role-architect` — single brief (lightweight; may be parent-owned given
|
||||
infra-only classification).
|
||||
2. `role-implementer` — one brief.
|
||||
3. `role-reviewer` — post-PR only (design + a11y skipped).
|
||||
|
||||
## Todos
|
||||
|
||||
- [ ] Architect: brief-1 with exact line targets
|
||||
- [ ] Fix use-auth import on scanner page
|
||||
- [ ] Add per-row in-flight lock for Mark-Owned
|
||||
- [ ] Replace setTimeout bulk-toolbar pattern
|
||||
- [ ] Wire logCollectionActivity on collection card POST
|
||||
|
||||
## Operator action required
|
||||
|
||||
**None.**
|
||||
|
||||
## Multitask dispatch
|
||||
|
||||
Single brief — no implementer fan-out.
|
||||
|
||||
**Cross-convoy parallelism:** after #1 merges, run alongside **#2 Brief 1**
|
||||
and **#5 Brief 1** — disjoint files (`pages/scanner.js` vs migrations vs
|
||||
copy sweep). Coordinate if both #6 and #5 touch `pages/scanner.js` (IA
|
||||
should sequence copy sweep after correctness or split files in briefs).
|
||||
|
||||
Post-PR audit:
|
||||
|
||||
```
|
||||
/multitask role-reviewer
|
||||
```
|
||||
|
||||
Group id: `audit-scanner-correctness-polish-<pr>`.
|
||||
|
||||
## CI impact
|
||||
|
||||
| Workflow / job | Behavior |
|
||||
| --- | --- |
|
||||
| `ci.yml` | Standard lint + vitest; no new gates. |
|
||||
| `preview-smoke.yml` | Fires. |
|
||||
| `visual-diff.yml` | **May fire** if `pages/scanner.js` changes — behavior-only diff should not move pixels; baseline swallow if triggered. |
|
||||
|
||||
## Known constraints
|
||||
|
||||
- **`logCollectionActivity` import** — use existing helper from
|
||||
permission-middleware or documented activity module; match sibling handlers.
|
||||
- **No UX copy changes** — button labels stay as-is until #5 (or avoid
|
||||
overlapping scanner.js edits between #5 and #6).
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
1. `pages/scanner.js` imports from `lib/use-auth` only.
|
||||
2. Double-click Mark-Owned produces one POST.
|
||||
3. Bulk toolbar actions complete without setTimeout race.
|
||||
4. Collection card POST emits activity log row.
|
||||
5. Vitest 21/21; lint baseline preserved.
|
||||
124
.convoys/secure-scanner-gemini-key.md
Normal file
124
.convoys/secure-scanner-gemini-key.md
Normal file
|
|
@ -0,0 +1,124 @@
|
|||
---
|
||||
name: secure-scanner-gemini-key
|
||||
classification: server-only
|
||||
success_metric: |
|
||||
GEMINI_AI_API_KEY rotated; no endpoint returns the key to a browser;
|
||||
lib/rate-limit.js exposes a sixth scan class (5 req / 1 min, user-keyed);
|
||||
forbidden-client-side-llm-keys CI gate green.
|
||||
skip:
|
||||
- ia
|
||||
- ux
|
||||
- visual
|
||||
- a11y
|
||||
- design
|
||||
status: in-progress
|
||||
created: 2026-05-27
|
||||
---
|
||||
|
||||
# Convoy: secure-scanner-gemini-key
|
||||
|
||||
Stop leaking the Gemini API key to browsers and add a scan-specific rate
|
||||
limiter before the server-side scan pipeline lands in convoy #2.
|
||||
|
||||
## Why
|
||||
|
||||
The scanner audit found that `pages/api/config/gemini.js` returns
|
||||
`process.env.GEMINI_AI_API_KEY` to any authenticated caller, and
|
||||
`components/CameraScanner.js` auto-fetches that endpoint on mount (lines
|
||||
53–67). A logged-in user — or anyone who obtains a session token — can
|
||||
read the production LLM key from DevTools and call Gemini directly,
|
||||
bypassing app rate limits and billing controls. This is a **live secret
|
||||
exposure** that must be closed before any further scanner work ships.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- **Delete** `pages/api/config/gemini.js` — remove the key-returning
|
||||
endpoint entirely.
|
||||
- **`components/CameraScanner.js`** — remove the auto-load block at lines
|
||||
53–67 that fetches `/api/config/gemini` and stores the key client-side.
|
||||
The scanner will break temporarily until convoy #2 lands; that is
|
||||
acceptable for the security-only window.
|
||||
- **`lib/rate-limit.js`** — add a sixth `scan` entry to `LIMITER_CONFIG`
|
||||
(5 req / 1 min, user-keyed) and export `checkScanRateLimit(req,
|
||||
userId)` following the Decision-2 hybrid named-limiter pattern from
|
||||
`.convoys/add-rate-limiting.md`. Redis prefix: `deckhearth:scan`.
|
||||
- **`.github/workflows/ci.yml`** — add a new blocking
|
||||
`forbidden-client-side-llm-keys` job (~30s grep gate, same shape as
|
||||
`forbidden-cors-headers`):
|
||||
- Fail if `apiKey:` appears under `pages/api/config/*`.
|
||||
- Fail if `generativelanguage.googleapis.com` or `api.openai.com`
|
||||
appears outside `pages/api/` (client-side LLM URL leakage).
|
||||
- **`.cursor/rules/api-routes.mdc`** § Rate limiting — document the new
|
||||
`scan` class (architect may fold into brief or defer to convoy #2;
|
||||
implementer should at minimum add the lib export).
|
||||
|
||||
### Out of scope
|
||||
|
||||
- **`/api/scan/identify`** server route — convoy `server-side-scan-pipeline` (#2).
|
||||
- **Deleting `lib/ai-ocr.js` browser classes** (`GeminiVisionOCR`,
|
||||
`AICardOCR`, etc.) — deferred to #2 to avoid breaking the live scanner
|
||||
during this security-only window (the client still needs those classes
|
||||
until the server pipeline replaces them).
|
||||
- **Rotating the key in source control** — the key lives in Vercel env
|
||||
only; rotation is operator action (see below).
|
||||
|
||||
## Roles invoked
|
||||
|
||||
1. `role-architect` — ratify scan limiter values; confirm grep patterns
|
||||
for the CI gate; single brief decomposition.
|
||||
2. `role-implementer` — one brief, no fan-out.
|
||||
3. `role-reviewer` — post-PR audit (design + a11y skipped per `skip:`).
|
||||
|
||||
## Todos
|
||||
|
||||
- [ ] Architect: write `.convoys/secure-scanner-gemini-key/brief-1-*.md`
|
||||
- [x] Implementer: delete config endpoint + client auto-load; extend rate-limit lib
|
||||
- [x] Implementer: add `forbidden-client-side-llm-keys` CI job
|
||||
- [x] Operator: rotate `GEMINI_AI_API_KEY` in Google AI Studio + Vercel **before merge**
|
||||
- [ ] Reviewer: verify no key material in diff or CI logs
|
||||
|
||||
## Operator action required
|
||||
|
||||
**URGENT — rotate before merge:**
|
||||
|
||||
1. **Google AI Studio** — revoke the current `GEMINI_AI_API_KEY` and
|
||||
issue a new key. The old key has been exposed to every browser session
|
||||
that loaded the scanner page.
|
||||
2. **Vercel** — update the `GEMINI_AI_API_KEY` env var on Production +
|
||||
Preview to the new value.
|
||||
3. **Verify** — after deploy, confirm `GET /api/config/gemini` returns
|
||||
404 and no network tab in the scanner shows key material.
|
||||
|
||||
Do **not** merge this PR until rotation is complete. The CI gate prevents
|
||||
re-introduction of the leak pattern but does not invalidate a key that
|
||||
was already exfiltrated.
|
||||
|
||||
## Multitask dispatch
|
||||
|
||||
Single brief — **no implementer fan-out**.
|
||||
|
||||
Post-PR audit fan-out:
|
||||
|
||||
```
|
||||
/multitask role-reviewer
|
||||
```
|
||||
|
||||
Group id: `audit-secure-scanner-gemini-key-<pr>` (design-system + a11y
|
||||
auditors skipped per `skip:` flags).
|
||||
|
||||
**Cross-convoy parallelism (after #1 merges):** open three worktrees and
|
||||
`/multitask role-implementer` on **#2 Brief 1 + #5 Brief 1 + #6 (sole
|
||||
brief)** — disjoint file sets (migrations / UI strings / scanner polish).
|
||||
|
||||
## CI impact
|
||||
|
||||
| Workflow / job | Behavior |
|
||||
| --- | --- |
|
||||
| `ci.yml` → `forbidden-client-side-llm-keys` | **New blocking job.** ~30s grep; no `npm ci`. |
|
||||
| `ci.yml` → lint, vitest, forbidden-endpoints, forbidden-cors-headers | Unchanged; must stay green. |
|
||||
| `preview-smoke.yml` | Fires (no `paths:` filter). Scanner page may error until #2; smoke spec does not exercise scanner. |
|
||||
| `visual-diff.yml` | **Does not fire** — API-only + `lib/` + CI YAML; `!pages/api/**` exclusion applies. |
|
||||
|
||||
**Preserve `pipeline: skip smoke`** PR-body directive for iterative pushes.
|
||||
212
.convoys/server-side-scan-pipeline.md
Normal file
212
.convoys/server-side-scan-pipeline.md
Normal file
|
|
@ -0,0 +1,212 @@
|
|||
---
|
||||
name: server-side-scan-pipeline
|
||||
classification: feature
|
||||
success_metric: |
|
||||
Scanner identifies cards via server-owned /api/scan/identify (no client-side
|
||||
LLM keys); unknown cards write to card_submissions for admin review (no
|
||||
user-writable cards INSERT); disambiguation envelope consumed by the client
|
||||
(needsUserSelection no longer silently swallowed).
|
||||
skip: []
|
||||
status: open
|
||||
created: 2026-05-27
|
||||
depends_on:
|
||||
- secure-scanner-gemini-key
|
||||
---
|
||||
|
||||
# Convoy: server-side-scan-pipeline
|
||||
|
||||
Move card identification entirely server-side, stop users from INSERTing
|
||||
into the global `cards` catalog, and surface disambiguation when OCR is
|
||||
ambiguous.
|
||||
|
||||
## Why
|
||||
|
||||
After convoy #1 closes the key leak, the scanner still runs Gemini Vision
|
||||
in the browser via `lib/ai-ocr.js`. That architecture cannot be secured:
|
||||
any client-side LLM call requires credentials in JS bundle or runtime fetches.
|
||||
Additionally, `pages/api/cards/find-or-create.js` lets authenticated users
|
||||
INSERT new rows into the global `cards` table — polluting the shared catalog
|
||||
with OCR garbage. The audit also found that `CameraScanner.js` silently
|
||||
swallows `needsUserSelection: true` from the identify response, forcing
|
||||
wrong-card adds without user confirmation.
|
||||
|
||||
## Scope
|
||||
|
||||
### In scope
|
||||
|
||||
- **Migration** under `migrations/` (via `npm run migrate create`):
|
||||
- `card_submissions` — id, user_id, ocr_text, ocr_confidence,
|
||||
scan_image_url, candidate_card_ids, status, reviewed_by, created_at.
|
||||
- `scan_attempts` — per audit § 5.2 (layer, outcome, timing fields).
|
||||
- Update `docs/SCHEMA_MAP.md`.
|
||||
- **`pages/api/scan/identify.js`** (new) — auth +
|
||||
`checkScanRateLimit(req, user.userId)` + server-side Gemini Flash via
|
||||
`process.env.GEMINI_AI_API_KEY`; returns match / disambiguation /
|
||||
unknown envelope.
|
||||
- **`pages/api/cards/find-or-create.js`** — delete the user-writable
|
||||
INSERT path; unknown cards write to `card_submissions` instead.
|
||||
- **`lib/ai-ocr.js`** — delete browser classes: `GeminiVisionOCR`,
|
||||
`AICardOCR`, `OllamaVisionOCR`, `PuterVisionOCR`. Keep any shared
|
||||
utilities the server route needs (architect decides split).
|
||||
- **`components/CameraScanner.js`** — rewrite `processConfirmedCard` to
|
||||
call `/api/scan/identify`; render disambiguation UI when
|
||||
`needsUserSelection: true`.
|
||||
- **`pages/scanner.js`** — wire queue + error states to new API contract.
|
||||
- **`pages/admin/card-submissions.js`** (new) — admin review queue;
|
||||
promote-to-`cards` action.
|
||||
- **`pages/api/admin/card-submissions/**`** (new) — list, approve, reject.
|
||||
- **`.cursor/rules/api-routes.mdc`** § Rate limiting — document `scan` class
|
||||
if not fully done in #1.
|
||||
- **Extend `forbidden-client-side-llm-keys`** — also fail if
|
||||
`import.*ai-ocr` appears in `components/` (browser classes deleted).
|
||||
|
||||
### Out of scope
|
||||
|
||||
- **Tesseract / pg_trgm Layer-1 OCR** — convoy `add-real-ocr-layer` (#3).
|
||||
- **Stack-destination UX, condition/foil/quantity** — convoy
|
||||
`redesign-scanner-flow` (#4).
|
||||
- **OpenCV perspective transform** — queued as
|
||||
`improve-scan-card-detection` if Layer-1 hit rate stays below 70%.
|
||||
- **Schema cleanup** (`cards.quantity`, dual visibility flags) — see
|
||||
`schema-cleanup-from-scanner-audit` stub in `.convoys/ship-readiness.md`.
|
||||
|
||||
## Roles invoked
|
||||
|
||||
1. `role-ia-architect` — admin review queue IA, disambiguation flow labels.
|
||||
2. `role-ux-reviewer` — disambiguation picker UX, error/loading states.
|
||||
3. `role-architect` — API contract, migration shape, brief decomposition.
|
||||
4. `role-implementer` — 4 briefs (2 parallel at gate 1).
|
||||
5. `role-reviewer` + `role-design-system-auditor` + `role-a11y-auditor` —
|
||||
post-PR audit fan-out.
|
||||
|
||||
## Todos
|
||||
|
||||
- [ ] IA: admin card-submissions queue information architecture
|
||||
- [ ] UX: disambiguation picker + scanner error states
|
||||
- [ ] Architect: ratify Decisions 1–5; write 4 briefs + `slice_dependencies`
|
||||
- [ ] Brief 1 — migration + SCHEMA_MAP
|
||||
- [ ] Brief 2 — `/api/scan/identify` server route
|
||||
- [ ] Brief 3 — client disambiguation + delete browser LLM classes (after Brief 2)
|
||||
- [ ] Brief 4 — admin queue + replace find-or-create INSERT (after Briefs 1+2)
|
||||
- [ ] Extend CI gate to forbid `import.*ai-ocr` in `components/`
|
||||
|
||||
## Operator action required
|
||||
|
||||
**None beyond #1.** Assumes `GEMINI_AI_API_KEY` is already rotated and
|
||||
only present server-side. Confirm Vercel env var is set before testing
|
||||
`/api/scan/identify` on preview.
|
||||
|
||||
## Multitask dispatch
|
||||
|
||||
### Slice dependencies (multitask-ready)
|
||||
|
||||
```yaml
|
||||
slice_dependencies:
|
||||
- brief: 1
|
||||
depends_on: []
|
||||
files:
|
||||
- migrations/*
|
||||
- docs/SCHEMA_MAP.md
|
||||
- brief: 2
|
||||
depends_on: []
|
||||
files:
|
||||
- pages/api/scan/**
|
||||
- brief: 3
|
||||
depends_on: [2]
|
||||
files:
|
||||
- components/CameraScanner.js
|
||||
- lib/ai-ocr.js
|
||||
- pages/scanner.js
|
||||
- brief: 4
|
||||
depends_on: [1, 2]
|
||||
files:
|
||||
- pages/api/cards/find-or-create.js
|
||||
- pages/admin/card-submissions.js
|
||||
- pages/api/admin/card-submissions/**
|
||||
```
|
||||
|
||||
**Gate 1:** `/multitask role-implementer briefs 1, 2` (disjoint files).
|
||||
|
||||
**Gate 2:** Brief 3 after Brief 2 merges; Brief 4 after Briefs 1+2 merge
|
||||
(Briefs 3 and 4 can run sequentially or Brief 4 parallel with Brief 3 if
|
||||
Brief 2 is merged — file sets are disjoint between 3 and 4).
|
||||
|
||||
Post-PR audit fan-out:
|
||||
|
||||
```
|
||||
/multitask role-reviewer + role-design-system-auditor + role-a11y-auditor
|
||||
```
|
||||
|
||||
Group id: `audit-server-side-scan-pipeline-<pr>`.
|
||||
|
||||
**Cross-convoy (after Brief 1 + Brief 2 merge):** `/multitask` **#3 Brief 1
|
||||
+ #4 Brief 1** — disjoint (DB+route / UX restructure).
|
||||
|
||||
## CI impact
|
||||
|
||||
| Workflow / job | Behavior |
|
||||
| --- | --- |
|
||||
| `forbidden-client-side-llm-keys` | **Extended** — grep for `import.*ai-ocr` in `components/`. |
|
||||
| `ci.yml` → lint, vitest, schema-map-fresh | Fires on migration + SCHEMA_MAP changes. |
|
||||
| `preview-smoke.yml` | Fires; scanner not in smoke spec today. |
|
||||
| `visual-diff.yml` | **Fires** — touches `pages/scanner.js`, `pages/admin/**`, `components/CameraScanner.js`. `continue-on-error: true` until baselines seeded. |
|
||||
|
||||
## Decisions to ratify (architect)
|
||||
|
||||
1. **`/api/scan/identify` request shape.** Image as base64 in JSON vs.
|
||||
multipart vs. Blob URL reference? Parent recommends base64 for v1
|
||||
(matches current canvas capture); architect confirms payload size
|
||||
limits + Vercel function timeout budget.
|
||||
|
||||
2. **Disambiguation response envelope.** Verbatim fields:
|
||||
`{ needsUserSelection, candidates: [{ id, name, set, score }], … }`.
|
||||
Architect locks the contract before Brief 2 ships.
|
||||
|
||||
3. **`card_submissions.status` enum.** Recommend:
|
||||
`pending | approved | rejected`. Promotion copies vetted row into
|
||||
`cards` with admin attribution.
|
||||
|
||||
4. **What happens to `find-or-create` callers?** Inventory all fetch sites;
|
||||
migrate to `/api/scan/identify` + submissions path. Architect lists
|
||||
in brief.
|
||||
|
||||
5. **Gemini model + prompt ownership.** Server-side only; single module
|
||||
(extend existing server helper or new `lib/scan-identify.js`).
|
||||
|
||||
## Known constraints
|
||||
|
||||
- **`checkScanRateLimit` gate-ordering** — MUST sit AFTER
|
||||
`getUserFromRequest` per `.convoys/add-rate-limiting.md` Decision 4
|
||||
(`extractUserIdentifier` THROWS without userId).
|
||||
- **`GEMINI_AI_API_KEY` server-only** — never returned in JSON; never
|
||||
logged. CI gate from #1 is the regression lock.
|
||||
- **No user INSERT into `cards`** — any path that let non-admins create
|
||||
catalog rows must be removed or admin-gated.
|
||||
- **Historical `scripts/add-*`** — no-go-zone; schema via `migrations/` only.
|
||||
- **Card-import rate limits unchanged** — import routes stay admin-only;
|
||||
scan limiter is a separate class.
|
||||
|
||||
## Acceptance criteria
|
||||
|
||||
1. Zero client-side references to `GEMINI_AI_API_KEY`, `apiKey:`, or
|
||||
direct `generativelanguage.googleapis.com` calls outside `pages/api/`.
|
||||
2. `GET/POST /api/scan/identify` requires auth; returns 401 without
|
||||
token; returns 429 when scan limit exceeded.
|
||||
3. Authenticated scan of unknown card creates `card_submissions` row, NOT
|
||||
a `cards` row.
|
||||
4. Disambiguation UI renders when API returns `needsUserSelection: true`;
|
||||
user must pick before card enters queue.
|
||||
5. Admin can list + approve/reject submissions at `/admin/card-submissions`.
|
||||
6. Approve action promotes to `cards` with audit trail (`reviewed_by`).
|
||||
7. `forbidden-client-side-llm-keys` CI job green including `components/`
|
||||
ai-ocr import check.
|
||||
8. `npm run lint` baseline preserved; `npm run test:run` 21/21 green.
|
||||
9. `docs/SCHEMA_MAP.md` updated for new tables.
|
||||
|
||||
## Out of scope follow-ups
|
||||
|
||||
- **`add-real-ocr-layer`** (#3) — Tesseract + pg_trgm before Gemini.
|
||||
- **`redesign-scanner-flow`** (#4) — destination stack, condition/foil.
|
||||
- **`improve-scan-card-detection`** — OpenCV perspective if Layer-1 < 70%.
|
||||
- **`fill-vitest-handler-coverage`** — per-route handler tests deferred.
|
||||
- **`schema-cleanup-from-scanner-audit`** — global catalog column smells.
|
||||
|
|
@ -484,6 +484,53 @@ Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch seq
|
|||
- **`audit-node-pg-migrate-transitive-deps`** (priority: P3 hygiene). Surfaced 2026-05-26 by `migration-tool` (PR #32) — R5 in the convoy file. `npm audit` reports 11 vulnerabilities (6 moderate, 5 high) coming from `node-pg-migrate@8.0.4`'s `glob@~11.1.0` + `yargs@~17.7.0` transitive deps (older `brace-expansion`, `minimatch`, `picomatch` versions with known advisories). All in dev-only paths; the migration tool runs in scripts/CI, never in the deployed Next.js bundle, and the affected APIs (glob's shell-injection CLI; brace-expansion's ReDoS) are not exercised by node-pg-migrate's call sites. Surface only if a security audit specifically flags this surface, or if `node-pg-migrate` ships a v9 that updates the transitive tree.
|
||||
- **`add-migration-template`** (priority: P3 DX). Surfaced 2026-05-26 by `migration-tool` (PR #32). Add a custom template via `--template-file-name` so generated migrations include the project's preferred docstring shape + a reminder about `docs/SCHEMA_MAP.md` updates. Surface if migration authoring proves inconsistent.
|
||||
|
||||
### Scanner audit portfolio (2026-05-27)
|
||||
|
||||
Six convoys authored from the scanner audit portfolio plan. Dependency order:
|
||||
`secure-scanner-gemini-key` → `server-side-scan-pipeline` → (`add-real-ocr-layer` ∥
|
||||
`redesign-scanner-flow`); `rename-collections-vocabulary` and
|
||||
`scanner-correctness-polish` parallel after #1.
|
||||
|
||||
- **`secure-scanner-gemini-key`** (priority: **P0 security** — ships first).
|
||||
Delete `pages/api/config/gemini.js`; remove CameraScanner client key auto-load;
|
||||
add sixth `scan` rate-limit class; new `forbidden-client-side-llm-keys` CI gate.
|
||||
**Operator action:** rotate `GEMINI_AI_API_KEY` in Google AI Studio + Vercel
|
||||
before merge. Convoy: `.convoys/secure-scanner-gemini-key.md`.
|
||||
- **`server-side-scan-pipeline`** (priority: P1 feature;
|
||||
`depends_on: secure-scanner-gemini-key`). Server-owned `/api/scan/identify`;
|
||||
`card_submissions` + `scan_attempts` tables; remove user-writable `cards`
|
||||
INSERT; admin review queue; client disambiguation UI. Four briefs; gate-1
|
||||
`/multitask` briefs 1+2. Convoy: `.convoys/server-side-scan-pipeline.md`.
|
||||
- **`add-real-ocr-layer`** (priority: P1 feature;
|
||||
`depends_on: server-side-scan-pipeline`). Tesseract Worker + `pg_trgm`
|
||||
identify-by-text route; ≥70% Layer-1 hit rate via `scan_attempts.layer`.
|
||||
Piggybacks `schema-map-fresh` CI path fix for `migrations/`. Convoy:
|
||||
`.convoys/add-real-ocr-layer.md`.
|
||||
- **`redesign-scanner-flow`** (priority: P1 feature;
|
||||
`depends_on: server-side-scan-pipeline`). Stack-destination UX,
|
||||
condition/foil/quantity, ownership badge, Blob scan-image persistence. Three
|
||||
briefs; `/multitask` briefs 2+3 after brief 1. Can run parallel with
|
||||
`add-real-ocr-layer`. Convoy: `.convoys/redesign-scanner-flow.md`.
|
||||
- **`rename-collections-vocabulary`** (priority: P2 IA/copy; parallel to #1+#2).
|
||||
"My Collection" / "Lists" / "Binders" copy sweep; `forbidden-stale-strings`
|
||||
CI gate; `AGENTS.md` vocabulary table. Skips architect (`skip: arch`); IA +
|
||||
UX run. Convoy: `.convoys/rename-collections-vocabulary.md`.
|
||||
- **`scanner-correctness-polish`** (priority: P2 infra; parallel to #1+#2).
|
||||
Idempotent Mark-Owned, fix bulk `setTimeout` race, `lib/use-auth` import,
|
||||
`logCollectionActivity` on collection card POST. Single brief. Convoy:
|
||||
`.convoys/scanner-correctness-polish.md`.
|
||||
- **`schema-cleanup-from-scanner-audit`** (priority: P2 schema; **deferred** —
|
||||
NOT scanner-specific). Separate convoy when ready; surfaced by the scanner
|
||||
audit but applies globally:
|
||||
1. **`cards.quantity` + `cards.favorited`** on the global catalog — belong on
|
||||
`user_cards` / `user_favorites`; drop from `cards`.
|
||||
2. **`collections.is_public` vs `visibility`** — dual visibility flags;
|
||||
reconcile to one mechanism (see also P2 §14 in this file).
|
||||
3. **`is_system_collection` vs `user_cards` unification** — ownership model
|
||||
smell; IA + schema convoy, not a scanner deliverable.
|
||||
Do not fold into the six scanner convoys above; queue as its own architect-led
|
||||
migration convoy after the scan pipeline stabilizes.
|
||||
|
||||
## Self-analytics
|
||||
|
||||
After each convoy, `scripts/log-convoy-event.sh` emits a record to `.convoys/.metrics.jsonl` (gitignored). After 3-5 convoys, run the upstream `agent-pipeline/analytics/` aggregator to see where token spend goes — that data feeds whether to add or remove rules.
|
||||
|
|
|
|||
|
|
@ -103,7 +103,7 @@ await logCollectionActivity(collectionId, userId, 'card_added', { cardId, quanti
|
|||
|
||||
## Rate limiting
|
||||
|
||||
`lib/rate-limit.js` exposes five named limiters, one per route class. Each named export takes `req` (and `userId` for user-keyed classes) and returns `{ allowed, remaining, reset }`. The auth-only limiter shipped in `fix-auth-bypass` Brief 4 (commit `297afca`, login + register); the four remaining classes — `search`, `upload`, `generate`, `import` — and the seven currently-gated routes shipped in `add-rate-limiting` (squash commit `708ef45`, PR #20, 2026-05-24), the convoy that closed P0 #6 and brought the launch-readiness P0 set to 8/8 RESOLVED. The five Redis key prefixes were renamed `tcgvault:*` → `deckhearth:*` in `pick-a-name` (squash commit `9abbab6`, PR #21, 2026-05-24) — call shape, return shape, and gate-ordering rules below are byte-identical post-rename; only the on-Redis namespace changed (one-time per-window counter reset accepted).
|
||||
`lib/rate-limit.js` exposes six named limiters, one per route class. Each named export takes `req` (and `userId` for user-keyed classes) and returns `{ allowed, remaining, reset }`. The auth-only limiter shipped in `fix-auth-bypass` Brief 4 (commit `297afca`, login + register); the four remaining classes — `search`, `upload`, `generate`, `import` — and the seven currently-gated routes shipped in `add-rate-limiting` (squash commit `708ef45`, PR #20, 2026-05-24), the convoy that closed P0 #6 and brought the launch-readiness P0 set to 8/8 RESOLVED. The `scan` class shipped in `secure-scanner-gemini-key` (2026-05-27) for `/api/scan/identify` (wired in `server-side-scan-pipeline`). The six Redis key prefixes were renamed `tcgvault:*` → `deckhearth:*` in `pick-a-name` (squash commit `9abbab6`, PR #21, 2026-05-24) — call shape, return shape, and gate-ordering rules below are byte-identical post-rename; only the on-Redis namespace changed (one-time per-window counter reset accepted).
|
||||
|
||||
| Class | Limit | Window | Key | Used by | Helper |
|
||||
| --- | --- | --- | --- | --- | --- |
|
||||
|
|
@ -112,8 +112,9 @@ await logCollectionActivity(collectionId, userId, 'card_added', { cardId, quanti
|
|||
| `upload` | 10 | 1 hour | user | `/api/user/avatar` | `checkUploadRateLimit(req, userId)` |
|
||||
| `generate` | 5 | 1 hour | user | `/api/user/avatar/generate` | `checkGenerateRateLimit(req, userId)` |
|
||||
| `import` | 5 | 1 hour | user | `/api/cards/import-mtg`, `/api/cards/import-pokemon`, `/api/cards/import-lorcana` | `checkImportRateLimit(req, userId)` |
|
||||
| `scan` | 5 | 1 min | user | `/api/scan/identify` | `checkScanRateLimit(req, userId)` |
|
||||
|
||||
**Verbatim call shape** (identical across all five classes — only the helper name and the optional `userId` argument differ):
|
||||
**Verbatim call shape** (identical across all six classes — only the helper name and the optional `userId` argument differ):
|
||||
|
||||
```js
|
||||
import { checkSearchRateLimit } from '../../../lib/rate-limit.js';
|
||||
|
|
@ -142,7 +143,7 @@ export default async function handler(req, res) {
|
|||
**Gate ordering rules:**
|
||||
|
||||
1. **Method check first.** Reject the wrong verb with 405 before doing any limiter work.
|
||||
2. **Auth check before any user-keyed limiter.** `extractUserIdentifier(userId)` THROWS when `userId` is null/undefined/empty (defensive). For `upload`, `generate`, and `import`, the handler MUST call `getUserFromRequest(req)` (or equivalent JWT verification) and confirm a non-null user BEFORE calling the limiter. Wrong order = anonymous user bypasses (the THROW surfaces immediately during dev; do not catch and silently fall back to IP).
|
||||
2. **Auth check before any user-keyed limiter.** `extractUserIdentifier(userId)` THROWS when `userId` is null/undefined/empty (defensive). For `upload`, `generate`, `import`, and `scan`, the handler MUST call `getUserFromRequest(req)` (or equivalent JWT verification) and confirm a non-null user BEFORE calling the limiter. Wrong order = anonymous user bypasses (the THROW surfaces immediately during dev; do not catch and silently fall back to IP).
|
||||
3. **For IP-keyed limiters (`auth`, `search`), gate placement is flexible** — either at the top of the handler (after the method check) or after a separate auth check that the route happens to also have (e.g. `users/search` JWT-verifies before rate-limiting, both are correct). The limiter only needs `req` for IP extraction.
|
||||
4. **Admin-role check, if applicable, goes between auth and rate-limit.** Used by all three `/api/cards/import-*` routes: `if (user.role !== 'admin') return res.status(403).json({ error: 'Admin access required' })` sits between the `if (!user)` 401 and the import rate-limit call.
|
||||
|
||||
|
|
@ -153,7 +154,7 @@ export default async function handler(req, res) {
|
|||
|
||||
**Env vars (unchanged from Brief 4):** `KV_REST_API_URL` + `KV_REST_API_TOKEN` (auto-provisioned by Vercel's Upstash Marketplace integration). In prod, missing either var is a **fail-closed throw** on the first call. In dev / test, the module warn-and-no-ops so local work isn't blocked. See `AGENTS.md` Gotcha #12 for the full env-var contract.
|
||||
|
||||
**429 response shape is uniform across all five classes.** Same error message (`'Too many attempts. Try again later.'`) and same `Retry-After` header calculation. Per-class variation would fingerprint the limits to an attacker.
|
||||
**429 response shape is uniform across all six classes.** Same error message (`'Too many attempts. Try again later.'`) and same `Retry-After` header calculation. Per-class variation would fingerprint the limits to an attacker.
|
||||
|
||||
**Fail-open on Upstash outage.** A network failure inside `ratelimit.limit(...)` returns `{ allowed: true, remaining: Infinity, reset: 0 }` with a single `console.error('[rate-limit]', err)`. Reasoning: a hard Upstash outage should not lock the entire user base out of every gated route. Brute-force / abuse protection lives behind defense-in-depth (Vercel firewall, future fail2ban-style lockout).
|
||||
|
||||
|
|
|
|||
53
.github/workflows/ci.yml
vendored
53
.github/workflows/ci.yml
vendored
|
|
@ -142,6 +142,59 @@ jobs:
|
|||
fi
|
||||
echo "OK: no Access-Control-Allow-* headers under pages/api/."
|
||||
|
||||
forbidden-client-side-llm-keys:
|
||||
name: No client-side LLM key leakage
|
||||
runs-on: ubuntu-latest
|
||||
steps:
|
||||
- uses: actions/checkout@v4
|
||||
- name: Fail on key-returning config endpoints or new browser LLM URLs
|
||||
run: |
|
||||
# Deleted by secure-scanner-gemini-key — must not return API keys to browsers.
|
||||
if [ -f pages/api/config/gemini.js ]; then
|
||||
echo "::error file=pages/api/config/gemini.js::Forbidden config endpoint — do not return API keys to browsers."
|
||||
exit 1
|
||||
fi
|
||||
CONFIG_MATCHES=$(grep -rEn 'apiKey:' pages/api/config/ 2>/dev/null || true)
|
||||
if [ -n "$CONFIG_MATCHES" ]; then
|
||||
echo "::error::Forbidden apiKey response under pages/api/config/."
|
||||
echo "$CONFIG_MATCHES" | while IFS= read -r line; do
|
||||
file=$(echo "$line" | cut -d: -f1)
|
||||
lineno=$(echo "$line" | cut -d: -f2)
|
||||
echo "::error file=${file},line=${lineno}::Do not return API keys from config endpoints."
|
||||
done
|
||||
exit 1
|
||||
fi
|
||||
# Grandfathered until server-side-scan-pipeline (#2) removes browser LLM clients.
|
||||
GRANDFATHER=(
|
||||
lib/ai-ocr.js
|
||||
components/OCRSettings.js
|
||||
)
|
||||
LLM_PATTERN='generativelanguage\.googleapis\.com|api\.openai\.com'
|
||||
FOUND=()
|
||||
while IFS= read -r file; do
|
||||
skip=false
|
||||
for gf in "${GRANDFATHER[@]}"; do
|
||||
if [ "$file" = "$gf" ]; then
|
||||
skip=true
|
||||
break
|
||||
fi
|
||||
done
|
||||
if [ "$skip" = true ]; then
|
||||
continue
|
||||
fi
|
||||
if grep -qE "$LLM_PATTERN" "$file" 2>/dev/null; then
|
||||
FOUND+=("$file")
|
||||
fi
|
||||
done < <(find components lib pages -name '*.js' ! -path 'pages/api/*' 2>/dev/null || true)
|
||||
if [ ${#FOUND[@]} -gt 0 ]; then
|
||||
echo "::error::Client-side LLM API URLs must not appear outside pages/api/ (except grandfathered files pending server-side-scan-pipeline)."
|
||||
for path in "${FOUND[@]}"; do
|
||||
echo "::error file=${path}::Move LLM calls server-side or add to server-side-scan-pipeline removal list."
|
||||
done
|
||||
exit 1
|
||||
fi
|
||||
echo "OK: no client-side LLM key leakage patterns detected."
|
||||
|
||||
test:
|
||||
name: Unit tests (vitest)
|
||||
runs-on: ubuntu-latest
|
||||
|
|
|
|||
|
|
@ -29,9 +29,8 @@ export default function CameraScanner({ onCardScanned, onError }) {
|
|||
// Mana symbol settings
|
||||
const [manaSymbolSettings, setManaSymbolSettings] = useState({ useSVG: false });
|
||||
|
||||
// Load OCR settings and auto-configure Gemini
|
||||
// Load OCR settings from localStorage only (never fetch server-side API keys)
|
||||
useEffect(() => {
|
||||
const loadOcrSettings = async () => {
|
||||
let settings = {
|
||||
service: 'gemini',
|
||||
openaiApiKey: '',
|
||||
|
|
@ -49,26 +48,8 @@ export default function CameraScanner({ onCardScanned, onError }) {
|
|||
}
|
||||
}
|
||||
|
||||
// Auto-load Gemini API key from environment
|
||||
if (!settings.geminiApiKey) {
|
||||
try {
|
||||
const response = await fetch('/api/config/gemini');
|
||||
if (response.ok) {
|
||||
const data = await response.json();
|
||||
if (data.hasKey && data.apiKey) {
|
||||
settings.geminiApiKey = data.apiKey;
|
||||
settings.service = 'gemini';
|
||||
console.log('✅ Auto-configured Gemini API key from environment');
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
console.log('Could not auto-load Gemini API key:', error);
|
||||
}
|
||||
}
|
||||
|
||||
setOcrSettings(settings);
|
||||
|
||||
// Configure AI services
|
||||
if (settings.openaiApiKey) {
|
||||
aiCardOCR.setApiKey(settings.openaiApiKey);
|
||||
}
|
||||
|
|
@ -78,9 +59,6 @@ export default function CameraScanner({ onCardScanned, onError }) {
|
|||
if (settings.ollamaUrl) {
|
||||
ollamaCardOCR.setBaseUrl(settings.ollamaUrl);
|
||||
}
|
||||
};
|
||||
|
||||
loadOcrSettings();
|
||||
}, []);
|
||||
|
||||
// Configure canvas contexts for optimal performance
|
||||
|
|
|
|||
|
|
@ -11,9 +11,8 @@ export default function OCRSettings({ isOpen, onClose }) {
|
|||
const [isTesting, setIsTesting] = useState(false);
|
||||
const [testResult, setTestResult] = useState(null);
|
||||
|
||||
// Load settings from localStorage on mount
|
||||
// Load settings from localStorage on mount (never fetch server-side API keys)
|
||||
useEffect(() => {
|
||||
const loadSettings = async () => {
|
||||
const savedSettings = localStorage.getItem('ocrSettings');
|
||||
let currentSettings = {
|
||||
service: 'gemini',
|
||||
|
|
@ -31,26 +30,7 @@ export default function OCRSettings({ isOpen, onClose }) {
|
|||
}
|
||||
}
|
||||
|
||||
// Try to auto-load Gemini API key from environment if not already set
|
||||
if (!currentSettings.geminiApiKey) {
|
||||
try {
|
||||
const response = await fetch('/api/config/gemini');
|
||||
if (response.ok) {
|
||||
const data = await response.json();
|
||||
if (data.hasKey && data.apiKey) {
|
||||
currentSettings.geminiApiKey = data.apiKey;
|
||||
currentSettings.service = 'gemini'; // Default to Gemini if key is available
|
||||
}
|
||||
}
|
||||
} catch (error) {
|
||||
console.log('Could not auto-load Gemini API key:', error);
|
||||
}
|
||||
}
|
||||
|
||||
setSettings(currentSettings);
|
||||
};
|
||||
|
||||
loadSettings();
|
||||
}, []);
|
||||
|
||||
const saveSettings = () => {
|
||||
|
|
|
|||
|
|
@ -12,6 +12,7 @@ const LIMITER_CONFIG = {
|
|||
upload: { limit: 10, window: '1 h', prefix: 'deckhearth:upload' },
|
||||
generate: { limit: 5, window: '1 h', prefix: 'deckhearth:generate' },
|
||||
import: { limit: 5, window: '1 h', prefix: 'deckhearth:import' },
|
||||
scan: { limit: 5, window: '1 m', prefix: 'deckhearth:scan' },
|
||||
};
|
||||
|
||||
// Lazy singleton. Module-load init would throw in environments without
|
||||
|
|
@ -129,3 +130,7 @@ export async function checkGenerateRateLimit(req, userId) {
|
|||
export async function checkImportRateLimit(req, userId) {
|
||||
return check('import', extractUserIdentifier(userId));
|
||||
}
|
||||
|
||||
export async function checkScanRateLimit(req, userId) {
|
||||
return check('scan', extractUserIdentifier(userId));
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,25 +0,0 @@
|
|||
export default async function handler(req, res) {
|
||||
if (req.method !== 'GET') {
|
||||
return res.status(405).json({ error: 'Method not allowed' });
|
||||
}
|
||||
|
||||
try {
|
||||
// Get Gemini API key from environment
|
||||
const geminiApiKey = process.env.GEMINI_AI_API_KEY;
|
||||
|
||||
if (geminiApiKey) {
|
||||
return res.status(200).json({
|
||||
hasKey: true,
|
||||
apiKey: geminiApiKey
|
||||
});
|
||||
} else {
|
||||
return res.status(200).json({
|
||||
hasKey: false,
|
||||
message: 'No Gemini API key found in environment'
|
||||
});
|
||||
}
|
||||
} catch (error) {
|
||||
console.error('Error getting Gemini config:', error);
|
||||
return res.status(500).json({ error: 'Internal server error' });
|
||||
}
|
||||
}
|
||||
Loading…
Reference in a new issue