fix(relay): pin the rate-limit source to the proxy-supplied address (#43)

User-Visible: no
Issue: #43

scripts/support-relay/README.md

@@ -36,6 +36,13 @@
 сегодняшней даты, ключ живёт сутки. Штатный логгер `BaseHTTPRequestHandler`
 заменён — он печатал адрес клиента.
 
+Из `X-Forwarded-For` берётся **последний** элемент, а не первый: первый прислал
+клиент, и подделать его может кто угодно, а последний проставлен ближайшим
+звеном — нашим же Caddy. Caddy при этом настроен перезаписывать заголовок
+целиком (`header_up X-Forwarded-For {remote_host}`). Две меры вместо одной
+потому, что цена ошибки здесь — обход частотного лимита сменой одной строки в
+запросе, и полагаться на умолчания чужого конфига для этого нельзя.
+
 Сообщение и контакт нормализуются и очищаются от управляющих символов, включая
 маркеры двунаправленного письма: доставка выводит их буквальным текстом без
 разметки, поэтому подделать вид сообщения нельзя.
@@ -93,6 +100,11 @@ sudo systemctl enable --now hp-support-relay-purge@prod.timer hp-support-relay-p
 недоступным доменом уронит сервис при рестарте, тогда как reload оставит
 работать прежний.
 
+## Ответственный
+
+За рубильник, оповещения и ротацию секрета отвечает Sergey Matyunin
+(владелец проекта).
+
 ## Runbook
 
 **Проверить состояние**
@@ -139,10 +151,11 @@ sudo -u hprelay cat /var/lib/hp-support-relay/prod/reports/2026-09/hpr-…/repor
 cd scripts/support-relay && python3 -m unittest discover -s tests -q
 ```
 
-Тридцать проверок: схема, размеры, хеш, идемпотентность, частота, ретеншн,
-буквальность текста, отсутствие адреса в журналах, поведение рубильника.
-Каждая проверялась отрицательным прогоном — десять мутаций рабочего кода
+Тридцать одна проверка: схема, размеры, хеш, идемпотентность, частота,
+ретеншн, буквальность текста, отсутствие адреса в журналах, невозможность
+выбрать себе корзину лимита подделкой заголовка, поведение рубильника.
+Каждая проверялась отрицательным прогоном — одиннадцать мутаций рабочего кода
 (снять сверку хеша, разрешить лишнюю часть, не чистить управляющие символы,
 снять лимит, писать адрес в журнал, игнорировать идемпотентность, отключить
-рубильник, отключить ретеншн, не проверять секции пакета) роняют ровно те
-проверки, ради которых написаны.
+рубильник, отключить ретеншн, не проверять секции пакета, брать первый элемент
+`X-Forwarded-For`) роняют ровно те проверки, ради которых написаны.

scripts/support-relay/deploy/Caddyfile.fragment

@@ -8,12 +8,18 @@ support.houseplan.tech {
 	request_body {
 		max_size 8.7MB
 	}
-	reverse_proxy 127.0.0.1:8130
+	reverse_proxy 127.0.0.1:8130 {
+		# Заголовок ПЕРЕЗАПИСЫВАЕТСЯ, а не дополняется: иначе клиент задаёт
+		# первый элемент сам и выбирает себе корзину частотного лимита.
+		header_up X-Forwarded-For {remote_host}
+	}
 }
 
 support-staging.houseplan.tech {
 	request_body {
 		max_size 8.7MB
 	}
-	reverse_proxy 127.0.0.1:8131
+	reverse_proxy 127.0.0.1:8131 {
+		header_up X-Forwarded-For {remote_host}
+	}
 }

scripts/support-relay/hp_relay/app.py

@@ -145,7 +145,13 @@ def make_handler(service: Service):
             if service.cfg.trusted_proxy:
                 forwarded = self.headers.get("X-Forwarded-For", "")
                 if forwarded:
-                    return forwarded.split(",")[0].strip()
+                    # ПОСЛЕДНИЙ элемент, а не первый. Первый — тот, что прислал
+                    # клиент, и подделать его может кто угодно; последний
+                    # проставлен ближайшим звеном, то есть нашим же Caddy.
+                    # Caddy настроен перезаписывать заголовок целиком, но код не
+                    # обязан полагаться на чужой конфиг: цена ошибки здесь —
+                    # обход частотного лимита сменой одной строки в запросе.
+                    return forwarded.split(",")[-1].strip()
             return self.client_address[0]
 
         def do_GET(self) -> None:  # noqa: N802 - имя задано базовым классом

scripts/support-relay/tests/test_relay.py

@@ -377,6 +377,24 @@ class HttpSurfaceTestCase(unittest.TestCase):
         self.assertNotIn("127.0.0.1", written)          # адрес соединения
         self.assertNotIn("198.51.100.5", written)       # адрес из заголовка
 
+    def test_client_cannot_pick_its_own_rate_bucket(self):
+        """Подделанный X-Forwarded-For не создаёт новый ключ источника.
+
+        Каждый запрос приходит с чужим адресом в заголовке; ключей частоты после
+        этого должно остаться столько же, сколько при одном источнике, иначе
+        лимит обходится сменой одной строки в запросе.
+        """
+        blob = package_bytes()
+        for index, forged in enumerate(("203.0.113.1", "198.51.100.2", "192.0.2.3")):
+            content_type, body = build_body(request_json(blob, key=f"forged-key-{index:04d}"), blob)
+            status, _ = self.call("POST", "/v1/reports", body, content_type,
+                                  headers={"X-Forwarded-For": f"{forged}, 127.0.0.1"})
+            self.assertEqual(status, 200)
+        spool = self.service.cfg.spool
+        buckets = [path.name for path in (spool / "rate").glob("*.json")
+                   if path.name != "_global.json"]
+        self.assertEqual(len(buckets), 1, buckets)
+
     def test_forwarded_for_is_used_as_the_source(self):
         blob = package_bytes()
         content_type, body = build_body(request_json(blob), blob)