mirror of https://github.com/buster-so/buster.git
permission and read only
This commit is contained in:
parent
db3a0a7c50
commit
d44277d1a1
|
@ -227,6 +227,72 @@ describe('Permission Validator', () => {
|
|||
expect(result.error).toContain('Failed to parse SQL');
|
||||
});
|
||||
|
||||
it('should reject INSERT statements', async () => {
|
||||
vi.mocked(accessControls.getPermissionedDatasets).mockResolvedValueOnce([
|
||||
{
|
||||
ymlFile: `
|
||||
models:
|
||||
- name: users
|
||||
schema: public
|
||||
`,
|
||||
},
|
||||
] as any);
|
||||
|
||||
const result = await validateSqlPermissions('INSERT INTO public.users (name) VALUES ("test")', 'user123');
|
||||
|
||||
expect(result.isAuthorized).toBe(false);
|
||||
expect(result.error).toContain("Query type 'insert' is not allowed");
|
||||
expect(result.unauthorizedTables).toHaveLength(0);
|
||||
});
|
||||
|
||||
it('should reject UPDATE statements', async () => {
|
||||
vi.mocked(accessControls.getPermissionedDatasets).mockResolvedValueOnce([
|
||||
{
|
||||
ymlFile: `
|
||||
models:
|
||||
- name: users
|
||||
schema: public
|
||||
`,
|
||||
},
|
||||
] as any);
|
||||
|
||||
const result = await validateSqlPermissions('UPDATE public.users SET name = "updated" WHERE id = 1', 'user123');
|
||||
|
||||
expect(result.isAuthorized).toBe(false);
|
||||
expect(result.error).toContain("Query type 'update' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject DELETE statements', async () => {
|
||||
vi.mocked(accessControls.getPermissionedDatasets).mockResolvedValueOnce([
|
||||
{
|
||||
ymlFile: `
|
||||
models:
|
||||
- name: users
|
||||
schema: public
|
||||
`,
|
||||
},
|
||||
] as any);
|
||||
|
||||
const result = await validateSqlPermissions('DELETE FROM public.users WHERE id = 1', 'user123');
|
||||
|
||||
expect(result.isAuthorized).toBe(false);
|
||||
expect(result.error).toContain("Query type 'delete' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject CREATE TABLE statements', async () => {
|
||||
const result = await validateSqlPermissions('CREATE TABLE new_table (id INT)', 'user123');
|
||||
|
||||
expect(result.isAuthorized).toBe(false);
|
||||
expect(result.error).toContain("Query type 'create' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject DROP TABLE statements', async () => {
|
||||
const result = await validateSqlPermissions('DROP TABLE users', 'user123');
|
||||
|
||||
expect(result.isAuthorized).toBe(false);
|
||||
expect(result.error).toContain("Query type 'drop' is not allowed");
|
||||
});
|
||||
|
||||
it('should handle multiple datasets with overlapping tables', async () => {
|
||||
vi.mocked(accessControls.getPermissionedDatasets).mockResolvedValueOnce([
|
||||
{
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
import { getPermissionedDatasets } from '@buster/access-controls';
|
||||
import { extractPhysicalTables, extractTablesFromYml, tablesMatch, type ParsedTable } from './sql-parser-helpers';
|
||||
import { extractPhysicalTables, extractTablesFromYml, tablesMatch, checkQueryIsReadOnly, type ParsedTable } from './sql-parser-helpers';
|
||||
|
||||
export interface PermissionValidationResult {
|
||||
isAuthorized: boolean;
|
||||
|
@ -17,6 +17,16 @@ export async function validateSqlPermissions(
|
|||
dataSourceSyntax?: string
|
||||
): Promise<PermissionValidationResult> {
|
||||
try {
|
||||
// First check if query is read-only
|
||||
const readOnlyCheck = checkQueryIsReadOnly(sql, dataSourceSyntax);
|
||||
if (!readOnlyCheck.isReadOnly) {
|
||||
return {
|
||||
isAuthorized: false,
|
||||
unauthorizedTables: [],
|
||||
error: readOnlyCheck.error || 'Only read-only queries are allowed'
|
||||
};
|
||||
}
|
||||
|
||||
// Extract physical tables from SQL
|
||||
const tablesInQuery = extractPhysicalTables(sql, dataSourceSyntax);
|
||||
|
||||
|
|
|
@ -5,6 +5,7 @@ import {
|
|||
normalizeTableIdentifier,
|
||||
tablesMatch,
|
||||
extractTablesFromYml,
|
||||
checkQueryIsReadOnly,
|
||||
type ParsedTable
|
||||
} from './sql-parser-helpers';
|
||||
|
||||
|
@ -410,4 +411,82 @@ models:
|
|||
expect(tables2).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('checkQueryIsReadOnly', () => {
|
||||
it('should allow SELECT statements', () => {
|
||||
const result = checkQueryIsReadOnly('SELECT * FROM users');
|
||||
expect(result.isReadOnly).toBe(true);
|
||||
expect(result.queryType).toBe('select');
|
||||
expect(result.error).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should allow SELECT with JOIN', () => {
|
||||
const result = checkQueryIsReadOnly('SELECT u.id, o.total FROM users u JOIN orders o ON u.id = o.user_id');
|
||||
expect(result.isReadOnly).toBe(true);
|
||||
});
|
||||
|
||||
it('should allow SELECT with CTEs', () => {
|
||||
const sql = `
|
||||
WITH stats AS (
|
||||
SELECT user_id, COUNT(*) as count FROM orders GROUP BY user_id
|
||||
)
|
||||
SELECT * FROM stats
|
||||
`;
|
||||
const result = checkQueryIsReadOnly(sql);
|
||||
expect(result.isReadOnly).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject INSERT statements', () => {
|
||||
const result = checkQueryIsReadOnly('INSERT INTO users (name, email) VALUES ("John", "john@example.com")');
|
||||
expect(result.isReadOnly).toBe(false);
|
||||
expect(result.queryType).toBe('insert');
|
||||
expect(result.error).toContain("Query type 'insert' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject UPDATE statements', () => {
|
||||
const result = checkQueryIsReadOnly('UPDATE users SET name = "Jane" WHERE id = 1');
|
||||
expect(result.isReadOnly).toBe(false);
|
||||
expect(result.queryType).toBe('update');
|
||||
expect(result.error).toContain("Query type 'update' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject DELETE statements', () => {
|
||||
const result = checkQueryIsReadOnly('DELETE FROM users WHERE id = 1');
|
||||
expect(result.isReadOnly).toBe(false);
|
||||
expect(result.queryType).toBe('delete');
|
||||
expect(result.error).toContain("Query type 'delete' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject CREATE statements', () => {
|
||||
const result = checkQueryIsReadOnly('CREATE TABLE new_users (id INT, name VARCHAR(100))');
|
||||
expect(result.isReadOnly).toBe(false);
|
||||
expect(result.queryType).toBe('create');
|
||||
expect(result.error).toContain("Query type 'create' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject DROP statements', () => {
|
||||
const result = checkQueryIsReadOnly('DROP TABLE users');
|
||||
expect(result.isReadOnly).toBe(false);
|
||||
expect(result.queryType).toBe('drop');
|
||||
expect(result.error).toContain("Query type 'drop' is not allowed");
|
||||
});
|
||||
|
||||
it('should reject ALTER statements', () => {
|
||||
const result = checkQueryIsReadOnly('ALTER TABLE users ADD COLUMN age INT');
|
||||
expect(result.isReadOnly).toBe(false);
|
||||
expect(result.queryType).toBe('alter');
|
||||
expect(result.error).toContain("Query type 'alter' is not allowed");
|
||||
});
|
||||
|
||||
it('should handle PostgreSQL dialect', () => {
|
||||
const result = checkQueryIsReadOnly('SELECT * FROM postgres.public.users', 'postgres');
|
||||
expect(result.isReadOnly).toBe(true);
|
||||
});
|
||||
|
||||
it('should handle invalid SQL gracefully', () => {
|
||||
const result = checkQueryIsReadOnly('NOT VALID SQL');
|
||||
expect(result.isReadOnly).toBe(false);
|
||||
expect(result.error).toContain('Failed to parse SQL');
|
||||
});
|
||||
});
|
||||
});
|
|
@ -9,6 +9,12 @@ export interface ParsedTable {
|
|||
alias?: string;
|
||||
}
|
||||
|
||||
export interface QueryTypeCheckResult {
|
||||
isReadOnly: boolean;
|
||||
queryType?: string;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
// Map data source syntax to node-sql-parser dialect
|
||||
const DIALECT_MAPPING: Record<string, string> = {
|
||||
// Direct mappings
|
||||
|
@ -331,3 +337,47 @@ export function extractTablesFromYml(ymlContent: string): ParsedTable[] {
|
|||
|
||||
return tables;
|
||||
}
|
||||
|
||||
/**
|
||||
* Checks if a SQL query is read-only (SELECT statements only)
|
||||
* Returns error if query contains write operations
|
||||
*/
|
||||
export function checkQueryIsReadOnly(sql: string, dataSourceSyntax?: string): QueryTypeCheckResult {
|
||||
const dialect = getParserDialect(dataSourceSyntax);
|
||||
const parser = new Parser();
|
||||
|
||||
try {
|
||||
// Parse SQL into AST with the appropriate dialect
|
||||
const ast = parser.astify(sql, { database: dialect });
|
||||
|
||||
// Handle single statement or array of statements
|
||||
const statements = Array.isArray(ast) ? ast : [ast];
|
||||
|
||||
// Check each statement
|
||||
for (const statement of statements) {
|
||||
// Check if statement has a type property
|
||||
if ('type' in statement && statement.type) {
|
||||
const queryType = statement.type.toLowerCase();
|
||||
|
||||
// Only allow SELECT statements
|
||||
if (queryType !== 'select') {
|
||||
return {
|
||||
isReadOnly: false,
|
||||
queryType: statement.type,
|
||||
error: `Query type '${statement.type}' is not allowed. Only SELECT statements are permitted for read-only access.`
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
isReadOnly: true,
|
||||
queryType: 'select'
|
||||
};
|
||||
} catch (error) {
|
||||
return {
|
||||
isReadOnly: false,
|
||||
error: `Failed to parse SQL for query type check: ${error instanceof Error ? error.message : 'Unknown error'}`
|
||||
};
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue