Skip to content

Commit 9ec9760

Browse files
Copilothuangyiirene
andcommitted
fix: Address code review comments
- Remove duplicate comment in util.ts - Add table existence validation before SQLite PRAGMA queries - Sanitize error messages to not expose table names - Make console.log messages more concise and consistent Co-authored-by: huangyiirene <7665279+huangyiirene@users.noreply.github.com>
1 parent 782db58 commit 9ec9760

3 files changed

Lines changed: 26 additions & 7 deletions

File tree

packages/drivers/sql/src/index.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -638,7 +638,18 @@ export class SqlDriver implements Driver {
638638
});
639639
}
640640
} else if (this.config.client === 'sqlite3') {
641-
// SQLite PRAGMA doesn't support parameter binding, so we need to ensure safe identifier
641+
// SQLite PRAGMA doesn't support parameter binding, so we need to ensure safe identifier.
642+
// First, verify that the requested table actually exists using a parameterized query.
643+
const tableExistsResult = await this.knex.raw(
644+
"SELECT name FROM sqlite_master WHERE type = 'table' AND name = ?",
645+
[tableName]
646+
);
647+
648+
if (!tableExistsResult || tableExistsResult.length === 0) {
649+
// If the table does not exist, there are no foreign keys to introspect.
650+
return foreignKeys;
651+
}
652+
642653
// Table names in ObjectQL are validated and should be safe, but we add extra protection
643654
const safeTableName = tableName.replace(/[^a-zA-Z0-9_]/g, '');
644655
const result = await this.knex.raw(`PRAGMA foreign_key_list(${safeTableName})`);
@@ -653,7 +664,7 @@ export class SqlDriver implements Driver {
653664
}
654665
}
655666
} catch (error) {
656-
console.warn(`Could not introspect foreign keys for table ${tableName}:`, error);
667+
console.warn('Could not introspect foreign keys for requested table:', error);
657668
}
658669

659670
return foreignKeys;
@@ -693,8 +704,17 @@ export class SqlDriver implements Driver {
693704
}
694705
} else if (this.config.client === 'sqlite3') {
695706
// SQLite PRAGMA doesn't support parameter binding, so we need to ensure safe identifier
696-
// Table names in ObjectQL are validated and should be safe, but we add extra protection
697707
const safeTableName = tableName.replace(/[^a-zA-Z0-9_]/g, '');
708+
709+
// Validate that the sanitized table name exists in the database before using it in PRAGMA
710+
const tablesResult = await this.knex.raw("SELECT name FROM sqlite_master WHERE type = 'table'");
711+
const tableNames = tablesResult.map((row: any) => row.name);
712+
713+
if (!tableNames.includes(safeTableName)) {
714+
console.warn('Could not introspect primary keys for SQLite table: table does not exist after sanitization.');
715+
return primaryKeys;
716+
}
717+
698718
const result = await this.knex.raw(`PRAGMA table_info(${safeTableName})`);
699719

700720
for (const row of result) {
@@ -704,7 +724,7 @@ export class SqlDriver implements Driver {
704724
}
705725
}
706726
} catch (error) {
707-
console.warn(`Could not introspect primary keys for table ${tableName}:`, error);
727+
console.warn('Could not introspect primary keys for a table:', error);
708728
}
709729

710730
return primaryKeys;

packages/foundation/core/src/app.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,13 +181,13 @@ export class ObjectQL implements IObjectQL {
181181
throw new Error(`Driver for datasource '${datasourceName}' does not support schema introspection`);
182182
}
183183

184-
console.log(`Introspecting database schema from datasource '${datasourceName}'...`);
184+
console.log(`Introspecting datasource '${datasourceName}'...`);
185185
const introspectedSchema = await driver.introspectSchema();
186186

187187
// Convert introspected schema to ObjectQL objects
188188
const objects = convertIntrospectedSchemaToObjects(introspectedSchema, options);
189189

190-
console.log(`Discovered ${objects.length} tables. Registering as objects...`);
190+
console.log(`Discovered ${objects.length} table(s), registering as objects...`);
191191

192192
// Register each discovered object
193193
for (const obj of objects) {

packages/foundation/core/src/util.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,6 @@ export function convertIntrospectedSchemaToObjects(
123123
fieldConfig.unique = true;
124124
}
125125

126-
// Add max length for text fields
127126
// Add max length for text fields
128127
if (column.maxLength && (fieldType === 'text' || fieldType === 'textarea')) {
129128
fieldConfig.max_length = column.maxLength;

0 commit comments

Comments
 (0)