bind a follower's confirm and stop link to their address so a spent link cannot stop a reused id (v3.1.27)

@@ -1,6 +1,6 @@11 {22 	"name": "kepr",3-	"version": "3.1.26",3+	"version": "3.1.27",44 	"private": true,55 	"type": "module",66 	"scripts": {

@@ -23,6 +23,7 @@2323 	ipHash: string,2424 	now: number,2525 ): boolean {26+	const address = email.toLowerCase();2627 	const row = kepr2728 		.query<{ id: number }, [number, string, string, number]>(2829 			`INSERT INTO issue_followers (issue_id, email, ip_hash, created_at)@@ -30,12 +31,12 @@3031 ON CONFLICT (issue_id, email) DO NOTHING3132 RETURNING id`,3233 		)33-		.get(issue.id, email.toLowerCase(), ipHash, now);34+		.get(issue.id, address, ipHash, now);3435 	if (!row) return false;35-	const token = followToken(kepr, row.id);36+	const token = followToken(kepr, row.id, address);3637 	const base = `/${repo}/issues/${issue.number}`;3738 	queueEmail(kepr, {38-		to: email.toLowerCase(),39+		to: address,3940 		subject: `Confirm email updates for ${repo} #${issue.number}`,4041 		href: base,4142 		text: `Someone asked for an email whenever "${issue.title}" (${repo} #${issue.number})@@ -50,9 +51,33 @@5051 	return true;5152 }5253 54+/** The address holding a follower link, or null when the id holds no follower of that issue. */55+function followerEmail(kepr: Database, issueId: number, id: number): string | null {56+	const row = kepr57+		.query<{ email: string }, [number, number]>(58+			"SELECT email FROM issue_followers WHERE id = ? AND issue_id = ?",59+		)60+		.get(id, issueId);61+	return row?.email ?? null;62+}63+64+/**65+ * True when a link's token is the one this follower of this issue was sent (§23). Read through the66+ * row, so a spent link fails once its row is gone rather than reaching whatever holds its id now.67+ */68+export function followerLinkMatches(69+	kepr: Database,70+	issueId: number,71+	id: number,72+	token: string,73+): boolean {74+	const email = followerEmail(kepr, issueId, id);75+	return email !== null && followTokenMatches(kepr, id, email, token);76+}77+5378 /** Confirms one follower of an issue when the link's token is theirs. */5479 export function confirmFollower(kepr: Database, issueId: number, id: number, token: string) {55-	if (!followTokenMatches(kepr, id, token)) return false;80+	if (!followerLinkMatches(kepr, issueId, id, token)) return false;5681 	const changed = kepr5782 		.query("UPDATE issue_followers SET confirmed = 1 WHERE id = ? AND issue_id = ?")5883 		.run(id, issueId).changes;@@ -61,7 +86,7 @@6186 6287 /** Stops one follower's emails about an issue when the link's token is theirs. */6388 export function unfollow(kepr: Database, issueId: number, id: number, token: string): boolean {64-	if (!followTokenMatches(kepr, id, token)) return false;89+	if (!followerLinkMatches(kepr, issueId, id, token)) return false;6590 	const changed = kepr6691 		.query("DELETE FROM issue_followers WHERE id = ? AND issue_id = ?")6792 		.run(id, issueId).changes;@@ -106,7 +131,7 @@106131 		.all(issue.id, except.toLowerCase());107132 	kepr.transaction(() => {108133 		for (const follower of followers) {109-			const token = followToken(kepr, follower.id);134+			const token = followToken(kepr, follower.id, follower.email);110135 			const stop = `/${repo}/issues/${issue.number}/unfollow?f=${follower.id}&t=${token}`;111136 			queueEmail(kepr, {112137 				to: follower.email,

@@ -31,6 +31,7 @@3131 import {3232 	confirmFollower,3333 	follow,34+	followerLinkMatches,3435 	followsSince,3536 	isEmail,3637 	tellFollowers,@@ -39,7 +40,6 @@3940 import {4041 	checkStamp,4142 	FILINGS_PER_HOUR,42-	followTokenMatches,4343 	formStamp,4444 	ipHash,4545 	MAX_CHARS,@@ -353,7 +353,9 @@353353 		const id = Number(c.req.query("f"));354354 		const token = c.req.query("t") ?? "";355355 		privatePage(c);356-		if (!followTokenMatches(ctx.kepr, id, token)) return c.redirect(`${base}?notice=expired`, 303);356+		if (!followerLinkMatches(ctx.kepr, found.issue.id, id, token)) {357+			return c.redirect(`${base}?notice=expired`, 303);358+		}357359 		const query = new URLSearchParams({ f: String(id), t: token });358360 		return c.html(359361 			page(ctx, {

@@ -31,13 +31,22 @@3131 	return mac(db, `ip:${clientIp(c)}`);3232 }3333 34-/** What a follower's confirm and stop links carry (§23): only their inbox ever sees it. */35-export function followToken(db: Database, followerId: number): string {36-	return mac(db, `follow:${followerId}`);34+/**35+ * What a follower's confirm and stop links carry (§23): only their inbox ever sees it. The36+ * address is covered too, so a link whose row is gone cannot reach a later follower that reuses37+ * the id — `issue_followers.id` is a rowid, and SQLite hands a deleted row's id back out.38+ */39+export function followToken(db: Database, followerId: number, email: string): string {40+	return mac(db, `follow:${followerId}:${email.toLowerCase()}`);3741 }3842 39-export function followTokenMatches(db: Database, followerId: number, token: string): boolean {40-	const expected = Buffer.from(followToken(db, followerId));43+export function followTokenMatches(44+	db: Database,45+	followerId: number,46+	email: string,47+	token: string,48+): boolean {49+	const expected = Buffer.from(followToken(db, followerId, email));4150 	const given = Buffer.from(token);4251 	return given.length === expected.length && timingSafeEqual(given, expected);4352 }

@@ -155,4 +155,28 @@155155 		});156156 		expect(bad.status).toBe(400);157157 	});158+159+	test("a spent link cannot stop a follower that reuses its id", async () => {160+		// A follower's id is a rowid, so SQLite hands a stopped follower's id to the next one.161+		const gone = new Browser(inst.app, "198.51.100.70");162+		await send(gone, "/app/issues/1/follow", { email: "[email protected]" }, "/app/issues/1");163+		const stale = link(queued("[email protected]")[0]?.body ?? "", "follow").replace(164+			"/follow?",165+			"/unfollow?",166+		);167+		const freed = Number(/f=(\d+)/.exec(stale)?.[1]);168+		await inst.app.request(stale, { method: "POST" });169+		const next = new Browser(inst.app, "198.51.100.71");170+		await send(next, "/app/issues/1/follow", { email: "[email protected]" }, "/app/issues/1");171+		const reused = inst.kepr172+			.query<{ id: number }, [string]>("SELECT id FROM issue_followers WHERE email = ?")173+			.get("[email protected]")?.id;174+		expect(reused).toBe(freed);175+		await inst.app.request(link(queued("[email protected]")[0]?.body ?? "", "follow"));176+		const replay = await inst.app.request(stale, { method: "POST" });177+		expect(replay.headers.get("location")).toBe("/app/issues/1?notice=expired");178+		const other = new Browser(inst.app, "198.51.100.72");179+		await send(other, "/app/issues/1", { body: "One more." });180+		expect(queued("[email protected]")).toHaveLength(2);181+	});158182 });