From 9bfd9a52d36109d9c01338e7228de25689603def Mon Sep 17 00:00:00 2001 From: Parth Bansal Date: Wed, 3 Jun 2026 15:54:14 +0000 Subject: [PATCH] fix(auth): clearer U2M errors for missing profiles and modern CLI detection ## Summary Fixes two misleading failure modes in the Databricks CLI ("U2M") credential provider: a non-existent profile no longer surfaces a confusing host error with a doubled `Error:` prefix, and modern CLI wrappers/shims smaller than 1 MB are no longer misclassified as the legacy Python CLI. ## Why `newU2mCredentials` shells out to the `databricks` CLI to obtain access tokens, and two of its error paths produced output that pointed developers in the wrong direction. First, when the requested profile does not exist, `databricks auth token --profile X` treats the unknown profile as "no profile configured" and fails with a host-related message such as `Error: cannot configure default credentials, please check ... or specify --host`. The provider passed that stderr through verbatim and wrapped it as `cannot get access token: `. Because the CLI already prefixes its stderr with `Error:`, the rendered result read `cannot get access token: Error: ...`, and once the surrounding `U2mCredentialsError` is logged the prefix doubled again. A developer who simply mistyped a profile name was told to specify a host, with a stutter of `Error:` labels. Second, the provider distinguished the modern Go-based CLI (>= 0.100.0) from the legacy Python CLI purely by binary size: any file under 1 MB was reported as `LEGACY_CLI_DETECTED`. That heuristic flags legitimate setups where `databricks` is a small wrapper, shim, or symlink-resolving launcher rather than the full statically linked binary, blocking auth even when a perfectly modern CLI is installed. ## What changed ### Interface changes - **`U2mCredentialsErrorCode`** gains a `'PROFILE_NOT_FOUND'` member. `newU2mCredentials` now rejects with this code (and the message `profile "" was not found in the Databricks config file`) when the requested profile is absent. ### Behavioral changes - A request for a non-existent profile now fails fast with `PROFILE_NOT_FOUND` naming the missing profile, instead of running the CLI and surfacing its host error. The profile is pre-checked via `resolve` from `@databricks/sdk-core/profiles`, which already reports `PROFILE_NOT_FOUND` for an explicitly requested-but-missing profile; any other resolution error is rethrown unchanged. - CLI stderr that is already prefixed with `Error:` has that prefix stripped before the SDK wraps it, so the message no longer doubles the label. - CLI modernity is now determined by running `databricks version` and parsing its `Databricks CLI v` banner, requiring `>= 0.100.0`. The 1 MB binary-size check is kept only as a fallback for when the version command cannot be run or its output cannot be parsed. Wrappers and shims that report a modern version are accepted. ### Internal changes - `validateCliPath` delegates the legacy-vs-modern decision to a new `isModernCli(path, size)` helper, backed by `cliVersion` (runs ` version`, parses the semver) and `isAtLeastMinVersion` (compares against `MIN_CLI_VERSION`). - Added `ensureProfileExists` and `stripErrorPrefix` helpers in `u2m.ts`. - The `u2m` test suite mocks `@databricks/sdk-core/profiles` so the profile pre-check is deterministic and does not read the developer's `~/.databrickscfg`. The CLI exec mock now routes by sub-command (`version` vs `auth token`) so the version probe and the token request can be stubbed independently. New cases cover the profile pre-check, the `Error:`-prefix stripping, a pre-0.100.0 reported version, and the size fallback when the version cannot be read. ### Go-parity note The Go reference SDK in this checkout implements U2M via a persistent OAuth flow rather than a CLI shell-out, so there is no Go counterpart for this provider's CLI detection. The `databricks version` parsing here follows the Databricks CLI's own version banner (`Databricks CLI v`); the legacy Python launcher does not emit that banner, so it falls through to the size heuristic. This is a deliberate, TypeScript-only divergence rather than a port of a specific Go function. ## How is this tested? `npm run build`, `npm test`, `npm run typecheck`, and `npm run lint` for `@databricks/sdk-auth` (built on top of `@databricks/sdk-core`) all pass. The 14-case `u2m` suite covers the new `PROFILE_NOT_FOUND` path, `Error:`-prefix de-duplication, version-based modern-CLI detection, and the binary-size fallback. Co-authored-by: Isaac --- packages/auth/src/credentials/errors.ts | 1 + packages/auth/src/credentials/u2m.ts | 114 +++++++++++++++++- packages/auth/tests/credentials/u2m.test.ts | 126 +++++++++++++++++--- 3 files changed, 218 insertions(+), 23 deletions(-) diff --git a/packages/auth/src/credentials/errors.ts b/packages/auth/src/credentials/errors.ts index 304151f9..5c642078 100644 --- a/packages/auth/src/credentials/errors.ts +++ b/packages/auth/src/credentials/errors.ts @@ -48,6 +48,7 @@ export class M2mCredentialsError extends Error { /** Discriminant codes for {@link U2mCredentialsError}. */ export type U2mCredentialsErrorCode = | 'PROFILE_REQUIRED' + | 'PROFILE_NOT_FOUND' | 'CLI_NOT_FOUND' | 'LEGACY_CLI_DETECTED' | 'TOKEN_FETCH_FAILED' diff --git a/packages/auth/src/credentials/u2m.ts b/packages/auth/src/credentials/u2m.ts index 008016bd..cfd32fb4 100644 --- a/packages/auth/src/credentials/u2m.ts +++ b/packages/auth/src/credentials/u2m.ts @@ -12,6 +12,7 @@ import {join, sep} from 'node:path'; import {env, platform} from 'node:process'; import {promisify} from 'node:util'; +import {ProfileError, resolve} from '@databricks/sdk-core/profiles'; import {z} from 'zod'; import type {Token, TokenCredentials} from '../auth'; @@ -22,8 +23,15 @@ import {U2mCredentialsError} from './errors'; const execFileAsync = promisify(execFile); /** - * Distinguishes the modern Go-based Databricks CLI (>= 0.100.0) from the - * legacy Python CLI by minimum file size. + * Minimum Databricks CLI version that supports `databricks auth token`. The + * legacy Python CLI predates this and is not compatible. + */ +const MIN_CLI_VERSION = {major: 0, minor: 100, patch: 0}; + +/** + * Fallback heuristic for {@link isModernCli} when `databricks version` cannot + * be parsed: the modern Go-based CLI binary is always larger than this, while + * the legacy Python launcher is smaller. */ const DATABRICKS_CLI_MIN_SIZE = 1024 * 1024; @@ -61,6 +69,7 @@ export function newU2mCredentials( } async function fetchCliToken(options: U2mCredentialsOptions): Promise { + await ensureProfileExists(options.profile); const cliPath = await findDatabricksCli(options.cliPath); return execCliCommand([ cliPath, @@ -71,6 +80,27 @@ async function fetchCliToken(options: U2mCredentialsOptions): Promise { ]); } +/** + * Verifies that the requested profile exists in the Databricks config file. + * + * `databricks auth token --profile X` treats an unknown profile as "no + * profile" and fails with a host-related error, which is misleading. Checking + * up front lets us throw a precise error that names the missing profile. + */ +async function ensureProfileExists(profile: string): Promise { + try { + await resolve({profile}); + } catch (e) { + if (e instanceof ProfileError && e.code === 'PROFILE_NOT_FOUND') { + throw new U2mCredentialsError( + 'PROFILE_NOT_FOUND', + `profile "${profile}" was not found in the Databricks config file` + ); + } + throw e; + } +} + const cliTokenResponseSchema = z.object({ access_token: z.string().min(1), token_type: z.string().optional(), @@ -135,17 +165,30 @@ function cliErrorMessage(e: unknown): string { if (typeof e === 'object' && e !== null) { const err = e as ExecFileError; if (err.stderr !== undefined) { - return err.stderr.toString().trim(); + return stripErrorPrefix(err.stderr.toString().trim()); } return err.message; } return String(e); } +/** + * Removes a leading "Error:" label that the CLI prints on its stderr. Without + * this the wrapped message reads "Error: Error: ..." once the SDK adds its own + * context. + */ +function stripErrorPrefix(message: string): string { + const match = /^Error:\s*/.exec(message); + if (match === null) { + return message; + } + return message.slice(match[0].length); +} + /** * Locates the `databricks` CLI binary, either at `cliPath` (if provided) or - * by searching `PATH`. Validates that the binary is the modern Go CLI and - * not the legacy Python one, via a minimum-size check. + * by searching `PATH`. Validates that the binary is the modern Go CLI and not + * the legacy Python one. */ async function findDatabricksCli(cliPath?: string): Promise { if (cliPath !== undefined) { @@ -202,7 +245,7 @@ async function validateCliPath(path: string): Promise { if (info.isDirectory()) { throw new U2mCredentialsError('CLI_NOT_FOUND', 'databricks CLI not found'); } - if (info.size < DATABRICKS_CLI_MIN_SIZE) { + if (!(await isModernCli(path, info.size))) { throw new U2mCredentialsError( 'LEGACY_CLI_DETECTED', 'legacy databricks CLI detected; upgrade to >= 0.100.0' @@ -210,3 +253,62 @@ async function validateCliPath(path: string): Promise { } return path; } + +/** + * Reports whether the binary at `path` is the modern Go-based Databricks CLI. + * + * The modern CLI reports its version as `Databricks CLI v`; the legacy + * Python CLI does not. When the version cannot be obtained (e.g. the binary is + * not executable in this environment), fall back to the binary-size heuristic, + * since the legacy launcher is far smaller than `size` bytes. + */ +async function isModernCli(path: string, size: number): Promise { + const version = await cliVersion(path); + if (version !== undefined) { + return isAtLeastMinVersion(version); + } + return size >= DATABRICKS_CLI_MIN_SIZE; +} + +interface CliVersion { + major: number; + minor: number; + patch: number; +} + +const CLI_VERSION_PATTERN = /Databricks CLI v(\d+)\.(\d+)\.(\d+)/; + +/** + * Runs ` version` and parses the reported semantic version. Returns + * undefined when the command fails or its output does not match the modern + * CLI's version banner. + */ +async function cliVersion(path: string): Promise { + let stdout: string; + try { + const result = await execFileAsync(path, ['version']); + stdout = result.stdout; + } catch { + return undefined; + } + const match = CLI_VERSION_PATTERN.exec(stdout); + if (match === null) { + return undefined; + } + return { + major: Number(match[1]), + minor: Number(match[2]), + patch: Number(match[3]), + }; +} + +/** Reports whether `version` is at least {@link MIN_CLI_VERSION}. */ +function isAtLeastMinVersion(version: CliVersion): boolean { + if (version.major !== MIN_CLI_VERSION.major) { + return version.major > MIN_CLI_VERSION.major; + } + if (version.minor !== MIN_CLI_VERSION.minor) { + return version.minor > MIN_CLI_VERSION.minor; + } + return version.patch >= MIN_CLI_VERSION.patch; +} diff --git a/packages/auth/tests/credentials/u2m.test.ts b/packages/auth/tests/credentials/u2m.test.ts index 31c0fde0..63ea788f 100644 --- a/packages/auth/tests/credentials/u2m.test.ts +++ b/packages/auth/tests/credentials/u2m.test.ts @@ -1,5 +1,8 @@ import type {Stats} from 'node:fs'; +import {ProfileError} from '@databricks/sdk-core/profiles'; +import type {Profile, ResolveOptions} from '@databricks/sdk-core/profiles'; +import type * as profiles from '@databricks/sdk-core/profiles'; import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest'; import type {U2mCredentialsErrorCode} from '../../src/credentials'; @@ -14,6 +17,9 @@ type ExecFileFn = (cmd: string, args: string[], cb: ExecFileCallback) => void; const execFileMock = vi.hoisted(() => vi.fn()); const statMock = vi.hoisted(() => vi.fn<(path: string) => Promise>()); +const resolveMock = vi.hoisted(() => + vi.fn<(options?: ResolveOptions) => Promise>() +); // Mock node:child_process to intercept CLI invocations. The hoisted mock is // invoked via util.promisify, so it must accept a node-style callback. @@ -27,8 +33,20 @@ vi.mock('node:fs/promises', () => ({ stat: (p: string): Promise => statMock(p), })); +// Mock the profile resolver so the profile pre-check is deterministic and does +// not read the developer's ~/.databrickscfg. +vi.mock('@databricks/sdk-core/profiles', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + resolve: (options?: ResolveOptions): Promise => + resolveMock(options), + }; +}); + const MODERN_CLI_SIZE = 5 * 1024 * 1024; const LEGACY_CLI_SIZE = 100 * 1024; +const MODERN_CLI_VERSION = 'Databricks CLI v0.221.0\n'; const DEFAULT_PROFILE = 'DEFAULT'; const DEFAULT_RESOLVED_CLI_PATH = '/usr/local/bin/databricks'; @@ -45,8 +63,18 @@ function statReturnsModernFile(): void { type CliStub = {kind: 'ok'; stdout: string} | {kind: 'err'; stderr: string}; -function stubCliRun(stub: CliStub): void { - execFileMock.mockImplementationOnce((_cmd, _args, cb) => { +// Routes mocked CLI invocations by sub-command: `databricks version` reports +// versionStub, while `databricks auth token` reports tokenStub. A missing stub +// fails the invocation, surfacing unexpected calls. +function stubCli(opts: {versionStub?: CliStub; tokenStub?: CliStub}): void { + execFileMock.mockImplementation((_cmd, args, cb) => { + const stub = args[0] === 'version' ? opts.versionStub : opts.tokenStub; + if (stub === undefined) { + const err = new Error('command failed') as Error & {stderr: string}; + err.stderr = ''; + cb(err, {stdout: '', stderr: ''}); + return; + } if (stub.kind === 'err') { const err = new Error('command failed') as Error & {stderr: string}; err.stderr = stub.stderr; @@ -57,6 +85,17 @@ function stubCliRun(stub: CliStub): void { }); } +function modernVersionStub(): CliStub { + return {kind: 'ok', stdout: MODERN_CLI_VERSION}; +} + +// Configures the version probe to report a modern CLI and routes the token +// invocation to the given stub. +function stubModernCliWithToken(tokenStub: CliStub): void { + statReturnsModernFile(); + stubCli({versionStub: modernVersionStub(), tokenStub}); +} + function okResponse(partial: { access_token?: string; token_type?: string; @@ -75,11 +114,14 @@ function okResponse(partial: { describe('newU2mCredentials', () => { beforeEach(() => { vi.stubEnv('PATH', '/usr/local/bin:/usr/bin'); + // By default the profile exists, so the pre-check passes. + resolveMock.mockResolvedValue({name: DEFAULT_PROFILE}); }); afterEach(() => { execFileMock.mockReset(); statMock.mockReset(); + resolveMock.mockReset(); vi.unstubAllEnvs(); }); @@ -137,8 +179,7 @@ describe('newU2mCredentials', () => { ]; it.each(successCases)('$name', async ({profile, cliPath, cliStub, want}) => { - statReturnsModernFile(); - stubCliRun(cliStub); + stubModernCliWithToken(cliStub); const creds = newU2mCredentials({ profile, @@ -148,10 +189,25 @@ describe('newU2mCredentials', () => { const headers = await creds.authHeaders(); expect(headers).toEqual([{key: 'Authorization', value: want.authHeader}]); - expect(execFileMock).toHaveBeenCalledOnce(); - const [calledCliPath, args] = execFileMock.mock.calls[0]; - expect(calledCliPath).toBe(want.cliPath); - expect(args).toEqual(['auth', 'token', '--profile', profile]); + expect(resolveMock).toHaveBeenCalledWith({profile}); + + // The version probe and the token request both target the resolved path. + const versionCall = execFileMock.mock.calls.find( + ([, args]) => args[0] === 'version' + ); + if (versionCall === undefined) { + expect.fail('expected a `databricks version` probe'); + } + expect(versionCall[0]).toBe(want.cliPath); + + const tokenCall = execFileMock.mock.calls.find( + ([, args]) => args[0] === 'auth' + ); + if (tokenCall === undefined) { + expect.fail('expected a `databricks auth token` invocation'); + } + expect(tokenCall[0]).toBe(want.cliPath); + expect(tokenCall[1]).toEqual(['auth', 'token', '--profile', profile]); }); const errorCases: { @@ -179,29 +235,53 @@ describe('newU2mCredentials', () => { wantMessage: /databricks CLI not found/, }, { - name: 'only legacy (undersized) binary available', + name: 'undersized binary whose version cannot be read is legacy', setup: (): void => { statReturnsFile(LEGACY_CLI_SIZE); + // The version probe fails, so detection falls back to file size. + stubCli({}); }, profile: DEFAULT_PROFILE, wantCode: 'LEGACY_CLI_DETECTED', wantMessage: /legacy databricks CLI detected/, }, { - name: 'CLI invocation surfaces stderr', + name: 'binary reporting a pre-0.100.0 version is legacy', setup: (): void => { statReturnsModernFile(); - stubCliRun({kind: 'err', stderr: 'not logged in'}); + stubCli({ + versionStub: {kind: 'ok', stdout: 'Databricks CLI v0.99.0\n'}, + }); + }, + profile: DEFAULT_PROFILE, + wantCode: 'LEGACY_CLI_DETECTED', + wantMessage: /legacy databricks CLI detected/, + }, + { + name: 'CLI invocation surfaces stderr', + setup: (): void => { + stubModernCliWithToken({kind: 'err', stderr: 'not logged in'}); }, profile: DEFAULT_PROFILE, wantCode: 'TOKEN_FETCH_FAILED', wantMessage: /not logged in/, }, + { + name: 'CLI stderr already prefixed with Error: is not doubled', + setup: (): void => { + stubModernCliWithToken({ + kind: 'err', + stderr: 'Error: cannot configure default credentials', + }); + }, + profile: DEFAULT_PROFILE, + wantCode: 'TOKEN_FETCH_FAILED', + wantMessage: /cannot get access token: cannot configure default/, + }, { name: 'CLI output is not valid JSON', setup: (): void => { - statReturnsModernFile(); - stubCliRun({kind: 'ok', stdout: 'not json'}); + stubModernCliWithToken({kind: 'ok', stdout: 'not json'}); }, profile: DEFAULT_PROFILE, wantCode: 'INVALID_RESPONSE', @@ -210,8 +290,7 @@ describe('newU2mCredentials', () => { { name: 'CLI response is missing access_token', setup: (): void => { - statReturnsModernFile(); - stubCliRun({ + stubModernCliWithToken({ kind: 'ok', stdout: JSON.stringify({ token_type: 'Bearer', @@ -226,13 +305,26 @@ describe('newU2mCredentials', () => { { name: 'expiry cannot be parsed as a date', setup: (): void => { - statReturnsModernFile(); - stubCliRun(okResponse({expiry: 'totally-not-a-date'})); + stubModernCliWithToken(okResponse({expiry: 'totally-not-a-date'})); }, profile: DEFAULT_PROFILE, wantCode: 'INVALID_RESPONSE', wantMessage: /cannot parse token expiry/, }, + { + name: 'profile not found in the config file', + setup: (): void => { + resolveMock.mockRejectedValue( + new ProfileError( + 'PROFILE_NOT_FOUND', + 'profile not found: "ghost" in /home/u/.databrickscfg' + ) + ); + }, + profile: 'ghost', + wantCode: 'PROFILE_NOT_FOUND', + wantMessage: /profile "ghost" was not found/, + }, ]; it.each(errorCases)(