Commit `c9cf3e5403`

c9cf3e5403ade1938ac68460d6238a8409872528

parent: 2979e93075

Verified · cmc

cmc <hello@cleberg.net> · 2026-09-20 09:20 UTC

store: fix AddPushDevice id on token-reuse path

LastInsertId is left unchanged by SQLite when ON CONFLICT DO UPDATE
fires instead of the INSERT, so a re-registered token could return an
unrelated row's id. Read the id back by token instead. Strengthen the
test to assert on the returned id across both the fresh-registration
and re-registration paths.

Ref #89

internal/store/push.go +7 −3

		@@ -19,16 +19,20 @@ type PushDevice struct {
19	19
20	20	// AddPushDevice registers a token to an account. A token already present
21	21	// changes hands rather than erroring: Apple reuses tokens, and a reinstall
22		// hands the same one to whichever account signs in next.
	22	// hands the same one to whichever account signs in next. The id is read
	23	// back by token rather than taken from LastInsertId, which SQLite leaves
	24	// unchanged when the DO UPDATE arm fires instead of the INSERT.
23	25	func (s *Store) AddPushDevice(userID int64, token, label string) (int64, error) {
24		res, err := s.DB.Exec(`
	26	_, err := s.DB.Exec(`
25	27	INSERT INTO push_devices (user_id, token, label) VALUES (?, ?, ?)
26	28	ON CONFLICT(token) DO UPDATE SET user_id = excluded.user_id, label = excluded.label`,
27	29	userID, token, label)
28	30	if err != nil {
29	31	return 0, err
30	32	}
31		return res.LastInsertId()
	33	var id int64
	34	err = s.DB.QueryRow("SELECT id FROM push_devices WHERE token = ?", token).Scan(&id)
	35	return id, err
32	36	}
33	37
34	38	func (s *Store) PushDevices(userID int64) ([]PushDevice, error) {

internal/store/push_test.go +17 −3

		@@ -18,7 +18,8 @@ func TestPushDevices(t *testing.T) {
18	18	t.Fatal(err)
19	19	}
20	20
21		if _, err := s.AddPushDevice(uid, "tok-a", "iphone"); err != nil {
	21	firstID, err := s.AddPushDevice(uid, "tok-a", "iphone")
	22	if err != nil {
22	23	t.Fatalf("AddPushDevice: %v", err)
23	24	}
24	25	devices, err := s.PushDevices(uid)
		@@ -28,14 +29,21 @@ func TestPushDevices(t *testing.T) {
28	29	if len(devices) != 1 \|\| devices[0].Token != "tok-a" \|\| devices[0].Label != "iphone" {
29	30	t.Fatalf("got %+v", devices)
30	31	}
	32	if firstID != devices[0].ID {
	33	t.Fatalf("AddPushDevice returned %d, row id is %d", firstID, devices[0].ID)
	34	}
31	35
32	36	// Apple reuses tokens: re-registering updates the label and the owner
33		// rather than erroring, so a reinstall under another account works.
	37	// rather than erroring, so a reinstall under another account works. The
	38	// returned id must be the existing row's, not an unrelated rowid left
	39	// over from SQLite's last real INSERT (the DO UPDATE arm does not
	40	// advance last_insert_rowid()).
34	41	bob, err := s.CreateUser("bob", false)
35	42	if err != nil {
36	43	t.Fatal(err)
37	44	}
38		if _, err := s.AddPushDevice(bob, "tok-a", "ipad"); err != nil {
	45	reregID, err := s.AddPushDevice(bob, "tok-a", "ipad")
	46	if err != nil {
39	47	t.Fatalf("re-register: %v", err)
40	48	}
41	49	if d, _ := s.PushDevices(uid); len(d) != 0 {
		@@ -45,6 +53,12 @@ func TestPushDevices(t *testing.T) {
45	53	if len(d) != 1 \|\| d[0].Label != "ipad" {
46	54	t.Fatalf("got %+v", d)
47	55	}
	56	if reregID != d[0].ID {
	57	t.Fatalf("re-register returned %d, existing row id is %d", reregID, d[0].ID)
	58	}
	59	if reregID != firstID {
	60	t.Fatalf("re-register returned %d, want the reused row's original id %d", reregID, firstID)
	61	}
48	62
49	63	// Removal is scoped to the owner: alice cannot remove bob's device.
50	64	if err := s.RemovePushDevice(uid, d[0].ID); err != ErrNotFound {

Commit c9cf3e5403

Commit `c9cf3e5403`