Skip to content

Revisit disabling sqlparse grouping caps in the toolkit runtime #121

Description

@tkislan

Split out of #120 (sqlparse 0.6.0 bump) — this is a pre-existing design decision, not a regression introduced there.

What

deepnote_toolkit/sql/sql_utils.py:17-35 disables sqlparse's built-in grouping caps outright:

def configure_sqlparse_limits(
    max_grouping_tokens: Optional[int] = None,
    max_grouping_depth: Optional[int] = None,
) -> None:
    ...
    sqlparse.engine.grouping.MAX_GROUPING_TOKENS = max_grouping_tokens
    sqlparse.engine.grouping.MAX_GROUPING_DEPTH = max_grouping_depth

deepnote_toolkit/runtime_initialization.py:58 calls it with no arguments at startup, so the runtime ships with both caps set to None (sqlparse's defaults are 10_000 tokens / depth 100).

The stated rationale is sound as far as it goes:

Since the toolkit runtime is isolated and users write their own queries, we disable limits by default.

Why revisit

Two of the advisories fixed in #120 were specifically about the cost of reaching those caps (PYSEC-2026-3697, PYSEC-2026-3699). 0.6.0 makes the parser much faster — a 2 KB deeply-nested payload went from 2.7 s to 0.024 s, and 39 KB of comments from 9.3 s to 0.062 s — but with the caps at None there is still no ceiling at all, only a faster slope. A sufficiently large input can still pin a kernel core for as long as it takes.

The disabled-caps decision was made against sqlparse 0.5.4's cap behaviour (see the docstring reference). Now that 0.6.0 has changed the cost model underneath it, the tradeoff is worth re-deriving rather than inheriting.

Options

  1. Keep None. Defensible if the kernel is genuinely single-tenant and a user can only hurt themselves. Worth writing down explicitly as a threat-model decision, with a note that it was re-confirmed against 0.6.0.
  2. Set a high ceiling instead of None — e.g. MAX_GROUPING_TOKENS=1_000_000, MAX_GROUPING_DEPTH=10_000. Preserves the "large analytical queries must work" goal that motivated disabling them, while still bounding the pathological case. Needs a number derived from the largest queries actually seen in practice, not a guess.
  3. Re-enable defaults and handle SQLParseError gracefully. Most conservative; likely breaks legitimately large generated queries, which is exactly what the current code was written to avoid. tests/unit/test_sql_utils.py::test_large_query_fails_with_default_limits shows a 5000-column SELECT already trips the default cap, so this would be a real behaviour change.

Option 2 seems like the right shape, but picking the ceiling needs data on real query sizes that I don't have.

Context

  • Caps confirmed still present in sqlparse 0.6.0 at sqlparse/engine/grouping.py:15,20 with unchanged names, defaults and is not None semantics — configure_sqlparse_limits() keeps working either way.
  • Relevant tests: tests/unit/test_sql_utils.py::TestSqlparseLimits.
  • Advisories for background: PYSEC-2026-3697 (CVE-2026-71491), PYSEC-2026-3699 (CVE-2026-54284).

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions