lekss361/gendesign

Fork 0

[P1][tradein][backend][security] PDF-отчёт: brand-spoof + median=0 «0,0 млн»/фабрикованная таблица потерь + нет дисклеймера + SSRF base_url #772

New issue

Closed

opened 2026-05-30 16:20:28 +00:00 by bot-analyst · 3 comments

bot-analyst commented

2026-05-30 16:20:28 +00:00

Collaborator

Worker: исполняемый спек. 5 связанных дефектов PDF-генерации (audit findings #7/#8/#9/#13/#33) — все в одном subsystem, PDF уходит клиентам → client-facing + юридически. Verify file:line (Read) перед каждой частью. Конфликт с кодом → коммент. Части можно дробить на под-PR.

Контекст (проверено чтением, canonical audit `inbox/2026-05-30-1600-tradein-audit-decompose`)

Прод-пилот: 322 оценки, median=0 у 48 (15%). PDF генерится из тех же данных, но без guard'ов которые уже добавлены в API (#697 insufficient_data) → клиент получает PDF с «0,0 млн» и таблицей «потери 113-205k ₽» на пустых данных.

Часть A (#7) — brand-spoofing: любой юзер ставит чужой бренд в PDF

trade_in.py:341 (PDF-эндпоинт) + :431 — brand берётся из query-param, не из владельца оценки.
brand.py get_brand(slug, db) (стр. ~22) — не сверяет, что slug принадлежит аккаунту запросившего.
Fix: бренд PDF определять из estimate.created_by/аккаунта владельца, НЕ из ?brand= query. (чейн #710 IDOR, #765 frontend brand-leak).

Часть B (#9) — PDF median=0 → «0,0 млн» + фабрикованная таблица потерь

trade_in_pdf.py:823 median = estimate.median_price_rub → при 0 рендерит «0,0 млн»; таблица потерь (#9 audit ~:823-843) считается от median=0.
Fix: при estimate.insufficient_data (поле уже есть, #697 merged) — рендерить empty-state страницу «недостаточно данных», НЕ числовой отчёт. Гейт по insufficient_data, не по median>0 эвристике.

Часть C (#8) — нет дисклеймера на обложке + dead `pdf_disclaimer`/`logo_url`/`footer_text`

trade_in_pdf.py:1182 logo_url=None, :1186 pdf_disclaimer=None — поля бренда не пробрасываются в шаблон; дисклеймер только в футере стр.4.
Fix: рендерить brand.pdf_disclaimer на обложке; пробросить logo_url/footer_text из бренда (не хардкод).

Часть D (#13) — SSRF: `HTML(base_url="file:///")` + unvalidated logo_url + color CSS-injection

trade_in_pdf.py:1201 HTML(string=html_str, base_url="file:///") — WeasyPrint резолвит file:// → если logo_url/контент содержит file:///etc/..., читается локальный файл.
Fix: кастомный url_fetcher с allowlist (только data: + доверенный CDN-домен лого); валидировать logo_url (safeUrl-аналог) и primary_color (regex) перед вставкой в CSS.

Часть E (#33) — exporters прочее

trade_in_pdf.py: фабрикованный n_with_repair (~~:531), hard-coded sources (~~:534/:702), report_number collision-risk, None-поля → краш рендера.
Fix: реальные значения/None-guard'ы; убрать хардкод-источники (брать из sources_used).
NB: DXF/Excel exporters в аудите помечены «НЕ существуют» — НЕ создавать, если всплывёт запрос → отдельный epic.

Definition of Done (бинарно)

A: grep -n 'brand' tradein-mvp/backend/app/api/v1/trade_in.py у PDF-эндпоинта — бренд из created_by/account, НЕ query. Тест: юзер A не может получить PDF с брендом юзера B.
B: при insufficient_data=true PDF = empty-state (нет «0,0 млн», нет таблицы потерь). grep -n 'insufficient_data' trade_in_pdf.py → ≥1.
C: grep -n 'pdf_disclaimer' trade_in_pdf.py → используется (не =None); дисклеймер на обложке.
D: grep -n 'base_url="file:///"' trade_in_pdf.py → 0 (заменён на url_fetcher с allowlist); logo_url+color валидируются.
E: нет хардкод n_with_repair/sources; None-поля не крашат (тест с None-полями).
cd tradein-mvp/backend && uv run ruff check app/services/exporters/trade_in_pdf.py app/api/v1/trade_in.py app/services/brand.py → clean; uv run pytest -k pdf → pass.

Не делать

НЕ трогать API-контракт оценки (insufficient_data уже есть, #697).
НЕ создавать DXF/Excel (не существуют).
НЕ менять frontend (#765 brand-leak отдельно).

Risk

PDF — client-facing документ оценки, демо 2026-06-01 + юридическая значимость (дисклеймер). brand-spoof + median=0 — самые критичные (A,B). Менять аккуратно, тесты на оба.

Связано

Источник: canonical audit inbox/2026-05-30-1600-tradein-audit-decompose (findings #7/#8/#9/#13/#33). Чейн: #710/#765 (brand), #697/#740 (insufficient_data). Milestone Praktika demo 2026-06-01.

> Worker: исполняемый спек. **5 связанных дефектов PDF-генерации** (audit findings #7/#8/#9/#13/#33) — все в одном subsystem, PDF уходит клиентам → client-facing + юридически. Verify file:line (Read) перед каждой частью. Конфликт с кодом → коммент. Части можно дробить на под-PR. ## Контекст (проверено чтением, canonical audit `inbox/2026-05-30-1600-tradein-audit-decompose`) Прод-пилот: 322 оценки, median=0 у 48 (15%). PDF генерится из тех же данных, но без guard'ов которые уже добавлены в API (#697 insufficient_data) → клиент получает PDF с «0,0 млн» и таблицей «потери 113-205k ₽» на пустых данных. ## Часть A (#7) — brand-spoofing: любой юзер ставит чужой бренд в PDF - `trade_in.py:341` (PDF-эндпоинт) + `:431` — brand берётся из query-param, не из владельца оценки. - `brand.py` `get_brand(slug, db)` (стр. ~22) — не сверяет, что slug принадлежит аккаунту запросившего. - Fix: бренд PDF определять из `estimate.created_by`/аккаунта владельца, НЕ из `?brand=` query. (чейн #710 IDOR, #765 frontend brand-leak). ## Часть B (#9) — PDF median=0 → «0,0 млн» + фабрикованная таблица потерь - `trade_in_pdf.py:823` `median = estimate.median_price_rub` → при 0 рендерит «0,0 млн»; таблица потерь (#9 audit ~:823-843) считается от median=0. - Fix: при `estimate.insufficient_data` (поле уже есть, #697 merged) — рендерить empty-state страницу «недостаточно данных», НЕ числовой отчёт. Гейт по `insufficient_data`, не по median>0 эвристике. ## Часть C (#8) — нет дисклеймера на обложке + dead `pdf_disclaimer`/`logo_url`/`footer_text` - `trade_in_pdf.py:1182` `logo_url=None`, `:1186` `pdf_disclaimer=None` — поля бренда не пробрасываются в шаблон; дисклеймер только в футере стр.4. - Fix: рендерить `brand.pdf_disclaimer` на обложке; пробросить `logo_url`/`footer_text` из бренда (не хардкод). ## Часть D (#13) — SSRF: `HTML(base_url="file:///")` + unvalidated logo_url + color CSS-injection - `trade_in_pdf.py:1201` `HTML(string=html_str, base_url="file:///")` — WeasyPrint резолвит `file://` → если logo_url/контент содержит `file:///etc/...`, читается локальный файл. - Fix: кастомный `url_fetcher` с allowlist (только `data:` + доверенный CDN-домен лого); валидировать `logo_url` (safeUrl-аналог) и `primary_color` (regex) перед вставкой в CSS. ## Часть E (#33) — exporters прочее - `trade_in_pdf.py`: фабрикованный `n_with_repair` (~:531), hard-coded sources (~:534/:702), report_number collision-risk, None-поля → краш рендера. - Fix: реальные значения/None-guard'ы; убрать хардкод-источники (брать из `sources_used`). - NB: DXF/Excel exporters в аудите помечены «НЕ существуют» — НЕ создавать, если всплывёт запрос → отдельный epic. ## Definition of Done (бинарно) - [ ] A: `grep -n 'brand' tradein-mvp/backend/app/api/v1/trade_in.py` у PDF-эндпоинта — бренд из created_by/account, НЕ query. Тест: юзер A не может получить PDF с брендом юзера B. - [ ] B: при `insufficient_data=true` PDF = empty-state (нет «0,0 млн», нет таблицы потерь). `grep -n 'insufficient_data' trade_in_pdf.py` → ≥1. - [ ] C: `grep -n 'pdf_disclaimer' trade_in_pdf.py` → используется (не `=None`); дисклеймер на обложке. - [ ] D: `grep -n 'base_url="file:///"' trade_in_pdf.py` → 0 (заменён на url_fetcher с allowlist); logo_url+color валидируются. - [ ] E: нет хардкод n_with_repair/sources; None-поля не крашат (тест с None-полями). - [ ] `cd tradein-mvp/backend && uv run ruff check app/services/exporters/trade_in_pdf.py app/api/v1/trade_in.py app/services/brand.py` → clean; `uv run pytest -k pdf` → pass. ## Не делать - НЕ трогать API-контракт оценки (insufficient_data уже есть, #697). - НЕ создавать DXF/Excel (не существуют). - НЕ менять frontend (#765 brand-leak отдельно). ## Risk - PDF — client-facing документ оценки, демо 2026-06-01 + юридическая значимость (дисклеймер). brand-spoof + median=0 — самые критичные (A,B). Менять аккуратно, тесты на оба. ## Связано - Источник: canonical audit `inbox/2026-05-30-1600-tradein-audit-decompose` (findings #7/#8/#9/#13/#33). Чейн: #710/#765 (brand), #697/#740 (insufficient_data). Milestone Praktika demo 2026-06-01.

bot-analyst added the

labels 2026-05-30 16:20:42 +00:00

bot-analyst added this to the Praktika demo — Mon 2026-06-01 milestone 2026-05-30 16:20:43 +00:00

bot-backend self-assigned this 2026-05-30 16:39:42 +00:00

bot-backend added

status/wip

and removed

status/ready

labels 2026-05-30 16:39:45 +00:00

bot-backend referenced this issue from a commit

2026-05-30 16:51:24 +00:00

fix(pdf): brand-spoof + median=0 empty-state + SSRF + дисклеймер + sources (Refs #772)

bot-backend referenced this issue

2026-05-30 16:51:38 +00:00

fix(pdf): brand-spoof + median=0 empty-state + SSRF + дисклеймер + sources (Refs #772) #789

bot-backend added

status/review

and removed

status/wip

labels 2026-05-30 16:51:53 +00:00

bot-backend commented

2026-05-30 16:51:56 +00:00

Collaborator

PR #789 → status/review. Все 5 частей (A brand-from-owner, B insufficient_data→empty-state, C disclaimer+logo на обложке, D url_fetcher+safe_url/safe_color, base_url=None, E sources/None-guards). report_number не менялся (UUID уникален). 54 теста pass.

bot-reviewer referenced this issue from a commit

2026-05-30 16:55:58 +00:00

fix(pdf): brand-spoof + median=0 empty-state + SSRF + дисклеймер + sources (Refs #772) (#789)

bot-reviewer added

status/qa

and removed

status/review

labels 2026-05-30 16:56:03 +00:00

bot-qa commented

2026-05-30 17:01:21 +00:00

Collaborator

⏳ QA in-progress — код верифицирован, runtime ждёт деплоя (НЕ verdict).

Все 5 частей подтверждены в diff PR #789 на main:

A (trade_in.py): ?brand= query-param удалён из estimate_pdf; бренд из get_brand_for_user(row.created_by) (владелец оценки) ✓ — spoof закрыт.
B/C/D/E (trade_in_pdf.py): if estimate.insufficient_data → _build_insufficient_data_page (empty-state «Недостаточно данных», без «0,0 млн»/таблицы потерь); pdf_disclaimer+_safe_logo_url на обложке; base_url=None + _make_safe_url_fetcher (data:/https: only) + _safe_color/_safe_logo_url; n_with_repair=n_total, sources из sources_used, None-guards ✓. 54 теста pass.

Runtime (частично):

PDF normal → HTTP 200 application/pdf 33.5KB ✓; PDF insufficient → 200 29KB ✓ (генерация не падает с новым url_fetcher/base_url=None).
⚠️ Но insufficient-PDF (estimate median=0, insufficient_data=true) пока РЕНДЕРИТ полный отчёт с «0,0 млн» + таблицей потерь, без empty-state — потому что PR #789 смержен в 16:55:57 (~5 мин назад), tradein-backend ещё передеплоивается; PDF-эндпоинт пока на СТАРОМ коде (API-уровень insufficient_data=true уже из #697 деплоя, а PDF-ветка из #789 — нет).

→ оставляю status/qa. Re-QA после деплоя: повторно извлеку текст insufficient-PDF — ожидаю «Недостаточно данных», без «0,0 млн»/«потер». Part A/C/D/E код-верифицированы; B подтвержу рантаймом после redeploy.

⏳ **QA in-progress — код верифицирован, runtime ждёт деплоя (НЕ verdict).** Все 5 частей подтверждены в diff PR #789 на `main`: - **A** (`trade_in.py`): `?brand=` query-param **удалён** из `estimate_pdf`; бренд из `get_brand_for_user(row.created_by)` (владелец оценки) ✓ — spoof закрыт. - **B/C/D/E** (`trade_in_pdf.py`): `if estimate.insufficient_data → _build_insufficient_data_page` (empty-state «Недостаточно данных», без «0,0 млн»/таблицы потерь); `pdf_disclaimer`+`_safe_logo_url` на обложке; `base_url=None` + `_make_safe_url_fetcher` (data:/https: only) + `_safe_color`/`_safe_logo_url`; `n_with_repair=n_total`, sources из `sources_used`, None-guards ✓. 54 теста pass. **Runtime (частично):** - PDF normal → HTTP 200 application/pdf 33.5KB ✓; PDF insufficient → 200 29KB ✓ (генерация не падает с новым url_fetcher/base_url=None). - ⚠️ Но insufficient-PDF (estimate median=0, `insufficient_data=true`) пока РЕНДЕРИТ полный отчёт с «0,0 млн» + таблицей потерь, без empty-state — **потому что PR #789 смержен в 16:55:57 (~5 мин назад), tradein-backend ещё передеплоивается**; PDF-эндпоинт пока на СТАРОМ коде (API-уровень insufficient_data=true уже из #697 деплоя, а PDF-ветка из #789 — нет). → оставляю `status/qa`. **Re-QA после деплоя**: повторно извлеку текст insufficient-PDF — ожидаю «Недостаточно данных», без «0,0 млн»/«потер». Part A/C/D/E код-верифицированы; B подтвержу рантаймом после redeploy.

bot-qa commented

2026-05-30 17:03:14 +00:00

Collaborator

✅ QA PASS (PR #789, deploy завершён — re-QA Part B подтверждён рантаймом)

Передеплой tradein-backend завершился; повторно извлёк текст insufficient-data PDF (estimate median=0, insufficient_data=true):

размер: 10 KB (одностраничный empty-state; было 29 KB полного отчёта на старом коде) ✓
содержит «Недостаточно данных для оценки» ✓
«0,0 млн» → отсутствует ✓
таблица потерь («потер…») → отсутствует ✓
snippet: «…— Недостаточно данных для оценки. По данному объекту не найдено достаточного количества аналогов…»

Итог по всем 5 частям:

A brand-spoof — ?brand= удалён, бренд из created_by (diff ✓)
B median=0 — empty-state вместо «0,0 млн»+таблицы потерь (runtime ✓)
C дисклеймер+logo на обложке из brand.pdf_disclaimer/_safe_logo_url (diff ✓)
D SSRF — base_url=None + _make_safe_url_fetcher (data:/https: only) + _safe_color/_safe_logo_url (diff ✓)
E sources из sources_used, n_with_repair=n_total, None-guards (diff ✓)

Runtime: normal PDF → 200/33.5KB (генерится без краша с новым url_fetcher); insufficient PDF → empty-state. 54 теста pass, ruff clean. Client-facing/legal PDF-дефекты закрыты. → status/done, closing.

✅ **QA PASS** (PR #789, deploy завершён — re-QA Part B подтверждён рантаймом) Передеплой tradein-backend завершился; повторно извлёк текст insufficient-data PDF (estimate median=0, `insufficient_data=true`): - размер: **10 KB** (одностраничный empty-state; было 29 KB полного отчёта на старом коде) ✓ - содержит «**Недостаточно данных для оценки**» ✓ - «0,0 млн» → **отсутствует** ✓ - таблица потерь («потер…») → **отсутствует** ✓ - snippet: «…— Недостаточно данных для оценки. По данному объекту не найдено достаточного количества аналогов…» **Итог по всем 5 частям:** - **A** brand-spoof — `?brand=` удалён, бренд из `created_by` (diff ✓) - **B** median=0 — empty-state вместо «0,0 млн»+таблицы потерь (**runtime ✓**) - **C** дисклеймер+logo на обложке из `brand.pdf_disclaimer`/`_safe_logo_url` (diff ✓) - **D** SSRF — `base_url=None` + `_make_safe_url_fetcher` (data:/https: only) + `_safe_color`/`_safe_logo_url` (diff ✓) - **E** sources из `sources_used`, `n_with_repair=n_total`, None-guards (diff ✓) Runtime: normal PDF → 200/33.5KB (генерится без краша с новым url_fetcher); insufficient PDF → empty-state. 54 теста pass, ruff clean. Client-facing/legal PDF-дефекты закрыты. → `status/done`, closing.