refactor(security): table-drive wrapper approval pinning tests

src/node-host/invoke-system-run-plan.test.ts

@@ -7,81 +7,105 @@ import {
   hardenApprovedExecutionPaths,
 } from "./invoke-system-run-plan.js";
 
+type PathTokenSetup = {
+  expected: string;
+};
+
+type HardeningCase = {
+  name: string;
+  mode: "build-plan" | "harden";
+  argv: string[];
+  shellCommand?: string | null;
+  withPathToken?: boolean;
+  expectedArgv: (ctx: { pathToken: PathTokenSetup | null }) => string[];
+  expectedCmdText?: string;
+};
+
 describe("hardenApprovedExecutionPaths", () => {
-  it.runIf(process.platform !== "win32")(
+  const cases: HardeningCase[] = [
-    "preserves shell-wrapper argv during approval hardening",
+    {
-    () => {
+      name: "preserves shell-wrapper argv during approval hardening",
-      const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-shell-wrapper-"));
+      mode: "build-plan",
-      try {
+      argv: ["env", "sh", "-c", "echo SAFE"],
-        const prepared = buildSystemRunApprovalPlan({
+      expectedArgv: () => ["env", "sh", "-c", "echo SAFE"],
-          command: ["env", "sh", "-c", "echo SAFE"],
+      expectedCmdText: "echo SAFE",
-          cwd: tmp,
-        });
-        expect(prepared.ok).toBe(true);
-        if (!prepared.ok) {
-          throw new Error("unreachable");
-        }
-        expect(prepared.plan.argv).toEqual(["env", "sh", "-c", "echo SAFE"]);
-        expect(prepared.cmdText).toBe("echo SAFE");
-      } finally {
-        fs.rmSync(tmp, { recursive: true, force: true });
-      }
     },
-  );
+    {
-
+      name: "preserves dispatch-wrapper argv during approval hardening",
-  it.runIf(process.platform !== "win32")(
+      mode: "harden",
-    "preserves dispatch-wrapper argv during approval hardening",
+      argv: ["env", "tr", "a", "b"],
-    () => {
+      shellCommand: null,
-      const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-dispatch-wrapper-"));
+      expectedArgv: () => ["env", "tr", "a", "b"],
-      try {
-        const hardened = hardenApprovedExecutionPaths({
-          approvedByAsk: true,
-          argv: ["env", "tr", "a", "b"],
-          shellCommand: null,
-          cwd: tmp,
-        });
-        expect(hardened.ok).toBe(true);
-        if (!hardened.ok) {
-          throw new Error("unreachable");
-        }
-        expect(hardened.argv).toEqual(["env", "tr", "a", "b"]);
-      } finally {
-        fs.rmSync(tmp, { recursive: true, force: true });
-      }
     },
-  );
+    {
+      name: "pins direct PATH-token executable during approval hardening",
+      mode: "harden",
+      argv: ["poccmd", "SAFE"],
+      shellCommand: null,
+      withPathToken: true,
+      expectedArgv: ({ pathToken }) => [pathToken!.expected, "SAFE"],
+    },
+    {
+      name: "preserves env-wrapper PATH-token argv during approval hardening",
+      mode: "harden",
+      argv: ["env", "poccmd", "SAFE"],
+      shellCommand: null,
+      withPathToken: true,
+      expectedArgv: () => ["env", "poccmd", "SAFE"],
+    },
+  ];
 
-  it.runIf(process.platform !== "win32")(
+  for (const testCase of cases) {
-    "pins direct PATH-token executable during approval hardening",
+    it.runIf(process.platform !== "win32")(testCase.name, () => {
-    () => {
+      const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approval-hardening-"));
-      const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-direct-pin-"));
-      const binDir = path.join(tmp, "bin");
-      fs.mkdirSync(binDir, { recursive: true });
-      const link = path.join(binDir, "poccmd");
-      fs.symlinkSync("/bin/echo", link);
-      const expected = fs.realpathSync(link);
       const oldPath = process.env.PATH;
-      process.env.PATH = `${binDir}${path.delimiter}${oldPath ?? ""}`;
+      let pathToken: PathTokenSetup | null = null;
+      if (testCase.withPathToken) {
+        const binDir = path.join(tmp, "bin");
+        fs.mkdirSync(binDir, { recursive: true });
+        const link = path.join(binDir, "poccmd");
+        fs.symlinkSync("/bin/echo", link);
+        pathToken = { expected: fs.realpathSync(link) };
+        process.env.PATH = `${binDir}${path.delimiter}${oldPath ?? ""}`;
+      }
       try {
+        if (testCase.mode === "build-plan") {
+          const prepared = buildSystemRunApprovalPlan({
+            command: testCase.argv,
+            cwd: tmp,
+          });
+          expect(prepared.ok).toBe(true);
+          if (!prepared.ok) {
+            throw new Error("unreachable");
+          }
+          expect(prepared.plan.argv).toEqual(testCase.expectedArgv({ pathToken }));
+          if (testCase.expectedCmdText) {
+            expect(prepared.cmdText).toBe(testCase.expectedCmdText);
+          }
+          return;
+        }
+
         const hardened = hardenApprovedExecutionPaths({
           approvedByAsk: true,
-          argv: ["poccmd", "SAFE"],
+          argv: testCase.argv,
-          shellCommand: null,
+          shellCommand: testCase.shellCommand ?? null,
           cwd: tmp,
         });
         expect(hardened.ok).toBe(true);
         if (!hardened.ok) {
           throw new Error("unreachable");
         }
-        expect(hardened.argv).toEqual([expected, "SAFE"]);
+        expect(hardened.argv).toEqual(testCase.expectedArgv({ pathToken }));
       } finally {
-        if (oldPath === undefined) {
+        if (testCase.withPathToken) {
-          delete process.env.PATH;
+          if (oldPath === undefined) {
-        } else {
+            delete process.env.PATH;
-          process.env.PATH = oldPath;
+          } else {
+            process.env.PATH = oldPath;
+          }
         }
         fs.rmSync(tmp, { recursive: true, force: true });
       }
-    },
+    });
-  );
+  }
 });

src/node-host/invoke-system-run-plan.ts

@@ -53,6 +53,16 @@ function hasMutableSymlinkPathComponentSync(targetPath: string): boolean {
   return false;
 }
 
+function shouldPinExecutableForApproval(params: {
+  shellCommand: string | null;
+  wrapperChain: string[] | undefined;
+}): boolean {
+  if (params.shellCommand !== null) {
+    return false;
+  }
+  return (params.wrapperChain?.length ?? 0) === 0;
+}
+
 export function hardenApprovedExecutionPaths(params: {
   approvedByAsk: boolean;
   argv: string[];
@@ -116,19 +126,19 @@ export function hardenApprovedExecutionPaths(params: {
     return { ok: true, argv: params.argv, cwd: hardenedCwd };
   }
 
-  // Preserve shell-wrapper semantics. Rewriting argv[0] for wrappers can change
+  const resolution = resolveCommandResolutionFromArgv(params.argv, hardenedCwd);
-  // runtime behavior (for example: env sh -c ... -> /bin/sh sh -c ...).
+  if (
-  if (params.shellCommand !== null) {
+    !shouldPinExecutableForApproval({
+      shellCommand: params.shellCommand,
+      wrapperChain: resolution?.wrapperChain,
+    })
+  ) {
+    // Preserve wrapper semantics for approval-based execution. Pinning the
+    // effective executable while keeping wrapper argv shape can shift positional
+    // arguments and execute a different command than approved.
     return { ok: true, argv: params.argv, cwd: hardenedCwd };
   }
 
-  const resolution = resolveCommandResolutionFromArgv(params.argv, hardenedCwd);
-  // Preserve transparent wrapper semantics for approval-based execution.
-  // Pinning the effective executable while keeping wrapper argv shape can shift
-  // positional arguments and execute a different command than approved.
-  if ((resolution?.wrapperChain?.length ?? 0) > 0) {
-    return { ok: true, argv: params.argv, cwd: hardenedCwd };
-  }
   const pinnedExecutable = resolution?.resolvedRealPath ?? resolution?.resolvedPath;
   if (!pinnedExecutable) {
     return {

src/node-host/invoke-system-run.test.ts

@@ -300,6 +300,81 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
     });
   });
 
+  const approvedEnvShellWrapperCases = [
+    {
+      name: "preserves wrapper argv for approved env shell commands in local execution",
+      preferMacAppExecHost: false,
+    },
+    {
+      name: "preserves wrapper argv for approved env shell commands in mac app exec host forwarding",
+      preferMacAppExecHost: true,
+    },
+  ] as const;
+
+  for (const testCase of approvedEnvShellWrapperCases) {
+    it.runIf(process.platform !== "win32")(testCase.name, async () => {
+      const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approved-wrapper-"));
+      const marker = path.join(tmp, "marker");
+      const attackerScript = path.join(tmp, "sh");
+      fs.writeFileSync(attackerScript, "#!/bin/sh\necho exploited > marker\n");
+      fs.chmodSync(attackerScript, 0o755);
+      const runCommand = vi.fn(async (argv: string[]) => {
+        if (argv[0] === "/bin/sh" && argv[1] === "sh" && argv[2] === "-c") {
+          fs.writeFileSync(marker, "rewritten");
+        }
+        return createLocalRunResult();
+      });
+      const sendInvokeResult = vi.fn(async () => {});
+      try {
+        const invoke = await runSystemInvoke({
+          preferMacAppExecHost: testCase.preferMacAppExecHost,
+          command: ["env", "sh", "-c", "echo SAFE"],
+          cwd: tmp,
+          approved: true,
+          security: "allowlist",
+          ask: "on-miss",
+          runCommand,
+          sendInvokeResult,
+          runViaResponse: testCase.preferMacAppExecHost
+            ? {
+                ok: true,
+                payload: {
+                  success: true,
+                  stdout: "app-ok",
+                  stderr: "",
+                  timedOut: false,
+                  exitCode: 0,
+                  error: null,
+                },
+              }
+            : undefined,
+        });
+
+        if (testCase.preferMacAppExecHost) {
+          const canonicalCwd = fs.realpathSync(tmp);
+          expect(invoke.runCommand).not.toHaveBeenCalled();
+          expect(invoke.runViaMacAppExecHost).toHaveBeenCalledWith({
+            approvals: expect.anything(),
+            request: expect.objectContaining({
+              command: ["env", "sh", "-c", "echo SAFE"],
+              rawCommand: "echo SAFE",
+              cwd: canonicalCwd,
+            }),
+          });
+          expectInvokeOk(invoke.sendInvokeResult, { payloadContains: "app-ok" });
+          return;
+        }
+
+        const runArgs = vi.mocked(invoke.runCommand).mock.calls[0]?.[0] as string[] | undefined;
+        expect(runArgs).toEqual(["env", "sh", "-c", "echo SAFE"]);
+        expect(fs.existsSync(marker)).toBe(false);
+        expectInvokeOk(invoke.sendInvokeResult);
+      } finally {
+        fs.rmSync(tmp, { recursive: true, force: true });
+      }
+    });
+  }
+
   it("handles transparent env wrappers in allowlist mode", async () => {
     const { runCommand, sendInvokeResult } = await runSystemInvoke({
       preferMacAppExecHost: false,
@@ -479,42 +554,6 @@ describe("handleSystemRunInvoke mac app exec host routing", () => {
     }
   });
 
-  it.runIf(process.platform !== "win32")(
-    "preserves wrapper argv for approved env shell commands",
-    async () => {
-      const tmp = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-approved-wrapper-"));
-      const marker = path.join(tmp, "marker");
-      const attackerScript = path.join(tmp, "sh");
-      fs.writeFileSync(attackerScript, "#!/bin/sh\necho exploited > marker\n");
-      fs.chmodSync(attackerScript, 0o755);
-      const runCommand = vi.fn(async (argv: string[]) => {
-        if (argv[0] === "/bin/sh" && argv[1] === "sh" && argv[2] === "-c") {
-          fs.writeFileSync(marker, "rewritten");
-        }
-        return createLocalRunResult();
-      });
-      const sendInvokeResult = vi.fn(async () => {});
-      try {
-        await runSystemInvoke({
-          preferMacAppExecHost: false,
-          command: ["env", "sh", "-c", "echo SAFE"],
-          cwd: tmp,
-          approved: true,
-          security: "allowlist",
-          ask: "on-miss",
-          runCommand,
-          sendInvokeResult,
-        });
-        const runArgs = vi.mocked(runCommand).mock.calls[0]?.[0] as string[] | undefined;
-        expect(runArgs).toEqual(["env", "sh", "-c", "echo SAFE"]);
-        expect(fs.existsSync(marker)).toBe(false);
-        expectInvokeOk(sendInvokeResult);
-      } finally {
-        fs.rmSync(tmp, { recursive: true, force: true });
-      }
-    },
-  );
-
   it("denies ./sh wrapper spoof in allowlist on-miss mode before execution", async () => {
     const marker = path.join(os.tmpdir(), `openclaw-wrapper-spoof-${process.pid}-${Date.now()}`);
     const runCommand = vi.fn(async () => {