From 69ebc03105f75bf6006829080ef93b5cfefec149 Mon Sep 17 00:00:00 2001 From: August Cayzer Date: Fri, 29 May 2026 21:57:34 +0100 Subject: [PATCH] Prevent .sqlx files in actions.yaml configs with a clear error Putting a .sqlx file under the ``filename`` field of an entry in ``definitions/actions.yaml`` used to fall through to ``nativeRequire``, which then failed with a cryptic error far from the source of the mistake. ``.sqlx`` files are loaded directly from the ``definitions/`` directory by the sqlx compiler, not referenced through ``actions.yaml``. Add an explicit validation step in ``loadActionConfigs`` that runs before each action is constructed. If the action's ``filename`` ends in ``.sqlx`` (case-insensitive), emit a ``compileError`` naming the action type, the offending filename, and pointing at the fix. The action is then skipped so we don't double-up with a downstream ``nativeRequire`` error. The check covers every action type that accepts a ``filename``: ``table``, ``view``, ``incrementalTable``, ``assertion``, ``operation``, ``declaration``, ``notebook`` and ``dataPreparation``. Data preparations are included because a ``.dp.sqlx`` file is also auto-compiled from ``definitions/`` - referencing one from ``actions.yaml`` doesn't resolve its path, ignores its ``config {}`` block and double-registers the action, so it's the same mistake. ``.dp.yaml`` data preparations are unaffected. Closes #1785 --- core/main.ts | 49 ++++++++++++++++++ core/main_test.ts | 127 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 176 insertions(+) diff --git a/core/main.ts b/core/main.ts index b4ead165a..844b5b48d 100644 --- a/core/main.ts +++ b/core/main.ts @@ -75,6 +75,51 @@ export function main(coreExecutionRequest: Uint8Array | string): Uint8Array | st return dataform.CoreExecutionResponse.encode(coreExecutionResponse).finish(); } +// Every action type that accepts a `filename` in actions.yaml. Referencing a +// .sqlx file (including a data-preparation .dp.sqlx file) is unsupported: .sqlx +// files are compiled directly from the definitions/ directory by the sqlx +// compiler, not through the `actions.yaml` config. Without this check a .sqlx +// filename produces a cryptic `nativeRequire` error rather than a useful +// diagnostic, so we emit a clear compilation error up front. See issue #1785. +const ACTION_TYPES_REJECTING_SQLX_FILENAMES: string[] = [ + "table", + "view", + "incrementalTable", + "assertion", + "operation", + "declaration", + "notebook", + "dataPreparation" +]; + +function actionConfigFilenameIsInvalidSqlx( + session: Session, + actionConfig: dataform.ActionConfig, + actionConfigsPath: string +): boolean { + for (const actionType of ACTION_TYPES_REJECTING_SQLX_FILENAMES) { + const subConfig = (actionConfig as any)[actionType] as + | { filename?: string } + | undefined + | null; + const filename = subConfig?.filename; + if (filename && filename.toLowerCase().endsWith(".sqlx")) { + session.compileError( + new Error( + `Action config "${actionType}" has filename "${filename}", but .sqlx ` + + `files cannot be referenced from actions.yaml. .sqlx files are ` + + `compiled directly from the definitions/ directory. Either use a ` + + `.sql file with the same contents, or remove the actions.yaml ` + + `entry and let Dataform pick up the .sqlx file automatically.` + ), + actionConfigsPath + ); + return true; + } + } + return false; +} + function loadActionConfigs(session: Session, filePaths: string[]) { filePaths .filter( @@ -89,6 +134,10 @@ function loadActionConfigs(session: Session, filePaths: string[]) { actionConfigs.actions.forEach(nonProtoActionConfig => { const actionConfig = dataform.ActionConfig.create(nonProtoActionConfig); + if (actionConfigFilenameIsInvalidSqlx(session, actionConfig, actionConfigsPath)) { + return; + } + if (actionConfig.table) { session.actions.push( new Table( diff --git a/core/main_test.ts b/core/main_test.ts index c23f62f2e..7fb25b185 100644 --- a/core/main_test.ts +++ b/core/main_test.ts @@ -994,6 +994,133 @@ actions:` ); }); + suite(`.sqlx filenames in actions.yaml are rejected with a clear error`, () => { + [ + { + actionType: "table", + yamlBody: ` +actions: +- table: + filename: table.sqlx` + }, + { + actionType: "view", + yamlBody: ` +actions: +- view: + filename: view.sqlx` + }, + { + actionType: "incrementalTable", + yamlBody: ` +actions: +- incrementalTable: + filename: incremental.sqlx` + }, + { + actionType: "assertion", + yamlBody: ` +actions: +- assertion: + filename: assertion.sqlx` + }, + { + actionType: "operation", + yamlBody: ` +actions: +- operation: + filename: operation.sqlx` + }, + { + actionType: "declaration", + yamlBody: ` +actions: +- declaration: + name: declaration + filename: declaration.sqlx` + }, + { + actionType: "notebook", + yamlBody: ` +actions: +- notebook: + filename: notebook.sqlx` + } + ].forEach(({ actionType, yamlBody }) => { + test(`for action type "${actionType}"`, () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + fs.writeFileSync( + path.join(projectDir, "workflow_settings.yaml"), + VALID_WORKFLOW_SETTINGS_YAML + ); + fs.mkdirSync(path.join(projectDir, "definitions")); + fs.writeFileSync(path.join(projectDir, "definitions/actions.yaml"), yamlBody); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + const errorMessages = result.compile.compiledGraph.graphErrors.compilationErrors.map( + ({ message }) => message + ); + expect(errorMessages).to.have.lengthOf(1); + expect(errorMessages[0]).to.include(`Action config "${actionType}" has filename`); + expect(errorMessages[0]).to.include( + ".sqlx files cannot be referenced from actions.yaml" + ); + }); + }); + + test(`is case-insensitive`, () => { + const projectDir = tmpDirFixture.createNewTmpDir(); + fs.writeFileSync( + path.join(projectDir, "workflow_settings.yaml"), + VALID_WORKFLOW_SETTINGS_YAML + ); + fs.mkdirSync(path.join(projectDir, "definitions")); + fs.writeFileSync( + path.join(projectDir, "definitions/actions.yaml"), + ` +actions: +- table: + filename: table.SQLX` + ); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + expect( + result.compile.compiledGraph.graphErrors.compilationErrors.map(({ message }) => message) + ).to.have.lengthOf(1); + }); + + test(`for data preparations referencing a .dp.sqlx file`, () => { + // .dp.sqlx data preparations are compiled directly from the definitions/ + // directory, so referencing one from actions.yaml is the same mistake as + // for any other action type and must produce the same clear error rather + // than silently loading a malformed, duplicated action. + const projectDir = tmpDirFixture.createNewTmpDir(); + fs.writeFileSync( + path.join(projectDir, "workflow_settings.yaml"), + VALID_WORKFLOW_SETTINGS_YAML + ); + fs.mkdirSync(path.join(projectDir, "definitions")); + fs.writeFileSync( + path.join(projectDir, "definitions/actions.yaml"), + ` +actions: +- dataPreparation: + filename: prep.dp.sqlx` + ); + + const result = runMainInVm(coreExecutionRequestFromPath(projectDir)); + + const errorMessages = result.compile.compiledGraph.graphErrors.compilationErrors.map( + ({ message }) => message + ); + expect(errorMessages).to.have.lengthOf(1); + expect(errorMessages[0]).to.include(`Action config "dataPreparation" has filename`); + expect(errorMessages[0]).to.include(".sqlx files cannot be referenced from actions.yaml"); + }); + }); + test(`filenames with non-UTF8 characters are valid`, () => { const projectDir = tmpDirFixture.createNewTmpDir(); fs.writeFileSync(