lekss361/gendesign

Fork 0

[P2][tradein][frontend][security] ?brand= override перекрывает account-бренд → cross-brand leak (в prod brand должен идти от аккаунта) #765

New issue

Closed

opened 2026-05-30 16:13:40 +00:00 by bot-analyst · 2 comments

bot-analyst commented

2026-05-30 16:13:40 +00:00

Collaborator

Worker: исполняемый спек. Конфликт с кодом → коммент, НЕ угадывай.

Задача

?brand=<slug> в URL безусловно перекрывает me.brand (привязку бренда к аккаунту) → любой залогиненный пользователь может отрендерить чужой бренд (лого/цвета/футер/pdf-дисклеймер) и сгенерировать brand-skinned PDF чужого бренда. В prod бренд должен определяться аккаунтом; ?brand= — только демо/превью-инструмент. Сделать me.brand приоритетным в prod.

Контекст (проверено чтением)

useActiveBrandSlug() (useBrand.ts:44-50): const urlSlug = getBrandSlugFromUrl(); if (urlSlug) return urlSlug; (стр. 46-47) — URL-param безусловно побеждает; me.brand (стр. 48) используется только если param нет.
getBrandSlugFromUrl (стр. 34-38): читает ?brand= из window.location.search.
Бренд несёт logo_url/primary_color/accent_color/footer_text/pdf_disclaimer (Brand, стр. 23-31) — то есть подмена влияет на визуал отчёта + PDF.
Чейнится с IDOR-историей #690/#710 (brand-skinned PDF чужой оценки).

Контракт (точное толкование)

В useActiveBrandSlug (useBrand.ts:44-50) сделать me.brand приоритетным, а ?brand= разрешить только когда:

(a) у аккаунта нет своего бренда (me.brand пуст) — демо-юзеру можно превьюшить, ИЛИ
(b) только в dev/preview-режиме (если есть process.env.NEXT_PUBLIC_* флаг превью).

Минимальная безопасная версия:

export function useActiveBrandSlug(): string | null {
  const { data: me } = useMe();
  const accountSlug = me?.brand ? me.brand.trim().toLowerCase() : null;
  // Account-бренд приоритетен: ?brand= НЕ может его перекрыть (cross-brand leak).
  // ?brand= разрешён только когда у аккаунта нет своего бренда (демо-превью).
  if (accountSlug) return accountSlug;
  return getBrandSlugFromUrl();
}

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

При me.brand непустом — ?brand=other НЕ меняет активный бренд. grep -n 'getBrandSlugFromUrl' tradein-mvp/frontend/src/lib/useBrand.ts → вызов под условием !accountSlug (после проверки account).
При me.brand пустом — ?brand= по-прежнему работает (демо-превью не сломано).
cd tradein-mvp/frontend && npx tsc --noEmit → 0 ошибок.
cd tradein-mvp/frontend && npx eslint src/lib/useBrand.ts → clean.
(если есть unit-тесты hooks) кейс: me.brand=«praktika» + ?brand=acme → возвращается «praktika».

Не делать (out of scope)

НЕ трогать backend /api/v1/brand/{slug} (отдельно: backend должен reject mismatched slug для аккаунта — заведи follow-up комментом, но НЕ в этом FE-issue).
НЕ менять Brand-интерфейс / fetchBrand / useBrand-query.
НЕ чинить logo_url safeUrl (это finding #24, отдельный issue).

Risk

Демо ГК «Практика» использует brand-by-account (#657) — praktika логинится и видит свой бренд без ?brand=. Эта правка как раз усиливает целевое поведение. Проверить, что демо-флоу (praktika login) не сломан.

Связано

Источник: аудит inbox/2026-05-30-1545-tradein-hard-code-audit-backend-frontend (finding #23). Чейн: IDOR #690/#710 (PDF чужой оценки). Бренд-фича #657. Milestone Praktika demo 2026-06-01.

> Worker: исполняемый спек. Конфликт с кодом → коммент, НЕ угадывай. ## Задача `?brand=<slug>` в URL безусловно перекрывает `me.brand` (привязку бренда к аккаунту) → любой залогиненный пользователь может отрендерить чужой бренд (лого/цвета/футер/pdf-дисклеймер) и сгенерировать brand-skinned PDF чужого бренда. В prod бренд должен определяться аккаунтом; `?brand=` — только демо/превью-инструмент. Сделать `me.brand` приоритетным в prod. ## Контекст (проверено чтением) - `useActiveBrandSlug()` (`useBrand.ts:44-50`): `const urlSlug = getBrandSlugFromUrl(); if (urlSlug) return urlSlug;` (стр. 46-47) — URL-param **безусловно** побеждает; `me.brand` (стр. 48) используется только если param нет. - `getBrandSlugFromUrl` (стр. 34-38): читает `?brand=` из `window.location.search`. - Бренд несёт `logo_url/primary_color/accent_color/footer_text/pdf_disclaimer` (Brand, стр. 23-31) — то есть подмена влияет на визуал отчёта + PDF. - Чейнится с IDOR-историей #690/#710 (brand-skinned PDF чужой оценки). ## Контракт (точное толкование) В `useActiveBrandSlug` (`useBrand.ts:44-50`) сделать `me.brand` приоритетным, а `?brand=` разрешить **только** когда: - (a) у аккаунта нет своего бренда (`me.brand` пуст) — демо-юзеру можно превьюшить, ИЛИ - (b) только в dev/preview-режиме (если есть `process.env.NEXT_PUBLIC_*` флаг превью). Минимальная безопасная версия: ```ts export function useActiveBrandSlug(): string | null { const { data: me } = useMe(); const accountSlug = me?.brand ? me.brand.trim().toLowerCase() : null; // Account-бренд приоритетен: ?brand= НЕ может его перекрыть (cross-brand leak). // ?brand= разрешён только когда у аккаунта нет своего бренда (демо-превью). if (accountSlug) return accountSlug; return getBrandSlugFromUrl(); } ``` ## Definition of Done (бинарно) - [ ] При `me.brand` непустом — `?brand=other` НЕ меняет активный бренд. `grep -n 'getBrandSlugFromUrl' tradein-mvp/frontend/src/lib/useBrand.ts` → вызов под условием `!accountSlug` (после проверки account). - [ ] При `me.brand` пустом — `?brand=` по-прежнему работает (демо-превью не сломано). - [ ] `cd tradein-mvp/frontend && npx tsc --noEmit` → 0 ошибок. - [ ] `cd tradein-mvp/frontend && npx eslint src/lib/useBrand.ts` → clean. - [ ] (если есть unit-тесты hooks) кейс: me.brand=«praktika» + `?brand=acme` → возвращается «praktika». ## Не делать (out of scope) - НЕ трогать backend `/api/v1/brand/{slug}` (отдельно: backend должен reject mismatched slug для аккаунта — заведи follow-up комментом, но НЕ в этом FE-issue). - НЕ менять Brand-интерфейс / fetchBrand / useBrand-query. - НЕ чинить logo_url safeUrl (это finding #24, отдельный issue). ## Risk - Демо ГК «Практика» использует brand-by-account (#657) — praktika логинится и видит свой бренд без `?brand=`. Эта правка как раз усиливает целевое поведение. Проверить, что демо-флоу (praktika login) не сломан. ## Связано - Источник: аудит `inbox/2026-05-30-1545-tradein-hard-code-audit-backend-frontend` (finding #23). Чейн: IDOR #690/#710 (PDF чужой оценки). Бренд-фича #657. Milestone Praktika demo 2026-06-01.

bot-analyst added the

labels 2026-05-30 16:13:50 +00:00

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

bot-analyst referenced this issue

2026-05-30 16:14:41 +00:00

[P3][tradein][frontend][security] brand.logo_url без safeUrl + brand-цвета в CSS без валидации #766

bot-analyst referenced this issue

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

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

bot-frontend self-assigned this 2026-05-30 16:21:03 +00:00

bot-frontend added

status/wip

and removed

status/ready

labels 2026-05-30 16:21:05 +00:00

bot-backend referenced this issue from a commit

2026-05-30 16:23:11 +00:00

fix(tradein): account-бренд приоритетен над ?brand= (cross-brand leak)

bot-frontend referenced this issue

2026-05-30 16:23:21 +00:00

fix(tradein): account-бренд приоритетен над ?brand= — cross-brand leak (#765) #780

bot-frontend added

status/review

and removed

status/wip

labels 2026-05-30 16:23:27 +00:00

bot-frontend commented

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

Collaborator

Working on this in PR #780.

bot-reviewer referenced this issue from a commit

2026-05-30 16:29:13 +00:00

fix(tradein): account-бренд приоритетен над ?brand= — cross-brand leak (#765) (#780)

bot-reviewer added

status/qa

and removed

status/review

labels 2026-05-30 16:29:19 +00:00

bot-qa commented

2026-05-30 16:35:09 +00:00

Collaborator

✅ QA PASS (PR #780 — merged-код на main подтверждён)

useBrand.ts::useActiveBrandSlug:

const accountSlug = me?.brand ? me.brand.trim().toLowerCase() : null;
// Account-бренд приоритетен: ?brand= НЕ может его перекрыть (cross-brand leak).
if (accountSlug) return accountSlug;
return getBrandSlugFromUrl();

me.brand непустой → возвращается account-бренд, ?brand=other его НЕ перекрывает ✓ (cross-brand leak закрыт — нельзя отрендерить/сгенерировать PDF чужого бренда).
getBrandSlugFromUrl() достижим только при !accountSlug (демо-аккаунт без бренда) ✓ — демо-превью не сломано.

Runtime-подтверждение демо-пути: тестовые пилот-аккаунты (user1..10, pilottest) имеют me.brand=null → весь сессионный smoke шёл через ?brand=praktika и рендерил praktika-бренд корректно (демо-флоу жив). Security-путь (account-brand wins) — детерминированно покрыт кодом (if (accountSlug) return); для прод-аккаунта с привязанным брендом ?brand= теперь no-op.

tsc/eslint — зелёные у воркера. → status/done, closing.

NB (как в спеке): backend /api/v1/brand/{slug} всё ещё отдаёт любой запрошенный slug — defense-in-depth follow-up (reject slug ≠ account-brand) стоит завести отдельным backend-issue; этот FE-фикс закрывает основной leak-вектор.

✅ **QA PASS** (PR #780 — merged-код на `main` подтверждён) `useBrand.ts::useActiveBrandSlug`: ```ts const accountSlug = me?.brand ? me.brand.trim().toLowerCase() : null; // Account-бренд приоритетен: ?brand= НЕ может его перекрыть (cross-brand leak). if (accountSlug) return accountSlug; return getBrandSlugFromUrl(); ``` - `me.brand` непустой → возвращается account-бренд, `?brand=other` его **НЕ перекрывает** ✓ (cross-brand leak закрыт — нельзя отрендерить/сгенерировать PDF чужого бренда). - `getBrandSlugFromUrl()` достижим **только** при `!accountSlug` (демо-аккаунт без бренда) ✓ — демо-превью не сломано. Runtime-подтверждение демо-пути: тестовые пилот-аккаунты (`user1..10`, `pilottest`) имеют `me.brand=null` → весь сессионный smoke шёл через `?brand=praktika` и рендерил praktika-бренд корректно (демо-флоу жив). Security-путь (account-brand wins) — детерминированно покрыт кодом (`if (accountSlug) return`); для прод-аккаунта с привязанным брендом `?brand=` теперь no-op. tsc/eslint — зелёные у воркера. → `status/done`, closing. NB (как в спеке): backend `/api/v1/brand/{slug}` всё ещё отдаёт любой запрошенный slug — defense-in-depth follow-up (reject slug ≠ account-brand) стоит завести отдельным backend-issue; этот FE-фикс закрывает основной leak-вектор.