mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-09-03 01:59:23 +00:00
feat: complete multi-tenant isolation — phases 2–4 (#1614)
* feat: complete multi-tenant isolation — per-user budgets, model selection, heartbeat cycling Finishes the remaining isolation work from phases 2–4 of #59: Phase 2 (DB scoping): Fix /status and /list commands to use _for_user DB variants instead of global queries that leaked cross-user job data. Phase 3 (Runtime isolation): Per-user workspace in routine engine's spawn_fire so lightweight routines run in the correct user context. Per-user daily cost tracking in CostGuard with configurable budget via MAX_COST_PER_USER_PER_DAY_CENTS. Multi-user heartbeat that cycles through all users with routines, auto-detected from GATEWAY_USER_TOKENS. Phase 4 (Provider/tools): Per-user model selection via preferred_model setting — looked up from SettingsStore on first iteration, threaded through ReasoningContext.model_override to CompletionRequest. Works with providers that support per-request model overrides (NearAI). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: use selected_model setting key to match /model command persistence The dispatcher was reading "preferred_model" but the /model command (merged from staging) persists to "selected_model". Since set_setting is already per-user scoped, using the same key makes /model work as the per-user model override in multi-tenant mode. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: heartbeat hygiene, /model multi-tenant guard, RigAdapter model override Three follow-up fixes for multi-tenant isolation: 1. Multi-user heartbeat now runs memory hygiene per user before each heartbeat check, matching single-user heartbeat behavior. 2. /model command in multi-tenant mode only persists to per-user settings (selected_model) without calling set_model() on the shared LlmProvider. The per-request model_override in the dispatcher reads from the same setting. Added multi_tenant flag to AgentConfig (auto-detected from GATEWAY_USER_TOKENS). 3. RigAdapter now supports per-request model overrides by injecting the model name into rig-core's additional_params. OpenAI/Anthropic/Ollama API servers use last-key-wins for duplicate JSON keys, so the override takes effect via serde's flatten serialization order. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: address PR review — cost model attribution, heartbeat concurrency, pruning Fixes from review comments on #1614: - Cost tracking now uses the override model name (not active_model_name) when a per-user model override is active, for accurate attribution. - Multi-user heartbeat runs per-user checks concurrently via JoinSet instead of sequentially, preventing one slow user from blocking others. - Per-user failure counts tracked independently; users exceeding max_failures are skipped (matching single-user semantics). - per_user_daily_cost HashMap pruned on day rollover to prevent unbounded growth in long-lived deployments. - Doc comment fixed: says "routines" not "active routines". Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: /status ownership, model persistence scoping, heartbeat robustness Addresses second round of PR review on #1614: - /status <job_id> DB path now validates job.user_id == requesting user before returning data (was missing ownership check, security fix). - persist_selected_model takes user_id param instead of owner_id, and skips .env/TOML writes in multi-tenant mode (these are shared global files). handle_system_command now receives user_id from caller. - JoinSet collection handles Err(JoinError) explicitly instead of silently dropping panicked tasks. - Notification forwarder extracts owner_id from response metadata in multi-tenant mode for per-user routing instead of broadcasting to the agent owner. Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * fix: cost pricing, fire_manual workspace, heartbeat concurrency cap Round 3 review fixes: - Cost tracking passes None for cost_per_token when model override is active, letting CostGuard look up pricing by model name instead of using the default provider's rates (serrrfirat). - fire_manual() now uses per-user workspace, matching spawn_fire() pattern (serrrfirat). - Removed MULTI_TENANT env var — multi-tenant mode is auto-detected solely from GATEWAY_USER_TOKENS presence (serrrfirat + Copilot). - Multi-user heartbeat capped at 8 concurrent tasks to avoid flooding the LLM provider (serrrfirat + Copilot). - Fixed inject_model_override doc comment accuracy (Copilot). - Added comment explaining multi-tenant notification routing priority (Copilot). Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: user-scoped webhook endpoint for multi-tenant isolation Adds POST /api/webhooks/u/{user_id}/{path} — a user-scoped webhook endpoint that filters the routine lookup by user_id, preventing cross-user webhook triggering when paths collide. The existing /api/webhooks/{path} endpoint remains unchanged for backward compatibility in single-user deployments. Changes: - get_webhook_routine_by_path gains user_id: Option<&str> param - Both postgres and libsql implementations add AND user_id = ? filter when user_id is provided - New webhook_trigger_user_scoped_handler extracts (user_id, path) from URL and passes to shared fire_webhook_inner logic - Route registered on public router (webhooks are called by external services that can't send bearer tokens) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * feat: add TenantCtx for compile-time tenant isolation Implements zmanian's architectural proposal from #1614 review: two-tier scoped database access (TenantScope/AdminScope) so handler code cannot accidentally bypass tenant scoping. TenantScope (default): wraps user_id + Arc<dyn Database>, auto-binds user_id on every operation. ID-based lookups return None for cross- tenant resources. No escape hatch — forgetting to scope is a compile error. AdminScope (explicit opt-in): cross-tenant access for system-level components (heartbeat, routine engine, self-repair, scheduler, worker). TenantCtx bundles TenantScope + workspace + cost guard + per-user rate limiting. Constructed once per request in handle_message, threaded through all command handlers and ChatDelegate. Key changes: - New src/tenant.rs (~920 lines): TenantScope, AdminScope, TenantCtx, TenantRateState, TenantRateRegistry - All command handlers: user_id: &str → ctx: &TenantCtx - ChatDelegate: cost check/record/settings via self.tenant - System components: store field changed to AdminScope - Config: TENANT_MAX_LLM_CONCURRENT, TENANT_MAX_JOBS_CONCURRENT env vars - Fixes bug: /status <job_id> cross-tenant leak (now auto-filtered) Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
Illia Polosukhin
co-authored by
Claude Opus 4.6
Claude Opus 4.6
parent
86d1143064
commit
4c043bf057
+136
-37
@@ -13,7 +13,7 @@ use futures::StreamExt;
|
||||
use uuid::Uuid;
|
||||
|
||||
use crate::agent::context_monitor::ContextMonitor;
|
||||
use crate::agent::heartbeat::spawn_heartbeat;
|
||||
use crate::agent::heartbeat::{spawn_heartbeat, spawn_multi_user_heartbeat};
|
||||
use crate::agent::routine_engine::{RoutineEngine, spawn_cron_ticker};
|
||||
use crate::agent::self_repair::{DefaultSelfRepair, RepairResult, SelfRepair};
|
||||
use crate::agent::session::ThreadState;
|
||||
@@ -182,6 +182,8 @@ pub struct AgentDeps {
|
||||
/// Resolved LLM backend identifier (e.g., "nearai", "openai", "groq").
|
||||
/// Used by `/model` persistence to determine which env var to update.
|
||||
pub llm_backend: String,
|
||||
/// Per-tenant rate limiting registry (lazily creates rate state per user).
|
||||
pub tenant_rates: Arc<crate::tenant::TenantRateRegistry>,
|
||||
}
|
||||
|
||||
/// The main agent that coordinates all components.
|
||||
@@ -244,7 +246,10 @@ impl Agent {
|
||||
SchedulerDeps {
|
||||
tools: deps.tools.clone(),
|
||||
extension_manager: deps.extension_manager.clone(),
|
||||
store: deps.store.clone(),
|
||||
store: deps
|
||||
.store
|
||||
.as_ref()
|
||||
.map(|db| crate::tenant::AdminScope::new(Arc::clone(db))),
|
||||
hooks: deps.hooks.clone(),
|
||||
},
|
||||
);
|
||||
@@ -325,6 +330,50 @@ impl Agent {
|
||||
&self.deps.cost_guard
|
||||
}
|
||||
|
||||
/// Build a tenant-scoped execution context for the given user.
|
||||
///
|
||||
/// This is the standard entry point for per-user operations. The returned
|
||||
/// [`TenantCtx`] provides a [`TenantScope`] that auto-binds `user_id` on
|
||||
/// every database operation and a per-user rate limiter.
|
||||
pub(super) async fn tenant_ctx(&self, user_id: &str) -> crate::tenant::TenantCtx {
|
||||
let rate = self.deps.tenant_rates.get_or_create(user_id).await;
|
||||
|
||||
let store = self
|
||||
.deps
|
||||
.store
|
||||
.as_ref()
|
||||
.map(|db| crate::tenant::TenantScope::new(user_id, Arc::clone(db)));
|
||||
|
||||
// Reuse the owner workspace if user matches, otherwise create per-user.
|
||||
let workspace = match &self.deps.workspace {
|
||||
Some(ws) if ws.user_id() == user_id => Some(Arc::clone(ws)),
|
||||
_ => self
|
||||
.deps
|
||||
.store
|
||||
.as_ref()
|
||||
.map(|db| Arc::new(Workspace::new_with_db(user_id, Arc::clone(db)))),
|
||||
};
|
||||
|
||||
crate::tenant::TenantCtx::new(
|
||||
user_id,
|
||||
store,
|
||||
workspace,
|
||||
Arc::clone(&self.deps.cost_guard),
|
||||
rate,
|
||||
)
|
||||
}
|
||||
|
||||
/// Get an admin-scoped database accessor for cross-tenant operations.
|
||||
///
|
||||
/// Only for system-level components (heartbeat, routine engine, self-repair,
|
||||
/// scheduler). Handler code should use [`tenant_ctx()`](Self::tenant_ctx) instead.
|
||||
pub(super) fn admin_store(&self) -> Option<crate::tenant::AdminScope> {
|
||||
self.deps
|
||||
.store
|
||||
.as_ref()
|
||||
.map(|db| crate::tenant::AdminScope::new(Arc::clone(db)))
|
||||
}
|
||||
|
||||
pub(super) fn skill_registry(&self) -> Option<&Arc<std::sync::RwLock<SkillRegistry>>> {
|
||||
self.deps.skill_registry.as_ref()
|
||||
}
|
||||
@@ -410,8 +459,8 @@ impl Agent {
|
||||
self.config.stuck_threshold,
|
||||
self.config.max_repair_attempts,
|
||||
);
|
||||
if let Some(ref store) = self.deps.store {
|
||||
self_repair = self_repair.with_store(Arc::clone(store));
|
||||
if let Some(admin) = self.admin_store() {
|
||||
self_repair = self_repair.with_store(admin);
|
||||
}
|
||||
if let Some(ref builder) = self.deps.builder {
|
||||
self_repair = self_repair.with_builder(Arc::clone(builder), Arc::clone(self.tools()));
|
||||
@@ -518,6 +567,7 @@ impl Agent {
|
||||
.with_interval(std::time::Duration::from_secs(hb_config.interval_secs));
|
||||
config.quiet_hours_start = hb_config.quiet_hours_start;
|
||||
config.quiet_hours_end = hb_config.quiet_hours_end;
|
||||
config.multi_tenant = hb_config.multi_tenant;
|
||||
config.timezone = hb_config
|
||||
.timezone
|
||||
.clone()
|
||||
@@ -547,30 +597,52 @@ impl Agent {
|
||||
.await;
|
||||
let notify_user = heartbeat_notify_user;
|
||||
let channels = self.channels.clone();
|
||||
let is_multi_tenant = hb_config.multi_tenant;
|
||||
tokio::spawn(async move {
|
||||
while let Some(response) = notify_rx.recv().await {
|
||||
// In multi-tenant mode, extract the owning user_id from
|
||||
// the response metadata so notifications reach the
|
||||
// correct user rather than the agent's owner.
|
||||
// This intentionally overrides the configured notify_target
|
||||
// because each user's heartbeat should notify that user.
|
||||
let effective_user = if is_multi_tenant {
|
||||
response
|
||||
.metadata
|
||||
.get("owner_id")
|
||||
.and_then(|v| v.as_str())
|
||||
.map(String::from)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
|
||||
// Try the configured channel first, fall back to
|
||||
// broadcasting on all channels.
|
||||
let targeted_ok = if let Some(ref channel) = notify_channel
|
||||
&& let Some(ref user) = notify_target
|
||||
{
|
||||
channels
|
||||
.broadcast(channel, user, response.clone())
|
||||
.await
|
||||
.is_ok()
|
||||
let targeted_ok = if let Some(ref channel) = notify_channel {
|
||||
let target = effective_user.as_deref().or(notify_target.as_deref());
|
||||
if let Some(user) = target {
|
||||
channels
|
||||
.broadcast(channel, user, response.clone())
|
||||
.await
|
||||
.is_ok()
|
||||
} else {
|
||||
false
|
||||
}
|
||||
} else {
|
||||
false
|
||||
};
|
||||
|
||||
if !targeted_ok && let Some(ref user) = notify_user {
|
||||
let results = channels.broadcast_all(user, response).await;
|
||||
for (ch, result) in results {
|
||||
if let Err(e) = result {
|
||||
tracing::warn!(
|
||||
"Failed to broadcast heartbeat to {}: {}",
|
||||
ch,
|
||||
e
|
||||
);
|
||||
if !targeted_ok {
|
||||
let fallback = effective_user.as_deref().or(notify_user.as_deref());
|
||||
if let Some(user) = fallback {
|
||||
let results = channels.broadcast_all(user, response).await;
|
||||
for (ch, result) in results {
|
||||
if let Err(e) = result {
|
||||
tracing::warn!(
|
||||
"Failed to broadcast heartbeat to {}: {}",
|
||||
ch,
|
||||
e
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -583,14 +655,29 @@ impl Agent {
|
||||
.map(|h| h.to_workspace_config())
|
||||
.unwrap_or_default();
|
||||
|
||||
Some(spawn_heartbeat(
|
||||
config,
|
||||
hygiene,
|
||||
workspace.clone(),
|
||||
self.cheap_llm().clone(),
|
||||
Some(notify_tx),
|
||||
self.store().map(Arc::clone),
|
||||
))
|
||||
if config.multi_tenant {
|
||||
if let Some(admin) = self.admin_store() {
|
||||
Some(spawn_multi_user_heartbeat(
|
||||
config,
|
||||
hygiene,
|
||||
self.cheap_llm().clone(),
|
||||
Some(notify_tx),
|
||||
admin,
|
||||
))
|
||||
} else {
|
||||
tracing::warn!("Multi-tenant heartbeat requires a database store");
|
||||
None
|
||||
}
|
||||
} else {
|
||||
Some(spawn_heartbeat(
|
||||
config,
|
||||
hygiene,
|
||||
workspace.clone(),
|
||||
self.cheap_llm().clone(),
|
||||
Some(notify_tx),
|
||||
self.admin_store(),
|
||||
))
|
||||
}
|
||||
} else {
|
||||
tracing::warn!("Heartbeat enabled but no workspace available");
|
||||
None
|
||||
@@ -612,7 +699,7 @@ impl Agent {
|
||||
|
||||
let engine = Arc::new(RoutineEngine::new(
|
||||
rt_config.clone(),
|
||||
Arc::clone(store),
|
||||
crate::tenant::AdminScope::new(Arc::clone(store)),
|
||||
self.llm().clone(),
|
||||
Arc::clone(workspace),
|
||||
notify_tx,
|
||||
@@ -1173,13 +1260,22 @@ impl Agent {
|
||||
}
|
||||
}
|
||||
|
||||
// Build per-tenant execution context once; threaded through all handlers.
|
||||
let tenant = self.tenant_ctx(&message.user_id).await;
|
||||
|
||||
let session_for_empty_exit = Arc::clone(&session);
|
||||
|
||||
// Process based on submission type
|
||||
let result = match submission {
|
||||
Submission::UserInput { content } => {
|
||||
let mut result = self
|
||||
.process_user_input(message, session.clone(), thread_id, &content)
|
||||
.process_user_input(
|
||||
message,
|
||||
tenant.clone(),
|
||||
session.clone(),
|
||||
thread_id,
|
||||
&content,
|
||||
)
|
||||
.await;
|
||||
|
||||
// Drain any messages queued during processing.
|
||||
@@ -1246,7 +1342,13 @@ impl Agent {
|
||||
let mut queued_msg = message.clone();
|
||||
queued_msg.attachments.clear();
|
||||
result = self
|
||||
.process_user_input(&queued_msg, session.clone(), thread_id, &next_content)
|
||||
.process_user_input(
|
||||
&queued_msg,
|
||||
tenant.clone(),
|
||||
session.clone(),
|
||||
thread_id,
|
||||
&next_content,
|
||||
)
|
||||
.await;
|
||||
|
||||
// If processing failed, re-queue the drained content so it
|
||||
@@ -1294,7 +1396,7 @@ impl Agent {
|
||||
};
|
||||
}
|
||||
// Authorization checks (including restart channel check) are enforced in handle_system_command
|
||||
self.handle_system_command(&command, &args, &message.channel)
|
||||
self.handle_system_command(&command, &args, &message.channel, &tenant)
|
||||
.await
|
||||
}
|
||||
Submission::Undo => self.process_undo(session, thread_id).await,
|
||||
@@ -1307,12 +1409,9 @@ impl Agent {
|
||||
Submission::Summarize => self.process_summarize(session, thread_id).await,
|
||||
Submission::Suggest => self.process_suggest(session, thread_id).await,
|
||||
Submission::JobStatus { job_id } => {
|
||||
self.process_job_status(&message.user_id, job_id.as_deref())
|
||||
.await
|
||||
}
|
||||
Submission::JobCancel { job_id } => {
|
||||
self.process_job_cancel(&message.user_id, &job_id).await
|
||||
self.process_job_status(&tenant, job_id.as_deref()).await
|
||||
}
|
||||
Submission::JobCancel { job_id } => self.process_job_cancel(&tenant, &job_id).await,
|
||||
Submission::Quit => return Ok(None),
|
||||
Submission::SwitchThread { thread_id: target } => {
|
||||
self.process_switch_thread(message, target).await
|
||||
|
||||
Reference in New Issue
Block a user