feat(security): RBAC — single-decision authz/check endpoint (#3054)#3231
Merged
Conversation
Closes the last Phase-4 surface gap on the RBAC umbrella: external
callers can now ask "can user X invoke tool Y on channel Z?" without
having to fetch the full effective-permissions snapshot and reproduce
the gate logic client-side.
Surface
- GET /api/authz/check?user=<id-or-name>&action=<tool>&channel=<opt>
- Admin-only (same require_admin gate as /api/authz/effective).
- Returns:
{ user, action, channel,
decision: "allow" | "deny" | "needs_approval",
allowed: bool,
reason: string | null }
Implementation
- Single line of substance: calls AuthManager::resolve_user_tool_decision
with system_call=false. That's the SAME function the runtime gate path
uses, so this endpoint never drifts from production decisions.
- 404 when the user doesn't match a registered identity. The runtime
treats unknown senders as guests; surfacing the same shape on this
diagnostic endpoint would silently mask a misconfigured channel
binding from the operator. We bail BEFORE calling the gate.
Tests (5 new)
- allow path (per-user allowed_tools)
- deny path (per-user denied_tools, reason includes the tool name)
- unknown user → 404
- viewer caller → 403
- anonymous → 401 (middleware short-circuits, same shape as other
admin endpoints)
This is the last item on the "close #3054" punch list.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
`resolve_user_tool_decision` requires a (channel, sender_id) pair to resolve a user identity. The check route only has the canonical UserId, so passing the stringified id with channel=None silently fell through to the guest gate and returned `needs_approval` for users whose policy actually hard-denies the action. Introduce `AuthManager::resolve_decision_for_user` that takes the already-resolved UserId and runs the same Layer A/B walk, then have the diagnostic endpoint call it. Behaviour for runtime gate paths is unchanged — they still go through the channel-keyed entry point. Fixes test_authz_check_returns_deny_for_blocked_tool.
houko
added a commit
that referenced
this pull request
Apr 26, 2026
…ion (#3242) The /api/authz/check endpoint was documented as 'production single source of truth' for the runtime gate, but it only walks AuthManager::resolve_decision_for_user (Layer A user policy + Layer B role escalation). The runtime tool-gate also intersects with: - per-agent ToolPolicy::check_tool (agent.toml allow/blocklist) - global ApprovalPolicy.channel_rules (e.g. shell_* always-approve) neither of which depends on inputs this endpoint accepts. So an 'allow' here can still surface as 'needs_approval' or 'deny' at runtime. Document-of-truth fix: keep the existing decision logic unchanged (callers depend on it), but advertise the actual scope honestly. Changes: - Replace ad-hoc serde_json::json! body with typed AuthzCheckResponse struct (utoipa::ToSchema) carrying . - Long docstring on the handler + the response struct explaining the layers NOT consulted. - Update OpenAPI 200 response to point at the new struct. Tests added: - Existing test_authz_check_returns_allow_for_permitted_tool extended to assert body["scope"] == "user_policy_only". - New test_authz_check_response_advertises_user_policy_only_scope pins the scope marker on both Allow and Deny paths. Future RFC tracked: widening to the full gate path requires an agent_id query param (breaking change) — out of scope here. Refs PR #3231 review item #14.
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
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
Closes the last Phase-4 surface gap on the RBAC umbrella issue #3054. External callers (skills / channel adapters / monitoring tools) can now ask "can user X invoke tool Y on channel Z?" without having to fetch the full
effective-permissionssnapshot and reproduce the gate logic client-side.Surface
require_admingate as/api/authz/effective(feat(security): RBAC effective-permissions snapshot — wire simulator (#3054) #3228).{ "user": "Bob", "action": "shell_exec", "channel": "telegram", "decision": "allow" | "deny" | "needs_approval", "allowed": true, "reason": null }Why "single source of truth"
The handler is one substantive line: it calls
AuthManager::resolve_user_tool_decision(action, Some(&user_id_str), channel.as_deref(), false)— the same function the runtime gate path calls on every tool invocation. We deliberately do NOT reproduce the four-layer intersection here; reproducing it would create a parallel implementation that could silently drift from production. If the runtime would deny the call, this endpoint says "deny" with the same reason string the model would see.404 vs guest fallback
The runtime treats unknown senders as guests and routes them through
guest_gate(tool_name). This endpoint deliberately does NOT mirror that — instead it returns 404 when the user can't be matched. Reasoning:telegram = "12345"but the actual user id is123450") would silently look identical to a registered user with no policy under guest fallbackThe runtime continues to use guest fallback because production traffic must always get a decision (denying with a useful reason beats hanging).
Tests (5 new)
test_authz_check_returns_allow_for_permitted_toolallowed_tools=[web_search]→allowtest_authz_check_returns_deny_for_blocked_tooldenied_tools=[shell_exec]→deny, reason contains the tool nametest_authz_check_unknown_user_returns_404test_authz_check_viewer_caller_rejected_403test_authz_check_rejects_anonymousPer the project's "禁止 build" instruction this PR is not locally cargo-checked; CI will validate. The handler complexity is minimal (one function call to a well-tested kernel method) and follows the exact pattern of the sibling
effective_permissionshandler that landed in #3228.Closes #3054 punch list
After this PR merges, every Phase-4 checkbox in the umbrella issue is delivered (LDAP/OIDC was split off into #3226 to unblock the close). #3054 is ready to close.
Refs #3054, builds on #3228 (
effective_permissionsinfrastructure).