Доказва noindex+маскиране по .data twins; поправя 500 на /companies от #183 - #89
Draft
lyubomir-bozhinov wants to merge 2 commits into
Draft
Conversation
…rader masking works (midt-bg#173) toCompanyListItem masks natural persons on r.legal_form (PR midt-bg#183) and COLS selects legal_form, but listCompanies read the unfiltered rollup via source(p) — which omits the bidders LEFT JOIN and the b.legal_form projection. company_totals has no legal_form column, so the query referenced a non-existent column and D1 500'd the whole /companies leaderboard and its /companies.data single-fetch twin. The mock-based unit lane never executed the SQL, so it stayed green; the real-D1 integration lane exposes the 500. Switch listCompanies to source(p, { legalForm: true }) so the rollup path LEFT JOINs bidders by primary key and projects legal_form — the join does not change the row set or the count. Update the source()-projection test to assert the join is present on the list path (masking requires it), reversing its now-invalid perf-optimisation intent. Refs midt-bg#183
…midt-bg#173) Add an integration test (real RRv7 single-fetch handler via wrangler.getPlatformProxy + real D1, PR midt-bg#177's lane) that locks in the previously-unproven claim: RRv7 7.18 runs a route's headers export on the /<path>.data request and carries the loader-set X-Privacy-Mask marker onto the .data HTTP response, so hardenResponse translates it to X-Robots-Tag: noindex and deletes the internal marker. The prior app.nofollow.test.ts hand-injects the marker into a mocked createRequestHandler and could not prove this. Covers /companies.data, /companies/:eik.data, /contracts.data and /contracts/:id.json for a sole trader (masked), a legal entity (verbatim, not noindexed), and a consortium led by a sole trader (over-mask guard). A decodeSingleFetch helper reconstructs the turbo-stream payload so row assertions read real loader data. seedRows lets a test seed extra rows into the lane's isolated per-file D1. Refs midt-bg#183
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
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.
Надгражда lane-а от midt-bg#177 и маскирането от midt-bg#183. Затваря дупката в строгостта около
noindexпо/<path>.datatwin-а: досега тя се доказваше само срещу mock-натcreateRequestHandler(app.nofollow.test.tsръчно инжектира маркера). Тук се доказваend-to-end през реалния RRv7 single-fetch handler.
Ground truth (емпирично)
Реален worker + D1 — и през integration lane-а (
wrangler.getPlatformProxy), и презwrangler dev+curl(prod build). Двете съвпадат, без разминаване по.dataheader-ите:X-Robots-Tag/companies.data(има маскиран ЕТ ред)noindexname="Частно лице",eik=null/companies/<ет>.datanoindexeik=null, търговското име се пази (ADR-0039)/companies.data?count=21-100(само ЮЛ)/contracts.data(ЕТ изпълнител)noindexbidderName="Частно лице"/contracts/<id>.json(ЕТ)noindexbidder_legal_form,eik=null, име маскираноX-Privacy-Maskмаркерът е изтрит от всеки клиентски отговор. RRv7 7.18 изпълняваheadersexport-а върху.dataзаявката и пренася loader-set маркера — такаhardenResponseго превежда вX-Robots-Tag: noindex.Открит и поправен Major (отделен от noindex въпроса)
toCompanyListItemмаскира поr.legal_form(midt-bg#183), аCOLSселектираlegal_form— ноlistCompaniesчетеше нефилтрирания rollup презsource(p), който пропуска JOIN-а къмbiddersи проекцията наb.legal_form.company_totalsняма колонаlegal_form, затовазаявката сочи несъществуваща колона и D1 връща 500 за целия
/companiesleaderboard инеговия
.datatwin. Mock-натият unit lane никога не изпълнява SQL-а, затова е зелен;реалният D1 integration lane показва 500-цата.
Fix:
source(p, { legalForm: true })— LEFT JOIN по primary key, не променя row set-а илиброя. Тестът за проекцията на
source()е обърнат да твърди, че JOIN-ът е налице по listпътя (маскирането го изисква).
Тест
apps/web/test/integration/privacy-noindex-data.test.ts— 6-те реда от таблицата + маркерabsence, всеки sensitive (доказано: изключване на маскирането ⇒ 4-те позитивни падат;
премахване само на
headersforward-а ⇒ пада точно.datanoindex assert-а). Пълен suite:1195 passed, 0 failed.
Отговор в един ред
.datanoindex propagation-ът вече работеше коректно; ground-truth тестването обачеизкара наяве отделен Major —
legal_form-ът от midt-bg#183 сваляше целия/companiesleaderboard на реален D1 (невидимо за mock-натите unit тестове). Поправено.
Refs midt-bg#173, midt-bg#183
CI бележка —
checkе червен по НАСЛЕДЕНА причина (не от този diff)test,cacbg,semgrep,coverage-commentса зелени.checkпада на coverage-ratchet-а:apps/web/coverage/coverage-summary.jsonлипсва. Причина: midt-bg#177 насочиtestскрипта къмvitest.workspace.ts(multi-project), а coverage конфигът (sharedCoverage, вкл.json-summaryreporter-а) стои само на unit проекта — а vitest 4 чете coverage от root-а на multi-project run.
Затова
pnpm test -- --coverageне издава summary за apps/web и ratchet-ът гърми. Наследено —пада идентично и на base branch-а
integration-base-177-183; този PR не пипа coverage wiring-а.Отделен, по-голям проблем който изплува при хойстване на конфига: реалното покритие на apps/web е
~82.8% (unit) / ~67.8% (workspace) срещу baseline 91. Baseline-ът е застоял (ratchet-ът за apps/web
е бил неизпълняван, откакто midt-bg#177 спря summary-то). Правилното решение НЕ е сваляне на прага — а
(1) поправка на coverage emission-а на workspace runner-а и (2) добавяне на тестове до 91, или
преразглеждане на baseline-а от owner. И двете са midt-bg#177-line / coverage-debt, извън обхвата на този
noindex proof PR — оставено за решение.