diff --git a/crates/forge_app/src/infra.rs b/crates/forge_app/src/infra.rs index 8c1c567772..64fb3c2d16 100644 --- a/crates/forge_app/src/infra.rs +++ b/crates/forge_app/src/infra.rs @@ -168,6 +168,12 @@ pub trait UserInfra: Send + Sync { /// Returns None if the user interrupts the prompt async fn prompt_question(&self, question: &str) -> anyhow::Result>; + /// Prompts the user with a yes/no confirmation. + /// + /// Returns `Some(true)` if confirmed, `Some(false)` if denied, or `None` + /// if the user cancels. + async fn confirm(&self, message: &str) -> anyhow::Result>; + /// Prompts the user to select a single option from a list /// Returns None if the user interrupts the selection async fn select_one( diff --git a/crates/forge_infra/src/forge_infra.rs b/crates/forge_infra/src/forge_infra.rs index 2ae84ab33f..4fc6450cb4 100644 --- a/crates/forge_infra/src/forge_infra.rs +++ b/crates/forge_infra/src/forge_infra.rs @@ -235,6 +235,10 @@ impl UserInfra for ForgeInfra { self.inquire_service.prompt_question(question).await } + async fn confirm(&self, message: &str) -> anyhow::Result> { + self.inquire_service.confirm(message).await + } + async fn select_one( &self, message: &str, diff --git a/crates/forge_infra/src/inquire.rs b/crates/forge_infra/src/inquire.rs index f31b8f2a2f..6b08f6901c 100644 --- a/crates/forge_infra/src/inquire.rs +++ b/crates/forge_infra/src/inquire.rs @@ -32,6 +32,12 @@ impl UserInfra for ForgeInquire { .await } + async fn confirm(&self, message: &str) -> Result> { + let message = message.to_string(); + self.prompt(move || ForgeWidget::confirm(&message).with_default(true).prompt()) + .await + } + async fn select_one( &self, message: &str, diff --git a/crates/forge_repo/src/forge_repo.rs b/crates/forge_repo/src/forge_repo.rs index ce203f6c4b..5b874a5f34 100644 --- a/crates/forge_repo/src/forge_repo.rs +++ b/crates/forge_repo/src/forge_repo.rs @@ -391,6 +391,10 @@ where self.infra.prompt_question(question).await } + async fn confirm(&self, message: &str) -> anyhow::Result> { + self.infra.confirm(message).await + } + async fn select_one( &self, message: &str, diff --git a/crates/forge_services/src/policy.rs b/crates/forge_services/src/policy.rs index 97621bc04b..498c1082ec 100644 --- a/crates/forge_services/src/policy.rs +++ b/crates/forge_services/src/policy.rs @@ -3,29 +3,11 @@ use std::sync::{Arc, LazyLock}; use anyhow::Context; use bytes::Bytes; -use forge_app::domain::{ - ExecuteRule, Fetch, Permission, PermissionOperation, Policy, PolicyConfig, PolicyEngine, - ReadRule, Rule, WriteRule, -}; +use forge_app::domain::{Permission, PermissionOperation, PolicyConfig, PolicyEngine}; use forge_app::{ DirectoryReaderInfra, EnvironmentInfra, FileInfoInfra, FileReaderInfra, FileWriterInfra, PolicyDecision, PolicyService, UserInfra, }; -use strum_macros::{Display, EnumIter}; - -/// User response for permission confirmation requests -#[derive(Debug, Clone, PartialEq, Eq, Display, EnumIter, strum_macros::EnumString)] -pub enum PolicyPermission { - /// Accept the operation - #[strum(to_string = "Accept")] - Accept, - /// Reject the operation - #[strum(to_string = "Reject")] - Reject, - /// Accept the operation and remember this choice for similar operations - #[strum(to_string = "Accept and Remember")] - AcceptAndRemember, -} #[derive(Clone)] pub struct ForgePolicyService { @@ -57,23 +39,6 @@ where DEFAULT_POLICIES.clone() } - /// Add a policy for a specific operation type - async fn add_policy_for_operation( - &self, - operation: &PermissionOperation, - ) -> anyhow::Result> - where - I: UserInfra, - { - if let Some(new_policy) = create_policy_for_operation(operation, None) { - // TODO: Can return a diff later - self.modify_policy(new_policy).await?; - Ok(Some(self.permissions_path())) - } else { - Ok(None) - } - } - /// Load all policy definitions from the forge/policies directory async fn read_policies(&self) -> anyhow::Result> { let policies_path = self.permissions_path(); @@ -88,26 +53,6 @@ where Ok(Some(policies)) } - /// Add or modify a policy in the policies file - async fn modify_policy(&self, policy: Policy) -> anyhow::Result<()> { - let policies_path = self.permissions_path(); - let mut policies = self.read_policies().await?.unwrap_or_default(); - - // Add the new policy to the collection - policies = policies.add_policy(policy); - - // Serialize the updated policies to YAML - let new_content = serde_yml::to_string(&policies) - .with_context(|| "Failed to serialize policies to YAML")?; - - // Write the updated content - self.infra - .write(&policies_path, Bytes::from(new_content.to_owned())) - .await?; - - Ok(()) - } - /// Create a default policies file if it does not exist async fn init_policies(&self) -> anyhow::Result<()> { let policies_path = self.permissions_path(); @@ -171,276 +116,25 @@ where Permission::Deny => Ok(PolicyDecision { allowed: false, path }), Permission::Allow => Ok(PolicyDecision { allowed: true, path }), Permission::Confirm => { - // Request user confirmation using UserInfra + // Request user confirmation using the confirm widget let confirmation_msg = match operation { PermissionOperation::Read { message, .. } => { - format!("{message}. How would you like to proceed?") + format!("{message}. Allow?") } PermissionOperation::Write { message, .. } => { - format!("{message}. How would you like to proceed?") - } - PermissionOperation::Execute { .. } => { - "How would you like to proceed?".to_string() + format!("{message}. Allow?") } + PermissionOperation::Execute { .. } => "Allow this operation?".to_string(), PermissionOperation::Fetch { message, .. } => { - format!("{message}. How would you like to proceed?") + format!("{message}. Allow?") } }; - match self - .infra - .select_one_enum::(&confirmation_msg) - .await? - { - Some(PolicyPermission::Accept) => Ok(PolicyDecision { allowed: true, path }), - Some(PolicyPermission::AcceptAndRemember) => { - let update_path = self.add_policy_for_operation(operation).await?; - Ok(PolicyDecision { allowed: true, path: update_path.or(path) }) - } - Some(PolicyPermission::Reject) | None => { - Ok(PolicyDecision { allowed: false, path }) - } + match self.infra.confirm(&confirmation_msg).await? { + Some(true) => Ok(PolicyDecision { allowed: true, path }), + Some(false) | None => Ok(PolicyDecision { allowed: false, path }), } } } } } - -/// Create a policy for an operation based on its type -fn create_policy_for_operation( - operation: &PermissionOperation, - dir: Option, -) -> Option { - fn create_file_policy( - path: &std::path::Path, - rule_constructor: fn(String) -> Rule, - ) -> Option { - path.extension() - .and_then(|ext| ext.to_str()) - .map(|extension| Policy::Simple { - permission: Permission::Allow, - rule: rule_constructor(format!("*.{extension}")), - }) - } - - match operation { - PermissionOperation::Read { path, cwd: _, message: _ } => { - create_file_policy(path, |pattern| { - Rule::Read(ReadRule { read: pattern, dir: None }) - }) - } - PermissionOperation::Write { path, cwd: _, message: _ } => { - create_file_policy(path, |pattern| { - Rule::Write(WriteRule { write: pattern, dir: None }) - }) - } - - PermissionOperation::Fetch { url, cwd: _, message: _ } => { - if let Ok(parsed_url) = url::Url::parse(url) { - parsed_url.host_str().map(|host| Policy::Simple { - permission: Permission::Allow, - rule: Rule::Fetch(Fetch { url: format!("{host}*"), dir: None }), - }) - } else { - Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Fetch(Fetch { url: url.to_string(), dir: None }), - }) - } - } - PermissionOperation::Execute { command, cwd: _ } => { - let parts: Vec<&str> = command.split_whitespace().collect(); - match parts.as_slice() { - [] => None, - [cmd] => Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Execute(ExecuteRule { command: format!("{cmd}*"), dir }), - }), - [cmd, subcmd, ..] => Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Execute(ExecuteRule { command: format!("{cmd} {subcmd}*"), dir }), - }), - } - } - } -} - -#[cfg(test)] -mod tests { - use pretty_assertions::assert_eq; - - use super::*; - - #[test] - fn test_create_policy_for_read_operation() { - let path = PathBuf::from("/path/to/file.rs"); - let operation = PermissionOperation::Read { - path, - cwd: std::path::PathBuf::from("/test/cwd"), - message: "Read file: /path/to/file.rs".to_string(), - }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Read(ReadRule { read: "*.rs".to_string(), dir: None }), - }); - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_write_operation() { - let path = PathBuf::from("/path/to/file.json"); - let operation = PermissionOperation::Write { - path, - cwd: std::path::PathBuf::from("/test/cwd"), - message: "Create/overwrite file: /path/to/file.json".to_string(), - }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Write(WriteRule { write: "*.json".to_string(), dir: None }), - }); - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_write_patch_operation() { - let path = PathBuf::from("/path/to/file.toml"); - let operation = PermissionOperation::Write { - path, - cwd: std::path::PathBuf::from("/test/cwd"), - message: "Modify file: /path/to/file.toml".to_string(), - }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Write(WriteRule { write: "*.toml".to_string(), dir: None }), - }); - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_net_fetch_operation() { - let url = "https://example.com/api/data".to_string(); - let operation = PermissionOperation::Fetch { - url, - cwd: std::path::PathBuf::from("/test/cwd"), - message: "Fetch content from URL: https://example.com/api/data".to_string(), - }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Fetch(Fetch { url: "example.com*".to_string(), dir: None }), - }); - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_execute_operation_with_subcommand() { - let command = "git push origin main".to_string(); - let operation = - PermissionOperation::Execute { command, cwd: std::path::PathBuf::from("/test/cwd") }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Execute(ExecuteRule { command: "git push*".to_string(), dir: None }), - }); - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_execute_operation_single_command() { - let command = "ls".to_string(); - let operation = - PermissionOperation::Execute { command, cwd: std::path::PathBuf::from("/test/cwd") }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Execute(ExecuteRule { command: "ls*".to_string(), dir: None }), - }); - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_file_without_extension() { - let path = PathBuf::from("/path/to/file"); - let operation = PermissionOperation::Read { - path, - cwd: std::path::PathBuf::from("/test/cwd"), - message: "Read file: /path/to/file".to_string(), - }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = None; - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_invalid_url() { - let url = "not-a-valid-url".to_string(); - let operation = PermissionOperation::Fetch { - url, - cwd: std::path::PathBuf::from("/test/cwd"), - message: "Fetch content from URL: not-a-valid-url".to_string(), - }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Fetch(Fetch { url: "not-a-valid-url".to_string(), dir: None }), - }); - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_empty_execute_command() { - let command = "".to_string(); - let operation = - PermissionOperation::Execute { command, cwd: std::path::PathBuf::from("/test/cwd") }; - - let actual = create_policy_for_operation(&operation, None); - - let expected = None; - - assert_eq!(actual, expected); - } - - #[test] - fn test_create_policy_for_execute_operation_with_working_directory() { - let command = "ls".to_string(); - let operation = - PermissionOperation::Execute { command, cwd: std::path::PathBuf::from("/test/cwd") }; - let working_directory = Some("/home/user/project".to_string()); - - let actual = create_policy_for_operation(&operation, working_directory.clone()); - - let expected = Some(Policy::Simple { - permission: Permission::Allow, - rule: Rule::Execute(ExecuteRule { command: "ls*".to_string(), dir: working_directory }), - }); - - assert_eq!(actual, expected); - } -}