diff --git a/CHANGELOG.md b/CHANGELOG.md index b495fb798bb..f486b910056 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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. diff --git a/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift b/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift index c7d9d0928e1..a36e58db1d8 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovalEvaluation.swift @@ -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]) -> 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]) -> Bool + { + self.isSkillAutoAllowed(resolutions, trustedBinsByName: trustedBinsByName) + } } diff --git a/apps/macos/Sources/OpenClaw/ExecApprovals.swift b/apps/macos/Sources/OpenClaw/ExecApprovals.swift index ba49b37cd9f..7fc4385b96c 100644 --- a/apps/macos/Sources/OpenClaw/ExecApprovals.swift +++ b/apps/macos/Sources/OpenClaw/ExecApprovals.swift @@ -777,6 +777,7 @@ actor SkillBinsCache { static let shared = SkillBinsCache() private var bins: Set = [] + private var trustByName: [String: Set] = [:] 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] { + if force || self.isStale() { + await self.refresh() + } + return self.trustByName + } + func refresh() async { do { let report = try await GatewayConnection.shared.skillsStatus() - var next = Set() - 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() + var pathsByName: [String: Set] = [:] + + 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() + 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 + let pathsByName: [String: Set] } diff --git a/apps/macos/Tests/OpenClawIPCTests/ExecSkillBinTrustTests.swift b/apps/macos/Tests/OpenClawIPCTests/ExecSkillBinTrustTests.swift new file mode 100644 index 00000000000..779b59a3499 --- /dev/null +++ b/apps/macos/Tests/OpenClawIPCTests/ExecSkillBinTrustTests.swift @@ -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: []) + ]) + } +}