Skip to content

Безопасность и баги: OIDC-флаги, send.bounced, DMS-инъекции, web/auth харднинг, deep-link карантина - #83

Merged
friench merged 5 commits into
mainfrom
fix/security-and-bugs
Jul 7, 2026
Merged

Безопасность и баги: OIDC-флаги, send.bounced, DMS-инъекции, web/auth харднинг, deep-link карантина#83
friench merged 5 commits into
mainfrom
fix/security-and-bugs

Conversation

@friench

@friench friench commented Jul 7, 2026

Copy link
Copy Markdown
Owner

Первый заход по итогам код-ревью — группа «Безопасность и баги». Пять отдельных коммитов, по одному на issue, каждый с тестами.

Что вошло

fix(env) — #64 (security/bug). z.coerce.boolean() == Boolean(input), поэтому строка "false" давала true: OIDC_AUTO_PROVISION=false фактически включал авто-провижининг, а OIDC_REQUIRE_VERIFIED_EMAIL нельзя было отключить. Оба флага переведены на паттерн enum(['true','false']).transform(...) как у остальных булевых переменных. Регрессионный тест.

fix(webhooks) — #65 (bug). send.bounced диспатчился из BounceService, но отсутствовал в WEBHOOK_EVENTS, поэтому подписка отклонялась и событие было недоставляемым. Добавлен в реестр + в подсказку UI. Плюс EventDispatcher.dispatch типизирован WebhookEvent вместо string — теперь диспатч события без записи в реестре не компилируется. Интеграционный тест на подписку.

fix(validators) — #66 (security). Инъекция управляющих символов в провижининг DMS:

  • alias target имел только min/max — \n инъектировал вторую строку в postfix-virtual.cf (кросс-тенант редирект / DoS). Теперь control-char проверка на сыром значении (до trim/split) + каждый comma-токен обязан быть email / @domain / devnull.
  • bulk-import валидировал лишь длину и звал сервисы напрямую (без ревалидации): foo\nevil@registered.com или ../../etc (path traversal в DKIM-cat + глобальный клин генерации nginx-vhost) проходили. Импорт переиспользует строгие схемы domainName/dkimSelector/aliasAddress/aliasTarget и z.email().
  • заодно ужесточён dkimSelector (общая схема) на create/update домена.

feat(security) — #67 (частично). Харднинг web/auth:

  • COOKIE_SECURE явно управляет флагом Secure сессионного cookie (fallback на NODE_ENV==='production').

  • /metrics fail-closed: без METRICS_TOKEN теперь 404, если явно не задан METRICS_PUBLIC=true.

  • Origin-guard на мутациях /admin/api (второй слой поверх SameSite=lax): отклоняет cross-origin state-changing запросы; пропускает safe-методы, API-key (не подвержен CSRF) и запросы без Origin. TRUSTED_ORIGINS для split-host дашборда.

  • Unit- и интеграционные тесты.

    SSRF DNS-rebinding pin (4-й пункт [security] Харднинг web/auth: COOKIE_SECURE, /metrics, Origin-check, SSRF DNS-pin #67) отложен: корректный фикс требует кастомного undici-dispatcher, а путь уже admin-only + redirect:manual + fail-closed блок-лист. Остаётся на [security] Харднинг web/auth: COOKIE_SECURE, /metrics, Origin-check, SSRF DNS-pin #67.

fix(ui) — #82 (bug, регресс #50). Страница карантина не читала ?mailboxId (deep-link со страницы ящика попадал на неотфильтрованный список). Фильтр переведён на useSearchParams — deep-link работает, вид шареабелен.

Проверки

typecheck · lint · format:check · backend vitest 550 ✓ (+новые) · ui typecheck+build ✓

Closes #64
Closes #65
Closes #66
Closes #82
Refs #67

🤖 Generated with Claude Code

friench and others added 5 commits July 6, 2026 18:47
@friench @claude
fix(env): OIDC boolean flags use enum coercion, not z.coerce.boolean
ca63709 
z.coerce.boolean() is Boolean(input), so the string "false" coerces to
true — OIDC_AUTO_PROVISION=false actually enabled auto-provisioning and
OIDC_REQUIRE_VERIFIED_EMAIL could not be turned off. Switch both to the
enum(['true','false']).transform(v => v === 'true') pattern used by every
other boolean flag in env.ts, and pin the behaviour with a regression test.

Closes #64

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@friench @claude
fix(webhooks): make send.bounced a subscribable event
2ca4e65 
BounceService dispatched 'send.bounced', but it was absent from
WEBHOOK_EVENTS, so the subscription validator (z.enum(WEBHOOK_EVENTS))
rejected it and no webhook could ever match — bounce notifications went
nowhere. Add it to the event registry and surface it in the UI hint.

Also type EventDispatcher.dispatch(event: WebhookEvent, …) instead of
string, so dispatching an event that has no WEBHOOK_EVENTS entry now
fails to compile — preventing this class of drift going forward.

Closes #65

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@friench @claude
fix(validators): reject control chars in alias target; stop bulk-impo…
35f7ed2 
…rt bypass

User input with control chars (CR/LF/…) could reach docker-mailserver
provisioning as an argv element and inject extra config lines:

- Alias target had only min/max validation. A newline let a domain_admin
  inject a second postfix-virtual.cf mapping in another domain (cross-tenant
  mail redirection / map-file DoS), escaping the route's domain scope. Target
  is now control-char-checked on the raw string (before trim/split) and each
  comma token must be an email, @Domain, or devnull.
- Bulk import validated only min-length, then called the services directly
  (which don't re-validate). A mailbox address like 'foo\nevil@registered.com'
  or a domain name like '../../etc' (→ path traversal in the DKIM key cat, and
  a global nginx vhost-regeneration wedge) got through. Import now reuses the
  strict domainName / dkimSelector / aliasAddress / aliasTarget schemas and a
  proper email address.

Also tightens dkimSelector validation (shared schema) on domain create/update,
closing selector traversal on the normal create-domain path too.

Closes #66

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@friench @claude
feat(security): cookie-secure control, fail-closed /metrics, Origin C…
12a32b9 
…SRF guard

Web/auth hardening (partial #67):
- COOKIE_SECURE env explicitly controls the Secure flag on the session cookie
  (falls back to NODE_ENV==='production' when unset), so a proxy-TLS deploy that
  forgets NODE_ENV=production no longer ships the cookie over plaintext.
- /metrics fails closed: without METRICS_TOKEN it now 404s unless METRICS_PUBLIC
  is explicitly set, instead of silently exposing metrics to anyone.
- New Origin guard on /admin/api mutations: rejects a state-changing request
  whose Origin doesn't match the served host, as a second layer over
  SameSite=lax. Skipped for safe methods, API-key auth (not CSRF-able), and
  requests with no Origin (non-browser clients). TRUSTED_ORIGINS allows a
  split-host dashboard.

The SSRF DNS-rebinding pin (4th bullet of #67) is deferred: a correct fix needs
a custom undici dispatcher and the path is already admin-only + redirect:manual
+ fail-closed block-list. Tracked on #67.

Refs #67

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@friench @claude
fix(ui): honor ?mailboxId deep link on the quarantine page
b76931a 
The mailbox settings page links to /admin/quarantine?mailboxId=<id>, but the
quarantine page initialised its filter from useState('') and never read the
query string, so the link landed on the unfiltered view. Drive the filter from
the URL via useSearchParams (and keep the URL in sync when the select changes),
so the deep link works and the filtered view is shareable.

Closes #82

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@friench friench mentioned this pull request Jul 7, 2026
Open
@friench
friench merged commit 7c2f69e into main Jul 7, 2026
1 check passed
@friench
friench deleted the fix/security-and-bugs branch July 7, 2026 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment