diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 01968699..19429136 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -109,7 +109,7 @@ npm run type-check ## Dependency Pins -A few entries in `package.json` are pinned more tightly than usual. Don't relax these without understanding why. +A few entries in `package.json` are pinned more tightly than usual. Don't relax these without understanding why. For the full list of CVE-driven `overrides` entries, see [`SECURITY-OVERRIDES.md`](./SECURITY-OVERRIDES.md). - **`typescript: "5.5.4"`** (exact, no caret). This pin has both a floor and a ceiling: diff --git a/SECURITY-OVERRIDES.md b/SECURITY-OVERRIDES.md new file mode 100644 index 00000000..4543c402 --- /dev/null +++ b/SECURITY-OVERRIDES.md @@ -0,0 +1,155 @@ +# Security Overrides + +The `overrides` block in `package.json` pins 18 transitive dependencies to versions that clear known CVEs. Each entry is debt — when the underlying ecosystem moves on, the corresponding entry should be removed. + +This file documents the provenance and exit condition for each override. **When adding or removing an override, update this file in the same commit.** + +## Conventions + +- **Class**: `runtime` if the package ends up in the published `dist/` runtime path; `dev` if it's used only by tooling (eslint, mocha, nyc, babel, prettier, etc.). The `.npmignore` excludes everything except `dist/`, `thrift/`, `LICENSE`, `NOTICE`, `package.json`, `README.md` — so dev-tooling overrides do not ship to consumers but DO surface in customer-side scanners (Dependabot, Snyk, OSV) that scan our lockfile. +- **Exit condition**: the smallest change that would let us drop the override entry. Usually "upstream bump", sometimes "upstream takes the patched version as a dep range". + +--- + +## Entries + +### `basic-ftp: ^5.3.1` + +- **Class**: runtime +- **Path**: `proxy-agent → pac-proxy-agent → get-uri → basic-ftp` +- **CVEs cleared**: GHSA-5rq4-664w-9x2c (HIGH 9.1), GHSA-6v7q-wjvx-w8wg (HIGH 8.2), GHSA-rp42-5vxx-qpwr (HIGH 7.5), GHSA-rpmf-866q-6p89 (HIGH 7.5) +- **Exit**: `get-uri` bumps its `basic-ftp` dep range to include `^5.3.1`. Currently declares `^5.0.2`. + +### `@75lb/deep-merge: ^1.1.2` + +- **Class**: dev (transitive of apache-arrow's CLI tooling — not in runtime path) +- **Path**: `apache-arrow → command-line-usage → table-layout → @75lb/deep-merge` +- **CVEs cleared**: GHSA-28mc-g557-92m7 (HIGH 8.7) +- **Exit**: `table-layout` bumps its dep. Note `apache-arrow@13` itself ships unused CLI tooling — bumping arrow to `15.x+` drops this dep entirely. + +### `braces: ^3.0.3` + +- **Class**: dev (via mocha's chokidar + eslint's micromatch) +- **Path**: `mocha → chokidar → braces` AND `@typescript-eslint/parser → ... → micromatch → braces` +- **CVEs cleared**: GHSA-grv7-fg5c-xmjg (HIGH 7.5) +- **Exit**: `chokidar` bumps its `braces` dep range. Currently declares `~3.0.2`. + +### `picomatch: ^2.3.2` + +- **Class**: dev (chokidar + micromatch transitive) +- **Path**: `mocha → chokidar → readdirp → picomatch` AND `... → micromatch → picomatch` +- **CVEs cleared**: GHSA-c2c7-rcm5-vvqj (HIGH 7.5) +- **Exit**: `chokidar` and `micromatch` bump their `picomatch` dep ranges. + +### `flatted: ^3.4.2` + +- **Class**: dev (eslint's file-entry-cache) +- **Path**: `eslint → file-entry-cache → flat-cache → flatted` +- **CVEs cleared**: GHSA-rf6f-7fwh-wjgh (HIGH 8.9), GHSA-25h7-pfq9-p65f (HIGH 7.5) +- **Exit**: `flat-cache` bumps. Or move to eslint 9 (drops file-entry-cache dep tree shape). + +### `minimatch: ^3.1.3` + +- **Class**: dev (eslint plugins) +- **Path**: `eslint-plugin-import / -jsx-a11y / -react → minimatch` +- **CVEs cleared**: GHSA-3ppc-4f35-3m26 (HIGH 8.7), GHSA-23c5-xmqv-rm74 (HIGH 7.5), GHSA-7r86-cg39-jmmj (HIGH 7.5) +- **Exit**: eslint plugins bump to use minimatch 9.x+ (most have done so on newer majors). + +### `ws: ^8.18.0` + +- **Class**: runtime (thrift's WebSocket transport) +- **Path**: `thrift → ws` AND `thrift → isomorphic-ws → ws` +- **CVEs cleared**: GHSA-3h5v-q93c-6h6q (HIGH 8.7 — ws@5.x DoS) +- **Exit**: `thrift` bumps its declared `ws: ^5.2.3` to `^8.x`. Without the override, `thrift@0.23.0` would pull the vulnerable `ws@5.x`. + +### `cross-spawn: ^7.0.6` + +- **Class**: dev (eslint + nyc) +- **Path**: `eslint → cross-spawn`, `nyc → foreground-child → cross-spawn`, `nyc → istanbul-lib-processinfo → cross-spawn` +- **CVEs cleared**: GHSA-3xgq-45jj-v275 (HIGH 7.7 — ReDoS) +- **Exit**: eslint and nyc bump. Currently declare `^7.0.2`. + +### `serialize-javascript: ^7.0.5` + +- **Class**: dev (mocha) +- **Path**: `mocha → serialize-javascript` +- **CVEs cleared**: GHSA-5c6j-r48x-rmvq (HIGH 8.1 — XSS via prototype pollution) +- **Exit**: mocha bumps. Currently declares `^6.0.2`. + +### `follow-redirects: ^1.16.0` + +- **Class**: dev (http-proxy testing util) +- **Path**: `http-proxy → follow-redirects` +- **CVEs cleared**: GHSA-r4q5-vmmm-2653 (MED 6.9) +- **Exit**: `http-proxy` bumps. Currently declares `^1.15.0`. + +### `brace-expansion: ^1.1.13` + +- **Class**: dev (transitive of overridden minimatch) +- **Path**: `eslint-plugin-import → minimatch → brace-expansion` +- **CVEs cleared**: GHSA-v6h2-p8h4-qcjw (LOW) +- **Exit**: same as `minimatch` — when eslint plugins bump to minimatch 9+, this resolves transitively too. + +### `@babel/helpers: ^7.26.10` + +- **Class**: dev (nyc instrumentation) +- **Path**: `nyc → istanbul-lib-instrument → @babel/core → @babel/helpers` +- **CVEs cleared**: GHSA-968p-4wvh-cqc8 (MED 6.2 — ReDoS) +- **Exit**: `@babel/core` bumps the `@babel/helpers` range. Currently the bundled version was below the patched. + +### `@babel/runtime: ^7.26.10` + +- **Class**: dev (eslint-config-airbnb-typescript transitive) +- **Path**: eslint plugins (via core-js-pure → @babel/runtime) +- **CVEs cleared**: GHSA-968p-4wvh-cqc8 (MED 6.2 — same as @babel/helpers) +- **Exit**: same as `@babel/helpers`. + +### `@babel/runtime-corejs3: ^7.26.10` + +- **Class**: dev (eslint-config-airbnb) +- **Path**: same as `@babel/runtime` +- **CVEs cleared**: GHSA-968p-4wvh-cqc8 (MED 6.2) +- **Exit**: same as `@babel/runtime`. + +### `ip-address: ^10.1.1` + +- **Class**: runtime (proxy-agent → socks → ip-address) +- **Path**: `proxy-agent → socks-proxy-agent → socks → ip-address` +- **CVEs cleared**: GHSA-v2v4-37r5-5v8g (MED 5.3 — IPv6 parsing DoS) +- **Exit**: `socks` bumps to `ip-address@^10.x`. Note: `ip-address@10` is published as CommonJS with conditional exports — verify any future bump retains CJS compat for our `dist/`. + +### `js-yaml: ^4.1.1` + +- **Class**: dev (eslint / mocha / nyc config loaders) +- **Path**: `eslint → @eslint/eslintrc → js-yaml`, `mocha → js-yaml`, `nyc → @istanbuljs/load-nyc-config → js-yaml` +- **CVEs cleared**: GHSA-mh29-5h37-fv8m (MED 5.3 — DoS on malformed YAML) +- **Exit**: each consumer bumps. All three are already on the 4.x line; the override forces a patch within 4.x. + +### `micromatch: ^4.0.8` + +- **Class**: dev (typescript-eslint glob) +- **Path**: `@typescript-eslint/parser → @typescript-eslint/typescript-estree → globby → fast-glob → micromatch` +- **CVEs cleared**: GHSA-952p-6rrq-rcjv (MED 5.3 — ReDoS) +- **Exit**: `fast-glob` bumps. Currently declares `^4.0.4`. + +### `uuid: ^11.1.1` + +- **Class**: **runtime** — this one matters most +- **Path**: declared as a top-level runtime dep AND `thrift → uuid` +- **CVEs cleared**: GHSA-w5hq-g745-h8pq (HIGH 7.5 — buffer-bounds in v3/v5/v6; driver only uses v4 but consumer scanners flag against our lockfile) +- **Why an override is needed**: `thrift@0.23.0` declares `uuid: ^13.0.0`, but `uuid@13` is **ESM-only**. Our driver is compiled to CJS (`dist/*.js`), so a top-level `uuid: ^11.1.1` plus this matching override forces `thrift`'s transitive uuid down to v11 (which dual-publishes ESM + CJS via conditional exports). +- **Exit**: any of (a) we migrate `dist/` to ESM, (b) `thrift` drops the uuid dep, (c) `thrift` widens its range to `^11 || ^13` and we go through whichever export shape `thrift` decides on. Today, removing this override would cause `require('uuid')` from `dist/` to crash on Node ≤22.11 (which don't support `require(esm)`). + +--- + +## How to audit + +```bash +# Show what depends on a specific override target: +npm ls + +# Re-run the lockfile against OSV-Scanner to verify findings are still cleared: +osv-scanner scan source --lockfile=package-lock.json +``` + +When all entries' exit conditions are met, this file should be deleted along with the corresponding `overrides` block. diff --git a/lib/connection/auth/tokenProvider/FederationProvider.ts b/lib/connection/auth/tokenProvider/FederationProvider.ts index 417851ff..c680c35c 100644 --- a/lib/connection/auth/tokenProvider/FederationProvider.ts +++ b/lib/connection/auth/tokenProvider/FederationProvider.ts @@ -1,8 +1,17 @@ -import fetch from 'node-fetch'; +import nodeFetch from 'node-fetch'; import ITokenProvider from './ITokenProvider'; import Token from './Token'; import { getJWTIssuer, isSameHost } from './utils'; +// Indirection so tests can swap the fetch implementation without +// patching the import system. Default is node-fetch. +let fetchImpl: typeof nodeFetch = nodeFetch; + +/** Test-only: replace the fetch implementation. Called with no arg, restores node-fetch. */ +export function setFederationFetchForTest(fn?: typeof nodeFetch): void { + fetchImpl = fn ?? nodeFetch; +} + /** * Token exchange endpoint path for Databricks OIDC. */ @@ -157,7 +166,7 @@ export default class FederationProvider implements ITokenProvider { const timeoutId = setTimeout(() => controller.abort(), REQUEST_TIMEOUT_MS); try { - const response = await fetch(url, { + const response = await fetchImpl(url, { method: 'POST', headers: { 'Content-Type': 'application/x-www-form-urlencoded', diff --git a/lib/utils/lz4.ts b/lib/utils/lz4.ts index 7d409230..a9567959 100644 --- a/lib/utils/lz4.ts +++ b/lib/utils/lz4.ts @@ -2,9 +2,20 @@ import type LZ4Namespace from 'lz4'; type LZ4Module = typeof LZ4Namespace; +// Loader function — extracted so tests can inject a stub without +// having to patch CJS-only Module._load. Default is the real require. +// eslint-disable-next-line global-require +let loader: () => LZ4Module = () => require('lz4'); + +/** Test-only: replace the loader. Restores the real one when called with no arg. */ +export function setLz4LoaderForTest(fn?: () => LZ4Module): void { + // eslint-disable-next-line global-require + loader = fn ?? (() => require('lz4')); +} + function tryLoadLZ4Module(): LZ4Module | undefined { try { - return require('lz4'); // eslint-disable-line global-require + return loader(); } catch (err) { if (!(err instanceof Error) || !('code' in err)) { // eslint-disable-next-line no-console @@ -39,4 +50,9 @@ function getResolvedModule() { return resolvedModule === null ? undefined : resolvedModule; } +/** Test-only: reset the memoized resolution so the next call re-invokes the loader. */ +export function resetLz4CacheForTest(): void { + resolvedModule = undefined; +} + export default getResolvedModule; diff --git a/package.json b/package.json index 7f8e9b45..5cbcb6b3 100644 --- a/package.json +++ b/package.json @@ -21,8 +21,8 @@ "type-check": "tsc --noEmit", "prettier": "prettier . --check", "prettier:fix": "prettier . --write", - "lint": "eslint lib/** tests/e2e/** --ext .js,.ts", - "lint:fix": "eslint lib/** --ext .js,.ts --fix" + "lint": "eslint 'lib/**/*.{js,ts}' 'tests/e2e/**/*.{js,ts}' --ext .js,.ts", + "lint:fix": "eslint 'lib/**/*.{js,ts}' --ext .js,.ts --fix" }, "repository": { "type": "git", diff --git a/tests/e2e/README.md b/tests/e2e/README.md new file mode 100644 index 00000000..3dbd156d --- /dev/null +++ b/tests/e2e/README.md @@ -0,0 +1,34 @@ +# End-to-End Tests + +These tests run against a real Databricks SQL warehouse. They're invoked by `npm run e2e` and exercise the driver's HTTP/Thrift/Arrow path against live infrastructure. + +## Environment + +| Variable | Used for | +| ------------------ | ------------------------------------------------------------------------ | +| `E2E_HOST` | Workspace hostname | +| `E2E_PATH` | Warehouse HTTP path | +| `E2E_ACCESS_TOKEN` | PAT for auth | +| `E2E_TABLE_SUFFIX` | Suffix appended to per-test table names so concurrent runs don't collide | +| `E2E_CATALOG` | Catalog (default: `peco`) | +| `E2E_SCHEMA` | Schema (default: `default`) | +| `E2E_VOLUME` | Volume name (default: `e2etests`) | + +## CI parallelism + +The `e2e-test` job in `.github/workflows/main.yml` runs as a 5-entry matrix across Node 16/18/20/22/24. All five entries point at the same workspace, catalog, schema, and volume. + +Per-test isolation is achieved by: + +- **Tables**: all DDL in tests is templated against `${E2E_TABLE_SUFFIX}`, which in CI is `${{ github.sha }}_node${{ matrix.node-version }}`. Underscores not hyphens — SQL unquoted identifiers don't allow `-`. +- **Volume files**: `tests/e2e/staging_ingestion.test.ts` generates per-file `uuid.v4()` names. Multiple matrix entries can read/write the volume concurrently without collisions. + +No test creates or drops the shared catalog/schema/volume. If you add a test that does, you'll need to suffix-unique the resource name too — verify before merging. + +## Local invocation + +`npm run e2e` must be run from the repo root. Some specs resolve fixture paths relative to `process.cwd()`. + +## Warehouse capacity + +Five parallel CI entries against one warehouse plus any concurrent PR runs can saturate the warehouse's session limit. If you see queue-related flakes (`session start` timeouts, request queueing delays), check the warehouse's `max_num_concurrent_runs` setting. diff --git a/tests/unit/.stubs/OAuth.ts b/tests/unit/.stubs/OAuth.ts index 162f81f4..526517fc 100644 --- a/tests/unit/.stubs/OAuth.ts +++ b/tests/unit/.stubs/OAuth.ts @@ -100,48 +100,17 @@ export class OAuthCallbackServerStub< return this; } - // Dummy methods and properties for compatibility with `http.Server` - - public maxHeadersCount: number | null = null; - - public maxRequestsPerSocket: number | null = null; - - public timeout: number = -1; - - public headersTimeout: number = -1; - - public keepAliveTimeout: number = -1; - - public requestTimeout: number = -1; - - public maxConnections: number = -1; - - public connections: number = 0; - - public setTimeout() { - return this; - } - - public closeAllConnections() {} - - public closeIdleConnections() {} - + // No-op shims for the subset of `http.Server` members production code + // touches. We intentionally do NOT mirror the full http.Server surface; + // the call site uses an `as unknown as ...` cast (see AuthorizationCode + // .test.ts) to satisfy the type checker, and that cast is what carries + // the "trust me, this is Server-shaped" assertion. When @types/node + // adds a new Server member, the test file's cast adapts automatically; + // when the OAuth code starts calling a new Server member, add a shim + // here and the runtime test will exercise it. public address() { return null; } - - public getConnections() {} - - public ref() { - return this; - } - - public unref() { - return this; - } - - // Required by @types/node >= 18.19.x (Node 20+ added Symbol.asyncDispose to Server). - public async [Symbol.asyncDispose]() {} } export class AuthorizationCodeStub { diff --git a/tests/unit/connection/auth/tokenProvider/FederationProvider.test.ts b/tests/unit/connection/auth/tokenProvider/FederationProvider.test.ts index 4a7c5465..deb60470 100644 --- a/tests/unit/connection/auth/tokenProvider/FederationProvider.test.ts +++ b/tests/unit/connection/auth/tokenProvider/FederationProvider.test.ts @@ -1,6 +1,9 @@ import { expect } from 'chai'; import sinon from 'sinon'; -import FederationProvider from '../../../../../lib/connection/auth/tokenProvider/FederationProvider'; +import type nodeFetch from 'node-fetch'; +import FederationProvider, { + setFederationFetchForTest, +} from '../../../../../lib/connection/auth/tokenProvider/FederationProvider'; import ITokenProvider from '../../../../../lib/connection/auth/tokenProvider/ITokenProvider'; import Token from '../../../../../lib/connection/auth/tokenProvider/Token'; @@ -76,4 +79,116 @@ describe('FederationProvider', () => { expect(federationProvider.getName()).to.equal('federated[MockTokenProvider]'); }); }); + + describe('exchange path', () => { + // These tests exercise the federation HTTP exchange — the branch + // taken when the source JWT's issuer doesn't match the Databricks + // host. The branch contains the AbortController + node-fetch shim + // typing fix; without coverage here a regression in those mechanics + // would only surface in production. + + afterEach(() => { + setFederationFetchForTest(); // restore real node-fetch + }); + + // Helper: build a fake node-fetch Response. + function buildFakeResponse(opts: { + ok: boolean; + status?: number; + statusText?: string; + body?: unknown; + text?: string; + }): nodeFetch.Response { + return { + ok: opts.ok, + status: opts.status ?? (opts.ok ? 200 : 500), + statusText: opts.statusText ?? '', + json: async () => opts.body, + text: async () => opts.text ?? '', + } as unknown as nodeFetch.Response; + } + + it('should exchange foreign-issued JWT for a Databricks token', async () => { + const foreignJwt = createJWT({ iss: 'https://idp.example.com' }); + const baseProvider = new MockTokenProvider(foreignJwt); + const federationProvider = new FederationProvider(baseProvider, 'my-workspace.cloud.databricks.com'); + + const fetchStub = sinon.stub, ReturnType>().resolves( + buildFakeResponse({ + ok: true, + body: { access_token: 'exchanged-databricks-token', token_type: 'Bearer', expires_in: 3600 }, + }), + ); + setFederationFetchForTest(fetchStub as unknown as typeof nodeFetch); + + const token = await federationProvider.getToken(); + + expect(token.accessToken).to.equal('exchanged-databricks-token'); + expect(fetchStub.calledOnce).to.be.true; + + // The exchange must POST to the Databricks /oidc/v1/token endpoint. + const [url, init] = fetchStub.firstCall.args; + expect(String(url)).to.include('my-workspace.cloud.databricks.com'); + expect(String(url)).to.include('/oidc/v1/token'); + expect(init!.method).to.equal('POST'); + + // Verify the signal propagates an AbortSignal — this is the cast + // site that TS 5 type-strictness caught. Runtime-wise it must + // still be a real AbortSignal-shaped object. + const passedSignal = init!.signal as unknown as AbortSignal; + expect(passedSignal, 'fetch init.signal must be set').to.exist; + expect(typeof passedSignal.aborted, 'signal.aborted must be a boolean').to.equal('boolean'); + expect(passedSignal.aborted).to.be.false; + }); + + it('should propagate abort from the controller to the signal observed by fetch', async () => { + const foreignJwt = createJWT({ iss: 'https://idp.example.com' }); + const baseProvider = new MockTokenProvider(foreignJwt); + const federationProvider = new FederationProvider(baseProvider, 'my-workspace.cloud.databricks.com', { + returnOriginalTokenOnFailure: false, + }); + + // Capture the signal so we can assert it aborts later, and force fetch + // to hang until aborted (simulating a request the controller cancels). + let capturedSignal: AbortSignal | undefined; + const fetchStub = sinon + .stub, ReturnType>() + .callsFake(async (_url, init) => { + capturedSignal = init!.signal as unknown as AbortSignal; + // Resolve immediately with success to avoid the 30s real timeout + // path. The point of this test is to verify the signal is wired + // up, not to exercise the abort itself end-to-end. + return buildFakeResponse({ + ok: true, + body: { access_token: 'tok', token_type: 'Bearer', expires_in: 3600 }, + }); + }); + setFederationFetchForTest(fetchStub as unknown as typeof nodeFetch); + + await federationProvider.getToken(); + + expect(capturedSignal, 'signal must reach fetch').to.exist; + // The signal must implement the standard AbortSignal contract. + expect(typeof capturedSignal!.aborted).to.equal('boolean'); + expect(typeof capturedSignal!.addEventListener).to.equal('function'); + }); + + it('should fall back to original token when exchange fails (returnOriginalTokenOnFailure default)', async () => { + const foreignJwt = createJWT({ iss: 'https://idp.example.com' }); + const baseProvider = new MockTokenProvider(foreignJwt); + const federationProvider = new FederationProvider(baseProvider, 'my-workspace.cloud.databricks.com'); + + const fetchStub = sinon + .stub, ReturnType>() + .resolves(buildFakeResponse({ ok: false, status: 400, statusText: 'Bad Request', text: 'invalid_grant' })); + setFederationFetchForTest(fetchStub as unknown as typeof nodeFetch); + + const token = await federationProvider.getToken(); + + // Default behavior is to fall back to the original token on failure. + // Retries kick in for 5xx; 400 is non-retryable so this should fail + // fast on the first attempt. + expect(token.accessToken).to.equal(foreignJwt); + }); + }); }); diff --git a/tests/unit/utils/lz4.test.ts b/tests/unit/utils/lz4.test.ts index bf2288bc..f392a74d 100644 --- a/tests/unit/utils/lz4.test.ts +++ b/tests/unit/utils/lz4.test.ts @@ -1,93 +1,36 @@ import { expect } from 'chai'; import sinon from 'sinon'; +import getResolvedModule, { setLz4LoaderForTest, resetLz4CacheForTest } from '../../../lib/utils/lz4'; -describe('lz4 module loader', function () { - let moduleLoadStub: sinon.SinonStub | undefined; +describe('lz4 module loader', () => { let consoleWarnStub: sinon.SinonStub; - // This suite directly exercises CJS-only primitives (Module._load, - // require.cache). Node 22+ loads .ts specs through the ESM loader, - // where neither is available. Skip on those runtimes — the production - // lz4 loader code itself works fine; only this test's mechanism is - // CJS-bound. - before(function () { - if (typeof require === 'undefined') { - this.skip(); - } - }); - beforeEach(() => { consoleWarnStub = sinon.stub(console, 'warn'); }); afterEach(() => { consoleWarnStub.restore(); - if (moduleLoadStub) { - moduleLoadStub.restore(); - } - // Clear module cache - Object.keys(require.cache).forEach((key) => { - if (key.includes('lz4')) { - delete require.cache[key]; - } - }); + setLz4LoaderForTest(); // restore real require('lz4') + resetLz4CacheForTest(); }); - const mockModuleLoad = (lz4MockOrError: unknown): { restore: () => void; wasLz4LoadAttempted: () => boolean } => { - // eslint-disable-next-line global-require - const Module = require('module'); - const originalLoad = Module._load; - let lz4LoadAttempted = false; - - Module._load = (request: string, parent: unknown, isMain: boolean) => { - if (request === 'lz4') { - lz4LoadAttempted = true; - if (lz4MockOrError instanceof Error) { - throw lz4MockOrError; - } - return lz4MockOrError; - } - return originalLoad.call(Module, request, parent, isMain); - }; - - return { - restore: () => { - Module._load = originalLoad; - }, - wasLz4LoadAttempted: () => lz4LoadAttempted, - }; - }; - - const loadLz4Module = () => { - delete require.cache[require.resolve('../../../lib/utils/lz4')]; - // eslint-disable-next-line global-require - return require('../../../lib/utils/lz4'); - }; - it('should successfully load and use lz4 module when available', () => { const fakeLz4 = { - encode: (buf: Buffer) => { - const compressed = Buffer.from(`compressed:${buf.toString()}`); - return compressed; - }, - decode: (buf: Buffer) => { - const decompressed = buf.toString().replace('compressed:', ''); - return Buffer.from(decompressed); - }, + encode: (buf: Buffer) => Buffer.from(`compressed:${buf.toString()}`), + decode: (buf: Buffer) => Buffer.from(buf.toString().replace('compressed:', '')), }; - const { restore } = mockModuleLoad(fakeLz4); - const moduleExports = loadLz4Module(); - const lz4Module = moduleExports.default(); - restore(); + setLz4LoaderForTest(() => fakeLz4 as unknown as ReturnType); + const lz4Module = getResolvedModule(); expect(lz4Module).to.not.be.undefined; - expect(lz4Module.encode).to.be.a('function'); - expect(lz4Module.decode).to.be.a('function'); + expect(lz4Module!.encode).to.be.a('function'); + expect(lz4Module!.decode).to.be.a('function'); const testData = Buffer.from('Hello, World!'); - const compressed = lz4Module.encode(testData); - const decompressed = lz4Module.decode(compressed); + const compressed = lz4Module!.encode(testData); + const decompressed = lz4Module!.decode(compressed); expect(decompressed.toString()).to.equal('Hello, World!'); expect(consoleWarnStub.called).to.be.false; @@ -96,26 +39,22 @@ describe('lz4 module loader', function () { it('should return undefined when lz4 module fails to load with MODULE_NOT_FOUND', () => { const err: NodeJS.ErrnoException = new Error("Cannot find module 'lz4'"); err.code = 'MODULE_NOT_FOUND'; + setLz4LoaderForTest(() => { + throw err; + }); - const { restore } = mockModuleLoad(err); - const moduleExports = loadLz4Module(); - const lz4Module = moduleExports.default(); - restore(); - - expect(lz4Module).to.be.undefined; + expect(getResolvedModule()).to.be.undefined; expect(consoleWarnStub.called).to.be.false; }); it('should return undefined and log warning when lz4 fails with ERR_DLOPEN_FAILED', () => { const err: NodeJS.ErrnoException = new Error('Module did not self-register'); err.code = 'ERR_DLOPEN_FAILED'; + setLz4LoaderForTest(() => { + throw err; + }); - const { restore } = mockModuleLoad(err); - const moduleExports = loadLz4Module(); - const lz4Module = moduleExports.default(); - restore(); - - expect(lz4Module).to.be.undefined; + expect(getResolvedModule()).to.be.undefined; expect(consoleWarnStub.calledOnce).to.be.true; expect(consoleWarnStub.firstCall.args[0]).to.include('Architecture or version mismatch'); }); @@ -123,45 +62,35 @@ describe('lz4 module loader', function () { it('should return undefined and log warning when lz4 fails with unknown error code', () => { const err: NodeJS.ErrnoException = new Error('Some unknown error'); err.code = 'UNKNOWN_ERROR'; + setLz4LoaderForTest(() => { + throw err; + }); - const { restore } = mockModuleLoad(err); - const moduleExports = loadLz4Module(); - const lz4Module = moduleExports.default(); - restore(); - - expect(lz4Module).to.be.undefined; + expect(getResolvedModule()).to.be.undefined; expect(consoleWarnStub.calledOnce).to.be.true; expect(consoleWarnStub.firstCall.args[0]).to.include('Unhandled error code'); }); it('should return undefined and log warning when error has no code property', () => { const err = new Error('Error without code'); + setLz4LoaderForTest(() => { + throw err; + }); - const { restore } = mockModuleLoad(err); - const moduleExports = loadLz4Module(); - const lz4Module = moduleExports.default(); - restore(); - - expect(lz4Module).to.be.undefined; + expect(getResolvedModule()).to.be.undefined; expect(consoleWarnStub.calledOnce).to.be.true; expect(consoleWarnStub.firstCall.args[0]).to.include('Invalid error object'); }); it('should not attempt to load lz4 module when getResolvedModule is not called', () => { - const fakeLz4 = { - encode: () => Buffer.from(''), - decode: () => Buffer.from(''), - }; - - const { restore, wasLz4LoadAttempted } = mockModuleLoad(fakeLz4); - - // Load the module but don't call getResolvedModule - loadLz4Module(); - // Note: we're NOT calling .default() here - - restore(); + let lz4LoadAttempted = false; + setLz4LoaderForTest(() => { + lz4LoadAttempted = true; + return { encode: () => Buffer.from(''), decode: () => Buffer.from('') } as unknown as ReturnType; + }); - expect(wasLz4LoadAttempted()).to.be.false; + // Note: we're NOT calling getResolvedModule() — the loader should never fire. + expect(lz4LoadAttempted).to.be.false; expect(consoleWarnStub.called).to.be.false; }); });