fix(site): refuse a starter that does not exist instead of serving an empty root

src/serve.mjs

@@ -22,6 +22,8 @@ import { promises as fs } from "node:fs";
 import path from "node:path";
 import { fileURLToPath } from "node:url";
 
+import { classifySource, listTemplates } from "./templates.mjs";
+
 /**
  * What a freshly installed site contains before anyone has written anything.
  *
@@ -36,6 +38,42 @@ import { fileURLToPath } from "node:url";
  */
 export const DEFAULT_TEMPLATE = "caddy-static";
 
+/**
+ * Which starter to seed, and whether that answer is usable at all.
+ *
+ * `--template` is checked rather than trusted, because every way of getting it
+ * wrong lands in the same place: the directory does not exist, the seed step is
+ * quietly dropped, and the root is left empty — which serves the 404 the
+ * seeding exists to prevent, after reporting that everything worked. A typo has
+ * to be louder than that.
+ *
+ * The shape check is templates.mjs's existing rule rather than a new one: a
+ * bundled name is one label, so anything carrying a slash or a dot is not a
+ * name and must not be joined into a path underneath examples/templates.
+ */
+export async function chooseTemplate(rest = [], { list = listTemplates } = {}) {
+  if (rest.includes("--empty")) return { ok: true, template: null };
+
+  const at = rest.indexOf("--template");
+  if (at < 0) return { ok: true, template: DEFAULT_TEMPLATE };
+
+  const asked = rest[at + 1];
+  // `--template --install` reads the next flag as the starter, and the flag it
+  // ate still takes effect, so the site installs with nothing in its root.
+  if (asked === undefined || asked.startsWith("-")) {
+    return { ok: false, error: "--template takes the name of a starter" };
+  }
+  if (classifySource(asked).kind !== "bundled") {
+    return { ok: false, error: `${JSON.stringify(asked)} is not a starter name` };
+  }
+
+  const available = (await list()).map((t) => t.name);
+  if (!available.includes(asked)) {
+    return { ok: false, error: `there is no starter called ${JSON.stringify(asked)}`, available };
+  }
+  return { ok: true, template: asked };
+}
+
 /** Where each server keeps drop-in site config. */
 const SERVERS = {
   nginx: { dir: "/etc/nginx/conf.d", ext: ".conf", reload: ["systemctl", "reload", "nginx"], check: ["nginx", "-t"] },
@@ -211,7 +249,14 @@ export async function serveCommand(args = [], out = console.log, deps = {}) {
     return 1;
   }
   const root = flag("--root") || `/srv/${name}`;
-  const template = rest.includes("--empty") ? null : (flag("--template") || DEFAULT_TEMPLATE);
+  const choice = await chooseTemplate(rest);
+  if (!choice.ok) {
+    out(`moshcode site: ${choice.error}`);
+    if (choice.available) out(`  bundled: ${choice.available.join(", ")}`);
+    out("  `moshcode template list` shows them all, or use --empty to seed nothing.");
+    return 1;
+  }
+  const template = choice.template;
   const here = path.dirname(fileURLToPath(import.meta.url));
   const seedFrom = template ? path.join(here, "..", "examples", "templates", template, "site") : null;
   // An empty root serves 404, which reads as a broken install. Seed only when

test/serve.test.mjs

@@ -5,7 +5,7 @@
 import test from "node:test";
 import assert from "node:assert/strict";
 
-import { caddySite, detectServer, nginxSite, servePlan, serveCommand } from "../src/serve.mjs";
+import { caddySite, chooseTemplate, detectServer, nginxSite, servePlan, serveCommand } from "../src/serve.mjs";
 
 test("no HTTPS redirect is ever emitted, for either server", () => {
   // The trap: a box's default vhost usually 301s to https, and for an ending
@@ -153,3 +153,79 @@ test("an existing site is never seeded over", async () => {
   assert.equal(seeded, false, "/etc is not empty, so nothing is seeded");
   assert.doesNotMatch(lines.join("\n"), /seed /);
 });
+
+test("a starter that does not exist is refused, not silently skipped", async () => {
+  // The failure this prevents: a typo makes the seed directory missing, the
+  // seed step is dropped, and the root is left empty — which is the 404 the
+  // seeding exists to prevent, arrived at while reporting success.
+  const list = async () => [{ name: "caddy-static" }, { name: "bun-caddy-sqlite" }];
+
+  const typo = await chooseTemplate(["--template", "caddy-statik"], { list });
+  assert.equal(typo.ok, false);
+  assert.match(typo.error, /no starter called/);
+  // Naming the two that do exist is most of the fix: the typo is one letter.
+  assert.deepEqual(typo.available, ["caddy-static", "bun-caddy-sqlite"]);
+
+  assert.equal((await chooseTemplate(["--template", "caddy-static"], { list })).template, "caddy-static");
+  // No --template at all is still the default starter, and --empty still wins.
+  assert.equal((await chooseTemplate([], { list })).template, "caddy-static");
+  assert.equal((await chooseTemplate(["--empty"], { list })).template, null);
+  assert.equal((await chooseTemplate(["--empty", "--template", "caddy-statik"], { list })).ok, true);
+});
+
+test("--template with no value does not read the next flag as the starter", async () => {
+  // `site x.y --template --install` took "--install" as the name, found no
+  // such directory, seeded nothing — and installed anyway, because --install
+  // is matched separately.
+  const list = async () => [{ name: "caddy-static" }];
+  for (const args of [["--template"], ["--template", "--install"], ["--template", "--empty"]]) {
+    const choice = await chooseTemplate(args, { list });
+    assert.equal(choice.ok, args.includes("--empty"), JSON.stringify(args));
+    if (!choice.ok) assert.match(choice.error, /takes the name of a starter/);
+  }
+});
+
+test("a starter name cannot climb out of the bundled directory", async () => {
+  // templates.mjs already refuses to treat anything with a slash or a dot as a
+  // bundled name, for this reason. `site` joined the raw value into a path
+  // under examples/templates without that check, so a value with enough ../ in
+  // it named a copy source anywhere on the box — and the copy lands in a root a
+  // web server is about to publish.
+  const list = async () => [{ name: "caddy-static" }];
+  for (const bad of ["../../../../etc", "a/b", "./caddy-static", "caddy static"]) {
+    const choice = await chooseTemplate(["--template", bad], { list });
+    assert.equal(choice.ok, false, bad);
+    assert.match(choice.error, /not a starter name/);
+  }
+});
+
+test("a bad --template stops the install before anything is written", async () => {
+  for (const args of [
+    ["blue.eggs", "--install", "--template", "caddy-statik"],
+    ["blue.eggs", "--install", "--template", "../../../../tmp"],
+    ["blue.eggs", "--template", "--install"],
+  ]) {
+    const lines = [];
+    let touched = 0;
+    const code = await serveCommand(args, (l) => lines.push(l), {
+      detect: async () => "nginx",
+      write: async () => { touched += 1; },
+      mkdir: async () => { touched += 1; },
+      copy: async () => { touched += 1; },
+    });
+    assert.equal(code, 1, JSON.stringify(args));
+    assert.equal(touched, 0, "a starter we cannot find is not a reason to install a site with an empty root");
+  }
+
+  // The control: the starter that does exist still seeds.
+  const lines = [];
+  let seeded = null;
+  const code = await serveCommand(["blue.eggs", "--install", "--template", "caddy-static"], (l) => lines.push(l), {
+    detect: async () => "nginx",
+    write: async () => {},
+    mkdir: async () => {},
+    copy: async (from) => { seeded = from; },
+  });
+  assert.equal(code, 0);
+  assert.match(String(seeded), /examples\/templates\/caddy-static\/site$/);
+});