From c553e68595857190aed43af20aae9a08e5a7c672 Mon Sep 17 00:00:00 2001 From: Tushar Date: Sun, 5 Apr 2026 15:24:29 +0530 Subject: [PATCH 1/2] feat(permissions): add yes/no confirm prompt for policy decisions --- crates/forge_app/src/infra.rs | 6 + crates/forge_infra/src/forge_infra.rs | 4 + crates/forge_infra/src/inquire.rs | 6 + crates/forge_repo/src/forge_repo.rs | 4 + crates/forge_services/src/policy.rs | 319 +------------------------- 5 files changed, 29 insertions(+), 310 deletions(-) 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..6dc35af2b4 100644 --- a/crates/forge_services/src/policy.rs +++ b/crates/forge_services/src/policy.rs @@ -4,28 +4,12 @@ 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, + 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 +41,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 +55,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 +118,28 @@ 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?") + format!("{message}. Allow?") } PermissionOperation::Execute { .. } => { - "How would you like to proceed?".to_string() + "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); - } -} From 32b8190823e7c3bbb307f2658b543721b970320d Mon Sep 17 00:00:00 2001 From: "autofix-ci[bot]" <114827586+autofix-ci[bot]@users.noreply.github.com> Date: Sun, 5 Apr 2026 09:56:22 +0000 Subject: [PATCH 2/2] [autofix.ci] apply automated fixes --- crates/forge_services/src/policy.rs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/crates/forge_services/src/policy.rs b/crates/forge_services/src/policy.rs index 6dc35af2b4..498c1082ec 100644 --- a/crates/forge_services/src/policy.rs +++ b/crates/forge_services/src/policy.rs @@ -3,9 +3,7 @@ use std::sync::{Arc, LazyLock}; use anyhow::Context; use bytes::Bytes; -use forge_app::domain::{ - Permission, PermissionOperation, PolicyConfig, PolicyEngine, -}; +use forge_app::domain::{Permission, PermissionOperation, PolicyConfig, PolicyEngine}; use forge_app::{ DirectoryReaderInfra, EnvironmentInfra, FileInfoInfra, FileReaderInfra, FileWriterInfra, PolicyDecision, PolicyService, UserInfra, @@ -126,9 +124,7 @@ where PermissionOperation::Write { message, .. } => { format!("{message}. Allow?") } - PermissionOperation::Execute { .. } => { - "Allow this operation?".to_string() - } + PermissionOperation::Execute { .. } => "Allow this operation?".to_string(), PermissionOperation::Fetch { message, .. } => { format!("{message}. Allow?") } @@ -142,4 +138,3 @@ where } } } -