diff --git a/apps/web/app/lib/json-ld.test.ts b/apps/web/app/lib/json-ld.test.ts new file mode 100644 index 000000000..0adacaf95 --- /dev/null +++ b/apps/web/app/lib/json-ld.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest'; +import { serializeJsonForScript } from './json-ld'; + +const LS = String.fromCharCode(0x2028); // U+2028 line separator +const PS = String.fromCharCode(0x2029); // U+2029 paragraph separator + +describe('serializeJsonForScript', () => { + it('escapes < so a string value cannot close the ' }); + // The raw breakout sequence must not survive — this is the whole point of the sink. + expect(out).not.toContain(''); + expect(out).not.toContain(' B', + nested: ['', { k: '' }], + n: 42, + }; + expect(JSON.parse(serializeJsonForScript(value))).toEqual(value); + }); + + it('escapes the U+2028 / U+2029 line/paragraph separators', () => { + const value = { s: `a${LS}b${PS}c` }; + const out = serializeJsonForScript(value); + expect(out).toContain('\\u2028'); + expect(out).toContain('\\u2029'); + expect(out).not.toContain(LS); + expect(out).not.toContain(PS); + // Still round-trips to the original value. + expect(JSON.parse(out)).toEqual(value); + }); + + it('leaves injection-free content byte-identical to JSON.stringify', () => { + const value = { '@context': 'https://schema.org', name: 'СИГМА', url: 'https://sigma.midt.bg' }; + expect(serializeJsonForScript(value)).toBe(JSON.stringify(value)); + }); + + it('does NOT escape > or & (only < can break out of a script raw-text context)', () => { + const out = serializeJsonForScript({ s: 'a > b && c' }); + expect(out).toContain('a > b && c'); // left verbatim, byte-minimal + expect(out).not.toContain('&'); + expect(out).not.toContain('>'); + }); + + it('returns valid JSON (not a throw) for values that stringify to undefined', () => { + // JSON.stringify(undefined | function | symbol) === undefined; the helper must not call .replace + // on it. Emitting "null" keeps the ` in any string value would close the +// script element early and inject markup (stored XSS). Escaping `<` as \u003c is JSON-equivalent +// — JSON.parse returns the identical value — and closes that hole; the U+2028/U+2029 escapes keep +// the payload safe if a consumer evaluates it as JS rather than parsing it as JSON. +// +// `>` and `&` are deliberately NOT escaped: only `<` can start a markup/comment token in a script +// raw-text context (``/`&` need no escaping — leaving them keeps the output byte-minimal and still valid JSON. +// +// This is the SINGLE shared implementation of the project's review standard (docs/review-security.md +// "Инжекции и валидация"): both root.tsx's JSON-LD island and routes/contract.json.tsx's response use +// it, so the two sinks cannot drift. Kept as defense-in-depth — today the only value reaching the +// JSON-LD is the request origin (which `new URL()` cannot make carry ``), but this keeps the +// sink safe for any DB/user-derived field added later. +export function serializeJsonForScript(value: unknown): string { + // JSON.stringify returns `undefined` (not a string) for `undefined`, a function, or a symbol \u2014 a + // later `.replace` on it would throw. Emit valid JSON (`null`) instead, so the helper is safe for + // any value even though today's callers always pass an object. + const json = JSON.stringify(value); + if (json === undefined) return 'null'; + return json + .replace(/ DESC, bidder_id DESC (queries/companies.ts). +CREATE INDEX IF NOT EXISTS idx_company_totals_count + ON company_totals(contracts DESC, bidder_id DESC); +CREATE INDEX IF NOT EXISTS idx_company_totals_authorities + ON company_totals(authorities DESC, bidder_id DESC); + +-- /authorities ?sort=count | avg — ORDER BY DESC, authority_id DESC (queries/authorities.ts). +CREATE INDEX IF NOT EXISTS idx_authority_totals_count + ON authority_totals(contracts DESC, authority_id DESC); +CREATE INDEX IF NOT EXISTS idx_authority_totals_avg + ON authority_totals(avg_eur DESC, authority_id DESC); diff --git a/packages/db/src/list-sort-indexes.test.ts b/packages/db/src/list-sort-indexes.test.ts new file mode 100644 index 000000000..95d407118 --- /dev/null +++ b/packages/db/src/list-sort-indexes.test.ts @@ -0,0 +1,207 @@ +/// +import { execFileSync } from 'node:child_process'; +import { mkdtempSync, readdirSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +// The list pages paginate with a keyset ORDER BY , LIMIT N. When the sort +// column/expression has a matching index, SQLite walks it and stops at LIMIT. When it does NOT, the +// planner falls back to "SCAN … USE TEMP B-TREE FOR ORDER BY": it reads and sorts the WHOLE +// table before applying LIMIT — a full-corpus scan on every request. D1 bills on rows SCANNED, so a +// missing ordering index is a real cost/latency defect (docs/review-security.md "D1 разход и индекси"). +// +// This proves, on a real sqlite3 with no ANALYZE stats (matching production D1), that BEFORE the +// list-sort-indexes migration the six non-default list sorts full-scan (temp-B-tree sort), and AFTER +// it each walks its new index with no ORDER BY sort step — on the first page AND on a keyset page. +// +// Known boundaries of this guarantee: +// - The local sqlite3 CLI's query planner is not version-identical to Cloudflare D1's; the EXPLAIN +// plans are a strong indication, not a bit-exact production proof. (The sqlite3 binary itself is a +// pre-existing suite-wide dependency — migrations/refresh-slice/ship-domain tests all exec it — so +// a missing binary fails the whole suite, not just this file.) +// - The index-walk claim is asserted for the UNFILTERED sort paths (the default list views) and, for +// contracts, with an active list filter as well (FILTERED_SORTS below): the planner keeps walking the +// ordering index and still drops the sort step, so a filtered page is not a full-corpus sort either. + +const root = resolve(dirname(fileURLToPath(import.meta.url)), '../../..'); +const migrationsDir = resolve(root, 'packages/db/migrations'); + +// Apply EVERY migration on the branch, not a hardcoded subset — so the "BEFORE" base is exactly the +// real served schema minus this PR's index, and the test survives any renumbering. +const allMigrations = readdirSync(migrationsDir) + .filter((f) => f.endsWith('.sql')) + .sort(); +const sortIndexMigration = allMigrations.find((f) => f.includes('list_sort_indexes')); +if (!sortIndexMigration) throw new Error('list_sort_indexes migration not found'); +const baseMigrations = allMigrations.filter((f) => f !== sortIndexMigration); + +function readScript(dbPath: string, file: string): void { + execFileSync('sqlite3', ['-bail', dbPath], { + input: `.read ${resolve(migrationsDir, file)}\n`, + stdio: 'pipe', + }); +} + +function plan(dbPath: string, sql: string): string { + return execFileSync('sqlite3', [dbPath], { + input: `EXPLAIN QUERY PLAN ${sql}\n`, + encoding: 'utf8', + }); +} + +// Faithful shapes of the keyset list queries (queries/{contracts,companies,authorities}.ts): the same +// FROM/JOINs and the same ORDER BY expression, on the first page (no cursor) and on a keyset page +// (the `WHERE (expr ? OR (expr = ? AND id ?))` seek every page after the first uses). +const CONTRACTS_FROM = + 'FROM contracts c JOIN tenders t ON t.id = c.tender_id ' + + 'JOIN authorities a ON a.id = t.authority_id JOIN bidders b ON b.id = c.bidder_id'; + +const SORTS = [ + { + name: 'contracts date-desc', + index: 'idx_contracts_signed_desc', + firstPage: `SELECT c.id ${CONTRACTS_FROM} ORDER BY COALESCE(c.signed_at, '') DESC, c.id DESC LIMIT 16`, + keysetPage: `SELECT c.id ${CONTRACTS_FROM} WHERE (COALESCE(c.signed_at, '') < '2023-05-20' OR (COALESCE(c.signed_at, '') = '2023-05-20' AND c.id < 'c:500')) ORDER BY COALESCE(c.signed_at, '') DESC, c.id DESC LIMIT 16`, + }, + { + name: 'contracts date-asc', + index: 'idx_contracts_signed_asc', + firstPage: `SELECT c.id ${CONTRACTS_FROM} ORDER BY COALESCE(c.signed_at, '9999-99') ASC, c.id ASC LIMIT 16`, + keysetPage: `SELECT c.id ${CONTRACTS_FROM} WHERE (COALESCE(c.signed_at, '9999-99') > '2023-05-20' OR (COALESCE(c.signed_at, '9999-99') = '2023-05-20' AND c.id > 'c:500')) ORDER BY COALESCE(c.signed_at, '9999-99') ASC, c.id ASC LIMIT 16`, + }, + { + name: 'companies count-desc', + index: 'idx_company_totals_count', + firstPage: `SELECT bidder_id FROM company_totals ORDER BY contracts DESC, bidder_id DESC LIMIT 26`, + keysetPage: `SELECT bidder_id FROM company_totals WHERE (contracts < 10 OR (contracts = 10 AND bidder_id < 'eik:100')) ORDER BY contracts DESC, bidder_id DESC LIMIT 26`, + }, + { + name: 'companies authorities-desc', + index: 'idx_company_totals_authorities', + firstPage: `SELECT bidder_id FROM company_totals ORDER BY authorities DESC, bidder_id DESC LIMIT 26`, + keysetPage: `SELECT bidder_id FROM company_totals WHERE (authorities < 5 OR (authorities = 5 AND bidder_id < 'eik:100')) ORDER BY authorities DESC, bidder_id DESC LIMIT 26`, + }, + { + name: 'authorities count-desc', + index: 'idx_authority_totals_count', + firstPage: `SELECT authority_id FROM authority_totals ORDER BY contracts DESC, authority_id DESC LIMIT 26`, + keysetPage: `SELECT authority_id FROM authority_totals WHERE (contracts < 10 OR (contracts = 10 AND authority_id < 'auth:50')) ORDER BY contracts DESC, authority_id DESC LIMIT 26`, + }, + { + name: 'authorities avg-desc', + index: 'idx_authority_totals_avg', + firstPage: `SELECT authority_id FROM authority_totals ORDER BY avg_eur DESC, authority_id DESC LIMIT 26`, + keysetPage: `SELECT authority_id FROM authority_totals WHERE (avg_eur < 100 OR (avg_eur = 100 AND authority_id < 'auth:50')) ORDER BY avg_eur DESC, authority_id DESC LIMIT 26`, + }, +] as const; + +// The same defect/fix, but with an active list filter on top of the sort — the case a reader of the +// unfiltered assertions above cannot infer. Filtering does NOT make the ordering index redundant: the +// planner keeps walking it and drops the sort step, so a filtered page stops being a whole-table sort +// too. Contracts only: it is the one list whose filters (sector via tenders.cpv_code, EU funding) +// join out to another table, so it is the case where the planner could most plausibly switch away. +const FILTERED_SORTS = [ + { + name: 'contracts date-desc + sector filter', + index: 'idx_contracts_signed_desc', + sql: `SELECT c.id ${CONTRACTS_FROM} WHERE t.cpv_code LIKE '45%' ORDER BY COALESCE(c.signed_at, '') DESC, c.id DESC LIMIT 16`, + }, + { + name: 'contracts date-desc + eu-funded filter', + index: 'idx_contracts_signed_desc', + sql: `SELECT c.id ${CONTRACTS_FROM} WHERE c.eu_funded = 1 ORDER BY COALESCE(c.signed_at, '') DESC, c.id DESC LIMIT 16`, + }, +] as const; + +// A modest, unanalyzed dataset — enough that the planner weighs a real table, none of ANALYZE's +// stats (production D1 never runs ANALYZE; verified via grep over scripts/ + migrations). +function seed(dbPath: string): void { + const stmts: string[] = ['BEGIN;']; + for (let i = 0; i < 40; i++) + stmts.push(`INSERT INTO authorities(id,name) VALUES('auth:${i}','A${i}');`); + for (let i = 0; i < 60; i++) + stmts.push(`INSERT INTO bidders(id,name) VALUES('eik:${i}','B${i}');`); + for (let i = 0; i < 120; i++) + stmts.push( + `INSERT INTO tenders(id,source_id,title,authority_id,cpv_code,procedure_type,status) ` + + `VALUES('t:${i}','U${i}','T${i}','auth:${i % 40}','45000000','открита','awarded');`, + ); + for (let i = 0; i < 600; i++) + stmts.push( + `INSERT INTO contracts(id,tender_id,bidder_id,amount,amount_eur,currency,value_flag,signed_at,bids_received) ` + + `VALUES('c:${i}','t:${i % 120}','eik:${i % 60}',${i * 10},${i * 10},'EUR','ok','202${i % 5}-0${(i % 9) + 1}-15',${(i % 4) + 1});`, + ); + for (let i = 0; i < 300; i++) + stmts.push( + `INSERT INTO company_totals(bidder_id,name,kind,eik_valid,won_eur,contracts,authorities,eu_eur) ` + + `VALUES('eik:${i}','B${i}','company',1,${i * 100},${i % 50},${i % 20},0);`, + ); + for (let i = 0; i < 200; i++) + stmts.push( + `INSERT INTO authority_totals(authority_id,name,spent_eur,contracts,suppliers,avg_eur,eu_eur) ` + + `VALUES('auth:${i}','A${i}',${i * 1000},${i % 90},${i % 30},${i * 3},0);`, + ); + stmts.push('COMMIT;'); + execFileSync('sqlite3', ['-bail', dbPath], { input: stmts.join('\n'), stdio: 'pipe' }); +} + +describe('list sort ordering indexes', () => { + let dir: string; + let before: string; // every migration EXCEPT the sort-index one (= real main minus this PR) + let after: string; // every migration (base + the sort-index one) + + beforeAll(() => { + // Every step below shells out to `sqlite3`. Without this probe a missing binary surfaces as an + // opaque ENOENT from the first execFileSync, which reads like a broken test rather than a missing + // tool. Fail loudly with the fix instead — deliberately NOT a skip: this file is a perf/cost gate, + // and silently passing it on a runner image that dropped sqlite3 would retire the gate unnoticed. + try { + execFileSync('sqlite3', ['-version'], { stdio: 'pipe' }); + } catch { + throw new Error( + 'The `sqlite3` CLI is required by this suite (the migration, refresh-slice and ship-domain ' + + 'tests exec it too) but is not on PATH. Install it (e.g. `apt-get install -y sqlite3`) and re-run.', + ); + } + dir = mkdtempSync(resolve(tmpdir(), 'sigma-sort-idx-')); + before = resolve(dir, 'before.sqlite'); + after = resolve(dir, 'after.sqlite'); + for (const db of [before, after]) { + for (const m of baseMigrations) readScript(db, m); + seed(db); + } + readScript(after, sortIndexMigration); + }); + + afterAll(() => rmSync(dir, { recursive: true, force: true })); + + // The defect: without the ordering index, the sort sorts the whole table — first page AND keyset page. + it.each(SORTS)('$name full-scans + sorts BEFORE the fix', ({ firstPage, keysetPage }) => { + expect(plan(before, firstPage)).toContain('USE TEMP B-TREE FOR ORDER BY'); + expect(plan(before, keysetPage)).toContain('USE TEMP B-TREE FOR ORDER BY'); + }); + + // The fix: each sort walks its dedicated index and drops the ORDER BY sort step — on both pages. + it.each(SORTS)( + '$name walks $index with no sort step AFTER the fix', + ({ index, firstPage, keysetPage }) => { + for (const sql of [firstPage, keysetPage]) { + const p = plan(after, sql); + expect(p).toContain(index); + expect(p).not.toContain('USE TEMP B-TREE FOR ORDER BY'); + } + }, + ); + + it.each(FILTERED_SORTS)('$name full-scans + sorts BEFORE the fix', ({ sql }) => { + expect(plan(before, sql)).toContain('USE TEMP B-TREE FOR ORDER BY'); + }); + + it.each(FILTERED_SORTS)('$name still walks $index AFTER the fix', ({ index, sql }) => { + const p = plan(after, sql); + expect(p).toContain(index); + expect(p).not.toContain('USE TEMP B-TREE FOR ORDER BY'); + }); +}); diff --git a/packages/db/src/queries/contracts.ts b/packages/db/src/queries/contracts.ts index 68a523022..4bbb70200 100644 --- a/packages/db/src/queries/contracts.ts +++ b/packages/db/src/queries/contracts.ts @@ -50,6 +50,13 @@ export const CONTRACT_FILTER_KEYS = [ // errors, add the new filter key to CONTRACT_FILTER_KEYS. assertCovers(); +// SYNC: each expr is backed by a matching expression index so a keyset page walks it instead of +// full-scanning + temp-B-tree-sorting the whole table (D1 bills rows scanned). The COALESCE sentinels +// must stay byte-identical to the indexes: value → idx_contracts_value_desc/asc (migrations/0000), +// date → idx_contracts_signed_desc/asc (migrations/0005). Changing a default here without the index +// silently drops the index — list-sort-indexes.test.ts asserts the EXPLAIN plan to catch that. +// Scope: the index-walk guarantee covers the UNFILTERED sort paths; with an active filter the planner +// may prefer the filter's index and temp-sort the (much smaller) filtered set — acceptable by design. const SORTS: Record = lookup({ 'value-desc': { expr: 'COALESCE(c.amount_eur, -1)', dir: 'desc' }, 'value-asc': { expr: 'COALESCE(c.amount_eur, 1e18)', dir: 'asc' }, diff --git a/packages/db/src/sort-index-sentinel-sync.test.ts b/packages/db/src/sort-index-sentinel-sync.test.ts new file mode 100644 index 000000000..6eef242c1 --- /dev/null +++ b/packages/db/src/sort-index-sentinel-sync.test.ts @@ -0,0 +1,36 @@ +/// +import { readFileSync } from 'node:fs'; +import { dirname, resolve } from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describe, expect, it } from 'vitest'; + +// The date sort indexes only get used if their COALESCE sentinel is byte-identical to the ORDER BY +// expression the query layer emits (queries/contracts.ts SORTS). The EXPLAIN test proves the plan on a +// local sqlite3, whose planner is not bit-exact to D1 — so back it with a planner-INDEPENDENT static +// check that the sentinels themselves match. A .sql migration can't import a TS constant, so this +// compares the two source files directly; a drift fails here regardless of the DB engine. + +// This file is packages/db/src/…, so one level up is the @sigma/db package root. +const dbRoot = resolve(dirname(fileURLToPath(import.meta.url)), '..'); +const migrationSql = readFileSync(resolve(dbRoot, 'migrations/0005_list_sort_indexes.sql'), 'utf8'); +const contractsTs = readFileSync(resolve(dbRoot, 'src/queries/contracts.ts'), 'utf8'); + +// Collect the sentinel literal of every COALESCE(signed_at, '') in a source. +function signedSentinels(src: string): string[] { + const out: string[] = []; + const re = /COALESCE\((?:c\.)?signed_at,\s*'([^']*)'\)/g; + let m: RegExpExecArray | null; + while ((m = re.exec(src)) !== null) out.push(m[1]!); + return [...new Set(out)].sort(); +} + +describe('date-sort sentinel sync (migration ↔ query layer)', () => { + it('the signed_at COALESCE sentinels are byte-identical in the migration and SORTS', () => { + const fromMigration = signedSentinels(migrationSql); + const fromQuery = signedSentinels(contractsTs); + + // Guard against a regex that silently matches nothing (which would make the test pass vacuously). + expect(fromMigration).toEqual(['', '9999-99']); + expect(fromQuery).toEqual(fromMigration); + }); +});