Reconcile PR #33 + #34 audit remediation (fixes broken checkout) + post-review hardening

.github/workflows/ci.yml

@@ -125,6 +125,10 @@ jobs:
     env:
       DATABASE_URL: postgresql://test:test@localhost:5432/amph_v2_e2e
       JWT_SECRET: test-secret-min-32-characters-long-aaaaaaaa
+      # The seed fails loudly if these are unset (C5 hardening). CI-only test
+      # credentials, never used outside this ephemeral Postgres.
+      ADMIN_EMAIL: admin@ci.local
+      ADMIN_PASSWORD: ci-test-password-not-secret
 
     steps:
       - uses: actions/checkout@v7
@@ -183,6 +187,10 @@ jobs:
     env:
       DATABASE_URL: postgresql://test:test@localhost:5432/amph_v2_lhci
       JWT_SECRET: test-secret-min-32-characters-long-aaaaaaaa
+      # The seed fails loudly if these are unset (C5 hardening). CI-only test
+      # credentials, never used outside this ephemeral Postgres.
+      ADMIN_EMAIL: admin@ci.local
+      ADMIN_PASSWORD: ci-test-password-not-secret
 
     steps:
       - uses: actions/checkout@v7

prisma/migrations/20260718000000_audit_integrity_fixes/migration.sql

@@ -0,0 +1,107 @@
+-- Audit remediation (2026-07-17): integrity + security fixes on main.
+-- Covers: XP ledger (C10), guest-claim token (C5/C6), unique refund id (C8),
+-- one-active-refund-per-payment (C7), one-active-certificate-per-course (H7),
+-- and canonical (lowercase) email backfill (H6).
+--
+-- This is a single hand-assembled migration reconciling PR #33's
+-- 20260717000001_claim_token and PR #34's 20260717000000_audit_integrity_fixes
+-- migrations, which both independently added `User.claimTokenHash` /
+-- `User.claimTokenExpiresAt` and would collide (duplicate-column error) if
+-- stacked directly on top of one another.
+
+-- ----------------------------------------------------------------------------
+-- C5/C6: guest-checkout account-claim token columns on User.
+-- ----------------------------------------------------------------------------
+ALTER TABLE "User" ADD COLUMN "claimTokenHash" TEXT;
+ALTER TABLE "User" ADD COLUMN "claimTokenExpiresAt" TIMESTAMP(3);
+
+CREATE INDEX "User_claimTokenHash_idx" ON "User"("claimTokenHash");
+
+-- ----------------------------------------------------------------------------
+-- C10: XP ledger. Unique (userId, eventKey) makes every award exactly-once.
+-- ----------------------------------------------------------------------------
+CREATE TABLE "XpLedger" (
+    "id" TEXT NOT NULL,
+    "userId" TEXT NOT NULL,
+    "eventKey" TEXT NOT NULL,
+    "amount" INTEGER NOT NULL,
+    "reason" TEXT NOT NULL,
+    "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
+
+    CONSTRAINT "XpLedger_pkey" PRIMARY KEY ("id")
+);
+
+CREATE UNIQUE INDEX "XpLedger_userId_eventKey_key" ON "XpLedger" ("userId", "eventKey");
+CREATE INDEX "XpLedger_userId_idx" ON "XpLedger" ("userId");
+CREATE INDEX "XpLedger_createdAt_idx" ON "XpLedger" ("createdAt");
+
+ALTER TABLE "XpLedger" ADD CONSTRAINT "XpLedger_userId_fkey"
+    FOREIGN KEY ("userId") REFERENCES "User" ("id") ON DELETE CASCADE ON UPDATE CASCADE;
+
+-- ----------------------------------------------------------------------------
+-- C8: a PayMongo refund id may be recorded at most once.
+-- ----------------------------------------------------------------------------
+CREATE UNIQUE INDEX "RefundRequest_paymongoRefundId_key"
+    ON "RefundRequest" ("paymongoRefundId");
+
+-- ----------------------------------------------------------------------------
+-- C7: at most one active (PENDING or APPROVED) refund request per payment.
+-- Partial unique index — cannot be expressed in schema.prisma, so it is
+-- managed by raw SQL here. Concurrent duplicate requests now fail with P2002.
+-- ----------------------------------------------------------------------------
+CREATE UNIQUE INDEX "RefundRequest_active_per_payment_key"
+    ON "RefundRequest" ("paymentId")
+    WHERE "deletedAt" IS NULL AND "status" IN ('PENDING', 'APPROVED');
+
+-- ----------------------------------------------------------------------------
+-- H7: at most one active certificate per (user, course).
+-- ----------------------------------------------------------------------------
+CREATE UNIQUE INDEX "Certificate_active_per_user_course_key"
+    ON "Certificate" ("userId", "courseId")
+    WHERE "deletedAt" IS NULL AND "status" = 'ACTIVE';
+
+-- ----------------------------------------------------------------------------
+-- H6: backfill existing emails to canonical (trimmed, lowercase) form.
+-- Rows whose canonical form would collide with another account are left
+-- untouched, and need manual reconciliation before a case-insensitive
+-- uniqueness constraint (citext / normalized column) can be enforced.
+--
+-- The collision guard compares canonical-to-CANONICAL, not canonical-to-exact.
+-- Two rows that are both non-canonical but share a canonical form (e.g.
+-- 'Foo@Bar.com' and 'FOO@bar.com') would otherwise BOTH satisfy an exact-match
+-- guard and both UPDATE to the same value in one statement (WHERE is evaluated
+-- against the MVCC snapshot), violating the case-sensitive unique email index
+-- and failing the migration. Canonical-to-canonical skips every member of a
+-- colliding group instead.
+-- ----------------------------------------------------------------------------
+UPDATE "User" u
+SET email = lower(btrim(u.email))
+WHERE u.email <> lower(btrim(u.email))
+  AND NOT EXISTS (
+    SELECT 1 FROM "User" o
+    WHERE o.id <> u.id
+      AND lower(btrim(o.email)) = lower(btrim(u.email))
+  );
+
+-- Surface the rows we deliberately skipped (their canonical form collides with
+-- another account) so they don't vanish silently: canonicalized lookups
+-- (findOrCreateUserByEmail, signUpAction) query by lowercase and can't find a
+-- still-mixed-case row. These need manual reconciliation. Emitted as a WARNING
+-- so it shows in migration/deploy logs; run scripts/report-email-collisions.ts
+-- afterwards to list them again on demand.
+DO $$
+DECLARE
+  skipped_count INTEGER;
+BEGIN
+  SELECT count(*) INTO skipped_count
+  FROM "User" u
+  WHERE u.email <> lower(btrim(u.email))
+    AND EXISTS (
+      SELECT 1 FROM "User" o
+      WHERE o.id <> u.id
+        AND lower(btrim(o.email)) = lower(btrim(u.email))
+    );
+  IF skipped_count > 0 THEN
+    RAISE WARNING 'H6 email canonicalization: % account(s) left un-normalized due to a case-insensitive collision. Run scripts/report-email-collisions.ts and reconcile manually.', skipped_count;
+  END IF;
+END $$;

prisma/schema.prisma

@@ -26,6 +26,12 @@ model User {
   image         String?
   passwordHash  String? // null for OAuth-only
 
+  // Guest-checkout account claiming. A guest purchase creates a placeholder
+  // user; the buyer claims it by presenting the raw claim token (only its
+  // hash is stored). Consumed atomically at signup, then both fields null.
+  claimTokenHash      String?
+  claimTokenExpiresAt DateTime?
+
   // Gamification
   role         String   @default("STUDENT")
   xp           Int      @default(0)
@@ -59,6 +65,7 @@ model User {
   reviewedRefunds      RefundRequest[]         @relation("RefundReviewer")
   payments             Payment[]
   checkoutSessions     CheckoutSession[]
+  xpLedger             XpLedger[]
   authoredCourses      Course[]                @relation("CourseAuthor")
   authoredLessons      Lesson[]                @relation("LessonAuthor")
   authoredBadges       Badge[]                 @relation("BadgeAuthor")
@@ -71,6 +78,7 @@ model User {
   @@index([status])
   @@index([role])
   @@index([lastActiveAt])
+  @@index([claimTokenHash])
 }
 
 model Account {
@@ -736,7 +744,7 @@ model RefundRequest {
   reviewedById     String?
   reviewedAt       DateTime?
   reviewerNotes    String?
-  paymongoRefundId String?
+  paymongoRefundId String?   @unique
   processedAt      DateTime?
   failedAt         DateTime?
   failureReason    String?
@@ -777,6 +785,33 @@ model ProcessedWebhook {
   @@index([processedAt])
 }
 
+// ============================================================================
+// XP LEDGER (idempotent gamification awards)
+// ============================================================================
+
+// Every XP award is recorded here with a unique (userId, eventKey). The
+// User.xp increment and the ledger insert happen in ONE transaction: the
+// insert's unique constraint is what makes an award exactly-once under
+// concurrency. A duplicate completion (two racing requests, a webhook replay)
+// hits P2002 on insert, the transaction rolls back, and no XP is granted twice.
+//
+// eventKey convention: "<event>:<scope-id>" — e.g.
+//   lesson-complete:<lessonId>, quiz-pass:<quizId>, tool-pass:<sessionId>.
+model XpLedger {
+  id        String   @id @default(cuid())
+  userId    String
+  eventKey  String
+  amount    Int
+  reason    String
+  createdAt DateTime @default(now())
+
+  user User @relation(fields: [userId], references: [id], onDelete: Cascade)
+
+  @@unique([userId, eventKey])
+  @@index([userId])
+  @@index([createdAt])
+}
+
 // ============================================================================
 // INVOICES (BIR-compliant sequential numbering for receipts)
 // ============================================================================

prisma/seed.ts

@@ -34,8 +34,29 @@ function hashPassword(password: string): string {
 }
 
 async function upsertAdminUser(): Promise<void> {
-  const email = process.env.ADMIN_EMAIL ?? '[email protected]';
-  const password = process.env.ADMIN_PASSWORD ?? 'ChangeMe123!';
+  // C5: fail loudly when ADMIN_EMAIL or ADMIN_PASSWORD is missing instead of
+  // silently seeding a well-known ([email protected] / ChangeMe123!) admin
+  // account — that fallback shipping to production would be a standing
+  // account-takeover hole.
+  const isProduction = process.env.NODE_ENV === 'production';
+  // Canonicalize to match the app's trim().toLowerCase() contract, so a
+  // mixed-case ADMIN_EMAIL still matches canonicalized sign-in lookups.
+  const rawEmail = (process.env.ADMIN_EMAIL ?? '').trim().toLowerCase();
+  const rawPassword = process.env.ADMIN_PASSWORD ?? '';
+  if (isProduction && (!rawEmail || !rawPassword.trim())) {
+    throw new Error(
+      'ADMIN_EMAIL and ADMIN_PASSWORD environment variables are required in production. ' +
+      'Set them in .env.local or your deployment environment.',
+    );
+  }
+  if (!rawEmail) {
+    throw new Error('ADMIN_EMAIL must not be empty.');
+  }
+  if (!rawPassword.trim()) {
+    throw new Error('ADMIN_PASSWORD must not be empty or whitespace-only.');
+  }
+  const email = rawEmail;
+  const password = rawPassword;
 
   await prisma.user.upsert({
     where: { email },
@@ -382,9 +403,8 @@ async function main(): Promise<void> {
   await grandfatherFreeEnrollment();
 
   console.log('\nSeed complete.');
-  console.log(`\nSign in with: ${process.env.ADMIN_EMAIL ?? '[email protected]'}`);
-  console.log(`Default password: ${process.env.ADMIN_PASSWORD ?? 'ChangeMe123!'}`);
-  console.log('(Change the password after first sign-in.)');
+  console.log(`\nSign in with the admin email you configured (ADMIN_EMAIL): ${process.env.ADMIN_EMAIL ?? '(unset)'}`);
+  console.log('Use the ADMIN_PASSWORD you set. Change it after first sign-in.');
 }
 
 main()

scripts/report-email-collisions.ts

@@ -0,0 +1,83 @@
+#!/usr/bin/env tsx
+/**
+ * Report User rows whose email is not yet canonical (trimmed + lowercase)
+ * because canonicalizing it would collide with another account's email.
+ *
+ * The H6 backfill migration (20260718000000_audit_integrity_fixes) skips these
+ * rows on purpose - normalizing them would violate the case-sensitive unique
+ * email constraint. Canonicalized lookups (findOrCreateUserByEmail,
+ * signUpAction) query by lowercase, so a still-mixed-case row is effectively
+ * unreachable until an operator reconciles the duplicate accounts by hand.
+ *
+ * This script lists them so the reconciliation can actually happen.
+ *
+ * Usage:
+ *   DATABASE_URL="postgres://..." tsx scripts/report-email-collisions.ts
+ */
+import { PrismaClient } from '@prisma/client';
+import { PrismaPg } from '@prisma/adapter-pg';
+
+if (!process.env.DATABASE_URL) {
+  throw new Error('DATABASE_URL environment variable is not set.');
+}
+const prisma = new PrismaClient({ adapter: new PrismaPg(process.env.DATABASE_URL) });
+
+// Raw email addresses are PII; a plain run may land in CI/cron/aggregated logs
+// (CWE-532). Mask by default and require an explicit opt-in to print full
+// addresses for the actual reconciliation. The opaque user id is always shown
+// (it's the key an operator acts on) and is not itself PII.
+const SHOW_RAW_EMAILS = process.env.SHOW_EMAILS === '1' || process.env.SHOW_EMAILS === 'true';
+
+/** Mask the local part, keep the domain: 'Maria@Example.com' -> 'M***@example.com'. */
+function maskEmail(email: string): string {
+  const at = email.lastIndexOf('@');
+  if (at <= 0) return '***';
+  const first = email[0] ?? '';
+  return `${first}***${email.slice(at)}`;
+}
+
+async function main() {
+  // Group by canonical (lowercased) email and report every member of any group
+  // with more than one row. This catches collisions where BOTH rows are
+  // non-canonical (e.g. 'Foo@Bar.com' + 'FOO@bar.com') and share a canonical
+  // form, which an exact-value EXISTS lookup would miss - exactly the rows the
+  // H6 migration leaves un-normalized.
+  const rows = await prisma.$queryRaw<
+    Array<{ id: string; email: string; canonical: string }>
+  >`
+    SELECT u.id, u.email, lower(btrim(u.email)) AS canonical
+    FROM "User" u
+    WHERE lower(btrim(u.email)) IN (
+      SELECT lower(btrim(email))
+      FROM "User"
+      GROUP BY lower(btrim(email))
+      HAVING COUNT(*) > 1
+    )
+    ORDER BY canonical, u.email
+  `;
+
+  if (rows.length === 0) {
+    console.log('No email-canonicalization collisions - every canonical email is unique.');
+    return;
+  }
+
+  console.log(`Found ${rows.length} account(s) in ${new Set(rows.map((r) => r.canonical)).size} colliding group(s), needing manual reconciliation:\n`);
+  for (const r of rows) {
+    const email = SHOW_RAW_EMAILS ? r.email : maskEmail(r.email);
+    const canonical = SHOW_RAW_EMAILS ? r.canonical : maskEmail(r.canonical);
+    console.log(`  ${r.id}  ${email}  ->  canonical "${canonical}"`);
+  }
+  if (!SHOW_RAW_EMAILS) {
+    console.log('\n(Emails masked. Re-run with SHOW_EMAILS=1 to print full addresses for reconciliation.)');
+  }
+  console.log(
+    '\nResolve each group by merging or renaming the duplicate accounts, then re-run the H6 UPDATE.',
+  );
+}
+
+main()
+  .catch((err) => {
+    console.error(err);
+    process.exit(1);
+  })
+  .finally(() => prisma.$disconnect());

src/app/(dashboard)/courses/[courseSlug]/certificate/page.tsx

@@ -9,6 +9,7 @@ import {
   evaluateCourseCompletion,
   issueCertificate,
 } from '@/lib/certificates';
+import { logger } from '@/lib/logger';
 import { IssueButton } from './IssueButton';
 import styles from './certificate.module.css';
 
@@ -70,10 +71,21 @@ export default async function CertificatePage({ params }: PageProps) {
 
   // Auto-issue on page visit if eligible. This way the user sees their existing
   // cert immediately on the final lesson completion without a manual click.
+  // Issuance is best-effort here: a transient failure (or a lost issuance race)
+  // must not crash the whole page to the error boundary. We fall back to showing
+  // progress, and the manual "Issue" button remains as a retry.
   let verificationHash: string | null = activeCert?.verificationHash ?? null;
   if (!verificationHash && completion.isComplete) {
-    const issued = await issueCertificate(user.id, course.id);
-    verificationHash = issued?.verificationHash ?? null;
+    try {
+      const issued = await issueCertificate(user.id, course.id);
+      verificationHash = issued?.verificationHash ?? null;
+    } catch (err) {
+      logger.error(
+        { err, userId: user.id, courseId: course.id },
+        'Certificate auto-issue failed; showing progress fallback',
+      );
+      verificationHash = null;
+    }
   }
 
   const issuedDateFmt = new Intl.DateTimeFormat('en-PH', {

src/app/(public)/auth/signin/SignInForm.tsx

@@ -5,6 +5,7 @@ import { useRouter } from 'next/navigation';
 import { Button, Input } from '@/components/ui';
 import { Icon } from '@/components/ui/Icon';
 import { signInAction, signUpAction } from '@/app/actions/auth';
+import { validateRedirectUrl } from '@/lib/redirect-url';
 import { Toast } from '@/components/ui/Toast';
 import styles from './auth.module.css';
 
@@ -27,7 +28,7 @@ export function SignInForm({
       password: formData.get('password'),
     });
     if (result.success) {
-      const target = result.data.role === 'ADMIN' ? '/admin' : (redirectTo || '/');
+      const target = result.data.role === 'ADMIN' ? '/admin' : validateRedirectUrl(redirectTo);
       router.push(target);
       router.refresh();
       toast('Signed in', 'success');