feat(security): derive themed, deduped backlog entries from live Kubescape findings - #2876
Conversation
…scape findings Report-only first slice of the live-finding ingestion bridge (#2854). Groups findings into themes (one per failed posture control, one per CVE severity class) rather than per resource, and gives each a fingerprint that excludes counts and timestamps so unchanged state never re-files. Fails closed on Kubescape's spec-stripped LIST shape: measured on prod, one workload reads critical=0 via LIST and critical=18 via a per-object GET, so a LIST-fed bridge would report a spotless cluster forever. Writes stay off behind -mode; both states are tested.
… input Self-review findings. A control reported at different severities by different workloads took whichever the map yielded first, so the field depended on iteration order in a struct whose whole contract is determinism; it now keeps the highest, and the report surfaces it. An invocation with no input reported "nothing to file", which is the same false all-clear the stripped-LIST guard exists to prevent.
Exercised as a user against the live prod cluster at
Ablation — each guarantee was removed in place and the covering test confirmed to go red: component sorting (fingerprint stability), the stripped-LIST guard, the failed-only control filter, severity-count independence, the highest-severity rule, and the empty-input guard. Restored green after each. Self-review found and fixed two defects before requesting review: theme severity depended on map iteration order, and an empty invocation reported "nothing to file". |
✅
|
@coderabbitai review |
|
✅ Action performedReview finished.
|
📝 WalkthroughWalkthroughAdds the report-only Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/kubescape-backlog-bridge/main.go`:
- Around line 407-417: The readList function must reject JSON documents that are
not list-shaped before returning l.Items. Validate that the top-level input
contains an items field represented as an array, return a parse/validation error
otherwise, and add a test covering a bare single-object JSON document to ensure
it cannot produce zero findings with a successful exit.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 992a69cf-a547-45ed-bff8-2dcb9c955807
📒 Files selected for processing (3)
.github/workflows/ci.yamlscripts/kubescape-backlog-bridge/main.goscripts/kubescape-backlog-bridge/main_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
.github/workflows/*.yaml
📄 CodeRabbit inference engine (AGENTS.md)
CI for Kubernetes-related changes must remain static: run validation and Kubescape scanning without starting a cluster.
Files:
.github/workflows/ci.yaml
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Use Conventional Commit messages and titles, keep changes small and focused, and create pull requests as drafts.
Land new behavior disabled, validate it, then enable it using the appropriate layer: OpenFeature for per-request flags, Flagger for traffic shifting, Helm values/Kustomize for component toggles, and Kubernetes feature gates for platform behavior.
Files:
scripts/kubescape-backlog-bridge/main.goscripts/kubescape-backlog-bridge/main_test.go
🧠 Learnings (2)
📚 Learning: 2026-07-28T00:33:10.828Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 2825
File: .github/workflows/dr-rebuild.yaml:70-73
Timestamp: 2026-07-28T00:33:10.828Z
Learning: For GitHub Actions concurrency configuration in this repo, don’t use `queue: single` on a shared production concurrency group (e.g., `prod-deploy`) that is used for normal production deployments—`queue: single` will cancel/replace any pending run in that group. Normal production deployments must use a uniform `queue: max` for that shared group to ensure serialization without coalescing. If you need DR rebuild coalescing (replacement of pending DR rebuild runs), it must use a separate DR-only concurrency scope/group (not the shared production `prod-deploy` group).
Applied to files:
.github/workflows/ci.yaml
📚 Learning: 2026-07-29T07:29:34.804Z
Learnt from: devantler
Repo: devantler-tech/platform PR: 2848
File: scripts/validate-publication-order/main.go:315-353
Timestamp: 2026-07-29T07:29:34.804Z
Learning: When reviewing GitHub Actions workflow steps with `run:` blocks, remember that the default non-interactive Bash invocation won’t expand shell aliases unless the script explicitly enables it (e.g., `shopt -s expand_aliases`). Therefore, when assessing potential command-name shadowing (e.g., whether an alias could change what command runs), only treat alias-based behavior as possible if the step explicitly opts into alias expansion via `shopt -s expand_aliases` (or equivalent).
Applied to files:
.github/workflows/ci.yaml
🪛 golangci-lint (2.12.2)
scripts/kubescape-backlog-bridge/main.go
[medium] 408-408: G304: Potential file inclusion via variable
(gosec)
scripts/kubescape-backlog-bridge/main_test.go
[high] 100-100: G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand)
(gosec)
🔇 Additional comments (3)
scripts/kubescape-backlog-bridge/main.go (1)
1-119: LGTM!Also applies to: 124-406, 418-434
scripts/kubescape-backlog-bridge/main_test.go (1)
1-329: LGTM!.github/workflows/ci.yaml (1)
81-81: LGTM!Also applies to: 439-447
… it clean A per-object `kubectl get <crd> <name> -n <ns> -o json` — the form this command's own documentation asks for — emits a bare object with no "items" key. Decoding it into a list left Items nil, so a document carrying critical=18 printed "nothing to file" and exited 0. That is the same false all-clear the stripped-LIST guard exists to prevent, reached by following the instructions. The document shape is now determined explicitly: a list, a single object, or a hard error.
@coderabbitai review |
|
✅ Action performedReview finished.
|
CodeRabbit rate-limited this request rather than reviewing it. The acknowledgement at 15:00:38Z Worth recording because the freshness signal lies here — the summary comment's A stated retry window still counts as not-delivering, so recording CodeRabbit as no-gate at this |
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c56f5b98c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codex round at
|
The bridge's success path and its genuinely-empty path were indistinguishable, so every input mistake landed in exit 0 "nothing to file" — a false all-clear on live security findings. Five review findings were instances of that one root cause or of gaps around it. Measured against prod: a stripped LIST differs from a real per-object GET by SHAPE, not by size. 2215/2215 posture LIST objects carry `controls: null` while a genuinely compliant GET still carries its control map; 121/121 CVE LIST objects blank `vulnerabilitiesRef` while a genuinely clean image still names its manifest. So "examined and empty" is separable from "never examined" at any input size, and the size heuristic (which held only on a large cluster) is gone. The same markers identify which surface a document belongs to, so a CVE file passed to -posture is rejected instead of deriving no controls and exiting 0. Also: the surface flags are repeatable, because a cluster-wide run produces one per-object GET per workload and a single-valued flag made the promised grouping unreachable; CVE counts are summed and rendered (still never fingerprinted) so an update can show 1 critical becoming 999; and declared ClusterSecurityExceptions are applied before deriving themes, filtered PER-COMPONENT so a kind/name-scoped exception cannot silence a workload it does not name. Exceptions are read from the artifact scripts/generate-kubescape-exceptions already produces, so the CI scan and this bridge cannot drift apart in what they consider accepted.
readList decodes the two document shapes explicitly, and the tests build fixtures as raw JSON, so nothing references it. golangci-lint's unused linter flagged it; the compiler does not.
revive requires one per non-test file, and this is the repo's only multi-file Go package, so the rule had never fired before. Putting the comment on exceptions.go alone displaced the command's own documentation: Go concatenates package comments in filename order, so `go doc` led with the exceptions blurb. The package documentation now lives in doc.go, which is both idiomatic and deterministically first.
@coderabbitai review |
|
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/kubescape-backlog-bridge/main_test.go`:
- Line 190: Annotate the deterministic rand.New initialization in the test with
the project’s standard inline gosec G404 suppression comment, preserving the
fixed seed and reproducible shuffle behavior.
In `@scripts/kubescape-backlog-bridge/main.go`:
- Around line 585-592: Update the items parsing logic around probe["items"] to
reject an explicit JSON null before unmarshalling, while continuing to accept an
explicit empty array as valid. Return the existing malformed-items error for
null values, and preserve the current json.Unmarshal handling for non-null
arrays.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3c2cae02-762d-4a8c-9c87-9d83fed54b72
📒 Files selected for processing (5)
scripts/kubescape-backlog-bridge/doc.goscripts/kubescape-backlog-bridge/exceptions.goscripts/kubescape-backlog-bridge/exceptions_test.goscripts/kubescape-backlog-bridge/main.goscripts/kubescape-backlog-bridge/main_test.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
**/*: Use Conventional Commit messages and titles, keep changes small and focused, and create pull requests as drafts.
Land new behavior disabled, validate it, then enable it using the appropriate layer: OpenFeature for per-request flags, Flagger for traffic shifting, Helm values/Kustomize for component toggles, and Kubernetes feature gates for platform behavior.
Files:
scripts/kubescape-backlog-bridge/exceptions_test.goscripts/kubescape-backlog-bridge/doc.goscripts/kubescape-backlog-bridge/exceptions.goscripts/kubescape-backlog-bridge/main.goscripts/kubescape-backlog-bridge/main_test.go
🪛 golangci-lint (2.12.2)
scripts/kubescape-backlog-bridge/main_test.go
[high] 190-190: G404: Use of weak random number generator (math/rand or math/rand/v2 instead of crypto/rand)
(gosec)
🔇 Additional comments (6)
scripts/kubescape-backlog-bridge/doc.go (1)
1-72: LGTM!scripts/kubescape-backlog-bridge/main.go (1)
25-121: LGTM!Also applies to: 124-167, 169-229, 231-287, 289-297, 330-437, 439-457, 459-533, 535-558, 594-608, 610-629
scripts/kubescape-backlog-bridge/main_test.go (1)
11-143: LGTM!Also applies to: 145-201, 205-273, 275-290, 292-351, 353-368, 372-397, 401-424, 426-452, 454-513, 515-552, 554-595
scripts/kubescape-backlog-bridge/exceptions.go (2)
1-56: LGTM!Also applies to: 84-131, 133-206
57-82: 🗄️ Data Integrity & IntegrationNo change needed for designator attributes.
resolveMatchonly emits Attributes designators withnamespacefromnamespaceSelectororkind/namefromresources, somatchescovers the current generator output.scripts/kubescape-backlog-bridge/exceptions_test.go (1)
1-50: LGTM!Also applies to: 52-134, 136-173, 175-230, 232-263
… result
It decodes into a nil slice WITHOUT error, so every guard passed
vacuously and the command reported a clean cluster at exit 0 — the same
false all-clear as the stripped skeleton, reached by a different shape.
Reproduced on the previous head: `{"items":null}` printed
"nothing to file" and exited 0 for both surfaces.
`items: []` stays a legitimate pass; that distinction is the point.
@coderabbitai review |
|
✅ Action performedReview finished.
|
Lane state at
|
@codex review |
@coderabbitai review |
|
✅ Action performedReview finished.
|
CodeRabbit did not deliver at this head — rate limit, not a verdict. Its summary comment was CI is green at this head (25/25, 0 failures), @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7706a0d416
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ying exceptions An identity check comparing against the empty string accepted " ". The generated cluster-wide designator uses kind: ".*", which full-matches a whitespace value just as it does an empty one, so a failed control was suppressed against an identity that had never been established and the run exited clean with nothing to file — the false all-clear this command exists to prevent. Both the guard and the diagnostic that names the missing fields now trim before deciding, so the error stays accurate for a blank value.
Round 16's P2 is fixed (whitespace-only workload identity was accepted and then suppressed by the @coderabbitai review |
|
✅ Action performedReview finished.
|
CodeRabbit is still rate-limited at this head — its ack said it would review CI green at this head (25/25, 0 failures), @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c48dd8680
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The previous commit trimmed the workload-identity check on the posture path and in the shared diagnostic, but left deriveCVE comparing against the empty string — so a whitespace-only kind or name still produced a valid-looking theme naming an unidentified component, and several blank identities collapsed into one another in the component set. deriveCVE's own comment already stated the rule this violated: the check belongs on both paths or it covers only one of two.
Round 17's P2 is fixed: the trimmed identity check now applies to the CVE path too, which my previous @coderabbitai review |
|
✅ Action performedReview finished.
|
Retriggering after the stated rate-limit window elapsed (it reported 2 minutes). @coderabbitai review |
|
✅ Action performedReview finished.
|
Readiness — self-promoting
User evaluation, with controls (a guard that rejected everything would look identical to a correct one, so each case had to come out differently):
Rounds 15–17 found four things; three were real and fixed, one refuted on the thread. Round 17's was The residual regex-satisfiability analysis is split out to #2910 rather than spun for an eighteenth
|
Evicted from the merge queue — root-caused, deliberately NOT re-queuedThis PR was promoted and enqueued at The failing job is
Why I am not simply re-queuingRe- This PR is a poor candidate for having caused it. Its entire diff is a standalone Go tool under The discriminator is already running: pr-2908 entered the queue right behind this one and its Run logs were not readable while the run was still pending (the heal job holds it open), so the next Leaving the PR promoted, green, and pentad-clear at |
Merge-queue recoveryThe first merge-group attempt (run 30760329199) failed while Recovered at the root:
This PR remains unchanged at reviewed head |
1 participant
Why
Live-only security findings — posture controls that only fail once workloads run, and CVEs that only exist against a pulled image — never reach the backlog on their own, because the engineer's survey is GitHub-only. Today they get worked when a human happens to notice, which is the structural reason posture drift goes unnoticed for stretches.
While building this I measured something that changes the design: Kubescape's apiserver returns spec-stripped objects on LIST. The same workload reads
critical=0viakubectl get <crd> -Aandcritical=18via a per-objectGET. A bridge fed from LIST output would report a spotless cluster forever while looking perfectly healthy — the exact "a broken scanner and a compliant one read identically" failure this program exists to prevent. So the tool refuses that shape rather than trusting it.What
The report-only half of the bridge. It reads per-object Kubescape summaries and derives themed backlog entries — one per failed control, one per CVE severity class, never one per resource — each with a fingerprint that deliberately excludes counts and timestamps, so unchanged cluster state re-derives byte-identical output and a fluctuating count updates rather than re-files.
Controls the platform has already accepted are excluded before anything is queued: the tool reads the same declared-exception artifact the CI scan uses, so the two cannot disagree about what counts as accepted. Filtering is per-workload, so an exception scoped to one workload never silences the same control elsewhere.
Issue writes are the default-off half and are refused behind
-mode; both states are tested. Findings carry only the sanitized minimum (class + affected public component), with reachability evidence left out.Exercised against the live prod cluster in both shapes: the stripped LIST fails closed with an actionable message, and real per-object output produces correctly grouped, sanitized themes.
Part of #2451 and #2854.
This slice deliberately leaves #2854 open: the write half (at-most-one-issue-per-theme, close-when-gone) is still outstanding and carries its own acceptance criteria.