fix(plugins): render template function config values before plugins see them (#608)

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

crates/yaak-commands/src/auth.rs

@@ -6,14 +6,11 @@
 //! own environment chain before a plugin sees them, or an auth plugin receives
 //! `${[ api_key ]}` where it expected a key.
 
-use crate::error::{Error, Result};
+use crate::error::Result;
 use crate::host::PluginHost;
-use crate::render::render_json_value;
-use std::collections::HashMap;
-use yaak_models::models::AnyModel;
+use crate::render::render_form_values;
 use yaak_plugins::events::{
-    GetHttpAuthenticationConfigResponse, GetHttpAuthenticationSummaryResponse, JsonPrimitive,
-    RenderPurpose,
+    GetHttpAuthenticationConfigResponse, GetHttpAuthenticationSummaryResponse, RenderPurpose,
 };
 use yaak_rpc_schema::*;
 use yaak_templates::RenderOptions;
@@ -31,7 +28,7 @@ pub async fn cmd_get_http_authentication_config<H: PluginHost>(
 ) -> Result<GetHttpAuthenticationConfigResponse> {
     // A config form is being displayed, so a template that cannot resolve
     // should show as blank rather than refuse to open the form.
-    let values = render_auth_values(
+    let values = render_form_values(
         &host,
         &req.model,
         req.environment_id.as_deref(),
@@ -50,7 +47,7 @@ pub async fn cmd_call_http_authentication_action<H: PluginHost>(
 ) -> Result<()> {
     // An action actually uses these values, so an unresolvable template is an
     // error rather than an empty string that would silently authenticate wrong.
-    let values = render_auth_values(
+    let values = render_form_values(
         &host,
         &req.model,
         req.environment_id.as_deref(),
@@ -63,40 +60,3 @@ pub async fn cmd_call_http_authentication_action<H: PluginHost>(
     host.call_http_authentication_action(&req.auth_name, req.action_index, values, req.model.id())
         .await
 }
-
-/// Render the form's values against the environment chain the model sits in.
-///
-/// The chain depends on where the model lives — a request inherits through its
-/// folder, a workspace has only its own — so the model is what decides which
-/// variables are in scope.
-async fn render_auth_values<H: PluginHost>(
-    host: &H,
-    model: &AnyModel,
-    environment_id: Option<&str>,
-    values: HashMap<String, JsonPrimitive>,
-    purpose: RenderPurpose,
-    options: &RenderOptions,
-) -> Result<HashMap<String, JsonPrimitive>> {
-    let (workspace_id, folder_id) = match model {
-        AnyModel::HttpRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()),
-        AnyModel::GrpcRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()),
-        AnyModel::WebsocketRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()),
-        AnyModel::Folder(f) => (f.workspace_id.clone(), f.folder_id.clone()),
-        AnyModel::Workspace(w) => (w.id.clone(), None),
-        other => {
-            return Err(Error::Generic(format!(
-                "Cannot resolve authentication for a {}",
-                other.model()
-            )));
-        }
-    };
-
-    let environment_chain =
-        host.db().resolve_environments(&workspace_id, folder_id.as_deref(), environment_id)?;
-
-    let cb = host.template_callback(purpose);
-    let rendered =
-        render_json_value(serde_json::to_value(&values)?, environment_chain, &cb, options).await?;
-
-    Ok(serde_json::from_value(rendered)?)
-}

crates/yaak-commands/src/host.rs

@@ -130,6 +130,7 @@ pub trait PluginHost: Host {
     ) -> impl Future<Output = crate::Result<Vec<GetTemplateFunctionSummaryResponse>>>;
 
     /// The form a template function wants to show for the given values.
+    /// `values` arrive already rendered.
     fn template_function_config(
         &self,
         function_name: &str,

crates/yaak-commands/src/render.rs

@@ -1,12 +1,19 @@
 //! Rendering a template against an environment chain.
 //!
 //! The variables come from the chain, the functions come from the host's
-//! template callback. Neither of these knows which host it is running under —
-//! that is the whole point of taking the callback as a parameter.
+//! template callback. `render_template` and `render_json_value` know nothing
+//! about which host they run under — that is the whole point of taking the
+//! callback as a parameter. `render_form_values` sits one level up: resolving
+//! the chain a model sits in is an ordinary database read, so it takes the
+//! host and does that read before rendering.
 
+use crate::error::{Error, Result};
+use crate::host::PluginHost;
 use serde_json::Value;
-use yaak_models::models::Environment;
+use std::collections::HashMap;
+use yaak_models::models::{AnyModel, Environment};
 use yaak_models::render::make_vars_hashmap;
+use yaak_plugins::events::{JsonPrimitive, RenderPurpose};
 use yaak_templates::{RenderOptions, TemplateCallback, parse_and_render, render_json_value_raw};
 
 pub async fn render_template<T: TemplateCallback>(
@@ -28,3 +35,40 @@ pub async fn render_json_value<T: TemplateCallback>(
     let vars = &make_vars_hashmap(environment_chain);
     render_json_value_raw(value, vars, cb, opt).await
 }
+
+/// Render a config form's values against the environment chain the model sits in.
+///
+/// The chain depends on where the model lives — a request inherits through its
+/// folder, a workspace has only its own — so the model is what decides which
+/// variables are in scope.
+pub(crate) async fn render_form_values<H: PluginHost>(
+    host: &H,
+    model: &AnyModel,
+    environment_id: Option<&str>,
+    values: HashMap<String, JsonPrimitive>,
+    purpose: RenderPurpose,
+    options: &RenderOptions,
+) -> Result<HashMap<String, JsonPrimitive>> {
+    let (workspace_id, folder_id) = match model {
+        AnyModel::HttpRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()),
+        AnyModel::GrpcRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()),
+        AnyModel::WebsocketRequest(r) => (r.workspace_id.clone(), r.folder_id.clone()),
+        AnyModel::Folder(f) => (f.workspace_id.clone(), f.folder_id.clone()),
+        AnyModel::Workspace(w) => (w.id.clone(), None),
+        other => {
+            return Err(Error::Generic(format!(
+                "Cannot resolve environments for a {}",
+                other.model()
+            )));
+        }
+    };
+
+    let environment_chain =
+        host.db().resolve_environments(&workspace_id, folder_id.as_deref(), environment_id)?;
+
+    let cb = host.template_callback(purpose);
+    let rendered =
+        render_json_value(serde_json::to_value(&values)?, environment_chain, &cb, options).await?;
+
+    Ok(serde_json::from_value(rendered)?)
+}

crates/yaak-commands/src/templates.rs

@@ -7,7 +7,7 @@
 
 use crate::error::Result;
 use crate::host::PluginHost;
-use crate::render::render_template;
+use crate::render::{render_form_values, render_template};
 use yaak_plugins::events::{
     GetTemplateFunctionConfigResponse, GetTemplateFunctionSummaryResponse, GetThemesResponse,
     RenderPurpose,
@@ -56,7 +56,19 @@ pub async fn cmd_template_function_config<H: PluginHost>(
     host: H,
     req: CmdTemplateFunctionConfigReq,
 ) -> Result<GetTemplateFunctionConfigResponse> {
-    host.template_function_config(&req.function_name, req.values, req.model.id()).await
+    // A config form is being displayed, so a template that cannot resolve
+    // should show as blank rather than refuse to open the form.
+    let values = render_form_values(
+        &host,
+        &req.model,
+        req.environment_id.as_deref(),
+        req.values,
+        RenderPurpose::Preview,
+        &RenderOptions::return_empty(),
+    )
+    .await?;
+
+    host.template_function_config(&req.function_name, values, req.model.id()).await
 }
 
 pub async fn cmd_get_themes<H: PluginHost>(

crates/yaak-commands/tests/test_host.rs

@@ -18,7 +18,7 @@ use yaak_commands::models::{
     cmd_default_headers, cmd_get_workspace_meta, models_delete, models_upsert,
     models_workspace_models,
 };
-use yaak_commands::templates::cmd_render_template;
+use yaak_commands::templates::{cmd_render_template, cmd_template_function_config};
 use yaak_commands::{Host, PluginHost};
 use yaak_core::WorkspaceContext;
 use yaak_crypto::manager::EncryptionManager;
@@ -172,6 +172,8 @@ struct SingleThreadedHost {
     /// The values the last auth-config call arrived with, so a test can check
     /// they were rendered before the host ever saw them.
     auth_values: Rc<RefCell<Option<HashMap<String, JsonPrimitive>>>>,
+    /// Same, for the last template-function-config call.
+    fn_values: Rc<RefCell<Option<HashMap<String, JsonPrimitive>>>>,
 }
 
 impl Host for SingleThreadedHost {
@@ -261,9 +263,10 @@ impl PluginHost for SingleThreadedHost {
     async fn template_function_config(
         &self,
         function_name: &str,
-        _values: HashMap<String, JsonPrimitive>,
+        values: HashMap<String, JsonPrimitive>,
         _model_id: &str,
     ) -> yaak_commands::Result<GetTemplateFunctionConfigResponse> {
+        *self.fn_values.borrow_mut() = Some(values);
         Err(yaak_commands::Error::Generic(format!("no plugin provides {function_name}()")))
     }
 
@@ -376,6 +379,7 @@ async fn a_single_threaded_host_can_implement_the_trait() {
     let host = SingleThreadedHost {
         inner: Rc::new(Arc::into_inner(inner).expect("sole owner")),
         auth_values: Rc::new(RefCell::new(None)),
+        fn_values: Rc::new(RefCell::new(None)),
     };
 
     let workspace = Workspace { name: "From one thread".to_string(), ..Default::default() };
@@ -448,6 +452,7 @@ async fn auth_values_are_rendered_before_the_host_sees_them() {
     let host = SingleThreadedHost {
         inner: Rc::new(Arc::into_inner(inner).expect("sole owner")),
         auth_values: Rc::new(RefCell::new(None)),
+        fn_values: Rc::new(RefCell::new(None)),
     };
 
     let workspace = host
@@ -499,3 +504,65 @@ async fn auth_values_are_rendered_before_the_host_sees_them() {
         seen.get("password"),
     );
 }
+
+/// Same contract as auth: template function argument values may contain
+/// templates (the 1Password token argument defaults to `${[1PASSWORD_TOKEN]}`),
+/// and the shared handler renders them before the host is called.
+#[tokio::test]
+async fn template_function_values_are_rendered_before_the_host_sees_them() {
+    let TestHost { inner } = TestHost::new();
+    let host = SingleThreadedHost {
+        inner: Rc::new(Arc::into_inner(inner).expect("sole owner")),
+        auth_values: Rc::new(RefCell::new(None)),
+        fn_values: Rc::new(RefCell::new(None)),
+    };
+
+    let workspace = host
+        .db()
+        .upsert_workspace(
+            &Workspace { name: "Functions".to_string(), ..Default::default() },
+            &host.update_source(),
+        )
+        .expect("workspace");
+    host.db()
+        .upsert_environment(
+            &Environment {
+                workspace_id: workspace.id.clone(),
+                name: "Env".to_string(),
+                variables: vec![EnvironmentVariable {
+                    enabled: true,
+                    name: "1PASSWORD_TOKEN".to_string(),
+                    value: "ops_abc123".to_string(),
+                    id: None,
+                }],
+                ..Default::default()
+            },
+            &host.update_source(),
+        )
+        .expect("environment");
+    let environment =
+        host.db().list_environments_ensure_base(&workspace.id).expect("list").remove(0);
+
+    let mut values = HashMap::new();
+    values.insert("token".to_string(), JsonPrimitive::String("${[1PASSWORD_TOKEN]}".to_string()));
+
+    // The host refuses the call itself — it has no plugins — but only after the
+    // handler has rendered and handed over the values, which is what matters.
+    let _ = cmd_template_function_config(
+        host.clone(),
+        yaak_rpc_schema::CmdTemplateFunctionConfigReq {
+            function_name: "1password.item".to_string(),
+            values,
+            model: AnyModel::Workspace(workspace),
+            environment_id: Some(environment.id),
+        },
+    )
+    .await;
+
+    let seen = host.fn_values.borrow().clone().expect("the host should have been called");
+    assert!(
+        matches!(seen.get("token"), Some(JsonPrimitive::String(v)) if v == "ops_abc123"),
+        "the template should have been rendered before reaching the host, got {:?}",
+        seen.get("token"),
+    );
+}