From 2a03dcacb3475b3d3bded64cd6eceb403fbda28f Mon Sep 17 00:00:00 2001 From: Isolator acm Date: Fri, 19 Jun 2026 17:41:56 +0200 Subject: [PATCH 1/6] harden(deploy): add CSP + security headers, drop external Google Fonts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defense-in-depth fixes from the security review: 1. Self-contained fonts. Remove the Google Fonts s from build/template.html; the --ui/--mono CSS stacks already fall back to system fonts. The served page now makes zero third-party requests (privacy + air-gap + truly self-contained). 2. Security headers. deploy/http_handlers.xml now sends a strict CSP (default-src 'none'; connect-src 'self' ; frame-ancestors 'none'; base-uri 'none'; img-src data:; script/style 'unsafe-inline' since the bundle is inlined), plus nosniff and Referrer-Policy: no-referrer. connect-src is the real win — it bounds where the sessionStorage tokens can be sent if an XSS ever lands. install.sh resolves the issuer's OIDC discovery and rewrites connect-src to the real issuer + token-endpoint origins (fail-soft to the Google default if discovery is unreachable), writing the rendered file to dist/http_handlers.xml. New --dry-run flag renders config.json + http_handlers.xml and prints them with no ClickHouse contact. README + DEPLOYMENT docs updated. No src/ changes; 319 tests still pass. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef --- README.md | 40 +++++++++++++++++++++---- build/template.html | 3 -- deploy/http_handlers.xml | 11 +++++++ deploy/install.sh | 63 +++++++++++++++++++++++++++++++++------- docs/DEPLOYMENT.md | 13 +++++++-- 5 files changed, 110 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 9b8ea42..e9cf2b3 100644 --- a/README.md +++ b/README.md @@ -4,7 +4,8 @@ A zero-dependency, OAuth-gated **SQL browser for any ClickHouse cluster** — schema explorer, tabbed SQL editor with syntax highlighting, streaming results with table / JSON / chart views, saved queries, history, and shareable links. It ships as a **single self-contained HTML file served from ClickHouse itself** -(no Node server, no CDN, no runtime dependencies). +(no Node server, no CDN, no external fonts, no runtime dependencies) — the page +makes **zero third-party requests** and renders in the OS's native UI font. Refactored from a single-file SPA into a fully modular, test-first codebase held at **100% test coverage**. @@ -52,11 +53,13 @@ client_id) and the browser sends that as the bearer — so ClickHouse's `expected_audience` must be the **client_id**, not an API audience. Passing `--audience` switches to the **access_token** path. See `docs/CLICKHOUSE-OAUTH.md`. -The installer builds `dist/sql.html`, renders `config.json`, and uploads both -into ClickHouse `user_files/`. Then: +The installer builds `dist/sql.html`, renders `config.json`, renders +`dist/http_handlers.xml` (with the CSP `connect-src` filled in for your issuer — +see "Security headers" below), and uploads the SPA + config into ClickHouse +`user_files/`. Then: -1. Add `deploy/http_handlers.xml` to the server's `config.d/` (or push it as an - ACM cluster setting `config.d/sql-browser.xml`) and reload ClickHouse. +1. Add the rendered `dist/http_handlers.xml` to the server's `config.d/` (or push + it as an ACM cluster setting `config.d/sql-browser.xml`) and reload ClickHouse. 2. Register the redirect URI `https:///sql` with your OAuth IdP. 3. Make sure ClickHouse accepts the bearer JWT — either a CH `` entry validating your IdP's JWKS, or a delegated @@ -85,6 +88,33 @@ on your IdP and threat model. Common, all valid, variants: The code treats `client_secret` as optional, so any of these is a config-only choice. +### Security headers + +`deploy/http_handlers.xml` sends a strict **Content-Security-Policy** plus +`X-Content-Type-Options: nosniff` and `Referrer-Policy: no-referrer` on the SPA +response. The CSP is `default-src 'none'` with everything re-allowed explicitly: + +- `script-src`/`style-src 'unsafe-inline'` — the JS and CSS are inlined into the + single HTML file, so they can't be matched by `'self'`. (No `eval`, no remote + scripts; the real protection below is `connect-src`.) +- `connect-src 'self' ` — the one that matters: it bounds where + the page can send data, so an injected script can't exfiltrate the + `sessionStorage` tokens to an attacker. `'self'` covers ClickHouse queries + + `config.json`; the IdP origins cover OIDC discovery and the token endpoint. +- `img-src data:`, `frame-ancestors 'none'` (anti-clickjacking), `base-uri 'none'`. + +`install.sh` fills `connect-src` automatically: it fetches your issuer's OIDC +discovery document and rewrites the host list to your real issuer + token-endpoint +origins (falling back to the Google default if discovery is unreachable). For a +**manual install with a non-Google IdP**, edit the `connect-src` line in +`deploy/http_handlers.xml` to list your issuer + token-endpoint origins. + +Preview the rendered artifacts without touching ClickHouse: + +```bash +./deploy/install.sh --dry-run --client-id [--issuer https://your-idp] +``` + ## Layout ``` diff --git a/build/template.html b/build/template.html index 90bb87b..1e3b93b 100644 --- a/build/template.html +++ b/build/template.html @@ -5,9 +5,6 @@ Altinity SQL Browser - - - diff --git a/deploy/http_handlers.xml b/deploy/http_handlers.xml index a20c71b..d94b802 100644 --- a/deploy/http_handlers.xml +++ b/deploy/http_handlers.xml @@ -30,6 +30,15 @@ text/html; charset=UTF-8 no-store + + default-src 'none'; script-src 'unsafe-inline'; style-src 'unsafe-inline'; img-src data:; font-src 'self'; connect-src 'self' https://accounts.google.com https://oauth2.googleapis.com; base-uri 'none'; frame-ancestors 'none' + nosniff + no-referrer file://sql.html @@ -43,6 +52,8 @@ application/json; charset=UTF-8 no-store + nosniff + no-referrer file://sql-config.json diff --git a/deploy/install.sh b/deploy/install.sh index 2ed38b2..2d50e8a 100755 --- a/deploy/install.sh +++ b/deploy/install.sh @@ -2,8 +2,9 @@ # Install the Altinity SQL Browser onto a ClickHouse cluster: # 1. build the single-file SPA (dist/sql.html) # 2. render config.json from the OAuth args -# 3. upload both into ClickHouse user_files/ (sql.html, sql-config.json) -# 4. print the http_handlers config to enable /sql +# 3. render dist/http_handlers.xml (CSP connect-src filled from OIDC discovery) +# 4. upload the SPA + config into ClickHouse user_files/ (sql.html, sql-config.json) +# 5. print the next step to enable /sql with the rendered http_handlers.xml # # The password is read from the CLICKHOUSE_PASSWORD env var or prompted — never # passed on the command line (it would leak via `ps`/shell history). @@ -17,13 +18,14 @@ # [--audience ] \ # audience-gated CH → also sends access_token # [--ch-auth basic] \ # OSS CH + ch-jwt-verify → JWT as Basic password # [--cluster my_cluster] \ # single-shard multi-replica only -# [--secure] +# [--secure] \ +# [--dry-run] # build + render config.json + http_handlers.xml, print, no CH contact set -euo pipefail ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" CH_HOST="" CH_USER="default" ISSUER="https://accounts.google.com" -CLIENT_ID="" AUDIENCE="" CLUSTER="" SECURE=0 CH_AUTH="" +CLIENT_ID="" AUDIENCE="" CLUSTER="" SECURE=0 CH_AUTH="" DRY_RUN=0 while [[ $# -gt 0 ]]; do case "$1" in --ch-host) CH_HOST="$2"; shift 2 ;; @@ -34,14 +36,16 @@ while [[ $# -gt 0 ]]; do --ch-auth) CH_AUTH="$2"; shift 2 ;; --cluster) CLUSTER="$2"; shift 2 ;; --secure) SECURE=1; shift ;; + --dry-run) DRY_RUN=1; shift ;; # build + render config.json + http_handlers.xml, print, no ClickHouse contact *) echo "unknown arg: $1" >&2; exit 2 ;; esac done -[[ -n "$CH_HOST" ]] || { echo "--ch-host is required" >&2; exit 2; } [[ -n "$CLIENT_ID" ]] || { echo "--client-id is required" >&2; exit 2; } +# --ch-host is only needed to reach ClickHouse; a --dry-run just renders artifacts. +[[ -n "$CH_HOST" || "$DRY_RUN" == 1 ]] || { echo "--ch-host is required" >&2; exit 2; } -if [[ -z "${CLICKHOUSE_PASSWORD:-}" ]]; then +if [[ "$DRY_RUN" != 1 && -z "${CLICKHOUSE_PASSWORD:-}" ]]; then read -r -s -p "ClickHouse password for $CH_USER@$CH_HOST: " CLICKHOUSE_PASSWORD echo fi @@ -52,7 +56,7 @@ CH=(clickhouse-client --host "$CH_HOST" --user "$CH_USER") # user_files is node-local, and clusterAllReplicas cannot write to a multi-shard # Distributed target, so a --cluster install only works on a single shard. -if [[ -n "$CLUSTER" ]]; then +if [[ "$DRY_RUN" != 1 && -n "$CLUSTER" ]]; then SHARDS=$("${CH[@]}" --query "SELECT max(shard_num) FROM system.clusters WHERE cluster = '${CLUSTER}'" 2>/dev/null || true) if [[ "$SHARDS" =~ ^[0-9]+$ ]] && (( SHARDS > 1 )); then echo "ERROR: cluster '${CLUSTER}' has ${SHARDS} shards. clusterAllReplicas can't" >&2 @@ -84,6 +88,44 @@ CONFIG_FILE="$(mktemp)" trap 'rm -f "$CONFIG_FILE"' EXIT printf '%s\n' "$CONFIG_JSON" > "$CONFIG_FILE" +echo "==> Rendering http_handlers.xml (CSP connect-src from OIDC discovery)" +# The CSP connect-src must allow same-origin ('self', for ClickHouse + config.json) +# plus the IdP origins the browser fetches: OIDC discovery (issuer origin) and the +# token endpoint (exchange + refresh). The OAuth /authorize step is a top-level +# navigation, not a fetch, so it needs no connect-src entry. Resolve the real +# origins from the issuer's discovery document; fail soft to the Google default. +ISSUER_ORIGIN="$(printf '%s' "$ISSUER" | grep -oiE '^https?://[^/]+' || true)" +CONNECT_HOSTS="https://accounts.google.com https://oauth2.googleapis.com" +DISC_URL="${ISSUER%/}/.well-known/openid-configuration" +if DISC_JSON="$(curl -fsS --max-time 10 "$DISC_URL" 2>/dev/null)"; then + # Pull the origin (scheme://host[:port]) of token/authorization endpoints, add + # the issuer origin, dedupe. Tolerates whitespace variations in the JSON. + EP_ORIGINS="$(printf '%s' "$DISC_JSON" \ + | grep -oE '"(token_endpoint|authorization_endpoint)"[[:space:]]*:[[:space:]]*"[^"]+"' \ + | grep -oiE 'https?://[^/"]+' || true)" + CONNECT_HOSTS="$(printf '%s\n%s\n' "$ISSUER_ORIGIN" "$EP_ORIGINS" \ + | sed '/^$/d' | sort -u | paste -sd' ' -)" + echo " connect-src origins: $CONNECT_HOSTS" +else + echo "WARN: could not fetch $DISC_URL — using the Google default connect-src." >&2 + echo " If your IdP is not Google, edit connect-src in dist/http_handlers.xml." >&2 +fi +# Rewrite only the connect-src host list in the committed template; everything else +# (the rest of the CSP, the other headers) is copied verbatim. +HANDLERS_OUT="$ROOT/dist/http_handlers.xml" +sed -E "s#(connect-src 'self')[^;]*#\1 ${CONNECT_HOSTS}#" \ + "$ROOT/deploy/http_handlers.xml" > "$HANDLERS_OUT" + +if [[ "$DRY_RUN" == 1 ]]; then + echo + echo "==> DRY RUN — no ClickHouse contact. Rendered artifacts:" + echo "--- config.json ---" + cat "$CONFIG_FILE" + echo "--- dist/http_handlers.xml ---" + cat "$HANDLERS_OUT" + exit 0 +fi + # Upload raw bytes via FORMAT RawBLOB on stdin — no base64, no command-line # length limit, written as the clickhouse process so perms are correct. upload() { # upload @@ -113,9 +155,10 @@ cat < Assets uploaded to ClickHouse user_files/. -Final step — enable the HTTP routes. Add deploy/http_handlers.xml to the -server config.d/ (or push it as an ACM cluster setting named -"config.d/sql-browser.xml") and reload ClickHouse. Then open: +Final step — enable the HTTP routes. Add the rendered dist/http_handlers.xml +(its CSP connect-src is filled in for your issuer) to the server config.d/ (or +push it as an ACM cluster setting named "config.d/sql-browser.xml") and reload +ClickHouse. Then open: http${SECURE:+s}://$CH_HOST/sql diff --git a/docs/DEPLOYMENT.md b/docs/DEPLOYMENT.md index eb292fa..5f7d1e5 100644 --- a/docs/DEPLOYMENT.md +++ b/docs/DEPLOYMENT.md @@ -29,10 +29,19 @@ env var or prompted — never placed on the command line. ## 3. HTTP routes -Add `deploy/http_handlers.xml` to ClickHouse `config.d/` (or push it through +Add the http_handlers fragment to ClickHouse `config.d/` (or push it through your control plane as `config.d/sql-browser.xml`) and reload. It adds static rules for `/sql` and `/sql/config.json` and keeps `` so the dynamic -query handler at `/` still works. +query handler at `/` still works. The SPA rule also sends a strict +Content-Security-Policy (`default-src 'none'`, `frame-ancestors 'none'`, and a +`connect-src` scoped to same-origin + your IdP) plus `nosniff` and +`Referrer-Policy: no-referrer` — see README "Security headers". + +`deploy/http_handlers.xml` is the committed default (Google `connect-src`). +`install.sh` renders `dist/http_handlers.xml` with `connect-src` filled in for +your `--issuer`; deploy that rendered file. For a manual install with a +non-Google IdP, edit the `connect-src` line to your issuer + token-endpoint +origins. ## 4. Make ClickHouse accept the JWT From 65a6d08f0c523080f3a2809f689d6bb5f4c94b96 Mon Sep 17 00:00:00 2001 From: Isolator acm Date: Fri, 19 Jun 2026 17:55:53 +0200 Subject: [PATCH 2/6] harden: escape backslashes in sqlString, surface OAuth errors, clear pkce transients MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security-review follow-ups (#4/#6/#7): - sqlString (core/format.js): escape `\` -> `\\` before doubling `'`. CH honors backslash escapes in string literals, so a value ending in `\` escaped the closing quote and could break out (second-order via loadColumns' system.tables names). Now closed. - bootstrap (main.js): handle an IdP error redirect (?error=access_denied&…) instead of dropping silently to the login screen — surfaces error_description||error, and the URL cleanup now strips the error params too. - setTokens (ui/app.js): clear the one-shot oauth_verifier + oauth_state from sessionStorage once tokens are held (refresh path is a harmless no-op). Tests added in the same change; 323 pass, per-file coverage gate holds. Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01QennTvGKAtJZrv9EpQagef --- src/core/format.js | 5 ++++- src/main.js | 12 ++++++++++-- src/ui/app.js | 4 ++++ tests/unit/app.test.js | 9 +++++++++ tests/unit/format.test.js | 5 +++++ tests/unit/main.test.js | 20 ++++++++++++++++++++ 6 files changed, 52 insertions(+), 3 deletions(-) diff --git a/src/core/format.js b/src/core/format.js index cc7f040..32bf5f5 100644 --- a/src/core/format.js +++ b/src/core/format.js @@ -44,7 +44,10 @@ export function timeAgo(ts, now = Date.now()) { /** Quote + escape a string as a ClickHouse SQL string literal. */ export function sqlString(s) { - return "'" + String(s).replace(/'/g, "''") + "'"; + // Escape the backslash first (CH honors backslash escapes in string literals, + // so a trailing `\` would otherwise escape the closing quote and break out), + // then double the single quote. + return "'" + String(s).replace(/\\/g, '\\\\').replace(/'/g, "''") + "'"; } /** diff --git a/src/main.js b/src/main.js index a13c047..cba7f2b 100644 --- a/src/main.js +++ b/src/main.js @@ -16,9 +16,14 @@ export async function bootstrap(app, env) { const u = new URL(loc.href); const code = u.searchParams.get('code'); const stateParam = u.searchParams.get('state'); + const errorParam = u.searchParams.get('error'); let callbackError = null; - if (code && stateParam) { + if (errorParam) { + // The IdP bounced back with an error (e.g. ?error=access_denied) instead of + // a code — surface it rather than dropping silently onto the login screen. + callbackError = 'Sign-in failed: ' + (u.searchParams.get('error_description') || errorParam); + } else if (code && stateParam) { if (stateParam !== ss.getItem('oauth_state')) { callbackError = 'OAuth state mismatch — please try again.'; } else { @@ -36,7 +41,10 @@ export async function bootstrap(app, env) { callbackError = 'OAuth token exchange failed: ' + ((e && e.message) || e); } } - ['code', 'state', 'scope', 'authuser', 'prompt'].forEach((k) => u.searchParams.delete(k)); + } + if (errorParam || (code && stateParam)) { + ['code', 'state', 'scope', 'authuser', 'prompt', 'error', 'error_description', 'error_uri'] + .forEach((k) => u.searchParams.delete(k)); const qs = u.searchParams.toString(); hist.replaceState(null, '', loc.origin + loc.pathname + (qs ? '?' + qs : '') + loc.hash); } diff --git a/src/ui/app.js b/src/ui/app.js index 699d334..bc1f79c 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -67,6 +67,10 @@ export function createApp(env = {}) { app.refreshToken = refresh; ss.setItem('oauth_refresh_token', refresh); } + // The PKCE verifier + CSRF state are one-shot — done with them once we hold + // tokens. (The refresh path also lands here; they're already gone → no-op.) + ss.removeItem('oauth_verifier'); + ss.removeItem('oauth_state'); } function clearTokens() { app.token = null; diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index dcfbcc3..9912916 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -110,6 +110,15 @@ describe('renderApp shell', () => { expect(e.sessionStorage.getItem('oauth_id_token')).toBeNull(); expect(app.root.querySelector('.login-screen')).not.toBeNull(); }); + it('setTokens clears the one-shot pkce verifier and csrf state', () => { + const e = env({ sessionStorage: memSession({ oauth_verifier: 'v', oauth_state: 's' }) }); + const app = createApp(e); + app.setTokens('tok'); + expect(app.token).toBe('tok'); + expect(e.sessionStorage.getItem('oauth_id_token')).toBe('tok'); + expect(e.sessionStorage.getItem('oauth_verifier')).toBeNull(); + expect(e.sessionStorage.getItem('oauth_state')).toBeNull(); + }); it('changing the format select persists the choice', () => { const { app } = rendered(); app.dom.fmtSelect.value = 'JSON'; diff --git a/tests/unit/format.test.js b/tests/unit/format.test.js index 7c85100..365af41 100644 --- a/tests/unit/format.test.js +++ b/tests/unit/format.test.js @@ -62,6 +62,11 @@ describe('sqlString', () => { expect(sqlString("a'b")).toBe("'a''b'"); expect(sqlString(42)).toBe("'42'"); }); + it('escapes backslashes so a trailing one cannot break out of the literal', () => { + expect(sqlString('a\\b')).toBe("'a\\\\b'"); + expect(sqlString('x\\')).toBe("'x\\\\'"); + expect(sqlString("\\'")).toBe("'\\\\'''"); + }); }); describe('inferQueryName', () => { diff --git a/tests/unit/main.test.js b/tests/unit/main.test.js index 3fbffc7..01fae81 100644 --- a/tests/unit/main.test.js +++ b/tests/unit/main.test.js @@ -68,6 +68,26 @@ describe('bootstrap', () => { expect(app.showLogin).toHaveBeenCalledWith('OAuth state mismatch — please try again.'); }); + it('surfaces an IdP error callback with its description', async () => { + const app = fakeApp(); + const env = fakeEnv({ + location: { href: 'https://ch/sql?error=access_denied&error_description=User+denied', origin: 'https://ch', pathname: '/sql', search: '?error=access_denied&error_description=User+denied', hash: '' }, + }); + await bootstrap(app, env); + expect(app.showLogin).toHaveBeenCalledWith('Sign-in failed: User denied'); + expect(env.history.replaceState).toHaveBeenCalled(); + expect(app.renderApp).not.toHaveBeenCalled(); + }); + + it('falls back to the error code when no description is given', async () => { + const app = fakeApp(); + const env = fakeEnv({ + location: { href: 'https://ch/sql?error=access_denied', origin: 'https://ch', pathname: '/sql', search: '?error=access_denied', hash: '' }, + }); + await bootstrap(app, env); + expect(app.showLogin).toHaveBeenCalledWith('Sign-in failed: access_denied'); + }); + it('reports a token-exchange failure', async () => { const app = fakeApp(); const env = fakeEnv({ From 8680ec32f52bc594d810cc516de43d7d4e4abe0a Mon Sep 17 00:00:00 2001 From: Isolator acm Date: Fri, 19 Jun 2026 18:05:09 +0200 Subject: [PATCH 3/6] =?UTF-8?q?fix(editor):=20align=20selection=20highligh?= =?UTF-8?q?t=20=E2=80=94=20integer=20line-height?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The editor overlays a