From 90383e00e978a07c2bc004c8cc71b96a3170c318 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Feb 2026 02:52:57 +0000 Subject: [PATCH] fix(security): harden autoAllowSkills exec matching --- CHANGELOG.md | 1 + src/infra/exec-approvals-allowlist.ts | 21 +++++++++-- src/infra/exec-approvals.test.ts | 53 +++++++++++++++++++++++++++ 3 files changed, 71 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0f15aaa71f..54c30bc75e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ Docs: https://docs.openclaw.ai - Security/Voice Call: harden Twilio webhook replay handling by preserving provider event IDs through normalization, adding bounded replay dedupe, and enforcing per-call turn-token matching for call-state transitions. This ships in the next npm release. Thanks @jiseoung for reporting. - Security/Export session HTML: escape raw HTML markdown tokens in the exported session viewer, harden tree/header metadata rendering against HTML injection, and sanitize image data-URL MIME types in export output to prevent stored XSS when opening exported HTML files. This ships in the next npm release. Thanks @allsmog for reporting. - Security/iOS deep links: require local confirmation (or trusted key) before forwarding `openclaw://agent` requests from iOS to gateway `agent.request`, and strip unkeyed delivery-routing fields to reduce exfiltration risk. This ships in the next npm release. Thanks @GCXWLP for reporting. +- Security/Exec approvals: harden `autoAllowSkills` matching to require pathless invocations with resolved executables, blocking `./`/absolute-path basename collisions from satisfying skill auto-allow checks under allowlist mode. - Security/Commands: enforce sender-only matching for `commands.allowFrom` by blocking conversation-shaped `From` identities (`channel:`, `group:`, `thread:`, `@g.us`) while preserving direct-message fallback when sender fields are missing. Ships in the next npm release. Thanks @jiseoung. - Config/Kilo Gateway: Kilo provider flow now surfaces an updated list of models. (#24921) thanks @gumadeiras. - Security/Sandbox: enforce `tools.exec.applyPatch.workspaceOnly` and `tools.fs.workspaceOnly` for `apply_patch` in sandbox-mounted paths so writes/deletes cannot escape the workspace boundary via mounts like `/agent` unless explicitly opted out (`tools.exec.applyPatch.workspaceOnly=false`). This ships in the next npm release. Thanks @tdjackey for reporting. diff --git a/src/infra/exec-approvals-allowlist.ts b/src/infra/exec-approvals-allowlist.ts index ba321b609c7..6d48347e403 100644 --- a/src/infra/exec-approvals-allowlist.ts +++ b/src/infra/exec-approvals-allowlist.ts @@ -92,6 +92,10 @@ export function isSafeBinUsage(params: { return validateSafeBinArgv(argv, profile); } +function isPathScopedExecutableToken(token: string): boolean { + return token.includes("/") || token.includes("\\"); +} + export type ExecAllowlistEvaluation = { allowlistSatisfied: boolean; allowlistMatches: ExecAllowlistEntry[]; @@ -147,10 +151,19 @@ function evaluateSegments( platform: params.platform, trustedSafeBinDirs: params.trustedSafeBinDirs, }); - const skillAllow = - allowSkills && segment.resolution?.executableName - ? params.skillBins?.has(segment.resolution.executableName) - : false; + const rawExecutable = segment.resolution?.rawExecutable?.trim() ?? ""; + const executableName = segment.resolution?.executableName; + const usesExplicitPath = isPathScopedExecutableToken(rawExecutable); + let skillAllow = false; + if ( + allowSkills && + segment.resolution?.resolvedPath && + rawExecutable.length > 0 && + !usesExplicitPath && + executableName + ) { + skillAllow = Boolean(params.skillBins?.has(executableName)); + } const by: ExecSegmentSatisfiedBy = match ? "allowlist" : safe diff --git a/src/infra/exec-approvals.test.ts b/src/infra/exec-approvals.test.ts index 2e7702714be..cddc8cfbdf6 100644 --- a/src/infra/exec-approvals.test.ts +++ b/src/infra/exec-approvals.test.ts @@ -627,6 +627,59 @@ describe("exec approvals allowlist evaluation", () => { }); expect(result.allowlistSatisfied).toBe(true); }); + + it("does not satisfy auto-allow skills for explicit relative paths", () => { + const analysis = { + ok: true, + segments: [ + { + raw: "./skill-bin", + argv: ["./skill-bin", "--help"], + resolution: { + rawExecutable: "./skill-bin", + resolvedPath: "/tmp/skill-bin", + executableName: "skill-bin", + }, + }, + ], + }; + const result = evaluateExecAllowlist({ + analysis, + allowlist: [], + safeBins: new Set(), + skillBins: new Set(["skill-bin"]), + autoAllowSkills: true, + cwd: "/tmp", + }); + expect(result.allowlistSatisfied).toBe(false); + expect(result.segmentSatisfiedBy).toEqual([null]); + }); + + it("does not satisfy auto-allow skills when command resolution is missing", () => { + const analysis = { + ok: true, + segments: [ + { + raw: "skill-bin --help", + argv: ["skill-bin", "--help"], + resolution: { + rawExecutable: "skill-bin", + executableName: "skill-bin", + }, + }, + ], + }; + const result = evaluateExecAllowlist({ + analysis, + allowlist: [], + safeBins: new Set(), + skillBins: new Set(["skill-bin"]), + autoAllowSkills: true, + cwd: "/tmp", + }); + expect(result.allowlistSatisfied).toBe(false); + expect(result.segmentSatisfiedBy).toEqual([null]); + }); }); describe("exec approvals policy helpers", () => {