mirror of
https://github.com/outbackdingo/optimclaw.git
synced 2026-08-26 23:50:17 +00:00
Add owner-scoped permissions for full-job routines (#1440)
* docs: add comments explaining CLI_ENABLED=false in service templates (#990) Clarify that CLI_ENABLED=false is needed in daemon mode (launchd/systemd) to prevent blocking on stdin when running as a background service. Closes #990 Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]> * Add owner-scoped full-job routine permissions * Address PR review feedback * Fix owner gate test timing --------- Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
c4ab382522
commit
cac6f4013c
+238
-14
@@ -17,7 +17,7 @@
|
||||
//! └──────────────┘
|
||||
//! ```
|
||||
|
||||
use std::collections::hash_map::DefaultHasher;
|
||||
use std::collections::{HashSet, hash_map::DefaultHasher};
|
||||
use std::hash::{Hash, Hasher};
|
||||
use std::str::FromStr;
|
||||
use std::time::Duration;
|
||||
@@ -28,6 +28,171 @@ use uuid::Uuid;
|
||||
|
||||
use crate::error::RoutineError;
|
||||
|
||||
pub const FULL_JOB_OWNER_ALLOWED_TOOLS_SETTING_KEY: &str = "routines.full_job_owner_allowed_tools";
|
||||
pub const FULL_JOB_DEFAULT_PERMISSION_MODE_SETTING_KEY: &str =
|
||||
"routines.full_job_default_permission_mode";
|
||||
|
||||
/// Persisted per-routine permission mode for autonomous `full_job` routines.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize, Default)]
|
||||
#[serde(rename_all = "snake_case")]
|
||||
pub enum FullJobPermissionMode {
|
||||
/// Only use the routine's stored `tool_permissions`.
|
||||
#[default]
|
||||
Explicit,
|
||||
/// Union the owner-scoped allowlist with the routine's `tool_permissions`.
|
||||
InheritOwner,
|
||||
}
|
||||
|
||||
impl FullJobPermissionMode {
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Explicit => "explicit",
|
||||
Self::InheritOwner => "inherit_owner",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl FromStr for FullJobPermissionMode {
|
||||
type Err = ();
|
||||
|
||||
fn from_str(s: &str) -> Result<Self, Self::Err> {
|
||||
match s {
|
||||
"explicit" => Ok(Self::Explicit),
|
||||
"inherit_owner" => Ok(Self::InheritOwner),
|
||||
_ => Err(()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Owner-scoped default behavior for newly-created `full_job` routines.
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)]
|
||||
pub enum FullJobPermissionDefaultMode {
|
||||
Explicit,
|
||||
#[default]
|
||||
InheritOwner,
|
||||
CopyOwner,
|
||||
}
|
||||
|
||||
impl FullJobPermissionDefaultMode {
|
||||
pub fn as_str(self) -> &'static str {
|
||||
match self {
|
||||
Self::Explicit => "explicit",
|
||||
Self::InheritOwner => "inherit_owner",
|
||||
Self::CopyOwner => "copy_owner",
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl FromStr for FullJobPermissionDefaultMode {
|
||||
type Err = ();
|
||||
|
||||
fn from_str(s: &str) -> Result<Self, Self::Err> {
|
||||
match s {
|
||||
"explicit" => Ok(Self::Explicit),
|
||||
"inherit_owner" => Ok(Self::InheritOwner),
|
||||
"copy_owner" => Ok(Self::CopyOwner),
|
||||
_ => Err(()),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Default)]
|
||||
pub struct FullJobPermissionSettings {
|
||||
pub owner_allowed_tools: Vec<String>,
|
||||
pub default_mode: FullJobPermissionDefaultMode,
|
||||
}
|
||||
|
||||
pub fn normalize_tool_names<I>(tools: I) -> Vec<String>
|
||||
where
|
||||
I: IntoIterator<Item = String>,
|
||||
{
|
||||
let mut seen = HashSet::new();
|
||||
let mut normalized = Vec::new();
|
||||
for tool in tools {
|
||||
let trimmed = tool.trim();
|
||||
if trimmed.is_empty() {
|
||||
continue;
|
||||
}
|
||||
let normalized_name = trimmed.to_string();
|
||||
if seen.insert(normalized_name.clone()) {
|
||||
normalized.push(normalized_name);
|
||||
}
|
||||
}
|
||||
normalized
|
||||
}
|
||||
|
||||
pub fn parse_full_job_permission_mode(value: &serde_json::Value) -> FullJobPermissionMode {
|
||||
value
|
||||
.get("permission_mode")
|
||||
.and_then(|v| v.as_str())
|
||||
.and_then(|mode| FullJobPermissionMode::from_str(mode).ok())
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
fn parse_owner_allowed_tools_setting(value: Option<serde_json::Value>) -> Vec<String> {
|
||||
match value {
|
||||
Some(serde_json::Value::Array(values)) => normalize_tool_names(
|
||||
values
|
||||
.into_iter()
|
||||
.filter_map(|value| value.as_str().map(ToOwned::to_owned)),
|
||||
),
|
||||
Some(serde_json::Value::String(csv)) => normalize_tool_names(
|
||||
csv.split([',', '\n'])
|
||||
.map(str::trim)
|
||||
.filter(|value| !value.is_empty())
|
||||
.map(ToOwned::to_owned),
|
||||
),
|
||||
_ => Vec::new(),
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_default_permission_mode_setting(
|
||||
value: Option<serde_json::Value>,
|
||||
) -> FullJobPermissionDefaultMode {
|
||||
value
|
||||
.and_then(|v| v.as_str().map(ToOwned::to_owned))
|
||||
.and_then(|mode| FullJobPermissionDefaultMode::from_str(&mode).ok())
|
||||
.unwrap_or_default()
|
||||
}
|
||||
|
||||
pub async fn load_full_job_permission_settings(
|
||||
store: &(dyn crate::db::SettingsStore + Sync),
|
||||
user_id: &str,
|
||||
) -> Result<FullJobPermissionSettings, crate::error::DatabaseError> {
|
||||
let owner_allowed_tools = parse_owner_allowed_tools_setting(
|
||||
store
|
||||
.get_setting(user_id, FULL_JOB_OWNER_ALLOWED_TOOLS_SETTING_KEY)
|
||||
.await?,
|
||||
);
|
||||
let default_mode = parse_default_permission_mode_setting(
|
||||
store
|
||||
.get_setting(user_id, FULL_JOB_DEFAULT_PERMISSION_MODE_SETTING_KEY)
|
||||
.await?,
|
||||
);
|
||||
Ok(FullJobPermissionSettings {
|
||||
owner_allowed_tools,
|
||||
default_mode,
|
||||
})
|
||||
}
|
||||
|
||||
pub fn effective_full_job_tool_permissions(
|
||||
permission_mode: FullJobPermissionMode,
|
||||
routine_tool_permissions: &[String],
|
||||
owner_allowed_tools: &[String],
|
||||
) -> Vec<String> {
|
||||
match permission_mode {
|
||||
FullJobPermissionMode::Explicit => {
|
||||
normalize_tool_names(routine_tool_permissions.iter().cloned())
|
||||
}
|
||||
FullJobPermissionMode::InheritOwner => normalize_tool_names(
|
||||
owner_allowed_tools
|
||||
.iter()
|
||||
.cloned()
|
||||
.chain(routine_tool_permissions.iter().cloned()),
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// A routine is a named, persistent, user-owned task with a trigger and an action.
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct Routine {
|
||||
@@ -240,6 +405,10 @@ pub enum RoutineAction {
|
||||
/// automatically permitted in routine jobs without listing them here.
|
||||
#[serde(default)]
|
||||
tool_permissions: Vec<String>,
|
||||
/// Whether this routine should inherit the owner's durable full-job
|
||||
/// permission allowlist or use only its explicit `tool_permissions`.
|
||||
#[serde(default)]
|
||||
permission_mode: FullJobPermissionMode,
|
||||
},
|
||||
}
|
||||
|
||||
@@ -266,15 +435,14 @@ fn clamp_max_tool_rounds(value: u64) -> u32 {
|
||||
|
||||
/// Parse a `tool_permissions` JSON array into a `Vec<String>`.
|
||||
pub fn parse_tool_permissions(value: &serde_json::Value) -> Vec<String> {
|
||||
value
|
||||
.get("tool_permissions")
|
||||
.and_then(|v| v.as_array())
|
||||
.map(|arr| {
|
||||
arr.iter()
|
||||
.filter_map(|v| v.as_str().map(String::from))
|
||||
.collect()
|
||||
})
|
||||
.unwrap_or_default()
|
||||
normalize_tool_names(
|
||||
value
|
||||
.get("tool_permissions")
|
||||
.and_then(|v| v.as_array())
|
||||
.into_iter()
|
||||
.flatten()
|
||||
.filter_map(|v| v.as_str().map(String::from)),
|
||||
)
|
||||
}
|
||||
|
||||
impl RoutineAction {
|
||||
@@ -352,11 +520,13 @@ impl RoutineAction {
|
||||
.unwrap_or(default_max_iterations() as u64)
|
||||
as u32;
|
||||
let tool_permissions = parse_tool_permissions(&config);
|
||||
let permission_mode = parse_full_job_permission_mode(&config);
|
||||
Ok(RoutineAction::FullJob {
|
||||
title,
|
||||
description,
|
||||
max_iterations,
|
||||
tool_permissions,
|
||||
permission_mode,
|
||||
})
|
||||
}
|
||||
other => Err(RoutineError::UnknownActionType {
|
||||
@@ -386,11 +556,13 @@ impl RoutineAction {
|
||||
description,
|
||||
max_iterations,
|
||||
tool_permissions,
|
||||
permission_mode,
|
||||
} => serde_json::json!({
|
||||
"title": title,
|
||||
"description": description,
|
||||
"max_iterations": max_iterations,
|
||||
"tool_permissions": tool_permissions,
|
||||
"permission_mode": permission_mode,
|
||||
}),
|
||||
}
|
||||
}
|
||||
@@ -704,8 +876,8 @@ pub fn describe_cron(schedule: &str, timezone: Option<&str>) -> String {
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use crate::agent::routine::{
|
||||
MAX_TOOL_ROUNDS_LIMIT, RoutineAction, RoutineGuardrails, RunStatus, Trigger, content_hash,
|
||||
describe_cron, next_cron_fire,
|
||||
FullJobPermissionMode, MAX_TOOL_ROUNDS_LIMIT, RoutineAction, RoutineGuardrails, RunStatus,
|
||||
Trigger, content_hash, describe_cron, effective_full_job_tool_permissions, next_cron_fire,
|
||||
};
|
||||
|
||||
#[test]
|
||||
@@ -773,15 +945,67 @@ mod tests {
|
||||
description: "Review and deploy pending changes".to_string(),
|
||||
max_iterations: 5,
|
||||
tool_permissions: vec!["shell".to_string()],
|
||||
permission_mode: FullJobPermissionMode::InheritOwner,
|
||||
};
|
||||
let json = action.to_config_json();
|
||||
let parsed = RoutineAction::from_db("full_job", json).expect("parse full_job");
|
||||
assert!(
|
||||
matches!(parsed, RoutineAction::FullJob { title, max_iterations, tool_permissions, .. }
|
||||
if title == "Deploy review" && max_iterations == 5 && tool_permissions == vec!["shell".to_string()])
|
||||
matches!(parsed, RoutineAction::FullJob { title, max_iterations, tool_permissions, permission_mode, .. }
|
||||
if title == "Deploy review"
|
||||
&& max_iterations == 5
|
||||
&& tool_permissions == vec!["shell".to_string()]
|
||||
&& permission_mode == FullJobPermissionMode::InheritOwner)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_action_full_job_missing_permission_mode_defaults_to_explicit() {
|
||||
let parsed = RoutineAction::from_db(
|
||||
"full_job",
|
||||
serde_json::json!({
|
||||
"title": "Deploy review",
|
||||
"description": "Review and deploy pending changes",
|
||||
"max_iterations": 5,
|
||||
"tool_permissions": ["shell"]
|
||||
}),
|
||||
)
|
||||
.expect("parse full_job");
|
||||
assert!(matches!(
|
||||
parsed,
|
||||
RoutineAction::FullJob {
|
||||
permission_mode: FullJobPermissionMode::Explicit,
|
||||
..
|
||||
}
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_effective_full_job_tool_permissions_inherit_owner_unions_lists() {
|
||||
let resolved = effective_full_job_tool_permissions(
|
||||
FullJobPermissionMode::InheritOwner,
|
||||
&["shell".to_string(), "message".to_string()],
|
||||
&["message".to_string(), "http".to_string()],
|
||||
);
|
||||
assert_eq!(
|
||||
resolved,
|
||||
vec![
|
||||
"message".to_string(),
|
||||
"http".to_string(),
|
||||
"shell".to_string()
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_effective_full_job_tool_permissions_explicit_ignores_owner_defaults() {
|
||||
let resolved = effective_full_job_tool_permissions(
|
||||
FullJobPermissionMode::Explicit,
|
||||
&["shell".to_string()],
|
||||
&["message".to_string(), "http".to_string()],
|
||||
);
|
||||
assert_eq!(resolved, vec!["shell".to_string()]);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_run_status_display_parse() {
|
||||
for status in [
|
||||
|
||||
+49
-19
@@ -22,7 +22,8 @@ use uuid::Uuid;
|
||||
|
||||
use crate::agent::Scheduler;
|
||||
use crate::agent::routine::{
|
||||
NotifyConfig, Routine, RoutineAction, RoutineRun, RunStatus, Trigger, next_cron_fire,
|
||||
NotifyConfig, Routine, RoutineAction, RoutineRun, RunStatus, Trigger,
|
||||
effective_full_job_tool_permissions, load_full_job_permission_settings, next_cron_fire,
|
||||
};
|
||||
use crate::channels::OutgoingResponse;
|
||||
use crate::config::RoutineConfig;
|
||||
@@ -890,17 +891,16 @@ async fn execute_routine(ctx: EngineContext, routine: Routine, run: RoutineRun)
|
||||
description,
|
||||
max_iterations,
|
||||
tool_permissions,
|
||||
permission_mode,
|
||||
} => {
|
||||
execute_full_job(
|
||||
&ctx,
|
||||
&routine,
|
||||
&run,
|
||||
let execution = FullJobExecutionConfig {
|
||||
title,
|
||||
description,
|
||||
*max_iterations,
|
||||
max_iterations: *max_iterations,
|
||||
tool_permissions,
|
||||
)
|
||||
.await
|
||||
permission_mode: *permission_mode,
|
||||
};
|
||||
execute_full_job(&ctx, &routine, &run, &execution).await
|
||||
}
|
||||
};
|
||||
|
||||
@@ -1026,14 +1026,19 @@ fn sanitize_routine_name(name: &str) -> String {
|
||||
/// non-active state (not Pending/InProgress/Stuck). Returns the final
|
||||
/// `RunStatus` mapped from the job outcome. This keeps the routine run
|
||||
/// active for the full job lifetime so concurrency guardrails apply.
|
||||
struct FullJobExecutionConfig<'a> {
|
||||
title: &'a str,
|
||||
description: &'a str,
|
||||
max_iterations: u32,
|
||||
tool_permissions: &'a [String],
|
||||
permission_mode: crate::agent::routine::FullJobPermissionMode,
|
||||
}
|
||||
|
||||
async fn execute_full_job(
|
||||
ctx: &EngineContext,
|
||||
routine: &Routine,
|
||||
run: &RoutineRun,
|
||||
title: &str,
|
||||
description: &str,
|
||||
max_iterations: u32,
|
||||
tool_permissions: &[String],
|
||||
execution: &FullJobExecutionConfig<'_>,
|
||||
) -> Result<(RunStatus, Option<String>, Option<i32>), RoutineError> {
|
||||
let scheduler = ctx
|
||||
.scheduler
|
||||
@@ -1042,8 +1047,10 @@ async fn execute_full_job(
|
||||
reason: "scheduler not available".to_string(),
|
||||
})?;
|
||||
|
||||
let mut metadata =
|
||||
serde_json::json!({ "max_iterations": max_iterations, "owner_id": routine.user_id });
|
||||
let mut metadata = serde_json::json!({
|
||||
"max_iterations": execution.max_iterations,
|
||||
"owner_id": routine.user_id
|
||||
});
|
||||
// Carry the routine's notify config in job metadata so the message tool
|
||||
// can resolve channel/target per-job without global state mutation.
|
||||
if let Some(channel) = &routine.notify.channel {
|
||||
@@ -1051,15 +1058,38 @@ async fn execute_full_job(
|
||||
}
|
||||
metadata["notify_user"] = serde_json::json!(&routine.notify.user);
|
||||
|
||||
let effective_permissions = match execution.permission_mode {
|
||||
crate::agent::routine::FullJobPermissionMode::Explicit => {
|
||||
effective_full_job_tool_permissions(
|
||||
execution.permission_mode,
|
||||
execution.tool_permissions,
|
||||
&[],
|
||||
)
|
||||
}
|
||||
crate::agent::routine::FullJobPermissionMode::InheritOwner => {
|
||||
let owner_permissions =
|
||||
load_full_job_permission_settings(ctx.store.as_ref(), &routine.user_id)
|
||||
.await
|
||||
.map_err(|e| RoutineError::Database {
|
||||
reason: format!("failed to load routine permission settings: {e}"),
|
||||
})?;
|
||||
effective_full_job_tool_permissions(
|
||||
execution.permission_mode,
|
||||
execution.tool_permissions,
|
||||
&owner_permissions.owner_allowed_tools,
|
||||
)
|
||||
}
|
||||
};
|
||||
|
||||
// Build approval context: UnlessAutoApproved tools are auto-approved for routines;
|
||||
// Always tools require explicit listing in tool_permissions.
|
||||
let approval_context = ApprovalContext::autonomous_with_tools(tool_permissions.iter().cloned());
|
||||
// Always tools require explicit listing in the resolved effective permissions.
|
||||
let approval_context = ApprovalContext::autonomous_with_tools(effective_permissions);
|
||||
|
||||
let job_id = scheduler
|
||||
.dispatch_job_with_context(
|
||||
&routine.user_id,
|
||||
title,
|
||||
description,
|
||||
execution.title,
|
||||
execution.description,
|
||||
Some(metadata),
|
||||
approval_context,
|
||||
)
|
||||
@@ -1082,7 +1112,7 @@ async fn execute_full_job(
|
||||
tracing::info!(
|
||||
routine = %routine.name,
|
||||
job_id = %job_id,
|
||||
max_iterations = max_iterations,
|
||||
max_iterations = execution.max_iterations,
|
||||
"Dispatched full job for routine, watching for completion"
|
||||
);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user