fix resync carrier validation and live watches

Author: schickling

crates/agent-spec/src/spec.rs

@@ -771,35 +771,63 @@ fn validate_resource_uri(uri: &str) -> Result<(), &'static str> {
     if uri.is_empty() {
         return Err("catalog-relative uri must be a non-empty relative path");
     }
-    let decoded = percent_decode_path(uri)?;
-    if decoded.starts_with(b"/") || decoded.split(|byte| *byte == b'/').any(|part| part == b"..") {
+    let decoded = decode_percent_path(uri)?;
+    if decoded.starts_with('/') || decoded.split('/').any(|part| part == "..") {
         return Err(
             "catalog-relative uri must be a relative path without parent (`..`) components",
         );
     }
     Ok(())
 }
 
-fn percent_decode_path(uri: &str) -> Result<Vec<u8>, &'static str> {
-    let bytes = uri.as_bytes();
+/// Decode filesystem-path percent escapes without allowing an escape to change path segmentation.
+///
+/// Consumers may apply additional policy to the decoded path (for example, catalog-relative
+/// resources reject every parent component). This shared boundary rejects bytes that cannot name
+/// the same UTF-8 filesystem path the resource URI visibly denotes.
+pub fn decode_percent_path(encoded_path: &str) -> Result<String, &'static str> {
+    let bytes = encoded_path.as_bytes();
     let mut decoded = Vec::with_capacity(bytes.len());
+    let mut component_start = 0;
+    let mut component_had_escape = false;
     let mut offset = 0;
     while offset < bytes.len() {
+        if bytes[offset] == b'/' {
+            if component_had_escape && decoded[component_start..] == *b".." {
+                return Err("path contains an encoded parent (`..`) component");
+            }
+            decoded.push(b'/');
+            component_start = decoded.len();
+            component_had_escape = false;
+            offset += 1;
+            continue;
+        }
         if bytes[offset] != b'%' {
             decoded.push(bytes[offset]);
             offset += 1;
             continue;
         }
         let Some(high) = bytes.get(offset + 1).and_then(|byte| hex_digit(*byte)) else {
-            return Err("catalog-relative uri contains an invalid percent escape");
+            return Err("path contains an invalid percent escape");
         };
         let Some(low) = bytes.get(offset + 2).and_then(|byte| hex_digit(*byte)) else {
-            return Err("catalog-relative uri contains an invalid percent escape");
+            return Err("path contains an invalid percent escape");
         };
-        decoded.push((high << 4) | low);
+        let byte = (high << 4) | low;
+        if matches!(byte, b'/' | b'\\') {
+            return Err("path contains an encoded separator");
+        }
+        if byte == b'\0' {
+            return Err("path decodes to NUL");
+        }
+        decoded.push(byte);
+        component_had_escape = true;
         offset += 3;
     }
-    Ok(decoded)
+    if component_had_escape && decoded[component_start..] == *b".." {
+        return Err("path contains an encoded parent (`..`) component");
+    }
+    String::from_utf8(decoded).map_err(|_| "percent-decoded path is not valid UTF-8")
 }
 
 fn hex_digit(byte: u8) -> Option<u8> {
@@ -1407,12 +1435,13 @@ mod tests {
     }
 
     #[test]
-    fn catalog_relative_uris_reject_only_parent_components_after_percent_decoding() {
+    fn catalog_relative_uris_share_filesystem_safe_percent_decoding() {
         for uri in [
             "report..md",
             "reports/.../goal.md",
             "reports/%2Ereport.md",
             "reports/child..name/goal.md",
+            "reports/with%20space/%E2%82%AC.md",
         ] {
             assert_eq!(validate_resource_uri(uri), Ok(()), "{uri}");
         }
@@ -1424,6 +1453,11 @@ mod tests {
             "%2e%2e/goal.md",
             "reports/%2E%2e/goal.md",
             "reports%2f..%2fgoal.md",
+            "reports/encoded%2Fseparator",
+            "reports/encoded%5cseparator",
+            "reports/a%00b",
+            "reports/%FF.md",
+            "reports/bad%escape",
         ] {
             assert!(validate_resource_uri(uri).is_err(), "{uri}");
         }

crates/agent-spec/tests/discovery.rs

@@ -1256,6 +1256,108 @@ fn catalog_relative_resource_uris_are_an_st2_extension_resolved_against_the_decl
     assert!(absolute.is_err(), "absolute paths must keep a scheme");
 }
 
+#[test]
+fn resource_percent_paths_are_validated_consistently_across_public_inputs() {
+    let tmp = tempfile::tempdir().unwrap();
+    write(
+        tmp.path(),
+        "agents/h/valid-kdl/agent.kdl",
+        r#"agent "valid-kdl" {
+  host "h"
+  command "true"
+  resource "work" uri="resources/with%20space.md" reason="Task."
+}"#,
+    );
+    write(
+        tmp.path(),
+        "agents/h/invalid-kdl/agent.kdl",
+        r#"agent "invalid-kdl" {
+  host "h"
+  command "true"
+  resource "work" uri="resources/a%00b" reason="Task."
+}"#,
+    );
+    write(
+        tmp.path(),
+        "agents/h/valid-json/agent.json",
+        r#"{
+  "identity": "valid-json",
+  "host": "h",
+  "command": "true",
+  "resource": {"work": {"uri": "resources/with%20space.md", "reason": "Task."}}
+}"#,
+    );
+    write(
+        tmp.path(),
+        "agents/h/invalid-json/agent.json",
+        r#"{
+  "identity": "invalid-json",
+  "host": "h",
+  "command": "true",
+  "resource": {"work": {"uri": "resources/encoded%2Fseparator", "reason": "Task."}}
+}"#,
+    );
+    write(
+        tmp.path(),
+        "agents/h/valid-toml/agent.toml",
+        r#"identity = "valid-toml"
+host = "h"
+command = "true"
+[resource.work]
+uri = "resources/with%20space.md"
+reason = "Task."
+"#,
+    );
+    write(
+        tmp.path(),
+        "agents/h/invalid-toml/agent.toml",
+        r#"identity = "invalid-toml"
+host = "h"
+command = "true"
+[resource.work]
+uri = "resources/%FF.md"
+reason = "Task."
+"#,
+    );
+
+    let found = discover(tmp.path());
+    let identities = found
+        .specs
+        .iter()
+        .map(|spec| spec.identity.as_str())
+        .collect::<Vec<_>>();
+    assert_eq!(
+        identities,
+        vec!["valid-json", "valid-kdl", "valid-toml"],
+        "{:?}",
+        found.errors
+    );
+    assert_eq!(found.errors.len(), 3, "{:?}", found.errors);
+
+    let valid: Resource = serde_json::from_str(
+        r#"{"name":"work","uri":"resources/with%20space/%E2%82%AC.md","reason":"Task."}"#,
+    )
+    .unwrap();
+    assert_eq!(
+        valid.uri(),
+        "resources/with%20space/%E2%82%AC.md",
+        "validation must preserve the URI identity instead of normalizing it"
+    );
+    for uri in [
+        "resources/a%00b",
+        "resources/encoded%2Fseparator",
+        "resources/encoded%5cseparator",
+        "resources/%FF.md",
+        "resources/%2e%2e/goal.md",
+    ] {
+        let descriptor = format!(r#"{{"name":"work","uri":"{uri}","reason":"Task."}}"#);
+        assert!(
+            serde_json::from_str::<Resource>(&descriptor).is_err(),
+            "{uri}"
+        );
+    }
+}
+
 #[test]
 fn duplicate_json_resource_names_are_rejected_instead_of_last_write_winning() {
     let tmp = tempfile::tempdir().unwrap();

src/resync.rs

@@ -18,7 +18,7 @@ use std::time::{Duration, Instant};
 use notify::Watcher as _;
 use sha2::{Digest as _, Sha256};
 
-use agent_spec::spec::AgentSpec;
+use agent_spec::spec::{AgentSpec, decode_percent_path};
 
 /// The reserved stream used only by the supervisor's crate-internal resync publisher.
 pub const RESYNC_STREAM: &str = "resync";
@@ -127,62 +127,13 @@ fn resolve_local_path(agent_dir: &Path, uri: &str) -> Option<PathBuf> {
         {
             return None;
         }
-        let path = decode_percent_path(encoded_path)?;
+        let path = PathBuf::from(decode_percent_path(encoded_path).ok()?);
         return path.is_absolute().then(|| lexical_clean(&path));
     }
-    let path = decode_percent_path(uri)?;
+    let path = PathBuf::from(decode_percent_path(uri).ok()?);
     Some(lexical_clean(&agent_dir.join(path)))
 }
 
-fn decode_percent_path(encoded_path: &str) -> Option<PathBuf> {
-    let bytes = encoded_path.as_bytes();
-    let mut decoded = Vec::with_capacity(bytes.len());
-    let mut component_start = 0;
-    let mut component_had_escape = false;
-    let mut index = 0;
-    while index < bytes.len() {
-        if bytes[index] == b'/' {
-            if component_had_escape && decoded[component_start..] == *b".." {
-                return None;
-            }
-            decoded.push(b'/');
-            component_start = decoded.len();
-            component_had_escape = false;
-            index += 1;
-            continue;
-        }
-        if bytes[index] != b'%' {
-            decoded.push(bytes[index]);
-            index += 1;
-            continue;
-        }
-        let high = hex_value(*bytes.get(index + 1)?)?;
-        let low = hex_value(*bytes.get(index + 2)?)?;
-        let byte = (high << 4) | low;
-        // A decoded separator changes URI path segmentation, and NUL cannot be a filesystem path
-        // byte. Reject both rather than silently resolving a different carrier.
-        if matches!(byte, b'/' | b'\\' | b'\0') {
-            return None;
-        }
-        decoded.push(byte);
-        component_had_escape = true;
-        index += 3;
-    }
-    if component_had_escape && decoded[component_start..] == *b".." {
-        return None;
-    }
-    Some(PathBuf::from(String::from_utf8(decoded).ok()?))
-}
-
-fn hex_value(byte: u8) -> Option<u8> {
-    match byte {
-        b'0'..=b'9' => Some(byte - b'0'),
-        b'a'..=b'f' => Some(byte - b'a' + 10),
-        b'A'..=b'F' => Some(byte - b'A' + 10),
-        _ => None,
-    }
-}
-
 /// Remove `.` and `..` components lexically. This deliberately does not inspect the filesystem:
 /// classification follows the authored path structure without resolving symlinks.
 fn lexical_clean(path: &Path) -> PathBuf {
@@ -2195,10 +2146,14 @@ mod tests {
             "file:///tmp/encoded%5Cseparator",
             "file:///tmp/bad%escape",
             "file:///tmp/%2E%2E/escape",
+            "file:///tmp/a%00b",
+            "file:///tmp/%FF.md",
             "resources/encoded%2Fseparator",
             "resources/encoded%5Cseparator",
             "resources/%2E%2E/outside.md",
             "resources/bad%escape",
+            "resources/a%00b",
+            "resources/%FF.md",
             "http://x/y",
             "worktree://repo/main",
             "GitHub-Issue://org/repo/41",