From 9824a32add26858d7f768ebd33e0de0acdab5efe Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 3 Aug 2026 00:32:05 +0200 Subject: [PATCH 1/3] General Improvements --- Cargo.lock | 66 ++++- Cargo.toml | 4 + src/agents.rs | 2 +- src/ai_response.rs | 7 +- src/data_retention.rs | 393 ++++++++++++++++++++++++++++ src/enums.rs | 176 +++++++++++++ src/errors.rs | 3 +- src/krill.rs | 4 +- src/lib.rs | 24 +- src/mutations.rs | 64 +++++ src/principal.rs | 588 ++++++++++++++++++++++++++++++++++++++++++ src/sync.rs | 24 ++ src/task.rs | 19 +- src/todo.rs | 26 +- src/tools.rs | 24 +- 15 files changed, 1376 insertions(+), 48 deletions(-) create mode 100644 src/data_retention.rs create mode 100644 src/principal.rs diff --git a/Cargo.lock b/Cargo.lock index b8b8ebf..5bda36e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -12,6 +12,15 @@ dependencies = [ "generic-array", ] +[[package]] +name = "aho-corasick" +version = "1.1.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ddd31a130427c27518df266943a5308ed92d4b226cc639f5a8f1002816174301" +dependencies = [ + "memchr", +] + [[package]] name = "android_system_properties" version = "0.1.5" @@ -101,6 +110,12 @@ dependencies = [ "pkg-config", ] +[[package]] +name = "base64" +version = "0.21.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "9d297deb1925b89f2ccc13d7635fa0714f12c87adce1c75356b39ca9b7178567" + [[package]] name = "base64" version = "0.22.1" @@ -422,6 +437,7 @@ checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" dependencies = [ "block-buffer 0.10.4", "crypto-common 0.1.7", + "subtle", ] [[package]] @@ -599,7 +615,16 @@ version = "0.13.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "4aaa26c720c68b866f2c96ef5c1264b3e6f473fe5d4ce61cd44bbe913e553018" dependencies = [ - "hmac", + "hmac 0.13.0", +] + +[[package]] +name = "hmac" +version = "0.12.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6c49c37c09c17a53d937dfbb742eb3a961d65a994e6bcdcf37e7399d0cc8ab5e" +dependencies = [ + "digest 0.10.7", ] [[package]] @@ -972,7 +997,7 @@ name = "mtp-codec" version = "0.1.0" source = "git+https://git@git.methanium.net/methanium/mtp.git#6a65e43ca9f7d128b691c8946adaeecccc2cb2ec" dependencies = [ - "base64", + "base64 0.22.1", "byteorder", "mtp-common", "mtp-crypto", @@ -996,7 +1021,7 @@ name = "mtp-crypto" version = "0.1.0" source = "git+https://git@git.methanium.net/methanium/mtp.git#6a65e43ca9f7d128b691c8946adaeecccc2cb2ec" dependencies = [ - "base64", + "base64 0.22.1", "chacha20poly1305", "ed25519-dalek", "getrandom 0.4.3", @@ -1154,7 +1179,7 @@ version = "3.0.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "1d30c53c26bc5b31a98cd02d20f25a7c8567146caf63ed593a9d87b2775291be" dependencies = [ - "base64", + "base64 0.22.1", "serde_core", ] @@ -1401,6 +1426,35 @@ dependencies = [ "bitflags", ] +[[package]] +name = "regex" +version = "1.13.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f020237b6c8eed93db2e2cb53c00c60a8e1bc73da7d073199a1180401450218d" +dependencies = [ + "aho-corasick", + "memchr", + "regex-automata", + "regex-syntax", +] + +[[package]] +name = "regex-automata" +version = "0.4.16" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8fcfdb36bda0c880c5931cdc7a2bcdc8ba4556847b9d912bca70bc94708711ad" +dependencies = [ + "aho-corasick", + "memchr", + "regex-syntax", +] + +[[package]] +name = "regex-syntax" +version = "0.8.11" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6f6ff9a378485b298a5286656da665ba74413d36db0979633275d2e708145d4" + [[package]] name = "ring" version = "0.17.14" @@ -1668,10 +1722,14 @@ name = "shoal-types" version = "0.1.0" dependencies = [ "async-trait", + "base64 0.21.7", "chrono", + "hmac 0.12.1", "mtp", + "regex", "serde", "serde_json", + "sha2 0.10.9", "tempfile", "thiserror 2.0.18", "tokio", diff --git a/Cargo.toml b/Cargo.toml index d393b94..bcee5e3 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -16,6 +16,10 @@ mtp = { git = "https://git@git.methanium.net/methanium/mtp.git", features = [ tokio = "1" async-trait = "0.1" url = "2" +regex = "1" +hmac = "0.12" +sha2 = "0.10" +base64 = "0.21" [dev-dependencies] tokio = { version = "1", features = ["full"] } diff --git a/src/agents.rs b/src/agents.rs index b4d6d33..12cc7ab 100644 --- a/src/agents.rs +++ b/src/agents.rs @@ -1,5 +1,5 @@ -use serde::{Deserialize, Serialize}; use crate::task::SessionBudget; +use serde::{Deserialize, Serialize}; /* ExploreAgent is intentionally limited to read-only tools so it can be dispatched without approval for safe, unrestricted codebase exploration. */ diff --git a/src/ai_response.rs b/src/ai_response.rs index 9d52aff..d9fa1e9 100644 --- a/src/ai_response.rs +++ b/src/ai_response.rs @@ -105,12 +105,7 @@ impl StreamEvent { Self::ToolExecuting { tool_name, tool_id } } - pub fn tool_result( - tool_name: String, - tool_id: String, - status: String, - result: String, - ) -> Self { + pub fn tool_result(tool_name: String, tool_id: String, status: String, result: String) -> Self { Self::ToolResult { tool_name, tool_id, diff --git a/src/data_retention.rs b/src/data_retention.rs new file mode 100644 index 0000000..795d05e --- /dev/null +++ b/src/data_retention.rs @@ -0,0 +1,393 @@ +use regex::Regex; +use std::sync::LazyLock; + +/// Patterns for sensitive data that should be redacted from logs and event payloads. +static REDACTION_PATTERNS: LazyLock> = LazyLock::new(|| { + vec![ + ( + "api_key".to_string(), + Regex::new(r#"(?i)(api[_-]?key|apikey)\s*[:=]\s*["']?([A-Za-z0-9\-_]{20,})["']?"#) + .unwrap(), + ), + ( + "bearer_token".to_string(), + Regex::new( + r#"(?i)(bearer|token|authorization)\s*[:=]\s*["']?([A-Za-z0-9\-_\.]{20,})["']?"#, + ) + .unwrap(), + ), + ( + "password".to_string(), + Regex::new(r#"(?i)(password|passwd|pwd)\s*[:=]\s*["']?(\S{8,})["']?"#).unwrap(), + ), + ( + "private_key".to_string(), + Regex::new(r#"-----BEGIN\s+(RSA\s+)?PRIVATE\s+KEY-----"#).unwrap(), + ), + ( + "connection_string".to_string(), + Regex::new(r#"(?i)(mongodb|postgres|mysql|redis|amqp)://\S+"#).unwrap(), + ), + ( + "jwt_token".to_string(), + Regex::new(r#"eyJ[A-Za-z0-9_-]{10,}\.eyJ[A-Za-z0-9_-]{10,}"#).unwrap(), + ), + ] +}); + +/// Redact sensitive information from a string. +/// +/// Replaces matched patterns with `` to prevent accidental +/// leakage of secrets through logs, event payloads, or error messages. +pub fn redact_secrets(input: &str) -> String { + let mut result = input.to_string(); + for (label, pattern) in REDACTION_PATTERNS.iter() { + result = pattern + .replace_all(&result, format!("", label)) + .to_string(); + } + result +} + +/// Truncate a string to a maximum byte length, appending "..." if truncated. +pub fn truncate_bytes(input: &str, max_bytes: usize) -> String { + let bytes = input.as_bytes(); + if bytes.len() <= max_bytes { + input.to_string() + } else { + let truncated = String::from_utf8_lossy(&bytes[..max_bytes.saturating_sub(3)]); + format!("{}...", truncated) + } +} + +/// Data classification for storage rules. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum DataClass { + /// Source code files — retained for the project lifetime. + SourceCode, + /// LLM prompts and system instructions — confidential, short retention. + Prompt, + /// Model-generated output — confidential, short retention. + ModelOutput, + /// Tool call arguments — confidential, may contain secrets. + ToolArguments, + /// Tool call results — confidential, may contain file content. + ToolResult, + /// User approval decisions — audit trail, long retention. + ApprovalData, + /// API keys, tokens, credentials — secret, never persisted in logs. + Credentials, + /// File mutation content (diffs, snapshots) — internal, medium retention. + MutationContent, + /// Operational traces (heartbeats, health checks) — internal, short retention. + OperationalTrace, + /// Domain events — internal, medium retention. + DomainEvent, + /// UI events (Lookout replay) — internal, short retention. + UiEvent, + /// Execution logs — confidential, short retention. + ExecutionLog, + /// Agent run records — internal, medium retention. + AgentRun, + /// Documentation output — source code, project lifetime. + Documentation, +} + +/// Retention configuration per data class. +#[derive(Debug, Clone)] +pub struct RetentionPolicy { + /// Maximum number of days to retain processed events. + pub event_retention_days: u32, + /// Maximum number of days to retain completed agent runs. + pub run_retention_days: u32, + /// Maximum number of days to retain audit logs. + pub audit_retention_days: u32, + /// Maximum size of event payloads in bytes (larger payloads are truncated). + pub max_event_payload_bytes: usize, + /// Maximum number of days to retain execution logs. + pub execution_log_retention_days: u32, + /// Maximum number of days to retain UI replay events. + pub ui_event_retention_days: u32, + /// Maximum number of days to retain documentation. + pub documentation_retention_days: u32, +} + +impl Default for RetentionPolicy { + fn default() -> Self { + Self { + event_retention_days: 30, + run_retention_days: 90, + audit_retention_days: 60, + max_event_payload_bytes: 64 * 1024, // 64 KB + execution_log_retention_days: 14, + ui_event_retention_days: 7, + documentation_retention_days: 365, + } + } +} + +impl RetentionPolicy { + /// Create a retention policy from environment variables with sensible defaults. + pub fn from_env() -> Self { + Self { + event_retention_days: read_env_u32("SHOAL_EVENT_RETENTION_DAYS", 30), + run_retention_days: read_env_u32("SHOAL_RUN_RETENTION_DAYS", 90), + audit_retention_days: read_env_u32("SHOAL_AUDIT_RETENTION_DAYS", 60), + max_event_payload_bytes: read_env_usize("SHOAL_MAX_EVENT_PAYLOAD_BYTES", 64 * 1024), + execution_log_retention_days: read_env_u32("SHOAL_EXEC_LOG_RETENTION_DAYS", 14), + ui_event_retention_days: read_env_u32("SHOAL_UI_EVENT_RETENTION_DAYS", 7), + documentation_retention_days: read_env_u32("SHOAL_DOCS_RETENTION_DAYS", 365), + } + } + + /// Returns the retention period in days for a given data class. + pub fn retention_days(&self, class: DataClass) -> u32 { + match class { + DataClass::SourceCode | DataClass::Documentation => { + self.documentation_retention_days + } + DataClass::Prompt | DataClass::ModelOutput => 7, + DataClass::ToolArguments | DataClass::ToolResult => { + self.execution_log_retention_days + } + DataClass::ApprovalData | DataClass::AgentRun => self.run_retention_days, + DataClass::Credentials => 0, // never retained + DataClass::MutationContent => self.run_retention_days, + DataClass::OperationalTrace => 3, + DataClass::DomainEvent => self.event_retention_days, + DataClass::UiEvent => self.ui_event_retention_days, + DataClass::ExecutionLog => self.execution_log_retention_days, + } + } + + /// Whether the data class should be redacted before persistence. + pub fn should_redact(&self, class: DataClass) -> bool { + matches!( + class, + DataClass::ToolArguments + | DataClass::ToolResult + | DataClass::Credentials + | DataClass::Prompt + | DataClass::ModelOutput + | DataClass::ExecutionLog + ) + } +} + +/// Applied retention policy service. Wraps a `RetentionPolicy` and provides +/// methods to enforce redaction and truncation on data before it reaches any +/// store (domain events, UI events, execution logs, agent runs, etc.). +pub struct DataRetentionService { + policy: RetentionPolicy, +} + +impl DataRetentionService { + pub fn new(policy: RetentionPolicy) -> Self { + Self { policy } + } + + pub fn from_env() -> Self { + Self::new(RetentionPolicy::from_env()) + } + + /// Returns a reference to the underlying policy. + pub fn policy(&self) -> &RetentionPolicy { + &self.policy + } + + /// Apply redaction and truncation to a payload string based on its data class. + pub fn prepare_for_storage(&self, data: &str, class: DataClass) -> String { + let result = if self.policy.should_redact(class) { + redact_secrets(data) + } else { + data.to_string() + }; + let max_bytes = match class { + DataClass::DomainEvent | DataClass::UiEvent => self.policy.max_event_payload_bytes, + DataClass::ExecutionLog => 32 * 1024, // 32 KB + DataClass::AgentRun => 128 * 1024, // 128 KB + _ => usize::MAX, + }; + truncate_bytes(&result, max_bytes) + } + + /// Prepare a JSON value for storage, redacting sensitive fields. + pub fn prepare_json_for_storage( + &self, + value: &serde_json::Value, + class: DataClass, + ) -> serde_json::Value { + if !self.policy.should_redact(class) { + return value.clone(); + } + redact_json_secrets(value) + } + + /// Returns true if the given data class should be retained at all. + /// Credentials, for example, should never be persisted. + pub fn should_retain(&self, class: DataClass) -> bool { + self.policy.retention_days(class) > 0 + } +} + +/// Recursively redact sensitive fields in a JSON value. +fn redact_json_secrets(value: &serde_json::Value) -> serde_json::Value { + match value { + serde_json::Value::String(s) => serde_json::Value::String(redact_secrets(s)), + serde_json::Value::Object(map) => { + let redacted: serde_json::Map = map + .iter() + .map(|(k, v)| { + let new_v = if is_sensitive_json_field(k) { + serde_json::Value::String("".to_string()) + } else { + redact_json_secrets(v) + }; + (k.clone(), new_v) + }) + .collect(); + serde_json::Value::Object(redacted) + } + serde_json::Value::Array(arr) => { + serde_json::Value::Array(arr.iter().map(redact_json_secrets).collect()) + } + other => other.clone(), + } +} + +fn is_sensitive_json_field(name: &str) -> bool { + matches!( + name.to_lowercase().as_str(), + "password" | "secret" | "api_key" | "apikey" | "token" | "authorization" + | "private_key" | "credentials" | "env" | "environment" | "bearer" + ) +} + +fn read_env_u32(key: &str, default: u32) -> u32 { + std::env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +fn read_env_usize(key: &str, default: usize) -> usize { + std::env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_redact_api_key() { + let input = r#"{"api_key": "sk-abcdefghijklmnopqrstuvwxyz123456"}"#; + let redacted = redact_secrets(input); + assert!(redacted.contains("")); + assert!(!redacted.contains("sk-abcdefghijklmnopqrstuvwxyz123456")); + } + + #[test] + fn test_redact_bearer_token() { + let input = "Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9"; + let redacted = redact_secrets(input); + assert!(redacted.contains("")); + } + + #[test] + fn test_redact_password() { + let input = r#"password = "supersecret123""#; + let redacted = redact_secrets(input); + assert!(redacted.contains("")); + assert!(!redacted.contains("supersecret123")); + } + + #[test] + fn test_redact_private_key() { + let input = "-----BEGIN RSA PRIVATE KEY-----"; + let redacted = redact_secrets(input); + assert!(redacted.contains("")); + } + + #[test] + fn test_no_redaction_for_clean_text() { + let input = "This is a normal log message with no secrets."; + let redacted = redact_secrets(input); + assert_eq!(input, redacted); + } + + #[test] + fn test_truncate_bytes() { + assert_eq!(truncate_bytes("hello", 10), "hello"); + assert_eq!(truncate_bytes("hello world", 8), "hello..."); + assert_eq!(truncate_bytes("", 5), ""); + } + + #[test] + fn test_retention_policy_defaults() { + let policy = RetentionPolicy::default(); + assert_eq!(policy.event_retention_days, 30); + assert_eq!(policy.run_retention_days, 90); + assert_eq!(policy.audit_retention_days, 60); + assert_eq!(policy.max_event_payload_bytes, 64 * 1024); + assert_eq!(policy.execution_log_retention_days, 14); + assert_eq!(policy.ui_event_retention_days, 7); + assert_eq!(policy.documentation_retention_days, 365); + } + + #[test] + fn test_data_retention_service_redacts_tool_args() { + let svc = DataRetentionService::from_env(); + let input = r#"{"api_key": "sk-abcdefghijklmnopqrstuvwxyz123456"}"#; + let result = svc.prepare_for_storage(input, DataClass::ToolArguments); + assert!(result.contains("")); + } + + #[test] + fn test_data_retention_service_no_redact_for_source_code() { + let svc = DataRetentionService::from_env(); + let input = "fn main() { let api_key = \"test\"; }"; + let result = svc.prepare_for_storage(input, DataClass::SourceCode); + assert_eq!(input, result); + } + + #[test] + fn test_data_retention_service_credentials_never_retained() { + let svc = DataRetentionService::from_env(); + assert!(!svc.should_retain(DataClass::Credentials)); + } + + #[test] + fn test_redact_json_secrets() { + let value = serde_json::json!({ + "name": "test", + "api_key": "sk-abcdefghijklmnopqrstuvwxyz123456", + "nested": { + "password": "hunter2" + } + }); + let redacted = redact_json_secrets(&value); + assert_eq!(redacted["name"], "test"); + assert_eq!(redacted["api_key"], ""); + assert_eq!(redacted["nested"]["password"], ""); + } + + #[test] + fn test_redact_connection_string() { + let input = "Connecting to postgres://user:pass@localhost/db"; + let redacted = redact_secrets(input); + assert!(redacted.contains("")); + } + + #[test] + fn test_retention_days_by_class() { + let policy = RetentionPolicy::default(); + assert_eq!(policy.retention_days(DataClass::Credentials), 0); + assert_eq!(policy.retention_days(DataClass::OperationalTrace), 3); + assert_eq!(policy.retention_days(DataClass::Prompt), 7); + assert_eq!(policy.retention_days(DataClass::DomainEvent), 30); + assert_eq!(policy.retention_days(DataClass::AgentRun), 90); + } +} diff --git a/src/enums.rs b/src/enums.rs index e4befa6..48ddcf9 100644 --- a/src/enums.rs +++ b/src/enums.rs @@ -2,6 +2,7 @@ use serde::{Deserialize, Serialize}; use std::fmt; #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] pub enum ToDoMode { Discussion, Finalized, @@ -34,6 +35,7 @@ impl std::str::FromStr for ToDoMode { } #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] pub enum ToDoStatus { Pending, InProgress, @@ -44,6 +46,10 @@ pub enum ToDoStatus { Failed, PendingApproval, Draft, + /// Agent succeeded and reported mutations; awaiting mutation review/approval. + ChangesPending, + /// Mutations have been approved; awaiting application and validation. + ChangesApproved, } impl Default for ToDoStatus { @@ -64,6 +70,8 @@ impl fmt::Display for ToDoStatus { ToDoStatus::Failed => write!(f, "failed"), ToDoStatus::Draft => write!(f, "draft"), ToDoStatus::PendingApproval => write!(f, "pending_approval"), + ToDoStatus::ChangesPending => write!(f, "changes_pending"), + ToDoStatus::ChangesApproved => write!(f, "changes_approved"), } } } @@ -81,6 +89,8 @@ impl std::str::FromStr for ToDoStatus { "failed" => Ok(ToDoStatus::Failed), "draft" => Ok(ToDoStatus::Draft), "pending_approval" => Ok(ToDoStatus::PendingApproval), + "changes_pending" => Ok(ToDoStatus::ChangesPending), + "changes_approved" => Ok(ToDoStatus::ChangesApproved), _ => Err(format!("unknown status: {}", s)), } } @@ -262,6 +272,172 @@ impl std::fmt::Display for ToDoSource { } } +impl std::str::FromStr for ToDoSource { + type Err = String; + + fn from_str(s: &str) -> Result { + match s { + "user" => Ok(ToDoSource::User), + "planner" => Ok(ToDoSource::Planner), + "automation" => Ok(ToDoSource::Automation), + "delegation" => Ok(ToDoSource::Delegation), + _ => Err(format!("unknown todo source: {}", s)), + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ExecutionKind { + Manual, + Planner, + Build, + TestEvaluation, + TestImplementation, + Documentation, + Explore, +} + +impl Default for ExecutionKind { + fn default() -> Self { + ExecutionKind::Manual + } +} + +impl std::fmt::Display for ExecutionKind { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + ExecutionKind::Manual => write!(f, "manual"), + ExecutionKind::Planner => write!(f, "planner"), + ExecutionKind::Build => write!(f, "build"), + ExecutionKind::TestEvaluation => write!(f, "test_evaluation"), + ExecutionKind::TestImplementation => write!(f, "test_implementation"), + ExecutionKind::Documentation => write!(f, "documentation"), + ExecutionKind::Explore => write!(f, "explore"), + } + } +} + +impl std::str::FromStr for ExecutionKind { + type Err = String; + fn from_str(s: &str) -> Result { + match s { + "manual" => Ok(ExecutionKind::Manual), + "planner" => Ok(ExecutionKind::Planner), + "build" => Ok(ExecutionKind::Build), + "test_evaluation" => Ok(ExecutionKind::TestEvaluation), + "test_implementation" => Ok(ExecutionKind::TestImplementation), + "documentation" => Ok(ExecutionKind::Documentation), + "explore" => Ok(ExecutionKind::Explore), + _ => Err(format!("unknown execution kind: {}", s)), + } + } +} + +/// Lifecycle stage of a mutation set reported by an agent. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum MutationLifecycle { + /// Agent generated mutations but they have not yet been staged for review. + Generated, + /// Mutations are staged and ready for human or agent-flow review. + Staged, + /// Mutations have been reviewed (may follow with approve/reject). + Reviewed, + /// Mutations have been approved for application. + Approved, + /// Mutations have been applied to the working tree. + Applied, + /// Applied mutations have passed validation (tests, checks). + Validated, + /// Mutations have been committed. + Committed, + /// Mutations were rejected. + Rejected, +} + +impl Default for MutationLifecycle { + fn default() -> Self { + MutationLifecycle::Generated + } +} + +impl fmt::Display for MutationLifecycle { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + MutationLifecycle::Generated => write!(f, "generated"), + MutationLifecycle::Staged => write!(f, "staged"), + MutationLifecycle::Reviewed => write!(f, "reviewed"), + MutationLifecycle::Approved => write!(f, "approved"), + MutationLifecycle::Applied => write!(f, "applied"), + MutationLifecycle::Validated => write!(f, "validated"), + MutationLifecycle::Committed => write!(f, "committed"), + MutationLifecycle::Rejected => write!(f, "rejected"), + } + } +} + +impl std::str::FromStr for MutationLifecycle { + type Err = String; + fn from_str(s: &str) -> Result { + match s { + "generated" => Ok(MutationLifecycle::Generated), + "staged" => Ok(MutationLifecycle::Staged), + "reviewed" => Ok(MutationLifecycle::Reviewed), + "approved" => Ok(MutationLifecycle::Approved), + "applied" => Ok(MutationLifecycle::Applied), + "validated" => Ok(MutationLifecycle::Validated), + "committed" => Ok(MutationLifecycle::Committed), + "rejected" => Ok(MutationLifecycle::Rejected), + _ => Err(format!("unknown mutation lifecycle: {}", s)), + } + } +} + +/// Controls how task completion depends on mutation lifecycle. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum CompletionPolicy { + /// Mutations are auto-applied without review; task completes after validation. + AutoApply, + /// Mutations require human/agent-flow approval before application. + ApprovalRequired, + /// Mutations are recorded but never applied (sandbox-only). + DryRun, + /// No mutations expected; task completes when agent succeeds. + DocumentationOnly, +} + +impl Default for CompletionPolicy { + fn default() -> Self { + CompletionPolicy::ApprovalRequired + } +} + +impl fmt::Display for CompletionPolicy { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + CompletionPolicy::AutoApply => write!(f, "auto_apply"), + CompletionPolicy::ApprovalRequired => write!(f, "approval_required"), + CompletionPolicy::DryRun => write!(f, "dry_run"), + CompletionPolicy::DocumentationOnly => write!(f, "documentation_only"), + } + } +} + +impl std::str::FromStr for CompletionPolicy { + type Err = String; + fn from_str(s: &str) -> Result { + match s { + "auto_apply" => Ok(CompletionPolicy::AutoApply), + "approval_required" => Ok(CompletionPolicy::ApprovalRequired), + "dry_run" => Ok(CompletionPolicy::DryRun), + "documentation_only" => Ok(CompletionPolicy::DocumentationOnly), + _ => Err(format!("unknown completion policy: {}", s)), + } + } +} + #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] pub enum TestStrategy { diff --git a/src/errors.rs b/src/errors.rs index 08ab112..589b04c 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -29,8 +29,7 @@ impl ValidationError { | ValidationError::InvalidPriority | ValidationError::InvalidStatusTransition { .. } | ValidationError::InvalidField(_) => 400, - ValidationError::CircularDependency(_) - | ValidationError::DependencyNotFound(_) => 409, + ValidationError::CircularDependency(_) | ValidationError::DependencyNotFound(_) => 409, } } } diff --git a/src/krill.rs b/src/krill.rs index b8316e4..9fefb7f 100644 --- a/src/krill.rs +++ b/src/krill.rs @@ -9,7 +9,9 @@ pub enum PodProtocolVersion { } impl Default for PodProtocolVersion { - fn default() -> Self { Self::V1 } + fn default() -> Self { + Self::V1 + } } pub type KrillId = Uuid; diff --git a/src/lib.rs b/src/lib.rs index 183cd88..c27f069 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -7,10 +7,12 @@ pub mod agents; pub mod ai_response; pub mod conclusion; pub mod coral; +pub mod data_retention; pub mod enums; pub mod errors; pub mod krill; pub mod mutations; +pub mod principal; pub mod project; pub mod sandbox; pub mod sync; @@ -26,21 +28,24 @@ pub use conclusion::{ Artifact, ArtifactType, ConclusionCard, ConclusionMetrics, ConclusionTrigger, }; pub use coral::{Coral, CoralId}; +pub use data_retention::{ + redact_secrets, truncate_bytes, DataClass, DataRetentionService, RetentionPolicy, +}; pub use enums::*; pub use errors::{ShoalError, ValidationError}; pub use krill::{KrillConfig, KrillDescriptor, KrillId, PodProtocolVersion}; -pub use mutations::FileMutation; +pub use mutations::{FileMutation, PathValidationError, merged_affected_files, normalize_repository_path}; +pub use principal::ExecutionPrincipal; pub use project::{Project, ProjectFile, ProjectId, ProjectSettings}; pub use task::{Task, TaskResult}; pub use todo::{Dependency, ToDo}; pub use tools::{ - compute_tools_hash, format_tool_error, format_tool_result, format_tools_json, - is_global_tool, is_reef_proxy_tool, - parse_tool_call_blocks, parse_tool_call_stream, tool_definitions, tool_ids_for_agent_type, - ApprovalRequirement, CompositionStep, ContextVisibility, ExecutionContext, - MarketplaceSearchResults, ParsedToolCall, TestAction, ToolComposition, ToolDefinition, - ToolDependency, ToolDocumentation, ToolExample, ToolExecutor, ToolListing, ToolParser, - ToolPlugin, ToolRegistry, ToolState, ToolTest, ToolTestResult, ToolTestSuite, + compute_tools_hash, format_tool_error, format_tool_result, format_tools_json, is_global_tool, + is_reef_proxy_tool, parse_tool_call_blocks, parse_tool_call_stream, tool_definitions, + tool_ids_for_agent_type, ApprovalRequirement, CompositionStep, ContextVisibility, + ExecutionContext, MarketplaceSearchResults, ParsedToolCall, TestAction, ToolComposition, + ToolDefinition, ToolDependency, ToolDocumentation, ToolExample, ToolExecutor, ToolListing, + ToolParser, ToolPlugin, ToolRegistry, ToolState, ToolTest, ToolTestResult, ToolTestSuite, ToolTestSuiteResult, ToolVersion, GLOBAL_TOOLS, REEF_PROXY_TOOLS, }; @@ -60,7 +65,8 @@ mod tests { "TypeMap missing AuthRequest" ); assert!( - tm.comm_id_enum(CommunicationType::PasswordAuthRequest).is_some(), + tm.comm_id_enum(CommunicationType::PasswordAuthRequest) + .is_some(), "TypeMap missing PasswordAuthRequest" ); assert!( diff --git a/src/mutations.rs b/src/mutations.rs index 8c14787..e03b519 100644 --- a/src/mutations.rs +++ b/src/mutations.rs @@ -1,4 +1,5 @@ use serde::{Deserialize, Serialize}; +use std::collections::BTreeSet; /// A single file change inferred from a sandbox diff: `hash_before` absent means the file /// was created, `hash_after` absent means it was deleted, both present means it was modified. @@ -8,3 +9,66 @@ pub struct FileMutation { pub hash_before: Option, pub hash_after: Option, } + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PathValidationError { + AbsolutePath(String), + UnresolvedParent(String), + EmptyPath, +} + +impl std::fmt::Display for PathValidationError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::AbsolutePath(p) => write!(f, "absolute path not allowed: {}", p), + Self::UnresolvedParent(p) => write!(f, "unresolved .. segment: {}", p), + Self::EmptyPath => write!(f, "empty path"), + } + } +} + +impl std::error::Error for PathValidationError {} + +/// Normalize a repository-relative file path: convert backslashes, strip leading `./`, +/// reject absolute paths and unresolved `..` segments. +pub fn normalize_repository_path(path: &str) -> Result { + let normalized = path.replace('\\', "/"); + let trimmed = normalized.trim_start_matches("./"); + + if trimmed.is_empty() { + return Err(PathValidationError::EmptyPath); + } + + if trimmed.starts_with('/') { + return Err(PathValidationError::AbsolutePath(trimmed.to_string())); + } + + // Check for unresolved .. segments + for component in trimmed.split('/') { + if component == ".." { + return Err(PathValidationError::UnresolvedParent(trimmed.to_string())); + } + } + + Ok(trimmed.to_string()) +} + +/// Merge declared affected files with mutation-derived paths, normalizing and +/// deduplicating the result. Deletion mutations are preserved — a deleted source +/// file can require documentation removal or test updates. +pub fn merged_affected_files( + declared: &[String], + mutations: &[FileMutation], +) -> Result, PathValidationError> { + let mut files = BTreeSet::new(); + + for path in declared + .iter() + .cloned() + .chain(mutations.iter().map(|m| m.file.clone())) + { + files.insert(normalize_repository_path(&path)?); + } + + Ok(files.into_iter().collect()) +} diff --git a/src/principal.rs b/src/principal.rs new file mode 100644 index 0000000..7267fa9 --- /dev/null +++ b/src/principal.rs @@ -0,0 +1,588 @@ +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use crate::agents::AgentType; + +/// Errors that can occur during credential verification. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum CredentialError { + /// The credential signature does not match the payload. + InvalidSignature, + /// The credential has expired. + Expired, + /// The credential payload is malformed or could not be deserialized. + MalformedPayload, + /// The credential ID has been revoked. + Revoked, + /// The pod identity does not match the assigned pod. + PodMismatch, + /// The attempt is no longer active (completed, cancelled, or not found). + AttemptInactive, +} + +impl std::fmt::Display for CredentialError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::InvalidSignature => write!(f, "invalid credential signature"), + Self::Expired => write!(f, "credential expired"), + Self::MalformedPayload => write!(f, "malformed credential payload"), + Self::Revoked => write!(f, "credential revoked"), + Self::PodMismatch => write!(f, "credential not valid for this pod"), + Self::AttemptInactive => write!(f, "attempt inactive"), + } + } +} + +impl std::error::Error for CredentialError {} + +/// The verifiable claims inside a signed execution credential. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct CredentialClaims { + /// The project this execution is scoped to. + pub project_id: Uuid, + /// The todo being serviced. + pub todo_id: Uuid, + /// The unique run identifier. + pub run_id: Uuid, + /// Attempt number (1-indexed). + pub attempt_id: u32, + /// The pod this credential is bound to. + pub pod_id: Uuid, + /// Agent type classification. + pub agent_type: AgentType, + /// Tools this principal is authorized to invoke. + pub allowed_tools: Vec, + /// Absolute deadline after which this principal is invalid. + pub expires_at: DateTime, + /// When this credential was minted. + pub issued_at: DateTime, + /// Unique credential identifier for revocation tracking. + pub credential_id: Uuid, +} + +/// A signed execution credential that binds an agent run attempt to a specific +/// pod, project, and tool set. The credential is HMAC-SHA256 signed by the +/// dispatcher and verified by the API before any tool execution is authorized. +/// +/// # Format +/// +/// The credential string is `base64(payload).base64(hmac_signature)` where +/// payload is the JSON-serialized `CredentialClaims`. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct SignedCredential { + /// The base64-encoded JSON payload. + pub payload: String, + /// The base64-encoded HMAC-SHA256 signature. + pub signature: String, +} + +impl SignedCredential { + /// Parse a credential string into its two base64 components. + pub fn parse(credential: &str) -> Option<(&str, &str)> { + let (payload, sig) = credential.split_once('.')?; + if payload.is_empty() || sig.is_empty() { + return None; + } + Some((payload, sig)) + } +} + +/// A cryptographically bounded execution identity for agent runs. +/// +/// Instead of trusting agent-supplied request headers or model-generated IDs +/// for mutation authorization, every tool execution carries a signed +/// `ExecutionPrincipal` that is minted by the dispatcher at dispatch time and +/// verified before any state mutation. This prevents agents from escalating +/// privileges or spoofing their identity. +/// +/// # Fields +/// +/// * `subject` – The agent type that was authorized to run. +/// * `project_id` – The project this execution is scoped to. +/// * `todo_id` – The specific todo this run is servicing. +/// * `run_id` – The unique run identifier minted by the dispatcher. +/// * `attempt_id` – Monotonic attempt counter for this run (1-indexed). +/// * `pod_id` – The pod this credential is bound to. +/// * `agent_type` – The agent type classification for tool-visibility checks. +/// * `allowed_tools` – Explicit tool allow-list derived from the agent policy. +/// * `expires_at` – Absolute deadline after which the principal is no longer valid. +/// * `issued_at` – Timestamp when this principal was minted. +/// * `credential_id` – Unique identifier for revocation tracking. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ExecutionPrincipal { + /// The identity subject (agent type string). + pub subject: String, + /// Project scope for this execution. + pub project_id: Uuid, + /// The todo being serviced. + pub todo_id: Uuid, + /// The unique run identifier. + pub run_id: Uuid, + /// Attempt number (1-indexed). + pub attempt_id: u32, + /// The pod this credential is bound to. + pub pod_id: Uuid, + /// Agent type classification. + pub agent_type: AgentType, + /// Tools this principal is authorized to invoke. + pub allowed_tools: Vec, + /// Absolute deadline after which this principal is invalid. + pub expires_at: DateTime, + /// When this principal was minted. + pub issued_at: DateTime, + /// Unique credential identifier for revocation tracking. + pub credential_id: Uuid, +} + +impl ExecutionPrincipal { + /// Mint a new principal for a dispatch attempt. + pub fn mint( + project_id: Uuid, + todo_id: Uuid, + run_id: Uuid, + attempt_id: u32, + pod_id: Uuid, + agent_type: AgentType, + allowed_tools: Vec, + timeout_secs: u64, + ) -> Self { + let now = Utc::now(); + Self { + subject: agent_type.as_str().to_string(), + project_id, + todo_id, + run_id, + attempt_id, + pod_id, + agent_type, + allowed_tools, + expires_at: now + chrono::Duration::seconds(timeout_secs as i64), + issued_at: now, + credential_id: Uuid::new_v4(), + } + } + + /// Check whether this principal is still valid at the given timestamp. + pub fn is_valid_at(&self, now: DateTime) -> bool { + now <= self.expires_at + } + + /// Check whether the given tool is in the allow-list. + pub fn can_invoke_tool(&self, tool_name: &str) -> bool { + self.allowed_tools.iter().any(|t| t == tool_name) + } + + /// Convert this principal into a `CredentialClaims` suitable for signing. + pub fn to_claims(&self) -> CredentialClaims { + CredentialClaims { + project_id: self.project_id, + todo_id: self.todo_id, + run_id: self.run_id, + attempt_id: self.attempt_id, + pod_id: self.pod_id, + agent_type: self.agent_type.clone(), + allowed_tools: self.allowed_tools.clone(), + expires_at: self.expires_at, + issued_at: self.issued_at, + credential_id: self.credential_id, + } + } + + /// Create an expired principal (for testing). + #[cfg(test)] + pub fn expired(project_id: Uuid, todo_id: Uuid, run_id: Uuid) -> Self { + Self { + subject: "test".to_string(), + project_id, + todo_id, + run_id, + attempt_id: 1, + pod_id: Uuid::new_v4(), + agent_type: AgentType::BuildAgent, + allowed_tools: vec![], + expires_at: Utc::now() - chrono::Duration::hours(1), + issued_at: Utc::now() - chrono::Duration::hours(2), + credential_id: Uuid::new_v4(), + } + } +} + +/// Sign a principal's claims with HMAC-SHA256, producing a `SignedCredential`. +/// +/// The shared secret is held only by the Reef server; Pods receive the +/// credential string and verify it via `verify_credential`. +pub fn sign_credential( + principal: &ExecutionPrincipal, + secret: &[u8], +) -> Result { + use base64::Engine; + use hmac::{Hmac, Mac}; + use sha2::Sha256; + + let claims = principal.to_claims(); + let payload_json = serde_json::to_vec(&claims).map_err(|_| CredentialError::MalformedPayload)?; + let payload_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(&payload_json); + + let mut mac = + Hmac::::new_from_slice(secret).map_err(|_| CredentialError::MalformedPayload)?; + mac.update(payload_b64.as_bytes()); + let signature = mac.finalize().into_bytes(); + let sig_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(signature); + + Ok(format!("{}.{}", payload_b64, sig_b64)) +} + +/// Verify and decode a signed credential, returning the claims if valid. +/// +/// Checks performed: +/// 1. Signature verification (HMAC-SHA256) +/// 2. Expiry verification +/// 3. Pod identity binding +/// +/// Revocation and attempt-active checks require external state (database) and +/// must be performed by the caller after this returns `Ok`. +pub fn verify_credential( + credential: &str, + secret: &[u8], + expected_pod_id: Uuid, +) -> Result { + use base64::Engine; + use hmac::{Hmac, Mac}; + use sha2::Sha256; + + let (payload_b64, sig_b64) = SignedCredential::parse(credential) + .ok_or(CredentialError::MalformedPayload)?; + + // Decode payload + let payload_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(payload_b64) + .map_err(|_| CredentialError::MalformedPayload)?; + let claims: CredentialClaims = + serde_json::from_slice(&payload_bytes).map_err(|_| CredentialError::MalformedPayload)?; + + // Verify signature + let mut mac = + Hmac::::new_from_slice(secret).map_err(|_| CredentialError::MalformedPayload)?; + mac.update(payload_b64.as_bytes()); + let sig_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(sig_b64) + .map_err(|_| CredentialError::MalformedPayload)?; + mac.verify_slice(&sig_bytes) + .map_err(|_| CredentialError::InvalidSignature)?; + + // Check expiry + if Utc::now() > claims.expires_at { + return Err(CredentialError::Expired); + } + + // Check pod binding + if claims.pod_id != expected_pod_id { + return Err(CredentialError::PodMismatch); + } + + Ok(claims) +} + +/// Convert verified `CredentialClaims` back into an `ExecutionPrincipal`. +impl From for ExecutionPrincipal { + fn from(claims: CredentialClaims) -> Self { + Self { + subject: claims.agent_type.as_str().to_string(), + project_id: claims.project_id, + todo_id: claims.todo_id, + run_id: claims.run_id, + attempt_id: claims.attempt_id, + pod_id: claims.pod_id, + agent_type: claims.agent_type, + allowed_tools: claims.allowed_tools, + expires_at: claims.expires_at, + issued_at: claims.issued_at, + credential_id: claims.credential_id, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn test_secret() -> Vec { + b"test-secret-key-for-hmac-signing-operations".to_vec() + } + + #[test] + fn test_mint_principal() { + let project_id = Uuid::new_v4(); + let todo_id = Uuid::new_v4(); + let run_id = Uuid::new_v4(); + let pod_id = Uuid::new_v4(); + + let principal = ExecutionPrincipal::mint( + project_id, + todo_id, + run_id, + 1, + pod_id, + AgentType::BuildAgent, + vec!["read_file".to_string(), "write_file".to_string()], + 3600, + ); + + assert_eq!(principal.subject, "build_agent"); + assert_eq!(principal.project_id, project_id); + assert_eq!(principal.todo_id, todo_id); + assert_eq!(principal.run_id, run_id); + assert_eq!(principal.attempt_id, 1); + assert_eq!(principal.pod_id, pod_id); + assert!(principal.is_valid_at(Utc::now())); + } + + #[test] + fn test_tool_authorization() { + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + Uuid::new_v4(), + AgentType::ExploreAgent, + vec!["read_file".to_string(), "search_files".to_string()], + 3600, + ); + + assert!(principal.can_invoke_tool("read_file")); + assert!(principal.can_invoke_tool("search_files")); + assert!(!principal.can_invoke_tool("write_file")); + assert!(!principal.can_invoke_tool("edit_file")); + } + + #[test] + fn test_empty_allowed_tools_denies_all() { + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + Uuid::new_v4(), + AgentType::BuildAgent, + vec![], + 3600, + ); + + assert!(!principal.can_invoke_tool("any_tool")); + } + + #[test] + fn test_expired_principal() { + let project_id = Uuid::new_v4(); + let todo_id = Uuid::new_v4(); + let run_id = Uuid::new_v4(); + let principal = ExecutionPrincipal::expired(project_id, todo_id, run_id); + + assert!(!principal.is_valid_at(Utc::now())); + } + + #[test] + fn test_serialization_roundtrip() { + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 2, + Uuid::new_v4(), + AgentType::Planner, + vec!["read_file".to_string()], + 600, + ); + + let json = serde_json::to_string(&principal).unwrap(); + let deserialized: ExecutionPrincipal = serde_json::from_str(&json).unwrap(); + + assert_eq!(principal.subject, deserialized.subject); + assert_eq!(principal.project_id, deserialized.project_id); + assert_eq!(principal.run_id, deserialized.run_id); + assert_eq!(principal.attempt_id, deserialized.attempt_id); + assert_eq!(principal.pod_id, deserialized.pod_id); + } + + #[test] + fn test_sign_and_verify_credential() { + let secret = test_secret(); + let pod_id = Uuid::new_v4(); + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + pod_id, + AgentType::BuildAgent, + vec!["read_file".to_string()], + 3600, + ); + + let credential = sign_credential(&principal, &secret).unwrap(); + let claims = verify_credential(&credential, &secret, pod_id).unwrap(); + + assert_eq!(claims.project_id, principal.project_id); + assert_eq!(claims.todo_id, principal.todo_id); + assert_eq!(claims.run_id, principal.run_id); + assert_eq!(claims.attempt_id, 1); + assert_eq!(claims.pod_id, pod_id); + assert_eq!(claims.agent_type, AgentType::BuildAgent); + assert_eq!(claims.allowed_tools, vec!["read_file".to_string()]); + } + + #[test] + fn test_verify_rejects_wrong_signature() { + let secret = test_secret(); + let wrong_secret = b"wrong-secret-key-for-hmac-signing-operations"; + let pod_id = Uuid::new_v4(); + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + pod_id, + AgentType::BuildAgent, + vec![], + 3600, + ); + + let credential = sign_credential(&principal, &secret).unwrap(); + let result = verify_credential(&credential, wrong_secret, pod_id); + assert_eq!(result.unwrap_err(), CredentialError::InvalidSignature); + } + + #[test] + fn test_verify_rejects_wrong_pod() { + let secret = test_secret(); + let pod_id = Uuid::new_v4(); + let wrong_pod_id = Uuid::new_v4(); + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + pod_id, + AgentType::BuildAgent, + vec![], + 3600, + ); + + let credential = sign_credential(&principal, &secret).unwrap(); + let result = verify_credential(&credential, &secret, wrong_pod_id); + assert_eq!(result.unwrap_err(), CredentialError::PodMismatch); + } + + #[test] + fn test_verify_rejects_expired_credential() { + let secret = test_secret(); + let pod_id = Uuid::new_v4(); + let mut principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + pod_id, + AgentType::BuildAgent, + vec![], + 3600, + ); + // Force expiry + principal.expires_at = Utc::now() - chrono::Duration::hours(1); + + let credential = sign_credential(&principal, &secret).unwrap(); + let result = verify_credential(&credential, &secret, pod_id); + assert_eq!(result.unwrap_err(), CredentialError::Expired); + } + + #[test] + fn test_verify_rejects_malformed_credential() { + let result = verify_credential("not-a-valid-credential", b"secret", Uuid::new_v4()); + assert_eq!(result.unwrap_err(), CredentialError::MalformedPayload); + } + + #[test] + fn test_verify_rejects_tampered_payload() { + let secret = test_secret(); + let pod_id = Uuid::new_v4(); + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + pod_id, + AgentType::BuildAgent, + vec![], + 3600, + ); + + let credential = sign_credential(&principal, &secret).unwrap(); + // Tamper with the payload + let parts: Vec<&str> = credential.splitn(2, '.').collect(); + let tampered = format!("{}.{}", parts[0], parts[1]); + // Modify a char in the signature to invalidate it + let mut sig_chars: Vec = tampered.chars().collect(); + if let Some(c) = sig_chars.last_mut() { + *c = if *c == 'A' { 'B' } else { 'A' }; + } + let tampered_credential: String = sig_chars.iter().collect(); + let result = verify_credential(&tampered_credential, &secret, pod_id); + assert_eq!(result.unwrap_err(), CredentialError::InvalidSignature); + } + + #[test] + fn test_credential_claims_roundtrip() { + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 3, + Uuid::new_v4(), + AgentType::Planner, + vec!["read_file".to_string(), "kanban_create_todo".to_string()], + 600, + ); + + let claims = principal.to_claims(); + let restored: ExecutionPrincipal = claims.into(); + + assert_eq!(restored.project_id, principal.project_id); + assert_eq!(restored.run_id, principal.run_id); + assert_eq!(restored.attempt_id, 3); + assert_eq!(restored.agent_type, AgentType::Planner); + assert_eq!( + restored.allowed_tools, + vec!["read_file".to_string(), "kanban_create_todo".to_string()] + ); + } + + #[test] + fn test_credential_format_is_two_base64_parts() { + let secret = test_secret(); + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + Uuid::new_v4(), + AgentType::BuildAgent, + vec![], + 3600, + ); + + let credential = sign_credential(&principal, &secret).unwrap(); + let (payload, sig) = SignedCredential::parse(&credential).unwrap(); + assert!(!payload.is_empty()); + assert!(!sig.is_empty()); + // Verify both parts are valid base64 + use base64::Engine; + assert!(base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(payload) + .is_ok()); + assert!(base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(sig) + .is_ok()); + } +} diff --git a/src/sync.rs b/src/sync.rs index 08879bb..2083fea 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -8,6 +8,29 @@ pub enum ConflictResolution { ResolvedManual, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub enum ResolutionStrategy { + Local, + Remote, +} + +impl ResolutionStrategy { + pub fn from_str(s: &str) -> Option { + match s { + "local" => Some(ResolutionStrategy::Local), + "remote" => Some(ResolutionStrategy::Remote), + _ => None, + } + } + + pub fn as_str(&self) -> &'static str { + match self { + ResolutionStrategy::Local => "local", + ResolutionStrategy::Remote => "remote", + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct FileConflict { pub file: String, @@ -89,6 +112,7 @@ pub struct TopicChange { pub file_path: String, pub operation: TopicChangeOperation, pub blob_hash: String, + pub approved: Option, } #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] diff --git a/src/task.rs b/src/task.rs index 8ab94c5..19b8448 100644 --- a/src/task.rs +++ b/src/task.rs @@ -218,19 +218,13 @@ impl ConnectorConfig { if let Some(tokens) = self.max_tokens { if tokens <= 0 { - errors.push(format!( - "max_tokens must be > 0, got {}", - tokens - )); + errors.push(format!("max_tokens must be > 0, got {}", tokens)); } } if let Some(temp) = self.temperature { if !(0.0..=2.0).contains(&temp) { - errors.push(format!( - "temperature must be in [0.0, 2.0], got {}", - temp - )); + errors.push(format!("temperature must be in [0.0, 2.0], got {}", temp)); } } @@ -253,7 +247,12 @@ pub struct AgentTaskSpec { pub resource_limits: ResourceLimits, pub parent_task_id: Option, pub pre_approved_plan_id: Option, - pub execution_token: Option, + /// Signed execution credential minted by the dispatcher. Carries the + /// verified identity (project, todo, run, attempt, pod, agent type, + /// allowed tools) as an HMAC-signed token. The API verifies this before + /// authorizing any tool execution. + #[serde(default)] + pub execution_credential: Option, pub session_budget: Option, /// Maximum number of checklist tasks this agent run may add. `None` leaves /// task creation unconstrained for backwards-compatible callers. @@ -304,7 +303,7 @@ impl AgentTaskSpec { }, parent_task_id: None, pre_approved_plan_id: None, - execution_token: None, + execution_credential: None, session_budget: None, task_budget: None, agent_type: None, diff --git a/src/todo.rs b/src/todo.rs index 0551472..26a23c9 100644 --- a/src/todo.rs +++ b/src/todo.rs @@ -2,7 +2,7 @@ use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use uuid::Uuid; -use crate::enums::{ToDoMode, ToDoSource, ToDoStatus}; +use crate::enums::{ExecutionKind, ToDoMode, ToDoSource, ToDoStatus}; use crate::errors::ValidationError; use std::collections::HashSet; @@ -41,6 +41,16 @@ pub struct ToDo { pub tags: Vec, pub estimated_tokens: u32, pub tools_json: Option, + #[serde(default)] + pub execution_kind: ExecutionKind, + #[serde(default)] + pub assigned_agent_type: Option, + #[serde(default)] + pub revision: u64, + pub completed_at: Option>, + pub failure_reason: Option, + pub created_by_run_id: Option, + pub idempotency_key: Option, } impl Default for ToDo { @@ -68,6 +78,13 @@ impl Default for ToDo { tags: Vec::new(), estimated_tokens: 0, tools_json: None, + execution_kind: ExecutionKind::default(), + assigned_agent_type: None, + revision: 0, + completed_at: None, + failure_reason: None, + created_by_run_id: None, + idempotency_key: None, } } } @@ -97,6 +114,13 @@ impl ToDo { tags: Vec::new(), estimated_tokens: 0, tools_json: None, + execution_kind: ExecutionKind::default(), + assigned_agent_type: None, + revision: 0, + completed_at: None, + failure_reason: None, + created_by_run_id: None, + idempotency_key: None, } } diff --git a/src/tools.rs b/src/tools.rs index 7bfb86c..c62b2de 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -1865,7 +1865,7 @@ pub fn tool_definitions() -> Vec { // ======================== tool!( "report_completion", - "Report completion status of a todo back to the Bridge", + "Report completion status of the current task. The todo_id is derived from the execution credential.", "agent", false, false, @@ -1873,14 +1873,14 @@ pub fn tool_definitions() -> Vec { serde_json::json!({ "type": "object", "properties": { - "todo_id": { "type": "string", "description": "ID of the todo being reported" }, - "status": { "type": "string", "enum": ["done", "failed", "delegated", "needs_retry", "pending_approval"], "description": "Completion status" }, + "status": { "type": "string", "enum": ["completed", "failed", "delegated", "needs_retry", "pending_approval"], "description": "Completion status" }, "summary": { "type": "string", "description": "Summary of what was done (max 3 sentences)" }, - "artifact_refs": { "type": "array", "items": { "type": "string" }, "description": "File paths, test logs" }, + "affected_files": { "type": "array", "items": { "type": "string" }, "description": "Files that were modified" }, + "validation_commands": { "type": "array", "items": { "type": "string" }, "description": "Commands run to validate the work" }, "retry_plan": { "type": "string", "description": "Optional retry plan if status is needs_retry" }, - "subtask_ids": { "type": "array", "items": { "type": "string" }, "description": "Subtask IDs if status is delegated" } + "proposed_mutations": { "type": "array", "items": { "type": "string" }, "description": "Proposed file mutations" } }, - "required": ["todo_id", "status"] + "required": ["status"] }) ), tool!( @@ -2071,8 +2071,6 @@ pub fn tool_ids_for_agent_type(agent_type: &crate::agents::AgentType) -> Option< "list_files", "list_directory", "bash", - "kanban_create_todo", - "kanban_update_todo", "report_completion", ]), crate::agents::AgentType::Planner => by_visibility(&[ @@ -2434,12 +2432,10 @@ Done with tools. let results = parse_tool_call_blocks(text); assert_eq!(results.len(), 1); assert_eq!(results[0].call_id, "call_01"); - assert!( - results[0].payload["content"] - .as_str() - .unwrap_or("") - .contains("🎉") - ); + assert!(results[0].payload["content"] + .as_str() + .unwrap_or("") + .contains("🎉")); } #[test] From 5140f4ddc879dacaeae4fcc483b10224b962193b Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 6 Aug 2026 00:15:22 +0200 Subject: [PATCH 2/3] General Improvements 4 --- src/principal.rs | 41 ++++++++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/src/principal.rs b/src/principal.rs index 7267fa9..8f38761 100644 --- a/src/principal.rs +++ b/src/principal.rs @@ -233,26 +233,26 @@ pub fn sign_credential( Ok(format!("{}.{}", payload_b64, sig_b64)) } -/// Verify and decode a signed credential, returning the claims if valid. +/// Verify the cryptographic signature and expiry of a signed credential. /// -/// Checks performed: -/// 1. Signature verification (HMAC-SHA256) -/// 2. Expiry verification -/// 3. Pod identity binding +/// This performs only the pure cryptographic checks that require no external state: +/// 1. Parse the credential format +/// 2. Verify HMAC-SHA256 signature +/// 3. Check expiry /// -/// Revocation and attempt-active checks require external state (database) and -/// must be performed by the caller after this returns `Ok`. -pub fn verify_credential( +/// Returns the verified claims on success. The caller is responsible for +/// performing persistence-level validation (pod binding, attempt active, +/// revocation) after this returns `Ok`. +pub fn verify_signature( credential: &str, secret: &[u8], - expected_pod_id: Uuid, ) -> Result { use base64::Engine; use hmac::{Hmac, Mac}; use sha2::Sha256; - let (payload_b64, sig_b64) = SignedCredential::parse(credential) - .ok_or(CredentialError::MalformedPayload)?; + let (payload_b64, sig_b64) = + SignedCredential::parse(credential).ok_or(CredentialError::MalformedPayload)?; // Decode payload let payload_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD @@ -276,6 +276,25 @@ pub fn verify_credential( return Err(CredentialError::Expired); } + Ok(claims) +} + +/// Verify and decode a signed credential, returning the claims if valid. +/// +/// Checks performed: +/// 1. Signature verification (HMAC-SHA256) +/// 2. Expiry verification +/// 3. Pod identity binding +/// +/// Revocation and attempt-active checks require external state (database) and +/// must be performed by the caller after this returns `Ok`. +pub fn verify_credential( + credential: &str, + secret: &[u8], + expected_pod_id: Uuid, +) -> Result { + let claims = verify_signature(credential, secret)?; + // Check pod binding if claims.pod_id != expected_pod_id { return Err(CredentialError::PodMismatch); From 7a139d855f638e7581a5d44ae38e6555872d1169 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 12 Aug 2026 14:21:20 +0200 Subject: [PATCH 3/3] Hollidays --- src/data_retention.rs | 21 +- src/errors.rs | 70 ++++ src/lib.rs | 25 +- src/principal.rs | 918 +++++++++++++++++++++++++++++++++++++++++- src/tools.rs | 638 ++++++++++++++++++++++++++++- 5 files changed, 1653 insertions(+), 19 deletions(-) diff --git a/src/data_retention.rs b/src/data_retention.rs index 795d05e..6c99f27 100644 --- a/src/data_retention.rs +++ b/src/data_retention.rs @@ -143,13 +143,9 @@ impl RetentionPolicy { /// Returns the retention period in days for a given data class. pub fn retention_days(&self, class: DataClass) -> u32 { match class { - DataClass::SourceCode | DataClass::Documentation => { - self.documentation_retention_days - } + DataClass::SourceCode | DataClass::Documentation => self.documentation_retention_days, DataClass::Prompt | DataClass::ModelOutput => 7, - DataClass::ToolArguments | DataClass::ToolResult => { - self.execution_log_retention_days - } + DataClass::ToolArguments | DataClass::ToolResult => self.execution_log_retention_days, DataClass::ApprovalData | DataClass::AgentRun => self.run_retention_days, DataClass::Credentials => 0, // never retained DataClass::MutationContent => self.run_retention_days, @@ -258,8 +254,17 @@ fn redact_json_secrets(value: &serde_json::Value) -> serde_json::Value { fn is_sensitive_json_field(name: &str) -> bool { matches!( name.to_lowercase().as_str(), - "password" | "secret" | "api_key" | "apikey" | "token" | "authorization" - | "private_key" | "credentials" | "env" | "environment" | "bearer" + "password" + | "secret" + | "api_key" + | "apikey" + | "token" + | "authorization" + | "private_key" + | "credentials" + | "env" + | "environment" + | "bearer" ) } diff --git a/src/errors.rs b/src/errors.rs index 589b04c..7558fd1 100644 --- a/src/errors.rs +++ b/src/errors.rs @@ -1,4 +1,74 @@ +use serde::Serialize; use thiserror::Error; +use uuid::Uuid; + +// --------------------------------------------------------------------------- +// Public error envelope (Section 21) +// --------------------------------------------------------------------------- + +/// A structured error returned to API callers. Internal details are never +/// exposed; they are logged on the server with the same `correlation_id`. +#[derive(Debug, Clone, Serialize)] +pub struct PublicError { + /// Stable machine-readable code (e.g. `"unauthorized"`, `"not_found"`). + pub code: &'static str, + /// Human-safe message. No internal paths, SQL, tokens, or stack traces. + pub message: String, + /// Correlation ID that links client-facing error to server logs. + pub correlation_id: Uuid, + /// Whether retrying the same request may succeed. + pub retryable: bool, + /// Optional per-field validation errors for 400-class responses. + pub field_errors: Vec, +} + +/// A per-field validation error within a `PublicError`. +#[derive(Debug, Clone, Serialize)] +pub struct FieldError { + pub field: String, + pub code: &'static str, + pub message: String, +} + +/// A durable audit record that captures who did what to which resource. +#[derive(Debug, Clone, Serialize)] +pub struct AuditRecord { + pub id: Uuid, + pub correlation_id: Uuid, + pub principal_id: Uuid, + pub project_id: Option, + pub action: String, + pub resource: String, + pub outcome: AuditOutcome, + pub details: serde_json::Value, + pub created_at: chrono::DateTime, +} + +/// The outcome of an audited operation. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum AuditOutcome { + Success, + Denied, + Failure, +} + +/// Errors that can arise from the authorization and audit layer. +#[derive(Debug, Clone, Error)] +pub enum AuthorizationError { + #[error("authentication required")] + AuthenticationRequired, + #[error("not authorized: {0}")] + Denied(String), + #[error("authorization database error: {0}")] + DatabaseError(String), + #[error("project not found")] + ProjectNotFound, +} + +// --------------------------------------------------------------------------- +// Existing types (unchanged) +// --------------------------------------------------------------------------- #[derive(Debug, Clone, Error)] pub enum ValidationError { diff --git a/src/lib.rs b/src/lib.rs index c27f069..0e14a5b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -32,21 +32,30 @@ pub use data_retention::{ redact_secrets, truncate_bytes, DataClass, DataRetentionService, RetentionPolicy, }; pub use enums::*; -pub use errors::{ShoalError, ValidationError}; +pub use errors::{AuditOutcome, AuditRecord, FieldError, PublicError, ShoalError, ValidationError}; pub use krill::{KrillConfig, KrillDescriptor, KrillId, PodProtocolVersion}; -pub use mutations::{FileMutation, PathValidationError, merged_affected_files, normalize_repository_path}; -pub use principal::ExecutionPrincipal; +pub use mutations::{ + merged_affected_files, normalize_repository_path, FileMutation, PathValidationError, +}; +pub use principal::{ + AuthError, AuthenticatedPrincipal, AuthorizationService, DispatchAuthError, + DiscussionCredentialClaims, DiscussionExecutionPrincipal, ExecutionPrincipal, GlobalRole, + ModelGatewayClaims, ModelGatewayPrincipal, PodPrincipal, ProjectAction, ProjectRole, + RequestContext, ReservoirConnectionContext, ReservoirPrincipal, ReservoirServiceRole, + TokenValidationResult, +}; pub use project::{Project, ProjectFile, ProjectId, ProjectSettings}; pub use task::{Task, TaskResult}; pub use todo::{Dependency, ToDo}; pub use tools::{ compute_tools_hash, format_tool_error, format_tool_result, format_tools_json, is_global_tool, is_reef_proxy_tool, parse_tool_call_blocks, parse_tool_call_stream, tool_definitions, - tool_ids_for_agent_type, ApprovalRequirement, CompositionStep, ContextVisibility, - ExecutionContext, MarketplaceSearchResults, ParsedToolCall, TestAction, ToolComposition, - ToolDefinition, ToolDependency, ToolDocumentation, ToolExample, ToolExecutor, ToolListing, - ToolParser, ToolPlugin, ToolRegistry, ToolState, ToolTest, ToolTestResult, ToolTestSuite, - ToolTestSuiteResult, ToolVersion, GLOBAL_TOOLS, REEF_PROXY_TOOLS, + tool_ids_for_agent_type, apply_verified_scope, normalize_tool_call, resolve_effective_project, + ApprovalRequirement, CompositionStep, ContextVisibility, EffectiveProject, ExecutionContext, + MarketplaceSearchResults, ParsedToolCall, TestAction, ToolComposition, ToolDefinition, + ToolDependency, ToolDocumentation, ToolExample, ToolExecutor, ToolListing, ToolParser, + ToolPlugin, ToolRegistry, ToolState, ToolTest, ToolTestResult, ToolTestSuite, + ToolTestSuiteResult, ToolVersion, VerifiedToolScope, GLOBAL_TOOLS, REEF_PROXY_TOOLS, }; #[cfg(test)] diff --git a/src/principal.rs b/src/principal.rs index 8f38761..60064e5 100644 --- a/src/principal.rs +++ b/src/principal.rs @@ -1,9 +1,361 @@ +use std::collections::HashSet; + use chrono::{DateTime, Utc}; use serde::{Deserialize, Serialize}; use uuid::Uuid; use crate::agents::AgentType; +// --------------------------------------------------------------------------- +// Unified request principal and project RBAC (Section 9) +// --------------------------------------------------------------------------- + +/// Global user roles that apply across all projects. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash)] +#[serde(rename_all = "snake_case")] +pub enum GlobalRole { + User, + Admin, +} + +impl GlobalRole { + pub fn is_admin(&self) -> bool { + matches!(self, GlobalRole::Admin) + } +} + +/// Project-level roles that determine what a member can do within a project. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[serde(rename_all = "snake_case")] +pub enum ProjectRole { + Viewer, + Contributor, + Reviewer, + Operator, + Owner, +} + +impl ProjectRole { + /// Returns the minimum role required for the given action. + pub fn required_for(action: ProjectAction) -> Self { + match action { + ProjectAction::Read => ProjectRole::Viewer, + ProjectAction::CreateTodo => ProjectRole::Contributor, + ProjectAction::ModifyFiles => ProjectRole::Contributor, + ProjectAction::ReviewChanges => ProjectRole::Reviewer, + ProjectAction::OperateAgents => ProjectRole::Operator, + ProjectAction::ManageMembers => ProjectRole::Owner, + ProjectAction::DeleteProject => ProjectRole::Owner, + } + } +} + +/// Actions that can be performed on a project. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ProjectAction { + Read, + CreateTodo, + ModifyFiles, + ReviewChanges, + OperateAgents, + ManageMembers, + DeleteProject, +} + +/// An authenticated principal that has proven identity at the transport boundary. +/// +/// Every transport adapter (HTTP, MTP) must construct one of these variants +/// from authenticated connection state. Handlers and application services +/// receive this type instead of raw UUIDs. +#[derive(Debug, Clone)] +pub enum AuthenticatedPrincipal { + User { + user_id: Uuid, + global_role: GlobalRole, + }, + Pod(PodPrincipal), + Execution(ExecutionPrincipal), + Service { + service_id: Uuid, + }, + System { + component: &'static str, + }, +} + +impl AuthenticatedPrincipal { + pub fn user_id(&self) -> Option { + match self { + AuthenticatedPrincipal::User { user_id, .. } => Some(*user_id), + _ => None, + } + } + + pub fn pod_id(&self) -> Option { + match self { + AuthenticatedPrincipal::Pod(pod) => Some(pod.pod_id), + _ => None, + } + } + + pub fn is_admin(&self) -> bool { + match self { + AuthenticatedPrincipal::User { global_role, .. } => global_role.is_admin(), + _ => false, + } + } + + pub fn is_internal_service(&self) -> bool { + matches!( + self, + AuthenticatedPrincipal::Service { .. } | AuthenticatedPrincipal::System { .. } + ) + } + + pub fn id(&self) -> Uuid { + match self { + AuthenticatedPrincipal::User { user_id, .. } => *user_id, + AuthenticatedPrincipal::Pod(pod) => pod.pod_id, + AuthenticatedPrincipal::Execution(exec) => Uuid::from_u128(exec.run_id.as_u128()), + AuthenticatedPrincipal::Service { service_id } => *service_id, + AuthenticatedPrincipal::System { .. } => Uuid::nil(), + } + } + + /// Returns true if this principal can operate on the given project based + /// on the project role. Administrators bypass project-level checks. + pub fn can_operate_project(&self, project_role: Option) -> bool { + if self.is_admin() { + return true; + } + project_role + .map(|r| r >= ProjectRole::Operator) + .unwrap_or(false) + } +} + +/// A Pod identity established through cryptographic challenge-response. +#[derive(Debug, Clone)] +pub struct PodPrincipal { + pub pod_id: Uuid, + pub device_id: Uuid, + pub key_fingerprint: String, + pub approved_at: DateTime, +} + +/// A transport-level request context that carries the authenticated principal +/// and correlation metadata through the application layer. +#[derive(Debug, Clone)] +pub struct RequestContext { + pub principal: AuthenticatedPrincipal, + pub correlation_id: Uuid, + pub authenticated_at: DateTime, +} + +impl RequestContext { + pub fn new(principal: AuthenticatedPrincipal) -> Self { + Self { + principal, + correlation_id: Uuid::new_v4(), + authenticated_at: Utc::now(), + } + } + + pub fn with_correlation_id(principal: AuthenticatedPrincipal, correlation_id: Uuid) -> Self { + Self { + principal, + correlation_id, + authenticated_at: Utc::now(), + } + } +} + +// --------------------------------------------------------------------------- +// Reservoir principal types (Section 1) +// --------------------------------------------------------------------------- + +/// A principal that can authenticate to Reservoir. +#[derive(Debug, Clone)] +pub enum ReservoirPrincipal { + User { + user_id: Uuid, + global_role: GlobalRole, + }, + Pod { + pod_id: Uuid, + }, + Execution(ExecutionPrincipal), + Service { + service_id: Uuid, + role: ReservoirServiceRole, + }, +} + +/// Explicit infrastructure roles accepted by Reservoir. Adding a new service +/// does not implicitly grant it Reef's storage authority. +#[derive(Debug, Clone, Copy, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "snake_case")] +pub enum ReservoirServiceRole { + ReefStorage, +} + +/// Authoritative result returned when Reef validates a user session token. +/// +/// Consumers must use `expires_at` as the upper bound for any derived +/// connection authentication rather than granting a new local lifetime. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] +pub struct TokenValidationResult { + pub valid: bool, + pub user_id: Option, + pub global_role: Option, + pub expires_at: Option>, + pub session_id: Option, +} + +/// Connection-level context for Reservoir MTP connections. +/// +/// Starts unauthenticated and is populated after an `AuthRequest` exchange. +/// The dispatcher must check `principal.is_some()` before accepting any +/// storage request. +#[derive(Debug, Clone)] +pub struct ReservoirConnectionContext { + pub principal: Option, + pub authenticated_until: Option>, + pub session_id: Option, + pub correlation_id: Uuid, + /// The user's access token, retained for remote role resolution. + access_token: Option, +} + +impl ReservoirConnectionContext { + pub fn new() -> Self { + Self { + principal: None, + authenticated_until: None, + session_id: None, + correlation_id: Uuid::new_v4(), + access_token: None, + } + } + + pub fn is_authenticated(&self) -> bool { + if let Some(expiry) = self.authenticated_until { + self.principal.is_some() && Utc::now() < expiry + } else { + false + } + } + + pub fn require_authenticated(&self) -> Result<&ReservoirPrincipal, AuthError> { + if self.is_authenticated() { + Ok(self.principal.as_ref().expect("checked above")) + } else { + Err(AuthError::AuthenticationRequired) + } + } + + /// Authenticate a connection until an authoritative absolute deadline. + pub fn authenticate_until( + &mut self, + principal: ReservoirPrincipal, + expires_at: DateTime, + session_id: Option, + ) { + self.principal = Some(principal); + self.authenticated_until = Some(expires_at); + self.session_id = session_id; + } + + /// Set the access token for remote role resolution. + pub fn set_access_token(&mut self, token: String) { + self.access_token = Some(token); + } + + /// Get the access token, if available. + pub fn access_token(&self) -> Option<&str> { + self.access_token.as_deref() + } +} + +// --------------------------------------------------------------------------- +// Authorization errors +// --------------------------------------------------------------------------- + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum AuthError { + AuthenticationRequired, + AuthorizationDenied(String), + Expired, + InvalidPrincipal, + /// Pod must complete challenge-response before accessing protected operations. + PodAuthenticationRequired, + /// A challenge-response handshake is required to complete Pod registration. + ChallengeRequired, +} + +impl std::fmt::Display for AuthError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + AuthError::AuthenticationRequired => write!(f, "authentication required"), + AuthError::AuthorizationDenied(msg) => write!(f, "authorization denied: {}", msg), + AuthError::Expired => write!(f, "credential expired"), + AuthError::InvalidPrincipal => write!(f, "invalid principal"), + AuthError::PodAuthenticationRequired => write!(f, "pod authentication required"), + AuthError::ChallengeRequired => write!(f, "challenge-response required"), + } + } +} + +impl std::error::Error for AuthError {} + +/// Errors for dispatch ownership checks (Section 5). +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum DispatchAuthError { + NotFound, + WrongPod, + LeaseExpired, + InvalidState, +} + +impl std::fmt::Display for DispatchAuthError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DispatchAuthError::NotFound => write!(f, "dispatch not found"), + DispatchAuthError::WrongPod => write!(f, "dispatch assigned to different pod"), + DispatchAuthError::LeaseExpired => write!(f, "dispatch lease expired"), + DispatchAuthError::InvalidState => write!(f, "dispatch in invalid state"), + } + } +} + +impl std::error::Error for DispatchAuthError {} + +// --------------------------------------------------------------------------- +// Authorization service trait (Section 9) +// --------------------------------------------------------------------------- + +/// A project authorization service. Implementations check whether an +/// authenticated principal has the required role for an action on a project. +/// +/// The same implementation must be used by HTTP and MTP handlers to ensure +/// consistent authorization behavior across transports. +pub trait AuthorizationService: Send + Sync { + /// Check whether `context.principal` may perform `action` on `project_id`. + /// + /// Returns the resolved project role on success, or an `AuthError` on + /// denial. Database errors must also map to denial (fail-closed). + fn authorize_project( + &self, + context: &RequestContext, + project_id: Uuid, + action: ProjectAction, + ) -> Result; + + /// Resolve the set of project IDs visible to the given principal. + fn visible_projects(&self, principal: &AuthenticatedPrincipal) -> HashSet; +} + /// Errors that can occur during credential verification. #[derive(Debug, Clone, PartialEq, Eq)] pub enum CredentialError { @@ -19,6 +371,8 @@ pub enum CredentialError { PodMismatch, /// The attempt is no longer active (completed, cancelled, or not found). AttemptInactive, + /// The requested tool is not in the credential's allow-list. + ToolNotAllowed, } impl std::fmt::Display for CredentialError { @@ -30,6 +384,7 @@ impl std::fmt::Display for CredentialError { Self::Revoked => write!(f, "credential revoked"), Self::PodMismatch => write!(f, "credential not valid for this pod"), Self::AttemptInactive => write!(f, "attempt inactive"), + Self::ToolNotAllowed => write!(f, "tool not in credential allow-list"), } } } @@ -221,7 +576,8 @@ pub fn sign_credential( use sha2::Sha256; let claims = principal.to_claims(); - let payload_json = serde_json::to_vec(&claims).map_err(|_| CredentialError::MalformedPayload)?; + let payload_json = + serde_json::to_vec(&claims).map_err(|_| CredentialError::MalformedPayload)?; let payload_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(&payload_json); let mut mac = @@ -322,10 +678,345 @@ impl From for ExecutionPrincipal { } } +// --------------------------------------------------------------------------- +// Discussion execution credentials (Task 4) +// --------------------------------------------------------------------------- + +/// The verifiable claims inside a signed discussion execution credential. +/// +/// Discussion credentials carry chat/project context instead of task/run context, +/// binding the discussion agent to a specific chat session and project scope. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DiscussionCredentialClaims { + /// The user who initiated the discussion. + pub user_id: Uuid, + /// The chat session this credential is scoped to. + pub discuss_id: Uuid, + /// The project this discussion can access (None for non-project chats). + pub project_id: Option, + /// The pod this credential is bound to. + pub pod_id: Uuid, + /// Tools this principal is authorized to invoke. + pub allowed_tools: Vec, + /// Absolute deadline after which this principal is invalid. + pub expires_at: DateTime, + /// When this credential was minted. + pub issued_at: DateTime, + /// Unique credential identifier for revocation tracking. + pub credential_id: Uuid, +} + +/// A cryptographically bounded execution identity for discussion agents. +/// +/// Minted by Reef when dispatching a discuss turn to a Pod, this principal +/// carries the chat/project scope needed for tool authorization without +/// reusing task-agent fields. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct DiscussionExecutionPrincipal { + pub user_id: Uuid, + pub discuss_id: Uuid, + pub project_id: Option, + pub pod_id: Uuid, + pub allowed_tools: Vec, + pub expires_at: DateTime, + pub issued_at: DateTime, + pub credential_id: Uuid, +} + +impl DiscussionExecutionPrincipal { + /// Mint a new discussion principal for a chat turn dispatch. + pub fn mint( + user_id: Uuid, + discuss_id: Uuid, + project_id: Option, + pod_id: Uuid, + allowed_tools: Vec, + timeout_secs: u64, + ) -> Self { + let now = Utc::now(); + Self { + user_id, + discuss_id, + project_id, + pod_id, + allowed_tools, + expires_at: now + chrono::Duration::seconds(timeout_secs as i64), + issued_at: now, + credential_id: Uuid::new_v4(), + } + } + + /// Convert into `DiscussionCredentialClaims` suitable for signing. + pub fn to_claims(&self) -> DiscussionCredentialClaims { + DiscussionCredentialClaims { + user_id: self.user_id, + discuss_id: self.discuss_id, + project_id: self.project_id, + pod_id: self.pod_id, + allowed_tools: self.allowed_tools.clone(), + expires_at: self.expires_at, + issued_at: self.issued_at, + credential_id: self.credential_id, + } + } + + /// Check whether this principal is still valid at the given timestamp. + pub fn is_valid_at(&self, now: DateTime) -> bool { + now <= self.expires_at + } +} + +/// Sign a discussion principal's claims with HMAC-SHA256. +pub fn sign_discussion_credential( + principal: &DiscussionExecutionPrincipal, + secret: &[u8], +) -> Result { + use base64::Engine; + use hmac::{Hmac, Mac}; + use sha2::Sha256; + + let claims = principal.to_claims(); + let payload_json = + serde_json::to_vec(&claims).map_err(|_| CredentialError::MalformedPayload)?; + let payload_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(&payload_json); + + let mut mac = + Hmac::::new_from_slice(secret).map_err(|_| CredentialError::MalformedPayload)?; + mac.update(payload_b64.as_bytes()); + let signature = mac.finalize().into_bytes(); + let sig_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(signature); + + Ok(format!("{}.{}", payload_b64, sig_b64)) +} + +/// Verify the cryptographic signature and expiry of a signed discussion credential. +/// +/// Returns the verified discussion claims on success. The caller must still +/// perform stateful authorization (chat existence, project access, revocation). +pub fn verify_discussion_signature( + credential: &str, + secret: &[u8], +) -> Result { + use base64::Engine; + use hmac::{Hmac, Mac}; + use sha2::Sha256; + + let (payload_b64, sig_b64) = + SignedCredential::parse(credential).ok_or(CredentialError::MalformedPayload)?; + + let payload_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(payload_b64) + .map_err(|_| CredentialError::MalformedPayload)?; + let claims: DiscussionCredentialClaims = + serde_json::from_slice(&payload_bytes).map_err(|_| CredentialError::MalformedPayload)?; + + let mut mac = + Hmac::::new_from_slice(secret).map_err(|_| CredentialError::MalformedPayload)?; + mac.update(payload_b64.as_bytes()); + let sig_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(sig_b64) + .map_err(|_| CredentialError::MalformedPayload)?; + mac.verify_slice(&sig_bytes) + .map_err(|_| CredentialError::InvalidSignature)?; + + if Utc::now() > claims.expires_at { + return Err(CredentialError::Expired); + } + + Ok(claims) +} + +// --------------------------------------------------------------------------- +// Model gateway credentials (Release Blockers 5, Task 2) +// --------------------------------------------------------------------------- + +/// The verifiable claims inside a signed model gateway credential. +/// +/// A model gateway credential is a short-lived, attempt-scoped capability that +/// authorizes a sandboxed connector-krill process to make model-provider API +/// calls through the Pod-local gateway. The credential binds to a specific +/// attempt, pod, and provider, preventing cross-attempt or cross-pod abuse. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ModelGatewayClaims { + /// Unique credential identifier for revocation tracking. + pub credential_id: Uuid, + /// The attempt this credential is scoped to. + pub attempt_id: Uuid, + /// The pod this credential is bound to. + pub pod_id: Uuid, + /// The model provider this credential grants access to (e.g. "openai", "anthropic"). + pub provider: String, + /// Models this credential is allowed to access. Empty means all models for the provider. + pub allowed_models: Vec, + /// Absolute deadline after which this credential is invalid. + pub expires_at: DateTime, + /// When this credential was minted. + pub issued_at: DateTime, +} + +/// A cryptographically bounded model-provider access identity. +/// +/// Minted by the Pod when dispatching an attempt, this principal carries the +/// attempt/pod/provider scope needed for model gateway authorization without +/// exposing the actual provider API key to the sandbox. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct ModelGatewayPrincipal { + pub attempt_id: Uuid, + pub pod_id: Uuid, + pub provider: String, + pub allowed_models: Vec, + pub expires_at: DateTime, + pub issued_at: DateTime, + pub credential_id: Uuid, +} + +impl ModelGatewayPrincipal { + /// Mint a new model gateway principal for an attempt. + pub fn mint( + attempt_id: Uuid, + pod_id: Uuid, + provider: String, + allowed_models: Vec, + timeout_secs: u64, + ) -> Self { + let now = Utc::now(); + Self { + attempt_id, + pod_id, + provider, + allowed_models, + expires_at: now + chrono::Duration::seconds(timeout_secs as i64), + issued_at: now, + credential_id: Uuid::new_v4(), + } + } + + /// Convert into `ModelGatewayClaims` suitable for signing. + pub fn to_claims(&self) -> ModelGatewayClaims { + ModelGatewayClaims { + credential_id: self.credential_id, + attempt_id: self.attempt_id, + pod_id: self.pod_id, + provider: self.provider.clone(), + allowed_models: self.allowed_models.clone(), + expires_at: self.expires_at, + issued_at: self.issued_at, + } + } + + /// Check whether this principal is still valid at the given timestamp. + pub fn is_valid_at(&self, now: DateTime) -> bool { + now <= self.expires_at + } +} + +/// Sign a model gateway principal's claims with HMAC-SHA256. +/// +/// The shared secret is held only by the Pod; connector-krill receives the +/// credential string and the Pod validates it at the gateway boundary. +pub fn sign_model_gateway_credential( + principal: &ModelGatewayPrincipal, + secret: &[u8], +) -> Result { + use base64::Engine; + use hmac::{Hmac, Mac}; + use sha2::Sha256; + + let claims = principal.to_claims(); + let payload_json = + serde_json::to_vec(&claims).map_err(|_| CredentialError::MalformedPayload)?; + let payload_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(&payload_json); + + let mut mac = + Hmac::::new_from_slice(secret).map_err(|_| CredentialError::MalformedPayload)?; + mac.update(payload_b64.as_bytes()); + let signature = mac.finalize().into_bytes(); + let sig_b64 = base64::engine::general_purpose::URL_SAFE_NO_PAD.encode(signature); + + Ok(format!("{}.{}", payload_b64, sig_b64)) +} + +/// Verify the cryptographic signature and expiry of a signed model gateway credential. +/// +/// Returns the verified claims on success. The caller must still perform +/// stateful authorization (attempt active, pod binding) after this returns `Ok`. +pub fn verify_model_gateway_signature( + credential: &str, + secret: &[u8], +) -> Result { + use base64::Engine; + use hmac::{Hmac, Mac}; + use sha2::Sha256; + + let (payload_b64, sig_b64) = + SignedCredential::parse(credential).ok_or(CredentialError::MalformedPayload)?; + + let payload_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(payload_b64) + .map_err(|_| CredentialError::MalformedPayload)?; + let claims: ModelGatewayClaims = + serde_json::from_slice(&payload_bytes).map_err(|_| CredentialError::MalformedPayload)?; + + let mut mac = + Hmac::::new_from_slice(secret).map_err(|_| CredentialError::MalformedPayload)?; + mac.update(payload_b64.as_bytes()); + let sig_bytes = base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(sig_b64) + .map_err(|_| CredentialError::MalformedPayload)?; + mac.verify_slice(&sig_bytes) + .map_err(|_| CredentialError::InvalidSignature)?; + + if Utc::now() > claims.expires_at { + return Err(CredentialError::Expired); + } + + Ok(claims) +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn reservoir_connection_uses_authoritative_expiry() { + let user_id = Uuid::new_v4(); + let session_id = Uuid::new_v4(); + let expires_at = Utc::now() + chrono::Duration::minutes(5); + let mut context = ReservoirConnectionContext::new(); + + context.authenticate_until( + ReservoirPrincipal::User { + user_id, + global_role: GlobalRole::User, + }, + expires_at, + Some(session_id), + ); + + assert!(context.is_authenticated()); + assert_eq!(context.authenticated_until, Some(expires_at)); + assert_eq!(context.session_id, Some(session_id)); + } + + #[test] + fn reservoir_connection_rejects_expired_session() { + let mut context = ReservoirConnectionContext::new(); + context.authenticate_until( + ReservoirPrincipal::User { + user_id: Uuid::new_v4(), + global_role: GlobalRole::User, + }, + Utc::now() - chrono::Duration::seconds(1), + Some(Uuid::new_v4()), + ); + + assert!(!context.is_authenticated()); + assert_eq!( + context.require_authenticated().unwrap_err(), + AuthError::AuthenticationRequired + ); + } + fn test_secret() -> Vec { b"test-secret-key-for-hmac-signing-operations".to_vec() } @@ -604,4 +1295,229 @@ mod tests { .decode(sig) .is_ok()); } + + // ── Discussion credential tests ───────────────────────────────── + + #[test] + fn test_mint_discussion_principal() { + let user_id = Uuid::new_v4(); + let discuss_id = Uuid::new_v4(); + let project_id = Some(Uuid::new_v4()); + let pod_id = Uuid::new_v4(); + + let principal = DiscussionExecutionPrincipal::mint( + user_id, + discuss_id, + project_id, + pod_id, + vec!["read_file".to_string(), "kanban_list_board".to_string()], + 300, + ); + + assert_eq!(principal.user_id, user_id); + assert_eq!(principal.discuss_id, discuss_id); + assert_eq!(principal.project_id, project_id); + assert_eq!(principal.pod_id, pod_id); + assert!(principal.is_valid_at(Utc::now())); + } + + #[test] + fn test_sign_and_verify_discussion_credential() { + let secret = test_secret(); + let user_id = Uuid::new_v4(); + let discuss_id = Uuid::new_v4(); + let project_id = Some(Uuid::new_v4()); + let pod_id = Uuid::new_v4(); + let principal = DiscussionExecutionPrincipal::mint( + user_id, + discuss_id, + project_id, + pod_id, + vec!["read_file".to_string()], + 300, + ); + + let credential = sign_discussion_credential(&principal, &secret).unwrap(); + let claims = verify_discussion_signature(&credential, &secret).unwrap(); + + assert_eq!(claims.user_id, user_id); + assert_eq!(claims.discuss_id, discuss_id); + assert_eq!(claims.project_id, project_id); + assert_eq!(claims.pod_id, pod_id); + assert_eq!(claims.allowed_tools, vec!["read_file".to_string()]); + } + + #[test] + fn test_discussion_credential_rejects_wrong_signature() { + let secret = test_secret(); + let wrong_secret = b"wrong-secret-key-for-hmac-signing-operations"; + let principal = DiscussionExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Some(Uuid::new_v4()), + Uuid::new_v4(), + vec![], + 300, + ); + + let credential = sign_discussion_credential(&principal, &secret).unwrap(); + let result = verify_discussion_signature(&credential, wrong_secret); + assert_eq!(result.unwrap_err(), CredentialError::InvalidSignature); + } + + #[test] + fn test_discussion_credential_rejects_expired() { + let secret = test_secret(); + let mut principal = DiscussionExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Some(Uuid::new_v4()), + Uuid::new_v4(), + vec![], + 300, + ); + // Force expiry + principal.expires_at = Utc::now() - chrono::Duration::hours(1); + + let credential = sign_discussion_credential(&principal, &secret).unwrap(); + let result = verify_discussion_signature(&credential, &secret); + assert_eq!(result.unwrap_err(), CredentialError::Expired); + } + + #[test] + fn test_discussion_credential_format_is_two_base64_parts() { + let secret = test_secret(); + let principal = DiscussionExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + None, + Uuid::new_v4(), + vec!["web_search".to_string()], + 300, + ); + + let credential = sign_discussion_credential(&principal, &secret).unwrap(); + let (payload, sig) = SignedCredential::parse(&credential).unwrap(); + assert!(!payload.is_empty()); + assert!(!sig.is_empty()); + use base64::Engine; + assert!(base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(payload) + .is_ok()); + assert!(base64::engine::general_purpose::URL_SAFE_NO_PAD + .decode(sig) + .is_ok()); + } + + #[test] + fn test_discussion_credential_no_project_id() { + let secret = test_secret(); + let principal = DiscussionExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + None, // no project + Uuid::new_v4(), + vec!["web_search".to_string()], + 300, + ); + + let credential = sign_discussion_credential(&principal, &secret).unwrap(); + let claims = verify_discussion_signature(&credential, &secret).unwrap(); + assert!(claims.project_id.is_none()); + } + + #[test] + fn test_task_credential_rejected_as_discussion() { + let secret = test_secret(); + let principal = ExecutionPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + Uuid::new_v4(), + 1, + Uuid::new_v4(), + AgentType::BuildAgent, + vec![], + 3600, + ); + + // A task credential should fail discussion verification because + // the JSON structure is different. + let credential = sign_credential(&principal, &secret).unwrap(); + let result = verify_discussion_signature(&credential, &secret); + assert!(result.is_err()); + } + + #[test] + fn test_model_gateway_credential_roundtrip() { + let secret = test_secret(); + let principal = ModelGatewayPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + "openai".to_string(), + vec!["gpt-4".to_string(), "gpt-3.5-turbo".to_string()], + 300, + ); + + let credential = sign_model_gateway_credential(&principal, &secret).unwrap(); + let claims = verify_model_gateway_signature(&credential, &secret).unwrap(); + assert_eq!(claims.credential_id, principal.credential_id); + assert_eq!(claims.attempt_id, principal.attempt_id); + assert_eq!(claims.pod_id, principal.pod_id); + assert_eq!(claims.provider, "openai"); + assert_eq!( + claims.allowed_models, + vec!["gpt-4".to_string(), "gpt-3.5-turbo".to_string()] + ); + } + + #[test] + fn test_model_gateway_credential_rejects_wrong_signature() { + let secret = test_secret(); + let wrong_secret = b"wrong-secret-key-for-hmac-signing-operations"; + let principal = ModelGatewayPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + "openai".to_string(), + vec![], + 300, + ); + + let credential = sign_model_gateway_credential(&principal, &secret).unwrap(); + let result = verify_model_gateway_signature(&credential, wrong_secret); + assert_eq!(result.unwrap_err(), CredentialError::InvalidSignature); + } + + #[test] + fn test_model_gateway_credential_rejects_expired() { + let secret = test_secret(); + let mut principal = ModelGatewayPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + "openai".to_string(), + vec![], + 300, + ); + // Force expiry + principal.expires_at = Utc::now() - chrono::Duration::hours(1); + + let credential = sign_model_gateway_credential(&principal, &secret).unwrap(); + let result = verify_model_gateway_signature(&credential, &secret); + assert_eq!(result.unwrap_err(), CredentialError::Expired); + } + + #[test] + fn test_model_gateway_credential_empty_models_allows_all() { + let secret = test_secret(); + let principal = ModelGatewayPrincipal::mint( + Uuid::new_v4(), + Uuid::new_v4(), + "anthropic".to_string(), + vec![], // empty = all models + 300, + ); + + let credential = sign_model_gateway_credential(&principal, &secret).unwrap(); + let claims = verify_model_gateway_signature(&credential, &secret).unwrap(); + assert!(claims.allowed_models.is_empty()); + } } diff --git a/src/tools.rs b/src/tools.rs index c62b2de..0fee8a7 100644 --- a/src/tools.rs +++ b/src/tools.rs @@ -1,5 +1,27 @@ use async_trait::async_trait; use serde::{Deserialize, Serialize}; +use uuid::Uuid; + +use crate::principal::ProjectAction; + +/// Authoritative execution ownership for every tool. +/// Each tool declares exactly one execution location. Connector-krill uses this +/// to route calls and must never fall back to local execution for Reef-owned tools. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ToolExecutionLocation { + /// Tool executes exclusively on Reef (the authorization boundary). + /// Connector must not locally execute after Reef rejection/failure. + Reef, + /// Tool executes locally on the connector (e.g. web_search, web_fetch). + Local, +} + +impl Default for ToolExecutionLocation { + fn default() -> Self { + Self::Local + } +} /* Context in which a tool should be available */ #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] @@ -64,6 +86,266 @@ impl ApprovalRequirement { } } +// --------------------------------------------------------------------------- +// Verified tool scope (Task 1) +// --------------------------------------------------------------------------- + +/// Authoritative scope derived from a validated execution credential. +/// Each field overrides any client- or model-supplied value for the +/// corresponding argument. +#[derive(Debug, Clone, Default)] +pub struct VerifiedToolScope { + pub project_id: Option, + pub run_id: Option, + pub attempt_id: Option, + pub todo_id: Option, + pub pod_id: Option, +} + +// --------------------------------------------------------------------------- +// Canonical scope argument names +// --------------------------------------------------------------------------- + +/// Canonical argument names for tool execution scope. These are the only +/// argument names that should be used for scope-related fields in tool +/// executors and approval records. +/// +/// * `project_id` — The project UUID this tool call is scoped to. +/// * `run_id` — The unique run identifier for task-agent execution. +/// * `agent_run_id` — Alias for `run_id`. Both are set to the same trusted +/// value by `apply_verified_scope` so executors using either alias receive +/// the verified scope. +/// * `attempt_id` — The attempt number (1-indexed) for task-agent execution. +/// * `todo_id` — The todo being serviced. +/// * `pod_id` — The pod this credential is bound to. +/// * `discuss_id` — The chat session this discussion credential is scoped to. +/// +/// When a credential provides a `run_id`, both `run_id` and `agent_run_id` +/// are normalized to the same value. Executors should prefer `run_id` for new +/// code, but existing executors using `agent_run_id` continue to work. + +/// Replace the corresponding tool arguments with the trusted values from a +/// validated credential. Client-supplied values are overwritten unconditionally +/// when the credential covers that field. +/// +/// Normalization includes all known aliases: when the credential provides a +/// `run_id`, both `run_id` and `agent_run_id` are set to the same trusted +/// value so that executors using either alias receive the verified scope. +pub fn apply_verified_scope( + args: &mut serde_json::Map, + scope: &VerifiedToolScope, +) { + if let Some(project_id) = scope.project_id { + args.insert( + "project_id".into(), + serde_json::json!(project_id.to_string()), + ); + } + + if let Some(run_id) = scope.run_id { + let value = serde_json::json!(run_id.to_string()); + // Normalize both aliases to the same trusted value. + args.insert("run_id".into(), value.clone()); + args.insert("agent_run_id".into(), value); + } + + if let Some(attempt_id) = scope.attempt_id { + args.insert("attempt_id".into(), serde_json::json!(attempt_id)); + } + + if let Some(todo_id) = scope.todo_id { + args.insert( + "todo_id".into(), + serde_json::json!(todo_id.to_string()), + ); + } + + if let Some(pod_id) = scope.pod_id { + args.insert( + "pod_id".into(), + serde_json::json!(pod_id.to_string()), + ); + } +} + +/// Normalize all credential-derived tool arguments immediately after caller +/// authentication and credential validation. This ensures that: +/// +/// 1. Verified scope (project_id, run_id, attempt_id, todo_id, pod_id) is +/// applied before any approval or authorization decision. +/// 2. All aliases (e.g., `run_id` and `agent_run_id`) are set to the same +/// trusted value. +/// 3. Approval records store normalized arguments that would execute. +/// +/// For task-agent calls with verified claims, this applies the full verified +/// scope. For discussion-agent calls, it applies the discussion scope. +/// For human calls without credentials, it resolves the effective project +/// from all client sources. +pub fn normalize_tool_call( + args: &mut serde_json::Map, + verified_claims: Option<&crate::principal::CredentialClaims>, + verified_discussion_claims: Option<&crate::principal::DiscussionCredentialClaims>, + payload_project: Option, + argument_project: Option, +) -> Result<(), String> { + if let Some(claims) = verified_claims { + // Task-agent call: apply full verified scope from credential. + apply_verified_scope( + args, + &VerifiedToolScope { + project_id: Some(claims.project_id), + run_id: Some(claims.run_id), + attempt_id: Some(claims.attempt_id), + todo_id: Some(claims.todo_id), + pod_id: Some(claims.pod_id), + }, + ); + } else if let Some(disc_claims) = verified_discussion_claims { + // Discussion-agent call: apply discussion scope from credential. + if let Some(project_id) = disc_claims.project_id { + args.insert( + "project_id".into(), + serde_json::json!(project_id.to_string()), + ); + } + args.insert( + "discuss_id".into(), + serde_json::json!(disc_claims.discuss_id.to_string()), + ); + args.insert( + "pod_id".into(), + serde_json::json!(disc_claims.pod_id.to_string()), + ); + } else { + // Human call without credential: resolve effective project from all + // client sources and reject contradictions. + let effective = resolve_effective_project(payload_project, argument_project, None); + match effective { + EffectiveProject::Resolved(pid) => { + args.insert("project_id".into(), serde_json::json!(pid.to_string())); + } + EffectiveProject::Conflicting { first, second } => { + return Err(format!( + "conflicting project_id between request envelope ({first}) and arguments ({second})" + )); + } + EffectiveProject::None => {} + } + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Effective project resolution (Task 2) +// --------------------------------------------------------------------------- + +/// Outcome of resolving a single authoritative project from multiple sources. +#[derive(Debug, Clone)] +pub enum EffectiveProject { + /// Exactly one project UUID was identified (from any source). + Resolved(Uuid), + /// No project source supplied — valid for non-project-scoped tools. + None, + /// Two or more distinct project UUIDs were supplied and conflict. + Conflicting { + first: Uuid, + second: Uuid, + }, +} + +/// Resolve a single effective project from the three main sources available +/// during tool routing: +/// +/// * `payload_project` — top-level `project_id` from the request envelope. +/// * `argument_project` — `project_id` supplied inside the `arguments` map. +/// * `derived_project` — project resolved from server-owned state (discussion, +/// entity lookup, etc.). +/// +/// The function distinguishes server-derived values (authoritative) from +/// client-supplied assertions. When a server-derived project exists it wins; +/// when only client values exist they must agree. +pub fn resolve_effective_project( + payload_project: Option, + argument_project: Option, + derived_project: Option, +) -> EffectiveProject { + // Collect all non-None sources. + let mut sources: Vec<(Uuid, bool)> = Vec::new(); + if let Some(p) = payload_project { + sources.push((p, false)); // client-supplied + } + if let Some(a) = argument_project { + sources.push((a, false)); // client-supplied + } + if let Some(d) = derived_project { + sources.push((d, true)); // server-derived + } + + if sources.is_empty() { + return EffectiveProject::None; + } + + // Prefer the server-derived value when present. + if let Some(&(derived_id, true)) = sources.iter().find(|(_, server)| *server) { + // Verify that all other sources agree with the derived value. + for &(other_id, _is_server) in &sources { + if other_id != derived_id { + return EffectiveProject::Conflicting { + first: derived_id, + second: other_id, + }; + } + } + return EffectiveProject::Resolved(derived_id); + } + + // No server-derived value: all client sources must agree. + let first = sources[0].0; + for &(other_id, _) in &sources[1..] { + if other_id != first { + return EffectiveProject::Conflicting { + first, + second: other_id, + }; + } + } + EffectiveProject::Resolved(first) +} + +// --------------------------------------------------------------------------- +// Rate-limit identity (Task 8) +// --------------------------------------------------------------------------- + +/// Specific execution identity used for tool rate limiting. +/// +/// Each variant derives from trusted server state (validated session, +/// verified credential, etc.) rather than from client-supplied values. +/// This ensures different execution contexts get independent rate-limit +/// buckets and prevents unrelated callers from sharing a quota. +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub enum ToolRateLimitIdentity { + /// Authenticated human user session. + UserSession(Uuid), + /// Task-agent execution attempt (from verified execution credential). + TaskAttempt(Uuid), + /// Discussion-agent session (from verified discussion credential). + Discussion(Uuid), + /// Trusted internal service call. + Service(String), +} + +impl ToolRateLimitIdentity { + /// Generate a rate-limit bucket key from this identity. + pub fn key(&self) -> String { + match self { + Self::UserSession(id) => format!("session:{id}"), + Self::TaskAttempt(id) => format!("attempt:{id}"), + Self::Discussion(id) => format!("discussion:{id}"), + Self::Service(id) => format!("service:{id}"), + } + } +} + #[derive(Debug, Clone, Serialize, Deserialize)] pub struct ToolDefinition { #[serde(default)] @@ -82,6 +364,11 @@ pub struct ToolDefinition { Project-scoped tools are filtered out when no project is selected. */ #[serde(default)] pub project_scoped: bool, + /* The project-level authorization action required to invoke this tool. + Must be `Some` for every project-scoped tool. A `None` value for a + project-scoped tool causes authorization to fail closed. */ + #[serde(default)] + pub required_project_action: Option, /* Maximum execution time in milliseconds. Defaults to 30000 (30s). */ #[serde(default = "default_timeout_ms")] pub timeout_ms: u64, @@ -102,6 +389,12 @@ pub struct ToolDefinition { 1+ = only visible to agents with max_depth >= this value. */ #[serde(default)] pub required_depth: u32, + /* Authoritative execution ownership. Every privileged or project-changing + tool declares Reef; tools that execute only locally declare Local. + Connector-krill must never locally execute a Reef-owned tool after + any Reef failure (rejection, auth failure, timeout, connection error). */ + #[serde(default)] + pub execution_location: ToolExecutionLocation, } const fn default_timeout_ms() -> u64 { @@ -119,12 +412,14 @@ impl Default for ToolDefinition { approval_requirement: ApprovalRequirement::None, version: String::new(), project_scoped: false, + required_project_action: None, timeout_ms: default_timeout_ms(), deprecated: false, deprecation_message: String::new(), execution_context: ExecutionContext::default(), context_visibility: ContextVisibility::default(), required_depth: 0, + execution_location: ToolExecutionLocation::default(), } } } @@ -671,12 +966,14 @@ macro_rules! tool { approval_requirement: ApprovalRequirement::from_flags($dangerous, $approval), version: "1.0.0".to_string(), project_scoped: $scoped, + required_project_action: None, timeout_ms: default_timeout_ms(), deprecated: false, deprecation_message: String::new(), execution_context: ExecutionContext::default(), context_visibility: ContextVisibility::default(), required_depth: 0, + execution_location: ToolExecutionLocation::default(), } }; ($name:expr, $desc:expr, $cat:expr, $dangerous:expr, $approval:expr, $scoped:expr, $params:expr, $timeout:expr) => { @@ -689,12 +986,14 @@ macro_rules! tool { approval_requirement: ApprovalRequirement::from_flags($dangerous, $approval), version: "1.0.0".to_string(), project_scoped: $scoped, + required_project_action: None, timeout_ms: $timeout, deprecated: false, deprecation_message: String::new(), execution_context: ExecutionContext::default(), context_visibility: ContextVisibility::default(), required_depth: 0, + execution_location: ToolExecutionLocation::default(), } }; } @@ -907,7 +1206,11 @@ when Reef is unreachable the local fallback executes instead. This is the Krill → Reef proxy set, not the full set of Reef-only tools. Tools like kanban_*, document_*, and strategic items are always executed on -Reef via MTP and are NOT in this list — they have no local implementation. */ +Reef via MTP and are NOT in this list — they have no local implementation. + +DEPRECATED: Use `ToolDefinition::execution_location` instead. This list is +retained only for backward compatibility and will be removed in a future release. */ +#[deprecated(note = "Use ToolDefinition::execution_location instead")] pub const REEF_PROXY_TOOLS: &[&str] = &[ "read_file", "file_read", @@ -932,7 +1235,10 @@ pub fn is_global_tool(name: &str) -> bool { GLOBAL_TOOLS.contains(&name) } -/* Returns true if the tool name should proxy to Reef when Reef is available. */ +/* Returns true if the tool name should proxy to Reef when Reef is available. + +DEPRECATED: Use `ToolDefinition::execution_location` instead. */ +#[deprecated(note = "Use ToolDefinition::execution_location instead")] pub fn is_reef_proxy_tool(name: &str) -> bool { REEF_PROXY_TOOLS.contains(&name) } @@ -1932,6 +2238,62 @@ pub fn tool_definitions() -> Vec { } } + /* Assign required_project_action for every project-scoped tool. + This replaces the old mutates()-based authorization with explicit + per-tool metadata. A project-scoped tool without an action causes + authorization to fail closed. */ + let action_map: std::collections::HashMap<&str, ProjectAction> = [ + // Kanban read + ("kanban_list_board", ProjectAction::Read), + // Kanban todo mutations + ("kanban_create_todo", ProjectAction::CreateTodo), + ("kanban_update_todo", ProjectAction::ModifyFiles), + ("kanban_delete_todo", ProjectAction::ModifyFiles), + ("kanban_move_todo", ProjectAction::ModifyFiles), + // Kanban sub-task mutations + ("kanban_create_task", ProjectAction::ModifyFiles), + ("kanban_update_task", ProjectAction::ModifyFiles), + ("kanban_add_task", ProjectAction::ModifyFiles), + ("kanban_complete_task", ProjectAction::ModifyFiles), + ("kanban_remove_task", ProjectAction::ModifyFiles), + // Kanban tag mutations + ("kanban_add_tag", ProjectAction::ModifyFiles), + // Documentation read + ("document_file", ProjectAction::Read), + ("document_project", ProjectAction::Read), + ("find_references", ProjectAction::Read), + ("file_dependencies", ProjectAction::Read), + ("documentation_tree", ProjectAction::Read), + ("verify_documentation", ProjectAction::Read), + ("document_folder", ProjectAction::Read), + ("get_documentation_context", ProjectAction::Read), + // Documentation write + ("store_file_doc", ProjectAction::ModifyFiles), + // File tools + ("list_files", ProjectAction::Read), + // Strategic tools (read-only analysis) + ("propose_strategic_item", ProjectAction::CreateTodo), + ("split_task", ProjectAction::ModifyFiles), + ("change_planner_mode", ProjectAction::Read), + ("audit_assumptions", ProjectAction::Read), + ("identify_blind_spots", ProjectAction::Read), + ("check_dependencies", ProjectAction::Read), + ("evaluate_plan_risk", ProjectAction::Read), + ("compare_project_patterns", ProjectAction::Read), + ("find_similar_risks", ProjectAction::Read), + ] + .iter() + .cloned() + .collect(); + + for tool in tools.iter_mut() { + if tool.project_scoped { + if let Some(&action) = action_map.get(tool.id.as_str()) { + tool.required_project_action = Some(action); + } + } + } + /* Assign required_depth per tool. Depth 0 = visible to all agents. Depth 1 = needs one planning cycle (bash, write/edits, build tools). Depth 2 = needs moderate depth (kanban mutation, workspace creation). @@ -2010,6 +2372,87 @@ pub fn tool_definitions() -> Vec { } } + /* Assign execution_location for every tool. + Reef-owned tools must never fall back to local execution. This metadata + replaces the old REEF_PROXY_TOOLS list and name-prefix routing. */ + let reef_tools: std::collections::HashSet<&str> = [ + // Filesystem (Reef-backed via Reservoir) + "read_file", + "file_read", + "write_file", + "file_write", + "edit_file", + "list_directory", + "search_files", + "grep", + "list_files", + // Execution (Reef-backed) + "bash", + "execute", + "delete_workspace", + // Git (Reef-backed) + "git_status", + "git_diff", + "git_log", + "git_branch", + // Build (Reef-backed) + "cargo_check", + "npm_build", + "python_check", + "test_runner", + // Workspace (Reef-backed) + "create_workspace", + "create_venv", + "install_dependencies", + "workspace_info", + // Kanban (always Reef — no valid local fallback) + "kanban_list_board", + "kanban_create_todo", + "kanban_update_todo", + "kanban_delete_todo", + "kanban_move_todo", + "kanban_create_task", + "kanban_update_task", + "kanban_add_task", + "kanban_complete_task", + "kanban_remove_task", + "kanban_add_tag", + // Documentation (Reef-backed) + "document_file", + "document_project", + "find_references", + "file_dependencies", + "documentation_tree", + "verify_documentation", + "document_folder", + "store_file_doc", + "get_documentation_context", + // Strategic (Reef-backed) + "propose_strategic_item", + "split_task", + "change_planner_mode", + "audit_assumptions", + "identify_blind_spots", + "check_dependencies", + "evaluate_plan_risk", + "compare_project_patterns", + "find_similar_risks", + // Agent (Reef-backed) + "report_completion", + "submit_batch_plan", + ] + .iter() + .copied() + .collect(); + + for tool in tools.iter_mut() { + if reef_tools.contains(tool.id.as_str()) { + tool.execution_location = ToolExecutionLocation::Reef; + } else { + tool.execution_location = ToolExecutionLocation::Local; + } + } + tools } @@ -2854,4 +3297,195 @@ Done with tools. "tool-result: without closing fence should be rejected by finish()" ); } + + #[test] + fn every_project_scoped_tool_has_required_project_action() { + let tools = tool_definitions(); + let mut violations = Vec::new(); + for tool in &tools { + if tool.project_scoped && tool.required_project_action.is_none() { + violations.push(tool.id.clone()); + } + } + assert!( + violations.is_empty(), + "project-scoped tools missing required_project_action: {:?}", + violations + ); + } + + #[test] + fn non_project_scoped_tools_have_no_required_project_action() { + let tools = tool_definitions(); + let mut violations = Vec::new(); + for tool in &tools { + if !tool.project_scoped && tool.required_project_action.is_some() { + violations.push(tool.id.clone()); + } + } + assert!( + violations.is_empty(), + "non-project-scoped tools with required_project_action set: {:?}", + violations + ); + } + + #[test] + fn every_tool_has_declared_execution_location() { + let tools = tool_definitions(); + for tool in &tools { + // Every tool must have either Reef or Local — the enum guarantees this, + // but we verify the field is present and meaningful. + assert!( + tool.execution_location == ToolExecutionLocation::Reef + || tool.execution_location == ToolExecutionLocation::Local, + "tool '{}' has unclassified execution_location", + tool.id + ); + } + } + + #[test] + fn reef_owned_mutation_tools_are_classified() { + let tools = tool_definitions(); + let reef_tools: Vec<&str> = tools + .iter() + .filter(|t| t.execution_location == ToolExecutionLocation::Reef) + .map(|t| t.id.as_str()) + .collect(); + // Privileged mutation tools must be Reef-owned + for name in &["write_file", "edit_file", "bash", "delete_workspace"] { + assert!( + reef_tools.contains(name), + "privileged tool '{}' must be Reef-owned", + name + ); + } + } + + #[test] + fn global_network_tools_are_local() { + let tools = tool_definitions(); + for name in &["web_search", "web_fetch", "web_api"] { + let tool = tools.iter().find(|t| t.id == *name).unwrap(); + assert_eq!( + tool.execution_location, + ToolExecutionLocation::Local, + "global tool '{}' must be Local", + name + ); + } + } + + #[test] + fn no_tool_is_both_reef_and_local() { + let tools = tool_definitions(); + for tool in &tools { + // The enum guarantees exactly one variant, but we verify + // the field is consistently set. + assert!( + tool.execution_location == ToolExecutionLocation::Reef + || tool.execution_location == ToolExecutionLocation::Local, + "tool '{}' has invalid execution_location", + tool.id + ); + } + } + + #[test] + fn all_kanban_tools_are_reef_owned() { + let tools = tool_definitions(); + for tool in &tools { + if tool.id.starts_with("kanban_") { + assert_eq!( + tool.execution_location, + ToolExecutionLocation::Reef, + "kanban tool '{}' must be Reef-owned", + tool.id + ); + } + } + } + + #[test] + fn all_strategic_tools_are_reef_owned() { + let tools = tool_definitions(); + for tool in &tools { + if tool.category == "strategic" { + assert_eq!( + tool.execution_location, + ToolExecutionLocation::Reef, + "strategic tool '{}' must be Reef-owned", + tool.id + ); + } + } + } + + #[test] + fn all_documentation_tools_are_reef_owned() { + let tools = tool_definitions(); + for tool in &tools { + if tool.category == "documentation" { + assert_eq!( + tool.execution_location, + ToolExecutionLocation::Reef, + "documentation tool '{}' must be Reef-owned", + tool.id + ); + } + } + } + + #[test] + fn apply_verified_scope_normalizes_agent_run_id() { + let run_id = Uuid::new_v4(); + let scope = VerifiedToolScope { + run_id: Some(run_id), + ..Default::default() + }; + let mut args = serde_json::Map::new(); + // Client supplies a conflicting agent_run_id + args.insert( + "agent_run_id".into(), + serde_json::json!("spoofed-value"), + ); + args.insert( + "run_id".into(), + serde_json::json!("spoofed-run"), + ); + + apply_verified_scope(&mut args, &scope); + + // Both aliases must be set to the trusted value + assert_eq!( + args.get("run_id").and_then(|v| v.as_str()), + Some(run_id.to_string().as_str()) + ); + assert_eq!( + args.get("agent_run_id").and_then(|v| v.as_str()), + Some(run_id.to_string().as_str()) + ); + } + + #[test] + fn apply_verified_scope_overwrites_conflicting_project_id() { + let project_id = Uuid::new_v4(); + let scope = VerifiedToolScope { + project_id: Some(project_id), + ..Default::default() + }; + let mut args = serde_json::Map::new(); + args.insert( + "project_id".into(), + serde_json::json!("spoofed-project"), + ); + + apply_verified_scope(&mut args, &scope); + + assert_eq!( + args.get("project_id").and_then(|v| v.as_str()), + Some(project_id.to_string().as_str()) + ); + } }