Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 134
fix: altimate-dbt compile, execute, and children commands fail with runtime errors#255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
9f0afa8a582fbd5f9e772108b7bed803cfe0ece4e3613ea88d329159f3cd0da7a7dfc0File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| 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) <noreply@anthropic.com> |
| 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 | ||
| } | ||
anandgupta42 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. anandgupta42 marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| } | ||
| 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 { | ||
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.tsonly catchesTypeError, unlikeexecute.tsandgraph.ts, potentially missing other error types and preventing the intended fallback from executing.Severity: MEDIUM
Suggested Fix
Align the error handling in
compile.tswith the pattern used inexecute.tsandgraph.ts. Broaden thecatchcondition to includee instanceof Errorand check for specific, relevant error messages if known, or handle allErrorinstances to ensure the fallback is always triggered on parsing failures.Prompt for AI Agent