The showcase was a Barcelona market: Catalan names, +34 numbers, euro rates and "Carrer Example 12" on every job. Presented to a Mexican client, all of that reads as somebody else's product. City comes from NEXT_PUBLIC_CITY_* as before, now Ciudad de México at 19.4326/-99.1332, with MAPBOX_COUNTRY=mx. The seed's fallbacks were Barcelona literals, so an unset env quietly seeded a different city than the app rendered — they now agree. Two db tests pinned the Barcelona centre as a hardcoded constant, which is why the deck returned zero cards on the first run here: every pro was a continent outside the radius. They read the same env as the seed now, so the trap cannot recur. Money: formatCents defaults to USD/en-US, and the nine hardcoded euro signs across the card, search rows, quote strip and forms are dollars. The rate NUMBERS are unchanged and still read high for CDMX — that is a pricing decision, not a currency one, and is left alone deliberately. Seed people are Mexican, addressed on real Roma/Condesa streets rotated by index rather than one placeholder repeated. Phones moved to +52 55, which moves the demo login to +525500000000 / 000000. Also in here, from the same session: - Sending a job now confirms. The mutation always succeeded; the sheet just closed with no receipt, which from the customer's side is indistinguishable from a dead button. Dismissing that receipt resolves as 'sent', so the card does not return to the deck. - Media moves to DigitalOcean Spaces, with the public origin derived from bucket and region instead of a second env var to keep in sync. - Managed-Postgres TLS: DATABASE_CA_CERT takes a path or inline PEM. - The client-facing project panel beside the running app. - Two profiles removed and four renamed to match their photos. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
4.8 KiB
4.8 KiB
Open security findings (M1 auth review)
An adversarial review of the M1 auth, session and authorization code produced 28 confirmed findings (~20 after deduplication). The ones fixed are listed first so nobody re-reports them; the rest are open and are the M1 exit criteria.
Fixed
| # | Issue | Fix |
|---|---|---|
| 1 | pro.upsertProfile computed requiresReReview and never applied it — a verified plumber could silently become a verified electrician 40 km away |
Demotes a verified profile to pending in the same transaction, writes an audit row, and now counts a trade change as material (it did not before). Needed a new verified -> pending edge in VERIFICATION_GRAPH |
| 2 | phoneNumber() registers /phone-number/request-password-reset, /phone-number/reset-password and /sign-in/phone-number unconditionally — not gated on emailAndPassword.enabled: false. Together they mint a password credential with no SMS sent to the owner, then accept it forever with no OTP |
All three served as 404 via disabledPaths (checked in onRequest, before rate limiting) |
| 3 | bearer() accepted the raw plaintext sessions.token column as an Authorization: Bearer credential — one leaked DB row is a replayable login |
Plugin removed. The mobile client it was for does not exist yet |
| 4 | Phone numbers stored exactly as typed, so +34600111222 and 0034600111222 are two "unique" accounts — defeating the UNIQUE constraint, bans, and duplicate detection |
phoneNumberValidator pins E.164 on send-otp and sign-in; toE164/isE164 added to @linkdr/shared |
| 5 | NEXT_PUBLIC_APP_URL fell back to http://localhost:3000, which drops Secure and the __Secure- prefix from the production session cookie |
Throws at boot in production |
Open — must close before M1 ships
Authorization / attribution
resolveSessiondiscardssession.impersonatedBy, so every admin action taken while impersonating is written toaudit_logas the victim. TheSessiontype has no field for it, so no procedure can tell. Admin impersonation is enabled (admin()plugin) and is currently invisible to the application. Fix: addimpersonatedBytoSession, populate it inresolveSession, and record it on all threeaudit_loginsert sites.user.setRoleis a check-then-write across two connections; a concurrentjob.createpermanently strands the job it was meant to protect.deck.swipechecksjob.statusoutside its transaction — a concurrentjob.cancelloses the race and a pending request lands on a cancelled job.deck.undo's "already reached the pro" guard is check-then-write: it deletes the tombstone for a request that really was sent, and reports success.
Storage / uploads
- Government ID and insurance scans go to the public bucket;
isPrivateKindis never called from production code. - Presigned PUT does not pin
Content-Type, so a caller can store HTML under an image key in a public bucket. pro.addMedia,pro.addCredentialandjob.createaccept any URL the caller sends, with no binding to an object that caller actually uploaded — which bypasses the presign design entirely.
Abuse / spend
- No per-number cap, destination allowlist, or global ceiling on
/phone-number/send-otp. SMS spend is unmetered. - better-auth rate limiting is per-process in-memory and collapses to one global
bucket behind a proxy chain — 10 req/min then locks out the whole platform.
Needs the Redis-backed
rateLimit.customStorage, notsecondary-storage(which throws unless all fiveSecondaryStoragemethods exist).
Lower severity
user.setEmaillets any authenticated user claim an arbitrary unverified address, locking the real owner out of Google sign-up and leaking which addresses are registered.- Open redirect: the
nextquery param on the sign-in form goes straight torouter.push. audit_log.iptrusts the leftmostX-Forwarded-Forentry verbatim — attacker-controlled.findPossibleDuplicateshas no call site; the guarantee it was written to provide never executes.DeckClientrenders a frozeninitialCardsprop and invalidates a query nothing subscribes to, capping the deck at 20 cards per page load.- A right swipe after a left swipe creates a live request while the stored swipe row still records a rejection.
Note on the OTP-in-plaintext decision
apps/web/src/lib/auth.ts argues the plaintext OTP is acceptable partly because
"anyone who can read that table can already read session.token". With
bearer() removed that argument is weaker than when it was written — a session
token now requires an httpOnly browser cookie to use, an OTP does not. The
decision still stands (300s window, 3 attempts, rate limited) but revisit it if
OTP lifetime or attempt budget ever increases.