diff --git a/CLAUDE.md b/CLAUDE.md index daced9b..1b1e959 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -60,6 +60,28 @@ For multi-step tasks, state a brief plan: Strong success criteria let you loop independently. Weak criteria ("make it work") require constant clarification. +## 5. Learn From Corrections + +**When corrected, capture the pattern. Don't repeat the mistake.** + +After any user correction or rewrite: +- Identify the rule that would have prevented it. +- Log it in `tasks/lessons.md` as a short, actionable rule. +- Review relevant lessons at the start of related tasks. + +The goal: mistake rate drops over time, not just within a session. + +## 6. Autonomous Execution + +**When the problem is clear, just solve it. Don't ask to be led.** + +If you have enough signal (error message, failing test, logs, reproduction steps): +- Fix it. Don't ask for step-by-step hand-holding. +- Point at the evidence, state what you're doing, then do it. +- Over-asking when the answer is visible wastes the user's time. + +The line: ask when genuinely ambiguous. Execute when it's not. + --- **These guidelines are working if:** fewer unnecessary changes in diffs, fewer rewrites due to overcomplication, and clarifying questions come before implementation rather than after mistakes. diff --git a/app/admin/activity-logs/page.tsx b/app/admin/activity-logs/page.tsx index e40e642..ec223f3 100644 --- a/app/admin/activity-logs/page.tsx +++ b/app/admin/activity-logs/page.tsx @@ -29,6 +29,14 @@ const ACTION_TYPES = [ 'poc_uploaded', 'report_viewed', 'report_decrypted', + 'comment_posted', + 'scope_created', + 'scope_updated', + 'scope_archived', + 'template_used', + 'role_changed', + 'user_suspended', + 'user_unsuspended', ]; const ACTION_ICONS: Record = { @@ -39,6 +47,14 @@ const ACTION_ICONS: Record = { poc_uploaded: '📎', report_viewed: '👁️', report_decrypted: '🔓', + comment_posted: '💬', + scope_created: '🎯', + scope_updated: '✏️', + scope_archived: '📦', + template_used: '📄', + role_changed: '🎭', + user_suspended: '🚫', + user_unsuspended: '✅', }; const ACTION_COLORS: Record = { @@ -49,12 +65,21 @@ const ACTION_COLORS: Record = { poc_uploaded: 'bg-indigo-100 text-indigo-700', report_viewed: 'bg-gray-100 text-gray-700', report_decrypted: 'bg-yellow-100 text-yellow-700', + comment_posted: 'bg-sky-100 text-sky-700', + scope_created: 'bg-teal-100 text-teal-700', + scope_updated: 'bg-amber-100 text-amber-700', + scope_archived: 'bg-stone-100 text-stone-600', + template_used: 'bg-cyan-100 text-cyan-700', + role_changed: 'bg-violet-100 text-violet-700', + user_suspended: 'bg-red-100 text-red-700', + user_unsuspended: 'bg-emerald-100 text-emerald-700', }; export default function ActivityLogsPage() { const [logs, setLogs] = useState([]); const [loading, setLoading] = useState(true); const [toast, setToast] = useState(null); + const [scoped, setScoped] = useState(false); // Filters const [actionFilter, setActionFilter] = useState(''); @@ -99,6 +124,7 @@ export default function ActivityLogsPage() { setLogs(data.logs || []); setTotalPages(data.pagination.totalPages); setTotal(data.pagination.total); + setScoped(data.scoped || false); } catch (error) { console.error('[fetchLogs]', error); setToast({ message: 'Failed to load activity logs', type: 'error' }); @@ -191,6 +217,14 @@ export default function ActivityLogsPage() { + {/* Scoped notice for triagers */} + {scoped && ( +
+ 🔍 + Showing your own actions only. Admins can view all platform activity. +
+ )} + {/* Stats */}
diff --git a/app/api/admin/activity-logs/export/route.ts b/app/api/admin/activity-logs/export/route.ts index d5c683e..aaad1a2 100644 --- a/app/api/admin/activity-logs/export/route.ts +++ b/app/api/admin/activity-logs/export/route.ts @@ -8,11 +8,11 @@ import { requireRole } from '@/lib/auth'; export async function GET(request: NextRequest) { try { - await requireRole('ADMIN'); + const { userId, role } = await requireRole('TRIAGER'); const sp = request.nextUrl.searchParams; const action = sp.get('action'); - const actorId = sp.get('actor_id'); + const actorId = role === 'ADMIN' ? sp.get('actor_id') : userId; const reportId = sp.get('report_id'); const startDate = sp.get('start_date'); const endDate = sp.get('end_date'); diff --git a/app/api/admin/activity-logs/route.ts b/app/api/admin/activity-logs/route.ts index b25c094..7d45ef1 100644 --- a/app/api/admin/activity-logs/route.ts +++ b/app/api/admin/activity-logs/route.ts @@ -10,11 +10,13 @@ import { getDisplayName } from '@/lib/redact'; export async function GET(request: NextRequest) { try { - await requireRole('ADMIN'); // Only admins can view activity logs + // TRIAGERs can view their own logs; ADMINs can view all logs + const { userId, role } = await requireRole('TRIAGER'); const sp = request.nextUrl.searchParams; const action = sp.get('action'); - const actorId = sp.get('actor_id'); + // TRIAGERs are automatically scoped to their own actor_id + const actorId = role === 'ADMIN' ? sp.get('actor_id') : userId; const reportId = sp.get('report_id'); const startDate = sp.get('start_date'); const endDate = sp.get('end_date'); @@ -100,6 +102,7 @@ export async function GET(request: NextRequest) { total, totalPages: Math.ceil(total / limit), }, + scoped: role !== 'ADMIN', }); } catch (err) { if (err instanceof Response) return err; diff --git a/app/api/admin/scopes/[id]/route.ts b/app/api/admin/scopes/[id]/route.ts index 024e084..9aca582 100644 --- a/app/api/admin/scopes/[id]/route.ts +++ b/app/api/admin/scopes/[id]/route.ts @@ -6,6 +6,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { getDb, getCfEnv } from '@/lib/db'; import { scopes } from '@/lib/db/schema'; import { requireRole } from '@/lib/auth'; +import { logAudit } from '@/lib/audit'; import { z } from 'zod'; import { eq, isNull, and } from 'drizzle-orm'; import { VULN_TYPES, SEVERITIES } from '@/lib/validation'; @@ -28,7 +29,7 @@ export async function PATCH( context: { params: Promise<{ id: string }> }, ) { try { - await requireRole('ADMIN'); + const { userId } = await requireRole('ADMIN'); const { id } = await context.params; let body: unknown; @@ -78,6 +79,16 @@ export async function PATCH( await db.update(scopes).set(updateData).where(eq(scopes.id, id)); const updated = await db.select().from(scopes).where(eq(scopes.id, id)).get(); + await logAudit({ + db, + entityType: 'system', + entityId: id, + actorId: userId, + action: data.restore ? 'scope_updated' : 'scope_updated', + oldValue: existing.domain, + newValue: data.domain ?? existing.domain, + }); + return NextResponse.json({ success: true, scope: updated }); } catch (err) { if (err instanceof Response) return err; @@ -92,7 +103,7 @@ export async function DELETE( context: { params: Promise<{ id: string }> }, ) { try { - await requireRole('ADMIN'); + const { userId } = await requireRole('ADMIN'); const { id } = await context.params; const db = getDb(getCfEnv().DB); @@ -108,6 +119,15 @@ export async function DELETE( .set({ deletedAt: Date.now(), updatedAt: Date.now() }) .where(eq(scopes.id, id)); + await logAudit({ + db, + entityType: 'system', + entityId: id, + actorId: userId, + action: 'scope_archived', + oldValue: existing.domain, + }); + return NextResponse.json({ success: true, message: 'Scope archived successfully' }); } catch (err) { if (err instanceof Response) return err; diff --git a/app/api/admin/scopes/route.ts b/app/api/admin/scopes/route.ts index 95af7ff..84b1da8 100644 --- a/app/api/admin/scopes/route.ts +++ b/app/api/admin/scopes/route.ts @@ -6,6 +6,7 @@ import { NextRequest, NextResponse } from 'next/server'; import { getDb, getCfEnv } from '@/lib/db'; import { scopes } from '@/lib/db/schema'; import { requireRole } from '@/lib/auth'; +import { logAudit } from '@/lib/audit'; import { z } from 'zod'; import { eq, isNull } from 'drizzle-orm'; import { VULN_TYPES, SEVERITIES } from '@/lib/validation'; @@ -87,6 +88,15 @@ export async function POST(request: NextRequest) { const newScope = await db.select().from(scopes).where(eq(scopes.id, id)).get(); + await logAudit({ + db, + entityType: 'system', + entityId: id, + actorId: userId, + action: 'scope_created', + newValue: data.domain, + }); + return NextResponse.json({ success: true, scope: newScope }, { status: 201 }); } catch (err) { if (err instanceof Response) return err; diff --git a/app/api/reports/[id]/comments/route.ts b/app/api/reports/[id]/comments/route.ts index 78561fe..9fefdbe 100644 --- a/app/api/reports/[id]/comments/route.ts +++ b/app/api/reports/[id]/comments/route.ts @@ -9,10 +9,12 @@ import { auth, clerkClient } from '@clerk/nextjs/server'; import { eq, desc } from 'drizzle-orm'; import { z } from 'zod'; import { getDisplayName } from '@/lib/redact'; +import { logAudit } from '@/lib/audit'; const CreateCommentSchema = z.object({ message: z.string().min(1).max(5000), isInternal: z.boolean().optional(), + templateId: z.string().min(1).max(100).optional(), }); export async function GET( @@ -75,7 +77,8 @@ export async function POST( } const { id: reportId } = await params; - const db = getDb(getCfEnv().DB); + const env = getCfEnv(); + const db = getDb(env.DB); // Verify user has access to this report const report = await db.select().from(reports).where(eq(reports.id, reportId)).get(); @@ -133,6 +136,32 @@ export async function POST( .where(eq(comments.id, commentId)) .get(); + if (data.templateId && isStaff) { + const template = await env.DB + .prepare('SELECT name FROM response_templates WHERE id = ? AND is_active = 1') + .bind(data.templateId) + .first<{ name?: string }>(); + + if (template) { + await logAudit({ + db, + reportId, + actorId: userId, + action: 'template_used', + oldValue: data.templateId, + newValue: template.name || data.templateId, + }); + } + } + + await logAudit({ + db, + reportId, + actorId: userId, + action: 'comment_posted', + newValue: isInternal ? 'internal' : 'public', + }); + return NextResponse.json({ success: true, comment: newComment }, { status: 201 }); } catch (err) { console.error('[POST /api/reports/[id]/comments]', err); diff --git a/app/triage/reports/[id]/page.tsx b/app/triage/reports/[id]/page.tsx index 6cb2d7f..bbcbc3d 100644 --- a/app/triage/reports/[id]/page.tsx +++ b/app/triage/reports/[id]/page.tsx @@ -288,6 +288,7 @@ export default function AdminReportDetail() { body: JSON.stringify({ message: newComment, isInternal: isInternalComment, + templateId: selectedTemplate || undefined, }), }); diff --git a/lib/audit.ts b/lib/audit.ts index 7818c29..99f052f 100644 --- a/lib/audit.ts +++ b/lib/audit.ts @@ -19,7 +19,12 @@ export type AuditAction = | 'report_decrypted' | 'role_changed' | 'user_suspended' - | 'user_unsuspended'; + | 'user_unsuspended' + | 'comment_posted' + | 'scope_created' + | 'scope_updated' + | 'scope_archived' + | 'template_used'; // Internal actions that should only be visible to triagers/admins const INTERNAL_ACTIONS: AuditAction[] = [ diff --git a/middleware.ts b/middleware.ts index 4a14fd8..a1a97db 100644 --- a/middleware.ts +++ b/middleware.ts @@ -7,6 +7,7 @@ import { NextResponse } from 'next/server'; const isTriageRoute = createRouteMatcher(['/triage(.*)']); const isAdminPageRoute = createRouteMatcher(['/admin(.*)']); // Page routes only, not API +const isActivityLogsPageRoute = createRouteMatcher(['/admin/activity-logs(.*)']); const isDashboardRoute = createRouteMatcher(['/dashboard(.*)']); const isSubmitRoute = createRouteMatcher(['/submit(.*)']); @@ -25,12 +26,13 @@ export default clerkMiddleware(async (auth, request) => { } } if (isAdminPageRoute(request) && !isApiRoute) { - // Admin page route - ADMIN only + // Admin page route - ADMIN only, except triagers can view their own activity logs. const { userId } = await auth.protect(); const client = await clerkClient(); const user = await client.users.getUser(userId); const role = (user.publicMetadata as { role?: string })?.role; - if (role !== 'ADMIN') { + const canViewScopedActivityLogs = isActivityLogsPageRoute(request) && role === 'TRIAGER'; + if (role !== 'ADMIN' && !canViewScopedActivityLogs) { return NextResponse.redirect(new URL('/', request.url)); } } diff --git a/playwright-report/index.html b/playwright-report/index.html new file mode 100644 index 0000000..226fb08 --- /dev/null +++ b/playwright-report/index.html @@ -0,0 +1,90 @@ + + + + + + + + + Playwright Test Report + + + + +
+ + + \ No newline at end of file diff --git a/tasks/lessons.md b/tasks/lessons.md new file mode 100644 index 0000000..0d83fb3 --- /dev/null +++ b/tasks/lessons.md @@ -0,0 +1,197 @@ +# Lessons Learned — Vanguard VDP + +Patterns extracted from past mistakes, corrections, and bug fixes. +Add a new entry whenever you correct a mistake or get pushback. +Review the relevant section before starting a task in that domain. + +--- + +## Cloudflare / Edge Runtime + +**L01 — Never use `Zod.default()` in edge runtime** +`TypeError: Cannot read properties of undefined (reading 'default')` at runtime. +Use `.optional()` instead and handle defaults in application code: +```typescript +// ❌ z.coerce.number().default(1) +// ✅ z.coerce.number().optional() → const { page = 1 } = parsed.data +``` +*Source: ISSUES.md §1, CHANGELOG.md v2.1.0* + +**L02 — Never declare `export const runtime = 'edge'` in API routes** +OpenNext/Cloudflare already sets the runtime. Adding the declaration causes conflicts. +*Source: ISSUES.md §2, AGENTS.md, docs/blueprint.md* + +**L03 — D1 binding requires `npm run dev:cf`, not `npm run dev`** +`npm run dev` is a plain Next.js server with no Cloudflare bindings. +Any code that calls `getCfEnv()` or accesses D1 will throw at runtime. +`npm run dev:cf` wraps the server in Miniflare, which injects all CF bindings. +*Source: AGENTS.md, docs/agent.md* + +**L04 — `wrangler pages dev` has no `--remote` flag** +You cannot point local dev at production D1. To read/write remote D1 you must: +- Read: `npx wrangler d1 execute vanguard-security --remote --command="..."` +- Write: `npm run deploy` +*Source: docs/agent.md* + +**L05 — `serverExternalPackages` is mandatory in `next.config.ts`** +Without it, Webpack/Turbopack tries to bundle CF-native packages and the build fails +with cryptic module resolution errors: +```typescript +serverExternalPackages: ['wrangler', 'miniflare', '@cloudflare/next-on-pages', 'workerd'] +``` +*Source: docs/agent.md* + +**L06 — esbuild cannot parse em-dashes (`—`) in route files** +An em-dash in a comment or string produces `Unexpected character '—'` at build time. +Replace all `—` with `--` in any file under `app/api/`. +Run `grep -r "—" app/` before building if unsure. +*Source: docs/agent.md* + +--- + +## Next.js 15+ / 16 Specifics + +**L07 — Dynamic route params are a `Promise` in Next.js 15+** +```typescript +// ❌ Wrong — was valid in Next.js 14 +export async function GET(req: Request, { params }: { params: { id: string } }) { + +// ✅ Correct +export async function GET(req: Request, context: { params: Promise<{ id: string }> }) { + const { id } = await context.params +``` +*Source: docs/agent.md, CHANGELOG.md v2.3.0* + +**L08 — No top-level await in `next.config.ts`** +It's a CJS file evaluated at build time. Async setup belongs in `instrumentation.ts` +and must be gated with `process.env.NEXT_RUNTIME === 'nodejs'` to prevent Turbopack +from bundling wrangler native binaries into the edge bundle. +*Source: docs/agent.md* + +--- + +## Auth & Middleware + +**L09 — API routes must not be blocked by middleware role checks** +Middleware should only guard page routes. API routes authenticate themselves via `requireRole()`. +```typescript +// ❌ const isAdminRoute = createRouteMatcher(['/admin(.*)', '/api/admin(.*)']) +// ✅ const isAdminPageRoute = createRouteMatcher(['/admin(.*)']) +// if (isAdminPageRoute(req) && !req.nextUrl.pathname.startsWith('/api/')) { ... } +``` +*Source: ISSUES.md §3, CHANGELOG.md v2.1.0* + +**L10 — Report owners need to decrypt their own data, not just staff** +When the single-report API was first written, only TRIAGER/ADMIN could decrypt the body. +This caused researchers to get "Report not found" (effectively a 403) on their own reports. +The API must check `clerkUserId === report.clerk_user_id` as a second gate. +*Source: CHANGELOG.md v2.3.0* + +--- + +## Database & Migrations + +**L11 — Follow `docs/MIGRATION_ORDER.md` for migration order; don't rely on filenames** +Migration filenames were not consistently zero-padded in early history. +The canonical order is in `docs/MIGRATION_ORDER.md`. New migrations start at `0013_`. +*Source: docs/MIGRATION_ORDER.md* + +**L12 — Code must never deploy before its migration runs** +Schema and code are coupled. Always run migrations on remote D1 before pushing code +that depends on the new columns or tables. +```bash +npx wrangler d1 execute vanguard-security --remote --file=migrations/XXXX.sql +``` +*Source: ISSUES.md §DB, AGENTS.md* + +**L13 — Don't store user names in the database; fetch from Clerk dynamically** +Stored names go stale when users update their profile in Clerk. +Use `clerk_user_id` as the key and fetch the display name at query time via the Clerk API. +Display name priority: alias → first+last → first → username → email prefix → `'Anonymous'`. +*Source: docs/SCHEMA_REFACTOR.md* + +**L14 — Don't commit one-off migration files that contain user-specific data** +Email-to-user-ID conversion scripts are one-time operations; they contain PII and can't +be replayed. Create the file locally, execute it manually, then discard — do not `git add`. +*Source: docs/agent.md, AGENTS.md* + +--- + +## API Design + +**L15 — Mirror Zod schemas exactly between client and server** +If the server enforces `min(10)` but the client only checks "required", the form submits, +hits the server, returns 422, and the client shows success because it didn't check `res.ok`. +Always copy the exact min/max/enum constraints to the client-side `validate()` function. +*Source: docs/agent.md, CHANGELOG.md v2.3.0* + +**L16 — Always check `res.ok` before reading the success payload** +```typescript +const res = await fetch('/api/reports', { method: 'POST', body: ... }) +if (!res.ok) { /* handle error */ return } +const data = await res.json() // only safe here +``` +*Source: docs/agent.md* + +**L17 — Verify the exact API response shape before wiring up the frontend** +`data.report` vs `data.data` vs plain array — a mismatch silently produces `undefined` +and shows an empty or broken UI. Check the route handler's `return Response.json(...)` call +before writing the consumer. +*Source: CHANGELOG.md v2.3.0* + +--- + +## Security + +**L18 — Never log PII to the console** +Email addresses, names, and report bodies must not appear in logs. +Log only non-sensitive identifiers: reference ID, severity, role, status. +```typescript +// ❌ console.log(`Email: ${body.email}`) +// ✅ console.log(`[REPORT] ${referenceId} | severity: ${body.severity}`) +``` +*Source: CHANGELOG.md v2.0.0* + +**L19 — Gate debug logs with `NODE_ENV === 'development'`** +Verbose auth/session logs running on every request pollute production logs and can +expose session token fragments. +```typescript +if (process.env.NODE_ENV === 'development') { + console.log('[getSessionRole] role:', role) +} +``` +*Source: CHANGELOG.md v2.0.0* + +**L20 — Sanitize LIKE query parameters to prevent SQL injection** +Drizzle's `like()` helper does not escape `%` and `_` in user input. +```typescript +const sanitized = q.replace(/[%_\\]/g, '\\$&') +conditions.push(like(reports.title, `%${sanitized}%`)) +``` +*Source: CHANGELOG.md v2.0.0* + +--- + +## Route Changes + +**L21 — When renaming a route, search the whole codebase for old references** +Navigation components, breadcrumbs, back buttons, Discord webhook URLs, and API response +links can all silently break. +```bash +grep -r "/old-route" app/ lib/ --include="*.ts" --include="*.tsx" +``` +*Source: ISSUES.md §Route Restructuring* + +--- + +## Development Workflow + +**L22 — `npm run lint` has pre-existing failures; report them, don't treat them as blockers** +Lint failures that existed before your change are noise. Call them out explicitly +so the reviewer knows they are pre-existing. +*Source: AGENTS.md* + +**L23 — `.npmrc` must have `legacy-peer-deps=true`** +`@cloudflare/next-on-pages` runs its own `npm install` internally during build. +Without `legacy-peer-deps=true`, peer dependency conflicts fail the CF build for `next@16.x`. +*Source: docs/agent.md* diff --git a/test-results/.last-run.json b/test-results/.last-run.json new file mode 100644 index 0000000..cbcc1fb --- /dev/null +++ b/test-results/.last-run.json @@ -0,0 +1,4 @@ +{ + "status": "passed", + "failedTests": [] +} \ No newline at end of file