From 688d32cc9f8006202e3a7ff6ef7d0e3ddb32fd61 Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 10 Apr 2026 16:20:34 +0700 Subject: [PATCH 1/5] registry: reject checker registration with empty ID Prevent silent bugs where a CheckerDefinition with an unset ID would be registered under the empty string key, becoming unfindable and potentially colliding with other empty-ID registrations. --- checker/registry.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/checker/registry.go b/checker/registry.go index 2a8af85..e8f5d55 100644 --- a/checker/registry.go +++ b/checker/registry.go @@ -33,6 +33,10 @@ var observationProviderRegistry = map[ObservationKey]ObservationProvider{} // always indicates a deployment mistake (two plugins shipping the same // checker, or a plugin shadowing a built-in). func RegisterChecker(c *CheckerDefinition) { + if c.ID == "" { + log.Println("Warning: refusing to register checker with empty ID") + return + } if _, exists := checkerRegistry[c.ID]; exists { log.Printf("Warning: checker %q is already registered; ignoring duplicate registration", c.ID) return From ec4efcf671ff4ec8922811b35ed131a6014c8baa Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 10 Apr 2026 16:20:48 +0700 Subject: [PATCH 2/5] server: limit request body size on POST endpoints Add io.LimitReader (1 MB cap) to /collect, /evaluate, and /report handlers to prevent memory exhaustion from oversized requests. --- checker/server.go | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/checker/server.go b/checker/server.go index 35fa80d..1036e99 100644 --- a/checker/server.go +++ b/checker/server.go @@ -18,12 +18,16 @@ import ( "context" "encoding/json" "fmt" + "io" "log" "net/http" "strings" "time" ) +// maxRequestBodySize is the maximum allowed size for incoming request bodies (1 MB). +const maxRequestBodySize = 1 << 20 + // Server is a generic HTTP server for external checkers. // It always exposes /health and /collect. If the provider implements // CheckerDefinitionProvider, it also exposes /definition and /evaluate. @@ -101,7 +105,7 @@ func (s *Server) handleDefinition(w http.ResponseWriter, r *http.Request) { func (s *Server) handleCollect(w http.ResponseWriter, r *http.Request) { var req ExternalCollectRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err := json.NewDecoder(io.LimitReader(r.Body, maxRequestBodySize)).Decode(&req); err != nil { writeJSON(w, http.StatusBadRequest, ExternalCollectResponse{ Error: fmt.Sprintf("invalid request body: %v", err), }) @@ -131,7 +135,7 @@ func (s *Server) handleCollect(w http.ResponseWriter, r *http.Request) { func (s *Server) handleEvaluate(w http.ResponseWriter, r *http.Request) { var req ExternalEvaluateRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err := json.NewDecoder(io.LimitReader(r.Body, maxRequestBodySize)).Decode(&req); err != nil { writeJSON(w, http.StatusBadRequest, ExternalEvaluateResponse{ Error: fmt.Sprintf("invalid request body: %v", err), }) @@ -159,7 +163,7 @@ func (s *Server) handleEvaluate(w http.ResponseWriter, r *http.Request) { func (s *Server) handleReport(w http.ResponseWriter, r *http.Request) { var req ExternalReportRequest - if err := json.NewDecoder(r.Body).Decode(&req); err != nil { + if err := json.NewDecoder(io.LimitReader(r.Body, maxRequestBodySize)).Decode(&req); err != nil { writeJSON(w, http.StatusBadRequest, map[string]string{ "error": fmt.Sprintf("invalid request body: %v", err), }) From ef7fffd4b765a9d0a0372d6c9ca471306e3bc16a Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 10 Apr 2026 16:24:11 +0700 Subject: [PATCH 3/5] tests: add coverage for options, types, and HTTP server - options_test.go: GetOption, GetFloatOption, GetIntOption, GetBoolOption with native types, JSON round-trips, missing keys, and wrong types - types_test.go: Status JSON marshal/unmarshal (strings, legacy ints, round-trip, unknown values), CheckTarget.Scope/String, BuildRulesInfo, empty-ID rejection - server_test.go: /health, /collect (success, error, bad body), /definition, /evaluate (all rules, disabled rule), /report (HTML, metrics, bad body), missing endpoints without CheckerDefinitionProvider --- checker/options_test.go | 127 ++++++++++++++++ checker/registry_test.go | 3 + checker/server_test.go | 319 +++++++++++++++++++++++++++++++++++++++ checker/types_test.go | 182 ++++++++++++++++++++++ 4 files changed, 631 insertions(+) create mode 100644 checker/options_test.go create mode 100644 checker/server_test.go create mode 100644 checker/types_test.go diff --git a/checker/options_test.go b/checker/options_test.go new file mode 100644 index 0000000..e736719 --- /dev/null +++ b/checker/options_test.go @@ -0,0 +1,127 @@ +// Copyright 2020-2026 The happyDomain Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package checker + +import ( + "encoding/json" + "testing" +) + +func TestGetOption_DirectType(t *testing.T) { + opts := CheckerOptions{"key": "hello"} + got, ok := GetOption[string](opts, "key") + if !ok || got != "hello" { + t.Errorf("GetOption[string] = (%q, %v), want (\"hello\", true)", got, ok) + } +} + +func TestGetOption_MissingKey(t *testing.T) { + opts := CheckerOptions{} + got, ok := GetOption[string](opts, "missing") + if ok || got != "" { + t.Errorf("GetOption[string] missing key = (%q, %v), want (\"\", false)", got, ok) + } +} + +func TestGetOption_JSONRoundTrip(t *testing.T) { + // Simulate what happens when options come from JSON: numbers become float64, + // structs become map[string]any. + type inner struct { + X int `json:"x"` + Y string `json:"y"` + } + opts := CheckerOptions{ + "obj": map[string]any{"x": float64(42), "y": "test"}, + } + got, ok := GetOption[inner](opts, "obj") + if !ok { + t.Fatal("GetOption[inner] returned false") + } + if got.X != 42 || got.Y != "test" { + t.Errorf("GetOption[inner] = %+v, want {X:42 Y:test}", got) + } +} + +func TestGetOption_WrongType(t *testing.T) { + opts := CheckerOptions{"key": 123} + got, ok := GetOption[string](opts, "key") + // int 123 cannot be unmarshaled into string via JSON + if ok { + t.Errorf("GetOption[string] with int value = (%q, true), want (\"\", false)", got) + } +} + +func TestGetFloatOption_NativeFloat(t *testing.T) { + opts := CheckerOptions{"f": 3.14} + if got := GetFloatOption(opts, "f", 0); got != 3.14 { + t.Errorf("GetFloatOption = %v, want 3.14", got) + } +} + +func TestGetFloatOption_JSONNumber(t *testing.T) { + opts := CheckerOptions{"f": json.Number("2.718")} + if got := GetFloatOption(opts, "f", 0); got != 2.718 { + t.Errorf("GetFloatOption(json.Number) = %v, want 2.718", got) + } +} + +func TestGetFloatOption_Missing(t *testing.T) { + opts := CheckerOptions{} + if got := GetFloatOption(opts, "f", 99.9); got != 99.9 { + t.Errorf("GetFloatOption missing = %v, want 99.9", got) + } +} + +func TestGetFloatOption_WrongType(t *testing.T) { + opts := CheckerOptions{"f": "not a number"} + if got := GetFloatOption(opts, "f", 1.0); got != 1.0 { + t.Errorf("GetFloatOption wrong type = %v, want 1.0", got) + } +} + +func TestGetIntOption(t *testing.T) { + opts := CheckerOptions{"i": float64(42)} + if got := GetIntOption(opts, "i", 0); got != 42 { + t.Errorf("GetIntOption = %v, want 42", got) + } +} + +func TestGetIntOption_Missing(t *testing.T) { + opts := CheckerOptions{} + if got := GetIntOption(opts, "i", 10); got != 10 { + t.Errorf("GetIntOption missing = %v, want 10", got) + } +} + +func TestGetBoolOption(t *testing.T) { + opts := CheckerOptions{"b": true} + if got := GetBoolOption(opts, "b", false); got != true { + t.Errorf("GetBoolOption = %v, want true", got) + } +} + +func TestGetBoolOption_Missing(t *testing.T) { + opts := CheckerOptions{} + if got := GetBoolOption(opts, "b", true); got != true { + t.Errorf("GetBoolOption missing = %v, want true", got) + } +} + +func TestGetBoolOption_WrongType(t *testing.T) { + opts := CheckerOptions{"b": "yes"} + if got := GetBoolOption(opts, "b", false); got != false { + t.Errorf("GetBoolOption wrong type = %v, want false", got) + } +} diff --git a/checker/registry_test.go b/checker/registry_test.go index bfe33d2..e4170b2 100644 --- a/checker/registry_test.go +++ b/checker/registry_test.go @@ -22,6 +22,9 @@ import ( // resetRegistries clears the global registries between tests so that one // test's registration cannot leak into the next. The package-level maps are // the only shared state. +// +// Tests in this package MUST NOT use t.Parallel() because they mutate +// these shared maps without synchronization. func resetRegistries() { checkerRegistry = map[string]*CheckerDefinition{} observationProviderRegistry = map[ObservationKey]ObservationProvider{} diff --git a/checker/server_test.go b/checker/server_test.go new file mode 100644 index 0000000..5802532 --- /dev/null +++ b/checker/server_test.go @@ -0,0 +1,319 @@ +// Copyright 2020-2026 The happyDomain Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package checker + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" +) + +// --- test doubles --- + +type testProvider struct { + key ObservationKey + collectFn func(ctx context.Context, opts CheckerOptions) (any, error) + definition *CheckerDefinition + htmlFn func(raw json.RawMessage) (string, error) + metricsFn func(raw json.RawMessage, t time.Time) ([]CheckMetric, error) +} + +func (p *testProvider) Key() ObservationKey { return p.key } +func (p *testProvider) Collect(ctx context.Context, opts CheckerOptions) (any, error) { + if p.collectFn != nil { + return p.collectFn(ctx, opts) + } + return map[string]string{"result": "ok"}, nil +} +func (p *testProvider) Definition() *CheckerDefinition { return p.definition } +func (p *testProvider) GetHTMLReport(raw json.RawMessage) (string, error) { + if p.htmlFn != nil { + return p.htmlFn(raw) + } + return "

report

", nil +} +func (p *testProvider) ExtractMetrics(raw json.RawMessage, t time.Time) ([]CheckMetric, error) { + if p.metricsFn != nil { + return p.metricsFn(raw, t) + } + return []CheckMetric{{Name: "m1", Value: 1.0, Timestamp: t}}, nil +} + +// dummyRule is a minimal CheckRule for testing evaluate. +type dummyRule struct { + name string + desc string +} + +func (r *dummyRule) Name() string { return r.name } +func (r *dummyRule) Description() string { return r.desc } +func (r *dummyRule) Evaluate(ctx context.Context, obs ObservationGetter, opts CheckerOptions) CheckState { + return CheckState{Status: StatusOK, Message: r.name + " passed"} +} + +// --- helpers --- + +func newTestServer(p *testProvider) *Server { + return NewServer(p) +} + +func doRequest(handler http.Handler, method, path string, body any, headers map[string]string) *httptest.ResponseRecorder { + var buf bytes.Buffer + if body != nil { + json.NewEncoder(&buf).Encode(body) + } + req := httptest.NewRequest(method, path, &buf) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + for k, v := range headers { + req.Header.Set(k, v) + } + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + return rec +} + +// --- tests --- + +func TestServer_Health(t *testing.T) { + p := &testProvider{key: "test", definition: &CheckerDefinition{ID: "test", Rules: []CheckRule{}}} + srv := newTestServer(p) + rec := doRequest(srv.Handler(), "GET", "/health", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("GET /health = %d, want %d", rec.Code, http.StatusOK) + } + var resp map[string]string + json.NewDecoder(rec.Body).Decode(&resp) + if resp["status"] != "ok" { + t.Errorf("GET /health status = %q, want \"ok\"", resp["status"]) + } +} + +func TestServer_Collect_Success(t *testing.T) { + p := &testProvider{ + key: "test", + definition: &CheckerDefinition{ID: "test", Rules: []CheckRule{}}, + collectFn: func(ctx context.Context, opts CheckerOptions) (any, error) { + return map[string]int{"count": 42}, nil + }, + } + srv := newTestServer(p) + rec := doRequest(srv.Handler(), "POST", "/collect", ExternalCollectRequest{ + Key: "test", + Options: CheckerOptions{"a": "b"}, + }, nil) + if rec.Code != http.StatusOK { + t.Fatalf("POST /collect = %d, want %d", rec.Code, http.StatusOK) + } + var resp ExternalCollectResponse + json.NewDecoder(rec.Body).Decode(&resp) + if resp.Error != "" { + t.Errorf("POST /collect error = %q, want empty", resp.Error) + } + if resp.Data == nil { + t.Fatal("POST /collect data is nil") + } +} + +func TestServer_Collect_ProviderError(t *testing.T) { + p := &testProvider{ + key: "test", + definition: &CheckerDefinition{ID: "test", Rules: []CheckRule{}}, + collectFn: func(ctx context.Context, opts CheckerOptions) (any, error) { + return nil, errors.New("provider failed") + }, + } + srv := newTestServer(p) + rec := doRequest(srv.Handler(), "POST", "/collect", ExternalCollectRequest{Key: "test"}, nil) + if rec.Code != http.StatusOK { + t.Fatalf("POST /collect = %d, want %d", rec.Code, http.StatusOK) + } + var resp ExternalCollectResponse + json.NewDecoder(rec.Body).Decode(&resp) + if resp.Error == "" { + t.Error("expected error in response, got empty") + } +} + +func TestServer_Collect_BadBody(t *testing.T) { + p := &testProvider{key: "test", definition: &CheckerDefinition{ID: "test", Rules: []CheckRule{}}} + srv := newTestServer(p) + req := httptest.NewRequest("POST", "/collect", bytes.NewBufferString("{invalid")) + rec := httptest.NewRecorder() + srv.Handler().ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("POST /collect bad body = %d, want %d", rec.Code, http.StatusBadRequest) + } +} + +func TestServer_Definition(t *testing.T) { + def := &CheckerDefinition{ + ID: "test-checker", + Name: "Test Checker", + Rules: []CheckRule{ + &dummyRule{name: "rule1", desc: "first rule"}, + }, + } + p := &testProvider{key: "test", definition: def} + srv := newTestServer(p) + rec := doRequest(srv.Handler(), "GET", "/definition", nil, nil) + if rec.Code != http.StatusOK { + t.Fatalf("GET /definition = %d, want %d", rec.Code, http.StatusOK) + } + var got CheckerDefinition + json.NewDecoder(rec.Body).Decode(&got) + if got.ID != "test-checker" { + t.Errorf("definition ID = %q, want \"test-checker\"", got.ID) + } + if len(got.RulesInfo) != 1 { + t.Errorf("definition rules = %d, want 1", len(got.RulesInfo)) + } +} + +func TestServer_Evaluate(t *testing.T) { + def := &CheckerDefinition{ + ID: "test-checker", + Name: "Test Checker", + Rules: []CheckRule{ + &dummyRule{name: "rule1", desc: "first rule"}, + &dummyRule{name: "rule2", desc: "second rule"}, + }, + } + p := &testProvider{key: "test", definition: def} + srv := newTestServer(p) + + rec := doRequest(srv.Handler(), "POST", "/evaluate", ExternalEvaluateRequest{ + Observations: map[ObservationKey]json.RawMessage{ + "test": json.RawMessage(`{"count":42}`), + }, + Options: CheckerOptions{}, + }, nil) + if rec.Code != http.StatusOK { + t.Fatalf("POST /evaluate = %d, want %d", rec.Code, http.StatusOK) + } + var resp ExternalEvaluateResponse + json.NewDecoder(rec.Body).Decode(&resp) + if len(resp.States) != 2 { + t.Fatalf("evaluate states = %d, want 2", len(resp.States)) + } + if resp.States[0].Code != "rule1" { + t.Errorf("evaluate state[0].Code = %q, want \"rule1\"", resp.States[0].Code) + } +} + +func TestServer_Evaluate_DisabledRule(t *testing.T) { + def := &CheckerDefinition{ + ID: "test-checker", + Rules: []CheckRule{ + &dummyRule{name: "rule1", desc: "first"}, + &dummyRule{name: "rule2", desc: "second"}, + }, + } + p := &testProvider{key: "test", definition: def} + srv := newTestServer(p) + + rec := doRequest(srv.Handler(), "POST", "/evaluate", ExternalEvaluateRequest{ + Observations: map[ObservationKey]json.RawMessage{ + "test": json.RawMessage(`{}`), + }, + EnabledRules: map[string]bool{"rule1": false}, + }, nil) + if rec.Code != http.StatusOK { + t.Fatalf("POST /evaluate = %d, want %d", rec.Code, http.StatusOK) + } + var resp ExternalEvaluateResponse + json.NewDecoder(rec.Body).Decode(&resp) + if len(resp.States) != 1 { + t.Fatalf("evaluate with disabled rule: states = %d, want 1", len(resp.States)) + } + if resp.States[0].Code != "rule2" { + t.Errorf("remaining state code = %q, want \"rule2\"", resp.States[0].Code) + } +} + +func TestServer_Report_HTML(t *testing.T) { + p := &testProvider{ + key: "test", + definition: &CheckerDefinition{ID: "test-checker", Rules: []CheckRule{}}, + htmlFn: func(raw json.RawMessage) (string, error) { + return "

hello

", nil + }, + } + srv := newTestServer(p) + rec := doRequest(srv.Handler(), "POST", "/report", ExternalReportRequest{ + Key: "test", + Data: json.RawMessage(`{}`), + }, map[string]string{"Accept": "text/html"}) + if rec.Code != http.StatusOK { + t.Fatalf("POST /report html = %d, want %d", rec.Code, http.StatusOK) + } + if ct := rec.Header().Get("Content-Type"); ct != "text/html; charset=utf-8" { + t.Errorf("Content-Type = %q, want text/html", ct) + } + if body := rec.Body.String(); body != "

hello

" { + t.Errorf("body = %q, want \"

hello

\"", body) + } +} + +func TestServer_Report_Metrics(t *testing.T) { + p := &testProvider{ + key: "test", + definition: &CheckerDefinition{ID: "test-checker", Rules: []CheckRule{}}, + } + srv := newTestServer(p) + rec := doRequest(srv.Handler(), "POST", "/report", ExternalReportRequest{ + Key: "test", + Data: json.RawMessage(`{}`), + }, map[string]string{"Accept": "application/json"}) + if rec.Code != http.StatusOK { + t.Fatalf("POST /report metrics = %d, want %d", rec.Code, http.StatusOK) + } + var metrics []CheckMetric + json.NewDecoder(rec.Body).Decode(&metrics) + if len(metrics) != 1 { + t.Errorf("metrics count = %d, want 1", len(metrics)) + } +} + +func TestServer_Report_BadBody(t *testing.T) { + p := &testProvider{ + key: "test", + definition: &CheckerDefinition{ID: "test-checker", Rules: []CheckRule{}}, + } + srv := newTestServer(p) + req := httptest.NewRequest("POST", "/report", bytes.NewBufferString("{bad")) + rec := httptest.NewRecorder() + srv.Handler().ServeHTTP(rec, req) + if rec.Code != http.StatusBadRequest { + t.Errorf("POST /report bad body = %d, want %d", rec.Code, http.StatusBadRequest) + } +} + +func TestServer_NoDefinition_NoEvaluateEndpoint(t *testing.T) { + // A provider that does NOT implement CheckerDefinitionProvider + p := &stubProvider{key: "basic"} + srv := NewServer(p) + rec := doRequest(srv.Handler(), "POST", "/evaluate", nil, nil) + // Should 404 or 405 since /evaluate is not registered + if rec.Code == http.StatusOK { + t.Error("POST /evaluate should not be available without CheckerDefinitionProvider") + } +} diff --git a/checker/types_test.go b/checker/types_test.go new file mode 100644 index 0000000..7b66af2 --- /dev/null +++ b/checker/types_test.go @@ -0,0 +1,182 @@ +// Copyright 2020-2026 The happyDomain Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package checker + +import ( + "encoding/json" + "testing" +) + +func TestStatus_MarshalJSON(t *testing.T) { + tests := []struct { + status Status + want string + }{ + {StatusOK, `"OK"`}, + {StatusInfo, `"INFO"`}, + {StatusUnknown, `"UNKNOWN"`}, + {StatusWarn, `"WARN"`}, + {StatusCrit, `"CRIT"`}, + {StatusError, `"ERROR"`}, + } + for _, tt := range tests { + got, err := json.Marshal(tt.status) + if err != nil { + t.Errorf("Marshal(%v) error: %v", tt.status, err) + continue + } + if string(got) != tt.want { + t.Errorf("Marshal(%v) = %s, want %s", tt.status, got, tt.want) + } + } +} + +func TestStatus_UnmarshalJSON_String(t *testing.T) { + tests := []struct { + input string + want Status + }{ + {`"OK"`, StatusOK}, + {`"INFO"`, StatusInfo}, + {`"UNKNOWN"`, StatusUnknown}, + {`""`, StatusUnknown}, + {`"WARN"`, StatusWarn}, + {`"CRIT"`, StatusCrit}, + {`"ERROR"`, StatusError}, + } + for _, tt := range tests { + var got Status + if err := json.Unmarshal([]byte(tt.input), &got); err != nil { + t.Errorf("Unmarshal(%s) error: %v", tt.input, err) + continue + } + if got != tt.want { + t.Errorf("Unmarshal(%s) = %v, want %v", tt.input, got, tt.want) + } + } +} + +func TestStatus_UnmarshalJSON_LegacyInt(t *testing.T) { + tests := []struct { + input string + want Status + }{ + {`-2`, StatusOK}, + {`-1`, StatusInfo}, + {`0`, StatusUnknown}, + {`1`, StatusWarn}, + {`2`, StatusCrit}, + {`3`, StatusError}, + } + for _, tt := range tests { + var got Status + if err := json.Unmarshal([]byte(tt.input), &got); err != nil { + t.Errorf("Unmarshal(%s) error: %v", tt.input, err) + continue + } + if got != tt.want { + t.Errorf("Unmarshal(%s) = %v, want %v", tt.input, got, tt.want) + } + } +} + +func TestStatus_UnmarshalJSON_UnknownString(t *testing.T) { + var s Status + err := json.Unmarshal([]byte(`"BOGUS"`), &s) + if err == nil { + t.Error("Unmarshal(\"BOGUS\") should return error, got nil") + } +} + +func TestStatus_RoundTrip(t *testing.T) { + for _, s := range []Status{StatusOK, StatusInfo, StatusUnknown, StatusWarn, StatusCrit, StatusError} { + data, err := json.Marshal(s) + if err != nil { + t.Fatalf("Marshal(%v) error: %v", s, err) + } + var got Status + if err := json.Unmarshal(data, &got); err != nil { + t.Fatalf("Unmarshal(%s) error: %v", data, err) + } + if got != s { + t.Errorf("round-trip %v: got %v", s, got) + } + } +} + +func TestStatus_String(t *testing.T) { + if got := StatusOK.String(); got != "OK" { + t.Errorf("StatusOK.String() = %q, want \"OK\"", got) + } + if got := Status(99).String(); got != "Status(99)" { + t.Errorf("Status(99).String() = %q, want \"Status(99)\"", got) + } +} + +func TestCheckTarget_Scope(t *testing.T) { + tests := []struct { + target CheckTarget + want CheckScopeType + }{ + {CheckTarget{}, CheckScopeUser}, + {CheckTarget{UserId: "u1"}, CheckScopeUser}, + {CheckTarget{DomainId: "d1"}, CheckScopeDomain}, + {CheckTarget{DomainId: "d1", ServiceId: "s1"}, CheckScopeService}, + {CheckTarget{ServiceId: "s1"}, CheckScopeService}, + } + for _, tt := range tests { + if got := tt.target.Scope(); got != tt.want { + t.Errorf("%+v.Scope() = %v, want %v", tt.target, got, tt.want) + } + } +} + +func TestCheckTarget_String(t *testing.T) { + tests := []struct { + target CheckTarget + want string + }{ + {CheckTarget{}, ""}, + {CheckTarget{UserId: "u1"}, "u1"}, + {CheckTarget{UserId: "u1", DomainId: "d1"}, "u1/d1"}, + {CheckTarget{UserId: "u1", DomainId: "d1", ServiceId: "s1"}, "u1/d1/s1"}, + } + for _, tt := range tests { + if got := tt.target.String(); got != tt.want { + t.Errorf("%+v.String() = %q, want %q", tt.target, got, tt.want) + } + } +} + +func TestCheckerDefinition_BuildRulesInfo(t *testing.T) { + d := &CheckerDefinition{ + Rules: []CheckRule{&dummyRule{name: "r1", desc: "desc1"}}, + } + d.BuildRulesInfo() + if len(d.RulesInfo) != 1 { + t.Fatalf("BuildRulesInfo: got %d rules, want 1", len(d.RulesInfo)) + } + if d.RulesInfo[0].Name != "r1" || d.RulesInfo[0].Description != "desc1" { + t.Errorf("BuildRulesInfo: got %+v, want {Name:r1, Description:desc1}", d.RulesInfo[0]) + } +} + +func TestRegisterChecker_EmptyIDRejected(t *testing.T) { + resetRegistries() + RegisterChecker(&CheckerDefinition{ID: "", Name: "bad"}) + if len(GetCheckers()) != 0 { + t.Error("checker with empty ID should not be registered") + } +} From 2fa44f69a41308953fd089a763ab92cc95d1f108 Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 10 Apr 2026 16:33:34 +0700 Subject: [PATCH 4/5] server: return 500 status on collect errors instead of 200 Errors from provider.Collect() and json.Marshal were returned with HTTP 200, making failures invisible to monitoring, proxies, and clients that check status codes. Return 500 Internal Server Error so HTTP-level tooling can detect failures without parsing the response body. --- checker/server.go | 4 ++-- checker/server_test.go | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/checker/server.go b/checker/server.go index 1036e99..6065fbd 100644 --- a/checker/server.go +++ b/checker/server.go @@ -114,7 +114,7 @@ func (s *Server) handleCollect(w http.ResponseWriter, r *http.Request) { data, err := s.provider.Collect(r.Context(), req.Options) if err != nil { - writeJSON(w, http.StatusOK, ExternalCollectResponse{ + writeJSON(w, http.StatusInternalServerError, ExternalCollectResponse{ Error: err.Error(), }) return @@ -122,7 +122,7 @@ func (s *Server) handleCollect(w http.ResponseWriter, r *http.Request) { raw, err := json.Marshal(data) if err != nil { - writeJSON(w, http.StatusOK, ExternalCollectResponse{ + writeJSON(w, http.StatusInternalServerError, ExternalCollectResponse{ Error: fmt.Sprintf("failed to marshal result: %v", err), }) return diff --git a/checker/server_test.go b/checker/server_test.go index 5802532..3bc36bd 100644 --- a/checker/server_test.go +++ b/checker/server_test.go @@ -143,8 +143,8 @@ func TestServer_Collect_ProviderError(t *testing.T) { } srv := newTestServer(p) rec := doRequest(srv.Handler(), "POST", "/collect", ExternalCollectRequest{Key: "test"}, nil) - if rec.Code != http.StatusOK { - t.Fatalf("POST /collect = %d, want %d", rec.Code, http.StatusOK) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("POST /collect = %d, want %d", rec.Code, http.StatusInternalServerError) } var resp ExternalCollectResponse json.NewDecoder(rec.Body).Decode(&resp) From 36a72f013a2cd1ec8eacea9a92928e06c38daba5 Mon Sep 17 00:00:00 2001 From: Pierre-Olivier Mercier Date: Fri, 10 Apr 2026 16:43:41 +0700 Subject: [PATCH 5/5] server: document lack of built-in authentication on Server type --- checker/server.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/checker/server.go b/checker/server.go index 6065fbd..c036b07 100644 --- a/checker/server.go +++ b/checker/server.go @@ -33,6 +33,10 @@ const maxRequestBodySize = 1 << 20 // CheckerDefinitionProvider, it also exposes /definition and /evaluate. // If the provider implements CheckerHTMLReporter or CheckerMetricsReporter, // it also exposes /report. +// +// Security: Server does not perform any authentication or authorization. +// It is intended to be run behind a reverse proxy or in a trusted network +// where access control is handled externally (e.g. by the happyDomain server). type Server struct { provider ObservationProvider definition *CheckerDefinition