lekss361/gendesign

Fork 0

fix(security): caddy/** в deploy paths + username из 401 попытки #434

Merged

lekss361 merged 1 commit from fix/auth-forwarder-paths-and-username into main

2026-05-23 09:25:41 +00:00

lekss361 commented

2026-05-23 09:19:03 +00:00

Owner

⚠️ Production hotfix — kopylov не работает + missing username в логах

Проблема 1 — BLOCKER

PR #433 (feat(security): добавить юзера kopylov) merged de1652a в 09:07:51 UTC, но curl -u kopylov:<pass> https://gendsgn.ru/ → 401.

Причина: .forgejo/workflows/deploy.yml:10-17 on.push.paths НЕ содержит caddy/**. PR #433 затронул только caddy/users.caddy.snippet → deploy workflow skipped. На VPS старый snippet без kopylov.

Проблема 2

GlitchTip events (issue #73) показывают user_id: "(none)" всегда. User жалуется: «логи не считывают ввод в окно авторизации».

Причина: Caddy по default не пишет username провалившейся basic_auth попытки в user_id поле access log. Доступно через request.headers.Authorization (Caddy пишет все headers в JSON log по default), но в plain Base64.

Fix

1. `.forgejo/workflows/deploy.yml`

Добавил "caddy/**" в двух местах:

on.push.paths — чтобы любое изменение в caddy/ триггерило deploy
jobs.changes.filters.infra — чтобы paths-filter в downstream jobs отметил изменения как infra

2. `ops/glitchtip-auth-forwarder/forwarder.py`

Новая функция _extract_attempted_username(entry):

Парсит Authorization header из Caddy log entry
Base64-decode Basic xxx
Split на : → берёт только username
Password отбрасывается (нигде не логируется, не сохраняется)
Graceful возврат None если: header отсутствует, не Basic, decode fail, нет :, username > 128 chars

emit_event добавляет:

Sentry tag attempted_username (для фильтра)
Поле attempted_username в caddy_log context
В message: (tried: kopylov)

Security

⚠️ НЕ включаем log_credentials true в Caddyfile — это раскрыло бы plain password в access log. Auth header уже там по default, username extract на этапе forwarder (не в Caddy).

Password в Sentry не попадает.

Tests локально

Input	Expected	Result
`Basic <b64(kopylov:pass)>`	`kopylov`	✅
No Authorization header	`None`	✅
`Bearer xxx`	`None`	✅
Invalid base64	`None`	✅

Changes

2 файла, +49 / -4.

После merge

Deploy.yml сам запустится (PR меняет deploy.yml + caddy/**)
Caddy force-recreate → новый snippet с kopylov активен
Forwarder force-recreate → username extraction live
curl -u kopylov:wGLG9RS1uEIBjYCL https://gendsgn.ru/ → 200
Любой failed login с username → в GlitchTip tag attempted_username:<user>

PR #426 (basic_auth gate)
PR #430 (forwarder sidecar)
PR #432 (hotfix permission)
PR #433 (add kopylov — заблокирован этим PR)
GlitchTip issue #71 (permission, fixed), issue #73 (basic_auth_failed)

## ⚠️ Production hotfix — kopylov не работает + missing username в логах ## Проблема 1 — BLOCKER PR #433 (`feat(security): добавить юзера kopylov`) merged `de1652a` в 09:07:51 UTC, но `curl -u kopylov:<pass> https://gendsgn.ru/` → **401**. Причина: `.forgejo/workflows/deploy.yml:10-17` `on.push.paths` НЕ содержит `caddy/**`. PR #433 затронул только `caddy/users.caddy.snippet` → deploy workflow skipped. На VPS старый snippet без `kopylov`. ## Проблема 2 GlitchTip events (issue #73) показывают `user_id: "(none)"` всегда. User жалуется: «логи не считывают ввод в окно авторизации». Причина: Caddy по default не пишет username провалившейся basic_auth попытки в `user_id` поле access log. Доступно через `request.headers.Authorization` (Caddy пишет все headers в JSON log по default), но в plain Base64. ## Fix ### 1. `.forgejo/workflows/deploy.yml` Добавил `"caddy/**"` в двух местах: - `on.push.paths` — чтобы любое изменение в `caddy/` триггерило deploy - `jobs.changes.filters.infra` — чтобы paths-filter в downstream jobs отметил изменения как `infra` ### 2. `ops/glitchtip-auth-forwarder/forwarder.py` Новая функция `_extract_attempted_username(entry)`: - Парсит `Authorization` header из Caddy log entry - Base64-decode `Basic xxx` - Split на `:` → берёт **только username** - **Password отбрасывается** (нигде не логируется, не сохраняется) - Graceful возврат `None` если: header отсутствует, не Basic, decode fail, нет `:`, username > 128 chars `emit_event` добавляет: - Sentry tag `attempted_username` (для фильтра) - Поле `attempted_username` в `caddy_log` context - В message: ` (tried: kopylov)` ### Security ⚠️ НЕ включаем `log_credentials true` в Caddyfile — это раскрыло бы plain password в access log. Auth header уже там по default, username extract на этапе forwarder (не в Caddy). Password в Sentry не попадает. ### Tests локально | Input | Expected | Result | |---|---|---| | `Basic <b64(kopylov:pass)>` | `kopylov` | ✅ | | No Authorization header | `None` | ✅ | | `Bearer xxx` | `None` | ✅ | | Invalid base64 | `None` | ✅ | ## Changes 2 файла, +49 / -4. ## После merge 1. Deploy.yml сам запустится (PR меняет deploy.yml + caddy/**) 2. Caddy force-recreate → новый snippet с kopylov активен 3. Forwarder force-recreate → username extraction live 4. `curl -u kopylov:wGLG9RS1uEIBjYCL https://gendsgn.ru/` → 200 5. Любой failed login с username → в GlitchTip tag `attempted_username:<user>` ## Related - PR #426 (basic_auth gate) - PR #430 (forwarder sidecar) - PR #432 (hotfix permission) - PR #433 (add kopylov — заблокирован этим PR) - GlitchTip issue #71 (permission, fixed), issue #73 (basic_auth_failed)

lekss361 added 1 commit 2026-05-23 09:19:04 +00:00

fix(security): caddy/** в deploy paths + username из 401 попытки b063b7a791

Проблема 1: PR #433 (caddy/users.caddy.snippet) не запустил deploy —
путь caddy/** не был в on.push.paths и changes.infra filter.
Итог: на VPS остался старый snippet без юзера kopylov → 401.

Проблема 2: GlitchTip events показывали user_id="(none)" всегда.
Caddy не пишет username провалившейся basic_auth попытки в user_id.
Решение: парсим Authorization: Basic header, base64-decode,
берём только username (password отбрасывается, не логируется).

lekss361 merged commit 3295808b41 into main

2026-05-23 09:25:41 +00:00

lekss361 referenced this pull request from a commit

2026-05-23 09:25:43 +00:00

fix(security): caddy/** в deploy paths + username из 401 попытки (#434)

lekss361 commented

2026-05-23 09:26:16 +00:00

Author

Owner

Merged via deep-code-reviewer — verdict APPROVE.

Security verification passed:

Password NOT leaked — split(":", 1)[0] discards password element; never returned/logged
log_credentials NOT set in Caddyfile (verified at base SHA)
send_default_pii=False preserved in sentry_sdk.init()
15 edge cases tested locally: Bearer / empty / no-colon / multi-colon / invalid b64 / huge 1MB DoS / case-insensitive header — all return graceful None or expected username
validate=True on b64decode prevents silent garbage acceptance

deploy.yml fix verified:

caddy/** added symmetrically to on.push.paths (line 15) AND jobs.changes.filters.infra (line 50) — downstream conditional builds + caddy force-recreate will trigger
YAML indent correct, no other paths broken
ops/glitchtip-auth-forwarder/** already in paths (line 17) → forwarder.py change alone would have triggered too

Non-blocking follow-ups (separate PR, MEDIUM severity):

_extract_attempted_username should sanitise control chars / ANSI escapes / newlines in username before returning. Suggested: re.match(r"^[A-Za-z0-9._@+-]{1,64}$", username). Current behaviour passes alice\nbob, \x1b[31mEVIL\x1b[0m, null bytes to Sentry tag verbatim — low-impact log-injection risk for downstream notification channels (Slack/email rendering ANSI).
is_monitor_ua (out of scope) still uses startswith → does not match Mozilla/5.0 (compatible; SentryUptimeBot/1.0). Inherited from PR #430, not regressed by this PR.
Recommend vault entry fixes/Fix_Deploy_Paths_Caddy_Snippet.md — third occurrence of "silent deploy skip on new top-level dir" gotcha (data/sql/**, ops/glitchtip-auth-forwarder/**, now caddy/**). Pattern: when PR adds files under a NEW top-level dir, audit deploy.yml on.push.paths coverage.

Acknowledged review miss on PR #433: I (deep-code-reviewer) did not verify deploy.yml on.push.paths covered caddy/** — that was the dir's first appearance. New rule added to my mental checklist: paths-trigger audit when any PR touches a NEW top-level directory.

Post-merge verify (~3 min after deploy completes):

curl -u kopylov:<pass> https://gendsgn.ru/  # expect 200
curl -u kopylov:wrong https://gendsgn.ru/  # expect 401 + GlitchTip tag attempted_username:kopylov

Merged via deep-code-reviewer — verdict APPROVE. **Security verification passed:** - Password NOT leaked — `split(":", 1)[0]` discards password element; never returned/logged - `log_credentials` NOT set in Caddyfile (verified at base SHA) - `send_default_pii=False` preserved in `sentry_sdk.init()` - 15 edge cases tested locally: Bearer / empty / no-colon / multi-colon / invalid b64 / huge 1MB DoS / case-insensitive header — all return graceful `None` or expected username - `validate=True` on `b64decode` prevents silent garbage acceptance **deploy.yml fix verified:** - `caddy/**` added symmetrically to `on.push.paths` (line 15) AND `jobs.changes.filters.infra` (line 50) — downstream conditional builds + caddy force-recreate will trigger - YAML indent correct, no other paths broken - `ops/glitchtip-auth-forwarder/**` already in paths (line 17) → forwarder.py change alone would have triggered too **Non-blocking follow-ups (separate PR, MEDIUM severity):** 1. `_extract_attempted_username` should sanitise control chars / ANSI escapes / newlines in username before returning. Suggested: `re.match(r"^[A-Za-z0-9._@+-]{1,64}$", username)`. Current behaviour passes `alice\nbob`, `\x1b[31mEVIL\x1b[0m`, null bytes to Sentry tag verbatim — low-impact log-injection risk for downstream notification channels (Slack/email rendering ANSI). 2. `is_monitor_ua` (out of scope) still uses `startswith` → does not match `Mozilla/5.0 (compatible; SentryUptimeBot/1.0)`. Inherited from PR #430, not regressed by this PR. 3. Recommend vault entry `fixes/Fix_Deploy_Paths_Caddy_Snippet.md` — third occurrence of "silent deploy skip on new top-level dir" gotcha (`data/sql/**`, `ops/glitchtip-auth-forwarder/**`, now `caddy/**`). Pattern: when PR adds files under a NEW top-level dir, audit `deploy.yml on.push.paths` coverage. **Acknowledged review miss on PR #433:** I (deep-code-reviewer) did not verify `deploy.yml on.push.paths` covered `caddy/**` — that was the dir's first appearance. New rule added to my mental checklist: paths-trigger audit when any PR touches a NEW top-level directory. **Post-merge verify (~3 min after deploy completes):** ``` curl -u kopylov:<pass> https://gendsgn.ru/ # expect 200 curl -u kopylov:wrong https://gendsgn.ru/ # expect 401 + GlitchTip tag attempted_username:kopylov ```

lekss361 referenced this pull request

2026-05-23 09:38:56 +00:00

fix(tradein): security hotfix — C-3 config default + C-4 empty estimate persist + C-6 PDF URL allowlist #435

lekss361 referenced this pull request

2026-05-23 09:52:41 +00:00

fix(security): log_credentials + auth_audit.log → username из 401 #436

lekss361 referenced this pull request

2026-05-23 09:59:40 +00:00

fix(security): log_credentials + auth_audit.log → username из 401 #436

lekss361 referenced this pull request

2026-05-23 11:32:40 +00:00

fix(ui): использовать <Link> вместо <a> для internal routes (build error) #439