Native SQL Templating #32

Problem

Currently, Named Pipes likely rely on basic string replacement to inject user parameters into SQL templates.

Here are some reasons why this is bad:

• SQL Injection: An attacker could provide a parameter like 1; DROP TABLE events, which, if replaced directly into the string, could execute malicious commands on ClickHouse.
• Type Mismatches: String replacement doesn't respect data types. If a user provides a string for a column that ClickHouse expects to be a DateTime or UInt64, the query will fail with an unhelpful database error instead of a clear validation error at the API level.

Proposed Solution

We can use some SQL templating engine that separates the query logic from the user data. Some things to consider:

• Placeholders, not Replacement: The engine should identify {{ param }} tags and convert them into standard SQL placeholders (e.g., ? or :name) supported by the ClickHouse driver.
• Automatic Escaping: Instead of manually adding quotes, the driver should handle the sanitization of inputs based on the target column's type.
• Type-Safe Casting: The system should look at the ClickHouse schema (using the SchemaRegistry we already built) to ensure that a parameter passed as a string is correctly cast or rejected before the query is even sent to the database.

Alternatives Considered

fmt.Sprintf: No no immediately as it is the primary cause of SQL injection vulnerabilities.

Additional Context

The following is background gathered while reviewing the current implementation and comparing it to Tinybird's sql_template.py (the directly inspired feature). Recorded here as context only — not a decision on direction.

Current implementation summary (internal/pipes/pipes.go)

• Templating: regex \{\{(\w+)(?::([^}]*))?\}\}, supporting formal Parameters plus inline {{name:default}}.
• Escaping: hand-written in formatParamValue (pipes.go:195). Strings get \ → \\ then ' → '', wrapped in single quotes. Booleans → 1/0. Numbers via %d/%g. nil → NULL.
• ParamDef.Type ("string" | "number" | "boolean") is declared but not validated against supplied values; only Required is enforced at bind time.
• BindParams returns (sql, []any, error) but the []any is always nil — the clickhouse-go driver receives no bindings; all values are inlined into the SQL string.
• formatParamValue at pipes.go:202: any string whose contents parse as a float is emitted unquoted. e.g. {"page": "1"} against a String column renders page = 1, which CH rejects with a confusing error — this is exactly the "Type Mismatches" pain point described above.
• The comment at pipes.go:144 ("avoids driver-level positional parameter limitations (e.g. LIMIT position)") reflects v1 clickhouse-go positional-binding behavior. clickhouse-go v2 supports CH-native {name:Type} parameters which work in LIMIT and most other positions, so that original rationale is worth re-checking before relying on it.

For reference, the structured-query path (internal/query/builder.go) already uses ? positional placeholders and threads them through CHConn.Query(ctx, sql, params...) at api/query.go:115. The driver-side binding path is already wired up; the pipes module is the outlier.

Injection assessment of the current code

Based on a read of formatParamValue only — not a formal review:

• "1; DROP TABLE events" fails strconv.ParseFloat, falls into the string branch, gets quote-doubled → emitted as '1; DROP TABLE events' (a literal). Treated as a string, not executed.
• "O'Brien" → 'O''Brien'. Treated as a string.
• Backslash injection ("\\'; DROP --") — escaped first to \\\\, then quote-doubled. Treated as a string.
• NUL / \r / \n / etc. are not stripped, but clickhouse-go v2 handles framing at the wire layer.

So the SQL-injection case the issue cites (1; DROP TABLE events) appears to be defended against by the escaping today. The more material risks observed:

1. Type confusion: the ParseFloat branch silently changes the SQL shape based on the value's content, not the declared type. A String column gets an unquoted literal for any numeric-looking string.
2. No upfront validation: declared ParamDef.Type is not checked against supplied values. Every mismatch surfaces as a ClickHouse runtime error rather than an API-level 400.
3. Inline defaults are author-controlled (e.g. {{x:foo}}). A pipe author could write a malicious default, but pipe CRUD endpoints (PUT /v1/pipes/:name) are admin-only — within the existing trust boundary today.

Tinybird's approach (for reference)

tinybird/sql_template.py — ~3000 lines, a custom engine built on Tornado templates. Worth contrasting because Named Pipes are the directly inspired feature. Findings from the recon:

• Typed transformer wrappers are the actual safety net for values: {{Int32(foo)}}, {{String(foo)}}, {{Float32(foo)}}, {{DateTime(foo)}}, {{DateTime64(foo, '2020-09-09 10:10:10.000')}}, {{Boolean(foo)}}, {{Array(foo, 'Int32')}}, plus UInt8..UInt256, Int8..Int256, Date, JSON. Each one validates the input against a per-type tester and renders as to<Type>('value') — invalid input raises SQLTemplateException returned as a 400.
• Untyped {{foo}} is still escape-only in Tinybird — they inline the escaped value into the SQL string, same shape as WaveHouse today (Tinybird uses \' escapes via a translation table; WaveHouse uses '' quote-doubling — both valid ClickHouse escapes). Driver-level placeholders are not universal in Tinybird either.
• Identifier helpers: {{column(x)}}, {{columns(x, 'a,b,c')}}, {{symbol(x)}}, {{table(x)}} — backtick-escape via the same translation table that strips ", \0, \r, etc. Required for any identifier interpolation since identifiers are not bindable as parameters.
• Dynamic WHERE composition: {{sql_and(a=a, b__in=Array(b), c__gt=Int32(c))}} — operator suffixes (__in, __not_in, __gt, __gte, __lt, __lte, __eq, __neq). Keys whose values are None or Placeholder are dropped from the rendered WHERE.
• Control flow: full {% if %} / {% for %} blocks. Templates compile to Python and are exec_in'd, which forces an AST sandbox (tornado_template.SecurityException). The allowlist bans import, eval, globals, __builtins__, Pow/Div ops, method calls like .read(), attribute access on builtins, etc. They have shipped CVEs around this attack surface.
• CH-native {name:Type} parameters are used only for secrets — {{tb_secret('foo')}} renders as Symbol("{tb_secret_foo: String}") and the value flows over HTTP as a param_* parameter, never inlined into the SQL string. The set of bindings is captured in TemplateExecutionResults.ch_params and passed to the driver alongside the rendered SQL.
• Reserved param names: pipeline, q, token, format, query_id, playground, explain, debug, __tb__semver, tag, output_format_*, release_replacements, cli_version, template_parameters, ~15 total. Rejected as parameter names so user params can't clobber API-level control params. WaveHouse currently has no such list.
• Post-render parsing: rendered SQL goes through chtoolset.query (a native C++ binding to their CH fork) — chquery.format(sql), chquery.tables(sql), chquery.replace_tables(sql). Used for table-name rewriting (their workspace/database layer) and AST-level normalization. Not easily replicable without their ClickHouse fork or a separate Go-side CH SQL parser.

Gaps observed against what's in scope here

Mapping the issue's bullets back to what's present today:

• "Placeholders, not Replacement" — closest existing primitive in clickhouse-go v2 is the CH-native {name:Type} parameter syntax (supports LIMIT and most positions). Tinybird applies this pattern only to secrets; for user values they validate then inline. Either approach (driver-bound or typed-then-inline) is consistent with how Tinybird handles the same problem, and the choice carries different ergonomic and implementation tradeoffs.
• "Automatic Escaping" — the driver handles this only when values flow as parameters. The current formatParamValue hand-escaping is not obviously wrong for the SQL-injection cases listed, but the existence of the ParseFloat branch means escaping behavior depends on value content.
• "Type-Safe Casting via SchemaRegistry" — discovery.isTypeCompatible (in internal/discovery/validation.go) already does CH-type-aware value checking for ingest payloads. Reusing it from BindParams would be straightforward, but ParamDef.Type describes the parameter's declared type rather than a target column type — so the param → column mapping is not currently expressed in the pipe definition. Bridging that would require either annotation in the NamedQuery/ParamDef schema or some form of template inspection.

Observation noted in passing (probably its own issue)

api/pipes.go:87 — if role != "" { ... }. If auth.enabled is true but the JWT lacks the configured role_claim, RoleFromContext returns "" and the AllowedRoles check is skipped entirely. Compare to other policy enforcement that fails closed. Out of scope for #32; flagging here so it doesn't get lost.