Add SHOW COLUMNS access check to hasColumnInTable

[groeneai]

Related: #110881

Changelog category (leave one):

• Critical Bug Fix (crash, data loss, RBAC)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

The hasColumnInTable function now requires the SHOW COLUMNS privilege on the target table, the same grant required by DESCRIBE and SHOW CREATE TABLE. Previously any user could call hasColumnInTable to probe column names and test table and database existence without any access check.

Description

hasColumnInTable read table column metadata without an access check. A user with no privileges on a table (for example one granted only SELECT ON system.one) could still enumerate its column names (the function returns 1/0), probe table existence (UNKNOWN_TABLE vs 0), and probe database existence. DESCRIBE and SHOW CREATE TABLE on the same table are gated by SHOW COLUMNS; this function was a bypass.

Rebased onto master after #110881 removed the remote-server overload, so only the local hasColumnInTable(database, table, column) form remains and is the only path that needs this gate.

Why the naive fix does not work

create() stored the global context (context_->getGlobalContext()), which has full access by construction (it never has a user id set), so a checkAccess on it would always pass. The fix keeps the query context, which carries the caller's rights.

Fix

create() now retains the query context. executeImpl checks SHOW_COLUMNS on the raw database and table names before the table is resolved, so denial does not depend on the table or database existing (closing the existence oracle). This mirrors InterpreterDescribeQuery (checkAccess(AccessType::SHOW_COLUMNS, table_id)).

Testing

New stateless test 04401_has_column_in_table_access_check: a user with only SELECT ON system.one gets ACCESS_DENIED from hasColumnInTable for an existing table, a non-existing table, and a non-existing database (existence-independent), then succeeds after GRANT SHOW COLUMNS. Verified locally that an unprivileged user got the column/existence result before this change (bypass) and gets ACCESS_DENIED after (50/50 runs). The existing test 00386_has_column_in_table is unchanged and still passes (it runs as a full-access user).

Version info

• Merged into: 26.7.1.1206 (included in 26.7 and later)

[groeneai]

Pre-PR validation gate (click to expand)

#	Question	Answer
a	Deterministic repro?	Yes. As a user with only GRANT SELECT ON system.one: SELECT hasColumnInTable('db','table','col') returns 1 (or leaks UNKNOWN_TABLE/UNKNOWN_DATABASE) on master, every time.
b	Root cause explained?	create() downgrades to getGlobalContext(), which has full access by construction (no user id), and executeImpl read column metadata via getTable() with no access check. So column existence, table existence, and database existence were disclosed to any user, unlike the SHOW_COLUMNS-gated DESCRIBE/SHOW CREATE TABLE.
c	Fix matches root cause?	Yes. Retain the query context (caller's rights) separately from the global context, and call query_context->checkAccess(AccessType::SHOW_COLUMNS, database, table) on the raw names before resolving the table. Not a band-aid: it adds the missing authorization at the exact gap.
d	Test intent preserved / new tests added?	New stateless test 04401_has_column_in_table_access_check asserts ACCESS_DENIED without the grant (existing table / non-existing table / non-existing db) and success after GRANT SHOW COLUMNS. Existing 00386_has_column_in_table is unchanged and still passes (runs as full-access user → 1 1 1 1 0 0 0 0).
e	Both directions demonstrated?	Yes. Unpatched binary (Build ID 2e34eb…): bypass returns 1 and leaks UNKNOWN_TABLE. Patched binary (Build ID 1854cf…): ACCESS_DENIED (Code 497, "grant SHOW COLUMNS ON db.table") for existing/non-existing-table/non-existing-db; GRANT SHOW COLUMNS restores 1/0.
f	Fix is general across code paths?	Yes. hasColumnInTable was the only column-enumeration function in src/Functions/ reaching getTable/getStructureOfRemoteTable without an access check; the sibling FunctionJoinGet already does checkAccess(SELECT, …). The remote (hostname) path is intentionally unchanged because the remote server enforces its own access.
g	Fix generalizes across inputs?	Check runs on the raw argument names before any type/lookup work, so it fires identically for existing/non-existing tables and databases, for system tables, and for every column-name input. No type-wrapper or value dependence.
h	Backward compatible?	No setting default, on-disk/wire format, or replication metadata changes. This adds an authorization check that was missing; the only behavior change is that an unprivileged caller now gets ACCESS_DENIED instead of a silent answer (the intended fix). No SettingsChangesHistory entry needed.
i	Invariants and contracts preserved?	Yes. The global context is still used for storage initialization (unchanged lifetime/behavior of getTable); only an additional read-only access check on the query context is introduced, before table resolution. No locking, block-structure, or ownership invariants are touched.

Session id: cron:clickhouse-worker-slot-17:20260625-113500

[groeneai]

cc @pufit @vitlibar — could you review this? hasColumnInTable read column metadata with no access check (it stored the full-access global context), letting any user enumerate columns and probe table/database existence. The fix retains the query context and checks SHOW_COLUMNS on the raw names before resolving the table, matching DESCRIBE/SHOW CREATE TABLE and closing the existence oracle.

[clickhouse-gh]

Workflow [PR], commit [0221328]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR closes the hasColumnInTable RBAC bypass by retaining the caller context, checking SHOW COLUMNS before resolving the table, and adding a focused stateless regression test. I do not see a remaining correctness issue in the current diff; the only outstanding item is that the PR template still understates the severity of an RBAC fix.

PR Metadata

The chosen Changelog category is not correct for the actual change. This PR fixes an RBAC / information-disclosure bug, so the category should be Critical Bug Fix (crash, data loss, RBAC) rather than Bug Fix (user-visible misbehavior in an official stable release).

The Changelog entry is required for both categories, and the current entry is specific, user-readable, and explains the user impact clearly. No change is needed there.

Exact replacement:
- Critical Bug Fix (crash, data loss, RBAC)

Findings

💡 Nits

• [dismissed by author -- https://github.com/ClickHouse/ClickHouse/pull/108464#discussion_r3537229452] The PR template category still mismatches the contract of the change. This is explicitly an RBAC bypass fix, so leaving it as Bug Fix understates a security-sensitive privilege-check correction.

Final Verdict

• Status: ✅ Approve
• The access-check fix and regression test look correct on the current head. The only follow-up I would still make is switching the Changelog category to Critical Bug Fix (crash, data loss, RBAC).

[groeneai]

Pushed a style fix (force-update to 7aa0d8d): the Style check flagged "Avoid holding a copy of ContextPtr in Functions". Dropped the separate `query_context` member and now store the query context in `WithContext` directly (read via `getContext()` for the `SHOW_COLUMNS` check), matching `InterpreterDescribeQuery`. Access semantics unchanged; re-verified both directions (bypass to ACCESS_DENIED, GRANT SHOW COLUMNS restores 1/0, existence oracle closed) and the test still matches the reference.

[groeneai]

CI finish ledger — 803717f

Every failure below has an owner: a fixing PR (ours or external). The PR's own diff (src/Functions/hasColumnInTable.cpp SHOW_COLUMNS access check + a stateless RBAC test) cannot cause any of these; none are PR-caused.

Check / test	Reason	Owner / fixing PR
Integration tests (amd_asan_ubsan, db disk, old analyzer, 4/6) / test_s3_plain_rewritable test[cache_s3_plain_rewritable-data/]	REPLACE PARTITION from empty source now throws (regression from merged #104939)	#108350 (merged 2026-06-25 21:26Z; this build predates the fix)
Integration tests (arm_binary, distributed plan, 2/4) / test_keeper_disks test_snapshots_with_disks	flaky (local_snapshot_files count race)	#108513 (ours, open)
Stress test (arm_debug) / Hung check failed, possible deadlock found	deadlock (chronic leaked-local-ThreadPool / GlobalThreadPool shutdown family)	#101680 / #105905 (ours, open)

Session id: cron:our-pr-ci-monitor:20260625-213000

[Ergus]

At minimum, please confirm behavior of hasColumnInTable('', db, tbl, col) as an unprivileged user in a default single-node setup.

[groeneai]

Confirmed and fixed (option 2, gate the remote path).

Reproduced on a single node as an unprivileged user (only GRANT SELECT ON system.one), server listening on 0.0.0.0, table d1.secret(id, password):

hasColumnInTable('d1','secret','password')             -> ACCESS_DENIED   (local, gated)
hasColumnInTable('localhost','d1','secret','password') -> ACCESS_DENIED   (local shard, gated)
hasColumnInTable('127.0.0.2','d1','secret','password') -> 1               <-- bypass
hasColumnInTable('127.0.0.2:PORT','d1','secret','pw')  -> 1               <-- bypass

127.0.0.2 is not matched by isLocalAddress (it is reserved as non-local per the comment in isLocalAddress.cpp), so it routes through the remote branch, connects as default, and returns the metadata the local check blocks. Same for any external/LB address resolving to the node.

Fix: the SHOW_COLUMNS check now runs at the top of executeImpl, before the local/remote split, so it gates both paths. After the fix, all four cases above are ACCESS_DENIED; after GRANT SHOW COLUMNS ON d1.secret, the 127.0.0.2 overload correctly returns 1 (legitimate remote-with-grant use is preserved).

Test 04401 now also covers the 127.0.0.2 remote-to-self overload in the without-grant block. Because denial happens before dispatch, it is deterministic regardless of what the server listens on. Description and changelog updated.

[groeneai]

CI finish ledger — 72a9159

CI fully finished on this head (Finish Workflow SUCCESS, all checks green). No failed checks.

Check / test	Reason	Owner / fixing PR
(none)	all checks passed	—
Sync	—	CH Inc sync (private, not actionable)

Session id: cron:our-pr-ci-monitor:20260707-180000

[Ergus]

@groeneai you didn't reply to all my review comments. WHy?

It replied.

[alexey-milovidov]

LGTM

[groeneai]

CI finish ledger - 0221328

Every failure below has an owner (a fixing PR). Only CH Inc sync is exempt.

Check / test	Reason	Owner / fixing PR
Integration tests (amd_tsan, 3/6) / test_replicated_database::test_replicated_table_structure_alter	flaky (chronic ReplicatedDB DDL-recovery race; 92 master hits + 20+ unrelated PRs in 30d; assert 1\t2\t3 vs 1\t2)	#110030 (ours, open)
Stateless tests (arm_binary, parallel) / 01730_distributed_group_by_no_merge_order_by_long	flaky (Code 241 MEMORY_LIMIT, transient; CI rerun 138/138 passed, "not reproducible"; 15 master hits + 20+ unrelated PRs in 30d)	#109847 (external, open)

PR-caused: none. This PR's diff is a SHOW COLUMNS access check in hasColumnInTable plus one stateless RBAC test; it cannot cause an amd_tsan ReplicatedDatabase DDL-ordering race or an arm_binary distributed-GROUP-BY memory-limit transient.

[clickhouse-gh]

LLVM Coverage Report

Metric	Baseline	Current	Δ
Lines	86.10%	86.10%	+0.00%
Functions	91.80%	91.80%	+0.00%
Branches	78.20%	78.20%	+0.00%

Changed lines: Changed C/C++ lines covered: 15/15 (100.00%) · Uncovered code

Full report · Diff report

[clickgapai]

Found via ClickGap automated review. Please close or comment if this is incorrect or needs adjustment.

TL;DR

Add SHOW COLUMNS access check to hasColumnInTable

ClickGap verified: reproduced it with the script below on 26.6; tested 7 version(s) — 6 affected, 1 clean (see Affects).

Reproduction

-- As an admin (default user):
CREATE USER probe IDENTIFIED WITH plaintext_password BY 'pw';
GRANT SELECT ON system.one TO probe;
CREATE TABLE default.secret (id UInt64, password String) ENGINE = MergeTree ORDER BY id;
-- Now connect as user `probe` (has ONLY SELECT on system.one, no grant on default.secret):
SELECT hasColumnInTable('default', 'secret', 'password');
-- Before the fix: returns 1 (column-existence/table-existence bypass with no privilege).
-- After the fix: fails with ACCESS_DENIED (SHOW COLUMNS required, like DESCRIBE).

▶ Run on ClickHouse Fiddle

Bisect

• Bisected on master in several steps (confidence: MEDIUM); a single introducing PR could not be pinned (culprit confirmation was inconclusive).

Affects

• ✓ Fix is on master — this PR's change fixes the bug on master. Branches below still have the bug and need the backport.
• Reproduces on: 26.6, 26.5, 26.4, 26.3, 26.2, 26.1
• Does NOT reproduce on: master
• Backport needed: 26.6, 26.5, 26.4, 26.3, 26.2, 26.1

Suggested next step

The fix has landed on master; the affected release branches above still need the backport.

Component: comp-rbac
Severity: P2

cc fix PR author: @groeneai
cc fix PR reviewer: @alexey-milovidov

---

ClickGap · Finding: phase_d_pr108464

Triage metadata

• Primary component: comp-rbac
• Secondary components: comp-regular-function, comp-testing
• Component owners (comp-rbac): @vitlibar @pufit

[groeneai]

Finding confirmed: the SHOW COLUMNS access check in hasColumnInTable is on master; 26.1-26.6 still carry the bypass. Backports to release branches are maintainer-driven via the pr-must-backport label, so I am not opening manual backport PRs per project process.