← back to Grant

REVIEW-2026-05-04.md

160 lines

# Grant — Tick 14 Review (extension to overnight rotation)

**Date:** 2026-05-04
**Reviewers:** code-reviewer (completed) + architect-reviewer (rate-limited by Anthropic mid-flight)
**Constraints:** local-only safe patches; NO pm2 restart, NO deploy, NO new deps, NO schema changes.

## Headline

Grant is the **fundraiser-side companion to Freddy** — both Next.js 16 + TS + PG, both wave-3 `@dw/nextjs-admin-login` consumers. Code-reviewer surfaced 4 P0s (timing-attack login, hardcoded creds in CLAUDE.md, two cross-tenant IDORs on PATCH/proposals), 5 P1s (rate-limit absent on AI routes, prompt-injection surface, stored-XSS surface, Gemini fetch timeouts, Secure cookie flag), and 5 P2s. **9 patches landed local; 4 P1/P2 deferred (need new deps or schema migration).**

## Findings + dispositions

| # | Severity | Finding | Status |
|---|---|---|---|
| P0-1 | Critical | `app/api/auth/login/route.ts:23` uses `!==` for both username and password — timing-attack enumerable | **Patched** — `timingSafeEqual` with equal-length-pad fallback |
| P0-2 | Critical | `CLAUDE.md:6` ships hardcoded `postgresql://dw_admin:DW2024SecurePass@...` cleartext | **Patched** — replaced with `.env.local` placeholder. **Steve must rotate `DW2024SecurePass` if this repo was ever pushed.** |
| P0-3 | Critical | PATCH `/api/grants` and PATCH `/api/collaborations` UPDATE without `AND org_id = $N` — cross-tenant write IDOR | **Patched** — both UPDATEs scoped via `LIMIT 1` org lookup + scoped WHERE |
| P0-4 | Critical | GET `/api/grants/[id]/proposals` reads any org's proposals — cross-tenant read IDOR (subject, body, recipient_email) | **Patched** — JOIN through grants on `g.org_id = $2` |
| P1-1 | High | No rate-limit on 5 AI routes; single session can burn unlimited Gemini credits | Deferred — needs design decision (in-memory Map vs PG `ai_call_log` table) |
| P1-2 | High | `context`, `target_name`, `target_org` interpolated into prompts unbounded — prompt-injection + multi-MB DoS | **Patched** — capped at 2000 chars (context) / 500 chars (names) in proposals + outreach |
| P1-3 | High | `body_html` accepted from client and stored verbatim — stored-XSS time-bomb | **Partial** — 200KB size cap + type guard. Sanitizer (sanitize-html) deferred (new dep). |
| P1-4 | High | Five Gemini `fetch()` calls with no timeout — hung upstream exhausts pg pool max=10 | **Patched (5/5)** — `AbortSignal.timeout(25_000)` on all five, return 504 on abort |
| P1-5 | High | `Secure` cookie flag only when NODE_ENV=production — Kamatera dev-mode leaks cookie over plaintext | Deferred — fix lives in `@dw/nextjs-admin-login` shim, not in Grant |
| P2-1 | Med | `SELECT id FROM organizations LIMIT 1` is a multi-tenancy time-bomb (~10 sites) | Out of scope — single-org for now; flagged in summary |
| P2-2 | Med | News-discover prompt fabricates "plausible URL at that outlet" — credibility/liability risk | Deferred — needs schema column + UI label |
| P2-3 | Med | AI-call routes (4) not audited; no IP/correlation trail if Gemini key abused | Deferred — touches `lib/audit.ts` + every AI route |
| P2-4 | Med | `grants POST` accepts undefined `title` → opaque 500 | **Patched** — explicit type + trim guard, returns 400 |
| P2-5 | Low | Logout cookie missing `Secure` flag | Deferred — same `@dw/nextjs-admin-login` shim location as P1-5 |
| Refactor | — | 5× duplication of `callGemini` pattern (key + URL + fetch + parse + strip-fence) — extract to `lib/gemini.ts` | Deferred — natural home for P1-1 rate-limit + P1-4 timeout consolidation |

## Patches landed (9 edits across 6 files)

1. `app/api/auth/login/route.ts` — `timingSafeEqual`-based `safeCompare()` + use for both username and password (P0-1)
2. `app/api/grants/route.ts` POST — explicit `title` guard returns 400 (P2-4)
3. `app/api/grants/route.ts` PATCH — scoped UPDATE with `AND org_id = $N` (P0-3)
4. `app/api/collaborations/route.ts` PATCH — scoped UPDATE with `AND org_id = $N` (P0-3)
5. `app/api/grants/[id]/proposals/route.ts` GET — JOIN through grants for tenant isolation (P0-4)
6. `app/api/grants/[id]/proposals/route.ts` POST — context length cap (P1-2), body_html size cap + type guard (P1-3), Gemini AbortSignal 25s (P1-4)
7. `app/api/grants/discover/route.ts` POST — Gemini AbortSignal 25s (P1-4)
8. `app/api/collaborations/discover/route.ts` POST — Gemini AbortSignal 25s (P1-4)
9. `app/api/news/discover/route.ts` POST — Gemini AbortSignal 25s (P1-4)
10. `app/api/outreach/generate/route.ts` POST — input field caps (P1-2), Gemini AbortSignal 25s (P1-4)
11. `CLAUDE.md` — DB cleartext credential stripped (P0-2)

`./node_modules/.bin/tsc --noEmit` clean across all 6 patched .ts files.

## Production-touching items pending Steve

- **Rotate `DW2024SecurePass`** — was committed in `CLAUDE.md`. If this repo ever lived on a remote (GitHub/GitLab/Kamatera bare), the password is compromised. Mint a new value via `/secrets`, route to Grant + every other service that uses `dw_admin@127.0.0.1`.
- **Deploy patches** — 9 patches sit local; no Mac2 pm2 process is running grant-app right now (`pm2 jlist | grep grant` returned nothing), so this is "next time you start grant-app, the patched code goes live."
- **Address P1-1** — pick rate-limit approach (in-memory Map for v0, PG `ai_call_log` table for production).
- **Add `sanitize-html` dep** — close P1-3 properly, not just the size cap.
- **Patch the shim** — P1-5 + P2-5 fix lives in `@dw/nextjs-admin-login` source, not in Grant. The shim is loaded from a tarball at `/tmp/dw-nextjs-admin-login-0.1.0.tgz`; its actual source is at `~/Projects/dw-nextjs-admin-login/` (or wherever Steve maintains it).

## Architect-reviewer note

The architect agent hit Anthropic's "Server is temporarily limiting requests (not your usage limit)" rate-limit and bailed at 672ms with no findings. The launchd watcher (`~/Library/LaunchAgents/com.steve.yolo-rate-limit-watcher.plist`) caught it. Architect re-run can be queued in a follow-up tick if Steve wants the place-graph-style cross-project verdict against Freddy/Patty/PoppyPetitions.

## Tick summary

Tick 14 (extension to the FINAL 13-tick rotation). Code-reviewer landed clean, architect-reviewer rate-limited mid-flight. Grant — Steve's AI-powered fundraising platform for non-profits, sibling to Freddy — got 9 local-only safe patches across 6 files closing 4 P0 (timing-attack login, cleartext PG password in docs, two cross-tenant IDORs) + 4 P1 (input length caps, body_html size guard, 5 Gemini upstream timeouts) + 1 P2 (title null guard). All TypeScript clean. Patches NOT deployed; no pm2 process running grant-app locally. **The big DW2024SecurePass rotation is the most urgent Steve-side item** — that password was visible cleartext in `CLAUDE.md` until this tick.

---

# Tick 15 — architect retry + sibling P0 leak + lib/gemini.ts extraction

**Reviewers:** architect-reviewer (retry succeeded after Anthropic rate-limit)
**Constraints:** local-only safe patches; NO pm2 restart, NO deploy.

## Architect verdict (one-liner)

Architecture is sound (clean App Router + thin lib/) but ships with **5x duplication of the Gemini call pattern** and a **placeholder org-resolution baked into every handler**. Biggest issue: the `SELECT id FROM organizations LIMIT 1` placeholder leaks into every route. Biggest opportunity: extract `lib/gemini.ts` (PoppyPetitions already has a working reference impl).

## Cross-project P0 (NEW, found by architect)

**P0-CROSS: Hardcoded Gemini API key in `~/Projects/PoppyPetitions/lib/gemini.ts:1`** — cleartext value `AIzaSy...` (47-char Google API key prefix) committed in plaintext.
- **Patched** — replaced with `process.env.GEMINI_API_KEY`, fail-fast module-load check, AbortSignal 25s timeout added to match Grant pattern.
- **Steve-side rotation REQUIRED** — the literal value is compromised; do not assume git history is clean. Mint new key at https://aistudio.google.com/apikey, route via `/secrets` to all consumers (PoppyPetitions, Grant, future siblings).

## Tick-15 patches landed (3 edits)

1. `~/Projects/PoppyPetitions/lib/gemini.ts` — scrubbed hardcoded key, added env-var fail-fast + AbortSignal 25s
2. `~/Projects/Grant/lib/gemini.ts` — NEW. `callGemini<T>()` helper per architect spec: Result-shaped (`{ok, data, raw, inputTokens, outputTokens}` | `{ok:false, reason, status, detail}`). Centralizes key/URL/timeout/fence-strip/parse. Reasons: `no_key`/`timeout`/`upstream`/`parse`. No Zod (architect: "schema validation belongs at the call site").
3. `~/Projects/Grant/app/api/news/discover/route.ts` — migrated to `callGemini<NewsItem[]>({prompt, maxTokens: 8192})`. Removed ~30 LOC (boilerplate fetch + try/catch + parse + fence-strip). 1/5 callsites done.

`tsc --noEmit` clean across all 3.

## Pending Gemini-callsite migrations (4/5 remaining)

Recipe — migrate each in turn:
```ts
// BEFORE (per-route boilerplate, ~30 LOC):
//   const GEMINI_KEY = process.env.GEMINI_API_KEY; ...
//   try { fetch(GEMINI_URL, { ...signal: AbortSignal.timeout(25_000) }) } catch { 504 }
//   if (!geminiRes.ok) { 502 }
//   text = data.candidates?.[0]... ; text.replace(/```json\s*/, '')...
//   try { JSON.parse } catch { 500 }

// AFTER (~10 LOC):
import { callGemini } from '@/lib/gemini';
const result = await callGemini<ParsedShape>({ prompt, maxTokens: 8192 });
if (!result.ok) {
  return NextResponse.json(
    { error: result.reason === 'no_key' ? 'AI service not configured' : 'AI service error' },
    { status: result.status },
  );
}
const items = result.data;
```

Files to migrate:
- `app/api/grants/discover/route.ts` (maxTokens=8192)
- `app/api/collaborations/discover/route.ts` (maxTokens=8192)
- `app/api/outreach/generate/route.ts` (maxTokens=4096; parsed shape: `{subject, body_html, body_text}`)
- `app/api/grants/[id]/proposals/route.ts` (maxTokens=4096; parsed shape: `{subject, recipient_name, body_html, body_text}`)

## Architect's other actionable findings (deferred — Steve-level)

1. **Multi-tenancy**: extend `@dw/nextjs-admin-login` cookie payload with `org_id`, change `verifyAuth` return shape from `User | null` to `{user, orgId} | null`. Then a single search-and-replace eliminates the 7+ `LIMIT 1` placeholder lookups. Patty already has `lib/sister-auth.ts` as precedent for shim extension.
2. **Middleware.ts**: Grant has none. Recommended scope (priority order): auth gate for `/api/**` except `/api/auth/*` (collapses 14 copies of `verifyAuth(request); if (!user) return 401`), `x-request-id` injection, sliding-window rate-limit on the 5 Gemini routes.
3. **Don't share `lib/db.ts`** (project-config, not framework code). **Don't share `lib/audit.ts`** (35 lines, no shared migration story yet — Grant's evolved version adds `org_id`+`user_id` columns vs Freddy's 5-col schema). **Do share `lib/gemini.ts`** as `@dw/gemini-flash` once it's been battle-tested in Grant.

## Tick-15 cumulative (ticks 14+15 against Grant + 1 sibling P0)

- 12 patches across 7 files (Grant: 6 files, 11 patches; PoppyPetitions: 1 file, 1 patch)
- New file: `Grant/lib/gemini.ts` (~85 LOC) — single source of truth for Gemini calls
- Cross-project P0 closed: PoppyPetitions hardcoded API key scrubbed
- All TypeScript clean
- **Two Steve-side rotations now required**: `DW2024SecurePass` (PG, leaked in CLAUDE.md), `AIzaSyB...` Gemini key (leaked in PoppyPetitions/lib/gemini.ts)
- 4 of 5 Gemini callsites still on the inline pattern; migration recipe documented above

**Tick 15 summary paragraph:** Architect-reviewer's retry landed cleanly and surfaced a real P0 across a sibling project (PoppyPetitions had a hardcoded Google API key). Scrubbed the key, made env-var-only, added the 25s timeout pattern. Then built the architect-spec'd `lib/gemini.ts` wrapper for Grant — Result-shaped, generic-typed, no Zod. Migrated one callsite (news/discover) end-to-end as proof; 4 remain on the inline pattern with a documented mechanical recipe. **Two Steve rotations still required**: PG password + leaked Gemini key. Both are pre-existing leaks discovered tonight, not regressions introduced by the loop.

---

# DECISION TO FINALIZE — what's the right next move on Grant in a single ~30-60 min focused session?

**Steve's framing:** "use dream team to finalize" — debate the four options below, rank, pick one, defend it.

## Options on the table

**A) Mechanical sweep** — write `middleware.ts` (auth gate for `/api/**` except `/api/auth/*`, `x-request-id` injection, sliding-window rate-limit on the 5 Gemini routes) + migrate the 4 remaining Gemini callsites to `callGemini<T>()`. Low-risk, ~5 files, completes the architect's recommendation. Cost: doesn't unlock new behavior, just closes hygiene debt and exercises the new wrapper.

**B) Multi-tenancy fix** — extend `@dw/nextjs-admin-login` cookie payload with `org_id`, change `verifyAuth` return shape from `User|null` to `{user, orgId}|null`. Then a single search-and-replace eliminates the 7+ `SELECT id FROM organizations LIMIT 1` placeholders. Patty already has `lib/sister-auth.ts` as precedent for shim extension. Cross-project (touches Grant + Freddy + Patty + PoppyPetitions). Higher leverage, higher blast radius.

**C) Real product feature** — replace the AI-fabricated `*/discover` routes with actual integrations. Grants.gov public API (free, no auth) + Federal Register API + real news via NewsAPI. Current routes ask Gemini to invent "plausible URL at that outlet" which ships fake data to users (P2-2 from code review). Visible product progress, but biggest scope.

**D) Something else** — consolidate the 5 `lib/audit.ts` variants across the 4 wave-3 projects; or build the `ai_call_log` rate-limit table now since P1-1 is the highest-cost open issue; or address the two Steve-side key rotations as actual code changes (move key validation into `lib/secrets.ts` that fails fast on missing env, etc.).

## Constraints

- Single-tenant for now (one org row in PG); Steve hasn't said multi-tenant is imminent.
- pm2 grant-app is NOT running locally → patches can land any time without restart drama.
- No Kamatera deploys, no DNS, no Shopify writes, no domain registration.
- Wave-3 shim source is at `~/Projects/dw-nextjs-admin-login/`; tarball at `/tmp/dw-nextjs-admin-login-0.1.0.tgz` is what gets installed into projects.

## Question for the dream team

Pick ONE option. Defend it. Steve wants finalization, not "well, it depends." The reviewer should also flag any option I haven't listed.