diff --git a/lib/DBSQLClient.ts b/lib/DBSQLClient.ts index f7837c68..f021edf9 100644 --- a/lib/DBSQLClient.ts +++ b/lib/DBSQLClient.ts @@ -1,5 +1,7 @@ import thrift from 'thrift'; import os from 'os'; +import fs from 'fs'; +import tls from 'tls'; import { EventEmitter } from 'events'; import TCLIService from '../thrift/TCLIService'; @@ -14,7 +16,7 @@ import IAuthentication from './connection/contracts/IAuthentication'; import HttpConnection from './connection/connections/HttpConnection'; import IConnectionOptions from './connection/contracts/IConnectionOptions'; import HiveDriverError from './errors/HiveDriverError'; -import { buildUserAgentString } from './utils'; +import { buildUserAgentString, normalizePemBytes } from './utils'; import IBackend from './contracts/IBackend'; import { InternalConnectionOptions } from './contracts/InternalConnectionOptions'; import ThriftBackend from './thrift-backend/ThriftBackend'; @@ -190,7 +192,51 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I this.logger.log(LogLevel.info, 'Created DBSQLClient'); } + // Node folds `NODE_EXTRA_CA_CERTS` into the default trust store ONLY when the + // `ca` option is left unset — and `tls.rootCertificates` does not include those + // extra roots. Since we set `ca` explicitly to append `customCaCert`, we must + // re-read `NODE_EXTRA_CA_CERTS` ourselves so callers relying on it (e.g. a + // corporate proxy) do not silently lose those roots. + private static getExtraCaCerts(): Array { + const extraCertsPath = process.env.NODE_EXTRA_CA_CERTS; + if (!extraCertsPath) { + return []; + } + try { + return [fs.readFileSync(extraCertsPath, 'utf8')]; + } catch { + // Node itself silently ignores an unreadable NODE_EXTRA_CA_CERTS; mirror that. + return []; + } + } + private getConnectionOptions(options: ConnectionOptions): IConnectionOptions { + // mTLS requires both a client certificate and its private key. If exactly one is + // supplied, Node fails deep in the TLS handshake with an opaque error, so surface + // a clear client-side message instead. + const hasClientCert = options.clientCert !== undefined; + const hasClientKey = options.clientKey !== undefined; + if (hasClientCert !== hasClientKey) { + throw new HiveDriverError( + `DBSQLClient: mutual TLS requires both clientCert and clientKey; only \`${ + hasClientCert ? 'clientCert' : 'clientKey' + }\` was supplied. Provide the matching ${hasClientCert ? '`clientKey` (private key)' : '`clientCert`'}.`, + ); + } + + // Validate the PEM inputs up front with the same ordered BEGIN…END check the + // kernel path uses (normalizePemBytes), so a truncated/headerless/DER blob is + // rejected here with a named, actionable error instead of surfacing as an + // opaque failure deep in Node's TLS handshake. + const clientCert = + options.clientCert === undefined + ? undefined + : normalizePemBytes(options.clientCert, 'clientCert', 'certificate', 'DBSQLClient'); + const clientKey = + options.clientKey === undefined + ? undefined + : normalizePemBytes(options.clientKey, 'clientKey', 'private key', 'DBSQLClient'); + return { host: options.host, port: options.port || 443, @@ -198,6 +244,26 @@ export default class DBSQLClient extends EventEmitter implements IDBSQLClient, I https: true, socketTimeout: options.socketTimeout, proxy: options.proxy, + // `customCaCert` is ADDITIVE: Node's `ca` option replaces the system trust + // store, so we append the custom cert to the built-in roots AND any roots + // supplied via NODE_EXTRA_CA_CERTS to keep public Databricks warehouses + // trusted while also trusting the caller's CA. + ca: + options.customCaCert === undefined + ? undefined + : [ + ...tls.rootCertificates, + ...DBSQLClient.getExtraCaCerts(), + // Push the normalized Buffer as-is (Node's `ca` accepts a mixed + // Array) to match the cert/key treatment and keep + // byte-fidelity for Buffer inputs instead of round-tripping through utf-8. + normalizePemBytes(options.customCaCert, 'customCaCert', 'certificate', 'DBSQLClient'), + ], + // Client certificate + key for mutual TLS (mTLS). Both must be supplied together. + cert: clientCert, + key: clientKey, + // Validate the server certificate unless the caller explicitly opts out. + rejectUnauthorized: options.checkServerCertificate ?? true, headers: { 'User-Agent': buildUserAgentString(options.userAgentEntry), }, diff --git a/lib/connection/connections/HttpConnection.ts b/lib/connection/connections/HttpConnection.ts index 8c56019c..e8a511d1 100644 --- a/lib/connection/connections/HttpConnection.ts +++ b/lib/connection/connections/HttpConnection.ts @@ -68,7 +68,9 @@ export default class HttpConnection implements IConnectionProvider { const httpsAgentOptions: https.AgentOptions = { ...this.getAgentDefaultOptions(), minVersion: 'TLSv1.2', - rejectUnauthorized: false, + // Validate the server certificate by default; only skip verification when + // the caller explicitly opts out via `rejectUnauthorized: false`. + rejectUnauthorized: this.options.rejectUnauthorized ?? true, ca: this.options.ca, cert: this.options.cert, key: this.options.key, diff --git a/lib/connection/contracts/IConnectionOptions.ts b/lib/connection/contracts/IConnectionOptions.ts index 340b6fae..cf4e9ba3 100644 --- a/lib/connection/contracts/IConnectionOptions.ts +++ b/lib/connection/contracts/IConnectionOptions.ts @@ -19,7 +19,11 @@ export default interface IConnectionOptions { socketTimeout?: number; proxy?: ProxyOptions; - ca?: Buffer | string; + ca?: Buffer | string | Array; cert?: Buffer | string; key?: Buffer | string; + + // Whether the TLS server certificate is validated against the trusted CA set. + // When omitted the connection provider treats it as `true` (validation enabled). + rejectUnauthorized?: boolean; } diff --git a/lib/contracts/IDBSQLClient.ts b/lib/contracts/IDBSQLClient.ts index ec91d94c..bbaa4c69 100644 --- a/lib/contracts/IDBSQLClient.ts +++ b/lib/contracts/IDBSQLClient.ts @@ -59,6 +59,54 @@ export type ConnectionOptions = { proxy?: ProxyOptions; enableMetricViewMetadata?: boolean; + /** + * Verify the server's TLS certificate on the primary Thrift transport. + * Secure-by-default: omitting this leaves full chain + hostname verification + * enabled (`true`), matching Node's `https` default, the JDBC/ODBC drivers, + * and the SEA/kernel backend. + * + * Setting it to `false` disables server certificate verification entirely + * (any self-signed, expired, or wrong-hostname certificate is accepted), + * which exposes the connection — including bearer-token auth headers — to + * man-in-the-middle attacks. Only use `false` for local development against a + * trusted endpoint, and prefer supplying `customCaCert` instead. + * + * Mirrors the `checkServerCertificate` option on the SEA backend. + */ + checkServerCertificate?: boolean; + + /** + * PEM-encoded CA certificate (string or `Buffer`) added to the trust store + * **on top of** the built-in roots — for TLS-inspecting proxies or on-prem + * internal CAs. Because it is additive, connections to public Databricks + * warehouses keep working. + * + * Note: supplying this rebuilds the trust store from Node's **bundled Mozilla + * roots** (`tls.rootCertificates`) plus any roots from the `NODE_EXTRA_CA_CERTS` + * environment variable, then appends this certificate. It does **not** include + * OS-installed roots that Node would otherwise consult (e.g. on Node >= 22 run + * with `--use-system-ca`). If you rely on an enterprise root installed in the + * OS trust store, add it explicitly via `NODE_EXTRA_CA_CERTS` or `customCaCert` + * when using this option. + * + * Mirrors the `customCaCert` option on the SEA backend. + */ + customCaCert?: Buffer | string; + + /** + * PEM-encoded client certificate (string or `Buffer`) presented to the server + * for mutual TLS (mTLS). Must be supplied together with `clientKey`. Leave + * both unset for the usual token/OAuth flows, which do not require a client + * certificate. + */ + clientCert?: Buffer | string; + + /** + * PEM-encoded private key (string or `Buffer`) for `clientCert`, used for + * mutual TLS (mTLS). Must be supplied together with `clientCert`. + */ + clientKey?: Buffer | string; + /** * Retry-policy knobs governing how the driver retries retryable requests. * They apply to **both** backends: the Thrift `HttpRetryPolicy` reads them diff --git a/lib/kernel/KernelAuth.ts b/lib/kernel/KernelAuth.ts index 295fbb9b..289c3e47 100644 --- a/lib/kernel/KernelAuth.ts +++ b/lib/kernel/KernelAuth.ts @@ -16,7 +16,7 @@ import { ConnectionOptions } from '../contracts/IDBSQLClient'; import { InternalConnectionOptions } from '../contracts/InternalConnectionOptions'; import AuthenticationError from '../errors/AuthenticationError'; import HiveDriverError from '../errors/HiveDriverError'; -import { buildUserAgentString } from '../utils'; +import { buildUserAgentString, normalizePemBytes } from '../utils'; /** * Default local listener port for the U2M authorization-code callback. @@ -267,50 +267,6 @@ export function isBlankOrReserved(s: string): boolean { /** napi-rs marshals `maxConnections` as a `u32`; reject values it can't hold. */ const MAX_U32 = 0xffffffff; -/** - * Normalise a PEM input (`string` or `Buffer`) accepted on the public - * surface into the `Buffer` the napi shape requires. Does a light, - * ordered BEGIN…END sanity check so a truncated/headerless blob (or a - * stray page that merely contains the literals out of order, e.g. a - * proxy-intercept page) is rejected here rather than surfacing as an - * opaque kernel TLS error. The bytes are NOT fully parsed in JS — that - * is deferred to the kernel, which returns a meaningful error on a - * malformed PEM/key. - * - * `kind` selects the expected block: `'certificate'` matches a - * `CERTIFICATE` block; `'private key'` matches any `… PRIVATE KEY` block - * (PKCS#8 `PRIVATE KEY`, PKCS#1 `RSA PRIVATE KEY`, SEC1 `EC PRIVATE KEY`). - * - * Throws `HiveDriverError` when the value is empty or (for strings) - * lacks the expected PEM header. - */ -function normalizePemBytes(value: Buffer | string, optionName: string, kind: 'certificate' | 'private key'): Buffer { - if (typeof value === 'string') { - const re = - kind === 'certificate' - ? /-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/ - : /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]+?-----END [A-Z0-9 ]*PRIVATE KEY-----/; - if (!re.test(value)) { - const expected = - kind === 'certificate' - ? "a '-----BEGIN CERTIFICATE-----' … '-----END CERTIFICATE-----' block" - : "a 'BEGIN … PRIVATE KEY' / 'END … PRIVATE KEY' PEM block (PKCS#8, PKCS#1, or SEC1)"; - throw new HiveDriverError( - `kernel backend: \`${optionName}\` string does not look like a PEM ${kind} (expected ${expected}). ` + - 'Pass PEM text or a Buffer of PEM bytes.', - ); - } - return Buffer.from(value, 'utf8'); - } - if (Buffer.isBuffer(value)) { - if (value.length === 0) { - throw new HiveDriverError(`kernel backend: \`${optionName}\` Buffer is empty.`); - } - return value; - } - throw new HiveDriverError(`kernel backend: \`${optionName}\` must be a PEM string or a Buffer.`); -} - /** * Normalise the public TLS options into the napi shape. * @@ -321,11 +277,13 @@ function normalizePemBytes(value: Buffer | string, optionName: string, kind: 'ce * master verify toggle is on). Mirrors Python's `tls_verify_hostname`. * - `customCaCert` accepts a PEM string or `Buffer`; normalised to a * `Buffer` via {@link normalizePemBytes}. - * - `clientCertPem` / `clientKeyPem` carry the mutual-TLS client identity. - * They must be supplied **together** — supplying only one is rejected - * here with an actionable error (rather than waiting for the kernel's - * `InvalidArgument` at `openSession`). Each accepts a PEM string or - * `Buffer`, normalised the same way. + * - `clientCertPem` / `clientKeyPem` (or their public aliases + * `clientCert` / `clientKey`) carry the mutual-TLS client identity. The + * internal `*Pem` names win when both are present. They must be supplied + * **together** — supplying only one is rejected here with an actionable + * error (rather than waiting for the kernel's `InvalidArgument` at + * `openSession`). Each accepts a PEM string or `Buffer`, normalised the + * same way. * * Throws `HiveDriverError` when a cert/key is empty, mis-typed, lacks the * expected PEM header, or when only one half of the mTLS pair is set. @@ -334,8 +292,17 @@ export function buildKernelTlsOptions(options: ConnectionOptions): KernelTlsOpti // Read the kernel-only fields through the purpose-built internal options type // rather than an ad-hoc inline cast, so the shape can't silently drift from // its declaration and a typo'd key fails to compile. - const { checkServerCertificate, checkServerCertificateHostname, customCaCert, clientCertPem, clientKeyPem } = - options as ConnectionOptions & InternalConnectionOptions; + const merged = options as ConnectionOptions & InternalConnectionOptions; + const { checkServerCertificate, checkServerCertificateHostname, customCaCert } = merged; + + // The public mTLS options are `clientCert`/`clientKey` (see `ConnectionOptions`); + // the internal kernel-only aliases are `clientCertPem`/`clientKeyPem`. Accept + // both here — preferring the explicit internal alias when present — so a caller + // who sets the public `clientCert`/`clientKey` and runs on the kernel backend + // still gets mTLS configured instead of having their client identity silently + // dropped. + const clientCertPem = merged.clientCertPem ?? merged.clientCert; + const clientKeyPem = merged.clientKeyPem ?? merged.clientKey; const tls: KernelTlsOptions = {}; @@ -348,7 +315,7 @@ export function buildKernelTlsOptions(options: ConnectionOptions): KernelTlsOpti } if (customCaCert !== undefined) { - tls.customCaCert = normalizePemBytes(customCaCert, 'customCaCert', 'certificate'); + tls.customCaCert = normalizePemBytes(customCaCert, 'customCaCert', 'certificate', 'kernel backend'); } // mTLS client identity. Enforce both-or-neither up front so a caller who @@ -365,8 +332,18 @@ export function buildKernelTlsOptions(options: ConnectionOptions): KernelTlsOpti ); } if (hasCert && hasKey) { - tls.clientCertPem = normalizePemBytes(clientCertPem as Buffer | string, 'clientCertPem', 'certificate'); - tls.clientKeyPem = normalizePemBytes(clientKeyPem as Buffer | string, 'clientKeyPem', 'private key'); + tls.clientCertPem = normalizePemBytes( + clientCertPem as Buffer | string, + 'clientCertPem', + 'certificate', + 'kernel backend', + ); + tls.clientKeyPem = normalizePemBytes( + clientKeyPem as Buffer | string, + 'clientKeyPem', + 'private key', + 'kernel backend', + ); } return tls; diff --git a/lib/utils/index.ts b/lib/utils/index.ts index daddf482..86cece50 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -4,6 +4,7 @@ import formatProgress, { ProgressUpdateTransformer } from './formatProgress'; import LZ4 from './lz4'; import * as ProtocolVersion from './protocolVersion'; import serializeQueryTags from './queryTags'; +import normalizePemBytes from './normalizePemBytes'; export { definedOrError, @@ -13,4 +14,5 @@ export { LZ4, ProtocolVersion, serializeQueryTags, + normalizePemBytes, }; diff --git a/lib/utils/normalizePemBytes.ts b/lib/utils/normalizePemBytes.ts new file mode 100644 index 00000000..3c142c5f --- /dev/null +++ b/lib/utils/normalizePemBytes.ts @@ -0,0 +1,52 @@ +import HiveDriverError from '../errors/HiveDriverError'; + +/** + * Normalise a PEM input (`string` or `Buffer`) accepted on the public surface + * into a `Buffer`. Does a light, ordered BEGIN…END sanity check so a + * truncated/headerless/DER blob (or a stray page that merely contains the + * literals out of order, e.g. a proxy-intercept page) is rejected here rather + * than surfacing as an opaque TLS handshake error further down. The bytes are + * NOT fully parsed in JS — that is deferred to the TLS stack, which returns a + * meaningful error on a malformed PEM/key. + * + * `kind` selects the expected block: `'certificate'` matches a `CERTIFICATE` + * block; `'private key'` matches any `… PRIVATE KEY` block (PKCS#8 `PRIVATE + * KEY`, PKCS#1 `RSA PRIVATE KEY`, SEC1 `EC PRIVATE KEY`). + * + * `backendLabel` prefixes the error message so the caller (e.g. `DBSQLClient` + * on the Thrift path, `kernel backend` on the kernel path) is named accurately. + * + * Throws `HiveDriverError` when the value is empty or (for strings) lacks the + * expected PEM header. + */ +export default function normalizePemBytes( + value: Buffer | string, + optionName: string, + kind: 'certificate' | 'private key', + backendLabel: string, +): Buffer { + if (typeof value === 'string') { + const re = + kind === 'certificate' + ? /-----BEGIN CERTIFICATE-----[\s\S]+?-----END CERTIFICATE-----/ + : /-----BEGIN [A-Z0-9 ]*PRIVATE KEY-----[\s\S]+?-----END [A-Z0-9 ]*PRIVATE KEY-----/; + if (!re.test(value)) { + const expected = + kind === 'certificate' + ? "a '-----BEGIN CERTIFICATE-----' … '-----END CERTIFICATE-----' block" + : "a 'BEGIN … PRIVATE KEY' / 'END … PRIVATE KEY' PEM block (PKCS#8, PKCS#1, or SEC1)"; + throw new HiveDriverError( + `${backendLabel}: \`${optionName}\` string does not look like a PEM ${kind} (expected ${expected}). ` + + 'Pass PEM text or a Buffer of PEM bytes.', + ); + } + return Buffer.from(value, 'utf8'); + } + if (Buffer.isBuffer(value)) { + if (value.length === 0) { + throw new HiveDriverError(`${backendLabel}: \`${optionName}\` Buffer is empty.`); + } + return value; + } + throw new HiveDriverError(`${backendLabel}: \`${optionName}\` must be a PEM string or a Buffer.`); +} diff --git a/osv-scanner.toml b/osv-scanner.toml index b15bf0d4..d34d8d9c 100644 --- a/osv-scanner.toml +++ b/osv-scanner.toml @@ -30,3 +30,29 @@ # This file starts empty -- populate iteratively as the first scan run # surfaces real false positives or dev-only findings worth excluding. # Do not pre-populate with speculative suppressions. + +# brace-expansion DoS advisories, both with NO published fix as of +# 2026-08-05: the advisories name 1.1.17/1.1.18 and 2.1.4 as fixed, but +# npm's latest published releases are 1.1.16 and 2.1.3. So there is +# nothing to bump to -- we already bumped as far as the registry allows +# (1.1.15 -> 1.1.16, 2.1.1 -> 2.1.3, which cleared the earlier +# GHSA-3jxr-9vmj-r5cp). +# +# Dev-only: brace-expansion reaches us solely through the eslint / +# glob / test-exclude toolchains via minimatch. `npm ls brace-expansion +# --omit=dev` is empty, and both lockfile entries are marked +# "dev": true, so it is not reachable from the published dist/. +# The impact is DoS (ReDoS / OOM) on adversarial brace patterns, which +# would require untrusted input to our own lint/test globs. +# +# Revisit when 1.1.18 / 2.1.4 land on npm and drop these entries. + +[[IgnoredVulns]] +id = "GHSA-mh99-v99m-4gvg" +ignoreUntil = "2027-02-05T00:00:00Z" +reason = "dev-only (eslint/glob/test-exclude -> minimatch); not reachable from shipped dist/. No published fix: advisory lists 1.1.17 as fixed but 1.1.16 is npm's latest 1.x." + +[[IgnoredVulns]] +id = "GHSA-rgw5-rvv9-x895" +ignoreUntil = "2027-02-05T00:00:00Z" +reason = "dev-only (eslint/glob/test-exclude -> minimatch); not reachable from shipped dist/. No published fix: advisory lists 1.1.18/2.1.4 as fixed but npm's latest are 1.1.16/2.1.3." diff --git a/package-lock.json b/package-lock.json index 417060fe..2e65e587 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1860,9 +1860,9 @@ } }, "node_modules/brace-expansion": { - "version": "1.1.15", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.15.tgz", - "integrity": "sha512-EwOCDEex4quD37XhqM3omwtMoJjr//isUZz1JopUNWms+4Z2ViyM/k1YIRePpoVNnQhENnxtFjLaxNHrT7xIUg==", + "version": "1.1.16", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", + "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", "dev": true, "license": "MIT", "dependencies": { @@ -3993,9 +3993,9 @@ } }, "node_modules/ip-address": { - "version": "10.2.0", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.2.0.tgz", - "integrity": "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA==", + "version": "10.3.1", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.3.1.tgz", + "integrity": "sha512-1e9d3kb97NHJTIJDZW9rKqW2h6+dFa50Dy0fpPSMQp2ADje5gvKsXmdiK6dwY5t76TaTt5+P5N1Y/LoToIxP6g==", "license": "MIT", "engines": { "node": ">= 12" @@ -4786,9 +4786,9 @@ } }, "node_modules/minimatch/node_modules/brace-expansion": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.1.tgz", - "integrity": "sha512-WR1cURNjuvBLMZBMbqM0UoE+WAfdUcEV1ccD8PVBVOI+Z3ND4+SZbN8RsfT2bMuG1qwz5RFvPukSZm5fF2D5eA==", + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.3.tgz", + "integrity": "sha512-DRdx5neNsG/QXbniLFWi2YmC/68oeOOmKz6zOjVk6ZS1ZLXgLIKqVEc6hWsmkjBbgii0SwaBTcJ5XKj5gzY/4A==", "dev": true, "license": "MIT", "dependencies": { @@ -8109,9 +8109,9 @@ "dev": true }, "brace-expansion": { - "version": "1.1.15", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.15.tgz", - "integrity": "sha512-EwOCDEex4quD37XhqM3omwtMoJjr//isUZz1JopUNWms+4Z2ViyM/k1YIRePpoVNnQhENnxtFjLaxNHrT7xIUg==", + "version": "1.1.16", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-1.1.16.tgz", + "integrity": "sha512-IDw48K2/2kRkg9LdJxurvq3lV3aBgq0REY89duEqFRthjlPdXHKMj7EnQOXVckxzgisinf3nHfrcE2FufFLXMw==", "dev": true, "requires": { "balanced-match": "^1.0.0", @@ -9633,9 +9633,9 @@ } }, "ip-address": { - "version": "10.2.0", - "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.2.0.tgz", - "integrity": "sha512-/+S6j4E9AHvW9SWMSEY9Xfy66O5PWvVEJ08O0y5JGyEKQpojb0K0GKpz/v5HJ/G0vi3D2sjGK78119oXZeE0qA==" + "version": "10.3.1", + "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-10.3.1.tgz", + "integrity": "sha512-1e9d3kb97NHJTIJDZW9rKqW2h6+dFa50Dy0fpPSMQp2ADje5gvKsXmdiK6dwY5t76TaTt5+P5N1Y/LoToIxP6g==" }, "is-arrayish": { "version": "0.3.2", @@ -10204,9 +10204,9 @@ }, "dependencies": { "brace-expansion": { - "version": "2.1.1", - "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.1.tgz", - "integrity": "sha512-WR1cURNjuvBLMZBMbqM0UoE+WAfdUcEV1ccD8PVBVOI+Z3ND4+SZbN8RsfT2bMuG1qwz5RFvPukSZm5fF2D5eA==", + "version": "2.1.3", + "resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.1.3.tgz", + "integrity": "sha512-DRdx5neNsG/QXbniLFWi2YmC/68oeOOmKz6zOjVk6ZS1ZLXgLIKqVEc6hWsmkjBbgii0SwaBTcJ5XKj5gzY/4A==", "dev": true, "requires": { "balanced-match": "^1.0.0" diff --git a/tests/unit/DBSQLClient.test.ts b/tests/unit/DBSQLClient.test.ts index 4cf16838..312cf603 100644 --- a/tests/unit/DBSQLClient.test.ts +++ b/tests/unit/DBSQLClient.test.ts @@ -1,5 +1,6 @@ import { expect, AssertionError } from 'chai'; import sinon from 'sinon'; +import fs from 'fs'; import DBSQLClient, { ThriftLibrary } from '../../lib/DBSQLClient'; import DBSQLSession from '../../lib/DBSQLSession'; import ThriftBackend from '../../lib/thrift-backend/ThriftBackend'; @@ -62,6 +63,134 @@ describe('DBSQLClient.connect', () => { expect(connectionOptions.path).to.equal(path); }); + it('should validate the server certificate by default', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions'](connectOptions); + + expect(connectionOptions.rejectUnauthorized).to.be.true; + }); + + it('should map checkServerCertificate to rejectUnauthorized', async () => { + const client = new DBSQLClient(); + + expect(client['getConnectionOptions']({ ...connectOptions, checkServerCertificate: false }).rejectUnauthorized).to + .be.false; + expect(client['getConnectionOptions']({ ...connectOptions, checkServerCertificate: true }).rejectUnauthorized).to.be + .true; + }); + + it('should not set a custom CA when customCaCert is omitted', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions'](connectOptions); + + expect(connectionOptions.ca).to.be.undefined; + }); + + it('should add customCaCert on top of the system root certificates (additive)', async () => { + const client = new DBSQLClient(); + const customCaCert = '-----BEGIN CERTIFICATE-----\ncustom\n-----END CERTIFICATE-----\n'; + + const connectionOptions = client['getConnectionOptions']({ ...connectOptions, customCaCert }); + + expect(connectionOptions.ca).to.be.an('array'); + const ca = connectionOptions.ca as Array; + // The custom cert must be present, pushed as a Buffer (byte-fidelity, parity with cert/key)... + expect(ca.map((entry) => entry.toString('utf8'))).to.include(customCaCert); + // ...alongside the built-in system roots (so public warehouses still validate). + expect(ca.length).to.be.greaterThan(1); + }); + + it('should preserve NODE_EXTRA_CA_CERTS roots when customCaCert is set', async () => { + const client = new DBSQLClient(); + const customCaCert = '-----BEGIN CERTIFICATE-----\ncustom\n-----END CERTIFICATE-----\n'; + const extraCaCert = '-----BEGIN CERTIFICATE-----\nextra\n-----END CERTIFICATE-----\n'; + + const previousEnv = process.env.NODE_EXTRA_CA_CERTS; + process.env.NODE_EXTRA_CA_CERTS = '/path/to/extra-ca.pem'; + const readFileSync = sinon.stub(fs, 'readFileSync').returns(extraCaCert); + try { + const connectionOptions = client['getConnectionOptions']({ ...connectOptions, customCaCert }); + + const ca = connectionOptions.ca as Array; + const caStrings = ca.map((entry) => entry.toString('utf8')); + // The custom cert is pushed as a Buffer (byte-fidelity, parity with cert/key). + expect(caStrings).to.include(customCaCert); + // Roots injected via NODE_EXTRA_CA_CERTS must survive the additive rebuild, + // otherwise callers relying on that env var lose their trust anchors. + expect(caStrings).to.include(extraCaCert); + } finally { + readFileSync.restore(); + if (previousEnv === undefined) { + delete process.env.NODE_EXTRA_CA_CERTS; + } else { + process.env.NODE_EXTRA_CA_CERTS = previousEnv; + } + } + }); + + it('should not set client cert/key when mTLS options are omitted', async () => { + const client = new DBSQLClient(); + + const connectionOptions = client['getConnectionOptions'](connectOptions); + + expect(connectionOptions.cert).to.be.undefined; + expect(connectionOptions.key).to.be.undefined; + }); + + it('should map clientCert/clientKey to cert/key for mutual TLS', async () => { + const client = new DBSQLClient(); + + const clientCert = '-----BEGIN CERTIFICATE-----\nclient-cert\n-----END CERTIFICATE-----\n'; + const clientKey = '-----BEGIN PRIVATE KEY-----\nclient-key\n-----END PRIVATE KEY-----\n'; + const connectionOptions = client['getConnectionOptions']({ + ...connectOptions, + clientCert, + clientKey, + }); + + // Normalised to a Buffer of the PEM bytes (parity with the kernel path). + expect((connectionOptions.cert as Buffer).toString('utf8')).to.equal(clientCert); + expect((connectionOptions.key as Buffer).toString('utf8')).to.equal(clientKey); + }); + + it('should reject a malformed PEM clientCert with a named error (parity with the kernel path)', async () => { + const client = new DBSQLClient(); + + expect(() => + client['getConnectionOptions']({ + ...connectOptions, + clientCert: 'not-a-pem', + clientKey: '-----BEGIN PRIVATE KEY-----\nk\n-----END PRIVATE KEY-----\n', + }), + ).to.throw(/`clientCert` string does not look like a PEM certificate/); + }); + + it('should reject a malformed PEM customCaCert with a named error (parity with the kernel path)', async () => { + const client = new DBSQLClient(); + + expect(() => client['getConnectionOptions']({ ...connectOptions, customCaCert: 'not-a-pem' })).to.throw( + /`customCaCert` string does not look like a PEM certificate/, + ); + }); + + it('should throw if only clientCert is supplied for mutual TLS', async () => { + const client = new DBSQLClient(); + + expect(() => client['getConnectionOptions']({ ...connectOptions, clientCert: 'client-cert' })).to.throw( + /only `clientCert` was supplied/, + ); + }); + + it('should throw if only clientKey is supplied for mutual TLS', async () => { + const client = new DBSQLClient(); + + expect(() => client['getConnectionOptions']({ ...connectOptions, clientKey: 'client-key' })).to.throw( + /only `clientKey` was supplied/, + ); + }); + it('should initialize connection state', async () => { const client = new DBSQLClient(); diff --git a/tests/unit/connection/connections/HttpConnection.test.ts b/tests/unit/connection/connections/HttpConnection.test.ts index 44d38a69..ec37e72f 100644 --- a/tests/unit/connection/connections/HttpConnection.test.ts +++ b/tests/unit/connection/connections/HttpConnection.test.ts @@ -26,7 +26,7 @@ describe('HttpConnection.connect', () => { expect(anotherConnection).to.eq(thriftConnection); }); - it('should set SSL certificates and disable rejectUnauthorized', async () => { + it('should set SSL certificates and validate the server certificate by default', async () => { const connection = new HttpConnection( { host: 'localhost', @@ -42,12 +42,46 @@ describe('HttpConnection.connect', () => { const thriftConnection = await connection.getThriftConnection(); - expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.false; + expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.true; expect(thriftConnection.config.agent.options.ca).to.be.eq('ca'); expect(thriftConnection.config.agent.options.cert).to.be.eq('cert'); expect(thriftConnection.config.agent.options.key).to.be.eq('key'); }); + it('should allow disabling server certificate validation via rejectUnauthorized', async () => { + const connection = new HttpConnection( + { + host: 'localhost', + port: 10001, + path: '/hive', + https: true, + rejectUnauthorized: false, + }, + new ClientContextStub(), + ); + + const thriftConnection = await connection.getThriftConnection(); + + expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.false; + }); + + it('should keep server certificate validation enabled when rejectUnauthorized is true', async () => { + const connection = new HttpConnection( + { + host: 'localhost', + port: 10001, + path: '/hive', + https: true, + rejectUnauthorized: true, + }, + new ClientContextStub(), + ); + + const thriftConnection = await connection.getThriftConnection(); + + expect(thriftConnection.config.agent.options.rejectUnauthorized).to.be.true; + }); + it('should initialize http agents', async () => { const connection = new HttpConnection( { diff --git a/tests/unit/kernel/connectionOptions.test.ts b/tests/unit/kernel/connectionOptions.test.ts index efa99aa7..b5b13d41 100644 --- a/tests/unit/kernel/connectionOptions.test.ts +++ b/tests/unit/kernel/connectionOptions.test.ts @@ -172,6 +172,20 @@ describe('KernelAuth mTLS options (buildKernelTlsOptions)', () => { expect(tls.clientKeyPem).to.equal(key); }); + it('accepts the public clientCert/clientKey aliases (kernel path still gets mTLS)', () => { + const tls = buildKernelTlsOptions(opts({ clientCert: CERT_PEM, clientKey: KEY_PEM })); + expect(tls.clientCertPem?.toString('utf8')).to.equal(CERT_PEM); + expect(tls.clientKeyPem?.toString('utf8')).to.equal(KEY_PEM); + }); + + it('prefers the internal clientCertPem/clientKeyPem over the public aliases when both are set', () => { + const tls = buildKernelTlsOptions( + opts({ clientCertPem: CERT_PEM, clientKeyPem: KEY_PEM, clientCert: 'nope', clientKey: 'nope' }), + ); + expect(tls.clientCertPem?.toString('utf8')).to.equal(CERT_PEM); + expect(tls.clientKeyPem?.toString('utf8')).to.equal(KEY_PEM); + }); + it('rejects supplying only the client cert', () => { expect(() => buildKernelTlsOptions(opts({ clientCertPem: CERT_PEM }))).to.throw( HiveDriverError,