Merge pull request Expensify#740 from callstack-internal/JKobrynski/feat/80091-prevent-storage-reads-for-ram-only-keys

roryabraham · web-flow · commit 9475c570927a · 2026-02-26T08:41:25.000-08:00
[Onyx Audit] prevent storage reads for RAM-only keys
diff --git a/lib/Onyx.ts b/lib/Onyx.ts
@@ -59,6 +59,12 @@ function init({
 
     if (shouldSyncMultipleInstances) {
         Storage.keepInstancesSync?.((key, value) => {
+            // RAM-only keys should never sync from storage as they may have stale persisted data
+            // from before the key was migrated to RAM-only.
+            if (OnyxUtils.isRamOnlyKey(key)) {
+                return;
+            }
+
             cache.set(key, value);
 
             // Check if this is a collection member key to prevent duplicate callbacks
diff --git a/lib/OnyxUtils.ts b/lib/OnyxUtils.ts
@@ -265,6 +265,14 @@ function get<TKey extends OnyxKey, TValue extends OnyxValue<TKey>>(key: TKey): P
         return Promise.resolve(cache.get(key) as TValue);
     }
 
+    // RAM-only keys should never read from storage (they may have stale persisted data
+    // from before the key was migrated to RAM-only). Mark as nullish so future get() calls
+    // short-circuit via hasCacheForKey and avoid re-running this branch.
+    if (isRamOnlyKey(key)) {
+        cache.addNullishStorageKey(key);
+        return Promise.resolve(undefined as TValue);
+    }
+
     const taskName = `${TASK.GET}:${key}` as const;
 
     // When a value retrieving task for this key is still running hook to it
@@ -324,6 +332,15 @@ function multiGet<TKey extends OnyxKey>(keys: CollectionKeyBase[]): Promise<Map<
      * These missingKeys will be later used to multiGet the data from the storage.
      */
     for (const key of keys) {
+        // RAM-only keys should never read from storage as they may have stale persisted data
+        // from before the key was migrated to RAM-only.
+        if (isRamOnlyKey(key)) {
+            if (cache.hasCacheForKey(key)) {
+                dataMap.set(key, cache.get(key) as OnyxValue<TKey>);
+            }
+            continue;
+        }
+
         const cacheValue = cache.get(key) as OnyxValue<TKey>;
         if (cacheValue) {
             dataMap.set(key, cacheValue);
@@ -441,7 +458,10 @@ function getAllKeys(): Promise<Set<OnyxKey>> {
 
     // Otherwise retrieve the keys from storage and capture a promise to aid concurrent usages
     const promise = Storage.getAllKeys().then((keys) => {
-        cache.setAllKeys(keys);
+        // Filter out RAM-only keys from storage results as they may be stale entries
+        // from before the key was migrated to RAM-only.
+        const filteredKeys = keys.filter((key) => !isRamOnlyKey(key));
+        cache.setAllKeys(filteredKeys);
 
         // return the updated set of keys
         return cache.getAllKeys();
@@ -1091,7 +1111,10 @@ function mergeInternal<TValue extends OnyxInput<OnyxKey> | undefined, TChange ex
  * Merge user provided default key value pairs.
  */
 function initializeWithDefaultKeyStates(): Promise<void> {
-    return Storage.multiGet(Object.keys(defaultKeyStates)).then((pairs) => {
+    // Filter out RAM-only keys from storage reads as they may have stale persisted data
+    // from before the key was migrated to RAM-only.
+    const keysToFetch = Object.keys(defaultKeyStates).filter((key) => !isRamOnlyKey(key));
+    return Storage.multiGet(keysToFetch).then((pairs) => {
         const existingDataAsObject = Object.fromEntries(pairs) as Record<string, unknown>;
 
         const merged = utils.fastMerge(existingDataAsObject, defaultKeyStates, {
diff --git a/tests/unit/onyxTest.ts b/tests/unit/onyxTest.ts
@@ -3112,3 +3112,299 @@ describe('Onyx.init', () => {
         });
     });
 });
+
+// Separate describe block to control Onyx.init() per-test so we can pre-seed storage before init.
+describe('RAM-only keys should not read from storage', () => {
+    let cache: typeof OnyxCache;
+
+    beforeEach(() => {
+        // Resets the deferred init task before each test.
+        Object.assign(OnyxUtils.getDeferredInitTask(), createDeferredTask());
+        cache = require('../../lib/OnyxCache').default;
+    });
+
+    afterEach(() => {
+        jest.restoreAllMocks();
+        return Onyx.clear();
+    });
+
+    it('should not return stale storage data for a RAM-only key via get', async () => {
+        // Simulate stale data left in storage from before the key was RAM-only
+        await StorageMock.setItem(ONYX_KEYS.RAM_ONLY_TEST_KEY, 'stale_value');
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        let receivedValue: unknown;
+        const connection = Onyx.connect({
+            key: ONYX_KEYS.RAM_ONLY_TEST_KEY,
+            callback: (value) => {
+                receivedValue = value;
+            },
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        expect(receivedValue).toBeUndefined();
+        expect(cache.get(ONYX_KEYS.RAM_ONLY_TEST_KEY)).toBeUndefined();
+
+        Onyx.disconnect(connection);
+    });
+
+    it('should not return stale storage data for RAM-only collection members via multiGet', async () => {
+        const collectionMember1 = `${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`;
+        const collectionMember2 = `${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}2`;
+
+        // Simulate stale collection members in storage
+        await StorageMock.setItem(collectionMember1, {name: 'stale_1'});
+        await StorageMock.setItem(collectionMember2, {name: 'stale_2'});
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        let receivedCollection: OnyxCollection<unknown>;
+        const connection = Onyx.connect({
+            key: ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION,
+            callback: (value) => {
+                receivedCollection = value;
+            },
+            waitForCollectionCallback: true,
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        expect(receivedCollection).toBeUndefined();
+        expect(cache.get(collectionMember1)).toBeUndefined();
+        expect(cache.get(collectionMember2)).toBeUndefined();
+
+        Onyx.disconnect(connection);
+    });
+
+    it('should not include stale RAM-only keys in getAllKeys results', async () => {
+        // Simulate stale data in storage
+        await StorageMock.setItem(ONYX_KEYS.RAM_ONLY_TEST_KEY, 'stale_value');
+        await StorageMock.setItem(`${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`, {stale: 'member'});
+        await StorageMock.setItem(ONYX_KEYS.OTHER_TEST, 'normal_value');
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        const keys = await OnyxUtils.getAllKeys();
+
+        expect(keys.has(ONYX_KEYS.RAM_ONLY_TEST_KEY)).toBe(false);
+        expect(keys.has(`${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`)).toBe(false);
+        // Normal keys should still be present
+        expect(keys.has(ONYX_KEYS.OTHER_TEST)).toBe(true);
+    });
+
+    it('should not read stale storage data for RAM-only keys during initializeWithDefaultKeyStates', async () => {
+        // Simulate stale data for a RAM-only key that also has a default key state
+        await StorageMock.setItem(ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE, 'stale_value');
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            initialKeyStates: {
+                [ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE]: 'default_value',
+            },
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // The cache should have the default value, not the stale storage value
+        expect(cache.get(ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE)).toEqual('default_value');
+    });
+
+    it('should not use stale storage data as merge base for RAM-only keys', async () => {
+        // Simulate stale data in storage
+        await StorageMock.setItem(ONYX_KEYS.RAM_ONLY_TEST_KEY, {name: 'stale', token: 'old_token'});
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Merge new data — should NOT merge with stale storage value
+        await Onyx.merge(ONYX_KEYS.RAM_ONLY_TEST_KEY, {name: 'new'});
+
+        // The result should only contain the merged value, not the stale token
+        expect(cache.get(ONYX_KEYS.RAM_ONLY_TEST_KEY)).toEqual({name: 'new'});
+    });
+
+    it('should not read stale storage data when subscribing to individual RAM-only collection members', async () => {
+        const collectionMember = `${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`;
+
+        // Simulate stale data in storage
+        await StorageMock.setItem(collectionMember, {data: 'stale'});
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        const receivedValues: unknown[] = [];
+        const connection = Onyx.connect({
+            key: collectionMember,
+            callback: (value) => {
+                receivedValues.push(value);
+            },
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Should never receive the stale value
+        expect(receivedValues.every((v) => v === undefined || v === null)).toBe(true);
+
+        Onyx.disconnect(connection);
+    });
+
+    it('should still work correctly for normal keys when RAM-only keys have stale storage data', async () => {
+        // Simulate both normal and RAM-only stale data in storage
+        await StorageMock.setItem(ONYX_KEYS.TEST_KEY, 'normal_value');
+        await StorageMock.setItem(ONYX_KEYS.RAM_ONLY_TEST_KEY, 'stale_ram_value');
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        let normalValue: unknown;
+        let ramOnlyValue: unknown;
+
+        const connection1 = Onyx.connect({
+            key: ONYX_KEYS.TEST_KEY,
+            callback: (value) => {
+                normalValue = value;
+            },
+        });
+        const connection2 = Onyx.connect({
+            key: ONYX_KEYS.RAM_ONLY_TEST_KEY,
+            callback: (value) => {
+                ramOnlyValue = value;
+            },
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Normal key should read from storage as expected
+        expect(normalValue).toEqual('normal_value');
+        // RAM-only key should NOT read stale value from storage
+        expect(ramOnlyValue).toBeUndefined();
+
+        Onyx.disconnect(connection1);
+        Onyx.disconnect(connection2);
+    });
+
+    it('should not sync RAM-only keys from other instances via keepInstancesSync', async () => {
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+            shouldSyncMultipleInstances: true,
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Get the callback that was passed to keepInstancesSync
+        const syncCallback = (StorageMock.keepInstancesSync as jest.Mock).mock.calls[0]?.[0];
+        expect(syncCallback).toBeDefined();
+
+        let receivedValue: unknown;
+        const connection = Onyx.connect({
+            key: ONYX_KEYS.RAM_ONLY_TEST_KEY,
+            callback: (value) => {
+                receivedValue = value;
+            },
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Simulate another tab syncing a stale RAM-only key value
+        syncCallback(ONYX_KEYS.RAM_ONLY_TEST_KEY, 'synced_stale_value');
+        await act(async () => waitForPromisesToResolve());
+
+        // The RAM-only key should NOT have been updated from the sync
+        expect(receivedValue).toBeUndefined();
+        expect(cache.get(ONYX_KEYS.RAM_ONLY_TEST_KEY)).toBeUndefined();
+
+        // Verify that normal keys still sync correctly
+        let normalValue: unknown;
+        const connection2 = Onyx.connect({
+            key: ONYX_KEYS.OTHER_TEST,
+            callback: (value) => {
+                normalValue = value;
+            },
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        syncCallback(ONYX_KEYS.OTHER_TEST, 'synced_normal_value');
+        await act(async () => waitForPromisesToResolve());
+
+        expect(normalValue).toEqual('synced_normal_value');
+
+        Onyx.disconnect(connection);
+        Onyx.disconnect(connection2);
+    });
+
+    it('should serve RAM-only keys from cache and normal keys from storage in multiGet', async () => {
+        const ramOnlyMember = `${ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION}1`;
+        const normalMember = `${ONYX_KEYS.COLLECTION.TEST_KEY}1`;
+
+        // Pre-seed storage with stale data for both normal and RAM-only keys
+        await StorageMock.setItem(normalMember, 'normal_from_storage');
+        await StorageMock.setItem(ramOnlyMember, {data: 'stale_collection_member'});
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Set a RAM-only collection member via Onyx (goes to cache only)
+        await Onyx.set(ramOnlyMember, {data: 'fresh_from_cache'});
+
+        // multiGet receives individual keys (e.g. collection members), not collection base keys
+        const result = await OnyxUtils.multiGet([normalMember, ramOnlyMember]);
+
+        // Normal key should come from storage
+        expect(result.get(normalMember)).toEqual('normal_from_storage');
+        // RAM-only collection member should come from cache, not stale storage
+        expect(result.get(ramOnlyMember)).toEqual({data: 'fresh_from_cache'});
+    });
+
+    it('should return cached value for RAM-only key after set then connect', async () => {
+        await StorageMock.setItem(ONYX_KEYS.RAM_ONLY_TEST_KEY, 'stale_value');
+
+        Onyx.init({
+            keys: ONYX_KEYS,
+            ramOnlyKeys: [ONYX_KEYS.RAM_ONLY_TEST_KEY, ONYX_KEYS.COLLECTION.RAM_ONLY_COLLECTION, ONYX_KEYS.RAM_ONLY_WITH_INITIAL_VALUE],
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Write a fresh value to the RAM-only key
+        await Onyx.set(ONYX_KEYS.RAM_ONLY_TEST_KEY, 'fresh_value');
+
+        let receivedValue: unknown;
+        const connection = Onyx.connect({
+            key: ONYX_KEYS.RAM_ONLY_TEST_KEY,
+            callback: (value) => {
+                receivedValue = value;
+            },
+        });
+        await act(async () => waitForPromisesToResolve());
+
+        // Should get the fresh cached value, not the stale storage value
+        expect(receivedValue).toEqual('fresh_value');
+        expect(cache.get(ONYX_KEYS.RAM_ONLY_TEST_KEY)).toEqual('fresh_value');
+
+        // Verify storage was NOT written to
+        const storageValue = await StorageMock.getItem(ONYX_KEYS.RAM_ONLY_TEST_KEY);
+        expect(storageValue).toEqual('stale_value');
+
+        Onyx.disconnect(connection);
+    });
+});
diff --git a/tests/unit/useOnyxTest.ts b/tests/unit/useOnyxTest.ts
