sea-auth-u2m: round-3 fixup — namespace kernel metadata, dedupe predicate, type-guard envelope, treat blank secret as U2M

Addresses devils-advocate-auth-u2m-2 round-1 findings on commit
98d5ecf. NF-N1 is a real bug (collision between the kernel envelope's
textual `errorCode` field and the pre-existing enum-typed `errorCode`
on `OperationStateError` / `RetryError`). NF-N2..NF-N4 are mediums.
Includes B-4 collapse (one defineProperty helper for both top-level
sqlState and the kernel metadata namespace).

## NF-N1 (HIGH) — namespace kernel metadata + B-4 collapse

Before this commit, `decodeNapiKernelError` `defineProperty`d each of
the 5 kernel envelope fields (`errorCode`, `vendorCode`, `httpStatus`,
`retryable`, `queryId`) directly on the JS error. But
`OperationStateError.ts:21` and `RetryError.ts:12` already declare a
top-level `errorCode: enum` field, and `DBSQLOperation.ts:209`
switches on `err.errorCode === OperationStateErrorCode.Canceled`. A
Cancelled kernel envelope with `errorCode: "USER_REQUESTED_CANCEL"`
would clobber the enum string `'CANCELED'`, silently breaking
cancel detection.

Going with option (a) from team-lead's three remediation paths:
nest the 5 kernel envelope fields under a single
`error.kernelMetadata.*` namespace. Clean separation, no surprise,
matches the way `attachSqlState`'s pattern keeps `sqlState` at the
top level (which is collision-free).

Folded B-4 simultaneously: replaced the two helpers (`attachSqlState`,
`attachMetadata`) with one `defineErrorMetadata(error, key, value)`
that owns the `defineProperty` flags. Both `sqlState` (top-level)
and `kernelMetadata` (namespaced) go through the same helper now.

## NF-N2 (medium) — dedupe e2e `looksReal` against production

`auth-m2m-e2e.test.ts:58-62` had a `looksReal` predicate that was
still case-sensitive even though round-2's `isBlankOrReserved` is
case-insensitive. Exported `isBlankOrReserved` from `SeaAuth.ts` and
imported it in the e2e test. Eliminates the predicate-drift risk
(also resolves the bloat-watchdog's B-3).

## NF-N3 (medium) — blank/reserved oauthClientSecret routes to U2M

A user passing `oauthClientSecret: process.env.MY_SECRET || ''`
previously hit the M2M arm's "secret must be non-empty" rejection,
which never mentions U2M. Now blank/whitespace/reserved-literal
secrets route to the U2M arm — where if `oauthClientId` is also
set, the dedicated "not supported on U2M" rejection fires (round-2
NF-2 work). The error message correctly points at the right flow.

Updated 5 existing test cases that had assumed the old M2M-rejects
behavior; they now assert the U2M-via-id-rejection path. Added 3
new cases (empty string, whitespace-only, literal `'undefined'`
routing to U2M happy path when no clientId is set).

## NF-N4 (medium) — per-field envelope type-guards

`decodeNapiKernelError` previously cast `parsed` to a typed shape
without runtime-validating the 5 optional fields. A kernel bug that
emits `retryable: "true"` (string) instead of `true` (boolean)
would propagate the wrong-typed property to JS callers. Added a
`buildKernelMetadata(parsed: Record<string, unknown>)` helper that
checks `typeof` per-field and discards mis-typed values. New unit
test verifies all 5 wrong-type variants are dropped while a single
correctly-typed field survives.

Also: when the parsed envelope has no validated metadata fields,
the decoder now omits the `kernelMetadata` namespace entirely
(rather than attaching `{}`-shaped noise). Pinned by a new unit
test.

## DEFERRED to Phase 2

- NF-N5 (low — SeaNativeLoader top-level require): per team-lead's
  guidance, defer to Phase 2 (deploy-time visibility issue).
- Language-expert-auth-u2m-2's 1 medium + 6 low.

## Kernel fix consumption note

team-lead's message indicated kernel-author landed the
Error::io() → Error::unauthenticated() fix on `krn-napi-binding` at
commit `a64479a`. My napi binding's path-dep
(`native/sea/Cargo.toml`) points to
`../../../../databricks-sql-kernel-sea-WT/async-public-api`, not
`krn-napi-binding`. As of the round-3 build, `async-public-api`
still has the OLD `Error::io()` at `m2m.rs:270`. So my rebuild
this round picks up the new TS code only — NOT the kernel error-
class fix.

Consequence for the bad-secret e2e: it would STILL fail with
HiveDriverError (not AuthenticationError) on a live run today,
because the kernel envelope on the worktree my path-dep reaches
still carries `code: "Internal"`. The kernel author's fix needs to
land on `async-public-api` (the branch my path-dep tracks), or my
path-dep needs to point at `krn-napi-binding`. Flagging to
team-lead in the reply.

Tests:
- Unit: 85/85 pass (was 79 before this commit: +6 net — added 4
  new cases for NF-N3 routing + NF-N1 collision + NF-N4 type-guard +
  NF-N4 metadata-omitted; flipped 3 existing M2M-rejection cases to
  U2M-rejection-via-id; updated 2 NF-4 metadata tests to read
  through the new namespace).
- TypeScript build: clean.
- Native build: cached (no Rust changes from this commit).

Co-authored-by: Isaac

Author: msrathore-db

lib/sea/SeaAuth.ts

@@ -84,8 +84,11 @@ function prependSlash(str: string): string {
  * shell exports (e.g. `export FOO="$UNSET_VAR"`) produce. Surfacing
  * these here means an OAuth flow's `invalid_client` from the workspace
  * is always a real credential mismatch, never a malformed-input passthrough.
+ *
+ * Exported so the integration-test env-gate can reuse the same predicate
+ * and stay in lockstep with production (B-3 fix).
  */
-function isBlankOrReserved(s: string): boolean {
+export function isBlankOrReserved(s: string): boolean {
   const normalized = s.trim().toLowerCase();
   return normalized.length === 0 || normalized === 'undefined' || normalized === 'null';
 }
@@ -180,7 +183,14 @@ export function buildSeaConnectionOptions(options: ConnectionOptions): SeaNative
 
     // Flow selector mirrors thrift's `DBSQLClient.createAuthProvider`
     // (`DBSQLClient.ts:143`): `oauthClientSecret defined ? M2M : U2M`.
-    if (oauth.oauthClientSecret === undefined) {
+    // Blank or buggy-shell-export secrets route to U2M (rather than
+    // M2M-with-bad-secret) so the error message correctly points the user
+    // at the right flow. `oauthClientSecret: process.env.MY_SECRET || ''`
+    // is a common shape; routing it to the M2M arm would surface an
+    // M2M-specific error that never mentions U2M.
+    const secretIsBlank =
+      typeof oauth.oauthClientSecret === 'string' && isBlankOrReserved(oauth.oauthClientSecret);
+    if (oauth.oauthClientSecret === undefined || secretIsBlank) {
       // U2M.
       if (oauth.oauthClientId !== undefined) {
         // The kernel hardcodes `client_id = "databricks-cli"` for U2M;

lib/sea/SeaErrorMapping.ts

@@ -52,31 +52,52 @@ export type KernelErrorCode =
   | 'SqlError';
 
 /**
- * An `Error` with a preserved SQLSTATE on the `sqlState` property. Used as the
- * narrowed return type of {@link mapKernelErrorToJsError} so callers that need
- * the SQLSTATE can `error.sqlState` without an `any` cast.
+ * Optional metadata fields the kernel may attach via the
+ * `__databricks_error__:` envelope (per `native/sea/src/error.rs:50-89`).
+ *
+ * `errorCode` is namespaced under `kernelMetadata` rather than placed at
+ * the top level because two existing JS-side error classes
+ * (`OperationStateError`, `RetryError`) already declare a top-level
+ * `errorCode: enum` field, and `DBSQLOperation.ts:209` switches on it
+ * (`err.errorCode === OperationStateErrorCode.Canceled`). Top-level
+ * defineProperty would clobber that enum with a kernel string and break
+ * cancel/close detection.
+ */
+export interface KernelMetadata {
+  errorCode?: string;
+  vendorCode?: number;
+  httpStatus?: number;
+  retryable?: boolean;
+  queryId?: string;
+}
+
+/**
+ * An `Error` carrying optional SEA-side kernel context. `sqlState` is
+ * exposed at the top level (no collision in the existing driver error
+ * tree); the remaining envelope fields live under a `kernelMetadata`
+ * namespace to avoid clobbering pre-existing `errorCode` semantics on
+ * `OperationStateError` / `RetryError`.
  */
 export interface ErrorWithSqlState extends Error {
   sqlState?: string;
+  kernelMetadata?: KernelMetadata;
 }
 
 /**
- * Attach the kernel's SQLSTATE to the JS error object via the `sqlState` property.
- * The driver has no pre-existing `sqlState` convention (no other error class
- * sets it today) so this single helper defines it for the SEA path.
+ * Attach a non-enumerable own-property to the error. The shape matches
+ * Node's convention for attaching `.code` to system errors:
+ * non-enumerable (clean `JSON.stringify`) but readable via direct
+ * property access and `Object.getOwnPropertyDescriptor`. One helper for
+ * both the top-level `sqlState` and the namespaced `kernelMetadata`
+ * object so the `defineProperty` flags live in exactly one place.
  */
-function attachSqlState(error: ErrorWithSqlState, sqlstate?: string): ErrorWithSqlState {
-  if (sqlstate !== undefined) {
-    // Using Object.defineProperty so the property is non-enumerable but still
-    // visible via direct access — matches the way Node attaches `.code` to system errors.
-    Object.defineProperty(error, 'sqlState', {
-      value: sqlstate,
-      writable: true,
-      enumerable: false,
-      configurable: true,
-    });
-  }
-  return error;
+function defineErrorMetadata<K extends string, V>(error: Error, key: K, value: V): void {
+  Object.defineProperty(error, key, {
+    value,
+    writable: true,
+    enumerable: false,
+    configurable: true,
+  });
 }
 
 /**
@@ -146,37 +167,37 @@ export function mapKernelErrorToJsError(kErr: KernelErrorShape): ErrorWithSqlSta
       break;
   }
 
-  return attachSqlState(error, sqlstate);
+  if (sqlstate !== undefined) {
+    defineErrorMetadata(error, 'sqlState', sqlstate);
+  }
+  return error;
 }
 
 /**
- * Optional metadata fields the kernel may attach via the
- * `__databricks_error__:` envelope (per `native/sea/src/error.rs:50-89`).
- * Attached to the decoded JS error as non-enumerable own-properties so
- * callers can read them (e.g. `error.httpStatus`) without polluting
- * `JSON.stringify(error)` output. Matches the way Node attaches
- * `.code` to system errors and the way `attachSqlState` works above.
+ * Build a {@link KernelMetadata} object from a parsed envelope, applying
+ * per-field type validation. A kernel-side bug that emits, say,
+ * `retryable: "true"` (string) instead of `true` (boolean) would
+ * otherwise leak the wrong-typed value through to JS callers; the
+ * type-guard discards the malformed field rather than passing it through.
  */
-interface KernelErrorMetadata {
-  errorCode?: string;
-  vendorCode?: number;
-  httpStatus?: number;
-  retryable?: boolean;
-  queryId?: string;
-}
-
-function attachMetadata(error: Error, meta: KernelErrorMetadata): void {
-  for (const key of ['errorCode', 'vendorCode', 'httpStatus', 'retryable', 'queryId'] as const) {
-    const value = meta[key];
-    if (value !== undefined) {
-      Object.defineProperty(error, key, {
-        value,
-        writable: true,
-        enumerable: false,
-        configurable: true,
-      });
-    }
+function buildKernelMetadata(parsed: Record<string, unknown>): KernelMetadata {
+  const meta: KernelMetadata = {};
+  if (typeof parsed.errorCode === 'string') {
+    meta.errorCode = parsed.errorCode;
+  }
+  if (typeof parsed.vendorCode === 'number') {
+    meta.vendorCode = parsed.vendorCode;
   }
+  if (typeof parsed.httpStatus === 'number') {
+    meta.httpStatus = parsed.httpStatus;
+  }
+  if (typeof parsed.retryable === 'boolean') {
+    meta.retryable = parsed.retryable;
+  }
+  if (typeof parsed.queryId === 'string') {
+    meta.queryId = parsed.queryId;
+  }
+  return meta;
 }
 
 /**
@@ -186,11 +207,12 @@ function attachMetadata(error: Error, meta: KernelErrorMetadata): void {
  *  - Structured kernel error: `Error.message` starts with
  *    {@link ERROR_SENTINEL} followed by a JSON envelope. We strip the
  *    sentinel, parse the JSON, route the {@link KernelErrorShape}
- *    through {@link mapKernelErrorToJsError}, and attach all remaining
- *    envelope fields (`errorCode`, `vendorCode`, `httpStatus`,
- *    `retryable`, `queryId`) as non-enumerable own-properties on the
- *    returned error. Thrift parity demand: thrift errors carry these
- *    fields, so SEA errors must too.
+ *    through {@link mapKernelErrorToJsError}, and attach the remaining
+ *    envelope fields under a single non-enumerable `kernelMetadata`
+ *    namespace. Namespacing avoids the collision with
+ *    `OperationStateError.errorCode` (an enum) and `RetryError.errorCode`
+ *    (an enum), each of which is already switched on at the JS layer
+ *    (see `DBSQLOperation.ts:209`).
  *  - Binding-side error (e.g. `napi::Error::new(InvalidArg, "openSession:
  *    \`token\` is required for the requested auth mode")` produced by
  *    the binding's own validation): returned unchanged. These don't
@@ -229,28 +251,25 @@ export function decodeNapiKernelError(err: unknown): Error {
     return err;
   }
 
-  const kErr = parsed as {
-    code: string;
-    message: string;
-    sqlState?: string;
-    errorCode?: string;
-    vendorCode?: number;
-    httpStatus?: number;
-    retryable?: boolean;
-    queryId?: string;
-  };
+  const envelope = parsed as Record<string, unknown>;
+  const code = envelope.code as string;
+  const msg = envelope.message as string;
+  const sqlState = typeof envelope.sqlState === 'string' ? envelope.sqlState : undefined;
 
-  const jsErr = mapKernelErrorToJsError({
-    code: kErr.code,
-    message: kErr.message,
-    sqlstate: kErr.sqlState,
-  });
-  attachMetadata(jsErr, {
-    errorCode: kErr.errorCode,
-    vendorCode: kErr.vendorCode,
-    httpStatus: kErr.httpStatus,
-    retryable: kErr.retryable,
-    queryId: kErr.queryId,
-  });
+  const jsErr = mapKernelErrorToJsError({ code, message: msg, sqlstate: sqlState });
+
+  const meta = buildKernelMetadata(envelope);
+  // Skip the namespace attachment entirely when no fields validated
+  // through — keeps `err.kernelMetadata` absent rather than `{}` for
+  // simple envelopes (the common case).
+  if (
+    meta.errorCode !== undefined ||
+    meta.vendorCode !== undefined ||
+    meta.httpStatus !== undefined ||
+    meta.retryable !== undefined ||
+    meta.queryId !== undefined
+  ) {
+    defineErrorMetadata(jsErr, 'kernelMetadata', meta);
+  }
   return jsErr;
 }

tests/integration/sea/auth-m2m-e2e.test.ts

@@ -15,6 +15,7 @@
 import { expect } from 'chai';
 import { DBSQLClient } from '../../../lib';
 import AuthenticationError from '../../../lib/errors/AuthenticationError';
+import { isBlankOrReserved } from '../../../lib/sea/SeaAuth';
 
 /**
  * sea-auth M1 OAuth M2M end-to-end:
@@ -50,16 +51,15 @@ describe('sea-auth e2e — OAuth M2M through DBSQLClient ↔ SeaBackend ↔ napi
   this.timeout(120_000);
 
   before(function gate() {
-    // Reject not just absent env vars but also literal `'undefined'` /
-    // `'null'` / whitespace-only values from buggy shell exports — these
+    // Reject not just absent env vars but also blank/whitespace/literal-
+    // `'undefined'`/`'null'` values from buggy shell exports — these
     // would otherwise reach the workspace as bogus creds and yield an
     // `invalid_client` indistinguishable from a real SP-not-registered
-    // issue.
-    const looksReal = (s: string | undefined): s is string => {
-      if (typeof s !== 'string') return false;
-      const t = s.trim();
-      return t.length > 0 && t !== 'undefined' && t !== 'null';
-    };
+    // issue. Reuse the production `isBlankOrReserved` predicate so the
+    // test gate stays in lockstep with the case-insensitive variant
+    // shipped in round-2 (B-3 fix).
+    const looksReal = (s: string | undefined): s is string =>
+      typeof s === 'string' && !isBlankOrReserved(s);
     if (!looksReal(host) || !looksReal(path) || !looksReal(oauthClientId) || !looksReal(oauthClientSecret)) {
       // eslint-disable-next-line no-invalid-this
       this.skip();