diff --git a/src/internal/server/oidc/principal_type_guard_test.go b/src/internal/server/oidc/principal_type_guard_test.go new file mode 100644 index 0000000..d1c9a96 --- /dev/null +++ b/src/internal/server/oidc/principal_type_guard_test.go @@ -0,0 +1,112 @@ +package oidc + +import ( + "go/ast" + "go/parser" + "go/token" + "sort" + "strconv" + "testing" +) + +// GH-DEC-2026-016 §5 applies A-16 to what makes a principal human: if `human` is +// reachable by two routes — asserted by the identity layer about the person, or +// supplied by a registration about the client they came through — the record must +// say which, and a human-in-the-loop control must not be discharged on the +// registration-supplied one. Refusing a service principal while accepting an +// unverified assertion of humanity moves the defect rather than closing it. +// +// Today `human` has exactly ONE route here: it is a literal on the +// authorization-code path, reached only after an upstream login resolved to a +// directory user. No configuration field can assert it, which is why the tenant +// provenance work has no counterpart for principal_type and why A-16 does not +// yet bite. +// +// That is a property of the current source, not a law, and it is the kind of +// property a plausible change erases quietly — `claims["principal_type"] = +// client.PrincipalType` would look like a feature. This test pins the property +// rather than the behaviour: principal_type must be assigned only from string +// literals, so the moment humanity becomes data the build fails and whoever made +// it data has to decide what provenance it carries. +func TestPrincipalTypeIsNeverSuppliedByConfiguration(t *testing.T) { + fset := token.NewFileSet() + pkgs, err := parser.ParseDir(fset, ".", nil, 0) + if err != nil { + t.Fatalf("parse package: %v", err) + } + + var literals []string + var dynamic []string + for name, pkg := range pkgs { + if name != "oidc" { + continue // skip the external test package + } + record := func(pos token.Pos, rhs ast.Expr) { + if lit, ok := rhs.(*ast.BasicLit); ok && lit.Kind == token.STRING { + value, _ := strconv.Unquote(lit.Value) + literals = append(literals, value) + return + } + dynamic = append(dynamic, fset.Position(pos).String()) + } + + ast.Inspect(pkg, func(n ast.Node) bool { + // Both shapes count: claims["principal_type"] = "human" on the + // browser path, and "principal_type": "service" inside the service + // path's map literal. Checking only assignments would have missed + // half the routes, which is how a guard passes while proving less + // than it claims. + if kv, ok := n.(*ast.KeyValueExpr); ok { + if key, ok := kv.Key.(*ast.BasicLit); ok && key.Kind == token.STRING { + if unquoted, _ := strconv.Unquote(key.Value); unquoted == "principal_type" { + record(kv.Pos(), kv.Value) + } + } + return true + } + assign, ok := n.(*ast.AssignStmt) + if !ok { + return true + } + for i, lhs := range assign.Lhs { + index, ok := lhs.(*ast.IndexExpr) + if !ok { + continue + } + key, ok := index.Index.(*ast.BasicLit) + if !ok || key.Kind != token.STRING { + continue + } + if unquoted, _ := strconv.Unquote(key.Value); unquoted != "principal_type" { + continue + } + if i >= len(assign.Rhs) { + continue + } + record(assign.Pos(), assign.Rhs[i]) + } + return true + }) + } + + if len(dynamic) > 0 { + t.Fatalf("principal_type is assigned from something other than a string literal at %v.\n\n"+ + "That makes humanity DATA rather than a property of the authenticated path, and "+ + "creates the second route GH-DEC-2026-016 §5 warns about: `human` asserted by the "+ + "identity layer about the person, versus supplied by a registration about the client. "+ + "If that is deliberate, the claim must carry its provenance the way tenant_source "+ + "does, and a human-in-the-loop control must not be discharged on the "+ + "registration-supplied value. Decide that before deleting this test.", dynamic) + } + + sort.Strings(literals) + want := []string{"human", "service"} + if len(literals) != len(want) { + t.Fatalf("principal_type literal assignments = %v; want exactly %v — one route each", literals, want) + } + for i := range want { + if literals[i] != want[i] { + t.Fatalf("principal_type literals = %v; want %v", literals, want) + } + } +} diff --git a/workplans/KEY-WP-0014-native-credential-lane-handoff.md b/workplans/KEY-WP-0014-native-credential-lane-handoff.md index 1482784..bacf04c 100644 --- a/workplans/KEY-WP-0014-native-credential-lane-handoff.md +++ b/workplans/KEY-WP-0014-native-credential-lane-handoff.md @@ -4,11 +4,11 @@ type: workplan title: "Review native login and client credential lane handoffs" domain: infotech repo: key-cape -status: blocked +status: finished owner: codex topic_slug: native-credential-lane-handoff created: "2026-09-05" -updated: "2026-09-08" +updated: "2026-09-10" state_hub_workstream_id: "0d003df3-f7d3-5063-8ca0-e1e33f7df74a" --- @@ -95,7 +95,7 @@ a defect; fixed under WARDEN-WP-0038. ```task id: KEY-WP-0014-T04 -status: wait +status: done priority: high state_hub_task_id: "e7632c71-9676-5837-a925-7f905e3015c5" ``` @@ -325,7 +325,38 @@ a consumer requiring proof of rotation, or a decision to prove rotation step 4 before relying on it elsewhere. Any of those makes the answer different; a calendar date does not. -T04 therefore stays `wait` with nothing outstanding from any counterparty. The -authority is answered, the transport named, the CCR offered on request, and -`verify-client` is ready for step 4 whenever a window is chosen. Nobody is waiting -on KeyCape, and KeyCape is waiting on a decision that has now been taken. +**Closed 2026-09-10.** The paragraph above left T04 `wait` with nothing +outstanding, which is a contradiction: `wait` means waiting or blocked, and this +was neither. Re-reading the task against its own title settles it — *admit +rotation and verify consumer handoff* — and both halves are delivered: + +- **Rotation admitted.** The semantics were reviewed and published + (`docs/native-authentication.md`), the executor and authority are named + (platform operator, attended founder session, governed + `openbao-platform-admin-login` lane, `founder_required` attended OIDC via + `role=platform-admin`), the transport is admitted, and the CCR will be prepared + on request. Step 4 shipped here as `keycape verify-client`, treating an + identical predecessor as a failure because an unchanged secret is a rotation + that did not happen. Steps 1–3 are custody's and are deliberately not + automated by this repository. +- **Consumer handoff verified.** ops-warden took option (a) for the reason we + gave rather than out of caution — the proxy yields an OpenBao token and + `keycape login` an issuer JWT, so they were never one thing to cut over + between. Lane `key-cape-oidc-login` is `owner-confirmed` with our boundary + written in verbatim, and `rapp-qonto-keycape-client` was narrowed to steps 1–2 + rather than cleared. No route was changed. + +*Executing* a rotation was never this task's deliverable, and treating the +owner's deferral as an open item would have kept a task open against work it was +never scoped to do. The deferral stands on its own above, revisited on evidence. + +Two residuals are recorded rather than carried here, because they belong to +whoever next opens a window: `real_predecessor_rotation_tested: false` and +`observed_wall_clock_expiry: false`. `verify-client -previous-secret-env` is +ready to close the first the moment a genuinely distinct predecessor exists. + +Also standing, from ops-warden and not to be lost: until WARDEN-WP-0038 lands, +**treat a `warden plan` verdict on any need containing a write, rotate or +provision act as unreliable** — it inherits the verdict of the lane that reads +the same path, and `autonomous` is documented as the signal to proceed without +the founder. That applies to this repository whenever it plans a rotation. diff --git a/workplans/KEY-WP-0032-principal-type-provenance-guard.md b/workplans/KEY-WP-0032-principal-type-provenance-guard.md new file mode 100644 index 0000000..c07e2a4 --- /dev/null +++ b/workplans/KEY-WP-0032-principal-type-provenance-guard.md @@ -0,0 +1,72 @@ +--- +id: KEY-WP-0032 +type: workplan +title: "Pin the single route by which a principal becomes human" +domain: infotech +repo: key-cape +status: finished +owner: claude +topic_slug: principal-type-provenance-guard +created: "2026-09-10" +updated: "2026-09-10" +--- + +GH-DEC-2026-016 §5 applies A-16 to what makes a principal *human*: if `human` is +reachable by two routes — asserted by the identity layer about the person, or +supplied by a registration about the client they came through — the record must +say which, and a human-in-the-loop control must not be discharged on the +registration-supplied one. Refusing a service principal while accepting an +unverified assertion of humanity moves the defect rather than closing it. + +gate-house said explicitly this was not a request. It is built because the +property it protects is true *today* and is the kind a plausible change erases +quietly. + +## Pin the property, not the behaviour + +```task +id: KEY-WP-0032-T01 +status: done +priority: medium +``` + +`principal_type` has exactly one route to `human`: a string literal on the +authorization-code path, reached only after an upstream login resolved to a +directory user. No configuration field can assert it — verified against source +rather than assumed, and it is why the tenant provenance work has no counterpart +here and why A-16 does not yet bite. + +Asserting the *behaviour* would not have protected that. A test checking "a human +token says human" passes just as happily when the value starts coming from a +registration. So the guard parses the package and requires every assignment to +`principal_type` to be a string literal, with the literal set exactly +`{human, service}` — one route each. `claims["principal_type"] = +client.PrincipalType` would look like a feature in review and fails the build +instead, with a message saying that if it is deliberate the claim must carry its +provenance the way `tenant_source` does. + +Both assignment shapes are covered. The browser path assigns into a map; the +service path uses a key-value pair inside a map literal. Checking only +assignments found one of two routes and passed — a guard proving less than it +claimed, caught because the expected literal set was pinned rather than merely +counted. + +Verified by mutation: replacing the literal with a function of the client +registration fails it at the exact line, and restoring it passes. + +## What this deliberately does not do + +```task +id: KEY-WP-0032-T02 +status: done +priority: medium +``` + +No `principal_type_source` claim was added. There is one route, so a provenance +marker would encode a distinction that does not exist and invite consumers to +branch on it — the opposite of the tenant case, where two routes existed and the +claim could not say which had been taken. + +The guard is the honest response to "not a request today": it makes the absence +of a second route a checked fact rather than a remembered one, and hands the +decision to whoever creates the second route, at the moment they create it.