diff --git a/src/actions/errors.go b/src/actions/errors.go index 7dbd98a..93edcd9 100644 --- a/src/actions/errors.go +++ b/src/actions/errors.go @@ -31,7 +31,6 @@ import ( "time" "git.dcntrlized.org/Dcntrlized/Alarm/db" - "git.dcntrlized.org/Dcntrlized/Alarm/internal/alertstate" "git.dcntrlized.org/Dcntrlized/Alarm/internal/apierr" "git.dcntrlized.org/dcntrlized/dcntrlized-auth/authclient" "github.com/google/uuid" @@ -40,15 +39,16 @@ import ( // Frozen v1 error codes, aliased from internal/apierr so the WebSocket layer // (which cannot import this package) uses the same strings. const ( - CodeAlertNotFound = apierr.AlertNotFound - CodeAlertTokenInvalid = apierr.AlertTokenInvalid - CodeAlertNotAdmin = apierr.AlertNotAdmin - CodeAlertAlreadyJoined = apierr.AlertAlreadyJoined - CodeAlertNotSubscribed = apierr.AlertNotSubscribed - CodeAlertState = apierr.AlertState - CodeValidation = apierr.Validation - CodeUnauthorized = apierr.Unauthorized - CodeInternal = apierr.Internal + CodeAlertNotFound = apierr.AlertNotFound + CodeAlertTokenInvalid = apierr.AlertTokenInvalid + CodeAlertNotAdmin = apierr.AlertNotAdmin + CodeAlertAlreadyJoined = apierr.AlertAlreadyJoined + CodeAlertNotSubscribed = apierr.AlertNotSubscribed + CodeAlertState = apierr.AlertState + CodeAlarmTriggeredOffline = apierr.AlarmTriggeredOffline + CodeValidation = apierr.Validation + CodeUnauthorized = apierr.Unauthorized + CodeInternal = apierr.Internal ) // maxMessageBytes / maxNameChars bound the free-text fields. @@ -60,22 +60,11 @@ const ( maxPlatformLen = 32 ) -// errorEnvelope is the platform error body. -type errorEnvelope struct { - Error struct { - Code string `json:"code"` - Message string `json:"message"` - } `json:"error"` -} - -// writeError emits {"error":{"code","message"}}. +// writeError emits {"error":{"code","message"}}. It delegates to apierr.Write, +// which guarantees both fields are non-empty: this package can pass a literal +// message, but the invariant is enforced centrally (see internal/apierr). func writeError(w http.ResponseWriter, status int, code, message string) { - var body errorEnvelope - body.Error.Code = code - body.Error.Message = message - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(status) - _ = json.NewEncoder(w).Encode(body) + apierr.Write(w, status, code, message) } // writeJSON emits a success body. @@ -180,13 +169,6 @@ func validateMessage(raw string) (string, error) { return raw, nil } -func validateAutoStop(seconds int) error { - if seconds < 0 || seconds > alertstate.MaxAutoStopSeconds { - return fmt.Errorf("autoStopSeconds must be between 0 and %d", alertstate.MaxAutoStopSeconds) - } - return nil -} - func validateDeviceID(raw string) (string, error) { id := strings.TrimSpace(raw) if id == "" { diff --git a/src/handlers/actions/dispatch_test.go b/src/handlers/actions/dispatch_test.go new file mode 100644 index 0000000..b6f6c80 --- /dev/null +++ b/src/handlers/actions/dispatch_test.go @@ -0,0 +1,191 @@ +// Copyright (C) 2026 Dcntrlized +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published +// by the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package actions + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + "time" +) + +// The dispatch layer routes by verb and answers in plain text. This test walks +// every routed action and every dispatch-level rejection and asserts the +// invariant the user requires: an error is never a bare status line — every +// response carries a human-readable message, and every JSON error carries both +// "code" and "message". +// +// Requests carry a short deadline: the shared-auth proxy actions need a live +// dcntrlized-auth (which this gate does not assume), and the assertion is about +// the error body, not about the proxy hop. The deadline bounds the unreachable +// hop so the walk stays fast. +const dispatchTimeout = 600 * time.Millisecond + +var ( + alertActions = []string{ + "createAlert", "getAlert", "joinAlert", "listSubscriptions", + "removeSubscription", "triggerAlert", "disarmAlert", "alarmStatus", + "ackAlarm", "deleteAlert", "qrImage", + } + authProxyActions = []string{"login", "register", "refresh", "delete", "logout"} + readOnlyActions = []string{"getAlert", "listSubscriptions", "alarmStatus", "qrImage", "logout", "me"} + deleteActions = []string{"deleteAlert", "removeSubscription"} +) + +type verbCase struct { + method string + handler func(http.ResponseWriter, *http.Request, map[string]string) + names []string + body bool +} + +func verbs() []verbCase { + return []verbCase{ + {http.MethodGet, ActionActionNameGet, readOnlyActions, false}, + {http.MethodPost, ActionActionNamePost, append(append([]string{}, alertActions...), authProxyActions...), true}, + {http.MethodPut, ActionActionNamePut, nil, true}, + {http.MethodDelete, ActionActionNameDelete, deleteActions, true}, + } +} + +// serve runs one dispatch call with the given action name and body. +func serve(t *testing.T, vc verbCase, name, body string) *httptest.ResponseRecorder { + t.Helper() + ctx, cancel := context.WithTimeout(context.Background(), dispatchTimeout) + defer cancel() + + var req *http.Request + if vc.body { + req = httptest.NewRequest(vc.method, "/action/"+name, strings.NewReader(body)) + } else { + req = httptest.NewRequest(vc.method, "/action/"+name, nil) + } + req = req.WithContext(ctx) + req.Header.Set("Content-Type", "application/json") + + rec := httptest.NewRecorder() + vc.handler(rec, req, map[string]string{"actionName": name}) + return rec +} + +// assertReadableError is the invariant: non-empty body, and a complete JSON +// envelope when the response claims to be JSON. +func assertReadableError(t *testing.T, rec *httptest.ResponseRecorder, what string) { + t.Helper() + if rec.Code < 400 { + t.Fatalf("%s: expected an error status, got %d", what, rec.Code) + } + body := strings.TrimSpace(rec.Body.String()) + if body == "" { + t.Fatalf("%s: status %d with an empty body — an error must carry a message", what, rec.Code) + } + if ct := rec.Header().Get("Content-Type"); strings.Contains(ct, "application/json") { + var envelope struct { + Error struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` + } + if err := json.Unmarshal([]byte(body), &envelope); err != nil { + t.Fatalf("%s: JSON error body does not parse: %v (%s)", what, err, body) + } + if strings.TrimSpace(envelope.Error.Code) == "" { + t.Fatalf("%s: JSON error without a code: %s", what, body) + } + if strings.TrimSpace(envelope.Error.Message) == "" { + t.Fatalf("%s: JSON error without a message: %s", what, body) + } + } +} + +// TestUnknownActionsAreReadablePlainText walks GET/POST/PUT/DELETE with names no +// switch routes. Every one is 501 and every one must read. +func TestUnknownActionsAreReadablePlainText(t *testing.T) { + for _, vc := range verbs() { + for _, name := range []string{"nope", "getAlerts", "triggeralert", ""} { + rec := serve(t, vc, name, "{}") + if rec.Code != http.StatusNotImplemented { + t.Fatalf("%s /action/%s = %d, want 501", vc.method, name, rec.Code) + } + if ct := rec.Header().Get("Content-Type"); !strings.Contains(ct, "text/plain") { + t.Fatalf("%s /action/%s Content-Type = %q, want text/plain", vc.method, name, ct) + } + if strings.TrimSpace(rec.Body.String()) == "" { + t.Fatalf("%s /action/%s: 501 with an empty body", vc.method, name) + } + } + } +} + +// TestMalformedEnvelopesAreReadablePlainText covers the decoder failures that +// the dispatcher writes itself. +func TestMalformedEnvelopesAreReadablePlainText(t *testing.T) { + bodies := []string{ + "not json at all", + `{"payload":{},"unknown":"top-level"}`, + `[1,2,3]`, + `"a string"`, + } + for _, vc := range verbs() { + if !vc.body { + continue + } + for _, body := range bodies { + rec := serve(t, vc, "getAlert", body) + if rec.Code != http.StatusBadRequest { + t.Fatalf("%s body %q = %d, want 400", vc.method, body, rec.Code) + } + if strings.TrimSpace(rec.Body.String()) == "" { + t.Fatalf("%s body %q: 400 with an empty body", vc.method, body) + } + } + } +} + +// TestEveryRoutedActionHasReadableError is the route walk proper: every action +// the switches route, with a valid envelope and no access token, must answer +// with a readable error (401/403/404/409/500/502 depending on the action). +func TestEveryRoutedActionHasReadableError(t *testing.T) { + // Point the shared-auth proxy at a closed port so the proxy actions fail + // immediately (502 with a message) instead of depending on DNS or a live + // auth service. This keeps the walk hermetic and fast. + t.Setenv("AUTH_SERVICE_URL", "http://127.0.0.1:1") + + for _, vc := range verbs() { + for _, name := range vc.names { + rec := serve(t, vc, name, "{}") + assertReadableError(t, rec, vc.method+" /action/"+name) + } + } +} + +// TestPutIsReadableForKnownActions pins that PUT is routed but implements +// nothing: 501 for every name, known or not, with a message. +func TestPutIsReadableForKnownActions(t *testing.T) { + vc := verbCase{http.MethodPut, ActionActionNamePut, nil, true} + for _, name := range append(append([]string{}, alertActions...), "me", "logout") { + rec := serve(t, vc, name, "{}") + if rec.Code != http.StatusNotImplemented { + t.Fatalf("PUT /action/%s = %d, want 501", name, rec.Code) + } + if strings.TrimSpace(rec.Body.String()) == "" { + t.Fatalf("PUT /action/%s: 501 with an empty body", name) + } + } +} diff --git a/src/internal/apierr/apierr.go b/src/internal/apierr/apierr.go new file mode 100644 index 0000000..272b650 --- /dev/null +++ b/src/internal/apierr/apierr.go @@ -0,0 +1,143 @@ +// Copyright (C) 2026 Dcntrlized +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published +// by the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +// This file makes "every error response carries a human-readable message" a +// structural property instead of a convention: Write and WriteText are the only +// emitters of an error status in the service, and both substitute a non-empty +// message when handed an empty one. +// +// It exists because an error status with no readable body is a client-breaking +// bug that no amount of review reliably catches: http.Error(w, "", 500) and a +// JSON envelope built without a message both look harmless at the call site. +package apierr + +import ( + "encoding/json" + "fmt" + "net/http" + "strings" +) + +// fallbackMessage is the last resort for a code nobody registered. +const fallbackMessage = "unexpected error" + +// defaultMessages is the canonical human-readable text for each wire code. The +// handlers usually send a more specific message; this is the floor. +var defaultMessages = map[string]string{ + AlertNotFound: "alert not found", + AlertTokenInvalid: "the provided token does not match this alert", + AlertNotAdmin: "caller is not the alert admin", + AlertAlreadyJoined: "this device is already subscribed to this alert", + AlertNotSubscribed: "caller is not subscribed to this alert", + AlertState: "the alert is not in a state that allows this action", + AlarmTriggeredOffline: "an alarm was triggered while this device was offline", + Validation: "the request payload is not valid", + Unauthorized: "missing or invalid access token", + Internal: "internal error", + MethodNotAllowed: "method not allowed", + Unavailable: "service is unavailable", +} + +// Entry describes one documented wire code. +type Entry struct { + Code string + Message string +} + +// All returns every documented wire code with its default message, in a stable +// order (the order of the const block in codes.go). +func All() []Entry { + return []Entry{ + {AlertNotFound, defaultMessages[AlertNotFound]}, + {AlertTokenInvalid, defaultMessages[AlertTokenInvalid]}, + {AlertNotAdmin, defaultMessages[AlertNotAdmin]}, + {AlertAlreadyJoined, defaultMessages[AlertAlreadyJoined]}, + {AlertNotSubscribed, defaultMessages[AlertNotSubscribed]}, + {AlertState, defaultMessages[AlertState]}, + {AlarmTriggeredOffline, defaultMessages[AlarmTriggeredOffline]}, + {Validation, defaultMessages[Validation]}, + {Unauthorized, defaultMessages[Unauthorized]}, + {Internal, defaultMessages[Internal]}, + {MethodNotAllowed, defaultMessages[MethodNotAllowed]}, + {Unavailable, defaultMessages[Unavailable]}, + } +} + +// Message returns the canonical message for code. It never returns an empty +// string: an unknown or empty code gets a generic message (and Write upgrades an +// empty code to Internal before asking). +func Message(code string) string { + if m, ok := defaultMessages[code]; ok && strings.TrimSpace(m) != "" { + return m + } + return fallbackMessage +} + +// ErrorBody is the platform error envelope. +type ErrorBody struct { + Error struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` +} + +// Write emits {"error":{"code","message"}} and is the only way this service +// writes a JSON error. An empty code becomes ERR_INTERNAL and an empty message +// becomes the code's canonical message, so the body always carries both a code +// and a readable message. +func Write(w http.ResponseWriter, status int, code, message string) { + code = strings.TrimSpace(code) + if code == "" { + code = Internal + } + message = strings.TrimSpace(message) + if message == "" { + message = Message(code) + } + if status < 400 { + // Callers only reach this on an error path; keep a nonsensical status + // from producing an apparently successful error response. + status = http.StatusInternalServerError + } + + var body ErrorBody + body.Error.Code = code + body.Error.Message = message + + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(status) + _ = json.NewEncoder(w).Encode(body) +} + +// WriteText emits a plain-text error body (the dispatch-level responses). An +// empty message falls back to the status text and then to a generic phrase, so +// it can never write a bare status line. +func WriteText(w http.ResponseWriter, status int, message string) { + message = strings.TrimSpace(message) + if message == "" { + message = strings.TrimSpace(http.StatusText(status)) + } + if message == "" { + message = fallbackMessage + } + if status < 400 { + status = http.StatusInternalServerError + } + + w.Header().Set("Content-Type", "text/plain; charset=utf-8") + w.Header().Set("X-Content-Type-Options", "nosniff") + w.WriteHeader(status) + _, _ = fmt.Fprintln(w, message) +} diff --git a/src/internal/apierr/apierr_test.go b/src/internal/apierr/apierr_test.go new file mode 100644 index 0000000..7c7e6cf --- /dev/null +++ b/src/internal/apierr/apierr_test.go @@ -0,0 +1,160 @@ +// Copyright (C) 2026 Dcntrlized +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published +// by the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package apierr + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "regexp" + "strings" + "testing" +) + +// TestEveryCodeHasMessage walks the documented codes and fails if any of them +// would emit an empty message. +func TestEveryCodeHasMessage(t *testing.T) { + entries := All() + if len(entries) == 0 { + t.Fatal("All() returned no codes") + } + for _, e := range entries { + if strings.TrimSpace(e.Code) == "" { + t.Fatalf("code entry with empty code: %+v", e) + } + if strings.TrimSpace(e.Message) == "" { + t.Fatalf("%s has no default message", e.Code) + } + if got := Message(e.Code); got != e.Message { + t.Fatalf("Message(%s) = %q, want %q", e.Code, got, e.Message) + } + } +} + +// TestAllCoversEveryDeclaredCode reads codes.go and checks that no wire code +// constant exists without a default message registered in All(). Adding a code +// to the const block without registering it fails here rather than at runtime +// with a bodyless error. +func TestAllCoversEveryDeclaredCode(t *testing.T) { + src, err := os.ReadFile("codes.go") + if err != nil { + t.Fatalf("read codes.go: %v", err) + } + declared := regexp.MustCompile(`=\s*"(ERR_[A-Z_]+)"`).FindAllStringSubmatch(string(src), -1) + if len(declared) == 0 { + t.Fatal("found no ERR_ constants in codes.go") + } + known := make(map[string]bool, len(All())) + for _, e := range All() { + known[e.Code] = true + } + for _, m := range declared { + if !known[m[1]] { + t.Fatalf("code %s is declared in codes.go but missing from apierr.All() — it would have no default message", m[1]) + } + } +} + +// TestWriteFillsEmptyMessageAndCode pins the structural guarantee: the emitter, +// not the caller, is responsible for a readable body. +func TestWriteFillsEmptyMessageAndCode(t *testing.T) { + rec := httptest.NewRecorder() + Write(rec, http.StatusInternalServerError, "", "") + assertJSONError(t, rec, Internal, "") + + rec = httptest.NewRecorder() + Write(rec, http.StatusForbidden, AlertTokenInvalid, " ") + assertJSONError(t, rec, AlertTokenInvalid, Message(AlertTokenInvalid)) + + rec = httptest.NewRecorder() + Write(rec, http.StatusTeapot, "ERR_NOT_A_REAL_CODE", "") + assertJSONError(t, rec, "ERR_NOT_A_REAL_CODE", fallbackMessage) +} + +// TestWriteTextNeverEmpty covers the plain-text dispatch path, including the +// statuses it actually uses. This is the http.Error(w, "", 500) case. +func TestWriteTextNeverEmpty(t *testing.T) { + for _, status := range []int{ + http.StatusBadRequest, + http.StatusMethodNotAllowed, + http.StatusNotImplemented, + http.StatusServiceUnavailable, + } { + rec := httptest.NewRecorder() + WriteText(rec, status, "") + body := strings.TrimSpace(rec.Body.String()) + if rec.Code != status { + t.Fatalf("status %d: got %d", status, rec.Code) + } + if body == "" { + t.Fatalf("status %d: empty plain-text body", status) + } + if !strings.Contains(rec.Header().Get("Content-Type"), "text/plain") { + t.Fatalf("status %d: Content-Type = %q", status, rec.Header().Get("Content-Type")) + } + } + + rec := httptest.NewRecorder() + WriteText(rec, http.StatusBadRequest, "invalid request body: boom") + if got := strings.TrimSpace(rec.Body.String()); got != "invalid request body: boom" { + t.Fatalf("message not passed through: %q", got) + } +} + +// TestWriteAlwaysHasBothJSONFields is the shape assertion: {code, message} even +// when the caller supplies neither. +func TestWriteAlwaysHasBothJSONFields(t *testing.T) { + rec := httptest.NewRecorder() + Write(rec, http.StatusConflict, AlertState, "") + var body struct { + Error struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("body is not JSON: %v (%s)", err, rec.Body.String()) + } + if body.Error.Code == "" || body.Error.Message == "" { + t.Fatalf("incomplete envelope: %s", rec.Body.String()) + } +} + +func assertJSONError(t *testing.T, rec *httptest.ResponseRecorder, wantCode, wantMessage string) { + t.Helper() + if !strings.Contains(rec.Header().Get("Content-Type"), "application/json") { + t.Fatalf("Content-Type = %q", rec.Header().Get("Content-Type")) + } + var body struct { + Error struct { + Code string `json:"code"` + Message string `json:"message"` + } `json:"error"` + } + if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil { + t.Fatalf("body is not JSON: %v (%s)", err, rec.Body.String()) + } + if body.Error.Code != wantCode { + t.Fatalf("code = %q, want %q", body.Error.Code, wantCode) + } + if wantMessage != "" && body.Error.Message != wantMessage { + t.Fatalf("message = %q, want %q", body.Error.Message, wantMessage) + } + if strings.TrimSpace(body.Error.Message) == "" { + t.Fatal("empty message in the error envelope") + } +} diff --git a/src/internal/apierr/codes.go b/src/internal/apierr/codes.go index c6f53ff..b032e9a 100644 --- a/src/internal/apierr/codes.go +++ b/src/internal/apierr/codes.go @@ -28,7 +28,18 @@ const ( AlertAlreadyJoined = "ERR_ALERT_ALREADY_JOINED" AlertNotSubscribed = "ERR_ALERT_NOT_SUBSCRIBED" AlertState = "ERR_ALERT_STATE" - Validation = "ERR_VALIDATION" - Unauthorized = "ERR_UNAUTHORIZED" - Internal = "ERR_INTERNAL" + + // AlarmTriggeredOffline is NOT an error: it is the alarmStatus check-in + // answering 409 because this device still owes a trigger it missed while + // offline. The body carries the alarm frame and the client must ring. + AlarmTriggeredOffline = "ERR_ALARM_TRIGGERED_OFFLINE" + Validation = "ERR_VALIDATION" + Unauthorized = "ERR_UNAUTHORIZED" + Internal = "ERR_INTERNAL" + + // MethodNotAllowed and Unavailable are emitted on the /ws/alarm handshake + // only. They are JSON codes; the dispatch-level plain-text 405 carries no + // code (see API.md). + MethodNotAllowed = "ERR_METHOD_NOT_ALLOWED" + Unavailable = "ERR_UNAVAILABLE" )