chore(node-client): harden existing platform implementation#1403
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
|
@cursor review |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/browser size report |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
@cursor review |
|
@cursor review |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| try { | ||
| await fs.writeFile(this._tempFile, content, { encoding: 'utf8', mode: 0o600 }); | ||
| try { | ||
| await fs.unlink(this._tempFile); |
There was a problem hiding this comment.
I assume the file being missing isn't the only reason it cannot be deleted. For example if it was created by a different user or with different permissions.
There was a problem hiding this comment.
yea that would also cause issues, but the exception will be caught further down.
This is a larger problem that needs to be resolved in a separate PR
this is not standard for our sdks
| await handle.close(); | ||
| handle = undefined; | ||
| await fs.rename(this._tempFile, this._storageFile); | ||
| } catch (error) { |
There was a problem hiding this comment.
It does feel like we probably should log something in this error handler.
Co-authored-by: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
joker23
deleted the
skz/sdk-2195/node-client-sdk-next-port-client-harden-platform-io
branch
2 participants
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.
wxopen so the write cannot be redirected through a symlink; warn whengetNodeStorageis called with alocalStoragePaththat differs from the process singleton's.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.
NodeStoragenow 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 exclusivewxtemp open (after removing any existing temp path) so writes cannot follow a symlink planted on the temp file path.getNodeStoragerecords the firstlocalStoragePathand warns if a later call passes a different path;resetNodeStorageclears that path tracking too.Tests cover first-run behavior, malformed JSON warnings, non-string cache entries, symlink safety, singleton path mismatch warnings, and
NodeInfono longer pins the SDK version string in expectations.Reviewed by Cursor Bugbot for commit 1962f54. Bugbot is set up for automated code reviews on this repo. Configure here.