Skip to content

fix(privacy): apply noindex+mask policy to machine-readable outputs (#173) - #183

Open
LyuboslavLyubenov wants to merge 33 commits into
midt-bg:mainfrom
LyuboslavLyubenov:main
Open

fix(privacy): apply noindex+mask policy to machine-readable outputs (#173)#183
LyuboslavLyubenov wants to merge 33 commits into
midt-bg:mainfrom
LyuboslavLyubenov:main

Conversation

@LyuboslavLyubenov

@LyuboslavLyubenov LyuboslavLyubenov commented Jun 30, 2026

Copy link
Copy Markdown

Какво и защо

HTML профилът на фирма вече прилага noindex за разпознати физически лица / еднолични търговци (ЕТ),
но същите идентификатори остават достъпни от машинно-четливите повърхности — JSON записът
на договора (/contracts/:id.json) и трите CSV експорта (/contracts.csv, /companies.csv,
/authorities.csv). Те връщат ЕИК и оригиналното име от източника без X-Robots-Tag: noindex
и без маскиране, edge-кешират се (Cache-Control: public, max-age=3600) и попадат в индексите
на търсачките и ботовете.

Несъответствието прави идентификаторите, които HTML умишлено държи извън търсачките,
търсещи се и изтегляеми накуп — класът CWE-359, описан в issue #173.

Решение

Прилага се политика „noindex плюс маскиране" с един общ предикат от
packages/shared/src/format.ts, който заменя досегашната дублирана логика в company.tsx:

  • Споделена логика. Нов isNaturalPersonBidder(name, legalForm) комбинира legal_form LIKE 'ЕТ%'
    (вкл. латинското ET, разширените форми ЕДНОЛИЧЕН ТЪРГОВЕЦ, SOLE TRADER, INDIVIDUAL)
    с водещия ЕТ суфикс в името (който вече беше в isNaturalPersonProfileName).
    MASKED_NATURAL_PERSON_LABEL ('Частно лице') е константа, която се внася по символ от
    всеки консуматор — преименуването ѝ не чупи нито един тест. Inline isSingleNaturalPersonProfile
    е премахнат от company.tsx.
  • CSV стриймовете в packages/db/src/queries/{contracts,companies}.ts правят
    SELECT b.legal_form и маскират contractor_eik/eik и contractor/name преди
    байтовете да стигнат до R2
    — edge кешът не може никога да сервира немаскиран
    естествено-личностен ред. apps/web/app/lib/csv-export.ts добавя X-Robots-Tag: noindex
    на четирите клона: 200/206 HIT, dynamic (филтриран) и 304.
  • JSON маскиране. packages/db/src/queries/details.ts разширява getContract с
    bidder_legal_form (server-only, не изтича към клиента — ContractRecord остава непроменен).
    apps/web/app/routes/contract.json.tsx изнася чист maskContractForPrivacy(record, bidderLegalForm) помощник, който слага X-Robots-Tag: noindex само когато маскирането
    реално е приложило — reference-equality гейт masked !== record прави повторното
    извикване на предиката ненужно.
  • Потребителска документация. Нова секция #natural-person-data в
    apps/web/app/routes/privacy.tsx описва кои полета се маскират, кои повърхности
    носят noindex-а и че HTML профилът остава непокътнат. Кадърът е инженерно ръководство,
    не правен съвет — същият disclaimer носи и ADR-0002.
  • ADR-0002 в docs/architecture.md (български, огледало на ADR-0001): Контекст / Решение / Последствия / Засегнати повърхности / Доказателство. Цитираните exit code-ове
    са от реален пост-едит прогон в чисто и замърсено дърво, не оценки.

Юридическите лица (legal entity) са непроменени — ЕИК и имената им остават видими
във всички повърхности.

Валидация (реални данни)

  • pnpm typecheck — exit 0 (7/7 turbo задачи).
  • pnpm test --force — exit 1, заради 3 пред-съществуващи повреди в @sigma/db
    (integrity-checks.test.ts reconciliation gate + 2 таймаута в refresh-slice.test.ts).
    Същият набор е налице и в предишния main, не е въведен от този PR.
    Всичките 38 нови теста минават — пост-едит и пре-едит прогонът показват същия набор
    пред-съществуващи повреди, без нови въведени от този PR.
  • pnpm lint — exit 1, 6 prettier warnings: 2 пред-съществуващи (RiskIndicators.tsx,
    riskLogic.test.ts) + 4 нововъведени (contract.json.test.ts, privacy.tsx,
    companies.test.ts, companies.ts). Няма нови lint-видове — само пренасяне на редове
    заради български / EN текст в JSX.
  • Тестове по пакет (фокусни): @sigma/shared 42/42, @sigma/db
    (contracts.test.ts + companies.test.ts + details.test.ts) 26/26,
    @sigma/web (contract.json.test.ts + csv-export.test.ts) 33/33.

Извън обхвата

  • Няма миграция на схемата — bidders.legal_form вече присъстваше в migrations/0000_init.sql.
  • ContractRecord (API contract) е непроменен от страна на клиента; полето bidder_legal_form
    остава server-only (добавя се в details.ts, използва се от route-а, не се изпраща на клиента).
  • HTML профилът не е пипан — неговият noindex мета-етикет минава през същия споделен предикат.

Чеклист

  • Комитите следват conventional commits и нямат Co-Authored-By: trailer
  • PR-ът е с един логически обхват и е от форк към midt-bg/sigma:main
  • pnpm typecheck минава; пред-съществуващите pnpm test/pnpm lint повреди са
    документирани като baseline в секция „Валидация (реални данни)" по-горе

Closes #173

@lyubomir-bozhinov lyubomir-bozhinov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добре структуриран — маскирането е на query слой преди R2, noindex-gate-ът на .json е чист, тестовете са солидни. Но тезата на PR-а („noindex+mask на всички machine-readable изходи") не е изпълнена докрай:

🔴 .data single-fetch payload е непокрита machine-readable повърхност. При ssr:true RRv7 сервира всеки loader и на GET /<път>.data. company.tsx loader-ът връща eik + displayName немаскирани, а headers() е publicCache(...) → отговорът минава през hardenResponse (само baseSecurityHeaders, без X-Robots-Tag) и се кешира на edge. Значи /companies/:eik.data връща немаскиран ЕИК на физическо лице, кеширан, без noindex — точно експозицията от #173, на повърхността, която PR-ът не изброява. (Потвърдих reachability-то на .data живо на публичния prod; robots.txt не покрива /*.data.) Поправка: приложи предиката/маската и за .data, или централизирай X-Robots-Tag в hardenResponse вместо per-route.

🟠 Sitemap-ът ползва по-тесния предикат. sitemaps.ts филтрира с isNaturalPersonProfileName (само по име), не с разширения isNaturalPersonBidder (+legal_form), който PR-ът ползва навсякъде другаде. ЕТ, разпознат само по legal_form, се маскира/noindex-ва навсякъде, но пак се рекламира в /sitemap-companies — каним crawl на страница, която политиката де-индексира.

За проверка (не потвърдено срещу данни): премахнатият consortium guard в isNaturalPersonBidder — нито един caller не филтрира kind==='consortium', та консорциум с водещ „ЕТ …" в името може да се over-маскира като „Частно лице". Струва си да се потвърди срещу корпуса, преди да се третира като дефект.

@lyubomir-bozhinov lyubomir-bozhinov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Инлайн котви към горния преглед — .data single-fetch privacy gap.

Comment thread apps/web/app/routes/company.tsx Outdated
Comment thread apps/web/app/routes/contract.json.tsx
@ydimitrof

Copy link
Copy Markdown
Contributor

Прегледах PR #183 изцяло — целия diff (24 файла, +1882/−45), всички коментари по ревюто, issue #173, както и текущото състояние на кода в работното дърво (sitemaps.ts, details.ts, company.tsx render). По-долу е обобщението.


Обобщение

Централизацията на X-Robots-Tag: noindex през маркера X-Privacy-Mask в hardenResponse е чиста и добре тествана. Маскирането на CSV става на query слоя преди записа в R2 (streamContractsCsv / streamCompaniesCsv), което е правилният ред — edge кешът не може да сервира немаскиран ред. Реакцията на предишното ревю (.data близнакът и per-route → централизиран автор) е адресирана коректно. Тестовете (38 нови) са смислени, не тривиални. Двете конкретни забележки от предишния преглед обаче не са затворени докрай, а при локалната проверка изникнаха и нови пропуски в същия клас CWE-359.

Сигурност / SQL / OWASP

  • SQL инжекция: няма. Всички нови SQL фрагменти (b.legal_form AS bidder_legal_form, LEFT JOIN bidders AS b ON b.id = ct.bidder_id) са статични идентификатори; параметрите остават през ?-binding. LEFT JOIN е 1:1 (bidders.id е PK), няма fan-out на редовете.
  • Секрети / обфускация / backdoor: няма. safeJson продължава да екранира <, > и line separators.
  • OWASP: A01/A04 се подобряват; не се въвежда нова инжекционна повърхност.

🔴 Съществени пропуски (data-integrity / поверителност)

1. Подизпълнителят (subcontractor) остава немаскиран в /contracts/:id.json. maskContractForPrivacy маскира само bidder; getContract дори не селектира legal_form на подизпълнителя. При договор с юридическо лице като изпълнител и едноличен търговец като подизпълнител, maskContractForPrivacy връща записа по референция (masked === record) → маркерът не се слага → отговорът е без noindex, кеширан, с непокрит ЕИК и оригинално име на физическото лице (subcontractor.eik = r.subcontractor_eik, subcontractor.name). Това е точно експозицията от #173 („any machine-readable record carrying a natural-person identifier"), на повърхността, която PR-ът твърди, че покрива изцяло. Рядка (~0.8% договори), но е същият клас уязвимост, който PR-ът затваря.

2. Sitemap-ът ползва по-тесния предикат (незатворена забележка от предишното ревю). packages/db/src/queries/sitemaps.ts:108 филтрира с isNaturalPersonProfileName (само по име), не с разширения isNaturalPersonBidder (+legal_form). ЕТ, разпознат само по legal_form (име без водещо „ЕТ "), се маскира и получава noindex навсякъде другаде и в HTML профила, но продължава да се обявява в /sitemap-companies — активно каним crawl на страница, която политиката де-индексира.

3. HTML профилът скрива ЕИК на физическите лица — противоречи на собствената документация в PR-а. Споделеният loader прави company.eik = null (company.tsx:77) и за HTML отговора. В render-а {c.hasEik && c.eik && (…ЕИК…)} (company.tsx:126) става falsy → блокът с ЕИК изчезва. А новата секция в privacy.tsx изрично уверява потребителя, че „HTML профилът … остава непокътнат по съдържание — името, ЕИК и всички останали полета се показват както в първичния източник." Поведението е по-защитно (fail-safe), но е необявена функционална промяна и прави потребителската документация невярна.

🟠 По-малки бележки

4. .data близнакът връща оригиналното име немаскирано, докато privacy.tsx твърди, че за /companies/:eik.data „ЕИК и оригиналното име … се заменят с неутрален етикет". Реално се нулира само eik; displayName остава дословен (тестът company.data.test.ts го потвърждава). Тъй като HTML показва същото име и .data вече носи noindex, експозицията ≈ HTML — но документацията надценява маскирането.

5. Възможно over-маскиране на консорциум. isNaturalPersonBidder премахна консорциум-гарда; CSV-каналът за договори не филтрира kind, така че консорциум с водещ „ЕТ …" в display name-а (пръв член ЕТ) би се маскирал като „Частно лице". Fail-safe, не е теч — струва си потвърждение срещу корпуса.

Съответствие с issue #173

Ядрото на issue-то (bulk-indexable ЕИК на ЕТ в .json + .csv) е адресирано за изпълнителя. Остатъчните повърхности от т.1 (подизпълнител) и т.2 (sitemap) означават, че тезата „noindex+mask на всички machine-readable изходи" още не е напълно изпълнена.

Бележки

  • CI: „no checks reported on the 'main' branch" — PR-ът е от форк с branch main; препоръчвам да се провери дали required checks реално минават преди merge.
  • pnpm lint е exit 1 с нови prettier warnings в scope (contract.json.test.ts, privacy.tsx, companies.ts, companies.test.ts) — CI е конфигуриран като blocking lint (2d93cd5), така че тези трябва да се forматират.

Вердикт: Заявка за промени (Request changes) — блокиращо е т.1 (немаскиран ЕИК/име на подизпълнител-физическо лице в /contracts/:id.json, без noindex) и т.2 (sitemap с по-тесен предикат); т.3 изисква или коригиране на кода, или коригиране на потребителската документация, за да не е подвеждаща.

Благодаря за прегледната работа по маркерния договор и тестовете — след затварянето на горните пропуски PR-ът ще е в много добра форма.

@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Блокерът е затворен — ре-проверих на връх d55a1cc:

  • ЕИК маскирането е на ниво заявка (companies.ts:271const eik = isNatural ? '' : r.eik), значи важи автоматично и за /companies/:eik.data близнака (един и същ loader обслужва HTML и .data).
  • noindex е централизиран на worker ниво: app.ts hardenResponse превежда X-Privacy-Mask маркера в X-Robots-Tag: noindex и го маха преди кеширане; app.nofollow.test.ts покрива изрично .data близнаци — физ. лице → noindex (T-008), юр. лице → без маркер (негативен fixture).

Точно това затваря находката ми (немаскиран ЕИК на физ. лице, кеширан, без noindex, изтичащ през .data). Одобрявам по същество.

Единствено: branch-ът е в конфликт с main (mergeable_state=dirty) — rebase преди merge.

LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 6, 2026
…mber worker adr to 0008

Rebase of midt-bg#183 onto upstream/main (post-midt-bg#182 ADR reorganization) restructured the privacy-policy and worker-level X-Robots-Tag ADRs to live in docs/adr/ rather than inline in docs/architecture.md:

- New docs/adr/0007-privacy-masking.md — content extracted from the inline ADR-0002 in architecture.md; relative paths adjusted (../ → ../../) for the new adr/ location; cross-link to the worker ADR now points to 0008.
- docs/adr/0003-centralized-x-robots-tag-worker.md → docs/adr/0008-centralized-x-robots-tag-worker.md — renumbered to free the 0003 slot taken by upstream's value-flag ADR; internal cross-link from architecture.md#adr-0002-... to 0007-privacy-masking.md.
- docs/adr/README.md — index extended with the two new entries.
- docs/architecture.md — adopted upstream's short summary form; the inline ADR-0001+0002 contents are removed (the rendering ADR lives at adr/0001-rendering-and-security.md and the privacy policy at adr/0007-privacy-masking.md); Решения (ADR) section now also points to 0007 and 0008.
- docs/privacy-masking.md — cross-link from architecture.md#adr-0002-... to adr/0007-privacy-masking.md; ADR-0003 to ADR-0008.

No code changes; verified pnpm check:docs (docs-integrity gate from midt-bg#182) passes.
LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 6, 2026
The three files modified by PR midt-bg#183 carried pre-existing prettier debt that the original review flagged (`pnpm lint` exit 1 with `contract.json.test.ts`, `companies.test.ts`, `companies.ts`). The repo's CI is configured as blocking lint (`2d93cd5`, comment in .github/workflows/ci.yml), so this would have blocked the PR from merging. Run `pnpm prettier --write` on the three files — no semantic changes.
@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Прегледах отново новия връх 468a116 — доста по-широк (CSV/JSON/.data/company маскирани, ADR-и), добра работа. Но adversarial pass показва, че маскирането е закачено за export/twin routes, а СПОДЕЛЕНИТЕ проекции остават немаскирани — та ЕИК на физ. лице (ЕТ) още изтича на най-видимите ИНДЕКСИРАНИ HTML страници:

1) Списък „Фирми" (companies.tsx). listCompanies мапва през toCompanyListItem (rows.ts:58 eik: r.eik — суров; rows.ts е +1/-0, проекцията не е пипана). companies.tsx:70 рендира „ЕИК {c.eik}". ЕТ → ЕИК на индексиран списък.

2) Начална страница (home.tsx). getHomeDatatopCompanies: companies.results.map(toCompanyListItem) (home.ts:84) — същата немаскирана проекция; home.tsx:196 показва ЕИК на топ-10. ЕТ в топ-10 → ЕИК на най-трафикираната страница.

3) Страница на договора (contract.tsx). Не е в PR-а; getContract (details.ts:583 eik: r.bidder_eik суров) → contract.tsx:221-222 показва „ЕИК {c.bidder.eik}" + линк към регистъра. .json близнакът маскира, HTML — не.

4) Подизпълнител. maskContractForPrivacy маскира само bidder, не subcontractor; contract.tsx:258-263 рендира c.subcontractor.eik суров. Изтича и на HTML, и на .json.

isNaturalPersonBidder хваща ЕТ (format.ts:210), а ЕТ има валиден 9-цифрен ЕИК (eik_valid=1) → hasEik е true и се рендира — точно случаите, които streamCompaniesCsv вече маскира (companies.ts:270). CSV/JSON са затворени; голите HTML страници — не.

Корен (altitude): маскирането е на leaf routes/exports, не на споделените проекции. Устойчивият фикс: маскирай в toCompanyListItem (rows.ts — има name/kind/legal_form → isNaturalPersonBidder) и в getContract/getContractDetail (details.ts — има bidder_name/bidder_legal_form + subcontractor_name за name-based проверка). Тогава списък, начална, договор, .data наследяват маската веднъж — вместо всяка нова повърхност да помни да маскира (CSV помни, списъкът — не).

PR-ът затваря буквата на #173 (.json/.csv), но не и духа — ЕТ ЕИК на индексираните HTML страници. Блокер до фикса на споделените проекции.

@nedda76 nedda76 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Прегледах PR-а спрямо #173. Механиката marker→header е добре построена и тествана: CSV се маскира преди записа в R2, worker-ът слага X-Robots-Tag: noindex и трие marker-а преди edge cache (cache-safe — app.nofollow.test.ts го покрива). Но маската не покрива всяка machine-readable повърхност и на места е обезсилена — има потвърдени пътища, по които суровият ЕИК на физическо лице все още изтича. Тоест PR-ът още не затваря #173.

Блокиращи — потвърдени течове на суров ЕИК

  1. slug пресейва ЕИК-а. Маската нулира eik, но оставя slug — а за eik-базирани субекти slug е самият нормализиран ЕИК (companySlug('eik:222222222') === '222222222'). Тоест bidder.slug / company.slug в /contracts/:id.json и /companies/:eik.data носят точно идентификатора, който маската маха. ADR-0007 („slug-ът не е PII") не важи за физически лица — а те са именно маскираната популация.

  2. /contracts/:id.data не е маскиран. HTML маршрутът contract.tsx е недокоснат; при ssr:true single-fetch /contracts/:id.data сервира payload-а на loader-а машинно четимо — bidder.eik и subcontractor.eik, без noindex. .json е маскиран, но .data близнакът му — не (точно класът повърхности, заради който съществува ADR-0008).

  3. Подизпълнителят не се маскира. maskContractForPrivacy пипа само bidder + sourceNames.bidder; subcontractor минава непокътнат, а подизпълнителят може да е ЕТ/физическо лице → ЕИК изтича. Рядко (~0.8% от договорите), но реален непокрит път.

  4. /companies.data (списък) не е маскиран. /companies.csv е, но .data близнакът на списъчния маршрут връща CompanyListItem със суров eik + име за физически лица → точно „bulk searchable/downloadable" вредата от #173, само през .data вместо .csv.

За обсъждане — по-нисък приоритет

  1. Мрежовият граф в маскирания company.data носи node id-та eik:<ЕИК> → физическо лице като възел изтича ЕИК. Частично фундаментално за eik-базираните id-та (като #1).
  2. Регресия за легитимни фирми: предикатът пада към name-евристика (ЕТ /ET префикс) дори при реален legal_form (ООД) → фирма с име „ET Engineering" получава скрит валиден ЕИК (нарушава изискване #5). Евристиката е заварена, но сега тя контролира скриване на ЕИК, не само мек noindex — цената на false positive расте.
  3. Несъгласуваност: .json/.csv заменят името с етикет, но company.data/HTML пазят ЕТ името дословно. Трите machine-readable пътя маскират различни полета.

Чисто (за фокус)

marker→header плъмбингът и cache safety; обединеният предикат isNaturalPersonBidder (и двата source() клона проектират legal_form); запазеното показване на юридически лица по покритите пътища; authorities.csv (публични органи — умишлено без body-маска).

Коренът на #1/#2/#4 е един: маската покрива .json/.csv, но не и .data близнаците и не пипа slug. Докато .data повърхностите и slug не се покрият, #173 остава отворен.

@ydimitrof ydimitrof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Преглед на PR #173 — „fix(privacy): noindex + маскиране за машинно-четими изходи"

Какво прави PR-ът

Прилага политика за поверителност върху машинно-четимите изходи (turbo-stream .data, JSON, CSV), така че идентификаторите и имената на физически лица/ЕТ да бъдат маскирани, а не само маркирани с noindex. Архитектурата е чиста и с ясно разделение на отговорностите: route-овете маркират чрез markPrivacyMaskApplied, worker-ът превежда маркера в X-Robots-Tag: noindex чрез applyPrivacyMaskHeaders и го изтрива преди кеш/клиент. Въвежда се единен предикат isNaturalPersonBidder и константа MASKED_NATURAL_PERSON_LABEL в @sigma/shared, преизползвани в streamCompaniesCsv и streamContractsCsv; legal_form се прокарва коректно през двата клона на source() и през getContract.

Сигурност (Фаза 0)

ЧИСТО и в двете партиди — няма зашити тайни/ключове, нови зависимости, нови или подозрителни URL адреси, нито зловредни шаблони. Промяната всъщност намалява изтичането на лични данни. Тестовото покритие е силно и смислено: покрити са MISS/HIT/dynamic, идемпотентност, edge-cache инвариантите, негативните случаи, двата клона на source(), name-евристиката при legal_form=null и таблица на истинност на предиката. Документацията (ADR-0007, ADR-0008, privacy-masking.md) е изчерпателна и синхронизирана с кода.

Блокираща забележка

  1. (Средно — поверителност/консистентност) Машинно-четимият близнак /companies/:eik.data изчиства само company.eik, но връща пълното име на физическото лице (displayName) буквално — за разлика от /contracts/:id.json и CSV, които заменят името с MASKED_NATURAL_PERSON_LABEL. Тъй като .data е машинно-четим изход, а самата обосновка на PR-а (ADR-0007) гласи, че noindex е недостатъчен срещу ботове, оставянето на суровото име само зад noindex противоречи на декларираната цел. Тестът company.data.test.ts дори утвърждава това като очаквано. Нужно е явно решение: да се маскира името и в .data, или изрично да се документира защо .data остава немаскиран.

Некритични забележки

  1. (Ниско — поверителност) При маскиране bidder.slug / company.slug се запазват. Ако slug-овете се извеждат от името на субекта, URL-фрагментът може да разкрие идентичността въпреки маскирането на name/eik. ADR-0007 приема slug като „URL фрагмент, не PII", но не адресира случая на slug, изведен от име.
  2. (Ниско — точност на маскирането) isNaturalPersonBidder се вика без предварителна проверка за bidder_kind, въпреки че документацията му възлага филтрирането на консорциумите на викащия. Консорциум с име, започващо с „ЕТ ", ще бъде over-маскиран като „Частно лице" — privacy-safe, но с загуба на информация.
  3. (Ниско — DB/производителност) Клонът по подразбиране на source() вече винаги прави LEFT JOIN bidders, а това е и hot path за listCompanies. Да се потвърди, че company_totals няма собствена колона legal_form (иначе ct.* + b.legal_form дава двусмислена дублирана колона) и че има индекс по bidders.id.
  4. (Ниско — дублиран/мъртъв код) Няколко случая на дублиране, противоречащи на правилото „NO CODE DUPLICATION": излишните предварителни извиквания на markPrivacyMaskApplied в responseFromR2Object (csv-export.ts), които markCsvCache веднага презаписва; и inline вариант на правилата за legal_form в apps/web/app/routes/company.tsx.
  5. (За потвърждение) details.ts прокарва bidder_legal_form без маскиране и без тест в прегледаната партида — да се потвърди, че JSON маскирането се извършва другаде.

Вердикт

Промяната е висококачествена и без изтичане на данни в прегледаните файлове. Единствената блокираща точка е несъответствието в поверителността при /companies/:eik.data (т.1) в PR, чиято цел е именно защита на лични данни — тя трябва да бъде адресирана или изрично обоснована преди одобрение. Останалите забележки са некритични.

Comment thread apps/web/app/routes/company.tsx
Comment thread apps/web/app/routes/contract.json.tsx
Comment thread apps/web/app/lib/csv-export.ts Outdated
Comment thread packages/db/src/queries/companies.ts Outdated
Comment thread packages/db/src/queries/contracts.ts Outdated
Comment thread packages/shared/src/format.ts Outdated

@lyubomir-bozhinov lyubomir-bozhinov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Благодаря — маскирането на .json и CSV е издържано. Но остава отворена точно най-чувствителната machine-readable повърхност, заради която е #173: detail страницата на договора и нейният .data twin.

apps/web/app/routes/contract.tsx — loader-ът (:62-67) връща { contract } суров; файлът не се пипа в този PR и няма нито маска, нито noindex. getContract селектира b.eik_normalized AS bidder_eik (packages/db/src/queries/details.ts:440) и го отдава суров (:583 eik: r.bidder_eik, :595 eik: r.subcontractor_eik). contract.tsx ги рендира без маска — :221 ЕИК на изпълнителя, :261 ЕИК на подизпълнителя. Worker-ът прилага само applyPrivacyMaskHeaders(headers) (header-only, noindex) — няма body маска на ниво worker.

Ефект (prod е публичен, unauthenticated):

  • GET /contracts/<slug>.data → turbo-stream body с bidder.eik / subcontractor.eik на физическо лице (ЕТ), немаскиран и без X-Robots-Tag.
  • GET /contracts/<slug> (HTML) → същият ЕИК, индексируем.

Същата политика, различно прилагане и на списъците:

  • /companies.data: toCompanyListItem (packages/db/src/queries/rows.ts:58) връща eik: r.eik без isNaturalPersonBidder проверка, докато /companies.csv го маскира.
  • /contracts.data: toItem в contracts.ts дава немаскирани имена на физически лица (CSV пътят вече маскира).

Предложение: маскирай в споделения слой — в getContract, или в loader-а на contract.tsx огледално на company.tsx:77-85 — за да го наследят HTML, .data и .json, вместо per-route. Същото за toCompanyListItem (редът вече носи legal_form) и за подизпълнителя в maskContractForPrivacy.

Докато .data twin-ът не минава през същата маска като .json, #173 не е затворен. Проверих горните редове на HEAD (468a116). Блокиращо за merge.

LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 27, 2026
…port

The R2-body branch (responseFromR2Object) and the 304 branch each called
markPrivacyMaskApplied directly, then handed the response to markCsvCache,
which calls it again internally. The marker was applied twice on MISS/HIT/304
paths — idempotent in effect, but dead code that hid markCsvCache as the single
source of truth for the privacy marker on every CSV path (PR midt-bg#183 review T-004,
"NO DEAD CODE / NO CODE DUPLICATION").

Drop the direct calls; rely solely on markCsvCache. Add a TDD guard that spies
on markPrivacyMaskApplied and asserts exactly one call per response path
(MISS/HIT/dynamic/304), so a future duplicate cannot sneak back in.
LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 27, 2026
…ortium over-masking

isNaturalPersonBidder's docstring delegates consortium filtering to the caller —
a JV is a legal entity even if a lead member's name / legal_form matches a
sole-trader signal. But streamContractsCsv and streamCompaniesCsv both invoked
it WITHOUT a bidder_kind guard, so a consortium such as "ЕТ Иван Петров; Строй
ООД" (or any consortium whose legal_form collided with a sole-trader form) was
masked to MASKED_NATURAL_PERSON_LABEL with its ЕИК cleared.

The result was privacy-safe (over-masking, no leak) but a behavioral change
that dropped the lead member's name + ЕИК and contradicted the predicate's
contract. Add an early bidder_kind/kind !== 'consortium' guard in both
streamers so consortium rows keep the "… и др." shape and their ЕИК.

TDD: failing tests first (consortium with ЕТ lead name + ЕТ legal_form, and the
leading-ЕТ name heuristic with legal_form null), then the guard (PR midt-bg#183 T-006).
LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 27, 2026
…ne duplication)

The docstring claimed the legal_form rules were "carried inline in
apps/web/app/routes/company.tsx until the route migrates" — but ADR-0007 §1
already removed the legacy inline isSingleNaturalPersonProfile, and company.tsx
now calls this shared predicate directly (verified: no legal_form string-
matching exists outside packages/shared). The stale claim created exactly the
divergence risk the PR midt-bg#183 reviewer flagged under "NO CODE DUPLICATION": a
future reader could believe a second copy still lives in the route and maintain
it separately.

Rewrite the docstring to state the predicate is the single source of truth and
enumerate the downstream surfaces that consume it (HTML noindex, CSV masking,
JSON masking), with a pointer to the bidder_kind/kind consortium guards added
in the CSV streamers (PR midt-bg#183 T-006). No behavior change.
LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 27, 2026
…6, §7)

Two PR midt-bg#183 review threads asked for explicit product decisions on the company
profile masking surface. Both are recorded here as policy.

§6 — displayName stays visible in the HTML profile and its `.data` twin; only the
ЕИК is masked. The trading name is PUBLIC (rendered verbatim on the HTML page and
in <title>); the sensitive natural-person identifier is the ЕИК. The `.data`
turbo-stream is React Router v7's single-fetch transport for client-side
navigations, NOT a standalone export like /contracts/:id.json — masking the name
there would break client-rendered pages. Consistent policy: name = public, ЕИК =
sensitive. company.tsx loader comment now states this; the company.data.test.ts
assertion locks displayName-verbatim + eik-null as the contract.

§7 — the name-keyed natural-person slug (n + base64url(name)) is a tracked
limitation, not changed in this PR. The name is public (§6), the sitemap already
filters these records, and reworking the slug scheme is cross-cutting (URL
stability, internal links, identity system) and out of scope for a masking PR.

No behavior change.
@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Прегледах #183 на дълбочина срещу head a9b18ae (валидирано срещу дифа). Ядрото за CSV + /contracts/:id.json е солидно, но има два реални пропуска в покритието, които оставят точно този ЕИК незащитен.

Силната част (потвърдено):

  • CSV маскирането пази consortium коректно: contracts.ts:459 / companies.ts гейтват r.bidder_kind !== 'consortium' ПРЕДИ isNaturalPersonBidder, така че „ЕТ Иван Петров; Строй ООД" НЕ се маскира като физ. лице. Тестовете дискриминират (ЕТ маскиран, ООД дословно, consortium не; стабилни колони).
  • Marker plumbing-ът е идемпотентен: applyPrivacyMaskHeaders реагира само на точния литерал, трие маркера преди edge-cache и преди клиента; „marks exactly once" е покрит за MISS/HIT/304.
  • Маскира се и ЕИК→null, и име→етикет (Частно лице); без слабо частично ЕИК.

MAJOR 1 — JSON masker-ът НЯМА consortium guard-а, който CSV пътят има. contract.json.tsx:27: if (!isNaturalPersonBidder(record.bidder.name, bidderLegalForm)) return record; — за разлика от contracts.ts:459, тук record.bidder.kind не се проверява. Docstring-ът на isNaturalPersonBidder делегира consortium филтрирането на викащия, а този викащ не го прави → consortium с име, започващо с „ЕТ …" и legal_form = null, се over-маскира до „Частно лице" + noindex в JSON. Точно багът, който вече е поправен в CSV, не е пренесен тук; няма consortium тест в contract.json.test.ts. Фикс: същият bidder.kind !== 'consortium' гард (добави полето към record-а, ако липсва).

MAJOR 2 — най-изложената повърхност остава отворена: страницата на договора + .data близнакът ѝ. contract.tsx рендерира ЕИК на изпълнителя (:219-222) без noindex и без маркер (meta()/headers() не ги слагат), а robots.txt забранява само /search + /*.csv (robots.tsx:5) — НЕ .data/.json. Значи договор, спечелен от „ЕТ …", излага личния ЕИК на едноличния търговец през /contracts/:id И през /contracts/:id.data, индексируемо. #183 затваря .json/.csv (каквото #173 назовава), но ADR-0008 обещава „всяка повърхност наследява noindex" — а дизайнът е marker-based, т.е. наследява само ако route-ът сложи маркера, а тази повърхност не го слага. Понеже страницата на договора е сред най-обхожданите, това е по-голямата дупка от .json. Препоръка: сложи маркера на contract.tsx (worker-ът ще покрие и HTML, и .data).

MAJOR 3 (test-gap + архитектура) — препращането на маркера към worker-а за .data не е доказано end-to-end. Механизмът зависи RR v7 да пренесе loader header-а върху реалния .data HTTP отговор; нито един тест не кара реалния single-fetch pipeline (мокват createRequestHandler / викат loader() директно). Ако RR не го препрати, .data близнакът на компанията тихо тръгва без X-Robots-Tag — зелени тестове, скрит пропуск. За сравнение: седмичният дайджест реши същия .data проблем path-based на worker-а (workers/app.ts DIGEST_DETAIL_PATH мачва и /weeks/:iso.data), което не зависи от RR forwarding. Същият подход за машинно-четимите повърхности е по-надежден + добави реален .data e2e тест.

MINOR — не-ЕТ физически лица могат да минат немаскирани. isNaturalPersonBidder (format.ts:210) хваща само ЕТ (legal_form или префикс „ЕТ "). Физ. лице с голо име и legal_form = null минава с ЕИК. #173 е скоупнат до ЕТ, ок като документирано ограничение — но копито в privacy.tsx казва „физическо лице или едноличен търговец", по-широко от това, което предикатът реално лови.

NITauthorities.csv и другите вече носят X-Robots-Tag: noindex (безусловен marker в markCsvCache). По ADR за консистентност, без маскиране на тялото — но е поведенческа промяна за журналисти/инструменти, разчитащи на discovery; струва си да е изрично в release бележките.

PR-ът е CONFLICTING спрямо main — нужен е rebase (отделно от горното).

Насоката е правилна; двата masker-а — consortium guard в JSON и покриване на страницата/.data — са това, което да се затвори преди merge.

LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 28, 2026
… path

The `/contracts/:id.json` masker (`maskContractForPrivacy`) lacked the
`bidder_kind !== 'consortium'` guard that the CSV streamer already has
(`contracts.ts:459`). A consortium whose display name begins with „ЕТ "
(first member is a sole trader, e.g. „ЕТ Иван Петров; Строй ООД") was
over-masked to „Частно лице" — losing the „… и др." shape, the consortium
ЕИК, and gaining an unearned `noindex`.

`isNaturalPersonBidder`'s docstring delegates consortium filtering to the
caller; this adds the caller guard, mirroring the CSV path exactly. Flagged
as MAJOR 1 in the PR midt-bg#183 review of head a9b18ae.

TDD: failing consortium cases first (name-based + legal_form-based, plus a
loader-level marker-omission case), then the guard.
LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 28, 2026
`contract.tsx` was the most-indexable surface still open: its loader returned
`{ contract }` raw with no privacy marker, `robots.txt` does not block
`/contracts/:id` (or its `.data` twin), and the page rendered `c.bidder.eik`
verbatim — so a sole-trader's ЕИК was indexable on both the HTML page and the
RRv7 single-fetch `.data` payload. That is a worse exposure than the already-
closed `.json`/`.csv` paths.

Masking + signalling in the SHARED loader covers both surfaces at once (the
`.data` twin reuses the same loader), mirroring `company.tsx:89` exactly:
ЕИК (the sensitive natural-person ID) → null on the returned object, the
trading displayName stays PUBLIC (ADR-0007 §6), and the `X-Privacy-Mask:
applied` marker is translated to `X-Robots-Tag: noindex` by the worker. The
`kind === 'consortium'` guard matches the JSON masker (MAJOR 1) and the CSV
streamer so a JV is never over-masked/noindexed. `headers()` forwards the
marker onto the HTML response (RR does not auto-propagate loader headers).
Flagged as MAJOR 2 in the PR midt-bg#183 review of head a9b18ae.

TDD: failing loader/headers/pipeline cases first, then the loader change.
LyuboslavLyubenov added a commit to LyuboslavLyubenov/sigma that referenced this pull request Jul 28, 2026
…eal worker

The PR midt-bg#183 review (MAJOR 3) noted the marker→`.data`→`X-Robots-Tag` forwarding
was only proven through fixtures that INJECT the marker by hand in the stubbed
RR handler — which proves the worker CAN translate a marker, not that a real
loader's marker survives the pipeline to the final `.data` HTTP response. That
left a „green tests, hidden gap" risk on the most-indexable surface.

Add four cases driving the REAL `worker.fetch` (→ handleRequest → hardenResponse
→ applyPrivacyMaskHeaders → edgeCache.put) against `/contracts/<x>.data`:
masked sole-trader → noindex + marker stripped + masked body preserved; cached
entry carries noindex (HIT-path invariant); second request HITs and serves
noindex verbatim; legal-entity negative (no marker → no noindex). The handler
returns the exact shape `contract.tsx`'s masked loader branch now produces
(MAJOR 2), so this is an honest end-to-end proof of the forwarding guarantee.

Note: the review's suggested path-based worker match (the weekly-digest
`DIGEST_DETAIL_PATH` precedent) does not exist in this codebase — the worker
does no path-based matching; the marker-based design (ADR-0008) is the
established architecture and is sound, so this keeps it.
@LyuboslavLyubenov

Copy link
Copy Markdown
Author

@lyubomir-bozhinov Благодаря за задълбочения adversarial pass — трите MAJOR забележки са затворени в три фокусирани комита върху a9b18ae:

MAJOR 1 — consortium guard в JSON masker-а (59bcece). maskContractForPrivacy (contract.json.tsx) вече gating-ва record.bidder.kind === 'consortium' ПРЕДИ isNaturalPersonBidder, огледално на CSV пътя (contracts.ts:459, bidder_kind !== 'consortium'). Консорциум с водещ „ЕТ …" вече не се over-маскира до „Частно лице" / не получава noindex. Докстринг-ът на isNaturalPersonBidder делегира consortium филтрирането на викащия — guard-ът е точно този caller. TDD: failing случаи първо (name-based + legal_form-based + loader-level маркер-омисия), после guard-а.

MAJOR 2 — маскиране на страницата на договора + .data близнака (0ea9230). contract.tsx беше най-индексируемата отворена повърхност: loader-ът връщаше { contract } суров, без маркер, robots.txt не блокира /contracts/:id (нито .data близнака), а страницата рендираше c.bidder.eik буквално. Сега маскирането + сигнализирането са в споделения loader (затова покриват HTML + .data едновременно), огледално на company.tsx:89: ЕИК → null, displayName остава ПУБЛИЧЕН (ADR-0007 §6), X-Privacy-Mask: applied се превежда в X-Robots-Tag: noindex от worker-а. kind === 'consortium' guard-ът от MAJOR 1 е приложен и тук. headers() препраща маркера (RR не auto-пропагира loader headers). TDD: failing loader/headers/pipeline случаи, после промяната.

MAJOR 3 — end-to-end доказателство, че маркерът достига X-Robots-Tag на .data през реалния worker (5acd42b). Добавени 4 случая в app.nofollow.test.ts, които драйвят реалния worker.fetch (→ handleRequesthardenResponseapplyPrivacyMaskHeadersedgeCache.put) срещу /contracts/<x>.data с body, носещ точно формата, който новият loader produce-ва (маскиран ЕИК + маркер): masked sole-trader → noindex + маркер изтрит + body запазен; кеширан entry носи noindex (HIT-инвариант); втори request HIT-ва и сервира noindex verbatim; legal-entity негативен (без маркер → без noindex). Предишните fixtures инжектираха маркера на ръка в stub-натия handler — доказваха само че worker-ът може да преведе маркер, не че реален loader-маркер оцелява. Това затваря „green tests, hidden gap" риска.

Забележка към предложението ти за path-based worker match (седмичният digest DIGEST_DETAIL_PATH): този precedent не съществува в кода — worker-ът (app.ts) няма path-based matching, архитектурата е изцяло marker-based (ADR-0008) и е коректна, така че не въвеждам path-based схема.

За останалите ти точки (не са код-промени в този PR):

  • NIT (authorities.csv безусловен noindex) — умишлено по ADR-0007 §last (политическа консистентност, без body-маска). Ще го отбележа изрично в release notes.
  • MINOR (не-ЕТ физически лица с голо име) — документирано ограничение: Privacy: .json/.csv expose natural-person ЕИК without the noindex applied to HTML profiles #173 е скоупнат до ЕТ, а privacy.tsx е по-широко формулиран. Отделна задача за разширяване на предиката.
  • Подизпълнителят (т.4 от по-ранен пас) — subcontractor няма legal_form колона в заявката (details.ts строи обекта само от subcontractor_name + subcontractor_eik), така че маскирането му изисква query промяна + policy решение извън ADR-0007 §3. Остава като проследявано ограничение за follow-up PR — не го сгъвам в този бранч (AGENTS.md scope).

Rebase: PR-ът е CONFLICTING спрямо main (22 upstream комита, включително identity-system af4977e/f7a3e50 и value-base 463e22a/210fd88, с тежко припокриване точно в details.ts/contracts.ts/companies.ts/contract.tsx/format.ts). Не съм ребейзвал тук — коректното resolution изисква повторна валидация на masking-политиката срещу новата upstream identity система, което е отделна, по-голяма задача от този review пас, и не искам да пренаписвам 14 комита, които ревюиращите вече коментират. Оставям ребейза за merge-стъпката. Трите MAJOR фикс-а са independently reviewable върху текущия head 5acd42b.

Пълна локална проверка: pnpm --filter @sigma/web test → 385 passing (0 new failures), pnpm --filter @sigma/web typecheck → exit 0, форматирано с prettier.

LyuboslavLyubenov and others added 4 commits July 28, 2026 13:52
…-Mask marker

The literal X-Robots-Tag header is no longer set anywhere in apps/web;
it is now written by exactly one helper (applyPrivacyMaskHeaders in
apps/web/app/lib/security.ts), called by the worker hardenResponse
after the base security headers and before the cacheable-HTML branch.

A new internal marker X-Privacy-Mask: applied is the route-side signal
that the response carries masked natural-person data. Route handlers
(csv-export.ts markCsvCache + 304 branch, contract.json.tsx loader) and
the worker consume that marker; it is deleted unconditionally before
the response is returned or stored in edgeCache.put so it never reaches
clients.

The HIT path in handleRequest (apps/web/workers/app.ts) is unchanged:
it copies cached.headers verbatim, and the cached entry is the
post-hardenResponse response, so the header survives the edge cache by
construction.

apps/web/workers/app.nofollow.test.ts (T-005 + T-008) exercises the
end-to-end worker flow for the .data twin of /companies/:eik and the
contract.json natural-person branch.

No edits to packages/db or to the public API contract;
bidders.legal_form stays server-only and company.eik stays on the
CompanyRecord type (masked to null by the loader, not removed).
…ough headers()

The single-fetch .data twin of the company profile now clears company.eik to null
on the natural-person branch (per isNaturalPersonBidder) and signals the
worker via the internal X-Privacy-Mask: applied header on the Response.json
return. The route's headers() export now destructures { loaderHeaders } from
Route.HeadersArgs and forwards the marker explicitly so the worker
hardenResponse can translate it into X-Robots-Tag: noindex on the HTML
response (getDocumentHeadersImpl only auto-propagates Set-Cookie).

Legal-entity records keep the plain-object return unchanged — no marker, no
mutation, no Response.json wrap. The not-found short-circuit (throw new
Response('Not Found', ...)) runs before the masking gate, so 404s never
carry the marker.

The HTML meta() noindex branch is unchanged — natural-person pages
continue to emit <meta name="robots" content="noindex"> via the existing
seoMeta + isNaturalPersonBidder gate. The new headers() forward adds a
redundant X-Robots-Tag: noindex HTTP header alongside the meta tag, which
is acceptable (the worker translates the marker for all responses).

apps/web/app/routes/company.data.test.ts is the focused new test suite
(7 tests across 5 describe blocks): natural-person loader return asserts
company.eik === null and X-Privacy-Mask: applied; legal-entity loader
return asserts a plain object with eik unchanged and no marker; headers()
test exercises both branches (marker present → forwarded + Cache-Control;
marker absent → Cache-Control only); meta() test covers the natural-person
noindex HTML tag; the worker-pipeline describe calls applyPrivacyMaskHeaders
on the loader return and asserts X-Robots-Tag: noindex is set while
X-Privacy-Mask is stripped, proving the worker translate end-to-end.
…rage

ADR-0002 (docs/architecture.md): the Решение section now describes the
centralized X-Robots-Tag: noindex write site (hardenResponse in
apps/web/workers/app.ts, via the applyPrivacyMaskHeaders helper in
apps/web/app/lib/security.ts). The bullet on per-route CSV/contract-json
writes is replaced by a single sentence naming hardenResponse, the marker
flow, and the deletion pre-edgeCache.put.

The Засегнати повърхности list grows to explicitly enumerate:
  - the .data twin of /companies/:eik (React Router v7 single-fetch,
    automatic via the shared loader in company.tsx)
  - apps/web/workers/app.ts (hardenResponse) as the centralized
    enforcement point under a new 'Worker — централизирана точка за
    прилагане' sub-heading
  - apps/web/app/lib/security.ts as the policy helper home, with
    PRIVACY_MASK_APPLIED as the literal-typed constant.

The privacy page (apps/web/app/routes/privacy.tsx) #natural-person-data
section grows to enumerate /companies/:eik.data alongside the existing
/contracts/:id.json and the three CSV exports. A follow-up paragraph in
Bulgarian prose explains that the X-Robots-Tag: noindex policy is now
applied uniformly at the worker edge so future machine-readable surfaces
inherit it automatically — without naming the X-Privacy-Mask marker or
the helper functions (user-facing wording only).

No edits to package.json, pnpm-lock.yaml, or the public API contract.
@LyuboslavLyubenov

Copy link
Copy Markdown
Author

Daily autonomous rebase — conflict resolved, PR now MERGEABLE

Head bumped from 93e816ee85f17c. Upstream main advanced by 5 ETL/cache-bust commits (etl double-count, related-persons registry evidence, etc.) since the last merge in this branch — the diff collided on docs/adr/README.md (PR's 0033-privacy-masking/0034-centralized-x-robots-tag-worker slots had been independently taken by upstream's registry-evidence ADRs).

What changed

Validation (clean tree, fresh pnpm install --frozen-lockfile)

  • pnpm typecheck — exit 0 (7/7 turbo tasks).
  • pnpm --filter @sigma/web test543 passing across 47 files; focused privacy suites (contract.json.test.ts, contract.data.test.ts, company.data.test.ts, csv-export.test.ts, app.nofollow.test.ts) — 79/79 passing.
  • pnpm --filter @sigma/shared test — 60/60 passing (isNaturalPersonBidder table-of-truth intact).
  • pnpm --filter @sigma/db test — pre-existing baseline failures (spawnSync sqlite3 ENOENT, etc., documented in the original PR description as identical to upstream main); none new from this rebase.
  • pnpm lint (prettier --check) — clean.
  • pnpm check:docs — clean.

Behaviour preserved through the merge

  • maskContractForPrivacy still gates record.bidder.kind === 'consortium' before isNaturalPersonBidder (MAJOR 1 of the original review).
  • The CSV streamer bidder_kind !== 'consortium' guard at packages/db/src/queries/contracts.ts:472 is intact.
  • company.tsx loader still zeros company.eik for natural persons on the SHARED loader (HTML + .data twin).
  • contract.tsx loader still zeros contract.bidder.eik on the SHARED loader (HTML + .data twin).
  • Worker-side marker → X-Robots-Tag: noindex translation (hardenResponseapplyPrivacyMaskHeaders) is unchanged.

Status

  • All 8 review threads already resolved before this pass.
  • No new threads opened or replied to.
  • mergeable: MERGEABLE, mergeStateStatus: BLOCKED — branch protection still requires a maintainer re-approval on the new head e85f17c before merge. I have not force-merged via --admin; leaving the approval/merge decision to maintainers.

@ydimitrof ydimitrof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ревю на PR #173 — политика noindex + маскиране за машинно-четими изходи

Какво прави PR-ът

Прилага единна политика за поверителност (маскиране + noindex) към машинно-четимите изходи (CSV/JSON). Маскирането на данни за физически лица (ЕИК → null, символен етикет MASKED_NATURAL_PERSON_LABEL) се извършва per-row преди запис в кеша/R2. Route слоят само поставя вътрешен маркер (X-Privacy-Mask / markCsvCache), а worker-ът (hardenResponse) го превежда до публичния X-Robots-Tag: noindex и премахва вътрешния маркер — чиста separation of concerns с единствен източник на истина. Добавени са ADR-0036/0037, оперативна документация и разширение на getContract.

Обща оценка

Логиката е солидна, а тестовото покритие — силно: проверяват се двата source() клона, случаите ЕТ/ООД/ДЗЗД, leading-ЕТ евристиката, консорциуми, идемпотентност („точно веднъж"), както и end-to-end worker pipeline-а (маркер → noindex, стрип преди кеш, HIT verbatim). Сигурността е чиста: няма тайни, нови URL/зависимости или злонамерени шаблони.

Блокиращи находки

  1. Оставен merge-conflict маркер <<<<<<< HEAD в docs/adr/README.md — счупва ADR индекс-таблицата и е признак за неразрешен merge. Трябва да се премахне преди merge.
  2. company.tsx (loader) — липсва guard за консорциум. За разлика от contract.tsx / contract.json.tsx, които добавят kind !== 'consortium' преди isNaturalPersonBidder(...), loader-ът за компанията маскира ЕИК за всяко съвпадение без изключение за консорциуми. Тъй като isNaturalPersonBidder делегира филтрирането на консорциуми на извикващия, консорциум с displayName, започващ с „ЕТ …", ще бъде над-маскиран. Коментарите в другите файлове твърдят, че „огледално повтарят company.tsx:89", но там guard няма — трябва да се изравни за консистентност на политиката.

Незначителни находки

  1. Грешни номера на ADR в docs/architecture.md(0033)/(0034) сочат към файлове 0036/0037.
  2. Възможна регресия в производителността: rollup-клонът на source() вече прави LEFT JOIN bidders за всички списъчни заявки (вкл. HTML listCompanies), макар legal_form да е нужен само на CSV стрийма.
  3. csv-export.tsmarkPrivacyMaskApplied се извиква безусловно в markCsvCache, докато docstring-ът гласи да се извиква само при маскирани данни за физическо лице. Ако blanket-политиката е умишлена (както подсказва privacy.tsx), актуализирайте docstring-а, за да не подвежда.

Заключение

REQUEST_CHANGES. Кодовата логика и тестовете са в добро състояние; блокиращи са conflict-маркерът в docs/adr/README.md и липсващият consortium-guard в company.tsx. След тяхната поправка (плюс номерата на ADR и джойн-а) PR-ът е близо до одобрение.

Comment thread apps/web/app/routes/company.tsx Outdated
Comment thread apps/web/app/lib/csv-export.ts
Comment thread docs/adr/README.md Outdated
Comment thread docs/architecture.md Outdated
Comment thread packages/db/src/queries/companies.ts Outdated
PR midt-bg#183 review (ydimitrof, 2026-08-18) flagged three doc issues:

- docs/adr/README.md had a stray `<<<<<<< HEAD` line on the index table
  (PR branch carried 0033-0034 from the privacy work; upstream brought
  0033-0037 from the registry-evidence work; the merge was botched and
  dropped the closing half of the conflict).
- docs/README.md had the same kind of conflict — both sides listed
  different amendment-implementation plans (midt-bg#305 vs midt-bg#306). Kept both.
- docs/architecture.md referenced ADR (0033)/(0034) but linked to the
  privacy ADRs at adr/0036-privacy-masking.md / 0037-centralized-x-robots-tag-worker.md.
  Fixed the visible numbers to (0036)/(0037) so the reader is not misled
  into looking up unrelated registry-evidence records.

Verified with `git grep -nE '^(<{7}|={7}|>{7})'` — no conflict markers remain.
PR midt-bg#183 review (ydimitrof, BLOCKING midt-bg#1, 2026-08-18) caught the same
over-masking hole that contract.tsx and contract.json.tsx already guard
against: `isNaturalPersonBidder` delegates consortium filtering to the
caller, so a ДЗЗД whose first member is an ЕТ ("ЕТ Иван Петров; Строй
ООД") was being over-masked to "Частно лице" with a zeroed ЕИК — exactly
the privacy-safe-but-information-losing case the contract siblings
already prevent.

Added a regression test (consortium-with-sole-trader-first-member) that
asserts the loader returns the plain object with `company.eik` unchanged
and no `X-Privacy-Mask` marker. Implemented the mirror guard:
`company.kind !== 'consortium'` precedes the `isNaturalPersonBidder`
check. Updated the inline ADR-0036 §3 comment to call out the symmetry
with the contract loaders.
PR midt-bg#183 review (ydimitrof, midt-bg#5, 2026-08-18): `source()` in
companies.ts was projecting `b.legal_form AS legal_form` (and doing a
LEFT JOIN on bidders) on every `listCompanies` query, but
`toCompanyListItem` does not consume it — only the CSV streamer
(`streamCompaniesCsv`) needs it for the natural-person masker. The
join is on PK so the cost was bounded, but on uncached list queries it
was wasted work.

Added a `legalForm` option to `source()` that controls only the
unfiltered rollup subquery. The base-aggregation CTE always projects
legal_form (it already INNER JOINs bidders for the GROUP BY, so the
projection is free, and keeping it consistent lets both consumers share
the same SQL when filters are active). Added two tests pinning the SQL
shape: list path must not contain `LEFT JOIN bidders` /
`b.legal_form AS legal_form` on the rollup branch; CSV path must.
…kApplied

PR midt-bg#183 review (ydimitrof, midt-bg#3, 2026-08-18): the docstring on
`markPrivacyMaskApplied` said "callers must invoke this only when
the response body contains masked natural-person data", but
`markCsvCache` in csv-export.ts invokes it unconditionally for every
CSV response. The blanket call is intentional — the policy documented
in `apps/web/app/routes/privacy.tsx` and `docs/privacy-masking.md`
applies `noindex` to all three public CSV exports regardless of body
content, because CSV is a bulk machine-readable surface — but the
docstring was misleading future callers.

Expanded the docstring to enumerate the two legitimate call sites
(per-row maskers vs blanket-policy surfaces) and reference the policy
docs. Added a regression test pinning the blanket behaviour across all
three CSV routes (contracts, companies, authorities) when the body
contains zero masked rows.
@LyuboslavLyubenov

Copy link
Copy Markdown
Author

Ежедневен автономен преглед — 5-те нишки от прегледа на ydimitrof (2026-08-18) са адресирани и затворени.

Commit-и:

  • 8521e4ddocs: clean leftover merge-conflict markers and align ADR numbers
  • 5d33ea5fix(privacy): guard company loader against consortium over-mask
  • 3cd5d23perf(db): project legal_form only on the CSV path
  • a13e9a5docs(security): document the blanket-CSV call site for markPrivacyMaskApplied

Какво е затворено:

  1. Блокиращ Add OG/Twitter meta, JSON-LD schema, sitemap #1 (consortium guard в company.tsx:89) — решен в 5d33ea5 с guard огледален на contract.tsx/contract.json.tsx и регресионен тест за ДЗЗД с ЕТ-водещ член.
  2. Блокиращ Грешка при опит за зареждане на "още X институции - виж всички договори" #2 (merge-conflict маркер в docs/adr/README.md) — решен в 8521e4d. Същият commit почиства и docs/README.md и подравнява ADR номерата в docs/architecture.md.
  3. Незначителен fix(web): guard og:image sub-properties behind imageUrl check #3 (docs/architecture.md номера 0033/0034 → 0036/0037) — решен в 8521e4d.
  4. Незначителен Multiple visual bugs -mobile #4 (LEFT JOIN bidders + b.legal_form AS legal_form на list path) — решен в 3cd5d23 чрез опция { legalForm: true } на source().
  5. Незначителен fix(web): retry transient D1 read failures on entity loaders #5 (markPrivacyMaskApplied docstring vs blanket CSV policy) — решен в a13e9a5 с разширен docstring (per-row vs blanket-policy call sites) и регресионен тест, който проверява blanket-а и в трите CSV route-а.

Верификация: pnpm vitest в apps/web (545 теста) и в packages/db/src/queries/ (208 теста) — зелени; pnpm typecheck — зелен; pnpm check:docs — зелен; pnpm prettier --check . — зелен.

HEAD на PR-а: a13e9a5.

@lyubomir-bozhinov lyubomir-bozhinov left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Потвърждавам срещу HEAD — това затваря моя CHANGES_REQUESTED за детайл-страницата на договора и .data близнака:

  • loader-ът нулира bidder.eik на споделения обект и връща Response.json с маркера → един loader покрива и HTML, и .data наведнъж; headers export-ът форуърдва X-Privacy-Mask явно (RR не авто-пропагира loader headers освен Set-Cookie), тъй че hardenResponse слага X-Robots-Tag: noindex и на двете повърхности.
  • consortium guard-ът (kind !== 'consortium') пази JV с водещ ЕТ от over-mask; contract.data.test.ts фиксира eik === null + маркера.

Проверих и bidder.slug — остава ЕИК (companySlug за eik:-ключ), но това е рутиращият ключ към noindex-натата /companies/:eik (както в company.tsx), не изтичане.

Чисто от моя страна. Мога да сменя ревюто на approve при merge, за да падне и формалният block.

@ydimitrof ydimitrof left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Преглед — batch 1/2: политика noindex+mask за машинно-четливи изходи (#173)

Фаза 0 — Сканиране за критична сигурност: ЧИСТО

  • Няма hardcoded тайни (API ключове, пароли, токени) в дифа.
  • Няма нови/променени външни URL адреси извън whitelist.
  • Няма зловредни шаблони (backdoors, code injection, обфускация).
  • Няма нови/променени зависимости.
  • Добавеният вътрешен header X-Privacy-Mask се изтрива от applyPrivacyMaskHeaders преди отговорът да достигне edge cache/клиента — маркерът не изтича навън.

Обща оценка

Промяната е добре структурирана, атомарна и с изключително добро тестово покритие (нови тестове за security, csv-export, company.data, contract.data, contract.json). Разделянето „route-слой поставя маркер → worker-слой го превежда в X-Robots-Tag: noindex“ е чисто и добре документирано. Consortium guard-овете (kind !== 'consortium') са консистентни между CSV streamer, JSON masker, contract.tsx и company.tsx loader и са добре покрити с тестове. Документацията в privacy.tsx е ясна и на български.

Намерения

  1. Несъответствие в company.tsx meta() спрямо loader-а за консорциуми (correctness) — виж inline коментара. meta() добавя robots: noindex за консорциум с водещо „ЕТ …“ в displayName, докато loader-ът умишлено НЕ маскира/noindex-ва консорциуми чрез kind !== 'consortium' guard. Това противоречи на изричния consortium guard, който е сърцевина на PR-а.

По-малки бележки (не блокиращи)

  • Стилова непоследователност в подхода за noindex: contract.json.tsx задава X-Robots-Tag: noindex директно, докато contract.tsx/company.tsx разчитат на вътрешния маркер, преведен от worker-а. Коментарът в contract.data.test.ts дори твърди „loader-ът НЕ трябва да емитира X-Robots-Tag директно — това е работа на worker-а (ADR-0037)“, което се разминава с реалното поведение на .json route-а. Двата подхода работят, но си струва да се уеднаквят или да се документира защо .json е изключение.
  • Липсва изричен Cache-Control на natural-person Response в company.tsx loader: връща Response.json(..., { headers: { 'X-Privacy-Mask': 'applied' } }) без Cache-Control, докато contract.tsx го задава изрично. headers() компенсира за document заявките, но е добре да е симетрично с contract.tsx за яснота и за .data пътя.

Качествени порти

  • Тестове: отлично покритие, тестовете са смислени (проверяват реалното маскиране, reference-equality, идемпотентност, blanket CSV политика, 304 клон) — не са тривиални.
  • Код: чист, следва установените шаблони, без дублиране (JSON masker използва shared serializer; маркерът е с единствен източник на истина в markCsvCache).
  • Сигурност (agent-level): маскира се чувствителният идентификатор (ЕИК); публичните имена остават verbatim по продуктова политика (ADR-0036). Няма SQL/XSS повърхности в дифа.
  • Документация: privacy.tsx е актуализирана и съответства на кода.

Препоръка: COMMENT — едно смислено несъответствие за адресиране (meta/loader консорциуми) + две минорни бележки; в останалото PR-ът е с високо качество.

Comment thread apps/web/app/routes/company.tsx Outdated
@LyuboslavLyubenov

Copy link
Copy Markdown
Author

Daily autonomous review — consortium guard в company meta()

Единствената отворена нишка на #183 (ydimitrof, 2026-08-19) е адресирана.

Commit: 3fd03affix(privacy): mirror the loader consortium guard in company meta()

Какво е затворено:

  • company.tsx:38meta() липсваше consortium guard. Предишният код извикваше isNaturalPersonBidder(...) без kind !== 'consortium' гард → ДЗЗД с водещ „ЕТ …" получаваше <meta name="robots" content="noindex">, въпреки че loader-ът изрично НЕ го маскира (огледалният гард беше добавен в 5d33ea5). Противоречие между HTML meta и loader/.data политиката.
  • Фиксът gating-ва isNaturalPersonBidder(...) с data.company.kind !== 'consortium' огледално на loader-а. Prose-клонът (kind === 'consortium' && membershipNote) остава непроменен — той е за консорциуми с нерезолнати членове.

TDD:

  • Добавени два failing теста в company.data.test.ts (describe('company.data meta() — consortium-with-sole-trader-first-member branch')):
    1. ДЗЗД с displayName: 'ЕТ ДРИФТ - НИКОЛАЙ КИРОВ; СТРОЙ ООД', legalForm: nullexpect(robots).toBeUndefined() (FAIL → PASS).
    2. Prose-консорциум с membershipNoteexpect(robots).toBeDefined() (PASS и преди, и след — регресионен гард).
  • След фикса: 10/10 в company.data.test.ts, 547/547 в @sigma/web, 60/60 в @sigma/shared, pnpm typecheck (7/7 пакета), pnpm prettier --check — зелени.

HEAD: 3fd03af (от a13e9a5).

Comment thread apps/web/app/routes/company.tsx
PR midt-bg#183 review (lyubomir-bozhinov, 2026-08-20) caught the same class
of .data/HTML asymmetry as the prior consortium guards: meta() emits
<meta robots noindex> for a prose-consortium (kind === 'consortium'
&& membershipNote), but the loader returned the plain object without
the X-Privacy-Mask marker, so the worker did not stamp X-Robots-Tag:
noindex on the .data twin. A crawler that doesn't honour <meta> would
index the raw membershipNote (which itself can carry identifying
names).

TDD: three new cases in company.data.test.ts pin the new branch
(marker set, ЕИК unchanged) and the negative case (membershipNote null
falls through to the plain-object path).

Implementation: a second guard in company.tsx loader mirrors the
natural-person branch — same Response.json wrap, same marker — but
without the field mutation, because the consortium ЕИК is a public
legal-entity identifier and must not be zeroed. ADR-0036 §8 records
the policy decision and cites the loader branch.

Verified: pnpm --filter @sigma/web test (550 passing, +3),
pnpm typecheck (7/7), pnpm prettier --check (clean),
pnpm check:docs (ok).
…upstream

Includes the prose-consortium noindex fix (1d316a3, this branch's new
commit) so it lands together with the upstream sync — single integration
point, single green run on CI.

# Conflicts:
#	docs/adr/README.md
@LyuboslavLyubenov

Copy link
Copy Markdown
Author

Daily autonomous review — prose-consortium noindex за .data близнака

Единствената отворена нишка на #183 (lyubomir-bozhinov, 2026-08-20) е адресирана и затворена.

Какво се случи

meta() на company.tsx:42-49 вече излъчваше <meta robots noindex> за prose-консорциум (kind === 'consortium' && membershipNote), но loader-ът връщаше гол обект без X-Privacy-Mask маркер. Worker-ът превежда само маркирани отговори в X-Robots-Tag: noindex, така че .data близнакът оставаше индексируем — точно дупката, която #173 затваря, но отворена за prose-консорциумите.

Commit-и (нови на върха на 3fd03af)

  1. 1d316a3fix(privacy): extend noindex to prose-consortium .data twin

    • Втори guard в company.tsx:120-125: при kind === 'consortium' && membershipNote връща Response.json(..., { headers: { 'X-Privacy-Mask': 'applied' } }) БЕЗ нулиране на полета (ЕИК на обединението е публичен).
    • 3 нови теста в company.data.test.ts: prose-консорциум с маркер, консорциум с membershipNote: null пада нататък (negative case), и worker pipeline превежда маркера end-to-end.
    • Нов §8 в ADR-0039 (privacy masking policy), който записва решението.
  2. 059d7bamerge upstream/main into PR #183: keep mergeable onto current upstream

Верификация

  • pnpm --filter @sigma/web test → 550 passing (преди: 547, +3 нови теста за prose-консорциум клона)
  • pnpm typecheck → 7/7 packages clean
  • pnpm lint → prettier --check clean
  • pnpm check:docs → ok — всички refs резолвват и всеки doc е индексиран

Нишки

  • Адресирана: lyubomir-bozhinov (2026-08-20) — prose-консорциум .data липса на noindex
  • Резолната: 1 (PRRT_kwDOS183M86ayE1i)

PR head: 059d7ba (преди: 3fd03af). Няма отворени нишки, няма отворени reviewer-ски активности.

@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Ядрото е издържано — маскирането в CSV pull() преди байтовете да стигнат R2, marker→X-Robots-Tag плъмбингът (security.ts/workers/app.ts, вкл. HIT пътя, който вгражда noindex в кешираното копие), и consortium over-mask гардът (kind !== 'consortium' преди isNaturalPersonBidder) са коректни; юридическите лица не се over-маскират. Проверих при HEAD 059d7bac. Три неща:

1) Major — leaderboard .data близнаците остават немаскирани (същият #173 клас)

PR-ът затваря .csv + detail страниците, но списъчните loader-и не минават през предиката:

  • toCompanyListItem (packages/db/src/queries/rows.ts:49) връща eik/name/displayName сурови — без isNaturalPersonBidder гейт, макар CompanyTotalsRow вече да носи legal_form.
  • companies.tsx:70 рендерира ЕИК ${c.eik} за всеки ред; meta() е обикновен seoMeta без noindex; headers() дава само Cache-Control/companies.data сервира сурово, индексируемо, без rate-limit.
  • listContracts (contracts.ts:261) — същото; маскиране има само в streamContractsCsv.

Конкретно: ЕТ на лидерборда → /companies.csv дава „Частно лице" + празен ЕИК + noindex, но /companies.data (машинно-четимият близнак на същия списък) връща {"eik":"…","displayName":"ЕТ …"} без noindex. company.tsx (детайлът) нарочно маскира точно този ЕИК като „sensitive ID" — и в HTML, и в .data. Значи спрямо собствената политика на PR-а (+ ADR-0039) списъчният близнак е теч от точно този CWE-359 клас, който #173 адресира. Фикс: прекарай реда на toCompanyListItem/listContracts през isNaturalPersonBidder (маскирай ЕИК+име) и сложи noindex на списъчните route-и — или изрично изведи лидерборда извън обхвата на #173.

2) Low — bidder_legal_form изтича към клиента въпреки „server-only" в описанието

getContract връща ContractRecord & { bidder_legal_form } (details.ts:469/750); contract.tsx връща { contract } / Response.json({ contract }) (редове 141/139), а contract.json.tsx:59-66 сериализира masked, който пази полето (спред на ...record). Значи .data, hydration payload-ът и /contracts/:id.json го носят. Безобидно е (публичен регистърен низ; етикетът „Частно лице" и без това подсказва ЕТ), но противоречи на инварианта в тялото — свали полето преди return/serialize, за да е вярно твърдението „не изтича към клиента".

3) Test-rigor — .data noindex-ът е доказан само срещу mock

app.nofollow.test.ts:1792 mock-ва createRequestHandler и връща ръчни Response-и с вече наличен X-Privacy-Mask, така че тестът доказва че worker-ът превежда marker-а — не че реален loader-set хедър оцелява до .data отговора (RRv7 getDocumentHeadersImpl форуърдва само Set-Cookie, вж. коментара в company.tsx/contract.tsx). За детайла payload-ът е маскиран на data слоя така или иначе, така че PII теч няма; но noindex сигналът върху .data не е доказан end-to-end — integration тест през lane-а (#177/#186) би го затворил.

The shared toCompanyListItem (used by /companies + /companies.data and the
home top-10) and toItem (used by /contracts + /contracts.data and the home
single-offer tables) returned ЕИК + source name verbatim for sole traders,
so the leaderboard HTML page AND its RRv7 single-fetch .data twin both
served the natural-person identifier un-masked — exactly the midt-bg#173 CWE-359
class the existing CSV/JSON streamers already guard against. This is the
third surface (PR midt-bg#183 review midt-bg#1).

Mirror the CSV streamer guard: bidder_kind !== 'consortium' &&
isNaturalPersonBidder(...) zeroes ЕИК, replaces name/displayName with
MASKED_NATURAL_PERSON_LABEL, and drops hasEik to false. JVs whose first
member is a sole trader (e.g. 'ЕТ Иван Петров; Строй ООД') keep their
consortium name + ЕИК verbatim, matching the existing consortium guards
in streamContractsCsv / streamCompaniesCsv / maskContractForPrivacy.

The contract list path gains a SELECT b.legal_form AS bidder_legal_form
projection on the shared SELECT/FROM block (listContracts,
listSingleOfferContracts, streamContractsCsv, contractsSummary) so the
masker has the sole-trader signal on every list query — base-aggregation
CTE was not touched, it already projects legal_form for its grouping.
…sked

The leaderboard list mappers now mask sole-trader rows (prior commit), but
the .data twin of /companies and /contracts is a separate machine-readable
surface — search engines that don't honour the HTML meta/noindex tag would
still index the masked row (PR midt-bg#183 review midt-bg#1).

Stamp the internal X-Privacy-Mask: applied marker when ANY item on the
page is masked, and forward it via the route's headers() export. The
worker hardenResponse → applyPrivacyMaskHeaders translates the marker
into X-Robots-Tag: noindex on the .data response and strips the marker
before the edge cache. Mirrors the company.tsx + contract.tsx per-row
pattern. Marker is internal — it never reaches the client.
…body

The masker maskContractForPrivacy widens its input to ContractRecord &
{ bidder_legal_form: string | null } so it has the sole-trader signal, but
the public ContractRecord API contract does NOT include that field — the
'not on the wire' invariant from the PR description was violated on every
loader branch:

  - masked branch: ...record spread preserved the extra field → JSON body
    carried the natural-person classifier alongside the masked name
  - passthrough branch (legal entity / consortium): the masker returns the
    record BY REFERENCE, so serializeJsonForScript(masked) serialized
    bidder_legal_form straight from getContract's widened return shape

Add an explicit destructure that strips the field on every branch (TDD:
two loader tests assert body.bidder_legal_form is undefined on both the
masked sole-trader path and the legal-entity passthrough path). The
masker's own masked branch also drops the field, defense-in-depth.
@lyubomir-bozhinov

Copy link
Copy Markdown
Collaborator

Подходът на маскирането е правилен, но при проверка на реален D1 излизат един blocker и една дупка в доказателството около .data.

1. Blocker (Major): /companies и /companies.data връщат 500 на реален D1

toCompanyListItem вече маскира по r.legal_form, а COLS селектира legal_form — но listCompanies чете нефилтрирания rollup през source(p), който пропуска JOIN-а към bidders и проекцията на b.legal_form. company_totals няма колона legal_form, затова заявката сочи несъществуваща колона → D1 гърми с 500 за целия leaderboard и неговия .data twin. Филтрираният път (?sector=…) минава през base-aggregation CTE, който вече проектира legal_form, затова 500-цата е само на default (нефилтрирания) /companies.

Unit lane-ът остава зелен, защото mock-ва D1Database.prepare (лови SQL текста, не го изпълнява) — затова се вижда само на реален D1 (напр. integration lane-а от #177).

Fix — една линия + обръщане на теста за source() projection:

// packages/db/src/queries/companies.ts — listCompanies
-  const src = source(p); // legal_form not needed — toCompanyListItem drops it
+  // toCompanyListItem masks on r.legal_form (#183); company_totals has no such column, so the
+  // rollup MUST LEFT JOIN bidders and project it — иначе /companies + /companies.data дават 500.
+  const src = source(p, { legalForm: true });

JOIN-ът е по primary key (b.id = ct.bidder_id) — не мени row set-а или COUNT(*). Тестът listCompanies source() projection — legal_form only when needed трябва да твърди обратното: JOIN-ът + проекцията са налице по list пътя (маскирането ги изисква), не само по CSV пътя.

2. Дупка в доказателството: noindex на .data е доказан само срещу mock

app.nofollow.test.ts ръчно инжектира X-Privacy-Mask в mock-нат createRequestHandler — доказва превода marker→X-Robots-Tag, но не че RRv7 single-fetch реално пуска headers export-а върху .data. (getDocumentHeadersImpl по подразбиране пренася само Set-Cookie; затова /conflicts+/search ползват worker path-rule, а leaderboard/detail разчитат на headers export-а да пренесе маркера.)

Проверих го емпирично на реален worker — lane-ът от #177 (wrangler.getPlatformProxy) и wrangler dev + curl (prod build); двете съвпадат без разминаване по .data header-ите:

Заявка HTTP X-Robots-Tag body
/companies.data (маскиран ЕТ ред) 200 noindex name="Частно лице", eik=null
/companies/<ет>.data 200 noindex eik=null, търговското име се пази (ADR-0039)
/companies.data?count=21-100 (само ЮЛ) 200 липсва ЕИК + име verbatim
/contracts.data (ЕТ изпълнител) 200 noindex bidderName="Частно лице"
/contracts/<id>.json (ЕТ) 200 noindex без bidder_legal_form, eik=null, име маскирано

Т.е. RRv7 7.18 изпълнява headers export-а на .data и пренася маркера — propagation-ът работи, X-Privacy-Mask се трие от клиентския отговор. Предлагам да го заковем с integration тест (върху lane-а от #177), за да не регресира при RRv7 ъпгрейд или изпуснат headers export. Всеки ред е доказано sensitive: изключиш ли маскирането → 4-те позитивни падат; махнеш ли само headers forward-а в companies.tsx → пада точно .data noindex assert-ът.

Предложен тест + 2 helper-а (пълен код)

apps/web/test/integration/privacy-noindex-data.test.ts

// 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 `/<path>.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<Response> {
  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<T extends { slug?: string; bidderSlug?: string }>(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/<et>.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/<id>.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<string, unknown>;

    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);
  });
});

apps/web/test/integration/helpers/single-fetch.ts (turbo-stream декодер, за да четат ред-по-ред реалните loader данни)

// Decoder for React Router v7's single-fetch (`/<path>.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 `{ "_<keyIndex>": 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<string, { data: unknown }> {
  const arr = JSON.parse(text) as unknown[];
  const cache = new Map<number, unknown>();

  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<string, unknown> = {};
    cache.set(ref, out);
    for (const [encodedKey, valueRef] of Object.entries(value)) {
      // Object keys are `_<keyIndex>` — 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<string, { data: unknown }>;
}

/** Read one route's decoded loader `data` from a decoded single-fetch payload. */
export function routeData<T = unknown>(
  decoded: Record<string, { data: unknown }>,
  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;
}

apps/web/test/integration/setup.ts — добавя seedRows() (seed на допълнителни редове в изолираната per-file D1 на lane-а):

export async function seedRows(statements: readonly string[]): Promise<void> {
  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);
    }
  }
}

seedRows минава statement-ите през stripSqlCommentsAndCollapse, защото DB.exec дели по нов ред.

Ако е удобно, мога да оформя fix-а като отделен commit/patch за pull.

…n't 500

PR midt-bg#183 review (lyubomir-bozhinov, 2026-08-24, MAJOR midt-bg#1): COLS in companies.ts
names 'legal_form', but the rollup subquery in source() only projects it on
the CSV path (3cd5d23 made it conditional to skip the LEFT JOIN on the list
hot path). The conditional predated the masking mapper added in 3458dae —
toCompanyListItem reads r.legal_form for the sole-trader mask, so on real D1
the SELECT fails with 'no such column: legal_form' and /companies +
/companies.data return 500. The mocked-DB unit suite never executes the SQL
and shipped the bug.

Restore the LEFT JOIN + projection on the rollup branch (PK lookup, bounded
cost), flip the unit test that asserted the broken shape, and add an
end-to-end SQL test against real node:sqlite that pins the real-D1 behavior
(sole trader masked, ООД verbatim, consortium not over-masked, CSV parity).
@LyuboslavLyubenov

Copy link
Copy Markdown
Author

Daily autonomous review — blocker за /companies + .data е адресиран, тест-ригорът остава проследяван

Благодаря за дълбочинния анализ на реалния D1. Блокерът (MAJOR #1) е фиксиран, тест-ригорът за .data noindex остава отворена координация с lane-а от #177.

1) Blocker — закрит

Commit: 75251ebfix(db): project legal_form on the list path so /companies + .data don't 500

Коренът на проблема: комитът 3cd5d23 perf(db): project legal_form only on the CSV path (2026-08-19) направи проекцията на legal_form в rollup подзаявката условна, под допускането че toCompanyListItem не я чете. Това допускане е вярно за 3cd5d23, но по-късният 3458dae fix(privacy): mask sole-trader rows in leaderboard list mappers (2026-08-22) добави r.legal_form консумация в toCompanyListItem БЕЗ да обнови SQL — COLS включва legal_form, а source() не го проектира. Mock-DB unit suite никога не е изпълнявал SQL и е green-вал грешната форма.

Фиксът:

  • source(p, { legalForm: true }) вече е default-ът (флагът остава за експлицитен intent на call site-овете).
  • listCompanies изрично подава { legalForm: true } за търсимост.
  • Unit тестът, който твърдеше обратното (omits LEFT JOIN bidders in the rollup subquery on the listCompanies path), е обърнат да твърди правилната форма.

TDD:

  1. Добавен е packages/db/src/queries/companies-rollup-sql.test.ts — real-SQLite end-to-end тест през node:sqlite (същият harness като value-base-sql.test.ts), който преди фикса fail-ва с no such column: legal_form на всяка от 5-те асерции, след фикса — 5/5 зелени:
    • Sole trader (legal_form='ЕТ') → eik=null + name=MASKED_NATURAL_PERSON_LABEL + displayName=MASKED_NATURAL_PERSON_LABEL + hasEik=false.
    • Юридическо лице (legal_form='ООД') → eik='200000002' + name='СТРОЙ ООД' + hasEik=true.
    • Консорциум с водещ ЕТ (kind='consortium', legal_form='ДЗЗД') → НЕ over-masked, name='ЕТ Иван Петров; Строй ООД' + eik='300000003' запазени (kind guard).
    • CSV парност: маскираният ЕТ ред има eik='' и name=MASKED_NATURAL_PERSON_LABEL и в двата изхода.
    • Директен replay на production SQL без преминаване през listCompanies — доказва, че колоната се проектира.
  2. Unit тестът в companies.test.ts обърнат и пренаписан: keeps LEFT JOIN bidders + b.legal_form projection in the rollup subquery on the listCompanies path (PASS и преди, и след — ама ПРЕДИ беше грешен заради expect.toBe(true) vs expect.toBe(false)).

Верификация:

  • pnpm typecheck — 7/7 packages clean.
  • pnpm --filter @sigma/db exec vitest run src/queries/220/220 зелени (включително новите 5 интеграционни и обърнатите 16 от companies.test.ts).
  • pnpm --filter @sigma/web exec vitest run552/552 зелени (не са пипани).
  • pnpm --filter @sigma/shared exec vitest run60/60 зелени (не са пипани).
  • pnpm exec prettier --check — clean.

Забележка: full-suite @sigma/db има pre-existing baseline failures от spawnSync sqlite3 ENOENT (нямаме sqlite3 CLI в cron runner-а); идентични са на upstream/main преди моите промени — нищо ново от този фикс. src/queries/ subset-ът, който ползва node:sqlite + mock D1, е изцяло зелен.

2) Test-rigor — остава за lane-а от #177

Бележката за noindex на .data-а, доказан само срещу mock, е точна — app.nofollow.test.ts mock-ва createRequestHandler и подава canned Responses с вече сложен X-Privacy-Mask. Координацията с PR #177 (feat(web): worker route integration-test lane for #94) е правилното място за пълния end-to-end тест: lane-ът вече има appFetch + wrangler.getPlatformProxy() + real D1 през migrations, и може да докаже, че RRv7 7.18 действително извиква headers() export-а върху .data request и че маркерът стига до worker-а.

Какво направих в този commit: добавих real-SQLite интеграционен тест на DB-слоя (покрива маскирането, което е преди да byte-овете стигнат RRv7/worker), но НЕ пипам @sigma/web тестовата инфраструктура — това е скопът на #177. Предлагам: при merge на #177 (който е prerequisite за production-ready privacy PR-и), да се добави apps/web/test/integration/privacy-noindex-data.test.ts от теб по предложения в ревюто скелет (real worker pipeline + decodeSingleFetch за row-по-ред асерции). Ако искаш, мога да я драсна като отделен follow-up PR след #177.

HEAD на PR-а

75251eb (от 7e4773c). mergeable: MERGEABLE, mergeStateStatus: BEHIND — очаквано, защото локалният клон е изпреварил upstream/main с 1 commit (този). Maintainer rebase / merge от ваша страна, когато блокерът и дупката в доказателството се преценят за достатъчно затворени.

…upstream

Resolve additive conflicts in test files where both sides introduced independent
imports (PR midt-bg#183 MASKED_NATURAL_PERSON_LABEL + upstream fakeD1 from @sigma/test-support)
and in docs/README.md (new implementation-plans/287 entry from upstream).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Privacy: .json/.csv expose natural-person ЕИК without the noindex applied to HTML profiles

4 participants