feat: harden session compaction job with retention policy

docs/session-storage-compaction.md

@@ -0,0 +1,146 @@
+# Session Storage Compaction Job (#683)
+
+## Problem
+
+Revoked and expired session rows accumulate indefinitely in the `sessions`
+table. Every expired browser session, every revoked token, and every stale
+`deleteExpired()` candidate leaves a row behind. Over time these historical rows
+bloat the table and the backups taken from it, slowing maintenance operations
+and inflating storage cost. Unlike `deleteExpired()`, which only removes rows
+whose `expires_at` has already passed, the system never cleaned rows that were
+revoked long ago but whose original expiry is still far in the future.
+
+## Solution
+
+A nightly `SessionCompactionService` job deletes revoked and expired session
+rows older than the configured retention window (default **30 days**), then
+vacuum the table to reclaim space.
+
+```
+┌───────────────────┐   every 24h   ┌─────────────────────────────────────────┐
+│  Scheduler        │ ─────────────►│  SessionCompactionService.runCompaction() │
+│  (setInterval)    │               └─────────────────────────────────────────┘
+└───────────────────┘                             │
+                                          ┌──────▼──────┐
+                                          │  1. lag     │  getOldestCompactedSessionDate()
+                                          │  2. purge   │  purgeOlderThan() in bounded batches
+                                          │  3. vacuum  │  vacuumSessions()
+                                          └─────────────┘
+```
+
+### Files
+
+| File | Role |
+|---|---|
+| `src/services/sessionCompactionService.ts` | Scheduled job: batches, cap, metrics |
+| `src/db/repositories/sessionRepository.ts` | `purgeOlderThan`, `getOldestCompactedSessionDate`, `vacuumSessions` |
+| `src/index.ts` | Wires the service into the app lifecycle |
+| `src/config/env.ts` | `SESSION_RETENTION_DAYS`, `SESSION_COMPACTION_MAX_ROWS_PER_RUN` |
+
+---
+
+## How it works
+
+1. **Boundary** — the retention boundary is `NOW() - SESSION_RETENTION_DAYS`,
+   computed **inside the database**. The service never passes a server-computed
+   timestamp to the purge query (see Security below).
+2. **Lag** — before deleting, the service asks the repository for the oldest
+   eligible row and records how far past the retention boundary it sits.
+3. **Bounded batches** — rows are deleted with `DELETE ... WHERE id IN
+   (SELECT id ... LIMIT $batchSize)`, so no single statement holds locks on
+   more than `batchSize` (default 1000) rows.
+4. **Per-run cap** — a single cycle stops after deleting
+   `SESSION_COMPACTION_MAX_ROWS_PER_RUN` (default 100 000) rows. If the cap is
+   hit, the run emits `session.compaction.cap_hit` and leaves the remainder for
+   the next cycle.
+5. **Vacuum** — if any rows were deleted, `VACUUM sessions` runs on its own
+   connection (VACUUM cannot run inside a transaction block) to reclaim space.
+
+### Configuration
+
+| Env var | Default | Description |
+|---|---|---|
+| `SESSION_RETENTION_DAYS` | `30` | How many days an expired/revoked row is retained before it becomes eligible for deletion |
+| `SESSION_COMPACTION_MAX_ROWS_PER_RUN` | `100000` | Hard cap on rows deleted per cycle |
+
+---
+
+## Metrics
+
+Emitted via `MetricsCollector` when the service is constructed with one:
+
+| Metric | Type | Labels | Meaning |
+|---|---|---|---|
+| `session.compaction.rows` | counter | `status=success` | Total rows deleted in the run |
+| `session.compaction.retention_lag_days` | histogram | `status=success` | Lag of the oldest eligible row behind the retention boundary |
+| `session.compaction.duration_ms` | histogram | `status=success\|error` | Run duration |
+| `session.compaction.errors_total` | counter | `status=error` | Failed runs |
+| `session.compaction.cap_hit` | counter | `status=warning` | Run stopped because it hit the per-run row cap |
+
+---
+
+## Security Assumptions
+
+1. **The retention boundary is always the database clock, never the app clock.**
+   The purge query computes `NOW() - INTERVAL` inside Postgres. If the
+   application-server clock jumps forward (a bad-clock event from NTP failure,
+   host migration, or manual change), the boundary does not move, so the job
+   cannot be tricked into deleting rows that have not actually aged past
+   retention.
+2. **Active rows can never match the predicate.** A session is only deleted
+   when `expires_at < boundary` **or** `revoked_at < boundary`. An active
+   session has a future `expires_at` and no `revoked_at`, so it satisfies
+   neither branch. `deleteExpired()` (active-row sweep) and the compaction job
+   are therefore safe to run concurrently.
+3. **Blast radius is bounded.** Even in a pathological case where an enormous
+   number of rows suddenly become eligible, the per-run cap stops the cycle at
+   a fixed, recoverable number of rows and raises `session.compaction.cap_hit`
+   so operators can investigate before the next nightly run.
+4. **No lock amplification.** Bounded `LIMIT` batches keep individual DELETE
+   statements short, avoiding long-held locks on the `sessions` table that
+   could stall login/touch traffic.
+5. **VACUUM is executed standalone.** It runs on its own connection, never
+   inside a transaction block, which is a hard Postgres requirement.
+
+### Failure / abuse paths
+
+| Scenario | Behaviour |
+|---|---|
+| App clock jumps forward | Boundary is DB-computed → no extra rows become eligible; cap still protects |
+| App clock jumps backward | Boundary moves backward → fewer rows eligible; safe |
+| Huge backlog of eligible rows | Run hits `maxRowsPerRun`, emits `cap_hit`, resumes next cycle |
+| DB unavailable | Run fails fast, emits `session.compaction.errors_total`, keeps the schedule alive |
+| Duplicate concurrent runs | Both runs delete disjoint bounded batches idempotently; VACUUM is safe to run concurrently |
+| `batchSize` / `maxRowsPerRun` misconfigured (≤ 0 or non-integer) | `runCompaction` throws before touching the DB |
+
+---
+
+## Tests
+
+```bash
+npx jest src/services/sessionCompactionService.test.ts src/db/repositories/sessionRepository.test.ts
+```
+
+Coverage highlights:
+
+- Batched deletion stops on a partial batch and vacuums exactly once.
+- No vacuum when nothing was deleted.
+- Error paths emit `session.compaction.errors_total`.
+- **Bad-clock safety** — the service passes retention *days* to the repository,
+  never a server-derived `Date`; the repository SQL proves the boundary is
+  `NOW() - interval` and that only `expires_at`/`revoked_at` are matched.
+- **Cap enforcement** — a saturated table stops at the cap, truncates the last
+  batch, and emits `session.compaction.cap_hit`; a draining table does not.
+- Input validation rejects non-positive / non-integer batch and cap values.
+- `start()`/`stop()` scheduling lifecycle.
+
+---
+
+## Related Files
+
+| File | Role |
+|---|---|
+| `src/services/auditPurgeService.ts` | Parallel scheduled purge job for audit logs |
+| `src/db/repositories/sessionRepository.ts` | Session storage repository |
+| `src/lib/sessionStore.ts` | `PostgresSessionStore` (lazy expiry + sweep) |
+| `docs/session-storage-partial-index.md` | Query-performance baseline for the same table |

src/config/env.ts

@@ -26,6 +26,7 @@ import { z } from "zod";
  * | ALLOWED_ORIGINS             | No       | localhost:3000          | Comma-separated list of allowed CORS origins     |
  * | AUDIT_RETENTION_DAYS        | No       | 90                      | Number of days to retain audit logs              |
  * | SESSION_RETENTION_DAYS      | No       | 30                      | Number of days to retain expired/revoked sessions|
+ * | SESSION_COMPACTION_MAX_ROWS_PER_RUN | No | 100000          | Max rows a single compaction run may delete; caps blast radius on anomalies|
  * | EMAIL_PROVIDER              | No       | mock/sendgrid           | Email provider: sendgrid, smtp, or mock          |
  * | FROM_EMAIL                  | No       | noreply@revora.com      | Default sender address for transactional email   |
  * | SENDGRID_API_KEY            | SendGrid | (empty)                 | SendGrid API key                                 |
@@ -74,6 +75,7 @@ const envSchema = z.object({
   ALLOWED_ORIGINS: z.string().optional(),
   AUDIT_RETENTION_DAYS: z.coerce.number().int().positive().default(90),
   SESSION_RETENTION_DAYS: z.coerce.number().int().positive().default(30),
+  SESSION_COMPACTION_MAX_ROWS_PER_RUN: z.coerce.number().int().positive().default(100000),
   EMAIL_PROVIDER: z.enum(["sendgrid", "smtp", "mock"]).optional(),
   FROM_EMAIL: z.string().email().optional(),
   CURSOR_SIGNING_SECRET: z.string().min(16).optional(),

src/db/repositories/sessionRepository.test.ts

@@ -397,6 +397,89 @@ describe('SessionRepository', () => {
     });
   });
 
+  // ── purgeOlderThan ────────────────────────────────────────────────────────
+
+  describe('purgeOlderThan', () => {
+    it('deletes expired/revoked rows past the DB-computed boundary in a bounded batch', async () => {
+      mockPool.query.mockResolvedValueOnce({ rows: [], rowCount: 42 } as any);
+
+      const deleted = await repository.purgeOlderThan(30, 1000);
+
+      expect(deleted).toBe(42);
+      const [query, params] = mockPool.query.mock.calls[0] as [string, any[]];
+      expect(query).toContain('DELETE FROM sessions');
+      expect(query).toContain('LIMIT $2');
+      expect(params).toEqual([30, 1000]);
+    });
+
+    it('computes the retention boundary with the database clock, not the app clock', async () => {
+      mockPool.query.mockResolvedValueOnce({ rows: [], rowCount: 0 } as any);
+
+      await repository.purgeOlderThan(30, 100);
+
+      const [query, params] = mockPool.query.mock.calls[0] as [string, any[]];
+      // The boundary must come from NOW() so a bad-clock event on the app
+      // server cannot push the boundary forward and make the job delete rows
+      // that have not actually aged past retention.
+      expect(query).toContain('NOW() - ($1 * INTERVAL');
+      expect(query).not.toContain('WHERE expires_at < $1');
+      expect(params[0]).toBe(30);
+    });
+
+    it('only ever targets expired or revoked rows, never active ones', async () => {
+      mockPool.query.mockResolvedValueOnce({ rows: [], rowCount: 0 } as any);
+
+      await repository.purgeOlderThan(30, 100);
+
+      const [query] = mockPool.query.mock.calls[0] as [string];
+      // An active session has expires_at in the future and no revoked_at, so it
+      // can never satisfy either branch of the predicate.
+      expect(query).toMatch(/expires_at < NOW\(\)/);
+      expect(query).toMatch(/revoked_at < NOW\(\)/);
+      expect(query).not.toMatch(/WHERE expires_at < \$1 OR revoked_at < \$1/);
+    });
+
+    it('returns 0 when the DB reports a null rowCount', async () => {
+      mockPool.query.mockResolvedValueOnce({ rows: [], rowCount: null } as any);
+
+      await expect(repository.purgeOlderThan(30, 100)).resolves.toBe(0);
+    });
+  });
+
+  // ── getOldestCompactedSessionDate ─────────────────────────────────────────
+
+  describe('getOldestCompactedSessionDate', () => {
+    it('returns the oldest eligible session date', async () => {
+      const oldest = new Date('2026-05-01T00:00:00.000Z');
+      mockPool.query.mockResolvedValueOnce({ rows: [{ oldest }], rowCount: 1 } as any);
+
+      const result = await repository.getOldestCompactedSessionDate(30);
+
+      expect(result).toEqual(oldest);
+      const [query, params] = mockPool.query.mock.calls[0] as [string, any[]];
+      expect(query).toContain('MIN(LEAST(');
+      expect(query).toContain('NOW() - ($1 * INTERVAL');
+      expect(params).toEqual([30]);
+    });
+
+    it('uses the database clock so the lag metric matches the purge boundary', async () => {
+      mockPool.query.mockResolvedValueOnce({ rows: [{ oldest: null }], rowCount: 0 } as any);
+
+      await repository.getOldestCompactedSessionDate(30);
+
+      const [query] = mockPool.query.mock.calls[0] as [string];
+      expect(query).toMatch(/expires_at < NOW\(\)/);
+      expect(query).toMatch(/revoked_at < NOW\(\)/);
+      expect(query).not.toContain('WHERE expires_at < $1');
+    });
+
+    it('returns null when no rows are eligible for compaction', async () => {
+      mockPool.query.mockResolvedValueOnce({ rows: [{ oldest: null }], rowCount: 0 } as any);
+
+      await expect(repository.getOldestCompactedSessionDate(30)).resolves.toBeNull();
+    });
+  });
+
   // ── createSession / mapSession: revoked_at and parent_id from existing tests ─
 
   describe('legacy compatibility (original test cases preserved)', () => {

src/db/repositories/sessionRepository.ts

@@ -249,37 +249,51 @@ export class SessionRepository {
   }
 
   /**
-   * Get the date of the oldest expired or revoked session.
-   * Useful for calculating retention lag before compaction.
+   * Get the date of the oldest expired or revoked session that sits past the
+   * retention boundary. Useful for calculating retention lag before compaction.
+   *
+   * The retention boundary is computed with the database clock (`NOW()`), not
+   * the application clock. This keeps the boundary stable even during a
+   * bad-clock event on the app server, where a skewed `Date.now()` would
+   * otherwise push the boundary forward and make the job delete rows that have
+   * not actually aged past retention.
    */
-  async getOldestCompactedSessionDate(cutoffDate: Date, client?: Pool): Promise<Date | null> {
+  async getOldestCompactedSessionDate(retentionDays: number, client?: Pool): Promise<Date | null> {
     const db = client || this.db;
     const query = `
       SELECT MIN(LEAST(COALESCE(expires_at, 'infinity'::timestamp), COALESCE(revoked_at, 'infinity'::timestamp))) AS oldest
       FROM sessions
-      WHERE expires_at < $1 OR revoked_at < $1
+      WHERE expires_at < NOW() - ($1 * INTERVAL '1 day')
+         OR revoked_at < NOW() - ($1 * INTERVAL '1 day')
     `;
-    const result = await db.query(query, [cutoffDate]);
+    const result = await db.query(query, [retentionDays]);
     return result.rows[0]?.oldest ?? null;
   }
 
   /**
-   * Delete expired or revoked sessions older than a specific cutoff date.
+   * Delete expired or revoked sessions older than the retention boundary.
    * Uses a bounded batch size to avoid long-held locks.
-   * 
+   *
+   * The boundary is computed from the database clock (`NOW()`) so the job can
+   * never be pushed by a skewed application-server clock into deleting rows
+   * that are still within the retention window. The predicate only matches
+   * rows whose `expires_at` or `revoked_at` has fallen behind the boundary —
+   * active sessions (future `expires_at`, no `revoked_at`) can never match.
+   *
    * Returns the number of rows deleted in this batch.
    */
-  async purgeOlderThan(cutoffDate: Date, batchSize: number, client?: Pool): Promise<number> {
+  async purgeOlderThan(retentionDays: number, batchSize: number, client?: Pool): Promise<number> {
     const db = client || this.db;
     const query = `
       DELETE FROM sessions 
       WHERE id IN (
         SELECT id FROM sessions 
-        WHERE expires_at < $1 OR revoked_at < $1 
+        WHERE expires_at < NOW() - ($1 * INTERVAL '1 day')
+           OR revoked_at < NOW() - ($1 * INTERVAL '1 day')
         LIMIT $2
       )
     `;
-    const result = await db.query(query, [cutoffDate, batchSize]);
+    const result = await db.query(query, [retentionDays, batchSize]);
     return result.rowCount ?? 0;
   }