From e275b1111af103a451a9cec4987a9b478cdf96fb Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 12:48:01 +0000 Subject: [PATCH 01/20] fix(config): refuse unusable rate-limit windows at boot RATE_LIMIT_WINDOW (and the auth service's PRIVACY_RATE_LIMIT_WINDOW) was an unchecked string. A value the limiter cannot read ("abc", "1 parsec", empty, "0") let the service start and then answer every business request with a 500; a negative window or a bare number ("60" is read as 60 ms) started fine and limited nothing. Both are now validated when the service boots, between one second and seven days, as " ". Also documents RATE_LIMIT_MAX and RATE_LIMIT_WINDOW in .env.example, starts NEXT.md (working list) and docs/17_break_report.md (findings), and rebuilds the committed bundles. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1 --- .env.example | 7 +++ NEXT.md | 42 ++++++++++++++ api/admin/index.js | 40 ++++++++++++- api/analytics/index.js | 40 ++++++++++++- api/applications/index.js | 40 ++++++++++++- api/auth/index.js | 42 +++++++++++++- api/jobs/index.js | 40 ++++++++++++- api/learning/index.js | 40 ++++++++++++- api/matching/index.js | 40 ++++++++++++- api/outreach/index.js | 40 ++++++++++++- api/profile/index.js | 40 ++++++++++++- api/referrals/index.js | 40 ++++++++++++- docs/17_break_report.md | 18 ++++++ packages/config/src/env.ts | 57 ++++++++++++++++++- .../config/test/rate-limit-window.test.ts | 49 ++++++++++++++++ services/auth/src/env.ts | 3 +- 16 files changed, 565 insertions(+), 13 deletions(-) create mode 100644 NEXT.md create mode 100644 docs/17_break_report.md create mode 100644 packages/config/test/rate-limit-window.test.ts diff --git a/.env.example b/.env.example index 731321b..79b5b55 100644 --- a/.env.example +++ b/.env.example @@ -75,6 +75,13 @@ SUPABASE_ACCESS_TOKEN= # jobs-service, referrals-service and learning-service only. PLATFORM_ADMIN_USER_IDS= +# --- Rate limiting (every service) -------------------------------------------- +# At most RATE_LIMIT_MAX requests per client address in each RATE_LIMIT_WINDOW. The window is a +# number and a unit between 1 second and 7 days ("30 seconds", "1 minute", "1 hour"). A bare number +# such as 60, or text the limiter cannot read, stops the service from starting. +# RATE_LIMIT_MAX=300 +# RATE_LIMIT_WINDOW=1 minute + # --- Account privacy (auth service) ------------------------------------------- # Per-client-address budgets for GET /v1/auth/me/export and DELETE /v1/auth/me. They replace # (do not add to) the general RATE_LIMIT_* budget on those two routes. Counters live in each diff --git a/NEXT.md b/NEXT.md new file mode 100644 index 0000000..0427876 --- /dev/null +++ b/NEXT.md @@ -0,0 +1,42 @@ +# NEXT + +Working list for the autonomous stretch that started 2026-10-31. Most valuable first. + +**Goal for this stretch:** break the project locally, find real defects, fix them, and record what held, so +the 2026-11-17 launch (individuals plus one university pilot) rests on tested behaviour. The brief that +describes the attack areas (A to G) lives outside the repo; each task below names its area. + +**Ground rules for this stretch** +- Local only: tests and scripts run against a private Postgres (`atlas_integ`) and in-process Fastify apps. Nothing is run against Vercel, Supabase or any live system. +- No secrets or `.env` files touched. No new dependencies unless a task says why. +- Work happens on `work/` branches cut from `claude/blissful-darwin-2n33wv` (the unmerged launch branch). Nothing is committed to `main`, nothing is merged. +- A task that needs a decision goes under "Needs Nikshey" and is skipped. +- Checks per task: `pnpm typecheck`, `npx eslint . --ignore-pattern '.claude/worktrees/**'`, and the affected tests (`ATLAS_TEST_DATABASE_URL=postgres://atlas:atlas@localhost:5432/atlas_integ npx vitest run `). `node scripts/bundle-backend.mjs` when service or package sources change. + +## Queue + +2. [ ] **Internal routes and rewrites (A/F).** Secret comparison edge cases on every `/internal/*` route (empty, wrong length, duplicate headers, query string, other methods); `vercel.json` rewrite confusion toward `/internal/*`. +3. [ ] **Env boot fuzz (F).** Each service with each env var missing, empty, garbage: `RATE_LIMIT_MAX=0`, `CORS_ORIGINS` variants, `PLATFORM_ADMIN_USER_IDS` oddities, `CRON_SECRET` short. Fail fast or fail safe, never open. +4. [ ] **Authorization matrix (A).** Every route of every service against anonymous, other user, other org staff, same org roles, platform admin. Mass assignment on create and update bodies. +5. [ ] **Validation and pagination fuzz (B).** Malformed JSON, wrong types, oversized bodies, huge `limit` and `offset`, odd UUIDs. Always a clean 4xx envelope, never 500. +6. [ ] **Job-source fetchers (D).** SSRF via board token and redirects, response size and time limits, hostile postings (script tags, `javascript:` apply links), cross-company overwrite. +7. [ ] **ReDoS audit (D).** Every regex in `packages/ai`, `services/profile/src`, `services/jobs/src` with adversarial 200 KB inputs. +8. [ ] **Upload parsers (D).** Hostile PDFs and DOCX (zip bombs, traversal names, truncation), filenames, multipart edge cases. +9. [ ] **Races and idempotency (C).** Parallel application creates, parallel status moves, double drains, double account deletion. +10. [ ] **Committed bundles end to end (G).** Boot `api/*/index.js` locally with a fake JWKS issuer, check `scripts/bundle-backend.mjs` is byte-for-byte repeatable. +11. [ ] **Secrets and logging hygiene (F).** Secret-like strings in tree, history and bundles; log redaction; log injection. +12. [ ] **Frontends with hostile data (E).** Web and admin rendering with script-like and oversized data, open-redirect params, CSV formula safety, security headers. + +## Done + +- [x] **Rate limiter vs real client IP (F)** — `work/break-fix-1`. Found and fixed: a bad `RATE_LIMIT_WINDOW` booted and then 500'd every request, or silently disabled limiting. Checked and held: store is a bounded LRU, exempt routes are by scope not path. Spoofable `X-Forwarded-For` is safe on Vercel (it overwrites the header); noted in `docs/17_break_report.md`. Typecheck, lint, full suite (119 files, 1,730 tests) pass; bundles rebuilt. + +## Blocked + +(nothing yet) + +## Needs Nikshey + +- [ ] **Goal line was blank.** The goal in your message was still the placeholder text. I assumed "break the project locally, fix what I find, get ready for the 2026-11-17 launch". Tell me if you want a different focus. +- [ ] **Google Fonts at build time.** `next/font/google` made the admin build fail once in CI. The fix is to ship the font files with the repo (`next/font/local`) or use a system font stack. That changes how the site looks or adds font files, so it is your call. +- [ ] **Pushing branches.** Every push of any branch builds twelve Vercel projects and the free plan allows 100 deployments a day. I will commit locally and push only at clean points, in batches. Say if you want a different rhythm. diff --git a/api/admin/index.js b/api/admin/index.js index c393443..3192aae 100644 --- a/api/admin/index.js +++ b/api/admin/index.js @@ -77396,13 +77396,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/analytics/index.js b/api/analytics/index.js index de157a5..96b16e9 100644 --- a/api/analytics/index.js +++ b/api/analytics/index.js @@ -86034,13 +86034,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/applications/index.js b/api/applications/index.js index 7902cf7..b67a07e 100644 --- a/api/applications/index.js +++ b/api/applications/index.js @@ -86052,13 +86052,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/auth/index.js b/api/auth/index.js index e99cc3f..3248b44 100644 --- a/api/auth/index.js +++ b/api/auth/index.js @@ -77570,13 +77570,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { @@ -78559,7 +78597,7 @@ var authServiceEnvSchema = external_exports.object({ */ PRIVACY_EXPORT_RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(10), PRIVACY_DELETE_RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(3), - PRIVACY_RATE_LIMIT_WINDOW: external_exports.string().min(1).default("1 hour") + PRIVACY_RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 hour") }); function loadAuthServiceEnv(source = process.env) { const normalized = { ...source, PORT: source.PORT ?? source.AUTH_SERVICE_PORT }; diff --git a/api/jobs/index.js b/api/jobs/index.js index b9dc9ee..d4454bd 100644 --- a/api/jobs/index.js +++ b/api/jobs/index.js @@ -86083,13 +86083,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/learning/index.js b/api/learning/index.js index f851db4..c4a74d2 100644 --- a/api/learning/index.js +++ b/api/learning/index.js @@ -86110,13 +86110,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/matching/index.js b/api/matching/index.js index c887141..a6ee174 100644 --- a/api/matching/index.js +++ b/api/matching/index.js @@ -85277,13 +85277,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/outreach/index.js b/api/outreach/index.js index 62003bf..98fad24 100644 --- a/api/outreach/index.js +++ b/api/outreach/index.js @@ -86066,13 +86066,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/profile/index.js b/api/profile/index.js index 5fd821b..03a6ad4 100644 --- a/api/profile/index.js +++ b/api/profile/index.js @@ -120324,13 +120324,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/api/referrals/index.js b/api/referrals/index.js index bc8dcab..cc0e892 100644 --- a/api/referrals/index.js +++ b/api/referrals/index.js @@ -86128,13 +86128,51 @@ var storageEnvSchema = external_exports.object({ var supabaseAuthEnvSchema = external_exports.object({ SUPABASE_URL: external_exports.string().url("SUPABASE_URL must be a valid URL") }); +var WINDOW_UNIT_MS = { + s: 1e3, + sec: 1e3, + secs: 1e3, + second: 1e3, + seconds: 1e3, + m: 6e4, + min: 6e4, + mins: 6e4, + minute: 6e4, + minutes: 6e4, + h: 36e5, + hr: 36e5, + hrs: 36e5, + hour: 36e5, + hours: 36e5, + d: 864e5, + day: 864e5, + days: 864e5 +}; +var RATE_LIMIT_WINDOW_MIN_MS = 1e3; +var RATE_LIMIT_WINDOW_MAX_MS = 7 * 864e5; +function parseRateLimitWindowMs(text2) { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text2.trim()); + if (match === null) return void 0; + const unit = WINDOW_UNIT_MS[(match[2] ?? "").toLowerCase()]; + if (unit === void 0) return void 0; + return Math.round(Number(match[1]) * unit); +} +var rateLimitWindowSchema = external_exports.string().refine( + (text2) => { + const ms = parseRateLimitWindowMs(text2); + return ms !== void 0 && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' + } +); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: external_exports.string().default(""), RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: external_exports.string().default("1 minute"), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) }); var ConfigError = class extends Error { diff --git a/docs/17_break_report.md b/docs/17_break_report.md new file mode 100644 index 0000000..d19e6b6 --- /dev/null +++ b/docs/17_break_report.md @@ -0,0 +1,18 @@ +# 17. Break report + +Findings from trying to break Project Atlas locally (in-process apps, a private Postgres, no live +systems). Newest cycle last. Severity follows the scale used for the exercise: Critical (cross-tenant +access, privilege escalation, secret exposure), High (data loss, business rule bypass), Medium (500s or +hangs on hostile input, silent fail-open settings), Low (hardening gaps), Info (things that held). +Work list: `NEXT.md`. + +| # | Sev | Area | Finding | State | +|---|-----|------|---------|-------| +| 1 | Medium | Config | `RATE_LIMIT_WINDOW` was an unchecked string. `abc`, `1 parsec`, an empty value or `0` booted fine and then every business request answered 500 (the limiter threw per request). A negative value (`-5 minutes`) and a bare number (`60`, read as 60 ms) booted fine and limited nothing. The same held for `PRIVACY_RATE_LIMIT_WINDOW`. | Fixed: both are validated at boot (`rateLimitWindowSchema`, one second to seven days, ` ` only). Tests in `packages/config/test`. | +| 2 | Info | Rate limit | The limiter's in-memory store is a 5,000-entry LRU, so many unique clients cannot grow memory without bound. | Held. | +| 3 | Low | Rate limit | Clients are told apart by `request.ip` with `trustProxy: true`. That is the first `X-Forwarded-For` entry, which a client chooses unless the edge overwrites the header. Vercel does overwrite it, so the launch deployment is not affected; behind a proxy that only appends, one client could pick fresh buckets at will, or push others' buckets out of the LRU. | Accepted for launch, documented in `registerRateLimitedRoutes`. Revisit if the API ever moves behind another proxy. | + +## What was checked and held + +- `RATE_LIMIT_MAX` of 0 is refused at boot (`min(1)`). +- `/health`, `/metrics` and `/internal/*` are exempt because they are registered outside the limited scope, not by path matching, so no path trick can move a business route into the exempt set. diff --git a/packages/config/src/env.ts b/packages/config/src/env.ts index 11825f8..b4bab47 100644 --- a/packages/config/src/env.ts +++ b/packages/config/src/env.ts @@ -57,13 +57,68 @@ export const supabaseAuthEnvSchema = z.object({ SUPABASE_URL: z.string().url('SUPABASE_URL must be a valid URL'), }); +const WINDOW_UNIT_MS: Record = { + s: 1_000, + sec: 1_000, + secs: 1_000, + second: 1_000, + seconds: 1_000, + m: 60_000, + min: 60_000, + mins: 60_000, + minute: 60_000, + minutes: 60_000, + h: 3_600_000, + hr: 3_600_000, + hrs: 3_600_000, + hour: 3_600_000, + hours: 3_600_000, + d: 86_400_000, + day: 86_400_000, + days: 86_400_000, +}; + +const RATE_LIMIT_WINDOW_MIN_MS = 1_000; +const RATE_LIMIT_WINDOW_MAX_MS = 7 * 86_400_000; + +/** + * The length in milliseconds of a rate-limit window written as ` ` (`30 seconds`, + * `1 minute`, `2h`), or `undefined` when the text is not that. A bare number is refused on purpose: + * the rate-limit plugin reads `60` as 60 milliseconds, which switches limiting off in practice. + */ +export function parseRateLimitWindowMs(text: string): number | undefined { + const match = /^(\d+(?:\.\d+)?)\s*([a-z]+)$/i.exec(text.trim()); + if (match === null) return undefined; + const unit = WINDOW_UNIT_MS[(match[2] ?? '').toLowerCase()]; + if (unit === undefined) return undefined; + return Math.round(Number(match[1]) * unit); +} + +/** + * A rate-limit window such as `1 minute`. The plugin accepts any string at boot and then answers + * every request with a 500 for text it cannot read (`abc`, `1 parsec`, empty), and a negative + * window quietly turns limiting off, so the value is checked here, where a bad one stops the boot. + * Between one second and seven days: below that the limit means nothing, above it a client that + * hits the maximum is locked out for practically ever. + */ +export const rateLimitWindowSchema = z.string().refine( + (text) => { + const ms = parseRateLimitWindowMs(text); + return ms !== undefined && ms >= RATE_LIMIT_WINDOW_MIN_MS && ms <= RATE_LIMIT_WINDOW_MAX_MS; + }, + { + message: + 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"', + }, +); + export const httpEnvSchema = z.object({ HOST: z.string().min(1).default('0.0.0.0'), PORT: z.coerce.number().int().min(1).max(65_535), /** Comma-separated list, or `*` in development only. */ CORS_ORIGINS: z.string().default(''), RATE_LIMIT_MAX: z.coerce.number().int().min(1).default(300), - RATE_LIMIT_WINDOW: z.string().default('1 minute'), + RATE_LIMIT_WINDOW: rateLimitWindowSchema.default('1 minute'), BODY_LIMIT_BYTES: z.coerce.number().int().min(1024).default(1_048_576), }); diff --git a/packages/config/test/rate-limit-window.test.ts b/packages/config/test/rate-limit-window.test.ts new file mode 100644 index 0000000..2322c84 --- /dev/null +++ b/packages/config/test/rate-limit-window.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest'; +import { httpEnvSchema, parseRateLimitWindowMs, rateLimitWindowSchema } from '../src/index.js'; + +describe('rate-limit window', () => { + it.each([ + ['1 second', 1_000], + ['30 seconds', 30_000], + ['1 minute', 60_000], + ['2 mins', 120_000], + ['5m', 300_000], + ['1 hour', 3_600_000], + ['2H', 7_200_000], + ['1 day', 86_400_000], + ['1.5 minutes', 90_000], + [' 10 seconds ', 10_000], + ])('reads %j as %d ms', (text, ms) => { + expect(parseRateLimitWindowMs(text)).toBe(ms); + expect(rateLimitWindowSchema.safeParse(text).success).toBe(true); + }); + + // Each of these booted a service that then answered every request with a 500, or one that + // never limited anything, before the value was checked at boot. + it.each([ + '', + ' ', + 'abc', + '1 parsec', + '0', + '60', + '1000', + '0 seconds', + '-5 minutes', + '1e400', + 'NaN', + '500 ms', + '8 days', + '99999 years', + '1 minute; drop table', + ])('refuses %j', (text) => { + expect(rateLimitWindowSchema.safeParse(text).success).toBe(false); + }); + + it('is what RATE_LIMIT_WINDOW is validated with, and keeps the one-minute default', () => { + const base = { PORT: '4000' }; + expect(httpEnvSchema.parse(base).RATE_LIMIT_WINDOW).toBe('1 minute'); + expect(httpEnvSchema.safeParse({ ...base, RATE_LIMIT_WINDOW: 'abc' }).success).toBe(false); + expect(httpEnvSchema.safeParse({ ...base, RATE_LIMIT_WINDOW: '-5 minutes' }).success).toBe(false); + }); +}); diff --git a/services/auth/src/env.ts b/services/auth/src/env.ts index 7ebee8a..2ef22b8 100644 --- a/services/auth/src/env.ts +++ b/services/auth/src/env.ts @@ -3,6 +3,7 @@ import { databaseEnvSchema, httpEnvSchema, loadEnv, + rateLimitWindowSchema, storageEnvSchema, supabaseAuthEnvSchema, } from '@atlas/config'; @@ -27,7 +28,7 @@ export const authServiceEnvSchema = z.object({ */ PRIVACY_EXPORT_RATE_LIMIT_MAX: z.coerce.number().int().min(1).default(10), PRIVACY_DELETE_RATE_LIMIT_MAX: z.coerce.number().int().min(1).default(3), - PRIVACY_RATE_LIMIT_WINDOW: z.string().min(1).default('1 hour'), + PRIVACY_RATE_LIMIT_WINDOW: rateLimitWindowSchema.default('1 hour'), }); export type AuthServiceEnv = z.infer; From fdba82f2505514b8e27ac835926f59baa4f589cd Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 12:51:14 +0000 Subject: [PATCH 02/20] fix(config): refuse CORS origins that can never match; test vercel routing CORS_ORIGINS entries were not checked: a trailing slash, a bare host, a URL with a path or "null" let a service start and then matched no browser Origin, so every browser call failed with a CORS error and nothing said why. Origins are now normalized to the exact form a browser sends (a trailing slash and upper-case are forgiven) and an entry that can never match stops the boot, with the entry named. Adds a vercel.json routing test (every public route of each deployed service resolves to its own function; every rewrite has a bundle and a function entry), records both in docs/17_break_report.md, documents CORS_ORIGINS in .env.example and rebuilds the committed bundles. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1 --- .env.example | 6 ++ NEXT.md | 6 +- api/admin/index.js | 34 +++++++- api/analytics/index.js | 34 +++++++- api/applications/index.js | 34 +++++++- api/auth/index.js | 34 +++++++- api/jobs/index.js | 34 +++++++- api/learning/index.js | 34 +++++++- api/matching/index.js | 34 +++++++- api/outreach/index.js | 34 +++++++- api/profile/index.js | 34 +++++++- api/referrals/index.js | 34 +++++++- docs/17_break_report.md | 4 + packages/config/src/env.ts | 61 +++++++++++-- packages/config/test/cors-origins.test.ts | 71 ++++++++++++++++ packages/utils/test/vercel-routing.test.ts | 99 ++++++++++++++++++++++ 16 files changed, 548 insertions(+), 39 deletions(-) create mode 100644 packages/config/test/cors-origins.test.ts create mode 100644 packages/utils/test/vercel-routing.test.ts diff --git a/.env.example b/.env.example index 79b5b55..df68fb8 100644 --- a/.env.example +++ b/.env.example @@ -75,6 +75,12 @@ SUPABASE_ACCESS_TOKEN= # jobs-service, referrals-service and learning-service only. PLATFORM_ADMIN_USER_IDS= +# --- Browser access (every service) --------------------------------------------- +# Origins allowed to call the API from a browser, comma separated: scheme and host only, with no +# path (https://app.example.com). A trailing slash is forgiven; an entry that can never match, such +# as a bare host or a URL with a path, stops the service from starting. Leave empty to allow none. +# CORS_ORIGINS=https://app.example.com,https://admin.example.com + # --- Rate limiting (every service) -------------------------------------------- # At most RATE_LIMIT_MAX requests per client address in each RATE_LIMIT_WINDOW. The window is a # number and a unit between 1 second and 7 days ("30 seconds", "1 minute", "1 hour"). A bare number diff --git a/NEXT.md b/NEXT.md index 0427876..35fc7ab 100644 --- a/NEXT.md +++ b/NEXT.md @@ -15,8 +15,7 @@ describes the attack areas (A to G) lives outside the repo; each task below name ## Queue -2. [ ] **Internal routes and rewrites (A/F).** Secret comparison edge cases on every `/internal/*` route (empty, wrong length, duplicate headers, query string, other methods); `vercel.json` rewrite confusion toward `/internal/*`. -3. [ ] **Env boot fuzz (F).** Each service with each env var missing, empty, garbage: `RATE_LIMIT_MAX=0`, `CORS_ORIGINS` variants, `PLATFORM_ADMIN_USER_IDS` oddities, `CRON_SECRET` short. Fail fast or fail safe, never open. +3. [ ] **Env boot fuzz, rest of it (F).** Done: `RATE_LIMIT_*`, `CORS_ORIGINS`. Still to do: `PLATFORM_ADMIN_USER_IDS` oddities, `SUPABASE_URL` (http, localhost, userinfo), `MESSAGING_DRIVER=memory` on serverless, `STORAGE_DRIVER` combinations, `.env.example` against the env schemas. 4. [ ] **Authorization matrix (A).** Every route of every service against anonymous, other user, other org staff, same org roles, platform admin. Mass assignment on create and update bodies. 5. [ ] **Validation and pagination fuzz (B).** Malformed JSON, wrong types, oversized bodies, huge `limit` and `offset`, odd UUIDs. Always a clean 4xx envelope, never 500. 6. [ ] **Job-source fetchers (D).** SSRF via board token and redirects, response size and time limits, hostile postings (script tags, `javascript:` apply links), cross-company overwrite. @@ -30,6 +29,8 @@ describes the attack areas (A to G) lives outside the repo; each task below name ## Done - [x] **Rate limiter vs real client IP (F)** — `work/break-fix-1`. Found and fixed: a bad `RATE_LIMIT_WINDOW` booted and then 500'd every request, or silently disabled limiting. Checked and held: store is a bounded LRU, exempt routes are by scope not path. Spoofable `X-Forwarded-For` is safe on Vercel (it overwrites the header); noted in `docs/17_break_report.md`. Typecheck, lint, full suite (119 files, 1,730 tests) pass; bundles rebuilt. +- [x] **Internal routes and rewrites (A/F)** — `work/break-fix-1`. Held. Added `packages/utils/test/vercel-routing.test.ts`, which fails if a public route stops resolving to its own function. +- [x] **Env boot fuzz, first half (F)** — `work/break-fix-1`. `CORS_ORIGINS` entries that can never match now stop the boot (trailing slash is forgiven). Rate-limit window done earlier. ## Blocked @@ -39,4 +40,5 @@ describes the attack areas (A to G) lives outside the repo; each task below name - [ ] **Goal line was blank.** The goal in your message was still the placeholder text. I assumed "break the project locally, fix what I find, get ready for the 2026-11-17 launch". Tell me if you want a different focus. - [ ] **Google Fonts at build time.** `next/font/google` made the admin build fail once in CI. The fix is to ship the font files with the repo (`next/font/local`) or use a system font stack. That changes how the site looks or adds font files, so it is your call. +- [ ] **`CORS_ORIGINS=*` in production.** `docs/16` says never, but nothing enforces it. Enforcing it would stop a service from starting if your Vercel settings use `*` today (for example on previews). Tell me whether to enforce it, or whether `*` is fine on previews and only production should refuse it. - [ ] **Pushing branches.** Every push of any branch builds twelve Vercel projects and the free plan allows 100 deployments a day. I will commit locally and push only at clean points, in batches. Say if you want a different rhythm. diff --git a/api/admin/index.js b/api/admin/index.js index 3192aae..9b36c04 100644 --- a/api/admin/index.js +++ b/api/admin/index.js @@ -77434,11 +77434,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -77466,8 +77495,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/analytics/index.js b/api/analytics/index.js index 96b16e9..7ab277f 100644 --- a/api/analytics/index.js +++ b/api/analytics/index.js @@ -86072,11 +86072,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -86104,8 +86133,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/applications/index.js b/api/applications/index.js index b67a07e..d72b325 100644 --- a/api/applications/index.js +++ b/api/applications/index.js @@ -86090,11 +86090,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -86122,8 +86151,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/auth/index.js b/api/auth/index.js index 3248b44..082ab71 100644 --- a/api/auth/index.js +++ b/api/auth/index.js @@ -77608,11 +77608,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -77640,8 +77669,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/jobs/index.js b/api/jobs/index.js index d4454bd..69faa95 100644 --- a/api/jobs/index.js +++ b/api/jobs/index.js @@ -86121,11 +86121,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -86153,8 +86182,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/learning/index.js b/api/learning/index.js index c4a74d2..f1b11cf 100644 --- a/api/learning/index.js +++ b/api/learning/index.js @@ -86148,11 +86148,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -86180,8 +86209,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/matching/index.js b/api/matching/index.js index a6ee174..2c6863f 100644 --- a/api/matching/index.js +++ b/api/matching/index.js @@ -85315,11 +85315,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -85347,8 +85376,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/outreach/index.js b/api/outreach/index.js index 98fad24..4bed2f3 100644 --- a/api/outreach/index.js +++ b/api/outreach/index.js @@ -86104,11 +86104,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -86136,8 +86165,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/profile/index.js b/api/profile/index.js index 03a6ad4..1a354c6 100644 --- a/api/profile/index.js +++ b/api/profile/index.js @@ -120362,11 +120362,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -120394,8 +120423,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/api/referrals/index.js b/api/referrals/index.js index cc0e892..46e27ae 100644 --- a/api/referrals/index.js +++ b/api/referrals/index.js @@ -86166,11 +86166,40 @@ var rateLimitWindowSchema = external_exports.string().refine( message: 'must be a number and a unit between 1 second and 7 days, for example "30 seconds", "1 minute" or "1 hour"' } ); +function normalizeCorsOrigin(entry) { + const text2 = entry.trim(); + let url2; + try { + url2 = new URL(text2); + } catch { + return void 0; + } + if (url2.protocol !== "http:" && url2.protocol !== "https:") return void 0; + if (url2.username !== "" || url2.password !== "" || url2.search !== "" || url2.hash !== "") { + return void 0; + } + if (url2.pathname !== "/") return void 0; + return url2.origin; +} +function splitCorsOrigins(value) { + return value.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); +} +var corsOriginsSchema = external_exports.string().default("").superRefine((value, ctx) => { + if (value.trim() === "*") return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === void 0) { + ctx.addIssue({ + code: "custom", + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com` + }); + } + } +}); var httpEnvSchema = external_exports.object({ HOST: external_exports.string().min(1).default("0.0.0.0"), PORT: external_exports.coerce.number().int().min(1).max(65535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: external_exports.string().default(""), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: external_exports.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default("1 minute"), BODY_LIMIT_BYTES: external_exports.coerce.number().int().min(1024).default(1048576) @@ -86198,8 +86227,7 @@ function loadEnv(service, schema, source = process.env) { function parseCorsOrigins(value) { const trimmed = value.trim(); if (trimmed === "*") return true; - if (trimmed === "") return []; - return trimmed.split(",").map((origin) => origin.trim()).filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed).map(normalizeCorsOrigin).filter((origin) => origin !== void 0); } // packages/observability/src/metrics.ts diff --git a/docs/17_break_report.md b/docs/17_break_report.md index d19e6b6..165b9c9 100644 --- a/docs/17_break_report.md +++ b/docs/17_break_report.md @@ -11,8 +11,12 @@ Work list: `NEXT.md`. | 1 | Medium | Config | `RATE_LIMIT_WINDOW` was an unchecked string. `abc`, `1 parsec`, an empty value or `0` booted fine and then every business request answered 500 (the limiter threw per request). A negative value (`-5 minutes`) and a bare number (`60`, read as 60 ms) booted fine and limited nothing. The same held for `PRIVACY_RATE_LIMIT_WINDOW`. | Fixed: both are validated at boot (`rateLimitWindowSchema`, one second to seven days, ` ` only). Tests in `packages/config/test`. | | 2 | Info | Rate limit | The limiter's in-memory store is a 5,000-entry LRU, so many unique clients cannot grow memory without bound. | Held. | | 3 | Low | Rate limit | Clients are told apart by `request.ip` with `trustProxy: true`. That is the first `X-Forwarded-For` entry, which a client chooses unless the edge overwrites the header. Vercel does overwrite it, so the launch deployment is not affected; behind a proxy that only appends, one client could pick fresh buckets at will, or push others' buckets out of the LRU. | Accepted for launch, documented in `registerRateLimitedRoutes`. Revisit if the API ever moves behind another proxy. | +| 4 | Medium | Config | `CORS_ORIGINS` entries were not checked. `https://app.example.com/` (trailing slash), `app.example.com` (no scheme), a URL with a path, or `null` started the service fine and then matched no browser `Origin`, so every browser call failed with a CORS error and nothing in the logs said why. | Fixed: a trailing slash and upper-case are forgiven (normalized to the exact origin form), and an entry that can never match stops the boot with the entry named. Tests in `packages/config/test/cors-origins.test.ts`. | +| 5 | Low | Config | `CORS_ORIGINS=*` is accepted in production although `docs/16` says never to use it and the config comment says development only. The API authenticates with bearer tokens, not cookies, so a wildcard does not expose a session, but the rule is not enforced. | Open: needs an owner decision, because the live preview may rely on `*` (see `NEXT.md`, "Needs Nikshey"). | ## What was checked and held +- `vercel.json`: every `/v1/...` and `/internal//...` route that each deployed service declares resolves to that service's function, every rewrite destination has a bundle and a function entry, and the service-local `/internal/drain`, `/health` and `/metrics` are not routed at all. New test `packages/utils/test/vercel-routing.test.ts` (it fails when a rewrite is removed). +- Drain and ingest secret checks (`isAuthorizedDrainRequest`): hashed and compared in constant time, no length oracle, empty secret never matches, a duplicate `Authorization` header (joined with a comma by Node) is refused, a secret containing spaces fails closed. - `RATE_LIMIT_MAX` of 0 is refused at boot (`min(1)`). - `/health`, `/metrics` and `/internal/*` are exempt because they are registered outside the limited scope, not by path matching, so no path trick can move a business route into the exempt set. diff --git a/packages/config/src/env.ts b/packages/config/src/env.ts index b4bab47..b3e14bf 100644 --- a/packages/config/src/env.ts +++ b/packages/config/src/env.ts @@ -112,11 +112,60 @@ export const rateLimitWindowSchema = z.string().refine( }, ); +/** + * The form of an allowed origin that a browser's `Origin` header can equal: lower-case scheme and + * host, no path, no trailing slash. `undefined` for text that no browser would ever send, such as a + * host without a scheme, an entry with a path, or `null`. + */ +export function normalizeCorsOrigin(entry: string): string | undefined { + const text = entry.trim(); + let url: URL; + try { + url = new URL(text); + } catch { + return undefined; + } + if (url.protocol !== 'http:' && url.protocol !== 'https:') return undefined; + if (url.username !== '' || url.password !== '' || url.search !== '' || url.hash !== '') { + return undefined; + } + // A single trailing slash is the usual slip and is forgiven; any other path is not an origin. + if (url.pathname !== '/') return undefined; + return url.origin; +} + +function splitCorsOrigins(value: string): string[] { + return value + .split(',') + .map((origin) => origin.trim()) + .filter((origin) => origin.length > 0); +} + +/** + * `CORS_ORIGINS`: empty, `*`, or a comma-separated list of origins. An entry that can never match + * a browser's `Origin` header (`app.example.com`, `https://app.example.com/login`, `null`) is + * refused at boot: it would otherwise start fine and block every browser request. + */ +export const corsOriginsSchema = z + .string() + .default('') + .superRefine((value, ctx) => { + if (value.trim() === '*') return; + for (const entry of splitCorsOrigins(value)) { + if (normalizeCorsOrigin(entry) === undefined) { + ctx.addIssue({ + code: 'custom', + message: `"${entry}" is not an origin; use scheme and host only, such as https://app.example.com`, + }); + } + } + }); + export const httpEnvSchema = z.object({ HOST: z.string().min(1).default('0.0.0.0'), PORT: z.coerce.number().int().min(1).max(65_535), /** Comma-separated list, or `*` in development only. */ - CORS_ORIGINS: z.string().default(''), + CORS_ORIGINS: corsOriginsSchema, RATE_LIMIT_MAX: z.coerce.number().int().min(1).default(300), RATE_LIMIT_WINDOW: rateLimitWindowSchema.default('1 minute'), BODY_LIMIT_BYTES: z.coerce.number().int().min(1024).default(1_048_576), @@ -153,13 +202,11 @@ export function loadEnv( return result.data as z.infer; } -/** `CORS_ORIGINS` → array, with `*` preserved as a wildcard sentinel. */ +/** `CORS_ORIGINS` → array of canonical origins, with `*` preserved as a wildcard sentinel. */ export function parseCorsOrigins(value: string): string[] | true { const trimmed = value.trim(); if (trimmed === '*') return true; - if (trimmed === '') return []; - return trimmed - .split(',') - .map((origin) => origin.trim()) - .filter((origin) => origin.length > 0); + return splitCorsOrigins(trimmed) + .map(normalizeCorsOrigin) + .filter((origin): origin is string => origin !== undefined); } diff --git a/packages/config/test/cors-origins.test.ts b/packages/config/test/cors-origins.test.ts new file mode 100644 index 0000000..69ae099 --- /dev/null +++ b/packages/config/test/cors-origins.test.ts @@ -0,0 +1,71 @@ +import { describe, expect, it } from 'vitest'; +import { + corsOriginsSchema, + httpEnvSchema, + normalizeCorsOrigin, + parseCorsOrigins, +} from '../src/index.js'; + +describe('CORS_ORIGINS', () => { + it.each([ + ['https://app.example.com', 'https://app.example.com'], + ['https://app.example.com/', 'https://app.example.com'], + ['HTTPS://App.Example.COM', 'https://app.example.com'], + ['http://localhost:3000', 'http://localhost:3000'], + ['https://app.example.com:443', 'https://app.example.com'], + [' https://app.example.com ', 'https://app.example.com'], + ])('reads %j as the origin %j', (entry, origin) => { + expect(normalizeCorsOrigin(entry)).toBe(origin); + }); + + // Each of these started a service that then blocked every browser request, or matched nothing. + it.each([ + 'app.example.com', + '//app.example.com', + 'null', + 'https://app.example.com/login', + 'https://app.example.com?x=1', + 'https://app.example.com#top', + 'https://user:pass@app.example.com', + 'ftp://app.example.com', + 'javascript:alert(1)', + 'https://', + '*.example.com', + '/', + '', + ])('refuses the entry %j', (entry) => { + expect(normalizeCorsOrigin(entry)).toBeUndefined(); + }); + + it('turns the setting into canonical origins and keeps the wildcard as a sentinel', () => { + expect(parseCorsOrigins('')).toEqual([]); + expect(parseCorsOrigins(' ')).toEqual([]); + expect(parseCorsOrigins('*')).toBe(true); + expect(parseCorsOrigins(' * ')).toBe(true); + expect(parseCorsOrigins('https://a.example.com/, http://localhost:3000 ,')).toEqual([ + 'https://a.example.com', + 'http://localhost:3000', + ]); + }); + + it('accepts empty, a wildcard on its own, and a clean list; names the bad entry otherwise', () => { + expect(corsOriginsSchema.safeParse('').success).toBe(true); + expect(corsOriginsSchema.safeParse('*').success).toBe(true); + expect(corsOriginsSchema.safeParse('https://a.example.com/,https://b.example.com').success).toBe(true); + const bad = corsOriginsSchema.safeParse('https://a.example.com,app.example.com,https://b.example.com/x'); + expect(bad.success).toBe(false); + const messages = bad.success ? [] : bad.error.issues.map((issue) => issue.message); + expect(messages).toHaveLength(2); + expect(messages[0]).toContain('"app.example.com"'); + expect(messages[1]).toContain('"https://b.example.com/x"'); + }); + + it('refuses a wildcard mixed into a list, where it would be a literal that matches nothing', () => { + expect(corsOriginsSchema.safeParse('https://a.example.com,*').success).toBe(false); + }); + + it('is what the shared HTTP env validates, with an empty default', () => { + expect(httpEnvSchema.parse({ PORT: '4000' }).CORS_ORIGINS).toBe(''); + expect(httpEnvSchema.safeParse({ PORT: '4000', CORS_ORIGINS: 'app.example.com' }).success).toBe(false); + }); +}); diff --git a/packages/utils/test/vercel-routing.test.ts b/packages/utils/test/vercel-routing.test.ts new file mode 100644 index 0000000..481cb0f --- /dev/null +++ b/packages/utils/test/vercel-routing.test.ts @@ -0,0 +1,99 @@ +import { existsSync, readFileSync, readdirSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +const ROOT = fileURLToPath(new URL('../../../', import.meta.url)); + +interface Rewrite { + source: string; + destination: string; +} +const config = JSON.parse(readFileSync(join(ROOT, 'vercel.json'), 'utf8')) as { + functions: Record; + rewrites: Rewrite[]; +}; + +/** What Vercel does with these: `/v1/jobs/:path*` matches `/v1/jobs` and everything below it, first match wins. */ +function matches(source: string, path: string): boolean { + const prefix = source.replace(/\/:path\*$/, ''); + return path === prefix || path.startsWith(`${prefix}/`); +} +function destinationOf(path: string): string | undefined { + return config.rewrites.find((rewrite) => matches(rewrite.source, path))?.destination; +} + +/** Services that are not deployed (no bundle, no rewrite): the connectors service is built and tested only. */ +const NOT_DEPLOYED = new Set(['connectors']); + +const services = readdirSync(join(ROOT, 'services'), { withFileTypes: true }) + .filter((entry) => entry.isDirectory()) + .map((entry) => entry.name) + .filter((name) => !NOT_DEPLOYED.has(name)); + +function sourceFiles(dir: string): string[] { + return readdirSync(dir, { withFileTypes: true }).flatMap((entry) => + entry.isDirectory() + ? sourceFiles(join(dir, entry.name)) + : entry.name.endsWith('.ts') + ? [join(dir, entry.name)] + : [], + ); +} + +/** Every quoted `/v1/...` or `/internal/...` path literal a service's code declares or mentions. */ +function pathsOf(service: string): string[] { + const found = new Set(); + for (const file of sourceFiles(join(ROOT, 'services', service, 'src'))) { + for (const match of readFileSync(file, 'utf8').matchAll(/['"`](\/(?:v1|internal)\/[^'"`\s]*)['"`]/g)) { + const path = match[1] as string; + // Outgoing request templates, doc placeholders and rewrite patterns, not routes. + if (path.includes('${') || path.includes('...') || path.includes(':path*')) continue; + found.add(path); + } + } + return [...found].sort(); +} + +describe('vercel.json routes every public path to the function that serves it', () => { + it('has a function entry and a committed bundle for every rewrite destination, and the reverse', () => { + const destinations = new Set(config.rewrites.map((rewrite) => rewrite.destination)); + const functionNames = Object.keys(config.functions).map((key) => key.replace(/^api\/(.+)\/index\.js$/, '/api/$1')); + expect([...destinations].sort()).toEqual([...functionNames].sort()); + for (const destination of destinations) { + expect(existsSync(join(ROOT, `${destination.slice(1)}/index.js`)), destination).toBe(true); + } + }); + + it('never rewrites to a service that is not deployed', () => { + for (const service of NOT_DEPLOYED) { + expect(config.rewrites.some((rewrite) => rewrite.destination === `/api/${service}`)).toBe(false); + } + }); + + it.each(services)('%s: every public route resolves to its own function', (service) => { + const own = pathsOf(service).filter( + // `/internal/drain` is the service-local path; only `/internal//...` is public. + (path) => path.startsWith('/v1/') && path !== '/v1/' || path.startsWith(`/internal/${service}/`), + ); + const wrong = own + .map((path) => ({ path, destination: destinationOf(path) })) + .filter(({ destination }) => destination !== `/api/${service}`); + expect(wrong).toEqual([]); + }); + + it('finds routes to check in every deployed service (so the extraction cannot quietly go blind)', () => { + for (const service of services) { + const routes = pathsOf(service).filter( + (path) => (path.startsWith('/v1/') && path !== '/v1/') || path.startsWith(`/internal/${service}/`), + ); + expect(routes.length, service).toBeGreaterThan(0); + } + }); + + it('leaves the service-local drain paths unrouted, so only the scheduler-facing path is public', () => { + expect(destinationOf('/internal/drain')).toBeUndefined(); + expect(destinationOf('/health')).toBeUndefined(); + expect(destinationOf('/metrics')).toBeUndefined(); + }); +}); From ecbb721b9bdfb02675a2bdfb1b016f03a69f0aaf Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 12:54:55 +0000 Subject: [PATCH 03/20] fix(auth-kit): key-set outage is a 503, not a 401; require exp and sub When the Supabase key set (JWKS) could not be fetched or read - project paused, network failure, a 5xx, a page that is not a key set - token verification answered 401 on every request. The web app treats a 401 as a dead session and signs the user out, so a short outage logged everyone out instead of showing "try again". Key-set failures now answer 503 UPSTREAM_UNAVAILABLE; a bad token against a healthy key set is still 401. jose treats a missing exp as a token that never expires unless told otherwise, so exp and sub are now required claims. Supabase always sets both. Adds an adversarial token suite (alg none, HS256 key confusion, foreign keys and kids, jwk/jku/x5u headers, swapped payloads, issuer/audience/nbf, junk and 1 MB tokens, hostile claims, user_metadata) - all of which were already refused and now stay refused - and rebuilds the bundles. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1 --- NEXT.md | 3 +- api/admin/index.js | 14 +- api/analytics/index.js | 14 +- api/applications/index.js | 14 +- api/auth/index.js | 14 +- api/jobs/index.js | 14 +- api/learning/index.js | 14 +- api/outreach/index.js | 14 +- api/profile/index.js | 14 +- api/referrals/index.js | 14 +- docs/17_break_report.md | 4 + packages/auth-kit/src/tokens.ts | 25 ++ packages/auth-kit/test/tokens-attacks.test.ts | 258 ++++++++++++++++++ 13 files changed, 406 insertions(+), 10 deletions(-) create mode 100644 packages/auth-kit/test/tokens-attacks.test.ts diff --git a/NEXT.md b/NEXT.md index 35fc7ab..a240bc2 100644 --- a/NEXT.md +++ b/NEXT.md @@ -16,7 +16,7 @@ describes the attack areas (A to G) lives outside the repo; each task below name ## Queue 3. [ ] **Env boot fuzz, rest of it (F).** Done: `RATE_LIMIT_*`, `CORS_ORIGINS`. Still to do: `PLATFORM_ADMIN_USER_IDS` oddities, `SUPABASE_URL` (http, localhost, userinfo), `MESSAGING_DRIVER=memory` on serverless, `STORAGE_DRIVER` combinations, `.env.example` against the env schemas. -4. [ ] **Authorization matrix (A).** Every route of every service against anonymous, other user, other org staff, same org roles, platform admin. Mass assignment on create and update bodies. +4. [ ] **Authorization matrix, runtime part (A).** Token forgery and the ownership checks were attacked and read (see Done). Still to do: run every route against anonymous, other user, other org staff, same-org roles and platform admin as a table-driven test, and mass assignment (extra keys such as `user_id`, `organization_id`, `role`, `status`) on every create and update body; RLS as the `anon`/`authenticated` roles on every table. 5. [ ] **Validation and pagination fuzz (B).** Malformed JSON, wrong types, oversized bodies, huge `limit` and `offset`, odd UUIDs. Always a clean 4xx envelope, never 500. 6. [ ] **Job-source fetchers (D).** SSRF via board token and redirects, response size and time limits, hostile postings (script tags, `javascript:` apply links), cross-company overwrite. 7. [ ] **ReDoS audit (D).** Every regex in `packages/ai`, `services/profile/src`, `services/jobs/src` with adversarial 200 KB inputs. @@ -31,6 +31,7 @@ describes the attack areas (A to G) lives outside the repo; each task below name - [x] **Rate limiter vs real client IP (F)** — `work/break-fix-1`. Found and fixed: a bad `RATE_LIMIT_WINDOW` booted and then 500'd every request, or silently disabled limiting. Checked and held: store is a bounded LRU, exempt routes are by scope not path. Spoofable `X-Forwarded-For` is safe on Vercel (it overwrites the header); noted in `docs/17_break_report.md`. Typecheck, lint, full suite (119 files, 1,730 tests) pass; bundles rebuilt. - [x] **Internal routes and rewrites (A/F)** — `work/break-fix-1`. Held. Added `packages/utils/test/vercel-routing.test.ts`, which fails if a public route stops resolving to its own function. - [x] **Env boot fuzz, first half (F)** — `work/break-fix-1`. `CORS_ORIGINS` entries that can never match now stop the boot (trailing slash is forgiven). Rate-limit window done earlier. +- [x] **Token forgery and ownership checks (A)** — `work/break-fix-1`. 13 forgery vectors held. Fixed: a Supabase key-set outage answered 401 (the web app then signs everyone out), now 503; tokens without `exp` were accepted, now refused. New `packages/auth-kit/test/tokens-attacks.test.ts`. Suite: 122 files, 1,783 tests. ## Blocked diff --git a/api/admin/index.js b/api/admin/index.js index 9b36c04..675dcec 100644 --- a/api/admin/index.js +++ b/api/admin/index.js @@ -67489,14 +67489,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/analytics/index.js b/api/analytics/index.js index 7ab277f..69563e9 100644 --- a/api/analytics/index.js +++ b/api/analytics/index.js @@ -75390,14 +75390,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/applications/index.js b/api/applications/index.js index d72b325..ec15482 100644 --- a/api/applications/index.js +++ b/api/applications/index.js @@ -75416,14 +75416,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/auth/index.js b/api/auth/index.js index 082ab71..13bc817 100644 --- a/api/auth/index.js +++ b/api/auth/index.js @@ -67464,14 +67464,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/jobs/index.js b/api/jobs/index.js index 69faa95..9586f60 100644 --- a/api/jobs/index.js +++ b/api/jobs/index.js @@ -75418,14 +75418,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/learning/index.js b/api/learning/index.js index f1b11cf..5bef3cb 100644 --- a/api/learning/index.js +++ b/api/learning/index.js @@ -75397,14 +75397,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/outreach/index.js b/api/outreach/index.js index 4bed2f3..3e12f03 100644 --- a/api/outreach/index.js +++ b/api/outreach/index.js @@ -75416,14 +75416,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/profile/index.js b/api/profile/index.js index 1a354c6..d42a945 100644 --- a/api/profile/index.js +++ b/api/profile/index.js @@ -109488,14 +109488,26 @@ function createTokenVerifierConfig(env3) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/api/referrals/index.js b/api/referrals/index.js index 46e27ae..a388af0 100644 --- a/api/referrals/index.js +++ b/api/referrals/index.js @@ -75415,14 +75415,26 @@ function createTokenVerifierConfig(env2) { issuer: authBase }; } +function isKeySetUnavailable(cause) { + if (!(cause instanceof errors_exports.JOSEError)) return true; + return cause instanceof errors_exports.JWKSTimeout || cause instanceof errors_exports.JWKSInvalid || cause.constructor === errors_exports.JOSEError; +} async function verifyAccessToken(token, config2) { let payload; try { ({ payload } = await jwtVerify(token, config2.jwks, { issuer: config2.issuer, - audience: "authenticated" + audience: "authenticated", + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ["exp", "sub"] })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError("UPSTREAM_UNAVAILABLE", "Sign-in keys are temporarily unavailable", { + cause + }); + } const message2 = cause instanceof errors_exports.JWTExpired ? "Access token has expired" : "Access token is invalid"; throw new AppError("UNAUTHENTICATED", message2, { cause }); } diff --git a/docs/17_break_report.md b/docs/17_break_report.md index 165b9c9..3a0d00a 100644 --- a/docs/17_break_report.md +++ b/docs/17_break_report.md @@ -13,10 +13,14 @@ Work list: `NEXT.md`. | 3 | Low | Rate limit | Clients are told apart by `request.ip` with `trustProxy: true`. That is the first `X-Forwarded-For` entry, which a client chooses unless the edge overwrites the header. Vercel does overwrite it, so the launch deployment is not affected; behind a proxy that only appends, one client could pick fresh buckets at will, or push others' buckets out of the LRU. | Accepted for launch, documented in `registerRateLimitedRoutes`. Revisit if the API ever moves behind another proxy. | | 4 | Medium | Config | `CORS_ORIGINS` entries were not checked. `https://app.example.com/` (trailing slash), `app.example.com` (no scheme), a URL with a path, or `null` started the service fine and then matched no browser `Origin`, so every browser call failed with a CORS error and nothing in the logs said why. | Fixed: a trailing slash and upper-case are forgiven (normalized to the exact origin form), and an entry that can never match stops the boot with the entry named. Tests in `packages/config/test/cors-origins.test.ts`. | | 5 | Low | Config | `CORS_ORIGINS=*` is accepted in production although `docs/16` says never to use it and the config comment says development only. The API authenticates with bearer tokens, not cookies, so a wildcard does not expose a session, but the rule is not enforced. | Open: needs an owner decision, because the live preview may rely on `*` (see `NEXT.md`, "Needs Nikshey"). | +| 6 | Medium | Auth | When the Supabase key set (JWKS) could not be fetched or read (project paused, network failure, 5xx, a page that is not a key set), token verification answered **401 on every request**. The web app treats a 401 as "this session is dead" and signs the user out, so a short outage or a paused project logged everyone out instead of showing "try again". | Fixed: key-set failures now answer 503 `UPSTREAM_UNAVAILABLE`; a bad token on a healthy key set is still 401. Tests in `packages/auth-kit/test/tokens-attacks.test.ts`. | +| 7 | Low | Auth | `jwtVerify` was called without required claims, and jose treats a missing `exp` as a token that never expires. Exploiting it needs a validly signed token, which Supabase never issues without `exp`, so this is hardening. | Fixed: `exp` and `sub` are required. | ## What was checked and held - `vercel.json`: every `/v1/...` and `/internal//...` route that each deployed service declares resolves to that service's function, every rewrite destination has a bundle and a function entry, and the service-local `/internal/drain`, `/health` and `/metrics` are not routed at all. New test `packages/utils/test/vercel-routing.test.ts` (it fails when a rewrite is removed). - Drain and ingest secret checks (`isAuthorizedDrainRequest`): hashed and compared in constant time, no length oracle, empty secret never matches, a duplicate `Authorization` header (joined with a comma by Node) is refused, a secret containing spaces fails closed. +- Token forgery (new `tokens-attacks.test.ts`, all refused with 401): `alg: none` (with, without and with a junk signature), HS256 signed with the public key as secret (PEM, JWK JSON and raw coordinates), a token signed by another key under the right, an unknown or no `kid`, an embedded `jwk` header, `jku` and `x5u` pointing at an attacker, a payload swapped under a valid signature, wrong issuer (including a trailing slash), wrong or missing audience (string and array), `nbf` in the future, junk and 1 MB tokens, a missing or non-uuid `sub`, an unknown role, a non-uuid organization, and role or organization supplied through `user_metadata` or the top level of the token (ignored). +- Ownership checks read: resumes, messages, applications (owner or same-organization admin/owner, 404 otherwise), recommendations (own, or a member of the caller's organization), contacts (a company's contacts only to someone who applied there), learning metrics and job ingest/sources (platform admins only). All answer 404 for what the caller may not see. - `RATE_LIMIT_MAX` of 0 is refused at boot (`min(1)`). - `/health`, `/metrics` and `/internal/*` are exempt because they are registered outside the limited scope, not by path matching, so no path trick can move a business route into the exempt set. diff --git a/packages/auth-kit/src/tokens.ts b/packages/auth-kit/src/tokens.ts index 488f521..fe2b7fd 100644 --- a/packages/auth-kit/src/tokens.ts +++ b/packages/auth-kit/src/tokens.ts @@ -26,6 +26,23 @@ export function createTokenVerifierConfig(env: TokenVerifierEnvSource): TokenVer }; } +/** + * True when verification failed because the key set could not be obtained or read, not because + * of anything wrong with the token: the endpoint is unreachable, slow, answers with an error, or + * serves something that is not a key set. A project that is paused or briefly down must read as + * "try again" (503), not as "this session is invalid" (401), or every client signs its user out. + * A plain JOSEError (neither a token problem such as a bad signature or an expired claim, nor a + * more specific key-set error) is what jose raises for a non-200 key-set response. + */ +function isKeySetUnavailable(cause: unknown): boolean { + if (!(cause instanceof joseErrors.JOSEError)) return true; + return ( + cause instanceof joseErrors.JWKSTimeout || + cause instanceof joseErrors.JWKSInvalid || + cause.constructor === joseErrors.JOSEError + ); +} + /** Verifies signature, issuer, and expiry, then re-validates the claim shape. */ export async function verifyAccessToken( token: string, @@ -36,8 +53,16 @@ export async function verifyAccessToken( ({ payload } = await jwtVerify(token, config.jwks, { issuer: config.issuer, audience: 'authenticated', + // jose treats a missing `exp` as "never expires" unless told otherwise. Supabase always + // sets both, so a token without them was not issued by it. + requiredClaims: ['exp', 'sub'], })); } catch (cause) { + if (isKeySetUnavailable(cause)) { + throw new AppError('UPSTREAM_UNAVAILABLE', 'Sign-in keys are temporarily unavailable', { + cause, + }); + } const message = cause instanceof joseErrors.JWTExpired ? 'Access token has expired' : 'Access token is invalid'; throw new AppError('UNAUTHENTICATED', message, { cause }); diff --git a/packages/auth-kit/test/tokens-attacks.test.ts b/packages/auth-kit/test/tokens-attacks.test.ts new file mode 100644 index 0000000..7774463 --- /dev/null +++ b/packages/auth-kit/test/tokens-attacks.test.ts @@ -0,0 +1,258 @@ +import { + SignJWT, + base64url, + exportJWK, + exportSPKI, + generateKeyPair, + type CryptoKey, + type JWK, +} from 'jose'; +import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'; +import { AppError } from '@atlas/utils'; + +const SUPABASE_URL = 'https://test-project.supabase.co'; +const ISSUER = `${SUPABASE_URL}/auth/v1`; +const SUB = '11111111-1111-4111-8111-111111111111'; +const SESSION = '22222222-2222-4222-8222-222222222222'; +const ORG = '33333333-3333-4333-8333-333333333333'; + +let signingKey: CryptoKey; +let otherKey: CryptoKey; +let publicJwk: JWK; +let publicPem: string; + +/** What the stubbed JWKS endpoint does on the next fetch. */ +let jwksBehavior: () => Promise; +const serveKeys = () => + Promise.resolve( + new Response(JSON.stringify({ keys: [publicJwk] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + +vi.stubGlobal('fetch', () => jwksBehavior()); + +const { createTokenVerifierConfig, verifyAccessToken } = await import('../src/tokens.js'); + +beforeAll(async () => { + const pair = await generateKeyPair('ES256', { extractable: true }); + signingKey = pair.privateKey; + otherKey = (await generateKeyPair('ES256')).privateKey; + publicJwk = { ...(await exportJWK(pair.publicKey)), kid: 'good-key', alg: 'ES256', use: 'sig' }; + publicPem = await exportSPKI(pair.publicKey); +}); + +beforeEach(() => { + jwksBehavior = serveKeys; +}); + +interface Options { + key?: CryptoKey; + header?: Record; + payload?: Record; + omit?: string[]; + exp?: number | null; + nbf?: number; +} + +async function token(options: Options = {}): Promise { + const now = Math.floor(Date.now() / 1000); + const payload: Record = { + session_id: SESSION, + email: 'ada@example.com', + app_metadata: {}, + ...options.payload, + }; + const jwt = new SignJWT(payload) + .setProtectedHeader({ alg: 'ES256', kid: 'good-key', ...options.header }) + .setIssuer(ISSUER) + .setAudience('authenticated') + .setIssuedAt(); + if (!options.omit?.includes('sub')) jwt.setSubject(SUB); + if (options.exp !== null) jwt.setExpirationTime(options.exp ?? now + 900); + if (options.nbf !== undefined) jwt.setNotBefore(options.nbf); + return jwt.sign(options.key ?? signingKey); +} + +function verify(jwt: string) { + // A fresh config each time: it holds the JWKS cache. + return verifyAccessToken(jwt, createTokenVerifierConfig({ SUPABASE_URL })); +} + +async function status(jwt: string): Promise<{ code: string; httpStatus: number } | 'accepted'> { + try { + await verify(jwt); + return 'accepted'; + } catch (error) { + if (error instanceof AppError) return { code: error.code, httpStatus: error.statusCode }; + throw error; + } +} + +const rejected = { code: 'UNAUTHENTICATED', httpStatus: 401 }; + +describe('forged and tampered tokens are all refused as unauthenticated', () => { + it('accepts a correctly signed token (the control for everything below)', async () => { + expect(await status(await token())).toBe('accepted'); + }); + + it('refuses alg "none", with and without a signature part', async () => { + const header = base64url.encode(JSON.stringify({ alg: 'none', typ: 'JWT' })); + const body = base64url.encode( + JSON.stringify({ + sub: SUB, + session_id: SESSION, + email: 'ada@example.com', + iss: ISSUER, + aud: 'authenticated', + exp: Math.floor(Date.now() / 1000) + 900, + app_metadata: { role: 'owner', org_id: ORG }, + }), + ); + expect(await status(`${header}.${body}.`)).toEqual(rejected); + expect(await status(`${header}.${body}`)).toEqual(rejected); + expect(await status(`${header}.${body}.AAAA`)).toEqual(rejected); + }); + + it('refuses an HS256 token whose secret is the public key (algorithm confusion)', async () => { + const claims = { session_id: SESSION, email: 'ada@example.com', app_metadata: { role: 'owner', org_id: ORG } }; + for (const secret of [publicPem, JSON.stringify(publicJwk), String(publicJwk.x) + String(publicJwk.y)]) { + const forged = await new SignJWT(claims) + .setProtectedHeader({ alg: 'HS256', kid: 'good-key' }) + .setSubject(SUB) + .setIssuer(ISSUER) + .setAudience('authenticated') + .setExpirationTime('15m') + .sign(new TextEncoder().encode(secret)); + expect(await status(forged)).toEqual(rejected); + } + }); + + it('refuses a token signed by another key, whatever kid it claims', async () => { + expect(await status(await token({ key: otherKey }))).toEqual(rejected); + expect(await status(await token({ key: otherKey, header: { kid: 'unknown-key' } }))).toEqual(rejected); + expect(await status(await token({ key: otherKey, header: { kid: undefined } }))).toEqual(rejected); + }); + + it('refuses a token carrying its own key in the header (jwk) or a key URL (jku, x5u)', async () => { + const attackerPublic = { ...(await exportJWK((await generateKeyPair('ES256', { extractable: true })).publicKey)) }; + expect(await status(await token({ key: otherKey, header: { jwk: attackerPublic } }))).toEqual(rejected); + expect( + await status(await token({ key: otherKey, header: { jku: 'https://evil.example/jwks.json' } })), + ).toEqual(rejected); + expect(await status(await token({ key: otherKey, header: { x5u: 'https://evil.example/cert.pem' } }))).toEqual( + rejected, + ); + }); + + it('refuses a payload swapped under a valid signature', async () => { + const [header, , signature] = (await token()).split('.'); + const swapped = base64url.encode( + JSON.stringify({ + sub: SUB, + session_id: SESSION, + email: 'ada@example.com', + iss: ISSUER, + aud: 'authenticated', + exp: Math.floor(Date.now() / 1000) + 900, + app_metadata: { role: 'owner', org_id: ORG }, + }), + ); + expect(await status(`${header}.${swapped}.${signature}`)).toEqual(rejected); + }); + + it('refuses wrong issuer, wrong audience (string and array), and a token that is not yet valid', async () => { + const now = Math.floor(Date.now() / 1000); + const custom = (overrides: (jwt: SignJWT) => SignJWT) => + overrides( + new SignJWT({ session_id: SESSION, email: 'ada@example.com', app_metadata: {} }) + .setProtectedHeader({ alg: 'ES256', kid: 'good-key' }) + .setSubject(SUB) + .setExpirationTime(now + 900), + ).sign(signingKey); + + expect(await status(await custom((jwt) => jwt.setIssuer('https://evil.example/auth/v1').setAudience('authenticated')))).toEqual(rejected); + expect(await status(await custom((jwt) => jwt.setIssuer(`${ISSUER}/`).setAudience('authenticated')))).toEqual(rejected); + expect(await status(await custom((jwt) => jwt.setIssuer(ISSUER).setAudience('anon')))).toEqual(rejected); + expect(await status(await custom((jwt) => jwt.setIssuer(ISSUER).setAudience(['anon', 'service_role'])))).toEqual(rejected); + expect(await status(await custom((jwt) => jwt.setIssuer(ISSUER)))).toEqual(rejected); // no audience at all + expect(await status(await custom((jwt) => jwt.setIssuer(ISSUER).setAudience('authenticated').setNotBefore(now + 3600)))).toEqual(rejected); + }); + + it('refuses garbage that is not a token, including very large input, without throwing anything but a 401', async () => { + for (const junk of ['', ' ', 'x', 'a.b', 'a.b.c', '....', '\u0000', '🔑.🔑.🔑', 'a'.repeat(1_000_000), `${'a'.repeat(2000)}.${'b'.repeat(2000)}.${'c'.repeat(2000)}`]) { + expect(await status(junk)).toEqual(rejected); + } + }); +}); + +describe('claims', () => { + it('refuses a token with no expiry', async () => { + expect(await status(await token({ exp: null }))).toEqual(rejected); + }); + + it('refuses a token with no subject, or a subject that is not a uuid', async () => { + expect(await status(await token({ omit: ['sub'] }))).toEqual(rejected); + const notUuid = new SignJWT({ session_id: SESSION, email: 'ada@example.com', app_metadata: {} }) + .setProtectedHeader({ alg: 'ES256', kid: 'good-key' }) + .setSubject("1'; drop table users;--") + .setIssuer(ISSUER) + .setAudience('authenticated') + .setExpirationTime('15m') + .sign(signingKey); + expect(await status(await notUuid)).toEqual(rejected); + }); + + it('refuses an unknown role, or an org id that is not a uuid, in app_metadata', async () => { + expect(await status(await token({ payload: { app_metadata: { role: 'superuser', org_id: ORG } } }))).toEqual(rejected); + expect(await status(await token({ payload: { app_metadata: { role: 'owner', org_id: 'not-a-uuid' } } }))).toEqual(rejected); + expect(await status(await token({ payload: { app_metadata: { role: ['owner'], org_id: ORG } } }))).toEqual(rejected); + }); + + it('takes role and organization only from app_metadata, never from user_metadata or the top level', async () => { + const claims = await verify( + await token({ + payload: { + role: 'owner', + org_id: ORG, + org: ORG, + user_metadata: { role: 'owner', org_id: ORG }, + app_metadata: {}, + }, + }), + ); + expect(claims.role).toBeNull(); + expect(claims.org).toBeNull(); + }); +}); + +describe('when the key set cannot be fetched', () => { + // A paused or unreachable project is not the caller's fault: answering 401 makes every client think + // its session died and sign the user out, where 503 tells it to try again. + const unavailable = { code: 'UPSTREAM_UNAVAILABLE', httpStatus: 503 }; + + it('answers 503 when the key endpoint cannot be reached', async () => { + jwksBehavior = () => Promise.reject(new TypeError('fetch failed')); + expect(await status(await token())).toEqual(unavailable); + }); + + it('answers 503 when the key endpoint answers with a server error or a non-key body', async () => { + jwksBehavior = () => Promise.resolve(new Response('upstream broke', { status: 502 })); + expect(await status(await token())).toEqual(unavailable); + jwksBehavior = () => Promise.resolve(new Response('paused', { status: 200 })); + expect(await status(await token())).toEqual(unavailable); + jwksBehavior = () => Promise.resolve(new Response('{"keys":"nope"}', { status: 200 })); + expect(await status(await token())).toEqual(unavailable); + }); + + it('still refuses (401) a bad token when the key endpoint is healthy but has no matching key', async () => { + jwksBehavior = () => Promise.resolve(new Response('{"keys":[]}', { status: 200 })); + expect(await status(await token())).toEqual(rejected); + }); + + it('never lets a token through when the key endpoint is down', async () => { + jwksBehavior = () => Promise.reject(new TypeError('fetch failed')); + expect(await status(await token())).not.toBe('accepted'); + }); +}); From 6e8d48d5039e2a5c812ea82bd377d8fe9ca5593a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 12:58:01 +0000 Subject: [PATCH 04/20] fix(jobs): strip NUL, control and bidi characters from board postings A job board that put a NUL byte in a title, location or description made Postgres refuse that posting, so it was counted as rejected. A run with any rejected posting skips the sweep that deactivates closed roles, so one such posting kept every stale posting of that board active and recommended for as long as the board listed it. Control characters (BEL, ESC) and bidirectional overrides (U+202E) were also stored as is. Every text field a mapper produces now passes through stripUnsafeChars (NUL, C0/C1 controls except tab/LF/CR, bidi overrides and isolates). Adds an end-to-end hostile-postings test against Postgres (NUL, script and data links, markup, control characters, oversized fields, malformed entries), notes what held in docs/17_break_report.md, and rebuilds the bundles. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1 --- NEXT.md | 2 +- api/jobs/index.js | 6 +- docs/17_break_report.md | 2 + services/jobs/src/lib/job-sources/common.ts | 18 ++- services/jobs/test/hostile-postings.test.ts | 112 ++++++++++++++++++ services/jobs/test/job-source-mappers.test.ts | 8 ++ 6 files changed, 144 insertions(+), 4 deletions(-) create mode 100644 services/jobs/test/hostile-postings.test.ts diff --git a/NEXT.md b/NEXT.md index a240bc2..c6d42c0 100644 --- a/NEXT.md +++ b/NEXT.md @@ -18,7 +18,6 @@ describes the attack areas (A to G) lives outside the repo; each task below name 3. [ ] **Env boot fuzz, rest of it (F).** Done: `RATE_LIMIT_*`, `CORS_ORIGINS`. Still to do: `PLATFORM_ADMIN_USER_IDS` oddities, `SUPABASE_URL` (http, localhost, userinfo), `MESSAGING_DRIVER=memory` on serverless, `STORAGE_DRIVER` combinations, `.env.example` against the env schemas. 4. [ ] **Authorization matrix, runtime part (A).** Token forgery and the ownership checks were attacked and read (see Done). Still to do: run every route against anonymous, other user, other org staff, same-org roles and platform admin as a table-driven test, and mass assignment (extra keys such as `user_id`, `organization_id`, `role`, `status`) on every create and update body; RLS as the `anon`/`authenticated` roles on every table. 5. [ ] **Validation and pagination fuzz (B).** Malformed JSON, wrong types, oversized bodies, huge `limit` and `offset`, odd UUIDs. Always a clean 4xx envelope, never 500. -6. [ ] **Job-source fetchers (D).** SSRF via board token and redirects, response size and time limits, hostile postings (script tags, `javascript:` apply links), cross-company overwrite. 7. [ ] **ReDoS audit (D).** Every regex in `packages/ai`, `services/profile/src`, `services/jobs/src` with adversarial 200 KB inputs. 8. [ ] **Upload parsers (D).** Hostile PDFs and DOCX (zip bombs, traversal names, truncation), filenames, multipart edge cases. 9. [ ] **Races and idempotency (C).** Parallel application creates, parallel status moves, double drains, double account deletion. @@ -32,6 +31,7 @@ describes the attack areas (A to G) lives outside the repo; each task below name - [x] **Internal routes and rewrites (A/F)** — `work/break-fix-1`. Held. Added `packages/utils/test/vercel-routing.test.ts`, which fails if a public route stops resolving to its own function. - [x] **Env boot fuzz, first half (F)** — `work/break-fix-1`. `CORS_ORIGINS` entries that can never match now stop the boot (trailing slash is forgiven). Rate-limit window done earlier. - [x] **Token forgery and ownership checks (A)** — `work/break-fix-1`. 13 forgery vectors held. Fixed: a Supabase key-set outage answered 401 (the web app then signs everyone out), now 503; tokens without `exp` were accepted, now refused. New `packages/auth-kit/test/tokens-attacks.test.ts`. Suite: 122 files, 1,783 tests. +- [x] **Job-source fetchers and hostile postings (D)** — `work/break-fix-1`. SSRF, redirect, size and time limits all held. Fixed: a NUL byte in a posting made it unstorable, which also blocked the board's closed-role cleanup; control and bidi characters are now stripped. Suite: 123 files, 1,785 tests. ## Blocked diff --git a/api/jobs/index.js b/api/jobs/index.js index 9586f60..a009cf1 100644 --- a/api/jobs/index.js +++ b/api/jobs/index.js @@ -86881,9 +86881,13 @@ var SourcePayloadError = class extends Error { this.name = "SourcePayloadError"; } }; +var UNSAFE_TEXT_CHARS = /[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f-\u009f\u202a-\u202e\u2066-\u2069]/g; +function stripUnsafeChars(text2) { + return text2.replace(UNSAFE_TEXT_CHARS, ""); +} function clampText(value, max) { if (typeof value !== "string") return null; - const trimmed = value.trim(); + const trimmed = stripUnsafeChars(value).trim(); if (trimmed === "") return null; return trimmed.length > max ? trimmed.slice(0, max).trimEnd() : trimmed; } diff --git a/docs/17_break_report.md b/docs/17_break_report.md index 3a0d00a..887a91a 100644 --- a/docs/17_break_report.md +++ b/docs/17_break_report.md @@ -15,6 +15,7 @@ Work list: `NEXT.md`. | 5 | Low | Config | `CORS_ORIGINS=*` is accepted in production although `docs/16` says never to use it and the config comment says development only. The API authenticates with bearer tokens, not cookies, so a wildcard does not expose a session, but the rule is not enforced. | Open: needs an owner decision, because the live preview may rely on `*` (see `NEXT.md`, "Needs Nikshey"). | | 6 | Medium | Auth | When the Supabase key set (JWKS) could not be fetched or read (project paused, network failure, 5xx, a page that is not a key set), token verification answered **401 on every request**. The web app treats a 401 as "this session is dead" and signs the user out, so a short outage or a paused project logged everyone out instead of showing "try again". | Fixed: key-set failures now answer 503 `UPSTREAM_UNAVAILABLE`; a bad token on a healthy key set is still 401. Tests in `packages/auth-kit/test/tokens-attacks.test.ts`. | | 7 | Low | Auth | `jwtVerify` was called without required claims, and jose treats a missing `exp` as a token that never expires. Exploiting it needs a validly signed token, which Supabase never issues without `exp`, so this is hardening. | Fixed: `exp` and `sub` are required. | +| 8 | Medium | Jobs ingestion | A job board that put a NUL byte (`\u0000`) in any title, location or description made Postgres refuse that posting, so it was counted as rejected. Any rejected posting makes the run skip the sweep that deactivates closed roles, so one such posting kept every stale posting of that board active and recommended for as long as the board listed it. The same fields also stored other control characters (BEL, ESC) and bidirectional overrides (U+202E), which garble logs and can make a title read as something else. | Fixed: every posting text field is stripped of NUL, control characters (except tab, line feed, carriage return) and bidi overrides/isolates before it is stored (`stripUnsafeChars` in `job-sources/common.ts`). Tests: `services/jobs/test/hostile-postings.test.ts` (fails without the fix), `job-source-mappers.test.ts`. | ## What was checked and held @@ -22,5 +23,6 @@ Work list: `NEXT.md`. - Drain and ingest secret checks (`isAuthorizedDrainRequest`): hashed and compared in constant time, no length oracle, empty secret never matches, a duplicate `Authorization` header (joined with a comma by Node) is refused, a secret containing spaces fails closed. - Token forgery (new `tokens-attacks.test.ts`, all refused with 401): `alg: none` (with, without and with a junk signature), HS256 signed with the public key as secret (PEM, JWK JSON and raw coordinates), a token signed by another key under the right, an unknown or no `kid`, an embedded `jwk` header, `jku` and `x5u` pointing at an attacker, a payload swapped under a valid signature, wrong issuer (including a trailing slash), wrong or missing audience (string and array), `nbf` in the future, junk and 1 MB tokens, a missing or non-uuid `sub`, an unknown role, a non-uuid organization, and role or organization supplied through `user_metadata` or the top level of the token (ignored). - Ownership checks read: resumes, messages, applications (owner or same-organization admin/owner, 404 otherwise), recommendations (own, or a member of the caller's organization), contacts (a company's contacts only to someone who applied there), learning metrics and job ingest/sources (platform admins only). All answer 404 for what the caller may not see. +- Job-board reading: the request URL is the provider's fixed origin plus a token checked against a pattern (and checked again after the URL is built); redirects are refused; the response is capped at 25 MB by header and by streamed size; the deadline holds even for a `fetch` that ignores the abort signal; error text stored on the source is fixed, never copied from the response. Hostile postings ingested end to end against Postgres: `javascript:` and `data:` apply links are stored as no link, markup in a title is kept as text, a 100 KB title and a 500 KB description are cut to 300 and 200,000 characters, entries with no id or the wrong shape are skipped without failing the board. - `RATE_LIMIT_MAX` of 0 is refused at boot (`min(1)`). - `/health`, `/metrics` and `/internal/*` are exempt because they are registered outside the limited scope, not by path matching, so no path trick can move a business route into the exempt set. diff --git a/services/jobs/src/lib/job-sources/common.ts b/services/jobs/src/lib/job-sources/common.ts index 32ec807..e197c84 100644 --- a/services/jobs/src/lib/job-sources/common.ts +++ b/services/jobs/src/lib/job-sources/common.ts @@ -37,10 +37,24 @@ export class SourcePayloadError extends Error { } } -/** Trimmed, length-capped text; null when nothing is left. */ +/** + * Characters that have no place in a posting and do harm elsewhere: NUL (Postgres cannot store it, + * so the posting would be rejected, and one rejected posting stops a board's closed roles from ever + * being deactivated), the other control characters except tab, line feed and carriage return + * (they garble logs and terminals), and the bidirectional overrides and isolates that reverse the + * text around them, which can make a title read as something else. + */ +// eslint-disable-next-line no-control-regex +const UNSAFE_TEXT_CHARS = /[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f-\u009f\u202a-\u202e\u2066-\u2069]/g; + +export function stripUnsafeChars(text: string): string { + return text.replace(UNSAFE_TEXT_CHARS, ''); +} + +/** Cleaned, trimmed, length-capped text; null when nothing is left. */ export function clampText(value: string | null | undefined, max: number): string | null { if (typeof value !== 'string') return null; - const trimmed = value.trim(); + const trimmed = stripUnsafeChars(value).trim(); if (trimmed === '') return null; return trimmed.length > max ? trimmed.slice(0, max).trimEnd() : trimmed; } diff --git a/services/jobs/test/hostile-postings.test.ts b/services/jobs/test/hostile-postings.test.ts new file mode 100644 index 0000000..b4ef832 --- /dev/null +++ b/services/jobs/test/hostile-postings.test.ts @@ -0,0 +1,112 @@ +import { randomUUID } from 'node:crypto'; +import { eq, inArray } from 'drizzle-orm'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { createDatabase, schema, type DatabaseHandle } from '@atlas/db'; +import type { EventBroker } from '@atlas/messaging'; +import { createSilentLogger } from '@atlas/utils'; +import { runSourceIngestion } from '../src/lib/run-sources.js'; +import { upsert } from '../src/repo/job-sources.js'; + +// DB-backed: self-skips when ATLAS_TEST_DATABASE_URL is unset (see .env.example). +const testDatabaseUrl = process.env.ATLAS_TEST_DATABASE_URL; + +/** + * A job board is somebody else's server, so what it returns is hostile input: the postings land in + * the shared catalog that every user reads. These feed one board a mix of hostile and ordinary + * postings and check that the ordinary ones still arrive and nothing hostile is stored as is. + */ +describe.skipIf(!testDatabaseUrl)('a job board that sends hostile postings', () => { + let dbHandle: DatabaseHandle; + const tag = randomUUID().slice(0, 8); + const companyName = `Hostile Co ${tag}`; + const sourceIds: string[] = []; + + const broker: EventBroker = { + publish: async () => undefined, + subscribe: async (options) => ({ consumer: options.consumer, stop: async () => undefined }), + isHealthy: () => true, + close: async () => undefined, + }; + + beforeAll(() => { + dbHandle = createDatabase({ url: testDatabaseUrl!, maxConnections: 4 }); + }); + + afterAll(async () => { + const [company] = await dbHandle.db.select().from(schema.companies).where(eq(schema.companies.name, companyName)); + if (company) { + await dbHandle.db.delete(schema.jobs).where(eq(schema.jobs.companyId, company.companyId)); + await dbHandle.db.delete(schema.companies).where(eq(schema.companies.companyId, company.companyId)); + } + if (sourceIds.length > 0) { + await dbHandle.db.delete(schema.jobSources).where(inArray(schema.jobSources.sourceId, sourceIds)); + } + await dbHandle.close(); + }); + + async function ingest(jobs: unknown[]) { + const { row } = await upsert(dbHandle.db, { + provider: 'greenhouse', + boardToken: `host-${tag}`, + companyName, + }); + sourceIds.push(row.sourceId); + const fetchImpl: typeof fetch = async () => + new Response(JSON.stringify({ jobs }), { status: 200, headers: { 'content-type': 'application/json' } }); + const report = await runSourceIngestion( + { db: dbHandle.db, broker, logger: createSilentLogger(), fetch: fetchImpl, userAgent: 'AtlasJobIngest/test' }, + { budgetMs: 20_000, sourceIds: [row.sourceId] }, + ); + const stored = await dbHandle.db + .select({ job: schema.jobs }) + .from(schema.jobs) + .innerJoin(schema.companies, eq(schema.jobs.companyId, schema.companies.companyId)) + .where(eq(schema.companies.name, companyName)) + .then((rows) => rows.map((entry) => entry.job)); + return { report, stored }; + } + + it('stores the ordinary postings, drops or cleans the hostile ones, and never stores a NUL or a script link', async () => { + const { report, stored } = await ingest([ + { id: 9001, title: `Good Engineer ${tag}`, content: 'Build services in TypeScript.', absolute_url: 'https://boards.greenhouse.io/x/jobs/9001', location: { name: 'Berlin' } }, + { id: 9002, title: `Nul Title\u0000 ${tag}`, content: 'Fine description', absolute_url: 'https://boards.greenhouse.io/x/jobs/9002' }, + { id: 9003, title: `Nul Body ${tag}`, content: 'Description with a \u0000 inside', location: { name: 'Ber\u0000lin' }, absolute_url: 'https://boards.greenhouse.io/x/jobs/9003' }, + { id: 9004, title: `Script Link ${tag}`, content: 'x', absolute_url: 'javascript:alert(document.cookie)' }, + { id: 9005, title: `Data Link ${tag}`, content: 'x', absolute_url: 'data:text/html,' }, + { id: 9006, title: `Control Chars ${tag} \u0007\u001b[31mred‮evil`, content: 'x​x', absolute_url: 'https://boards.greenhouse.io/x/jobs/9006' }, + { id: 9007, title: `Huge ${tag} ${'x'.repeat(100_000)}`, content: 'y'.repeat(500_000), absolute_url: 'https://boards.greenhouse.io/x/jobs/9007' }, + { id: 9008, title: `Markup ${tag} `, content: '

Real text

', absolute_url: 'https://boards.greenhouse.io/x/jobs/9008' }, + { id: 'not-a-number-but-allowed', title: `String Id ${tag}`, content: 'x', absolute_url: 'https://boards.greenhouse.io/x/jobs/9009' }, + { id: null, title: 'No id at all' }, + 'a string, not an object', + null, + { id: 9010 }, + ]); + + // The run as a whole worked: one hostile posting must not take the board down. + expect(report.failed).toBe(0); + expect(report.succeeded).toBe(1); + expect(stored.map((job) => job.title)).toContain(`Good Engineer ${tag}`); + + // A NUL used to make its posting unstorable, and a single unstorable posting stops the board's + // closed roles from ever being deactivated. It is cleaned instead, so nothing is rejected. + expect(report.rejected).toBe(0); + expect(report.sources[0]?.deactivation_skipped).not.toBe('rejected_postings'); + expect(stored.map((job) => job.title)).toContain(`Nul Title ${tag}`); + expect(stored.find((job) => job.title === `Nul Body ${tag}`)?.location).toBe('Berlin'); + + for (const job of stored) { + const strings = [job.title, job.description, job.location, job.applyUrl].filter((value): value is string => typeof value === 'string'); + for (const text of strings) expect(text.includes('\u0000')).toBe(false); + // Controls other than tab, newline and carriage return, and bidi overrides, are gone. + for (const text of [job.title, job.description ?? '', job.location ?? '']) { + // eslint-disable-next-line no-control-regex + expect(text).not.toMatch(/[\u0000-\u0008\u000b\u000c\u000e-\u001f\u007f-\u009f\u202a-\u202e\u2066-\u2069]/); + } + if (job.applyUrl !== null) expect(job.applyUrl).toMatch(/^https?:\/\//); + expect(job.title.length).toBeLessThanOrEqual(300); + expect((job.description ?? '').length).toBeLessThanOrEqual(200_000); + expect(job.description ?? '').not.toMatch(/, <\s*li[^>]*>) that rescan the rest of the text from every "<". A description of up to 200,000 characters made of "<", "a<", "< ", " took seconds at 50,000 characters (1.5 s for a run of "<", about 25 s at the cap) and grows with the square. The server-side cleaner was written to avoid this and has a test for it; the client had its own, older copy. A hostile or sloppy job board reaches every user through the shared catalog. stripTags is one linear pass: the next ">" is remembered, a "<" with no ">" left ends the search for good, and a script or style block with no closing tag is searched for once, not from every opening tag. It reads ordinary markup exactly as the old chain did (a differential test keeps the old chain beside it, on 18 samples), and tags with attributes such as
now turn into the breaks they stand for. Adds 22 growth tests (eight failed before) and the differential tests. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1 --- NEXT.md | 3 +- .../web/src/components/jobs/job-view-model.ts | 120 ++++++++++++++-- apps/web/test/description-hostile.test.ts | 128 ++++++++++++++++++ docs/17_break_report.md | 1 + 4 files changed, 243 insertions(+), 9 deletions(-) create mode 100644 apps/web/test/description-hostile.test.ts diff --git a/NEXT.md b/NEXT.md index e9a7c2c..7c5d7c3 100644 --- a/NEXT.md +++ b/NEXT.md @@ -18,7 +18,7 @@ describes the attack areas (A to G) lives outside the repo; each task below name 3. [ ] **Env boot fuzz, last part (F).** Done: rate limit, CORS, `SUPABASE_URL`, `CRON_SECRET`, `MESSAGING_DRIVER` on Vercel, `.env.example` drift. Still to do: `STORAGE_DRIVER` combinations (supabase needs URL, key and bucket; local driver on serverless), `DATABASE_URL` oddities (pooler/prepared statements), and a per-service boot with every variable blank. 4. [ ] **Authorization matrix, runtime part (A).** Token forgery and the ownership checks were attacked and read (see Done). Still to do: run every route against anonymous, other user, other org staff, same-org roles and platform admin as a table-driven test, and mass assignment (extra keys such as `user_id`, `organization_id`, `role`, `status`) on every create and update body; RLS as the `anon`/`authenticated` roles on every table. 5. [ ] **Validation fuzz, rest of it (B).** Cursors are done (see Done). Still to do: malformed JSON bodies, wrong types and oversized bodies on every POST, `__proto__` keys, odd UUIDs in paths and query filters (`location=%`, `remote_type`), duplicate query keys (`?limit=1&limit=2`), content-type mismatches, and the error envelope for each. -7. [ ] **ReDoS audit, last part (D).** `packages/ai`, the services' libraries and the roster CSV parser are done. Still to do: the web apps' client-side parsing (the admin console parses the CSV in the browser before upload) and `scripts/`. +7. [ ] **ReDoS audit, last part (D).** `packages/ai`, the services' libraries, the roster CSV parser and the web app's description cleaner are done. Still to do: the admin console's browser-side CSV preflight (`apps/admin/src/lib/import-csv.ts`) and `scripts/`. 8. [ ] **Upload parsers, last part (D).** Still to do: multipart edge cases (no boundary, duplicate or many parts, a field larger than the limit) and a wall-clock limit on PDF text extraction (pdf.js can also be slow without being large). Also: a failed insert after the file was stored leaves the blob behind (cleanup on error). 9. [ ] **Races, rest of it (C).** Applications are done (see Done). Still to do: double drains (each event's effects applied once), double account deletion and deletion during activity, parallel roster imports, concurrent job ingest of the same posting, and `setScheduled` in outreach (unconditional update; only matters if delivery is ever enabled). 10. [ ] **Committed bundles, last flows (G).** Main loop, drains, roster import and account deletion are done (see Done). Still to do: an outreach draft through the outreach bundle (needs a contact from a platform admin) and referrals. @@ -45,6 +45,7 @@ describes the attack areas (A to G) lives outside the repo; each task below name - [x] **Main product loop through the deployed bundles (G)** — `work/break-fix-1`. Resume upload, job ingest, matching drain and recommendations work across four bundles and the outbox; the other consumers' drains behave. Suite: 136 files, 2,180 tests. - [x] **Account deletion and the university roster through the bundles (G)** — `work/break-fix-1`. Found: a deleted account's old token could still upload, leaving a stored file (and a 500). The record now comes first; a deleted account gets 401. Roster import/read/export/funnel and non-member isolation work in the admin bundle. Suite: 136 files, 2,182 tests. - [x] **Browser sweep of signed-out pages, workflows review, outreach race (E/F/C)** — `work/break-fix-1`. Nothing broke: pages render with the API down, guarded pages redirect, the bad-code and open-redirect URLs stay on the site; workflows keep secrets out of scripts; twenty simultaneous identical drafts make one. +- [x] **Browser-side description cleaner (D/E)** — `work/break-fix-1`. The web app's regex chain was quadratic: a 200,000-character job description of `<` froze the tab for about 25 s for every visitor of that job. Replaced with one linear pass; same output as before on ordinary markup. Suite: 137 files, 2,227 tests. ## Blocked diff --git a/apps/web/src/components/jobs/job-view-model.ts b/apps/web/src/components/jobs/job-view-model.ts index 0215b00..e5a8d09 100644 --- a/apps/web/src/components/jobs/job-view-model.ts +++ b/apps/web/src/components/jobs/job-view-model.ts @@ -96,6 +96,117 @@ function decodeEntities(text: string): string { }); } +/** Block-level tags whose closing tag ends a paragraph. */ +const PARAGRAPH_CLOSERS = new Set(['p', 'div', 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'ul', 'ol', 'tr']); + +const WHITESPACE = /\s/; +const isSpaceAt = (text: string, index: number): boolean => WHITESPACE.test(text.charAt(index)); +const isNameCharAt = (text: string, index: number, first: boolean): boolean => { + const code = text.charCodeAt(index) | 0x20; // folds A-Z onto a-z + return (code >= 0x61 && code <= 0x7a) || (!first && text.charCodeAt(index) >= 0x30 && text.charCodeAt(index) <= 0x39); +}; + +interface TagInfo { + closing: boolean; + name: string; + /** Nothing but whitespace follows the name before the final ">" (so `` but not ``). */ + bare: boolean; +} + +/** Reads `<` + `text[start, end)` + `>` as a tag, by index, without copying: whitespace, an optional "/", a name. */ +function readTag(text: string, start: number, end: number): TagInfo { + let k = start; + while (k < end && isSpaceAt(text, k)) k += 1; + const closing = text.charAt(k) === '/' && k < end; + if (closing) { + k += 1; + while (k < end && isSpaceAt(text, k)) k += 1; + } + const nameStart = k; + while (k < end && isNameCharAt(text, k, k === nameStart)) k += 1; + const name = text.slice(nameStart, k).toLowerCase(); + while (k < end && isSpaceAt(text, k)) k += 1; + return { closing, name, bare: k === end }; +} + +function tagToText({ closing, name }: TagInfo): string { + if (closing) { + if (name === 'li') return '\n'; + return PARAGRAPH_CLOSERS.has(name) ? '\n\n' : ''; + } + if (name === 'br') return '\n'; + if (name === 'ul' || name === 'ol') return '\n\n'; + return name === 'li' ? '• ' : ''; +} + +/** + * Removes HTML tags (and ``) rescans the rest of the text + * from every "<", which on a description of 200,000 characters made of "<" or unclosed tags froze the + * tab of everyone who opened the job. Two things keep it linear: a "<" with no ">" anywhere after it + * (or no closing tag for a script) ends the search for good, and the next ">" is remembered. + */ +function stripTags(text: string): string { + const length = text.length; + // The first ">" at or after `cacheFrom` (-1: there is none), remembered so a long run of "<" with a + // ">" far away does not search for it again from every "<". + let cacheFrom = 0; + let cacheGt = -2; // -2: nothing cached yet + const gtAfter = (from: number): number => { + if (cacheGt !== -2 && from >= cacheFrom && (cacheGt === -1 || from <= cacheGt)) return cacheGt; + cacheFrom = from; + cacheGt = text.indexOf('>', from); + return cacheGt; + }; + const closedBlocks = new Set(); // block names (script, style) known to have no closing tag left + + /** The index just past the closing tag of a `c')).toEqual(['ac']); + expect(descriptionToParagraphs('ac')).toEqual(['ac']); + expect(descriptionToParagraphs('aHello').join(' '); + expect(joined).toBe('Hello'); + }); +}); diff --git a/docs/17_break_report.md b/docs/17_break_report.md index fbe5ade..93f7d3f 100644 --- a/docs/17_break_report.md +++ b/docs/17_break_report.md @@ -31,6 +31,7 @@ Work list: `NEXT.md`. | 21 | Medium | Profile | The uploaded file's name was stored as the client sent it. A NUL byte or a name over 400 characters made the insert fail, so the upload answered **500 after the file had already been written to storage** (an orphaned blob each time); an empty name was stored as an empty string, breaking the API's own `min(1)` contract; control characters and a right-to-left override (`resume\u202Egnp.txt` shows as `resumept.png`-style reversed text) were stored and shown back. The storage path itself is built from the user id and content hash, so traversal names never reached the filesystem. | Fixed: `sanitizeFilename` keeps the last path segment, drops control characters and bidi overrides, collapses whitespace, falls back to `resume` for names that say nothing, and cuts to 255 characters keeping a short extension without splitting a character. Tests: `services/profile/test/filename.test.ts` and ten hostile-name uploads in `resume-flow.test.ts` (five failed before). | | 22 | Low | All services | When the database could not be reached or refused the connection (the pooler out of connections, a restart, a network failure), every request answered **500 `INTERNAL_ERROR`**, the same as a bug in our code. A client cannot tell "try again" from "this is broken", and an outage reads as a crash in the logs. | Fixed: connection-class failures (`53300`, `57P01-03`, class `08`, and the driver and socket codes `CONNECTION_CLOSED`, `CONNECT_TIMEOUT`, `ECONNREFUSED`, `ECONNRESET`, ...) answer **503 `UPSTREAM_UNAVAILABLE`** with a fixed message and no driver text. Constraint, syntax and data errors stay 500/409 as before. Tests in `packages/utils/test/utils.test.ts` and over HTTP against a dead port in `body-fuzz.test.ts`. | | 23 | Medium | Profile / privacy | After an account was deleted, its still-valid access token (it lives until it expires, up to an hour) could keep uploading resumes: the file was **written to storage first**, then the insert failed on the missing user and the request answered **500**, leaving a file of a user who had asked for everything to be erased, with nothing pointing at it and no sweeper to find it. Reproduced through the deployed auth and profile bundles: two such files were left behind by the test itself. The same ordering left a file behind whenever the insert failed for any other reason. | Fixed: the record is inserted first and the file stored second; a deleted account (foreign-key violation) is answered **401 "This account no longer exists"** and nothing is written; if storing the file fails the record is removed. Test: `bundles-e2e.test.ts` (account deletion across the auth and profile bundles). | +| 24 | High | Web app | **A job posting could freeze the browser tab of everyone who opened it.** The web app cleans a job description (`descriptionToParagraphs`) with a chain of regular expressions (`<[^>]*>`, ``, `<\s*li[^>]*>`) that rescan the rest of the text from every `<`. A description of up to 200,000 characters made of `<`, `a<`, `< `, `` took seconds at 50,000 characters (1.5 s for a run of `<`, so about 25 s at the cap) and grows with the square. The server-side cleaner had been written to avoid exactly this and has a test for it; the client had its own, older copy. A hostile or sloppy job board reaches every user through the shared catalog (plain-text fields of Lever and Ashby postings arrive uncleaned). | Fixed: one linear pass over the text (`stripTags`): the next `>` is remembered, a `<` with no `>` left ends the search for good, a script or style block with no closing tag is searched for once, not from every opening tag. Same output as the old chain on 18 samples of ordinary markup (a differential test keeps the old chain beside it), and `
` and other tags with attributes now turn into the breaks they stand for. 22 growth tests in `apps/web/test/description-hostile.test.ts` (eight failed before). | ## What was checked and held From 003380cf66944c8dae30a4f17e40f53f5c49efec Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 31 Oct 2026 14:26:03 +0000 Subject: [PATCH 20/20] test(admin): the console's browser-side CSV check stays linear on hostile files Runs the 15 hostile shapes used for the server's roster parser (commas, newlines, bare carriage returns, unclosed and doubled quotes, one enormous cell, many rows and columns, NUL, formula prefixes) against preflightCsv, which reads the chosen file in the browser before upload. Growth is checked at two sizes; all held. Co-Authored-By: Claude Sonnet 5.5 Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1 --- NEXT.md | 2 +- apps/admin/test/csv-hostile.test.ts | 47 +++++++++++++++++++++++++++++ docs/17_break_report.md | 1 + 3 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 apps/admin/test/csv-hostile.test.ts diff --git a/NEXT.md b/NEXT.md index 7c5d7c3..72b9e46 100644 --- a/NEXT.md +++ b/NEXT.md @@ -18,7 +18,7 @@ describes the attack areas (A to G) lives outside the repo; each task below name 3. [ ] **Env boot fuzz, last part (F).** Done: rate limit, CORS, `SUPABASE_URL`, `CRON_SECRET`, `MESSAGING_DRIVER` on Vercel, `.env.example` drift. Still to do: `STORAGE_DRIVER` combinations (supabase needs URL, key and bucket; local driver on serverless), `DATABASE_URL` oddities (pooler/prepared statements), and a per-service boot with every variable blank. 4. [ ] **Authorization matrix, runtime part (A).** Token forgery and the ownership checks were attacked and read (see Done). Still to do: run every route against anonymous, other user, other org staff, same-org roles and platform admin as a table-driven test, and mass assignment (extra keys such as `user_id`, `organization_id`, `role`, `status`) on every create and update body; RLS as the `anon`/`authenticated` roles on every table. 5. [ ] **Validation fuzz, rest of it (B).** Cursors are done (see Done). Still to do: malformed JSON bodies, wrong types and oversized bodies on every POST, `__proto__` keys, odd UUIDs in paths and query filters (`location=%`, `remote_type`), duplicate query keys (`?limit=1&limit=2`), content-type mismatches, and the error envelope for each. -7. [ ] **ReDoS audit, last part (D).** `packages/ai`, the services' libraries, the roster CSV parser and the web app's description cleaner are done. Still to do: the admin console's browser-side CSV preflight (`apps/admin/src/lib/import-csv.ts`) and `scripts/`. +7. [ ] **ReDoS audit, last part (D).** Everything with user-supplied text is done (`packages/ai`, services, both CSV parsers, the web description cleaner). Still to do: `scripts/` (the smoke script parses only responses from our own deployment, so low priority). 8. [ ] **Upload parsers, last part (D).** Still to do: multipart edge cases (no boundary, duplicate or many parts, a field larger than the limit) and a wall-clock limit on PDF text extraction (pdf.js can also be slow without being large). Also: a failed insert after the file was stored leaves the blob behind (cleanup on error). 9. [ ] **Races, rest of it (C).** Applications are done (see Done). Still to do: double drains (each event's effects applied once), double account deletion and deletion during activity, parallel roster imports, concurrent job ingest of the same posting, and `setScheduled` in outreach (unconditional update; only matters if delivery is ever enabled). 10. [ ] **Committed bundles, last flows (G).** Main loop, drains, roster import and account deletion are done (see Done). Still to do: an outreach draft through the outreach bundle (needs a contact from a platform admin) and referrals. diff --git a/apps/admin/test/csv-hostile.test.ts b/apps/admin/test/csv-hostile.test.ts new file mode 100644 index 0000000..8b5ebd4 --- /dev/null +++ b/apps/admin/test/csv-hostile.test.ts @@ -0,0 +1,47 @@ +import { describe, expect, it } from 'vitest'; +import { preflightCsv } from '../src/lib/import-csv'; + +/** + * The console reads the chosen file in the browser before uploading it. Reading must stay linear in + * its size (a quadratic pattern would freeze the tab on a 4 MB file) and must always answer. Each shape runs at two sizes and the growth is + * checked rather than running the full size, so a regression fails fast instead of hanging. + */ +const SMALL = 100_000; +const LARGE = 400_000; +const HEADER = 'email,full_name\n'; +const fill = (unit: string) => (size: number) => unit.repeat(Math.ceil(size / unit.length)).slice(0, size); + +const SHAPES: ReadonlyArray string]> = [ + ['only commas', (size) => HEADER + fill(',')(size)], + ['only newlines', (size) => HEADER + fill('\n')(size)], + ['only carriage returns', (size) => HEADER + fill('\r')(size)], + ['CRLF blank lines', (size) => HEADER + fill('\r\n')(size)], + ['an open quote that never closes', (size) => `${HEADER}"${'a'.repeat(size)}`], + ['quotes inside an unquoted field', (size) => HEADER + fill('a"b,')(size)], + ['doubled quotes inside a quoted field', (size) => `${HEADER}"${fill('""')(size)}"`], + ['alternating quotes and commas', (size) => HEADER + fill('","')(size)], + ['one enormous cell', (size) => `${HEADER}a@b.co,${'x'.repeat(size)}`], + ['many tiny rows', (size) => HEADER + fill('a@b.co,N\n')(size)], + ['many columns in one row', (size) => `${HEADER}${fill('a,')(size)}`], + ['spaces and tabs', (size) => HEADER + fill(' \t')(size)], + ['a BOM repeated', (size) => HEADER + fill('')(size)], + ['NUL bytes', (size) => `${HEADER}a@b.co,${fill('\u0000')(size)}`], + ['formula prefixes', (size) => HEADER + fill('=cmd|\' /C calc\'!A0,+1\n')(size)], +]; + +function runtime(text: string): number { + const start = performance.now(); + // A file the console refuses is fine; what matters is that it answers, and quickly. + const result = preflightCsv(text); + expect(typeof result.ok).toBe('boolean'); + return performance.now() - start; +} + +describe('the browser-side roster CSV check on hostile input', () => { + it.each(SHAPES)('stays linear: %s', (_label, shape) => { + const small = Math.min(runtime(shape(SMALL)), runtime(shape(SMALL))); + const large = Math.min(runtime(shape(LARGE)), runtime(shape(LARGE))); + if (large <= 80) return; + expect(large / Math.max(small, 1), `${small.toFixed(1)} ms at ${SMALL}, ${large.toFixed(1)} ms at ${LARGE}`).toBeLessThan(9); + }); +}); diff --git a/docs/17_break_report.md b/docs/17_break_report.md index 93f7d3f..8ab50d1 100644 --- a/docs/17_break_report.md +++ b/docs/17_break_report.md @@ -56,5 +56,6 @@ Work list: `NEXT.md`. - A real browser (Chromium at phone width) against the built web app with the API and Supabase unreachable: the home, login, register, forgot/reset password, privacy and 404 pages render with no script errors and no sideways scroll; `/dashboard`, `/dashboard/jobs` and `/dashboard/resume` send a signed-out visitor to `/login`; `/auth/callback` with a bad code and `next=//evil.com` ends on `/login?oauth_error=session`, not on another site; `/login?next=//evil.com` and `next=/\evil.com` stay on the login page. Not done: signed-in pages with hostile data (needs a stand-in Supabase for the session). - Scheduled workflows: secrets reach `curl` through environment variables (never spliced into the script), the one user input (`source_id`) is passed through an environment variable and checked against a UUID pattern before use, `permissions: {}`, a concurrency group stops runs piling up, and CI uses `pull_request` (not `pull_request_target`), so fork pull requests get no secrets. Actions are pinned by major tag (`@v4`), not by commit; acceptable here, a hardening choice for later. The repository is public, so Actions minutes are not a constraint on the five-minute drain schedule. - Outreach: twenty simultaneous draft creates with one idempotency key make one draft (new test); marking a draft sent while ten requests race was already covered. +- The admin console's own CSV check, run in the browser before upload, on the same 15 hostile shapes as the server's parser (`apps/admin/test/csv-hostile.test.ts`): linear, and it always answers. - `RATE_LIMIT_MAX` of 0 is refused at boot (`min(1)`). - `/health`, `/metrics` and `/internal/*` are exempt because they are registered outside the limited scope, not by path matching, so no path trick can move a business route into the exempt set.