From 5dd3c8ebaa203768366010b420fb94d49de003ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Kova=C4=BE?= <kuba.koval@gmail.com>
Date: Wed, 10 Jun 2026 19:00:30 +0200
Subject: [PATCH] fix: validate skills/agents/extends are lists (#12)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A YAML scalar in skills, agents, or extends survived the 'as string[]'
cast and was iterated character-by-character downstream — crashing
'umbel list' (TypeError on .join) and producing per-character nonsense
refs/parents in 'umbel build'.

Apply the same Array.isArray + every(string) guard already used for
hooks/mcps so a scalar yields one actionable 'must be a list' error.
Discovery already catches the throw and marks the bundle malformed, so
'umbel list' no longer crashes — matching the existing hooks/mcps path.
---
 src/bundle/manifest.ts            | 13 +++++++++++++
 test/unit/bundle/manifest.test.ts | 23 +++++++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/src/bundle/manifest.ts b/src/bundle/manifest.ts
index 58627b8..e831127 100644
--- a/src/bundle/manifest.ts
+++ b/src/bundle/manifest.ts
@@ -84,12 +84,25 @@ export function loadManifest(path: string): ManifestResult {
     manifest.description = data.description as string;
   }
   if (data.extends !== undefined) {
+    if (!Array.isArray(data.extends) || !data.extends.every((e) => typeof e === "string")) {
+      throw new UsageError(`bundle ${path}: 'extends' must be a list of names`);
+    }
     manifest.extends = data.extends as string[];
   }
   if (data.skills !== undefined) {
+    if (!Array.isArray(data.skills) || !data.skills.every((s) => typeof s === "string")) {
+      throw new UsageError(
+        `bundle ${path}: 'skills' must be a list of qualified <source>/<name> refs`,
+      );
+    }
     manifest.skills = data.skills as string[];
   }
   if (data.agents !== undefined) {
+    if (!Array.isArray(data.agents) || !data.agents.every((a) => typeof a === "string")) {
+      throw new UsageError(
+        `bundle ${path}: 'agents' must be a list of qualified <source>/<name> refs`,
+      );
+    }
     manifest.agents = data.agents as string[];
   }
   if (data.hooks !== undefined) {
diff --git a/test/unit/bundle/manifest.test.ts b/test/unit/bundle/manifest.test.ts
index cd7e74e..617d403 100644
--- a/test/unit/bundle/manifest.test.ts
+++ b/test/unit/bundle/manifest.test.ts
@@ -171,6 +171,29 @@ mcps:
     expect(() => loadManifest(path)).toThrow(/mcps.*list.*qualified/i);
   });
 
+  it.each([
+    ["extends", "good"],
+    ["skills", "local/demo"],
+    ["agents", "data-scientist"],
+  ])("rejects scalar %s where a list is required", (field, value) => {
+    const path = writeBundle(
+      `scalar-${field}`,
+      `---\nname: scalar-${field}\n${field}: ${value}\n---\n`,
+    );
+    expect(() => loadManifest(path)).toThrow(new RegExp(`${field}.*list`, "i"));
+  });
+
+  it.each(["extends", "skills", "agents"])(
+    "rejects %s list containing a non-string element",
+    (field) => {
+      const path = writeBundle(
+        `nonstring-${field}`,
+        `---\nname: nonstring-${field}\n${field}: [123]\n---\n`,
+      );
+      expect(() => loadManifest(path)).toThrow(new RegExp(`${field}.*list`, "i"));
+    },
+  );
+
   it("warns on unknown frontmatter field but does not error", () => {
     const path = writeBundle(
       "with-unknown",