harden file writes and reject empty kv/queue capability values (#35)

Author: markthom-as

cli/provenact-cli/src/fileio.rs

@@ -35,5 +35,58 @@ pub fn read_file_limited(
 }
 
 pub fn write_file(path: &Path, bytes: &[u8]) -> Result<(), String> {
+    match fs::symlink_metadata(path) {
+        Ok(meta) if meta.file_type().is_symlink() => {
+            return Err(format!(
+                "refusing to write through symlink: {}",
+                path.display()
+            ))
+        }
+        Ok(_) => {}
+        Err(err) if err.kind() == std::io::ErrorKind::NotFound => {}
+        Err(err) => return Err(format!("{}: {err}", path.display())),
+    }
     fs::write(path, bytes).map_err(|e| format!("{}: {e}", path.display()))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::path::PathBuf;
+    use std::time::{SystemTime, UNIX_EPOCH};
+
+    fn test_temp_dir(prefix: &str) -> PathBuf {
+        let mut path = std::env::temp_dir();
+        let nonce = SystemTime::now()
+            .duration_since(UNIX_EPOCH)
+            .map(|duration| duration.as_nanos())
+            .unwrap_or(0);
+        path.push(format!("{prefix}-{}-{nonce}", std::process::id()));
+        fs::create_dir_all(&path).expect("temp dir should be created");
+        path
+    }
+
+    #[test]
+    fn write_file_writes_regular_paths() {
+        let dir = test_temp_dir("provenact-fileio-write");
+        let path = dir.join("regular.txt");
+        write_file(&path, b"hello").expect("regular path write should succeed");
+        assert_eq!(fs::read(&path).expect("read"), b"hello");
+        let _ = fs::remove_dir_all(&dir);
+    }
+
+    #[cfg(unix)]
+    #[test]
+    fn write_file_rejects_symlink_targets() {
+        let dir = test_temp_dir("provenact-fileio-symlink");
+        let target = dir.join("target.txt");
+        let link = dir.join("link.txt");
+        fs::write(&target, b"original").expect("target write");
+        std::os::unix::fs::symlink(&target, &link).expect("symlink create");
+
+        let err = write_file(&link, b"replacement").expect_err("symlink writes must fail");
+        assert!(err.contains("refusing to write through symlink"));
+        assert_eq!(fs::read(&target).expect("target read"), b"original");
+        let _ = fs::remove_dir_all(&dir);
+    }
+}

core/verifier/src/lib.rs

@@ -973,6 +973,9 @@ fn is_capability_allowed(capability: &Capability, ceiling: &CapabilityCeiling) -
         "time.now" => !capability.value.is_empty() && ceiling.time.unwrap_or(false),
         "random.bytes" => !capability.value.is_empty() && ceiling.random.unwrap_or(false),
         "kv.read" => {
+            if capability.value.is_empty() {
+                return false;
+            }
             let Some(kv) = &ceiling.kv else {
                 return false;
             };
@@ -984,6 +987,9 @@ fn is_capability_allowed(capability: &Capability, ceiling: &CapabilityCeiling) -
                 .any(|item| item == "*" || item == &capability.value)
         }
         "kv.write" => {
+            if capability.value.is_empty() {
+                return false;
+            }
             let Some(kv) = &ceiling.kv else {
                 return false;
             };
@@ -995,6 +1001,9 @@ fn is_capability_allowed(capability: &Capability, ceiling: &CapabilityCeiling) -
                 .any(|item| item == "*" || item == &capability.value)
         }
         "queue.publish" => {
+            if capability.value.is_empty() {
+                return false;
+            }
             let Some(queue) = &ceiling.queue else {
                 return false;
             };
@@ -1006,6 +1015,9 @@ fn is_capability_allowed(capability: &Capability, ceiling: &CapabilityCeiling) -
                 .any(|item| item == "*" || item == &capability.value)
         }
         "queue.consume" => {
+            if capability.value.is_empty() {
+                return false;
+            }
             let Some(queue) = &ceiling.queue else {
                 return false;
             };
@@ -1171,13 +1183,23 @@ fn validate_policy_constraints(policy: &Policy) -> Result<(), VerifyError> {
     }
     if let Some(kv) = &ceiling.kv {
         if let Some(read) = &kv.read {
+            if read.iter().any(String::is_empty) {
+                return Err(VerifyError::PolicyConstraint(
+                    "capability_ceiling.kv.read items must be non-empty".to_string(),
+                ));
+            }
             if has_duplicates(read) {
                 return Err(VerifyError::PolicyConstraint(
                     "capability_ceiling.kv.read items must be unique".to_string(),
                 ));
             }
         }
         if let Some(write) = &kv.write {
+            if write.iter().any(String::is_empty) {
+                return Err(VerifyError::PolicyConstraint(
+                    "capability_ceiling.kv.write items must be non-empty".to_string(),
+                ));
+            }
             if has_duplicates(write) {
                 return Err(VerifyError::PolicyConstraint(
                     "capability_ceiling.kv.write items must be unique".to_string(),
@@ -1187,13 +1209,23 @@ fn validate_policy_constraints(policy: &Policy) -> Result<(), VerifyError> {
     }
     if let Some(queue) = &ceiling.queue {
         if let Some(publish) = &queue.publish {
+            if publish.iter().any(String::is_empty) {
+                return Err(VerifyError::PolicyConstraint(
+                    "capability_ceiling.queue.publish items must be non-empty".to_string(),
+                ));
+            }
             if has_duplicates(publish) {
                 return Err(VerifyError::PolicyConstraint(
                     "capability_ceiling.queue.publish items must be unique".to_string(),
                 ));
             }
         }
         if let Some(consume) = &queue.consume {
+            if consume.iter().any(String::is_empty) {
+                return Err(VerifyError::PolicyConstraint(
+                    "capability_ceiling.queue.consume items must be non-empty".to_string(),
+                ));
+            }
             if has_duplicates(consume) {
                 return Err(VerifyError::PolicyConstraint(
                     "capability_ceiling.queue.consume items must be unique".to_string(),
@@ -1756,6 +1788,68 @@ capability_ceiling:
         ));
     }
 
+    #[test]
+    fn denies_empty_kv_and_queue_capability_values_even_with_wildcards() {
+        let policy = Policy {
+            version: 1,
+            trusted_signers: vec!["alice.dev".to_string()],
+            capability_ceiling: CapabilityCeiling {
+                fs: None,
+                net: None,
+                kv: Some(PolicyKv {
+                    read: Some(vec!["*".to_string()]),
+                    write: Some(vec!["*".to_string()]),
+                }),
+                queue: Some(PolicyQueue {
+                    publish: Some(vec!["*".to_string()]),
+                    consume: Some(vec!["*".to_string()]),
+                }),
+                env: None,
+                exec: Some(false),
+                time: Some(false),
+                random: Some(false),
+            },
+        };
+        let requested = vec![
+            Capability {
+                kind: "kv.read".to_string(),
+                value: String::new(),
+            },
+            Capability {
+                kind: "kv.write".to_string(),
+                value: String::new(),
+            },
+            Capability {
+                kind: "queue.publish".to_string(),
+                value: String::new(),
+            },
+            Capability {
+                kind: "queue.consume".to_string(),
+                value: String::new(),
+            },
+        ];
+        assert!(matches!(
+            enforce_capability_ceiling(&requested, &policy),
+            Err(VerifyError::CapabilityDenied(_))
+        ));
+    }
+
+    #[test]
+    fn denies_policy_with_empty_kv_or_queue_entries() {
+        let raw = br#"{
+          "version": 1,
+          "trusted_signers": ["alice.dev"],
+          "capability_ceiling": {
+            "kv": { "read": [""] },
+            "queue": { "publish": ["*"] }
+          }
+        }"#;
+        assert!(matches!(
+            parse_policy_document(raw),
+            Err(VerifyError::PolicyConstraint(_))
+        ));
+    }
+
     #[test]
     fn denies_policy_net_prefix_with_query_or_fragment() {
         let raw = br#"{