Skip to content

Commit e0bdd74

Browse files
serrrfiratclaudeilblackdragon
authored
feat(admin): admin tool policy to disable tools for users (#2154)
* feat(admin): admin tool policy to disable tools for users (#2078) Adds the ability for admins to disable specific tools (e.g. build_software, tool_install, skill_install) for all non-admin users or specific users in multi-tenant deployments. - AdminToolPolicy stored in settings table under __admin__ scope - GET/PUT /api/admin/tool-policy endpoints (admin-only, multi-tenant gated) - Enforcement in dispatcher before_llm_call strips disabled tools from LLM context - Admin users are exempt from the policy Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: enforce admin tool policy across all execution paths - Extract inline filtering into shared `filter_admin_disabled_tools()` helper - Apply in JobDelegate and ContainerDelegate (not just ChatDelegate) - Change from fail-open to fail-closed: DB errors return empty tool list - Log warnings on deserialization failures instead of silent fallback - Add `multi_tenant` field to WorkerDeps for job-level enforcement - Add `db()` accessor to SystemScope for system-level DB access Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: tighten admin tool policy review follow-ups * fix(admin-policy): enforce ordering and add dispatcher e2e regression * fix(admin-policy): address review feedback — encapsulate DB access, cache policy, strengthen validation - Remove raw `SystemScope::db()` accessor; add purpose-built `get_admin_tool_policy()`, `set_admin_tool_policy()`, `get_user_role()` methods to preserve tenant isolation boundary - Canonicalize admin role detection: add `UserRole::is_admin()` helper, replace string comparisons in engine.rs and role enum comparisons across dispatcher/job delegates with the single canonical path - Cache admin tool policy per agentic loop via `AdminToolPolicyCache` (tokio::sync::OnceCell) to avoid DB reads on every LLM iteration - Switch `disabled_tools` from Vec<String> to HashSet<String> for O(1) lookups - Extract shared `validate_admin_tool_policy()` with tool name format checks, user key validation, and 32KB max payload size; deduplicate from HTTP handler - Add `parse_admin_tool_policy()` helper with tracing::warn on deserialization failure (was silently falling back to default) - Document PUT endpoint's last-write-wins replacement semantics - Add regression tests: path-like tool names, invalid user keys, oversized policy, and E2E test verifying disabled tools don't reach the LLM Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(admin-policy): entry count caps, fail-closed GET, dispatch-exempt annotation - Add entry count limits: 1000 global disabled tools, 1000 user keys, 1000 per-user entries — prevents multi-MB policy payloads - GET handler now returns 500 on corrupt stored policy instead of silently falling back to empty default (fail-closed, consistent with enforcement) - Add dispatch-exempt annotation explaining why these admin handlers access state.store directly (consistent with users/secrets/tokens handlers) - Remove redundant debug_assert_ne in users_create_handler (runtime guard already covers both debug and release builds) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(admin-policy): merge staging, add inline dispatch-exempt annotations Merge staging to pick up ToolDispatcher (#2049) and the "everything goes through tools" pre-commit check. Add inline // dispatch-exempt: annotations on the state.store access lines so the pre-commit check passes. These handlers are admin-only infrastructure operating on a cross-tenant policy scope — consistent with other admin handlers that haven't been migrated to the dispatcher yet. Also fix post-merge compilation: add auth_manager and tool_dispatcher fields to test struct initializers. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix(admin-policy): satisfy per-line dispatch-exempt check on all state.* accesses The pre-commit hook (scripts/pre-commit-safety.sh) checks each added line that touches state.{store,workspace_pool,...} for a trailing // dispatch-exempt: comment on the same physical line. The previous annotations were either: - on the workspace_pool lines: missing entirely - on the store.as_ref().ok_or(( lines: rustfmt-broken because the trailing comment was placed inside the tuple, where the per-line check no longer matches it Lift each state.workspace_pool / state.store access into a dedicated local binding that carries the trailing dispatch-exempt comment, so the annotation survives cargo fmt and the per-line hook regex matches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Co-authored-by: Illia Polosukhin <ilblackdragon@gmail.com>
1 parent 9399fcc commit e0bdd74

13 files changed

Lines changed: 1346 additions & 4 deletions

File tree

src/agent/dispatcher.rs

Lines changed: 220 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ impl Agent {
238238
user_tz,
239239
turn_usage: std::sync::Mutex::new(TurnUsageSummary::default()),
240240
cached_tool_permissions: std::sync::Mutex::new(None),
241+
cached_admin_tool_policy: tokio::sync::OnceCell::new(),
241242
};
242243

243244
// If /skill-name mentions were expanded, rewrite the last user message
@@ -350,6 +351,7 @@ struct ChatDelegate<'a> {
350351
turn_usage: std::sync::Mutex<TurnUsageSummary>,
351352
cached_tool_permissions:
352353
std::sync::Mutex<Option<std::collections::HashMap<String, PermissionState>>>,
354+
cached_admin_tool_policy: crate::tools::permissions::AdminToolPolicyCache,
353355
}
354356

355357
impl ChatDelegate<'_> {
@@ -422,6 +424,22 @@ impl<'a> LoopDelegate for ChatDelegate<'a> {
422424
tool_defs
423425
};
424426

427+
// Apply admin tool policy first so admin-disabled tools are removed
428+
// before per-user permission filtering and session auto-approval.
429+
let is_admin = self.tenant.identity().role.is_admin();
430+
let admin_policy = crate::tools::permissions::load_cached_admin_tool_policy(
431+
self.agent.store(),
432+
&self.cached_admin_tool_policy,
433+
)
434+
.await;
435+
let tool_defs = crate::tools::permissions::filter_admin_disabled_tools(
436+
tool_defs,
437+
self.agent.config.multi_tenant,
438+
is_admin,
439+
self.tenant.user_id(),
440+
admin_policy,
441+
);
442+
425443
// Apply per-user tool permission filtering.
426444
//
427445
// Load tool_permissions from the per-user DB settings store (same
@@ -3110,6 +3128,56 @@ mod tests {
31103128
}
31113129
}
31123130

3131+
#[derive(Default)]
3132+
struct RecordingToolsProvider {
3133+
seen_tools: std::sync::Mutex<Vec<Vec<String>>>,
3134+
}
3135+
3136+
#[async_trait]
3137+
impl LlmProvider for RecordingToolsProvider {
3138+
fn model_name(&self) -> &str {
3139+
"recording-tools"
3140+
}
3141+
3142+
fn cost_per_token(&self) -> (Decimal, Decimal) {
3143+
(Decimal::ZERO, Decimal::ZERO)
3144+
}
3145+
3146+
async fn complete(
3147+
&self,
3148+
_request: CompletionRequest,
3149+
) -> Result<CompletionResponse, crate::error::LlmError> {
3150+
Ok(CompletionResponse {
3151+
content: "ok".to_string(),
3152+
input_tokens: 0,
3153+
output_tokens: 1,
3154+
finish_reason: FinishReason::Stop,
3155+
cache_read_input_tokens: 0,
3156+
cache_creation_input_tokens: 0,
3157+
})
3158+
}
3159+
3160+
async fn complete_with_tools(
3161+
&self,
3162+
request: ToolCompletionRequest,
3163+
) -> Result<ToolCompletionResponse, crate::error::LlmError> {
3164+
let names: Vec<String> = request.tools.iter().map(|t| t.name.clone()).collect();
3165+
self.seen_tools
3166+
.lock()
3167+
.expect("recording tools mutex poisoned")
3168+
.push(names);
3169+
Ok(ToolCompletionResponse {
3170+
content: Some("ok".to_string()),
3171+
tool_calls: Vec::new(),
3172+
input_tokens: 0,
3173+
output_tokens: 1,
3174+
finish_reason: FinishReason::Stop,
3175+
cache_read_input_tokens: 0,
3176+
cache_creation_input_tokens: 0,
3177+
})
3178+
}
3179+
}
3180+
31133181
/// Helper to build a test Agent with a custom LLM provider and
31143182
/// `max_tool_iterations` override.
31153183
fn make_test_agent_with_llm(llm: Arc<dyn LlmProvider>, max_tool_iterations: usize) -> Agent {
@@ -3224,6 +3292,158 @@ mod tests {
32243292
);
32253293
}
32263294

3295+
#[cfg(feature = "libsql")]
3296+
#[tokio::test]
3297+
async fn test_admin_policy_filter_happens_before_auto_approval_and_llm_call() {
3298+
use crate::agent::session::Session;
3299+
use crate::channels::IncomingMessage;
3300+
use crate::llm::ChatMessage;
3301+
use crate::tools::builtin::{EchoTool, TimeTool};
3302+
use crate::tools::permissions::{ADMIN_SETTINGS_USER_ID, ADMIN_TOOL_POLICY_KEY};
3303+
use tokio::sync::Mutex;
3304+
3305+
let (db, _tmp_dir) = crate::testing::test_db().await;
3306+
db.set_setting(
3307+
"member-user",
3308+
"tool_permissions",
3309+
&serde_json::json!({
3310+
"echo": "always_allow",
3311+
"time": "always_allow"
3312+
}),
3313+
)
3314+
.await
3315+
.expect("failed to seed member tool permissions");
3316+
db.set_setting(
3317+
ADMIN_SETTINGS_USER_ID,
3318+
ADMIN_TOOL_POLICY_KEY,
3319+
&serde_json::json!({
3320+
"disabled_tools": ["echo"]
3321+
}),
3322+
)
3323+
.await
3324+
.expect("failed to seed admin tool policy");
3325+
3326+
let llm = Arc::new(RecordingToolsProvider::default());
3327+
let llm_for_assert = Arc::clone(&llm);
3328+
let tools = Arc::new(ToolRegistry::new());
3329+
tools.register_sync(Arc::new(EchoTool));
3330+
tools.register_sync(Arc::new(TimeTool));
3331+
3332+
let deps = AgentDeps {
3333+
owner_id: "default".to_string(),
3334+
store: Some(db),
3335+
llm: llm as Arc<dyn LlmProvider>,
3336+
cheap_llm: None,
3337+
safety: Arc::new(SafetyLayer::new(&SafetyConfig {
3338+
max_output_length: 100_000,
3339+
injection_check_enabled: false,
3340+
})),
3341+
tools,
3342+
workspace: None,
3343+
extension_manager: None,
3344+
skill_registry: None,
3345+
skill_catalog: None,
3346+
skills_config: SkillsConfig::default(),
3347+
hooks: Arc::new(HookRegistry::new()),
3348+
cost_guard: Arc::new(CostGuard::new(CostGuardConfig::default())),
3349+
sse_tx: None,
3350+
http_interceptor: None,
3351+
transcription: None,
3352+
document_extraction: None,
3353+
auth_manager: None,
3354+
sandbox_readiness: crate::agent::routine_engine::SandboxReadiness::DisabledByConfig,
3355+
builder: None,
3356+
llm_backend: "nearai".to_string(),
3357+
tenant_rates: Arc::new(crate::tenant::TenantRateRegistry::new(4, 3)),
3358+
};
3359+
3360+
let agent = Agent::new(
3361+
AgentConfig {
3362+
name: "test-agent".to_string(),
3363+
max_parallel_jobs: 1,
3364+
job_timeout: Duration::from_secs(60),
3365+
stuck_threshold: Duration::from_secs(60),
3366+
repair_check_interval: Duration::from_secs(30),
3367+
max_repair_attempts: 1,
3368+
use_planning: false,
3369+
session_idle_timeout: Duration::from_secs(300),
3370+
allow_local_tools: false,
3371+
max_cost_per_day_cents: None,
3372+
max_actions_per_hour: None,
3373+
max_cost_per_user_per_day_cents: None,
3374+
max_tool_iterations: 5,
3375+
auto_approve_tools: true,
3376+
default_timezone: "UTC".to_string(),
3377+
max_jobs_per_user: None,
3378+
max_tokens_per_job: 0,
3379+
multi_tenant: true,
3380+
max_llm_concurrent_per_user: None,
3381+
max_jobs_concurrent_per_user: None,
3382+
engine_v2: false,
3383+
},
3384+
deps,
3385+
Arc::new(ChannelManager::new()),
3386+
None,
3387+
None,
3388+
None,
3389+
Some(Arc::new(ContextManager::new(1))),
3390+
None,
3391+
);
3392+
3393+
let session = Arc::new(Mutex::new(Session::new("member-user")));
3394+
let thread_id = {
3395+
let mut sess = session.lock().await;
3396+
sess.create_thread(Some("admin-policy")).id
3397+
};
3398+
let tenant = agent.tenant_ctx("member-user").await;
3399+
let message = IncomingMessage::new("test", "member-user", "hello");
3400+
let initial_messages = vec![ChatMessage::user("hello")];
3401+
3402+
let result = agent
3403+
.run_agentic_loop(
3404+
&message,
3405+
tenant,
3406+
Arc::clone(&session),
3407+
thread_id,
3408+
initial_messages,
3409+
)
3410+
.await;
3411+
assert!(result.is_ok(), "dispatcher run failed");
3412+
3413+
// admin-disabled tools must not remain auto-approved in session
3414+
let sess = session.lock().await;
3415+
assert!(
3416+
!sess.is_tool_auto_approved("echo"),
3417+
"echo is admin-disabled and must not be auto-approved"
3418+
);
3419+
assert!(
3420+
sess.is_tool_auto_approved("time"),
3421+
"time should remain auto-approved"
3422+
);
3423+
drop(sess);
3424+
3425+
// LLM should never see admin-disabled tools in available_tools.
3426+
let calls = llm_for_assert
3427+
.seen_tools
3428+
.lock()
3429+
.expect("recording tools mutex poisoned")
3430+
.clone();
3431+
assert!(
3432+
!calls.is_empty(),
3433+
"LLM should have been called at least once"
3434+
);
3435+
assert!(
3436+
!calls[0].iter().any(|name| name == "echo"),
3437+
"admin-disabled tool leaked into LLM tool list: {:?}",
3438+
calls[0]
3439+
);
3440+
assert!(
3441+
calls[0].iter().any(|name| name == "time"),
3442+
"expected non-disabled tool to remain available: {:?}",
3443+
calls[0]
3444+
);
3445+
}
3446+
32273447
/// Verify that the max_iterations guard terminates the loop even when the
32283448
/// LLM always returns tool calls and those calls succeed.
32293449
#[tokio::test]

src/agent/scheduler.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,6 +313,7 @@ impl Scheduler {
313313
sse_tx: self.sse_tx.clone(),
314314
approval_context,
315315
http_interceptor: self.http_interceptor.clone(),
316+
multi_tenant: self.config.multi_tenant,
316317
};
317318
let worker = Worker::new(job_id, deps);
318319

src/channels/web/handlers/engine.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ pub async fn engine_mission_pause_handler(
190190
AuthenticatedUser(user): AuthenticatedUser,
191191
Path(id): Path<String>,
192192
) -> Result<Json<EngineActionResponse>, (StatusCode, String)> {
193-
let is_admin = user.role == "admin";
193+
let is_admin = crate::ownership::UserRole::from_db_role(&user.role).is_admin();
194194
crate::bridge::pause_engine_mission(&id, &user.user_id, is_admin)
195195
.await
196196
.map_err(|e| {
@@ -214,7 +214,7 @@ pub async fn engine_mission_resume_handler(
214214
AuthenticatedUser(user): AuthenticatedUser,
215215
Path(id): Path<String>,
216216
) -> Result<Json<EngineActionResponse>, (StatusCode, String)> {
217-
let is_admin = user.role == "admin";
217+
let is_admin = crate::ownership::UserRole::from_db_role(&user.role).is_admin();
218218
crate::bridge::resume_engine_mission(&id, &user.user_id, is_admin)
219219
.await
220220
.map_err(|e| {

src/channels/web/handlers/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ pub mod secrets;
1212
pub mod skills;
1313
pub mod system_prompt;
1414
pub mod tokens;
15+
pub mod tool_policy;
1516
pub mod users;
1617

1718
// Modules not yet wired into server.rs router -- suppress dead_code until
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
//! Admin tool policy handlers.
2+
//!
3+
//! Allows an admin to define which tools are disabled for all non-admin users
4+
//! or for specific users. The policy is stored in the settings table under the
5+
//! well-known `__admin__` scope.
6+
//!
7+
//! dispatch-exempt: These endpoints access `state.store` directly (not through
8+
//! the agentic tool pipeline) because they are admin-only infrastructure
9+
//! operations gated behind `AdminUser` auth, consistent with the other admin
10+
//! handlers in this module (users, secrets, tokens).
11+
12+
use std::sync::Arc;
13+
14+
use axum::{Json, extract::State, http::StatusCode};
15+
16+
use crate::channels::web::auth::AdminUser;
17+
use crate::channels::web::server::GatewayState;
18+
use crate::tools::permissions::{
19+
ADMIN_SETTINGS_USER_ID, ADMIN_TOOL_POLICY_KEY, AdminToolPolicy, parse_admin_tool_policy,
20+
validate_admin_tool_policy,
21+
};
22+
23+
/// GET /api/admin/tool-policy — retrieve the current admin tool policy.
24+
///
25+
/// Only available in multi-tenant mode (returns 404 in single-user deployments).
26+
pub async fn tool_policy_get_handler(
27+
State(state): State<Arc<GatewayState>>,
28+
AdminUser(_admin): AdminUser,
29+
) -> Result<Json<AdminToolPolicy>, (StatusCode, String)> {
30+
let pool = state.workspace_pool.as_ref(); // dispatch-exempt: gateway-mode probe, not a state mutation
31+
if pool.is_none() {
32+
return Err((
33+
StatusCode::NOT_FOUND,
34+
"Admin tool policy is only available in multi-tenant mode".to_string(),
35+
));
36+
}
37+
38+
let store = state.store.as_ref(); // dispatch-exempt: admin-only read of cross-tenant policy scope
39+
let store = store.ok_or((
40+
StatusCode::SERVICE_UNAVAILABLE,
41+
"Database not available".to_string(),
42+
))?;
43+
44+
let policy = match store
45+
.get_setting(ADMIN_SETTINGS_USER_ID, ADMIN_TOOL_POLICY_KEY)
46+
.await
47+
{
48+
Ok(Some(value)) => parse_admin_tool_policy(value, "http_get").map_err(|e| {
49+
(
50+
StatusCode::INTERNAL_SERVER_ERROR,
51+
format!("Stored admin tool policy is corrupt: {e}"),
52+
)
53+
})?,
54+
Ok(None) => AdminToolPolicy::default(),
55+
Err(e) => {
56+
return Err((StatusCode::INTERNAL_SERVER_ERROR, e.to_string()));
57+
}
58+
};
59+
60+
Ok(Json(policy))
61+
}
62+
63+
/// PUT /api/admin/tool-policy — replace the admin tool policy.
64+
///
65+
/// Body must be a JSON `AdminToolPolicy`. Tool names and user IDs are
66+
/// validated for basic sanity (non-empty, reasonable length).
67+
///
68+
/// This endpoint is a full replacement with last-write-wins semantics.
69+
/// Each PUT overwrites the previously stored policy; there is no merge/patch.
70+
///
71+
/// Only available in multi-tenant mode (returns 404 in single-user deployments).
72+
pub async fn tool_policy_put_handler(
73+
State(state): State<Arc<GatewayState>>,
74+
AdminUser(_admin): AdminUser,
75+
Json(policy): Json<AdminToolPolicy>,
76+
) -> Result<Json<AdminToolPolicy>, (StatusCode, String)> {
77+
let pool = state.workspace_pool.as_ref(); // dispatch-exempt: gateway-mode probe, not a state mutation
78+
if pool.is_none() {
79+
return Err((
80+
StatusCode::NOT_FOUND,
81+
"Admin tool policy is only available in multi-tenant mode".to_string(),
82+
));
83+
}
84+
85+
validate_admin_tool_policy(&policy).map_err(|error| (StatusCode::BAD_REQUEST, error))?;
86+
87+
let store = state.store.as_ref(); // dispatch-exempt: admin-only write to cross-tenant policy scope
88+
let store = store.ok_or((
89+
StatusCode::SERVICE_UNAVAILABLE,
90+
"Database not available".to_string(),
91+
))?;
92+
93+
let value = serde_json::to_value(&policy).map_err(|e| {
94+
(
95+
StatusCode::INTERNAL_SERVER_ERROR,
96+
format!("Failed to serialize policy: {e}"),
97+
)
98+
})?;
99+
100+
store
101+
.set_setting(ADMIN_SETTINGS_USER_ID, ADMIN_TOOL_POLICY_KEY, &value)
102+
.await
103+
.map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?;
104+
105+
Ok(Json(policy))
106+
}

0 commit comments

Comments
 (0)