diff --git a/config/milo/iam/resources/capabilitybindings.yaml b/config/milo/iam/resources/capabilitybindings.yaml index 71cfaa1..ff0cc34 100644 --- a/config/milo/iam/resources/capabilitybindings.yaml +++ b/config/milo/iam/resources/capabilitybindings.yaml @@ -88,24 +88,20 @@ spec: # cache miss; watch is declared for `kubectl get -w` # and for the catalog's own controller, which watches # what it projects. - # patch the assistant's status writeback. A status update is - # a PATCH to the status subresource, so the review - # arrives with verb `patch`; without it here the write - # 403s and the feedback loop this design exists to - # build is silently dead. `update` covers the - # update-shaped path and is already present for the - # catalog. + # patch retained for compatibility while subresource + # authorization is off. The publisher Role does not + # grant this base permission. # - # Note what is NOT expressible here: this list has no subresource axis, so it - # cannot distinguish "patch the spec" from "patch status". The split is - # enforced by the ROLES instead — capability-publisher omits patch, so the - # catalog cannot write status; see config/milo/iam/roles/. Ordinary Kubernetes - # RBAC *does* have a subresource axis, which is why the assistant's own grant - # (../../rbac/control-plane-auth-delegator.yaml) can be get/list on the object - # and patch on capabilitybindings/status only, and this list cannot. Whether - # Milo's authorizer evaluates a status write as `capabilitybindings.patch` on - # the parent (assumed here) or as a distinct subresource permission still needs - # confirmation for the CATALOG's side; it no longer blocks the assistant. + # The CRD serves GET, PUT and PATCH on /status. Register them separately so + # enabling Milo subresource authorization requires an explicit status grant. + # Customer and publisher IAM Roles receive none; normal reads still include + # the assistant's reported conditions. Its own writes use Kubernetes RBAC. + subresources: + - name: status + permissions: + - get + - update + - patch permissions: - create - update diff --git a/config/milo/iam/roles/capability-publisher.yaml b/config/milo/iam/roles/capability-publisher.yaml index fcc645f..1f1f10d 100644 --- a/config/milo/iam/roles/capability-publisher.yaml +++ b/config/milo/iam/roles/capability-publisher.yaml @@ -9,39 +9,17 @@ # project's assistant can reach. It is not a role for a human project member; # the read-only capability-viewer below is. # -# Note what it does NOT include: `patch`. CONFIRMED CORRECT, 2026-09, and it is -# better than the house convention — which is inert. +# The publisher manages binding specifications and receives no /status +# permissions. With Milo subresource authorization enabled, only an explicit +# capabilitybindings/status.update or .patch grant can change the assistant's +# Accepted and Composed conditions. Leaving patch out of the base permissions +# alone is not a status boundary: legacy authorization also accepts base update +# for PUT /status. Enforcement remains off by default until the platform enables +# the provider feature. # -# service-catalog and ipam-range both declare an `updateStatus` permission on -# their ProtectedResources and include it in admin roles -# (service-catalog/config/components/iam/protected-resources/service.yaml:19, -# ipam-range/config/components/iam/roles/ipam-admin.yaml:22-24). Nothing ever -# checks it: Milo's authorizer builds the permission from -# attributes.GetVerb() (openfga-provider/internal/webhook/ -# subjectaccessreview_authorizer.go:611), and GetVerb() returns Kubernetes verbs -# — get/list/create/update/patch/delete/watch — never `updateStatus`. A status -# write arrives as a PATCH (or an UPDATE) and is checked as plain `patch` on the -# parent, so `updateStatus` is a permission no request can ever request and -# `patch` silently confers status-write to anyone holding it. Roles that grant -# `patch` "for spec edits" and withhold `updateStatus` "to protect status" have -# it exactly backwards. -# -# Withholding `patch` is therefore the only thing that actually keeps a producer -# off status on this axis. -# -# The assistant writes its report back -# about what it validated and composed (the Accepted and Composed conditions) by -# PATCHing the binding's status subresource. The assistant's own authority for -# that does NOT come from this axis — Milo IAM cannot express a grant for a -# certificate identity at all, so the assistant is authorized on the RBAC leg -# (../../rbac/control-plane-auth-delegator.yaml, and ./capability-viewer.yaml for -# the full reasoning). Milo's permission list carries no subresource axis -# (see ../resources/capabilitybindings.yaml), so withholding `patch` from this -# role is what keeps the producer off status here; the RBAC grant can and does -# use the subresource axis, giving the assistant patch on -# capabilitybindings/status only. A producer that could write status -# could claim its binding was accepted when it was skipped, which would defeat -# the entire point of having conditions. +# The assistant's own status writeback uses the existing Kubernetes RBAC grant +# in ../../rbac/control-plane-auth-delegator.yaml: get/list on bindings and patch +# on capabilitybindings/status. This Role does not authorize that identity. apiVersion: iam.miloapis.com/v1alpha1 kind: Role metadata: diff --git a/config/milo/rbac/control-plane-auth-delegator.yaml b/config/milo/rbac/control-plane-auth-delegator.yaml index d5e7ad1..cbf20ed 100644 --- a/config/milo/rbac/control-plane-auth-delegator.yaml +++ b/config/milo/rbac/control-plane-auth-delegator.yaml @@ -106,13 +106,10 @@ rules: # (docs/enhancements/crd-capability-source.md, "Rejected alternatives"). # # - # The spec/status split below is expressible only because ORDINARY KUBERNETES - # RBAC HAS A SUBRESOURCE AXIS. Milo IAM's permission list does not (see - # ../iam/resources/capabilitybindings.yaml), which is why the equivalent - # separation over there has to be done by withholding `patch` from a whole - # role. Here the assistant can be given get/list on the object and patch on - # /status ONLY: a service that could rewrite the spec it was handed is not - # reading an entitlement, it is granting itself one. + # The assistant reads binding specifications and patches only their status. + # This certificate identity uses Kubernetes RBAC; the separate Milo IAM status + # registration does not change its grant. A service that could rewrite the + # spec it was handed could grant itself an entitlement. - apiGroups: ["capabilities.assistant.miloapis.com"] resources: ["capabilitybindings"] verbs: ["get", "list"] diff --git a/config/overlays/dev-crd/binding-reader-rbac.yaml b/config/overlays/dev-crd/binding-reader-rbac.yaml index 77c890a..010e120 100644 --- a/config/overlays/dev-crd/binding-reader-rbac.yaml +++ b/config/overlays/dev-crd/binding-reader-rbac.yaml @@ -25,15 +25,11 @@ # never on the parent. The assistant writes conditions back onto a binding (the # feedback loop to whoever authored it) and must never be able to edit the spec # it was handed: a service that can rewrite its own entitlement is not reading an -# entitlement. Ordinary Kubernetes RBAC has a subresource axis and both this -# grant and the production one use it; Milo IAM does not, which is why the -# equivalent split on that axis has to be done by withholding `patch` from a -# whole role (config/milo/iam/roles/capability-publisher.yaml). These rules are -# verb-for-verb identical to the production ClusterRole's, so a writeback that -# works here is at least not failing on the verb set — it may still 403 in -# production if the root-RBAC-authorizes-project-paths assumption is wrong, which -# logs capability.crd.status_forbidden, deliberately its own event rather than -# folded into generic write errors. +# entitlement. These Kubernetes RBAC rules match the production ClusterRole. +# Milo IAM separately registers status permissions for UID-bearing callers; +# enabling that feature does not change this certificate identity's RBAC grant. +# A failed status write logs capability.crd.status_forbidden separately from +# generic write errors. apiVersion: rbac.authorization.k8s.io/v1 kind: ClusterRole metadata: diff --git a/config/overlays/production/alerts.yaml b/config/overlays/production/alerts.yaml index 6d5b0eb..ead028d 100644 --- a/config/overlays/production/alerts.yaml +++ b/config/overlays/production/alerts.yaml @@ -265,15 +265,10 @@ spec: var. See the runbook. - alert: AssistantCapabilityStatusWriteForbidden - # A named hypothesis, instrumented. Status conditions are written with - # a merge PATCH to capabilitybindings/status, and the grant assumes - # Milo evaluates that as `patch` on the parent resource — the - # updateStatus convention that appears across the platform is inert - # (GetVerb never returns it), so no SubjectAccessReview can match it. - # If that assumption is wrong, EVERY status write 403s and nothing - # else is visibly wrong: it presents as "conditions never appear", - # which reads like a bug in the writer, in the catalog, or in nobody. - # This rule makes it say so instead. + # The assistant patches capabilitybindings/status using its dedicated + # Kubernetes RBAC grant. Missing status permission prevents Accepted + # and Composed conditions from reaching providers, even when binding + # reads and conversation requests continue to succeed. # # 403 is never normal here, so the threshold is zero and for: 5m # exists only to require a second occurrence rather than a single diff --git a/docs/enhancements/crd-capability-source.md b/docs/enhancements/crd-capability-source.md index 691fd3d..57c4025 100644 --- a/docs/enhancements/crd-capability-source.md +++ b/docs/enhancements/crd-capability-source.md @@ -325,19 +325,17 @@ Status writes are a `PATCH` to the binding's `status` subresource through the identity. The LIST that produced the document already yielded the object's name, which — with the project in the path — is all a status patch needs. -**A note on the subresource axis, because it is a platform trap.** An earlier -version of this document said Milo's permission model "has no subresource -axis". That is not quite right, and the truth is worse. A subresource-flavoured -*convention* does exist and is used across `service-catalog` and `ipam-range`: -permissions named `updateStatus`. It is **inert** — `GetVerb()` never returns -`updateStatus`, so no SubjectAccessReview can ever match such a permission, -while plain `patch` silently confers the ability to write status. Anyone reading -a `ProtectedResource` would reasonably conclude that granting `updateStatus` -grants status writes and that withholding it withholds them; neither is true. -Our `capability-publisher` role therefore withholds `patch` outright rather than -relying on the convention (`config/milo/iam/roles/capability-publisher.yaml`), -which is correct by accident of not trusting it. This is a latent platform bug -worth reporting upstream; it is not this design's to fix. +Milo registers `capabilitybindings/status.get`, `.update`, and `.patch` +separately from binding permissions. With subresource authorization enabled, +customer and capability-publisher Roles cannot access `/status`; neither receives +these grants. Customers can still read conditions through ordinary binding reads. +The assistant continues to patch conditions through its Kubernetes RBAC grant. + +The provider feature remains off by default. Until it is enabled, IAM checks +status requests against base resource verbs: withholding `patch` alone does not +prevent a publisher with `update` from issuing `PUT /status`. The historical +`updateStatus` permission does not correspond to a Kubernetes request verb and +must not be used as a status authorization boundary. The writes are **rate-limited and coalesced** — a turn must not issue an API write per binding per message, and this matters more without an informer, since diff --git a/docs/runbooks/capability-source-degraded.md b/docs/runbooks/capability-source-degraded.md index b47670a..4bb27e6 100644 --- a/docs/runbooks/capability-source-degraded.md +++ b/docs/runbooks/capability-source-degraded.md @@ -149,15 +149,16 @@ assumption. and the ClusterRole grants `patch` on `capabilitybindings/status` *only* — never on the parent, because a service that can rewrite the spec it was handed -is not reading an entitlement, it is granting itself one. That split is -expressible because ordinary Kubernetes RBAC has a subresource axis. **Milo IAM -does not** — its `updateStatus`-style permission convention is inert (`GetVerb` -never returns it), so on that leg a status write is just `patch` on the parent. -The open question is which leg answers, and whether the project router's -`RequestInfo` recomputation preserves the `status` subresource across the -prefix strip. If it does not, the request authorizes as plain `patch` on -`capabilitybindings`, which this identity does not have, and **every** status -write 403s. +is not reading an entitlement, it is granting itself one. The assistant's +certificate uses this Kubernetes RBAC grant. Milo IAM also registers separate +status permissions, enforced only when the provider's subresource authorization +feature is enabled; no customer or publisher Role receives them. With that +feature off, IAM still checks status requests against base resource verbs. + +For a failed writeback, verify that the project router preserves the `status` +subresource and that the assistant's identity has the dedicated RBAC grant. +Losing the subresource would check `patch` on the parent binding, which this +identity does not have. **How to test it, in this order:**