Guard Cedar policy attribute access with has operator#41
Merged
Conversation
The safe-tools Cedar policies accessed tool annotation attributes (readOnlyHint, destructiveHint, openWorldHint) directly in `when` clauses. When an MCP server omits an annotation, Cedar treats the missing attribute access as an evaluation error. Toolhive's pre-flight authz check then hard-denies the tool, even if another policy would have permitted it. This broke safe-tools for servers that set partial annotations (GitHub MCP server sets readOnlyHint but not destructiveHint; context7 sets only readOnlyHint). All their read-only tools were incorrectly denied. Fix by prepending `resource has <attr> &&` before each attribute access. Missing attributes now evaluate to false (policy not satisfied) instead of error, letting other policies permit the tool. Also rewrites profiles_test.go to exercise the actual Cedar engine against 6 realistic annotation scenarios x 2 profiles x 2 actions, replacing the previous substring-matching tests that wouldn't have caught this bug. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
resource has <attr> &&guards before accessing tool annotation attributesreadOnlyHintbut notdestructiveHint) had all their tools incorrectly denied — Cedar treats missing attribute access as an evaluation error, and toolhive's pre-flight check treats any error as a hard denyprofiles_test.goto exercise the actual Cedar engine against 6 annotation scenarios × 2 profiles × 2 actions (24 cases), replacing substring-matching tests that wouldn't have caught this bugBefore: safe-tools allowed only 7 tools (osv + oci-registry — the only servers with complete annotations)
After: safe-tools allows 59 tools (all read-only tools from GitHub, context7, osv, oci-registry)
Test plan
task verifypasses (fmt, lint, test)TestCedarAuthorizationDecisionscovers the exact partial-annotation scenario that was broken (scenario 3: onlyreadOnlyHint=true, no other hints)🤖 Generated with Claude Code