From 753b8fefc2ae87679ed3e48fb5d242159e27fd9d Mon Sep 17 00:00:00 2001 From: Antony Prasad Thevaraj Date: Thu, 2 Jul 2026 22:43:40 -0400 Subject: [PATCH] fix(security): keep Lakebase PAT off the command line + unify shell escaping MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two hardening fixes to the deploy/UC/secret shell-out seams. 1. secret-auth.ts stored the minted PAT with `databricks secrets put-secret … --string-value ""`, putting the secret in the process table (ps) and any shell trace for the call's lifetime. Feed the PAT on stdin instead (the CLI's documented alternative to --string-value); it no longer appears as a process arg. exec() gains an `input` option that writes to the child's stdin and closes it. 2. Six modules (secret-auth, deploy-credentials, deploy-rollback, uc-resources, databricks-host, deploy-app-endpoint) each defined a local escapeShellArg that only escaped double quotes, leaving $, backticks, and other metacharacters shell-active inside the double-quoted interpolations - a correctness bug and injection surface for any scope/key/catalog/comment value containing them. Replace every call site with the canonical POSIX single-quote escaper shq() from util/exec.ts and delete the duplicated local escapers. Behavior is unchanged for well-formed inputs; the fixes close the ps exposure and the metacharacter gap. Full hermetic suite green; added exec() stdin coverage. Co-authored-by: Isaac --- CHANGELOG.md | 19 +++++++++++++++++++ scripts/lakebase/databricks-host.ts | 7 ++----- scripts/lakebase/deploy-app-endpoint.ts | 17 +++++++---------- scripts/lakebase/deploy-credentials.ts | 16 ++-------------- scripts/lakebase/deploy-rollback.ts | 7 ++----- scripts/lakebase/secret-auth.ts | 20 +++++++++----------- scripts/lakebase/uc-resources.ts | 22 +++++++--------------- scripts/util/exec.ts | 14 +++++++++++++- tests/bdd/exec.test.ts | 14 ++++++++++++++ 9 files changed, 75 insertions(+), 61 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a5e2bc0..eebe3715 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,25 @@ adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- **Lakebase-auth PAT no longer passed on the command line.** + `ensureLakebaseSecretAuth` stored the minted PAT with + `databricks secrets put-secret … --string-value ""`, which exposed the + secret in the process table (`ps`) and any shell trace for the call's + lifetime. The PAT is now fed to the CLI on **stdin** (the documented + alternative to `--string-value`), so it never appears as a process argument. + `exec()` gained an `input` option to support this. +- **Hardened shell-out escaping across the deploy/UC/secret seams.** Six + modules (`secret-auth`, `deploy-credentials`, `deploy-rollback`, + `uc-resources`, `databricks-host`, `deploy-app-endpoint`) each defined a local + `escapeShellArg` that only escaped `"` — leaving `$`, backticks, and other + shell metacharacters active inside the double-quoted interpolations, a + correctness bug (and injection surface) for any scope / key / catalog / + comment value containing them. All call sites now use the canonical POSIX + single-quote escaper `shq()` from `util/exec.ts`; the duplicated local + escapers are deleted. + ## [0.3.0-beta.6] - 2026-06-30 EMU / CI robustness, surfaced by a partner project on an Enterprise Managed Users diff --git a/scripts/lakebase/databricks-host.ts b/scripts/lakebase/databricks-host.ts index 5f3565a8..9f3e7961 100644 --- a/scripts/lakebase/databricks-host.ts +++ b/scripts/lakebase/databricks-host.ts @@ -13,7 +13,7 @@ // token cache is invalidated by a CLI upgrade); we tolerate that by // trimming to the first `{` before parsing. -import { exec } from "../util/exec.js"; +import { exec, shq } from "../util/exec.js"; import { KIT_TIMEOUTS } from "./kit-config.js"; export interface ResolveDatabricksHostArgs { @@ -34,7 +34,7 @@ export async function resolveDatabricksHost( ): Promise { const timeoutMs = args.timeoutMs ?? KIT_TIMEOUTS.cliDefault; const out = await exec( - `databricks auth describe --profile "${escapeShellArg(args.profile)}" -o json`, + `databricks auth describe --profile ${shq(args.profile)} -o json`, { timeout: timeoutMs } ); return parseHostFromAuthDescribe(out); @@ -60,6 +60,3 @@ export function parseHostFromAuthDescribe(out: string): string | undefined { } } -function escapeShellArg(s: string): string { - return s.replace(/"/g, '\\"'); -} diff --git a/scripts/lakebase/deploy-app-endpoint.ts b/scripts/lakebase/deploy-app-endpoint.ts index 6db207af..8a36ed54 100644 --- a/scripts/lakebase/deploy-app-endpoint.ts +++ b/scripts/lakebase/deploy-app-endpoint.ts @@ -21,7 +21,7 @@ // separately (slice 5). import { spawn } from "node:child_process"; -import { exec } from "../util/exec.js"; +import { exec, shq } from "../util/exec.js"; import { KIT_TIMEOUTS } from "./kit-config.js"; import { uploadDirectory, UploadDirectoryResult } from "./deploy-workspace-upload.js"; @@ -147,7 +147,7 @@ export async function getAppEndpoint(args: GetAppEndpointArgs): Promise; @@ -195,7 +195,7 @@ export async function deleteAppEndpoint(args: DeleteAppEndpointArgs): Promise { const appName = args.appName ?? deriveCiAppName(args.instance, args.branch); const timeoutMs = args.timeoutMs ?? KIT_TIMEOUTS.cliDefault; - const profileFlag = args.profile ? ` --profile "${escapeShellArg(args.profile)}"` : ""; + const profileFlag = args.profile ? ` --profile ${shq(args.profile)}` : ""; try { const stdout = await exec( - `databricks apps get "${escapeShellArg(appName)}"${profileFlag} -o json`, + `databricks apps get ${shq(appName)}${profileFlag} -o json`, { timeout: timeoutMs } ); const info = JSON.parse(stdout) as Record; @@ -425,6 +425,3 @@ function runDeploy(args: { }); } -function escapeShellArg(s: string): string { - return s.replace(/"/g, '\\"'); -} diff --git a/scripts/lakebase/deploy-credentials.ts b/scripts/lakebase/deploy-credentials.ts index b01939b6..925efa3c 100644 --- a/scripts/lakebase/deploy-credentials.ts +++ b/scripts/lakebase/deploy-credentials.ts @@ -13,7 +13,7 @@ // DeployTarget + profile + appName, it resolves the SP, grants the // permission(s), returns a structured result. -import { exec } from "../util/exec.js"; +import { exec, shq } from "../util/exec.js"; import { KIT_TIMEOUTS } from "./kit-config.js"; import { DeployTarget } from "./deploy-targets.js"; import { getAppEndpoint } from "./deploy-app-endpoint.js"; @@ -111,7 +111,7 @@ export async function grantLakebasePermission( ], }); await exec( - `databricks api patch "/api/2.0/permissions/database-projects/${escapeShellArg(args.projectName)}" --profile "${escapeShellArg(args.profile)}" --json '${escapeSingleQuoted(payload)}'`, + `databricks api patch /api/2.0/permissions/database-projects/${shq(args.projectName)} --profile ${shq(args.profile)} --json ${shq(payload)}`, { timeout: timeoutMs } ); return { granted: true }; @@ -178,15 +178,3 @@ export async function propagateCredentials( }; } -// ─── helpers ──────────────────────────────────────────────────── - -function escapeShellArg(s: string): string { - return s.replace(/"/g, '\\"'); -} - -function escapeSingleQuoted(s: string): string { - // Within single-quoted shell strings, the only escape needed is to - // close + reopen for embedded apostrophes. The JSON payload itself - // contains no apostrophes, but harden anyway for future-proofing. - return s.replace(/'/g, `'\\''`); -} diff --git a/scripts/lakebase/deploy-rollback.ts b/scripts/lakebase/deploy-rollback.ts index 3004df40..8992d036 100644 --- a/scripts/lakebase/deploy-rollback.ts +++ b/scripts/lakebase/deploy-rollback.ts @@ -22,7 +22,7 @@ // structured result shape as `ensureAppEndpoint`. import { spawn } from "node:child_process"; -import { exec } from "../util/exec.js"; +import { exec, shq } from "../util/exec.js"; import { KIT_TIMEOUTS } from "./kit-config.js"; export interface RollbackDeployArgs { @@ -71,7 +71,7 @@ export async function listAppDeployments(args: { }): Promise { const timeoutMs = args.timeoutMs ?? KIT_TIMEOUTS.cliDefault; const stdout = await exec( - `databricks apps list-deployments "${escapeShellArg(args.appName)}" --profile "${escapeShellArg(args.profile)}" -o json`, + `databricks apps list-deployments ${shq(args.appName)} --profile ${shq(args.profile)} -o json`, { timeout: timeoutMs } ); const parsed = JSON.parse(stdout) as unknown; @@ -218,6 +218,3 @@ function runRollbackDeploy(args: { }); } -function escapeShellArg(s: string): string { - return s.replace(/"/g, '\\"'); -} diff --git a/scripts/lakebase/secret-auth.ts b/scripts/lakebase/secret-auth.ts index abd8a3fb..04fe7aa0 100644 --- a/scripts/lakebase/secret-auth.ts +++ b/scripts/lakebase/secret-auth.ts @@ -10,7 +10,7 @@ // Lifted from the lakebase-scm-extension's bespoke // DeployService.ensureLakebaseSecretAuth. -import { exec } from "../util/exec.js"; +import { exec, shq } from "../util/exec.js"; import { KIT_TIMEOUTS } from "./kit-config.js"; const NINETY_DAYS_SECONDS = 90 * 24 * 60 * 60; @@ -79,7 +79,7 @@ export async function ensureLakebaseSecretAuth( let scopeCreated = false; try { await exec( - `databricks secrets create-scope "${escapeShellArg(scopeName)}" --profile "${escapeShellArg(profile)}"`, + `databricks secrets create-scope ${shq(scopeName)} --profile ${shq(profile)}`, { timeout: timeoutMs } ); scopeCreated = true; @@ -90,7 +90,7 @@ export async function ensureLakebaseSecretAuth( // 2. Mint PAT const tokenJson = await exec( - `databricks tokens create --comment "${escapeShellArg(tokenComment)}" --lifetime-seconds ${tokenLifetimeSeconds} -o json --profile "${escapeShellArg(profile)}"`, + `databricks tokens create --comment ${shq(tokenComment)} --lifetime-seconds ${tokenLifetimeSeconds} -o json --profile ${shq(profile)}`, { timeout: timeoutMs } ); const tokenStart = tokenJson.indexOf("{"); @@ -103,10 +103,12 @@ export async function ensureLakebaseSecretAuth( throw new Error("databricks tokens create returned no token_value"); } - // 3. Store the PAT + // 3. Store the PAT. Feed it on stdin (the CLI reads the value from stdin + // when --string-value is omitted) so the secret never appears on the + // command line / in the process table. await exec( - `databricks secrets put-secret "${escapeShellArg(scopeName)}" "${escapeShellArg(keyName)}" --string-value "${escapeShellArg(pat)}" --profile "${escapeShellArg(profile)}"`, - { timeout: timeoutMs } + `databricks secrets put-secret ${shq(scopeName)} ${shq(keyName)} --profile ${shq(profile)}`, + { timeout: timeoutMs, input: pat } ); // 4. ACL (best-effort) @@ -114,7 +116,7 @@ export async function ensureLakebaseSecretAuth( if (servicePrincipalClientId) { try { await exec( - `databricks secrets put-acl "${escapeShellArg(scopeName)}" "${escapeShellArg(servicePrincipalClientId)}" READ --profile "${escapeShellArg(profile)}"`, + `databricks secrets put-acl ${shq(scopeName)} ${shq(servicePrincipalClientId)} READ --profile ${shq(profile)}`, { timeout: timeoutMs } ); aclGranted = true; @@ -135,7 +137,3 @@ export async function ensureLakebaseSecretAuth( function isAlreadyExistsError(msg: string): boolean { return /already exists|SCOPE_ALREADY_EXISTS|RESOURCE_ALREADY_EXISTS/i.test(msg); } - -function escapeShellArg(s: string): string { - return s.replace(/"/g, '\\"'); -} diff --git a/scripts/lakebase/uc-resources.ts b/scripts/lakebase/uc-resources.ts index 073ebf58..e8201af2 100644 --- a/scripts/lakebase/uc-resources.ts +++ b/scripts/lakebase/uc-resources.ts @@ -10,7 +10,7 @@ // are idempotent (existence check + conditional create), and the // permission grant matches the platform's standard CHANGES shape. -import { exec } from "../util/exec.js"; +import { exec, shq } from "../util/exec.js"; import { KIT_TIMEOUTS } from "./kit-config.js"; const DEFAULT_CREATE_COMMENT = "Created by lakebase-app-dev-kit"; @@ -30,7 +30,7 @@ export async function catalogExists(args: CatalogExistsArgs): Promise { const timeoutMs = args.timeoutMs ?? KIT_TIMEOUTS.cliDefault; try { await exec( - `databricks api get /api/2.1/unity-catalog/catalogs/${escapeShellArg(args.catalog)} --profile "${escapeShellArg(args.profile)}"`, + `databricks api get /api/2.1/unity-catalog/catalogs/${shq(args.catalog)} --profile ${shq(args.profile)}`, { timeout: timeoutMs } ); return true; @@ -68,7 +68,7 @@ export async function tryCreateCatalog(args: TryCreateCatalogArgs): Promise { try { - await exec(`databricks api get ${apiPath} --profile "${escapeShellArg(profile)}"`, { timeout: timeoutMs }); + await exec(`databricks api get ${apiPath} --profile ${shq(profile)}`, { timeout: timeoutMs }); return true; } catch (err) { if (isUcMissingError((err as Error).message)) return false; @@ -229,11 +229,3 @@ async function ucResourceExists(apiPath: string, profile: string, timeoutMs: num function isUcMissingError(msg: string): boolean { return /RESOURCE_DOES_NOT_EXIST|does not exist|status:? 404\b|NOT_FOUND/i.test(msg); } - -function escapeShellArg(s: string): string { - return s.replace(/"/g, '\\"'); -} - -function escapeSingleQuoted(s: string): string { - return s.replace(/'/g, `'\\''`); -} diff --git a/scripts/util/exec.ts b/scripts/util/exec.ts index 94aa4a38..61a6df50 100644 --- a/scripts/util/exec.ts +++ b/scripts/util/exec.ts @@ -9,6 +9,13 @@ export interface ExecOptions { env?: Record; /** Milliseconds before SIGTERM. Default: 60_000. */ timeout?: number; + /** + * Written to the child's stdin, then stdin is closed. Use this to feed a + * secret to a CLI that reads it from stdin (e.g. `databricks secrets + * put-secret` with no `--string-value`) so the value never appears in the + * command line / process table. Omit for commands that take no input. + */ + input?: string; } /** @@ -49,7 +56,7 @@ export function exec(command: string, opts: ExecOptions = {}): Promise { if (opts.env) { options.env = { ...process.env, ...opts.env }; } - cp.exec(command, options, (err, stdout, stderr) => { + const child = cp.exec(command, options, (err, stdout, stderr) => { if (err) { const msg = String(stderr || err.message); reject(new Error(`${command}: ${msg}`)); @@ -57,5 +64,10 @@ export function exec(command: string, opts: ExecOptions = {}): Promise { } resolve(String(stdout).trim()); }); + if (opts.input !== undefined) { + // Feed the value on stdin, then close it, so a secret never rides on + // the command line (where `ps` / shell traces would expose it). + child.stdin?.end(opts.input); + } }); } diff --git a/tests/bdd/exec.test.ts b/tests/bdd/exec.test.ts index 85b61fa6..8d65e8f4 100644 --- a/tests/bdd/exec.test.ts +++ b/tests/bdd/exec.test.ts @@ -22,4 +22,18 @@ describe("exec", () => { }); expect(out).toBe("xyzzy"); }); + + it("feeds `input` to the child on stdin", async () => { + // `cat` echoes back exactly what it reads from stdin, proving the value + // reached the child without appearing anywhere on the command line. + const out = await exec("cat", { input: "secret-on-stdin" }); + expect(out).toBe("secret-on-stdin"); + }); + + it("closes stdin so a stdin-reading command terminates", async () => { + // Without end()ing stdin, `cat` would block until timeout. A prompt + // return proves the stream was closed. + const out = await exec("cat", { input: "" }); + expect(out).toBe(""); + }); });