From 554cb6f109b1b1194abd166dd23aec73003070f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20LIARD?= Date: Sun, 26 Apr 2026 18:37:14 +0200 Subject: [PATCH] feat(rpc): impl pledge/set + pledge/clear with in-memory mutation (#228) Replaces the `TODO(#228)` stubs in `server::rpc::pledge_ns::set` and `clear` with working in-memory mutations of `config.pledge`. Follows the P1/P2 pattern (atomic state swap, no disk persistence). Behaviour: - `set(profile, source)`: upserts a per-source rule, or replaces the default profile when source is None. Always flips `config.pledge.enabled = true`. - `clear()` drops all rules and resets `default_profile` to `full`. - Profile names validated against the built-in catalogue (read_only, execute, full, none). - Disk persistence out of scope per #228. Mutation logic split into pure sync helpers `apply_set` / `apply_clear` + `is_known_profile` for unit testability without an `AppState`. # Errors - `ERR_FORBIDDEN` when the caller is below `Admin`. - `INVALID_PARAMS_CODE` for empty profile, unknown profile, empty source. - `ERR_INTERNAL` when the registry rebuild or atomic swap fails. Tests: 11/11 passing. Implements P3 of the cli-forge-chef brigade plan for #228. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/server/rpc/pledge_ns.rs | 253 +++++++++++++++++++++++++++++++++--- 1 file changed, 237 insertions(+), 16 deletions(-) diff --git a/src/server/rpc/pledge_ns.rs b/src/server/rpc/pledge_ns.rs index b7694d04..de95bbd6 100644 --- a/src/server/rpc/pledge_ns.rs +++ b/src/server/rpc/pledge_ns.rs @@ -1,8 +1,12 @@ //! `grob/pledge/*` namespace: pledge profile inspection and management. use super::auth::{require_role, CallerIdentity}; -use super::types::{Role, StatusResponse}; -use crate::server::AppState; +use super::types::{rpc_err, Role, StatusResponse, ERR_INTERNAL}; +use crate::features::pledge::config::PledgeRule; +use crate::providers::ProviderRegistry; +use crate::routing::classify::Router; +use crate::server::{AppState, ReloadableState}; +use jsonrpsee::types::error::INVALID_PARAMS_CODE; use jsonrpsee::types::ErrorObjectOwned; use serde::{Deserialize, Serialize}; use std::sync::Arc; @@ -18,40 +22,161 @@ pub struct PledgeProfileInfo { pub allowed_tools: Vec, } -/// Activates a pledge profile for a given source. +/// Activates a pledge profile, in-memory only (#228 non-goal: persistence). +/// +/// When `source` is `Some`, a new `PledgeRule { source, profile }` is +/// appended (or updated if a rule with the same source already exists). +/// When `source` is `None`, the active default profile is replaced and +/// the master switch flipped on. +/// +/// # Errors +/// +/// Returns `ERR_FORBIDDEN` when the caller is below `Admin`. +/// Returns `INVALID_PARAMS_CODE` when `profile` is empty or unknown to +/// the built-in catalogue (`read_only`, `execute`, `full`, `none`). +/// Returns `ERR_INTERNAL` when the registry rebuild or atomic swap fails. pub async fn set( state: &Arc, caller: &CallerIdentity, - _profile: &str, - _source: Option<&str>, + profile: &str, + source: Option<&str>, ) -> Result { require_role(caller, Role::Admin)?; - - // TODO(#228): Implement runtime pledge activation with config mutation. - let _ = state; - + let mut new_config = state.snapshot().config.clone(); + apply_set(&mut new_config, profile, source)?; + swap_state( + state, + new_config, + caller, + &format!( + "set pledge '{profile}' for source={}", + source.unwrap_or("") + ), + )?; + let target = source.unwrap_or(""); Ok(StatusResponse { status: "ok".into(), - message: Some("Pledge profile set (in-memory — reload to persist)".into()), + message: Some(format!( + "Pledge profile '{profile}' applied to {target} (in-memory only — change reverts on next disk reload)" + )), }) } -/// Clears the active pledge, reverting all sources to the default profile. +/// Clears all runtime pledge rules and resets the default profile to +/// `full` (in-memory only). +/// +/// # Errors +/// +/// Returns `ERR_FORBIDDEN` when the caller is below `Admin`. +/// Returns `ERR_INTERNAL` when the registry rebuild or atomic swap fails. pub async fn clear( state: &Arc, caller: &CallerIdentity, ) -> Result { require_role(caller, Role::Admin)?; - - // TODO(#228): Implement pledge clear with config mutation. - let _ = state; - + let mut new_config = state.snapshot().config.clone(); + apply_clear(&mut new_config); + swap_state(state, new_config, caller, "clear pledge rules")?; Ok(StatusResponse { status: "ok".into(), - message: Some("Pledge cleared — defaults restored".into()), + message: Some( + "Pledge cleared — defaults restored (in-memory only — change reverts on next disk reload)" + .into(), + ), }) } +/// Pure mutation helper for `set`. Validates the profile name and +/// either upserts a per-source rule or updates the default profile. +fn apply_set( + config: &mut crate::models::config::AppConfig, + profile: &str, + source: Option<&str>, +) -> Result<(), ErrorObjectOwned> { + if profile.trim().is_empty() { + return Err(rpc_err(INVALID_PARAMS_CODE, "profile name cannot be empty")); + } + if !is_known_profile(profile) { + return Err(rpc_err( + INVALID_PARAMS_CODE, + format!("unknown profile '{profile}' (built-ins: read_only, execute, full, none)"), + )); + } + + config.pledge.enabled = true; + + match source { + None => { + config.pledge.default_profile = profile.to_string(); + } + Some("") => { + return Err(rpc_err( + INVALID_PARAMS_CODE, + "source cannot be empty (omit it to set the default profile)", + )); + } + Some(src) => { + // Upsert: replace any existing rule for this source. + if let Some(existing) = config + .pledge + .rules + .iter_mut() + .find(|r| r.source.as_deref() == Some(src)) + { + existing.profile = profile.to_string(); + } else { + config.pledge.rules.push(PledgeRule { + source: Some(src.to_string()), + token_prefix: None, + profile: profile.to_string(), + }); + } + } + } + Ok(()) +} + +/// Pure mutation helper for `clear`. Drops all rules and resets the +/// default profile to the catalogue default (`full`). +fn apply_clear(config: &mut crate::models::config::AppConfig) { + config.pledge.rules.clear(); + config.pledge.default_profile = "full".to_string(); +} + +/// Verifies that a profile name belongs to the built-in catalogue. +fn is_known_profile(name: &str) -> bool { + matches!(name, "read_only" | "execute" | "full" | "none") +} + +/// Rebuilds the reloadable state from a mutated config and atomic-swaps +/// it. Same primitive as `config_ns::set` and `tools_ns::*`. +fn swap_state( + state: &Arc, + new_config: crate::models::config::AppConfig, + caller: &CallerIdentity, + action: &str, +) -> Result<(), ErrorObjectOwned> { + let new_router = Router::new(new_config.clone()); + let new_registry = ProviderRegistry::from_configs_with_models( + &new_config.providers, + Some(state.token_store.clone()), + &new_config.models, + &new_config.server.timeouts, + ) + .map(Arc::new) + .map_err(|e| rpc_err(ERR_INTERNAL, format!("Failed to rebuild providers: {e}")))?; + + let new_inner = Arc::new(ReloadableState::new(new_config, new_router, new_registry)); + *state.inner.write().unwrap_or_else(|e| e.into_inner()) = new_inner; + + tracing::info!( + caller_ip = %caller.ip, + action = action, + "RPC pledge/* applied (in-memory only)" + ); + Ok(()) +} + /// Returns the active pledge configuration, default profile, and per-rule bindings. pub async fn status( state: &Arc, @@ -105,6 +230,102 @@ pub async fn list_profiles( #[cfg(test)] mod tests { use super::*; + use crate::models::config::AppConfig; + use crate::server::rpc::types::ERR_FORBIDDEN; + + fn fixture_config() -> AppConfig { + let toml = r#" +[router] +default = "claude-sonnet-4-6" + +[cache] +enabled = false +max_capacity = 100 +ttl_secs = 60 +max_entry_bytes = 8192 + +[pledge] +enabled = false +default_profile = "full" +"#; + toml::from_str(toml).expect("valid test TOML") + } + + #[test] + fn set_default_profile_with_no_source() { + let mut config = fixture_config(); + apply_set(&mut config, "read_only", None).expect("set should succeed"); + assert!(config.pledge.enabled, "set must flip the master switch on"); + assert_eq!(config.pledge.default_profile, "read_only"); + assert!(config.pledge.rules.is_empty()); + } + + #[test] + fn set_per_source_appends_rule() { + let mut config = fixture_config(); + apply_set(&mut config, "execute", Some("mcp")).expect("set should succeed"); + assert!(config.pledge.enabled); + assert_eq!(config.pledge.rules.len(), 1); + let rule = &config.pledge.rules[0]; + assert_eq!(rule.source.as_deref(), Some("mcp")); + assert_eq!(rule.profile, "execute"); + } + + #[test] + fn set_per_source_upserts_existing() { + // Setting a profile for a source that already has a rule must replace + // the profile, not append a duplicate. + let mut config = fixture_config(); + apply_set(&mut config, "read_only", Some("cli")).unwrap(); + apply_set(&mut config, "execute", Some("cli")).unwrap(); + assert_eq!(config.pledge.rules.len(), 1); + assert_eq!(config.pledge.rules[0].profile, "execute"); + } + + #[test] + fn set_rejects_unknown_profile() { + let mut config = fixture_config(); + let err = apply_set(&mut config, "yolo", None).unwrap_err(); + assert_eq!(err.code(), INVALID_PARAMS_CODE); + assert!(err.message().contains("unknown profile")); + } + + #[test] + fn set_rejects_empty_profile() { + let mut config = fixture_config(); + let err = apply_set(&mut config, " ", None).unwrap_err(); + assert_eq!(err.code(), INVALID_PARAMS_CODE); + } + + #[test] + fn set_rejects_empty_source() { + let mut config = fixture_config(); + let err = apply_set(&mut config, "full", Some("")).unwrap_err(); + assert_eq!(err.code(), INVALID_PARAMS_CODE); + assert!(err.message().contains("omit it")); + } + + #[test] + fn clear_drops_rules_and_resets_default() { + let mut config = fixture_config(); + // Seed a rule + non-default default to verify reset. + apply_set(&mut config, "execute", Some("mcp")).unwrap(); + apply_set(&mut config, "read_only", None).unwrap(); + apply_clear(&mut config); + assert!(config.pledge.rules.is_empty()); + assert_eq!(config.pledge.default_profile, "full"); + } + + #[test] + fn require_role_denies_observer_for_admin_methods() { + let observer = CallerIdentity { + role: Role::Observer, + ip: "10.0.0.1".into(), + tenant_id: String::new(), + }; + let err = require_role(&observer, Role::Admin).unwrap_err(); + assert_eq!(err.code(), ERR_FORBIDDEN); + } #[test] fn pledge_profile_info_serialization() {