Preserve explicit audit selections before reads and webhook updates

Reject damaged query encoding and repeated singleton selectors in the canonical list, export and summary handlers before reading events. Require an explicit webhook urls array so absent selections cannot silently disable forwarding; intentional empty arrays, URL validation and tenant boundaries remain intact. Add executable handler/exporter and mutation preservation controls and update the owning contracts.

Change-source: pulse-maintainer
This commit is contained in:
pulse-triage[bot]
2026-10-24 05:21:01 +01:00
parent 9182e7b206
commit 433f617db2
6 changed files with 368 additions and 4 deletions
@@ -4011,6 +4011,14 @@ existing export/summary non-pagination semantics. This changes only audit
request validation, not agent identity, enrolment, collection or recovery.
Fake-logger handler controls live in `internal/api/audit_filter_validation_test.go`.
The canonical audit read handlers also refuse damaged raw query encoding and
repeated named singleton selectors before any event read, rather than silently
losing a filter or taking its first value. Absent webhook URL selections are
not deliberate removal: POST/PUT require an explicit array, with `[]` retaining
its intentional clear semantics. `internal/api/audit_selection_validation_test.go`
pins read non-execution, selected wire results, tenant isolation and mutation
preservation/clear/restore. Agent operations and storage internals are unchanged.
## Forbidden Paths
1. New install or update continuity behavior hidden only inside broad monitoring ownership.
@@ -4082,6 +4082,26 @@ exporter are pinned by `internal/api/audit_filter_validation_test.go` with
fake persistent loggers, including query non-execution and tenant isolation;
`internal/api/audit_handlers_test.go` pins valid export/summary filter payloads.
List/export/summary parse the entire raw query with `url.ParseQuery`; damaged
encoding (including an invalid unknown pair) returns `400 invalid_query`, never
a partial selection. The named singleton parameters `event`, `user`,
`startTime`, `endTime`, `success`, `limit`, `offset`, `format` and `verify` may
appear at most once, including under encoded keys; repeated values return
`400 ambiguous_query`, even when identical. Valid encoded values, unknown
well-formed parameters and existing per-endpoint filter semantics remain.
Rejections occur before Query/Count/signature checks or export attachment
headers. `internal/api/audit_selection_validation_test.go` exercises the actual
handlers/exporter, scoped wire results and tenant isolation without a database.
POST/PUT `/api/admin/webhooks/audit` require an explicit `urls` array.
`null`, `{}` and `{"urls":null}` return `400 invalid_request` without updating
existing destinations. `{"urls":[]}` is intentional removal of every endpoint;
non-empty selections still undergo all existing URL/SSRF validation before one
update. Body bounds, organisation/access/entitlement gates and best-effort
forwarding remain unchanged; these controls establish no receiver delivery.
The same test file pins both methods, unchanged state after refusal, deliberate
clear/restore, invalid-URL atomicity and tenant-scoped mutation.
## Forbidden Paths
1. Handler-local payload shape drift without a contract test
@@ -2744,6 +2744,14 @@ This boundary adds no database/schema, retention, signing, quota or recovery
change. Fake persistent-logger controls in
`internal/api/audit_filter_validation_test.go` prove rejection without a store.
The canonical audit read handlers also refuse damaged raw query encoding and
repeated named singleton selectors before any event read, rather than silently
losing a filter or taking its first value. Absent webhook URL selections are
not deliberate removal: POST/PUT require an explicit array, with `[]` retaining
its intentional clear semantics. `internal/api/audit_selection_validation_test.go`
pins read non-execution, selected wire results, tenant isolation and mutation
preservation/clear/restore. Agent operations and storage internals are unchanged.
## Forbidden Paths
1. Reintroducing storage or recovery product logic as ad hoc dashboard-only summaries without a canonical page-surface owner
+40 -4
View File
@@ -99,7 +99,10 @@ type auditQueryValidationError struct {
}
func auditEventListFilterFromRequest(r *http.Request) (audit.QueryFilter, *auditQueryValidationError) {
query := r.URL.Query()
query, validationErr := auditEventQueryFromRequest(r)
if validationErr != nil {
return audit.QueryFilter{}, validationErr
}
filter := audit.QueryFilter{
EventType: query.Get("event"),
User: query.Get("user"),
@@ -136,6 +139,27 @@ func auditEventListFilterFromRequest(r *http.Request) (audit.QueryFilter, *audit
return filter, nil
}
// URL.Query silently discards damaged pairs. An audit selection must be parsed
// completely, and singleton selectors must not depend on which value came first.
func auditEventQueryFromRequest(r *http.Request) (url.Values, *auditQueryValidationError) {
query, err := url.ParseQuery(r.URL.RawQuery)
if err != nil {
return nil, &auditQueryValidationError{
code: "invalid_query",
message: "Invalid audit query encoding",
}
}
for _, key := range []string{"event", "user", "startTime", "endTime", "success", "limit", "offset", "format", "verify"} {
if len(query[key]) > 1 {
return nil, &auditQueryValidationError{
code: "ambiguous_query",
message: fmt.Sprintf("Audit query parameter %s must be supplied at most once", key),
}
}
}
return query, nil
}
// A supplied malformed or empty bound must not become an unfiltered read.
// Keep list, export and summary on the same time-window validation boundary.
func auditTimeFilterFromQuery(query url.Values, filter *audit.QueryFilter) *auditQueryValidationError {
@@ -283,9 +307,13 @@ func (h *AuditHandlers) HandleUpdateWebhooks(w http.ResponseWriter, r *http.Requ
writeErrorResponse(w, http.StatusBadRequest, "invalid_request", "Invalid request body", nil)
return
}
if req.URLs == nil {
writeErrorResponse(w, http.StatusBadRequest, "invalid_request", "An explicit urls array is required; use [] to remove all audit webhooks", nil)
return
}
// Validate all webhook URLs
var validatedURLs []string
validatedURLs := make([]string, 0, len(req.URLs))
for _, rawURL := range req.URLs {
if err := validateWebhookURL(rawURL); err != nil {
writeErrorResponse(w, http.StatusBadRequest, "invalid_url", fmt.Sprintf("Invalid webhook URL: %s", err.Error()), map[string]string{"url": rawURL})
@@ -443,7 +471,11 @@ func (h *AuditHandlers) HandleExportAuditEvents(w http.ResponseWriter, r *http.R
return
}
query := r.URL.Query()
query, validationErr := auditEventQueryFromRequest(r)
if validationErr != nil {
writeErrorResponse(w, http.StatusBadRequest, validationErr.code, validationErr.message, nil)
return
}
// Parse format
format := audit.ExportFormatJSON
@@ -509,7 +541,11 @@ func (h *AuditHandlers) HandleAuditSummary(w http.ResponseWriter, r *http.Reques
return
}
query := r.URL.Query()
query, validationErr := auditEventQueryFromRequest(r)
if validationErr != nil {
writeErrorResponse(w, http.StatusBadRequest, validationErr.code, validationErr.message, nil)
return
}
// Parse filter
filter := audit.QueryFilter{
+23
View File
@@ -407,6 +407,29 @@ func TestHandleUpdateWebhooks(t *testing.T) {
}
}
func TestHandleUpdateWebhooksRejectsNonArrayAndOversizedSelections(t *testing.T) {
for _, body := range []string{
`{"urls":"https://8.8.8.8/audit"}`,
`{"urls":false}`,
`{"urls":[{}]}`,
strings.Repeat(" ", 64*1024) + `{"urls":[]}`,
} {
logger := newAuditSelectionLogger()
logger.urls = []string{"https://8.8.8.8/audit"}
setAuditLogger(t, logger)
rec := httptest.NewRecorder()
NewAuditHandlers().HandleUpdateWebhooks(rec, httptest.NewRequest(http.MethodPut,
"/api/admin/webhooks/audit", strings.NewReader(body)))
var payload APIError
if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil {
t.Fatal(err)
}
if rec.Code != http.StatusBadRequest || payload.Code != "invalid_request" || logger.updates != 0 || len(logger.urls) != 1 {
t.Fatalf("invalid selection changed destination: status/code/updates = %d/%q/%d", rec.Code, payload.Code, logger.updates)
}
}
}
func TestHandleExportAuditEvents_NotPersistent(t *testing.T) {
setAuditLogger(t, audit.NewConsoleLogger())
handler := NewAuditHandlers()
@@ -0,0 +1,269 @@
package api
import (
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"net/url"
"reflect"
"strings"
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/pkg/audit"
)
// Exercise the canonical handlers and exporter without a database or receiver.
func TestAuditSelectionRejectsDamagedQuery(t *testing.T) {
queries := []string{
"user=%zz", "event=%", "success=%zz", "startTime=%zz", "endTime=%zz",
"limit=%zz", "offset=%zz", "format=%zz", "verify=%zz", "%zz=operator",
"user=operator;event=login", "user=operator&user=%zz", "event=login&unknown=%zz",
}
for _, endpoint := range auditScopeEndpoints() {
for _, query := range queries {
t.Run(endpoint.name+"/"+query, func(t *testing.T) {
assertAuditSelectionRejected(t, endpoint.handle, query, "invalid_query")
})
}
}
}
func TestAuditSelectionRejectsRepeatedParameters(t *testing.T) {
values := map[string]string{
"event": "login", "user": "operator", "success": "true", "startTime": "2026-01-01T00:00:00Z",
"endTime": "2026-01-02T00:00:00Z", "limit": "10", "offset": "0", "format": "json", "verify": "true",
}
for _, endpoint := range auditScopeEndpoints() {
for _, key := range []string{"event", "user", "success", "startTime", "endTime", "limit", "offset", "format", "verify"} {
for _, variant := range []string{"identical", "conflicting", "encoded_key"} {
t.Run(endpoint.name+"/"+key+"/"+variant, func(t *testing.T) {
first, second := key, url.QueryEscape(values[key])
if variant == "conflicting" {
second = "different"
}
if variant == "encoded_key" {
first = fmt.Sprintf("%%%02x%s", key[0], key[1:])
}
query := first + "=" + url.QueryEscape(values[key]) + "&" + key + "=" + second
assertAuditSelectionRejected(t, endpoint.handle, query, "ambiguous_query")
})
}
}
}
}
func assertAuditSelectionRejected(t *testing.T, handle http.HandlerFunc, query, code string) {
t.Helper()
logger := &auditScopeLogger{testAuditLogger: testAuditLogger{events: []audit.Event{{ID: "unselected-event", Signature: "synthetic"}}}}
setAuditLogger(t, logger)
rec := httptest.NewRecorder()
handle(rec, httptest.NewRequest(http.MethodGet, "/api/audit?"+query, nil))
var body APIError
if err := json.Unmarshal(rec.Body.Bytes(), &body); err != nil {
t.Fatal(err)
}
if rec.Code != http.StatusBadRequest || body.Code != code {
t.Errorf("status/code = %d/%q, want 400/%q", rec.Code, body.Code, code)
}
if logger.queries != 0 || logger.counts != 0 || logger.verifications != 0 {
t.Errorf("query/count/verify = %d/%d/%d, want no event read", logger.queries, logger.counts, logger.verifications)
}
if strings.Contains(rec.Body.String(), "unselected-event") || rec.Header().Get("Content-Disposition") != "" || rec.Header().Get("X-Event-Count") != "" {
t.Error("rejected selection returned event/export content")
}
}
// A filter-backed, in-memory logger makes widening visible in the wire result.
type auditSelectionLogger struct {
auditScopeLogger
urls []string
updates int
}
func (l *auditSelectionLogger) selected(f audit.QueryFilter) []audit.Event {
events := []audit.Event{}
for _, e := range l.events {
if f.EventType != "" && e.EventType != f.EventType || f.User != "" && e.User != f.User || f.Success != nil && e.Success != *f.Success || f.StartTime != nil && e.Timestamp.Before(*f.StartTime) || f.EndTime != nil && !e.Timestamp.Before(*f.EndTime) {
continue
}
events = append(events, e)
}
return events
}
func (l *auditSelectionLogger) Query(f audit.QueryFilter) ([]audit.Event, error) {
l.queries++
l.lastQuery = f
events := l.selected(f)
if f.Offset >= len(events) {
return []audit.Event{}, nil
}
events = events[f.Offset:]
if f.Limit > 0 && len(events) > f.Limit {
events = events[:f.Limit]
}
return events, nil
}
func (l *auditSelectionLogger) Count(f audit.QueryFilter) (int, error) {
l.counts++
l.lastCount = f
return len(l.selected(f)), nil
}
func (l *auditSelectionLogger) GetWebhookURLs() []string { return append([]string{}, l.urls...) }
func (l *auditSelectionLogger) UpdateWebhookURLs(urls []string) error {
l.updates++
l.urls = append([]string{}, urls...)
return l.updateErr
}
func newAuditSelectionLogger() *auditSelectionLogger {
at := time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC)
return &auditSelectionLogger{auditScopeLogger: auditScopeLogger{testAuditLogger: testAuditLogger{verifyResult: true, events: []audit.Event{
{ID: "selected-event", EventType: "login", User: "operator+é@example.invalid", Timestamp: at, Success: true, Signature: "synthetic"},
{ID: "other-user", EventType: "login", User: "another", Timestamp: at, Success: true},
{ID: "other-event", EventType: "logout", User: "operator+é@example.invalid", Timestamp: at, Success: true},
{ID: "outside-window", EventType: "login", User: "operator+é@example.invalid", Timestamp: at.Add(-48 * time.Hour), Success: true},
}}}}
}
func TestAuditSelectionPreservesEncodedWireResults(t *testing.T) {
for _, endpoint := range auditScopeEndpoints() {
for _, format := range []string{"json", "csv"} {
t.Run(endpoint.name+"/"+format, func(t *testing.T) {
logger := newAuditSelectionLogger()
setAuditLogger(t, logger)
q := url.Values{"user": {"operator+é@example.invalid"}, "event": {"login"}, "startTime": {"2026-01-01T01:00:00+01:00"}, "endTime": {"2026-01-02T00:00:00Z"}, "verify": {"true"}, "format": {format}, "unknown": {"one", "two"}}
rec := httptest.NewRecorder()
endpoint.handle(rec, httptest.NewRequest(http.MethodGet, "/api/audit?"+q.Encode(), nil))
if rec.Code != 200 || logger.queries != 1 || logger.lastQuery.User != q.Get("user") || logger.lastQuery.EventType != "login" {
t.Fatalf("status/query/filter = %d/%d/%+v", rec.Code, logger.queries, logger.lastQuery)
}
for _, id := range []string{"other-user", "other-event", "outside-window"} {
if strings.Contains(rec.Body.String(), id) {
t.Fatalf("widened selection contains %s", id)
}
}
if endpoint.name == "summary" {
var summary audit.ExportSummary
if err := json.Unmarshal(rec.Body.Bytes(), &summary); err != nil {
t.Fatal(err)
}
if summary.TotalEvents != 1 {
t.Fatalf("summary count = %d", summary.TotalEvents)
}
} else if !strings.Contains(rec.Body.String(), "selected-event") {
t.Fatal("selected event missing")
}
if endpoint.name == "export" && rec.Header().Get("X-Event-Count") != "1" {
t.Fatal("export count not scoped")
}
})
}
}
}
func TestAuditWebhookSelectionRequiresExplicitArray(t *testing.T) {
for _, method := range []string{http.MethodPost, http.MethodPut} {
for _, body := range []string{"null", "{}", `{"urls":null}`} {
t.Run(method+"/"+body, func(t *testing.T) {
logger := newAuditSelectionLogger()
logger.urls = []string{"https://8.8.8.8/audit"}
setAuditLogger(t, logger)
rec := httptest.NewRecorder()
NewAuditHandlers().HandleUpdateWebhooks(rec, httptest.NewRequest(method, "/api/admin/webhooks/audit", strings.NewReader(body)))
var payload APIError
if err := json.Unmarshal(rec.Body.Bytes(), &payload); err != nil {
t.Fatal(err)
}
if rec.Code != 400 || payload.Code != "invalid_request" || logger.updates != 0 || !reflect.DeepEqual(logger.urls, []string{"https://8.8.8.8/audit"}) {
t.Fatalf("status/code/updates/urls = %d/%q/%d/%v", rec.Code, payload.Code, logger.updates, logger.urls)
}
})
}
}
}
func TestAuditWebhookSelectionExplicitClearRestoreAndValidation(t *testing.T) {
for _, method := range []string{http.MethodPost, http.MethodPut} {
t.Run(method, func(t *testing.T) {
logger := newAuditSelectionLogger()
logger.urls = []string{"https://8.8.8.8/audit"}
setAuditLogger(t, logger)
for _, tc := range []struct {
body string
status, updates int
urls []string
}{
{`{"urls":["http://127.0.0.1/private"]}`, 400, 0, []string{"https://8.8.8.8/audit"}},
{`{"urls":[]}`, 204, 1, []string{}},
{`{"urls":["https://8.8.8.8/audit"]}`, 204, 2, []string{"https://8.8.8.8/audit"}},
{`{"urls":["https://8.8.8.8/changed","http://127.0.0.1/private"]}`, 400, 2, []string{"https://8.8.8.8/audit"}},
} {
rec := httptest.NewRecorder()
NewAuditHandlers().HandleUpdateWebhooks(rec, httptest.NewRequest(method, "/api/admin/webhooks/audit", strings.NewReader(tc.body)))
if rec.Code != tc.status || logger.updates != tc.updates || !reflect.DeepEqual(logger.urls, tc.urls) {
t.Fatalf("status/updates/urls = %d/%d/%v", rec.Code, logger.updates, logger.urls)
}
}
rec := httptest.NewRecorder()
NewAuditHandlers().HandleUpdateWebhooks(rec, httptest.NewRequest(http.MethodGet, "/api/admin/webhooks/audit", strings.NewReader(`{"urls":[]}`)))
if rec.Code != 405 || logger.updates != 2 {
t.Fatal("safe method mutated selection")
}
})
}
}
// Tenant selection still comes only from the authenticated request context.
type auditSelectionFactory map[string]*auditSelectionLogger
func (f auditSelectionFactory) CreateLogger(path string) (audit.Logger, error) {
for org, l := range f {
if strings.HasSuffix(path, "/"+org+"/audit.db") {
return l, nil
}
}
return nil, context.Canceled
}
func TestAuditSelectionTenantIsolation(t *testing.T) {
previous := GetTenantAuditManager()
factory := auditSelectionFactory{"org-a": newAuditSelectionLogger(), "org-b": newAuditSelectionLogger()}
for org, l := range factory {
l.urls = []string{"https://8.8.8.8/" + org}
l.events[0].ID = org
}
manager := audit.NewTenantLoggerManager(t.TempDir(), factory)
SetTenantAuditManager(manager)
t.Cleanup(func() { manager.Close(); SetTenantAuditManager(previous) })
for _, org := range []string{"org-a", "org-b"} {
rec := httptest.NewRecorder()
req := httptest.NewRequest(http.MethodGet, "/api/audit?user=operator%2B%C3%A9%40example.invalid&event=login&startTime=2026-01-01T00:00:00Z", nil)
req = req.WithContext(context.WithValue(req.Context(), OrgIDContextKey, org))
NewAuditHandlers().HandleListAuditEvents(rec, req)
other := "org-a"
if org == other {
other = "org-b"
}
if rec.Code != 200 || !strings.Contains(rec.Body.String(), org) || strings.Contains(rec.Body.String(), other) {
t.Fatalf("tenant list: %d/%s", rec.Code, rec.Body.String())
}
req = httptest.NewRequest(http.MethodPut, "/api/admin/webhooks/audit", strings.NewReader(`{"urls":null}`))
req = req.WithContext(context.WithValue(req.Context(), OrgIDContextKey, org))
rec = httptest.NewRecorder()
NewAuditHandlers().HandleUpdateWebhooks(rec, req)
if rec.Code != 400 || factory[org].updates != 0 || factory[other].updates != 0 {
t.Fatal("absent selection mutated tenant receivers")
}
req = httptest.NewRequest(http.MethodPut, "/api/admin/webhooks/audit", strings.NewReader(`{"urls":[]}`))
req = req.WithContext(context.WithValue(req.Context(), OrgIDContextKey, org))
rec = httptest.NewRecorder()
before := factory[other].updates
NewAuditHandlers().HandleUpdateWebhooks(rec, req)
if rec.Code != 204 || len(factory[org].urls) != 0 || factory[other].updates != before {
t.Fatal("explicit clear crossed tenant boundary")
}
}
}