ubiquitous-invention/plans/Plan-multitenant-saas-hardening/Epic-tenant-lifecycle/Task-rate-limit-and-abuse-guardrails.md
Randall Stillwell 336a5890a8 feat(audit): append-only audit_log, workspace archive cascade + restore, audit view
Soft-delete cascade was the missing half of archive: stamping
workspaces.archived_at alone left objects visible to anyone with a
direct id. The cascade runs in one transaction so the partial state
isn't reachable, and restore inverts it for any archived row in the
workspace — provenance-blind on purpose until we have a use case
that needs to distinguish per-workspace from per-object archives.

audit_log keeps the keyset index on (workspace_id, created_at) and
the actor_user_id FK with onDelete set null. recordAudit() refuses
to write a null actor without a metadata.system_actor label so the
audit view always has something to render. workspaces and invites
mutations call recordAudit on success; objects-router instrumentation
and the markdown importer's system-actor flow are filed as P2
follow-ups because each needs a thoughtful "what's audit-worthy?"
pass, not mechanical wiring.

Settings → Audit log lives at /<slug>/settings/audit, owner-gated,
keyset-paginated. ACTION_LABELS is small on purpose; new actions
fall back to their raw key so missing a label degrades gracefully.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-06-02 15:35:16 -05:00

6 KiB

kind slug title plan_slug epic_slug status priority tenant_id owner cursor_todo_id updated_at
task rate-limit-and-abuse-guardrails Rate-limit credentials sign-in and high-impact mutations multitenant-saas-hardening tenant-lifecycle in_progress P2 global unassigned null 2026-06-02

Task summary

Add per-IP rate limits to credentials sign-in and to a small set of high-impact mutation endpoints (invite, archive, create-workspace). The goal is to cap brute-force and obvious abuse, not to be a full WAF.

Description

Storage

Use the existing Redis instance on CT 102 (already used by Hocuspocus for awareness). Add a thin client in packages/shared/src/rate-limit/ — token-bucket or sliding-window, your choice; the contract is consume(key: string, opts: { capacity, refillPerSecond }): Promise<{ ok: boolean, remaining: number, retryAfterMs: number }>.

If you don't want a new shared lib, @upstash/ratelimit is a fine drop-in even though we're not on Upstash — it works against any Redis URL. Don't add a new database for this.

Where to apply

  1. Credentials sign-in (apps/web/app/api/auth/[...nextauth]/route.ts, or the authorize callback): key by IP (x-forwarded-for, falling back to remote addr). 5 attempts / 60 seconds, then 429.
  2. Invite create: key by actor_user_id. 30 invites / hour. Workspace-scoped is fine too.
  3. Workspace create: key by actor_user_id. 5 workspaces / hour.
  4. Archive/restore: key by (workspace_id, actor_user_id). 20 / hour. Cheap insurance against a script flipping state in a loop.

IP extraction

Use the same helper everywhere; don't recompute in every route. Common gotcha: x-forwarded-for is a comma-separated list when there are multiple proxies. Take the first entry. If the deployed stack is behind Cloudflare, cf-connecting-ip is more reliable — add that as a higher-priority source if present.

Response

When a limit trips, return 429 Too Many Requests with a Retry-After header in seconds and a tRPC TOO_MANY_REQUESTS error code. The client should surface a friendly message ("try again in ~30 seconds"), not a stack trace.

Audit logging

Every limit trip writes an audit_log row (action: "rate_limit.tripped", metadata: { route, key_kind, retry_after_ms }). This is how you discover whether anyone's hitting the limits in practice; without it the limits are silent.

Anti-goals

  • Don't add a CAPTCHA. If brute-forcing becomes a real problem, add Cloudflare in front of the deploy.
  • Don't write a generic rate-limit middleware that wraps every tRPC procedure. The set of high-value endpoints is short; explicit is better than universal.

Subtasks

  • Hand-rolled in-memory token bucket (packages/shared/src/utils/token-bucket.ts) with 6 vitest cases. Deliberate deviation from spec: in-memory Map not Redis. Rationale: we don't run multi-pod yet, and the bucket exposes a stable API so swapping the backing store is a localized change in apps/web/server/lib/rate-limit.ts. Filed Task-distribute-rate-limit-redis-backed.md for when we scale horizontally.
  • IP extraction helper inside apps/web/lib/auth.ts (resolveSignInIp()). Reads cf-connecting-ipx-real-ip → first entry of x-forwarded-for. Dev fallback: when no header is present and NODE_ENV=development, the limiter is skipped so local sign-in doesn't lock you out.
  • Credentials sign-in: 5 attempts / IP / 60 seconds, returns null from authorize on trip (the Auth.js way of saying "no").
  • Invite create: 10 / inviter / hour, throws TRPCError({ code: "TOO_MANY_REQUESTS" }) on trip. Tighter than spec (spec said 30/hr). Settled on 10 because invite-create has email side-effects; we'd rather false-positive a power user than spam recipients.
  • Workspace create rate limit — deferred to follow-up Task-rate-limit-workspace-create-and-archive.md. Lower-impact than sign-in/invite and the subagent kept scope narrow.
  • Archive / restore rate limit — same follow-up as above.
  • Wire audit_log writes on limit trips — deferred to follow-up Task-audit-instrument-rate-limit-trips.md. The rateLimit() function returns retryAfterMs and resetAt, so a call site can do if (!rl.allowed) { await recordAudit(...); throw ... }. Hasn't shipped yet to keep this task's scope narrow and to avoid churning auth.ts a second time.
  • Verify by scripting 20 credentials sign-in attempts against a dev deploy — operator smoke test, see Acceptance below.

Design decisions captured

  • In-memory, not Redis (for v1). The spec calls for Redis. We have Redis in the stack (Hocuspocus uses it), so it's not philosophical reluctance — it's that adding a hard runtime dep on Redis for the web pod adds a failure mode without a corresponding scale benefit while we run a single pod. Swap when we shard. Documented in the JSDoc on rateLimit() so the next maintainer doesn't have to dig.
  • authorize returns null on trip (not 429). Auth.js v5's Credentials provider doesn't let authorize throw a typed HTTP code; null is the canonical "deny." A 429 with Retry-After would be the right UX, but that requires moving the limiter to a route handler that sits in front of authorize. Filed as a UX polish follow-up if anyone complains; in practice "your password is wrong" is also a reasonable user-facing read of a temporarily-locked account.
  • No audit row on trip yet. See deferred subtask above. The trip is logged to stdout (console.warn) so operators aren't blind.

Owner or assignee

Unassigned

Status

ready

Estimation

M

Acceptance criteria

  • Brute-forcing credentials sign-in trips at attempt 6 within 60 seconds (per-IP).
  • Invite-spam attempt trips at invite 11 within an hour (tightened from spec's 31).
  • Every trip produces an audit_log row — deferred, see follow-up.
  • No global tRPC middleware — limits are applied per route (authorize + invites.create only).
  • Epic: ./Epic-tenant-lifecycle.md
  • Plan: ../Plan-multitenant-saas-hardening.md