Skip to content

fix(auth): reject password-reset tokens after they expire#44

Open
clawedassistant26 wants to merge 1 commit into
moshcoder:masterfrom
clawedassistant26:fix/reset-token-expiry
Open

fix(auth): reject password-reset tokens after they expire#44
clawedassistant26 wants to merge 1 commit into
moshcoder:masterfrom
clawedassistant26:fix/reset-token-expiry

Conversation

@clawedassistant26

@clawedassistant26 clawedassistant26 commented Jul 20, 2026

Copy link
Copy Markdown

Bug: password-reset tokens don't actually expire (up to ~24h instead of 1h)

Root cause

lib/db.ts accountForResetToken() gates the token on:

SELECT id FROM accounts WHERE reset_token = ? AND reset_expires > datetime('now')

But reset_expires is written by setResetToken() with an ISO-8601 string from Date#toISOString() (see app/api/auth/reset/route.ts: new Date(Date.now() + 60*60*1000).toISOString()), e.g. 2026-07-20T20:53:43.696Z. SQLite's datetime('now') returns a space-separated form, e.g. 2026-07-20 21:23:43.

The comparison is a lexical string comparison of two different formats. At index 10 the ISO T (0x54) always sorts after the space (0x20), so for any token whose expiry timestamp falls on the current UTC day, reset_expires > datetime('now') is always true regardless of the real time. The 1-hour expiry is defeated — a reset link stays valid for the remainder of the UTC day (up to ~24h).

Reproduction

In-memory libSQL, issue a token that expired 30 minutes ago:

process.env.TURSO_DATABASE_URL = ":memory:";
const { ensureSchema, createAccount, setResetToken, accountForResetToken } = await import("./lib/db.ts");
await ensureSchema();
const acct = await createAccount({ email: "a@b.com", passwordHash: "x", passwordSalt: "y", domain: "ex.com" });
await setResetToken("a@b.com", "TESTTOKEN", new Date(Date.now() - 30*60*1000).toISOString());
console.log(await accountForResetToken("TESTTOKEN")); // -> account id (BUG: expired token accepted)

Before the fix this returns the account id; it should return null.

Fix

Compare against an ISO "now" so it is a like-for-like comparison (ISO-8601 UTC strings sort chronologically):

... AND reset_expires > ?     -- args: [token, new Date().toISOString()]

reset_expires is only ever written as an ISO string (or NULL), so ISO-vs-ISO is correct and self-consistent. Minimal, one-query change.

Test

Adds tests/reset-token-expiry.test.mjs (in-memory libSQL): a future token is accepted, an already-expired token is rejected. It fails on the current code and passes with the fix.

Suite

node --experimental-strip-types --test tests/*.test.mjs -> 21 tests, 21 pass, 0 fail (20 existing + 1 new).

accountForResetToken compared reset_expires against SQLite's
datetime('now') as a string. reset_expires is written by setResetToken
as an ISO-8601 timestamp (Date#toISOString, e.g. "2026-07-20T20:53:43.696Z"),
whereas datetime('now') returns a space-separated form
("2026-07-20 21:23:43"). Lexically the ISO "T" separator (0x54) always
sorts after the space (0x20), so any token whose expiry falls on the
current UTC day read as still valid — defeating the intended 1-hour
expiry and keeping reset links usable for up to ~24h.

Compare against an ISO "now" (new Date().toISOString()) so it is a
like-for-like comparison; ISO-8601 UTC strings sort chronologically.

Adds a regression test (in-memory libSQL) asserting an expired token is
rejected while a future one is accepted.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant