Config: reject unknown keys, bad widths, non-http origins, blank selectors

src/cli.ts

@@ -16,6 +16,7 @@ import { ACCEPTED_FILE } from "./accept.ts";
 import type { Results } from "./report.ts";
 import {
   alive,
+  isHttpUrl,
   label,
   loadConfig,
   name,
@@ -509,7 +510,7 @@ async function probe(a: Args, loads: number): Promise<never> {
   if ((a.widths?.length ?? 0) > 1) die("probe takes one --width");
   if (a.selector) die("probe does not take --selector; it walks the whole page");
   if (a.only) die("probe does not take --only");
-  if (!URL.canParse(a.left)) die(`not a URL: ${a.left}`);
+  if (!isHttpUrl(a.left)) die(`not an http(s) URL: ${a.left}`);
   const u = new URL(a.left);
   let route: Route;
   try {

src/config.ts

@@ -390,6 +390,37 @@ const CONFIG_NAMES = [
 const isStrArray = (v: unknown): v is string[] =>
   Array.isArray(v) && v.every((x) => typeof x === "string");
 
+export const isHttpUrl = (s: string) => URL.canParse(s) && /^https?:$/.test(new URL(s).protocol);
+
+type KeysOf<T> = T extends unknown ? keyof T : never;
+const CONFIG_KEYS = Object.keys({
+  left: 0,
+  right: 0,
+  routes: 0,
+  before: 0,
+  beforeShot: 0,
+  storageState: 0,
+  widths: 0,
+  threshold: 0,
+  outDir: 0,
+  concurrency: 0,
+  blockUrlPatterns: 0,
+  hideSelectors: 0,
+  extraCss: 0,
+  masks: 0,
+  computed: 0,
+  aria: 0,
+  maskMedia: 0,
+  freezeVideo: 0,
+  lottieFrame: 0,
+} satisfies Record<keyof RenderdiffConfig, 0>);
+const MASK_KEYS = Object.keys({
+  selector: 0,
+  locate: 0,
+  reason: 0,
+  urlPath: 0,
+} satisfies Record<KeysOf<Mask>, 0>);
+
 /**
  * Load a config file. An explicit `--config` path must exist; otherwise cwd is probed
  * and finding nothing is fine — the CLI works with flags alone.
@@ -447,6 +478,11 @@ function validate(c: Record<string, unknown>, file: string) {
     throw new Error(`${file}: \`${key}\` must be ${expected}`);
   };
 
+  const unknown = Object.keys(c).find((k) => !CONFIG_KEYS.includes(k));
+  if (unknown) {
+    throw new Error(`${file}: \`${unknown}\` is not a config key (${CONFIG_KEYS.join(", ")})`);
+  }
+
   for (const key of ["left", "right", "outDir", "extraCss"]) {
     if (c[key] !== undefined && typeof c[key] !== "string") bad(key, "a string");
   }
@@ -455,6 +491,9 @@ function validate(c: Record<string, unknown>, file: string) {
       bad(key, "an array of strings");
     }
   }
+  if ((c.hideSelectors as string[] | undefined)?.some((s) => !s.trim())) {
+    bad("hideSelectors", "an array of non-empty strings");
+  }
   const hook = "a function (page, { side, route, width }) => Promise<void>";
   if (c.before !== undefined && typeof c.before !== "function") bad("before", hook);
   if (c.beforeShot !== undefined && typeof c.beforeShot !== "function") bad("beforeShot", hook);
@@ -517,9 +556,10 @@ function validate(c: Record<string, unknown>, file: string) {
   if (c.widths !== undefined) {
     if (
       !Array.isArray(c.widths) ||
-      !c.widths.every((w) => typeof w === "number" && Number.isFinite(w) && w > 0)
+      !c.widths.length ||
+      !c.widths.every((w) => Number.isInteger(w) && w > 0)
     ) {
-      bad("widths", "an array of positive numbers");
+      bad("widths", "a non-empty array of positive integers");
     }
   }
   if (c.computed !== undefined && typeof c.computed !== "boolean") bad("computed", "a boolean");
@@ -544,15 +584,21 @@ function validate(c: Record<string, unknown>, file: string) {
         bad(`masks[${i}]`, "an object");
         return;
       }
+      const unknown = Object.keys(o).find((k) => !MASK_KEYS.includes(k));
+      if (unknown) {
+        throw new Error(
+          `${file}: \`masks[${i}].${unknown}\` is not a mask key (${MASK_KEYS.join(", ")})`,
+        );
+      }
       if (o.urlPath !== undefined && typeof o.urlPath !== "string") {
         bad(`masks[${i}].urlPath`, "a string");
       }
       if (typeof o.reason !== "string") bad(`masks[${i}].reason`, "a string");
       if ("selector" in o === "locate" in o) {
         throw new Error(`${file}: \`masks[${i}]\` needs exactly one of \`selector\` or \`locate\``);
       }
-      if (o.selector !== undefined && typeof o.selector !== "string") {
-        bad(`masks[${i}].selector`, "a string");
+      if (o.selector !== undefined && (typeof o.selector !== "string" || !o.selector.trim())) {
+        bad(`masks[${i}].selector`, "a non-empty string");
       }
       if (o.locate !== undefined && typeof o.locate !== "function") {
         bad(`masks[${i}].locate`, "a function (page) => Locator");
@@ -604,6 +650,9 @@ export function resolveConfig(file: RenderdiffConfig, cli: CliOverrides): Resolv
           : "right origin is required (second positional URL, --right, or right in renderdiff.config.*)",
     );
   }
+  for (const [side, url] of Object.entries({ left, right })) {
+    if (!isHttpUrl(url)) throw new Error(`${side} origin must be an http(s) URL, got ${url}`);
+  }
 
   const fileState =
     typeof file.storageState === "string"

tests/unit/cli-args.test.ts

@@ -128,6 +128,12 @@ describe("required arguments", () => {
   test("a missing --left is named, in the singular", () =>
     rejects(["/a", "--right", R, "--dry-run"], "left origin is required (first positional URL"));
 
+  test("an origin without a scheme is refused", () =>
+    rejects(
+      ["/a", "--left", "localhost:3000", "--right", R, "--dry-run"],
+      "left origin must be an http(s) URL, got localhost:3000",
+    ));
+
   test("a missing --right is named", () =>
     rejects(["/a", "--left", L, "--dry-run"], "right origin is required (second positional URL"));
 
@@ -463,6 +469,9 @@ describe("probe", () => {
     await rejects(["probe", U, "/b"], "probe takes one URL");
   });
 
+  test("refuses a URL without a scheme", () =>
+    rejects(["probe", "--left", "localhost:3000"], "not an http(s) URL: localhost:3000"));
+
   test("refuses --only", () => rejects(["probe", U, "--only", "/a"], "probe does not take --only"));
 
   test("--loads is an integer of at least 2", async () => {

tests/unit/config-load.test.ts

@@ -112,7 +112,7 @@ describe("validation", () => {
       "bad-routes.mjs",
       /`routes\[0\]` must be a string or an object \{ left, right\?, selector\?, before\?, beforeShot\? \}/,
     ],
-    ["bad-widths.mjs", /`widths` must be an array of positive numbers/],
+    ["bad-widths.mjs", /`widths` must be a non-empty array of positive integers/],
     ["bad-threshold.mjs", /`threshold` must be a number between 0 and 1/],
     ["bad-concurrency.mjs", /`concurrency` must be a positive integer/],
     ["bad-mask-shape.mjs", /`masks` must be an array/],
@@ -153,7 +153,22 @@ describe("validation", () => {
       '{ masks: [{ reason: "x", selector: "a", locate: (p) => p.locator("a") }] }',
       /`masks\[0\]` needs exactly one of `selector` or `locate`/,
     ],
-    ['{ masks: [{ reason: "x", selector: 1 }] }', /`masks\[0\]\.selector` must be a string/],
+    [
+      '{ masks: [{ reason: "x", selector: 1 }] }',
+      /`masks\[0\]\.selector` must be a non-empty string/,
+    ],
+    [
+      '{ masks: [{ reason: "x", selector: " " }] }',
+      /`masks\[0\]\.selector` must be a non-empty string/,
+    ],
+    [
+      '{ masks: [{ reason: "x", selector: "a", urlpath: "/a" }] }',
+      /`masks\[0\]\.urlpath` is not a mask key \(selector, locate, reason, urlPath\)/,
+    ],
+    ['{ lfet: "http://x.test" }', /`lfet` is not a config key \(left, right, routes,/],
+    ['{ hideSelectors: [" "] }', /`hideSelectors` must be an array of non-empty strings/],
+    ["{ widths: [] }", /`widths` must be a non-empty array of positive integers/],
+    ["{ widths: [375.5] }", /`widths` must be a non-empty array of positive integers/],
     [
       '{ routes: [{ left: "/a", selector: 1 }] }',
       /`routes\[0\]\.selector` must be a non-empty string or \{ left, right \}/,