Repository: denoland/deno
Pull request: feat(unstable): permission broker - support custom error messages

This allows changing the error message displayed in Deno for errors by providing a "reason" property when denying a permission.

Changed files:
filename: cli/schemas/permission-broker-response.v1.json
patch: @@ -15,6 +15,11 @@
         "grant",
         "deny"
       ]
+    },
+    "reason": {
+      "type": "string",
+      "description": "Optional message to display to the user on deny. This will override the default error message text."
     }
-  }
+  },
+  "required": ["id", "result"]
 }

filename: runtime/permissions/broker.rs
patch: @@ -27,6 +27,7 @@ pub fn has_broker() -> bool {
 }
 
 #[derive(serde::Serialize)]
+#[serde(rename_all = "camelCase")]
 struct PermissionBrokerRequest<'a> {
   v: u32,
   id: u32,
@@ -36,9 +37,11 @@ struct PermissionBrokerRequest<'a> {
 }
 
 #[derive(Debug, serde::Deserialize)]
+#[serde(rename_all = "camelCase")]
 struct PermissionBrokerResponse {
   id: u32,
   result: String,
+  reason: Option<String>,
 }
 
 pub struct PermissionBroker {
@@ -102,7 +105,9 @@ impl PermissionBroker {
 
     let prompt_response = match response.result.as_str() {
       "allow" => BrokerResponse::Allow,
-      "deny" => BrokerResponse::Deny,
+      "deny" => BrokerResponse::Deny {
+        message: response.reason,
+      },
       _ => {
         return Err(std::io::Error::other(
           "Permission broker unknown result variant",

filename: runtime/permissions/lib.rs
patch: @@ -46,7 +46,7 @@ use self::which::WhichSys;
 #[derive(Debug, Eq, PartialEq)]
 pub enum BrokerResponse {
   Allow,
-  Deny,
+  Deny { message: Option<String> },
 }
 
 use self::broker::has_broker;
@@ -55,11 +55,12 @@ use self::broker::maybe_check_with_broker;
 pub static AUDIT_FILE: OnceLock<Mutex<std::fs::File>> = OnceLock::new();
 
 #[derive(Debug, thiserror::Error, deno_error::JsError)]
-#[error("Requires {access}, {}", format_permission_error(.name))]
+#[error("{}", custom_message.as_ref().cloned().unwrap_or_else(|| format!("Requires {access}, {}", format_permission_error(.name))))]
 #[class("NotCapable")]
 pub struct PermissionDeniedError {
   pub access: String,
   pub name: &'static str,
+  pub custom_message: Option<String>,
 }
 
 fn format_permission_error(name: &'static str) -> String {
@@ -446,6 +447,7 @@ impl PermissionState {
     PermissionDeniedError {
       access: Self::fmt_access(name, info),
       name,
+      custom_message: None,
     }
   }
 
@@ -489,9 +491,13 @@ impl PermissionState {
           Self::log_perm_access(name, info);
           return (Ok(()), false, false);
         }
-        BrokerResponse::Deny => {
+        BrokerResponse::Deny { message } => {
           return (
-            Err(Self::permission_denied_error(name, info().as_deref())),
+            Err(PermissionDeniedError {
+              access: Self::fmt_access(name, info().as_deref()),
+              name,
+              custom_message: message,
+            }),
             false,
             false,
           );

filename: tests/integration/run_tests.rs
patch: @@ -3505,8 +3505,8 @@ fn handle_invalid_path_error() {
   assert_contains!(String::from_utf8_lossy(&output.stderr), "Module not found");
 }
 
-#[tokio::test]
-async fn test_permission_broker_doesnt_exit() {
+#[flaky_test::flaky_test]
+fn test_permission_broker_doesnt_exit() {
   let context = TestContext::default();
   let socket_path = if cfg!(windows) {
     PathRef::new(r"\\.\pipe\deno-permission-broker")
@@ -3525,8 +3525,8 @@ async fn test_permission_broker_doesnt_exit() {
   );
 }
 
-#[tokio::test]
-async fn test_permission_broker() {
+#[flaky_test::flaky_test]
+fn test_permission_broker() {
   use std::io::BufRead;
   use std::io::BufReader;
 
@@ -3572,7 +3572,7 @@ async fn test_permission_broker() {
     .run();
   output.assert_exit_code(1);
   output.assert_matches_text(
-    "Warning Permission broker is an experimental feature\nerror:[WILDCARD]NotCapable: Requires env access[WILDCARD]",
+    "Warning Permission broker is an experimental feature\nerror:[WILDCARD]NotCapable: Make sure to enable reading env vars.[WILDCARD]",
   );
 
   let _ = broker.kill();

filename: tests/testdata/run/permission_broker/broker.ts
patch: @@ -46,6 +46,7 @@ async function createUnixSocketServer(socketPath: string): Promise<void> {
         const response = { id: request.id, result: "allow" };
         if (request.permission === "env") {
           response.result = "deny";
+          response.reason = "Make sure to enable reading env vars.";
         }
         const buf = new TextEncoder().encode(JSON.stringify(response) + "\n");
         const written = await conn.write(buf);
@@ -205,6 +206,9 @@ async function createWindowsNamedPipeServer(spec: string): Promise<void> {
         const response = {
           id: request.id,
           result: request.permission === "env" ? "deny" : "allow",
+          reason: request.permission === "env"
+            ? "Make sure to enable reading env vars."
+            : undefined,
         };
         const out = encoder.encode(JSON.stringify(response) + "\n");
         writeAll(hPipe, out);