Merge pull request #6525 from Turbo87/secrecy

Use `secrecy` crate for database URLs and git credentials

Author: Turbo87

Cargo.lock

@@ -17,6 +17,7 @@ anyhow = "=1.0.71"
 base64 = "=0.13.1"
 dotenvy = "=0.15.7"
 git2 = "=0.17.1"
+secrecy = "=0.8.0"
 serde = { version = "=1.0.163", features = ["derive"] }
 serde_json = "=1.0.96"
 tempfile = "=3.5.0"

cargo-registry-index/Cargo.toml

@@ -1,15 +1,21 @@
 use anyhow::{anyhow, Context};
+use secrecy::{ExposeSecret, SecretString};
 use std::io::Write;
 
 #[derive(Clone)]
 pub enum Credentials {
     Missing,
-    Http { username: String, password: String },
-    Ssh { key: String },
+    Http {
+        username: String,
+        password: SecretString,
+    },
+    Ssh {
+        key: SecretString,
+    },
 }
 
 impl Credentials {
-    pub(crate) fn ssh_key(&self) -> Option<&str> {
+    pub(crate) fn ssh_key(&self) -> Option<&SecretString> {
         match self {
             Credentials::Ssh { key } => Some(key),
             _ => None,
@@ -48,7 +54,7 @@ impl Credentials {
             .context("Failed to create temporary file")?;
 
         temp_key_file
-            .write_all(key.as_bytes())
+            .write_all(key.expose_secret().as_bytes())
             .context("Failed to write SSH key to temporary file")?;
 
         Ok(temp_key_file.into_temp_path())

cargo-registry-index/credentials.rs

@@ -1,5 +1,6 @@
 use crate::credentials::Credentials;
 use anyhow::{anyhow, Context};
+use secrecy::{ExposeSecret, SecretString};
 use std::path::{Path, PathBuf};
 use std::process::Command;
 use tempfile::TempDir;
@@ -13,10 +14,10 @@ pub struct RepositoryConfig {
 impl RepositoryConfig {
     pub fn from_environment() -> Self {
         let username = dotenvy::var("GIT_HTTP_USER");
-        let password = dotenvy::var("GIT_HTTP_PWD");
+        let password = dotenvy::var("GIT_HTTP_PWD").map(SecretString::from);
         let http_url = dotenvy::var("GIT_REPO_URL");
 
-        let ssh_key = dotenvy::var("GIT_SSH_KEY");
+        let ssh_key = dotenvy::var("GIT_SSH_KEY").map(SecretString::from);
         let ssh_url = dotenvy::var("GIT_SSH_REPO_URL");
 
         match (username, password, http_url, ssh_key, ssh_url) {
@@ -29,12 +30,11 @@ impl RepositoryConfig {
                 let index_location =
                     Url::parse(&ssh_url).expect("failed to parse GIT_SSH_REPO_URL");
 
-                let credentials = Credentials::Ssh {
-                    key: String::from_utf8(
-                        base64::decode(encoded_key).expect("failed to base64 decode the ssh key"),
-                    )
-                    .expect("failed to convert the ssh key to a string"),
-                };
+                let key = base64::decode(encoded_key.expose_secret())
+                    .expect("failed to base64 decode the ssh key");
+                let key =
+                    String::from_utf8(key).expect("failed to convert the ssh key to a string");
+                let credentials = Credentials::Ssh { key: key.into() };
 
                 Self {
                     index_location,

cargo-registry-index/repo.rs

@@ -20,6 +20,7 @@ use cargo_registry::worker::cloudfront::CloudFront;
 use cargo_registry::{background_jobs::*, db, ssh};
 use cargo_registry_index::{Repository, RepositoryConfig};
 use reqwest::blocking::Client;
+use secrecy::ExposeSecret;
 use std::sync::{Arc, Mutex};
 use std::thread::sleep;
 use std::time::{Duration, Instant};
@@ -49,7 +50,7 @@ fn main() {
         }
     }
 
-    let db_url = db::connection_url(&config.db, &config.db.primary.url);
+    let db_url = db::connection_url(&config.db, config.db.primary.url.expose_secret());
 
     let job_start_timeout = dotenvy::var("BACKGROUND_JOB_TIMEOUT")
         .unwrap_or_else(|_| "30".into())

src/bin/background-worker.rs

@@ -13,6 +13,7 @@
 
 use crate::config::Base;
 use crate::{env, Env};
+use secrecy::SecretString;
 
 pub struct DatabasePools {
     /// Settings for the primary database. This is usually writeable, but will be read-only in
@@ -32,7 +33,7 @@ pub struct DatabasePools {
 
 #[derive(Debug)]
 pub struct DbPoolConfig {
-    pub url: String,
+    pub url: SecretString,
     pub read_only_mode: bool,
     pub pool_size: u32,
     pub min_idle: Option<u32>,
@@ -53,8 +54,8 @@ impl DatabasePools {
     ///
     /// This function panics if `DB_OFFLINE=leader` but `READ_ONLY_REPLICA_URL` is unset.
     pub fn full_from_environment(base: &Base) -> Self {
-        let leader_url = env("DATABASE_URL");
-        let follower_url = dotenvy::var("READ_ONLY_REPLICA_URL").ok();
+        let leader_url = env("DATABASE_URL").into();
+        let follower_url = dotenvy::var("READ_ONLY_REPLICA_URL").map(Into::into).ok();
         let read_only_mode = dotenvy::var("READ_ONLY_MODE").is_ok();
 
         let primary_pool_size = match dotenvy::var("DB_PRIMARY_POOL_SIZE") {
@@ -136,7 +137,7 @@ impl DatabasePools {
     pub fn test_from_environment() -> Self {
         DatabasePools {
             primary: DbPoolConfig {
-                url: env("TEST_DATABASE_URL"),
+                url: env("TEST_DATABASE_URL").into(),
                 read_only_mode: false,
                 pool_size: 1,
                 min_idle: None,

src/config/database_pools.rs

@@ -1,6 +1,7 @@
 use diesel::prelude::*;
 use diesel::r2d2::{self, ConnectionManager, CustomizeConnection};
 use prometheus::Histogram;
+use secrecy::{ExposeSecret, SecretString};
 use std::sync::{Arc, Mutex, MutexGuard};
 use std::{
     ops::{Deref, DerefMut},
@@ -27,12 +28,12 @@ pub enum DieselPool {
 
 impl DieselPool {
     pub(crate) fn new(
-        url: &str,
+        url: &SecretString,
         config: &config::DatabasePools,
         r2d2_config: r2d2::Builder<ConnectionManager<PgConnection>>,
         time_to_obtain_connection_metric: Histogram,
     ) -> Result<DieselPool, PoolError> {
-        let manager = ConnectionManager::new(connection_url(config, url));
+        let manager = ConnectionManager::new(connection_url(config, url.expose_secret()));
 
         // For crates.io we want the behavior of creating a database pool to be slightly different
         // than the defaults of R2D2: the library's build() method assumes its consumers always
@@ -69,8 +70,8 @@ impl DieselPool {
         }
     }
 
-    pub(crate) fn new_test(config: &config::DatabasePools, url: &str) -> DieselPool {
-        let mut conn = PgConnection::establish(&connection_url(config, url))
+    pub(crate) fn new_test(config: &config::DatabasePools, url: &SecretString) -> DieselPool {
+        let mut conn = PgConnection::establish(&connection_url(config, url.expose_secret()))
             .expect("failed to establish connection");
         conn.begin_test_transaction()
             .expect("failed to begin test transaction");
@@ -167,7 +168,7 @@ impl DerefMut for DieselPooledConn<'_> {
 pub fn oneoff_connection_with_config(
     config: &config::DatabasePools,
 ) -> ConnectionResult<PgConnection> {
-    let url = connection_url(config, &config.primary.url);
+    let url = connection_url(config, config.primary.url.expose_secret());
     PgConnection::establish(&url)
 }
 

src/db.rs

@@ -13,6 +13,7 @@ use cargo_registry::swirl::Runner;
 use diesel::PgConnection;
 use oauth2::{ClientId, ClientSecret};
 use reqwest::{blocking::Client, Proxy};
+use secrecy::ExposeSecret;
 use std::collections::HashSet;
 
 struct TestAppInner {
@@ -201,17 +202,17 @@ impl TestAppBuilder {
         // The schema will be cleared up once the app is dropped.
         let (primary_db_chaosproxy, replica_db_chaosproxy, fresh_schema) =
             if !self.config.use_test_database_pool {
-                let fresh_schema = FreshSchema::new(&self.config.db.primary.url);
+                let fresh_schema = FreshSchema::new(self.config.db.primary.url.expose_secret());
                 let (primary_proxy, url) =
                     ChaosProxy::proxy_database_url(fresh_schema.database_url()).unwrap();
-                self.config.db.primary.url = url;
+                self.config.db.primary.url = url.into();
 
                 let replica_proxy = match self.test_database {
                     TestDatabase::SlowRealPool { replica: true } => {
                         let (replica_proxy, url) =
                             ChaosProxy::proxy_database_url(fresh_schema.database_url()).unwrap();
                         self.config.db.replica = Some(DbPoolConfig {
-                            url,
+                            url: url.into(),
                             read_only_mode: true,
                             pool_size: 1,
                             min_idle: None,