villebro opened a new pull request, #324: URL: https://github.com/apache/superset-kubernetes-operator/pull/324
## Summary A Claude security scan flagged several reconcile paths that could delete or adopt Kubernetes resources the operator does not own, bypassing the existing `deleteIfNotForeignOwned` guard and the adoption/cleanup boundary documented in `docs/reference/security.md`. Because resource names are derived from the CR name, a CR author can arrange collisions that steer the manager's namespace-wide delete RBAC at resources they hold no permissions over. This PR closes that family of gaps with one consistent rule: every delete/adopt checks controller ownership. ## Details - **Lifecycle task Jobs.** The four in-reconcile delete paths (stale checksum, failed-retry, wedged pod, image mismatch) fetched a Job purely by name and called bare `Delete`. A CR named `db` could make the operator destroy and name-squat a foreign Job `db-init` (e.g. CronJob-owned) and misattribute its status as task state. All four now route through the guarded `deleteLifecycleJob`, and a Job at a managed task name controller-owned by a foreign owner is treated as a collision — never deleted, never read as task state — surfacing a `TaskNameCollision` condition and Warning event while the task waits. - **HPA/PDB label cleanup.** `deleteMatches` deleted every label match with no ownership check, and the HPA/PDB selector carried no reserved parent label, so a CR author could target a third party's generically-labeled HPA/PDB. `deleteMatches` now takes the parent owner, skips objects controller-owned by a foreign owner, and deletes with a UID precondition; the owner is threaded through `deleteByLabels`/`pruneOrphans`, and HPA/PDB creation labels plus the cleanup selector now include the reserved `superset.apache.org/parent` label. - **ServiceAccount adoption.** The adopt-refusal guard ran a separate `Get` outside the mutate closure and only checked ownership on success, so a transient error or a create-window race could let the operator adopt an unowned SA, wipe its annotations (e.g. cloud IAM bindings), and GC it with the CR. The check now runs inside the `CreateOrUpdate` mutate closure (atomic and re-evaluated per retry), and `isOwnedBy` uses `metav1.GetControllerOf` so it matches only a controller reference. A new CEL rule requires `serviceAccount.name == metadata.name` when the operator creates the SA, so operator-created SA names cannot be pre-registered across instances. ## Release notes - **Breaking:** CRs with `serviceAccount.create=true` (or unset) and `serviceAccount.name != metadata.name` are now rejected at admission. To use a pre-existing ServiceAccount with a different name, set `serviceAccount.create=false`. - **Upgrade window:** HPA/PDB objects created by a pre-patch operator lack the `superset.apache.org/parent` label until an enabled-path reconcile re-stamps them; label-based cleanup now skips (and logs) anything controller-owned by a foreign controller, and owner-reference GC still removes them on CR delete. Found via a Claude security scan. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
