diff --git a/history/2026-10-31-oidc-service-client-follow-through-review.md b/history/2026-10-31-oidc-service-client-follow-through-review.md new file mode 100644 index 0000000..a391c9e --- /dev/null +++ b/history/2026-10-31-oidc-service-client-follow-through-review.md @@ -0,0 +1,202 @@ +# Review — `oidc-service-client` 0.1 `follow_through` (canon §8 owner review) + +Reviewed: net-kingdom `security-case-types/oidc-service-client/` (0.1, +proposed, net-kingdom commit `5d5f15d`): `type.yaml`, `README.md`, +`examples/nk-bind-executor-approval.yaml` and `decision/packet.md` (memo +`nk-wp-0056-t14-oidc-service-client-r1`, hub decision `f8044f81`). Compared +against `oidc-browser-relying-party` 1.1 and this repo's earlier review +(`history/2026-10-08-oidc-rp-follow-through-review.md`, R1–R8, R4′). Also checked +against the routing catalog (`registry/routing/catalog.yaml`), the +`keycape-client-secret-generate` playbook, and railiance-platform +`scripts/keycape_client_secret_generate.py` (source read only). +Reviewer: ops-warden, 2026-10-31. Request: hub message `44400f30`. + +**Verdict: change needed before acknowledgement.** The step order, the +value handling, and the custody design are sound, and they follow the browser +type's reviewed shape. A native per-call read with no Secret copy beside the +consumer is better than ESO delivery into the consumer's namespace. Three type +changes (R5, R6, R7) are needed. One external blocker (R1) must be recorded in +the type and memo, because `mint-secret` cannot run today. R8–R10 are +questions for net-kingdom, and the founder should see their answers in memo r2. + +## Lanes and value handling + +**R1 — `mint-secret` lane does not admit this type yet. Verdict: change needed +(record it as a blocker; the fix belongs to railiance-platform).** The lane +`keycape-client-secret-generate` is active, but the broker behind it refuses +this type in two places: + +- `ACKNOWLEDGED_TYPES` contains only `oidc-browser-relying-party@1.1`, pinned by + source hash and parameter fingerprint. Any `oidc-service-client@0.1` case + exits 2 with `case_type_unacknowledged`. +- `check_case` requires `parameters.client_type == "confidential"`. This type + has no `client_type` parameter, because every service client is confidential. + The case therefore exits 2 with `case_client_not_confidential`, even after the + type is admitted. + +railiance-platform needs a separately reviewed admission binding for +`oidc-service-client@0.1`. That binding needs: + +- the type's pin and parameter fingerprint; +- a confidential check that follows from the type, not from a parameter. + +The broker's own comment already asks for this ("a future … entry requires real +acknowledgment and a separately reviewed admission binding"). This should not be +worked around by adding a fixed `client_type` parameter to the type just to +satisfy the check. + +Until that binding lands, the type, README and memo should state that +`mint-secret` is blocked on railiance-platform admission. Once railiance-platform +confirms, ops-warden will add this type to the catalog entry's `need_keywords` +and to "Used by" in the playbook. + +**R2 — `mint-secret`: `warden_executes: true`, `attended: false`, +`generated_in_custody`. Verdict: ok, with one clarification.** This matches the +narrow canon §3 definition (R2 of the browser review). ops-warden starts the +owner's broker command as a child process in the caller's own environment and +holds no token. The value is generated inside custody, written once with CAS 0, +and verified from metadata only. + +The clarification: `attended: false` means "no interactive change to a live +system". It does not mean "no human". The issuer role needs a +`net-kingdom-admins` aal2 OIDC login, and the broker refuses `WARDEN_AGENT_ID` +and root. So under NK-WP-0056, the bounded executor cannot mint its own client +secret. That is correct, and the README should say it. + +**R3 — `deliver-secret`. Verdict: ok.** The ESO role is bound to +`external-secrets/external-secrets` only, with a 15m TTL, one policy, and no +default policy. It reads one path, with no metadata or list access. The +`ready_check` reads status only, which matches browser R5. The step has no +`lane` field, the same as browser 1.1. The draft catalog lane +`platform-secret-delivery` (CATALOG-1, 2026-10-17) belongs to the browser type's +1.3 work. Adopt it here only when browser 1.3 does. + +**R4 — `bind-env` and `register-client` attended, in one `key-cape-oidc-login` +session. Verdict: ok for 0.1; see R9 for the forward path.** Both justifications +are accepted for the same reasons as the browser type: + +- The live client registry is a Kubernetes Secret. +- KeyCape resolves the secret at start and fails closed. +- `bind-env` runs before `register-client` in one session. +- The run goes in a quiet window. +- `keycape check-config` runs before the restart. + +ops-warden groups these steps and does not execute them. The type also lists the +consumer's possible actors (another workload in the namespace, a coding agent, +a compromised consumer), and it attends exactly the steps that touch a Secret. + +## Catalog entry + +**R5 — Rotation in the catalog-entry body points at a lane that refuses +rotation. Verdict: change needed.** The body says `rotation: mint-secret then +verification`, and the README says "Rotation is mint-secret, then the consumer's +next exchange". But `keycape-client-secret-generate` covers first generation +only. It refuses an existing path (exit 3), and its own catalog entry routes +rotation to railiance-platform as a task that does not exist yet. + +Change the body to: + +- `rotation: {method: rotate, owner: railiance-platform, automatable: false}`; +- a step that says no rotation lane exists yet, and that a rotation is a CAS + write at the custody path plus an attended KeyCape restart (KeyCape reads the + secret at start). + +Correct the README limit to match. Writing a lane name that does not exist into +every per-case entry is the failure mode ADR-0001 warns about. + +**R6 — Entry name `{consuming_service}-keycape-{client_id}`, and the missing +reader fields. Verdict: ok on the name; change needed on the body.** + +*Name.* Adding `{client_id}` is right for this type. The type allows one +audience per case, so one consumer can hold several cases. The browser shape +`{consuming_service}-keycape-client` would collide on the second one. The +example yields `bounded-executor-keycape-nk-bind-executor-approval`, which is +unique and readable. `workload_ref` can point at the published declaration +`net-kingdom/tools/service-identity/declarations/bounded-executor.yaml`, so the +explicit unknown form is not needed for this case. + +*Body.* The body omits the fields that tell `warden route` / `warden access` +how the consumer reads the secret, and that there is no human fetch path. Add: + +- `lane: secret` +- `auth_method: "Kubernetes auth role workload-kv-read-keycape-{client_id}-{consuming_service}, ServiceAccount {consumer_namespace}/{consumer_service_account}, audience openbao, 15m; KeyCape reads through ESO role external-secrets-keycape-{client_id}"` +- `exec_capable: false` and no `fetch_command`. Only the workload reads the + secret. Without this, a catalog consumer could infer an operator + `bao kv get` route, the way `rapp-qonto-keycape-client` has one. +- `need_keywords` drawn from the case: `{consuming_service}`, `{client_id}`, + `{audience}`, `client-credentials`, `service-token`. Keep them case-specific, + as with the `keycape-client-secret-generate` lesson: broad phrases outrank + named lanes. +- `subsystem: KeyCape + OpenBao` + +`owner_repo: key-cape` and `delegation: native, intended_owner key-cape` are +consistent with browser R4′. Keep them. + +## Verification + +**R7 — The claim "no Secret copy in the consumer's namespace" does not hold for +the first case, and nothing verifies it. Verdict: change needed.** In +`nk-bind-executor-approval`, `consumer_namespace` is `sso`. That is the same +namespace where `deliver-secret` writes `sso/keycape-{client_id}-client` for +KeyCape. So the copy does exist beside this consumer. It is acceptable only +because ServiceAccount `sso/nk-bind-executor` has no Secret access: its Role +covers `deployments/authelia` and `externalsecrets/authelia-secrets` only. The +type relies on that fact but never checks it. Add: + +- verification `consumer-cannot-read-delivered-secret`: + `kubectl auth can-i get secrets/keycape-{client_id}-client -n sso --as system:serviceaccount:{consumer_namespace}:{consumer_service_account}` + must return `no`. Also check `list` and `watch` on secrets in `sso`. Set + `expect: deny` and `reads_secret_value: false`. +- in the README threat model, change "no Secret copy in the consumer's + namespace" to "the consumer has no Secret read anywhere; when + `consumer_namespace` is `sso`, the KeyCape copy is beside it and is protected + by RBAC alone". + +**R8 — `service-token-claims` is reported by the consumer. Verdict: question.** +"Claims only; the verifier reads no secret and no token" is the right boundary, +and ops-warden accepts consumer-reported claims as evidence. But the consumer is +also the party under test. Two questions: + +1. Can a second, consumer-independent check be added that reads the non-secret + KeyCape client view in the attended `register-client` session? It would + confirm `grant_types` = `client_credentials` only, no redirect URIs, and that + `serviceSubject`, `tenant`, `audience`, `scopes` and lifetime match the case. + The browser type does this with `mfa-pinned`. +2. Is `keycape verify-client` (key-cape `docs/native-authentication.md`) the + intended owner command for `service-token-claims` and `excess-scope-refused`? + It already reports a positive exchange and excess-scope denial without + printing tokens. If so, name it as the entry point, so the consumer's report + has a fixed shape. For the bounded executor, also say whether these test + exchanges need their own admission. Under NK-WP-0056 the executor acts only + under a consumed admission. + +## Ownership and forward path + +**R9 — Attended lane: `key-cape-oidc-login` or `keycape-attended-case-change`. +Verdict: question.** Since 2026-10-17 (OW-1.3-C1) the catalog has had a draft +`keycape-attended-case-change` lane, with `key-cape-oidc-login` as its +`session_lane`. It separates the change authority from the login. 0.1 correctly +follows the admitted browser 1.1. Please confirm that 0.1 moves to the new lane +in step with the browser type once that lane is promoted, and not on its own. + +**R10 — `hand-over` owner. Verdict: question.** +`sso-mfa/k8s/bounded-executor/rbac.yaml` says the executor's packaging and apply +belong to `rapp-bounded-executor` (ADR-0015, founder decision `6d1adce9`). The +first case sets `consumer_owner_repo: net-kingdom`. If the consumer's +configuration (issuer, client, audience, OpenBao role) ships in the rapp, then +`consumer_owner_repo` should be `rapp-bounded-executor`. It names both the +`hand-over` owner and the rollback owner. + +## Not in scope for ops-warden + +Not reviewed here: + +- `materialization` of the policies and roles: railiance-platform CCRs, as for + the browser type; +- the scope set `approval:create,approval:read,approval:consume`, and whether + one workload should both create and consume approvals: approval-engine and the + founder decide that; +- the `audience`-to-scope check named under "Known limits". + +The rollback order is sound: KeyCape before custody, roles before policies, and +the ExternalSecret before the path.