Skip to content

security(controller): scope serviceoffer-controller Secret RBAC to named secrets#570

Merged
bussyjd merged 1 commit into
mainfrom
security/controller-secrets-rbac-scope
May 30, 2026
Merged

security(controller): scope serviceoffer-controller Secret RBAC to named secrets#570
bussyjd merged 1 commit into
mainfrom
security/controller-secrets-rbac-scope

Conversation

@bussyjd
Copy link
Copy Markdown
Collaborator

@bussyjd bussyjd commented May 29, 2026

Summary

This PR hardens the serviceoffer-controller Secret RBAC after the agent-backed readiness/teardown fix in #572. It is intentionally stacked on fix/agent-backed-readiness because the RBAC change and the safe teardown guard share the same Secret ownership contract.

The old ClusterRole granted secrets: [get, create, update, patch] cluster-wide with no resourceNames. The reconciler only needs three named Secrets:

Secret Namespace Verbs used by code Why
litellm-secrets llm get read LITELLM_MASTER_KEY
hermes-api-server agent namespace get / create / delete preserve or mint the Hermes API token, then clean it up
remote-signer-keystore agent namespace get / create / delete preserve or mint the wallet keystore, then clean it up

RBAC before -> after

  - apiGroups: [""]
    resources: ["secrets"]
-   verbs: ["get", "create", "update", "patch"]          # cluster-wide
+   resourceNames: ["litellm-secrets"]
+   verbs: ["get"]
+
+ - apiGroups: [""]
+   resources: ["secrets"]
+   resourceNames: ["hermes-api-server", "remote-signer-keystore"]
+   verbs: ["get", "delete"]
+
+ - apiGroups: [""]
+   resources: ["secrets"]
+   verbs: ["create"]                                    # cannot be name-scoped

create stays unscoped because Kubernetes cannot authorize it with resourceNames before the object exists. All disclosure and destructive verbs are now scoped by name, and litellm-secrets is not deletable.

Companion changes

  • Adds Secret to isCreateOnlyKind, removing the need for Secret update / patch.
  • Removes the legacy in-place keystore-key migration.
  • Keeps fresh remote-signer-keystore Secrets labeled with app.kubernetes.io/instance and obol.org/agent, so Fix agent-backed service readiness reconciliation #572's ownership-checked teardown can safely delete only controller-owned resources.
  • Tightens TestServiceOfferControllerSecretRBAC_Scoped so it catches broad Secret reads, Secret update/patch/list/watch, and accidental delete access on litellm-secrets.

Tradeoff

Existing pre-fix unlabeled keystore Secrets are reused but not relabeled in place, because relabeling would require keeping Secret update or patch. Freshly-created keystores have the correct ownership labels.

Validation

  • go test ./internal/serviceoffercontroller ./internal/embed -count=1
  • go test ./internal/serviceoffercontroller ./internal/embed ./internal/x402 ./cmd/obol -count=1
  • go test ./internal/serviceoffercontroller/... ./internal/embed/... -count=1
  • kubectl --kubeconfig /private/tmp/obol-demo-kubeconfig apply --dry-run=server -f internal/embed/infrastructure/base/templates/x402.yaml

Release smoke was not run in this pass.

@bussyjd bussyjd force-pushed the security/controller-secrets-rbac-scope branch from 4aa3746 to f16a36f Compare May 29, 2026 23:05
@bussyjd bussyjd changed the base branch from main to fix/agent-backed-readiness May 29, 2026 23:05
@bussyjd bussyjd force-pushed the security/controller-secrets-rbac-scope branch from f16a36f to f395bd0 Compare May 29, 2026 23:06
@bussyjd bussyjd changed the base branch from fix/agent-backed-readiness to main May 30, 2026 08:26
…med secrets

The serviceoffer-controller ClusterRole granted secrets:[get,create,update,patch] cluster-wide with no resourceNames. The reconciler only needs three named Secrets:

  - litellm-secrets: get only for LITELLM_MASTER_KEY

  - hermes-api-server: get/create/delete for the agent API token

  - remote-signer-keystore: get/create/delete for the agent wallet keystore

This keeps create as its own unscoped rule because Kubernetes cannot resourceName-scope create, while all reads and deletes are name-scoped. Delete is intentionally limited to the two per-agent Secret names; litellm-secrets remains read-only.

The branch is stacked on the agent-backed readiness/teardown fix so Secret handling composes with ownership-checked teardown. Fresh remote-signer keystores keep the agent ownership labels needed by teardown, but the legacy in-place keystore migration is removed so the controller no longer needs update/patch on Secrets.

Tests cover the RBAC split, create-only Secret handling, existing wallet reuse, and agent teardown behavior.
@bussyjd bussyjd force-pushed the security/controller-secrets-rbac-scope branch from f395bd0 to d5b7fd9 Compare May 30, 2026 08:27
@bussyjd bussyjd marked this pull request as ready for review May 30, 2026 08:30
@bussyjd bussyjd merged commit da6bd41 into main May 30, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant