feat: surface caveat evaluation diagnostics for denied checks#2933
Open
ugoenyioha wants to merge 1 commit intoauthzed:mainfrom
Open
feat: surface caveat evaluation diagnostics for denied checks#2933ugoenyioha wants to merge 1 commit intoauthzed:mainfrom
ugoenyioha wants to merge 1 commit intoauthzed:mainfrom
Conversation
…hzed#2802) When CheckPermission returns NOT_MEMBER because a caveat evaluated to false and debugging is enabled, the response now includes diagnostic information identifying which specific caveat(s) caused the denial. Diagnostics are gated behind the existing debug mechanism (the io.spicedb.requestdebuginfo header or WithTracing) to avoid: - Information disclosure: caveat expressions and context values would leak schema internals and relationship data in production responses - Performance impact: re-evaluation of caveat expressions on every denied caveated check Changes: - Add CaveatEvalResult message to internal dispatch proto with fields for caveat name, result, expression, context, and missing params - Add CaveatEvalInfo field to ResourceCheckResult in dispatch proto - Extend ExpressionResult interface with LeafCaveatResults() to collect per-leaf caveat evaluation results from expression trees - In computeCaveatedCheckResult, re-evaluate with debug enabled on NOT_MEMBER to collect per-caveat diagnostics (only when DebugOption is not NoDebugging) - Surface diagnostics via gRPC response trailer metadata (io.spicedb.respmeta.caveat-eval-info) as JSON, requiring no changes to the public API proto in authzed/api - Add 5 test cases covering: single false caveat, written context precedence, intersection with mixed results, non-caveated denial, and allowed result; each test also verifies diagnostics are NOT populated without debug mode Fixes authzed#2802 Ref authzed#1970
|
CLA Assistant Lite bot: I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request |
github-actions
bot
added
area/api v1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CaveatEvalResultandcaveat_eval_infoto internal dispatch check results so denied caveated checks can report which caveat(s) evaluated to false.computeCaveatedCheckResultand attach diagnostics only when debug/tracing is enabled, avoiding unconditional data exposure and extra hot-path cost.io.spicedb.respmeta.caveat-eval-info) and add targeted tests covering false caveats, mixed intersection outcomes, and no-diagnostics behavior without debug mode.Validation
go test ./internal/graph/computed/... -count=1 -timeout=300sgo test ./internal/services/v1/... -count=1 -timeout=300sgo test ./internal/caveats/... ./pkg/caveats/... -count=1 -timeout=300s