diff --git a/packages/sdk/node-client/__tests__/platform/NodePlatform.test.ts b/packages/sdk/node-client/__tests__/platform/NodePlatform.test.ts index 04e97db88a..fd4dc4e413 100644 --- a/packages/sdk/node-client/__tests__/platform/NodePlatform.test.ts +++ b/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'), ); }); diff --git a/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts b/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts index 1367d76a9c..deba88940a 100644 --- a/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts +++ b/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', () => { diff --git a/packages/sdk/node-client/src/LDCommon.ts b/packages/sdk/node-client/src/LDCommon.ts index a4a613db74..c0d278968a 100644 --- a/packages/sdk/node-client/src/LDCommon.ts +++ b/packages/sdk/node-client/src/LDCommon.ts @@ -1,4 +1,5 @@ export type { + ConnectionMode, InitializerEntry, ModeDefinition, SynchronizerEntry, diff --git a/packages/sdk/node-client/src/platform/NodeStorage.ts b/packages/sdk/node-client/src/platform/NodeStorage.ts index 6840ffbc13..6b1434ea03 100644 --- a/packages/sdk/node-client/src/platform/NodeStorage.ts +++ b/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; + private readonly _initialized: Promise; private readonly _logger?: LDLogger; - private _initError?: Error; + private _persistenceDisabled: boolean = false; private _cache: Map = new Map(); private _flushPending: boolean = false; private _flushQueue: Promise = Promise.resolve(); @@ -33,10 +32,17 @@ export default class NodeStorage implements Storage { this._initialized = this._initialize(); } - private async _initialize(): Promise { + private async _initialize(): Promise { 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 { @@ -44,6 +50,11 @@ export default class NodeStorage implements Storage { } 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 { - 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 { - 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 { - 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) {