diff --git a/.secrets.baseline b/.secrets.baseline index d56b0f10..1eb65fd2 100644 --- a/.secrets.baseline +++ b/.secrets.baseline @@ -549,14 +549,14 @@ "filename": "internal/server/api/server.gen.go", "hashed_secret": "9fd0aaae1a3d0bc789d081307161ea9a821f9dee", "is_verified": false, - "line_number": 4595 + "line_number": 4596 }, { "type": "Secret Keyword", "filename": "internal/server/api/server.gen.go", "hashed_secret": "eca525ee60b3564d9633eb140726685271d52341", "is_verified": false, - "line_number": 4733 + "line_number": 4734 } ], "internal/server/api_scans_test.go": [ @@ -809,5 +809,5 @@ } ] }, - "generated_at": "2026-09-22T00:14:21Z" + "generated_at": "2026-09-22T01:51:28Z" } diff --git a/api/openapi.yaml b/api/openapi.yaml index 3c58e7fe..0595de02 100644 --- a/api/openapi.yaml +++ b/api/openapi.yaml @@ -1338,6 +1338,9 @@ paths: - name: action in: query schema: {type: string} + - name: correlation_id + in: query + schema: {type: string} - name: actor_type in: query schema: {type: string} @@ -1354,6 +1357,11 @@ paths: in: query schema: {type: string, format: date-time} responses: + '400': + description: A query parameter the export does not declare (request.unknown_parameter), or one that fails to parse. + content: + application/json: + schema: {$ref: '#/components/schemas/ErrorEnvelope'} '200': description: Audit export file (CSV or JSON attachment) content: diff --git a/docs/guides/API_GUIDE.md b/docs/guides/API_GUIDE.md index 1703cef5..c23d4522 100644 --- a/docs/guides/API_GUIDE.md +++ b/docs/guides/API_GUIDE.md @@ -366,13 +366,12 @@ List query parameters: `action`, `correlation_id`, `actor_type`, and `limit` (1 to 200, default 50). Each page carries `next_cursor`; pass it as the next request's `cursor`. -The export takes `action`, `actor_type`, `resource_type`, `resource_id`, -`since` and `until`, returns the whole filtered set newest first, and stops -at 10,000 rows; a capped export carries an `X-OpenWatch-Export-Truncated` -header. Today the export does not accept `correlation_id`, and a query -parameter it does not recognize is ignored rather than rejected, so a -misspelled or unsupported filter widens the export to everything the caller -may see. CP `bugs/OW-064` tracks both. +The export takes the same seven filters, returns the whole filtered set +newest first, and stops at 10,000 rows; a capped export carries an +`X-OpenWatch-Export-Truncated` header. A query parameter the export does +not declare is refused with `400` `request.unknown_parameter` naming it, so +a misspelled filter cannot silently widen an export you will file; the list +endpoint ignores unknown parameters as before. --- diff --git a/frontend/src/api/schema.d.ts b/frontend/src/api/schema.d.ts index e0510903..356b32c2 100644 --- a/frontend/src/api/schema.d.ts +++ b/frontend/src/api/schema.d.ts @@ -6784,6 +6784,7 @@ export interface operations { /** @description Output format. Defaults to csv. */ format?: "csv" | "json"; action?: string; + correlation_id?: string; actor_type?: string; resource_type?: string; resource_id?: string; @@ -6806,6 +6807,15 @@ export interface operations { "application/json": components["schemas"]["AuditEvent"][]; }; }; + /** @description A query parameter the export does not declare (request.unknown_parameter), or one that fails to parse. */ + 400: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["ErrorEnvelope"]; + }; + }; /** @description Caller is not authenticated */ 401: { headers: { diff --git a/internal/server/api/server.gen.go b/internal/server/api/server.gen.go index 94ae82c2..c61d76cf 100644 --- a/internal/server/api/server.gen.go +++ b/internal/server/api/server.gen.go @@ -4350,13 +4350,14 @@ type GetAuditEventsParams struct { // GetAuditEventsExportParams defines parameters for GetAuditEventsExport. type GetAuditEventsExportParams struct { // Format Output format. Defaults to csv. - Format *GetAuditEventsExportParamsFormat `form:"format,omitempty" json:"format,omitempty"` - Action *string `form:"action,omitempty" json:"action,omitempty"` - ActorType *string `form:"actor_type,omitempty" json:"actor_type,omitempty"` - ResourceType *string `form:"resource_type,omitempty" json:"resource_type,omitempty"` - ResourceId *string `form:"resource_id,omitempty" json:"resource_id,omitempty"` - Since *time.Time `form:"since,omitempty" json:"since,omitempty"` - Until *time.Time `form:"until,omitempty" json:"until,omitempty"` + Format *GetAuditEventsExportParamsFormat `form:"format,omitempty" json:"format,omitempty"` + Action *string `form:"action,omitempty" json:"action,omitempty"` + CorrelationId *string `form:"correlation_id,omitempty" json:"correlation_id,omitempty"` + ActorType *string `form:"actor_type,omitempty" json:"actor_type,omitempty"` + ResourceType *string `form:"resource_type,omitempty" json:"resource_type,omitempty"` + ResourceId *string `form:"resource_id,omitempty" json:"resource_id,omitempty"` + Since *time.Time `form:"since,omitempty" json:"since,omitempty"` + Until *time.Time `form:"until,omitempty" json:"until,omitempty"` } // GetAuditEventsExportParamsFormat defines parameters for GetAuditEventsExport. @@ -6732,6 +6733,19 @@ func (siw *ServerInterfaceWrapper) GetAuditEventsExport(w http.ResponseWriter, r return } + // ------------- Optional query parameter "correlation_id" ------------- + + err = runtime.BindQueryParameterWithOptions("form", true, false, "correlation_id", r.URL.Query(), ¶ms.CorrelationId, runtime.BindQueryParameterOptions{Type: "string", Format: ""}) + if err != nil { + var requiredError *runtime.RequiredParameterError + if errors.As(err, &requiredError) { + siw.ErrorHandlerFunc(w, r, &RequiredParamError{ParamName: "correlation_id"}) + } else { + siw.ErrorHandlerFunc(w, r, &InvalidParamFormatError{ParamName: "correlation_id", Err: err}) + } + return + } + // ------------- Optional query parameter "actor_type" ------------- err = runtime.BindQueryParameterWithOptions("form", true, false, "actor_type", r.URL.Query(), ¶ms.ActorType, runtime.BindQueryParameterOptions{Type: "string", Format: ""}) diff --git a/internal/server/api_audit_query_test.go b/internal/server/api_audit_query_test.go index 95b9ef7d..27838257 100644 --- a/internal/server/api_audit_query_test.go +++ b/internal/server/api_audit_query_test.go @@ -10,6 +10,8 @@ import ( "io" "net/http" neturl "net/url" + "os" + "path/filepath" "strings" "testing" "time" @@ -647,3 +649,126 @@ func TestAPI_AuditEvents_ExportRequiresAuditExport(t *testing.T) { } }) } + +// @ac AC-17 +// api-audit-events-query/AC-17 (v1.5.0): the export takes every filter the +// list takes, correlation_id included, and refuses a filter it does not +// declare instead of silently exporting everything. The list endpoint keeps +// its lenient behavior, so the strictness is confined to the export. +func TestAPI_AuditEvents_ExportFilterParityAndNoSilentWidening(t *testing.T) { + t.Run("api-audit-events-query/AC-17", func(t *testing.T) { + url, pool := freshAPIServer(t) + ctx := context.Background() + seed := func(corr string) { + t.Helper() + id := uuid.Must(uuid.NewV7()) + if _, err := pool.Exec(ctx, + `INSERT INTO audit_events + (id, correlation_id, actor_type, actor_label, action, severity, occurred_at) + VALUES ($1,$2,'user','alice@example.com','host.created','info',now())`, + id, corr); err != nil { + t.Fatalf("seed audit event: %v", err) + } + } + seed("corr-a") + seed("corr-a") + seed("corr-b") + + // correlation_id narrows the export in both formats. + resp := doReq(t, asRole(t, "GET", url+"/api/v1/audit/events/export?format=json&correlation_id=corr-a", auth.RoleAuditor, nil)) + if resp.StatusCode != http.StatusOK { + t.Fatalf("json export status = %d, want 200", resp.StatusCode) + } + var events []map[string]any + if err := json.NewDecoder(resp.Body).Decode(&events); err != nil { + t.Fatalf("decode json export: %v", err) + } + resp.Body.Close() + if len(events) != 2 { + t.Fatalf("json export rows = %d, want 2 (corr-a only)", len(events)) + } + for _, ev := range events { + if ev["correlation_id"] != "corr-a" { + t.Errorf("json export leaked correlation_id %v", ev["correlation_id"]) + } + } + resp = doReq(t, asRole(t, "GET", url+"/api/v1/audit/events/export?correlation_id=corr-b", auth.RoleAuditor, nil)) + raw, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("csv export status = %d, want 200", resp.StatusCode) + } + if lines := strings.Count(strings.TrimSpace(string(raw)), "\n"); lines != 1 { + t.Errorf("csv export data rows = %d, want 1 (corr-b only); body=%q", lines, raw) + } + + // A misspelled filter is refused, and nothing is exported. + resp = doReq(t, asRole(t, "GET", url+"/api/v1/audit/events/export?correlation_id=corr-a&actr_type=user", auth.RoleAuditor, nil)) + raw, _ = io.ReadAll(resp.Body) + resp.Body.Close() + if resp.StatusCode != http.StatusBadRequest { + t.Fatalf("misspelled filter status = %d, want 400; body=%q", resp.StatusCode, raw) + } + if resp.Header.Get("Content-Disposition") != "" { + t.Errorf("a refused export must not set Content-Disposition") + } + var env struct { + Error struct { + Code string `json:"code"` + HumanMessage string `json:"human_message"` + } `json:"error"` + } + if err := json.Unmarshal(raw, &env); err != nil { + t.Fatalf("400 body is not the envelope: %v; body=%q", err, raw) + } + if env.Error.Code != "request.unknown_parameter" || !strings.Contains(env.Error.HumanMessage, "actr_type") { + t.Errorf("envelope = %+v, want request.unknown_parameter naming actr_type", env.Error) + } + + // The list endpoint keeps ignoring an unknown parameter. + resp = doReq(t, asRole(t, "GET", url+"/api/v1/audit/events?actr_type=user", auth.RoleAuditor, nil)) + resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Errorf("list with unknown parameter status = %d, want 200 (lenient)", resp.StatusCode) + } + }) +} + +// auditExportParams must equal the query parameters getAuditEventsExport +// declares, or the unknown-parameter guard would reject a declared filter +// or admit an undeclared one. Read from the contract, not remembered. +func TestAPI_AuditEvents_ExportParamGuardMatchesContract(t *testing.T) { + t.Run("api-audit-events-query/AC-17", func(t *testing.T) { + raw, err := os.ReadFile(filepath.Join("..", "..", "api", "openapi.yaml")) + if err != nil { + t.Fatal(err) + } + doc := string(raw) + start := strings.Index(doc, " /api/v1/audit/events/export:") + if start < 0 { + t.Fatal("export path not found in api/openapi.yaml") + } + end := strings.Index(doc[start:], " responses:") + block := doc[start : start+end] + declared := map[string]struct{}{} + for _, line := range strings.Split(block, "\n") { + line = strings.TrimSpace(line) + if strings.HasPrefix(line, "- name: ") { + declared[strings.TrimPrefix(line, "- name: ")] = struct{}{} + } + } + if len(declared) == 0 { + t.Fatal("no query parameters parsed from the export operation") + } + for name := range declared { + if _, ok := auditExportParams[name]; !ok { + t.Errorf("contract declares %q but auditExportParams would reject it", name) + } + } + for name := range auditExportParams { + if _, ok := declared[name]; !ok { + t.Errorf("auditExportParams admits %q which the contract does not declare", name) + } + } + }) +} diff --git a/internal/server/audit_export_handler.go b/internal/server/audit_export_handler.go index 61ddee61..f026d227 100644 --- a/internal/server/audit_export_handler.go +++ b/internal/server/audit_export_handler.go @@ -12,6 +12,8 @@ import ( "fmt" "log/slog" "net/http" + "net/url" + "strings" "time" "github.com/Hanalyx/openwatch/internal/auth" @@ -23,6 +25,35 @@ import ( // cap is logged + flagged (X-OpenWatch-Export-Truncated) so a truncated export is never silently mistaken for "all". const auditExportCap = 10000 +// auditExportParams is the query surface the export declares in +// api/openapi.yaml, one entry per parameter of getAuditEventsExport. The +// contract-coverage test keeps it equal to the declaration. +var auditExportParams = map[string]struct{}{ + "format": {}, "action": {}, "correlation_id": {}, "actor_type": {}, + "resource_type": {}, "resource_id": {}, "since": {}, "until": {}, +} + +// firstUnknownQueryParam returns the first query key not in allowed, in +// the request's own order, or "" when every key is declared. +func firstUnknownQueryParam(r *http.Request, allowed map[string]struct{}) string { + for _, pair := range strings.Split(r.URL.RawQuery, "&") { + if pair == "" { + continue + } + key := pair + if i := strings.IndexByte(pair, '='); i >= 0 { + key = pair[:i] + } + if unescaped, err := url.QueryUnescape(key); err == nil { + key = unescaped + } + if _, ok := allowed[key]; !ok { + return key + } + } + return "" +} + // GetAuditEventsExport streams the filtered audit events as a downloadable // CSV (default) or JSON file. audit:export gated, independently of the // audit:read list (v1.4.0; audit:read through 1.3.1, which let every reader @@ -32,14 +63,27 @@ func (h *handlers) GetAuditEventsExport(w http.ResponseWriter, r *http.Request, return } + // A filter the export does not declare is rejected, never ignored. The + // generated router drops unknown query parameters silently, and for + // this route that turns a misspelled filter into an export of the + // whole trail that the caller files as if it were the narrow one. The + // list endpoint stays lenient; the strictness is this route's alone + // (v1.5.0, CP bugs/OW-064). + if unknown := firstUnknownQueryParam(r, auditExportParams); unknown != "" { + writeError(w, http.StatusBadRequest, "request.unknown_parameter", "client", + "the export does not accept the "+unknown+" parameter", false) + return + } + // Reuse the list query with the same filters at the export cap. lp := api.GetAuditEventsParams{ - Action: params.Action, - ActorType: params.ActorType, - ResourceType: params.ResourceType, - ResourceId: params.ResourceId, - Since: params.Since, - Until: params.Until, + Action: params.Action, + CorrelationId: params.CorrelationId, + ActorType: params.ActorType, + ResourceType: params.ResourceType, + ResourceId: params.ResourceId, + Since: params.Since, + Until: params.Until, } rows, err := h.queryEvents(r.Context(), lp, auditExportCap) if err != nil { diff --git a/specs/api/audit-events-query.spec.yaml b/specs/api/audit-events-query.spec.yaml index 9e1ab615..ea4510f6 100644 --- a/specs/api/audit-events-query.spec.yaml +++ b/specs/api/audit-events-query.spec.yaml @@ -8,7 +8,7 @@ spec: # bugs/OW-056, reproduced 2026-09-19). Founder decision 2026-09-19: the # registry is the intent. viewer and ops_lead lose the export route and # keep the list. AC-16 is new; C-08 and AC-13 say audit:export. - version: "1.4.0" + version: "1.5.0" status: approved tier: 2 @@ -63,7 +63,7 @@ spec: type: technical enforcement: error - id: C-08 - description: 'v1.3.0 — GET /api/v1/audit/events/export MUST stream the filtered audit trail as a downloadable attachment (NIST 800-53 AU-7). It is audit:export gated (v1.4.0; audit:read through 1.3.1), independent of the list endpoint which stays audit:read, accepts the same filters (action, actor_type, resource_type, resource_id, since, until), and returns the whole filtered set newest-first capped at 10000 rows (not one page). format=csv (default) emits a header row + one row per event with columns occurred_at, action, message, severity, actor_type, actor_label, actor_id, resource_type, resource_id, correlation_id; format=json emits the AuditEvent array. Both set Content-Disposition: attachment with a timestamped filename. The message column reuses activity.FormatAudit (same as the list). v1.3.1 hardening: (a) every CSV cell MUST be neutralized against spreadsheet formula injection (CWE-1236) — a value whose first character is =, +, -, @, tab, or CR is prefixed with a single quote so it renders as literal text, not an executed formula; (b) a truncated export (row count == the 10000 cap) MUST set an X-OpenWatch-Export-Truncated response header and log a warning, so a capped export is never silently mistaken for the complete trail' + description: 'v1.3.0 — GET /api/v1/audit/events/export MUST stream the filtered audit trail as a downloadable attachment (NIST 800-53 AU-7). It is audit:export gated (v1.4.0; audit:read through 1.3.1), independent of the list endpoint which stays audit:read, accepts the same filters as the list (action, correlation_id, actor_type, resource_type, resource_id, since, until; correlation_id added in v1.5.0, CP bugs/OW-064), rejects a query parameter it does not declare with 400 request.unknown_parameter naming the parameter (v1.5.0; a misspelled filter must fail rather than silently widen an artifact someone will file, and this strictness is specific to the export route by founder decision, not a server-wide policy), and returns the whole filtered set newest-first capped at 10000 rows (not one page). format=csv (default) emits a header row + one row per event with columns occurred_at, action, message, severity, actor_type, actor_label, actor_id, resource_type, resource_id, correlation_id; format=json emits the AuditEvent array. Both set Content-Disposition: attachment with a timestamped filename. The message column reuses activity.FormatAudit (same as the list). v1.3.1 hardening: (a) every CSV cell MUST be neutralized against spreadsheet formula injection (CWE-1236) — a value whose first character is =, +, -, @, tab, or CR is prefixed with a single quote so it renders as literal text, not an executed formula; (b) a truncated export (row count == the 10000 cap) MUST set an X-OpenWatch-Export-Truncated response header and log a warning, so a capped export is never silently mistaken for the complete trail' type: technical enforcement: error @@ -134,3 +134,16 @@ spec: constant. priority: critical references_constraints: [C-08] + - id: AC-17 + description: > + v1.5.0 — Export filter parity and no silent widening. With events + seeded under two correlation ids, GET + /api/v1/audit/events/export?correlation_id= returns only the + rows for in both csv and json; GET + /api/v1/audit/events/export?correlation_id=&actr_type=user (a + misspelled filter) returns 400 request.unknown_parameter naming + actr_type and exports nothing; the same misspelling on GET + /api/v1/audit/events keeps the list endpoint's lenient behavior + (200), so the strictness is measurably confined to the export. + priority: high + references_constraints: [C-08]