Refactor: Split Monolithic SqliteApi.ts & SupabaseApi.ts into Domain-Specific Modules#4712
Refactor: Split Monolithic SqliteApi.ts & SupabaseApi.ts into Domain-Specific Modules#4712guddu-sutara wants to merge 1 commit into
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
There was a problem hiding this comment.
Code Review
This pull request introduces SQLite-first API implementations alongside their Supabase counterparts to support offline data synchronization and local database management. The review feedback highlights several critical SQL injection vulnerabilities across multiple SQLite API files (such as SqliteApi.ops.ts, SqliteApi.assignment.ts, SqliteApi.core.ts, SqliteApi.course.ts, SqliteApi.rewards.ts, and SqliteApi.school.ts) caused by direct string interpolation in SQL queries. It is highly recommended to parameterize these queries using placeholders. Additionally, the feedback advises replacing the boxed String object type with the primitive string type in the course and assignment APIs.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const query = `SELECT code | ||
| FROM ${TABLES.ClassInvite_code} | ||
| WHERE class_id='${class_id}' | ||
| AND is_deleted = FALSE | ||
| AND (expires_at >= '${currentDate}')`; | ||
|
|
||
| try { | ||
| const res = await this._db?.query(query); |
There was a problem hiding this comment.
The query is constructed using string interpolation for class_id and currentDate, which introduces a SQL injection vulnerability and can cause runtime errors if the values contain special characters. Please use parameterized queries with ? placeholders instead.
| const query = `SELECT code | |
| FROM ${TABLES.ClassInvite_code} | |
| WHERE class_id='${class_id}' | |
| AND is_deleted = FALSE | |
| AND (expires_at >= '${currentDate}')`; | |
| try { | |
| const res = await this._db?.query(query); | |
| const query = `SELECT code | |
| FROM ${TABLES.ClassInvite_code} | |
| WHERE class_id = ? | |
| AND is_deleted = 0 | |
| AND (expires_at >= ?)`; | |
| try { | |
| const res = await this._db?.query(query, [class_id, currentDate]); |
| const query = `SELECT * | ||
| FROM ${TABLES.Result} | ||
| WHERE chapter_id = '${chapter_id}' | ||
| AND course_id = '${course_id}' | ||
| AND class_id ='${classId}' | ||
| AND created_at BETWEEN '${startDate}' AND '${endDate}' | ||
| ORDER BY created_at DESC;`; | ||
|
|
||
| const res = await this._db?.query(query); |
There was a problem hiding this comment.
This query uses string interpolation for several parameters (chapter_id, course_id, classId, startDate, endDate), making it vulnerable to SQL injection. Please parameterize the query using ? placeholders.
| const query = `SELECT * | |
| FROM ${TABLES.Result} | |
| WHERE chapter_id = '${chapter_id}' | |
| AND course_id = '${course_id}' | |
| AND class_id ='${classId}' | |
| AND created_at BETWEEN '${startDate}' AND '${endDate}' | |
| ORDER BY created_at DESC;`; | |
| const res = await this._db?.query(query); | |
| const query = `SELECT * | |
| FROM ${TABLES.Result} | |
| WHERE chapter_id = ? | |
| AND course_id = ? | |
| AND class_id = ? | |
| AND created_at BETWEEN ? AND ? | |
| ORDER BY created_at DESC;`; | |
| const res = await this._db?.query(query, [chapter_id, course_id, classId, startDate, endDate]); |
| const query = ` | ||
| SELECT DISTINCT school.* | ||
| FROM ${TABLES.SchoolUser} AS su | ||
| JOIN ${TABLES.School} AS school ON su.school_id = school.id | ||
| WHERE su.school_id IN (${placeholders}) | ||
| AND su.user_id = '${userId}' | ||
| AND su.role = '${RoleType.AUTOUSER}' | ||
| AND su.is_deleted = false; | ||
| `; | ||
|
|
||
| const res = await this._db?.query(query, schoolIds); |
There was a problem hiding this comment.
The userId and RoleType.AUTOUSER values are interpolated directly into the query string, which is a security risk. Please parameterize these values along with the schoolIds array.
| const query = ` | |
| SELECT DISTINCT school.* | |
| FROM ${TABLES.SchoolUser} AS su | |
| JOIN ${TABLES.School} AS school ON su.school_id = school.id | |
| WHERE su.school_id IN (${placeholders}) | |
| AND su.user_id = '${userId}' | |
| AND su.role = '${RoleType.AUTOUSER}' | |
| AND su.is_deleted = false; | |
| `; | |
| const res = await this._db?.query(query, schoolIds); | |
| const query = ` | |
| SELECT DISTINCT school.* | |
| FROM ${TABLES.SchoolUser} AS su | |
| JOIN ${TABLES.School} AS school ON su.school_id = school.id | |
| WHERE su.school_id IN (${placeholders}) | |
| AND su.user_id = ? | |
| AND su.role = ? | |
| AND su.is_deleted = 0; | |
| `; | |
| const res = await this._db?.query(query, [...schoolIds, userId, RoleType.AUTOUSER]); |
| const latestBatchQuery = ` | ||
| SELECT a.batch_id | ||
| FROM assignment a | ||
| LEFT JOIN assignment_user au | ||
| ON a.id = au.assignment_id | ||
| AND au.is_deleted = false | ||
| WHERE a.class_id = '${classId}' | ||
| AND a.course_id = '${courseId}' | ||
| AND a.type = 'assessment' | ||
| AND a.is_deleted = false | ||
| AND a.batch_id IS NOT NULL | ||
|
|
||
| -- Active time window | ||
| AND ( | ||
| a.starts_at IS NULL | ||
| OR a.starts_at = '' | ||
| OR datetime(a.starts_at) <= datetime('${nowIso}') | ||
| ) | ||
| AND ( | ||
| a.ends_at IS NULL | ||
| OR a.ends_at = '' | ||
| OR datetime(a.ends_at) > datetime('${nowIso}') | ||
| ) | ||
|
|
||
| -- Assigned to this student | ||
| AND ( | ||
| a.is_class_wise = true | ||
| OR au.user_id = '${studentId}' | ||
| ) | ||
|
|
||
| ORDER BY a.created_at DESC | ||
| LIMIT 1; | ||
| `; | ||
|
|
||
| const batchRes = await this._db?.query(latestBatchQuery); |
There was a problem hiding this comment.
This query is constructed using string interpolation for classId, courseId, nowIso, and studentId, which poses a SQL injection risk. Please parameterize the query using ? placeholders and pass the values as arguments to query().
const latestBatchQuery = `
SELECT a.batch_id
FROM assignment a
LEFT JOIN assignment_user au
ON a.id = au.assignment_id
AND au.is_deleted = 0
WHERE a.class_id = ?
AND a.course_id = ?
AND a.type = 'assessment'
AND a.is_deleted = 0
AND a.batch_id IS NOT NULL
-- Active time window
AND (
a.starts_at IS NULL
OR a.starts_at = ''
OR datetime(a.starts_at) <= datetime(?)
)
AND (
a.ends_at IS NULL
OR a.ends_at = ''
OR datetime(a.ends_at) > datetime(?)
)
-- Assigned to this student
AND (
a.is_class_wise = 1
OR au.user_id = ?
)
ORDER BY a.created_at DESC
LIMIT 1;
`;
const batchRes = await this._db?.query(latestBatchQuery, [
classId,
courseId,
nowIso,
nowIso,
studentId,
]);| this.executeQuery( | ||
| `UPDATE pull_sync_info SET last_pulled = '${formattedTimestamp}' WHERE table_name IN (${tables})`, | ||
| ); |
There was a problem hiding this comment.
The formattedTimestamp is interpolated directly into the SQL statement. To prevent potential SQL syntax issues and adhere to safe query practices, please parameterize this value using a ? placeholder.
| this.executeQuery( | |
| `UPDATE pull_sync_info SET last_pulled = '${formattedTimestamp}' WHERE table_name IN (${tables})`, | |
| ); | |
| this.executeQuery( | |
| `UPDATE pull_sync_info SET last_pulled = ? WHERE table_name IN (${tables})`, | |
| [formattedTimestamp], | |
| ); |
| async updateSoundFlag(userId: string, value: boolean) { | ||
| const query = ` | ||
| UPDATE "user" | ||
| SET sfx_off = ${value ? 1 : 0} | ||
| WHERE id = "${userId}"; | ||
| `; | ||
| await this.executeQuery(query); |
There was a problem hiding this comment.
The userId is interpolated directly into the query string. Please parameterize this query to prevent SQL injection and ensure robustness.
async updateSoundFlag(userId: string, value: boolean) {
const query = `
UPDATE "user"
SET sfx_off = ?
WHERE id = ?;
`;
await this.executeQuery(query, [value ? 1 : 0, userId]);| const quotedIds = ids.map((id) => `"${id}"`).join(', '); | ||
| try { | ||
| const res = await this._db?.query( | ||
| `SELECT * FROM ${TABLES.Badge} WHERE id IN (${quotedIds})`, | ||
| ); |
There was a problem hiding this comment.
Instead of manually quoting and interpolating the IDs into the query string, please use parameterized placeholders (?) to prevent SQL injection and syntax errors.
| const quotedIds = ids.map((id) => `"${id}"`).join(', '); | |
| try { | |
| const res = await this._db?.query( | |
| `SELECT * FROM ${TABLES.Badge} WHERE id IN (${quotedIds})`, | |
| ); | |
| const placeholders = ids.map(() => '?').join(', '); | |
| try { | |
| const res = await this._db?.query( | |
| `SELECT * FROM ${TABLES.Badge} WHERE id IN (${placeholders})`, | |
| ids, | |
| ); |
| async getStudentResult( | ||
| studentId: string, | ||
| fromCache?: boolean, | ||
| ): Promise<TableTypes<'result'>[]> { | ||
| await this.ensureInitialized(); | ||
| const query = ` | ||
| SELECT * FROM ${TABLES.Result} | ||
| where student_id = '${studentId}' | ||
| `; | ||
| const res = await this._db?.query(query); |
There was a problem hiding this comment.
The studentId is interpolated directly into the query string. Please parameterize this query to prevent SQL injection.
| async getStudentResult( | |
| studentId: string, | |
| fromCache?: boolean, | |
| ): Promise<TableTypes<'result'>[]> { | |
| await this.ensureInitialized(); | |
| const query = ` | |
| SELECT * FROM ${TABLES.Result} | |
| where student_id = '${studentId}' | |
| `; | |
| const res = await this._db?.query(query); | |
| async getStudentResult( | |
| studentId: string, | |
| fromCache?: boolean, | |
| ): Promise<TableTypes<'result'>[]> { | |
| await this.ensureInitialized(); | |
| const query = ` | |
| SELECT * FROM ${TABLES.Result} | |
| where student_id = ? | |
| `; | |
| const res = await this._db?.query(query, [studentId]); |
| lessonId: string, | ||
| classId?: string, | ||
| userId?: string, | ||
| ): Promise<String | undefined> { |
| ); | ||
|
|
||
| if (!assessmentLessonIds.length) { | ||
| return {} as TableTypes<'subject_lesson'>; |
…odules