From 469d860ef5defcdcfcec4ff979309b4da9e5061d Mon Sep 17 00:00:00 2001 From: Gnani Rahul Date: Fri, 17 Jul 2026 10:32:17 -0500 Subject: [PATCH] feat(pep): admit bound typed proposals Extend the fail-closed policy enforcement point with a proposal-only admission path bound to the verified tenant, actor, canonical action verb, normalized target, intent identifier, and validated argument digest. Keep raw proposal material out of policy audit and metrics, preserve the read-only default hook, and expose distinct typed deny and approval-required outcomes without adding any execution capability. Closes #230 GSTACK-Checkpoint: 2026-07-17/pep-proposal-admission#1 Signed-off-by: Gnani Rahul --- internal/pep/audit.go | 12 +- internal/pep/audit_test.go | 84 ++++++ internal/pep/boundary_test.go | 21 +- internal/pep/metrics.go | 7 +- internal/pep/metrics_test.go | 18 ++ internal/pep/pep.go | 215 +++++++++++++-- internal/pep/pep_test.go | 14 + internal/pep/proposal_test.go | 476 ++++++++++++++++++++++++++++++++++ 8 files changed, 812 insertions(+), 35 deletions(-) create mode 100644 internal/pep/proposal_test.go diff --git a/internal/pep/audit.go b/internal/pep/audit.go index 5a23a6f..646fba6 100644 --- a/internal/pep/audit.go +++ b/internal/pep/audit.go @@ -66,7 +66,17 @@ func (event AuditEvent) Validate() error { if err := validateSafeText("policy audit actor", event.Actor, maxActorBytes); err != nil { return err } - if !event.Role.Valid() || event.Action != tenancy.ActionRead || !event.Verb.Valid() { + if !event.Role.Valid() || (event.Action != tenancy.ActionRead && event.Action != tenancy.ActionProposeIntent) { + return fmt.Errorf("policy audit has unsupported role, action, or verb") + } + if !event.Role.Allows(event.Action) && (event.Verdict != VerdictDeny || (event.ReasonCode != "role-denied" && event.ReasonCode != "invalid-request")) { + return fmt.Errorf("policy audit has impossible role and action outcome") + } + if event.Verb == invalidVerb { + if event.Verdict != VerdictDeny || event.ReasonCode != "invalid-request" { + return fmt.Errorf("policy audit has unsafe invalid-request sentinel") + } + } else if !event.Verb.validForAction(event.Action) { return fmt.Errorf("policy audit has unsupported role, action, or verb") } return (Decision{Verdict: event.Verdict, ReasonCode: event.ReasonCode}).Validate() diff --git a/internal/pep/audit_test.go b/internal/pep/audit_test.go index ff5ad0d..6a3faf3 100644 --- a/internal/pep/audit_test.go +++ b/internal/pep/audit_test.go @@ -110,6 +110,90 @@ func TestSlogAuditorTextHandlerPreservesSafeFieldsAndSeverity(t *testing.T) { } } +func TestSlogAuditorAcceptsProposalAndConstrainedInvalidRequestEvents(t *testing.T) { + var output bytes.Buffer + auditor, err := NewSlogAuditor(slog.New(slog.NewJSONHandler(&output, nil))) + if err != nil { + t.Fatal(err) + } + proposal := policyAuditEvent(time.Now().UTC(), VerdictAllow, "proposal-allowed") + proposal.Actor = "user:operator" + proposal.Role = tenancy.RoleOperator + proposal.Action = tenancy.ActionProposeIntent + proposal.Verb = "deployment.restart" + invalid := proposal + invalid.Verb = invalidVerb + invalid.Verdict = VerdictDeny + invalid.ReasonCode = "invalid-request" + invalidRead := policyAuditEvent(time.Now().UTC(), VerdictDeny, "invalid-request") + invalidRead.Verb = invalidVerb + for _, event := range []AuditEvent{proposal, invalid, invalidRead} { + if err := auditor.Record(context.Background(), event); err != nil { + t.Fatalf("Record(%s) error = %v", event.Verb, err) + } + } + logged := output.String() + for _, field := range []string{`"action":"propose-intent"`, `"verb":"deployment.restart"`, `"verb":"invalid"`} { + if !strings.Contains(logged, field) { + t.Fatalf("proposal audit missing %s: %s", field, logged) + } + } + for _, forbidden := range []string{"arguments_digest", "resolved_digest", "payments", "token", "secret"} { + if strings.Contains(logged, forbidden) { + t.Fatalf("proposal audit leaked %q: %s", forbidden, logged) + } + } +} + +func TestAuditEventRejectsUnsafeInvalidRequestSentinel(t *testing.T) { + base := policyAuditEvent(time.Now().UTC(), VerdictDeny, "invalid-request") + base.Verb = invalidVerb + if err := base.Validate(); err != nil { + t.Fatalf("baseline invalid-request event should validate: %v", err) + } + for _, mutate := range []func(*AuditEvent){ + func(event *AuditEvent) { event.Verdict = VerdictAllow }, + func(event *AuditEvent) { event.Verdict = VerdictRequireApproval }, + func(event *AuditEvent) { event.ReasonCode = "policy-deny" }, + } { + event := base + mutate(&event) + if err := event.Validate(); err == nil { + t.Fatalf("AuditEvent.Validate() accepted unsafe invalid sentinel: %#v", event) + } + } +} + +func TestAuditEventRejectsImpossibleProposalRoleOutcome(t *testing.T) { + base := policyAuditEvent(time.Now().UTC(), VerdictDeny, "role-denied") + base.Action = tenancy.ActionProposeIntent + base.Verb = "deployment.restart" + if err := base.Validate(); err != nil { + t.Fatalf("role-denied proposal event should validate: %v", err) + } + + invalidRequest := base + invalidRequest.ReasonCode = "invalid-request" + if err := invalidRequest.Validate(); err != nil { + t.Fatalf("pre-role invalid-request event should validate: %v", err) + } + + for _, mutate := range []func(*AuditEvent){ + func(event *AuditEvent) { event.Verdict = VerdictAllow; event.ReasonCode = "proposal-allowed" }, + func(event *AuditEvent) { + event.Verdict = VerdictRequireApproval + event.ReasonCode = "approval-required" + }, + func(event *AuditEvent) { event.ReasonCode = "policy-deny" }, + } { + event := base + mutate(&event) + if err := event.Validate(); err == nil { + t.Fatalf("AuditEvent.Validate() accepted impossible role outcome: %#v", event) + } + } +} + func policyAuditEvent(at time.Time, verdict Verdict, reason string) AuditEvent { return AuditEvent{ At: at, TraceID: tracing.ID("0123456789abcdef0123456789abcdef"), WorkspaceID: "workspace-a", Actor: "user:reader", Role: tenancy.RoleReader, diff --git a/internal/pep/boundary_test.go b/internal/pep/boundary_test.go index b62abbe..c596b39 100644 --- a/internal/pep/boundary_test.go +++ b/internal/pep/boundary_test.go @@ -40,8 +40,23 @@ func TestPEPHasNoNetworkOrDispatchImports(t *testing.T) { } } -func TestAuditEventOmitsArgumentDigest(t *testing.T) { - if _, exists := reflect.TypeFor[AuditEvent]().FieldByName("ArgumentsDigest"); exists { - t.Fatal("audit event must not retain the policy argument digest") +func TestPolicyBoundaryOmitsRawProposalAndDigestMaterial(t *testing.T) { + assertExactFields(t, reflect.TypeFor[AuditEvent](), []string{ + "At", "TraceID", "WorkspaceID", "Actor", "Role", "Action", "Verb", "Verdict", "ReasonCode", + }) + assertExactFields(t, reflect.TypeFor[ProposalInput](), []string{ + "intentID", "workspaceID", "actor", "verb", "target", "argumentsDigest", "resolvedDigest", + }) +} + +func assertExactFields(t *testing.T, value reflect.Type, expected []string) { + t.Helper() + if value.NumField() != len(expected) { + t.Fatalf("%s fields = %d, want exactly %d", value.Name(), value.NumField(), len(expected)) + } + for _, name := range expected { + if _, exists := value.FieldByName(name); !exists { + t.Fatalf("%s is missing approved field %s", value.Name(), name) + } } } diff --git a/internal/pep/metrics.go b/internal/pep/metrics.go index ca2a051..8083072 100644 --- a/internal/pep/metrics.go +++ b/internal/pep/metrics.go @@ -6,10 +6,11 @@ import ( "context" "time" + "github.com/ArdurAI/sith/internal/intent" "github.com/ArdurAI/sith/internal/tracing" ) -// DecisionOutcome is the bounded self-observability result of one policy read attempt. +// DecisionOutcome is the bounded self-observability result of one policy decision. // It intentionally carries no workspace, actor, selector, credential, or reason-code material. type DecisionOutcome string @@ -21,7 +22,7 @@ const ( DecisionOutcomeError DecisionOutcome = "error" ) -// DecisionObserver receives passive, bounded measurements for policy reads. Implementations must +// DecisionObserver receives passive, bounded measurements for policy decisions. Implementations must // not block or mutate the authorization path. The enforcer isolates observer panics defensively. type DecisionObserver interface { ObserveDecision(verb Verb, outcome DecisionOutcome, duration time.Duration) @@ -66,7 +67,7 @@ func traceOutcome(outcome DecisionOutcome) tracing.Outcome { } func normalizedObservedVerb(verb Verb) Verb { - if verb.Valid() { + if verb.Valid() || intent.Verb(verb).Valid() { return verb } return "invalid" diff --git a/internal/pep/metrics_test.go b/internal/pep/metrics_test.go index b282594..df44a7f 100644 --- a/internal/pep/metrics_test.go +++ b/internal/pep/metrics_test.go @@ -65,6 +65,24 @@ func TestEnforcerRecoversFromPanickingObserver(t *testing.T) { } } +func TestEnforcerObservesCanonicalProposalVerb(t *testing.T) { + observer := &recordingDecisionObserver{} + enforcer, err := NewEnforcer(Config{ + Hook: proposalDecision(VerdictAllow, "proposal-allowed"), + Auditor: AuditFunc(func(context.Context, AuditEvent) error { return nil }), + Observer: observer, + }) + if err != nil { + t.Fatal(err) + } + if err := enforcer.AuthorizeProposal(context.Background(), operatorScope(t), testProposalInput(t)); err != nil { + t.Fatalf("AuthorizeProposal() error = %v", err) + } + if len(observer.events) != 1 || observer.events[0].verb != "deployment.restart" || observer.events[0].outcome != DecisionOutcomeAllow { + t.Fatalf("observations = %#v", observer.events) + } +} + type decisionObservation struct { verb Verb outcome DecisionOutcome diff --git a/internal/pep/pep.go b/internal/pep/pep.go index 2c8979c..ce72206 100644 --- a/internal/pep/pep.go +++ b/internal/pep/pep.go @@ -6,18 +6,32 @@ import ( "context" "crypto/sha256" "encoding/hex" + "errors" "fmt" "strings" "time" "unicode" + "github.com/ArdurAI/sith/internal/fleet" + "github.com/ArdurAI/sith/internal/intent" "github.com/ArdurAI/sith/internal/tenancy" "github.com/ArdurAI/sith/internal/tracing" ) const ( - maxActorBytes = 256 - maxReasonCodeBytes = 64 + maxActorBytes = 256 + maxIntentIDBytes = 253 + maxReasonCodeBytes = 64 + maxTargetComponentBytes = 256 + invalidVerb = Verb("invalid") + proposalDigestDomain = "sith-pep-proposal/v1" +) + +// Stable policy refusal classes let later approval and action orchestration distinguish a deny +// from a pending approval without parsing error text. Both remain fail-closed outcomes. +var ( + ErrDenied = errors.New("policy denied request") + ErrApprovalRequired = errors.New("policy requires approval") ) // Verb identifies one closed PEP operation. New verbs are an explicit policy-boundary change. @@ -44,19 +58,32 @@ func (verb Verb) Valid() bool { } } +func (verb Verb) validForAction(action tenancy.Action) bool { + switch action { + case tenancy.ActionRead: + return verb.Valid() + case tenancy.ActionProposeIntent: + return intent.Verb(verb).Valid() + default: + return false + } +} + // Verdict is the PDP-compatible outcome returned at the policy hook. type Verdict string -// Supported policy outcomes. A non-allow outcome never reaches the read dependency. +// Supported policy outcomes. A non-allow outcome never reaches the downstream operation. const ( VerdictAllow Verdict = "allow" VerdictDeny Verdict = "deny" VerdictRequireApproval Verdict = "require-approval" ) -// Request is the normalized, post-authentication input to the policy hook. Scope identity comes -// only from a signed tenancy scope; raw credentials, headers, selectors, and result data are never -// carried into the audit event. +// Request is the normalized, post-authentication input to the policy hook. For reads, +// ArgumentsDigest binds canonical validated arguments. For proposals, it binds the complete +// resolved proposal envelope. Scope identity comes only from a signed tenancy scope; raw +// credentials, headers, selectors, targets, arguments, and result data are never carried into the +// hook or audit event. type Request struct { WorkspaceID tenancy.WorkspaceID Actor string @@ -73,6 +100,19 @@ type ReadInput struct { ArgumentsDigest string } +// ProposalInput is an immutable, privacy-minimizing binding for a validated and resolved typed +// proposal. Callers construct it only after handler-owned argument validation and target +// resolution. It retains digests and normalized identifiers, never raw arguments. +type ProposalInput struct { + intentID string + workspaceID tenancy.WorkspaceID + actor string + verb intent.Verb + target fleet.ResourceRef + argumentsDigest string + resolvedDigest string +} + // NewReadInput hashes canonical typed arguments for the policy hook. Callers must validate their // concrete argument schema before constructing this input. func NewReadInput(verb Verb, canonicalArguments []byte) ReadInput { @@ -80,6 +120,32 @@ func NewReadInput(verb Verb, canonicalArguments []byte) ReadInput { return ReadInput{Verb: verb, ArgumentsDigest: "sha256:" + hex.EncodeToString(digest[:])} } +// NewProposalInput binds one exact resolved proposal. target must be the normalized target +// returned by planning and argumentsDigest must come from the already schema-validated argument +// document. The resulting digest changes if any bound identity, verb, target, or argument digest +// changes. +func NewProposalInput( + intentID string, + workspaceID tenancy.WorkspaceID, + actor string, + verb intent.Verb, + target fleet.ResourceRef, + argumentsDigest string, +) (ProposalInput, error) { + input := ProposalInput{ + intentID: intentID, workspaceID: workspaceID, actor: actor, verb: verb, + target: target, argumentsDigest: argumentsDigest, + } + if err := input.validateFields(); err != nil { + return ProposalInput{}, fmt.Errorf("construct policy proposal input: %w", err) + } + // An empty non-nil attributes map is semantically valid but remains caller-mutable. Discard it + // so the retained proposal binding cannot be changed or raced after construction. + input.target.Attributes = nil + input.resolvedDigest = input.digest() + return input, nil +} + // Decision records one PDP-compatible policy result using a safe reason code rather than free text. type Decision struct { Verdict Verdict @@ -114,7 +180,7 @@ type AuditEvent struct { ReasonCode string } -// Auditor durably records one PEP decision. An audit failure fails closed before a read runs. +// Auditor durably records one PEP decision. An audit failure fails closed before an operation runs. type Auditor interface { Record(context.Context, AuditEvent) error } @@ -136,7 +202,7 @@ type Config struct { Now func() time.Time } -// Enforcer applies the fixed Phase-1 read pipeline and creates one audit record for every decision. +// Enforcer applies the fixed policy pipeline and creates one audit record for every decision. type Enforcer struct { hook PolicyHook auditor Auditor @@ -178,40 +244,62 @@ func (AllowReadHook) Decide(_ context.Context, request Request) (Decision, error // hook → audit. A deny, approval requirement, malformed decision, hook error, or audit error blocks // the downstream reader. func (enforcer *Enforcer) AuthorizeRead(ctx context.Context, scope tenancy.Scope, input ReadInput) error { + request := Request{ + WorkspaceID: scope.WorkspaceID(), Actor: scope.Subject(), Role: scope.Role(), Action: tenancy.ActionRead, + Verb: input.Verb, ArgumentsDigest: input.ArgumentsDigest, + } + return enforcer.authorize(ctx, scope, request, true, "read") +} + +// AuthorizeProposal runs a resolved typed proposal through the same policy hook and mandatory +// audit boundary as reads. It grants no execution capability: AllowReadHook denies this action, and +// both deny and require-approval return typed fail-closed errors. +func (enforcer *Enforcer) AuthorizeProposal(ctx context.Context, scope tenancy.Scope, input ProposalInput) error { + request := Request{ + WorkspaceID: scope.WorkspaceID(), Actor: scope.Subject(), Role: scope.Role(), Action: tenancy.ActionProposeIntent, + Verb: Verb(input.verb), ArgumentsDigest: input.resolvedDigest, + } + inputValid := input.validate() == nil && input.workspaceID == scope.WorkspaceID() && input.actor == scope.Subject() + return enforcer.authorize(ctx, scope, request, inputValid, "proposal") +} + +func (enforcer *Enforcer) authorize( + ctx context.Context, + scope tenancy.Scope, + request Request, + inputValid bool, + operation string, +) error { if enforcer == nil || enforcer.hook == nil || enforcer.auditor == nil || ctx == nil { - return fmt.Errorf("authorize read: enforcer and context are required") + return fmt.Errorf("authorize %s: enforcer and context are required", operation) } traceContext, _, err := tracing.Ensure(ctx) if err != nil { - return fmt.Errorf("authorize read: establish trace context: %w", err) + return fmt.Errorf("authorize %s: establish trace context: %w", operation, err) } ctx = traceContext startedAt := time.Now() outcome := DecisionOutcomeError defer func() { - enforcer.observeDecision(input.Verb, outcome, time.Since(startedAt)) + enforcer.observeDecision(request.Verb, outcome, time.Since(startedAt)) enforcer.observeTrace(ctx, outcome, time.Since(startedAt)) }() - request := Request{ - WorkspaceID: scope.WorkspaceID(), Actor: scope.Subject(), Role: scope.Role(), Action: tenancy.ActionRead, - Verb: input.Verb, ArgumentsDigest: input.ArgumentsDigest, - } - if err := request.Validate(); err != nil { + if !inputValid || request.Validate() != nil { outcome = DecisionOutcomeDeny - return enforcer.refuse(ctx, request, VerdictDeny, "invalid-request", "authorize read: invalid policy request") + return enforcer.refuse(ctx, request, VerdictDeny, "invalid-request", fmt.Sprintf("authorize %s: invalid policy request", operation), ErrDenied) } - if err := scope.Authorize(tenancy.ActionRead); err != nil { + if err := scope.Authorize(request.Action); err != nil { outcome = DecisionOutcomeDeny - return enforcer.refuse(ctx, request, VerdictDeny, "role-denied", "authorize read: role does not permit read") + return enforcer.refuse(ctx, request, VerdictDeny, "role-denied", fmt.Sprintf("authorize %s: role does not permit %s", operation, request.Action), ErrDenied) } decision, err := enforcer.hook.Decide(ctx, request) if err != nil { outcome = DecisionOutcomeError - return enforcer.refuse(ctx, request, VerdictDeny, "hook-error", "authorize read: policy hook failed") + return enforcer.refuse(ctx, request, VerdictDeny, "hook-error", fmt.Sprintf("authorize %s: policy hook failed", operation), nil) } if err := decision.Validate(); err != nil { outcome = DecisionOutcomeError - return enforcer.refuse(ctx, request, VerdictDeny, "invalid-decision", "authorize read: policy hook returned an invalid decision") + return enforcer.refuse(ctx, request, VerdictDeny, "invalid-decision", fmt.Sprintf("authorize %s: policy hook returned an invalid decision", operation), nil) } if decision.Verdict != VerdictAllow { if decision.Verdict == VerdictRequireApproval { @@ -221,16 +309,16 @@ func (enforcer *Enforcer) AuthorizeRead(ctx context.Context, scope tenancy.Scope } if err := enforcer.record(ctx, request, decision); err != nil { outcome = DecisionOutcomeError - return fmt.Errorf("authorize read: audit policy refusal: %w", err) + return fmt.Errorf("authorize %s: audit policy refusal: %w", operation, err) } if decision.Verdict == VerdictRequireApproval { - return fmt.Errorf("authorize read: policy requires approval") + return fmt.Errorf("authorize %s: %w", operation, ErrApprovalRequired) } - return fmt.Errorf("authorize read: policy denied request") + return fmt.Errorf("authorize %s: %w", operation, ErrDenied) } if err := enforcer.record(ctx, request, decision); err != nil { outcome = DecisionOutcomeError - return fmt.Errorf("authorize read: audit policy decision: %w", err) + return fmt.Errorf("authorize %s: audit policy decision: %w", operation, err) } outcome = DecisionOutcomeAllow return nil @@ -244,13 +332,13 @@ func (request Request) Validate() error { if err := validateSafeText("policy actor", request.Actor, maxActorBytes); err != nil { return err } - if !request.Role.Valid() || request.Action != tenancy.ActionRead || !request.Verb.Valid() || !validDigest(request.ArgumentsDigest) { + if !request.Role.Valid() || !request.Verb.validForAction(request.Action) || !validDigest(request.ArgumentsDigest) { return fmt.Errorf("policy request uses an unsupported role, action, or verb") } return nil } -// Validate rejects incomplete, unknown, or unsafe PDP responses before they affect a read. +// Validate rejects incomplete, unknown, or unsafe PDP responses before they affect an operation. func (decision Decision) Validate() error { switch decision.Verdict { case VerdictAllow, VerdictDeny, VerdictRequireApproval: @@ -260,13 +348,40 @@ func (decision Decision) Validate() error { return validateReasonCode(decision.ReasonCode) } -func (enforcer *Enforcer) refuse(ctx context.Context, request Request, verdict Verdict, reasonCode, message string) error { +func (enforcer *Enforcer) refuse(ctx context.Context, request Request, verdict Verdict, reasonCode, message string, classification error) error { + request, auditable := normalizedAuditRequest(request) + if !auditable { + if classification != nil { + return fmt.Errorf("%s: %w", message, classification) + } + return fmt.Errorf("%s", message) + } if err := enforcer.record(ctx, request, Decision{Verdict: verdict, ReasonCode: reasonCode}); err != nil { + if classification != nil { + return fmt.Errorf("%s: %w: audit refusal: %w", message, classification, err) + } return fmt.Errorf("%s: audit refusal: %w", message, err) } + if classification != nil { + return fmt.Errorf("%s: %w", message, classification) + } return fmt.Errorf("%s", message) } +func normalizedAuditRequest(request Request) (Request, bool) { + if tenancy.ValidateWorkspaceID(request.WorkspaceID) != nil || validateSafeText("policy actor", request.Actor, maxActorBytes) != nil || + !request.Role.Valid() || (request.Action != tenancy.ActionRead && request.Action != tenancy.ActionProposeIntent) { + return Request{}, false + } + if !request.Verb.validForAction(request.Action) { + request.Verb = invalidVerb + } + // AuditEvent deliberately has no binding-digest field. Clear it here as defense in depth so a + // malformed or caller-supplied value cannot survive refusal normalization into future sinks. + request.ArgumentsDigest = "" + return request, true +} + func (enforcer *Enforcer) record(ctx context.Context, request Request, decision Decision) error { traceID, ok := tracing.FromContext(ctx) if !ok { @@ -314,3 +429,47 @@ func validDigest(value string) bool { } return true } + +func (input ProposalInput) validate() error { + if err := input.validateFields(); err != nil { + return err + } + if !validDigest(input.resolvedDigest) || input.resolvedDigest != input.digest() { + return fmt.Errorf("proposal binding digest is invalid") + } + return nil +} + +func (input ProposalInput) validateFields() error { + if validateSafeText("proposal intent identifier", input.intentID, maxIntentIDBytes) != nil || + tenancy.ValidateWorkspaceID(input.workspaceID) != nil || + validateSafeText("proposal actor", input.actor, maxActorBytes) != nil || !input.verb.Valid() || + validateProposalTarget(input.target) != nil || !validDigest(input.argumentsDigest) { + return fmt.Errorf("proposal fields are invalid") + } + return nil +} + +func (input ProposalInput) digest() string { + values := []string{ + proposalDigestDomain, input.intentID, string(input.workspaceID), input.actor, string(input.verb), + input.target.SourceKind, input.target.Scope, input.target.Kind, input.target.Namespace, input.target.Name, + input.argumentsDigest, + } + digest := sha256.Sum256([]byte(strings.Join(values, "\x00"))) + return "sha256:" + hex.EncodeToString(digest[:]) +} + +func validateProposalTarget(target fleet.ResourceRef) error { + if len(target.Attributes) != 0 || + validateSafeText("proposal target source", target.SourceKind, maxTargetComponentBytes) != nil || + validateSafeText("proposal target scope", target.Scope, maxTargetComponentBytes) != nil || + validateSafeText("proposal target kind", target.Kind, maxTargetComponentBytes) != nil || + validateSafeText("proposal target name", target.Name, maxTargetComponentBytes) != nil { + return fmt.Errorf("proposal target is invalid") + } + if target.Namespace != "" && validateSafeText("proposal target namespace", target.Namespace, maxTargetComponentBytes) != nil { + return fmt.Errorf("proposal target is invalid") + } + return nil +} diff --git a/internal/pep/pep_test.go b/internal/pep/pep_test.go index 9d30edd..93039a8 100644 --- a/internal/pep/pep_test.go +++ b/internal/pep/pep_test.go @@ -127,6 +127,20 @@ func TestReadInputBindsCanonicalArgumentsAndRejectsAlteredDigest(t *testing.T) { } } +func TestNormalizedAuditRequestErasesPolicyBindingDigest(t *testing.T) { + request := Request{ + WorkspaceID: "workspace-a", Actor: "user:reader", Role: tenancy.RoleReader, + Action: tenancy.ActionRead, Verb: VerbFleetRead, ArgumentsDigest: "token=caller-secret", + } + normalized, ok := normalizedAuditRequest(request) + if !ok { + t.Fatal("normalizedAuditRequest() rejected safe audit identity") + } + if normalized.ArgumentsDigest != "" { + t.Fatalf("normalized audit request retained binding digest %q", normalized.ArgumentsDigest) + } +} + type recordingAuditor struct { events []AuditEvent } diff --git a/internal/pep/proposal_test.go b/internal/pep/proposal_test.go new file mode 100644 index 0000000..b91cfb9 --- /dev/null +++ b/internal/pep/proposal_test.go @@ -0,0 +1,476 @@ +// SPDX-License-Identifier: Apache-2.0 + +package pep + +import ( + "context" + "crypto/sha256" + "errors" + "fmt" + "strings" + "sync" + "sync/atomic" + "testing" + + "github.com/ArdurAI/sith/internal/fleet" + "github.com/ArdurAI/sith/internal/intent" + "github.com/ArdurAI/sith/internal/tenancy" +) + +func TestEnforcerAuthorizesBoundTypedProposalAndAudits(t *testing.T) { + input := testProposalInput(t) + var captured Request + auditor := &recordingAuditor{} + enforcer, err := NewEnforcer(Config{ + Hook: HookFunc(func(_ context.Context, request Request) (Decision, error) { + captured = request + return Decision{Verdict: VerdictAllow, ReasonCode: "proposal-allowed"}, nil + }), + Auditor: auditor, + }) + if err != nil { + t.Fatal(err) + } + if err := enforcer.AuthorizeProposal(context.Background(), operatorScope(t), input); err != nil { + t.Fatalf("AuthorizeProposal() error = %v", err) + } + if captured.WorkspaceID != "workspace-a" || captured.Actor != "user:operator" || captured.Role != tenancy.RoleOperator || + captured.Action != tenancy.ActionProposeIntent || captured.Verb != Verb(intent.VerbDeploymentRestart) || + captured.ArgumentsDigest != input.resolvedDigest || !validDigest(captured.ArgumentsDigest) { + t.Fatalf("policy request = %#v", captured) + } + if len(auditor.events) != 1 { + t.Fatalf("audit events = %#v, want one", auditor.events) + } + event := auditor.events[0] + if event.WorkspaceID != "workspace-a" || event.Actor != "user:operator" || event.Role != tenancy.RoleOperator || + event.Action != tenancy.ActionProposeIntent || event.Verb != Verb(intent.VerbDeploymentRestart) || + event.Verdict != VerdictAllow || event.ReasonCode != "proposal-allowed" { + t.Fatalf("audit event = %#v", event) + } +} + +func TestAllowReadHookDeniesProposalByDefault(t *testing.T) { + enforcer, err := NewEnforcer(Config{Hook: AllowReadHook{}, Auditor: &recordingAuditor{}}) + if err != nil { + t.Fatal(err) + } + err = enforcer.AuthorizeProposal(context.Background(), operatorScope(t), testProposalInput(t)) + if !errors.Is(err, ErrDenied) { + t.Fatalf("AuthorizeProposal() error = %v, want ErrDenied", err) + } +} + +func TestEnforcerRejectsNilProposalContext(t *testing.T) { + var hookCalls atomic.Int64 + var auditCalls atomic.Int64 + enforcer, err := NewEnforcer(Config{ + Hook: HookFunc(func(context.Context, Request) (Decision, error) { + hookCalls.Add(1) + return Decision{Verdict: VerdictAllow, ReasonCode: "unexpected"}, nil + }), + Auditor: AuditFunc(func(context.Context, AuditEvent) error { + auditCalls.Add(1) + return nil + }), + }) + if err != nil { + t.Fatal(err) + } + var nilContext context.Context + if err := enforcer.AuthorizeProposal(nilContext, operatorScope(t), testProposalInput(t)); err == nil || !strings.Contains(err.Error(), "context are required") { + t.Fatalf("AuthorizeProposal(nil) error = %v", err) + } + if hookCalls.Load() != 0 || auditCalls.Load() != 0 { + t.Fatalf("nil-context hook/audit calls = %d/%d, want zero", hookCalls.Load(), auditCalls.Load()) + } +} + +func TestEnforcerFailsClosedForUnsafeProposalPolicyOutcomes(t *testing.T) { + tests := []struct { + name string + hook PolicyHook + want Verdict + reason string + wantError error + contains string + auditFails bool + }{ + { + name: "deny", hook: proposalDecision(VerdictDeny, "policy-deny"), + want: VerdictDeny, reason: "policy-deny", wantError: ErrDenied, + }, + { + name: "approval", hook: proposalDecision(VerdictRequireApproval, "approval-required"), + want: VerdictRequireApproval, reason: "approval-required", wantError: ErrApprovalRequired, + }, + { + name: "hook error", hook: HookFunc(func(context.Context, Request) (Decision, error) { + return Decision{}, errors.New("pdp token=secret") + }), + want: VerdictDeny, reason: "hook-error", contains: "policy hook failed", + }, + { + name: "invalid decision", hook: proposalDecision("maybe", "invalid"), + want: VerdictDeny, reason: "invalid-decision", contains: "invalid decision", + }, + { + name: "audit failure", hook: proposalDecision(VerdictAllow, "proposal-allowed"), + contains: "audit policy decision", auditFails: true, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + auditor := &recordingAuditor{} + var sink Auditor = auditor + if test.auditFails { + sink = AuditFunc(func(context.Context, AuditEvent) error { return errors.New("audit unavailable") }) + } + enforcer, err := NewEnforcer(Config{Hook: test.hook, Auditor: sink}) + if err != nil { + t.Fatal(err) + } + err = enforcer.AuthorizeProposal(context.Background(), operatorScope(t), testProposalInput(t)) + if err == nil { + t.Fatal("AuthorizeProposal() returned nil") + } + if test.wantError != nil && !errors.Is(err, test.wantError) { + t.Fatalf("AuthorizeProposal() error = %v, want %v", err, test.wantError) + } + if test.contains != "" && !strings.Contains(err.Error(), test.contains) { + t.Fatalf("AuthorizeProposal() error = %v, want %q", err, test.contains) + } + if strings.Contains(err.Error(), "token=secret") { + t.Fatalf("AuthorizeProposal() leaked hook error: %v", err) + } + if !test.auditFails && strings.Contains(fmt.Sprintf("%#v", auditor.events), "token=secret") { + t.Fatalf("AuthorizeProposal() leaked hook error into audit: %#v", auditor.events) + } + if !test.auditFails && (len(auditor.events) != 1 || auditor.events[0].Verdict != test.want || auditor.events[0].ReasonCode != test.reason) { + t.Fatalf("audit events = %#v, want %s/%s", auditor.events, test.want, test.reason) + } + }) + } +} + +func TestEnforcerProposalRoleMatrixFailsClosed(t *testing.T) { + for _, role := range []tenancy.Role{tenancy.RoleReader, tenancy.RoleApprover, tenancy.RoleAdmin} { + t.Run(string(role), func(t *testing.T) { + var hookCalls atomic.Int64 + auditor := &recordingAuditor{} + enforcer, err := NewEnforcer(Config{ + Hook: HookFunc(func(context.Context, Request) (Decision, error) { + hookCalls.Add(1) + return Decision{Verdict: VerdictAllow, ReasonCode: "unexpected"}, nil + }), + Auditor: auditor, + }) + if err != nil { + t.Fatal(err) + } + err = enforcer.AuthorizeProposal(context.Background(), scopeForRole(t, role), testProposalInputFor(t, "user:"+string(role), "workspace-a")) + if err == nil || !errors.Is(err, ErrDenied) || !strings.Contains(err.Error(), "role does not permit") { + t.Fatalf("AuthorizeProposal() error = %v, want role denial", err) + } + if hookCalls.Load() != 0 { + t.Fatalf("policy hook calls = %d, want zero", hookCalls.Load()) + } + if len(auditor.events) != 1 || auditor.events[0].Verdict != VerdictDeny || auditor.events[0].ReasonCode != "role-denied" { + t.Fatalf("audit events = %#v", auditor.events) + } + }) + } +} + +func TestEnforcerRejectsTamperedProposalBindingBeforePolicyHook(t *testing.T) { + tests := []struct { + name string + mutate func(*ProposalInput) + }{ + {name: "workspace", mutate: func(input *ProposalInput) { input.workspaceID = "workspace-b" }}, + {name: "actor", mutate: func(input *ProposalInput) { input.actor = "user:other" }}, + {name: "verb", mutate: func(input *ProposalInput) { input.verb = "deployment.delete" }}, + {name: "target", mutate: func(input *ProposalInput) { input.target.Name = "payments-canary" }}, + {name: "arguments digest", mutate: func(input *ProposalInput) { input.argumentsDigest = digestFor("different") }}, + {name: "resolved digest", mutate: func(input *ProposalInput) { input.resolvedDigest = digestFor("forged") }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + input := testProposalInput(t) + test.mutate(&input) + var hookCalls atomic.Int64 + auditor := &recordingAuditor{} + enforcer, err := NewEnforcer(Config{ + Hook: HookFunc(func(context.Context, Request) (Decision, error) { + hookCalls.Add(1) + return Decision{Verdict: VerdictAllow, ReasonCode: "unexpected"}, nil + }), + Auditor: auditor, + }) + if err != nil { + t.Fatal(err) + } + err = enforcer.AuthorizeProposal(context.Background(), operatorScope(t), input) + if err == nil || !errors.Is(err, ErrDenied) || !strings.Contains(err.Error(), "invalid policy request") { + t.Fatalf("AuthorizeProposal() error = %v, want invalid request", err) + } + if hookCalls.Load() != 0 { + t.Fatalf("policy hook calls = %d, want zero", hookCalls.Load()) + } + if len(auditor.events) != 1 || auditor.events[0].Verdict != VerdictDeny || auditor.events[0].ReasonCode != "invalid-request" { + t.Fatalf("audit events = %#v", auditor.events) + } + }) + } +} + +func TestProposalDigestBindsEveryResolvedEnvelopeField(t *testing.T) { + target := testProposalTarget() + base := proposalCase{ + intentID: "intent-230", workspaceID: "workspace-a", actor: "user:operator", + verb: intent.VerbDeploymentRestart, target: target, argumentsDigest: digestFor("validated-arguments"), + } + variants := []proposalCase{ + base, + base.with(func(value *proposalCase) { value.intentID = "intent-231" }), + base.with(func(value *proposalCase) { value.workspaceID = "workspace-b" }), + base.with(func(value *proposalCase) { value.actor = "user:operator-2" }), + base.with(func(value *proposalCase) { value.verb = intent.VerbDeploymentScale }), + base.with(func(value *proposalCase) { value.target.SourceKind = "kubernetes" }), + base.with(func(value *proposalCase) { value.target.Scope = "cluster-b" }), + base.with(func(value *proposalCase) { value.target.Kind = "statefulset" }), + base.with(func(value *proposalCase) { value.target.Namespace = "operations" }), + base.with(func(value *proposalCase) { value.target.Name = "payments-canary" }), + base.with(func(value *proposalCase) { value.argumentsDigest = digestFor("other-arguments") }), + } + seen := make(map[string]struct{}, len(variants)) + for index, variant := range variants { + input, err := NewProposalInput(variant.intentID, variant.workspaceID, variant.actor, variant.verb, variant.target, variant.argumentsDigest) + if err != nil { + t.Fatalf("NewProposalInput(variant %d) error = %v", index, err) + } + if !validDigest(input.resolvedDigest) { + t.Fatalf("variant %d digest = %q", index, input.resolvedDigest) + } + if _, exists := seen[input.resolvedDigest]; exists { + t.Fatalf("variant %d reused resolved digest %q", index, input.resolvedDigest) + } + seen[input.resolvedDigest] = struct{}{} + } +} + +func TestNewProposalInputRejectsMalformedEnvelopeFields(t *testing.T) { + base := proposalCase{ + intentID: "intent-230", workspaceID: "workspace-a", actor: "user:operator", + verb: intent.VerbDeploymentRestart, target: testProposalTarget(), argumentsDigest: digestFor("validated-arguments"), + } + tests := []struct { + name string + mutate func(*proposalCase) + }{ + {name: "empty intent ID", mutate: func(value *proposalCase) { value.intentID = "" }}, + {name: "control intent ID", mutate: func(value *proposalCase) { value.intentID = "intent\n230" }}, + {name: "empty workspace", mutate: func(value *proposalCase) { value.workspaceID = "" }}, + {name: "foreign whitespace actor", mutate: func(value *proposalCase) { value.actor = " user:operator" }}, + {name: "unknown verb", mutate: func(value *proposalCase) { value.verb = "deployment.delete" }}, + {name: "empty target source", mutate: func(value *proposalCase) { value.target.SourceKind = "" }}, + {name: "control target name", mutate: func(value *proposalCase) { value.target.Name = "payments\nsecret" }}, + {name: "target attributes", mutate: func(value *proposalCase) { value.target.Attributes = map[string]string{"token": "secret"} }}, + {name: "malformed arguments digest", mutate: func(value *proposalCase) { value.argumentsDigest = "sha256:ABC" }}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + value := base + test.mutate(&value) + if _, err := NewProposalInput(value.intentID, value.workspaceID, value.actor, value.verb, value.target, value.argumentsDigest); err == nil { + t.Fatal("NewProposalInput() accepted malformed envelope") + } + }) + } +} + +func TestNewProposalInputSeversEmptyTargetAttributeAlias(t *testing.T) { + attributes := map[string]string{} + target := testProposalTarget() + target.Attributes = attributes + input, err := NewProposalInput( + "intent-230", "workspace-a", "user:operator", intent.VerbDeploymentRestart, + target, digestFor("validated-arguments"), + ) + if err != nil { + t.Fatalf("NewProposalInput() error = %v", err) + } + attributes["token"] = "caller-secret" + if input.target.Attributes != nil { + t.Fatalf("proposal retained caller-owned target attributes: %#v", input.target.Attributes) + } + enforcer, err := NewEnforcer(Config{ + Hook: proposalDecision(VerdictAllow, "proposal-allowed"), + Auditor: AuditFunc(func(context.Context, AuditEvent) error { return nil }), + }) + if err != nil { + t.Fatal(err) + } + if err := enforcer.AuthorizeProposal(context.Background(), operatorScope(t), input); err != nil { + t.Fatalf("AuthorizeProposal() changed after caller map mutation: %v", err) + } +} + +func TestEnforcerAuthorizesBoundProposalConcurrently(t *testing.T) { + const workers = 64 + var hookCalls atomic.Int64 + var auditCalls atomic.Int64 + enforcer, err := NewEnforcer(Config{ + Hook: HookFunc(func(context.Context, Request) (Decision, error) { + hookCalls.Add(1) + return Decision{Verdict: VerdictAllow, ReasonCode: "proposal-allowed"}, nil + }), + Auditor: AuditFunc(func(context.Context, AuditEvent) error { + auditCalls.Add(1) + return nil + }), + }) + if err != nil { + t.Fatal(err) + } + input := testProposalInput(t) + scope := operatorScope(t) + var group sync.WaitGroup + errorsByWorker := make(chan error, workers) + for worker := 0; worker < workers; worker++ { + group.Add(1) + go func() { + defer group.Done() + errorsByWorker <- enforcer.AuthorizeProposal(context.Background(), scope, input) + }() + } + group.Wait() + close(errorsByWorker) + for err := range errorsByWorker { + if err != nil { + t.Fatalf("AuthorizeProposal() concurrent error = %v", err) + } + } + if hookCalls.Load() != workers || auditCalls.Load() != workers { + t.Fatalf("hook/audit calls = %d/%d, want %d/%d", hookCalls.Load(), auditCalls.Load(), workers, workers) + } +} + +func FuzzProposalInputRejectsTamperedBinding(f *testing.F) { + f.Add("target-a", uint8(0)) + f.Add("target-b", uint8(5)) + f.Fuzz(func(t *testing.T, replacement string, field uint8) { + input := testProposalInput(t) + marker := fmt.Sprintf("caller-%x", sha256.Sum256([]byte(replacement))) + switch field % 6 { + case 0: + input.intentID = marker + case 1: + input.actor = marker + case 2: + input.verb = intent.Verb(marker) + case 3: + input.target.Name = marker + case 4: + input.argumentsDigest = marker + case 5: + input.resolvedDigest = marker + } + var hookCalls atomic.Int64 + var audits []AuditEvent + enforcer, err := NewEnforcer(Config{ + Hook: HookFunc(func(context.Context, Request) (Decision, error) { + hookCalls.Add(1) + return Decision{Verdict: VerdictAllow, ReasonCode: "unexpected"}, nil + }), + Auditor: AuditFunc(func(_ context.Context, event AuditEvent) error { + audits = append(audits, event) + return nil + }), + }) + if err != nil { + t.Fatal(err) + } + err = enforcer.AuthorizeProposal(context.Background(), operatorScope(t), input) + if err == nil { + t.Fatal("AuthorizeProposal() accepted a tampered proposal") + } + if strings.Contains(err.Error(), marker) || strings.Contains(fmt.Sprintf("%#v", audits), marker) { + t.Fatalf("AuthorizeProposal() leaked caller material") + } + if hookCalls.Load() != 0 { + t.Fatalf("policy hook calls = %d, want zero", hookCalls.Load()) + } + if len(audits) != 1 || audits[0].Action != tenancy.ActionProposeIntent || audits[0].Verdict != VerdictDeny || audits[0].ReasonCode != "invalid-request" { + t.Fatalf("tampered proposal audits = %#v, want one sanitized invalid-request denial", audits) + } + }) +} + +func proposalDecision(verdict Verdict, reason string) PolicyHook { + return HookFunc(func(context.Context, Request) (Decision, error) { + return Decision{Verdict: verdict, ReasonCode: reason}, nil + }) +} + +func testProposalInput(t testing.TB) ProposalInput { + t.Helper() + return testProposalInputFor(t, "user:operator", "workspace-a") +} + +func testProposalInputFor(t testing.TB, actor string, workspaceID tenancy.WorkspaceID) ProposalInput { + t.Helper() + input, err := NewProposalInput( + "intent-230", workspaceID, actor, intent.VerbDeploymentRestart, + testProposalTarget(), digestFor("validated-arguments"), + ) + if err != nil { + t.Fatalf("NewProposalInput() error = %v", err) + } + return input +} + +func testProposalTarget() fleet.ResourceRef { + return fleet.ResourceRef{SourceKind: "argocd", Scope: "cluster-a", Kind: "deployment", Namespace: "payments", Name: "payments"} +} + +func digestFor(value string) string { + return NewReadInput(VerbFleetRead, []byte(value)).ArgumentsDigest +} + +func operatorScope(t testing.TB) tenancy.Scope { + t.Helper() + return scopeForRole(t, tenancy.RoleOperator) +} + +func scopeForRole(t testing.TB, role tenancy.Role) tenancy.Scope { + t.Helper() + subject := "user:" + string(role) + principal, err := tenancy.NewPrincipal(subject, map[tenancy.WorkspaceID]tenancy.Role{"workspace-a": role}) + if err != nil { + t.Fatal(err) + } + scope, err := principal.Scope("workspace-a") + if err != nil { + t.Fatal(err) + } + return scope +} + +type proposalCase struct { + intentID string + workspaceID tenancy.WorkspaceID + actor string + verb intent.Verb + target fleet.ResourceRef + argumentsDigest string +} + +func (value proposalCase) with(mutate func(*proposalCase)) proposalCase { + mutate(&value) + return value +} + +func (value proposalCase) String() string { + return fmt.Sprintf("%s/%s/%s", value.workspaceID, value.verb, value.intentID) +}