Repository navigation
Break-and-fix pass: launch blocker in PDF parsing, races, config and input hardening - #10
Draft
NiksheyYadav wants to merge 20 commits into
Draft
NiksheyYadav wants to merge 20 commits into
NiksheyYadav wants to merge 20 commits into
Conversation
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 "<number> <unit>".
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
…uting 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
parseResumeText matched emails with open-ended quantifiers. On a long run of local-part characters with no "@" (a resume that is one long word, or "aaaa...", "----...", "1111...", "2020-2020-...") it rescanned from every start position, which is quadratic. A 200,000-character text/plain upload, the extraction cap, kept the function busy for minutes while the parse runs inside the upload request, so one authenticated request could pin an instance well past the 30 s limit. The same parse backs the optimize endpoint. The local part, domain and top-level domain are now bounded (64, 255, 24). Every 200,000-character shape in the new sweep parses in under 100 ms. Adds packages/ai/test/adversarial-input.test.ts: 37 hostile shapes through parse, score, skill extraction and job analysis, checking growth at two sizes rather than running the full size, so a regression fails in about two minutes instead of hanging the run. Rebuilds the profile bundle. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
The DOCX path had a zip-bomb guard; the PDF path had none. A page content stream of Flate data inflates about 1,000 to 1: a 299 KB PDF made pdf.js spend 3.1 s and about 670 MB, and the upload limit allows a file that inflates to roughly 1 GB, past a function's memory, inside the upload request. assertSafePdfStreams now inflates every stream that opens with a zlib header, with maxOutputLength set, before the file reaches pdf.js. One stream may inflate to 32 MB and all of them together to 128 MB; more is a 413 "too large or complex". The scan reads raw bytes and ignores the (attacker-controlled) stream dictionaries, and leaves damaged streams to pdf.js. A 300 MB bomb is now refused in 26 ms and a 1 GB one in 152 ms. Rebuilds the profile bundle. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
… concurrency Status moves were read, validated, then written with an unconditional UPDATE by id. Two requests that read the same status both passed validation and both wrote: 20 simultaneous "queued" requests all answered 200 and left 20 events, 15 simultaneous "submitted" requests published application.submitted 15 times, and with "offer" and "rejected" sent together an application was recorded as interviewing -> offer after it had been rejected (a terminal status). appendEvent now updates only while the application is still in the status the move was validated against, writes the trail row in the same transaction, and returns null otherwise; the route answers 409 CONFLICT and publishes nothing. Thirty identical creates at once (same idempotency key, same body) also answered 409 for twelve of them, because the winner's row was invisible to the key lookup but visible to the "already applied" check. A row with the same key and request found there is now a replay (200). Adds applications-races.test.ts (parallel creates, parallel moves, a terminal status under conflicting moves, duplicate publishes) and rebuilds the bundles. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
The page cursor is opaque, but any string was accepted. Valid JSON that was
not a cursor the API issued went to the database as a timestamp or a uuid:
{"createdAt":"not-a-date","id":"..."} (and 2026-02-31, which Date.parse
accepts but Postgres refuses) answered 500 on applications, jobs,
recommendations, resumes, messages and a company's contacts, while other
malformed shapes were silently treated as the first page. Only the admin
roster validated its cursor.
parseTimeCursor and parseScoreCursor in @atlas/utils accept exactly what
encodeCursor issues (a real calendar date and clock time, uuid ids, finite
bounded scores) and answer 400 VALIDATION_ERROR "Invalid cursor" for
anything else. Every list query uses them, including the roster.
Adds 42 unit cases, six forged-cursor cases over HTTP (all failed before),
and a test that six simultaneous deletions of one account all succeed with
one tombstone and no leftover rows. Rebuilds the bundles.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
SUPABASE_URL was any URL. With the trailing slash the dashboard shows, the expected issuer became "<url>//auth/v1", which no token carries, so the service started and then refused every request; http:// and credentials in the URL were accepted too. It is now reduced to its origin, and anything that is not https (http only for localhost) stops the boot. MESSAGING_DRIVER=memory, the value .env.example shows for local work, would silently drop every cross-service event on Vercel (each function is its own process). It is no longer honoured there; the durable outbox is used. CRON_SECRET kept a pasted trailing newline, so the scheduler's header could never match; surrounding whitespace is now dropped and whitespace inside is refused, through one schema shared by the five services that take it. .env.example now lists every variable a service reads, and a test keeps it that way. Adds a 76-case request body / query / path-id fuzz for the applications service (all held). Rebuilds the bundles. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
apps/web sent no security headers. The dashboard, which holds resumes and an account-deletion button, could be framed from another site, content-type sniffing was on, full addresses (job and application ids) went to other sites in the Referer, and X-Powered-By named the framework. Pages now carry X-Frame-Options: DENY with frame-ancestors 'none', nosniff, Referrer-Policy: strict-origin-when-cross-origin and a Permissions-Policy that switches off camera, microphone, location, payment and USB; the fingerprinted bundles keep their immutable cache headers (checked on a running build). X-Powered-By is off in both apps, and the admin console gets the frame-ancestors and Permissions-Policy lines too. A full Content-Security-Policy is left for later: Next's inline scripts need a nonce setup. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
pdf.js runs its parser as a worker module that it loads by file name, next
to the code that asks for it. Inside api/profile/index.js that is
api/profile/pdf.worker.mjs, which does not exist: a bundler cannot inline a
file loaded by name and vercel.json includes only the canvas addon. Every
PDF upload ended as status "failed" ("We could not read this file"), the
most common resume format. Text and DOCX worked, and every test passed,
because the tests run the TypeScript sources, where the worker is found
inside node_modules.
The worker is now handed to pdf.js as the data URL that pdf-parse ships
(PDFParse.setWorker(getData()) from pdf-parse/worker), which needs no file,
in the bundle and in the sources alike. The profile bundle grows from about
5.5 MB to 9.2 MB.
Adds packages/auth-kit/test/bundles-e2e.test.ts, which boots all ten
committed api/*/index.js bundles as Vercel would (default-exported handler,
no listening socket) behind real HTTP, against Postgres and a stand-in key
server, and exercises auth, jobs (admin-only ingest, listing, scheduler
secret), profile (text, PDF and DOCX upload, optimize, a PDF bomb),
applications (create, replay, lifecycle) and cross-user isolation. The PDF
upload failed in it before this fix.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
…tay out The logger redacted only a handful of exact paths. The same secrets one level over (headers.authorization, request.headers["x-api-key"], *.secret, *.token) and a resume's parsed_text would have been logged in full. redact(), maskEmail() and maskToken() are used nowhere although their comment said the shared logger applied them. The redaction paths now cover those shapes, createLogger takes an optional destination so a test can read what was written, and the helper's comment says what it is. New logger tests check each shape, that ordinary fields still log, and that a user-supplied line break or ANSI escape stays inside one JSON line. Rebuilds the bundles. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
The file's name was stored as the client sent it. A NUL byte, or a name over 400 characters, made the insert fail: the upload answered 500 after the file had already been written to storage, leaving an orphaned blob each time. An empty name was stored as an empty string, which breaks the API's own min(1) contract, and control characters and a right-to-left override (resume<U+202E>gnp.txt) were stored and shown back to the user. 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. The storage path was never built from the name. Adds unit tests, ten hostile-name uploads in resume-flow.test.ts (five failed before), and a 15-shape growth test for the roster CSV parser (all held). Rebuilds the profile bundle. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
… sign-in The bundle test now reads the route table of each booted bundle (Fastify's printed tree, not a list someone keeps up to date) and sends every route other than /health and /metrics, anonymously and with a forged token, expecting 401. All ten bundles pass; a new route that forgets its authentication check fails the test. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
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. toAppError now maps connection-class failures (Postgres 53300, 57P01-03 and class 08, and the driver and socket codes CONNECTION_CLOSED, CONNECT_TIMEOUT, ECONNREFUSED, ECONNRESET, ...) to 503 UPSTREAM_UNAVAILABLE with a fixed message and no driver text. Constraint, syntax and data errors are unchanged. All ten services route unhandled errors through it. The four consumer entrypoint tests that expected a 500 from a drain with no database now expect the 503 (still a failure for Vercel Cron to record). Rebuilds the bundles. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
…eployed bundles The bundle test now runs the product's main loop across four deployed functions and the Postgres outbox: a resume is uploaded (profile), a job is ingested (jobs), the matching drain route processes the queued events, and the job comes back from /v1/jobs/recommendations with a positive score and reasons naming the matched skills. The learning, analytics and referrals drain routes refuse a missing or wrong secret and drain cleanly with the right one. Resumes are uploaded so that the text resume is the latest parsed one: matching scores against the latest parsed resume. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
… second After an account was deleted, its still-valid access token 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. The same order left a file behind whenever the insert failed for any other reason. The record is now 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 so no row sits "pending" for good. The bundle test now also drives the university roster (import, read, export, funnel, non-member 404) through the admin bundle and account deletion across the auth and profile bundles (rows, stored file, the old token afterwards). Rebuilds the profile bundle. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
… the sweeps Adds a parallel-create test to the outreach flow (one 201, the rest 200, one row, no errors). Updates the break report with the browser sweep of the signed-out web pages (render with the API down, guarded pages redirect, the bad-code and open-redirect URLs stay on the site), the review of the scheduled workflows, and NEXT.md with the note that the repository is public and the report describes weaknesses that stay live until the PR is deployed. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
…r tab The web app cleans a job description with a chain of regular expressions (<[^>]*>, <script>...?</script>, <\s*li[^>]*>) that rescan the rest of the text from every "<". A description of up to 200,000 characters made of "<", "a<", "< ", "<li", "</", "<!--" or unclosed <style> 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 <br class="x"> 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
…tile 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1
This branch had an error being deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Local-only attack pass over the launch branch (#8): in-process apps, a private Postgres, the committed
api/*/index.jsbundles booted over real HTTP, and the built web app in a real browser. Nothing was run against Vercel, Supabase or any live system. Each finding has a failing test before the fix and a passing one after. The full list with severities is indocs/17_break_report.md(24 findings); the working list isNEXT.md.This PR targets
claude/blissful-darwin-2n33wv, notmain, because that branch is not merged yet.Fixed
api/profile/pdf.worker.mjsdoes not exist. Text and DOCX worked and all source-level tests passed. Found by the new test that boots the committed bundles. The worker is now supplied as the data URLpdf-parseships.<: a 200,000-character description took about 25 s. Replaced with one linear pass; same output on ordinary markup.queuedrequests all succeeded,application.submittedpublished 15 times, and a rejected application could becomeoffer. Now a conditional update (409 for the loser). Identical simultaneous creates no longer answer 409.expare refused.SUPABASE_URLwith a trailing slash refused every token;RATE_LIMIT_WINDOWtypos gave 500s or disabled limiting; unmatchedCORS_ORIGINSentries blocked every browser call;MESSAGING_DRIVER=memory(the.env.examplevalue) would drop events on Vercel;CRON_SECRETwith a pasted newline could never match. All normalized or refused at boot with a message naming the variable..env.examplenow lists every variable and a test keeps it so.Held (details in the report)
Token forgery (13 vectors), ownership checks, authentication on every route of every deployed bundle, SSRF and redirects in job-board reading, internal route secrets,
vercel.jsonrouting, request body / query / path fuzz, simultaneous account deletions and draft creates, the resume-to-recommendation loop and the university roster flow through the real bundles, both roster CSV parsers, secrets in git history and bundles, bundles byte-for-byte repeatable, signed-out web pages in a real browser with the backend down, scheduled workflows.Verification
pnpm typecheck,eslint, and the full suite against Postgres 16: 138 files, 2,242 tests pass (from 118 files / 1,704 tests on the launch branch).apps/webandapps/adminbuild; headers checked on a running build.node scripts/bundle-backend.mjsrun after each source change; the committed bundles are current. CI was green on the first push of this branch; the latest push is running.Needs the owner (also in
NEXT.md, "Needs Nikshey")SUPABASE_URL,MESSAGING_DRIVER,CRON_SECRET,CORS_ORIGINS. A wrong value now stops the service with a message naming the variable.CORS_ORIGINS=*should be refused in production (docs/16says never; nothing enforces it).docs/17_break_report.mddescribes weaknesses that stay live on the deployed site until this is merged and deployed (mostly availability problems that need a signed-in account, plus the browser freeze above). Merge soon, or move that file out of the public history; no history was rewritten.next/font/googleflaked CI once).🤖 Generated with Claude Code
https://claude.ai/code/session_01BdqVHQRXj76uudaef6gTD1