fix: Security and code quality bug fixes

Security:
- Fix potential SQL injection in Savepoint class by sanitizing savepoint names
  - Only allow alphanumeric characters and underscores
  - Prefix with "sp_" if name starts with number
  - Limit to 63 characters (PostgreSQL identifier limit)

Code quality:
- Add missing radix parameter to parseInt calls in:
  - collections.ts (4 occurrences)
  - groups.ts (1 occurrence)
  - revisions.ts (1 occurrence)
  - users.ts (1 occurrence)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
2026-01-31 15:36:07 +00:00
parent b4ba42cbf1
commit 22601e1680
5 changed files with 32 additions and 8 deletions

View File

@@ -93,7 +93,7 @@ export const collectionsTools: BaseTool<any>[] = [
countQuery += ` AND "teamId" = $1`; countQuery += ` AND "teamId" = $1`;
} }
const countResult = await pool.query(countQuery, countParams); const countResult = await pool.query(countQuery, countParams);
const totalCount = parseInt(countResult.rows[0].count); const totalCount = parseInt(countResult.rows[0].count, 10);
return { return {
content: [ content: [
@@ -614,7 +614,7 @@ export const collectionsTools: BaseTool<any>[] = [
// Get total count // Get total count
const countQuery = 'SELECT COUNT(*) FROM documents WHERE "collectionId" = $1 AND "deletedAt" IS NULL'; const countQuery = 'SELECT COUNT(*) FROM documents WHERE "collectionId" = $1 AND "deletedAt" IS NULL';
const countResult = await pool.query(countQuery, [collectionId]); const countResult = await pool.query(countQuery, [collectionId]);
const totalCount = parseInt(countResult.rows[0].count); const totalCount = parseInt(countResult.rows[0].count, 10);
return { return {
content: [ content: [
@@ -842,7 +842,7 @@ export const collectionsTools: BaseTool<any>[] = [
// Get total count // Get total count
const countQuery = 'SELECT COUNT(*) FROM collection_users WHERE "collectionId" = $1'; const countQuery = 'SELECT COUNT(*) FROM collection_users WHERE "collectionId" = $1';
const countResult = await pool.query(countQuery, [collectionId]); const countResult = await pool.query(countQuery, [collectionId]);
const totalCount = parseInt(countResult.rows[0].count); const totalCount = parseInt(countResult.rows[0].count, 10);
return { return {
content: [ content: [
@@ -1069,7 +1069,7 @@ export const collectionsTools: BaseTool<any>[] = [
// Get total count // Get total count
const countQuery = 'SELECT COUNT(*) FROM collection_groups WHERE "collectionId" = $1'; const countQuery = 'SELECT COUNT(*) FROM collection_groups WHERE "collectionId" = $1';
const countResult = await pool.query(countQuery, [collectionId]); const countResult = await pool.query(countQuery, [collectionId]);
const totalCount = parseInt(countResult.rows[0].count); const totalCount = parseInt(countResult.rows[0].count, 10);
return { return {
content: [ content: [

View File

@@ -80,7 +80,7 @@ const listGroups: BaseTool<GroupArgs> = {
{ {
data: { data: {
groups: result.rows, groups: result.rows,
total: result.rows.length > 0 ? parseInt(result.rows[0].total) : 0, total: result.rows.length > 0 ? parseInt(result.rows[0].total, 10) : 0,
limit, limit,
offset, offset,
}, },

View File

@@ -87,7 +87,7 @@ const listRevisions: BaseTool<RevisionArgs> = {
pagination: { pagination: {
limit, limit,
offset, offset,
total: parseInt(countQuery.rows[0].total), total: parseInt(countQuery.rows[0].total, 10),
}, },
}, },
null, null,

View File

@@ -109,7 +109,7 @@ const listUsers: BaseTool<UserArgs> = {
{ {
data: { data: {
users: result.rows, users: result.rows,
total: result.rows.length > 0 ? parseInt(result.rows[0].total) : 0, total: result.rows.length > 0 ? parseInt(result.rows[0].total, 10) : 0,
limit, limit,
offset, offset,
}, },

View File

@@ -235,6 +235,30 @@ export async function withReadOnlyTransaction<T>(
} }
} }
/**
* Validate and sanitize savepoint name to prevent SQL injection
* Only allows alphanumeric characters and underscores
*/
function sanitizeSavepointName(name: string): string {
// Remove any non-alphanumeric characters except underscore
const sanitized = name.replace(/[^a-zA-Z0-9_]/g, '');
if (sanitized.length === 0) {
throw new Error('Savepoint name must contain at least one alphanumeric character');
}
if (sanitized.length > 63) {
throw new Error('Savepoint name must be 63 characters or less');
}
// Ensure it doesn't start with a number
if (/^[0-9]/.test(sanitized)) {
return `sp_${sanitized}`;
}
return sanitized;
}
/** /**
* Savepoint helper for nested transaction-like behavior * Savepoint helper for nested transaction-like behavior
*/ */
@@ -245,7 +269,7 @@ export class Savepoint {
constructor(client: PoolClient, name: string) { constructor(client: PoolClient, name: string) {
this.client = client; this.client = client;
this.name = name; this.name = sanitizeSavepointName(name);
} }
/** /**