diff --git a/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts b/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts index 84d34aa693..a44bf0b41c 100644 --- a/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts +++ b/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts @@ -70,6 +70,65 @@ it('recovers when the storage file contains invalid JSON', async () => { await expect(storage.get('alpha')).resolves.toBe('one'); }); +it('does not warn on first run when the cache file does not exist', async () => { + const logger = createMockLogger(); + const storage = new NodeStorage(tmpRoot, logger); + await expect(storage.get('anything')).resolves.toBeNull(); + + expect(logger.warn).not.toHaveBeenCalled(); +}); + +it('does not preemptively write the cache file on first run', async () => { + const storage = new NodeStorage(tmpRoot); + await expect(storage.get('anything')).resolves.toBeNull(); + + await expect(fs.access(path.join(tmpRoot, 'ldcache.json'))).rejects.toMatchObject({ + code: 'ENOENT', + }); +}); + +it('warns when the cache file is not valid JSON', async () => { + await fs.writeFile(path.join(tmpRoot, 'ldcache.json'), 'not json', 'utf8'); + + 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'), + ); +}); + +it('ignores non-string values when loading the cache', async () => { + await fs.writeFile( + path.join(tmpRoot, 'ldcache.json'), + JSON.stringify({ good: 'keep', obj: { nested: true }, arr: [1, 2], num: 5 }), + 'utf8', + ); + + const storage = new NodeStorage(tmpRoot); + await expect(storage.get('good')).resolves.toBe('keep'); + await expect(storage.get('obj')).resolves.toBeNull(); + await expect(storage.get('arr')).resolves.toBeNull(); + await expect(storage.get('num')).resolves.toBeNull(); +}); + +it('does not follow a symlink planted at the temp file path', async () => { + const storage = new NodeStorage(tmpRoot); + // Ensure initialization (which clears any temp file) has completed before planting. + await storage.get('warmup'); + + const victim = path.join(tmpRoot, 'victim.txt'); + await fs.writeFile(victim, 'protected', 'utf8'); + await fs.symlink(victim, path.join(tmpRoot, 'ldcache.json.tmp')); + + await storage.set('alpha', 'one'); + + // The exclusive open removes the symlink and writes a fresh file, so the victim is untouched. + await expect(fs.readFile(victim, 'utf8')).resolves.toBe('protected'); + 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'); @@ -104,3 +163,14 @@ it('rebuilds the singleton after resetNodeStorage', () => { const second = getNodeStorage(tmpRoot); expect(second).not.toBe(first); }); + +it('warns when getNodeStorage is called with a different localStoragePath', () => { + getNodeStorage(tmpRoot); + + const logger = createMockLogger(); + getNodeStorage(path.join(tmpRoot, 'different'), logger); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('different localStoragePath'), + ); +}); diff --git a/packages/sdk/node-client/src/platform/NodeStorage.ts b/packages/sdk/node-client/src/platform/NodeStorage.ts index 264e37301d..26b9fc1320 100644 --- a/packages/sdk/node-client/src/platform/NodeStorage.ts +++ b/packages/sdk/node-client/src/platform/NodeStorage.ts @@ -46,11 +46,19 @@ export default class NodeStorage implements Storage { try { const data = await fs.readFile(this._storageFile, 'utf8'); const parsed = JSON.parse(data); - if (parsed && typeof parsed === 'object') { - this._cache = new Map(Object.entries(parsed as Record)); + if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) { + const entries = Object.entries(parsed).filter( + ([, value]) => typeof value === 'string', + ) as [string, string][]; + this._cache = new Map(entries); + } + } catch (error) { + if ((error as NodeJS.ErrnoException)?.code !== 'ENOENT') { + this._logger?.warn( + `Discarding malformed flag cache at ${this._storageFile}: ${error instanceof Error ? error.message : error}`, + ); + await this._atomicWriteToFile(this._cache); } - } catch { - await this._atomicWriteToFile(this._cache); } return true; @@ -62,10 +70,29 @@ export default class NodeStorage implements Storage { private async _atomicWriteToFile(data: Map): Promise { const content = JSON.stringify(Object.fromEntries(data)); + let handle: fs.FileHandle | undefined; try { - await fs.writeFile(this._tempFile, content, { encoding: 'utf8', mode: 0o600 }); + try { + await fs.unlink(this._tempFile); + } catch { + // Either the temp file didn't exist, which we don't care about or there was + // a problem deleting the file, for example a problem with permissions, in + // which case the subsequent write will likely also fail and be handled by its + // exception handler. + } + handle = await fs.open(this._tempFile, 'wx', 0o600); + await handle.writeFile(content, 'utf8'); + await handle.close(); + handle = undefined; await fs.rename(this._tempFile, this._storageFile); } catch (error) { + if (handle) { + try { + await handle.close(); + } catch { + // Ignore close errors during cleanup. + } + } try { await fs.unlink(this._tempFile); } catch { @@ -142,10 +169,16 @@ export default class NodeStorage implements Storage { // process share the same cache file. The first call's storagePath / logger wins; // later calls ignore the arguments. let instance: NodeStorage | undefined; +let instancePath: string | undefined; export function getNodeStorage(storagePath?: string, logger?: LDLogger): NodeStorage { if (!instance) { instance = new NodeStorage(storagePath, logger); + instancePath = storagePath; + } else if (storagePath !== undefined && storagePath !== instancePath) { + logger?.warn( + `NodeStorage was already initialized with a different localStoragePath; ignoring '${storagePath}'.`, + ); } return instance; } @@ -153,4 +186,5 @@ export function getNodeStorage(storagePath?: string, logger?: LDLogger): NodeSto /** @internal Visible for testing only. */ export function resetNodeStorage(): void { instance = undefined; + instancePath = undefined; }