feat(secrets): stop retaining Secret values in the control plane

cmd/main.go

@@ -23,12 +23,14 @@ import (
 	"go.uber.org/zap/zapcore"
 	_ "k8s.io/client-go/plugin/pkg/client/auth"
 
+	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
 	"k8s.io/apimachinery/pkg/runtime"
 	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
 	clientgoscheme "k8s.io/client-go/kubernetes/scheme"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/certwatcher"
+	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
 	"sigs.k8s.io/controller-runtime/pkg/metrics/filters"
 	metricsserver "sigs.k8s.io/controller-runtime/pkg/metrics/server"
@@ -843,6 +845,18 @@ func newManager(
 		Metrics:                metricsOptions,
 		HealthProbeBindAddress: probeAddr,
 		WebhookServer:          webhookServer,
+		// Never cache Secret values. The control plane reads a small set of named
+		// Secrets (Git credentials, signing keys, age keys) directly by name; caching
+		// them would start a cluster-wide Secret informer that retains every Secret
+		// value in memory. Typed Secret Get/List go straight to the API server instead,
+		// which also keeps credential/age-key rotation reads fresh. Mirrored Secrets
+		// selected by a WatchRule use the separate dynamic-watch path, not this cache.
+		// See docs/future/secret-value-retention-plan.md.
+		Client: client.Options{
+			Cache: &client.CacheOptions{
+				DisableFor: []client.Object{&corev1.Secret{}},
+			},
+		},
 	})
 	if err != nil {
 		setupLog.Error(err, "unable to start manager")

docs/design/manifest/gitpathaccepted-projection-race-and-external-drift.md

@@ -25,6 +25,16 @@ is and is not the cause, then the design direction.
 
 ## 1. Facts and observations (no interpretation yet)
 
+> **Update (2026-07-07, secret-value-retention):** §1 is a point-in-time snapshot of the
+> code at investigation time. Two facts below have since changed. (1) The GitTarget
+> control-plane **Secret watch was removed**, so `SetupWithManager` now watches
+> `GitProvider`, `WatchRule`, and `ClusterWatchRule` — not `Secret` (§1.5). (2) The
+> `RequeueShortInterval`/`RequeueLongInterval` split (2 min / 10 min) collapsed into a single
+> 5-minute `RequeueSteadyInterval`, so the happy-path GitTarget reconcile requeues after
+> **5 minutes, not 10** (§1.5, §2). The projection-race analysis and its GitPath-channel fix
+> are otherwise unaffected. See
+> [`../../future/secret-value-retention-plan.md`](../../future/secret-value-retention-plan.md).
+
 ### 1.1 The failure
 
 CI run [`28331751067`](https://github.com/ConfigButler/gitops-reverser/actions/runs/28331751067)

docs/design/reconcile-triggering.md

@@ -28,23 +28,33 @@ Facts first, then the fix, then the gaps, then the toolbox.
 | Constant | Value |
 |---|---|
 | `RequeueStreamSettleInterval` | 10 s |
-| `RequeueShortInterval` | 2 min |
-| `RequeueMediumInterval` | 5 min |
-| `RequeueLongInterval` | 10 min |
+| `RequeueSteadyInterval` | 5 min |
+
+> **Update (2026-07-07, secret-value-retention):** the former `RequeueShortInterval`
+> (2 min), `RequeueMediumInterval` (5 min), and `RequeueLongInterval` (10 min) collapsed
+> into a single 5-minute `RequeueSteadyInterval`. The control plane no longer watches
+> Secrets, so out-of-band credential/age-key changes are picked up on this unified steady
+> cadence instead of via a Secret informer. Where the narrative below says "up to 10
+> minutes," read 5 minutes. See
+> [`future/secret-value-retention-plan.md`](../future/secret-value-retention-plan.md).
 
 ### 1.2 Per-controller triggers
 
 | Controller | Watches (besides `For`) | Predicate | Happy-path requeue | Data-plane edge |
 |---|---|---|---|---|
-| **GitProvider** | *(none)* | `GenerationChanged` on self | 10 min | ❌ |
-| **GitTarget** | Secret, GitProvider, WatchRule, ClusterWatchRule | `GenerationChanged` on deps | 10 min | ✅ GitPath channel (new) |
+| **GitProvider** | *(none)* | `GenerationChanged` on self | 5 min | ❌ |
+| **GitTarget** | GitProvider, WatchRule, ClusterWatchRule | `GenerationChanged` on deps | 5 min | ✅ GitPath channel (new) |
 | **WatchRule** | GitTarget, GitProvider | `GenerationChanged` | 5 min | ❌ |
 | **ClusterWatchRule** | GitTarget, GitProvider | `GenerationChanged` | 5 min | ❌ |
 | **CommitRequest** | *(none)* | — | polls every 2 s | polls worker, no push |
 
-Sources: `gitprovider_controller.go:428`, `gittarget_controller.go:967`,
-`watchrule_controller.go:458`, `clusterwatchrule_controller.go:455`,
-`commitrequest_controller.go:377`.
+The GitTarget no longer watches Secrets: the encryption-Secret watch was removed so the
+process stops retaining every Secret value in the cluster; generated-age-Secret recovery
+and out-of-band age-key updates ride the 5-minute reconcile instead.
+
+Sources: `gitprovider_controller.go`, `gittarget_controller.go`,
+`watchrule_controller.go`, `clusterwatchrule_controller.go`,
+`commitrequest_controller.go`.
 
 ### 1.3 Two structural consequences
 
@@ -136,19 +146,24 @@ already exists; only the push edge is missing. A nudge would replace the poll.
 ### Gap B — control-plane status does not propagate to dependents
 
 **B1. WatchRule / ClusterWatchRule lag their GitTarget/GitProvider.** They watch
-those deps with `GenerationChangedPredicate` (`watchrule_controller.go:465`,
-`clusterwatchrule_controller.go:463`), so when a GitTarget flips to refused or a
+those deps with `GenerationChangedPredicate`, so when a GitTarget flips to refused or a
 GitProvider goes `Ready=False` (status-only changes), the rules do **not**
 re-reconcile; their projected `GitTargetReady` dependency condition is stale for up
-to `RequeueMediumInterval` (5 min).
+to `RequeueSteadyInterval` (5 min).
 
 ### Gap C — missing dependency watches
 
 **C1. GitProvider does not watch its credentials Secret.** `SetupWithManager` is
-`For(GitProvider)` only (`gitprovider_controller.go:428`). A rotated or corrected
-Git credentials Secret is not picked up until the 10-minute requeue, so
-`Ready=False (ConnectionFailed)` lingers after the fix. (GitTarget watches Secrets
-for *encryption*, but no controller watches the *auth* Secret.)
+`For(GitProvider)` only. A rotated or corrected Git credentials Secret is not picked up
+until the 5-minute steady requeue, so `Ready=False (ConnectionFailed)` lingers that long.
+
+> **Superseded (2026-07-07, secret-value-retention):** not watching the auth Secret is now a
+> deliberate least-privilege choice, not a gap to fix with a Secret watch. The control plane
+> reads credential and age-key Secrets directly by name and relies on the 5-minute
+> `RequeueSteadyInterval` to pick up rotations; the GitTarget's former encryption-Secret watch
+> was also removed. Adding a Secret watch would reintroduce cluster-wide Secret retention. See
+> [`future/secret-value-retention-plan.md`](../future/secret-value-retention-plan.md). The
+> §5 recommendation to "watch the auth Secret" below is retained only as historical context.
 
 ---
 

docs/future/scoped-rbac-least-privilege-plan.md

@@ -0,0 +1,253 @@
+# Scoped RBAC and least-privilege install
+
+> Status: design plan — **not started** (this is Track B). Its runtime prerequisites
+> (stop retaining Secret values, drop control-plane Secret watches, 5-minute reconcile
+> fallback) shipped in PR
+> [#208](https://github.com/ConfigButler/gitops-reverser/pull/208), so the runtime no longer
+> *needs* Secret `list/watch` for controller-owned inputs. What remains is the packaging/RBAC
+> work below: the default install still *grants* the broad Secret permission and the
+> `*/* get,list,watch` wildcard.
+> Date: 2026-07-07 (updated 2026-07-08).
+> Roadmap: [Secret handling roadmap](secret-handling-roadmap.md).
+> Related issue: [#205](https://github.com/ConfigButler/gitops-reverser/issues/205).
+
+## Summary
+
+This plan is about what the operator's ServiceAccount is allowed to read. It is separate
+from the Secret cache cleanup in
+[secret-value-retention-plan.md](secret-value-retention-plan.md), which reduces what the
+process keeps in memory.
+
+The key constraint is Kubernetes RBAC: permissions are additive. There is no deny rule and
+no "all resources except Secrets" rule. The default install currently grants:
+
+- core `secrets get,list,watch,create,update,patch`;
+- core `namespaces get,list,watch`;
+- `*/* get,list,watch` for dynamic watching.
+
+While the wildcard exists, a namespaced Secret Role does not reduce the ServiceAccount's
+cluster-wide Secret read permission. Real least privilege means running without the
+wildcard and granting only the resources the install needs.
+
+## What the reporter wants
+
+The reported use case is narrower than the default product model:
+
+- use one Git credential Secret in one namespace;
+- do not mirror Kubernetes Secrets into Git at all;
+- avoid cluster-wide Secret reads;
+- keep SOPS encryption one-way and avoid private material on the write path.
+
+The right answer is not only `--secret-namespaces`. That flag name also implies a scoped
+value cache, which is not the target after the retention cleanup. The better plan is:
+
+1. ✅ stop retaining Secret values in the controller process (PR #208);
+2. ✅ drop control-plane Secret watches and use a 5-minute reconcile fallback (PR #208);
+3. ⬜ add a mode that refuses to mirror Secrets;
+4. ⬜ support BYO-RBAC without the wildcard;
+5. ⬜ generate the narrow RBAC needed for a specific install.
+
+## Current RBAC sources
+
+The generated ClusterRole is in
+[config/rbac/role.yaml](../../config/rbac/role.yaml) and mirrored into the chart at
+[charts/gitops-reverser/config/role.yaml](../../charts/gitops-reverser/config/role.yaml).
+
+Important grants:
+
+```yaml
+- apiGroups: [""]
+  resources: ["namespaces"]
+  verbs: ["get", "list", "watch"]
+- apiGroups: [""]
+  resources: ["secrets"]
+  verbs: ["create", "get", "list", "patch", "update", "watch"]
+- apiGroups: ["*"]
+  resources: ["*"]
+  verbs: ["get", "list", "watch"]
+```
+
+Why they exist:
+
+- The wildcard comes from the dynamic watch manager:
+  [internal/watch/manager.go](../../internal/watch/manager.go#L33). It lets
+  `WatchRule` and `ClusterWatchRule` select many current and future resource types without
+  an RBAC redeploy.
+- Secret reads support Git credentials, age encryption configuration, and signing keys.
+  Secret list/watch is only needed for the current Secret reactivity watch and should go
+  away in the retention plan.
+- Secret writes support generated signing and age-key Secrets.
+- namespace reads are needed for namespaced rule resolution.
+
+## Supported install modes
+
+### Default mode
+
+Keep the generated ClusterRole and wildcard.
+
+Good for convenience and broad discovery. Not least privilege.
+
+### Hardened static mode
+
+Operators set:
+
+```yaml
+rbac:
+  create: false
+serviceAccount:
+  create: false
+  name: existing-service-account
+```
+
+They bind that ServiceAccount to a hand-written or generated role set. This already works
+at the packaging layer. What is missing is documentation and better runtime behavior when
+a rule selects something not granted.
+
+### Future dynamic mode
+
+The operator reconciles its own Roles and RoleBindings from active rules. This is not a
+near-term target. It requires RBAC write permission, privilege-escalation safeguards, and
+an explicit opt-in.
+
+## Plan
+
+### 1. Document BYO-RBAC
+
+Write an operator-facing recipe for hardened static installs:
+
+- disable chart-created RBAC and ServiceAccount;
+- create a ServiceAccount owned by the platform team;
+- grant fixed controller permissions;
+- grant variable watch permissions based on the install's rules;
+- document the trade-off: no wildcard means no automatic access to new resource types.
+
+Fixed permissions include:
+
+- GitOps Reverser CRDs, `/status`, and `/finalizers`;
+- leader-election leases only if leader election is enabled. It is not enabled today, and
+  the generated role does not currently grant `coordination.k8s.io/leases`;
+- events if emitted;
+- namespace reads for namespaced rule resolution;
+- scoped Secret `get` for referenced Git credentials and age-key inputs;
+- scoped Secret writes for generated signing and age-key material;
+- Secret `list/watch` only if optional fast-rotation metadata watches are enabled.
+
+Variable permissions include:
+
+- one `get,list,watch` rule per selected GVR;
+- Role when every selection is namespace-scoped;
+- ClusterRole when the selected resource or selection scope is cluster-wide.
+
+### 2. Add "never mirror Secrets"
+
+Add an exclusion policy, for example:
+
+```text
+--exclude-resources=secrets
+```
+
+or a structured counterpart to `SensitiveResourcePolicy`.
+
+Behavior:
+
+- Secrets are refused by the followability funnel;
+- `WatchRule` or `ClusterWatchRule` status explains the refusal;
+- the dynamic target watch path never opens a Secret watch;
+- generated RBAC can omit mirrored-Secret read grants.
+
+This serves the reporter's strongest requirement: Git credentials remain controller input,
+but cluster Secrets are not mirrored output.
+
+### 3. Make followability permission-aware
+
+The current funnel checks discovery-advertised verbs, not the ServiceAccount's actual
+authorization:
+[internal/typeset/funnel.go](../../internal/typeset/funnel.go#L16).
+
+With narrow RBAC, a type can look followable in discovery and then fail with `403
+Forbidden` when the dynamic watch opens. That should become a clean refusal.
+
+Implementation shape:
+
+- collect effective permissions for relevant namespaces and cluster scope;
+- prefer `SelfSubjectRulesReview` for bulk rules;
+- use `SelfSubjectAccessReview` as a fallback for ambiguous wildcard or aggregated cases;
+- add permitted verbs to the type observation;
+- fail the verbs requirement with a new reason such as `ReasonNotPermitted`;
+- surface the reason on `WatchRule` and `ClusterWatchRule` status.
+
+This is not required to remove Secret values from the cache, but it is required to make
+least-privilege installs pleasant and diagnosable.
+
+### 4. Add an RBAC generator
+
+Add a command or task that reads manifests or live cluster state and emits the narrow role
+set for:
+
+- `GitProvider`;
+- `GitTarget`;
+- `WatchRule`;
+- `ClusterWatchRule`.
+
+Output:
+
+- fixed controller grants;
+- Secret input grants for referenced Git credentials and age-key Secrets;
+- generated Secret write grants where `generateWhenMissing` or signing-key generation can
+  create/update Secrets;
+- one watch grant per selected GVR;
+- no `*/*` wildcard when `--no-wildcard` is selected.
+
+Start with manifest input. It is reviewable in CI and does not require cluster access.
+Live-cluster input can come later.
+
+### 5. Use a 5-minute control-plane reconcile fallback
+
+The hardened default should not need Secret watches for controller-owned inputs. Instead,
+control-plane controllers should use a common 5-minute periodic reconcile:
+
+- credential or age-key rotation is picked up within 5 minutes if no related work happens
+  sooner;
+- branch workers still read credentials directly when work is happening;
+- generated-key recovery and recipient annotation refresh can wait for the next GitTarget
+  reconcile;
+- RBAC for controller-owned Secret inputs can drop `list/watch` and keep scoped `get`.
+
+This does not affect data-plane mirroring watches. Those still need `get,list,watch` for
+the resources the user chooses to mirror.
+
+### 6. Optional: namespace-scope Secret metadata watches
+
+After [secret-value-retention-plan.md](secret-value-retention-plan.md) switches to direct
+reads and a 5-minute reconcile fallback, metadata watches can be added back only for
+installs that need faster Secret rotation. Avoid adding a public flag first if a generated
+static namespace set is enough.
+
+```text
+--secret-watch-namespaces=ns1,ns2
+```
+
+If a flag is needed, prefer `--secret-watch-namespaces` over the issue's
+`--secret-namespaces`: it says exactly what is scoped. This should not create a full
+Secret value cache. It only scopes rotation reactivity. The matching RBAC still needs
+normal Secret `get,list,watch` permissions in those namespaces.
+
+## What not to do
+
+- Do not claim metadata-only watch lowers RBAC. It does not.
+- Do not reintroduce a namespace-scoped full Secret cache as the primary fix.
+- Do not require a Secret metadata watch by default when a 5-minute reconcile is enough.
+- Do not claim a namespaced Secret Role helps while `*/* get,list,watch` is still bound.
+- Do not replace the default wildcard install for everyone.
+- Do not use `resourceNames` as the main model for Secret watches; list/watch and rotation
+  behavior make that brittle.
+
+## Done
+
+This plan is complete when:
+
+- hardened static install docs exist;
+- users can opt out of mirroring Secrets;
+- missing watch permissions surface as rule status, not unexplained watch churn;
+- an RBAC generator can produce a no-wildcard role set for a concrete install;
+- public docs clearly distinguish RBAC blast radius from in-process Secret exposure.