From 0eac005fb2ddacaa78ba20e607f852cee3b4d2f1 Mon Sep 17 00:00:00 2001 From: Jeff Larson Date: Wed, 1 Jul 2026 00:41:34 -0700 Subject: [PATCH] feat(observe): watch Secrets metadata-only so credential bytes never enter memory (JEF-268) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit protector's graph models Secrets purely by identity (namespace + name via `SecretMeta`), yet it fetched full Secret objects — the reflector watched `Secret` and the initial list called `.list()` — so every credential's `.data` crossed the wire and sat in the in-memory reflector store though nothing read it. Switch both Secret reads to metadata-only: - Reflector now reflects `PartialObjectMeta` via `watcher(Api::>, _)` — the non-deprecated equivalent of `metadata_watcher` in kube 4.0.0 (metadata_watcher is deprecated in favor of this exact form; using it directly would trip clippy warns-as-errors). - Initial list uses `Api::::list_metadata(..)` instead of `.list(..)`. `ObjectMeta` carries name+namespace, so this is behavior-preserving: `SecretMeta` construction and the graph's secret-objective nodes are identical, but `.data` never enters memory. A comment at the watch site records why the RBAC grant stays (vanilla RBAC can't express metadata-only on secrets; this is voluntary client restraint, not a narrowed grant — dropping the grant is a separate ticket). Tests: two new tests pin the metadata-only guarantee to the reflected type — one asserts `metadata_api()` is true, one proves a full Secret payload deserializes as `PartialObjectMeta` with `.data`/`stringData` dropped and identity kept. Existing graph/reachability tests stay green (461 tests pass). Co-Authored-By: Claude Opus 4.8 (1M context) Claude-Session: https://claude.ai/code/session_01VtjoJttCvBY4dzCoE4f9vP --- engine/src/engine/observe/mod.rs | 9 ++-- engine/src/engine/run_loop.rs | 90 +++++++++++++++++++++++++++++++- 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/engine/src/engine/observe/mod.rs b/engine/src/engine/observe/mod.rs index 9cceffb..ee79489 100644 --- a/engine/src/engine/observe/mod.rs +++ b/engine/src/engine/observe/mod.rs @@ -281,12 +281,15 @@ impl Snapshot { ) }, async { anyhow::Ok(Api::::all(client.clone()).list(&lp).await?.items) }, - // Secrets are listed for their metadata only; values are dropped here and - // never enter the graph. + // Secrets are listed METADATA-ONLY (JEF-268): `list_metadata` asks the + // apiserver for `PartialObjectMeta`, so `.data`/`stringData` never + // cross the wire. Only identity (namespace + name) is retained, exactly what + // `SecretMeta` and the graph's secret-objective nodes need. See the RBAC + // caveat at the reflector watch site in `run_loop.rs`. async { anyhow::Ok( Api::::all(client.clone()) - .list(&lp) + .list_metadata(&lp) .await? .items .into_iter() diff --git a/engine/src/engine/run_loop.rs b/engine/src/engine/run_loop.rs index 0fca1a3..67a1bf5 100644 --- a/engine/src/engine/run_loop.rs +++ b/engine/src/engine/run_loop.rs @@ -199,6 +199,7 @@ pub async fn run_watch( use k8s_openapi::api::networking::v1::NetworkPolicy; use k8s_openapi::api::rbac::v1::{ClusterRole, ClusterRoleBinding, Role, RoleBinding}; use kube::Api; + use kube::core::PartialObjectMeta; use kube::runtime::{WatchStreamExt, reflector, watcher}; // Diagnostic judgement log: the full prompt + raw reply + verdict per judgement, @@ -334,7 +335,21 @@ pub async fn run_watch( let (pods, pods_w) = reflector::store::(); let (netpols, netpols_w) = reflector::store::(); let (services, services_w) = reflector::store::(); - let (secrets, secrets_w) = reflector::store::(); + // Secrets are watched METADATA-ONLY (JEF-268): the graph only ever needs a + // Secret's identity (namespace + name — see `SecretMeta`), never its `.data`, so + // we reflect `PartialObjectMeta`. `Api::>` issues + // metadata-only requests, so the apiserver never sends — and this in-memory store + // never holds — any credential bytes. (`metadata_watcher` is the deprecated spelling + // of the same behavior in kube 4.0.0; the `watcher(Api::>, _)` + // form below is its non-deprecated equivalent.) + // + // RBAC caveat: vanilla k8s RBAC can't express "metadata-only on secrets" — + // `get/list/watch` on `secrets` is all-or-nothing — so protector's grant necessarily + // still permits reading values. This change removes the *exposure* (what protector + // holds in memory), a voluntary client-side restraint; it does not narrow the grant. + // Dropping the grant entirely (deriving secret nodes from mounts + RBAC) is a + // separate ticket, deliberately out of scope here. + let (secrets, secrets_w) = reflector::store::>(); let (roles, roles_w) = reflector::store::(); let (rolebindings, rolebindings_w) = reflector::store::(); let (clusterroles, clusterroles_w) = reflector::store::(); @@ -371,7 +386,9 @@ pub async fn run_watch( spawn_reflector!(pods_w, Pod); spawn_reflector!(netpols_w, NetworkPolicy); spawn_reflector!(services_w, Service); - spawn_reflector!(secrets_w, Secret); + // Metadata-only Secret watch (JEF-268): reflects `PartialObjectMeta`, so the + // stream carries identity only — `.data` never crosses the wire or lands in the store. + spawn_reflector!(secrets_w, PartialObjectMeta); spawn_reflector!(roles_w, Role); spawn_reflector!(rolebindings_w, RoleBinding); spawn_reflector!(clusterroles_w, ClusterRole); @@ -468,3 +485,72 @@ pub async fn run_watch( } Ok(()) } + +#[cfg(test)] +mod tests { + //! JEF-268: the Secret informer (reflector watch + initial list) must be + //! metadata-only — protector reasons about a Secret's *identity* (namespace + + //! name), never its contents, so no credential bytes must ever cross the wire or + //! sit in the in-memory store. These tests pin that guarantee to the exact type the + //! informer reflects, `PartialObjectMeta`; a regression to the full `Secret` + //! type (which carries `.data`) fails them. + + use k8s_openapi::api::core::v1::Secret; + use kube::Resource; + use kube::core::PartialObjectMeta; + + /// The reflected element type asks the apiserver for metadata only. `metadata_api()` + /// is what drives both `watcher(Api::>, _)` and + /// `Api::::list_metadata` to issue `.../secrets` requests that return + /// `PartialObjectMeta` (no `.data`) rather than full Secret objects. + #[test] + fn secret_informer_requests_metadata_only() { + assert!( + as Resource>::metadata_api(), + "Secret informer must reflect a metadata-only type; a full Secret would \ + fetch and retain credential bytes" + ); + } + + /// Even handed a full Secret payload (as an apiserver bug or a mistaken watch would + /// deliver), the reflected type structurally cannot retain `.data`/`stringData`: it + /// is dropped on deserialize, while the identity the graph needs survives. This is the + /// "no full Secret with `.data` retained" guarantee. + #[test] + fn reflected_secret_drops_data_keeps_identity() { + let full_secret = serde_json::json!({ + "apiVersion": "v1", + "kind": "Secret", + "metadata": { "namespace": "prod", "name": "db-creds" }, + "type": "Opaque", + "data": { "password": "c3VwZXItc2VjcmV0" }, + "stringData": { "token": "super-secret" }, + }); + + let reflected: PartialObjectMeta = + serde_json::from_value(full_secret).expect("deserialize as metadata-only"); + + // Identity — exactly what `SecretMeta` / the graph's secret-objective nodes need — + // is preserved. + assert_eq!(reflected.metadata.namespace.as_deref(), Some("prod")); + assert_eq!(reflected.metadata.name.as_deref(), Some("db-creds")); + + // Round-trip back to JSON and prove no credential bytes survived anywhere. The + // keys are matched quoted (`"data"`) so the `data` inside `"metadata"` doesn't + // give a false positive. + let round_trip = serde_json::to_value(&reflected).expect("serialize"); + let text = round_trip.to_string(); + assert!( + !text.contains("\"data\""), + "reflected Secret must not carry a `data` field" + ); + assert!( + !text.contains("\"stringData\""), + "reflected Secret must not carry a `stringData` field" + ); + assert!( + !text.contains("c3VwZXItc2VjcmV0") && !text.contains("super-secret"), + "no credential bytes may survive into the reflected store" + ); + } +}