dashboard: table-dependency graph (ported from ClickHouse /schema), DDL credential redaction - #34
Open
CamiloSierraH wants to merge 6 commits into
Open
CamiloSierraH wants to merge 6 commits into
CamiloSierraH wants to merge 6 commits into
Conversation
system.tables.create_table_query / engine_full / as_select and system.dictionaries.source carry engine arguments verbatim: an S3 secret key, a MySQL password, kafka_sasl_password in a SETTINGS clause. The config sanitizer never saw them — it is wired to the XML/YAML collector only, and query results were written to disk as the server returned them. Verified on 22.8.21.38: both the S3 secret and the MySQL password ship raw. Servers from 23.x mask the positional secrets as '[HIDDEN]' themselves, but leave AWS key ids and SETTINGS-clause passwords visible. Adds collection.RedactSQLText: a small parser that walks every known engine and table-function call (S3-family incl. the *Cluster variants, MySQL, PostgreSQL, MongoDB, remote(), Redis, Azure, ExternalDistributed; nesting and quoting respected, so a function inside an MV's SELECT is found too), masks the argument positions that carry secrets, then runs the byte-shape heuristics the config sanitizer already has — URL basic-auth, AWS key ids, JWTs, PEM blocks, keyword = 'value'. The replacement is '[HIDDEN]', the server's own token, so a 22.x bundle and a 26.x bundle read alike and every downstream reader needs one convention. Idempotent over already-masked text. The executor applies it to the JSONL of the collectors named in SensitiveCollectorFields, rewriting only the named string fields and only where a value changed — every other byte, including quoted 64-bit integers and the server's key order, is preserved. Native/TSV cannot be redacted field by field and are written as-is with a note in execution_log.txt; the ok entry records how many values were replaced. redactHeuristicsWith parameterises the sentinel so the config sanitizer keeps writing REMOVED; a test pins that. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…freshes system.columns gains is_in_primary_key / is_in_sorting_key in every mode (booleans, safe in gov). system.tables gains total_rows / total_bytes on every rung, and a new 26.6.1.0 rung adds target_database / target_table — the table an MV writes to, including the implicit .inner_id.<uuid> table of an MV declared with an ENGINE, which dependencies_* never names. Pinned to 26.6 by reading StorageSystemTables.cpp at v26.5.7.64-stable (absent) and v26.6.1.1193-stable (present). system.view_refreshes is new, at a 23.12.1.0 rung with no root file — the table arrived with refreshable MVs in 23.12, newer than every floor, so it is skipped rather than failing below that. A REFRESH EVERY view never appears in query_views_log; this is the only place its schedule, last success and last error live. Cloud reads every replica with hostName(); gov hashes database and view and ships has_exception instead of the text. Gov edges: dependencies_* / loading_dependencies_* used to be forbidden outright in queries.gov. They are the edges of the graph, and an array of hashed table names exposes no more than the hashed database / name next to it, so they are now collected as arrayMap(x -> hex(SHA256(concat(x, salt)))) and the guard in gov_leak_test moved from "never mentioned" to "must be hashed" — with target_database / target_table and view added to the must-hash list. loading_dependent_* stay forbidden; one direction of every edge is enough. Every file was run against 22.8.21.38 (roots) and 26.7.5.10 (roots and rungs) before committing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A new page, schema_graph.html, written next to dashboard.html: the graph of
tables, materialized views, dictionaries and Distributed tables with the
edges data flows along, nodes coloured by engine, a sidebar with keys,
columns, neighbours and the CREATE statement, search over table and column
names, drag, zoom and a database filter. Adapted from ClickHouse
programs/server/schema.html (Apache-2.0, attributed in the file header and
the page); every live query is replaced by JSON embedded at generation time
from the same five result sets — system.tables, columns, dictionaries,
view_refreshes — with system databases excluded. The heat map from
system.query_views_log is deferred to a follow-up.
Why a second file. The graph needs every column of every user table
embedded: ~750 KiB on a near-empty server, tens of MB on a service with
thousands of tables, which would slow dashboard.html for readers who never
open the graph. Tested the alternatives in headless Chrome from file://:
fetch() and XHR of a sibling file are blocked (opaque origin), an <iframe>
renders it, and assigning iframe.src on click is a navigation the browser
allows and does not request the file until then. So the dashboard gains a
Schema Graph tab whose frame gets its src on the first click. The parent
cannot script the frame across the file:// boundary and does not need to;
the two pages share the theme through localStorage, which file:// documents
in one folder do share (verified, and the toggle propagates live).
The Click UI token layer moves out of htmlTemplate into themeTokensCSS and
both pages are assembled from it, so they cannot drift apart in theme.
schema.html's own palette is remapped onto those tokens; only the engine
colours keep their values, in both dark scopes. The edge colour is --edge,
because --link is the dashboard's hyperlink token.
Payload: five safeQuery calls behind hasColumn("tables","target_table") and
hasTable("view_refreshes"), plain system.* in every mode (tables and columns
are shared across replicas; dictionaries would duplicate under
clusterAllReplicas). create_table_query, engine_full and dictionary source
pass through collection.RedactSQLText before embedding, so the sidebar can
never show more than the JSONL does. json.Marshal HTML-escapes <, > and &,
so a "</script>" inside a customer's DDL cannot terminate the inline script
— a test embeds one to prove it.
Verified against 26.7.5.10 with a schema holding every node kind (MergeTree
source, MV with TO target, MV with implicit .inner_id target, plain view,
dictionary, Distributed, refreshable MV): the tab appears, the frame has no
src until clicked, 22 nodes render with the right kinds, the sidebar opens,
the theme follows the dashboard, the standalone page shows a back link,
search dims non-matches; no non-file requests, no page errors. make
dashboard-preview now also writes bin/schema_graph_preview.html from a
fixture pipeline, reachable from the preview dashboard's Schema tab.
Docs: README (section 18, a Schema graph subsection, the credentials note,
version-table rows for view_refreshes and target_*), bundle-layout (tree,
§8 key, new §8b, the tables/columns rows, gov notes), file-guide (a
system.view_refreshes entry, system.tables and dashboard.html addenda) and
HC-7.6 for a refreshable MV that stopped refreshing.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Critical credential-redaction issues and multiple graph correctness and accessibility issues remain unresolved.
Review effort: Lite
Findings: 2
Open (6)
What changed in this PR
Adds an offline schema-dependency graph, expanded schema collectors, dashboard integration, and credential redaction for collected DDL.
Changes:
- Adds on-demand graph rendering, previews, shared theming, and metadata collection.
- Adds SQL/JSONL credential redaction and governance hashing updates.
- Updates documentation, queries, tests, and dashboard generation.
| File | Changes | Final review status |
|---|---|---|
skills/clickhouse-diagnostic/references/health-checks.md |
Documents refreshable-MV health checks. | — |
skills/clickhouse-diagnostic/references/file-guide.md |
Documents schema and refresh metadata. | — |
skills/clickhouse-diagnostic/references/bundle-layout.md |
Documents graph payloads and files. | — |
README.md |
Documents collectors, graph, and redaction. | — |
queries.onprem/system.tables.sql |
Extends table metadata collection. | — |
queries.onprem/system.columns.sql |
Adds key metadata collection. | — |
queries.onprem/26.6.1.0/system.tables.sql |
Adds materialized-view target fields. | — |
queries.onprem/25.4.1.0/system.tables.sql |
Updates versioned table collection. | — |
queries.onprem/24.2.1.0/system.tables.sql |
Updates versioned table collection. | — |
queries.onprem/23.12.1.0/system.view_refreshes.sql |
Adds refreshable-view collection. | — |
queries.gov/system.tables.sql |
Adds hashed dependency metadata. | — |
queries.gov/system.columns.sql |
Adds hashed key metadata. | — |
queries.gov/26.6.1.0/system.tables.sql |
Adds hashed MV target fields. | — |
queries.gov/24.2.1.0/system.tables.sql |
Updates versioned governance collection. | — |
queries.gov/23.12.1.0/system.view_refreshes.sql |
Adds governance refresh collection. | — |
queries.cloud/system.tables.sql |
Extends cloud table metadata collection. | — |
queries.cloud/system.columns.sql |
Adds cloud key metadata collection. | — |
queries.cloud/26.6.1.0/system.tables.sql |
Adds cloud MV target fields. | — |
queries.cloud/25.4.1.0/system.tables.sql |
Updates versioned cloud collection. | — |
queries.cloud/24.2.1.0/system.tables.sql |
Updates versioned cloud collection. | — |
queries.cloud/23.12.1.0/system.view_refreshes.sql |
Adds cloud refresh collection. | — |
Makefile |
Extends dashboard preview generation. | — |
internal/query/versioned_test.go |
Tests versioned collector coverage. | — |
internal/query/redact_hook_test.go |
Tests executor redaction wiring. | — |
internal/query/gov_leak_test.go |
Updates governance hashing checks. | — |
internal/query/executor.go |
Applies redaction before persistence. | — |
internal/dashboard/schema_graph.go |
Implements graph collection and rendering. | Moderate (4 votes): failed dictionaries are omitted; Moderate (1 vote): empty schemas are treated as failures; Moderate (1 vote): exceptions lack the collector’s length bound; Moderate (2 votes): cloud existence checks precede local-only queries; Moderate (2 votes): graph nodes lack keyboard interaction; Moderate (2 votes): dictionary nodes and source edges are not created. |
internal/dashboard/schema_graph_test.go |
Tests graph generation and embedding. | — |
internal/dashboard/keeper_preview_test.go |
Links graph data into previews. | — |
internal/dashboard/generator.go |
Generates and integrates the graph page. | Critical (1 vote): raw dictionary sources can still leak credentials in the dashboard payload. |
internal/dashboard/files_panel_test.go |
Verifies graph file exclusion. | — |
internal/collection/sqlredact.go |
Implements SQL and JSONL redaction. | Moderate (1 vote): unchanged function calls are unnecessarily reconstructed, breaking byte preservation. |
internal/collection/sqlredact_test.go |
Tests redaction behavior. | — |
internal/collection/heuristics.go |
Generalizes redaction heuristics. | Moderate (1 vote): [HIDDEN] is not idempotently recognized; Critical (1 vote): escaped SQL string literals can leave password content unredacted. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…s, cloud refreshes, config dictionaries, keyboard Six findings on #34, each verified before changing anything. Keyword redaction stopped at an escaped quote. The rule's value pattern is [^'"]+, so password = 'pa\'ss' matched 'pa\' and produced '[HIDDEN]'ss' — the tail of the password stayed on disk. The value's END is now found by scanning the literal with SQL escaping (\' and '' inside '…', \" inside "…") instead of by the regex; an unterminated literal falls back to the regex span. The config sanitizer's REMOVED path is unchanged and pinned. The dashboard's own Dictionaries panel embedded system.dictionaries.source and last_exception raw. Same class of leak as the JSONL one commit 1 fixed, missed because it lives in collect(), not in the graph. Both fields now go through the same redactor. The dictionaries query kept schema.html's WHERE status IN (LOADED, NOT_LOADED, LOADING). In a diagnostic bundle a FAILED dictionary is the node to look at first, so every status is read and the sidebar shows "Dictionary status" and "Dictionary error". The local test server proved the point unprompted: its dict_users had gone FAILED (auth) between runs, and the old filter would have hidden exactly that. Note the finding's claim that filtered dictionaries "disappear entirely" was not accurate — nodes come from system.tables, only the source text was missing — but the fix is right regardless. view_refreshes read the local table while hasTable probed every replica. Now read through g.sysTable (clusterAllReplicas in cloud) and folded to one row per view: argMax(status, status != 'RunningOnAnotherReplica') picks the replica that actually runs the refresh, max() of the timestamps, argMax of a non-empty exception. Verified on both the plain and the cluster form. Dictionaries declared in server config (XML) have database = '' and no system.tables row, so they had no node — true of the live page too. The page now adds a node for any dictionary absent from tables, under a "(config)" section, with its source in the sidebar; it has no dependency arrays, so it draws without edges. DDL dictionaries were never affected: they are in system.tables and rendered before. Nodes were clickable <div>s with no keyboard path. Each is now role=button, tabIndex=0, labelled, and Enter/Space select it; sidebar cross-references are real links; a focus ring is drawn for keyboard users. Driven in headless Chrome: Tab to a node, Enter opens the sidebar, Escape closes it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unresolved critical credential-redaction gaps and additional graph and test-coverage issues must be addressed.
Review effort: Lite
Findings: 2
Open (3)
Resolved since last review (6)
Comment on lines
+90
to
+93
| out := []int{3} | ||
| if len(args) >= 4 && !looksLikeFormat(args[3]) && !looksLikeCompression(args[3]) { | ||
| out = append(out, 4) // session token | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Fixed in fcac910 — agreed. The AWS-shape heuristic covers AKIA…/ASIA…, but an S3-compatible endpoint (MinIO, GCS HMAC GOOG1…, R2, OSS LTAI…) issues ids of any shape, and the id is half of the credential pair. The S3 rule now masks positions 2 and 3 structurally; session-token and format detection unchanged. Test added with a GOOG1… id.
Comment on lines
+71
to
+72
| if len(tables) == 0 { | ||
| return nil |
Collaborator
Author
There was a problem hiding this comment.
Fixed in fcac910 — correct, and it defeated the config-dictionary path I added in the previous round. The empty check now runs after both system.tables and system.dictionaries are read: tables alone, dictionaries alone, or both produce the page; only an empty pair does not. schemaGraphHasContent is tested for all three cases, and the summary counts a dictionaries-only payload honestly.
What the page shows and how to read it — node colours by engine, edges along the data flow (MV between its source and its target, dictionary to its source, Distributed to its shard table), what the sidebar holds, search and keyboard use — with a real capture from a 26.7 run. The existing subsection under Dashboard stays as the design record (why a second file, theme sharing, credential redaction); the two cross-reference each other. The capture is downscaled to 1600 px (456 KB): the README renders narrower than that anyway, and it is the repo's first tracked image. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…action, dictionaries-only graphs Three findings on the second pass, all consistency gaps rather than new classes of problem. The S3 rule masked the secret (argument 3) and left the key id (argument 2) to the AWS-shape heuristic. AKIA…/ASIA… ids were caught; an S3-compatible service — MinIO, GCS HMAC "GOOG1…", R2, OSS "LTAI…" — issues ids of any shape, and the id is the credential's identifier, not a public value. Both positions are now masked structurally, session-token and format handling unchanged; a test uses a GOOG1… id. system.dictionaries.last_exception was redacted for the dashboard panel and the graph in the previous round but not for the persisted JSONL — the very field the graph fixture shows quoting a password. Added to SensitiveCollectorFields, with a test that the JSONL path scrubs it. collectSchemaGraph returned before reading system.dictionaries when system.tables had no user rows, so a server whose only user objects are config-declared (XML) dictionaries — which have no system.tables row and whose nodes the page builds from the dictionaries result — got no page at all. The decision now comes after both reads: tables alone, dictionaries alone, or both draw; nothing draws only when both are empty. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
A Schema Graph tab in
dashboard.htmlthat opensschema_graph.html— the table-dependency graph from ClickHouse's built-in/schemapage (programs/server/schema.html, Apache-2.0, attributed), rendered entirely from data embedded at collection time. Tables, materialized views, refreshable MVs, dictionaries, Distributed tables and views, coloured by engine, with the edges data flows along; a sidebar with keys, columns, neighbours and theCREATEstatement; search over table and column names; drag, zoom, database filter. Light and dark follow the dashboard.Three commits, each reviewable alone:
system.view_refreshes.1. The leak this fixes on the way
create_table_query,engine_full,as_selectandsystem.dictionaries.sourcecarry engine arguments verbatim, and the config sanitizer was wired to the XML/YAML collector only — query results went to disk as returned. Verified on 22.8.21.38: the S3 secret key and the MySQL password ship raw in today'ssystem.tablesJSONL. Servers from 23.x mask the positional secrets as'[HIDDEN]'themselves but leave AWS key ids andSETTINGS … password = …visible.collection.RedactSQLTextparses every known engine / table-function call (S3-family and the*Clustervariants, MySQL, PostgreSQL, MongoDB,remote(), Redis, Azure, ExternalDistributed; nesting and quoting respected, so a function inside an MV'sSELECTis found), masks the credential positions, then runs the sanitizer's byte-shape heuristics. It writes the server's own[HIDDEN]token so old and new bundles read alike, and is idempotent over already-masked text. The executor applies it to the named string fields of the JSONL only where a value changed — every other byte, including quoted 64-bit integers and key order, is preserved.execution_log.txtrecords the count. Native/TSV can't be redacted field by field and say so.Before/after on 22.8:
2. Collectors
system.columns+ is_in_primary_key, is_in_sorting_key(all modes)system.tables+ total_rows, total_byteson every rung; new26.6.1.0/rung withtarget_database, target_tablesystem.view_refreshes23.12.1.0/rung, no root file (table newer than every floor)system.tablesarrayMaptarget_tableis the only source that names the implicit.inner_id.<uuid>target of an MV declared with anENGINE. Pinned to 26.6 by readingStorageSystemTables.cppatv26.5.7.64-stable(absent) andv26.6.1.1193-stable(present).dependencies_*/loading_dependencies_*move fromgov_leak_test'sforbiddenlist tomustBeHashed. An array of hashed table names exposes no more than the hasheddatabase/namebeside it, and it is what lets a gov analyst count MVs per source or follow a chain by hash.loading_dependent_*stay forbidden. If you disagree, dropping the four lines from the gov files reverts it without touching anything else.3. Why a second file, and how the tab works
The graph needs every column of every user table embedded: ~750 KiB on my near-empty server, tens of MB for a service with thousands of tables — that would slow
dashboard.htmlfor every reader who never opens the graph. So it's a sibling file, loaded on demand.Tested the alternatives in headless Chrome from
file://rather than assuming:fetch()/ XHR of a sibling file<iframe src>iframe.srcassigned on clicklocalStoragebetween the two documentsstorageeventThe theme tokens move out of
htmlTemplateintothemeTokensCSS; both pages are assembled from it.schema.html's palette is remapped onto those tokens; only the engine colours keep their values.Payload: five
safeQuerycalls behindhasColumn("tables","target_table")/hasTable("view_refreshes"), plainsystem.*in every mode (tables and columns are replica-shared; dictionaries would duplicate underclusterAllReplicas), DDL redacted before embedding.json.MarshalHTML-escapes<, so a</script>inside a customer's DDL can't terminate the inline script — a test embeds one.Deferred: the INSERT-pipeline heat map from
system.query_views_log. It's a consumer of #33's hourly collector and lands once that merges. Gov never generates a dashboard, so gov needs no HTML work.Verification
make test,go vet ./...,gofmtclean. New tests: 18 DDL shapes for the redactor, JSONL byte-preservation, the executor hook, template pins (offline, attributed, no--linkshadowing,innerHTMLonly static), on-demand loading, fixture embedding with</script>escaping, file-index exclusion.TO, MV with implicit target, view, dictionary, Distributed, refreshable MV): tab appears, framesrcisnullbefore the click, 22 nodes with correct kinds, sidebar showsCREATEand neighbours, theme follows the dashboard, standalone page shows the back link, search dims non-matches. Network blocked except the Chart.js CDN: zero other requests, zero page errors.dashboard.html232 KB,schema_graph.html85 KB.[HIDDEN], zero credential strings remain,execution_log.txtsays7 credential(s) replaced.make dashboard-previewwritesbin/schema_graph_preview.html(11 tables, all node kinds, a redacted S3 DDL), reachable from the preview dashboard's Schema tab.Merge note
This branch adds
HC-7.6(refreshable MV stopped refreshing) tohealth-checks.md; #33 addsHC-7.6–7.8forquery_views_log. Whichever merges second renumbers its row — one line.Review follow-up
Six findings on the first pass, all addressed in the fourth commit:
'pa\'ss'→'[HIDDEN]'ss'\','',\"); regressions addedsource/last_exceptionrawcollect()WHERE status IN (…)dropped FAILED dictionariessystem.tables)dict_usershad gone FAILED between runs and would have been hiddenview_refreshesread the local table whilehasTableprobed all replicasg.sysTable(...)+ fold to one row per view (argMax(status, status != 'RunningOnAnotherReplica')); verified on the cluster formsystem.tables(config)section, source in sidebar, no edgesrole=button,tabIndex=0,aria-label, Enter/Space, focus ring, focusable sidebar linksThe README gained a Schema graph section (before Dry-run mode) with this screenshot and a plain-language guide to reading the page; the design record stays under Dashboard → Schema graph.
Second pass (three more, all consistency gaps, fixed in the sixth commit): the S3 rule now masks the key id structurally as well as the secret (non-AWS-shaped ids — MinIO, GCS HMAC, R2, OSS — escaped the heuristic);
system.dictionaries.last_exceptionis redacted in the persisted JSONL, not only in the dashboard and graph; and a server whose only user objects are config-declared dictionaries now gets a graph — the empty check runs after bothsystem.tablesandsystem.dictionariesare read.Re-verified in headless Chrome against a fresh collection: Tab + Enter opens a node's sidebar, Escape closes it, dictionary nodes show their status, zero non-CDN requests, zero page errors.
🤖 Generated with Claude Code