review fixups: drop inert mocharc require, tighten casts, add CI guards, v2.0.0

Addresses code-review-squad findings from the self-review pass:

- F1 (HIGH): drop `require: ['ts-node/register']` from both mocharc files.
  Verified locally that this option fires after mocha's import() resolution
  on Node 22+ and does NOT restore `__dirname`. The CJS-loader path on
  Node ≤20 is registered by nyc.config.js. Per-file path.resolve(...) fixes
  remain — they're what actually works.

- F3 (MED): bump version 1.15.0 → 2.0.0 + CHANGELOG entry enumerating the
  three breaking signals (engines.node 14→16, uuid major, thrift major).
  Matches npm-ecosystem convention for runtime-dep majors + engine bumps.

- F5 (MED): document the typescript 5.5.4 sandwich pin in CONTRIBUTING.md
  "Dependency Pins" section. Also documents the uuid override rationale
  and lockfile v2 pin policy.

- F6 (MED): post-rewrite hard-check in setup-jfrog/action.yml. Fails CI
  loudly if a non-JFrog host slips into package-lock.json (rather than
  hanging npm ci for 8min like the regression we just fixed).

- F7 (MED): timeout-minutes on unit-test (20) and e2e-test (30) jobs.
  Caps the blast radius of a wedged matrix entry — without these caps
  one hung e2e job could hold a runner + warehouse session for 6h.

- F9 (LOW): add CWD invariant checks to the two tests that resolve from
  process.cwd(). Fails loudly with a clear error if mocha is ever
  invoked from a non-repo-root CWD, rather than producing an opaque
  ENOENT.

- F10 (LOW): tighten the two as-any casts. FederationProvider's
  controller.signal cast now goes through `unknown as import('node-fetch')
  .RequestInit['signal']` to narrow the assertion. AuthorizationCode test
  uses `as unknown as (typeof authCode)['createHttpServer']` so the
  assertion is verifiable.

- F12 (LOW): CI lint that fails if package-lock.json's lockfileVersion
  drifts from 2. Catches the silent v3 rewrite that modern npm performs
  by default on `npm install`.

Verified locally:
- npm test passes on Node 16 (904) and Node 22 (898 + 6 pending — the
  Node-22-only lz4 skip is unchanged from prior commits)
- OSV-Scanner v2.3.8 reports zero findings on the lockfile
- tsc --noEmit clean against tsconfig.build.json

Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>

Author: vikrantpuppala

.github/actions/setup-jfrog/action.yml

@@ -59,6 +59,13 @@ runs:
           echo "package-lock.json resolved: URLs rewritten to JFrog"
           echo "Resolved URL distribution after rewrite:"
           grep -oE '"resolved": "https://[^/]+' package-lock.json | sort | uniq -c
+          # Fail loud if any non-JFrog host remains — a new registry hostname
+          # added to the tree would otherwise silently hang `npm ci` for 8min.
+          LEAKS=$(grep -oE '"resolved": "https://[^/]+' package-lock.json | grep -v 'databricks\.jfrog\.io' | sort -u || true)
+          if [ -n "$LEAKS" ]; then
+            echo "::error::JFrog rewrite incomplete; non-JFrog hosts remain: $LEAKS"
+            exit 1
+          fi
         else
           echo "no package-lock.json found; skipping rewrite"
         fi

.github/workflows/main.yml

@@ -19,6 +19,16 @@ jobs:
       labels: linux-ubuntu-latest
     steps:
       - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
+      - name: Pin lockfileVersion to 2
+        # See CONTRIBUTING.md "Dependency Pins". Modern npm writes v3
+        # by default; catching drift here prevents silent format upgrades
+        # from sneaking in via `npm install`.
+        run: |
+          actual=$(jq -r '.lockfileVersion' package-lock.json)
+          if [ "$actual" != "2" ]; then
+            echo "::error::package-lock.json lockfileVersion is $actual; expected 2. Regenerate with 'npm install --lockfile-version=2'."
+            exit 1
+          fi
       - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
         with:
           node-version: 20
@@ -44,6 +54,7 @@ jobs:
     runs-on:
       group: databricks-protected-runner-group
       labels: linux-ubuntu-latest
+    timeout-minutes: 20
     strategy:
       matrix:
         # LTS versions: 16/18/20 are the currently-supported floor; 22
@@ -88,6 +99,10 @@ jobs:
       group: databricks-protected-runner-group
       labels: linux-ubuntu-latest
     environment: azure-prod
+    # Cap job time so a wedged matrix entry doesn't hold a runner +
+    # warehouse session for GitHub's 6-hour default. Tests historically
+    # complete in <15min; 30min leaves room for warehouse cold-start.
+    timeout-minutes: 30
     strategy:
       # Run all matrix entries even if one fails so a Node-version-specific
       # network/TLS regression doesn't hide other versions' results.

CHANGELOG.md

@@ -1,5 +1,24 @@
 # Release History
 
+## 2.0.0
+
+**Breaking changes:**
+
+- **`engines.node` raised from `>=14.0.0` to `>=16.0.0`.** Node 14 is dropped. The modern npm ecosystem ships ES2021 syntax (`||=`) in widely-used transitive deps (e.g. `@dabh/diagnostics@2.0.7+` via `winston`) that Node 14's V8 cannot parse. Node 14 has been EOL upstream since April 2023.
+- **`uuid` runtime dep bumped from `^9.0.0` to `^11.1.1`.** uuid 11 is dual-published as ESM and CJS; this driver's CJS-compiled `dist/` continues to `require('uuid')` without changes. Customers whose code constructs uuids via this driver's transitive copy will see the v11 API surface (`v4`, `stringify`, `NIL` — the functions this driver uses — are API-stable across v9→v11).
+- **`thrift` runtime dep bumped from `^0.16.0` to `^0.23.0`.** Seven minor versions of the Apache Thrift Node.js client. Clears GHSA-r67j-r569-jrwp (HIGH) and GHSA-526f-jxpj-jmg2.
+
+**Security:**
+
+- Clear all OSV-Scanner HIGH/MED/LOW findings on the lockfile via dep bumps and an `overrides` block pinning 18 transitives to patched versions (databricks/databricks-sql-nodejs#390 by @vikrantpuppala)
+- Notably clears GHSA-w5hq-g745-h8pq (uuid v3/v5/v6 buffer-bounds, HIGH 7.5 — not reachable in this driver's usage but visible to customer-side scanners against our lockfile)
+
+**Other:**
+
+- Extend CI matrix to Node 16/18/20/22/24 for both unit and e2e tests (was 14/16/18/20 unit, single-version Node 20 e2e)
+- CI: rewrite lockfile `resolved:` URLs to JFrog mirror before `npm ci` so protected runners can fetch
+- TypeScript pinned to exact `5.5.4` (constraint: >=5.0 for uuid@11 d.ts, <5.6 to avoid Buffer-generic emit changing published declarations)
+
 ## 1.15.0
 
 - Add SPOG routing support: parse `?o=<workspaceId>` from `httpPath` and inject `x-databricks-org-id` on Thrift, telemetry, and feature-flag requests. Expose `customHeaders` on `ConnectionOptions` for caller-supplied headers (databricks/databricks-sql-nodejs#391 by @samikshya-db)

CONTRIBUTING.md

@@ -107,6 +107,19 @@ npm run lint:fix
 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.
+
+- **`typescript: "5.5.4"`** (exact, no caret). This pin has both a floor and a ceiling:
+  - Floor (TS >= 5.0) is required because `uuid@11`'s shipped `.d.ts` uses `export type * from './types.js'`, a TS 5.0+ feature.
+  - Ceiling (TS < 5.6) is required because TS 5.6 changed how `@types/node`'s generic `Buffer<TArrayBuffer extends ArrayBufferLike>` declarations get emitted into our published `dist/*.d.ts`. Allowing TS 5.6+ would leak `Buffer<ArrayBufferLike>` into the published types, which fails to compile for consumers on stale `@types/node`.
+  - If you bump TS, run `npm run build` and `git diff dist/` and verify no `Buffer<...>` generics appear in any `.d.ts`. If they do, you need to either roll back or also bump `@types/node` consumer expectations (a customer-facing change).
+
+- **`overrides.uuid: "^11.1.1"`**. Forces `thrift@0.23.0`'s declared `uuid: ^13.0.0` (ESM-only) down to v11 (dual ESM+CJS). Without this override, the driver's CJS-compiled `dist/` would crash on `require('uuid')` at runtime. Remove this override only after migrating the driver to ESM or when `thrift` drops the uuid dep.
+
+- **`package-lock.json` is pinned to `lockfileVersion: 2`.** Modern npm writes v3 by default. To regenerate the lockfile, run `npm install --lockfile-version=2` so CI's lint step doesn't reject your PR. v2 is kept for compat with older toolchains; revisit when the team is ready to drop them.
+
 ## Pull Request Process
 
 1. Update the [CHANGELOG](CHANGELOG.md) with details of your changes, if applicable.

lib/connection/auth/tokenProvider/FederationProvider.ts

@@ -166,8 +166,9 @@ export default class FederationProvider implements ITokenProvider {
         // node-fetch ships its own AbortSignal shim that differs slightly
         // from the native AbortSignal (subtle `this`-typing mismatch on
         // onabort handler). TS 4 didn't catch this; TS 5+ does. The two
-        // are runtime-compatible, so cast through any.
-        signal: controller.signal as any,
+        // are runtime-compatible, so cast through `unknown` (rather than
+        // `any`) to keep the assertion narrow.
+        signal: controller.signal as unknown as import('node-fetch').RequestInit['signal'],
       });
 
       if (!response.ok) {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@databricks/sql",
-  "version": "1.15.0",
+  "version": "2.0.0",
   "description": "Driver for connection to Databricks SQL via Thrift API.",
   "main": "dist/index.js",
   "types": "dist/index.d.ts",

tests/e2e/.mocharc.js

@@ -7,7 +7,4 @@ const argvSpecs = process.argv.slice(4);
 module.exports = {
   spec: argvSpecs.length > 0 ? argvSpecs : allSpecs,
   timeout: '300000',
-  // Force the CommonJS loader path so .ts specs go through ts-node's
-  // require hook. See tests/unit/.mocharc.js for context.
-  require: ['ts-node/register'],
 };

tests/unit/.mocharc.js

@@ -6,10 +6,4 @@ const argvSpecs = process.argv.slice(4);
 
 module.exports = {
   spec: argvSpecs.length > 0 ? argvSpecs : allSpecs,
-  // Force the CommonJS loader path so .ts specs go through ts-node's
-  // require hook. Mocha 10.5+ otherwise picks the ESM import() path
-  // for .ts files on some Node versions (notably Node 22+), which
-  // bypasses ts-node/register and breaks tests that rely on CJS
-  // globals like `__dirname` and `require`.
-  require: ['ts-node/register'],
 };

tests/unit/connection/auth/DatabricksOAuth/AuthorizationCode.test.ts

@@ -98,11 +98,10 @@ function prepareTestInstances(options: Partial<AuthorizationCodeOptions>) {
     return httpServer;
   });
 
-  // Cast: createHttpServer's return type (OAuthCallbackServerStub) intentionally
-  // omits some http.Server typing (e.g. Symbol.asyncDispose, listen overloads with
-  // exact backlog typing). Relaxing the assignment keeps the stub minimal without
-  // having to fully mirror the http.Server interface.
-  authCode['createHttpServer'] = createHttpServer as any;
+  // OAuthCallbackServerStub intentionally omits some http.Server typing
+  // (e.g. Symbol.asyncDispose, listen overloads with exact backlog typing).
+  // Cast through `unknown` to keep the assertion narrow rather than `any`.
+  authCode['createHttpServer'] = createHttpServer as unknown as (typeof authCode)['createHttpServer'];
 
   openAuthUrl.callsFake(async (authUrl) => {
     const params = JSON.parse(authUrl);

tests/unit/result/ArrowResultConverter.test.ts

@@ -57,6 +57,14 @@ const sampleArrowBatch = [
 // Resolve from CWD (always the repo root when mocha runs) rather than
 // __dirname. Node 22+'s ESM auto-detection loads .ts specs as ES modules,
 // where CJS globals like __dirname are unavailable.
+// Loud check for the CWD assumption: this file reads stub fixtures at
+// import time, so a wrong CWD must fail clearly rather than producing
+// an opaque ENOENT.
+if (!fs.existsSync('package.json')) {
+  throw new Error(
+    `Expected mocha to be invoked from repo root; CWD=${process.cwd()} has no package.json`,
+  );
+}
 const ARROW_STUBS_DIR = path.resolve('tests/unit/result/.stubs');
 const arrowBatchAllNulls = [
   fs.readFileSync(path.join(ARROW_STUBS_DIR, 'arrowSchemaAllNulls.arrow')),