feat: add tokenCacheEnabled to control kernel U2M on-disk token cache by eric-wang-1990 · Pull Request #513 · databricks/databricks-sql-nodejs · GitHub

What & why

Adds a tokenCacheEnabled?: boolean connection option (on the databricks-oauth auth type) to control the kernel backend's U2M on-disk OAuth token cache.

On the kernel path with OAuth U2M, the Rust kernel can persist the refresh token to an AES-256 encrypted on-disk cache at dirs::config_dir()/databricks-sql-kernel/oauth/ (~/Library/Application Support/... on macOS, ~/.config/... on Linux), so a later process skips the interactive browser login. This wires a client option through to the napi binding's tokenCacheEnabled field.

Behavior

U2M-only. No effect on PAT/M2M.
Disabled by default. When unset, the connector explicitly passes tokenCacheEnabled: false to the kernel — matching Thrift's in-memory (no on-disk persistence) posture, so migrating onto the kernel path doesn't silently start writing tokens to disk. Enabling is opt-in via tokenCacheEnabled: true. (The kernel's own default is enabled; the connector always sends false when the option is unset, so disable-by-default is the connector's deliberate choice.)
Enable flag only for now (no passphrase surface); when enabled the kernel encrypts at rest with a machine-local derived key.
The existing Thrift-only persistence (custom OAuthPersistence store) is untouched and still rejected on the kernel path — this new flag is a separate, simpler control.

Dependency

Consumes the napi field tokenCacheEnabled from databricks-sql-kernel #283, now merged to kernel main (merge commit 628abd6). This PR bumps KERNEL_REV 5e5dea9 → 628abd6 (additive over #282, so the Azure SP surface is preserved) and regenerates the committed napi binding, which now exposes the field. The kernel-e2e CI path builds the binding from KERNEL_REV, so it picks up the field.

Published npm pins stay at 0.2.0. No @databricks/databricks-sql-kernel 0.2.1 release has shipped yet, so bumping the 8 platform optionalDependencies to a nonexistent version would break npm install. Until that release lands (a follow-up commit here bumps the pins), npm consumers still get the 0.2.0 binding, which silently drops the unknown field — so disable-by-default doesn't take effect for them yet. Do not merge until the 0.2.1 release ships and the pins are bumped.

Testing

Unit

mocha U2M mapping tests: unset ⇒ false, true ⇒ true, and the persistence rejection still throws. 322 kernel unit tests green against the rebuilt binding.

Runtime E2E (new)

Added an interactive U2M E2E suite (tests/e2e/kernel/auth-u2m-e2e.test.ts), gated behind DATABRICKS_KERNEL_U2M_INTERACTIVE + workspace host/path so CI still skips it (the browser flow needs a human). It backs up and restores the real on-disk cache dir around the run so a developer's own cached tokens are never destroyed.

Ran it against a live Azure pecotesting workspace, completing the browser login for each case:

Case	Flow	On-disk cache result
tokenCacheEnabled unset	U2M browser login → SELECT 1 = 1	No file written (cache dir empty) ✓
tokenCacheEnabled: true	U2M browser login → SELECT 1 = 1	An AES-encrypted (non-plaintext-JSON) {sha256}.json is written ✓

Result: 2 passing (17s). Confirms disable-by-default (Thrift parity) and opt-in enable end to end through DBSQLClient → KernelBackend → napi binding → live workspace. (Azure U2M routes through the kernel's cloud-blind in-house OAuthU2m flow, as expected — it uses the workspace's OIDC-discovered authorize endpoint verbatim.)

Related: databricks-sql-kernel #283.

This pull request and its description were written by Isaac.

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

✅ No issues identified by the review bot.

Pull request overview

Adds a new tokenCacheEnabled?: boolean connection option for authType: 'databricks-oauth' to control whether the kernel backend’s U2M OAuth flow may use an on-disk refresh-token cache, while keeping “no persistence” as the default posture when migrating to the kernel path.

Changes:

Plumbs tokenCacheEnabled through buildKernelConnectionOptions for OAuth U2M, defaulting to false when unset.
Extends the public connection option types/docs to describe the new flag and its kernel/U2M-only behavior.
Adds unit tests covering default false and opt-in true mapping for U2M.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
tests/unit/kernel/auth-u2m.test.ts	Adds assertions/tests to ensure tokenCacheEnabled is forwarded (default false, opt-in true) for kernel OAuth U2M.
lib/kernel/KernelAuth.ts	Adds tokenCacheEnabled to the kernel native option type and wires defaulting logic into the U2M mapping.
lib/contracts/IDBSQLClient.ts	Exposes tokenCacheEnabled?: boolean on databricks-oauth connection options with API docs.
CONNECTION_PARAMETERS.md	Documents the new connection parameter and calls out kernel-only behavior and default.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Fixed the misleading error wording and confirmed the mocked U2M auth suite still passes.

Pushed 83b91f0 (bundled with 1 other thread(s)).

Resolved with a documentation edit — the reviewer's point was valid and fully actionable in this PR.

Pushed 83b91f0 (bundled with 1 other thread(s)).

Addresses: - #3858665815 at lib/kernel/KernelAuth.ts:840 - #3858665845 at CONNECTION_PARAMETERS.md:68 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

✅ No issues identified by the review bot.

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Adds tokenCacheEnabled on the databricks-oauth option, mapped to the napi ConnectionOptions field on the U2M path. U2M-only; disabled by default (matches Thrift no-persistence posture); enable-flag only; the Thrift-only persistence custom-store hook is untouched and still rejected on the kernel path. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>

Addresses: - #3858665815 at lib/kernel/KernelAuth.ts:840 - #3858665845 at CONNECTION_PARAMETERS.md:68 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Signed-off-by: eric-wang-1990 <e.wang@databricks.com>

Bumps KERNEL_REV 5e5dea9 -> 628abd6 (databricks-sql-kernel #283, "U2M token-cache control"; additive over #282 so the Azure SP surface is preserved) and regenerates the committed napi binding, which now exposes the `tokenCacheEnabled` / `tokenCachePassphrase` fields consumed by buildKernelConnectionOptions. The only index.js churn is napi-rs's embedded version guard (0.2.0 -> 0.2.1); the export set is unchanged. Replaces the placeholder skipped U2M e2e with a real interactive suite (gated behind DATABRICKS_KERNEL_U2M_INTERACTIVE so CI still skips it) that proves, end to end against a live workspace: - tokenCacheEnabled unset -> no on-disk token written (Thrift parity) - tokenCacheEnabled: true -> an AES-encrypted (non-JSON) token is persisted under dirs::config_dir()/ databricks-sql-kernel/oauth/ Verified: 2/2 e2e passing against an Azure pecotesting workspace; 322 kernel unit tests green against the rebuilt binding. Note: published optionalDependency pins remain 0.2.0 (no 0.2.1 kernel release yet) per the PR hold; local dev and the KERNEL_REV-built CI path use the freshly built .node. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>

Normalizes trailing whitespace in two table rows so `prettier . --check` (the repo's lint gate) passes. No content change. Signed-off-by: eric-wang-1990 <e.wang@databricks.com>

Verdict: 1 Medium

Clean, well-scoped change wiring tokenCacheEnabled through to the kernel U2M path with sensible disable-by-default parity and good unit + interactive-e2e coverage. One medium concern: the regenerated native/kernel/index.js now enforces binding version 0.2.1 while package.json still pins 0.2.0 — an intentional-but-inconsistent split the author already flags as "do not merge" until the 0.2.1 release and pin bump land together.

🟡 Medium — The regenerated napi router now expects binding version 0.2.1 in every platform branch (and the WASI branch at ~L651), but package.json optionalDependencies still pin all 8 kernel packages to 0.2.0. The two were bumped out of lockstep.

The check is gated on NAPI_RS_ENFORCE_VERSION_CHECK, so default npm consumers are unaffected — but any environment that sets that flag will now hit Native binding package version mismatch, expected 0.2.1 but got 0.2.0 and fail to load, because npm installs the pinned 0.2.0. This contradicts the PR's own stated intent to keep consumers on 0.2.0 until 0.2.1 ships.

The PR description acknowledges this ("Do not merge until the 0.2.1 release ships and the pins are bumped"), so this is a state-of-the-PR gate rather than a logic bug — flagging so the version-check bump and the dependency pins land together in the same commit that flips to 0.2.1, not split across the merge boundary.

⛔ NEEDS HUMAN DECISION — the bots can't resolve this thread; a maintainer's input is required.

Valid point, but it cannot be actioned in this job. The version-check expects 0.2.1 in native/kernel/index.js (a generated file under the denied native/ path — not writable), while package.json optionalDependencies pin the 8 kernel packages to 0.2.0. Reconciling them requires either reverting the native router to 0.2.0 or bumping the package.json pins to 0.2.1 — but native/ and package.json are both off-limits here, and the reviewer/PR description explicitly gate this on the 0.2.1 kernel release shipping ("Do not merge until 0.2.1 ships and the pins are bumped"). This is a release-coordination / merge-boundary decision for a human: the napi-router version bump and the dependency pins must land together in the commit that flips to 0.2.1, once those packages are published.

The regenerated native/kernel/index.js baked the expected binding version to 0.2.1, but package.json optionalDependencies still pin all 8 kernel packages to 0.2.0 (no 0.2.1 npm release has shipped). The guard is gated on NAPI_RS_ENFORCE_VERSION_CHECK, so default consumers are unaffected, but any environment that sets the flag would hit "expected 0.2.1 but got 0.2.0" and fail to load. Keep the version literal at 0.2.0 so the loader stays in lockstep with the pinned binding. Bump both the guard literals and the optionalDependencies pins to 0.2.1 together in the follow-up commit once 0.2.1 publishes. Co-authored-by: Isaac

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Verdict: 1 High

Looks solid — the tokenCacheEnabled wiring in KernelAuth.ts is correctly scoped to the U2M branch, and unit/contract changes are consistent. One high concern: the new e2e test's after cleanup can delete a developer's real kernel OAuth token cache on the skip path, because this.skip() runs before the backup is taken but the after hook still deletes the cache dir unconditionally.

The connector wires only tokenCacheEnabled through to the kernel U2M path; the passphrase surface is intentionally out of scope for this PR (enable flag only). Remove the tokenCachePassphrase type entry so the committed napi surface matches what the connector actually forwards. It can be re-added alongside real wiring when the passphrase option is surfaced. Co-authored-by: Isaac

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

Addresses: - #3860354328 at tests/e2e/kernel/auth-u2m-e2e.test.ts:111 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>

Thanks for your contribution! To satisfy the DCO policy in our contributing guide every commit message must include a sign-off message. One or more of your commits is missing this message. You can reword previous commit messages with an interactive rebase (git rebase -i main).

✅ No issues identified by the review bot.

eric-wang-1990 deployed to azure-prod August 25, 2026 23:22 — with GitHub Actions Active

eric-wang-1990 marked this pull request as ready for review August 26, 2026 00:25

Copilot AI lite review requested due to automatic review settings August 26, 2026 00:25

eric-wang-1990 added the engineer-bot Maintainer-only: triggers the autonomous engineer-bot (bug-fix author on issues; follow-up on PRs) label Aug 26, 2026

eric-wang-1990 deployed to azure-prod August 26, 2026 00:25 — with GitHub Actions Active

Copilot started reviewing on behalf of eric-wang-1990 August 26, 2026 00:25 View session

peco-review-bot Bot approved these changes Aug 26, 2026

View reviewed changes

Copilot AI reviewed Aug 26, 2026

View reviewed changes

eric-wang-1990 added engineer-bot Maintainer-only: triggers the autonomous engineer-bot (bug-fix author on issues; follow-up on PRs) and removed engineer-bot Maintainer-only: triggers the autonomous engineer-bot (bug-fix author on issues; follow-up on PRs) labels Aug 26, 2026

peco-engineer-bot Bot deployed to azure-prod August 26, 2026 01:28 Active

peco-review-bot Bot approved these changes Aug 26, 2026

View reviewed changes

vuanhphung approved these changes Aug 26, 2026

View reviewed changes

eric-wang-1990 deployed to azure-prod August 26, 2026 05:12 — with GitHub Actions Active

eric-wang-1990 and others added 4 commits August 25, 2026 22:30

ai: apply changes for #513 (2 review threads)

1f7dcde

Addresses: - #3858665815 at lib/kernel/KernelAuth.ts:840 - #3858665845 at CONNECTION_PARAMETERS.md:68 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Signed-off-by: eric-wang-1990 <e.wang@databricks.com>

style: prettier-format CONNECTION_PARAMETERS.md

5ecd802

Normalizes trailing whitespace in two table rows so `prettier . --check` (the repo's lint gate) passes. No content change. Signed-off-by: eric-wang-1990 <e.wang@databricks.com>

eric-wang-1990 force-pushed the eric-wang/kernel-token-cache-control branch from e1f20b4 to 5ecd802 Compare August 26, 2026 05:32

eric-wang-1990 deployed to azure-prod August 26, 2026 05:32 — with GitHub Actions Active

peco-review-bot Bot reviewed Aug 26, 2026

View reviewed changes

eric-wang-1990 deployed to azure-prod August 26, 2026 06:56 — with GitHub Actions Active

peco-review-bot Bot reviewed Aug 26, 2026

View reviewed changes

Comment thread tests/e2e/kernel/auth-u2m-e2e.test.ts

eric-wang-1990 deployed to azure-prod August 26, 2026 07:02 — with GitHub Actions Active

ai: apply changes for #513 (1 review thread)

b3f1f18

Addresses: - #3860354328 at tests/e2e/kernel/auth-u2m-e2e.test.ts:111 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>

peco-engineer-bot Bot deployed to azure-prod August 26, 2026 07:05 Active

peco-review-bot Bot approved these changes Aug 26, 2026

View reviewed changes

@@ @@ -66,6 +66,7 @@ column. @@
               | `oauthClientSecret` (M2M)                      | `string`                                                     |   ✅   |   ✅   | —                                                  | M2M client-credentials secret; its **presence** is the U2M-vs-M2M flow selector on both backends (`undefined` ⇒ U2M). Thrift → `DatabricksOAuth.clientSecret`. Kernel → native `oauthClientSecret` (workspace-OIDC M2M) or remapped to `azureClientSecret` (Entra-direct `AzureSpM2m`). A blank/reserved secret is forwarded verbatim and still selects M2M (Thrift parity) — except the Azure SP arm, which rejects it.                                                                                                                                                                                                                                                                                                                             |
               | `azureTenantId` / `useDatabricksOAuthInAzure`  | `string` / `boolean`                                         |   ✅   |   ⚠️   | —                                                  | **Honored on both.** By design the kernel routes **all U2M** (no secret, any cloud) to its cloud-blind in-house OAuth U2M flow — there is no Azure-specific U2M mode, so `useDatabricksOAuthInAzure` is inert on U2M and every Azure workspace (including `.databricks.azure.us` US-gov) is always supported, on any kernel build. `useDatabricksOAuthInAzure` selects only the **M2M** mechanism on an Azure host: absent/`false` → Entra-direct service-principal M2M (native `AzureSpM2m` mode, creds ride `oauthClientId`/`oauthClientSecret`, `azureTenantId` optional — kernel auto-discovers from the workspace `/aad/auth` redirect when omitted); `true` → workspace-OIDC M2M. (`lib/kernel/KernelAuth.ts` `buildKernelConnectionOptions`.) |
               | `persistence` (custom OAuth token store)       | `OAuthPersistence`                                           |   ✅   |   ❌   | —                                                  | **Thrift-only.** Kernel throws; it auto-persists U2M tokens to `~/.config/databricks-sql-kernel/oauth/` and does not cache M2M.                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                      |

                       const bindingPackageVersion = require('@databricks/databricks-sql-kernel-android-arm64/package.json').version
-                      if (bindingPackageVersion !== '0.2.0' && process.env.NAPI_RS_ENFORCE_VERSION_CHECK && process.env.NAPI_RS_ENFORCE_VERSION_CHECK !== '0') {
-                        throw new Error(`Native binding package version mismatch, expected 0.2.0 but got ${bindingPackageVersion}. You can reinstall dependencies to fix this issue.`)
+                      if (bindingPackageVersion !== '0.2.1' && process.env.NAPI_RS_ENFORCE_VERSION_CHECK && process.env.NAPI_RS_ENFORCE_VERSION_CHECK !== '0') {

+                      // Token cache is disabled by default for security (silent-no-persist parity);
+                      // explicitly set to false unless the caller opts in.
+                      tokenCacheEnabled: oauth.tokenCacheEnabled ?? false,

feat: add tokenCacheEnabled to control kernel U2M on-disk token cache by eric-wang-1990 · Pull Request #513 · databricks/databricks-sql-nodejs · GitHub

Uh oh!

feat: add tokenCacheEnabled to control kernel U2M on-disk token cache - #513

Conversation

eric-wang-1990 commented Aug 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What & why

Behavior

Dependency

Testing

Unit

Runtime E2E (new)

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

peco-engineer-bot Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

peco-engineer-bot Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

peco-review-bot Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

peco-engineer-bot Bot Aug 26, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

github-actions Bot commented Aug 26, 2026

Uh oh!

peco-review-bot Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eric-wang-1990 commented Aug 25, 2026 •
edited
Loading