# 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 | 🟢 |