From 4d6c0911bc52f547f770de4645510dbdac38ec04 Mon Sep 17 00:00:00 2001 From: lyubomir-bozhinov Date: Mon, 24 Aug 2026 10:29:01 +0300 Subject: [PATCH 1/2] fix(privacy): project legal_form on the companies list path so sole-trader masking works (#173) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit toCompanyListItem masks natural persons on r.legal_form (PR #183) and COLS selects legal_form, but listCompanies read the unfiltered rollup via source(p) — which omits the bidders LEFT JOIN and the b.legal_form projection. company_totals has no legal_form column, so the query referenced a non-existent column and D1 500'd the whole /companies leaderboard and its /companies.data single-fetch twin. The mock-based unit lane never executed the SQL, so it stayed green; the real-D1 integration lane exposes the 500. Switch listCompanies to source(p, { legalForm: true }) so the rollup path LEFT JOINs bidders by primary key and projects legal_form — the join does not change the row set or the count. Update the source()-projection test to assert the join is present on the list path (masking requires it), reversing its now-invalid perf-optimisation intent. Refs #183 --- packages/db/src/queries/companies.test.ts | 23 +++++++++++++---------- packages/db/src/queries/companies.ts | 8 +++++++- 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/packages/db/src/queries/companies.test.ts b/packages/db/src/queries/companies.test.ts index 5a93beeb1..e719184ea 100644 --- a/packages/db/src/queries/companies.test.ts +++ b/packages/db/src/queries/companies.test.ts @@ -366,13 +366,16 @@ describe('streamCompaniesCsv masking', () => { }); }); -describe('listCompanies source() projection — legal_form only when needed (PR #183 review T-007)', () => { - // The listCompanies hot path returns CompanyListItem via toCompanyListItem, which does not read - // legal_form. Projecting `b.legal_form AS legal_form` plus the LEFT JOIN on every list query is - // wasted work. Only the CSV streamer needs legal_form (for the natural-person masker). The - // unfiltered rollup subquery must therefore omit the join + projection on the list path but - // keep them on the CSV path. (The base-aggregation CTE is shared and must project legal_form - // because the CSV path may go through it too.) +describe('listCompanies source() projection — legal_form is required for masking (PR #183)', () => { + // PR #183 taught `toCompanyListItem` to mask sole traders on `r.legal_form`, and `COLS` selects + // `legal_form`. `company_totals` has no `legal_form` column, so the rollup source MUST LEFT JOIN + // `bidders` and project `b.legal_form AS legal_form` on the LIST path too — not only on the CSV + // path. The earlier "omit the join on the list path as a perf optimisation" intent was invalidated + // by the masking change: without the projection the unfiltered `/companies` query references a + // non-existent column and D1 500s the whole leaderboard (and its `.data` twin). A mock DB that + // only captures SQL strings never executes the query, so this class of break is invisible to the + // unit lane — it is asserted end-to-end in the integration lane + // (`test/integration/privacy-noindex-data.test.ts`). function spySqlDb(): { db: D1Database; sql: string[] } { const db = fakeDb(); const sql: string[] = []; @@ -384,13 +387,13 @@ describe('listCompanies source() projection — legal_form only when needed (PR return { db, sql }; } - it('omits LEFT JOIN bidders in the rollup subquery on the listCompanies path (no legal_form needed)', async () => { + it('keeps LEFT JOIN bidders + b.legal_form projection in the rollup subquery on the listCompanies path (masking needs it)', async () => { const { db, sql } = spySqlDb(); await listCompanies(db, {}); const rollupQueries = sql.filter((q) => q.includes('company_totals') && q.includes('FROM (')); expect(rollupQueries.length).toBeGreaterThan(0); - expect(rollupQueries.every((q) => !q.includes('LEFT JOIN bidders'))).toBe(true); - expect(rollupQueries.every((q) => !q.includes('b.legal_form AS legal_form'))).toBe(true); + expect(rollupQueries.some((q) => q.includes('LEFT JOIN bidders'))).toBe(true); + expect(rollupQueries.some((q) => q.includes('b.legal_form AS legal_form'))).toBe(true); }); it('keeps LEFT JOIN bidders + b.legal_form projection in the rollup subquery on the streamCompaniesCsv path', async () => { diff --git a/packages/db/src/queries/companies.ts b/packages/db/src/queries/companies.ts index 7e365b8a1..bd7d9e917 100644 --- a/packages/db/src/queries/companies.ts +++ b/packages/db/src/queries/companies.ts @@ -166,7 +166,13 @@ export async function listCompanies( ): Promise> { const sort = SORTS[p.sort as keyof typeof SORTS] ?? SORTS['won']; const pageSize = p.pageSize ?? 25; - const src = source(p); // legal_form not needed — toCompanyListItem drops it + // `toCompanyListItem` masks sole traders on `r.legal_form` (PR #183), and `COLS` selects + // `legal_form`, so the rollup source MUST project it — `company_totals` has no `legal_form` + // column, so without the `{ legalForm: true }` LEFT JOIN on `bidders` the unfiltered list query + // references a non-existent column and D1 500s the whole `/companies` leaderboard (and its + // `.data` twin). The join is by primary key (`b.id = ct.bidder_id`), so it does not change the + // row set or the count. + const src = source(p, { legalForm: true }); const ew = entityWhere(p); const signature = companyFilterSignature(p); const ks = keyset({ From 868d115ef503c36618a5d3f00380d5dd7ec114ac Mon Sep 17 00:00:00 2001 From: lyubomir-bozhinov Date: Mon, 24 Aug 2026 10:29:01 +0300 Subject: [PATCH 2/2] test(privacy): prove noindex+mask on the .data/.json twins end-to-end (#173) Add an integration test (real RRv7 single-fetch handler via wrangler.getPlatformProxy + real D1, PR #177's lane) that locks in the previously-unproven claim: RRv7 7.18 runs a route's headers export on the /.data request and carries the loader-set X-Privacy-Mask marker onto the .data HTTP response, so hardenResponse translates it to X-Robots-Tag: noindex and deletes the internal marker. The prior app.nofollow.test.ts hand-injects the marker into a mocked createRequestHandler and could not prove this. Covers /companies.data, /companies/:eik.data, /contracts.data and /contracts/:id.json for a sole trader (masked), a legal entity (verbatim, not noindexed), and a consortium led by a sole trader (over-mask guard). A decodeSingleFetch helper reconstructs the turbo-stream payload so row assertions read real loader data. seedRows lets a test seed extra rows into the lane's isolated per-file D1. Refs #183 --- .../test/integration/helpers/single-fetch.ts | 65 +++++++ .../integration/privacy-noindex-data.test.ts | 180 ++++++++++++++++++ apps/web/test/integration/setup.ts | 18 ++ 3 files changed, 263 insertions(+) create mode 100644 apps/web/test/integration/helpers/single-fetch.ts create mode 100644 apps/web/test/integration/privacy-noindex-data.test.ts diff --git a/apps/web/test/integration/helpers/single-fetch.ts b/apps/web/test/integration/helpers/single-fetch.ts new file mode 100644 index 000000000..f9e078fe3 --- /dev/null +++ b/apps/web/test/integration/helpers/single-fetch.ts @@ -0,0 +1,65 @@ +// Decoder for React Router v7's single-fetch (`/.data`) turbo-stream payload. +// +// RRv7 serves the `.data` twin of a document route as a flat, de-duplicated value table (its +// vendored `turbo-stream` encoding): a JSON array where index 0 is the root value and every nested +// value is referenced by its index in the array. Objects are encoded as `{ "_": valueIndex }`, +// arrays as `[index, …]`, primitives (string / number / boolean / literal `null`) inline, and a +// handful of NEGATIVE indices are sentinels for the JS values that JSON can't carry +// (`undefined` / `NaN` / `±Infinity`) plus `null`. +// +// This decoder reconstructs the object graph so integration tests can assert on the real +// client-visible loader data — the same bytes a browser turns back into `loaderData` on a +// client-side navigation — instead of grepping the opaque flat array. It is intentionally minimal: +// it collapses every negative sentinel to `null`, which is all the privacy assertions need (a +// masked `eik` is `null`; an unmasked one is a non-empty string — the two never collide). If a +// future test needs to distinguish `undefined` from `null` or read a `NaN`, extend the sentinel +// handling then. +// +// The `cache` (index → built value) both memoises shared references and terminates the cyclic +// graphs RRv7 can emit (a value that refers back to an ancestor index). +export function decodeSingleFetch(text: string): Record { + const arr = JSON.parse(text) as unknown[]; + const cache = new Map(); + + function build(ref: unknown): unknown { + if (typeof ref !== 'number') return ref; + if (ref < 0) return null; // turbo-stream sentinel (undefined / null / NaN / ±Infinity) → nullish + if (cache.has(ref)) return cache.get(ref); + + const value = arr[ref]; + if (value === null || typeof value !== 'object') { + cache.set(ref, value); + return value; + } + if (Array.isArray(value)) { + const out: unknown[] = []; + cache.set(ref, out); + for (const item of value) out.push(build(item)); + return out; + } + const out: Record = {}; + cache.set(ref, out); + for (const [encodedKey, valueRef] of Object.entries(value)) { + // Object keys are `_` — the property name itself lives in the value table. + const key = build(Number(encodedKey.slice(1))); + out[String(key)] = build(valueRef); + } + return out; + } + + return build(0) as Record; +} + +/** Read one route's decoded loader `data` from a decoded single-fetch payload. */ +export function routeData( + decoded: Record, + routeId: string, +): T { + const entry = decoded[routeId]; + if (!entry) { + throw new Error( + `[single-fetch] route "${routeId}" not present in payload; got routes: ${Object.keys(decoded).join(', ')}`, + ); + } + return entry.data as T; +} diff --git a/apps/web/test/integration/privacy-noindex-data.test.ts b/apps/web/test/integration/privacy-noindex-data.test.ts new file mode 100644 index 000000000..2abaed3aa --- /dev/null +++ b/apps/web/test/integration/privacy-noindex-data.test.ts @@ -0,0 +1,180 @@ +// Privacy masking + `noindex` on the machine-readable twins — end-to-end through the REAL worker +// pipeline (issue #173, PR #183). This is the durable regression guard that the earlier lane +// (`apps/web/workers/app.nofollow.test.ts`) could not be: that test hand-injects the internal +// `X-Privacy-Mask` marker into a MOCKED `createRequestHandler`, so it proves the worker's +// marker→`X-Robots-Tag` translation but NOT that React Router v7's single-fetch pipeline actually +// runs a route's `headers` export (and forwards a loader-set marker) onto the `/.data` +// response. RRv7's `getDocumentHeadersImpl` forwards only `Set-Cookie` by default; the `headers` +// export in `companies.tsx` / `contracts.tsx` / `company.tsx` is the explicit forward that PR #183 +// relies on. Whether that forward fires on the `.data` request is the whole open question, and it +// is only answerable by driving the real handler — which the #177 integration lane does, via +// `wrangler.getPlatformProxy()` (real D1 + migrations) and `appFetch()`. +// +// Ground truth (recorded in the PR): RRv7 7.18.0 DOES run the `headers` export on `.data` and +// carries the returned marker onto the `.data` HTTP response, so `hardenResponse` translates it to +// `X-Robots-Tag: noindex` and deletes the internal marker. These assertions lock that in and would +// fail if a RRv7 upgrade, a dropped `headers` export, or a broken loader marker regressed it. +// +// The `.data` body is RRv7's single-fetch turbo-stream; `decodeSingleFetch` reconstructs the exact +// client-visible loader data so the row assertions read real objects, not a grepped flat array. + +import { describe, expect, it, beforeAll } from 'vitest'; +import { appFetch, seedRows } from './setup'; +import { decodeSingleFetch, routeData } from './helpers/single-fetch'; + +// ── Fixture entities (seeded into THIS file's isolated proxy DB) ─────────────────────────────── +// A sole trader (ЕТ, natural person), a legal entity (ООД, has a public ЕИК), and a consortium +// whose name leads with a sole trader — the MAJOR-class over-mask trap the `kind !== 'consortium'` +// guard exists for. ЕИК values and contract counts are chosen so each lands in a distinct +// `?count` bucket, letting a filtered request isolate a natural-person-free page. +const ET_EIK = '999000111'; +const ET_NAME = 'ЕТ ДРИФТ - НИКОЛАЙ КИРОВ'; +const ET_NAME_TOKEN = 'НИКОЛАЙ КИРОВ'; // the sensitive source-name fragment that must never leak +const OOD_EIK = '200000002'; +const OOD_NAME = 'СТРОЙ ООД'; +const CONSORTIUM_EIK = '300000003'; +const CONSORTIUM_NAME = 'ЕТ Иван Петров; Строй ООД'; +const CONSORTIUM_MEMBER_TOKEN = 'Строй ООД'; // a member name that must survive verbatim (not masked) +const MASK_LABEL = 'Частно лице'; + +const SEED: readonly string[] = [ + // Sole trader — legal_form 'ЕТ' AND a leading-"ЕТ " name both flag it as a natural person. + `INSERT OR IGNORE INTO bidders (id, name, bulstat, eik_normalized, eik_valid, is_consortium, kind, legal_form) + VALUES ('eik:${ET_EIK}', '${ET_NAME}', '${ET_EIK}', '${ET_EIK}', 1, 0, 'company', 'ЕТ')`, + `INSERT OR IGNORE INTO company_totals (bidder_id, name, kind, eik, eik_valid, won_eur, contracts, authorities, eu_eur, first_date, last_date) + VALUES ('eik:${ET_EIK}', '${ET_NAME}', 'company', '${ET_EIK}', 1, 9000000.0, 5, 1, 0, '2021-01-01', '2022-12-01')`, + // Legal entity — plain ООД with a public ЕИК; must stay verbatim and out of the noindex bucket. + `INSERT OR IGNORE INTO bidders (id, name, bulstat, eik_normalized, eik_valid, is_consortium, kind, legal_form, settlement) + VALUES ('eik:${OOD_EIK}', '${OOD_NAME}', '${OOD_EIK}', '${OOD_EIK}', 1, 0, 'company', 'ООД', 'Пловдив')`, + `INSERT OR IGNORE INTO company_totals (bidder_id, name, kind, eik, eik_valid, settlement, won_eur, contracts, authorities, eu_eur, first_date, last_date) + VALUES ('eik:${OOD_EIK}', '${OOD_NAME}', 'company', '${OOD_EIK}', 1, 'Пловдив', 8000000.0, 50, 1, 0, '2020-01-01', '2022-12-28')`, + // Consortium whose lead member is a sole trader — the over-mask guard target. Kept verbatim. + `INSERT OR IGNORE INTO bidders (id, name, bulstat, eik_normalized, eik_valid, is_consortium, kind, legal_form) + VALUES ('eik:${CONSORTIUM_EIK}', '${CONSORTIUM_NAME}', '${CONSORTIUM_EIK}', '${CONSORTIUM_EIK}', 1, 1, 'consortium', 'ДЗЗД')`, + `INSERT OR IGNORE INTO company_totals (bidder_id, name, kind, eik, eik_valid, won_eur, contracts, authorities, eu_eur, first_date, last_date) + VALUES ('eik:${CONSORTIUM_EIK}', '${CONSORTIUM_NAME}', 'consortium', '${CONSORTIUM_EIK}', 1, 7000000.0, 10, 1, 0, '2021-01-01', '2022-06-01')`, + // One contract won by the sole trader — exercises /contracts.data (list mask) + /contracts/:id.json. + `INSERT OR IGNORE INTO contracts (id, tender_id, bidder_id, amount, currency, signed_at, value_flag, date_flag, amount_eur, fx_converted) + VALUES ('c:ET-1', 't:FIX-1', 'eik:${ET_EIK}', 5000000, 'BGN', '2022-06-01', 'ok', 'ok', 5000000, 0)`, +]; + +type ListItem = { + slug: string; + name: string; + displayName: string; + eik: string | null; + hasEik: boolean; + isConsortium: boolean; +}; +type ContractItem = { id: string; bidderSlug: string; bidderName: string; bidderDisplayName: string }; + +async function getData(path: string): Promise { + return appFetch(new Request(`https://sigma.bg${path}`, { headers: { 'CF-Connecting-IP': '203.0.113.201' } })); +} + +async function companiesItems(query = ''): Promise<{ res: Response; items: ListItem[] }> { + const res = await getData(`/companies.data${query}`); + const data = routeData<{ page: { items: ListItem[] } }>(decodeSingleFetch(await res.text()), 'routes/companies'); + return { res, items: data.page.items }; +} + +function bySlug(items: T[], slug: string): T { + const hit = items.find((i) => i.slug === slug || i.bidderSlug === slug); + if (!hit) throw new Error(`row with slug ${slug} not on page; got ${items.map((i) => i.slug ?? i.bidderSlug).join(', ')}`); + return hit; +} + +describe('privacy: noindex + masking on machine-readable twins (real RRv7 single-fetch)', () => { + beforeAll(async () => { + await seedRows(SEED); + }); + + it('the internal X-Privacy-Mask marker never reaches the client on any .data response', async () => { + for (const path of ['/companies.data', `/companies/${ET_EIK}.data`, '/contracts.data']) { + const res = await getData(path); + expect(res.headers.get('X-Privacy-Mask'), `marker leaked on ${path}`).toBeNull(); + } + }); + + it('/companies.data — masked ЕТ page carries noindex; row is masked (name + null ЕИК)', async () => { + const { res, items } = await companiesItems(); + expect(res.headers.get('X-Robots-Tag')).toBe('noindex'); + + const et = bySlug(items, ET_EIK); + expect(et.name).toBe(MASK_LABEL); + expect(et.displayName).toBe(MASK_LABEL); + expect(et.eik).toBeNull(); + expect(et.hasEik).toBe(false); + + // Sensitivity: the sensitive source name must be absent from the raw twin, not just the object. + expect(await getData('/companies.data').then((r) => r.text())).not.toContain(ET_NAME_TOKEN); + }); + + it('/companies/.data — detail twin carries noindex; ЕИК masked, trading name preserved (ADR-0039)', async () => { + const res = await getData(`/companies/${ET_EIK}.data`); + expect(res.status).toBe(200); + expect(res.headers.get('X-Robots-Tag')).toBe('noindex'); + + const company = routeData<{ company: { eik: string | null; displayName: string } }>( + decodeSingleFetch(await res.text()), + 'routes/company', + ).company; + // The sensitive identifier (ЕИК) is masked; the trading name stays public — the `.data` twin is + // the client-nav transport that re-renders the same HTML page (ADR-0039 §3). + expect(company.eik).toBeNull(); + expect(company.displayName).toContain('НИКОЛАЙ КИРОВ'); + }); + + it('/contracts.data — page with an ЕТ bidder carries noindex; bidder masked', async () => { + const res = await getData('/contracts.data'); + expect(res.headers.get('X-Robots-Tag')).toBe('noindex'); + + const items = routeData<{ result: { items: ContractItem[] } }>( + decodeSingleFetch(await res.text()), + 'routes/contracts', + ).result.items; + const et = bySlug(items, ET_EIK); + expect(et.bidderName).toBe(MASK_LABEL); + expect(et.bidderDisplayName).toBe(MASK_LABEL); + }); + + it('/contracts/.json — ЕТ bidder masked, ЕИК null, server-only legal_form stripped, noindex', async () => { + const res = await getData('/contracts/ET-1.json'); + expect(res.headers.get('X-Robots-Tag')).toBe('noindex'); + const body = (await res.json()) as { + bidder: { name: string; displayName: string; eik: string | null }; + sourceNames: { bidder: string }; + } & Record; + + expect(body.bidder.name).toBe(MASK_LABEL); + expect(body.bidder.displayName).toBe(MASK_LABEL); + expect(body.bidder.eik).toBeNull(); + expect(body.sourceNames.bidder).toBe(MASK_LABEL); + // The server-only natural-person classifier must never reach the client body. + expect('bidder_legal_form' in body).toBe(false); + }); + + it('NEGATIVE — a legal-entity-only page is NOT noindexed and keeps ЕИК + name verbatim', async () => { + // ?count=21-100 selects the ООД (50 contracts) and the base fixture company (30), excludes the + // ЕТ (5) and the consortium (10) — a page with zero natural persons. Proves the noindex signal + // is data-driven, not blanket (no over-noindexing of public companies). + const { res, items } = await companiesItems('?count=21-100'); + expect(res.status).toBe(200); + expect(res.headers.get('X-Robots-Tag')).toBeNull(); + + const ood = bySlug(items, OOD_EIK); + expect(ood.name).toBe(OOD_NAME); + expect(ood.eik).toBe(OOD_EIK); + expect(ood.hasEik).toBe(true); + expect(items.some((i) => i.name === MASK_LABEL)).toBe(false); + }); + + it('GUARD — a consortium led by a sole trader is NOT over-masked (name + ЕИК kept verbatim)', async () => { + const { items } = await companiesItems(); + const consortium = bySlug(items, CONSORTIUM_EIK); + expect(consortium.isConsortium).toBe(true); + expect(consortium.name).not.toBe(MASK_LABEL); + expect(consortium.name).toContain(CONSORTIUM_MEMBER_TOKEN); + expect(consortium.eik).toBe(CONSORTIUM_EIK); + }); +}); diff --git a/apps/web/test/integration/setup.ts b/apps/web/test/integration/setup.ts index 9efa5b036..1551ebe6e 100644 --- a/apps/web/test/integration/setup.ts +++ b/apps/web/test/integration/setup.ts @@ -108,6 +108,24 @@ export async function appFetch(request: Request): Promise { return app.default.fetch(request, proxy.env, proxy.ctx); } +/** + * Seed extra rows into THIS worker's shared D1 (the same DB `appFetch` reads through). Bootstraps + * the proxy first (migrations + base fixtures) if it has not booted yet, then execs each statement + * in order. Vitest runs each test file in its own worker process with its own proxy, so rows seeded + * here are visible only to `appFetch` calls from the same file — they never perturb other lanes. + */ +export async function seedRows(statements: readonly string[]): Promise { + const proxy = await getProxy(); + // `DB.exec` treats each newline as a statement boundary, so a multi-line statement errors with + // "incomplete input". Route every statement through the same string-aware collapser the migration + // seeder uses (`stripSqlCommentsAndCollapse`) so callers can write readable multi-line SQL. + for (const stmt of statements) { + for (const collapsed of stripSqlCommentsAndCollapse(stmt)) { + await proxy.env.DB.exec(collapsed); + } + } +} + /** Reset the memoised worker import. Test-only escape hatch. */ export function __resetSigmaAppForTesting(): void { appPromise = null;