From 922614d7f294030e654274bb0877f0a39987f2c5 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 1 Jun 2026 12:43:19 -0400 Subject: [PATCH 1/7] fix: Harden Node platform I/O durability and resource limits Improves robustness of the already-merged Node platform layer: - NodeStorage logs swallowed flush errors, validates and logs on malformed cache reads, writes the temp file with an exclusive (symlink-safe) open, and warns on localStoragePath mismatch across the process singleton. - NodeResponse caps the buffered response body to guard against memory exhaustion from an oversized or hostile response. - NodeRequests applies a default request timeout so a stuck server cannot hang polling or event delivery indefinitely. Co-Authored-By: Claude Opus 4.7 --- .../node-client/src/platform/NodeRequests.ts | 4 +- .../node-client/src/platform/NodeResponse.ts | 11 +++++ .../node-client/src/platform/NodeStorage.ts | 45 ++++++++++++++++--- 3 files changed, 54 insertions(+), 6 deletions(-) diff --git a/packages/sdk/node-client/src/platform/NodeRequests.ts b/packages/sdk/node-client/src/platform/NodeRequests.ts index 1ed819e94d..bdc3633990 100644 --- a/packages/sdk/node-client/src/platform/NodeRequests.ts +++ b/packages/sdk/node-client/src/platform/NodeRequests.ts @@ -13,6 +13,8 @@ import NodeResponse from './NodeResponse'; const gzip = promisify(zlib.gzip); +const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; + function processTlsOptions(tlsOptions: LDTLSOptions): https.AgentOptions { const options: https.AgentOptions & { [index: string]: any } = { ca: tlsOptions.ca, @@ -76,7 +78,7 @@ export default class NodeRequests implements platform.Requests { const req = impl.request( url, { - timeout: options.timeout, + timeout: options.timeout ?? DEFAULT_REQUEST_TIMEOUT_MS, headers, method: options.method, agent: this._agent, diff --git a/packages/sdk/node-client/src/platform/NodeResponse.ts b/packages/sdk/node-client/src/platform/NodeResponse.ts index 63615bbf8a..52bb134f38 100644 --- a/packages/sdk/node-client/src/platform/NodeResponse.ts +++ b/packages/sdk/node-client/src/platform/NodeResponse.ts @@ -6,14 +6,25 @@ import { platform } from '@launchdarkly/js-client-sdk-common'; import HeaderWrapper from './HeaderWrapper'; +// Upper bound on a buffered response body. Flag and event responses are far smaller than this; +// the cap prevents a misbehaving or hostile endpoint from exhausting memory with a huge body. +const MAX_RESPONSE_BYTES = 100 * 1024 * 1024; + export default class NodeResponse implements platform.Response { incomingMessage: http.IncomingMessage; chunks: any[] = []; + private _totalBytes: number = 0; + memoryStream: Writable = new Writable({ decodeStrings: true, write: (chunk, _enc, next) => { + this._totalBytes += chunk.length; + if (this._totalBytes > MAX_RESPONSE_BYTES) { + next(new Error(`Response body exceeded maximum size of ${MAX_RESPONSE_BYTES} bytes`)); + return; + } this.chunks.push(chunk); next(); }, diff --git a/packages/sdk/node-client/src/platform/NodeStorage.ts b/packages/sdk/node-client/src/platform/NodeStorage.ts index 264e37301d..a4c1ebee8e 100644 --- a/packages/sdk/node-client/src/platform/NodeStorage.ts +++ b/packages/sdk/node-client/src/platform/NodeStorage.ts @@ -46,10 +46,16 @@ 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 { + } catch (error) { + this._logger?.warn( + `Discarding malformed flag cache at ${this._storageFile}: ${error instanceof Error ? error.message : error}`, + ); await this._atomicWriteToFile(this._cache); } @@ -62,10 +68,26 @@ 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 { + // Ignore if temp file does not exist. + } + 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 { @@ -133,7 +155,13 @@ export default class NodeStorage implements Storage { await this._atomicWriteToFile(new Map(this._cache)); }); - this._flushQueue = flush.catch(() => {}); + // Batched callers chain off _flushQueue; log here so a failed write is never silently + // masked for callers that did not directly await this flush. + this._flushQueue = flush.catch((error) => { + this._logger?.error( + `Storage flush failed: ${error instanceof Error ? error.message : error}`, + ); + }); return flush; } } @@ -142,10 +170,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 +187,5 @@ export function getNodeStorage(storagePath?: string, logger?: LDLogger): NodeSto /** @internal Visible for testing only. */ export function resetNodeStorage(): void { instance = undefined; + instancePath = undefined; } From 8203c6391a501f4c7ba11a2359f4cc6fbc0867e4 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 1 Jun 2026 12:56:00 -0400 Subject: [PATCH 2/7] test: Add coverage for Node platform hardening Pins the new hardening behavior: - NodeStorage: warn on malformed cache, non-string cache values ignored, symlink-safe temp write, localStoragePath-mismatch warning. - NodeResponse: rejects when the body exceeds the size cap (exports MAX_RESPONSE_BYTES so the test references the exact threshold). Co-Authored-By: Claude Opus 4.7 --- .../__tests__/platform/NodeResponse.test.ts | 9 +++- .../__tests__/platform/NodeStorage.test.ts | 53 +++++++++++++++++++ .../node-client/src/platform/NodeResponse.ts | 2 +- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts b/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts index fd33d45d2f..004b2d5598 100644 --- a/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts +++ b/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts @@ -2,7 +2,7 @@ import * as http from 'http'; import { Readable } from 'stream'; import * as zlib from 'zlib'; -import NodeResponse from '../../src/platform/NodeResponse'; +import NodeResponse, { MAX_RESPONSE_BYTES } from '../../src/platform/NodeResponse'; function makeIncomingMessage( body: Buffer | string, @@ -51,6 +51,13 @@ it('decodes a gzip-encoded body', async () => { await expect(res.text()).resolves.toBe('compressed payload'); }); +it('rejects when the response body exceeds the maximum size', async () => { + // One chunk just over the cap trips the limit on the first write, so nothing is buffered. + const oversized = Buffer.allocUnsafe(MAX_RESPONSE_BYTES + 1); + const res = new NodeResponse(makeIncomingMessage(oversized)); + await expect(res.text()).rejects.toThrow(/exceeded maximum size/); +}); + it('rejects text() when the pipeline encounters an error', async () => { const erroring = new Readable({ read() { diff --git a/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts b/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts index 84d34aa693..9364ca27ec 100644 --- a/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts +++ b/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts @@ -70,6 +70,48 @@ it('recovers when the storage file contains invalid JSON', async () => { await expect(storage.get('alpha')).resolves.toBe('one'); }); +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 +146,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/NodeResponse.ts b/packages/sdk/node-client/src/platform/NodeResponse.ts index 52bb134f38..04758346a8 100644 --- a/packages/sdk/node-client/src/platform/NodeResponse.ts +++ b/packages/sdk/node-client/src/platform/NodeResponse.ts @@ -8,7 +8,7 @@ import HeaderWrapper from './HeaderWrapper'; // Upper bound on a buffered response body. Flag and event responses are far smaller than this; // the cap prevents a misbehaving or hostile endpoint from exhausting memory with a huge body. -const MAX_RESPONSE_BYTES = 100 * 1024 * 1024; +export const MAX_RESPONSE_BYTES = 100 * 1024 * 1024; export default class NodeResponse implements platform.Response { incomingMessage: http.IncomingMessage; From 60da3e642ac2b2a27a9fe6e26433e9373b24b0d4 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 1 Jun 2026 13:33:57 -0400 Subject: [PATCH 3/7] chore: bot comments --- .../__tests__/platform/NodeStorage.test.ts | 17 +++++++++++++++++ .../sdk/node-client/src/platform/NodeStorage.ts | 10 ++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts b/packages/sdk/node-client/__tests__/platform/NodeStorage.test.ts index 9364ca27ec..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,23 @@ 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'); diff --git a/packages/sdk/node-client/src/platform/NodeStorage.ts b/packages/sdk/node-client/src/platform/NodeStorage.ts index a4c1ebee8e..d01328d22b 100644 --- a/packages/sdk/node-client/src/platform/NodeStorage.ts +++ b/packages/sdk/node-client/src/platform/NodeStorage.ts @@ -53,10 +53,12 @@ export default class NodeStorage implements Storage { this._cache = new Map(entries); } } catch (error) { - this._logger?.warn( - `Discarding malformed flag cache at ${this._storageFile}: ${error instanceof Error ? error.message : error}`, - ); - await this._atomicWriteToFile(this._cache); + 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); + } } return true; From f87b76d2479dbc858fd404f07679038e546f00b2 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 1 Jun 2026 13:56:12 -0400 Subject: [PATCH 4/7] chore: bot comment --- packages/sdk/node-client/src/platform/NodeStorage.ts | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/packages/sdk/node-client/src/platform/NodeStorage.ts b/packages/sdk/node-client/src/platform/NodeStorage.ts index d01328d22b..14ad8ba777 100644 --- a/packages/sdk/node-client/src/platform/NodeStorage.ts +++ b/packages/sdk/node-client/src/platform/NodeStorage.ts @@ -157,13 +157,7 @@ export default class NodeStorage implements Storage { await this._atomicWriteToFile(new Map(this._cache)); }); - // Batched callers chain off _flushQueue; log here so a failed write is never silently - // masked for callers that did not directly await this flush. - this._flushQueue = flush.catch((error) => { - this._logger?.error( - `Storage flush failed: ${error instanceof Error ? error.message : error}`, - ); - }); + this._flushQueue = flush.catch(() => {}); return flush; } } From 696e0852c60ba6621ef3ab3e24945567938bdeca Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 1 Jun 2026 17:00:55 -0400 Subject: [PATCH 5/7] chore: remove timeout from NodeRequests This is a larger problem that needs to be resolved in a separate PR --- packages/sdk/node-client/src/platform/NodeRequests.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/sdk/node-client/src/platform/NodeRequests.ts b/packages/sdk/node-client/src/platform/NodeRequests.ts index bdc3633990..1ed819e94d 100644 --- a/packages/sdk/node-client/src/platform/NodeRequests.ts +++ b/packages/sdk/node-client/src/platform/NodeRequests.ts @@ -13,8 +13,6 @@ import NodeResponse from './NodeResponse'; const gzip = promisify(zlib.gzip); -const DEFAULT_REQUEST_TIMEOUT_MS = 30_000; - function processTlsOptions(tlsOptions: LDTLSOptions): https.AgentOptions { const options: https.AgentOptions & { [index: string]: any } = { ca: tlsOptions.ca, @@ -78,7 +76,7 @@ export default class NodeRequests implements platform.Requests { const req = impl.request( url, { - timeout: options.timeout ?? DEFAULT_REQUEST_TIMEOUT_MS, + timeout: options.timeout, headers, method: options.method, agent: this._agent, From 2e0e11a66a1bd6b587f247cd68e82f62a40c8022 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 1 Jun 2026 17:21:39 -0400 Subject: [PATCH 6/7] chore: reverting response body limit this is not standard for our sdks --- .../__tests__/platform/NodeResponse.test.ts | 9 +-------- packages/sdk/node-client/src/platform/NodeResponse.ts | 11 ----------- 2 files changed, 1 insertion(+), 19 deletions(-) diff --git a/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts b/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts index 004b2d5598..fd33d45d2f 100644 --- a/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts +++ b/packages/sdk/node-client/__tests__/platform/NodeResponse.test.ts @@ -2,7 +2,7 @@ import * as http from 'http'; import { Readable } from 'stream'; import * as zlib from 'zlib'; -import NodeResponse, { MAX_RESPONSE_BYTES } from '../../src/platform/NodeResponse'; +import NodeResponse from '../../src/platform/NodeResponse'; function makeIncomingMessage( body: Buffer | string, @@ -51,13 +51,6 @@ it('decodes a gzip-encoded body', async () => { await expect(res.text()).resolves.toBe('compressed payload'); }); -it('rejects when the response body exceeds the maximum size', async () => { - // One chunk just over the cap trips the limit on the first write, so nothing is buffered. - const oversized = Buffer.allocUnsafe(MAX_RESPONSE_BYTES + 1); - const res = new NodeResponse(makeIncomingMessage(oversized)); - await expect(res.text()).rejects.toThrow(/exceeded maximum size/); -}); - it('rejects text() when the pipeline encounters an error', async () => { const erroring = new Readable({ read() { diff --git a/packages/sdk/node-client/src/platform/NodeResponse.ts b/packages/sdk/node-client/src/platform/NodeResponse.ts index 04758346a8..63615bbf8a 100644 --- a/packages/sdk/node-client/src/platform/NodeResponse.ts +++ b/packages/sdk/node-client/src/platform/NodeResponse.ts @@ -6,25 +6,14 @@ import { platform } from '@launchdarkly/js-client-sdk-common'; import HeaderWrapper from './HeaderWrapper'; -// Upper bound on a buffered response body. Flag and event responses are far smaller than this; -// the cap prevents a misbehaving or hostile endpoint from exhausting memory with a huge body. -export const MAX_RESPONSE_BYTES = 100 * 1024 * 1024; - export default class NodeResponse implements platform.Response { incomingMessage: http.IncomingMessage; chunks: any[] = []; - private _totalBytes: number = 0; - memoryStream: Writable = new Writable({ decodeStrings: true, write: (chunk, _enc, next) => { - this._totalBytes += chunk.length; - if (this._totalBytes > MAX_RESPONSE_BYTES) { - next(new Error(`Response body exceeded maximum size of ${MAX_RESPONSE_BYTES} bytes`)); - return; - } this.chunks.push(chunk); next(); }, From a00473eb1ba93684c1646448e4fd908884ccc70c Mon Sep 17 00:00:00 2001 From: joker23 <2494686+joker23@users.noreply.github.com> Date: Mon, 1 Jun 2026 18:55:31 -0400 Subject: [PATCH 7/7] Update packages/sdk/node-client/src/platform/NodeStorage.ts Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> --- packages/sdk/node-client/src/platform/NodeStorage.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/sdk/node-client/src/platform/NodeStorage.ts b/packages/sdk/node-client/src/platform/NodeStorage.ts index 14ad8ba777..26b9fc1320 100644 --- a/packages/sdk/node-client/src/platform/NodeStorage.ts +++ b/packages/sdk/node-client/src/platform/NodeStorage.ts @@ -75,7 +75,10 @@ export default class NodeStorage implements Storage { try { await fs.unlink(this._tempFile); } catch { - // Ignore if temp file does not exist. + // 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');