Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 48 additions & 0 deletions apps/web/app/lib/json-ld.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
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 <script> element', () => {
const out = serializeJsonForScript({ url: 'https://x/</script><script>alert(1)</script>' });
// The raw breakout sequence must not survive — this is the whole point of the sink.
expect(out).not.toContain('</script>');
expect(out).not.toContain('<script>');
expect(out).toContain('\\u003c/script');
});

it('is JSON-equivalent: parsing the output returns the identical value', () => {
const value = {
name: 'A </script> B',
nested: ['<a>', { k: '</SCRIPT >' }],
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('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 <script> body parseable.
expect(serializeJsonForScript(undefined)).toBe('null');
expect(serializeJsonForScript(() => 1)).toBe('null');
expect(JSON.parse(serializeJsonForScript(undefined))).toBeNull();
});
});
21 changes: 21 additions & 0 deletions apps/web/app/lib/json-ld.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
// Serialize a value for embedding inside an inline <script> (e.g. a JSON-LD data island). Plain
// JSON.stringify does NOT escape `<`, so a raw `</script>` 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.
//
// Mirrors the project's own review standard (docs/review-security.md "Инжекции и валидация") and the
// safeJson helper in routes/contract.json.tsx. Kept as defense-in-depth: today the only value that
Comment thread
todorkolev marked this conversation as resolved.
Outdated
// reaches root.tsx's JSON-LD is the request origin (which `new URL()` cannot make carry `</script>`),
// but this makes the sink safe for any DB/user-derived field added to the graph 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
Comment thread
todorkolev marked this conversation as resolved.
// any value even though today's callers always pass an object (review ydimitrof).
const json = JSON.stringify(value);
Comment thread
todorkolev marked this conversation as resolved.
if (json === undefined) return 'null';
return json
.replace(/</g, '\\u003c')
Comment thread
todorkolev marked this conversation as resolved.
.replace(/\u2028/g, '\\u2028')
Comment thread
todorkolev marked this conversation as resolved.
Comment thread
todorkolev marked this conversation as resolved.
.replace(/\u2029/g, '\\u2029');
}
3 changes: 2 additions & 1 deletion apps/web/app/root.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { SiteFooter } from './components/SiteFooter';
import { AccessibilityWidget } from './components/AccessibilityWidget';
import { PageHeader } from './components/PageHeader';
import { getCoverageMeta } from './lib/coverage';
import { serializeJsonForScript } from './lib/json-ld';
import { withDbRetry } from './lib/retry';
import stylesheet from './app.css?url';

Expand Down Expand Up @@ -68,7 +69,7 @@ export function Layout({ children }: { children: React.ReactNode }) {
const origin = rootData?.origin;
const imageUrl = origin ? `${origin}/og.png` : undefined;
const schemaOrg = origin
? JSON.stringify({
? serializeJsonForScript({
'@context': 'https://schema.org',
'@graph': [
{
Expand Down
31 changes: 31 additions & 0 deletions packages/db/migrations/0005_list_sort_indexes.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
-- Ordering indexes for the non-default list sorts, so a keyset page walks an index and stops at
-- LIMIT instead of scanning + temp-B-tree-sorting the whole table on every request (D1 bills rows
-- SCANNED). The default sorts already had matching indexes (idx_contracts_value_desc/asc,
-- idx_company_totals_won/name, idx_authority_totals_spent/name); these cover the sorts that were
-- missing one. Each index matches the EXACT ORDER BY expression the query layer emits (the COALESCE
-- forms in queries/contracts.ts SORTS) plus the keyset id tiebreak in the same direction, so SQLite
-- neither sorts nor buffers. Additive + idempotent; the rollup tables are DELETE+INSERT-refreshed
-- (never dropped), so these survive every ETL ship.

Comment thread
todorkolev marked this conversation as resolved.
-- /contracts ?sort=date-desc | date-asc — ORDER BY COALESCE(signed_at, …), c.id (queries/contracts.ts).
-- idx_contracts_signed is on the bare signed_at column and does NOT match the COALESCE expression.
-- SYNC: the sentinels below ('' for desc, '9999-99' for asc) must stay byte-identical to the SORTS
-- map in queries/contracts.ts (`date-desc`/`date-asc` expr). If a default there changes, the index
-- expression stops matching and SQLite silently falls back to a full scan + temp-B-tree sort — change
-- both together. The list-sort-indexes.test.ts EXPLAIN assertions catch a drift.
CREATE INDEX IF NOT EXISTS idx_contracts_signed_desc
Comment thread
todorkolev marked this conversation as resolved.
Comment thread
todorkolev marked this conversation as resolved.
ON contracts(COALESCE(signed_at, '') DESC, id DESC);
CREATE INDEX IF NOT EXISTS idx_contracts_signed_asc
ON contracts(COALESCE(signed_at, '9999-99') ASC, id ASC);
Comment thread
todorkolev marked this conversation as resolved.

-- /companies ?sort=count | authorities — ORDER BY <col> DESC, bidder_id DESC (queries/companies.ts).
CREATE INDEX IF NOT EXISTS idx_company_totals_count
Comment thread
todorkolev marked this conversation as resolved.
Comment thread
todorkolev marked this conversation as resolved.
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 <col> 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);
158 changes: 158 additions & 0 deletions packages/db/src/list-sort-indexes.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/// <reference types="node" />
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 <sortExpr> <dir>, <id> <dir> 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 <table> … 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.

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 (review ydimitrof).
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], {
Comment thread
todorkolev marked this conversation as resolved.
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 <cmp> ? OR (expr = ? AND id <cmp> ?))` seek every page after the first uses).
const CONTRACTS_FROM =
Comment thread
todorkolev marked this conversation as resolved.
'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;

// 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;
Comment thread
todorkolev marked this conversation as resolved.
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(() => {
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');
}
},
);
});
Comment thread
todorkolev marked this conversation as resolved.
5 changes: 5 additions & 0 deletions packages/db/src/queries/contracts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,11 @@ export const CONTRACT_FILTER_KEYS = [
// errors, add the new filter key to CONTRACT_FILTER_KEYS.
assertCovers<ContractListParams, typeof CONTRACT_FILTER_KEYS>();

// SYNC: each expr is backed by a matching expression index so a keyset page walks it instead of
Comment thread
todorkolev marked this conversation as resolved.
// 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.
const SORTS: Record<ContractSort, { expr: string; dir: 'asc' | 'desc' }> = lookup({
'value-desc': { expr: 'COALESCE(c.amount_eur, -1)', dir: 'desc' },
'value-asc': { expr: 'COALESCE(c.amount_eur, 1e18)', dir: 'asc' },
Expand Down