diff --git a/pkg/appview/db/device_store_scan_test.go b/pkg/appview/db/device_store_scan_test.go new file mode 100644 index 0000000..cba420d --- /dev/null +++ b/pkg/appview/db/device_store_scan_test.go @@ -0,0 +1,118 @@ +package db + +import ( + "fmt" + "testing" + "time" + + "golang.org/x/crypto/bcrypt" +) + +// scanGuardCost makes a single bcrypt comparison expensive enough to be visible +// in wall-clock time. The defect being guarded is O(n) bcrypt over the devices +// table, so the only way to observe it is to make each comparison cost +// something; at the production cost of 10 the difference between "scanned six +// rows" and "scanned none" is lost in test noise. +const scanGuardCost = 13 + +// seedIndexedDeviceRows inserts n devices that are already indexed +// (secret_lookup populated) and whose bcrypt hashes are deliberately expensive. +// +// Only one hash is generated: the rest are copies with a mutated final +// character. bcrypt reads its cost from the hash prefix and runs the full key +// derivation before comparing, so a mutated tail costs exactly as much as a +// genuine hash and then fails to match — which is what a scan over non-matching +// rows does anyway. +func seedIndexedDeviceRows(t *testing.T, store *DeviceStore, did, handle string, n int) { + t.Helper() + + base, err := bcrypt.GenerateFromPassword([]byte("filler-secret"), scanGuardCost) + if err != nil { + t.Fatalf("GenerateFromPassword: %v", err) + } + + for i := 0; i < n; i++ { + hash := append([]byte(nil), base...) + // Vary the tail so secret_hash stays UNIQUE, keeping the base64 alphabet. + hash[len(hash)-1] = byte('a' + (i % 26)) + if _, err := store.db.Exec(` + INSERT INTO devices (id, did, handle, name, secret_hash, secret_lookup, + ip_address, user_agent, created_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, datetime('now')) + `, + fmt.Sprintf("filler-device-%d", i), did, handle, fmt.Sprintf("Filler %d", i), + string(hash), deviceSecretLookup(fmt.Sprintf("atcr_device_filler_%d", i)), + "192.168.1.1", "Test Agent", + ); err != nil { + t.Fatalf("seed device %d: %v", i, err) + } + } +} + +// TestDeviceStore_IndexedRowsAreNeverBcryptCompared is the real regression guard +// for the O(n) bcrypt scan. +// +// Its sibling TestDeviceStore_ValidateDoesNotScanIndexedRows does not catch the +// defect: it asserts every row is indexed and that an unknown secret errors, +// and both of those hold whether or not the code scans. Removing the +// `WHERE secret_lookup IS NULL` filter from the fallback query leaves it green. +// +// What actually has to be true is that indexed rows are never handed to bcrypt. +// That is only observable in time, so the assertion is a deadline: six expensive +// rows cost ~1.8s to scan and ~0ms to skip. +func TestDeviceStore_IndexedRowsAreNeverBcryptCompared(t *testing.T) { + const fillerRows = 6 + + store := setupTestDB(t) + createTestUser(t, store, "did:plc:alice123", "alice.bsky.social") + seedIndexedDeviceRows(t, store, "did:plc:alice123", "alice.bsky.social", fillerRows) + + // A real device, indexed at creation like every post-0028 row. + secret := newDeviceForTest(t, store, "did:plc:alice123", "alice.bsky.social", "My Device") + + var unindexed int + if err := store.db.QueryRow( + `SELECT COUNT(*) FROM devices WHERE secret_lookup IS NULL OR secret_lookup = ''`, + ).Scan(&unindexed); err != nil { + t.Fatalf("count unindexed: %v", err) + } + if unindexed != 0 { + t.Fatalf("fixture is wrong: %d rows unindexed, so the scan has legitimate work to do", unindexed) + } + + // Anything above one expensive comparison means rows were scanned. The + // budget is generous against a ~1.8s full scan so this does not flake on a + // loaded machine. + const budget = 400 * time.Millisecond + + t.Run("unknown secret", func(t *testing.T) { + start := time.Now() + _, err := store.ValidateDeviceSecret("atcr_device_no_such_secret") + elapsed := time.Since(start) + + if err == nil { + t.Error("expected an error for an unknown secret") + } + // The worst case for the old code: no row ever matches, so it paid for + // every bcrypt comparison in the table before giving up. + if elapsed > budget { + t.Errorf("rejecting an unknown secret took %v (budget %v) — indexed rows are being bcrypt-compared, which is the O(n) scan this fix removed", elapsed, budget) + } + }) + + t.Run("known secret", func(t *testing.T) { + start := time.Now() + device, err := store.ValidateDeviceSecret(secret) + elapsed := time.Since(start) + + if err != nil { + t.Fatalf("ValidateDeviceSecret() error = %v", err) + } + if device.Name != "My Device" { + t.Errorf("Name = %v, want My Device", device.Name) + } + if elapsed > budget { + t.Errorf("resolving an indexed secret took %v (budget %v) — it is not using the index", elapsed, budget) + } + }) +}