linkder/SECURITY-FINDINGS.md

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 @linkder/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

• resolveSession discards session.impersonatedBy, so every admin action taken while impersonating is written to audit_log as the victim. The Session type has no field for it, so no procedure can tell. Admin impersonation is enabled (admin() plugin) and is currently invisible to the application. Fix: add impersonatedBy to Session, populate it in resolveSession, and record it on all three audit_log insert sites.
• user.setRole is a check-then-write across two connections; a concurrent job.create permanently strands the job it was meant to protect.
• deck.swipe checks job.status outside its transaction — a concurrent job.cancel loses 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; isPrivateKind is 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.addCredential and job.create accept 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, not secondary-storage (which throws unless all five SecondaryStorage methods exist).

Lower severity

• user.setEmail lets 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 next query param on the sign-in form goes straight to router.push.
• audit_log.ip trusts the leftmost X-Forwarded-For entry verbatim — attacker-controlled.
• findPossibleDuplicates has no call site; the guarantee it was written to provide never executes.
• DeckClient renders a frozen initialCards prop 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.