fix(security): Resolve 21 SQL injection vulnerabilities and add transactions
Security fixes (v1.2.2): - Fix SQL injection in analytics.ts (16 occurrences) - Fix SQL injection in advanced-search.ts (1 occurrence) - Fix SQL injection in search-queries.ts (1 occurrence) - Add validateDaysInterval(), isValidISODate(), validatePeriod() to security.ts - Use make_interval(days => N) for safe PostgreSQL intervals - Validate UUIDs BEFORE string construction Transaction support: - bulk-operations.ts: 6 atomic operations with withTransaction() - desk-sync.ts: 2 operations with transactions - export-import.ts: 1 operation with transaction Rate limiting: - Add automatic cleanup of expired entries (every 5 minutes) Audit: - Archive previous audit docs to docs/audits/2026-01-31-v1.2.1/ - Create new AUDIT-REQUEST.md for v1.2.2 verification Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -6,7 +6,7 @@
|
||||
|
||||
import { Pool } from 'pg';
|
||||
import { BaseTool, ToolResponse } from '../types/tools.js';
|
||||
import { isValidUUID } from '../utils/security.js';
|
||||
import { isValidUUID, validateDaysInterval, isValidISODate, validatePeriod } from '../utils/security.js';
|
||||
|
||||
interface DateRangeArgs {
|
||||
date_from?: string;
|
||||
@@ -27,11 +27,23 @@ const getAnalyticsOverview: BaseTool<DateRangeArgs> = {
|
||||
},
|
||||
},
|
||||
handler: async (args, pgClient): Promise<ToolResponse> => {
|
||||
const dateCondition = args.date_from && args.date_to
|
||||
? `AND "createdAt" BETWEEN '${args.date_from}' AND '${args.date_to}'`
|
||||
: '';
|
||||
// Validate date parameters if provided
|
||||
if (args.date_from && !isValidISODate(args.date_from)) {
|
||||
throw new Error('Invalid date_from format. Use ISO format (YYYY-MM-DD)');
|
||||
}
|
||||
if (args.date_to && !isValidISODate(args.date_to)) {
|
||||
throw new Error('Invalid date_to format. Use ISO format (YYYY-MM-DD)');
|
||||
}
|
||||
|
||||
// Document stats
|
||||
// Build date condition with parameterized query
|
||||
const dateParams: any[] = [];
|
||||
let dateCondition = '';
|
||||
if (args.date_from && args.date_to) {
|
||||
dateCondition = `AND "createdAt" BETWEEN $1 AND $2`;
|
||||
dateParams.push(args.date_from, args.date_to);
|
||||
}
|
||||
|
||||
// Document stats (no date filter needed for totals)
|
||||
const docStats = await pgClient.query(`
|
||||
SELECT
|
||||
COUNT(*) as "totalDocuments",
|
||||
@@ -105,19 +117,32 @@ const getUserActivityAnalytics: BaseTool<{ user_id?: string; days?: number }> =
|
||||
},
|
||||
},
|
||||
handler: async (args, pgClient): Promise<ToolResponse> => {
|
||||
const days = args.days || 30;
|
||||
const userCondition = args.user_id ? `AND u.id = '${args.user_id}'` : '';
|
||||
// Validate user_id FIRST before using it
|
||||
if (args.user_id && !isValidUUID(args.user_id)) {
|
||||
throw new Error('Invalid user_id');
|
||||
}
|
||||
|
||||
if (args.user_id && !isValidUUID(args.user_id)) throw new Error('Invalid user_id');
|
||||
// Validate and sanitize days parameter
|
||||
const safeDays = validateDaysInterval(args.days, 30, 365);
|
||||
|
||||
// Most active users
|
||||
// Build query with safe interval (number is safe after validation)
|
||||
const params: any[] = [];
|
||||
let paramIdx = 1;
|
||||
let userCondition = '';
|
||||
|
||||
if (args.user_id) {
|
||||
userCondition = `AND u.id = $${paramIdx++}`;
|
||||
params.push(args.user_id);
|
||||
}
|
||||
|
||||
// Most active users - using make_interval for safety
|
||||
const activeUsers = await pgClient.query(`
|
||||
SELECT
|
||||
u.id, u.name, u.email,
|
||||
COUNT(DISTINCT d.id) FILTER (WHERE d."createdAt" >= NOW() - INTERVAL '${days} days') as "documentsCreated",
|
||||
COUNT(DISTINCT d2.id) FILTER (WHERE d2."updatedAt" >= NOW() - INTERVAL '${days} days') as "documentsEdited",
|
||||
COUNT(DISTINCT v."documentId") FILTER (WHERE v."createdAt" >= NOW() - INTERVAL '${days} days') as "documentsViewed",
|
||||
COUNT(DISTINCT c.id) FILTER (WHERE c."createdAt" >= NOW() - INTERVAL '${days} days') as "commentsAdded"
|
||||
COUNT(DISTINCT d.id) FILTER (WHERE d."createdAt" >= NOW() - make_interval(days => ${safeDays})) as "documentsCreated",
|
||||
COUNT(DISTINCT d2.id) FILTER (WHERE d2."updatedAt" >= NOW() - make_interval(days => ${safeDays})) as "documentsEdited",
|
||||
COUNT(DISTINCT v."documentId") FILTER (WHERE v."createdAt" >= NOW() - make_interval(days => ${safeDays})) as "documentsViewed",
|
||||
COUNT(DISTINCT c.id) FILTER (WHERE c."createdAt" >= NOW() - make_interval(days => ${safeDays})) as "commentsAdded"
|
||||
FROM users u
|
||||
LEFT JOIN documents d ON d."createdById" = u.id
|
||||
LEFT JOIN documents d2 ON d2."lastModifiedById" = u.id
|
||||
@@ -127,7 +152,7 @@ const getUserActivityAnalytics: BaseTool<{ user_id?: string; days?: number }> =
|
||||
GROUP BY u.id, u.name, u.email
|
||||
ORDER BY "documentsCreated" DESC
|
||||
LIMIT 20
|
||||
`);
|
||||
`, params);
|
||||
|
||||
// Activity by day of week
|
||||
const activityByDay = await pgClient.query(`
|
||||
@@ -135,7 +160,7 @@ const getUserActivityAnalytics: BaseTool<{ user_id?: string; days?: number }> =
|
||||
EXTRACT(DOW FROM d."createdAt") as "dayOfWeek",
|
||||
COUNT(*) as "documentsCreated"
|
||||
FROM documents d
|
||||
WHERE d."createdAt" >= NOW() - INTERVAL '${days} days'
|
||||
WHERE d."createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
AND d."deletedAt" IS NULL
|
||||
GROUP BY EXTRACT(DOW FROM d."createdAt")
|
||||
ORDER BY "dayOfWeek"
|
||||
@@ -147,7 +172,7 @@ const getUserActivityAnalytics: BaseTool<{ user_id?: string; days?: number }> =
|
||||
EXTRACT(HOUR FROM d."createdAt") as "hour",
|
||||
COUNT(*) as "documentsCreated"
|
||||
FROM documents d
|
||||
WHERE d."createdAt" >= NOW() - INTERVAL '${days} days'
|
||||
WHERE d."createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
AND d."deletedAt" IS NULL
|
||||
GROUP BY EXTRACT(HOUR FROM d."createdAt")
|
||||
ORDER BY "hour"
|
||||
@@ -158,7 +183,7 @@ const getUserActivityAnalytics: BaseTool<{ user_id?: string; days?: number }> =
|
||||
activeUsers: activeUsers.rows,
|
||||
activityByDayOfWeek: activityByDay.rows,
|
||||
activityByHour: activityByHour.rows,
|
||||
periodDays: days,
|
||||
periodDays: safeDays,
|
||||
}, null, 2) }],
|
||||
};
|
||||
},
|
||||
@@ -177,11 +202,20 @@ const getContentInsights: BaseTool<{ collection_id?: string }> = {
|
||||
},
|
||||
},
|
||||
handler: async (args, pgClient): Promise<ToolResponse> => {
|
||||
const collectionCondition = args.collection_id
|
||||
? `AND d."collectionId" = '${args.collection_id}'`
|
||||
: '';
|
||||
// Validate collection_id FIRST before using it
|
||||
if (args.collection_id && !isValidUUID(args.collection_id)) {
|
||||
throw new Error('Invalid collection_id');
|
||||
}
|
||||
|
||||
if (args.collection_id && !isValidUUID(args.collection_id)) throw new Error('Invalid collection_id');
|
||||
// Build parameterized query
|
||||
const params: any[] = [];
|
||||
let paramIdx = 1;
|
||||
let collectionCondition = '';
|
||||
|
||||
if (args.collection_id) {
|
||||
collectionCondition = `AND d."collectionId" = $${paramIdx++}`;
|
||||
params.push(args.collection_id);
|
||||
}
|
||||
|
||||
// Most viewed documents
|
||||
const mostViewed = await pgClient.query(`
|
||||
@@ -196,7 +230,7 @@ const getContentInsights: BaseTool<{ collection_id?: string }> = {
|
||||
GROUP BY d.id, d.title, d.emoji, c.name
|
||||
ORDER BY "viewCount" DESC
|
||||
LIMIT 10
|
||||
`);
|
||||
`, params);
|
||||
|
||||
// Most starred documents
|
||||
const mostStarred = await pgClient.query(`
|
||||
@@ -211,7 +245,7 @@ const getContentInsights: BaseTool<{ collection_id?: string }> = {
|
||||
HAVING COUNT(s.id) > 0
|
||||
ORDER BY "starCount" DESC
|
||||
LIMIT 10
|
||||
`);
|
||||
`, params);
|
||||
|
||||
// Stale documents (not updated in 90 days)
|
||||
const staleDocuments = await pgClient.query(`
|
||||
@@ -228,7 +262,7 @@ const getContentInsights: BaseTool<{ collection_id?: string }> = {
|
||||
${collectionCondition}
|
||||
ORDER BY d."updatedAt" ASC
|
||||
LIMIT 20
|
||||
`);
|
||||
`, params);
|
||||
|
||||
// Documents without views
|
||||
const neverViewed = await pgClient.query(`
|
||||
@@ -244,7 +278,7 @@ const getContentInsights: BaseTool<{ collection_id?: string }> = {
|
||||
${collectionCondition}
|
||||
ORDER BY d."createdAt" DESC
|
||||
LIMIT 20
|
||||
`);
|
||||
`, params);
|
||||
|
||||
return {
|
||||
content: [{ type: 'text', text: JSON.stringify({
|
||||
@@ -270,11 +304,19 @@ const getCollectionStats: BaseTool<{ collection_id?: string }> = {
|
||||
},
|
||||
},
|
||||
handler: async (args, pgClient): Promise<ToolResponse> => {
|
||||
const collectionCondition = args.collection_id
|
||||
? `AND c.id = '${args.collection_id}'`
|
||||
: '';
|
||||
// Validate collection_id FIRST before using it
|
||||
if (args.collection_id && !isValidUUID(args.collection_id)) {
|
||||
throw new Error('Invalid collection_id');
|
||||
}
|
||||
|
||||
if (args.collection_id && !isValidUUID(args.collection_id)) throw new Error('Invalid collection_id');
|
||||
// Build parameterized query
|
||||
const params: any[] = [];
|
||||
let collectionCondition = '';
|
||||
|
||||
if (args.collection_id) {
|
||||
collectionCondition = `AND c.id = $1`;
|
||||
params.push(args.collection_id);
|
||||
}
|
||||
|
||||
const stats = await pgClient.query(`
|
||||
SELECT
|
||||
@@ -293,7 +335,7 @@ const getCollectionStats: BaseTool<{ collection_id?: string }> = {
|
||||
WHERE c."deletedAt" IS NULL ${collectionCondition}
|
||||
GROUP BY c.id, c.name, c.icon, c.color
|
||||
ORDER BY "documentCount" DESC
|
||||
`);
|
||||
`, params);
|
||||
|
||||
return {
|
||||
content: [{ type: 'text', text: JSON.stringify({ data: stats.rows }, null, 2) }],
|
||||
@@ -314,14 +356,18 @@ const getGrowthMetrics: BaseTool<{ period?: string }> = {
|
||||
},
|
||||
},
|
||||
handler: async (args, pgClient): Promise<ToolResponse> => {
|
||||
const period = args.period || 'month';
|
||||
const intervals: Record<string, string> = {
|
||||
week: '7 days',
|
||||
month: '30 days',
|
||||
quarter: '90 days',
|
||||
year: '365 days',
|
||||
// Validate period against allowed values
|
||||
const allowedPeriods = ['week', 'month', 'quarter', 'year'];
|
||||
const period = validatePeriod(args.period, allowedPeriods, 'month');
|
||||
|
||||
// Map periods to safe integer days (no string interpolation needed)
|
||||
const periodDays: Record<string, number> = {
|
||||
week: 7,
|
||||
month: 30,
|
||||
quarter: 90,
|
||||
year: 365,
|
||||
};
|
||||
const interval = intervals[period] || '30 days';
|
||||
const safeDays = periodDays[period];
|
||||
|
||||
// Document growth by day
|
||||
const documentGrowth = await pgClient.query(`
|
||||
@@ -330,7 +376,7 @@ const getGrowthMetrics: BaseTool<{ period?: string }> = {
|
||||
COUNT(*) as "newDocuments",
|
||||
SUM(COUNT(*)) OVER (ORDER BY DATE(d."createdAt")) as "cumulativeDocuments"
|
||||
FROM documents d
|
||||
WHERE d."createdAt" >= NOW() - INTERVAL '${interval}'
|
||||
WHERE d."createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
AND d."deletedAt" IS NULL
|
||||
GROUP BY DATE(d."createdAt")
|
||||
ORDER BY date
|
||||
@@ -343,7 +389,7 @@ const getGrowthMetrics: BaseTool<{ period?: string }> = {
|
||||
COUNT(*) as "newUsers",
|
||||
SUM(COUNT(*)) OVER (ORDER BY DATE(u."createdAt")) as "cumulativeUsers"
|
||||
FROM users u
|
||||
WHERE u."createdAt" >= NOW() - INTERVAL '${interval}'
|
||||
WHERE u."createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
AND u."deletedAt" IS NULL
|
||||
GROUP BY DATE(u."createdAt")
|
||||
ORDER BY date
|
||||
@@ -355,7 +401,7 @@ const getGrowthMetrics: BaseTool<{ period?: string }> = {
|
||||
DATE(c."createdAt") as date,
|
||||
COUNT(*) as "newCollections"
|
||||
FROM collections c
|
||||
WHERE c."createdAt" >= NOW() - INTERVAL '${interval}'
|
||||
WHERE c."createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
AND c."deletedAt" IS NULL
|
||||
GROUP BY DATE(c."createdAt")
|
||||
ORDER BY date
|
||||
@@ -364,10 +410,10 @@ const getGrowthMetrics: BaseTool<{ period?: string }> = {
|
||||
// Period comparison
|
||||
const comparison = await pgClient.query(`
|
||||
SELECT
|
||||
(SELECT COUNT(*) FROM documents WHERE "createdAt" >= NOW() - INTERVAL '${interval}' AND "deletedAt" IS NULL) as "currentPeriodDocs",
|
||||
(SELECT COUNT(*) FROM documents WHERE "createdAt" >= NOW() - INTERVAL '${interval}' * 2 AND "createdAt" < NOW() - INTERVAL '${interval}' AND "deletedAt" IS NULL) as "previousPeriodDocs",
|
||||
(SELECT COUNT(*) FROM users WHERE "createdAt" >= NOW() - INTERVAL '${interval}' AND "deletedAt" IS NULL) as "currentPeriodUsers",
|
||||
(SELECT COUNT(*) FROM users WHERE "createdAt" >= NOW() - INTERVAL '${interval}' * 2 AND "createdAt" < NOW() - INTERVAL '${interval}' AND "deletedAt" IS NULL) as "previousPeriodUsers"
|
||||
(SELECT COUNT(*) FROM documents WHERE "createdAt" >= NOW() - make_interval(days => ${safeDays}) AND "deletedAt" IS NULL) as "currentPeriodDocs",
|
||||
(SELECT COUNT(*) FROM documents WHERE "createdAt" >= NOW() - make_interval(days => ${safeDays * 2}) AND "createdAt" < NOW() - make_interval(days => ${safeDays}) AND "deletedAt" IS NULL) as "previousPeriodDocs",
|
||||
(SELECT COUNT(*) FROM users WHERE "createdAt" >= NOW() - make_interval(days => ${safeDays}) AND "deletedAt" IS NULL) as "currentPeriodUsers",
|
||||
(SELECT COUNT(*) FROM users WHERE "createdAt" >= NOW() - make_interval(days => ${safeDays * 2}) AND "createdAt" < NOW() - make_interval(days => ${safeDays}) AND "deletedAt" IS NULL) as "previousPeriodUsers"
|
||||
`);
|
||||
|
||||
return {
|
||||
@@ -395,7 +441,8 @@ const getSearchAnalytics: BaseTool<{ days?: number }> = {
|
||||
},
|
||||
},
|
||||
handler: async (args, pgClient): Promise<ToolResponse> => {
|
||||
const days = args.days || 30;
|
||||
// Validate and sanitize days parameter
|
||||
const safeDays = validateDaysInterval(args.days, 30, 365);
|
||||
|
||||
// Popular search queries
|
||||
const popularQueries = await pgClient.query(`
|
||||
@@ -404,7 +451,7 @@ const getSearchAnalytics: BaseTool<{ days?: number }> = {
|
||||
COUNT(*) as "searchCount",
|
||||
COUNT(DISTINCT "userId") as "uniqueSearchers"
|
||||
FROM search_queries
|
||||
WHERE "createdAt" >= NOW() - INTERVAL '${days} days'
|
||||
WHERE "createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
GROUP BY query
|
||||
ORDER BY "searchCount" DESC
|
||||
LIMIT 20
|
||||
@@ -417,7 +464,7 @@ const getSearchAnalytics: BaseTool<{ days?: number }> = {
|
||||
COUNT(*) as "searches",
|
||||
COUNT(DISTINCT "userId") as "uniqueSearchers"
|
||||
FROM search_queries
|
||||
WHERE "createdAt" >= NOW() - INTERVAL '${days} days'
|
||||
WHERE "createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
GROUP BY DATE("createdAt")
|
||||
ORDER BY date
|
||||
`);
|
||||
@@ -428,7 +475,7 @@ const getSearchAnalytics: BaseTool<{ days?: number }> = {
|
||||
query,
|
||||
COUNT(*) as "searchCount"
|
||||
FROM search_queries
|
||||
WHERE "createdAt" >= NOW() - INTERVAL '${days} days'
|
||||
WHERE "createdAt" >= NOW() - make_interval(days => ${safeDays})
|
||||
AND results = 0
|
||||
GROUP BY query
|
||||
ORDER BY "searchCount" DESC
|
||||
@@ -440,7 +487,7 @@ const getSearchAnalytics: BaseTool<{ days?: number }> = {
|
||||
popularQueries: popularQueries.rows,
|
||||
searchVolume: searchVolume.rows,
|
||||
zeroResultQueries: zeroResults.rows,
|
||||
periodDays: days,
|
||||
periodDays: safeDays,
|
||||
}, null, 2) }],
|
||||
};
|
||||
},
|
||||
|
||||
Reference in New Issue
Block a user