fix(privacy): mask sole-trader / natural-person pairs on /flows and /competition - #345
Open
LyuboslavLyubenov wants to merge 49 commits into
Open
Conversation
…-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.
…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.
…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).
…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.
…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.
… 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.
`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.
…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.
…ty changes After rebasing midt-bg#183 onto upstream/main, the masking test fixtures needed two adaptations to upstream's new APIs (no behaviour change to the production masking logic): - Add `getDb` to the `@sigma/db` mocks in the three loader tests. Upstream's read-only D1 chokepoint (midt-bg#199/midt-bg#225) means loaders now call `getContract(getDb(env), …)` instead of `getContract(env.DB, …)`; the mock passes the env's DB through so the stubbed `getContract` still resolves. - Add the new required `orderingUnit: null` (canonical-identity midt-bg#251) and `amendments: []` (annex history midt-bg#165) fields to the `ContractParty` / `ContractRecord` test builders so they satisfy the widened types. All masking assertions unchanged. `pnpm --filter @sigma/web test` → 424 passing; `pnpm --filter @sigma/db test` → 297 passing; typecheck exit 0.
… and resolve conflicts Conflict resolution notes: - docs/adr/README.md: upstream introduced new ADRs (0007-scope-and-certainty-bar, 0008-deterministic-name-to-eik-resolution, ... up to 0032). The PR's 0007-privacy-masking and 0008-centralized-x-robots-tag-worker are renumbered to 0033 and 0034 (the next two free slots), and cross-references in architecture.md, privacy-masking.md, and the ADR files themselves are updated accordingly. File renames via git mv preserve blame. - apps/web/app/routes/contract.json.tsx: upstream refactored to use the shared serializeJsonForScript helper (lib/json-ld.ts) and added X-Content-Type-Options: nosniff. The PR's maskContractForPrivacy function and its consortium guard are preserved; the X-Privacy-Mask marker is replaced with a direct X-Robots-Tag: noindex header because the upstream refactor of the worker (apps/web/workers/app.ts isNoindexNamesPath) no longer translates the marker. The direct header keeps the privacy guarantee for the masked record. - apps/web/app/routes/contract.tsx: import block conflict only; both isNaturalPersonBidder (PR) and isNaturalPersonProfileName (upstream meta noindex) are kept. The PR's loader masking is preserved; the worker's noindex path is now path-based so the contract page noindex must be either added to isNoindexNamesPath or set on the route itself. This commit keeps the route-level masking only; a follow-up may want to align with the worker's path-based noindex policy. - contract.json.test.ts: tests that asserted X-Privacy-Mask: applied / X-Robots-Tag: null are updated to assert the new direct X-Robots-Tag: noindex header (the marker mechanism was removed upstream). The negative cases (legal entity, consortium, not_found) keep asserting X-Robots-Tag: null. The behavior assertion is the same: a masked response gets noindex, a passthrough does not. Verified: pnpm typecheck, pnpm --filter @sigma/web test (429 passing). Refs midt-bg#183, fixes the merge conflict with the post-2026-08-04 upstream work (related-persons, undici bump, cacbg fix).
…fter rebase The merge onto current upstream surfaced three pre-existing issues that need to be addressed for the test suite and lint to pass: - apps/web/app/routes/contract.json.test.ts and contract.data.test.ts: add cohort: null to the makeRecord() fixture. Upstream's ContractRecord type now requires ContractCohortBenchmark | null (the 'Подобни договори' benchmark from the new cohort-band feature in PR midt-bg#210), and the fixtures predated it. - apps/web/app/lib/csv-export.test.ts, packages/db/src/queries/companies.ts, packages/db/src/queries/contracts.ts: prettier format. These three files were reformatted by the upstream prettier version (3.8.3 vs whatever the original PR ran on) — same content, just whitespace. The lint gate is blocking on these, so format fixes are non-optional. Verification: pnpm typecheck (7/7 packages clean), pnpm --filter @sigma/web test (532 passing), pnpm --filter @sigma/shared test (60 passing), pnpm lint (prettier --check clean).
… and resolve conflicts
…urrent upstream
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 #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.
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
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 #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 #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.
…n't 500 PR midt-bg#183 review (lyubomir-bozhinov, 2026-08-24, MAJOR #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).
…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).
…aderboard list (PR midt-bg#183 review)
… masked rows (PR midt-bg#183 review)
… rows
For a masked sole-trader / natural-person row, the list mapper used to return
the bare ЕИК in `slug` (e.g. `'121817309'`), because `companySlug('eik:121817309')`
returns the digits verbatim. The slug is serialised on `/companies.data` (RRv7
single-fetch turbo-stream, machine-readable twin of the leaderboard) and on the
HTML hydration payload of the public indexable leaderboard — so even though masked
rows render as a non-link `<span>` in companies.tsx already, the ЕИК still leaks
through the JSON payload and is reachable by any consumer reading the response
body (curl, a search-engine scraper, an attacker snapshotting the leaderboard).
`getKey` on companies.tsx:249 uses the slug as the React row key, so the slug
must remain unique per row, but it no longer needs to round-trip to a bidder_id
for masked rows — those are not linkable from the public leaderboard by design.
Add `maskedCompanySlug(bidderId)` in identity.ts — a one-way `m<base64url(bidder_id)>`
token that is stable across rebuilds (depends only on the bidder id), does NOT
contain the ЕИК or the raw name, and does NOT round-trip via `bidderIdFromSlug`
(the `m` prefix is not handled by the decoder — masked slugs return null). The
`m` prefix keeps opaque tokens separate from `n` (name-keyed, round-trippable)
and bare ЕИК digits. Wire it through `toCompanyListItem` so the masked branch of
the mapper produces the opaque form; the legal-entity and consortium branches
are untouched and still return the round-trippable `companySlug`.
TDD:
- New test in rows.test.ts asserts masked rows produce a slug that does NOT
decode via bidderIdFromSlug and does NOT contain the bare ЕИК digits, while
legal-entity and consortium rows keep the bare ЕИК (round-trippable).
- New describe in identity.test.ts covers maskedCompanySlug directly:
prefix-is-`m`, non-round-trippable, no ЕИК digits in the output, stable and
unique per bidder id, and handles name-keyed ids the same way (no key/name
leaks in the slug).
- companies-rollup-sql.test.ts updated to find the masked list row by its
masking signal (`i.masked && i.eik === null`) — the slug is opaque now and
cannot be grepped by ЕИК digits.
lyubomir-bozhinov review 2026-09-02, thread on packages/db/src/queries/rows.ts:86.
Verification:
- pnpm --filter @sigma/db exec vitest run src/queries/ → 24 files / 230 tests green.
- pnpm --filter @sigma/shared exec vitest run → 4 files / 60 tests green.
- pnpm --filter @sigma/web exec vitest run → 49 files / 588 tests green.
- pnpm exec prettier --check (touched files) → clean.
The home top-10 (`/topCompanies`) is the public indexable summary page that
mirrors the leaderboard. Before this fix, masked sole-trader rows rendered as
`<Link to={`/companies/${c.slug}`}>` with `c.slug` being the bare ЕИК —
tying the masked name to its identifier on the most public surface of the site
(both via the clickable href and the inline "ЕИК …" / "непотвърден ЕИК"
subtitle). The previous fix to companies.tsx (ydimitrof review 2026-08-31) only
covered the /companies leaderboard; home.tsx kept the same leak.
After the sibling commit that makes masked slugs opaque (`m<base64url(bidder_id)>`,
non-round-trippable), this leak turned into a 404 — clicking the masked row
would land on a URL that does not resolve. So the home top-10 must mirror the
leaderboard's branch: render masked rows as a non-link `<span>`, drop the
"ЕИК …" / "непотвърден ЕИК" subtitle (the masked row has no valid ЕИК to
display, and the "непотвърден ЕИК" fallback is wrong for masked sole traders
— they are neither unconfirmed nor legal), and drop the OwnershipChip.
Masked rows are reachable via direct URL or via the noindexed contract-page
backlink — never via a clickable href on the public homepage. The summary is
indexable, so this is the same CWE-359 leak that PR midt-bg#183 / ADR-0040 close
everywhere else.
TDD (apps/web/app/routes/home.render.test.tsx — new, jsdom + createRoutesStub,
mirrors the conflicts.render.test.tsx pattern):
1. Masked top-10 row renders as <span>, not as <a href="/companies/<slug>"> —
fails on the pre-fix `<Link>` and on the post-fix opaque-slug form.
2. Subtitle for a masked row omits both "непотвърден ЕИК" and the bare ЕИК —
fails on the pre-fix "ЕИК ${c.eik}" / "непотвърден ЕИК" ternary.
3. `headers()` returns the constant Cache-Control and never stamps the
privacy-mask marker on the HTML home page — the home is public and
indexable, a single masked row is too narrow a signal to noindex the
whole homepage.
lyubomir-bozhinov review 2026-09-02, thread on packages/db/src/queries/rows.ts:86
(the "summary" in the original comment).
Verification:
- pnpm --filter @sigma/web exec vitest run → 49 files / 588 tests green
(including the new home.render.test.tsx — 3 tests).
- pnpm --filter @sigma/db exec vitest run src/queries/ → 24 files / 230 tests green.
- pnpm --filter @sigma/shared exec vitest run → 4 files / 60 tests green.
- pnpm exec prettier --check (touched files) → clean.
Note: pnpm typecheck reports one pre-existing error in
apps/web/app/routes/companies.render.test.tsx:119 that predates this PR
(verified by `git stash`-ing the patch and re-running `tsc -b --force`);
unrelated to the privacy fix and out of scope.
The flows Sankey + top-pairs table share the masked-bidder invariant with
the leaderboard (rows.ts:86), the contract mapper, and the home
single-offer tables. Pre-fix, the flows mapper passed through
bidder_name + companySlug(r.bidder_id) verbatim — so a masked
sole-trader pair on /flows serialised the bare ЕИК in:
- the top-pairs table href (the public indexable HTML page);
- the /flows.data RRv7 single-fetch turbo-stream (machine-readable twin);
- the Sankey right-column node href (buildSankey built
href: `/companies/${companySlug(id)}` per bidder);
- the ribbon title text (AuthorityName → BidderDisplayName: €…).
JOIN against bidders to recover bidder_legal_form (the flow_pairs
rollup is denormalised — it does not store it). The PK index on
bidders(id) keeps the JOIN cheap; the (authority_id, bidder_id) PK on
the rollup still drives the won-order scan. Wire the
bidder_kind !== consortium && isNaturalPersonBidder(...) guard into
both the pairs mapper (table) and buildSankey (Sankey nodes +
ribbons). Masked pairs read „Частно лице" everywhere, get an opaque
m<base64(bidder_id)> non-round-trippable bidderSlug, and the Sankey
right-column href is dropped (the masked profile is reachable only via
direct URL or a noindexed contract-page backlink). The loader stamps
X-Privacy-Mask when ANY pair on the page is masked, and the route
headers() forwards it to the worker hardenResponse for the
X-Robots-Tag: noindex translation on the .data twin.
TDD:
- masked: boolean added to FlowPair (mirrors ContractListItem.masked).
- 5 new tests in flows.test.ts under a new describe block:
- sole trader → masked: true + label „Частно лице" + opaque slug;
- legal entity → masked: false + round-trippable bare ЕИК;
- consortium (lead sole trader) → masked: false + verbatim name
(the bidder_kind !== consortium guard preserves the JV);
- Sankey right-column node has NO href when masked;
- Sankey right-column node keeps its /companies/... href for
legal entities.
Verification:
- pnpm --filter @sigma/db exec vitest run src/queries/ → 24 files / 240 tests green.
- pnpm --filter @sigma/web exec vitest run → 49 files / 589 tests green.
- pnpm prettier --check (touched files) → clean.
- pnpm --filter @sigma/web exec tsc --noEmit → clean.
- pnpm --filter @sigma/api-contract exec tsc --noEmit → clean.
lyubomir-bozhinov review 2026-09-02, thread on
packages/db/src/queries/rows.ts:86 (extended from the company mapper to
the flows mapper + its Sankey + table consumer).
The competition recurring-pairs table shares the masked-bidder
invariant with the leaderboard (rows.ts:86), the contract mapper,
flows, and the home single-offer tables. Pre-fix, the competition
mapper passed through bidder_name + companySlug(r.bidder_id)
verbatim — so a masked sole-trader pair on /competition serialised
the bare ЕИК in:
- the recurring-pairs table href (the public indexable HTML page);
- the /competition.data RRv7 single-fetch turbo-stream
(machine-readable twin).
JOIN against bidders to recover bidder_legal_form (the flow_pairs
rollup is denormalised — it does not store it). The PK index on
bidders(id) keeps the JOIN cheap; the (authority_id, bidder_id) PK on
the rollup still drives the contract-count scan. Wire the
bidder_kind !== consortium && isNaturalPersonBidder(...) guard into
the topRecurringPairs mapper. Masked pairs read „Частно лице" with an
opaque m<base64(bidder_id)> non-round-trippable bidderSlug. The loader
stamps X-Privacy-Mask when ANY pair on the page is masked, and the
route headers() forwards it to the worker hardenResponse for the
X-Robots-Tag: noindex translation on the .data twin. Render masked
pairs as <span>, not <Link>, mirroring the leaderboard / home / flows
/ contracts invariant.
TDD:
- masked: boolean added to CompetitionPair (mirrors ContractListItem.masked).
- 2 new tests in competition.test.ts under a new describe block:
- sole trader → masked: true + label „Частно лице" + opaque slug
(no round-trip, no bare ЕИК);
- legal entity → masked: false + round-trippable bare ЕИК + verbatim
name.
- pairRow fixtures in flows.test.ts and competition.test.ts extended
with bidder_legal_form so the JOIN result shape matches.
Verification:
- pnpm --filter @sigma/db exec vitest run src/queries/ → 24 files / 240 tests green.
- pnpm --filter @sigma/web exec vitest run → 49 files / 589 tests green.
- pnpm prettier --check (touched files) → clean.
- pnpm --filter @sigma/web exec tsc --noEmit → clean.
- pnpm --filter @sigma/api-contract exec tsc --noEmit → clean.
lyubomir-bozhinov review 2026-09-02, thread on
packages/db/src/queries/rows.ts:86 (extended from the company mapper to
the competition mapper + its recurring-pairs consumer).
ydimitrof
suggested changes
Sep 3, 2026
ydimitrof
left a comment
ydimitrof
left a comment
Contributor
There was a problem hiding this comment.
Механизмът с вътрешния маркер X-Privacy-Mask → X-Robots-Tag: noindex е чист, добре документиран и много добре покрит с тестове (единично маркиране на CSV пътищата, идемпотентност на applyPrivacyMaskHeaders, guard-ове за консорциуми и prose-консорциуми). Няма следи от зловреден код, нови мрежови извиквания или изтичане на тайни.
Едно съществено притеснение за защита на данните обаче остава на страницата /companies: маскираните редове коректно не рендерират линк, но идентификаторът, който маскирането трябва да скрие (ЕИК), продължава да пътува в payload-а. Виж инлайн коментара. Останалото (company.tsx, competition.tsx, security.ts, csv-export.ts) изглежда коректно — company/competition правилно noindex-ват целия отговор, а /competition допълнително ползва opaque slug, така че там теч няма.
Дребна бележка: за профила на едноличен търговец displayName остава дословно (напр. „ЕТ ДРИФТ - НИКОЛАЙ КИРОВ", съдържащо име на физическо лице) и се рендерира на HTML страницата. Това е документирано решение по ADR-0039 и страницата е noindex, затова не блокирам — но си струва да е ясно, че името на физическото лице остава публично видимо, за разлика от .json/CSV, които го маскират.
3 tasks
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
Collaborator
There was a problem hiding this comment.
Свързано с #183 review-а: slug-овете на /flows + /competition за маскирани редове ползва споделения maskedCompanySlug ('m' + fnv1a64Hex(bidderId), identity.ts). FNV-1a е unsalted бърз hash на публично изброим ЕИК (спечелилите ЕИК = собствения OCDS dataset), тъй че slug-ът е precompute-recoverable — ЕИК-ът се обръща с reverse-lookup таблица. Пълният finding + дефинитивният fix (keyed HMAC, или не-ЕИК-деривиран ключ понеже маскираните редове са non-navigable) са на #183. Поправка в identity.ts затваря и тази повърхност. Per-surface маскирането тук го проверявам отделно.
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
Collaborator
There was a problem hiding this comment.
MINOR (doc-drift на споделения DTO) — packages/api-contract/src/index.ts:625: CompetitionPair.bidderName носи коментар „Cleaned raw name, kept for parity … and a future CSV export", но мапърът вече го маскира (bidderName: maskedBidderName). Тук коментарът е жив footgun: изрично сочи към „бъдещ CSV export" — ако такъв export се довери на „raw name" и сериализира bidderName без пре-маскиране, това е leak на име на физическо лице. Оправи коментара да отразява, че полето е вече маскирано.
added 4 commits
September 4, 2026 13:14
…toCompanyListItem
… post-masking invariant
Author
|
Автономен обзор на ревютата — ежедневен cron pass, 2026-09-04 (EEST). Адресирани неща в този pass:
Комити в този branch:
Верификация: |
…upstream Conflict resolution: kept both sides — the privacy describe blocks (servedCsvExport privacy, getFlows — privacy masking, getCompetition — privacy masking) added by this branch live alongside the upstream describe blocks (csv-export — remaining branches, getFlows — funding scope and label truncation, authority-detail wrappers, getCompetitionSummary). All test imports merged: MASKED_NATURAL_PERSON_LABEL + FakeD1 type + upstream contracts helpers (contractsSummary, listSingleOfferContracts, getCompanyFacets, normalizeCompanySort).
Author
|
Daily autonomous review (2026-09-05) — rebase to keep mergeable. State of action items from prior review. All 1 review thread remains What I did this pass (rebase only):
Conflict resolution (5 files, all test-side):
No production code touched. No force-push. No Co-Authored-By trailers. No edits to the privacy files outside the merge. Verification (local, on the post-merge tip
State:
|
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Two of the public analytics surfaces — the Sankey flow diagram + top-pairs table on /flows, and the recurring-pairs leaderboard on /competition — used mappers that passed through
companySlug(r.bidder_id)+entityName(cleanName(bidder_name), r.bidder_kind)verbatim. A masked sole-trader / natural-person pair on these pages serialised the bare ЕИК in:<Link to={/companies/${p.bidderSlug}}>) on the public indexable HTML page;.dataRRv7 single-fetch turbo-stream (machine-readable twin);buildSankeybuilthref:/companies/${companySlug(id)}`` per bidder) and the ribbon title text.This is the same leak class as PR #183 #115163a (leaderboard) and PR #344 (contract mapper + home single-offer). One invariant across every public surface that carries a bidder href.
Why a separate PR
The flows + competition mappers share an additional SQL concern the contract mapper does not: the
flow_pairsrollup table is denormalised and does NOT carrybidder_legal_form. To recover the natural-person signal, both branches now JOIN againstbiddersonb.id = fp.bidder_id/b.id = c.bidder_id. The PK index onbidders(id)keeps the JOIN cheap; the(authority_id, bidder_id)PK on the rollup still drives the won/contracts-order scan.Grouping /flows + /competition in one PR because they share the same DTO field addition (
masked: booleanonFlowPairandCompetitionPair) and the sameX-Privacy-Maskplumbing pattern (headers()forward + loader stamp ondata.pairs.some((p) => p.masked)).Changes
packages/api-contract/src/index.ts—masked: booleanonFlowPairandCompetitionPair(mirrorsCompanyListItem.masked/ContractListItem.masked).packages/db/src/queries/flows.ts:topPairs(both unfiltered and filtered branches) SELECTsb.legal_form AS bidder_legal_formfrom a JOIN againstbidders.bidder_kind !== "consortium" && isNaturalPersonBidder(...)guard.buildSankeyaggregates the masked label across a bidder's incoming flows (a sole trader that recurs across authorities must read "Частно лице" on the Sankey bar, not its real name). Masked bidders get NOhrefon the Sankey node (opaque slug is non-resolvable).packages/db/src/queries/competition.ts: same JOIN + same masking pattern intopRecurringPairs.apps/web/app/routes/flows.tsx:headers()forwardsX-Privacy-Maskfrom loader (mirrorscontracts.tsx:42-57).X-Privacy-Maskwhendata.pairs.some((p) => p.masked).p.masked ? <span> : <Link>in the top-pairs table.apps/web/app/routes/competition.tsx: same three plumbing changes for the recurring-pairs cell.TDD
packages/db/src/queries/flows.test.tsunder a new describe block "getFlows — privacy masking on the flows Sankey + top-pairs table":masked: true;masked: false;masked: false+ verbatim name (the consortium guard);hrefwhen masked;/companies/...hreffor legal entities.packages/db/src/queries/competition.test.tsunder a new describe block "getCompetition — privacy masking on the recurring-pairs table":masked: true;masked: false.pairRowfixtures inflows.test.tsandcompetition.test.tsextended withbidder_legal_formso the JOIN result shape matches.Verification
pnpm --filter @sigma/db exec vitest run src/queries/→ 24 files / 240 tests green (baseline 230 → +10 new: 5 flows + 2 competition + 3 from PR fix(privacy): mask bidderSlug on the contract mapper + render masked sole-trader rows as <span> on /contracts + home single-offer #344 on the contract mapper).pnpm --filter @sigma/web exec vitest run→ 49 files / 588 tests green (this PR touches no web-side tests).pnpm prettier --check(touched files) → clean.pnpm --filter @sigma/web exec tsc --noEmit→ clean.pnpm --filter @sigma/api-contract exec tsc --noEmit→ clean.lyubomir-bozhinov review 2026-09-02, thread on packages/db/src/queries/rows.ts:86 (extended from the company mapper to the flows + competition mappers and their public consumers).