fix(node-client-sdk): better handling for bad filesystem states

- refuse to load from symlinks
- fallsback to inmemory store if filesystem loading fails
- additionally, exposing `ConnectionMode` type for downstream reference

Author: joker23

packages/sdk/node-client/__tests__/platform/NodePlatform.test.ts

@@ -37,13 +37,13 @@ it('round-trips storage values through the file-backed NodeStorage', async () =>
   await expect(platform.storage.get('alpha')).resolves.toBeNull();
 });
 
-it('forwards the logger to NodeStorage so storage failures surface', async () => {
+it('forwards the logger to NodeStorage so storage init failures surface', async () => {
   const platform = new NodePlatform(logger, {
     localStoragePath: path.join(tmpRoot, 'never-created', '\0bad'),
   });
   await expect(platform.storage.get('alpha')).resolves.toBeNull();
-  expect(logger.error).toHaveBeenCalledWith(
-    expect.stringContaining('Error getting key from storage'),
+  expect(logger.warn).toHaveBeenCalledWith(
+    expect.stringContaining('Using in-memory storage as a fallback'),
   );
 });
 

packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts

@@ -129,26 +129,115 @@ it('does not follow a symlink planted at the temp file path', async () => {
   await expect(storage.get('alpha')).resolves.toBe('one');
 });
 
-it('logs and returns sentinel values when initialization fails', async () => {
-  const filePath = path.join(tmpRoot, 'not-a-dir');
-  await fs.writeFile(filePath, 'sentinel', 'utf8');
+it('falls back to in-memory storage instead of following a symlinked storage directory', async () => {
+  const victimDir = path.join(tmpRoot, 'victim-dir');
+  await fs.mkdir(victimDir);
+  const storagePath = path.join(tmpRoot, 'ldcache');
+  await fs.symlink(victimDir, storagePath);
 
   const logger = createMockLogger();
-  const storage = new NodeStorage(filePath, logger);
+  const storage = new NodeStorage(storagePath, logger);
+
+  await storage.set('alpha', 'one');
+  await expect(storage.get('alpha')).resolves.toBe('one');
+  expect(logger.error).not.toHaveBeenCalled();
+  expect(logger.warn).toHaveBeenCalledWith(
+    expect.stringContaining('Using in-memory storage as a fallback'),
+  );
+
+  // The victim directory the symlink pointed at was never written to.
+  await expect(fs.readdir(victimDir)).resolves.toEqual([]);
+});
+
+it('discards a symlink planted at the storage file path instead of reading through it', async () => {
+  const victim = path.join(tmpRoot, 'victim.json');
+  await fs.writeFile(victim, JSON.stringify({ secret: 'do-not-load' }), 'utf8');
+  await fs.symlink(victim, path.join(tmpRoot, 'ldcache.json'));
+
+  const logger = createMockLogger();
+  const storage = new NodeStorage(tmpRoot, logger);
+
+  // The symlinked "cache" is discarded rather than followed, so its contents never load.
+  await expect(storage.get('secret')).resolves.toBeNull();
+  expect(logger.warn).toHaveBeenCalledWith(
+    expect.stringContaining('Discarding malformed flag cache'),
+  );
+  expect(logger.error).not.toHaveBeenCalled();
+
+  // The victim file itself is untouched, and the symlink has been replaced by a real file.
+  await expect(fs.readFile(victim, 'utf8')).resolves.toBe(JSON.stringify({ secret: 'do-not-load' }));
+  await storage.set('alpha', 'one');
+  await expect(storage.get('alpha')).resolves.toBe('one');
+  const onDisk = await fs.readFile(path.join(tmpRoot, 'ldcache.json'), 'utf8');
+  expect(JSON.parse(onDisk)).toEqual({ alpha: 'one' });
+});
+
+it('falls back to in-memory storage when a file occupies the storage directory path', async () => {
+  // No prerelease (v0) installation has customer data to migrate, so a plain file sitting at
+  // the storage directory path is just treated as a generic, unrecoverable init failure.
+  const storagePath = path.join(tmpRoot, 'ldcache');
+  await fs.writeFile(storagePath, 'not a directory', 'utf8');
+
+  const logger = createMockLogger();
+  const storage = new NodeStorage(storagePath, logger);
+
+  await storage.set('alpha', 'one');
+  await expect(storage.get('alpha')).resolves.toBe('one');
+  expect(logger.error).not.toHaveBeenCalled();
+  expect(logger.warn).toHaveBeenCalledTimes(1);
+  expect(logger.warn).toHaveBeenCalledWith(
+    expect.stringContaining('Using in-memory storage as a fallback'),
+  );
+  expect((await fs.stat(storagePath)).isDirectory()).toBe(false);
+});
+
+it('falls back to in-memory storage and warns once when the storage directory cannot be created', async () => {
+  // An intermediate path segment that is a file (rather than the storage directory itself)
+  // means mkdir cannot traverse through it, so this must fall back to in-memory storage.
+  const fileInThePath = path.join(tmpRoot, 'not-a-dir');
+  await fs.writeFile(fileInThePath, 'sentinel', 'utf8');
+  const storagePath = path.join(fileInThePath, 'subdir');
+
+  const logger = createMockLogger();
+  const storage = new NodeStorage(storagePath, logger);
 
   await expect(storage.get('alpha')).resolves.toBeNull();
-  await expect(storage.set('alpha', 'one')).resolves.toBeUndefined();
-  await expect(storage.clear('alpha')).resolves.toBeUndefined();
+  await storage.set('alpha', 'one');
+  await expect(storage.get('alpha')).resolves.toBe('one');
+  await storage.clear('alpha');
+  await expect(storage.get('alpha')).resolves.toBeNull();
 
-  expect(logger.error).toHaveBeenCalledWith(
-    expect.stringContaining('Error getting key from storage'),
+  expect(logger.error).not.toHaveBeenCalled();
+  expect(logger.warn).toHaveBeenCalledTimes(1);
+  expect(logger.warn).toHaveBeenCalledWith(
+    expect.stringContaining('Using in-memory storage as a fallback'),
   );
-  expect(logger.error).toHaveBeenCalledWith(
-    expect.stringContaining('Error setting key in storage'),
+});
+
+it('falls back to in-memory storage when rewriting a discarded malformed cache fails', async () => {
+  // The storage directory itself is created successfully, but the rewrite that normally follows
+  // discarding a malformed cache file fails because a directory occupies the temp-file path.
+  // This exercises the fallback triggering from a site other than storage-directory creation.
+  await fs.writeFile(path.join(tmpRoot, 'ldcache.json'), 'not json', 'utf8');
+  await fs.mkdir(path.join(tmpRoot, 'ldcache.json.tmp'));
+
+  const logger = createMockLogger();
+  const storage = new NodeStorage(tmpRoot, logger);
+
+  await expect(storage.get('anything')).resolves.toBeNull();
+  expect(logger.warn).toHaveBeenCalledWith(
+    expect.stringContaining('Discarding malformed flag cache'),
   );
-  expect(logger.error).toHaveBeenCalledWith(
-    expect.stringContaining('Error clearing key from storage'),
+  expect(logger.warn).toHaveBeenCalledWith(
+    expect.stringContaining('Using in-memory storage as a fallback'),
   );
+  expect(logger.error).not.toHaveBeenCalled();
+
+  await storage.set('alpha', 'one');
+  await expect(storage.get('alpha')).resolves.toBe('one');
+
+  // Persistence is disabled, so the untouched malformed file on disk proves no flush was attempted.
+  await expect(fs.readFile(path.join(tmpRoot, 'ldcache.json'), 'utf8')).resolves.toBe('not json');
 });
 
 it('returns the same singleton across getNodeStorage calls', () => {

packages/sdk/node-client/src/LDCommon.ts

@@ -1,4 +1,5 @@
 export type {
+  ConnectionMode,
   InitializerEntry,
   ModeDefinition,
   SynchronizerEntry,

packages/sdk/node-client/src/platform/NodeStorage.ts

@@ -7,7 +7,6 @@ const DEFAULT_DIR_NAME = 'ldclient-user-cache';
 const STORAGE_FILE_NAME = 'ldcache.json';
 
 const ERROR_PREFIX = {
-  get: 'Error getting key from storage',
   set: 'Error setting key in storage',
   clear: 'Error clearing key from storage',
 } as const;
@@ -18,9 +17,9 @@ export default class NodeStorage implements Storage {
   private readonly _storageDir: string;
   private readonly _storageFile: string;
   private readonly _tempFile: string;
-  private readonly _initialized: Promise<boolean>;
+  private readonly _initialized: Promise<void>;
   private readonly _logger?: LDLogger;
-  private _initError?: Error;
+  private _persistenceDisabled: boolean = false;
   private _cache: Map<string, string> = new Map();
   private _flushPending: boolean = false;
   private _flushQueue: Promise<void> = Promise.resolve();
@@ -33,17 +32,29 @@ export default class NodeStorage implements Storage {
     this._initialized = this._initialize();
   }
 
-  private async _initialize(): Promise<boolean> {
+  private async _initialize(): Promise<void> {
     try {
       await fs.mkdir(this._storageDir, { recursive: true });
 
+      // fs.mkdir succeeds silently if the path already exists as a symlink to a directory, so a
+      // pre-planted symlink would otherwise redirect where the cache is read from and written to
+      // without ever surfacing as an init failure.
+      if ((await fs.lstat(this._storageDir)).isSymbolicLink()) {
+        throw new Error(`Storage directory path is a symlink, not a real directory: ${this._storageDir}`);
+      }
+
       try {
         await fs.unlink(this._tempFile);
       } catch {
         // Ignore if temp file does not exist.
       }
 
       try {
+        const fileStat = await fs.lstat(this._storageFile);
+        if (!fileStat.isFile()) {
+          throw new Error(`Storage file exists but is not a regular file: ${this._storageFile}`);
+        }
+
         const data = await fs.readFile(this._storageFile, 'utf8');
         const parsed = JSON.parse(data);
         if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
@@ -60,11 +71,11 @@ export default class NodeStorage implements Storage {
           await this._atomicWriteToFile(this._cache);
         }
       }
-
-      return true;
     } catch (error) {
-      this._initError = error instanceof Error ? error : new Error(String(error));
-      return false;
+      this._persistenceDisabled = true;
+      this._logger?.warn(
+        `Failed to initialize local flag cache at ${this._storageDir}: ${error instanceof Error ? error.message : error}. Using in-memory storage as a fallback - flags will not persist across restarts.`,
+      );
     }
   }
 
@@ -108,26 +119,17 @@ export default class NodeStorage implements Storage {
     this._logger?.error(`${ERROR_PREFIX[op]}: ${key}, reason: ${reason}`);
   }
 
-  private _initFailureReason(): string {
-    return `not initialized${this._initError ? `: ${this._initError.message}` : ''}`;
-  }
-
   async get(key: string): Promise<string | null> {
-    const initialized = await this._initialized;
-    if (!initialized) {
-      this._logError('get', key, this._initFailureReason());
-      return null;
-    }
+    await this._initialized;
     return this._cache.get(key) ?? null;
   }
 
   async set(key: string, value: string): Promise<void> {
-    const initialized = await this._initialized;
-    if (!initialized) {
-      this._logError('set', key, this._initFailureReason());
+    await this._initialized;
+    this._cache.set(key, value);
+    if (this._persistenceDisabled) {
       return;
     }
-    this._cache.set(key, value);
     try {
       await this._scheduleFlush();
     } catch (error) {
@@ -136,13 +138,12 @@ export default class NodeStorage implements Storage {
   }
 
   async clear(key: string): Promise<void> {
-    const initialized = await this._initialized;
-    if (!initialized) {
-      this._logError('clear', key, this._initFailureReason());
-      return;
-    }
+    await this._initialized;
     if (this._cache.has(key)) {
       this._cache.delete(key);
+      if (this._persistenceDisabled) {
+        return;
+      }
       try {
         await this._scheduleFlush();
       } catch (error) {