From 4a1fc874c1d3b32e2b3f1e11862f6941c6e5de77 Mon Sep 17 00:00:00 2001 From: mariosemes Date: Thu, 26 Mar 2026 23:24:56 +0100 Subject: [PATCH] Security and code quality audit fixes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Security: - Fix SQL injection in updateStore — whitelist allowed field names - Restrict CORS to same-origin in production - Cap results at 200 per store to prevent memory issues Code quality: - Extract shared queryAll/queryOne to src/server/db/query.ts - Remove duplicated DB helpers from 5 files - Handle render_js boolean-to-integer conversion in updateStore UX: - Validate headers_json as valid JSON before saving (both forms) - Show error message if JSON is invalid Co-Authored-By: Claude Opus 4.6 (1M context) --- .../src/routes/admin/stores/[id]/+page.svelte | 1 + .../src/routes/admin/stores/new/+page.svelte | 2 ++ src/server/db/query.ts | 18 ++++++++++++ src/server/index.ts | 4 ++- src/server/models/category.ts | 18 +----------- src/server/models/scrape-log.ts | 18 +----------- src/server/models/store.ts | 29 +++++++------------ src/server/routes/health.ts | 14 +-------- src/server/scraper/engine.ts | 5 ++-- src/server/services/store-sync.ts | 17 +---------- 10 files changed, 41 insertions(+), 85 deletions(-) create mode 100644 src/server/db/query.ts diff --git a/src/client/src/routes/admin/stores/[id]/+page.svelte b/src/client/src/routes/admin/stores/[id]/+page.svelte index a906c32..bb6a943 100644 --- a/src/client/src/routes/admin/stores/[id]/+page.svelte +++ b/src/client/src/routes/admin/stores/[id]/+page.svelte @@ -37,6 +37,7 @@ try { const data = { ...form }; if (data.category_id) data.category_id = Number(data.category_id); else delete data.category_id; + if (data.headers_json) { try { JSON.parse(data.headers_json); } catch { error = 'Extra Headers must be valid JSON'; saving = false; return; } } await updateStore(Number($page.params.id), data); goto('/admin'); } catch (err) { error = err.message || 'Failed to update store'; } diff --git a/src/client/src/routes/admin/stores/new/+page.svelte b/src/client/src/routes/admin/stores/new/+page.svelte index 2db6dd4..e8b793b 100644 --- a/src/client/src/routes/admin/stores/new/+page.svelte +++ b/src/client/src/routes/admin/stores/new/+page.svelte @@ -27,6 +27,8 @@ if (!data.user_agent) delete data.user_agent; if (!data.proxy_url) delete data.proxy_url; if (!data.headers_json) delete data.headers_json; + else { try { JSON.parse(data.headers_json); } catch { error = 'Extra Headers must be valid JSON'; saving = false; return; } } + if (!data.test_query) delete data.test_query; const store = await createStore(data); goto(`/admin/stores/${store.id}/test`); } catch (err) { error = err.message || 'Failed to create store'; } diff --git a/src/server/db/query.ts b/src/server/db/query.ts new file mode 100644 index 0000000..c83c022 --- /dev/null +++ b/src/server/db/query.ts @@ -0,0 +1,18 @@ +import { getDatabase } from './connection.js'; + +export function queryAll(sql: string, params: any[] = []): any[] { + const db = getDatabase(); + const stmt = db.prepare(sql); + if (params.length) stmt.bind(params); + const rows: any[] = []; + while (stmt.step()) { + rows.push(stmt.getAsObject()); + } + stmt.free(); + return rows; +} + +export function queryOne(sql: string, params: any[] = []): any | undefined { + const rows = queryAll(sql, params); + return rows[0]; +} diff --git a/src/server/index.ts b/src/server/index.ts index 28a596e..5eb9f49 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -24,7 +24,9 @@ const app = Fastify({ }, }); -await app.register(cors, { origin: true }); +await app.register(cors, { + origin: config.isProduction ? false : true, +}); // API routes await app.register(storeRoutes, { prefix: '/api' }); diff --git a/src/server/models/category.ts b/src/server/models/category.ts index 85cd61a..fae65d7 100644 --- a/src/server/models/category.ts +++ b/src/server/models/category.ts @@ -1,4 +1,5 @@ import { getDatabase, saveDatabase } from '../db/connection.js'; +import { queryAll, queryOne } from '../db/query.js'; export interface Category { id: number; @@ -17,23 +18,6 @@ export interface StoreGroupWithMembers extends StoreGroup { store_ids: number[]; } -function queryAll(sql: string, params: any[] = []): any[] { - const db = getDatabase(); - const stmt = db.prepare(sql); - if (params.length) stmt.bind(params); - const rows: any[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject()); - } - stmt.free(); - return rows; -} - -function queryOne(sql: string, params: any[] = []): any | undefined { - const rows = queryAll(sql, params); - return rows[0]; -} - // Categories export function getAllCategories(): Category[] { diff --git a/src/server/models/scrape-log.ts b/src/server/models/scrape-log.ts index fdd6cce..34f624d 100644 --- a/src/server/models/scrape-log.ts +++ b/src/server/models/scrape-log.ts @@ -1,4 +1,5 @@ import { getDatabase, saveDatabase } from '../db/connection.js'; +import { queryAll, queryOne } from '../db/query.js'; export interface ScrapeLog { id: number; @@ -11,23 +12,6 @@ export interface ScrapeLog { scraped_at: string; } -function queryAll(sql: string, params: any[] = []): any[] { - const db = getDatabase(); - const stmt = db.prepare(sql); - if (params.length) stmt.bind(params); - const rows: any[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject()); - } - stmt.free(); - return rows; -} - -function queryOne(sql: string, params: any[] = []): any | undefined { - const rows = queryAll(sql, params); - return rows[0]; -} - export function logScrape( storeId: number, query: string, diff --git a/src/server/models/store.ts b/src/server/models/store.ts index 2b32637..25025a9 100644 --- a/src/server/models/store.ts +++ b/src/server/models/store.ts @@ -1,4 +1,5 @@ import { getDatabase, saveDatabase } from '../db/connection.js'; +import { queryAll, queryOne } from '../db/query.js'; export interface Store { id: number; @@ -53,23 +54,6 @@ export interface CreateStoreInput { category_id?: number; } -function queryAll(sql: string, params: any[] = []): any[] { - const db = getDatabase(); - const stmt = db.prepare(sql); - if (params.length) stmt.bind(params); - const rows: any[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject()); - } - stmt.free(); - return rows; -} - -function queryOne(sql: string, params: any[] = []): any | undefined { - const rows = queryAll(sql, params); - return rows[0]; -} - function slugify(text: string): string { return text .toLowerCase() @@ -148,10 +132,17 @@ export function updateStore(id: number, input: Partial): Store const fields: string[] = []; const values: any[] = []; + const allowedFields = new Set([ + 'name', 'slug', 'base_url', 'search_url', + 'sel_container', 'sel_name', 'sel_price', 'sel_link', 'sel_image', + 'render_js', 'test_query', 'rate_limit', 'rate_window', + 'proxy_url', 'user_agent', 'headers_json', 'currency', 'category_id', + ]); + for (const [key, value] of Object.entries(input)) { - if (value !== undefined) { + if (value !== undefined && allowedFields.has(key)) { fields.push(`${key} = ?`); - values.push(value); + values.push(key === 'render_js' ? (value ? 1 : 0) : value); } } diff --git a/src/server/routes/health.ts b/src/server/routes/health.ts index c204517..1ca3928 100644 --- a/src/server/routes/health.ts +++ b/src/server/routes/health.ts @@ -1,19 +1,7 @@ import type { FastifyPluginAsync } from 'fastify'; import fs from 'node:fs'; import { config } from '../config.js'; -import { getDatabase } from '../db/connection.js'; - -function queryOne(sql: string, params: any[] = []): any | undefined { - const db = getDatabase(); - const stmt = db.prepare(sql); - if (params.length) stmt.bind(params); - let result: any; - if (stmt.step()) { - result = stmt.getAsObject(); - } - stmt.free(); - return result; -} +import { queryOne } from '../db/query.js'; export const healthRoutes: FastifyPluginAsync = async (app) => { app.get('/health', async () => { diff --git a/src/server/scraper/engine.ts b/src/server/scraper/engine.ts index 08e93b7..b02ee3a 100644 --- a/src/server/scraper/engine.ts +++ b/src/server/scraper/engine.ts @@ -9,6 +9,7 @@ import { getLimiter } from './rate-limiter.js'; const MAX_CONCURRENCY = 5; const SEARCH_TIMEOUT = 60_000; +const MAX_RESULTS_PER_STORE = 200; export interface SearchOptions { query: string; @@ -85,7 +86,7 @@ export async function search(options: SearchOptions): Promise { const result = await rateLimiter.schedule(scrapeFn); const duration = Date.now() - storeStart; - const products = result.items.map((item) => + const products = result.items.slice(0, MAX_RESULTS_PER_STORE).map((item) => normalizeResult(item, store.id, store.name, store.base_url, store.currency) ); @@ -185,7 +186,7 @@ export async function searchStreaming( const result = await rateLimiter.schedule(scrapeFn); const duration = Date.now() - storeStart; - const products = result.items.map((item) => + const products = result.items.slice(0, MAX_RESULTS_PER_STORE).map((item) => normalizeResult(item, store.id, store.name, store.base_url, store.currency) ); diff --git a/src/server/services/store-sync.ts b/src/server/services/store-sync.ts index 715966d..b13c80a 100644 --- a/src/server/services/store-sync.ts +++ b/src/server/services/store-sync.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import path from 'node:path'; import YAML from 'yaml'; import { getDatabase, saveDatabase } from '../db/connection.js'; +import { queryAll, queryOne } from '../db/query.js'; export interface StoreFileConfig { name: string; @@ -26,22 +27,6 @@ export interface StoreFileConfig { headers?: Record; } -function queryAll(sql: string, params: any[] = []): any[] { - const db = getDatabase(); - const stmt = db.prepare(sql); - if (params.length) stmt.bind(params); - const rows: any[] = []; - while (stmt.step()) { - rows.push(stmt.getAsObject()); - } - stmt.free(); - return rows; -} - -function queryOne(sql: string, params: any[] = []): any | undefined { - const rows = queryAll(sql, params); - return rows[0]; -} function slugify(text: string): string { return text