Skip to content

feat(security): RBAC — single-decision authz/check endpoint (#3054)#3231

Merged
houko merged 4 commits into
mainfrom
feat/rbac-authz-check
Apr 26, 2026
Merged

feat(security): RBAC — single-decision authz/check endpoint (#3054)#3231
houko merged 4 commits into
mainfrom
feat/rbac-authz-check

Conversation

@houko

@houko houko commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

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-permissions snapshot and reproduce the gate logic client-side.

Surface

GET /api/authz/check?user=<id-or-name>&action=<tool>&channel=<opt>

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:

  • This endpoint is a diagnostic for operators / external integrators
  • A misconfigured channel binding ("oh I bound telegram = "12345" but the actual user id is 123450") would silently look identical to a registered user with no policy under guest fallback
  • Surfacing 404 forces the caller to notice the gap

The 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 Verifies
test_authz_check_returns_allow_for_permitted_tool Bob (user) with allowed_tools=[web_search]allow
test_authz_check_returns_deny_for_blocked_tool Bob with denied_tools=[shell_exec]deny, reason contains the tool name
test_authz_check_unknown_user_returns_404 Path that would silently fall through to guest in runtime → 404 here
test_authz_check_viewer_caller_rejected_403 Viewer can't query authz/check (RBAC self-protection)
test_authz_check_rejects_anonymous 401 from middleware, same shape as other admin endpoints

Per 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_permissions handler 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_permissions infrastructure).

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.
@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

@github-actions github-actions Bot added size/L 250-999 lines changed ready-for-review PR is ready for maintainer review labels Apr 26, 2026
houko and others added 2 commits April 26, 2026 21:09
`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.
@github-actions github-actions Bot added the area/kernel Core kernel (scheduling, RBAC, workflows) label Apr 26, 2026
@houko
houko merged commit 355097e into main Apr 26, 2026
@houko
houko deleted the feat/rbac-authz-check branch April 26, 2026 12:47
@github-actions github-actions Bot removed the ready-for-review PR is ready for maintainer review label Apr 26, 2026
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.
@houko houko mentioned this pull request Apr 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/kernel Core kernel (scheduling, RBAC, workflows) size/L 250-999 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(security): unified granular RBAC — user-centric permissions across all channels, tools, agents, and memory

1 participant