doc: результаты код-ревью Соннета (10 пунктов, приоритеты)

HISTORY/code-review-results.md

@@ -0,0 +1,134 @@
+# Code Review — Соннет (2026-08-08)
+
+## Файлы ревью
+- `server/main.go` — HTTP, S3, роуты
+- `server/gpg_key.go` — GPG-ключи
+- `server/docs.go` — статика из S3
+- `Dockerfile` — сборка
+
+---
+
+## 1. `defer` внутри `if err == nil` — ✅ OK
+
+```go
+if err == nil {
+    defer shasumsObj.Close()  // выполнится при выходе из ФУНКЦИИ, не из if
+}
+```
+Не утечка. `defer` всегда выполняется при выходе из функции. `shasumsObj` открывается один раз на вызов и закрывается при возврате — правильный паттерн.
+
+---
+
+## 2. `readyzHandler` без проверки S3 — ⚠️ надо чинить
+
+```go
+// Проверить доступность S3  ← комментарий, не реализовано
+w.Write([]byte("ok"))
+```
+K8s readiness probe всегда 200 — даже если S3 недоступен. Pod остаётся в rotation с неработающим S3.
+
+**Фикс:** `s3Client.BucketExists(ctx, bucketName)`
+
+---
+
+## 3. Версии не сортируются — ⚠️ надо чинить
+
+`seenVersions` — map, итерация недетерминирована. Terraform может некорректно выбрать последнюю версию.
+
+**Фикс:** `sort.Slice` по семверу.
+
+---
+
+## 4. `proxyHandler` без лимитов — 🔴 КРИТИЧНО
+
+### 4a. Размер/таймаут — некритично для внутреннего сервиса.
+
+### 4b. SSRF / Authorization bypass — КРИТИЧНО
+
+```go
+bucket := r.URL.Query().Get("bucket")  // ← принимает ЛЮБОЙ bucket!
+key    := r.URL.Query().Get("key")
+```
+Кто знает URL сервера — может читать объекты из любого бакета в том же S3.
+
+**Фикс:** захардкодить `bucket = bucketName`, игнорировать параметр `bucket` из запроса. `key` валидировать что начинается с разрешённого префикса.
+
+---
+
+## 5. HTML хардкод — ✅ OK
+
+```go
+fmt.Fprintf(w, `...` + VERSION + `...`)
+```
+Для крошечной страницы приемлемо. `VERSION` сейчас `"1.0.2"` — безопасно.
+
+---
+
+## 6. Graceful Shutdown — ⚠️ желательно
+
+```go
+log.Fatal(http.ListenAndServe(":"+port, nil))
+```
+SIGTERM убивает процесс мгновенно. In-flight download обрывается → Terraform-клиент теряет кэш.
+
+**Фикс:**
+```go
+srv := &http.Server{Addr: ":" + port}
+go srv.ListenAndServe()
+// signal.NotifyContext + srv.Shutdown(ctx)
+```
+
+---
+
+## 7. Индентация в docs.go — ✅ косметика
+
+Смешаны табы и пробелы. Go компилирует нормально, но `gofmt -w docs.go` переформатирует полностью. Если CI проверяет `gofmt -l` — упадёт.
+
+---
+
+## 8. GPG-ключи в коде — ✅ OK
+
+Это **публичные** ключи — их задача быть известными. Смена ключа = rebuild + redeploy — для редкого события приемлемо.
+
+---
+
+## 9. `go mod init` + `go mod tidy` в Dockerfile — 🔴 КРИТИЧНО
+
+```dockerfile
+RUN go mod init tf-registry && go mod tidy && ...
+```
+Каждая сборка качает зависимости из интернета. Если proxy недоступен или версия изменилась — сборка сломается.
+
+**Фикс:** добавить `go.mod` + `go.sum` в репо (`server/go.mod`, `server/go.sum`).
+```dockerfile
+COPY server/go.mod server/go.sum ./
+RUN go mod download
+COPY server/ .
+RUN CGO_ENABLED=0 go build -o registry .
+```
+
+---
+
+## 10. Общая архитектура и безопасность
+
+| Проблема | Серьёзность | Статус |
+|---|---|---|
+| `proxyHandler` принимает любой `bucket` | 🔴 Высокая | SSRF — фикс п.4b |
+| Нет аутентификации | 🟡 Средняя | Внутренний сервис |
+| `key` не валидируется | 🟡 Средняя | `../` MinIO отклонит, но лучше проверять |
+| JSON-ошибки не логируются | 🟢 Низкая | `json.NewEncoder` без проверки ошибок |
+| Нет `go.sum` | 🟡 Средняя | Нерепродуцируемые сборки |
+| `readyz` всегда 200 | 🟡 Средняя | см. п.2 |
+
+---
+
+## Приоритеты к исправлению
+
+| # | Что | Серьёзность |
+|---|---|---|
+| 1 | `proxyHandler` — убрать параметр `bucket` из запроса | 🔴 SSRF |
+| 2 | `go.sum` в репо | 🔴 Воспроизводимость |
+| 3 | `readyz` — реальная проверка S3 | 🟡 |
+| 4 | Сортировка версий | 🟡 |
+| 5 | Graceful shutdown | 🟡 |
+| 6 | `gofmt` docs.go | 🟢 |