Review oidc-service-client 0.1 follow-through (canon §8, net-kingdom msg 44400f30)
All checks were successful
CI Smoke / host-smoke (push) Successful in 0s
CI Smoke / container-smoke (push) Successful in 1s

Change needed: R1 broker does not admit the type, R5 rotation lane, R6 catalog
reader fields, R7 consumer Secret-read verification; R8-R10 questions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Assistant: claude-code
Assistant-Model: opus
Assistant-Process: 5502@bnt-lap001
Assistant-Session: 68b3b02f-5d0f-4e71-8421-796f62e50196
This commit is contained in:
tegwick 2026-10-31 23:02:15 +01:00
parent c61c0d9bf0
commit 95d31c3177

View file

@ -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.