Remove skill prelude exec allowlist (#84570)

Summary:
- The PR removes the legacy `cat SKILL.md && printf ... && <skill-wrapper>` exec-approval allowlist path, updates focused exec-approval tests, and adds a changelog entry.
- Reproducibility: yes. Current-main source and tests show the old `cat SKILL.md && printf ... && <wrapper>` c ... ed this by source and test inspection rather than executing tests because the checkout review is read-only.

Automerge notes:
- PR branch already contained follow-up commit before automerge: Remove skill prelude exec allowlist

Validation:
- ClawSweeper review passed for head 0ca7f3e8ef.
- Required merge gates passed before the squash merge.

Prepared head SHA: 0ca7f3e8ef
Review: https://github.com/openclaw/openclaw/pull/84570#issuecomment-4498357535

Co-authored-by: jesse-merhi <79823012+jesse-merhi@users.noreply.github.com>
Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com>
Approved-by: jesse-merhi

CHANGELOG.md

@@ -6,6 +6,7 @@ Docs: https://docs.openclaw.ai
 
 ### Changes
 
+- Exec approvals: remove the old `cat SKILL.md && printf ... && <skill-wrapper>` allowlist compatibility path so skill files must be loaded with the read tool and only the real skill executable is auto-allowed.
 - Discord: let voice sessions follow configured Discord users into voice channels, with allowed-channel checks, multi-user handoff, bounded reconciliation, and DAVE recovery preservation. (#84264) Thanks @fuller-stack-dev.
 - Discord/voice: include bounded `IDENTITY.md`, `USER.md`, and `SOUL.md` profile context in realtime voice session instructions by default, with `voice.realtime.bootstrapContextFiles: []` available to disable it. (#84499) Thanks @fuller-stack-dev.
 - Dependencies: bump the bundled Codex harness to `@openai/codex` `0.132.0` and refresh the app-server model-list docs for the new catalog.

src/agents/bash-tools.exec.approval-id.test.ts

@@ -1228,28 +1228,25 @@ describe("exec approvals", () => {
     expect(calls).toContain("exec.approval.request");
   });
 
-  it("runs a skill wrapper chain without prompting when the wrapper is allowlisted", async () => {
+  it("runs a direct skill wrapper command without prompting when the wrapper is allowlisted", async () => {
     if (process.platform === "win32") {
       return;
     }
     const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skill-wrapper-"));
     try {
-      const skillDir = path.join(tempDir, ".openclaw", "skills", "gog");
-      const skillPath = path.join(skillDir, "SKILL.md");
       const binDir = path.join(tempDir, "bin");
       const wrapperPath = path.join(binDir, "gog-wrapper");
-      await fs.mkdir(skillDir, { recursive: true });
       await fs.mkdir(binDir, { recursive: true });
-      await fs.writeFile(skillPath, "# gog skill\n");
       await fs.writeFile(wrapperPath, "#!/bin/sh\necho '{\"events\":[]}'\n");
       await fs.chmod(wrapperPath, 0o755);
+      const trustedWrapperPath = await fs.realpath(wrapperPath);
 
       await writeExecApprovalsConfig({
         version: 1,
         defaults: { security: "allowlist", ask: "off", askFallback: "deny" },
         agents: {
           main: {
-            allowlist: [{ pattern: wrapperPath }],
+            allowlist: [{ pattern: trustedWrapperPath }],
           },
         },
       });
@@ -1265,7 +1262,7 @@ describe("exec approvals", () => {
       });
 
       const result = await tool.execute("call-skill-wrapper", {
-        command: `cat ${JSON.stringify(skillPath)} && printf '\\n---CMD---\\n' && ${JSON.stringify(wrapperPath)} calendar events primary --today --json`,
+        command: `${JSON.stringify(wrapperPath)} calendar events primary --today --json`,
         workdir: tempDir,
       });
 
@@ -1277,6 +1274,65 @@ describe("exec approvals", () => {
     }
   });
 
+  it("requires approval for the legacy skill display prelude even when the wrapper is allowlisted", async () => {
+    if (process.platform === "win32") {
+      return;
+    }
+    const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-skill-prelude-"));
+    try {
+      const skillDir = path.join(tempDir, ".openclaw", "skills", "gog");
+      const skillPath = path.join(skillDir, "SKILL.md");
+      const binDir = path.join(tempDir, "bin");
+      const wrapperPath = path.join(binDir, "gog-wrapper");
+      await fs.mkdir(skillDir, { recursive: true });
+      await fs.mkdir(binDir, { recursive: true });
+      await fs.writeFile(skillPath, "# gog skill\n");
+      await fs.writeFile(wrapperPath, "#!/bin/sh\necho '{\"events\":[]}'\n");
+      await fs.chmod(wrapperPath, 0o755);
+      const trustedWrapperPath = await fs.realpath(wrapperPath);
+
+      await writeExecApprovalsConfig({
+        version: 1,
+        defaults: { security: "allowlist", ask: "on-miss", askFallback: "deny" },
+        agents: {
+          main: {
+            allowlist: [{ pattern: trustedWrapperPath }],
+          },
+        },
+      });
+
+      const calls: string[] = [];
+      vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {
+        calls.push(method);
+        if (method === "exec.approval.request") {
+          return acceptedApprovalResponse(params);
+        }
+        if (method === "exec.approval.waitDecision") {
+          return { decision: "deny" };
+        }
+        return { ok: true };
+      });
+
+      const tool = createExecTool({
+        host: "gateway",
+        ask: "on-miss",
+        security: "allowlist",
+        approvalRunningNoticeMs: 0,
+      });
+
+      const command = `cat ${JSON.stringify(skillPath)} && printf '\\n---CMD---\\n' && ${JSON.stringify(wrapperPath)} calendar events primary --today --json`;
+      const result = await tool.execute("call-skill-prelude", {
+        command,
+        workdir: tempDir,
+      });
+
+      expectPendingCommandText(result, command);
+      expect(calls).toContain("exec.approval.request");
+    } finally {
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
+  });
+
   it("shows full chained node commands in approval-pending message", async () => {
     const calls: string[] = [];
     vi.mocked(callGatewayTool).mockImplementation(async (method, _opts, params) => {

src/infra/exec-approvals-allowlist.ts

@@ -27,7 +27,6 @@ import {
   type ExecCommandAnalysis,
   type ExecCommandSegment,
   type ExecutableResolution,
-  type ShellChainOperator,
 } from "./exec-approvals-analysis.js";
 import type { ExecAllowlistEntry } from "./exec-approvals.types.js";
 import {
@@ -132,13 +131,7 @@ export type ExecAllowlistEvaluation = {
   segmentSatisfiedBy: ExecSegmentSatisfiedBy[];
 };
 
-export type ExecSegmentSatisfiedBy =
-  | "allowlist"
-  | "safeBins"
-  | "inlineChain"
-  | "skills"
-  | "skillPrelude"
-  | null;
+export type ExecSegmentSatisfiedBy = "allowlist" | "safeBins" | "inlineChain" | "skills" | null;
 export type SkillBinTrustEntry = {
   name: string;
   resolvedPath: string;
@@ -232,163 +225,6 @@ function isSkillAutoAllowedSegment(params: {
   return Boolean(params.skillBinTrust.get(executableName)?.has(resolvedPath));
 }
 
-function resolveSkillPreludePath(rawPath: string, cwd?: string): string {
-  const expanded = rawPath.startsWith("~") ? expandHomePrefix(rawPath) : rawPath;
-  if (path.isAbsolute(expanded)) {
-    return path.resolve(expanded);
-  }
-  return path.resolve(cwd?.trim() || process.cwd(), expanded);
-}
-
-function isSkillMarkdownPreludePath(filePath: string): boolean {
-  const normalized = filePath.replace(/\\/g, "/");
-  const lowerNormalized = normalizeLowercaseStringOrEmpty(normalized);
-  if (!lowerNormalized.endsWith("/skill.md")) {
-    return false;
-  }
-  const parts = lowerNormalized.split("/").filter(Boolean);
-  if (parts.length < 2) {
-    return false;
-  }
-  for (let index = parts.length - 2; index >= 0; index -= 1) {
-    if (parts[index] !== "skills") {
-      continue;
-    }
-    const segmentsAfterSkills = parts.length - index - 1;
-    if (segmentsAfterSkills === 1 || segmentsAfterSkills === 2) {
-      return true;
-    }
-  }
-  return false;
-}
-
-function resolveSkillMarkdownPreludeId(filePath: string): string | null {
-  const normalized = filePath.replace(/\\/g, "/");
-  const lowerNormalized = normalizeLowercaseStringOrEmpty(normalized);
-  if (!lowerNormalized.endsWith("/skill.md")) {
-    return null;
-  }
-  const parts = lowerNormalized.split("/").filter(Boolean);
-  if (parts.length < 3) {
-    return null;
-  }
-  for (let index = parts.length - 2; index >= 0; index -= 1) {
-    if (parts[index] !== "skills") {
-      continue;
-    }
-    if (parts.length - index - 1 !== 2) {
-      continue;
-    }
-    const skillId = parts[index + 1]?.trim();
-    return skillId || null;
-  }
-  return null;
-}
-
-function isSkillPreludeReadSegment(segment: ExecCommandSegment, cwd?: string): boolean {
-  const execution = resolveExecutionTargetResolution(segment.resolution);
-  if (normalizeLowercaseStringOrEmpty(execution?.executableName) !== "cat") {
-    return false;
-  }
-  // Keep the display-prelude exception narrow: only a plain `cat <...>/SKILL.md`
-  // qualifies, not extra argv forms or arbitrary file reads.
-  if (segment.argv.length !== 2) {
-    return false;
-  }
-  const rawPath = segment.argv[1]?.trim();
-  if (!rawPath) {
-    return false;
-  }
-  return isSkillMarkdownPreludePath(resolveSkillPreludePath(rawPath, cwd));
-}
-
-function isSkillPreludeMarkerSegment(segment: ExecCommandSegment): boolean {
-  const execution = resolveExecutionTargetResolution(segment.resolution);
-  if (normalizeLowercaseStringOrEmpty(execution?.executableName) !== "printf") {
-    return false;
-  }
-  if (segment.argv.length !== 2) {
-    return false;
-  }
-  const marker = segment.argv[1];
-  return marker === "\\n---CMD---\\n" || marker === "\n---CMD---\n";
-}
-
-function isSkillPreludeSegment(segment: ExecCommandSegment, cwd?: string): boolean {
-  return isSkillPreludeReadSegment(segment, cwd) || isSkillPreludeMarkerSegment(segment);
-}
-
-function isSkillPreludeOnlyEvaluation(
-  segments: ExecCommandSegment[],
-  cwd: string | undefined,
-): boolean {
-  return segments.length > 0 && segments.every((segment) => isSkillPreludeSegment(segment, cwd));
-}
-
-function resolveSkillPreludeIds(
-  segments: ExecCommandSegment[],
-  cwd: string | undefined,
-): ReadonlySet<string> {
-  const skillIds = new Set<string>();
-  for (const segment of segments) {
-    if (!isSkillPreludeReadSegment(segment, cwd)) {
-      continue;
-    }
-    const rawPath = segment.argv[1]?.trim();
-    if (!rawPath) {
-      continue;
-    }
-    const skillId = resolveSkillMarkdownPreludeId(resolveSkillPreludePath(rawPath, cwd));
-    if (skillId) {
-      skillIds.add(skillId);
-    }
-  }
-  return skillIds;
-}
-
-function resolveAllowlistedSkillWrapperId(segment: ExecCommandSegment): string | null {
-  const execution = resolveExecutionTargetResolution(segment.resolution);
-  const executableName = normalizeExecutableToken(
-    execution?.executableName ?? segment.argv[0] ?? "",
-  );
-  if (!executableName.endsWith("-wrapper")) {
-    return null;
-  }
-  const skillId = executableName.slice(0, -"-wrapper".length).trim();
-  return skillId || null;
-}
-
-function resolveTrustedSkillExecutionIds(params: {
-  analysis: ExecCommandAnalysis;
-  evaluation: ExecAllowlistEvaluation;
-}): ReadonlySet<string> {
-  const skillIds = new Set<string>();
-  if (!params.evaluation.allowlistSatisfied) {
-    return skillIds;
-  }
-  for (const [index, segment] of params.analysis.segments.entries()) {
-    const satisfiedBy = params.evaluation.segmentSatisfiedBy[index];
-    if (satisfiedBy === "skills") {
-      const execution = resolveExecutionTargetResolution(segment.resolution);
-      const executableName = normalizeExecutableToken(
-        execution?.executableName ?? execution?.rawExecutable ?? segment.argv[0] ?? "",
-      );
-      if (executableName) {
-        skillIds.add(executableName);
-      }
-      continue;
-    }
-    if (satisfiedBy !== "allowlist") {
-      continue;
-    }
-    const wrapperSkillId = resolveAllowlistedSkillWrapperId(segment);
-    if (wrapperSkillId) {
-      skillIds.add(wrapperSkillId);
-    }
-  }
-  return skillIds;
-}
-
 const MAX_SHELL_WRAPPER_INLINE_EVAL_DEPTH = 3;
 
 type InlineChainAllowlistEvaluation = {
@@ -1305,7 +1141,7 @@ export function evaluateShellAllowlist(
     };
   }
 
-  const chainEvaluations = chainParts.map(({ part, opToNext }) => {
+  const chainEvaluations = chainParts.map(({ part }) => {
     const analysis = analyzeShellCommand({
       command: part,
       cwd: params.cwd,
@@ -1318,7 +1154,6 @@ export function evaluateShellAllowlist(
     return {
       analysis,
       evaluation: evaluateExecAllowlist({ analysis, ...allowlistContext }),
-      opToNext,
     };
   });
   if (chainEvaluations.some((entry) => entry === null)) {
@@ -1328,60 +1163,18 @@ export function evaluateShellAllowlist(
   const finalizedEvaluations = chainEvaluations as Array<{
     analysis: ExecCommandAnalysis;
     evaluation: ExecAllowlistEvaluation;
-    opToNext: ShellChainOperator | null;
   }>;
-  const allowSkillPreludeAtIndex = new Set<number>();
-  const reachableSkillIds = new Set<string>();
-  // Only allow the `cat SKILL.md && printf ...` display prelude when it sits on a
-  // contiguous `&&` chain that actually reaches a later trusted skill-wrapper execution.
-  for (let index = finalizedEvaluations.length - 1; index >= 0; index -= 1) {
-    const { analysis, evaluation, opToNext } = finalizedEvaluations[index];
-    const trustedSkillIds = resolveTrustedSkillExecutionIds({
-      analysis,
-      evaluation,
-    });
-    if (trustedSkillIds.size > 0) {
-      for (const skillId of trustedSkillIds) {
-        reachableSkillIds.add(skillId);
-      }
-      continue;
-    }
-
-    const isPreludeOnly =
-      !evaluation.allowlistSatisfied && isSkillPreludeOnlyEvaluation(analysis.segments, params.cwd);
-    const preludeSkillIds = isPreludeOnly
-      ? resolveSkillPreludeIds(analysis.segments, params.cwd)
-      : new Set<string>();
-    const reachesTrustedSkillExecution =
-      opToNext === "&&" &&
-      (preludeSkillIds.size === 0
-        ? reachableSkillIds.size > 0
-        : [...preludeSkillIds].some((skillId) => reachableSkillIds.has(skillId)));
-    if (isPreludeOnly && reachesTrustedSkillExecution) {
-      allowSkillPreludeAtIndex.add(index);
-      continue;
-    }
-
-    reachableSkillIds.clear();
-  }
   const allowlistMatches: ExecAllowlistEntry[] = [];
   const segments: ExecCommandSegment[] = [];
   const segmentAllowlistEntries: Array<ExecAllowlistEntry | null> = [];
   const segmentSatisfiedBy: ExecSegmentSatisfiedBy[] = [];
 
-  for (const [index, { analysis, evaluation }] of finalizedEvaluations.entries()) {
-    const effectiveSegmentSatisfiedBy = allowSkillPreludeAtIndex.has(index)
-      ? analysis.segments.map(() => "skillPrelude" as const)
-      : evaluation.segmentSatisfiedBy;
-    const effectiveSegmentAllowlistEntries = allowSkillPreludeAtIndex.has(index)
-      ? analysis.segments.map(() => null)
-      : evaluation.segmentAllowlistEntries;
-
+  for (const { analysis, evaluation } of finalizedEvaluations) {
     segments.push(...analysis.segments);
     allowlistMatches.push(...evaluation.allowlistMatches);
-    segmentAllowlistEntries.push(...effectiveSegmentAllowlistEntries);
-    segmentSatisfiedBy.push(...effectiveSegmentSatisfiedBy);
-    if (!evaluation.allowlistSatisfied && !allowSkillPreludeAtIndex.has(index)) {
+    segmentAllowlistEntries.push(...evaluation.segmentAllowlistEntries);
+    segmentSatisfiedBy.push(...evaluation.segmentSatisfiedBy);
+    if (!evaluation.allowlistSatisfied) {
       return {
         analysisOk: true,
         allowlistSatisfied: false,

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

@@ -27,21 +27,14 @@ function expectAnalyzedShellCommand(
   return res;
 }
 
-function createSkillPreludeFixture(options: { withWrapper?: boolean } = {}) {
+function createSkillWrapperFixture() {
   const skillRoot = makeTempDir();
-  const skillDir = path.join(skillRoot, "skills", "gog");
-  const skillPath = path.join(skillDir, "SKILL.md");
   const wrapperPath = path.join(skillRoot, "bin", "gog-wrapper");
 
-  fs.mkdirSync(skillDir, { recursive: true });
-  fs.writeFileSync(skillPath, "# gog\n");
+  fs.mkdirSync(path.dirname(wrapperPath), { recursive: true });
+  fs.writeFileSync(wrapperPath, "#!/bin/sh\n", { mode: 0o755 });
 
-  if (options.withWrapper) {
-    fs.mkdirSync(path.dirname(wrapperPath), { recursive: true });
-    fs.writeFileSync(wrapperPath, "#!/bin/sh\n", { mode: 0o755 });
-  }
-
-  return { skillRoot, skillPath, wrapperPath };
+  return { skillRoot, wrapperPath };
 }
 
 describe("exec approvals shell analysis", () => {
@@ -613,16 +606,14 @@ describe("exec approvals shell analysis", () => {
       expect(result.allowlistSatisfied).toBe(testCase.expectedAllowlistSatisfied);
     });
 
-    it("allows the skill display prelude when a later skill wrapper is allowlisted", () => {
+    it("allows a direct skill wrapper command when the wrapper is allowlisted", () => {
       if (process.platform === "win32") {
         return;
       }
-      const { skillRoot, skillPath, wrapperPath } = createSkillPreludeFixture({
-        withWrapper: true,
-      });
+      const { skillRoot, wrapperPath } = createSkillWrapperFixture();
 
       const result = evaluateShellAllowlist({
-        command: `cat ${skillPath} && printf '\\n---CMD---\\n' && ${wrapperPath} calendar events primary --today --json`,
+        command: `${wrapperPath} calendar events primary --today --json`,
         allowlist: [{ pattern: wrapperPath }],
         safeBins: new Set(),
         cwd: skillRoot,
@@ -630,55 +621,21 @@ describe("exec approvals shell analysis", () => {
 
       expect(result.analysisOk).toBe(true);
       expect(result.allowlistSatisfied).toBe(true);
-      expect(result.segmentSatisfiedBy).toEqual(["skillPrelude", "skillPrelude", "allowlist"]);
+      expect(result.segmentSatisfiedBy).toEqual(["allowlist"]);
     });
 
-    it("does not treat arbitrary allowlisted binaries as trusted skill wrappers", () => {
+    it("rejects the legacy skill display prelude when only the wrapper is allowlisted", () => {
       if (process.platform === "win32") {
         return;
       }
-      const { skillRoot, skillPath } = createSkillPreludeFixture();
+      const { skillRoot, wrapperPath } = createSkillWrapperFixture();
+      const skillDir = path.join(skillRoot, "skills", "gog");
+      const skillPath = path.join(skillDir, "SKILL.md");
+      fs.mkdirSync(skillDir, { recursive: true });
+      fs.writeFileSync(skillPath, "# gog\n");
 
       const result = evaluateShellAllowlist({
-        command: `cat ${skillPath} && printf '\\n---CMD---\\n' && /bin/echo calendar events primary --today --json`,
-        allowlist: [{ pattern: "/bin/echo" }],
-        safeBins: new Set(),
-        cwd: skillRoot,
-      });
-
-      expect(result.analysisOk).toBe(true);
-      expect(result.allowlistSatisfied).toBe(false);
-      expect(result.segmentSatisfiedBy).toEqual([null]);
-    });
-
-    it("still rejects the skill display prelude when no trusted skill command follows", () => {
-      if (process.platform === "win32") {
-        return;
-      }
-      const { skillRoot, skillPath } = createSkillPreludeFixture();
-
-      const result = evaluateShellAllowlist({
-        command: `cat ${skillPath} && printf '\\n---CMD---\\n'`,
-        allowlist: [],
-        safeBins: new Set(),
-        cwd: skillRoot,
-      });
-
-      expect(result.analysisOk).toBe(true);
-      expect(result.allowlistSatisfied).toBe(false);
-      expect(result.segmentSatisfiedBy).toEqual([null]);
-    });
-
-    it("rejects the skill display prelude when a trusted wrapper is not reachable", () => {
-      if (process.platform === "win32") {
-        return;
-      }
-      const { skillRoot, skillPath, wrapperPath } = createSkillPreludeFixture({
-        withWrapper: true,
-      });
-
-      const result = evaluateShellAllowlist({
-        command: `cat ${skillPath} && printf '\\n---CMD---\\n' && false && ${wrapperPath} calendar events primary --today --json`,
+        command: `cat ${skillPath} && printf '\\n---CMD---\\n' && ${wrapperPath} calendar events primary --today --json`,
         allowlist: [{ pattern: wrapperPath }],
         safeBins: new Set(),
         cwd: skillRoot,
@@ -1011,7 +968,7 @@ describe("exec approvals shell analysis", () => {
           const printfPath = binPath("printf");
           const gogPath = binPath("gog-wrapper");
           const result = evaluateShellAllowlist({
-            command: `${shellPath} -c "cat SKILL.md && printf '---CMD---' && gog-wrapper calendar events"`,
+            command: `${shellPath} -c "cat README.md && printf ready && gog-wrapper calendar events"`,
             allowlist: [{ pattern: catPath }, { pattern: printfPath }, { pattern: gogPath }],
             safeBins: new Set(),
             cwd: dir,
@@ -1036,7 +993,7 @@ describe("exec approvals shell analysis", () => {
           const catPath = binPath("cat");
           const gogPath = binPath("gog-wrapper");
           const result = evaluateShellAllowlist({
-            command: `${shellPath} -c "cat SKILL.md && rm -rf / && gog-wrapper calendar events"`,
+            command: `${shellPath} -c "cat README.md && rm -rf / && gog-wrapper calendar events"`,
             allowlist: [{ pattern: catPath }, { pattern: gogPath }],
             safeBins: new Set(),
             cwd: dir,

src/infra/exec-approvals-analysis.ts

@@ -1104,14 +1104,7 @@ function renderInlineChainSegmentArgv(params: {
 export function buildSafeBinsShellCommand(params: {
   command: string;
   segments: ExecCommandSegment[];
-  segmentSatisfiedBy: (
-    | "allowlist"
-    | "safeBins"
-    | "inlineChain"
-    | "skills"
-    | "skillPrelude"
-    | null
-  )[];
+  segmentSatisfiedBy: ("allowlist" | "safeBins" | "inlineChain" | "skills" | null)[];
   cwd?: string;
   env?: NodeJS.ProcessEnv;
   platform?: string | null;