From 83c95dff0886fa99d71d2ef8207a6277031cf684 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Balv=C3=ADn?= Date: Sun, 26 Jul 2026 20:40:16 +0200 Subject: [PATCH] fix: apply code-review and insight findings across nuntius --- .gitea/workflows/release.yml | 11 +++++--- .gitea/workflows/test.yml | 30 +++++++++++++++++++++- CONTRIBUTING.md | 4 +-- cmd/server/main.go | 2 +- docs/security.md | 2 +- internal/email/sender.go | 9 +++++-- internal/handler/contact.go | 50 ++++++++++++++++++++++++++++++------ justfile | 4 +++ 8 files changed, 94 insertions(+), 18 deletions(-) diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index db111f3..e6cd667 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -2,7 +2,7 @@ name: Release on: push: - tags: ["v*"] + tags: ["v*.*.*"] jobs: build: @@ -53,9 +53,9 @@ jobs: VERSION_NO_V="${VERSION#v}" echo "version_no_v=${VERSION_NO_V}" >> "$GITEA_OUTPUT" - LDFLAGS="-s -w -X sourcedock.dev/petrbalvin/nuntius/internal/version.Version=${VERSION}" + LDFLAGS="-s -w -X sourcedock.dev/petrbalvin/nuntius/internal/version.Version=${VERSION_NO_V}" mkdir -p bin - GOOS=${{ matrix.goos }} GOARCH=${{ matrix.goarch }} \ + GOOS=${{ matrix.goos }} GOARCH=${{ matrix.goarch }} CGO_ENABLED=0 \ go build -ldflags "$LDFLAGS" \ -o "bin/nuntius-${VERSION_NO_V}-${{ matrix.goos }}-${{ matrix.goarch }}" \ ./cmd/server @@ -67,6 +67,11 @@ jobs: path: bin/nuntius-${{ steps.build.outputs.version_no_v }}-${{ matrix.goos }}-${{ matrix.goarch }} if-no-files-found: error + - name: Smoke test + run: | + chmod +x bin/nuntius-${{ steps.build.outputs.version_no_v }}-${{ matrix.goos }}-${{ matrix.goarch }} + ./bin/nuntius-${{ steps.build.outputs.version_no_v }}-${{ matrix.goos }}-${{ matrix.goarch }} --version + release: runs-on: fedora needs: build diff --git a/.gitea/workflows/test.yml b/.gitea/workflows/test.yml index 3f6181c..17d13e8 100644 --- a/.gitea/workflows/test.yml +++ b/.gitea/workflows/test.yml @@ -22,7 +22,13 @@ jobs: - name: gofmt # `gofmt -l` prints the names of unformatted files. An empty # output is the only acceptable result. - run: gofmt -l cmd internal pkg + run: | + unformatted=$(gofmt -l cmd internal pkg) + if [ -n "$unformatted" ]; then + echo "The following files need gofmt:" + echo "$unformatted" + exit 1 + fi - name: go vet run: go vet ./... @@ -45,3 +51,25 @@ jobs: - name: go test ./examples run: go build ./examples/... + + build: + runs-on: fedora + needs: test + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-go@v5 + with: + go-version: "1.26" + + - name: Download dependencies + run: go mod download + + - name: Build + run: go build -ldflags="-s -w" -o bin/nuntius ./cmd/server + + - name: Test + run: go test ./... + + - name: Smoke test + run: ./bin/nuntius --version diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 58fdf84..8e8fcc1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -231,7 +231,7 @@ Triggered by pushing a tag that matches `v*.*.*` (e.g. `v1.0.0`, `v1.2.3`) onto 1. **`build`** — Runs on a `fedora` runner in a 7-way matrix: `linux/amd64`, `linux/arm64`, `linux/riscv64`, `linux/loong64`, `freebsd/amd64`, `freebsd/arm64`, `freebsd/riscv64`. The default `actions/checkout@v4` checks out the tagged commit (the trigger was a tag push), which is exactly the commit on `main` we want to ship. The version is baked in via `-ldflags`: ```bash - -X sourcedock.dev/petrbalvin/nuntius/internal/version.Version=${VERSION#v} + -X sourcedock.dev/petrbalvin/nuntius/internal/version.Version=${VERSION_NO_V} ``` The resulting binary is uploaded as a workflow artifact and smoke-tested with `--version` on the build host. 2. **`release`** — Runs on a `fedora` runner (no Go toolchain needed). Downloads all artifacts, extracts the matching section from `CHANGELOG.md` for the tagged version, creates a release via the Gitea REST API, and uploads each binary as a release asset. The release is **non-draft, non-prerelease**; if the CHANGELOG section is missing, the job fails fast so a release cannot ship without notes. @@ -248,7 +248,7 @@ Triggered by pushing a tag that matches `v*.*.*` (e.g. `v1.0.0`, `v1.2.3`) onto git push origin main git push origin vX.Y.Z ``` -6. The release workflow builds the four platform binaries and creates the release with the CHANGELOG section as the body. No CI step performs the merge — that already happened in step 4. +6. The release workflow builds the seven platform binaries and creates the release with the CHANGELOG section as the body. No CI step performs the merge — that already happened in step 4. If the release job fails, the tag can be deleted and re-pushed; the workflow re-runs on the new tag push. Do not re-tag an already-released commit — the Gitea API rejects duplicate release tag names. diff --git a/cmd/server/main.go b/cmd/server/main.go index e6158d3..e2777ba 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -48,7 +48,7 @@ func main() { h.Register(mux) srv := &http.Server{ - Addr: ":" + itoa(cfg.Server.Port), + Addr: "[::]:" + itoa(cfg.Server.Port), Handler: withRequestLog(mux, logger), ReadHeaderTimeout: 10 * time.Second, ReadTimeout: 15 * time.Second, diff --git a/docs/security.md b/docs/security.md index ca4e145..b29e837 100644 --- a/docs/security.md +++ b/docs/security.md @@ -139,4 +139,4 @@ The graceful shutdown budget is 15 s (`srv.Shutdown` with `context.WithTimeout`) ## Reporting a Vulnerability -Email **petrbalvin@yandex.com** with a description and a reproducer. Please do not file a public issue for security-sensitive reports. +Email **opensource@petrbalvin.org** with a description and a reproducer. Please do not file a public issue for security-sensitive reports. diff --git a/internal/email/sender.go b/internal/email/sender.go index 675a23d..8bd98ce 100644 --- a/internal/email/sender.go +++ b/internal/email/sender.go @@ -198,14 +198,19 @@ func compose(from, to string, req contactform.Request, formName, formType string case "generic": tmpl = emailTemplateGeneric } - _ = tmpl.Execute(&htmlBuf, map[string]string{ + if err := tmpl.Execute(&htmlBuf, map[string]string{ "FormName": formName, "Name": req.Name, "Email": req.Email, "Service": req.Service, "Message": req.Message, "Received": received, - }) + }); err != nil { + // template.Must guarantees valid templates; this path is + // unreachable in normal operation. + htmlBuf.Reset() + fmt.Fprintf(&htmlBuf, "

Email generation error: %v

", err) + } // --- Subject + plain-text body per form type --- var subject, text string diff --git a/internal/handler/contact.go b/internal/handler/contact.go index 7f611df..f8d42d8 100644 --- a/internal/handler/contact.go +++ b/internal/handler/contact.go @@ -114,10 +114,12 @@ func (h *ContactHandler) makeHandler(path string) http.HandlerFunc { if errs := contactform.Validate(&req, form.Type); len(errs) > 0 { w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(http.StatusBadRequest) - _ = json.NewEncoder(w).Encode(contactform.ErrorResponse{ + if err := json.NewEncoder(w).Encode(contactform.ErrorResponse{ Error: "validation", Details: errs, - }) + }); err != nil { + slog.Error("failed to encode json response", "err", err) + } return } @@ -155,10 +157,12 @@ func (h *ContactHandler) makeHandler(path string) http.HandlerFunc { // Health handles GET /health. func (h *ContactHandler) Health(w http.ResponseWriter, r *http.Request) { w.Header().Set("Content-Type", "application/json; charset=utf-8") - _ = json.NewEncoder(w).Encode(map[string]any{ + if err := json.NewEncoder(w).Encode(map[string]any{ "status": "ok", "forms": len(h.forms), - }) + }); err != nil { + slog.Error("failed to encode health response", "err", err) + } } // --- internals --- @@ -201,10 +205,36 @@ type bucket struct { } func newRateLimiter(perHour int) *rateLimiter { - return &rateLimiter{ + r := &rateLimiter{ perHour: perHour, buckets: make(map[string]*bucket), } + // Clean up entries older than twice the refill window every hour. + r.startCleanup(1*time.Hour, 2*time.Hour) + return r +} + +// startCleanup launches a background goroutine that periodically removes +// expired bucket entries to prevent unbounded memory growth. +func (r *rateLimiter) startCleanup(interval, maxAge time.Duration) { + go func() { + ticker := time.NewTicker(interval) + defer ticker.Stop() + for range ticker.C { + r.cleanup(maxAge) + } + }() +} + +func (r *rateLimiter) cleanup(maxAge time.Duration) { + r.mu.Lock() + defer r.mu.Unlock() + cutoff := time.Now().Add(-maxAge) + for ip, b := range r.buckets { + if b.last.Before(cutoff) { + delete(r.buckets, ip) + } + } } func (r *rateLimiter) allow(ip string) bool { @@ -239,16 +269,20 @@ func minF(a, b float64) float64 { func respondOK(w http.ResponseWriter) { w.Header().Set("Content-Type", "application/json; charset=utf-8") - _ = json.NewEncoder(w).Encode(contactform.Response{OK: true}) + if err := json.NewEncoder(w).Encode(contactform.Response{OK: true}); err != nil { + slog.Error("failed to encode ok response", "err", err) + } } func respondError(w http.ResponseWriter, code int, err, msg string) { w.Header().Set("Content-Type", "application/json; charset=utf-8") w.WriteHeader(code) - _ = json.NewEncoder(w).Encode(contactform.ErrorResponse{ + if encErr := json.NewEncoder(w).Encode(contactform.ErrorResponse{ Error: err, Message: msg, - }) + }); encErr != nil { + slog.Error("failed to encode error response", "err", encErr) + } } func clientIP(r *http.Request) string { diff --git a/justfile b/justfile index d038e64..543e468 100644 --- a/justfile +++ b/justfile @@ -25,3 +25,7 @@ build: test: @echo "→ Running tests…" go test ./... + +# Format Go source files +fmt: + gofmt -w cmd internal pkg