test: tighten install mode and allowlist coverage

steipete · steipete · commit 0db1c3110309 · 2026-03-13T18:46:11.000Z
diff --git a/src/infra/exec-allowlist-pattern.test.ts b/src/infra/exec-allowlist-pattern.test.ts
@@ -2,13 +2,47 @@ import { describe, expect, it } from "vitest";
 import { matchesExecAllowlistPattern } from "./exec-allowlist-pattern.js";
 
 describe("matchesExecAllowlistPattern", () => {
+  it.each([
+    { pattern: "", target: "/tmp/tool", expected: false },
+    { pattern: "   ", target: "/tmp/tool", expected: false },
+    { pattern: "/tmp/tool", target: "/tmp/tool", expected: true },
+  ])("handles literal patterns for %j", ({ pattern, target, expected }) => {
+    expect(matchesExecAllowlistPattern(pattern, target)).toBe(expected);
+  });
+
   it("does not let ? cross path separators", () => {
     expect(matchesExecAllowlistPattern("/tmp/a?b", "/tmp/a/b")).toBe(false);
     expect(matchesExecAllowlistPattern("/tmp/a?b", "/tmp/acb")).toBe(true);
   });
 
-  it("keeps ** matching across path separators", () => {
-    expect(matchesExecAllowlistPattern("/tmp/**/tool", "/tmp/a/b/tool")).toBe(true);
+  it.each([
+    { pattern: "/tmp/*/tool", target: "/tmp/a/tool", expected: true },
+    { pattern: "/tmp/*/tool", target: "/tmp/a/b/tool", expected: false },
+    { pattern: "/tmp/**/tool", target: "/tmp/a/b/tool", expected: true },
+  ])("handles star patterns for %j", ({ pattern, target, expected }) => {
+    expect(matchesExecAllowlistPattern(pattern, target)).toBe(expected);
+  });
+
+  it("expands home-prefix patterns", () => {
+    const prevOpenClawHome = process.env.OPENCLAW_HOME;
+    const prevHome = process.env.HOME;
+    process.env.OPENCLAW_HOME = "/srv/openclaw-home";
+    process.env.HOME = "/home/other";
+    try {
+      expect(matchesExecAllowlistPattern("~/bin/tool", "/srv/openclaw-home/bin/tool")).toBe(true);
+      expect(matchesExecAllowlistPattern("~/bin/tool", "/home/other/bin/tool")).toBe(false);
+    } finally {
+      if (prevOpenClawHome === undefined) {
+        delete process.env.OPENCLAW_HOME;
+      } else {
+        process.env.OPENCLAW_HOME = prevOpenClawHome;
+      }
+      if (prevHome === undefined) {
+        delete process.env.HOME;
+      } else {
+        process.env.HOME = prevHome;
+      }
+    }
   });
 
   it.runIf(process.platform !== "win32")("preserves case sensitivity on POSIX", () => {
diff --git a/src/infra/install-mode-options.test.ts b/src/infra/install-mode-options.test.ts
@@ -5,47 +5,76 @@ import {
 } from "./install-mode-options.js";
 
 describe("install mode option helpers", () => {
-  it("applies logger, mode, and dryRun defaults", () => {
-    const logger = { warn: (_message: string) => {} };
-    const result = resolveInstallModeOptions({}, logger);
+  it.each([
+    {
+      name: "applies logger, mode, and dryRun defaults",
+      params: {},
+      expected: { loggerKey: "default", mode: "install", dryRun: false },
+    },
+    {
+      name: "preserves explicit mode and dryRun values",
+      params: { loggerKey: "explicit", mode: "update" as const, dryRun: true },
+      expected: { loggerKey: "explicit", mode: "update", dryRun: true },
+    },
+    {
+      name: "preserves explicit false dryRun values",
+      params: { mode: "update" as const, dryRun: false },
+      expected: { loggerKey: "default", mode: "update", dryRun: false },
+    },
+  ])("$name", ({ params, expected }) => {
+    const loggers = {
+      default: { warn: (_message: string) => {} },
+      explicit: { warn: (_message: string) => {} },
+    };
 
-    expect(result).toEqual({
-      logger,
-      mode: "install",
-      dryRun: false,
+    expect(
+      resolveInstallModeOptions(
+        {
+          logger: params.loggerKey ? loggers[params.loggerKey] : undefined,
+          mode: params.mode,
+          dryRun: params.dryRun,
+        },
+        loggers.default,
+      ),
+    ).toEqual({
+      logger: loggers[expected.loggerKey],
+      mode: expected.mode,
+      dryRun: expected.dryRun,
     });
   });
 
-  it("preserves explicit mode and dryRun values", () => {
+  it.each([
+    {
+      name: "uses default timeout when not provided",
+      params: {},
+      defaultTimeoutMs: undefined,
+      expectedTimeoutMs: 120_000,
+      expectedMode: "install",
+      expectedDryRun: false,
+    },
+    {
+      name: "honors custom timeout default override",
+      params: {},
+      defaultTimeoutMs: 5000,
+      expectedTimeoutMs: 5000,
+      expectedMode: "install",
+      expectedDryRun: false,
+    },
+    {
+      name: "preserves explicit timeout values",
+      params: { timeoutMs: 0, mode: "update" as const, dryRun: true },
+      defaultTimeoutMs: 5000,
+      expectedTimeoutMs: 0,
+      expectedMode: "update",
+      expectedDryRun: true,
+    },
+  ])("$name", ({ params, defaultTimeoutMs, expectedTimeoutMs, expectedMode, expectedDryRun }) => {
     const logger = { warn: (_message: string) => {} };
-    const result = resolveInstallModeOptions(
-      {
-        logger,
-        mode: "update",
-        dryRun: true,
-      },
-      { warn: () => {} },
-    );
-
-    expect(result).toEqual({
-      logger,
-      mode: "update",
-      dryRun: true,
-    });
-  });
-
-  it("uses default timeout when not provided", () => {
-    const logger = { warn: (_message: string) => {} };
-    const result = resolveTimedInstallModeOptions({}, logger);
-
-    expect(result.timeoutMs).toBe(120_000);
-    expect(result.mode).toBe("install");
-    expect(result.dryRun).toBe(false);
-  });
-
-  it("honors custom timeout default override", () => {
-    const result = resolveTimedInstallModeOptions({}, { warn: () => {} }, 5000);
+    const result = resolveTimedInstallModeOptions(params, logger, defaultTimeoutMs);
 
-    expect(result.timeoutMs).toBe(5000);
+    expect(result.timeoutMs).toBe(expectedTimeoutMs);
+    expect(result.mode).toBe(expectedMode);
+    expect(result.dryRun).toBe(expectedDryRun);
+    expect(result.logger).toBe(logger);
   });
 });
