fix(privacy): mask bidderSlug on the contract mapper + render masked sole-trader rows as <span> on /contracts + home single-offer - #344
Open
LyuboslavLyubenov wants to merge 50 commits into
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.
For a sole-trader / natural-person row, the shared `toItem()` mapper used by `listContracts`, `contractsSummary`, and the home single-offer tables used to return the bare ЕИК in `bidderSlug` (e.g. `121817309`), because `companySlug(eik:121817309)` returns the digits verbatim. The slug is in the loader payload that RRv7 single-fetch turbo-streams to /contracts.data and to the HTML hydration payload of the public indexable home single-offer tables — so even though masked rows already render with the masking label in those surfaces, 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 page). Add the `masked` boolean to `ContractListItem` (mirrors `CompanyListItem` from PR midt-bg#183 #115163a) so consumers can branch on a single source-of-truth instead of string-comparing `MASKED_NATURAL_PERSON_LABEL`. The mapper sets `masked: isNaturalPerson` on the same branch as the label, and swaps `bidderSlug` for the opaque `m<base64(bidder_id)>` token from `maskedCompanySlug()`. The legal-entity and consortium branches are untouched and still return the round-trippable bare ЕИК. TDD: - 3 new tests in `contracts.test.ts` under the existing `listContracts — privacy masking on the leaderboard list` describe block: - masked sole trader → `masked: true` + opaque slug (non-round-trippable, no bare ЕИК digits, stable per bidder id); - legal entity → `masked: false` + round-trippable bare ЕИК; - consortium (lead sole trader) → `masked: false` + round-trippable bare ЕИК (the `bidder_kind !== consortium` guard keeps the JV verbatim, same invariant as `toCompanyListItem`). Verification: - pnpm --filter @sigma/db exec vitest run src/queries/ → 24 files / 233 tests green. - pnpm --filter @sigma/shared exec vitest run → 4 files / 60 tests green. lyubomir-bozhinov review 2026-09-02, thread on packages/db/src/queries/rows.ts:86 (extended to the contract mapper).
…fer tables
The home single-offer tables (`recentSingleOffer`, `topSingleOffer`) share
the contract `toItem()` mapper with `/contracts`. After the mapper fix in
the previous commit a masked sole-trader row carries `masked: true` and an
opaque `m<base64(bidder_id)>` `bidderSlug` — neither a working href (the
opaque slug is non-resolvable by design) nor a privacy-safe one (the
pre-fix bare ЕИК would have leaked through the JSON payload).
Render the bidder as a non-link `<span>` when `c.masked`, mirroring the
top-10 invariant from #9308672 and the leaderboard invariant from
`companies.tsx:174-178`. The home page stays intentionally indexable
(a single masked row on a page of otherwise-indexable rows must not
deindex the whole homepage); the per-row `<span>` is the privacy guard.
TDD:
- New `home.render.test.tsx` describe block — "home.render — masked
sole-trader rows on the home single-offer tables" — renders a mixed
legal/masked contract list into both single-offer tables and asserts:
- the legal-entity row stays a working <Link> to its profile (one per
table → two total);
- the masked row must NOT be a link in either table (no `a[href^="/companies/m"]`,
no link text „Частно лице");
- the masked label still renders as visible text (the row stays in the
page; it is just not clickable).
Verification:
- 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.
lyubomir-bozhinov review 2026-09-02, thread on
packages/db/src/queries/rows.ts:86 (extended from the home top-10 fix
#9308672 to the contract mapper and its consumers).
…ity with home + leaderboard) The /contracts leaderboard is already noindexed when any row on the page is masked — the loader stamps `X-Privacy-Mask` for the worker `hardenResponse` to translate to `X-Robots-Tag: noindex` on the .data twin (RRv7 single-fetch). But the pre-fix `bidderSlug` was still the bare ЕИК, so the rendered anchor on the HTML hydration payload carried the natural-person ЕИК to any direct visitor (search engines are kept away by the page-level noindex, but the HTML payload is still a leak surface for an attacker snapshotting the page or reading view-source). After the contract mapper fix in the previous commit the slug is the opaque `m<base64(bidder_id)>` token, which is non-resolvable by design — rendering it as a `<Link>` would also 404. Render the bidder as a non-link `<span>` when `c.masked` on /contracts, matching the invariant on home top-10 (#9308672), home single-offer tables, and the /companies leaderboard (`companies.tsx:174-178`). The masked profile is reachable only via direct URL or a noindexed contract-page backlink. Also switch the loaders masked-row detection from `bidderName === MASKED_NATURAL_PERSON_LABEL` to `c.masked` (the boolean the mapper sets in lock-step with the label and the opaque slug inside `toItem`). The flag is the single source-of-truth — not a brittle string compare on the masking label. Verification: - 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. lyubomir-bozhinov review 2026-09-02, thread on packages/db/src/queries/rows.ts:86 (extended to the contract mapper and its consumers).
ydimitrof
suggested changes
Sep 3, 2026
ydimitrof
left a comment
ydimitrof
left a comment
Contributor
There was a problem hiding this comment.
Ревю на PR: маскиране на bidderSlug и рендиране на маскирани редове за еднолични търговци
Какво прави PR-ът
Промяната укрепва privacy-маскирането на физически лица / еднолични търговци през всички повърхности на данните:
- Механизъм за noindex сигнал:
markPrivacyMaskAppliedпоставя вътрешен маркерX-Privacy-Mask: applied, койтоapplyPrivacyMaskHeadersпревежда вX-Robots-Tag: noindexи изтрива, така че маркерът не изтича към клиента или edge кеша. Пазачите за консорциум в loader-а и вmeta()наcompany.tsxсъвпадат; разделянето „HTML остава индексируем /.datatwin получава noindex" за leaderboard-а е последователно. - Мапъри за маскиране: маскиране на едноличните търговци/физическите лица с консорциумен guard (
kind !== 'consortium'), паритет между CSV/list/JSON повърхностите и възстановена SQL проекция наlegal_formвърху rollup клона. Промяната вsource()е безопасна (JOIN по PKbidders.id). - Нова помощна функция:
isNaturalPersonBidderи константаMASKED_NATURAL_PERSON_LABELвpackages/shared/src/format.ts, целящи да бъдат единствен източник на истина за правилата.
Като цяло промяната е чиста, добре обмислена и много добре покрита с тестове. Не е открит злонамерен код, инжекции, изтичане на тайни, нови мрежови/CI стъпки или проблеми с целостта на данните. Коментарите в diff-а реферират минали ревюта (PR #183, ADR-0039) — това е контекст, не инструкции, така че няма prompt injection.
Блокиращо
maskedCompanySlugвidentity.tsреверсивно кодира ЕИК. Кодирането е base64url наbidder_id, който съдържа ЕИК, така че „маскираният" slug реално пренася ЕИК в машинно-четимите отговори (/contracts.data,/companies.data, HTML hydration). Това подкопава именно маскирането на тялото, заради което ADR-0039 сметнаnoindexза недостатъчно. Docstring коментарите („one-way token, no ЕИК") надценяват гаранцията. Това е същностният проблем, който самият PR трябва да затвори, и следва да бъде адресиран преди сливане.
Незадължителни бележки
- Две дребни бележки за поддръжливост/консистентност по механизма за маскиране и noindex (Batch 1).
- Коректност на
isNaturalPersonBidder: широкото съвпадение наINDIVIDUALи защита отnullзаname. - Да се потвърди, че старият
isSingleNaturalPersonProfileе премахнат навсякъде и че няма вградени дубликати на правилата заlegal_formв маршрутите (company.tsx,streamContractsCsv/streamCompaniesCsv,/contracts/:id.json), за да не се разминат правилата за маскиране между повърхностите (недоказано в рамките на прегледаните партиди).
Етикетът „Частно лице" е безопасен за HTML/JSON/CSV (без запетая/кавички).
3 tasks
lyubomir-bozhinov
left a comment
lyubomir-bozhinov
left a comment
Collaborator
There was a problem hiding this comment.
Свързано с #183 review-а: bidderSlug за маскирани редове ползва споделения 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 маскирането тук го проверявам отделно.
added 4 commits
September 4, 2026 13:27
Author
|
Автономен обзор на ревютата — ежедневен cron pass, 2026-09-04 (EEST). Адресирани неща в този pass:
Комити в този branch:
Верификация: Нерезолнати/отворени нишки: няма — и петте thread-а на ydimitrof са резолнати. |
…upstream Conflict resolution: kept both sides — the privacy describe block (servedCsvExport privacy) added by this branch lives alongside the upstream describe blocks (csv-export — remaining branches, putStreamMultipart — abort on failure, servedCsvExport — R2 range shapes). 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 5 review threads remain What I did this pass (rebase only):
Conflict resolution (3 files, all test-side):
No production code touched. No force-push. No Co-Authored-By trailers. 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
For sole-trader / natural-person rows, the shared
toItem()mapper (used bylistContracts,contractsSummary, and the home single-offer tables) maskedbidderName/bidderDisplayName/eikbut leftbidderSlugas the bare ЕИК. The slug was serialised on/contracts.data(RRv7 single-fetch turbo-stream) and on the HTML hydration payload of the public indexable home single-offer tables — so even though masked rows already rendered with the masking label in those surfaces, the ЕИК still leaked through the JSON payload and was reachable by any consumer reading the response body.This is the same leak class as the PR #183 #115163a leaderboard fix (
rows.ts:86): opaquem<base64(bidder_id)>slug frommaskedCompanySlug(), no bare ЕИК digits, non-round-trippable viabidderIdFromSlug.Why a separate PR
The original review #5086672877 named one MAJOR (the leaderboard
slug) and was closed by #115163a + #9308672 (already on PR #183). This PR extends the same invariant to three more consumers of the contract mapper that the review did NOT name but share the same mapper:/contractsleaderboard row (page is already noindexed when ANY row is masked — per-row<span>is the consistent invariant).recentSingleOffer,topSingleOffer) — indexable, public (same severity as the home top-10 fix #9308672).Keeping it scoped to one mapper + its two consumers keeps the reviewable diff small. A follow-on PR (
fix/extend-mask-invariant-to-other-surfaces) covers/flowsand/competitionseparately.Changes
packages/api-contract/src/index.ts—masked: booleanonContractListItem(mirrorsCompanyListItem.masked).packages/db/src/queries/contracts.ts—toItem()setsbidderSlug: isNaturalPerson ? maskedCompanySlug(r.bidder_id) : companySlug(r.bidder_id)andmasked: isNaturalPerson. Samebidder_kind !== "consortium"guard astoCompanyListItem.apps/web/app/routes/home.tsx—SingleOfferTablebranches onc.masked ? <span> : <Link>.apps/web/app/routes/contracts.tsx— list-row bidder cell branches onc.masked ? <span> : <Link>. Loader mask detection switches from string-comparingMASKED_NATURAL_PERSON_LABELtoc.masked(the boolean is the single source-of-truth).TDD
3 new tests in
packages/db/src/queries/contracts.test.tsunder the existinglistContracts — privacy masking on the leaderboard listdescribe block:masked: true+ opaque slug (non-round-trippable, no bare ЕИК, stable per bidder id);masked: false+ round-trippable bare ЕИК;masked: false+ round-trippable bare ЕИК (thebidder_kind !== "consortium"guard preserves the JV verbatim).1 new describe block + test in
apps/web/app/routes/home.render.test.tsx— "home.render — masked sole-trader rows on the home single-offer tables" — renders a mixed legal/masked contract list into both single-offer tables and asserts the legal row is a working<Link>, the masked row is NOT a link, and the masked label still renders as visible text.Verification
pnpm --filter @sigma/db exec vitest run src/queries/→ 24 files / 233 tests green (baseline 230 → +3 new).pnpm --filter @sigma/web exec vitest run→ 49 files / 589 tests green (baseline 588 → +1 new).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 contract mapper and its /contracts + home single-offer consumers).