fix: improve macOS exec approvals
This commit is contained in:
@@ -27,6 +27,7 @@ Docs: https://docs.clawd.bot
|
|||||||
- **BREAKING:** Control UI now rejects insecure HTTP without device identity by default. Use HTTPS (Tailscale Serve) or set `gateway.controlUi.allowInsecureAuth: true` to allow token-only auth. https://docs.clawd.bot/web/control-ui#insecure-http
|
- **BREAKING:** Control UI now rejects insecure HTTP without device identity by default. Use HTTPS (Tailscale Serve) or set `gateway.controlUi.allowInsecureAuth: true` to allow token-only auth. https://docs.clawd.bot/web/control-ui#insecure-http
|
||||||
|
|
||||||
### Fixes
|
### Fixes
|
||||||
|
- Nodes/macOS: prompt on allowlist miss for node exec approvals, persist allowlist decisions, and flatten node invoke errors. (#1394) Thanks @ngutman.
|
||||||
- Gateway: keep auto bind loopback-first and add explicit tailnet binding to avoid Tailscale taking over local UI. (#1380)
|
- Gateway: keep auto bind loopback-first and add explicit tailnet binding to avoid Tailscale taking over local UI. (#1380)
|
||||||
- Embedded runner: persist injected history images so attachments aren’t reloaded each turn. (#1374) Thanks @Nicell.
|
- Embedded runner: persist injected history images so attachments aren’t reloaded each turn. (#1374) Thanks @Nicell.
|
||||||
- Nodes tool: include agent/node/gateway context in tool failure logs to speed approval debugging.
|
- Nodes tool: include agent/node/gateway context in tool failure logs to speed approval debugging.
|
||||||
|
|||||||
@@ -554,6 +554,30 @@ enum ExecCommandFormatter {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
enum ExecApprovalHelpers {
|
||||||
|
static func parseDecision(_ raw: String?) -> ExecApprovalDecision? {
|
||||||
|
let trimmed = raw?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
|
||||||
|
guard !trimmed.isEmpty else { return nil }
|
||||||
|
return ExecApprovalDecision(rawValue: trimmed)
|
||||||
|
}
|
||||||
|
|
||||||
|
static func requiresAsk(
|
||||||
|
ask: ExecAsk,
|
||||||
|
security: ExecSecurity,
|
||||||
|
allowlistMatch: ExecAllowlistEntry?,
|
||||||
|
skillAllow: Bool) -> Bool
|
||||||
|
{
|
||||||
|
if ask == .always { return true }
|
||||||
|
if ask == .onMiss, security == .allowlist, allowlistMatch == nil, !skillAllow { return true }
|
||||||
|
return false
|
||||||
|
}
|
||||||
|
|
||||||
|
static func allowlistPattern(command: [String], resolution: ExecCommandResolution?) -> String? {
|
||||||
|
let pattern = resolution?.resolvedPath ?? resolution?.rawExecutable ?? command.first ?? ""
|
||||||
|
return pattern.isEmpty ? nil : pattern
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
enum ExecAllowlistMatcher {
|
enum ExecAllowlistMatcher {
|
||||||
static func match(entries: [ExecAllowlistEntry], resolution: ExecCommandResolution?) -> ExecAllowlistEntry? {
|
static func match(entries: [ExecAllowlistEntry], resolution: ExecCommandResolution?) -> ExecAllowlistEntry? {
|
||||||
guard let resolution, !entries.isEmpty else { return nil }
|
guard let resolution, !entries.isEmpty else { return nil }
|
||||||
|
|||||||
@@ -314,7 +314,7 @@ private enum ExecHostExecutor {
|
|||||||
}
|
}
|
||||||
|
|
||||||
var approvedByAsk = approvalDecision != nil
|
var approvedByAsk = approvalDecision != nil
|
||||||
if self.requiresAsk(
|
if ExecApprovalHelpers.requiresAsk(
|
||||||
ask: context.ask,
|
ask: context.ask,
|
||||||
security: context.security,
|
security: context.security,
|
||||||
allowlistMatch: context.allowlistMatch,
|
allowlistMatch: context.allowlistMatch,
|
||||||
@@ -417,36 +417,20 @@ private enum ExecHostExecutor {
|
|||||||
skillAllow: skillAllow)
|
skillAllow: skillAllow)
|
||||||
}
|
}
|
||||||
|
|
||||||
private static func requiresAsk(
|
|
||||||
ask: ExecAsk,
|
|
||||||
security: ExecSecurity,
|
|
||||||
allowlistMatch: ExecAllowlistEntry?,
|
|
||||||
skillAllow: Bool) -> Bool
|
|
||||||
{
|
|
||||||
if ask == .always { return true }
|
|
||||||
if ask == .onMiss, security == .allowlist, allowlistMatch == nil, !skillAllow { return true }
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
|
|
||||||
private static func persistAllowlistEntry(
|
private static func persistAllowlistEntry(
|
||||||
decision: ExecApprovalDecision?,
|
decision: ExecApprovalDecision?,
|
||||||
context: ExecApprovalContext)
|
context: ExecApprovalContext)
|
||||||
{
|
{
|
||||||
guard decision == .allowAlways, context.security == .allowlist else { return }
|
guard decision == .allowAlways, context.security == .allowlist else { return }
|
||||||
guard let pattern = self.allowlistPattern(command: context.command, resolution: context.resolution) else {
|
guard let pattern = ExecApprovalHelpers.allowlistPattern(
|
||||||
|
command: context.command,
|
||||||
|
resolution: context.resolution)
|
||||||
|
else {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
ExecApprovalsStore.addAllowlistEntry(agentId: context.trimmedAgent, pattern: pattern)
|
ExecApprovalsStore.addAllowlistEntry(agentId: context.trimmedAgent, pattern: pattern)
|
||||||
}
|
}
|
||||||
|
|
||||||
private static func allowlistPattern(
|
|
||||||
command: [String],
|
|
||||||
resolution: ExecCommandResolution?) -> String?
|
|
||||||
{
|
|
||||||
let pattern = resolution?.resolvedPath ?? resolution?.rawExecutable ?? command.first ?? ""
|
|
||||||
return pattern.isEmpty ? nil : pattern
|
|
||||||
}
|
|
||||||
|
|
||||||
private static func ensureScreenRecordingAccess(_ needsScreenRecording: Bool?) async -> ExecHostResponse? {
|
private static func ensureScreenRecordingAccess(_ needsScreenRecording: Bool?) async -> ExecHostResponse? {
|
||||||
guard needsScreenRecording == true else { return nil }
|
guard needsScreenRecording == true else { return nil }
|
||||||
let authorized = await PermissionManager
|
let authorized = await PermissionManager
|
||||||
|
|||||||
@@ -480,13 +480,13 @@ actor MacNodeRuntime {
|
|||||||
message: "SYSTEM_RUN_DISABLED: security=deny")
|
message: "SYSTEM_RUN_DISABLED: security=deny")
|
||||||
}
|
}
|
||||||
|
|
||||||
let requiresAsk: Bool = {
|
let requiresAsk = ExecApprovalHelpers.requiresAsk(
|
||||||
if ask == .always { return true }
|
ask: ask,
|
||||||
if ask == .onMiss, security == .allowlist, allowlistMatch == nil, !skillAllow { return true }
|
security: security,
|
||||||
return false
|
allowlistMatch: allowlistMatch,
|
||||||
}()
|
skillAllow: skillAllow)
|
||||||
|
|
||||||
let decisionFromParams = Self.parseApprovalDecision(params.approvalDecision)
|
let decisionFromParams = ExecApprovalHelpers.parseDecision(params.approvalDecision)
|
||||||
var approvedByAsk = params.approved == true || decisionFromParams != nil
|
var approvedByAsk = params.approved == true || decisionFromParams != nil
|
||||||
var persistAllowlist = decisionFromParams == .allowAlways
|
var persistAllowlist = decisionFromParams == .allowAlways
|
||||||
if decisionFromParams == .deny {
|
if decisionFromParams == .deny {
|
||||||
@@ -536,14 +536,10 @@ actor MacNodeRuntime {
|
|||||||
approvedByAsk = true
|
approvedByAsk = true
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
if persistAllowlist, security == .allowlist {
|
if persistAllowlist, security == .allowlist,
|
||||||
let pattern = resolution?.resolvedPath
|
let pattern = ExecApprovalHelpers.allowlistPattern(command: command, resolution: resolution)
|
||||||
?? resolution?.rawExecutable
|
{
|
||||||
?? command.first
|
ExecApprovalsStore.addAllowlistEntry(agentId: agentId, pattern: pattern)
|
||||||
?? ""
|
|
||||||
if !pattern.isEmpty {
|
|
||||||
ExecApprovalsStore.addAllowlistEntry(agentId: agentId, pattern: pattern)
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
if security == .allowlist, allowlistMatch == nil, !skillAllow, !approvedByAsk {
|
if security == .allowlist, allowlistMatch == nil, !skillAllow, !approvedByAsk {
|
||||||
@@ -807,12 +803,6 @@ extension MacNodeRuntime {
|
|||||||
UserDefaults.standard.object(forKey: cameraEnabledKey) as? Bool ?? false
|
UserDefaults.standard.object(forKey: cameraEnabledKey) as? Bool ?? false
|
||||||
}
|
}
|
||||||
|
|
||||||
private static func parseApprovalDecision(_ raw: String?) -> ExecApprovalDecision? {
|
|
||||||
let trimmed = raw?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
|
|
||||||
guard !trimmed.isEmpty else { return nil }
|
|
||||||
return ExecApprovalDecision(rawValue: trimmed)
|
|
||||||
}
|
|
||||||
|
|
||||||
private static let blockedEnvKeys: Set<String> = [
|
private static let blockedEnvKeys: Set<String> = [
|
||||||
"PATH",
|
"PATH",
|
||||||
"NODE_OPTIONS",
|
"NODE_OPTIONS",
|
||||||
|
|||||||
@@ -0,0 +1,60 @@
|
|||||||
|
import Foundation
|
||||||
|
import Testing
|
||||||
|
@testable import Clawdbot
|
||||||
|
|
||||||
|
@Suite struct ExecApprovalHelpersTests {
|
||||||
|
@Test func parseDecisionTrimsAndRejectsInvalid() {
|
||||||
|
#expect(ExecApprovalHelpers.parseDecision("allow-once") == .allowOnce)
|
||||||
|
#expect(ExecApprovalHelpers.parseDecision(" allow-always ") == .allowAlways)
|
||||||
|
#expect(ExecApprovalHelpers.parseDecision("deny") == .deny)
|
||||||
|
#expect(ExecApprovalHelpers.parseDecision("") == nil)
|
||||||
|
#expect(ExecApprovalHelpers.parseDecision("nope") == nil)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test func allowlistPatternPrefersResolution() {
|
||||||
|
let resolved = ExecCommandResolution(
|
||||||
|
rawExecutable: "rg",
|
||||||
|
resolvedPath: "/opt/homebrew/bin/rg",
|
||||||
|
executableName: "rg",
|
||||||
|
cwd: nil)
|
||||||
|
#expect(ExecApprovalHelpers.allowlistPattern(command: ["rg"], resolution: resolved) == resolved.resolvedPath)
|
||||||
|
|
||||||
|
let rawOnly = ExecCommandResolution(
|
||||||
|
rawExecutable: "rg",
|
||||||
|
resolvedPath: nil,
|
||||||
|
executableName: "rg",
|
||||||
|
cwd: nil)
|
||||||
|
#expect(ExecApprovalHelpers.allowlistPattern(command: ["rg"], resolution: rawOnly) == "rg")
|
||||||
|
#expect(ExecApprovalHelpers.allowlistPattern(command: ["rg"], resolution: nil) == "rg")
|
||||||
|
#expect(ExecApprovalHelpers.allowlistPattern(command: [], resolution: nil) == nil)
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test func requiresAskMatchesPolicy() {
|
||||||
|
let entry = ExecAllowlistEntry(pattern: "/bin/ls", lastUsedAt: nil, lastUsedCommand: nil, lastResolvedPath: nil)
|
||||||
|
#expect(ExecApprovalHelpers.requiresAsk(
|
||||||
|
ask: .always,
|
||||||
|
security: .deny,
|
||||||
|
allowlistMatch: nil,
|
||||||
|
skillAllow: false))
|
||||||
|
#expect(ExecApprovalHelpers.requiresAsk(
|
||||||
|
ask: .onMiss,
|
||||||
|
security: .allowlist,
|
||||||
|
allowlistMatch: nil,
|
||||||
|
skillAllow: false))
|
||||||
|
#expect(!ExecApprovalHelpers.requiresAsk(
|
||||||
|
ask: .onMiss,
|
||||||
|
security: .allowlist,
|
||||||
|
allowlistMatch: entry,
|
||||||
|
skillAllow: false))
|
||||||
|
#expect(!ExecApprovalHelpers.requiresAsk(
|
||||||
|
ask: .onMiss,
|
||||||
|
security: .allowlist,
|
||||||
|
allowlistMatch: nil,
|
||||||
|
skillAllow: true))
|
||||||
|
#expect(!ExecApprovalHelpers.requiresAsk(
|
||||||
|
ask: .off,
|
||||||
|
security: .allowlist,
|
||||||
|
allowlistMatch: nil,
|
||||||
|
skillAllow: false))
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user