Radish alpha
r
Radicle CI broker
Radicle
Git (anonymous pull)
Log in to clone via SSH
feat: make it harder to leak sensitive environment variables
Lars Wirzenius committed 1 year ago
commit 5aea92df7b1fb702ce8c52f5db63016dfbf89523
parent 26d55f0604218e2705ca303343f1aecd647a42bf
6 files changed +149 -6
modified ci-broker.md
@@ -50,7 +50,8 @@ adapters:
    command: ./adapter.sh
    env:
      RADICLE_NATIVE_CI: native-ci.yaml
-
    sensitive_env: {}
+
    sensitive_env:
+
      API_KEY: xyzzy
filters:
  - !Branch "main"
~~~
modified src/adapter.rs
@@ -21,6 +21,7 @@ use crate::{
    msg::{MessageError, Request, Response},
    notif::NotificationSender,
    run::{Run, RunState},
+
    sensitive::Sensitive,
    timeoutcmd::{TimeoutCommand, TimeoutError},
};

@@ -48,6 +49,13 @@ impl Adapter {
        self
    }

+
    pub fn with_sensitive_environment(mut self, env: &HashMap<String, Sensitive>) -> Self {
+
        for (key, value) in env.iter() {
+
            self.env.insert(key.into(), value.as_str().into());
+
        }
+
        self
+
    }
+

    fn envs(&self) -> impl Iterator<Item = (&OsStr, &OsStr)> {
        self.env.iter().map(|(k, v)| (k.as_ref(), v.as_ref()))
    }
modified src/bin/cib.rs
@@ -158,7 +158,7 @@ impl QueuedCmd {
                ))?;
        let adapter = Adapter::new(&spec.command)
            .with_environment(spec.envs())
-
            .with_environment(spec.sensitive_envs());
+
            .with_sensitive_environment(spec.sensitive_envs());
        logger::adapter_config(config);
        broker.set_default_adapter(&adapter);

@@ -246,7 +246,7 @@ impl ProcessEventsCmd {
                ))?;
        let adapter = Adapter::new(&spec.command)
            .with_environment(spec.envs())
-
            .with_environment(spec.sensitive_envs());
+
            .with_sensitive_environment(spec.sensitive_envs());
        logger::adapter_config(config);
        broker.set_default_adapter(&adapter);

modified src/config.rs
@@ -10,7 +10,7 @@ use std::{
use duration_str::deserialize_duration;
use serde::{Deserialize, Serialize};

-
use crate::filter::EventFilter;
+
use crate::{filter::EventFilter, sensitive::Sensitive};

const DEFAULT_MAX_RUN_TIME: Duration = Duration::from_secs(3600);
const DEFAULT_STATUS_PAGE_UPDATE_INTERVAL: u64 = 10;
@@ -80,14 +80,14 @@ impl fmt::Debug for Adapter {
pub struct Adapter {
    pub command: PathBuf,
    pub env: HashMap<String, String>,
-
    pub sensitive_env: HashMap<String, String>,
+
    pub sensitive_env: HashMap<String, Sensitive>,
}

impl Adapter {
    pub fn envs(&self) -> &HashMap<String, String> {
        &self.env
    }
-
    pub fn sensitive_envs(&self) -> &HashMap<String, String> {
+
    pub fn sensitive_envs(&self) -> &HashMap<String, Sensitive> {
        &self.sensitive_env
    }
}
modified src/lib.rs
@@ -22,6 +22,7 @@ pub mod pages;
pub mod queueadd;
pub mod queueproc;
pub mod run;
+
pub mod sensitive;
#[cfg(test)]
pub mod test;
pub mod timeoutcmd;
added src/sensitive.rs
@@ -0,0 +1,133 @@
+
//! A data type for holding sensitive data to avoid accidentally
+
//! leaking it.
+
//!
+
//! Once a `Sensitive` value has been created, it contains a string
+
//! value. It can be created by de-serializing using `serde`.
+
//!
+
//! The sensitive data can be accessed with [`Sensitive::as_str`]. The
+
//! caller needs to be careful to not leak that.
+
//!
+
//! `Sensitive` value itself can't be printed (via the `Display`
+
//! trait), even in debug mode (`Debug` trait), or serialized with
+
//! `serde`. Instead, the value is replaced with the string
+
//! `<REDACTED>`.
+
//!
+
//! Note that this does not prevent the value from ending up in a core
+
//! dump.
+

+
use std::fmt;
+

+
use serde::{de, Deserialize, Deserializer, Serialize, Serializer};
+

+
const PLACEHOLDER: &str = "<REDACTED>";
+

+
/// Hold a sensitive string, such as a password or an API token. The
+
/// value can be accessed ([`Sensitive::as_str`]), but won't be
+
/// printed, even in debug output, and won't be serialized by `serde`.
+
#[derive()]
+
pub struct Sensitive {
+
    #[allow(dead_code)]
+
    //#[serde(serialize_with = "serialize")]
+
    data: String,
+
}
+

+
impl fmt::Display for Sensitive {
+
    /// Serialize for normal output.
+
    fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+
        write!(f, "{}", PLACEHOLDER)
+
    }
+
}
+

+
impl fmt::Debug for Sensitive {
+
    /// Serialize for debug output.
+
    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> Result<(), fmt::Error> {
+
        write!(f, "{}", PLACEHOLDER)
+
    }
+
}
+

+
impl Serialize for Sensitive {
+
    /// Serialize for `serde`.
+
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
+
    where
+
        S: Serializer,
+
    {
+
        serializer.serialize_str(PLACEHOLDER)
+
    }
+
}
+

+
impl<'de> Deserialize<'de> for Sensitive {
+
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
+
    where
+
        D: Deserializer<'de>,
+
    {
+
        deserializer.deserialize_str(SensitiveVisitor)
+
    }
+
}
+

+
struct SensitiveVisitor;
+

+
impl<'de> de::Visitor<'de> for SensitiveVisitor {
+
    type Value = Sensitive;
+

+
    fn expecting(&self, _formatter: &mut fmt::Formatter) -> fmt::Result {
+
        Ok(())
+
    }
+

+
    fn visit_str<E>(self, s: &str) -> Result<Self::Value, E>
+
    where
+
        E: de::Error,
+
    {
+
        Ok(Sensitive { data: s.into() })
+
    }
+
}
+

+
impl Sensitive {
+
    #[cfg(test)]
+
    fn new(data: &str) -> Self {
+
        Self { data: data.into() }
+
    }
+

+
    /// Return the contained string in cleartext. Do not leak this string.
+
    pub fn as_str(&self) -> &str {
+
        &self.data
+
    }
+
}
+

+
#[cfg(test)]
+
mod test_sensitive {
+
    use super::*;
+

+
    #[test]
+
    fn displayed() {
+
        let s = Sensitive::new("foo");
+
        let output = format!("{s}");
+
        assert!(!output.contains("foo"));
+
    }
+

+
    #[test]
+
    fn debugged() {
+
        let s = Sensitive::new("foo");
+
        let output = format!("{s:?}");
+
        assert!(!output.contains("foo"));
+
    }
+

+
    #[test]
+
    fn ser() -> Result<(), Box<dyn std::error::Error>> {
+
        let s = Sensitive::new("foo");
+
        let output = serde_yml::to_string(&s)?;
+
        println!("{output:#?}");
+
        assert!(!output.contains("foo"));
+
        Ok(())
+
    }
+

+
    #[test]
+
    fn deser() -> Result<(), Box<dyn std::error::Error>> {
+
        #[derive(Deserialize)]
+
        struct Foo {
+
            secret: Sensitive,
+
        }
+
        let s: Foo = serde_yml::from_str("secret: foo")?;
+
        assert_eq!(s.secret.data, "foo");
+
        Ok(())
+
    }
+
}