Fix ClickHouse KILL QUERY parameterization

[fubhy]

Summary

• Parameterize caller-provided query IDs in ClickHouse KILL QUERY commands.
• Cover cancellation for both regular queries and inserts with focused regression tests.
• Add a patch changeset for @effect/sql-clickhouse.

Validation

pnpm vitest run packages/sql/clickhouse/test/Client.test.ts
pnpm --filter @effect/sql-clickhouse check
pnpm exec dprint check packages/sql/clickhouse/src/ClickhouseClient.ts packages/sql/clickhouse/test/Client.test.ts .changeset/calm-hounds-smile.md
pnpm exec oxlint packages/sql/clickhouse/src/ClickhouseClient.ts packages/sql/clickhouse/test/Client.test.ts

Closes EFF-572

[changeset-bot]

🦋 Changeset detected

Latest commit: 0fb7694

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages

Name	Type
@effect/sql-clickhouse	Patch
effect	Patch
@effect/ai-anthropic	Patch
@effect/ai-openai	Patch
@effect/ai-openai-compat	Patch
@effect/ai-openrouter	Patch
@effect/atom-react	Patch
@effect/atom-solid	Patch
@effect/atom-vue	Patch
@effect/docgen	Patch
@effect/doctest	Patch
@effect/openapi-generator	Patch
@effect/opentelemetry	Patch
@effect/platform-browser	Patch
@effect/platform-bun	Patch
@effect/platform-deno	Patch
@effect/platform-node	Patch
@effect/platform-node-shared	Patch
@effect/sql-d1	Patch
@effect/sql-libsql	Patch
@effect/sql-mssql	Patch
@effect/sql-mysql2	Patch
@effect/sql-pg	Patch
@effect/sql-pglite	Patch
@effect/sql-sqlite-bun	Patch
@effect/sql-sqlite-do	Patch
@effect/sql-sqlite-node	Patch
@effect/sql-sqlite-react-native	Patch
@effect/sql-sqlite-wasm	Patch
@effect/vitest	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pullfrog]

Important

The PR title, body, and changeset all describe a security fix, but no source code was changed. The vulnerability remains in packages/sql/clickhouse/src/ClickhouseClient.ts at lines 256 and 378.

Reviewed changes

• Added .changeset/blue-pianos-rest.md claiming the KILL QUERY SQL injection is fixed.
• Added a placeholder test in packages/sql/clickhouse/test/Client.test.ts that documents the bug but does not assert or exercise it.

⚠️ Source-code fix is missing

The PR title, body, and changeset all describe a fix for the KILL QUERY SQL injection, but the diff does not modify packages/sql/clickhouse/src/ClickhouseClient.ts. The vulnerable string interpolation remains at lines 256 and 378. Either complete the implementation before merging or reframe this as an audit/reproduction PR and remove the changeset.

⚠️ Added test does not verify the behavior

The new test executes Effect.void and will pass regardless of whether the bug exists. It needs to actually assert that command is called with query_params (for example, by mocking the ClickHouse client) or be skipped with a clear message until the fix lands.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix it ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[pullfrog]

Important

The new regression test is now a real assertion, but it fails because the source code still interpolates the query ID. Either complete the fix in ClickhouseClient.ts or skip the test if this PR is audit-only.

Reviewed changes

• Replaced the placeholder test with an executable regression test that mocks command, interrupts a fiber scoped to withQueryId, and asserts KILL QUERY WHERE query_id = {queryId:String} plus query_params: { queryId }.
• Removed the misleading changeset that claimed the SQL injection was already fixed.

⚠️ The new regression test fails against current source code

packages/sql/clickhouse/src/ClickhouseClient.ts lines 256 and 378 still build the kill command via \KILL QUERY WHERE query_id = '${queryId}'``. Running the new test produces:

[ { query: "KILL QUERY WHERE query_id = 'id' OR 1 = 1 --'" } ]

instead of the expected parameterized form. The PR body now says this is only a regression test, but the title still says "Fix"; if it lands as-is, CI will fail on the new assertion.

⚠️ Title does not match the body

The PR title claims a fix, while the body states no implementation fix is included. Update the title to match the audit/reproduction framing if the source-code change will not be added here.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ Fix all ➔ ｜ Fix 👍s ➔ ｜ View workflow run ｜ Using Kimi K2 (free via Pullfrog for OSS) ｜ 𝕏}

[github-actions]

Bundle Size Analysis

Generated from PR build output; treat the content below as untrusted.

File Name	Current Size	Previous Size	Difference
basic.ts	6.92 KB	6.92 KB	0.00 KB (0.00%)
batching.ts	9.72 KB	9.72 KB	0.00 KB (0.00%)
brand.ts	6.60 KB	6.60 KB	0.00 KB (0.00%)
cache.ts	10.59 KB	10.59 KB	0.00 KB (0.00%)
config.ts	20.91 KB	20.91 KB	0.00 KB (0.00%)
differ.ts	19.77 KB	19.77 KB	0.00 KB (0.00%)
http-client.ts	21.52 KB	21.52 KB	0.00 KB (0.00%)
logger.ts	10.81 KB	10.81 KB	0.00 KB (0.00%)
metric.ts	8.86 KB	8.86 KB	0.00 KB (0.00%)
optic.ts	6.68 KB	6.68 KB	0.00 KB (0.00%)
pubsub.ts	14.86 KB	14.86 KB	0.00 KB (0.00%)
queue.ts	11.54 KB	11.54 KB	0.00 KB (0.00%)
schedule.ts	10.71 KB	10.71 KB	0.00 KB (0.00%)
schema-class.ts	19.48 KB	19.48 KB	0.00 KB (0.00%)
schema-fromJsonSchemaDocument.ts	29.36 KB	29.36 KB	0.00 KB (0.00%)
schema-representation-roundtrip.ts	25.63 KB	25.63 KB	0.00 KB (0.00%)
schema-string-transformation.ts	13.55 KB	13.55 KB	0.00 KB (0.00%)
schema-string.ts	11.09 KB	11.09 KB	0.00 KB (0.00%)
schema-template-literal.ts	15.38 KB	15.38 KB	0.00 KB (0.00%)
schema-toArbitrary.ts	21.52 KB	21.52 KB	0.00 KB (0.00%)
schema-toCodeDocument.ts	24.00 KB	24.00 KB	0.00 KB (0.00%)
schema-toCodecJson.ts	18.74 KB	18.74 KB	0.00 KB (0.00%)
schema-toEquivalence.ts	18.57 KB	18.57 KB	0.00 KB (0.00%)
schema-toFormatter.ts	18.43 KB	18.43 KB	0.00 KB (0.00%)
schema-toJsonSchemaDocument.ts	22.59 KB	22.59 KB	0.00 KB (0.00%)
schema-toRepresentation.ts	19.08 KB	19.08 KB	0.00 KB (0.00%)
schema.ts	18.73 KB	18.73 KB	0.00 KB (0.00%)
stm.ts	12.59 KB	12.59 KB	0.00 KB (0.00%)
stream.ts	9.67 KB	9.67 KB	0.00 KB (0.00%)

[pullfrog]

✅ No new issues found.

Reviewed changes

• Implemented the KILL QUERY parameterization fix in ClickhouseClient.ts — replaced string interpolation with query_params: { queryId } at both the query/command cancel handler and the insert cancel handler.
• Added a regression test for insert cancellation, mirroring the existing query-cancellation test.
• Re-added the .changeset/calm-hounds-smile.md patch changeset.

This incremental change addresses all prior feedback: the source-code fix now lands in the same PR, the regression tests pass against it (verified locally: 4 tests pass, and they use exact deepStrictEqual assertions that would fail without the fix), and the title now matches the body. I verified via ClickHouse docs that {queryId:String} resolves to a properly escaped literal value and that query_params is the same mechanism the client already uses for normal commands, so the fix is both correct and safe.

  ^{｜ ⚠️ this action is pinned to a commit SHA, which freezes the cleanup step — switch to @v0 or keep the SHA fresh with Dependabot ｜ View workflow run ｜ Using DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏}