From 44304d06256ae95a1d487879a26e8739d7e46b83 Mon Sep 17 00:00:00 2001
From: Anders Olsson <anders.e.olsson@gmail.com>
Date: Fri, 7 Aug 2026 16:29:48 +0200
Subject: [PATCH] fix(protocol): reject one-line KDL blocks with multiple keys

In KDL, a `{ }` block's children are newline- or semicolon-separated,
so a block like `restart { policy "always" backoff-initial "10ms" }`
written on one line parses as a SINGLE node named `policy` whose
remaining words become extra arguments, not sibling keys. The parser
only ever read the first argument of each child node, so every key
after the first was silently discarded with no error.

Add `single_arg`, applied to every restart/stop/wait-for key that
takes exactly one value (all except `wait-for`'s `args`, which
legitimately takes zero or more), so a one-line block with 2+ keys
now fails to parse instead of silently keeping defaults.

`optional_string_map` (the `env { }` block) has the identical trap
but is out of scope here: it returns a bare BTreeMap with no error
path and would need a signature change to report a config error.
---
 crates/xy-protocol/src/kdl_parse.rs | 83 +++++++++++++++++++++++++++++
 1 file changed, 83 insertions(+)

diff --git a/crates/xy-protocol/src/kdl_parse.rs b/crates/xy-protocol/src/kdl_parse.rs
index 82f3dae..14294ce 100644
--- a/crates/xy-protocol/src/kdl_parse.rs
+++ b/crates/xy-protocol/src/kdl_parse.rs
@@ -154,6 +154,8 @@ fn parse_restart(doc: &KdlDocument, path: &Path) -> Result<RestartConfig, Config
     for child in children.nodes() {
         match child.name().value() {
             "policy" => {
+                single_arg(child, "restart", path)?;
+
                 let s = string_arg(child, "policy", path)?;
 
                 out.policy = match s.as_str() {
@@ -170,12 +172,18 @@ fn parse_restart(doc: &KdlDocument, path: &Path) -> Result<RestartConfig, Config
                 };
             }
             "backoff-initial" => {
+                single_arg(child, "restart", path)?;
+
                 out.backoff_initial = parse_duration_arg(child, "restart.backoff-initial", path)?;
             }
             "backoff-max" => {
+                single_arg(child, "restart", path)?;
+
                 out.backoff_max = parse_duration_arg(child, "restart.backoff-max", path)?;
             }
             "max-retries-per-minute" => {
+                single_arg(child, "restart", path)?;
+
                 let v = child
                     .entries()
                     .first()
@@ -220,6 +228,8 @@ fn parse_stop(doc: &KdlDocument, path: &Path) -> Result<StopConfig, ConfigError>
     for child in children.nodes() {
         match child.name().value() {
             "grace" => {
+                single_arg(child, "stop", path)?;
+
                 out.grace = parse_duration_arg(child, "stop.grace", path)?;
             }
             other => {
@@ -274,16 +284,22 @@ fn parse_wait_for(doc: &KdlDocument, path: &Path) -> Result<Option<WaitForConfig
     for child in children.nodes() {
         match child.name().value() {
             "path" => {
+                single_arg(child, "wait-for", path)?;
+
                 let raw = string_arg(child, "wait-for", path)?;
 
                 conditions.push(WaitCondition::Path(expand_tilde(&raw, home.as_deref())));
             }
             "tcp" => {
+                single_arg(child, "wait-for", path)?;
+
                 let addr = string_arg(child, "wait-for", path)?;
 
                 conditions.push(WaitCondition::Tcp(addr));
             }
             "command" => {
+                single_arg(child, "wait-for", path)?;
+
                 if command.is_some() {
                     return Err(invalid("duplicate key `command`".into()));
                 }
@@ -304,9 +320,13 @@ fn parse_wait_for(doc: &KdlDocument, path: &Path) -> Result<Option<WaitForConfig
                 );
             }
             "timeout" => {
+                single_arg(child, "wait-for", path)?;
+
                 timeout = parse_duration_arg(child, "wait-for", path)?;
             }
             "interval" => {
+                single_arg(child, "wait-for", path)?;
+
                 interval = parse_duration_arg(child, "wait-for", path)?;
             }
             other => {
@@ -339,6 +359,21 @@ fn parse_wait_for(doc: &KdlDocument, path: &Path) -> Result<Option<WaitForConfig
     }
 }
 
+fn single_arg(node: &KdlNode, field: &'static str, path: &Path) -> Result<(), ConfigError> {
+    if node.entries().len() == 1 {
+        return Ok(());
+    }
+
+    Err(ConfigError::InvalidValue {
+        path: path.to_path_buf(),
+        field,
+        message: format!(
+            "unexpected extra arguments for key `{}`; put each key on its own line",
+            node.name().value()
+        ),
+    })
+}
+
 fn string_arg(node: &KdlNode, field: &'static str, path: &Path) -> Result<String, ConfigError> {
     node.entries()
         .first()
@@ -727,4 +762,52 @@ stop {
 
         assert_eq!(expand_tilde("", Some(&home)), PathBuf::from(""));
     }
+
+    #[test]
+    fn one_line_restart_block_with_two_keys_fails() {
+        let text =
+            "command \"/bin/x\"\nport 1\nrestart { policy \"always\" backoff-initial \"10ms\" }";
+        let err = parse_server_config("foo", text, p()).unwrap_err();
+
+        assert!(matches!(
+            err,
+            ConfigError::InvalidValue {
+                field: "restart",
+                ..
+            }
+        ));
+    }
+
+    #[test]
+    fn one_line_wait_for_block_with_two_keys_fails() {
+        let text = "command \"/bin/x\"\nport 1\nwait-for { path \"/x\" timeout \"5s\" }";
+        let err = parse_server_config("foo", text, p()).unwrap_err();
+
+        assert!(matches!(
+            err,
+            ConfigError::InvalidValue {
+                field: "wait-for",
+                ..
+            }
+        ));
+    }
+
+    #[test]
+    fn one_line_single_key_stop_block_still_parses() {
+        let text = "command \"/bin/x\"\nport 1\nstop { grace \"30s\" }";
+        let cfg = parse_server_config("foo", text, p()).unwrap();
+
+        assert_eq!(cfg.stop.grace, Duration::from_secs(30));
+    }
+
+    #[test]
+    fn one_line_single_key_wait_for_block_still_parses() {
+        let text = "command \"/bin/x\"\nport 1\nwait-for { path \"/x\" }";
+        let cfg = parse_server_config("foo", text, p()).unwrap();
+
+        assert_eq!(
+            cfg.wait_for.unwrap().condition,
+            WaitCondition::Path(PathBuf::from("/x"))
+        );
+    }
 }