[wip] cli capability parity

crates/worker/src/secrets.rs

@@ -2,31 +2,42 @@
 //!
 //! Handles fetching, decrypting, and injecting secrets into execution environments.
 //! Secrets are stored encrypted in the database and decrypted on-demand for execution.
+//!
+//! Key values are stored as JSONB — they can be plain strings, objects, arrays,
+//! numbers, or booleans. When encrypted, the JSON value is serialised to a
+//! compact string, encrypted, and stored as a JSON string. Decryption reverses
+//! this process, recovering the original structured value.
+//!
+//! Encryption and decryption use the shared `attune_common::crypto` module
+//! (`encrypt_json` / `decrypt_json`) which stores ciphertext in the format
+//! `BASE64(nonce ++ ciphertext)`. This is the same format used by the API
+//! service, so keys encrypted by the API can be decrypted by the worker and
+//! vice versa.
 
-use aes_gcm::{
-    aead::{Aead, AeadCore, KeyInit, OsRng},
-    Aes256Gcm, Key as AesKey, Nonce,
-};
 use attune_common::error::{Error, Result};
 use attune_common::models::{key::Key, Action, OwnerType};
 use attune_common::repositories::key::KeyRepository;
-use base64::{engine::general_purpose::STANDARD as BASE64, Engine};
-use sha2::{Digest, Sha256};
+use serde_json::Value as JsonValue;
 use sqlx::PgPool;
 use std::collections::HashMap;
 use tracing::{debug, warn};
 
-/// Secret manager for handling secret operations
+/// Secret manager for handling secret operations.
+///
+/// Holds the database connection pool and the raw encryption key string.
+/// The encryption key is passed through to `attune_common::crypto` which
+/// derives the AES-256 key internally via SHA-256.
 pub struct SecretManager {
     pool: PgPool,
-    encryption_key: Option<Vec<u8>>,
+    encryption_key: Option<String>,
 }
 
 impl SecretManager {
-    /// Create a new secret manager
+    /// Create a new secret manager.
+    ///
+    /// `encryption_key` is the raw key string (≥ 32 characters) used for
+    /// AES-256-GCM encryption/decryption via `attune_common::crypto`.
     pub fn new(pool: PgPool, encryption_key: Option<String>) -> Result<Self> {
-        let encryption_key = encryption_key.map(|key| Self::derive_key(&key));
-
         if encryption_key.is_none() {
             warn!("No encryption key configured - encrypted secrets will fail to decrypt");
         }
@@ -37,14 +48,7 @@ impl SecretManager {
         })
     }
 
-    /// Derive encryption key from password/key string
-    fn derive_key(key: &str) -> Vec<u8> {
-        let mut hasher = Sha256::new();
-        hasher.update(key.as_bytes());
-        hasher.finalize().to_vec()
-    }
-
-    /// Fetch all secrets relevant to an action execution
+    /// Fetch all secrets relevant to an action execution.
     ///
     /// Secrets are fetched in order of precedence:
     /// 1. System-level secrets (owner_type='system')
@@ -52,10 +56,12 @@ impl SecretManager {
     /// 3. Action-level secrets (owner_type='action')
     ///
     /// More specific secrets override less specific ones with the same name.
+    /// Values are returned as [`JsonValue`] — they may be strings, objects,
+    /// arrays, numbers, or booleans.
     pub async fn fetch_secrets_for_action(
         &self,
         action: &Action,
-    ) -> Result<HashMap<String, String>> {
+    ) -> Result<HashMap<String, JsonValue>> {
         debug!("Fetching secrets for action: {}", action.r#ref);
 
         let mut secrets = HashMap::new();
@@ -126,13 +132,17 @@ impl SecretManager {
         .map_err(Into::into)
     }
 
-    /// Decrypt a secret if it's encrypted, otherwise return the value as-is
-    fn decrypt_if_needed(&self, key: &Key) -> Result<String> {
+    /// Decrypt a secret if it's encrypted, otherwise return the value as-is.
+    ///
+    /// For unencrypted keys the JSONB value is returned directly.
+    /// For encrypted keys the value (a JSON string containing base64 ciphertext)
+    /// is decrypted via `attune_common::crypto::decrypt_json` and parsed back
+    /// into the original [`JsonValue`].
+    fn decrypt_if_needed(&self, key: &Key) -> Result<JsonValue> {
         if !key.encrypted {
             return Ok(key.value.clone());
         }
 
-        // Encrypted secret requires encryption key
         let encryption_key = self
             .encryption_key
             .as_ref()
@@ -140,7 +150,7 @@ impl SecretManager {
 
         // Verify encryption key hash if present
         if let Some(expected_hash) = &key.encryption_key_hash {
-            let actual_hash = Self::compute_key_hash_from_bytes(encryption_key);
+            let actual_hash = attune_common::crypto::hash_encryption_key(encryption_key);
             if &actual_hash != expected_hash {
                 return Err(Error::Internal(format!(
                     "Encryption key hash mismatch for secret '{}'",
@@ -149,100 +159,23 @@ impl SecretManager {
             }
         }
 
-        Self::decrypt_value(&key.value, encryption_key)
+        attune_common::crypto::decrypt_json(&key.value, encryption_key)
+            .map_err(|e| Error::Internal(format!("Failed to decrypt key '{}': {}", key.name, e)))
     }
 
-    /// Decrypt an encrypted value
+    /// Compute hash of the encryption key.
     ///
-    /// Format: "nonce:ciphertext" (both base64-encoded)
-    fn decrypt_value(encrypted_value: &str, key: &[u8]) -> Result<String> {
-        // Parse format: "nonce:ciphertext"
-        let parts: Vec<&str> = encrypted_value.split(':').collect();
-        if parts.len() != 2 {
-            return Err(Error::Internal(
-                "Invalid encrypted value format. Expected 'nonce:ciphertext'".to_string(),
-            ));
-        }
-
-        let nonce_bytes = BASE64
-            .decode(parts[0])
-            .map_err(|e| Error::Internal(format!("Failed to decode nonce: {}", e)))?;
-
-        let ciphertext = BASE64
-            .decode(parts[1])
-            .map_err(|e| Error::Internal(format!("Failed to decode ciphertext: {}", e)))?;
-
-        // Create cipher
-        let key_array: [u8; 32] = key
-            .try_into()
-            .map_err(|_| Error::Internal("Invalid key length".to_string()))?;
-        let cipher_key = AesKey::<Aes256Gcm>::from_slice(&key_array);
-        let cipher = Aes256Gcm::new(cipher_key);
-
-        // Create nonce
-        let nonce = Nonce::from_slice(&nonce_bytes);
-
-        // Decrypt
-        let plaintext = cipher
-            .decrypt(nonce, ciphertext.as_ref())
-            .map_err(|e| Error::Internal(format!("Decryption failed: {}", e)))?;
-
-        String::from_utf8(plaintext)
-            .map_err(|e| Error::Internal(format!("Invalid UTF-8 in decrypted value: {}", e)))
-    }
-
-    /// Encrypt a value (for testing and future use)
-    #[allow(dead_code)]
-    pub fn encrypt_value(&self, plaintext: &str) -> Result<String> {
-        let encryption_key = self
-            .encryption_key
-            .as_ref()
-            .ok_or_else(|| Error::Internal("No encryption key configured".to_string()))?;
-
-        Self::encrypt_value_with_key(plaintext, encryption_key)
-    }
-
-    /// Encrypt a value with a specific key (static method)
-    fn encrypt_value_with_key(plaintext: &str, encryption_key: &[u8]) -> Result<String> {
-        // Create cipher
-        let key_array: [u8; 32] = encryption_key
-            .try_into()
-            .map_err(|_| Error::Internal("Invalid key length".to_string()))?;
-        let cipher_key = AesKey::<Aes256Gcm>::from_slice(&key_array);
-        let cipher = Aes256Gcm::new(cipher_key);
-
-        // Generate random nonce
-        let nonce = Aes256Gcm::generate_nonce(&mut OsRng);
-
-        // Encrypt
-        let ciphertext = cipher
-            .encrypt(&nonce, plaintext.as_bytes())
-            .map_err(|e| Error::Internal(format!("Encryption failed: {}", e)))?;
-
-        // Format: "nonce:ciphertext" (both base64-encoded)
-        let nonce_b64 = BASE64.encode(nonce);
-        let ciphertext_b64 = BASE64.encode(&ciphertext);
-
-        Ok(format!("{}:{}", nonce_b64, ciphertext_b64))
-    }
-
-    /// Compute hash of the encryption key
+    /// Uses the shared `attune_common::crypto::hash_encryption_key` so the
+    /// hash format is consistent with values stored by the API.
     pub fn compute_key_hash(&self) -> String {
         if let Some(key) = &self.encryption_key {
-            Self::compute_key_hash_from_bytes(key)
+            attune_common::crypto::hash_encryption_key(key)
         } else {
             String::new()
         }
     }
 
-    /// Compute hash from key bytes (static method)
-    fn compute_key_hash_from_bytes(key: &[u8]) -> String {
-        let mut hasher = Sha256::new();
-        hasher.update(key);
-        format!("{:x}", hasher.finalize())
-    }
-
-    /// Prepare secrets as environment variables
+    /// Prepare secrets as environment variables.
     ///
     /// **DEPRECATED - SECURITY VULNERABILITY**: This method exposes secrets in the process
     /// environment, making them visible in process listings (`ps auxe`) and `/proc/[pid]/environ`.
@@ -252,16 +185,26 @@ impl SecretManager {
     ///
     /// Secret names are converted to uppercase and prefixed with "SECRET_"
     /// Example: "api_key" becomes "SECRET_API_KEY"
+    ///
+    /// String values are used directly; structured values are serialised to
+    /// compact JSON.
     #[deprecated(
         since = "0.2.0",
         note = "Secrets in environment variables are insecure. Pass secrets via stdin instead."
     )]
-    pub fn prepare_secret_env(&self, secrets: &HashMap<String, String>) -> HashMap<String, String> {
+    pub fn prepare_secret_env(
+        &self,
+        secrets: &HashMap<String, JsonValue>,
+    ) -> HashMap<String, String> {
         secrets
             .iter()
             .map(|(name, value)| {
                 let env_name = format!("SECRET_{}", name.to_uppercase().replace('-', "_"));
-                (env_name, value.clone())
+                let env_value = match value {
+                    JsonValue::String(s) => s.clone(),
+                    other => other.to_string(),
+                };
+                (env_name, env_value)
             })
             .collect()
     }
@@ -270,78 +213,79 @@ impl SecretManager {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use attune_common::crypto;
 
-    // Helper to derive a test encryption key
-    fn derive_test_key(key: &str) -> Vec<u8> {
-        let mut hasher = Sha256::new();
-        hasher.update(key.as_bytes());
-        hasher.finalize().to_vec()
+    // ── encrypt / decrypt round-trip using shared crypto ───────────
+
+    const TEST_KEY: &str = "this_is_a_test_key_that_is_32_chars_long!!!!";
+
+    #[test]
+    fn test_encrypt_decrypt_roundtrip_string() {
+        let value = serde_json::json!("my-secret-value");
+        let encrypted = crypto::encrypt_json(&value, TEST_KEY).unwrap();
+        let decrypted = crypto::decrypt_json(&encrypted, TEST_KEY).unwrap();
+        assert_eq!(value, decrypted);
     }
 
     #[test]
-    fn test_encrypt_decrypt_roundtrip() {
-        let key = derive_test_key("test-encryption-key-12345");
-        let plaintext = "my-secret-value";
-        let encrypted = SecretManager::encrypt_value_with_key(plaintext, &key).unwrap();
-
-        // Verify format
-        assert!(encrypted.contains(':'));
-        let parts: Vec<&str> = encrypted.split(':').collect();
-        assert_eq!(parts.len(), 2);
-
-        // Decrypt and verify
-        let decrypted = SecretManager::decrypt_value(&encrypted, &key).unwrap();
-        assert_eq!(decrypted, plaintext);
+    fn test_encrypt_decrypt_roundtrip_object() {
+        let value = serde_json::json!({"user": "admin", "password": "s3cret"});
+        let encrypted = crypto::encrypt_json(&value, TEST_KEY).unwrap();
+        let decrypted = crypto::decrypt_json(&encrypted, TEST_KEY).unwrap();
+        assert_eq!(value, decrypted);
     }
 
     #[test]
-    fn test_encrypt_decrypt_different_values() {
-        let key = derive_test_key("test-encryption-key-12345");
+    fn test_encrypt_produces_different_ciphertext() {
+        let value = serde_json::json!("my-secret-value");
+        let encrypted1 = crypto::encrypt_json(&value, TEST_KEY).unwrap();
+        let encrypted2 = crypto::encrypt_json(&value, TEST_KEY).unwrap();
 
-        let plaintext1 = "secret1";
-        let plaintext2 = "secret2";
-
-        let encrypted1 = SecretManager::encrypt_value_with_key(plaintext1, &key).unwrap();
-        let encrypted2 = SecretManager::encrypt_value_with_key(plaintext2, &key).unwrap();
-
-        // Encrypted values should be different (due to random nonces)
+        // Different ciphertexts due to random nonces
         assert_ne!(encrypted1, encrypted2);
 
-        // Both should decrypt correctly
-        let decrypted1 = SecretManager::decrypt_value(&encrypted1, &key).unwrap();
-        let decrypted2 = SecretManager::decrypt_value(&encrypted2, &key).unwrap();
-
-        assert_eq!(decrypted1, plaintext1);
-        assert_eq!(decrypted2, plaintext2);
+        // Both decrypt to the same value
+        assert_eq!(crypto::decrypt_json(&encrypted1, TEST_KEY).unwrap(), value);
+        assert_eq!(crypto::decrypt_json(&encrypted2, TEST_KEY).unwrap(), value);
     }
 
     #[test]
-    fn test_decrypt_with_wrong_key() {
-        let key1 = derive_test_key("key1");
-        let key2 = derive_test_key("key2");
+    fn test_decrypt_with_wrong_key_fails() {
+        let value = serde_json::json!("secret");
+        let encrypted = crypto::encrypt_json(&value, TEST_KEY).unwrap();
 
-        let plaintext = "secret";
-        let encrypted = SecretManager::encrypt_value_with_key(plaintext, &key1).unwrap();
-
-        // Decrypting with wrong key should fail
-        let result = SecretManager::decrypt_value(&encrypted, &key2);
-        assert!(result.is_err());
+        let wrong_key = "wrong_key_that_is_also_32_chars_long!!!";
+        assert!(crypto::decrypt_json(&encrypted, wrong_key).is_err());
     }
 
+    // ── prepare_secret_env ────────────────────────────────────────
+
     #[test]
     fn test_prepare_secret_env() {
-        // Test the static method directly without creating a SecretManager instance
-        let mut secrets = HashMap::new();
-        secrets.insert("api_key".to_string(), "secret123".to_string());
-        secrets.insert("db-password".to_string(), "pass456".to_string());
-        secrets.insert("oauth_token".to_string(), "token789".to_string());
+        let mut secrets: HashMap<String, JsonValue> = HashMap::new();
+        secrets.insert(
+            "api_key".to_string(),
+            JsonValue::String("secret123".to_string()),
+        );
+        secrets.insert(
+            "db-password".to_string(),
+            JsonValue::String("pass456".to_string()),
+        );
+        secrets.insert(
+            "oauth_token".to_string(),
+            JsonValue::String("token789".to_string()),
+        );
 
-        // Call prepare_secret_env as a static-like method
+        // Replicate the logic without constructing a full SecretManager
         let env: HashMap<String, String> = secrets
             .iter()
             .map(|(name, value)| {
                 let env_name = format!("SECRET_{}", name.to_uppercase().replace('-', "_"));
-                (env_name, value.clone())
+                let env_value = match value {
+                    JsonValue::String(s) => s.clone(),
+                    other => other.to_string(),
+                };
+                (env_name, env_value)
             })
             .collect();
 
@@ -352,35 +296,47 @@ mod tests {
     }
 
     #[test]
-    fn test_compute_key_hash() {
-        let key1 = derive_test_key("test-key");
-        let key2 = derive_test_key("test-key");
-        let key3 = derive_test_key("different-key");
+    fn test_prepare_secret_env_structured_value() {
+        let mut secrets: HashMap<String, JsonValue> = HashMap::new();
+        secrets.insert(
+            "db_config".to_string(),
+            serde_json::json!({"host": "db.example.com", "port": 5432}),
+        );
 
-        let hash1 = SecretManager::compute_key_hash_from_bytes(&key1);
-        let hash2 = SecretManager::compute_key_hash_from_bytes(&key2);
-        let hash3 = SecretManager::compute_key_hash_from_bytes(&key3);
+        let env: HashMap<String, String> = secrets
+            .iter()
+            .map(|(name, value)| {
+                let env_name = format!("SECRET_{}", name.to_uppercase().replace('-', "_"));
+                let env_value = match value {
+                    JsonValue::String(s) => s.clone(),
+                    other => other.to_string(),
+                };
+                (env_name, env_value)
+            })
+            .collect();
 
-        // Same key should produce same hash
+        // Structured values should be serialised to compact JSON
+        let db_config = env.get("SECRET_DB_CONFIG").unwrap();
+        let parsed: serde_json::Value = serde_json::from_str(db_config).unwrap();
+        assert_eq!(parsed["host"], "db.example.com");
+        assert_eq!(parsed["port"], 5432);
+    }
+
+    // ── compute_key_hash ──────────────────────────────────────────
+
+    #[test]
+    fn test_compute_key_hash_consistent() {
+        let hash1 = crypto::hash_encryption_key(TEST_KEY);
+        let hash2 = crypto::hash_encryption_key(TEST_KEY);
         assert_eq!(hash1, hash2);
-        // Different key should produce different hash
-        assert_ne!(hash1, hash3);
-        // Hash should not be empty
-        assert!(!hash1.is_empty());
+        // SHA-256 → 64 hex characters
+        assert_eq!(hash1.len(), 64);
     }
 
     #[test]
-    fn test_invalid_encrypted_format() {
-        let key = derive_test_key("test-key");
-
-        // Invalid formats should fail
-        let result = SecretManager::decrypt_value("no-colon", &key);
-        assert!(result.is_err());
-
-        let result = SecretManager::decrypt_value("too:many:colons", &key);
-        assert!(result.is_err());
-
-        let result = SecretManager::decrypt_value("invalid-base64:also-invalid", &key);
-        assert!(result.is_err());
+    fn test_compute_key_hash_different_keys() {
+        let hash1 = crypto::hash_encryption_key(TEST_KEY);
+        let hash2 = crypto::hash_encryption_key("different_key_that_is_32_chars_long!!");
+        assert_ne!(hash1, hash2);
     }
 }