Skip to content

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

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

fix(privacy): apply noindex+mask policy to machine-readable outputs (#173)#183
LyuboslavLyubenov wants to merge 7 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.

data?.company &&
(isSingleNaturalPersonProfile(data.company.kind, data.company.legalForm) ||
isNaturalPersonProfileName(data.company.displayName) ||
(isNaturalPersonBidder(data.company.displayName, data.company.legalForm) ||

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.

Тук HTML-ът получава noindex за физическо лице, но /companies/:eik.data (RRv7 single-fetch при ssr:true) няма еквивалент: loader-ът връща немаскиран eik, а .data отговорът минава през hardenResponse (само baseSecurityHeaders, без X-Robots-Tag) и се кешира на edge. Значи немаскиран ЕИК на физ. лице, кеширан, без noindex — точно експозицията от #173. Покрий .data.

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

Copy link
Copy Markdown

Прегледах 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 and others added 7 commits July 6, 2026 15:33
…-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.
…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.
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.
@LyuboslavLyubenov LyuboslavLyubenov force-pushed the main branch 2 times, most recently from cdc68e5 to 468a116 Compare July 6, 2026 12:40
@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

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, чиято цел е именно защита на лични данни — тя трябва да бъде адресирана или изрично обоснована преди одобрение. Останалите забележки са некритични.

// `X-Robots-Tag: noindex` by `hardenResponse` in `apps/web/workers/app.ts`. Legal-entity
// records keep the plain-object return (no marker, no mutation).
if (isNaturalPersonBidder(company.displayName, company.legalForm)) {
company.eik = null;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Поверителност / консистентност: Тук се изчиства само company.eik, но company.displayName (пълното име на физическото лице, напр. „ЕТ ДРИФТ - НИКОЛАЙ КИРОВ") се връща буквално в машинно-четимия близнак /companies/:eik.data.

Това е несъответствие спрямо /contracts/:id.json и CSV експортите, които заменят името с MASKED_NATURAL_PERSON_LABEL. Тъй като .data е машинно-четим изход, а самата обосновка на PR-а (ADR-0007, контекст т.1) гласи, че X-Robots-Tag: noindex е недостатъчен срещу ботове, които го игнорират — именно затова е въведено маскирането — оставянето на суровото име тук, защитено единствено с noindex, отваря дупката, която PR-ът твърди, че затваря.

Разбирам техническото напрежение: .data близнакът споделя loader-а с HTML отговора, а HTML умишлено показва името. Но тогава е нужно явно решение — или да се маскира името и в .data (без да се пипа HTML), или изрично да се документира и обоснове защо .data, за разлика от JSON/CSV, остава немаскиран по име. Тестът company.data.test.ts (поведение 1) в момента утвърждава разкриването на името като очаквано.

...record.bidder,
eik: null,
name: MASKED_NATURAL_PERSON_LABEL,
displayName: MASKED_NATURAL_PERSON_LABEL,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Поверителност (ниско): Разпръскването на ...record.bidder запазва bidder.slug при маскиране. Ако slug-овете се извеждат от името на субекта (често срещано), URL-фрагментът може да разкрие идентичността на физическото лице въпреки маскирането на name/displayName/eik. ADR-0007 приема запазването на slug като неутрален URL фрагмент, но не адресира случая на slug, изведен от име. Струва си да се потвърди, че slug-овете за ЕТ/физически лица не съдържат лични имена.

'Content-Length': String(range?.length ?? obj.size),
});
if (range) headers.set('Content-Range', `bytes ${range.start}-${range.end}/${obj.size}`);
markPrivacyMaskApplied(headers);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Дублиран/излишен код: Това извикване на markPrivacyMaskApplied(headers) (както и аналогичното на ред ~102 в клона за 304) е излишно — отговорът веднага се подава на markCsvCache, който сам извиква markPrivacyMaskApplied върху финалните хедъри. Маркерът се задава двойно. Предлагам да се разчита единствено на markCsvCache, за да се спази правилото „NO DEAD CODE / NO CODE DUPLICATION".

if (!needsBase(p)) return { from: 'company_totals', params: [] };
if (!needsBase(p))
return {
from: `(SELECT ct.*, b.legal_form AS legal_form FROM company_totals AS ct LEFT JOIN bidders AS b ON b.id = ct.bidder_id)`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Клонът по подразбиране (без филтри) вече винаги прави LEFT JOIN bidders само за да вземе legal_form. Този source() се използва и от listCompanies (страницирано JSON листване, hot path), не само от CSV експорта — така rollup таблицата company_totals вече не е самодостатъчна за листването. Моля потвърдете: (1) че има индекс по bidders.id (PK), за да остане join-ът lookup; (2) че company_totals НЯМА собствена колона legal_form — иначе ct.* + b.legal_form AS legal_form произвежда двусмислена/дублирана колона legal_form, която външният SELECT ... legal_form може да разреши неочаквано.

let block = '';
for (const r of results) {
const bidderName = cleanName(r.bidder_name);
const isNatural = isNaturalPersonBidder(bidderName, r.bidder_legal_form);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

isNaturalPersonBidder се извиква без проверка на r.bidder_kind, докато докстрингът на самата функция гласи, че „викащият отговаря за филтриране на консорциумите“. Тук филтриране няма: консорциум, чието bidder_name започва с „ЕТ “ (напр. водещ член ЕТ: „ЕТ Иван Петров; Строй ООД“) или чиято legal_form съвпада, ще бъде маскиран като MASKED_NATURAL_PERSON_LABEL и contractor_eik изчистен, губейки името на водещия член и ЕИК. Това е privacy-safe (over-masking, не изтичане), но е поведенческа промяна спрямо стария entityName(..., 'consortium') изход „… и др.“. Обмислете ранен if (r.bidder_kind === 'consortium') → без маскиране, за да съответства на договора на предиката.


/**
* Canonical natural-person / sole-trader predicate for machine-readable masking. Combines the
* `legal_form` rules (carried inline in `apps/web/app/routes/company.tsx` until the route migrates)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Докстрингът признава, че правилата за legal_form са „carried inline in apps/web/app/routes/company.tsx until the route migrates“ — т.е. има дублиран вариант на същата логика. Това нарушава правилото „NO CODE DUPLICATION“ от CLAUDE.md. Моля добавете проследяващ issue/TODO с референция или мигрирайте company.tsx към този споделен предикат, за да не се разминат двете реализации (риск от divergence в решението за noindex/маскиране).

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