Skip to content

Commit 1ebc033

Browse files
iamclaude697claude
andauthored
chore(cmd/proxy): decompose main() for testability + close delta-coverage gap (#121)
## Summary Decomposes `cmd/proxy/main.go` `func main()` into separately-testable startup helpers and adds a helper-process `TestMain` that exercises `main()` end-to-end. Closes the structural delta-coverage gap surfaced by #118 — every function in `cmd/proxy/` now meets the 95% per-function gate, so future PRs touching this file (including #120's `--generate-ca` flag addition) land without false coverage failures. ## Motivation - #118 closed a silent-skip bug in `.github/scripts/delta-coverage.sh`. Before that fix, PRs touching `cmd/proxy/main.go` quietly bypassed the per-function gate. - #120 is the first PR to actually hit the now-loud gate. `main()` has always been 0% covered (network setup + signal handling), so any change to the file fails 95%. - Rather than per-PR workarounds, this PR fixes the root cause on `main`: refactor `main()` into testable helpers + helper-process tests that exercise the real binary. ## What changed - New `cmd/proxy/startup.go` — `proxyHTTPServer`, `startManagementAPI`, `runManagementAPI`, `runHTTPServer`, `closeProxyServer` (extracted from `main()`). `closeProxyServer` takes `io.Closer` so the error-log branch is exercisable with a fake. - New `cmd/proxy/shutdown.go` — `installShutdownHandler` (extracted from `main()`). - New unit tests in `cmd/proxy/startup_test.go` and `cmd/proxy/shutdown_test.go` covering each helper at 100%. `TestCloseProxyServer_*` capture the default logger's output and assert the error message is/isn't emitted, not just that both branches execute. - `cmd/proxy/main_test.go` — `TestMain` dispatcher + four helper-process tests (lifecycle, zero-packs guard, proxy-port conflict, mgmt-port conflict) using the canonical Go helper-process pattern (`GO_WANT_HELPER_PROCESS=1` + re-exec of `os.Args[0]`). `TestPrintBanner_ZeroValueConfig_DoesNotPanic` preserves the zero-value-config guarantee previously owned by `TestMain_Smoke`. - `cmd/proxy/main.go` `func main()` reduced to orchestration; no behaviour change. ## Per-function coverage (`go tool cover -func`) ``` cmd/proxy/main.go:39: main 100.0% cmd/proxy/main.go:66: printBanner 100.0% cmd/proxy/shutdown.go:13: installShutdownHandler 100.0% cmd/proxy/startup.go:17: proxyHTTPServer 100.0% cmd/proxy/startup.go:28: startManagementAPI 100.0% cmd/proxy/startup.go:37: runManagementAPI 100.0% cmd/proxy/startup.go:45: runHTTPServer 100.0% cmd/proxy/startup.go:54: closeProxyServer 100.0% total: 100.0% ``` Local `delta-coverage.sh coverage.out 95.0 origin/main` exits 0: ``` === Delta Coverage Check (threshold: 95.0%) === Changed source files: cmd/proxy/main.go cmd/proxy/shutdown.go cmd/proxy/startup.go Checked 8 functions in 3 changed files. SUCCESS: All functions in changed files meet 95.0% coverage threshold. ``` ## Quality gates - [x] `go test -race ./...` green (all packages) — see `TestTokenFormatNonRetriggering` evidence below - [x] `bash .github/scripts/delta-coverage.sh coverage.out 95.0 origin/main` exits 0 with `SUCCESS` - [x] Coverage minimums hold: `internal/anonymizer` 95.6%, `internal/config` 100%, `internal/anonymizer/packs` 97.6%; overall well above 85% - [x] Lifecycle helper-process test stable across 5 consecutive local runs - [x] No `t.Skip()`, no `// coverage-ignore`, no hardcoded test-satisfying values. Four `//nolint:gosec` and two `// #nosec` directives carry substantive reason comments per the existing repo convention. - [ ] `make lint` / `make check` — `golangci-lint` in the local sandbox is built with go1.25 and refuses go1.26.3 modules (same failure on a clean `main` checkout). CI runs all four sub-gates against a fresher toolchain. ## Gate #2 — `TestTokenFormatNonRetriggering` for every PII type Both variants live in `internal/anonymizer/anonymizer_test.go` and ran green at head `c69911f`: ``` === RUN TestTokenFormatNonRetriggering [ANONYMIZER] loaded 41 patterns from 7 enabled packs: [SECRETS GLOBAL DE FR NL FINANCE_EU HEALTHCARE] --- PASS: TestTokenFormatNonRetriggering (0.04s) === RUN TestTokenFormatNonRetriggeringAllPacks [ANONYMIZER] loaded 47 patterns from 8 enabled packs: [SECRETS GLOBAL DE FR US NL FINANCE_EU HEALTHCARE] --- PASS: TestTokenFormatNonRetriggeringAllPacks (0.04s) ``` Each variant iterates over a static list of `PIIType` values and, for each, asserts the generated `[PII_*_<16hex>]` token does not match any loaded pack pattern. The PII types covered: | Pack | PII types asserted by the test | |---|---| | Generic / built-in | `PIIEmail`, `PIIPhone`, `PIISSN`, `PIICreditCard`, `PIIIPAddress`, `PIIAPIKey`, `PIIName`, `PIIAddress`, `PIIMedical`, `PIISalary`, `PIICompany`, `PIIJobTitle` | | DE | `PIISteuerID`, `PIISVNR`, `PIIKFZ` | | SECRETS | `PIISSHKey`, `PIIJWT`, `PIIBearer`, `PIIDBConn`, `PIIAWSKey`, `PIIGHToken` | | FR | `PIINIR`, `PIISIRET`, `PIISIREN` | | NL | `PIIBSN`, `PIIKVK` | | FINANCE_EU | `PIIIBAN`, `PIISWIFTBIC`, `PIIVATID` | | HEALTHCARE | `PIIMRN`, `PIIICD10`, `PIIInsuranceID` | That's 32 PII types in the default-packs variant. `TestTokenFormatNonRetriggeringAllPacks` adds the US pack and drops the four AI-only types (`PIIMedical/Salary/Company/JobTitle`), covering 28 types under the full pattern set (`PIIKFZ`/`PIIDBConn` are noted in the test as known low-confidence retriggers against the broad US phone pattern and routed through the AI gate — documented in the test, not silenced). The two variants together cross every PII type declared in `internal/anonymizer/anonymizer.go`. None of the changed files in this PR touch the anonymizer, the pack files, or the PII type list, so no per-type evidence shifted between baseline and head — but the gate is exercised end-to-end at head as shown above. ## §6 Test Inventory — baseline vs head Keyed by **pack** per `docs/test-plans/ai-proxy-test-method.md` §6. Captured by running `go test -count=1 -json ./internal/anonymizer ./internal/anonymizer/packs` on `origin/main` (`98511d7`) and on PR head (`c69911f`), then attributing each top-level Test event to its pack via the file list in §6. | Pack | Files (per §6) | Baseline PASS | Baseline FAIL | Head PASS | Head FAIL | Delta | |---|---|---:|---:|---:|---:|---:| | GLOBAL | `packs/global_test.go` | 6 | 0 | 6 | 0 | 0 | | DE | `packs/de_test.go` | 5 | 0 | 5 | 0 | 0 | | US | `packs/us_test.go` | 10 | 0 | 10 | 0 | 0 | | FR | `packs/fr_test.go` | 9 | 0 | 9 | 0 | 0 | | SECRETS | `packs/secrets_test.go` + `secrets_report_test.go` + `secrets_priority_report_test.go` | 31 | 0 | 31 | 0 | 0 | | NL | `packs/nl_test.go` + `nl_report_test.go` | 6 | 0 | 6 | 0 | 0 | | FINANCE_EU | `packs/finance_eu_test.go` + `finance_eu_report_test.go` | 7 | 0 | 7 | 0 | 0 | | HEALTHCARE | `packs/healthcare_test.go` + `healthcare_report_test.go` | 6 | 0 | 6 | 0 | 0 | | Cross-pack | all `*_report_test.go` | 9 | 0 | 9 | 0 | 0 | Counts are top-level `func Test*` events from the JSON stream, filtered by the test-name list extracted from each pack's `_test.go` file(s). The `secrets_priority_report_test.go` file lives alongside `secrets_report_test.go`, was added after §6 was written, and is included under SECRETS (its prefix matches the SECRETS pack convention). The Cross-pack row overlaps with the pack rows by design (it sums all `*_report_test.go` content, which the relevant per-pack rows already include). **Supplementary Go-package view** — `go test -count=1 -json ./...` gives 861 → 871 PASS / 0 → 0 FAIL across all 10 Go packages; the `cmd/proxy` row went 4 → 14, every other row unchanged. The per-pack table above is the gate-required view. ## §6 changed-files attribution None of the changed files in this PR (`cmd/proxy/main.go`, `cmd/proxy/startup.go`, `cmd/proxy/shutdown.go`, plus the corresponding `_test.go` files) live under any pack's file list, so the identical per-pack counts are the expected measured outcome, not a substitution for measurement. ## Test plan - [x] `go test -race ./cmd/proxy/...` — all green including new helper-process tests - [x] `go test -race -run '^TestTokenFormatNonRetriggering' ./internal/anonymizer/...` — both variants PASS at head - [x] `go tool cover -func` — every function in `cmd/proxy/` at 100% - [x] Local `delta-coverage.sh` simulation against `origin/main` — exits 0 - [x] Lifecycle test run 5x consecutively — 5/5 pass - [x] CI green (Lint, Test, Security Scan, Benchmark, CodeQL × 2, Build) at `c69911f` ## Out of scope - Any changes under `packaging/linux/` — Fedora `ca-certificates` and openSUSE trust-store path belong to #120 follow-up commits after rebase. - Any changes to `internal/*`, `.github/workflows/`, or `.github/scripts/delta-coverage.sh`. - Adding a second binary or the `--generate-ca` flag (#120's territory). --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 98511d7 commit 1ebc033

6 files changed

Lines changed: 516 additions & 56 deletions

File tree

cmd/proxy/main.go

Lines changed: 6 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,8 @@
2323
package main
2424

2525
import (
26-
"context"
2726
"fmt"
2827
"log"
29-
"net/http"
3028
"os"
3129
"os/signal"
3230
"syscall"
@@ -41,62 +39,28 @@ import (
4139
func main() {
4240
cfg := config.Load()
4341

44-
// Startup guard: zero enabled packs is a fatal misconfiguration.
4542
if len(cfg.EnabledPacks) == 0 {
4643
log.Fatalf("[PROXY] Fatal: no PII detection packs enabled. Configure enabledPacks in proxy-config.json or set ENABLED_PACKS env var.")
4744
}
4845

4946
printBanner(cfg)
5047

51-
// Build the management domain registry so both servers share the same state.
52-
// Runtime domain changes are persisted to ai-domains.json and restored on restart.
5348
registry := management.NewDomainRegistry(cfg, "ai-domains.json")
54-
55-
// Shared metrics collector — passed to both servers so counters are unified.
5649
m := metrics.New()
5750

58-
// Start management API in background.
59-
// Fatal is intentional: the proxy should not run without its control plane.
60-
mgmt := management.New(cfg, registry, m)
61-
go func() {
62-
if err := mgmt.ListenAndServe(); err != nil {
63-
log.Fatalf("[MANAGEMENT] Fatal: %v", err)
64-
}
65-
}()
51+
_ = startManagementAPI(cfg, registry, m)
6652

67-
// Start proxy server
6853
proxyServer := proxy.New(cfg, registry, m)
69-
defer func() {
70-
if err := proxyServer.Close(); err != nil {
71-
log.Printf("[PROXY] Cache close error: %v", err)
72-
}
73-
}()
74-
75-
addr := fmt.Sprintf("%s:%d", cfg.BindAddress, cfg.ProxyPort)
76-
log.Printf("[PROXY] Listening on %s", addr)
54+
defer closeProxyServer(proxyServer)
7755

78-
srv := &http.Server{
79-
Addr: addr,
80-
Handler: proxyServer,
81-
ReadHeaderTimeout: 10 * time.Second,
82-
}
56+
srv := proxyHTTPServer(cfg, proxyServer)
57+
log.Printf("[PROXY] Listening on %s", srv.Addr)
8358

84-
// Graceful shutdown on SIGINT / SIGTERM
8559
quit := make(chan os.Signal, 1)
8660
signal.Notify(quit, syscall.SIGINT, syscall.SIGTERM)
87-
go func() {
88-
<-quit
89-
log.Printf("[PROXY] Shutting down…")
90-
ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
91-
defer cancel()
92-
if err := srv.Shutdown(ctx); err != nil {
93-
log.Printf("[PROXY] Shutdown error: %v", err)
94-
}
95-
}()
61+
go installShutdownHandler(quit, srv, 15*time.Second)
9662

97-
if err := srv.ListenAndServe(); err != nil && err != http.ErrServerClosed {
98-
log.Fatalf("[PROXY] Fatal: %v", err)
99-
}
63+
runHTTPServer(srv)
10064
}
10165

10266
func printBanner(cfg *config.Config) {

cmd/proxy/main_test.go

Lines changed: 194 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,33 @@
11
package main
22

33
import (
4+
"bytes"
45
"fmt"
56
"io"
7+
"net"
68
"os"
9+
"os/exec"
10+
"path/filepath"
711
"strings"
12+
"sync"
13+
"syscall"
814
"testing"
15+
"time"
916

1017
"ai-anonymizing-proxy/internal/config"
1118
)
1219

20+
// TestMain dispatches to main() when GO_WANT_HELPER_PROCESS=1, allowing
21+
// helper-process tests to re-exec this test binary as the production binary.
22+
// Pattern copied from stdlib os/exec internal tests.
23+
func TestMain(m *testing.M) {
24+
if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" {
25+
main()
26+
os.Exit(0)
27+
}
28+
os.Exit(m.Run())
29+
}
30+
1331
// captureStdout redirects os.Stdout to a pipe for the duration of fn,
1432
// then returns everything written to it.
1533
func captureStdout(t *testing.T, fn func()) string {
@@ -76,21 +94,183 @@ func TestPrintBanner_NoProxy_ShowsDirect(t *testing.T) {
7694
}
7795
}
7896

79-
// TestMain_Smoke verifies the package compiles and the binary entry point exists.
80-
// The actual main() starts network listeners so it cannot be called in tests.
81-
func TestMain_Smoke(t *testing.T) {
82-
// Verify printBanner doesn't panic with zero-value config
83-
func() {
84-
defer func() {
85-
if r := recover(); r != nil {
86-
t.Errorf("printBanner panicked: %v", r)
87-
}
88-
}()
89-
captureStdout(t, func() { printBanner(&config.Config{}) })
97+
// TestPrintBanner_ZeroValueConfig_DoesNotPanic asserts that printBanner
98+
// survives a zero-value *config.Config — a regression like a nil-map deref
99+
// or missing-field panic on uninitialised input would be caught here.
100+
// Inherited from the original TestMain_Smoke, kept as its own test so the
101+
// guarantee survives the helper-process TestMain rewrite.
102+
func TestPrintBanner_ZeroValueConfig_DoesNotPanic(t *testing.T) {
103+
defer func() {
104+
if r := recover(); r != nil {
105+
t.Errorf("printBanner panicked on zero-value config: %v", r)
106+
}
90107
}()
108+
_ = captureStdout(t, func() { printBanner(&config.Config{}) })
109+
}
110+
111+
// TestMain_HelperProcess_Lifecycle re-execs this test binary as the proxy
112+
// daemon, waits for it to bind its listener, sends SIGTERM, and verifies a
113+
// clean exit. Exercises main()'s full startup-and-shutdown lifecycle.
114+
func TestMain_HelperProcess_Lifecycle(t *testing.T) {
115+
proxyPort := freePort(t)
116+
mgmtPort := freePort(t)
117+
118+
cmd := exec.CommandContext(t.Context(), os.Args[0]) //nolint:gosec // G204: helper-process pattern: os.Args[0] is the test binary itself, not external input
119+
cmd.Env = append(os.Environ(),
120+
"GO_WANT_HELPER_PROCESS=1",
121+
"BIND_ADDRESS=127.0.0.1",
122+
fmt.Sprintf("PROXY_PORT=%d", proxyPort),
123+
fmt.Sprintf("MANAGEMENT_PORT=%d", mgmtPort),
124+
"ENABLED_PACKS=SECRETS,GLOBAL",
125+
"USE_AI_DETECTION=false",
126+
)
127+
cmd.Dir = t.TempDir()
128+
stderr := &syncBuffer{}
129+
cmd.Stderr = stderr
130+
cmd.Stdout = io.Discard
131+
132+
if err := cmd.Start(); err != nil {
133+
t.Fatalf("start: %v", err)
134+
}
135+
t.Cleanup(func() {
136+
if cmd.ProcessState == nil {
137+
_ = cmd.Process.Kill()
138+
_ = cmd.Wait()
139+
}
140+
})
141+
142+
waitForLog(t, stderr, "[PROXY] Listening on", 10*time.Second)
91143

92-
// Self-referential sanity: package name is main
93-
if fmt.Sprintf("%T", main) != "func()" {
94-
t.Error("expected main to be func()")
144+
if err := cmd.Process.Signal(syscall.SIGTERM); err != nil {
145+
t.Fatalf("signal: %v", err)
146+
}
147+
148+
done := make(chan error, 1)
149+
go func() { done <- cmd.Wait() }()
150+
select {
151+
case err := <-done:
152+
if err != nil {
153+
t.Errorf("process exited with error: %v\n%s", err, stderr.String())
154+
}
155+
case <-time.After(10 * time.Second):
156+
_ = cmd.Process.Kill()
157+
t.Fatalf("process did not exit within 10s of SIGTERM\n%s", stderr.String())
158+
}
159+
}
160+
161+
// TestMain_HelperProcess_ProxyPortConflict_Fatal pre-binds the proxy port so
162+
// the subprocess's srv.ListenAndServe() fails, exercising runHTTPServer's
163+
// log.Fatalf branch.
164+
func TestMain_HelperProcess_ProxyPortConflict_Fatal(t *testing.T) {
165+
ln := listenLocal(t)
166+
defer func() { _ = ln.Close() }()
167+
addr, ok := ln.Addr().(*net.TCPAddr)
168+
if !ok {
169+
t.Fatalf("listener address %T is not *net.TCPAddr", ln.Addr())
170+
}
171+
proxyPort := addr.Port
172+
mgmtPort := freePort(t)
173+
174+
cmd := exec.CommandContext(t.Context(), os.Args[0]) //nolint:gosec // G204: helper-process pattern: os.Args[0] is the test binary itself, not external input
175+
cmd.Env = append(os.Environ(),
176+
"GO_WANT_HELPER_PROCESS=1",
177+
"BIND_ADDRESS=127.0.0.1",
178+
fmt.Sprintf("PROXY_PORT=%d", proxyPort),
179+
fmt.Sprintf("MANAGEMENT_PORT=%d", mgmtPort),
180+
"ENABLED_PACKS=SECRETS,GLOBAL",
181+
"USE_AI_DETECTION=false",
182+
)
183+
cmd.Dir = t.TempDir()
184+
out, err := cmd.CombinedOutput()
185+
if err == nil {
186+
t.Fatalf("expected non-zero exit on proxy port conflict, got success\n%s", out)
187+
}
188+
if !strings.Contains(string(out), "[PROXY] Fatal") {
189+
t.Errorf("expected '[PROXY] Fatal' in output, got:\n%s", out)
190+
}
191+
}
192+
193+
// TestMain_HelperProcess_MgmtPortConflict_Fatal pre-binds the management port
194+
// so the subprocess's mgmt.ListenAndServe() fails, exercising runManagementAPI's
195+
// log.Fatalf branch.
196+
func TestMain_HelperProcess_MgmtPortConflict_Fatal(t *testing.T) {
197+
ln := listenLocal(t)
198+
defer func() { _ = ln.Close() }()
199+
addr, ok := ln.Addr().(*net.TCPAddr)
200+
if !ok {
201+
t.Fatalf("listener address %T is not *net.TCPAddr", ln.Addr())
202+
}
203+
mgmtPort := addr.Port
204+
proxyPort := freePort(t)
205+
206+
cmd := exec.CommandContext(t.Context(), os.Args[0]) //nolint:gosec // G204: helper-process pattern: os.Args[0] is the test binary itself, not external input
207+
cmd.Env = append(os.Environ(),
208+
"GO_WANT_HELPER_PROCESS=1",
209+
"BIND_ADDRESS=127.0.0.1",
210+
fmt.Sprintf("PROXY_PORT=%d", proxyPort),
211+
fmt.Sprintf("MANAGEMENT_PORT=%d", mgmtPort),
212+
"ENABLED_PACKS=SECRETS,GLOBAL",
213+
"USE_AI_DETECTION=false",
214+
)
215+
cmd.Dir = t.TempDir()
216+
out, err := cmd.CombinedOutput()
217+
if err == nil {
218+
t.Fatalf("expected non-zero exit on mgmt port conflict, got success\n%s", out)
219+
}
220+
if !strings.Contains(string(out), "[MANAGEMENT] Fatal") {
221+
t.Errorf("expected '[MANAGEMENT] Fatal' in output, got:\n%s", out)
222+
}
223+
}
224+
225+
// TestMain_HelperProcess_ZeroPacks_Fatal verifies that a config with no packs
226+
// enabled triggers the startup guard's log.Fatalf (non-zero exit). Covers that
227+
// branch of main() which is otherwise unreachable from the lifecycle test.
228+
func TestMain_HelperProcess_ZeroPacks_Fatal(t *testing.T) {
229+
dir := t.TempDir()
230+
cfgPath := filepath.Join(dir, "proxy-config.json")
231+
if err := os.WriteFile(cfgPath, []byte(`{"enabledPacks":[]}`), 0o600); err != nil {
232+
t.Fatalf("write config: %v", err)
233+
}
234+
235+
cmd := exec.CommandContext(t.Context(), os.Args[0]) //nolint:gosec // G204: helper-process pattern: os.Args[0] is the test binary itself, not external input
236+
cmd.Env = append(os.Environ(), "GO_WANT_HELPER_PROCESS=1")
237+
cmd.Dir = dir
238+
out, err := cmd.CombinedOutput()
239+
if err == nil {
240+
t.Fatalf("expected non-zero exit on zero-packs guard, got success\n%s", out)
241+
}
242+
if !strings.Contains(string(out), "no PII detection packs enabled") {
243+
t.Errorf("expected guard message in output, got:\n%s", out)
244+
}
245+
}
246+
247+
// syncBuffer is a goroutine-safe wrapper around bytes.Buffer for use as
248+
// cmd.Stderr while a poller in the parent reads it concurrently.
249+
type syncBuffer struct {
250+
mu sync.Mutex
251+
buf bytes.Buffer
252+
}
253+
254+
func (b *syncBuffer) Write(p []byte) (int, error) {
255+
b.mu.Lock()
256+
defer b.mu.Unlock()
257+
return b.buf.Write(p)
258+
}
259+
260+
func (b *syncBuffer) String() string {
261+
b.mu.Lock()
262+
defer b.mu.Unlock()
263+
return b.buf.String()
264+
}
265+
266+
func waitForLog(t *testing.T, buf *syncBuffer, substr string, timeout time.Duration) {
267+
t.Helper()
268+
deadline := time.Now().Add(timeout)
269+
for time.Now().Before(deadline) {
270+
if strings.Contains(buf.String(), substr) {
271+
return
272+
}
273+
time.Sleep(50 * time.Millisecond)
95274
}
275+
t.Fatalf("did not see %q in log within %v\n%s", substr, timeout, buf.String())
96276
}

cmd/proxy/shutdown.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package main
2+
3+
import (
4+
"context"
5+
"log"
6+
"net/http"
7+
"os"
8+
"time"
9+
)
10+
11+
// installShutdownHandler blocks on quit, then calls srv.Shutdown with the given
12+
// timeout. Intended to run in a goroutine spawned by main().
13+
func installShutdownHandler(quit <-chan os.Signal, srv *http.Server, timeout time.Duration) {
14+
<-quit
15+
log.Printf("[PROXY] Shutting down…")
16+
ctx, cancel := context.WithTimeout(context.Background(), timeout)
17+
defer cancel()
18+
if err := srv.Shutdown(ctx); err != nil {
19+
log.Printf("[PROXY] Shutdown error: %v", err)
20+
}
21+
}

0 commit comments

Comments
 (0)