ROB-997 Redact Secret data from outbound notifications

[naomi-robusta]

Fixes ROB-997 (High, CWE-201).

The problem

pkg/handlers/cloudevent put whole runtime.Object values into Obj/OldObj and json.Marshal'd them. With Secret watching enabled (resource.secret: true), every Secret create, update and delete JSON-serialized the Secret's data (base64) and stringData (plaintext) to the configured receiver. Updates sent the previous values too, via oldObj.

Reproduced before fixing — one message per operation, sentinel bytes present in all three:

"obj":{"kind":"Secret",...,"data":{"password":"U0VOVElORUwt...LU5FVw=="},
                                   "stringData":{"token":"SENTINEL-c0ffee-DO-NOT-LEAK-NEW"}},
"oldObj":{"kind":"Secret",...,"data":{"password":"U0VOVElORUwt...LU9MRA=="},
                                      "stringData":{"token":"SENTINEL-c0ffee-DO-NOT-LEAK-OLD"}}

A second path the ticket doesn't mention

customresources (controller.go:562) goes through the dynamic client and yields *unstructured.Unstructured. Configuring group: "", version: v1, resource: secrets produces the same disclosure while resource.secret stays false — the flag is not a control on that path at all. Both paths are covered here.

Approach: redact, not truncate

The ticket recommends replacing Obj/OldObj with a metadata-only DTO. That would break the platform: the Robusta runner is the CloudEvent consumer and matches playbooks on the full object body (base_triggers.py loads hikaru objects from obj/oldObj, and scope matchers walk arbitrary nested paths). So this redacts the secret material and leaves every other resource's payload intact.

New pkg/redact, applied in two layers:

1. Typed — in the controller as events are built, so every handler is covered rather than just the ones that serialize objects today. A Secret's data/stringData values become [redacted by kubewatch]. Objects are deep-copied first: they are the shared informer cache's own objects, and redacting one in place would corrupt the cache for every other reader in the process.
2. Defensive — on the marshalled bytes in the CloudEvent handler, immediately before the POST. Walks the document and redacts the data fields of any object whose kind is Secret, at any depth. This is the backstop for Secrets the typed layer cannot see, notably the unstructured path above. A payload that fails the round-trip returns an error rather than falling back to the unredacted bytes.

What survives: key names, labels, annotations, type, and the rest of the metadata — so a notification still says which Secret changed and which keys it has, just not their values. Redacted data values stay valid base64 in both layers (W3JlZGFjdGVkIGJ5IGt1YmV3YXRjaF0=), so receivers that decode them still can. Non-Secret objects pass straight through — same pointer, no copy, no allocation.

Actual outbound body after the fix:

"obj": {
  "kind": "Secret",
  "metadata": { "name": "registry-creds", "namespace": "prod", "labels": {"app": "api"},
                "generation": 4, "resourceVersion": "1245" },
  "type": "kubernetes.io/dockerconfigjson",
  "data": { ".dockerconfigjson": "W3JlZGFjdGVkIGJ5IGt1YmV3YXRjaF0=",
            "tls.key":           "W3JlZGFjdGVkIGJ5IGt1YmV3YXRjaF0=" },
  "stringData": { "token": "[redacted by kubewatch]" }
}

Tests performed

• go build ./..., go vet ./... and go test -race ./... (what make test runs) all pass. One pre-existing failure is unchanged — see below.
• Regression tests drive the real trigger path — Kubernetes API → informer → controller → handler → HTTP — with sentinel bytes, asserting neither the raw nor the base64 form reaches the receiver on create, update or delete (pkg/controller/controller_test.go, pkg/handlers/cloudevent/cloudevent_test.go).
• Non-Secret resources keep their full object body: a Pod event is asserted to still carry spec, status, labels, image, node name, and oldObj.
• The shared informer cache is asserted unmutated after redaction, at all three levels (pkg/redact, controller, handler).
• Envelope integrity: objName() names the resource type "Secret", so data.kind is literally the string the defensive layer keys off. A test pins down that the envelope and event metadata come out whole regardless.
• Unit coverage for nested Secrets, Secrets inside lists, a non-map data field, large-integer fidelity across the JSON round-trip, and invalid JSON being rejected rather than passed through.
• The PR-description check itself: verified the fixed logic against this very body, a minimal valid body, three descriptions that must still be rejected (no section / no bullets / bullets only in a later section), and an injection attempt that now passes through as inert data. Also confirmed the old logic fails on this same body with the identical conditional binary operator expected / exit 2 that CI reported.

Mutation-tested rather than only run green. Four mutations, each caught by a distinct test:

Mutation	Caught by
Typed layer disabled	TestSecretEventsReachHandlersRedacted — cloudevent still held, via layer 2
Defensive layer disabled	TestJSONRedactsNestedAndListedSecrets, TestJSONRedactsNonMapDataField
Redact in place (no DeepCopy)	3 cache-integrity tests across all three packages
Both layers removed	5 tests

A CI fix rides along

The first run of check-pr-description failed on this PR, and the cause was the check, not the change. It interpolated ${{ github.event.pull_request.body }} straight into its run: block, so the script text was assembled from untrusted input:

• backticks in a description ran as commands on the runner — reproducing it locally, this body really executed go build ./..., go test and make test;
• a double quote ended the string bash was parsing, which is the conditional binary operator expected / exit 2 the run reported.

Second commit passes the body through the environment and quotes it. The check keeps its teeth — no ## Tests performed section, or a section with no bullet list, still fails. It affects every PR in the repo, not just this one, so say the word if you would rather it were split into its own PR.

For reviewers

Two judgment calls worth a look:

• Key names are kept; only values are redacted. The ticket says "clear Data and StringData". Key names are not secret material and carry real signal (which keys exist, which were added or removed), while name/namespace/labels go out regardless. Trivial to switch to clearing the maps outright if you'd rather.
• A Secret payload's JSON key order changes. The defensive layer re-marshals, so keys come out map-sorted instead of in struct-field order. Only for payloads that actually contain a Secret; key order is not semantically meaningful and both the runner (pydantic/hikaru) and any JSON parser are unaffected.

Not addressed here, and worth separate tickets:

• pkg/handlers/slackwebhook.TestWebhookInit fails on master — the test reuses one SlackWebhook across cases while Init mutates it. Confirmed pre-existing by stashing this branch; left alone as out of scope.
• processItem re-reads the object from the informer cache instead of using the event's own, so a rapid create→delete burst can silently drop the create notification. Hit while writing the controller test, which sequences around it with a comment.
• ROB-962 (the cluster-wide Secret read grant that makes this reachable) is a separate change. This PR stops the data going off-cluster; that one narrows the access.