feat: add field-level redaction for denied_resources

[Yurzs]

Problem

When using kubernetes-mcp-server with AI agents, denied_resources is all-or-nothing — you either block an entire resource type or allow full access to it.

This is a problem for resources like Secrets: blocking them entirely means the agent can't see that a Secret exists, what keys it contains, its type, ownership, or annotations. But allowing full access exposes the actual secret values to the LLM context.

Solution

Add a built-in redact_secrets configuration option that redacts Secret values while preserving metadata. This is a hardcoded, Secret-specific redaction that avoids the complexity and security risks of a generic field-walking approach.

What gets redacted

When redact_secrets is set, the following fields are redacted on all Secret resources regardless of API version:

• data.* — all values replaced with redaction markers
• stringData.* — all values replaced with redaction markers
• kubectl.kubernetes.io/last-applied-configuration annotation — stripped entirely (it contains unredacted Secret data in plaintext)

Keys, metadata (name, namespace, labels, annotations, ownerReferences), and type are all preserved.

Configuration

# Replace Secret values with [REDACTED]
redact_secrets = "opaque"

# Replace Secret values with deterministic HMAC-SHA256 hashes
# Allows agents to detect duplicate values without seeing actual content
redact_secrets = "hashed"

Redaction modes

• opaque — values become [REDACTED]
• hashed — values become [REDACTED:gen_<id>:<hash>] where the hash is a deterministic HMAC-SHA256 with a per-session random salt. Same values produce the same hash within a session. The generation ID changes on restart so consumers know when hashes from different sessions are not comparable.

Security properties

• Fail-closed: The redactor is a hardcoded list of fields (data.*, stringData.*, last-applied-configuration). There is no user-configurable field walker — nothing to misconfigure.
• Version-agnostic: Matches kind=Secret regardless of API group or version, preventing bypass via version pinning (e.g. v1beta1).
• Annotation stripping: kubectl.kubernetes.io/last-applied-configuration is removed because it stores the full previous manifest including unredacted Secret data.
• Applied at the Core layer: Redaction happens in the Kubernetes wrapper layer (Core struct), so all tool handlers benefit automatically without per-handler code.

Example output (hashed mode)

apiVersion: v1
kind: Secret
metadata:
  name: web-ui
  namespace: production
type: Opaque
data:
  client_secret: '[REDACTED:gen_ee128e09:df2e09b64a1c8f37]'
  jira_api_token: '[REDACTED:gen_ee128e09:3f2501db82e4a90c]'
  sentry_dsn: '[REDACTED:gen_ee128e09:039c47e8f15b6d2a]'

The agent can see: the Secret exists, it has 3 keys (client_secret, jira_api_token, sentry_dsn), it's type Opaque, and it's owned by a SealedSecret controller. It cannot see any actual values.

Changes

• pkg/redaction/ — Rewrote from a 212-line generic field walker to a 110-line Secret-specific redactor
• pkg/config/ — Replaced [[redacted_resources]] array-of-tables with redact_secrets string field
• pkg/kubernetes/core.go — Merged two Core constructors into one: NewCore(client, redactSecretsMode)
• pkg/toolsets/ — Updated all 25 call sites across 9 files
• docs/configuration.md — Updated documentation

[Cali0707]

Hey @Yurzs thanks for working on this! This is a huge improvement to the safety controls provided by the server, looking forward to landing this 😄

A couple thoughts:

1. Let's move the redaction from being applied in the tool handlers to being applied in the kubernetes wrapper layer (e.g. pkg/kubernetes/resources.go, pkg/kubernetes/pods.go, etc.). As some toolsets call the dynamic client directly, let's export a helper for those toolsets to use (and add a rule to CLAUDE.md to always use it)
2. Can we separate out resource redaction from denial in the config? IMO it will be clearer to users if they can specify something like;

[[denies_resources]]
group = ""
version = "v1"
kind = "ClusterRoles"

[[redacted_resources]]
group = ""
version = "v1"
kind = "Secret"
fields = ["data.*", "stringData.*"]
mode = "hashed"

[Yurzs]

Thanks for the review @Cali0707! Both changes are done in the updated commit:

1. Redaction moved to the wrapper layer

pkg/kubernetes/core.go now has NewCoreWithRedaction() which embeds a Redactor in the Core struct. Redaction is applied automatically in ResourcesGet, ResourcesList, and resourcesCreateOrUpdate.

Exported RedactResource()/RedactResourceList() helpers are available for toolsets that bypass the wrapper. All core, health check, and kubevirt create handlers use NewCoreWithRedaction.

2. Separate config section

[[redacted_resources]] is fully separated from [[denied_resources]] with its own type (RedactedResource) using fields and mode parameters, exactly as you suggested:

[[redacted_resources]]
group = ""
version = "v1"
kind = "Secret"
fields = ["data.*", "stringData.*"]
mode = "hashed"

Validation requires version, kind, non-empty valid field paths, and a valid mode.

Note: table-format list responses are not redacted because the API server returns computed column values (e.g. key count for Secrets), not the raw resource fields. This is documented in resourcesListAsTable.

[matzew]

there are no MCP-layer integration tests for redacted_resources. The denied_resources feature has extensive coverage at this layer (e.g. TestResourcesListDenied in resources_test.go, plus tests in pods_test.go, helm_test.go, etc.), but redacted_resources is only tested at the unit level against hand-crafted unstructured objects.

A test in pkg/mcp/resources_test.go that creates a Secret in envtest, configures redacted_resources via TOML, calls resources_get/resources_list, and verifies the response contains [REDACTED] markers while preserving metadata would close the gap. This would verify the full path from TOML config parsing through NewCoreWithRedaction to response serialization.

[Yurzs]

@matzew
Added MCP-layer integration tests in the latest push. Four new test methods in pkg/mcp/resources_test.go:

• TestResourcesGetRedacted — creates a Secret in envtest, configures [[redacted_resources]] via toml.Unmarshal with opaque mode, calls resources_get, and verifies data.* values are [REDACTED] while metadata (name, namespace, kind) is preserved. Also includes a "non-matching resource is not redacted" subtest that fetches a ConfigMap and confirms its data passes through unchanged.
• TestResourcesGetRedactedHashed — same flow but with mode = "hashed", verifies [REDACTED:gen_...] prefixes and that identical input values produce identical hashes.
• TestResourcesListRedacted — configures opaque redaction, creates a labeled Secret, calls resources_list with a label selector, and verifies data is redacted in listed items while metadata is preserved.
• TestResourcesCreateOrUpdateRedacted — configures opaque redaction, creates a Secret via resources_create_or_update using stringData (which Kubernetes converts to data server-side), and verifies the returned resource has redacted data values.

All tests follow the established patterns (toml.Unmarshal into s.Cfg, s.InitMcpClient(), nested s.Run(), testify/suite assertions) and verify the full path from TOML config parsing through NewCoreWithRedaction to response serialization.

[manusa]

In general, I like the feature, it's something very much needed.

However, we need to be very careful about how we implement it and how it's handled by the toolset API.

Regarding the salt (pepper), I'm not sure about the promise to deliver here. Also about its intersection with statelessness and statefulness.
It's a good security measure, but I wouldn't make any sort of promise around it. Currently, it's kept while the process lives and regenerated upon restart. That's fine, but I'd try to not make any promises around that, if it's rotated in-process, there shouldn't be a problem.

Added a few comments inline with my main concerns.

[manusa]

Why do we need another constructor function?

[Yurzs]

Resolved — merged into a single NewCore(client, redactSecretsMode) constructor. The second parameter is just the mode string ("opaque", "hashed", or "" for no redaction). All 25 call sites updated.

[manusa]

Redaction in this layer is brittle, if a toolset implementer doesn't comply and its tool returns a Kubernetes resource, it might not get redacted.

I think this was already reported by Cali0707

Let's move the redaction from being applied in the tool handlers to being applied in the Kubernetes wrapper layer

We need to find a way to do this at the HTTP client level like we do for resource denial.

Or maybe make it part of the pkg/output (as discussed internally with Cali0707), the challenge here would be to enforce the output for all toolsets (which relates to #920 / #753)

We can probably already prototype this (redaction in output), since it would essentially move what we're doing here to the presentation layer

[Yurzs]

Agreed that the generic approach was brittle here. In the rework, redaction is still applied at the Core layer (in Apply/ApplyToList calls within ResourcesGet, ResourcesList, etc.), but it's no longer a configurable path walker — it's a hardcoded Secret check (kind == "Secret") that redacts known fields. This makes it impossible to miss fields due to path misconfiguration.

Moving to the output/presentation layer is a good idea for the future and would close the gap for toolsets that bypass the wrapper entirely. Happy to explore that in a follow-up.

[manusa]

The wildcard is also something that worries me a lot.
It needs to be very clear what the wildcard does.
IMO it should be a very narrow behavior, at most allow for a wildcard as the final placeholder which would essentially mean redacting the entire previous field.

[Yurzs]

The wildcard behavior is intentionally narrow and deterministic:

• On a map: * iterates all keys at that level
• On a slice: * iterates all elements at that level

Mid-path wildcards are needed for practical use cases like redacting env values in Deployments:

[[redacted_resources]]
group = "apps"
version = "v1"
kind = "Deployment"
fields = ["spec.template.spec.containers.*.env.*.value"]

Without mid-path *, you'd have to redact containers.* entirely, which would hide container names, images, ports — not just the sensitive env values.

I'll add config-time validation that:

• Rejects a bare * as the entire field path (would redact every top-level key including metadata)
• Rejects empty segments and consecutive wildcards

Would that address the concern, or do you feel strongly about terminal-only wildcards?

[manusa]

The problem is not whether * is greedy or deterministic. The problem is that the whole mechanism fails open: every mismatch between the path and the actual data shape results in a silent leak, not an error.

Concrete examples:

1. Forget one * and nothing is redacted, silently:

fields = ["spec.template.spec.containers.env.*.value"]  # missing .* after containers

walkSlice hits a named segment on a slice and skips (// Named segment on a slice: not meaningful, skip). No error, no log, values fully exposed. The config-time validation you propose doesn't catch this: the path is syntactically valid, it just doesn't match the data.

2. The PR's own example is already bypassed today:

fields = ["data.*", "stringData.*"]

Any Secret managed with client-side kubectl apply carries the full object, including data, in metadata.annotations["kubectl.kubernetes.io/last-applied-configuration"]. The rule doesn't touch it.

3. Version pinning is a bypass. matchesGVK requires an exact version match and validation requires version to be set. For a multi-version CRD, redact v1 and the client just requests v1beta1 to get the unredacted object.

4. Type-dependent dispatch means the same rule behaves differently depending on the data. * on a map iterates keys, on a slice iterates items, and on a scalar mid-path it does nothing (no default case in walkValue). If a CRD field evolves from map to list, redaction silently stops working.

This is the core of my concern with the wildcard: a denylist-by-path used as a security control must fail closed, with no silent fail-open path anywhere.

For this to move forward we should consider:

• When the walker can't follow a path (shape mismatch, unexpected type), redact the whole subtree or fail, never skip.
• Match group+kind across all served versions, not a pinned version.
• Strip last-applied-configuration whenever a rule matches the object.

Another alternative worth considering: the most common use case is probably "redact Secret values", and that could ship as a built-in (covering data, stringData and the last-applied annotation) behind a single config flag, with no path language at all. The generic field walker could then come later if there's real demand for it.

[Yurzs]

@manusa Fully agreed with your analysis — every point was valid. I've reworked the entire approach in the latest force-push, taking your suggestion of a built-in Secret redaction behind a single config flag.

What changed:

The generic field walker (walkValue/walkMap/walkSlice with dot-path segments and wildcards) is completely removed. In its place is a hardcoded Secret-specific redactor that addresses every concern you raised:

1. Fail-closed, no path language — The redactor has a hardcoded list of what it redacts: data.*, stringData.*, and kubectl.kubernetes.io/last-applied-configuration. There are no user-configurable paths, so there's nothing to misconfigure and no silent fail-open path.

2. last-applied-configuration stripping — The annotation is stripped on every Secret, preventing the bypass you identified.

3. Version-agnostic matching — Matches kind=Secret regardless of API version, so v1beta1 requests get the same redaction as v1.

4. No type-dependent dispatch — No wildcards, no map-vs-slice branching. The redactor knows the exact shape of a Secret and operates on it directly.

Config is now a single flag:

redact_secrets = "opaque"   # or "hashed"

Replaces the entire [[redacted_resources]] section. Validation is trivial: must be "opaque", "hashed", or empty.

The generic field walker can come later if there's real demand for it, as you suggested.

[Yurzs]

Agreed on the salt — I've kept the same per-process random salt mechanism but the rework doesn't make any promises about rotation behavior. The generation ID is there purely so consumers can detect when hashes are no longer comparable (e.g. after restart), not as a guarantee about when rotation happens.