Skip to content

Address all known vulnerabilities#124

Merged
aarsilv merged 3 commits into
mainfrom
aarsilv/address-vulnerabilities
May 12, 2026
Merged

Address all known vulnerabilities#124
aarsilv merged 3 commits into
mainfrom
aarsilv/address-vulnerabilities

Conversation

@aarsilv
Copy link
Copy Markdown
Contributor

@aarsilv aarsilv commented May 6, 2026

See commit 7dac7b2 for full per-package detail and verification notes. Summary:

  • yarn audit: 112 paths → 0 (was 2 Critical / 59 High / 44 Moderate / 7 Low).
  • Resolutions trimmed from 12 → 2. The previous round had scoped overrides for minimatch, brace-expansion, diff, flatted, lodash, path-to-regexp, picomatch, qs, plus schema-utils/ajv and ajv-formats/ajv. After the dep bumps below, every one of those clears transitively. The two that remain in package.json:
    • @types/node ^20.0.0 — intentional pin to keep the dev type surface aligned with engines.node >=20.x. Not a vuln fix.
    • @tootallnate/once ^3.0.1 — GHSA-rj4j-rrv4-3xc4 (low). Pulled by @google-cloud/storageteeny-requesthttp-proxy-agent. The latest @google-cloud/storage 7.19.x still ships the unpatched chain and there's no newer release; the chain is dev-only (test-fixture downloader).
  • Direct dep bumps: @eppo/js-client-sdk-common 4.15.2 → ^5 (drops uuid runtime dep, replaced upstream with crypto.randomUUID for event IDs), express ^4 → ^5 (drops vulnerable path-to-regexp 0.x and qs <6.15 transitively without needing scoped resolutions; express is a devDep used only by the mock API server in tests, where the simple app.get(regex, handler) usage works the same in v5), @typescript-eslint/* 5 → 8, eslint-config-prettier 8 → 10, eslint-plugin-promise 6 → 7, husky 6 → 9, lint-staged 12 → 16, prettier 2 → 3 (drops the yaml@1.x transitive that triggered the yaml advisory), @google-cloud/storage to latest 7.x.
  • Two small source-side adjustments forced by the typescript-eslint v8 rule renames: bare catch for an unused catch (error) parameter in src/util/index.ts, and replacing deprecated no-var-requires disable directives with no-require-imports.

Test plan

  • CI runs yarn lint, yarn typecheck, yarn test (115 tests) green on Node 20/22/24
  • yarn audit reports 0 vulnerabilities
  • yarn tsc (build) succeeds; dist/ exports unchanged

Manual verification

Built a tiny toy node app that consumes this branch's SDK via a file: reference and calls init(...).getStringAssignment(...) against an Eppo Sandbox account. Resolved My-Flag (with attrs {accountId, sd:'dsf'} to hit the 50/50 allocation) and megan-test-blahh for 12 distinct subject keys, comparing against the same 12 subjects in a parallel toy browser app on js-client-sdk PR #284's branch. All 12 subjects produced identical variation assignments across the two SDK branches, distribution 8 treatment / 4 control, assignment-logger fired correctly on both sides.

🤖 Generated with Claude Code

Pattern follows the visual-editor / sdk-test-data / js-client-sdk
cleanup: bump direct deps to latest within their compatible major (or
to a known-fixed major) so audit findings clear transitively. Yarn
`resolutions` are kept to a minimum — only two remain in this round.

yarn audit: 112 paths → 0 (was 2 Critical / 59 High / 44 Moderate / 7 Low).

Direct dep bumps:
- @eppo/js-client-sdk-common 4.15.2 → ^5  — drops the uuid runtime dep
  entirely (replaced upstream with crypto.randomUUID for event IDs),
  clearing the uuid moderate advisory and most of the deep transitive
  vulns the earlier `resolutions` block was working around.
- express ^4 → ^5 (drops vulnerable path-to-regexp 0.x and qs <6.15
  transitively without needing scoped resolutions; express is a devDep
  used only by the mock API server in tests, and the simple
  `app.get(regexp, handler)` usage there works the same in v5)
- @microsoft/api-documenter, @microsoft/api-extractor → latest
- @types/jest 29 → 30 (paired with jest 30)
- @typescript-eslint/{eslint-plugin,parser} 5 → 8 (rule renames forced
  the lint fixes below)
- eslint-config-prettier 8 → 10
- eslint-import-resolver-typescript 2 → 4
- eslint-plugin-prettier 4 → 5
- eslint-plugin-promise 6 → 7
- husky 6 → 9 (current major; old 6.x was years EOL)
- jest stays on 29 (jest-environment-jsdom isn't used here)
- lint-staged 12 → 16 (17 needs Node 22; we still target Node 20)
- prettier 2 → 3 (drops the `yaml@1.x` transitive that triggered the
  yaml advisory)
- @google-cloud/storage to latest 7.x

Resolutions left in package.json (each annotated inline):
- @types/node ^20.0.0 — intentional pin to keep the dev type surface
  aligned with engines.node >=20.x. Not a vuln fix; carried over from
  the previous resolutions block.
- @tootallnate/once ^3.0.1 — GHSA-rj4j-rrv4-3xc4 (low). Pulled by
  @google-cloud/storage → teeny-request → http-proxy-agent. The latest
  @google-cloud/storage 7.19.x still ships the unpatched chain and
  there's no newer release; the chain is dev-only.

Everything else from the previous round (minimatch, brace-expansion,
diff, flatted, lodash, path-to-regexp, picomatch, qs, schema-utils/ajv,
ajv-formats/ajv) was dropped — the direct dep bumps above clear them
transitively.

Source-code adjustments forced by the dep bumps:
- src/util/index.ts: unused `catch (error)` parameter → bare `catch`
  (typescript-eslint v8's stricter `no-unused-vars`).
- src/sdk-data.ts and src/index.spec.ts: replace deprecated
  `@typescript-eslint/no-var-requires` disable directives with
  `no-require-imports`.

Verification:
- `yarn lint`, `yarn typecheck`, `yarn test` (115 tests),
  `yarn tsc` (build) all pass locally.
- `yarn audit` reports 0 vulnerabilities.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aarsilv aarsilv force-pushed the aarsilv/address-vulnerabilities branch from 5073d19 to 7dac7b2 Compare May 6, 2026 14:57
No public API change in this branch; the @eppo/js-client-sdk-common 4 →
5 dependency bump and devDep updates are all consumer-transparent and
engines.node was already >=20.x.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@aarsilv aarsilv marked this pull request as ready for review May 7, 2026 00:44
@aarsilv aarsilv requested a review from Copilot May 7, 2026 00:44
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR focuses on eliminating dependency vulnerabilities and modernizing the repo’s tooling/runtime dependencies, while making small source tweaks required by updated lint rules.

Changes:

  • Updated a large set of dependencies (including @eppo/js-client-sdk-common to ^5, express to ^5, tooling like prettier/eslint/@typescript-eslint/*) and refreshed yarn.lock.
  • Reduced the number of resolutions overrides, keeping only @types/node and @tootallnate/once.
  • Updated a few lint-related call sites (e.g., bare catch, updated ESLint disable rule name).

Reviewed changes

Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
yarn.lock Lockfile refresh reflecting dependency/toolchain upgrades and vulnerability remediation.
package.json Dependency/devDependency upgrades, version bump, and pared-down resolutions.
src/util/index.ts Removes unused catch binding to satisfy updated linting rules.
src/sdk-data.ts Updates ESLint disable directive for require(...) to the new rule name.
src/index.spec.ts Updates ESLint disable directives where require('.') is intentionally used in tests.

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

Comment thread package.json Outdated
Comment on lines +38 to +55
"@types/express": "^4",
"@types/jest": "^30",
"@typescript-eslint/eslint-plugin": "^8",
"@typescript-eslint/parser": "^8",
"eslint": "^8",
"eslint-config-prettier": "^10",
"eslint-import-resolver-typescript": "^4",
"eslint-plugin-import": "^2",
"eslint-plugin-prettier": "^5",
"eslint-plugin-promise": "^7",
"express": "^5",
"husky": "^9",
"jest": "^29",
"lint-staged": "^16",
"prettier": "^3",
"testdouble": "^3",
"ts-jest": "^29",
"typescript": "^5"
Comment thread package.json
Comment on lines +35 to +49
"@google-cloud/storage": "^7",
"@microsoft/api-documenter": "^7.30.5",
"@microsoft/api-extractor": "^7.58.7",
"@types/express": "^4",
"@types/jest": "^30",
"@typescript-eslint/eslint-plugin": "^8",
"@typescript-eslint/parser": "^8",
"eslint": "^8",
"eslint-config-prettier": "^10",
"eslint-import-resolver-typescript": "^4",
"eslint-plugin-import": "^2",
"eslint-plugin-prettier": "^5",
"eslint-plugin-promise": "^7",
"express": "^5",
"husky": "^9",
Comment thread package.json
Comment on lines 62 to +66
"resolutions": {
"minimatch": "^3.1.2",
"@types/node": "^20.0.0"
"//@types/node": "Pinned to ^20 to keep the dev type surface aligned with engines.node >=20.x; newer @types/node would let consumer code use APIs that aren't actually available on Node 20",
"@types/node": "^20.0.0",
"//@tootallnate/once": "GHSA-rj4j-rrv4-3xc4 (low). Pulled deeply by @google-cloud/storage → teeny-request → http-proxy-agent. The latest @google-cloud/storage (7.19.x) still ships the unpatched chain and there's no newer release; the chain is dev-only (test fixture downloader)",
"@tootallnate/once": "^3.0.1"
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From Claude: the published-package concern is unfounded — resolutions is not honored on installed packages. npm and yarn both deliberately ignore the resolutions field of any dependency in node_modules; only the field in the root project's package.json controls dependency resolution. So while these "//<key>" entries do get published into the package's package.json, they never affect a downstream consumer's install graph or behavior. They're essentially dead bytes for anyone who installs @eppo/node-server-sdk.

What's left after that is a developer-experience consideration: yarn does emit one warning per pseudo-comment key during yarn install here (Resolution field "//@types/node" does not end with a valid package name and will be ignored). The trade-off we picked, in line with visual-editor and sdk-test-data's vuln-cleanup PRs, is that having the GHSA reasoning inline next to each resolution is more useful for the next person who has to revisit them than the cost of two install-time warnings. Happy to convert these to comments above the block in a sibling RESOLUTIONS.md if there's a preference for that convention instead.

Two small follow-ups from PR review:

- jest ^29 → ^30, matching the @types/jest ^30 already on this branch
  and the jest 30 baseline used by js-client-sdk's parallel PR. ts-jest
  stays on ^29 because no 30.x has been published; ts-jest 29.4.x
  declares jest 30 as a supported peer (peerDependencies.jest:
  ^29.0.0 || ^30.0.0).
- @types/express ^4 → ^5, matching the express ^5 runtime bump from
  the prior commit. tsc stays clean.

Verification: yarn typecheck, yarn lint, yarn test (115 tests),
yarn audit all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@sameerank sameerank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving, but please consider re-adding the lower bounds on the version constraints

Comment thread package.json
"testdouble": "^3.16.4",
"ts-jest": "^29.4.6",
"typescript": "^5.4.5"
"@google-cloud/storage": "^7",
Copy link
Copy Markdown
Contributor

@sameerank sameerank May 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless we're intentionally allowing downgrades to older versions (I only see versions going up in the yarn.lock), why are we loosening the lower bounds on these versions, i.e. dropping the minimum minor/patch constraints?

@aarsilv aarsilv merged commit 81c5e0e into main May 12, 2026
6 checks passed
@aarsilv aarsilv deleted the aarsilv/address-vulnerabilities branch May 12, 2026 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants