fix(audit): export takes correlation_id and refuses a filter it does not declare - #872
Merged
Merged
Conversation
…not declare The export accepted six of the list's seven filters; correlation_id was missing from the contract and the handler. Because the generated router drops undeclared query parameters, an export narrowed by correlation_id (or any misspelled filter) returned the caller's whole trail up to the 10,000-row cap with no signal, and an incident package built from it would be wrong without anyone noticing (CP bugs/OW-064). Contract first: api-audit-events-query 1.5.0 amends C-08 (the export accepts the same filters as the list, correlation_id included, and rejects an undeclared query parameter with 400 request.unknown_parameter naming it; the strictness is confined to the export route by founder decision) and adds AC-17. The export operation in api/openapi.yaml declares correlation_id and the 400 response; generated code follows. The handler copies correlation_id into the list query and checks the raw query string against the set of declared parameters before running anything, in the request's own order, so the first unknown key is named. The list endpoint is unchanged and stays lenient. Tests: AC-17 seeds events under two correlation ids and asserts the narrowed export in both formats, the 400 envelope for a misspelled filter with no Content-Disposition, and the list's 200 for the same misspelling. A second test reads the export operation from api/openapi.yaml and requires the guard's set to equal the declared parameters, so the two cannot drift; removing correlation_id from the guard turns both tests red. The API guide's audit section documents the export beside the list. The detect-secrets baseline is the hook's own line-number refresh. CP: bugs/doing/OW-064
github-actions
Bot
added
documentation
A blank line between the two rows split the table, so the export row would have rendered as loose text.
The audit section keeps one export table row (both sides carried the same joined row) and takes this branch's export paragraph, which describes the implemented behavior, over #870's temporary limitation paragraph.
Merging main kept main's baseline; this branch's regenerated server.gen.go shifts two findings by one line (4595 to 4596, 4733 to 4734). Rescanned with the pinned detect-secrets 1.5.0: 83 fingerprints, none added or removed, two locations corrected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CP
bugs/doing/OW-064. Contract first.Contract.
api-audit-events-query1.5.0: C-08 now says the export accepts the same filters as the list (correlation_idadded) and rejects an undeclared query parameter with400request.unknown_parameternaming it; the strictness is confined to the export route by founder decision and is not a server-wide policy. AC-17 covers parity, the refusal, and the list's unchanged leniency.api/openapi.yaml:getAuditEventsExportdeclarescorrelation_idand a400envelope response; generated Go and TypeScript follow.Code.
audit_export_handler.gocopiescorrelation_idinto the list query and, before any work, checks the raw query string againstauditExportParams(the declared set) in request order, answeringrequest.unknown_parameterwith the first unknown key.GET /api/v1/audit/eventsis unchanged.Tests.
TestAPI_AuditEvents_ExportFilterParityAndNoSilentWidening(AC-17): two correlation ids seeded;correlation_id=corr-ayields exactly those rows in JSON and CSV;actr_type=user(misspelled) yields400 request.unknown_parameternamingactr_typewith noContent-Disposition; the same misspelling on the list returns200.TestAPI_AuditEvents_ExportParamGuardMatchesContract(AC-17): reads the export operation fromapi/openapi.yamland requires the guard's set to equal the declared parameters, so they cannot drift. Mutation check: droppingcorrelation_idfrom the guard turns both tests red.Guide. The audit section documents the export beside the list with its seven filters, the cap, the truncation header and the refusal rule. Merge after #870 (OW-065); this paragraph supersedes its "today the export drops correlation_id" sentence, and I will refresh on conflict.
Also in this PR: one format-only commit (
8e65349b) for three frontend files that drifted onmainearlier (24057cb3,1507b799). The pre-commit prettier hook checks the whole frontend directory, so any PR that regeneratesschema.d.tsis blocked by that drift; no CI job runs the check. No logic changes in those files.Checks
go test ./internal/server/ -run TestAPI_AuditEventsgreen (dedicated test DB);make spec-check121/121, 100%;make check-generatedin sync; doc style clean; pre-commit and pre-push hooks passed.Candidate impact
v0.8.0-rc.5is immutable and carries the old behavior; nothing here changes it.