chore(node-client): harden existing platform implementation (#1403)

Hardens the Node platform layer landed in #1393, ahead of the
client-side SDK implementation that builds on it. First atomic in the
node-client port stack.

- **NodeStorage**: log (no longer swallow) flush errors; validate/log on
malformed cache reads and ignore non-string cache values; write the temp
file via an exclusive `wx` open so the write cannot be redirected
through a symlink; warn when `getNodeStorage` is called with a
`localStoragePath` that differs from the process singleton's.


<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Changes on-disk cache semantics and filesystem write behavior; low
blast radius but incorrect handling could affect flag persistence or
local file security.
> 
> **Overview**
> Hardens the Node client **platform** layer around flag disk cache and
tightens related tests.
> 
> **`NodeStorage`** now treats a missing cache file as a normal first
run (no warning, no empty rewrite). Malformed or unreadable cache files
trigger a **warn** and recovery write; loaded JSON must be a plain
object and only **string** values are kept. Persists via an exclusive
**`wx`** temp open (after removing any existing temp path) so writes
cannot follow a symlink planted on the temp file path.
**`getNodeStorage`** records the first `localStoragePath` and **warns**
if a later call passes a different path; **`resetNodeStorage`** clears
that path tracking too.
> 
> **Tests** cover first-run behavior, malformed JSON warnings,
non-string cache entries, symlink safety, singleton path mismatch
warnings, and **`NodeInfo`** no longer pins the SDK **version** string
in expectations.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
1962f54. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

---------

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>

Author: 3 people

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

@@ -24,11 +24,10 @@ afterEach(() => {
 
 it('returns sdk data with the package name, version, and user-agent base', () => {
   const info = new NodeInfo();
-  expect(info.sdkData()).toEqual({
-    name: 'node-client-sdk',
-    version: '0.0.1',
-    userAgentBase: 'NodeClient',
-  });
+  const data = info.sdkData();
+
+  expect(data.name).toEqual('node-client-sdk')
+  expect(data.userAgentBase).toEqual('NodeClient')
 });
 
 it('reports the runtime node version under platformData additional', () => {

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'),
+  );
+});

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<string, string>));
+        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,31 @@ export default class NodeStorage implements Storage {
 
   private async _atomicWriteToFile(data: Map<string, string>): Promise<void> {
     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 (closeError) {
+          this._logger?.warn(
+            `Failed to close storage temp file during cleanup: ${closeError}`,
+          );
+        }
+      }
       try {
         await fs.unlink(this._tempFile);
       } catch {
@@ -142,15 +171,22 @@ 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;
 }
 
 /** @internal Visible for testing only. */
 export function resetNodeStorage(): void {
   instance = undefined;
+  instancePath = undefined;
 }