Skip to content

[PAN-11212] release/2.9.0 fix: get npm audit --production to pass - #124

Merged
DanielStevenLewis merged 8 commits into
masterfrom
PAN-11212-fix-vulnerable-dependencies-v2
Apr 10, 2026
Merged

[PAN-11212] release/2.9.0 fix: get npm audit --production to pass#124
DanielStevenLewis merged 8 commits into
masterfrom
PAN-11212-fix-vulnerable-dependencies-v2

Conversation

@DanielStevenLewis

@DanielStevenLewis DanielStevenLewis commented Apr 1, 2026

Copy link
Copy Markdown
Contributor

PAN-11212

Also see #125

@DanielStevenLewis DanielStevenLewis changed the title PAN-11212 fix: get npm audit --production to pass PAN-11212 fix: get npm audit --production to pass Apr 1, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the SDK’s HTTP client dependencies to address security/audit findings by removing the deprecated request package and modernizing the axios/lodash versions.

Changes:

  • Replaced request with axios for auth token retrieval and API calls.
  • Updated tests to validate axios request configuration for auth token URL selection.
  • Updated dependency versions in package.json / package-lock.json (axios, lodash) and removed request.

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
lib/auth.js Migrates token request logic from request.post to axios.request.
lib/api.js Migrates generic API call logic from request(...) to axios(...).
test/auth.test.js Updates URL selection tests to spy on axios instead of request.
package.json Removes request, bumps axios and lodash versions.
package-lock.json Reflects dependency graph changes after removing request and bumping packages.

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

Comment thread lib/api.js
Comment thread lib/auth.js
Comment thread package.json
It appears that the remaining 40 vulnerabilities cannot be fixed
without breaking Node 10+ support or test compatibility,
and they don't impact the library's functionality or security for end users.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

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


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

Comment thread lib/auth.js Outdated
Comment thread package.json
Comment thread lib/api.js Outdated
DanielStevenLewis and others added 2 commits April 2, 2026 16:32
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@DanielStevenLewis
DanielStevenLewis marked this pull request as ready for review April 2, 2026 21:33
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Replace request library with axios to fix npm audit vulnerabilities

🐞 Bug fix ✨ Enhancement

Grey Divider

Walkthroughs

Description
• Replace deprecated request library with axios to fix npm audit vulnerabilities
• Update axios from 0.21.4 to 1.14.0 and lodash from 4.17.21 to 4.17.23
• Refactor API and auth modules to use axios promise-based API instead of callbacks
• Maintain backward compatibility by creating request-like objects for test compatibility
• Update dev dependencies (eslint, gh-pages, standard-version) to latest versions
Diagram
flowchart LR
  request["request library<br/>deprecated & vulnerable"]
  axios["axios library<br/>v1.14.0"]
  apiModule["lib/api.js<br/>refactored"]
  authModule["lib/auth.js<br/>refactored"]
  tests["test/auth.test.js<br/>updated"]
  packageJson["package.json<br/>dependencies updated"]
  
  request -->|"replaced with"| axios
  axios -->|"used in"| apiModule
  axios -->|"used in"| authModule
  apiModule -->|"tested by"| tests
  authModule -->|"tested by"| tests
  axios -->|"version bump"| packageJson
Loading

Grey Divider

File Changes

1. lib/api.js ✨ Enhancement +97/-62

Migrate from request to axios library

• Replace request library with axios for HTTP calls
• Convert callback-based request to promise-based axios with .then() and .catch()
• Refactor request options to axios config format (e.g., qsparams, bodydata)
• Create request-like objects for test compatibility with URL object for parsed URI properties
• Maintain all error handling logic and 401 token refresh behavior

lib/api.js


2. lib/auth.js ✨ Enhancement +108/-82

Migrate authentication from request to axios

• Replace request.post() with axios.request() for token authentication
• Convert callback-based implementation to promise-based with .then() and .catch()
• Refactor timeout and error handling to work with axios error codes (e.g., ECONNABORTED)
• Maintain retry logic and token expiration calculation
• Preserve all authentication error handling and emitter event firing

lib/auth.js


3. test/auth.test.js 🧪 Tests +11/-9

Update auth tests for axios migration

• Update test mocks from jest.spyOn(request, 'post') to jest.spyOn(axios, 'request')
• Change mock implementation from callback-based to promise-based with mockRejectedValue()
• Update test expectations to use toHaveBeenCalledWith() instead of toBeCalledWith()
• Add mockRestore() calls to clean up spies after each test

test/auth.test.js


View more (1)
4. package.json Dependencies +5/-6

Update dependencies and remove request library

• Upgrade axios from 0.21.4 to 1.14.0 (major version bump)
• Upgrade lodash from 4.17.21 to 4.17.23 (patch version bump)
• Remove request dependency (2.88.2) entirely
• Upgrade dev dependencies: eslint (8.28.0 → 8.57.0), gh-pages (2.2.0 → 6.3.0),
 standard-version (9.3.2 → 9.5.0)

package.json


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Apr 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)   🎨 UX Issues (0)
🐞\ ⚙ Maintainability (1)

Grey Divider


Remediation recommended

1. Malformed request URI params🐞
Description
lib/api.apiCall builds the callback "request" object by appending raw axiosConfig.params values into
URL.searchParams, which string-coerces arrays/objects (e.g., arrays become comma-joined strings and
objects become "[object Object]"). This makes the 4th callback argument’s request.uri inaccurate
and breaks backward compatibility for callers relying on request-style request metadata when params
include arrays/nested objects.
Code

lib/api.js[R93-98]

+          const requestUri = new URL(axiosConfig.url);
+          if (axiosConfig.params) {
+            Object.entries(axiosConfig.params).forEach(([key, value]) => {
+              requestUri.searchParams.append(key, value);
+            });
+          }
Evidence
The SDK supports complex params (arrays and nested objects) for GET requests, but apiCall appends
those values directly to URLSearchParams without serialization, producing an incorrect/misleading
URI in the request object passed to callbacks.

lib/api.js[72-103]
test/api.test.js[279-286]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`lib/api.js` constructs `requestObj.uri` by doing `requestUri.searchParams.append(key, value)` over `axiosConfig.params`. When params contain arrays/nested objects (which this SDK already uses), the URLSearchParams API will coerce those values to strings, producing an inaccurate URI in the `request` object passed as the 4th callback argument.
### Issue Context
This `requestObj` is intended as a compatibility replacement for the old `request` library’s request object. Currently, it can misrepresent the actual outgoing request for non-scalar query params.
### Fix Focus Areas
- lib/api.js[72-103]
- test/api.test.js[279-306]
### Suggested fix
- Introduce a single, explicit query serialization strategy for complex params (arrays/nested objects).
- Use that strategy in **both** places:
1) in the axios request (`axiosConfig.paramsSerializer`) so outgoing requests are stable/intentional, and
2) when building `requestObj.uri` (set `requestUri.search` to the same serialized string rather than calling `searchParams.append` on raw values).
- Add/adjust unit coverage to assert the produced `requestObj.uri` contains the expected query representation for array/object params.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Advisory comments

2. Inconsistent axios call style 🐞
Description
lib/api.apiCall uses axios(axiosConfig) while lib/auth.requestToken uses
axios.request(axiosConfig), which needlessly diverges the HTTP-call style across the codebase.
This inconsistency makes future maintenance (and consistent stubbing patterns) harder than
necessary.
Code

lib/api.js[R79-80]

+      axios(axiosConfig)
+        .then((response) => {
Evidence
The codebase now uses two different axios invocation styles for the same kind of request flow
(config-driven request + promise chain). Standardizing improves consistency.

lib/api.js[79-80]
lib/auth.js[120-133]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`lib/api.js` calls axios via `axios(axiosConfig)` while `lib/auth.js` uses `axios.request(axiosConfig)`. Both work, but the inconsistency is unnecessary and increases maintenance friction.
### Issue Context
Using one style everywhere makes it easier to apply shared patterns (wrappers/interceptors, consistent stubs, etc.).
### Fix Focus Areas
- lib/api.js[79-80]
### Suggested fix
- Replace `axios(axiosConfig)` with `axios.request(axiosConfig)` in `lib/api.js` to match `lib/auth.js`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

requestObj.uri was being built by manually appending axiosConfig.params values
to a URL via searchParams.append().
This coerces arrays/objects to strings
(e.g., arrays become comma-joined, objects become "[object Object]")
and will not reflect the actual query string Axios sends.
@DanielStevenLewis DanielStevenLewis changed the title PAN-11212 fix: get npm audit --production to pass [PAN-11212] release/2.8.0 fix: get npm audit --production to pass Apr 6, 2026

@imlytvynenko imlytvynenko left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good, thanks

@DanielStevenLewis
DanielStevenLewis merged commit 82fabbe into master Apr 10, 2026
1 check passed
@DanielStevenLewis
DanielStevenLewis deleted the PAN-11212-fix-vulnerable-dependencies-v2 branch April 10, 2026 17:49
@DanielStevenLewis DanielStevenLewis changed the title [PAN-11212] release/2.8.0 fix: get npm audit --production to pass [PAN-11212] release/2.9.0 fix: get npm audit --production to pass Apr 10, 2026
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.

4 participants