From adc7a9be312be32c34ebb667e930be13d6abaf3b Mon Sep 17 00:00:00 2001 From: Claude Opus 5 Date: Mon, 7 Sep 2026 02:19:10 +0200 Subject: [PATCH] fix(analytics): replace the control-character class with a code-point scan (no-control-regex) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ASCII-only check used a regex character class of control characters. It tripped eslint's no-control-regex, which the repo's quality gate enforces — and the rule has a point: such a class is unreadable and is exactly the kind of expression a reviewer nods at without checking. Same behaviour, written as an explicit scan naming the permitted set. Co-Authored-By: Claude Opus 5 --- evidence/bi-p017/analyst-sql-live-bypass.json | 10 +++++----- src/analytics/sql/statement-guard.ts | 20 +++++++++++++------ 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/evidence/bi-p017/analyst-sql-live-bypass.json b/evidence/bi-p017/analyst-sql-live-bypass.json index b3f5c30..bc233bb 100644 --- a/evidence/bi-p017/analyst-sql-live-bypass.json +++ b/evidence/bi-p017/analyst-sql-live-bypass.json @@ -2,7 +2,7 @@ "prompt": "BI-P017", "claim": "Every bypass attempt, executed directly against a live ClickHouse as the tenant's restricted user WITH THE STATEMENT GUARD REMOVED, fails to return a foreign-tenant row, an ungranted-table row, or server metadata.", "clickhouse_version": "24.8.14.39", - "recorded_at": "2026-09-07T00:15:15.942Z", + "recorded_at": "2026-09-07T00:19:07.198Z", "base_table_rows_both_tenants": 100, "tenant_view_rows_alpha": 50, "attempts_total": 64, @@ -607,7 +607,7 @@ "leaked_foreign_tenant_row": false, "leaked_ungranted_table_row": false, "leaked_server_metadata": false, - "guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: statement contains a non-ASCII character (U+0430) outside a string literal", + "guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: statement contains a disallowed character (U+0430) outside a string literal", "engine_response_head": "Code: 62. DB::Exception: Syntax error: failed at position 17 ('�'): аlpha_sql.sales . Unrecognized token: '�'. (SYNTAX_ERROR) (version 24.8.14.39 (official buil" }, { @@ -619,7 +619,7 @@ "leaked_foreign_tenant_row": false, "leaked_ungranted_table_row": false, "leaked_server_metadata": false, - "guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: statement contains a non-ASCII character (U+200b) outside a string literal", + "guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: statement contains a disallowed character (U+200B) outside a string literal", "engine_response_head": "Code: 62. DB::Exception: Syntax error: failed at position 24 ('.'): .tables . Expected one of: FINAL, SAMPLE, table, table function, subquery or list of joined" }, { @@ -631,7 +631,7 @@ "leaked_foreign_tenant_row": false, "leaked_ungranted_table_row": false, "leaked_server_metadata": false, - "guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: statement contains a non-ASCII character (U+0000) outside a string literal", + "guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: statement contains a disallowed character (U+0000) outside a string literal", "engine_response_head": "Code: 62. DB::Exception: Syntax error: failed at position 40 ('\u0000'): \u0000 UNION ALL SELECT name FROM system.tables . Unrecognized token: '\u0000'. (SYNTAX_ERROR) (versio" }, { @@ -656,7 +656,7 @@ "leaked_ungranted_table_row": false, "leaked_server_metadata": false, "guard_verdict_for_reference": "refused: [AUTHORIZATION_DENIED] 0pt.analytics.sql: \"SYSTEM\" is not permitted in a read-only analyst session", - "engine_response_head": "opt_analytics events f6d0a7de-8f53-4fe5-ad8f-7cff5fdd2005 MergeTree 0 ['/var/lib/clickhouse/store/f6d/f6d0a7de-8f53-4fe5-ad8f-7cff5fdd2005/'] /var/lib/clickhous" + "engine_response_head": "opt_analytics events 12706469-5605-436f-94ff-fc3c112aec6f MergeTree 0 ['/var/lib/clickhouse/store/127/12706469-5605-436f-94ff-fc3c112aec6f/'] /var/lib/clickhous" }, { "id": "describe-subquery", diff --git a/src/analytics/sql/statement-guard.ts b/src/analytics/sql/statement-guard.ts index 892bc5b..2255b8f 100644 --- a/src/analytics/sql/statement-guard.ts +++ b/src/analytics/sql/statement-guard.ts @@ -377,13 +377,21 @@ export function guardAnalystStatement(sql: string, opts: StatementGuardOptions): // Homoglyphs, zero-width joiners and RTL overrides stop being expressible // against the keyword scan. Values are unaffected: they live in literals, // which have already been removed at this point. - const nonAscii = /[^\x09\x0a\x0d\x20-\x7e]/.exec(stripped); - if (nonAscii) { - deny( - `statement contains a non-ASCII character (U+${nonAscii[0].codePointAt(0)?.toString(16).padStart(4, '0')}) outside a string literal`, - ); + // Written as a code-point scan rather than a character class: a regex of + // control characters is unreadable, trips `no-control-regex`, and is the + // kind of expression a reviewer nods at without checking. Permitted: tab, + // LF, CR and printable ASCII. Everything else, NUL included, is refused. + for (let i = 0; i < stripped.length; i += 1) { + const code = stripped.charCodeAt(i); + const printable = code >= 0x20 && code <= 0x7e; + const allowedWhitespace = code === 0x09 || code === 0x0a || code === 0x0d; + if (!printable && !allowedWhitespace) { + deny( + `statement contains a disallowed character (U+${code.toString(16).toUpperCase().padStart(4, '0')}) ` + + `outside a string literal`, + ); + } } - if (stripped.includes('\0')) deny('statement contains a NUL byte'); // ── exactly one statement ────────────────────────────────────────────── const withoutTrailingSemi = stripped.replace(/;\s*$/, '');