Fixed KnexKvStore returning null for missing keys

Fedify's KvStore contract requires get() to resolve to undefined when a
key does not exist; null is reserved for stored values. Since Fedify
introduced persistent negative key caching, its KvKeyCache strictly
distinguishes the two: undefined is a cache miss (fetch the key), while
null marks the key as known-unavailable and skips fetching entirely.

Because KnexKvStore returned null for missing rows, every signing key
that had never been cached was misreported as a cached negative entry,
so Fedify never fetched the sender's key and inbound HTTP signature
verification failed with 401 for any new peer. This only affected
deployments using the default MySQL-backed store (self-hosters) - the
Redis store used in production follows the contract correctly.

Also handle stored JSON null values on read: Fedify persists null to
mark failed key lookups, and reading such a row previously threw a
TypeError in Object.hasOwn().

Fixes #1961

Author: sagzy

src/knex.kvstore.integration.test.ts

@@ -18,10 +18,11 @@ describe('KnexKvStore', () => {
         const store = KnexKvStore.create(client, table, logger);
 
         // checkReadingUnsetKey
+        // Fedify's KvStore contract requires undefined (not null) for missing
+        // keys - null means a stored negative entry (see checkNullValue)
         {
             const actual = await store.get(['unsetkey']);
-            const expected = null;
-            expect(actual).toEqual(expected);
+            expect(actual).toBeUndefined();
         }
 
         // checkReadingSetKey
@@ -46,8 +47,15 @@ describe('KnexKvStore', () => {
             await store.set(['deleted'], { initial: 'value' });
             await store.delete(['deleted']);
             const actual = await store.get(['deleted']);
-            const expected = null;
-            expect(actual).toEqual(expected);
+            expect(actual).toBeUndefined();
+        }
+
+        // checkNullValue - Fedify's KvKeyCache stores null to mark a key
+        // lookup as failed, and must read it back as null (not undefined)
+        {
+            await store.set(['nullvalue'], null);
+            const actual = await store.get(['nullvalue']);
+            expect(actual).toBeNull();
         }
     });
 
@@ -131,7 +139,7 @@ describe('KnexKvStore', () => {
 
         const result = await store.get(['expired-key']);
 
-        expect(result).toBeNull();
+        expect(result).toBeUndefined();
     });
 
     it('Deletes expired entries from the database on read', async () => {

src/knex.kvstore.ts

@@ -46,15 +46,22 @@ export class KnexKvStore implements KvStore {
             key: this.keyToString(key),
         };
         const row = await this.knex(this.table).where(query).first();
+        // Fedify distinguishes a missing key (undefined) from a stored null:
+        // its KvKeyCache treats null as a cached "key is unavailable" marker
+        // and skips fetching the key entirely, so returning null for missing
+        // keys breaks HTTP signature verification for never-seen senders.
         if (!row) {
-            return null;
+            return undefined;
         }
         if (row.expires !== null && row.expires <= new Date()) {
             this.logging.debug(
                 `KnexKvStore: Deleting expired key ${key}`,
                 keyInfo,
             );
             await this.knex(this.table).where(query).del();
+            return undefined;
+        }
+        if (row.value === null) {
             return null;
         }
         if (Object.hasOwn(row.value, '@@BOOLEAN@@')) {