Address all known vulnerabilities (4.0.0) by aarsilv · Pull Request #118 · Eppo-exp/react-native-sdk

aarsilv · 2026-05-25T22:23:50Z

Vulnerability counts — both clean

Tool	Before	After
`yarn audit` (root)	19 unique (moderate + high)	0
`yarn audit` (`example/`)	155 (44 moderate / 111 high)	0

Strategy

Direct dep / major-version bumps over resolutions wherever possible. Pattern follows @eppo/js-client-sdk-common@5.0.0: bump the common SDK itself, modernize the toolchain, and drop unused devDeps that drag in vulnerable transitives.

SDK bumped to 4.0.0 because engines.node tightens to >=20.0.0 (mirroring js-client-sdk-common@5.0.0). Public peer ranges (react, react-native) stay *.

Only two new resolutions, both in example/ (postcss, uuid) — transitives reachable only via expo SDK 54. example/ is private, so they don't propagate to SDK consumers. The root package.json ends up with zero resolutions.

Per-file inline comments on this PR explain each non-trivial change.

Test plan

yarn audit clean in root (0 / 753 packages) and example/ (0 / 1003 packages).
yarn typecheck, yarn lint, yarn test (38/38), yarn prepack all green.
CI green on the new Node 20 matrix (build, lint, test).
Runtime test in example/: ran expo start --web against this branch, then loaded it in headless Chrome with the real Eppo SDK key in EXPO_PUBLIC_EPPO_API_KEY. Both My-Flag and onboarding-flow resolved real assignments (control, not the default-value fallback) — confirms init(), network fetch, and getStringAssignment work end-to-end on the new dep stack.

🤖 Generated with Claude Code

## Vulnerability counts — both clean | Tool | Before | After | | ------------------- | ---------------------------- | ----- | | `yarn audit` (root) | 19 unique (moderate + high) | 0 | | `yarn audit` (example) | 155 (44 moderate / 111 high) | 0 | ## Approach Followed the pattern from `@eppo/js-client-sdk-common@5.0.0`: bump direct deps (taking major versions where reasonable) and drop unused deps that were dragging in vulnerable transitives. Only two new `resolutions` entries — both in `example/` for transitives buried under `expo` that have no path-through fix at SDK 54. ## Production dep changes - `@eppo/js-client-sdk-common` 4.15.1 → 5.0.0 - The 5.0.0 release of the common SDK was cut to clear vulnerabilities — it drops `uuid` entirely (replaced with `crypto.randomUUID`), which kills the only production vuln in this repo. - The matching `engines.node` floor (>=18 → >=20) is mirrored here, driving this SDK's major bump. - `@react-native-async-storage/async-storage` ^1.18.0 → ^2.2.0 (the surface we use — `getItem`/`setItem`/`removeItem` — is unchanged). ## Dev dep cleanup Removed entirely (unused — these were the source of the bulk of the audit findings): - `express` + `@types/express` — never imported in src/, scripts/, or test setup. Source of `path-to-regexp` and `qs` advisories. - `del-cli`, `pod-install` — declared but not invoked from any script. - `@types/react-native` — RN ships its own types since 0.71; `@types/react-native` is officially a deprecated stub. - `@react-native-community/eslint-config` — pulled in old `@typescript-eslint` v5, `eslint-plugin-prettier@^4`, `babel/eslint-parser`, etc., all of which carried vulnerable transitives. The only rule we actually relied on was `prettier/prettier`. Replaced with a minimal `eslint:recommended` + `@typescript-eslint/recommended` + `prettier` setup with the same prettier options. - `metro-react-native-babel-preset` (via babel.config.js) — replaced with `@react-native/babel-preset` (the RN 0.73+ standard, ships with the RN devDep). Bumped: - `react-native` 0.71.3 → 0.85.3 + `react` 18.2 → 19.2 + `@types/react` 17 → 19 (devDeps only — the published peer range stays at `*`). - `@react-native/{babel-preset,jest-preset}` 0.85.3 added explicitly so the jest preset reference resolves and we can drop the deprecated built-in `react-native` preset. - `jest` 29 → 30, `@types/jest` 28 → 30 — clears the `glob`/`picomatch` chain. - `@typescript-eslint/{eslint-plugin,parser}` added at 8.59.1 — clears the rest of the `minimatch` chain. - `eslint-config-prettier` 8 → 10, `eslint-plugin-prettier` 4 → 5, `prettier` 2 → 3. - `typescript` 4.9 → 5.9 — required for `verbatimModuleSyntax` (replacement for the now-removed `importsNotUsedAsValues`). - `react-native-builder-bob` 0.20 → 0.41 — config syntax is compatible; it now reads babel via `@react-native/babel-preset`. ## Source/config touch-ups driven by the toolchain bumps - `tsconfig.json`: `importsNotUsedAsValues: "error"` (removed in TS 5.5) → `verbatimModuleSyntax: true`. - `src/index.tsx`, `src/__tests__/index.test.tsx`, `src/cache/simple-assignment-cache.ts`: split mixed type/value imports so type-only imports use `import type` (required by `verbatimModuleSyntax`); same treatment for type-only re-exports in `src/index.tsx`. - `src/cache/async-storage-assignment-shim.ts`: dropped explicit `IterableIterator<…>` return types on `entries`/`keys`/`values`/ `[Symbol.iterator]`. TS 5.x lib types declare `Map`'s versions as `MapIterator<…>`; inferring the return type lets the override match the base class without us hard-coding the new name. - ESLint config: existing call sites in `check-types.js`, `scripts/bootstrap.js`, `jestSetup.js`, `src/sdk-data.ts` use CJS `require()` (legitimately — these run as Node scripts under `metro-react-native-babel-preset` historically). Turned off `@typescript-eslint/no-require-imports` and `no-var`, kept `no-explicit-any` off (the codebase has a few intentional `any`s), and ignored `example/` from root lint since Expo runs its own ESLint config there. ## Example app - Added two `resolutions`: `postcss@^8.5.10` and `uuid@^11.1.1`. Both are transitives reachable only via the `expo` package (postcss via `@expo/metro-config`, uuid via `@expo/config-plugins > xcode`). No expo SDK 54.x patch ships either at a fixed version yet, and bumping the entire Expo SDK family (54 → 56) for a demo app is more risk than the targeted resolutions. These resolutions don't propagate to SDK consumers since `example/` is a private, non-published workspace. ## Engines / CI / .nvmrc - `engines.node` `>=18.0.0` → `>=20.0.0`. - `.nvmrc` `16.20.2` → `20`. - CI (`ci.yml`, `publish.yaml`) `node-version: '18'`/`'18.x'` → `'20'`/ `'20.x'`. ## Version bump Treating this as **4.0.0** because of the `engines.node` floor change (>=18 → >=20), matching the SemVer-major precedent from `@eppo/js-client-sdk-common@5.0.0` and `@eppo/node-server-sdk@4.0.0`. Public TS surface and runtime behavior are unchanged. ## Test plan - [x] `yarn audit` clean in root (0 / 753 packages). - [x] `yarn audit` clean in `example/` (0 / 1003 packages). - [x] `yarn typecheck` passes. - [x] `yarn test`: 38 / 38 pass across 2 suites. - [x] `yarn lint`: 0 errors after auto-fix. - [x] `yarn prepack` (bob build + check-types): commonjs, module, and typescript targets all written. - [x] `npx expo export` succeeds for `web`, `ios`, and `android` platforms in `example/`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The reusable setup action in .github/actions/setup/action.yml had its own hard-coded `node-version: '18.x'`, which overrode the `'20'` set in the job-level setup-node step earlier in ci.yml. The first install failed on `engines.node: ">= 20.0.0"` because the composite reinstalled Node 18 right after. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR upgrades the React Native SDK’s dependency/toolchain stack to address all known vulnerabilities, including a major version bump driven by raising the Node.js engine requirement to >=20.

Changes:

Bump production dependencies (notably @eppo/js-client-sdk-common to v5 and AsyncStorage to v2) and modernize the toolchain (TS 5.9, Jest 30, ESLint/Prettier updates).
Update TS/SDK source to be compatible with verbatimModuleSyntax (type-only imports/exports) and adjust small type-signature details.
Move CI, publish workflow, and local Node tooling to Node 20; update Babel/Jest presets accordingly; add targeted example/ resolutions.

Reviewed changes

Copilot reviewed 12 out of 15 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`tsconfig.json`	Switch to `verbatimModuleSyntax` for TS 5.x compatibility.
`src/index.tsx`	Split mixed value/type imports and use `export type` for type-only exports.
`src/cache/simple-assignment-cache.ts`	Convert type-only import to `import type` to satisfy TS verbatim module syntax.
`src/cache/async-storage-assignment-shim.ts`	Let iterator return types be inferred to match updated TS lib typings.
`src/async-storage.ts`	Formatting/type layout adjustment for TS/lint compatibility.
`src/__tests__/index.test.tsx`	Split value vs type imports (`VariationType` vs type-only symbols).
`package.json`	Major version bump, dependency/toolchain upgrades, Node engine bump, Jest preset and ESLint config updates.
`example/package.json`	Minor Expo bump plus targeted `resolutions` to eliminate remaining transitive vulnerabilities.
`babel.config.js`	Switch to `@react-native/babel-preset`.
`.nvmrc`	Update local Node version target to 20.
`.github/workflows/publish.yaml`	Publish workflow now runs on Node 20.x.
`.github/workflows/ci.yml`	CI jobs updated to Node 20.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

aarsilv · 2026-05-25T22:45:44Z

        with:
-          node-version: '18'
+          node-version: '20'

      - name: Setup
        uses: ./.github/actions/setup


🤖 response from AI: Good catch — already fixed in commit 34f494f. The composite action .github/actions/setup/action.yml had a hard-coded node-version: '20.x' after the bump (was '18.x'). CI is now green on the latest commit: build, lint, and test all pass.

aarsilv · 2026-05-25T22:45:45Z

        with:
-          node-version: '18'
+          node-version: '20'

      - name: Setup
        uses: ./.github/actions/setup


🤖 response from AI: Good catch — already fixed in commit 34f494f. The composite action .github/actions/setup/action.yml had a hard-coded node-version: '20.x' after the bump (was '18.x'). CI is now green on the latest commit: build, lint, and test all pass.

aarsilv · 2026-05-25T22:45:46Z

        with:
-          node-version: '18'
+          node-version: '20'

      - name: Setup
        uses: ./.github/actions/setup


🤖 response from AI: Good catch — already fixed in commit 34f494f. The composite action .github/actions/setup/action.yml had a hard-coded node-version: '20.x' after the bump (was '18.x'). CI is now green on the latest commit: build, lint, and test all pass.

Copilot

Pull request overview

Copilot reviewed 12 out of 15 changed files in this pull request and generated 1 comment.

aarsilv · 2026-05-25T22:57:51Z

+          "varsIgnorePattern": "^_"
+        }
+      ],
+      "no-var": "off"


🤖 response from AI: Acted on this — re-enabled no-var and converted the single var key in entries in src/async-storage.ts to const key in commit (pending push). Lint, typecheck, and tests all still pass.

Copilot suggested keeping `no-var` enabled to prevent stray `var` declarations from slipping into library code. Repo only had one `var` (in a `for...in` loop in `src/async-storage.ts`), so converted that to `const` and dropped the rule override. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 15 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/async-storage.ts:48

AsyncStorage.setItem() returns a Promise but it isn’t awaited or given a .catch() handler here. If the write fails, this can surface as an unhandled promise rejection (and the cache will be marked initialized regardless). Consider attaching a rejection handler (e.g., .catch(...)) or otherwise explicitly handling failures so storage errors don’t crash/log noisily in production.

  public setEntries(entries: Record<string, T>): void {
    for (const key in entries) {
      this.cache[key] = entries[key];
    }

    AsyncStorage.setItem(STORAGE_KEY, JSON.stringify(this.cache));
    this._isInitialized = true;

.github/actions/setup/action.yml:20

The composite action caches **/node_modules keyed only by OS + yarn.lock. Since this action now pins Node 20.x, restoring a cache created under a different Node version can produce incompatible native/addon binaries and the install step is skipped on cache hit. Consider including the Node version in the cache key (or caching Yarn’s cache instead of node_modules) so dependency artifacts are rebuilt when the Node runtime changes.

    - name: Setup Node.js
      uses: actions/setup-node@v3
      with:
        node-version: '20.x'  # Specify the Node.js version directly here

    - name: Cache dependencies
      id: yarn-cache
      uses: actions/cache@v3
      with:
        path: |
          **/node_modules
        key: ${{ runner.os }}-yarn-${{ hashFiles('**/yarn.lock') }}
        restore-keys: |
          ${{ runner.os }}-yarn-

aarsilv · 2026-05-25T23:14:22Z

🤖 response from AI: Copilot's third pass had no new high-confidence comments. The two low-confidence observations it flagged:

src/async-storage.ts:48 — un-awaited AsyncStorage.setItem() — pre-existing behavior in the file (untouched by this PR). The non-blocking write is intentional for the in-memory serving path; addressing it properly would be a behavior change worth a separate ticket so we can think about retry/error semantics for the persistent cache.
.github/actions/setup/action.yml:20 — cache key doesn't include Node version — also pre-existing, but worth noting that the lockfile churn in this PR forces a cache miss anyway (key includes hashFiles('**/yarn.lock')), so we won't restore an old Node-18-built node_modules here. Tightening the key to include Node version is a sensible follow-up but not vuln-blocking.

Both are valid but out of scope for this PR. CI is fully green (build, lint, test) and yarn audit is clean in both root and example/. Closing the iteration loop.

aarsilv · 2026-05-26T00:37:34Z

 {
  "name": "@eppo/react-native-sdk",
-  "version": "3.8.0",
+  "version": "4.0.0",


🤖 context from AI: SemVer-major because engines.node tightens to >=20. The bulk of this diff is dropping unused devDeps (express, @types/express, del-cli, pod-install, @types/react-native) and replacing the @react-native-community/eslint-config stack with a minimal @typescript-eslint + prettier setup — both of which were the main sources of audit findings.

aarsilv · 2026-05-26T00:37:35Z

+    "eslint-config-expo": "~10.0.0",
+    "typescript": "~5.9.2"
+  },
+  "resolutions": {


🤖 context from AI: Two targeted resolutions clear the last vulns (postcss, uuid) reachable only via expo SDK 54 transitives. example/ is private, so they do not propagate to SDK consumers.

aarsilv · 2026-05-26T00:37:36Z

    "allowUnusedLabels": false,
    "esModuleInterop": true,
-    "importsNotUsedAsValues": "error",
+    "verbatimModuleSyntax": true,


🤖 context from AI: importsNotUsedAsValues was removed in TS 5.5; verbatimModuleSyntax is the modern equivalent — drove the import type splits in src/.

aarsilv · 2026-05-26T00:37:36Z

@@ -1,15 +1,17 @@
 import {


🤖 context from AI: Split mixed value/type imports (and re-exports below) so type-only symbols use import type / export type, required by verbatimModuleSyntax.

aarsilv · 2026-05-26T00:37:37Z

 import type { EppoAsyncStorage } from 'src/async-storage';
-import {
-  VariationType,
+import { VariationType } from '@eppo/js-client-sdk-common';


🤖 context from AI: Same verbatimModuleSyntax split as in src/index.tsx.

aarsilv · 2026-05-26T00:37:38Z

  NonExpiringInMemoryAssignmentCache,
-  AssignmentCacheEntry,
 } from '@eppo/js-client-sdk-common';
+import type { AssignmentCacheEntry } from '@eppo/js-client-sdk-common';


🤖 context from AI: AssignmentCacheEntry moved to its own import type line for verbatimModuleSyntax.

aarsilv · 2026-05-26T00:37:39Z

  }

-  entries(): IterableIterator<[string, string]> {
+  entries() {


🤖 context from AI: TS 5.x lib declares Map iterators as MapIterator<…> (not IterableIterator<…>). Inferring the return type lets the override match the base class without hard-coding the new type name.

aarsilv · 2026-05-26T00:37:40Z


  public setEntries(entries: Record<string, T>): void {
-    for (var key in entries) {
+    for (const key in entries) {


🤖 context from AI: Sole var in src/; switched to const per Copilot second-pass review (rule re-enabled in package.json).

aarsilv · 2026-05-26T00:37:41Z

      uses: actions/setup-node@v3
      with:
-        node-version: '18.x'  # Specify the Node.js version directly here
+        node-version: '20.x'  # Specify the Node.js version directly here


🤖 context from AI: Composite action was overriding the job-level Node 20 back to 18, breaking the engines.node check — caught in the first CI run.

aarsilv requested a review from Copilot May 25, 2026 22:23

Copilot started reviewing on behalf of aarsilv May 25, 2026 22:24 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

aarsilv requested a review from Copilot May 25, 2026 22:46

Copilot started reviewing on behalf of aarsilv May 25, 2026 22:46 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

aarsilv requested a review from Copilot May 25, 2026 22:58

Copilot started reviewing on behalf of aarsilv May 25, 2026 22:58 View session

Copilot AI reviewed May 25, 2026

View reviewed changes

aarsilv commented May 26, 2026

View reviewed changes

aarsilv requested a review from greghuels May 26, 2026 01:22

aarsilv assigned greghuels May 26, 2026

This was referenced May 26, 2026

Catch unhandled rejection on fire-and-forget AsyncStorage.setItem #119

Merged

Include Node version in CI setup cache key #120

Merged

greghuels approved these changes May 26, 2026

View reviewed changes

aarsilv merged commit a7ca097 into main May 26, 2026
4 checks passed

aarsilv deleted the aarsilv/ffesupport-728/address-vulnerabilities branch May 26, 2026 11:59

Conversation

aarsilv commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Vulnerability counts — both clean

Strategy

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

aarsilv commented May 25, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

aarsilv commented May 25, 2026 •

edited

Loading