Skip to content

Commit e5c9e55

Browse files
committed
Guard against access to tenant columns that aren't selectable
1 parent 6276661 commit e5c9e55

File tree

2 files changed

+283
-6
lines changed

2 files changed

+283
-6
lines changed

internal-packages/tsql/src/query/printer.test.ts

Lines changed: 188 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2424,3 +2424,191 @@ describe("Field Mapping Value Transformation", () => {
24242424
// But the column metadata should use the exposed name
24252425
});
24262426
});
2427+
2428+
describe("Internal-only column blocking", () => {
2429+
/**
2430+
* These tests verify that internal columns (tenant columns, required filter columns)
2431+
* that are NOT exposed in tableSchema.columns are blocked from user queries in
2432+
* SELECT, ORDER BY, GROUP BY, and HAVING clauses, but allowed in system-generated
2433+
* tenant isolation guards.
2434+
*/
2435+
2436+
// Schema with hidden tenant columns (not exposed in public columns)
2437+
const hiddenTenantSchema: TableSchema = {
2438+
name: "hidden_tenant_runs",
2439+
clickhouseName: "trigger_dev.task_runs_v2",
2440+
columns: {
2441+
// Public columns - these are exposed to users
2442+
id: { name: "id", ...column("String") },
2443+
status: { name: "status", ...column("String") },
2444+
task_identifier: { name: "task_identifier", ...column("String") },
2445+
created_at: { name: "created_at", ...column("DateTime64") },
2446+
},
2447+
// Tenant columns are NOT in the public columns above
2448+
tenantColumns: {
2449+
organizationId: "organization_id",
2450+
projectId: "project_id",
2451+
environmentId: "environment_id",
2452+
},
2453+
};
2454+
2455+
// Schema with hidden required filter column
2456+
const hiddenFilterSchema: TableSchema = {
2457+
name: "hidden_filter_runs",
2458+
clickhouseName: "trigger_dev.task_runs_v2",
2459+
columns: {
2460+
id: { name: "id", ...column("String") },
2461+
status: { name: "status", ...column("String") },
2462+
created_at: { name: "created_at", ...column("DateTime64") },
2463+
},
2464+
tenantColumns: {
2465+
organizationId: "organization_id",
2466+
projectId: "project_id",
2467+
environmentId: "environment_id",
2468+
},
2469+
requiredFilters: [
2470+
{ column: "engine", value: "V2" }, // 'engine' is not in public columns
2471+
],
2472+
};
2473+
2474+
function createHiddenTenantContext(): PrinterContext {
2475+
const schema = createSchemaRegistry([hiddenTenantSchema]);
2476+
return createPrinterContext({
2477+
organizationId: "org_test",
2478+
projectId: "proj_test",
2479+
environmentId: "env_test",
2480+
schema,
2481+
});
2482+
}
2483+
2484+
function createHiddenFilterContext(): PrinterContext {
2485+
const schema = createSchemaRegistry([hiddenFilterSchema]);
2486+
return createPrinterContext({
2487+
organizationId: "org_test",
2488+
projectId: "proj_test",
2489+
environmentId: "env_test",
2490+
schema,
2491+
});
2492+
}
2493+
2494+
describe("should block internal-only columns in SELECT", () => {
2495+
it("should throw error when selecting hidden tenant column (organization_id)", () => {
2496+
const ctx = createHiddenTenantContext();
2497+
expect(() => {
2498+
printQuery("SELECT id, organization_id FROM hidden_tenant_runs", ctx);
2499+
}).toThrow(QueryError);
2500+
expect(() => {
2501+
printQuery("SELECT id, organization_id FROM hidden_tenant_runs", ctx);
2502+
}).toThrow(/not available for querying/i);
2503+
});
2504+
2505+
it("should throw error when selecting hidden tenant column (project_id)", () => {
2506+
const ctx = createHiddenTenantContext();
2507+
expect(() => {
2508+
printQuery("SELECT project_id FROM hidden_tenant_runs", ctx);
2509+
}).toThrow(/not available for querying/i);
2510+
});
2511+
2512+
it("should throw error when selecting hidden tenant column (environment_id)", () => {
2513+
const ctx = createHiddenTenantContext();
2514+
expect(() => {
2515+
printQuery("SELECT environment_id FROM hidden_tenant_runs", ctx);
2516+
}).toThrow(/not available for querying/i);
2517+
});
2518+
2519+
it("should throw error when selecting hidden required filter column", () => {
2520+
const ctx = createHiddenFilterContext();
2521+
expect(() => {
2522+
printQuery("SELECT id, engine FROM hidden_filter_runs", ctx);
2523+
}).toThrow(/not available for querying/i);
2524+
});
2525+
});
2526+
2527+
describe("should block internal-only columns in ORDER BY", () => {
2528+
it("should throw error when ordering by hidden tenant column", () => {
2529+
const ctx = createHiddenTenantContext();
2530+
expect(() => {
2531+
printQuery("SELECT id, status FROM hidden_tenant_runs ORDER BY organization_id", ctx);
2532+
}).toThrow(/not available for querying/i);
2533+
});
2534+
2535+
it("should throw error when ordering by hidden required filter column", () => {
2536+
const ctx = createHiddenFilterContext();
2537+
expect(() => {
2538+
printQuery("SELECT id, status FROM hidden_filter_runs ORDER BY engine", ctx);
2539+
}).toThrow(/not available for querying/i);
2540+
});
2541+
});
2542+
2543+
describe("should block internal-only columns in GROUP BY", () => {
2544+
it("should throw error when grouping by hidden tenant column", () => {
2545+
const ctx = createHiddenTenantContext();
2546+
expect(() => {
2547+
printQuery("SELECT count(*) FROM hidden_tenant_runs GROUP BY organization_id", ctx);
2548+
}).toThrow(/not available for querying/i);
2549+
});
2550+
2551+
it("should throw error when grouping by hidden required filter column", () => {
2552+
const ctx = createHiddenFilterContext();
2553+
expect(() => {
2554+
printQuery("SELECT count(*) FROM hidden_filter_runs GROUP BY engine", ctx);
2555+
}).toThrow(/not available for querying/i);
2556+
});
2557+
});
2558+
2559+
describe("should block internal-only columns in HAVING", () => {
2560+
it("should throw error when using hidden column in HAVING", () => {
2561+
const ctx = createHiddenTenantContext();
2562+
expect(() => {
2563+
printQuery(
2564+
"SELECT status, count(*) as cnt FROM hidden_tenant_runs GROUP BY status HAVING organization_id = 'x'",
2565+
ctx
2566+
);
2567+
}).toThrow(/not available for querying/i);
2568+
});
2569+
});
2570+
2571+
describe("should allow tenant guard in WHERE clause", () => {
2572+
it("should successfully execute query with tenant isolation (hidden tenant columns work internally)", () => {
2573+
const ctx = createHiddenTenantContext();
2574+
// This should succeed - the tenant guard is injected internally and should work
2575+
const { sql } = printQuery("SELECT id, status FROM hidden_tenant_runs", ctx);
2576+
2577+
// Verify tenant guards are present in WHERE clause
2578+
expect(sql).toContain("organization_id");
2579+
expect(sql).toContain("project_id");
2580+
expect(sql).toContain("environment_id");
2581+
// But NOT in SELECT
2582+
expect(sql).toMatch(/SELECT\s+id,\s*status\s+FROM/i);
2583+
});
2584+
2585+
it("should successfully execute query with required filter (hidden filter columns work internally)", () => {
2586+
const ctx = createHiddenFilterContext();
2587+
const { sql, params } = printQuery("SELECT id, status FROM hidden_filter_runs", ctx);
2588+
2589+
// Verify required filter is present in WHERE clause
2590+
expect(sql).toContain("engine");
2591+
// The value V2 is parameterized, check that it's in the params
2592+
expect(Object.values(params)).toContain("V2");
2593+
});
2594+
});
2595+
2596+
describe("should allow exposed tenant columns", () => {
2597+
// In task_runs schema, tenant columns ARE exposed in the public columns
2598+
it("should allow selecting exposed tenant column", () => {
2599+
// task_runs schema exposes organization_id in its columns
2600+
const { sql } = printQuery("SELECT id, organization_id FROM task_runs");
2601+
expect(sql).toContain("SELECT id, organization_id");
2602+
});
2603+
2604+
it("should allow ordering by exposed tenant column", () => {
2605+
const { sql } = printQuery("SELECT id, status FROM task_runs ORDER BY organization_id");
2606+
expect(sql).toContain("ORDER BY organization_id");
2607+
});
2608+
2609+
it("should allow grouping by exposed tenant column", () => {
2610+
const { sql } = printQuery("SELECT organization_id, count(*) FROM task_runs GROUP BY organization_id");
2611+
expect(sql).toContain("GROUP BY organization_id");
2612+
});
2613+
});
2614+
});

internal-packages/tsql/src/query/printer.ts

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,17 @@ export class ClickHousePrinter {
126126
* These are populated from tableSchema.tenantColumns when processing joins.
127127
*/
128128
private allowedInternalColumns: Set<string> = new Set();
129+
/**
130+
* Set of internal-only column names that are NOT user-queryable.
131+
* These are tenant columns and required filter columns that are not exposed in tableSchema.columns.
132+
* Used to block user queries from accessing internal columns in SELECT, ORDER BY, GROUP BY, HAVING.
133+
*/
134+
private internalOnlyColumns: Set<string> = new Set();
135+
/**
136+
* Whether we're currently processing user projection clauses (SELECT, ORDER BY, GROUP BY, HAVING).
137+
* When true, internal-only columns will be rejected.
138+
*/
139+
private inProjectionContext = false;
129140

130141
constructor(
131142
private context: PrinterContext,
@@ -324,14 +335,17 @@ export class ClickHousePrinter {
324335
// Top-level queries clear contexts; subqueries save parent context and create fresh context
325336
const savedTableContexts = new Map(this.tableContexts);
326337
const savedInternalColumns = new Set(this.allowedInternalColumns);
338+
const savedInternalOnlyColumns = new Set(this.internalOnlyColumns);
327339
if (isTopLevelQuery) {
328340
this.tableContexts.clear();
329341
this.allowedInternalColumns.clear();
342+
this.internalOnlyColumns.clear();
330343
} else {
331344
// Subqueries get fresh contexts - they don't inherit parent tables
332345
// (the parent will restore its context after the subquery is processed)
333346
this.tableContexts = new Map();
334347
this.allowedInternalColumns = new Set();
348+
this.internalOnlyColumns = new Set();
335349
}
336350

337351
// Build WHERE clause starting with any existing where
@@ -380,15 +394,18 @@ export class ClickHousePrinter {
380394

381395
// Process SELECT columns and collect metadata
382396
// Using flatMap because asterisk expansion can return multiple columns
397+
// Set inProjectionContext to block internal-only columns in user projections
383398
let columns: string[];
384399
if (node.select && node.select.length > 0) {
385400
// Only collect metadata for top-level queries (not subqueries)
386401
if (isTopLevelQuery) {
387402
this.outputColumns = [];
388403
}
404+
this.inProjectionContext = true;
389405
columns = node.select.flatMap((col) =>
390406
this.visitSelectColumnWithMetadata(col, isTopLevelQuery)
391407
);
408+
this.inProjectionContext = false;
392409
} else {
393410
columns = ["1"];
394411
}
@@ -406,16 +423,33 @@ export class ClickHousePrinter {
406423
const prewhere = node.prewhere ? this.visit(node.prewhere) : null;
407424
const whereStr = where ? this.visit(where) : null;
408425

409-
// Process GROUP BY with context flag to use raw columns for whereTransform columns
426+
// Process GROUP BY with context flags:
427+
// - inGroupByContext: use raw columns for whereTransform columns
428+
// - inProjectionContext: block internal-only columns
410429
let groupBy: string[] | null = null;
411430
if (node.group_by) {
412431
this.inGroupByContext = true;
432+
this.inProjectionContext = true;
413433
groupBy = node.group_by.map((col) => this.visit(col));
434+
this.inProjectionContext = false;
414435
this.inGroupByContext = false;
415436
}
416437

417-
const having = node.having ? this.visit(node.having) : null;
418-
const orderBy = node.order_by ? node.order_by.map((col) => this.visit(col)) : null;
438+
// Process HAVING with inProjectionContext to block internal-only columns
439+
let having: string | null = null;
440+
if (node.having) {
441+
this.inProjectionContext = true;
442+
having = this.visit(node.having);
443+
this.inProjectionContext = false;
444+
}
445+
446+
// Process ORDER BY with inProjectionContext to block internal-only columns
447+
let orderBy: string[] | null = null;
448+
if (node.order_by) {
449+
this.inProjectionContext = true;
450+
orderBy = node.order_by.map((col) => this.visit(col));
451+
this.inProjectionContext = false;
452+
}
419453

420454
// Process ARRAY JOIN
421455
let arrayJoin = "";
@@ -511,6 +545,7 @@ export class ClickHousePrinter {
511545
this.selectAliases = savedAliases;
512546
this.tableContexts = savedTableContexts;
513547
this.allowedInternalColumns = savedInternalColumns;
548+
this.internalOnlyColumns = savedInternalOnlyColumns;
514549

515550
return response;
516551
}
@@ -1416,18 +1451,38 @@ export class ClickHousePrinter {
14161451

14171452
// Register tenant columns as allowed internal columns
14181453
// These are ClickHouse column names used for tenant isolation
1454+
// Also mark them as internal-only if they're not exposed in tableSchema.columns
14191455
if (tableSchema.tenantColumns) {
14201456
const { organizationId, projectId, environmentId } = tableSchema.tenantColumns;
1421-
if (organizationId) this.allowedInternalColumns.add(organizationId);
1422-
if (projectId) this.allowedInternalColumns.add(projectId);
1423-
if (environmentId) this.allowedInternalColumns.add(environmentId);
1457+
if (organizationId) {
1458+
this.allowedInternalColumns.add(organizationId);
1459+
if (!this.isColumnExposedInSchema(tableSchema, organizationId)) {
1460+
this.internalOnlyColumns.add(organizationId);
1461+
}
1462+
}
1463+
if (projectId) {
1464+
this.allowedInternalColumns.add(projectId);
1465+
if (!this.isColumnExposedInSchema(tableSchema, projectId)) {
1466+
this.internalOnlyColumns.add(projectId);
1467+
}
1468+
}
1469+
if (environmentId) {
1470+
this.allowedInternalColumns.add(environmentId);
1471+
if (!this.isColumnExposedInSchema(tableSchema, environmentId)) {
1472+
this.internalOnlyColumns.add(environmentId);
1473+
}
1474+
}
14241475
}
14251476

14261477
// Register required filter columns as allowed internal columns
14271478
// These are ClickHouse columns used for internal filtering (e.g., engine = 'V2')
1479+
// Also mark them as internal-only if they're not exposed in tableSchema.columns
14281480
if (tableSchema.requiredFilters) {
14291481
for (const filter of tableSchema.requiredFilters) {
14301482
this.allowedInternalColumns.add(filter.column);
1483+
if (!this.isColumnExposedInSchema(tableSchema, filter.column)) {
1484+
this.internalOnlyColumns.add(filter.column);
1485+
}
14311486
}
14321487
}
14331488

@@ -2219,8 +2274,19 @@ export class ClickHousePrinter {
22192274
return chain; // Valid alias reference
22202275
}
22212276

2277+
// Check if this is an internal-only column being accessed in a user projection context
2278+
// (SELECT, ORDER BY, GROUP BY, HAVING). These columns are used internally for
2279+
// tenant isolation but should not be user-queryable unless explicitly exposed.
2280+
if (this.inProjectionContext && this.internalOnlyColumns.has(columnName)) {
2281+
const availableColumns = this.getAvailableColumnNames();
2282+
throw new QueryError(
2283+
`Column "${columnName}" is not available for querying. Available columns: ${availableColumns.join(", ")}`
2284+
);
2285+
}
2286+
22222287
// Check if this is an allowed internal column (e.g., tenant columns)
22232288
// These are ClickHouse column names that are allowed for internal use only
2289+
// (e.g., in WHERE clauses for tenant isolation)
22242290
if (this.allowedInternalColumns.has(columnName)) {
22252291
return chain;
22262292
}
@@ -2256,6 +2322,29 @@ export class ClickHousePrinter {
22562322
return [...new Set(columns)].sort();
22572323
}
22582324

2325+
/**
2326+
* Check if a ClickHouse column name is exposed in the table schema's public columns.
2327+
* A column is considered exposed if:
2328+
* - It exists as a column name in tableSchema.columns, OR
2329+
* - It is the clickhouseName of a column in tableSchema.columns
2330+
*
2331+
* @param tableSchema The table schema to check
2332+
* @param clickhouseColumnName The ClickHouse column name to check
2333+
* @returns true if the column is exposed to users, false if it's internal-only
2334+
*/
2335+
private isColumnExposedInSchema(tableSchema: TableSchema, clickhouseColumnName: string): boolean {
2336+
for (const [tsqlName, columnSchema] of Object.entries(tableSchema.columns)) {
2337+
// Check if the ClickHouse column name matches either:
2338+
// 1. The TSQL name (if no clickhouseName mapping exists)
2339+
// 2. The explicit clickhouseName mapping
2340+
const actualClickhouseName = columnSchema.clickhouseName || tsqlName;
2341+
if (actualClickhouseName === clickhouseColumnName) {
2342+
return true;
2343+
}
2344+
}
2345+
return false;
2346+
}
2347+
22592348
/**
22602349
* Find the TSQL column name for a given ClickHouse column name.
22612350
* This is used to provide helpful suggestions when users accidentally

0 commit comments

Comments
 (0)