fix: security and code quality improvements (SONAR:dallay_corvus)

.agents/skills/kotlin-multiplatform/scripts/validate-kmp-structure.sh

@@ -63,24 +63,20 @@ fi
 # Check 4: Unmatched expect/actual declarations
 echo
 echo "📋 Checking expect/actual pairs..."
-expect_files_count=$(
-    find ./*/src/commonMain -name "*.kt" -print0 2>/dev/null \
-        | xargs -0 grep -l --null "^expect " 2>/dev/null \
-        | tr -cd '\0' \
-        | wc -c
-)
+expect_files_count=$(find ./*/src/commonMain -name "*.kt" -print0 2>/dev/null | xargs -0 grep -l --null -E "^\s*expect\s+(class|object|fun|interface)\b" 2>/dev/null | tr -cd '\0' | wc -c)
 if [[ "$expect_files_count" -gt 0 ]]; then
     while IFS= read -r -d '' file; do
         # Extract declarations
-        expects=$(grep "^expect \(class\|object\|fun\|interface\)" "$file" | sed 's/expect //' | awk '{print $2}' | sed 's/[({].*$//')
+        expects=$(grep -E "^\s*expect\s+(class|object|fun|interface)\b" "$file" | sed -E 's/^\s*expect\s+(class|object|fun|interface)\s+//' | awk '{print $1}' | sed 's/[({].*$//')
 
         # Check for actuals in platform source sets
         for expect_name in $expects; do
             actual_count=0
             for platform in androidMain jvmMain iosMain; do
                 platform_dir=$(dirname "$file" | sed "s/commonMain/$platform/")
+                ESCAPED_EXPECT_NAME=$(echo "$expect_name" | sed 's/\./\\./g')
                 platform_file="${platform_dir}/$(basename "$file")"
-                if [[ -f "$platform_file" ]] && grep -q "actual.*$expect_name" "$platform_file"; then
+                if [[ -f "$platform_file" ]] && grep -E -q "actual[[:space:]]+(class|object|fun|interface)[[:space:]]+${ESCAPED_EXPECT_NAME}([[:space:]]|\b|\()" "$platform_file"; then
                     actual_count=$((actual_count + 1))
                 fi
             done
@@ -91,10 +87,7 @@ if [[ "$expect_files_count" -gt 0 ]]; then
                 ISSUES_FOUND=$((ISSUES_FOUND + 1))
             fi
         done
-    done < <(
-        find ./*/src/commonMain -name "*.kt" -print0 2>/dev/null \
-            | xargs -0 grep -l --null "^expect " 2>/dev/null || true
-    )
+    done < <(find ./*/src/commonMain -name "*.kt" -print0 2>/dev/null | xargs -0 grep -l --null -E "^\s*expect\s+(class|object|fun|interface)\b" 2>/dev/null || true)
 else
     echo -e "${GREEN}✓${NC} No expect declarations to validate"
 fi
@@ -106,17 +99,16 @@ echo "📋 Checking for potential code duplication..."
 common_functions=$(find ./*/src/commonMain -name "*.kt" -print0 2>/dev/null | xargs -0 grep -h "^fun " | awk '{print $2}' | sed 's/[({<].*$//' | sort -u || true)
 if [[ -n "$common_functions" ]]; then
     for func in $common_functions; do
+        ESCAPED_FUNC=$(echo "$func" | sed 's/\./\\./g')
         android_count=$(
             find ./*/src/androidMain -name "*.kt" -print0 2>/dev/null \
-                | xargs -0 grep -l --null "^fun $func" 2>/dev/null \
-                | tr -cd '\0' \
-                | wc -c
+                | xargs -0 grep -l --null -E "^fun[[:space:]]+${ESCAPED_FUNC}" 2>/dev/null \
+                | tr -cd '\0' | wc -c
         )
         jvm_count=$(
             find ./*/src/jvmMain -name "*.kt" -print0 2>/dev/null \
-                | xargs -0 grep -l --null "^fun $func" 2>/dev/null \
-                | tr -cd '\0' \
-                | wc -c
+                | xargs -0 grep -l --null -E "^fun[[:space:]]+${ESCAPED_FUNC}" 2>/dev/null \
+                | tr -cd '\0' | wc -c
         )
 
         if [[ "$android_count" -gt 0 ]] && [[ "$jvm_count" -gt 0 ]]; then

clients/agent-runtime/src/identity.rs

@@ -4,6 +4,7 @@
 //! portable AI identity. This module handles loading and converting AIEOS v1.1
 //! JSON to Corvus's system prompt format.
 
+use std::path::PathBuf;
 use crate::config::IdentityConfig;
 use anyhow::{Context, Result};
 use serde::{Deserialize, Serialize};
@@ -166,13 +167,26 @@ pub fn load_aieos_identity(
 
     // Try aieos_path first
     if let Some(ref path) = config.aieos_path {
+        let workspace_canonical = std::fs::canonicalize(workspace_dir)
+            .with_context(|| format!("Failed to canonicalize workspace directory: {}", workspace_dir.display()))?;
+
         let full_path = if Path::new(path).is_absolute() {
             PathBuf::from(path)
         } else {
             workspace_dir.join(path)
         };
 
-        let content = std::fs::read_to_string(&full_path)
+        let full_path_canonical = std::fs::canonicalize(&full_path)
+            .with_context(|| format!("Failed to canonicalize AIEOS path: {}", full_path.display()))?;
+
+        if !full_path_canonical.starts_with(&workspace_canonical) {
+            anyhow::bail!(
+                "Security error: AIEOS path {} is outside the workspace directory {}",
+                full_path.display(),
+                workspace_dir.display()
+            );
+        }
+        let content = std::fs::read_to_string(&full_path_canonical)
             .with_context(|| format!("Failed to read AIEOS file: {}", full_path.display()))?;
 
         let identity: AieosIdentity = serde_json::from_str(&content)
@@ -206,8 +220,6 @@ pub fn load_aieos_identity(
     )
 }
 
-use std::path::PathBuf;
-
 /// Convert AIEOS identity to a system prompt string.
 ///
 /// Formats the AIEOS data into a structured markdown prompt compatible

clients/agent-runtime/src/tools/browser.rs

@@ -214,7 +214,7 @@ impl BrowserTool {
     ) -> Self {
         Self {
             security,
-            allowed_domains: normalize_domains(allowed_domains),
+            allowed_domains: normalize_domains(allowed_domains).expect("Invalid allowlist entries"),
             session_name,
             backend,
             native_headless,
@@ -1933,12 +1933,14 @@ fn unavailable_action_for_backend_error(action: &str, backend: ResolvedBackend)
     )
 }
 
-fn normalize_domains(domains: Vec<String>) -> Vec<String> {
-    domains
-        .into_iter()
-        .map(|d| d.trim().to_lowercase())
-        .filter(|d| !d.is_empty())
-        .collect()
+fn normalize_domains(domains: Vec<String>) -> anyhow::Result<Vec<String>> {
+    let mut normalized = Vec::new();
+    for domain in domains {
+        normalized.push(crate::tools::url_safety::normalize_domain(&domain)?);
+    }
+    normalized.sort_unstable();
+    normalized.dedup();
+    Ok(normalized)
 }
 
 fn endpoint_reachable(endpoint: &reqwest::Url, timeout: Duration) -> bool {
@@ -2085,7 +2087,7 @@ mod tests {
             "docs.example.com".into(),
             String::new(),
         ];
-        let normalized = normalize_domains(domains);
+        let normalized = normalize_domains(domains).unwrap();
         assert_eq!(normalized, vec!["example.com", "docs.example.com"]);
     }
 

clients/agent-runtime/src/tools/browser_open.rs

@@ -17,7 +17,7 @@ impl BrowserOpenTool {
     pub fn new(security: Arc<SecurityPolicy>, allowed_domains: Vec<String>) -> Self {
         Self {
             security,
-            allowed_domains: normalize_allowed_domains(allowed_domains),
+            allowed_domains: normalize_allowed_domains(allowed_domains).expect("Invalid allowlist entries"),
         }
     }
 
@@ -258,7 +258,7 @@ mod tests {
             "EXAMPLE.COM".into(),
             "https://example.com/".into(),
         ]);
-        assert_eq!(got, vec!["example.com".to_string()]);
+        assert_eq!(got.unwrap(), vec!["example.com".to_string()]);
     }
 
     #[test]

clients/agent-runtime/src/tools/http_request.rs

@@ -70,7 +70,7 @@ impl HttpRequestTool {
     ) -> Self {
         Self {
             security,
-            allowed_domains: normalize_allowed_domains(allowed_domains),
+            allowed_domains: normalize_allowed_domains(allowed_domains).expect("Invalid allowlist entries"),
             max_response_size,
             timeout_secs,
         }
@@ -472,7 +472,7 @@ mod tests {
             "EXAMPLE.COM".into(),
             "https://example.com/".into(),
         ]);
-        assert_eq!(got, vec!["example.com".to_string()]);
+        assert_eq!(got.unwrap(), vec!["example.com".to_string()]);
     }
 
     #[test]

clients/agent-runtime/src/tools/url_safety.rs

@@ -1,19 +1,19 @@
 use url::{Host, Url};
 
-pub(crate) fn normalize_allowed_domains(domains: Vec<String>) -> Vec<String> {
-    let mut normalized = domains
-        .into_iter()
-        .filter_map(|domain| normalize_domain(&domain))
-        .collect::<Vec<_>>();
+pub(crate) fn normalize_allowed_domains(domains: Vec<String>) -> anyhow::Result<Vec<String>> {
+    let mut normalized = Vec::new();
+    for domain in domains {
+        normalized.push(normalize_domain(&domain)?);
+    }
     normalized.sort_unstable();
     normalized.dedup();
-    normalized
+    Ok(normalized)
 }
 
-pub(crate) fn normalize_domain(raw: &str) -> Option<String> {
+pub(crate) fn normalize_domain(raw: &str) -> anyhow::Result<String> {
     let mut domain = raw.trim().to_lowercase();
     if domain.is_empty() {
-        return None;
+        anyhow::bail!("Domain cannot be empty");
     }
 
     if let Some(stripped) = domain.strip_prefix("https://") {
@@ -31,15 +31,15 @@ pub(crate) fn normalize_domain(raw: &str) -> Option<String> {
         .trim_end_matches('.')
         .to_string();
 
-    if let Some((host, _)) = domain.split_once(':') {
-        domain = host.to_string();
+    if domain.contains(':') {
+        anyhow::bail!("Domain cannot contain a port: {}", domain);
     }
 
     if domain.is_empty() || domain.chars().any(char::is_whitespace) {
-        return None;
+        anyhow::bail!("Invalid domain: {}", domain);
     }
 
-    Some(domain)
+    Ok(domain)
 }
 
 pub(crate) fn extract_host(
@@ -82,3 +82,27 @@ pub(crate) fn host_matches_allowlist(host: &str, allowed_domains: &[String]) ->
                 .is_some_and(|prefix| prefix.ends_with('.'))
     })
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_normalize_domain() {
+        assert_eq!(normalize_domain("HTTPS://EXAMPLE.COM/path").unwrap(), "example.com".to_string());
+        assert!(normalize_domain("example.com:8080").is_err());
+        assert_eq!(normalize_domain("  .example.com.  ").unwrap(), "example.com".to_string());
+        assert!(normalize_domain("example com").is_err());
+        assert!(normalize_domain("").is_err());
+    }
+
+    #[test]
+    fn test_normalize_allowed_domains() {
+        let domains = vec!["example.com".into(), "EXAMPLE.COM".into(), "https://google.com/".into()];
+        let normalized = normalize_allowed_domains(domains).unwrap();
+        assert_eq!(normalized, vec!["example.com".to_string(), "google.com".to_string()]);
+
+        let bad_domains = vec!["example.com".into(), "localhost:8080".into()];
+        assert!(normalize_allowed_domains(bad_domains).is_err());
+    }
+}

clients/agent-runtime/tests/identity_security.rs

@@ -0,0 +1,64 @@
+use corvus::identity::load_aieos_identity;
+use corvus::config::IdentityConfig;
+use tempfile::tempdir;
+use std::fs;
+use std::os::unix::fs::symlink;
+
+#[test]
+fn test_load_aieos_identity_within_workspace() {
+    let workspace = tempdir().unwrap();
+    let identity_file = workspace.path().join("identity.json");
+    fs::write(&identity_file, "{\"identity\":{\"names\":{\"first\":\"Test\"}}}").unwrap();
+
+    let config = IdentityConfig {
+        format: "aieos".into(),
+        aieos_path: Some("identity.json".into()),
+        aieos_inline: None,
+    };
+
+    let result = load_aieos_identity(&config, workspace.path()).unwrap();
+    assert!(result.is_some());
+    assert_eq!(result.unwrap().identity.unwrap().names.unwrap().first.unwrap(), "Test");
+}
+
+#[test]
+fn test_load_aieos_identity_traversal_dots() {
+    let workspace = tempdir().unwrap();
+    let outside = tempdir().unwrap();
+    let identity_file = outside.path().join("identity.json");
+    fs::write(&identity_file, "{}").unwrap();
+
+    let relative_path = format!("../{}/identity.json",
+        outside.path().file_name().unwrap().to_str().unwrap());
+
+    let config = IdentityConfig {
+        format: "aieos".into(),
+        aieos_path: Some(relative_path),
+        aieos_inline: None,
+    };
+
+    let result = load_aieos_identity(&config, workspace.path());
+    assert!(result.is_err());
+    assert!(result.unwrap_err().to_string().contains("outside the workspace"));
+}
+
+#[test]
+fn test_load_aieos_identity_traversal_symlink() {
+    let workspace = tempdir().unwrap();
+    let outside = tempdir().unwrap();
+    let identity_file = outside.path().join("identity.json");
+    fs::write(&identity_file, "{}").unwrap();
+
+    let link_path = workspace.path().join("malicious_link.json");
+    symlink(&identity_file, &link_path).unwrap();
+
+    let config = IdentityConfig {
+        format: "aieos".into(),
+        aieos_path: Some("malicious_link.json".into()),
+        aieos_inline: None,
+    };
+
+    let result = load_aieos_identity(&config, workspace.path());
+    assert!(result.is_err());
+    assert!(result.unwrap_err().to_string().contains("outside the workspace"));
+}