Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 26 additions & 6 deletions .github/meta/commit.txt
Original file line number Diff line number Diff line change
@@ -1,10 +1,30 @@
fix: comprehensive XSS hardening for trace viewer HTML
fix: `altimate-dbt` compile, execute, and children commands — CLI fallbacks for dbt 1.11+ (#252)

Systematically escape all user-controllable fields in `viewer.ts`:
The `@altimateai/dbt-integration` library's JSON output parsing breaks with
newer dbt versions (1.11.x) where the log format changed. Three commands
were affected:

- Escape `span.kind` and `span.status` in detail panel, waterfall, tree, and log views
- Escape `span.spanId` in `data-sid` attributes
- Coerce all numeric fields with `Number()` to prevent string injection via `.toLocaleString()`
- Add single-quote escaping (`'`) to the `e()` function for defense-in-depth
- `execute`: `dbt show` output no longer contains `data.preview` in the
expected format — `d[0].data` throws when the filter returns empty.
- `compile`: `dbt compile` output no longer contains `data.compiled` —
same `o[0].data` pattern failure.
- `children`: `nodeMetaMap.lookupByBaseName()` fails when the manifest
file-path resolution doesn't populate the model-name lookup map.

Additionally, `tryExecuteViaDbt` in opencode incorrectly expected
`raw.table` on `QueryExecutionResult`, which actually has `{ columnNames,
columnTypes, data }` — causing the dbt-first execution path to always
fall through to native drivers silently.

Fixes:
- Add try-catch in execute/compile/graph commands with fallback to direct
`dbt` CLI subprocess calls (`dbt show`, `dbt compile`, `dbt ls`)
- New `dbt-cli.ts` module with resilient multi-format JSON output parsing
(handles `data.preview`, `data.rows`, `data.compiled`, `data.compiled_code`,
`result.node.compiled_code`)
- Fix `tryExecuteViaDbt` to recognize `QueryExecutionResult` shape first,
then fall back to legacy `raw.table` format

Closes #252

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
24 changes: 20 additions & 4 deletions packages/dbt-tools/src/commands/compile.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,34 @@
import type { DBTProjectIntegrationAdapter } from "@altimateai/dbt-integration"
import { execDbtCompile, execDbtCompileInline } from "../dbt-cli"

export async function compile(adapter: DBTProjectIntegrationAdapter, args: string[]) {
const model = flag(args, "model")
if (!model) return { error: "Missing --model" }
const sql = await adapter.unsafeCompileNode(model)
return { sql }
try {
const sql = await adapter.unsafeCompileNode(model)
return { sql }
} catch (e) {
// Use TypeError check (not message strings) to work across V8 and Bun/JavaScriptCore
if (e instanceof TypeError) {
return execDbtCompile(model)
}
Comment on lines +12 to +14
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: The error handling in compile.ts only catches TypeError, unlike execute.ts and graph.ts, potentially missing other error types and preventing the intended fallback from executing.
Severity: MEDIUM

Suggested Fix

Align the error handling in compile.ts with the pattern used in execute.ts and graph.ts. Broaden the catch condition to include e instanceof Error and check for specific, relevant error messages if known, or handle all Error instances to ensure the fallback is always triggered on parsing failures.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: packages/dbt-tools/src/commands/compile.ts#L12-L14

Potential issue: The error handling in `compile.ts` is less defensive than in
`execute.ts` and `graph.ts`. It only catches `TypeError` to trigger a fallback
mechanism. However, the underlying parsing library might throw other `Error` types with
specific messages, as is handled in the other command files. If a non-`TypeError` is
thrown during compilation, the `catch` block will be missed, the fallback will not
execute, and the user will see an unhelpful error message instead of the expected
compiled output.

throw e
}
}

export async function query(adapter: DBTProjectIntegrationAdapter, args: string[]) {
const sql = flag(args, "query")
if (!sql) return { error: "Missing --query" }
const model = flag(args, "model")
const result = await adapter.unsafeCompileQuery(sql, model)
return { sql: result }
try {
const result = await adapter.unsafeCompileQuery(sql, model)
return { sql: result }
} catch (e) {
if (e instanceof TypeError) {
return execDbtCompileInline(sql, model)
}
throw e
}
}

function flag(args: string[], name: string): string | undefined {
Expand Down
14 changes: 12 additions & 2 deletions packages/dbt-tools/src/commands/execute.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
import type { DBTProjectIntegrationAdapter } from "@altimateai/dbt-integration"
import { execDbtShow } from "../dbt-cli"

export async function execute(adapter: DBTProjectIntegrationAdapter, args: string[]) {
const sql = flag(args, "query")
if (!sql) return { error: "Missing --query" }
const model = flag(args, "model") ?? ""
const raw = flag(args, "limit")
const limit = raw !== undefined ? parseInt(raw, 10) : undefined
if (limit !== undefined && !Number.isNaN(limit)) return adapter.immediatelyExecuteSQLWithLimit(sql, model, limit)
return adapter.immediatelyExecuteSQL(sql, model)
try {
if (limit !== undefined && !Number.isNaN(limit)) return await adapter.immediatelyExecuteSQLWithLimit(sql, model, limit)
return await adapter.immediatelyExecuteSQL(sql, model)
} catch (e) {
// Library's dbt show parsing may fail with newer dbt versions — fall back to direct CLI.
// Use TypeError check (not message strings) to work across V8 and Bun/JavaScriptCore.
if (e instanceof TypeError || (e instanceof Error && e.message.includes("Could not find previewLine"))) {
return execDbtShow(sql, limit)
}
throw e
}
}

function flag(args: string[], name: string): string | undefined {
Expand Down
31 changes: 27 additions & 4 deletions packages/dbt-tools/src/commands/graph.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,38 @@
import type { DBTProjectIntegrationAdapter } from "@altimateai/dbt-integration"
import { execDbtLs } from "../dbt-cli"

export function children(adapter: DBTProjectIntegrationAdapter, args: string[]) {
export async function children(adapter: DBTProjectIntegrationAdapter, args: string[]) {
const model = flag(args, "model")
if (!model) return { error: "Missing --model" }
return adapter.getChildrenModels({ table: model })
try {
return await adapter.getChildrenModels({ table: model })
} catch (e) {
// nodeMetaMap/graphMetaMap errors are specific to the library's manifest parsing.
// Also catch TypeError for property-access failures on undefined nodes.
if (
e instanceof TypeError ||
(e instanceof Error && (e.message.includes("nodeMetaMap has no entries") || e.message.includes("graphMetaMap")))
) {
return execDbtLs(model, "children")
}
throw e
}
}

export function parents(adapter: DBTProjectIntegrationAdapter, args: string[]) {
export async function parents(adapter: DBTProjectIntegrationAdapter, args: string[]) {
const model = flag(args, "model")
if (!model) return { error: "Missing --model" }
return adapter.getParentModels({ table: model })
try {
return await adapter.getParentModels({ table: model })
} catch (e) {
if (
e instanceof TypeError ||
(e instanceof Error && (e.message.includes("nodeMetaMap has no entries") || e.message.includes("graphMetaMap")))
) {
return execDbtLs(model, "parents")
}
throw e
}
}

function flag(args: string[], name: string): string | undefined {
Expand Down
37 changes: 35 additions & 2 deletions packages/dbt-tools/src/commands/init.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,40 @@ function find(start: string): string | null {
}
}

function python(): string {
/**
* Discover the Python binary, checking multiple environment managers.
*
* Priority:
* 1. Project-local .venv/bin/python (uv, pdm, venv, poetry in-project)
* 2. VIRTUAL_ENV/bin/python (activated venv)
* 3. CONDA_PREFIX/bin/python (conda)
* 4. `which python3` / `which python` (system PATH)
* 5. Fallback "python3" (hope for the best)
*/
function python(projectRoot?: string): string {
// Check project-local venvs first (most reliable for dbt projects)
if (projectRoot) {
for (const venvDir of [".venv", "venv", "env"]) {
const py = join(projectRoot, venvDir, "bin", "python")
if (existsSync(py)) return py
}
}

// Check VIRTUAL_ENV (set by activate scripts)
const virtualEnv = process.env.VIRTUAL_ENV
if (virtualEnv) {
const py = join(virtualEnv, "bin", "python")
if (existsSync(py)) return py
}

// Check CONDA_PREFIX (set by conda activate)
const condaPrefix = process.env.CONDA_PREFIX
if (condaPrefix) {
const py = join(condaPrefix, "bin", "python")
if (existsSync(py)) return py
}

// Fall back to PATH-based discovery
for (const cmd of ["python3", "python"]) {
try {
return execFileSync("which", [cmd], { encoding: "utf-8" }).trim()
Expand All @@ -35,7 +68,7 @@ export async function init(args: string[]) {

const cfg: Config = {
projectRoot: project,
pythonPath: py ?? python(),
pythonPath: py ?? python(project),
dbtIntegration: "corecommand",
queryLimit: 500,
}
Expand Down
Loading
Loading