Repository navigation
Preserve control exclusions in scan requests - #194
Conversation
Signed-off-by: Harsh K. Singh <harshsinghrajpoot07@gmail.com>
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configuration
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
matthyx
left a comment
There was a problem hiding this comment.
Approve the shared request-contract change at ddffc009208e09b2b1b3117b9d8f6b35f3eb688f, targeting main at 2e68be120cec82739a947eaf453e2e7f1b2a0fdc.
The change is needed: current main silently loses excludeControls during PostScanRequest JSON decoding/encoding. The new preservation test fails against that exact base and passes on this head. Operator #429 needs this released contract; its existing dependency blocker supports the need rather than blocking this standalone library addition. A custom framework is a workaround, not transport support for the requested setting.
No blocking findings in the three changed files. Two independent static reviews found no correctness/security/maintenance blocker; architectural status is CLEAR. httpserver/meta/v1/datastructure.go:49 follows the adjacent optional namespace-list pattern, fixes the loss at the schema boundary, and adds no dependency or evaluation logic. Tests cover nonempty round trips and omitted/empty/null lists. Old empty requests retain their JSON shape; older consumers cannot apply the new field. The README correctly qualifies behavior as requiring a supporting scanner. This approval does not establish scanner filtering/scoring or installed operator behavior; those belong to their consumer changes.
History: merged #146 / #145 added posture exceptions, a different contract; merged #138, #141, and #127 added other scan-request capabilities. #127's earlier object-interface concern was addressed and approved; it is unrelated to this scalar list. No relevant explicitly rejected exclusion approach or duplicate/superseding fix was found. Indexed searches across open/closed/merged PRs and issues used exclude, excludeControls, exclude-controls, control exclusions, skip controls, excluded controls, PostScanRequest, datastructure.go, and scan request, capped at 50–100 results/query. These bounded searches are not proof of absence.
Validation in an isolated credential-free container with read-only source, no privileged access or host-sensitive mounts: go test -p 2 ./httpserver/meta/v1 -run TestPostScanRequest -count=1 passed; go vet -p 2 ./httpserver/meta/v1 passed. Replacing only the request-definition file with the exact base version made the same preservation test fail with the exclusion field absent. git diff --check and independent gofmt checks passed. Initial offline dependency setup and a 2-GiB temporary-storage build failed for infrastructure reasons; the successful retry resolved both. No full-suite, full-build, or end-to-end execution is claimed. Head DCO/security checks pass; CodeRabbit's success represents a skipped review, and no Go CI check is visible.
Open/non-draft status, conflict-free mergeability, unchanged head/base, and absence of new reviews/inline threads were rechecked immediately before submission. Main protection requires one approval and last-push approval, with no required status contexts. This review supplies approval for the reviewed commit; no merge is performed.
Verdict: approve this bounded library change; no blockers found.
Operator scans currently cannot carry the CLI’s
--exclude-controlschoice because the shared scan request has no field for it. This change adds an optionalexcludeControlsfield and documents its JSON format.This is the shared-library change needed by operator PR #429, as pointed out during review. It lets requests carry the list of controls to exclude. Applying those exclusions in the scanner and configuring them in the operator are separate changes.
Tests verify that a nonempty list survives JSON encoding and decoding, and that omitted, empty, and null lists do not add the field to outgoing requests. The full
go test -p 2 ./...suite passed on the Linux test VM.This change is ready for review independently of the operator and scanner changes. Once it is merged and included in a release, those projects can update their dependencies and repeat testing without the temporary local library copies used during development.