From 3a92c090072c9516c40e75d331b7464cb92666c6 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Thu, 11 Sep 2025 16:35:36 +1000 Subject: [PATCH 01/12] Add the '--impersonate-service-account' argument to the 'run' and 'test' commands and the required changes to allow for the impersonation of service accounts without the need to change ADC --- cli/api/BUILD | 1 + cli/api/dbadapters/bigquery.ts | 63 ++++++++++++++++++++++------------ cli/commands/run_command.ts | 3 ++ cli/commands/run_options.ts | 4 +++ cli/commands/test_command.ts | 7 ++++ cli/common_options.ts | 12 +++++++ package.json | 1 + packages/@dataform/cli/BUILD | 1 + protos/profiles.proto | 2 ++ 9 files changed, 72 insertions(+), 22 deletions(-) diff --git a/cli/api/BUILD b/cli/api/BUILD index 9b817c962..aed10e738 100644 --- a/cli/api/BUILD +++ b/cli/api/BUILD @@ -40,6 +40,7 @@ ts_library( "@npm//deepmerge", "@npm//fs-extra", "@npm//glob", + "@npm//google-auth-library", "@npm//google-sql-syntax-ts", "@npm//js-beautify", "@npm//js-yaml", diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index 303e08024..9a5149481 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -1,7 +1,7 @@ import { BigQuery, GetTablesResponse, TableField, TableMetadata } from "@google-cloud/bigquery"; +import { GoogleAuth, Impersonated } from "google-auth-library"; import Long from "long"; import { PromisePoolExecutor } from "promise-pool-executor"; - import { collectEvaluationQueries, QueryOrAction } from "df/cli/api/dbadapters/execution_sql"; import { IBigQueryError, @@ -37,24 +37,40 @@ export interface IBigQueryExecutionOptions { reservation?: string; } -export type BigQueryClientProvider = (projectId?: string) => BigQuery; +export type BigQueryClientProvider = (projectId?: string) => BigQuery | Promise; export function createBigQueryClientProvider( credentials: dataform.IBigQuery ): BigQueryClientProvider { const clients = new Map(); - return (projectId?: string) => { + return async (projectId?: string) => { projectId = projectId || credentials.projectId; if (!clients.has(projectId)) { - clients.set( + const clientConfig: any = { projectId, - new BigQuery({ + scopes: EXTRA_GOOGLE_SCOPES, + location: credentials.location + }; + + if (credentials.impersonateServiceAccount) { + const sourceAuth = new GoogleAuth({ + scopes: ["https://www.googleapis.com/auth/cloud-platform"], projectId, - scopes: EXTRA_GOOGLE_SCOPES, - location: credentials.location, credentials: credentials.credentials && JSON.parse(credentials.credentials) - }) - ); + }); + + const authClient = await sourceAuth.getClient(); + + clientConfig.authClient = new Impersonated({ + sourceClient: authClient, + targetPrincipal: credentials.impersonateServiceAccount, + targetScopes: ["https://www.googleapis.com/auth/cloud-platform"] + }); + } else { + clientConfig.credentials = credentials.credentials && JSON.parse(credentials.credentials); + } + + clients.set(projectId, new BigQuery(clientConfig)); } return clients.get(projectId); }; @@ -143,7 +159,7 @@ export class BigQueryDbAdapter implements IDbAdapter { return this.pool .addSingleTask({ generator: async () => { - const [rows, , apiResponse] = await this.getClient().query({ + const [rows, , apiResponse] = await (await this.getClient()).query({ ...this.prepareQueryOptions(statement, options.rowLimit, options.bigquery, options.params), skipParsing: true } as any); @@ -162,8 +178,8 @@ export class BigQueryDbAdapter implements IDbAdapter { try { await this.pool .addSingleTask({ - generator: () => - this.getClient().query({ + generator: async () => + (await this.getClient()).query({ useLegacySql: false, query, dryRun: true @@ -193,7 +209,7 @@ export class BigQueryDbAdapter implements IDbAdapter { await Promise.all( datasetIds.map(async datasetId => { - const [tables] = await this.getClient(database) + const [tables] = await (await this.getClient(database)) .dataset(datasetId) .getTables({ autoPaginate: true, maxResults: 1000 }); await Promise.all( @@ -286,14 +302,17 @@ export class BigQueryDbAdapter implements IDbAdapter { } public async deleteTable(target: dataform.ITarget): Promise { - await this.getClient(target.database) + await (await this.getClient(target.database)) .dataset(target.schema) .table(target.name) .delete({ ignoreNotFound: true }); } public async schemas(database: string): Promise { - const data = await this.getClient(database).getDatasets({ autoPaginate: true, maxResults: 1000 }); + const data = await (await this.getClient(database)).getDatasets({ + autoPaginate: true, + maxResults: 1000 + }); return data[0].map(dataset => dataset.id); } @@ -313,7 +332,7 @@ export class BigQueryDbAdapter implements IDbAdapter { metadata.schema.fields ); - await this.getClient(target.database) + await (await this.getClient(target.database)) .dataset(target.schema) .table(target.name) .setMetadata({ @@ -325,7 +344,7 @@ export class BigQueryDbAdapter implements IDbAdapter { private async getMetadata(target: dataform.ITarget): Promise { try { - const table = await this.getClient(target.database) + const table = await (await this.getClient(target.database)) .dataset(target.schema) .table(target.name) .getMetadata(); @@ -340,8 +359,8 @@ export class BigQueryDbAdapter implements IDbAdapter { } } - private getClient(projectId?: string) { - return this.clientProvider(projectId); + private async getClient(projectId?: string) { + return await this.clientProvider(projectId); } private async runQuery( @@ -351,12 +370,12 @@ export class BigQueryDbAdapter implements IDbAdapter { byteLimit?: number, location?: string ): Promise { - const results = await new Promise((resolve, reject) => { + const results = await new Promise(async (resolve, reject) => { const allRows = new LimitedResultSet({ rowLimit, byteLimit }); - const stream = this.getClient().createQueryStream({ + const stream = (await this.getClient()).createQueryStream({ query, params, location @@ -412,7 +431,7 @@ export class BigQueryDbAdapter implements IDbAdapter { return retry( async () => { try { - const job = await this.getClient().createQueryJob( + const job = await (await this.getClient()).createQueryJob( this.prepareQueryOptions( query, rowLimit, diff --git a/cli/commands/run_command.ts b/cli/commands/run_command.ts index 643be8dc0..63a786c11 100644 --- a/cli/commands/run_command.ts +++ b/cli/commands/run_command.ts @@ -93,6 +93,9 @@ export const runCommand: ICommand = { const readCredentials = credentials.read( actuallyResolve(argv.projectDir, argv.credentials) ); + if (argv.impersonateServiceAccount) { + (readCredentials as any).impersonateServiceAccount = argv.impersonateServiceAccount; + } const dbadapter = new BigQueryDbAdapter(readCredentials); const executionGraph = await build( diff --git a/cli/commands/run_options.ts b/cli/commands/run_options.ts index d042623fb..6ac6cb4be 100644 --- a/cli/commands/run_options.ts +++ b/cli/commands/run_options.ts @@ -6,6 +6,8 @@ import { credentialsOption, IActionsArgs, ICredentialsArgs, + IImpersonateServiceAccountArgs, + impersonateServiceAccountOption, IJsonOutputArgs, IProjectDirArgs, ITimeoutArgs, @@ -21,6 +23,7 @@ export interface IRunArgs extends IProjectDirArgs, IProjectConfigArgs, ICredentialsArgs, + IImpersonateServiceAccountArgs, IActionsArgs, IJsonOutputArgs, ITimeoutArgs { @@ -173,6 +176,7 @@ export const runOptions: Array> = [ actionRetryLimitOption, actionsOption, credentialsOption, + impersonateServiceAccountOption, emitLineageOption, fullRefreshOption, includeDepsOption, diff --git a/cli/commands/test_command.ts b/cli/commands/test_command.ts index be9fcb6ce..fa6bc8b45 100644 --- a/cli/commands/test_command.ts +++ b/cli/commands/test_command.ts @@ -5,6 +5,8 @@ import { assertProjectDirExists, credentialsOption, ICredentialsArgs, + IImpersonateServiceAccountArgs, + impersonateServiceAccountOption, IJsonOutputArgs, IProjectDirArgs, ITimeoutArgs, @@ -26,6 +28,7 @@ import { ICommand } from "df/cli/yargswrapper"; export interface ITestArgs extends IProjectDirArgs, ICredentialsArgs, + IImpersonateServiceAccountArgs, ITimeoutArgs, IJsonOutputArgs, IProjectConfigArgs {} @@ -37,6 +40,7 @@ export const testCommand: ICommand = { check: [assertProjectDirExists], options: [ credentialsOption, + impersonateServiceAccountOption, timeoutOption, jsonOutputOption, ...ProjectConfigOptions.allYargsOptions @@ -60,6 +64,9 @@ export const testCommand: ICommand = { const readCredentials = credentials.read( actuallyResolve(argv.projectDir, argv.credentials) ); + if (argv.impersonateServiceAccount) { + (readCredentials as any).impersonateServiceAccount = argv.impersonateServiceAccount; + } if (!compiledGraph.tests.length) { printError("No unit tests found."); diff --git a/cli/common_options.ts b/cli/common_options.ts index 9ebf58875..06b6404fa 100644 --- a/cli/common_options.ts +++ b/cli/common_options.ts @@ -63,6 +63,18 @@ export const credentialsOption: INamedOption = actuallyResolve(argv.projectDir, argv.credentials) }; +export interface IImpersonateServiceAccountArgs { + impersonateServiceAccount?: string; +} + +export const impersonateServiceAccountOption: INamedOption = { + name: "impersonate-service-account", + option: { + describe: "Service account email to impersonate during authentication.", + type: "string" + } +}; + export interface IJsonOutputArgs { json: boolean; } diff --git a/package.json b/package.json index 5582ee7a1..ef7e701d7 100644 --- a/package.json +++ b/package.json @@ -39,6 +39,7 @@ "estraverse": "^5.1.0", "fs-extra": "^9.0.0", "glob": "13.0.6", + "google-auth-library": "^10.0.0-rc.1", "google-sql-syntax-ts": "^1.0.3", "js-beautify": "^1.10.2", "js-yaml": "^4.2.0", diff --git a/packages/@dataform/cli/BUILD b/packages/@dataform/cli/BUILD index 31309c581..541e3ef99 100644 --- a/packages/@dataform/cli/BUILD +++ b/packages/@dataform/cli/BUILD @@ -37,6 +37,7 @@ externals = [ "deepmerge", "fs-extra", "glob", + "google-auth-library", "google-sql-syntax-ts", "js-beautify", "js-yaml", diff --git a/protos/profiles.proto b/protos/profiles.proto index 09eabcdb8..37b982ac9 100644 --- a/protos/profiles.proto +++ b/protos/profiles.proto @@ -11,6 +11,8 @@ message BigQuery { string credentials = 3; // Options are listed here: https://cloud.google.com/bigquery/docs/locations string location = 4; + // Service account email to impersonate during authentication + string impersonate_service_account = 5; reserved 2; } From 25118f09f92aa91ccb9342bd63a9dded56d3bb4d Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Tue, 6 Jan 2026 00:41:23 +1100 Subject: [PATCH 02/12] Make changes as requested --- cli/api/dbadapters/bigquery.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index 9a5149481..c1b23e64a 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -205,11 +205,12 @@ export class BigQueryDbAdapter implements IDbAdapter { public async tables(database: string, schema?: string): Promise { const datasetIds = schema ? [schema] : await this.schemas(database); + const client = await this.getClient(database); const tablesMetadata: dataform.ITableMetadata[] = []; await Promise.all( datasetIds.map(async datasetId => { - const [tables] = await (await this.getClient(database)) + const [tables] = await client .dataset(datasetId) .getTables({ autoPaginate: true, maxResults: 1000 }); await Promise.all( From 7299f9a5be3441a9d454e293ed0b7e165d1c1184 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Tue, 6 Jan 2026 00:46:06 +1100 Subject: [PATCH 03/12] Make changes as requested --- cli/api/dbadapters/bigquery.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index c1b23e64a..215c17365 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -432,7 +432,8 @@ export class BigQueryDbAdapter implements IDbAdapter { return retry( async () => { try { - const job = await (await this.getClient()).createQueryJob( + const client = await this.getClient(); + const job = await client.createQueryJob( this.prepareQueryOptions( query, rowLimit, From 61fd22c317bbac291beeab9e6df06d34123183fd Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Tue, 6 Jan 2026 01:35:43 +1100 Subject: [PATCH 04/12] Make changes as requested --- cli/api/dbadapters/bigquery.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index 215c17365..0775c60d3 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -54,8 +54,8 @@ export function createBigQueryClientProvider( if (credentials.impersonateServiceAccount) { const sourceAuth = new GoogleAuth({ - scopes: ["https://www.googleapis.com/auth/cloud-platform"], projectId, + scopes: ["https://www.googleapis.com/auth/cloud-platform"], credentials: credentials.credentials && JSON.parse(credentials.credentials) }); From 3e905825162957c9d48481f23d00a20842e21500 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Tue, 6 Jan 2026 00:41:23 +1100 Subject: [PATCH 05/12] Make changes as requested --- cli/api/dbadapters/bigquery.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index 0775c60d3..d6df3ac12 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -54,7 +54,6 @@ export function createBigQueryClientProvider( if (credentials.impersonateServiceAccount) { const sourceAuth = new GoogleAuth({ - projectId, scopes: ["https://www.googleapis.com/auth/cloud-platform"], credentials: credentials.credentials && JSON.parse(credentials.credentials) }); From 2ca3d8311d0956536ebbb08be478a94753b4466f Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Tue, 6 Jan 2026 01:35:43 +1100 Subject: [PATCH 06/12] Make changes as requested --- cli/api/dbadapters/bigquery.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index d6df3ac12..6a9f7bb65 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -2,6 +2,7 @@ import { BigQuery, GetTablesResponse, TableField, TableMetadata } from "@google- import { GoogleAuth, Impersonated } from "google-auth-library"; import Long from "long"; import { PromisePoolExecutor } from "promise-pool-executor"; + import { collectEvaluationQueries, QueryOrAction } from "df/cli/api/dbadapters/execution_sql"; import { IBigQueryError, @@ -54,6 +55,7 @@ export function createBigQueryClientProvider( if (credentials.impersonateServiceAccount) { const sourceAuth = new GoogleAuth({ + projectId, scopes: ["https://www.googleapis.com/auth/cloud-platform"], credentials: credentials.credentials && JSON.parse(credentials.credentials) }); From e8329e850a022228b2dbac260edd2ee09601e7b0 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Fri, 10 Apr 2026 10:56:49 +1000 Subject: [PATCH 07/12] fix(cli): remove parse-duration dependency --- cli/BUILD | 1 - cli/common_options.ts | 5 ++- cli/util.ts | 68 ++++++++++++++++++++++++++++++++++++ cli/util_test.ts | 25 +++++++++++++ package.json | 1 - packages/@dataform/cli/BUILD | 1 - yarn.lock | 5 --- 7 files changed, 95 insertions(+), 11 deletions(-) diff --git a/cli/BUILD b/cli/BUILD index aa20288d4..17a47bb9c 100644 --- a/cli/BUILD +++ b/cli/BUILD @@ -41,7 +41,6 @@ ts_library( "@npm//@types/yargs", "@npm//chokidar", "@npm//glob", - "@npm//parse-duration", "@npm//readline-sync", "@npm//untildify", "@npm//yargs", diff --git a/cli/common_options.ts b/cli/common_options.ts index 06b6404fa..76e96e47b 100644 --- a/cli/common_options.ts +++ b/cli/common_options.ts @@ -1,10 +1,9 @@ import * as fs from "fs"; -import parseDuration from "parse-duration"; import * as path from "path"; import yargs from "yargs"; import { CREDENTIALS_FILENAME } from "df/cli/api/commands/credentials"; -import { actuallyResolve, assertPathExists } from "df/cli/util"; +import { actuallyResolve, assertPathExists, parseCliDuration } from "df/cli/util"; import { INamedOption } from "df/cli/yargswrapper"; export interface IProjectDirArgs { @@ -89,7 +88,7 @@ export const jsonOutputOption: INamedOption = { }; export const coerceTimeout = (rawTimeoutString: string | null) => - rawTimeoutString ? parseDuration(rawTimeoutString) : null; + rawTimeoutString ? parseCliDuration(rawTimeoutString) : null; export interface ITimeoutArgs { timeout: number | null; diff --git a/cli/util.ts b/cli/util.ts index dcd77b37b..2d46bb878 100644 --- a/cli/util.ts +++ b/cli/util.ts @@ -52,6 +52,74 @@ export function formatBytesInHumanReadableFormat(bytes: number): string { return `${value} ${units[i]}`; } +const DURATION_UNITS_IN_MILLIS: { [unit: string]: number } = { + ms: 1, + msec: 1, + msecs: 1, + millisecond: 1, + milliseconds: 1, + s: 1000, + sec: 1000, + secs: 1000, + second: 1000, + seconds: 1000, + m: 60 * 1000, + min: 60 * 1000, + mins: 60 * 1000, + minute: 60 * 1000, + minutes: 60 * 1000, + h: 60 * 60 * 1000, + hr: 60 * 60 * 1000, + hrs: 60 * 60 * 1000, + hour: 60 * 60 * 1000, + hours: 60 * 60 * 1000, + d: 24 * 60 * 60 * 1000, + day: 24 * 60 * 60 * 1000, + days: 24 * 60 * 60 * 1000, + w: 7 * 24 * 60 * 60 * 1000, + week: 7 * 24 * 60 * 60 * 1000, + weeks: 7 * 24 * 60 * 60 * 1000 +}; + +export function parseCliDuration(rawDuration: string): number { + const normalizedDuration = rawDuration?.trim().toLowerCase(); + if (!normalizedDuration) { + throw new Error("Duration cannot be empty."); + } + + if (/^[+-]?\d+(\.\d+)?$/.test(normalizedDuration)) { + return Number(normalizedDuration); + } + + let totalDurationMillis = 0; + let matchFound = false; + let cursor = 0; + const durationPattern = /([+-]?\d+(?:\.\d+)?)\s*([a-z]+)/g; + + for (let match = durationPattern.exec(normalizedDuration); match; match = durationPattern.exec(normalizedDuration)) { + if (normalizedDuration.slice(cursor, match.index).trim()) { + throw new Error(`Invalid duration: ${rawDuration}`); + } + + const durationValue = Number(match[1]); + const durationUnit = match[2]; + const unitMillis = DURATION_UNITS_IN_MILLIS[durationUnit]; + if (unitMillis === undefined) { + throw new Error(`Unsupported duration unit: ${durationUnit}`); + } + + totalDurationMillis += durationValue * unitMillis; + cursor = durationPattern.lastIndex; + matchFound = true; + } + + if (!matchFound || normalizedDuration.slice(cursor).trim()) { + throw new Error(`Invalid duration: ${rawDuration}`); + } + + return totalDurationMillis; +} + /** * Handles prompting and validation for defaultBucketName, defaultTableFolderRoot * and defaultTableFolderSubpath if the user provides the --iceberg flag when diff --git a/cli/util_test.ts b/cli/util_test.ts index b86f89370..cc15781fe 100644 --- a/cli/util_test.ts +++ b/cli/util_test.ts @@ -3,6 +3,7 @@ import { expect } from "chai"; import { formatBytesInHumanReadableFormat, formatExecutionSuffix, + parseCliDuration, validateIcebergConfigBucketName, validateIcebergConfigTableFolderRoot, validateIcebergConfigTableFolderSubpath, @@ -35,6 +36,30 @@ suite('format bytes in human readable format', () => { }); }); +suite("parse cli duration", () => { + test("parses numeric durations as milliseconds", () => { + expect(parseCliDuration("1500")).equals(1500); + }); + + test("parses single-unit durations", () => { + expect(parseCliDuration("1s")).equals(1000); + expect(parseCliDuration("10m")).equals(600000); + expect(parseCliDuration("2 hours")).equals(7200000); + }); + + test("parses compound and fractional durations", () => { + expect(parseCliDuration("1h30m")).equals(5400000); + expect(parseCliDuration("1.5m")).equals(90000); + expect(parseCliDuration("1 week 2 days")).equals(777600000); + }); + + test("rejects invalid durations", () => { + expect(() => parseCliDuration("")).to.throw("Duration cannot be empty."); + expect(() => parseCliDuration("tomorrow")).to.throw("Invalid duration: tomorrow"); + expect(() => parseCliDuration("1fortnight")).to.throw("Unsupported duration unit: fortnight"); + }); +}); + suite('Iceberg Config Validation', () => { suite('validateIcebergConfigBucketName', () => { test('valid bucket names do not throw errors', () => { diff --git a/package.json b/package.json index ef7e701d7..502d8e0c5 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,6 @@ "minimist": "^1.2.6", "moo": "^0.5.0", "object-sizeof": "^1.6.1", - "parse-duration": "^1.0.0", "prettier": "^1.14.2", "promise-pool-executor": "^1.1.1", "protobufjs": "^7.6.5", diff --git a/packages/@dataform/cli/BUILD b/packages/@dataform/cli/BUILD index 541e3ef99..367add25e 100644 --- a/packages/@dataform/cli/BUILD +++ b/packages/@dataform/cli/BUILD @@ -43,7 +43,6 @@ externals = [ "js-yaml", "moo", "object-sizeof", - "parse-duration", "promise-pool-executor", "protobufjs", "readline-sync", diff --git a/yarn.lock b/yarn.lock index ee7a9c3cb..d6c7ea5aa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -3772,11 +3772,6 @@ parent-module@^1.0.0: dependencies: callsites "^3.0.0" -parse-duration@^1.0.0: - version "1.0.0" - resolved "https://registry.yarnpkg.com/parse-duration/-/parse-duration-1.0.0.tgz#8605651745f61088f6fb14045c887526c291858c" - integrity "sha1-hgVlF0X2EIj2+xQEXIh1JsKRhYw= sha512-X4kUkCTHU1N/kEbwK9FpUJ0UZQa90VzeczfS704frR30gljxDG0pSziws06XlK+CGRSo/1wtG1mFIdBFQTMQNw==" - parse-semver@^1.1.1: version "1.1.1" resolved "https://registry.yarnpkg.com/parse-semver/-/parse-semver-1.1.1.tgz#9a4afd6df063dc4826f93fba4a99cf223f666cb8" From a10c281509af6218431b92badad9e494cb7c89e5 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Fri, 10 Apr 2026 11:08:00 +1000 Subject: [PATCH 08/12] refactor(cli): dedupe bigquery impersonation scopes --- cli/api/dbadapters/bigquery.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index 6a9f7bb65..8d4a3119b 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -18,7 +18,9 @@ import { coerceAsError } from "df/common/errors/errors"; import { retry } from "df/common/promises"; import { dataform } from "df/protos/ts"; +const GOOGLE_CLOUD_PLATFORM_SCOPE = "https://www.googleapis.com/auth/cloud-platform"; const EXTRA_GOOGLE_SCOPES = ["https://www.googleapis.com/auth/drive"]; +const IMPERSONATION_GOOGLE_SCOPES = [GOOGLE_CLOUD_PLATFORM_SCOPE, ...EXTRA_GOOGLE_SCOPES]; const BIGQUERY_DATE_RELATED_FIELDS = [ "BigQueryDate", @@ -56,7 +58,7 @@ export function createBigQueryClientProvider( if (credentials.impersonateServiceAccount) { const sourceAuth = new GoogleAuth({ projectId, - scopes: ["https://www.googleapis.com/auth/cloud-platform"], + scopes: IMPERSONATION_GOOGLE_SCOPES, credentials: credentials.credentials && JSON.parse(credentials.credentials) }); @@ -65,7 +67,7 @@ export function createBigQueryClientProvider( clientConfig.authClient = new Impersonated({ sourceClient: authClient, targetPrincipal: credentials.impersonateServiceAccount, - targetScopes: ["https://www.googleapis.com/auth/cloud-platform"] + targetScopes: IMPERSONATION_GOOGLE_SCOPES }); } else { clientConfig.credentials = credentials.credentials && JSON.parse(credentials.credentials); From 0573649f96aa00c8b8ddfcc0161ba0a80f2109d5 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Wed, 17 Jun 2026 17:16:12 +1000 Subject: [PATCH 09/12] refactor(cli): rewrite duration parser --- cli/util.ts | 89 ++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 81 insertions(+), 8 deletions(-) diff --git a/cli/util.ts b/cli/util.ts index 2d46bb878..921e26c13 100644 --- a/cli/util.ts +++ b/cli/util.ts @@ -87,39 +87,112 @@ export function parseCliDuration(rawDuration: string): number { throw new Error("Duration cannot be empty."); } - if (/^[+-]?\d+(\.\d+)?$/.test(normalizedDuration)) { + if (isCliDurationNumber(normalizedDuration)) { return Number(normalizedDuration); } let totalDurationMillis = 0; let matchFound = false; let cursor = 0; - const durationPattern = /([+-]?\d+(?:\.\d+)?)\s*([a-z]+)/g; - for (let match = durationPattern.exec(normalizedDuration); match; match = durationPattern.exec(normalizedDuration)) { - if (normalizedDuration.slice(cursor, match.index).trim()) { + while (cursor < normalizedDuration.length) { + while (normalizedDuration[cursor] === " ") { + cursor++; + } + if (cursor >= normalizedDuration.length) { + break; + } + + const numberStart = cursor; + if (normalizedDuration[cursor] === "+" || normalizedDuration[cursor] === "-") { + cursor++; + } + + const integerStart = cursor; + while (isAsciiDigit(normalizedDuration[cursor])) { + cursor++; + } + if (cursor === integerStart) { + throw new Error(`Invalid duration: ${rawDuration}`); + } + + if (normalizedDuration[cursor] === ".") { + cursor++; + const fractionStart = cursor; + while (isAsciiDigit(normalizedDuration[cursor])) { + cursor++; + } + if (cursor === fractionStart) { + throw new Error(`Invalid duration: ${rawDuration}`); + } + } + + while (normalizedDuration[cursor] === " ") { + cursor++; + } + + const unitStart = cursor; + while (isAsciiLetter(normalizedDuration[cursor])) { + cursor++; + } + if (cursor === unitStart) { throw new Error(`Invalid duration: ${rawDuration}`); } - const durationValue = Number(match[1]); - const durationUnit = match[2]; + const durationValue = Number(normalizedDuration.slice(numberStart, unitStart).trim()); + const durationUnit = normalizedDuration.slice(unitStart, cursor); const unitMillis = DURATION_UNITS_IN_MILLIS[durationUnit]; if (unitMillis === undefined) { throw new Error(`Unsupported duration unit: ${durationUnit}`); } totalDurationMillis += durationValue * unitMillis; - cursor = durationPattern.lastIndex; matchFound = true; } - if (!matchFound || normalizedDuration.slice(cursor).trim()) { + if (!matchFound) { throw new Error(`Invalid duration: ${rawDuration}`); } return totalDurationMillis; } +function isCliDurationNumber(value: string): boolean { + let cursor = 0; + if (value[cursor] === "+" || value[cursor] === "-") { + cursor++; + } + + const integerStart = cursor; + while (isAsciiDigit(value[cursor])) { + cursor++; + } + if (cursor === integerStart) { + return false; + } + + if (value[cursor] === ".") { + cursor++; + const fractionStart = cursor; + while (isAsciiDigit(value[cursor])) { + cursor++; + } + if (cursor === fractionStart) { + return false; + } + } + + return cursor === value.length; +} + +function isAsciiDigit(value: string): boolean { + return value >= "0" && value <= "9"; +} + +function isAsciiLetter(value: string): boolean { + return value >= "a" && value <= "z"; +} + /** * Handles prompting and validation for defaultBucketName, defaultTableFolderRoot * and defaultTableFolderSubpath if the user provides the --iceberg flag when From 8fde3c89bb10e26042c8b6757679399eb1ebe09c Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Thu, 3 Sep 2026 12:49:01 +1000 Subject: [PATCH 10/12] fix(cli): address impersonation review feedback --- cli/api/dbadapters/bigquery.ts | 8 +++++--- cli/commands/run_command.ts | 2 +- cli/commands/test_command.ts | 2 +- cli/util.ts | 4 ++++ 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index 8d4a3119b..06467099c 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -1,4 +1,4 @@ -import { BigQuery, GetTablesResponse, TableField, TableMetadata } from "@google-cloud/bigquery"; +import { BigQuery, BigQueryOptions, GetTablesResponse, TableField, TableMetadata } from "@google-cloud/bigquery"; import { GoogleAuth, Impersonated } from "google-auth-library"; import Long from "long"; import { PromisePoolExecutor } from "promise-pool-executor"; @@ -49,13 +49,14 @@ export function createBigQueryClientProvider( return async (projectId?: string) => { projectId = projectId || credentials.projectId; if (!clients.has(projectId)) { - const clientConfig: any = { + const clientConfig: BigQueryOptions = { projectId, - scopes: EXTRA_GOOGLE_SCOPES, location: credentials.location }; if (credentials.impersonateServiceAccount) { + // Impersonation requires cloud-platform for the source and target credentials, while the + // Drive scope remains necessary for BigQuery external tables backed by Google Drive. const sourceAuth = new GoogleAuth({ projectId, scopes: IMPERSONATION_GOOGLE_SCOPES, @@ -70,6 +71,7 @@ export function createBigQueryClientProvider( targetScopes: IMPERSONATION_GOOGLE_SCOPES }); } else { + clientConfig.scopes = EXTRA_GOOGLE_SCOPES; clientConfig.credentials = credentials.credentials && JSON.parse(credentials.credentials); } diff --git a/cli/commands/run_command.ts b/cli/commands/run_command.ts index 63a786c11..c596f5ab0 100644 --- a/cli/commands/run_command.ts +++ b/cli/commands/run_command.ts @@ -94,7 +94,7 @@ export const runCommand: ICommand = { actuallyResolve(argv.projectDir, argv.credentials) ); if (argv.impersonateServiceAccount) { - (readCredentials as any).impersonateServiceAccount = argv.impersonateServiceAccount; + readCredentials.impersonateServiceAccount = argv.impersonateServiceAccount; } const dbadapter = new BigQueryDbAdapter(readCredentials); diff --git a/cli/commands/test_command.ts b/cli/commands/test_command.ts index fa6bc8b45..59486e829 100644 --- a/cli/commands/test_command.ts +++ b/cli/commands/test_command.ts @@ -65,7 +65,7 @@ export const testCommand: ICommand = { actuallyResolve(argv.projectDir, argv.credentials) ); if (argv.impersonateServiceAccount) { - (readCredentials as any).impersonateServiceAccount = argv.impersonateServiceAccount; + readCredentials.impersonateServiceAccount = argv.impersonateServiceAccount; } if (!compiledGraph.tests.length) { diff --git a/cli/util.ts b/cli/util.ts index 921e26c13..230283686 100644 --- a/cli/util.ts +++ b/cli/util.ts @@ -81,6 +81,10 @@ const DURATION_UNITS_IN_MILLIS: { [unit: string]: number } = { weeks: 7 * 24 * 60 * 60 * 1000 }; +// Security-fixed parse-duration releases (2.1.3 and later) are ESM-only, but this Bazel +// TypeScript target emits CommonJS for unbundled CLI and test execution. Keep this parser as a +// linear scanner so those entrypoints remain compatible without reintroducing CVE-2025-25283's +// unsafe regular-expression behavior. export function parseCliDuration(rawDuration: string): number { const normalizedDuration = rawDuration?.trim().toLowerCase(); if (!normalizedDuration) { From c268d85fbd9fe80021918cd9384e81ebef6d2406 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:10:49 +1000 Subject: [PATCH 11/12] fix(cli): await BigQuery client before query streams --- cli/api/dbadapters/bigquery.ts | 5 ++-- cli/api/dbadapters/bigquery_test.ts | 39 +++++++++++++++++++++++++++++ cli/index_project_test.ts | 35 ++++++++++++++++++++++++++ cli/index_test_base.ts | 12 ++++++++- 4 files changed, 88 insertions(+), 3 deletions(-) diff --git a/cli/api/dbadapters/bigquery.ts b/cli/api/dbadapters/bigquery.ts index 06467099c..b94594289 100644 --- a/cli/api/dbadapters/bigquery.ts +++ b/cli/api/dbadapters/bigquery.ts @@ -376,12 +376,13 @@ export class BigQueryDbAdapter implements IDbAdapter { byteLimit?: number, location?: string ): Promise { - const results = await new Promise(async (resolve, reject) => { + const client = await this.getClient(); + const results = await new Promise((resolve, reject) => { const allRows = new LimitedResultSet({ rowLimit, byteLimit }); - const stream = (await this.getClient()).createQueryStream({ + const stream = client.createQueryStream({ query, params, location diff --git a/cli/api/dbadapters/bigquery_test.ts b/cli/api/dbadapters/bigquery_test.ts index 6659941e8..92462d096 100644 --- a/cli/api/dbadapters/bigquery_test.ts +++ b/cli/api/dbadapters/bigquery_test.ts @@ -1,5 +1,6 @@ import { Dataset, Table } from "@google-cloud/bigquery"; import { expect } from "chai"; +import { Readable } from "stream"; import { anything, instance, mock, verify, when } from "ts-mockito"; import { BigQueryDbAdapter } from "df/cli/api/dbadapters/bigquery"; @@ -9,6 +10,8 @@ import { suite, test } from "df/testing"; suite("BigQueryDbAdapter", () => { test("tables() with schema filters correctly", async () => { const mockBigQuery = mock(); + // ts-mockito invents a then() method, which prevents await from resolving this client. + when(mockBigQuery.then).thenReturn(undefined); const mockDataset = mock(); const mockTable = mock(); @@ -49,6 +52,7 @@ suite("BigQueryDbAdapter", () => { test("tables() without schema lists all datasets and tables", async () => { const mockBigQuery = mock(); + when(mockBigQuery.then).thenReturn(undefined); const mockDataset = mock(); const mockTable = mock
(); const schemaName = "schema1"; @@ -84,6 +88,41 @@ suite("BigQueryDbAdapter", () => { expect(result[0].target.name).to.equal(tableName); }); + for (const asynchronous of [false, true]) { + test(`interactive queries accept ${asynchronous ? "asynchronous" : "synchronous"} clients`, async () => { + const client = { + createQueryStream: () => Readable.from([{ id: 1 }]) + } as any; + const adapter = new BigQueryDbAdapter({ projectId: "project" }, { + clientProvider: () => asynchronous ? Promise.resolve(client) : client + }); + + const result = await adapter.execute("SELECT 1 AS id", { interactive: true }); + expect(result.rows).deep.equals([{ id: 1 }]); + }); + } + + for (const failure of ["client initialization", "stream creation"]) { + test(`interactive queries reject on ${failure} failure`, async () => { + const expectedError = new Error(failure); + const adapter = new BigQueryDbAdapter({ projectId: "project" }, { + clientProvider: async () => { + if (failure === "client initialization") { + throw expectedError; + } + return { + createQueryStream: () => { throw expectedError; } + } as any; + } + }); + + await adapter.execute("SELECT 1", { interactive: true }).then( + () => { throw new Error("Expected the query to reject"); }, + error => expect(error).equals(expectedError) + ); + }); + } + test("setMetadata handles action without columns", async () => { // Partial mock for BigQuery client to avoid real network calls const mockBigQuery: any = { diff --git a/cli/index_project_test.ts b/cli/index_project_test.ts index c53814da6..3141a4243 100644 --- a/cli/index_project_test.ts +++ b/cli/index_project_test.ts @@ -1,5 +1,7 @@ import { expect } from "chai"; import * as fs from "fs-extra"; +import { createServer } from "http"; +import { AddressInfo } from "net"; import * as path from "path"; import { @@ -12,6 +14,39 @@ import { TmpDirFixture } from "df/testing/fixtures"; suite("project ops", ({ afterEach }) => { const tmpDirFixture = new TmpDirFixture(afterEach); + test("project setup installs the local core package without registry requests", async () => { + const requests: string[] = []; + const registry = createServer((request, response) => { + requests.push(request.url); + response.writeHead(503); + response.end(); + }); + await new Promise(resolve => registry.listen(0, "127.0.0.1", resolve)); + const previousRegistry = process.env.npm_config_registry; + const previousRetries = process.env.npm_config_fetch_retries; + try { + process.env.npm_config_registry = `http://127.0.0.1:${(registry.address() as AddressInfo).port}`; + process.env.npm_config_fetch_retries = "0"; + const projectDir = tmpDirFixture.createNewTmpDir(); + await setupProject(tmpDirFixture, projectDir); + + expect(fs.existsSync(path.join(projectDir, "node_modules/@dataform/core/bundle.js"))).equals(true); + expect(requests).deep.equals([]); + } finally { + if (previousRegistry === undefined) { + delete process.env.npm_config_registry; + } else { + process.env.npm_config_registry = previousRegistry; + } + if (previousRetries === undefined) { + delete process.env.npm_config_fetch_retries; + } else { + process.env.npm_config_fetch_retries = previousRetries; + } + await new Promise(resolve => registry.close(() => resolve())); + } + }); + suite("install command", () => { test("install throws an error when dataformCoreVersion in workflow_settings.yaml", async () => { const projectDir = tmpDirFixture.createNewTmpDir(); diff --git a/cli/index_test_base.ts b/cli/index_test_base.ts index 5845c5723..750acfa60 100644 --- a/cli/index_test_base.ts +++ b/cli/index_test_base.ts @@ -88,9 +88,16 @@ export async function setupProject( } }` ); - await getProcessResult( + const installResult = await getProcessResult( execFile(npmPath, [ "install", + // Core is bundled with no runtime dependencies. Keep fixture setup independent of npm + // registry availability, including the audit and update checks performed by npm install. + "--offline", + "--no-audit", + "--no-fund", + "--ignore-scripts", + "--update-notifier=false", "--prefix", projectDir, "--cache", @@ -98,6 +105,9 @@ export async function setupProject( corePackageTarPath ]) ); + if (installResult.exitCode !== 0) { + throw new Error(`Failed to install the local core package: ${installResult.stderr}`); + } return projectDir; } From 3b2eb651fe757f02343946bc89ca2b4039a2a364 Mon Sep 17 00:00:00 2001 From: Matthew Winter <33818+wintermi@users.noreply.github.com> Date: Mon, 14 Sep 2026 23:14:00 +1000 Subject: [PATCH 12/12] fix(cli): accept leading and trailing decimals in duration parser --- cli/util.ts | 21 ++++++++------------- cli/util_test.ts | 20 ++++++++++++++++++++ 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/cli/util.ts b/cli/util.ts index 230283686..07601dc8b 100644 --- a/cli/util.ts +++ b/cli/util.ts @@ -116,9 +116,7 @@ export function parseCliDuration(rawDuration: string): number { while (isAsciiDigit(normalizedDuration[cursor])) { cursor++; } - if (cursor === integerStart) { - throw new Error(`Invalid duration: ${rawDuration}`); - } + let digitCount = cursor - integerStart; if (normalizedDuration[cursor] === ".") { cursor++; @@ -126,9 +124,10 @@ export function parseCliDuration(rawDuration: string): number { while (isAsciiDigit(normalizedDuration[cursor])) { cursor++; } - if (cursor === fractionStart) { - throw new Error(`Invalid duration: ${rawDuration}`); - } + digitCount += cursor - fractionStart; + } + if (digitCount === 0) { + throw new Error(`Invalid duration: ${rawDuration}`); } while (normalizedDuration[cursor] === " ") { @@ -171,9 +170,7 @@ function isCliDurationNumber(value: string): boolean { while (isAsciiDigit(value[cursor])) { cursor++; } - if (cursor === integerStart) { - return false; - } + let digitCount = cursor - integerStart; if (value[cursor] === ".") { cursor++; @@ -181,12 +178,10 @@ function isCliDurationNumber(value: string): boolean { while (isAsciiDigit(value[cursor])) { cursor++; } - if (cursor === fractionStart) { - return false; - } + digitCount += cursor - fractionStart; } - return cursor === value.length; + return digitCount > 0 && cursor === value.length; } function isAsciiDigit(value: string): boolean { diff --git a/cli/util_test.ts b/cli/util_test.ts index cc15781fe..f4c9e9cb9 100644 --- a/cli/util_test.ts +++ b/cli/util_test.ts @@ -53,10 +53,30 @@ suite("parse cli duration", () => { expect(parseCliDuration("1 week 2 days")).equals(777600000); }); + for (const [input, expected] of [ + [".5s", 500], + ["5.s", 5000], + ["-.5s", -500], + ["+5.s", 5000], + ["1m .5s", 60500], + ["1m5.s", 65000], + [".5", 0.5], + ["5.", 5], + ["-.5", -0.5], + ["+5.", 5] + ] as Array<[string, number]>) { + test(`parses decimal duration ${input}`, () => { + expect(parseCliDuration(input)).equals(expected); + }); + } + test("rejects invalid durations", () => { expect(() => parseCliDuration("")).to.throw("Duration cannot be empty."); expect(() => parseCliDuration("tomorrow")).to.throw("Invalid duration: tomorrow"); expect(() => parseCliDuration("1fortnight")).to.throw("Unsupported duration unit: fortnight"); + for (const input of [".", ".s", "+.s", "-.s", "5..s"]) { + expect(() => parseCliDuration(input)).to.throw(`Invalid duration: ${input}`); + } }); });