JITSU-27: fix(ingest): sanitize __sql_type hint values on s2s ingest - #1473
Conversation
Browser ingest already drops __sql_type* keys entirely (FilterEvent).
The s2s path kept passing them through verbatim, and hint values end up
in SQL DDL in bulker. Keep the feature for authenticated s2s callers but
allow only type-shaped values: a string (or 1-2 element string array)
matching ^[A-Za-z][A-Za-z0-9 _,()<>]{0,127}$ — no quotes, semicolons,
dashes or slashes, so a hint cannot break out of the type position.
Malformed hints are dropped; the event itself is never rejected.
JITSU-27 (s2s action item; bulker-side expression filter and per-driver
type whitelist remain)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
![jitsu-code-review[bot]](https://avatars.githubusercontent.com/in/3391383?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed the ingest/sql-hint sanitization changes in router.go, router_classic_handler.go, and jitsubase/types/json.go (+ tests). The direction looks good and tests pass locally for touched packages. I left one inline security concern about the current hint regex still permitting DDL breakout tokens in some cases.
…hints A separate ddlType is a wider DDL surface (the review bot noted it can carry a payload independently of the cast type) and has no known external user — only string hints pass s2s ingest now. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Replace the type-shape regex with a case-insensitive allowlist of bare object/string type names (json, jsonb, string, text, varchar, variant, object, super). The hint's only supported use at ingest is mapping a nested object to a JSON- or string-typed column; an allowlist leaves no grammar to abuse in generated DDL (no parameters, commas or expressions). Original casing is passed through for case-sensitive drivers like ClickHouse. Richer hints can still be produced by transformation functions downstream. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Reviewed bulker/ingest/router.go, bulker/ingest/router_classic_handler.go, and bulker/jitsubase/types/json* for SQL-hint sanitization, ingest-path behavior, and test coverage.
Security hardening direction looks good overall. I left one inline question about a potential silent compatibility regression when invalid hints are now dropped without surfacing an error/warning.
There was a problem hiding this comment.
Reviewed the diff in bulker/ingest/router.go, bulker/ingest/router_classic_handler.go, and bulker/jitsubase/types/json*.go with focus on security/correctness regressions around __sql_type* handling.
I also ran go test ./ingest/... ./jitsubase/types/... under bulker/ and both packages passed.
I did not find additional actionable issues beyond the already existing discussion threads.
1 participant
Summary
Implements the s2s action item of
JITSU-27(browser sanitization was already done; the Bulker-side defense-in-depth items moved toJITSU-179).The s2s ingest path passed
__sql_type*type hints through verbatim, and hint values end up interpolated into SQL DDL by bulker. The feature stays available to authenticated s2s callers — prodevents_logshows one active customer relying on\"__sql_type_\": \"JSON\"— but hints are now restricted at ingest to their supported use case, mapping a (nested) object to a JSON- or string-typed column:json,jsonb,string,text,varchar,variant,object,super. Original casing is passed through (ClickHouse types are case-sensitive, e.g.String).[castType, ddlType]array form accepted by bulkerlib'sextractSQLTypesHintsis not allowed through ingest.[]hint →val[0]index-out-of-range panic inprocessor.go).FilterEventstill drops all hints);filterEventis refactored to a shared predicate walk with identical browser semantics.patchEvent(modern/batch/pixel/funcs handlers — now a catch-allelseso it fails closed) and the classic handler.JITSU-179tracks bulker-side filtering/whitelisting as defense in depth for those paths.Tests
New unit tests in
jitsubase/types: sanitizer walk (real customer shape + injection payloads) and validator table (allowlist across casings; rejects for parameterized types, quotes/semicolons/comments, comma injection, arrays, non-strings).go test ./ingest/... ./jitsubase/types/...passes.🤖 Generated with Claude Code