fix: block side-effecting command wrappers [AI] (#87292)

* fix: block side-effecting command wrappers

* docs: add changelog entry for PR merge

CHANGELOG.md

@@ -11,6 +11,7 @@ Docs: https://docs.openclaw.ai
 
 ### Fixes
 
+- fix: block side-effecting command wrappers [AI]. (#87292) Thanks @pgondhi987.
 - Block unsafe Node runtime env overrides [AI]. (#87308) Thanks @pgondhi987.
 - Telegram: route `sendMessage` action replies through durable outbound delivery so completed agent responses remain retryable when the gateway send path times out. (#87261) Thanks @mbelinky.
 - Gateway/security: require `operator.admin` for node and other non-operator device-role pairing approvals, including trusted-proxy sessions, while keeping pairing-only approvals available for operator-role requests. (#87146)

src/infra/dispatch-wrapper-resolution.ts

@@ -235,12 +235,44 @@ function unwrapTimeInvocation(argv: string[]): string[] | null {
   });
 }
 
+function timeInvocationWritesOutputFile(argv: string[]): boolean {
+  let expectsOptionValue = false;
+  for (let idx = 1; idx < argv.length; idx += 1) {
+    const token = argv[idx]?.trim() ?? "";
+    if (!token) {
+      continue;
+    }
+    if (expectsOptionValue) {
+      expectsOptionValue = false;
+      continue;
+    }
+    if (token === "--") {
+      return false;
+    }
+    if (!token.startsWith("-") || token === "-") {
+      return false;
+    }
+    const lower = normalizeLowercaseStringOrEmpty(token);
+    const [flag] = lower.split("=", 2);
+    if (flag === "-o" || flag === "--output") {
+      return true;
+    }
+    if (TIME_OPTIONS_WITH_VALUE.has(flag) && !lower.includes("=")) {
+      expectsOptionValue = true;
+    }
+  }
+  return false;
+}
+
 function supportsScriptPositionalCommand(platform: NodeJS.Platform = process.platform): boolean {
   return platform === "darwin" || platform === "freebsd";
 }
 
-function unwrapScriptInvocation(argv: string[]): string[] | null {
-  if (!supportsScriptPositionalCommand()) {
+function unwrapScriptInvocation(
+  argv: string[],
+  platform: NodeJS.Platform = process.platform,
+): string[] | null {
+  if (!supportsScriptPositionalCommand(platform)) {
     return null;
   }
   return scanWrapperInvocation(argv, {
@@ -368,12 +400,16 @@ const DISPATCH_WRAPPER_SPECS: readonly DispatchWrapperSpec[] = [
   { name: "nice", unwrap: unwrapNiceInvocation, transparentUsage: true },
   { name: "nohup", unwrap: unwrapNohupInvocation, transparentUsage: true },
   { name: "sandbox-exec", unwrap: unwrapSandboxExecInvocation, transparentUsage: true },
-  { name: "script", unwrap: unwrapScriptInvocation, transparentUsage: true },
+  { name: "script", unwrap: unwrapScriptInvocation, transparentUsage: false },
   { name: "setsid" },
   { name: "stdbuf", unwrap: unwrapStdbufInvocation, transparentUsage: true },
   { name: "sudo" },
   { name: "taskset" },
-  { name: "time", unwrap: unwrapTimeInvocation, transparentUsage: true },
+  {
+    name: "time",
+    unwrap: unwrapTimeInvocation,
+    transparentUsage: (argv) => !timeInvocationWritesOutputFile(argv),
+  },
   { name: "timeout", unwrap: unwrapTimeoutInvocation, transparentUsage: true },
   {
     name: "xcrun",

src/infra/exec-approvals-allowlist.ts

@@ -408,6 +408,7 @@ function resolveSegmentAllowlistMatch(params: {
           segment: allowlistSegment,
           cwd: params.context.cwd,
           env: params.context.env,
+          platform: params.context.platform,
         })
       : undefined;
   const shellPositionalArgvMatch = shellPositionalArgvCandidatePath
@@ -822,6 +823,7 @@ function resolveShellWrapperPositionalArgvCandidatePath(params: {
   segment: ExecCommandSegment;
   cwd?: string;
   env?: NodeJS.ProcessEnv;
+  platform?: string | null;
 }): string | undefined {
   if (!isShellWrapperSegment(params.segment)) {
     return undefined;
@@ -860,7 +862,12 @@ function resolveShellWrapperPositionalArgvCandidatePath(params: {
     return undefined;
   }
 
-  const resolution = resolveCommandResolutionFromArgv([carriedExecutable], params.cwd, params.env);
+  const resolution = resolveCommandResolutionFromArgv(
+    [carriedExecutable],
+    params.cwd,
+    params.env,
+    (params.platform ?? undefined) as NodeJS.Platform | undefined,
+  );
   return resolveExecutionTargetCandidatePath(resolution, params.cwd);
 }
 
@@ -965,7 +972,11 @@ function collectAllowAlwaysPatterns(params: {
     return;
   }
 
-  const trustPlan = resolveExecWrapperTrustPlan(params.segment.argv);
+  const trustPlan = resolveExecWrapperTrustPlan(
+    params.segment.argv,
+    undefined,
+    (params.platform ?? undefined) as NodeJS.Platform | undefined,
+  );
   if (trustPlan.policyBlocked) {
     return;
   }
@@ -976,7 +987,12 @@ function collectAllowAlwaysPatterns(params: {
           raw: trustPlan.argv.join(" "),
           argv: trustPlan.argv,
           sourceArgv: params.segment.sourceArgv,
-          resolution: resolveCommandResolutionFromArgv(trustPlan.argv, params.cwd, params.env),
+          resolution: resolveCommandResolutionFromArgv(
+            trustPlan.argv,
+            params.cwd,
+            params.env,
+            (params.platform ?? undefined) as NodeJS.Platform | undefined,
+          ),
         };
 
   const candidatePath = resolveExecutionTargetTrustPath(segment.resolution, params.cwd);
@@ -1005,6 +1021,7 @@ function collectAllowAlwaysPatterns(params: {
           segment,
           cwd: params.cwd,
           env: params.env,
+          platform: params.platform,
         })
       : undefined;
   if (positionalArgvPath) {

src/infra/exec-approvals-analysis.test.ts

@@ -624,6 +624,49 @@ describe("exec approvals shell analysis", () => {
       expect(result.segmentSatisfiedBy).toEqual(["allowlist"]);
     });
 
+    it.each([
+      {
+        name: "BSD script transcript",
+        command: "script ~/.zshenv git log -1 --format='payload'",
+        platform: "darwin" as const,
+        blockedWrapper: "script",
+      },
+      {
+        name: "GNU time output file",
+        command: "/usr/bin/time -o ~/.bashrc -a -f 'payload' git status",
+        platform: "linux" as const,
+        blockedWrapper: "time",
+      },
+    ])("rejects side-effecting dispatch wrapper allowlist bypasses for $name", (testCase) => {
+      const result = evaluateShellAllowlist({
+        command: testCase.command,
+        allowlist: [{ pattern: "git" }],
+        safeBins: new Set(),
+        cwd: "/tmp",
+        platform: testCase.platform,
+      });
+
+      expect(result.analysisOk).toBe(true);
+      expect(result.allowlistSatisfied).toBe(false);
+      expect(result.segments[0]?.resolution?.policyBlocked).toBe(true);
+      expect(result.segments[0]?.resolution?.blockedWrapper).toBe(testCase.blockedWrapper);
+      expect(result.segmentSatisfiedBy).toEqual([null]);
+    });
+
+    it("keeps GNU time transparent when it only reports to stderr", () => {
+      const result = evaluateShellAllowlist({
+        command: "/usr/bin/time -p git status",
+        allowlist: [{ pattern: "git" }],
+        safeBins: new Set(),
+        cwd: "/tmp",
+        platform: "linux",
+      });
+
+      expect(result.analysisOk).toBe(true);
+      expect(result.allowlistSatisfied).toBe(true);
+      expect(result.segmentSatisfiedBy).toEqual(["allowlist"]);
+    });
+
     it("rejects the legacy skill display prelude when only the wrapper is allowlisted", () => {
       if (process.platform === "win32") {
         return;

src/infra/exec-approvals-analysis.ts

@@ -644,6 +644,7 @@ function analyzeWindowsShellCommand(params: {
   command: string;
   cwd?: string;
   env?: NodeJS.ProcessEnv;
+  platform?: string | null;
 }): ExecCommandAnalysis {
   const effective = stripWindowsShellWrapper(params.command.trim());
   const unsupported = findWindowsUnsupportedToken(effective);
@@ -664,7 +665,12 @@ function analyzeWindowsShellCommand(params: {
       {
         raw: params.command,
         argv,
-        resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env),
+        resolution: resolveCommandResolutionFromArgv(
+          argv,
+          params.cwd,
+          params.env,
+          (params.platform ?? undefined) as NodeJS.Platform | undefined,
+        ),
       },
     ],
   };
@@ -679,6 +685,7 @@ function parseSegmentsFromParts(
   parts: string[],
   cwd?: string,
   env?: NodeJS.ProcessEnv,
+  platform?: string | null,
 ): ExecCommandSegment[] | null {
   const segments: ExecCommandSegment[] = [];
   for (const raw of parts) {
@@ -689,7 +696,12 @@ function parseSegmentsFromParts(
     segments.push({
       raw,
       argv,
-      resolution: resolveCommandResolutionFromArgv(argv, cwd, env),
+      resolution: resolveCommandResolutionFromArgv(
+        argv,
+        cwd,
+        env,
+        (platform ?? undefined) as NodeJS.Platform | undefined,
+      ),
     });
   }
   return segments;
@@ -1206,7 +1218,12 @@ export function analyzeShellCommand(params: {
       if (!pipelineSplit.ok) {
         return { ok: false, reason: pipelineSplit.reason, segments: [] };
       }
-      const segments = parseSegmentsFromParts(pipelineSplit.segments, params.cwd, params.env);
+      const segments = parseSegmentsFromParts(
+        pipelineSplit.segments,
+        params.cwd,
+        params.env,
+        params.platform,
+      );
       if (!segments) {
         return { ok: false, reason: "unable to parse shell segment", segments: [] };
       }
@@ -1222,7 +1239,7 @@ export function analyzeShellCommand(params: {
   if (!split.ok) {
     return { ok: false, reason: split.reason, segments: [] };
   }
-  const segments = parseSegmentsFromParts(split.segments, params.cwd, params.env);
+  const segments = parseSegmentsFromParts(split.segments, params.cwd, params.env, params.platform);
   if (!segments) {
     return { ok: false, reason: "unable to parse shell segment", segments: [] };
   }
@@ -1233,6 +1250,7 @@ export function analyzeArgvCommand(params: {
   argv: string[];
   cwd?: string;
   env?: NodeJS.ProcessEnv;
+  platform?: string | null;
 }): ExecCommandAnalysis {
   const argv = params.argv.filter((entry) => entry.trim().length > 0);
   if (argv.length === 0) {
@@ -1245,7 +1263,12 @@ export function analyzeArgvCommand(params: {
         raw: argv.join(" "),
         argv,
         sourceArgv: [...params.argv],
-        resolution: resolveCommandResolutionFromArgv(argv, params.cwd, params.env),
+        resolution: resolveCommandResolutionFromArgv(
+          argv,
+          params.cwd,
+          params.env,
+          (params.platform ?? undefined) as NodeJS.Platform | undefined,
+        ),
       },
     ],
   };

src/infra/exec-command-resolution.test.ts

@@ -189,6 +189,32 @@ describe("exec-command-resolution", () => {
     expect(timeResolution?.execution.executableName).toBe(fixture.exeName);
   });
 
+  it("keeps file-writing dispatch wrappers on the policy boundary", () => {
+    const timeResolution = resolveCommandResolutionFromArgv([
+      "/usr/bin/time",
+      "-o",
+      "/tmp/time.log",
+      "-a",
+      "-f",
+      "payload",
+      "git",
+      "status",
+    ]);
+    expect(timeResolution?.policyBlocked).toBe(true);
+    expect(timeResolution?.blockedWrapper).toBe("time");
+    expect(timeResolution?.execution.rawExecutable).toBe("/usr/bin/time");
+
+    const scriptResolution = resolveCommandResolutionFromArgv(
+      ["script", "/tmp/session.log", "git", "status"],
+      undefined,
+      undefined,
+      "darwin",
+    );
+    expect(scriptResolution?.policyBlocked).toBe(true);
+    expect(scriptResolution?.blockedWrapper).toBe("script");
+    expect(scriptResolution?.execution.rawExecutable).toBe("script");
+  });
+
   it("keeps shell multiplexer wrappers as a separate policy target", () => {
     if (process.platform === "win32") {
       return;

src/infra/exec-command-resolution.ts

@@ -145,8 +145,9 @@ export function resolveCommandResolutionFromArgv(
   argv: string[],
   cwd?: string,
   env?: NodeJS.ProcessEnv,
+  platform: NodeJS.Platform = process.platform,
 ): CommandResolution | null {
-  const plan = resolveExecWrapperTrustPlan(argv);
+  const plan = resolveExecWrapperTrustPlan(argv, undefined, platform);
   const effectiveArgv = plan.argv;
   const rawExecutable = effectiveArgv[0]?.trim();
   if (!rawExecutable) {

src/infra/exec-wrapper-resolution.test.ts

@@ -366,6 +366,33 @@ describe("resolveDispatchWrapperTrustPlan", () => {
     });
   });
 
+  test("blocks script transcript wrappers even when the inner command is parseable", () => {
+    expect(
+      resolveDispatchWrapperTrustPlan(
+        ["script", "-q", "/tmp/session.log", "bash", "-lc", "echo hi"],
+        undefined,
+        "darwin",
+      ),
+    ).toEqual({
+      argv: ["script", "-q", "/tmp/session.log", "bash", "-lc", "echo hi"],
+      wrappers: ["script"],
+      policyBlocked: true,
+      blockedWrapper: "script",
+    });
+  });
+
+  test.each([
+    ["short output option", ["time", "-o", "/tmp/time.log", "bash", "-lc", "echo hi"]],
+    ["long output option", ["time", "--output=/tmp/time.log", "bash", "-lc", "echo hi"]],
+  ])("blocks GNU time file-output wrappers for %s", (_name, argv) => {
+    expect(resolveDispatchWrapperTrustPlan(argv)).toEqual({
+      argv,
+      wrappers: ["time"],
+      policyBlocked: true,
+      blockedWrapper: "time",
+    });
+  });
+
   test("blocks wrapper overflow beyond the configured depth", () => {
     expect(
       resolveDispatchWrapperTrustPlan(["nohup", "timeout", "5s", "bash", "-lc", "echo hi"], 1),

src/infra/exec-wrapper-trust-plan.ts

@@ -65,13 +65,18 @@ function finalizeExecWrapperTrustPlan(
 export function resolveExecWrapperTrustPlan(
   argv: string[],
   maxDepth = MAX_DISPATCH_WRAPPER_DEPTH,
+  platform: NodeJS.Platform = process.platform,
 ): ExecWrapperTrustPlan {
   let current = argv;
   let policyArgv = argv;
   let sawShellMultiplexer = false;
   const wrapperChain: string[] = [];
   for (let depth = 0; depth < maxDepth; depth += 1) {
-    const dispatchPlan = resolveDispatchWrapperTrustPlan(current, maxDepth - wrapperChain.length);
+    const dispatchPlan = resolveDispatchWrapperTrustPlan(
+      current,
+      maxDepth - wrapperChain.length,
+      platform,
+    );
     if (dispatchPlan.policyBlocked) {
       return blockedExecWrapperTrustPlan({
         argv: dispatchPlan.argv,
@@ -119,7 +124,7 @@ export function resolveExecWrapperTrustPlan(
   }
 
   if (wrapperChain.length >= maxDepth) {
-    const dispatchOverflow = unwrapKnownDispatchWrapperInvocation(current);
+    const dispatchOverflow = unwrapKnownDispatchWrapperInvocation(current, platform);
     if (dispatchOverflow.kind === "blocked" || dispatchOverflow.kind === "unwrapped") {
       return blockedExecWrapperTrustPlan({
         argv: current,