MLE-30263: Fix Uncaught Exception in getAccessToken Error Handler

[ngodugu-marklogic]

Jira: MLE-30263
Severity: Medium | CVSS: 7.5 High | CWE: CWE-755

Problem

In Cloud authentication (authType: 'CLOUD'), getAccessToken() threw errors from async event handlers (res.on('data') and res.on('error')). Throwing inside these callbacks can cause uncaught exceptions and crash the Node.js process instead of rejecting the calling operation.

The implementation also double-wrapped errors (new Error(existingError)), producing confusing error messages.

Solution

Updated token retrieval error handling in lib/requester.js to use operation-level error propagation instead of throw-based async failures.

• Added internal handleTokenError(error, contextMessage) helper that:
  • ensures token failures are reported once (tokenErrorReported)
  • releases operation.lockAccessToken on failure
  • routes errors through operation.errorListener(new Error(...))
• Replaced statusCode === 400 throw path with:
  • handleTokenError(null, 'Token endpoint returned 400: ...')
  • immediate return
• Added defensive token response parsing/validation:
  • wraps JSON parsing in try/catch
  • validates access_token and .expires
  • routes invalid payloads via operation.errorListener
• Replaced response error throw path with handleTokenError(...).
• Added req.on('error', ...) handling for request/socket failures (e.g. ECONNRESET).

Changes

File Change
lib/requester.js Reworked getAccessToken() error paths to use operation.errorListener; removed throw-based async failures; added request-error handling and token response validation
test-basic/cloud_authentication-test.js Added 2 regression tests for request network errors and token endpoint 400 responses

Tests

Pre-existing tests in test-basic/cloud_authentication-test.js continue to pass (with one existing skipped test).

New tests added:

• should route token request network errors to operation.errorListener
• should route token endpoint 400 responses to operation.errorListener

Validated result:

- 6 passing
- 1 pending

Testing instructions

Project requires Node >= 22

Install dependencies

npm ci --no-audit --no-fund

Run targeted cloud auth tests

npx mocha test-basic/cloud_authentication-test.js

If your default Node is not 22, run explicitly with Node 22

npx -y node@22 node_modules/mocha/bin/mocha.js test-basic/cloud_authentication-test.js

[Copilot]

Pull request overview

This PR addresses a reliability/security issue in Cloud authentication by preventing uncaught exceptions from getAccessToken()’s async response handlers, and by routing token retrieval failures through operation.errorListener so callers see a rejected operation instead of a process crash.

Changes:

• Reworked getAccessToken() error propagation to avoid throw inside async event handlers and added request-level error handling.
• Added defensive parsing/validation for token responses and improved token-error messaging.
• Added two regression tests covering token request network errors and 400 responses from the token endpoint.

Reviewed changes

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

File	Description
lib/requester.js	Reworks Cloud token retrieval error handling and response parsing/validation.
test-basic/cloud_authentication-test.js	Adds regression tests to assert token failures route through operation.errorListener.

[RitaChen609]

throw new Error is used here. Should it be replaced with handleTokenError and return earlier?

[RitaChen609]

throw new Error is used here. Should it be replaced with handleTokenError?

[rjdew-progress]

I think that this is intentional to be picked up by req.on('error', (e) =>

[RitaChen609]

I think the throw new Error would be caught by catch(error) block (line 100) later. It then will call handleTokenError. So I think using throw new Error is not a bug, but replacing with handleTokenError and return earlier would be clearer and more explicit.

[RitaChen609]

If authenticatedRequest throws for any reason unrelated to token parsing, that error will be caught and reported as "Invalid token endpoint response", which is misleading. Should authenticatedRequest be moved outside the try block?

[ngodugu-marklogic]

Makes Sense.. Fixed it!