villebro opened a new pull request, #277:
URL: https://github.com/apache/superset-kubernetes-operator/pull/277
## Summary
Fixes two related access-control gaps (security-scan findings **f001** and
**f002**) that let a user holding only `supersets` create/update turn the
operator's namespace-wide RBAC against resources owned by *other* controllers.
Both contradict guarantees already documented in `docs/reference/security.md`
("controller-owner semantics prevent adopting resources already controlled by
another controller" and the cleanup semantics that permit deleting only unowned
same-name or operator-labeled resources), so this brings the code in line with
the documented threat model rather than changing it.
## Details
**f001 — web-server Service owner-reference wipe.**
`reconcileWebServerService` set `svc.OwnerReferences = nil` inside the
CreateOrUpdate mutate function before `controllerutil.SetControllerReference`,
defeating the `AlreadyOwnedError` guard (the only site in the package that did
this). A CR named to collide with a foreign controller-owned Service
`{name}-web-server` would seize it — selector/ports rewritten to the attacker's
pods — and garbage-collect it when the CR is deleted. The wipe is now scoped to
legacy `superset.apache.org`-group owner refs (the actual upgrade case) via
`stripLegacySupersetOwnerRefs`; foreign controller refs are preserved so
`SetControllerReference` fails with `AlreadyOwnedError` and surfaces the
conflict.
**f002 — name-derived deletes ignore ownership.** Several cleanup paths
(`deleteComponentResources`, `drainComponents`, `deleteMaintenanceResources`,
`deleteLifecycleJob`, and the empty-config ConfigMap branches) deleted purely
by CR-derived name with only `IgnoreNotFound`, so a foreign controller-owned
resource at a colliding managed name was deleted on every reconcile — a
persistent DoS the victim controller cannot win. A new
`deleteIfNotForeignOwned` helper `Get`s the live object, skips it when it is
controller-owned by a different owner, and otherwise deletes with a UID
precondition (closing the Get/Delete race). All by-name delete sites route
through it. Unowned same-name resources are still deleted, matching the
documented cleanup contract.
Adds regression tests: foreign-owner preservation + reconcile error, legacy
same-group owner adoption, and the ownership-gated delete predicate
(unowned/CR-owned deleted, foreign preserved, missing tolerated).
## Testing
- `go build ./...`, `gofmt`, `golangci-lint run` clean
- `go test ./internal/controller/...` passes, including the new
`TestDeleteIfNotForeignOwned`,
`TestReconcileWebServerService_DoesNotSeizeForeignOwnedService`, and
`TestReconcileWebServerService_AdoptsLegacySupersetOwnedService`
Part of a series of security-scan follow-ups; opened as a draft.
--
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]
