diff --git a/docs/STDLIB_REFERENCE.md b/docs/STDLIB_REFERENCE.md index ca9a29e..5f17951 100644 --- a/docs/STDLIB_REFERENCE.md +++ b/docs/STDLIB_REFERENCE.md @@ -6661,7 +6661,7 @@ cache_fetch(cache_obj: Map, url_or_options: String | Map) -> Result Value { } fn cache_fetch(cache_id: u64, url: &str, opts: Option<&HashMap>) -> Result { + if opts.is_some_and(|options| options.values().any(Value::contains_secret)) { + return Err(IntentError::type_error( + "cache_fetch() cannot cache responses for secret-bearing requests; use fetch() directly" + .to_string(), + )); + } + // Check cache first { let mut registry = CACHE_REGISTRY.lock().unwrap(); @@ -1092,6 +1099,7 @@ pub fn init() -> HashMap { // Fetch a URL using a cache, returning a cached response if available. // // Checks the cache for a previously stored response matching the URL. + // Secret-bearing request options are rejected because cache keys do not include credentials. // On a cache miss, performs the HTTP request via fetch(), stores the // successful response in the cache, and returns it. This is the internal // function backing cache.fetch() method calls. @@ -1233,3 +1241,33 @@ pub fn init() -> HashMap { module } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cache_fetch_rejects_secret_bearing_options_before_cache_lookup() { + let canary = "cache-secret-canary"; + let options = HashMap::from([ + ( + "url".to_string(), + Value::String("http://127.0.0.1:1/never-requested".to_string()), + ), + ( + "headers".to_string(), + Value::Map(HashMap::from([( + "Authorization".to_string(), + Value::Secret( + crate::interpreter::SecretValue::new("CACHE_SECRET", canary) + .expect("valid secret"), + ), + )])), + ), + ]); + + let error = cache_fetch(0, "http://127.0.0.1:1/never-requested", Some(&options)) + .expect_err("secret-bearing requests must not use the response cache"); + assert!(!error.to_string().contains(canary)); + } +} diff --git a/src/stdlib/postgres.rs b/src/stdlib/postgres.rs index bf1f9a8..2fdb4c2 100644 --- a/src/stdlib/postgres.rs +++ b/src/stdlib/postgres.rs @@ -156,8 +156,14 @@ impl ToSql for SqlParam { } /// Convert an Intent Value to a SQL parameter -fn value_to_sql_param(value: &Value) -> SqlParam { - match value { +fn value_to_sql_param(value: &Value) -> Result { + if value.contains_secret() { + return Err(IntentError::type_error( + "PostgreSQL parameters cannot contain Secret values".to_string(), + )); + } + + Ok(match value { Value::Int(i) => { // Use i32 for smaller values, i64 for larger if *i >= i32::MIN as i64 && *i <= i32::MAX as i64 { @@ -206,7 +212,7 @@ fn value_to_sql_param(value: &Value) -> SqlParam { } } _ => SqlParam::String(format!("{}", value)), // Fallback to string representation - } + }) } /// Convert a tokio-postgres row to an Intent Map @@ -819,7 +825,10 @@ async fn pg_execute_cached( /// Execute a query using either the transaction client or a fresh pool connection fn pg_query(conn: &Value, sql: &str, params: &[Value]) -> Result { - let sql_params: Vec = params.iter().map(value_to_sql_param).collect(); + let sql_params: Vec = params + .iter() + .map(value_to_sql_param) + .collect::>()?; let param_refs: Vec<&(dyn ToSql + Sync)> = sql_params .iter() .map(|p| p as &(dyn ToSql + Sync)) @@ -869,7 +878,10 @@ fn pg_query(conn: &Value, sql: &str, params: &[Value]) -> Result { /// Execute a query and return a single row (or null) fn pg_query_one(conn: &Value, sql: &str, params: &[Value]) -> Result { - let sql_params: Vec = params.iter().map(value_to_sql_param).collect(); + let sql_params: Vec = params + .iter() + .map(value_to_sql_param) + .collect::>()?; let param_refs: Vec<&(dyn ToSql + Sync)> = sql_params .iter() .map(|p| p as &(dyn ToSql + Sync)) @@ -913,7 +925,10 @@ fn pg_query_one(conn: &Value, sql: &str, params: &[Value]) -> Result { /// Execute a statement (INSERT/UPDATE/DELETE) and return affected row count fn pg_execute(conn: &Value, sql: &str, params: &[Value]) -> Result { - let sql_params: Vec = params.iter().map(value_to_sql_param).collect(); + let sql_params: Vec = params + .iter() + .map(value_to_sql_param) + .collect::>()?; let param_refs: Vec<&(dyn ToSql + Sync)> = sql_params .iter() .map(|p| p as &(dyn ToSql + Sync)) @@ -1473,27 +1488,39 @@ mod tests { #[test] fn test_value_to_sql_param() { // Test integer conversion - let param = value_to_sql_param(&Value::Int(42)); + let param = value_to_sql_param(&Value::Int(42)).expect("integer parameter"); match param { SqlParam::Int(v) => assert_eq!(v, 42), _ => panic!("Expected Int"), } // Test boolean conversion - let param = value_to_sql_param(&Value::Bool(true)); + let param = value_to_sql_param(&Value::Bool(true)).expect("boolean parameter"); match param { SqlParam::Bool(v) => assert!(v), _ => panic!("Expected Bool"), } // Test string conversion - let param = value_to_sql_param(&Value::String("test".to_string())); + let param = + value_to_sql_param(&Value::String("test".to_string())).expect("string parameter"); match param { SqlParam::String(v) => assert_eq!(v, "test"), _ => panic!("Expected String"), } } + #[test] + fn secret_values_are_rejected_from_postgres_parameters() { + let canary = "postgres-secret-canary"; + let value = Value::Array(vec![Value::Secret( + crate::interpreter::SecretValue::new("POSTGRES_SECRET", canary).expect("valid secret"), + )]); + + let error = value_to_sql_param(&value).expect_err("PostgreSQL must reject secrets"); + assert!(!error.to_string().contains(canary)); + } + #[test] fn test_pg_query_cached_recovers_from_stale_result_plan() { let Some(url) = std::env::var("NTNT_POSTGRES_TEST_URL") diff --git a/src/stdlib/sqlite.rs b/src/stdlib/sqlite.rs index 0739c7a..60ea1db 100644 --- a/src/stdlib/sqlite.rs +++ b/src/stdlib/sqlite.rs @@ -29,8 +29,14 @@ static CONNECTION_REGISTRY: std::sync::LazyLock rusqlite::types::Value { - match value { +fn value_to_sqlite(value: &Value) -> Result { + if value.contains_secret() { + return Err(IntentError::type_error( + "SQLite parameters cannot contain Secret values".to_string(), + )); + } + + Ok(match value { Value::Int(i) => rusqlite::types::Value::Integer(*i), Value::Float(f) => rusqlite::types::Value::Real(*f), Value::String(s) => rusqlite::types::Value::Text(s.clone()), @@ -40,7 +46,7 @@ fn value_to_sqlite(value: &Value) -> rusqlite::types::Value { enum_name, variant, .. } if enum_name == "Option" && variant == "None" => rusqlite::types::Value::Null, _ => rusqlite::types::Value::Text(format!("{}", value)), - } + }) } /// Convert a SQLite ValueRef to an Intent Value @@ -121,7 +127,8 @@ fn sqlite_query(conn: &Value, sql: &str, params: &[Value]) -> Result { .lock() .map_err(|e| IntentError::runtime_error(format!("Failed to lock connection: {}", e)))?; - let sqlite_params: Vec = params.iter().map(value_to_sqlite).collect(); + let sqlite_params: Vec = + params.iter().map(value_to_sqlite).collect::>()?; let mut stmt = conn_guard .prepare(sql) @@ -162,7 +169,8 @@ fn sqlite_query_one(conn: &Value, sql: &str, params: &[Value]) -> Result .lock() .map_err(|e| IntentError::runtime_error(format!("Failed to lock connection: {}", e)))?; - let sqlite_params: Vec = params.iter().map(value_to_sqlite).collect(); + let sqlite_params: Vec = + params.iter().map(value_to_sqlite).collect::>()?; let mut stmt = conn_guard .prepare(sql) @@ -193,7 +201,8 @@ fn sqlite_execute(conn: &Value, sql: &str, params: &[Value]) -> Result { .lock() .map_err(|e| IntentError::runtime_error(format!("Failed to lock connection: {}", e)))?; - let sqlite_params: Vec = params.iter().map(value_to_sqlite).collect(); + let sqlite_params: Vec = + params.iter().map(value_to_sqlite).collect::>()?; match conn_guard.execute(sql, rusqlite::params_from_iter(sqlite_params.iter())) { Ok(count) => Ok(Value::ok(Value::Int(count as i64))), @@ -937,36 +946,51 @@ mod tests { #[test] fn test_value_to_sqlite_conversion() { - match value_to_sqlite(&Value::Int(42)) { + match value_to_sqlite(&Value::Int(42)).expect("integer parameter") { rusqlite::types::Value::Integer(v) => assert_eq!(v, 42), _ => panic!("Expected Integer"), } - match value_to_sqlite(&Value::Float(3.14)) { + match value_to_sqlite(&Value::Float(3.14)).expect("float parameter") { rusqlite::types::Value::Real(v) => assert!((v - 3.14).abs() < f64::EPSILON), _ => panic!("Expected Real"), } - match value_to_sqlite(&Value::String("hello".to_string())) { + match value_to_sqlite(&Value::String("hello".to_string())).expect("string parameter") { rusqlite::types::Value::Text(v) => assert_eq!(v, "hello"), _ => panic!("Expected Text"), } - match value_to_sqlite(&Value::Bool(true)) { + match value_to_sqlite(&Value::Bool(true)).expect("boolean parameter") { rusqlite::types::Value::Integer(v) => assert_eq!(v, 1), _ => panic!("Expected Integer for true"), } - match value_to_sqlite(&Value::Bool(false)) { + match value_to_sqlite(&Value::Bool(false)).expect("boolean parameter") { rusqlite::types::Value::Integer(v) => assert_eq!(v, 0), _ => panic!("Expected Integer for false"), } - match value_to_sqlite(&Value::Unit) { + match value_to_sqlite(&Value::Unit).expect("unit parameter") { rusqlite::types::Value::Null => {} _ => panic!("Expected Null for Unit"), } - match value_to_sqlite(&Value::none()) { + match value_to_sqlite(&Value::none()).expect("none parameter") { rusqlite::types::Value::Null => {} _ => panic!("Expected Null for None"), } } + #[test] + fn secret_values_are_rejected_from_sqlite_parameters() { + let canary = "sqlite-secret-canary"; + let value = Value::Map(HashMap::from([( + "token".to_string(), + Value::Secret( + crate::interpreter::SecretValue::new("SQLITE_SECRET", canary) + .expect("valid secret"), + ), + )])); + + let error = value_to_sqlite(&value).expect_err("SQLite must reject secrets"); + assert!(!error.to_string().contains(canary)); + } + #[test] fn test_error_on_invalid_sql() { let conn = match sqlite_connect(":memory:").unwrap() {