Skip to content

feat(security): RBAC M3 — per-user tool policy + memory namespace ACL (#3054)#3205

Merged
houko merged 10 commits into
mainfrom
feat/rbac-m3-tool-memory-policy
Apr 26, 2026
Merged

feat(security): RBAC M3 — per-user tool policy + memory namespace ACL (#3054)#3205
houko merged 10 commits into
mainfrom
feat/rbac-m3-tool-memory-policy

Conversation

@houko

@houko houko commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

RBAC M3 Phase 2 — wire per-user tool policy and memory namespace
ACL
through every production path. Refs #3054.

Rework (PR #3205 review feedback)

The first three commits scaffolded the types, runtime gate, and memory
ACL helpers. This rework wires each one to the real call sites and
closes the bypass routes the review found. The PR no longer claims any
enforcement that isn't actually live.

BLOCKING — fixed

  • B1. Memory ACL was dead code. MemoryNamespaceGuard is now built
    at every production read site:
    • GET /api/memory/searchsearch_all_with_guard
    • GET /api/memorylist_all_with_guard
    • GET /api/memory/user/{id}get_with_guard
    • GET /api/memory/agents/{id}list_with_guard
    • GET /api/memory/agents/{id}/searchsearch_with_guard
    • Agent-loop auto_retrieve now resolves the guard from the
      attributed end-user (via KernelHandle::memory_acl_for_sender)
      and short-circuits if proactive reads are denied; PII redaction
      is applied to the items it does return.
    • Denial yields HTTP 403 (not a generic 500).
  • B2. shell_exec Full mode silently dropped user-gate NeedsApproval.
    Re-gated skip_approval_for_full_exec on !force_approval so a
    user-policy escalation MUST still route through the approval queue
    even under ExecPolicy.mode = Full.
  • B3. Hand-agent auto-approval bypassed user-gate NeedsApproval.
    Added DeferredToolExecution.force_human (defaults false,
    #[serde(default)]-compatible). The runtime sets it to true when
    the user gate returned NeedsApproval; the kernel's
    submit_tool_approval skips the hand:* carve-out when
    force_human=true. Hand-agents still auto-approve for the standard
    require_approval path — the carve-out is preserved everywhere it
    was safe.

HIGH — fixed

  • H4. config_reload did not rebuild AuthManager. Added
    HotAction::ReloadAuth. Detected when [[users]] or
    [tool_policy.groups] change. AuthManager::reload(&[UserConfig], &[ToolGroup]) clears and repopulates the indexes in place. Design
    decision Bump json5 from 0.4.1 to 1.3.1 #7 (invalidate per-user permission cache on reload) is
    honoured for the first time.
  • H5. Precedence docstring vs PR body mismatch. Pinned the
    canonical order to tool_policy → channel_tool_rules → tool_categories
    in the user_policy.rs module docstring. Added a regression test
    asserting user.denied=["foo"] beats channel.allowed=["foo"].
  • H6. sender_index first-write-wins on platform-id collisions.
    Removed the bare-platform_id fallback. resolve_user now requires
    an explicit (channel, platform_id) tuple. Two users sharing a
    platform-id on different channels can no longer alias on a third
    unbound channel.
  • H7. sender_id.is_none() failed OPEN. Replaced with an explicit
    system_call: bool flag on
    AuthManager::resolve_user_tool_decision. The kernel's KernelHandle
    impl marks cron / system / internal channels as system calls;
    every other no-sender path now goes through the guest gate
    (default-deny) instead of returning Allow.
  • H8. PR body claimed a test that didn't exist. Added an
    end-to-end integration test (tests/rbac_m3_evaluate_tool_call.rs)
    that boots a real LibreFangKernel and exercises the user/role/
    category/reload matrix through the actual KernelHandle trait.

What's actually enforced (honest list)

Surface Guard / gate Test
tool_runner::execute_tool (every tool dispatch) KernelHandle::resolve_user_tool_decisionDeny/NeedsApproval/Allow tool_runner::tests::tool_runner_rbac_*
shell_exec under ExecPolicy.mode = Full force_approval overrides the Full bypass tool_runner_rbac_full_mode_does_not_bypass_user_needs_approval
Hand-tagged agent submitting tool approvals DeferredToolExecution.force_human disables the auto-approve carve-out submit_tool_approval_hand_agent_force_human_skips_auto_approve
GET /api/memory* (search, list, get) MemoryNamespaceGuard built from the auth-middleware-attached user, with PII redaction search_all_with_guard_*
Agent-loop auto_retrieve Guard resolved via KernelHandle::memory_acl_for_sender(sender, channel); deny short-circuits, PII redacted covered indirectly by auto_retrieve path + namespace_acl tests
KV get_with_guard / set_with_guard / delete_with_guard MemoryNamespaceGuard (existing wrappers, unchanged) kv_namespace_guard_*
/api/config/reload editing [[users]] or [tool_policy.groups] HotAction::ReloadAuth rebuilds the AuthManager indexes evaluate_tool_call_reload_picks_up_new_policy
Cross-channel platform-id collision resolve_user requires explicit (channel, sid); no bare-id fallback rbac_m3_platform_id_collision_no_longer_aliases_across_channels
Internal callers without sender_id Default-deny via guest gate; explicit system_call=true opt-out rbac_m3_sender_none_no_system_flag_does_not_fail_open

Test plan (new tests added in this rework)

  • crates/librefang-types/src/user_policy.rs
    • evaluate_user_deny_beats_channel_allow_for_same_tool — H5 precedence pin
  • crates/librefang-runtime/src/tool_runner.rs::tests
    • tool_runner_rbac_full_mode_does_not_bypass_user_needs_approval — B2
    • tool_runner_rbac_force_human_propagates_to_deferred — B3 wire-through
    • tool_runner_rbac_force_human_stays_false_for_global_require_approval — B3 negative
  • crates/librefang-memory/src/proactive.rs::tests
    • search_all_with_guard_denies_unauthorised_read — B1 deny path
    • search_all_with_guard_redacts_pii — B1 redaction
  • crates/librefang-kernel/src/auth.rs::tests
    • rbac_m3_platform_id_collision_no_longer_aliases_across_channels — H6
    • rbac_m3_sender_none_no_system_flag_does_not_fail_open — H7
  • crates/librefang-kernel/tests/rbac_m3_evaluate_tool_call.rs (new file)
    • evaluate_tool_call_user_deny_short_circuits — case (a)
    • evaluate_tool_call_both_allow — case (c)
    • evaluate_tool_call_user_role_no_allow_list_needs_approval — case (d)
    • evaluate_tool_call_user_categories_resolve_against_kernel_groups
    • evaluate_tool_call_unrecognised_sender_no_longer_fail_open
    • submit_tool_approval_hand_agent_force_human_skips_auto_approve — B3 kernel-side
    • evaluate_tool_call_reload_picks_up_new_policy — H4

Verify locally

cargo check -p librefang-types -p librefang-runtime -p librefang-memory -p librefang-kernel -p librefang-api
cargo test  -p librefang-types  -- user_policy
cargo test  -p librefang-runtime --lib -- tool_runner
cargo test  -p librefang-memory -- namespace_acl
cargo test  -p librefang-kernel  -- auth
cargo test  -p librefang-kernel --test rbac_m3_evaluate_tool_call
cargo test  -p librefang-api    -- memory
cargo clippy -p librefang-types -p librefang-runtime -p librefang-memory -p librefang-kernel -p librefang-api -- -D warnings

Notes / follow-ups

  • (b) "user allows but agent doesn't have capability" → Deny is
    enforced by the existing capability check at the top of
    execute_tool (line ~1022), not by the new RBAC layer. A test at
    the kernel boundary for that case would require booting an agent
    with a constrained capability list — left as out of scope; the
    capability check is unchanged.
  • The KernelHandle::resolve_user_tool_decision default impl was
    briefly flipped to NeedsApproval (NIT from review) but reverted —
    the change broke 8 unrelated runtime tests that rely on the default
    mock kernel. The trait doc now explicitly notes this trade-off.
  • is_unconfigured() semantics documented (intentionally all-or-nothing).
  • metadata.entry().or_insert_with() swapped to .insert() so the
    redaction signal is authoritative.

Refs #3054

houko added 3 commits April 26, 2026 10:00
Adds the data layer for RBAC M3 (issue #3054 Phase 2):

- New module librefang_types::user_policy with UserToolPolicy,
  UserToolCategories, ChannelToolPolicy, UserMemoryAccess, and a
  layered ResolvedUserPolicy::evaluate helper returning
  UserToolDecision::{Allow, Deny, NeedsRoleEscalation}.
- Extends UserConfig with optional tool_policy, tool_categories,
  memory_access, and channel_tool_rules fields. All default to
  None/empty so existing config.toml files keep working unchanged.
- Round-trip serde tests (JSON + TOML) for every new struct, plus
  layering precedence tests for evaluate.
Wires RBAC M3 tool checks (#3054 Phase 2) into the agent runtime.

Kernel side (librefang-kernel):
- AuthManager now caches a ResolvedUserPolicy per user, built from
  UserConfig.{tool_policy,tool_categories,memory_access,channel_tool_rules}
  at config load. with_tool_groups() takes the kernel ToolGroup list so
  per-user category lookups can resolve group names.
- New resolve_user_tool_decision() returns UserToolGate::{Allow, Deny,
  NeedsApproval}. Order: user tool_policy -> per-user channel rule ->
  tool_categories -> role escalation. Admin/owner role passes through;
  user/viewer escalates unknown tools to NeedsApproval; explicit deny
  short-circuits.
- New memory_acl_for() merges the user UserMemoryAccess with a
  role-default ACL (owner/admin = full, user = proactive+kv:*, viewer =
  proactive read-only).
- Unknown senders fall through guest_gate(): allow well-known read-only
  tools, NeedsApproval for everything else. Empty user list keeps legacy
  behaviour (UserToolGate::Allow everywhere).

Runtime side (librefang-runtime tool_runner):
- After the existing channel-deny check and before the approval gate,
  consult kernel.resolve_user_tool_decision(). Deny short-circuits with
  a hard error containing the reason. NeedsApproval flips
  force_approval=true so the tool routes through submit_tool_approval()
  regardless of the global require_approval list. Allow defers to the
  existing approval logic (no bypass).

Trait surface (librefang-kernel-handle):
- New default-Allow KernelHandle::resolve_user_tool_decision so existing
  test stubs and downstream KernelHandle impls keep compiling unchanged.

Tests:
- 8 new auth.rs tests covering tool_policy deny, role escalation,
  channel-rule precedence, category resolution, guest gate, memory ACL
  fallback, memory ACL override.
- 3 new tool_runner.rs tests driving execute_tool() through each
  UserToolGate variant via a stub KernelHandle.
Adds the memory layer for RBAC M3 (#3054 Phase 2):

- New module librefang_memory::namespace_acl with MemoryNamespaceGuard,
  NamespaceGate, and PII-redaction helpers. The guard wraps the per-user
  UserMemoryAccess ACL and exposes check_read/check_write/check_delete/
  check_export plus redact_item/redact_all.
- Redaction logic checks two signals on each MemoryItem:
  metadata['taint_labels'] containing 'Pii' (replaces full content), or
  the regex stack from taint::redact_pii_in_text (replaces just the
  matched substrings). When pii_access is true, the guard never
  redacts.
- New public taint::redact_pii_in_text() exposes the existing
  email/phone/SSN/credit-card regex stack so the memory crate can reuse
  it without copying.

Wires the guard into two real call sites:
- ProactiveMemoryStore gains search_with_guard / delete_with_guard /
  add_with_guard. Search runs PII redaction on output; delete honours
  delete_allowed; add gates writes to the 'proactive' namespace.
- StructuredStore gains get_with_guard / set_with_guard /
  delete_with_guard. The KV namespace presented to the guard is
  'kv:<key>' so policies can use prefix patterns
  (e.g. readable_namespaces = ['kv:user_*']).

Tests: 11 new tests covering namespace allow/deny, delete/export flag
gating, PII metadata path, PII regex path, pii_access bypass, KV guard
read/write/delete, and prefix matching against globs.
@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 ready-for-review PR is ready for maintainer review area/runtime Agent loop, LLM drivers, WASM sandbox area/kernel Core kernel (scheduling, RBAC, workflows) labels Apr 26, 2026
@github-actions github-actions Bot added the size/XL 1000+ lines changed label Apr 26, 2026
github-actions Bot and others added 4 commits April 26, 2026 01:25
Closes the bypass routes and dead-code paths that PR #3205 review
flagged as BLOCKING. Refs #3054.

B1 — Memory namespace ACL was scaffolded but every read site still
called the unguarded methods. Routed through *_with_guard variants:

  * api/routes/memory.rs: search/list/get/list_agent/search_agent
    endpoints now build a MemoryNamespaceGuard from the
    AuthenticatedApiUser extension (via AuthManager::memory_acl_for)
    and return 403 on deny.
  * runtime/agent_loop.rs: auto_retrieve resolves the guard via the
    new KernelHandle::memory_acl_for_sender, short-circuits the call
    when "proactive" is denied, and applies PII redaction otherwise.
  * memory/proactive.rs: added search_all_with_guard /
    list_all_with_guard / get_with_guard / list_with_guard wrappers
    for cross-agent paths.
  * memory/namespace_acl.rs: switched the redacted=true marker to
    insert() instead of or_insert_with() so the signal is
    authoritative.

B2 — shell_exec under ExecPolicy.mode=Full silently dropped a
user-gate NeedsApproval. Re-gated skip_approval_for_full_exec on
!force_approval so a user-policy escalation MUST still route through
the approval queue even under Full mode.

B3 — Hand-tagged agent auto-approval ignored the user-gate
NeedsApproval. Threaded a force_human flag through
DeferredToolExecution (default false, #[serde(default)]-compatible).
The runtime sets it true when the user gate returned NeedsApproval;
the kernel's submit_tool_approval skips the hand:* carve-out when
force_human=true.

Tests:
  - tool_runner_rbac_full_mode_does_not_bypass_user_needs_approval (B2)
  - tool_runner_rbac_force_human_propagates_to_deferred (B3)
  - tool_runner_rbac_force_human_stays_false_for_global_require_approval
  - search_all_with_guard_denies_unauthorised_read (B1)
  - search_all_with_guard_redacts_pii (B1)

Refs #3054, #3205
Closes the remaining HIGH findings from PR #3205 review. Refs #3054.

H4 — `/api/config/reload` did not rebuild AuthManager. Editing
`[[users]]`, `[users.tool_policy]`, or `[tool_policy.groups]` then
hitting `/api/config/reload` was a silent no-op — design decision #7
("invalidate per-user permission cache on config reload") was being
violated. Added `HotAction::ReloadAuth`, detected when `users` or
`tool_policy.groups` change. `AuthManager::reload(&[UserConfig],
&[ToolGroup])` clears and repopulates the indexes in place.

H5 — Precedence docstring didn't match the impl. Pinned the canonical
order `tool_policy → channel_tool_rules → tool_categories` in the
`user_policy.rs` module docstring (matches `evaluate()`'s actual
short-circuit order). Added a regression test:
`evaluate_user_deny_beats_channel_allow_for_same_tool`.

H6 — `sender_index` first-write-wins on platform-id collisions.
Two users sharing a platform-id on different channels would alias on
a third unbound channel; if the first registered was Owner, an
unrecognised inbound on a third channel inherited Owner rights.
Removed the bare-`platform_id` index; `resolve_user` now requires an
explicit `(channel, sid)` tuple.

H7 — `sender_id.is_none()` failed OPEN. `resolve_user_tool_decision`
returned `Allow` whenever `sender_id` was missing, bypassing RBAC for
any internal call site. Added an explicit `system_call: bool` flag —
the kernel marks cron / system / internal channels as system calls;
every other no-sender path now goes through the guest gate
(default-deny).

H8 — End-to-end integration test (`tests/rbac_m3_evaluate_tool_call.rs`)
that boots a real `LibreFangKernel` and exercises the full chain:
user_deny / both_allow / no_allow_list_needs_approval /
categories_against_kernel_groups / unrecognised_sender_no_fail_open /
hand_agent_force_human_skips_auto_approve / reload_picks_up_new_policy.
Replaces the test claim in the PR body that pointed to no actual file.

Tests:
  - rbac_m3_platform_id_collision_no_longer_aliases_across_channels (H6)
  - rbac_m3_sender_none_no_system_flag_does_not_fail_open (H7)
  - evaluate_user_deny_beats_channel_allow_for_same_tool (H5)
  - tests/rbac_m3_evaluate_tool_call.rs — 7 cases (H4 + H8)

Refs #3054, #3205
@cloudflare-workers-and-pages

Copy link
Copy Markdown

@github-actions github-actions Bot added has-conflicts PR has merge conflicts that need resolution and removed ready-for-review PR is ready for maintainer review labels Apr 26, 2026
houko added 2 commits April 26, 2026 15:20
CI was failing on `kernel_config_schema_matches_golden_fixture` because
the M3 schema additions (`tool_policy`, per-user `tool_categories`,
`memory_access`) never had the golden fixture regenerated — actual
14639 lines vs expected 14496.

While in here, address two PR review items:

  1. `KernelHandle::resolve_user_tool_decision` was treating
     `(sender_id=None, channel=None)` AND `Some("system")` /
     `Some("internal")` as system-internal calls, silently re-opening
     the H7 fail-open the AuthManager unit tests were written to
     close. Only the synthetic `"cron"` channel (the one actually
     synthesized in `kernel/mod.rs ~10876`) keeps the bypass; every
     other unattributed inbound now flows through the guest gate so
     RBAC fails closed end-to-end. New trait-layer test pins this.

  3. `routes/memory.rs::guard_for_request` falls back to an
     owner-equivalent guard when no AuthenticatedApiUser extension is
     attached. The original comment described this as "preserves the
     M2 contract" without flagging the security trade-off; replaced
     with an explicit SECURITY note that the per-user namespace ACL
     only applies when the auth middleware binds a real user, and
     calls out what operators must do to enforce it on every request.

Also: swap `RwLock<Vec<ToolGroup>>` for `RwLock<Arc<Vec<ToolGroup>>>`
in AuthManager so `config_reload`'s in-place swap stays cheap while
the resolution-path readers only pay an `Arc::clone` instead of a
per-call `Vec` clone.
…ory-policy

# Conflicts:
#	crates/librefang-kernel/src/kernel/mod.rs
#	crates/librefang-types/src/taint.rs
@github-actions github-actions Bot added ready-for-review PR is ready for maintainer review and removed has-conflicts PR has merge conflicts that need resolution labels Apr 26, 2026
@houko
houko merged commit 9217556 into main Apr 26, 2026
20 checks passed
@houko
houko deleted the feat/rbac-m3-tool-memory-policy branch April 26, 2026 06:37
@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
…orrupt config

Address PR #3209 review:

1. BLOCKING — Admin per-user API key could POST /api/users to create a new
   `role: "owner"` user with a chosen `api_key_hash` and self-promote. The
   handlers had no role check and the comment said the gate would arrive
   "with M3" — M3 (#3205) did not in fact add it. Extend the existing
   `is_owner_only_write` allowlist with a `/api/users*` prefix so every
   mutating call under that surface requires Owner, mirroring
   `Action::ManageUsers` in the kernel. GET stays Admin-or-above so the
   permission simulator's user list keeps working.

2. HIGH — `persist_users` had `unwrap_or_default()` on both
   `read_to_string` and `toml::parse`, so a transient read failure or a
   hand-edited-into-corruption `config.toml` would silently get replaced
   by a stub document containing only `[[users]]` — wiping every other
   section. Both branches now return `PersistError::Internal`; corrupt
   files stay untouched on disk so `backups/config.toml.prev` plus the
   bad file are sufficient for human recovery.

3. MEDIUM — `api_key_hash` was accepted as any string. New
   `validate_api_key_hash` runs `argon2::password_hash::PasswordHash::new`
   to reject anything that isn't a real Argon2id PHC string, applied in
   create / update / bulk import. Empty / whitespace still maps to
   `None` (clear the hash), unchanged.

Tests:
- middleware: `test_user_role_admin_cannot_mutate_users_endpoints` /
  `test_user_role_viewer_can_still_list_users_for_simulator`
- users_test: `users_create_rejects_invalid_api_key_hash` /
  `users_create_refuses_to_overwrite_corrupt_config_toml`
- middleware suite 34/34, users integration 10/10, clippy clean.
houko added a commit that referenced this pull request Apr 26, 2026
…ate flag-without-namespace combos

PR #3205 (M3) shipped two paper cuts that this commit closes. Both
land on the M5 branch because M5 is already touching the same files
(`crates/librefang-types/src/config/validation.rs` for the
`UserBudgetConfig` clamp, `crates/librefang-memory/src/namespace_acl.rs`
unchanged but in scope) and a separate PR would conflict with no
benefit.

1. **`MemoryNamespaceGuard::has_pii_label` is now case-insensitive.**
   The metadata-stored label is the `Display` form of `TaintLabel::Pii`
   (literal `"Pii"`), but external writers commonly hand-stamp `"PII"`
   or `"pii"`. The previous exact-match comparison silently skipped
   those, leaving structured PII fragments to fall through to the
   regex backstop — fine for free-form text but a leak for custom
   field names that have no e-mail/phone/SSN tokens to grep. Both the
   array and scalar-string metadata shapes now lowercase-normalise the
   target *and* candidate before comparing.

2. **`KernelConfig::validate()` now warns on `pii_access` /
   `export_allowed` / `delete_allowed` set with empty
   `readable_namespaces`.** The runtime guard requires read access
   for every code path those flags gate (PII redaction only runs on
   readable items; export/delete check write access AND the flag).
   An admin who toggles a flag without declaring namespaces gets a
   silent no-op that looks like it works — exactly the
   misconfiguration `validate()` exists to surface. Warning names the
   user, lists the flags, and suggests the missing key.

Tests:
- `redact_via_metadata_label_matches_case_insensitively` covers
  `"PII"` / `"pii"` / `"Pii"` for both array and scalar-string
  metadata shapes.
- `test_validate_warns_on_memory_access_flags_without_readable_namespaces`
  pins the warning shape: warns on the typo'd user, suppresses on a
  correctly-configured user, names both the user and the flag.

`cargo test -p librefang-memory --lib namespace_acl`: 9/9.
`cargo test -p librefang-types --lib config`: 139/139 plus the new
validate test.

(A third proposed item — startup warning when memory routes are in
the auth allowlist while users have memory_access policies — turned
out to be moot: `/api/memory/*` is not in any allowlist, the only
fallback path is the loopback bypass which is deliberate and
documented in `guard_for_request`.)
houko added a commit that referenced this pull request Apr 26, 2026
…or, CSV import + stubs) (#3054) (#3209)

* feat(api): admin-only user CRUD + bulk CSV import endpoints

Add a thin RBAC user-management surface so the dashboard slice (#3054 / M6)
can list, create, edit, delete and bulk-import operator accounts without
going through the generic config_set path.

- crates/librefang-api/src/routes/users.rs: GET / POST /api/users,
  GET / PUT / DELETE /api/users/{name}, POST /api/users/import. Writes
  rebuild the [[users]] array-of-tables in config.toml via toml_edit so
  unrelated comments and sections stay intact, then trigger a kernel
  reload.
- crates/librefang-kernel/src/auth.rs: AuthManager::replace_users — clears
  + repopulates the channel index in place so identity resolution picks
  up the new set without restart.
- crates/librefang-kernel/src/config_reload.rs + kernel/mod.rs: classify
  users diffs as a hot action (HotAction::ReloadUsers) and wire the
  apply_hot_actions handler.
- Sanitize the wire shape: UserView omits api_key_hash and only exposes
  has_api_key. Bulk import accepts a pre-parsed JSON body (the dashboard
  parses CSV client-side) and supports dry_run for preview.

Tests cover full CRUD, validation errors, duplicate detection, rename,
dry-run vs commit semantics, and round-trip persistence (boots a kernel
on a temp home_dir and walks the routes via tower::ServiceExt::oneshot).

Refs #3054

* feat(dashboard): users page with CRUD + identity-linking wizard

Surfaces the new admin-only /api/users endpoints from the previous commit
through a single Users page at /users. Three jobs:

- list/edit/delete users with search + role filter
- a 4-step identity-linking wizard (User -> Platform -> Identifier ->
  Confirm) that walks an admin through binding a Telegram / Discord /
  Slack / Email / WeChat platform_id to an existing user. Each platform
  tile carries the expected platform_id format so admins don't have to
  spelunk the channel docs.
- a drag-drop CSV bulk-import modal with client-side parsing, dry-run
  preview, and a separate commit step that maps to /api/users/import.
  CSV header is name,role,<platform>… — every non-name/role column is
  treated as a channel binding.

Data layer follows AGENTS.md to the letter:

- src/api.ts: listUsers / getUser / createUser / updateUser / deleteUser /
  importUsers + sanitized UserItem shape (no api_key_hash leak).
- lib/http/client.ts: explicit re-exports.
- lib/queries/keys.ts: userKeys factory (hierarchical, anchored).
- lib/queries/users.ts: useUsers / useUser with a client-side filter so
  pages can subscribe with role + search and still share the cache key
  for the underlying list.
- lib/mutations/users.ts: useCreateUser / useUpdateUser / useDeleteUser /
  useImportUsers — every write invalidates the narrowest matching keys
  (lists + the touched detail). The import hook invalidates userKeys.all
  per the bulk-reset escape hatch in AGENTS.md.

App.tsx grows a Users entry under Core; router.tsx grows the /users route.

Refs #3054

* feat(dashboard): permission simulator using existing 4-role enum

Add /users/simulator — pick a user, see which Action variants their role
allows. Decision is computed locally from the ROLE_ORDER index, mirroring
`librefang_kernel::auth::UserRole as u8` (Viewer < User < Admin < Owner).
The action -> required_role table is duplicated from `auth::Action` so the
matrix stays stable even when the daemon doesn't have a server-side
simulator endpoint yet.

When M3 (#3205) lands the per-user policy editor will refine these
results further; this page already pre-wires `usePermissionPolicy` via
the stub hook so hooking it up is a one-line swap.

Refs #3054

* feat(dashboard): audit viewer + budget charts + permission matrix stubs awaiting M3/M5

Three thin pages whose payload depends on still-open PRs:

- /audit: searchable / filterable audit trail. Real shape is wired via
  `useAuditQuery` in lib/queries/audit.ts hitting GET /api/audit/query.
  Endpoint ships with M5 (#3203). Hook is invoked with enabled: false
  so we don't 404-spam pre-M5 daemons; the page renders a placeholder
  pointing at the dependency.
- /users/{name}/budget: per-user spend charts + alert thresholds.
  `useUserBudget` keyed via userBudgetKeys.detail(name) maps to
  /api/budget/users/{name}, also gated until M5.
- /users/{name}/policy: per-user tool/memory policy matrix editor.
  `usePermissionPolicy` + `useUpdateUserPolicy` hit
  /api/users/{name}/policy, gated until M3 (#3205) ships the endpoint.

Key factories are added to keys.ts (auditKeys.query / queries,
userBudgetKeys, permissionPolicyKeys) and registered in keys.test.ts so
the anchoring + 'all factories exist' invariants stay green. The plumbing
(query hooks, mutation invalidation, page routing, nav entry) lands now
so the only delta when M3/M5 merge is dropping the `enabled: false` guard
and rendering the actual table/chart.

Refs #3054

* fix(rbac-m6): gate /api/users to Owner, validate hash, fail loud on corrupt config

Address PR #3209 review:

1. BLOCKING — Admin per-user API key could POST /api/users to create a new
   `role: "owner"` user with a chosen `api_key_hash` and self-promote. The
   handlers had no role check and the comment said the gate would arrive
   "with M3" — M3 (#3205) did not in fact add it. Extend the existing
   `is_owner_only_write` allowlist with a `/api/users*` prefix so every
   mutating call under that surface requires Owner, mirroring
   `Action::ManageUsers` in the kernel. GET stays Admin-or-above so the
   permission simulator's user list keeps working.

2. HIGH — `persist_users` had `unwrap_or_default()` on both
   `read_to_string` and `toml::parse`, so a transient read failure or a
   hand-edited-into-corruption `config.toml` would silently get replaced
   by a stub document containing only `[[users]]` — wiping every other
   section. Both branches now return `PersistError::Internal`; corrupt
   files stay untouched on disk so `backups/config.toml.prev` plus the
   bad file are sufficient for human recovery.

3. MEDIUM — `api_key_hash` was accepted as any string. New
   `validate_api_key_hash` runs `argon2::password_hash::PasswordHash::new`
   to reject anything that isn't a real Argon2id PHC string, applied in
   create / update / bulk import. Empty / whitespace still maps to
   `None` (clear the hash), unchanged.

Tests:
- middleware: `test_user_role_admin_cannot_mutate_users_endpoints` /
  `test_user_role_viewer_can_still_list_users_for_simulator`
- users_test: `users_create_rejects_invalid_api_key_hash` /
  `users_create_refuses_to_overwrite_corrupt_config_toml`
- middleware suite 34/34, users integration 10/10, clippy clean.

* fix(rbac-m6): persist RBAC M3 policy fields + simplify update_user

Re-review of #3209 found the preserve-and-merge logic in `update_user`
and `import_users` only protected the in-memory `Vec<UserConfig>` —
the on-disk write path in `persist_users` hand-emitted just the four M6
fields (name / role / channel_bindings / api_key_hash) and silently
dropped the M3 ones (tool_policy / tool_categories / memory_access /
channel_tool_rules). After every successful PUT, `reload_config()` read
the truncated file back and the per-user policy collapsed to None.

Two changes:

1. `persist_users` now serializes each user via `toml_edit::ser::to_document`
   so the full serde shape (incl. `#[serde(skip_serializing_if)]`-gated
   M3 fields) round-trips through `config.toml`. We use `toml_edit::ser`
   rather than `toml::ser` because `UserConfig` interleaves scalar fields
   (`api_key_hash`) after a nested table (`channel_bindings`), which the
   strict serializer rejects with `ValueAfterTable`. Enabled the `serde`
   feature on `toml_edit` to expose `to_document`.

2. Generalized `persist_users` to `FnOnce(&mut Vec<UserConfig>) -> Result<R>`
   and let the closure thread an arbitrary `R` back to the caller. The
   `Arc<Mutex<Option<UserConfig>>>` capture pattern in `update_user` is
   gone — closure now returns `Ok(users[idx].clone())` directly. Other
   call sites stay at `R = ()`.

Tests:
- New `users_update_and_import_preserve_rbac_m3_policy_fields` seeds a
  Bob with non-default `tool_policy` + `memory_access`, runs PUT (rename
  + role bump) and bulk-import update, asserts the policy fields survive
  by reading back through `state.kernel.config_ref()` (i.e. after the
  disk round-trip). This test FAILED before the persist_users fix.
- 11/11 users_test pass, clippy clean.
houko added a commit that referenced this pull request Apr 26, 2026
…3054) (#3203)

* feat(types): UserConfig.budget for per-user spend limits

RBAC M5 introduces per-user spending caps so an operator can limit
individual users on top of the existing global / per-agent / per-provider
budgets.

UserConfig gains an optional UserBudgetConfig with the same hourly /
daily / monthly window structure as BudgetConfig plus an alert_threshold
(0..=1) shared with the per-agent path. None means "no per-user cap" —
the user is still bounded by the other layers.

Refs #3054

* feat(runtime): UserLogin/RoleChange/PermissionDenied/BudgetExceeded audit variants

Append four RBAC-themed variants to AuditAction so the new admin-only
endpoints in librefang-api can correlate denials and quota breaches
with the user / channel that triggered them.

The hash chain stays intact because the new variants only add fresh
rows — pre-existing entries hash identically (their action string is
unchanged). A regression test pins the Display output of every new
variant so a casual rename surfaces as a test failure rather than a
silent invalidation of every persisted hash.

Refs #3054

* feat(memory): per-user spend rollup via usage_events schema v23

RBAC M5 needs per-user budget detail and ranking endpoints. Adding
user_id / channel columns to usage_events lets the metering pipeline
attribute every LLM call to the caller (resolved from SenderContext
via auth.identify) so the dashboard can roll spend up by user without
scanning the whole table.

- Schema v23 migration adds NULL-able user_id / channel columns and
  indexes both (idx_usage_user_time, idx_usage_channel_time). Pre-M5
  rows return NULL — they fall outside any per-user filter, which is
  the right default since they pre-date attribution.
- UsageRecord gains optional user_id / channel fields plus an
  anonymous() helper for kernel-internal call sites that don't carry
  a SenderContext (background_skill_review, ephemeral side-questions).
- New UsageStore queries: query_user_hourly / _daily / _monthly and a
  query_user_ranking that aggregates all three windows in a single
  CASE-when round-trip.
- Kernel cost-attribution wiring: execute_llm_agent and the streaming
  spawn now resolve UserId from sender_context (via auth.identify)
  and stamp every UsageRecord with it. When check_all_and_record
  returns QuotaExceeded the kernel now also records a BudgetExceeded
  audit entry so an operator can correlate denials with users.

Refs #3054

* feat(api): audit query/export and per-user budget endpoints

Lands the four endpoints requested by RBAC M5:

- GET /api/audit/query — filter the hash-chained audit log by user,
  action, agent, channel, and ISO-8601 time range. Newest-first, hard
  cap 5000 rows. Filtering is in-memory (the AuditLog::recent path
  already verifies the chain on read), so SQL injection surface is
  zero.
- GET /api/audit/export?format=json|csv — same filters, streamed via
  axum::body::Body::from_stream so the kernel never buffers a full
  result set. CSV emits a fixed header row with RFC-4180 cell
  escaping pinned by a regression test.
- GET /api/budget/users — per-user cost ranking (top N) joined with
  the user's configured limits when present so the dashboard can
  render % bars without a follow-up call.
- GET /api/budget/users/{user_id} — single-user detail with hourly /
  daily / monthly spend, alert threshold, and an alert_breach flag.
  Accepts both the canonical UUID and the raw configured name
  (re-derived via UserId::from_name) so operators can paste either.

All four endpoints are admin-only and gate in-handler against
UserRole::Admin — they stay AUTHENTICATED (not on the is_public
allowlist). A 403 also fires a PermissionDenied audit event so an
operator can spot probes via /api/audit/query.

Middleware glue:
- AuthState gains an optional audit_log handle; when a user-scoped
  role check returns FORBIDDEN we now record PermissionDenied with
  the caller's UserId so the chain shows who tripped which path.
- The dirty middleware/config glue from PR #3196 (threading
  authenticated UserId through to record_with_context) is folded in
  unchanged.

OpenAPI registers all four new operations.

Refs #3054

* feat(api): thread authenticated UserId into runtime audit context

The /api/shutdown, /api/config/reload, and /api/config/set handlers now
accept the AuthenticatedApiUser extension and pass the caller's UserId
+ a 'api' channel through to record_with_context. Combined with the
RBAC M5 audit chain, an operator can now see exactly which user
triggered each ConfigChange entry instead of an anonymous 'system'
record.

This is the dirty middleware glue originally scoped under M2 — picked
up unchanged so the M5 endpoints have a stable contract for caller
attribution.

Refs #3054

* fix(security): RBAC M5 — clippy, CSV formula guard, per-user budget gap noted

Address review of #3203:

1) UsageRecord::anonymous's 8 positional args trip clippy too_many_arguments under -D warnings, breaking the Quality CI lane. The shape mirrors the metering schema 1:1 and a builder would push call-site noise into ~20 internal kernel paths without gaining type safety, so suppress locally with explanation.

2) csv_escape did not neutralise CSV-formula injection (CWE-1236). A username or detail string starting with =, +, -, @, TAB, or CR round-trips through Excel/Google Sheets as a live formula. OWASP-recommended mitigation: prepend an apostrophe inside the quoted cell. New test test_csv_escape_neutralises_formula_injection pins the behaviour for all six sentinels and confirms inner-position sentinels are untouched.

3) UserConfig.budget is read-only display data in this slice — the metering pipeline (check_all_and_record) only enforces global / per-agent / per-provider caps, not per-user. Add an inline TODO at the kernel call site so the gap is greppable; full per-user enforcement (lookup the resolved UserConfig via attribution_user_id, call a new metering.check_user_budget arm) lands in a follow-up. PR body updated separately to reflect this.

* fix(api): regen kernel_config_schema golden after UserConfig.budget added

M5 added the UserConfig.budget field to KernelConfig but the golden JSON schema fixture was not regenerated, breaking kernel_config_schema_matches_golden_fixture across Test/Ubuntu and Test/macOS.

* fix(security): RBAC M5 review follow-ups — fail-closed admin gate, parsed-instant time filter

Addresses code review on PR #3203:

- audit/budget admin gates: drop the "anonymous = Owner" loopback trust.
  `require_admin` and `require_admin_for_user_budget` now reject when no
  `AuthenticatedApiUser` is present, so a co-resident process at
  `127.0.0.1` (or `LIBREFANG_ALLOW_NO_AUTH=1` deploy) can no longer
  exfiltrate the hash-chained audit log or per-user spend without
  configuring an admin api_key.
- /api/audit/{query,export}: parse `from`/`to` as RFC-3339 instants
  instead of doing lexicographic string compare. Prior behaviour
  silently dropped entries when the operator passed `Z` and the entries
  were stored with `+00:00` (the chrono default), because `Z` (0x5A)
  > `+` (0x2B). Malformed bounds now return 400 instead of being
  ignored. Adds `test_time_range_normalises_z_and_offset_suffix` and
  `test_parse_time_bounds_rejects_garbage`.
- /api/budget/users/{id}: add `enforced: false` to the response so the
  M6 dashboard can warn that `alert_breach` is informational until the
  M5-followup enforcement arm in `kernel/mod.rs::execute_llm_agent`
  lands. Comment links the flag back to that TODO.
- usage::query_user_ranking: bind LIMIT via `rusqlite::params!` instead
  of `format!`-ing the clamped u32 into the SQL string. Value is a
  clamped u32 so injection isn't a real risk, but keeping the
  convention uniform avoids future copy-paste landing on
  user-controlled input. `None` maps to `LIMIT -1` (SQLite "no limit").
- Drop `_USER_SPEND_FRESHNESS_SECS` (referenced nowhere, just a
  comment) and the `#[allow(clippy::type_complexity)]` on the
  `Option<Arc<AuditLog>>` field (well below the threshold).
- Honesty pass on the export-streaming comment: the body is still
  materialised in memory before chunked transmission. Real lazy
  streaming would need `async-stream`; left for a follow-up.

Verification:
- cargo check -p librefang-api -p librefang-memory --lib → clean
- cargo test -p librefang-api --lib routes::audit::tests → 8/8 passed
- cargo test -p librefang-memory --lib usage → 14/14 passed
- cargo clippy -p librefang-api -p librefang-memory --lib --tests
  -- -D warnings → clean

* fix(rbac-m5): clamp UserBudgetConfig.alert_threshold + UTC doc accuracy

Re-review of #3203 surfaced two MEDIUM issues both fixed here:

1. `UserBudgetConfig.alert_threshold` is documented "clamped to 0..=1"
   but the field is bare `f64` and TOML parsing accepts any value. With
   no clamp, `alert_threshold = 5.0` made `alert_breach` permanently
   false (the dashboard never warned) and `-1.0` made it permanently
   true. `KernelConfig::clamp_bounds` now reins both ends back to
   [0.0, 1.0] and resets NaN to the default 0.8. New regression test
   `test_clamp_bounds_user_alert_threshold` covers above-1, below-0,
   NaN, and in-range cases.

2. `query_user_daily` / `query_user_monthly` doc-comments said "calendar
   day, server-local" but SQLite's `datetime('now', 'start of day')`
   returns UTC. Operators in non-UTC zones see "today" sliced on UTC
   midnight, not local midnight. Updated the per-user rollup module
   header to spell out the UTC contract (matching the pre-existing
   `query_global_*` and `query_agent_*` rollups) and the `_daily` /
   `_monthly` doclines now say UTC.

Also corrected the PR description: `audit_export` is described as
"streamed via from_stream so the kernel never buffers". The handler
DOES materialise the full filtered Vec before chunking — wire transfer
is progressive but kernel-side memory is O(filtered_rows). Updated PR
body to reflect that, with a follow-up note that switching to
per-row pull from `AuditLog::recent` is the path to true streaming.

* test(rbac-m5): integration tests for admin-only audit/budget endpoints

Closes the integration-test gap called out in PR #3203 review:
the four new HTTP endpoints (`/api/audit/query`, `/api/audit/export`,
`/api/budget/users`, `/api/budget/users/{id}`) had unit tests for
`apply_filter` / `csv_escape` / `parse_time_bounds` in isolation but
no test that exercised the full request → middleware → handler →
`require_admin` pipeline. That gap matters because:

- The middleware role-gate (`user_role_allows_request`) lets every
  authenticated GET through regardless of role; the in-handler
  `require_admin` is the *only* thing stopping a Viewer from reading
  the hash-chained audit log. A future refactor that drops it would
  silently expose the chain.
- The `a612c09b` follow-up made the gate fail-closed for anonymous
  callers in `LIBREFANG_ALLOW_NO_AUTH=1` deployments — without an
  end-to-end test, that fix could be silently reverted.

Adds `start_test_server_with_rbac_users` helper that wires:
- `KernelConfig.users` AND `AuthState.user_api_keys` from a list of
  `(name, role, api_key)` tuples (mirrors `server.rs::build_router`).
- The audit + budget routers under `/api/`.
- `AuthState.audit_log = Some(kernel.audit().clone())` so denials
  hit the same hash chain as production.
- `allow_no_auth = true` so anonymous-rejection tests can synthesize
  Bearer-less requests without 401-ing at the middleware layer.

Tests:
- `test_audit_query_rejects_anonymous_403` — pins the fail-closed
  admin gate from `a612c09b`.
- `test_audit_query_rejects_viewer_admin_returns_200` — pins the
  in-handler `require_admin` boundary; Viewer 403, Admin 200 with
  documented JSON shape (`entries[]`, `count`, `limit`).
- `test_audit_export_csv_emits_documented_headers` — pins
  `Content-Type: text/csv` + `Content-Disposition: attachment;
  filename="audit.csv"` + the canonical CSV header row schema, so
  the dashboard download flow stays compatible.
- `test_user_budget_detail_includes_enforced_false` — pins the M6
  dashboard contract that `enforced` stays `false` until per-user
  budget enforcement (the M5-followup arm in
  `kernel/mod.rs::execute_llm_agent`) actually lands.

`cargo check -p librefang-api --tests` clean.

* fix(rbac-m3): close 2 review gaps — case-insensitive PII label, validate flag-without-namespace combos

PR #3205 (M3) shipped two paper cuts that this commit closes. Both
land on the M5 branch because M5 is already touching the same files
(`crates/librefang-types/src/config/validation.rs` for the
`UserBudgetConfig` clamp, `crates/librefang-memory/src/namespace_acl.rs`
unchanged but in scope) and a separate PR would conflict with no
benefit.

1. **`MemoryNamespaceGuard::has_pii_label` is now case-insensitive.**
   The metadata-stored label is the `Display` form of `TaintLabel::Pii`
   (literal `"Pii"`), but external writers commonly hand-stamp `"PII"`
   or `"pii"`. The previous exact-match comparison silently skipped
   those, leaving structured PII fragments to fall through to the
   regex backstop — fine for free-form text but a leak for custom
   field names that have no e-mail/phone/SSN tokens to grep. Both the
   array and scalar-string metadata shapes now lowercase-normalise the
   target *and* candidate before comparing.

2. **`KernelConfig::validate()` now warns on `pii_access` /
   `export_allowed` / `delete_allowed` set with empty
   `readable_namespaces`.** The runtime guard requires read access
   for every code path those flags gate (PII redaction only runs on
   readable items; export/delete check write access AND the flag).
   An admin who toggles a flag without declaring namespaces gets a
   silent no-op that looks like it works — exactly the
   misconfiguration `validate()` exists to surface. Warning names the
   user, lists the flags, and suggests the missing key.

Tests:
- `redact_via_metadata_label_matches_case_insensitively` covers
  `"PII"` / `"pii"` / `"Pii"` for both array and scalar-string
  metadata shapes.
- `test_validate_warns_on_memory_access_flags_without_readable_namespaces`
  pins the warning shape: warns on the typo'd user, suppresses on a
  correctly-configured user, names both the user and the flag.

`cargo test -p librefang-memory --lib namespace_acl`: 9/9.
`cargo test -p librefang-types --lib config`: 139/139 plus the new
validate test.

(A third proposed item — startup warning when memory routes are in
the auth allowlist while users have memory_access policies — turned
out to be moot: `/api/memory/*` is not in any allowlist, the only
fallback path is the loopback bypass which is deliberate and
documented in `guard_for_request`.)

* feat(security): RBAC M5 — wire per-user budget enforcement

Closes the M5-followup TODO that left UserConfig.budget as read-only
display data. Per-user spend caps now actually deny over-budget calls,
matching the global / per-agent / per-provider enforcement semantics.

Surface:
- AuthManager: UserIdentity gains an optional UserBudgetConfig (cloned
  from UserConfig.budget at populate time). New `budget_for(user_id)`
  getter mirrors `memory_acl_for` / `user_policy`.
- MeteringEngine: new `check_user_budget(user_id, &user_budget)`. Each
  window with `0.0` is treated as unlimited. On breach returns
  `QuotaExceeded` with the user id in the message so dashboard / log
  triage can grep by uid.
- kernel::execute_llm_agent (sync) and the streaming spawn now run
  the per-user check after `check_all_and_record` succeeds. Same
  post-call semantics as the existing global / agent / provider arms
  — a breach trips a BudgetExceeded audit (already attribution-tagged
  with the user / channel) so the next call from this user gets denied
  at the gate; the current response is returned unchanged because the
  tokens are already billed.
- /api/budget/users/{id} flips `enforced` from false to true; the
  earlier `enforced=false` was a UX guard while enforcement was
  deferred. Comment now explains the cap-to-denial chain.

Tests:
- 4 new tests in librefang-kernel-metering covering under-limit,
  zero-means-unlimited, hourly breach with uid in message, and
  user isolation (Bob's spend doesn't trigger Alice's gate).

Verification:
- cargo check -p librefang-kernel-metering -p librefang-kernel
  -p librefang-api --lib → clean
- cargo test -p librefang-kernel-metering --lib check_user_budget → all passing
- cargo clippy -p librefang-kernel-metering -p librefang-kernel
  -p librefang-api --lib --tests -- -D warnings → clean

* fix(rbac-m5): add missing budget field to UserConfig literal in M4 channel role test

Auto-merge missed adding `budget: None` to the `UserConfig` literal at
`auth.rs:1383` (the M4 `channel_role_explicit_user_config_wins` test).
Three other UserConfig literals in the same file (lines 852/868/883)
and the `user_with_policy` helper at 1047 already carried the field
from earlier merges; this one slipped through because it was added by
M4 after M5's field addition fanned out to its sibling literals.

CI failure: E0063 "missing field `budget` in initializer` on Test /
Ubuntu / Windows / macOS / Quality.

Caught by Test / Windows in run 24951702471.

* fix(rbac-m5): impl Default for UserConfig so test fixtures can ..Default::default()

CI Test/Ubuntu (run 24952194471) failed on
crates/librefang-api/tests/api_integration_test.rs:2565 — the new
RBAC integration test fixture `start_test_server_with_rbac_users`
uses `..Default::default()` to fill the optional RBAC fields, but
UserConfig had no Default impl.

The integration test file is a separate compilation unit from
`--lib --tests` runs targeting only the lib's unit tests, which is
why my prior local verification missed it. CI's full
`cargo test -p librefang-api` caught it.

Add a hand-written `impl Default for UserConfig` that mirrors the
per-field `#[serde(default)]` attributes — `name = ""`, `role =
default_role()` ("user"), all RBAC `Option`s = `None`, both
`HashMap`s empty. Now `serde::from_str("name = \"x\"")` and
`UserConfig { name: ..., ..Default::default() }` produce identical
values.

Verification:
- cargo test -p librefang-api --tests → all passing
- cargo clippy -p librefang-api -p librefang-types --all-targets
  -- -D warnings → clean

* fix(rbac-m5): add budget field to UserConfig literals after M6 merge

PR #3209 (RBAC M6 dashboard) landed in main and added
crates/librefang-api/src/routes/users.rs with three UserConfig
literals — create, update, CSV import — that predate the per-user
budget field this PR added to UserConfig. Compile breaks with E0063
"missing field `budget`" when the two land together.

- create_user / import_users new-row: budget=None (no per-user cap;
  global/per-agent/per-provider caps still apply). Dedicated
  /api/users/{name}/budget owns budget edits.
- update_user / import_users update-row: preserve from existing config
  alongside the M3 policy fields, same reasoning — the M6 dashboard
  doesn't surface budget yet, clobbering it on a name/role edit would
  silently wipe a user's spend cap.

Also adds budget=None to the seed in users_test.rs so the existing
M6 preservation test still compiles.

* test(rbac-m5): pin budget preservation across PUT and CSV reimport

Catches the latent bug in users.rs:568 where `..new_u.clone()` would
silently overwrite an existing user's `budget` with `None` because CSV
rows always carry no budget. Mirrors the existing M3 policy preservation
test (`users_update_and_import_preserve_rbac_m3_policy_fields`).

Seeds Carol with a non-zero UserBudgetConfig, then:
1. PUT /api/users/Carol — rename + role bump, no budget in body. Asserts
   the budget round-trips.
2. POST /api/users/import — same name, no budget in CSV row. Asserts the
   budget still round-trips through the import-update branch.

Both assertions name the failure mode in the message so a future
regression points at the exact line that broke.

* test(rbac-m5): unstale two assertions hidden by earlier compile errors

Both tests landed earlier in the M5 stack but were never actually
exercised by CI — the UserConfig.budget compile errors after the M6
merge masked them. Now that the literals compile, the tests run and
fail against the current behavior, which is correct in both cases.

1) test_audit_query_rejects_anonymous_403 → ..._anonymous (asserts 401)
   The setup asks for `allow_no_auth = true` to bypass middleware, but
   middleware.rs:501-526 only honors that flag when both `api_key` and
   `user_api_keys` are empty. The test configures both, so anonymous
   requests are 401'd at the middleware before reaching the in-handler
   `require_admin` 403 gate. Earlier rejection is the stricter outcome
   ("anonymous cannot read audit" still holds).

2) test_user_budget_detail_includes_enforced_false → ..._true
   Per-user budget enforcement landed in 4a00a64 ("RBAC M5 — wire
   per-user budget enforcement") — AuthManager::budget_for,
   MeteringEngine::check_user_budget, and the post-call arm in
   kernel::execute_llm_agent now actually deny over-budget calls. The
   handler was updated to report `enforced: true` accordingly; the
   stale assertion was missed.

No production changes — only the assertions move to track the
already-shipped behavior.

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
houko added a commit that referenced this pull request Apr 26, 2026
…3229)

* feat(security): RBAC M3 — per-user policy GET/PUT + dashboard editor

Wires the M6 dashboard's stub `UserPolicyPage` to a live `PUT /api/users/{name}/policy`
endpoint that upserts the per-user `tool_policy`, `tool_categories`,
`memory_access`, and `channel_tool_rules` slice landed by M3 (#3205). M5
already exposes per-user budget edits the same way; this PR mirrors that
shape for the policy slice so the dashboard can stop linking to "after M3
ships".

Endpoint semantics:
  - Each top-level field independently nullable: absent = preserve,
    `null` = clear, object = replace. Implemented by deserializing the
    body as a raw `serde_json::Map` so we can distinguish absent from
    null (`Option<T>` would collapse them).
  - Validates non-empty / non-duplicate string entries, rejects
    unknown top-level keys, and enforces a new invariant: every
    `writable_namespaces` entry must appear in `readable_namespaces`
    (no upstream check exists today; the resolver assumes well-formed
    config).
  - Owner-only via the existing `is_owner_only_write` allowlist —
    per-user policy edits change someone's authorization surface, so
    they ride the same gate as create/delete.

Dashboard:
  - `UserPolicyPage` becomes a real form with sections for tool
    allow/deny, categories, memory access (PII / export / delete
    toggles), and per-channel rules. Client-side validation mirrors
    the server's so typos surface inline.
  - `useUpdateUserPolicy` now invalidates `permissionPolicyKeys.detail`
    plus `userKeys.detail` and `userKeys.lists` (policy fields are
    part of the underlying `UserConfig`).
  - `getUserPolicy` / `updateUserPolicy` rewritten to match the wire
    shape; new `PermissionPolicyUpdate` distinguishes preserve from
    clear via `undefined` vs `null`.

Tests:
  - `users_policy_get_round_trip` — non-default seed survives GET.
  - `users_policy_put_replaces_only_specified_fields` — clear one
    slot, preserve another that's absent from the body.
  - `users_policy_put_validates_writable_subset_of_readable` — the
    new subset invariant.
  - `users_policy_put_validates_no_empty_tool_strings`.
  - `users_policy_put_owner_only` (full middleware stack) — Admin gets
    403, Owner gets 200; pins the owner-only gate.

* style(api/users): apply cargo fmt to policy update handler and test

* ci: re-trigger workflows

* style(api/users): drop trailing blank line in policy test (cargo fmt)
houko added a commit that referenced this pull request Apr 26, 2026
…us callers (#3239)

* fix(security): RBAC M3 follow-up — memory ACL fail-closed for anonymous callers

Closes the fail-open in routes/memory.rs::guard_for_request — when the
auth middleware doesn't attach an AuthenticatedApiUser (loopback dev or
LIBREFANG_ALLOW_NO_AUTH=1), the previous fallback granted full
read/write across every namespace plus pii_access, export_allowed, and
delete_allowed. Any process at 127.0.0.1 or any deployment with the
no-auth env opt-in could exfiltrate every memory fragment including
PII and bulk-delete across agents.

Other admin-gated RBAC endpoints (audit query, per-user budget, authz
effective) already reject anonymous callers; memory ACL was the
outlier.

Replaced the fallback with a Viewer-equivalent ACL inlined as
anonymous_fallback_acl(): read access scoped to the proactive
namespace only; writes, exports, deletes, and PII access all denied.
Loopback dashboard reads continue to work for the proactive surface;
sensitive operations now fail closed at the namespace_acl layer (return
AuthDenied → 403).

Two new unit tests in #[cfg(test)] mod tests pin the contract:
- anonymous_fallback_denies_pii_export_and_delete asserts each flag
- anonymous_fallback_guard_gates_match_acl_intent walks the guard
  through every namespace gate to make sure the four leak channels
  (read kv:*/shared:*/kg, write proactive, export proactive, delete
  proactive) all return Deny

Refs PR #3205 review item #6.

* style: cargo fmt for memory.rs ACL helper
houko added a commit that referenced this pull request Apr 26, 2026
…tive deny + memory audit emit (#3205) (#3249)

* fix(security): RBAC M3 follow-up — namespace traversal + case-insensitive deny + memory audit emit

Three reviewer-flagged bugs in PR #3205 (RBAC M3):

#7 — Namespace path traversal (REAL):
`UserMemoryAccess::can_read/can_write` deferred to the generic
`capability::glob_matches`, where `*` greedily matches any text including
path separators. So a pattern of `kv:user_*` matched `kv:user_../admin`
or `kv:user_evil/etc/passwd` — letting a memory tool that builds the
namespace from user-controlled input cross into another user's bucket.

Fix: introduce `namespace_glob_matches` with two stricter rules:
  1. Reject any candidate containing a `..` segment (delimited by `/`,
     `:`, or whitespace) outright.
  2. `*` may not span `/` or `:` separators inside a longer pattern
     (e.g. `kv:user_*` only matches a single component after the prefix).

`*` standalone still matches any non-traversing namespace so the
owner / admin "see everything" UX is preserved. Kept separate from the
generic `glob_matches` (which is used for tool-name and capability
matching where collapsing across separators is desirable).

#8a — Case-sensitive deny lists (PARTIAL → fixed):
`UserToolPolicy`, `ChannelToolPolicy`, and `UserToolCategories` matched
tool names case-sensitively, so a deny rule for `shell_exec` would not
catch a hallucinated `SHELL_EXEC` invocation. Built-in tools dispatch
by exact case so the call would have failed downstream anyway, but
MCP / skill tool providers may accept their own case variants and the
deny list is supposed to be the authoritative gate. ASCII-lowercased
both pattern and tool name at every per-user check site.

Limited to per-user policy layers — the rate-limit buckets and
capability matchers continue using exact-case comparison since tool
identities there are content-addressed.

#8b — Memory ACL denial silently dropped from audit chain (REAL):
`routes/memory.rs::auth_denied` returned a 403 without recording a
`PermissionDenied` row, while the parallel `routes/audit.rs`,
`routes/budget.rs`, `routes/authz.rs`, and the global auth middleware
all emit one. A privilege probe against `/api/memory*` was therefore
invisible to an admin reading `/api/audit`.

Fix: extended `auth_denied` to take `&AppState` + `&Extensions` and
emit a `PermissionDenied` audit row before returning 403. Anonymous
callers are recorded with `user_id = None`; authenticated-but-denied
callers carry their attributed `user_id` plus user / role in the
detail.

Tests added:
- `memory_access_namespace_blocks_path_traversal` — `kv:user_../admin`,
  `kv:user_alice/../bob`, and separator-crossing globs all denied.
- `memory_access_star_pattern_still_rejects_traversal` — even `["*"]`
  rejects `..` candidates.
- `user_deny_is_case_insensitive`,
  `channel_deny_is_case_insensitive`,
  `categories_deny_is_case_insensitive` — pin lowercase normalisation.
- `auth_denied_emits_audit_row_for_anonymous` and
  `auth_denied_emits_audit_row_for_authenticated_user` — pin the
  audit-emit contract for both anonymous and attributed denials.

* fix(test): add user_api_keys field to memory.rs audit_test_app_state

#3233 added Arc<RwLock<Vec<ApiUserAuth>>> user_api_keys to AppState.
The new audit_test_app_state helper landed before that merged so its
struct-literal init was missing the field. CI flagged E0063 on macOS
and Windows after rebase. Initialize as empty Vec — these tests don't
exercise auth at all, they call auth_denied directly with synthesized
extensions.
@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) area/runtime Agent loop, LLM drivers, WASM sandbox size/XL 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant