fix(decision): registry facts win over caller-supplied attributes
secrets-engine's first live request rejected our allow: binding. request_digest is computed over material they never sent, because we enrich subject and resource from the registry before hashing. Answering that meant reading the enrichment path, which had a worse defect in it. Enrichment was additive-if-absent — addAttribute wrote a registry value only where the request had no value for that key. So where a caller supplied a key, the caller's value won and the registry's never applied. Every registry ceiling and allowlist was advisory. Verified against the shipped ops-warden package, each one added key on an otherwise-denied request: max_ttl_hours: 99 registry says 8 -> allowed a 12h certificate allowed_principals registry allowlist -> disallowed_principal bypassed allowed_subjects registry allowlist -> unknown_subject bypassed The third is the one to read twice: a subject the registry does not know authorized itself by naming itself in the allowlist it was being checked against. Not remotely reachable today — the PEP builds the CheckRequest, ops-warden sends no resource.attributes, and enforce admits one identity. It is a defence-in-depth failure: any path that lets attacker-influenced data into a CheckRequest field became a full policy bypass rather than a bounded input problem. Callers sending resource.attributes is not hypothetical; secrets-engine does it on every request. Registry facts now win, and diagnostics.registry_overrode names every displaced key, because a registry that silently discards a contradicting claim hides that a caller asserted authority it did not have. subject.type is carved out, and the reason is a finding of its own. Making the registry win there denied every secrets-engine allow: the registry's type is CARING vocabulary (Human, Agent, Automation, Service) and the request's is the protected system's actor vocabulary (service, adm, agt, atm). Two fields sharing a name; substituting one for the other is translation rather than identity, which GH-DEC-2026-008 ruled against. Note what surfaced it — the registry's type had been dead data since the field existed, because the caller's value always won. Also publishes binding.submitted_request_digest, over the request exactly as sent. request_digest was published as the consumer replay test and cannot be one. Nothing is lost hashing the pre-enrichment form: enrichment is a function of the request and the snapshot, and registry_snapshot_digest already pins the snapshot. Existing pins do not move. All three replay fixtures' request_digest and approval_binding_digest values are byte-identical — those requests contradict no registry fact. A field to add, not a value to correct. Regression tests verified failing against the old behaviour before being kept. FLEX-DEC-2026-012; FLEX-WP-0025 carries the residual, that a policy still cannot tell a fact from an assertion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aQMM1dPXaPiXVn6DwwtLd Assistant: claude-code Assistant-Model: opus Assistant-Process: 715613@bnt-lap001 Assistant-Session: fabd95c1-4c9e-4080-8849-8707ae025f80
This commit is contained in:
parent
c861d75703
commit
0bc624ba62
14 changed files with 822 additions and 34 deletions
|
|
@ -1,6 +1,7 @@
|
|||
package decision
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"crypto/sha256"
|
||||
"encoding/hex"
|
||||
|
|
@ -8,6 +9,7 @@ import (
|
|||
"fmt"
|
||||
"reflect"
|
||||
"slices"
|
||||
"sort"
|
||||
"strings"
|
||||
"sync"
|
||||
"time"
|
||||
|
|
@ -110,7 +112,7 @@ func (e *Engine) Check(ctx context.Context, request api.CheckRequest) (api.Decis
|
|||
return api.DecisionEnvelope{}, err
|
||||
}
|
||||
|
||||
decision := e.envelope(normalized, expectation, facts)
|
||||
decision := e.envelope(normalized, request, expectation, facts)
|
||||
if err := e.recordDecision(decision); err != nil {
|
||||
return api.DecisionEnvelope{}, err
|
||||
}
|
||||
|
|
@ -202,12 +204,19 @@ type registryFacts struct {
|
|||
resource api.Resource
|
||||
matchedRelationship string
|
||||
descriptor *api.CaringAccessDescriptor
|
||||
// overriddenByRegistry names request attribute keys whose caller-supplied
|
||||
// value was displaced by a registry fact. Empty on an honest request; a
|
||||
// non-empty list means the caller asserted something the registry contradicts,
|
||||
// which is worth seeing even though the registry won (FLEX-DEC-2026-012).
|
||||
overriddenByRegistry []string
|
||||
}
|
||||
|
||||
func (e *Engine) normalizeRequest(request api.CheckRequest) (api.CheckRequest, registryFacts) {
|
||||
normalized := request
|
||||
facts := registryFacts{}
|
||||
|
||||
overridden := []string{}
|
||||
|
||||
if normalized.Tenant != "" {
|
||||
if normalized.Subject.Tenant == "" {
|
||||
normalized.Subject.Tenant = normalized.Tenant
|
||||
|
|
@ -220,21 +229,24 @@ func (e *Engine) normalizeRequest(request api.CheckRequest) (api.CheckRequest, r
|
|||
if subject, ok := e.store.Subject(request.Subject.ID); ok {
|
||||
facts.subjectFound = true
|
||||
facts.subject = subject
|
||||
normalized.Subject = enrichSubjectRef(request.Subject, subject)
|
||||
normalized.Subject = enrichSubjectRef(request.Subject, subject, &overridden)
|
||||
}
|
||||
|
||||
if resource, ok := e.store.Resource(request.Resource.System, request.Resource.ID); ok {
|
||||
facts.resourceFound = true
|
||||
facts.resource = resource
|
||||
normalized.Resource = enrichResourceRef(request.Resource, resource)
|
||||
normalized.Resource = enrichResourceRef(request.Resource, resource, &overridden)
|
||||
}
|
||||
|
||||
if normalized.CaringContext != nil {
|
||||
descriptor := *normalized.CaringContext
|
||||
facts.descriptor = &descriptor
|
||||
facts.overriddenByRegistry = overridden
|
||||
return normalized, facts
|
||||
}
|
||||
|
||||
facts.overriddenByRegistry = overridden
|
||||
|
||||
if descriptor, relationshipID := e.matchCaringDescriptor(normalized, facts); descriptor != nil {
|
||||
facts.descriptor = descriptor
|
||||
facts.matchedRelationship = relationshipID
|
||||
|
|
@ -273,40 +285,67 @@ func (e *Engine) matchCaringDescriptor(request api.CheckRequest, facts registryF
|
|||
return nil, ""
|
||||
}
|
||||
|
||||
func enrichSubjectRef(ref api.SubjectRef, subject api.Subject) api.SubjectRef {
|
||||
// enrichSubjectRef overlays registry facts onto the subject the caller sent.
|
||||
//
|
||||
// Registry facts WIN. A value in the registry is an authority statement; the
|
||||
// same key on the request is the caller's proposal about itself. Letting the
|
||||
// proposal survive made a registry ceiling advisory — see FLEX-DEC-2026-012,
|
||||
// where a caller supplying resource.attributes.allowed_subjects authorized a
|
||||
// subject the registry did not know.
|
||||
//
|
||||
// Keys the registry does not define still pass through from the request. That
|
||||
// residual is stated in docs/request-enrichment.md rather than fixed here: a
|
||||
// policy reading a key its manifest omits is consuming caller input, and the
|
||||
// structural fix is separating the two namespaces (FLEX-WP-0025).
|
||||
func enrichSubjectRef(ref api.SubjectRef, subject api.Subject, overridden *[]string) api.SubjectRef {
|
||||
out := ref
|
||||
// subject.Type is deliberately NOT overwritten, unlike every other registry
|
||||
// fact here. The registry's Type is CARING vocabulary (Human, Agent,
|
||||
// Automation, Service); the request's is the protected system's actor
|
||||
// vocabulary (service, adm, agt, atm). They are two different fields sharing
|
||||
// a name, and substituting one for the other is translation rather than
|
||||
// identity — the error GH-DEC-2026-008 ruled against. Making the registry win
|
||||
// here denied every secrets-engine allow, because "Service" is not "service".
|
||||
//
|
||||
// The residual is that a caller can still assert its own subject.type and
|
||||
// packages branch on it. That is not fixable by overwriting from a different
|
||||
// vocabulary; it needs the two fields separated. FLEX-WP-0025.
|
||||
if out.Type == "" {
|
||||
out.Type = subject.Type
|
||||
}
|
||||
if out.Tenant == "" {
|
||||
if subject.Tenant != "" {
|
||||
out.Tenant = subject.Tenant
|
||||
}
|
||||
out.Attributes = copyMap(out.Attributes)
|
||||
addAttribute(out.Attributes, "display_name", subject.DisplayName)
|
||||
addAttribute(out.Attributes, "organization_relation", subject.OrganizationRelation)
|
||||
addAttribute(out.Attributes, "roles", subject.Roles)
|
||||
addAttribute(out.Attributes, "groups", subject.Groups)
|
||||
addAttributes(out.Attributes, subject.Claims)
|
||||
addAttributes(out.Attributes, subject.Metadata)
|
||||
setAuthoritative(out.Attributes, "display_name", subject.DisplayName, "subject", overridden)
|
||||
setAuthoritative(out.Attributes, "organization_relation", subject.OrganizationRelation, "subject", overridden)
|
||||
setAuthoritative(out.Attributes, "roles", subject.Roles, "subject", overridden)
|
||||
setAuthoritative(out.Attributes, "groups", subject.Groups, "subject", overridden)
|
||||
setAuthoritatives(out.Attributes, subject.Claims, "subject", overridden)
|
||||
setAuthoritatives(out.Attributes, subject.Metadata, "subject", overridden)
|
||||
return out
|
||||
}
|
||||
|
||||
func enrichResourceRef(ref api.ResourceRef, resource api.Resource) api.ResourceRef {
|
||||
// enrichResourceRef overlays registry facts onto the resource the caller sent.
|
||||
// Registry facts win, for the reason given on enrichSubjectRef: ops-warden's
|
||||
// max_ttl_hours, allowed_subjects, and allowed_principals are ceilings and
|
||||
// allowlists, and a ceiling a caller can raise is not a ceiling.
|
||||
func enrichResourceRef(ref api.ResourceRef, resource api.Resource, overridden *[]string) api.ResourceRef {
|
||||
out := ref
|
||||
if out.Type == "" {
|
||||
if resource.Type != "" {
|
||||
out.Type = resource.Type
|
||||
}
|
||||
out.Attributes = copyMap(out.Attributes)
|
||||
addAttribute(out.Attributes, "path", resource.Path)
|
||||
addAttribute(out.Attributes, "parent", resource.Parent)
|
||||
addAttribute(out.Attributes, "labels", resource.Labels)
|
||||
addAttribute(out.Attributes, "trust_zone", resource.TrustZone)
|
||||
addAttribute(out.Attributes, "owner", resource.Owner)
|
||||
addAttributes(out.Attributes, resource.Attributes)
|
||||
setAuthoritative(out.Attributes, "path", resource.Path, "resource", overridden)
|
||||
setAuthoritative(out.Attributes, "parent", resource.Parent, "resource", overridden)
|
||||
setAuthoritative(out.Attributes, "labels", resource.Labels, "resource", overridden)
|
||||
setAuthoritative(out.Attributes, "trust_zone", resource.TrustZone, "resource", overridden)
|
||||
setAuthoritative(out.Attributes, "owner", resource.Owner, "resource", overridden)
|
||||
setAuthoritatives(out.Attributes, resource.Attributes, "resource", overridden)
|
||||
return out
|
||||
}
|
||||
|
||||
func (e *Engine) envelope(request api.CheckRequest, expectation api.DecisionExpectation, facts registryFacts) api.DecisionEnvelope {
|
||||
func (e *Engine) envelope(request, submitted api.CheckRequest, expectation api.DecisionExpectation, facts registryFacts) api.DecisionEnvelope {
|
||||
envelope := api.DecisionEnvelope{
|
||||
RequestID: request.ID,
|
||||
Effect: expectation.Effect,
|
||||
|
|
@ -315,7 +354,7 @@ func (e *Engine) envelope(request api.CheckRequest, expectation api.DecisionExpe
|
|||
MatchedRule: expectation.Reason,
|
||||
Resource: request.Resource,
|
||||
Subject: request.Subject,
|
||||
Binding: api.NewDecisionBinding(request),
|
||||
Binding: api.NewDecisionBindingFor(request, submitted),
|
||||
Obligations: expectation.Obligations,
|
||||
Diagnostics: map[string]any{
|
||||
"action": request.Action,
|
||||
|
|
@ -324,6 +363,7 @@ func (e *Engine) envelope(request api.CheckRequest, expectation api.DecisionExpe
|
|||
"registry_subject": facts.subjectFound,
|
||||
"registry_resource": facts.resourceFound,
|
||||
"matched_relationship": facts.matchedRelationship,
|
||||
"registry_overrode": facts.overriddenByRegistry,
|
||||
},
|
||||
Provenance: api.DecisionProvenance{
|
||||
Evaluator: "flex-auth/local",
|
||||
|
|
@ -407,6 +447,43 @@ func copyMap(in map[string]any) map[string]any {
|
|||
return out
|
||||
}
|
||||
|
||||
// setAuthoritative writes a registry fact over whatever the request carried for
|
||||
// that key, and records the key when a caller value was displaced so the
|
||||
// override is visible in diagnostics rather than silent.
|
||||
func setAuthoritative(target map[string]any, key string, value any, scope string, overridden *[]string) {
|
||||
if isEmptyAttribute(value) {
|
||||
return
|
||||
}
|
||||
if existing, exists := target[key]; exists && overridden != nil && !equalAttribute(existing, value) {
|
||||
*overridden = append(*overridden, scope+"."+key)
|
||||
}
|
||||
target[key] = value
|
||||
}
|
||||
|
||||
func setAuthoritatives(target map[string]any, attrs map[string]any, scope string, overridden *[]string) {
|
||||
for _, key := range sortedKeys(attrs) {
|
||||
setAuthoritative(target, key, attrs[key], scope, overridden)
|
||||
}
|
||||
}
|
||||
|
||||
func equalAttribute(a, b any) bool {
|
||||
left, errLeft := json.Marshal(a)
|
||||
right, errRight := json.Marshal(b)
|
||||
if errLeft != nil || errRight != nil {
|
||||
return false
|
||||
}
|
||||
return bytes.Equal(left, right)
|
||||
}
|
||||
|
||||
func sortedKeys(attrs map[string]any) []string {
|
||||
keys := make([]string, 0, len(attrs))
|
||||
for key := range attrs {
|
||||
keys = append(keys, key)
|
||||
}
|
||||
sort.Strings(keys)
|
||||
return keys
|
||||
}
|
||||
|
||||
func addAttributes(target map[string]any, attrs map[string]any) {
|
||||
for key, value := range attrs {
|
||||
addAttribute(target, key, value)
|
||||
|
|
|
|||
198
internal/decision/enrichment_precedence_test.go
Normal file
198
internal/decision/enrichment_precedence_test.go
Normal file
|
|
@ -0,0 +1,198 @@
|
|||
package decision_test
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"encoding/json"
|
||||
"testing"
|
||||
|
||||
"github.com/netkingdom/flex-auth/pkg/api"
|
||||
)
|
||||
|
||||
// A registry fact is an authority statement; the same key on the request is the
|
||||
// caller's proposal about itself. Enrichment once let the proposal survive, which
|
||||
// made every registry ceiling and allowlist advisory — a caller supplying
|
||||
// resource.attributes.max_ttl_hours raised its own ceiling, and one supplying
|
||||
// allowed_subjects authorized a subject the registry did not know
|
||||
// (FLEX-DEC-2026-012).
|
||||
//
|
||||
// These tests assert the registry value reaches policy on a request that
|
||||
// contradicts it. They are written against attribute keys real packages branch
|
||||
// on, because a test over an unused key would pass while the escalation stayed
|
||||
// open.
|
||||
func TestRegistryFactsOverrideCallerSuppliedAttributes(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
if err := store.ImportResourceManifest(api.ResourceManifest{
|
||||
ID: "ceilings",
|
||||
System: "test-system",
|
||||
Resources: []api.Resource{{
|
||||
ID: "resource:ceiling",
|
||||
Type: "document",
|
||||
Attributes: map[string]any{"max_ttl_hours": 8, "allowed_subjects": []any{"user:alice"}},
|
||||
}},
|
||||
}); err != nil {
|
||||
t.Fatalf("ImportResourceManifest: %v", err)
|
||||
}
|
||||
engine := newTestEngineWithStore(t, store)
|
||||
|
||||
decision, err := engine.Check(context.Background(), api.CheckRequest{
|
||||
Subject: api.SubjectRef{ID: "user:alice"},
|
||||
Action: "read",
|
||||
Resource: api.ResourceRef{
|
||||
ID: "resource:ceiling",
|
||||
System: "test-system",
|
||||
Attributes: map[string]any{
|
||||
"max_ttl_hours": 99,
|
||||
"allowed_subjects": []any{"user:mallory"},
|
||||
},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Check: %v", err)
|
||||
}
|
||||
|
||||
attributes := decision.Binding.Resource.Attributes
|
||||
if got := attributes["max_ttl_hours"]; !equalJSON(t, got, 8) {
|
||||
t.Fatalf("max_ttl_hours = %v; want the registry's 8, not the caller's 99", got)
|
||||
}
|
||||
if got := attributes["allowed_subjects"]; !equalJSON(t, got, []any{"user:alice"}) {
|
||||
t.Fatalf("allowed_subjects = %v; caller displaced the registry allowlist", got)
|
||||
}
|
||||
|
||||
// The override must be visible. A registry that silently discards a
|
||||
// contradicting claim hides that a caller asserted authority it did not have,
|
||||
// which is the half worth alerting on even though the decision is now correct.
|
||||
overrode, _ := decision.Diagnostics["registry_overrode"].([]string)
|
||||
if !containsAll(overrode, "resource.max_ttl_hours", "resource.allowed_subjects") {
|
||||
t.Fatalf("registry_overrode = %v; want both displaced keys named", overrode)
|
||||
}
|
||||
}
|
||||
|
||||
// Tenant is a registry fact and the registry wins. subject.type deliberately does
|
||||
// NOT, and the asymmetry is the finding rather than an oversight: the registry's
|
||||
// Type is CARING vocabulary (Human, Agent, Automation, Service) while the
|
||||
// request's is the protected system's actor vocabulary (service, adm, agt, atm).
|
||||
// Two fields sharing a name. Overwriting substituted one vocabulary for the other
|
||||
// and denied every secrets-engine allow, because "Service" is not "service".
|
||||
//
|
||||
// This test pins both halves so the residual cannot be closed by accident, and so
|
||||
// a later reader sees the exclusion is reasoned. FLEX-WP-0025 separates them.
|
||||
func TestRegistryTenantWinsWhileSubjectTypeVocabularyIsPreserved(t *testing.T) {
|
||||
store := newTestStore(t)
|
||||
if err := store.ImportSubjectManifest(api.SubjectManifest{
|
||||
ID: "principals",
|
||||
Tenants: []api.Tenant{{ID: "tenant:real"}},
|
||||
Subjects: []api.Subject{{
|
||||
ID: "subject:claimant",
|
||||
Type: "human",
|
||||
Tenant: "tenant:real",
|
||||
}},
|
||||
}); err != nil {
|
||||
t.Fatalf("ImportSubjectManifest: %v", err)
|
||||
}
|
||||
engine := newTestEngineWithStore(t, store)
|
||||
|
||||
decision, err := engine.Check(context.Background(), api.CheckRequest{
|
||||
Tenant: "tenant:claimed",
|
||||
Subject: api.SubjectRef{ID: "subject:claimant", Type: "service", Tenant: "tenant:claimed"},
|
||||
Action: "read",
|
||||
Resource: api.ResourceRef{ID: "document:internal-note", System: "markitect-tool"},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Check: %v", err)
|
||||
}
|
||||
if got := decision.Binding.Subject.Type; got != "service" {
|
||||
t.Fatalf("subject.type = %q; the caller's actor vocabulary must survive", got)
|
||||
}
|
||||
if got := decision.Binding.Subject.Tenant; got != "tenant:real" {
|
||||
t.Fatalf("subject.tenant = %q; want the registry's tenant:real", got)
|
||||
}
|
||||
}
|
||||
|
||||
// An unregistered subject or resource has no facts to overlay, so what the caller
|
||||
// sent stands. That is the stated residual rather than an oversight: a policy
|
||||
// reading a key its manifest omits is consuming caller input, and no override
|
||||
// should be reported for a key nobody contradicted.
|
||||
func TestUnregisteredResourceKeepsCallerAttributesAndReportsNoOverride(t *testing.T) {
|
||||
engine := newTestEngine(t)
|
||||
|
||||
decision, err := engine.Check(context.Background(), api.CheckRequest{
|
||||
Subject: api.SubjectRef{ID: "user:alice"},
|
||||
Action: "read",
|
||||
Resource: api.ResourceRef{
|
||||
ID: "document:not-in-registry",
|
||||
System: "markitect-tool",
|
||||
Attributes: map[string]any{"stage": "prod"},
|
||||
},
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatalf("Check: %v", err)
|
||||
}
|
||||
if got := decision.Binding.Resource.Attributes["stage"]; got != "prod" {
|
||||
t.Fatalf("stage = %v; caller attributes on an unregistered resource must survive", got)
|
||||
}
|
||||
if overrode, _ := decision.Diagnostics["registry_overrode"].([]string); len(overrode) != 0 {
|
||||
t.Fatalf("registry_overrode = %v; nothing was contradicted", overrode)
|
||||
}
|
||||
}
|
||||
|
||||
func equalJSON(t *testing.T, got, want any) bool {
|
||||
t.Helper()
|
||||
left, err := json.Marshal(got)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
right, err := json.Marshal(want)
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
return bytes.Equal(left, right)
|
||||
}
|
||||
|
||||
func containsAll(values []string, wanted ...string) bool {
|
||||
for _, want := range wanted {
|
||||
found := false
|
||||
for _, value := range values {
|
||||
if value == want {
|
||||
found = true
|
||||
break
|
||||
}
|
||||
}
|
||||
if !found {
|
||||
return false
|
||||
}
|
||||
}
|
||||
return true
|
||||
}
|
||||
|
||||
// binding.request_digest is computed over the ENRICHED request, so a consumer
|
||||
// recomputing it over what it sent gets a different value on every request whose
|
||||
// subject or resource the registry knows. It was published as the §6.4.2 replay
|
||||
// test for consumers, which it cannot be. submitted_request_digest is that test:
|
||||
// RequestDigest over the request exactly as received (FLEX-DEC-2026-012).
|
||||
func TestSubmittedRequestDigestIsComputableByTheConsumer(t *testing.T) {
|
||||
engine := newTestEngine(t)
|
||||
|
||||
request := api.CheckRequest{
|
||||
Tenant: "tenant:platform",
|
||||
Subject: api.SubjectRef{ID: "user:alice"},
|
||||
Action: "read",
|
||||
Resource: api.ResourceRef{ID: "document:internal-note", System: "markitect-tool"},
|
||||
}
|
||||
|
||||
decision, err := engine.Check(context.Background(), request)
|
||||
if err != nil {
|
||||
t.Fatalf("Check: %v", err)
|
||||
}
|
||||
|
||||
// The consumer holds only the request it sent. This is the whole property.
|
||||
if got, want := decision.Binding.SubmittedRequestDigest, api.RequestDigest(request); got != want {
|
||||
t.Fatalf("submitted_request_digest = %q; consumer computes %q", got, want)
|
||||
}
|
||||
|
||||
// And it must genuinely differ from the enriched digest here, or the test
|
||||
// would pass on a request the registry never touched and prove nothing.
|
||||
if decision.Binding.SubmittedRequestDigest == decision.Binding.RequestDigest {
|
||||
t.Fatal("digests agree on an enriched request; the distinction is not being exercised")
|
||||
}
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue