fix: bind macOS skill trust to resolved paths

This commit is contained in:
Peter Steinberger 2026-03-13 21:00:48 +00:00
parent fc140bb02b
commit 4d686b47f0
No known key found for this signature in database
4 changed files with 187 additions and 10 deletions

View File

@ -35,6 +35,7 @@ Docs: https://docs.openclaw.ai
- Security/exec approvals: recognize PowerShell `-File` and `-f` wrapper forms during inline-command extraction so approval and command-analysis paths treat file-based PowerShell launches like the existing `-Command` variants.
- Security/exec approvals: unwrap `env` dispatch wrappers inside shell-segment allowlist resolution on macOS so `env FOO=bar /path/to/bin` resolves against the effective executable instead of the wrapper token.
- Security/exec approvals: treat backslash-newline as shell line continuation during macOS shell-chain parsing so line-continued `$(` substitutions fail closed instead of slipping past command-substitution checks.
- Security/exec approvals: bind macOS skill auto-allow trust to both executable name and resolved path so same-basename binaries no longer inherit trust from unrelated skill bins.
- Security/external content: strip zero-width and soft-hyphen marker-splitting characters during boundary sanitization so spoofed `EXTERNAL_UNTRUSTED_CONTENT` markers fall back to the existing hardening path instead of bypassing marker normalization.
- Control UI/insecure auth: preserve explicit shared token and password auth on plain-HTTP Control UI connects so LAN and reverse-proxy sessions no longer drop shared auth before the first WebSocket handshake. (#45088) Thanks @velvet-shark.
- macOS/onboarding: avoid self-restarting freshly bootstrapped launchd gateways and give new daemon installs longer to become healthy, so `openclaw onboard --install-daemon` no longer false-fails on slower Macs and fresh VM snapshots.

View File

@ -45,8 +45,8 @@ enum ExecApprovalEvaluator {
let skillAllow: Bool
if approvals.agent.autoAllowSkills, !allowlistResolutions.isEmpty {
let bins = await SkillBinsCache.shared.currentBins()
skillAllow = allowlistResolutions.allSatisfy { bins.contains($0.executableName) }
let bins = await SkillBinsCache.shared.currentTrust()
skillAllow = self.isSkillAutoAllowed(allowlistResolutions, trustedBinsByName: bins)
} else {
skillAllow = false
}
@ -65,4 +65,26 @@ enum ExecApprovalEvaluator {
allowlistMatch: allowlistSatisfied ? allowlistMatches.first : nil,
skillAllow: skillAllow)
}
static func isSkillAutoAllowed(
_ resolutions: [ExecCommandResolution],
trustedBinsByName: [String: Set<String>]) -> Bool
{
guard !resolutions.isEmpty, !trustedBinsByName.isEmpty else { return false }
return resolutions.allSatisfy { resolution in
guard let executableName = SkillBinsCache.normalizeSkillBinName(resolution.executableName),
let resolvedPath = SkillBinsCache.normalizeResolvedPath(resolution.resolvedPath)
else {
return false
}
return trustedBinsByName[executableName]?.contains(resolvedPath) == true
}
}
static func _testIsSkillAutoAllowed(
_ resolutions: [ExecCommandResolution],
trustedBinsByName: [String: Set<String>]) -> Bool
{
self.isSkillAutoAllowed(resolutions, trustedBinsByName: trustedBinsByName)
}
}

View File

@ -777,6 +777,7 @@ actor SkillBinsCache {
static let shared = SkillBinsCache()
private var bins: Set<String> = []
private var trustByName: [String: Set<String>] = [:]
private var lastRefresh: Date?
private let refreshInterval: TimeInterval = 90
@ -787,27 +788,90 @@ actor SkillBinsCache {
return self.bins
}
func currentTrust(force: Bool = false) async -> [String: Set<String>] {
if force || self.isStale() {
await self.refresh()
}
return self.trustByName
}
func refresh() async {
do {
let report = try await GatewayConnection.shared.skillsStatus()
var next = Set<String>()
for skill in report.skills {
for bin in skill.requirements.bins {
let trimmed = bin.trimmingCharacters(in: .whitespacesAndNewlines)
if !trimmed.isEmpty { next.insert(trimmed) }
}
}
self.bins = next
let trust = Self.buildTrustIndex(report: report, searchPaths: CommandResolver.preferredPaths())
self.bins = trust.names
self.trustByName = trust.pathsByName
self.lastRefresh = Date()
} catch {
if self.lastRefresh == nil {
self.bins = []
self.trustByName = [:]
}
}
}
static func normalizeSkillBinName(_ value: String) -> String? {
let trimmed = value.trimmingCharacters(in: .whitespacesAndNewlines).lowercased()
return trimmed.isEmpty ? nil : trimmed
}
static func normalizeResolvedPath(_ value: String?) -> String? {
let trimmed = value?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
guard !trimmed.isEmpty else { return nil }
return URL(fileURLWithPath: trimmed).standardizedFileURL.path
}
static func buildTrustIndex(
report: SkillsStatusReport,
searchPaths: [String]) -> SkillBinTrustIndex
{
var names = Set<String>()
var pathsByName: [String: Set<String>] = [:]
for skill in report.skills {
for bin in skill.requirements.bins {
let trimmed = bin.trimmingCharacters(in: .whitespacesAndNewlines)
guard !trimmed.isEmpty else { continue }
names.insert(trimmed)
guard let name = self.normalizeSkillBinName(trimmed),
let resolvedPath = self.resolveSkillBinPath(trimmed, searchPaths: searchPaths),
let normalizedPath = self.normalizeResolvedPath(resolvedPath)
else {
continue
}
var paths = pathsByName[name] ?? Set<String>()
paths.insert(normalizedPath)
pathsByName[name] = paths
}
}
return SkillBinTrustIndex(names: names, pathsByName: pathsByName)
}
private static func resolveSkillBinPath(_ bin: String, searchPaths: [String]) -> String? {
let expanded = bin.hasPrefix("~") ? (bin as NSString).expandingTildeInPath : bin
if expanded.contains("/") || expanded.contains("\\") {
return FileManager().isExecutableFile(atPath: expanded) ? expanded : nil
}
return CommandResolver.findExecutable(named: expanded, searchPaths: searchPaths)
}
private func isStale() -> Bool {
guard let lastRefresh else { return true }
return Date().timeIntervalSince(lastRefresh) > self.refreshInterval
}
static func _testBuildTrustIndex(
report: SkillsStatusReport,
searchPaths: [String]) -> SkillBinTrustIndex
{
self.buildTrustIndex(report: report, searchPaths: searchPaths)
}
}
struct SkillBinTrustIndex {
let names: Set<String>
let pathsByName: [String: Set<String>]
}

View File

@ -0,0 +1,90 @@
import Foundation
import Testing
@testable import OpenClaw
struct ExecSkillBinTrustTests {
@Test func `build trust index resolves skill bin paths`() throws {
let fixture = try Self.makeExecutable(named: "jq")
defer { try? FileManager.default.removeItem(at: fixture.root) }
let trust = SkillBinsCache._testBuildTrustIndex(
report: Self.makeReport(bins: ["jq"]),
searchPaths: [fixture.root.path])
#expect(trust.names == ["jq"])
#expect(trust.pathsByName["jq"] == [fixture.path])
}
@Test func `skill auto allow accepts trusted resolved skill bin path`() throws {
let fixture = try Self.makeExecutable(named: "jq")
defer { try? FileManager.default.removeItem(at: fixture.root) }
let trust = SkillBinsCache._testBuildTrustIndex(
report: Self.makeReport(bins: ["jq"]),
searchPaths: [fixture.root.path])
let resolution = ExecCommandResolution(
rawExecutable: "jq",
resolvedPath: fixture.path,
executableName: "jq",
cwd: nil)
#expect(ExecApprovalEvaluator._testIsSkillAutoAllowed([resolution], trustedBinsByName: trust.pathsByName))
}
@Test func `skill auto allow rejects same basename at different path`() throws {
let trusted = try Self.makeExecutable(named: "jq")
let untrusted = try Self.makeExecutable(named: "jq")
defer {
try? FileManager.default.removeItem(at: trusted.root)
try? FileManager.default.removeItem(at: untrusted.root)
}
let trust = SkillBinsCache._testBuildTrustIndex(
report: Self.makeReport(bins: ["jq"]),
searchPaths: [trusted.root.path])
let resolution = ExecCommandResolution(
rawExecutable: "jq",
resolvedPath: untrusted.path,
executableName: "jq",
cwd: nil)
#expect(!ExecApprovalEvaluator._testIsSkillAutoAllowed([resolution], trustedBinsByName: trust.pathsByName))
}
private static func makeExecutable(named name: String) throws -> (root: URL, path: String) {
let root = FileManager.default.temporaryDirectory
.appendingPathComponent("openclaw-skill-bin-\(UUID().uuidString)", isDirectory: true)
try FileManager.default.createDirectory(at: root, withIntermediateDirectories: true)
let file = root.appendingPathComponent(name)
try "#!/bin/sh\nexit 0\n".write(to: file, atomically: true, encoding: .utf8)
try FileManager.default.setAttributes(
[.posixPermissions: NSNumber(value: Int16(0o755))],
ofItemAtPath: file.path)
return (root, file.path)
}
private static func makeReport(bins: [String]) -> SkillsStatusReport {
SkillsStatusReport(
workspaceDir: "/tmp/workspace",
managedSkillsDir: "/tmp/skills",
skills: [
SkillStatus(
name: "test-skill",
description: "test",
source: "local",
filePath: "/tmp/skills/test-skill/SKILL.md",
baseDir: "/tmp/skills/test-skill",
skillKey: "test-skill",
primaryEnv: nil,
emoji: nil,
homepage: nil,
always: false,
disabled: false,
eligible: true,
requirements: SkillRequirements(bins: bins, env: [], config: []),
missing: SkillMissing(bins: [], env: [], config: []),
configChecks: [],
install: [])
])
}
}