Hide sensitive chain values in template preview

LucasPickering · Nov 28, 2023 · 92a7622 · 92a7622
1 parent a0e3306
commit 92a7622
Show file tree

Hide file tree

Showing 6 changed files with 95 additions and 23 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## [Unreleased] - ReleaseDate
+
+### Changed
+
+- Hide sensitive chain values in preview
+
 ## [0.9.0] - 2023-11-28
 
 ### Added

diff --git a/src/template.rs b/src/template.rs
@@ -3,7 +3,7 @@ mod parse;
 mod prompt;
 mod render;
 
-pub use error::{ChainError, TemplateError, TemplateResult};
+pub use error::{ChainError, TemplateError};
 pub use parse::Span;
 pub use prompt::{Prompt, Prompter};
 
@@ -102,7 +102,7 @@ pub enum TemplateChunk {
     /// large block of text.
     Raw(Span),
     /// Outcome of rendering a template key
-    Rendered(String),
+    Rendered { value: String, sensitive: bool },
     /// An error occurred while rendering a template key
     Error(TemplateError),
 }
@@ -396,7 +396,6 @@ mod tests {
 
         let chains = vec![create!(
             Chain,
-
             source: ChainSource::File(file_path),
         )];
         let context = create!(TemplateContext, chains: chains);
@@ -424,13 +423,11 @@ mod tests {
     async fn test_chain_prompt() {
         let chains = vec![create!(
             Chain,
-
             source: ChainSource::Prompt(Some("password".into())),
         )];
         let context = create!(
             TemplateContext,
             chains: chains,
-            // Prompter gives no response
             prompter: Box::new(TestPrompter::new(Some("hello!"))),
         );
 
@@ -458,6 +455,31 @@ mod tests {
         );
     }
 
+    /// Values marked sensitive should have that flag set in the rendered output
+    #[tokio::test]
+    async fn test_chain_sensitive() {
+        let chains = vec![create!(
+            Chain,
+            source: ChainSource::Prompt(Some("password".into())),
+            sensitive: true,
+        )];
+        let context = create!(
+            TemplateContext,
+            chains: chains,
+            // Prompter gives no response
+            prompter: Box::new(TestPrompter::new(Some("hello!"))),
+        );
+        assert_eq!(
+            Template::from("{{chains.chain1}}")
+                .render_chunks(&context)
+                .await,
+            vec![TemplateChunk::Rendered {
+                value: "hello!".into(),
+                sensitive: true
+            }]
+        );
+    }
+
     #[tokio::test]
     async fn test_environment_success() {
         let context = create!(TemplateContext);
@@ -488,7 +510,10 @@ mod tests {
             chunks,
             vec![
                 TemplateChunk::Raw(Span::new(0, 6)),
-                TemplateChunk::Rendered("🧡💛".into()),
+                TemplateChunk::Rendered {
+                    value: "🧡💛".into(),
+                    sensitive: false
+                },
                 // Each emoji is 4 bytes
                 TemplateChunk::Raw(Span::new(17, 14)),
                 TemplateChunk::Error(TemplateError::FieldUnknown {

diff --git a/src/template/error.rs b/src/template/error.rs
@@ -4,8 +4,6 @@ use serde_json_path::ExactlyOneError;
 use std::{env::VarError, io, path::PathBuf, string::FromUtf8Error};
 use thiserror::Error;
 
-pub type TemplateResult = Result<String, TemplateError>;
-
 /// An error while parsing a template. This is derived from a nom error
 #[derive(Debug, Error)]
 #[error("{0}")]

diff --git a/src/template/render.rs b/src/template/render.rs
@@ -5,7 +5,7 @@ use crate::{
     http::{ContentType, Json},
     template::{
         parse::TemplateInputChunk, ChainError, Prompt, Template, TemplateChunk,
-        TemplateContext, TemplateError, TemplateKey, TemplateResult,
+        TemplateContext, TemplateError, TemplateKey,
     },
     util::ResultExt,
 };
@@ -20,6 +20,16 @@ use std::{
 use tokio::{fs, process::Command, sync::oneshot};
 use tracing::{info, instrument, trace};
 
+/// Outcome of rendering a single chunk. This allows attaching some metadata to
+/// the render.
+#[derive(Debug)]
+struct RenderedChunk {
+    value: String,
+    sensitive: bool,
+}
+
+type TemplateResult = Result<RenderedChunk, TemplateError>;
+
 impl Template {
     /// Render the template string using values from the given context. If an
     /// error occurs, it is returned as general `anyhow` error. If you need a
@@ -65,7 +75,13 @@ impl Template {
                                 value,
                                 "Rendered template key from override"
                             );
-                            Ok(value.into())
+                            Ok(RenderedChunk {
+                                value: value.clone(),
+                                // The overriden value *could* be marked
+                                // sensitive, but we're taking a shortcut and
+                                // assuming it isn't
+                                sensitive: false,
+                            })
                         }
                         None => {
                             // Standard case - parse the key and render it
@@ -74,7 +90,7 @@ impl Template {
                             if let Ok(value) = &result {
                                 trace!(
                                     key = raw,
-                                    value,
+                                    ?value,
                                     "Rendered template key"
                                 );
                             }
@@ -95,7 +111,7 @@ impl Template {
     pub(super) async fn render_stitched(
         &self,
         context: &TemplateContext,
-    ) -> TemplateResult {
+    ) -> Result<String, TemplateError> {
         // Render each individual template chunk in the string
         let chunks = self.render_chunks(context).await;
 
@@ -106,7 +122,9 @@ impl Template {
                 TemplateChunk::Raw(span) => {
                     buffer.push_str(self.substring(span));
                 }
-                TemplateChunk::Rendered(value) => buffer.push_str(&value),
+                TemplateChunk::Rendered { value, .. } => {
+                    buffer.push_str(&value)
+                }
                 TemplateChunk::Error(error) => return Err(error),
             }
         }
@@ -117,7 +135,10 @@ impl Template {
 impl From<TemplateResult> for TemplateChunk {
     fn from(result: TemplateResult) -> Self {
         match result {
-            Ok(value) => Self::Rendered(value),
+            Ok(outcome) => Self::Rendered {
+                value: outcome.value,
+                sensitive: outcome.sensitive,
+            },
             Err(error) => Self::Error(error),
         }
     }
@@ -165,8 +186,8 @@ impl<'a> TemplateSource<'a> for FieldTemplateSource<'a> {
                 field: field.to_owned(),
             }
         })?;
-        match value {
-            ProfileValue::Raw(value) => Ok(value.clone()),
+        let rendered = match value {
+            ProfileValue::Raw(value) => value.clone(),
             // recursion!
             ProfileValue::Template(template) => {
                 trace!(%field, %template, "Rendering recursive template");
@@ -175,9 +196,13 @@ impl<'a> TemplateSource<'a> for FieldTemplateSource<'a> {
                         template: template.clone(),
                         error: Box::new(error),
                     }
-                })
+                })?
             }
-        }
+        };
+        Ok(RenderedChunk {
+            value: rendered,
+            sensitive: false,
+        })
     }
 }
 
@@ -220,9 +245,14 @@ impl<'a> TemplateSource<'a> for ChainTemplateSource<'a> {
             };
 
             // If a selector path is present, filter down the value
-            Ok(match &chain.selector {
+            let value = match &chain.selector {
                 Some(path) => self.apply_selector(&value, path)?,
                 None => value,
+            };
+
+            Ok(RenderedChunk {
+                value,
+                sensitive: chain.sensitive,
             })
         }
         .await;
@@ -346,11 +376,15 @@ struct EnvironmentTemplateSource<'a> {
 #[async_trait]
 impl<'a> TemplateSource<'a> for EnvironmentTemplateSource<'a> {
     async fn render(&self, _: &'a TemplateContext) -> TemplateResult {
-        env::var(self.variable).map_err(|err| {
+        let value = env::var(self.variable).map_err(|err| {
             TemplateError::EnvironmentVariable {
                 variable: self.variable.to_owned(),
                 error: err,
             }
+        })?;
+        Ok(RenderedChunk {
+            value,
+            sensitive: false,
         })
     }
 }
diff --git a/src/tui/view/common/template_preview.rs b/src/tui/view/common/template_preview.rs
@@ -133,8 +133,16 @@ impl<'a> TextStitcher<'a> {
                 TemplateChunk::Raw(span) => {
                     (template.substring(*span), Style::default())
                 }
-                TemplateChunk::Rendered(value) => {
-                    (value.as_str(), theme.template_preview_text)
+                TemplateChunk::Rendered { value, sensitive } => {
+                    let value = if *sensitive {
+                        // Hide sensitive values. Ratatui has a Masked type, but
+                        // it complicates the string ownership a lot and also
+                        // exposes the length of the sensitive text
+                        "<sensitive>"
+                    } else {
+                        value.as_str()
+                    };
+                    (value, theme.template_preview_text)
                 }
                 // There's no good way to render the entire error inline
                 TemplateChunk::Error(_) => {
@@ -156,6 +164,7 @@ impl<'a> TextStitcher<'a> {
             Some((a, b)) => {
                 self.add_span(a, style);
                 self.end_line();
+                // Recursion!
                 self.add_area(b, style);
             }
             // This area has no line breaks, just add it and move on

diff --git a/src/tui/view/component/misc.rs b/src/tui/view/component/misc.rs
@@ -69,7 +69,7 @@ impl PromptModal {
     pub fn new(prompt: Prompt) -> Self {
         let mut text_area = TextArea::default();
         if prompt.sensitive() {
-            text_area.set_mask_char('\u{2022}');
+            text_area.set_mask_char('•');
         }
         Self {
             prompt,